Hello,
David Bidner, le lun. 21 sept. 2026 20:24:54 +0200, a ecrit:
> Reserve the running BSP as kernel ID 0 before the MADT walk, so it
> survives firmware that lists it later. Ignore duplicate LAPIC entries
> and CPUs beyond NCPUS, count eligible against accepted CPUs, and handle
> x2APIC and NMI source entries instead of printing them as unhandled.
>
> Stop the walk on a zero-length or truncated entry, and name the ACPI
> error codes for the startup path.
Some of these changes are harmless (handling x2APIC/NMI, adding
acpi_error_string) while the others have consequences. Please keep
separate patches for the changes that have consequences, so we can
bisect in cases of regressions.
Samuel
> ---
> i386/i386at/acpi_parse_apic.c | 105 +++++++++++++++++++++++++++-------
> i386/i386at/acpi_parse_apic.h | 4 +-
> 2 files changed, 86 insertions(+), 23 deletions(-)
>
> diff --git a/i386/i386at/acpi_parse_apic.c b/i386/i386at/acpi_parse_apic.c
> index 848585c..5edf76a 100644
> --- a/i386/i386at/acpi_parse_apic.c
> +++ b/i386/i386at/acpi_parse_apic.c
> @@ -36,6 +36,23 @@ static struct acpi_apic *apic_madt = NULL;
> unsigned lapic_addr;
> uint32_t *hpet_addr;
>
> +const char *
> +acpi_error_string(int err)
> +{
> + switch (err) {
> + case ACPI_BAD_CHECKSUM: return "bad ACPI table checksum";
> + case ACPI_BAD_ALIGN: return "bad ACPI RSDP alignment";
> + case ACPI_NO_RSDP: return "no ACPI RSDP found";
> + case ACPI_NO_RSDT: return "no ACPI RSDT/XSDT found";
> + case ACPI_BAD_SIGNATURE: return "bad ACPI table signature";
> + case ACPI_NO_APIC: return "no APIC/MADT table found";
> + case ACPI_NO_LAPIC: return "cannot map the Local APIC";
> + case ACPI_APIC_FAILURE: return "no usable APIC CPU topology";
> + case ACPI_FIT_FAILURE: return "cannot size the accepted CPU list";
> + default: return "unknown ACPI/APIC error";
> + }
> +}
> +
> /*
> * acpi_print_info: shows by screen the ACPI's rsdp and rsdt virtual address
> * and the number of entries stored in RSDT table.
> @@ -371,16 +388,28 @@ acpi_get_apic2(struct acpi_xsdt *xsdt, int acpi_xsdt_n)
> * and increase the number of cpus.
> *
> * Receives as input the Local APIC entry in MADT/APIC table.
> + * Duplicate entries and CPUs beyond NCPUS are ignored.
> */
> static void
> acpi_apic_add_lapic(struct acpi_apic_lapic *lapic_entry)
> {
> + uint16_t apic_id;
> + int i;
> +
> /* If cpu flag is correct */
> - if (lapic_entry->flags & (ACPI_LAPIC_FLAG_ENABLED |
> ACPI_LAPIC_FLAG_CAPABLE)) {
> - /* Add cpu to processors' list. */
> - apic_add_cpu(lapic_entry->apic_id & apic_id_mask);
> + if (!(lapic_entry->flags & (ACPI_LAPIC_FLAG_ENABLED |
> ACPI_LAPIC_FLAG_CAPABLE)))
> + return;
> +
> + apic_id = lapic_entry->apic_id & apic_id_mask;
> +
> + for (i = 0; i < apic_get_numcpus(); i++) {
> + if (apic_get_cpu_apic_id(i) == apic_id)
> + return;
> }
>
> + if (apic_add_cpu(apic_id) != 0)
> + printf("APIC: ignoring LAPIC ID %#x: NCPUS=%d limit reached\n",
> + apic_id, NCPUS);
> }
>
> /*
> @@ -445,7 +474,7 @@ acpi_apic_parse_table(struct acpi_apic *apic)
> {
> struct acpi_apic_dhdr *apic_entry = NULL;
> vm_offset_t end = 0;
> - uint8_t numcpus = 1;
> + unsigned int eligible = 0;
>
> /* Get the address of first APIC entry */
> apic_entry = (struct acpi_apic_dhdr*) apic->entry;
> @@ -455,26 +484,29 @@ acpi_apic_parse_table(struct acpi_apic *apic)
>
> printf("APIC entry=0x%p end=0x%x\n", apic_entry, end);
>
> - /* Initialize number of cpus */
> - numcpus = apic_get_numcpus();
> -
> - /* Search in APIC entry. */
> - while ((vm_offset_t)apic_entry < end) {
> + /* Stop on a truncated or zero-length entry. */
> + while ((vm_offset_t)apic_entry + sizeof(struct acpi_apic_dhdr) <= end) {
> struct acpi_apic_lapic *lapic_entry;
> struct acpi_apic_ioapic *ioapic_entry;
> struct acpi_apic_irq_override *irq_override_entry;
>
> printf("APIC entry=0x%p end=0x%x\n", apic_entry, end);
> +
> + if (apic_entry->length == 0) {
> + printf("APIC: zero-length MADT entry type %#x, stopping\n",
> + apic_entry->type);
> + break;
> + }
> +
> /* Check entry type. */
> switch(apic_entry->type) {
>
> /* If APIC entry is a CPU's Local APIC. */
> case ACPI_APIC_ENTRY_LAPIC:
> - if(numcpus < NCPUS) {
> - /* Store Local APIC data. */
> - lapic_entry = (struct acpi_apic_lapic*) apic_entry;
> - acpi_apic_add_lapic(lapic_entry);
> - }
> + lapic_entry = (struct acpi_apic_lapic*) apic_entry;
> + if (lapic_entry->flags & (ACPI_LAPIC_FLAG_ENABLED |
> ACPI_LAPIC_FLAG_CAPABLE))
> + eligible++;
> + acpi_apic_add_lapic(lapic_entry);
> break;
>
> /* If APIC entry is an IOAPIC. */
> @@ -494,20 +526,31 @@ acpi_apic_parse_table(struct acpi_apic *apic)
> acpi_apic_add_irq_override(irq_override_entry);
> break;
>
> + /* x2APIC entries hold 32-bit IDs this xAPIC port cannot map. */
> + case ACPI_APIC_ENTRY_X2APIC:
> + printf("APIC: x2APIC entry (type %d) unsupported, ignored\n",
> + apic_entry->type);
> + break;
> +
> + /* Local APIC NMI entries do not describe a processor. */
> + case ACPI_APIC_ENTRY_NONMASK_IRQ:
> + break;
> +
> /* FIXME: There is another unhandled case */
> - default:
> - printf("Unhandled APIC entry type 0x%x\n", apic_entry->type);
> - break;
> + default:
> + printf("Unhandled APIC entry type 0x%x\n", apic_entry->type);
> + break;
> }
>
> /* Get next APIC entry. */
> apic_entry = (struct acpi_apic_dhdr*)((vm_offset_t) apic_entry
> + apic_entry->length);
> -
> - /* Update number of cpus. */
> - numcpus = apic_get_numcpus();
> }
>
> + if (eligible > apic_get_numcpus())
> + printf("APIC: %u eligible LAPIC entries, accepted %u (NCPUS=%d)\n",
> + eligible, apic_get_numcpus(), NCPUS);
> +
> return ACPI_SUCCESS;
> }
>
> @@ -544,19 +587,37 @@ acpi_apic_setup(struct acpi_apic *apic)
>
> fix_apic_id_mask();
>
> + /* The BSP must keep kernel ID 0; reserve it before the MADT walk. */
> + if (apic_add_cpu(apic_get_current_cpu()) != 0)
> + return ACPI_APIC_FAILURE;
> +
> acpi_apic_parse_table(apic);
>
> ncpus = apic_get_numcpus();
> nioapics = apic_get_num_ioapics();
>
> - if (ncpus == 0 || nioapics == 0 || ncpus > NCPUS)
> + if (ncpus == 0) {
> + printf("ACPI: no usable Local APIC (NCPUS=%d)\n", NCPUS);
> + return ACPI_APIC_FAILURE;
> + }
> +
> + if (ncpus > NCPUS) {
> + printf("ACPI: %u accepted CPUs exceed NCPUS=%d\n", ncpus, NCPUS);
> return ACPI_APIC_FAILURE;
> + }
> +
> + if (nioapics == 0) {
> + printf("ACPI: no IOAPIC in MADT\n");
> + return ACPI_APIC_FAILURE;
> + }
>
> /* Refit the apic-cpu array. */
> if(ncpus < NCPUS) {
> int refit = apic_refit_cpulist();
> - if (refit != 0)
> + if (refit != 0) {
> + printf("ACPI: cannot shrink CPU list to %u entries\n", ncpus);
> return ACPI_FIT_FAILURE;
> + }
> }
>
> apic_generate_cpu_id_lut();
> diff --git a/i386/i386at/acpi_parse_apic.h b/i386/i386at/acpi_parse_apic.h
> index df8d4ba..c8cf833 100644
> --- a/i386/i386at/acpi_parse_apic.h
> +++ b/i386/i386at/acpi_parse_apic.h
> @@ -107,7 +107,8 @@ enum ACPI_APIC_ENTRY_TYPE {
> ACPI_APIC_ENTRY_LAPIC = 0,
> ACPI_APIC_ENTRY_IOAPIC = 1,
> ACPI_APIC_ENTRY_IRQ_OVERRIDE = 2,
> - ACPI_APIC_ENTRY_NONMASK_IRQ = 4
> + ACPI_APIC_ENTRY_NONMASK_IRQ = 4,
> + ACPI_APIC_ENTRY_X2APIC = 9
> };
>
> /*
> @@ -197,6 +198,7 @@ struct acpi_hpet {
>
> int acpi_apic_init(void);
> void acpi_print_info(phys_addr_t rsdp, void *rsdt, int acpi_rsdt_n);
> +const char *acpi_error_string(int err);
>
> extern unsigned lapic_addr;
>
>
--
Samuel
>Ever heard of .cshrc?
That's a city in Bosnia. Right?
(Discussion in comp.os.linux.misc on the intuitiveness of commands.)
