linux-kernel - Re: [PATCH] ACPI: scan: Release PM resources blocked by unused objects

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] [day] [month] [year] [list]
Message-ID: <3e7b5acc-a3f8-09e2-7d83-bd0b58ba031a@amd.com>
Date:   Tue, 12 Oct 2021 16:10:16 -0500
From:   "Limonciello, Mario" <mario.limonciello@....com>
To:     "Rafael J. Wysocki" <rjw@...ysocki.net>,
        Linux ACPI <linux-acpi@...r.kernel.org>
Cc:     Linux PCI <linux-pci@...r.kernel.org>,
        Linux PM <linux-pm@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] ACPI: scan: Release PM resources blocked by unused
 objects

On 10/9/2021 09:22, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
> 
> On some systems the ACPI namespace contains device objects that are
> not used in certain configurations of the system.  If they start off
> in the D0 power state configuration, they will stay in it until the
> system reboots, because of the lack of any mechanism possibly causing
> their configuration to change.  If that happens, they may prevent
> some power resources from being turned off or generally they may
> prevent the platform from getting into the deepest low-power states
> thus causing some energy to be wasted.
> 
> Address this issue by changing the configuration of unused ACPI
> device objects to the D3cold power state one after carrying out
> the ACPI-based enumeration of devices.
> 
> BugLink: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.kernel.org%2Fshow_bug.cgi%3Fid%3D214091&amp;data=04%7C01%7Cmario.limonciello%40amd.com%7C6e87f8a1465c424d721c08d98b303310%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637693861879301335%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=zCknM%2FyK8unP%2BI0fstH8OoB5UUVfwOKqeERZOVu9lQY%3D&amp;reserved=0
> Link: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Flinux-acpi%2F20211007205126.11769-1-mario.limonciello%40amd.com%2F&amp;data=04%7C01%7Cmario.limonciello%40amd.com%7C6e87f8a1465c424d721c08d98b303310%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637693861879301335%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=4yE1GvAineqku3yjQyfXv6nifmx1uqoEAAbf57eDiFY%3D&amp;reserved=0
> Reported-by: Mario Limonciello <mario.limonciello@....com>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@...el.com>

Tested-by: Mario Limonciello <mario.limonciello@....com>

> ---
> 
> Mario,
> 
> I've changed acpi_dev_turn_off_if_unused() to check pnp.type.hardware_id
> instead of pnp.type.bus_address, because on some systems there are objects
> with both _ADR and _HID (which is against the spec) and the former is bogus.
> 
> Please test this one, even though for PCI devices there should be no difference.
> 

Yes, Thanks.  I had a try and this new version is also working correctly 
on the system this was found.

> Thanks!
> 
> ---
>   drivers/acpi/glue.c     |   25 +++++++++++++++++++++++++
>   drivers/acpi/internal.h |    1 +
>   drivers/acpi/scan.c     |    6 ++++++
>   3 files changed, 32 insertions(+)
> 
> Index: linux-pm/drivers/acpi/scan.c
> ===================================================================
> --- linux-pm.orig/drivers/acpi/scan.c
> +++ linux-pm/drivers/acpi/scan.c
> @@ -2559,6 +2559,12 @@ int __init acpi_scan_init(void)
>   		}
>   	}
>   
> +	/*
> +	 * Make sure that power management resources are not blocked by ACPI
> +	 * device objects with no users.
> +	 */
> +	bus_for_each_dev(&acpi_bus_type, NULL, NULL, acpi_dev_turn_off_if_unused);
> +
>   	acpi_turn_off_unused_power_resources();
>   
>   	acpi_scan_initialized = true;
> Index: linux-pm/drivers/acpi/glue.c
> ===================================================================
> --- linux-pm.orig/drivers/acpi/glue.c
> +++ linux-pm/drivers/acpi/glue.c
> @@ -350,3 +350,28 @@ void acpi_device_notify_remove(struct de
>   
>   	acpi_unbind_one(dev);
>   }
> +
> +int acpi_dev_turn_off_if_unused(struct device *dev, void *not_used)
> +{
> +	struct acpi_device *adev = to_acpi_device(dev);
> +
> +	/*
> +	 * Skip device objects with device IDs, because they may be in use even
> +	 * if they are not companions of any physical device objects.
> +	 */
> +	if (adev->pnp.type.hardware_id)
> +		return 0;
> +
> +	mutex_lock(&adev->physical_node_lock);
> +
> +	/*
> +	 * Device objects without device IDs are not in use if they have no
> +	 * corresponding physical device objects.
> +	 */
> +	if (list_empty(&adev->physical_node_list))
> +		acpi_device_set_power(adev, ACPI_STATE_D3_COLD);
> +
> +	mutex_unlock(&adev->physical_node_lock);
> +
> +	return 0;
> +}
> Index: linux-pm/drivers/acpi/internal.h
> ===================================================================
> --- linux-pm.orig/drivers/acpi/internal.h
> +++ linux-pm/drivers/acpi/internal.h
> @@ -117,6 +117,7 @@ bool acpi_device_is_battery(struct acpi_
>   bool acpi_device_is_first_physical_node(struct acpi_device *adev,
>   					const struct device *dev);
>   int acpi_bus_register_early_device(int type);
> +int acpi_dev_turn_off_if_unused(struct device *dev, void *not_used);
>   
>   /* --------------------------------------------------------------------------
>                        Device Matching and Notification
> 
> 
>