[PATCH v4 0/2] add new options to set smbios type 4 fields

[PATCH v4 0/2] add new options to set smbios type 4 fields

[Heyi Guo]

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.

v3 -> v4:
- Fix the default value when not specifying "-smbios type=4" option;
  it would be 0 instead of 2000 in previous versions
- Use uint64_t type to check value overflow
- Add test case to check smbios type 4 CPU speed

Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
Cc: Thomas Huth <thuth@redhat.com>
Cc: Laurent Vivier <lvivier@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>

Heyi Guo (2):
  hw/smbios: add options for type 4 max-speed and current-speed
  tests/bios-tables-test: add smbios cpu speed test

 hw/smbios/smbios.c             | 36 +++++++++++++++++++++++++----
 qemu-options.hx                |  3 ++-
 tests/qtest/bios-tables-test.c | 42 ++++++++++++++++++++++++++++++++++
 3 files changed, 76 insertions(+), 5 deletions(-)

-- 
2.19.1

[PATCH v4 1/2] hw/smbios: add options for type 4 max-speed and current-speed

[Heyi Guo]

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>

v3 -> v4:
- Fix the default value when not specifying "-smbios type=4" option;
  it would be 0 instead of 2000 in previous versions
- Use uint64_t type to check value overflow

v2 -> v3:
- Refine comments per Igor's suggestion.

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 | 36 ++++++++++++++++++++++++++++++++----
 qemu-options.hx    |  3 ++-
 2 files changed, 34 insertions(+), 5 deletions(-)

diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
index ffd98727ee..b95de935e8 100644
--- a/hw/smbios/smbios.c
+++ b/hw/smbios/smbios.c
@@ -92,9 +92,21 @@ static struct {
     const char *manufacturer, *version, *serial, *asset, *sku;
 } type3;
 
+/*
+ * SVVP requires max_speed and current_speed to be set and not being
+ * 0 which counts as unknown (SMBIOS 3.1.0/Table 21). Set the
+ * default value to 2000MHz as we did before.
+ */
+#define DEFAULT_CPU_SPEED 2000
+
 static struct {
     const char *sock_pfx, *manufacturer, *version, *serial, *asset, *part;
-} type4;
+    uint64_t max_speed;
+    uint64_t current_speed;
+} type4 = {
+    .max_speed = DEFAULT_CPU_SPEED,
+    .current_speed = DEFAULT_CPU_SPEED
+};
 
 static struct {
     size_t nvalues;
@@ -272,6 +284,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 +606,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 +1148,15 @@ 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");
+            type4.max_speed = qemu_opt_get_number(opts, "max-speed",
+                                                  DEFAULT_CPU_SPEED);
+            type4.current_speed = qemu_opt_get_number(opts, "current-speed",
+                                                      DEFAULT_CPU_SPEED);
+            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 962a5ebaa6..7665addc78 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2277,6 +2277,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"
@@ -2298,7 +2299,7 @@ SRST
 ``-smbios type=3[,manufacturer=str][,version=str][,serial=str][,asset=str][,sku=str]``
     Specify SMBIOS type 3 fields
 
-``-smbios type=4[,sock_pfx=str][,manufacturer=str][,version=str][,serial=str][,asset=str][,part=str]``
+``-smbios type=4[,sock_pfx=str][,manufacturer=str][,version=str][,serial=str][,asset=str][,part=str][,max-speed=%d][,current-speed=%d]``
     Specify SMBIOS type 4 fields
 
 ``-smbios type=17[,loc_pfx=str][,bank=str][,manufacturer=str][,serial=str][,asset=str][,part=str][,speed=%d]``
-- 
2.19.1

[PATCH v4 2/2] tests/bios-tables-test: add smbios cpu speed test

[Heyi Guo]

Add smbios type 4 CPU speed check for we added new options to set
smbios type 4 "max speed" and "current speed". The default value
should be 2000 when no option is specified, just as the old version
did.

We add the test case to one machine of each architecture, though it
doesn't really run on aarch64 platform for smbios test can't run on
uefi only platform yet.

Signed-off-by: Heyi Guo <guoheyi@huawei.com>

---
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Cc: Thomas Huth <thuth@redhat.com>
Cc: Laurent Vivier <lvivier@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
---
 tests/qtest/bios-tables-test.c | 42 ++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
index 0a597bbacf..f2d2e97b4a 100644
--- a/tests/qtest/bios-tables-test.c
+++ b/tests/qtest/bios-tables-test.c
@@ -77,6 +77,8 @@ typedef struct {
     GArray *tables;
     uint32_t smbios_ep_addr;
     struct smbios_21_entry_point smbios_ep_table;
+    uint16_t smbios_cpu_max_speed;
+    uint16_t smbios_cpu_curr_speed;
     uint8_t *required_struct_types;
     int required_struct_types_len;
     QTestState *qts;
@@ -560,6 +562,31 @@ static inline bool smbios_single_instance(uint8_t type)
     }
 }
 
