Re: [PATCH v3 8/9] s390: ap: Cleanup on removing the AP device

Pierre Morel
2019-02-19T19:41:32Z

On 16/02/2019 00:36, Tony Krowiak wrote:
> On 2/14/19 8:51 AM, Pierre Morel wrote:
>> When the device is remove, we must make sure to
>> clear the interruption and reset the AP device.
>>
...snip...

>> @@ -74,6 +159,13 @@ static void vfio_ap_queue_dev_remove(struct
>> ap_device *apdev)
>>       struct vfio_ap_queue *q;
>>       q = dev_get_drvdata(&apdev->device);
>> +    if (!q)
>> +        return;
>> +
>> +    vfio_ap_update_crycb(q);
>
> The root user is warned in the Limitations section of the vfio-ap.txt
> doc delivered with the AP pass-through support warns that the
> administrator (i.e., root user) should ensure that AP devices are not
> removed without taking proper care to ensure they are not in use by a
> guest. I am currently working on a patch set to handle this, so this
> may simply get ripped out when those patches are integrated. That may
> very well be simultaneously with this patch series as I plan on posting
> those soon.
>
> If this call is to remain, then you ought to update the vfio-ap.txt
> document to let users know that when queues are unbound, the guests
> will lose access to them unbeknown to the admin of the guest.

I do not see where is the problem, the admin should still take care the
APQN are not in use by the guest when he does an unbind.

This just makes sure it is not used anymore by the guest when the admin
rebound it to the host or another guest.

>
>> +    vfio_ap_zapq(q);
>
> One last thing. I've explained before that prior to the AP bus
> invoking this remove callback, it flushes and zeroizes the
> queue. Why do you insist it needs to be done again in the remove
> callback?

The problem is that the AP_BUS is not aware from the CRYCB and let the
guest play with the queue.

The sequence an be like:

-> AP_BUS remove RESET the queue and zeroes with ZAPQ
-> AP_BUS call remove from driver
- the APQN still belong to the guest !
-> the guest issue a NQAP
===> We need to take the queue away from the guest
===> and we need to RESET the queue with ZAPQ and wait
until no more message is in the queue
-> driver remove ends

Regards,
Pierre

--
Pierre Morel
Linux/KVM/QEMU in Böblingen - Germany