[PATCH 1/2] PCI: Device driver function reset notification

[PATCH 1/2] PCI: Device driver function reset notification

[Keith Busch]

A user can issue a pci function level reset to a device using sysfs entry
/sys/bus/pci/devices/.../reset. A kernel driver handling the pci device
might like to know this reset is about to occur and when the reset attempt
completes. This is so the driver has a chance to take appropriate device
specific actions; for example, it may need to quiesce before the reset,
and reinitialize the device after.

Signed-off-by: Keith Busch <keith.busch@intel.com>
---
 drivers/pci/pci.c   |   13 +++++++++++++
 include/linux/pci.h |    3 +++
 2 files changed, 16 insertions(+)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index fdbc294..cb24bbe 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -3320,6 +3320,15 @@ static int pci_dev_reset(struct pci_dev *dev, int probe)
 
 	return rc;
 }
+
+static void pci_reset_notify(struct pci_dev *dev, bool prepare)
+{
+	const struct pci_error_handlers *err_handler =
+			dev->driver ? dev->driver->err_handler : NULL;
+	if (err_handler && err_handler->reset_notify)
+		err_handler->reset_notify(dev, prepare);
+}
+
 /**
  * __pci_reset_function - reset a PCI device function
  * @dev: PCI device to reset
@@ -3408,11 +3417,13 @@ int pci_reset_function(struct pci_dev *dev)
 	if (rc)
 		return rc;
 
+	pci_reset_notify(dev, true);
 	pci_dev_save_and_disable(dev);
 
 	rc = pci_dev_reset(dev, 0);
 
 	pci_dev_restore(dev);
+	pci_reset_notify(dev, false);
 
 	return rc;
 }
@@ -3432,6 +3443,7 @@ int pci_try_reset_function(struct pci_dev *dev)
 	if (rc)
 		return rc;
 
+	pci_reset_notify(dev, true);
 	pci_dev_save_and_disable(dev);
 
 	if (pci_dev_trylock(dev)) {
@@ -3441,6 +3453,7 @@ int pci_try_reset_function(struct pci_dev *dev)
 		rc = -EAGAIN;
 
 	pci_dev_restore(dev);
+	pci_reset_notify(dev, false);
 
 	return rc;
 }
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 33aa2ca..d82dd3f 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -603,6 +603,9 @@ struct pci_error_handlers {
 	/* PCI slot has been reset */
 	pci_ers_result_t (*slot_reset)(struct pci_dev *dev);
 
+	/* PCI function reset prepare or completed */
+	void (*reset_notify)(struct pci_dev *dev, bool prepare);
+
 	/* Device driver may resume normal operations */
 	void (*resume)(struct pci_dev *dev);
 };
-- 
1.7.10.4

[PATCH 2/2] NVMe: Implement PCI-e reset notification callback

[Keith Busch]

Disable and shutdown the device prior to reset, and restart the device
after.

Signed-off-by: Keith Busch <keith.busch@intel.com>
---
 drivers/block/nvme-core.c |   11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/block/nvme-core.c b/drivers/block/nvme-core.c
index 625259d..273ff12 100644
--- a/drivers/block/nvme-core.c
+++ b/drivers/block/nvme-core.c
@@ -2605,6 +2605,16 @@ static void nvme_reset_failed_dev(struct work_struct *ws)
 	nvme_dev_reset(dev);
 }
 
+static void nvme_reset_notify(struct pci_dev *pdev, int prepare)
+{
+	struct nvme_dev *dev = pci_get_drvdata(pdev);
+
+	if (prepare)
+		nvme_dev_shutdown(dev);
+	else
+		nvme_dev_resume(dev);
+}
+
 static int nvme_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 {
 	int result = -ENOMEM;
@@ -2744,6 +2754,7 @@ static const struct pci_error_handlers nvme_err_handler = {
 	.link_reset	= nvme_link_reset,
 	.slot_reset	= nvme_slot_reset,
 	.resume		= nvme_error_resume,
+	.reset_notify	= nvme_reset_notify,
 };
 
 /* Move to pci_ids.h later */
-- 
1.7.10.4

Re: [PATCH 2/2] NVMe: Implement PCI-e reset notification callback

[Learner Study]

Hi Keith,

I think NVMe Reset should apply to PF mode driver only, and not to VF
mode driver.
Is that understanding correct? Does the NVMe driver know which mode
its running in?

Thanks!


On Tue, Apr 8, 2014 at 4:42 PM, Keith Busch <keith.busch@intel.com> wrote:
>
> Disable and shutdown the device prior to reset, and restart the device
> after.
>
> Signed-off-by: Keith Busch <keith.busch@intel.com>
> ---
>  drivers/block/nvme-core.c |   11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/drivers/block/nvme-core.c b/drivers/block/nvme-core.c
> index 625259d..273ff12 100644
> --- a/drivers/block/nvme-core.c
> +++ b/drivers/block/nvme-core.c
> @@ -2605,6 +2605,16 @@ static void nvme_reset_failed_dev(struct work_struct *ws)
>         nvme_dev_reset(dev);
>  }
>
> +static void nvme_reset_notify(struct pci_dev *pdev, int prepare)
> +{
> +       struct nvme_dev *dev = pci_get_drvdata(pdev);
> +
> +       if (prepare)
> +               nvme_dev_shutdown(dev);
> +       else
> +               nvme_dev_resume(dev);
> +}
> +
>  static int nvme_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>  {
>         int result = -ENOMEM;
> @@ -2744,6 +2754,7 @@ static const struct pci_error_handlers nvme_err_handler = {
>         .link_reset     = nvme_link_reset,
>         .slot_reset     = nvme_slot_reset,
>         .resume         = nvme_error_resume,
> +       .reset_notify   = nvme_reset_notify,
>  };
>
>  /* Move to pci_ids.h later */
> --
> 1.7.10.4
>
>
> _______________________________________________
> Linux-nvme mailing list
> Linux-nvme@lists.infradead.org
> http://merlin.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCH 2/2] NVMe: Implement PCI-e reset notification callback

[Keith Busch]

On Mon, 21 Apr 2014, Learner Study wrote:
> Hi Keith,
>
> I think NVMe Reset should apply to PF mode driver only, and not to VF
> mode driver.
> Is that understanding correct? Does the NVMe driver know which mode
> its running in?

Oh, this driver doesn't enable SR-IOV and has no PF/VF awareness. Shame
on us, I'll add it to my list unless someone beats me to it.

Still, I think we'd like to be able to reset a VF if only because it
gets the queues back to a pristine state. A VF reset should not affect
any of the other functions.

Re: [PATCH 2/2] NVMe: Implement PCI-e reset notification callback

[Learner]

But Won't resetting from a VF impact functionality of other VFs?

On Apr 21, 2014, at 6:57 PM, Keith Busch <keith.busch@intel.com> wrote:

> On Mon, 21 Apr 2014, Learner Study wrote:
>> Hi Keith,
>> 
>> I think NVMe Reset should apply to PF mode driver only, and not to VF
>> mode driver.
>> Is that understanding correct? Does the NVMe driver know which mode
>> its running in?
> 
> Oh, this driver doesn't enable SR-IOV and has no PF/VF awareness. Shame
> on us, I'll add it to my list unless someone beats me to it.
> 
> Still, I think we'd like to be able to reset a VF if only because it
> gets the queues back to a pristine state. A VF reset should not affect
> any of the other functions.

Re: [PATCH 2/2] NVMe: Implement PCI-e reset notification callback

[Learner Study]

Per the spec, "When an NVM Subsystem Reset occurs, the entire NVM
subsystem is reset"....so all VFs would get impacted, if a VF does a
Reset. So, I think Reset kind of control should be in PF mode only.

On Mon, Apr 21, 2014 at 7:45 PM, Learner <learner.study@gmail.com> wrote:
> But Won't resetting from a VF impact functionality of other VFs?
>
> On Apr 21, 2014, at 6:57 PM, Keith Busch <keith.busch@intel.com> wrote:
>
>> On Mon, 21 Apr 2014, Learner Study wrote:
>>> Hi Keith,
>>>
>>> I think NVMe Reset should apply to PF mode driver only, and not to VF
>>> mode driver.
>>> Is that understanding correct? Does the NVMe driver know which mode
>>> its running in?
>>
>> Oh, this driver doesn't enable SR-IOV and has no PF/VF awareness. Shame
>> on us, I'll add it to my list unless someone beats me to it.
>>
>> Still, I think we'd like to be able to reset a VF if only because it
>> gets the queues back to a pristine state. A VF reset should not affect
>> any of the other functions.

RE: [PATCH 2/2] NVMe: Implement PCI-e reset notification callback

[Mayes, Barrett N]

Section 8.5 of the NVMe 1.1 spec contains the following:  "While the details associated with implementing a controller that supports SR-IOV are outside the scope of this specification, such a controller shall implement fully compliant NVM Express Virtual Functions (VFs). This ensures that the same host software developed for non-virtualized environments is capable of running unmodified within an SI. No such requirement exists for the Physical Function (PF)."

If a VF is a fully NVMe compliant device then it must at least act like it implements NVMe subsystem reset.  How the SR-IOV-capable controller actually implements this is left up to the vendor.  But the spec does not require that a reset of one VF initiate a reset of other VF.

-----Original Message-----
From: Linux-nvme [mailto:linux-nvme-bounces@lists.infradead.org] On Behalf Of Learner Study
Sent: Monday, April 21, 2014 8:34 PM
To: Busch, Keith
Cc: bhelgaas@google.com; linux-pci@vger.kernel.org; linux-nvme
Subject: Re: [PATCH 2/2] NVMe: Implement PCI-e reset notification callback

Per the spec, "When an NVM Subsystem Reset occurs, the entire NVM subsystem is reset"....so all VFs would get impacted, if a VF does a Reset. So, I think Reset kind of control should be in PF mode only.

On Mon, Apr 21, 2014 at 7:45 PM, Learner <learner.study@gmail.com> wrote:
> But Won't resetting from a VF impact functionality of other VFs?
>
> On Apr 21, 2014, at 6:57 PM, Keith Busch <keith.busch@intel.com> wrote:
>
>> On Mon, 21 Apr 2014, Learner Study wrote:
>>> Hi Keith,
>>>
>>> I think NVMe Reset should apply to PF mode driver only, and not to 
>>> VF mode driver.
>>> Is that understanding correct? Does the NVMe driver know which mode 
>>> its running in?
>>
>> Oh, this driver doesn't enable SR-IOV and has no PF/VF awareness. 
>> Shame on us, I'll add it to my list unless someone beats me to it.
>>
>> Still, I think we'd like to be able to reset a VF if only because it 
>> gets the queues back to a pristine state. A VF reset should not 
>> affect any of the other functions.

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCH 2/2] NVMe: Implement PCI-e reset notification callback

