All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] efi/arm64: don't apply MEMBLOCK_NOMAP to UEFI memory map mapping
@ 2016-03-30  7:46 ` Ard Biesheuvel
  0 siblings, 0 replies; 16+ messages in thread
From: Ard Biesheuvel @ 2016-03-30  7:46 UTC (permalink / raw)
  To: linux-efi-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	matt-mF/unelCI9GS6iBeEJttW/XRex20P6io
  Cc: mark.rutland-5wv7dgnIgG8, msalter-H+wXaHxf7aLQT0dZR+AlfA,
	mlangsdo-H+wXaHxf7aLQT0dZR+AlfA, jeremy.linton-5wv7dgnIgG8,
	leif.lindholm-QSEj5FYQhm4dnm+yROfE0A, Ard Biesheuvel

Hi Matt,

Could we queue this as a fix for v4.6 with a cc:stable for v4.5, please?
(assuming no objections from any of the cc'ees)

Thanks,
Ard.

----------8<--------------
Commit 4dffbfc48d65 ("arm64/efi: mark UEFI reserved regions as
MEMBLOCK_NOMAP") updated the mapping logic of both the RuntimeServices
regions as well as the kernel's copy of the UEFI memory map to set the
MEMBLOCK_NOMAP flag, which causes these regions to be omitted from the
kernel direct mapping, and from being covered by a struct page.
For the RuntimeServices regions, this is an obvious win, since the contents
of these regions have significance to the firmware executable code itself,
and are mapped in the EFI page tables using attributes that are described in
the UEFI memory map, and which may differ from the attributes we use for
mapping system RAM. It also prevents the contents from being modified
inadvertently, since the EFI page tables are only live during runtime
service invocations.

None of these concerns apply to the allocation that covers the UEFI memory
map, since it is entirely owned by the kernel. Setting the MEMBLOCK_NOMAP on
the region did allow us to use ioremap_cache() to map it both on arm64 and
on ARM, since the latter does not allow ioremap_cache() to be used on
regions that are covered by a struct page.

The ioremap_cache() on ARM restriction will be lifted in the v4.7 timeframe,
but in the mean time, it has been reported that commit 4dffbfc48d65 causes
a regression on 64k granule kernels. This is due to the fact that, given
the 64 KB page size, the region that we end up removing from the kernel
direct mapping is rounded up to 64 KB, and this 64 KB page frame may be
shared with the initrd when booting via GRUB (which does not align its
EFI_LOADER_DATA allocations to 64 KB like the stub does). This will crash
the kernel as soon as it tries to access the initrd.

Since the issue is specific to arm64, revert back to memblock_reserve()'ing
the UEFI memory map when running on arm64. This is a temporary fix for v4.5
and v4.6, and will be superseded in the v4.7 timeframe when we will be able
to move back to memblock_reserve() unconditionally.

Fixes: 4dffbfc48d65 ("arm64/efi: mark UEFI reserved regions as MEMBLOCK_NOMAP")
Reported-by: Mark Salter <msalter-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 drivers/firmware/efi/arm-init.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/drivers/firmware/efi/arm-init.c b/drivers/firmware/efi/arm-init.c
index aa1f743152a2..8714f8c271ba 100644
--- a/drivers/firmware/efi/arm-init.c
+++ b/drivers/firmware/efi/arm-init.c
@@ -203,7 +203,19 @@ void __init efi_init(void)
 
 	reserve_regions();
 	early_memunmap(memmap.map, params.mmap_size);
-	memblock_mark_nomap(params.mmap & PAGE_MASK,
-			    PAGE_ALIGN(params.mmap_size +
-				       (params.mmap & ~PAGE_MASK)));
+
+	if (IS_ENABLED(CONFIG_ARM)) {
+		/*
+		 * ARM currently does not allow ioremap_cache() to be called on
+		 * memory regions that are covered by struct page. So remove the
+		 * UEFI memory map from the linear mapping.
+		 */
+		memblock_mark_nomap(params.mmap & PAGE_MASK,
+				    PAGE_ALIGN(params.mmap_size +
+					       (params.mmap & ~PAGE_MASK)));
+	} else {
+		memblock_reserve(params.mmap & PAGE_MASK,
+				 PAGE_ALIGN(params.mmap_size +
+					    (params.mmap & ~PAGE_MASK)));
+	}
 }
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH] efi/arm64: don't apply MEMBLOCK_NOMAP to UEFI memory map mapping
@ 2016-03-30  7:46 ` Ard Biesheuvel
  0 siblings, 0 replies; 16+ messages in thread
From: Ard Biesheuvel @ 2016-03-30  7:46 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Matt,

