Am 18.09.2011 00:52 schrieb Stefan Tauner:
On Sat, 17 Sep 2011 23:52:39 +0200 Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net wrote:
As a followup to the layout usage scenarios I'd like to suggest a structure for hardware restrictions.
struct chipaccess { unsigned int start; unsigned int len; unsigned int perms; };
struct chipaccess chipaccess_flash[32]; struct chipaccess chipaccess_controller[32]; struct chipaccess chipaccess[32]; //merged
is using a list here an option? i.e. adding a struct chipaccess *next to the struct?
Sure, that works as well. We should try to reuse the list management macros from the Linux kernel in case we really use lists.
start and len should be self-explanatory. perms has two bits: WRITE_ALLOWED and READ_ALLOWED (preferably with a better name).
_EN or _ENABLE?
Good idea.
i'd prefer bit-fields (or two distinct variables) to store the perms actually.
Good point. Given that even MSVC supports bit fields, that should work out fine for all supported (and maybe-supported-in-the-future) compilers.
why combine them in one variable? an optional char* name would be nice, this could be used for layout file writing later and UI.
Not really. The array (or list) should only represent permissions, not layouts.
and of course "unsigned int" is not specific enough.
Right, we want uint32_t.
The chipaccess* arrays MUST (in the RFC 2119 sense of the word) cover the whole flash chip, holes are not allowed.
why? just define a default (probably RW) and be happy (or at least do not have problems like you outlined below :)
Dealing with non-adjacent regions means we have complicated code because we need to handle a special case: array/list does not mention the region we're interested in. You do have a point about the unknown size challenge, though. We could allow holes, but then we have to make the default explicit, and that's not easy because ICH SPI defaults to non-accessible and other controllers default to accessible.
This presents a unique challenge for controller restrictions because those restrictions are detected before the flash chip is known. As such, it is not obvious how to store controller restrictions. One way would be to set chip size as maximum chip size supported by the controller and store the alignment direction (i.e. upper alignment in x86). The chipaccess array for the flash chip itself (chipaccess_flash) would be separate from the chipaccess array for the controller (chipaccess_controller) and after detection of the flash chip both would be merged into chipaccess, taking alignment direction into account.
what about overlapping regions? i guess you want to merge them. i dont thank that is a good idea. they should be addressable separately.
Merge is optional. You can always merge chipaccess arrays/lists as a final step.
my ich-based laptop uses normal regions and one PR: 0x54: 0x00000000 (FREG0: Flash Descriptor) 0x00000000-0x00000fff is read-only 0x58: 0x07ff0500 (FREG1: BIOS) 0x00500000-0x007fffff is read-write 0x5C: 0x04ff0003 (FREG2: Management Engine) 0x00003000-0x004fffff is locked 0x60: 0x00020001 (FREG3: Gigabit Ethernet) 0x00001000-0x00002fff is read-write 0x64: 0x00000fff (FREG4: Platform Data) Platform Data region is unused. 0x74: 0x9fff07d0 (PR0) 0x007d0000-0x01ffffff is read-only
PR0 seems to be some kind of boot block protection or so... it starts in the middle of the bios region and spans till the end of the universe^Hflash chip. i would still like to be able to read the whole bios region, but without the rest of the chip (in the case the bios region would be r/o too).
Well, that shouldn't be a problem with the scheme I described. If anything, using defaults will be a problem because FREG defaults to non-accessible and PR defaults to accessible.
Regards, Carl-Daniel