[PATCH] ACPI:remove panic in case hardware has changed after S4

[PATCH] ACPI:remove panic in case hardware has changed after S4

[oliver]

From: Oliver Neukum <oneukum@suse.de>

Some BIOSes change hardware based on the state of
a laptop's lid. If the lid is closed, the touchpad is
disabled and the checksum changes.

Signed-off-by: Oliver Neukum <oneukum@suse.de>
---
 drivers/acpi/sleep.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c
index 9c1a435..14744e5 100644
--- a/drivers/acpi/sleep.c
+++ b/drivers/acpi/sleep.c
@@ -664,11 +664,9 @@ static void acpi_hibernation_leave(void)
 	/* Reprogram control registers */
 	acpi_leave_sleep_state_prep(ACPI_STATE_S4);
 	/* Check the hardware signature */
-	if (facs && s4_hardware_signature != facs->hardware_signature) {
-		printk(KERN_EMERG "ACPI: Hardware changed while hibernated, "
-			"cannot resume!\n");
-		panic("ACPI S4 hardware signature mismatch");
-	}
+	if (facs && s4_hardware_signature != facs->hardware_signature)
+		printk(KERN_CRIT "ACPI: Hardware changed while hibernated, "
+			"success doubtful!\n");
 	/* Restore the NVS memory area */
 	suspend_nvs_restore();
 	/* Allow EC transactions to happen. */
-- 
1.8.1.4

Re: [PATCH] ACPI:remove panic in case hardware has changed after S4

[joeyli]

於 四，2013-07-11 於 14:14 +0200，oliver@neukum.org 提到：
> From: Oliver Neukum <oneukum@suse.de>
> 
> Some BIOSes change hardware based on the state of
> a laptop's lid. If the lid is closed, the touchpad is
> disabled and the checksum changes.
> 
> Signed-off-by: Oliver Neukum <oneukum@suse.de>
> ---
>  drivers/acpi/sleep.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c
> index 9c1a435..14744e5 100644
> --- a/drivers/acpi/sleep.c
> +++ b/drivers/acpi/sleep.c
> @@ -664,11 +664,9 @@ static void acpi_hibernation_leave(void)
>  	/* Reprogram control registers */
>  	acpi_leave_sleep_state_prep(ACPI_STATE_S4);
>  	/* Check the hardware signature */
> -	if (facs && s4_hardware_signature != facs->hardware_signature) {
> -		printk(KERN_EMERG "ACPI: Hardware changed while hibernated, "
> -			"cannot resume!\n");
> -		panic("ACPI S4 hardware signature mismatch");
> -	}
> +	if (facs && s4_hardware_signature != facs->hardware_signature)
> +		printk(KERN_CRIT "ACPI: Hardware changed while hibernated, "
> +			"success doubtful!\n");

This panic is also reflect to the memory size changed by user when
machine in S4 state, BIOS should update hardware_signature to raise
memory changed.

Does it a good idea don't panic here if memory size changed?

Should we need try to detect the change of memory or touchpad for know
should we panic or not?

>  	/* Restore the NVS memory area */
>  	suspend_nvs_restore();
>  	/* Allow EC transactions to happen. */

On the other hand, I saw another BIOS changed hardware_signature because
the VGA detect output resolution changed when S4 resume. It's a problem
for sometimes BIOS widely used hardware_signature on non-critical
hardware change.


Thanks a lot!
Joey Lee

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ACPI:remove panic in case hardware has changed after S4

[Thomas Renninger]

On Friday, July 12, 2013 04:50:22 AM joeyli wrote:
> 於 四，2013-07-11 於 14:14 +0200，oliver@neukum.org 提到：
...
 
> On the other hand, I saw another BIOS changed hardware_signature because
> the VGA detect output resolution changed when S4 resume. It's a problem
> for sometimes BIOS widely used hardware_signature on non-critical
> hardware change.

I wonder how Windows is handling that.
The spec is rather clear about aborting the resuming:

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ACPI:remove panic in case hardware has changed after S4

[Thomas Renninger]

On Friday, July 12, 2013 04:50:22 AM joeyli wrote:
> 於 四，2013-07-11 於 14:14 +0200，oliver@neukum.org 提到：
...
> On the other hand, I saw another BIOS changed hardware_signature because
> the VGA detect output resolution changed when S4 resume. It's a problem
> for sometimes BIOS widely used hardware_signature on non-critical
> hardware change.

I wonder how Windows is handling that.
The spec is rather clear and OS should abort the resuming:
------
5.2.10 Firmware ACPI Control Structure (FACS)
...
If the values are not the same, OSPM assumes
that the saved non-volatile image is from a different hardware
configuration and cannot be restored.
------
I would say either:
   - someone checks Windows behavior and if they do it wrong
     and still resume, resuming unconditionally is an option.
     But they might fix this at some point of time and force vendors
     to do things right, then we would still try to resume even we
     must not.

  - a boot parameter for the poor souls with such BIOS defects
    so that they can still resume


My 2 cents,

           Thomas
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ACPI:remove panic in case hardware has changed after S4

[joeyli]

於 五，2013-07-12 於 09:37 +0200，Thomas Renninger 提到：
> On Friday, July 12, 2013 04:50:22 AM joeyli wrote:
> > 於 四，2013-07-11 於 14:14 +0200，oliver@neukum.org 提到：
> ...
> > On the other hand, I saw another BIOS changed hardware_signature because
> > the VGA detect output resolution changed when S4 resume. It's a problem
> > for sometimes BIOS widely used hardware_signature on non-critical
> > hardware change.
> 
> I wonder how Windows is handling that.
> The spec is rather clear and OS should abort the resuming:
> ------
> 5.2.10 Firmware ACPI Control Structure (FACS)
> ...
> If the values are not the same, OSPM assumes
> that the saved non-volatile image is from a different hardware
> configuration and cannot be restored.

Thanks for your kick.

Per information from OEM, Windows 8 didn't really block S4 resume when
hardware_signature not match, I think as Oliver's patch.

> ------
> I would say either:
>    - someone checks Windows behavior and if they do it wrong
>      and still resume, resuming unconditionally is an option.
>      But they might fix this at some point of time and force vendors
>      to do things right, then we would still try to resume even we
>      must not.
> 
>   - a boot parameter for the poor souls with such BIOS defects
>     so that they can still resume
> 
> 
> My 2 cents,
> 
>            Thomas
> --

I agreed your point, we should find a machine make sure the
hardware_signature changed by BIOS then install Windows 8 to verify the
behavior.


Thanks a lot!
Joey Lee

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ACPI:remove panic in case hardware has changed after S4

[Oliver Neukum]

On Friday 12 July 2013 16:45:15 joeyli wrote:

> Per information from OEM, Windows 8 didn't really block S4 resume when
> hardware_signature not match, I think as Oliver's patch.

Is that confidential information?
 
If it is indeed true, introducing a blacklist makes no sense. And I
see no alternative to my patch. Should I resubmit with an improved
comment?

	Regards
		Oliver

Re: [PATCH] ACPI:remove panic in case hardware has changed after S4

[Rafael J. Wysocki]

On Monday, July 15, 2013 01:29:47 PM Oliver Neukum wrote:
> On Friday 12 July 2013 16:45:15 joeyli wrote:
> 
> > Per information from OEM, Windows 8 didn't really block S4 resume when
> > hardware_signature not match, I think as Oliver's patch.
> 
> Is that confidential information?
>  
> If it is indeed true, introducing a blacklist makes no sense. And I
> see no alternative to my patch. Should I resubmit with an improved
> comment?

Yes, it would be good to update the changelog with this info.

Thanks,
Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH] ACPI:remove panic in case hardware has changed after S4

[Oliver Neukum]

On Monday 15 July 2013 13:40:09 Rafael J. Wysocki wrote:
> On Monday, July 15, 2013 01:29:47 PM Oliver Neukum wrote:
> > On Friday 12 July 2013 16:45:15 joeyli wrote:
> > 
> > > Per information from OEM, Windows 8 didn't really block S4 resume when
> > > hardware_signature not match, I think as Oliver's patch.
> > 
> > Is that confidential information?
> >  
> > If it is indeed true, introducing a blacklist makes no sense. And I
> > see no alternative to my patch. Should I resubmit with an improved
> > comment?
> 
> Yes, it would be good to update the changelog with this info.

Done.

	Regards
		Oliver

Re: [PATCH] ACPI:remove panic in case hardware has changed after S4

[joeyli]

於 一，2013-07-15 於 13:29 +0200，Oliver Neukum 提到：
> On Friday 12 July 2013 16:45:15 joeyli wrote:
> 
> > Per information from OEM, Windows 8 didn't really block S4 resume when
> > hardware_signature not match, I think as Oliver's patch.
> 
> Is that confidential information?
>  
> If it is indeed true, introducing a blacklist makes no sense. And I
> see no alternative to my patch. Should I resubmit with an improved
> comment?
> 
> 	Regards
> 		Oliver
> 

It's not confidential but non-official from OEM.

I looking for a additional DRAM module to test this behavior on my Acer
machine. 


Thank
Joey Lee


--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ACPI:remove panic in case hardware has changed after S4

[Rafael J. Wysocki]

On Friday, January 10, 2014 10:51:53 AM oliver@neukum.org wrote:
> From: Oliver Neukum <oneukum@suse.de>
> 
> Some BIOSes change hardware based on the state of
> a laptop's lid. If the lid is closed, the touchpad is
> disabled and the checksum changes. Windows 8 no longer
> aborts resume if the checksum has changed.
> 
> Signed-off-by: Oliver Neukum <oneukum@suse.de>

