Hi,
before we change flashrom to work with current layout requirements, we should summarize the features we need/want, and then decide how to handle the individual layout regions internally.
Terms: "write strategy" describes a combination of erase+write commands with a given block size, touching some blocks. "read protected" describes a region which can not be read. "write protected" describes a region which can not be written. "write once" describes a region which can be written exactly once in the chip lifetime.
If you add a feature to the list, please give it a nickname so we know which feature people are referring to. Some of the features below might not be desirable, but I want to list them anyway so we can explicitly declare them as unsupported if needed.
fullwrite-unrestricted: Write a chip-sized image, no special read/write restrictions of the chip, no layout file needs to be specified. Default write case right now.
fullwrite-noread: Write a chip-sized image, reading anything from the chip is not possible. Many DVD writers fall in that category. No verification, violates our reliability guarantees.
fullwrite-postread: Write a chip-sized image, reading anything from the chip is only possible after write. Chips which are read-locked until a full erase/write fall in that category. Do those exist as standalone flash chips or only integrated into processors?
fullwrite-partialread: Write a chip-sized image, reading is only possible in some chip regions. Only partial verification, violates our reliability guarantees.
fullwrite-partialwrite: Write a chip-sized image, but writing is only possible in some regions. This is obviously a conflict unless the image has the same contents as the chip in the write-protected regions and there is a possible write strategy for the whole image which does not touch the write-protected regions. Should flashrom always refuse this scenario, or only refuse it in case of conflicts?
partialwrite-unrestricted: Write only parts of an image, the rest of the chip contents is kept, no special read/write restrictions.
partialwrite-partialread: Write only parts of an image, the rest of the chip contents is kept, reading is only possible in some chip regions. If no read-protected regions are written and a suitable write strategy exists, should flashrom warn? If a read-protected region is written, should flashrom warn/refuse due to reliability requirements?
partialwrite-partialwrite: Write only parts of an image, the rest of the chip contents is kept, writing is only possible in some chip regions. If no write-protected regions are written and a suitable write strategy exists, should flashrom warn? flashrom will refuse to write a write-protected region.
fullread-unrestricted: Read the full chip, no special read restrictions of the chip.
partialread-unrestricted: Read only parts of a chip, no special read restrictions of the chip.
partialread-partialread: Read only parts of a chip, some regions are read-protected. flashrom should refuse to read any read-protected regions.
partialread-imagefiller: If only parts of a chip are read and the read image has full chip size, what should be used as filler for unread regions? 0x00 or 0xff?
partialread-layout-imagesize: If only parts of a chip are read, should the read image still have full chip size with all read regions filled in at the respective locations?
partialread-layout-split: If only parts of a chip are read, should it be possible to write each region (or a combination thereof) to a separate image file, and would that mapping be specified in the layout file?
partialwrite-layout-split: If only parts of a chip are written, should it be possible to collect each part of the new image from a separate image file, and would that mapping be specified in the layout file?
readwrite-protection-time: Which read protection and write protection times exist? Temporary lock until unlock, temporary lock until chip soft reset, temporary lock until chip/programmer hard reset (powerdown or reset line), permanent eternal lock.
readwrite-protection-type: Which read protection and write protection types exist? Programmer lock (e.g. Intel SPI), hardware chip lock (WP pin), software chip lock (chip command).
readwrite-protection-interaction: How should we express this situation: A region is write-locked with a software chip lock, but to remove that software chip lock, a hardware chip lock has to be disabled first, then the software chip lock can be disabled.
partialaccess-crash: Some regions in the chip are really off-limits and will cause an unrecoverable error (system crash) when accessed (read or write). That seems to be the case for some EC/flash interactions.
Comments? Any forks of flashrom (e.g. chromium) which need infrastructure features not mentioned above?
Regards, Carl-Daniel
On Tr, 2011-08-17 at 02:00 +0200, Carl-Daniel Hailfinger wrote:
Hi,
before we change flashrom to work with current layout requirements, we should summarize the features we need/want, and then decide how to handle the individual layout regions internally.
Terms: "write strategy" describes a combination of erase+write commands with a given block size, touching some blocks. "read protected" describes a region which can not be read. "write protected" describes a region which can not be written. "write once" describes a region which can be written exactly once in the chip lifetime.
If you add a feature to the list, please give it a nickname so we know which feature people are referring to. Some of the features below might not be desirable, but I want to list them anyway so we can explicitly declare them as unsupported if needed.
fullwrite-unrestricted: Write a chip-sized image, no special read/write restrictions of the chip, no layout file needs to be specified. Default write case right now.
fullwrite-noread: Write a chip-sized image, reading anything from the chip is not possible. Many DVD writers fall in that category. No verification, violates our reliability guarantees.
fullwrite-postread: Write a chip-sized image, reading anything from the chip is only possible after write. Chips which are read-locked until a full erase/write fall in that category. Do those exist as standalone flash chips or only integrated into processors?
fullwrite-partialread: Write a chip-sized image, reading is only possible in some chip regions. Only partial verification, violates our reliability guarantees.
fullwrite-partialwrite: Write a chip-sized image, but writing is only possible in some regions. This is obviously a conflict unless the image has the same contents as the chip in the write-protected regions and there is a possible write strategy for the whole image which does not touch the write-protected regions. Should flashrom always refuse this scenario, or only refuse it in case of conflicts?
partialwrite-unrestricted: Write only parts of an image, the rest of the chip contents is kept, no special read/write restrictions.
partialwrite-partialread: Write only parts of an image, the rest of the chip contents is kept, reading is only possible in some chip regions. If no read-protected regions are written and a suitable write strategy exists, should flashrom warn? If a read-protected region is written, should flashrom warn/refuse due to reliability requirements?
partialwrite-partialwrite: Write only parts of an image, the rest of the chip contents is kept, writing is only possible in some chip regions. If no write-protected regions are written and a suitable write strategy exists, should flashrom warn? flashrom will refuse to write a write-protected region.
fullread-unrestricted: Read the full chip, no special read restrictions of the chip.
partialread-unrestricted: Read only parts of a chip, no special read restrictions of the chip.
partialread-partialread: Read only parts of a chip, some regions are read-protected. flashrom should refuse to read any read-protected regions.
partialread-imagefiller: If only parts of a chip are read and the read image has full chip size, what should be used as filler for unread regions? 0x00 or 0xff?
partialread-layout-imagesize: If only parts of a chip are read, should the read image still have full chip size with all read regions filled in at the respective locations?
partialread-layout-split: If only parts of a chip are read, should it be possible to write each region (or a combination thereof) to a separate image file, and would that mapping be specified in the layout file?
partialwrite-layout-split: If only parts of a chip are written, should it be possible to collect each part of the new image from a separate image file, and would that mapping be specified in the layout file?
readwrite-protection-time: Which read protection and write protection times exist? Temporary lock until unlock, temporary lock until chip soft reset, temporary lock until chip/programmer hard reset (powerdown or reset line), permanent eternal lock.
readwrite-protection-type: Which read protection and write protection types exist? Programmer lock (e.g. Intel SPI), hardware chip lock (WP pin), software chip lock (chip command).
readwrite-protection-interaction: How should we express this situation: A region is write-locked with a software chip lock, but to remove that software chip lock, a hardware chip lock has to be disabled first, then the software chip lock can be disabled.
partialaccess-crash: Some regions in the chip are really off-limits and will cause an unrecoverable error (system crash) when accessed (read or write). That seems to be the case for some EC/flash interactions.
Comments? Any forks of flashrom (e.g. chromium) which need infrastructure features not mentioned above?
Regards, Carl-Daniel
Hi, wow, that's a big list you generated :)
Now, I think that any partial read/write must supply the image with only it's contents (no 0xff or similar fills) and a layoaut file similar to that we have now, except names point to binary files, and addresses are offsets, where to write that binary to chip. Or it could be one concatenated binary file.
Overlapping regions - never support it. It's not a flashrom's job to make a good image for a chip. So do one region at a time in one place.
Partial write: don't read full chip. If needs erase: user supplied image must contain all the eraseregion. Or the user must explicitly mark that it needs erase somehow. I believe you won't go with me here, but I'm open for discussion :)
Anyway, thanks for starting this thread - it's time to make it clear :)
Use UNIX philosophy: do one thing and do it well.
Thanks, Tadas
On Thu, Aug 18, 2011 at 4:41 AM, Tadas Slotkus devtadas@gmail.com wrote:
Overlapping regions - never support it. It's not a flashrom's job to make a good image for a chip. So do one region at a time in one place.
Agreed.
On Thu, Aug 18, 2011 at 4:41 AM, Tadas Slotkus devtadas@gmail.com wrote:
Partial write: don't read full chip. If needs erase: user supplied image must contain all the eraseregion. Or the user must explicitly mark that it needs erase somehow. I believe you won't go with me here, but I'm open for discussion :)
I'll have to disagree with you here. That makes the logic involved to utilize partial writes much more complicated. I think the best default action (and overwhelming usage case) for Flashrom is to handle erase regions automagically as it does now.
We'll need to improve the partial write logic a bit so it calculates all the necessary erase regions and checks to ensure they are all readable and erasable. Much of the work will be done to address issues mentioned earlier in Carl-Daniel's list.
On Thu, Aug 18, 2011 at 4:41 AM, Tadas Slotkus devtadas@gmail.com wrote:
Use UNIX philosophy: do one thing and do it well.
Amen.
On Tue, Aug 16, 2011 at 5:00 PM, Carl-Daniel Hailfinger < c-d.hailfinger.devel.2006@gmx.net> wrote:
fullwrite-noread: Write a chip-sized image, reading anything from the chip is not possible. Many DVD writers fall in that category. No verification, violates our reliability guarantees.
While this is an understandable constraint, it means we'll need to be more careful with how we handle partial write logic (no more reading old content, merging new content, and writing back).
On Tue, Aug 16, 2011 at 5:00 PM, Carl-Daniel Hailfinger < c-d.hailfinger.devel.2006@gmx.net> wrote:
fullwrite-partialwrite: Write a chip-sized image, but writing is only possible in some regions. This is obviously a conflict unless the image has the same contents as the chip in the write-protected regions and there is a possible write strategy for the whole image which does not touch the write-protected regions. Should flashrom always refuse this scenario, or only refuse it in case of conflicts?
Process include arguments first. User must specify regions which are fully writeable. Flashrom should check that they are beforehand and abort if any foreseeable problems arise so that the image on the chip is not left in an inconsistent state.
On Tue, Aug 16, 2011 at 5:00 PM, Carl-Daniel Hailfinger < c-d.hailfinger.devel.2006@gmx.net> wrote:
partialwrite-partialread: Write only parts of an image, the rest of the chip contents is kept, reading is only possible in some chip regions. If no read-protected regions are written and a suitable write strategy exists, should flashrom warn? If a read-protected region is written, should flashrom warn/refuse due to reliability requirements?
In the former case (no read-protected regions are written and suitable write strategy exists), do not warn. In the latter case (read-protected region is written, like with the DVD write example above), refuse and print a warning to use --force.
On Tue, Aug 16, 2011 at 5:00 PM, Carl-Daniel Hailfinger < c-d.hailfinger.devel.2006@gmx.net> wrote:
partialwrite-partialwrite: Write only parts of an image, the rest of the chip contents is kept, writing is only possible in some chip regions. If no write-protected regions are written and a suitable write strategy exists, should flashrom warn? flashrom will refuse to write a write-protected region.
No warning if no write-protected regions are used.
On Tue, Aug 16, 2011 at 5:00 PM, Carl-Daniel Hailfinger < c-d.hailfinger.devel.2006@gmx.net> wrote:
partialread-partialread: Read only parts of a chip, some regions are read-protected. flashrom should refuse to read any read-protected regions.
Flashrom should probably also print a warning so the user knows the output might not meet expectations.
On Tue, Aug 16, 2011 at 5:00 PM, Carl-Daniel Hailfinger < c-d.hailfinger.devel.2006@gmx.net> wrote:
partialread-imagefiller: If only parts of a chip are read and the read image has full chip size, what should be used as filler for unread regions? 0x00 or 0xff?
Whatever the ROM's default is. I presume 0xff is a safe bet for modern flash memory. I also suggest this to avoid unnecessary erase/write operations.
On Tue, Aug 16, 2011 at 5:00 PM, Carl-Daniel Hailfinger < c-d.hailfinger.devel.2006@gmx.net> wrote:
partialread-layout-imagesize: If only parts of a chip are read, should the read image still have full chip size with all read regions filled in at the respective locations?
partialread-layout-split: If only parts of a chip are read, should it be possible to write each region (or a combination thereof) to a separate image file, and would that mapping be specified in the layout file?
partialwrite-layout-split: If only parts of a chip are written, should it be possible to collect each part of the new image from a separate image file, and would that mapping be specified in the layout file?
Good question. We actually found a use for this scenario in Chromium OS, and the answer is that we do both now. The syntax we arrived at (Louis Lo added this) goes like this:
./flashrom -p internal:bus=spi -l layout.txt -i region_0:region_0.bin -i region_1:region_1.bin -r foo.bin
The result is three files: region_0.bin containing only region_0's content, region_1.bin containing only region_1's content, and foo.bin. foo.bin is the size of the chip, contains region_1 and region_2 content at their respective locations, and is filled with 0xff elsewhere.
This is useful in a few ways: - Speed (obviously) for selectively reading content. - The chip-sized image can be modified and written back to the chip easily using familiar partial write methods. - The chip-sized image is also good for debugging since no arithmetic is required to know what offset is being examined :-)
On Tue, Aug 16, 2011 at 5:00 PM, Carl-Daniel Hailfinger < c-d.hailfinger.devel.2006@gmx.net> wrote:
readwrite-protection-time: Which read protection and write protection times exist? Temporary lock until unlock, temporary lock until chip soft reset, temporary lock until chip/programmer hard reset (powerdown or reset line), permanent eternal lock.
readwrite-protection-type: Which read protection and write protection types exist? Programmer lock (e.g. Intel SPI), hardware chip lock (WP pin), software chip lock (chip command).
readwrite-protection-interaction: How should we express this situation: A region is write-locked with a software chip lock, but to remove that software chip lock, a hardware chip lock has to be disabled first, then the software chip lock can be disabled.
Just fail if it's not easy to defeat the write protection (and hopefully restore it). There are so many hardware write protection mechanisms out there that I don't think it's within Flashrom's scope to try and defeat them all.
On Wed, 17 Aug 2011 02:00:28 +0200 Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net wrote:
Hi,
before we change flashrom to work with current layout requirements, we should summarize the features we need/want, and then decide how to handle the individual layout regions internally.
Terms: "write strategy" describes a combination of erase+write commands with a given block size, touching some blocks. "read protected" describes a region which can not be read. "write protected" describes a region which can not be written. "write once" describes a region which can be written exactly once in the chip lifetime.
missing term: range (and derived terms, especially "write-protected range" because it relies on write protection borders to coincide with erase block borders(?))
If you add a feature to the list, please give it a nickname so we know which feature people are referring to. Some of the features below might not be desirable, but I want to list them anyway so we can explicitly declare them as unsupported if needed.
fullwrite-unrestricted: Write a chip-sized image, no special read/write restrictions of the chip, no layout file needs to be specified. Default write case right now.
fullwrite-noread: Write a chip-sized image, reading anything from the chip is not possible. Many DVD writers fall in that category. No verification, violates our reliability guarantees.
^--------------------^ are they specified somewhere explicitly? i guess everyone reading this is aware of the general read-erase-write-read pattern, but arguing about any feature with the aid of those without specifying exactly what it means is pointless.
fullwrite-postread: Write a chip-sized image, reading anything from the chip is only possible after write. Chips which are read-locked until a full erase/write fall in that category. Do those exist as standalone flash chips or only integrated into processors?
it would be enough if specific programmers with attached chips behave like that. there might be some FPGA boards that use an external SPI flash, but restrict access to it in that way. the nickname is a bit misleading imho (sounds like reading is only possible after write. fullwrite-nopostread might be an alternative).
fullwrite-partialread: Write a chip-sized image, reading is only possible in some chip regions. Only partial verification, violates our reliability guarantees.
does this exist in the wild? ICH could be configured this way theoretically.
fullwrite-partialwrite: Write a chip-sized image, but writing is only possible in some regions. This is obviously a conflict unless the image has the same contents as the chip in the write-protected regions and there is a possible write strategy for the whole image which does not touch the write-protected regions. Should flashrom always refuse this scenario, or only refuse it in case of conflicts?
what david said (i.e. refuse unless pointed by layout+image options to writable ranges only, so always refuse (in) this scenario).
partialwrite-unrestricted: Write only parts of an image, the rest of the chip contents is kept, no special read/write restrictions.
partialwrite-partialread: Write only parts of an image, the rest of the chip contents is kept, reading is only possible in some chip regions. If no read-protected regions are written and a suitable write strategy exists, should flashrom warn? If a read-protected region is written, should flashrom warn/refuse due to reliability requirements?
what david said.
partialwrite-partialwrite: Write only parts of an image, the rest of the chip contents is kept, writing is only possible in some chip regions. If no write-protected regions are written and a suitable write strategy exists, should flashrom warn? flashrom will refuse to write a write-protected region.
what david said.
fullread-unrestricted: Read the full chip, no special read restrictions of the chip.
partialread-unrestricted: Read only parts of a chip, no special read restrictions of the chip.
partialread-partialread: Read only parts of a chip, some regions are read-protected. flashrom should refuse to read any read-protected regions.
yes. bail out and warn, even if parts of the regions requested might be readable (to be consistent with *-partialwrite).
partialread-imagefiller: If only parts of a chip are read and the read image has full chip size, what should be used as filler for unread regions? 0x00 or 0xff?
publicly undefined (and use either in implementation). what are the use cases for those regions?
partialread-layout-imagesize: If only parts of a chip are read, should the read image still have full chip size with all read regions filled in at the respective locations?
what david said already... with my (*ahem* chromium's) patches both is possible (even in the same execution): having one big image with the parts at their offsets, or a file for (some or or every) region with offset 0. and that's how it should be imho :)
partialread-layout-split: If only parts of a chip are read, should it be possible to write each region (or a combination thereof) to a separate image file, and would that mapping be specified in the layout file?
see above and patches ;)
partialwrite-layout-split: If only parts of a chip are written, should it be possible to collect each part of the new image from a separate image file, and would that mapping be specified in the layout file?
yes, yes, see patches.
readwrite-protection-time: Which read protection and write protection times exist? Temporary lock until unlock, temporary lock until chip soft reset, temporary lock until chip/programmer hard reset (powerdown or reset line), permanent eternal lock.
readwrite-protection-type: Which read protection and write protection types exist? Programmer lock (e.g. Intel SPI), hardware chip lock (WP pin), software chip lock (chip command).
readwrite-protection-interaction: How should we express this situation: A region is write-locked with a software chip lock, but to remove that software chip lock, a hardware chip lock has to be disabled first, then the software chip lock can be disabled.
do we need to express it somehwere? we should just unlock as much as possible at startup, return layout data describing what ranges are readable and writable according to the module (chipset enable/programmer init and flash chip probe/query lock). if the union of all those layouts indicate we can do what we want fine, if not bail out.
partialaccess-crash: Some regions in the chip are really off-limits and will cause an unrecoverable error (system crash) when accessed (read or write). That seems to be the case for some EC/flash interactions.
so far we have only discussed don't read and don't write ranges. the crash regions should simply be described as both (WP and RP) and handled implicitly. is there a problem with that?
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
start and len should be self-explanatory. perms has two bits: WRITE_ALLOWED and READ_ALLOWED (preferably with a better name).
The chipaccess* arrays MUST (in the RFC 2119 sense of the word) cover the whole flash chip, holes are not allowed. 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.
Regards, Carl-Daniel
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?
start and len should be self-explanatory. perms has two bits: WRITE_ALLOWED and READ_ALLOWED (preferably with a better name).
_EN or _ENABLE? i'd prefer bit-fields (or two distinct variables) to store the perms actually. why combine them in one variable? an optional char* name would be nice, this could be used for layout file writing later and UI. and of course "unsigned int" is not specific enough.
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 :)
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.
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).
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
On Sun, 18 Sep 2011 01:26:11 +0200 Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net wrote:
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.
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.
forgive me this stupid question, but why should this be distinct? i mean the struct/type, not necessarily all uses. we will want to map them into each other anyway eventually.
and of course "unsigned int" is not specific enough.
Right, we want uint32_t.
actually we will want our new typedefed type(s) i guess?
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.
while(cur != null) { if (match(cur)) { do(cur); return ok; } cur = cur->next; } default(); return not_found;
does not look so complicated? :) what may be not so trivial is in deciding what default() should do then... but i am to tired to think of something. can you name an example?
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.
ichspi is *not* a problem at all. it is able to return a thorough map of access on/after init. it can make the defaults explicit by setting the unused ranges to the according meaning. it may define a mapping for addresses higher than possible with the attached chip, but that should not be a problem for our code. think of the array of a list of access restrictions. if it is not forbidden, it is allowed. if a chip is 2 Mb then it will define everything above 2Mb-1 as inaccessible.
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.
sure it is, but why did you name it explicitly in your first mail, what are your plans for it? i should probably read the start of the thread again, OutOfContextException :)
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.
see above.
maybe i am missing something. too tired and too much work to do, sorry :)
This is slightly related to the write strategy discussion, so I'll hijack its thread. While rebasing my layout patch set I rewrote the layout file parser. It is now more flexible and it would be easy to support more features. One feature it does already support are comments starting with a # (anywhere in a line).
ATM it parses all previously valid layout files (AFAIK) and it should continue to do so, even if they are not too common yet. We do not know the exact numbers of usage, so breaking an unknown number of systems does not seem like a good idea. We could and should provide a script that converts 1.0 files to 2.0 syntax though. That should be trivial anyway. One way to guarantee compatibility would be to have two modes of operation in the parser and to mark layout 2.0 files somehow. This way we could make sure that old formats are always handled correctly without making it hard to add new features. The marks do not need to be backward compatible IMHO (would be hard and/or awkward to do anyway because the old format is so limited). So in case we need it I propose the following mark: Layout 2.0 files have to start with a comment in the first line including "flashrom layout 2.0" (which is trivially implementable: skipping whitespace till # in a loop, find the string with strstr (and rewind)).
NB: Currently everything after the first space is considered the name of the region. A typical line in 1.0 looks like this: 00009000:0003ffff normal and it cant look much differently.
Things I definitely want to see in 2.0 files: - Comments. :) - Replacing mandatory hex with auto-detected address range bases. While probably everyone will use hex numbers I dont see a reason why we should enforce this. Converting 1.0 files by adding 0x in front of the addresses is easy to do too.
Other possible features (just food for thought): - Default image names. For example one might want to always use me.bin for the ME region without the need to specify it all the time on the cli. (add the name after the region name, separated by : just like in the cli) - Default inclusion status. It would be nice to be able to mark some regions as to be included by default so that one does not even need to use -i. (no idea about syntax yet) - Includes of other layout files (include .*) - Some kind of priority if one selects overlapping regions.
Comments and further suggestions very welcome!
My current plan is to use the modified parser which parses the old layout file without a problem but includes comments in my first batch of layout patches. That consists mostly of the known patches to add read, erase and verify support for layout files + that parser patch + what else I can implement before the batch gets merged, but probably without any other features discussed in here.
On Thu, Jul 11, 2013 at 9:42 AM, Stefan Tauner < stefan.tauner@student.tuwien.ac.at> wrote:
This is slightly related to the write strategy discussion, so I'll hijack its thread. While rebasing my layout patch set I rewrote the layout file parser. It is now more flexible and it would be easy to support more features. One feature it does already support are comments starting with a # (anywhere in a line).
Yay!
ATM it parses all previously valid layout files (AFAIK) and it should continue to do so, even if they are not too common yet. We do not know the exact numbers of usage, so breaking an unknown number of systems does not seem like a good idea. We could and should provide a script that converts 1.0 files to 2.0 syntax though. That should be trivial anyway. One way to guarantee compatibility would be to have two modes of operation in the parser and to mark layout 2.0 files somehow. This way we could make sure that old formats are always handled correctly without making it hard to add new features. The marks do not need to be backward compatible IMHO (would be hard and/or awkward to do anyway because the old format is so limited). So in case we need it I propose the following mark: Layout 2.0 files have to start with a comment in the first line including "flashrom layout 2.0" (which is trivially implementable: skipping whitespace till # in a loop, find the string with strstr (and rewind)).
NB: Currently everything after the first space is considered the name of the region. A typical line in 1.0 looks like this: 00009000:0003ffff normal and it cant look much differently.
Things I definitely want to see in 2.0 files:
- Comments. :)
- Replacing mandatory hex with auto-detected address range bases. While probably everyone will use hex numbers I dont see a reason why we should enforce this. Converting 1.0 files by adding 0x in front of the addresses is easy to do too.
Sounds good. I think it also makes it easier to automatically generate layout files in scripts and such. CDH and I had a brief discussion about this here: http://www.flashrom.org/pipermail/flashrom/2010-November/005465.html .
Other possible features (just food for thought):
- Default image names. For example one might want to always use me.bin for the ME region without the need to specify it all the time on the cli. (add the name after the region name, separated by : just like in the cli)
+1. It's pretty silly to need to specify a ROM-sized file for "-w" just to update a single region.
- Default inclusion status. It would be nice to be able to mark some regions as to be included by default so that one does not even need to use -i. (no idea about syntax yet)
This one could get complicated... Maybe for the layout 2.0 you can add a column to specify include options like "always" or "never". That could be made into a comma-separated list with other region attributes (TBD).
- Includes of other layout files (include .*)
Interesting! I like that idea.
- Some kind of priority if one selects overlapping regions.
Interesting... Not sure I like it though. It seems too easy to botch this IMO.
Comments and further suggestions very welcome!
My current plan is to use the modified parser which parses the old layout file without a problem but includes comments in my first batch of layout patches. That consists mostly of the known patches to add read, erase and verify support for layout files + that parser patch + what else I can implement before the batch gets merged, but probably without any other features discussed in here.
SGTM.
On 7/11/13 9:42 AM, Stefan Tauner wrote:
This is slightly related to the write strategy discussion, so I'll hijack its thread. While rebasing my layout patch set I rewrote the layout file parser. It is now more flexible and it would be easy to support more features. One feature it does already support are comments starting with a # (anywhere in a line).
ATM it parses all previously valid layout files (AFAIK) and it should continue to do so, even if they are not too common yet. We do not know the exact numbers of usage, so breaking an unknown number of systems does not seem like a good idea. We could and should provide a script that converts 1.0 files to 2.0 syntax though. That should be trivial anyway.
Current layout files have not ever been used afaik, except by a customer of mine who went out of business several years ago. Feel free to just deprecate the old format, instead of engineering a tedious migration path.
Stefan
On Thu, 11 Jul 2013 23:40:02 -0700 Stefan Reinauer stefan.reinauer@coreboot.org wrote:
On 7/11/13 9:42 AM, Stefan Tauner wrote:
This is slightly related to the write strategy discussion, so I'll hijack its thread. While rebasing my layout patch set I rewrote the layout file parser. It is now more flexible and it would be easy to support more features. One feature it does already support are comments starting with a # (anywhere in a line).
ATM it parses all previously valid layout files (AFAIK) and it should continue to do so, even if they are not too common yet. We do not know the exact numbers of usage, so breaking an unknown number of systems does not seem like a good idea. We could and should provide a script that converts 1.0 files to 2.0 syntax though. That should be trivial anyway.
Current layout files have not ever been used afaik, except by a customer of mine who went out of business several years ago. Feel free to just deprecate the old format, instead of engineering a tedious migration path.
Hm interesting. I am not so sure about that, but it is certainly not a much used feature (yet). Which is probably related to the fact, that it is not very usable as it was. :) OTOH there was a huge interest in that features in the last months from Intel users due to the ME limitations (which was also my main motivation to work on it). In any case I don't think that the migration would be very tedious. Carldani brought up the compatibility issue back when david wanted to change the address base: http://www.flashrom.org/pipermail/flashrom/2010-November/005465.html
-
Carl-Daniel Hailfinger -
David Hendricks -
Stefan Reinauer -
Stefan Tauner -
Tadas Slotkus