On Tue, Jan 11, 2022 at 5:13 AM Igor Mammedov <imamm...@redhat.com> wrote:
> On Thu, 6 Jan 2022 14:33:16 -0800
> Patrick Venture <vent...@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 <pefo...@google.com>
> > Reviewed-by: Titus Rwantare <tit...@google.com>
> > Signed-off-by: Patrick Venture <vent...@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!
