Re: [PATCH v11 01/14] s390/vfio-ap: No need to disable IRQ after queue reset

From: Tony Krowiak
Date: Fri Oct 30 2020 - 16:53:12 EST




On 10/30/20 1:54 PM, Halil Pasic wrote:
On Thu, 29 Oct 2020 19:29:35 -0400
Tony Krowiak <akrowiak@xxxxxxxxxxxxx> wrote:

@@ -1177,7 +1166,10 @@ static int vfio_ap_mdev_reset_queues(struct mdev_device *mdev)
*/
if (ret)
rc = ret;
- vfio_ap_irq_disable_apqn(AP_MKQID(apid, apqi));
+ q = vfio_ap_get_queue(matrix_mdev,
+ AP_MKQID(apid, apqi));
+ if (q)
+ vfio_ap_free_aqic_resources(q);
[..]

Under what circumstances do we expect !q? If we don't, then we need to
complain one way or another.
In the current code (i.e., prior to introducing the subsequent hot
plug patches), an APQN can not be assigned to an mdev unless it
references a queue device bound to the vfio_ap device driver; however,
there is nothing preventing a queue device from getting unbound
while the guest is running (one of the problems mostly resolved by this
series). In that case, q would be NULL.
But if the queue does not belong to us any more it does not make sense
call vfio_ap_mdev_reset_queue() on it's APQN, or?

This is precisely why we prevent a queue from being taken away
from vfio_ap (the in-use callback) when its APQN is assigned to an
mdev in this patch series. On the other hand, this is a very good
point.


I think we should have

if(!q)
continue;
at the very beginning of the loop body, or we want to be sure that q is
not null.

I agree, I'll go ahead and make this change.