| Message ID | 1469877001-18393-2-git-send-email-bhe@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, this > will be added to boot log. it is not at all clear to me from this changelog whether the change is supposed to change anything. If not then please spell it out explicitly: "This patch is not supposed to change any behavior." Or so. 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 08/10/16 at 04:02pm, 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, this > > will be added to boot log. > > it is not at all clear to me from this changelog whether the change is supposed to > change anything. If not then please spell it out explicitly: > > "This patch is not supposed to change any behavior." I don't know if adding new information to boot log can be seen as behavior change. If lapic override entry exist, the code change will add one line of message to boot log: LAPIC_ADDR_OVR (address[0xXXXXXXXX]) If this is not behavior change, I will add the sentence you suggested. 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
* Baoquan He <bhe@redhat.com> wrote: > On 08/10/16 at 04:02pm, 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, this > > > will be added to boot log. > > > > it is not at all clear to me from this changelog whether the change is supposed to > > change anything. If not then please spell it out explicitly: > > > > "This patch is not supposed to change any behavior." > > I don't know if adding new information to boot log can be seen as > behavior change. If lapic override entry exist, the code change will > add one line of message to boot log: > > LAPIC_ADDR_OVR (address[0xXXXXXXXX]) > > If this is not behavior change, I will add the sentence you suggested. Yeah, you can write it: "This patch is not supposed to change any runtime behavior, other than improving kernel messages." 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 08/11/16 at 10:37am, Ingo Molnar wrote: > > * Baoquan He <bhe@redhat.com> wrote: > > > On 08/10/16 at 04:02pm, 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, this > > > > will be added to boot log. > > > > > > it is not at all clear to me from this changelog whether the change is supposed to > > > change anything. If not then please spell it out explicitly: > > > > > > "This patch is not supposed to change any behavior." > > > > I don't know if adding new information to boot log can be seen as > > behavior change. If lapic override entry exist, the code change will > > add one line of message to boot log: > > > > LAPIC_ADDR_OVR (address[0xXXXXXXXX]) > > > > If this is not behavior change, I will add the sentence you suggested. > > Yeah, you can write it: > > "This patch is not supposed to change any runtime behavior, other than > improving kernel messages." Got it, will copy this to patch log and repost. Thanks a lot! 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 6738e5c..9f5b332 100644 --- a/arch/x86/kernel/acpi/boot.c +++ b/arch/x86/kernel/acpi/boot.c @@ -282,6 +282,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; @@ -998,21 +1000,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 ac8d8ad..346ec91 100644 --- a/arch/x86/kernel/apic/apic.c +++ b/arch/x86/kernel/apic/apic.c @@ -1801,7 +1801,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, this will be added to boot log. Signed-off-by: Baoquan He <bhe@redhat.com> --- v1->v2: -Remove the incorrect code comment above early_acpi_boot_init() as Rafael suggested. v2->v3: -Spell out that lapic override entry information is added to boot log as Ingo suggested. arch/x86/kernel/acpi/boot.c | 17 ++--------------- arch/x86/kernel/apic/apic.c | 2 +- 2 files changed, 3 insertions(+), 16 deletions(-)