[PATCH v7 05/10] tools/libacpi: Use LUT of APIC IDs rather than function pointer

Alejandro Vallejo posted 10 patches 3 weeks, 2 days ago
[PATCH v7 05/10] tools/libacpi: Use LUT of APIC IDs rather than function pointer
Posted by Alejandro Vallejo 3 weeks, 2 days ago
Refactors libacpi so that a single LUT is the authoritative source of
truth for the CPU to APIC ID mappings. This has a know-on effect in
reducing complexity on future patches, as the same LUT can be used for
configuring the APICs and configuring the ACPI tables for PVH.

Not functional change intended, because the same mappings are preserved.

Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com>
---
v7:
  * NOTE: didn't add assert to libacpi as initially accepted in order to
    protect libvirt from an assert failure.
  * s/uint32_t/unsigned int/ in for loop of libxl.
  * turned Xen-style loop in libxl to libxl-style.
---
 tools/firmware/hvmloader/util.c   | 7 +------
 tools/include/xenguest.h          | 5 +++++
 tools/libacpi/build.c             | 6 +++---
 tools/libacpi/libacpi.h           | 2 +-
 tools/libs/light/libxl_dom.c      | 5 +++++
 tools/libs/light/libxl_x86_acpi.c | 7 +------
 6 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/tools/firmware/hvmloader/util.c b/tools/firmware/hvmloader/util.c
index 821b3086a87d..afa3eb9d5775 100644
--- a/tools/firmware/hvmloader/util.c
+++ b/tools/firmware/hvmloader/util.c
@@ -825,11 +825,6 @@ static void acpi_mem_free(struct acpi_ctxt *ctxt,
     /* ACPI builder currently doesn't free memory so this is just a stub */
 }
 
