linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] PCI: quirk for preventing bus reset on TI C667X
@ 2021-01-12 15:36 Antti Järvinen
  2021-01-21 23:55 ` Bjorn Helgaas
  0 siblings, 1 reply; 14+ messages in thread
From: Antti Järvinen @ 2021-01-12 15:36 UTC (permalink / raw)
  To: Bjorn Helgaas, linux-pci, linux-kernel; +Cc: Antti Järvinen

TI C667X does not support bus/hot reset.
See https://e2e.ti.com/support/processors/f/791/t/954382

Signed-off-by: Antti Järvinen <antti.jarvinen@gmail.com>
---
 drivers/pci/quirks.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 653660e3ba9e..c8fcf24c5bd0 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -3578,6 +3578,12 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_ATHEROS, 0x0034, quirk_no_bus_reset);
  */
 DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_CAVIUM, 0xa100, quirk_no_bus_reset);
 
+/*
+ * Some TI keystone C667X devices do no support bus/hot reset.
+ * https://e2e.ti.com/support/processors/f/791/t/954382
+ */
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_TI, 0xb005, quirk_no_bus_reset);
+
 static void quirk_no_pm_reset(struct pci_dev *dev)
 {
 	/*
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH] PCI: quirk for preventing bus reset on TI C667X
  2021-01-12 15:36 [PATCH] PCI: quirk for preventing bus reset on TI C667X Antti Järvinen
@ 2021-01-21 23:55 ` Bjorn Helgaas
  2021-01-26 11:22   ` Antti Järvinen
  2021-02-17 21:18   ` Bjorn Helgaas
  0 siblings, 2 replies; 14+ messages in thread
From: Bjorn Helgaas @ 2021-01-21 23:55 UTC (permalink / raw)
  To: Antti Järvinen
  Cc: Bjorn Helgaas, linux-pci, linux-kernel, Alex Williamson,
	Murali Karicheri, Kishon Vijay Abraham I

[+cc Alex, Murali, Kishon]

On Tue, Jan 12, 2021 at 03:36:43PM +0000, Antti Järvinen wrote:
> TI C667X does not support bus/hot reset.
> See https://e2e.ti.com/support/processors/f/791/t/954382

You can cite the URL as the source, but the URL will eventually become
stale, so let's include the relevant details here directly.  

From the forum, it looks like the device doesn't respond after a
reset (config accesses return ~0).  It seems somewhat surprising that
something as basic as a reset would be completely broken.  I wonder if
we're not doing the reset correctly.

It looks like we would probably be trying a Secondary Bus Reset using
the bridge leading to the C667X.  Can you confirm?  Wonder if you
could try doing what pci_reset_secondary_bus() does by hand:

  # BRIDGE=...                              # PCI address, e.g., 00:1c.0
  # C667X=...
  # setpci -s$C667X VENDOR_ID.w
  # setpci -s$BRIDGE BRIDGE_CONTROL.w       # prints "val"
  # setpci -s$BRIDGE BRIDGE_CONTROL.w=      # val | 0x40 (set SBR)
  # sleep 1
  # setpci -s$BRIDGE BRIDGE_CONTROL.w=      # val (clear SBR)
  # sleep 1
  # setpci -s$C667X VENDOR_ID.w=0
  # setpci -s$C667X VENDOR_ID.w

If we use this quirk and avoid the reset, I assume that means
assigning the device to VMs with VFIO will leak state between VMs?

