All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] PCI: Mark Haswell Power Control Unit as having non-compliant BARs
@ 2016-08-31 15:50 Bjorn Helgaas
  2016-08-31 16:46 ` Myron Stowe
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Bjorn Helgaas @ 2016-08-31 15:50 UTC (permalink / raw)
  To: linux-pci
  Cc: Prarit Bhargava, Paul Menzel, Andi Kleen, Myron Stowe, linux-kernel

The Haswell Power Control Unit has a non-PCI register (CONFIG_TDP_NOMINAL)
where BAR 0 is supposed to be.  This is erratum HSE43 in the spec update
referenced below:

  The PCIe* Base Specification indicates that Configuration Space Headers
  have a base address register at offset 0x10.  Due to this erratum, the
  Power Control Unit's CONFIG_TDP_NOMINAL CSR (Bus 1; Device 30; Function
  3; Offset 0x10) is located where a base register is expected.

Mark the PCU as having non-compliant BARs so we don't try to probe any of
them.  There are no other BARs on this device.

Rename the quirk so it's not Broadwell-specific.

Link: http://www.intel.com/content/www/us/en/processors/xeon/xeon-e5-v3-spec-update.html
Link: https://bugzilla.kernel.org/show_bug.cgi?id=153881
Reported-by: Paul Menzel <pmenzel@molgen.mpg.de>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 arch/x86/pci/fixup.c |   20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/arch/x86/pci/fixup.c b/arch/x86/pci/fixup.c
index 837ea36..6d52b94 100644
--- a/arch/x86/pci/fixup.c
+++ b/arch/x86/pci/fixup.c
@@ -553,15 +553,21 @@ static void twinhead_reserve_killing_zone(struct pci_dev *dev)
 DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x27B9, twinhead_reserve_killing_zone);
 
 /*
- * Broadwell EP Home Agent BARs erroneously return non-zero values when read.
+ * Device [8086:2fc0]
+ * Erratum HSE43
+ * CONFIG_TDP_NOMINAL CSR Implemented at Incorrect Offset
+ * http://www.intel.com/content/www/us/en/processors/xeon/xeon-e5-v3-spec-update.html
  *
- * See http://www.intel.com/content/www/us/en/processors/xeon/xeon-e5-v4-spec-update.html
- * entry BDF2.
+ * Devices [8086:6f60,6fa0,6fc0]
+ * Erratum BDF2
+ * PCI BARs in the Home Agent Will Return Non-Zero Values During Enumeration
+ * http://www.intel.com/content/www/us/en/processors/xeon/xeon-e5-v4-spec-update.html
  */
-static void pci_bdwep_bar(struct pci_dev *dev)
+static void pci_invalid_bar(struct pci_dev *dev)
 {
 	dev->non_compliant_bars = 1;
 }
-DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x6f60, pci_bdwep_bar);
-DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x6fa0, pci_bdwep_bar);
-DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x6fc0, pci_bdwep_bar);
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x2fc0, pci_invalid_bar);
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x6f60, pci_invalid_bar);
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x6fa0, pci_invalid_bar);
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x6fc0, pci_invalid_bar);

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

* Re: [PATCH] PCI: Mark Haswell Power Control Unit as having non-compliant BARs
  2016-08-31 15:50 [PATCH] PCI: Mark Haswell Power Control Unit as having non-compliant BARs Bjorn Helgaas
@ 2016-08-31 16:46 ` Myron Stowe
  2016-08-31 17:12   ` Prarit Bhargava
  2016-08-31 17:42 ` Prarit Bhargava
  2016-09-01 13:53 ` Bjorn Helgaas
  2 siblings, 1 reply; 7+ messages in thread
From: Myron Stowe @ 2016-08-31 16:46 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: linux-pci, Prarit Bhargava, Paul Menzel, Andi Kleen, Myron Stowe, LKML

On Wed, Aug 31, 2016 at 9:50 AM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> The Haswell Power Control Unit has a non-PCI register (CONFIG_TDP_NOMINAL)
> where BAR 0 is supposed to be.  This is erratum HSE43 in the spec update
> referenced below:
>
>   The PCIe* Base Specification indicates that Configuration Space Headers
>   have a base address register at offset 0x10.  Due to this erratum, the
>   Power Control Unit's CONFIG_TDP_NOMINAL CSR (Bus 1; Device 30; Function
>   3; Offset 0x10) is located where a base register is expected.
>
> Mark the PCU as having non-compliant BARs so we don't try to probe any of
> them.  There are no other BARs on this device.
>
> Rename the quirk so it's not Broadwell-specific.

While it's not a big enough battle to fight it sure is irritating to
see vendors make the same mistakes over and over again.  Wondering if
this an error on the respective vendor's BIOS team's part or is this
is just being inherited over and over again from Intel's Premier BIOS
team reference code?

>
> Link: http://www.intel.com/content/www/us/en/processors/xeon/xeon-e5-v3-spec-update.html
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=153881
> Reported-by: Paul Menzel <pmenzel@molgen.mpg.de>
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> ---
>  arch/x86/pci/fixup.c |   20 +++++++++++++-------
>  1 file changed, 13 insertions(+), 7 deletions(-)
>
> diff --git a/arch/x86/pci/fixup.c b/arch/x86/pci/fixup.c
> index 837ea36..6d52b94 100644
> --- a/arch/x86/pci/fixup.c
> +++ b/arch/x86/pci/fixup.c
> @@ -553,15 +553,21 @@ static void twinhead_reserve_killing_zone(struct pci_dev *dev)
>  DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x27B9, twinhead_reserve_killing_zone);
>
>  /*
> - * Broadwell EP Home Agent BARs erroneously return non-zero values when read.
> + * Device [8086:2fc0]
> + * Erratum HSE43
> + * CONFIG_TDP_NOMINAL CSR Implemented at Incorrect Offset
> + * http://www.intel.com/content/www/us/en/processors/xeon/xeon-e5-v3-spec-update.html
>   *
> - * See http://www.intel.com/content/www/us/en/processors/xeon/xeon-e5-v4-spec-update.html
> - * entry BDF2.
> + * Devices [8086:6f60,6fa0,6fc0]
> + * Erratum BDF2
> + * PCI BARs in the Home Agent Will Return Non-Zero Values During Enumeration
> + * http://www.intel.com/content/www/us/en/processors/xeon/xeon-e5-v4-spec-update.html
>   */
> -static void pci_bdwep_bar(struct pci_dev *dev)
> +static void pci_invalid_bar(struct pci_dev *dev)
>  {
>         dev->non_compliant_bars = 1;
>  }
> -DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x6f60, pci_bdwep_bar);
> -DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x6fa0, pci_bdwep_bar);
> -DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x6fc0, pci_bdwep_bar);
> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x2fc0, pci_invalid_bar);
> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x6f60, pci_invalid_bar);
> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x6fa0, pci_invalid_bar);
> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x6fc0, pci_invalid_bar);
>

Acked-by: Myron Stowe <myron.stowe@redhat.com>

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

* Re: [PATCH] PCI: Mark Haswell Power Control Unit as having non-compliant BARs
  2016-08-31 16:46 ` Myron Stowe
