[LinuxBIOS] flashrom support for AMD Geode CS5536

Lane Brooks lbrooks at MIT.EDU
Tue Oct 2 20:03:48 CEST 2007


Here it is again with the patch actually attached.

Lane Brooks wrote:
> 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
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: cs5536.patch
Type: text/x-patch
Size: 2397 bytes
Desc: not available
URL: <http://www.coreboot.org/pipermail/coreboot/attachments/20071002/b620f8d8/attachment.patch>


More information about the coreboot mailing list