VF doesn't have the ability to reset itself completely which will cause the
hardware in unstable state. So notify PF driver when the VF has been reset
to let the PF resets the VF completely, and remove the VF out of schedule.
How to implement this?
Add the reset callback function in pci_driver
Implement the callback functin in VFIO_PCI driver.
Add the VF RESET IRQ for user mode driver to let the user mode driver
know the VF has been reset.
Signed-off-by: Emily Deng <[email protected]>
---
drivers/pci/pci.c | 8 ++++++++
include/linux/pci.h | 1 +
2 files changed, 9 insertions(+)
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 60230da957e0..aca937b05531 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -4780,6 +4780,14 @@ EXPORT_SYMBOL_GPL(pcie_flr);
*/
int pcie_reset_flr(struct pci_dev *dev, bool probe)
{
+ struct pci_dev *pf_dev;
+
+ if (dev->is_virtfn) {
+ pf_dev = dev->physfn;
+ if (pf_dev->driver->sriov_vf_reset_notification)
+ pf_dev->driver->sriov_vf_reset_notification(pf_dev, dev);
+ }
+
if (dev->dev_flags & PCI_DEV_FLAGS_NO_FLR_RESET)
return -ENOTTY;
diff --git a/include/linux/pci.h b/include/linux/pci.h
index c69a2cc1f412..4fa31d9b0aa7 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -926,6 +926,7 @@ struct pci_driver {
int (*sriov_configure)(struct pci_dev *dev, int num_vfs); /* On PF */
int (*sriov_set_msix_vec_count)(struct pci_dev *vf, int msix_vec_count); /* On PF */
u32 (*sriov_get_vf_total_msix)(struct pci_dev *pf);
+ void (*sriov_vf_reset_notification)(struct pci_dev *pf, struct pci_dev *vf);
const struct pci_error_handlers *err_handler;
const struct attribute_group **groups;
const struct attribute_group **dev_groups;
--
2.36.1
VF doesn't have the ability to reset itself completely which will cause the
hardware in unstable state. So notify PF driver when the VF has been reset
to let the PF resets the VF completely, and remove the VF out of schedule.
How to implement this?
Add the reset callback function in pci_driver
Implement the callback functin in VFIO_PCI driver.
Add the VF RESET IRQ for user mode driver to let the user mode driver
know the VF has been reset.
Signed-off-by: Emily Deng <[email protected]>
---
drivers/vfio/pci/vfio_pci.c | 14 ++++++++++++++
drivers/vfio/pci/vfio_pci_core.c | 26 ++++++++++++++++++++++++++
drivers/vfio/pci/vfio_pci_intrs.c | 30 ++++++++++++++++++++++++++++++
include/linux/vfio_pci_core.h | 1 +
include/uapi/linux/vfio.h | 1 +
5 files changed, 72 insertions(+)
diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
index 29091ee2e984..25b8d0a69532 100644
--- a/drivers/vfio/pci/vfio_pci.c
+++ b/drivers/vfio/pci/vfio_pci.c
@@ -185,6 +185,19 @@ static int vfio_pci_sriov_configure(struct pci_dev *pdev, int nr_virtfn)
return vfio_pci_core_sriov_configure(vdev, nr_virtfn);
}
+static void vfio_pci_vf_reset_notification(struct pci_dev *pf, struct pci_dev *vf)
+{
+ struct vfio_pci_core_device *vdev = dev_get_drvdata(&pf->dev);
+ int i = pci_iov_vf_id(vf);
+
+ mutex_lock(&vdev->igate);
+
+ if (pf->is_physfn && vdev->vf_reset_trigger && vdev->vf_reset_trigger[i])
+ eventfd_signal(vdev->vf_reset_trigger[i], 1);
+
+ mutex_unlock(&vdev->igate);
+}
+
static const struct pci_device_id vfio_pci_table[] = {
{ PCI_DRIVER_OVERRIDE_DEVICE_VFIO(PCI_ANY_ID, PCI_ANY_ID) }, /* match all by default */
{}
@@ -198,6 +211,7 @@ static struct pci_driver vfio_pci_driver = {
.probe = vfio_pci_probe,
.remove = vfio_pci_remove,
.sriov_configure = vfio_pci_sriov_configure,
+ .sriov_vf_reset_notification = vfio_pci_vf_reset_notification,
.err_handler = &vfio_pci_core_err_handlers,
.driver_managed_dma = true,
};
diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
index 20d7b69ea6ff..1740d41231c9 100644
--- a/drivers/vfio/pci/vfio_pci_core.c
+++ b/drivers/vfio/pci/vfio_pci_core.c
@@ -686,6 +686,7 @@ void vfio_pci_core_close_device(struct vfio_device *core_vdev)
{
struct vfio_pci_core_device *vdev =
container_of(core_vdev, struct vfio_pci_core_device, vdev);
+ int i;
if (vdev->sriov_pf_core_dev) {
mutex_lock(&vdev->sriov_pf_core_dev->vf_token->lock);
@@ -707,6 +708,17 @@ void vfio_pci_core_close_device(struct vfio_device *core_vdev)
eventfd_ctx_put(vdev->req_trigger);
vdev->req_trigger = NULL;
}
+
+ if (vdev->pdev->is_physfn) {
+ for (i = 0; i < pci_sriov_get_totalvfs(vdev->pdev); i++) {
+ if (vdev->vf_reset_trigger && vdev->vf_reset_trigger[i]) {
+ eventfd_ctx_put(vdev->vf_reset_trigger[i]);
+ vdev->vf_reset_trigger[i] = NULL;
+ }
+ }
+ if (vdev->vf_reset_trigger)
+ kfree(vdev->vf_reset_trigger);
+ }
mutex_unlock(&vdev->igate);
}
EXPORT_SYMBOL_GPL(vfio_pci_core_close_device);
@@ -718,6 +730,13 @@ void vfio_pci_core_finish_enable(struct vfio_pci_core_device *vdev)
eeh_dev_open(vdev->pdev);
#endif
+ if (vdev->pdev->is_physfn) {
+ vdev->vf_reset_trigger = kzalloc(pci_sriov_get_totalvfs(vdev->pdev) *
+ sizeof(*vdev->vf_reset_trigger), GFP_KERNEL);
+ if (!vdev->vf_reset_trigger)
+ pci_info(vdev->pdev, "%s: couldn't enable vf reset interrupt\n", __func__);
+ }
+
if (vdev->sriov_pf_core_dev) {
mutex_lock(&vdev->sriov_pf_core_dev->vf_token->lock);
vdev->sriov_pf_core_dev->vf_token->users++;
@@ -764,6 +783,9 @@ static int vfio_pci_get_irq_count(struct vfio_pci_core_device *vdev, int irq_typ
return 1;
} else if (irq_type == VFIO_PCI_REQ_IRQ_INDEX) {
return 1;
+ } else if (irq_type == VFIO_PCI_VF_RESET_IRQ_INDEX) {
+ if (vdev->pdev->is_physfn)
+ return pci_sriov_get_totalvfs(vdev->pdev);
}
return 0;
@@ -1141,6 +1163,10 @@ static int vfio_pci_ioctl_get_irq_info(struct vfio_pci_core_device *vdev,
if (pci_is_pcie(vdev->pdev))
break;
fallthrough;
+ case VFIO_PCI_VF_RESET_IRQ_INDEX:
+ if (vdev->pdev->is_physfn)
+ break;
+ fallthrough;
default:
return -EINVAL;
}
diff --git a/drivers/vfio/pci/vfio_pci_intrs.c b/drivers/vfio/pci/vfio_pci_intrs.c
index cbb4bcbfbf83..afca7eb1aa3a 100644
--- a/drivers/vfio/pci/vfio_pci_intrs.c
+++ b/drivers/vfio/pci/vfio_pci_intrs.c
@@ -776,6 +776,28 @@ static int vfio_pci_set_req_trigger(struct vfio_pci_core_device *vdev,
count, flags, data);
}
+static int vfio_pci_vf_reset_trigger(struct vfio_pci_core_device *vdev,
+ unsigned index, unsigned start,
+ unsigned count, uint32_t flags, void *data)
+{
+ int i;
+ int ret;
+ int *fd = data;
+
+ if (!vdev->vf_reset_trigger || index != VFIO_PCI_VF_RESET_IRQ_INDEX ||
+ start != 0 || count > pci_sriov_get_totalvfs(vdev->pdev))
+ return -EINVAL;
+
+ for (i = start; i < count; i++) {
+ ret = vfio_pci_set_ctx_trigger_single(&vdev->vf_reset_trigger[i],
+ 1, flags, &fd[i]);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
int vfio_pci_set_irqs_ioctl(struct vfio_pci_core_device *vdev, uint32_t flags,
unsigned index, unsigned start, unsigned count,
void *data)
@@ -825,6 +847,14 @@ int vfio_pci_set_irqs_ioctl(struct vfio_pci_core_device *vdev, uint32_t flags,
break;
}
break;
+ case VFIO_PCI_VF_RESET_IRQ_INDEX:
+ switch (flags & VFIO_IRQ_SET_ACTION_TYPE_MASK) {
+ case VFIO_IRQ_SET_ACTION_TRIGGER:
+ if (vdev->pdev->is_physfn)
+ func = vfio_pci_vf_reset_trigger;
+ break;
+ }
+ break;
}
if (!func)
diff --git a/include/linux/vfio_pci_core.h b/include/linux/vfio_pci_core.h
index 562e8754869d..f188c99dd82f 100644
--- a/include/linux/vfio_pci_core.h
+++ b/include/linux/vfio_pci_core.h
@@ -85,6 +85,7 @@ struct vfio_pci_core_device {
int ioeventfds_nr;
struct eventfd_ctx *err_trigger;
struct eventfd_ctx *req_trigger;
+ struct eventfd_ctx **vf_reset_trigger;
struct eventfd_ctx *pm_wake_eventfd_ctx;
struct list_head dummy_resources_list;
struct mutex ioeventfds_lock;
diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
index 20c804bdc09c..e2504182b34d 100644
--- a/include/uapi/linux/vfio.h
+++ b/include/uapi/linux/vfio.h
@@ -643,6 +643,7 @@ enum {
VFIO_PCI_MSIX_IRQ_INDEX,
VFIO_PCI_ERR_IRQ_INDEX,
VFIO_PCI_REQ_IRQ_INDEX,
+ VFIO_PCI_VF_RESET_IRQ_INDEX,
VFIO_PCI_NUM_IRQS
};
--
2.36.1
On Mon, Feb 05, 2024 at 03:15:37PM +0800, Emily Deng wrote:
> VF doesn't have the ability to reset itself completely which will cause the
> hardware in unstable state. So notify PF driver when the VF has been reset
> to let the PF resets the VF completely, and remove the VF out of schedule.
I'm sorry but this explanation is not different from the previous
version. Please provide a better explanation of the problem, why it is
needed, which VFs need and can't reset themselves, how and why it worked
before e.t.c.
In addition, please follow kernel submission guidelines, write
changelong, add versions, cover letter e.t.c.
Thanks
>
> How to implement this?
> Add the reset callback function in pci_driver
>
> Implement the callback functin in VFIO_PCI driver.
>
> Add the VF RESET IRQ for user mode driver to let the user mode driver
> know the VF has been reset.
>
> Signed-off-by: Emily Deng <[email protected]>
> ---
> drivers/pci/pci.c | 8 ++++++++
> include/linux/pci.h | 1 +
> 2 files changed, 9 insertions(+)
>
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 60230da957e0..aca937b05531 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -4780,6 +4780,14 @@ EXPORT_SYMBOL_GPL(pcie_flr);
> */
> int pcie_reset_flr(struct pci_dev *dev, bool probe)
> {
> + struct pci_dev *pf_dev;
> +
> + if (dev->is_virtfn) {
> + pf_dev = dev->physfn;
> + if (pf_dev->driver->sriov_vf_reset_notification)
> + pf_dev->driver->sriov_vf_reset_notification(pf_dev, dev);
> + }
> +
> if (dev->dev_flags & PCI_DEV_FLAGS_NO_FLR_RESET)
> return -ENOTTY;
>
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index c69a2cc1f412..4fa31d9b0aa7 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -926,6 +926,7 @@ struct pci_driver {
> int (*sriov_configure)(struct pci_dev *dev, int num_vfs); /* On PF */
> int (*sriov_set_msix_vec_count)(struct pci_dev *vf, int msix_vec_count); /* On PF */
> u32 (*sriov_get_vf_total_msix)(struct pci_dev *pf);
> + void (*sriov_vf_reset_notification)(struct pci_dev *pf, struct pci_dev *vf);
> const struct pci_error_handlers *err_handler;
> const struct attribute_group **groups;
> const struct attribute_group **dev_groups;
> --
> 2.36.1
>
>
On Mon, 5 Feb 2024 15:15:37 +0800
Emily Deng <[email protected]> wrote:
> VF doesn't have the ability to reset itself completely which will cause the
> hardware in unstable state. So notify PF driver when the VF has been reset
> to let the PF resets the VF completely, and remove the VF out of schedule.
>
> How to implement this?
> Add the reset callback function in pci_driver
>
> Implement the callback functin in VFIO_PCI driver.
>
> Add the VF RESET IRQ for user mode driver to let the user mode driver
> know the VF has been reset.
The solution that already exists for this sort of issue is a vfio-pci
variant driver for the VF which communicates with an in-kernel PF
driver to coordinate the VF FLR with the PF driver. This can be done
by intercepting the userspace access to the VF FLR config space region.
This solution of involving PCI-core and extending the vfio-pci interface
only exists for userspace PF drivers. I don't see that facilitating
vendors to implement their PF drivers in userspace to avoid upstreaming
is a compelling reason to extend the vfio-pci interface. Thanks,
Alex
> Signed-off-by: Emily Deng <[email protected]>
> ---
> drivers/pci/pci.c | 8 ++++++++
> include/linux/pci.h | 1 +
> 2 files changed, 9 insertions(+)
>
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 60230da957e0..aca937b05531 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -4780,6 +4780,14 @@ EXPORT_SYMBOL_GPL(pcie_flr);
> */
> int pcie_reset_flr(struct pci_dev *dev, bool probe)
> {
> + struct pci_dev *pf_dev;
> +
> + if (dev->is_virtfn) {
> + pf_dev = dev->physfn;
> + if (pf_dev->driver->sriov_vf_reset_notification)
> + pf_dev->driver->sriov_vf_reset_notification(pf_dev, dev);
> + }
> +
> if (dev->dev_flags & PCI_DEV_FLAGS_NO_FLR_RESET)
> return -ENOTTY;
>
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index c69a2cc1f412..4fa31d9b0aa7 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -926,6 +926,7 @@ struct pci_driver {
> int (*sriov_configure)(struct pci_dev *dev, int num_vfs); /* On PF */
> int (*sriov_set_msix_vec_count)(struct pci_dev *vf, int msix_vec_count); /* On PF */
> u32 (*sriov_get_vf_total_msix)(struct pci_dev *pf);
> + void (*sriov_vf_reset_notification)(struct pci_dev *pf, struct pci_dev *vf);
> const struct pci_error_handlers *err_handler;
> const struct attribute_group **groups;
> const struct attribute_group **dev_groups;
On Tue, 6 Feb 2024 04:03:46 +0000
"Liu, Monk" <[email protected]> wrote:
> [AMD Official Use Only - General]
>
> Hi Alex
>
> Thanks for your comment, I’m still not quite following you.
>
> >>This can be done by intercepting the userspace access to the VF FLR config space region.
>
> Would you mind let us know to do that ?
See basically any of the vfio-pci variant drivers in drivers/vfio/pci/*/
For instance the virtio-vfio-pci driver is emulating a PCI IO BAR in
config space and accesses through that BAR interact with the in-kernel
PF driver.
> our scenario is that:
> 1, vf already pass-throughed to qemu
> 2, a user mode driver on PF’s vfio arch is running there, and it want
> to receive VF’s reset request (by Qemu through vfio interface), and
> do some hw/fw sequences to achieve the real Vf reset goal
>
> >>. I don't see that facilitating vendors to implement their PF
> >>drivers in userspace to avoid upstreaming
> is a compelling reason to extend the vfio-pci interface.
>
> Some background here (our user mode PF driver is not given the
> purpose to avoid upstream): We don’t see value to upstream a user
> mode pf driver that only benefit AMD device, as it must be out of
> kernel-tree so we don’t see where the appropriate repo for it is to
> upstream to … (we cannot make it in Qemu right ? otherwise Qemu will
> be over designed if it knows hw/fw details of vendors)
An in-kernel driver within the mainline kernel is the right place for
it. I'm not asking to upstream a user mode driver, you're right that
there is no repo for that, I'm questioning the motivation for making it
a user mode PF driver in the first place.
All device drivers are essentially meant to benefit the device vendor,
but in-kernel drivers also offer a benefit to the community and users of
those devices for ongoing support and development.
Let me turn the question around, what benefit does it provide this
PF driver to exist in userspace?
Without having seen it, I'd venture that there's nothing this userspace
PF driver could do that couldn't also be done via a kernel driver,
either in-tree or out-of-tree, but the userspace driver avoids the
upstreaming work of an in-tree driver and the tainting of an
out-of-tree driver.
> Isn’t VFIO arch intentionally to give user mode driver freedom and
> ability to manipulate the HW ?
VFIO is not intended to provide an alternative means to implement
kernel drivers. VFIO is intended to provide secure, isolated access to
devices for userspace drivers.
What's the isolation relative to the VF if this PF driver needs to be
involved in reset? How much VF data does the PF device have access to?
This is the reason that vfio-pci introduced the vf-token barrier with
SR-IOV support. The intention of this vf-token is to indicate a gap in
trust. Only another userspace process knowing the vf-token configured
by the PF userspace driver can access the device. We should not
normalize vf-tokens.
The requirement of a vf-token for a driver not strictly developed
alongside the PF userspace driver should effectively be considered a
tainted device.
Therefore, what does this userspace PF driver offer that isn't better
reflected using the existing vfio-pci-core code split to implement a
vfio-pci variant driver, which has no need for the extension proposed
here? Thanks,
Alex
> From: Alex Williamson <[email protected]>
> Date: Tuesday, February 6, 2024 at 00:43
> To: Deng, Emily <[email protected]>
> Cc: [email protected] <[email protected]>,
> [email protected] <[email protected]>,
> [email protected] <[email protected]>,
> [email protected] <[email protected]>, Jiang, Jerry (SW)
> <[email protected]>, Zhang, Andy <[email protected]>, Chang,
> HaiJun <[email protected]>, Liu, Monk <[email protected]>, Chen,
> Horace <[email protected]>, Yin, ZhenGuo (Chris)
> <[email protected]> Subject: Re: [PATCH 1/2] PCI: Add VF reset
> notification to PF's VFIO user mode driver On Mon, 5 Feb 2024
> 15:15:37 +0800 Emily Deng <[email protected]> wrote:
>
> > VF doesn't have the ability to reset itself completely which will
> > cause the hardware in unstable state. So notify PF driver when the
> > VF has been reset to let the PF resets the VF completely, and
> > remove the VF out of schedule.
> >
> > How to implement this?
> > Add the reset callback function in pci_driver
> >
> > Implement the callback functin in VFIO_PCI driver.
> >
> > Add the VF RESET IRQ for user mode driver to let the user mode
> > driver know the VF has been reset.
>
> The solution that already exists for this sort of issue is a vfio-pci
> variant driver for the VF which communicates with an in-kernel PF
> driver to coordinate the VF FLR with the PF driver. This can be done
> by intercepting the userspace access to the VF FLR config space
> region.
>
> This solution of involving PCI-core and extending the vfio-pci
> interface only exists for userspace PF drivers. I don't see that
> facilitating vendors to implement their PF drivers in userspace to
> avoid upstreaming is a compelling reason to extend the vfio-pci
> interface. Thanks,
>
> Alex
>
> > Signed-off-by: Emily Deng <[email protected]>
> > ---
> > drivers/pci/pci.c | 8 ++++++++
> > include/linux/pci.h | 1 +
> > 2 files changed, 9 insertions(+)
> >
> > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> > index 60230da957e0..aca937b05531 100644
> > --- a/drivers/pci/pci.c
> > +++ b/drivers/pci/pci.c
> > @@ -4780,6 +4780,14 @@ EXPORT_SYMBOL_GPL(pcie_flr);
> > */
> > int pcie_reset_flr(struct pci_dev *dev, bool probe)
> > {
> > + struct pci_dev *pf_dev;
> > +
> > + if (dev->is_virtfn) {
> > + pf_dev = dev->physfn;
> > + if (pf_dev->driver->sriov_vf_reset_notification)
> > +
> > pf_dev->driver->sriov_vf_reset_notification(pf_dev, dev);
> > + }
> > +
> > if (dev->dev_flags & PCI_DEV_FLAGS_NO_FLR_RESET)
> > return -ENOTTY;
> >
> > diff --git a/include/linux/pci.h b/include/linux/pci.h
> > index c69a2cc1f412..4fa31d9b0aa7 100644
> > --- a/include/linux/pci.h
> > +++ b/include/linux/pci.h
> > @@ -926,6 +926,7 @@ struct pci_driver {
> > int (*sriov_configure)(struct pci_dev *dev, int num_vfs);
> > /* On PF */ int (*sriov_set_msix_vec_count)(struct pci_dev *vf,
> > int msix_vec_count); /* On PF */ u32
> > (*sriov_get_vf_total_msix)(struct pci_dev *pf);
> > + void (*sriov_vf_reset_notification)(struct pci_dev *pf,
> > struct pci_dev *vf); const struct pci_error_handlers *err_handler;
> > const struct attribute_group **groups;
> > const struct attribute_group **dev_groups;
On Tue, 6 Feb 2024 04:08:18 +0000
"Liu, Monk" <[email protected]> wrote:
> [AMD Official Use Only - General]
>
> Hi Leon
>
> The thing is when qemu reset a VM it calls vfio’s reset ioctl to the
> given VF device, and in kernel the VFIO-pci module will do the reset
> to that VF device via its PCI config space register, but
> unfortunately our VF GPU isnot designed to support those
> “reset”/”flr” commands … not supported by the VF, (and even many PF
> cannot handle those commands well)
PFs are not required to implement FLR, VFs are.
SR-IOV spec, rev. 1.1:
2.2.2. FLR That Targets a VF
VFs must support Function Level Reset (FLR).
>
> So the idea we can cook up is to move those Vf’s reset notification
> to our PF driver (which is a user mode driver running on PF’s VFIO
> arch), and our user mode driver can program HW and do the reset for
> that VF.
The PF driver being able to arbitrarily reset a VF device provided to
another userspace process doesn't sound like separate, isolated
devices. What else can the PF access?
As noted in my other reply, vf-tokens should not be normalized and
users of VFs provided by third party userspace PF drivers should
consider the device no less tainted than if it were provided by an
out-of-tree kernel driver.
The idea to virtualize FLR on the VF to resolve the hardware defect is a
good one, but it should be done in the context of a vfio-pci variant
driver. Thanks,
Alex
> From: Leon Romanovsky <[email protected]>
> Date: Monday, February 5, 2024 at 17:04
> To: Deng, Emily <[email protected]>
> Cc: [email protected] <[email protected]>,
> [email protected] <[email protected]>,
> [email protected] <[email protected]>,
> [email protected] <[email protected]>,
> [email protected] <[email protected]>, Jiang, Jerry (SW)
> <[email protected]>, Zhang, Andy <[email protected]>, Chang,
> HaiJun <[email protected]>, Liu, Monk <[email protected]>, Chen,
> Horace <[email protected]>, Yin, ZhenGuo (Chris)
> <[email protected]> Subject: Re: [PATCH 1/2] PCI: Add VF reset
> notification to PF's VFIO user mode driver On Mon, Feb 05, 2024 at
> 03:15:37PM +0800, Emily Deng wrote:
> > VF doesn't have the ability to reset itself completely which will
> > cause the hardware in unstable state. So notify PF driver when the
> > VF has been reset to let the PF resets the VF completely, and
> > remove the VF out of schedule.
>
>
> I'm sorry but this explanation is not different from the previous
> version. Please provide a better explanation of the problem, why it is
> needed, which VFs need and can't reset themselves, how and why it
> worked before e.t.c.
>
> In addition, please follow kernel submission guidelines, write
> changelong, add versions, cover letter e.t.c.
>
> Thanks
>
> >
> > How to implement this?
> > Add the reset callback function in pci_driver
> >
> > Implement the callback functin in VFIO_PCI driver.
> >
> > Add the VF RESET IRQ for user mode driver to let the user mode
> > driver know the VF has been reset.
> >
> > Signed-off-by: Emily Deng <[email protected]>
> > ---
> > drivers/pci/pci.c | 8 ++++++++
> > include/linux/pci.h | 1 +
> > 2 files changed, 9 insertions(+)
> >
> > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> > index 60230da957e0..aca937b05531 100644
> > --- a/drivers/pci/pci.c
> > +++ b/drivers/pci/pci.c
> > @@ -4780,6 +4780,14 @@ EXPORT_SYMBOL_GPL(pcie_flr);
> > */
> > int pcie_reset_flr(struct pci_dev *dev, bool probe)
> > {
> > + struct pci_dev *pf_dev;
> > +
> > + if (dev->is_virtfn) {
> > + pf_dev = dev->physfn;
> > + if (pf_dev->driver->sriov_vf_reset_notification)
> > +
> > pf_dev->driver->sriov_vf_reset_notification(pf_dev, dev);
> > + }
> > +
> > if (dev->dev_flags & PCI_DEV_FLAGS_NO_FLR_RESET)
> > return -ENOTTY;
> >
> > diff --git a/include/linux/pci.h b/include/linux/pci.h
> > index c69a2cc1f412..4fa31d9b0aa7 100644
> > --- a/include/linux/pci.h
> > +++ b/include/linux/pci.h
> > @@ -926,6 +926,7 @@ struct pci_driver {
> > int (*sriov_configure)(struct pci_dev *dev, int num_vfs);
> > /* On PF */ int (*sriov_set_msix_vec_count)(struct pci_dev *vf,
> > int msix_vec_count); /* On PF */ u32
> > (*sriov_get_vf_total_msix)(struct pci_dev *pf);
> > + void (*sriov_vf_reset_notification)(struct pci_dev *pf,
> > struct pci_dev *vf); const struct pci_error_handlers *err_handler;
> > const struct attribute_group **groups;
> > const struct attribute_group **dev_groups;
> > --
> > 2.36.1
> >
> >