Could we queue this as a fix for v4.6 with a cc:stable for v4.5, please?
(assuming no objections from any of the cc'ees)

Thanks,
Ard.

----------8<--------------
Commit 4dffbfc48d65 ("arm64/efi: mark UEFI reserved regions as
MEMBLOCK_NOMAP") updated the mapping logic of both the RuntimeServices
regions as well as the kernel's copy of the UEFI memory map to set the
MEMBLOCK_NOMAP flag, which causes these regions to be omitted from the
kernel direct mapping, and from being covered by a struct page.
For the RuntimeServices regions, this is an obvious win, since the contents
of these regions have significance to the firmware executable code itself,
and are mapped in the EFI page tables using attributes that are described in
the UEFI memory map, and which may differ from the attributes we use for
mapping system RAM. It also prevents the contents from being modified
inadvertently, since the EFI page tables are only live during runtime
service invocations.

None of these concerns apply to the allocation that covers the UEFI memory
map, since it is entirely owned by the kernel. Setting the MEMBLOCK_NOMAP on
the region did allow us to use ioremap_cache() to map it both on arm64 and
on ARM, since the latter does not allow ioremap_cache() to be used on
regions that are covered by a struct page.

The ioremap_cache() on ARM restriction will be lifted in the v4.7 timeframe,
but in the mean time, it has been reported that commit 4dffbfc48d65 causes
a regression on 64k granule kernels. This is due to the fact that, given
the 64 KB page size, the region that we end up removing from the kernel
direct mapping is rounded up to 64 KB, and this 64 KB page frame may be
shared with the initrd when booting via GRUB (which does not align its
EFI_LOADER_DATA allocations to 64 KB like the stub does). This will crash
the kernel as soon as it tries to access the initrd.

Since the issue is specific to arm64, revert back to memblock_reserve()'ing
the UEFI memory map when running on arm64. This is a temporary fix for v4.5
and v4.6, and will be superseded in the v4.7 timeframe when we will be able
to move back to memblock_reserve() unconditionally.

Fixes: 4dffbfc48d65 ("arm64/efi: mark UEFI reserved regions as MEMBLOCK_NOMAP")
Reported-by: Mark Salter <msalter@redhat.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 drivers/firmware/efi/arm-init.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/drivers/firmware/efi/arm-init.c b/drivers/firmware/efi/arm-init.c
index aa1f743152a2..8714f8c271ba 100644
--- a/drivers/firmware/efi/arm-init.c
+++ b/drivers/firmware/efi/arm-init.c
@@ -203,7 +203,19 @@ void __init efi_init(void)
 
 	reserve_regions();
 	early_memunmap(memmap.map, params.mmap_size);
-	memblock_mark_nomap(params.mmap & PAGE_MASK,
-			    PAGE_ALIGN(params.mmap_size +
-				       (params.mmap & ~PAGE_MASK)));
+
+	if (IS_ENABLED(CONFIG_ARM)) {
+		/*
+		 * ARM currently does not allow ioremap_cache() to be called on
+		 * memory regions that are covered by struct page. So remove the
+		 * UEFI memory map from the linear mapping.
+		 */
+		memblock_mark_nomap(params.mmap & PAGE_MASK,
+				    PAGE_ALIGN(params.mmap_size +
+					       (params.mmap & ~PAGE_MASK)));
+	} else {
+		memblock_reserve(params.mmap & PAGE_MASK,
+				 PAGE_ALIGN(params.mmap_size +
+					    (params.mmap & ~PAGE_MASK)));
+	}
 }
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH] efi/arm64: don't apply MEMBLOCK_NOMAP to UEFI memory map mapping
  2016-03-30  7:46 ` Ard Biesheuvel
@ 2016-03-31 10:53     ` Will Deacon
  -1 siblings, 0 replies; 16+ messages in thread
From: Will Deacon @ 2016-03-31 10:53 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: linux-efi-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	matt-mF/unelCI9GS6iBeEJttW/XRex20P6io, mark.rutland-5wv7dgnIgG8,
	mlangsdo-H+wXaHxf7aLQT0dZR+AlfA,
	leif.lindholm-QSEj5FYQhm4dnm+yROfE0A, jeremy.linton-5wv7dgnIgG8,
	msalter-H+wXaHxf7aLQT0dZR+AlfA

