* [PATCH] PCI: quirks: Fix register location for UPDCR @ 2019-09-17 18:07 Steffen Liebergeld 2019-09-18 10:42 ` Andrew Murray 0 siblings, 1 reply; 7+ messages in thread From: Steffen Liebergeld @ 2019-09-17 18:07 UTC (permalink / raw) To: linux-pci; +Cc: Bjorn Helgaas According to documentation [0] the correct offset for the Upstream Peer Decode Configuration Register (UPDCR) is 0x1014. It was previously defined as 0x1114. This patch fixes it. [0] https://www.intel.com/content/dam/www/public/us/en/documents/datasheets/4th-gen-core-family-mobile-i-o-datasheet.pdf (page 325) Signed-off-by: Steffen Liebergeld <steffen.liebergeld@kernkonzept.com> --- drivers/pci/quirks.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c index 208aacf39329..7e184beb2aa4 100644 --- a/drivers/pci/quirks.c +++ b/drivers/pci/quirks.c @@ -4602,7 +4602,7 @@ int pci_dev_specific_acs_enabled(struct pci_dev *dev, u16 acs_flags) #define INTEL_BSPR_REG_BPPD (1 << 9) /* Upstream Peer Decode Configuration Register */ -#define INTEL_UPDCR_REG 0x1114 +#define INTEL_UPDCR_REG 0x1014 /* 5:0 Peer Decode Enable bits */ #define INTEL_UPDCR_REG_MASK 0x3f -- 2.11.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] PCI: quirks: Fix register location for UPDCR 2019-09-17 18:07 [PATCH] PCI: quirks: Fix register location for UPDCR Steffen Liebergeld @ 2019-09-18 10:42 ` Andrew Murray 2019-09-18 12:02 ` Steffen Liebergeld 0 siblings, 1 reply; 7+ messages in thread From: Andrew Murray @ 2019-09-18 10:42 UTC (permalink / raw) To: Steffen Liebergeld; +Cc: linux-pci, Bjorn Helgaas, Alex Williamson On Tue, Sep 17, 2019 at 08:07:13PM +0200, Steffen Liebergeld wrote: > According to documentation [0] the correct offset for the > Upstream Peer Decode Configuration Register (UPDCR) is 0x1014. > It was previously defined as 0x1114. This patch fixes it. > > [0] > https://www.intel.com/content/dam/www/public/us/en/documents/datasheets/4th-gen-core-family-mobile-i-o-datasheet.pdf > (page 325) > > Signed-off-by: Steffen Liebergeld <steffen.liebergeld@kernkonzept.com> You may also like to add: Fixes: d99321b63b1f ("PCI: Enable quirks for PCIe ACS on Intel PCH root ports") Reviewed-by: Andrew Murray <andrew.murray@arm.com> As well as CC'ing stable. I guess the side effect of this bug is that we claim to have peer isolation when we do not. This fix ensures that we get the advertised isolation. Thanks, Andrew Murray > --- > drivers/pci/quirks.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c > index 208aacf39329..7e184beb2aa4 100644 > --- a/drivers/pci/quirks.c > +++ b/drivers/pci/quirks.c > @@ -4602,7 +4602,7 @@ int pci_dev_specific_acs_enabled(struct pci_dev *dev, > u16 acs_flags) > #define INTEL_BSPR_REG_BPPD (1 << 9) > /* Upstream Peer Decode Configuration Register */ > -#define INTEL_UPDCR_REG 0x1114 > +#define INTEL_UPDCR_REG 0x1014 > /* 5:0 Peer Decode Enable bits */ > #define INTEL_UPDCR_REG_MASK 0x3f > -- 2.11.0 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] PCI: quirks: Fix register location for UPDCR 2019-09-18 10:42 ` Andrew Murray @ 2019-09-18 12:02 ` Steffen Liebergeld 2019-09-18 12:09 ` Andrew Murray 0 siblings, 1 reply; 7+ messages in thread From: Steffen Liebergeld @ 2019-09-18 12:02 UTC (permalink / raw) To: Andrew Murray; +Cc: linux-pci, Bjorn Helgaas, Alex Williamson On 18/09/2019 12:42, Andrew Murray wrote: > On Tue, Sep 17, 2019 at 08:07:13PM +0200, Steffen Liebergeld wrote: >> According to documentation [0] the correct offset for the >> Upstream Peer Decode Configuration Register (UPDCR) is 0x1014. >> It was previously defined as 0x1114. This patch fixes it. >> >> [0] >> https://www.intel.com/content/dam/www/public/us/en/documents/datasheets/4th-gen-core-family-mobile-i-o-datasheet.pdf >> (page 325) >> >> Signed-off-by: Steffen Liebergeld <steffen.liebergeld@kernkonzept.com> > > You may also like to add: > > Fixes: d99321b63b1f ("PCI: Enable quirks for PCIe ACS on Intel PCH root ports") > Reviewed-by: Andrew Murray <andrew.murray@arm.com> > > As well as CC'ing stable. Ok. Thank you. > I guess the side effect of this bug is that we claim to have peer > isolation when we do not. This fix ensures that we get the advertised > isolation. Yes, that is also my understanding. Should I explain that in the commit message? Best, Steffen -- Steffen Liebergeld +49-351-41 888 613 Kernkonzept GmbH. Sitz: Dresden. Amtsgericht Dresden, HRB 31129. Geschäftsführer: Dr.-Ing. Michael Hohmuth ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] PCI: quirks: Fix register location for UPDCR 2019-09-18 12:02 ` Steffen Liebergeld @ 2019-09-18 12:09 ` Andrew Murray 2019-09-18 16:46 ` Alex Williamson 0 siblings, 1 reply; 7+ messages in thread From: Andrew Murray @ 2019-09-18 12:09 UTC (permalink / raw) To: Steffen Liebergeld; +Cc: linux-pci, Bjorn Helgaas, Alex Williamson On Wed, Sep 18, 2019 at 02:02:59PM +0200, Steffen Liebergeld wrote: > On 18/09/2019 12:42, Andrew Murray wrote: > > On Tue, Sep 17, 2019 at 08:07:13PM +0200, Steffen Liebergeld wrote: > >> According to documentation [0] the correct offset for the > >> Upstream Peer Decode Configuration Register (UPDCR) is 0x1014. > >> It was previously defined as 0x1114. This patch fixes it. > >> > >> [0] > >> https://www.intel.com/content/dam/www/public/us/en/documents/datasheets/4th-gen-core-family-mobile-i-o-datasheet.pdf > >> (page 325) > >> > >> Signed-off-by: Steffen Liebergeld <steffen.liebergeld@kernkonzept.com> > > > > You may also like to add: > > > > Fixes: d99321b63b1f ("PCI: Enable quirks for PCIe ACS on Intel PCH root ports") > > Reviewed-by: Andrew Murray <andrew.murray@arm.com> > > > > As well as CC'ing stable. > > Ok. Thank you. > > > I guess the side effect of this bug is that we claim to have peer > > isolation when we do not. This fix ensures that we get the advertised > > isolation. > Yes, that is also my understanding. Should I explain that in the commit > message? I think something similar to that would be helpful. Thanks, Andrew Murray > > Best, > Steffen > -- > Steffen Liebergeld +49-351-41 888 613 > > Kernkonzept GmbH. Sitz: Dresden. Amtsgericht Dresden, HRB 31129. > Geschäftsführer: Dr.-Ing. Michael Hohmuth ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] PCI: quirks: Fix register location for UPDCR 2019-09-18 12:09 ` Andrew Murray @ 2019-09-18 16:46 ` Alex Williamson 2019-09-18 22:04 ` Alex Williamson 0 siblings, 1 reply; 7+ messages in thread From: Alex Williamson @ 2019-09-18 16:46 UTC (permalink / raw) To: Andrew Murray; +Cc: Steffen Liebergeld, linux-pci, Bjorn Helgaas, Raj, Ashok On Wed, 18 Sep 2019 13:09:18 +0100 Andrew Murray <andrew.murray@arm.com> wrote: > On Wed, Sep 18, 2019 at 02:02:59PM +0200, Steffen Liebergeld wrote: > > On 18/09/2019 12:42, Andrew Murray wrote: > > > On Tue, Sep 17, 2019 at 08:07:13PM +0200, Steffen Liebergeld wrote: > > >> According to documentation [0] the correct offset for the > > >> Upstream Peer Decode Configuration Register (UPDCR) is 0x1014. > > >> It was previously defined as 0x1114. This patch fixes it. > > >> > > >> [0] > > >> https://www.intel.com/content/dam/www/public/us/en/documents/datasheets/4th-gen-core-family-mobile-i-o-datasheet.pdf > > >> (page 325) > > >> > > >> Signed-off-by: Steffen Liebergeld <steffen.liebergeld@kernkonzept.com> > > > > > > You may also like to add: > > > > > > Fixes: d99321b63b1f ("PCI: Enable quirks for PCIe ACS on Intel PCH root ports") > > > Reviewed-by: Andrew Murray <andrew.murray@arm.com> > > > > > > As well as CC'ing stable. > > > > Ok. Thank you. > > > > > I guess the side effect of this bug is that we claim to have peer > > > isolation when we do not. This fix ensures that we get the advertised > > > isolation. > > Yes, that is also my understanding. Should I explain that in the commit > > message? > > I think something similar to that would be helpful. This is unfortunate, but my initial impression is that this may have just been a typo that slipped by everyone. It's difficult to actually test for isolation. Maybe someone from Intel could review this. Also, Steffen discussed this with me prior to posting and I believe this is untested, so while trivial from inspection, it would be preferable to know that some sample of hardware doesn't fall over as a result. Thanks, Alex ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] PCI: quirks: Fix register location for UPDCR 2019-09-18 16:46 ` Alex Williamson @ 2019-09-18 22:04 ` Alex Williamson 2019-09-18 22:07 ` Raj, Ashok 0 siblings, 1 reply; 7+ messages in thread From: Alex Williamson @ 2019-09-18 22:04 UTC (permalink / raw) To: Andrew Murray; +Cc: Steffen Liebergeld, linux-pci, Bjorn Helgaas, Raj, Ashok On Wed, 18 Sep 2019 10:46:51 -0600 Alex Williamson <alex.williamson@redhat.com> wrote: > On Wed, 18 Sep 2019 13:09:18 +0100 > Andrew Murray <andrew.murray@arm.com> wrote: > > > On Wed, Sep 18, 2019 at 02:02:59PM +0200, Steffen Liebergeld wrote: > > > On 18/09/2019 12:42, Andrew Murray wrote: > > > > On Tue, Sep 17, 2019 at 08:07:13PM +0200, Steffen Liebergeld wrote: > > > >> According to documentation [0] the correct offset for the > > > >> Upstream Peer Decode Configuration Register (UPDCR) is 0x1014. > > > >> It was previously defined as 0x1114. This patch fixes it. > > > >> > > > >> [0] > > > >> https://www.intel.com/content/dam/www/public/us/en/documents/datasheets/4th-gen-core-family-mobile-i-o-datasheet.pdf > > > >> (page 325) > > > >> > > > >> Signed-off-by: Steffen Liebergeld <steffen.liebergeld@kernkonzept.com> > > > > > > > > You may also like to add: > > > > > > > > Fixes: d99321b63b1f ("PCI: Enable quirks for PCIe ACS on Intel PCH root ports") > > > > Reviewed-by: Andrew Murray <andrew.murray@arm.com> > > > > > > > > As well as CC'ing stable. > > > > > > Ok. Thank you. > > > > > > > I guess the side effect of this bug is that we claim to have peer > > > > isolation when we do not. This fix ensures that we get the advertised > > > > isolation. > > > Yes, that is also my understanding. Should I explain that in the commit > > > message? > > > > I think something similar to that would be helpful. > > This is unfortunate, but my initial impression is that this may have > just been a typo that slipped by everyone. It's difficult to actually > test for isolation. Maybe someone from Intel could review this. Also, > Steffen discussed this with me prior to posting and I believe this is > untested, so while trivial from inspection, it would be preferable to > know that some sample of hardware doesn't fall over as a result. I've looked at 4 different systems, two 6-series (desktop + laptop), an 8-series desktop, and an X79 workstation. None of the 6/8 series even enter the branch where we read the UPDCR register, the value read from the BSPR register doesn't require it. In the case of the X79, using 0x1014 for UPDCR, the value read from the register is zero so code would not proceed into the inner branch to write the register, but using the current 0x1114 address, we read a non-zero value and changing it does stick on re-read. Neither address is defined in the public specs for this chipset, we're basing the information on word of mouth and ack from Intel as noted in commit 1a30fd0dba77. So of these systems, if 0x1014 is the correct UPDCR register address, nothing actually changes with respect to isolation other than we're not changing the value in mystery register 0x1114. Intel? Thanks, Alex ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] PCI: quirks: Fix register location for UPDCR 2019-09-18 22:04 ` Alex Williamson @ 2019-09-18 22:07 ` Raj, Ashok 0 siblings, 0 replies; 7+ messages in thread From: Raj, Ashok @ 2019-09-18 22:07 UTC (permalink / raw) To: Alex Williamson Cc: Andrew Murray, Steffen Liebergeld, linux-pci, Bjorn Helgaas, Ashok Raj Hi Alex On Wed, Sep 18, 2019 at 04:04:54PM -0600, Alex Williamson wrote: > > > > >> > > > > >> Signed-off-by: Steffen Liebergeld <steffen.liebergeld@kernkonzept.com> > > > > > > > > > > You may also like to add: > > > > > > > > > > Fixes: d99321b63b1f ("PCI: Enable quirks for PCIe ACS on Intel PCH root ports") > > > > > Reviewed-by: Andrew Murray <andrew.murray@arm.com> > > > > > > > > > > As well as CC'ing stable. > > > > > > > > Ok. Thank you. > > > > > > > > > I guess the side effect of this bug is that we claim to have peer > > > > > isolation when we do not. This fix ensures that we get the advertised > > > > > isolation. > > > > Yes, that is also my understanding. Should I explain that in the commit > > > > message? > > > > > > I think something similar to that would be helpful. > > > > This is unfortunate, but my initial impression is that this may have > > just been a typo that slipped by everyone. It's difficult to actually > > test for isolation. Maybe someone from Intel could review this. Also, > > Steffen discussed this with me prior to posting and I believe this is > > untested, so while trivial from inspection, it would be preferable to > > know that some sample of hardware doesn't fall over as a result. > > I've looked at 4 different systems, two 6-series (desktop + laptop), an > 8-series desktop, and an X79 workstation. None of the 6/8 series even > enter the branch where we read the UPDCR register, the value read from > the BSPR register doesn't require it. In the case of the X79, using > 0x1014 for UPDCR, the value read from the register is zero so code > would not proceed into the inner branch to write the register, but > using the current 0x1114 address, we read a non-zero value and changing > it does stick on re-read. Neither address is defined in the public > specs for this chipset, we're basing the information on word of mouth > and ack from Intel as noted in commit 1a30fd0dba77. > > So of these systems, if 0x1014 is the correct UPDCR register address, > nothing actually changes with respect to isolation other than we're not > changing the value in mystery register 0x1114. Intel? Thanks, I have asked the chipset folks if the spec has a typo.. waiting for an answer.. ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-09-18 22:07 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-09-17 18:07 [PATCH] PCI: quirks: Fix register location for UPDCR Steffen Liebergeld 2019-09-18 10:42 ` Andrew Murray 2019-09-18 12:02 ` Steffen Liebergeld 2019-09-18 12:09 ` Andrew Murray 2019-09-18 16:46 ` Alex Williamson 2019-09-18 22:04 ` Alex Williamson 2019-09-18 22:07 ` Raj, Ashok
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).