| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Tue, 3 Feb 2015 13:53:48 +0000
From: Mark Rutland <mark.rutland@....com>
To: "hanjun.guo@...aro.org" <hanjun.guo@...aro.org>
Cc: Catalin Marinas <Catalin.Marinas@....com>,
"Rafael J. Wysocki" <rjw@...ysocki.net>,
Olof Johansson <olof@...om.net>, Arnd Bergmann <arnd@...db.de>,
"grant.likely@...aro.org" <grant.likely@...aro.org>,
Will Deacon <Will.Deacon@....com>,
Lorenzo Pieralisi <Lorenzo.Pieralisi@....com>,
"graeme.gregory@...aro.org" <graeme.gregory@...aro.org>,
Sudeep Holla <Sudeep.Holla@....com>,
"jcm@...hat.com" <jcm@...hat.com>,
Jason Cooper <jason@...edaemon.net>,
Marc Zyngier <Marc.Zyngier@....com>,
Bjorn Helgaas <bhelgaas@...gle.com>,
Daniel Lezcano <daniel.lezcano@...aro.org>,
Mark Brown <broonie@...nel.org>, Rob Herring <robh@...nel.org>,
Robert Richter <rric@...nel.org>,
Randy Dunlap <rdunlap@...radead.org>,
Charles Garcia-Tobin <Charles.Garcia-Tobin@....com>,
"phoenix.liyi@...wei.com" <phoenix.liyi@...wei.com>,
Timur Tabi <timur@...eaurora.org>,
Ashwin Chaugule <ashwinc@...eaurora.org>,
"suravee.suthikulpanit@....com" <suravee.suthikulpanit@....com>,
Mark Langsdorf <mlangsdo@...hat.com>,
"wangyijing@...wei.com" <wangyijing@...wei.com>,
"linux-acpi@...r.kernel.org" <linux-acpi@...r.kernel.org>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linaro-acpi@...ts.linaro.org" <linaro-acpi@...ts.linaro.org>,
Tomasz Nowicki <tomasz.nowicki@...aro.org>
Subject: Re: [PATCH v8 13/21] ARM64 / ACPI: Parse MADT for SMP initialization
On Mon, Feb 02, 2015 at 12:45:41PM +0000, Hanjun Guo wrote:
> MADT contains the information for MPIDR which is essential for
> SMP initialization, parse the GIC cpu interface structures to
> get the MPIDR value and map it to cpu_logical_map(), and add
> enabled cpu with valid MPIDR into cpu_possible_map.
>
> ACPI 5.1 only has two explicit methods to boot up SMP, PSCI and
> Parking protocol, but the Parking protocol is only specified for
> ARMv7 now, so make PSCI as the only way for the SMP boot protocol
> before some updates for the ACPI spec or the Parking protocol spec.
>
> Parking protocol patches for SMP boot will be sent to upstream when
> the new version of Parking protocol is ready.
>
> CC: Lorenzo Pieralisi <lorenzo.pieralisi@....com>
> CC: Catalin Marinas <catalin.marinas@....com>
> CC: Will Deacon <will.deacon@....com>
> CC: Mark Rutland <mark.rutland@....com>
> Tested-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@....com>
> Tested-by: Yijing Wang <wangyijing@...wei.com>
> Tested-by: Mark Langsdorf <mlangsdo@...hat.com>
> Tested-by: Jon Masters <jcm@...hat.com>
> Tested-by: Timur Tabi <timur@...eaurora.org>
> Signed-off-by: Hanjun Guo <hanjun.guo@...aro.org>
> Signed-off-by: Tomasz Nowicki <tomasz.nowicki@...aro.org>
> ---
> arch/arm64/include/asm/acpi.h | 2 +
> arch/arm64/include/asm/cpu_ops.h | 1 +
> arch/arm64/include/asm/smp.h | 5 +-
> arch/arm64/kernel/acpi.c | 150 ++++++++++++++++++++++++++++++++++++++-
> arch/arm64/kernel/cpu_ops.c | 2 +-
> arch/arm64/kernel/setup.c | 7 +-
> arch/arm64/kernel/smp.c | 2 +-
> 7 files changed, 161 insertions(+), 8 deletions(-)
[...]
> +/**
> + * acpi_map_gic_cpu_interface - generates a logical cpu number
> + * and map to MPIDR represented by GICC structure
> + * @mpidr: CPU's hardware id to register, MPIDR represented in MADT
> + * @enabled: this cpu is enabled or not
> + *
> + * Returns the logical cpu number which maps to MPIDR
> + */
> +static int __init acpi_map_gic_cpu_interface(u64 mpidr, u8 enabled)
> +{
> + int cpu;
> +
> + if (mpidr == INVALID_HWID) {
> + pr_info("Skip MADT cpu entry with invalid MPIDR\n");
> + return -EINVAL;
> + }
> +
> + total_cpus++;
> + if (!enabled)
> + return -EINVAL;
> +
> + if (enabled_cpus >= NR_CPUS) {
> + pr_warn("NR_CPUS limit of %d reached, Processor %d/0x%llx ignored.\n",
> + NR_CPUS, total_cpus, mpidr);
> + return -EINVAL;
> + }
> +
> + /* No need to check duplicate MPIDRs for the first CPU */
> + if (enabled_cpus) {
> + /*
> + * Duplicate MPIDRs are a recipe for disaster. Scan
> + * all initialized entries and check for
> + * duplicates. If any is found just ignore the CPU.
> + */
> + for_each_possible_cpu(cpu) {
> + if (cpu_logical_map(cpu) == mpidr) {
> + pr_err("Firmware bug, duplicate CPU MPIDR: 0x%llx in MADT\n",
> + mpidr);
> + return -EINVAL;
> + }
> + }
> +
> + /* allocate a logical cpu id for the new comer */
> + cpu = cpumask_next_zero(-1, cpu_possible_mask);
> + } else {
> + /*
> + * First GICC entry must be BSP as ACPI spec said
> + * in section 5.2.12.15
> + */
> + if (cpu_logical_map(0) != mpidr) {
> + pr_err("First GICC entry with MPIDR 0x%llx is not BSP\n",
> + mpidr);
> + return -EINVAL;
> + }
> +
> + /*
> + * boot_cpu_init() already hold bit 0 in cpu_possible_mask
> + * for BSP, no need to allocate again.
> + */
> + cpu = 0;
> + }
If/when kexec comes, on systems where CPU0 can be hotplugged the next
kernel might boot on an AP rather than the BSP. Is there a requirement
Linux-side that CPU0 is the BSP, or is this just intended as a sanity
check of the tables the FW provided?
> +
> + if (!acpi_psci_present())
> + return -EOPNOTSUPP;
> +
> + cpu_ops[cpu] = cpu_get_ops("psci");
> + /* CPU 0 was already initialized */
> + if (cpu) {
> + if (!cpu_ops[cpu])
> + return -EINVAL;
> +
> + if (cpu_ops[cpu]->cpu_init(NULL, cpu))
> + return -EOPNOTSUPP;
> +
> + /* map the logical cpu id to cpu MPIDR */
> + cpu_logical_map(cpu) = mpidr;
> +
> + set_cpu_possible(cpu, true);
> + }
In the OF case we only set CPUs possible once we've scanned all the
nodes, and only when the boot CPU was actually found in a table. We
should keep the ACPI case consistent with that.
Can we not handle all of this in a later call once we've scanned all of
the GICC structures?
[...]
> diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
> index 43ae914..1099ddc 100644
> --- a/arch/arm64/kernel/setup.c
> +++ b/arch/arm64/kernel/setup.c
> @@ -449,13 +449,16 @@ void __init setup_arch(char **cmdline_p)
> if (acpi_disabled) {
> unflatten_device_tree();
> psci_dt_init();
> + cpu_read_bootcpu_ops();
> +#ifdef CONFIG_SMP
> + of_smp_init_cpus();
> +#endif
I was going to say that it would be a little nicer if we had empty stubs
for functions in the !SMP case, rather than #ifdefs all over the place.
Unfortunately it looks like the way asm/smp.h is handled is generally a
mess, so this isn't so bad for now.
Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists