2023-08-11 06:19:34

by Dexuan Cui

[permalink] [raw]
Subject: [PATCH] x86/hyperv: Fix the detection of E820_TYPE_PRAM in a Gen2 VM

A Gen2 VM doesn't support legacy PCI/PCIe, so both raw_pci_ops and
raw_pci_ext_ops are NULL, and pci_subsys_init() -> pcibios_init()
doesn't call pcibios_resource_survey() -> e820__reserve_resources_late();
as a result, any emulated persistent memory of E820_TYPE_PRAM (12) via
the kernel parameter memmap=nn[KMG]!ss is not added into iomem_resource
and hence can't be detected by register_e820_pmem().

Fix this by directly calling e820__reserve_resources_late() in
hv_pci_init(), which is called from arch_initcall(pci_arch_init).

It's ok to move a Gen2 VM's e820__reserve_resources_late() from
subsys_initcall(pci_subsys_init) to arch_initcall(pci_arch_init) because
the code in-between doesn't depend on the E820 resources.
e820__reserve_resources_late() depends on e820__reserve_resources(),
which has been called earlier from setup_arch().

For a Gen-2 VM, the new hv_pci_init() also adds any memory of
E820_TYPE_PMEM (7) into iomem_resource, and acpi_nfit_register_region() ->
acpi_nfit_insert_resource() -> region_intersects() returns
REGION_INTERSECTS, so the memory of E820_TYPE_PMEM won't get added twice.

Changed the local variable "int gen2vm" to "bool gen2vm".

Signed-off-by: Saurabh Sengar <[email protected]>
Signed-off-by: Dexuan Cui <[email protected]>
---
arch/x86/hyperv/hv_init.c | 25 +++++++++++++++++++++----
1 file changed, 21 insertions(+), 4 deletions(-)

diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
index b004370d3b01..6b22d49aee7b 100644
--- a/arch/x86/hyperv/hv_init.c
+++ b/arch/x86/hyperv/hv_init.c
@@ -13,6 +13,7 @@
#include <linux/io.h>
#include <asm/apic.h>
#include <asm/desc.h>
+#include <asm/e820/api.h>
#include <asm/sev.h>
#include <asm/hypervisor.h>
#include <asm/hyperv-tlfs.h>
@@ -282,15 +283,31 @@ static int hv_cpu_die(unsigned int cpu)

