| Message ID | 1424884133-323-4-git-send-email-imammedo@redhat.com |
|---|---|
| State | New |
| Headers | show |
> 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?
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.
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
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?
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.
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 >
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 >> >
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"?
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
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.
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 > >
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.
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
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
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