[RFC v2 0/2] hw/arm/virt-acpi-build: Add IORT RMR regions to handle MSI nested binding

[RFC v2 0/2] hw/arm/virt-acpi-build: Add IORT RMR regions to handle MSI nested binding

[Eric Auger]

To handle SMMUv3 nested stage support it is practical to
expose the guest with reserved memory regions (RMRs)
covering the IOVAs used by the host kernel to map
physical MSI doorbells.

Those IOVAs belong to [0x8000000, 0x8100000] matching
MSI_IOVA_BASE and MSI_IOVA_LENGTH definitions in kernel
arm-smmu-v3 driver. This is the window used to allocate
IOVAs matching physical MSI doorbells.

With those RMRs, the guest is forced to use a flat mapping
for this range. Hence the assigned device is programmed
with one IOVA from this range. Stage 1, owned by the guest
has a flat mapping for this IOVA. Stage2, owned by the VMM
then enforces a mapping from this IOVA to the physical
MSI doorbell.

At IORT table level, due to the single mapping flag being
set on the ID mapping, 256 IORT RMR nodes need to be
created per bus. This looks awkward from a specification
and implementation point of view.

This may also produce a warning at execution time:
qemu-system-aarch64: warning: ACPI table size 114709 exceeds
65536 bytes, migration may not work
(here with 5 pcie root ports, ie. 256 * 6 = 1536 RMR nodes!).

The creation of those RMR nodes only is relevant if nested
stage SMMU is in use, along with VFIO. As VFIO devices can be
hotplugged, all RMRs need to be created in advance. Hence
the patch introduces a new arm virt "nested-smmuv3" iommu type.

ARM DEN 0049E.b IORT specification also mandates that when
RMRs are present, the OS must preserve PCIe configuration
performed by the boot FW. So along with the RMR IORT nodes,
a _DSM function #5, as defined by PCI FIRMWARE SPECIFICATION
EVISION 3.3, chapter 4.6.5 is added to PCIe host bridge
and PCIe expander bridge objects.

The series applies on top of Igor's
[1] [PATCH v4 00/35] acpi: refactor error prone build_header() and
packed structures usage in ACPI tables and
[2] [PATCH v2 0/3] hw/arm/virt_acpi_build: Upgrate the IORT table up
  to revision E.b

The guest can use RMRs with Shameer's series:
[3] [PATCH v7 0/9] ACPI/IORT: Support for IORT RMR node

The latest IORT specification (ARM DEN 0049E.b) can be found at
IO Remapping Table - Platform Design Document
https://developer.arm.com/documentation/den0049/latest/

This series and its dependency can be found at
https://github.com/eauger/qemu.git
branch: igor_acpi_refactoring_v4_dbg2_v3_rmr_v2

History:
v1 -> v2:
- add DSM #5

Eric Auger (2):
  hw/pci-host/gpex: Allow to generate preserve boot config DSM #5
  hw/arm/virt-acpi-build: Add IORT RMR regions to handle MSI nested
    binding

 include/hw/arm/virt.h      |  7 ++++
 include/hw/pci-host/gpex.h |  1 +
 hw/arm/virt-acpi-build.c   | 84 ++++++++++++++++++++++++++++++++------
 hw/arm/virt.c              |  7 +++-
 hw/pci-host/gpex-acpi.c    | 12 ++++++
 5 files changed, 98 insertions(+), 13 deletions(-)

-- 
2.26.3

[RFC v2 1/2] hw/pci-host/gpex: Allow to generate preserve boot config DSM #5

[Eric Auger]

Add a 'preserve_config' field in struct GPEXConfig and
if set generate the DSM #5 for preserving PCI boot configurations.
The DSM presence is needed to expose RMRs.

At the moment the DSM generation is not yet enabled.

Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
 include/hw/pci-host/gpex.h |  1 +
 hw/pci-host/gpex-acpi.c    | 12 ++++++++++++
 2 files changed, 13 insertions(+)

diff --git a/include/hw/pci-host/gpex.h b/include/hw/pci-host/gpex.h
index fcf8b63820..3f8f8ec38d 100644
--- a/include/hw/pci-host/gpex.h
+++ b/include/hw/pci-host/gpex.h
@@ -64,6 +64,7 @@ struct GPEXConfig {
     MemMapEntry pio;
     int         irq;
     PCIBus      *bus;
+    bool        preserve_config;
 };
 
 int gpex_set_irq_num(GPEXHost *s, int index, int gsi);
diff --git a/hw/pci-host/gpex-acpi.c b/hw/pci-host/gpex-acpi.c
index e7e162a00a..7dab259379 100644
--- a/hw/pci-host/gpex-acpi.c
+++ b/hw/pci-host/gpex-acpi.c
@@ -164,6 +164,12 @@ void acpi_dsdt_add_gpex(Aml *scope, struct GPEXConfig *cfg)
                 aml_append(dev, aml_name_decl("_PXM", aml_int(numa_node)));
             }
 
