[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH][v2] xen: Pass the location of the ACPI RSDP to DOM0.



On 20/01/2014 22:08, Philip Wernersbach wrote:
xen: [v2] 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@xxxxxxxxx>

diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index b49256d..8c307c9 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);
         }

Please read the CODING_STYLE document in the SCM root.

In particular, there should be a newline here, and lines should (where possible) be wrapped at ~72-80 chars

+        if ( !strstr(dom0_cmdline, "acpi_rsdp=") )
+        {
+            char acpi_os_root_pointer_string[MAX_RSDP_ADDRESS_STRING];

char rp_str[sizeof(unsigned long)*2 + 1] would suffice with a much shorter name, and do without an extra define.

+            int acpi_os_root_pointer_string_size =

This return value from snprintf is bogus when used in the if() condition later; It is unconditionally within the provided bounds.  Furthermore, snprintf() is not going to fail for a single hex long into an appropriately sized buffer.

Furthermore, to correctly check for a failure of acpi_os_get_root_pointer(), you would be much better with something such as:

acpi_physical_address rsdp = acpi_os_get_root_pointer();

if ( rsdp )
{
  ... append
}
else
{
  ... error
}

snprintf(acpi_os_root_pointer_string,
+
MAX_RSDP_ADDRESS_STRING, "%08lX",
+
acpi_os_get_root_pointer());
+
+            if ( (acpi_os_root_pointer_string_size > 0) &&
(acpi_os_root_pointer_string_size <= MAX_RSDP_ADDRESS_STRING) )
+            {
+                safe_strcat(dom0_cmdline, " acpi_rsdp=0x");
+                safe_strcat(dom0_cmdline, acpi_os_root_pointer_string);
+            }
+            else
+            {
+                printk(XENLOG_WARNING "RSDP Address String Size Check
Failed!\n");
+                printk(XENLOG_WARNING "Not passing acpi_rsdp to Dom0!\n");
+                printk(XENLOG_WARNING "(Expected size 1-%d, got
%d)\n", MAX_RSDP_ADDRESS_STRING, acpi_os_root_pointer_string_size);

And talking of errors, this is overkill.  A simple:

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

should suffice.

~Andrew

+            }
+        }

         cmdline = dom0_cmdline;
     }
diff --git a/xen/include/public/xen.h b/xen/include/public/xen.h
index 8c5697e..a7c3905 100644
--- a/xen/include/public/xen.h
+++ b/xen/include/public/xen.h
@@ -750,6 +750,7 @@ struct start_info {
                                 /*  SIF_MOD_START_PFN set in flags).      */
     unsigned long mod_len;      /* Size (bytes) of pre-loaded module.     */
 #define MAX_GUEST_CMDLINE 1024
+#define MAX_RSDP_ADDRESS_STRING 21
     int8_t cmd_line[MAX_GUEST_CMDLINE];
     /* The pfn range here covers both page table and p->m table frames.   */
     unsigned long first_p2m_pfn;/* 1st pfn forming initial P->M table.    */


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

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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.