[GIT PULL] arm64: UEFI updates for 3.19

[GIT PULL] arm64: UEFI updates for 3.19

[Ard Biesheuvel]

Hi Will,

The following changes since commit cac7f2429872d3733dc3f9915857b1691da2eb2f:

  Linux 3.18-rc2 (2014-10-26 16:48:41 -0700)

are available in the git repository at:

  git://git.linaro.org/people/ard.biesheuvel/linux-arm.git tags/for-3.19

for you to fetch changes up to 0bcaa9040d058684d58c36ef273b8946996c7078:

  efi: efi-stub: notify on DTB absence (2014-11-05 09:03:34 +0100)

----------------------------------------------------------------
UEFI updates for arm64

This series consists of:
- fixes for compliance with PE/COFF and UEFI specs
- added support for SMBIOS, including upcoming version 3.0
- cleanups and diagnostic output improvements

----------------------------------------------------------------
Ard Biesheuvel (8):
      arm64/efi: efistub: jump to 'stext' directly, not through the header
      arm64/efi: set PE/COFF section alignment to 4 KB
      arm64/efi: set PE/COFF file alignment to 512 bytes
      arm64/efi: invert UEFI memory region reservation logic
      arm64/efi: drop redundant set_bit(EFI_CONFIG_TABLES)
      efi: dmi: add support for SMBIOS 3.0 UEFI configuration table
      dmi: add support for SMBIOS 3.0 64-bit entry point
      arm64: dmi: set DMI string as dump stack arch description

Mark Rutland (1):
      efi: efi-stub: notify on DTB absence

Yi Li (1):
      arm64: dmi: Add SMBIOS/DMI support

 arch/arm64/Kconfig                      | 11 +++++
 arch/arm64/include/asm/dmi.h            | 31 +++++++++++++
 arch/arm64/kernel/efi-entry.S           |  3 +-
 arch/arm64/kernel/efi.c                 | 37 ++++++++++-----
 arch/arm64/kernel/head.S                | 24 +++++++---
 arch/arm64/kernel/vmlinux.lds.S         | 17 +++++++
 drivers/firmware/dmi_scan.c             | 79 ++++++++++++++++++++++++++++++---
 drivers/firmware/efi/efi.c              |  4 ++
 drivers/firmware/efi/libstub/arm-stub.c | 11 ++++-
 drivers/xen/efi.c                       |  1 +
 include/linux/efi.h                     |  6 ++-
 11 files changed, 195 insertions(+), 29 deletions(-)
 create mode 100644 arch/arm64/include/asm/dmi.h

[GIT PULL] arm64: UEFI updates for 3.19

[Ard Biesheuvel]

On 5 November 2014 11:54, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> Hi Will,
>

You are not going to believe this, but I received a report about boot
failures on legacy (non-EFI) x86 using the SMBIOS 3.0 code, so could
you please hold off on pulling this until we know what is going on
there? This does not affect ARM at all, but I may need to update patch
'dmi: add support for SMBIOS 3.0 64-bit entry point' to drop the
SMBIOS 3.0 check in the non-EFI code path.

-- 
Ard.


> The following changes since commit cac7f2429872d3733dc3f9915857b1691da2eb2f:
>
>   Linux 3.18-rc2 (2014-10-26 16:48:41 -0700)
>
> are available in the git repository at:
>
>   git://git.linaro.org/people/ard.biesheuvel/linux-arm.git tags/for-3.19
>
> for you to fetch changes up to 0bcaa9040d058684d58c36ef273b8946996c7078:
>
>   efi: efi-stub: notify on DTB absence (2014-11-05 09:03:34 +0100)
>
> ----------------------------------------------------------------
> UEFI updates for arm64
>
> This series consists of:
> - fixes for compliance with PE/COFF and UEFI specs
> - added support for SMBIOS, including upcoming version 3.0
> - cleanups and diagnostic output improvements
>
> ----------------------------------------------------------------
> Ard Biesheuvel (8):
>       arm64/efi: efistub: jump to 'stext' directly, not through the header
>       arm64/efi: set PE/COFF section alignment to 4 KB
>       arm64/efi: set PE/COFF file alignment to 512 bytes
>       arm64/efi: invert UEFI memory region reservation logic
>       arm64/efi: drop redundant set_bit(EFI_CONFIG_TABLES)
>       efi: dmi: add support for SMBIOS 3.0 UEFI configuration table
>       dmi: add support for SMBIOS 3.0 64-bit entry point
>       arm64: dmi: set DMI string as dump stack arch description
>
> Mark Rutland (1):
>       efi: efi-stub: notify on DTB absence
>
> Yi Li (1):
>       arm64: dmi: Add SMBIOS/DMI support
>
>  arch/arm64/Kconfig                      | 11 +++++
>  arch/arm64/include/asm/dmi.h            | 31 +++++++++++++
>  arch/arm64/kernel/efi-entry.S           |  3 +-
>  arch/arm64/kernel/efi.c                 | 37 ++++++++++-----
>  arch/arm64/kernel/head.S                | 24 +++++++---
>  arch/arm64/kernel/vmlinux.lds.S         | 17 +++++++
>  drivers/firmware/dmi_scan.c             | 79 ++++++++++++++++++++++++++++++---
>  drivers/firmware/efi/efi.c              |  4 ++
>  drivers/firmware/efi/libstub/arm-stub.c | 11 ++++-
>  drivers/xen/efi.c                       |  1 +
>  include/linux/efi.h                     |  6 ++-
>  11 files changed, 195 insertions(+), 29 deletions(-)
>  create mode 100644 arch/arm64/include/asm/dmi.h

