[PATCH v4 0/3] Workaround for bus/slot reset on Cavium cn8xxx root ports

[PATCH v4 0/3] Workaround for bus/slot reset on Cavium cn8xxx root ports

[Jan Glauber]

Using vfio-pci on a combination of cn8xxx and some PCI devices results in
a kernel panic. This is triggered by issuing a bus or a slot reset
on the PCI device.

With this series both checks indicate that the reset is not possible
preventing the kernel panic.

David Daney (2):
  PCI: Allow PCI_DEV_FLAGS_NO_BUS_RESET to be used on bus device
  PCI: Avoid bus reset for Cavium cn8xxx root ports

Jan Glauber (1):
  PCI: Avoid slot reset if bus reset is not possible

 drivers/pci/pci.c    | 8 ++++++++
 drivers/pci/quirks.c | 8 ++++++++
 2 files changed, 16 insertions(+)

-- 
2.9.0.rc0.21.g7777322

[PATCH v4 1/3] PCI: Allow PCI_DEV_FLAGS_NO_BUS_RESET to be used on bus device

[Jan Glauber]

From: David Daney <david.daney@cavium.com>

When checking to see if a PCI bus can safely be reset, we check to see
if any of the children have their PCI_DEV_FLAGS_NO_BUS_RESET flag set.
As these devices are known not to behave well after a bus reset.

Some PCIe root port bridges also do not behave well after a bus reset,
sometimes causing the devices behind the bridge to become unusable.

Add a check for the PCI_DEV_FLAGS_NO_BUS_RESET flag being set in the
bridge device to allow these bridges to be flagged, and prevent their
buses from being reset.

A follow on patch will add a quirk for this type of bridge.

Signed-off-by: David Daney <david.daney@cavium.com>
[jglauber@cavium.com: fixed typo]
Signed-off-by: Jan Glauber <jglauber@cavium.com>
---
 drivers/pci/pci.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index fdf65a6c13f6..b2a46ca7f133 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -4325,6 +4325,10 @@ static bool pci_bus_resetable(struct pci_bus *bus)
 {
 	struct pci_dev *dev;
 
+
+	if (bus->self && (bus->self->dev_flags & PCI_DEV_FLAGS_NO_BUS_RESET))
+		return false;
+
 	list_for_each_entry(dev, &bus->devices, bus_list) {
 		if (dev->dev_flags & PCI_DEV_FLAGS_NO_BUS_RESET ||
 		    (dev->subordinate && !pci_bus_resetable(dev->subordinate)))
-- 
2.9.0.rc0.21.g7777322

[PATCH v4 2/3] PCI: Avoid bus reset for Cavium cn8xxx root ports

[Jan Glauber]

From: David Daney <david.daney@cavium.com>

Root ports of cn8xxx do not function after bus reset when used with
some e1000e and LSI HBA devices. Add a quirk to prevent bus reset on
these root ports.

Signed-off-by: David Daney <david.daney@cavium.com>
[jglauber@cavium.com: fixed typo and whitespaces]
Signed-off-by: Jan Glauber <jglauber@cavium.com>
---
 drivers/pci/quirks.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 140760403f36..2e4e7b6d1a79 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -3364,6 +3364,14 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_ATHEROS, 0x0032, quirk_no_bus_reset);
 DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_ATHEROS, 0x003c, quirk_no_bus_reset);
 DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_ATHEROS, 0x0033, quirk_no_bus_reset);
 
