[PATCH] xen: add a 'acpi_fake_s3' boot command line parameter

[PATCH] xen: add a 'acpi_fake_s3' boot command line parameter

[Dario Faggioli]

which, if provided, prevents the function that actually
put the system in ACPI S3 suspended state to be called.
The system will, therefore, go down the suspend path,
until the second to last step, and then resume right away.

This is useful when testing and debugging S3 suspend/resume
issues.

Signed-off-by: Ben Guthro <ben@guthro.net>
Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
---
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
---
 docs/misc/xen-command-line.markdown |   11 +++++++++++
 xen/arch/x86/acpi/power.c           |    6 +++++-
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-command-line.markdown
index aa684c0..adbe9b5 100644
--- a/docs/misc/xen-command-line.markdown
+++ b/docs/misc/xen-command-line.markdown
@@ -110,6 +110,17 @@ domain 0 command line
 Specify which ACPI MADT table to parse for APIC information, if more
 than one is present.
 
+### acpi\_fake\_s3
+> `= <boolean>`
+
+> Default: `false`
+
+When ACPI S3 suspend is requested, perform all the operations necessary
+to achieve that, except the actual low level register writes that puts
+the system to sleep (hence the system resumes right away).
+
+This is useful for testing and debugging ACPI S3 suspend/resume issues.
+
 ### acpi\_pstate\_strict
 > `= <boolean>`
 
diff --git a/xen/arch/x86/acpi/power.c b/xen/arch/x86/acpi/power.c
index f41f0de..562d25a 100644
--- a/xen/arch/x86/acpi/power.c
+++ b/xen/arch/x86/acpi/power.c
@@ -33,6 +33,9 @@
 
 uint32_t system_reset_counter = 1;
 
+static bool_t __read_mostly fake_s3 = 0;
+boolean_param("acpi_fake_s3", fake_s3);
+
 static char __initdata opt_acpi_sleep[20];
 string_param("acpi_sleep", opt_acpi_sleep);
 
@@ -177,7 +180,8 @@ static int enter_state(u32 state)
     switch ( state )
     {
     case ACPI_STATE_S3:
-        do_suspend_lowlevel();
+        if ( likely(!fake_s3) )
+            do_suspend_lowlevel();
         system_reset_counter++;
         error = tboot_s3_resume();
         break;

Re: [PATCH] xen: add a 'acpi_fake_s3' boot command line parameter

[Jan Beulich]

>>> On 22.06.15 at 13:28, <dario.faggioli@citrix.com> wrote:
> --- a/xen/arch/x86/acpi/power.c
> +++ b/xen/arch/x86/acpi/power.c
> @@ -33,6 +33,9 @@
>  
>  uint32_t system_reset_counter = 1;
>  
> +static bool_t __read_mostly fake_s3 = 0;
> +boolean_param("acpi_fake_s3", fake_s3);
> +
>  static char __initdata opt_acpi_sleep[20];
>  string_param("acpi_sleep", opt_acpi_sleep);
>  
> @@ -177,7 +180,8 @@ static int enter_state(u32 state)
>      switch ( state )
>      {
>      case ACPI_STATE_S3:
> -        do_suspend_lowlevel();
> +        if ( likely(!fake_s3) )
> +            do_suspend_lowlevel();
>          system_reset_counter++;
>          error = tboot_s3_resume();
>          break;

The change is so simple that, considering it's for debugging purposes
only, I don't see why people needing to debug this code couldn't
apply it themselves when needed. Imo, if to be considered at all, it
should be made !NDEBUG dependent.

Jan

Re: [PATCH] xen: add a 'acpi_fake_s3' boot command line parameter

[Dario Faggioli]

[-- Attachment #1.1: Type: text/plain, Size: 2146 bytes --]

On Mon, 2015-06-22 at 12:48 +0100, Jan Beulich wrote:
> >>> On 22.06.15 at 13:28, <dario.faggioli@citrix.com> wrote:
> > --- a/xen/arch/x86/acpi/power.c
> > +++ b/xen/arch/x86/acpi/power.c
> > @@ -33,6 +33,9 @@
> >  
> >  uint32_t system_reset_counter = 1;
> >  
> > +static bool_t __read_mostly fake_s3 = 0;
> > +boolean_param("acpi_fake_s3", fake_s3);
> > +
> >  static char __initdata opt_acpi_sleep[20];
> >  string_param("acpi_sleep", opt_acpi_sleep);
> >  
> > @@ -177,7 +180,8 @@ static int enter_state(u32 state)
> >      switch ( state )
> >      {
> >      case ACPI_STATE_S3:
> > -        do_suspend_lowlevel();
> > +        if ( likely(!fake_s3) )
> > +            do_suspend_lowlevel();
> >          system_reset_counter++;
> >          error = tboot_s3_resume();
> >          break;
> 
> The change is so simple that, considering it's for debugging purposes
> only, I don't see why people needing to debug this code couldn't
> apply it themselves when needed.
>
When Ben sent this out some time ago for the first time, one purpose was
also using it for making it possible to test S3 in osstest, even on
hardware that does not support (properly) that feature.

I've also done some work in that direction (I did write a test case),
and the point is probably still valid. However, this patch alone can't
make the above happen by itself, as on such hardware, it's not possible
to echo 'mem' in /sys/power/state, and hence trigger the (fake) suspend
(or at least that is my experience).
So this patch would still be a step in the right direction , but
together with something that allows us to trigger the suspensions from
Dom0, bypassing Linux's checks, which is something I haven't
investigated.

>  Imo, if to be considered at all, it
> should be made !NDEBUG dependent.
> 
Ok, I'll make that so when/if respinning.

Dario

-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel