coreboot May 2017

coreboot@coreboot.org

70 participants
202 discussions

Re: [coreboot] kernel payload
by Healer64 05 May '17

05 May '17

An auron yuna chromebook. Sent with [ProtonMail](https://protonmail.com) Secure Email. -------- Original Message -------- Subject: Re: [coreboot] kernel payload Local Time: May 5, 2017 11:18 AM UTC Time: May 5, 2017 5:18 PM From: rminnich(a)gmail.com To: Healer64 <Healer64(a)protonmail.com>, coreboot(a)coreboot.org <coreboot(a)coreboot.org> First off, what board is this on? On Fri, May 5, 2017 at 10:12 AM Healer64 via coreboot <coreboot(a)coreboot.org> wrote: I've looked through all the docs I can find on this, but there doesn't seem to be much available. Hopefully some of you can help answer my questions. My distribution includes a vmlinuz kernel. Do I need to compile a custom bzimage kernel or will the stock kernel suffice? I am using lvm on a dmcrypted disk for the root filesystem. Presumably these binaries and helper scripts will be needed in the initrd. So far the only help I've seen on creating the initrd for the payload is Ron's email to this list where he says "Your best bet is to do the standard trivial initramfs with a simple program: main(){printf("hi\n"); while (1);} Let's pretend it's called mymain.c cc -o mymain -static mymain.c put that in your prototype root file system with the name init. make sure you create /dev/console. Then bundle it all up with cpio and you've got an initramfs" That seems to describe more of a shim than a true initrd. Also he mentions initramfs whereas menuconfig asks for initrd. Which is correct? I don't see any reference to filesystem drivers in Ron's email, but surely they are required to mount the root. I'm guessing this is a test example that is not intended to boot. Can anyone provide help or instructions on an actual bootable initrd/initramfs? Can a pointer to the initramfs in /boot be used? What kind of size will it be? My rom is limited to 8Mb and the kernel is over 5Mb. Sent with [ProtonMail](https://protonmail.com) Secure Email. -- coreboot mailing list: coreboot(a)coreboot.org https://mail.coreboot.org/mailman/listinfo/coreboot

1 0

Re: [coreboot] kernel payload
by ron minnich 05 May '17

05 May '17

OK, so you told me off-list it's an auron. Did you try the full build/boot cycle with an unmodified coreboot first?

1 0

Re: [coreboot] kernel payload
by ron minnich 05 May '17

05 May '17

First off, what board is this on? On Fri, May 5, 2017 at 10:12 AM Healer64 via coreboot <coreboot(a)coreboot.org> wrote: > I've looked through all the docs I can find on this, but there doesn't > seem to be much available. Hopefully some of you can help answer my > questions. My distribution includes a vmlinuz kernel. Do I need to compile > a custom bzimage kernel or will the stock kernel suffice? > > I am using lvm on a dmcrypted disk for the root filesystem. Presumably > these binaries and helper scripts will be needed in the initrd. So far the > only help I've seen on creating the initrd for the payload is Ron's email > to this list where he says > > "Your best bet is to do the standard trivial initramfs with a simple > program: main(){printf("hi\n"); while (1);} Let's pretend it's called > mymain.c cc -o mymain -static mymain.c put that in your prototype root file > system with the name init. make sure you create /dev/console. Then bundle > it all up with cpio and you've got an initramfs" > > That seems to describe more of a shim than a true initrd. Also he mentions > initramfs whereas menuconfig asks for initrd. Which is correct? I don't see > any reference to filesystem drivers in Ron's email, but surely they are > required to mount the root. I'm guessing this is a test example that is not > intended to boot. > > Can anyone provide help or instructions on an actual bootable > initrd/initramfs? Can a pointer to the initramfs in /boot be used? What > kind of size will it be? My rom is limited to 8Mb and the kernel is over > 5Mb. > > > Sent with ProtonMail <https://protonmail.com> Secure Email. > > -- > coreboot mailing list: coreboot(a)coreboot.org > https://mail.coreboot.org/mailman/listinfo/coreboot

1 0

kernel payload
by Healer64 05 May '17

05 May '17

I've looked through all the docs I can find on this, but there doesn't seem to be much available. Hopefully some of you can help answer my questions. My distribution includes a vmlinuz kernel. Do I need to compile a custom bzimage kernel or will the stock kernel suffice? I am using lvm on a dmcrypted disk for the root filesystem. Presumably these binaries and helper scripts will be needed in the initrd. So far the only help I've seen on creating the initrd for the payload is Ron's email to this list where he says "Your best bet is to do the standard trivial initramfs with a simple program: main(){printf("hi\n"); while (1);} Let's pretend it's called mymain.c cc -o mymain -static mymain.c put that in your prototype root file system with the name init. make sure you create /dev/console. Then bundle it all up with cpio and you've got an initramfs" That seems to describe more of a shim than a true initrd. Also he mentions initramfs whereas menuconfig asks for initrd. Which is correct? I don't see any reference to filesystem drivers in Ron's email, but surely they are required to mount the root. I'm guessing this is a test example that is not intended to boot. Can anyone provide help or instructions on an actual bootable initrd/initramfs? Can a pointer to the initramfs in /boot be used? What kind of size will it be? My rom is limited to 8Mb and the kernel is over 5Mb. Sent with [ProtonMail](https://protonmail.com) Secure Email.

1 0

New Defects reported by Coverity Scan for coreboot
by scan-admin＠coverity.com 05 May '17

05 May '17

Hi, Please find the latest report on new defect(s) introduced to coreboot found with Coverity Scan. 1 new defect(s) introduced to coreboot found with Coverity Scan. 1 defect(s), reported by Coverity Scan earlier, were marked fixed in the recent build analyzed by Coverity Scan. New defect(s) Reported-by: Coverity Scan Showing 1 of 1 defect(s) ** CID 1374795: Memory - corruptions (STRING_OVERFLOW) /src/lib/edid.c: 473 in detailed_block() ________________________________________________________________________________________________________ *** CID 1374795: Memory - corruptions (STRING_OVERFLOW) /src/lib/edid.c: 473 in detailed_block() 467 case 0xFE: 468 /* 469 * TODO: Two of these in a row, in the third and fourth 470 * slots, seems to be specified by SPWG: 471 * https://u2389337.ct.sendgrid.net/wf/click?upn=eMT-2BqXhL85kf4kjMp0VpWCuCeHL… 472 */ >>> CID 1374795: Memory - corruptions (STRING_OVERFLOW) >>> You might overrun the 14-character destination string "result_edid->ascii_string" by writing 128 characters from "extract_string(x + 5, &c->has_valid_string_termination, 13)". 473 strcpy(result_edid->ascii_string, extract_string(x + 5, 474 &c->has_valid_string_termination, 475 EDID_ASCII_STRING_LENGTH)); 476 printk(BIOS_SPEW, "ASCII string: %s\n", 477 result_edid->ascii_string); 478 return 1; ________________________________________________________________________________________________________ To view the defects in Coverity Scan visit, https://u2389337.ct.sendgrid.net/wf/click?upn=08onrYu34A-2BWcWUl-2F-2BfV0V0… To manage Coverity Scan email notifications for "coreboot(a)coreboot.org", click https://u2389337.ct.sendgrid.net/wf/click?upn=08onrYu34A-2BWcWUl-2F-2BfV0V0…

1 0

Re: [coreboot] PCIe Role-based error reporting
by Zoran Stojsavljevic 05 May '17

05 May '17

> 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). I knew that you'll do both use cases (if Role-based Error Reporting capability (bit 15) is by default 0 or 1), especially in the case you discovered that Capability Register is RO. Conclusion: INTEL PCIe implementation for this register follows the PCIe 3.0 specification revision/recommendation. Problem solved. No need to involve anybody else. :-) The only unsolved (aside) issue remains to be understood: *"I still fail to see the link between RBER and ASPM".* I need to think about this more... Later. Zoran On Thu, May 4, 2017 at 11:16 PM, Youness Alaoui < kakaroto(a)kakaroto.homelinux.net> wrote: > Hi Zoran, > > 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). > > 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. > > 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 https://review.coreboot. > org/#/c/735), 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". > > Thanks, > > On Thu, May 4, 2017 at 8:42 AM, Zoran Stojsavljevic < > zoran.stojsavljevic(a)gmail.com> wrote: > >> I thought more about this problem, and how it is easy to miss the >> complete point?! :-( >> >> Revisited C code to be added (in *RED*): >> /* 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 initially ENABLED\n");* >> *else* >> *printk("Role-based Error Reporting capability in the DEVCAP (offset >> 0x04) is initially DISABLED\n");* >> *}* >> devcap |= PCI_EXP_DEVCAP_RBER; >> pci_write_config32(endp, endp_cap + PCI_EXP_DEVCAP, devcap); >> *devcap = pci_read_config32(endp, endp_cap + PCI_EXP_DEVCAP);* >> *if (**devcap & PCI_EXP_DEVCAP_RBER) {* >> *printk("Role-based Error Reporting capability in the DEVCAP (offset >> 0x04) is now ENABLED\n");* >> *else* >> *printk("Role-based Error Reporting capability in the DEVCAP (offset >> 0x04) stayed DISABLED\n");* >> *}* >> >> This will actually show (solve this mystery) it all! :-) >> >> Zoran >> _______ >> >> On Thu, May 4, 2017 at 11:26 AM, Zoran Stojsavljevic < >> zoran.stojsavljevic(a)gmail.com> wrote: >> >>> > 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/0686 >>> 90.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(a)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_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. >>>> >>>> >>>> -- >>>> coreboot mailing list: coreboot(a)coreboot.org >>>> https://mail.coreboot.org/mailman/listinfo/coreboot >>>> >>> >>> >> >> -- >> coreboot mailing list: coreboot(a)coreboot.org >> https://mail.coreboot.org/mailman/listinfo/coreboot >> > >

