[Qemu-devel] Re: [PATCH] Correct SMBIOS handling of multiple tables

[Alex Williamson]

On Wed, 2009-05-27 at 17:42 -0400, Beth Kon wrote:
> The current code prevents multiple entries of the same type table, as 
> required,
> for example, by table type 4.

Hi Beth,

Are you trying to add multiple type 4 entries from a single type 4
binary passed in via -smbios file=<type4>?  ex:

qemu -smp 2 -smbios file=type4.bin

My intention was that once the user overrides an entry with a binary
image, they're responsible for providing all of the entries of that
type.  That means for a 2-way guest, if you want to override the type 4
entry with a binary, you need to specify it twice on the command line.
The code below would allow you to load a type 4 entry for each vCPU
while only specifying one on the command line, but what if you want 2
slightly different entries?  We have no way of associating a provided
entry to the one needed at table creating time, which is why we load
them all on the first pass and ignore requests to add the same type
again later.  Thanks,

Alex


> diff --git a/bios/rombios32.c b/bios/rombios32.c
> index f861f81..43aa065 100644
> --- a/bios/rombios32.c
> +++ b/bios/rombios32.c
> @@ -2146,6 +2146,10 @@ struct smbios_table {
>  #define SMBIOS_FIELD_ENTRY 0
>  #define SMBIOS_TABLE_ENTRY 1
>  
> +#ifdef BX_QEMU
> +    static uint64_t smbios_used_bitmap[4] = { 0 };
> +#endif
> +
>  static size_t
>  smbios_load_field(int type, size_t offset, void *addr)
>  {
> @@ -2496,14 +2500,9 @@ smbios_load_external(int type, char **p, unsigned 
> *nr_structs,
>                       unsigned *max_struct_size)
>  {
>  #ifdef BX_QEMU
> -    static uint64_t used_bitmap[4] = { 0 };
>      char *start = *p;
>      int i;
>  
> -    /* Check if we've already reported these tables */
> -    if (used_bitmap[(type >> 6) & 0x3] & (1ULL << (type & 0x3f)))
> -        return 1;
> -
>      /* Don't introduce spurious end markers */
>      if (type == 127)
>          return 0;
> @@ -2555,7 +2554,7 @@ smbios_load_external(int type, char **p, unsigned 
> *nr_structs,
>      }
>  
>      /* Mark that we've reported on this type */
> -    used_bitmap[(type >> 6) & 0x3] |= (1ULL << (type & 0x3f));
> +    smbios_used_bitmap[(type >> 6) & 0x3] |= (1ULL << (type & 0x3f));
>  
>      return (start != *p);
>  #else /* !BX_QEMU */
> @@ -2612,6 +2611,9 @@ void smbios_init(void)
>      add_struct(32, p);
>      /* Add any remaining provided entries before the end marker */
>      for (i = 0; i < 256; i++)
> +        /* Check if we've already reported these tables */
> +        if (smbios_used_bitmap[(i >> 6) & 0x3] & (1ULL << (i & 0x3f)))
> +            continue;
>          smbios_load_external(i, &p, &nr_structs, &max_struct_size);
>      add_struct(127, p);
>  
>