On 2014年01月23日 23:56, Tomasz Nowicki wrote:
Hi Lorenzo,
W dniu 22.01.2014 12:54, Lorenzo Pieralisi pisze:
On Fri, Jan 17, 2014 at 12:24:58PM +0000, Hanjun Guo wrote:
[...]
diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c index bd9bbd0..2210353 100644 --- a/arch/arm64/kernel/setup.c +++ b/arch/arm64/kernel/setup.c @@ -41,6 +41,7 @@ #include <linux/memblock.h> #include <linux/of_fdt.h> #include <linux/of_platform.h> +#include <linux/acpi.h>
#include <asm/cputype.h> #include <asm/elf.h> @@ -225,6 +226,11 @@ void __init setup_arch(char **cmdline_p)
arm64_memblock_init();
- /* Parse the ACPI tables for possible boot-time configuration */
- acpi_boot_table_init();
- early_acpi_boot_init();
- acpi_boot_init();
paging_init();
Can I ask you please why we need to parse ACPI tables before paging_init() ?
This is for future usage and because of couple of reasons. Mainly SRAT table parsing should be done (before paging_init()) for proper NUMA initialization and then paging_init().
Yes, I agree, thanks for Tomasz's clarification.
[...]
+/*
- __acpi_map_table() will be called before page_init(), so
early_ioremap()
- or early_memremap() should be called here.
Again, why is this needed ? What's needed before paging_init() from ACPI ?
[...]
+/*
- acpi_boot_table_init() and acpi_boot_init()
- called from setup_arch(), always.
- checksums all tables
- enumerates lapics
- enumerates io-apics
- acpi_table_init() is separated to allow reading SRAT without
- other side effects.
- */
+void __init acpi_boot_table_init(void) +{
- /*
- If acpi_disabled, bail out
- */
- if (acpi_disabled)
- return;
- /*
- Initialize the ACPI boot-time table parser.
- */
- if (acpi_table_init()) {
- disable_acpi();
- return;
- }
+}
+int __init early_acpi_boot_init(void) +{
- /*
- If acpi_disabled, bail out
- */
- if (acpi_disabled)
- return -ENODEV;
- /*
- Process the Multiple APIC Description Table (MADT), if present
- */
- early_acpi_process_madt();
- return 0;
+}
+int __init acpi_boot_init(void) +{
- /*
- If acpi_disabled, bail out
- */
- if (acpi_disabled)
- return -ENODEV;
- acpi_table_parse(ACPI_SIG_FADT, acpi_parse_fadt);
- /*
- Process the Multiple APIC Description Table (MADT), if present
- */
- acpi_process_madt();
- return 0;
+}
Well, apart from having three init calls, one returning void and two returning proper values, do not understand why, and do not understand why we need three calls in the first place...why should we process MADT twice in two separate calls ? What is supposed to change in between that prevents you from merging the two together ?
Thanks for pointing this out. I can merge acpi_boot_table_init() and early_acpi_boot_init() together, but can not merge early_acpi_boot_init() and acpi_boot_init() together.
early_acpi_boot_init() and acpi_boot_init() was separated intentionally for memory hotplug reasons. memory allocated in this stage can not be migrated and cause memory hot-remove failed, in order to keep memory allocated at base node (general NUMA node 0 in the system) at boot stage, we should parse SRAT first before CPU is enumerated, does this make sense to you?
Thanks Hanjun