> Signed-off-by: Antti Järvinen <antti.jarvinen@gmail.com>
> ---
>  drivers/pci/quirks.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index 653660e3ba9e..c8fcf24c5bd0 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -3578,6 +3578,12 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_ATHEROS, 0x0034, quirk_no_bus_reset);
>   */
>  DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_CAVIUM, 0xa100, quirk_no_bus_reset);
>  
> +/*
> + * Some TI keystone C667X devices do no support bus/hot reset.
> + * https://e2e.ti.com/support/processors/f/791/t/954382
> + */
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_TI, 0xb005, quirk_no_bus_reset);
> +
>  static void quirk_no_pm_reset(struct pci_dev *dev)
>  {
>  	/*
> -- 
> 2.17.1
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] PCI: quirk for preventing bus reset on TI C667X
  2021-01-21 23:55 ` Bjorn Helgaas
@ 2021-01-26 11:22   ` Antti Järvinen
  2021-01-29 23:49     ` Bjorn Helgaas
  2021-02-17 21:18   ` Bjorn Helgaas
  1 sibling, 1 reply; 14+ messages in thread
From: Antti Järvinen @ 2021-01-26 11:22 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Bjorn Helgaas, linux-pci, linux-kernel, Alex Williamson,
	Murali Karicheri, Kishon Vijay Abraham I

On 22.1.2021 1.55, Bjorn Helgaas wrote:> 

> It looks like we would probably be trying a Secondary Bus Reset using
> the bridge leading to the C667X.  Can you confirm? 

Yes, this is my understanding too.

> Wonder if you
> could try doing what pci_reset_secondary_bus() does by hand:
> 

I tried this by hand. It looks that result is same as through VFIO.

# cat sbr.sh
BRIDGE=10:00.0
C667X=11:00.0

setpci -s$C667X VENDOR_ID.w

VAL=$(setpci -s$BRIDGE BRIDGE_CONTROL.w)
echo $VAL
setpci -s$BRIDGE BRIDGE_CONTROL.w=$(($VAL | 0x40))
sleep 1
setpci -s$BRIDGE BRIDGE_CONTROL.w=$VAL
sleep 1
setpci -s$C667X VENDOR_ID.w=0
setpci -s$C667X VENDOR_ID.w


# ./sbr.sh
104c
0003
ffff


>   # BRIDGE=...                              # PCI address, e.g., 00:1c.0
>   # C667X=...
>   # setpci -s$C667X VENDOR_ID.w
>   # setpci -s$BRIDGE BRIDGE_CONTROL.w       # prints "val"
>   # setpci -s$BRIDGE BRIDGE_CONTROL.w=      # val | 0x40 (set SBR)
>   # sleep 1
>   # setpci -s$BRIDGE BRIDGE_CONTROL.w=      # val (clear SBR)
>   # sleep 1
>   # setpci -s$C667X VENDOR_ID.w=0
>   # setpci -s$C667X VENDOR_ID.w
> 
> If we use this quirk and avoid the reset, I assume that means
> assigning the device to VMs with VFIO will leak state between VMs?

I think this is true.


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] PCI: quirk for preventing bus reset on TI C667X
  2021-01-26 11:22   ` Antti Järvinen
@ 2021-01-29 23:49     ` Bjorn Helgaas
  0 siblings, 0 replies; 14+ messages in thread
From: Bjorn Helgaas @ 2021-01-29 23:49 UTC (permalink / raw)
  To: Antti Järvinen
  Cc: Bjorn Helgaas, linux-pci, linux-kernel, Alex Williamson,
	Murali Karicheri, Kishon Vijay Abraham I

On Tue, Jan 26, 2021 at 01:22:18PM +0200, Antti Järvinen wrote:
> On 22.1.2021 1.55, Bjorn Helgaas wrote:> 
> 
> > It looks like we would probably be trying a Secondary Bus Reset using
> > the bridge leading to the C667X.  Can you confirm? 
> 
> Yes, this is my understanding too.
> 
> > Wonder if you
> > could try doing what pci_reset_secondary_bus() does by hand:
> > 
> 
> I tried this by hand. It looks that result is same as through VFIO.
> 
> # cat sbr.sh
> BRIDGE=10:00.0
> C667X=11:00.0
> 
> setpci -s$C667X VENDOR_ID.w
> 
> VAL=$(setpci -s$BRIDGE BRIDGE_CONTROL.w)
> echo $VAL
> setpci -s$BRIDGE BRIDGE_CONTROL.w=$(($VAL | 0x40))
> sleep 1
> setpci -s$BRIDGE BRIDGE_CONTROL.w=$VAL
> sleep 1
> setpci -s$C667X VENDOR_ID.w=0
> setpci -s$C667X VENDOR_ID.w
> 
> 
> # ./sbr.sh
> 104c
> 0003
> ffff
> 
> 
> >   # BRIDGE=...                              # PCI address, e.g., 00:1c.0
> >   # C667X=...
> >   # setpci -s$C667X VENDOR_ID.w
> >   # setpci -s$BRIDGE BRIDGE_CONTROL.w       # prints "val"
> >   # setpci -s$BRIDGE BRIDGE_CONTROL.w=      # val | 0x40 (set SBR)
> >   # sleep 1
> >   # setpci -s$BRIDGE BRIDGE_CONTROL.w=      # val (clear SBR)
> >   # sleep 1
> >   # setpci -s$C667X VENDOR_ID.w=0
> >   # setpci -s$C667X VENDOR_ID.w
> > 
> > If we use this quirk and avoid the reset, I assume that means
> > assigning the device to VMs with VFIO will leak state between VMs?
> 
> I think this is true.

Alex, is there some warning about situations like this where a device
may leak state between VMs?

There's nothing in quirk_no_bus_reset() itself where we set
PCI_DEV_FLAGS_NO_BUS_RESET, and nothing in pci_parent_bus_reset() or
pci_dev_reset_slot_function() where we test it, but I didn't chase
into VFIO.

Seems important enough that we might want to mention it at least once
and maybe even every time we try to reset the device.  I hope the leak
isn't completely silent.

Bjorn

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] PCI: quirk for preventing bus reset on TI C667X
  2021-01-21 23:55 ` Bjorn Helgaas
  2021-01-26 11:22   ` Antti Järvinen
@ 2021-02-17 21:18   ` Bjorn Helgaas
  2021-02-28 13:53     ` [PATCH v2] " Antti Järvinen
  1 sibling, 1 reply; 14+ messages in thread
From: Bjorn Helgaas @ 2021-02-17 21:18 UTC (permalink / raw)
  To: Antti Järvinen
  Cc: Bjorn Helgaas, linux-pci, linux-kernel, Alex Williamson,
	Murali Karicheri, Kishon Vijay Abraham I

On Thu, Jan 21, 2021 at 05:55:47PM -0600, Bjorn Helgaas wrote:
> On Tue, Jan 12, 2021 at 03:36:43PM +0000, Antti Järvinen wrote:
> > TI C667X does not support bus/hot reset.
> > See https://e2e.ti.com/support/processors/f/791/t/954382
> 
> You can cite the URL as the source, but the URL will eventually become
> stale, so let's include the relevant details here directly.  

Thanks for trying the experiment below.  I'll look for a repost that
includes details from the URL directly in the commit log.

> From the forum, it looks like the device doesn't respond after a
> reset (config accesses return ~0).  It seems somewhat surprising that
> something as basic as a reset would be completely broken.  I wonder if
> we're not doing the reset correctly.
> 
> It looks like we would probably be trying a Secondary Bus Reset using
> the bridge leading to the C667X.  Can you confirm?  Wonder if you
> could try doing what pci_reset_secondary_bus() does by hand:
> 
>   # BRIDGE=...                              # PCI address, e.g., 00:1c.0
>   # C667X=...
>   # setpci -s$C667X VENDOR_ID.w
>   # setpci -s$BRIDGE BRIDGE_CONTROL.w       # prints "val"
>   # setpci -s$BRIDGE BRIDGE_CONTROL.w=      # val | 0x40 (set SBR)
>   # sleep 1
>   # setpci -s$BRIDGE BRIDGE_CONTROL.w=      # val (clear SBR)
>   # sleep 1
>   # setpci -s$C667X VENDOR_ID.w=0
>   # setpci -s$C667X VENDOR_ID.w
> 
> If we use this quirk and avoid the reset, I assume that means
> assigning the device to VMs with VFIO will leak state between VMs?
> 
> > Signed-off-by: Antti Järvinen <antti.jarvinen@gmail.com>
> > ---
> >  drivers/pci/quirks.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> > index 653660e3ba9e..c8fcf24c5bd0 100644
> > --- a/drivers/pci/quirks.c
> > +++ b/drivers/pci/quirks.c
> > @@ -3578,6 +3578,12 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_ATHEROS, 0x0034, quirk_no_bus_reset);
> >   */
> >  DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_CAVIUM, 0xa100, quirk_no_bus_reset);
> >  
> > +/*
> > + * Some TI keystone C667X devices do no support bus/hot reset.
> > + * https://e2e.ti.com/support/processors/f/791/t/954382
> > + */
> > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_TI, 0xb005, quirk_no_bus_reset);
> > +
> >  static void quirk_no_pm_reset(struct pci_dev *dev)
> >  {
> >  	/*
> > -- 
> > 2.17.1
> > 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v2] PCI: quirk for preventing bus reset on TI C667X
  2021-02-17 21:18   ` Bjorn Helgaas
@ 2021-02-28 13:53     ` Antti Järvinen
  2021-03-07  0:22       ` Krzysztof Wilczyński
  0 siblings, 1 reply; 14+ messages in thread
From: Antti Järvinen @ 2021-02-28 13:53 UTC (permalink / raw)
  To: helgaas
  Cc: alex.williamson, antti.jarvinen, bhelgaas, kishon, linux-kernel,
	linux-pci, m-karicheri2

Some TI keystone C667X devices do no support bus/hot reset. Its PCIESS
automatically disables LTSSM when secondary bus reset is received and
device stops working. Prevent bus reset by adding quirk_no_bus_reset to
the device. With this change device can be assigned to VMs with VFIO,
but it will leak state between VMs.

Reference https://e2e.ti.com/support/processors/f/791/t/954382

Signed-off-by: Antti Järvinen <antti.jarvinen@gmail.com>
---
 drivers/pci/quirks.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 653660e3ba9e..d9201ad1ca39 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -3578,6 +3578,16 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_ATHEROS, 0x0034, quirk_no_bus_reset);
  */
 DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_CAVIUM, 0xa100, quirk_no_bus_reset);
 
