On 3/17/21 7:17 PM, Halil Pasic wrote:
On Wed, 10 Mar 2021 10:05:59 -0500 Tony Krowiak akrowiak@linux.ibm.com wrote:
ret = vfio_ap_mdev_reset_queues(mdev);
matrix_mdev = mdev_get_drvdata(mdev);
Is it guaranteed that matrix_mdev can't be NULL here? If yes, please remind me of the mechanism that ensures this.
The matrix_mdev is set as drvdata when the mdev is created and is only cleared when the mdev is removed. Likewise, this function is a callback defined by by vfio in the vfio_ap_matrix_ops structure when the matrix_dev is registered and is intended to handle ioctl calls from userspace during the lifetime of the mdev. While I can't speak definitively to the guarantee, I think it is extremely unlikely that matrix_mdev would be NULL at this point. On the other hand, it wouldn't hurt to check for NULL and log an error or warning message (I prefer an error here) if NULL.
/*
* If the KVM pointer is in the process of being set, wait until
* the process has completed.
*/
wait_event_cmd(matrix_mdev->wait_for_kvm,
matrix_mdev->kvm_busy == false,
mutex_unlock(&matrix_dev->lock),
mutex_lock(&matrix_dev->lock));
if (matrix_mdev->kvm)
ret = vfio_ap_mdev_reset_queues(mdev);
else
ret = -ENODEV;
Didn't we agree to make the call to vfio_ap_mdev_reset_queues() unconditional again (for reference please take look at Message-ID: 64afa72c-2d6a-2ca1-e576-34e15fa579ed@linux.ibm.com)?
Yes, we did agree to that and I changed it at the time. That change got lost somehow; I'll reinstate it.
Regards, Halil