* [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 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 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
* 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 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
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).