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!