[Learner Study]

It would be interesting to see which way the standard open source
driver decides to go...would we allow Reset in one VF to impact
operations in other VFs (as per spec 7.3.1 of NVMe 1.1 spec, "When an
NVM Subsystem Reset occurs, the entire NVM subsystem is reset"...)

On Mon, Apr 21, 2014 at 10:02 PM, Mayes, Barrett N
<barrett.n.mayes@intel.com> wrote:
> Section 8.5 of the NVMe 1.1 spec contains the following:  "While the details associated with implementing a controller that supports SR-IOV are outside the scope of this specification, such a controller shall implement fully compliant NVM Express Virtual Functions (VFs). This ensures that the same host software developed for non-virtualized environments is capable of running unmodified within an SI. No such requirement exists for the Physical Function (PF)."
>
> If a VF is a fully NVMe compliant device then it must at least act like it implements NVMe subsystem reset.  How the SR-IOV-capable controller actually implements this is left up to the vendor.  But the spec does not require that a reset of one VF initiate a reset of other VF.
>
> -----Original Message-----
> From: Linux-nvme [mailto:linux-nvme-bounces@lists.infradead.org] On Behalf Of Learner Study
> Sent: Monday, April 21, 2014 8:34 PM
> To: Busch, Keith
> Cc: bhelgaas@google.com; linux-pci@vger.kernel.org; linux-nvme
> Subject: Re: [PATCH 2/2] NVMe: Implement PCI-e reset notification callback
>
> Per the spec, "When an NVM Subsystem Reset occurs, the entire NVM subsystem is reset"....so all VFs would get impacted, if a VF does a Reset. So, I think Reset kind of control should be in PF mode only.
>
> On Mon, Apr 21, 2014 at 7:45 PM, Learner <learner.study@gmail.com> wrote:
>> But Won't resetting from a VF impact functionality of other VFs?
>>
>> On Apr 21, 2014, at 6:57 PM, Keith Busch <keith.busch@intel.com> wrote:
>>
>>> On Mon, 21 Apr 2014, Learner Study wrote:
>>>> Hi Keith,
>>>>
>>>> I think NVMe Reset should apply to PF mode driver only, and not to
>>>> VF mode driver.
>>>> Is that understanding correct? Does the NVMe driver know which mode
>>>> its running in?
>>>
>>> Oh, this driver doesn't enable SR-IOV and has no PF/VF awareness.
>>> Shame on us, I'll add it to my list unless someone beats me to it.
>>>
>>> Still, I think we'd like to be able to reset a VF if only because it
>>> gets the queues back to a pristine state. A VF reset should not
>>> affect any of the other functions.
>
> _______________________________________________
> Linux-nvme mailing list
> Linux-nvme@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: Wheather the NVMe driver has been tried on the IBM or Lenovo server?

[Bjorn Helgaas]

On Tue, Apr 22, 2014 at 5:33 AM, liaohengquan1986
<liaohengquan1986@163.com> wrote:
>
> Hi, everyone,
>             I want to ask that weather the NVMe driver has been tried on the
> IBM or Lenovo server?
>             I use it on IBM(Lenovo) server with suse 11 SP3, but the MSI-X
> irq is always could not be got by cpu(may be it is masked).
>             Has anyone got this kind of problem?

I think your original email contained non-plain text, so it won't
appear on the mailing list, and some recipients may auto-discard it as
well.  See http://vger.kernel.org/majordomo-info.html

If you can reproduce the problem with an upstream kernel, this is the
right place to debug it.  We'd need a complete dmesg log and "lspci
-vv" output to start with.

If the problem only happens with SUSE, then you'd want to work with
SUSE to figure it out.

Bjorn

Re: Wheather the NVMe driver has been tried on the IBM or Lenovo server?

[Learner Study]

I'd like to test out the latest NVMe driver. Could someone recommend a
NVMe controller should I use? I haven't been  able to find anything on
Amazon yet...

And perhaps, which version of kernel (OS distribution) should I pick?

Thanks


On Tue, Apr 22, 2014 at 8:57 AM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> On Tue, Apr 22, 2014 at 5:33 AM, liaohengquan1986
> <liaohengquan1986@163.com> wrote:
>>
>> Hi, everyone,
>>             I want to ask that weather the NVMe driver has been tried on the
>> IBM or Lenovo server?
>>             I use it on IBM(Lenovo) server with suse 11 SP3, but the MSI-X
>> irq is always could not be got by cpu(may be it is masked).
>>             Has anyone got this kind of problem?
>
> I think your original email contained non-plain text, so it won't
> appear on the mailing list, and some recipients may auto-discard it as
> well.  See http://vger.kernel.org/majordomo-info.html
>
> If you can reproduce the problem with an upstream kernel, this is the
> right place to debug it.  We'd need a complete dmesg log and "lspci
> -vv" output to start with.
>
> If the problem only happens with SUSE, then you'd want to work with
> SUSE to figure it out.
>
> Bjorn

RE: [PATCH 2/2] NVMe: Implement PCI-e reset notification callback

[Keith Busch]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1868 bytes --]

