Re: [Qemu-devel] [PATCH 2/2] i386: Add a Virtual Machine Generation ID device.

[Gal Hammer <ghammer@redhat.com>]

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

    Gal.

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

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

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

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


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