On 21.10.2024 17:45, Alejandro Vallejo wrote:
> --- 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_x2apicid[0]));
> }
In uses like this or ...
> --- a/tools/firmware/hvmloader/mp_tables.c
> +++ b/tools/firmware/hvmloader/mp_tables.c
> @@ -198,8 +198,10 @@ static void fill_mp_config_table(struct mp_config_table
> *mpct, int length)
> /* fills in an MP processor entry for VCPU 'vcpu_id' */
> static void fill_mp_proc_entry(struct mp_proc_entry *mppe, int vcpu_id)
> {
> + ASSERT(cpu_to_x2apicid[vcpu_id] < 0xFF );
> +
> mppe->type = ENTRY_TYPE_PROCESSOR;
> - mppe->lapic_id = LAPIC_ID(vcpu_id);
> + mppe->lapic_id = cpu_to_x2apicid[vcpu_id];
... this one (and also in acpi_lapic_id()), I consider the "x2" in the name
actively confusing, despite ...
> --- a/tools/firmware/hvmloader/smp.c
> +++ b/tools/firmware/hvmloader/smp.c
> @@ -29,7 +29,37 @@
>
> #include <xen/vcpu.h>
>
> -static int ap_callin;
> +/**
> + * Lookup table of (x2)APIC IDs.
> + *
> + * Each entry is populated its respective CPU as they come online. This is
> required
> + * for generating the MADT with minimal assumptions about ID relationships.
> + *
> + * While the name makes "x2" explicit, these may actually be xAPIC IDs if no
> + * x2APIC is present. "x2" merely highlights that each entry is 32 bits wide.
> + */
> +uint32_t cpu_to_x2apicid[HVM_MAX_VCPUS];
... the commentary here.
> +/** Tristate about x2apic being supported. -1=unknown */
> +static int has_x2apic = -1;
Why is this a tristate? Prior to the variable having been set, ...
> +static uint32_t read_apic_id(void)
> +{
> + uint32_t apic_id;
> +
> + if ( has_x2apic )
> + cpuid(0xb, NULL, NULL, NULL, &apic_id);
... this is bogus anyway.
Jan