Queued up for 3.14, thanks!

> ---
>  drivers/acpi/sleep.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c
> index 9c1a435..14744e5 100644
> --- a/drivers/acpi/sleep.c
> +++ b/drivers/acpi/sleep.c
> @@ -664,11 +664,9 @@ static void acpi_hibernation_leave(void)
>  	/* Reprogram control registers */
>  	acpi_leave_sleep_state_prep(ACPI_STATE_S4);
>  	/* Check the hardware signature */
> -	if (facs && s4_hardware_signature != facs->hardware_signature) {
> -		printk(KERN_EMERG "ACPI: Hardware changed while hibernated, "
> -			"cannot resume!\n");
> -		panic("ACPI S4 hardware signature mismatch");
> -	}
> +	if (facs && s4_hardware_signature != facs->hardware_signature)
> +		printk(KERN_CRIT "ACPI: Hardware changed while hibernated, "
> +			"success doubtful!\n");
>  	/* Restore the NVS memory area */
>  	suspend_nvs_restore();
>  	/* Allow EC transactions to happen. */
> 

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

[PATCH] ACPI:remove panic in case hardware has changed after S4

[oliver]

From: Oliver Neukum <oneukum@suse.de>

Some BIOSes change hardware based on the state of
a laptop's lid. If the lid is closed, the touchpad is
disabled and the checksum changes. Windows 8 no longer
aborts resume if the checksum has changed.

Signed-off-by: Oliver Neukum <oneukum@suse.de>
---
 drivers/acpi/sleep.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c
index 9c1a435..14744e5 100644
--- a/drivers/acpi/sleep.c
+++ b/drivers/acpi/sleep.c
@@ -664,11 +664,9 @@ static void acpi_hibernation_leave(void)
 	/* Reprogram control registers */
 	acpi_leave_sleep_state_prep(ACPI_STATE_S4);
 	/* Check the hardware signature */
-	if (facs && s4_hardware_signature != facs->hardware_signature) {
-		printk(KERN_EMERG "ACPI: Hardware changed while hibernated, "
-			"cannot resume!\n");
-		panic("ACPI S4 hardware signature mismatch");
-	}
+	if (facs && s4_hardware_signature != facs->hardware_signature)
+		printk(KERN_CRIT "ACPI: Hardware changed while hibernated, "
+			"success doubtful!\n");
 	/* Restore the NVS memory area */
 	suspend_nvs_restore();
 	/* Allow EC transactions to happen. */
-- 
1.8.4

Re: [PATCH] ACPI:remove panic in case hardware has changed after S4

[joeyli]

Hi all experts, 

於 四，2013-07-18 於 00:09 +0200，Rafael J. Wysocki 提到：
> On Wednesday, July 17, 2013 04:58:34 PM Thomas Renninger wrote:
> > On Monday, July 15, 2013 05:35:23 PM Oliver Neukum wrote:
> > > On Monday 15 July 2013 12:06:11 Henrique de Moraes Holschuh wrote:
> > > > On Mon, 15 Jul 2013, Thomas Renninger wrote:
> > > > > I would not make it too complicated.
> > > > > Sticking to the latest Windows version should be enough for this one.
> > > > > What bad should happen if we still try to resume and fail...
> > > > 
> > > > Hmm, why was that check added in the first place?
> > > 
> > > Safety.
> > 
> > I expect it is there, because it is stated rather clear in the specs.
> >  
> > > > If it is useless,
> > > > removing it for good is fine.  If it is _not_ useless, we should still do
> > > > the checking when not operating in windows-8 firmware mode.
> > > 
> > > Firmware will only be tested against Windows 8 pretty soon.
> > > We'd end up with a gigantic list of exceptions.
> > 
> > Joey found an interessting paper about Win8 behavior:
> > http://bizsupport2.austin.hp.com/bc/docs/support/SupportManual/c03654081/c03654081.pdf
> > 
> > It looks like Win8 behaves rather different compared to Linux and is providing
> > "Hybrid Boot" where the user has to end all his apps. They have no big problem 
> > when resume fails as the user will not lose any data or work he had done 
> > before.
> > 
> > Joey also found the rather hidden acpi_sleep=s4_nohwsig boot option
> > which helps as well.
> > So Linux could do:
> >    1) Invalidate suspend image and reboot  (disadvantage: message is gone)
> >    2) Panic (current behavior) -> above boot param helps
> >    3) Still try to resume with a "not so offending message"
> >    4) ?
> > 
> > 1. seem to be what Win8 is doing
> > 3. sounds best to me. It could be ugly if suspend fails and
> > it always worked before and all your work/data is gone. So trying
> > hard to resume sounds like a good idea
> 
> We can also blacklist systems that need acpi_sleep=s4_nohwsig due to idiotic
> hardware/BIOS design choices.
> 
> In any case, if we remove the panic, we should also drop s4_nohwsig and since
> we have that (which I forgot about, by bad), the whole issue isn't so urgent,
> so I'm dropping the patch for the time being.
> 
> Thanks,
> Rafael
> 
> 