[GIT PULL] arm64: UEFI updates for 3.19

[Will Deacon]

On Fri, Nov 07, 2014 at 07:34:12AM +0000, Ard Biesheuvel wrote:
> You are not going to believe this, but I received a report about boot
> failures on legacy (non-EFI) x86 using the SMBIOS 3.0 code, so could
> you please hold off on pulling this until we know what is going on
> there? This does not affect ARM at all, but I may need to update patch
> 'dmi: add support for SMBIOS 3.0 64-bit entry point' to drop the
> SMBIOS 3.0 check in the non-EFI code path.

D'oh, I put this into -next last night (perhaps that's where the report came
from), so I'll recreate that branch now without your series.

Please send another pull request without the SMBIOS patch, then we can
perhaps add that on top later on (that way it's easier to revert in the
future :).

Will

[GIT PULL] arm64: UEFI updates for 3.19

[Catalin Marinas]

On Fri, Nov 07, 2014 at 10:04:35AM +0000, Will Deacon wrote:
> On Fri, Nov 07, 2014 at 07:34:12AM +0000, Ard Biesheuvel wrote:
> > You are not going to believe this, but I received a report about boot
> > failures on legacy (non-EFI) x86 using the SMBIOS 3.0 code, so could
> > you please hold off on pulling this until we know what is going on
> > there? This does not affect ARM at all, but I may need to update patch
> > 'dmi: add support for SMBIOS 3.0 64-bit entry point' to drop the
> > SMBIOS 3.0 check in the non-EFI code path.
> 
> D'oh, I put this into -next last night (perhaps that's where the report came
> from), so I'll recreate that branch now without your series.
> 
> Please send another pull request without the SMBIOS patch, then we can
> perhaps add that on top later on (that way it's easier to revert in the
> future :).

You have a typo: s/perhaps/never/

;)

-- 
Catalin

[GIT PULL] arm64: UEFI updates for 3.19

[Ard Biesheuvel]

On 7 November 2014 11:04, Will Deacon <will.deacon@arm.com> wrote:
> On Fri, Nov 07, 2014 at 07:34:12AM +0000, Ard Biesheuvel wrote:
>> You are not going to believe this, but I received a report about boot
>> failures on legacy (non-EFI) x86 using the SMBIOS 3.0 code, so could
>> you please hold off on pulling this until we know what is going on
>> there? This does not affect ARM at all, but I may need to update patch
>> 'dmi: add support for SMBIOS 3.0 64-bit entry point' to drop the
>> SMBIOS 3.0 check in the non-EFI code path.
>
> D'oh, I put this into -next last night (perhaps that's where the report came
> from), so I'll recreate that branch now without your series.
>

Ah, yes, that is probably where they found it. Note that this is a
different patch than the one that has been causing us (me) grief
before, but apparently it now has infected other patches as well :-)

> Please send another pull request without the SMBIOS patch, then we can
> perhaps add that on top later on (that way it's easier to revert in the
> future :).
>

In the mean time, we have pinpointed this. It turns out that on x86,
the RHS of this expression

u64 dmi_base = get_unaligned_le32(buf + 8);

is promoted to a signed type before being assigned, even if
get_unaligned_le32() returns u32.
On ARM, it works as expected.

I have confirmation from Matt and another Intel engineer that adding a
(arguably redundant) 'u32' cast solves the issue.

Should I include the corrected patch and send an updated pull request?
Or if you and Matt prefer so, we could take this patch and the
preceding one (efi: dmi: add support for SMBIOS 3.0 UEFI configuration
table) through Matt's tree as well. There are no interdependencies
between those two and the other patches.

-- 
Ard.

[GIT PULL] arm64: UEFI updates for 3.19

[Matt Fleming]

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.

[GIT PULL] arm64: UEFI updates for 3.19

[Ard Biesheuvel]

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)

So, I'd happily share more objdumps, but perhaps we should find out
first which compiler Yuanhan has been using?

-- 
Ard.

[GIT PULL] arm64: UEFI updates for 3.19

[Yuanhan Liu]

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.

[GIT PULL] arm64: UEFI updates for 3.19

[Ard Biesheuvel]

On 7 November 2014 12:24, Yuanhan Liu <yuanhan.liu@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@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!
>

No worries. At the very least, we are a bit more confident now than
before that the code is fine.

Thanks,
Ard.