Re: [PATCH] s390/vfio-ap: fix sysfs status attribute for AP queue devices

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2023-11-06 17:03, Tony Krowiak wrote:
PING
This patch is pretty straight forward, does anyone see a reason why
this shouldn't be integrated?

On 10/20/23 16:48, Tony Krowiak wrote:
The 'status' attribute for AP queue devices bound to the vfio_ap device driver displays incorrect status when the mediated device is attached to a
guest, but the queue device is not passed through. In the current
implementation, the status displayed is 'in_use' which is not correct; it
should be 'assigned'. This can happen if one of the queue devices
associated with a given adapter is not bound to the vfio_ap device driver.
For example:

Queues listed in /sys/bus/ap/drivers/vfio_ap:
14.0005
14.0006
14.000d
16.0006
16.000d

Queues listed in /sys/devices/vfio_ap/matrix/$UUID/matrix
14.0005
14.0006
14.000d
16.0005
16.0006
16.000d

Queues listed in /sys/devices/vfio_ap/matrix/$UUID/guest_matrix
14.0005
14.0006
14.000d

The reason no queues for adapter 0x16 are listed in the guest_matrix is
because queue 16.0005 is not bound to the vfio_ap device driver, so no
queue associated with the adapter is passed through to the guest;
therefore, each queue device for adapter 0x16 should display 'assigned' instead of 'in_use', because those queues are not in use by a guest, but
only assigned to the mediated device.

Let's check the AP configuration for the guest to determine whether a
queue device is passed through before displaying a status of 'in_use'.

Signed-off-by: Tony Krowiak <akrowiak@xxxxxxxxxxxxx>
Fixes: f139862b92cf ("s390/vfio-ap: add status attribute to AP queue device's sysfs dir")
Cc: stable@xxxxxxxxxxxxxxx
---
  drivers/s390/crypto/vfio_ap_ops.c | 7 ++++++-
  1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c
index 4db538a55192..871c14a6921f 100644
--- a/drivers/s390/crypto/vfio_ap_ops.c
+++ b/drivers/s390/crypto/vfio_ap_ops.c
@@ -1976,6 +1976,7 @@ static ssize_t status_show(struct device *dev,
  {
  	ssize_t nchars = 0;
  	struct vfio_ap_queue *q;
+	unsigned long apid, apqi;
  	struct ap_matrix_mdev *matrix_mdev;
  	struct ap_device *apdev = to_ap_dev(dev);
@@ -1984,7 +1985,11 @@ static ssize_t status_show(struct device *dev,
  	matrix_mdev = vfio_ap_mdev_for_queue(q);
    	if (matrix_mdev) {
-		if (matrix_mdev->kvm)
+		apid = AP_QID_CARD(q->apqn);
+		apqi = AP_QID_QUEUE(q->apqn);
+		if (matrix_mdev->kvm &&
+		    test_bit_inv(apid, matrix_mdev->shadow_apcb.apm) &&
+		    test_bit_inv(apqi, matrix_mdev->shadow_apcb.aqm))
  			nchars = scnprintf(buf, PAGE_SIZE, "%s\n",
  					   AP_QUEUE_IN_USE);
  		else

I can give you an
Acked-by: Harald Freudenberger <freude@xxxxxxxxxxxxx>
for this. Your explanation sounds sane to me and fixes a wrong
display. However, I am not familiar with the code so, I can't tell
if that's correct.
Just a remark: How can it happen that one queue is not bound to the vfio dd? Didn't we actively remove the unbind possibility from the sysfs for devices
assigned to the vfio dd?




[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux