From: Jonathan Cameron <Jonathan.Cameron@Huawei.com>
To: "Russell King (Oracle)" <linux@armlinux.org.uk>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>,
<linux-pm@vger.kernel.org>, <loongarch@lists.linux.dev>,
<linux-acpi@vger.kernel.org>, <linux-arch@vger.kernel.org>,
<linux-kernel@vger.kernel.org>,
<linux-arm-kernel@lists.infradead.org>,
<linux-riscv@lists.infradead.org>, <kvmarm@lists.linux.dev>,
<x86@kernel.org>, <acpica-devel@lists.linuxfoundation.org>,
<linux-csky@vger.kernel.org>, <linux-doc@vger.kernel.org>,
<linux-ia64@vger.kernel.org>, <linux-parisc@vger.kernel.org>,
Salil Mehta <salil.mehta@huawei.com>,
Jean-Philippe Brucker <jean-philippe@linaro.org>,
<jianyong.wu@arm.com>, <justin.he@arm.com>,
James Morse <james.morse@arm.com>
Subject: Re: [PATCH RFC v3 03/21] ACPI: processor: Register CPUs that are online, but not described in the DSDT
Date: Mon, 22 Jan 2024 16:02:27 +0000 [thread overview]
Message-ID: <20240122160227.00002d83@Huawei.com> (raw)
In-Reply-To: <ZaURtUvWQyjYfiiO@shell.armlinux.org.uk>
On Mon, 15 Jan 2024 11:06:29 +0000
"Russell King (Oracle)" <linux@armlinux.org.uk> wrote:
> On Mon, Dec 18, 2023 at 09:22:03PM +0100, Rafael J. Wysocki wrote:
> > On Wed, Dec 13, 2023 at 1:49 PM Russell King <rmk+kernel@armlinux.org.uk> wrote:
> > >
> > > From: James Morse <james.morse@arm.com>
> > >
> > > ACPI has two descriptions of CPUs, one in the MADT/APIC table, the other
> > > in the DSDT. Both are required. (ACPI 6.5's 8.4 "Declaring Processors"
> > > says "Each processor in the system must be declared in the ACPI
> > > namespace"). Having two descriptions allows firmware authors to get
> > > this wrong.
> > >
> > > If CPUs are described in the MADT/APIC, they will be brought online
> > > early during boot. Once the register_cpu() calls are moved to ACPI,
> > > they will be based on the DSDT description of the CPUs. When CPUs are
> > > missing from the DSDT description, they will end up online, but not
> > > registered.
> > >
> > > Add a helper that runs after acpi_init() has completed to register
> > > CPUs that are online, but weren't found in the DSDT. Any CPU that
> > > is registered by this code triggers a firmware-bug warning and kernel
> > > taint.
> > >
> > > Qemu TCG only describes the first CPU in the DSDT, unless cpu-hotplug
> > > is configured.
> >
> > So why is this a kernel problem?
>
> So what are you proposing should be the behaviour here? What this
> statement seems to be saying is that QEMU as it exists today only
> describes the first CPU in DSDT.
This confuses me somewhat, because I'm far from sure which machines this
is true for in QEMU. I'm guessing it's a legacy thing with
some old distro version of QEMU - so we'll have to paper over it anyway
but for current QEMU I'm not sure it's true.
Helpfully there are a bunch of ACPI table tests so I've been checking
through all the multi CPU cases.
CPU hotplug not enabled.
pc/DSDT.dimmpxm - 4x Processor entries. -smp 4
pc/DSDT.acpihmat - 2x Processor entries. -smp 2
q35/DSDT.acpihmat - 2x Processor entries. -smp 2
virt/DSDT.acpihmatvirt - 4x ACPI0007 entries -smp 4
q35/DSDT.acpihmat-noinitiator - 4 x Processor () entries -smp 4
virt/DSDT.topology - 8x ACPI0007 entries
I've also looked at the code and we have various types of
CPU hotplug on x86 but they all build appropriate numbers of
Processor() entries in DSDT.
Arm likewise seems to build the right number of ACPI0007 entries
(and doesn't yet have CPU HP support).
If anyone can add a reference on why this is needed that would be very
helpful.
>
> As this patch series changes when arch_register_cpu() gets called (as
> described in the paragraph above) we obviously need to preserve the
> _existing_ behaviour to avoid causing regressions. So, if changing the
> kernel causes user visible regressions (e.g. sysfs entries to
> disappear) then it obviously _is_ a kernel problem that needs to be
> solved.
>
> We can't say "well fix QEMU then" without invoking the wrath of Linus.
Overall I'm fine with the defensive nature of this patch as there
'might' be firmware out there with this problem - I just can't establish
that there is! If anyone else recalls the history of this then give
a shout. I vaguely wondered if this was an ia64 thing but nope, QEMU
never generated tables for ia64 before dropping support back in QEMU 2.11
>
> > > Signed-off-by: James Morse <james.morse@arm.com>
> > > Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > > Reviewed-by: Gavin Shan <gshan@redhat.com>
> > > Tested-by: Miguel Luis <miguel.luis@oracle.com>
> > > Tested-by: Vishnu Pajjuri <vishnu@os.amperecomputing.com>
> > > Tested-by: Jianyong Wu <jianyong.wu@arm.com>
> > > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
> > > ---
> > > drivers/acpi/acpi_processor.c | 19 +++++++++++++++++++
> > > 1 file changed, 19 insertions(+)
> > >
> > > diff --git a/drivers/acpi/acpi_processor.c b/drivers/acpi/acpi_processor.c
> > > index 6a542e0ce396..0511f2bc10bc 100644
> > > --- a/drivers/acpi/acpi_processor.c
> > > +++ b/drivers/acpi/acpi_processor.c
> > > @@ -791,6 +791,25 @@ void __init acpi_processor_init(void)
> > > acpi_pcc_cpufreq_init();
> > > }
> > >
> > > +static int __init acpi_processor_register_missing_cpus(void)
> > > +{
> > > + int cpu;
> > > +
> > > + if (acpi_disabled)
> > > + return 0;
> > > +
> > > + for_each_online_cpu(cpu) {
> > > + if (!get_cpu_device(cpu)) {
> > > + pr_err_once(FW_BUG "CPU %u has no ACPI namespace description!\n", cpu);
> > > + add_taint(TAINT_FIRMWARE_WORKAROUND, LOCKDEP_STILL_OK);
> > > + arch_register_cpu(cpu);
> >
> > Which part of this code is related to ACPI?
>
> That's a good question, and I suspect it would be more suited to being
> placed in drivers/base/cpu.c except for the problem that the error
> message refers to ACPI.
>
> As long as we keep the acpi_disabled test, I guess that's fine.
> cpu_dev_register_generic() there already tests acpi_disabled.
>
Moving it seems fine to me.
Jonathan
next prev parent reply other threads:[~2024-01-22 16:02 UTC|newest]
Thread overview: 121+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-12-13 12:47 [RFC PATCH v3 00/21] ACPI/arm64: add support for virtual cpu hotplug Russell King (Oracle)
2023-12-13 12:49 ` [PATCH RFC v3 01/21] ACPI: Only enumerate enabled (or functional) devices Russell King
2023-12-14 17:32 ` Jonathan Cameron
2023-12-14 17:47 ` Rafael J. Wysocki
2023-12-14 18:10 ` Russell King (Oracle)
2023-12-14 18:16 ` Rafael J. Wysocki
2023-12-14 18:37 ` Rafael J. Wysocki
2023-12-15 15:31 ` Russell King (Oracle)
2023-12-15 16:15 ` Jonathan Cameron
2023-12-15 19:47 ` Rafael J. Wysocki
2024-01-02 14:39 ` Jonathan Cameron
2024-01-11 10:19 ` Jonathan Cameron
2024-01-11 10:26 ` Russell King (Oracle)
2024-01-12 11:52 ` Jonathan Cameron
2024-01-29 14:55 ` Russell King (Oracle)
2024-01-29 15:05 ` Rafael J. Wysocki
2024-01-29 15:16 ` Russell King (Oracle)
2024-01-29 15:34 ` Rafael J. Wysocki
2024-01-22 7:31 ` Gavin Shan
2023-12-14 17:55 ` Russell King (Oracle)
2023-12-13 12:49 ` [PATCH RFC v3 02/21] ACPI: processor: Add support for processors described as container packages Russell King
2023-12-14 17:36 ` Jonathan Cameron
2023-12-14 17:57 ` Russell King (Oracle)
2023-12-18 20:17 ` Rafael J. Wysocki
2024-01-09 15:49 ` Russell King (Oracle)
2024-01-09 16:05 ` Rafael J. Wysocki
2024-01-09 16:13 ` Russell King (Oracle)
2024-01-11 16:17 ` Jonathan Cameron
2024-01-11 17:59 ` Jonathan Cameron
2024-01-11 18:46 ` Russell King (Oracle)
2024-01-12 9:25 ` Jonathan Cameron
2024-01-12 15:01 ` Rafael J. Wysocki
2024-01-12 15:03 ` Jonathan Cameron
2024-01-15 10:47 ` Russell King (Oracle)
2023-12-13 12:49 ` [PATCH RFC v3 03/21] ACPI: processor: Register CPUs that are online, but not described in the DSDT Russell King
2023-12-18 20:22 ` Rafael J. Wysocki
2024-01-15 11:06 ` Russell King (Oracle)
2024-01-22 16:02 ` Jonathan Cameron [this message]
2024-01-22 16:22 ` Rafael J. Wysocki
2024-01-22 17:30 ` Russell King (Oracle)
2024-01-23 9:27 ` Jonathan Cameron
2024-01-25 13:56 ` Miguel Luis
2024-01-25 14:42 ` Rafael J. Wysocki
2024-01-29 13:03 ` Jonathan Cameron
2024-01-29 15:32 ` Russell King (Oracle)
2024-01-22 17:27 ` Russell King (Oracle)
2023-12-13 12:49 ` [PATCH RFC v3 04/21] ACPI: processor: Register all CPUs from acpi_processor_get_info() Russell King
2023-12-14 17:38 ` Jonathan Cameron
2023-12-18 20:30 ` Rafael J. Wysocki
2024-01-22 17:44 ` Jonathan Cameron
2024-01-22 18:03 ` Rafael J. Wysocki
2024-01-22 21:56 ` Russell King (Oracle)
2023-12-13 12:49 ` [PATCH RFC v3 05/21] ACPI: Rename ACPI_HOTPLUG_CPU to include 'present' Russell King
2023-12-14 17:41 ` Jonathan Cameron
2023-12-14 18:00 ` Russell King (Oracle)
2023-12-18 20:35 ` Rafael J. Wysocki
2024-01-22 18:00 ` Jonathan Cameron
2024-01-23 13:28 ` Russell King (Oracle)
2024-01-23 16:15 ` Rafael J. Wysocki
2024-01-23 16:36 ` Russell King (Oracle)
2024-01-23 17:43 ` Rafael J. Wysocki
2024-01-23 18:19 ` Russell King (Oracle)
2024-01-23 18:26 ` Rafael J. Wysocki
2024-01-23 18:59 ` Russell King (Oracle)
2024-01-23 19:27 ` Rafael J. Wysocki
2024-01-23 20:09 ` Russell King (Oracle)
2024-01-23 20:17 ` Rafael J. Wysocki
2024-01-23 20:57 ` Russell King (Oracle)
2024-01-23 21:12 ` Russell King (Oracle)
2024-01-23 22:05 ` Rafael J. Wysocki
2024-01-24 8:45 ` Russell King (Oracle)
2023-12-13 12:49 ` [PATCH RFC v3 06/21] ACPI: Move acpi_bus_trim_one() before acpi_scan_hot_remove() Russell King
2023-12-13 12:49 ` [PATCH RFC v3 07/21] ACPI: Rename acpi_processor_hotadd_init and remove pre-processor guards Russell King
2023-12-14 17:43 ` Jonathan Cameron
2023-12-14 18:03 ` Russell King (Oracle)
2023-12-13 12:49 ` [PATCH RFC v3 08/21] ACPI: Add post_eject to struct acpi_scan_handler for cpu hotplug Russell King
2023-12-13 12:49 ` [PATCH RFC v3 09/21] ACPI: convert acpi_processor_post_eject() to use IS_ENABLED() Russell King (Oracle)
2023-12-15 16:11 ` Jonathan Cameron
2023-12-13 12:50 ` [PATCH RFC v3 10/21] ACPI: Check _STA present bit before making CPUs not present Russell King
2023-12-15 16:18 ` Jonathan Cameron
2023-12-13 12:50 ` [PATCH RFC v3 11/21] ACPI: Warn when the present bit changes but the feature is not enabled Russell King
2023-12-13 12:50 ` [PATCH RFC v3 12/21] arm64: acpi: Move get_cpu_for_acpi_id() to a header Russell King
2023-12-13 12:50 ` [PATCH RFC v3 13/21] ACPICA: Add new MADT GICC flags fields Russell King
2023-12-15 16:23 ` Jonathan Cameron
2023-12-15 16:53 ` Russell King (Oracle)
2023-12-18 9:23 ` Lorenzo Pieralisi
2023-12-18 13:14 ` Rafael J. Wysocki
2023-12-18 16:28 ` Lorenzo Pieralisi
2023-12-27 11:15 ` Lorenzo Pieralisi
2023-12-13 12:50 ` [PATCH RFC v3 14/21] irqchip/gic-v3: Don't return errors from gic_acpi_match_gicc() Russell King
2023-12-15 16:33 ` Jonathan Cameron
2024-01-09 19:27 ` Russell King (Oracle)
2024-01-23 10:08 ` Jonathan Cameron
2023-12-13 12:50 ` [PATCH RFC v3 15/21] irqchip/gic-v3: Add support for ACPI's disabled but 'online capable' CPUs Russell King
2023-12-15 16:38 ` Jonathan Cameron
2023-12-13 12:50 ` [PATCH RFC v3 16/21] arm64: psci: Ignore DENIED CPUs Russell King
2023-12-15 16:40 ` Jonathan Cameron
2023-12-13 12:50 ` [PATCH RFC v3 17/21] ACPI: add support to register CPUs based on the _STA enabled bit Russell King
2023-12-18 13:03 ` Russell King (Oracle)
2024-01-02 14:53 ` Jonathan Cameron
2024-01-23 10:26 ` Jonathan Cameron
2024-01-23 13:10 ` Russell King (Oracle)
2024-01-23 14:22 ` Jonathan Cameron
2024-01-23 14:59 ` Russell King (Oracle)
2023-12-13 12:50 ` [PATCH RFC v3 18/21] ACPI: processor: Only call arch_unregister_cpu() if HOTPLUG_CPU is selected Russell King
2023-12-15 16:50 ` Jonathan Cameron
2023-12-18 12:58 ` Russell King (Oracle)
2024-01-23 10:29 ` Jonathan Cameron
2023-12-13 12:50 ` [PATCH RFC v3 19/21] arm64: document virtual CPU hotplug's expectations Russell King
2023-12-15 17:04 ` Jonathan Cameron
2023-12-13 12:50 ` [PATCH RFC v3 20/21] ACPI: Add _OSC bits to advertise OS support for toggling CPU present/enabled Russell King
2023-12-15 17:12 ` Jonathan Cameron
2024-01-02 13:07 ` Jose Marinho
2024-01-02 15:16 ` Jonathan Cameron
2024-01-02 15:35 ` Jose Marinho
2024-01-23 10:51 ` Jonathan Cameron
2023-12-13 12:50 ` [PATCH RFC v3 21/21] cpumask: Add enabled cpumask for present CPUs that can be brought online Russell King
2023-12-15 17:18 ` Jonathan Cameron
2023-12-18 12:14 ` Russell King (Oracle)
2024-01-02 15:19 ` Jonathan Cameron
2023-12-15 19:40 ` Thomas Gleixner
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=20240122160227.00002d83@Huawei.com \
--to=jonathan.cameron@huawei.com \
--cc=acpica-devel@lists.linuxfoundation.org \
--cc=james.morse@arm.com \
--cc=jean-philippe@linaro.org \
--cc=jianyong.wu@arm.com \
--cc=justin.he@arm.com \
--cc=kvmarm@lists.linux.dev \
--cc=linux-acpi@vger.kernel.org \
--cc=linux-arch@vger.kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-csky@vger.kernel.org \
--cc=linux-doc@vger.kernel.org \
--cc=linux-ia64@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-parisc@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=linux-riscv@lists.infradead.org \
--cc=linux@armlinux.org.uk \
--cc=loongarch@lists.linux.dev \
--cc=rafael@kernel.org \
--cc=salil.mehta@huawei.com \
--cc=x86@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 read-only IMAP folder(s) and NNTP newsgroup(s).