[PATCH v1 RESEND 0/2] x86/mm/KASLR: EFI region is mistakenly included into KASLR VA space for randomization

[PATCH v1 RESEND 0/2] x86/mm/KASLR: EFI region is mistakenly included into KASLR VA space for randomization

[Baoquan He]

Now EFI region is mistakenly counted into KASLR VA space for randomization
because of misusing EFI_VA_START macro and assuming EFI_VA_START < EFI_VA_END.
In fact EFI region reserved for runtime services virtual mapping will be
allocated using a top-down schema. It will be reused by kexec/kdump kernel.

So the mistake will cause failure because vmemmap may be randomized to own
EFI region and stomped on the EFI virtual mapping. It's need be fixed.

The original post can be found in below link. And this repost just updated
patch log, no new code change. The patch 1/2 need be added to stabe kernel
after 4.8+.

https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1347835.html

Baoquan He (2):
  x86/mm/KASLR: EFI region is mistakenly included into KASLR VA space
    for randomization
  x86/efi: Clean up a minor mistake in code comment

 arch/x86/mm/kaslr.c            | 4 ++--
 arch/x86/platform/efi/efi_64.c | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

-- 
2.5.5

[PATCH v1 RESEND 1/2] x86/mm/KASLR: EFI region is mistakenly included into KASLR VA space for randomization

[Baoquan He]

Currently KASLR is enabled on three regions: the direct mapping of physical
memory, vamlloc and vmemmap. However EFI region is also mistakenly included
for VA space randomization because of misusing EFI_VA_START macro and
assuming EFI_VA_START < EFI_VA_END.

The EFI region is reserved for EFI runtime services virtual mapping which
should not be included in kaslr ranges. It will be re-used by kexec/kdump
kernel, the mistake may cause failure when jump to kexec/kdump kernel if
vmemmap allocation stomps on the allocated efi mapping region.

In Documentation/x86/x86_64/mm.txt, we can see:
  ffffffef00000000 - fffffffeffffffff (=64 GB) EFI region mapping space
EFI use the space from -4G to -64G thus EFI_VA_START > EFI_VA_END
Here EFI_VA_START = -4G, and EFI_VA_END = -64G

Changing EFI_VA_START to EFI_VA_END in mm/kaslr.c fixes this problem.

Cc: <stable@vger.kernel.org> #4.8+
Signed-off-by: Baoquan He <bhe@redhat.com>
Acked-by: Dave Young <dyoung@redhat.com>
Reviewed-by: Bhupesh Sharma <bhsharma@redhat.com>
Acked-by: Thomas Garnier <thgarnie@google.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com> 
Cc: x86@kernel.org
Cc: Thomas Garnier <thgarnie@google.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
---
 arch/x86/mm/kaslr.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/mm/kaslr.c b/arch/x86/mm/kaslr.c
index 887e571..aed2064 100644
--- a/arch/x86/mm/kaslr.c
+++ b/arch/x86/mm/kaslr.c
@@ -48,7 +48,7 @@ static const unsigned long vaddr_start = __PAGE_OFFSET_BASE;
 #if defined(CONFIG_X86_ESPFIX64)
 static const unsigned long vaddr_end = ESPFIX_BASE_ADDR;
 #elif defined(CONFIG_EFI)
-static const unsigned long vaddr_end = EFI_VA_START;
+static const unsigned long vaddr_end = EFI_VA_END;
 #else
 static const unsigned long vaddr_end = __START_KERNEL_map;
 #endif
@@ -105,7 +105,7 @@ void __init kernel_randomize_memory(void)
 	 */
 	BUILD_BUG_ON(vaddr_start >= vaddr_end);
 	BUILD_BUG_ON(IS_ENABLED(CONFIG_X86_ESPFIX64) &&
-		     vaddr_end >= EFI_VA_START);
+		     vaddr_end >= EFI_VA_END);
 	BUILD_BUG_ON((IS_ENABLED(CONFIG_X86_ESPFIX64) ||
 		      IS_ENABLED(CONFIG_EFI)) &&
 		     vaddr_end >= __START_KERNEL_map);
-- 
2.5.5

[PATCH v1 RESEND 2/2] x86/efi: Clean up a minor mistake in code comment

