linux-kernel - Re: [PATCH RFC v3 02/21] ACPI: processor: Add support for processors described as container packages

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAJZ5v0g2CFPrSfNzHKBz_Spwt304QEQtR6w57VR11i5APPrD8Q@mail.gmail.com>
Date: Fri, 12 Jan 2024 16:01:40 +0100
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: Jonathan Cameron <Jonathan.Cameron@...wei.com>
Cc: "Russell King (Oracle)" <linux@...linux.org.uk>, "Rafael J. Wysocki" <rafael@...nel.org>, linux-pm@...r.kernel.org, 
	loongarch@...ts.linux.dev, linux-acpi@...r.kernel.org, 
	linux-arch@...r.kernel.org, linux-kernel@...r.kernel.org, 
	linux-arm-kernel@...ts.infradead.org, linux-riscv@...ts.infradead.org, 
	kvmarm@...ts.linux.dev, x86@...nel.org, 
	acpica-devel@...ts.linuxfoundation.org, linux-csky@...r.kernel.org, 
	linux-doc@...r.kernel.org, linux-ia64@...r.kernel.org, 
	linux-parisc@...r.kernel.org, Salil Mehta <salil.mehta@...wei.com>, 
	Jean-Philippe Brucker <jean-philippe@...aro.org>, jianyong.wu@....com, justin.he@....com, 
	James Morse <james.morse@....com>
Subject: Re: [PATCH RFC v3 02/21] ACPI: processor: Add support for processors
 described as container packages

On Fri, Jan 12, 2024 at 10:25 AM Jonathan Cameron
<Jonathan.Cameron@...wei.com> wrote:
>
> On Thu, 11 Jan 2024 18:46:47 +0000
> "Russell King (Oracle)" <linux@...linux.org.uk> wrote:
>
> > On Thu, Jan 11, 2024 at 05:59:08PM +0000, Jonathan Cameron wrote:
> > > On Mon, 18 Dec 2023 21:17:34 +0100
> > > "Rafael J. Wysocki" <rafael@...nel.org> wrote:
> > >
> > > > On Wed, Dec 13, 2023 at 1:49 PM Russell King <rmk+kernel@...linux.org.uk> wrote:
> > > > >
> > > > > From: James Morse <james.morse@....com>
> > >
> > > Done some digging + machine faking.  This is mid stage results at best.
> > >
> > > Summary: I don't think this patch is necessary.  If anyone happens to be in
> > > the mood for testing on various platforms, can you drop this patch and
> > > see if everything still works.
> > >
> > > With this patch in place, and a processor container containing
> > > Processor() objects acpi_process_add is called twice - once via
> > > the path added here and once via acpi_bus_attach etc.
> > >
> > > Maybe it's a left over from earlier approaches to some of this?
> >
> > From what you're saying, it seems that way. It would be really good to
> > get a reply from James to see whether he agrees - or at least get the
> > reason why this patch is in the series... but I suspect that will never
> > come.
> >
> > > Both cases are covered by the existing handling without this.
> > >
> > > I'm far from clear on why we need this patch.  Presumably
> > > it's the reference in the description on it breaking for
> > > Processor Package containing Processor() objects that matters
> > > after a move... I'm struggling to find that move though!
> >
> > I do know that James did a lot of testing, so maybe he found some
> > corner case somewhere which made this necessary - but without input
> > from James, we can't know that.
> >
> > So, maybe the right way forward on this is to re-test the series
> > with this patch dropped, and see whether there's any ill effects.
> > It should be possible to resurect the patch if it does turn out to
> > be necessary.
> >
> > Does that sound like a good way forward?
> >
> > Thanks.
> >
>
> Yes that sounds like the best plan. Note this patch can only make a
> difference on non arm64 arches because it's a firmware bug to combine
> Processor() with a GICC entry in APIC/MADT.  To even test on ARM64
> you have to skip the bug check.
>
> https://elixir.bootlin.com/linux/latest/source/drivers/acpi/processor_core.c#L101
>
>         /* device_declaration means Device object in DSDT, 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)) {
>         if (gicc->uid == acpi_id) {
>                 *mpidr = gicc->arm_mpidr;
>                 return 0;
>         }
>
> Only alternative is probably to go history diving and try and
> find another change that would have required this and is now gone.
>
> The ACPI scanning code has had a lot of changes whilst this work has
> been underway.  More than possible that this was papering over some
> issue that has long since been fixed. I can't find any deliberate
> functional changes, but there is some code generalization that 'might'
> have side effects in this area. Rafael, any expectation that anything
> changed in how scanning processor containers works?

There have been changes, but I can't recall when exactly without some
git history research.

In any case, it is always better to work on top of the current
mainline code IMO.