Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) client-ip=2620:137:e000::3:7;
Date:   Fri, 20 Oct 2023 16:45:26 +0100
From:   "Russell King (Oracle)" <linux@armlinux.org.uk>
To:     Gavin Shan <gshan@redhat.com>
Cc:     James Morse <james.morse@arm.com>, 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, Salil Mehta <salil.mehta@huawei.com>,
        Jean-Philippe Brucker <jean-philippe@linaro.org>,
        jianyong.wu@arm.com, justin.he@arm.com
Subject: Re: [RFC PATCH v2 14/35] ACPI: Only enumerate enabled (or
 functional) devices
Message-ID: <ZTKglklxz8PN9VmI@shell.armlinux.org.uk>
References: <20230913163823.7880-1-james.morse@arm.com>
 <20230913163823.7880-15-james.morse@arm.com>
 <ebfa8b5c-c09f-a1e6-e6ec-f4f3cda9de03@redhat.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <ebfa8b5c-c09f-a1e6-e6ec-f4f3cda9de03@redhat.com>
Sender: Russell King (Oracle) <linux@armlinux.org.uk>
Precedence: bulk

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!