+            if (cfg->preserve_config) {
+                method = aml_method("_DSM", 5, AML_SERIALIZED);
+                aml_append(method, aml_return(aml_int(0)));
+                aml_append(dev, method);
+            }
+
             acpi_dsdt_add_pci_route_table(dev, cfg->irq);
 
             /*
@@ -191,6 +197,12 @@ void acpi_dsdt_add_gpex(Aml *scope, struct GPEXConfig *cfg)
     aml_append(dev, aml_name_decl("_STR", aml_unicode("PCIe 0 Device")));
     aml_append(dev, aml_name_decl("_CCA", aml_int(1)));
 
+    if (cfg->preserve_config) {
+        method = aml_method("_DSM", 5, AML_SERIALIZED);
+        aml_append(method, aml_return(aml_int(0)));
+        aml_append(dev, method);
+    }
+
     acpi_dsdt_add_pci_route_table(dev, cfg->irq);
 
     method = aml_method("_CBA", 0, AML_NOTSERIALIZED);
-- 
2.26.3

[RFC v2 2/2] hw/arm/virt-acpi-build: Add IORT RMR regions to handle MSI nested binding

[Eric Auger]

To handle SMMUv3 nested stage support it is practical to
expose the guest with reserved memory regions (RMRs)
covering the IOVAs used by the host kernel to map
physical MSI doorbells.

Those IOVAs belong to [0x8000000, 0x8100000] matching
MSI_IOVA_BASE and MSI_IOVA_LENGTH definitions in kernel
arm-smmu-v3 driver. This is the window used to allocate
IOVAs matching physical MSI doorbells.

With those RMRs, the guest is forced to use a flat mapping
for this range. Hence the assigned device is programmed
with one IOVA from this range. Stage 1, owned by the guest
has a flat mapping for this IOVA. Stage2, owned by the VMM
then enforces a mapping from this IOVA to the physical
MSI doorbell.

At IORT table level, due to the single mapping flag being
set on the ID mapping, 256 IORT RMR nodes need to be
created per bus. This looks awkward from a specification
and implementation point of view.

This may also produce a warning at execution time:
qemu-system-aarch64: warning: ACPI table size 114709 exceeds
65536 bytes, migration may not work
(here with 5 pcie root ports, ie. 256 * 6 = 1536 RMR nodes!).

The creation of those RMR nodes only is relevant if nested
stage SMMU is in use, along with VFIO. As VFIO devices can be
hotplugged, all RMRs need to be created in advance. Hence
the patch introduces a new arm virt "nested-smmuv3" iommu type.

ARM DEN 0049E.b IORT specification also mandates that when
RMRs are present, the OS must preserve PCIe configuration
performed by the boot FW. So along with the RMR IORT nodes,
a _DSM function #5, as defined by PCI FIRMWARE SPECIFICATION
EVISION 3.3, chapter 4.6.5 is added to PCIe host bridge
and PCIe expander bridge objects.

Signed-off-by: Eric Auger <eric.auger@redhat.com>
Suggested-by: Jean-Philippe Brucker <jean-philippe@linaro.org>

---

v1 -> v2:
- add DSM #5
- use identifier increment

Instead of introducing a new IOMMU type, we could introduce
an array of qdev_prop_reserved_region(s).

Guest can parse the IORT RMR nodes with Shammer's series:
[PATCH v7 0/9] ACPI/IORT: Support for IORT RMR node

The patch applies on Igor's v4 series [1]+ IORT E.b upgrade [2]
[1] [PATCH v4 00/35] acpi: refactor error prone build_header()
    and packed structures usage in ACPI tables
[2] [PATCH 0/3] hw/arm/virt_acpi_build: Upgrate the IORT table
    up to revision E.b
---
 include/hw/arm/virt.h    |  7 ++++
 hw/arm/virt-acpi-build.c | 84 ++++++++++++++++++++++++++++++++++------
 hw/arm/virt.c            |  7 +++-
 3 files changed, 85 insertions(+), 13 deletions(-)

diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
index b461b8d261..f2f8aee219 100644
--- a/include/hw/arm/virt.h
+++ b/include/hw/arm/virt.h
@@ -99,6 +99,7 @@ enum {
 typedef enum VirtIOMMUType {
     VIRT_IOMMU_NONE,
     VIRT_IOMMU_SMMUV3,
+    VIRT_IOMMU_NESTED_SMMUV3,
     VIRT_IOMMU_VIRTIO,
 } VirtIOMMUType;
 
@@ -190,4 +191,10 @@ static inline int virt_gicv3_redist_region_count(VirtMachineState *vms)
     return MACHINE(vms)->smp.cpus > redist0_capacity ? 2 : 1;
 }
 
+static inline bool virt_has_smmuv3(const VirtMachineState *vms)
+{
+    return vms->iommu == VIRT_IOMMU_SMMUV3 ||
+           vms->iommu == VIRT_IOMMU_NESTED_SMMUV3;
+}
+
 #endif /* QEMU_ARM_VIRT_H */
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 789bac3134..7260e47c83 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -169,6 +169,14 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap,
         .bus    = vms->bus,
     };
 
+    /*
+     * Nested SMMU requires RMRs for MSI 1-1 mapping, which
+     * require _DSM for PreservingPCI Boot Configurations
+     */
+    if (vms->iommu == VIRT_IOMMU_NESTED_SMMUV3) {
+        cfg.preserve_config = true;
+    }
+
     if (use_highmem) {
         cfg.mmio64 = memmap[VIRT_HIGH_PCIE_MMIO];
     }