+/*
+ * Some TI keystone C667X devices do no support bus/hot reset.
+ * Its PCIESS automatically disables LTSSM when secondary bus reset is
+ * received and device stops working. Prevent bus reset by adding
+ * quirk_no_bus_reset to the device. With this change device can be
+ * assigned to VMs with VFIO, but it will leak state between VMs.
+ * Reference https://e2e.ti.com/support/processors/f/791/t/954382
+ */
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_TI, 0xb005, quirk_no_bus_reset);
+
 static void quirk_no_pm_reset(struct pci_dev *dev)
 {
 	/*
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] PCI: quirk for preventing bus reset on TI C667X
  2021-02-28 13:53     ` [PATCH v2] " Antti Järvinen
@ 2021-03-07  0:22       ` Krzysztof Wilczyński
  2021-03-08 14:21         ` [PATCH v3] PCI: Add " Antti Järvinen
  2021-03-08 14:28         ` [PATCH v2] PCI: " Antti Järvinen
  0 siblings, 2 replies; 14+ messages in thread
From: Krzysztof Wilczyński @ 2021-03-07  0:22 UTC (permalink / raw)
  To: Antti Järvinen
  Cc: helgaas, alex.williamson, bhelgaas, kishon, linux-kernel,
	linux-pci, m-karicheri2

Hi Antti,

A few nitpicks, so feel free to ignore these, of course.

If possible, capitalise the subject line.  Also, perhaps "Add quirk to
prevent bus (...)" might read better.

> Some TI keystone C667X devices do no support bus/hot reset. Its PCIESS
[...]

It would be KeyStone in the above sentence.

[...]
> With this change device can be assigned to VMs with VFIO, but it will
> leak state between VMs.

Following-up on Bojrn's question about the state leak, see:
  https://lore.kernel.org/linux-pci/20210129234946.GA124037@bjorn-Precision-5520/

Would there be anything else that has to be done?

Krzysztof

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v3] PCI: Add quirk for preventing bus reset on TI C667X
  2021-03-07  0:22       ` Krzysztof Wilczyński
@ 2021-03-08 14:21         ` Antti Järvinen
  2021-03-12 21:09           ` Bjorn Helgaas
  2021-03-08 14:28         ` [PATCH v2] PCI: " Antti Järvinen
  1 sibling, 1 reply; 14+ messages in thread
From: Antti Järvinen @ 2021-03-08 14:21 UTC (permalink / raw)
  To: kw
  Cc: alex.williamson, antti.jarvinen, bhelgaas, helgaas, kishon,
	linux-kernel, linux-pci, m-karicheri2

Some TI KeyStone C667X devices do no support bus/hot reset. Its PCIESS
automatically disables LTSSM when secondary bus reset is received and
device stops working. Prevent bus reset by adding quirk_no_bus_reset to
the device. With this change device can be assigned to VMs with VFIO,
but it will leak state between VMs.

Reference: https://e2e.ti.com/support/processors/f/791/t/954382
Signed-off-by: Antti Järvinen <antti.jarvinen@gmail.com>
---
 drivers/pci/quirks.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 653660e3ba9e..d9201ad1ca39 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -3578,6 +3578,16 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_ATHEROS, 0x0034, quirk_no_bus_reset);
  */
 DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_CAVIUM, 0xa100, quirk_no_bus_reset);
 
