[Qemu-devel] [PATCH v2 0/6] Add support for VM Generation ID

[Qemu-devel] [PATCH v2 0/6] Add support for VM Generation ID

[ben]

From: Ben Warren <ben@skyportsystems.com>

This patch set adds support for passing a GUID to Windows guests.  It is a
re-implementation of previous patch sets written by Igor Mammedov et al, but
this time passing the GUID data as a fw_cfg blob.

This is not yet fully functional because the data in the fw_cfg blob is immutable,
so that any changes via the monitor are not seen by the guest.
When that hurdle is passed, unit tests that work will be added.

v1->v2:
	Removed "changed" boolean parameter as it is unneeded
	Added ACPI Notify logic
	Style changes to pass checkpatch.pl
	Added support for dynamic sysbus to pc_piix boards

Ben Warren (4):
  docs: vm generation id device's description
  ACPI: Add a function for building named qword entries
  ACPI: Add Virtual Machine Generation ID support
  PC: Support dynamic sysbus on pc_i440fx

Igor Mammedov (2):
  qmp/hmp: add query-vm-generation-id and 'info vm-generation-id'
    commands
  qmp/hmp: add set-vm-generation-id commands

 default-configs/i386-softmmu.mak   |   1 +
 default-configs/x86_64-softmmu.mak |   1 +
 docs/specs/vmgenid.txt             |  38 +++++++
 hmp-commands-info.hx               |  13 +++
 hmp-commands.hx                    |  13 +++
 hmp.c                              |  21 ++++
 hmp.h                              |   2 +
 hw/acpi/Makefile.objs              |   1 +
 hw/acpi/aml-build.c                |  28 +++++
 hw/acpi/vmgenid.c                  | 208 +++++++++++++++++++++++++++++++++++++
 hw/i386/acpi-build.c               |   5 +
 hw/i386/pc_piix.c                  |   1 +
 hw/misc/Makefile.objs              |   1 +
 include/hw/acpi/aml-build.h        |   4 +
 include/hw/acpi/vmgenid.h          |  24 +++++
 qapi-schema.json                   |  32 ++++++
 stubs/Makefile.objs                |   1 +
 stubs/vmgenid.c                    |  14 +++
 18 files changed, 408 insertions(+)
 create mode 100644 docs/specs/vmgenid.txt
 create mode 100644 hw/acpi/vmgenid.c
 create mode 100644 include/hw/acpi/vmgenid.h
 create mode 100644 stubs/vmgenid.c

-- 
2.7.4

[Qemu-devel] [PATCH v2 1/6] docs: vm generation id device's description

[ben]

From: Ben Warren <ben@skyportsystems.com>

Signed-off-by: Ben Warren <ben@skyportsystems.com>
Cc: Gal Hammer <ghammer@redhat.com>
---
 docs/specs/vmgenid.txt | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)
 create mode 100644 docs/specs/vmgenid.txt

diff --git a/docs/specs/vmgenid.txt b/docs/specs/vmgenid.txt
new file mode 100644
index 0000000..afc1717
--- /dev/null
+++ b/docs/specs/vmgenid.txt
@@ -0,0 +1,38 @@
+VIRTUAL MACHINE GENERATION ID
+=============================
+
+Copyright (C) 2016 Red Hat, Inc.
+Copyright (C) 2016 Skyport Systems, Inc.
+
+This work is licensed under the terms of the GNU GPL, version 2 or later.
+See the COPYING file in the top-level directory.
+
+===
+
+The VM generation ID (vmgenid) device is an emulated device which
+exposes a 128-bit, cryptographically random, integer value identifier.
+This allows management applications (e.g. libvirt) to notify the guest
+operating system when the virtual machine is executed with a different
+configuration (e.g. snapshot execution or creation from a template).
+
+This is specified on the web at: http://go.microsoft.com/fwlink/?LinkId=260709
+
+---
+
+The vmgenid device is a sysbus device with the ACPI ID "QEMU_Gen_Counter_V1".
+
+The device has one properties, which can be set using the command line
+argument or the QMP interface:
+ guid - sets the value of the GUID
+For example:
+QEMU  -device vmgenid,guid="324e6eaf-d1d1-4bf6-bf41-b9bb6c91fb87"
+
+Or to change guid in runtime use:
+ set-vm-generation-id guid="124e6eaf-d1d1-4bf6-bf41-b9bb6c91fb87"
+
+According to the specification, any change to the GUID executes an
+ACPI notification. The vmgenid device triggers the \_GPE._E00 handler
+which executes the ACPI Notify operation.
+
+Although not specified in Microsoft's document, it is assumed that the
+device is expected to use the little-endian system.
-- 
2.7.4

Re: [Qemu-devel] [PATCH v2 1/6] docs: vm generation id device's description

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 2426 bytes --]

On 01/16/2017 01:20 PM, ben@skyportsystems.com wrote:
> From: Ben Warren <ben@skyportsystems.com>

meta-comment: This message was sent with headers:
> Message-Id: <d8737523bcb760c29d8d5ca9bd86800fca9cb3c1.1484594095.git.ben@skyportsystems.com>
> In-Reply-To: <cover.1484593565.git.ben@skyportsystems.com>
> References: <cover.1484593565.git.ben@skyportsystems.com>
> In-Reply-To: <cover.1484594095.git.ben@skyportsystems.com>
> References: <cover.1484594095.git.ben@skyportsystems.com>

while your cover letter's actual headers included:
> Message-Id: <cover.1484593565.git.ben@skyportsystems.com>

I don't know how you got your sending side to use two different sets of
In-Reply-To, but it breaks at least Thunderbird's abilities to rethread
the messages (thunderbird apparently only pays attention to the last
header, while only the first one would cause correct threading).


> 
> Signed-off-by: Ben Warren <ben@skyportsystems.com>
> Cc: Gal Hammer <ghammer@redhat.com>

If you are basing this patch off of earlier work (for example, you
mentioned Igor's work), you may need additional S-o-b lines for work you
copied from the earlier versions.

> ---
>  docs/specs/vmgenid.txt | 38 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 38 insertions(+)
>  create mode 100644 docs/specs/vmgenid.txt
> 
> diff --git a/docs/specs/vmgenid.txt b/docs/specs/vmgenid.txt
> new file mode 100644
> index 0000000..afc1717
> --- /dev/null
> +++ b/docs/specs/vmgenid.txt
> @@ -0,0 +1,38 @@
> +VIRTUAL MACHINE GENERATION ID
> +=============================
> +
> +Copyright (C) 2016 Red Hat, Inc.

Claiming Red Hat copyright makes it sound like you are basing this off
an earlier version.  That is the correct thing to do if you indeed
copied something with Red Hat copyright, but seems weird if you wrote it
from scratch.

> +Copyright (C) 2016 Skyport Systems, Inc.

Do you want to claim 2017 as well?


> +
> +The vmgenid device is a sysbus device with the ACPI ID "QEMU_Gen_Counter_V1".
> +
> +The device has one properties, which can be set using the command line

s/properties/property/

> +argument or the QMP interface:
> + guid - sets the value of the GUID
> +For example:
> +QEMU  -device vmgenid,guid="324e6eaf-d1d1-4bf6-bf41-b9bb6c91fb87"
> +

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH v2 1/6] docs: vm generation id device's description

[Ben Warren]

[-- Attachment #1: Type: text/plain, Size: 3803 bytes --]


> On Jan 16, 2017, at 11:51 AM, Eric Blake <eblake@redhat.com> wrote:
> 
> On 01/16/2017 01:20 PM, ben@skyportsystems.com <mailto:ben@skyportsystems.com> wrote:
>> From: Ben Warren <ben@skyportsystems.com <mailto:ben@skyportsystems.com>>
> 
> meta-comment: This message was sent with headers:
>> Message-Id: <d8737523bcb760c29d8d5ca9bd86800fca9cb3c1.1484594095.git.ben@skyportsystems.com <mailto:d8737523bcb760c29d8d5ca9bd86800fca9cb3c1.1484594095.git.ben@skyportsystems.com>>
>> In-Reply-To: <cover.1484593565.git.ben@skyportsystems.com <mailto:cover.1484593565.git.ben@skyportsystems.com>>
>> References: <cover.1484593565.git.ben@skyportsystems.com <mailto:cover.1484593565.git.ben@skyportsystems.com>>
>> In-Reply-To: <cover.1484594095.git.ben@skyportsystems.com <mailto:cover.1484594095.git.ben@skyportsystems.com>>
>> References: <cover.1484594095.git.ben@skyportsystems.com <mailto:cover.1484594095.git.ben@skyportsystems.com>>
> 
> while your cover letter's actual headers included:
>> Message-Id: <cover.1484593565.git.ben@skyportsystems.com <mailto:cover.1484593565.git.ben@skyportsystems.com>>
> 
> I don't know how you got your sending side to use two different sets of
> In-Reply-To, but it breaks at least Thunderbird's abilities to rethread
> the messages (thunderbird apparently only pays attention to the last
> header, while only the first one would cause correct threading).
> 
Hmm, I created the patches using “git format-patch” with the —thread switch, then ‘git send-email’, which is usually a solid combination.  I’ll dry-run it next time to make sure things are threaded properly.  Sorry.
> 
>> 
>> Signed-off-by: Ben Warren <ben@skyportsystems.com <mailto:ben@skyportsystems.com>>
>> Cc: Gal Hammer <ghammer@redhat.com <mailto:ghammer@redhat.com>>
> 
> If you are basing this patch off of earlier work (for example, you
> mentioned Igor's work), you may need additional S-o-b lines for work you
> copied from the earlier versions.
> 
yes, I used a previous patch set as a baseline and want to make sure credit goes to whoever did the real work.  In this case, Gal Hammer was the original author, so I CC’d Him (or her) hoping to get an SOB.  Should that be handled out-of-band?
>> ---
>> docs/specs/vmgenid.txt | 38 ++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 38 insertions(+)
>> create mode 100644 docs/specs/vmgenid.txt
>> 
>> diff --git a/docs/specs/vmgenid.txt b/docs/specs/vmgenid.txt
>> new file mode 100644
>> index 0000000..afc1717
>> --- /dev/null
>> +++ b/docs/specs/vmgenid.txt
>> @@ -0,0 +1,38 @@
>> +VIRTUAL MACHINE GENERATION ID
>> +=============================
>> +
>> +Copyright (C) 2016 Red Hat, Inc.
> 
> Claiming Red Hat copyright makes it sound like you are basing this off
> an earlier version.  That is the correct thing to do if you indeed
> copied something with Red Hat copyright, but seems weird if you wrote it
> from scratch.
Same as above, I reworked much of this document, but a great deal is not my original.  Guidance on the right way to do this is appreciated.
> 
>> +Copyright (C) 2016 Skyport Systems, Inc.
> 
> Do you want to claim 2017 as well?
> 
Good catch, I haven’t touched the file this year, but that makes sense.
> 
>> +
>> +The vmgenid device is a sysbus device with the ACPI ID "QEMU_Gen_Counter_V1".
>> +
>> +The device has one properties, which can be set using the command line
> 
> s/properties/property/
Will fix.
> 
>> +argument or the QMP interface:
>> + guid - sets the value of the GUID
>> +For example:
>> +QEMU  -device vmgenid,guid="324e6eaf-d1d1-4bf6-bf41-b9bb6c91fb87"
>> +
> 
> -- 
> Eric Blake   eblake redhat com    +1-919-301-3266
> Libvirt virtualization library http://libvirt.org <http://libvirt.org/>

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 3583 bytes --]

Re: [Qemu-devel] [PATCH v2 1/6] docs: vm generation id device's description

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1138 bytes --]

On 01/16/2017 02:08 PM, Ben Warren wrote:

>>>
>>> Signed-off-by: Ben Warren <ben@skyportsystems.com <mailto:ben@skyportsystems.com>>
>>> Cc: Gal Hammer <ghammer@redhat.com <mailto:ghammer@redhat.com>>
>>
>> If you are basing this patch off of earlier work (for example, you
>> mentioned Igor's work), you may need additional S-o-b lines for work you
>> copied from the earlier versions.
>>
> yes, I used a previous patch set as a baseline and want to make sure credit goes to whoever did the real work.  In this case, Gal Hammer was the original author, so I CC’d Him (or her) hoping to get an SOB.  Should that be handled out-of-band?

If Gal already had S-o-b on the version you started with, then the most
common two approaches are:

1) Use Gal's commit message verbatim, including his S-o-b, then add
another paragraph about your changes, with your S-o-b.

2) Rewrite the commit message, but state that you are basing on Gal's
work, then list S-o-b for both content contributors next to one another.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

[Qemu-devel] [PATCH v2 2/6] ACPI: Add a function for building named qword entries

[ben]

From: Ben Warren <ben@skyportsystems.com>

Signed-off-by: Ben Warren <ben@skyportsystems.com>
---
 hw/acpi/aml-build.c         | 28 ++++++++++++++++++++++++++++
 include/hw/acpi/aml-build.h |  4 ++++
 2 files changed, 32 insertions(+)

diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index b2a1e40..dc4edc2 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -285,6 +285,34 @@ build_append_named_dword(GArray *array, const char *name_format, ...)
     return offset;
 }
 
+/*
+ * Build NAME(XXXX, 0x00000000) where 0x00000000 is encoded as a qword,
+ * and return the offset to 0x00000000 for runtime patching.
+ *
+ * Warning: runtime patching is best avoided. Only use this as
+ * a replacement for DataTableRegion (for guests that don't
+ * support it).
+ */
+int
+build_append_named_qword(GArray *array, const char *name_format, ...)
+{
+    int offset;
+    va_list ap;
+
+    build_append_byte(array, 0x08); /* NameOp */
+    va_start(ap, name_format);
+    build_append_namestringv(array, name_format, ap);
+    va_end(ap);
+
+    build_append_byte(array, 0x0E); /* QWordPrefix */
+
+    offset = array->len;
+    build_append_int_noprefix(array, 0x00000000, 8);
+    assert(array->len == offset + 8);
+
+    return offset;
+}
+
 static GPtrArray *alloc_list;
 
 static Aml *aml_alloc(void)
diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
index 559326c..dbf63cf 100644
--- a/include/hw/acpi/aml-build.h
+++ b/include/hw/acpi/aml-build.h
@@ -385,6 +385,10 @@ int
 build_append_named_dword(GArray *array, const char *name_format, ...)
 GCC_FMT_ATTR(2, 3);
 
+int
+build_append_named_qword(GArray *array, const char *name_format, ...)
+GCC_FMT_ATTR(2, 3);
+
 void build_srat_memory(AcpiSratMemoryAffinity *numamem, uint64_t base,
                        uint64_t len, int node, MemoryAffinityFlags flags);
 
-- 
2.7.4

[Qemu-devel] [PATCH v2 3/6] ACPI: Add Virtual Machine Generation ID support

[ben]

From: Ben Warren <ben@skyportsystems.com>

This implements the VM Generation ID feature by passing a 128-bit
GUID to the guest via a fw_cfg blob.
Any time the GUID changes, and ACPI notify event is sent to the guest

The user interface is a simple device with one parameters:
 - guid (string, must be in UUID format
   xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx)

Signed-off-by: Ben Warren <ben@skyportsystems.com>
---
 default-configs/i386-softmmu.mak   |   1 +
 default-configs/x86_64-softmmu.mak |   1 +
 hw/acpi/Makefile.objs              |   1 +
 hw/acpi/vmgenid.c                  | 207 +++++++++++++++++++++++++++++++++++++
 hw/i386/acpi-build.c               |   5 +
 hw/misc/Makefile.objs              |   1 +
 include/hw/acpi/vmgenid.h          |  24 +++++
 7 files changed, 240 insertions(+)
 create mode 100644 hw/acpi/vmgenid.c
 create mode 100644 include/hw/acpi/vmgenid.h

diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak
index 0b51360..b2bccf6 100644
--- a/default-configs/i386-softmmu.mak
+++ b/default-configs/i386-softmmu.mak
@@ -56,3 +56,4 @@ CONFIG_IOH3420=y
 CONFIG_I82801B11=y
 CONFIG_SMBIOS=y
 CONFIG_HYPERV_TESTDEV=$(CONFIG_KVM)
+CONFIG_ACPI_VMGENID=y
diff --git a/default-configs/x86_64-softmmu.mak b/default-configs/x86_64-softmmu.mak
index 7f89503..c6bd310 100644
--- a/default-configs/x86_64-softmmu.mak
+++ b/default-configs/x86_64-softmmu.mak
@@ -56,3 +56,4 @@ CONFIG_IOH3420=y
 CONFIG_I82801B11=y
 CONFIG_SMBIOS=y
 CONFIG_HYPERV_TESTDEV=$(CONFIG_KVM)
+CONFIG_ACPI_VMGENID=y
diff --git a/hw/acpi/Makefile.objs b/hw/acpi/Makefile.objs
index 489e63b..7dc95cd 100644
--- a/hw/acpi/Makefile.objs
+++ b/hw/acpi/Makefile.objs
@@ -4,6 +4,7 @@ common-obj-$(CONFIG_ACPI_CPU_HOTPLUG) += cpu_hotplug.o
 common-obj-$(CONFIG_ACPI_MEMORY_HOTPLUG) += memory_hotplug.o memory_hotplug_acpi_table.o
 common-obj-$(CONFIG_ACPI_CPU_HOTPLUG) += cpu.o
 common-obj-$(CONFIG_ACPI_NVDIMM) += nvdimm.o
+common-obj-$(CONFIG_ACPI_VMGENID) += vmgenid.o
 common-obj-$(CONFIG_ACPI) += acpi_interface.o
 common-obj-$(CONFIG_ACPI) += bios-linker-loader.o
 common-obj-$(CONFIG_ACPI) += aml-build.o
diff --git a/hw/acpi/vmgenid.c b/hw/acpi/vmgenid.c
new file mode 100644
index 0000000..596c8b7
--- /dev/null
+++ b/hw/acpi/vmgenid.c
@@ -0,0 +1,207 @@
+/*
+ *  Virtual Machine Generation ID Device
+ *
+ *  Copyright (C) 2016 Skyport Systems.
+ *
+ *  Authors: Ben Warren <ben@skyportsystems.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 "qemu/osdep.h"
+#include "qmp-commands.h"
+#include "hw/acpi/acpi.h"
+#include "hw/acpi/aml-build.h"
+#include "hw/acpi/vmgenid.h"
+#include "hw/nvram/fw_cfg.h"
+#include "sysemu/sysemu.h"
+
+Object *find_vmgenid_dev(Error **errp)
+{
+    Object *obj = object_resolve_path_type("", VMGENID_DEVICE, NULL);
+    if (!obj) {
+        error_setg(errp, VMGENID_DEVICE " is not found");
+    }
+    return obj;
+}
+
+void vmgenid_build_acpi(GArray *table_offsets, GArray *table_data,
+        BIOSLinker *linker)
+{
+    Object *obj = find_vmgenid_dev(NULL);
+    if (!obj) {
+        return;
+    }
+    VmGenIdState *s = VMGENID(obj);
+
+    acpi_add_table(table_offsets, table_data);
+
+    GArray *guid = g_array_new(false, true, sizeof(s->guid.data));
+    g_array_append_val(guid, s->guid.data);
+
+    Aml *ssdt, *dev, *scope, *pkg, *method;
+
+    /* Put this in a separate SSDT table */
+    ssdt = init_aml_allocator();
+
+    /* Reserve space for header */
+    acpi_data_push(ssdt->buf, sizeof(AcpiTableHeader));
+
+    /* Storage for the GUID address */
+    uint32_t vgia_offset = table_data->len +
+        build_append_named_qword(ssdt->buf, "VGIA");
+    dev = aml_device("VGEN");
+    scope = aml_scope("\\_SB");
+    aml_append(dev, aml_name_decl("_HID", aml_string("QEMUVGID")));
+    aml_append(dev, aml_name_decl("_CID", aml_string("VM_Gen_Counter")));
+    aml_append(dev, aml_name_decl("_DDN", aml_string("VM_Gen_Counter")));
+
+    /* Simple status method to check that address is linked and non-zero */
+    method = aml_method("_STA", 0, AML_NOTSERIALIZED);
+    Aml *if_ctx = aml_if(aml_equal(aml_name("VGIA"), aml_int(0)));
+    aml_append(if_ctx, aml_return(aml_int(0)));
+    aml_append(method, if_ctx);
+    Aml *else_ctx = aml_else();
+    aml_append(else_ctx, aml_return(aml_int(0xf)));
+    aml_append(method, else_ctx);
+    aml_append(dev, method);
+
+    /* the ADDR method returns two 32-bit words representing the lower and
+     * upper halves * of the physical address of the fw_cfg blob
+     * (holding the GUID) */
+    method = aml_method("ADDR", 0, AML_NOTSERIALIZED);
+
+    pkg = aml_package(2);
+    aml_append(pkg, aml_int(0));
+    aml_append(pkg, aml_int(0));
+
+    aml_append(method, aml_name_decl("LPKG", pkg));
+    aml_append(method, aml_store(aml_and(aml_name("VGIA"),
+        aml_int(0xffffffff), NULL), aml_index(aml_name("LPKG"), aml_int(0))));
+    aml_append(method, aml_store(aml_shiftright(aml_name("VGIA"),
+        aml_int(32), NULL), aml_index(aml_name("LPKG"), aml_int(1))));
+    aml_append(method, aml_return(aml_name("LPKG")));
+
+    aml_append(dev, method);
+    aml_append(scope, dev);
+    aml_append(ssdt, scope);
+
+    /* attach an ACPI notify */
+    scope = aml_scope("_GPE");
+    method = aml_method("_E00", 0, AML_NOTSERIALIZED);
+    aml_append(method, aml_notify(aml_name("\\_SB.VGEN"), aml_int(0x80)));
+    aml_append(scope, method);
+    aml_append(ssdt, scope);
+
+    /* copy AML table into ACPI tables blob and patch in fw_cfg blob */
+    g_array_append_vals(table_data, ssdt->buf->data, ssdt->buf->len);
+    bios_linker_loader_alloc(linker, VMGENID_FW_CFG_FILE, guid, 0,
+                             false /* high memory */);
+    bios_linker_loader_add_pointer(linker,
+        ACPI_BUILD_TABLE_FILE, vgia_offset, sizeof(uint64_t),
+        VMGENID_FW_CFG_FILE, 0);
+
+    build_header(linker, table_data,
+        (void *)(table_data->data + table_data->len - ssdt->buf->len),
+        "SSDT", ssdt->buf->len, 1, NULL, "VMGENID");
+    free_aml_allocator();
+}
+
+void vmgenid_add_fw_cfg(FWCfgState *s)
+{
+    Object *obj = find_vmgenid_dev(NULL);
+    if (!obj) {
+        return;
+    }
+    VmGenIdState *vms = VMGENID(obj);
+    fw_cfg_add_file(s, VMGENID_FW_CFG_FILE, vms->guid.data,
+        sizeof(vms->guid.data));
+}
+
+static void vmgenid_notify_guest(VmGenIdState *s)
+{
+    Object *obj = object_resolve_path_type("", TYPE_ACPI_DEVICE_IF, NULL);
+    if (obj) {
+        /* Send _GPE.E00 event */
+        acpi_send_event(DEVICE(obj), 1 << 0);
+    }
+}
+
+static void vmgenid_set_guid(Object *obj, const char *value, Error **errp)
+{
+    VmGenIdState *s = VMGENID(obj);
+
+    if (qemu_uuid_parse(value, &s->guid) < 0) {
+        error_setg(errp, "'%s." VMGENID_GUID
+                   "': Failed to parse GUID string: %s",
+                   object_get_typename(OBJECT(s)),
+                   value);
+        return;
+    }
+    vmgenid_notify_guest(s);
+}
+
+static void vmgenid_state_change(void *priv, int running, RunState state)
+{
+    VmGenIdState *s;
+
+    if (state != RUN_STATE_RUNNING) {
+        return;
+    }
+    s = VMGENID((Object *)priv);
+    vmgenid_notify_guest(s);
+}
+
+static void vmgenid_initfn(Object *obj)
+{
+    object_property_add_str(obj, VMGENID_GUID, NULL, vmgenid_set_guid, NULL);
+    qemu_add_vm_change_state_handler(vmgenid_state_change, obj);
+}
+
+static const TypeInfo vmgenid_device_info = {
+    .name          = VMGENID_DEVICE,
+    .parent        = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(VmGenIdState),
+    .instance_init = vmgenid_initfn,
+};
+
+static void vmgenid_register_types(void)
+{
+    type_register_static(&vmgenid_device_info);
+}
+
+type_init(vmgenid_register_types)
+
+GuidInfo *qmp_query_vm_generation_id(Error **errp)
+{
+    GuidInfo *info;
+    VmGenIdState *vdev;
+    Object *obj = find_vmgenid_dev(errp);
+
+    if (!obj) {
+        return NULL;
+    }
+    vdev = VMGENID(obj);
+    info = g_malloc0(sizeof(*info));
+    info->guid = g_strdup_printf(UUID_FMT, vdev->guid.data[0],
+            vdev->guid.data[1], vdev->guid.data[2], vdev->guid.data[3],
+            vdev->guid.data[4], vdev->guid.data[5], vdev->guid.data[6],
+            vdev->guid.data[7], vdev->guid.data[8], vdev->guid.data[9],
+            vdev->guid.data[10], vdev->guid.data[11], vdev->guid.data[12],
+            vdev->guid.data[13], vdev->guid.data[14], vdev->guid.data[15]);
+    return info;
+}
+
+void qmp_set_vm_generation_id(const char *guid, Error **errp)
+{
+    Object *obj = find_vmgenid_dev(errp);
+
+    if (!obj) {
+        return;
+    }
+
+    object_property_set_str(obj, guid, VMGENID_GUID, errp);
+    return;
+}
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 9708cdc..cde81b7 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -42,6 +42,7 @@
 #include "hw/acpi/memory_hotplug.h"
 #include "sysemu/tpm.h"
 #include "hw/acpi/tpm.h"
