On Fri, Feb 10, 2017 at 11:12:13AM +0100, Laszlo Ersek wrote: > On 02/05/17 10:11, b...@skyportsystems.com wrote: > > From: Ben Warren <b...@skyportsystems.com> > > > > This patch set adds support for passing a GUID to Windows guests. It is a > > re-implementation of previous patch sets written by Igor Mammedov et al, but > > this time passing the GUID data as a fw_cfg blob. > > > > This patch set has dependencies on new guest functionality, in particular > > the > > support for a new linker-loader command and the ability to write back data > > to QEMU over a DMA link. Work is in flight in both SeaBIOS and OVMF to > > support this. > > > > v4->v5: > > - Added significantly more detail to the documentation. > > - Replaced the previously-implemented linker-loader command with a new > > one: > > "write pointer". This allows writing the guest address of a fw_cfg > > blob back > > to an arbitrary offset in a writeable fw_cfg file visible to QEMU. > > This will > > require support in SeaBIOS and OVMF (ongoing). > > - Fixed endianness issues throughout. > > - Several styling cleanups. > > Low importance question, but I'll ask it anyway: > > Can we somehow make the realization of "-device vmgenid" fail, if the PC > machine type in use is 2.6 or earlier? Because, those machine types do > not have DMA enabled for fw_cfg. Hence the WRITE_POINTER commands cannot > be effectively executed by firmware (the underlying virtual hardware is > not enabled). > > For example, if a user adds VMGENID to a 2.5 machine type, and migrates > the VM, then on the target host, > > vmgenid_post_load() > vmgenid_update_guest() > > will not be effective, because (back on the source host) the guest had > no way to write the address of the downloaded "etc/vmgenid" blob to > "etc/vmgenid_addr". > > I don't think it's a big deal, but perhaps we can be user-friendly a > bit.
The most user-friendly thing would be to DWIM and enable DMA. But that would be hard to implement so probably not worth the trouble. > Things that crossed my mind (different levels of ugliness): > > - in the realize function for vmgenid (which I've proposed anyway, for > registering the reset handler), we could look up the fw_cfg object, and > see if its "dma-enabled" property is on or off. Possible problem: not > sure if such cross-device checks are allowed in realize methods, and if > there are any ordering issues for this. (I.e., when vmgenid's realize > checks, has the property been set for fw_cfg?) > > - or else, add another boolean property to vmgenid, one that parallels > "dma-enabled" of "fw-cfg" precisely, in HW_COMPAT*. Then simply fail > realize() when this property is false. That's probably the easiest way. x-fw-cfg-dma-enabled. Won't delay merging because of this, can be done with patch on top. > There are probably better ways... > > Anyway, it's not a big deal, I think this can be addressed incrementally > (or not at all, perhaps). I don't want to delay the series with it, just > throwing it out there. Yes, pls do not delay because of this. > BTW, I tested the OVMF patches against v5, and it looks good. I > cross-referenced the firmware log with acpidump in the guest, plus I > used dump-guest-memory and the "crash" utility to check the memory > contents for the guest, before/after setting the GUID with the monitor > command. > > I'd like to verify S3 too, but for that I'd like to use v6 (which I hope > clears "vgia_le" on reset). > > Thanks, > Laszlo > > > > > v3->v4: > > - Rebased to top of tree. > > - Re-added document patch that was accidentally dropped from the last > > revision. > > - Added VMState functionality so that VGIA is restored properly. > > - Added Unit tests > > v2->v3: > > - Added second writeable fw_cfg for storing the VM Generaiton ID > > address. This uses a new linker-loader command for instructing the > > guest to write back the allocated address. A patch for SeaBIOS has > > been > > submitted > > (https://www.seabios.org/pipermail/seabios/2017-January/011079.html) > > and the resulting binary will need to be pulled into QEMU once > > accepted. > > - Setting VM Generation ID by command line or qmp/hmp now accepts an > > "auto" > > value, whereby QEMU generates a random GUID. > > - Incorporated review comments from v2 mainly around code styling and > > AML syntax > > - Changed to use the E05 ACPI event instead of E00 > > v1->v2: > > - Removed "changed" boolean parameter as it is unneeded > > - Added ACPI Notify logic > > - Style changes to pass checkpatch.pl > > - Added support for dynamic sysbus to pc_piix boards > > > > > > Ben Warren (8): > > ACPI: Add a function for building named qword entries > > linker-loader: Add new 'write pointer' command > > docs: VM Generation ID device description > > ACPI: Add vmgenid storage entries to the build tables > > ACPI: Add Virtual Machine Generation ID support > > PC: Support dynamic sysbus on pc_i440fx > > tests: Move reusable ACPI macros into a new header file > > tests: Add unit tests for the VM Generation ID feature > > > > Igor Mammedov (2): > > qmp/hmp: add query-vm-generation-id and 'info vm-generation-id' > > commands > > qmp/hmp: add set-vm-generation-id commands > > > > default-configs/i386-softmmu.mak | 1 + > > default-configs/x86_64-softmmu.mak | 1 + > > docs/specs/vmgenid.txt | 239 > > +++++++++++++++++++++++++++++++++++ > > hmp-commands-info.hx | 13 ++ > > hmp-commands.hx | 13 ++ > > hmp.c | 21 +++ > > hmp.h | 2 + > > hw/acpi/Makefile.objs | 1 + > > hw/acpi/aml-build.c | 39 +++++- > > hw/acpi/bios-linker-loader.c | 35 +++-- > > hw/acpi/nvdimm.c | 2 +- > > hw/acpi/vmgenid.c | 238 > > ++++++++++++++++++++++++++++++++++ > > hw/arm/virt-acpi-build.c | 4 +- > > hw/i386/acpi-build.c | 18 ++- > > hw/i386/pc_piix.c | 1 + > > include/hw/acpi/acpi_dev_interface.h | 1 + > > include/hw/acpi/aml-build.h | 5 + > > include/hw/acpi/bios-linker-loader.h | 3 +- > > include/hw/acpi/vmgenid.h | 37 ++++++ > > qapi-schema.json | 31 +++++ > > stubs/Makefile.objs | 1 + > > stubs/vmgenid.c | 14 ++ > > tests/Makefile.include | 2 + > > tests/acpi-utils.h | 75 +++++++++++ > > tests/bios-tables-test.c | 72 +---------- > > tests/vmgenid-test.c | 184 +++++++++++++++++++++++++++ > > 26 files changed, 962 insertions(+), 91 deletions(-) > > create mode 100644 docs/specs/vmgenid.txt > > create mode 100644 hw/acpi/vmgenid.c > > create mode 100644 include/hw/acpi/vmgenid.h > > create mode 100644 stubs/vmgenid.c > > create mode 100644 tests/acpi-utils.h > > create mode 100644 tests/vmgenid-test.c > >
