[LinuxBIOS] flashrom support for AMD Geode CS5536

Lane Brooks lbrooks at MIT.EDU
Tue Oct 2 20:00:16 CEST 2007


Here is the patch again with an updated comment regarding SMP and a 
signed-off-by line.

Signed-off-by: Lane Brooks <lbrooks at mit.edu>

ron minnich wrote:
> On 10/2/07, Lane Brooks <lbrooks at mit.edu> wrote:
>> Attached is a patch that enables AMD Geode CS5536 chipset support.  I
>> have tested it successfully on a MSM800 board from digital logic.  It
>> does, however, have a few issues that I would like some feedback on.
>>
>> In my discussions with Marc Jones, Geode systems write protect the BIOS
>> via RCONFs (cache settings similar to MTRRs). Unlocking requires
>> changing MSR 0x1808 top byte to 0x22. Reading and writing to the msr,
>> however, requires instrucitons rdmsr/wrmsr, which are ring0 privileged
>> instructions so only the kernel can do the read/write.  So my patch uses
>> the msr kernel module to access these instructions from user space using
>> the /dev/cpu/0/msr device.
>>
>> 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.
>> - My code assumes the msr kernel module is already loaded.  Is there a
>> way to force a kernel module to load from the C code?  My code does die
>> gracefully with a message reminding them to load the kernel module if
>> things fail.
> 
> it is possible I think to have udev help with this, but I am not sure.
> Graceful failure is certainly a good start.
> 
>> - It seems like there should be a way to revert the msr back after
>> flashing is completed to put the bios back in write protect mode.  Is
>> there a cleanup mechanism available?  Something like disable_flash...
> 
> I thought that was in the structure of flashrom. Now that I look, it
> seems like we lost it!
> I propose this at the end of flashrom:
> board_flash_disable(lb_vendor, lb_part);
> chipset_flash_disable(chipset);
> 
> 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?
> 
> one comment on your patch: you use /dev/cpu/0/msr. This is great for a
> single-cpu machine, and will always be fine on  geode. Lots of times,
> people use one piece of flashrom to write another. Imagine some future
> hacker, in a year or two, who has copied your code for some SMP
> system, not understanding why sometimes flashrom fails (i.e. they set
> CPU0 msr but end up running on cpu1). We had this very problem when we
> were getting graphics going (and, earlier, SMP going). I suggest a
> comment as to why the /dev/cpu/0/msr is always safe on geode, but
> future hackers beware on SMP. Or something like that.
> 
> That's up to you, however :-)
> 
> If you had a signed-off-by line, I would ack and commit for you :-)
> 
> ron
> 
> ron




More information about the coreboot mailing list