* [PATCH v2] hw/smbios: add options for type 4 max-speed and current-speed
@ 2020-03-02 9:29 Heyi Guo
2020-03-02 13:52 ` Igor Mammedov
0 siblings, 1 reply; 3+ messages in thread
From: Heyi Guo @ 2020-03-02 9:29 UTC (permalink / raw)
To: qemu-devel
Cc: Heyi Guo, wanghaibin.wang, Igor Mammedov,
Philippe Mathieu-Daudé, Michael S. Tsirkin
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>
Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
v1 -> v2:
- change "_" in option names to "-"
- check if option value is too large to fit in SMBIOS type 4 speed fields.
---
hw/smbios/smbios.c | 28 +++++++++++++++++++++++++---
qemu-options.hx | 3 ++-
2 files changed, 27 insertions(+), 4 deletions(-)
diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
index ffd98727ee..77135a1eca 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,19 @@ 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);
+ if (type4.max_speed > UINT16_MAX ||
+ type4.current_speed > UINT16_MAX) {
+ error_setg(errp, "SMBIOS CPU speed is too large (> %d)",
+ UINT16_MAX);
+ }
+
return;
case 11:
qemu_opts_validate(opts, qemu_smbios_type11_opts, &err);
diff --git a/qemu-options.hx b/qemu-options.hx
index ac315c1ac4..7a2f7c1f66 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}]
--
2.19.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v2] hw/smbios: add options for type 4 max-speed and current-speed
2020-03-02 9:29 [PATCH v2] hw/smbios: add options for type 4 max-speed and current-speed Heyi Guo
@ 2020-03-02 13:52 ` Igor Mammedov
2020-03-03 1:09 ` Heyi Guo
0 siblings, 1 reply; 3+ messages in thread
From: Igor Mammedov @ 2020-03-02 13:52 UTC (permalink / raw)
To: Heyi Guo
Cc: wanghaibin.wang, Philippe Mathieu-Daudé, qemu-devel,
Michael S. Tsirkin
On Mon, 2 Mar 2020 17:29:10 +0800
Heyi Guo <guoheyi@huawei.com> 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>
With comment fixed as suggested below:
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
PS:
pls add/extend test cases for these options (default and custom settings).
(see test_smbios_structs() for example)
I'd do it as an additional patch.
> ---
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Igor Mammedov <imammedo@redhat.com>
> Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
>
> v1 -> v2:
> - change "_" in option names to "-"
> - check if option value is too large to fit in SMBIOS type 4 speed fields.
>
> ---
> hw/smbios/smbios.c | 28 +++++++++++++++++++++++++---
> qemu-options.hx | 3 ++-
> 2 files changed, 27 insertions(+), 4 deletions(-)
>
> diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
> index ffd98727ee..77135a1eca 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,19 @@ 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
double negation is hard to read, so
s/
to not be unknown
/
to be set and not being 0 which counts as unknown (SMBIOS 3.1.0/Table 21)
/
> + * we set the default value to 2000MHz as we did before.
s/,and we set/. Set/
> + */
> + type4.max_speed = qemu_opt_get_number(opts, "max-speed", 2000);
> + type4.current_speed = qemu_opt_get_number(opts, "current-speed",
> + 2000);
> + if (type4.max_speed > UINT16_MAX ||
> + type4.current_speed > UINT16_MAX) {
> + error_setg(errp, "SMBIOS CPU speed is too large (> %d)",
> + UINT16_MAX);
> + }
> +
> return;
> case 11:
> qemu_opts_validate(opts, qemu_smbios_type11_opts, &err);
> diff --git a/qemu-options.hx b/qemu-options.hx
> index ac315c1ac4..7a2f7c1f66 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}]
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2] hw/smbios: add options for type 4 max-speed and current-speed
2020-03-02 13:52 ` Igor Mammedov
@ 2020-03-03 1:09 ` Heyi Guo
0 siblings, 0 replies; 3+ messages in thread
From: Heyi Guo @ 2020-03-03 1:09 UTC (permalink / raw)
To: Igor Mammedov
Cc: wanghaibin.wang, Philippe Mathieu-Daudé, qemu-devel,
Michael S. Tsirkin
On 2020/3/2 21:52, Igor Mammedov wrote:
> On Mon, 2 Mar 2020 17:29:10 +0800
> Heyi Guo <guoheyi@huawei.com> 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>
> With comment fixed as suggested below:
> Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Thanks; I've fixed that in v3 and add your R-b :)
>
> PS:
> pls add/extend test cases for these options (default and custom settings).
> (see test_smbios_structs() for example)
> I'd do it as an additional patch.
Let me take some time to learn how to write such test cases :)
Thanks,
Heyi
>
>
>> ---
>> Cc: "Michael S. Tsirkin" <mst@redhat.com>
>> Cc: Igor Mammedov <imammedo@redhat.com>
>> Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
>>
>> v1 -> v2:
>> - change "_" in option names to "-"
>> - check if option value is too large to fit in SMBIOS type 4 speed fields.
>>
>> ---
>> hw/smbios/smbios.c | 28 +++++++++++++++++++++++++---
>> qemu-options.hx | 3 ++-
>> 2 files changed, 27 insertions(+), 4 deletions(-)
>>
>> diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
>> index ffd98727ee..77135a1eca 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,19 @@ 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
> double negation is hard to read, so
>
> s/
> to not be unknown
> /
> to be set and not being 0 which counts as unknown (SMBIOS 3.1.0/Table 21)
> /
>
>> + * we set the default value to 2000MHz as we did before.
> s/,and we set/. Set/
>
>> + */
>> + type4.max_speed = qemu_opt_get_number(opts, "max-speed", 2000);
>> + type4.current_speed = qemu_opt_get_number(opts, "current-speed",
>> + 2000);
>> + if (type4.max_speed > UINT16_MAX ||
>> + type4.current_speed > UINT16_MAX) {
>> + error_setg(errp, "SMBIOS CPU speed is too large (> %d)",
>> + UINT16_MAX);
>> + }
>> +
>> return;
>> case 11:
>> qemu_opts_validate(opts, qemu_smbios_type11_opts, &err);
>> diff --git a/qemu-options.hx b/qemu-options.hx
>> index ac315c1ac4..7a2f7c1f66 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}]
>
> .
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-03-03 1:10 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-03-02 9:29 [PATCH v2] hw/smbios: add options for type 4 max-speed and current-speed Heyi Guo
2020-03-02 13:52 ` Igor Mammedov
2020-03-03 1:09 ` Heyi Guo
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).