mach-virt: Set VM's SMBIOS system version to mc->desc

PATCH Superseded Wei Huang 4 participants 7 messages Mar 22, 2018

Share & tools

Wei Huang

author

2018-03-22 03:12 UTC #0 lore raw

PATCH 1/1 mach-virt: Set VM's SMBIOS system version to mc->desc

Instead of using "1.0" as the system version of SMBIOS, we should use mc->desc for mach-virt machine type. With this patch, "dmidecode -t 1" (e.g., "-M virt-2.12,accel=kvm") will show:

Handle 0x0100, DMI type 1, 27 bytes System Information
Manufacturer: QEMU
Product Name: KVM Virtual Machine
Version: QEMU 2.12 ARM Virtual Machine Serial Number: Not Specified
...

instead of:

    Handle 0x0100, DMI type 1, 27 bytes
    System Information
            Manufacturer: QEMU
            Product Name: KVM Virtual Machine
            Version: 1.0
            Serial Number: Not Specified
            ...

Signed-off-by Wei Huang <[email protected]>

---
 hw/arm/virt.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

hw/arm/virt.c +2 -1

index 2c07245047..da7228b297 100644
@@ -1132,6 +1132,7 @@ static void *machvirt_dtb(const struct arm_boot_info *binfo, int *fdt_size)
 
 static void virt_build_smbios(VirtMachineState *vms)
 {
+    MachineClass *mc = MACHINE_GET_CLASS(vms);
     uint8_t *smbios_tables, *smbios_anchor;
     size_t smbios_tables_len, smbios_anchor_len;
     const char *product = "QEMU Virtual Machine";

@@ -1145,7 +1146,7 @@ static void virt_build_smbios(VirtMachineState *vms)
     }
 
     smbios_set_defaults("QEMU", product,
-                        "1.0", false, true, SMBIOS_ENTRY_POINT_30);
+                        mc->desc, false, true, SMBIOS_ENTRY_POINT_30);
 
     smbios_get_tables(NULL, 0, &smbios_tables, &smbios_tables_len,
                       &smbios_anchor, &smbios_anchor_len);

Signature · 1 line

2.14.3

Andrew Jones

2018-03-22 08:02 UTC #1 lore raw

Quoted 49 lines · Wei Huang

Instead of using "1.0" as the system version of SMBIOS, we should use mc->desc for mach-virt machine type. With this patch, "dmidecode -t 1" (e.g., "-M virt-2.12,accel=kvm") will show:

Handle 0x0100, DMI type 1, 27 bytes System Information
Manufacturer: QEMU
Product Name: KVM Virtual Machine
Version: QEMU 2.12 ARM Virtual Machine Serial Number: Not Specified
...

instead of:

    Handle 0x0100, DMI type 1, 27 bytes
    System Information
            Manufacturer: QEMU
            Product Name: KVM Virtual Machine
            Version: 1.0
            Serial Number: Not Specified
            ...

Signed-off-by Wei Huang <[email protected]>

---
 hw/arm/virt.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

hw/arm/virt.c +2 -1

index 2c07245047..da7228b297 100644
@@ -1132,6 +1132,7 @@ static void *machvirt_dtb(const struct arm_boot_info *binfo, int *fdt_size)
 
 static void virt_build_smbios(VirtMachineState *vms)
 {
+    MachineClass *mc = MACHINE_GET_CLASS(vms);
     uint8_t *smbios_tables, *smbios_anchor;
     size_t smbios_tables_len, smbios_anchor_len;
     const char *product = "QEMU Virtual Machine";

@@ -1145,7 +1146,7 @@ static void virt_build_smbios(VirtMachineState *vms)
     }
 
     smbios_set_defaults("QEMU", product,
-                        "1.0", false, true, SMBIOS_ENTRY_POINT_30);
+                        mc->desc, false, true, SMBIOS_ENTRY_POINT_30);
 
     smbios_get_tables(NULL, 0, &smbios_tables, &smbios_tables_len,
                       &smbios_anchor, &smbios_anchor_len);

Signature · 1 line

2.14.3

I agree we should change the useless 1.0, but shouldn't we use mc->name instead of mc->desc? mc->name would make it consistent with pc-piix and q35 and also be a less verbose "version". We'd get e.g.

System Information
Manufacturer: QEMU
Product Name: KVM Virtual Machine
Version: virt-2.12
Serial Number: Not Specified

Thanks,
drew

Peter Maydell

2018-03-22 10:54 UTC #2 lore raw

