linux-kernel - RE: [PATCH 1/6] ACPI / PM: Change the way power transitions to D3cold are carried out

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: <1AE640813FDE7649BE1B193DEA596E88BDD5F8@SHSMSX101.ccr.corp.intel.com>
Date:	Sat, 5 Jan 2013 02:37:35 +0000
From:	"Zheng, Lv" <lv.zheng@...el.com>
To:	"Rafael J. Wysocki" <rjw@...k.pl>,
	ACPI Devel Maling List <linux-acpi@...r.kernel.org>
CC:	LKML <linux-kernel@...r.kernel.org>, Len Brown <lenb@...nel.org>,
	"Huang, Ying" <ying.huang@...el.com>,
	"Cheng, Anton" <anton.cheng@...el.com>,
	"Palanisamy, Kanda K" <kanda.k.palanisamy@...el.com>,
	"Gough, Robert" <robert.gough@...el.com>,
	"Weyhing, Linda" <linda.weyhing@...el.com>,
	"Kalowsky, Daniel" <daniel.kalowsky@...el.com>
Subject: RE: [PATCH 1/6] ACPI / PM: Change the way power transitions to
 D3cold are carried out

Thanks for your patch, it might be useful as_PR3 off method of I2C hosts and targets might be different from their _PR0 off method, ACPI BIOS may implement protection in the _PR3 off method in order not to break the transactions during the powering off process:
As I2C is wired-AND logic bus, if a device will be LOW on SDA or SCL during the powering off process, it will pull the bus from HIGH to LOW which may break the current transaction on the bus that targeted to another slave device.

I just wonder one more thing which is not related to the ACPI BIOS.
If busses like I2C have such "non-hotpluggable" nature, we need to cut power of single target device only when there are not any transactions visible in the same segment.

How could an equivalent solution be implemented in the Linux kernel for I2C busses?
It could be useful for those platforms without such firmware deployed to protect the OS.

It seems we may need to redesign acpi_device_set_power/acpi_device_power_state to meet the following requirements:
1. suspend/resume -> One function can be used to switch device power state from 0 to 3 or 3 to 0.
2. poweroff -> One function can be used to cut device power currently applied.
And we may need new interface in the power core as "poweroff" (maybe also poweron) or likewise for platform_suspend/hibernate_ops.

Then I2C in the kernel can also implement a solution putting all of the devices (the masters and all of their slaves) in one segment (where there is wired-AND logic) into D3 (suspend all non-hotpluggable devices in one segment), and "poweroff" one of them when there are not any transactions visible on the bus.

Thanks and best regards
-Lv

> -----Original Message-----
> From: Rafael J. Wysocki [mailto:rjw@...k.pl]
> Sent: Saturday, January 05, 2013 6:00 AM
> To: ACPI Devel Maling List
> Cc: LKML; Len Brown; Zheng, Lv; Huang, Ying
> Subject: [PATCH 1/6] ACPI / PM: Change the way power transitions to D3cold
> are carried out
> 
> From: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
> 
> During power transitions into D3cold from any shallower power states we are
> supposed to transition the device into D3hot and remove power from it
> afterward, but the current code in acpi_device_set_power() doesn't work this
> way.
> 
> At the same time, though, we need to be careful enough to preserve
> backwards compatibility for systems that don't distinguish between D3hot and
> D3cold (e.g. designed before ACPI 4).
> 
> Modify acpi_device_set_power() so that it works in accordance with the
> expectations in both cases.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
> ---
>  drivers/acpi/bus.c |   12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> Index: linux/drivers/acpi/bus.c
> ================================================================
> ===
> --- linux.orig/drivers/acpi/bus.c
> +++ linux/drivers/acpi/bus.c
> @@ -270,6 +270,7 @@ int acpi_device_set_power(struct acpi_de
>  	int result = 0;
>  	acpi_status status = AE_OK;
>  	char object_name[5] = { '_', 'P', 'S', '0' + state, '\0' };
> +	bool cut_power = false;
> 
>  	if (!device || (state < ACPI_STATE_D0) || (state > ACPI_STATE_D3_COLD))
>  		return -EINVAL;
> @@ -294,9 +295,13 @@ int acpi_device_set_power(struct acpi_de
>  		return -ENODEV;
>  	}
> 
> -	/* For D3cold we should execute _PS3, not _PS4. */
> -	if (state == ACPI_STATE_D3_COLD)
> +	/* For D3cold we should first transition into D3hot. */
> +	if (state == ACPI_STATE_D3_COLD
> +	    &&
> device->power.states[ACPI_STATE_D3_COLD].flags.os_accessible) {
> +		state = ACPI_STATE_D3_HOT;
>  		object_name[3] = '3';
> +		cut_power = true;
> +	}
> 
>  	/*
>  	 * Transition Power
> @@ -341,6 +346,9 @@ int acpi_device_set_power(struct acpi_de
>  		}
>  	}
> 
> +	if (cut_power)
> +		result = acpi_power_transition(device, ACPI_STATE_D3_COLD);
> +
>        end:
>  	if (result)
>  		printk(KERN_WARNING PREFIX