[PATCH][v3] Pass the location of the ACPI RSDP to DOM0.

[PATCH][v3] Pass the location of the ACPI RSDP to DOM0.

[Philip Wernersbach]

[-- Attachment #1: Type: text/plain, Size: 1564 bytes --]

xen: [v3] Pass the location of the ACPI RSDP to DOM0.

Some machines, such as recent IBM servers, only allow the OS to get the
ACPI RSDP from EFI. Since Xen nukes DOM0's ability to access EFI, DOM0
cannot get the RSDP on these machines, leading to all sorts of
functionality reductions.

Signed-off-by: Philip Wernersbach <philip.wernersbach@gmail.com>

---
Changed since v2:
    * Fix coding style
    * Get rid of extra define
    * Use correct typedef'd type for the ACPI RSDP pointer
    * Better error checking conditional
    * Simplify error message

diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index b49256d..fdeb9f2 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -1378,6 +1378,25 @@ void __init __start_xen(unsigned long mbi_p)
             safe_strcat(dom0_cmdline, " acpi=");
             safe_strcat(dom0_cmdline, acpi_param);
         }
+        if ( !strstr(dom0_cmdline, "acpi_rsdp=") )
+        {
+            acpi_physical_address rp = acpi_os_get_root_pointer();
+            char rp_str[sizeof(acpi_physical_address)*2 + 1];
+
+            if ( rp )
+            {
+                snprintf(rp_str, sizeof(acpi_physical_address)*2 + 1,
+                         "%08lX", rp);
+
+                safe_strcat(dom0_cmdline, " acpi_rsdp=0x");
+                safe_strcat(dom0_cmdline, rp_str);
+            }
+            else
+            {
+                printk(XENLOG_WARNING
+                       "Failed to get acpi_rsdp to pass to dom0\n");
+            }
+        }

         cmdline = dom0_cmdline;
     }

[-- Attachment #2: xen-master-pass-acpi-rsdp.patch --]
[-- Type: text/x-patch, Size: 1568 bytes --]

xen: [v3] Pass the location of the ACPI RSDP to DOM0.
 
Some machines, such as recent IBM servers, only allow the OS to get the
ACPI RSDP from EFI. Since Xen nukes DOM0's ability to access EFI, DOM0
cannot get the RSDP on these machines, leading to all sorts of
functionality reductions.
 
Signed-off-by: Philip Wernersbach <philip.wernersbach@gmail.com>

---
Changed since v2:
    * Fix coding style
    * Get rid of extra define
    * Use correct typedef'd type for the ACPI RSDP pointer
    * Better error checking conditional
    * Simplify error message

diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index b49256d..fdeb9f2 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -1378,6 +1378,25 @@ void __init __start_xen(unsigned long mbi_p)
             safe_strcat(dom0_cmdline, " acpi=");
             safe_strcat(dom0_cmdline, acpi_param);
         }
+        if ( !strstr(dom0_cmdline, "acpi_rsdp=") )
+        {
+            acpi_physical_address rp = acpi_os_get_root_pointer();
+            char rp_str[sizeof(acpi_physical_address)*2 + 1];
+
+            if ( rp )
+            {
+                snprintf(rp_str, sizeof(acpi_physical_address)*2 + 1,
+                         "%08lX", rp);
+
+                safe_strcat(dom0_cmdline, " acpi_rsdp=0x");
+                safe_strcat(dom0_cmdline, rp_str);
+            }
+            else
+            {
+                printk(XENLOG_WARNING 
+                       "Failed to get acpi_rsdp to pass to dom0\n");
+            }
+        }
 
         cmdline = dom0_cmdline;
     }

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

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

Re: [PATCH][v3] Pass the location of the ACPI RSDP to DOM0.

[Andrew Cooper]

On 21/01/2014 20:55, Philip Wernersbach wrote:
> xen: [v3] Pass the location of the ACPI RSDP to DOM0.
>
> Some machines, such as recent IBM servers, only allow the OS to get the
> ACPI RSDP from EFI. Since Xen nukes DOM0's ability to access EFI, DOM0
> cannot get the RSDP on these machines, leading to all sorts of
> functionality reductions.
>
> Signed-off-by: Philip Wernersbach <philip.wernersbach@gmail.com>
>
> ---
> Changed since v2:
>     * Fix coding style
>     * Get rid of extra define
>     * Use correct typedef'd type for the ACPI RSDP pointer
>     * Better error checking conditional
>     * Simplify error message
>
> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
> index b49256d..fdeb9f2 100644
> --- a/xen/arch/x86/setup.c
> +++ b/xen/arch/x86/setup.c
> @@ -1378,6 +1378,25 @@ void __init __start_xen(unsigned long mbi_p)
>              safe_strcat(dom0_cmdline, " acpi=");
>              safe_strcat(dom0_cmdline, acpi_param);
>          }

You have still got incorrect coding style at this point, as indicated in
my previous email.

> +        if ( !strstr(dom0_cmdline, "acpi_rsdp=") )
> +        {
> +            acpi_physical_address rp = acpi_os_get_root_pointer();
> +            char rp_str[sizeof(acpi_physical_address)*2 + 1];
> +
> +            if ( rp )
> +            {
> +                snprintf(rp_str, sizeof(acpi_physical_address)*2 + 1,

sizeof(rp_str)

> +                         "%08lX", rp);

Personally, I prefer lowercase hexidecimal numbers, as they are easier
to read, particularly when 64bit.  What happens if the root pointer is
above the 4GB boundary? I dont see any reason at all for the leading 0s.

> +
> +                safe_strcat(dom0_cmdline, " acpi_rsdp=0x");
> +                safe_strcat(dom0_cmdline, rp_str);
> +            }
> +            else
> +            {

And coding style here.

> +                printk(XENLOG_WARNING
> +                       "Failed to get acpi_rsdp to pass to dom0\n");
> +            }
> +        }

And finally, you have yet to address Jan's concerns about this patch. 
Being an x86 maintainer, it is him you will have to convince to accept
the patch, even after I have run out of basic review points to cover.

~Andrew

Re: [PATCH][v3] Pass the location of the ACPI RSDP to DOM0.

[Jan Beulich]

>>> On 21.01.14 at 21:55, Philip Wernersbach <philip.wernersbach@gmail.com> wrote:
> --- a/xen/arch/x86/setup.c
> +++ b/xen/arch/x86/setup.c
> @@ -1378,6 +1378,25 @@ void __init __start_xen(unsigned long mbi_p)
>              safe_strcat(dom0_cmdline, " acpi=");
>              safe_strcat(dom0_cmdline, acpi_param);
>          }
> +        if ( !strstr(dom0_cmdline, "acpi_rsdp=") )

Apart from all other reservations I have, you absolutely must not
do this unconditionally: This should be probably be limited to when
booting from EFI, and this should definitely be done only when
enabled by a command line option.

Jan

> +        {
> +            acpi_physical_address rp = acpi_os_get_root_pointer();
> +            char rp_str[sizeof(acpi_physical_address)*2 + 1];
> +
> +            if ( rp )
> +            {
> +                snprintf(rp_str, sizeof(acpi_physical_address)*2 + 1,
> +                         "%08lX", rp);
> +
> +                safe_strcat(dom0_cmdline, " acpi_rsdp=0x");
> +                safe_strcat(dom0_cmdline, rp_str);
> +            }
> +            else
> +            {
> +                printk(XENLOG_WARNING
> +                       "Failed to get acpi_rsdp to pass to dom0\n");
> +            }
> +        }
> 
>          cmdline = dom0_cmdline;
>      }