Quoted 10 lines · Andrew Jones

I agree we should change the useless 1.0, but shouldn't we use mc->name instead of mc->desc? mc->name would make it consistent with pc-piix and q35 and also be a less verbose "version". We'd get e.g.
System Information
Manufacturer: QEMU
Product Name: KVM Virtual Machine
Version: virt-2.12
Serial Number: Not Specified

I would vote for consistency with x86, I think.

thanks
-- PMM

Wei Huang

author

2018-03-22 13:48 UTC #3 lore raw

Quoted 6 lines · Peter Maydell

Quoted 5 lines · Andrew Jones
I agree we should change the useless 1.0, but shouldn't we use mc->name instead of mc->desc? mc->name would make it consistent with pc-piix and q35 and also be a less verbose "version". We'd get e.g.

Quoted 7 lines

Quoted 5 lines
System Information
Manufacturer: QEMU
Product Name: KVM Virtual Machine
Version: virt-2.12
Serial Number: Not Specified
I would vote for consistency with x86, I think.

Drew and you like ->name better because of its conciseness consistency with x86. In my view virt-2.xx is not very informative. "Version" info in real machine could be verbose. Using my Thinkpad as example:

Manufacturer: LENOVO
Product Name: 20FAS1EG0N
Version: ThinkPad T460s

With that said, I will change it to mc->name.

Quoted 4 lines

thanks
-- PMM

Daniel P. Berrangé

2018-03-22 11:07 UTC #4 lore raw

In reply to Wei Huang

Quoted 39 lines · Wei Huang

Instead of using "1.0" as the system version of SMBIOS, we should use mc->desc for mach-virt machine type. With this patch, "dmidecode -t 1" (e.g., "-M virt-2.12,accel=kvm") will show:
Handle 0x0100, DMI type 1, 27 bytes System Information
Manufacturer: QEMU
Product Name: KVM Virtual Machine
Version: QEMU 2.12 ARM Virtual Machine Serial Number: Not Specified
...
instead of:
    Handle 0x0100, DMI type 1, 27 bytes
    System Information
            Manufacturer: QEMU
            Product Name: KVM Virtual Machine
            Version: 1.0
            Serial Number: Not Specified
            ...
Signed-off-by Wei Huang <[email protected]>
---
 hw/arm/virt.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
hw/arm/virt.c +1 -0
index 2c07245047..da7228b297 100644
@@ -1132,6 +1132,7 @@ static void *machvirt_dtb(const struct arm_boot_info *binfo, int *fdt_size)
 
 static void virt_build_smbios(VirtMachineState *vms)
 {
+    MachineClass *mc = MACHINE_GET_CLASS(vms);
     uint8_t *smbios_tables, *smbios_anchor;
     size_t smbios_tables_len, smbios_anchor_len;
     const char *product = "QEMU Virtual Machine";

hw/arm/virt.c static void virt_build_smbios(VirtMachineState *vms) · line 1149

Quoted 5 lines

hw/arm/virt.c +1 -1

@@ -1145,7 +1146,7 @@ static void virt_build_smbios(VirtMachineState *vms)
     }
 
     smbios_set_defaults("QEMU", product,
-                        "1.0", false, true, SMBIOS_ENTRY_POINT_30);
+                        mc->desc, false, true, SMBIOS_ENTRY_POINT_30);

Surely this needs to be handled in back compatible manner so that existing machine types are not changed - only the new virt-2.12 machine type should get new format.

Regards,
Daniel

Signature · 3 lines

|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Andrew Jones

2018-03-22 13:49 UTC #5 lore raw

Quoted 42 lines · Daniel P. Berrangé

Quoted 41 lines · Wei Huang
Instead of using "1.0" as the system version of SMBIOS, we should use mc->desc for mach-virt machine type. With this patch, "dmidecode -t 1" (e.g., "-M virt-2.12,accel=kvm") will show:
Handle 0x0100, DMI type 1, 27 bytes System Information
Manufacturer: QEMU
Product Name: KVM Virtual Machine
Version: QEMU 2.12 ARM Virtual Machine Serial Number: Not Specified
...
instead of:
    Handle 0x0100, DMI type 1, 27 bytes
    System Information
            Manufacturer: QEMU
            Product Name: KVM Virtual Machine
            Version: 1.0
            Serial Number: Not Specified
            ...
Signed-off-by Wei Huang <[email protected]>
---
 hw/arm/virt.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
