View Change

1 comment:

• Patchset:

  • Patch Set #10:

    Earlier if your platform was entering into earlier if loop and touching bit 7:0 then it was doing wrong programming.

    I either don't understand that sentence correctly or it is the opposite, the else clause is what touches bit 7:0.

    Its exactly opposite what i said, SKL SPI programming guide says, Bit 7:0 are reserved and other IFDv2 chipset has extended ranges hence it might need to protect.

    if (ifd_version >= IFD_VERSION_2) {
    		/* Clear non-reserved bits */
    		fmba->flmstr1 &= 0xff;
    		fmba->flmstr2 &= 0xff;
    		fmba->flmstr3 &= 0xff;
    		fmba->flmstr5 &= 0xff;
    } else {
    		wr_shift = FLMSTR_WR_SHIFT_V1;
    		rd_shift = FLMSTR_RD_SHIFT_V1;

    		fmba->flmstr1 = 0;
    		fmba->flmstr2 = 0;
    		/* Requestor ID */
    		fmba->flmstr3 = 0x118;
    }

    
    If ifd_version is set to IFD_VERSION_2 like it was before this change, then it will not touch bits 7:0 and set read and write permissions to true. In the else-clause of the condition, which it uses now, both 7:0 reserved bits are changed and (I think this is what breaks boot) read permission bits 15:8 are set to 0.

    if you could download the SPI programming guide from this link
    https://www.intel.com/content/www/us/en/search.html?ws=text#q=550696&t=All
    and go to page 33, Flash Descriptor Master Section, you will see 7:0 are reserved and what I could sense is that, existing code itself had issue

    https://review.coreboot.org/c/coreboot/+/56070/1/util/ifdtool/ifdtool.c#1278 should be as below to protect bits 15:8 as well.
    fmba->flmstr1 = 0xffffff00;
    fmba->flmstr2 = 0xffffff00;

    
    > I would say, it was wrong that SKL was considered as IFDv2.
    Even if IFDv2 is supposed to mean that 7:0 of the flash master are used for Extended Region Access Permissions somewhere else, in ifdtool it just decides whether FLMSTR uses 31:16 or 31:8 for permissions.

    IMHO adding SKLKBL to the ifd_2_platforms list is the easiest solution to fix this issue. This is the change I applied to make it work for me: https://review.coreboot.org/c/coreboot/+/56070

    Adding SKL into IFDv2 would be an easy W/A as SKL flash descriptor master register offsets are very close to IFDv2 (except the last byte).

    Have you verified other fields as well with CB:56070 for skl platform?

To view, visit change 54305. To unsubscribe, or for help writing mail filters, visit settings.