[Baoquan He]

EFI allocate runtime services regions from EFI_VA_START, -4G, down
to -64G, EFI_VA_END. The mechanism was introduced in
commit d2f7cbe7b26a7 ("x86/efi: Runtime services virtual mapping").

Clean it up to avoid confusion.

Signed-off-by: Baoquan He <bhe@redhat.com>
Cc: Matt Fleming <matt@codeblueprint.co.uk>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: x86@kernel.org
Cc: linux-efi@vger.kernel.org
---
 arch/x86/platform/efi/efi_64.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/platform/efi/efi_64.c b/arch/x86/platform/efi/efi_64.c
index a4695da..6cbf9e0 100644
--- a/arch/x86/platform/efi/efi_64.c
+++ b/arch/x86/platform/efi/efi_64.c
@@ -47,7 +47,7 @@
 #include <asm/pgalloc.h>
 
 /*
- * We allocate runtime services regions bottom-up, starting from -4G, i.e.
+ * We allocate runtime services regions top-down, starting from -4G, i.e.
  * 0xffff_ffff_0000_0000 and limit EFI VA mapping space to 64G.
  */
 static u64 efi_va = EFI_VA_START;
-- 
2.5.5

Re: [PATCH v1 RESEND 1/2] x86/mm/KASLR: EFI region is mistakenly included into KASLR VA space for randomization

[Baoquan He]

Hi,

Ping!

Since our kaslr mm back port has been held to wait for this upstream
fix, and post deadline is very close, can this patch be merged or picked
up into tip-bot treeu?

This is an obvious code bug, it has risk to cause kexec/kdump boot
failure, and the current code change won't bring other risk.

Thanks
Baoquan

On 03/23/17 at 11:27am, Baoquan He wrote:
> Currently KASLR is enabled on three regions: the direct mapping of physical
> memory, vamlloc and vmemmap. However EFI region is also mistakenly included
> for VA space randomization because of misusing EFI_VA_START macro and
> assuming EFI_VA_START < EFI_VA_END.
> 
> The EFI region is reserved for EFI runtime services virtual mapping which
> should not be included in kaslr ranges. It will be re-used by kexec/kdump
> kernel, the mistake may cause failure when jump to kexec/kdump kernel if
> vmemmap allocation stomps on the allocated efi mapping region.
> 
> In Documentation/x86/x86_64/mm.txt, we can see:
>   ffffffef00000000 - fffffffeffffffff (=64 GB) EFI region mapping space
> EFI use the space from -4G to -64G thus EFI_VA_START > EFI_VA_END
> Here EFI_VA_START = -4G, and EFI_VA_END = -64G
> 
> Changing EFI_VA_START to EFI_VA_END in mm/kaslr.c fixes this problem.
> 
> Cc: <stable@vger.kernel.org> #4.8+
> Signed-off-by: Baoquan He <bhe@redhat.com>
> Acked-by: Dave Young <dyoung@redhat.com>
> Reviewed-by: Bhupesh Sharma <bhsharma@redhat.com>
> Acked-by: Thomas Garnier <thgarnie@google.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: "H. Peter Anvin" <hpa@zytor.com> 
> Cc: x86@kernel.org
> Cc: Thomas Garnier <thgarnie@google.com>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
>  arch/x86/mm/kaslr.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/mm/kaslr.c b/arch/x86/mm/kaslr.c
> index 887e571..aed2064 100644
> --- a/arch/x86/mm/kaslr.c
> +++ b/arch/x86/mm/kaslr.c
> @@ -48,7 +48,7 @@ static const unsigned long vaddr_start = __PAGE_OFFSET_BASE;
>  #if defined(CONFIG_X86_ESPFIX64)
>  static const unsigned long vaddr_end = ESPFIX_BASE_ADDR;
>  #elif defined(CONFIG_EFI)
> -static const unsigned long vaddr_end = EFI_VA_START;
> +static const unsigned long vaddr_end = EFI_VA_END;
>  #else
>  static const unsigned long vaddr_end = __START_KERNEL_map;
>  #endif
> @@ -105,7 +105,7 @@ void __init kernel_randomize_memory(void)
>  	 */
>  	BUILD_BUG_ON(vaddr_start >= vaddr_end);
>  	BUILD_BUG_ON(IS_ENABLED(CONFIG_X86_ESPFIX64) &&
> -		     vaddr_end >= EFI_VA_START);
> +		     vaddr_end >= EFI_VA_END);
>  	BUILD_BUG_ON((IS_ENABLED(CONFIG_X86_ESPFIX64) ||
>  		      IS_ENABLED(CONFIG_EFI)) &&
>  		     vaddr_end >= __START_KERNEL_map);
> -- 
> 2.5.5
> 