About the testing on real machine to capture the Windows behavior of
hardware_signature... We confirm the behavior on fast boot and _REAL_ S4
are different on Windows.

First, on Windows 8, user could not see S4 option on default UI, they
need enable it in setup UI when user really want to use it. Windows 8's
default shutdown is fast boot but not real S5.

We found a notebook that has a buggy BIOS that changed the
hardware_signature in 3 times S4 cycles. With Linux kernel, normally we
can reproduced the FACS hardware signature fail at second time S4.

My friend, Max Lin help to install Windows 7 and Windows 8 on this buggy
machine to run S4 and fast boot testing, the result as following:

 + Linux v3.10 Kernel:
   Can reproduce error of hardware_signature changed in 3 times S4. 

 + Windows 8 _REAL_ S4:
   Run more then 10 S4 cycles, always resume like normally. Didn't see
any problem.

 + Windows 8 fast boot:
   Random happened blue screen as the attached picture (sorry for the
chinese message), the error is: PAGE_FAULT_IN_NONPAGED_AREA. Then
Windows 8 reboot and try to recover error.  We are not sure this error
is caused by hardware_signature changed, but it only reproduced on fast
boot but not S4.

 + Windows 7 _REAL_ S4:
   Run more then 10 S4 cycles, always resume like normally. Didn't see
any problem.


Base on the above testing result, our conclusion is: 

 + The _REAL_ S4 on Windows 7/8 ignored FACS hardware signature change.
It just result like normally, didn't lost any user data.

 + The fast boot resume on Windows 8, happen blue screen, but we are not
sure it reflect to FACS hardware signature change. Even that, Windows 8
didn't lost any user data because user already close all applications
before fast boot shutdown.


Due to Linux kernel is running REAL S4, so I suggest we can just ignore
the FACS hardware signature changed, means accept Oliver's patch. Just
let machine try to restore the S4 snapshot, if lucky, then user can get
their user data back.


Thanks a lot!
Joey Lee

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ACPI:remove panic in case hardware has changed after S4

[Rafael J. Wysocki]

On Wednesday, July 17, 2013 04:58:34 PM Thomas Renninger wrote:
> On Monday, July 15, 2013 05:35:23 PM Oliver Neukum wrote:
> > On Monday 15 July 2013 12:06:11 Henrique de Moraes Holschuh wrote:
> > > On Mon, 15 Jul 2013, Thomas Renninger wrote:
> > > > I would not make it too complicated.
> > > > Sticking to the latest Windows version should be enough for this one.
> > > > What bad should happen if we still try to resume and fail...
> > > 
> > > Hmm, why was that check added in the first place?
> > 
> > Safety.
> 
> I expect it is there, because it is stated rather clear in the specs.
>  
> > > If it is useless,
> > > removing it for good is fine.  If it is _not_ useless, we should still do
> > > the checking when not operating in windows-8 firmware mode.
> > 
> > Firmware will only be tested against Windows 8 pretty soon.
> > We'd end up with a gigantic list of exceptions.
> 
> Joey found an interessting paper about Win8 behavior:
> http://bizsupport2.austin.hp.com/bc/docs/support/SupportManual/c03654081/c03654081.pdf
> 
> It looks like Win8 behaves rather different compared to Linux and is providing
> "Hybrid Boot" where the user has to end all his apps. They have no big problem 
> when resume fails as the user will not lose any data or work he had done 
> before.
> 
> Joey also found the rather hidden acpi_sleep=s4_nohwsig boot option
> which helps as well.
> So Linux could do:
>    1) Invalidate suspend image and reboot  (disadvantage: message is gone)
>    2) Panic (current behavior) -> above boot param helps
>    3) Still try to resume with a "not so offending message"
>    4) ?
> 
> 1. seem to be what Win8 is doing
> 3. sounds best to me. It could be ugly if suspend fails and
> it always worked before and all your work/data is gone. So trying
> hard to resume sounds like a good idea

We can also blacklist systems that need acpi_sleep=s4_nohwsig due to idiotic
hardware/BIOS design choices.

In any case, if we remove the panic, we should also drop s4_nohwsig and since
we have that (which I forgot about, by bad), the whole issue isn't so urgent,
so I'm dropping the patch for the time being.

Thanks,
Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH] ACPI:remove panic in case hardware has changed after S4

[Thomas Renninger]

