[flashrom] [RFC] Layout handling...

[Stefan Tauner]

On Sun, 18 Sep 2011 01:26:11 +0200
Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at 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 at 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 :)

-- 
Kind regards/Mit freundlichen Grüßen, Stefan Tauner