static int __init hv_pci_init(void)
{
- int gen2vm = efi_enabled(EFI_BOOT);
+ bool gen2vm = efi_enabled(EFI_BOOT);

/*
- * For Generation-2 VM, we exit from pci_arch_init() by returning 0.
- * The purpose is to suppress the harmless warning:
+ * A Generation-2 VM doesn't support legacy PCI/PCIe, so both
+ * raw_pci_ops and raw_pci_ext_ops are NULL, and pci_subsys_init() ->
+ * pcibios_init() doesn't call pcibios_resource_survey() ->
+ * e820__reserve_resources_late(); as a result, any emulated persistent
+ * memory of E820_TYPE_PRAM (12) via the kernel parameter
+ * memmap=nn[KMG]!ss is not added into iomem_resource and hence can't be
+ * detected by register_e820_pmem(). Fix this by directly calling
+ * e820__reserve_resources_late() here: e820__reserve_resources_late()
+ * depends on e820__reserve_resources(), which has been called earlier
+ * from setup_arch(). Note: e820__reserve_resources_late() also adds
+ * any memory of E820_TYPE_PMEM (7) into iomem_resource, and
+ * acpi_nfit_register_region() -> acpi_nfit_insert_resource() ->
+ * region_intersects() returns REGION_INTERSECTS, so the memory of
+ * E820_TYPE_PMEM won't get added twice.
+ *
+ * We return 0 here so that pci_arch_init() won't print the warning:
* "PCI: Fatal: No config space access function found"
*/
- if (gen2vm)
+ if (gen2vm) {
+ e820__reserve_resources_late();
return 0;
+ }

/* For Generation-1 VM, we'll proceed in pci_arch_init(). */
return 1;
--
2.25.1



2023-09-19 19:31:01

by Saurabh Singh Sengar

[permalink] [raw]
Subject: Re: [PATCH] x86/hyperv: Fix the detection of E820_TYPE_PRAM in a Gen2 VM

On Thu, Aug 10, 2023 at 10:31:37PM -0700, Dexuan Cui wrote:
> A Gen2 VM doesn't support legacy PCI/PCIe, so both raw_pci_ops and
> raw_pci_ext_ops are NULL, and pci_subsys_init() -> pcibios_init()
> doesn't call pcibios_resource_survey() -> e820__reserve_resources_late();
> as a result, any emulated persistent memory of E820_TYPE_PRAM (12) via
> the kernel parameter memmap=nn[KMG]!ss is not added into iomem_resource
> and hence can't be detected by register_e820_pmem().
>
> Fix this by directly calling e820__reserve_resources_late() in
> hv_pci_init(), which is called from arch_initcall(pci_arch_init).
>
> It's ok to move a Gen2 VM's e820__reserve_resources_late() from
> subsys_initcall(pci_subsys_init) to arch_initcall(pci_arch_init) because
> the code in-between doesn't depend on the E820 resources.
> e820__reserve_resources_late() depends on e820__reserve_resources(),
> which has been called earlier from setup_arch().
>
> For a Gen-2 VM, the new hv_pci_init() also adds any memory of
> E820_TYPE_PMEM (7) into iomem_resource, and acpi_nfit_register_region() ->
> acpi_nfit_insert_resource() -> region_intersects() returns
> REGION_INTERSECTS, so the memory of E820_TYPE_PMEM won't get added twice.
>
> Changed the local variable "int gen2vm" to "bool gen2vm".
>
> Signed-off-by: Saurabh Sengar <[email protected]>
> Signed-off-by: Dexuan Cui <[email protected]>
> ---
> arch/x86/hyperv/hv_init.c | 25 +++++++++++++++++++++----
> 1 file changed, 21 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
> index b004370d3b01..6b22d49aee7b 100644
> --- a/arch/x86/hyperv/hv_init.c
> +++ b/arch/x86/hyperv/hv_init.c
> @@ -13,6 +13,7 @@
> #include <linux/io.h>
> #include <asm/apic.h>
> #include <asm/desc.h>
> +#include <asm/e820/api.h>
> #include <asm/sev.h>
> #include <asm/hypervisor.h>
> #include <asm/hyperv-tlfs.h>
> @@ -282,15 +283,31 @@ static int hv_cpu_die(unsigned int cpu)
>
> static int __init hv_pci_init(void)
> {
> - int gen2vm = efi_enabled(EFI_BOOT);
> + bool gen2vm = efi_enabled(EFI_BOOT);
>
> /*
> - * For Generation-2 VM, we exit from pci_arch_init() by returning 0.
> - * The purpose is to suppress the harmless warning:
> + * A Generation-2 VM doesn't support legacy PCI/PCIe, so both
> + * raw_pci_ops and raw_pci_ext_ops are NULL, and pci_subsys_init() ->
> + * pcibios_init() doesn't call pcibios_resource_survey() ->
> + * e820__reserve_resources_late(); as a result, any emulated persistent
> + * memory of E820_TYPE_PRAM (12) via the kernel parameter
> + * memmap=nn[KMG]!ss is not added into iomem_resource and hence can't be
> + * detected by register_e820_pmem(). Fix this by directly calling
> + * e820__reserve_resources_late() here: e820__reserve_resources_late()
> + * depends on e820__reserve_resources(), which has been called earlier
> + * from setup_arch(). Note: e820__reserve_resources_late() also adds
> + * any memory of E820_TYPE_PMEM (7) into iomem_resource, and
> + * acpi_nfit_register_region() -> acpi_nfit_insert_resource() ->
> + * region_intersects() returns REGION_INTERSECTS, so the memory of
> + * E820_TYPE_PMEM won't get added twice.
> + *
> + * We return 0 here so that pci_arch_init() won't print the warning:
> * "PCI: Fatal: No config space access function found"
> */
> - if (gen2vm)
> + if (gen2vm) {
> + e820__reserve_resources_late();
> return 0;
> + }


Kind reminder to review this.

- Saurabh

>
> /* For Generation-1 VM, we'll proceed in pci_arch_init(). */
> return 1;
>
> --
> 2.25.1
>