[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH V15 4/5] i386: add a Virtual Machine Generation
From: |
Michael S. Tsirkin |
Subject: |
Re: [Qemu-devel] [PATCH V15 4/5] i386: add a Virtual Machine Generation ID device |
Date: |
Wed, 3 Jun 2015 18:32:00 +0200 |
On Fri, May 29, 2015 at 01:46:44PM +0200, Igor Mammedov wrote:
> On Mon, 27 Apr 2015 14:19:50 +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 and its implementation.
> >
> > The GUID is set using a global "vmgenid.uuid" parameter.
> >
> > Signed-off-by: Gal Hammer <address@hidden>
> > ---
> > default-configs/i386-softmmu.mak | 1 +
> > default-configs/x86_64-softmmu.mak | 1 +
> > hw/i386/acpi-build.c | 41 +++++++++++++
> > hw/i386/pc.c | 8 +++
> > hw/misc/Makefile.objs | 1 +
> > hw/misc/vmgenid.c | 116
> > +++++++++++++++++++++++++++++++++++++
> > include/hw/i386/pc.h | 3 +
> > include/hw/misc/vmgenid.h | 21 +++++++
> > 8 files changed, 192 insertions(+)
> > 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 6a74e00..9fc0a3c 100644
> > --- a/default-configs/i386-softmmu.mak
> > +++ b/default-configs/i386-softmmu.mak
> > @@ -45,3 +45,4 @@ CONFIG_MEM_HOTPLUG=y
> > CONFIG_XIO3130=y
> > CONFIG_IOH3420=y
> > CONFIG_I82801B11=y
> > +CONFIG_VMGENID=y
> > diff --git a/default-configs/x86_64-softmmu.mak
> > b/default-configs/x86_64-softmmu.mak
> > index 46b87dd..f66fd3c 100644
> > --- a/default-configs/x86_64-softmmu.mak
> > +++ b/default-configs/x86_64-softmmu.mak
> > @@ -45,3 +45,4 @@ CONFIG_MEM_HOTPLUG=y
> > CONFIG_XIO3130=y
> > CONFIG_IOH3420=y
> > CONFIG_I82801B11=y
> > +CONFIG_VMGENID=y
> > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> > index e761005..8d1a761 100644
> > --- a/hw/i386/acpi-build.c
> > +++ b/hw/i386/acpi-build.c
> > @@ -42,6 +42,7 @@
> > #include "hw/acpi/memory_hotplug.h"
> > #include "sysemu/tpm.h"
> > #include "hw/acpi/tpm.h"
> > +#include "hw/misc/vmgenid.h"
> >
> > /* Supported chipsets: */
> > #include "hw/acpi/piix4.h"
> > @@ -116,6 +117,7 @@ typedef struct AcpiMiscInfo {
> > unsigned dsdt_size;
> > uint16_t pvpanic_port;
> > uint16_t applesmc_io_base;
> > + bool vm_generation_id_set;
> > } AcpiMiscInfo;
> >
> > typedef struct AcpiBuildPciBusHotplugState {
> > @@ -242,6 +244,7 @@ static void acpi_get_misc_info(AcpiMiscInfo *info)
> > info->has_tpm = tpm_find();
> > info->pvpanic_port = pvpanic_port();
> > info->applesmc_io_base = applesmc_port();
> > + info->vm_generation_id_set = vm_generation_id_set();
> > }
> >
> > static void acpi_get_pci_info(PcPciInfo *info)
> > @@ -815,6 +818,44 @@ build_ssdt(GArray *table_data, GArray *linker,
> > aml_append(ssdt, scope);
> > }
> >
> > + if (misc->vm_generation_id_set) {
> would be better to make it conditional depending on if vmgenid device is
> present
>
> > + scope = aml_scope("\\_SB");
> > +
> > + dev = aml_device("VMGI");
>
> [...]
>
> > +
> > + vmgenid = qdev_try_create(NULL, VMGENID_DEVICE);
> > + if (vmgenid) {
> > + qdev_init_nofail(vmgenid);
> > + sysbus_mmio_map(SYS_BUS_DEVICE(vmgenid), 0, VMGENID_BASE_ADDRESS);
> > + }
> it should be possible not to create device if user haven't asked for it,
> my guess is that you are using sysbus device as a convenience for mapping.
>
> I'd suggest to use plain Device type instead, with it
> vmgenid would be created when management asks for it using CLI:
> -device vmgenid,uuid=XXXX
> and from mgmt side it would treat device as any other device using the same
> APIs.
>
> Mapping device to board is also simple, for example see:
>
> pc_machine_device_plug_cb()->pc_dimm_plug()->memory_region_add_subregion()
>
> for PC machine pc_machine_device_plug_cb() callback is called for
> every bus-less device during device_realize() time
> hw/core/qdev.c:hotplug_handler_plug(hotplug_ctrl, dev, &local_err);
>
> you just need to add something like:
> pc_vmgenid_plug()
> addr = object_property_get_int(dev, VMGENID_BASE_ADDR_PROP, &local_err);
> VMGenIdDeviceClass vmgidc = CAST(dev)
> memory_region_add_subregion(&system_memory, addr,
> vmgidc->get_memory_region());
>
> [...]
>
Yes, I think it is best not to add devices by default, -device is safer.
> > +
> > +#define VMGENID_BASE_ADDRESS 0xfedf0000
> I'd make it a property, so that every target would be able to set
> its own value when times come and ACPI part would automatically pick it up,
> it also helps with test cases.
I think there's no need for this change as we plan
to modify this code down the road anyway.
> > +#define VMGENID_BASE_ADDR_LEN 16
> > +
> > +#endif
- Re: [Qemu-devel] [PATCH V15 4/5] i386: add a Virtual Machine Generation ID device,
Michael S. Tsirkin <=
- Re: [Qemu-devel] [PATCH V15 4/5] i386: add a Virtual Machine Generation ID device, Paolo Bonzini, 2015/06/03
- Re: [Qemu-devel] [PATCH V15 4/5] i386: add a Virtual Machine Generation ID device, Gal Hammer, 2015/06/08
- Re: [Qemu-devel] [PATCH V15 4/5] i386: add a Virtual Machine Generation ID device, Paolo Bonzini, 2015/06/08
- Re: [Qemu-devel] [PATCH V15 4/5] i386: add a Virtual Machine Generation ID device, Gal Hammer, 2015/06/08
- Re: [Qemu-devel] [PATCH V15 4/5] i386: add a Virtual Machine Generation ID device, Paolo Bonzini, 2015/06/08
- Re: [Qemu-devel] [PATCH V15 4/5] i386: add a Virtual Machine Generation ID device, Daniel P. Berrange, 2015/06/08
- Re: [Qemu-devel] [PATCH V15 4/5] i386: add a Virtual Machine Generation ID device, Gal Hammer, 2015/06/08
- Re: [Qemu-devel] [PATCH V15 4/5] i386: add a Virtual Machine Generation ID device, Michael S. Tsirkin, 2015/06/08
- Re: [Qemu-devel] [PATCH V15 4/5] i386: add a Virtual Machine Generation ID device, Paolo Bonzini, 2015/06/08
- Re: [Qemu-devel] [PATCH V15 4/5] i386: add a Virtual Machine Generation ID device, Michael S. Tsirkin, 2015/06/08