On Wed, Mar 30, 2016 at 09:46:23AM +0200, Ard Biesheuvel wrote:
> Hi Matt,
> 
> Could we queue this as a fix for v4.6 with a cc:stable for v4.5, please?
> (assuming no objections from any of the cc'ees)

FWIW:

Acked-by: Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>

Yes, this is a temporary hack, but it's small, fixes a real issue and
you're already working on a proper solution anyway.

Will

> ----------8<--------------
> Commit 4dffbfc48d65 ("arm64/efi: mark UEFI reserved regions as
> MEMBLOCK_NOMAP") updated the mapping logic of both the RuntimeServices
> regions as well as the kernel's copy of the UEFI memory map to set the
> MEMBLOCK_NOMAP flag, which causes these regions to be omitted from the
> kernel direct mapping, and from being covered by a struct page.
> For the RuntimeServices regions, this is an obvious win, since the contents
> of these regions have significance to the firmware executable code itself,
> and are mapped in the EFI page tables using attributes that are described in
> the UEFI memory map, and which may differ from the attributes we use for
> mapping system RAM. It also prevents the contents from being modified
> inadvertently, since the EFI page tables are only live during runtime
> service invocations.
> 
> None of these concerns apply to the allocation that covers the UEFI memory
> map, since it is entirely owned by the kernel. Setting the MEMBLOCK_NOMAP on
> the region did allow us to use ioremap_cache() to map it both on arm64 and
> on ARM, since the latter does not allow ioremap_cache() to be used on
> regions that are covered by a struct page.
> 
> The ioremap_cache() on ARM restriction will be lifted in the v4.7 timeframe,
> but in the mean time, it has been reported that commit 4dffbfc48d65 causes
> a regression on 64k granule kernels. This is due to the fact that, given
> the 64 KB page size, the region that we end up removing from the kernel
> direct mapping is rounded up to 64 KB, and this 64 KB page frame may be
> shared with the initrd when booting via GRUB (which does not align its
> EFI_LOADER_DATA allocations to 64 KB like the stub does). This will crash
> the kernel as soon as it tries to access the initrd.
> 
> Since the issue is specific to arm64, revert back to memblock_reserve()'ing
> the UEFI memory map when running on arm64. This is a temporary fix for v4.5
> and v4.6, and will be superseded in the v4.7 timeframe when we will be able
> to move back to memblock_reserve() unconditionally.
> 
> Fixes: 4dffbfc48d65 ("arm64/efi: mark UEFI reserved regions as MEMBLOCK_NOMAP")
> Reported-by: Mark Salter <msalter-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
>  drivers/firmware/efi/arm-init.c | 18 +++++++++++++++---
>  1 file changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/firmware/efi/arm-init.c b/drivers/firmware/efi/arm-init.c
> index aa1f743152a2..8714f8c271ba 100644
> --- a/drivers/firmware/efi/arm-init.c
> +++ b/drivers/firmware/efi/arm-init.c
> @@ -203,7 +203,19 @@ void __init efi_init(void)
>  
>  	reserve_regions();
>  	early_memunmap(memmap.map, params.mmap_size);
> -	memblock_mark_nomap(params.mmap & PAGE_MASK,
> -			    PAGE_ALIGN(params.mmap_size +
> -				       (params.mmap & ~PAGE_MASK)));
> +
> +	if (IS_ENABLED(CONFIG_ARM)) {
> +		/*
> +		 * ARM currently does not allow ioremap_cache() to be called on
> +		 * memory regions that are covered by struct page. So remove the
> +		 * UEFI memory map from the linear mapping.
> +		 */
> +		memblock_mark_nomap(params.mmap & PAGE_MASK,
> +				    PAGE_ALIGN(params.mmap_size +
> +					       (params.mmap & ~PAGE_MASK)));
> +	} else {
> +		memblock_reserve(params.mmap & PAGE_MASK,
> +				 PAGE_ALIGN(params.mmap_size +
> +					    (params.mmap & ~PAGE_MASK)));
> +	}
>  }
> -- 
> 2.5.0
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH] efi/arm64: don't apply MEMBLOCK_NOMAP to UEFI memory map mapping
@ 2016-03-31 10:53     ` Will Deacon
  0 siblings, 0 replies; 16+ messages in thread
From: Will Deacon @ 2016-03-31 10:53 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Mar 30, 2016 at 09:46:23AM +0200, Ard Biesheuvel wrote:
> Hi Matt,
> 
> Could we queue this as a fix for v4.6 with a cc:stable for v4.5, please?
> (assuming no objections from any of the cc'ees)

FWIW:

Acked-by: Will Deacon <will.deacon@arm.com>

Yes, this is a temporary hack, but it's small, fixes a real issue and
you're already working on a proper solution anyway.

Will

> ----------8<--------------
> Commit 4dffbfc48d65 ("arm64/efi: mark UEFI reserved regions as
> MEMBLOCK_NOMAP") updated the mapping logic of both the RuntimeServices
> regions as well as the kernel's copy of the UEFI memory map to set the
> MEMBLOCK_NOMAP flag, which causes these regions to be omitted from the
> kernel direct mapping, and from being covered by a struct page.
> For the RuntimeServices regions, this is an obvious win, since the contents
> of these regions have significance to the firmware executable code itself,
> and are mapped in the EFI page tables using attributes that are described in
> the UEFI memory map, and which may differ from the attributes we use for
> mapping system RAM. It also prevents the contents from being modified
> inadvertently, since the EFI page tables are only live during runtime
> service invocations.
> 
> None of these concerns apply to the allocation that covers the UEFI memory
> map, since it is entirely owned by the kernel. Setting the MEMBLOCK_NOMAP on
> the region did allow us to use ioremap_cache() to map it both on arm64 and
> on ARM, since the latter does not allow ioremap_cache() to be used on
> regions that are covered by a struct page.
> 
> The ioremap_cache() on ARM restriction will be lifted in the v4.7 timeframe,
> but in the mean time, it has been reported that commit 4dffbfc48d65 causes
> a regression on 64k granule kernels. This is due to the fact that, given
> the 64 KB page size, the region that we end up removing from the kernel
> direct mapping is rounded up to 64 KB, and this 64 KB page frame may be
> shared with the initrd when booting via GRUB (which does not align its
> EFI_LOADER_DATA allocations to 64 KB like the stub does). This will crash
> the kernel as soon as it tries to access the initrd.
> 
> Since the issue is specific to arm64, revert back to memblock_reserve()'ing
> the UEFI memory map when running on arm64. This is a temporary fix for v4.5
> and v4.6, and will be superseded in the v4.7 timeframe when we will be able
> to move back to memblock_reserve() unconditionally.
> 
> Fixes: 4dffbfc48d65 ("arm64/efi: mark UEFI reserved regions as MEMBLOCK_NOMAP")
> Reported-by: Mark Salter <msalter@redhat.com>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  drivers/firmware/efi/arm-init.c | 18 +++++++++++++++---
>  1 file changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/firmware/efi/arm-init.c b/drivers/firmware/efi/arm-init.c
> index aa1f743152a2..8714f8c271ba 100644
> --- a/drivers/firmware/efi/arm-init.c
> +++ b/drivers/firmware/efi/arm-init.c
> @@ -203,7 +203,19 @@ void __init efi_init(void)
>  
>  	reserve_regions();
>  	early_memunmap(memmap.map, params.mmap_size);
> -	memblock_mark_nomap(params.mmap & PAGE_MASK,
> -			    PAGE_ALIGN(params.mmap_size +
> -				       (params.mmap & ~PAGE_MASK)));
> +
> +	if (IS_ENABLED(CONFIG_ARM)) {
> +		/*
> +		 * ARM currently does not allow ioremap_cache() to be called on
> +		 * memory regions that are covered by struct page. So remove the
> +		 * UEFI memory map from the linear mapping.
> +		 */
> +		memblock_mark_nomap(params.mmap & PAGE_MASK,
> +				    PAGE_ALIGN(params.mmap_size +
> +					       (params.mmap & ~PAGE_MASK)));
> +	} else {
> +		memblock_reserve(params.mmap & PAGE_MASK,
> +				 PAGE_ALIGN(params.mmap_size +
> +					    (params.mmap & ~PAGE_MASK)));
> +	}
>  }
> -- 
> 2.5.0
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] efi/arm64: don't apply MEMBLOCK_NOMAP to UEFI memory map mapping
  2016-03-30  7:46 ` Ard Biesheuvel