1 0

Intel PCH: When is the code for SPI flash reading used?
by Marek Behun 05 May '17

05 May '17

Hello, is the code for SPI flash reading (from src/drivers/spi/spi_flash.c) used at all? From what I understood when reading Intel PCH datasheets, the SPI controller alone reads the whole BIOS. Is this wrong? Does the SPI controller read only the bootblock (on ThinkPad X230, for example), and the bootblock then reads the rest? Thank you. Marek

2 1

Re: [coreboot] PCIe Role-based error reporting
by Youness Alaoui 04 May '17

04 May '17

Hi Zoran, 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). 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. 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 https://review.coreboot.org/#/c/735), 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". Thanks, On Thu, May 4, 2017 at 8:42 AM, Zoran Stojsavljevic < zoran.stojsavljevic(a)gmail.com> wrote: > I thought more about this problem, and how it is easy to miss the complete > point?! :-( > > Revisited C code to be added (in *RED*): > /* 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 initially ENABLED\n");* > *else* > *printk("Role-based Error Reporting capability in the DEVCAP (offset 0x04) > is initially DISABLED\n");* > *}* > devcap |= PCI_EXP_DEVCAP_RBER; > pci_write_config32(endp, endp_cap + PCI_EXP_DEVCAP, devcap); > *devcap = pci_read_config32(endp, endp_cap + PCI_EXP_DEVCAP);* > *if (**devcap & PCI_EXP_DEVCAP_RBER) {* > *printk("Role-based Error Reporting capability in the DEVCAP (offset 0x04) > is now ENABLED\n");* > *else* > *printk("Role-based Error Reporting capability in the DEVCAP (offset 0x04) > stayed DISABLED\n");* > *}* > > This will actually show (solve this mystery) it all! :-) > > Zoran > _______ > > On Thu, May 4, 2017 at 11:26 AM, Zoran Stojsavljevic < > zoran.stojsavljevic(a)gmail.com> wrote: > >> > 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/0686 >> 90.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(a)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_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. >>> >>> >>> -- >>> coreboot mailing list: coreboot(a)coreboot.org >>> https://mail.coreboot.org/mailman/listinfo/coreboot >>> >> >> > > -- > coreboot mailing list: coreboot(a)coreboot.org > https://mail.coreboot.org/mailman/listinfo/coreboot >