+/*
+ * Root port on some Cavium CN8xxx chips do not successfully complete
+ * a bus reset when used with certain types of child devices. Config
+ * space access to the child may quit responding. Flag the root port
+ * as not supporting bus reset.
+ */
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_CAVIUM, 0xa100, quirk_no_bus_reset);
+
 static void quirk_no_pm_reset(struct pci_dev *dev)
 {
 	/*
-- 
2.9.0.rc0.21.g7777322

[PATCH v4 3/3] PCI: Avoid slot reset if bus reset is not possible

[Jan Glauber]

When checking to see if a PCI slot can safely be reset, we check to
see if any of the children have their PCI_DEV_FLAGS_NO_BUS_RESET flag
set.

Some PCIe root port bridges do not behave well after a slot reset,
and may cause the device in the slot to become unusable.

Add a check for the PCI_DEV_FLAGS_NO_BUS_RESET flag being set in the
bridge device to prevent the slot from being reset.

Signed-off-by: Jan Glauber <jglauber@cavium.com>
---
 drivers/pci/pci.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index b2a46ca7f133..45a086fc3592 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -4393,6 +4393,10 @@ static bool pci_slot_resetable(struct pci_slot *slot)
 {
 	struct pci_dev *dev;
 
+	if (slot->bus->self &&
+	    (slot->bus->self->dev_flags & PCI_DEV_FLAGS_NO_BUS_RESET))
+		return false;
+
 	list_for_each_entry(dev, &slot->bus->devices, bus_list) {
 		if (!dev->slot || dev->slot != slot)
 			continue;
-- 
2.9.0.rc0.21.g7777322

Re: [v4,0/3] Workaround for bus/slot reset on Cavium cn8xxx root ports

[Vadim Lomovtsev]

Hi all,

Are there any updates on this ?
Comments/objections/acks/nacks ?

WBBR,
Vadim

On Fri, Sep 08, 2017 at 10:10:30AM +0200, Jan Glauber wrote:
> Using vfio-pci on a combination of cn8xxx and some PCI devices results in
> a kernel panic. This is triggered by issuing a bus or a slot reset
> on the PCI device.
> 
> With this series both checks indicate that the reset is not possible
> preventing the kernel panic.
> 
> David Daney (2):
>   PCI: Allow PCI_DEV_FLAGS_NO_BUS_RESET to be used on bus device
>   PCI: Avoid bus reset for Cavium cn8xxx root ports
> 
> Jan Glauber (1):
>   PCI: Avoid slot reset if bus reset is not possible
> 
>  drivers/pci/pci.c    | 8 ++++++++
>  drivers/pci/quirks.c | 8 ++++++++
>  2 files changed, 16 insertions(+)

Re: [v4,0/3] Workaround for bus/slot reset on Cavium cn8xxx root ports

[Jon Masters]

On 09/12/2017 05:40 AM, Vadim Lomovtsev wrote:

> Are there any updates on this ?
> Comments/objections/acks/nacks ?

Any more comments?

Jon.

> On Fri, Sep 08, 2017 at 10:10:30AM +0200, Jan Glauber wrote:
>> Using vfio-pci on a combination of cn8xxx and some PCI devices results in
>> a kernel panic. This is triggered by issuing a bus or a slot reset
>> on the PCI device.
>>
>> With this series both checks indicate that the reset is not possible
>> preventing the kernel panic.
>>
>> David Daney (2):
>>   PCI: Allow PCI_DEV_FLAGS_NO_BUS_RESET to be used on bus device
>>   PCI: Avoid bus reset for Cavium cn8xxx root ports
>>
>> Jan Glauber (1):
>>   PCI: Avoid slot reset if bus reset is not possible
>>
>>  drivers/pci/pci.c    | 8 ++++++++
>>  drivers/pci/quirks.c | 8 ++++++++
>>  2 files changed, 16 insertions(+)


-- 
Computer Architect | Sent from my Fedora powered laptop

Re: [v4,0/3] Workaround for bus/slot reset on Cavium cn8xxx root ports

[Alex Williamson]

On Tue, 12 Sep 2017 02:40:49 -0700
Vadim Lomovtsev <Vadim.Lomovtsev@caviumnetworks.com> wrote:

> Hi all,
> 
> Are there any updates on this ?
> Comments/objections/acks/nacks ?
> 
> WBBR,
> Vadim
> 
> On Fri, Sep 08, 2017 at 10:10:30AM +0200, Jan Glauber wrote:
> > Using vfio-pci on a combination of cn8xxx and some PCI devices results in
> > a kernel panic. This is triggered by issuing a bus or a slot reset
> > on the PCI device.
> > 
> > With this series both checks indicate that the reset is not possible
> > preventing the kernel panic.
> > 
> > David Daney (2):
> >   PCI: Allow PCI_DEV_FLAGS_NO_BUS_RESET to be used on bus device
> >   PCI: Avoid bus reset for Cavium cn8xxx root ports
> > 
> > Jan Glauber (1):
> >   PCI: Avoid slot reset if bus reset is not possible
> > 
> >  drivers/pci/pci.c    | 8 ++++++++
> >  drivers/pci/quirks.c | 8 ++++++++
> >  2 files changed, 16 insertions(+)  


Looks ok to me, for series:

Reviewed-by: Alex Williamson <alex.williamson@redhat.com>

I am curious why we're happy targeting this quirk at a single device ID
while at the same time trying to expand the ACS quirk to a notable
fraction of the Cavium PCI device ID address space.  Thanks,

Alex

Re: [v4,0/3] Workaround for bus/slot reset on Cavium cn8xxx root ports

[Jan Glauber]

On Wed, Sep 20, 2017 at 12:09:12PM -0600, Alex Williamson wrote:
> On Tue, 12 Sep 2017 02:40:49 -0700
> Vadim Lomovtsev <Vadim.Lomovtsev@caviumnetworks.com> wrote:
> 
> > Hi all,
> > 
> > Are there any updates on this ?
> > Comments/objections/acks/nacks ?
> > 
> > WBBR,
> > Vadim
> > 
> > On Fri, Sep 08, 2017 at 10:10:30AM +0200, Jan Glauber wrote:
> > > Using vfio-pci on a combination of cn8xxx and some PCI devices results in
> > > a kernel panic. This is triggered by issuing a bus or a slot reset
> > > on the PCI device.
> > > 
> > > With this series both checks indicate that the reset is not possible
> > > preventing the kernel panic.
> > > 
> > > David Daney (2):
> > >   PCI: Allow PCI_DEV_FLAGS_NO_BUS_RESET to be used on bus device
> > >   PCI: Avoid bus reset for Cavium cn8xxx root ports
> > > 
> > > Jan Glauber (1):
> > >   PCI: Avoid slot reset if bus reset is not possible
> > > 
> > >  drivers/pci/pci.c    | 8 ++++++++
> > >  drivers/pci/quirks.c | 8 ++++++++
> > >  2 files changed, 16 insertions(+)  
> 
> 
> Looks ok to me, for series:
> 
> Reviewed-by: Alex Williamson <alex.williamson@redhat.com>

Thanks for the review. And also for being patient with my iterations.

> I am curious why we're happy targeting this quirk at a single device ID
> while at the same time trying to expand the ACS quirk to a notable
> fraction of the Cavium PCI device ID address space.  Thanks,

David, please correct me if I'm wrong but I think this problem only
exists on cn88xx (device id 0xa100) but not on cn81xx/cn83xx (0xa200,
0xa300). I've seen the bridge causing the problem only on cn88xx.

--Jan

Re: [v4,0/3] Workaround for bus/slot reset on Cavium cn8xxx root ports

[Jan Glauber]

On Wed, Sep 20, 2017 at 12:09:12PM -0600, Alex Williamson wrote:
> On Tue, 12 Sep 2017 02:40:49 -0700
> Vadim Lomovtsev <Vadim.Lomovtsev@caviumnetworks.com> wrote:
> 
> > Hi all,
> > 
> > Are there any updates on this ?
> > Comments/objections/acks/nacks ?
> > 
> > WBBR,
> > Vadim
> > 
> > On Fri, Sep 08, 2017 at 10:10:30AM +0200, Jan Glauber wrote:
> > > Using vfio-pci on a combination of cn8xxx and some PCI devices results in
> > > a kernel panic. This is triggered by issuing a bus or a slot reset
> > > on the PCI device.
> > > 
> > > With this series both checks indicate that the reset is not possible
> > > preventing the kernel panic.
> > > 
> > > David Daney (2):
> > >   PCI: Allow PCI_DEV_FLAGS_NO_BUS_RESET to be used on bus device
> > >   PCI: Avoid bus reset for Cavium cn8xxx root ports
> > > 
> > > Jan Glauber (1):
> > >   PCI: Avoid slot reset if bus reset is not possible
> > > 
> > >  drivers/pci/pci.c    | 8 ++++++++
> > >  drivers/pci/quirks.c | 8 ++++++++
> > >  2 files changed, 16 insertions(+)  
> 
> 
> Looks ok to me, for series:
> 
> Reviewed-by: Alex Williamson <alex.williamson@redhat.com>
> 
> I am curious why we're happy targeting this quirk at a single device ID
> while at the same time trying to expand the ACS quirk to a notable
> fraction of the Cavium PCI device ID address space.  Thanks,
> 
> Alex

Bjorn, would you take these patches? 

We might need to extend the quirk to cover more cn8xxx variants,
this is not yet entirely clear on our side.

Therefore I'd like to ask if we could merge this patches now to solve the
long-standing issue for cn88xx.

thanks,
Jan

Re: [PATCH v4 0/3] Workaround for bus/slot reset on Cavium cn8xxx root ports

[Robert Richter]

Bjorn,

On 08.09.17 10:10:30, Jan Glauber wrote:
> Using vfio-pci on a combination of cn8xxx and some PCI devices results in
> a kernel panic. This is triggered by issuing a bus or a slot reset
> on the PCI device.
> 
> With this series both checks indicate that the reset is not possible
> preventing the kernel panic.
> 
> David Daney (2):
>   PCI: Allow PCI_DEV_FLAGS_NO_BUS_RESET to be used on bus device
>   PCI: Avoid bus reset for Cavium cn8xxx root ports
> 
> Jan Glauber (1):
>   PCI: Avoid slot reset if bus reset is not possible
> 
>  drivers/pci/pci.c    | 8 ++++++++
>  drivers/pci/quirks.c | 8 ++++++++
>  2 files changed, 16 insertions(+)

if you don't have any objections, please apply this series.

Thanks,

-Robert

Re: [PATCH v4 0/3] Workaround for bus/slot reset on Cavium cn8xxx root ports

[Bjorn Helgaas]

On Fri, Sep 08, 2017 at 10:10:30AM +0200, Jan Glauber wrote:
> Using vfio-pci on a combination of cn8xxx and some PCI devices results in
> a kernel panic. This is triggered by issuing a bus or a slot reset
> on the PCI device.
> 
> With this series both checks indicate that the reset is not possible
> preventing the kernel panic.
> 
> David Daney (2):
>   PCI: Allow PCI_DEV_FLAGS_NO_BUS_RESET to be used on bus device
>   PCI: Avoid bus reset for Cavium cn8xxx root ports
> 
> Jan Glauber (1):
>   PCI: Avoid slot reset if bus reset is not possible
> 
>  drivers/pci/pci.c    | 8 ++++++++
>  drivers/pci/quirks.c | 8 ++++++++
>  2 files changed, 16 insertions(+)

Applied to pci/host-thunder for v4.15, thanks.

I reordered and renamed them like this:

  PCI: Mark Cavium CN8xxx to avoid bus reset
  PCI: Avoid bus reset if bridge itself is broken
  PCI: Avoid slot reset if bridge itself is broken