@ 2016-03-31 20:35     ` Matt Fleming
  -1 siblings, 0 replies; 16+ messages in thread
From: Matt Fleming @ 2016-03-31 20:35 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: linux-efi-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	mark.rutland-5wv7dgnIgG8, msalter-H+wXaHxf7aLQT0dZR+AlfA,
	mlangsdo-H+wXaHxf7aLQT0dZR+AlfA, jeremy.linton-5wv7dgnIgG8,
	leif.lindholm-QSEj5FYQhm4dnm+yROfE0A

On Wed, 30 Mar, at 09:46:23AM, Ard Biesheuvel wrote:
> Hi Matt,
> 
> Could we queue this as a fix for v4.6 with a cc:stable for v4.5, please?
> (assuming no objections from any of the cc'ees)

Yep, I've picked this up. I'll go out to tip tomorrow.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH] efi/arm64: don't apply MEMBLOCK_NOMAP to UEFI memory map mapping
@ 2016-03-31 20:35     ` Matt Fleming
  0 siblings, 0 replies; 16+ messages in thread
From: Matt Fleming @ 2016-03-31 20:35 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, 30 Mar, at 09:46:23AM, Ard Biesheuvel wrote:
> Hi Matt,
> 
> Could we queue this as a fix for v4.6 with a cc:stable for v4.5, please?
> (assuming no objections from any of the cc'ees)

Yep, I've picked this up. I'll go out to tip tomorrow.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] efi/arm64: don't apply MEMBLOCK_NOMAP to UEFI memory map mapping
  2016-03-31 20:35     ` Matt Fleming
@ 2016-04-15 16:44         ` Steve Capper
  -1 siblings, 0 replies; 16+ messages in thread
