linux-kernel - Re: [RFC PATCH v1] fixup! s390/vfio-ap: Notify userspace that guest's AP config changed when mdev removed

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <d5308330-23f3-4f55-836c-0c6e884587b0@linux.ibm.com>
Date: Tue, 4 Mar 2025 09:27:05 -0500
From: Anthony Krowiak <akrowiak@...ux.ibm.com>
To: Rorie Reyes <rreyes@...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: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

>> 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