[PATCH] x86/kexec: Add ACPI NVS region to the ident map

[PATCH] x86/kexec: Add ACPI NVS region to the ident map

[Kairui Song]

With the recent addition of RSDP parsing in decompression stage, kexec
kernel now needs ACPI tables to be covered by the identity mapping.
And in commit 6bbeb276b71f ("x86/kexec: Add the EFI system tables and
ACPI tables to the ident map"), ACPI tables memory region was added to
the ident map.

But on some machines, there is only ACPI NVS memory region, and the ACPI
tables is located in the NVS region instead. In such case second kernel
will still fail when trying to access ACPI tables.

So, to fix the problem, add NVS memory region in the ident map as well.

Fixes: 6bbeb276b71f ("x86/kexec: Add the EFI system tables and ACPI tables to the ident map")
Suggested-by: Junichi Nomura <j-nomura@ce.jp.nec.com>
Signed-off-by: Kairui Song <kasong@redhat.com>
---

Tested with my laptop and VM, on top of current tip:x86/boot.

 arch/x86/kernel/machine_kexec_64.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/machine_kexec_64.c b/arch/x86/kernel/machine_kexec_64.c
index 3c77bdf7b32a..a406602fdb3c 100644
--- a/arch/x86/kernel/machine_kexec_64.c
+++ b/arch/x86/kernel/machine_kexec_64.c
@@ -54,14 +54,26 @@ static int mem_region_callback(struct resource *res, void *arg)
 static int
 map_acpi_tables(struct x86_mapping_info *info, pgd_t *level4p)
 {
-	unsigned long flags = IORESOURCE_MEM | IORESOURCE_BUSY;
+	int ret;
+	unsigned long flags;
 	struct init_pgtable_data data;
 
 	data.info = info;
 	data.level4p = level4p;
 	flags = IORESOURCE_MEM | IORESOURCE_BUSY;
-	return walk_iomem_res_desc(IORES_DESC_ACPI_TABLES, flags, 0, -1,
-				   &data, mem_region_callback);
+
+	ret = walk_iomem_res_desc(IORES_DESC_ACPI_TABLES, flags, 0, -1,
+				  &data, mem_region_callback);
+	if (ret && ret != -EINVAL)
+		return ret;
+
+	/* ACPI tables could be located in ACPI Non-volatile Storage region */
+	ret = walk_iomem_res_desc(IORES_DESC_ACPI_NV_STORAGE, flags, 0, -1,
+				  &data, mem_region_callback);
+	if (ret && ret != -EINVAL)
+		return ret;
+
+	return 0;
 }
 #else
 static int map_acpi_tables(struct x86_mapping_info *info, pgd_t *level4p) { return 0; }
-- 
2.21.0

Re: [PATCH] x86/kexec: Add ACPI NVS region to the ident map

[Borislav Petkov]

On Mon, Jun 10, 2019 at 03:36:17PM +0800, Kairui Song wrote:
> With the recent addition of RSDP parsing in decompression stage, kexec
> kernel now needs ACPI tables to be covered by the identity mapping.
> And in commit 6bbeb276b71f ("x86/kexec: Add the EFI system tables and
> ACPI tables to the ident map"), ACPI tables memory region was added to
> the ident map.
> 
> But on some machines, there is only ACPI NVS memory region, and the ACPI
> tables is located in the NVS region instead. In such case second kernel

*are* located - plural.

> will still fail when trying to access ACPI tables.
> 
> So, to fix the problem, add NVS memory region in the ident map as well.
> 
> Fixes: 6bbeb276b71f ("x86/kexec: Add the EFI system tables and ACPI tables to the ident map")
> Suggested-by: Junichi Nomura <j-nomura@ce.jp.nec.com>
> Signed-off-by: Kairui Song <kasong@redhat.com>
> ---
> 
> Tested with my laptop and VM, on top of current tip:x86/boot.

You tested this in a VM and not on the *actual* machine with the NVS
region?

This is a joke, right?

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Re: [PATCH] x86/kexec: Add ACPI NVS region to the ident map

[Kairui Song]

On Mon, Jun 10, 2019 at 5:52 PM Borislav Petkov <bp@alien8.de> wrote:
>
> On Mon, Jun 10, 2019 at 03:36:17PM +0800, Kairui Song wrote:
> > With the recent addition of RSDP parsing in decompression stage, kexec
> > kernel now needs ACPI tables to be covered by the identity mapping.
> > And in commit 6bbeb276b71f ("x86/kexec: Add the EFI system tables and
> > ACPI tables to the ident map"), ACPI tables memory region was added to
> > the ident map.
> >
> > But on some machines, there is only ACPI NVS memory region, and the ACPI
> > tables is located in the NVS region instead. In such case second kernel
>
> *are* located - plural.
>
> > will still fail when trying to access ACPI tables.
> >
> > So, to fix the problem, add NVS memory region in the ident map as well.
> >
> > Fixes: 6bbeb276b71f ("x86/kexec: Add the EFI system tables and ACPI tables to the ident map")
> > Suggested-by: Junichi Nomura <j-nomura@ce.jp.nec.com>
> > Signed-off-by: Kairui Song <kasong@redhat.com>
> > ---
> >
> > Tested with my laptop and VM, on top of current tip:x86/boot.
>
> You tested this in a VM and not on the *actual* machine with the NVS
> region?
>
> This is a joke, right?
>

Hi Boris, unfortunately I don't have a real machine which only have
the NVS region.
I did fake the memmap to emulate such problem but can't really promise
this will fix the real case.
So just declare it won't break anything that is already working. And
I'm asking Junichi to have a try as he reported this issue on the
machines he has.

-- 
Best Regards,
Kairui Song

Re: [PATCH] x86/kexec: Add ACPI NVS region to the ident map

[Borislav Petkov]

On Mon, Jun 10, 2019 at 06:18:50PM +0800, Kairui Song wrote:
> Hi Boris, unfortunately I don't have a real machine which only have
> the NVS region. I did fake the memmap to emulate such problem but
> can't really promise this will fix the real case. So just declare it
> won't break anything that is already working. And I'm asking Junichi
> to have a try as he reported this issue on the machines he has.

Yes, this is how you should do it. First you test on a real hardware -
if the issue is such that needs a real hardware to verify - and if it
passes, *then* you send the patch.

If you don't have access to the box, then ask someone who has.

But for the future, please do not send untested patches in a hurry,
hoping that they would work. This could cause more trouble than the
little time you might save speculating it'll all go fine.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Re: [PATCH] x86/kexec: Add ACPI NVS region to the ident map

[Junichi Nomura]

Hi Kairui, Boris,

On 6/10/19 7:59 PM, Borislav Petkov wrote:
> On Mon, Jun 10, 2019 at 06:18:50PM +0800, Kairui Song wrote:
>> Hi Boris, unfortunately I don't have a real machine which only have
>> the NVS region. I did fake the memmap to emulate such problem but
>> can't really promise this will fix the real case. So just declare it
>> won't break anything that is already working. And I'm asking Junichi
>> to have a try as he reported this issue on the machines he has.
> 
> Yes, this is how you should do it. First you test on a real hardware -
> if the issue is such that needs a real hardware to verify - and if it
> passes, *then* you send the patch.
> 
> If you don't have access to the box, then ask someone who has.
> 
> But for the future, please do not send untested patches in a hurry,
> hoping that they would work. This could cause more trouble than the
> little time you might save speculating it'll all go fine.

I tested the patch on such a machine on top of the current tip/master
f9818950848 ("Marge branch 'linus'") and it works fine.
Without the patch, kexec -l fails.

I had tested the version I posted here on bunch of physical machines
I had access and confirmed it didn't broke working setups:
https://lore.kernel.org/lkml/20190515051717.GA13703@jeru.linux.bs1.fc.nec.co.jp/

Since the logic in the patch seems unchanged, I'm very sure it's ok.

-- 
Jun'ichi Nomura, NEC Corporation / NEC Solution Innovators, Ltd.

Re: [PATCH] x86/kexec: Add ACPI NVS region to the ident map

[Borislav Petkov]

On Mon, Jun 10, 2019 at 11:07:05AM +0000, Junichi Nomura wrote:
> I had tested the version I posted here on bunch of physical machines
> I had access and confirmed it didn't broke working setups:
> https://lore.kernel.org/lkml/20190515051717.GA13703@jeru.linux.bs1.fc.nec.co.jp/
> 
> Since the logic in the patch seems unchanged, I'm very sure it's ok.

Ok, thanks for testing.

I'll add your Tested-by.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Re: [PATCH] x86/kexec: Add ACPI NVS region to the ident map

[Kairui Song]

On Mon, Jun 10, 2019 at 3:37 PM Kairui Song <kasong@redhat.com> wrote:
>
> With the recent addition of RSDP parsing in decompression stage, kexec
> kernel now needs ACPI tables to be covered by the identity mapping.
> And in commit 6bbeb276b71f ("x86/kexec: Add the EFI system tables and
> ACPI tables to the ident map"), ACPI tables memory region was added to
> the ident map.
>
> But on some machines, there is only ACPI NVS memory region, and the ACPI
> tables is located in the NVS region instead. In such case second kernel
> will still fail when trying to access ACPI tables.
>
> So, to fix the problem, add NVS memory region in the ident map as well.
>
> Fixes: 6bbeb276b71f ("x86/kexec: Add the EFI system tables and ACPI tables to the ident map")
> Suggested-by: Junichi Nomura <j-nomura@ce.jp.nec.com>
> Signed-off-by: Kairui Song <kasong@redhat.com>
> ---
>
> Tested with my laptop and VM, on top of current tip:x86/boot.
>
>  arch/x86/kernel/machine_kexec_64.c | 18 +++++++++++++++---
>  1 file changed, 15 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/kernel/machine_kexec_64.c b/arch/x86/kernel/machine_kexec_64.c
> index 3c77bdf7b32a..a406602fdb3c 100644
> --- a/arch/x86/kernel/machine_kexec_64.c
> +++ b/arch/x86/kernel/machine_kexec_64.c
> @@ -54,14 +54,26 @@ static int mem_region_callback(struct resource *res, void *arg)
>  static int
>  map_acpi_tables(struct x86_mapping_info *info, pgd_t *level4p)
>  {
> -       unsigned long flags = IORESOURCE_MEM | IORESOURCE_BUSY;
> +       int ret;
> +       unsigned long flags;
>         struct init_pgtable_data data;
>
>         data.info = info;
>         data.level4p = level4p;
>         flags = IORESOURCE_MEM | IORESOURCE_BUSY;
> -       return walk_iomem_res_desc(IORES_DESC_ACPI_TABLES, flags, 0, -1,
> -                                  &data, mem_region_callback);
> +
> +       ret = walk_iomem_res_desc(IORES_DESC_ACPI_TABLES, flags, 0, -1,
> +                                 &data, mem_region_callback);
> +       if (ret && ret != -EINVAL)
> +               return ret;
> +
> +       /* ACPI tables could be located in ACPI Non-volatile Storage region */
> +       ret = walk_iomem_res_desc(IORES_DESC_ACPI_NV_STORAGE, flags, 0, -1,
> +                                 &data, mem_region_callback);
> +       if (ret && ret != -EINVAL)
> +               return ret;
> +
> +       return 0;
>  }
>  #else
>  static int map_acpi_tables(struct x86_mapping_info *info, pgd_t *level4p) { return 0; }
> --
> 2.21.0
>

Hi, could you help test the tip branch with this applied? This should
fix all the issues, I can't find any other issues now. Thanks.


--
Best Regards,
Kairui Song

[tip:x86/boot] x86/kexec: Add the ACPI NVS region to the ident map

[tip-bot for Kairui Song]

Commit-ID:  5a949b38839e284b1307540c56b03caf57da9736
Gitweb:     https://git.kernel.org/tip/5a949b38839e284b1307540c56b03caf57da9736
Author:     Kairui Song <kasong@redhat.com>
AuthorDate: Mon, 10 Jun 2019 15:36:17 +0800
Committer:  Borislav Petkov <bp@suse.de>
CommitDate: Mon, 10 Jun 2019 22:00:26 +0200

x86/kexec: Add the ACPI NVS region to the ident map

With the recent addition of RSDP parsing in the decompression stage,
a kexec-ed kernel now needs ACPI tables to be covered by the identity
mapping. And in commit

  6bbeb276b71f ("x86/kexec: Add the EFI system tables and ACPI tables to the ident map")

the ACPI tables memory region was added to the ident map.

But some machines have only an ACPI NVS memory region and the ACPI
tables are located in that region. In such case, the kexec-ed kernel
will still fail when trying to access ACPI tables if they're not mapped.

So add the NVS memory region to the ident map as well.

 [ bp: Massage. ]

Fixes: 6bbeb276b71f ("x86/kexec: Add the EFI system tables and ACPI tables to the ident map")
Suggested-by: Junichi Nomura <j-nomura@ce.jp.nec.com>
Signed-off-by: Kairui Song <kasong@redhat.com>
Signed-off-by: Borislav Petkov <bp@suse.de>
Tested-by: Junichi Nomura <j-nomura@ce.jp.nec.com>
Cc: Baoquan He <bhe@redhat.com>
Cc: Chao Fan <fanc.fnst@cn.fujitsu.com>
Cc: Dave Young <dyoung@redhat.com>
Cc: Dirk van der Merwe <dirk.vandermerwe@netronome.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: kexec@lists.infradead.org
Cc: Lianbo Jiang <lijiang@redhat.com>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: x86-ml <x86@kernel.org>
Link: https://lkml.kernel.org/r/20190610073617.19767-1-kasong@redhat.com
---
 arch/x86/kernel/machine_kexec_64.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/machine_kexec_64.c b/arch/x86/kernel/machine_kexec_64.c
index 3c77bdf7b32a..b2b88dcaaf88 100644
--- a/arch/x86/kernel/machine_kexec_64.c
+++ b/arch/x86/kernel/machine_kexec_64.c
@@ -54,14 +54,26 @@ static int mem_region_callback(struct resource *res, void *arg)
 static int
 map_acpi_tables(struct x86_mapping_info *info, pgd_t *level4p)
 {
-	unsigned long flags = IORESOURCE_MEM | IORESOURCE_BUSY;
 	struct init_pgtable_data data;
+	unsigned long flags;
+	int ret;
 
 	data.info = info;
 	data.level4p = level4p;
 	flags = IORESOURCE_MEM | IORESOURCE_BUSY;
-	return walk_iomem_res_desc(IORES_DESC_ACPI_TABLES, flags, 0, -1,
-				   &data, mem_region_callback);
+
+	ret = walk_iomem_res_desc(IORES_DESC_ACPI_TABLES, flags, 0, -1,
+				  &data, mem_region_callback);
+	if (ret && ret != -EINVAL)
+		return ret;
+
+	/* ACPI tables could be located in ACPI Non-volatile Storage region */
+	ret = walk_iomem_res_desc(IORES_DESC_ACPI_NV_STORAGE, flags, 0, -1,
+				  &data, mem_region_callback);
+	if (ret && ret != -EINVAL)
+		return ret;
+
+	return 0;
 }
 #else
 static int map_acpi_tables(struct x86_mapping_info *info, pgd_t *level4p) { return 0; }