[LinuxBIOS] flashrom support for AMD Geode CS5536

Carl-Daniel Hailfinger c-d.hailfinger.devel.2006 at gmx.net
Thu Oct 4 18:17:07 CEST 2007


On 02.10.2007 19:01, Stefan Reinauer wrote:
> * ron minnich <rminnich at gmail.com> [071002 18:38]:
>>> My questions are:
>>> - I do not think this is portable beyond linux.  Is that an issue?
>> it's not, but it is not really an issue at present.
> 
> Maybe the code should be #ifdef'ed __linux__ with an #else case just
> printing a warning a la "Not supported by this OS yet."
> 
>> I thought that was in the structure of flashrom. Now that I look, it
>> seems like we lost it!
> 
> Flashrom never did any such cleanup. I was about to implement it when we
> renamed the tool but then I stumbled upon this:
> 
> printf("OK, only ENABLING flash write, but NOT FLASHING.\n");
> 
> So obviously at some point there was a sense in leaving the system in
> such a state. But I want to propose that we drop this behavior and
> instead try to always leave the machine in the state we entered it.
> Especially when not flashing.
> 
> While one might want to mess with an unprotected flash on purpose, for
> 99% of the cases this is just opening another security issue.
> That one % that theoretically might use this as a feature is welcome to
> improve the flashrom utility instead of running it for flash
> unprotection before running another utility.

Agreed.

>> I propose this at the end of flashrom:
>> board_flash_disable(lb_vendor, lb_part);
>> chipset_flash_disable(chipset);
> 
> yepp. Agreed.

Disagree. We still have to store the previous state to be able to revert
to it.

>> but we'll need to change some things to make this all work. We need a
>> penable struct * to use for the disable; no point in searching each
>> time we touch a chip. or not?
> 
> To achieve this
> 
> struct board_pciid_enable *board in board_enable.c:board_flash_enable() 
> 
> and
> 
> enables[i] in chipset_enable.c:chipset_flash_enable() 
> 
> should be globally available.

Different proposal:
If a particular enable function has been run successfully, it should
store a pointer to the corresponding cleanup function and a pointer to
the data which has to be restored.

At least the restore information has to be available somewhere.
Otherwise the exercise is totally pointless since we can't know the
state before the enable function was run.

Carl-Daniel




More information about the coreboot mailing list