[flashrom] Patch to fix wrmsr for some toolchains

[Carl-Daniel Hailfinger]

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/