[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 2/2] i386: Add a Virtual Machine Generation ID d
|
From: |
Igor Mammedov |
|
Subject: |
Re: [Qemu-devel] [PATCH 2/2] i386: Add a Virtual Machine Generation ID device. |
|
Date: |
Wed, 17 Sep 2014 11:56:43 +0200 |
On Wed, 17 Sep 2014 03:36:58 -0400 (EDT)
Gal Hammer <address@hidden> wrote:
> Hi Igor,
>
> ----- Original Message -----
> > From: "Igor Mammedov" <address@hidden>
> > To: "Gal Hammer" <address@hidden>
> > Cc: address@hidden, address@hidden
> > Sent: Tuesday, September 16, 2014 5:54:28 PM
> > Subject: Re: [Qemu-devel] [PATCH 2/2] i386: Add a Virtual Machine
> > Generation ID device.
> >
> > On Tue, 16 Sep 2014 16:04:10 +0300
> > Gal Hammer <address@hidden> wrote:
> >
> > > Based on Microsoft's sepecifications (paper can be dowloaded from
> > > http://go.microsoft.com/fwlink/?LinkId=260709), add a device
> > > description to the SSDT ACPI table.
> > >
> > > The GUID is set using a new "vmgenid" device.
> > >
> > > Signed-off-by: Gal Hammer <address@hidden>
> > > ---
> > > default-configs/i386-softmmu.mak | 1 +
> > > default-configs/x86_64-softmmu.mak | 1 +
> > > hw/i386/Makefile.objs | 2 +-
> > > hw/i386/acpi-build.c | 39 +++++++++++++++++
> > > hw/i386/ssdt-vmgenid.dsl | 64 ++++++++++++++++++++++++++++
> > We also need corresponding ssdt-vmgenid.hex file for IASLless hosts,
> > so that build there won't fail.
>
> Isn't that file is created during the build process?
>
> For example:
>
> $ git blame ./x86_64-softmmu/hw/i386/ssdt-tpm.hex
> fatal: no such path 'x86_64-softmmu/hw/i386/ssdt-tpm.hex' in HEAD
check out hw/i386/*.hex*
These blobs are updated/added manually every time corresponding dsl files
changes.
>
> > With this and below notes fixed
> > Reviewed-By: Igor Mammedov <address@hidden>
>
> Thanks. Although I do like that empty line at the end of a file :-).
Well, it's fine for me.
It's just that most of files do not do this, I've checked it by grepping
"type_init(" which is usually the last line in file.
>
> Gal.
>
> > > hw/misc/Makefile.objs | 1 +
> > > hw/misc/vmgenid.c | 85
> > > ++++++++++++++++++++++++++++++++++++++
> > > include/hw/i386/pc.h | 3 ++
> > > 8 files changed, 195 insertions(+), 1 deletion(-)
> > > create mode 100644 hw/i386/ssdt-vmgenid.dsl
> > > create mode 100644 hw/misc/vmgenid.c
> > >
> > > diff --git a/default-configs/i386-softmmu.mak
> > > b/default-configs/i386-softmmu.mak
> > > index 8e08841..bd33c75 100644
> > > --- a/default-configs/i386-softmmu.mak
> > > +++ b/default-configs/i386-softmmu.mak
> > > @@ -45,3 +45,4 @@ CONFIG_IOAPIC=y
> > > CONFIG_ICC_BUS=y
> > > CONFIG_PVPANIC=y
> > > CONFIG_MEM_HOTPLUG=y
> > > +CONFIG_VMGENID=y
> > > diff --git a/default-configs/x86_64-softmmu.mak
> > > b/default-configs/x86_64-softmmu.mak
> > > index 66557ac..006fc7c 100644
> > > --- a/default-configs/x86_64-softmmu.mak
> > > +++ b/default-configs/x86_64-softmmu.mak
> > > @@ -45,3 +45,4 @@ CONFIG_IOAPIC=y
> > > CONFIG_ICC_BUS=y
> > > CONFIG_PVPANIC=y
> > > CONFIG_MEM_HOTPLUG=y
> > > +CONFIG_VMGENID=y
> > > diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs
> > > index 9d419ad..cd1beb3 100644
> > > --- a/hw/i386/Makefile.objs
> > > +++ b/hw/i386/Makefile.objs
> > > @@ -12,7 +12,7 @@ hw/i386/acpi-build.o: hw/i386/acpi-build.c
> > > hw/i386/acpi-dsdt.hex \
> > > hw/i386/ssdt-proc.hex hw/i386/ssdt-pcihp.hex hw/i386/ssdt-misc.hex \
> > > hw/i386/acpi-dsdt.hex hw/i386/q35-acpi-dsdt.hex \
> > > hw/i386/q35-acpi-dsdt.hex hw/i386/ssdt-mem.hex \
> > > - hw/i386/ssdt-tpm.hex
> > > + hw/i386/ssdt-tpm.hex hw/i386/ssdt-vmgenid.hex
> > >
> > > iasl-option=$(shell if test -z "`$(1) $(2) 2>&1 > /dev/null`" \
> > > ; then echo "$(2)"; else echo "$(3)"; fi ;)
> > > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> > > index a313321..72d5a88 100644
> > > --- a/hw/i386/acpi-build.c
> > > +++ b/hw/i386/acpi-build.c
> > > @@ -96,6 +96,8 @@ typedef struct AcpiMiscInfo {
> > > const unsigned char *dsdt_code;
> > > unsigned dsdt_size;
> > > uint16_t pvpanic_port;
> > > + bool vm_generation_id_set;
> > > + uint8_t vm_generation_id[16];
> > > } AcpiMiscInfo;
> > >
> > > typedef struct AcpiBuildPciBusHotplugState {
> > > @@ -216,6 +218,7 @@ static void acpi_get_misc_info(AcpiMiscInfo *info)
> > > info->has_hpet = hpet_find();
> > > info->has_tpm = tpm_find();
> > > info->pvpanic_port = pvpanic_port();
> > > + info->vm_generation_id_set =
> > > vm_generation_id(info->vm_generation_id);
> > > }
> > >
> > > static void acpi_get_pci_info(PcPciInfo *info)
> > > @@ -710,6 +713,7 @@ static inline char acpi_get_hex(uint32_t val)
> > > #include "hw/i386/ssdt-misc.hex"
> > > #include "hw/i386/ssdt-pcihp.hex"
> > > #include "hw/i386/ssdt-tpm.hex"
> > > +#include "hw/i386/ssdt-vmgenid.hex"
> > >
> > > static void
> > > build_append_notify_method(GArray *device, const char *name,
> > > @@ -1246,6 +1250,37 @@ build_tpm_ssdt(GArray *table_data, GArray *linker)
> > > memcpy(tpm_ptr, ssdt_tpm_aml, sizeof(ssdt_tpm_aml));
> > > }
> > >
> > > +static void
> > > +build_vmgenid_ssdt(GArray *table_data, GArray *linker, AcpiMiscInfo
> > > *info)
> > > +{
> > > + int vgid_start = table_data->len;
> > > + void *vgid_ptr;
> > > + uint8_t *vm_gid_ptr;
> > > + uint32_t vm_gid_physical_address;
> > > +
> > > + vgid_ptr = acpi_data_push(table_data, sizeof(ssdt_vmgenid_aml));
> > > + memcpy(vgid_ptr, ssdt_vmgenid_aml, sizeof(ssdt_vmgenid_aml));
> > > +
> > > + vm_gid_ptr = acpi_data_get_ptr(vgid_ptr, sizeof(ssdt_vmgenid_aml),
> > > + *ssdt_acpi_vm_gid,
> > > + sizeof(info->vm_generation_id));
> > > + memcpy(vm_gid_ptr, info->vm_generation_id,
> > > + sizeof(info->vm_generation_id));
> > > +
> > > + bios_linker_loader_add_pointer(linker, ACPI_BUILD_TABLE_FILE,
> > > + ACPI_BUILD_TABLE_FILE,
> > > + table_data,
> > > + vgid_ptr + *ssdt_acpi_vm_gid_addr,
> > > + sizeof(uint32_t));
> > > +
> > > + vm_gid_physical_address = vgid_start + *ssdt_acpi_vm_gid;
> > > + ACPI_BUILD_SET_LE(vgid_ptr, sizeof(ssdt_vmgenid_aml),
> > > + *ssdt_acpi_vm_gid_addr, 32,
> > > vm_gid_physical_address);
> > > +
> > > + build_header(linker, table_data, vgid_ptr, "SSDT",
> > > + sizeof(ssdt_vmgenid_aml), 1);
> > > +}
> > > +
> > > typedef enum {
> > > MEM_AFFINITY_NOFLAGS = 0,
> > > MEM_AFFINITY_ENABLED = (1 << 0),
> > > @@ -1617,6 +1652,10 @@ void acpi_build(PcGuestInfo *guest_info,
> > > AcpiBuildTables *tables)
> > > acpi_add_table(table_offsets, tables->table_data);
> > > build_tpm_ssdt(tables->table_data, tables->linker);
> > > }
> > > + if (misc.vm_generation_id_set) {
> > > + acpi_add_table(table_offsets, tables->table_data);
> > > + build_vmgenid_ssdt(tables->table_data, tables->linker, &misc);
> > > + }
> > > if (guest_info->numa_nodes) {
> > > acpi_add_table(table_offsets, tables->table_data);
> > > build_srat(tables->table_data, tables->linker, &cpu, guest_info);
> > > diff --git a/hw/i386/ssdt-vmgenid.dsl b/hw/i386/ssdt-vmgenid.dsl
> > > new file mode 100644
> > > index 0000000..54b5c34
> > > --- /dev/null
> > > +++ b/hw/i386/ssdt-vmgenid.dsl
> > > @@ -0,0 +1,64 @@
> > > +/*
> > > + * This program is free software; you can redistribute it and/or modify
> > > + * it under the terms of the GNU General Public License as published by
> > > + * the Free Software Foundation; either version 2 of the License, or
> > > + * (at your option) any later version.
> > > +
> > > + * This program is distributed in the hope that it will be useful,
> > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > > + * GNU General Public License for more details.
> > > +
> > > + * You should have received a copy of the GNU General Public License
> > > along
> > > + * with this program; if not, see <http://www.gnu.org/licenses/>.
> > > + */
> > > +
> > > +/****************************************************************
> > > + * Virtual Machine Generation ID Device
> > > + ****************************************************************/
> > > +
> > > +ACPI_EXTRACT_ALL_CODE ssdt_vmgenid_aml
> > > +
> > > +DefinitionBlock (
> > > + "ssdt-vmgenid.aml", // Output Filename
> > > + "SSDT", // Signature
> > > + 0x01, // SSDT Compliance Revision
> > > + "BXPC", // OEMID
> > > + "BXSSDTSUSP", // TABLE ID
> > > + 0x1 // OEM Revision
> > > + )
> > > +{
> > > + Scope(\_SB) {
> > > +
> > > + Device(VMGI) {
> > > + Name(_HID, "QEMU0002")
> > > + Name(_CID, "VM_Gen_Counter")
> > > + Name(_DDN, "VM_Gen_Counter")
> > > +
> > > + ACPI_EXTRACT_NAME_DWORD_CONST ssdt_acpi_vm_gid_addr
> > > + Name(VGIA, 0x12345678)
> > > +
> > > + ACPI_EXTRACT_NAME_BUFFER16 ssdt_acpi_vm_gid
> > > + Name(VGID, Buffer(16) {
> > > + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> > > + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 })
> > > +
> > > + Method(_STA, 0, NotSerialized) {
> > > + Store(VGIA, Local0)
> > > + If (LEqual(Local0, Zero)) {
> > > + Return (0x00)
> > > + } Else {
> > > + Return (0x0F)
> > > + }
> > > + }
> > > +
> > > + Method(ADDR, 0, Serialized) {
> > > + Store(Package(2) { }, Local0)
> > > + Store(VGIA, Index(Local0, 0))
> > > + Store(0x0000, Index(Local0, 1))
> > > + return (Local0)
> > > + }
> > > + }
> > > + }
> > > +}
> > > +
> > nit, not needed white space
> >
> > > diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
> > > index 979e532..c18b800 100644
> > > --- a/hw/misc/Makefile.objs
> > > +++ b/hw/misc/Makefile.objs
> > > @@ -41,3 +41,4 @@ obj-$(CONFIG_SLAVIO) += slavio_misc.o
> > > obj-$(CONFIG_ZYNQ) += zynq_slcr.o
> > >
> > > obj-$(CONFIG_PVPANIC) += pvpanic.o
> > > +obj-$(CONFIG_VMGENID) += vmgenid.o
> > > diff --git a/hw/misc/vmgenid.c b/hw/misc/vmgenid.c
> > > new file mode 100644
> > > index 0000000..76956d1
> > > --- /dev/null
> > > +++ b/hw/misc/vmgenid.c
> > > @@ -0,0 +1,85 @@
> > > +/*
> > > + * Virtual Machine Generation ID Device
> > > + *
> > > + * Copyright (C) 2014 Red Hat Inc.
> > > + *
> > > + * Authors: Gal Hammer <address@hidden>
> > > + *
> > > + * This work is licensed under the terms of the GNU GPL, version 2 or
> > > later.
> > > + * See the COPYING file in the top-level directory.
> > > + *
> > > + */
> > > +
> > > +#include "hw/i386/pc.h"
> > > +#include "hw/sysbus.h"
> > > +
> > > +#define VMGENID_DEVICE "vmgenid"
> > > +
> > > +#define PROPERTY_UUID "uuid"
> > > +
> > > +#define VMGENID(obj) OBJECT_CHECK(VmGenIdState, (obj), VMGENID_DEVICE)
> > > +
> > > +typedef struct VmGenIdState {
> > > + SysBusDevice parent_obj;
> > > + char *guid_arg;
> > > +} VmGenIdState;
> > > +
> > > +bool vm_generation_id(uint8_t id[16])
> > > +{
> > > + Object *o = object_resolve_path_type("", VMGENID_DEVICE, NULL);
> > > + char *guid;
> > > +
> > > + if (!o) {
> > > + return false;
> > > + }
> > > + guid = object_property_get_str(o, PROPERTY_UUID, NULL);
> > > + /* actual uuid validation was checked during realize. */
> > > + (void)qemu_uuid_parse(guid, id);
> > > + return true;
> > > +}
> > > +
> > > +static void vmgenid_realize(DeviceState *dev, Error **errp)
> > > +{
> > > + VmGenIdState *s = VMGENID(dev);
> > > + uint8_t id[16];
> > > +
> > > + if (!s->guid_arg) {
> > > + error_setg(errp, "missing uuid.");
> > > + return;
> > > + }
> > > +
> > > + if (qemu_uuid_parse(s->guid_arg, id) < 0) {
> > > + error_setg(errp, "Fail to parse UUID string.");
> > > + return;
> > > + }
> > > +}
> > > +
> > > +static Property vmgenid_device_properties[] = {
> > > + DEFINE_PROP_STRING(PROPERTY_UUID, VmGenIdState, guid_arg),
> > > + DEFINE_PROP_END_OF_LIST(),
> > > +};
> > > +
> > > +static void vmgenid_class_init(ObjectClass *klass, void *data)
> > > +{
> > > + DeviceClass *dc = DEVICE_CLASS(klass);
> > > +
> > > + dc->realize = vmgenid_realize;
> > > + dc->props = vmgenid_device_properties;
> > > + dc->cannot_instantiate_with_device_add_yet = false;
> > > + set_bit(DEVICE_CATEGORY_MISC, dc->categories);
> > > +}
> > > +
> > > +static const TypeInfo vmgenid_device_info = {
> > > + .name = VMGENID_DEVICE,
> > > + .parent = TYPE_SYS_BUS_DEVICE,
> > > + .instance_size = sizeof(VmGenIdState),
> > > + .class_init = vmgenid_class_init,
> > > +};
> > > +
> > > +static void vmgenid_register_types(void)
> > > +{
> > > + type_register_static(&vmgenid_device_info);
> > > +}
> > > +
> > > +type_init(vmgenid_register_types)
> > > +
> > ditto
> >
> > > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> > > index 77316d5..40ecccb 100644
> > > --- a/include/hw/i386/pc.h
> > > +++ b/include/hw/i386/pc.h
> > > @@ -290,6 +290,9 @@ void pc_system_firmware_init(MemoryRegion *rom_memory,
> > > /* pvpanic.c */
> > > uint16_t pvpanic_port(void);
> > >
> > > +/* vmgenid.c */
> > > +bool vm_generation_id(uint8_t id[16]);
> > > +
> > > /* e820 types */
> > > #define E820_RAM 1
> > > #define E820_RESERVED 2
> >
> >
> >