+#include "hw/acpi/vmgenid.h"
 #include "sysemu/tpm_backend.h"
 #include "hw/timer/mc146818rtc_regs.h"
 #include "sysemu/numa.h"
@@ -2785,6 +2786,8 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
     acpi_add_table(table_offsets, tables_blob);
     build_madt(tables_blob, tables->linker, pcms);
 
+    vmgenid_build_acpi(table_offsets, tables_blob, tables->linker);
+
     if (misc.has_hpet) {
         acpi_add_table(table_offsets, tables_blob);
         build_hpet(tables_blob, tables->linker);
@@ -2991,6 +2994,8 @@ void acpi_setup(void)
     fw_cfg_add_file(pcms->fw_cfg, ACPI_BUILD_TPMLOG_FILE,
                     tables.tcpalog->data, acpi_data_len(tables.tcpalog));
 
+    vmgenid_add_fw_cfg(pcms->fw_cfg);
+
     if (!pcmc->rsdp_in_ram) {
         /*
          * Keep for compatibility with old machine types.
diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
index 1a89615..ca0f1bb 100644
--- a/hw/misc/Makefile.objs
+++ b/hw/misc/Makefile.objs
@@ -53,3 +53,4 @@ obj-$(CONFIG_EDU) += edu.o
 obj-$(CONFIG_HYPERV_TESTDEV) += hyperv_testdev.o
 obj-$(CONFIG_AUX) += auxbus.o
 obj-$(CONFIG_ASPEED_SOC) += aspeed_scu.o aspeed_sdmc.o
+obj-$(CONFIG_VMGENID) += vmgenid.o
diff --git a/include/hw/acpi/vmgenid.h b/include/hw/acpi/vmgenid.h
new file mode 100644
index 0000000..f8bdff2
--- /dev/null
+++ b/include/hw/acpi/vmgenid.h
@@ -0,0 +1,24 @@
+#ifndef ACPI_VMGENID_H
+#define ACPI_VMGENID_H
+
+#include "hw/acpi/bios-linker-loader.h"
+#include "hw/sysbus.h"
+#include "qemu/uuid.h"
+
+#define VMGENID_DEVICE           "vmgenid"
+#define VMGENID_GUID             "guid"
+#define VMGENID_FW_CFG_FILE      "etc/vmgenid"
+
+Object *find_vmgenid_dev(Error **errp);
+void vmgenid_add_fw_cfg(FWCfgState *s);
+void vmgenid_build_acpi(GArray *table_offsets, GArray *table_data,
+                       BIOSLinker *linker);
+
+#define VMGENID(obj) OBJECT_CHECK(VmGenIdState, (obj), VMGENID_DEVICE)
+
+typedef struct VmGenIdState {
+    SysBusDevice parent_obj;
+    QemuUUID guid;
+} VmGenIdState;
+
+#endif
-- 
2.7.4

Re: [Qemu-devel] [PATCH v2 3/6] ACPI: Add Virtual Machine Generation ID support

[Igor Mammedov]

On Mon, 16 Jan 2017 11:20:55 -0800
ben@skyportsystems.com wrote:

> From: Ben Warren <ben@skyportsystems.com>
> 
> This implements the VM Generation ID feature by passing a 128-bit
> GUID to the guest via a fw_cfg blob.
> Any time the GUID changes, and ACPI notify event is sent to the guest
> 
> The user interface is a simple device with one parameters:
>  - guid (string, must be in UUID format
>    xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx)
> 
> Signed-off-by: Ben Warren <ben@skyportsystems.com>
> ---
>  default-configs/i386-softmmu.mak   |   1 +
>  default-configs/x86_64-softmmu.mak |   1 +
>  hw/acpi/Makefile.objs              |   1 +
>  hw/acpi/vmgenid.c                  | 207 +++++++++++++++++++++++++++++++++++++
>  hw/i386/acpi-build.c               |   5 +
>  hw/misc/Makefile.objs              |   1 +
>  include/hw/acpi/vmgenid.h          |  24 +++++
>  7 files changed, 240 insertions(+)
>  create mode 100644 hw/acpi/vmgenid.c
>  create mode 100644 include/hw/acpi/vmgenid.h
> 
> diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak
> index 0b51360..b2bccf6 100644
> --- a/default-configs/i386-softmmu.mak
> +++ b/default-configs/i386-softmmu.mak
> @@ -56,3 +56,4 @@ CONFIG_IOH3420=y
>  CONFIG_I82801B11=y
>  CONFIG_SMBIOS=y
>  CONFIG_HYPERV_TESTDEV=$(CONFIG_KVM)
> +CONFIG_ACPI_VMGENID=y
> diff --git a/default-configs/x86_64-softmmu.mak b/default-configs/x86_64-softmmu.mak
> index 7f89503..c6bd310 100644
> --- a/default-configs/x86_64-softmmu.mak
> +++ b/default-configs/x86_64-softmmu.mak
> @@ -56,3 +56,4 @@ CONFIG_IOH3420=y
>  CONFIG_I82801B11=y
>  CONFIG_SMBIOS=y
>  CONFIG_HYPERV_TESTDEV=$(CONFIG_KVM)
> +CONFIG_ACPI_VMGENID=y
> diff --git a/hw/acpi/Makefile.objs b/hw/acpi/Makefile.objs
> index 489e63b..7dc95cd 100644
> --- a/hw/acpi/Makefile.objs
> +++ b/hw/acpi/Makefile.objs
> @@ -4,6 +4,7 @@ common-obj-$(CONFIG_ACPI_CPU_HOTPLUG) += cpu_hotplug.o
>  common-obj-$(CONFIG_ACPI_MEMORY_HOTPLUG) += memory_hotplug.o memory_hotplug_acpi_table.o
>  common-obj-$(CONFIG_ACPI_CPU_HOTPLUG) += cpu.o
>  common-obj-$(CONFIG_ACPI_NVDIMM) += nvdimm.o
> +common-obj-$(CONFIG_ACPI_VMGENID) += vmgenid.o
>  common-obj-$(CONFIG_ACPI) += acpi_interface.o
>  common-obj-$(CONFIG_ACPI) += bios-linker-loader.o
>  common-obj-$(CONFIG_ACPI) += aml-build.o
> diff --git a/hw/acpi/vmgenid.c b/hw/acpi/vmgenid.c
> new file mode 100644
> index 0000000..596c8b7
> --- /dev/null
> +++ b/hw/acpi/vmgenid.c
> @@ -0,0 +1,207 @@
> +/*
> + *  Virtual Machine Generation ID Device
> + *
> + *  Copyright (C) 2016 Skyport Systems.
> + *
> + *  Authors: Ben Warren <ben@skyportsystems.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 "qemu/osdep.h"
> +#include "qmp-commands.h"
> +#include "hw/acpi/acpi.h"
> +#include "hw/acpi/aml-build.h"
> +#include "hw/acpi/vmgenid.h"
> +#include "hw/nvram/fw_cfg.h"
> +#include "sysemu/sysemu.h"
> +
> +Object *find_vmgenid_dev(Error **errp)
> +{
> +    Object *obj = object_resolve_path_type("", VMGENID_DEVICE, NULL);
> +    if (!obj) {
> +        error_setg(errp, VMGENID_DEVICE " is not found");
> +    }
> +    return obj;
> +}
> +
> +void vmgenid_build_acpi(GArray *table_offsets, GArray *table_data,
> +        BIOSLinker *linker)
wrong alignment, should be

   f12345(arg1,
          arg2);
  
> +{
> +    Object *obj = find_vmgenid_dev(NULL);
> +    if (!obj) {
> +        return;
> +    }
> +    VmGenIdState *s = VMGENID(obj);
> +
> +    acpi_add_table(table_offsets, table_data);
> +
> +    GArray *guid = g_array_new(false, true, sizeof(s->guid.data));
> +    g_array_append_val(guid, s->guid.data);
> +
> +    Aml *ssdt, *dev, *scope, *pkg, *method;
Pls read CODING_STYLE and amend patch accordingly.

> +
> +    /* Put this in a separate SSDT table */
> +    ssdt = init_aml_allocator();
> +
> +    /* Reserve space for header */
> +    acpi_data_push(ssdt->buf, sizeof(AcpiTableHeader));
> +
> +    /* Storage for the GUID address */
> +    uint32_t vgia_offset = table_data->len +
> +        build_append_named_qword(ssdt->buf, "VGIA");
> +    dev = aml_device("VGEN");
> +    scope = aml_scope("\\_SB");
swap scope and dev

> +    aml_append(dev, aml_name_decl("_HID", aml_string("QEMUVGID")));
> +    aml_append(dev, aml_name_decl("_CID", aml_string("VM_Gen_Counter")));
> +    aml_append(dev, aml_name_decl("_DDN", aml_string("VM_Gen_Counter")));
> +
> +    /* Simple status method to check that address is linked and non-zero */
> +    method = aml_method("_STA", 0, AML_NOTSERIALIZED);
> +    Aml *if_ctx = aml_if(aml_equal(aml_name("VGIA"), aml_int(0)));
> +    aml_append(if_ctx, aml_return(aml_int(0)));
> +    aml_append(method, if_ctx);
> +    Aml *else_ctx = aml_else();
> +    aml_append(else_ctx, aml_return(aml_int(0xf)));
> +    aml_append(method, else_ctx);
> +    aml_append(dev, method);
it would be cleaner if written like:

local0 = 0xf
if (vgia == 0) {
   local0 = 0
}
return local0

> +
> +    /* the ADDR method returns two 32-bit words representing the lower and
> +     * upper halves * of the physical address of the fw_cfg blob
> +     * (holding the GUID) */
> +    method = aml_method("ADDR", 0, AML_NOTSERIALIZED);
> +
> +    pkg = aml_package(2);
> +    aml_append(pkg, aml_int(0));
> +    aml_append(pkg, aml_int(0));
> +
> +    aml_append(method, aml_name_decl("LPKG", pkg));
creating named object in method requires method be serialized, however
there is no need to create named variable here, just use localX instead

like:
   addr = aml_local(0)
   store(pkg, addr)

> +    aml_append(method, aml_store(aml_and(aml_name("VGIA"),
> +        aml_int(0xffffffff), NULL), aml_index(aml_name("LPKG"), aml_int(0))));
then instead of aml_name("LPKG") it would become just 'addr'

> +    aml_append(method, aml_store(aml_shiftright(aml_name("VGIA"),
> +        aml_int(32), NULL), aml_index(aml_name("LPKG"), aml_int(1))));
> +    aml_append(method, aml_return(aml_name("LPKG")));
> +
> +    aml_append(dev, method);
> +    aml_append(scope, dev);
> +    aml_append(ssdt, scope);
> +
> +    /* attach an ACPI notify */
> +    scope = aml_scope("_GPE");
> +    method = aml_method("_E00", 0, AML_NOTSERIALIZED);
I don't recall reason why _E00 isn't used but to be safe maybe next unused
would be better (it seems to be _E05)

> +    aml_append(method, aml_notify(aml_name("\\_SB.VGEN"), aml_int(0x80)));
> +    aml_append(scope, method);
> +    aml_append(ssdt, scope);
you actually can skip scopes by providing full path in device/method name, like

  aml_method("\\_GPE._E05", ...

> +
> +    /* copy AML table into ACPI tables blob and patch in fw_cfg blob */
> +    g_array_append_vals(table_data, ssdt->buf->data, ssdt->buf->len);
> +    bios_linker_loader_alloc(linker, VMGENID_FW_CFG_FILE, guid, 0,
> +                             false /* high memory */);
> +    bios_linker_loader_add_pointer(linker,
> +        ACPI_BUILD_TABLE_FILE, vgia_offset, sizeof(uint64_t),
> +        VMGENID_FW_CFG_FILE, 0);
> +
> +    build_header(linker, table_data,
> +        (void *)(table_data->data + table_data->len - ssdt->buf->len),
> +        "SSDT", ssdt->buf->len, 1, NULL, "VMGENID");
> +    free_aml_allocator();
> +}
> +
> +void vmgenid_add_fw_cfg(FWCfgState *s)
> +{
> +    Object *obj = find_vmgenid_dev(NULL);
> +    if (!obj) {
> +        return;
> +    }
> +    VmGenIdState *vms = VMGENID(obj);
> +    fw_cfg_add_file(s, VMGENID_FW_CFG_FILE, vms->guid.data,
> +        sizeof(vms->guid.data));
> +}
> +
> +static void vmgenid_notify_guest(VmGenIdState *s)
> +{
> +    Object *obj = object_resolve_path_type("", TYPE_ACPI_DEVICE_IF, NULL);
> +    if (obj) {
> +        /* Send _GPE.E00 event */
> +        acpi_send_event(DEVICE(obj), 1 << 0);
> +    }
> +}
> +
> +static void vmgenid_set_guid(Object *obj, const char *value, Error **errp)
> +{
> +    VmGenIdState *s = VMGENID(obj);
> +
> +    if (qemu_uuid_parse(value, &s->guid) < 0) {
> +        error_setg(errp, "'%s." VMGENID_GUID
> +                   "': Failed to parse GUID string: %s",
> +                   object_get_typename(OBJECT(s)),
> +                   value);
> +        return;
> +    }
here should be code that would copy new GIUD to
buffer allocated by firmware and then after that don't forget
call memory_region_set_dirty() on it.

Question is how you'd get address of it and I think that's where
writable fwcfg files come in play.
I'd prefer fwcfg_loader in firmware to write it back after buffer
allocation is done as it's already has utilities for fw_cfg,
but that probably would mean extending loader protocol to support
new writeback command.

PS, address one would get from guest should be migrated as well


> +    vmgenid_notify_guest(s);
> +}
> +
> +static void vmgenid_state_change(void *priv, int running, RunState state)
> +{
> +    VmGenIdState *s;
> +
> +    if (state != RUN_STATE_RUNNING) {
> +        return;
> +    }
> +    s = VMGENID((Object *)priv);
> +    vmgenid_notify_guest(s);
> +}
> +
> +static void vmgenid_initfn(Object *obj)
> +{
> +    object_property_add_str(obj, VMGENID_GUID, NULL, vmgenid_set_guid, NULL);
> +    qemu_add_vm_change_state_handler(vmgenid_state_change, obj);
> +}
> +
> +static const TypeInfo vmgenid_device_info = {
> +    .name          = VMGENID_DEVICE,
> +    .parent        = TYPE_SYS_BUS_DEVICE,
> +    .instance_size = sizeof(VmGenIdState),
> +    .instance_init = vmgenid_initfn,
> +};
> +
> +static void vmgenid_register_types(void)
> +{
> +    type_register_static(&vmgenid_device_info);
> +}
> +
> +type_init(vmgenid_register_types)
> +
> +GuidInfo *qmp_query_vm_generation_id(Error **errp)
> +{
> +    GuidInfo *info;
> +    VmGenIdState *vdev;
> +    Object *obj = find_vmgenid_dev(errp);
> +
> +    if (!obj) {
> +        return NULL;
> +    }
> +    vdev = VMGENID(obj);
> +    info = g_malloc0(sizeof(*info));
> +    info->guid = g_strdup_printf(UUID_FMT, vdev->guid.data[0],
> +            vdev->guid.data[1], vdev->guid.data[2], vdev->guid.data[3],
> +            vdev->guid.data[4], vdev->guid.data[5], vdev->guid.data[6],
> +            vdev->guid.data[7], vdev->guid.data[8], vdev->guid.data[9],
> +            vdev->guid.data[10], vdev->guid.data[11], vdev->guid.data[12],
> +            vdev->guid.data[13], vdev->guid.data[14], vdev->guid.data[15]);

perhaps qemu_uuid_unparse_strdup() could be used

> +    return info;
> +}
> +
> +void qmp_set_vm_generation_id(const char *guid, Error **errp)
> +{
> +    Object *obj = find_vmgenid_dev(errp);
> +
> +    if (!obj) {
> +        return;
> +    }
> +
> +    object_property_set_str(obj, guid, VMGENID_GUID, errp);
> +    return;
> +}
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 9708cdc..cde81b7 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -42,6 +42,7 @@
>  #include "hw/acpi/memory_hotplug.h"
>  #include "sysemu/tpm.h"
>  #include "hw/acpi/tpm.h"
> +#include "hw/acpi/vmgenid.h"
>  #include "sysemu/tpm_backend.h"
>  #include "hw/timer/mc146818rtc_regs.h"
>  #include "sysemu/numa.h"
> @@ -2785,6 +2786,8 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
>      acpi_add_table(table_offsets, tables_blob);
>      build_madt(tables_blob, tables->linker, pcms);
>  
if (has_vmgenid()) {
> +    vmgenid_build_acpi(table_offsets, tables_blob, tables->linker);
}

and remove similar check for called function

> +
>      if (misc.has_hpet) {
>          acpi_add_table(table_offsets, tables_blob);
>          build_hpet(tables_blob, tables->linker);
> @@ -2991,6 +2994,8 @@ void acpi_setup(void)
>      fw_cfg_add_file(pcms->fw_cfg, ACPI_BUILD_TPMLOG_FILE,
>                      tables.tcpalog->data, acpi_data_len(tables.tcpalog));
>  
> +    vmgenid_add_fw_cfg(pcms->fw_cfg);
ditto

> +
>      if (!pcmc->rsdp_in_ram) {
>          /*
>           * Keep for compatibility with old machine types.
> diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
> index 1a89615..ca0f1bb 100644
> --- a/hw/misc/Makefile.objs
> +++ b/hw/misc/Makefile.objs
> @@ -53,3 +53,4 @@ obj-$(CONFIG_EDU) += edu.o
>  obj-$(CONFIG_HYPERV_TESTDEV) += hyperv_testdev.o
>  obj-$(CONFIG_AUX) += auxbus.o
>  obj-$(CONFIG_ASPEED_SOC) += aspeed_scu.o aspeed_sdmc.o
> +obj-$(CONFIG_VMGENID) += vmgenid.o
> diff --git a/include/hw/acpi/vmgenid.h b/include/hw/acpi/vmgenid.h
> new file mode 100644
> index 0000000..f8bdff2
> --- /dev/null
> +++ b/include/hw/acpi/vmgenid.h
> @@ -0,0 +1,24 @@
> +#ifndef ACPI_VMGENID_H
> +#define ACPI_VMGENID_H
> +
> +#include "hw/acpi/bios-linker-loader.h"
> +#include "hw/sysbus.h"
> +#include "qemu/uuid.h"
> +
> +#define VMGENID_DEVICE           "vmgenid"
> +#define VMGENID_GUID             "guid"
> +#define VMGENID_FW_CFG_FILE      "etc/vmgenid"
> +
> +Object *find_vmgenid_dev(Error **errp);
> +void vmgenid_add_fw_cfg(FWCfgState *s);
> +void vmgenid_build_acpi(GArray *table_offsets, GArray *table_data,
> +                       BIOSLinker *linker);
> +
> +#define VMGENID(obj) OBJECT_CHECK(VmGenIdState, (obj), VMGENID_DEVICE)
> +
> +typedef struct VmGenIdState {
> +    SysBusDevice parent_obj;
> +    QemuUUID guid;
> +} VmGenIdState;
> +
> +#endif

Re: [Qemu-devel] [PATCH v2 3/6] ACPI: Add Virtual Machine Generation ID support

[Laszlo Ersek]

On 01/17/17 14:00, Igor Mammedov wrote:
> On Mon, 16 Jan 2017 11:20:55 -0800
> ben@skyportsystems.com wrote:
> 
>> From: Ben Warren <ben@skyportsystems.com>
>>
>> This implements the VM Generation ID feature by passing a 128-bit
>> GUID to the guest via a fw_cfg blob.
>> Any time the GUID changes, and ACPI notify event is sent to the guest
>>
>> The user interface is a simple device with one parameters:
>>  - guid (string, must be in UUID format
>>    xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx)
>>
>> Signed-off-by: Ben Warren <ben@skyportsystems.com>
>> ---
>>  default-configs/i386-softmmu.mak   |   1 +
>>  default-configs/x86_64-softmmu.mak |   1 +
>>  hw/acpi/Makefile.objs              |   1 +
>>  hw/acpi/vmgenid.c                  | 207 +++++++++++++++++++++++++++++++++++++
>>  hw/i386/acpi-build.c               |   5 +
>>  hw/misc/Makefile.objs              |   1 +
>>  include/hw/acpi/vmgenid.h          |  24 +++++
>>  7 files changed, 240 insertions(+)
>>  create mode 100644 hw/acpi/vmgenid.c
>>  create mode 100644 include/hw/acpi/vmgenid.h
>>
>> diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak
>> index 0b51360..b2bccf6 100644
>> --- a/default-configs/i386-softmmu.mak
>> +++ b/default-configs/i386-softmmu.mak
>> @@ -56,3 +56,4 @@ CONFIG_IOH3420=y
>>  CONFIG_I82801B11=y
>>  CONFIG_SMBIOS=y
>>  CONFIG_HYPERV_TESTDEV=$(CONFIG_KVM)
>> +CONFIG_ACPI_VMGENID=y
>> diff --git a/default-configs/x86_64-softmmu.mak b/default-configs/x86_64-softmmu.mak
>> index 7f89503..c6bd310 100644
>> --- a/default-configs/x86_64-softmmu.mak
>> +++ b/default-configs/x86_64-softmmu.mak
>> @@ -56,3 +56,4 @@ CONFIG_IOH3420=y
>>  CONFIG_I82801B11=y
>>  CONFIG_SMBIOS=y
>>  CONFIG_HYPERV_TESTDEV=$(CONFIG_KVM)
>> +CONFIG_ACPI_VMGENID=y
>> diff --git a/hw/acpi/Makefile.objs b/hw/acpi/Makefile.objs
>> index 489e63b..7dc95cd 100644
>> --- a/hw/acpi/Makefile.objs
>> +++ b/hw/acpi/Makefile.objs
>> @@ -4,6 +4,7 @@ common-obj-$(CONFIG_ACPI_CPU_HOTPLUG) += cpu_hotplug.o
>>  common-obj-$(CONFIG_ACPI_MEMORY_HOTPLUG) += memory_hotplug.o memory_hotplug_acpi_table.o
>>  common-obj-$(CONFIG_ACPI_CPU_HOTPLUG) += cpu.o
>>  common-obj-$(CONFIG_ACPI_NVDIMM) += nvdimm.o
>> +common-obj-$(CONFIG_ACPI_VMGENID) += vmgenid.o
>>  common-obj-$(CONFIG_ACPI) += acpi_interface.o
>>  common-obj-$(CONFIG_ACPI) += bios-linker-loader.o
>>  common-obj-$(CONFIG_ACPI) += aml-build.o
>> diff --git a/hw/acpi/vmgenid.c b/hw/acpi/vmgenid.c
>> new file mode 100644
>> index 0000000..596c8b7
>> --- /dev/null
>> +++ b/hw/acpi/vmgenid.c
>> @@ -0,0 +1,207 @@
>> +/*
>> + *  Virtual Machine Generation ID Device
>> + *
>> + *  Copyright (C) 2016 Skyport Systems.
>> + *
>> + *  Authors: Ben Warren <ben@skyportsystems.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 "qemu/osdep.h"
>> +#include "qmp-commands.h"
>> +#include "hw/acpi/acpi.h"
>> +#include "hw/acpi/aml-build.h"
>> +#include "hw/acpi/vmgenid.h"
>> +#include "hw/nvram/fw_cfg.h"
>> +#include "sysemu/sysemu.h"
>> +
>> +Object *find_vmgenid_dev(Error **errp)
>> +{
>> +    Object *obj = object_resolve_path_type("", VMGENID_DEVICE, NULL);
>> +    if (!obj) {
>> +        error_setg(errp, VMGENID_DEVICE " is not found");
>> +    }
>> +    return obj;
>> +}
>> +
>> +void vmgenid_build_acpi(GArray *table_offsets, GArray *table_data,
>> +        BIOSLinker *linker)
> wrong alignment, should be
> 
>    f12345(arg1,
>           arg2);
>   
>> +{
>> +    Object *obj = find_vmgenid_dev(NULL);
>> +    if (!obj) {
>> +        return;
>> +    }
>> +    VmGenIdState *s = VMGENID(obj);
>> +
>> +    acpi_add_table(table_offsets, table_data);
>> +
>> +    GArray *guid = g_array_new(false, true, sizeof(s->guid.data));
>> +    g_array_append_val(guid, s->guid.data);
>> +
>> +    Aml *ssdt, *dev, *scope, *pkg, *method;
> Pls read CODING_STYLE and amend patch accordingly.
> 
>> +
>> +    /* Put this in a separate SSDT table */
>> +    ssdt = init_aml_allocator();
>> +
>> +    /* Reserve space for header */
>> +    acpi_data_push(ssdt->buf, sizeof(AcpiTableHeader));
>> +
>> +    /* Storage for the GUID address */
>> +    uint32_t vgia_offset = table_data->len +
>> +        build_append_named_qword(ssdt->buf, "VGIA");
>> +    dev = aml_device("VGEN");
>> +    scope = aml_scope("\\_SB");
> swap scope and dev
> 
>> +    aml_append(dev, aml_name_decl("_HID", aml_string("QEMUVGID")));
>> +    aml_append(dev, aml_name_decl("_CID", aml_string("VM_Gen_Counter")));
>> +    aml_append(dev, aml_name_decl("_DDN", aml_string("VM_Gen_Counter")));
>> +
>> +    /* Simple status method to check that address is linked and non-zero */
>> +    method = aml_method("_STA", 0, AML_NOTSERIALIZED);
>> +    Aml *if_ctx = aml_if(aml_equal(aml_name("VGIA"), aml_int(0)));
>> +    aml_append(if_ctx, aml_return(aml_int(0)));
>> +    aml_append(method, if_ctx);
>> +    Aml *else_ctx = aml_else();
>> +    aml_append(else_ctx, aml_return(aml_int(0xf)));
>> +    aml_append(method, else_ctx);
>> +    aml_append(dev, method);
> it would be cleaner if written like:
> 
> local0 = 0xf
> if (vgia == 0) {
>    local0 = 0
> }
> return local0
> 
>> +
>> +    /* the ADDR method returns two 32-bit words representing the lower and
>> +     * upper halves * of the physical address of the fw_cfg blob
>> +     * (holding the GUID) */
>> +    method = aml_method("ADDR", 0, AML_NOTSERIALIZED);
>> +
>> +    pkg = aml_package(2);
>> +    aml_append(pkg, aml_int(0));
>> +    aml_append(pkg, aml_int(0));
>> +
>> +    aml_append(method, aml_name_decl("LPKG", pkg));
> creating named object in method requires method be serialized, however
> there is no need to create named variable here, just use localX instead
> 
> like:
>    addr = aml_local(0)
>    store(pkg, addr)
> 
>> +    aml_append(method, aml_store(aml_and(aml_name("VGIA"),
>> +        aml_int(0xffffffff), NULL), aml_index(aml_name("LPKG"), aml_int(0))));
> then instead of aml_name("LPKG") it would become just 'addr'
> 
>> +    aml_append(method, aml_store(aml_shiftright(aml_name("VGIA"),
>> +        aml_int(32), NULL), aml_index(aml_name("LPKG"), aml_int(1))));
>> +    aml_append(method, aml_return(aml_name("LPKG")));
>> +
>> +    aml_append(dev, method);
>> +    aml_append(scope, dev);
>> +    aml_append(ssdt, scope);
>> +
>> +    /* attach an ACPI notify */
>> +    scope = aml_scope("_GPE");
>> +    method = aml_method("_E00", 0, AML_NOTSERIALIZED);
> I don't recall reason why _E00 isn't used but to be safe maybe next unused
> would be better (it seems to be _E05)

I searched my mailbox, and found this earlier discussion:

https://www.mail-archive.com/qemu-devel@nongnu.org/msg347337.html

If I understand correctly, we shouldn't expose _E00 and _L00 at the same
time.

> 
>> +    aml_append(method, aml_notify(aml_name("\\_SB.VGEN"), aml_int(0x80)));
>> +    aml_append(scope, method);
>> +    aml_append(ssdt, scope);
> you actually can skip scopes by providing full path in device/method name, like
> 
>   aml_method("\\_GPE._E05", ...
> 
>> +
>> +    /* copy AML table into ACPI tables blob and patch in fw_cfg blob */
>> +    g_array_append_vals(table_data, ssdt->buf->data, ssdt->buf->len);
>> +    bios_linker_loader_alloc(linker, VMGENID_FW_CFG_FILE, guid, 0,
>> +                             false /* high memory */);
>> +    bios_linker_loader_add_pointer(linker,
>> +        ACPI_BUILD_TABLE_FILE, vgia_offset, sizeof(uint64_t),
>> +        VMGENID_FW_CFG_FILE, 0);
>> +
>> +    build_header(linker, table_data,
>> +        (void *)(table_data->data + table_data->len - ssdt->buf->len),
>> +        "SSDT", ssdt->buf->len, 1, NULL, "VMGENID");
>> +    free_aml_allocator();
>> +}
>> +
>> +void vmgenid_add_fw_cfg(FWCfgState *s)
>> +{
>> +    Object *obj = find_vmgenid_dev(NULL);
>> +    if (!obj) {
>> +        return;
>> +    }
>> +    VmGenIdState *vms = VMGENID(obj);
>> +    fw_cfg_add_file(s, VMGENID_FW_CFG_FILE, vms->guid.data,
>> +        sizeof(vms->guid.data));
>> +}
>> +
>> +static void vmgenid_notify_guest(VmGenIdState *s)
>> +{
>> +    Object *obj = object_resolve_path_type("", TYPE_ACPI_DEVICE_IF, NULL);
>> +    if (obj) {
>> +        /* Send _GPE.E00 event */
>> +        acpi_send_event(DEVICE(obj), 1 << 0);
>> +    }
>> +}
>> +
>> +static void vmgenid_set_guid(Object *obj, const char *value, Error **errp)
>> +{
>> +    VmGenIdState *s = VMGENID(obj);
>> +
>> +    if (qemu_uuid_parse(value, &s->guid) < 0) {
>> +        error_setg(errp, "'%s." VMGENID_GUID
>> +                   "': Failed to parse GUID string: %s",
>> +                   object_get_typename(OBJECT(s)),
>> +                   value);
>> +        return;
>> +    }
> here should be code that would copy new GIUD to
> buffer allocated by firmware and then after that don't forget
> call memory_region_set_dirty() on it.
> 
> Question is how you'd get address of it and I think that's where
> writable fwcfg files come in play.

I think so, yes.

> I'd prefer fwcfg_loader in firmware to write it back after buffer
> allocation is done as it's already has utilities for fw_cfg,
> but that probably would mean extending loader protocol to support
> new writeback command.

I think it's technically possible, yes.

Thanks
Laszlo

> 
> PS, address one would get from guest should be migrated as well
> 
> 
>> +    vmgenid_notify_guest(s);
>> +}
>> +
>> +static void vmgenid_state_change(void *priv, int running, RunState state)
>> +{
>> +    VmGenIdState *s;
>> +
>> +    if (state != RUN_STATE_RUNNING) {
>> +        return;
>> +    }
>> +    s = VMGENID((Object *)priv);
>> +    vmgenid_notify_guest(s);
>> +}
>> +
>> +static void vmgenid_initfn(Object *obj)
>> +{
>> +    object_property_add_str(obj, VMGENID_GUID, NULL, vmgenid_set_guid, NULL);
>> +    qemu_add_vm_change_state_handler(vmgenid_state_change, obj);
>> +}
>> +
>> +static const TypeInfo vmgenid_device_info = {
>> +    .name          = VMGENID_DEVICE,
>> +    .parent        = TYPE_SYS_BUS_DEVICE,
>> +    .instance_size = sizeof(VmGenIdState),
>> +    .instance_init = vmgenid_initfn,
>> +};
>> +
>> +static void vmgenid_register_types(void)
>> +{
>> +    type_register_static(&vmgenid_device_info);
>> +}
>> +
>> +type_init(vmgenid_register_types)
>> +
>> +GuidInfo *qmp_query_vm_generation_id(Error **errp)
>> +{
>> +    GuidInfo *info;
>> +    VmGenIdState *vdev;
>> +    Object *obj = find_vmgenid_dev(errp);
>> +
>> +    if (!obj) {
>> +        return NULL;
>> +    }
>> +    vdev = VMGENID(obj);
>> +    info = g_malloc0(sizeof(*info));
>> +    info->guid = g_strdup_printf(UUID_FMT, vdev->guid.data[0],
>> +            vdev->guid.data[1], vdev->guid.data[2], vdev->guid.data[3],
>> +            vdev->guid.data[4], vdev->guid.data[5], vdev->guid.data[6],
>> +            vdev->guid.data[7], vdev->guid.data[8], vdev->guid.data[9],
>> +            vdev->guid.data[10], vdev->guid.data[11], vdev->guid.data[12],
>> +            vdev->guid.data[13], vdev->guid.data[14], vdev->guid.data[15]);
> 
> perhaps qemu_uuid_unparse_strdup() could be used
> 
>> +    return info;
>> +}
>> +
>> +void qmp_set_vm_generation_id(const char *guid, Error **errp)
>> +{
>> +    Object *obj = find_vmgenid_dev(errp);
>> +
>> +    if (!obj) {
>> +        return;
>> +    }
>> +
>> +    object_property_set_str(obj, guid, VMGENID_GUID, errp);
>> +    return;
>> +}
>> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
>> index 9708cdc..cde81b7 100644
>> --- a/hw/i386/acpi-build.c
>> +++ b/hw/i386/acpi-build.c
>> @@ -42,6 +42,7 @@
>>  #include "hw/acpi/memory_hotplug.h"
>>  #include "sysemu/tpm.h"
>>  #include "hw/acpi/tpm.h"
>> +#include "hw/acpi/vmgenid.h"
>>  #include "sysemu/tpm_backend.h"
>>  #include "hw/timer/mc146818rtc_regs.h"
>>  #include "sysemu/numa.h"
>> @@ -2785,6 +2786,8 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
>>      acpi_add_table(table_offsets, tables_blob);
>>      build_madt(tables_blob, tables->linker, pcms);
>>  
> if (has_vmgenid()) {
>> +    vmgenid_build_acpi(table_offsets, tables_blob, tables->linker);
> }
> 
> and remove similar check for called function
> 
>> +
>>      if (misc.has_hpet) {
>>          acpi_add_table(table_offsets, tables_blob);
>>          build_hpet(tables_blob, tables->linker);
>> @@ -2991,6 +2994,8 @@ void acpi_setup(void)
>>      fw_cfg_add_file(pcms->fw_cfg, ACPI_BUILD_TPMLOG_FILE,
>>                      tables.tcpalog->data, acpi_data_len(tables.tcpalog));
>>  
>> +    vmgenid_add_fw_cfg(pcms->fw_cfg);
> ditto
> 
>> +
>>      if (!pcmc->rsdp_in_ram) {
>>          /*
>>           * Keep for compatibility with old machine types.
>> diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
>> index 1a89615..ca0f1bb 100644
>> --- a/hw/misc/Makefile.objs
>> +++ b/hw/misc/Makefile.objs
>> @@ -53,3 +53,4 @@ obj-$(CONFIG_EDU) += edu.o
>>  obj-$(CONFIG_HYPERV_TESTDEV) += hyperv_testdev.o
>>  obj-$(CONFIG_AUX) += auxbus.o
>>  obj-$(CONFIG_ASPEED_SOC) += aspeed_scu.o aspeed_sdmc.o
>> +obj-$(CONFIG_VMGENID) += vmgenid.o
>> diff --git a/include/hw/acpi/vmgenid.h b/include/hw/acpi/vmgenid.h
>> new file mode 100644
>> index 0000000..f8bdff2
>> --- /dev/null
>> +++ b/include/hw/acpi/vmgenid.h
>> @@ -0,0 +1,24 @@
>> +#ifndef ACPI_VMGENID_H
>> +#define ACPI_VMGENID_H
>> +
>> +#include "hw/acpi/bios-linker-loader.h"
>> +#include "hw/sysbus.h"
>> +#include "qemu/uuid.h"
>> +
>> +#define VMGENID_DEVICE           "vmgenid"
>> +#define VMGENID_GUID             "guid"
>> +#define VMGENID_FW_CFG_FILE      "etc/vmgenid"
>> +
>> +Object *find_vmgenid_dev(Error **errp);
>> +void vmgenid_add_fw_cfg(FWCfgState *s);
>> +void vmgenid_build_acpi(GArray *table_offsets, GArray *table_data,
>> +                       BIOSLinker *linker);
>> +
>> +#define VMGENID(obj) OBJECT_CHECK(VmGenIdState, (obj), VMGENID_DEVICE)
>> +
>> +typedef struct VmGenIdState {
>> +    SysBusDevice parent_obj;
>> +    QemuUUID guid;
>> +} VmGenIdState;
>> +
>> +#endif
> 

Re: [Qemu-devel] [PATCH v2 3/6] ACPI: Add Virtual Machine Generation ID support

[Michael S. Tsirkin]

On Tue, Jan 17, 2017 at 02:00:58PM +0100, Igor Mammedov wrote:
> Question is how you'd get address of it and I think that's where
> writable fwcfg files come in play.
> I'd prefer fwcfg_loader in firmware to write it back after buffer
> allocation is done as it's already has utilities for fw_cfg,
> but that probably would mean extending loader protocol to support
> new writeback command.

Right. It's quite easy to do as at least seabios was written to
ignore commands it does not recognize. So with old bios you
just don't get a write. Not sure about UEFI, worth checking.

> PS, address one would get from guest should be migrated as well

IIRC writeable fw cfg blobs are migrated automatically.

-- 
MST

Re: [Qemu-devel] [PATCH v2 3/6] ACPI: Add Virtual Machine Generation ID support

[Laszlo Ersek]

On 01/17/17 15:41, Michael S. Tsirkin wrote:
> On Tue, Jan 17, 2017 at 02:00:58PM +0100, Igor Mammedov wrote:
>> Question is how you'd get address of it and I think that's where
>> writable fwcfg files come in play.
>> I'd prefer fwcfg_loader in firmware to write it back after buffer
>> allocation is done as it's already has utilities for fw_cfg,
>> but that probably would mean extending loader protocol to support
>> new writeback command.
> 
> Right. It's quite easy to do as at least seabios was written to
> ignore commands it does not recognize. So with old bios you
> just don't get a write. Not sure about UEFI, worth checking.

Same; OVMF skips (and logs) unknown commands.

> 
>> PS, address one would get from guest should be migrated as well
> 
> IIRC writeable fw cfg blobs are migrated automatically.
> 

When added with rom_add_blob() or derivative functions / function-like
macros, they are.

Thanks
Laszlo

Re: [Qemu-devel] [PATCH v2 3/6] ACPI: Add Virtual Machine Generation ID support

[Ben Warren]

[-- Attachment #1: Type: text/plain, Size: 14377 bytes --]

Hi Igor,
> On Jan 17, 2017, at 5:00 AM, Igor Mammedov <imammedo@redhat.com> wrote:
> 
> On Mon, 16 Jan 2017 11:20:55 -0800
> ben@skyportsystems.com wrote:
> 
>> From: Ben Warren <ben@skyportsystems.com>
>> 
>> This implements the VM Generation ID feature by passing a 128-bit
>> GUID to the guest via a fw_cfg blob.
>> Any time the GUID changes, and ACPI notify event is sent to the guest
>> 
>> The user interface is a simple device with one parameters:
>> - guid (string, must be in UUID format
>>   xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx)
>> 
>> Signed-off-by: Ben Warren <ben@skyportsystems.com>
>> ---
>> default-configs/i386-softmmu.mak   |   1 +
>> default-configs/x86_64-softmmu.mak |   1 +
>> hw/acpi/Makefile.objs              |   1 +
>> hw/acpi/vmgenid.c                  | 207 +++++++++++++++++++++++++++++++++++++
>> hw/i386/acpi-build.c               |   5 +
>> hw/misc/Makefile.objs              |   1 +
>> include/hw/acpi/vmgenid.h          |  24 +++++
>> 7 files changed, 240 insertions(+)
>> create mode 100644 hw/acpi/vmgenid.c
>> create mode 100644 include/hw/acpi/vmgenid.h
>> 
>> diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak
>> index 0b51360..b2bccf6 100644
>> --- a/default-configs/i386-softmmu.mak
>> +++ b/default-configs/i386-softmmu.mak
>> @@ -56,3 +56,4 @@ CONFIG_IOH3420=y
>> CONFIG_I82801B11=y
>> CONFIG_SMBIOS=y
>> CONFIG_HYPERV_TESTDEV=$(CONFIG_KVM)
>> +CONFIG_ACPI_VMGENID=y
>> diff --git a/default-configs/x86_64-softmmu.mak b/default-configs/x86_64-softmmu.mak
>> index 7f89503..c6bd310 100644
>> --- a/default-configs/x86_64-softmmu.mak
>> +++ b/default-configs/x86_64-softmmu.mak
>> @@ -56,3 +56,4 @@ CONFIG_IOH3420=y
>> CONFIG_I82801B11=y
>> CONFIG_SMBIOS=y
>> CONFIG_HYPERV_TESTDEV=$(CONFIG_KVM)
>> +CONFIG_ACPI_VMGENID=y
>> diff --git a/hw/acpi/Makefile.objs b/hw/acpi/Makefile.objs
>> index 489e63b..7dc95cd 100644
>> --- a/hw/acpi/Makefile.objs
>> +++ b/hw/acpi/Makefile.objs
>> @@ -4,6 +4,7 @@ common-obj-$(CONFIG_ACPI_CPU_HOTPLUG) += cpu_hotplug.o
>> common-obj-$(CONFIG_ACPI_MEMORY_HOTPLUG) += memory_hotplug.o memory_hotplug_acpi_table.o
>> common-obj-$(CONFIG_ACPI_CPU_HOTPLUG) += cpu.o
>> common-obj-$(CONFIG_ACPI_NVDIMM) += nvdimm.o
>> +common-obj-$(CONFIG_ACPI_VMGENID) += vmgenid.o
>> common-obj-$(CONFIG_ACPI) += acpi_interface.o
>> common-obj-$(CONFIG_ACPI) += bios-linker-loader.o
>> common-obj-$(CONFIG_ACPI) += aml-build.o
>> diff --git a/hw/acpi/vmgenid.c b/hw/acpi/vmgenid.c
>> new file mode 100644
>> index 0000000..596c8b7
>> --- /dev/null
>> +++ b/hw/acpi/vmgenid.c
>> @@ -0,0 +1,207 @@
>> +/*
>> + *  Virtual Machine Generation ID Device
>> + *
>> + *  Copyright (C) 2016 Skyport Systems.
>> + *
>> + *  Authors: Ben Warren <ben@skyportsystems.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 "qemu/osdep.h"
>> +#include "qmp-commands.h"
>> +#include "hw/acpi/acpi.h"
>> +#include "hw/acpi/aml-build.h"
>> +#include "hw/acpi/vmgenid.h"
>> +#include "hw/nvram/fw_cfg.h"
>> +#include "sysemu/sysemu.h"
>> +
>> +Object *find_vmgenid_dev(Error **errp)
>> +{
>> +    Object *obj = object_resolve_path_type("", VMGENID_DEVICE, NULL);
>> +    if (!obj) {
>> +        error_setg(errp, VMGENID_DEVICE " is not found");
>> +    }
>> +    return obj;
>> +}
>> +
>> +void vmgenid_build_acpi(GArray *table_offsets, GArray *table_data,
>> +        BIOSLinker *linker)
> wrong alignment, should be
> 
>   f12345(arg1,
>          arg2);
> 
OK
>> +{
>> +    Object *obj = find_vmgenid_dev(NULL);
>> +    if (!obj) {
>> +        return;
>> +    }
>> +    VmGenIdState *s = VMGENID(obj);
>> +
>> +    acpi_add_table(table_offsets, table_data);
>> +
>> +    GArray *guid = g_array_new(false, true, sizeof(s->guid.data));
>> +    g_array_append_val(guid, s->guid.data);
>> +
>> +    Aml *ssdt, *dev, *scope, *pkg, *method;
> Pls read CODING_STYLE and amend patch accordingly.
> 
I assume you’re concerned about mixing variable declarations and code.  I’ll fix.
>> +
>> +    /* Put this in a separate SSDT table */
>> +    ssdt = init_aml_allocator();
>> +
>> +    /* Reserve space for header */
>> +    acpi_data_push(ssdt->buf, sizeof(AcpiTableHeader));
>> +
>> +    /* Storage for the GUID address */
>> +    uint32_t vgia_offset = table_data->len +
>> +        build_append_named_qword(ssdt->buf, "VGIA");
>> +    dev = aml_device("VGEN");
>> +    scope = aml_scope("\\_SB");
> swap scope and dev
> 
OK
>> +    aml_append(dev, aml_name_decl("_HID", aml_string("QEMUVGID")));
>> +    aml_append(dev, aml_name_decl("_CID", aml_string("VM_Gen_Counter")));
>> +    aml_append(dev, aml_name_decl("_DDN", aml_string("VM_Gen_Counter")));
>> +
>> +    /* Simple status method to check that address is linked and non-zero */
>> +    method = aml_method("_STA", 0, AML_NOTSERIALIZED);
>> +    Aml *if_ctx = aml_if(aml_equal(aml_name("VGIA"), aml_int(0)));
>> +    aml_append(if_ctx, aml_return(aml_int(0)));
>> +    aml_append(method, if_ctx);
>> +    Aml *else_ctx = aml_else();
>> +    aml_append(else_ctx, aml_return(aml_int(0xf)));
>> +    aml_append(method, else_ctx);
>> +    aml_append(dev, method);
> it would be cleaner if written like:
> 
> local0 = 0xf
> if (vgia == 0) {
>   local0 = 0
> }
> return local0
> 
OK
>> +
>> +    /* the ADDR method returns two 32-bit words representing the lower and
>> +     * upper halves * of the physical address of the fw_cfg blob
>> +     * (holding the GUID) */
>> +    method = aml_method("ADDR", 0, AML_NOTSERIALIZED);
>> +
>> +    pkg = aml_package(2);
>> +    aml_append(pkg, aml_int(0));
>> +    aml_append(pkg, aml_int(0));
>> +
>> +    aml_append(method, aml_name_decl("LPKG", pkg));
> creating named object in method requires method be serialized, however
> there is no need to create named variable here, just use localX instead
> 
> like:
>   addr = aml_local(0)
>   store(pkg, addr)
> 
>> +    aml_append(method, aml_store(aml_and(aml_name("VGIA"),
>> +        aml_int(0xffffffff), NULL), aml_index(aml_name("LPKG"), aml_int(0))));
> then instead of aml_name("LPKG") it would become just ‘addr'
> 
Great idea!
>> +    aml_append(method, aml_store(aml_shiftright(aml_name("VGIA"),
>> +        aml_int(32), NULL), aml_index(aml_name("LPKG"), aml_int(1))));
>> +    aml_append(method, aml_return(aml_name("LPKG")));
>> +
>> +    aml_append(dev, method);
>> +    aml_append(scope, dev);
>> +    aml_append(ssdt, scope);
>> +
>> +    /* attach an ACPI notify */
>> +    scope = aml_scope("_GPE");
>> +    method = aml_method("_E00", 0, AML_NOTSERIALIZED);
> I don't recall reason why _E00 isn't used but to be safe maybe next unused
> would be better (it seems to be _E05)
> 
For some reason I thought E00 was spelled out in the VM Generation ID spec, but it isn’t.  I’ve reserved E05 for this
>> +    aml_append(method, aml_notify(aml_name("\\_SB.VGEN"), aml_int(0x80)));
>> +    aml_append(scope, method);
>> +    aml_append(ssdt, scope);
> you actually can skip scopes by providing full path in device/method name, like
> 
>  aml_method("\\_GPE._E05", …
> 
Nice

>> +
>> +    /* copy AML table into ACPI tables blob and patch in fw_cfg blob */
>> +    g_array_append_vals(table_data, ssdt->buf->data, ssdt->buf->len);
>> +    bios_linker_loader_alloc(linker, VMGENID_FW_CFG_FILE, guid, 0,
>> +                             false /* high memory */);
>> +    bios_linker_loader_add_pointer(linker,
>> +        ACPI_BUILD_TABLE_FILE, vgia_offset, sizeof(uint64_t),
>> +        VMGENID_FW_CFG_FILE, 0);
>> +
>> +    build_header(linker, table_data,
>> +        (void *)(table_data->data + table_data->len - ssdt->buf->len),
>> +        "SSDT", ssdt->buf->len, 1, NULL, "VMGENID");
>> +    free_aml_allocator();
>> +}
>> +
>> +void vmgenid_add_fw_cfg(FWCfgState *s)
>> +{
>> +    Object *obj = find_vmgenid_dev(NULL);
>> +    if (!obj) {
>> +        return;
>> +    }
>> +    VmGenIdState *vms = VMGENID(obj);
>> +    fw_cfg_add_file(s, VMGENID_FW_CFG_FILE, vms->guid.data,
>> +        sizeof(vms->guid.data));
>> +}
>> +
>> +static void vmgenid_notify_guest(VmGenIdState *s)
>> +{
>> +    Object *obj = object_resolve_path_type("", TYPE_ACPI_DEVICE_IF, NULL);
>> +    if (obj) {
>> +        /* Send _GPE.E00 event */
>> +        acpi_send_event(DEVICE(obj), 1 << 0);
>> +    }
>> +}
>> +
>> +static void vmgenid_set_guid(Object *obj, const char *value, Error **errp)
>> +{
>> +    VmGenIdState *s = VMGENID(obj);
>> +
>> +    if (qemu_uuid_parse(value, &s->guid) < 0) {
>> +        error_setg(errp, "'%s." VMGENID_GUID
>> +                   "': Failed to parse GUID string: %s",
>> +                   object_get_typename(OBJECT(s)),
>> +                   value);
>> +        return;
>> +    }
> here should be code that would copy new GIUD to
> buffer allocated by firmware and then after that don't forget
> call memory_region_set_dirty() on it.
> 
> Question is how you'd get address of it and I think that's where
> writable fwcfg files come in play.
> I'd prefer fwcfg_loader in firmware to write it back after buffer
> allocation is done as it's already has utilities for fw_cfg,
> but that probably would mean extending loader protocol to support
> new writeback command.
> 
> PS, address one would get from guest should be migrated as well
> 
OK, I’ll hack away at this part
> 
>> +    vmgenid_notify_guest(s);
>> +}
>> +
>> +static void vmgenid_state_change(void *priv, int running, RunState state)
>> +{
>> +    VmGenIdState *s;
>> +
>> +    if (state != RUN_STATE_RUNNING) {
>> +        return;
>> +    }
>> +    s = VMGENID((Object *)priv);
>> +    vmgenid_notify_guest(s);
>> +}
>> +
>> +static void vmgenid_initfn(Object *obj)
>> +{
>> +    object_property_add_str(obj, VMGENID_GUID, NULL, vmgenid_set_guid, NULL);
>> +    qemu_add_vm_change_state_handler(vmgenid_state_change, obj);
>> +}
>> +
>> +static const TypeInfo vmgenid_device_info = {
>> +    .name          = VMGENID_DEVICE,
>> +    .parent        = TYPE_SYS_BUS_DEVICE,
>> +    .instance_size = sizeof(VmGenIdState),
>> +    .instance_init = vmgenid_initfn,
>> +};
>> +
>> +static void vmgenid_register_types(void)
>> +{
>> +    type_register_static(&vmgenid_device_info);
>> +}
>> +
>> +type_init(vmgenid_register_types)
>> +
>> +GuidInfo *qmp_query_vm_generation_id(Error **errp)
>> +{
>> +    GuidInfo *info;
>> +    VmGenIdState *vdev;
>> +    Object *obj = find_vmgenid_dev(errp);
>> +
>> +    if (!obj) {
>> +        return NULL;
>> +    }
>> +    vdev = VMGENID(obj);
>> +    info = g_malloc0(sizeof(*info));
>> +    info->guid = g_strdup_printf(UUID_FMT, vdev->guid.data[0],
>> +            vdev->guid.data[1], vdev->guid.data[2], vdev->guid.data[3],
>> +            vdev->guid.data[4], vdev->guid.data[5], vdev->guid.data[6],
>> +            vdev->guid.data[7], vdev->guid.data[8], vdev->guid.data[9],
>> +            vdev->guid.data[10], vdev->guid.data[11], vdev->guid.data[12],
>> +            vdev->guid.data[13], vdev->guid.data[14], vdev->guid.data[15]);
> 
> perhaps qemu_uuid_unparse_strdup() could be used
> 
Nice, I didn’t know about that.
>> +    return info;
>> +}
>> +
>> +void qmp_set_vm_generation_id(const char *guid, Error **errp)
>> +{
>> +    Object *obj = find_vmgenid_dev(errp);
>> +
>> +    if (!obj) {
>> +        return;
>> +    }
>> +
>> +    object_property_set_str(obj, guid, VMGENID_GUID, errp);
>> +    return;
>> +}
>> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
>> index 9708cdc..cde81b7 100644
>> --- a/hw/i386/acpi-build.c
>> +++ b/hw/i386/acpi-build.c
>> @@ -42,6 +42,7 @@
>> #include "hw/acpi/memory_hotplug.h"
>> #include "sysemu/tpm.h"
>> #include "hw/acpi/tpm.h"
>> +#include "hw/acpi/vmgenid.h"
>> #include "sysemu/tpm_backend.h"
>> #include "hw/timer/mc146818rtc_regs.h"
>> #include "sysemu/numa.h"
>> @@ -2785,6 +2786,8 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
>>     acpi_add_table(table_offsets, tables_blob);
>>     build_madt(tables_blob, tables->linker, pcms);
>> 
> if (has_vmgenid()) {
>> +    vmgenid_build_acpi(table_offsets, tables_blob, tables->linker);
> }
> 
> and remove similar check for called function
> 
OK
>> +
>>     if (misc.has_hpet) {
>>         acpi_add_table(table_offsets, tables_blob);
>>         build_hpet(tables_blob, tables->linker);
>> @@ -2991,6 +2994,8 @@ void acpi_setup(void)
>>     fw_cfg_add_file(pcms->fw_cfg, ACPI_BUILD_TPMLOG_FILE,
>>                     tables.tcpalog->data, acpi_data_len(tables.tcpalog));
>> 
>> +    vmgenid_add_fw_cfg(pcms->fw_cfg);
> ditto
> 
>> +
>>     if (!pcmc->rsdp_in_ram) {
>>         /*
>>          * Keep for compatibility with old machine types.
>> diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
>> index 1a89615..ca0f1bb 100644
>> --- a/hw/misc/Makefile.objs
>> +++ b/hw/misc/Makefile.objs
>> @@ -53,3 +53,4 @@ obj-$(CONFIG_EDU) += edu.o
>> obj-$(CONFIG_HYPERV_TESTDEV) += hyperv_testdev.o
>> obj-$(CONFIG_AUX) += auxbus.o
>> obj-$(CONFIG_ASPEED_SOC) += aspeed_scu.o aspeed_sdmc.o
>> +obj-$(CONFIG_VMGENID) += vmgenid.o
>> diff --git a/include/hw/acpi/vmgenid.h b/include/hw/acpi/vmgenid.h
>> new file mode 100644
>> index 0000000..f8bdff2
>> --- /dev/null
>> +++ b/include/hw/acpi/vmgenid.h
>> @@ -0,0 +1,24 @@
>> +#ifndef ACPI_VMGENID_H
>> +#define ACPI_VMGENID_H
>> +
>> +#include "hw/acpi/bios-linker-loader.h"
>> +#include "hw/sysbus.h"
>> +#include "qemu/uuid.h"
>> +
>> +#define VMGENID_DEVICE           "vmgenid"
>> +#define VMGENID_GUID             "guid"
>> +#define VMGENID_FW_CFG_FILE      "etc/vmgenid"
>> +
>> +Object *find_vmgenid_dev(Error **errp);
>> +void vmgenid_add_fw_cfg(FWCfgState *s);
>> +void vmgenid_build_acpi(GArray *table_offsets, GArray *table_data,
>> +                       BIOSLinker *linker);
>> +
>> +#define VMGENID(obj) OBJECT_CHECK(VmGenIdState, (obj), VMGENID_DEVICE)
>> +
>> +typedef struct VmGenIdState {
>> +    SysBusDevice parent_obj;
>> +    QemuUUID guid;
>> +} VmGenIdState;
>> +
>> +#endif
> 


[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 3583 bytes --]

[Qemu-devel] [PATCH v2 4/6] qmp/hmp: add query-vm-generation-id and 'info vm-generation-id' commands

[ben]

From: Igor Mammedov <imammedo@redhat.com>

Add commands to query Virtual Machine Generation ID counter.

QMP command example:
    { "execute": "query-vm-generation-id" }

HMP command example:
    info vm-generation-id

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Ben Warren <ben@skyportsystems.com>
---
 hmp-commands-info.hx | 13 +++++++++++++
 hmp.c                |  9 +++++++++
 hmp.h                |  1 +
 qapi-schema.json     | 20 ++++++++++++++++++++
 stubs/Makefile.objs  |  1 +
 stubs/vmgenid.c      |  8 ++++++++
 6 files changed, 52 insertions(+)
 create mode 100644 stubs/vmgenid.c

diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
index 55d50c4..b0bb052 100644
--- a/hmp-commands-info.hx
+++ b/hmp-commands-info.hx
@@ -802,6 +802,19 @@ Show information about hotpluggable CPUs
 ETEXI
 
 STEXI
+@item info vm-generation-id
+Show Virtual Machine Generation ID
+ETEXI
+
+    {
+        .name       = "vm-generation-id",
+        .args_type  = "",
+        .params     = "",
+        .help       = "Show Virtual Machine Generation ID",
+        .cmd = hmp_info_vm_generation_id,
+    },
+
+STEXI
 @end table
 ETEXI
 
diff --git a/hmp.c b/hmp.c
index b869617..9ec27ae 100644
--- a/hmp.c
+++ b/hmp.c
@@ -2570,3 +2570,12 @@ void hmp_hotpluggable_cpus(Monitor *mon, const QDict *qdict)
 
     qapi_free_HotpluggableCPUList(saved);
 }
+
+void hmp_info_vm_generation_id(Monitor *mon, const QDict *qdict)
+{
+    GuidInfo *info = qmp_query_vm_generation_id(NULL);
+    if (info) {
+        monitor_printf(mon, "%s\n", info->guid);
+    }
+    qapi_free_GuidInfo(info);
+}
diff --git a/hmp.h b/hmp.h
index 05daf7c..799fd37 100644
--- a/hmp.h
+++ b/hmp.h
@@ -137,5 +137,6 @@ void hmp_rocker_of_dpa_flows(Monitor *mon, const QDict *qdict);
 void hmp_rocker_of_dpa_groups(Monitor *mon, const QDict *qdict);
 void hmp_info_dump(Monitor *mon, const QDict *qdict);
 void hmp_hotpluggable_cpus(Monitor *mon, const QDict *qdict);
+void hmp_info_vm_generation_id(Monitor *mon, const QDict *qdict);
 
 #endif
diff --git a/qapi-schema.json b/qapi-schema.json
index a0d3b5d..2348391 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -4776,3 +4776,23 @@
 # Since: 2.7
 ##
 { 'command': 'query-hotpluggable-cpus', 'returns': ['HotpluggableCPU'] }
+
+##
+# @GuidInfo:
+#
+# GUID information.
+#
+# @guid: the globally unique identifier
+#
+# Since: 2.9
+##
+{ 'struct': 'GuidInfo', 'data': {'guid': 'str'} }
+
+##
+# @query-vm-generation-id
+#
+# Show Virtual Machine Generation ID
+#
+# Since 2.9
+##
+{ 'command': 'query-vm-generation-id', 'returns': 'GuidInfo' }
diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
index 2b5bb74..988d5d7 100644
--- a/stubs/Makefile.objs
+++ b/stubs/Makefile.objs
@@ -50,3 +50,4 @@ stub-obj-y += smbios_type_38.o
 stub-obj-y += ipmi.o
 stub-obj-y += pc_madt_cpu_entry.o
 stub-obj-y += migration-colo.o
+stub-obj-y += vmgenid.o
diff --git a/stubs/vmgenid.c b/stubs/vmgenid.c
new file mode 100644
index 0000000..8c448ac
--- /dev/null
+++ b/stubs/vmgenid.c
@@ -0,0 +1,8 @@
+#include "qemu/osdep.h"
+#include "qmp-commands.h"
+
+GuidInfo *qmp_query_vm_generation_id(Error **errp)
+{
+    error_setg(errp, "this command is not currently supported");
+    return NULL;
+}
-- 
2.7.4

[Qemu-devel] [PATCH v2 5/6] qmp/hmp: add set-vm-generation-id commands

[ben]

From: Igor Mammedov <imammedo@redhat.com>

Add set-vm-generation-id command to set Virtual Machine
Generation ID counter.

QMP command example:
    { "execute": "set-vm-generation-id",
          "arguments": {
              "guid": "324e6eaf-d1d1-4bf6-bf41-b9bb6c91fb87"
          }
    }

HMP command example:
    set-vm-generation-id guid=324e6eaf-d1d1-4bf6-bf41-b9bb6c91fb87

Signed-off-by: Ben Warren <ben@skyportsystems.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Cc: Eric Blake <eblake@redhat.com>
---
 hmp-commands.hx  | 13 +++++++++++++
 hmp.c            | 12 ++++++++++++
 hmp.h            |  1 +
 qapi-schema.json | 12 ++++++++++++
 stubs/vmgenid.c  |  6 ++++++
 5 files changed, 44 insertions(+)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 8819281..56744aa 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1775,5 +1775,18 @@ ETEXI
     },
 
 STEXI
+@item set-vm-generation-id @var{uuid}
+Set Virtual Machine Generation ID counter to @var{guid}
+ETEXI
+
+    {
+        .name       = "set-vm-generation-id",
+        .args_type  = "guid:s",
+        .params     = "guid",
+        .help       = "Set Virtual Machine Generation ID counter",
+        .cmd = hmp_set_vm_generation_id,
+    },
+
+STEXI
 @end table
 ETEXI
diff --git a/hmp.c b/hmp.c
index 9ec27ae..a54a312 100644
--- a/hmp.c
+++ b/hmp.c
@@ -2579,3 +2579,15 @@ void hmp_info_vm_generation_id(Monitor *mon, const QDict *qdict)
     }
     qapi_free_GuidInfo(info);
 }
+
+void hmp_set_vm_generation_id(Monitor *mon, const QDict *qdict)
+{
+    Error *errp = NULL;
+    const char *guid = qdict_get_str(qdict, "guid");
+
+    qmp_set_vm_generation_id(guid, &errp);
+    if (errp) {
+        hmp_handle_error(mon, &errp);
+        return;
+    }
+}
diff --git a/hmp.h b/hmp.h
index 799fd37..e0ac1e8 100644
--- a/hmp.h
+++ b/hmp.h
@@ -138,5 +138,6 @@ void hmp_rocker_of_dpa_groups(Monitor *mon, const QDict *qdict);
 void hmp_info_dump(Monitor *mon, const QDict *qdict);
 void hmp_hotpluggable_cpus(Monitor *mon, const QDict *qdict);
 void hmp_info_vm_generation_id(Monitor *mon, const QDict *qdict);
+void hmp_set_vm_generation_id(Monitor *mon, const QDict *qdict);
 
 #endif
diff --git a/qapi-schema.json b/qapi-schema.json
index 2348391..c5ebea4 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -4796,3 +4796,15 @@
 # Since 2.9
 ##
 { 'command': 'query-vm-generation-id', 'returns': 'GuidInfo' }
+
+##
+# @set-vm-generation-id
+#
+# Set Virtual Machine Generation ID
+#
+# @changed: Is the Virtual Machine Generation ID a new value?
+# @guid: new GUID to set as Virtual Machine Generation ID
+#
+# Since 2.9
+##
+{ 'command': 'set-vm-generation-id', 'data': {'guid': 'str'} }
diff --git a/stubs/vmgenid.c b/stubs/vmgenid.c
index 8c448ac..d25d41b 100644
--- a/stubs/vmgenid.c
+++ b/stubs/vmgenid.c
@@ -6,3 +6,9 @@ GuidInfo *qmp_query_vm_generation_id(Error **errp)
     error_setg(errp, "this command is not currently supported");
     return NULL;
 }
+
+void qmp_set_vm_generation_id(const char *guid, Error **errp)
+{
+    error_setg(errp, "this command is not currently supported");
+    return;
+}
-- 
2.7.4

Re: [Qemu-devel] [PATCH v2 5/6] qmp/hmp: add set-vm-generation-id commands

[Igor Mammedov]

On Mon, 16 Jan 2017 11:20:57 -0800
ben@skyportsystems.com wrote:

> From: Igor Mammedov <imammedo@redhat.com>
> 
> Add set-vm-generation-id command to set Virtual Machine
> Generation ID counter.
> 
> QMP command example:
>     { "execute": "set-vm-generation-id",
>           "arguments": {
>               "guid": "324e6eaf-d1d1-4bf6-bf41-b9bb6c91fb87"
>           }
>     }
> 
> HMP command example:
>     set-vm-generation-id guid=324e6eaf-d1d1-4bf6-bf41-b9bb6c91fb87
> 
> Signed-off-by: Ben Warren <ben@skyportsystems.com>
> Cc: Igor Mammedov <imammedo@redhat.com>
> Cc: Eric Blake <eblake@redhat.com>
when you are reposting someone else written patch,
you are supposed to keep SoB lines patch has had and just add
your own SoB after original SoB lines.

If you made non trivial change to the patch then ask
original author(s) if they wish to keep their SoB
before posting modified patch.
(Usually I do it offline).

> ---
>  hmp-commands.hx  | 13 +++++++++++++
>  hmp.c            | 12 ++++++++++++
>  hmp.h            |  1 +
>  qapi-schema.json | 12 ++++++++++++
>  stubs/vmgenid.c  |  6 ++++++
>  5 files changed, 44 insertions(+)
> 
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index 8819281..56744aa 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -1775,5 +1775,18 @@ ETEXI
>      },
>  
>  STEXI
> +@item set-vm-generation-id @var{uuid}
> +Set Virtual Machine Generation ID counter to @var{guid}
> +ETEXI
> +
> +    {
> +        .name       = "set-vm-generation-id",
> +        .args_type  = "guid:s",
> +        .params     = "guid",
> +        .help       = "Set Virtual Machine Generation ID counter",
> +        .cmd = hmp_set_vm_generation_id,
> +    },
> +
> +STEXI
>  @end table
>  ETEXI
> diff --git a/hmp.c b/hmp.c
> index 9ec27ae..a54a312 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -2579,3 +2579,15 @@ void hmp_info_vm_generation_id(Monitor *mon, const QDict *qdict)
>      }
>      qapi_free_GuidInfo(info);
>  }
> +
> +void hmp_set_vm_generation_id(Monitor *mon, const QDict *qdict)
> +{
> +    Error *errp = NULL;
> +    const char *guid = qdict_get_str(qdict, "guid");
> +
> +    qmp_set_vm_generation_id(guid, &errp);
> +    if (errp) {
> +        hmp_handle_error(mon, &errp);
> +        return;
> +    }
> +}
> diff --git a/hmp.h b/hmp.h
> index 799fd37..e0ac1e8 100644
> --- a/hmp.h
> +++ b/hmp.h
> @@ -138,5 +138,6 @@ void hmp_rocker_of_dpa_groups(Monitor *mon, const QDict *qdict);
>  void hmp_info_dump(Monitor *mon, const QDict *qdict);
>  void hmp_hotpluggable_cpus(Monitor *mon, const QDict *qdict);
>  void hmp_info_vm_generation_id(Monitor *mon, const QDict *qdict);
> +void hmp_set_vm_generation_id(Monitor *mon, const QDict *qdict);
>  
>  #endif
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 2348391..c5ebea4 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -4796,3 +4796,15 @@
>  # Since 2.9
>  ##
>  { 'command': 'query-vm-generation-id', 'returns': 'GuidInfo' }
> +
> +##
> +# @set-vm-generation-id
> +#
> +# Set Virtual Machine Generation ID
> +#
> +# @changed: Is the Virtual Machine Generation ID a new value?
> +# @guid: new GUID to set as Virtual Machine Generation ID
> +#
> +# Since 2.9
> +##
> +{ 'command': 'set-vm-generation-id', 'data': {'guid': 'str'} }
> diff --git a/stubs/vmgenid.c b/stubs/vmgenid.c
> index 8c448ac..d25d41b 100644
> --- a/stubs/vmgenid.c
> +++ b/stubs/vmgenid.c
> @@ -6,3 +6,9 @@ GuidInfo *qmp_query_vm_generation_id(Error **errp)
>      error_setg(errp, "this command is not currently supported");
>      return NULL;
>  }
> +
> +void qmp_set_vm_generation_id(const char *guid, Error **errp)
> +{
> +    error_setg(errp, "this command is not currently supported");
> +    return;
> +}

Re: [Qemu-devel] [PATCH v2 5/6] qmp/hmp: add set-vm-generation-id commands

[Ben Warren]

[-- Attachment #1: Type: text/plain, Size: 4177 bytes --]


> On Jan 18, 2017, at 12:55 AM, Igor Mammedov <imammedo@redhat.com> wrote:
> 
> On Mon, 16 Jan 2017 11:20:57 -0800
> ben@skyportsystems.com <mailto:ben@skyportsystems.com> wrote:
> 
>> From: Igor Mammedov <imammedo@redhat.com>
>> 
>> Add set-vm-generation-id command to set Virtual Machine
>> Generation ID counter.
>> 
>> QMP command example:
>>    { "execute": "set-vm-generation-id",
>>          "arguments": {
>>              "guid": "324e6eaf-d1d1-4bf6-bf41-b9bb6c91fb87"
>>          }
>>    }
>> 
>> HMP command example:
>>    set-vm-generation-id guid=324e6eaf-d1d1-4bf6-bf41-b9bb6c91fb87
>> 
>> Signed-off-by: Ben Warren <ben@skyportsystems.com>
>> Cc: Igor Mammedov <imammedo@redhat.com>
>> Cc: Eric Blake <eblake@redhat.com>
> when you are reposting someone else written patch,
> you are supposed to keep SoB lines patch has had and just add
> your own SoB after original SoB lines.
> 
> If you made non trivial change to the patch then ask
> original author(s) if they wish to keep their SoB
> before posting modified patch.
> (Usually I do it offline).
> 
Yeah, this was a mistake on my part.  This is essentially your older patch rebased to ToT.  I meant to just append my SoB

>> ---
>> hmp-commands.hx  | 13 +++++++++++++
>> hmp.c            | 12 ++++++++++++
>> hmp.h            |  1 +
>> qapi-schema.json | 12 ++++++++++++
>> stubs/vmgenid.c  |  6 ++++++
>> 5 files changed, 44 insertions(+)
>> 
>> diff --git a/hmp-commands.hx b/hmp-commands.hx
>> index 8819281..56744aa 100644
>> --- a/hmp-commands.hx
>> +++ b/hmp-commands.hx
>> @@ -1775,5 +1775,18 @@ ETEXI
>>     },
>> 
>> STEXI
>> +@item set-vm-generation-id @var{uuid}
>> +Set Virtual Machine Generation ID counter to @var{guid}
>> +ETEXI
>> +
>> +    {
>> +        .name       = "set-vm-generation-id",
>> +        .args_type  = "guid:s",
>> +        .params     = "guid",
>> +        .help       = "Set Virtual Machine Generation ID counter",
>> +        .cmd = hmp_set_vm_generation_id,
>> +    },
>> +
>> +STEXI
>> @end table
>> ETEXI
>> diff --git a/hmp.c b/hmp.c
>> index 9ec27ae..a54a312 100644
>> --- a/hmp.c
>> +++ b/hmp.c
>> @@ -2579,3 +2579,15 @@ void hmp_info_vm_generation_id(Monitor *mon, const QDict *qdict)
>>     }
>>     qapi_free_GuidInfo(info);
>> }
>> +
>> +void hmp_set_vm_generation_id(Monitor *mon, const QDict *qdict)
>> +{
>> +    Error *errp = NULL;
>> +    const char *guid = qdict_get_str(qdict, "guid");
>> +
>> +    qmp_set_vm_generation_id(guid, &errp);
>> +    if (errp) {
>> +        hmp_handle_error(mon, &errp);
>> +        return;
>> +    }
>> +}
>> diff --git a/hmp.h b/hmp.h
>> index 799fd37..e0ac1e8 100644
>> --- a/hmp.h
>> +++ b/hmp.h
>> @@ -138,5 +138,6 @@ void hmp_rocker_of_dpa_groups(Monitor *mon, const QDict *qdict);
>> void hmp_info_dump(Monitor *mon, const QDict *qdict);
>> void hmp_hotpluggable_cpus(Monitor *mon, const QDict *qdict);
>> void hmp_info_vm_generation_id(Monitor *mon, const QDict *qdict);
>> +void hmp_set_vm_generation_id(Monitor *mon, const QDict *qdict);
>> 
>> #endif
>> diff --git a/qapi-schema.json b/qapi-schema.json
>> index 2348391..c5ebea4 100644
>> --- a/qapi-schema.json
>> +++ b/qapi-schema.json
>> @@ -4796,3 +4796,15 @@
>> # Since 2.9
>> ##
>> { 'command': 'query-vm-generation-id', 'returns': 'GuidInfo' }
>> +
>> +##
>> +# @set-vm-generation-id
>> +#
>> +# Set Virtual Machine Generation ID
>> +#
>> +# @changed: Is the Virtual Machine Generation ID a new value?
>> +# @guid: new GUID to set as Virtual Machine Generation ID
>> +#
>> +# Since 2.9
>> +##
>> +{ 'command': 'set-vm-generation-id', 'data': {'guid': 'str'} }
>> diff --git a/stubs/vmgenid.c b/stubs/vmgenid.c
>> index 8c448ac..d25d41b 100644
>> --- a/stubs/vmgenid.c
>> +++ b/stubs/vmgenid.c
>> @@ -6,3 +6,9 @@ GuidInfo *qmp_query_vm_generation_id(Error **errp)
>>     error_setg(errp, "this command is not currently supported");
>>     return NULL;
>> }
>> +
>> +void qmp_set_vm_generation_id(const char *guid, Error **errp)
>> +{
>> +    error_setg(errp, "this command is not currently supported");
>> +    return;
>> +}


[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 3583 bytes --]

[Qemu-devel] [PATCH v2 6/6] PC: Support dynamic sysbus on pc_i440fx

[ben]

From: Ben Warren <ben@skyportsystems.com>

Signed-off-by: Ben Warren <ben@skyportsystems.com>
---
 hw/i386/pc_piix.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index a54a468..8ac894c 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -435,6 +435,7 @@ static void pc_i440fx_machine_options(MachineClass *m)
     m->hot_add_cpu = pc_hot_add_cpu;
     m->default_machine_opts = "firmware=bios-256k.bin";
     m->default_display = "std";
+    m->has_dynamic_sysbus = true;
 }
 
 static void pc_i440fx_2_8_machine_options(MachineClass *m)
-- 
2.7.4

Re: [Qemu-devel] [PATCH v2 6/6] PC: Support dynamic sysbus on pc_i440fx

[Igor Mammedov]

On Mon, 16 Jan 2017 11:20:58 -0800
ben@skyportsystems.com wrote:

> From: Ben Warren <ben@skyportsystems.com>
commit message should give a reason why it's need.

> 
> Signed-off-by: Ben Warren <ben@skyportsystems.com>
> ---
>  hw/i386/pc_piix.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index a54a468..8ac894c 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -435,6 +435,7 @@ static void pc_i440fx_machine_options(MachineClass *m)
>      m->hot_add_cpu = pc_hot_add_cpu;
>      m->default_machine_opts = "firmware=bios-256k.bin";
>      m->default_display = "std";
> +    m->has_dynamic_sysbus = true;
>  }
>  
>  static void pc_i440fx_2_8_machine_options(MachineClass *m)