All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH RESEND 0/2 V2] Virtual Machine Generation ID
@ 2014-09-14  6:25 Gal Hammer
  2014-09-14  6:25 ` [Qemu-devel] [PATCH 1/2] i386: Add an ACPI_EXTRACT_NAME_BUFFER16 directive Gal Hammer
                   ` (2 more replies)
  0 siblings, 3 replies; 28+ messages in thread
From: Gal Hammer @ 2014-09-14  6:25 UTC (permalink / raw)
  To: qemu-devel; +Cc: Gal Hammer, pbonzini, armbru

Hi,

A two parts patch to add a QEmu support for Microsoft's Virtual Machine
Generation ID device.

The first one add a new ACPI directive which allow to use a 16-bytes
buffer in an ACPI table. This buffer is for storing the VM's UUID.

The second is the ACPI tables changes and the actual device.

Your comment are welcomed.

Thanks,

    Gal.

V2: - Remove "-uuid" command line parameter.
    - Move device's description from SSDT to DDST table.
    - Add new "vmgenid" sysbus device.

Gal Hammer (2):
  i386: Add an ACPI_EXTRACT_NAME_BUFFER16 directive.
  i386: Add a Virtual Machine Generation ID device.

 default-configs/i386-softmmu.mak   |  1 +
 default-configs/x86_64-softmmu.mak |  1 +
 hw/i386/acpi-build.c               | 23 ++++++++++-
 hw/i386/acpi-dsdt.dsl              | 37 +++++++++++++++++
 hw/misc/Makefile.objs              |  1 +
 hw/misc/vmgenid.c                  | 85 ++++++++++++++++++++++++++++++++++++++
 include/hw/i386/pc.h               |  3 ++
 scripts/acpi_extract.py            | 23 +++++++----
 8 files changed, 163 insertions(+), 11 deletions(-)
 create mode 100644 hw/misc/vmgenid.c

-- 
1.9.3

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [Qemu-devel] [PATCH 1/2] i386: Add an ACPI_EXTRACT_NAME_BUFFER16 directive.
  2014-09-14  6:25 [Qemu-devel] [PATCH RESEND 0/2 V2] Virtual Machine Generation ID Gal Hammer
@ 2014-09-14  6:25 ` Gal Hammer
  2014-09-14  8:48   ` Paolo Bonzini
  2014-09-14  6:25 ` [Qemu-devel] [PATCH 2/2] i386: Add a Virtual Machine Generation ID device Gal Hammer
  2014-09-14  8:48 ` [Qemu-devel] [PATCH RESEND 0/2 V2] Virtual Machine Generation ID Paolo Bonzini
  2 siblings, 1 reply; 28+ messages in thread
From: Gal Hammer @ 2014-09-14  6:25 UTC (permalink / raw)
  To: qemu-devel; +Cc: Gal Hammer, pbonzini, armbru

Add a 16-bytes buffer to allow storing a 128-bit UUID value in an
ACPI table.

Signed-off-by: Gal Hammer <ghammer@redhat.com>
---
 scripts/acpi_extract.py | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/scripts/acpi_extract.py b/scripts/acpi_extract.py
index 22ea468..88314fc 100755
--- a/scripts/acpi_extract.py
+++ b/scripts/acpi_extract.py
@@ -139,13 +139,16 @@ def aml_name_string(offset):
         offset += 1
     return offset;
 
-# Given data offset, find 8 byte buffer offset
-def aml_data_buffer8(offset):
-    #0x08 NameOp NameString DataRef
-    expect = [0x11, 0x0B, 0x0A, 0x08]
+# Given data offset, find variable length byte buffer offset
+def aml_data_buffer(offset, length):
+    #0x11 PkgLength BufferSize ByteList
+    if (length > 63):
+        die( "Name offset 0x%x: expected an one byte PkgLength (length<=63)" %
+             (offset));
+    expect = [0x11, length+3, 0x0A, length]
     if (aml[offset:offset+4] != expect):
         die( "Name offset 0x%x: expected %s actual %s" %
-             (offset, aml[offset:offset+4], expect))
+             (offset, expect, aml[offset:offset+4]))
     return offset + len(expect)
 
 # Given data offset, find dword const offset
@@ -172,9 +175,9 @@ def aml_data_byte_const(offset):
              (offset, aml[offset]));
     return offset + 1;
 
-# Find name'd buffer8
-def aml_name_buffer8(offset):
-    return aml_data_buffer8(aml_name_string(offset) + 4)
+# Find name'd buffer
+def aml_name_buffer(offset, length):
+    return aml_data_buffer(aml_name_string(offset) + 4, length)
 
 # Given name offset, find dword const offset
 def aml_name_dword_const(offset):
@@ -308,7 +311,9 @@ for i in range(len(asl)):
         output[array] = aml
         continue
     if (directive == "ACPI_EXTRACT_NAME_BUFFER8"):
-        offset = aml_name_buffer8(offset)
+        offset = aml_name_buffer(offset, 8)
+    elif (directive == "ACPI_EXTRACT_NAME_BUFFER16"):
+        offset = aml_name_buffer(offset, 16)
     elif (directive == "ACPI_EXTRACT_NAME_DWORD_CONST"):
         offset = aml_name_dword_const(offset)
     elif (directive == "ACPI_EXTRACT_NAME_WORD_CONST"):
-- 
1.9.3

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [Qemu-devel] [PATCH 2/2] i386: Add a Virtual Machine Generation ID device.
  2014-09-14  6:25 [Qemu-devel] [PATCH RESEND 0/2 V2] Virtual Machine Generation ID Gal Hammer
  2014-09-14  6:25 ` [Qemu-devel] [PATCH 1/2] i386: Add an ACPI_EXTRACT_NAME_BUFFER16 directive Gal Hammer
@ 2014-09-14  6:25 ` Gal Hammer
  2014-09-14  8:48 ` [Qemu-devel] [PATCH RESEND 0/2 V2] Virtual Machine Generation ID Paolo Bonzini
  2 siblings, 0 replies; 28+ messages in thread
From: Gal Hammer @ 2014-09-14  6:25 UTC (permalink / raw)
  To: qemu-devel; +Cc: Gal Hammer, pbonzini, armbru

Based on Microsoft's sepecifications (paper can be dowloaded from
http://go.microsoft.com/fwlink/?LinkId=260709), add a device
description to the DSDT ACPI table.

The GUID is set using a new "vmgenid" device.

Signed-off-by: Gal Hammer <ghammer@redhat.com>

---
 default-configs/i386-softmmu.mak   |  1 +
 default-configs/x86_64-softmmu.mak |  1 +
 hw/i386/acpi-build.c               | 23 ++++++++++-
 hw/i386/acpi-dsdt.dsl              | 37 +++++++++++++++++
 hw/misc/Makefile.objs              |  1 +
 hw/misc/vmgenid.c                  | 85 ++++++++++++++++++++++++++++++++++++++
 include/hw/i386/pc.h               |  3 ++
 7 files changed, 149 insertions(+), 2 deletions(-)
 create mode 100644 hw/misc/vmgenid.c

diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak
index 8e08841..bd33c75 100644
--- a/default-configs/i386-softmmu.mak
+++ b/default-configs/i386-softmmu.mak
@@ -45,3 +45,4 @@ CONFIG_IOAPIC=y
 CONFIG_ICC_BUS=y
 CONFIG_PVPANIC=y
 CONFIG_MEM_HOTPLUG=y
+CONFIG_VMGENID=y
diff --git a/default-configs/x86_64-softmmu.mak b/default-configs/x86_64-softmmu.mak
index 66557ac..006fc7c 100644
--- a/default-configs/x86_64-softmmu.mak
+++ b/default-configs/x86_64-softmmu.mak
@@ -45,3 +45,4 @@ CONFIG_IOAPIC=y
 CONFIG_ICC_BUS=y
 CONFIG_PVPANIC=y
 CONFIG_MEM_HOTPLUG=y
+CONFIG_VMGENID=y
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 85e5834..153dadc 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1390,15 +1390,34 @@ build_mcfg_q35(GArray *table_data, GArray *linker, AcpiMcfgInfo *info)
 static void
 build_dsdt(GArray *table_data, GArray *linker, AcpiMiscInfo *misc)
 {
-    AcpiTableHeader *dsdt;
+    int dsdt_start = table_data->len;
+    uint8_t *dsdt;
+    uint8_t *vm_gid_ptr;
+    uint32_t vm_gid_physical_address;
 
     assert(misc->dsdt_code && misc->dsdt_size);
 
     dsdt = acpi_data_push(table_data, misc->dsdt_size);
     memcpy(dsdt, misc->dsdt_code, misc->dsdt_size);
 
+    vm_gid_ptr = acpi_data_get_ptr(dsdt, misc->dsdt_size,
+                                   *dsdt_acpi_vm_gid, 16);
+    if (vm_generation_id(vm_gid_ptr)) {
+        vm_gid_physical_address = dsdt_start + *dsdt_acpi_vm_gid;
+        bios_linker_loader_add_pointer(linker, ACPI_BUILD_TABLE_FILE,
+                                       ACPI_BUILD_TABLE_FILE,
+                                       table_data,
+                                       dsdt + *dsdt_acpi_vm_gid_addr,
+                                       sizeof(uint32_t));
+    } else {
+        vm_gid_physical_address = 0;
+    }
+
+    ACPI_BUILD_SET_LE(dsdt, misc->dsdt_size,
+                      *dsdt_acpi_vm_gid_addr, 32, vm_gid_physical_address);
+
     memset(dsdt, 0, sizeof *dsdt);
-    build_header(linker, table_data, dsdt, "DSDT",
+    build_header(linker, table_data, (AcpiTableHeader *)dsdt, "DSDT",
                  misc->dsdt_size, 1);
 }
 
diff --git a/hw/i386/acpi-dsdt.dsl b/hw/i386/acpi-dsdt.dsl
index 559f4b6..9015881 100644
--- a/hw/i386/acpi-dsdt.dsl
+++ b/hw/i386/acpi-dsdt.dsl
@@ -294,6 +294,43 @@ DefinitionBlock (
         }
     }
 
+/****************************************************************
+ * Virtual Machine Generation ID Device
+ ****************************************************************/
+    Scope(\_SB) {
+
+        Device(VMGI) {
+            Name(_HID, "QEMU0002")
+            Name(_CID, "VM_Gen_Counter")
+            Name(_DDN, "VM_Gen_Counter")
+
+            ACPI_EXTRACT_NAME_DWORD_CONST dsdt_acpi_vm_gid_addr
+            Name(VGIA, 0x12345678)
+
+            ACPI_EXTRACT_NAME_BUFFER16 dsdt_acpi_vm_gid
+            Name(VGID, Buffer(16) {
+                0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+                0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 })
+
+            Method(_STA, 0, NotSerialized) {
+                Store(VGIA, Local0)
+                If (LEqual(Local0, Zero)) {
+                    Return (0x00)
+                } Else {
+                    Return (0x0F)
+                }
+            }
+
+            Method(ADDR, 0, Serialized) {
+                Store(Package(2) { }, Local0)
+                Store(VGIA, Index(Local0, 0))
+                Store(0x0000, Index(Local0, 1))
+                return (Local0)
+            }
+        }
+    }
+
+
 #include "hw/acpi/pc-hotplug.h"
 #define CPU_STATUS_BASE PIIX4_CPU_HOTPLUG_IO_BASE
 #include "acpi-dsdt-cpu-hotplug.dsl"
diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
index 979e532..c18b800 100644
--- a/hw/misc/Makefile.objs
+++ b/hw/misc/Makefile.objs
@@ -41,3 +41,4 @@ obj-$(CONFIG_SLAVIO) += slavio_misc.o
 obj-$(CONFIG_ZYNQ) += zynq_slcr.o
 
 obj-$(CONFIG_PVPANIC) += pvpanic.o
+obj-$(CONFIG_VMGENID) += vmgenid.o
diff --git a/hw/misc/vmgenid.c b/hw/misc/vmgenid.c
new file mode 100644
index 0000000..76956d1
--- /dev/null
+++ b/hw/misc/vmgenid.c
@@ -0,0 +1,85 @@
+/*
+ *  Virtual Machine Generation ID Device
+ *
+ *  Copyright (C) 2014 Red Hat Inc.
+ *
+ *  Authors: Gal Hammer <ghammer@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#include "hw/i386/pc.h"
+#include "hw/sysbus.h"
+
+#define VMGENID_DEVICE "vmgenid"
+
+#define PROPERTY_UUID "uuid"
+
+#define VMGENID(obj) OBJECT_CHECK(VmGenIdState, (obj), VMGENID_DEVICE)
+
+typedef struct VmGenIdState {
+    SysBusDevice parent_obj;
+    char *guid_arg;
+} VmGenIdState;
+
+bool vm_generation_id(uint8_t id[16])
+{
+    Object *o = object_resolve_path_type("", VMGENID_DEVICE, NULL);
+    char *guid;
+
+    if (!o) {
+        return false;
+    }
+    guid = object_property_get_str(o, PROPERTY_UUID, NULL);
+    /* actual uuid validation was checked during realize. */
+    (void)qemu_uuid_parse(guid, id);
+    return true;
+}
+
+static void vmgenid_realize(DeviceState *dev, Error **errp)
+{
+    VmGenIdState *s = VMGENID(dev);
+    uint8_t id[16];
+
+    if (!s->guid_arg) {
+        error_setg(errp, "missing uuid.");
+        return;
+    }
+
+    if (qemu_uuid_parse(s->guid_arg, id) < 0) {
+        error_setg(errp, "Fail to parse UUID string.");
+        return;
+    }
+}
+
+static Property vmgenid_device_properties[] = {
+    DEFINE_PROP_STRING(PROPERTY_UUID, VmGenIdState, guid_arg),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void vmgenid_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->realize = vmgenid_realize;
+    dc->props = vmgenid_device_properties;
+    dc->cannot_instantiate_with_device_add_yet = false;
+    set_bit(DEVICE_CATEGORY_MISC, dc->categories);
+}
+
+static const TypeInfo vmgenid_device_info = {
+    .name          = VMGENID_DEVICE,
+    .parent        = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(VmGenIdState),
+    .class_init    = vmgenid_class_init,
+};
+
+static void vmgenid_register_types(void)
+{
+    type_register_static(&vmgenid_device_info);
+}
+
+type_init(vmgenid_register_types)
+
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index a39cb42..b21c22e 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -290,6 +290,9 @@ void pc_system_firmware_init(MemoryRegion *rom_memory,
 /* pvpanic.c */
 uint16_t pvpanic_port(void);
 
+/* vmgenid.c */
+bool vm_generation_id(uint8_t id[16]);
+
 /* e820 types */
 #define E820_RAM        1
 #define E820_RESERVED   2
-- 
1.9.3

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [Qemu-devel] [PATCH RESEND 0/2 V2] Virtual Machine Generation ID
  2014-09-14  6:25 [Qemu-devel] [PATCH RESEND 0/2 V2] Virtual Machine Generation ID Gal Hammer
  2014-09-14  6:25 ` [Qemu-devel] [PATCH 1/2] i386: Add an ACPI_EXTRACT_NAME_BUFFER16 directive Gal Hammer
  2014-09-14  6:25 ` [Qemu-devel] [PATCH 2/2] i386: Add a Virtual Machine Generation ID device Gal Hammer
@ 2014-09-14  8:48 ` Paolo Bonzini
  2014-09-14 10:51   ` Gal Hammer
  2 siblings, 1 reply; 28+ messages in thread
From: Paolo Bonzini @ 2014-09-14  8:48 UTC (permalink / raw)
  To: Gal Hammer, qemu-devel; +Cc: armbru

Il 14/09/2014 08:25, Gal Hammer ha scritto:
> Hi,
> 
> A two parts patch to add a QEmu support for Microsoft's Virtual Machine
> Generation ID device.
> 
> The first one add a new ACPI directive which allow to use a 16-bytes
> buffer in an ACPI table. This buffer is for storing the VM's UUID.
> 
> The second is the ACPI tables changes and the actual device.
> 
> Your comment are welcomed.
> 
> Thanks,
> 
>     Gal.
> 
> V2: - Remove "-uuid" command line parameter.
>     - Move device's description from SSDT to DDST table.
>     - Add new "vmgenid" sysbus device.
> 
> Gal Hammer (2):
>   i386: Add an ACPI_EXTRACT_NAME_BUFFER16 directive.
>   i386: Add a Virtual Machine Generation ID device.
> 
>  default-configs/i386-softmmu.mak   |  1 +
>  default-configs/x86_64-softmmu.mak |  1 +
>  hw/i386/acpi-build.c               | 23 ++++++++++-
>  hw/i386/acpi-dsdt.dsl              | 37 +++++++++++++++++
>  hw/misc/Makefile.objs              |  1 +
>  hw/misc/vmgenid.c                  | 85 ++++++++++++++++++++++++++++++++++++++
>  include/hw/i386/pc.h               |  3 ++
>  scripts/acpi_extract.py            | 23 +++++++----
>  8 files changed, 163 insertions(+), 11 deletions(-)
>  create mode 100644 hw/misc/vmgenid.c
> 

The idea is okay, but the device has to be added to the Q35 DSDT as
well.  If you just #included a file in the same style as
acpi-dsdt-hpet.dsl, you would have a namespace clash between the PIIX
and Q35 DSDTs.  acpi-dsdt-hpet.dsl can do it because it has no
ACPI_EXTRACT directives.

I think because of this, it is a bit simpler to add the device
optionally to the SSDT, similar to how the TPM is handled.

Paolo

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [Qemu-devel] [PATCH 1/2] i386: Add an ACPI_EXTRACT_NAME_BUFFER16 directive.
  2014-09-14  6:25 ` [Qemu-devel] [PATCH 1/2] i386: Add an ACPI_EXTRACT_NAME_BUFFER16 directive Gal Hammer
@ 2014-09-14  8:48   ` Paolo Bonzini
  0 siblings, 0 replies; 28+ messages in thread
From: Paolo Bonzini @ 2014-09-14  8:48 UTC (permalink / raw)
  To: Gal Hammer, qemu-devel; +Cc: armbru

Il 14/09/2014 08:25, Gal Hammer ha scritto:
> Add a 16-bytes buffer to allow storing a 128-bit UUID value in an
> ACPI table.
> 
> Signed-off-by: Gal Hammer <ghammer@redhat.com>
> ---
>  scripts/acpi_extract.py | 23 ++++++++++++++---------
>  1 file changed, 14 insertions(+), 9 deletions(-)
> 
> diff --git a/scripts/acpi_extract.py b/scripts/acpi_extract.py
> index 22ea468..88314fc 100755
> --- a/scripts/acpi_extract.py
> +++ b/scripts/acpi_extract.py
> @@ -139,13 +139,16 @@ def aml_name_string(offset):
>          offset += 1
>      return offset;
>  
> -# Given data offset, find 8 byte buffer offset
> -def aml_data_buffer8(offset):
> -    #0x08 NameOp NameString DataRef
> -    expect = [0x11, 0x0B, 0x0A, 0x08]
> +# Given data offset, find variable length byte buffer offset
> +def aml_data_buffer(offset, length):
> +    #0x11 PkgLength BufferSize ByteList
> +    if (length > 63):
> +        die( "Name offset 0x%x: expected an one byte PkgLength (length<=63)" %

s/an/a/

> +             (offset));
> +    expect = [0x11, length+3, 0x0A, length]
>      if (aml[offset:offset+4] != expect):
>          die( "Name offset 0x%x: expected %s actual %s" %
> -             (offset, aml[offset:offset+4], expect))
> +             (offset, expect, aml[offset:offset+4]))
>      return offset + len(expect)
>  
>  # Given data offset, find dword const offset
> @@ -172,9 +175,9 @@ def aml_data_byte_const(offset):
>               (offset, aml[offset]));
>      return offset + 1;
>  
> -# Find name'd buffer8
> -def aml_name_buffer8(offset):
> -    return aml_data_buffer8(aml_name_string(offset) + 4)
> +# Find name'd buffer
> +def aml_name_buffer(offset, length):
> +    return aml_data_buffer(aml_name_string(offset) + 4, length)
>  
>  # Given name offset, find dword const offset
>  def aml_name_dword_const(offset):
> @@ -308,7 +311,9 @@ for i in range(len(asl)):
>          output[array] = aml
>          continue
>      if (directive == "ACPI_EXTRACT_NAME_BUFFER8"):
> -        offset = aml_name_buffer8(offset)
> +        offset = aml_name_buffer(offset, 8)
> +    elif (directive == "ACPI_EXTRACT_NAME_BUFFER16"):
> +        offset = aml_name_buffer(offset, 16)
>      elif (directive == "ACPI_EXTRACT_NAME_DWORD_CONST"):
>          offset = aml_name_dword_const(offset)
>      elif (directive == "ACPI_EXTRACT_NAME_WORD_CONST"):
> 

Apart from the above grammatical nit,

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Paolo

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [Qemu-devel] [PATCH RESEND 0/2 V2] Virtual Machine Generation ID
  2014-09-14  8:48 ` [Qemu-devel] [PATCH RESEND 0/2 V2] Virtual Machine Generation ID Paolo Bonzini
@ 2014-09-14 10:51   ` Gal Hammer
  2014-09-14 13:40     ` Paolo Bonzini
  0 siblings, 1 reply; 28+ messages in thread
From: Gal Hammer @ 2014-09-14 10:51 UTC (permalink / raw)
  To: Paolo Bonzini, qemu-devel; +Cc: armbru

On 14/09/2014 11:48, Paolo Bonzini wrote:
> Il 14/09/2014 08:25, Gal Hammer ha scritto:
>> Hi,
>>
>> A two parts patch to add a QEmu support for Microsoft's Virtual Machine
>> Generation ID device.
>>
>> The first one add a new ACPI directive which allow to use a 16-bytes
>> buffer in an ACPI table. This buffer is for storing the VM's UUID.
>>
>> The second is the ACPI tables changes and the actual device.
>>
>> Your comment are welcomed.
>>
>> Thanks,
>>
>>      Gal.
>>
>> V2: - Remove "-uuid" command line parameter.
>>      - Move device's description from SSDT to DDST table.
>>      - Add new "vmgenid" sysbus device.
>>
>> Gal Hammer (2):
>>    i386: Add an ACPI_EXTRACT_NAME_BUFFER16 directive.
>>    i386: Add a Virtual Machine Generation ID device.
>>
>>   default-configs/i386-softmmu.mak   |  1 +
>>   default-configs/x86_64-softmmu.mak |  1 +
>>   hw/i386/acpi-build.c               | 23 ++++++++++-
>>   hw/i386/acpi-dsdt.dsl              | 37 +++++++++++++++++
>>   hw/misc/Makefile.objs              |  1 +
>>   hw/misc/vmgenid.c                  | 85 ++++++++++++++++++++++++++++++++++++++
>>   include/hw/i386/pc.h               |  3 ++
>>   scripts/acpi_extract.py            | 23 +++++++----
>>   8 files changed, 163 insertions(+), 11 deletions(-)
>>   create mode 100644 hw/misc/vmgenid.c
>>
>
> The idea is okay, but the device has to be added to the Q35 DSDT as
> well.  If you just #included a file in the same style as
> acpi-dsdt-hpet.dsl, you would have a namespace clash between the PIIX
> and Q35 DSDTs.  acpi-dsdt-hpet.dsl can do it because it has no
> ACPI_EXTRACT directives.
>
> I think because of this, it is a bit simpler to add the device
> optionally to the SSDT, similar to how the TPM is handled.

Back to SSDT? Didn't you say something about migration issues that 
prevent me from using SSDT because the table's data is modified?

>
> Paolo
>

     Gal.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [Qemu-devel] [PATCH RESEND 0/2 V2] Virtual Machine Generation ID
  2014-09-14 10:51   ` Gal Hammer
@ 2014-09-14 13:40     ` Paolo Bonzini
  0 siblings, 0 replies; 28+ messages in thread
From: Paolo Bonzini @ 2014-09-14 13:40 UTC (permalink / raw)
  To: Gal Hammer, qemu-devel; +Cc: armbru

Il 14/09/2014 12:51, Gal Hammer ha scritto:
>>
>> I think because of this, it is a bit simpler to add the device
>> optionally to the SSDT, similar to how the TPM is handled.
> 
> Back to SSDT? Didn't you say something about migration issues that
> prevent me from using SSDT because the table's data is modified?

I said you have to either:

1) Use the DSDT, with a dynamic _STA method.

2) Use the SSDT, and only add the device if the device is specified on
the command line.  This avoids the avoid migration issues.  At this
point the _STA method can be static.

Using the DSDT is possible, it's just that you have to do it as well for
Q35, and I think it is harder than moving it back to the SSDT.  Sorry
for not thinking earlier about the different chipsets.

Paolo

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [Qemu-devel] [PATCH 2/2] i386: Add a Virtual Machine Generation ID device.
  2014-10-02 13:14     ` Gal Hammer
@ 2014-10-02 13:30       ` Michael S. Tsirkin
  0 siblings, 0 replies; 28+ messages in thread
From: Michael S. Tsirkin @ 2014-10-02 13:30 UTC (permalink / raw)
  To: Gal Hammer; +Cc: Paolo Bonzini, qemu-devel, Igor Mammedov