Re: [PATCH v1 RESEND 1/2] x86/mm/KASLR: EFI region is mistakenly included into KASLR VA space for randomization

[Dave Young]

Hi, Baoquan

On 03/23/17 at 11:27am, Baoquan He wrote:
> Currently KASLR is enabled on three regions: the direct mapping of physical
> memory, vamlloc and vmemmap. However EFI region is also mistakenly included
> for VA space randomization because of misusing EFI_VA_START macro and
> assuming EFI_VA_START < EFI_VA_END.
> 
> The EFI region is reserved for EFI runtime services virtual mapping which
> should not be included in kaslr ranges. It will be re-used by kexec/kdump
> kernel, the mistake may cause failure when jump to kexec/kdump kernel if
> vmemmap allocation stomps on the allocated efi mapping region.

No need to mention kexec/kdump in changelog although it is true that
kexec kernel will use the persistent efi runtime mapping. The main point
is it is wrong to use the reserved vm space for efi.

Also I think this patch can be sent as a standalone patch, no need to be
a patch series. For the second patch I think it depends on efi
maintainer's opinion, personally I think only this simple fix for kaslr only
will be better.

> 
> In Documentation/x86/x86_64/mm.txt, we can see:
>   ffffffef00000000 - fffffffeffffffff (=64 GB) EFI region mapping space
> EFI use the space from -4G to -64G thus EFI_VA_START > EFI_VA_END
> Here EFI_VA_START = -4G, and EFI_VA_END = -64G
> 
> Changing EFI_VA_START to EFI_VA_END in mm/kaslr.c fixes this problem.
> 
> Cc: <stable@vger.kernel.org> #4.8+
> Signed-off-by: Baoquan He <bhe@redhat.com>
> Acked-by: Dave Young <dyoung@redhat.com>
> Reviewed-by: Bhupesh Sharma <bhsharma@redhat.com>
> Acked-by: Thomas Garnier <thgarnie@google.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: "H. Peter Anvin" <hpa@zytor.com> 
> Cc: x86@kernel.org
> Cc: Thomas Garnier <thgarnie@google.com>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
>  arch/x86/mm/kaslr.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/mm/kaslr.c b/arch/x86/mm/kaslr.c
> index 887e571..aed2064 100644
> --- a/arch/x86/mm/kaslr.c
> +++ b/arch/x86/mm/kaslr.c
> @@ -48,7 +48,7 @@ static const unsigned long vaddr_start = __PAGE_OFFSET_BASE;
>  #if defined(CONFIG_X86_ESPFIX64)
>  static const unsigned long vaddr_end = ESPFIX_BASE_ADDR;
>  #elif defined(CONFIG_EFI)
> -static const unsigned long vaddr_end = EFI_VA_START;
> +static const unsigned long vaddr_end = EFI_VA_END;
>  #else
>  static const unsigned long vaddr_end = __START_KERNEL_map;
>  #endif
> @@ -105,7 +105,7 @@ void __init kernel_randomize_memory(void)
>  	 */
>  	BUILD_BUG_ON(vaddr_start >= vaddr_end);
>  	BUILD_BUG_ON(IS_ENABLED(CONFIG_X86_ESPFIX64) &&
> -		     vaddr_end >= EFI_VA_START);
> +		     vaddr_end >= EFI_VA_END);
>  	BUILD_BUG_ON((IS_ENABLED(CONFIG_X86_ESPFIX64) ||
>  		      IS_ENABLED(CONFIG_EFI)) &&
>  		     vaddr_end >= __START_KERNEL_map);
> -- 
> 2.5.5
> 

Thanks
Dave

Re: [PATCH v1 RESEND 1/2] x86/mm/KASLR: EFI region is mistakenly included into KASLR VA space for randomization

