<div dir="ltr">Hi Zoran,<div><br></div><div>I tried your changes and it is just as I expected, the register is read-only. It's always enabled (the spec seemed to indicate that all devices need to have that bit set to 1). I even tried to disable it but it had no effect (read-only).</div><div><br></div><div>The thing is that there is no RBER flag in the control register, there's just various error reporting flags (correctable, fatal, non-fatal and unsupported-request errors) and I think those 4 flags are the different 'roles' in the role-based error reporting. Since all PCIe devices since 1.0a need to support role-based error reporting, that value will always be set to 1. Trying to change it is useless since it's a read-only register. I still fail to see the link between RBER and ASPM.</div><div><br></div><div>Duncan Laurie isn't the one who added that code by the way, if you look at the diff in the link you sent (same change as <span style="font-size:12.8px"> </span><a href="https://review.coreboot.org/#/c/735/" target="_blank" style="font-size:12.8px">https://review.coreboot.org/#/<wbr>c/735</a>), then you'll see that code was already there, Duncan simply put it inside a "if (apmc != PCIE_ASPM_NONE)" and removed the comment "TODO make this dependent on ASPM".</div><div><br></div><div>Thanks,</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, May 4, 2017 at 8:42 AM, Zoran Stojsavljevic <span dir="ltr"><<a href="mailto:zoran.stojsavljevic@gmail.com" target="_blank">zoran.stojsavljevic@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">I thought more about this problem, and how it is easy to miss the complete point?! :-(<div><br></div><div>Revisited C code to be added (in <b><i><u><font color="#ff0000">RED</font></u></i></b>):</div><div><span class=""><span class="m_-5624364039645036717gmail-im" style="font-size:12.8px"><div style="font-size:12.8px"><span style="font-size:12.8px;white-space:pre-wrap"> </span>/* Enable ASPM role based error reporting. */</div><div style="font-size:12.8px"><span class="m_-5624364039645036717gmail-m_-3881861314826159405gmail-m_4371082674194637721gmail-m_6920781638293347396gmail-m_4420435145451042919gmail-Apple-tab-span" style="white-space:pre-wrap"> </span>devcap = pci_read_config32(endp, endp_cap + PCI_EXP_DEVCAP);</div></span><div style="font-size:12.8px"><span style="font-size:12.8px;white-space:pre-wrap"> <b><i><u><font color="#ff0000">// </font></u></i></b></span><b><i><u><font color="#ff0000"><span style="font-family:monaco,"courier new",courier,monospace;font-size:0.95em">#define </span><a href="http://lxr.free-electrons.com/ident?i=PCI_EXP_DEVCAP_RBER" style="font-family:monaco,"courier new",courier,monospace;font-size:0.95em;border-bottom:1px dotted rgb(153,153,153)" target="_blank">PCI_EXP_DEVCAP_RBER</a><span style="font-family:monaco,"courier new",courier,monospace;font-size:0.95em"> <wbr>0x00008000 </span><span style="font-family:monaco,"courier new",courier,monospace;font-size:0.95em">/* Role-Based Error Reporting */</span></font></u></i></b><br></div><div style="font-size:12.8px"><span class="m_-5624364039645036717gmail-m_-3881861314826159405gmail-m_4371082674194637721gmail-m_6920781638293347396gmail-m_4420435145451042919gmail-Apple-tab-span" style="white-space:pre-wrap"> <b><i><u><font color="#ff0000">if (</font></u></i></b></span><b><i><u><font color="#ff0000">devcap & PCI_EXP_DEVCAP_RBER) {</font></u></i></b></div></span><div style="font-size:12.8px"><span style="font-size:12.8px;white-space:pre-wrap"> </span><span style="font-size:12.8px;white-space:pre-wrap"> </span><b style="font-size:12.8px"><i><u><font color="#ff0000">printk("<span style="font-size:12.8px">Role-based Error Reporting capability in the DEVCAP (offset 0x04) is initially ENABLED\n");</span></font></u></i></b></div><div style="font-size:12.8px"><span style="font-size:12.8px;white-space:pre-wrap"> <b><i><u><font color="#ff0000">else</font></u></i></b></span><b style="font-size:12.8px"><i><u><font color="#ff0000"><span style="font-size:12.8px"><br></span></font></u></i></b></div><div style="font-size:12.8px"><span style="font-size:12.8px;white-space:pre-wrap"> </span><span style="font-size:12.8px;white-space:pre-wrap"> </span><b style="font-size:12.8px"><i><u><font color="#ff0000">printk("<span style="font-size:12.8px">Role-based Error Reporting capability in the DEVCAP (offset 0x04) is initially DISABLED\n");</span></font></u></i></b></div><span class="m_-5624364039645036717gmail-im" style="font-size:12.8px"><span class=""><div style="font-size:12.8px"><span style="font-size:12.8px;white-space:pre-wrap"> <b><i><u><font color="#ff0000">}</font></u></i></b></span></div><div style="font-size:12.8px"><span style="font-size:12.8px;white-space:pre-wrap"> </span><span style="font-size:12.8px">devcap |= PCI_EXP_DEVCAP_RBER;</span><span style="font-size:12.8px"><br></span></div><div style="font-size:12.8px"><span class="m_-5624364039645036717gmail-m_-3881861314826159405gmail-m_4371082674194637721gmail-m_6920781638293347396gmail-m_4420435145451042919gmail-Apple-tab-span" style="white-space:pre-wrap"> </span>pci_write_config32(endp, endp_cap + PCI_EXP_DEVCAP, devcap);</div></span><span class=""><div style="font-size:12.8px"><span class="m_-5624364039645036717gmail-m_-3881861314826159405gmail-m_4371082674194637721gmail-m_6920781638293347396gmail-m_4420435145451042919gmail-Apple-tab-span" style="font-size:12.8px;white-space:pre-wrap"> </span><span style="font-size:12.8px"><b><i><u><font color="#ff0000">devcap = pci_read_config32(endp, endp_cap + PCI_EXP_DEVCAP);</font></u></i></b></span><br></div></span><div style="font-size:12.8px"><div style="font-size:12.8px"><span class="m_-5624364039645036717gmail-m_-3881861314826159405gmail-m_4371082674194637721gmail-m_6920781638293347396gmail-m_4420435145451042919gmail-Apple-tab-span" style="white-space:pre-wrap"> <b><i><u><font color="#ff0000">if (</font></u></i></b></span><b><i><u><font color="#ff0000">devcap & PCI_EXP_DEVCAP_RBER) {</font></u></i></b></div><div style="font-size:12.8px"><span style="font-size:12.8px;white-space:pre-wrap"> </span><span style="font-size:12.8px;white-space:pre-wrap"> </span><b style="font-size:12.8px"><i><u><font color="#ff0000">printk("<span style="font-size:12.8px">Role-based Error Reporting capability in the DEVCAP (offset 0x04) is now ENABLED\n");</span></font></u></i></b></div><div style="font-size:12.8px"><span style="font-size:12.8px;white-space:pre-wrap"> <b><i><u><font color="#ff0000">else</font></u></i></b></span><b style="font-size:12.8px"><i><u><font color="#ff0000"><span style="font-size:12.8px"><br></span></font></u></i></b></div><div style="font-size:12.8px"><span style="font-size:12.8px;white-space:pre-wrap"> </span><span style="font-size:12.8px;white-space:pre-wrap"> </span><b style="font-size:12.8px"><i><u><font color="#ff0000">printk("<span style="font-size:12.8px">Role-based Error Reporting capability in the DEVCAP (offset 0x04) stayed DISABLED\n");</span></font></u></i></b></div><span class="m_-5624364039645036717gmail-im" style="font-size:12.8px"><div style="font-size:12.8px"><span style="font-size:12.8px;white-space:pre-wrap"> <b><i><u><font color="#ff0000">}</font></u></i></b></span></div><div style="font-size:12.8px"><br></div><div style="font-size:12.8px"><span style="font-size:12.8px;white-space:pre-wrap"><font color="#000000">This will actually show (solve this mystery) it all! :-)</font></span></div><div style="font-size:12.8px"><span style="font-size:12.8px;white-space:pre-wrap"><font color="#000000"><br></font></span></div><div style="font-size:12.8px"><span style="font-size:12.8px;white-space:pre-wrap"><font color="#000000">Zoran</font></span></div><div style="font-size:12.8px"><span style="font-size:12.8px;white-space:pre-wrap"><font color="#000000">_______</font></span></div></span></div></span></div></div><div class="HOEnZb"><div class="h5"><div class="gmail_extra"><br><div class="gmail_quote">On Thu, May 4, 2017 at 11:26 AM, Zoran Stojsavljevic <span dir="ltr"><<a href="mailto:zoran.stojsavljevic@gmail.com" target="_blank">zoran.stojsavljevic@gmail.com</a><wbr>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><span><div>> <span style="font-size:12.8px">Is that an error ?</span></div><div><br></div></span>I have looked into the document (BTW did some additional investigation) and tried to find the attributes explanation (found it on page 586):<div><br></div><div><img src="cid:ii_15bd29a69154a4e5" alt="Inline image 1" style="margin-right:0px"><br></div><div><br></div><div>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 <span style="font-size:12.8px">(</span><span style="font-size:12.8px">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.</span></div><div><span style="font-size:12.8px"><br></span></div><div><span style="font-size:12.8px">I would strongly advise you to add one printk() here and recompile Coreboot, something like this:</span></div><div><span><div style="font-size:12.8px"><span style="font-size:12.8px;white-space:pre-wrap"> </span>/* Enable ASPM role based error reporting. */</div><div style="font-size:12.8px"><span class="m_-5624364039645036717m_-3881861314826159405gmail-m_4371082674194637721gmail-m_6920781638293347396gmail-m_4420435145451042919gmail-Apple-tab-span" style="white-space:pre-wrap"> </span>devcap = pci_read_config32(endp, endp_cap + PCI_EXP_DEVCAP);</div></span><div style="font-size:12.8px"><span style="font-size:12.8px;white-space:pre-wrap"> // </span><span style="color:rgb(120,120,120);font-family:monaco,"courier new",courier,monospace;font-size:0.95em;font-weight:bold">#define </span><a href="http://lxr.free-electrons.com/ident?i=PCI_EXP_DEVCAP_RBER" style="font-family:monaco,"courier new",courier,monospace;font-size:0.95em;font-weight:bold;text-decoration-line:none;border-bottom:1px dotted rgb(153,153,153);color:black" target="_blank">PCI_EXP_DEVCAP_RBER</a><span style="color:rgb(120,120,120);font-family:monaco,"courier new",courier,monospace;font-size:0.95em;font-weight:bold"> 0x00008000 </span><b style="color:rgb(120,120,120);font-family:monaco,"courier new",courier,monospace;font-size:0.95em"><i>/* Role-Based Error Reporting */</i></b><br></div><div style="font-size:12.8px"><span class="m_-5624364039645036717m_-3881861314826159405gmail-m_4371082674194637721gmail-m_6920781638293347396gmail-m_4420435145451042919gmail-Apple-tab-span" style="white-space:pre-wrap"> <b><i><u><font color="#ff0000">if (</font></u></i></b></span><b><i><u><font color="#ff0000">devcap & PCI_EXP_DEVCAP_RBER) printk("<span style="font-size:12.8px">Role-based Error Reporting capability in the DEVCAP (offset 0x04) is ENABLED\n");</span></font></u></i></b></div><span><div style="font-size:12.8px"><span style="font-size:12.8px;white-space:pre-wrap"> </span><span style="font-size:12.8px">devcap |= PCI_EXP_DEVCAP_RBER;</span><span style="font-size:12.8px"><br></span></div><div style="font-size:12.8px"><span class="m_-5624364039645036717m_-3881861314826159405gmail-m_4371082674194637721gmail-m_6920781638293347396gmail-m_4420435145451042919gmail-Apple-tab-span" style="white-space:pre-wrap"> </span>pci_write_config32(endp, endp_cap + PCI_EXP_DEVCAP, devcap);</div></span></div><div style="font-size:12.8px"><br></div><div style="font-size:12.8px">Then, the Truth will reveal itself.</div><div><span style="font-size:12.8px"><br></span></div><div><span style="font-size:12.8px">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 </span><span style="color:rgb(0,0,0);white-space:pre-wrap">Duncan Laurie <</span><a href="http://www.coreboot.org/mailman/listinfo/coreboot" style="white-space:pre-wrap" target="_blank">dlaurie at google.com</a><span style="color:rgb(0,0,0);white-space:pre-wrap">> in the following patch: </span><font color="#000000"><span style="white-space:pre-wrap"><a href="https://mail.coreboot.org/pipermail/coreboot/2012-March/068690.html" target="_blank">https://mail.coreboot.org/pipe<wbr>rmail/coreboot/2012-March/0686<wbr>90.html</a></span></font></div><div><font color="#000000"><span style="white-space:pre-wrap"><br></span></font></div><div><font color="#000000"><span style="white-space:pre-wrap">I'll include also involved (last web pointer is certainly a smoking gun, isn't it?) in this matter.</span></font></div><div><font color="#000000"><span style="white-space:pre-wrap"><br></span></font></div><div><font color="#000000"><span style="white-space:pre-wrap">Zoran</span></font></div><div><font color="#000000"><span style="white-space:pre-wrap">_______</span></font></div><div class="gmail_extra"><br><div class="gmail_quote"><div><div class="m_-5624364039645036717h5">On Wed, May 3, 2017 at 11:54 PM, Youness Alaoui <span dir="ltr"><<a href="mailto:kakaroto@kakaroto.homelinux.net" target="_blank">kakaroto@kakaroto.homelinux.n<wbr>et</a>></span> wrote:<br></div></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div><div class="m_-5624364039645036717h5"><div dir="ltr">Hi,<div><br></div><div>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 :</div><div><div><span class="m_-5624364039645036717m_-3881861314826159405gmail-m_4371082674194637721m_8945027580687948962gmail-Apple-tab-span" style="white-space:pre-wrap"> </span>/* Enable ASPM role based error reporting. */</div><div><span class="m_-5624364039645036717m_-3881861314826159405gmail-m_4371082674194637721m_8945027580687948962gmail-Apple-tab-span" style="white-space:pre-wrap"> </span>devcap = pci_read_config32(endp, endp_cap + PCI_EXP_DEVCAP);</div><div><span class="m_-5624364039645036717m_-3881861314826159405gmail-m_4371082674194637721m_8945027580687948962gmail-Apple-tab-span" style="white-space:pre-wrap"> </span>devcap |= PCI_EXP_DEVCAP_RBER;</div><div><span class="m_-5624364039645036717m_-3881861314826159405gmail-m_4371082674194637721m_8945027580687948962gmail-Apple-tab-span" style="white-space:pre-wrap"> </span>pci_write_config32(endp, endp_cap + PCI_EXP_DEVCAP, devcap);</div></div><div><br></div><div>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 : <a href="http://composter.com.ua/documents/PCI_Express_Base_Specification_Revision_3.0.pdf" target="_blank">http://composter.com.ua/doc<wbr>uments/PCI_Express_Base_Specif<wbr>ication_Revision_3.0.pdf</a>), that structure is read-only (it's the device capabilities register, not the device control register).</div><div>Is that an error ? </div><div>I tracked that code (moved, then moved again) all the way to this change from 2011 : <a href="https://review.coreboot.org/#/c/735/" target="_blank">https://review.coreboot.org/#/<wbr>c/735/</a> but for some reason git blame can't find the file in commits before that.<br></div><div><br></div><div>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).</div><div><br></div><div>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 ?</div><div><br></div><div>Thanks,</div><div>Youness.</div><div><br></div></div>
<br></div></div><span class="m_-5624364039645036717HOEnZb"><font color="#888888">--<br>
coreboot mailing list: <a href="mailto:coreboot@coreboot.org" target="_blank">coreboot@coreboot.org</a><br>
<a href="https://mail.coreboot.org/mailman/listinfo/coreboot" rel="noreferrer" target="_blank">https://mail.coreboot.org/mail<wbr>man/listinfo/coreboot</a><br></font></span></blockquote></div><br></div></div>
</blockquote></div><br></div>
</div></div><br>--<br>
coreboot mailing list: <a href="mailto:coreboot@coreboot.org">coreboot@coreboot.org</a><br>
<a href="https://mail.coreboot.org/mailman/listinfo/coreboot" rel="noreferrer" target="_blank">https://mail.coreboot.org/<wbr>mailman/listinfo/coreboot</a><br></blockquote></div><br></div>