+static bool smbios_cpu_test(test_data *data, uint32_t addr)
+{
+    uint16_t expect_speed[2];
+    uint16_t real;
+    int offset[2];
+    int i;
+
+    /* Check CPU speed for backward compatibility */
+    offset[0] = offsetof(struct smbios_type_4, max_speed);
+    offset[1] = offsetof(struct smbios_type_4, current_speed);
+    expect_speed[0] = data->smbios_cpu_max_speed ? : 2000;
+    expect_speed[1] = data->smbios_cpu_curr_speed ? : 2000;
+
+    for (i = 0; i < 2; i++) {
+        real = qtest_readw(data->qts, addr + offset[i]);
+        if (real != expect_speed[i]) {
+            fprintf(stderr, "Unexpected SMBIOS CPU speed: real %u expect %u\n",
+                    real, expect_speed[i]);
+            return false;
+        }
+    }
+
+    return true;
+}
+
 static void test_smbios_structs(test_data *data)
 {
     DECLARE_BITMAP(struct_bitmap, SMBIOS_MAX_TYPE+1) = { 0 };
@@ -582,6 +609,10 @@ static void test_smbios_structs(test_data *data)
         }
         set_bit(type, struct_bitmap);
 
+        if (type == 4) {
+            g_assert(smbios_cpu_test(data, addr));
+        }
+
         /* seek to end of unformatted string area of this struct ("\0\0") */
         prv = crt = 1;
         while (prv || crt) {
@@ -716,6 +747,11 @@ static void test_acpi_q35_tcg(void)
     data.required_struct_types_len = ARRAY_SIZE(base_required_struct_types);
     test_acpi_one(NULL, &data);
     free_test_data(&data);
+
+    data.smbios_cpu_max_speed = 3000;
+    data.smbios_cpu_curr_speed = 2600;
+    test_acpi_one("-smbios type=4,max-speed=3000,current-speed=2600", &data);
+    free_test_data(&data);
 }
 
 static void test_acpi_q35_tcg_bridge(void)
@@ -1017,6 +1053,12 @@ static void test_acpi_virt_tcg(void)
 
     test_acpi_one("-cpu cortex-a57", &data);
     free_test_data(&data);
+
+    data.smbios_cpu_max_speed = 2900;
+    data.smbios_cpu_curr_speed = 2700;
+    test_acpi_one("-cpu cortex-a57 "
+                  "-smbios type=4,max-speed=2900,current-speed=2700", &data);
+    free_test_data(&data);
 }
 
 int main(int argc, char *argv[])
-- 
2.19.1

Re: [PATCH v4 1/2] hw/smbios: add options for type 4 max-speed and current-speed

[Igor Mammedov]

