Replace uses of the LAPIC_ID() macro with accesses to the
cpu_to_apicid[] lookup table. This table contains the APIC IDs of each
vCPU as probed at runtime rather than assuming a predefined relation.
Moved smp_initialise() ahead of apic_setup() in order to initialise
cpu_to_apicid ASAP and avoid using it uninitialised. Note that bringing
up the APs doesn't need the APIC in hvmloader becasue it always runs
virtualized and uses the PV interface.
Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com>
---
Changes from v7 of the longer topology series:
* Removed ASSERT() for the MP tables and merely skipped writing them
if any vCPU has an APIC ID >=255.
---
tools/firmware/hvmloader/config.h | 3 ++-
tools/firmware/hvmloader/hvmloader.c | 6 +++---
tools/firmware/hvmloader/mp_tables.c | 2 +-
tools/firmware/hvmloader/util.c | 2 +-
4 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/tools/firmware/hvmloader/config.h b/tools/firmware/hvmloader/config.h
index cd716bf39245..6e1da137d779 100644
--- a/tools/firmware/hvmloader/config.h
+++ b/tools/firmware/hvmloader/config.h
@@ -48,8 +48,9 @@ extern uint8_t ioapic_version;
#define IOAPIC_ID 0x01
+extern uint32_t *cpu_to_apicid;
+
#define LAPIC_BASE_ADDRESS 0xfee00000
-#define LAPIC_ID(vcpu_id) ((vcpu_id) * 2)
#define PCI_ISA_DEVFN 0x08 /* dev 1, fn 0 */
#define PCI_ISA_IRQ_MASK 0x0c20U /* ISA IRQs 5,10,11 are PCI connected */
diff --git a/tools/firmware/hvmloader/hvmloader.c b/tools/firmware/hvmloader/hvmloader.c
index f8af88fabf24..4e330fc1e241 100644
--- a/tools/firmware/hvmloader/hvmloader.c
+++ b/tools/firmware/hvmloader/hvmloader.c
@@ -224,7 +224,7 @@ static void apic_setup(void)
/* 8259A ExtInts are delivered through IOAPIC pin 0 (Virtual Wire Mode). */
ioapic_write(0x10, APIC_DM_EXTINT);
- ioapic_write(0x11, SET_APIC_ID(LAPIC_ID(0)));
+ ioapic_write(0x11, SET_APIC_ID(cpu_to_apicid[0]));
}
struct bios_info {
@@ -341,11 +341,11 @@ int main(void)
printf("CPU speed is %u MHz\n", get_cpu_mhz());
+ smp_initialise();
+
apic_setup();
pci_setup();
- smp_initialise();
-
perform_tests();
if ( bios->bios_info_setup )
diff --git a/tools/firmware/hvmloader/mp_tables.c b/tools/firmware/hvmloader/mp_tables.c
index 77d3010406d0..3c93a5c947d9 100644
--- a/tools/firmware/hvmloader/mp_tables.c
+++ b/tools/firmware/hvmloader/mp_tables.c
@@ -199,7 +199,7 @@ static void fill_mp_config_table(struct mp_config_table *mpct, int length)
static void fill_mp_proc_entry(struct mp_proc_entry *mppe, int vcpu_id)
{
mppe->type = ENTRY_TYPE_PROCESSOR;
- mppe->lapic_id = LAPIC_ID(vcpu_id);
+ mppe->lapic_id = cpu_to_apicid[vcpu_id];
mppe->lapic_version = 0x11;
mppe->cpu_flags = CPU_FLAG_ENABLED;
if ( vcpu_id == 0 )
diff --git a/tools/firmware/hvmloader/util.c b/tools/firmware/hvmloader/util.c
index d3b3f9038e64..2d07ce129013 100644
--- a/tools/firmware/hvmloader/util.c
+++ b/tools/firmware/hvmloader/util.c
@@ -827,7 +827,7 @@ static void acpi_mem_free(struct acpi_ctxt *ctxt,
static uint32_t acpi_lapic_id(unsigned cpu)
{
- return LAPIC_ID(cpu);
+ return cpu_to_apicid[cpu];
}
void hvmloader_acpi_build_tables(struct acpi_config *config,
--
2.48.1On 28.01.2025 17:33, Alejandro Vallejo wrote:
> Replace uses of the LAPIC_ID() macro with accesses to the
> cpu_to_apicid[] lookup table. This table contains the APIC IDs of each
> vCPU as probed at runtime rather than assuming a predefined relation.
>
> Moved smp_initialise() ahead of apic_setup() in order to initialise
> cpu_to_apicid ASAP and avoid using it uninitialised. Note that bringing
> up the APs doesn't need the APIC in hvmloader becasue it always runs
> virtualized and uses the PV interface.
>
> Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com>
> ---
> Changes from v7 of the longer topology series:
> * Removed ASSERT() for the MP tables and merely skipped writing them
> if any vCPU has an APIC ID >=255.
Throughout the patch I can't anything matching this; ...
> --- a/tools/firmware/hvmloader/config.h
> +++ b/tools/firmware/hvmloader/config.h
> @@ -48,8 +48,9 @@ extern uint8_t ioapic_version;
>
> #define IOAPIC_ID 0x01
>
> +extern uint32_t *cpu_to_apicid;
> +
> #define LAPIC_BASE_ADDRESS 0xfee00000
> -#define LAPIC_ID(vcpu_id) ((vcpu_id) * 2)
>
> #define PCI_ISA_DEVFN 0x08 /* dev 1, fn 0 */
> #define PCI_ISA_IRQ_MASK 0x0c20U /* ISA IRQs 5,10,11 are PCI connected */
> --- a/tools/firmware/hvmloader/hvmloader.c
> +++ b/tools/firmware/hvmloader/hvmloader.c
> @@ -224,7 +224,7 @@ static void apic_setup(void)
>
> /* 8259A ExtInts are delivered through IOAPIC pin 0 (Virtual Wire Mode). */
> ioapic_write(0x10, APIC_DM_EXTINT);
> - ioapic_write(0x11, SET_APIC_ID(LAPIC_ID(0)));
> + ioapic_write(0x11, SET_APIC_ID(cpu_to_apicid[0]));
> }
>
> struct bios_info {
> @@ -341,11 +341,11 @@ int main(void)
>
> printf("CPU speed is %u MHz\n", get_cpu_mhz());
>
> + smp_initialise();
> +
> apic_setup();
> pci_setup();
>
> - smp_initialise();
> -
> perform_tests();
>
> if ( bios->bios_info_setup )
> --- a/tools/firmware/hvmloader/mp_tables.c
> +++ b/tools/firmware/hvmloader/mp_tables.c
> @@ -199,7 +199,7 @@ static void fill_mp_config_table(struct mp_config_table *mpct, int length)
> static void fill_mp_proc_entry(struct mp_proc_entry *mppe, int vcpu_id)
> {
> mppe->type = ENTRY_TYPE_PROCESSOR;
> - mppe->lapic_id = LAPIC_ID(vcpu_id);
> + mppe->lapic_id = cpu_to_apicid[vcpu_id];
> mppe->lapic_version = 0x11;
> mppe->cpu_flags = CPU_FLAG_ENABLED;
> if ( vcpu_id == 0 )
> --- a/tools/firmware/hvmloader/util.c
> +++ b/tools/firmware/hvmloader/util.c
> @@ -827,7 +827,7 @@ static void acpi_mem_free(struct acpi_ctxt *ctxt,
>
> static uint32_t acpi_lapic_id(unsigned cpu)
> {
> - return LAPIC_ID(cpu);
> + return cpu_to_apicid[cpu];
> }
>
> void hvmloader_acpi_build_tables(struct acpi_config *config,
... no conditional is being added anywhere. What am I missing?
Thing is - this way I'm uncertain whether the wording above is
imprecise, or whether I should really ask that we continue to write all
entries with at most 8-bit-wide APIC IDs, omitting just those with
wider ones.
Jan
© 2016 - 2025 Red Hat, Inc.