Re: [PATCH 1/3] s390x/pci: avoid double enable/disable of aif

[Matthew Rosato]

On 1/17/24 5:54 AM, Cédric Le Goater wrote:
> On 1/16/24 23:31, Matthew Rosato wrote:
>> Use a flag to keep track of whether AIF is currently enabled.  This can be
>> used to avoid enabling/disabling AIF multiple times as well as to determine
>> whether or not it should be disabled during reset processing.
> 
> Why don't we disable AIF always at reset ? Doesn't KVM handle multiple calls
> to KVM_S390_ZPCIOP_DEREG_AEN cleanly ? Just asking, I am no expert there.
>

This may be some amount of defensive programming on my part :) Really, we're 
more concerned about enabling AIF twice without disabling AIF in between, and 
if we attempt to do so we should fail immediately rather than try messing with 
the hostdev.

The kernel warning you were seeing was exactly because we got the guest ISC 
users count wrong due to a mismatch between the number of enables and disables; 
in a sense, that warning is the KVM cleanup handling things (by disabling the 
gisc that AIF was using but also spitting out the warning that we got the gisc 
usage count wrong).  

I suspect you're right in that there's room to improve the KVM code so that we 
can catch this earlier rather than continuing to increment/decrement the guest 
ISC count and waiting to catch it at the end of the VM lifecycle (the GISC 
count is per-VM, but we should only ever have 1 instance per-device but are 
allowing >1 per-device).  I'll have a look at that separately from this series 
but IMHO QEMU should also try and 'behave' and be aware of what it previously 
enabled/disabled; if QEMU is trying to enable AIF multiple times then QEMU is 
doing something wrong.