[PATCH] x86/efi: Don't unmap EFI boot services code/data regions for EFI_OLD_MEMMAP and EFI_MIXED_MODE

[PATCH] x86/efi: Don't unmap EFI boot services code/data regions for EFI_OLD_MEMMAP and EFI_MIXED_MODE

[Sai Praneeth Prakhya]

Commit d5052a7130a6 ("x86/efi: Unmap EFI boot services code/data regions
from efi_pgd") forgets to take two EFI modes into consideration namely
EFI_OLD_MEMMAP and EFI_MIXED_MODE.

EFI_OLD_MEMMAP is a legacy way of mapping EFI regions into swapper_pg_dir
using ioremap() and init_memory_mapping(). This feature can be enabled by
passing "efi=old_map" as kernel command line argument. But,
efi_unmap_pages() unmaps EFI boot services code/data regions *only* from
efi_pgd and hence cannot be used for unmapping EFI boot services code/data
regions from swapper_pg_dir.

Introduce a temporary fix to not unmap EFI boot services code/data regions
when EFI_OLD_MEMMAP is enabled while working on a real fix.

EFI_MIXED_MODE is another feature where a 64-bit kernel runs on a
64-bit platform crippled by a 32-bit firmware. To support EFI_MIXED_MODE,
all RAM (i.e. namely EFI regions like EFI_CONVENTIONAL_MEMORY,
EFI_LOADER_<CODE/DATA>, EFI_BOOT_SERVICES_<CODE/DATA> and
EFI_RUNTIME_CODE/DATA regions) is mapped into efi_pgd all the time to
facilitate EFI runtime calls access it's arguments in 1:1 mode. Hence,
don't unmap EFI boot services code/data regions when booted in mixed mode.

Signed-off-by: Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Dave Hansen <dave.hansen@intel.com>
Cc: Bhupesh Sharma <bhsharma@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 arch/x86/platform/efi/quirks.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/arch/x86/platform/efi/quirks.c b/arch/x86/platform/efi/quirks.c
index 09e811b9da26..9c34230aaeae 100644
--- a/arch/x86/platform/efi/quirks.c
+++ b/arch/x86/platform/efi/quirks.c
@@ -380,6 +380,22 @@ static void __init efi_unmap_pages(efi_memory_desc_t *md)
 	u64 pa = md->phys_addr;
 	u64 va = md->virt_addr;
 
+	/*
+	 * To Do: Remove this check after adding functionality to unmap EFI boot
+	 * services code/data regions from direct mapping area because
+	 * "efi=old_map" maps EFI regions in swapper_pg_dir.
+	 */
+	if (efi_enabled(EFI_OLD_MEMMAP))
+		return;
+
+	/*
+	 * EFI mixed mode has all RAM mapped to access arguments while making
+	 * EFI runtime calls, hence don't unmap EFI boot services code/data
+	 * regions.
+	 */
+	if (!efi_is_native() && IS_ENABLED(CONFIG_EFI_MIXED))
+		return;
+
 	if (kernel_unmap_pages_in_pgd(pgd, pa, md->num_pages))
 		pr_err("Failed to unmap 1:1 mapping for 0x%llx\n", pa);
 
-- 
2.19.1

Re: [PATCH] x86/efi: Don't unmap EFI boot services code/data regions for EFI_OLD_MEMMAP and EFI_MIXED_MODE

[Ingo Molnar]

* Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com> wrote:

> Commit d5052a7130a6 ("x86/efi: Unmap EFI boot services code/data regions
> from efi_pgd") forgets to take two EFI modes into consideration namely
> EFI_OLD_MEMMAP and EFI_MIXED_MODE.

So the commit sha1 ended up being this one in tip:efi/core:

  08cfb38f3ef4: x86/efi: Unmap EFI boot services code/data regions from efi_pgd

> EFI_OLD_MEMMAP is a legacy way of mapping EFI regions into swapper_pg_dir
> using ioremap() and init_memory_mapping(). This feature can be enabled by
> passing "efi=old_map" as kernel command line argument. But,
> efi_unmap_pages() unmaps EFI boot services code/data regions *only* from
> efi_pgd and hence cannot be used for unmapping EFI boot services code/data
> regions from swapper_pg_dir.
> 
> Introduce a temporary fix to not unmap EFI boot services code/data regions
> when EFI_OLD_MEMMAP is enabled while working on a real fix.
> 
> EFI_MIXED_MODE is another feature where a 64-bit kernel runs on a
> 64-bit platform crippled by a 32-bit firmware. To support EFI_MIXED_MODE,
> all RAM (i.e. namely EFI regions like EFI_CONVENTIONAL_MEMORY,
> EFI_LOADER_<CODE/DATA>, EFI_BOOT_SERVICES_<CODE/DATA> and
> EFI_RUNTIME_CODE/DATA regions) is mapped into efi_pgd all the time to
> facilitate EFI runtime calls access it's arguments in 1:1 mode. Hence,
> don't unmap EFI boot services code/data regions when booted in mixed mode.
> 
> Signed-off-by: Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: Andy Lutomirski <luto@kernel.org>
> Cc: Dave Hansen <dave.hansen@intel.com>
> Cc: Bhupesh Sharma <bhsharma@redhat.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  arch/x86/platform/efi/quirks.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/arch/x86/platform/efi/quirks.c b/arch/x86/platform/efi/quirks.c
> index 09e811b9da26..9c34230aaeae 100644
> --- a/arch/x86/platform/efi/quirks.c
> +++ b/arch/x86/platform/efi/quirks.c
> @@ -380,6 +380,22 @@ static void __init efi_unmap_pages(efi_memory_desc_t *md)
>  	u64 pa = md->phys_addr;
>  	u64 va = md->virt_addr;
>  
> +	/*
> +	 * To Do: Remove this check after adding functionality to unmap EFI boot
> +	 * services code/data regions from direct mapping area because
> +	 * "efi=old_map" maps EFI regions in swapper_pg_dir.
> +	 */
> +	if (efi_enabled(EFI_OLD_MEMMAP))
> +		return;
> +
> +	/*
> +	 * EFI mixed mode has all RAM mapped to access arguments while making
> +	 * EFI runtime calls, hence don't unmap EFI boot services code/data
> +	 * regions.
> +	 */
> +	if (!efi_is_native() && IS_ENABLED(CONFIG_EFI_MIXED))
> +		return;

I suppose old_mmap and mixed mode stopped working altogether after the 
unmapping changes? What are the symptoms, instant reboots, crasher, or 
some more benign behavior like non-working runtime EFI functionality?

If Ard acks this I'll apply it immediately, as these bugs look like 
show-stoppers for merging the EFI tree into v4.21.

Thanks,

	Ingo

Re: [PATCH] x86/efi: Don't unmap EFI boot services code/data regions for EFI_OLD_MEMMAP and EFI_MIXED_MODE

[Ard Biesheuvel]

On Sat, 22 Dec 2018 at 11:54, Ingo Molnar <mingo@kernel.org> wrote:
>
>
> * Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com> wrote:
>
> > Commit d5052a7130a6 ("x86/efi: Unmap EFI boot services code/data regions
> > from efi_pgd") forgets to take two EFI modes into consideration namely
> > EFI_OLD_MEMMAP and EFI_MIXED_MODE.
>
> So the commit sha1 ended up being this one in tip:efi/core:
>
>   08cfb38f3ef4: x86/efi: Unmap EFI boot services code/data regions from efi_pgd
>
> > EFI_OLD_MEMMAP is a legacy way of mapping EFI regions into swapper_pg_dir
> > using ioremap() and init_memory_mapping(). This feature can be enabled by
> > passing "efi=old_map" as kernel command line argument. But,
> > efi_unmap_pages() unmaps EFI boot services code/data regions *only* from
> > efi_pgd and hence cannot be used for unmapping EFI boot services code/data
> > regions from swapper_pg_dir.
> >
> > Introduce a temporary fix to not unmap EFI boot services code/data regions
> > when EFI_OLD_MEMMAP is enabled while working on a real fix.
> >
> > EFI_MIXED_MODE is another feature where a 64-bit kernel runs on a
> > 64-bit platform crippled by a 32-bit firmware. To support EFI_MIXED_MODE,
> > all RAM (i.e. namely EFI regions like EFI_CONVENTIONAL_MEMORY,
> > EFI_LOADER_<CODE/DATA>, EFI_BOOT_SERVICES_<CODE/DATA> and
> > EFI_RUNTIME_CODE/DATA regions) is mapped into efi_pgd all the time to
> > facilitate EFI runtime calls access it's arguments in 1:1 mode. Hence,
> > don't unmap EFI boot services code/data regions when booted in mixed mode.
> >
> > Signed-off-by: Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>
> > Cc: Borislav Petkov <bp@alien8.de>
> > Cc: Ingo Molnar <mingo@kernel.org>
> > Cc: Andy Lutomirski <luto@kernel.org>
> > Cc: Dave Hansen <dave.hansen@intel.com>
> > Cc: Bhupesh Sharma <bhsharma@redhat.com>
> > Cc: Peter Zijlstra <peterz@infradead.org>
> > Cc: Thomas Gleixner <tglx@linutronix.de>
> > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > ---
> >  arch/x86/platform/efi/quirks.c | 16 ++++++++++++++++
> >  1 file changed, 16 insertions(+)
> >
> > diff --git a/arch/x86/platform/efi/quirks.c b/arch/x86/platform/efi/quirks.c
> > index 09e811b9da26..9c34230aaeae 100644
> > --- a/arch/x86/platform/efi/quirks.c
> > +++ b/arch/x86/platform/efi/quirks.c
> > @@ -380,6 +380,22 @@ static void __init efi_unmap_pages(efi_memory_desc_t *md)
> >       u64 pa = md->phys_addr;
> >       u64 va = md->virt_addr;
> >
> > +     /*
> > +      * To Do: Remove this check after adding functionality to unmap EFI boot
> > +      * services code/data regions from direct mapping area because
> > +      * "efi=old_map" maps EFI regions in swapper_pg_dir.
> > +      */
> > +     if (efi_enabled(EFI_OLD_MEMMAP))
> > +             return;
> > +
> > +     /*
> > +      * EFI mixed mode has all RAM mapped to access arguments while making
> > +      * EFI runtime calls, hence don't unmap EFI boot services code/data
> > +      * regions.
> > +      */
> > +     if (!efi_is_native() && IS_ENABLED(CONFIG_EFI_MIXED))

AFAIK efi_is_native() can only return false is CONFIG_EFI_MIXED is
set, so this expression can be simplified.

> > +             return;
>
> I suppose old_mmap and mixed mode stopped working altogether after the
> unmapping changes? What are the symptoms, instant reboots, crasher, or
> some more benign behavior like non-working runtime EFI functionality?
>
> If Ard acks this I'll apply it immediately, as these bugs look like
> show-stoppers for merging the EFI tree into v4.21.
>

Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

with the sidenote that I won't be able to test this myself until
monday at the earliest.

RE: [PATCH] x86/efi: Don't unmap EFI boot services code/data regions for EFI_OLD_MEMMAP and EFI_MIXED_MODE

[Prakhya, Sai Praneeth]

> 
> * Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com> wrote:
> 
> > Commit d5052a7130a6 ("x86/efi: Unmap EFI boot services code/data
> > regions from efi_pgd") forgets to take two EFI modes into
> > consideration namely EFI_OLD_MEMMAP and EFI_MIXED_MODE.
> 
> So the commit sha1 ended up being this one in tip:efi/core:
> 
>   08cfb38f3ef4: x86/efi: Unmap EFI boot services code/data regions from
> efi_pgd

Ya.. sorry! I took the sha1 from efi tree.

> 
> > EFI_OLD_MEMMAP is a legacy way of mapping EFI regions into
> > swapper_pg_dir using ioremap() and init_memory_mapping(). This feature
> > can be enabled by passing "efi=old_map" as kernel command line
> > argument. But,
> > efi_unmap_pages() unmaps EFI boot services code/data regions *only*
> > from efi_pgd and hence cannot be used for unmapping EFI boot services
> > code/data regions from swapper_pg_dir.


[.........]


> > +	/*
> > +	 * EFI mixed mode has all RAM mapped to access arguments while
> making
> > +	 * EFI runtime calls, hence don't unmap EFI boot services code/data
> > +	 * regions.
> > +	 */
> > +	if (!efi_is_native() && IS_ENABLED(CONFIG_EFI_MIXED))
> > +		return;
> 
> I suppose old_mmap and mixed mode stopped working altogether after the
> unmapping changes?

Yes.. that's true.

> What are the symptoms, instant reboots, crasher, or some
> more benign behavior like non-working runtime EFI functionality?

I noticed a kernel panic due to page fault.

AFAIK, it happens because efi_pgd is not defined for old_map and mixed mode.
Hence efi_unmap_pages() which calls __change_page_attr() unmaps mappings 
from swapper_pg_dir (please note that this my theory but I haven't checked if
this is really true).

> 
> If Ard acks this I'll apply it immediately, as these bugs look like show-stoppers for
> merging the EFI tree into v4.21.
> 
> Thanks,
> 
> 	Ingo

[tip:efi/core] x86/efi: Don't unmap EFI boot services code/data regions for EFI_OLD_MEMMAP and EFI_MIXED_MODE

[tip-bot for Sai Praneeth Prakhya]

Commit-ID:  1debf0958fa27b7c469dbf22754929ec59a7c0e7
Gitweb:     https://git.kernel.org/tip/1debf0958fa27b7c469dbf22754929ec59a7c0e7
Author:     Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>
AuthorDate: Fri, 21 Dec 2018 18:22:34 -0800
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Sat, 22 Dec 2018 20:58:30 +0100

x86/efi: Don't unmap EFI boot services code/data regions for EFI_OLD_MEMMAP and EFI_MIXED_MODE

The following commit:

  d5052a7130a6 ("x86/efi: Unmap EFI boot services code/data regions from efi_pgd")

forgets to take two EFI modes into consideration, namely EFI_OLD_MEMMAP and
EFI_MIXED_MODE:

- EFI_OLD_MEMMAP is a legacy way of mapping EFI regions into swapper_pg_dir
  using ioremap() and init_memory_mapping(). This feature can be enabled by
  passing "efi=old_map" as kernel command line argument. But,
  efi_unmap_pages() unmaps EFI boot services code/data regions *only* from
  efi_pgd and hence cannot be used for unmapping EFI boot services code/data
  regions from swapper_pg_dir.

Introduce a temporary fix to not unmap EFI boot services code/data regions
when EFI_OLD_MEMMAP is enabled while working on a real fix.

- EFI_MIXED_MODE is another feature where a 64-bit kernel runs on a
  64-bit platform crippled by a 32-bit firmware. To support EFI_MIXED_MODE,
  all RAM (i.e. namely EFI regions like EFI_CONVENTIONAL_MEMORY,
  EFI_LOADER_<CODE/DATA>, EFI_BOOT_SERVICES_<CODE/DATA> and
  EFI_RUNTIME_CODE/DATA regions) is mapped into efi_pgd all the time to
  facilitate EFI runtime calls access it's arguments in 1:1 mode.

Hence, don't unmap EFI boot services code/data regions when booted in mixed mode.

Signed-off-by: Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>
Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Bhupesh Sharma <bhsharma@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Dave Hansen <dave.hansen@intel.com>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Rik van Riel <riel@surriel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-efi@vger.kernel.org
Link: http://lkml.kernel.org/r/20181222022234.7573-1-sai.praneeth.prakhya@intel.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/platform/efi/quirks.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/arch/x86/platform/efi/quirks.c b/arch/x86/platform/efi/quirks.c
index 09e811b9da26..17456a1d3f04 100644
--- a/arch/x86/platform/efi/quirks.c
+++ b/arch/x86/platform/efi/quirks.c
@@ -380,6 +380,22 @@ static void __init efi_unmap_pages(efi_memory_desc_t *md)
 	u64 pa = md->phys_addr;
 	u64 va = md->virt_addr;
 
+	/*
+	 * To Do: Remove this check after adding functionality to unmap EFI boot
+	 * services code/data regions from direct mapping area because
+	 * "efi=old_map" maps EFI regions in swapper_pg_dir.
+	 */
+	if (efi_enabled(EFI_OLD_MEMMAP))
+		return;
+
+	/*
+	 * EFI mixed mode has all RAM mapped to access arguments while making
+	 * EFI runtime calls, hence don't unmap EFI boot services code/data
+	 * regions.
+	 */
+	if (!efi_is_native())
+		return;
+
 	if (kernel_unmap_pages_in_pgd(pgd, pa, md->num_pages))
 		pr_err("Failed to unmap 1:1 mapping for 0x%llx\n", pa);
 

Re: [PATCH] x86/efi: Don't unmap EFI boot services code/data regions for EFI_OLD_MEMMAP and EFI_MIXED_MODE

[Ard Biesheuvel]

On Sat, 22 Dec 2018 at 12:04, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>
> On Sat, 22 Dec 2018 at 11:54, Ingo Molnar <mingo@kernel.org> wrote:
> >
> >
> > * Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com> wrote:
> >
> > > Commit d5052a7130a6 ("x86/efi: Unmap EFI boot services code/data regions
> > > from efi_pgd") forgets to take two EFI modes into consideration namely
> > > EFI_OLD_MEMMAP and EFI_MIXED_MODE.
> >
> > So the commit sha1 ended up being this one in tip:efi/core:
> >
> >   08cfb38f3ef4: x86/efi: Unmap EFI boot services code/data regions from efi_pgd
> >
> > > EFI_OLD_MEMMAP is a legacy way of mapping EFI regions into swapper_pg_dir
> > > using ioremap() and init_memory_mapping(). This feature can be enabled by
> > > passing "efi=old_map" as kernel command line argument. But,
> > > efi_unmap_pages() unmaps EFI boot services code/data regions *only* from
> > > efi_pgd and hence cannot be used for unmapping EFI boot services code/data
> > > regions from swapper_pg_dir.
> > >
> > > Introduce a temporary fix to not unmap EFI boot services code/data regions
> > > when EFI_OLD_MEMMAP is enabled while working on a real fix.
> > >
> > > EFI_MIXED_MODE is another feature where a 64-bit kernel runs on a
> > > 64-bit platform crippled by a 32-bit firmware. To support EFI_MIXED_MODE,
> > > all RAM (i.e. namely EFI regions like EFI_CONVENTIONAL_MEMORY,
> > > EFI_LOADER_<CODE/DATA>, EFI_BOOT_SERVICES_<CODE/DATA> and
> > > EFI_RUNTIME_CODE/DATA regions) is mapped into efi_pgd all the time to
> > > facilitate EFI runtime calls access it's arguments in 1:1 mode. Hence,
> > > don't unmap EFI boot services code/data regions when booted in mixed mode.
> > >
> > > Signed-off-by: Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>
> > > Cc: Borislav Petkov <bp@alien8.de>
> > > Cc: Ingo Molnar <mingo@kernel.org>
> > > Cc: Andy Lutomirski <luto@kernel.org>
> > > Cc: Dave Hansen <dave.hansen@intel.com>
> > > Cc: Bhupesh Sharma <bhsharma@redhat.com>
> > > Cc: Peter Zijlstra <peterz@infradead.org>
> > > Cc: Thomas Gleixner <tglx@linutronix.de>
> > > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > > ---
> > >  arch/x86/platform/efi/quirks.c | 16 ++++++++++++++++
> > >  1 file changed, 16 insertions(+)
> > >
> > > diff --git a/arch/x86/platform/efi/quirks.c b/arch/x86/platform/efi/quirks.c
> > > index 09e811b9da26..9c34230aaeae 100644
> > > --- a/arch/x86/platform/efi/quirks.c
> > > +++ b/arch/x86/platform/efi/quirks.c
> > > @@ -380,6 +380,22 @@ static void __init efi_unmap_pages(efi_memory_desc_t *md)
> > >       u64 pa = md->phys_addr;
> > >       u64 va = md->virt_addr;
> > >
> > > +     /*
> > > +      * To Do: Remove this check after adding functionality to unmap EFI boot
> > > +      * services code/data regions from direct mapping area because
> > > +      * "efi=old_map" maps EFI regions in swapper_pg_dir.
> > > +      */
> > > +     if (efi_enabled(EFI_OLD_MEMMAP))
> > > +             return;
> > > +
> > > +     /*
> > > +      * EFI mixed mode has all RAM mapped to access arguments while making
> > > +      * EFI runtime calls, hence don't unmap EFI boot services code/data
> > > +      * regions.
> > > +      */
> > > +     if (!efi_is_native() && IS_ENABLED(CONFIG_EFI_MIXED))
>
> AFAIK efi_is_native() can only return false is CONFIG_EFI_MIXED is
> set, so this expression can be simplified.
>
> > > +             return;
> >
> > I suppose old_mmap and mixed mode stopped working altogether after the
> > unmapping changes? What are the symptoms, instant reboots, crasher, or
> > some more benign behavior like non-working runtime EFI functionality?
> >
> > If Ard acks this I'll apply it immediately, as these bugs look like
> > show-stoppers for merging the EFI tree into v4.21.
> >
>
> Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>
> with the sidenote that I won't be able to test this myself until
> monday at the earliest.

OK, so I have tested both efi=old_map and mixed mode before and after
applying this patch, using QEMU/KVM with 64-bit and 32-bit builds of
OVMF [respectively]

efi=old_map is indeed broken before and needs this patch.

Mixed mode works just fine both before and after applying the patch,
but I suggest we keep this patch as-is and address mixed mode later if
needed (I spotted a couple of things in the boot log that may need
some attention but I'm not sure if the issue is in Linux or in OVMF)

Re: [PATCH] x86/efi: Don't unmap EFI boot services code/data regions for EFI_OLD_MEMMAP and EFI_MIXED_MODE

[Ingo Molnar]

* Prakhya, Sai Praneeth <sai.praneeth.prakhya@intel.com> wrote:

> > > > +     /*
> > > > +      * To Do: Remove this check after adding functionality to unmap EFI
> > boot
> > > > +      * services code/data regions from direct mapping area because
> > > > +      * "efi=old_map" maps EFI regions in swapper_pg_dir.
> > > > +      */
> > > > +     if (efi_enabled(EFI_OLD_MEMMAP))
> > > > +             return;
> > > > +
> > > > +     /*
> > > > +      * EFI mixed mode has all RAM mapped to access arguments while
> > making
> > > > +      * EFI runtime calls, hence don't unmap EFI boot services code/data
> > > > +      * regions.
> > > > +      */
> > > > +     if (!efi_is_native() && IS_ENABLED(CONFIG_EFI_MIXED))
> > 
> > AFAIK efi_is_native() can only return false is CONFIG_EFI_MIXED is set, so this
> > expression can be simplified.
> 
> Makes sense.
> efi_is_native() returns true for 32-bit machines running 32-bit firmware.

Forgot to mention that I performed this simplification in the commit I 
applied:

+       /*
+        * To Do: Remove this check after adding functionality to unmap EFI boot
+        * services code/data regions from direct mapping area because
+        * "efi=old_map" maps EFI regions in swapper_pg_dir.
+        */
+       if (efi_enabled(EFI_OLD_MEMMAP))
+               return;
+
+       /*
+        * EFI mixed mode has all RAM mapped to access arguments while making
+        * EFI runtime calls, hence don't unmap EFI boot services code/data
+        * regions.
+        */
+       if (!efi_is_native())
+               return;

Thanks,

	Ingo

RE: [PATCH] x86/efi: Don't unmap EFI boot services code/data regions for EFI_OLD_MEMMAP and EFI_MIXED_MODE

[Prakhya, Sai Praneeth]

> > Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> >
> > with the sidenote that I won't be able to test this myself until
> > monday at the earliest.
> 
> OK, so I have tested both efi=old_map and mixed mode before and after
> applying this patch, using QEMU/KVM with 64-bit and 32-bit builds of OVMF
> [respectively]
> 
> efi=old_map is indeed broken before and needs this patch.
> 
> Mixed mode works just fine both before and after applying the patch, but I
> suggest we keep this patch as-is and address mixed mode later if needed (I
> spotted a couple of things in the boot log that may need some attention but I'm
> not sure if the issue is in Linux or in OVMF)

Thanks for testing this Ard.

For testing Mixed mode, I have used QEMU/KVM with 64-bit and 32-bit OVMF 
and mixed mode is broken for me without this patch.

I am using an older OVMF build and probably that might be the reason for us seeing 
different behaviors. Also, kernel config file might have some role to play.. I guess.

Regards,
Sai