| Message ID | 20250128163342.1491-4-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: > MP Tables have no means of representing APIC IDs wider than 255. At the > moment the effect is wrapping around which would give a corrupted table > with duplicate APIC IDs (some of them possibly the broadcast 255). > > Skip writing it altogether. While it would be possible to restrict the > APs shown it's just not worth the work. Any OS that needs such > adjustments should not have been booted with that many vCPUs to begin > with. > > Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com> Oh, here we go. Yet then indeed ... > --- a/tools/firmware/hvmloader/config.h > +++ b/tools/firmware/hvmloader/config.h > @@ -49,6 +49,7 @@ extern uint8_t ioapic_version; > #define IOAPIC_ID 0x01 > > extern uint32_t *cpu_to_apicid; > +extern uint32_t max_apicid; .. I don't think we need this, as ... > --- a/tools/firmware/hvmloader/hvmloader.c > +++ b/tools/firmware/hvmloader/hvmloader.c > @@ -389,7 +389,11 @@ int main(void) > > if ( (hvm_info->nr_vcpus > 1) || hvm_info->apic_mode ) > { > - if ( bios->create_mp_tables ) > + /* > + * Legacy MP tables hold strictly xAPIC IDs. Skip writing > + * the tables altogether if we have IDs wider than 8bits. > + */ > + if ( max_apicid < 0xFF && bios->create_mp_tables ) > bios->create_mp_tables(); I think we ought to continue to write MP tables in this case, merely omitting processor entries with APIC IDs that don't fit. Jan
diff --git a/tools/firmware/hvmloader/config.h b/tools/firmware/hvmloader/config.h index 6e1da137d779..53be34e48a02 100644 --- a/tools/firmware/hvmloader/config.h +++ b/tools/firmware/hvmloader/config.h @@ -49,6 +49,7 @@ extern uint8_t ioapic_version; #define IOAPIC_ID 0x01 extern uint32_t *cpu_to_apicid; +extern uint32_t max_apicid; #define LAPIC_BASE_ADDRESS 0xfee00000 diff --git a/tools/firmware/hvmloader/hvmloader.c b/tools/firmware/hvmloader/hvmloader.c index 4e330fc1e241..54299e27364d 100644 --- a/tools/firmware/hvmloader/hvmloader.c +++ b/tools/firmware/hvmloader/hvmloader.c @@ -389,7 +389,11 @@ int main(void) if ( (hvm_info->nr_vcpus > 1) || hvm_info->apic_mode ) { - if ( bios->create_mp_tables ) + /* + * Legacy MP tables hold strictly xAPIC IDs. Skip writing + * the tables altogether if we have IDs wider than 8bits. + */ + if ( max_apicid < 0xFF && bios->create_mp_tables ) bios->create_mp_tables(); if ( bios->create_pir_tables ) bios->create_pir_tables(); diff --git a/tools/firmware/hvmloader/smp.c b/tools/firmware/hvmloader/smp.c index c61ed524f947..0a01cdc18caa 100644 --- a/tools/firmware/hvmloader/smp.c +++ b/tools/firmware/hvmloader/smp.c @@ -34,6 +34,9 @@ static int ap_callin; /** True if x2apic support is exposed to the guest. */ static bool has_x2apic; +/** Highest entry in `cpu_to_apicid`. */ +uint32_t max_apicid; + /** * Lookup table of (x2)APIC IDs. * @@ -61,6 +64,7 @@ static uint32_t read_apic_id(void) static void cpu_setup(unsigned int cpu) { uint32_t apicid = cpu_to_apicid[cpu] = read_apic_id(); + max_apicid = max(max_apicid, apicid); printf(" - CPU%u[%u] ... ", cpu, apicid); cacheattr_init();
MP Tables have no means of representing APIC IDs wider than 255. At the moment the effect is wrapping around which would give a corrupted table with duplicate APIC IDs (some of them possibly the broadcast 255). Skip writing it altogether. While it would be possible to restrict the APs shown it's just not worth the work. Any OS that needs such adjustments should not have been booted with that many vCPUs to begin with. Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com> --- Changes with respect to v7 in the longer topology series: * This patch replaces the previous assert in hvmloader/mp_tables.c --- tools/firmware/hvmloader/config.h | 1 + tools/firmware/hvmloader/hvmloader.c | 6 +++++- tools/firmware/hvmloader/smp.c | 4 ++++ 3 files changed, 10 insertions(+), 1 deletion(-)