Re: [PATCH v2 6/6] ACPI: scan: Take PCI device enumeration into account directly

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Bypassing `acpi_bus_get_status()` for PCI companions leaves `device->status` stale, which can cause `acpi_device_is_present()` to incorrectly return false.
--

commit 7f00077fea1f2ada305a470b1a050f31934232c4
Author: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

ACPI: scan: Take PCI device enumeration into account directly

This commit skips evaluating ACPI device status during hierarchy walks
for devices already associated with a PCI companion, assuming they are
present and functional because the PCI devices are already enumerated.

> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> index f4718b0207e0e..34f7f477a25ad 100644
> --- a/drivers/acpi/scan.c
> +++ b/drivers/acpi/scan.c
[ ... ]
> @@ -2353,10 +2355,26 @@ static int attach_subtree(struct acpi_device *device, void *not_used)
>  	if (ACPI_SUCCESS(acpi_bus_get_ejd(device->handle, &ejd)))
>  		register_dock_dependent_device(device, ejd);
>  
> -	acpi_bus_get_status(device);
> -	/* Skip devices that are not ready for enumeration (e.g. not present) */
> -	if (!acpi_dev_ready_for_enumeration(device))
> -		return 0;
> +	/*
> +	 * If the given ACPI device object has been already associated with a
> +	 * PCI device found on the bus, its status is effectively "present
> +	 * and functional".
> +	 */
> +	pci = acpi_dev_get_pci_dev(device);
> +	if (pci) {
> +		acpi_handle_debug(device->handle, "PCI companion %s found\n",
> +				  pci_name(pci));
> +
> +		pci_dev_put(pci);
> +	} else {

[Severity: High]
Bypassing acpi_bus_get_status() for PCI companions leaves device->status
stale. Could this cause acpi_device_is_present() to incorrectly return false?

If the initial evaluation of the device status returned 0 (e.g., due to unmet
dependencies at boot or firmware quirks), device->status.present will remain
0 permanently because it is not explicitly updated here to reflect the
"present and functional" assumption.

This stale status might prevent ACPI power management features, such as
acpi_device_fix_up_power(), and other subsystems relying on
acpi_device_is_present() from properly operating on the device.

> +		acpi_bus_get_status(device);
> +		/*
> +		 * Skip devices that are not ready for enumeration (e.g. not
> +		 * present).
> +		 */
> +		if (!acpi_dev_ready_for_enumeration(device))
> +			return 0;
> +	}
>  
>  	acpi_ec_register_opregions(device);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/5144065.31r3eYUQgx@rafael.j.wysocki?part=6