-static uint32_t acpi_lapic_id(unsigned cpu)
-{
-    return cpu_to_x2apic_id[cpu];
-}
-
 void hvmloader_acpi_build_tables(struct acpi_config *config,
                                  unsigned int physical)
 {
@@ -859,7 +854,7 @@ void hvmloader_acpi_build_tables(struct acpi_config *config,
     }
 
     config->lapic_base_address = LAPIC_BASE_ADDRESS;
-    config->lapic_id = acpi_lapic_id;
+    config->cpu_to_apicid = cpu_to_x2apicid;
     config->ioapic_base_address = IOAPIC_BASE_ADDRESS;
     config->ioapic_id = IOAPIC_ID;
     config->pci_isa_irq_mask = PCI_ISA_IRQ_MASK; 
diff --git a/tools/include/xenguest.h b/tools/include/xenguest.h
index e01f494b772a..aa50b78dfb89 100644
--- a/tools/include/xenguest.h
+++ b/tools/include/xenguest.h
@@ -22,6 +22,8 @@
 #ifndef XENGUEST_H
 #define XENGUEST_H
 
+#include "xen/hvm/hvm_info_table.h"
+
 #define XC_NUMA_NO_NODE   (~0U)
 
 #define XCFLAGS_LIVE      (1 << 0)
@@ -236,6 +238,9 @@ struct xc_dom_image {
 #if defined(__i386__) || defined(__x86_64__)
     struct e820entry *e820;
     unsigned int e820_entries;
+
+    /* LUT mapping cpu id to (x2)APIC ID */
+    uint32_t cpu_to_apicid[HVM_MAX_VCPUS];
 #endif
 
     xen_pfn_t vuart_gfn;
diff --git a/tools/libacpi/build.c b/tools/libacpi/build.c
index 2f29863db154..2ad1d461a2ec 100644
--- a/tools/libacpi/build.c
+++ b/tools/libacpi/build.c
@@ -74,7 +74,7 @@ static struct acpi_20_madt *construct_madt(struct acpi_ctxt *ctxt,
     const struct hvm_info_table   *hvminfo = config->hvminfo;
     int i, sz;
 
-    if ( config->lapic_id == NULL )
+    if ( config->cpu_to_apicid == NULL )
         return NULL;
 
     sz  = sizeof(struct acpi_20_madt);
@@ -148,7 +148,7 @@ static struct acpi_20_madt *construct_madt(struct acpi_ctxt *ctxt,
         lapic->length  = sizeof(*lapic);
         /* Processor ID must match processor-object IDs in the DSDT. */
         lapic->acpi_processor_id = i;
-        lapic->apic_id = config->lapic_id(i);
+        lapic->apic_id = config->cpu_to_apicid[i];
         lapic->flags = (test_bit(i, hvminfo->vcpu_online)
                         ? ACPI_LOCAL_APIC_ENABLED : 0);
         lapic++;
@@ -236,7 +236,7 @@ static struct acpi_20_srat *construct_srat(struct acpi_ctxt *ctxt,
         processor->type     = ACPI_PROCESSOR_AFFINITY;
         processor->length   = sizeof(*processor);
         processor->domain   = config->numa.vcpu_to_vnode[i];
-        processor->apic_id  = config->lapic_id(i);
+        processor->apic_id  = config->cpu_to_apicid[i];
         processor->flags    = ACPI_LOCAL_APIC_AFFIN_ENABLED;
         processor++;
     }
diff --git a/tools/libacpi/libacpi.h b/tools/libacpi/libacpi.h
index deda39e5dbc4..e8f603ee18ee 100644
--- a/tools/libacpi/libacpi.h
+++ b/tools/libacpi/libacpi.h
@@ -84,7 +84,7 @@ struct acpi_config {
     unsigned long rsdp;
 
     /* x86-specific parameters */
-    uint32_t (*lapic_id)(unsigned cpu);
+    const uint32_t *cpu_to_apicid; /* LUT mapping cpu id to (x2)APIC ID */
     uint32_t lapic_base_address;
     uint32_t ioapic_base_address;
     uint16_t pci_isa_irq_mask;
diff --git a/tools/libs/light/libxl_dom.c b/tools/libs/light/libxl_dom.c
index 94fef374014e..5f4f6830e850 100644
--- a/tools/libs/light/libxl_dom.c
+++ b/tools/libs/light/libxl_dom.c
@@ -1082,6 +1082,11 @@ int libxl__build_hvm(libxl__gc *gc, uint32_t domid,
 
     dom->container_type = XC_DOM_HVM_CONTAINER;
 
+#if defined(__i386__) || defined(__x86_64__)
+    for (unsigned int i = 0; i < info->max_vcpus; i++)
+        dom->cpu_to_apicid[i] = 2 * i; /* TODO: Replace by topo calculation */
+#endif
+
     /* The params from the configuration file are in Mb, which are then
      * multiplied by 1 Kb. This was then divided off when calling
      * the old xc_hvm_build_target_mem() which then turned them to bytes.
diff --git a/tools/libs/light/libxl_x86_acpi.c b/tools/libs/light/libxl_x86_acpi.c
index 5cf261bd6794..585d4c8755cb 100644
--- a/tools/libs/light/libxl_x86_acpi.c
+++ b/tools/libs/light/libxl_x86_acpi.c
@@ -75,11 +75,6 @@ static void acpi_mem_free(struct acpi_ctxt *ctxt,
 {
 }
 
-static uint32_t acpi_lapic_id(unsigned cpu)
-{
-    return cpu * 2;
-}
-
 static int init_acpi_config(libxl__gc *gc, 
                             struct xc_dom_image *dom,
                             const libxl_domain_build_info *b_info,
@@ -144,7 +139,7 @@ static int init_acpi_config(libxl__gc *gc,
     config->hvminfo = hvminfo;
 
     config->lapic_base_address = LAPIC_BASE_ADDRESS;
-    config->lapic_id = acpi_lapic_id;
+    config->cpu_to_apicid = dom->cpu_to_apicid;
     config->acpi_revision = 5;
 
     rc = 0;
-- 
2.47.0
Re: [PATCH v7 05/10] tools/libacpi: Use LUT of APIC IDs rather than function pointer
Posted by Andrew Cooper 2 weeks ago
On 21/10/2024 4:45 pm, Alejandro Vallejo wrote:
> diff --git a/tools/include/xenguest.h b/tools/include/xenguest.h
> index e01f494b772a..aa50b78dfb89 100644
> --- a/tools/include/xenguest.h
> +++ b/tools/include/xenguest.h
> @@ -22,6 +22,8 @@
>  #ifndef XENGUEST_H
>  #define XENGUEST_H
>  
> +#include "xen/hvm/hvm_info_table.h"
> +
>  #define XC_NUMA_NO_NODE   (~0U)
>  
>  #define XCFLAGS_LIVE      (1 << 0)
> @@ -236,6 +238,9 @@ struct xc_dom_image {
>  #if defined(__i386__) || defined(__x86_64__)
>      struct e820entry *e820;
>      unsigned int e820_entries;
> +
> +    /* LUT mapping cpu id to (x2)APIC ID */
> +    uint32_t cpu_to_apicid[HVM_MAX_VCPUS];

Same note as the previous patch.

This needs to be a plain dynamically allocated array, because it mustn't
use HVM_MAX_VCPUS.

~Andrew