From: Steve Capper @ 2016-04-15 16:44 UTC (permalink / raw)
  To: Matt Fleming
  Cc: Ard Biesheuvel, Mark Rutland, Mark Langsdorf, Jeremy Linton,
	linux-efi-u79uwXL29TY76Z2rM5mHXA, Leif Lindholm, Mark Salter,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

On 31 March 2016 at 21:35, Matt Fleming <matt-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org> wrote:
> On Wed, 30 Mar, at 09:46:23AM, Ard Biesheuvel wrote:
>> Hi Matt,
>>
>> Could we queue this as a fix for v4.6 with a cc:stable for v4.5, please?
>> (assuming no objections from any of the cc'ees)
>
> Yep, I've picked this up. I'll go out to tip tomorrow.
>

Hello,
Is this going to make it in for 4.6?
I ran into this issue the other day, and can confirm that Ard's patch
is helpful.

Cheers,
-- 
Steve

> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH] efi/arm64: don't apply MEMBLOCK_NOMAP to UEFI memory map mapping
@ 2016-04-15 16:44         ` Steve Capper
  0 siblings, 0 replies; 16+ messages in thread
From: Steve Capper @ 2016-04-15 16:44 UTC (permalink / raw)
  To: linux-arm-kernel

On 31 March 2016 at 21:35, Matt Fleming <matt@codeblueprint.co.uk> wrote:
> On Wed, 30 Mar, at 09:46:23AM, Ard Biesheuvel wrote:
>> Hi Matt,
>>
>> Could we queue this as a fix for v4.6 with a cc:stable for v4.5, please?
>> (assuming no objections from any of the cc'ees)
>
> Yep, I've picked this up. I'll go out to tip tomorrow.
>

Hello,
Is this going to make it in for 4.6?
I ran into this issue the other day, and can confirm that Ard's patch
is helpful.

Cheers,
-- 
Steve

> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] efi/arm64: don't apply MEMBLOCK_NOMAP to UEFI memory map mapping
  2016-04-15 16:44         ` Steve Capper
@ 2016-04-15 22:15             ` Matt Fleming
  -1 siblings, 0 replies; 16+ messages in thread
From: Matt Fleming @ 2016-04-15 22:15 UTC (permalink / raw)
  To: Steve Capper, Ingo Molnar
  Cc: Ard Biesheuvel, Mark Rutland, Mark Langsdorf, Jeremy Linton,
	linux-efi-u79uwXL29TY76Z2rM5mHXA, Leif Lindholm, Mark Salter,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

On Fri, 15 Apr, at 05:44:02PM, Steve Capper wrote:
> 
> Hello,
> Is this going to make it in for 4.6?
> I ran into this issue the other day, and can confirm that Ard's patch
> is helpful.

Good question.

Ingo did you send the patch in the last EFI urgent pull request to
Linus yet? I can find it in tip/efi-urgent-for-linus but not in Linus'
tree.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH] efi/arm64: don't apply MEMBLOCK_NOMAP to UEFI memory map mapping
@ 2016-04-15 22:15             ` Matt Fleming
  0 siblings, 0 replies; 16+ messages in thread
From: Matt Fleming @ 2016-04-15 22:15 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, 15 Apr, at 05:44:02PM, Steve Capper wrote:
> 
> Hello,
> Is this going to make it in for 4.6?
> I ran into this issue the other day, and can confirm that Ard's patch
> is helpful.

Good question.

Ingo did you send the patch in the last EFI urgent pull request to
Linus yet? I can find it in tip/efi-urgent-for-linus but not in Linus'
tree.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] efi/arm64: don't apply MEMBLOCK_NOMAP to UEFI memory map mapping
  2016-04-15 22:15             ` Matt Fleming
@ 2016-04-16  9:09                 ` Ingo Molnar
  -1 siblings, 0 replies; 16+ messages in thread
From: Ingo Molnar @ 2016-04-16  9:09 UTC (permalink / raw)
  To: Matt Fleming
  Cc: Steve Capper, Ard Biesheuvel, Mark Rutland, Mark Langsdorf,
	Jeremy Linton, linux-efi-u79uwXL29TY76Z2rM5mHXA, Leif Lindholm,
	Mark Salter, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r


