[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <48221e73-4ea2-4b23-aa7d-53f485e42b12@linux.ibm.com>
Date: Tue, 4 Mar 2025 09:29:29 -0500
From: Rorie Reyes <rreyes@...ux.ibm.com>
To: Anthony Krowiak <akrowiak@...ux.ibm.com>, linux-s390@...r.kernel.org,
linux-kernel@...r.kernel.org, kvm@...r.kernel.org
Cc: hca@...ux.ibm.com, borntraeger@...ibm.com, agordeev@...ux.ibm.com,
gor@...ux.ibm.com, pasic@...ux.ibm.com, jjherne@...ux.ibm.com,
alex.williamson@...hat.com
Subject: Re: [RFC PATCH v1] fixup! s390/vfio-ap: Notify userspace that guest's
AP config changed when mdev removed
On 3/4/25 9:27 AM, Anthony Krowiak wrote:
>
>
>
> On 3/4/25 9:18 AM, Rorie Reyes wrote:
>>
>>
>> On 3/4/25 8:36 AM, Anthony Krowiak wrote:
>>>> static void vfio_ap_mdev_update_guest_apcb(struct
>>>> ap_matrix_mdev *matrix_mdev)
>>>> @@ -1870,7 +1870,6 @@ static void vfio_ap_mdev_unset_kvm(struct
>>>> ap_matrix_mdev *matrix_mdev)
>>>> get_update_locks_for_kvm(kvm);
>>>> kvm_arch_crypto_clear_masks(kvm);
>>>> - signal_guest_ap_cfg_changed(matrix_mdev);
>>>> vfio_ap_mdev_reset_queues(matrix_mdev);
>>>> kvm_put_kvm(kvm);
>>>> matrix_mdev->kvm = NULL;
>>>> @@ -2057,6 +2056,14 @@ static void vfio_ap_mdev_request(struct
>>>> vfio_device *vdev, unsigned int count)
>>>> matrix_mdev = container_of(vdev, struct ap_matrix_mdev, vdev);
>>>> + if (matrix_mdev->kvm) {
>>>> + get_update_locks_for_kvm(matrix_mdev->kvm);
>>>
>>> I know we talked about this prior to submission of this patch, but
>>> looking at this again I think
>>> you should use the get_update_locks_for_mdev() function for two
>>> reasons:
>>>
>>> 1. It is safer because it will take the matrix_dev->guests_lock
>>> which will prevent the matrix_mdev->kvm
>>> field from changing before you check it
>>>
>> So I'll replace *get_update_locks_for_kvm(matrix_mdev->kvm)* with
>> *get_update_locks_for_mdev(&matrix_dev->guests_lock)*
>
> See code below:get_update_locks_for_mdev(matrix_mdev)
> That function will take the guests_lock
>
Ah ok, I see now. I'll make those changes. Thank you!
>>> 2. I will eliminate the need for the else
>>>
>>> get_update_locks_for_mdev(matrix_mdev)
>>> if (matrix_mdev->kvm) {
>>> clear the masks
>>> signal guest config changed
>>> }
>>> ...
>>> release_update_locks_for_mdev(matrix_mdev); Sorry about not seeing
>>> this before you posted this patch.
>>>> + kvm_arch_crypto_clear_masks(matrix_mdev->kvm);
>>>> + signal_guest_ap_cfg_changed(matrix_mdev);
>>>> + } else {
>>>> + mutex_lock(&matrix_dev->mdevs_lock);
>>>> + }
>> So remove the else statement that contains the mutex function
>
Powered by blists - more mailing lists