2016-11-24 18:03:03

by Ard Biesheuvel

[permalink] [raw]
Subject: [GIT PULL] one more EFI patch for v4.10

The following changes since commit 9110bc036062fcd31994a35540d63f8deed22efa:

thunderbolt: Use Device ROM retrieved from EFI (2016-11-12 21:14:43 +0000)

are available in the git repository at:

git://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git tags/efi-next

for you to fetch changes up to bec0c3c8151b7e1506fa2ae19500a8b8a32a79c0:

efi/libstub: Make efi_random_alloc() allocate below 4 GB on 32-bit (2016-11-22 09:53:49 +0000)

----------------------------------------------------------------
A single fix, for commit a6a144698d ("efi/libstub: Add random.c to ARM
build") that is queued for v4.10 in efi/core in the tip tree, since it
turned out that the compiler ends up emitting a call to a compiler
intrinsic to perform 64-bit logical shift when called with -Os. Upon
closer inspection of that code, it turned out we should explicitly ensure
that we do not allocate above 4 GB, which is not addressable by the stub
on 32-bit ARM.

----------------------------------------------------------------
Ard Biesheuvel (1):
efi/libstub: Make efi_random_alloc() allocate below 4 GB on 32-bit

drivers/firmware/efi/libstub/random.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)


2016-11-24 18:03:07

by Ard Biesheuvel

[permalink] [raw]
Subject: [PATCH] efi/libstub: Make efi_random_alloc() allocate below 4 GB on 32-bit

The UEFI stub executes in the context of the firmware, which identity
maps the available system RAM, which implies that only memory below
4 GB can be used for allocations on 32-bit architectures, even on [L]PAE
capable hardware.

So ignore any reported memory above 4 GB in efi_random_alloc(). This
also fixes a reported build problem on ARM under -Os, where the 64-bit
logical shift relies on a software routine that the ARM decompressor does
not provide.

A second [minor] issue is also fixed, where the '+ 1' is moved out of
the shift, where it belongs: the reason for its presence is that a
memory region where start == end should count as a single slot, given
that 'end' takes the desired size and alignment of the allocation into
account.

To clarify the code in this regard, rename start/end to 'first_slot' and
'last_slot', respectively, and introduce 'region_end' to describe the
last usable address of the current region.

Reported-by: Arnd Bergmann <[email protected]>
Cc: Matt Fleming <[email protected]>
Signed-off-by: Ard Biesheuvel <[email protected]>
---
drivers/firmware/efi/libstub/random.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/firmware/efi/libstub/random.c b/drivers/firmware/efi/libstub/random.c
index 3a3feacc329f..63cd3f262b6e 100644
--- a/drivers/firmware/efi/libstub/random.c
+++ b/drivers/firmware/efi/libstub/random.c
@@ -45,19 +45,21 @@ static unsigned long get_entry_num_slots(efi_memory_desc_t *md,
unsigned long align_shift)
{
unsigned long align = 1UL << align_shift;
- u64 start, end;
+ u64 first_slot, last_slot, region_end;

if (md->type != EFI_CONVENTIONAL_MEMORY)
return 0;

- start = round_up(md->phys_addr, align);
- end = round_down(md->phys_addr + md->num_pages * EFI_PAGE_SIZE - size,
- align);
+ region_end = min((u64)ULONG_MAX,
+ md->phys_addr + md->num_pages * EFI_PAGE_SIZE - 1);

- if (start > end)
+ first_slot = round_up(md->phys_addr, align);
+ last_slot = round_down(region_end - size + 1, align);
+
+ if (first_slot > last_slot)
return 0;

- return (end - start + 1) >> align_shift;
+ return ((unsigned long)(last_slot - first_slot) >> align_shift) + 1;
}

