On Tue, 22 Dec 2020 10:37:01 -0500 Tony Krowiak akrowiak@linux.ibm.com wrote:
On 12/21/20 11:05 PM, Halil Pasic wrote:
On Mon, 21 Dec 2020 13:56:25 -0500 Tony Krowiak akrowiak@linux.ibm.com wrote:
static int vfio_ap_mdev_group_notifier(struct notifier_block *nb, unsigned long action, void *data) {
- int ret;
- int ret, notify_rc = NOTIFY_DONE; struct ap_matrix_mdev *matrix_mdev;
if (action != VFIO_GROUP_NOTIFY_SET_KVM) return NOTIFY_OK; matrix_mdev = container_of(nb, struct ap_matrix_mdev, group_notifier);
- mutex_lock(&matrix_dev->lock);
if (!data) {
matrix_mdev->kvm = NULL;return NOTIFY_OK;
if (matrix_mdev->kvm)vfio_ap_mdev_unset_kvm(matrix_mdev);notify_rc = NOTIFY_OK;goto notify_done;ret = vfio_ap_mdev_set_kvm(matrix_mdev, data); if (ret)
return NOTIFY_DONE;
goto notify_done;/* If there is no CRYCB pointer, then we can't copy the masks */ if (!matrix_mdev->kvm->arch.crypto.crycbd)
return NOTIFY_DONE;
goto notify_done;kvm_arch_crypto_set_masks(matrix_mdev->kvm, matrix_mdev->matrix.apm, matrix_mdev->matrix.aqm, matrix_mdev->matrix.adm);
- return NOTIFY_OK;
Shouldn't there be an
- notify_rc = NOTIFY_OK;
here? I mean you initialize notify_rc to NOTIFY_DONE, in the !data branch on success you set notify_rc to NOTIFY_OK, but in the !!data branch it just stays NOTIFY_DONE. Or am I missing something?
I don't think it matters much since NOTIFY_OK and NOTIFY_DONE have no further effect on processing of the notification queue, but I believe you are correct, this is a change from what we originally had. I can restore the original return values if you'd prefer.
Even if they have the same semantics now, that might change in the future; restoring the original behaviour looks like the right thing to do.
Otherwise LGTM!
Same here.
Regards, Halil
+notify_done:
- mutex_unlock(&matrix_dev->lock);
- return notify_rc; }
[..]