[GIT PULL] arm64: UEFI updates for 3.19
ard.biesheuvel at linaro.org
Fri Nov 7 04:25:09 PST 2014
On 7 November 2014 12:24, Yuanhan Liu <yuanhan.liu at linux.intel.com> wrote:
> On Fri, Nov 07, 2014 at 12:06:02PM +0100, Ard Biesheuvel wrote:
>> On 7 November 2014 11:48, Matt Fleming <matt.fleming at intel.com> wrote:
>> > On Fri, 2014-11-07 at 11:29 +0100, Ard Biesheuvel wrote:
>> >> I have confirmation from Matt and another Intel engineer that adding a
>> >> (arguably redundant) 'u32' cast solves the issue.
>> > I think we need to get a better handle on this.
>> > It is completely surprising to me that type promotion from u32 to u64
>> > goes through sign extension.
>> > And by "surprising" I mean, it sounds wrong.
>> > Ard, if you could throw me a unified diff of objdump -dr vmlinux, with
>> > and without the u32 cast, I'll take a look at figuring out what's
>> > happening.
>> With my compiler
>> gcc version 4.8.1 (Ubuntu/Linaro 4.8.1-10ubuntu9)
>> the objdump of drivers/firmware/dmi_scan.o is identical with and
>> without the 'u32' cast.
>> If I look at the original code:
>> dmi_base = (buf << 24) | (buf << 16) | (buf << 8) | buf;
>> I see a 'cltq' instruction in the dump that disappears once I add the
>> 'u32' cast (which is what we addressed by introducing the
>> get_unaligned_le32() in the 1st place)
> Yes, that's another mistake I made :(
> The story is that LKP reports this issue with an old commit:
> aacdce6e880894acb57d71dcb2e3fc61b4ed4e96("dmi: add support for SMBIOS
> 3.0 64-bit entry point"), where still use the original code you showed
> dmi_num = (buf << 8) | buf;
> dmi_len = (buf << 8) | buf;
> dmi_base = (buf << 24) | (buf << 16) |
> (buf << 8) | buf;
> I didn't except there are two version, thus when it failed to apply the
> debug patch you gave me, I just thought you wrote the debug patch on top
> your branch, and I applied on based of the first bad commit("aacdce6e").
> Hence I fixed it manually and got your debug patch applied, but still
> with those original code, hence it never works.
> And then you ask me to do the (u32) cast, which is based on
> get_unaligned_le32(), I then changed the original code to
> get_unaligned_le16/32(), and did the cast, which works as expected, and
> I then thought the cast did work. And actually, it's the change to
> get_unaligned_le16/32() works: I double confirmed it this time, and
> that's why I didn't see such panic with your updated commits from
> efi-for-arm64(the code we used bisect is from efi-for-3.19).
> So, it's totally kind of stupid mistakes I made. Very sorry for the
> noise and for taking you guys time!
No worries. At the very least, we are a bit more confident now than
before that the code is fine.
More information about the linux-arm-kernel