> 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):

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 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> 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 <kakaroto@kakaroto.homelinux.net> 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_Specification_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.

--
coreboot mailing list: coreboot@coreboot.org
https://mail.coreboot.org/mailman/listinfo/coreboot