[SRU][J:linux-bluefield][PATCH v3 3/6] UBUNTU: SAUCE: vfio_pci: Do not open code pci_try_reset_function()
William Tu
witu at nvidia.com
Wed Sep 11 14:53:35 UTC 2024
Hi Bartlomiej,
Thanks for your review!
Following the original patch
https://patchwork.kernel.org/project/linux-rdma/patch/3-v2-472615b3877e+28f7-vfio_dma_buf_jgg@nvidia.com/
The
vfio_pci_set_power_state(vdev, PCI_D0);
is no longer needed, as it is done in
pci_dev_save_and_disable
call trace is like
vfio_pci_try_reset_function
pci_dev_save_and_disable
pci_set_power_state(dev, PCI_D0);
Thanks
William
From: Bartlomiej Zolnierkiewicz <bartlomiej.zolnierkiewicz at canonical.com>
Date: Wednesday, September 11, 2024 at 12:52 AM
To: William Tu <witu at nvidia.com>
Cc: kernel-team at lists.ubuntu.com <kernel-team at lists.ubuntu.com>, Vladimir Sokolovsky <vlad at nvidia.com>, Bodong Wang <bodong at nvidia.com>, Sergey Gorenko <sergeygo at nvidia.com>, Jason Gunthorpe <jgg at nvidia.com>, Ziv Waksman <zwaksman at nvidia.com>
Subject: Re: [SRU][J:linux-bluefield][PATCH v3 3/6] UBUNTU: SAUCE: vfio_pci: Do not open code pci_try_reset_function()
External email: Use caution opening links or attachments
On Fri, Sep 6, 2024 at 9:50 PM William Tu <witu at nvidia.com> wrote:
>
> From: Jason Gunthorpe <jgg at nvidia.com>
>
> BugLink: https://bugs.launchpad.net/bugs/2077887
>
> FLR triggered by an emulated config space write should not behave
> differently from a FLR triggered by VFIO_DEVICE_RESET, currently the
> config space path misses the power management.
After this patch all paths seem to miss the power management, please see below.
> Consolidate all the call sites to invoke a single function.
>
> Signed-off-by: Jason Gunthorpe <jgg at nvidia.com>
> Signed-off-by: William Tu <witu at nvidia.com>
> ---
> drivers/vfio/pci/vfio_pci_config.c | 14 +++-------
> drivers/vfio/pci/vfio_pci_core.c | 41 +++++++++++++-----------------
> drivers/vfio/pci/vfio_pci_priv.h | 7 +++++
> 3 files changed, 28 insertions(+), 34 deletions(-)
> create mode 100644 drivers/vfio/pci/vfio_pci_priv.h
>
> diff --git a/drivers/vfio/pci/vfio_pci_config.c b/drivers/vfio/pci/vfio_pci_config.c
> index 6e58b4bf7a60..78c626b8907d 100644
> --- a/drivers/vfio/pci/vfio_pci_config.c
> +++ b/drivers/vfio/pci/vfio_pci_config.c
> @@ -857,11 +857,8 @@ static int vfio_exp_config_write(struct vfio_pci_core_device *vdev, int pos,
> pos - offset + PCI_EXP_DEVCAP,
> &cap);
>
> - if (!ret && (cap & PCI_EXP_DEVCAP_FLR)) {
> - vfio_pci_zap_and_down_write_memory_lock(vdev);
> - pci_try_reset_function(vdev->pdev);
> - up_write(&vdev->memory_lock);
> - }
> + if (!ret && (cap & PCI_EXP_DEVCAP_FLR))
> + vfio_pci_try_reset_function(vdev);
> }
>
> /*
> @@ -939,11 +936,8 @@ static int vfio_af_config_write(struct vfio_pci_core_device *vdev, int pos,
> pos - offset + PCI_AF_CAP,
> &cap);
>
> - if (!ret && (cap & PCI_AF_CAP_FLR) && (cap & PCI_AF_CAP_TP)) {
> - vfio_pci_zap_and_down_write_memory_lock(vdev);
> - pci_try_reset_function(vdev->pdev);
> - up_write(&vdev->memory_lock);
> - }
> + if (!ret && (cap & PCI_AF_CAP_FLR) && (cap & PCI_AF_CAP_TP))
> + vfio_pci_try_reset_function(vdev);
> }
>
> return count;
> diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
> index 4f1aceadd4fb..5904801f6f77 100644
> --- a/drivers/vfio/pci/vfio_pci_core.c
> +++ b/drivers/vfio/pci/vfio_pci_core.c
> @@ -29,6 +29,8 @@
>
> #include <linux/vfio_pci_core.h>
>
> +#include "vfio_pci_priv.h"
> +
> #define DRIVER_AUTHOR "Alex Williamson <alex.williamson at redhat.com>"
> #define DRIVER_DESC "core driver for VFIO based PCI devices"
>
> @@ -648,6 +650,20 @@ int vfio_pci_register_dev_region(struct vfio_pci_core_device *vdev,
> }
> EXPORT_SYMBOL_GPL(vfio_pci_register_dev_region);
>
> +int vfio_pci_try_reset_function(struct vfio_pci_core_device *vdev)
> +{
> + int ret;
> +
> + if (!vdev->reset_works)
> + return -EINVAL;
> +
> + vfio_pci_zap_and_down_write_memory_lock(vdev);
> + ret = pci_try_reset_function(vdev->pdev);
> + up_write(&vdev->memory_lock);
> +
> + return ret;
> +}
> +
> long vfio_pci_core_ioctl(struct vfio_device *core_vdev, unsigned int cmd,
> unsigned long arg)
> {
> @@ -929,30 +945,7 @@ long vfio_pci_core_ioctl(struct vfio_device *core_vdev, unsigned int cmd,
> return ret;
>
> } else if (cmd == VFIO_DEVICE_RESET) {
> - int ret;
> -
> - if (!vdev->reset_works)
> - return -EINVAL;
> -
> - vfio_pci_zap_and_down_write_memory_lock(vdev);
> -
> - /*
> - * This function can be invoked while the power state is non-D0.
> - * If pci_try_reset_function() has been called while the power
> - * state is non-D0, then pci_try_reset_function() will
> - * internally set the power state to D0 without vfio driver
> - * involvement. For the devices which have NoSoftRst-, the
> - * reset function can cause the PCI config space reset without
> - * restoring the original state (saved locally in
> - * 'vdev->pm_save').
> - */
> - vfio_pci_set_power_state(vdev, PCI_D0);
The above call is missing from vfio_pci_try_reset_function(), is this
correct/intended?
--
Best regards,
Bartlomiej
> -
> - ret = pci_try_reset_function(vdev->pdev);
> - up_write(&vdev->memory_lock);
> -
> - return ret;
> -
> + return vfio_pci_try_reset_function(vdev);
> } else if (cmd == VFIO_DEVICE_GET_PCI_HOT_RESET_INFO) {
> struct vfio_pci_hot_reset_info hdr;
> struct vfio_pci_fill_info fill = { 0 };
> diff --git a/drivers/vfio/pci/vfio_pci_priv.h b/drivers/vfio/pci/vfio_pci_priv.h
> new file mode 100644
> index 000000000000..4971cd95b431
> --- /dev/null
> +++ b/drivers/vfio/pci/vfio_pci_priv.h
> @@ -0,0 +1,7 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +#ifndef VFIO_PCI_PRIV_H
> +#define VFIO_PCI_PRIV_H
> +
> +int vfio_pci_try_reset_function(struct vfio_pci_core_device *vdev);
> +
> +#endif
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20240911/a9005065/attachment-0001.html>
More information about the kernel-team
mailing list