On 18.05.2010 14:19, Mart Raudsepp wrote: > Hello, > > On E, 2010-05-17 at 16:20 -0700, ron minnich wrote: > >> Hi, I'd like to know a bit more. >> >> This basic use of the msr device has worked for a very long time. I'm >> not saying there is not a problem, but if there is, it might be best >> to change the struct than to add this code. >> > > Right. This got broken on our toolchains ever since this struct was > introduced to flashrom fater 0.9 release. > I'm not even sure the same thing hit me now and him back in October, but > the same patch fixes it. > It would be weird if this is a new regression in newer GCC version > though, as that would be an ABI break I believe.. > > Do however note that rdmsr goes through a temporary uint32_t array as > well, just like this patch makes wrmsr. So with this patch it's sort of > uniform that way too. > > >> Do you have some assembly code showing how it failed? >> > > No I didn't. But now I got some :) > Attached is the physmap.c file from r998 compiled to assembly with all > the same optimization and other flags as the object file is made, just > saved to assembly with -S option. So if anyone can read from that what's > going on, then feel free to; I don't speak assembler well :( > > >> For example ... certainly, in user mode, why isn't the msr_t just a >> 64-bit number? >> > > Don't really know. All I know is that C language does not really give a > guarantee of structs being tightly packed (are there other rules though, > like for default alignment instead of compiler specific?), unless using > compiler extensions like GCC packed attribute. I have it hard to imagine > why it would be adding alignment padding in there either, if that's what > it is doing per the attached assembler. > However sizeof(msr_t) seem to be returning 8 as desired... >
Thanks, r999. We will need to revisit this in the future once we want to run flashrom on top of libpayload. Until then, this fix is good enough unless someone sends a patch to convert the MSR stuff to a 64bit variable. Regards, Carl-Daniel -- http://www.hailfinger.org/ _______________________________________________ flashrom mailing list [email protected] http://www.flashrom.org/mailman/listinfo/flashrom
