| Message ID | 20160630080118.GD7175@x1.redhat.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
* Baoquan He <bhe@redhat.com> wrote: > ACPI MADT has a 32-bit field providing lapic address at which > each processor can access its lapic information. MADT also contains > an optional entry to provide a 64-bit address to override the 32-bit > one. However the current code does the lapic address override entry > parsing twice. One is in early_acpi_boot_init() because AMD NUMA need > get boot_cpu_id earlier. The other is in acpi_boot_init() which parses > all MADT entries. > > So in this patch remove the repeated code in the 2nd part. Meanwhile > print lapic override entry information like other MADT entry. So this patch is not supposed to change behavior (modulo kernel messages), right? If so it would make sense to spell that out explicitly in the changelog. Thanks, Ingo -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 07/08/16 at 02:27pm, Ingo Molnar wrote: > > * Baoquan He <bhe@redhat.com> wrote: > > > ACPI MADT has a 32-bit field providing lapic address at which > > each processor can access its lapic information. MADT also contains > > an optional entry to provide a 64-bit address to override the 32-bit > > one. However the current code does the lapic address override entry > > parsing twice. One is in early_acpi_boot_init() because AMD NUMA need > > get boot_cpu_id earlier. The other is in acpi_boot_init() which parses > > all MADT entries. > > > > So in this patch remove the repeated code in the 2nd part. Meanwhile > > print lapic override entry information like other MADT entry. > > So this patch is not supposed to change behavior (modulo kernel messages), right? > If so it would make sense to spell that out explicitly in the changelog. Yes, it won't. I will mention it in changelog and repost. Thanks for your reviewing. Thanks Baoquan -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Ingo, On 07/08/16 at 02:27pm, Ingo Molnar wrote: > > * Baoquan He <bhe@redhat.com> wrote: > > > ACPI MADT has a 32-bit field providing lapic address at which > > each processor can access its lapic information. MADT also contains > > an optional entry to provide a 64-bit address to override the 32-bit > > one. However the current code does the lapic address override entry > > parsing twice. One is in early_acpi_boot_init() because AMD NUMA need > > get boot_cpu_id earlier. The other is in acpi_boot_init() which parses > > all MADT entries. > > > > So in this patch remove the repeated code in the 2nd part. Meanwhile > > print lapic override entry information like other MADT entry. > > So this patch is not supposed to change behavior (modulo kernel messages), right? > If so it would make sense to spell that out explicitly in the changelog. I am not sure if I understand your question correctly. In this patch I added the calling of acpi_table_print_madt_entry(header) in acpi_parse_lapic_addr_ovr, it will print information related if a lapic address override entry is provided as below: case ACPI_MADT_TYPE_LOCAL_APIC_OVERRIDE: { struct acpi_madt_local_apic_override *p = (struct acpi_madt_local_apic_override*)header; pr_info("LAPIC_ADDR_OVR (address[%p])\n", (void *)(unsigned long)p->address); } break; This will add one line of message to boot log if lapic addr override entry provided: "LAPIC_ADDR_OVR (address[0xXXXXXXXX])" I don't know if this is the behaviour change (modulo kernel messages) you mentioned. Thanks Baoquan -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c index 9414f84..6ef3694 100644 --- a/arch/x86/kernel/acpi/boot.c +++ b/arch/x86/kernel/acpi/boot.c @@ -274,6 +274,8 @@ acpi_parse_lapic_addr_ovr(struct acpi_subtable_header * header, if (BAD_MADT_ENTRY(lapic_addr_ovr, end)) return -EINVAL; + acpi_table_print_madt_entry(header); + acpi_lapic_addr = lapic_addr_ovr->address; return 0; @@ -990,21 +992,6 @@ static int __init acpi_parse_madt_lapic_entries(void) if (!boot_cpu_has(X86_FEATURE_APIC)) return -ENODEV; - /* - * Note that the LAPIC address is obtained from the MADT (32-bit value) - * and (optionally) overridden by a LAPIC_ADDR_OVR entry (64-bit value). - */ - - count = acpi_table_parse_madt(ACPI_MADT_TYPE_LOCAL_APIC_OVERRIDE, - acpi_parse_lapic_addr_ovr, 0); - if (count < 0) { - printk(KERN_ERR PREFIX - "Error parsing LAPIC address override entry\n"); - return count; - } - - register_lapic_address(acpi_lapic_addr); - count = acpi_table_parse_madt(ACPI_MADT_TYPE_LOCAL_SAPIC, acpi_parse_sapic, MAX_LOCAL_APIC); diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c index 60078a6..504311c 100644 --- a/arch/x86/kernel/apic/apic.c +++ b/arch/x86/kernel/apic/apic.c @@ -1799,7 +1799,7 @@ void __init register_lapic_address(unsigned long address) if (!x2apic_mode) { set_fixmap_nocache(FIX_APIC_BASE, address); apic_printk(APIC_VERBOSE, "mapped APIC to %16lx (%16lx)\n", - APIC_BASE, mp_lapic_addr); + APIC_BASE, address); } if (boot_cpu_physical_apicid == -1U) { boot_cpu_physical_apicid = read_apic_id();
ACPI MADT has a 32-bit field providing lapic address at which each processor can access its lapic information. MADT also contains an optional entry to provide a 64-bit address to override the 32-bit one. However the current code does the lapic address override entry parsing twice. One is in early_acpi_boot_init() because AMD NUMA need get boot_cpu_id earlier. The other is in acpi_boot_init() which parses all MADT entries. So in this patch remove the repeated code in the 2nd part. Meanwhile print lapic override entry information like other MADT entry. Signed-off-by: Baoquan He <bhe@redhat.com> --- v1->v2: -Remove the not correct code comment above early_acpi_boot_init() as Rafael suggested. arch/x86/kernel/acpi/boot.c | 17 ++--------------- arch/x86/kernel/apic/apic.c | 2 +- 2 files changed, 3 insertions(+), 16 deletions(-)