[Dave Young]

This should also cc linux-efi

On 03/24/17 at 10:29am, Dave Young wrote:
> Hi, Baoquan
> 
> On 03/23/17 at 11:27am, Baoquan He wrote:
> > Currently KASLR is enabled on three regions: the direct mapping of physical
> > memory, vamlloc and vmemmap. However EFI region is also mistakenly included
> > for VA space randomization because of misusing EFI_VA_START macro and
> > assuming EFI_VA_START < EFI_VA_END.
> > 
> > The EFI region is reserved for EFI runtime services virtual mapping which
> > should not be included in kaslr ranges. It will be re-used by kexec/kdump
> > kernel, the mistake may cause failure when jump to kexec/kdump kernel if
> > vmemmap allocation stomps on the allocated efi mapping region.
> 
> No need to mention kexec/kdump in changelog although it is true that
> kexec kernel will use the persistent efi runtime mapping. The main point
> is it is wrong to use the reserved vm space for efi.

Explain more about this:

It is a general issue instead of a kexec/kdump issue and it is a real
bug. Although efi has its own page tables, it will still sync kernel
page tables along with the mapping of efi reserved area. So if vmalloc
etc use the vm space of efi reserved area, then some of them will be
missed when efi sync the low kernel page tables..

> 
> Also I think this patch can be sent as a standalone patch, no need to be
> a patch series. For the second patch I think it depends on efi
> maintainer's opinion, personally I think only this simple fix for kaslr only
> will be better.
> 
> > 
> > In Documentation/x86/x86_64/mm.txt, we can see:
> >   ffffffef00000000 - fffffffeffffffff (=64 GB) EFI region mapping space
> > EFI use the space from -4G to -64G thus EFI_VA_START > EFI_VA_END
> > Here EFI_VA_START = -4G, and EFI_VA_END = -64G
> > 
> > Changing EFI_VA_START to EFI_VA_END in mm/kaslr.c fixes this problem.
> > 
> > Cc: <stable@vger.kernel.org> #4.8+
> > Signed-off-by: Baoquan He <bhe@redhat.com>
> > Acked-by: Dave Young <dyoung@redhat.com>
> > Reviewed-by: Bhupesh Sharma <bhsharma@redhat.com>
> > Acked-by: Thomas Garnier <thgarnie@google.com>
> > Cc: Thomas Gleixner <tglx@linutronix.de>
> > Cc: Ingo Molnar <mingo@redhat.com>
> > Cc: "H. Peter Anvin" <hpa@zytor.com> 
> > Cc: x86@kernel.org
> > Cc: Thomas Garnier <thgarnie@google.com>
> > Cc: Kees Cook <keescook@chromium.org>
> > Cc: Borislav Petkov <bp@alien8.de>
> > Cc: Andrew Morton <akpm@linux-foundation.org>
> > Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
> > ---
> >  arch/x86/mm/kaslr.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/x86/mm/kaslr.c b/arch/x86/mm/kaslr.c
> > index 887e571..aed2064 100644
> > --- a/arch/x86/mm/kaslr.c
> > +++ b/arch/x86/mm/kaslr.c
> > @@ -48,7 +48,7 @@ static const unsigned long vaddr_start = __PAGE_OFFSET_BASE;
> >  #if defined(CONFIG_X86_ESPFIX64)
> >  static const unsigned long vaddr_end = ESPFIX_BASE_ADDR;
> >  #elif defined(CONFIG_EFI)
> > -static const unsigned long vaddr_end = EFI_VA_START;
> > +static const unsigned long vaddr_end = EFI_VA_END;
> >  #else
> >  static const unsigned long vaddr_end = __START_KERNEL_map;
> >  #endif
> > @@ -105,7 +105,7 @@ void __init kernel_randomize_memory(void)
> >  	 */
> >  	BUILD_BUG_ON(vaddr_start >= vaddr_end);
> >  	BUILD_BUG_ON(IS_ENABLED(CONFIG_X86_ESPFIX64) &&
> > -		     vaddr_end >= EFI_VA_START);
> > +		     vaddr_end >= EFI_VA_END);
> >  	BUILD_BUG_ON((IS_ENABLED(CONFIG_X86_ESPFIX64) ||
> >  		      IS_ENABLED(CONFIG_EFI)) &&
> >  		     vaddr_end >= __START_KERNEL_map);
> > -- 
> > 2.5.5
> > 
> 
> Thanks
> Dave