On Wed, 18 Mar 2020 14:48:19 +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>

Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> 
> ---
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Igor Mammedov <imammedo@redhat.com>
> Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
> 
> v3 -> v4:
> - Fix the default value when not specifying "-smbios type=4" option;
>   it would be 0 instead of 2000 in previous versions
> - Use uint64_t type to check value overflow
> 
> v2 -> v3:
> - Refine comments per Igor's suggestion.
> 
> 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 | 36 ++++++++++++++++++++++++++++++++----
>  qemu-options.hx    |  3 ++-
>  2 files changed, 34 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
> index ffd98727ee..b95de935e8 100644
> --- a/hw/smbios/smbios.c
> +++ b/hw/smbios/smbios.c
> @@ -92,9 +92,21 @@ static struct {
>      const char *manufacturer, *version, *serial, *asset, *sku;
>  } type3;
>  
> +/*
> + * SVVP requires max_speed and current_speed to be set and not being
> + * 0 which counts as unknown (SMBIOS 3.1.0/Table 21). Set the
> + * default value to 2000MHz as we did before.
> + */
> +#define DEFAULT_CPU_SPEED 2000
> +
>  static struct {
>      const char *sock_pfx, *manufacturer, *version, *serial, *asset, *part;
> -} type4;
> +    uint64_t max_speed;
> +    uint64_t current_speed;
> +} type4 = {
> +    .max_speed = DEFAULT_CPU_SPEED,
> +    .current_speed = DEFAULT_CPU_SPEED
> +};
>  
>  static struct {
>      size_t nvalues;
> @@ -272,6 +284,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 +606,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 +1148,15 @@ 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");
> +            type4.max_speed = qemu_opt_get_number(opts, "max-speed",
> +                                                  DEFAULT_CPU_SPEED);
> +            type4.current_speed = qemu_opt_get_number(opts, "current-speed",
> +                                                      DEFAULT_CPU_SPEED);
> +            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 962a5ebaa6..7665addc78 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -2277,6 +2277,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"
> @@ -2298,7 +2299,7 @@ SRST
>  ``-smbios type=3[,manufacturer=str][,version=str][,serial=str][,asset=str][,sku=str]``
>      Specify SMBIOS type 3 fields
>  
> -``-smbios type=4[,sock_pfx=str][,manufacturer=str][,version=str][,serial=str][,asset=str][,part=str]``
> +``-smbios type=4[,sock_pfx=str][,manufacturer=str][,version=str][,serial=str][,asset=str][,part=str][,max-speed=%d][,current-speed=%d]``
>      Specify SMBIOS type 4 fields
>  
>  ``-smbios type=17[,loc_pfx=str][,bank=str][,manufacturer=str][,serial=str][,asset=str][,part=str][,speed=%d]``

Re: [PATCH v4 2/2] tests/bios-tables-test: add smbios cpu speed test

[Igor Mammedov]

On Wed, 18 Mar 2020 14:48:20 +0800
Heyi Guo <guoheyi@huawei.com> wrote:

> Add smbios type 4 CPU speed check for we added new options to set
> smbios type 4 "max speed" and "current speed". The default value
> should be 2000 when no option is specified, just as the old version
> did.
> 
> We add the test case to one machine of each architecture, though it
> doesn't really run on aarch64 platform for smbios test can't run on
> uefi only platform yet.
I suggest to drop aarch64 part then as it just wastes resources,
alternatively you can enable smbios tests with UEFI.
(Not sure if uefi already exposes smbios entry point in test structure,
CCing Lazslo)

> 
> Signed-off-by: Heyi Guo <guoheyi@huawei.com>
> 
> ---
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Igor Mammedov <imammedo@redhat.com>
> Cc: Thomas Huth <thuth@redhat.com>
> Cc: Laurent Vivier <lvivier@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  tests/qtest/bios-tables-test.c | 42 ++++++++++++++++++++++++++++++++++
>  1 file changed, 42 insertions(+)
> 
> diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
> index 0a597bbacf..f2d2e97b4a 100644
> --- a/tests/qtest/bios-tables-test.c
> +++ b/tests/qtest/bios-tables-test.c
> @@ -77,6 +77,8 @@ typedef struct {
>      GArray *tables;
>      uint32_t smbios_ep_addr;
>      struct smbios_21_entry_point smbios_ep_table;
> +    uint16_t smbios_cpu_max_speed;
> +    uint16_t smbios_cpu_curr_speed;
>      uint8_t *required_struct_types;
>      int required_struct_types_len;
>      QTestState *qts;
> @@ -560,6 +562,31 @@ static inline bool smbios_single_instance(uint8_t type)
>      }
>  }
>  
> +static bool smbios_cpu_test(test_data *data, uint32_t addr)
> +{
> +    uint16_t expect_speed[2];
> +    uint16_t real;
> +    int offset[2];
> +    int i;
> +
> +    /* Check CPU speed for backward compatibility */
> +    offset[0] = offsetof(struct smbios_type_4, max_speed);
> +    offset[1] = offsetof(struct smbios_type_4, current_speed);
> +    expect_speed[0] = data->smbios_cpu_max_speed ? : 2000;
> +    expect_speed[1] = data->smbios_cpu_curr_speed ? : 2000;
> +
> +    for (i = 0; i < 2; i++) {
> +        real = qtest_readw(data->qts, addr + offset[i]);
> +        if (real != expect_speed[i]) {
> +            fprintf(stderr, "Unexpected SMBIOS CPU speed: real %u expect %u\n",
> +                    real, expect_speed[i]);
> +            return false;
> +        }
> +    }
> +
> +    return true;
> +}
> +
>  static void test_smbios_structs(test_data *data)
>  {
>      DECLARE_BITMAP(struct_bitmap, SMBIOS_MAX_TYPE+1) = { 0 };
> @@ -582,6 +609,10 @@ static void test_smbios_structs(test_data *data)
>          }
>          set_bit(type, struct_bitmap);
>  
> +        if (type == 4) {
> +            g_assert(smbios_cpu_test(data, addr));
> +        }
> +
>          /* seek to end of unformatted string area of this struct ("\0\0") */
>          prv = crt = 1;
>          while (prv || crt) {
> @@ -716,6 +747,11 @@ static void test_acpi_q35_tcg(void)
>      data.required_struct_types_len = ARRAY_SIZE(base_required_struct_types);
>      test_acpi_one(NULL, &data);
>      free_test_data(&data);
> +
> +    data.smbios_cpu_max_speed = 3000;
> +    data.smbios_cpu_curr_speed = 2600;
> +    test_acpi_one("-smbios type=4,max-speed=3000,current-speed=2600", &data);
> +    free_test_data(&data);
>  }
>  
>  static void test_acpi_q35_tcg_bridge(void)
> @@ -1017,6 +1053,12 @@ static void test_acpi_virt_tcg(void)
>  
>      test_acpi_one("-cpu cortex-a57", &data);
>      free_test_data(&data);
> +
> +    data.smbios_cpu_max_speed = 2900;
> +    data.smbios_cpu_curr_speed = 2700;
> +    test_acpi_one("-cpu cortex-a57 "
> +                  "-smbios type=4,max-speed=2900,current-speed=2700", &data);
> +    free_test_data(&data);
>  }
>  
>  int main(int argc, char *argv[])

