Re: [Linaro-acpi] [PATCH 04/20] ARM64 / ACPI: Introduce arm_core.cand its related head file

[Hanjun Guo]

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.
> > > + * 1. checksums all tables
> > > + * 2. enumerates lapics
> > > + * 3. 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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/