Re: [flashrom] [PATCH 2/8] ichspi: disable writes when locked or read-only regions are detected

[Carl-Daniel Hailfinger]

Am 04.09.2011 00:21 schrieb Stefan Tauner:
> On Fri, 02 Sep 2011 20:27:37 +0200
> Carl-Daniel Hailfinger  wrote:
> > Am 20.08.2011 12:39 schrieb Stefan Tauner:
> > > Until we are able to unlock regions or at least understand better what
> > > consequences writing single regions have, disable writing completely
> > > whenever a non-writeable region is detected.
> > >
> > > NB: the registers might not always reflect the real access status.
> > > http://www.flashrom.org/pipermail/flashrom/2011-April/006309.html
> > > (this is the sole such report i can remember)
> > >
> > > Might not be the best location to do this, but currently i don't recognize a better
> > > one. The warnings are repeated for each non-writeable region. We could
> > > surpress it by checking if programmer_may_write is already 0 before printing.
> > > the output on my thinkpad is:
> > > […]
> > > Proceeding anyway because user specified laptop=force_I_want_a_brick
> > > Found chipset "Intel QS57". Enabling flash write... WARNING: SPI Configuration Lockdown activated.
> > > WARNING: Flash Descriptor region is read-only. Disabling writes.
> > > WARNING: Management Engine region is locked. Disabling writes.
> > > OK.
> > > This chipset supports the following protocols: FWH, SPI.
> > > Found Winbond flash chip "W25X64" (8192 kB, SPI) at physical address 0xff800000.
> > > […]
> > >
> > > verbose output is:
> > > […]
> > > 0x54: 0x00000000 (FREG0: Flash Descriptor)
> > > 0x00000000-0x00000fff is read-only
> > > WARNING: Flash Descriptor region is read-only. Disabling writes.
> > > 0x58: 0x07ff0500 (FREG1: BIOS)
> > > 0x00500000-0x007fffff is read-write
> > > 0x5C: 0x04ff0003 (FREG2: Management Engine)
> > > 0x00003000-0x004fffff is locked
> > > WARNING: Management Engine region is locked. Disabling writes.
> > > 0x60: 0x00020001 (FREG3: Gigabit Ethernet)
> > > 0x00001000-0x00002fff is read-write
> > > 0x64: 0x00000fff (FREG4: Platform Data)
> > > Platform Data region is unused.
> > > […]
> >
> > How do we handle chips where a part of the chip is not part of any
> > region? What does the hardware enforce in that situation?
>
> i am not entirely sure yet if my hypothesis is correct, but i think
> the hardware prohibits any accesses outside defined regions (i.e.
> flash cycle error and/or access error log) - most probably only if the
> flash descriptor is valid of course (and the flash descriptor
> security override pin is not pulled down)...
>
> for some more detail please see
> [e-mail] in the "report
> for Intel QM67 | Winbond W25Q64" thread.
>
> and due to this new information, i am not so sure anymore this patch is
> a good idea... either we should add a check for PR-based protections and
> that the regions cover the whole chip too (probably the right thing to
> do according to my guts),

I think that would be a good idea. Maybe print a warning "please report
foo to flashrom@" in case the chip is not completely covered by regions
so we get a feeling for what's ok and what's not ok.
> or drop the patch.
>
> OTOH improved layout support would make it possible to
> read/write/verify unprotected parts without a problem. with this patch
> --force is needed to write them, correct?

I'm not sure how --force would change anything here.
> would be ok with me... as long
> as it would not completely hinder the user from writing.

If any part of the write has a chance of not working reliably, we should
completely block writing until we can exactly limit writes to writable
regions.

Regards,
Carl-Daniel