Re: [LinuxBIOS] flashrom support for AMD Geode CS5536

[Jordan Crouse]

Sigh - I hit 'r' instead of 'g'.  I blame mutt... :)

On 05/10/07 11:35 -0600, Jordan Crouse wrote:
> On 05/10/07 10:16 -0600, Marc Jones wrote:
> > Lane Brooks wrote:
> > > 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
> > > >
> > > > ron minnich wrote:
> > > > > On 10/2/07, Lane Brooks  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
> >
> > Did anyone ack and commit this last version?
>
> I think ron Acked it, but I'll ack again.
>
> Acked-by: Jordan Crouse
> > Marc