| Message ID | 20250128163342.1491-3-alejandro.vallejo@cloud.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | tools/hvmloader: Decouple APIC IDs from vCPU IDs | expand |
On 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
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,
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(-)