linux-kernel - Re: [RFC PATCH v2 14/35] ACPI: Only enumerate enabled (or functional) devices

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: <ZTKglklxz8PN9VmI@shell.armlinux.org.uk>
Date:   Fri, 20 Oct 2023 16:45:26 +0100
From:   "Russell King (Oracle)" <linux@...linux.org.uk>
To:     Gavin Shan <gshan@...hat.com>
Cc:     James Morse <james.morse@....com>, 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, Salil Mehta <salil.mehta@...wei.com>,
        Jean-Philippe Brucker <jean-philippe@...aro.org>,
        jianyong.wu@....com, justin.he@....com
Subject: Re: [RFC PATCH v2 14/35] ACPI: Only enumerate enabled (or
 functional) devices

On Tue, Sep 19, 2023 at 09:43:46AM +1000, Gavin Shan wrote:
> On 9/14/23 02:38, James Morse wrote:
> > +	if (!device->status.present && !device->status.enabled)
> > +		return device->status.functional;
> > +
> > +	return device->status.present && device->status.enabled;
> >   }
> >   EXPORT_SYMBOL_GPL(acpi_dev_ready_for_enumeration);
> 
> Looking at Salil's latest branch (vcpu-hotplug-RFCv2-rc7), there are 3 possible statuses:
> 
>   0x0       when CPU isn't present
>   0xD       when CPU is present, but not enabled
>   0xF       when CPU is present and enabled
> 
> Previously, the ACPI device is enumerated on 0xD and 0xF. We want to avoid the enumeration
> on 0xD since the processor isn't ready for enumeration in this specific case. The changed
> check (device->status.present && device->status.enabled) can ensure it. So the addition
> of checking @device->state.functional seems irrelevant to ARM64 vCPU hot-add? I guess we
> probably want a relaxation after the condition (device->status.present || device->status.enabled)
> becomes a more strict one (device->status.present && device->status.enabled)

Okay, I'm confused by your comment.

As mentioned in my reply to Jonathan, the current code tests for
device->status.present || device->status.functional, not
device->status.present || device->status.enabled.

Digging back in the history, the acpi_device_is_present() helper
was added in 202317a573b2 "ACPI / scan: Add acpi_device objects for all
device nodes in the namespace". The commit description states:

    Modify the ACPI namespace scanning code to register a struct
    acpi_device object for every namespace node representing a device,
    processor and so on, even if the device represented by that namespace
    node is reported to be not present and not functional by _STA.

It seems the code originally used this test

-       if (!(sta & ACPI_STA_DEVICE_PRESENT) &&
-           !(sta & ACPI_STA_DEVICE_FUNCTIONING)) {

So this commit is just continuing that "tradition".

Digging further back, we find:

778cbc1d3abd ACPI: factor out device type and status checking

-       case ACPI_BUS_TYPE_PROCESSOR:
-       case ACPI_BUS_TYPE_DEVICE:
...
-               /*
-                * When the device is neither present nor functional, the
-                * device should not be added to Linux ACPI device tree.
-                * When the status of the device is not present but functinal,
-                * it should be added to Linux ACPI tree. For example : bay
-                * device , dock device.
-                * In such conditions it is unncessary to check whether it is
-                * bay device or dock device.
-                */
-               if (!device->status.present && !device->status.functional) {

and that comment seems to indicate where the !present && functional
case comes from.

So, I think it's necessary to continue supporting the !present &&
functional case otherwise it seems to me that we'll be regressing some
platforms.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!