Re: [PATCH] hw/smbios: Add table 4 parameter, "processor-id"

[Igor Mammedov]

On Thu,  6 Jan 2022 14:33:16 -0800
Patrick Venture <venture@google.com> wrote:

> This parameter is to be used in the processor_id lower 32-bit entry in

I'd call it processor_id_lo throughout the patch
or if you prefer to keep current name, then make it support full QWORD
as spec says.

> the type 4 table.  The upper 32-bits represent the features for the CPU.
> This patch leaves those as 0 when the lower 32-bits are set.

why 0 it out instead of using smbios_cpuid_features value?


> This parameter is set as optional and if left will use the values from
> the CPU model.
> 
> This enables hiding the host information from the guest and allowing AMD
> VMs to run pretending to be Intel for some userspace software concerns.

> Reviewed-by: Peter Foley <pefoley@google.com>
> Reviewed-by: Titus Rwantare <titusr@google.com>
> Signed-off-by: Patrick Venture <venture@google.com>
> ---
>  hw/smbios/smbios.c | 19 ++++++++++++++++---
>  qemu-options.hx    |  1 +
>  2 files changed, 17 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
> index 7397e56737..0553ee0b17 100644
> --- a/hw/smbios/smbios.c
> +++ b/hw/smbios/smbios.c
> @@ -104,9 +104,11 @@ static struct {
>      const char *sock_pfx, *manufacturer, *version, *serial, *asset, *part;
>      uint64_t max_speed;
>      uint64_t current_speed;
> +    uint32_t processor_id;



>  } type4 = {
>      .max_speed = DEFAULT_CPU_SPEED,
> -    .current_speed = DEFAULT_CPU_SPEED
> +    .current_speed = DEFAULT_CPU_SPEED,
> +    .processor_id = 0,
>  };
>  
>  static struct {
> @@ -327,6 +329,10 @@ static const QemuOptDesc qemu_smbios_type4_opts[] = {
>          .name = "part",
>          .type = QEMU_OPT_STRING,
>          .help = "part number",
> +    }, {
> +        .name = "processor-id",
> +        .type = QEMU_OPT_NUMBER,
> +        .help = "processor id",
>      },
>      { /* end of list */ }
>  };
> @@ -669,8 +675,13 @@ static void smbios_build_type_4_table(MachineState *ms, 
> unsigned instance)
>      t->processor_type = 0x03; /* CPU */
>      t->processor_family = 0x01; /* Other */
>      SMBIOS_TABLE_SET_STR(4, processor_manufacturer_str, type4.manufacturer);
> -    t->processor_id[0] = cpu_to_le32(smbios_cpuid_version);
> -    t->processor_id[1] = cpu_to_le32(smbios_cpuid_features);
> +    if (type4.processor_id == 0) {
> +        t->processor_id[0] = cpu_to_le32(smbios_cpuid_version);
> +        t->processor_id[1] = cpu_to_le32(smbios_cpuid_features);
> +    } else {
> +        t->processor_id[0] = cpu_to_le32(type4.processor_id);
> +        t->processor_id[1] = 0;
> +    }
>      SMBIOS_TABLE_SET_STR(4, processor_version_str, type4.version);
>      t->voltage = 0;
>      t->external_clock = cpu_to_le16(0); /* Unknown */
> @@ -1292,6 +1303,8 @@ void smbios_entry_add(QemuOpts *opts, Error **errp)
>              save_opt(&type4.serial, opts, "serial");
>              save_opt(&type4.asset, opts, "asset");
>              save_opt(&type4.part, opts, "part");
> +            /* If the value is 0, it will take the value from the CPU model. 
> */
> +            type4.processor_id = qemu_opt_get_number(opts, "processor-id", 
> 0);
>              type4.max_speed = qemu_opt_get_number(opts, "max-speed",
>                                                    DEFAULT_CPU_SPEED);
>              type4.current_speed = qemu_opt_get_number(opts, "current-speed",
> diff --git a/qemu-options.hx b/qemu-options.hx
> index ec90505d84..3c51b6cf8f 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -2527,6 +2527,7 @@ DEF("smbios", HAS_ARG, QEMU_OPTION_smbios,
>      "                specify SMBIOS type 3 fields\n"
>      "-smbios 
> type=4[,sock_pfx=str][,manufacturer=str][,version=str][,serial=str]\n"
>      "              
> [,asset=str][,part=str][,max-speed=%d][,current-speed=%d]\n"
> +    "              [,processor-id=%d]\n"
>      "                specify SMBIOS type 4 fields\n"
>      "-smbios type=11[,value=str][,path=filename]\n"
>      "                specify SMBIOS type 11 fields\n"

missing update of SRST part