/*
--
2.7.4

Subject: [tip:efi/core] efi/libstub: Make efi_random_alloc() allocate below 4 GB on 32-bit

Commit-ID: 018edcfac4c3b140366ad51b0907f3becb5bb624
Gitweb: http://git.kernel.org/tip/018edcfac4c3b140366ad51b0907f3becb5bb624
Author: Ard Biesheuvel <[email protected]>
AuthorDate: Thu, 24 Nov 2016 18:02:23 +0000
Committer: Ingo Molnar <[email protected]>
CommitDate: Fri, 25 Nov 2016 07:15:23 +0100

efi/libstub: Make efi_random_alloc() allocate below 4 GB on 32-bit

The UEFI stub executes in the context of the firmware, which identity
maps the available system RAM, which implies that only memory below
4 GB can be used for allocations on 32-bit architectures, even on [L]PAE
capable hardware.

So ignore any reported memory above 4 GB in efi_random_alloc(). This
also fixes a reported build problem on ARM under -Os, where the 64-bit
logical shift relies on a software routine that the ARM decompressor does
not provide.

A second [minor] issue is also fixed, where the '+ 1' is moved out of
the shift, where it belongs: the reason for its presence is that a
memory region where start == end should count as a single slot, given
that 'end' takes the desired size and alignment of the allocation into
account.

To clarify the code in this regard, rename start/end to 'first_slot' and
'last_slot', respectively, and introduce 'region_end' to describe the
last usable address of the current region.

Reported-by: Arnd Bergmann <[email protected]>
Signed-off-by: Ard Biesheuvel <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: Matt Fleming <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: [email protected]
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Ingo Molnar <[email protected]>
---
drivers/firmware/efi/libstub/random.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/firmware/efi/libstub/random.c b/drivers/firmware/efi/libstub/random.c
index 3a3feac..7e72954 100644
--- a/drivers/firmware/efi/libstub/random.c
+++ b/drivers/firmware/efi/libstub/random.c
@@ -45,19 +45,20 @@ static unsigned long get_entry_num_slots(efi_memory_desc_t *md,
unsigned long align_shift)
{
unsigned long align = 1UL << align_shift;
- u64 start, end;
+ u64 first_slot, last_slot, region_end;

if (md->type != EFI_CONVENTIONAL_MEMORY)
return 0;

- start = round_up(md->phys_addr, align);
- end = round_down(md->phys_addr + md->num_pages * EFI_PAGE_SIZE - size,
- align);
+ region_end = min((u64)ULONG_MAX, md->phys_addr + md->num_pages*EFI_PAGE_SIZE - 1);

- if (start > end)
+ first_slot = round_up(md->phys_addr, align);
+ last_slot = round_down(region_end - size + 1, align);
+
+ if (first_slot > last_slot)
return 0;

- return (end - start + 1) >> align_shift;
+ return ((unsigned long)(last_slot - first_slot) >> align_shift) + 1;
}

/*

2016-12-04 14:17:50

by Matt Fleming

[permalink] [raw]
Subject: Re: [tip:efi/core] efi/libstub: Make efi_random_alloc() allocate below 4 GB on 32-bit

On Thu, 24 Nov, at 10:47:15PM, tip-bot for Ard Biesheuvel wrote:
> Commit-ID: 018edcfac4c3b140366ad51b0907f3becb5bb624
> Gitweb: http://git.kernel.org/tip/018edcfac4c3b140366ad51b0907f3becb5bb624
> Author: Ard Biesheuvel <[email protected]>
> AuthorDate: Thu, 24 Nov 2016 18:02:23 +0000
> Committer: Ingo Molnar <[email protected]>
> CommitDate: Fri, 25 Nov 2016 07:15:23 +0100
>
> efi/libstub: Make efi_random_alloc() allocate below 4 GB on 32-bit
>
> The UEFI stub executes in the context of the firmware, which identity
> maps the available system RAM, which implies that only memory below
> 4 GB can be used for allocations on 32-bit architectures, even on [L]PAE
> capable hardware.
>
> So ignore any reported memory above 4 GB in efi_random_alloc(). This
> also fixes a reported build problem on ARM under -Os, where the 64-bit
> logical shift relies on a software routine that the ARM decompressor does
> not provide.
>
> A second [minor] issue is also fixed, where the '+ 1' is moved out of
> the shift, where it belongs: the reason for its presence is that a
> memory region where start == end should count as a single slot, given
> that 'end' takes the desired size and alignment of the allocation into
> account.
>
> To clarify the code in this regard, rename start/end to 'first_slot' and
> 'last_slot', respectively, and introduce 'region_end' to describe the
> last usable address of the current region.
>
> Reported-by: Arnd Bergmann <[email protected]>
> Signed-off-by: Ard Biesheuvel <[email protected]>
> Cc: Linus Torvalds <[email protected]>
> Cc: Matt Fleming <[email protected]>
> Cc: Peter Zijlstra <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Cc: [email protected]
> Link: http://lkml.kernel.org/r/[email protected]
> Signed-off-by: Ingo Molnar <[email protected]>
> ---
> drivers/firmware/efi/libstub/random.c | 13 +++++++------
> 1 file changed, 7 insertions(+), 6 deletions(-)

Ard, was this picked up for the correct tip branch? If it fixes a
build failure it should have gone into tip/efi/urgent, right?

2016-12-04 14:31:40

by Ard Biesheuvel

[permalink] [raw]
Subject: Re: [tip:efi/core] efi/libstub: Make efi_random_alloc() allocate below 4 GB on 32-bit

On 4 December 2016 at 14:17, Matt Fleming <[email protected]> wrote:
> On Thu, 24 Nov, at 10:47:15PM, tip-bot for Ard Biesheuvel wrote:
>> Commit-ID: 018edcfac4c3b140366ad51b0907f3becb5bb624
>> Gitweb: http://git.kernel.org/tip/018edcfac4c3b140366ad51b0907f3becb5bb624
>> Author: Ard Biesheuvel <[email protected]>
>> AuthorDate: Thu, 24 Nov 2016 18:02:23 +0000
>> Committer: Ingo Molnar <[email protected]>
>> CommitDate: Fri, 25 Nov 2016 07:15:23 +0100
>>
>> efi/libstub: Make efi_random_alloc() allocate below 4 GB on 32-bit
>>
>> The UEFI stub executes in the context of the firmware, which identity
>> maps the available system RAM, which implies that only memory below
>> 4 GB can be used for allocations on 32-bit architectures, even on [L]PAE
>> capable hardware.
>>
>> So ignore any reported memory above 4 GB in efi_random_alloc(). This
>> also fixes a reported build problem on ARM under -Os, where the 64-bit
>> logical shift relies on a software routine that the ARM decompressor does
>> not provide.
>>
>> A second [minor] issue is also fixed, where the '+ 1' is moved out of
>> the shift, where it belongs: the reason for its presence is that a
>> memory region where start == end should count as a single slot, given
>> that 'end' takes the desired size and alignment of the allocation into
>> account.
>>
>> To clarify the code in this regard, rename start/end to 'first_slot' and
>> 'last_slot', respectively, and introduce 'region_end' to describe the
>> last usable address of the current region.
>>
>> Reported-by: Arnd Bergmann <[email protected]>
>> Signed-off-by: Ard Biesheuvel <[email protected]>
>> Cc: Linus Torvalds <[email protected]>
>> Cc: Matt Fleming <[email protected]>
>> Cc: Peter Zijlstra <[email protected]>
>> Cc: Thomas Gleixner <[email protected]>
>> Cc: [email protected]
>> Link: http://lkml.kernel.org/r/[email protected]
>> Signed-off-by: Ingo Molnar <[email protected]>
>> ---
>> drivers/firmware/efi/libstub/random.c | 13 +++++++------
>> 1 file changed, 7 insertions(+), 6 deletions(-)
>
> Ard, was this picked up for the correct tip branch? If it fixes a
> build failure it should have gone into tip/efi/urgent, right?

The failure was in -next, with a patch queued up for v4.10, so that is
where the fix went as well.

2016-12-04 21:38:41

by Matt Fleming

[permalink] [raw]
Subject: Re: [tip:efi/core] efi/libstub: Make efi_random_alloc() allocate below 4 GB on 32-bit

On Sun, 04 Dec, at 02:31:23PM, Ard Biesheuvel wrote:
> On 4 December 2016 at 14:17, Matt Fleming <[email protected]> wrote:
> >
> > Ard, was this picked up for the correct tip branch? If it fixes a
> > build failure it should have gone into tip/efi/urgent, right?
>
> The failure was in -next, with a patch queued up for v4.10, so that is
> where the fix went as well.

Oops, sorry I missed that. I was looking at the v4.11 queue and my
brain suffered an off-by-one bug - I thought we'd already had the
v4.10 release.

2016-12-05 09:01:07

by Ard Biesheuvel

[permalink] [raw]
Subject: Re: [tip:efi/core] efi/libstub: Make efi_random_alloc() allocate below 4 GB on 32-bit

On 4 December 2016 at 21:38, Matt Fleming <[email protected]> wrote:
> On Sun, 04 Dec, at 02:31:23PM, Ard Biesheuvel wrote:
>> On 4 December 2016 at 14:17, Matt Fleming <[email protected]> wrote:
>> >
>> > Ard, was this picked up for the correct tip branch? If it fixes a
>> > build failure it should have gone into tip/efi/urgent, right?
>>
>> The failure was in -next, with a patch queued up for v4.10, so that is
>> where the fix went as well.
>
> Oops, sorry I missed that. I was looking at the v4.11 queue and my
> brain suffered an off-by-one bug - I thought we'd already had the
> v4.10 release.

We haven't even had the v4.9 release :-)