On Thu, Oct 02, 2014 at 04:14:05PM +0300, Gal Hammer wrote:
> On 02/10/2014 15:49, Michael S. Tsirkin wrote:
> >On Wed, Sep 17, 2014 at 02:39:52PM +0300, Gal Hammer wrote:
> >>Based on Microsoft's sepecifications (paper can be dowloaded from
> >>http://go.microsoft.com/fwlink/?LinkId=260709), add a device
> >>description to the SSDT ACPI table.
> >>
> >>The GUID is set using a new "vmgenid" device.
> >>
> >>Signed-off-by: Gal Hammer <ghammer@redhat.com>
> >>Reviewed-By: Igor Mammedov <imammedo@redhat.com>
> >>---
> >>  default-configs/i386-softmmu.mak   |   1 +
> >>  default-configs/x86_64-softmmu.mak |   1 +
> >>  hw/i386/Makefile.objs              |   2 +-
> >>  hw/i386/acpi-build.c               |  39 +++++++
> >>  hw/i386/ssdt-vmgenid.dsl           |  63 ++++++++++++
> >>  hw/i386/ssdt-vmgenid.hex.generated | 206 +++++++++++++++++++++++++++++++++++++
> >>  hw/misc/Makefile.objs              |   1 +
> >>  hw/misc/vmgenid.c                  |  84 +++++++++++++++
> >>  include/hw/i386/pc.h               |   3 +
> >
> >Patch scripts/update-acpi.sh as well please.
> 
> I understand what should be changed there. The script just copy all the
> *.hex files to *.hex.generated.
> 
> >>  9 files changed, 399 insertions(+), 1 deletion(-)
> >>  create mode 100644 hw/i386/ssdt-vmgenid.dsl
> >>  create mode 100644 hw/i386/ssdt-vmgenid.hex.generated
> >>  create mode 100644 hw/misc/vmgenid.c
> >>
> >>diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak
> >>index 8e08841..bd33c75 100644
> >>--- a/default-configs/i386-softmmu.mak
> >>+++ b/default-configs/i386-softmmu.mak
> >>@@ -45,3 +45,4 @@ CONFIG_IOAPIC=y
> >>  CONFIG_ICC_BUS=y
> >>  CONFIG_PVPANIC=y
> >>  CONFIG_MEM_HOTPLUG=y
> >>+CONFIG_VMGENID=y
> >>diff --git a/default-configs/x86_64-softmmu.mak b/default-configs/x86_64-softmmu.mak
> >>index 66557ac..006fc7c 100644
> >>--- a/default-configs/x86_64-softmmu.mak
> >>+++ b/default-configs/x86_64-softmmu.mak
> >>@@ -45,3 +45,4 @@ CONFIG_IOAPIC=y
> >>  CONFIG_ICC_BUS=y
> >>  CONFIG_PVPANIC=y
> >>  CONFIG_MEM_HOTPLUG=y
> >>+CONFIG_VMGENID=y
> >>diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs
> >>index 9d419ad..cd1beb3 100644
> >>--- a/hw/i386/Makefile.objs
> >>+++ b/hw/i386/Makefile.objs
> >>@@ -12,7 +12,7 @@ hw/i386/acpi-build.o: hw/i386/acpi-build.c hw/i386/acpi-dsdt.hex \
> >>  	hw/i386/ssdt-proc.hex hw/i386/ssdt-pcihp.hex hw/i386/ssdt-misc.hex \
> >>  	hw/i386/acpi-dsdt.hex hw/i386/q35-acpi-dsdt.hex \
> >>  	hw/i386/q35-acpi-dsdt.hex hw/i386/ssdt-mem.hex \
> >>-	hw/i386/ssdt-tpm.hex
> >>+	hw/i386/ssdt-tpm.hex hw/i386/ssdt-vmgenid.hex
> >>
> >>  iasl-option=$(shell if test -z "`$(1) $(2) 2>&1 > /dev/null`" \
> >>      ; then echo "$(2)"; else echo "$(3)"; fi ;)
> >>diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> >>index a313321..72d5a88 100644
> >>--- a/hw/i386/acpi-build.c
> >>+++ b/hw/i386/acpi-build.c
> >>@@ -96,6 +96,8 @@ typedef struct AcpiMiscInfo {
> >>      const unsigned char *dsdt_code;
> >>      unsigned dsdt_size;
> >>      uint16_t pvpanic_port;
> >>+    bool vm_generation_id_set;
> >>+    uint8_t vm_generation_id[16];
> >>  } AcpiMiscInfo;
> >>
> >>  typedef struct AcpiBuildPciBusHotplugState {
> >>@@ -216,6 +218,7 @@ static void acpi_get_misc_info(AcpiMiscInfo *info)
> >>      info->has_hpet = hpet_find();
> >>      info->has_tpm = tpm_find();
> >>      info->pvpanic_port = pvpanic_port();
> >>+    info->vm_generation_id_set = vm_generation_id(info->vm_generation_id);
> >>  }
> >>
> >>  static void acpi_get_pci_info(PcPciInfo *info)
> >>@@ -710,6 +713,7 @@ static inline char acpi_get_hex(uint32_t val)
> >>  #include "hw/i386/ssdt-misc.hex"
> >>  #include "hw/i386/ssdt-pcihp.hex"
> >>  #include "hw/i386/ssdt-tpm.hex"
> >>+#include "hw/i386/ssdt-vmgenid.hex"
> >>
> >>  static void
> >>  build_append_notify_method(GArray *device, const char *name,
> >>@@ -1246,6 +1250,37 @@ build_tpm_ssdt(GArray *table_data, GArray *linker)
> >>      memcpy(tpm_ptr, ssdt_tpm_aml, sizeof(ssdt_tpm_aml));
> >>  }
> >>
> >>+static void
> >>+build_vmgenid_ssdt(GArray *table_data, GArray *linker, AcpiMiscInfo *info)
> >>+{
> >>+    int vgid_start = table_data->len;
> >>+    void *vgid_ptr;
> >>+    uint8_t *vm_gid_ptr;
> >>+    uint32_t vm_gid_physical_address;
> >>+
> >>+    vgid_ptr = acpi_data_push(table_data, sizeof(ssdt_vmgenid_aml));
> >>+    memcpy(vgid_ptr, ssdt_vmgenid_aml, sizeof(ssdt_vmgenid_aml));
> >>+
> >>+    vm_gid_ptr = acpi_data_get_ptr(vgid_ptr, sizeof(ssdt_vmgenid_aml),
> >>+                                   *ssdt_acpi_vm_gid,
> >>+                                   sizeof(info->vm_generation_id));
> >>+    memcpy(vm_gid_ptr, info->vm_generation_id,
> >>+           sizeof(info->vm_generation_id));
> >>+
> >>+    bios_linker_loader_add_pointer(linker, ACPI_BUILD_TABLE_FILE,
> >>+                                   ACPI_BUILD_TABLE_FILE,
> >>+                                   table_data,
> >>+                                   vgid_ptr + *ssdt_acpi_vm_gid_addr,
> >>+                                   sizeof(uint32_t));
> >>+
> >>+    vm_gid_physical_address = vgid_start + *ssdt_acpi_vm_gid;
> >>+    ACPI_BUILD_SET_LE(vgid_ptr, sizeof(ssdt_vmgenid_aml),
> >>+                      *ssdt_acpi_vm_gid_addr, 32, vm_gid_physical_address);
> >>+
> >>+    build_header(linker, table_data, vgid_ptr, "SSDT",
> >>+                 sizeof(ssdt_vmgenid_aml), 1);
> >>+}
> >>+
> >>  typedef enum {
> >>      MEM_AFFINITY_NOFLAGS      = 0,
> >>      MEM_AFFINITY_ENABLED      = (1 << 0),
> >>@@ -1617,6 +1652,10 @@ void acpi_build(PcGuestInfo *guest_info, AcpiBuildTables *tables)
> >>          acpi_add_table(table_offsets, tables->table_data);
> >>          build_tpm_ssdt(tables->table_data, tables->linker);
> >>      }
> >>+    if (misc.vm_generation_id_set) {
> >>+        acpi_add_table(table_offsets, tables->table_data);
> >>+        build_vmgenid_ssdt(tables->table_data, tables->linker, &misc);
> >>+    }
> >>      if (guest_info->numa_nodes) {
> >>          acpi_add_table(table_offsets, tables->table_data);
> >>          build_srat(tables->table_data, tables->linker, &cpu, guest_info);
> >>diff --git a/hw/i386/ssdt-vmgenid.dsl b/hw/i386/ssdt-vmgenid.dsl
> >>new file mode 100644
> >>index 0000000..fee376f
> >>--- /dev/null
> >>+++ b/hw/i386/ssdt-vmgenid.dsl
> >>@@ -0,0 +1,63 @@
> >>+/*
> >>+ * This program is free software; you can redistribute it and/or modify
> >>+ * it under the terms of the GNU General Public License as published by
> >>+ * the Free Software Foundation; either version 2 of the License, or
> >>+ * (at your option) any later version.
> >>+
> >>+ * This program is distributed in the hope that it will be useful,
> >>+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
> >>+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> >>+ * GNU General Public License for more details.
> >>+
> >>+ * You should have received a copy of the GNU General Public License along
> >>+ * with this program; if not, see <http://www.gnu.org/licenses/>.
> >>+ */
> >>+
> >>+/****************************************************************
> >>+ * Virtual Machine Generation ID Device
> >>+ ****************************************************************/
> >>+
> >>+ACPI_EXTRACT_ALL_CODE ssdt_vmgenid_aml
> >>+
> >>+DefinitionBlock (
> >>+    "ssdt-vmgenid.aml", // Output Filename
> >>+    "SSDT",             // Signature
> >>+    0x01,               // SSDT Compliance Revision
> >>+    "BXPC",             // OEMID
> >>+    "BXSSDTSUSP",       // TABLE ID
> >>+    0x1                 // OEM Revision
> >>+    )
> >>+{
> >>+    Scope(\_SB) {
> >>+
> >>+        Device(VMGI) {
> >>+            Name(_HID, "QEMU0002")
> >>+            Name(_CID, "VM_Gen_Counter")
> >>+            Name(_DDN, "VM_Gen_Counter")
> >>+
> >>+            ACPI_EXTRACT_NAME_DWORD_CONST ssdt_acpi_vm_gid_addr
> >>+            Name(VGIA, 0x12345678)
> >>+
> >>+            ACPI_EXTRACT_NAME_BUFFER16 ssdt_acpi_vm_gid
> >>+            Name(VGID, Buffer(16) {
> >>+                0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> >>+                0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 })
> >
> >IMHO, statically allocating the ID in ACPI like this is a mistake.
> 
> Do you have an idea where else it can be allocated? Since it is just an ACPI
> declared device without resources. A PCI device with a memory region looks
> over-kill just to have a known physical address.

I guess you can reserve some memory in E820, but that's pretty
involved.

It might be a better idea to allocate buffer e.g. like TPM does, but
fill the data in from hypervisor, by passing the physical address back
to host.  _INI might work for this.

> >For example, your guest might be in hybernation, in this
> >case it will not re-read ACPI tables on boot.
> 
> In this case the VGID physical address should stay the same and only the
> value is updated. So I don't think should be a problem.

What will update the value? ACPI tables in qemu are ignored.

> >Also - what guarantees that this buffer is 8 byte aligned?
> 
> The spec say nothing about the memory-alignment of the VGID's physical
> address.
> If the driver can't read an unaligned address I guess it can do a
> simple pointers calculation and read from the closest aligned address and
> beyond the VGID buffer, no?
> >>+
> >>+            Method(_STA, 0, NotSerialized) {
> >>+                Store(VGIA, Local0)
> >>+                If (LEqual(Local0, Zero)) {
> >>+                    Return (0x00)
> >>+                } Else {
> >>+                    Return (0x0F)
> >>+                }
> >>+            }
> >>+
> >>+            Method(ADDR, 0, Serialized) {
> >>+                Store(Package(2) { }, Local0)
> >>+                Store(VGIA, Index(Local0, 0))
> >>+                Store(0x0000, Index(Local0, 1))
> >>+                return (Local0)
> >>+            }
> >>+        }
> >>+    }
> >>+}
> >>diff --git a/hw/i386/ssdt-vmgenid.hex.generated b/hw/i386/ssdt-vmgenid.hex.generated
> >>new file mode 100644
> >>index 0000000..e96cb36
> >>--- /dev/null
> >>+++ b/hw/i386/ssdt-vmgenid.hex.generated
> >>@@ -0,0 +1,206 @@
> >>+static unsigned char ssdt_vmgenid_aml[] = {
> >>+0x53,
> >>+0x53,
> >>+0x44,
> >>+0x54,
> >>+0xc6,
> >>+0x0,
> >>+0x0,
> >>+0x0,
> >>+0x1,
> >>+0xeb,
> >>+0x42,
> >>+0x58,
> >>+0x50,
> >>+0x43,
> >>+0x0,
> >>+0x0,
> >>+0x42,
> >>+0x58,
> >>+0x53,
> >>+0x53,
> >>+0x44,
> >>+0x54,
> >>+0x53,
> >>+0x55,
> >>+0x1,
> >>+0x0,
> >>+0x0,
> >>+0x0,
> >>+0x49,
> >>+0x4e,
> >>+0x54,
> >>+0x4c,
> >>+0x24,
> >>+0x4,
> >>+0x14,
> >>+0x20,
> >>+0x10,
> >>+0x41,
> >>+0xa,
> >>+0x5c,
> >>+0x5f,
> >>+0x53,
> >>+0x42,
> >>+0x5f,
> >>+0x5b,
> >>+0x82,
> >>+0x48,
> >>+0x9,
> >>+0x56,
> >>+0x4d,
> >>+0x47,
> >>+0x49,
> >>+0x8,
> >>+0x5f,
> >>+0x48,
> >>+0x49,
> >>+0x44,
> >>+0xd,
> >>+0x51,
> >>+0x45,
> >>+0x4d,
> >>+0x55,
> >>+0x30,
> >>+0x30,
> >>+0x30,
> >>+0x32,
> >>+0x0,
> >>+0x8,
> >>+0x5f,
> >>+0x43,
> >>+0x49,
> >>+0x44,
> >>+0xd,
> >>+0x56,
> >>+0x4d,
> >>+0x5f,
> >>+0x47,
> >>+0x65,
> >>+0x6e,
> >>+0x5f,
> >>+0x43,
> >>+0x6f,
> >>+0x75,
> >>+0x6e,
> >>+0x74,
> >>+0x65,
> >>+0x72,
> >>+0x0,
> >>+0x8,
> >>+0x5f,
> >>+0x44,
> >>+0x44,
> >>+0x4e,
> >>+0xd,
> >>+0x56,
> >>+0x4d,
> >>+0x5f,
> >>+0x47,
> >>+0x65,
> >>+0x6e,
> >>+0x5f,
> >>+0x43,
> >>+0x6f,
> >>+0x75,
> >>+0x6e,
> >>+0x74,
> >>+0x65,
> >>+0x72,
> >>+0x0,
> >>+0x8,
> >>+0x56,
> >>+0x47,
> >>+0x49,
> >>+0x41,
> >>+0xc,
> >>+0x78,
> >>+0x56,
> >>+0x34,
> >>+0x12,
> >>+0x8,
> >>+0x56,
> >>+0x47,
> >>+0x49,
> >>+0x44,
> >>+0x11,
> >>+0x13,
> >>+0xa,
> >>+0x10,
> >>+0x0,
> >>+0x0,
> >>+0x0,
> >>+0x0,
> >>+0x0,
> >>+0x0,
> >>+0x0,
> >>+0x0,
> >>+0x0,
> >>+0x0,
> >>+0x0,
> >>+0x0,
> >>+0x0,
> >>+0x0,
> >>+0x0,
> >>+0x0,
> >>+0x14,
> >>+0x18,
> >>+0x5f,
> >>+0x53,
> >>+0x54,
> >>+0x41,
> >>+0x0,
> >>+0x70,
> >>+0x56,
> >>+0x47,
> >>+0x49,
> >>+0x41,
> >>+0x60,
> >>+0xa0,
> >>+0x6,
> >>+0x93,
> >>+0x60,
> >>+0x0,
> >>+0xa4,
> >>+0x0,
> >>+0xa1,
> >>+0x4,
> >>+0xa4,
> >>+0xa,
> >>+0xf,
> >>+0x14,
> >>+0x1c,
> >>+0x41,
> >>+0x44,
> >>+0x44,
> >>+0x52,
> >>+0x8,
> >>+0x70,
> >>+0x12,
> >>+0x2,
> >>+0x2,
> >>+0x60,
> >>+0x70,
> >>+0x56,
> >>+0x47,
> >>+0x49,
> >>+0x41,
> >>+0x88,
> >>+0x60,
> >>+0x0,
> >>+0x0,
> >>+0x70,
> >>+0x0,
> >>+0x88,
> >>+0x60,
> >>+0x1,
> >>+0x0,
> >>+0xa4,
> >>+0x60
> >>+};
> >>+static unsigned char ssdt_acpi_vm_gid_addr[] = {
> >>+0x73
> >>+};
> >>+static unsigned char ssdt_acpi_vm_gid[] = {
> >>+0x80
> >>+};
> >>diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
> >>index 979e532..c18b800 100644
> >>--- a/hw/misc/Makefile.objs
> >>+++ b/hw/misc/Makefile.objs
> >>@@ -41,3 +41,4 @@ obj-$(CONFIG_SLAVIO) += slavio_misc.o
> >>  obj-$(CONFIG_ZYNQ) += zynq_slcr.o
> >>
> >>  obj-$(CONFIG_PVPANIC) += pvpanic.o
> >>+obj-$(CONFIG_VMGENID) += vmgenid.o
> >>diff --git a/hw/misc/vmgenid.c b/hw/misc/vmgenid.c
> >>new file mode 100644
> >>index 0000000..81cc49e
> >>--- /dev/null
> >>+++ b/hw/misc/vmgenid.c
> >>@@ -0,0 +1,84 @@
> >>+/*
> >>+ *  Virtual Machine Generation ID Device
> >>+ *
> >>+ *  Copyright (C) 2014 Red Hat Inc.
> >>+ *
> >>+ *  Authors: Gal Hammer <ghammer@redhat.com>
> >>+ *
> >>+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
> >>+ * See the COPYING file in the top-level directory.
> >>+ *
> >>+ */
> >>+
> >>+#include "hw/i386/pc.h"
> >>+#include "hw/sysbus.h"
> >>+
> >>+#define VMGENID_DEVICE "vmgenid"
> >>+
> >>+#define PROPERTY_UUID "uuid"
> >>+
> >>+#define VMGENID(obj) OBJECT_CHECK(VmGenIdState, (obj), VMGENID_DEVICE)
> >>+
> >>+typedef struct VmGenIdState {
> >>+    SysBusDevice parent_obj;
> >>+    char *guid_arg;
> >>+} VmGenIdState;
> >>+
> >>+bool vm_generation_id(uint8_t id[16])
> >>+{
> >>+    Object *o = object_resolve_path_type("", VMGENID_DEVICE, NULL);
> >>+    char *guid;
> >>+
> >>+    if (!o) {
> >>+        return false;
> >>+    }
> >>+    guid = object_property_get_str(o, PROPERTY_UUID, NULL);
> >>+    /* actual uuid validation was checked during realize. */
> >>+    (void)qemu_uuid_parse(guid, id);
> >>+    return true;
> >>+}
> >>+
> >>+static void vmgenid_realize(DeviceState *dev, Error **errp)
> >>+{
> >>+    VmGenIdState *s = VMGENID(dev);
> >>+    uint8_t id[16];
> >>+
> >>+    if (!s->guid_arg) {
> >>+        error_setg(errp, "missing uuid.");
> >>+        return;
> >>+    }
> >>+
> >>+    if (qemu_uuid_parse(s->guid_arg, id) < 0) {
> >>+        error_setg(errp, "Fail to parse UUID string.");
> >>+        return;
> >>+    }
> >>+}
> >>+
> >>+static Property vmgenid_device_properties[] = {
> >>+    DEFINE_PROP_STRING(PROPERTY_UUID, VmGenIdState, guid_arg),
> >>+    DEFINE_PROP_END_OF_LIST(),
> >>+};
> >>+
> >>+static void vmgenid_class_init(ObjectClass *klass, void *data)
> >>+{
> >>+    DeviceClass *dc = DEVICE_CLASS(klass);
> >>+
> >>+    dc->realize = vmgenid_realize;
> >>+    dc->props = vmgenid_device_properties;
> >>+    dc->cannot_instantiate_with_device_add_yet = false;
> >>+    set_bit(DEVICE_CATEGORY_MISC, dc->categories);
> >>+}
> >>+
> >>+static const TypeInfo vmgenid_device_info = {
> >>+    .name          = VMGENID_DEVICE,
> >>+    .parent        = TYPE_SYS_BUS_DEVICE,
> >>+    .instance_size = sizeof(VmGenIdState),
> >>+    .class_init    = vmgenid_class_init,
> >>+};
> >>+
> >>+static void vmgenid_register_types(void)
> >>+{
> >>+    type_register_static(&vmgenid_device_info);
> >>+}
> >>+
> >>+type_init(vmgenid_register_types)
> >>diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> >>index 77316d5..40ecccb 100644
> >>--- a/include/hw/i386/pc.h
> >>+++ b/include/hw/i386/pc.h
> >>@@ -290,6 +290,9 @@ void pc_system_firmware_init(MemoryRegion *rom_memory,
> >>  /* pvpanic.c */
> >>  uint16_t pvpanic_port(void);
> >>
> >>+/* vmgenid.c */
> >>+bool vm_generation_id(uint8_t id[16]);
> >>+
> >>  /* e820 types */
> >>  #define E820_RAM        1
> >>  #define E820_RESERVED   2
> >>--
> >>1.9.3
> >>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [Qemu-devel] [PATCH 2/2] i386: Add a Virtual Machine Generation ID device.
  2014-10-02 12:49   ` Michael S. Tsirkin
@ 2014-10-02 13:14     ` Gal Hammer
  2014-10-02 13:30       ` Michael S. Tsirkin
  0 siblings, 1 reply; 28+ messages in thread
From: Gal Hammer @ 2014-10-02 13:14 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: Paolo Bonzini, qemu-devel, Igor Mammedov

On 02/10/2014 15:49, Michael S. Tsirkin wrote:
> On Wed, Sep 17, 2014 at 02:39:52PM +0300, Gal Hammer wrote:
>> Based on Microsoft's sepecifications (paper can be dowloaded from
>> http://go.microsoft.com/fwlink/?LinkId=260709), add a device
>> description to the SSDT ACPI table.
>>
>> The GUID is set using a new "vmgenid" device.
>>
>> Signed-off-by: Gal Hammer <ghammer@redhat.com>
>> Reviewed-By: Igor Mammedov <imammedo@redhat.com>
>> ---
>>   default-configs/i386-softmmu.mak   |   1 +
>>   default-configs/x86_64-softmmu.mak |   1 +
>>   hw/i386/Makefile.objs              |   2 +-
>>   hw/i386/acpi-build.c               |  39 +++++++
>>   hw/i386/ssdt-vmgenid.dsl           |  63 ++++++++++++
>>   hw/i386/ssdt-vmgenid.hex.generated | 206 +++++++++++++++++++++++++++++++++++++
>>   hw/misc/Makefile.objs              |   1 +
>>   hw/misc/vmgenid.c                  |  84 +++++++++++++++
>>   include/hw/i386/pc.h               |   3 +
>
> Patch scripts/update-acpi.sh as well please.

I understand what should be changed there. The script just copy all the 
*.hex files to *.hex.generated.

>>   9 files changed, 399 insertions(+), 1 deletion(-)
>>   create mode 100644 hw/i386/ssdt-vmgenid.dsl
>>   create mode 100644 hw/i386/ssdt-vmgenid.hex.generated
>>   create mode 100644 hw/misc/vmgenid.c
>>
>> diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak
>> index 8e08841..bd33c75 100644
>> --- a/default-configs/i386-softmmu.mak
>> +++ b/default-configs/i386-softmmu.mak
>> @@ -45,3 +45,4 @@ CONFIG_IOAPIC=y
>>   CONFIG_ICC_BUS=y
>>   CONFIG_PVPANIC=y
>>   CONFIG_MEM_HOTPLUG=y
>> +CONFIG_VMGENID=y
>> diff --git a/default-configs/x86_64-softmmu.mak b/default-configs/x86_64-softmmu.mak
>> index 66557ac..006fc7c 100644
>> --- a/default-configs/x86_64-softmmu.mak
>> +++ b/default-configs/x86_64-softmmu.mak
>> @@ -45,3 +45,4 @@ CONFIG_IOAPIC=y
>>   CONFIG_ICC_BUS=y
>>   CONFIG_PVPANIC=y
>>   CONFIG_MEM_HOTPLUG=y
>> +CONFIG_VMGENID=y
>> diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs
>> index 9d419ad..cd1beb3 100644
>> --- a/hw/i386/Makefile.objs
>> +++ b/hw/i386/Makefile.objs
>> @@ -12,7 +12,7 @@ hw/i386/acpi-build.o: hw/i386/acpi-build.c hw/i386/acpi-dsdt.hex \
>>   	hw/i386/ssdt-proc.hex hw/i386/ssdt-pcihp.hex hw/i386/ssdt-misc.hex \
>>   	hw/i386/acpi-dsdt.hex hw/i386/q35-acpi-dsdt.hex \
>>   	hw/i386/q35-acpi-dsdt.hex hw/i386/ssdt-mem.hex \
>> -	hw/i386/ssdt-tpm.hex
>> +	hw/i386/ssdt-tpm.hex hw/i386/ssdt-vmgenid.hex
>>
>>   iasl-option=$(shell if test -z "`$(1) $(2) 2>&1 > /dev/null`" \
>>       ; then echo "$(2)"; else echo "$(3)"; fi ;)
>> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
>> index a313321..72d5a88 100644
>> --- a/hw/i386/acpi-build.c
>> +++ b/hw/i386/acpi-build.c
>> @@ -96,6 +96,8 @@ typedef struct AcpiMiscInfo {
>>       const unsigned char *dsdt_code;
>>       unsigned dsdt_size;
>>       uint16_t pvpanic_port;
>> +    bool vm_generation_id_set;
>> +    uint8_t vm_generation_id[16];
>>   } AcpiMiscInfo;
>>
>>   typedef struct AcpiBuildPciBusHotplugState {
>> @@ -216,6 +218,7 @@ static void acpi_get_misc_info(AcpiMiscInfo *info)
>>       info->has_hpet = hpet_find();
>>       info->has_tpm = tpm_find();
>>       info->pvpanic_port = pvpanic_port();
>> +    info->vm_generation_id_set = vm_generation_id(info->vm_generation_id);
>>   }
>>
>>   static void acpi_get_pci_info(PcPciInfo *info)
>> @@ -710,6 +713,7 @@ static inline char acpi_get_hex(uint32_t val)
>>   #include "hw/i386/ssdt-misc.hex"
>>   #include "hw/i386/ssdt-pcihp.hex"
>>   #include "hw/i386/ssdt-tpm.hex"
>> +#include "hw/i386/ssdt-vmgenid.hex"
>>
>>   static void
>>   build_append_notify_method(GArray *device, const char *name,
>> @@ -1246,6 +1250,37 @@ build_tpm_ssdt(GArray *table_data, GArray *linker)
>>       memcpy(tpm_ptr, ssdt_tpm_aml, sizeof(ssdt_tpm_aml));
>>   }
>>
>> +static void
>> +build_vmgenid_ssdt(GArray *table_data, GArray *linker, AcpiMiscInfo *info)
>> +{
>> +    int vgid_start = table_data->len;
>> +    void *vgid_ptr;
>> +    uint8_t *vm_gid_ptr;
>> +    uint32_t vm_gid_physical_address;
>> +
>> +    vgid_ptr = acpi_data_push(table_data, sizeof(ssdt_vmgenid_aml));
>> +    memcpy(vgid_ptr, ssdt_vmgenid_aml, sizeof(ssdt_vmgenid_aml));
>> +
>> +    vm_gid_ptr = acpi_data_get_ptr(vgid_ptr, sizeof(ssdt_vmgenid_aml),
>> +                                   *ssdt_acpi_vm_gid,
>> +                                   sizeof(info->vm_generation_id));
>> +    memcpy(vm_gid_ptr, info->vm_generation_id,
>> +           sizeof(info->vm_generation_id));
>> +
>> +    bios_linker_loader_add_pointer(linker, ACPI_BUILD_TABLE_FILE,
>> +                                   ACPI_BUILD_TABLE_FILE,
>> +                                   table_data,
>> +                                   vgid_ptr + *ssdt_acpi_vm_gid_addr,
>> +                                   sizeof(uint32_t));
>> +
>> +    vm_gid_physical_address = vgid_start + *ssdt_acpi_vm_gid;
>> +    ACPI_BUILD_SET_LE(vgid_ptr, sizeof(ssdt_vmgenid_aml),
>> +                      *ssdt_acpi_vm_gid_addr, 32, vm_gid_physical_address);
>> +
>> +    build_header(linker, table_data, vgid_ptr, "SSDT",
>> +                 sizeof(ssdt_vmgenid_aml), 1);
>> +}
>> +
>>   typedef enum {
>>       MEM_AFFINITY_NOFLAGS      = 0,
>>       MEM_AFFINITY_ENABLED      = (1 << 0),
>> @@ -1617,6 +1652,10 @@ void acpi_build(PcGuestInfo *guest_info, AcpiBuildTables *tables)
>>           acpi_add_table(table_offsets, tables->table_data);
>>           build_tpm_ssdt(tables->table_data, tables->linker);
>>       }
>> +    if (misc.vm_generation_id_set) {
>> +        acpi_add_table(table_offsets, tables->table_data);
>> +        build_vmgenid_ssdt(tables->table_data, tables->linker, &misc);
>> +    }
>>       if (guest_info->numa_nodes) {
>>           acpi_add_table(table_offsets, tables->table_data);
>>           build_srat(tables->table_data, tables->linker, &cpu, guest_info);
>> diff --git a/hw/i386/ssdt-vmgenid.dsl b/hw/i386/ssdt-vmgenid.dsl
>> new file mode 100644
>> index 0000000..fee376f
>> --- /dev/null
>> +++ b/hw/i386/ssdt-vmgenid.dsl
>> @@ -0,0 +1,63 @@
>> +/*
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> +
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> +
>> + * You should have received a copy of the GNU General Public License along
>> + * with this program; if not, see <http://www.gnu.org/licenses/>.
>> + */
>> +
>> +/****************************************************************
>> + * Virtual Machine Generation ID Device
>> + ****************************************************************/
>> +
>> +ACPI_EXTRACT_ALL_CODE ssdt_vmgenid_aml
>> +
>> +DefinitionBlock (
>> +    "ssdt-vmgenid.aml", // Output Filename
>> +    "SSDT",             // Signature
>> +    0x01,               // SSDT Compliance Revision
>> +    "BXPC",             // OEMID
>> +    "BXSSDTSUSP",       // TABLE ID
>> +    0x1                 // OEM Revision
>> +    )
>> +{
>> +    Scope(\_SB) {
>> +
>> +        Device(VMGI) {
>> +            Name(_HID, "QEMU0002")
>> +            Name(_CID, "VM_Gen_Counter")
>> +            Name(_DDN, "VM_Gen_Counter")
>> +
>> +            ACPI_EXTRACT_NAME_DWORD_CONST ssdt_acpi_vm_gid_addr
>> +            Name(VGIA, 0x12345678)
>> +
>> +            ACPI_EXTRACT_NAME_BUFFER16 ssdt_acpi_vm_gid
>> +            Name(VGID, Buffer(16) {
>> +                0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
>> +                0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 })
>
> IMHO, statically allocating the ID in ACPI like this is a mistake.

Do you have an idea where else it can be allocated? Since it is just an 
ACPI declared device without resources. A PCI device with a memory 
region looks over-kill just to have a known physical address.

> For example, your guest might be in hybernation, in this
> case it will not re-read ACPI tables on boot.

In this case the VGID physical address should stay the same and only the 
value is updated. So I don't think should be a problem.

> Also - what guarantees that this buffer is 8 byte aligned?

The spec say nothing about the memory-alignment of the VGID's physical 
address. If the driver can't read an unaligned address I guess it can do 
a simple pointers calculation and read from the closest aligned address 
and beyond the VGID buffer, no?

>> +
>> +            Method(_STA, 0, NotSerialized) {
>> +                Store(VGIA, Local0)
>> +                If (LEqual(Local0, Zero)) {
>> +                    Return (0x00)
>> +                } Else {
>> +                    Return (0x0F)
>> +                }
>> +            }
>> +
>> +            Method(ADDR, 0, Serialized) {
>> +                Store(Package(2) { }, Local0)
>> +                Store(VGIA, Index(Local0, 0))
>> +                Store(0x0000, Index(Local0, 1))
>> +                return (Local0)
>> +            }
>> +        }
>> +    }
>> +}
>> diff --git a/hw/i386/ssdt-vmgenid.hex.generated b/hw/i386/ssdt-vmgenid.hex.generated
>> new file mode 100644
>> index 0000000..e96cb36
>> --- /dev/null
>> +++ b/hw/i386/ssdt-vmgenid.hex.generated
>> @@ -0,0 +1,206 @@
>> +static unsigned char ssdt_vmgenid_aml[] = {
>> +0x53,
>> +0x53,
>> +0x44,
>> +0x54,
>> +0xc6,
>> +0x0,
>> +0x0,
>> +0x0,
>> +0x1,
>> +0xeb,
>> +0x42,
>> +0x58,
>> +0x50,
>> +0x43,
>> +0x0,
>> +0x0,
>> +0x42,
>> +0x58,
>> +0x53,
>> +0x53,
>> +0x44,
>> +0x54,
>> +0x53,
>> +0x55,
>> +0x1,
>> +0x0,
>> +0x0,
>> +0x0,
>> +0x49,
>> +0x4e,
>> +0x54,
>> +0x4c,
>> +0x24,
>> +0x4,
>> +0x14,
>> +0x20,
>> +0x10,
>> +0x41,
>> +0xa,
>> +0x5c,
>> +0x5f,
>> +0x53,
>> +0x42,
>> +0x5f,
>> +0x5b,
>> +0x82,
>> +0x48,
>> +0x9,
>> +0x56,
>> +0x4d,
>> +0x47,
>> +0x49,
>> +0x8,
>> +0x5f,
>> +0x48,
>> +0x49,
>> +0x44,
>> +0xd,
>> +0x51,
>> +0x45,
>> +0x4d,
>> +0x55,
>> +0x30,
>> +0x30,
>> +0x30,
>> +0x32,
>> +0x0,
>> +0x8,
>> +0x5f,
>> +0x43,
>> +0x49,
>> +0x44,
>> +0xd,
>> +0x56,
>> +0x4d,
>> +0x5f,
>> +0x47,
>> +0x65,
>> +0x6e,
>> +0x5f,
>> +0x43,
>> +0x6f,
>> +0x75,
>> +0x6e,
>> +0x74,
>> +0x65,
>> +0x72,
>> +0x0,
>> +0x8,
>> +0x5f,
>> +0x44,
>> +0x44,
>> +0x4e,
>> +0xd,
>> +0x56,
>> +0x4d,
>> +0x5f,
>> +0x47,
>> +0x65,
>> +0x6e,
>> +0x5f,
>> +0x43,
>> +0x6f,
>> +0x75,
>> +0x6e,
>> +0x74,
>> +0x65,
>> +0x72,
>> +0x0,
>> +0x8,
>> +0x56,
>> +0x47,
>> +0x49,
>> +0x41,
>> +0xc,
>> +0x78,
>> +0x56,
>> +0x34,
>> +0x12,
>> +0x8,
>> +0x56,
>> +0x47,
>> +0x49,
>> +0x44,
>> +0x11,
>> +0x13,
>> +0xa,
>> +0x10,
>> +0x0,
>> +0x0,
>> +0x0,
>> +0x0,
>> +0x0,
>> +0x0,
>> +0x0,
>> +0x0,
>> +0x0,
>> +0x0,
>> +0x0,
>> +0x0,
>> +0x0,
>> +0x0,
>> +0x0,
>> +0x0,
>> +0x14,
>> +0x18,
>> +0x5f,
>> +0x53,
>> +0x54,
>> +0x41,
>> +0x0,
>> +0x70,
>> +0x56,
>> +0x47,
>> +0x49,
>> +0x41,
>> +0x60,
>> +0xa0,
>> +0x6,
>> +0x93,
>> +0x60,
>> +0x0,
>> +0xa4,
>> +0x0,
>> +0xa1,
>> +0x4,
>> +0xa4,
>> +0xa,
>> +0xf,
>> +0x14,
>> +0x1c,
>> +0x41,
>> +0x44,
>> +0x44,
>> +0x52,
>> +0x8,
>> +0x70,
>> +0x12,
>> +0x2,
>> +0x2,
>> +0x60,
>> +0x70,
>> +0x56,
>> +0x47,
>> +0x49,
>> +0x41,
>> +0x88,
>> +0x60,
>> +0x0,
>> +0x0,
>> +0x70,
>> +0x0,
>> +0x88,
>> +0x60,
>> +0x1,
>> +0x0,
>> +0xa4,
>> +0x60
>> +};
>> +static unsigned char ssdt_acpi_vm_gid_addr[] = {
>> +0x73
>> +};
>> +static unsigned char ssdt_acpi_vm_gid[] = {
>> +0x80
>> +};
>> diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
>> index 979e532..c18b800 100644
>> --- a/hw/misc/Makefile.objs
>> +++ b/hw/misc/Makefile.objs
>> @@ -41,3 +41,4 @@ obj-$(CONFIG_SLAVIO) += slavio_misc.o
>>   obj-$(CONFIG_ZYNQ) += zynq_slcr.o
>>
>>   obj-$(CONFIG_PVPANIC) += pvpanic.o
>> +obj-$(CONFIG_VMGENID) += vmgenid.o
>> diff --git a/hw/misc/vmgenid.c b/hw/misc/vmgenid.c
>> new file mode 100644
>> index 0000000..81cc49e
>> --- /dev/null
>> +++ b/hw/misc/vmgenid.c
>> @@ -0,0 +1,84 @@
>> +/*
>> + *  Virtual Machine Generation ID Device
>> + *
>> + *  Copyright (C) 2014 Red Hat Inc.
>> + *
>> + *  Authors: Gal Hammer <ghammer@redhat.com>
>> + *
>> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
>> + * See the COPYING file in the top-level directory.
>> + *
>> + */
>> +
>> +#include "hw/i386/pc.h"
>> +#include "hw/sysbus.h"
>> +
>> +#define VMGENID_DEVICE "vmgenid"
>> +
>> +#define PROPERTY_UUID "uuid"
>> +
>> +#define VMGENID(obj) OBJECT_CHECK(VmGenIdState, (obj), VMGENID_DEVICE)
>> +
>> +typedef struct VmGenIdState {
>> +    SysBusDevice parent_obj;
>> +    char *guid_arg;
>> +} VmGenIdState;
>> +
>> +bool vm_generation_id(uint8_t id[16])
>> +{
>> +    Object *o = object_resolve_path_type("", VMGENID_DEVICE, NULL);
>> +    char *guid;
>> +
>> +    if (!o) {
>> +        return false;
>> +    }
>> +    guid = object_property_get_str(o, PROPERTY_UUID, NULL);
>> +    /* actual uuid validation was checked during realize. */
>> +    (void)qemu_uuid_parse(guid, id);
>> +    return true;
>> +}
>> +
>> +static void vmgenid_realize(DeviceState *dev, Error **errp)
>> +{
>> +    VmGenIdState *s = VMGENID(dev);
>> +    uint8_t id[16];
>> +
>> +    if (!s->guid_arg) {
>> +        error_setg(errp, "missing uuid.");
>> +        return;
>> +    }
>> +
>> +    if (qemu_uuid_parse(s->guid_arg, id) < 0) {
>> +        error_setg(errp, "Fail to parse UUID string.");
>> +        return;
>> +    }
>> +}
>> +
>> +static Property vmgenid_device_properties[] = {
>> +    DEFINE_PROP_STRING(PROPERTY_UUID, VmGenIdState, guid_arg),
>> +    DEFINE_PROP_END_OF_LIST(),
>> +};
>> +
>> +static void vmgenid_class_init(ObjectClass *klass, void *data)
>> +{
>> +    DeviceClass *dc = DEVICE_CLASS(klass);
>> +
>> +    dc->realize = vmgenid_realize;
>> +    dc->props = vmgenid_device_properties;
>> +    dc->cannot_instantiate_with_device_add_yet = false;
>> +    set_bit(DEVICE_CATEGORY_MISC, dc->categories);
>> +}
>> +
>> +static const TypeInfo vmgenid_device_info = {
>> +    .name          = VMGENID_DEVICE,
>> +    .parent        = TYPE_SYS_BUS_DEVICE,
>> +    .instance_size = sizeof(VmGenIdState),
>> +    .class_init    = vmgenid_class_init,
>> +};
>> +
>> +static void vmgenid_register_types(void)
>> +{
>> +    type_register_static(&vmgenid_device_info);
>> +}
>> +
>> +type_init(vmgenid_register_types)
>> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
>> index 77316d5..40ecccb 100644
>> --- a/include/hw/i386/pc.h
>> +++ b/include/hw/i386/pc.h
>> @@ -290,6 +290,9 @@ void pc_system_firmware_init(MemoryRegion *rom_memory,
>>   /* pvpanic.c */
>>   uint16_t pvpanic_port(void);
>>
>> +/* vmgenid.c */
>> +bool vm_generation_id(uint8_t id[16]);
>> +
>>   /* e820 types */
>>   #define E820_RAM        1
>>   #define E820_RESERVED   2
>> --
>> 1.9.3
>>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [Qemu-devel] [PATCH 2/2] i386: Add a Virtual Machine Generation ID device.
  2014-09-17 11:39 ` [Qemu-devel] [PATCH 2/2] i386: Add a Virtual Machine Generation ID device Gal Hammer
@ 2014-10-02 12:49   ` Michael S. Tsirkin
  2014-10-02 13:14     ` Gal Hammer
  0 siblings, 1 reply; 28+ messages in thread
From: Michael S. Tsirkin @ 2014-10-02 12:49 UTC (permalink / raw)
  To: Gal Hammer; +Cc: Paolo Bonzini, qemu-devel, Igor Mammedov

On Wed, Sep 17, 2014 at 02:39:52PM +0300, Gal Hammer wrote:
> Based on Microsoft's sepecifications (paper can be dowloaded from
> http://go.microsoft.com/fwlink/?LinkId=260709), add a device
> description to the SSDT ACPI table.
> 
> The GUID is set using a new "vmgenid" device.
> 
> Signed-off-by: Gal Hammer <ghammer@redhat.com>
> Reviewed-By: Igor Mammedov <imammedo@redhat.com>
> ---
>  default-configs/i386-softmmu.mak   |   1 +
>  default-configs/x86_64-softmmu.mak |   1 +
>  hw/i386/Makefile.objs              |   2 +-
>  hw/i386/acpi-build.c               |  39 +++++++
>  hw/i386/ssdt-vmgenid.dsl           |  63 ++++++++++++
>  hw/i386/ssdt-vmgenid.hex.generated | 206 +++++++++++++++++++++++++++++++++++++
>  hw/misc/Makefile.objs              |   1 +
>  hw/misc/vmgenid.c                  |  84 +++++++++++++++
>  include/hw/i386/pc.h               |   3 +

Patch scripts/update-acpi.sh as well please.

>  9 files changed, 399 insertions(+), 1 deletion(-)
>  create mode 100644 hw/i386/ssdt-vmgenid.dsl
>  create mode 100644 hw/i386/ssdt-vmgenid.hex.generated
>  create mode 100644 hw/misc/vmgenid.c
> 
> diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak
> index 8e08841..bd33c75 100644
> --- a/default-configs/i386-softmmu.mak
> +++ b/default-configs/i386-softmmu.mak
> @@ -45,3 +45,4 @@ CONFIG_IOAPIC=y
>  CONFIG_ICC_BUS=y
>  CONFIG_PVPANIC=y
>  CONFIG_MEM_HOTPLUG=y
> +CONFIG_VMGENID=y
> diff --git a/default-configs/x86_64-softmmu.mak b/default-configs/x86_64-softmmu.mak
> index 66557ac..006fc7c 100644
> --- a/default-configs/x86_64-softmmu.mak
> +++ b/default-configs/x86_64-softmmu.mak
> @@ -45,3 +45,4 @@ CONFIG_IOAPIC=y
>  CONFIG_ICC_BUS=y
>  CONFIG_PVPANIC=y
>  CONFIG_MEM_HOTPLUG=y
> +CONFIG_VMGENID=y
> diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs
> index 9d419ad..cd1beb3 100644
> --- a/hw/i386/Makefile.objs
> +++ b/hw/i386/Makefile.objs
> @@ -12,7 +12,7 @@ hw/i386/acpi-build.o: hw/i386/acpi-build.c hw/i386/acpi-dsdt.hex \
>  	hw/i386/ssdt-proc.hex hw/i386/ssdt-pcihp.hex hw/i386/ssdt-misc.hex \
>  	hw/i386/acpi-dsdt.hex hw/i386/q35-acpi-dsdt.hex \
>  	hw/i386/q35-acpi-dsdt.hex hw/i386/ssdt-mem.hex \
> -	hw/i386/ssdt-tpm.hex
> +	hw/i386/ssdt-tpm.hex hw/i386/ssdt-vmgenid.hex
>  
>  iasl-option=$(shell if test -z "`$(1) $(2) 2>&1 > /dev/null`" \
>      ; then echo "$(2)"; else echo "$(3)"; fi ;)
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index a313321..72d5a88 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -96,6 +96,8 @@ typedef struct AcpiMiscInfo {
>      const unsigned char *dsdt_code;
>      unsigned dsdt_size;
>      uint16_t pvpanic_port;
> +    bool vm_generation_id_set;
> +    uint8_t vm_generation_id[16];
>  } AcpiMiscInfo;
>  
>  typedef struct AcpiBuildPciBusHotplugState {
> @@ -216,6 +218,7 @@ static void acpi_get_misc_info(AcpiMiscInfo *info)
>      info->has_hpet = hpet_find();
>      info->has_tpm = tpm_find();
>      info->pvpanic_port = pvpanic_port();
> +    info->vm_generation_id_set = vm_generation_id(info->vm_generation_id);
>  }
>  
>  static void acpi_get_pci_info(PcPciInfo *info)
> @@ -710,6 +713,7 @@ static inline char acpi_get_hex(uint32_t val)
>  #include "hw/i386/ssdt-misc.hex"
>  #include "hw/i386/ssdt-pcihp.hex"
>  #include "hw/i386/ssdt-tpm.hex"
> +#include "hw/i386/ssdt-vmgenid.hex"
>  
>  static void
>  build_append_notify_method(GArray *device, const char *name,
> @@ -1246,6 +1250,37 @@ build_tpm_ssdt(GArray *table_data, GArray *linker)
>      memcpy(tpm_ptr, ssdt_tpm_aml, sizeof(ssdt_tpm_aml));
>  }
>  
> +static void
> +build_vmgenid_ssdt(GArray *table_data, GArray *linker, AcpiMiscInfo *info)
> +{
> +    int vgid_start = table_data->len;
> +    void *vgid_ptr;
> +    uint8_t *vm_gid_ptr;
> +    uint32_t vm_gid_physical_address;
> +
> +    vgid_ptr = acpi_data_push(table_data, sizeof(ssdt_vmgenid_aml));
> +    memcpy(vgid_ptr, ssdt_vmgenid_aml, sizeof(ssdt_vmgenid_aml));
> +
> +    vm_gid_ptr = acpi_data_get_ptr(vgid_ptr, sizeof(ssdt_vmgenid_aml),
> +                                   *ssdt_acpi_vm_gid,
> +                                   sizeof(info->vm_generation_id));
> +    memcpy(vm_gid_ptr, info->vm_generation_id,
> +           sizeof(info->vm_generation_id));
> +
> +    bios_linker_loader_add_pointer(linker, ACPI_BUILD_TABLE_FILE,
> +                                   ACPI_BUILD_TABLE_FILE,
> +                                   table_data,
> +                                   vgid_ptr + *ssdt_acpi_vm_gid_addr,
> +                                   sizeof(uint32_t));
> +
> +    vm_gid_physical_address = vgid_start + *ssdt_acpi_vm_gid;
> +    ACPI_BUILD_SET_LE(vgid_ptr, sizeof(ssdt_vmgenid_aml),
> +                      *ssdt_acpi_vm_gid_addr, 32, vm_gid_physical_address);
> +
> +    build_header(linker, table_data, vgid_ptr, "SSDT",
> +                 sizeof(ssdt_vmgenid_aml), 1);
> +}
> +
>  typedef enum {
>      MEM_AFFINITY_NOFLAGS      = 0,
>      MEM_AFFINITY_ENABLED      = (1 << 0),
> @@ -1617,6 +1652,10 @@ void acpi_build(PcGuestInfo *guest_info, AcpiBuildTables *tables)
>          acpi_add_table(table_offsets, tables->table_data);
>          build_tpm_ssdt(tables->table_data, tables->linker);
>      }
> +    if (misc.vm_generation_id_set) {
> +        acpi_add_table(table_offsets, tables->table_data);
> +        build_vmgenid_ssdt(tables->table_data, tables->linker, &misc);
> +    }
>      if (guest_info->numa_nodes) {
>          acpi_add_table(table_offsets, tables->table_data);
>          build_srat(tables->table_data, tables->linker, &cpu, guest_info);
> diff --git a/hw/i386/ssdt-vmgenid.dsl b/hw/i386/ssdt-vmgenid.dsl
> new file mode 100644
> index 0000000..fee376f
> --- /dev/null
> +++ b/hw/i386/ssdt-vmgenid.dsl
> @@ -0,0 +1,63 @@
> +/*
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> +
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> +
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +/****************************************************************
> + * Virtual Machine Generation ID Device
> + ****************************************************************/
> +
> +ACPI_EXTRACT_ALL_CODE ssdt_vmgenid_aml
> +
> +DefinitionBlock (
> +    "ssdt-vmgenid.aml", // Output Filename
> +    "SSDT",             // Signature
> +    0x01,               // SSDT Compliance Revision
> +    "BXPC",             // OEMID
> +    "BXSSDTSUSP",       // TABLE ID
> +    0x1                 // OEM Revision
> +    )
> +{
> +    Scope(\_SB) {
> +
> +        Device(VMGI) {
> +            Name(_HID, "QEMU0002")
> +            Name(_CID, "VM_Gen_Counter")
> +            Name(_DDN, "VM_Gen_Counter")
> +
> +            ACPI_EXTRACT_NAME_DWORD_CONST ssdt_acpi_vm_gid_addr
> +            Name(VGIA, 0x12345678)
> +
> +            ACPI_EXTRACT_NAME_BUFFER16 ssdt_acpi_vm_gid
> +            Name(VGID, Buffer(16) {
> +                0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +                0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 })

IMHO, statically allocating the ID in ACPI like this is a mistake.

For example, your guest might be in hybernation, in this
case it will not re-read ACPI tables on boot.

Also - what guarantees that this buffer is 8 byte aligned?


> +
> +            Method(_STA, 0, NotSerialized) {
> +                Store(VGIA, Local0)
> +                If (LEqual(Local0, Zero)) {
> +                    Return (0x00)
> +                } Else {
> +                    Return (0x0F)
> +                }
> +            }
> +
> +            Method(ADDR, 0, Serialized) {
> +                Store(Package(2) { }, Local0)
> +                Store(VGIA, Index(Local0, 0))
> +                Store(0x0000, Index(Local0, 1))
> +                return (Local0)
> +            }
> +        }
> +    }
> +}
> diff --git a/hw/i386/ssdt-vmgenid.hex.generated b/hw/i386/ssdt-vmgenid.hex.generated
> new file mode 100644
> index 0000000..e96cb36
> --- /dev/null
> +++ b/hw/i386/ssdt-vmgenid.hex.generated
> @@ -0,0 +1,206 @@
> +static unsigned char ssdt_vmgenid_aml[] = {
> +0x53,
> +0x53,
> +0x44,
> +0x54,
> +0xc6,
> +0x0,
> +0x0,
> +0x0,
> +0x1,
> +0xeb,
> +0x42,
> +0x58,
> +0x50,
> +0x43,
> +0x0,
> +0x0,
> +0x42,
> +0x58,
> +0x53,
> +0x53,
> +0x44,
> +0x54,
> +0x53,
> +0x55,
> +0x1,
> +0x0,
> +0x0,
> +0x0,
> +0x49,
> +0x4e,
> +0x54,
> +0x4c,
> +0x24,
> +0x4,
> +0x14,
> +0x20,
> +0x10,
> +0x41,
> +0xa,
> +0x5c,
> +0x5f,
> +0x53,
> +0x42,
> +0x5f,
> +0x5b,
> +0x82,
> +0x48,
> +0x9,
> +0x56,
> +0x4d,
> +0x47,
> +0x49,
> +0x8,
> +0x5f,
> +0x48,
> +0x49,
> +0x44,
> +0xd,
> +0x51,
> +0x45,
> +0x4d,
> +0x55,
> +0x30,
> +0x30,
> +0x30,
> +0x32,
> +0x0,
> +0x8,
> +0x5f,
> +0x43,
> +0x49,
> +0x44,
> +0xd,
> +0x56,
> +0x4d,
> +0x5f,
> +0x47,
> +0x65,
> +0x6e,
> +0x5f,
> +0x43,
> +0x6f,
> +0x75,
> +0x6e,
> +0x74,
> +0x65,
> +0x72,
> +0x0,
> +0x8,
> +0x5f,
> +0x44,
> +0x44,
> +0x4e,
> +0xd,
> +0x56,
> +0x4d,
> +0x5f,
> +0x47,
> +0x65,
> +0x6e,
> +0x5f,
> +0x43,
> +0x6f,
> +0x75,
> +0x6e,
> +0x74,
> +0x65,
> +0x72,
> +0x0,
> +0x8,
> +0x56,
> +0x47,
> +0x49,
> +0x41,
> +0xc,
> +0x78,
> +0x56,
> +0x34,
> +0x12,
> +0x8,
> +0x56,
> +0x47,
> +0x49,
> +0x44,
> +0x11,
> +0x13,
> +0xa,
> +0x10,
> +0x0,
> +0x0,
> +0x0,
> +0x0,
> +0x0,
> +0x0,
> +0x0,
> +0x0,
> +0x0,
> +0x0,
> +0x0,
> +0x0,
> +0x0,
> +0x0,
> +0x0,
> +0x0,
> +0x14,
> +0x18,
> +0x5f,
> +0x53,
> +0x54,
> +0x41,
> +0x0,
> +0x70,
> +0x56,
> +0x47,
> +0x49,
> +0x41,
> +0x60,
> +0xa0,
> +0x6,
> +0x93,
> +0x60,
> +0x0,
> +0xa4,
> +0x0,
> +0xa1,
> +0x4,
> +0xa4,
> +0xa,
> +0xf,
> +0x14,
> +0x1c,
> +0x41,
> +0x44,
> +0x44,
> +0x52,
> +0x8,
> +0x70,
> +0x12,
> +0x2,
> +0x2,
> +0x60,
> +0x70,
> +0x56,
> +0x47,
> +0x49,
> +0x41,
> +0x88,
> +0x60,
> +0x0,
> +0x0,
> +0x70,
> +0x0,
> +0x88,
> +0x60,
> +0x1,
> +0x0,
> +0xa4,
> +0x60
> +};
> +static unsigned char ssdt_acpi_vm_gid_addr[] = {
> +0x73
> +};
> +static unsigned char ssdt_acpi_vm_gid[] = {
> +0x80
> +};
> diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
> index 979e532..c18b800 100644
> --- a/hw/misc/Makefile.objs
> +++ b/hw/misc/Makefile.objs
> @@ -41,3 +41,4 @@ obj-$(CONFIG_SLAVIO) += slavio_misc.o
>  obj-$(CONFIG_ZYNQ) += zynq_slcr.o
>  
>  obj-$(CONFIG_PVPANIC) += pvpanic.o
> +obj-$(CONFIG_VMGENID) += vmgenid.o
> diff --git a/hw/misc/vmgenid.c b/hw/misc/vmgenid.c
> new file mode 100644
> index 0000000..81cc49e
> --- /dev/null
> +++ b/hw/misc/vmgenid.c
> @@ -0,0 +1,84 @@
> +/*
> + *  Virtual Machine Generation ID Device
> + *
> + *  Copyright (C) 2014 Red Hat Inc.
> + *
> + *  Authors: Gal Hammer <ghammer@redhat.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + *
> + */
> +
> +#include "hw/i386/pc.h"
> +#include "hw/sysbus.h"
> +
> +#define VMGENID_DEVICE "vmgenid"
> +
> +#define PROPERTY_UUID "uuid"
> +
> +#define VMGENID(obj) OBJECT_CHECK(VmGenIdState, (obj), VMGENID_DEVICE)
> +
> +typedef struct VmGenIdState {
> +    SysBusDevice parent_obj;
> +    char *guid_arg;
> +} VmGenIdState;
> +
> +bool vm_generation_id(uint8_t id[16])
> +{
> +    Object *o = object_resolve_path_type("", VMGENID_DEVICE, NULL);
> +    char *guid;
> +
> +    if (!o) {
> +        return false;
> +    }
> +    guid = object_property_get_str(o, PROPERTY_UUID, NULL);
> +    /* actual uuid validation was checked during realize. */
> +    (void)qemu_uuid_parse(guid, id);
> +    return true;
> +}
> +
> +static void vmgenid_realize(DeviceState *dev, Error **errp)
> +{
> +    VmGenIdState *s = VMGENID(dev);
> +    uint8_t id[16];
> +
> +    if (!s->guid_arg) {
> +        error_setg(errp, "missing uuid.");
> +        return;
> +    }
> +
> +    if (qemu_uuid_parse(s->guid_arg, id) < 0) {
> +        error_setg(errp, "Fail to parse UUID string.");
> +        return;
> +    }
> +}
> +
> +static Property vmgenid_device_properties[] = {
> +    DEFINE_PROP_STRING(PROPERTY_UUID, VmGenIdState, guid_arg),
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
> +static void vmgenid_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +
> +    dc->realize = vmgenid_realize;
> +    dc->props = vmgenid_device_properties;
> +    dc->cannot_instantiate_with_device_add_yet = false;
> +    set_bit(DEVICE_CATEGORY_MISC, dc->categories);
> +}
> +
> +static const TypeInfo vmgenid_device_info = {
> +    .name          = VMGENID_DEVICE,
> +    .parent        = TYPE_SYS_BUS_DEVICE,
> +    .instance_size = sizeof(VmGenIdState),
> +    .class_init    = vmgenid_class_init,
> +};
> +
> +static void vmgenid_register_types(void)
> +{
> +    type_register_static(&vmgenid_device_info);
> +}
> +
> +type_init(vmgenid_register_types)
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 77316d5..40ecccb 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -290,6 +290,9 @@ void pc_system_firmware_init(MemoryRegion *rom_memory,
>  /* pvpanic.c */
>  uint16_t pvpanic_port(void);
>  
> +/* vmgenid.c */
> +bool vm_generation_id(uint8_t id[16]);
> +
>  /* e820 types */
>  #define E820_RAM        1
>  #define E820_RESERVED   2
> -- 
> 1.9.3
> 

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [Qemu-devel] [PATCH 2/2] i386: Add a Virtual Machine Generation ID device.
  2014-09-17 11:39 [Qemu-devel] [PATCH 0/2 V5] " Gal Hammer
@ 2014-09-17 11:39 ` Gal Hammer
  2014-10-02 12:49   ` Michael S. Tsirkin
  0 siblings, 1 reply; 28+ messages in thread
From: Gal Hammer @ 2014-09-17 11:39 UTC (permalink / raw)
  To: qemu-devel; +Cc: Gal Hammer, Paolo Bonzini, Igor Mammedov

Based on Microsoft's sepecifications (paper can be dowloaded from
http://go.microsoft.com/fwlink/?LinkId=260709), add a device
description to the SSDT ACPI table.

The GUID is set using a new "vmgenid" device.

Signed-off-by: Gal Hammer <ghammer@redhat.com>
Reviewed-By: Igor Mammedov <imammedo@redhat.com>
---
 default-configs/i386-softmmu.mak   |   1 +
 default-configs/x86_64-softmmu.mak |   1 +
 hw/i386/Makefile.objs              |   2 +-
 hw/i386/acpi-build.c               |  39 +++++++
 hw/i386/ssdt-vmgenid.dsl           |  63 ++++++++++++
 hw/i386/ssdt-vmgenid.hex.generated | 206 +++++++++++++++++++++++++++++++++++++
 hw/misc/Makefile.objs              |   1 +
 hw/misc/vmgenid.c                  |  84 +++++++++++++++
 include/hw/i386/pc.h               |   3 +
 9 files changed, 399 insertions(+), 1 deletion(-)
 create mode 100644 hw/i386/ssdt-vmgenid.dsl
 create mode 100644 hw/i386/ssdt-vmgenid.hex.generated
 create mode 100644 hw/misc/vmgenid.c

diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak
index 8e08841..bd33c75 100644
--- a/default-configs/i386-softmmu.mak
+++ b/default-configs/i386-softmmu.mak
@@ -45,3 +45,4 @@ CONFIG_IOAPIC=y
 CONFIG_ICC_BUS=y
 CONFIG_PVPANIC=y
 CONFIG_MEM_HOTPLUG=y
+CONFIG_VMGENID=y
diff --git a/default-configs/x86_64-softmmu.mak b/default-configs/x86_64-softmmu.mak
index 66557ac..006fc7c 100644
--- a/default-configs/x86_64-softmmu.mak
+++ b/default-configs/x86_64-softmmu.mak
@@ -45,3 +45,4 @@ CONFIG_IOAPIC=y
 CONFIG_ICC_BUS=y
 CONFIG_PVPANIC=y
 CONFIG_MEM_HOTPLUG=y
+CONFIG_VMGENID=y
diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs
index 9d419ad..cd1beb3 100644
--- a/hw/i386/Makefile.objs
+++ b/hw/i386/Makefile.objs
@@ -12,7 +12,7 @@ hw/i386/acpi-build.o: hw/i386/acpi-build.c hw/i386/acpi-dsdt.hex \
 	hw/i386/ssdt-proc.hex hw/i386/ssdt-pcihp.hex hw/i386/ssdt-misc.hex \
 	hw/i386/acpi-dsdt.hex hw/i386/q35-acpi-dsdt.hex \
 	hw/i386/q35-acpi-dsdt.hex hw/i386/ssdt-mem.hex \
-	hw/i386/ssdt-tpm.hex
+	hw/i386/ssdt-tpm.hex hw/i386/ssdt-vmgenid.hex
 
 iasl-option=$(shell if test -z "`$(1) $(2) 2>&1 > /dev/null`" \
     ; then echo "$(2)"; else echo "$(3)"; fi ;)
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index a313321..72d5a88 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -96,6 +96,8 @@ typedef struct AcpiMiscInfo {
     const unsigned char *dsdt_code;
     unsigned dsdt_size;
     uint16_t pvpanic_port;
+    bool vm_generation_id_set;
+    uint8_t vm_generation_id[16];
 } AcpiMiscInfo;
 
 typedef struct AcpiBuildPciBusHotplugState {
@@ -216,6 +218,7 @@ static void acpi_get_misc_info(AcpiMiscInfo *info)
     info->has_hpet = hpet_find();
     info->has_tpm = tpm_find();
     info->pvpanic_port = pvpanic_port();
+    info->vm_generation_id_set = vm_generation_id(info->vm_generation_id);
 }
 
 static void acpi_get_pci_info(PcPciInfo *info)
@@ -710,6 +713,7 @@ static inline char acpi_get_hex(uint32_t val)
 #include "hw/i386/ssdt-misc.hex"
 #include "hw/i386/ssdt-pcihp.hex"
 #include "hw/i386/ssdt-tpm.hex"
+#include "hw/i386/ssdt-vmgenid.hex"
 
 static void
 build_append_notify_method(GArray *device, const char *name,
@@ -1246,6 +1250,37 @@ build_tpm_ssdt(GArray *table_data, GArray *linker)
     memcpy(tpm_ptr, ssdt_tpm_aml, sizeof(ssdt_tpm_aml));
 }
 
+static void
+build_vmgenid_ssdt(GArray *table_data, GArray *linker, AcpiMiscInfo *info)
+{
+    int vgid_start = table_data->len;
+    void *vgid_ptr;
+    uint8_t *vm_gid_ptr;
+    uint32_t vm_gid_physical_address;
+
+    vgid_ptr = acpi_data_push(table_data, sizeof(ssdt_vmgenid_aml));
+    memcpy(vgid_ptr, ssdt_vmgenid_aml, sizeof(ssdt_vmgenid_aml));
+
+    vm_gid_ptr = acpi_data_get_ptr(vgid_ptr, sizeof(ssdt_vmgenid_aml),
+                                   *ssdt_acpi_vm_gid,
+                                   sizeof(info->vm_generation_id));
+    memcpy(vm_gid_ptr, info->vm_generation_id,
+           sizeof(info->vm_generation_id));
+
+    bios_linker_loader_add_pointer(linker, ACPI_BUILD_TABLE_FILE,
+                                   ACPI_BUILD_TABLE_FILE,
+                                   table_data,
+                                   vgid_ptr + *ssdt_acpi_vm_gid_addr,
+                                   sizeof(uint32_t));
+
+    vm_gid_physical_address = vgid_start + *ssdt_acpi_vm_gid;
+    ACPI_BUILD_SET_LE(vgid_ptr, sizeof(ssdt_vmgenid_aml),
+                      *ssdt_acpi_vm_gid_addr, 32, vm_gid_physical_address);
+
+    build_header(linker, table_data, vgid_ptr, "SSDT",
+                 sizeof(ssdt_vmgenid_aml), 1);
+}
+
 typedef enum {
     MEM_AFFINITY_NOFLAGS      = 0,
     MEM_AFFINITY_ENABLED      = (1 << 0),
@@ -1617,6 +1652,10 @@ void acpi_build(PcGuestInfo *guest_info, AcpiBuildTables *tables)
         acpi_add_table(table_offsets, tables->table_data);
         build_tpm_ssdt(tables->table_data, tables->linker);
     }
+    if (misc.vm_generation_id_set) {
+        acpi_add_table(table_offsets, tables->table_data);
+        build_vmgenid_ssdt(tables->table_data, tables->linker, &misc);
+    }
     if (guest_info->numa_nodes) {
         acpi_add_table(table_offsets, tables->table_data);
         build_srat(tables->table_data, tables->linker, &cpu, guest_info);
diff --git a/hw/i386/ssdt-vmgenid.dsl b/hw/i386/ssdt-vmgenid.dsl
new file mode 100644
index 0000000..fee376f
--- /dev/null
+++ b/hw/i386/ssdt-vmgenid.dsl
@@ -0,0 +1,63 @@
+/*
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+/****************************************************************
+ * Virtual Machine Generation ID Device
+ ****************************************************************/
+
+ACPI_EXTRACT_ALL_CODE ssdt_vmgenid_aml
+
+DefinitionBlock (
+    "ssdt-vmgenid.aml", // Output Filename
+    "SSDT",             // Signature
+    0x01,               // SSDT Compliance Revision
+    "BXPC",             // OEMID
+    "BXSSDTSUSP",       // TABLE ID
+    0x1                 // OEM Revision
+    )
+{
+    Scope(\_SB) {
+
+        Device(VMGI) {
+            Name(_HID, "QEMU0002")
+            Name(_CID, "VM_Gen_Counter")
+            Name(_DDN, "VM_Gen_Counter")
+
+            ACPI_EXTRACT_NAME_DWORD_CONST ssdt_acpi_vm_gid_addr
+            Name(VGIA, 0x12345678)
+
+            ACPI_EXTRACT_NAME_BUFFER16 ssdt_acpi_vm_gid
+            Name(VGID, Buffer(16) {
+                0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+                0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 })
+
+            Method(_STA, 0, NotSerialized) {
+                Store(VGIA, Local0)
+                If (LEqual(Local0, Zero)) {
+                    Return (0x00)
+                } Else {
+                    Return (0x0F)
+                }
+            }
+
+            Method(ADDR, 0, Serialized) {
+                Store(Package(2) { }, Local0)
+                Store(VGIA, Index(Local0, 0))
+                Store(0x0000, Index(Local0, 1))
+                return (Local0)
+            }
+        }
+    }
+}
diff --git a/hw/i386/ssdt-vmgenid.hex.generated b/hw/i386/ssdt-vmgenid.hex.generated
new file mode 100644
index 0000000..e96cb36
--- /dev/null
+++ b/hw/i386/ssdt-vmgenid.hex.generated
@@ -0,0 +1,206 @@
+static unsigned char ssdt_vmgenid_aml[] = {
+0x53,
+0x53,
+0x44,
+0x54,
+0xc6,
+0x0,
+0x0,
+0x0,
+0x1,
+0xeb,
+0x42,
+0x58,
+0x50,
+0x43,
+0x0,
+0x0,
+0x42,
+0x58,
+0x53,
+0x53,
+0x44,
+0x54,
+0x53,
+0x55,
+0x1,
+0x0,
+0x0,
+0x0,
+0x49,
+0x4e,
+0x54,
+0x4c,
+0x24,
+0x4,
+0x14,
+0x20,
+0x10,
+0x41,
+0xa,
+0x5c,
+0x5f,
+0x53,
+0x42,
+0x5f,
+0x5b,
+0x82,
+0x48,
+0x9,
+0x56,
+0x4d,
+0x47,
+0x49,
+0x8,
+0x5f,
+0x48,
+0x49,
+0x44,
+0xd,
+0x51,
+0x45,
+0x4d,
+0x55,
+0x30,
+0x30,
+0x30,
+0x32,
+0x0,
+0x8,
+0x5f,
+0x43,
+0x49,
+0x44,
+0xd,
+0x56,
+0x4d,
+0x5f,
+0x47,
+0x65,
+0x6e,
+0x5f,
+0x43,
+0x6f,
+0x75,
+0x6e,
+0x74,
+0x65,
+0x72,
+0x0,
+0x8,
+0x5f,
+0x44,
+0x44,
+0x4e,
+0xd,
+0x56,
+0x4d,
+0x5f,
+0x47,
+0x65,
+0x6e,
+0x5f,
+0x43,
+0x6f,
+0x75,
+0x6e,
+0x74,
+0x65,
+0x72,
+0x0,
+0x8,
+0x56,
+0x47,
+0x49,
+0x41,
+0xc,
+0x78,
+0x56,
+0x34,
+0x12,
+0x8,
+0x56,
+0x47,
+0x49,
+0x44,
+0x11,
+0x13,
+0xa,
+0x10,
+0x0,
+0x0,
+0x0,
+0x0,
+0x0,
+0x0,
+0x0,
+0x0,
+0x0,
+0x0,
+0x0,
+0x0,
+0x0,
+0x0,
+0x0,
+0x0,
+0x14,
+0x18,
+0x5f,
+0x53,
+0x54,
+0x41,
+0x0,
+0x70,
+0x56,
+0x47,
+0x49,
+0x41,
+0x60,
+0xa0,
+0x6,
+0x93,
+0x60,
+0x0,
+0xa4,
+0x0,
+0xa1,
+0x4,
+0xa4,
+0xa,
+0xf,
+0x14,
+0x1c,
+0x41,
+0x44,
+0x44,
+0x52,
+0x8,
+0x70,
+0x12,
+0x2,
+0x2,
+0x60,
+0x70,
+0x56,
+0x47,
+0x49,
+0x41,
+0x88,
+0x60,
+0x0,
+0x0,
+0x70,
+0x0,
+0x88,
+0x60,
+0x1,
+0x0,
+0xa4,
+0x60
+};
+static unsigned char ssdt_acpi_vm_gid_addr[] = {
+0x73
+};
+static unsigned char ssdt_acpi_vm_gid[] = {
+0x80
+};
diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
index 979e532..c18b800 100644
--- a/hw/misc/Makefile.objs
+++ b/hw/misc/Makefile.objs
@@ -41,3 +41,4 @@ obj-$(CONFIG_SLAVIO) += slavio_misc.o
 obj-$(CONFIG_ZYNQ) += zynq_slcr.o
 
 obj-$(CONFIG_PVPANIC) += pvpanic.o
+obj-$(CONFIG_VMGENID) += vmgenid.o
diff --git a/hw/misc/vmgenid.c b/hw/misc/vmgenid.c
new file mode 100644
index 0000000..81cc49e
--- /dev/null
+++ b/hw/misc/vmgenid.c
@@ -0,0 +1,84 @@
+/*
+ *  Virtual Machine Generation ID Device
+ *
+ *  Copyright (C) 2014 Red Hat Inc.
+ *
+ *  Authors: Gal Hammer <ghammer@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#include "hw/i386/pc.h"
+#include "hw/sysbus.h"
+
+#define VMGENID_DEVICE "vmgenid"
+
+#define PROPERTY_UUID "uuid"
+
+#define VMGENID(obj) OBJECT_CHECK(VmGenIdState, (obj), VMGENID_DEVICE)
+
+typedef struct VmGenIdState {
+    SysBusDevice parent_obj;
+    char *guid_arg;
+} VmGenIdState;
+
+bool vm_generation_id(uint8_t id[16])
+{
+    Object *o = object_resolve_path_type("", VMGENID_DEVICE, NULL);
+    char *guid;
+
+    if (!o) {
+        return false;
+    }
+    guid = object_property_get_str(o, PROPERTY_UUID, NULL);
+    /* actual uuid validation was checked during realize. */
+    (void)qemu_uuid_parse(guid, id);
+    return true;
+}
+
+static void vmgenid_realize(DeviceState *dev, Error **errp)
+{
+    VmGenIdState *s = VMGENID(dev);
+    uint8_t id[16];
+
+    if (!s->guid_arg) {
+        error_setg(errp, "missing uuid.");
+        return;
+    }
+
+    if (qemu_uuid_parse(s->guid_arg, id) < 0) {
+        error_setg(errp, "Fail to parse UUID string.");
+        return;
+    }
+}
+
+static Property vmgenid_device_properties[] = {
+    DEFINE_PROP_STRING(PROPERTY_UUID, VmGenIdState, guid_arg),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void vmgenid_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->realize = vmgenid_realize;
+    dc->props = vmgenid_device_properties;
+    dc->cannot_instantiate_with_device_add_yet = false;
+    set_bit(DEVICE_CATEGORY_MISC, dc->categories);
+}
+
+static const TypeInfo vmgenid_device_info = {
+    .name          = VMGENID_DEVICE,
+    .parent        = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(VmGenIdState),
+    .class_init    = vmgenid_class_init,
+};
+
+static void vmgenid_register_types(void)
+{
+    type_register_static(&vmgenid_device_info);
+}
+
+type_init(vmgenid_register_types)
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 77316d5..40ecccb 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -290,6 +290,9 @@ void pc_system_firmware_init(MemoryRegion *rom_memory,
 /* pvpanic.c */
 uint16_t pvpanic_port(void);
 
+/* vmgenid.c */
+bool vm_generation_id(uint8_t id[16]);
+
 /* e820 types */
 #define E820_RAM        1
 #define E820_RESERVED   2
-- 
1.9.3

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [Qemu-devel] [PATCH 2/2] i386: Add a Virtual Machine Generation ID device.
  2014-09-17  7:36     ` Gal Hammer
@ 2014-09-17  9:56       ` Igor Mammedov
  0 siblings, 0 replies; 28+ messages in thread
From: Igor Mammedov @ 2014-09-17  9:56 UTC (permalink / raw)
  To: Gal Hammer; +Cc: pbonzini, qemu-devel

On Wed, 17 Sep 2014 03:36:58 -0400 (EDT)
Gal Hammer <ghammer@redhat.com> wrote:

> Hi Igor,
> 
> ----- Original Message -----
> > From: "Igor Mammedov" <imammedo@redhat.com>
> > To: "Gal Hammer" <ghammer@redhat.com>
> > Cc: pbonzini@redhat.com, qemu-devel@nongnu.org
> > Sent: Tuesday, September 16, 2014 5:54:28 PM
> > Subject: Re: [Qemu-devel] [PATCH 2/2] i386: Add a Virtual Machine Generation ID device.
> > 
> > On Tue, 16 Sep 2014 16:04:10 +0300
> > Gal Hammer <ghammer@redhat.com> wrote:
> > 
> > > Based on Microsoft's sepecifications (paper can be dowloaded from
> > > http://go.microsoft.com/fwlink/?LinkId=260709), add a device
> > > description to the SSDT ACPI table.
> > > 
> > > The GUID is set using a new "vmgenid" device.
> > > 
> > > Signed-off-by: Gal Hammer <ghammer@redhat.com>
> > > ---
> > >  default-configs/i386-softmmu.mak   |  1 +
> > >  default-configs/x86_64-softmmu.mak |  1 +
> > >  hw/i386/Makefile.objs              |  2 +-
> > >  hw/i386/acpi-build.c               | 39 +++++++++++++++++
> > >  hw/i386/ssdt-vmgenid.dsl           | 64 ++++++++++++++++++++++++++++
> > We also need corresponding ssdt-vmgenid.hex file for IASLless hosts,
> > so that build there won't fail.
> 
> Isn't that file is created during the build process?
> 
> For example:
> 
> $ git blame ./x86_64-softmmu/hw/i386/ssdt-tpm.hex
> fatal: no such path 'x86_64-softmmu/hw/i386/ssdt-tpm.hex' in HEAD
check out hw/i386/*.hex*

These blobs are updated/added manually every time corresponding dsl files changes.

> 
> > With this and below notes fixed
> > Reviewed-By: Igor Mammedov <imammedo@redhat.com>
> 
> Thanks. Although I do like that empty line at the end of a file :-).
Well, it's fine for me.

It's just that most of files do not do this, I've checked it by grepping
"type_init(" which is usually the last line in file.

> 
>     Gal.
> 
> > >  hw/misc/Makefile.objs              |  1 +
> > >  hw/misc/vmgenid.c                  | 85
> > >  ++++++++++++++++++++++++++++++++++++++
> > >  include/hw/i386/pc.h               |  3 ++
> > >  8 files changed, 195 insertions(+), 1 deletion(-)
> > >  create mode 100644 hw/i386/ssdt-vmgenid.dsl
> > >  create mode 100644 hw/misc/vmgenid.c
> > > 
> > > diff --git a/default-configs/i386-softmmu.mak
> > > b/default-configs/i386-softmmu.mak
> > > index 8e08841..bd33c75 100644
> > > --- a/default-configs/i386-softmmu.mak
> > > +++ b/default-configs/i386-softmmu.mak
> > > @@ -45,3 +45,4 @@ CONFIG_IOAPIC=y
> > >  CONFIG_ICC_BUS=y
> > >  CONFIG_PVPANIC=y
> > >  CONFIG_MEM_HOTPLUG=y
> > > +CONFIG_VMGENID=y
> > > diff --git a/default-configs/x86_64-softmmu.mak
> > > b/default-configs/x86_64-softmmu.mak
> > > index 66557ac..006fc7c 100644
> > > --- a/default-configs/x86_64-softmmu.mak
> > > +++ b/default-configs/x86_64-softmmu.mak
> > > @@ -45,3 +45,4 @@ CONFIG_IOAPIC=y
> > >  CONFIG_ICC_BUS=y
> > >  CONFIG_PVPANIC=y
> > >  CONFIG_MEM_HOTPLUG=y
> > > +CONFIG_VMGENID=y
> > > diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs
> > > index 9d419ad..cd1beb3 100644
> > > --- a/hw/i386/Makefile.objs
> > > +++ b/hw/i386/Makefile.objs
> > > @@ -12,7 +12,7 @@ hw/i386/acpi-build.o: hw/i386/acpi-build.c
> > > hw/i386/acpi-dsdt.hex \
> > >  	hw/i386/ssdt-proc.hex hw/i386/ssdt-pcihp.hex hw/i386/ssdt-misc.hex \
> > >  	hw/i386/acpi-dsdt.hex hw/i386/q35-acpi-dsdt.hex \
> > >  	hw/i386/q35-acpi-dsdt.hex hw/i386/ssdt-mem.hex \
> > > -	hw/i386/ssdt-tpm.hex
> > > +	hw/i386/ssdt-tpm.hex hw/i386/ssdt-vmgenid.hex
> > >  
> > >  iasl-option=$(shell if test -z "`$(1) $(2) 2>&1 > /dev/null`" \
> > >      ; then echo "$(2)"; else echo "$(3)"; fi ;)
> > > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> > > index a313321..72d5a88 100644
> > > --- a/hw/i386/acpi-build.c
> > > +++ b/hw/i386/acpi-build.c
> > > @@ -96,6 +96,8 @@ typedef struct AcpiMiscInfo {
> > >      const unsigned char *dsdt_code;
> > >      unsigned dsdt_size;
> > >      uint16_t pvpanic_port;
> > > +    bool vm_generation_id_set;
> > > +    uint8_t vm_generation_id[16];
> > >  } AcpiMiscInfo;
> > >  
> > >  typedef struct AcpiBuildPciBusHotplugState {
> > > @@ -216,6 +218,7 @@ static void acpi_get_misc_info(AcpiMiscInfo *info)
> > >      info->has_hpet = hpet_find();
> > >      info->has_tpm = tpm_find();
> > >      info->pvpanic_port = pvpanic_port();
> > > +    info->vm_generation_id_set = vm_generation_id(info->vm_generation_id);
> > >  }
> > >  
> > >  static void acpi_get_pci_info(PcPciInfo *info)
> > > @@ -710,6 +713,7 @@ static inline char acpi_get_hex(uint32_t val)
> > >  #include "hw/i386/ssdt-misc.hex"
> > >  #include "hw/i386/ssdt-pcihp.hex"
> > >  #include "hw/i386/ssdt-tpm.hex"
> > > +#include "hw/i386/ssdt-vmgenid.hex"
> > >  
> > >  static void
> > >  build_append_notify_method(GArray *device, const char *name,
> > > @@ -1246,6 +1250,37 @@ build_tpm_ssdt(GArray *table_data, GArray *linker)
> > >      memcpy(tpm_ptr, ssdt_tpm_aml, sizeof(ssdt_tpm_aml));
> > >  }
> > >  
> > > +static void
> > > +build_vmgenid_ssdt(GArray *table_data, GArray *linker, AcpiMiscInfo *info)
> > > +{
> > > +    int vgid_start = table_data->len;
> > > +    void *vgid_ptr;
> > > +    uint8_t *vm_gid_ptr;
> > > +    uint32_t vm_gid_physical_address;
> > > +
> > > +    vgid_ptr = acpi_data_push(table_data, sizeof(ssdt_vmgenid_aml));
> > > +    memcpy(vgid_ptr, ssdt_vmgenid_aml, sizeof(ssdt_vmgenid_aml));
> > > +
> > > +    vm_gid_ptr = acpi_data_get_ptr(vgid_ptr, sizeof(ssdt_vmgenid_aml),
> > > +                                   *ssdt_acpi_vm_gid,
> > > +                                   sizeof(info->vm_generation_id));
> > > +    memcpy(vm_gid_ptr, info->vm_generation_id,
> > > +           sizeof(info->vm_generation_id));
> > > +
> > > +    bios_linker_loader_add_pointer(linker, ACPI_BUILD_TABLE_FILE,
> > > +                                   ACPI_BUILD_TABLE_FILE,
> > > +                                   table_data,
> > > +                                   vgid_ptr + *ssdt_acpi_vm_gid_addr,
> > > +                                   sizeof(uint32_t));
> > > +
> > > +    vm_gid_physical_address = vgid_start + *ssdt_acpi_vm_gid;
> > > +    ACPI_BUILD_SET_LE(vgid_ptr, sizeof(ssdt_vmgenid_aml),
> > > +                      *ssdt_acpi_vm_gid_addr, 32,
> > > vm_gid_physical_address);
> > > +
> > > +    build_header(linker, table_data, vgid_ptr, "SSDT",
> > > +                 sizeof(ssdt_vmgenid_aml), 1);
> > > +}
> > > +
> > >  typedef enum {
> > >      MEM_AFFINITY_NOFLAGS      = 0,
> > >      MEM_AFFINITY_ENABLED      = (1 << 0),
> > > @@ -1617,6 +1652,10 @@ void acpi_build(PcGuestInfo *guest_info,
> > > AcpiBuildTables *tables)
> > >          acpi_add_table(table_offsets, tables->table_data);
> > >          build_tpm_ssdt(tables->table_data, tables->linker);
> > >      }
> > > +    if (misc.vm_generation_id_set) {
> > > +        acpi_add_table(table_offsets, tables->table_data);
> > > +        build_vmgenid_ssdt(tables->table_data, tables->linker, &misc);
> > > +    }
> > >      if (guest_info->numa_nodes) {
> > >          acpi_add_table(table_offsets, tables->table_data);
> > >          build_srat(tables->table_data, tables->linker, &cpu, guest_info);
> > > diff --git a/hw/i386/ssdt-vmgenid.dsl b/hw/i386/ssdt-vmgenid.dsl
> > > new file mode 100644
> > > index 0000000..54b5c34
> > > --- /dev/null
> > > +++ b/hw/i386/ssdt-vmgenid.dsl
> > > @@ -0,0 +1,64 @@
> > > +/*
> > > + * This program is free software; you can redistribute it and/or modify
> > > + * it under the terms of the GNU General Public License as published by
> > > + * the Free Software Foundation; either version 2 of the License, or
> > > + * (at your option) any later version.
> > > +
> > > + * This program is distributed in the hope that it will be useful,
> > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > > + * GNU General Public License for more details.
> > > +
> > > + * You should have received a copy of the GNU General Public License along
> > > + * with this program; if not, see <http://www.gnu.org/licenses/>.
> > > + */
> > > +
> > > +/****************************************************************
> > > + * Virtual Machine Generation ID Device
> > > + ****************************************************************/
> > > +
> > > +ACPI_EXTRACT_ALL_CODE ssdt_vmgenid_aml
> > > +
> > > +DefinitionBlock (
> > > +    "ssdt-vmgenid.aml", // Output Filename
> > > +    "SSDT",             // Signature
> > > +    0x01,               // SSDT Compliance Revision
> > > +    "BXPC",             // OEMID
> > > +    "BXSSDTSUSP",       // TABLE ID
> > > +    0x1                 // OEM Revision
> > > +    )
> > > +{
> > > +    Scope(\_SB) {
> > > +
> > > +        Device(VMGI) {
> > > +            Name(_HID, "QEMU0002")
> > > +            Name(_CID, "VM_Gen_Counter")
> > > +            Name(_DDN, "VM_Gen_Counter")
> > > +
> > > +            ACPI_EXTRACT_NAME_DWORD_CONST ssdt_acpi_vm_gid_addr
> > > +            Name(VGIA, 0x12345678)
> > > +
> > > +            ACPI_EXTRACT_NAME_BUFFER16 ssdt_acpi_vm_gid
> > > +            Name(VGID, Buffer(16) {
> > > +                0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> > > +                0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 })
> > > +
> > > +            Method(_STA, 0, NotSerialized) {
> > > +                Store(VGIA, Local0)
> > > +                If (LEqual(Local0, Zero)) {
> > > +                    Return (0x00)
> > > +                } Else {
> > > +                    Return (0x0F)
> > > +                }
> > > +            }
> > > +
> > > +            Method(ADDR, 0, Serialized) {
> > > +                Store(Package(2) { }, Local0)
> > > +                Store(VGIA, Index(Local0, 0))
> > > +                Store(0x0000, Index(Local0, 1))
> > > +                return (Local0)
> > > +            }
> > > +        }
> > > +    }
> > > +}
> > > +
> > nit, not needed white space
> > 
> > > diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
> > > index 979e532..c18b800 100644
> > > --- a/hw/misc/Makefile.objs
> > > +++ b/hw/misc/Makefile.objs
> > > @@ -41,3 +41,4 @@ obj-$(CONFIG_SLAVIO) += slavio_misc.o
> > >  obj-$(CONFIG_ZYNQ) += zynq_slcr.o
> > >  
> > >  obj-$(CONFIG_PVPANIC) += pvpanic.o
> > > +obj-$(CONFIG_VMGENID) += vmgenid.o
> > > diff --git a/hw/misc/vmgenid.c b/hw/misc/vmgenid.c
> > > new file mode 100644
> > > index 0000000..76956d1
> > > --- /dev/null
> > > +++ b/hw/misc/vmgenid.c
> > > @@ -0,0 +1,85 @@
> > > +/*
> > > + *  Virtual Machine Generation ID Device
> > > + *
> > > + *  Copyright (C) 2014 Red Hat Inc.
> > > + *
> > > + *  Authors: Gal Hammer <ghammer@redhat.com>
> > > + *
> > > + * This work is licensed under the terms of the GNU GPL, version 2 or
> > > later.
> > > + * See the COPYING file in the top-level directory.
> > > + *
> > > + */
> > > +
> > > +#include "hw/i386/pc.h"
> > > +#include "hw/sysbus.h"
> > > +
> > > +#define VMGENID_DEVICE "vmgenid"
> > > +
> > > +#define PROPERTY_UUID "uuid"
> > > +
> > > +#define VMGENID(obj) OBJECT_CHECK(VmGenIdState, (obj), VMGENID_DEVICE)
> > > +
> > > +typedef struct VmGenIdState {
> > > +    SysBusDevice parent_obj;
> > > +    char *guid_arg;
> > > +} VmGenIdState;
> > > +
> > > +bool vm_generation_id(uint8_t id[16])
> > > +{
> > > +    Object *o = object_resolve_path_type("", VMGENID_DEVICE, NULL);
> > > +    char *guid;
> > > +
> > > +    if (!o) {
> > > +        return false;
> > > +    }
> > > +    guid = object_property_get_str(o, PROPERTY_UUID, NULL);
> > > +    /* actual uuid validation was checked during realize. */
> > > +    (void)qemu_uuid_parse(guid, id);
> > > +    return true;
> > > +}
> > > +
> > > +static void vmgenid_realize(DeviceState *dev, Error **errp)
> > > +{
> > > +    VmGenIdState *s = VMGENID(dev);
> > > +    uint8_t id[16];
> > > +
> > > +    if (!s->guid_arg) {
> > > +        error_setg(errp, "missing uuid.");
> > > +        return;
> > > +    }
> > > +
> > > +    if (qemu_uuid_parse(s->guid_arg, id) < 0) {
> > > +        error_setg(errp, "Fail to parse UUID string.");
> > > +        return;
> > > +    }
> > > +}
> > > +
> > > +static Property vmgenid_device_properties[] = {
> > > +    DEFINE_PROP_STRING(PROPERTY_UUID, VmGenIdState, guid_arg),
> > > +    DEFINE_PROP_END_OF_LIST(),
> > > +};
> > > +
> > > +static void vmgenid_class_init(ObjectClass *klass, void *data)
> > > +{
> > > +    DeviceClass *dc = DEVICE_CLASS(klass);
> > > +
> > > +    dc->realize = vmgenid_realize;
> > > +    dc->props = vmgenid_device_properties;
> > > +    dc->cannot_instantiate_with_device_add_yet = false;
> > > +    set_bit(DEVICE_CATEGORY_MISC, dc->categories);
> > > +}
> > > +
> > > +static const TypeInfo vmgenid_device_info = {
> > > +    .name          = VMGENID_DEVICE,
> > > +    .parent        = TYPE_SYS_BUS_DEVICE,
> > > +    .instance_size = sizeof(VmGenIdState),
> > > +    .class_init    = vmgenid_class_init,
> > > +};
> > > +
> > > +static void vmgenid_register_types(void)
> > > +{
> > > +    type_register_static(&vmgenid_device_info);
> > > +}
> > > +
> > > +type_init(vmgenid_register_types)
> > > +
> > ditto
> > 
> > > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> > > index 77316d5..40ecccb 100644
> > > --- a/include/hw/i386/pc.h
> > > +++ b/include/hw/i386/pc.h
> > > @@ -290,6 +290,9 @@ void pc_system_firmware_init(MemoryRegion *rom_memory,
> > >  /* pvpanic.c */
> > >  uint16_t pvpanic_port(void);
> > >  
> > > +/* vmgenid.c */
> > > +bool vm_generation_id(uint8_t id[16]);
> > > +
> > >  /* e820 types */
> > >  #define E820_RAM        1
> > >  #define E820_RESERVED   2
> > 
> > 
> > 

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [Qemu-devel] [PATCH 2/2] i386: Add a Virtual Machine Generation ID device.
  2014-09-16 14:54   ` Igor Mammedov
@ 2014-09-17  7:36     ` Gal Hammer
  2014-09-17  9:56       ` Igor Mammedov
  0 siblings, 1 reply; 28+ messages in thread
From: Gal Hammer @ 2014-09-17  7:36 UTC (permalink / raw)
  To: Igor Mammedov; +Cc: pbonzini, qemu-devel

Hi Igor,

----- Original Message -----
> From: "Igor Mammedov" <imammedo@redhat.com>
> To: "Gal Hammer" <ghammer@redhat.com>
> Cc: pbonzini@redhat.com, qemu-devel@nongnu.org
> Sent: Tuesday, September 16, 2014 5:54:28 PM
> Subject: Re: [Qemu-devel] [PATCH 2/2] i386: Add a Virtual Machine Generation ID device.
> 
> On Tue, 16 Sep 2014 16:04:10 +0300
> Gal Hammer <ghammer@redhat.com> wrote:
> 
> > Based on Microsoft's sepecifications (paper can be dowloaded from
> > http://go.microsoft.com/fwlink/?LinkId=260709), add a device
> > description to the SSDT ACPI table.
> > 
> > The GUID is set using a new "vmgenid" device.
> > 
> > Signed-off-by: Gal Hammer <ghammer@redhat.com>
> > ---
> >  default-configs/i386-softmmu.mak   |  1 +
> >  default-configs/x86_64-softmmu.mak |  1 +
> >  hw/i386/Makefile.objs              |  2 +-
> >  hw/i386/acpi-build.c               | 39 +++++++++++++++++
> >  hw/i386/ssdt-vmgenid.dsl           | 64 ++++++++++++++++++++++++++++
> We also need corresponding ssdt-vmgenid.hex file for IASLless hosts,
> so that build there won't fail.

Isn't that file is created during the build process?

For example:

$ git blame ./x86_64-softmmu/hw/i386/ssdt-tpm.hex
fatal: no such path 'x86_64-softmmu/hw/i386/ssdt-tpm.hex' in HEAD

> With this and below notes fixed
> Reviewed-By: Igor Mammedov <imammedo@redhat.com>

Thanks. Although I do like that empty line at the end of a file :-).

    Gal.

> >  hw/misc/Makefile.objs              |  1 +
> >  hw/misc/vmgenid.c                  | 85
> >  ++++++++++++++++++++++++++++++++++++++
> >  include/hw/i386/pc.h               |  3 ++
> >  8 files changed, 195 insertions(+), 1 deletion(-)
> >  create mode 100644 hw/i386/ssdt-vmgenid.dsl
> >  create mode 100644 hw/misc/vmgenid.c
> > 
> > diff --git a/default-configs/i386-softmmu.mak
> > b/default-configs/i386-softmmu.mak
> > index 8e08841..bd33c75 100644
> > --- a/default-configs/i386-softmmu.mak
> > +++ b/default-configs/i386-softmmu.mak
> > @@ -45,3 +45,4 @@ CONFIG_IOAPIC=y
> >  CONFIG_ICC_BUS=y
> >  CONFIG_PVPANIC=y
> >  CONFIG_MEM_HOTPLUG=y
> > +CONFIG_VMGENID=y
> > diff --git a/default-configs/x86_64-softmmu.mak
> > b/default-configs/x86_64-softmmu.mak
> > index 66557ac..006fc7c 100644
> > --- a/default-configs/x86_64-softmmu.mak
> > +++ b/default-configs/x86_64-softmmu.mak
> > @@ -45,3 +45,4 @@ CONFIG_IOAPIC=y
> >  CONFIG_ICC_BUS=y
> >  CONFIG_PVPANIC=y
> >  CONFIG_MEM_HOTPLUG=y
> > +CONFIG_VMGENID=y
> > diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs
> > index 9d419ad..cd1beb3 100644
> > --- a/hw/i386/Makefile.objs
> > +++ b/hw/i386/Makefile.objs
> > @@ -12,7 +12,7 @@ hw/i386/acpi-build.o: hw/i386/acpi-build.c
> > hw/i386/acpi-dsdt.hex \
> >  	hw/i386/ssdt-proc.hex hw/i386/ssdt-pcihp.hex hw/i386/ssdt-misc.hex \
> >  	hw/i386/acpi-dsdt.hex hw/i386/q35-acpi-dsdt.hex \
> >  	hw/i386/q35-acpi-dsdt.hex hw/i386/ssdt-mem.hex \
> > -	hw/i386/ssdt-tpm.hex
> > +	hw/i386/ssdt-tpm.hex hw/i386/ssdt-vmgenid.hex
> >  
> >  iasl-option=$(shell if test -z "`$(1) $(2) 2>&1 > /dev/null`" \
> >      ; then echo "$(2)"; else echo "$(3)"; fi ;)
> > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> > index a313321..72d5a88 100644
> > --- a/hw/i386/acpi-build.c
> > +++ b/hw/i386/acpi-build.c
> > @@ -96,6 +96,8 @@ typedef struct AcpiMiscInfo {
> >      const unsigned char *dsdt_code;
> >      unsigned dsdt_size;
> >      uint16_t pvpanic_port;
> > +    bool vm_generation_id_set;
> > +    uint8_t vm_generation_id[16];
> >  } AcpiMiscInfo;
> >  
> >  typedef struct AcpiBuildPciBusHotplugState {
> > @@ -216,6 +218,7 @@ static void acpi_get_misc_info(AcpiMiscInfo *info)
> >      info->has_hpet = hpet_find();
> >      info->has_tpm = tpm_find();
> >      info->pvpanic_port = pvpanic_port();
> > +    info->vm_generation_id_set = vm_generation_id(info->vm_generation_id);
> >  }
> >  
> >  static void acpi_get_pci_info(PcPciInfo *info)
> > @@ -710,6 +713,7 @@ static inline char acpi_get_hex(uint32_t val)
> >  #include "hw/i386/ssdt-misc.hex"
> >  #include "hw/i386/ssdt-pcihp.hex"
> >  #include "hw/i386/ssdt-tpm.hex"
> > +#include "hw/i386/ssdt-vmgenid.hex"
> >  
> >  static void
> >  build_append_notify_method(GArray *device, const char *name,
> > @@ -1246,6 +1250,37 @@ build_tpm_ssdt(GArray *table_data, GArray *linker)
> >      memcpy(tpm_ptr, ssdt_tpm_aml, sizeof(ssdt_tpm_aml));
> >  }
> >  
> > +static void
> > +build_vmgenid_ssdt(GArray *table_data, GArray *linker, AcpiMiscInfo *info)
> > +{
> > +    int vgid_start = table_data->len;
> > +    void *vgid_ptr;
> > +    uint8_t *vm_gid_ptr;
> > +    uint32_t vm_gid_physical_address;
> > +
> > +    vgid_ptr = acpi_data_push(table_data, sizeof(ssdt_vmgenid_aml));
> > +    memcpy(vgid_ptr, ssdt_vmgenid_aml, sizeof(ssdt_vmgenid_aml));
> > +
> > +    vm_gid_ptr = acpi_data_get_ptr(vgid_ptr, sizeof(ssdt_vmgenid_aml),
> > +                                   *ssdt_acpi_vm_gid,
> > +                                   sizeof(info->vm_generation_id));
> > +    memcpy(vm_gid_ptr, info->vm_generation_id,
> > +           sizeof(info->vm_generation_id));
> > +
> > +    bios_linker_loader_add_pointer(linker, ACPI_BUILD_TABLE_FILE,
> > +                                   ACPI_BUILD_TABLE_FILE,
> > +                                   table_data,
> > +                                   vgid_ptr + *ssdt_acpi_vm_gid_addr,
> > +                                   sizeof(uint32_t));
> > +
> > +    vm_gid_physical_address = vgid_start + *ssdt_acpi_vm_gid;
> > +    ACPI_BUILD_SET_LE(vgid_ptr, sizeof(ssdt_vmgenid_aml),
> > +                      *ssdt_acpi_vm_gid_addr, 32,
> > vm_gid_physical_address);
> > +
> > +    build_header(linker, table_data, vgid_ptr, "SSDT",
> > +                 sizeof(ssdt_vmgenid_aml), 1);
> > +}
> > +
> >  typedef enum {
> >      MEM_AFFINITY_NOFLAGS      = 0,
> >      MEM_AFFINITY_ENABLED      = (1 << 0),
> > @@ -1617,6 +1652,10 @@ void acpi_build(PcGuestInfo *guest_info,
> > AcpiBuildTables *tables)
> >          acpi_add_table(table_offsets, tables->table_data);
> >          build_tpm_ssdt(tables->table_data, tables->linker);
> >      }
> > +    if (misc.vm_generation_id_set) {
> > +        acpi_add_table(table_offsets, tables->table_data);
> > +        build_vmgenid_ssdt(tables->table_data, tables->linker, &misc);
> > +    }
> >      if (guest_info->numa_nodes) {
> >          acpi_add_table(table_offsets, tables->table_data);
> >          build_srat(tables->table_data, tables->linker, &cpu, guest_info);
> > diff --git a/hw/i386/ssdt-vmgenid.dsl b/hw/i386/ssdt-vmgenid.dsl
> > new file mode 100644
> > index 0000000..54b5c34
> > --- /dev/null
> > +++ b/hw/i386/ssdt-vmgenid.dsl
> > @@ -0,0 +1,64 @@
> > +/*
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation; either version 2 of the License, or
> > + * (at your option) any later version.
> > +
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > + * GNU General Public License for more details.
> > +
> > + * You should have received a copy of the GNU General Public License along
> > + * with this program; if not, see <http://www.gnu.org/licenses/>.
> > + */
> > +
> > +/****************************************************************
> > + * Virtual Machine Generation ID Device
> > + ****************************************************************/
> > +
> > +ACPI_EXTRACT_ALL_CODE ssdt_vmgenid_aml
> > +
> > +DefinitionBlock (
> > +    "ssdt-vmgenid.aml", // Output Filename
> > +    "SSDT",             // Signature
> > +    0x01,               // SSDT Compliance Revision
> > +    "BXPC",             // OEMID
> > +    "BXSSDTSUSP",       // TABLE ID
> > +    0x1                 // OEM Revision
> > +    )
> > +{
> > +    Scope(\_SB) {
> > +
> > +        Device(VMGI) {
> > +            Name(_HID, "QEMU0002")
> > +            Name(_CID, "VM_Gen_Counter")
> > +            Name(_DDN, "VM_Gen_Counter")
> > +
> > +            ACPI_EXTRACT_NAME_DWORD_CONST ssdt_acpi_vm_gid_addr
> > +            Name(VGIA, 0x12345678)
> > +
> > +            ACPI_EXTRACT_NAME_BUFFER16 ssdt_acpi_vm_gid
> > +            Name(VGID, Buffer(16) {
> > +                0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> > +                0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 })
> > +
> > +            Method(_STA, 0, NotSerialized) {
> > +                Store(VGIA, Local0)
> > +                If (LEqual(Local0, Zero)) {
> > +                    Return (0x00)
> > +                } Else {
> > +                    Return (0x0F)
> > +                }
> > +            }
> > +
> > +            Method(ADDR, 0, Serialized) {
> > +                Store(Package(2) { }, Local0)
> > +                Store(VGIA, Index(Local0, 0))
> > +                Store(0x0000, Index(Local0, 1))
> > +                return (Local0)
> > +            }
> > +        }
> > +    }
> > +}
> > +
> nit, not needed white space
> 
> > diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
> > index 979e532..c18b800 100644
> > --- a/hw/misc/Makefile.objs
> > +++ b/hw/misc/Makefile.objs
> > @@ -41,3 +41,4 @@ obj-$(CONFIG_SLAVIO) += slavio_misc.o
> >  obj-$(CONFIG_ZYNQ) += zynq_slcr.o
> >  
> >  obj-$(CONFIG_PVPANIC) += pvpanic.o
> > +obj-$(CONFIG_VMGENID) += vmgenid.o
> > diff --git a/hw/misc/vmgenid.c b/hw/misc/vmgenid.c
> > new file mode 100644
> > index 0000000..76956d1
> > --- /dev/null
> > +++ b/hw/misc/vmgenid.c
> > @@ -0,0 +1,85 @@
> > +/*
> > + *  Virtual Machine Generation ID Device
> > + *
> > + *  Copyright (C) 2014 Red Hat Inc.
> > + *
> > + *  Authors: Gal Hammer <ghammer@redhat.com>
> > + *
> > + * This work is licensed under the terms of the GNU GPL, version 2 or
> > later.
> > + * See the COPYING file in the top-level directory.
> > + *
> > + */
> > +
> > +#include "hw/i386/pc.h"
> > +#include "hw/sysbus.h"
> > +
> > +#define VMGENID_DEVICE "vmgenid"
> > +
> > +#define PROPERTY_UUID "uuid"
> > +
> > +#define VMGENID(obj) OBJECT_CHECK(VmGenIdState, (obj), VMGENID_DEVICE)
> > +
> > +typedef struct VmGenIdState {
> > +    SysBusDevice parent_obj;
> > +    char *guid_arg;
> > +} VmGenIdState;
> > +
> > +bool vm_generation_id(uint8_t id[16])
> > +{
> > +    Object *o = object_resolve_path_type("", VMGENID_DEVICE, NULL);
> > +    char *guid;
> > +
> > +    if (!o) {
> > +        return false;
> > +    }
> > +    guid = object_property_get_str(o, PROPERTY_UUID, NULL);
> > +    /* actual uuid validation was checked during realize. */
> > +    (void)qemu_uuid_parse(guid, id);
> > +    return true;
> > +}
> > +
> > +static void vmgenid_realize(DeviceState *dev, Error **errp)
> > +{
> > +    VmGenIdState *s = VMGENID(dev);
> > +    uint8_t id[16];
> > +
> > +    if (!s->guid_arg) {
> > +        error_setg(errp, "missing uuid.");
> > +        return;
> > +    }
> > +
> > +    if (qemu_uuid_parse(s->guid_arg, id) < 0) {
> > +        error_setg(errp, "Fail to parse UUID string.");
> > +        return;
> > +    }
> > +}
> > +
> > +static Property vmgenid_device_properties[] = {
> > +    DEFINE_PROP_STRING(PROPERTY_UUID, VmGenIdState, guid_arg),
> > +    DEFINE_PROP_END_OF_LIST(),
> > +};
> > +
> > +static void vmgenid_class_init(ObjectClass *klass, void *data)
> > +{
> > +    DeviceClass *dc = DEVICE_CLASS(klass);
> > +
> > +    dc->realize = vmgenid_realize;
> > +    dc->props = vmgenid_device_properties;
> > +    dc->cannot_instantiate_with_device_add_yet = false;
> > +    set_bit(DEVICE_CATEGORY_MISC, dc->categories);
> > +}
> > +
> > +static const TypeInfo vmgenid_device_info = {
> > +    .name          = VMGENID_DEVICE,
> > +    .parent        = TYPE_SYS_BUS_DEVICE,
> > +    .instance_size = sizeof(VmGenIdState),
> > +    .class_init    = vmgenid_class_init,
> > +};
> > +
> > +static void vmgenid_register_types(void)
> > +{
> > +    type_register_static(&vmgenid_device_info);
> > +}
> > +
> > +type_init(vmgenid_register_types)
> > +
> ditto
> 
> > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> > index 77316d5..40ecccb 100644
> > --- a/include/hw/i386/pc.h
> > +++ b/include/hw/i386/pc.h
> > @@ -290,6 +290,9 @@ void pc_system_firmware_init(MemoryRegion *rom_memory,
> >  /* pvpanic.c */
> >  uint16_t pvpanic_port(void);
> >  
> > +/* vmgenid.c */
> > +bool vm_generation_id(uint8_t id[16]);
> > +
> >  /* e820 types */
> >  #define E820_RAM        1
> >  #define E820_RESERVED   2
> 
> 
> 

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [Qemu-devel] [PATCH 2/2] i386: Add a Virtual Machine Generation ID device.
  2014-09-16 13:04 ` [Qemu-devel] [PATCH 2/2] i386: Add a Virtual Machine Generation ID device Gal Hammer
  2014-09-16 13:16   ` Paolo Bonzini
@ 2014-09-16 14:54   ` Igor Mammedov
  2014-09-17  7:36     ` Gal Hammer
  1 sibling, 1 reply; 28+ messages in thread
From: Igor Mammedov @ 2014-09-16 14:54 UTC (permalink / raw)
  To: Gal Hammer; +Cc: pbonzini, qemu-devel

On Tue, 16 Sep 2014 16:04:10 +0300
Gal Hammer <ghammer@redhat.com> wrote:

> Based on Microsoft's sepecifications (paper can be dowloaded from
> http://go.microsoft.com/fwlink/?LinkId=260709), add a device
> description to the SSDT ACPI table.
> 
> The GUID is set using a new "vmgenid" device.
> 
> Signed-off-by: Gal Hammer <ghammer@redhat.com>
> ---
>  default-configs/i386-softmmu.mak   |  1 +
>  default-configs/x86_64-softmmu.mak |  1 +
>  hw/i386/Makefile.objs              |  2 +-
>  hw/i386/acpi-build.c               | 39 +++++++++++++++++
>  hw/i386/ssdt-vmgenid.dsl           | 64 ++++++++++++++++++++++++++++
We also need corresponding ssdt-vmgenid.hex file for IASLless hosts,
so that build there won't fail.

With this and below notes fixed
Reviewed-By: Igor Mammedov <imammedo@redhat.com>

>  hw/misc/Makefile.objs              |  1 +
>  hw/misc/vmgenid.c                  | 85 ++++++++++++++++++++++++++++++++++++++
>  include/hw/i386/pc.h               |  3 ++
>  8 files changed, 195 insertions(+), 1 deletion(-)
>  create mode 100644 hw/i386/ssdt-vmgenid.dsl
>  create mode 100644 hw/misc/vmgenid.c
> 
> diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak
> index 8e08841..bd33c75 100644
> --- a/default-configs/i386-softmmu.mak
> +++ b/default-configs/i386-softmmu.mak
> @@ -45,3 +45,4 @@ CONFIG_IOAPIC=y
>  CONFIG_ICC_BUS=y
>  CONFIG_PVPANIC=y
>  CONFIG_MEM_HOTPLUG=y
> +CONFIG_VMGENID=y
> diff --git a/default-configs/x86_64-softmmu.mak b/default-configs/x86_64-softmmu.mak
> index 66557ac..006fc7c 100644
> --- a/default-configs/x86_64-softmmu.mak
> +++ b/default-configs/x86_64-softmmu.mak
> @@ -45,3 +45,4 @@ CONFIG_IOAPIC=y
>  CONFIG_ICC_BUS=y
>  CONFIG_PVPANIC=y
>  CONFIG_MEM_HOTPLUG=y
> +CONFIG_VMGENID=y
> diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs
> index 9d419ad..cd1beb3 100644
> --- a/hw/i386/Makefile.objs
> +++ b/hw/i386/Makefile.objs
> @@ -12,7 +12,7 @@ hw/i386/acpi-build.o: hw/i386/acpi-build.c hw/i386/acpi-dsdt.hex \
>  	hw/i386/ssdt-proc.hex hw/i386/ssdt-pcihp.hex hw/i386/ssdt-misc.hex \
>  	hw/i386/acpi-dsdt.hex hw/i386/q35-acpi-dsdt.hex \
>  	hw/i386/q35-acpi-dsdt.hex hw/i386/ssdt-mem.hex \
> -	hw/i386/ssdt-tpm.hex
> +	hw/i386/ssdt-tpm.hex hw/i386/ssdt-vmgenid.hex
>  
>  iasl-option=$(shell if test -z "`$(1) $(2) 2>&1 > /dev/null`" \
>      ; then echo "$(2)"; else echo "$(3)"; fi ;)
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index a313321..72d5a88 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -96,6 +96,8 @@ typedef struct AcpiMiscInfo {
>      const unsigned char *dsdt_code;
>      unsigned dsdt_size;
>      uint16_t pvpanic_port;
> +    bool vm_generation_id_set;
> +    uint8_t vm_generation_id[16];
>  } AcpiMiscInfo;
>  
>  typedef struct AcpiBuildPciBusHotplugState {
> @@ -216,6 +218,7 @@ static void acpi_get_misc_info(AcpiMiscInfo *info)
>      info->has_hpet = hpet_find();
>      info->has_tpm = tpm_find();
>      info->pvpanic_port = pvpanic_port();
> +    info->vm_generation_id_set = vm_generation_id(info->vm_generation_id);
>  }
>  
>  static void acpi_get_pci_info(PcPciInfo *info)
> @@ -710,6 +713,7 @@ static inline char acpi_get_hex(uint32_t val)
>  #include "hw/i386/ssdt-misc.hex"
>  #include "hw/i386/ssdt-pcihp.hex"
>  #include "hw/i386/ssdt-tpm.hex"
> +#include "hw/i386/ssdt-vmgenid.hex"
>  
>  static void
>  build_append_notify_method(GArray *device, const char *name,
> @@ -1246,6 +1250,37 @@ build_tpm_ssdt(GArray *table_data, GArray *linker)
>      memcpy(tpm_ptr, ssdt_tpm_aml, sizeof(ssdt_tpm_aml));
>  }
>  
> +static void
> +build_vmgenid_ssdt(GArray *table_data, GArray *linker, AcpiMiscInfo *info)
> +{
> +    int vgid_start = table_data->len;
> +    void *vgid_ptr;
> +    uint8_t *vm_gid_ptr;
> +    uint32_t vm_gid_physical_address;
> +
> +    vgid_ptr = acpi_data_push(table_data, sizeof(ssdt_vmgenid_aml));
> +    memcpy(vgid_ptr, ssdt_vmgenid_aml, sizeof(ssdt_vmgenid_aml));
> +
> +    vm_gid_ptr = acpi_data_get_ptr(vgid_ptr, sizeof(ssdt_vmgenid_aml),
> +                                   *ssdt_acpi_vm_gid,
> +                                   sizeof(info->vm_generation_id));
> +    memcpy(vm_gid_ptr, info->vm_generation_id,
> +           sizeof(info->vm_generation_id));
> +
> +    bios_linker_loader_add_pointer(linker, ACPI_BUILD_TABLE_FILE,
> +                                   ACPI_BUILD_TABLE_FILE,
> +                                   table_data,
> +                                   vgid_ptr + *ssdt_acpi_vm_gid_addr,
> +                                   sizeof(uint32_t));
> +
> +    vm_gid_physical_address = vgid_start + *ssdt_acpi_vm_gid;
> +    ACPI_BUILD_SET_LE(vgid_ptr, sizeof(ssdt_vmgenid_aml),
> +                      *ssdt_acpi_vm_gid_addr, 32, vm_gid_physical_address);
> +
> +    build_header(linker, table_data, vgid_ptr, "SSDT",
> +                 sizeof(ssdt_vmgenid_aml), 1);
> +}
> +
>  typedef enum {
>      MEM_AFFINITY_NOFLAGS      = 0,
>      MEM_AFFINITY_ENABLED      = (1 << 0),
> @@ -1617,6 +1652,10 @@ void acpi_build(PcGuestInfo *guest_info, AcpiBuildTables *tables)
>          acpi_add_table(table_offsets, tables->table_data);
>          build_tpm_ssdt(tables->table_data, tables->linker);
>      }
> +    if (misc.vm_generation_id_set) {
> +        acpi_add_table(table_offsets, tables->table_data);
> +        build_vmgenid_ssdt(tables->table_data, tables->linker, &misc);
> +    }
>      if (guest_info->numa_nodes) {
>          acpi_add_table(table_offsets, tables->table_data);
>          build_srat(tables->table_data, tables->linker, &cpu, guest_info);
> diff --git a/hw/i386/ssdt-vmgenid.dsl b/hw/i386/ssdt-vmgenid.dsl
> new file mode 100644
> index 0000000..54b5c34
> --- /dev/null
> +++ b/hw/i386/ssdt-vmgenid.dsl
> @@ -0,0 +1,64 @@
> +/*
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> +
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> +
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +/****************************************************************
> + * Virtual Machine Generation ID Device
> + ****************************************************************/
> +
> +ACPI_EXTRACT_ALL_CODE ssdt_vmgenid_aml
> +
> +DefinitionBlock (
> +    "ssdt-vmgenid.aml", // Output Filename
> +    "SSDT",             // Signature
> +    0x01,               // SSDT Compliance Revision
> +    "BXPC",             // OEMID
> +    "BXSSDTSUSP",       // TABLE ID
> +    0x1                 // OEM Revision
> +    )
> +{
> +    Scope(\_SB) {
> +
> +        Device(VMGI) {
> +            Name(_HID, "QEMU0002")
> +            Name(_CID, "VM_Gen_Counter")
> +            Name(_DDN, "VM_Gen_Counter")
> +
> +            ACPI_EXTRACT_NAME_DWORD_CONST ssdt_acpi_vm_gid_addr
> +            Name(VGIA, 0x12345678)
> +
> +            ACPI_EXTRACT_NAME_BUFFER16 ssdt_acpi_vm_gid
> +            Name(VGID, Buffer(16) {
> +                0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +                0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 })
> +
> +            Method(_STA, 0, NotSerialized) {
> +                Store(VGIA, Local0)
> +                If (LEqual(Local0, Zero)) {
> +                    Return (0x00)
> +                } Else {
> +                    Return (0x0F)
> +                }
> +            }
> +
> +            Method(ADDR, 0, Serialized) {
> +                Store(Package(2) { }, Local0)
> +                Store(VGIA, Index(Local0, 0))
> +                Store(0x0000, Index(Local0, 1))
> +                return (Local0)
> +            }
> +        }
> +    }
> +}
> +
nit, not needed white space

> diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
> index 979e532..c18b800 100644
> --- a/hw/misc/Makefile.objs
> +++ b/hw/misc/Makefile.objs
> @@ -41,3 +41,4 @@ obj-$(CONFIG_SLAVIO) += slavio_misc.o
>  obj-$(CONFIG_ZYNQ) += zynq_slcr.o
>  
>  obj-$(CONFIG_PVPANIC) += pvpanic.o
> +obj-$(CONFIG_VMGENID) += vmgenid.o
> diff --git a/hw/misc/vmgenid.c b/hw/misc/vmgenid.c
> new file mode 100644
> index 0000000..76956d1
> --- /dev/null
> +++ b/hw/misc/vmgenid.c
> @@ -0,0 +1,85 @@
> +/*
> + *  Virtual Machine Generation ID Device
> + *
> + *  Copyright (C) 2014 Red Hat Inc.
> + *
> + *  Authors: Gal Hammer <ghammer@redhat.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + *
> + */
> +
> +#include "hw/i386/pc.h"
> +#include "hw/sysbus.h"
> +
> +#define VMGENID_DEVICE "vmgenid"
> +
> +#define PROPERTY_UUID "uuid"
> +
> +#define VMGENID(obj) OBJECT_CHECK(VmGenIdState, (obj), VMGENID_DEVICE)
> +
> +typedef struct VmGenIdState {
> +    SysBusDevice parent_obj;
> +    char *guid_arg;
> +} VmGenIdState;
> +
> +bool vm_generation_id(uint8_t id[16])
> +{
> +    Object *o = object_resolve_path_type("", VMGENID_DEVICE, NULL);
> +    char *guid;
> +
> +    if (!o) {
> +        return false;
> +    }
> +    guid = object_property_get_str(o, PROPERTY_UUID, NULL);
> +    /* actual uuid validation was checked during realize. */
> +    (void)qemu_uuid_parse(guid, id);
> +    return true;
> +}
> +
> +static void vmgenid_realize(DeviceState *dev, Error **errp)
> +{
> +    VmGenIdState *s = VMGENID(dev);
> +    uint8_t id[16];
> +
> +    if (!s->guid_arg) {
> +        error_setg(errp, "missing uuid.");
> +        return;
> +    }
> +
> +    if (qemu_uuid_parse(s->guid_arg, id) < 0) {
> +        error_setg(errp, "Fail to parse UUID string.");
> +        return;
> +    }
> +}
> +
> +static Property vmgenid_device_properties[] = {
> +    DEFINE_PROP_STRING(PROPERTY_UUID, VmGenIdState, guid_arg),
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
> +static void vmgenid_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +
> +    dc->realize = vmgenid_realize;
> +    dc->props = vmgenid_device_properties;
> +    dc->cannot_instantiate_with_device_add_yet = false;
> +    set_bit(DEVICE_CATEGORY_MISC, dc->categories);
> +}
> +
> +static const TypeInfo vmgenid_device_info = {
> +    .name          = VMGENID_DEVICE,
> +    .parent        = TYPE_SYS_BUS_DEVICE,
> +    .instance_size = sizeof(VmGenIdState),
> +    .class_init    = vmgenid_class_init,
> +};
> +
> +static void vmgenid_register_types(void)
> +{
> +    type_register_static(&vmgenid_device_info);
> +}
> +
> +type_init(vmgenid_register_types)
> +
ditto

> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 77316d5..40ecccb 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -290,6 +290,9 @@ void pc_system_firmware_init(MemoryRegion *rom_memory,
>  /* pvpanic.c */
>  uint16_t pvpanic_port(void);
>  
> +/* vmgenid.c */
> +bool vm_generation_id(uint8_t id[16]);
> +
>  /* e820 types */
>  #define E820_RAM        1
>  #define E820_RESERVED   2

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [Qemu-devel] [PATCH 2/2] i386: Add a Virtual Machine Generation ID device.
  2014-09-16 13:04 ` [Qemu-devel] [PATCH 2/2] i386: Add a Virtual Machine Generation ID device Gal Hammer
@ 2014-09-16 13:16   ` Paolo Bonzini
  2014-09-16 14:54   ` Igor Mammedov
  1 sibling, 0 replies; 28+ messages in thread
From: Paolo Bonzini @ 2014-09-16 13:16 UTC (permalink / raw)
  To: Gal Hammer, qemu-devel

Il 16/09/2014 15:04, Gal Hammer ha scritto:
> Based on Microsoft's sepecifications (paper can be dowloaded from
> http://go.microsoft.com/fwlink/?LinkId=260709), add a device
> description to the SSDT ACPI table.
> 
> The GUID is set using a new "vmgenid" device.
> 
> Signed-off-by: Gal Hammer <ghammer@redhat.com>
> ---
>  default-configs/i386-softmmu.mak   |  1 +
>  default-configs/x86_64-softmmu.mak |  1 +
>  hw/i386/Makefile.objs              |  2 +-
>  hw/i386/acpi-build.c               | 39 +++++++++++++++++
>  hw/i386/ssdt-vmgenid.dsl           | 64 ++++++++++++++++++++++++++++
>  hw/misc/Makefile.objs              |  1 +
>  hw/misc/vmgenid.c                  | 85 ++++++++++++++++++++++++++++++++++++++
>  include/hw/i386/pc.h               |  3 ++
>  8 files changed, 195 insertions(+), 1 deletion(-)
>  create mode 100644 hw/i386/ssdt-vmgenid.dsl
>  create mode 100644 hw/misc/vmgenid.c
> 
> diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak
> index 8e08841..bd33c75 100644
> --- a/default-configs/i386-softmmu.mak
> +++ b/default-configs/i386-softmmu.mak
> @@ -45,3 +45,4 @@ CONFIG_IOAPIC=y
>  CONFIG_ICC_BUS=y
>  CONFIG_PVPANIC=y
>  CONFIG_MEM_HOTPLUG=y
> +CONFIG_VMGENID=y
> diff --git a/default-configs/x86_64-softmmu.mak b/default-configs/x86_64-softmmu.mak
> index 66557ac..006fc7c 100644
> --- a/default-configs/x86_64-softmmu.mak
> +++ b/default-configs/x86_64-softmmu.mak
> @@ -45,3 +45,4 @@ CONFIG_IOAPIC=y
>  CONFIG_ICC_BUS=y
>  CONFIG_PVPANIC=y
>  CONFIG_MEM_HOTPLUG=y
> +CONFIG_VMGENID=y
> diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs
> index 9d419ad..cd1beb3 100644
> --- a/hw/i386/Makefile.objs
> +++ b/hw/i386/Makefile.objs
> @@ -12,7 +12,7 @@ hw/i386/acpi-build.o: hw/i386/acpi-build.c hw/i386/acpi-dsdt.hex \
>  	hw/i386/ssdt-proc.hex hw/i386/ssdt-pcihp.hex hw/i386/ssdt-misc.hex \
>  	hw/i386/acpi-dsdt.hex hw/i386/q35-acpi-dsdt.hex \
>  	hw/i386/q35-acpi-dsdt.hex hw/i386/ssdt-mem.hex \
> -	hw/i386/ssdt-tpm.hex
> +	hw/i386/ssdt-tpm.hex hw/i386/ssdt-vmgenid.hex
>  
>  iasl-option=$(shell if test -z "`$(1) $(2) 2>&1 > /dev/null`" \
>      ; then echo "$(2)"; else echo "$(3)"; fi ;)
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index a313321..72d5a88 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -96,6 +96,8 @@ typedef struct AcpiMiscInfo {
>      const unsigned char *dsdt_code;
>      unsigned dsdt_size;
>      uint16_t pvpanic_port;
> +    bool vm_generation_id_set;
> +    uint8_t vm_generation_id[16];
>  } AcpiMiscInfo;
>  
>  typedef struct AcpiBuildPciBusHotplugState {
> @@ -216,6 +218,7 @@ static void acpi_get_misc_info(AcpiMiscInfo *info)
>      info->has_hpet = hpet_find();
>      info->has_tpm = tpm_find();
>      info->pvpanic_port = pvpanic_port();
> +    info->vm_generation_id_set = vm_generation_id(info->vm_generation_id);
>  }
>  
>  static void acpi_get_pci_info(PcPciInfo *info)
> @@ -710,6 +713,7 @@ static inline char acpi_get_hex(uint32_t val)
>  #include "hw/i386/ssdt-misc.hex"
>  #include "hw/i386/ssdt-pcihp.hex"
>  #include "hw/i386/ssdt-tpm.hex"
> +#include "hw/i386/ssdt-vmgenid.hex"
>  
>  static void
>  build_append_notify_method(GArray *device, const char *name,
> @@ -1246,6 +1250,37 @@ build_tpm_ssdt(GArray *table_data, GArray *linker)
>      memcpy(tpm_ptr, ssdt_tpm_aml, sizeof(ssdt_tpm_aml));
>  }
>  
> +static void
> +build_vmgenid_ssdt(GArray *table_data, GArray *linker, AcpiMiscInfo *info)
> +{
> +    int vgid_start = table_data->len;
> +    void *vgid_ptr;
> +    uint8_t *vm_gid_ptr;
> +    uint32_t vm_gid_physical_address;
> +
> +    vgid_ptr = acpi_data_push(table_data, sizeof(ssdt_vmgenid_aml));
> +    memcpy(vgid_ptr, ssdt_vmgenid_aml, sizeof(ssdt_vmgenid_aml));
> +
> +    vm_gid_ptr = acpi_data_get_ptr(vgid_ptr, sizeof(ssdt_vmgenid_aml),
> +                                   *ssdt_acpi_vm_gid,
> +                                   sizeof(info->vm_generation_id));
> +    memcpy(vm_gid_ptr, info->vm_generation_id,
> +           sizeof(info->vm_generation_id));
> +
> +    bios_linker_loader_add_pointer(linker, ACPI_BUILD_TABLE_FILE,
> +                                   ACPI_BUILD_TABLE_FILE,
> +                                   table_data,
> +                                   vgid_ptr + *ssdt_acpi_vm_gid_addr,
> +                                   sizeof(uint32_t));
> +
> +    vm_gid_physical_address = vgid_start + *ssdt_acpi_vm_gid;
> +    ACPI_BUILD_SET_LE(vgid_ptr, sizeof(ssdt_vmgenid_aml),
> +                      *ssdt_acpi_vm_gid_addr, 32, vm_gid_physical_address);
> +
> +    build_header(linker, table_data, vgid_ptr, "SSDT",
> +                 sizeof(ssdt_vmgenid_aml), 1);
> +}
> +
>  typedef enum {
>      MEM_AFFINITY_NOFLAGS      = 0,
>      MEM_AFFINITY_ENABLED      = (1 << 0),
> @@ -1617,6 +1652,10 @@ void acpi_build(PcGuestInfo *guest_info, AcpiBuildTables *tables)
>          acpi_add_table(table_offsets, tables->table_data);
>          build_tpm_ssdt(tables->table_data, tables->linker);
>      }
> +    if (misc.vm_generation_id_set) {
> +        acpi_add_table(table_offsets, tables->table_data);
> +        build_vmgenid_ssdt(tables->table_data, tables->linker, &misc);
> +    }
>      if (guest_info->numa_nodes) {
>          acpi_add_table(table_offsets, tables->table_data);
>          build_srat(tables->table_data, tables->linker, &cpu, guest_info);
> diff --git a/hw/i386/ssdt-vmgenid.dsl b/hw/i386/ssdt-vmgenid.dsl
> new file mode 100644
> index 0000000..54b5c34
> --- /dev/null
> +++ b/hw/i386/ssdt-vmgenid.dsl
> @@ -0,0 +1,64 @@
> +/*
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> +
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> +
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +/****************************************************************
> + * Virtual Machine Generation ID Device
> + ****************************************************************/
> +
> +ACPI_EXTRACT_ALL_CODE ssdt_vmgenid_aml
> +
> +DefinitionBlock (
> +    "ssdt-vmgenid.aml", // Output Filename
> +    "SSDT",             // Signature
> +    0x01,               // SSDT Compliance Revision
> +    "BXPC",             // OEMID
> +    "BXSSDTSUSP",       // TABLE ID
> +    0x1                 // OEM Revision
> +    )
> +{
> +    Scope(\_SB) {
> +
> +        Device(VMGI) {
> +            Name(_HID, "QEMU0002")
> +            Name(_CID, "VM_Gen_Counter")
> +            Name(_DDN, "VM_Gen_Counter")
> +
> +            ACPI_EXTRACT_NAME_DWORD_CONST ssdt_acpi_vm_gid_addr
> +            Name(VGIA, 0x12345678)
> +
> +            ACPI_EXTRACT_NAME_BUFFER16 ssdt_acpi_vm_gid
> +            Name(VGID, Buffer(16) {
> +                0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +                0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 })
> +
> +            Method(_STA, 0, NotSerialized) {
> +                Store(VGIA, Local0)
> +                If (LEqual(Local0, Zero)) {
> +                    Return (0x00)
> +                } Else {
> +                    Return (0x0F)
> +                }
> +            }
> +
> +            Method(ADDR, 0, Serialized) {
> +                Store(Package(2) { }, Local0)
> +                Store(VGIA, Index(Local0, 0))
> +                Store(0x0000, Index(Local0, 1))
> +                return (Local0)
> +            }
> +        }
> +    }
> +}
> +
> diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
> index 979e532..c18b800 100644
> --- a/hw/misc/Makefile.objs
> +++ b/hw/misc/Makefile.objs
> @@ -41,3 +41,4 @@ obj-$(CONFIG_SLAVIO) += slavio_misc.o
>  obj-$(CONFIG_ZYNQ) += zynq_slcr.o
>  
>  obj-$(CONFIG_PVPANIC) += pvpanic.o
> +obj-$(CONFIG_VMGENID) += vmgenid.o
> diff --git a/hw/misc/vmgenid.c b/hw/misc/vmgenid.c
> new file mode 100644
> index 0000000..76956d1
> --- /dev/null
> +++ b/hw/misc/vmgenid.c
> @@ -0,0 +1,85 @@
> +/*
> + *  Virtual Machine Generation ID Device
> + *
> + *  Copyright (C) 2014 Red Hat Inc.
> + *
> + *  Authors: Gal Hammer <ghammer@redhat.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + *
> + */
> +
> +#include "hw/i386/pc.h"
> +#include "hw/sysbus.h"
> +
> +#define VMGENID_DEVICE "vmgenid"
> +
> +#define PROPERTY_UUID "uuid"
> +
> +#define VMGENID(obj) OBJECT_CHECK(VmGenIdState, (obj), VMGENID_DEVICE)
> +
> +typedef struct VmGenIdState {
> +    SysBusDevice parent_obj;
> +    char *guid_arg;
> +} VmGenIdState;
> +
> +bool vm_generation_id(uint8_t id[16])
> +{
> +    Object *o = object_resolve_path_type("", VMGENID_DEVICE, NULL);
> +    char *guid;
> +
> +    if (!o) {
> +        return false;
> +    }
> +    guid = object_property_get_str(o, PROPERTY_UUID, NULL);
> +    /* actual uuid validation was checked during realize. */
> +    (void)qemu_uuid_parse(guid, id);
> +    return true;
> +}
> +
> +static void vmgenid_realize(DeviceState *dev, Error **errp)
> +{
> +    VmGenIdState *s = VMGENID(dev);
> +    uint8_t id[16];
> +
> +    if (!s->guid_arg) {
> +        error_setg(errp, "missing uuid.");
> +        return;
> +    }
> +
> +    if (qemu_uuid_parse(s->guid_arg, id) < 0) {
> +        error_setg(errp, "Fail to parse UUID string.");
> +        return;
> +    }
> +}
> +
> +static Property vmgenid_device_properties[] = {
> +    DEFINE_PROP_STRING(PROPERTY_UUID, VmGenIdState, guid_arg),
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
> +static void vmgenid_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +
> +    dc->realize = vmgenid_realize;
> +    dc->props = vmgenid_device_properties;
> +    dc->cannot_instantiate_with_device_add_yet = false;
> +    set_bit(DEVICE_CATEGORY_MISC, dc->categories);
> +}
> +
> +static const TypeInfo vmgenid_device_info = {
> +    .name          = VMGENID_DEVICE,
> +    .parent        = TYPE_SYS_BUS_DEVICE,
> +    .instance_size = sizeof(VmGenIdState),
> +    .class_init    = vmgenid_class_init,
> +};
> +
> +static void vmgenid_register_types(void)
> +{
> +    type_register_static(&vmgenid_device_info);
> +}
> +
> +type_init(vmgenid_register_types)
> +
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 77316d5..40ecccb 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -290,6 +290,9 @@ void pc_system_firmware_init(MemoryRegion *rom_memory,
>  /* pvpanic.c */
>  uint16_t pvpanic_port(void);
>  
> +/* vmgenid.c */
> +bool vm_generation_id(uint8_t id[16]);
> +
>  /* e820 types */
>  #define E820_RAM        1
>  #define E820_RESERVED   2
> 

This looks good.  Thanks!

Paolo

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [Qemu-devel] [PATCH 2/2] i386: Add a Virtual Machine Generation ID device.
  2014-09-16 13:04 [Qemu-devel] [PATCH 0/2 V4] Virtual Machine Generation ID Gal Hammer
@ 2014-09-16 13:04 ` Gal Hammer
  2014-09-16 13:16   ` Paolo Bonzini
  2014-09-16 14:54   ` Igor Mammedov
  0 siblings, 2 replies; 28+ messages in thread
From: Gal Hammer @ 2014-09-16 13:04 UTC (permalink / raw)
  To: qemu-devel; +Cc: Gal Hammer, pbonzini

Based on Microsoft's sepecifications (paper can be dowloaded from
http://go.microsoft.com/fwlink/?LinkId=260709), add a device
description to the SSDT ACPI table.

The GUID is set using a new "vmgenid" device.

Signed-off-by: Gal Hammer <ghammer@redhat.com>
---
 default-configs/i386-softmmu.mak   |  1 +
 default-configs/x86_64-softmmu.mak |  1 +
 hw/i386/Makefile.objs              |  2 +-
 hw/i386/acpi-build.c               | 39 +++++++++++++++++
 hw/i386/ssdt-vmgenid.dsl           | 64 ++++++++++++++++++++++++++++
 hw/misc/Makefile.objs              |  1 +
 hw/misc/vmgenid.c                  | 85 ++++++++++++++++++++++++++++++++++++++
 include/hw/i386/pc.h               |  3 ++
 8 files changed, 195 insertions(+), 1 deletion(-)
 create mode 100644 hw/i386/ssdt-vmgenid.dsl
 create mode 100644 hw/misc/vmgenid.c

diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak
index 8e08841..bd33c75 100644
--- a/default-configs/i386-softmmu.mak
+++ b/default-configs/i386-softmmu.mak
@@ -45,3 +45,4 @@ CONFIG_IOAPIC=y
 CONFIG_ICC_BUS=y
 CONFIG_PVPANIC=y
 CONFIG_MEM_HOTPLUG=y
+CONFIG_VMGENID=y
diff --git a/default-configs/x86_64-softmmu.mak b/default-configs/x86_64-softmmu.mak
index 66557ac..006fc7c 100644
--- a/default-configs/x86_64-softmmu.mak
+++ b/default-configs/x86_64-softmmu.mak
@@ -45,3 +45,4 @@ CONFIG_IOAPIC=y
 CONFIG_ICC_BUS=y
 CONFIG_PVPANIC=y
 CONFIG_MEM_HOTPLUG=y
+CONFIG_VMGENID=y
diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs
index 9d419ad..cd1beb3 100644
--- a/hw/i386/Makefile.objs
+++ b/hw/i386/Makefile.objs
@@ -12,7 +12,7 @@ hw/i386/acpi-build.o: hw/i386/acpi-build.c hw/i386/acpi-dsdt.hex \
 	hw/i386/ssdt-proc.hex hw/i386/ssdt-pcihp.hex hw/i386/ssdt-misc.hex \
 	hw/i386/acpi-dsdt.hex hw/i386/q35-acpi-dsdt.hex \
 	hw/i386/q35-acpi-dsdt.hex hw/i386/ssdt-mem.hex \
-	hw/i386/ssdt-tpm.hex
+	hw/i386/ssdt-tpm.hex hw/i386/ssdt-vmgenid.hex
 
 iasl-option=$(shell if test -z "`$(1) $(2) 2>&1 > /dev/null`" \
     ; then echo "$(2)"; else echo "$(3)"; fi ;)
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index a313321..72d5a88 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -96,6 +96,8 @@ typedef struct AcpiMiscInfo {
     const unsigned char *dsdt_code;
     unsigned dsdt_size;
     uint16_t pvpanic_port;
+    bool vm_generation_id_set;
+    uint8_t vm_generation_id[16];
 } AcpiMiscInfo;
 
 typedef struct AcpiBuildPciBusHotplugState {
@@ -216,6 +218,7 @@ static void acpi_get_misc_info(AcpiMiscInfo *info)
     info->has_hpet = hpet_find();
     info->has_tpm = tpm_find();
     info->pvpanic_port = pvpanic_port();
+    info->vm_generation_id_set = vm_generation_id(info->vm_generation_id);
 }
 
 static void acpi_get_pci_info(PcPciInfo *info)
@@ -710,6 +713,7 @@ static inline char acpi_get_hex(uint32_t val)
 #include "hw/i386/ssdt-misc.hex"
 #include "hw/i386/ssdt-pcihp.hex"
 #include "hw/i386/ssdt-tpm.hex"
+#include "hw/i386/ssdt-vmgenid.hex"
 
 static void
 build_append_notify_method(GArray *device, const char *name,
@@ -1246,6 +1250,37 @@ build_tpm_ssdt(GArray *table_data, GArray *linker)
     memcpy(tpm_ptr, ssdt_tpm_aml, sizeof(ssdt_tpm_aml));
 }
 
+static void
+build_vmgenid_ssdt(GArray *table_data, GArray *linker, AcpiMiscInfo *info)
+{
+    int vgid_start = table_data->len;
+    void *vgid_ptr;
+    uint8_t *vm_gid_ptr;
+    uint32_t vm_gid_physical_address;
+
+    vgid_ptr = acpi_data_push(table_data, sizeof(ssdt_vmgenid_aml));
+    memcpy(vgid_ptr, ssdt_vmgenid_aml, sizeof(ssdt_vmgenid_aml));
+
+    vm_gid_ptr = acpi_data_get_ptr(vgid_ptr, sizeof(ssdt_vmgenid_aml),
+                                   *ssdt_acpi_vm_gid,
+                                   sizeof(info->vm_generation_id));
+    memcpy(vm_gid_ptr, info->vm_generation_id,
+           sizeof(info->vm_generation_id));
+
+    bios_linker_loader_add_pointer(linker, ACPI_BUILD_TABLE_FILE,
+                                   ACPI_BUILD_TABLE_FILE,
+                                   table_data,
+                                   vgid_ptr + *ssdt_acpi_vm_gid_addr,
+                                   sizeof(uint32_t));
+
+    vm_gid_physical_address = vgid_start + *ssdt_acpi_vm_gid;
+    ACPI_BUILD_SET_LE(vgid_ptr, sizeof(ssdt_vmgenid_aml),
+                      *ssdt_acpi_vm_gid_addr, 32, vm_gid_physical_address);
+
+    build_header(linker, table_data, vgid_ptr, "SSDT",
+                 sizeof(ssdt_vmgenid_aml), 1);
+}
+
 typedef enum {
     MEM_AFFINITY_NOFLAGS      = 0,
     MEM_AFFINITY_ENABLED      = (1 << 0),
@@ -1617,6 +1652,10 @@ void acpi_build(PcGuestInfo *guest_info, AcpiBuildTables *tables)
         acpi_add_table(table_offsets, tables->table_data);
         build_tpm_ssdt(tables->table_data, tables->linker);
     }
+    if (misc.vm_generation_id_set) {
+        acpi_add_table(table_offsets, tables->table_data);
+        build_vmgenid_ssdt(tables->table_data, tables->linker, &misc);
+    }
     if (guest_info->numa_nodes) {
         acpi_add_table(table_offsets, tables->table_data);
         build_srat(tables->table_data, tables->linker, &cpu, guest_info);
diff --git a/hw/i386/ssdt-vmgenid.dsl b/hw/i386/ssdt-vmgenid.dsl
new file mode 100644
index 0000000..54b5c34
--- /dev/null
+++ b/hw/i386/ssdt-vmgenid.dsl
@@ -0,0 +1,64 @@
+/*
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+/****************************************************************
+ * Virtual Machine Generation ID Device
+ ****************************************************************/
+
+ACPI_EXTRACT_ALL_CODE ssdt_vmgenid_aml
+
+DefinitionBlock (
+    "ssdt-vmgenid.aml", // Output Filename
+    "SSDT",             // Signature
+    0x01,               // SSDT Compliance Revision
+    "BXPC",             // OEMID
+    "BXSSDTSUSP",       // TABLE ID
+    0x1                 // OEM Revision
+    )
+{
+    Scope(\_SB) {
+
+        Device(VMGI) {
+            Name(_HID, "QEMU0002")
+            Name(_CID, "VM_Gen_Counter")
+            Name(_DDN, "VM_Gen_Counter")
+
+            ACPI_EXTRACT_NAME_DWORD_CONST ssdt_acpi_vm_gid_addr
+            Name(VGIA, 0x12345678)
+
+            ACPI_EXTRACT_NAME_BUFFER16 ssdt_acpi_vm_gid
+            Name(VGID, Buffer(16) {
+                0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+                0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 })
+
+            Method(_STA, 0, NotSerialized) {
+                Store(VGIA, Local0)
+                If (LEqual(Local0, Zero)) {
+                    Return (0x00)
+                } Else {
+                    Return (0x0F)
+                }
+            }
+
+            Method(ADDR, 0, Serialized) {
+                Store(Package(2) { }, Local0)
+                Store(VGIA, Index(Local0, 0))
+                Store(0x0000, Index(Local0, 1))
+                return (Local0)
+            }
+        }
+    }
+}
+
diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
index 979e532..c18b800 100644
--- a/hw/misc/Makefile.objs
+++ b/hw/misc/Makefile.objs
@@ -41,3 +41,4 @@ obj-$(CONFIG_SLAVIO) += slavio_misc.o
 obj-$(CONFIG_ZYNQ) += zynq_slcr.o
 
 obj-$(CONFIG_PVPANIC) += pvpanic.o
+obj-$(CONFIG_VMGENID) += vmgenid.o
diff --git a/hw/misc/vmgenid.c b/hw/misc/vmgenid.c
new file mode 100644
index 0000000..76956d1
--- /dev/null
+++ b/hw/misc/vmgenid.c
@@ -0,0 +1,85 @@
+/*
+ *  Virtual Machine Generation ID Device
+ *
+ *  Copyright (C) 2014 Red Hat Inc.
+ *
+ *  Authors: Gal Hammer <ghammer@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#include "hw/i386/pc.h"
+#include "hw/sysbus.h"
+
+#define VMGENID_DEVICE "vmgenid"
+
+#define PROPERTY_UUID "uuid"
+
+#define VMGENID(obj) OBJECT_CHECK(VmGenIdState, (obj), VMGENID_DEVICE)
+
+typedef struct VmGenIdState {
+    SysBusDevice parent_obj;
+    char *guid_arg;
+} VmGenIdState;
+
+bool vm_generation_id(uint8_t id[16])
+{
+    Object *o = object_resolve_path_type("", VMGENID_DEVICE, NULL);
+    char *guid;
+
+    if (!o) {
+        return false;
+    }
+    guid = object_property_get_str(o, PROPERTY_UUID, NULL);
+    /* actual uuid validation was checked during realize. */
+    (void)qemu_uuid_parse(guid, id);
+    return true;
+}
+
+static void vmgenid_realize(DeviceState *dev, Error **errp)
+{
+    VmGenIdState *s = VMGENID(dev);
+    uint8_t id[16];
+
+    if (!s->guid_arg) {
+        error_setg(errp, "missing uuid.");
+        return;
+    }
+
+    if (qemu_uuid_parse(s->guid_arg, id) < 0) {
+        error_setg(errp, "Fail to parse UUID string.");
+        return;
+    }
+}
+
+static Property vmgenid_device_properties[] = {
+    DEFINE_PROP_STRING(PROPERTY_UUID, VmGenIdState, guid_arg),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void vmgenid_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->realize = vmgenid_realize;
+    dc->props = vmgenid_device_properties;
+    dc->cannot_instantiate_with_device_add_yet = false;
+    set_bit(DEVICE_CATEGORY_MISC, dc->categories);
+}
+
+static const TypeInfo vmgenid_device_info = {
+    .name          = VMGENID_DEVICE,
+    .parent        = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(VmGenIdState),
+    .class_init    = vmgenid_class_init,
+};
+
+static void vmgenid_register_types(void)
+{
+    type_register_static(&vmgenid_device_info);
+}
+
+type_init(vmgenid_register_types)
+
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 77316d5..40ecccb 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -290,6 +290,9 @@ void pc_system_firmware_init(MemoryRegion *rom_memory,
 /* pvpanic.c */
 uint16_t pvpanic_port(void);
 
+/* vmgenid.c */
+bool vm_generation_id(uint8_t id[16]);
+
 /* e820 types */
 #define E820_RAM        1
 #define E820_RESERVED   2
-- 
1.9.3

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [Qemu-devel] [PATCH 2/2] i386: Add a Virtual Machine Generation ID device.
  2014-09-15  8:46   ` Igor Mammedov
@ 2014-09-15  9:15     ` Gal Hammer
  0 siblings, 0 replies; 28+ messages in thread
From: Gal Hammer @ 2014-09-15  9:15 UTC (permalink / raw)
  To: Igor Mammedov; +Cc: pbonzini, qemu-devel

Got it. I'll start to work on V4 then...

    Gal.

----- Original Message -----
From: "Igor Mammedov" <imammedo@redhat.com>
To: "Gal Hammer" <ghammer@redhat.com>
Cc: pbonzini@redhat.com, qemu-devel@nongnu.org
Sent: Monday, September 15, 2014 11:46:41 AM
Subject: Re: [Qemu-devel] [PATCH 2/2] i386: Add a Virtual Machine Generation ID device.

On Mon, 15 Sep 2014 10:27:08 +0300
Gal Hammer <ghammer@redhat.com> wrote:

> Based on Microsoft's sepecifications (paper can be dowloaded from
> http://go.microsoft.com/fwlink/?LinkId=260709), add a device
> description to the DSDT ACPI table.
> 
> The GUID is set using a new "vmgenid" device.
> 
> Signed-off-by: Gal Hammer <ghammer@redhat.com>
> ---
>  default-configs/i386-softmmu.mak   |  1 +
>  default-configs/x86_64-softmmu.mak |  1 +
>  hw/i386/acpi-build.c               | 18 ++++++++
>  hw/i386/ssdt-misc.dsl              | 36 ++++++++++++++++
>  hw/misc/Makefile.objs              |  1 +
>  hw/misc/vmgenid.c                  | 85 ++++++++++++++++++++++++++++++++++++++
>  include/hw/i386/pc.h               |  3 ++
>  7 files changed, 145 insertions(+)
>  create mode 100644 hw/misc/vmgenid.c
> 
> diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak
> index 8e08841..bd33c75 100644
> --- a/default-configs/i386-softmmu.mak
> +++ b/default-configs/i386-softmmu.mak
> @@ -45,3 +45,4 @@ CONFIG_IOAPIC=y
>  CONFIG_ICC_BUS=y
>  CONFIG_PVPANIC=y
>  CONFIG_MEM_HOTPLUG=y
> +CONFIG_VMGENID=y
> diff --git a/default-configs/x86_64-softmmu.mak b/default-configs/x86_64-softmmu.mak
> index 66557ac..006fc7c 100644
> --- a/default-configs/x86_64-softmmu.mak
> +++ b/default-configs/x86_64-softmmu.mak
> @@ -45,3 +45,4 @@ CONFIG_IOAPIC=y
>  CONFIG_ICC_BUS=y
>  CONFIG_PVPANIC=y
>  CONFIG_MEM_HOTPLUG=y
> +CONFIG_VMGENID=y
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index a313321..613ecfc 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -1063,6 +1063,8 @@ build_ssdt(GArray *table_data, GArray *linker,
>      unsigned acpi_cpus = guest_info->apic_id_limit;
>      int ssdt_start = table_data->len;
>      uint8_t *ssdt_ptr;
> +    uint8_t *vm_gid_ptr;
> +    uint32_t vm_gid_physical_address;
>      int i;
>  
>      /* The current AML generator can cover the APIC ID range [0..255],
> @@ -1088,6 +1090,22 @@ build_ssdt(GArray *table_data, GArray *linker,
>      ACPI_BUILD_SET_LE(ssdt_ptr, sizeof(ssdp_misc_aml),
>                        ssdt_isa_pest[0], 16, misc->pvpanic_port);
>  
> +    vm_gid_ptr = acpi_data_get_ptr(ssdt_ptr, sizeof(ssdp_misc_aml),
> +                                   ssdt_acpi_vm_gid[0], 16);
> +    if (vm_generation_id(vm_gid_ptr)) {
> +        vm_gid_physical_address = ssdt_start + ssdt_acpi_vm_gid[0];
> +        bios_linker_loader_add_pointer(linker, ACPI_BUILD_TABLE_FILE,
> +                                       ACPI_BUILD_TABLE_FILE,
> +                                       table_data,
> +                                       ssdt_ptr + ssdt_acpi_vm_gid_addr[0],
> +                                       sizeof(uint32_t));
> +    } else {
> +        vm_gid_physical_address = 0;
> +    }
> +
> +    ACPI_BUILD_SET_LE(ssdt_ptr, sizeof(ssdp_misc_aml),
> +                      ssdt_acpi_vm_gid_addr[0], 32, vm_gid_physical_address);
> +
>      ACPI_BUILD_SET_LE(ssdt_ptr, sizeof(ssdp_misc_aml),
>                        ssdt_mctrl_nr_slots[0], 32, nr_mem);
>  
> diff --git a/hw/i386/ssdt-misc.dsl b/hw/i386/ssdt-misc.dsl
> index 0fd4480..630cd35 100644
> --- a/hw/i386/ssdt-misc.dsl
> +++ b/hw/i386/ssdt-misc.dsl
> @@ -118,6 +118,42 @@ DefinitionBlock ("ssdt-misc.aml", "SSDT", 0x01, "BXPC", "BXSSDTSUSP", 0x1)
>          }
>      }
>  
> +/****************************************************************
> + * Virtual Machine Generation ID Device
> + ****************************************************************/
> +    Scope(\_SB) {
> +
> +        Device(VMGI) {
> +            Name(_HID, "QEMU0002")
> +            Name(_CID, "VM_Gen_Counter")
> +            Name(_DDN, "VM_Gen_Counter")
> +
> +            ACPI_EXTRACT_NAME_DWORD_CONST ssdt_acpi_vm_gid_addr
> +            Name(VGIA, 0x12345678)
> +
> +            ACPI_EXTRACT_NAME_BUFFER16 ssdt_acpi_vm_gid
> +            Name(VGID, Buffer(16) {
> +                0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +                0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 })
> +
> +            Method(_STA, 0, NotSerialized) {
> +                Store(VGIA, Local0)
> +                If (LEqual(Local0, Zero)) {
> +                    Return (0x00)
> +                } Else {
> +                    Return (0x0F)
> +                }
> +            }
> +
> +            Method(ADDR, 0, Serialized) {
> +                Store(Package(2) { }, Local0)
> +                Store(VGIA, Index(Local0, 0))
> +                Store(0x0000, Index(Local0, 1))
> +                return (Local0)
> +            }
> +        }
> +    }
> 
It's not exactly what Paolo've meant under dynamic inclusion wrt migration issues.
Sine ssdt-misc is always included in SSDT, so ACPI table size will change regardless
whether VMGENID_DEVICE is present or not.

To make it dynamice, create a separate template file like hw/i386/ssdt-tpm.dsl and
include its dump like it's done in build_tpm_ssdt() only when VMGENID_DEVICE is present.


>      External(MEMORY_SLOT_NOTIFY_METHOD, MethodObj)
>      Scope(\_SB.PCI0) {
>          Device(MEMORY_HOTPLUG_DEVICE) {
> diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
> index 979e532..c18b800 100644
> --- a/hw/misc/Makefile.objs
> +++ b/hw/misc/Makefile.objs
> @@ -41,3 +41,4 @@ obj-$(CONFIG_SLAVIO) += slavio_misc.o
>  obj-$(CONFIG_ZYNQ) += zynq_slcr.o
>  
>  obj-$(CONFIG_PVPANIC) += pvpanic.o
> +obj-$(CONFIG_VMGENID) += vmgenid.o
> diff --git a/hw/misc/vmgenid.c b/hw/misc/vmgenid.c
> new file mode 100644
> index 0000000..76956d1
> --- /dev/null
> +++ b/hw/misc/vmgenid.c
> @@ -0,0 +1,85 @@
> +/*
> + *  Virtual Machine Generation ID Device
> + *
> + *  Copyright (C) 2014 Red Hat Inc.
> + *
> + *  Authors: Gal Hammer <ghammer@redhat.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + *
> + */
> +
> +#include "hw/i386/pc.h"
> +#include "hw/sysbus.h"
> +
> +#define VMGENID_DEVICE "vmgenid"
> +
> +#define PROPERTY_UUID "uuid"
> +
> +#define VMGENID(obj) OBJECT_CHECK(VmGenIdState, (obj), VMGENID_DEVICE)
> +
> +typedef struct VmGenIdState {
> +    SysBusDevice parent_obj;
> +    char *guid_arg;
> +} VmGenIdState;
> +
> +bool vm_generation_id(uint8_t id[16])
> +{
> +    Object *o = object_resolve_path_type("", VMGENID_DEVICE, NULL);
> +    char *guid;
> +
> +    if (!o) {
> +        return false;
> +    }
> +    guid = object_property_get_str(o, PROPERTY_UUID, NULL);
> +    /* actual uuid validation was checked during realize. */
> +    (void)qemu_uuid_parse(guid, id);
> +    return true;
> +}
> +
> +static void vmgenid_realize(DeviceState *dev, Error **errp)
> +{
> +    VmGenIdState *s = VMGENID(dev);
> +    uint8_t id[16];
> +
> +    if (!s->guid_arg) {
> +        error_setg(errp, "missing uuid.");
> +        return;
> +    }
> +
> +    if (qemu_uuid_parse(s->guid_arg, id) < 0) {
> +        error_setg(errp, "Fail to parse UUID string.");
> +        return;
> +    }
> +}
> +
> +static Property vmgenid_device_properties[] = {
> +    DEFINE_PROP_STRING(PROPERTY_UUID, VmGenIdState, guid_arg),
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
> +static void vmgenid_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +
> +    dc->realize = vmgenid_realize;
> +    dc->props = vmgenid_device_properties;
> +    dc->cannot_instantiate_with_device_add_yet = false;
> +    set_bit(DEVICE_CATEGORY_MISC, dc->categories);
> +}
> +
> +static const TypeInfo vmgenid_device_info = {
> +    .name          = VMGENID_DEVICE,
> +    .parent        = TYPE_SYS_BUS_DEVICE,
> +    .instance_size = sizeof(VmGenIdState),
> +    .class_init    = vmgenid_class_init,
> +};
> +
> +static void vmgenid_register_types(void)
> +{
> +    type_register_static(&vmgenid_device_info);
> +}
> +
> +type_init(vmgenid_register_types)
> +
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 77316d5..40ecccb 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -290,6 +290,9 @@ void pc_system_firmware_init(MemoryRegion *rom_memory,
>  /* pvpanic.c */
>  uint16_t pvpanic_port(void);
>  
> +/* vmgenid.c */
> +bool vm_generation_id(uint8_t id[16]);
> +
>  /* e820 types */
>  #define E820_RAM        1
>  #define E820_RESERVED   2

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [Qemu-devel] [PATCH 2/2] i386: Add a Virtual Machine Generation ID device.
  2014-09-15  7:27 ` [Qemu-devel] [PATCH 2/2] i386: Add a Virtual Machine Generation ID device Gal Hammer
@ 2014-09-15  8:46   ` Igor Mammedov
  2014-09-15  9:15     ` Gal Hammer
  0 siblings, 1 reply; 28+ messages in thread
From: Igor Mammedov @ 2014-09-15  8:46 UTC (permalink / raw)
  To: Gal Hammer; +Cc: pbonzini, qemu-devel

On Mon, 15 Sep 2014 10:27:08 +0300
Gal Hammer <ghammer@redhat.com> wrote:

> Based on Microsoft's sepecifications (paper can be dowloaded from
> http://go.microsoft.com/fwlink/?LinkId=260709), add a device
> description to the DSDT ACPI table.
> 
> The GUID is set using a new "vmgenid" device.
> 
> Signed-off-by: Gal Hammer <ghammer@redhat.com>
> ---
>  default-configs/i386-softmmu.mak   |  1 +
>  default-configs/x86_64-softmmu.mak |  1 +
>  hw/i386/acpi-build.c               | 18 ++++++++
>  hw/i386/ssdt-misc.dsl              | 36 ++++++++++++++++
>  hw/misc/Makefile.objs              |  1 +
>  hw/misc/vmgenid.c                  | 85 ++++++++++++++++++++++++++++++++++++++
>  include/hw/i386/pc.h               |  3 ++
>  7 files changed, 145 insertions(+)
>  create mode 100644 hw/misc/vmgenid.c
> 
> diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak
> index 8e08841..bd33c75 100644
> --- a/default-configs/i386-softmmu.mak
> +++ b/default-configs/i386-softmmu.mak
> @@ -45,3 +45,4 @@ CONFIG_IOAPIC=y
>  CONFIG_ICC_BUS=y
>  CONFIG_PVPANIC=y
>  CONFIG_MEM_HOTPLUG=y
> +CONFIG_VMGENID=y
> diff --git a/default-configs/x86_64-softmmu.mak b/default-configs/x86_64-softmmu.mak
> index 66557ac..006fc7c 100644
> --- a/default-configs/x86_64-softmmu.mak
> +++ b/default-configs/x86_64-softmmu.mak
> @@ -45,3 +45,4 @@ CONFIG_IOAPIC=y
>  CONFIG_ICC_BUS=y
>  CONFIG_PVPANIC=y
>  CONFIG_MEM_HOTPLUG=y
> +CONFIG_VMGENID=y
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index a313321..613ecfc 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -1063,6 +1063,8 @@ build_ssdt(GArray *table_data, GArray *linker,
>      unsigned acpi_cpus = guest_info->apic_id_limit;
>      int ssdt_start = table_data->len;
>      uint8_t *ssdt_ptr;
> +    uint8_t *vm_gid_ptr;
> +    uint32_t vm_gid_physical_address;
>      int i;
>  
>      /* The current AML generator can cover the APIC ID range [0..255],
> @@ -1088,6 +1090,22 @@ build_ssdt(GArray *table_data, GArray *linker,
>      ACPI_BUILD_SET_LE(ssdt_ptr, sizeof(ssdp_misc_aml),
>                        ssdt_isa_pest[0], 16, misc->pvpanic_port);
>  
> +    vm_gid_ptr = acpi_data_get_ptr(ssdt_ptr, sizeof(ssdp_misc_aml),
> +                                   ssdt_acpi_vm_gid[0], 16);
> +    if (vm_generation_id(vm_gid_ptr)) {
> +        vm_gid_physical_address = ssdt_start + ssdt_acpi_vm_gid[0];
> +        bios_linker_loader_add_pointer(linker, ACPI_BUILD_TABLE_FILE,
> +                                       ACPI_BUILD_TABLE_FILE,
> +                                       table_data,
> +                                       ssdt_ptr + ssdt_acpi_vm_gid_addr[0],
> +                                       sizeof(uint32_t));
> +    } else {
> +        vm_gid_physical_address = 0;
> +    }
> +
> +    ACPI_BUILD_SET_LE(ssdt_ptr, sizeof(ssdp_misc_aml),
> +                      ssdt_acpi_vm_gid_addr[0], 32, vm_gid_physical_address);
> +
>      ACPI_BUILD_SET_LE(ssdt_ptr, sizeof(ssdp_misc_aml),
>                        ssdt_mctrl_nr_slots[0], 32, nr_mem);
>  
> diff --git a/hw/i386/ssdt-misc.dsl b/hw/i386/ssdt-misc.dsl
> index 0fd4480..630cd35 100644
> --- a/hw/i386/ssdt-misc.dsl
> +++ b/hw/i386/ssdt-misc.dsl
> @@ -118,6 +118,42 @@ DefinitionBlock ("ssdt-misc.aml", "SSDT", 0x01, "BXPC", "BXSSDTSUSP", 0x1)
>          }
>      }
>  
> +/****************************************************************
> + * Virtual Machine Generation ID Device
> + ****************************************************************/
> +    Scope(\_SB) {
> +
> +        Device(VMGI) {
> +            Name(_HID, "QEMU0002")
> +            Name(_CID, "VM_Gen_Counter")
> +            Name(_DDN, "VM_Gen_Counter")
> +
> +            ACPI_EXTRACT_NAME_DWORD_CONST ssdt_acpi_vm_gid_addr
> +            Name(VGIA, 0x12345678)
> +
> +            ACPI_EXTRACT_NAME_BUFFER16 ssdt_acpi_vm_gid
> +            Name(VGID, Buffer(16) {
> +                0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +                0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 })
> +
> +            Method(_STA, 0, NotSerialized) {
> +                Store(VGIA, Local0)
> +                If (LEqual(Local0, Zero)) {
> +                    Return (0x00)
> +                } Else {
> +                    Return (0x0F)
> +                }
> +            }
> +
> +            Method(ADDR, 0, Serialized) {
> +                Store(Package(2) { }, Local0)
> +                Store(VGIA, Index(Local0, 0))
> +                Store(0x0000, Index(Local0, 1))
> +                return (Local0)
> +            }
> +        }
> +    }
> 
It's not exactly what Paolo've meant under dynamic inclusion wrt migration issues.
Sine ssdt-misc is always included in SSDT, so ACPI table size will change regardless
whether VMGENID_DEVICE is present or not.

To make it dynamice, create a separate template file like hw/i386/ssdt-tpm.dsl and
include its dump like it's done in build_tpm_ssdt() only when VMGENID_DEVICE is present.


>      External(MEMORY_SLOT_NOTIFY_METHOD, MethodObj)
>      Scope(\_SB.PCI0) {
>          Device(MEMORY_HOTPLUG_DEVICE) {
> diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
> index 979e532..c18b800 100644
> --- a/hw/misc/Makefile.objs
> +++ b/hw/misc/Makefile.objs
> @@ -41,3 +41,4 @@ obj-$(CONFIG_SLAVIO) += slavio_misc.o
>  obj-$(CONFIG_ZYNQ) += zynq_slcr.o
>  
>  obj-$(CONFIG_PVPANIC) += pvpanic.o
> +obj-$(CONFIG_VMGENID) += vmgenid.o
> diff --git a/hw/misc/vmgenid.c b/hw/misc/vmgenid.c
> new file mode 100644
> index 0000000..76956d1
> --- /dev/null
> +++ b/hw/misc/vmgenid.c
> @@ -0,0 +1,85 @@
> +/*
> + *  Virtual Machine Generation ID Device
> + *
> + *  Copyright (C) 2014 Red Hat Inc.
> + *
> + *  Authors: Gal Hammer <ghammer@redhat.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + *
> + */
> +
> +#include "hw/i386/pc.h"
> +#include "hw/sysbus.h"
> +
> +#define VMGENID_DEVICE "vmgenid"
> +
> +#define PROPERTY_UUID "uuid"
> +
> +#define VMGENID(obj) OBJECT_CHECK(VmGenIdState, (obj), VMGENID_DEVICE)
> +
> +typedef struct VmGenIdState {
> +    SysBusDevice parent_obj;
> +    char *guid_arg;
> +} VmGenIdState;
> +
> +bool vm_generation_id(uint8_t id[16])
> +{
> +    Object *o = object_resolve_path_type("", VMGENID_DEVICE, NULL);
> +    char *guid;
> +
> +    if (!o) {
> +        return false;
> +    }
> +    guid = object_property_get_str(o, PROPERTY_UUID, NULL);
> +    /* actual uuid validation was checked during realize. */
> +    (void)qemu_uuid_parse(guid, id);
> +    return true;
> +}
> +
> +static void vmgenid_realize(DeviceState *dev, Error **errp)
> +{
> +    VmGenIdState *s = VMGENID(dev);
> +    uint8_t id[16];
> +
> +    if (!s->guid_arg) {
> +        error_setg(errp, "missing uuid.");
> +        return;
> +    }
> +
> +    if (qemu_uuid_parse(s->guid_arg, id) < 0) {
> +        error_setg(errp, "Fail to parse UUID string.");
> +        return;
> +    }
> +}
> +
> +static Property vmgenid_device_properties[] = {
> +    DEFINE_PROP_STRING(PROPERTY_UUID, VmGenIdState, guid_arg),
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
> +static void vmgenid_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +
> +    dc->realize = vmgenid_realize;
> +    dc->props = vmgenid_device_properties;
> +    dc->cannot_instantiate_with_device_add_yet = false;
> +    set_bit(DEVICE_CATEGORY_MISC, dc->categories);
> +}
> +
> +static const TypeInfo vmgenid_device_info = {
> +    .name          = VMGENID_DEVICE,
> +    .parent        = TYPE_SYS_BUS_DEVICE,
> +    .instance_size = sizeof(VmGenIdState),
> +    .class_init    = vmgenid_class_init,
> +};
> +
> +static void vmgenid_register_types(void)
> +{
> +    type_register_static(&vmgenid_device_info);
> +}
> +
> +type_init(vmgenid_register_types)
> +
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 77316d5..40ecccb 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -290,6 +290,9 @@ void pc_system_firmware_init(MemoryRegion *rom_memory,
>  /* pvpanic.c */
>  uint16_t pvpanic_port(void);
>  
> +/* vmgenid.c */
> +bool vm_generation_id(uint8_t id[16]);
> +
>  /* e820 types */
>  #define E820_RAM        1
>  #define E820_RESERVED   2

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [Qemu-devel] [PATCH 2/2] i386: Add a Virtual Machine Generation ID device.
  2014-09-15  7:27 [Qemu-devel] [PATCH 0/2 V3] Virtual Machine Generation ID Gal Hammer
@ 2014-09-15  7:27 ` Gal Hammer
  2014-09-15  8:46   ` Igor Mammedov
  0 siblings, 1 reply; 28+ messages in thread
From: Gal Hammer @ 2014-09-15  7:27 UTC (permalink / raw)
  To: qemu-devel; +Cc: Gal Hammer, pbonzini

Based on Microsoft's sepecifications (paper can be dowloaded from
http://go.microsoft.com/fwlink/?LinkId=260709), add a device
description to the DSDT ACPI table.

The GUID is set using a new "vmgenid" device.

Signed-off-by: Gal Hammer <ghammer@redhat.com>
---
 default-configs/i386-softmmu.mak   |  1 +
 default-configs/x86_64-softmmu.mak |  1 +
 hw/i386/acpi-build.c               | 18 ++++++++
 hw/i386/ssdt-misc.dsl              | 36 ++++++++++++++++
 hw/misc/Makefile.objs              |  1 +
 hw/misc/vmgenid.c                  | 85 ++++++++++++++++++++++++++++++++++++++
 include/hw/i386/pc.h               |  3 ++
 7 files changed, 145 insertions(+)
 create mode 100644 hw/misc/vmgenid.c

diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak
index 8e08841..bd33c75 100644
--- a/default-configs/i386-softmmu.mak
+++ b/default-configs/i386-softmmu.mak
@@ -45,3 +45,4 @@ CONFIG_IOAPIC=y
 CONFIG_ICC_BUS=y
 CONFIG_PVPANIC=y
 CONFIG_MEM_HOTPLUG=y
+CONFIG_VMGENID=y
diff --git a/default-configs/x86_64-softmmu.mak b/default-configs/x86_64-softmmu.mak
index 66557ac..006fc7c 100644
--- a/default-configs/x86_64-softmmu.mak
+++ b/default-configs/x86_64-softmmu.mak
@@ -45,3 +45,4 @@ CONFIG_IOAPIC=y
 CONFIG_ICC_BUS=y
 CONFIG_PVPANIC=y
 CONFIG_MEM_HOTPLUG=y
+CONFIG_VMGENID=y
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index a313321..613ecfc 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1063,6 +1063,8 @@ build_ssdt(GArray *table_data, GArray *linker,
     unsigned acpi_cpus = guest_info->apic_id_limit;
     int ssdt_start = table_data->len;
     uint8_t *ssdt_ptr;
+    uint8_t *vm_gid_ptr;
+    uint32_t vm_gid_physical_address;
     int i;
 
     /* The current AML generator can cover the APIC ID range [0..255],
@@ -1088,6 +1090,22 @@ build_ssdt(GArray *table_data, GArray *linker,
     ACPI_BUILD_SET_LE(ssdt_ptr, sizeof(ssdp_misc_aml),
                       ssdt_isa_pest[0], 16, misc->pvpanic_port);
 
+    vm_gid_ptr = acpi_data_get_ptr(ssdt_ptr, sizeof(ssdp_misc_aml),
+                                   ssdt_acpi_vm_gid[0], 16);
+    if (vm_generation_id(vm_gid_ptr)) {
+        vm_gid_physical_address = ssdt_start + ssdt_acpi_vm_gid[0];
+        bios_linker_loader_add_pointer(linker, ACPI_BUILD_TABLE_FILE,
+                                       ACPI_BUILD_TABLE_FILE,
+                                       table_data,
+                                       ssdt_ptr + ssdt_acpi_vm_gid_addr[0],
+                                       sizeof(uint32_t));
+    } else {
+        vm_gid_physical_address = 0;
+    }
+
+    ACPI_BUILD_SET_LE(ssdt_ptr, sizeof(ssdp_misc_aml),
+                      ssdt_acpi_vm_gid_addr[0], 32, vm_gid_physical_address);
+
     ACPI_BUILD_SET_LE(ssdt_ptr, sizeof(ssdp_misc_aml),
                       ssdt_mctrl_nr_slots[0], 32, nr_mem);
 
diff --git a/hw/i386/ssdt-misc.dsl b/hw/i386/ssdt-misc.dsl
index 0fd4480..630cd35 100644
--- a/hw/i386/ssdt-misc.dsl
+++ b/hw/i386/ssdt-misc.dsl
@@ -118,6 +118,42 @@ DefinitionBlock ("ssdt-misc.aml", "SSDT", 0x01, "BXPC", "BXSSDTSUSP", 0x1)
         }
     }
 
+/****************************************************************
+ * Virtual Machine Generation ID Device
+ ****************************************************************/
+    Scope(\_SB) {
+
+        Device(VMGI) {
+            Name(_HID, "QEMU0002")
+            Name(_CID, "VM_Gen_Counter")
+            Name(_DDN, "VM_Gen_Counter")
+
+            ACPI_EXTRACT_NAME_DWORD_CONST ssdt_acpi_vm_gid_addr
+            Name(VGIA, 0x12345678)
+
+            ACPI_EXTRACT_NAME_BUFFER16 ssdt_acpi_vm_gid
+            Name(VGID, Buffer(16) {
+                0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+                0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 })
+
+            Method(_STA, 0, NotSerialized) {
+                Store(VGIA, Local0)
+                If (LEqual(Local0, Zero)) {
+                    Return (0x00)
+                } Else {
+                    Return (0x0F)
+                }
+            }
+
+            Method(ADDR, 0, Serialized) {
+                Store(Package(2) { }, Local0)
+                Store(VGIA, Index(Local0, 0))
+                Store(0x0000, Index(Local0, 1))
+                return (Local0)
+            }
+        }
+    }
+
     External(MEMORY_SLOT_NOTIFY_METHOD, MethodObj)
     Scope(\_SB.PCI0) {
         Device(MEMORY_HOTPLUG_DEVICE) {
diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
index 979e532..c18b800 100644
--- a/hw/misc/Makefile.objs
+++ b/hw/misc/Makefile.objs
@@ -41,3 +41,4 @@ obj-$(CONFIG_SLAVIO) += slavio_misc.o
 obj-$(CONFIG_ZYNQ) += zynq_slcr.o
 
 obj-$(CONFIG_PVPANIC) += pvpanic.o
+obj-$(CONFIG_VMGENID) += vmgenid.o
diff --git a/hw/misc/vmgenid.c b/hw/misc/vmgenid.c
new file mode 100644
index 0000000..76956d1
--- /dev/null
+++ b/hw/misc/vmgenid.c
@@ -0,0 +1,85 @@
+/*
+ *  Virtual Machine Generation ID Device
+ *
+ *  Copyright (C) 2014 Red Hat Inc.
+ *
+ *  Authors: Gal Hammer <ghammer@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#include "hw/i386/pc.h"
+#include "hw/sysbus.h"
+
+#define VMGENID_DEVICE "vmgenid"
+
+#define PROPERTY_UUID "uuid"
+
+#define VMGENID(obj) OBJECT_CHECK(VmGenIdState, (obj), VMGENID_DEVICE)
+
+typedef struct VmGenIdState {
+    SysBusDevice parent_obj;
+    char *guid_arg;
+} VmGenIdState;
+
+bool vm_generation_id(uint8_t id[16])
+{
+    Object *o = object_resolve_path_type("", VMGENID_DEVICE, NULL);
+    char *guid;
+
+    if (!o) {
+        return false;
+    }
+    guid = object_property_get_str(o, PROPERTY_UUID, NULL);
+    /* actual uuid validation was checked during realize. */
+    (void)qemu_uuid_parse(guid, id);
+    return true;
+}
+
+static void vmgenid_realize(DeviceState *dev, Error **errp)
+{
+    VmGenIdState *s = VMGENID(dev);
+    uint8_t id[16];
+
+    if (!s->guid_arg) {
+        error_setg(errp, "missing uuid.");
+        return;
+    }
+
+    if (qemu_uuid_parse(s->guid_arg, id) < 0) {
+        error_setg(errp, "Fail to parse UUID string.");
+        return;
+    }
+}
+
+static Property vmgenid_device_properties[] = {
+    DEFINE_PROP_STRING(PROPERTY_UUID, VmGenIdState, guid_arg),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void vmgenid_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->realize = vmgenid_realize;
+    dc->props = vmgenid_device_properties;
+    dc->cannot_instantiate_with_device_add_yet = false;
+    set_bit(DEVICE_CATEGORY_MISC, dc->categories);
+}
+
+static const TypeInfo vmgenid_device_info = {
+    .name          = VMGENID_DEVICE,
+    .parent        = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(VmGenIdState),
+    .class_init    = vmgenid_class_init,
+};
+
+static void vmgenid_register_types(void)
+{
+    type_register_static(&vmgenid_device_info);
+}
+
+type_init(vmgenid_register_types)
+
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 77316d5..40ecccb 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -290,6 +290,9 @@ void pc_system_firmware_init(MemoryRegion *rom_memory,
 /* pvpanic.c */
 uint16_t pvpanic_port(void);
 
+/* vmgenid.c */
+bool vm_generation_id(uint8_t id[16]);
+
 /* e820 types */
 #define E820_RAM        1
 #define E820_RESERVED   2
-- 
1.9.3

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [Qemu-devel] [PATCH 2/2] i386: Add a Virtual Machine Generation ID device.
  2014-09-01  8:57           ` Paolo Bonzini
@ 2014-09-02 13:02             ` Gal Hammer
  0 siblings, 0 replies; 28+ messages in thread
From: Gal Hammer @ 2014-09-02 13:02 UTC (permalink / raw)
  To: Paolo Bonzini, qemu-devel

On 01/09/2014 11:57, Paolo Bonzini wrote:
> Il 01/09/2014 09:20, Gal Hammer ha scritto:
>>>
>>> We are still in the process of defining which devices/methods go in the
>>> DSDT and which go in the SSDT.  We had bad experiences with ACPI table
>>> migration in 2.1, and one plan to fix them is the following:
>>>
>>> * the DSDT should always be the same size no matter what command line
>>> options are there
>>>
>>> * the SSDT should have the exact same content (byte-for-byte) for
>>> different versions of QEMU, with the same command line options
>>> (including the machine type).
>>>
>>> Right now your code obeys the first rule, not the second rule, so it
>>> should add the device to the DSDT.
>>
>> Are you sure about selecting the DSDT? I don't see anyone else is using
>> the ACPI_EXTRACT_NAME_* macros in the DSDT table (and I keep crashing my
>> guest, but ignore it for now ;-)).
>
> There is one user:
>
> acpi-dsdt-isa.dsl:        ACPI_EXTRACT_NAME_BYTE_CONST DSDT_APPLESMC_STA

Found it after I've stopped looking only at the acpi-dsdt.dsl file.

>>> BTW, which events would cause the ID to change?  How should live cloning
>>> (or revert to a disk+RAM snapshot) be implemented by layers above QEMU
>>> for the VM gen ID to be patched?  Can you add something to docs/ about
>>> it?
>>
>> The VGID is expected to change when executing a VM with the -snapshot
>> option, when a VM is restored from a backup or when it is imported,
>> copied or cloned. So I would say it is a management's call.
>
> Ok, got it.
>
> So to support migration (which includes reverting to an earlier disk+RAM
> snapshot) you just need to ensure the VGID is patched accordingly.
> Whether VGID _will_ be different or not, that's management's call.
>
>> I think that the Microsoft's document describes the requirements better
>> than me :-).
>>
>>> Also, how does this ID compare to the UUID in the DMI info (-uuid)?
>>
>> The -uuid is not expected to change after the VM was created. Unlike the
>> -vmgenid that is designed to give the guest OS a notification that a
>> change has occurred. Microsoft, as an example, writes that is can be use
>> for a safer cryptographic software.
>
> I would say that cloning should change the UUID (and the VMGID).

Again, that's a management call as well. One might choose not to change 
the BIOS UUID when cloning the machine in order to prevent Windows from 
trying to re-activate itself (or any other licensed software for that 
matter).

> Paolo
>

A V2 of the patch was send to the list.

     Gal.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [Qemu-devel] [PATCH 2/2] i386: Add a Virtual Machine Generation ID device.
  2014-09-02 12:53 [Qemu-devel] [PATCH 0/2 V2] Virtual Machine Generation ID Gal Hammer
@ 2014-09-02 12:53 ` Gal Hammer
  0 siblings, 0 replies; 28+ messages in thread
From: Gal Hammer @ 2014-09-02 12:53 UTC (permalink / raw)
  To: qemu-devel; +Cc: Gal Hammer, pbonzini, armbru

Based on Microsoft's sepecifications (paper can be dowloaded from
http://go.microsoft.com/fwlink/?LinkId=260709), add a device
description to the DSDT ACPI table.

The GUID is set using a new "vmgenid" device.

Signed-off-by: Gal Hammer <ghammer@redhat.com>

---
 default-configs/i386-softmmu.mak   |  1 +
 default-configs/x86_64-softmmu.mak |  1 +
 hw/i386/acpi-build.c               | 23 ++++++++++-
 hw/i386/acpi-dsdt.dsl              | 37 +++++++++++++++++
 hw/misc/Makefile.objs              |  1 +
 hw/misc/vmgenid.c                  | 85 ++++++++++++++++++++++++++++++++++++++
 include/hw/i386/pc.h               |  3 ++
 7 files changed, 149 insertions(+), 2 deletions(-)
 create mode 100644 hw/misc/vmgenid.c

diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak
index 8e08841..bd33c75 100644
--- a/default-configs/i386-softmmu.mak
+++ b/default-configs/i386-softmmu.mak
@@ -45,3 +45,4 @@ CONFIG_IOAPIC=y
 CONFIG_ICC_BUS=y
 CONFIG_PVPANIC=y
 CONFIG_MEM_HOTPLUG=y
+CONFIG_VMGENID=y
diff --git a/default-configs/x86_64-softmmu.mak b/default-configs/x86_64-softmmu.mak
index 66557ac..006fc7c 100644
--- a/default-configs/x86_64-softmmu.mak
+++ b/default-configs/x86_64-softmmu.mak
@@ -45,3 +45,4 @@ CONFIG_IOAPIC=y
 CONFIG_ICC_BUS=y
 CONFIG_PVPANIC=y
 CONFIG_MEM_HOTPLUG=y
+CONFIG_VMGENID=y
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 85e5834..153dadc 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1390,15 +1390,34 @@ build_mcfg_q35(GArray *table_data, GArray *linker, AcpiMcfgInfo *info)
 static void
 build_dsdt(GArray *table_data, GArray *linker, AcpiMiscInfo *misc)
 {
-    AcpiTableHeader *dsdt;
+    int dsdt_start = table_data->len;
+    uint8_t *dsdt;
+    uint8_t *vm_gid_ptr;
+    uint32_t vm_gid_physical_address;
 
     assert(misc->dsdt_code && misc->dsdt_size);
 
     dsdt = acpi_data_push(table_data, misc->dsdt_size);
     memcpy(dsdt, misc->dsdt_code, misc->dsdt_size);
 
+    vm_gid_ptr = acpi_data_get_ptr(dsdt, misc->dsdt_size,
+                                   *dsdt_acpi_vm_gid, 16);
+    if (vm_generation_id(vm_gid_ptr)) {
+        vm_gid_physical_address = dsdt_start + *dsdt_acpi_vm_gid;
+        bios_linker_loader_add_pointer(linker, ACPI_BUILD_TABLE_FILE,
+                                       ACPI_BUILD_TABLE_FILE,
+                                       table_data,
+                                       dsdt + *dsdt_acpi_vm_gid_addr,
+                                       sizeof(uint32_t));
+    } else {
+        vm_gid_physical_address = 0;
+    }
+
+    ACPI_BUILD_SET_LE(dsdt, misc->dsdt_size,
+                      *dsdt_acpi_vm_gid_addr, 32, vm_gid_physical_address);
+
     memset(dsdt, 0, sizeof *dsdt);
-    build_header(linker, table_data, dsdt, "DSDT",
+    build_header(linker, table_data, (AcpiTableHeader *)dsdt, "DSDT",
                  misc->dsdt_size, 1);
 }
 
diff --git a/hw/i386/acpi-dsdt.dsl b/hw/i386/acpi-dsdt.dsl
index 559f4b6..9015881 100644
--- a/hw/i386/acpi-dsdt.dsl
+++ b/hw/i386/acpi-dsdt.dsl
@@ -294,6 +294,43 @@ DefinitionBlock (
         }
     }
 
+/****************************************************************
+ * Virtual Machine Generation ID Device
+ ****************************************************************/
+    Scope(\_SB) {
+
+        Device(VMGI) {
+            Name(_HID, "QEMU0002")
+            Name(_CID, "VM_Gen_Counter")
+            Name(_DDN, "VM_Gen_Counter")
+
+            ACPI_EXTRACT_NAME_DWORD_CONST dsdt_acpi_vm_gid_addr
+            Name(VGIA, 0x12345678)
+
+            ACPI_EXTRACT_NAME_BUFFER16 dsdt_acpi_vm_gid
+            Name(VGID, Buffer(16) {
+                0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+                0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 })
+
+            Method(_STA, 0, NotSerialized) {
+                Store(VGIA, Local0)
+                If (LEqual(Local0, Zero)) {
+                    Return (0x00)
+                } Else {
+                    Return (0x0F)
+                }
+            }
+
+            Method(ADDR, 0, Serialized) {
+                Store(Package(2) { }, Local0)
+                Store(VGIA, Index(Local0, 0))
+                Store(0x0000, Index(Local0, 1))
+                return (Local0)
+            }
+        }
+    }
+
+
 #include "hw/acpi/pc-hotplug.h"
 #define CPU_STATUS_BASE PIIX4_CPU_HOTPLUG_IO_BASE
 #include "acpi-dsdt-cpu-hotplug.dsl"
diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
index 979e532..c18b800 100644
--- a/hw/misc/Makefile.objs
+++ b/hw/misc/Makefile.objs
@@ -41,3 +41,4 @@ obj-$(CONFIG_SLAVIO) += slavio_misc.o
 obj-$(CONFIG_ZYNQ) += zynq_slcr.o
 
 obj-$(CONFIG_PVPANIC) += pvpanic.o
+obj-$(CONFIG_VMGENID) += vmgenid.o
diff --git a/hw/misc/vmgenid.c b/hw/misc/vmgenid.c
new file mode 100644
index 0000000..76956d1
--- /dev/null
+++ b/hw/misc/vmgenid.c
@@ -0,0 +1,85 @@
+/*
+ *  Virtual Machine Generation ID Device
+ *
+ *  Copyright (C) 2014 Red Hat Inc.
+ *
+ *  Authors: Gal Hammer <ghammer@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#include "hw/i386/pc.h"
+#include "hw/sysbus.h"
+
+#define VMGENID_DEVICE "vmgenid"
+
+#define PROPERTY_UUID "uuid"
+
+#define VMGENID(obj) OBJECT_CHECK(VmGenIdState, (obj), VMGENID_DEVICE)
+
+typedef struct VmGenIdState {
+    SysBusDevice parent_obj;
+    char *guid_arg;
+} VmGenIdState;
+
+bool vm_generation_id(uint8_t id[16])
+{
+    Object *o = object_resolve_path_type("", VMGENID_DEVICE, NULL);
+    char *guid;
+
+    if (!o) {
+        return false;
+    }
+    guid = object_property_get_str(o, PROPERTY_UUID, NULL);
+    /* actual uuid validation was checked during realize. */
+    (void)qemu_uuid_parse(guid, id);
+    return true;
+}
+
+static void vmgenid_realize(DeviceState *dev, Error **errp)
+{
+    VmGenIdState *s = VMGENID(dev);
+    uint8_t id[16];
+
+    if (!s->guid_arg) {
+        error_setg(errp, "missing uuid.");
+        return;
+    }
+
+    if (qemu_uuid_parse(s->guid_arg, id) < 0) {
+        error_setg(errp, "Fail to parse UUID string.");
+        return;
+    }
+}
+
+static Property vmgenid_device_properties[] = {
+    DEFINE_PROP_STRING(PROPERTY_UUID, VmGenIdState, guid_arg),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void vmgenid_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->realize = vmgenid_realize;
+    dc->props = vmgenid_device_properties;
+    dc->cannot_instantiate_with_device_add_yet = false;
+    set_bit(DEVICE_CATEGORY_MISC, dc->categories);
+}
+
+static const TypeInfo vmgenid_device_info = {
+    .name          = VMGENID_DEVICE,
+    .parent        = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(VmGenIdState),
+    .class_init    = vmgenid_class_init,
+};
+
+static void vmgenid_register_types(void)
+{
+    type_register_static(&vmgenid_device_info);
+}
+
+type_init(vmgenid_register_types)
+
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index a39cb42..b21c22e 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -290,6 +290,9 @@ void pc_system_firmware_init(MemoryRegion *rom_memory,
 /* pvpanic.c */
 uint16_t pvpanic_port(void);
 
+/* vmgenid.c */
+bool vm_generation_id(uint8_t id[16]);
+
 /* e820 types */
 #define E820_RAM        1
 #define E820_RESERVED   2
-- 
1.9.3

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [Qemu-devel] [PATCH 2/2] i386: Add a Virtual Machine Generation ID device.
  2014-09-01  7:20         ` Gal Hammer
@ 2014-09-01  8:57           ` Paolo Bonzini
  2014-09-02 13:02             ` Gal Hammer
  0 siblings, 1 reply; 28+ messages in thread
From: Paolo Bonzini @ 2014-09-01  8:57 UTC (permalink / raw)
  To: Gal Hammer, qemu-devel

Il 01/09/2014 09:20, Gal Hammer ha scritto:
>>
>> We are still in the process of defining which devices/methods go in the
>> DSDT and which go in the SSDT.  We had bad experiences with ACPI table
>> migration in 2.1, and one plan to fix them is the following:
>>
>> * the DSDT should always be the same size no matter what command line
>> options are there
>>
>> * the SSDT should have the exact same content (byte-for-byte) for
>> different versions of QEMU, with the same command line options
>> (including the machine type).
>>
>> Right now your code obeys the first rule, not the second rule, so it
>> should add the device to the DSDT.
> 
> Are you sure about selecting the DSDT? I don't see anyone else is using
> the ACPI_EXTRACT_NAME_* macros in the DSDT table (and I keep crashing my
> guest, but ignore it for now ;-)).

There is one user:

acpi-dsdt-isa.dsl:        ACPI_EXTRACT_NAME_BYTE_CONST DSDT_APPLESMC_STA

>> BTW, which events would cause the ID to change?  How should live cloning
>> (or revert to a disk+RAM snapshot) be implemented by layers above QEMU
>> for the VM gen ID to be patched?  Can you add something to docs/ about
>> it?
> 
> The VGID is expected to change when executing a VM with the -snapshot
> option, when a VM is restored from a backup or when it is imported,
> copied or cloned. So I would say it is a management's call.

Ok, got it.

So to support migration (which includes reverting to an earlier disk+RAM
snapshot) you just need to ensure the VGID is patched accordingly.
Whether VGID _will_ be different or not, that's management's call.

> I think that the Microsoft's document describes the requirements better
> than me :-).
> 
>> Also, how does this ID compare to the UUID in the DMI info (-uuid)?
> 
> The -uuid is not expected to change after the VM was created. Unlike the
> -vmgenid that is designed to give the guest OS a notification that a
> change has occurred. Microsoft, as an example, writes that is can be use
> for a safer cryptographic software.

I would say that cloning should change the UUID (and the VMGID).

Paolo

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [Qemu-devel] [PATCH 2/2] i386: Add a Virtual Machine Generation ID device.
  2014-08-17  9:49       ` Paolo Bonzini
  2014-08-18  8:47         ` Markus Armbruster
@ 2014-09-01  7:20         ` Gal Hammer
  2014-09-01  8:57           ` Paolo Bonzini
  1 sibling, 1 reply; 28+ messages in thread
From: Gal Hammer @ 2014-09-01  7:20 UTC (permalink / raw)
  To: Paolo Bonzini, qemu-devel

On 17/08/2014 12:49, Paolo Bonzini wrote:
> Il 12/08/2014 10:02, Gal Hammer ha scritto:
>> Hi,
>>
>> On 10/08/2014 20:22, Paolo Bonzini wrote:
>>
>>> Il 10/08/2014 13:32, Gal Hammer ha scritto:
>>>> Based on Microsoft's sepecifications (paper can be dowloaded from
>>>> http://go.microsoft.com/fwlink/?LinkId=260709), add a device
>>>> description to the SSDT ACPI table.
>>>>
>>>> The GUID is set using a new "-vmgenid" command line parameter.
>>>>
>>>> Signed-off-by: Gal Hammer <ghammer@redhat.com>
>>>> ---
>>>>    hw/i386/acpi-build.c  | 23 +++++++++++++++++++++++
>>>>    hw/i386/ssdt-misc.dsl | 33 +++++++++++++++++++++++++++++++++
>>>>    qemu-options.hx       |  9 +++++++++
>>>>    vl.c                  | 11 +++++++++++
>>>>    4 files changed, 76 insertions(+)
>>>
>>> Please make this a new device (like pvpanic), instead of adding a new
>>> command-line option.
>>
>> There is a problem with this request. I don't want to use ISA because it
>> is obsolete, PCI is overkill for such a device and a SYSBUS (like HPET)
>> device doesn't effect the command line options.
>>
>> Did I miss something in SYSBUS and that's was the reason it didn't
>> appear in the "-device ?" list?
>
> For a sysbus device, you can override the
> cannot_instantiate_with_device_add_yet field of DeviceClass in your
> class_init function.
>
>>>>
>>>> +    Scope(\_SB) {
>>>> +
>>>> +        Device(VMGI) {
>>>> +            Name(_HID, "QEMU0002")
>>>> +            Name(_CID, "VM_Gen_Counter")
>>>> +            Name(_DDN, "VM_Gen_Counter")
>>>> +
>>>> +            ACPI_EXTRACT_NAME_DWORD_CONST ssdt_acpi_vm_gid_addr
>>>> +            Name(VGIA, 0x12345678)
>>>> +
>>>> +            ACPI_EXTRACT_NAME_BUFFER16 ssdt_acpi_vm_gid
>>>> +            Name(VGID, Buffer(16) {
>>>> +                0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
>>>> +                0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 })
>>>> +
>>>> +            Method(_STA, 0, NotSerialized) {
>>>> +                Store(VGIA, Local0)
>>>> +                If (LEqual(Local0, Zero)) {
>>>> +                    Return (0x00)
>>>> +                } Else {
>>>> +                    Return (0x0F)
>>>> +                }
>>>> +            }
>>>> +
>>>> +            Method(ADDR, 0, Serialized) {
>>>> +                Store(Package(2) { }, Local0)
>>>> +                Store(VGIA, Index(Local0, 0))
>>>> +                Store(0x0000, Index(Local0, 1))
>>>> +                return (Local0)
>>>> +            }
>>>> +        }
>>>> +    }
>>>> +
>>>
>>> Please either put this in the DSDT, or omit the Device altogether if you
>>> put it in the SSDT and there is no VMGID device.
>>
>> I'm not sure I understand this comment. I've put the new device in the
>> SSDT table (like pvpanic) and add a _STA method which disable the device
>> if no GUID's address is set (VGIA). The device doesn't show in the
>> Device Manager if it was not added using the command line.
>
> We are still in the process of defining which devices/methods go in the
> DSDT and which go in the SSDT.  We had bad experiences with ACPI table
> migration in 2.1, and one plan to fix them is the following:
>
> * the DSDT should always be the same size no matter what command line
> options are there
>
> * the SSDT should have the exact same content (byte-for-byte) for
> different versions of QEMU, with the same command line options
> (including the machine type).
>
> Right now your code obeys the first rule, not the second rule, so it
> should add the device to the DSDT.

Are you sure about selecting the DSDT? I don't see anyone else is using 
the ACPI_EXTRACT_NAME_* macros in the DSDT table (and I keep crashing my 
guest, but ignore it for now ;-)).

> BTW, which events would cause the ID to change?  How should live cloning
> (or revert to a disk+RAM snapshot) be implemented by layers above QEMU
> for the VM gen ID to be patched?  Can you add something to docs/ about it?

The VGID is expected to change when executing a VM with the -snapshot 
option, when a VM is restored from a backup or when it is imported, 
copied or cloned. So I would say it is a management's call.

I think that the Microsoft's document describes the requirements better 
than me :-).

> Also, how does this ID compare to the UUID in the DMI info (-uuid)?

The -uuid is not expected to change after the VM was created. Unlike the 
-vmgenid that is designed to give the guest OS a notification that a 
change has occurred. Microsoft, as an example, writes that is can be use 
for a safer cryptographic software.

> Paolo
>

     Gal.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [Qemu-devel] [PATCH 2/2] i386: Add a Virtual Machine Generation ID device.
  2014-08-17  9:49       ` Paolo Bonzini
@ 2014-08-18  8:47         ` Markus Armbruster
  2014-09-01  7:20         ` Gal Hammer
  1 sibling, 0 replies; 28+ messages in thread
From: Markus Armbruster @ 2014-08-18  8:47 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: Gal Hammer, qemu-devel

Paolo Bonzini <pbonzini@redhat.com> writes:

> Il 12/08/2014 10:02, Gal Hammer ha scritto:
>> Hi,
>> 
>> On 10/08/2014 20:22, Paolo Bonzini wrote:
>> 
>>> Il 10/08/2014 13:32, Gal Hammer ha scritto:
>>>> Based on Microsoft's sepecifications (paper can be dowloaded from
>>>> http://go.microsoft.com/fwlink/?LinkId=260709), add a device
>>>> description to the SSDT ACPI table.
>>>>
>>>> The GUID is set using a new "-vmgenid" command line parameter.
>>>>
>>>> Signed-off-by: Gal Hammer <ghammer@redhat.com>
>>>> ---
>>>>   hw/i386/acpi-build.c  | 23 +++++++++++++++++++++++
>>>>   hw/i386/ssdt-misc.dsl | 33 +++++++++++++++++++++++++++++++++
>>>>   qemu-options.hx       |  9 +++++++++
>>>>   vl.c                  | 11 +++++++++++
>>>>   4 files changed, 76 insertions(+)
>>>
>>> Please make this a new device (like pvpanic), instead of adding a new
>>> command-line option.
>> 
>> There is a problem with this request. I don't want to use ISA because it
>> is obsolete, PCI is overkill for such a device and a SYSBUS (like HPET)
>> device doesn't effect the command line options.
>> 
>> Did I miss something in SYSBUS and that's was the reason it didn't
>> appear in the "-device ?" list?
>
> For a sysbus device, you can override the
> cannot_instantiate_with_device_add_yet field of DeviceClass in your
> class_init function.

Correct.

Sysbus devices are not available with device_add / -device by default,
because to actually work, they commonly require code to connect them to
other devices.  A sysbus device that doesn't need such connections can
be made available with device_add / -device in the way Paolo described.

[...]

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [Qemu-devel] [PATCH 2/2] i386: Add a Virtual Machine Generation ID device.
  2014-08-12  8:02     ` Gal Hammer
@ 2014-08-17  9:49       ` Paolo Bonzini
  2014-08-18  8:47         ` Markus Armbruster
  2014-09-01  7:20         ` Gal Hammer
  0 siblings, 2 replies; 28+ messages in thread
From: Paolo Bonzini @ 2014-08-17  9:49 UTC (permalink / raw)
  To: Gal Hammer, qemu-devel

Il 12/08/2014 10:02, Gal Hammer ha scritto:
> Hi,
> 
> On 10/08/2014 20:22, Paolo Bonzini wrote:
> 
>> Il 10/08/2014 13:32, Gal Hammer ha scritto:
>>> Based on Microsoft's sepecifications (paper can be dowloaded from
>>> http://go.microsoft.com/fwlink/?LinkId=260709), add a device
>>> description to the SSDT ACPI table.
>>>
>>> The GUID is set using a new "-vmgenid" command line parameter.
>>>
>>> Signed-off-by: Gal Hammer <ghammer@redhat.com>
>>> ---
>>>   hw/i386/acpi-build.c  | 23 +++++++++++++++++++++++
>>>   hw/i386/ssdt-misc.dsl | 33 +++++++++++++++++++++++++++++++++
>>>   qemu-options.hx       |  9 +++++++++
>>>   vl.c                  | 11 +++++++++++
>>>   4 files changed, 76 insertions(+)
>>
>> Please make this a new device (like pvpanic), instead of adding a new
>> command-line option.
> 
> There is a problem with this request. I don't want to use ISA because it
> is obsolete, PCI is overkill for such a device and a SYSBUS (like HPET)
> device doesn't effect the command line options.
> 
> Did I miss something in SYSBUS and that's was the reason it didn't
> appear in the "-device ?" list?

For a sysbus device, you can override the
cannot_instantiate_with_device_add_yet field of DeviceClass in your
class_init function.

>>>
>>> +    Scope(\_SB) {
>>> +
>>> +        Device(VMGI) {
>>> +            Name(_HID, "QEMU0002")
>>> +            Name(_CID, "VM_Gen_Counter")
>>> +            Name(_DDN, "VM_Gen_Counter")
>>> +
>>> +            ACPI_EXTRACT_NAME_DWORD_CONST ssdt_acpi_vm_gid_addr
>>> +            Name(VGIA, 0x12345678)
>>> +
>>> +            ACPI_EXTRACT_NAME_BUFFER16 ssdt_acpi_vm_gid
>>> +            Name(VGID, Buffer(16) {
>>> +                0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
>>> +                0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 })
>>> +
>>> +            Method(_STA, 0, NotSerialized) {
>>> +                Store(VGIA, Local0)
>>> +                If (LEqual(Local0, Zero)) {
>>> +                    Return (0x00)
>>> +                } Else {
>>> +                    Return (0x0F)
>>> +                }
>>> +            }
>>> +
>>> +            Method(ADDR, 0, Serialized) {
>>> +                Store(Package(2) { }, Local0)
>>> +                Store(VGIA, Index(Local0, 0))
>>> +                Store(0x0000, Index(Local0, 1))
>>> +                return (Local0)
>>> +            }
>>> +        }
>>> +    }
>>> +
>>
>> Please either put this in the DSDT, or omit the Device altogether if you
>> put it in the SSDT and there is no VMGID device.
> 
> I'm not sure I understand this comment. I've put the new device in the
> SSDT table (like pvpanic) and add a _STA method which disable the device
> if no GUID's address is set (VGIA). The device doesn't show in the
> Device Manager if it was not added using the command line.

We are still in the process of defining which devices/methods go in the
DSDT and which go in the SSDT.  We had bad experiences with ACPI table
migration in 2.1, and one plan to fix them is the following:

* the DSDT should always be the same size no matter what command line
options are there

* the SSDT should have the exact same content (byte-for-byte) for
different versions of QEMU, with the same command line options
(including the machine type).

Right now your code obeys the first rule, not the second rule, so it
should add the device to the DSDT.

BTW, which events would cause the ID to change?  How should live cloning
(or revert to a disk+RAM snapshot) be implemented by layers above QEMU
for the VM gen ID to be patched?  Can you add something to docs/ about it?

Also, how does this ID compare to the UUID in the DMI info (-uuid)?

Paolo

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [Qemu-devel] [PATCH 2/2] i386: Add a Virtual Machine Generation ID device.
  2014-08-10 17:22   ` Paolo Bonzini
@ 2014-08-12  8:02     ` Gal Hammer
  2014-08-17  9:49       ` Paolo Bonzini
  0 siblings, 1 reply; 28+ messages in thread
From: Gal Hammer @ 2014-08-12  8:02 UTC (permalink / raw)
  To: Paolo Bonzini, qemu-devel

Hi,

On 10/08/2014 20:22, Paolo Bonzini wrote:

> Il 10/08/2014 13:32, Gal Hammer ha scritto:
>> Based on Microsoft's sepecifications (paper can be dowloaded from
>> http://go.microsoft.com/fwlink/?LinkId=260709), add a device
>> description to the SSDT ACPI table.
>>
>> The GUID is set using a new "-vmgenid" command line parameter.
>>
>> Signed-off-by: Gal Hammer <ghammer@redhat.com>
>> ---
>>   hw/i386/acpi-build.c  | 23 +++++++++++++++++++++++
>>   hw/i386/ssdt-misc.dsl | 33 +++++++++++++++++++++++++++++++++
>>   qemu-options.hx       |  9 +++++++++
>>   vl.c                  | 11 +++++++++++
>>   4 files changed, 76 insertions(+)
>
> Please make this a new device (like pvpanic), instead of adding a new
> command-line option.

There is a problem with this request. I don't want to use ISA because it 
is obsolete, PCI is overkill for such a device and a SYSBUS (like HPET) 
device doesn't effect the command line options.

Did I miss something in SYSBUS and that's was the reason it didn't 
appear in the "-device ?" list?

>>
>> +    Scope(\_SB) {
>> +
>> +        Device(VMGI) {
>> +            Name(_HID, "QEMU0002")
>> +            Name(_CID, "VM_Gen_Counter")
>> +            Name(_DDN, "VM_Gen_Counter")
>> +
>> +            ACPI_EXTRACT_NAME_DWORD_CONST ssdt_acpi_vm_gid_addr
>> +            Name(VGIA, 0x12345678)
>> +
>> +            ACPI_EXTRACT_NAME_BUFFER16 ssdt_acpi_vm_gid
>> +            Name(VGID, Buffer(16) {
>> +                0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
>> +                0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 })
>> +
>> +            Method(_STA, 0, NotSerialized) {
>> +                Store(VGIA, Local0)
>> +                If (LEqual(Local0, Zero)) {
>> +                    Return (0x00)
>> +                } Else {
>> +                    Return (0x0F)
>> +                }
>> +            }
>> +
>> +            Method(ADDR, 0, Serialized) {
>> +                Store(Package(2) { }, Local0)
>> +                Store(VGIA, Index(Local0, 0))
>> +                Store(0x0000, Index(Local0, 1))
>> +                return (Local0)
>> +            }
>> +        }
>> +    }
>> +
>
> Please either put this in the DSDT, or omit the Device altogether if you
> put it in the SSDT and there is no VMGID device.

I'm not sure I understand this comment. I've put the new device in the 
SSDT table (like pvpanic) and add a _STA method which disable the device 
if no GUID's address is set (VGIA). The device doesn't show in the 
Device Manager if it was not added using the command line.

     Gal.

> Thanks,
>
> Paolo
>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [Qemu-devel] [PATCH 2/2] i386: Add a Virtual Machine Generation ID device.
  2014-08-10 11:32 ` [Qemu-devel] [PATCH 2/2] i386: Add a Virtual Machine Generation ID device Gal Hammer
@ 2014-08-10 17:22   ` Paolo Bonzini
  2014-08-12  8:02     ` Gal Hammer
  0 siblings, 1 reply; 28+ messages in thread
From: Paolo Bonzini @ 2014-08-10 17:22 UTC (permalink / raw)
  To: Gal Hammer, qemu-devel

Il 10/08/2014 13:32, Gal Hammer ha scritto:
> Based on Microsoft's sepecifications (paper can be dowloaded from
> http://go.microsoft.com/fwlink/?LinkId=260709), add a device
> description to the SSDT ACPI table.
> 
> The GUID is set using a new "-vmgenid" command line parameter.
> 
> Signed-off-by: Gal Hammer <ghammer@redhat.com>
> ---
>  hw/i386/acpi-build.c  | 23 +++++++++++++++++++++++
>  hw/i386/ssdt-misc.dsl | 33 +++++++++++++++++++++++++++++++++
>  qemu-options.hx       |  9 +++++++++
>  vl.c                  | 11 +++++++++++
>  4 files changed, 76 insertions(+)

Please make this a new device (like pvpanic), instead of adding a new
command-line option.

> 
> +    Scope(\_SB) {
> +
> +        Device(VMGI) {
> +            Name(_HID, "QEMU0002")
> +            Name(_CID, "VM_Gen_Counter")
> +            Name(_DDN, "VM_Gen_Counter")
> +
> +            ACPI_EXTRACT_NAME_DWORD_CONST ssdt_acpi_vm_gid_addr
> +            Name(VGIA, 0x12345678)
> +
> +            ACPI_EXTRACT_NAME_BUFFER16 ssdt_acpi_vm_gid
> +            Name(VGID, Buffer(16) {
> +                0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +                0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 })
> +
> +            Method(_STA, 0, NotSerialized) {
> +                Store(VGIA, Local0)
> +                If (LEqual(Local0, Zero)) {
> +                    Return (0x00)
> +                } Else {
> +                    Return (0x0F)
> +                }
> +            }
> +
> +            Method(ADDR, 0, Serialized) {
> +                Store(Package(2) { }, Local0)
> +                Store(VGIA, Index(Local0, 0))
> +                Store(0x0000, Index(Local0, 1))
> +                return (Local0)
> +            }
> +        }
> +    }
> +

Please either put this in the DSDT, or omit the Device altogether if you
put it in the SSDT and there is no VMGID device.

Thanks,

Paolo

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [Qemu-devel] [PATCH 2/2] i386: Add a Virtual Machine Generation ID device.
  2014-08-10 11:32 [Qemu-devel] [PATCH 0/2] Virtual Machine Generation ID Gal Hammer
@ 2014-08-10 11:32 ` Gal Hammer
  2014-08-10 17:22   ` Paolo Bonzini
  0 siblings, 1 reply; 28+ messages in thread
From: Gal Hammer @ 2014-08-10 11:32 UTC (permalink / raw)
  To: qemu-devel; +Cc: Gal Hammer

Based on Microsoft's sepecifications (paper can be dowloaded from
http://go.microsoft.com/fwlink/?LinkId=260709), add a device
description to the SSDT ACPI table.

The GUID is set using a new "-vmgenid" command line parameter.

Signed-off-by: Gal Hammer <ghammer@redhat.com>
---
 hw/i386/acpi-build.c  | 23 +++++++++++++++++++++++
 hw/i386/ssdt-misc.dsl | 33 +++++++++++++++++++++++++++++++++
 qemu-options.hx       |  9 +++++++++
 vl.c                  | 11 +++++++++++
 4 files changed, 76 insertions(+)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 816c6d9..838c72c 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -54,6 +54,9 @@
 #include "qapi/qmp/qint.h"
 #include "qom/qom-qobject.h"
 
+extern uint8_t vm_generation_id[16];
+extern bool vm_generation_id_set;
+
 /* These are used to size the ACPI tables for -M pc-i440fx-1.7 and
  * -M pc-i440fx-2.0.  Even if the actual amount of AML generated grows
  * a little bit, there should be plenty of free space since the DSDT
@@ -1051,6 +1054,7 @@ build_ssdt(GArray *table_data, GArray *linker,
     unsigned acpi_cpus = guest_info->apic_id_limit;
     int ssdt_start = table_data->len;
     uint8_t *ssdt_ptr;
+    uint32_t vm_gid_physical_address;
     int i;
 
     /* The current AML generator can cover the APIC ID range [0..255],
@@ -1076,6 +1080,25 @@ build_ssdt(GArray *table_data, GArray *linker,
     ACPI_BUILD_SET_LE(ssdt_ptr, sizeof(ssdp_misc_aml),
                       ssdt_isa_pest[0], 16, misc->pvpanic_port);
 
+    if (vm_generation_id_set) {
+        uint8_t *vm_gid_ptr;
+
+        vm_gid_physical_address = ssdt_start +  ssdt_acpi_vm_gid[0];
+        vm_gid_ptr = acpi_data_get_ptr(ssdt_ptr, sizeof(ssdp_misc_aml),
+                                       ssdt_acpi_vm_gid[0], 16);
+        memcpy(vm_gid_ptr, vm_generation_id, sizeof(vm_generation_id));
+
+        bios_linker_loader_add_pointer(linker, ACPI_BUILD_TABLE_FILE,
+                                       ACPI_BUILD_TABLE_FILE,
+                                       table_data, ssdt_ptr + ssdt_acpi_vm_gid_addr[0],
+                                       sizeof(uint32_t));
+    } else {
+        vm_gid_physical_address = 0;
+    }
+
+    ACPI_BUILD_SET_LE(ssdt_ptr, sizeof(ssdp_misc_aml),
+                      ssdt_acpi_vm_gid_addr[0], 32, vm_gid_physical_address);
+
     ACPI_BUILD_SET_LE(ssdt_ptr, sizeof(ssdp_misc_aml),
                       ssdt_mctrl_nr_slots[0], 32, nr_mem);
 
diff --git a/hw/i386/ssdt-misc.dsl b/hw/i386/ssdt-misc.dsl
index d329b8b..8a001a7 100644
--- a/hw/i386/ssdt-misc.dsl
+++ b/hw/i386/ssdt-misc.dsl
@@ -118,6 +118,39 @@ DefinitionBlock ("ssdt-misc.aml", "SSDT", 0x01, "BXPC", "BXSSDTSUSP", 0x1)
         }
     }
 
+    Scope(\_SB) {
+
+        Device(VMGI) {
+            Name(_HID, "QEMU0002")
+            Name(_CID, "VM_Gen_Counter")
+            Name(_DDN, "VM_Gen_Counter")
+
+            ACPI_EXTRACT_NAME_DWORD_CONST ssdt_acpi_vm_gid_addr
+            Name(VGIA, 0x12345678)
+
+            ACPI_EXTRACT_NAME_BUFFER16 ssdt_acpi_vm_gid
+            Name(VGID, Buffer(16) {
+                0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+                0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 })
+
+            Method(_STA, 0, NotSerialized) {
+                Store(VGIA, Local0)
+                If (LEqual(Local0, Zero)) {
+                    Return (0x00)
+                } Else {
+                    Return (0x0F)
+                }
+            }
+
+            Method(ADDR, 0, Serialized) {
+                Store(Package(2) { }, Local0)
+                Store(VGIA, Index(Local0, 0))
+                Store(0x0000, Index(Local0, 1))
+                return (Local0)
+            }
+        }
+    }
+
     External(MEMORY_SLOT_NOTIFY_METHOD, MethodObj)
     Scope(\_SB.PCI0) {
         Device(MEMORY_HOPTLUG_DEVICE) {
diff --git a/qemu-options.hx b/qemu-options.hx
index 96516c1..a6d475c 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -369,6 +369,15 @@ STEXI
 Set system UUID.
 ETEXI
 
+DEF("vmgenid", HAS_ARG, QEMU_OPTION_vmgenid,
+    "-vmgenid %08x-%04x-%04x-%04x-%012x\n"
+    "                specify the virtual machine generation ID\n", QEMU_ARCH_I386)
+STEXI
+@item -uuid @var{uuid}
+@findex -uuid
+Set the virtual machine generation ID.
+ETEXI
+
 STEXI
 @end table
 ETEXI
diff --git a/vl.c b/vl.c
index a8029d5..a5ef0d5 100644
--- a/vl.c
+++ b/vl.c
@@ -203,6 +203,9 @@ NodeInfo numa_info[MAX_NODES];
 uint8_t qemu_uuid[16];
 bool qemu_uuid_set;
 
+uint8_t vm_generation_id[16];
+bool vm_generation_id_set;
+
 static QEMUBootSetHandler *boot_set_handler;
 static void *boot_set_opaque;
 
@@ -3784,6 +3787,14 @@ int main(int argc, char **argv, char **envp)
                 }
                 qemu_uuid_set = true;
                 break;
+	    case QEMU_OPTION_vmgenid:
+	        if(qemu_uuid_parse(optarg, vm_generation_id) < 0) {
+	            fprintf(stderr, "Fail to parse UUID string."
+	                    " Wrong format.\n");
+	            exit(1);
+	        }
+	        vm_generation_id_set = true;
+	        break;
 	    case QEMU_OPTION_option_rom:
 		if (nb_option_roms >= MAX_OPTION_ROMS) {
 		    fprintf(stderr, "Too many option ROMs\n");
-- 
1.9.3

^ permalink raw reply related	[flat|nested] 28+ messages in thread

end of thread, other threads:[~2014-10-02 13:45 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-09-14  6:25 [Qemu-devel] [PATCH RESEND 0/2 V2] Virtual Machine Generation ID Gal Hammer
2014-09-14  6:25 ` [Qemu-devel] [PATCH 1/2] i386: Add an ACPI_EXTRACT_NAME_BUFFER16 directive Gal Hammer
2014-09-14  8:48   ` Paolo Bonzini
2014-09-14  6:25 ` [Qemu-devel] [PATCH 2/2] i386: Add a Virtual Machine Generation ID device Gal Hammer
2014-09-14  8:48 ` [Qemu-devel] [PATCH RESEND 0/2 V2] Virtual Machine Generation ID Paolo Bonzini
2014-09-14 10:51   ` Gal Hammer
2014-09-14 13:40     ` Paolo Bonzini
  -- strict thread matches above, loose matches on Subject: below --
2014-09-17 11:39 [Qemu-devel] [PATCH 0/2 V5] " Gal Hammer
2014-09-17 11:39 ` [Qemu-devel] [PATCH 2/2] i386: Add a Virtual Machine Generation ID device Gal Hammer
2014-10-02 12:49   ` Michael S. Tsirkin
2014-10-02 13:14     ` Gal Hammer
2014-10-02 13:30       ` Michael S. Tsirkin
2014-09-16 13:04 [Qemu-devel] [PATCH 0/2 V4] Virtual Machine Generation ID Gal Hammer
2014-09-16 13:04 ` [Qemu-devel] [PATCH 2/2] i386: Add a Virtual Machine Generation ID device Gal Hammer
2014-09-16 13:16   ` Paolo Bonzini
2014-09-16 14:54   ` Igor Mammedov
2014-09-17  7:36     ` Gal Hammer
2014-09-17  9:56       ` Igor Mammedov
2014-09-15  7:27 [Qemu-devel] [PATCH 0/2 V3] Virtual Machine Generation ID Gal Hammer
2014-09-15  7:27 ` [Qemu-devel] [PATCH 2/2] i386: Add a Virtual Machine Generation ID device Gal Hammer
2014-09-15  8:46   ` Igor Mammedov
2014-09-15  9:15     ` Gal Hammer
2014-09-02 12:53 [Qemu-devel] [PATCH 0/2 V2] Virtual Machine Generation ID Gal Hammer
2014-09-02 12:53 ` [Qemu-devel] [PATCH 2/2] i386: Add a Virtual Machine Generation ID device Gal Hammer
2014-08-10 11:32 [Qemu-devel] [PATCH 0/2] Virtual Machine Generation ID Gal Hammer
2014-08-10 11:32 ` [Qemu-devel] [PATCH 2/2] i386: Add a Virtual Machine Generation ID device Gal Hammer
2014-08-10 17:22   ` Paolo Bonzini
2014-08-12  8:02     ` Gal Hammer
2014-08-17  9:49       ` Paolo Bonzini
2014-08-18  8:47         ` Markus Armbruster
2014-09-01  7:20         ` Gal Hammer
2014-09-01  8:57           ` Paolo Bonzini
2014-09-02 13:02             ` Gal Hammer

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.