* Matt Fleming <matt-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org> wrote:

> On Fri, 15 Apr, at 05:44:02PM, Steve Capper wrote:
> > 
> > Hello,
> > Is this going to make it in for 4.6?
> > I ran into this issue the other day, and can confirm that Ard's patch
> > is helpful.
> 
> Good question.
> 
> Ingo did you send the patch in the last EFI urgent pull request to
> Linus yet? I can find it in tip/efi-urgent-for-linus but not in Linus'
> tree.

I just sent it to Linus.

It got delayed a bit because it had an x86/urgent dependency that had to be sent 
upstream first. (In the future you can avoid such delays by basing EFI urgent 
fixes on an upstream -rc kernel, not x86/urgent.)

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH] efi/arm64: don't apply MEMBLOCK_NOMAP to UEFI memory map mapping
@ 2016-04-16  9:09                 ` Ingo Molnar
  0 siblings, 0 replies; 16+ messages in thread
From: Ingo Molnar @ 2016-04-16  9:09 UTC (permalink / raw)
  To: linux-arm-kernel


* Matt Fleming <matt@codeblueprint.co.uk> wrote:

> On Fri, 15 Apr, at 05:44:02PM, Steve Capper wrote:
> > 
> > Hello,
> > Is this going to make it in for 4.6?
> > I ran into this issue the other day, and can confirm that Ard's patch
> > is helpful.
> 
> Good question.
> 
> Ingo did you send the patch in the last EFI urgent pull request to
> Linus yet? I can find it in tip/efi-urgent-for-linus but not in Linus'
> tree.

I just sent it to Linus.

It got delayed a bit because it had an x86/urgent dependency that had to be sent 
upstream first. (In the future you can avoid such delays by basing EFI urgent 
fixes on an upstream -rc kernel, not x86/urgent.)

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] efi/arm64: don't apply MEMBLOCK_NOMAP to UEFI memory map mapping
  2016-04-16  9:09                 ` Ingo Molnar
@ 2016-04-16 10:27                     ` Matt Fleming
  -1 siblings, 0 replies; 16+ messages in thread
From: Matt Fleming @ 2016-04-16 10:27 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Steve Capper, Ard Biesheuvel, Mark Rutland, Mark Langsdorf,
	Jeremy Linton, linux-efi-u79uwXL29TY76Z2rM5mHXA, Leif Lindholm,
	Mark Salter, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

On Sat, 16 Apr, at 11:09:10AM, Ingo Molnar wrote:
> 
> I just sent it to Linus.
> 
> It got delayed a bit because it had an x86/urgent dependency that had to be sent 
> upstream first. (In the future you can avoid such delays by basing EFI urgent 
> fixes on an upstream -rc kernel, not x86/urgent.)

Noted. I'll do this in future, thanks Ingo.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH] efi/arm64: don't apply MEMBLOCK_NOMAP to UEFI memory map mapping
@ 2016-04-16 10:27                     ` Matt Fleming
  0 siblings, 0 replies; 16+ messages in thread
From: Matt Fleming @ 2016-04-16 10:27 UTC (permalink / raw)
  To: linux-arm-kernel

On Sat, 16 Apr, at 11:09:10AM, Ingo Molnar wrote:
> 
> I just sent it to Linus.
> 
> It got delayed a bit because it had an x86/urgent dependency that had to be sent 
> upstream first. (In the future you can avoid such delays by basing EFI urgent 
> fixes on an upstream -rc kernel, not x86/urgent.)