@ 2016-08-31 17:12   ` Prarit Bhargava
  2016-08-31 17:45     ` Myron Stowe
  0 siblings, 1 reply; 7+ messages in thread
From: Prarit Bhargava @ 2016-08-31 17:12 UTC (permalink / raw)
  To: Myron Stowe, Bjorn Helgaas
  Cc: linux-pci, Paul Menzel, Andi Kleen, Myron Stowe, LKML

On 08/31/2016 12:46 PM, Myron Stowe wrote:
> On Wed, Aug 31, 2016 at 9:50 AM, Bjorn Helgaas <bhelgaas@google.com> wrote:
>> The Haswell Power Control Unit has a non-PCI register (CONFIG_TDP_NOMINAL)
>> where BAR 0 is supposed to be.  This is erratum HSE43 in the spec update
>> referenced below:
>>
>>   The PCIe* Base Specification indicates that Configuration Space Headers
>>   have a base address register at offset 0x10.  Due to this erratum, the
>>   Power Control Unit's CONFIG_TDP_NOMINAL CSR (Bus 1; Device 30; Function
>>   3; Offset 0x10) is located where a base register is expected.
>>
>> Mark the PCU as having non-compliant BARs so we don't try to probe any of
>> them.  There are no other BARs on this device.
>>
>> Rename the quirk so it's not Broadwell-specific.
> 
> While it's not a big enough battle to fight it sure is irritating to
> see vendors make the same mistakes over and over again.  Wondering if
> this an error on the respective vendor's BIOS team's part or is this
> is just being inherited over and over again from Intel's Premier BIOS
> team reference code?
> 