Re: [PATCH v1 RESEND 1/2] x86/mm/KASLR: EFI region is mistakenly included into KASLR VA space for randomization

[Baoquan He]

On 03/24/17 at 10:29am, Dave Young wrote:
> Hi, Baoquan
> 
> On 03/23/17 at 11:27am, Baoquan He wrote:
> > Currently KASLR is enabled on three regions: the direct mapping of physical
> > memory, vamlloc and vmemmap. However EFI region is also mistakenly included
> > for VA space randomization because of misusing EFI_VA_START macro and
> > assuming EFI_VA_START < EFI_VA_END.
> > 
> > The EFI region is reserved for EFI runtime services virtual mapping which
> > should not be included in kaslr ranges. It will be re-used by kexec/kdump
> > kernel, the mistake may cause failure when jump to kexec/kdump kernel if
> > vmemmap allocation stomps on the allocated efi mapping region.
> 
> No need to mention kexec/kdump in changelog although it is true that
> kexec kernel will use the persistent efi runtime mapping. The main point
> is it is wrong to use the reserved vm space for efi.

I only say the consequence from kdump point of view and point out that.

Anyway I am fine w/o kexec/kdump text. Will repost this patch only
without kexec-ed kernel saying.


> 
> Also I think this patch can be sent as a standalone patch, no need to be
> a patch series. For the second patch I think it depends on efi
> maintainer's opinion, personally I think only this simple fix for kaslr only
> will be better.
> 
> > 
> > In Documentation/x86/x86_64/mm.txt, we can see:
> >   ffffffef00000000 - fffffffeffffffff (=64 GB) EFI region mapping space
> > EFI use the space from -4G to -64G thus EFI_VA_START > EFI_VA_END
> > Here EFI_VA_START = -4G, and EFI_VA_END = -64G
> > 
> > Changing EFI_VA_START to EFI_VA_END in mm/kaslr.c fixes this problem.
> > 
> > Cc: <stable@vger.kernel.org> #4.8+
> > Signed-off-by: Baoquan He <bhe@redhat.com>
> > Acked-by: Dave Young <dyoung@redhat.com>
> > Reviewed-by: Bhupesh Sharma <bhsharma@redhat.com>
> > Acked-by: Thomas Garnier <thgarnie@google.com>
> > Cc: Thomas Gleixner <tglx@linutronix.de>
> > Cc: Ingo Molnar <mingo@redhat.com>
> > Cc: "H. Peter Anvin" <hpa@zytor.com> 
> > Cc: x86@kernel.org
> > Cc: Thomas Garnier <thgarnie@google.com>
> > Cc: Kees Cook <keescook@chromium.org>
> > Cc: Borislav Petkov <bp@alien8.de>
> > Cc: Andrew Morton <akpm@linux-foundation.org>
> > Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
> > ---
> >  arch/x86/mm/kaslr.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/x86/mm/kaslr.c b/arch/x86/mm/kaslr.c
> > index 887e571..aed2064 100644
> > --- a/arch/x86/mm/kaslr.c
> > +++ b/arch/x86/mm/kaslr.c
> > @@ -48,7 +48,7 @@ static const unsigned long vaddr_start = __PAGE_OFFSET_BASE;
> >  #if defined(CONFIG_X86_ESPFIX64)
> >  static const unsigned long vaddr_end = ESPFIX_BASE_ADDR;
> >  #elif defined(CONFIG_EFI)
> > -static const unsigned long vaddr_end = EFI_VA_START;
> > +static const unsigned long vaddr_end = EFI_VA_END;
> >  #else
> >  static const unsigned long vaddr_end = __START_KERNEL_map;
> >  #endif
> > @@ -105,7 +105,7 @@ void __init kernel_randomize_memory(void)
> >  	 */
> >  	BUILD_BUG_ON(vaddr_start >= vaddr_end);
> >  	BUILD_BUG_ON(IS_ENABLED(CONFIG_X86_ESPFIX64) &&
> > -		     vaddr_end >= EFI_VA_START);
> > +		     vaddr_end >= EFI_VA_END);
> >  	BUILD_BUG_ON((IS_ENABLED(CONFIG_X86_ESPFIX64) ||
> >  		      IS_ENABLED(CONFIG_EFI)) &&
> >  		     vaddr_end >= __START_KERNEL_map);
> > -- 
> > 2.5.5
> > 
> 
> Thanks
> Dave

Re: [PATCH v1 RESEND 2/2] x86/efi: Clean up a minor mistake in code comment

[Ard Biesheuvel]

On 23 March 2017 at 03:27, Baoquan He <bhe@redhat.com> wrote:
> EFI allocate runtime services regions from EFI_VA_START, -4G, down
> to -64G, EFI_VA_END.

OK, so is the size of the region 60 GB or 64 GB? This suggests 60 GB,
but the comment you update suggests 64 GB.

> The mechanism was introduced in
> commit d2f7cbe7b26a7 ("x86/efi: Runtime services virtual mapping").
>
> Clean it up to avoid confusion.
>
> Signed-off-by: Baoquan He <bhe@redhat.com>
> Cc: Matt Fleming <matt@codeblueprint.co.uk>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: x86@kernel.org
> Cc: linux-efi@vger.kernel.org
> ---
>  arch/x86/platform/efi/efi_64.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/x86/platform/efi/efi_64.c b/arch/x86/platform/efi/efi_64.c
> index a4695da..6cbf9e0 100644
> --- a/arch/x86/platform/efi/efi_64.c
> +++ b/arch/x86/platform/efi/efi_64.c
> @@ -47,7 +47,7 @@
>  #include <asm/pgalloc.h>
>
>  /*
> - * We allocate runtime services regions bottom-up, starting from -4G, i.e.
> + * We allocate runtime services regions top-down, starting from -4G, i.e.
>   * 0xffff_ffff_0000_0000 and limit EFI VA mapping space to 64G.
>   */
>  static u64 efi_va = EFI_VA_START;
> --
> 2.5.5
>

Re: [PATCH v1 RESEND 2/2] x86/efi: Clean up a minor mistake in code comment

[Baoquan He]

On 03/24/17 at 08:57am, Ard Biesheuvel wrote:
> On 23 March 2017 at 03:27, Baoquan He <bhe@redhat.com> wrote:
> > EFI allocate runtime services regions from EFI_VA_START, -4G, down
> > to -64G, EFI_VA_END.
> 
> OK, so is the size of the region 60 GB or 64 GB? This suggests 60 GB,
> but the comment you update suggests 64 GB.

Ah, sorry, here it should be -68G. The reserved EFI region is [-68G,
-4G), amount is 64G.

Thanks, will repost with updated log.

> 
> > The mechanism was introduced in
> > commit d2f7cbe7b26a7 ("x86/efi: Runtime services virtual mapping").
> >
> > Clean it up to avoid confusion.
> >
> > Signed-off-by: Baoquan He <bhe@redhat.com>
> > Cc: Matt Fleming <matt@codeblueprint.co.uk>
> > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > Cc: Thomas Gleixner <tglx@linutronix.de>
> > Cc: Ingo Molnar <mingo@redhat.com>
> > Cc: "H. Peter Anvin" <hpa@zytor.com>
> > Cc: Borislav Petkov <bp@alien8.de>
> > Cc: x86@kernel.org
> > Cc: linux-efi@vger.kernel.org
> > ---
> >  arch/x86/platform/efi/efi_64.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/x86/platform/efi/efi_64.c b/arch/x86/platform/efi/efi_64.c
> > index a4695da..6cbf9e0 100644
> > --- a/arch/x86/platform/efi/efi_64.c
> > +++ b/arch/x86/platform/efi/efi_64.c
> > @@ -47,7 +47,7 @@
> >  #include <asm/pgalloc.h>
> >
> >  /*
> > - * We allocate runtime services regions bottom-up, starting from -4G, i.e.
> > + * We allocate runtime services regions top-down, starting from -4G, i.e.
> >   * 0xffff_ffff_0000_0000 and limit EFI VA mapping space to 64G.
> >   */
> >  static u64 efi_va = EFI_VA_START;
> > --
> > 2.5.5
> >