From: yuanhan.liu@linux.intel.com (Yuanhan Liu)
To: linux-arm-kernel@lists.infradead.org
Subject: [GIT PULL] arm64: UEFI updates for 3.19
Date: Fri, 7 Nov 2014 19:24:07 +0800 [thread overview]
Message-ID: <20141107112407.GH24745@yliu-dev.sh.intel.com> (raw)
In-Reply-To: <CAKv+Gu9woybrvA0vikmcOeyVs0xTsAwBVu1=QsvaM9_Eb0pUEQ@mail.gmail.com>
On Fri, Nov 07, 2014 at 12:06:02PM +0100, Ard Biesheuvel wrote:
> On 7 November 2014 11:48, Matt Fleming <matt.fleming@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[11] << 24) | (buf[10] << 16) | (buf[9] << 8) | buf[8];
>
> 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
above:
dmi_num = (buf[13] << 8) | buf[12];
dmi_len = (buf[7] << 8) | buf[6];
dmi_base = (buf[11] << 24) | (buf[10] << 16) |
(buf[9] << 8) | buf[8];
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!
--yliu
> So, I'd happily share more objdumps, but perhaps we should find out
> first which compiler Yuanhan has been using?
>
> --
> Ard.
next prev parent reply other threads:[~2014-11-07 11:24 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-11-05 10:54 [GIT PULL] arm64: UEFI updates for 3.19 Ard Biesheuvel
2014-11-07 7:34 ` Ard Biesheuvel
2014-11-07 10:04 ` Will Deacon
2014-11-07 10:07 ` Catalin Marinas
2014-11-07 10:29 ` Ard Biesheuvel
2014-11-07 10:48 ` Matt Fleming
2014-11-07 11:06 ` Ard Biesheuvel
2014-11-07 11:24 ` Yuanhan Liu [this message]
2014-11-07 12:25 ` Ard Biesheuvel
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20141107112407.GH24745@yliu-dev.sh.intel.com \
--to=yuanhan.liu@linux.intel.com \
--cc=linux-arm-kernel@lists.infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).