On Monday, July 15, 2013 05:35:23 PM Oliver Neukum wrote:
> On Monday 15 July 2013 12:06:11 Henrique de Moraes Holschuh wrote:
> > On Mon, 15 Jul 2013, Thomas Renninger wrote:
> > > I would not make it too complicated.
> > > Sticking to the latest Windows version should be enough for this one.
> > > What bad should happen if we still try to resume and fail...
> > 
> > Hmm, why was that check added in the first place?
> 
> Safety.

I expect it is there, because it is stated rather clear in the specs.
 
> > If it is useless,
> > removing it for good is fine.  If it is _not_ useless, we should still do
> > the checking when not operating in windows-8 firmware mode.
> 
> Firmware will only be tested against Windows 8 pretty soon.
> We'd end up with a gigantic list of exceptions.

Joey found an interessting paper about Win8 behavior:
http://bizsupport2.austin.hp.com/bc/docs/support/SupportManual/c03654081/c03654081.pdf

It looks like Win8 behaves rather different compared to Linux and is providing
"Hybrid Boot" where the user has to end all his apps. They have no big problem 
when resume fails as the user will not lose any data or work he had done 
before.

Joey also found the rather hidden acpi_sleep=s4_nohwsig boot option
which helps as well.
So Linux could do:
   1) Invalidate suspend image and reboot  (disadvantage: message is gone)
   2) Panic (current behavior) -> above boot param helps
   3) Still try to resume with a "not so offending message"
   4) ?

1. seem to be what Win8 is doing
3. sounds best to me. It could be ugly if suspend fails and
it always worked before and all your work/data is gone. So trying
hard to resume sounds like a good idea

I guess this still needs some discussion?

At least some facts came up already and people have an idea what
happens...

       Thomas

Re: [PATCH] ACPI:remove panic in case hardware has changed after S4

[Rafael J. Wysocki]

On Tuesday, July 16, 2013 03:32:01 PM joeyli wrote:
> 於 一，2013-07-15 於 13:43 +0200，oliver@neukum.org 提到：
> > From: Oliver Neukum <oneukum@suse.de>
> > 
> > Some BIOSes change hardware based on the state of
> > a laptop's lid. If the lid is closed, the touchpad is
> > disabled and the checksum changes. Windows 8 no longer
> > aborts resume if the checksum has changed.
> > 
> > Signed-off-by: Oliver Neukum <oneukum@suse.de>
> > ---
> >  drivers/acpi/sleep.c | 8 +++-----
> >  1 file changed, 3 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c
> > index 9c1a435..14744e5 100644
> > --- a/drivers/acpi/sleep.c
> > +++ b/drivers/acpi/sleep.c
> > @@ -664,11 +664,9 @@ static void acpi_hibernation_leave(void)
> >  	/* Reprogram control registers */
> >  	acpi_leave_sleep_state_prep(ACPI_STATE_S4);
> >  	/* Check the hardware signature */
> > -	if (facs && s4_hardware_signature != facs->hardware_signature) {
> > -		printk(KERN_EMERG "ACPI: Hardware changed while hibernated, "
> > -			"cannot resume!\n");
> > -		panic("ACPI S4 hardware signature mismatch");
> > -	}
> > +	if (facs && s4_hardware_signature != facs->hardware_signature)
> > +		printk(KERN_CRIT "ACPI: Hardware changed while hibernated, "
> > +			"success doubtful!\n");
> >  	/* Restore the NVS memory area */
> >  	suspend_nvs_restore();
> >  	/* Allow EC transactions to happen. */
> 
> This morning I tried to reproduce the hardware_signature change on my
> Acer notebook, I hope can monitor the change on Linux then use Windows 8
> to check the behavior.
> Unfortunately I can not reproduce the hardware_signature change by
> remove memory or PCI device(wifi module) with Linux kernel.
> 
> For PCI device, as ACPI spec's declare, it didn't causes
> hardware_signature change, I print out in acpi_hibernation_leave() to
> confirm it.
> 
> For memory size, before the S4 resume path run to
> acpi_hibernation_leave(), kernel check the physical memory pages number
> in snapshot_write_next() then -EPERM if physical pages change . So, it
> doesn't have chance to check the hardware_signature changed.
> 
> Base on the above testing, correct my originally thinking, the
> hardware_signature check could not detect the change of memory size
> because it's too late.
> 
> Sorry for I didn't find out the actual behavior of Windows 8 to confirm
> it ignore hardware_signature change. At least could not verify on my
> machine.

OK, so it looks like we still need somebody to confirm that indeed Windows 8
doesn't check the hardware signature during resume from hibernation, right?

It is kind of important, because the *reason* for doing that change is the
reported Windows 8 compatibility issue.

Thanks,
Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ACPI:remove panic in case hardware has changed after S4

[joeyli]