This isn't a BIOS issue.  The actual hardware is manufactured this
way, and the documentation indicates that the hardware has
non-compliant BARs.

Testing and reviewing now ...

P.



>>
>> Link: http://www.intel.com/content/www/us/en/processors/xeon/xeon-e5-v3-spec-update.html
>> Link: https://bugzilla.kernel.org/show_bug.cgi?id=153881
>> Reported-by: Paul Menzel <pmenzel@molgen.mpg.de>
>> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
>> ---
>>  arch/x86/pci/fixup.c |   20 +++++++++++++-------
>>  1 file changed, 13 insertions(+), 7 deletions(-)
>>
>> diff --git a/arch/x86/pci/fixup.c b/arch/x86/pci/fixup.c
>> index 837ea36..6d52b94 100644
>> --- a/arch/x86/pci/fixup.c
>> +++ b/arch/x86/pci/fixup.c
>> @@ -553,15 +553,21 @@ static void twinhead_reserve_killing_zone(struct pci_dev *dev)
>>  DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x27B9, twinhead_reserve_killing_zone);
>>
>>  /*
>> - * Broadwell EP Home Agent BARs erroneously return non-zero values when read.
>> + * Device [8086:2fc0]
>> + * Erratum HSE43
>> + * CONFIG_TDP_NOMINAL CSR Implemented at Incorrect Offset
>> + * http://www.intel.com/content/www/us/en/processors/xeon/xeon-e5-v3-spec-update.html
>>   *
>> - * See http://www.intel.com/content/www/us/en/processors/xeon/xeon-e5-v4-spec-update.html
>> - * entry BDF2.
>> + * Devices [8086:6f60,6fa0,6fc0]
>> + * Erratum BDF2
>> + * PCI BARs in the Home Agent Will Return Non-Zero Values During Enumeration
>> + * http://www.intel.com/content/www/us/en/processors/xeon/xeon-e5-v4-spec-update.html
>>   */
>> -static void pci_bdwep_bar(struct pci_dev *dev)
>> +static void pci_invalid_bar(struct pci_dev *dev)
>>  {
>>         dev->non_compliant_bars = 1;
>>  }
>> -DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x6f60, pci_bdwep_bar);
>> -DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x6fa0, pci_bdwep_bar);
>> -DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x6fc0, pci_bdwep_bar);
>> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x2fc0, pci_invalid_bar);
>> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x6f60, pci_invalid_bar);
>> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x6fa0, pci_invalid_bar);
>> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x6fc0, pci_invalid_bar);
>>
> 
> Acked-by: Myron Stowe <myron.stowe@redhat.com>
> 
> 

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

* Re: [PATCH] PCI: Mark Haswell Power Control Unit as having non-compliant BARs
  2016-08-31 15:50 [PATCH] PCI: Mark Haswell Power Control Unit as having non-compliant BARs Bjorn Helgaas
  2016-08-31 16:46 ` Myron Stowe
@ 2016-08-31 17:42 ` Prarit Bhargava
  2016-08-31 17:55   ` Bjorn Helgaas
  2016-09-01 13:53 ` Bjorn Helgaas
  2 siblings, 1 reply; 7+ messages in thread