Noted. I'll do this in future, thanks Ingo.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH] efi/arm64: Don't apply MEMBLOCK_NOMAP to UEFI memory map mapping
@ 2016-04-01 19:10   ` Matt Fleming
  0 siblings, 0 replies; 16+ messages in thread
From: Matt Fleming @ 2016-04-01 19:10 UTC (permalink / raw)
  To: Ingo Molnar, Thomas Gleixner, H . Peter Anvin
  Cc: Ard Biesheuvel, linux-kernel, linux-efi, Matt Fleming,
	Jeremy Linton, Leif Lindholm, Mark Langsdorf, Mark Rutland,
	Mark Salter, stable, Will Deacon

From: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Commit 4dffbfc48d65 ("arm64/efi: mark UEFI reserved regions as
MEMBLOCK_NOMAP") updated the mapping logic of both the RuntimeServices
regions as well as the kernel's copy of the UEFI memory map to set the
MEMBLOCK_NOMAP flag, which causes these regions to be omitted from the
kernel direct mapping, and from being covered by a struct page.
For the RuntimeServices regions, this is an obvious win, since the contents
of these regions have significance to the firmware executable code itself,
and are mapped in the EFI page tables using attributes that are described in
the UEFI memory map, and which may differ from the attributes we use for
mapping system RAM. It also prevents the contents from being modified
inadvertently, since the EFI page tables are only live during runtime
service invocations.

None of these concerns apply to the allocation that covers the UEFI memory
map, since it is entirely owned by the kernel. Setting the MEMBLOCK_NOMAP on
the region did allow us to use ioremap_cache() to map it both on arm64 and
on ARM, since the latter does not allow ioremap_cache() to be used on
regions that are covered by a struct page.

The ioremap_cache() on ARM restriction will be lifted in the v4.7 timeframe,
but in the mean time, it has been reported that commit 4dffbfc48d65 causes
a regression on 64k granule kernels. This is due to the fact that, given
the 64 KB page size, the region that we end up removing from the kernel
direct mapping is rounded up to 64 KB, and this 64 KB page frame may be
shared with the initrd when booting via GRUB (which does not align its
EFI_LOADER_DATA allocations to 64 KB like the stub does). This will crash
the kernel as soon as it tries to access the initrd.

Since the issue is specific to arm64, revert back to memblock_reserve()'ing
the UEFI memory map when running on arm64. This is a temporary fix for v4.5
and v4.6, and will be superseded in the v4.7 timeframe when we will be able
to move back to memblock_reserve() unconditionally.

Fixes: 4dffbfc48d65 ("arm64/efi: mark UEFI reserved regions as MEMBLOCK_NOMAP")
Reported-by: Mark Salter <msalter@redhat.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Acked-by: Will Deacon <will.deacon@arm.com>
Cc: Leif Lindholm <leif.lindholm@linaro.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Jeremy Linton <jeremy.linton@arm.com>
Cc: Mark Langsdorf <mlangsdo@redhat.com>
Cc: <stable@vger.kernel.org> # v4.5
Signed-off-by: Matt Fleming <matt@codeblueprint.co.uk>
---
 drivers/firmware/efi/arm-init.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/drivers/firmware/efi/arm-init.c b/drivers/firmware/efi/arm-init.c
index aa1f743152a2..8714f8c271ba 100644
--- a/drivers/firmware/efi/arm-init.c
+++ b/drivers/firmware/efi/arm-init.c
@@ -203,7 +203,19 @@ void __init efi_init(void)
 
 	reserve_regions();
 	early_memunmap(memmap.map, params.mmap_size);
-	memblock_mark_nomap(params.mmap & PAGE_MASK,
-			    PAGE_ALIGN(params.mmap_size +
-				       (params.mmap & ~PAGE_MASK)));
+
+	if (IS_ENABLED(CONFIG_ARM)) {
+		/*
+		 * ARM currently does not allow ioremap_cache() to be called on
+		 * memory regions that are covered by struct page. So remove the
+		 * UEFI memory map from the linear mapping.
+		 */
+		memblock_mark_nomap(params.mmap & PAGE_MASK,
+				    PAGE_ALIGN(params.mmap_size +
+					       (params.mmap & ~PAGE_MASK)));
+	} else {
+		memblock_reserve(params.mmap & PAGE_MASK,
+				 PAGE_ALIGN(params.mmap_size +
+					    (params.mmap & ~PAGE_MASK)));
+	}
 }
-- 
2.7.3

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH] efi/arm64: Don't apply MEMBLOCK_NOMAP to UEFI memory map mapping
@ 2016-04-01 19:10   ` Matt Fleming
  0 siblings, 0 replies; 16+ messages in thread
From: Matt Fleming @ 2016-04-01 19:10 UTC (permalink / raw)
  To: Ingo Molnar, Thomas Gleixner, H . Peter Anvin
  Cc: Ard Biesheuvel, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-efi-u79uwXL29TY76Z2rM5mHXA, Matt Fleming, Jeremy Linton,
	Leif Lindholm, Mark Langsdorf, Mark Rutland, Mark Salter,
	stable-u79uwXL29TY76Z2rM5mHXA, Will Deacon

From: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

