Re: [flashrom] Patches + RFC: Deal with locked ICH flash descriptor regions transparently

[Stefan Tauner]

hey and thanks for working on getting stuff back upstream!

i have very little time at the moment (and the next few weeks) so i
just skimmed over the patches a few days ago and will just mention the
main points i noticed.

On Fri, 2 Dec 2011 17:38:20 -0800
David Hendricks  wrote:
> The following patches (attached and in-lined below) add support for
> "graceful" handling of certain errors and in particular the read/write
> errors that occur on newer Intel chipsets. With a little work, it could be
> extended to handle SPI hardware write protection schema, too.

1. the patches seemed to be divided in a little bit odd way, but that
might actually be because i did not read that carefully. btw please
post one patch per mail so that patchwork recognize them all.
> Note: I think we should probably add a "I_want_a_brick" programmer
> parameter for those daring enough to try this. I suspect certain OEM tools
> use unknown Mebx
> commandshttp://support.dell.com/support/edocs/systems/latd630/en/amt/MEBX.htmto
> disable ME read/write protection when they do updates. Perhaps that
> stuff can be added later as a board_enable thing.

2. i dont know enough about MEBx, but i have not found any mention of
it in any official documentation about ME unlocking - public or otherwise.
the user interface as shown on the dell screenshot might not be
accessible for users. i need to test if the MEBx settings have any
influence on the ICH registers on my ICH10 based (and locked) intel
board. wont happen soon though.

3. the official way to disabled the ME locking is via an HECI™/MEI™
service™ with™ the™ HMRFPO™ Enable™ Intel® command™. the MEI protocol is
packet-based and well documented including an open source linux kernel
module. i have ported this to flashrom and will try to get it into a
mergeable state in the future. the only missing part then is the right
MEI command/reply sequence to unlock the ME region. the integration
would not use board enables but the unlocking would be done
automatically in ich_init (and re-enabling in shutdown, if we could
figure out how... not that important imho because it is security be
obscurity anyway). the HMRFPO command is documented in the BIOS writer
guide(s).
> With the patches applied, the low-level ICH code will handle all the
> details of checking the address associated with an opcode against the
> permissions set in the flash descriptor. If an operation cannot be
> performed on a given region, an error code is propagated up the stack so
> that high-level read/write logic can decide to skip that region. As is, the
> patch will make the high-level logic fill in unreadable regions with 0xff
> bytes (for reads) and will quietly skip over unwriteable regions (for
> writes).
>
> The biggest downside is, of course, that read/write operations no longer do
> exactly what one might expect. A full read operation (flashrom -r) will
> give you a ROM-sized image with 0xff bytes wherever read is forbidden (e.g.
> where the management engine firmware lives). A full write operation will
> skip forbidden regions (e.g. flash descriptor and ME) which may leave stuff
> in an inconsistent and potentially unusable state (thanks, Intel!).

4. i dont like the overall approach of the patches at all. they
complicate the most complicated parts of flashrom even more (chunked
read/erase/write) for little gain for an even littler target
audience (no offense google :). the init code can already tell the
upper level code which parts are accessible and which are not. there is
no need to fiddle around with the opcodes on the fly (in addition to
what we do to reprogram opcodes because we are forced to... :)

i would like to do this in a less intrusive and more generic way by
means of layout(file)s.
patches for enabling the usage of layouts in all program modes exists
already and would give anyone who dares an easy way to operate on any
accessible region on its own (reviews are welcome! :P).
an additional layout file generation by programmer init code (when told
so) or a generic automagic mode that queries the programmer, the flash
chip and maybe other modules for access rights to plan the whole
operation before really issuing read/erase/write commands (a level
above your implementation) would give us the same functionality, but
also would be useful for non-ich users, generic to be extended in the
future and most importantly: it would not touch the core read/write
functions.