@@ -245,16 +253,16 @@ static void acpi_dsdt_add_tpm(Aml *scope, VirtMachineState *vms)
 #define ROOT_COMPLEX_ENTRY_SIZE 36
 #define IORT_NODE_OFFSET 48
 
-static void build_iort_id_mapping(GArray *table_data, uint32_t input_base,
-                                  uint32_t id_count, uint32_t out_ref)
+static void
+build_iort_id_mapping(GArray *table_data, uint32_t input_base,
+                      uint32_t id_count, uint32_t out_ref, uint32_t flags)
 {
     /* Table 4 ID mapping format */
     build_append_int_noprefix(table_data, input_base, 4); /* Input base */
     build_append_int_noprefix(table_data, id_count, 4); /* Number of IDs */
     build_append_int_noprefix(table_data, input_base, 4); /* Output base */
     build_append_int_noprefix(table_data, out_ref, 4); /* Output Reference */
-    /* Flags */
-    build_append_int_noprefix(table_data, 0 /* Single mapping */, 4);
+    build_append_int_noprefix(table_data, flags, 4); /* Flags */
 }
 
 struct AcpiIortIdMapping {
@@ -296,6 +304,50 @@ static int iort_idmap_compare(gconstpointer a, gconstpointer b)
     return idmap_a->input_base - idmap_b->input_base;
 }
 
+static void
+build_iort_rmr_nodes(GArray *table_data, GArray *smmu_idmaps, int smmu_offset, uint32_t *id) {
+    AcpiIortIdMapping *range;
+    int i, j;
+
+    for (i = 0; i < smmu_idmaps->len; i++) {
+        range = &g_array_index(smmu_idmaps, AcpiIortIdMapping, i);
+        for (j = 0; j < range->id_count; j++) {
+            int bdf = range->input_base + j;
+
+            /* Table 18 Reserved Memory Range Node */
+
+            build_append_int_noprefix(table_data, 6 /* RMR */, 1); /* Type */
+            /* Length */
+            build_append_int_noprefix(table_data, 28 + ID_MAPPING_ENTRY_SIZE + 20, 2);
+            build_append_int_noprefix(table_data, 1, 1); /* Revision */
+            build_append_int_noprefix(table_data, *id, 4); /* Identifier */
+            /* Number of ID mappings */
+            build_append_int_noprefix(table_data, 1, 4);
+            /* Reference to ID Array */
+            build_append_int_noprefix(table_data, 28, 4);
+
+            /* RMR specific data */
+
+            /* Flags */
+            build_append_int_noprefix(table_data, 0 /* Disallow remapping */, 4);
+            /* Number of Memory Range Descriptors */
+            build_append_int_noprefix(table_data, 1 , 4);
+            /* Reference to Memory Range Descriptors */
+            build_append_int_noprefix(table_data, 28 + ID_MAPPING_ENTRY_SIZE, 4);
+            build_iort_id_mapping(table_data, bdf, 1, smmu_offset, 1);
+
+            /* Table 19 Memory Range Descriptor */
+
+            /* Physical Range offset */
+            build_append_int_noprefix(table_data, 0x8000000, 8);
+            /* Physical Range length */
+            build_append_int_noprefix(table_data, 0x100000, 8);
+            build_append_int_noprefix(table_data, 0, 4); /* Reserved */
+	    *id += 1;
+        }
+    }
+}
+
 /*
  * Input Output Remapping Table (IORT)
  * Conforms to "IO Remapping Table System Software on ARM Platforms",
@@ -317,12 +369,14 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
     /* Table 2 The IORT */
     acpi_table_begin(&table, table_data);
 
-    if (vms->iommu == VIRT_IOMMU_SMMUV3) {
+    if (virt_has_smmuv3(vms)) {
         AcpiIortIdMapping next_range = {0};
 
         object_child_foreach_recursive(object_get_root(),
                                        iort_host_bridges, smmu_idmaps);
 
+        nb_nodes = 3; /* RC, ITS, SMMUv3 */
+
         /* Sort the smmu idmap by input_base */
         g_array_sort(smmu_idmaps, iort_idmap_compare);
 
@@ -339,6 +393,9 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
             }
 
             next_range.input_base = idmap->input_base + idmap->id_count;
+            if (vms->iommu == VIRT_IOMMU_NESTED_SMMUV3) {
+                nb_nodes += idmap->id_count;
+            }
         }
 
         /* Append the last RC -> ITS ID mapping */
@@ -347,7 +404,6 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
             g_array_append_val(its_idmaps, next_range);
         }
 