Re: [PATCH v4 0/2] add new options to set smbios type 4 fields

[Igor Mammedov]

On Wed, 18 Mar 2020 14:48:18 +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.

it's probably too late for this series due to soft-freeze,
pls repost once 5.0 is released

> 
> v3 -> v4:
> - Fix the default value when not specifying "-smbios type=4" option;
>   it would be 0 instead of 2000 in previous versions
> - Use uint64_t type to check value overflow
> - Add test case to check smbios type 4 CPU speed
> 
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Igor Mammedov <imammedo@redhat.com>
> Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
> Cc: Thomas Huth <thuth@redhat.com>
> Cc: Laurent Vivier <lvivier@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> 
> Heyi Guo (2):
>   hw/smbios: add options for type 4 max-speed and current-speed
>   tests/bios-tables-test: add smbios cpu speed test
> 
>  hw/smbios/smbios.c             | 36 +++++++++++++++++++++++++----
>  qemu-options.hx                |  3 ++-
>  tests/qtest/bios-tables-test.c | 42 ++++++++++++++++++++++++++++++++++
>  3 files changed, 76 insertions(+), 5 deletions(-)
> 

Re: [PATCH v4 0/2] add new options to set smbios type 4 fields

[Heyi Guo]

On 2020/3/19 22:46, Igor Mammedov wrote:
> On Wed, 18 Mar 2020 14:48:18 +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.
> it's probably too late for this series due to soft-freeze,
> pls repost once 5.0 is released

