| Message ID | 20200225075046.30151-1-guoheyi@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | hw/smbios: add options for type 4 max_speed and current_speed | expand |
On 2/25/20 8:50 AM, Heyi Guo wrote: > Common VM users sometimes care about CPU speed, so we add two new > options to allow VM vendors to present CPU speed to their users. > Normally these information can be fetched from host smbios. > > Strictly speaking, the "max speed" and "current speed" in type 4 > are not really for the max speed and current speed of processor, for > "max speed" identifies a capability of the system, and "current speed" > identifies the processor's speed at boot (see smbios spec), but some > applications do not tell the differences. > > Signed-off-by: Heyi Guo <guoheyi@huawei.com> > > --- > Cc: "Michael S. Tsirkin" <mst@redhat.com> > Cc: Igor Mammedov <imammedo@redhat.com> > --- > hw/smbios/smbios.c | 22 +++++++++++++++++++--- > qemu-options.hx | 3 ++- > 2 files changed, 21 insertions(+), 4 deletions(-) > > diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c > index ffd98727ee..1d5439643d 100644 > --- a/hw/smbios/smbios.c > +++ b/hw/smbios/smbios.c > @@ -94,6 +94,8 @@ static struct { > > static struct { > const char *sock_pfx, *manufacturer, *version, *serial, *asset, *part; > + uint32_t max_speed; > + uint32_t current_speed; > } type4; > > static struct { > @@ -272,6 +274,14 @@ static const QemuOptDesc qemu_smbios_type4_opts[] = { > .name = "version", > .type = QEMU_OPT_STRING, > .help = "version number", > + },{ > + .name = "max_speed", > + .type = QEMU_OPT_NUMBER, > + .help = "max speed in MHz", > + },{ > + .name = "current_speed", > + .type = QEMU_OPT_NUMBER, > + .help = "speed at system boot in MHz", > },{ > .name = "serial", > .type = QEMU_OPT_STRING, > @@ -586,9 +596,8 @@ static void smbios_build_type_4_table(MachineState *ms, unsigned instance) > SMBIOS_TABLE_SET_STR(4, processor_version_str, type4.version); > t->voltage = 0; > t->external_clock = cpu_to_le16(0); /* Unknown */ > - /* SVVP requires max_speed and current_speed to not be unknown. */ > - t->max_speed = cpu_to_le16(2000); /* 2000 MHz */ > - t->current_speed = cpu_to_le16(2000); /* 2000 MHz */ > + t->max_speed = cpu_to_le16(type4.max_speed); > + t->current_speed = cpu_to_le16(type4.current_speed); > t->status = 0x41; /* Socket populated, CPU enabled */ > t->processor_upgrade = 0x01; /* Other */ > t->l1_cache_handle = cpu_to_le16(0xFFFF); /* N/A */ > @@ -1129,6 +1138,13 @@ 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"); > + /* > + * SVVP requires max_speed and current_speed to not be unknown, and > + * we set the default value to 2000MHz as we did before. > + */ > + type4.max_speed = qemu_opt_get_number(opts, "max_speed", 2000); > + type4.current_speed = qemu_opt_get_number(opts, "current_speed", > + 2000); Maybe check speeds are <= UINT16_MAX else set errp? > return; > case 11: > qemu_opts_validate(opts, qemu_smbios_type11_opts, &err); > diff --git a/qemu-options.hx b/qemu-options.hx > index ac315c1ac4..bc9ef0fda8 100644 > --- a/qemu-options.hx > +++ b/qemu-options.hx > @@ -2233,6 +2233,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]\n" > + " [,max_speed=%d][,current_speed=%d]\n" > " specify SMBIOS type 4 fields\n" > "-smbios type=17[,loc_pfx=str][,bank=str][,manufacturer=str][,serial=str]\n" > " [,asset=str][,part=str][,speed=%d]\n" > @@ -2255,7 +2256,7 @@ Specify SMBIOS type 2 fields > @item -smbios type=3[,manufacturer=@var{str}][,version=@var{str}][,serial=@var{str}][,asset=@var{str}][,sku=@var{str}] > Specify SMBIOS type 3 fields > > -@item -smbios type=4[,sock_pfx=@var{str}][,manufacturer=@var{str}][,version=@var{str}][,serial=@var{str}][,asset=@var{str}][,part=@var{str}] > +@item -smbios type=4[,sock_pfx=@var{str}][,manufacturer=@var{str}][,version=@var{str}][,serial=@var{str}][,asset=@var{str}][,part=@var{str}][,max_speed=@var{%d}][,current_speed=@var{%d}] > Specify SMBIOS type 4 fields > > @item -smbios type=17[,loc_pfx=@var{str}][,bank=@var{str}][,manufacturer=@var{str}][,serial=@var{str}][,asset=@var{str}][,part=@var{str}][,speed=@var{%d}] >
On 2020/2/25 17:24, Philippe Mathieu-Daudé wrote: > On 2/25/20 8:50 AM, Heyi Guo wrote: >> Common VM users sometimes care about CPU speed, so we add two new >> options to allow VM vendors to present CPU speed to their users. >> Normally these information can be fetched from host smbios. >> >> Strictly speaking, the "max speed" and "current speed" in type 4 >> are not really for the max speed and current speed of processor, for >> "max speed" identifies a capability of the system, and "current speed" >> identifies the processor's speed at boot (see smbios spec), but some >> applications do not tell the differences. >> >> Signed-off-by: Heyi Guo <guoheyi@huawei.com> >> >> --- >> Cc: "Michael S. Tsirkin" <mst@redhat.com> >> Cc: Igor Mammedov <imammedo@redhat.com> >> --- >> hw/smbios/smbios.c | 22 +++++++++++++++++++--- >> qemu-options.hx | 3 ++- >> 2 files changed, 21 insertions(+), 4 deletions(-) >> >> diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c >> index ffd98727ee..1d5439643d 100644 >> --- a/hw/smbios/smbios.c >> +++ b/hw/smbios/smbios.c >> @@ -94,6 +94,8 @@ static struct { >> static struct { >> const char *sock_pfx, *manufacturer, *version, *serial, *asset, >> *part; >> + uint32_t max_speed; >> + uint32_t current_speed; >> } type4; >> static struct { >> @@ -272,6 +274,14 @@ static const QemuOptDesc >> qemu_smbios_type4_opts[] = { >> .name = "version", >> .type = QEMU_OPT_STRING, >> .help = "version number", >> + },{ >> + .name = "max_speed", >> + .type = QEMU_OPT_NUMBER, >> + .help = "max speed in MHz", >> + },{ >> + .name = "current_speed", >> + .type = QEMU_OPT_NUMBER, >> + .help = "speed at system boot in MHz", >> },{ >> .name = "serial", >> .type = QEMU_OPT_STRING, >> @@ -586,9 +596,8 @@ static void >> smbios_build_type_4_table(MachineState *ms, unsigned instance) >> SMBIOS_TABLE_SET_STR(4, processor_version_str, type4.version); >> t->voltage = 0; >> t->external_clock = cpu_to_le16(0); /* Unknown */ >> - /* SVVP requires max_speed and current_speed to not be unknown. */ >> - t->max_speed = cpu_to_le16(2000); /* 2000 MHz */ >> - t->current_speed = cpu_to_le16(2000); /* 2000 MHz */ >> + t->max_speed = cpu_to_le16(type4.max_speed); >> + t->current_speed = cpu_to_le16(type4.current_speed); >> t->status = 0x41; /* Socket populated, CPU enabled */ >> t->processor_upgrade = 0x01; /* Other */ >> t->l1_cache_handle = cpu_to_le16(0xFFFF); /* N/A */ >> @@ -1129,6 +1138,13 @@ 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"); >> + /* >> + * SVVP requires max_speed and current_speed to not be >> unknown, and >> + * we set the default value to 2000MHz as we did before. >> + */ >> + type4.max_speed = qemu_opt_get_number(opts, "max_speed", >> 2000); >> + type4.current_speed = qemu_opt_get_number(opts, >> "current_speed", >> + 2000); > > Maybe check speeds are <= UINT16_MAX else set errp? OK; I can do that in the v2. But I would wait for the maintainers to provide more comments :) Thanks, Heyi > >> return; >> case 11: >> qemu_opts_validate(opts, qemu_smbios_type11_opts, &err); >> diff --git a/qemu-options.hx b/qemu-options.hx >> index ac315c1ac4..bc9ef0fda8 100644 >> --- a/qemu-options.hx >> +++ b/qemu-options.hx >> @@ -2233,6 +2233,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]\n" >> + " [,max_speed=%d][,current_speed=%d]\n" >> " specify SMBIOS type 4 fields\n" >> "-smbios >> type=17[,loc_pfx=str][,bank=str][,manufacturer=str][,serial=str]\n" >> " [,asset=str][,part=str][,speed=%d]\n" >> @@ -2255,7 +2256,7 @@ Specify SMBIOS type 2 fields >> @item -smbios >> type=3[,manufacturer=@var{str}][,version=@var{str}][,serial=@var{str}][,asset=@var{str}][,sku=@var{str}] >> Specify SMBIOS type 3 fields >> -@item -smbios >> type=4[,sock_pfx=@var{str}][,manufacturer=@var{str}][,version=@var{str}][,serial=@var{str}][,asset=@var{str}][,part=@var{str}] >> +@item -smbios >> type=4[,sock_pfx=@var{str}][,manufacturer=@var{str}][,version=@var{str}][,serial=@var{str}][,asset=@var{str}][,part=@var{str}][,max_speed=@var{%d}][,current_speed=@var{%d}] >> Specify SMBIOS type 4 fields >> @item -smbios >> type=17[,loc_pfx=@var{str}][,bank=@var{str}][,manufacturer=@var{str}][,serial=@var{str}][,asset=@var{str}][,part=@var{str}][,speed=@var{%d}] >> > > > .
On Thu, 27 Feb 2020 17:12:21 +0800 Heyi Guo <guoheyi@huawei.com> wrote: > On 2020/2/25 17:24, Philippe Mathieu-Daudé wrote: > > On 2/25/20 8:50 AM, Heyi Guo wrote: > >> Common VM users sometimes care about CPU speed, so we add two new > >> options to allow VM vendors to present CPU speed to their users. > >> Normally these information can be fetched from host smbios. > >> > >> Strictly speaking, the "max speed" and "current speed" in type 4 > >> are not really for the max speed and current speed of processor, for > >> "max speed" identifies a capability of the system, and "current speed" > >> identifies the processor's speed at boot (see smbios spec), but some > >> applications do not tell the differences. > >> > >> Signed-off-by: Heyi Guo <guoheyi@huawei.com> > >> > >> --- > >> Cc: "Michael S. Tsirkin" <mst@redhat.com> > >> Cc: Igor Mammedov <imammedo@redhat.com> > >> --- > >> hw/smbios/smbios.c | 22 +++++++++++++++++++--- > >> qemu-options.hx | 3 ++- > >> 2 files changed, 21 insertions(+), 4 deletions(-) > >> > >> diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c > >> index ffd98727ee..1d5439643d 100644 > >> --- a/hw/smbios/smbios.c > >> +++ b/hw/smbios/smbios.c > >> @@ -94,6 +94,8 @@ static struct { > >> static struct { > >> const char *sock_pfx, *manufacturer, *version, *serial, *asset, > >> *part; > >> + uint32_t max_speed; > >> + uint32_t current_speed; > >> } type4; > >> static struct { > >> @@ -272,6 +274,14 @@ static const QemuOptDesc > >> qemu_smbios_type4_opts[] = { > >> .name = "version", > >> .type = QEMU_OPT_STRING, > >> .help = "version number", > >> + },{ > >> + .name = "max_speed", I'd suggest to use - instead of _ in option name > >> + .type = QEMU_OPT_NUMBER, > >> + .help = "max speed in MHz", > >> + },{ > >> + .name = "current_speed", ditto > >> + .type = QEMU_OPT_NUMBER, > >> + .help = "speed at system boot in MHz", > >> },{ > >> .name = "serial", > >> .type = QEMU_OPT_STRING, > >> @@ -586,9 +596,8 @@ static void > >> smbios_build_type_4_table(MachineState *ms, unsigned instance) > >> SMBIOS_TABLE_SET_STR(4, processor_version_str, type4.version); > >> t->voltage = 0; > >> t->external_clock = cpu_to_le16(0); /* Unknown */ > >> - /* SVVP requires max_speed and current_speed to not be unknown. */ > >> - t->max_speed = cpu_to_le16(2000); /* 2000 MHz */ > >> - t->current_speed = cpu_to_le16(2000); /* 2000 MHz */ > >> + t->max_speed = cpu_to_le16(type4.max_speed); > >> + t->current_speed = cpu_to_le16(type4.current_speed); > >> t->status = 0x41; /* Socket populated, CPU enabled */ > >> t->processor_upgrade = 0x01; /* Other */ > >> t->l1_cache_handle = cpu_to_le16(0xFFFF); /* N/A */ > >> @@ -1129,6 +1138,13 @@ 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"); > >> + /* > >> + * SVVP requires max_speed and current_speed to not be > >> unknown, and > >> + * we set the default value to 2000MHz as we did before. > >> + */ > >> + type4.max_speed = qemu_opt_get_number(opts, "max_speed", > >> 2000); > >> + type4.current_speed = qemu_opt_get_number(opts, > >> "current_speed", > >> + 2000); > > > > Maybe check speeds are <= UINT16_MAX else set errp? > > OK; I can do that in the v2. But I would wait for the maintainers to > provide more comments :) > > Thanks, > > Heyi > > > > >> return; > >> case 11: > >> qemu_opts_validate(opts, qemu_smbios_type11_opts, &err); > >> diff --git a/qemu-options.hx b/qemu-options.hx > >> index ac315c1ac4..bc9ef0fda8 100644 > >> --- a/qemu-options.hx > >> +++ b/qemu-options.hx > >> @@ -2233,6 +2233,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]\n" > >> + " [,max_speed=%d][,current_speed=%d]\n" > >> " specify SMBIOS type 4 fields\n" > >> "-smbios > >> type=17[,loc_pfx=str][,bank=str][,manufacturer=str][,serial=str]\n" > >> " [,asset=str][,part=str][,speed=%d]\n" > >> @@ -2255,7 +2256,7 @@ Specify SMBIOS type 2 fields > >> @item -smbios > >> type=3[,manufacturer=@var{str}][,version=@var{str}][,serial=@var{str}][,asset=@var{str}][,sku=@var{str}] > >> Specify SMBIOS type 3 fields > >> -@item -smbios > >> type=4[,sock_pfx=@var{str}][,manufacturer=@var{str}][,version=@var{str}][,serial=@var{str}][,asset=@var{str}][,part=@var{str}] > >> +@item -smbios > >> type=4[,sock_pfx=@var{str}][,manufacturer=@var{str}][,version=@var{str}][,serial=@var{str}][,asset=@var{str}][,part=@var{str}][,max_speed=@var{%d}][,current_speed=@var{%d}] > >> Specify SMBIOS type 4 fields > >> @item -smbios > >> type=17[,loc_pfx=@var{str}][,bank=@var{str}][,manufacturer=@var{str}][,serial=@var{str}][,asset=@var{str}][,part=@var{str}][,speed=@var{%d}] > >> > > > > > > . >
Hi Igor, On 2020/2/28 17:39, Igor Mammedov wrote: > On Thu, 27 Feb 2020 17:12:21 +0800 > Heyi Guo <guoheyi@huawei.com> wrote: > >> On 2020/2/25 17:24, Philippe Mathieu-Daudé wrote: >>> On 2/25/20 8:50 AM, Heyi Guo wrote: >>>> Common VM users sometimes care about CPU speed, so we add two new >>>> options to allow VM vendors to present CPU speed to their users. >>>> Normally these information can be fetched from host smbios. >>>> >>>> Strictly speaking, the "max speed" and "current speed" in type 4 >>>> are not really for the max speed and current speed of processor, for >>>> "max speed" identifies a capability of the system, and "current speed" >>>> identifies the processor's speed at boot (see smbios spec), but some >>>> applications do not tell the differences. >>>> >>>> Signed-off-by: Heyi Guo <guoheyi@huawei.com> >>>> >>>> --- >>>> Cc: "Michael S. Tsirkin" <mst@redhat.com> >>>> Cc: Igor Mammedov <imammedo@redhat.com> >>>> --- >>>> hw/smbios/smbios.c | 22 +++++++++++++++++++--- >>>> qemu-options.hx | 3 ++- >>>> 2 files changed, 21 insertions(+), 4 deletions(-) >>>> >>>> diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c >>>> index ffd98727ee..1d5439643d 100644 >>>> --- a/hw/smbios/smbios.c >>>> +++ b/hw/smbios/smbios.c >>>> @@ -94,6 +94,8 @@ static struct { >>>> static struct { >>>> const char *sock_pfx, *manufacturer, *version, *serial, *asset, >>>> *part; >>>> + uint32_t max_speed; >>>> + uint32_t current_speed; >>>> } type4; >>>> static struct { >>>> @@ -272,6 +274,14 @@ static const QemuOptDesc >>>> qemu_smbios_type4_opts[] = { >>>> .name = "version", >>>> .type = QEMU_OPT_STRING, >>>> .help = "version number", >>>> + },{ >>>> + .name = "max_speed", > I'd suggest to use - instead of _ in option name Thanks for your comments. However I can see other options like "sock_pfx" and "loc_pfx" also use "_" in option names. Should we keep consistent with the context? Thanks, Heyi > >>>> + .type = QEMU_OPT_NUMBER, >>>> + .help = "max speed in MHz", >>>> + },{ >>>> + .name = "current_speed", > ditto > >>>> + .type = QEMU_OPT_NUMBER, >>>> + .help = "speed at system boot in MHz", >>>> },{ >>>> .name = "serial", >>>> .type = QEMU_OPT_STRING, >>>> @@ -586,9 +596,8 @@ static void >>>> smbios_build_type_4_table(MachineState *ms, unsigned instance) >>>> SMBIOS_TABLE_SET_STR(4, processor_version_str, type4.version); >>>> t->voltage = 0; >>>> t->external_clock = cpu_to_le16(0); /* Unknown */ >>>> - /* SVVP requires max_speed and current_speed to not be unknown. */ >>>> - t->max_speed = cpu_to_le16(2000); /* 2000 MHz */ >>>> - t->current_speed = cpu_to_le16(2000); /* 2000 MHz */ >>>> + t->max_speed = cpu_to_le16(type4.max_speed); >>>> + t->current_speed = cpu_to_le16(type4.current_speed); >>>> t->status = 0x41; /* Socket populated, CPU enabled */ >>>> t->processor_upgrade = 0x01; /* Other */ >>>> t->l1_cache_handle = cpu_to_le16(0xFFFF); /* N/A */ >>>> @@ -1129,6 +1138,13 @@ 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"); >>>> + /* >>>> + * SVVP requires max_speed and current_speed to not be >>>> unknown, and >>>> + * we set the default value to 2000MHz as we did before. >>>> + */ >>>> + type4.max_speed = qemu_opt_get_number(opts, "max_speed", >>>> 2000); >>>> + type4.current_speed = qemu_opt_get_number(opts, >>>> "current_speed", >>>> + 2000); >>> Maybe check speeds are <= UINT16_MAX else set errp? >> OK; I can do that in the v2. But I would wait for the maintainers to >> provide more comments :) >> >> Thanks, >> >> Heyi >> >>> >>>> return; >>>> case 11: >>>> qemu_opts_validate(opts, qemu_smbios_type11_opts, &err); >>>> diff --git a/qemu-options.hx b/qemu-options.hx >>>> index ac315c1ac4..bc9ef0fda8 100644 >>>> --- a/qemu-options.hx >>>> +++ b/qemu-options.hx >>>> @@ -2233,6 +2233,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]\n" >>>> + " [,max_speed=%d][,current_speed=%d]\n" >>>> " specify SMBIOS type 4 fields\n" >>>> "-smbios >>>> type=17[,loc_pfx=str][,bank=str][,manufacturer=str][,serial=str]\n" >>>> " [,asset=str][,part=str][,speed=%d]\n" >>>> @@ -2255,7 +2256,7 @@ Specify SMBIOS type 2 fields >>>> @item -smbios >>>> type=3[,manufacturer=@var{str}][,version=@var{str}][,serial=@var{str}][,asset=@var{str}][,sku=@var{str}] >>>> Specify SMBIOS type 3 fields >>>> -@item -smbios >>>> type=4[,sock_pfx=@var{str}][,manufacturer=@var{str}][,version=@var{str}][,serial=@var{str}][,asset=@var{str}][,part=@var{str}] >>>> +@item -smbios >>>> type=4[,sock_pfx=@var{str}][,manufacturer=@var{str}][,version=@var{str}][,serial=@var{str}][,asset=@var{str}][,part=@var{str}][,max_speed=@var{%d}][,current_speed=@var{%d}] >>>> Specify SMBIOS type 4 fields >>>> @item -smbios >>>> type=17[,loc_pfx=@var{str}][,bank=@var{str}][,manufacturer=@var{str}][,serial=@var{str}][,asset=@var{str}][,part=@var{str}][,speed=@var{%d}] >>>> >>> >>> . > > .
On Sat, 29 Feb 2020 08:17:48 +0800 Heyi Guo <guoheyi@huawei.com> wrote: > Hi Igor, > > On 2020/2/28 17:39, Igor Mammedov wrote: > > On Thu, 27 Feb 2020 17:12:21 +0800 > > Heyi Guo <guoheyi@huawei.com> wrote: > > > >> On 2020/2/25 17:24, Philippe Mathieu-Daudé wrote: > >>> On 2/25/20 8:50 AM, Heyi Guo wrote: > >>>> Common VM users sometimes care about CPU speed, so we add two new > >>>> options to allow VM vendors to present CPU speed to their users. > >>>> Normally these information can be fetched from host smbios. > >>>> > >>>> Strictly speaking, the "max speed" and "current speed" in type 4 > >>>> are not really for the max speed and current speed of processor, for > >>>> "max speed" identifies a capability of the system, and "current speed" > >>>> identifies the processor's speed at boot (see smbios spec), but some > >>>> applications do not tell the differences. > >>>> > >>>> Signed-off-by: Heyi Guo <guoheyi@huawei.com> > >>>> > >>>> --- > >>>> Cc: "Michael S. Tsirkin" <mst@redhat.com> > >>>> Cc: Igor Mammedov <imammedo@redhat.com> > >>>> --- > >>>> hw/smbios/smbios.c | 22 +++++++++++++++++++--- > >>>> qemu-options.hx | 3 ++- > >>>> 2 files changed, 21 insertions(+), 4 deletions(-) > >>>> > >>>> diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c > >>>> index ffd98727ee..1d5439643d 100644 > >>>> --- a/hw/smbios/smbios.c > >>>> +++ b/hw/smbios/smbios.c > >>>> @@ -94,6 +94,8 @@ static struct { > >>>> static struct { > >>>> const char *sock_pfx, *manufacturer, *version, *serial, *asset, > >>>> *part; > >>>> + uint32_t max_speed; > >>>> + uint32_t current_speed; > >>>> } type4; > >>>> static struct { > >>>> @@ -272,6 +274,14 @@ static const QemuOptDesc > >>>> qemu_smbios_type4_opts[] = { > >>>> .name = "version", > >>>> .type = QEMU_OPT_STRING, > >>>> .help = "version number", > >>>> + },{ > >>>> + .name = "max_speed", > > I'd suggest to use - instead of _ in option name > > Thanks for your comments. However I can see other options like > "sock_pfx" and "loc_pfx" also use "_" in option names. Should we keep > consistent with the context? For new options one should use '-', that way we won't have to build wrappers around it if it becomes a qom property in the future. > > Thanks, > > Heyi > > > > > >>>> + .type = QEMU_OPT_NUMBER, > >>>> + .help = "max speed in MHz", > >>>> + },{ > >>>> + .name = "current_speed", > > ditto > > > >>>> + .type = QEMU_OPT_NUMBER, > >>>> + .help = "speed at system boot in MHz", > >>>> },{ > >>>> .name = "serial", > >>>> .type = QEMU_OPT_STRING, > >>>> @@ -586,9 +596,8 @@ static void > >>>> smbios_build_type_4_table(MachineState *ms, unsigned instance) > >>>> SMBIOS_TABLE_SET_STR(4, processor_version_str, type4.version); > >>>> t->voltage = 0; > >>>> t->external_clock = cpu_to_le16(0); /* Unknown */ > >>>> - /* SVVP requires max_speed and current_speed to not be unknown. */ > >>>> - t->max_speed = cpu_to_le16(2000); /* 2000 MHz */ > >>>> - t->current_speed = cpu_to_le16(2000); /* 2000 MHz */ > >>>> + t->max_speed = cpu_to_le16(type4.max_speed); > >>>> + t->current_speed = cpu_to_le16(type4.current_speed); > >>>> t->status = 0x41; /* Socket populated, CPU enabled */ > >>>> t->processor_upgrade = 0x01; /* Other */ > >>>> t->l1_cache_handle = cpu_to_le16(0xFFFF); /* N/A */ > >>>> @@ -1129,6 +1138,13 @@ 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"); > >>>> + /* > >>>> + * SVVP requires max_speed and current_speed to not be > >>>> unknown, and > >>>> + * we set the default value to 2000MHz as we did before. > >>>> + */ > >>>> + type4.max_speed = qemu_opt_get_number(opts, "max_speed", > >>>> 2000); > >>>> + type4.current_speed = qemu_opt_get_number(opts, > >>>> "current_speed", > >>>> + 2000); > >>> Maybe check speeds are <= UINT16_MAX else set errp? > >> OK; I can do that in the v2. But I would wait for the maintainers to > >> provide more comments :) > >> > >> Thanks, > >> > >> Heyi > >> > >>> > >>>> return; > >>>> case 11: > >>>> qemu_opts_validate(opts, qemu_smbios_type11_opts, &err); > >>>> diff --git a/qemu-options.hx b/qemu-options.hx > >>>> index ac315c1ac4..bc9ef0fda8 100644 > >>>> --- a/qemu-options.hx > >>>> +++ b/qemu-options.hx > >>>> @@ -2233,6 +2233,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]\n" > >>>> + " [,max_speed=%d][,current_speed=%d]\n" > >>>> " specify SMBIOS type 4 fields\n" > >>>> "-smbios > >>>> type=17[,loc_pfx=str][,bank=str][,manufacturer=str][,serial=str]\n" > >>>> " [,asset=str][,part=str][,speed=%d]\n" > >>>> @@ -2255,7 +2256,7 @@ Specify SMBIOS type 2 fields > >>>> @item -smbios > >>>> type=3[,manufacturer=@var{str}][,version=@var{str}][,serial=@var{str}][,asset=@var{str}][,sku=@var{str}] > >>>> Specify SMBIOS type 3 fields > >>>> -@item -smbios > >>>> type=4[,sock_pfx=@var{str}][,manufacturer=@var{str}][,version=@var{str}][,serial=@var{str}][,asset=@var{str}][,part=@var{str}] > >>>> +@item -smbios > >>>> type=4[,sock_pfx=@var{str}][,manufacturer=@var{str}][,version=@var{str}][,serial=@var{str}][,asset=@var{str}][,part=@var{str}][,max_speed=@var{%d}][,current_speed=@var{%d}] > >>>> Specify SMBIOS type 4 fields > >>>> @item -smbios > >>>> type=17[,loc_pfx=@var{str}][,bank=@var{str}][,manufacturer=@var{str}][,serial=@var{str}][,asset=@var{str}][,part=@var{str}][,speed=@var{%d}] > >>>> > >>> > >>> . > > > > . > >
On 2020/3/2 16:20, Igor Mammedov wrote: > On Sat, 29 Feb 2020 08:17:48 +0800 > Heyi Guo <guoheyi@huawei.com> wrote: > >> Hi Igor, >> >> On 2020/2/28 17:39, Igor Mammedov wrote: >>> On Thu, 27 Feb 2020 17:12:21 +0800 >>> Heyi Guo <guoheyi@huawei.com> wrote: >>> >>>> On 2020/2/25 17:24, Philippe Mathieu-Daudé wrote: >>>>> On 2/25/20 8:50 AM, Heyi Guo wrote: >>>>>> Common VM users sometimes care about CPU speed, so we add two new >>>>>> options to allow VM vendors to present CPU speed to their users. >>>>>> Normally these information can be fetched from host smbios. >>>>>> >>>>>> Strictly speaking, the "max speed" and "current speed" in type 4 >>>>>> are not really for the max speed and current speed of processor, for >>>>>> "max speed" identifies a capability of the system, and "current speed" >>>>>> identifies the processor's speed at boot (see smbios spec), but some >>>>>> applications do not tell the differences. >>>>>> >>>>>> Signed-off-by: Heyi Guo <guoheyi@huawei.com> >>>>>> >>>>>> --- >>>>>> Cc: "Michael S. Tsirkin" <mst@redhat.com> >>>>>> Cc: Igor Mammedov <imammedo@redhat.com> >>>>>> --- >>>>>> hw/smbios/smbios.c | 22 +++++++++++++++++++--- >>>>>> qemu-options.hx | 3 ++- >>>>>> 2 files changed, 21 insertions(+), 4 deletions(-) >>>>>> >>>>>> diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c >>>>>> index ffd98727ee..1d5439643d 100644 >>>>>> --- a/hw/smbios/smbios.c >>>>>> +++ b/hw/smbios/smbios.c >>>>>> @@ -94,6 +94,8 @@ static struct { >>>>>> static struct { >>>>>> const char *sock_pfx, *manufacturer, *version, *serial, *asset, >>>>>> *part; >>>>>> + uint32_t max_speed; >>>>>> + uint32_t current_speed; >>>>>> } type4; >>>>>> static struct { >>>>>> @@ -272,6 +274,14 @@ static const QemuOptDesc >>>>>> qemu_smbios_type4_opts[] = { >>>>>> .name = "version", >>>>>> .type = QEMU_OPT_STRING, >>>>>> .help = "version number", >>>>>> + },{ >>>>>> + .name = "max_speed", >>> I'd suggest to use - instead of _ in option name >> Thanks for your comments. However I can see other options like >> "sock_pfx" and "loc_pfx" also use "_" in option names. Should we keep >> consistent with the context? > For new options one should use '-', > that way we won't have to build wrappers around it if it becomes > a qom property in the future. Thanks for the explanation. I'll fix that in v2. Thanks, Heyi > >> Thanks, >> >> Heyi >> >> >>> >>>>>> + .type = QEMU_OPT_NUMBER, >>>>>> + .help = "max speed in MHz", >>>>>> + },{ >>>>>> + .name = "current_speed", >>> ditto >>> >>>>>> + .type = QEMU_OPT_NUMBER, >>>>>> + .help = "speed at system boot in MHz", >>>>>> },{ >>>>>> .name = "serial", >>>>>> .type = QEMU_OPT_STRING, >>>>>> @@ -586,9 +596,8 @@ static void >>>>>> smbios_build_type_4_table(MachineState *ms, unsigned instance) >>>>>> SMBIOS_TABLE_SET_STR(4, processor_version_str, type4.version); >>>>>> t->voltage = 0; >>>>>> t->external_clock = cpu_to_le16(0); /* Unknown */ >>>>>> - /* SVVP requires max_speed and current_speed to not be unknown. */ >>>>>> - t->max_speed = cpu_to_le16(2000); /* 2000 MHz */ >>>>>> - t->current_speed = cpu_to_le16(2000); /* 2000 MHz */ >>>>>> + t->max_speed = cpu_to_le16(type4.max_speed); >>>>>> + t->current_speed = cpu_to_le16(type4.current_speed); >>>>>> t->status = 0x41; /* Socket populated, CPU enabled */ >>>>>> t->processor_upgrade = 0x01; /* Other */ >>>>>> t->l1_cache_handle = cpu_to_le16(0xFFFF); /* N/A */ >>>>>> @@ -1129,6 +1138,13 @@ 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"); >>>>>> + /* >>>>>> + * SVVP requires max_speed and current_speed to not be >>>>>> unknown, and >>>>>> + * we set the default value to 2000MHz as we did before. >>>>>> + */ >>>>>> + type4.max_speed = qemu_opt_get_number(opts, "max_speed", >>>>>> 2000); >>>>>> + type4.current_speed = qemu_opt_get_number(opts, >>>>>> "current_speed", >>>>>> + 2000); >>>>> Maybe check speeds are <= UINT16_MAX else set errp? >>>> OK; I can do that in the v2. But I would wait for the maintainers to >>>> provide more comments :) >>>> >>>> Thanks, >>>> >>>> Heyi >>>> >>>>> >>>>>> return; >>>>>> case 11: >>>>>> qemu_opts_validate(opts, qemu_smbios_type11_opts, &err); >>>>>> diff --git a/qemu-options.hx b/qemu-options.hx >>>>>> index ac315c1ac4..bc9ef0fda8 100644 >>>>>> --- a/qemu-options.hx >>>>>> +++ b/qemu-options.hx >>>>>> @@ -2233,6 +2233,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]\n" >>>>>> + " [,max_speed=%d][,current_speed=%d]\n" >>>>>> " specify SMBIOS type 4 fields\n" >>>>>> "-smbios >>>>>> type=17[,loc_pfx=str][,bank=str][,manufacturer=str][,serial=str]\n" >>>>>> " [,asset=str][,part=str][,speed=%d]\n" >>>>>> @@ -2255,7 +2256,7 @@ Specify SMBIOS type 2 fields >>>>>> @item -smbios >>>>>> type=3[,manufacturer=@var{str}][,version=@var{str}][,serial=@var{str}][,asset=@var{str}][,sku=@var{str}] >>>>>> Specify SMBIOS type 3 fields >>>>>> -@item -smbios >>>>>> type=4[,sock_pfx=@var{str}][,manufacturer=@var{str}][,version=@var{str}][,serial=@var{str}][,asset=@var{str}][,part=@var{str}] >>>>>> +@item -smbios >>>>>> type=4[,sock_pfx=@var{str}][,manufacturer=@var{str}][,version=@var{str}][,serial=@var{str}][,asset=@var{str}][,part=@var{str}][,max_speed=@var{%d}][,current_speed=@var{%d}] >>>>>> Specify SMBIOS type 4 fields >>>>>> @item -smbios >>>>>> type=17[,loc_pfx=@var{str}][,bank=@var{str}][,manufacturer=@var{str}][,serial=@var{str}][,asset=@var{str}][,part=@var{str}][,speed=@var{%d}] >>>>>> >>>>> . >>> . >> > > .
diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c index ffd98727ee..1d5439643d 100644 --- a/hw/smbios/smbios.c +++ b/hw/smbios/smbios.c @@ -94,6 +94,8 @@ static struct { static struct { const char *sock_pfx, *manufacturer, *version, *serial, *asset, *part; + uint32_t max_speed; + uint32_t current_speed; } type4; static struct { @@ -272,6 +274,14 @@ static const QemuOptDesc qemu_smbios_type4_opts[] = { .name = "version", .type = QEMU_OPT_STRING, .help = "version number", + },{ + .name = "max_speed", + .type = QEMU_OPT_NUMBER, + .help = "max speed in MHz", + },{ + .name = "current_speed", + .type = QEMU_OPT_NUMBER, + .help = "speed at system boot in MHz", },{ .name = "serial", .type = QEMU_OPT_STRING, @@ -586,9 +596,8 @@ static void smbios_build_type_4_table(MachineState *ms, unsigned instance) SMBIOS_TABLE_SET_STR(4, processor_version_str, type4.version); t->voltage = 0; t->external_clock = cpu_to_le16(0); /* Unknown */ - /* SVVP requires max_speed and current_speed to not be unknown. */ - t->max_speed = cpu_to_le16(2000); /* 2000 MHz */ - t->current_speed = cpu_to_le16(2000); /* 2000 MHz */ + t->max_speed = cpu_to_le16(type4.max_speed); + t->current_speed = cpu_to_le16(type4.current_speed); t->status = 0x41; /* Socket populated, CPU enabled */ t->processor_upgrade = 0x01; /* Other */ t->l1_cache_handle = cpu_to_le16(0xFFFF); /* N/A */ @@ -1129,6 +1138,13 @@ 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"); + /* + * SVVP requires max_speed and current_speed to not be unknown, and + * we set the default value to 2000MHz as we did before. + */ + type4.max_speed = qemu_opt_get_number(opts, "max_speed", 2000); + type4.current_speed = qemu_opt_get_number(opts, "current_speed", + 2000); return; case 11: qemu_opts_validate(opts, qemu_smbios_type11_opts, &err); diff --git a/qemu-options.hx b/qemu-options.hx index ac315c1ac4..bc9ef0fda8 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -2233,6 +2233,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]\n" + " [,max_speed=%d][,current_speed=%d]\n" " specify SMBIOS type 4 fields\n" "-smbios type=17[,loc_pfx=str][,bank=str][,manufacturer=str][,serial=str]\n" " [,asset=str][,part=str][,speed=%d]\n" @@ -2255,7 +2256,7 @@ Specify SMBIOS type 2 fields @item -smbios type=3[,manufacturer=@var{str}][,version=@var{str}][,serial=@var{str}][,asset=@var{str}][,sku=@var{str}] Specify SMBIOS type 3 fields -@item -smbios type=4[,sock_pfx=@var{str}][,manufacturer=@var{str}][,version=@var{str}][,serial=@var{str}][,asset=@var{str}][,part=@var{str}] +@item -smbios type=4[,sock_pfx=@var{str}][,manufacturer=@var{str}][,version=@var{str}][,serial=@var{str}][,asset=@var{str}][,part=@var{str}][,max_speed=@var{%d}][,current_speed=@var{%d}] Specify SMBIOS type 4 fields @item -smbios type=17[,loc_pfx=@var{str}][,bank=@var{str}][,manufacturer=@var{str}][,serial=@var{str}][,asset=@var{str}][,part=@var{str}][,speed=@var{%d}]
Common VM users sometimes care about CPU speed, so we add two new options to allow VM vendors to present CPU speed to their users. Normally these information can be fetched from host smbios. Strictly speaking, the "max speed" and "current speed" in type 4 are not really for the max speed and current speed of processor, for "max speed" identifies a capability of the system, and "current speed" identifies the processor's speed at boot (see smbios spec), but some applications do not tell the differences. Signed-off-by: Heyi Guo <guoheyi@huawei.com> --- Cc: "Michael S. Tsirkin" <mst@redhat.com> Cc: Igor Mammedov <imammedo@redhat.com> --- hw/smbios/smbios.c | 22 +++++++++++++++++++--- qemu-options.hx | 3 ++- 2 files changed, 21 insertions(+), 4 deletions(-)