Re: [PATCH v5 5/9] s390/vfio-ap: Fix control domain removal in vfio_ap_mdev_cfg_remove

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] vfio_ap_mdev_hot_unplug_cfg incorrectly uses the return value of bitmap_andnot to track configuration changes, skipping KVM guest updates when the last AP resource is unplugged.
--

--- Patch [5]: [PATCH v5 5/9] s390/vfio-ap: Fix control domain removal in vfio_ap_mdev_cfg_remove ---
commit 2e94c01dff5c7206d21da99863677a285a17a31f
Author: Anthony Krowiak <[email protected]>
s390/vfio-ap: Fix control domain removal in vfio_ap_mdev_cfg_remove

This commit fixes the removal of control domains in the vfio_ap_mdev_cfg_remove
function. It correctly uses bitmap_and instead of bitmap_andnot to clear the
assigned control domains, allowing unplugged control domains to be properly
removed from the KVM guest.

> diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c
> index 21c502598f8ca..0f3537aadea87 100644
> --- a/drivers/s390/crypto/vfio_ap_ops.c
> +++ b/drivers/s390/crypto/vfio_ap_ops.c
> @@ -2636,9 +2636,9 @@ static void vfio_ap_mdev_cfg_remove(unsigned long *ap_remove,
>  		do_remove |= bitmap_and(aqrem, aq_remove,
>  					  matrix_mdev->matrix.aqm,
>  					  AP_DOMAINS);
> -		do_remove |= bitmap_andnot(cdrem, cd_remove,
> -					     matrix_mdev->matrix.adm,
> -					     AP_DOMAINS);
> +		do_remove |= bitmap_and(cdrem, cd_remove,
> +					matrix_mdev->matrix.adm,
> +					AP_DOMAINS);
>  
>  		if (do_remove)
>  			vfio_ap_mdev_hot_unplug_cfg(matrix_mdev, aprem, aqrem,

[Severity: High]
This is a pre-existing issue, but does vfio_ap_mdev_hot_unplug_cfg() correctly
handle configuration updates when the last AP resource is unplugged?

Looking at vfio_ap_mdev_hot_unplug_cfg():

drivers/s390/crypto/vfio_ap_ops.c:vfio_ap_mdev_hot_unplug_cfg() {
        int do_hotplug = 0;

        if (!bitmap_empty(aprem, AP_DEVICES)) {
                do_hotplug |= bitmap_andnot(matrix_mdev->shadow_apcb.apm,
                                            matrix_mdev->shadow_apcb.apm,
                                            aprem, AP_DEVICES);
        }

[ ... ]

        if (do_hotplug)
                vfio_ap_mdev_update_guest_apcb(matrix_mdev);
}

The bitmap_andnot() function returns true only if the resulting destination
bitmap is non-empty. If the last device or domain is removed, the resulting
bitmap becomes empty, meaning bitmap_andnot() returns false.

Consequently, do_hotplug evaluates to false, and the guest update function
vfio_ap_mdev_update_guest_apcb() is never called.

Could this skip updating the KVM guest, leaving it with access to AP hardware
that was hot-unplugged from the host configuration?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=5