1 0

Re: [coreboot] PCIe Role-based error reporting
by Zoran Stojsavljevic 04 May '17

04 May '17

I thought more about this problem, and how it is easy to miss the complete point?! :-( Revisited C code to be added (in *RED*): /* 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 initially ENABLED\n");* *else* *printk("Role-based Error Reporting capability in the DEVCAP (offset 0x04) is initially DISABLED\n");* *}* devcap |= PCI_EXP_DEVCAP_RBER; pci_write_config32(endp, endp_cap + PCI_EXP_DEVCAP, devcap); *devcap = pci_read_config32(endp, endp_cap + PCI_EXP_DEVCAP);* *if (**devcap & PCI_EXP_DEVCAP_RBER) {* *printk("Role-based Error Reporting capability in the DEVCAP (offset 0x04) is now ENABLED\n");* *else* *printk("Role-based Error Reporting capability in the DEVCAP (offset 0x04) stayed DISABLED\n");* *}* This will actually show (solve this mystery) it all! :-) Zoran _______ On Thu, May 4, 2017 at 11:26 AM, Zoran Stojsavljevic < zoran.stojsavljevic(a)gmail.com> wrote: > > 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 < > kakaroto(a)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_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. >> >> >> -- >> coreboot mailing list: coreboot(a)coreboot.org >> https://mail.coreboot.org/mailman/listinfo/coreboot >> > >

1 0

Re: [coreboot] PCIe Role-based error reporting
by Zoran Stojsavljevic 04 May '17

04 May '17

> 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 < kakaroto(a)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_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. > > > -- > coreboot mailing list: coreboot(a)coreboot.org > https://mail.coreboot.org/mailman/listinfo/coreboot >

1 0

← Newer
1
...
12
13
14
15
16
17
18
...
21
Older →

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

2008

2007

2006

2005

2004

2003

2002

coreboot May 2017