From: "Rafael J. Wysocki" <rjw@rjwysocki.net>
To: Hanjun Guo <hanjun.guo@linaro.org>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
Catalin Marinas <Catalin.Marinas@arm.com>,
Mark Rutland <Mark.Rutland@arm.com>,
Olof Johansson <olof@lixom.net>,
"grant.likely@linaro.org" <grant.likely@linaro.org>,
Will Deacon <Will.Deacon@arm.com>,
"graeme.gregory@linaro.org" <graeme.gregory@linaro.org>,
Arnd Bergmann <arnd@arndb.de>,
Sudeep Holla <Sudeep.Holla@arm.com>,
"jcm@redhat.com" <jcm@redhat.com>,
Jason Cooper <jason@lakedaemon.net>,
Marc Zyngier <Marc.Zyngier@arm.com>,
Bjorn Helgaas <bhelgaas@google.com>,
Daniel Lezcano <daniel.lezcano@linaro.org>,
Mark Brown <broonie@kernel.org>, Rob Herring <robh@kernel.org>,
Robert Richter <rric@kernel.org>, Lv Zheng <lv.zheng@intel.com>,
Robert Moore <robert.moore@intel.com>,
Liviu Dudau <Liviu.Dudau@arm.com>,
Randy Dunlap <rdunlap@infradead.org>,
Charles Garcia-Tobin <Charles.Garcia-Tobin@arm.com>,
"Kangkang.Shen@huawei.com" <Kangkang.Shen@huawei.com>,
"linux-acpi@vger.kernel.org" <linux-acpi@vger.kernel.org>,
"linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"linaro-acpi@lists.linaro.org" <linaro-acpi@lists.linaro.org>
Subject: Re: [PATCH v5 12/18] ACPI / processor: Make it possible to get CPU hardware ID via GICC
Date: Wed, 29 Oct 2014 22:33:06 +0100 [thread overview]
Message-ID: <4773594.btHp9vXayV@vostro.rjw.lan> (raw)
In-Reply-To: <544E1732.5090902@linaro.org>
On Monday, October 27, 2014 05:58:10 PM Hanjun Guo wrote:
> On 2014-10-25 1:39, Lorenzo Pieralisi wrote:
> > On Fri, Oct 17, 2014 at 02:37:08PM +0100, Hanjun Guo wrote:
> >> Introduce a new function map_gicc_mpidr() to allow MPIDRs to be obtained
> >> from the GICC Structure introduced by ACPI 5.1.
> >>
> >> MPIDR is the CPU hardware ID as local APIC ID on x86 platform, so we use
> >> MPIDR not the GIC CPU interface ID to identify CPUs.
> >>
> >> Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
> >> ---
> >> arch/arm64/include/asm/acpi.h | 29 +++++++++++++++++++++++++++++
> >> arch/arm64/kernel/acpi.c | 1 -
> >> drivers/acpi/processor_core.c | 37 +++++++++++++++++++++++++++++++++++++
> >> 3 files changed, 66 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h
> >> index c82d4a1..483ff45 100644
> >> --- a/arch/arm64/include/asm/acpi.h
> >> +++ b/arch/arm64/include/asm/acpi.h
> >> @@ -12,6 +12,8 @@
> >> #ifndef _ASM_ACPI_H
> >> #define _ASM_ACPI_H
> >>
> >> +#include <asm/smp_plat.h>
> >> +
> >> /* Basic configuration for ACPI */
> >> #ifdef CONFIG_ACPI
> >> #define acpi_strict 1 /* No out-of-spec workarounds on ARM64 */
> >> @@ -45,6 +47,33 @@ static inline void enable_acpi(void)
> >> acpi_noirq = 0;
> >> }
> >>
> >> +/* MPIDR value provided in GICC structure is 64 bits, but the
> >> + * existing apic_id (CPU hardware ID) using in acpi processor
> >> + * driver is 32-bit, to conform to the same datatype we need
> >> + * to repack the GICC structure MPIDR.
> >> + *
> >> + * Only 32 bits of MPIDR are used:
> >> + *
> >> + * Bits [0:7] Aff0;
> >> + * Bits [8:15] Aff1;
> >> + * Bits [16:23] Aff2;
> >> + * Bits [32:39] Aff3;
> >> + */
> >> +static inline u32 pack_mpidr(u64 mpidr)
> >> +{
> >> + return (u32) ((mpidr & 0xff00000000) >> 8) | mpidr;
> >> +}
> >> +
> >> +/*
> >> + * The ACPI processor driver for ACPI core code needs this macro
> >> + * to find out this cpu was already mapped (mapping from CPU hardware
> >> + * ID to CPU logical ID) or not.
> >> + *
> >> + * cpu_logical_map(cpu) is the mapping of MPIDR and the logical cpu,
> >> + * and MPIDR is the cpu hardware ID we needed to pack.
> >> + */
> >> +#define cpu_physical_id(cpu) pack_mpidr(cpu_logical_map(cpu))
> >> +
> >> /*
> >> * It's used from ACPI core in kdump to boot UP system with SMP kernel,
> >> * with this check the ACPI core will not override the CPU index
> >> diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
> >> index 34837e0..e91ec76 100644
> >> --- a/arch/arm64/kernel/acpi.c
> >> +++ b/arch/arm64/kernel/acpi.c
> >> @@ -24,7 +24,6 @@
> >> #include <linux/bootmem.h>
> >> #include <linux/smp.h>
> >>
> >> -#include <asm/smp_plat.h>
> >> #include <asm/cputype.h>
> >> #include <asm/cpu_ops.h>
> >>
> >> diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c
> >> index e32321c..4007313 100644
> >> --- a/drivers/acpi/processor_core.c
> >> +++ b/drivers/acpi/processor_core.c
> >> @@ -64,6 +64,38 @@ static int map_lsapic_id(struct acpi_subtable_header *entry,
> >> return 0;
> >> }
> >>
> >> +/*
> >> + * On ARM platform, MPIDR value is the hardware ID as apic ID
> >> + * on Intel platforms
> >> + */
> >> +static int map_gicc_mpidr(struct acpi_subtable_header *entry,
> >> + int device_declaration, u32 acpi_id, int *mpidr)
> >> +{
> >> + struct acpi_madt_generic_interrupt *gicc =
> >> + container_of(entry, struct acpi_madt_generic_interrupt, header);
> >> +
> >> + if (!(gicc->flags & ACPI_MADT_ENABLED))
> >> + return -ENODEV;
> >> +
> >> + /* In the GIC interrupt model, logical processors are
> >> + * required to have a Processor Device object in the DSDT,
> >> + * so we should check device_declaration here
> >> + */
> >> + if (device_declaration && (gicc->uid == acpi_id)) {
> >> + /*
> >> + * Only bits [0:7] Aff0, bits [8:15] Aff1, bits [16:23] Aff2
> >> + * and bits [32:39] Aff3 are meaningful, so pack the Affx
> >> + * fields into a single 32 bit identifier to accommodate the
> >> + * acpi processor drivers.
> >> + */
> >> + *mpidr = ((gicc->arm_mpidr & 0xff00000000) >> 8)
> >> + | gicc->arm_mpidr;
> >
> > The simple fact that you define a function to pack the mpidr value and
> > you can't use it here because this is *generic* code is telling, and
> > a very bad omen. At the cost of sounding like a broken record, I do not
> > like this mpidr->apic->logical_cpu song and dance at all.
> > ACPI is peppered with code (eg hotplug is another example, CPUidle driver
> > even worse) that is supposed to be generic but contains x86 code to carry
> > out this cpuid conversion, I really think that in order to start an ARM64
> > ACPI port properly we should at least try to factor out this physical to
> > logical cpu id conversion, and it is not the first time that I mention this
> > on the lists.
>
> I know, thanks for pointing this out. As I replied in previous version
> of this patch set, apic_id is x86/ia64 specific, but the meaning behind
> it is not. It means the CPU hardware id to identify itself in the system,
> it just like MPIDR on ARM.
>
> I will send out a patch for RFC to convert apic_id to physid which
> is generic for all platforms.
>
> >
> > I will also talk to Rafael about this at the earliest opportunity, I
> > guess that x86 code relies on apic-id because some ACPI versions could
> > not rely on the acpi-id or some other reasons I have to investigate.
>
> As ACPI spec (section 8.4, Declaring Processors) said, Each processor
> in the system must be declared in the ACPI namespace, so each cpu will
> have acpi_id in all ACPI versions, and in theory we can map acpi_id to
> logical cpu id if we want to.
>
> But things are complicated, apic_id is connected to many
> tables, MADT for smp init, DSDT for device driver, and SRAT for NUMA (there
> is no acpi_id in it, ONLY has apic_id in the table for x86/ia64), so if we
> want to factor the code to map acpi_id to logical cpu id, we need to modify:
>
> - ACPI drivers;
> - SMP init for x86 and ia64
> - the mappings for NUMA init for x86 and ia64
> that will be lots of work I think.
And error prone for that matter.
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
next prev parent reply other threads:[~2014-10-29 21:12 UTC|newest]
Thread overview: 109+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-10-17 13:36 [PATCH v5 00/18] Introduce ACPI for ARM64 based on ACPI 5.1 Hanjun Guo
2014-10-17 13:36 ` [PATCH v5 01/18] ARM64: Move the init of cpu_logical_map(0) before unflatten_device_tree() Hanjun Guo
2014-11-18 13:45 ` Hanjun Guo
2014-11-18 16:43 ` Catalin Marinas
2014-11-18 16:57 ` Will Deacon
2014-11-18 17:02 ` Sudeep Holla
2014-11-18 17:03 ` Will Deacon
2014-11-19 0:29 ` Hanjun Guo
2014-10-17 13:36 ` [PATCH v5 02/18] ACPI / table: Add new function to get table entries Hanjun Guo
2014-11-24 1:27 ` Rafael J. Wysocki
2014-11-24 11:03 ` Hanjun Guo
2014-11-24 14:51 ` Rafael J. Wysocki
2014-11-25 3:38 ` Hanjun Guo
2014-11-25 21:20 ` Rafael J. Wysocki
2014-11-26 1:42 ` Hanjun Guo
2014-10-17 13:36 ` [PATCH v5 03/18] ACPI / table: Count matched and successfully parsed entries without specifying max entries Hanjun Guo
2014-11-18 13:51 ` Hanjun Guo
2014-11-18 20:15 ` Rafael J. Wysocki
2014-11-19 0:34 ` Hanjun Guo
2014-11-24 1:45 ` Rafael J. Wysocki
2014-11-24 8:34 ` Tomasz Nowicki
2014-11-24 15:16 ` Rafael J. Wysocki
2014-11-24 15:01 ` Tomasz Nowicki
2014-11-24 15:37 ` Rafael J. Wysocki
2014-11-24 15:18 ` Tomasz Nowicki
2014-10-17 13:37 ` [PATCH v5 04/18] ARM64 / ACPI: Get RSDP and ACPI boot-time tables Hanjun Guo
2014-10-17 13:37 ` [PATCH v5 05/18] ARM64 / ACPI: Introduce sleep-arm.c Hanjun Guo
2014-10-17 13:37 ` [PATCH v5 06/18] ARM64 / ACPI: Introduce early_param for "acpi" and pass acpi=force to enable ACPI Hanjun Guo
2014-10-17 13:37 ` [PATCH v5 07/18] ARM64 / ACPI: If we chose to boot from acpi then disable FDT Hanjun Guo
2014-10-17 13:37 ` [PATCH v5 08/18] ARM64 / ACPI: Make PCI optional for ACPI on ARM64 Hanjun Guo
2014-10-17 13:37 ` [PATCH v5 09/18] ARM64 / ACPI: Parse FADT table to get PSCI flags for PSCI init Hanjun Guo
2014-10-17 13:37 ` [PATCH v5 10/18] ACPI / table: Print GIC information when MADT is parsed Hanjun Guo
2014-10-17 13:37 ` [PATCH v5 11/18] ARM64 / ACPI: Parse MADT for SMP initialization Hanjun Guo
2014-10-17 13:37 ` [PATCH v5 12/18] ACPI / processor: Make it possible to get CPU hardware ID via GICC Hanjun Guo
2014-10-24 17:39 ` Lorenzo Pieralisi
2014-10-27 9:58 ` Hanjun Guo
2014-10-29 10:43 ` Lorenzo Pieralisi
2014-10-30 8:27 ` Hanjun Guo
2014-10-29 21:33 ` Rafael J. Wysocki [this message]
2014-10-30 8:30 ` Hanjun Guo
2014-10-17 13:37 ` [PATCH v5 13/18] ARM64 / ACPI: Introduce ACPI_IRQ_MODEL_GIC and register device's gsi Hanjun Guo
2014-10-17 13:37 ` [PATCH v5 14/18] ARM64 / ACPI: Add GICv2 specific ACPI boot support Hanjun Guo
2014-10-17 13:37 ` [PATCH v5 15/18] ARM64 / ACPI: Parse GTDT to initialize arch timer Hanjun Guo
2014-10-17 13:37 ` [PATCH v5 16/18] ARM64 / ACPI: Select ACPI_REDUCED_HARDWARE_ONLY if ACPI is enabled on ARM64 Hanjun Guo
2014-10-17 13:37 ` [PATCH v5 17/18] ARM64 / ACPI: Enable ARM64 in Kconfig Hanjun Guo
2014-10-17 13:37 ` [PATCH v5 18/18] Documentation: ACPI for ARM64 Hanjun Guo
2014-12-18 20:01 ` Suravee Suthikulanit
2014-12-19 13:04 ` Hanjun Guo
2014-12-18 20:04 ` Timur Tabi
2014-12-19 13:53 ` Hanjun Guo
2014-12-24 17:18 ` Catalin Marinas
2014-12-24 19:33 ` Jon Masters
2014-12-26 13:23 ` Mark Brown
2014-12-30 11:23 ` Hanjun Guo
2015-01-05 13:13 ` Catalin Marinas
2015-01-05 20:16 ` Arnd Bergmann
2015-01-06 11:20 ` Catalin Marinas
2015-01-06 13:51 ` G Gregory
2015-01-06 14:03 ` Catalin Marinas
2015-01-06 13:59 ` [Linaro-acpi] " Arnd Bergmann
2015-01-06 14:11 ` Catalin Marinas
2015-01-06 19:30 ` Arnd Bergmann
2015-01-15 14:10 ` Grant Likely
2015-01-15 15:51 ` Jon Masters
2015-01-15 16:52 ` Arnd Bergmann
2015-01-15 17:22 ` Al Stone
2015-01-16 16:35 ` Arnd Bergmann
2015-01-15 18:00 ` Mark Brown
2015-01-06 16:24 ` Jon Masters
2015-01-06 19:21 ` [Linaro-acpi] " Arnd Bergmann
2015-01-06 22:06 ` Jon Masters
2015-01-07 4:55 ` Jon Masters
2015-01-07 10:36 ` Arnd Bergmann
2015-01-07 11:50 ` Catalin Marinas
2015-01-07 13:06 ` Arnd Bergmann
2015-01-07 17:27 ` Mark Brown
2015-01-07 17:44 ` Jon Masters
2015-01-07 19:48 ` Arnd Bergmann
2015-01-07 20:05 ` Mark Brown
2015-01-07 20:14 ` Jon Masters
2015-01-09 10:33 ` Catalin Marinas
2015-01-09 10:55 ` Arnd Bergmann
2015-01-09 15:13 ` Catalin Marinas
2015-01-07 18:41 ` Jason Cooper
2015-01-07 19:58 ` Jon Masters
2015-01-07 20:05 ` Jon Masters
2015-01-07 22:59 ` Jason Cooper
2015-01-08 11:26 ` Arnd Bergmann
2015-01-08 19:59 ` Kangkang Shen
2015-01-07 21:40 ` Jason Cooper
2015-01-07 22:10 ` Jon Masters
2015-01-04 9:39 ` Hanjun Guo
2015-01-05 11:05 ` Catalin Marinas
2015-01-06 11:11 ` Hanjun Guo
2015-01-06 11:29 ` Catalin Marinas
2015-01-06 13:50 ` Hanjun Guo
2015-01-06 13:54 ` G Gregory
2015-01-06 13:59 ` Hanjun Guo
2015-01-06 14:05 ` Arnd Bergmann
2015-01-06 14:16 ` Catalin Marinas
2015-01-06 14:37 ` Charles Garcia-Tobin
2015-01-06 16:37 ` Jon Masters
2015-01-09 23:12 ` Arnd Bergmann
[not found] ` <CAJ5Y-eZ5cu9_OhG24yAv+CZq7zKg0vU+eVGekyN+9dDzaz1OhQ@mail.gmail.com>
2014-12-30 20:13 ` ashwinc
2014-12-31 8:34 ` Hanjun Guo
2014-12-31 15:08 ` ashwinc
2015-01-01 20:04 ` Graeme Gregory
2015-01-02 9:28 ` Hanjun Guo
2015-01-02 16:47 ` Catalin Marinas
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4773594.btHp9vXayV@vostro.rjw.lan \
--to=rjw@rjwysocki.net \
--cc=Catalin.Marinas@arm.com \
--cc=Charles.Garcia-Tobin@arm.com \
--cc=Kangkang.Shen@huawei.com \
--cc=Liviu.Dudau@arm.com \
--cc=Marc.Zyngier@arm.com \
--cc=Mark.Rutland@arm.com \
--cc=Sudeep.Holla@arm.com \
--cc=Will.Deacon@arm.com \
--cc=arnd@arndb.de \
--cc=bhelgaas@google.com \
--cc=broonie@kernel.org \
--cc=daniel.lezcano@linaro.org \
--cc=graeme.gregory@linaro.org \
--cc=grant.likely@linaro.org \
--cc=hanjun.guo@linaro.org \
--cc=jason@lakedaemon.net \
--cc=jcm@redhat.com \
--cc=linaro-acpi@lists.linaro.org \
--cc=linux-acpi@vger.kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=lorenzo.pieralisi@arm.com \
--cc=lv.zheng@intel.com \
--cc=olof@lixom.net \
--cc=rdunlap@infradead.org \
--cc=robert.moore@intel.com \
--cc=robh@kernel.org \
--cc=rric@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).