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

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

[Patrick Venture]

This parameter is to be used in the processor_id lower 32-bit entry in
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.

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"
-- 
2.34.1.448.ga2b2bfdf31-goog

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

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

[Patrick Venture]

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

On Tue, Jan 11, 2022 at 5:13 AM Igor Mammedov <imammedo@redhat.com> wrote:

> 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.
>

Ack


>
> > 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?
>

Because presumably the cpuid_feature value would not match whatever
override was chosen here.  But I like your idea of just making it a
quadword.


>
>
> > 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
>

I grepped for SRST, where is this that I need to update also?

Thanks!

[-- Attachment #2: Type: text/html, Size: 6499 bytes --]

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

[Igor Mammedov]

On Tue, 18 Jan 2022 09:15:42 -0800
Patrick Venture <venture@google.com> wrote:

> On Tue, Jan 11, 2022 at 5:13 AM Igor Mammedov <imammedo@redhat.com> wrote:
> 
> > On Thu,  6 Jan 2022 14:33:16 -0800
> > Patrick Venture <venture@google.com> wrote:
> >  
[...]
> > > 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
> >  
> 
> I grepped for SRST, where is this that I need to update also?

option definition has 2 parts DEF() and SRST that follows right
after it, the later is used as help text for the option
SRST                                                                             
``-smbios file=binary`` 
...

> 
> Thanks!

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

[Patrick Venture]

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

On Tue, Jan 18, 2022 at 11:37 PM Igor Mammedov <imammedo@redhat.com> wrote:

> On Tue, 18 Jan 2022 09:15:42 -0800
> Patrick Venture <venture@google.com> wrote:
>
> > On Tue, Jan 11, 2022 at 5:13 AM Igor Mammedov <imammedo@redhat.com>
> wrote:
> >
> > > On Thu,  6 Jan 2022 14:33:16 -0800
> > > Patrick Venture <venture@google.com> wrote:
> > >
> [...]
> > > > 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
> > >
> >
> > I grepped for SRST, where is this that I need to update also?
>
> option definition has 2 parts DEF() and SRST that follows right
> after it, the later is used as help text for the option
> SRST


Ahh, yeah, I grepped in the wrong place :)

Working on validating v2 presently then sending out.


>
> ``-smbios file=binary``
> ...
>
> >
> > Thanks!
>

Thanks!

[-- Attachment #2: Type: text/html, Size: 2740 bytes --]