[PATCH] PCI: Device specific reset function

[PATCH] PCI: Device specific reset function

[Tadeusz Struk]

I have a use case where I need to cleanup resource allocated for Virtual
Functions after a guest OS that used it crashed. This cleanup needs to
be done before the VF is being FLRed. The only possible way to do this
seems to be by using pci_dev_specific_reset() function.
This patch adds specific reset function to the pci_driver struct.

Signed-off-by: Tadeusz Struk <tadeusz.struk@intel.com>
---
 drivers/pci/quirks.c |    4 ++++
 include/linux/pci.h  |    1 +
 2 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 6476547..13d224b 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -3081,8 +3081,12 @@ static const struct pci_dev_reset_methods pci_dev_reset_methods[] = {
 
 int pci_dev_specific_reset(struct pci_dev *dev, int probe)
 {
+	struct pci_driver *drv = dev->driver;
 	const struct pci_dev_reset_methods *i;
 
+	if (drv && drv->reset)
+		return drv->reset(dev, probe);
+
 	for (i = pci_dev_reset_methods; i->reset; i++) {
 		if ((i->vendor == dev->vendor ||
 		     i->vendor == (u16)PCI_ANY_ID) &&
diff --git a/include/linux/pci.h b/include/linux/pci.h
index a16b1df..2ccd838 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -560,6 +560,7 @@ struct pci_driver {
 	int  (*resume_early) (struct pci_dev *dev);
 	int  (*resume) (struct pci_dev *dev);	                /* Device woken up */
 	void (*shutdown) (struct pci_dev *dev);
+	int  (*reset) (struct pci_dev *dev, int probe);
 	struct pci_error_handlers *err_handler;
 	struct device_driver	driver;
 	struct pci_dynids dynids;
-- 
1.7.7.6

Re: [PATCH] PCI: Device specific reset function

[Alex Williamson]

On Mon, 2012-03-12 at 16:55 +0000, Tadeusz Struk wrote:
> I have a use case where I need to cleanup resource allocated for Virtual
> Functions after a guest OS that used it crashed. This cleanup needs to
> be done before the VF is being FLRed. The only possible way to do this
> seems to be by using pci_dev_specific_reset() function.
> This patch adds specific reset function to the pci_driver struct.
> 
> Signed-off-by: Tadeusz Struk <tadeusz.struk@intel.com>
> ---
>  drivers/pci/quirks.c |    4 ++++
>  include/linux/pci.h  |    1 +
>  2 files changed, 5 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index 6476547..13d224b 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -3081,8 +3081,12 @@ static const struct pci_dev_reset_methods pci_dev_reset_methods[] = {
>  
>  int pci_dev_specific_reset(struct pci_dev *dev, int probe)
>  {
> +	struct pci_driver *drv = dev->driver;
>  	const struct pci_dev_reset_methods *i;
>  
> +	if (drv && drv->reset)
> +		return drv->reset(dev, probe);
> +

Isn't this exactly what the below code is for (pci_dev_reset_methods)?
In the case of KVM device assignment, the device should be reset via
this path if the guest crashes, but it may or may not be attached to
pci-stub at the time.  I assume you're not adding a reset handler to
pci-stub, so we'll pass through here and use FLR, violating the premise
of the patch.  Thanks,

Alex

>  	for (i = pci_dev_reset_methods; i->reset; i++) {
>  		if ((i->vendor == dev->vendor ||
>  		     i->vendor == (u16)PCI_ANY_ID) &&
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index a16b1df..2ccd838 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -560,6 +560,7 @@ struct pci_driver {
>  	int  (*resume_early) (struct pci_dev *dev);
>  	int  (*resume) (struct pci_dev *dev);	                /* Device woken up */
>  	void (*shutdown) (struct pci_dev *dev);
> +	int  (*reset) (struct pci_dev *dev, int probe);
>  	struct pci_error_handlers *err_handler;
>  	struct device_driver	driver;
>  	struct pci_dynids dynids;

Re: [PATCH] PCI: Device specific reset function

[Tadeusz Struk]

On 13/03/12 02:42, Alex Williamson wrote:
> On Mon, 2012-03-12 at 16:55 +0000, Tadeusz Struk wrote:
>> I have a use case where I need to cleanup resource allocated for Virtual
>> Functions after a guest OS that used it crashed. This cleanup needs to
>> be done before the VF is being FLRed. The only possible way to do this
>> seems to be by using pci_dev_specific_reset() function.
>> This patch adds specific reset function to the pci_driver struct.
>>
>> Signed-off-by: Tadeusz Struk <tadeusz.struk@intel.com>
>> ---
>>  drivers/pci/quirks.c |    4 ++++
>>  include/linux/pci.h  |    1 +
>>  2 files changed, 5 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
>> index 6476547..13d224b 100644
>> --- a/drivers/pci/quirks.c
>> +++ b/drivers/pci/quirks.c
>> @@ -3081,8 +3081,12 @@ static const struct pci_dev_reset_methods pci_dev_reset_methods[] = {
>>  
>>  int pci_dev_specific_reset(struct pci_dev *dev, int probe)
>>  {
>> +	struct pci_driver *drv = dev->driver;
>>  	const struct pci_dev_reset_methods *i;
>>  
>> +	if (drv && drv->reset)
>> +		return drv->reset(dev, probe);
>> +
> 
> Isn't this exactly what the below code is for (pci_dev_reset_methods)?
> In the case of KVM device assignment, the device should be reset via
> this path if the guest crashes, but it may or may not be attached to
> pci-stub at the time.  I assume you're not adding a reset handler to
> pci-stub, so we'll pass through here and use FLR, violating the premise
> of the patch.  Thanks,
> 
> Alex

Hi,
The reset handler supposed to be in the "real" driver not in the pci-stub.
You are right, at this stage the VF can still be assigned to the pci-stub, which makes it useless in my case.
I think the only option for me will be to go with the v2 of the patch.
I'll do a bit more testing to make sure this is the case and resend the v2.
Thanks,
Tadeusz