On Mon, 21 Apr 2014, Mayes, Barrett N wrote:
> Section 8.5 of the NVMe 1.1 spec contains the following:  "While the details
> associated with implementing a controller that supports SR-IOV are outside
> the scope of this specification, such a controller shall implement fully
> compliant NVM Express Virtual Functions (VFs). This ensures that the same
> host software developed for non-virtualized environments is capable of
> running unmodified within an SI. No such requirement exists for the Physical
> Function (PF)."
>
> If a VF is a fully NVMe compliant device then it must at least act like it
> implements NVMe subsystem reset.  How the SR-IOV-capable controller actually
> implements this is left up to the vendor.  But the spec does not require that
> a reset of one VF initiate a reset of other VF.

The NVMe spec does not, but PCI does.

In case it wasn't clear from PATCH 1/2, this proposed callback is for a
function level reset. The PCI SR-IOV spec says all VF's implement FLR and
that this reset does not affect any other functions, from section 2.2.2:

   VFs must support Function Level Reset (FLR).

   Note: Software may use FLR to reset a VF. FLR to a VF affects a VF’s
   state but does not affect its existence in PCI Configuration Space or
   PCI Bus address space. The VFs BARn values (see Section 3.3.14) and
   VF MSE (see Section 3.3.3.4) in the PF’s SR-IOV extended capability
   are unaffected by FLRs issued to the VF.