於 一，2013-07-15 於 13:43 +0200，oliver@neukum.org 提到：
> From: Oliver Neukum <oneukum@suse.de>
> 
> Some BIOSes change hardware based on the state of
> a laptop's lid. If the lid is closed, the touchpad is
> disabled and the checksum changes. Windows 8 no longer
> aborts resume if the checksum has changed.
> 
> Signed-off-by: Oliver Neukum <oneukum@suse.de>
> ---
>  drivers/acpi/sleep.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c
> index 9c1a435..14744e5 100644
> --- a/drivers/acpi/sleep.c
> +++ b/drivers/acpi/sleep.c
> @@ -664,11 +664,9 @@ static void acpi_hibernation_leave(void)
>  	/* Reprogram control registers */
>  	acpi_leave_sleep_state_prep(ACPI_STATE_S4);
>  	/* Check the hardware signature */
> -	if (facs && s4_hardware_signature != facs->hardware_signature) {
> -		printk(KERN_EMERG "ACPI: Hardware changed while hibernated, "
> -			"cannot resume!\n");
> -		panic("ACPI S4 hardware signature mismatch");
> -	}
> +	if (facs && s4_hardware_signature != facs->hardware_signature)
> +		printk(KERN_CRIT "ACPI: Hardware changed while hibernated, "
> +			"success doubtful!\n");
>  	/* Restore the NVS memory area */
>  	suspend_nvs_restore();
>  	/* Allow EC transactions to happen. */

This morning I tried to reproduce the hardware_signature change on my
Acer notebook, I hope can monitor the change on Linux then use Windows 8
to check the behavior.
Unfortunately I can not reproduce the hardware_signature change by
remove memory or PCI device(wifi module) with Linux kernel.

For PCI device, as ACPI spec's declare, it didn't causes
hardware_signature change, I print out in acpi_hibernation_leave() to
confirm it.

For memory size, before the S4 resume path run to
acpi_hibernation_leave(), kernel check the physical memory pages number
in snapshot_write_next() then -EPERM if physical pages change . So, it
doesn't have chance to check the hardware_signature changed.

Base on the above testing, correct my originally thinking, the
hardware_signature check could not detect the change of memory size
because it's too late.

Sorry for I didn't find out the actual behavior of Windows 8 to confirm
it ignore hardware_signature change. At least could not verify on my
machine.


Thanks a lot!
Joey Lee

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ACPI:remove panic in case hardware has changed after S4

[Thomas Renninger]

On Tuesday, July 16, 2013 01:56:49 AM Rafael J. Wysocki wrote:
> On Monday, July 15, 2013 01:34:27 PM Henrique de Moraes Holschuh wrote:
...
> > We still have 10 to 15 years worth of users using non-win8 firmware boxes
> > on x86/x86-64.
> 
> But what's the risk from removing the check exactly for those systems?

Yeah, I guess this is the point.

Is there a security or data corruption issue that might show up?
And even if.., if you exchange HW you should do a real reboot, otherwise
you have to blame yourself. The check is/was nice to at least give a hint
and remember the user where the failure might have come from.
But this does not work if Windows does not check for it.
Maybe Windows never checked for it, but it's not worth trying out
different versions, just for the sake of absolute Windows version
compatibility.

       Thomas

Re: [PATCH] ACPI:remove panic in case hardware has changed after S4

[Rafael J. Wysocki]

On Monday, July 15, 2013 01:34:27 PM Henrique de Moraes Holschuh wrote:
> On Mon, 15 Jul 2013, Oliver Neukum wrote:
> > On Monday 15 July 2013 12:06:11 Henrique de Moraes Holschuh wrote:
> > > On Mon, 15 Jul 2013, Thomas Renninger wrote:
> > > > I would not make it too complicated.
> > > > Sticking to the latest Windows version should be enough for this one.
> > > > What bad should happen if we still try to resume and fail...
> > > 
> > > Hmm, why was that check added in the first place?
> > 
> > Safety.
> > 
> > > If it is useless,
> > > removing it for good is fine.  If it is _not_ useless, we should still do
> > > the checking when not operating in windows-8 firmware mode.
> > 
> > Firmware will only be tested against Windows 8 pretty soon.
> > We'd end up with a gigantic list of exceptions.
> 
> No, we won't.  We can query the ACPI core for the OSI compatibility level
> requested by the firmware, and ignore the test only for win8.
> 
> We still have 10 to 15 years worth of users using non-win8 firmware boxes on
> x86/x86-64.

But what's the risk from removing the check exactly for those systems?

Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH] ACPI:remove panic in case hardware has changed after S4

[Henrique de Moraes Holschuh]

