[Qemu-devel] [PATCH V13 0/4] Virtual Machine Generation ID

[Qemu-devel] [PATCH V13 0/4] Virtual Machine Generation ID

[Igor Mammedov]

Note to maintainer:
 this patch set doesn't include the *.hex.generated files and
 updated test ACPI tables blobs. Hex files for [q35-]acpi-dsdt
 should be updated.

Tested with WS2012R2DCx64, thanks Gal for providing test utilities.

Based on top of mst's PCI tree.
Git branch:
https://github.com/imammedo/qemu.git vmgenid_v13

V13 -
      * drop usage of bios_linker_loader and rewrite to use
        PCIDevice so that we don't have to mess with complex
        linker interface and patch ACPI tables then read VMGENID
        buffer adddress in guest OSPM and communicate it to QEMU
        via reserved MMIO region.
        It * makes initialization more stratforward (like any PCI
             device) but consumes 1 PCI slot (not a big deal with bridges),
           * saves us MMIO resources since there is no need
             to write buffer HPA from OSPM to QEMU
           * simplifies AML, replaces methods and OperationRegion with
             integer constant pointing to buffer HPA
        It also allows us to write a more complete unit test
        that wouldn't require to run OSPM so that it could update
        HPA in QEMU.
      * use AML API for instead of template patching
      * make 'vmgenid' optional, users who want to use it
        should add -device vmgenid,.... to QEMU CLI
        it also saves us some space in SSDT if device is not used
      * mark UUID buffer as dirty when it's updated via QMP in runtime
      * make 'uuid' property mandatory at -device
      * fix: make sure that test actually runs
      * use property to get UUID buffer HPA
      * add setting UUID via QMP at runtime test case

V12 - Fixed bios_linker_loader_add_pointer call parameters. Offset
      should be relative to the table.

V11 - Add required missing files.

V10 - Fixed typos in docs and a few clarification.

V9 - Add a unit test.
   - Rebased to version 2.2.
   - Removed hex.generated the binary files from patch.

V8 - Add a device's description file.
   - GUID is stored in fw cfg file and the guest writes the
     physical address to the device (reduces vmexits).

V7 - Move the device's description back to the static SSDT table.
   - The GUID is store in a "hard coded" physical address and not
     in the ACPI table itself.
   - ACPI notification is triggered when the GUID is changed.

V6 - include the pre-compiled ASL file
   - remove an empty line at end of files.

V5 - Move device's description to SSDT table (dynamic).

V4 - Fix a typo in error message string.
   - Move device's description from DSDT back to SSDT table.

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

Gal Hammer (1):
  docs: vm generation id device's description

Igor Mammedov (3):
  acpi: extend ACPI interface to provide access to ACPI registers and
    SCI irq
  pc: add a Virtual Machine Generation ID device
  tests: add a unit test for the vmgenid device.

 default-configs/i386-softmmu.mak     |   1 +
 default-configs/x86_64-softmmu.mak   |   1 +
 docs/specs/pci-ids.txt               |   1 +
 docs/specs/vmgenid.txt               |  36 ++++++++++
 hw/acpi/piix4.c                      |  17 +++++
 hw/i386/acpi-build.c                 |  33 +++++++++
 hw/i386/acpi-dsdt.dsl                |   2 -
 hw/i386/q35-acpi-dsdt.dsl            |   2 -
 hw/isa/lpc_ich9.c                    |  16 +++++
 hw/isa/vt82c686.c                    |  24 ++++++-
 hw/misc/Makefile.objs                |   1 +
 hw/misc/vmgenid.c                    | 130 +++++++++++++++++++++++++++++++++++
 include/hw/acpi/acpi.h               |   1 +
 include/hw/acpi/acpi_dev_interface.h |   9 +++
 include/hw/misc/vmgenid.h            |  21 ++++++
 include/hw/pci/pci.h                 |   1 +
 tests/Makefile                       |   2 +
 tests/vmgenid-test.c                 |  92 +++++++++++++++++++++++++
 18 files changed, 385 insertions(+), 5 deletions(-)
 create mode 100644 docs/specs/vmgenid.txt
 create mode 100644 hw/misc/vmgenid.c
 create mode 100644 include/hw/misc/vmgenid.h
 create mode 100644 tests/vmgenid-test.c

-- 
1.8.3.1

[Qemu-devel] [PATCH V13 1/4] acpi: extend ACPI interface to provide access to ACPI registers and SCI irq

[Igor Mammedov]

so that we don't have to always add proxy wrappers in piix4pm/ich9
to access ACPI regs and SCI kept in piix4pm/lcp_ich9 devices
and call acpi_foo() API directly.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
It will be used by following patch that add vmgenid device
---
 hw/acpi/piix4.c                      | 17 +++++++++++++++++
 hw/isa/lpc_ich9.c                    | 16 ++++++++++++++++
 hw/isa/vt82c686.c                    | 24 +++++++++++++++++++++++-
 include/hw/acpi/acpi_dev_interface.h |  9 +++++++++
 4 files changed, 65 insertions(+), 1 deletion(-)

diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index d1f1179..3acf650 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -580,6 +580,21 @@ static void piix4_ospm_status(AcpiDeviceIf *adev, ACPIOSTInfoList ***list)
     acpi_memory_ospm_status(&s->acpi_memory_hotplug, list);
 }
 
+static ACPIREGS *piix4_acpi_regs(AcpiDeviceIf *adev)
+{
+    PIIX4PMState *s = PIIX4_PM(adev);
+
+    return &s->ar;
+}
+
+static qemu_irq piix4_acpi_irq(AcpiDeviceIf *adev)
+{
+    PIIX4PMState *s = PIIX4_PM(adev);
+
+    return s->irq;
+}
+
+
 static Property piix4_pm_properties[] = {
     DEFINE_PROP_UINT32("smb_io_base", PIIX4PMState, smb_io_base, 0),
     DEFINE_PROP_UINT8(ACPI_PM_PROP_S3_DISABLED, PIIX4PMState, disable_s3, 0),
@@ -618,6 +633,8 @@ static void piix4_pm_class_init(ObjectClass *klass, void *data)
     hc->unplug_request = piix4_device_unplug_request_cb;
     hc->unplug = piix4_device_unplug_cb;
     adevc->ospm_status = piix4_ospm_status;
+    adevc->regs = piix4_acpi_regs;
+    adevc->sci = piix4_acpi_irq;
 }
 
 static const TypeInfo piix4_pm_info = {
diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c
index 16f5a0d..d818a84 100644
--- a/hw/isa/lpc_ich9.c
+++ b/hw/isa/lpc_ich9.c
@@ -662,6 +662,20 @@ static const VMStateDescription vmstate_ich9_lpc = {
     }
 };
 
+static ACPIREGS *ich9_acpi_regs(AcpiDeviceIf *adev)
+{
+    ICH9LPCState *s = ICH9_LPC_DEVICE(adev);
+
+    return &s->pm.acpi_regs;
+}
+
+static qemu_irq ich9_acpi_irq(AcpiDeviceIf *adev)
+{
+    ICH9LPCState *s = ICH9_LPC_DEVICE(adev);
+
+    return s->pm.irq;
+}
+
 static void ich9_lpc_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
@@ -688,6 +702,8 @@ static void ich9_lpc_class_init(ObjectClass *klass, void *data)
     hc->unplug_request = ich9_device_unplug_request_cb;
     hc->unplug = ich9_device_unplug_cb;
     adevc->ospm_status = ich9_pm_ospm_status;