From: Prarit Bhargava @ 2016-08-31 17:42 UTC (permalink / raw)
  To: Bjorn Helgaas, linux-pci
  Cc: Paul Menzel, Andi Kleen, Myron Stowe, linux-kernel

On 08/31/2016 11:50 AM, Bjorn Helgaas wrote:
> The Haswell Power Control Unit has a non-PCI register (CONFIG_TDP_NOMINAL)
> where BAR 0 is supposed to be.  This is erratum HSE43 in the spec update
> referenced below:
> 
>   The PCIe* Base Specification indicates that Configuration Space Headers
>   have a base address register at offset 0x10.  Due to this erratum, the
>   Power Control Unit's CONFIG_TDP_NOMINAL CSR (Bus 1; Device 30; Function
>   3; Offset 0x10) is located where a base register is expected.
> 
> Mark the PCU as having non-compliant BARs so we don't try to probe any of
> them.  There are no other BARs on this device.
> 
> Rename the quirk so it's not Broadwell-specific.
> 
> Link: http://www.intel.com/content/www/us/en/processors/xeon/xeon-e5-v3-spec-update.html
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=153881
> Reported-by: Paul Menzel <pmenzel@molgen.mpg.de>
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> ---
>  arch/x86/pci/fixup.c |   20 +++++++++++++-------
>  1 file changed, 13 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/x86/pci/fixup.c b/arch/x86/pci/fixup.c
> index 837ea36..6d52b94 100644
> --- a/arch/x86/pci/fixup.c
> +++ b/arch/x86/pci/fixup.c
> @@ -553,15 +553,21 @@ static void twinhead_reserve_killing_zone(struct pci_dev *dev)
>  DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x27B9, twinhead_reserve_killing_zone);
>  
>  /*
> - * Broadwell EP Home Agent BARs erroneously return non-zero values when read.
> + * Device [8086:2fc0]
> + * Erratum HSE43
> + * CONFIG_TDP_NOMINAL CSR Implemented at Incorrect Offset
> + * http://www.intel.com/content/www/us/en/processors/xeon/xeon-e5-v3-spec-update.html
>   *

Bjorn,

I really think you should point at this doc:

http://www.intel.com/content/www/us/en/processors/xeon/xeon-e5-v3-datasheet-vol-2.html

Section 5.4, Device 30 Function 3

which states (at the bottom of the config space diagram) "Note: The CSR located
at offset in Device 30, Function 3, Offset 0x10 is not a Configuration Space
Header and SW should not treat it as such."

as it is more informative of the issue.

I'm also slightly concerned about blacklisting the rest of the config space, but
I don't see any systems that have non-zero BARs but that's a lesser issue.

Beyond that, testing went well.

P.

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

* Re: [PATCH] PCI: Mark Haswell Power Control Unit as having non-compliant BARs
  2016-08-31 17:12   ` Prarit Bhargava
@ 2016-08-31 17:45     ` Myron Stowe
  0 siblings, 0 replies; 7+ messages in thread
From: Myron Stowe @ 2016-08-31 17:45 UTC (permalink / raw)
  To: Prarit Bhargava
  Cc: Bjorn Helgaas, linux-pci, Paul Menzel, Andi Kleen, Myron Stowe, LKML

On Wed, Aug 31, 2016 at 11:12 AM, Prarit Bhargava <prarit@redhat.com> wrote:
> On 08/31/2016 12:46 PM, Myron Stowe wrote:
>> On Wed, Aug 31, 2016 at 9:50 AM, Bjorn Helgaas <bhelgaas@google.com> wrote:
>>> The Haswell Power Control Unit has a non-PCI register (CONFIG_TDP_NOMINAL)
>>> where BAR 0 is supposed to be.  This is erratum HSE43 in the spec update
>>> referenced below:
>>>
>>>   The PCIe* Base Specification indicates that Configuration Space Headers
>>>   have a base address register at offset 0x10.  Due to this erratum, the
>>>   Power Control Unit's CONFIG_TDP_NOMINAL CSR (Bus 1; Device 30; Function
>>>   3; Offset 0x10) is located where a base register is expected.
>>>
>>> Mark the PCU as having non-compliant BARs so we don't try to probe any of
>>> them.  There are no other BARs on this device.
>>>
>>> Rename the quirk so it's not Broadwell-specific.
>>
>> While it's not a big enough battle to fight it sure is irritating to
>> see vendors make the same mistakes over and over again.  Wondering if
>> this an error on the respective vendor's BIOS team's part or is this
>> is just being inherited over and over again from Intel's Premier BIOS
>> team reference code?
>>
>
> This isn't a BIOS issue.  The actual hardware is manufactured this
> way, and the documentation indicates that the hardware has
> non-compliant BARs.

Granted, at the root it is a HW problem, and one that seems Intel
duplicated forward.  That said, the workaround for the HW is for BIOS
to "hide" the register which is what was done in the prior instance.
I've always wondered how BIOS does that so if anyone can explain I'd
be interested in learning that.

>
> Testing and reviewing now ...
>
> P.
>
>
>
>>>
>>> Link: http://www.intel.com/content/www/us/en/processors/xeon/xeon-e5-v3-spec-update.html
>>> Link: https://bugzilla.kernel.org/show_bug.cgi?id=153881
>>> Reported-by: Paul Menzel <pmenzel@molgen.mpg.de>
>>> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
>>> ---
>>>  arch/x86/pci/fixup.c |   20 +++++++++++++-------
>>>  1 file changed, 13 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/arch/x86/pci/fixup.c b/arch/x86/pci/fixup.c
>>> index 837ea36..6d52b94 100644
>>> --- a/arch/x86/pci/fixup.c
>>> +++ b/arch/x86/pci/fixup.c
>>> @@ -553,15 +553,21 @@ static void twinhead_reserve_killing_zone(struct pci_dev *dev)
>>>  DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x27B9, twinhead_reserve_killing_zone);
>>>
>>>  /*
>>> - * Broadwell EP Home Agent BARs erroneously return non-zero values when read.
>>> + * Device [8086:2fc0]
>>> + * Erratum HSE43
>>> + * CONFIG_TDP_NOMINAL CSR Implemented at Incorrect Offset
>>> + * http://www.intel.com/content/www/us/en/processors/xeon/xeon-e5-v3-spec-update.html
>>>   *
>>> - * See http://www.intel.com/content/www/us/en/processors/xeon/xeon-e5-v4-spec-update.html
>>> - * entry BDF2.
>>> + * Devices [8086:6f60,6fa0,6fc0]
>>> + * Erratum BDF2
>>> + * PCI BARs in the Home Agent Will Return Non-Zero Values During Enumeration
>>> + * http://www.intel.com/content/www/us/en/processors/xeon/xeon-e5-v4-spec-update.html
>>>   */
>>> -static void pci_bdwep_bar(struct pci_dev *dev)
>>> +static void pci_invalid_bar(struct pci_dev *dev)
>>>  {
>>>         dev->non_compliant_bars = 1;
>>>  }
>>> -DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x6f60, pci_bdwep_bar);
>>> -DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x6fa0, pci_bdwep_bar);
>>> -DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x6fc0, pci_bdwep_bar);
>>> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x2fc0, pci_invalid_bar);
>>> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x6f60, pci_invalid_bar);
>>> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x6fa0, pci_invalid_bar);
>>> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x6fc0, pci_invalid_bar);
>>>
>>
>> Acked-by: Myron Stowe <myron.stowe@redhat.com>
>>
>>
>

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