Further, an NVMe subsystem reset is not the same as a controller or
function level reset. I have not proposed doing a subsytem reset here.

Since this is a call-back invoked from an FLR that happens outside the
driver whether the driver wants it to happen or not, it's better the
driver is aware and prepared that this occured rather than not knowing
and left to wonder why the heck the controller stopped responding.

Re: [PATCH 2/2] NVMe: Implement PCI-e reset notification callback

[Learner Study]

Thanks for the clarification that the patch is for FLR, and not NVMe
subsystem Reset event.
Any thoughts on how NVMe subsystem Reset would be handled in future?

Thanks!

On Tue, Apr 22, 2014 at 7:22 AM, Keith Busch <keith.busch@intel.com> wrote:
> On Mon, 21 Apr 2014, Mayes, Barrett N wrote:
>>
>> Section 8.5 of the NVMe 1.1 spec contains the following:  "While the
>> details
>> associated with implementing a controller that supports SR-IOV are outside
>> the scope of this specification, such a controller shall implement fully
>> compliant NVM Express Virtual Functions (VFs). This ensures that the same
>> host software developed for non-virtualized environments is capable of
>> running unmodified within an SI. No such requirement exists for the
>> Physical
>> Function (PF)."
>>
>> If a VF is a fully NVMe compliant device then it must at least act like it
>> implements NVMe subsystem reset.  How the SR-IOV-capable controller
>> actually
>> implements this is left up to the vendor.  But the spec does not require
>> that
>> a reset of one VF initiate a reset of other VF.
>
>
> The NVMe spec does not, but PCI does.
>
> In case it wasn't clear from PATCH 1/2, this proposed callback is for a
> function level reset. The PCI SR-IOV spec says all VF's implement FLR and
> that this reset does not affect any other functions, from section 2.2.2:
>
>   VFs must support Function Level Reset (FLR).
>
>   Note: Software may use FLR to reset a VF. FLR to a VF affects a VF’s
>   state but does not affect its existence in PCI Configuration Space or
>   PCI Bus address space. The VFs BARn values (see Section 3.3.14) and
>   VF MSE (see Section 3.3.3.4) in the PF’s SR-IOV extended capability
>   are unaffected by FLRs issued to the VF.
>
> Further, an NVMe subsystem reset is not the same as a controller or
> function level reset. I have not proposed doing a subsytem reset here.
>
> Since this is a call-back invoked from an FLR that happens outside the
> driver whether the driver wants it to happen or not, it's better the
> driver is aware and prepared that this occured rather than not knowing
> and left to wonder why the heck the controller stopped responding.

Re: [PATCH 1/2] PCI: Device driver function reset notification

[Bjorn Helgaas]

On Tue, Apr 08, 2014 at 05:42:20PM -0600, Keith Busch wrote:
> A user can issue a pci function level reset to a device using sysfs entry
> /sys/bus/pci/devices/.../reset. A kernel driver handling the pci device
> might like to know this reset is about to occur and when the reset attempt
> completes. This is so the driver has a chance to take appropriate device
> specific actions; for example, it may need to quiesce before the reset,
> and reinitialize the device after.
> 
> Signed-off-by: Keith Busch <keith.busch@intel.com>
> ---
>  drivers/pci/pci.c   |   13 +++++++++++++
>  include/linux/pci.h |    3 +++
>  2 files changed, 16 insertions(+)
> 
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index fdbc294..cb24bbe 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -3320,6 +3320,15 @@ static int pci_dev_reset(struct pci_dev *dev, int probe)
>  
>  	return rc;
>  }
> +
> +static void pci_reset_notify(struct pci_dev *dev, bool prepare)
> +{
> +	const struct pci_error_handlers *err_handler =
> +			dev->driver ? dev->driver->err_handler : NULL;
> +	if (err_handler && err_handler->reset_notify)
> +		err_handler->reset_notify(dev, prepare);
> +}
> +
>  /**
>   * __pci_reset_function - reset a PCI device function
>   * @dev: PCI device to reset
> @@ -3408,11 +3417,13 @@ int pci_reset_function(struct pci_dev *dev)
>  	if (rc)
>  		return rc;
>  
> +	pci_reset_notify(dev, true);
>  	pci_dev_save_and_disable(dev);
>  
>  	rc = pci_dev_reset(dev, 0);
>  
>  	pci_dev_restore(dev);
> +	pci_reset_notify(dev, false);
>  
>  	return rc;
>  }
> @@ -3432,6 +3443,7 @@ int pci_try_reset_function(struct pci_dev *dev)
>  	if (rc)
>  		return rc;
>  
> +	pci_reset_notify(dev, true);
>  	pci_dev_save_and_disable(dev);
>  
>  	if (pci_dev_trylock(dev)) {
> @@ -3441,6 +3453,7 @@ int pci_try_reset_function(struct pci_dev *dev)
>  		rc = -EAGAIN;
>  
>  	pci_dev_restore(dev);
> +	pci_reset_notify(dev, false);

You put the notify in these functions:

  pci_reset_function()
  pci_try_reset_function()

but what about these:

  pci_reset_slot()
  pci_try_reset_slot()
  pci_reset_bus()
  pci_try_reset_bus()

It seems like this ought to work the same way over all kinds of reset.

>  	return rc;
>  }
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 33aa2ca..d82dd3f 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -603,6 +603,9 @@ struct pci_error_handlers {
>  	/* PCI slot has been reset */
>  	pci_ers_result_t (*slot_reset)(struct pci_dev *dev);
>  
> +	/* PCI function reset prepare or completed */
> +	void (*reset_notify)(struct pci_dev *dev, bool prepare);
> +
>  	/* Device driver may resume normal operations */
>  	void (*resume)(struct pci_dev *dev);
>  };
> -- 
> 1.7.10.4
> 