Commit 4dffbfc48d65 ("arm64/efi: mark UEFI reserved regions as
MEMBLOCK_NOMAP") updated the mapping logic of both the RuntimeServices
regions as well as the kernel's copy of the UEFI memory map to set the
MEMBLOCK_NOMAP flag, which causes these regions to be omitted from the
kernel direct mapping, and from being covered by a struct page.
For the RuntimeServices regions, this is an obvious win, since the contents
of these regions have significance to the firmware executable code itself,
and are mapped in the EFI page tables using attributes that are described in
the UEFI memory map, and which may differ from the attributes we use for
mapping system RAM. It also prevents the contents from being modified
inadvertently, since the EFI page tables are only live during runtime
service invocations.

None of these concerns apply to the allocation that covers the UEFI memory
map, since it is entirely owned by the kernel. Setting the MEMBLOCK_NOMAP on
the region did allow us to use ioremap_cache() to map it both on arm64 and
on ARM, since the latter does not allow ioremap_cache() to be used on
regions that are covered by a struct page.

The ioremap_cache() on ARM restriction will be lifted in the v4.7 timeframe,
but in the mean time, it has been reported that commit 4dffbfc48d65 causes
a regression on 64k granule kernels. This is due to the fact that, given
the 64 KB page size, the region that we end up removing from the kernel
direct mapping is rounded up to 64 KB, and this 64 KB page frame may be
shared with the initrd when booting via GRUB (which does not align its
EFI_LOADER_DATA allocations to 64 KB like the stub does). This will crash
the kernel as soon as it tries to access the initrd.

Since the issue is specific to arm64, revert back to memblock_reserve()'ing
the UEFI memory map when running on arm64. This is a temporary fix for v4.5
and v4.6, and will be superseded in the v4.7 timeframe when we will be able
to move back to memblock_reserve() unconditionally.

Fixes: 4dffbfc48d65 ("arm64/efi: mark UEFI reserved regions as MEMBLOCK_NOMAP")
Reported-by: Mark Salter <msalter-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Acked-by: Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>
Cc: Leif Lindholm <leif.lindholm-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
Cc: Jeremy Linton <jeremy.linton-5wv7dgnIgG8@public.gmane.org>
Cc: Mark Langsdorf <mlangsdo-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Cc: <stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org> # v4.5
Signed-off-by: Matt Fleming <matt-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org>
---
 drivers/firmware/efi/arm-init.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/drivers/firmware/efi/arm-init.c b/drivers/firmware/efi/arm-init.c
index aa1f743152a2..8714f8c271ba 100644
--- a/drivers/firmware/efi/arm-init.c
+++ b/drivers/firmware/efi/arm-init.c
@@ -203,7 +203,19 @@ void __init efi_init(void)
 
 	reserve_regions();
 	early_memunmap(memmap.map, params.mmap_size);
-	memblock_mark_nomap(params.mmap & PAGE_MASK,
-			    PAGE_ALIGN(params.mmap_size +
-				       (params.mmap & ~PAGE_MASK)));
+
+	if (IS_ENABLED(CONFIG_ARM)) {
+		/*
+		 * ARM currently does not allow ioremap_cache() to be called on
+		 * memory regions that are covered by struct page. So remove the
+		 * UEFI memory map from the linear mapping.
+		 */
+		memblock_mark_nomap(params.mmap & PAGE_MASK,
+				    PAGE_ALIGN(params.mmap_size +
+					       (params.mmap & ~PAGE_MASK)));
+	} else {
+		memblock_reserve(params.mmap & PAGE_MASK,
+				 PAGE_ALIGN(params.mmap_size +
+					    (params.mmap & ~PAGE_MASK)));
+	}
 }
-- 
2.7.3

^ permalink raw reply related	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2016-04-16 10:27 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-03-30  7:46 [PATCH] efi/arm64: don't apply MEMBLOCK_NOMAP to UEFI memory map mapping Ard Biesheuvel
2016-03-30  7:46 ` Ard Biesheuvel
     [not found] ` <1459323983-9120-1-git-send-email-ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
2016-03-31 10:53   ` Will Deacon
2016-03-31 10:53     ` Will Deacon
2016-03-31 20:35   ` Matt Fleming
2016-03-31 20:35     ` Matt Fleming
     [not found]     ` <20160331203520.GB2736-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org>
2016-04-15 16:44       ` Steve Capper
2016-04-15 16:44         ` Steve Capper
     [not found]         ` <CAPvkgC0Dy1c78LOR53PgyTdDom=fWF9RurJHQ02RuMVj5Pch3g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2016-04-15 22:15           ` Matt Fleming
2016-04-15 22:15             ` Matt Fleming
     [not found]             ` <20160415221516.GU2829-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org>
2016-04-16  9:09               ` Ingo Molnar
2016-04-16  9:09                 ` Ingo Molnar
     [not found]                 ` <20160416090910.GA30739-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2016-04-16 10:27                   ` Matt Fleming
2016-04-16 10:27                     ` Matt Fleming
2016-04-01 19:03 [GIT PULL] EFI urgent fix Matt Fleming
2016-04-01 19:10 ` [PATCH] efi/arm64: Don't apply MEMBLOCK_NOMAP to UEFI memory map mapping Matt Fleming
2016-04-01 19:10   ` Matt Fleming

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.