[LinuxBIOS] flashrom support for AMD Geode CS5536
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);
>> 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 :-)
-------------- next part --------------
A non-text attachment was scrubbed...
Size: 2397 bytes
Desc: not available
More information about the coreboot