hw/arm/virt.c +1 -0
index 2c07245047..da7228b297 100644
@@ -1132,6 +1132,7 @@ static void *machvirt_dtb(const struct arm_boot_info *binfo, int *fdt_size)
 
 static void virt_build_smbios(VirtMachineState *vms)
 {
+    MachineClass *mc = MACHINE_GET_CLASS(vms);
     uint8_t *smbios_tables, *smbios_anchor;
     size_t smbios_tables_len, smbios_anchor_len;
     const char *product = "QEMU Virtual Machine";
@@ -1145,7 +1146,7 @@ static void virt_build_smbios(VirtMachineState *vms)
     }
 

Quoted 7 lines

hw/arm/virt.c static void virt_build_smbios(VirtMachineState *vms) · line 1149
Quoted 3 lines
hw/arm/virt.c +1 -1
     smbios_set_defaults("QEMU", product,
-                        "1.0", false, true, SMBIOS_ENTRY_POINT_30);
+                        mc->desc, false, true, SMBIOS_ENTRY_POINT_30);
Surely this needs to be handled in back compatible manner so that existing machine types are not changed - only the new virt-2.12 machine type should get new format.

On real hardware this table can change when there's a firmware update, so I don't think any OS would expect it to stay the same across reboots. I also can't imagine any applications caring at all about this, particularly because it's currently the useless "1.0" string. I think we're safe to neglect compat code in this case, but I may be missing something. Is there a particular issue you're aware of that we'd risk hitting? (Well, besides Windows detecting its installation machine has "changed", invalidating its license or whatever. We don't yet have any Windows guests to worry about.)

Thanks,
drew

Daniel P. Berrangé

2018-03-22 13:55 UTC #6 lore raw

Quoted 51 lines · Andrew Jones

Quoted 42 lines · Daniel P. Berrangé
Quoted 41 lines · Wei Huang
Instead of using "1.0" as the system version of SMBIOS, we should use mc->desc for mach-virt machine type. With this patch, "dmidecode -t 1" (e.g., "-M virt-2.12,accel=kvm") will show:
Handle 0x0100, DMI type 1, 27 bytes System Information
Manufacturer: QEMU
Product Name: KVM Virtual Machine
Version: QEMU 2.12 ARM Virtual Machine Serial Number: Not Specified
...
instead of:
    Handle 0x0100, DMI type 1, 27 bytes
    System Information
            Manufacturer: QEMU
            Product Name: KVM Virtual Machine
            Version: 1.0
            Serial Number: Not Specified
            ...
Signed-off-by Wei Huang <[email protected]>
---
 hw/arm/virt.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
hw/arm/virt.c +1 -0
index 2c07245047..da7228b297 100644
@@ -1132,6 +1132,7 @@ static void *machvirt_dtb(const struct arm_boot_info *binfo, int *fdt_size)
 
 static void virt_build_smbios(VirtMachineState *vms)
 {
+    MachineClass *mc = MACHINE_GET_CLASS(vms);
     uint8_t *smbios_tables, *smbios_anchor;
     size_t smbios_tables_len, smbios_anchor_len;
     const char *product = "QEMU Virtual Machine";
@@ -1145,7 +1146,7 @@ static void virt_build_smbios(VirtMachineState *vms)
     }
 
Quoted 7 lines
hw/arm/virt.c static void virt_build_smbios(VirtMachineState *vms) · line 1149
Quoted 3 lines
hw/arm/virt.c +1 -1
     smbios_set_defaults("QEMU", product,
-                        "1.0", false, true, SMBIOS_ENTRY_POINT_30);
+                        mc->desc, false, true, SMBIOS_ENTRY_POINT_30);
Surely this needs to be handled in back compatible manner so that existing machine types are not changed - only the new virt-2.12 machine type should get new format.

Quoted 9 lines

On real hardware this table can change when there's a firmware update, so I don't think any OS would expect it to stay the same across reboots. I also can't imagine any applications caring at all about this, particularly because it's currently the useless "1.0" string. I think we're safe to neglect compat code in this case, but I may be missing something. Is there a particular issue you're aware of that we'd risk hitting? (Well, besides Windows detecting its installation machine has "changed", invalidating its license or whatever. We don't yet have any Windows guests to worry about.)

I don't have a particular scenario in mind - just the general rule that we don't change something which is guest visible without tieing it to machine type. I guess we've not tied machine types to specific SeaBIOS / UEFI snapshot builds though, so in terms of firmware changing we've already got that scenario.

Regards,
Daniel

Signature · 3 lines

|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|