Skip to content

Add instructions on write-protecting bootblock - #7

Merged
macpijan merged 1 commit into
wp_integrationfrom
wp-bootblock-doc
Sep 21, 2022
Merged

Add instructions on write-protecting bootblock#7
macpijan merged 1 commit into
wp_integrationfrom
wp-bootblock-doc

Conversation

@SergiiDmytruk

Copy link
Copy Markdown
Member

No description provided.

Comment thread Documentation/bootblock-protection.md Outdated
Comment thread Documentation/bootblock-protection.md Outdated
(`printf "%#x\n" $(( 0xff7100 + 36544 - 1 ))`)

Thus we need to write-protect the smallest area that covers the range from
`0xff7100` to `0xffffbf` (both bounds are inclusive).

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not correct, bootblock on x86 (except newest AMD boards that start from RAM) has to end at 0xffffffff. Offset from cbfstool print points to some kind of CBFS header that is not included in size reported by that same command. I haven't seen any other size of that metadata than 0x40, but I don't know if this is always true.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is another possible issue: at 0xfffffffc there is a pointer to CBFS master header which is used to find CBFS. IIRC there were plans to change it to depend on FMAP instead, so maybe this is no longer an issue, but this could be the reason why 4.13 bootblock doesn't work with 4.17 rest of coreboot.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that offset looked weird, but I forgot to check it. Doesn't look like the size is fixed ("offset" here is a field of component header):

/** This is a component header - every entry in the CBFS
    will have this header.

    This is how the component is arranged in the ROM:

    --------------   <- 0
    component header
    --------------   <- sizeof(struct component)
    component name
    --------------   <- offset
    data
    ...
    --------------   <- offset + len
*/

It's just that CBFS_ALIGNMENT is 64 which is why is most cases data will be offset by 64 bytes.


Looks like master CBFS header offset is still there on latest master (src/arch/x86/bootblock.ld) for the case of bootblock in CBFS:

	. = 0xfffffff0;
	_X86_RESET_VECTOR = .;
	.reset . : {
		*(.reset);
		. = _X86_RESET_VECTOR_FILLING;
		BYTE(0);
	}
	. = 0xfffffffc;
	.header_pointer . : {
		KEEP(*(.header_pointer));
	}
	_X86_RESET_VECTOR_FILLING = 15 - SIZEOF(.header_pointer);
	_ebootblock = .;

So it won't work if CBFS size changes. Size of image in 4.17 is 4 MiB vs. 255 KiB for 4.13, I didn't expect bootblock to contain that CBFS offset hard-coded and didn't try to make the size equal.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regarding master header offset, I know it is still there but I haven't seen it being used in current code. Then again, I didn't dig too deep.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even when ROM sizes match, offsets between bootblocks in 4.13 and 4.17 don't (12 byte difference).

Change-Id: Iaad1014fad5a60e78bef55c5f3aceaed782b66d6
Signed-off-by: Sergii Dmytruk <sergii.dmytruk@3mdeb.com>

@macpijan macpijan left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I went through this and it makes sense to me.

Based on that, I performed some testing on the x230 SPIs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants