[PATCH 2/3] SMBIOS table passthrough support

[PATCH 2/3] SMBIOS table passthrough support

[Ross Philipson]

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

Utility routines in hvmloader for SMBIOS table validation.

Signed-off-by: Ross Philipson <ross.philipson@citrix.com>


[-- Attachment #2: smbios-passthrough-02.patch --]
[-- Type: application/octet-stream, Size: 3362 bytes --]

diff -r 0900b1c905f1 tools/firmware/hvmloader/util.c
--- a/tools/firmware/hvmloader/util.c	Mon Feb 20 18:58:07 2012 +0000
+++ b/tools/firmware/hvmloader/util.c	Mon Feb 20 20:37:12 2012 -0500
@@ -709,37 +709,38 @@
     crash();
 }
 
-static void validate_hvm_info(struct hvm_info_table *t)
+static int validate_hvm_info_table(uint8_t *table, uint32_t table_length,
+    const char *table_sig, const char *sig)
 {
-    uint8_t *ptr = (uint8_t *)t;
     uint8_t sum = 0;
     int i;
 
-    if ( strncmp(t->signature, "HVM INFO", 8) )
+    if (table_length == 0)
+        printf("Empty HVM info table: %.*s\n", 8, sig);
+
+    /* NOTE made this a common routine to validate all HVM tables. Removed 
+       BUG() on failure in here - let the caller decide if a checksum failure
+       warrants that */
+
+    /* strncmp(t->signature, signature, 8) */
+    for ( i = 0; i < 8; i++ )
     {
-        printf("Bad hvm info signature\n");
-        BUG();
+        if ( table_sig[i] != sig[i] )
+        {
+            printf("Bad HVM info signature for: %.*s\n", 8, sig);
+            return 0;
+        }
     }
 
-    if ( t->length < sizeof(struct hvm_info_table) )
-    {
-        printf("Bad hvm info length\n");
-        BUG();
-    }
+    for ( i = 0; i < table_length; i++ )
+        sum += table[i];
 
-    for ( i = 0; i < t->length; i++ )
-        sum += ptr[i];
-
-    if ( sum != 0 )
-    {
-        printf("Bad hvm info checksum\n");
-        BUG();
-    }
+    return (sum == 0);
 }
 
 struct hvm_info_table *get_hvm_info_table(void)
 {
-    static struct hvm_info_table *table;
+    static struct hvm_info_table *table = NULL;
     struct hvm_info_table *t;
 
     if ( table != NULL )
@@ -747,13 +748,44 @@
 
     t = (struct hvm_info_table *)HVM_INFO_PADDR;
 
-    validate_hvm_info(t);
+    if ( !validate_hvm_info_table((uint8_t*)t, t->length, t->signature, "HVM INFO") )
+    {
+        printf("Bad HVM info table\n");
+        BUG();
+    }
 
     table = t;
 
     return table;
 }
 
+struct hvm_smbios_info_table *get_hvm_smbios_info_table(void)
+{
+    static struct hvm_smbios_info_table *table = NULL;
+    static int validated = 0;
+    struct hvm_smbios_info_table *t;
+
+    if ( validated )
+        return table;
+
+    t = (struct hvm_smbios_info_table *)HVM_SMBIOS_INFO_PADDR;
+
+    if ( (t->length == 0) && (t->count == 0) )
+    {
+        printf("Empty HVM SMBIOS info table\n");
+        validated = 1;
+        return table;
+    }
+
+    if ( validate_hvm_info_table((uint8_t*)t, t->length, t->signature, "SMBIOSPT") )
+        table = t;
+    else
+        printf("Bad or missing HVM SMBIOS info table\n");
+    validated = 1;
+
+    return table;
+}
+
 struct shared_info *get_shared_info(void) 
 {
     static struct shared_info *shared_info = NULL;
diff -r 0900b1c905f1 tools/firmware/hvmloader/util.h
--- a/tools/firmware/hvmloader/util.h	Mon Feb 20 18:58:07 2012 +0000
+++ b/tools/firmware/hvmloader/util.h	Mon Feb 20 20:37:12 2012 -0500
@@ -145,6 +145,8 @@
 /* HVM-builder info. */
 struct hvm_info_table *get_hvm_info_table(void) __attribute__ ((const));
 #define hvm_info (get_hvm_info_table())
+struct hvm_smbios_info_table *get_hvm_smbios_info_table(void) __attribute__ ((const));
+#define hvm_smbios_info (get_hvm_smbios_info_table())
 
 /* String and memory functions */
 int strcmp(const char *cs, const char *ct);

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

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

Re: [PATCH 2/3] SMBIOS table passthrough support

[Ian Campbell]

> -static void validate_hvm_info(struct hvm_info_table *t)
> +static int validate_hvm_info_table(uint8_t *table, uint32_t
> table_length,
> +    const char *table_sig, const char *sig)
>  {
> -    uint8_t *ptr = (uint8_t *)t;
>      uint8_t sum = 0;
>      int i;
>  
> -    if ( strncmp(t->signature, "HVM INFO", 8) )
> +    if (table_length == 0)
> +        printf("Empty HVM info table: %.*s\n", 8, sig);
> +
> +    /* NOTE made this a common routine to validate all HVM tables.
> Removed 
> +       BUG() on failure in here - let the caller decide if a checksum
> failure
> +       warrants that */

I think comments about what code looked like historically, what changed
and why belong in the commit message, not inline in the code.

Re: [PATCH 2/3] SMBIOS table passthrough support

[Ross Philipson]

> -----Original Message-----
> From: Ian Campbell [mailto:Ian.Campbell@citrix.com]
> Sent: Tuesday, February 21, 2012 3:48 AM
> To: Ross Philipson
> Cc: xen-devel@lists.xensource.com
> Subject: Re: [Xen-devel] [PATCH 2/3] SMBIOS table passthrough support
> 
> 
> > -static void validate_hvm_info(struct hvm_info_table *t)
> > +static int validate_hvm_info_table(uint8_t *table, uint32_t
> > table_length,
> > +    const char *table_sig, const char *sig)
> >  {
> > -    uint8_t *ptr = (uint8_t *)t;
> >      uint8_t sum = 0;
> >      int i;
> >
> > -    if ( strncmp(t->signature, "HVM INFO", 8) )
> > +    if (table_length == 0)
> > +        printf("Empty HVM info table: %.*s\n", 8, sig);
> > +
> > +    /* NOTE made this a common routine to validate all HVM tables.
> > Removed
> > +       BUG() on failure in here - let the caller decide if a checksum
> > failure
> > +       warrants that */
> 
> I think comments about what code looked like historically, what changed
> and why belong in the commit message, not inline in the code.
> 

Wilco

Ross