Am 18.09.2011 00:52 schrieb Stefan Tauner: > On Sat, 17 Sep 2011 23:52:39 +0200 > Carl-Daniel Hailfinger <[email protected]> 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 -- http://www.hailfinger.org/ _______________________________________________ flashrom mailing list [email protected] http://www.flashrom.org/mailman/listinfo/flashrom