On Mon, 15 Jul 2013, Oliver Neukum wrote:
> On Monday 15 July 2013 12:06:11 Henrique de Moraes Holschuh wrote:
> > On Mon, 15 Jul 2013, Thomas Renninger wrote:
> > > I would not make it too complicated.
> > > Sticking to the latest Windows version should be enough for this one.
> > > What bad should happen if we still try to resume and fail...
> > 
> > Hmm, why was that check added in the first place?
> 
> Safety.
> 
> > If it is useless,
> > removing it for good is fine.  If it is _not_ useless, we should still do
> > the checking when not operating in windows-8 firmware mode.
> 
> Firmware will only be tested against Windows 8 pretty soon.
> We'd end up with a gigantic list of exceptions.

No, we won't.  We can query the ACPI core for the OSI compatibility level
requested by the firmware, and ignore the test only for win8.

We still have 10 to 15 years worth of users using non-win8 firmware boxes on
x86/x86-64.

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

Re: [PATCH] ACPI:remove panic in case hardware has changed after S4

[Oliver Neukum]

On Monday 15 July 2013 12:06:11 Henrique de Moraes Holschuh wrote:
> On Mon, 15 Jul 2013, Thomas Renninger wrote:

> > I would not make it too complicated.
> > Sticking to the latest Windows version should be enough for this one.
> > What bad should happen if we still try to resume and fail...
> 
> Hmm, why was that check added in the first place?

Safety.

> If it is useless,
> removing it for good is fine.  If it is _not_ useless, we should still do
> the checking when not operating in windows-8 firmware mode.

Firmware will only be tested against Windows 8 pretty soon.
We'd end up with a gigantic list of exceptions.

	Regards
		Oliver

Re: [PATCH] ACPI:remove panic in case hardware has changed after S4

[Henrique de Moraes Holschuh]

On Mon, 15 Jul 2013, Thomas Renninger wrote:
> On Monday, July 15, 2013 09:31:27 AM Henrique de Moraes Holschuh wrote:
> > On Mon, 15 Jul 2013, Thomas Renninger wrote:
> > > On Monday, July 15, 2013 01:43:57 PM oliver@neukum.org wrote:
> > > > Some BIOSes change hardware based on the state of
> > > > a laptop's lid. If the lid is closed, the touchpad is
> > > > disabled and the checksum changes. Windows 8 no longer
> > > > aborts resume if the checksum has changed.
> > > 
> > > I expect more machines will show up and ignore s4_hardware_signature
> > > if Windows does not check it and we should do the same.
> > 
> > Windows *8* does not check.   But what about older versions?  Maybe this
> > needs to be guarded by verifying the firmware requested windows-8 mode?
> 
> I would not make it too complicated.
> Sticking to the latest Windows version should be enough for this one.
> What bad should happen if we still try to resume and fail...

Hmm, why was that check added in the first place?  If it is useless,
removing it for good is fine.  If it is _not_ useless, we should still do
the checking when not operating in windows-8 firmware mode.

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

Re: [PATCH] ACPI:remove panic in case hardware has changed after S4

[Thomas Renninger]

On Monday, July 15, 2013 09:31:27 AM Henrique de Moraes Holschuh wrote:
> On Mon, 15 Jul 2013, Thomas Renninger wrote:
> > On Monday, July 15, 2013 01:43:57 PM oliver@neukum.org wrote:
> > > Some BIOSes change hardware based on the state of
> > > a laptop's lid. If the lid is closed, the touchpad is
> > > disabled and the checksum changes. Windows 8 no longer
> > > aborts resume if the checksum has changed.
> > 
> > I expect more machines will show up and ignore s4_hardware_signature
> > if Windows does not check it and we should do the same.
> 
> Windows *8* does not check.   But what about older versions?  Maybe this
> needs to be guarded by verifying the firmware requested windows-8 mode?

I would not make it too complicated.
Sticking to the latest Windows version should be enough for this one.
What bad should happen if we still try to resume and fail...

      Thomas

Re: [PATCH] ACPI:remove panic in case hardware has changed after S4

[Henrique de Moraes Holschuh]

On Mon, 15 Jul 2013, Thomas Renninger wrote:
> On Monday, July 15, 2013 01:43:57 PM oliver@neukum.org wrote:
> > Some BIOSes change hardware based on the state of
> > a laptop's lid. If the lid is closed, the touchpad is
> > disabled and the checksum changes. Windows 8 no longer
> > aborts resume if the checksum has changed.

> I expect more machines will show up and ignore s4_hardware_signature
> if Windows does not check it and we should do the same.

Windows *8* does not check.   But what about older versions?  Maybe this
needs to be guarded by verifying the firmware requested windows-8 mode?

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

Re: [PATCH] ACPI:remove panic in case hardware has changed after S4

[Rafael J. Wysocki]

On Monday, July 15, 2013 01:55:56 PM Thomas Renninger wrote:
> On Monday, July 15, 2013 01:43:57 PM oliver@neukum.org wrote:
> > From: Oliver Neukum <oneukum@suse.de>
> > 
> > Some BIOSes change hardware based on the state of
> > a laptop's lid. If the lid is closed, the touchpad is
> > disabled and the checksum changes. Windows 8 no longer
> > aborts resume if the checksum has changed.
> I expect more machines will show up and ignore s4_hardware_signature
> if Windows does not check it and we should do the same.
> 
> So no objections from my side.
> 
> Rafael: If you agree, could you queue this one up, please?