Ah, I didn't pay enough attention to the merge window.

When will the soft-freeze be ended? Will it be announced in the mailing 
list?

Thanks,

Heyi

>
>> v3 -> v4:
>> - Fix the default value when not specifying "-smbios type=4" option;
>>    it would be 0 instead of 2000 in previous versions
>> - Use uint64_t type to check value overflow
>> - Add test case to check smbios type 4 CPU speed
>>
>> Cc: "Michael S. Tsirkin" <mst@redhat.com>
>> Cc: Igor Mammedov <imammedo@redhat.com>
>> Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
>> Cc: Thomas Huth <thuth@redhat.com>
>> Cc: Laurent Vivier <lvivier@redhat.com>
>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>>
>> Heyi Guo (2):
>>    hw/smbios: add options for type 4 max-speed and current-speed
>>    tests/bios-tables-test: add smbios cpu speed test
>>
>>   hw/smbios/smbios.c             | 36 +++++++++++++++++++++++++----
>>   qemu-options.hx                |  3 ++-
>>   tests/qtest/bios-tables-test.c | 42 ++++++++++++++++++++++++++++++++++
>>   3 files changed, 76 insertions(+), 5 deletions(-)
>>
>
> .

Re: [PATCH v4 0/2] add new options to set smbios type 4 fields

[Paolo Bonzini]

On 20/03/20 02:29, Heyi Guo wrote:
> 
> On 2020/3/19 22:46, Igor Mammedov wrote:
>> On Wed, 18 Mar 2020 14:48:18 +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.
>> it's probably too late for this series due to soft-freeze,
>> pls repost once 5.0 is released
> 
> Ah, I didn't pay enough attention to the merge window.
> 
> When will the soft-freeze be ended? Will it be announced in the mailing
> list?

You can repost about one month from now.  Thanks!

Paolo

Re: [PATCH v4 2/2] tests/bios-tables-test: add smbios cpu speed test

[Laszlo Ersek]

On 03/19/20 14:30, Igor Mammedov wrote:
> On Wed, 18 Mar 2020 14:48:20 +0800
> Heyi Guo <guoheyi@huawei.com> wrote:
> 
>> Add smbios type 4 CPU speed check for we added new options to set
>> smbios type 4 "max speed" and "current speed". The default value
>> should be 2000 when no option is specified, just as the old version
>> did.
>>
>> We add the test case to one machine of each architecture, though it
>> doesn't really run on aarch64 platform for smbios test can't run on
>> uefi only platform yet.
> I suggest to drop aarch64 part then as it just wastes resources,
> alternatively you can enable smbios tests with UEFI.
> (Not sure if uefi already exposes smbios entry point in test structure,
> CCing Lazslo)

Yes, it does. For aarch64 specifically, please see the field called
"Smbios30" in
"tests/uefi-test-tools/UefiTestToolsPkg/Include/Guid/BiosTablesTest.h".