+/*
+ * Some TI keystone C667X devices do no support bus/hot reset.
+ * Its PCIESS automatically disables LTSSM when secondary bus reset is
+ * received and device stops working. Prevent bus reset by adding
+ * quirk_no_bus_reset to the device. With this change device can be
+ * assigned to VMs with VFIO, but it will leak state between VMs.
+ * Reference https://e2e.ti.com/support/processors/f/791/t/954382
+ */
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_TI, 0xb005, quirk_no_bus_reset);
+
 static void quirk_no_pm_reset(struct pci_dev *dev)
 {
 	/*
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] PCI: quirk for preventing bus reset on TI C667X
  2021-03-07  0:22       ` Krzysztof Wilczyński
  2021-03-08 14:21         ` [PATCH v3] PCI: Add " Antti Järvinen
@ 2021-03-08 14:28         ` Antti Järvinen
  1 sibling, 0 replies; 14+ messages in thread
From: Antti Järvinen @ 2021-03-08 14:28 UTC (permalink / raw)
  To: Krzysztof Wilczyński
  Cc: helgaas, alex.williamson, bhelgaas, kishon, linux-kernel,
	linux-pci, m-karicheri2



On 7.3.2021 2.22, Krzysztof Wilczyński wrote:
> Hi Antti,
> 
> A few nitpicks, so feel free to ignore these, of course.
> 
> If possible, capitalise the subject line.  Also, perhaps "Add quirk to
> prevent bus (...)" might read better.
> 
>> Some TI keystone C667X devices do no support bus/hot reset. Its PCIESS
> [...]
> 
> It would be KeyStone in the above sentence.
> 
> [...]
>> With this change device can be assigned to VMs with VFIO, but it will
>> leak state between VMs.
> 
> Following-up on Bojrn's question about the state leak, see:
>   https://lore.kernel.org/linux-pci/20210129234946.GA124037@bjorn-Precision-5520/
> 
> Would there be anything else that has to be done?
>

To my understanding this is all that needs to be done. At least on other similar
case, adding quirk was the only change https://lore.kernel.org/patchwork/patch/1086083/


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3] PCI: Add quirk for preventing bus reset on TI C667X
  2021-03-08 14:21         ` [PATCH v3] PCI: Add " Antti Järvinen
@ 2021-03-12 21:09           ` Bjorn Helgaas
  2021-03-15 10:26             ` [PATCH v4] " Antti Järvinen
  2021-03-15 10:45             ` [PATCH v3] " Antti Järvinen
  0 siblings, 2 replies; 14+ messages in thread
From: Bjorn Helgaas @ 2021-03-12 21:09 UTC (permalink / raw)
  To: Antti Järvinen
  Cc: kw, alex.williamson, bhelgaas, kishon, linux-kernel, linux-pci,
	m-karicheri2

On Mon, Mar 08, 2021 at 02:21:30PM +0000, Antti Järvinen wrote:
> Some TI KeyStone C667X devices do no support bus/hot reset. Its PCIESS
> automatically disables LTSSM when secondary bus reset is received and
> device stops working. Prevent bus reset by adding quirk_no_bus_reset to
> the device. With this change device can be assigned to VMs with VFIO,
> but it will leak state between VMs.

s/do no/do/not/ (also in the comment below)

Does the user get any indication of this leaking state?  I looked
through drivers/vfio and drivers/pci, but I haven't found anything
yet.

We *could* log something in quirk_no_bus_reset(), but that would just
be noise for people who don't pass the device through to a VM.  So
maybe it would be nicer if we logged something when we actually *do*
pass it through to a VM.

> Reference: https://e2e.ti.com/support/processors/f/791/t/954382
> Signed-off-by: Antti Järvinen <antti.jarvinen@gmail.com>
> ---
>  drivers/pci/quirks.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index 653660e3ba9e..d9201ad1ca39 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -3578,6 +3578,16 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_ATHEROS, 0x0034, quirk_no_bus_reset);
>   */
>  DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_CAVIUM, 0xa100, quirk_no_bus_reset);
>  
> +/*
> + * Some TI keystone C667X devices do no support bus/hot reset.
> + * Its PCIESS automatically disables LTSSM when secondary bus reset is
> + * received and device stops working. Prevent bus reset by adding
> + * quirk_no_bus_reset to the device. With this change device can be
> + * assigned to VMs with VFIO, but it will leak state between VMs.
> + * Reference https://e2e.ti.com/support/processors/f/791/t/954382
> + */
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_TI, 0xb005, quirk_no_bus_reset);
> +
>  static void quirk_no_pm_reset(struct pci_dev *dev)
>  {
>  	/*
> -- 
> 2.17.1
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v4] PCI: Add quirk for preventing bus reset on TI C667X
  2021-03-12 21:09           ` Bjorn Helgaas
@ 2021-03-15 10:26             ` Antti Järvinen
  2021-04-19 12:44               ` Kishon Vijay Abraham I
  2021-05-27 23:07               ` Bjorn Helgaas
  2021-03-15 10:45             ` [PATCH v3] " Antti Järvinen
  1 sibling, 2 replies; 14+ messages in thread
From: Antti Järvinen @ 2021-03-15 10:26 UTC (permalink / raw)
  To: helgaas
  Cc: alex.williamson, antti.jarvinen, bhelgaas, kishon, kw,
	linux-kernel, linux-pci, m-karicheri2

Some TI KeyStone C667X devices do not support bus/hot reset. Its PCIESS
automatically disables LTSSM when secondary bus reset is received and
device stops working. Prevent bus reset by adding quirk_no_bus_reset to
the device. With this change device can be assigned to VMs with VFIO,
but it will leak state between VMs.

Reference: https://e2e.ti.com/support/processors/f/791/t/954382
Signed-off-by: Antti Järvinen <antti.jarvinen@gmail.com>
---
 drivers/pci/quirks.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 653660e3ba9e..d9201ad1ca39 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -3578,6 +3578,16 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_ATHEROS, 0x0034, quirk_no_bus_reset);
  */
 DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_CAVIUM, 0xa100, quirk_no_bus_reset);
 