Yes, I'm going to do that, although for 3.11 only, not for -stable.

If you need it in -stable, please send a separate inclusion request.

Thanks,
Rafael


> > 
> > Signed-off-by: Oliver Neukum <oneukum@suse.de>
> > ---
> >  drivers/acpi/sleep.c | 8 +++-----
> >  1 file changed, 3 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c
> > index 9c1a435..14744e5 100644
> > --- a/drivers/acpi/sleep.c
> > +++ b/drivers/acpi/sleep.c
> > @@ -664,11 +664,9 @@ static void acpi_hibernation_leave(void)
> >  	/* Reprogram control registers */
> >  	acpi_leave_sleep_state_prep(ACPI_STATE_S4);
> >  	/* Check the hardware signature */
> > -	if (facs && s4_hardware_signature != facs->hardware_signature) {
> > -		printk(KERN_EMERG "ACPI: Hardware changed while hibernated, "
> > -			"cannot resume!\n");
> > -		panic("ACPI S4 hardware signature mismatch");
> > -	}
> > +	if (facs && s4_hardware_signature != facs->hardware_signature)
> > +		printk(KERN_CRIT "ACPI: Hardware changed while hibernated, "
> > +			"success doubtful!\n");
> >  	/* Restore the NVS memory area */
> >  	suspend_nvs_restore();
> >  	/* Allow EC transactions to happen. */
-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH] ACPI:remove panic in case hardware has changed after S4

[Thomas Renninger]

On Monday, July 15, 2013 01:43:57 PM oliver@neukum.org wrote:
> From: Oliver Neukum <oneukum@suse.de>
> 
> Some BIOSes change hardware based on the state of
> a laptop's lid. If the lid is closed, the touchpad is
> disabled and the checksum changes. Windows 8 no longer
> aborts resume if the checksum has changed.
I expect more machines will show up and ignore s4_hardware_signature
if Windows does not check it and we should do the same.

So no objections from my side.

Rafael: If you agree, could you queue this one up, please?

Thanks,

       Thomas

> 
> Signed-off-by: Oliver Neukum <oneukum@suse.de>
> ---
>  drivers/acpi/sleep.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c
> index 9c1a435..14744e5 100644
> --- a/drivers/acpi/sleep.c
> +++ b/drivers/acpi/sleep.c
> @@ -664,11 +664,9 @@ static void acpi_hibernation_leave(void)
>  	/* Reprogram control registers */
>  	acpi_leave_sleep_state_prep(ACPI_STATE_S4);
>  	/* Check the hardware signature */
> -	if (facs && s4_hardware_signature != facs->hardware_signature) {
> -		printk(KERN_EMERG "ACPI: Hardware changed while hibernated, "
> -			"cannot resume!\n");
> -		panic("ACPI S4 hardware signature mismatch");
> -	}
> +	if (facs && s4_hardware_signature != facs->hardware_signature)
> +		printk(KERN_CRIT "ACPI: Hardware changed while hibernated, "
> +			"success doubtful!\n");
>  	/* Restore the NVS memory area */
>  	suspend_nvs_restore();
>  	/* Allow EC transactions to happen. */

[PATCH] ACPI:remove panic in case hardware has changed after S4

[oliver]

From: Oliver Neukum <oneukum@suse.de>

Some BIOSes change hardware based on the state of
a laptop's lid. If the lid is closed, the touchpad is
disabled and the checksum changes. Windows 8 no longer
aborts resume if the checksum has changed.

Signed-off-by: Oliver Neukum <oneukum@suse.de>
---
 drivers/acpi/sleep.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c
index 9c1a435..14744e5 100644
--- a/drivers/acpi/sleep.c
+++ b/drivers/acpi/sleep.c
@@ -664,11 +664,9 @@ static void acpi_hibernation_leave(void)
 	/* Reprogram control registers */
 	acpi_leave_sleep_state_prep(ACPI_STATE_S4);
 	/* Check the hardware signature */
-	if (facs && s4_hardware_signature != facs->hardware_signature) {
-		printk(KERN_EMERG "ACPI: Hardware changed while hibernated, "
-			"cannot resume!\n");
-		panic("ACPI S4 hardware signature mismatch");
-	}
+	if (facs && s4_hardware_signature != facs->hardware_signature)
+		printk(KERN_CRIT "ACPI: Hardware changed while hibernated, "
+			"success doubtful!\n");
 	/* Restore the NVS memory area */
 	suspend_nvs_restore();
 	/* Allow EC transactions to happen. */
-- 
1.8.1.4