Re: [PATCH 1/2] PCI: Device driver function reset notification

[Keith Busch]

On Wed, 30 Apr 2014, Bjorn Helgaas wrote:
> On Tue, Apr 08, 2014 at 05:42:20PM -0600, Keith Busch wrote:
> You put the notify in these functions:
>
>  pci_reset_function()
>  pci_try_reset_function()
>
> but what about these:
>
>  pci_reset_slot()
>  pci_try_reset_slot()
>  pci_reset_bus()
>  pci_try_reset_bus()

Good point. These all call pci_dev_save_and_disable at the start and
pci_dev_restore after, so I think just adding the reset prepare/complete
to those functions should cover all scenarios. Does this sound ok?

Re: [PATCH 1/2] PCI: Device driver function reset notification

[Bjorn Helgaas]

On Thu, May 1, 2014 at 1:57 PM, Keith Busch <keith.busch@intel.com> wrote:
> On Wed, 30 Apr 2014, Bjorn Helgaas wrote:
>>
>> On Tue, Apr 08, 2014 at 05:42:20PM -0600, Keith Busch wrote:
>> You put the notify in these functions:
>>
>>  pci_reset_function()
>>  pci_try_reset_function()
>>
>> but what about these:
>>
>>  pci_reset_slot()
>>  pci_try_reset_slot()
>>  pci_reset_bus()
>>  pci_try_reset_bus()
>
>
> Good point. These all call pci_dev_save_and_disable at the start and
> pci_dev_restore after, so I think just adding the reset prepare/complete
> to those functions should cover all scenarios. Does this sound ok?

Sound OK to me, as long as the save/restore functions aren't used elsewhere.

Re: [PATCH 1/2] PCI: Device driver function reset notification

[Keith Busch]

On Thu, 1 May 2014, Bjorn Helgaas wrote:
> On Thu, May 1, 2014 at 1:57 PM, Keith Busch <keith.busch@intel.com> wrote:
>> On Wed, 30 Apr 2014, Bjorn Helgaas wrote:
>>>
>>> On Tue, Apr 08, 2014 at 05:42:20PM -0600, Keith Busch wrote:
>>> You put the notify in these functions:
>>>
>>>  pci_reset_function()
>>>  pci_try_reset_function()
>>>
>>> but what about these:
>>>
>>>  pci_reset_slot()
>>>  pci_try_reset_slot()
>>>  pci_reset_bus()
>>>  pci_try_reset_bus()
>>
>>
>> Good point. These all call pci_dev_save_and_disable at the start and
>> pci_dev_restore after, so I think just adding the reset prepare/complete
>> to those functions should cover all scenarios. Does this sound ok?
>
> Sound OK to me, as long as the save/restore functions aren't used elsewhere.

Yep, there is currently 1:1 symmetry for these in all cases. I'll make
a note for any future use that the calling disabling/restore should
maintain this property.