On Thu, 16 Feb 2012 01:11:32 +0100
Carl-Daniel Hailfinger <[email protected]> wrote:

> Am 16.02.2012 00:58 schrieb Stefan Tauner:
> > This includes not only the notorious read-only flash descriptors and locked 
> > ME
> > regions, but also the more rarely used PRs (Protected Ranges).
> > The user can enforce write support by specifying ich_spi_force=yes in the
> > programmer options, but we don't tell him the exact syntax interactively. He
> > has to read it up in the man page.
> >
> > Signed-off-by: Stefan Tauner <[email protected]>
> > Acked-by: Carl-Daniel Hailfinger <[email protected]>
> 
> The Intel rant seems to be missing... I thought you only wanted to move it.

postponed... there is so much i should add to that simple rant ;)

> > diff --git a/flashrom.8 b/flashrom.8
> > index e5f9a29..76aacba 100644
> > --- a/flashrom.8
> > +++ b/flashrom.8
> > @@ -315,6 +315,21 @@ important opcodes are inaccessible due to lockdown; or 
> > if more than one flash
> >  chip is attached). The other options (swseq, hwseq) select the respective 
> > mode
> >  (if possible).
> >  .sp
> > +ICH8 and later southbridges may also have locked address ranges of 
> > different
> > +kinds if a valid descriptor was written to it. The flash address space is 
> > then
> > +partitioned in multiple so called "Flash Regions" containing the host 
> > firmware,
> > +the ME firmware and so on respectively. The flash descriptor can also 
> > specify up
> > +to 5 so called "Protected Regions", which are freely chosen address ranges
> > +independent from the aforementioned "Flash Regions". All of them can be 
> > write
> > +and/or read protected individually. If flashrom detects such a lock it will
> > +disable write support unless the user forces it with the
> > +.sp
> > +.B "  flashrom \-p internal:ich_spi_force=yes"
> > +.sp
> > +syntax. If this leads to erase or write accesses to the flash it would most
> > +probably bring it into an inconsistent and unbootable state and we will not
> > +provide any support in such a case.
> > +.sp
> >  If you have an Intel chipset with an ICH6 or later southbridge and if you 
> > want
> >  to set specific IDSEL values for a non-default flash chip or an embedded
> >  controller (EC), you can use the
> > diff --git a/ichspi.c b/ichspi.c
> > index 163ecf1..711f46c 100644
> > --- a/ichspi.c
> > +++ b/ichspi.c
> > @@ -1444,11 +1445,16 @@ static void do_ich9_spi_frap(uint32_t frap, int i)
> >     if (base > limit) {
> >             /* this FREG is disabled */
> >             msg_pdbg("%s region is unused.\n", region_names[i]);
> > -           return;
> > +           return 0;
> >     }
> >  
> > -   msg_pdbg("0x%08x-0x%08x is %s\n", base, (limit | 0x0fff),
> > +   msg_pdbg("0x%08x-0x%08x is %s.\n", base, (limit | 0x0fff),
> >              access_names[rwperms]);
> > +   if (rwperms == 0x3)
> > +           return 0;
> > +
> > +   msg_pinfo("WARNING: %s region is not fully accessible.\n", 
> > region_names[i]);
> 
> Odd. For FRAP, the _pinfo message just says "not fully accessible", but
> for PR, the _pinfo message mentions the actual read/write protection.
> Not terribly important to fix, just a small inconsistency I noticed.

terribly to fix actually. :)

> > +   int ich_spi_force = 0;
> > +   int ich_spi_has_locks = 0;
> 
> Rename to ich_spi_has_region_locks or ich_spi_has_locked_regions or
> somesuch. Otherwise there is possibility for confusion between
> ich_spi_has_locks and ichspi_lock.

ich_spi_rw_restricted

 
> Thanks for working tirelessly to get this code into an excellent shape!

Thanks for your very work-creating^Wuseful comments!
committed in r1494.

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

_______________________________________________
flashrom mailing list
[email protected]
http://www.flashrom.org/mailman/listinfo/flashrom

Reply via email to