mbox series

[v2,vfio,0/2] vfio/pds: Fix and simplify resets

Message ID 20240228003205.47311-1-brett.creeley@amd.com (mailing list archive)
Headers show
Series vfio/pds: Fix and simplify resets | expand

Message

Brett Creeley Feb. 28, 2024, 12:32 a.m. UTC
This small series contains a fix and readability improvements for
resets.

v2:
- Split single patch into 2 patches
- Improve commit messages

v1:
https://lore.kernel.org/kvm/20240126183225.19193-1-brett.creeley@amd.com/

Brett Creeley (2):
  vfio/pds: Always clear the save/restore FDs on reset
  vfio/pds: Refactor/simplify reset logic

 drivers/vfio/pci/pds/pci_drv.c  |  2 +-
 drivers/vfio/pci/pds/vfio_dev.c | 14 +++++++-------
 drivers/vfio/pci/pds/vfio_dev.h |  7 ++++++-
 3 files changed, 14 insertions(+), 9 deletions(-)

Comments

Shameerali Kolothum Thodi Feb. 28, 2024, 9:05 a.m. UTC | #1
> -----Original Message-----
> From: Brett Creeley <brett.creeley@amd.com>
> Sent: Wednesday, February 28, 2024 12:32 AM
> To: jgg@ziepe.ca; yishaih@nvidia.com; Shameerali Kolothum Thodi
> <shameerali.kolothum.thodi@huawei.com>; kevin.tian@intel.com;
> alex.williamson@redhat.com; kvm@vger.kernel.org; linux-
> kernel@vger.kernel.org
> Cc: shannon.nelson@amd.com; brett.creeley@amd.com
> Subject: [PATCH v2 vfio 0/2] vfio/pds: Fix and simplify resets
> 
> This small series contains a fix and readability improvements for
> resets.
> 
> v2:
> - Split single patch into 2 patches
> - Improve commit messages

Just a query on the reset_done handler and the deferred_reset()
logic in this driver. From a quick look, it doesn't look like you have 
a condition where a copy_to/from_user() is under state_mutex. So
do you think we can get rid of the deferred_reset logic from this 
driver? Please see the discussion here,
https://lore.kernel.org/kvm/20240220132459.GM13330@nvidia.com/

For HiSilicon, we do have the lock taken for PRE_COPY, but that needs fixing
and then can get rid of the deferred_reset. I will sent out a patch for
that soon.

Thanks,
Shameer

> 
> v1:
> https://lore.kernel.org/kvm/20240126183225.19193-1-
> brett.creeley@amd.com/
> 
> Brett Creeley (2):
>   vfio/pds: Always clear the save/restore FDs on reset
>   vfio/pds: Refactor/simplify reset logic
> 
>  drivers/vfio/pci/pds/pci_drv.c  |  2 +-
>  drivers/vfio/pci/pds/vfio_dev.c | 14 +++++++-------
>  drivers/vfio/pci/pds/vfio_dev.h |  7 ++++++-
>  3 files changed, 14 insertions(+), 9 deletions(-)
> 
> --
> 2.17.1
Brett Creeley Feb. 28, 2024, 6:16 p.m. UTC | #2
On 2/28/2024 1:05 AM, Shameerali Kolothum Thodi wrote:
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
> 
> 
>> -----Original Message-----
>> From: Brett Creeley <brett.creeley@amd.com>
>> Sent: Wednesday, February 28, 2024 12:32 AM
>> To: jgg@ziepe.ca; yishaih@nvidia.com; Shameerali Kolothum Thodi
>> <shameerali.kolothum.thodi@huawei.com>; kevin.tian@intel.com;
>> alex.williamson@redhat.com; kvm@vger.kernel.org; linux-
>> kernel@vger.kernel.org
>> Cc: shannon.nelson@amd.com; brett.creeley@amd.com
>> Subject: [PATCH v2 vfio 0/2] vfio/pds: Fix and simplify resets
>>
>> This small series contains a fix and readability improvements for
>> resets.
>>
>> v2:
>> - Split single patch into 2 patches
>> - Improve commit messages
> 
> Just a query on the reset_done handler and the deferred_reset()
> logic in this driver. From a quick look, it doesn't look like you have
> a condition where a copy_to/from_user() is under state_mutex. So
> do you think we can get rid of the deferred_reset logic from this
> driver? Please see the discussion here,
> https://lore.kernel.org/kvm/20240220132459.GM13330@nvidia.com/
> 
> For HiSilicon, we do have the lock taken for PRE_COPY, but that needs fixing
> and then can get rid of the deferred_reset. I will sent out a patch for
> that soon.
> 
> Thanks,
> Shameer

Hi Shameer,

You are probably right that we can get rid of this logic, but the 
current 2 patch series is very simple and I would prefer to keep it that 
way. If you plan to make changes to the HiSilicon driver in the near 
future, then I can use that as a reference in enhancing the pds-vfio-pci 
driver.

Thanks,

Brett
> 
>>
>> v1:
>> https://lore.kernel.org/kvm/20240126183225.19193-1-
>> brett.creeley@amd.com/
>>
>> Brett Creeley (2):
>>    vfio/pds: Always clear the save/restore FDs on reset
>>    vfio/pds: Refactor/simplify reset logic
>>
>>   drivers/vfio/pci/pds/pci_drv.c  |  2 +-
>>   drivers/vfio/pci/pds/vfio_dev.c | 14 +++++++-------
>>   drivers/vfio/pci/pds/vfio_dev.h |  7 ++++++-
>>   3 files changed, 14 insertions(+), 9 deletions(-)
>>
>> --
>> 2.17.1
>
Alex Williamson March 1, 2024, 11:09 p.m. UTC | #3
On Tue, 27 Feb 2024 16:32:03 -0800
Brett Creeley <brett.creeley@amd.com> wrote:

> This small series contains a fix and readability improvements for
> resets.
> 
> v2:
> - Split single patch into 2 patches
> - Improve commit messages
> 
> v1:
> https://lore.kernel.org/kvm/20240126183225.19193-1-brett.creeley@amd.com/
> 
> Brett Creeley (2):
>   vfio/pds: Always clear the save/restore FDs on reset
>   vfio/pds: Refactor/simplify reset logic
> 
>  drivers/vfio/pci/pds/pci_drv.c  |  2 +-
>  drivers/vfio/pci/pds/vfio_dev.c | 14 +++++++-------
>  drivers/vfio/pci/pds/vfio_dev.h |  7 ++++++-
>  3 files changed, 14 insertions(+), 9 deletions(-)
> 

Applied to vfio next branch for v6.9.  Thanks,

Alex