Re: [Xen-devel] [PATCH v2] hvmloader: support system enclosure asset tag (SMBIOS type 3)

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Mon, Aug 21, 2017 at 10:40:39AM +0100, VIVEK KUMAR CHAUBEY wrote:
> From: Vivek Kumar Chaubey <vivekkumar.chaubey@xxxxxxxxxx>
> 
> Allow setting System Enclosure Asset Tag for HVM guest. Guest OS can
> check and perform desired operation like support installation.
> 
> Signed-off-by: Vivek Kumar Chaubey <vivekkumar.chaubey@xxxxxxxxxx>
> ---
>  tools/firmware/hvmloader/smbios.c       | 20 ++++++++++++++------
>  xen/include/public/hvm/hvm_xs_strings.h |  1 +
>  2 files changed, 15 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/firmware/hvmloader/smbios.c 
> b/tools/firmware/hvmloader/smbios.c
> index 210c7b0..3ec41ad 100644
> --- a/tools/firmware/hvmloader/smbios.c
> +++ b/tools/firmware/hvmloader/smbios.c
> @@ -531,6 +531,7 @@ smbios_type_3_init(void *start)
>      const char *s;
>      void *pts;
>      uint32_t length;
> +    uint32_t counter = 0;
>  
>      pts = get_smbios_pt_struct(3, &length);
>      if ( (pts != NULL)&&(length > 0) )
> @@ -546,7 +547,7 @@ smbios_type_3_init(void *start)
>      p->header.length = sizeof(struct smbios_type_3);
>      p->header.handle = SMBIOS_HANDLE_TYPE3;
>  
> -    p->manufacturer_str = 1;
> +    p->manufacturer_str = ++counter;
>      p->type = 0x01; /* other */
>      p->version_str = 0;
>      p->serial_number_str = 0;
> @@ -559,16 +560,23 @@ smbios_type_3_init(void *start)
>      start += sizeof(struct smbios_type_3);
>      
>      s = xenstore_read(HVM_XS_ENCLOSURE_MANUFACTURER, "Xen");
> -    strcpy((char *)start, s);
> +    strcpy(start, s);

Ideally this should be in a separate patch, since it's unrelated to
the added code, or it should be mentioned in the commit log that you
are also including some style changes to existing code.

>      start += strlen(s) + 1;
>  
> -    /* No internal defaults for this if the value is not set */
> +    /* No internal defaults for following ones if the value is not set */
>      s = xenstore_read(HVM_XS_ENCLOSURE_SERIAL_NUMBER, NULL);
> -    if ( (s != NULL)&&(*s != '\0') )
> +    if ( (s != NULL) && (*s != '\0') )

Same here.

>      {
> -        strcpy((char *)start, s);
> +        strcpy(start, s);
> +        start += strlen(s) + 1;
> +        p->serial_number_str = ++counter;
> +    }
> +    s = xenstore_read(HVM_XS_ENCLOSURE_ASSET_TAG, NULL);
> +    if ( (s != NULL) && (*s != '\0') )
> +    {
> +        strcpy(start, s);
>          start += strlen(s) + 1;
> -        p->serial_number_str = 2;
> +        p->asset_tag_str = ++counter;
>      }
>  
>      *((uint8_t *)start) = 0;
> diff --git a/xen/include/public/hvm/hvm_xs_strings.h 
> b/xen/include/public/hvm/hvm_xs_strings.h
> index 146b0b0..fea1dd4 100644
> --- a/xen/include/public/hvm/hvm_xs_strings.h
> +++ b/xen/include/public/hvm/hvm_xs_strings.h
> @@ -71,6 +71,7 @@
>  #define HVM_XS_SYSTEM_SERIAL_NUMBER    "bios-strings/system-serial-number"
>  #define HVM_XS_ENCLOSURE_MANUFACTURER  "bios-strings/enclosure-manufacturer"
>  #define HVM_XS_ENCLOSURE_SERIAL_NUMBER "bios-strings/enclosure-serial-number"
> +#define HVM_XS_ENCLOSURE_ASSET_TAG     "bios-strings/enclosure-asset-tag"
>  #define HVM_XS_BATTERY_MANUFACTURER    "bios-strings/battery-manufacturer"
>  #define HVM_XS_BATTERY_DEVICE_NAME     "bios-strings/battery-device-name"

Would be nice to have this documented on the
docs/misc/xenstore-paths.markdown document, but there are already a
bunch of paths in hvm_xs_strings.h that are not documented. Hopefully
there's a comment in hvm_xs_strings.h.

Roger.

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