Re: [coreboot] PCIe Role-based error reporting

[Zoran Stojsavljevic]

> Is that an error ?

I have looked into the document (BTW did some additional investigation) and
tried to find the attributes explanation (found it on page 586):

[image: Inline image 1]

The first possibility (my best guess) is: PCH (particular one. please, note
that) has numerous of so called HW straps (IO pins which are either
strapped to level 0 or level 1). Depending upon what some (Very Important)
particular IO is strapped, bit 15 (Role-based error-reporting in the DEVCAP
(offset 0x04)) could be enabled or disabled... So I think this is what this
explanation really does mean.

I would strongly advise you to add one printk() here and recompile
Coreboot, something like this:
/* Enable ASPM role based error reporting. */
devcap = pci_read_config32(endp, endp_cap + PCI_EXP_DEVCAP);
// #define PCI_EXP_DEVCAP_RBER
http://lxr.free-electrons.com/ident?i=PCI_EXP_DEVCAP_RBER 0x00008000 */*
Role-Based Error Reporting */*
*if (**devcap & PCI_EXP_DEVCAP_RBER) printk("Role-based Error Reporting
capability in the DEVCAP (offset 0x04) is ENABLED\n");*
devcap |= PCI_EXP_DEVCAP_RBER;
pci_write_config32(endp, endp_cap + PCI_EXP_DEVCAP, devcap);

Then, the Truth will reveal itself.

The second possibility is that you are correct. It should be enabled in
control register, which is RWable. But this is subject of the
implementation by Duncan Laurie <dlaurie at google.com
http://www.coreboot.org/mailman/listinfo/coreboot> in the following
patch: https://mail.coreboot.org/pipermail/coreboot/2012-March/068690.html

I'll include also involved (last web pointer is certainly a smoking gun,
isn't it?) in this matter.

Zoran
_______

On Wed, May 3, 2017 at 11:54 PM, Youness Alaoui <
[e-mail]> wrote:
> Hi,
>
> I'm looking at the src/device/pciexp_device.c file trying to understand
> what it does and I've noticed this in pciexp_enable_aspm :
> /* Enable ASPM role based error reporting. */
> devcap = pci_read_config32(endp, endp_cap + PCI_EXP_DEVCAP);
> devcap |= PCI_EXP_DEVCAP_RBER;
> pci_write_config32(endp, endp_cap + PCI_EXP_DEVCAP, devcap);
>
> This looks like it tries to write a bit to 1 to enable Role-based
> error-reporting in the DEVCAP (offset 0x04) in the PCIe capability, but
> according to the spec (looking at page 612 of the spec version 3.0 that I
> found here  : http://composter.com.ua/documents/PCI_Express_Base_Specif
> ication_Revision_3.0.pdf), that structure is read-only (it's the device
> capabilities register, not the device control register).
> Is that an error ?
> I tracked that code (moved, then moved again) all the way to this change
> from 2011 : https://review.coreboot.org/#/c/735/ but for some reason git
> blame can't find the file in commits before that.
>
> That code is probably harmless since it's a Read-only field anyways, but
> I'm wondering if the code that should be there shouldn't instead be
> enabling the bits 0:3 in the device control register to enable the error
> reporting. Also, I don't see how this actually relates to ASPM (it only
> gets called if ASPM is enabled).
>
> Does anyone know if I'm on the right track in my interpretation or if I
> misunderstood the spec or what that code is meant to do ?
>
> Thanks,
> Youness.