[2/2] x86/apci: Adjust command line parsing for "acpi_sleep"

Commit Message

Andrew Cooper Sept. 2, 2019, 12:14 p.m. UTC

Perform parsing in a custom_param, rather than stashing the content in a
string and parsing in an initcall.  Adjust the parsing to conform to current
standards.

No practical change.

Signed-off-by: Andrew Cooper <[e-mail]>
---
CC: Jan Beulich <[e-mail]>
CC: Wei Liu <[e-mail]>
CC: Roger Pau Monné <[e-mail]>

The reason that flags is pulled into a local variable is that the codegen for
acpi_video_flags is attrocious, 260 bytes!, and doubles up when used twice.
---
 xen/arch/x86/acpi/power.c | 47 ++++++++++++++++++++++++++---------------------
 1 file changed, 26 insertions(+), 21 deletions(-)

Comments

Jan Beulich Sept. 2, 2019, 1:56 p.m. UTC | #1

On 02.09.2019 14:14, Andrew Cooper wrote:
> --- a/xen/arch/x86/acpi/power.c
> +++ b/xen/arch/x86/acpi/power.c
> @@ -33,8 +33,32 @@
>  
>  uint32_t system_reset_counter = 1;
>  
> -static char __initdata opt_acpi_sleep[20];
> -string_param("acpi_sleep", opt_acpi_sleep);
> +static int __init parse_acpi_sleep(const char *s)
> +{
> +    const char *ss;
> +    unsigned int flag = 0;
> +    int rc = 0;
> +
> +    do {
> +        ss = strchr(s, ',');
> +        if ( !ss )
> +            ss = strchr(s, '\0');
> +
> +        if ( !cmdline_strcmp(s, "s3_bios") )
> +            flag |= 1;
> +        else if ( !cmdline_strcmp(s, "s3_mode") )
> +            flag |= 2;

You didn't fancy using parse_boolean() here (to also allow
specifying the negative forms), did you?

> +        else
> +            rc = -EINVAL;
> +
> +        s = ss + 1;
> +    } while ( *ss );
> +
> +    acpi_video_flags = flag;

This wants to be |= , such that "acpi_sleep=s3_bios acpi_sleep=s3_mode"
has the same effect as "acpi_sleep=s3_mode,s3_bios". With at least this
adjustment
Reviewed-by: Jan Beulich <[e-mail]>

Jan

Patch

diff --git a/xen/arch/x86/acpi/power.c b/xen/arch/x86/acpi/power.c
index 6ae9e29229..414bda205d 100644
--- a/xen/arch/x86/acpi/power.c
+++ b/xen/arch/x86/acpi/power.c
@@ -33,8 +33,32 @@ 
 
 uint32_t system_reset_counter = 1;
 
-static char __initdata opt_acpi_sleep[20];
-string_param("acpi_sleep", opt_acpi_sleep);
+static int __init parse_acpi_sleep(const char *s)
+{
+    const char *ss;
+    unsigned int flag = 0;
+    int rc = 0;
+
+    do {
+        ss = strchr(s, ',');
+        if ( !ss )
+            ss = strchr(s, '\0');
+
+        if ( !cmdline_strcmp(s, "s3_bios") )
+            flag |= 1;
+        else if ( !cmdline_strcmp(s, "s3_mode") )
+            flag |= 2;
+        else
+            rc = -EINVAL;
+
+        s = ss + 1;
+    } while ( *ss );
+
+    acpi_video_flags = flag;
+
+    return 0;
+}
+custom_param("acpi_sleep", parse_acpi_sleep);
 
 static DEFINE_SPINLOCK(pm_lock);
 
@@ -456,22 +480,3 @@  acpi_status acpi_enter_sleep_state(u8 sleep_state)
 
     return_ACPI_STATUS(AE_OK);
 }
-
-static int __init acpi_sleep_init(void)
-{
-    char *p = opt_acpi_sleep;
-
-    while ( (p != NULL) && (*p != '\0') )
-    {
-        if ( !strncmp(p, "s3_bios", 7) )
-            acpi_video_flags |= 1;
-        if ( !strncmp(p, "s3_mode", 7) )
-            acpi_video_flags |= 2;
-        p = strchr(p, ',');
-        if ( p != NULL )
-            p += strspn(p, ", \t");
-    }
-
-    return 0;
-}
-__initcall(acpi_sleep_init);