Please see:
- <https://bugs.launchpad.net/qemu/+bug/1821884>,
- commit b097ba371a68 ("tests/uefi-test-tools: report the SMBIOS entry
point structures", 2019-05-03).

Thanks
Laszlo

> 
>>
>> Signed-off-by: Heyi Guo <guoheyi@huawei.com>
>>
>> ---
>> Cc: "Michael S. Tsirkin" <mst@redhat.com>
>> Cc: Igor Mammedov <imammedo@redhat.com>
>> Cc: Thomas Huth <thuth@redhat.com>
>> Cc: Laurent Vivier <lvivier@redhat.com>
>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>> ---
>>  tests/qtest/bios-tables-test.c | 42 ++++++++++++++++++++++++++++++++++
>>  1 file changed, 42 insertions(+)
>>
>> diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
>> index 0a597bbacf..f2d2e97b4a 100644
>> --- a/tests/qtest/bios-tables-test.c
>> +++ b/tests/qtest/bios-tables-test.c
>> @@ -77,6 +77,8 @@ typedef struct {
>>      GArray *tables;
>>      uint32_t smbios_ep_addr;
>>      struct smbios_21_entry_point smbios_ep_table;
>> +    uint16_t smbios_cpu_max_speed;
>> +    uint16_t smbios_cpu_curr_speed;
>>      uint8_t *required_struct_types;
>>      int required_struct_types_len;
>>      QTestState *qts;
>> @@ -560,6 +562,31 @@ static inline bool smbios_single_instance(uint8_t type)
>>      }
>>  }
>>  
>> +static bool smbios_cpu_test(test_data *data, uint32_t addr)
>> +{
>> +    uint16_t expect_speed[2];
>> +    uint16_t real;
>> +    int offset[2];
>> +    int i;
>> +
>> +    /* Check CPU speed for backward compatibility */
>> +    offset[0] = offsetof(struct smbios_type_4, max_speed);
>> +    offset[1] = offsetof(struct smbios_type_4, current_speed);
>> +    expect_speed[0] = data->smbios_cpu_max_speed ? : 2000;
>> +    expect_speed[1] = data->smbios_cpu_curr_speed ? : 2000;
>> +
>> +    for (i = 0; i < 2; i++) {
>> +        real = qtest_readw(data->qts, addr + offset[i]);
>> +        if (real != expect_speed[i]) {
>> +            fprintf(stderr, "Unexpected SMBIOS CPU speed: real %u expect %u\n",
>> +                    real, expect_speed[i]);
>> +            return false;
>> +        }
>> +    }
>> +
>> +    return true;
>> +}
>> +
>>  static void test_smbios_structs(test_data *data)
>>  {
>>      DECLARE_BITMAP(struct_bitmap, SMBIOS_MAX_TYPE+1) = { 0 };
>> @@ -582,6 +609,10 @@ static void test_smbios_structs(test_data *data)
>>          }
>>          set_bit(type, struct_bitmap);
>>  
>> +        if (type == 4) {
>> +            g_assert(smbios_cpu_test(data, addr));
>> +        }
>> +
>>          /* seek to end of unformatted string area of this struct ("\0\0") */
>>          prv = crt = 1;
>>          while (prv || crt) {
>> @@ -716,6 +747,11 @@ static void test_acpi_q35_tcg(void)
>>      data.required_struct_types_len = ARRAY_SIZE(base_required_struct_types);
>>      test_acpi_one(NULL, &data);
>>      free_test_data(&data);
>> +
>> +    data.smbios_cpu_max_speed = 3000;
>> +    data.smbios_cpu_curr_speed = 2600;
>> +    test_acpi_one("-smbios type=4,max-speed=3000,current-speed=2600", &data);
>> +    free_test_data(&data);
>>  }
>>  
>>  static void test_acpi_q35_tcg_bridge(void)
>> @@ -1017,6 +1053,12 @@ static void test_acpi_virt_tcg(void)
>>  
>>      test_acpi_one("-cpu cortex-a57", &data);
>>      free_test_data(&data);
>> +
>> +    data.smbios_cpu_max_speed = 2900;
>> +    data.smbios_cpu_curr_speed = 2700;
>> +    test_acpi_one("-cpu cortex-a57 "
>> +                  "-smbios type=4,max-speed=2900,current-speed=2700", &data);
>> +    free_test_data(&data);
>>  }
>>  
>>  int main(int argc, char *argv[])
>