-        nb_nodes = 3; /* RC, ITS, SMMUv3 */
         rc_mapping_count = smmu_idmaps->len + its_idmaps->len;
     } else {
         nb_nodes = 2; /* RC, ITS */
@@ -372,7 +428,7 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
     /* GIC ITS Identifier Array */
     build_append_int_noprefix(table_data, 0 /* MADT translation_id */, 4);
 
-    if (vms->iommu == VIRT_IOMMU_SMMUV3) {
+    if (virt_has_smmuv3(vms)) {
         int irq =  vms->irqmap[VIRT_SMMU] + ARM_SPI_BASE;
 
         smmu_offset = table_data->len - table.table_offset;
@@ -402,7 +458,7 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
         build_append_int_noprefix(table_data, 0, 4);
 
         /* output IORT node is the ITS group node (the first node) */
-        build_iort_id_mapping(table_data, 0, 0xFFFF, IORT_NODE_OFFSET);
+        build_iort_id_mapping(table_data, 0, 0xFFFF, IORT_NODE_OFFSET, 0);
     }
 
     /* Table 17 Root Complex Node */
@@ -435,7 +491,7 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
     build_append_int_noprefix(table_data, 0, 3); /* Reserved */
 
     /* Output Reference */
-    if (vms->iommu == VIRT_IOMMU_SMMUV3) {
+    if (virt_has_smmuv3(vms)) {
         AcpiIortIdMapping *range;
 
         /* translated RIDs connect to SMMUv3 node: RC -> SMMUv3 -> ITS */
@@ -443,7 +499,7 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
             range = &g_array_index(smmu_idmaps, AcpiIortIdMapping, i);
             /* output IORT node is the smmuv3 node */
             build_iort_id_mapping(table_data, range->input_base,
-                                  range->id_count, smmu_offset);
+                                  range->id_count, smmu_offset, 0);
         }
 
         /* bypassed RIDs connect to ITS group node directly: RC -> ITS */
@@ -451,11 +507,15 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
             range = &g_array_index(its_idmaps, AcpiIortIdMapping, i);
             /* output IORT node is the ITS group node (the first node) */
             build_iort_id_mapping(table_data, range->input_base,
-                                  range->id_count, iort_node_offset);
+                                  range->id_count, iort_node_offset, 0);
         }
     } else {
         /* output IORT node is the ITS group node (the first node) */
-        build_iort_id_mapping(table_data, 0, 0xFFFF, IORT_NODE_OFFSET);
+        build_iort_id_mapping(table_data, 0, 0xFFFF, IORT_NODE_OFFSET, 0);
+    }
+
+    if (vms->iommu == VIRT_IOMMU_NESTED_SMMUV3) {
+        build_iort_rmr_nodes(table_data, smmu_idmaps, smmu_offset, &id);
     }
 
     acpi_table_end(linker, &table);
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 1d59f0e59f..f538611f4e 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1251,7 +1251,7 @@ static void create_smmu(const VirtMachineState *vms,
     DeviceState *dev;
     MachineState *ms = MACHINE(vms);
 
-    if (vms->iommu != VIRT_IOMMU_SMMUV3 || !vms->iommu_phandle) {
+    if (!virt_has_smmuv3(vms) || !vms->iommu_phandle) {
         return;
     }
 
@@ -1441,6 +1441,7 @@ static void create_pcie(VirtMachineState *vms)
 
         switch (vms->iommu) {
         case VIRT_IOMMU_SMMUV3:
+        case VIRT_IOMMU_NESTED_SMMUV3:
             create_smmu(vms, vms->bus);
             qemu_fdt_setprop_cells(ms->fdt, nodename, "iommu-map",
                                    0x0, vms->iommu_phandle, 0x0, 0x10000);
@@ -2314,6 +2315,8 @@ static char *virt_get_iommu(Object *obj, Error **errp)
         return g_strdup("none");
     case VIRT_IOMMU_SMMUV3:
         return g_strdup("smmuv3");
+    case VIRT_IOMMU_NESTED_SMMUV3:
+        return g_strdup("nested-smmuv3");
     default:
         g_assert_not_reached();
     }
@@ -2325,6 +2328,8 @@ static void virt_set_iommu(Object *obj, const char *value, Error **errp)
 
     if (!strcmp(value, "smmuv3")) {
         vms->iommu = VIRT_IOMMU_SMMUV3;
+    } else if (!strcmp(value, "nested-smmuv3")) {
+        vms->iommu = VIRT_IOMMU_NESTED_SMMUV3;
     } else if (!strcmp(value, "none")) {
         vms->iommu = VIRT_IOMMU_NONE;
     } else {
-- 
2.26.3

Re: [RFC v2 1/2] hw/pci-host/gpex: Allow to generate preserve boot config DSM #5

[Igor Mammedov]

On Tue,  5 Oct 2021 10:53:12 +0200
Eric Auger <eric.auger@redhat.com> wrote:

> Add a 'preserve_config' field in struct GPEXConfig and
> if set generate the DSM #5 for preserving PCI boot configurations.
> The DSM presence is needed to expose RMRs.

here should be pointers to spec and location within it
where it says preserving PCI boot configuration is necessary
or in absence of that a bit more detailed explanation
why it's necessary.

> 
> At the moment the DSM generation is not yet enabled.
> 
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> ---
>  include/hw/pci-host/gpex.h |  1 +
>  hw/pci-host/gpex-acpi.c    | 12 ++++++++++++
>  2 files changed, 13 insertions(+)
> 
> diff --git a/include/hw/pci-host/gpex.h b/include/hw/pci-host/gpex.h
> index fcf8b63820..3f8f8ec38d 100644
> --- a/include/hw/pci-host/gpex.h
> +++ b/include/hw/pci-host/gpex.h
> @@ -64,6 +64,7 @@ struct GPEXConfig {
>      MemMapEntry pio;
>      int         irq;
>      PCIBus      *bus;
> +    bool        preserve_config;
s/^^^/preserve_fw_config/

>  };
>  
>  int gpex_set_irq_num(GPEXHost *s, int index, int gsi);
> diff --git a/hw/pci-host/gpex-acpi.c b/hw/pci-host/gpex-acpi.c
> index e7e162a00a..7dab259379 100644
> --- a/hw/pci-host/gpex-acpi.c
> +++ b/hw/pci-host/gpex-acpi.c
> @@ -164,6 +164,12 @@ void acpi_dsdt_add_gpex(Aml *scope, struct GPEXConfig *cfg)
>                  aml_append(dev, aml_name_decl("_PXM", aml_int(numa_node)));
>              }
>  


> +            if (cfg->preserve_config) {
> +                method = aml_method("_DSM", 5, AML_SERIALIZED);
> +                aml_append(method, aml_return(aml_int(0)));
> +                aml_append(dev, method);
> +            }
> +
>              acpi_dsdt_add_pci_route_table(dev, cfg->irq);
>  
>              /*
> @@ -191,6 +197,12 @@ void acpi_dsdt_add_gpex(Aml *scope, struct GPEXConfig *cfg)
>      aml_append(dev, aml_name_decl("_STR", aml_unicode("PCIe 0 Device")));
>      aml_append(dev, aml_name_decl("_CCA", aml_int(1)));
>  
> +    if (cfg->preserve_config) {
> +        method = aml_method("_DSM", 5, AML_SERIALIZED);
> +        aml_append(method, aml_return(aml_int(0)));
> +        aml_append(dev, method);
> +    }
> +


these ones seem to wrong ,
it adds duplicate _DSM methods with wrong ARGs number.

virt board already has _DSM defined, see

  acpi_dsdt_add_pci_osc()
    E5C937D0-3553-4D7A-9117-EA4D19C3434D

you need to modify that one (and possibly move out DSM into a separate function),
also preserving config might regress what commit 40c3472a29c9a was fixing.


>      acpi_dsdt_add_pci_route_table(dev, cfg->irq);
>  
>      method = aml_method("_CBA", 0, AML_NOTSERIALIZED);

Re: [RFC v2 1/2] hw/pci-host/gpex: Allow to generate preserve boot config DSM #5

[chenxiang (M) via]

Hi Eric,


在 2021/10/5 16:53, Eric Auger 写道:
> Add a 'preserve_config' field in struct GPEXConfig and
> if set generate the DSM #5 for preserving PCI boot configurations.
> The DSM presence is needed to expose RMRs.
>
> At the moment the DSM generation is not yet enabled.
>
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> ---
>   include/hw/pci-host/gpex.h |  1 +
>   hw/pci-host/gpex-acpi.c    | 12 ++++++++++++
>   2 files changed, 13 insertions(+)
>
> diff --git a/include/hw/pci-host/gpex.h b/include/hw/pci-host/gpex.h
> index fcf8b63820..3f8f8ec38d 100644
> --- a/include/hw/pci-host/gpex.h
> +++ b/include/hw/pci-host/gpex.h
> @@ -64,6 +64,7 @@ struct GPEXConfig {
>       MemMapEntry pio;
>       int         irq;
>       PCIBus      *bus;
> +    bool        preserve_config;
>   };
>   
>   int gpex_set_irq_num(GPEXHost *s, int index, int gsi);
> diff --git a/hw/pci-host/gpex-acpi.c b/hw/pci-host/gpex-acpi.c
> index e7e162a00a..7dab259379 100644
> --- a/hw/pci-host/gpex-acpi.c
> +++ b/hw/pci-host/gpex-acpi.c
> @@ -164,6 +164,12 @@ void acpi_dsdt_add_gpex(Aml *scope, struct GPEXConfig *cfg)
>                   aml_append(dev, aml_name_decl("_PXM", aml_int(numa_node)));
>               }
>   
> +            if (cfg->preserve_config) {
> +                method = aml_method("_DSM", 5, AML_SERIALIZED);

I notice there is a ACPI BIOS Error when booting virtual machine which 
seems be caused by this patch as I add this patchset in my branch to 
test the function of vsmmu.
It seems that it requires only 4 parameter for method _DSM, but use 5 
parameters here.
The error log is as following:

[    2.355459] ACPI BIOS Error (bug): Failure creating named object 
[\_SB.PCI0._DSM], AE_ALREADY_EXISTS (20210930/dswload2-327)
[    2.355467] ACPI Error: AE_ALREADY_EXISTS, During name lookup/catalog 
(20210930/psobject-221)
[    2.355470] ACPI: Skipping parse of AML opcode: OpcodeName 
unavailable (0x0014)
[    2.355657] ACPI: 1 ACPI AML tables successfully acquired and loaded
[    2.356321] ACPI: Interpreter enabled
[    2.356323] ACPI: Using GIC for interrupt routing
[    2.356333] ACPI: MCFG table detected, 1 entries
[    2.361359] ARMH0011:00: ttyAMA0 at MMIO 0x9000000 (irq = 16, 
base_baud = 0) is a SBSA
[    2.619805] printk: console [ttyAMA0] enabled
[    2.622114] ACPI: PCI Root Bridge [PCI0] (domain 0000 [bus 00-ff])
[    2.622788] acpi PNP0A08:00: _OSC: OS supports [ExtendedConfig ASPM 
ClockPM Segments MSI HPX-Type3]
[    2.623776] acpi PNP0A08:00: _OSC: platform does not support [LTR]
[    2.624600] acpi PNP0A08:00: _OSC: OS now controls [PCIeHotplug PME 
AER PCIeCapability]
[    2.625721] acpi PNP0A08:00: ECAM area [mem 
0x4010000000-0x401fffffff] reserved by PNP0C02:00
[    2.626645] acpi PNP0A08:00: ECAM at [mem 0x4010000000-0x401fffffff] 
for [bus 00-ff]
[    2.627450] ACPI: Remapped I/O 0x000000003eff0000 to [io 
0x0000-0xffff window]
[    2.628229] ACPI BIOS Error (bug): \_SB.PCI0._DSM: Excess arguments - 
ASL declared 5, ACPI requires 4 (20210930/nsarguments-166)
[    2.629576] PCI host bridge to bus 0000:00
[    2.630008] pci_bus 0000:00: root bus resource [mem 
0x10000000-0x3efeffff window]
[    2.630747] pci_bus 0000:00: root bus resource [io  0x0000-0xffff window]
[    2.631405] pci_bus 0000:00: root bus resource [mem 
0x8000000000-0xffffffffff window]
[    2.632177] pci_bus 0000:00: root bus resource [bus 00-ff]
[    2.632731] ACPI BIOS Error (bug): \_SB.PCI0._DSM: Excess arguments - 
ASL declared 5, ACPI requires 4 (20210930/nsarguments-166)


> +                aml_append(method, aml_return(aml_int(0)));
> +                aml_append(dev, method);
> +            }
> +
>               acpi_dsdt_add_pci_route_table(dev, cfg->irq);
>   
>               /*
> @@ -191,6 +197,12 @@ void acpi_dsdt_add_gpex(Aml *scope, struct GPEXConfig *cfg)
>       aml_append(dev, aml_name_decl("_STR", aml_unicode("PCIe 0 Device")));
>       aml_append(dev, aml_name_decl("_CCA", aml_int(1)));
>   
> +    if (cfg->preserve_config) {
> +        method = aml_method("_DSM", 5, AML_SERIALIZED);
> +        aml_append(method, aml_return(aml_int(0)));
> +        aml_append(dev, method);
> +    }
> +
>       acpi_dsdt_add_pci_route_table(dev, cfg->irq);
>   
>       method = aml_method("_CBA", 0, AML_NOTSERIALIZED);

Re: [RFC v2 1/2] hw/pci-host/gpex: Allow to generate preserve boot config DSM #5

[Eric Auger]

Hi CHenxiangn

On 12/29/21 8:13 AM, chenxiang (M) via wrote:
> Hi Eric,
>
>
> 在 2021/10/5 16:53, Eric Auger 写道:
>> Add a 'preserve_config' field in struct GPEXConfig and
>> if set generate the DSM #5 for preserving PCI boot configurations.
>> The DSM presence is needed to expose RMRs.
>>
>> At the moment the DSM generation is not yet enabled.
>>
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>> ---
>>   include/hw/pci-host/gpex.h |  1 +
>>   hw/pci-host/gpex-acpi.c    | 12 ++++++++++++
>>   2 files changed, 13 insertions(+)
>>
>> diff --git a/include/hw/pci-host/gpex.h b/include/hw/pci-host/gpex.h
>> index fcf8b63820..3f8f8ec38d 100644
>> --- a/include/hw/pci-host/gpex.h
>> +++ b/include/hw/pci-host/gpex.h
>> @@ -64,6 +64,7 @@ struct GPEXConfig {
>>       MemMapEntry pio;
>>       int         irq;
>>       PCIBus      *bus;
>> +    bool        preserve_config;
>>   };
>>     int gpex_set_irq_num(GPEXHost *s, int index, int gsi);
>> diff --git a/hw/pci-host/gpex-acpi.c b/hw/pci-host/gpex-acpi.c
>> index e7e162a00a..7dab259379 100644
>> --- a/hw/pci-host/gpex-acpi.c
>> +++ b/hw/pci-host/gpex-acpi.c
>> @@ -164,6 +164,12 @@ void acpi_dsdt_add_gpex(Aml *scope, struct
>> GPEXConfig *cfg)
>>                   aml_append(dev, aml_name_decl("_PXM",
>> aml_int(numa_node)));
>>               }
>>   +            if (cfg->preserve_config) {
>> +                method = aml_method("_DSM", 5, AML_SERIALIZED);
>
> I notice there is a ACPI BIOS Error when booting virtual machine which
> seems be caused by this patch as I add this patchset in my branch to
> test the function of vsmmu.
> It seems that it requires only 4 parameter for method _DSM, but use 5
> parameters here.
> The error log is as following:

Thank you for the heads up. Yes the problem was reported by Igor too in
https://www.mail-archive.com/qemu-devel@nongnu.org/msg842972.html.

At the moment the RMRR ACPI situation has not progressed on spec side or
kernel if I have not missed anything but sure I will take this into
account in my next respin.

Thanks!

Eric
>
> [    2.355459] ACPI BIOS Error (bug): Failure creating named object
> [\_SB.PCI0._DSM], AE_ALREADY_EXISTS (20210930/dswload2-327)
> [    2.355467] ACPI Error: AE_ALREADY_EXISTS, During name
> lookup/catalog (20210930/psobject-221)
> [    2.355470] ACPI: Skipping parse of AML opcode: OpcodeName
> unavailable (0x0014)
> [    2.355657] ACPI: 1 ACPI AML tables successfully acquired and loaded
> [    2.356321] ACPI: Interpreter enabled
> [    2.356323] ACPI: Using GIC for interrupt routing
> [    2.356333] ACPI: MCFG table detected, 1 entries
> [    2.361359] ARMH0011:00: ttyAMA0 at MMIO 0x9000000 (irq = 16,
> base_baud = 0) is a SBSA
> [    2.619805] printk: console [ttyAMA0] enabled
> [    2.622114] ACPI: PCI Root Bridge [PCI0] (domain 0000 [bus 00-ff])
> [    2.622788] acpi PNP0A08:00: _OSC: OS supports [ExtendedConfig ASPM
> ClockPM Segments MSI HPX-Type3]
> [    2.623776] acpi PNP0A08:00: _OSC: platform does not support [LTR]
> [    2.624600] acpi PNP0A08:00: _OSC: OS now controls [PCIeHotplug PME
> AER PCIeCapability]
> [    2.625721] acpi PNP0A08:00: ECAM area [mem
> 0x4010000000-0x401fffffff] reserved by PNP0C02:00
> [    2.626645] acpi PNP0A08:00: ECAM at [mem
> 0x4010000000-0x401fffffff] for [bus 00-ff]
> [    2.627450] ACPI: Remapped I/O 0x000000003eff0000 to [io
> 0x0000-0xffff window]
> [    2.628229] ACPI BIOS Error (bug): \_SB.PCI0._DSM: Excess arguments
> - ASL declared 5, ACPI requires 4 (20210930/nsarguments-166)
> [    2.629576] PCI host bridge to bus 0000:00
> [    2.630008] pci_bus 0000:00: root bus resource [mem
> 0x10000000-0x3efeffff window]
> [    2.630747] pci_bus 0000:00: root bus resource [io  0x0000-0xffff
> window]
> [    2.631405] pci_bus 0000:00: root bus resource [mem
> 0x8000000000-0xffffffffff window]
> [    2.632177] pci_bus 0000:00: root bus resource [bus 00-ff]
> [    2.632731] ACPI BIOS Error (bug): \_SB.PCI0._DSM: Excess arguments
> - ASL declared 5, ACPI requires 4 (20210930/nsarguments-166)
>
>
>> +                aml_append(method, aml_return(aml_int(0)));
>> +                aml_append(dev, method);
>> +            }
>> +
>>               acpi_dsdt_add_pci_route_table(dev, cfg->irq);
>>                 /*
>> @@ -191,6 +197,12 @@ void acpi_dsdt_add_gpex(Aml *scope, struct
>> GPEXConfig *cfg)
>>       aml_append(dev, aml_name_decl("_STR", aml_unicode("PCIe 0
>> Device")));
>>       aml_append(dev, aml_name_decl("_CCA", aml_int(1)));
>>   +    if (cfg->preserve_config) {
>> +        method = aml_method("_DSM", 5, AML_SERIALIZED);
>> +        aml_append(method, aml_return(aml_int(0)));
>> +        aml_append(dev, method);
>> +    }
>> +
>>       acpi_dsdt_add_pci_route_table(dev, cfg->irq);
>>         method = aml_method("_CBA", 0, AML_NOTSERIALIZED);
>
>

Re: [RFC v2 1/2] hw/pci-host/gpex: Allow to generate preserve boot config DSM #5

[chenxiang (M) via]

在 2022/1/6 19:00, Eric Auger 写道:
> Hi CHenxiangn
>
> On 12/29/21 8:13 AM, chenxiang (M) via wrote:
>> Hi Eric,
>>
>>
>> 在 2021/10/5 16:53, Eric Auger 写道:
>>> Add a 'preserve_config' field in struct GPEXConfig and
>>> if set generate the DSM #5 for preserving PCI boot configurations.
>>> The DSM presence is needed to expose RMRs.
>>>
>>> At the moment the DSM generation is not yet enabled.
>>>
>>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>> ---
>>>    include/hw/pci-host/gpex.h |  1 +
>>>    hw/pci-host/gpex-acpi.c    | 12 ++++++++++++
>>>    2 files changed, 13 insertions(+)
>>>
>>> diff --git a/include/hw/pci-host/gpex.h b/include/hw/pci-host/gpex.h
>>> index fcf8b63820..3f8f8ec38d 100644
>>> --- a/include/hw/pci-host/gpex.h
>>> +++ b/include/hw/pci-host/gpex.h
>>> @@ -64,6 +64,7 @@ struct GPEXConfig {
>>>        MemMapEntry pio;
>>>        int         irq;
>>>        PCIBus      *bus;
>>> +    bool        preserve_config;
>>>    };
>>>      int gpex_set_irq_num(GPEXHost *s, int index, int gsi);
>>> diff --git a/hw/pci-host/gpex-acpi.c b/hw/pci-host/gpex-acpi.c
>>> index e7e162a00a..7dab259379 100644
>>> --- a/hw/pci-host/gpex-acpi.c
>>> +++ b/hw/pci-host/gpex-acpi.c
>>> @@ -164,6 +164,12 @@ void acpi_dsdt_add_gpex(Aml *scope, struct
>>> GPEXConfig *cfg)
>>>                    aml_append(dev, aml_name_decl("_PXM",
>>> aml_int(numa_node)));
>>>                }
>>>    +            if (cfg->preserve_config) {
>>> +                method = aml_method("_DSM", 5, AML_SERIALIZED);
>> I notice there is a ACPI BIOS Error when booting virtual machine which
>> seems be caused by this patch as I add this patchset in my branch to
>> test the function of vsmmu.
>> It seems that it requires only 4 parameter for method _DSM, but use 5
>> parameters here.
>> The error log is as following:
> Thank you for the heads up. Yes the problem was reported by Igor too in
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg842972.html.
>
> At the moment the RMRR ACPI situation has not progressed on spec side or
> kernel if I have not missed anything but sure I will take this into
> account in my next respin.

Ok, thanks.

>
> Thanks!
>
> Eric
>> [    2.355459] ACPI BIOS Error (bug): Failure creating named object
>> [\_SB.PCI0._DSM], AE_ALREADY_EXISTS (20210930/dswload2-327)
>> [    2.355467] ACPI Error: AE_ALREADY_EXISTS, During name
>> lookup/catalog (20210930/psobject-221)
>> [    2.355470] ACPI: Skipping parse of AML opcode: OpcodeName
>> unavailable (0x0014)
>> [    2.355657] ACPI: 1 ACPI AML tables successfully acquired and loaded
>> [    2.356321] ACPI: Interpreter enabled
>> [    2.356323] ACPI: Using GIC for interrupt routing
>> [    2.356333] ACPI: MCFG table detected, 1 entries
>> [    2.361359] ARMH0011:00: ttyAMA0 at MMIO 0x9000000 (irq = 16,
>> base_baud = 0) is a SBSA
>> [    2.619805] printk: console [ttyAMA0] enabled
>> [    2.622114] ACPI: PCI Root Bridge [PCI0] (domain 0000 [bus 00-ff])
>> [    2.622788] acpi PNP0A08:00: _OSC: OS supports [ExtendedConfig ASPM
>> ClockPM Segments MSI HPX-Type3]
>> [    2.623776] acpi PNP0A08:00: _OSC: platform does not support [LTR]
>> [    2.624600] acpi PNP0A08:00: _OSC: OS now controls [PCIeHotplug PME
>> AER PCIeCapability]
>> [    2.625721] acpi PNP0A08:00: ECAM area [mem
>> 0x4010000000-0x401fffffff] reserved by PNP0C02:00
>> [    2.626645] acpi PNP0A08:00: ECAM at [mem
>> 0x4010000000-0x401fffffff] for [bus 00-ff]
>> [    2.627450] ACPI: Remapped I/O 0x000000003eff0000 to [io
>> 0x0000-0xffff window]
>> [    2.628229] ACPI BIOS Error (bug): \_SB.PCI0._DSM: Excess arguments
>> - ASL declared 5, ACPI requires 4 (20210930/nsarguments-166)
>> [    2.629576] PCI host bridge to bus 0000:00
>> [    2.630008] pci_bus 0000:00: root bus resource [mem
>> 0x10000000-0x3efeffff window]
>> [    2.630747] pci_bus 0000:00: root bus resource [io  0x0000-0xffff
>> window]
>> [    2.631405] pci_bus 0000:00: root bus resource [mem
>> 0x8000000000-0xffffffffff window]
>> [    2.632177] pci_bus 0000:00: root bus resource [bus 00-ff]
>> [    2.632731] ACPI BIOS Error (bug): \_SB.PCI0._DSM: Excess arguments
>> - ASL declared 5, ACPI requires 4 (20210930/nsarguments-166)
>>
>>
>>> +                aml_append(method, aml_return(aml_int(0)));
>>> +                aml_append(dev, method);
>>> +            }
>>> +
>>>                acpi_dsdt_add_pci_route_table(dev, cfg->irq);
>>>                  /*
>>> @@ -191,6 +197,12 @@ void acpi_dsdt_add_gpex(Aml *scope, struct
>>> GPEXConfig *cfg)
>>>        aml_append(dev, aml_name_decl("_STR", aml_unicode("PCIe 0
>>> Device")));
>>>        aml_append(dev, aml_name_decl("_CCA", aml_int(1)));
>>>    +    if (cfg->preserve_config) {
>>> +        method = aml_method("_DSM", 5, AML_SERIALIZED);
>>> +        aml_append(method, aml_return(aml_int(0)));
>>> +        aml_append(dev, method);
>>> +    }
>>> +
>>>        acpi_dsdt_add_pci_route_table(dev, cfg->irq);
>>>          method = aml_method("_CBA", 0, AML_NOTSERIALIZED);
>>
> .
>