+/*
+ * Some TI keystone C667X devices do no support bus/hot reset.
+ * Its PCIESS automatically disables LTSSM when secondary bus reset is
+ * received and device stops working. Prevent bus reset by adding
+ * quirk_no_bus_reset to the device. With this change device can be
+ * assigned to VMs with VFIO, but it will leak state between VMs.
+ * Reference https://e2e.ti.com/support/processors/f/791/t/954382
+ */
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_TI, 0xb005, quirk_no_bus_reset);
+
 static void quirk_no_pm_reset(struct pci_dev *dev)
 {
 	/*
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v3] PCI: Add quirk for preventing bus reset on TI C667X
  2021-03-12 21:09           ` Bjorn Helgaas
  2021-03-15 10:26             ` [PATCH v4] " Antti Järvinen
@ 2021-03-15 10:45             ` Antti Järvinen
  1 sibling, 0 replies; 14+ messages in thread
From: Antti Järvinen @ 2021-03-15 10:45 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: kw, alex.williamson, bhelgaas, kishon, linux-kernel, linux-pci,
	m-karicheri2



On 12.3.2021 23.09, Bjorn Helgaas wrote:
> On Mon, Mar 08, 2021 at 02:21:30PM +0000, Antti Järvinen wrote:
>> Some TI KeyStone C667X devices do no support bus/hot reset. Its PCIESS
>> automatically disables LTSSM when secondary bus reset is received and
>> device stops working. Prevent bus reset by adding quirk_no_bus_reset to
>> the device. With this change device can be assigned to VMs with VFIO,
>> but it will leak state between VMs.
> 
> s/do no/do/not/ (also in the comment below)
> 

Should be fixed in v4 patch.
 
> Does the user get any indication of this leaking state?  I looked
> through drivers/vfio and drivers/pci, but I haven't found anything
> yet.
> 

I haven't seen any indication too. 

Overall I think all devices having this quirk will leak state, as they
don't get resetted.


> We *could* log something in quirk_no_bus_reset(), but that would just
> be noise for people who don't pass the device through to a VM.  So
> maybe it would be nicer if we logged something when we actually *do*
> pass it through to a VM.
> 
>> Reference: https://e2e.ti.com/support/processors/f/791/t/954382
>> Signed-off-by: Antti Järvinen <antti.jarvinen@gmail.com>
>> ---
>>  drivers/pci/quirks.c | 10 ++++++++++
>>  1 file changed, 10 insertions(+)
>>
>> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
>> index 653660e3ba9e..d9201ad1ca39 100644
>> --- a/drivers/pci/quirks.c
>> +++ b/drivers/pci/quirks.c
>> @@ -3578,6 +3578,16 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_ATHEROS, 0x0034, quirk_no_bus_reset);
>>   */
>>  DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_CAVIUM, 0xa100, quirk_no_bus_reset);
>>  
>> +/*
>> + * Some TI keystone C667X devices do no support bus/hot reset.
>> + * Its PCIESS automatically disables LTSSM when secondary bus reset is
>> + * received and device stops working. Prevent bus reset by adding
>> + * quirk_no_bus_reset to the device. With this change device can be
>> + * assigned to VMs with VFIO, but it will leak state between VMs.
>> + * Reference https://e2e.ti.com/support/processors/f/791/t/954382
>> + */
>> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_TI, 0xb005, quirk_no_bus_reset);
>> +
>>  static void quirk_no_pm_reset(struct pci_dev *dev)
>>  {
>>  	/*
>> -- 
>> 2.17.1
>>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v4] PCI: Add quirk for preventing bus reset on TI C667X
  2021-03-15 10:26             ` [PATCH v4] " Antti Järvinen
@ 2021-04-19 12:44               ` Kishon Vijay Abraham I
  2021-05-27 23:07               ` Bjorn Helgaas
  1 sibling, 0 replies; 14+ messages in thread
From: Kishon Vijay Abraham I @ 2021-04-19 12:44 UTC (permalink / raw)
  To: Antti Järvinen, helgaas
  Cc: alex.williamson, bhelgaas, kw, linux-kernel, linux-pci



On 15/03/21 3:56 pm, Antti Järvinen wrote:
> Some TI KeyStone C667X devices do not support bus/hot reset. Its PCIESS
> automatically disables LTSSM when secondary bus reset is received and
> device stops working. Prevent bus reset by adding quirk_no_bus_reset to
> the device. With this change device can be assigned to VMs with VFIO,
> but it will leak state between VMs.
> 
> Reference: https://e2e.ti.com/support/processors/f/791/t/954382
> Signed-off-by: Antti Järvinen <antti.jarvinen@gmail.com>

Reviewed-by: Kishon Vijay Abraham I <kishon@ti.com>
> ---
>  drivers/pci/quirks.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index 653660e3ba9e..d9201ad1ca39 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -3578,6 +3578,16 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_ATHEROS, 0x0034, quirk_no_bus_reset);
>   */
>  DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_CAVIUM, 0xa100, quirk_no_bus_reset);
>  
> +/*
> + * Some TI keystone C667X devices do no support bus/hot reset.
> + * Its PCIESS automatically disables LTSSM when secondary bus reset is
> + * received and device stops working. Prevent bus reset by adding
> + * quirk_no_bus_reset to the device. With this change device can be
> + * assigned to VMs with VFIO, but it will leak state between VMs.
> + * Reference https://e2e.ti.com/support/processors/f/791/t/954382
> + */
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_TI, 0xb005, quirk_no_bus_reset);
> +
>  static void quirk_no_pm_reset(struct pci_dev *dev)
>  {
>  	/*
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v4] PCI: Add quirk for preventing bus reset on TI C667X
  2021-03-15 10:26             ` [PATCH v4] " Antti Järvinen
  2021-04-19 12:44               ` Kishon Vijay Abraham I
@ 2021-05-27 23:07               ` Bjorn Helgaas
  1 sibling, 0 replies; 14+ messages in thread
From: Bjorn Helgaas @ 2021-05-27 23:07 UTC (permalink / raw)
  To: Antti Järvinen
  Cc: alex.williamson, bhelgaas, kishon, kw, linux-kernel, linux-pci,
	m-karicheri2

On Mon, Mar 15, 2021 at 10:26:06AM +0000, Antti Järvinen wrote:
> Some TI KeyStone C667X devices do not support bus/hot reset. Its PCIESS
> automatically disables LTSSM when secondary bus reset is received and
> device stops working. Prevent bus reset by adding quirk_no_bus_reset to
> the device. With this change device can be assigned to VMs with VFIO,
> but it will leak state between VMs.
> 
> Reference: https://e2e.ti.com/support/processors/f/791/t/954382
> Signed-off-by: Antti Järvinen <antti.jarvinen@gmail.com>

Applied to pci/virtualization for v5.14 with subject

  PCI: Mark TI C667X to avoid bus reset

Thanks!

> ---
>  drivers/pci/quirks.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index 653660e3ba9e..d9201ad1ca39 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -3578,6 +3578,16 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_ATHEROS, 0x0034, quirk_no_bus_reset);
>   */
>  DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_CAVIUM, 0xa100, quirk_no_bus_reset);
>  
> +/*
> + * Some TI keystone C667X devices do no support bus/hot reset.
> + * Its PCIESS automatically disables LTSSM when secondary bus reset is
> + * received and device stops working. Prevent bus reset by adding
> + * quirk_no_bus_reset to the device. With this change device can be
> + * assigned to VMs with VFIO, but it will leak state between VMs.
> + * Reference https://e2e.ti.com/support/processors/f/791/t/954382
> + */
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_TI, 0xb005, quirk_no_bus_reset);
> +
>  static void quirk_no_pm_reset(struct pci_dev *dev)
>  {
>  	/*
> -- 
> 2.17.1
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2021-05-27 23:07 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-12 15:36 [PATCH] PCI: quirk for preventing bus reset on TI C667X Antti Järvinen
2021-01-21 23:55 ` Bjorn Helgaas
2021-01-26 11:22   ` Antti Järvinen
2021-01-29 23:49     ` Bjorn Helgaas
2021-02-17 21:18   ` Bjorn Helgaas
2021-02-28 13:53     ` [PATCH v2] " Antti Järvinen
2021-03-07  0:22       ` Krzysztof Wilczyński
2021-03-08 14:21         ` [PATCH v3] PCI: Add " Antti Järvinen
2021-03-12 21:09           ` Bjorn Helgaas
2021-03-15 10:26             ` [PATCH v4] " Antti Järvinen
2021-04-19 12:44               ` Kishon Vijay Abraham I
2021-05-27 23:07               ` Bjorn Helgaas
2021-03-15 10:45             ` [PATCH v3] " Antti Järvinen
2021-03-08 14:28         ` [PATCH v2] PCI: " Antti Järvinen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).