On 13.03.2024 16:35, Matthew Barnes wrote:
> libacpi is a tool that is used by libxl (for PVH guests) and hvmloader
> (for HVM guests) to construct ACPI tables for guests.
>
> Currently, libacpi only uses APIC entries to enumerate processors for
> guests in the MADT.
>
> The APIC ID field in APIC entries is an octet big, which is fine for
> xAPIC IDs, but not so for sufficiently large x2APIC IDs.
Yet where would those come from? I can see that down the road we will
have such, but right now I don't think we do. Without saying so, this
change could be mistaken for a fix of an active bug.
> This patch scans each APIC ID before constructing the MADT, and uses the
> x2APIC entry for each vCPU whose APIC ID exceeds the size limit imposed
> by regular APIC entries.
It is my understanding that if you use any x2APIC entry, every CPU needs
to have one.
> @@ -134,27 +151,45 @@ static struct acpi_20_madt *construct_madt(struct
> acpi_ctxt *ctxt,
> io_apic->ioapic_id = config->ioapic_id;
> io_apic->ioapic_addr = config->ioapic_base_address;
>
> - lapic = (struct acpi_20_madt_lapic *)(io_apic + 1);
> + apicid_entry = io_apic + 1;
> }
> else
> - lapic = (struct acpi_20_madt_lapic *)(madt + 1);
> + apicid_entry = madt + 1;
>
> info->nr_cpus = hvminfo->nr_vcpus;
> - info->madt_lapic0_addr = ctxt->mem_ops.v2p(ctxt, lapic);
> + info->madt_lapic0_addr = ctxt->mem_ops.v2p(ctxt, apicid_entry);
> for ( i = 0; i < hvminfo->nr_vcpus; i++ )
> {
> - memset(lapic, 0, sizeof(*lapic));
> - lapic->type = ACPI_PROCESSOR_LOCAL_APIC;
> - 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->flags = (test_bit(i, hvminfo->vcpu_online)
> - ? ACPI_LOCAL_APIC_ENABLED : 0);
> - lapic++;
> + uint32_t apic_id = config->lapic_id(i);
> + if ( apic_id < 255 )
Nit (here and below): This file uses hypervisor coding style. and hence a
blank line is wanted between declaration(s) and statement(s).
> + {
> + struct acpi_20_madt_lapic *lapic = apicid_entry;
> + memset(lapic, 0, sizeof(*lapic));
> + lapic->type = ACPI_PROCESSOR_LOCAL_APIC;
> + lapic->length = sizeof(*lapic);
> + /* Processor ID must match processor-object IDs in the DSDT. */
> + lapic->acpi_processor_id = i;
> + lapic->apic_id = apic_id;
> + lapic->flags = (test_bit(i, hvminfo->vcpu_online)
> + ? ACPI_LOCAL_APIC_ENABLED : 0);
> + apicid_entry = lapic + 1;
> + }
> + else
> + {
> + struct acpi_20_madt_x2apic *x2apic = apicid_entry;
> + memset(x2apic, 0, sizeof(*x2apic));
> + x2apic->type = ACPI_PROCESSOR_LOCAL_X2APIC;
> + x2apic->length = sizeof(*x2apic);
> + x2apic->apic_id = apic_id;
> + x2apic->flags = (test_bit(i, hvminfo->vcpu_online)
> + ? ACPI_LOCAL_APIC_ENABLED : 0);
Nit: Indentation off by 1.
Jan