* Re: [PATCH] PCI: Mark Haswell Power Control Unit as having non-compliant BARs
  2016-08-31 17:42 ` Prarit Bhargava
@ 2016-08-31 17:55   ` Bjorn Helgaas
  0 siblings, 0 replies; 7+ messages in thread
From: Bjorn Helgaas @ 2016-08-31 17:55 UTC (permalink / raw)
  To: Prarit Bhargava
  Cc: Bjorn Helgaas, linux-pci, Paul Menzel, Andi Kleen, Myron Stowe,
	linux-kernel

On Wed, Aug 31, 2016 at 01:42:13PM -0400, Prarit Bhargava wrote:
> On 08/31/2016 11:50 AM, Bjorn Helgaas wrote:
> > The Haswell Power Control Unit has a non-PCI register (CONFIG_TDP_NOMINAL)
> > where BAR 0 is supposed to be.  This is erratum HSE43 in the spec update
> > referenced below:
> > 
> >   The PCIe* Base Specification indicates that Configuration Space Headers
> >   have a base address register at offset 0x10.  Due to this erratum, the
> >   Power Control Unit's CONFIG_TDP_NOMINAL CSR (Bus 1; Device 30; Function
> >   3; Offset 0x10) is located where a base register is expected.
> > 
> > Mark the PCU as having non-compliant BARs so we don't try to probe any of
> > them.  There are no other BARs on this device.
> > 
> > Rename the quirk so it's not Broadwell-specific.
> > 
> > Link: http://www.intel.com/content/www/us/en/processors/xeon/xeon-e5-v3-spec-update.html
> > Link: https://bugzilla.kernel.org/show_bug.cgi?id=153881
> > Reported-by: Paul Menzel <pmenzel@molgen.mpg.de>
> > Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> > ---
> >  arch/x86/pci/fixup.c |   20 +++++++++++++-------
> >  1 file changed, 13 insertions(+), 7 deletions(-)
> > 
> > diff --git a/arch/x86/pci/fixup.c b/arch/x86/pci/fixup.c
> > index 837ea36..6d52b94 100644
> > --- a/arch/x86/pci/fixup.c
> > +++ b/arch/x86/pci/fixup.c
> > @@ -553,15 +553,21 @@ static void twinhead_reserve_killing_zone(struct pci_dev *dev)
> >  DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x27B9, twinhead_reserve_killing_zone);
> >  
> >  /*
> > - * Broadwell EP Home Agent BARs erroneously return non-zero values when read.
> > + * Device [8086:2fc0]
> > + * Erratum HSE43
> > + * CONFIG_TDP_NOMINAL CSR Implemented at Incorrect Offset
> > + * http://www.intel.com/content/www/us/en/processors/xeon/xeon-e5-v3-spec-update.html
> >   *
> 
> Bjorn,
> 
> I really think you should point at this doc:
> 
> http://www.intel.com/content/www/us/en/processors/xeon/xeon-e5-v3-datasheet-vol-2.html
> 
> Section 5.4, Device 30 Function 3
> 
> which states (at the bottom of the config space diagram) "Note: The CSR located
> at offset in Device 30, Function 3, Offset 0x10 is not a Configuration Space
> Header and SW should not treat it as such."
> 
> as it is more informative of the issue.
> 
> I'm also slightly concerned about blacklisting the rest of the config space, but
> I don't see any systems that have non-zero BARs but that's a lesser issue.
> 
> Beyond that, testing went well.

I added the link to the changelog, but I don't think it really adds
much about the issue.  It basically says "we screwed up and generic
PCI BAR sizing SW should magically ignore the register at 0x10."

Thanks for testing it; I added your Tested-by.

Bjorn

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

* Re: [PATCH] PCI: Mark Haswell Power Control Unit as having non-compliant BARs
  2016-08-31 15:50 [PATCH] PCI: Mark Haswell Power Control Unit as having non-compliant BARs Bjorn Helgaas
  2016-08-31 16:46 ` Myron Stowe
  2016-08-31 17:42 ` Prarit Bhargava
@ 2016-09-01 13:53 ` Bjorn Helgaas
  2 siblings, 0 replies; 7+ messages in thread
From: Bjorn Helgaas @ 2016-09-01 13:53 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: linux-pci, Prarit Bhargava, Paul Menzel, Andi Kleen, Myron Stowe,
	linux-kernel

On Wed, Aug 31, 2016 at 10:50:09AM -0500, Bjorn Helgaas wrote:
> The Haswell Power Control Unit has a non-PCI register (CONFIG_TDP_NOMINAL)
> where BAR 0 is supposed to be.  This is erratum HSE43 in the spec update
> referenced below:
> 
>   The PCIe* Base Specification indicates that Configuration Space Headers
>   have a base address register at offset 0x10.  Due to this erratum, the
>   Power Control Unit's CONFIG_TDP_NOMINAL CSR (Bus 1; Device 30; Function
>   3; Offset 0x10) is located where a base register is expected.
> 
> Mark the PCU as having non-compliant BARs so we don't try to probe any of
> them.  There are no other BARs on this device.
> 
> Rename the quirk so it's not Broadwell-specific.
> 
> Link: http://www.intel.com/content/www/us/en/processors/xeon/xeon-e5-v3-spec-update.html
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=153881
> Reported-by: Paul Menzel <pmenzel@molgen.mpg.de>
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>

I applied this with Myron's ack and Prarit's tested-by to for-linus
for v4.8.

> ---
>  arch/x86/pci/fixup.c |   20 +++++++++++++-------
>  1 file changed, 13 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/x86/pci/fixup.c b/arch/x86/pci/fixup.c
> index 837ea36..6d52b94 100644
> --- a/arch/x86/pci/fixup.c
> +++ b/arch/x86/pci/fixup.c
> @@ -553,15 +553,21 @@ static void twinhead_reserve_killing_zone(struct pci_dev *dev)
>  DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x27B9, twinhead_reserve_killing_zone);
>  
>  /*
> - * Broadwell EP Home Agent BARs erroneously return non-zero values when read.
> + * Device [8086:2fc0]
> + * Erratum HSE43
> + * CONFIG_TDP_NOMINAL CSR Implemented at Incorrect Offset
> + * http://www.intel.com/content/www/us/en/processors/xeon/xeon-e5-v3-spec-update.html
>   *
> - * See http://www.intel.com/content/www/us/en/processors/xeon/xeon-e5-v4-spec-update.html
> - * entry BDF2.
> + * Devices [8086:6f60,6fa0,6fc0]
> + * Erratum BDF2
> + * PCI BARs in the Home Agent Will Return Non-Zero Values During Enumeration
> + * http://www.intel.com/content/www/us/en/processors/xeon/xeon-e5-v4-spec-update.html
>   */
> -static void pci_bdwep_bar(struct pci_dev *dev)
> +static void pci_invalid_bar(struct pci_dev *dev)
>  {
>  	dev->non_compliant_bars = 1;
>  }
> -DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x6f60, pci_bdwep_bar);
> -DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x6fa0, pci_bdwep_bar);
> -DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x6fc0, pci_bdwep_bar);
> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x2fc0, pci_invalid_bar);
> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x6f60, pci_invalid_bar);
> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x6fa0, pci_invalid_bar);
> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x6fc0, pci_invalid_bar);
> 

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

end of thread, other threads:[~2016-09-01 13:53 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-31 15:50 [PATCH] PCI: Mark Haswell Power Control Unit as having non-compliant BARs Bjorn Helgaas
2016-08-31 16:46 ` Myron Stowe
2016-08-31 17:12   ` Prarit Bhargava
2016-08-31 17:45     ` Myron Stowe
2016-08-31 17:42 ` Prarit Bhargava
2016-08-31 17:55   ` Bjorn Helgaas
2016-09-01 13:53 ` Bjorn Helgaas

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.