+    adevc->regs = ich9_acpi_regs;
+    adevc->sci = ich9_acpi_irq;
 }
 
 static const TypeInfo ich9_lpc_info = {
diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index bc59f29..6d84024 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -155,6 +155,9 @@ static void vt82c686b_write_config(PCIDevice * d, uint32_t address,
 
 #define ACPI_DBG_IO_ADDR  0xb044
 
+#define TYPE_VT82C686B_PM "VT82C686B_PM"
+#define VT82C686B_PM(obj) OBJECT_CHECK(VT686PMState, (obj), TYPE_VT82C686B_PM)
+
 typedef struct VT686PMState {
     PCIDevice dev;
     MemoryRegion io;
@@ -375,6 +378,18 @@ I2CBus *vt82c686b_pm_init(PCIBus *bus, int devfn, uint32_t smb_io_base,
     return s->smb.smbus;
 }
 
+static ACPIREGS *via_pm_acpi_regs(AcpiDeviceIf *adev)
+{
+    VT686PMState *s = VT82C686B_PM(adev);
+
+    return &s->ar;
+}
+
+static qemu_irq via_pm_acpi_irq(AcpiDeviceIf *adev)
+{
+    return NULL;
+}
+
 static Property via_pm_properties[] = {
     DEFINE_PROP_UINT32("smb_io_base", VT686PMState, smb_io_base, 0),
     DEFINE_PROP_END_OF_LIST(),
@@ -384,6 +399,7 @@ static void via_pm_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+    AcpiDeviceIfClass *adevc = ACPI_DEVICE_IF_CLASS(klass);
 
     k->realize = vt82c686b_pm_realize;
     k->config_write = pm_write_config;
@@ -395,13 +411,19 @@ static void via_pm_class_init(ObjectClass *klass, void *data)
     dc->vmsd = &vmstate_acpi;
     set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
     dc->props = via_pm_properties;
+    adevc->regs = via_pm_acpi_regs;
+    adevc->sci = via_pm_acpi_irq;
 }
 
 static const TypeInfo via_pm_info = {
-    .name          = "VT82C686B_PM",
+    .name          = TYPE_VT82C686B_PM,
     .parent        = TYPE_PCI_DEVICE,
     .instance_size = sizeof(VT686PMState),
     .class_init    = via_pm_class_init,
+    .interfaces = (InterfaceInfo[]) {
+        { TYPE_ACPI_DEVICE_IF },
+        { }
+    }
 };
 
 static const VMStateDescription vmstate_via = {
diff --git a/include/hw/acpi/acpi_dev_interface.h b/include/hw/acpi/acpi_dev_interface.h
index f245f8d..e3f1bad 100644
--- a/include/hw/acpi/acpi_dev_interface.h
+++ b/include/hw/acpi/acpi_dev_interface.h
@@ -3,6 +3,9 @@
 
 #include "qom/object.h"
 #include "qapi-types.h"
+#include "hw/irq.h"
+
+typedef struct ACPIREGS ACPIREGS;
 
 #define TYPE_ACPI_DEVICE_IF "acpi-device-interface"
 
@@ -28,6 +31,10 @@ typedef struct AcpiDeviceIf {
  * ospm_status: returns status of ACPI device objects, reported
  *              via _OST method if device supports it.
  *
+ * regs: returns pointer to ACPI registers block
+ *
+ * sci: return pointer to IRQ object associated with SCI
+ *
  * Interface is designed for providing unified interface
  * to generic ACPI functionality that could be used without
  * knowledge about internals of actual device that implements
@@ -39,5 +46,7 @@ typedef struct AcpiDeviceIfClass {
 
     /* <public> */
     void (*ospm_status)(AcpiDeviceIf *adev, ACPIOSTInfoList ***list);
+    ACPIREGS *(*regs)(AcpiDeviceIf *adev);
+    qemu_irq (*sci)(AcpiDeviceIf *adev);
 } AcpiDeviceIfClass;
 #endif
-- 
1.8.3.1

[Qemu-devel] [PATCH V13 2/4] docs: vm generation id device's description

[Igor Mammedov]

From: Gal Hammer <ghammer@redhat.com>

Signed-off-by: Gal Hammer <ghammer@redhat.com>
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
v2:
 - ammend doc since no more MMIO occupied by vmgenid
   device and device became PCIDevice
---
 docs/specs/vmgenid.txt | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 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..718850a
--- /dev/null
+++ b/docs/specs/vmgenid.txt
@@ -0,0 +1,36 @@
+VIRTUAL MACHINE GENERATION ID
+=============================
+
+Copyright (C) 2014 Red Hat, 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 PCI device with the following ACPI ID: "QEMU0003".
+
+The device has a "vmgenid.uuid" property, which can be set using
+the command line argument or the QMP interface.
+For example:
+QEMU  -device vmgenid,id=FOO,uuid="324e6eaf-d1d1-4bf6-bf41-b9bb6c91fb87"
+
+Or to change uuid in runtime use:
+ qom-set "/machine/peripheral/FOO.uuid" "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.
-- 
1.8.3.1

[Qemu-devel] [PATCH V13 3/4] pc: add a Virtual Machine Generation ID device

[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 and its implementation.

The GUID is set using "vmgenid.uuid" property.

Example of using vmgenid device:
 -device vmgenid,id=FOO,uuid="324e6eaf-d1d1-4bf6-bf41-b9bb6c91fb87"

To change uuid in runtime use:
qom-set "/machine/peripheral/FOO.uuid" "124e6eaf-d1d1-4bf6-bf41-b9bb6c91fb87"

'vmgenid' device initialization flow is as following:
 1. vmgenid has RAM BAR resistered with size of UUID buffer
 2. BIOS initializes PCI devices and it maps BAR in PCI hole
 3. BIOS reads ACPI tables from QEMU, at that moment tables
    are generated with \_SB.VMGI.ADDR constant pointing to
    HPA where BIOS's mapped vmgenid's BAR earlier

Signed-off-by: Gal Hammer <ghammer@redhat.com>
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
v2:
  * rewrite to use PCIDevice so that we don't have to mess
    with complex fwcfg/linker and patch ACPI tables then
    read VMGENID buffer adddress in guest OSPM and communicate
    it to QEMU via reserved MMIO region.
    Which also allows us to write a more complete unit test
    that wouldn't require to run OSPM so that it could update
    HPA in QEMU.
  * make 'vmgenid' optional, users who want to use it
    should add -device vmgenid,.... to QEMU CLI
    it also saves us some space in SSDT if device is not used
  * mark UUID buffer as dirty when it's updated via QMP in runtime
  * make 'uiid' property mandatory at -device
---
 default-configs/i386-softmmu.mak   |   1 +
 default-configs/x86_64-softmmu.mak |   1 +
 docs/specs/pci-ids.txt             |   1 +
 hw/i386/acpi-build.c               |  33 ++++++++++
 hw/i386/acpi-dsdt.dsl              |   2 -
 hw/i386/q35-acpi-dsdt.dsl          |   2 -
 hw/misc/Makefile.objs              |   1 +
 hw/misc/vmgenid.c                  | 130 +++++++++++++++++++++++++++++++++++++
 include/hw/acpi/acpi.h             |   1 +
 include/hw/misc/vmgenid.h          |  21 ++++++
 include/hw/pci/pci.h               |   1 +
 11 files changed, 190 insertions(+), 4 deletions(-)
 create mode 100644 hw/misc/vmgenid.c
 create mode 100644 include/hw/misc/vmgenid.h

diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak
index bd99af9..0b913a8 100644
--- a/default-configs/i386-softmmu.mak
+++ b/default-configs/i386-softmmu.mak
@@ -43,3 +43,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 e7c2734..de5e6af 100644
--- a/default-configs/x86_64-softmmu.mak
+++ b/default-configs/x86_64-softmmu.mak
@@ -43,3 +43,4 @@ CONFIG_IOAPIC=y
 CONFIG_ICC_BUS=y
 CONFIG_PVPANIC=y
 CONFIG_MEM_HOTPLUG=y
+CONFIG_VMGENID=y
diff --git a/docs/specs/pci-ids.txt b/docs/specs/pci-ids.txt
index c6732fe..b398c5d 100644
--- a/docs/specs/pci-ids.txt
+++ b/docs/specs/pci-ids.txt
@@ -46,6 +46,7 @@ PCI devices (other than virtio):
 1b36:0004  PCI Quad-port 16550A adapter (docs/specs/pci-serial.txt)
 1b36:0005  PCI test device (docs/specs/pci-testdev.txt)
 1b36:0007  PCI SD Card Host Controller Interface (SDHCI)
+1b36:0009  PCI VM-Generation device
 
 All these devices are documented in docs/specs.
 
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 4d5d7e3..9290ae3 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -50,6 +50,7 @@
 #include "hw/pci/pci_bus.h"
 #include "hw/pci-host/q35.h"
 #include "hw/i386/intel_iommu.h"
+#include "hw/misc/vmgenid.h"
 
 #include "hw/i386/q35-acpi-dsdt.hex"
 #include "hw/i386/acpi-dsdt.hex"
@@ -110,6 +111,7 @@ typedef struct AcpiPmInfo {
 } AcpiPmInfo;
 
 typedef struct AcpiMiscInfo {
+    uint32_t vmgen_buf_paddr;
     bool has_hpet;
     bool has_tpm;
     DECLARE_BITMAP(slot_hotplug_enable, PCI_SLOT_MAX);
@@ -245,6 +247,14 @@ static void acpi_get_pm_info(AcpiPmInfo *pm)
 
 static void acpi_get_misc_info(AcpiMiscInfo *info)
 {
+    Object *obj;
+
+    obj = object_resolve_path_type("", VMGENID_DEVICE, NULL);
+    info->vmgen_buf_paddr = 0;
+    if (obj) {
+        info->vmgen_buf_paddr =
+            object_property_get_int(obj, VMGENID_VMGID_ADDR, NULL);
+    }
     info->has_hpet = hpet_find();
     info->has_tpm = tpm_find();
     info->pvpanic_port = pvpanic_port();
@@ -981,6 +991,29 @@ build_ssdt(GArray *table_data, GArray *linker,
 
     sb_scope = aml_scope("_SB");
     {
+        if (misc->vmgen_buf_paddr) {
+            dev = aml_device("VMGI");
+
+            aml_append(dev, aml_name_decl("_HID", aml_string("QEMU0003")));
+            aml_append(dev, aml_name_decl("_CID", aml_string("VM_Gen_Counter")));
+            aml_append(dev, aml_name_decl("_DDN", aml_string("VM_Gen_Counter")));
+
+            pkg = aml_package(2);
+            /* low 32 bits of UUID buffer addr*/
+            aml_append(pkg, aml_int(misc->vmgen_buf_paddr));
+            aml_append(pkg, aml_int(0)); /* high 32 bits of UUID buffer addr */
+            aml_append(dev, aml_name_decl("ADDR", pkg));
+
+            aml_append(sb_scope, dev);
+
+            scope = aml_scope("\\_GPE");
+            method = aml_method("_E00", 0);
+            aml_append(method,
+                aml_notify(aml_name("\\_SB.VMGI"), aml_int(0x80)));
+            aml_append(scope, method);
+            aml_append(ssdt, scope);
+        }
+
         /* create PCI0.PRES device and its _CRS to reserve CPU hotplug MMIO */
         dev = aml_device("PCI0." stringify(CPU_HOTPLUG_RESOURCE_DEVICE));
         aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A06")));
diff --git a/hw/i386/acpi-dsdt.dsl b/hw/i386/acpi-dsdt.dsl
index a611e07..884038b 100644
--- a/hw/i386/acpi-dsdt.dsl
+++ b/hw/i386/acpi-dsdt.dsl
@@ -306,8 +306,6 @@ DefinitionBlock (
     Scope(\_GPE) {
         Name(_HID, "ACPI0006")
 
-        Method(_L00) {
-        }
         Method(_E01) {
             // PCI hotplug event
             Acquire(\_SB.PCI0.BLCK, 0xFFFF)
diff --git a/hw/i386/q35-acpi-dsdt.dsl b/hw/i386/q35-acpi-dsdt.dsl
index e1cee5d..9eb794c 100644
--- a/hw/i386/q35-acpi-dsdt.dsl
+++ b/hw/i386/q35-acpi-dsdt.dsl
@@ -414,8 +414,6 @@ DefinitionBlock (
     Scope(\_GPE) {
         Name(_HID, "ACPI0006")
 
-        Method(_L00) {
-        }
         Method(_L01) {
         }
         Method(_E02) {
diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
index 029a56f..e047aea 100644
--- a/hw/misc/Makefile.objs
+++ b/hw/misc/Makefile.objs
@@ -41,3 +41,4 @@ obj-$(CONFIG_ZYNQ) += zynq_slcr.o
 
 obj-$(CONFIG_PVPANIC) += pvpanic.o
 obj-$(CONFIG_EDU) += edu.o
+obj-$(CONFIG_VMGENID) += vmgenid.o
diff --git a/hw/misc/vmgenid.c b/hw/misc/vmgenid.c
new file mode 100644
index 0000000..631c9a3
--- /dev/null
+++ b/hw/misc/vmgenid.c
@@ -0,0 +1,130 @@
+/*
+ *  Virtual Machine Generation ID Device
+ *
+ *  Copyright (C) 2014 Red Hat Inc.
+ *
+ *  Authors: Gal Hammer <ghammer@redhat.com>
+ *           Igor Mammedov <imammedo@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/pci/pci.h"
+#include "hw/misc/vmgenid.h"
+#include "hw/acpi/acpi.h"
+#include "qapi/visitor.h"
+
+#define VMGENID(obj) OBJECT_CHECK(VmGenIdState, (obj), VMGENID_DEVICE)
+
+typedef struct VmGenIdState {
+    PCIDevice parent_obj;
+    MemoryRegion iomem;
+    uint8_t guid[16];
+    bool guid_set;
+} VmGenIdState;
+
+static void vmgenid_update_guest(VmGenIdState *s)
+{
+    Object *acpi_obj;
+    void *ptr = memory_region_get_ram_ptr(&s->iomem);
+
+    memcpy(ptr, &s->guid, sizeof(s->guid));
+    memory_region_set_dirty(&s->iomem, 0, sizeof(s->guid));
+
+    acpi_obj = object_resolve_path_type("", TYPE_ACPI_DEVICE_IF, NULL);
+    if (acpi_obj) {
+        AcpiDeviceIfClass *adevc = ACPI_DEVICE_IF_GET_CLASS(acpi_obj);
+        AcpiDeviceIf *adev = ACPI_DEVICE_IF(acpi_obj);
+        ACPIREGS *acpi_regs = adevc->regs(adev);
+
+        acpi_regs->gpe.sts[0] |= 1; /* _GPE.E00 handler */
+        acpi_update_sci(acpi_regs, adevc->sci(adev));
+    }
+}
+
+static void vmgenid_set_uuid(Object *obj, const char *value, Error **errp)
+{
+    VmGenIdState *s = VMGENID(obj);
+
+    if (qemu_uuid_parse(value, s->guid) < 0) {
+        error_setg(errp, "'%s." VMGENID_UUID "': Fail to parse UUID string.",
+                   object_get_typename(OBJECT(s)));
+        return;
+    }
+
+    s->guid_set = true;
+    vmgenid_update_guest(s);
+}
+
+static void vmgenid_get_vmgid_addr(Object *obj, Visitor *v, void *opaque,
+                                   const char *name, Error **errp)
+{
+    VmGenIdState *s = VMGENID(obj);
+    int64_t value = pci_get_bar_addr(PCI_DEVICE(s), 0);
+
+    if (value == PCI_BAR_UNMAPPED) {
+        error_setg(errp, "'%s." VMGENID_VMGID_ADDR "': not initialized",
+                   object_get_typename(OBJECT(s)));
+        return;
+    }
+    visit_type_int(v, &value, name, errp);
+}
+
+static void vmgenid_initfn(Object *obj)
+{
+    VmGenIdState *s = VMGENID(obj);
+
+    memory_region_init_ram(&s->iomem, obj, "vgid.bar", sizeof(s->guid),
+                           &error_abort);
+
+    object_property_add_str(obj, VMGENID_UUID, NULL, vmgenid_set_uuid, NULL);
+    object_property_add(obj, VMGENID_VMGID_ADDR, "int", vmgenid_get_vmgid_addr,
+                        NULL, NULL, NULL, NULL);
+}
+
+
+static void vmgenid_realize(PCIDevice *dev, Error **errp)
+{
+    VmGenIdState *s = VMGENID(dev);
+
+    if (!s->guid_set) {
+        error_setg(errp, "'%s." VMGENID_UUID "' property is not set",
+                   object_get_typename(OBJECT(s)));
+        return;
+    }
+
+    vmstate_register_ram(&s->iomem, DEVICE(s));
+    pci_register_bar(PCI_DEVICE(s), 0, PCI_BASE_ADDRESS_SPACE_MEMORY, &s->iomem);
+    return;
+}
+
+static void vmgenid_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+
+    set_bit(DEVICE_CATEGORY_MISC, dc->categories);
+    dc->hotpluggable = false;
+    k->realize = vmgenid_realize;
+    k->vendor_id = PCI_VENDOR_ID_REDHAT;
+    k->device_id = PCI_DEVICE_ID_REDHAT_VMGENID;
+    k->class_id = PCI_CLASS_MEMORY_RAM;
+}
+
+static const TypeInfo vmgenid_device_info = {
+    .name          = VMGENID_DEVICE,
+    .parent        = TYPE_PCI_DEVICE,
+    .instance_size = sizeof(VmGenIdState),
+    .instance_init = vmgenid_initfn,
+    .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/acpi/acpi.h b/include/hw/acpi/acpi.h
index 1f678b4..a09cb3f 100644
--- a/include/hw/acpi/acpi.h
+++ b/include/hw/acpi/acpi.h
@@ -25,6 +25,7 @@
 #include "qemu/option.h"
 #include "exec/memory.h"
 #include "hw/irq.h"
+#include "hw/acpi/acpi_dev_interface.h"
 
 /*
  * current device naming scheme supports up to 256 memory devices
diff --git a/include/hw/misc/vmgenid.h b/include/hw/misc/vmgenid.h
new file mode 100644
index 0000000..325f095
--- /dev/null
+++ b/include/hw/misc/vmgenid.h
@@ -0,0 +1,21 @@
+/*
+ *  Virtual Machine Generation ID Device
+ *
+ *  Copyright (C) 2014 Red Hat Inc.
+ *
+ *  Authors: Gal Hammer <ghammer@redhat.com>
+ *           Igor Mammedov <imammedo@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.
+ *
+ */
+
+#ifndef HW_MISC_VMGENID_H
+#define HW_MISC_VMGENID_H
+
+#define VMGENID_DEVICE       "vmgenid"
+#define VMGENID_UUID         "uuid"
+#define VMGENID_VMGID_ADDR   "vmgid-addr"
+
+#endif
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index 3164fc3..245171b 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -90,6 +90,7 @@
 #define PCI_DEVICE_ID_REDHAT_TEST        0x0005
 #define PCI_DEVICE_ID_REDHAT_SDHCI       0x0007
 #define PCI_DEVICE_ID_REDHAT_PCIE_HOST   0x0008
+#define PCI_DEVICE_ID_REDHAT_VMGENID     0x0009
 #define PCI_DEVICE_ID_REDHAT_QXL         0x0100
 
 #define FMT_PCIBUS                      PRIx64
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH V13 3/4] pc: add a Virtual Machine Generation ID device

[Michael S. Tsirkin]

> diff --git a/hw/i386/acpi-dsdt.dsl b/hw/i386/acpi-dsdt.dsl
> index a611e07..884038b 100644
> --- a/hw/i386/acpi-dsdt.dsl
> +++ b/hw/i386/acpi-dsdt.dsl
> @@ -306,8 +306,6 @@ DefinitionBlock (
>      Scope(\_GPE) {
>          Name(_HID, "ACPI0006")
>  
> -        Method(_L00) {
> -        }
>          Method(_E01) {
>              // PCI hotplug event
>              Acquire(\_SB.PCI0.BLCK, 0xFFFF)
> diff --git a/hw/i386/q35-acpi-dsdt.dsl b/hw/i386/q35-acpi-dsdt.dsl
> index e1cee5d..9eb794c 100644
> --- a/hw/i386/q35-acpi-dsdt.dsl
> +++ b/hw/i386/q35-acpi-dsdt.dsl
> @@ -414,8 +414,6 @@ DefinitionBlock (
>      Scope(\_GPE) {
>          Name(_HID, "ACPI0006")
>  
> -        Method(_L00) {
> -        }
>          Method(_L01) {
>          }
>          Method(_E02) {

What is this change doing?

Re: [Qemu-devel] [PATCH V13 3/4] pc: add a Virtual Machine Generation ID device

[Igor Mammedov]

On Wed, 25 Feb 2015 20:59:41 +0100
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> > diff --git a/hw/i386/acpi-dsdt.dsl b/hw/i386/acpi-dsdt.dsl
> > index a611e07..884038b 100644
> > --- a/hw/i386/acpi-dsdt.dsl
> > +++ b/hw/i386/acpi-dsdt.dsl
> > @@ -306,8 +306,6 @@ DefinitionBlock (
> >      Scope(\_GPE) {
> >          Name(_HID, "ACPI0006")
> >  
> > -        Method(_L00) {
> > -        }
> >          Method(_E01) {
> >              // PCI hotplug event
> >              Acquire(\_SB.PCI0.BLCK, 0xFFFF)
> > diff --git a/hw/i386/q35-acpi-dsdt.dsl b/hw/i386/q35-acpi-dsdt.dsl
> > index e1cee5d..9eb794c 100644
> > --- a/hw/i386/q35-acpi-dsdt.dsl
> > +++ b/hw/i386/q35-acpi-dsdt.dsl
> > @@ -414,8 +414,6 @@ DefinitionBlock (
> >      Scope(\_GPE) {
> >          Name(_HID, "ACPI0006")
> >  
> > -        Method(_L00) {
> > -        }
> >          Method(_L01) {
> >          }
> >          Method(_E02) {
> 
> What is this change doing?
It's replaced by \_GPE._E00 handler in SSDT if vmgenid device is present.
Implementation MUST provide control method _[LE]xx IF hardware utilizes
corresponding GPE event [ACPI 1.0b 5.6.2.2.3].
From that follows if GPE event is not used, the corresponding method doesn't
have to be present in \_GPE block. That goes along with \_GPE blocks I've
seen on real hardware also. (i.e. they provide only some handlers and do
not implement stubs for unused ones).

I've tested it with XPsp3/ws2012R2, it boots fine with vmgenid either
present or not.

Re: [Qemu-devel] [PATCH V13 3/4] pc: add a Virtual Machine Generation ID device

[Michael S. Tsirkin]

On Wed, Feb 25, 2015 at 05:08:52PM +0000, Igor Mammedov 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 and its implementation.
> 
> The GUID is set using "vmgenid.uuid" property.
> 
> Example of using vmgenid device:
>  -device vmgenid,id=FOO,uuid="324e6eaf-d1d1-4bf6-bf41-b9bb6c91fb87"

If you do this, doesn't windows then prompt for a driver?


> To change uuid in runtime use:
> qom-set "/machine/peripheral/FOO.uuid" "124e6eaf-d1d1-4bf6-bf41-b9bb6c91fb87"
> 
> 'vmgenid' device initialization flow is as following:
>  1. vmgenid has RAM BAR resistered with size of UUID buffer
>  2. BIOS initializes PCI devices and it maps BAR in PCI hole
>  3. BIOS reads ACPI tables from QEMU, at that moment tables
>     are generated with \_SB.VMGI.ADDR constant pointing to
>     HPA where BIOS's mapped vmgenid's BAR earlier
> 
> Signed-off-by: Gal Hammer <ghammer@redhat.com>
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
> v2:
>   * rewrite to use PCIDevice so that we don't have to mess
>     with complex fwcfg/linker and patch ACPI tables then
>     read VMGENID buffer adddress in guest OSPM and communicate
>     it to QEMU via reserved MMIO region.
>     Which also allows us to write a more complete unit test
>     that wouldn't require to run OSPM so that it could update
>     HPA in QEMU.
>   * make 'vmgenid' optional, users who want to use it
>     should add -device vmgenid,.... to QEMU CLI
>     it also saves us some space in SSDT if device is not used
>   * mark UUID buffer as dirty when it's updated via QMP in runtime
>   * make 'uiid' property mandatory at -device
> ---
>  default-configs/i386-softmmu.mak   |   1 +
>  default-configs/x86_64-softmmu.mak |   1 +
>  docs/specs/pci-ids.txt             |   1 +
>  hw/i386/acpi-build.c               |  33 ++++++++++
>  hw/i386/acpi-dsdt.dsl              |   2 -
>  hw/i386/q35-acpi-dsdt.dsl          |   2 -
>  hw/misc/Makefile.objs              |   1 +
>  hw/misc/vmgenid.c                  | 130 +++++++++++++++++++++++++++++++++++++
>  include/hw/acpi/acpi.h             |   1 +
>  include/hw/misc/vmgenid.h          |  21 ++++++
>  include/hw/pci/pci.h               |   1 +
>  11 files changed, 190 insertions(+), 4 deletions(-)
>  create mode 100644 hw/misc/vmgenid.c
>  create mode 100644 include/hw/misc/vmgenid.h
> 
> diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak
> index bd99af9..0b913a8 100644
> --- a/default-configs/i386-softmmu.mak
> +++ b/default-configs/i386-softmmu.mak
> @@ -43,3 +43,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 e7c2734..de5e6af 100644
> --- a/default-configs/x86_64-softmmu.mak
> +++ b/default-configs/x86_64-softmmu.mak
> @@ -43,3 +43,4 @@ CONFIG_IOAPIC=y
>  CONFIG_ICC_BUS=y
>  CONFIG_PVPANIC=y
>  CONFIG_MEM_HOTPLUG=y
> +CONFIG_VMGENID=y
> diff --git a/docs/specs/pci-ids.txt b/docs/specs/pci-ids.txt
> index c6732fe..b398c5d 100644
> --- a/docs/specs/pci-ids.txt
> +++ b/docs/specs/pci-ids.txt
> @@ -46,6 +46,7 @@ PCI devices (other than virtio):
>  1b36:0004  PCI Quad-port 16550A adapter (docs/specs/pci-serial.txt)
>  1b36:0005  PCI test device (docs/specs/pci-testdev.txt)
>  1b36:0007  PCI SD Card Host Controller Interface (SDHCI)
> +1b36:0009  PCI VM-Generation device
>  
>  All these devices are documented in docs/specs.
>  
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 4d5d7e3..9290ae3 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -50,6 +50,7 @@
>  #include "hw/pci/pci_bus.h"
>  #include "hw/pci-host/q35.h"
>  #include "hw/i386/intel_iommu.h"
> +#include "hw/misc/vmgenid.h"
>  
>  #include "hw/i386/q35-acpi-dsdt.hex"
>  #include "hw/i386/acpi-dsdt.hex"
> @@ -110,6 +111,7 @@ typedef struct AcpiPmInfo {
>  } AcpiPmInfo;
>  
>  typedef struct AcpiMiscInfo {
> +    uint32_t vmgen_buf_paddr;
>      bool has_hpet;
>      bool has_tpm;
>      DECLARE_BITMAP(slot_hotplug_enable, PCI_SLOT_MAX);
> @@ -245,6 +247,14 @@ static void acpi_get_pm_info(AcpiPmInfo *pm)
>  
>  static void acpi_get_misc_info(AcpiMiscInfo *info)
>  {
> +    Object *obj;
> +
> +    obj = object_resolve_path_type("", VMGENID_DEVICE, NULL);
> +    info->vmgen_buf_paddr = 0;
> +    if (obj) {
> +        info->vmgen_buf_paddr =
> +            object_property_get_int(obj, VMGENID_VMGID_ADDR, NULL);
> +    }
>      info->has_hpet = hpet_find();
>      info->has_tpm = tpm_find();
>      info->pvpanic_port = pvpanic_port();
> @@ -981,6 +991,29 @@ build_ssdt(GArray *table_data, GArray *linker,
>  
>      sb_scope = aml_scope("_SB");
>      {
> +        if (misc->vmgen_buf_paddr) {
> +            dev = aml_device("VMGI");
> +
> +            aml_append(dev, aml_name_decl("_HID", aml_string("QEMU0003")));
> +            aml_append(dev, aml_name_decl("_CID", aml_string("VM_Gen_Counter")));
> +            aml_append(dev, aml_name_decl("_DDN", aml_string("VM_Gen_Counter")));
> +
> +            pkg = aml_package(2);
> +            /* low 32 bits of UUID buffer addr*/
> +            aml_append(pkg, aml_int(misc->vmgen_buf_paddr));
> +            aml_append(pkg, aml_int(0)); /* high 32 bits of UUID buffer addr */

Really should be full 64 bit, and use a 64 bit BAR.


> +            aml_append(dev, aml_name_decl("ADDR", pkg));
> +
> +            aml_append(sb_scope, dev);
> +
> +            scope = aml_scope("\\_GPE");
> +            method = aml_method("_E00", 0);
> +            aml_append(method,
> +                aml_notify(aml_name("\\_SB.VMGI"), aml_int(0x80)));
> +            aml_append(scope, method);
> +            aml_append(ssdt, scope);
> +        }
> +

This confuses me.
Shouldn't VMGI device be on PCI bus, where it was
added?



>          /* create PCI0.PRES device and its _CRS to reserve CPU hotplug MMIO */
>          dev = aml_device("PCI0." stringify(CPU_HOTPLUG_RESOURCE_DEVICE));
>          aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A06")));
> diff --git a/hw/i386/acpi-dsdt.dsl b/hw/i386/acpi-dsdt.dsl
> index a611e07..884038b 100644
> --- a/hw/i386/acpi-dsdt.dsl
> +++ b/hw/i386/acpi-dsdt.dsl
> @@ -306,8 +306,6 @@ DefinitionBlock (
>      Scope(\_GPE) {
>          Name(_HID, "ACPI0006")
>  
> -        Method(_L00) {
> -        }
>          Method(_E01) {
>              // PCI hotplug event
>              Acquire(\_SB.PCI0.BLCK, 0xFFFF)
> diff --git a/hw/i386/q35-acpi-dsdt.dsl b/hw/i386/q35-acpi-dsdt.dsl
> index e1cee5d..9eb794c 100644
> --- a/hw/i386/q35-acpi-dsdt.dsl
> +++ b/hw/i386/q35-acpi-dsdt.dsl
> @@ -414,8 +414,6 @@ DefinitionBlock (
>      Scope(\_GPE) {
>          Name(_HID, "ACPI0006")
>  
> -        Method(_L00) {
> -        }
>          Method(_L01) {
>          }
>          Method(_E02) {
> diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
> index 029a56f..e047aea 100644
> --- a/hw/misc/Makefile.objs
> +++ b/hw/misc/Makefile.objs
> @@ -41,3 +41,4 @@ obj-$(CONFIG_ZYNQ) += zynq_slcr.o
>  
>  obj-$(CONFIG_PVPANIC) += pvpanic.o
>  obj-$(CONFIG_EDU) += edu.o
> +obj-$(CONFIG_VMGENID) += vmgenid.o
> diff --git a/hw/misc/vmgenid.c b/hw/misc/vmgenid.c
> new file mode 100644
> index 0000000..631c9a3
> --- /dev/null
> +++ b/hw/misc/vmgenid.c
> @@ -0,0 +1,130 @@
> +/*
> + *  Virtual Machine Generation ID Device
> + *
> + *  Copyright (C) 2014 Red Hat Inc.
> + *
> + *  Authors: Gal Hammer <ghammer@redhat.com>
> + *           Igor Mammedov <imammedo@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/pci/pci.h"
> +#include "hw/misc/vmgenid.h"
> +#include "hw/acpi/acpi.h"
> +#include "qapi/visitor.h"
> +
> +#define VMGENID(obj) OBJECT_CHECK(VmGenIdState, (obj), VMGENID_DEVICE)
> +
> +typedef struct VmGenIdState {
> +    PCIDevice parent_obj;
> +    MemoryRegion iomem;
> +    uint8_t guid[16];
> +    bool guid_set;
> +} VmGenIdState;
> +
> +static void vmgenid_update_guest(VmGenIdState *s)
> +{
> +    Object *acpi_obj;
> +    void *ptr = memory_region_get_ram_ptr(&s->iomem);
> +
> +    memcpy(ptr, &s->guid, sizeof(s->guid));
> +    memory_region_set_dirty(&s->iomem, 0, sizeof(s->guid));
> +
> +    acpi_obj = object_resolve_path_type("", TYPE_ACPI_DEVICE_IF, NULL);
> +    if (acpi_obj) {
> +        AcpiDeviceIfClass *adevc = ACPI_DEVICE_IF_GET_CLASS(acpi_obj);
> +        AcpiDeviceIf *adev = ACPI_DEVICE_IF(acpi_obj);
> +        ACPIREGS *acpi_regs = adevc->regs(adev);
> +
> +        acpi_regs->gpe.sts[0] |= 1; /* _GPE.E00 handler */
> +        acpi_update_sci(acpi_regs, adevc->sci(adev));
> +    }
> +}
> +
> +static void vmgenid_set_uuid(Object *obj, const char *value, Error **errp)
> +{
> +    VmGenIdState *s = VMGENID(obj);
> +
> +    if (qemu_uuid_parse(value, s->guid) < 0) {
> +        error_setg(errp, "'%s." VMGENID_UUID "': Fail to parse UUID string.",
> +                   object_get_typename(OBJECT(s)));
> +        return;
> +    }
> +
> +    s->guid_set = true;
> +    vmgenid_update_guest(s);
> +}
> +
> +static void vmgenid_get_vmgid_addr(Object *obj, Visitor *v, void *opaque,
> +                                   const char *name, Error **errp)
> +{
> +    VmGenIdState *s = VMGENID(obj);
> +    int64_t value = pci_get_bar_addr(PCI_DEVICE(s), 0);
> +
> +    if (value == PCI_BAR_UNMAPPED) {
> +        error_setg(errp, "'%s." VMGENID_VMGID_ADDR "': not initialized",
> +                   object_get_typename(OBJECT(s)));

Looks wrong - this is guest configuration.
I don't think we want to print errors and exit in this way just
because guest did not map the device.
Better to mask it in acpi.


> +        return;
> +    }
> +    visit_type_int(v, &value, name, errp);
> +}
> +
> +static void vmgenid_initfn(Object *obj)
> +{
> +    VmGenIdState *s = VMGENID(obj);
> +
> +    memory_region_init_ram(&s->iomem, obj, "vgid.bar", sizeof(s->guid),
> +                           &error_abort);
> +
> +    object_property_add_str(obj, VMGENID_UUID, NULL, vmgenid_set_uuid, NULL);
> +    object_property_add(obj, VMGENID_VMGID_ADDR, "int", vmgenid_get_vmgid_addr,
> +                        NULL, NULL, NULL, NULL);
> +}
> +
> +
> +static void vmgenid_realize(PCIDevice *dev, Error **errp)
> +{
> +    VmGenIdState *s = VMGENID(dev);
> +
> +    if (!s->guid_set) {
> +        error_setg(errp, "'%s." VMGENID_UUID "' property is not set",
> +                   object_get_typename(OBJECT(s)));
> +        return;
> +    }
> +
> +    vmstate_register_ram(&s->iomem, DEVICE(s));
> +    pci_register_bar(PCI_DEVICE(s), 0, PCI_BASE_ADDRESS_SPACE_MEMORY, &s->iomem);

This means this BAR is 16 bytes in size?

Not good, should be full 4K.

You also make the BAR non prefetcheable, which makes
some OS-es (e.g. old linux) map it non-cacheable.






> +    return;
> +}
> +
> +static void vmgenid_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
> +
> +    set_bit(DEVICE_CATEGORY_MISC, dc->categories);
> +    dc->hotpluggable = false;
> +    k->realize = vmgenid_realize;
> +    k->vendor_id = PCI_VENDOR_ID_REDHAT;
> +    k->device_id = PCI_DEVICE_ID_REDHAT_VMGENID;
> +    k->class_id = PCI_CLASS_MEMORY_RAM;
> +}
> +
> +static const TypeInfo vmgenid_device_info = {
> +    .name          = VMGENID_DEVICE,
> +    .parent        = TYPE_PCI_DEVICE,
> +    .instance_size = sizeof(VmGenIdState),
> +    .instance_init = vmgenid_initfn,
> +    .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/acpi/acpi.h b/include/hw/acpi/acpi.h
> index 1f678b4..a09cb3f 100644
> --- a/include/hw/acpi/acpi.h
> +++ b/include/hw/acpi/acpi.h
> @@ -25,6 +25,7 @@
>  #include "qemu/option.h"
>  #include "exec/memory.h"
>  #include "hw/irq.h"
> +#include "hw/acpi/acpi_dev_interface.h"
>  
>  /*
>   * current device naming scheme supports up to 256 memory devices
> diff --git a/include/hw/misc/vmgenid.h b/include/hw/misc/vmgenid.h
> new file mode 100644
> index 0000000..325f095
> --- /dev/null
> +++ b/include/hw/misc/vmgenid.h
> @@ -0,0 +1,21 @@
> +/*
> + *  Virtual Machine Generation ID Device
> + *
> + *  Copyright (C) 2014 Red Hat Inc.
> + *
> + *  Authors: Gal Hammer <ghammer@redhat.com>
> + *           Igor Mammedov <imammedo@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.
> + *
> + */
> +
> +#ifndef HW_MISC_VMGENID_H
> +#define HW_MISC_VMGENID_H
> +
> +#define VMGENID_DEVICE       "vmgenid"
> +#define VMGENID_UUID         "uuid"
> +#define VMGENID_VMGID_ADDR   "vmgid-addr"
> +
> +#endif
> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> index 3164fc3..245171b 100644
> --- a/include/hw/pci/pci.h
> +++ b/include/hw/pci/pci.h
> @@ -90,6 +90,7 @@
>  #define PCI_DEVICE_ID_REDHAT_TEST        0x0005
>  #define PCI_DEVICE_ID_REDHAT_SDHCI       0x0007
>  #define PCI_DEVICE_ID_REDHAT_PCIE_HOST   0x0008
> +#define PCI_DEVICE_ID_REDHAT_VMGENID     0x0009
>  #define PCI_DEVICE_ID_REDHAT_QXL         0x0100
>  
>  #define FMT_PCIBUS                      PRIx64
> -- 
> 1.8.3.1

Re: [Qemu-devel] [PATCH V13 3/4] pc: add a Virtual Machine Generation ID device

[Igor Mammedov]

On Sun, 1 Mar 2015 16:09:33 +0100
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Wed, Feb 25, 2015 at 05:08:52PM +0000, Igor Mammedov 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 and its implementation.
> > 
> > The GUID is set using "vmgenid.uuid" property.
> > 
> > Example of using vmgenid device:
> >  -device vmgenid,id=FOO,uuid="324e6eaf-d1d1-4bf6-bf41-b9bb6c91fb87"
> 
> If you do this, doesn't windows then prompt for a driver?
it doesn't since PCI_CLASS_MEMORY_RAM is displayed as driver less 
"PCI standard RAM Controller" binding in device manager.

There was an issue with
 virtio balloon device + pseries firmware + kernel bug
 http://lists.gnu.org/archive/html/qemu-devel/2012-03/msg04704.html
but it shouldn't be an issue for x86 targets with which device is
supposed to be used.
CCing David and Laszlo in case UEFI might do some crazy stuff like
pseries firmware.

> 
> 
> > To change uuid in runtime use:
> > qom-set "/machine/peripheral/FOO.uuid" "124e6eaf-d1d1-4bf6-bf41-b9bb6c91fb87"
> > 
> > 'vmgenid' device initialization flow is as following:
> >  1. vmgenid has RAM BAR resistered with size of UUID buffer
> >  2. BIOS initializes PCI devices and it maps BAR in PCI hole
> >  3. BIOS reads ACPI tables from QEMU, at that moment tables
> >     are generated with \_SB.VMGI.ADDR constant pointing to
> >     HPA where BIOS's mapped vmgenid's BAR earlier
> > 
> > Signed-off-by: Gal Hammer <ghammer@redhat.com>
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > ---
> > v2:
> >   * rewrite to use PCIDevice so that we don't have to mess
> >     with complex fwcfg/linker and patch ACPI tables then
> >     read VMGENID buffer adddress in guest OSPM and communicate
> >     it to QEMU via reserved MMIO region.
> >     Which also allows us to write a more complete unit test
> >     that wouldn't require to run OSPM so that it could update
> >     HPA in QEMU.
> >   * make 'vmgenid' optional, users who want to use it
> >     should add -device vmgenid,.... to QEMU CLI
> >     it also saves us some space in SSDT if device is not used
> >   * mark UUID buffer as dirty when it's updated via QMP in runtime
> >   * make 'uiid' property mandatory at -device
> > ---
> >  default-configs/i386-softmmu.mak   |   1 +
> >  default-configs/x86_64-softmmu.mak |   1 +
> >  docs/specs/pci-ids.txt             |   1 +
> >  hw/i386/acpi-build.c               |  33 ++++++++++
> >  hw/i386/acpi-dsdt.dsl              |   2 -
> >  hw/i386/q35-acpi-dsdt.dsl          |   2 -
> >  hw/misc/Makefile.objs              |   1 +
> >  hw/misc/vmgenid.c                  | 130 +++++++++++++++++++++++++++++++++++++
> >  include/hw/acpi/acpi.h             |   1 +
> >  include/hw/misc/vmgenid.h          |  21 ++++++
> >  include/hw/pci/pci.h               |   1 +
> >  11 files changed, 190 insertions(+), 4 deletions(-)
> >  create mode 100644 hw/misc/vmgenid.c
> >  create mode 100644 include/hw/misc/vmgenid.h
> > 
> > diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak
> > index bd99af9..0b913a8 100644
> > --- a/default-configs/i386-softmmu.mak
> > +++ b/default-configs/i386-softmmu.mak
> > @@ -43,3 +43,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 e7c2734..de5e6af 100644
> > --- a/default-configs/x86_64-softmmu.mak
> > +++ b/default-configs/x86_64-softmmu.mak
> > @@ -43,3 +43,4 @@ CONFIG_IOAPIC=y
> >  CONFIG_ICC_BUS=y
> >  CONFIG_PVPANIC=y
> >  CONFIG_MEM_HOTPLUG=y
> > +CONFIG_VMGENID=y
> > diff --git a/docs/specs/pci-ids.txt b/docs/specs/pci-ids.txt
> > index c6732fe..b398c5d 100644
> > --- a/docs/specs/pci-ids.txt
> > +++ b/docs/specs/pci-ids.txt
> > @@ -46,6 +46,7 @@ PCI devices (other than virtio):
> >  1b36:0004  PCI Quad-port 16550A adapter (docs/specs/pci-serial.txt)
> >  1b36:0005  PCI test device (docs/specs/pci-testdev.txt)
> >  1b36:0007  PCI SD Card Host Controller Interface (SDHCI)
> > +1b36:0009  PCI VM-Generation device
> >  
> >  All these devices are documented in docs/specs.
> >  
> > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> > index 4d5d7e3..9290ae3 100644
> > --- a/hw/i386/acpi-build.c
> > +++ b/hw/i386/acpi-build.c
> > @@ -50,6 +50,7 @@
> >  #include "hw/pci/pci_bus.h"
> >  #include "hw/pci-host/q35.h"
> >  #include "hw/i386/intel_iommu.h"
> > +#include "hw/misc/vmgenid.h"
> >  
> >  #include "hw/i386/q35-acpi-dsdt.hex"
> >  #include "hw/i386/acpi-dsdt.hex"
> > @@ -110,6 +111,7 @@ typedef struct AcpiPmInfo {
> >  } AcpiPmInfo;
> >  
> >  typedef struct AcpiMiscInfo {
> > +    uint32_t vmgen_buf_paddr;
> >      bool has_hpet;
> >      bool has_tpm;
> >      DECLARE_BITMAP(slot_hotplug_enable, PCI_SLOT_MAX);
> > @@ -245,6 +247,14 @@ static void acpi_get_pm_info(AcpiPmInfo *pm)
> >  
> >  static void acpi_get_misc_info(AcpiMiscInfo *info)
> >  {
> > +    Object *obj;
> > +
> > +    obj = object_resolve_path_type("", VMGENID_DEVICE, NULL);
> > +    info->vmgen_buf_paddr = 0;
> > +    if (obj) {
> > +        info->vmgen_buf_paddr =
> > +            object_property_get_int(obj, VMGENID_VMGID_ADDR, NULL);
> > +    }
> >      info->has_hpet = hpet_find();
> >      info->has_tpm = tpm_find();
> >      info->pvpanic_port = pvpanic_port();
> > @@ -981,6 +991,29 @@ build_ssdt(GArray *table_data, GArray *linker,
> >  
> >      sb_scope = aml_scope("_SB");
> >      {
> > +        if (misc->vmgen_buf_paddr) {
> > +            dev = aml_device("VMGI");
> > +
> > +            aml_append(dev, aml_name_decl("_HID", aml_string("QEMU0003")));
> > +            aml_append(dev, aml_name_decl("_CID", aml_string("VM_Gen_Counter")));
> > +            aml_append(dev, aml_name_decl("_DDN", aml_string("VM_Gen_Counter")));
> > +
> > +            pkg = aml_package(2);
> > +            /* low 32 bits of UUID buffer addr*/
> > +            aml_append(pkg, aml_int(misc->vmgen_buf_paddr));
> > +            aml_append(pkg, aml_int(0)); /* high 32 bits of UUID buffer addr */
> 
> Really should be full 64 bit, and use a 64 bit BAR.
for compatibility with 32 windows guest, it probably should stay below
4Gb address space.

> 
> 
> > +            aml_append(dev, aml_name_decl("ADDR", pkg));
> > +
> > +            aml_append(sb_scope, dev);
> > +
> > +            scope = aml_scope("\\_GPE");
> > +            method = aml_method("_E00", 0);
> > +            aml_append(method,
> > +                aml_notify(aml_name("\\_SB.VMGI"), aml_int(0x80)));
> > +            aml_append(scope, method);
> > +            aml_append(ssdt, scope);
> > +        }
> > +
> 
> This confuses me.
> Shouldn't VMGI device be on PCI bus, where it was
> added?
I'll try to merge it with PCI slot description that associated with this device,
Though I'm not sure that windows would handle it correctly.

> 
> 
> 
> >          /* create PCI0.PRES device and its _CRS to reserve CPU hotplug MMIO */
> >          dev = aml_device("PCI0." stringify(CPU_HOTPLUG_RESOURCE_DEVICE));
> >          aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A06")));
> > diff --git a/hw/i386/acpi-dsdt.dsl b/hw/i386/acpi-dsdt.dsl
> > index a611e07..884038b 100644
> > --- a/hw/i386/acpi-dsdt.dsl
> > +++ b/hw/i386/acpi-dsdt.dsl
> > @@ -306,8 +306,6 @@ DefinitionBlock (
> >      Scope(\_GPE) {
> >          Name(_HID, "ACPI0006")
> >  
> > -        Method(_L00) {
> > -        }
> >          Method(_E01) {
> >              // PCI hotplug event
> >              Acquire(\_SB.PCI0.BLCK, 0xFFFF)
> > diff --git a/hw/i386/q35-acpi-dsdt.dsl b/hw/i386/q35-acpi-dsdt.dsl
> > index e1cee5d..9eb794c 100644
> > --- a/hw/i386/q35-acpi-dsdt.dsl
> > +++ b/hw/i386/q35-acpi-dsdt.dsl
> > @@ -414,8 +414,6 @@ DefinitionBlock (
> >      Scope(\_GPE) {
> >          Name(_HID, "ACPI0006")
> >  
> > -        Method(_L00) {
> > -        }
> >          Method(_L01) {
> >          }
> >          Method(_E02) {
> > diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
> > index 029a56f..e047aea 100644
> > --- a/hw/misc/Makefile.objs
> > +++ b/hw/misc/Makefile.objs
> > @@ -41,3 +41,4 @@ obj-$(CONFIG_ZYNQ) += zynq_slcr.o
> >  
> >  obj-$(CONFIG_PVPANIC) += pvpanic.o
> >  obj-$(CONFIG_EDU) += edu.o
> > +obj-$(CONFIG_VMGENID) += vmgenid.o
> > diff --git a/hw/misc/vmgenid.c b/hw/misc/vmgenid.c
> > new file mode 100644
> > index 0000000..631c9a3
> > --- /dev/null
> > +++ b/hw/misc/vmgenid.c
> > @@ -0,0 +1,130 @@
> > +/*
> > + *  Virtual Machine Generation ID Device
> > + *
> > + *  Copyright (C) 2014 Red Hat Inc.
> > + *
> > + *  Authors: Gal Hammer <ghammer@redhat.com>
> > + *           Igor Mammedov <imammedo@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/pci/pci.h"
> > +#include "hw/misc/vmgenid.h"
> > +#include "hw/acpi/acpi.h"
> > +#include "qapi/visitor.h"
> > +
> > +#define VMGENID(obj) OBJECT_CHECK(VmGenIdState, (obj), VMGENID_DEVICE)
> > +
> > +typedef struct VmGenIdState {
> > +    PCIDevice parent_obj;
> > +    MemoryRegion iomem;
> > +    uint8_t guid[16];
> > +    bool guid_set;
> > +} VmGenIdState;
> > +
> > +static void vmgenid_update_guest(VmGenIdState *s)
> > +{
> > +    Object *acpi_obj;
> > +    void *ptr = memory_region_get_ram_ptr(&s->iomem);
> > +
> > +    memcpy(ptr, &s->guid, sizeof(s->guid));
> > +    memory_region_set_dirty(&s->iomem, 0, sizeof(s->guid));
> > +
> > +    acpi_obj = object_resolve_path_type("", TYPE_ACPI_DEVICE_IF, NULL);
> > +    if (acpi_obj) {
> > +        AcpiDeviceIfClass *adevc = ACPI_DEVICE_IF_GET_CLASS(acpi_obj);
> > +        AcpiDeviceIf *adev = ACPI_DEVICE_IF(acpi_obj);
> > +        ACPIREGS *acpi_regs = adevc->regs(adev);
> > +
> > +        acpi_regs->gpe.sts[0] |= 1; /* _GPE.E00 handler */
> > +        acpi_update_sci(acpi_regs, adevc->sci(adev));
> > +    }
> > +}
> > +
> > +static void vmgenid_set_uuid(Object *obj, const char *value, Error **errp)
> > +{
> > +    VmGenIdState *s = VMGENID(obj);
> > +
> > +    if (qemu_uuid_parse(value, s->guid) < 0) {
> > +        error_setg(errp, "'%s." VMGENID_UUID "': Fail to parse UUID string.",
> > +                   object_get_typename(OBJECT(s)));
> > +        return;
> > +    }
> > +
> > +    s->guid_set = true;
> > +    vmgenid_update_guest(s);
> > +}
> > +
> > +static void vmgenid_get_vmgid_addr(Object *obj, Visitor *v, void *opaque,
> > +                                   const char *name, Error **errp)
> > +{
> > +    VmGenIdState *s = VMGENID(obj);
> > +    int64_t value = pci_get_bar_addr(PCI_DEVICE(s), 0);
> > +
> > +    if (value == PCI_BAR_UNMAPPED) {
> > +        error_setg(errp, "'%s." VMGENID_VMGID_ADDR "': not initialized",
> > +                   object_get_typename(OBJECT(s)));
> 
> Looks wrong - this is guest configuration.
> I don't think we want to print errors and exit in this way just
> because guest did not map the device.
> Better to mask it in acpi.
that would mean that user asked to run guest with VMGID but BIOS failed
to do so for some reason and error was just ignored.
I think it's better to abort guest at early start-up stage
i.e. when it loads ACPI tables, than proceed with invalid state
silently.

> 
> 
> > +        return;
> > +    }
> > +    visit_type_int(v, &value, name, errp);
> > +}
> > +
> > +static void vmgenid_initfn(Object *obj)
> > +{
> > +    VmGenIdState *s = VMGENID(obj);
> > +
> > +    memory_region_init_ram(&s->iomem, obj, "vgid.bar", sizeof(s->guid),
> > +                           &error_abort);
> > +
> > +    object_property_add_str(obj, VMGENID_UUID, NULL, vmgenid_set_uuid, NULL);
> > +    object_property_add(obj, VMGENID_VMGID_ADDR, "int", vmgenid_get_vmgid_addr,
> > +                        NULL, NULL, NULL, NULL);
> > +}
> > +
> > +
> > +static void vmgenid_realize(PCIDevice *dev, Error **errp)
> > +{
> > +    VmGenIdState *s = VMGENID(dev);
> > +
> > +    if (!s->guid_set) {
> > +        error_setg(errp, "'%s." VMGENID_UUID "' property is not set",
> > +                   object_get_typename(OBJECT(s)));
> > +        return;
> > +    }
> > +
> > +    vmstate_register_ram(&s->iomem, DEVICE(s));
> > +    pci_register_bar(PCI_DEVICE(s), 0, PCI_BASE_ADDRESS_SPACE_MEMORY, &s->iomem);
> 
> This means this BAR is 16 bytes in size?
> 
> Not good, should be full 4K.
yep, it should be page

> 
> You also make the BAR non prefetcheable, which makes
> some OS-es (e.g. old linux) map it non-cacheable.
we don't care about old linux here because it doesn't have
vmgenid driver neither a modern linux but at least it could
be added to it.
But there shouldn't be a issue with tagging it as prefetchable.


> 
> 
> 
> 
> 
> 
> > +    return;
> > +}
> > +
> > +static void vmgenid_class_init(ObjectClass *klass, void *data)
> > +{
> > +    DeviceClass *dc = DEVICE_CLASS(klass);
> > +    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
> > +
> > +    set_bit(DEVICE_CATEGORY_MISC, dc->categories);
> > +    dc->hotpluggable = false;
> > +    k->realize = vmgenid_realize;
> > +    k->vendor_id = PCI_VENDOR_ID_REDHAT;
> > +    k->device_id = PCI_DEVICE_ID_REDHAT_VMGENID;
> > +    k->class_id = PCI_CLASS_MEMORY_RAM;
> > +}
> > +
> > +static const TypeInfo vmgenid_device_info = {
> > +    .name          = VMGENID_DEVICE,
> > +    .parent        = TYPE_PCI_DEVICE,
> > +    .instance_size = sizeof(VmGenIdState),
> > +    .instance_init = vmgenid_initfn,
> > +    .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/acpi/acpi.h b/include/hw/acpi/acpi.h
> > index 1f678b4..a09cb3f 100644
> > --- a/include/hw/acpi/acpi.h
> > +++ b/include/hw/acpi/acpi.h
> > @@ -25,6 +25,7 @@
> >  #include "qemu/option.h"
> >  #include "exec/memory.h"
> >  #include "hw/irq.h"
> > +#include "hw/acpi/acpi_dev_interface.h"
> >  
> >  /*
> >   * current device naming scheme supports up to 256 memory devices
> > diff --git a/include/hw/misc/vmgenid.h b/include/hw/misc/vmgenid.h
> > new file mode 100644
> > index 0000000..325f095
> > --- /dev/null
> > +++ b/include/hw/misc/vmgenid.h
> > @@ -0,0 +1,21 @@
> > +/*
> > + *  Virtual Machine Generation ID Device
> > + *
> > + *  Copyright (C) 2014 Red Hat Inc.
> > + *
> > + *  Authors: Gal Hammer <ghammer@redhat.com>
> > + *           Igor Mammedov <imammedo@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.
> > + *
> > + */
> > +
> > +#ifndef HW_MISC_VMGENID_H
> > +#define HW_MISC_VMGENID_H
> > +
> > +#define VMGENID_DEVICE       "vmgenid"
> > +#define VMGENID_UUID         "uuid"
> > +#define VMGENID_VMGID_ADDR   "vmgid-addr"
> > +
> > +#endif
> > diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> > index 3164fc3..245171b 100644
> > --- a/include/hw/pci/pci.h
> > +++ b/include/hw/pci/pci.h
> > @@ -90,6 +90,7 @@
> >  #define PCI_DEVICE_ID_REDHAT_TEST        0x0005
> >  #define PCI_DEVICE_ID_REDHAT_SDHCI       0x0007
> >  #define PCI_DEVICE_ID_REDHAT_PCIE_HOST   0x0008
> > +#define PCI_DEVICE_ID_REDHAT_VMGENID     0x0009
> >  #define PCI_DEVICE_ID_REDHAT_QXL         0x0100
> >  
> >  #define FMT_PCIBUS                      PRIx64
> > -- 
> > 1.8.3.1
> 

Re: [Qemu-devel] [PATCH V13 3/4] pc: add a Virtual Machine Generation ID device

[Laszlo Ersek]

On 03/02/15 18:05, Igor Mammedov wrote:
> On Sun, 1 Mar 2015 16:09:33 +0100
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
> 
>> On Wed, Feb 25, 2015 at 05:08:52PM +0000, Igor Mammedov 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 and its implementation.
>>>
>>> The GUID is set using "vmgenid.uuid" property.
>>>
>>> Example of using vmgenid device:
>>>  -device vmgenid,id=FOO,uuid="324e6eaf-d1d1-4bf6-bf41-b9bb6c91fb87"
>>
>> If you do this, doesn't windows then prompt for a driver?
> it doesn't since PCI_CLASS_MEMORY_RAM is displayed as driver less 
> "PCI standard RAM Controller" binding in device manager.
> 
> There was an issue with
>  virtio balloon device + pseries firmware + kernel bug
>  http://lists.gnu.org/archive/html/qemu-devel/2012-03/msg04704.html
> but it shouldn't be an issue for x86 targets with which device is
> supposed to be used.
> CCing David and Laszlo in case UEFI might do some crazy stuff like
> pseries firmware.

UEFI doing crazy stuff? Never. :)

But, I need more context. Is this a new PCI device with class code
PCI_CLASS_MEMORY_RAM? I grepped edk2 for it, and for
PCI_CLASS_MEMORY_CONTROLLER too. Only the macro definitions are there, I
don't see them used anywhere.

If I wanted to add different kinds of RAM to the system, I'd have to
produce new memory resource descriptor HOBs (hand-off blocks) during
PEI, or call gDS->AddMemorySpace() during DXE. Since I won't do that, I
expect the addition of just another PCI device to QEMU will simply go
unnoticed in OVMF (with there being no particular driver for it in OVMF).

So, unless I misunderstood something, this should be safe to do in qemu.

Thanks
Laszlo

> 
>>
>>
>>> To change uuid in runtime use:
>>> qom-set "/machine/peripheral/FOO.uuid" "124e6eaf-d1d1-4bf6-bf41-b9bb6c91fb87"
>>>
>>> 'vmgenid' device initialization flow is as following:
>>>  1. vmgenid has RAM BAR resistered with size of UUID buffer
>>>  2. BIOS initializes PCI devices and it maps BAR in PCI hole
>>>  3. BIOS reads ACPI tables from QEMU, at that moment tables
>>>     are generated with \_SB.VMGI.ADDR constant pointing to
>>>     HPA where BIOS's mapped vmgenid's BAR earlier
>>>
>>> Signed-off-by: Gal Hammer <ghammer@redhat.com>
>>> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
>>> ---
>>> v2:
>>>   * rewrite to use PCIDevice so that we don't have to mess
>>>     with complex fwcfg/linker and patch ACPI tables then
>>>     read VMGENID buffer adddress in guest OSPM and communicate
>>>     it to QEMU via reserved MMIO region.
>>>     Which also allows us to write a more complete unit test
>>>     that wouldn't require to run OSPM so that it could update
>>>     HPA in QEMU.
>>>   * make 'vmgenid' optional, users who want to use it
>>>     should add -device vmgenid,.... to QEMU CLI
>>>     it also saves us some space in SSDT if device is not used
>>>   * mark UUID buffer as dirty when it's updated via QMP in runtime
>>>   * make 'uiid' property mandatory at -device
>>> ---
>>>  default-configs/i386-softmmu.mak   |   1 +
>>>  default-configs/x86_64-softmmu.mak |   1 +
>>>  docs/specs/pci-ids.txt             |   1 +
>>>  hw/i386/acpi-build.c               |  33 ++++++++++
>>>  hw/i386/acpi-dsdt.dsl              |   2 -
>>>  hw/i386/q35-acpi-dsdt.dsl          |   2 -
>>>  hw/misc/Makefile.objs              |   1 +
>>>  hw/misc/vmgenid.c                  | 130 +++++++++++++++++++++++++++++++++++++
>>>  include/hw/acpi/acpi.h             |   1 +
>>>  include/hw/misc/vmgenid.h          |  21 ++++++
>>>  include/hw/pci/pci.h               |   1 +
>>>  11 files changed, 190 insertions(+), 4 deletions(-)
>>>  create mode 100644 hw/misc/vmgenid.c
>>>  create mode 100644 include/hw/misc/vmgenid.h
>>>
>>> diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak
>>> index bd99af9..0b913a8 100644
>>> --- a/default-configs/i386-softmmu.mak
>>> +++ b/default-configs/i386-softmmu.mak
>>> @@ -43,3 +43,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 e7c2734..de5e6af 100644
>>> --- a/default-configs/x86_64-softmmu.mak
>>> +++ b/default-configs/x86_64-softmmu.mak
>>> @@ -43,3 +43,4 @@ CONFIG_IOAPIC=y
>>>  CONFIG_ICC_BUS=y
>>>  CONFIG_PVPANIC=y
>>>  CONFIG_MEM_HOTPLUG=y
>>> +CONFIG_VMGENID=y
>>> diff --git a/docs/specs/pci-ids.txt b/docs/specs/pci-ids.txt
>>> index c6732fe..b398c5d 100644
>>> --- a/docs/specs/pci-ids.txt
>>> +++ b/docs/specs/pci-ids.txt
>>> @@ -46,6 +46,7 @@ PCI devices (other than virtio):
>>>  1b36:0004  PCI Quad-port 16550A adapter (docs/specs/pci-serial.txt)
>>>  1b36:0005  PCI test device (docs/specs/pci-testdev.txt)
>>>  1b36:0007  PCI SD Card Host Controller Interface (SDHCI)
>>> +1b36:0009  PCI VM-Generation device
>>>  
>>>  All these devices are documented in docs/specs.
>>>  
>>> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
>>> index 4d5d7e3..9290ae3 100644
>>> --- a/hw/i386/acpi-build.c
>>> +++ b/hw/i386/acpi-build.c
>>> @@ -50,6 +50,7 @@
>>>  #include "hw/pci/pci_bus.h"
>>>  #include "hw/pci-host/q35.h"
>>>  #include "hw/i386/intel_iommu.h"
>>> +#include "hw/misc/vmgenid.h"
>>>  
>>>  #include "hw/i386/q35-acpi-dsdt.hex"
>>>  #include "hw/i386/acpi-dsdt.hex"
>>> @@ -110,6 +111,7 @@ typedef struct AcpiPmInfo {
>>>  } AcpiPmInfo;
>>>  
>>>  typedef struct AcpiMiscInfo {
>>> +    uint32_t vmgen_buf_paddr;
>>>      bool has_hpet;
>>>      bool has_tpm;
>>>      DECLARE_BITMAP(slot_hotplug_enable, PCI_SLOT_MAX);
>>> @@ -245,6 +247,14 @@ static void acpi_get_pm_info(AcpiPmInfo *pm)
>>>  
>>>  static void acpi_get_misc_info(AcpiMiscInfo *info)
>>>  {
>>> +    Object *obj;
>>> +
>>> +    obj = object_resolve_path_type("", VMGENID_DEVICE, NULL);
>>> +    info->vmgen_buf_paddr = 0;
>>> +    if (obj) {
>>> +        info->vmgen_buf_paddr =
>>> +            object_property_get_int(obj, VMGENID_VMGID_ADDR, NULL);
>>> +    }
>>>      info->has_hpet = hpet_find();
>>>      info->has_tpm = tpm_find();
>>>      info->pvpanic_port = pvpanic_port();
>>> @@ -981,6 +991,29 @@ build_ssdt(GArray *table_data, GArray *linker,
>>>  
>>>      sb_scope = aml_scope("_SB");
>>>      {
>>> +        if (misc->vmgen_buf_paddr) {
>>> +            dev = aml_device("VMGI");
>>> +
>>> +            aml_append(dev, aml_name_decl("_HID", aml_string("QEMU0003")));
>>> +            aml_append(dev, aml_name_decl("_CID", aml_string("VM_Gen_Counter")));
>>> +            aml_append(dev, aml_name_decl("_DDN", aml_string("VM_Gen_Counter")));
>>> +
>>> +            pkg = aml_package(2);
>>> +            /* low 32 bits of UUID buffer addr*/
>>> +            aml_append(pkg, aml_int(misc->vmgen_buf_paddr));
>>> +            aml_append(pkg, aml_int(0)); /* high 32 bits of UUID buffer addr */
>>
>> Really should be full 64 bit, and use a 64 bit BAR.
> for compatibility with 32 windows guest, it probably should stay below
> 4Gb address space.
> 
>>
>>
>>> +            aml_append(dev, aml_name_decl("ADDR", pkg));
>>> +
>>> +            aml_append(sb_scope, dev);
>>> +
>>> +            scope = aml_scope("\\_GPE");
>>> +            method = aml_method("_E00", 0);
>>> +            aml_append(method,
>>> +                aml_notify(aml_name("\\_SB.VMGI"), aml_int(0x80)));
>>> +            aml_append(scope, method);
>>> +            aml_append(ssdt, scope);
>>> +        }
>>> +
>>
>> This confuses me.
>> Shouldn't VMGI device be on PCI bus, where it was
>> added?
> I'll try to merge it with PCI slot description that associated with this device,
> Though I'm not sure that windows would handle it correctly.
> 
>>
>>
>>
>>>          /* create PCI0.PRES device and its _CRS to reserve CPU hotplug MMIO */
>>>          dev = aml_device("PCI0." stringify(CPU_HOTPLUG_RESOURCE_DEVICE));
>>>          aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A06")));
>>> diff --git a/hw/i386/acpi-dsdt.dsl b/hw/i386/acpi-dsdt.dsl
>>> index a611e07..884038b 100644
>>> --- a/hw/i386/acpi-dsdt.dsl
>>> +++ b/hw/i386/acpi-dsdt.dsl
>>> @@ -306,8 +306,6 @@ DefinitionBlock (
>>>      Scope(\_GPE) {
>>>          Name(_HID, "ACPI0006")
>>>  
>>> -        Method(_L00) {
>>> -        }
>>>          Method(_E01) {
>>>              // PCI hotplug event
>>>              Acquire(\_SB.PCI0.BLCK, 0xFFFF)
>>> diff --git a/hw/i386/q35-acpi-dsdt.dsl b/hw/i386/q35-acpi-dsdt.dsl
>>> index e1cee5d..9eb794c 100644
>>> --- a/hw/i386/q35-acpi-dsdt.dsl
>>> +++ b/hw/i386/q35-acpi-dsdt.dsl
>>> @@ -414,8 +414,6 @@ DefinitionBlock (
>>>      Scope(\_GPE) {
>>>          Name(_HID, "ACPI0006")
>>>  
>>> -        Method(_L00) {
>>> -        }
>>>          Method(_L01) {
>>>          }
>>>          Method(_E02) {
>>> diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
>>> index 029a56f..e047aea 100644
>>> --- a/hw/misc/Makefile.objs
>>> +++ b/hw/misc/Makefile.objs
>>> @@ -41,3 +41,4 @@ obj-$(CONFIG_ZYNQ) += zynq_slcr.o
>>>  
>>>  obj-$(CONFIG_PVPANIC) += pvpanic.o
>>>  obj-$(CONFIG_EDU) += edu.o
>>> +obj-$(CONFIG_VMGENID) += vmgenid.o
>>> diff --git a/hw/misc/vmgenid.c b/hw/misc/vmgenid.c
>>> new file mode 100644
>>> index 0000000..631c9a3
>>> --- /dev/null
>>> +++ b/hw/misc/vmgenid.c
>>> @@ -0,0 +1,130 @@
>>> +/*
>>> + *  Virtual Machine Generation ID Device
>>> + *
>>> + *  Copyright (C) 2014 Red Hat Inc.
>>> + *
>>> + *  Authors: Gal Hammer <ghammer@redhat.com>
>>> + *           Igor Mammedov <imammedo@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/pci/pci.h"
>>> +#include "hw/misc/vmgenid.h"
>>> +#include "hw/acpi/acpi.h"
>>> +#include "qapi/visitor.h"
>>> +
>>> +#define VMGENID(obj) OBJECT_CHECK(VmGenIdState, (obj), VMGENID_DEVICE)
>>> +
>>> +typedef struct VmGenIdState {
>>> +    PCIDevice parent_obj;
>>> +    MemoryRegion iomem;
>>> +    uint8_t guid[16];
>>> +    bool guid_set;
>>> +} VmGenIdState;
>>> +
>>> +static void vmgenid_update_guest(VmGenIdState *s)
>>> +{
>>> +    Object *acpi_obj;
>>> +    void *ptr = memory_region_get_ram_ptr(&s->iomem);
>>> +
>>> +    memcpy(ptr, &s->guid, sizeof(s->guid));
>>> +    memory_region_set_dirty(&s->iomem, 0, sizeof(s->guid));
>>> +
>>> +    acpi_obj = object_resolve_path_type("", TYPE_ACPI_DEVICE_IF, NULL);
>>> +    if (acpi_obj) {
>>> +        AcpiDeviceIfClass *adevc = ACPI_DEVICE_IF_GET_CLASS(acpi_obj);
>>> +        AcpiDeviceIf *adev = ACPI_DEVICE_IF(acpi_obj);
>>> +        ACPIREGS *acpi_regs = adevc->regs(adev);
>>> +
>>> +        acpi_regs->gpe.sts[0] |= 1; /* _GPE.E00 handler */
>>> +        acpi_update_sci(acpi_regs, adevc->sci(adev));
>>> +    }
>>> +}
>>> +
>>> +static void vmgenid_set_uuid(Object *obj, const char *value, Error **errp)
>>> +{
>>> +    VmGenIdState *s = VMGENID(obj);
>>> +
>>> +    if (qemu_uuid_parse(value, s->guid) < 0) {
>>> +        error_setg(errp, "'%s." VMGENID_UUID "': Fail to parse UUID string.",
>>> +                   object_get_typename(OBJECT(s)));
>>> +        return;
>>> +    }
>>> +
>>> +    s->guid_set = true;
>>> +    vmgenid_update_guest(s);
>>> +}
>>> +
>>> +static void vmgenid_get_vmgid_addr(Object *obj, Visitor *v, void *opaque,
>>> +                                   const char *name, Error **errp)
>>> +{
>>> +    VmGenIdState *s = VMGENID(obj);
>>> +    int64_t value = pci_get_bar_addr(PCI_DEVICE(s), 0);
>>> +
>>> +    if (value == PCI_BAR_UNMAPPED) {
>>> +        error_setg(errp, "'%s." VMGENID_VMGID_ADDR "': not initialized",
>>> +                   object_get_typename(OBJECT(s)));
>>
>> Looks wrong - this is guest configuration.
>> I don't think we want to print errors and exit in this way just
>> because guest did not map the device.
>> Better to mask it in acpi.
> that would mean that user asked to run guest with VMGID but BIOS failed
> to do so for some reason and error was just ignored.
> I think it's better to abort guest at early start-up stage
> i.e. when it loads ACPI tables, than proceed with invalid state
> silently.
> 
>>
>>
>>> +        return;
>>> +    }
>>> +    visit_type_int(v, &value, name, errp);
>>> +}
>>> +
>>> +static void vmgenid_initfn(Object *obj)
>>> +{
>>> +    VmGenIdState *s = VMGENID(obj);
>>> +
>>> +    memory_region_init_ram(&s->iomem, obj, "vgid.bar", sizeof(s->guid),
>>> +                           &error_abort);
>>> +
>>> +    object_property_add_str(obj, VMGENID_UUID, NULL, vmgenid_set_uuid, NULL);
>>> +    object_property_add(obj, VMGENID_VMGID_ADDR, "int", vmgenid_get_vmgid_addr,
>>> +                        NULL, NULL, NULL, NULL);
>>> +}
>>> +
>>> +
>>> +static void vmgenid_realize(PCIDevice *dev, Error **errp)
>>> +{
>>> +    VmGenIdState *s = VMGENID(dev);
>>> +
>>> +    if (!s->guid_set) {
>>> +        error_setg(errp, "'%s." VMGENID_UUID "' property is not set",
>>> +                   object_get_typename(OBJECT(s)));
>>> +        return;
>>> +    }
>>> +
>>> +    vmstate_register_ram(&s->iomem, DEVICE(s));
>>> +    pci_register_bar(PCI_DEVICE(s), 0, PCI_BASE_ADDRESS_SPACE_MEMORY, &s->iomem);
>>
>> This means this BAR is 16 bytes in size?
>>
>> Not good, should be full 4K.
> yep, it should be page
> 
>>
>> You also make the BAR non prefetcheable, which makes
>> some OS-es (e.g. old linux) map it non-cacheable.
> we don't care about old linux here because it doesn't have
> vmgenid driver neither a modern linux but at least it could
> be added to it.
> But there shouldn't be a issue with tagging it as prefetchable.
> 
> 
>>
>>
>>
>>
>>
>>
>>> +    return;
>>> +}
>>> +
>>> +static void vmgenid_class_init(ObjectClass *klass, void *data)
>>> +{
>>> +    DeviceClass *dc = DEVICE_CLASS(klass);
>>> +    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
>>> +
>>> +    set_bit(DEVICE_CATEGORY_MISC, dc->categories);
>>> +    dc->hotpluggable = false;
>>> +    k->realize = vmgenid_realize;
>>> +    k->vendor_id = PCI_VENDOR_ID_REDHAT;
>>> +    k->device_id = PCI_DEVICE_ID_REDHAT_VMGENID;
>>> +    k->class_id = PCI_CLASS_MEMORY_RAM;
>>> +}
>>> +
>>> +static const TypeInfo vmgenid_device_info = {
>>> +    .name          = VMGENID_DEVICE,
>>> +    .parent        = TYPE_PCI_DEVICE,
>>> +    .instance_size = sizeof(VmGenIdState),
>>> +    .instance_init = vmgenid_initfn,
>>> +    .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/acpi/acpi.h b/include/hw/acpi/acpi.h
>>> index 1f678b4..a09cb3f 100644
>>> --- a/include/hw/acpi/acpi.h
>>> +++ b/include/hw/acpi/acpi.h
>>> @@ -25,6 +25,7 @@
>>>  #include "qemu/option.h"
>>>  #include "exec/memory.h"
>>>  #include "hw/irq.h"
>>> +#include "hw/acpi/acpi_dev_interface.h"
>>>  
>>>  /*
>>>   * current device naming scheme supports up to 256 memory devices
>>> diff --git a/include/hw/misc/vmgenid.h b/include/hw/misc/vmgenid.h
>>> new file mode 100644
>>> index 0000000..325f095
>>> --- /dev/null
>>> +++ b/include/hw/misc/vmgenid.h
>>> @@ -0,0 +1,21 @@
>>> +/*
>>> + *  Virtual Machine Generation ID Device
>>> + *
>>> + *  Copyright (C) 2014 Red Hat Inc.
>>> + *
>>> + *  Authors: Gal Hammer <ghammer@redhat.com>
>>> + *           Igor Mammedov <imammedo@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.
>>> + *
>>> + */
>>> +
>>> +#ifndef HW_MISC_VMGENID_H
>>> +#define HW_MISC_VMGENID_H
>>> +
>>> +#define VMGENID_DEVICE       "vmgenid"
>>> +#define VMGENID_UUID         "uuid"
>>> +#define VMGENID_VMGID_ADDR   "vmgid-addr"
>>> +
>>> +#endif
>>> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
>>> index 3164fc3..245171b 100644
>>> --- a/include/hw/pci/pci.h
>>> +++ b/include/hw/pci/pci.h
>>> @@ -90,6 +90,7 @@
>>>  #define PCI_DEVICE_ID_REDHAT_TEST        0x0005
>>>  #define PCI_DEVICE_ID_REDHAT_SDHCI       0x0007
>>>  #define PCI_DEVICE_ID_REDHAT_PCIE_HOST   0x0008
>>> +#define PCI_DEVICE_ID_REDHAT_VMGENID     0x0009
>>>  #define PCI_DEVICE_ID_REDHAT_QXL         0x0100
>>>  
>>>  #define FMT_PCIBUS                      PRIx64
>>> -- 
>>> 1.8.3.1
>>
> 

Re: [Qemu-devel] [PATCH V13 3/4] pc: add a Virtual Machine Generation ID device

[Michael S. Tsirkin]

On Mon, Mar 02, 2015 at 06:05:43PM +0100, Igor Mammedov wrote:
> On Sun, 1 Mar 2015 16:09:33 +0100
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
> 
> > On Wed, Feb 25, 2015 at 05:08:52PM +0000, Igor Mammedov 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 and its implementation.
> > > 
> > > The GUID is set using "vmgenid.uuid" property.
> > > 
> > > Example of using vmgenid device:
> > >  -device vmgenid,id=FOO,uuid="324e6eaf-d1d1-4bf6-bf41-b9bb6c91fb87"
> > 
> > If you do this, doesn't windows then prompt for a driver?
> it doesn't since PCI_CLASS_MEMORY_RAM is displayed as driver less 
> "PCI standard RAM Controller" binding in device manager.
> 
> There was an issue with
>  virtio balloon device + pseries firmware + kernel bug
>  http://lists.gnu.org/archive/html/qemu-devel/2012-03/msg04704.html
> but it shouldn't be an issue for x86 targets with which device is
> supposed to be used.
> CCing David and Laszlo in case UEFI might do some crazy stuff like
> pseries firmware.

I have to say, if it's not RAM, using PCI_CLASS_MEMORY_RAM seems
wrong. Can't we tag it in ACPI in some way?

I guess I can somewhat buy 0580 "other memory controller".

I think we also want some space for future expansion
in this device.
How about we reserve first 4K, and set bit 0 to mean
"has uuid"?

-- 
MST

Re: [Qemu-devel] [PATCH V13 3/4] pc: add a Virtual Machine Generation ID device

[David Gibson]

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

On Mon, 2 Mar 2015 22:06:22 +0100
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Mon, Mar 02, 2015 at 06:05:43PM +0100, Igor Mammedov wrote:
> > On Sun, 1 Mar 2015 16:09:33 +0100
> > "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > 
> > > On Wed, Feb 25, 2015 at 05:08:52PM +0000, Igor Mammedov 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 and its implementation.
> > > > 
> > > > The GUID is set using "vmgenid.uuid" property.
> > > > 
> > > > Example of using vmgenid device:
> > > >  -device vmgenid,id=FOO,uuid="324e6eaf-d1d1-4bf6-bf41-b9bb6c91fb87"
> > > 
> > > If you do this, doesn't windows then prompt for a driver?
> > it doesn't since PCI_CLASS_MEMORY_RAM is displayed as driver less 
> > "PCI standard RAM Controller" binding in device manager.
> > 
> > There was an issue with
> >  virtio balloon device + pseries firmware + kernel bug
> >  http://lists.gnu.org/archive/html/qemu-devel/2012-03/msg04704.html
> > but it shouldn't be an issue for x86 targets with which device is
> > supposed to be used.
> > CCing David and Laszlo in case UEFI might do some crazy stuff like
> > pseries firmware.
> 
> I have to say, if it's not RAM, using PCI_CLASS_MEMORY_RAM seems
> wrong. Can't we tag it in ACPI in some way?

I agree.  PCI_CLASS_MEMORY_RAM means something quite specific, and this
device isn't it.  AFAICT, this would break pseries guests exactly like
the balloon device did before we removed the bogus class code.

I'm lacking context to see what the purpose of this device is, and
whether we could ever want something similar on ppc64.

> I guess I can somewhat buy 0580 "other memory controller".
> 
> I think we also want some space for future expansion
> in this device.
> How about we reserve first 4K, and set bit 0 to mean
> "has uuid"?
> 
> -- 
> MST


-- 
David Gibson <dgibson@redhat.com>
Senior Software Engineer, Virtualization, Red Hat

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

Re: [Qemu-devel] [PATCH V13 3/4] pc: add a Virtual Machine Generation ID device

[Michael S. Tsirkin]

On Tue, Mar 03, 2015 at 12:03:26PM +1100, David Gibson wrote:
> On Mon, 2 Mar 2015 22:06:22 +0100
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
> 
> > On Mon, Mar 02, 2015 at 06:05:43PM +0100, Igor Mammedov wrote:
> > > On Sun, 1 Mar 2015 16:09:33 +0100
> > > "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > > 
> > > > On Wed, Feb 25, 2015 at 05:08:52PM +0000, Igor Mammedov 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 and its implementation.
> > > > > 
> > > > > The GUID is set using "vmgenid.uuid" property.
> > > > > 
> > > > > Example of using vmgenid device:
> > > > >  -device vmgenid,id=FOO,uuid="324e6eaf-d1d1-4bf6-bf41-b9bb6c91fb87"
> > > > 
> > > > If you do this, doesn't windows then prompt for a driver?
> > > it doesn't since PCI_CLASS_MEMORY_RAM is displayed as driver less 
> > > "PCI standard RAM Controller" binding in device manager.
> > > 
> > > There was an issue with
> > >  virtio balloon device + pseries firmware + kernel bug
> > >  http://lists.gnu.org/archive/html/qemu-devel/2012-03/msg04704.html
> > > but it shouldn't be an issue for x86 targets with which device is
> > > supposed to be used.
> > > CCing David and Laszlo in case UEFI might do some crazy stuff like
> > > pseries firmware.
> > 
> > I have to say, if it's not RAM, using PCI_CLASS_MEMORY_RAM seems
> > wrong. Can't we tag it in ACPI in some way?
> 
> I agree.  PCI_CLASS_MEMORY_RAM means something quite specific, and this
> device isn't it.  AFAICT, this would break pseries guests exactly like
> the balloon device did before we removed the bogus class code.
> 
> I'm lacking context to see what the purpose of this device is, and
> whether we could ever want something similar on ppc64.
> 
> > I guess I can somewhat buy 0580 "other memory controller".
> > 
> > I think we also want some space for future expansion
> > in this device.
> > How about we reserve first 4K, and set bit 0 to mean
> > "has uuid"?
> > 
> > -- 
> > MST
> 
> 
> -- 
> David Gibson <dgibson@redhat.com>
> Senior Software Engineer, Virtualization, Red Hat


it just exposes a unique id as part of its BAR.

Re: [Qemu-devel] [PATCH V13 3/4] pc: add a Virtual Machine Generation ID device

[Igor Mammedov]

On Tue, 3 Mar 2015 12:03:26 +1100
David Gibson <dgibson@redhat.com> wrote:

> On Mon, 2 Mar 2015 22:06:22 +0100
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
> 
> > On Mon, Mar 02, 2015 at 06:05:43PM +0100, Igor Mammedov wrote:
> > > On Sun, 1 Mar 2015 16:09:33 +0100
> > > "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > > 
> > > > On Wed, Feb 25, 2015 at 05:08:52PM +0000, Igor Mammedov 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 and its implementation.
> > > > > 
> > > > > The GUID is set using "vmgenid.uuid" property.
> > > > > 
> > > > > Example of using vmgenid device:
> > > > >  -device vmgenid,id=FOO,uuid="324e6eaf-d1d1-4bf6-bf41-b9bb6c91fb87"
> > > > 
> > > > If you do this, doesn't windows then prompt for a driver?
> > > it doesn't since PCI_CLASS_MEMORY_RAM is displayed as driver less 
> > > "PCI standard RAM Controller" binding in device manager.
> > > 
> > > There was an issue with
> > >  virtio balloon device + pseries firmware + kernel bug
> > >  http://lists.gnu.org/archive/html/qemu-devel/2012-03/msg04704.html
> > > but it shouldn't be an issue for x86 targets with which device is
> > > supposed to be used.
> > > CCing David and Laszlo in case UEFI might do some crazy stuff like
> > > pseries firmware.
> > 
> > I have to say, if it's not RAM, using PCI_CLASS_MEMORY_RAM seems
> > wrong. Can't we tag it in ACPI in some way?
I'm not sure that it's not a RAM though, (according to MS spec
it could be RAM, ROM or something else). This device is some RAM
controller and though it provides some RAM as its resource
it doesn't mean that it provides RAM that could be used freely as
as a generic RAM reported to host via E802 table.

> 
> I agree.  PCI_CLASS_MEMORY_RAM means something quite specific, and this
> device isn't it.  AFAICT, this would break pseries guests exactly like
> the balloon device did before we removed the bogus class code.
Could you send me a reference to source that explains what
PCI_CLASS_MEMORY_RAM means?
From what I've goggled out so far it's not RAM by itself but
some RAM controller.
I've noticed that for example NVidia cards could expose
several of such devices (my guess it's VRAM) so BAR exposed by
such device shouldn't be treated as generic RAM blindly (like it seems
pseries does) without vendor specific driver for example like ivshmem.
I've also checked linux kernel, it only has unused define for this
class id.

> 
> I'm lacking context to see what the purpose of this device is, and
> whether we could ever want something similar on ppc64.
That is to provide guest with some UUID and possibly change it
at runtime and notify guest via ACPI about change as specified
by Microsoft spec.

> 
> > I guess I can somewhat buy 0580 "other memory controller".
I've tried other CLASS IDs but then windows asks for driver for
a unknown device. I have also tried to hide PCI device entry
by setting _STA bit "not shown in UI" on respective PCI slot
but it doesn't help and windows depending on version asks for driver
or just tags with question mark PCI device in device manager.

Merging ACPI description of VMGEN_ID device with PCI slot
description produces anyway 2 devices in device manager:
 1 PCI standard RAM controller
 2 VM Generation ID

> > 
> > I think we also want some space for future expansion
> > in this device.
> > How about we reserve first 4K, and set bit 0 to mean
That might be useful in future but then it wouldn't be vmgenid only device.
Also what do you mean by the first?

> > "has uuid"?
> > 
> > -- 
> > MST
> 
> 

Re: [Qemu-devel] [PATCH V13 3/4] pc: add a Virtual Machine Generation ID device

[David Gibson]

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

On Tue, 3 Mar 2015 12:06:05 +0100
Igor Mammedov <imammedo@redhat.com> wrote:

> On Tue, 3 Mar 2015 12:03:26 +1100
> David Gibson <dgibson@redhat.com> wrote:
> 
> > On Mon, 2 Mar 2015 22:06:22 +0100
> > "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > 
> > > On Mon, Mar 02, 2015 at 06:05:43PM +0100, Igor Mammedov wrote:
> > > > On Sun, 1 Mar 2015 16:09:33 +0100
> > > > "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > > > 
> > > > > On Wed, Feb 25, 2015 at 05:08:52PM +0000, Igor Mammedov 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 and its implementation.
> > > > > > 
> > > > > > The GUID is set using "vmgenid.uuid" property.
> > > > > > 
> > > > > > Example of using vmgenid device:
> > > > > >  -device vmgenid,id=FOO,uuid="324e6eaf-d1d1-4bf6-bf41-b9bb6c91fb87"
> > > > > 
> > > > > If you do this, doesn't windows then prompt for a driver?
> > > > it doesn't since PCI_CLASS_MEMORY_RAM is displayed as driver less 
> > > > "PCI standard RAM Controller" binding in device manager.
> > > > 
> > > > There was an issue with
> > > >  virtio balloon device + pseries firmware + kernel bug
> > > >  http://lists.gnu.org/archive/html/qemu-devel/2012-03/msg04704.html
> > > > but it shouldn't be an issue for x86 targets with which device is
> > > > supposed to be used.
> > > > CCing David and Laszlo in case UEFI might do some crazy stuff like
> > > > pseries firmware.
> > > 
> > > I have to say, if it's not RAM, using PCI_CLASS_MEMORY_RAM seems
> > > wrong. Can't we tag it in ACPI in some way?
> I'm not sure that it's not a RAM though, (according to MS spec
> it could be RAM, ROM or something else). This device is some RAM
> controller and though it provides some RAM as its resource
> it doesn't mean that it provides RAM that could be used freely as
> as a generic RAM reported to host via E802 table.

I think describing it as RAM is.. tremendously counter-intuitive at
very best.

> > I agree.  PCI_CLASS_MEMORY_RAM means something quite specific, and this
> > device isn't it.  AFAICT, this would break pseries guests exactly like
> > the balloon device did before we removed the bogus class code.
> Could you send me a reference to source that explains what
> PCI_CLASS_MEMORY_RAM means?
> From what I've goggled out so far it's not RAM by itself but
> some RAM controller.

I was mistaken.  Had a chat to someone with access to the PCI spec, and
in fact PCI_CLASS_MEMORY_RAM specifies hardly anything.

So, yeah, I think the fact that RAM class devices get treated as
"normal" RAM by the pseries firmware is a bug in that firmware.  I'll
look into getting that fixed.

> I've noticed that for example NVidia cards could expose
> several of such devices (my guess it's VRAM) so BAR exposed by
> such device shouldn't be treated as generic RAM blindly (like it seems
> pseries does) without vendor specific driver for example like ivshmem.
> I've also checked linux kernel, it only has unused define for this
> class id.

So, I still don't think PCI_CLASS_MEMORY_RAM is the best description.
But, I wonder, is there any point to the argument.  I wasn't clear if
this was based on a Hyper-V specification that specifies the class
code.  If so, we're pretty much stuck with it, no matter what other
irritations it causes.

> > I'm lacking context to see what the purpose of this device is, and
> > whether we could ever want something similar on ppc64.
> That is to provide guest with some UUID and possibly change it
> at runtime and notify guest via ACPI about change as specified
> by Microsoft spec.

Ok.  Can the device be used without extra ACPI stuff?  If not, it's
clearly useless on pseries.  If so, it's probably not the most natural
way of presenting such information to a pseries guest, but it might be
useful as a method that already exists.

-- 
David Gibson <dgibson@redhat.com>
Senior Software Engineer, Virtualization, Red Hat

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

Re: [Qemu-devel] [PATCH V13 3/4] pc: add a Virtual Machine Generation ID device

[Michael S. Tsirkin]

On Thu, Mar 05, 2015 at 01:51:26PM +1100, David Gibson wrote:
> On Tue, 3 Mar 2015 12:06:05 +0100
> Igor Mammedov <imammedo@redhat.com> wrote:
> 
> > On Tue, 3 Mar 2015 12:03:26 +1100
> > David Gibson <dgibson@redhat.com> wrote:
> > 
> > > On Mon, 2 Mar 2015 22:06:22 +0100
> > > "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > > 
> > > > On Mon, Mar 02, 2015 at 06:05:43PM +0100, Igor Mammedov wrote:
> > > > > On Sun, 1 Mar 2015 16:09:33 +0100
> > > > > "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > > > > 
> > > > > > On Wed, Feb 25, 2015 at 05:08:52PM +0000, Igor Mammedov 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 and its implementation.
> > > > > > > 
> > > > > > > The GUID is set using "vmgenid.uuid" property.
> > > > > > > 
> > > > > > > Example of using vmgenid device:
> > > > > > >  -device vmgenid,id=FOO,uuid="324e6eaf-d1d1-4bf6-bf41-b9bb6c91fb87"
> > > > > > 
> > > > > > If you do this, doesn't windows then prompt for a driver?
> > > > > it doesn't since PCI_CLASS_MEMORY_RAM is displayed as driver less 
> > > > > "PCI standard RAM Controller" binding in device manager.
> > > > > 
> > > > > There was an issue with
> > > > >  virtio balloon device + pseries firmware + kernel bug
> > > > >  http://lists.gnu.org/archive/html/qemu-devel/2012-03/msg04704.html
> > > > > but it shouldn't be an issue for x86 targets with which device is
> > > > > supposed to be used.
> > > > > CCing David and Laszlo in case UEFI might do some crazy stuff like
> > > > > pseries firmware.
> > > > 
> > > > I have to say, if it's not RAM, using PCI_CLASS_MEMORY_RAM seems
> > > > wrong. Can't we tag it in ACPI in some way?
> > I'm not sure that it's not a RAM though, (according to MS spec
> > it could be RAM, ROM or something else). This device is some RAM
> > controller and though it provides some RAM as its resource
> > it doesn't mean that it provides RAM that could be used freely as
> > as a generic RAM reported to host via E802 table.
> 
> I think describing it as RAM is.. tremendously counter-intuitive at
> very best.
> 
> > > I agree.  PCI_CLASS_MEMORY_RAM means something quite specific, and this
> > > device isn't it.  AFAICT, this would break pseries guests exactly like
> > > the balloon device did before we removed the bogus class code.
> > Could you send me a reference to source that explains what
> > PCI_CLASS_MEMORY_RAM means?
> > From what I've goggled out so far it's not RAM by itself but
> > some RAM controller.
> 
> I was mistaken.  Had a chat to someone with access to the PCI spec, and
> in fact PCI_CLASS_MEMORY_RAM specifies hardly anything.
> 
> So, yeah, I think the fact that RAM class devices get treated as
> "normal" RAM by the pseries firmware is a bug in that firmware.  I'll
> look into getting that fixed.

Thanks, pls keep us updated.

> > I've noticed that for example NVidia cards could expose
> > several of such devices (my guess it's VRAM) so BAR exposed by
> > such device shouldn't be treated as generic RAM blindly (like it seems
> > pseries does) without vendor specific driver for example like ivshmem.
> > I've also checked linux kernel, it only has unused define for this
> > class id.
> 
> So, I still don't think PCI_CLASS_MEMORY_RAM is the best description.
> But, I wonder, is there any point to the argument.  I wasn't clear if
> this was based on a Hyper-V specification that specifies the class
> code.  If so, we're pretty much stuck with it, no matter what other
> irritations it causes.

No. AFAIK, there's no spec that requires this class value.
This is simply a work around found by Igor to avoid getting driver
prompt from windows.  Apparently, if you set this class value, windows
does not prompt for driver even if there's no matching driver.

However, this behaviour seems to be undocumented.
Why this is the case, and whether this will keep being the case
in the future, I think it's safe to say, we don't know.


> > > I'm lacking context to see what the purpose of this device is, and
> > > whether we could ever want something similar on ppc64.
> > That is to provide guest with some UUID and possibly change it
> > at runtime and notify guest via ACPI about change as specified
> > by Microsoft spec.
> 
> Ok.  Can the device be used without extra ACPI stuff?

Absolutely: all the ACPI stuff does is return back BAR(0) value.
You can get at that without ACPI just as well.

> If not, it's
> clearly useless on pseries.  If so, it's probably not the most natural
> way of presenting such information to a pseries guest, but it might be
> useful as a method that already exists.
> 
> -- 
> David Gibson <dgibson@redhat.com>
> Senior Software Engineer, Virtualization, Red Hat

Re: [Qemu-devel] [PATCH V13 3/4] pc: add a Virtual Machine Generation ID device

[Michael S. Tsirkin]

On Wed, Feb 25, 2015 at 05:08:52PM +0000, Igor Mammedov wrote:
> diff --git a/include/hw/acpi/acpi.h b/include/hw/acpi/acpi.h
> index 1f678b4..a09cb3f 100644
> --- a/include/hw/acpi/acpi.h
> +++ b/include/hw/acpi/acpi.h
> @@ -25,6 +25,7 @@
>  #include "qemu/option.h"
>  #include "exec/memory.h"
>  #include "hw/irq.h"
> +#include "hw/acpi/acpi_dev_interface.h"
>  
>  /*
>   * current device naming scheme supports up to 256 memory devices

BTW why is this here?

-- 
MST

Re: [Qemu-devel] [PATCH V13 3/4] pc: add a Virtual Machine Generation ID device

[Igor Mammedov]

On Mon, 2 Mar 2015 09:50:39 +0100
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Wed, Feb 25, 2015 at 05:08:52PM +0000, Igor Mammedov wrote:
> > diff --git a/include/hw/acpi/acpi.h b/include/hw/acpi/acpi.h
> > index 1f678b4..a09cb3f 100644
> > --- a/include/hw/acpi/acpi.h
> > +++ b/include/hw/acpi/acpi.h
> > @@ -25,6 +25,7 @@
> >  #include "qemu/option.h"
> >  #include "exec/memory.h"
> >  #include "hw/irq.h"
> > +#include "hw/acpi/acpi_dev_interface.h"
> >  
> >  /*
> >   * current device naming scheme supports up to 256 memory devices
> 
> BTW why is this here?
> 

Yep, looks like it should have been in:
include/hw/acpi/pc-hotplug.h

I'll post patch.

[Qemu-devel] [PATCH V13 4/4] tests: add a unit test for the vmgenid device.

[Igor Mammedov]

* test that guest can read UUID provided on CLI from buffer
  accessing it at HPA which is available via 'vmgid-addr'
  property when device is inintialized.
* test setting UUID at runtime and check that it's updated
  at expected HPA.

Signed-off-by: Gal Hammer <ghammer@redhat.com>
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
v2:
 * fix: make sure that test actually runs
 * use property to get UUID buffer HPA
 * add setting UUID via QMP at runtime test case
---
 tests/Makefile       |  2 ++
 tests/vmgenid-test.c | 92 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 94 insertions(+)
 create mode 100644 tests/vmgenid-test.c

diff --git a/tests/Makefile b/tests/Makefile
index d5df168..ccd0eb5 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -171,6 +171,7 @@ gcov-files-i386-y += hw/usb/dev-hid.c
 gcov-files-i386-y += hw/usb/dev-storage.c
 check-qtest-i386-y += tests/usb-hcd-xhci-test$(EXESUF)
 gcov-files-i386-y += hw/usb/hcd-xhci.c
+check-qtest-i386-y += tests/vmgenid-test$(EXESUF)
 check-qtest-i386-$(CONFIG_LINUX) += tests/vhost-user-test$(EXESUF)
 check-qtest-x86_64-y = $(check-qtest-i386-y)
 gcov-files-i386-y += i386-softmmu/hw/timer/mc146818rtc.c
@@ -363,6 +364,7 @@ tests/vhost-user-test$(EXESUF): tests/vhost-user-test.o qemu-char.o qemu-timer.o
 tests/qemu-iotests/socket_scm_helper$(EXESUF): tests/qemu-iotests/socket_scm_helper.o
 tests/test-qemu-opts$(EXESUF): tests/test-qemu-opts.o libqemuutil.a libqemustub.a
 tests/test-write-threshold$(EXESUF): tests/test-write-threshold.o $(block-obj-y) libqemuutil.a libqemustub.a
+tests/vmgenid-test$(EXESUF): tests/vmgenid-test.o
 
 ifeq ($(CONFIG_POSIX),y)
 LIBS += -lutil
diff --git a/tests/vmgenid-test.c b/tests/vmgenid-test.c
new file mode 100644
index 0000000..8902d7f
--- /dev/null
+++ b/tests/vmgenid-test.c
@@ -0,0 +1,92 @@
+/*
+ * QTest testcase for VM Generation ID
+ *
+ * Copyright (c) 2015 Red Hat, 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.
+ */
+
+#include <glib.h>
+#include <string.h>
+#include <unistd.h>
+#include "libqtest.h"
+
+/* Wait at most 1 minute */
+#define TEST_DELAY (1 * G_USEC_PER_SEC / 10)
+#define TEST_CYCLES MAX((60 * G_USEC_PER_SEC / TEST_DELAY), 1)
+
+#define VGID_UUID "324e6eaf-d1d1-4bf6-bf41-b9bb6c91fb87"
+
+static void vmgenid_check_uuid(const uint8_t *expected)
+{
+    uint8_t guid[16];
+    int i;
+    uint32_t addr = 0;
+    QDict *response;
+
+    for (i = 0; i < TEST_CYCLES; ++i) {
+        response = qmp("{ 'execute': 'qom-get', 'arguments': { "
+                       "'path': '/machine/peripheral/testvgid', "
+                       "'property': 'vmgid-addr' } }");
+        if (qdict_haskey(response, "return")) {
+            addr = qdict_get_int(response, "return");
+        }
+        QDECREF(response);
+        if (addr) {
+            break;
+        }
+        g_usleep(TEST_DELAY);
+    }
+
+    // Skip the ACPI ADDR method and read the GUID directly from memory.
+    for (i = 0; i < 16; i++) {
+        guid[i] = readb(addr + i);
+    }
+
+    g_assert(memcmp(guid, expected, sizeof(guid)) == 0);
+}
+
+static void vmgenid_test(void)
+{
+    static const uint8_t expected[16] = {
+        0x32, 0x4e, 0x6e, 0xaf, 0xd1, 0xd1, 0x4b, 0xf6,
+        0xbf, 0x41, 0xb9, 0xbb, 0x6c, 0x91, 0xfb, 0x87
+    };
+    vmgenid_check_uuid(expected);
+}
+
+static void vmgenid_set_uuid_test(void)
+{
+    QDict *response;
+    static const uint8_t expected[16] = {
+        0x12, 0x4e, 0x6e, 0xaf, 0xd1, 0xd1, 0x4b, 0xf6,
+        0xbf, 0x41, 0xb9, 0xbb, 0x6c, 0x91, 0xfb, 0x87
+    };
+
+    response = qmp("{ 'execute': 'qom-set', 'arguments': { "
+                   "'path': '/machine/peripheral/testvgid', "
+                   "'property': 'uuid', 'value': '"
+                   "124e6eaf-d1d1-4bf6-bf41-b9bb6c91fb87' } }");
+    g_assert(qdict_haskey(response, "return"));
+    QDECREF(response);
+
+    vmgenid_check_uuid(expected);
+}
+
+int main(int argc, char **argv)
+{
+    int ret;
+
+    g_test_init(&argc, &argv, NULL);
+
+    qtest_start("-machine accel=tcg -device vmgenid,id=testvgid,"
+                "uuid=" VGID_UUID);
+    qtest_add_func("/vmgenid/vmgenid", vmgenid_test);
+    qtest_add_func("/vmgenid/vmgenid/set-uuid", vmgenid_set_uuid_test);
+    ret = g_test_run();
+
+    qtest_end();
+
+    return ret;
+}
-- 
1.8.3.1