> On Feb 15, 2017, at 7:24 AM, Laszlo Ersek <ler...@redhat.com> wrote:
>
> On 02/15/17 13:19, Igor Mammedov wrote:
>> On Tue, 14 Feb 2017 22:15:46 -0800
>> b...@skyportsystems.com wrote:
>>
>>> From: Ben Warren <b...@skyportsystems.com>
>>>
>>> This implements the VM Generation ID feature by passing a 128-bit
>>> GUID to the guest via a fw_cfg blob.
>>> Any time the GUID changes, an ACPI notify event is sent to the guest
>>>
>>> The user interface is a simple device with one parameter:
>>> - guid (string, must be "auto" or in UUID format
>>> xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx)
>>>
>>> Signed-off-by: Ben Warren <b...@skyportsystems.com>
>>> ---
>>> default-configs/i386-softmmu.mak | 1 +
>>> default-configs/x86_64-softmmu.mak | 1 +
>>> hw/acpi/Makefile.objs | 1 +
>>> hw/acpi/vmgenid.c | 237
>>> +++++++++++++++++++++++++++++++++++
>>> hw/i386/acpi-build.c | 16 +++
>>> include/hw/acpi/acpi_dev_interface.h | 1 +
>>> include/hw/acpi/vmgenid.h | 35 ++++++
>>> 7 files changed, 292 insertions(+)
>>> create mode 100644 hw/acpi/vmgenid.c
>>> create mode 100644 include/hw/acpi/vmgenid.h
>>>
>>> diff --git a/default-configs/i386-softmmu.mak
>>> b/default-configs/i386-softmmu.mak
>>> index 48b07a4..029e952 100644
>>> --- a/default-configs/i386-softmmu.mak
>>> +++ b/default-configs/i386-softmmu.mak
>>> @@ -59,3 +59,4 @@ CONFIG_I82801B11=y
>>> CONFIG_SMBIOS=y
>>> CONFIG_HYPERV_TESTDEV=$(CONFIG_KVM)
>>> CONFIG_PXB=y
>>> +CONFIG_ACPI_VMGENID=y
>>> diff --git a/default-configs/x86_64-softmmu.mak
>>> b/default-configs/x86_64-softmmu.mak
>>> index fd96345..d1d7432 100644
>>> --- a/default-configs/x86_64-softmmu.mak
>>> +++ b/default-configs/x86_64-softmmu.mak
>>> @@ -59,3 +59,4 @@ CONFIG_I82801B11=y
>>> CONFIG_SMBIOS=y
>>> CONFIG_HYPERV_TESTDEV=$(CONFIG_KVM)
>>> CONFIG_PXB=y
>>> +CONFIG_ACPI_VMGENID=y
>>> diff --git a/hw/acpi/Makefile.objs b/hw/acpi/Makefile.objs
>>> index 6acf798..11c35bc 100644
>>> --- a/hw/acpi/Makefile.objs
>>> +++ b/hw/acpi/Makefile.objs
>>> @@ -5,6 +5,7 @@ common-obj-$(CONFIG_ACPI_CPU_HOTPLUG) += cpu_hotplug.o
>>> common-obj-$(CONFIG_ACPI_MEMORY_HOTPLUG) += memory_hotplug.o
>>> common-obj-$(CONFIG_ACPI_CPU_HOTPLUG) += cpu.o
>>> common-obj-$(CONFIG_ACPI_NVDIMM) += nvdimm.o
>>> +common-obj-$(CONFIG_ACPI_VMGENID) += vmgenid.o
>>> common-obj-$(call lnot,$(CONFIG_ACPI_X86)) += acpi-stub.o
>>>
>>> common-obj-y += acpi_interface.o
>>> diff --git a/hw/acpi/vmgenid.c b/hw/acpi/vmgenid.c
>>> new file mode 100644
>>> index 0000000..b1b7b32
>>> --- /dev/null
>>> +++ b/hw/acpi/vmgenid.c
>>> @@ -0,0 +1,237 @@
>>> +/*
>>> + * Virtual Machine Generation ID Device
>>> + *
>>> + * Copyright (C) 2017 Skyport Systems.
>>> + *
>>> + * Author: Ben Warren <b...@skyportsystems.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 "qemu/osdep.h"
>>> +#include "qmp-commands.h"
>>> +#include "hw/acpi/acpi.h"
>>> +#include "hw/acpi/aml-build.h"
>>> +#include "hw/acpi/vmgenid.h"
>>> +#include "hw/nvram/fw_cfg.h"
>>> +#include "sysemu/sysemu.h"
>>> +
>>> +void vmgenid_build_acpi(VmGenIdState *vms, GArray *table_data, GArray
>>> *guid,
>>> + BIOSLinker *linker)
>>> +{
>>> + Aml *ssdt, *dev, *scope, *method, *addr, *if_ctx;
>>> + uint32_t vgia_offset;
>>> + QemuUUID guid_le;
>>> +
>>> + /* Fill in the GUID values. These need to be converted to
>>> little-endian
>>> + * first, since that's what the guest expects
>>> + */
>>> + g_array_set_size(guid, VMGENID_FW_CFG_SIZE);
>>> + memcpy(&guid_le.data, &vms->guid.data, sizeof(vms->guid.data));
>>> + qemu_uuid_bswap(&guid_le);
>>> + /* The GUID is written at a fixed offset into the fw_cfg file
>>> + * in order to implement the "OVMF SDT Header probe suppressor"
>>> + * see docs/specs/vmgenid.txt for more details
>>> + */
>>> + g_array_insert_vals(guid, VMGENID_GUID_OFFSET, guid_le.data,
>>> + ARRAY_SIZE(guid_le.data));
>
> Ben:
>
> (1) The logic is sane here, but the initial sizing of the array is not
> correct. The initial size should be
>
> (VMGENID_FW_CFG_SIZE - ARRAY_SIZE(guid_le.data))
>
> The reason for this is that g_array_insert_vals() really inserts (it
> doesn't overwrite) data, therefore it grows the array. From the GLib
> source code [glib/garray.c]:
>
> ------------------
> GArray*
> g_array_insert_vals (GArray *farray,
> guint index_,
> gconstpointer data,
> guint len)
> {
> GRealArray *array = (GRealArray*) farray;
>
> g_return_val_if_fail (array, NULL);
>
> g_array_maybe_expand (array, len);
>
> memmove (g_array_elt_pos (array, len + index_),
> g_array_elt_pos (array, index_),
> g_array_elt_len (array, array->len - index_));
>
> memcpy (g_array_elt_pos (array, index_), data, g_array_elt_len (array,
> len));
>
> array->len += len;
>
> g_array_zero_terminate (array);
>
> return farray;
> }
> ------------------
>
> This is an extremely minor wart, because later on:
>
Fixed in v7. Thanks for pointing this out.
>>> +
>>> + /* Put this in a separate SSDT table */
>>> + ssdt = init_aml_allocator();
>>> +
>>> + /* Reserve space for header */
>>> + acpi_data_push(ssdt->buf, sizeof(AcpiTableHeader));
>>> +
>>> + /* Storage for the GUID address */
>>> + vgia_offset = table_data->len +
>>> + build_append_named_dword(ssdt->buf, "VGIA");
>>> + scope = aml_scope("\\_SB");
>>> + dev = aml_device("VGEN");
>>> + aml_append(dev, aml_name_decl("_HID", aml_string("QEMUVGID")));
>>> + aml_append(dev, aml_name_decl("_CID", aml_string("VM_Gen_Counter")));
>>> + aml_append(dev, aml_name_decl("_DDN", aml_string("VM_Gen_Counter")));
>>> +
>>> + /* Simple status method to check that address is linked and non-zero */
>>> + method = aml_method("_STA", 0, AML_NOTSERIALIZED);
>>> + addr = aml_local(0);
>>> + aml_append(method, aml_store(aml_int(0xf), addr));
>>> + if_ctx = aml_if(aml_equal(aml_name("VGIA"), aml_int(0)));
>>> + aml_append(if_ctx, aml_store(aml_int(0), addr));
>>> + aml_append(method, if_ctx);
>>> + aml_append(method, aml_return(addr));
>>> + aml_append(dev, method);
>>> +
>>> + /* the ADDR method returns two 32-bit words representing the lower and
>>> + * upper halves * of the physical address of the fw_cfg blob
>>> + * (holding the GUID)
>>> + */
>>> + method = aml_method("ADDR", 0, AML_NOTSERIALIZED);
>>> +
>>> + addr = aml_local(0);
>>> + aml_append(method, aml_store(aml_package(2), addr));
>>> +
>>> + aml_append(method, aml_store(aml_add(aml_name("VGIA"),
>>> + aml_int(VMGENID_GUID_OFFSET),
>>> NULL),
>>> + aml_index(addr, aml_int(0))));
>>> + aml_append(method, aml_store(aml_int(0), aml_index(addr, aml_int(1))));
>> Just curious,
>> so suggested in v5 simple declaration style
>>
>> Package(2) {
>> ADD(VGIA, VMGENID_GUID_OFFSET),
>> 0
>> }
>>
>> wasn't working for you?
>>
>>> + aml_append(method, aml_return(addr));
>>> +
>>> + aml_append(dev, method);
>>> + aml_append(scope, dev);
>>> + aml_append(ssdt, scope);
>>> +
>>> + /* attach an ACPI notify */
>>> + method = aml_method("\\_GPE._E05", 0, AML_NOTSERIALIZED);
>>> + aml_append(method, aml_notify(aml_name("\\_SB.VGEN"), aml_int(0x80)));
>>> + aml_append(ssdt, method);
>>> +
>>> + g_array_append_vals(table_data, ssdt->buf->data, ssdt->buf->len);
>>> +
>>> + /* Allocate guest memory for the Data fw_cfg blob */
>>> + bios_linker_loader_alloc(linker, VMGENID_GUID_FW_CFG_FILE, guid, 4096,
>>> + false /* page boundary, high memory */);
>>> +
>>> + /* Patch address of GUID fw_cfg blob into the ADDR fw_cfg blob
>>> + * so QEMU can write the GUID there. The address is expected to be
>>> + * < 4GB, but write 64 bits anyway.
>>> + */
>>> + bios_linker_loader_write_pointer(linker,
>>> + VMGENID_ADDR_FW_CFG_FILE, 0, sizeof(uint64_t),
>>> + VMGENID_GUID_FW_CFG_FILE);
>>> +
>>> + /* Patch address of GUID fw_cfg blob into the AML so OSPM can retrieve
>>> + * and read it. Note that while we provide storage for 64 bits, only
>>> + * the least-signficant 32 get patched into AML.
>>> + */
>>> + bios_linker_loader_add_pointer(linker,
>>> + ACPI_BUILD_TABLE_FILE, vgia_offset, sizeof(uint32_t),
>>> + VMGENID_GUID_FW_CFG_FILE, 0);
>>> +
>>> + build_header(linker, table_data,
>>> + (void *)(table_data->data + table_data->len - ssdt->buf->len),
>>> + "SSDT", ssdt->buf->len, 1, NULL, "VMGENID");
>>> + free_aml_allocator();
>>> +}
>>> +
>>> +void vmgenid_add_fw_cfg(VmGenIdState *vms, FWCfgState *s, GArray *guid)
>>> +{
>>> + /* Create a read-only fw_cfg file for GUID */
>>> + fw_cfg_add_file(s, VMGENID_GUID_FW_CFG_FILE, guid->data,
>>> + VMGENID_FW_CFG_SIZE);
>
> we do expose the correct size to the guest (and the underlying QEMU-side
> allocation is larger, not smaller than that, so it is safe).
>
> So I guess I could call this an "innocent leak of 16 bytes" -- it's up
> to you if you want to fix it. (I really don't want to obsess about this
> at v6, I could have noticed the exact same in v5!)
>
>>> + /* Create a read-write fw_cfg file for Address */
>>> + fw_cfg_add_file_callback(s, VMGENID_ADDR_FW_CFG_FILE, NULL, NULL,
>>> + vms->vmgenid_addr_le,
>>> + ARRAY_SIZE(vms->vmgenid_addr_le), false);
>>> +}
>>> +
>>> +static void vmgenid_update_guest(VmGenIdState *vms)
>>> +{
>>> + Object *obj = object_resolve_path_type("", TYPE_ACPI_DEVICE_IF, NULL);
>>> + uint32_t vmgenid_addr;
>>> + QemuUUID guid_le;
>>> +
>>> + if (obj) {
>>> + /* Write the GUID to guest memory */
>>> + memcpy(&vmgenid_addr, vms->vmgenid_addr_le, sizeof(vmgenid_addr));
>>> + vmgenid_addr = le32_to_cpu(vmgenid_addr);
>>> + /* A zero value in vmgenid_addr means that BIOS has not yet written
>>> + * the address
>>> + */
>>> + if (vmgenid_addr) {
>>> + /* QemuUUID has the first three words as big-endian, and expect
>>> + * that any GUIDs passed in will always be BE. The guest,
>>> + * however, will expect the fields to be little-endian.
>>> + * Perform a byte swap immediately before writing.
>>> + */
>>> + memcpy(&guid_le.data, &vms->guid.data, sizeof(vms->guid.data));
>> potential stack corruption if guid_le and vms->guid types ever diverge
>> why not just do
>> guid_le.data = guid.data
>
> Igor:
>
> That would be an array assignment, and wouldn't work:
>
> typedef struct {
> union {
> unsigned char data[16];
> struct {
> /* Generated in BE endian, can be swapped with
> qemu_uuid_bswap. */
> uint32_t time_low;
> uint16_t time_mid;
> uint16_t time_high_and_version;
> uint8_t clock_seq_and_reserved;
> uint8_t clock_seq_low;
> uint8_t node[6];
> } fields;
> };
> } QemuUUID;
>
> I think the code is safe: both the local variable "guid_le" and
> "VmGenIdState.guid" are declared with type "QemuUUID".
>
> I agree that a style improvement would be
>
> guid_le = vms->guid;
>
Changed to this in v7
> since structure assignment is okay.
>
> Personally I feel neutrally about this.
>
>>
>>> + qemu_uuid_bswap(&guid_le);
>>> + /* The GUID is written at a fixed offset into the fw_cfg file
>>> + * in order to implement the "OVMF SDT Header probe suppressor"
>>> + * see docs/specs/vmgenid.txt for more details
>>> + */
>>> + cpu_physical_memory_write(vmgenid_addr + VMGENID_GUID_OFFSET,
>>> + guid_le.data, sizeof(guid_le.data));
>>> + /* Send _GPE.E05 event */
>>> + acpi_send_event(DEVICE(obj), ACPI_VMGENID_CHANGE_STATUS);
>>> + }
>>> + }
>>> +}
>>> +
>>> +static void vmgenid_set_guid(Object *obj, const char *value, Error **errp)
>>> +{
>>> + VmGenIdState *vms = VMGENID(obj);
>>> +
>>> + if (!strcmp(value, "auto")) {
>>> + qemu_uuid_generate(&vms->guid);
>>> + } else if (qemu_uuid_parse(value, &vms->guid) < 0) {
>>> + error_setg(errp, "'%s. %s': Failed to parse GUID string: %s",
>>> + object_get_typename(OBJECT(vms)), VMGENID_GUID, value);
>>> + return;
>>> + }
>>> +
>>> + vmgenid_update_guest(vms);
>>> +}
>>> +
>>> +/* After restoring an image, we need to update the guest memory and notify
>>> + * it of a potential change to VM Generation ID
>>> + */
>>> +static int vmgenid_post_load(void *opaque, int version_id)
>>> +{
>>> + VmGenIdState *vms = opaque;
>>> + vmgenid_update_guest(vms);
>>> + return 0;
>>> +}
>>> +
>>> +static const VMStateDescription vmstate_vmgenid = {
>>> + .name = "vmgenid",
>>> + .version_id = 1,
>>> + .minimum_version_id = 1,
>>> + .post_load = vmgenid_post_load,
>>> + .fields = (VMStateField[]) {
>>> + VMSTATE_UINT8_ARRAY(vmgenid_addr_le, VmGenIdState,
>>> sizeof(uint64_t)),
>>> + VMSTATE_END_OF_LIST()
>>> + },
>>> +};
>>> +
>>> +static void vmgenid_initfn(Object *obj)
>>> +{
>>> + object_property_add_str(obj, VMGENID_GUID, NULL, vmgenid_set_guid,
>>> NULL);
>> missing:
>> object_property_set_description()
>> or even better use class properties here:
>>
>> object_class_property_add_str()/object_class_property_set_description()
>>
>>> +}
>>> +
>>> +static void vmgenid_handle_reset(void *opaque)
>>> +{
>>> + VmGenIdState *vms = VMGENID(opaque);
>>> + /* Clear the guest-allocated GUID address when the VM resets */
>>> + memset(vms->vmgenid_addr_le, 0, ARRAY_SIZE(vms->vmgenid_addr_le));
>>> +}
>>> +
>>> +static void vmgenid_realize(DeviceState *dev, Error **errp)
>>> +{
>>> + VmGenIdState *vms = VMGENID(dev);
>>> + qemu_register_reset(vmgenid_handle_reset, vms);
>>> +}
>>> +
>>> +static void vmgenid_device_class_init(ObjectClass *klass, void *data)
>>> +{
>>> + DeviceClass *dc = DEVICE_CLASS(klass);
>>> +
>>> + dc->vmsd = &vmstate_vmgenid;
>>> + dc->realize = vmgenid_realize;
>> it needs:
>>
>> dc->hotpluggable = false;
>>
>>> +}
>>> +
>>> +static const TypeInfo vmgenid_device_info = {
>>> + .name = VMGENID_DEVICE,
>>> + .parent = TYPE_DEVICE,
>>> + .instance_size = sizeof(VmGenIdState),
>>> + .instance_init = vmgenid_initfn,
>>> + .class_init = vmgenid_device_class_init,
>>> +};
>>> +
>>> +static void vmgenid_register_types(void)
>>> +{
>>> + type_register_static(&vmgenid_device_info);
>>> +}
>>> +
>>> +type_init(vmgenid_register_types)
>>> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
>>> index 1c928ab..db04cf5 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/acpi/vmgenid.h"
>>> #include "sysemu/tpm_backend.h"
>>> #include "hw/timer/mc146818rtc_regs.h"
>>> #include "sysemu/numa.h"
>>> @@ -2610,6 +2611,7 @@ void acpi_build(AcpiBuildTables *tables, MachineState
>>> *machine)
>>> size_t aml_len = 0;
>>> GArray *tables_blob = tables->table_data;
>>> AcpiSlicOem slic_oem = { .id = NULL, .table_id = NULL };
>>> + Object *vmgenid_dev;
>>>
>>> acpi_get_pm_info(&pm);
>>> acpi_get_misc_info(&misc);
>>> @@ -2653,6 +2655,13 @@ void acpi_build(AcpiBuildTables *tables,
>>> MachineState *machine)
>>> acpi_add_table(table_offsets, tables_blob);
>>> build_madt(tables_blob, tables->linker, pcms);
>>>
>>> + vmgenid_dev = find_vmgenid_dev();
>>> + if (vmgenid_dev) {
>>> + acpi_add_table(table_offsets, tables_blob);
>>> + vmgenid_build_acpi(VMGENID(vmgenid_dev), tables_blob,
>>> + tables->vmgenid, tables->linker);
>>> + }
>>> +
>>> if (misc.has_hpet) {
>>> acpi_add_table(table_offsets, tables_blob);
>>> build_hpet(tables_blob, tables->linker);
>>> @@ -2823,6 +2832,7 @@ void acpi_setup(void)
>>> PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
>>> AcpiBuildTables tables;
>>> AcpiBuildState *build_state;
>>> + Object *vmgenid_dev;
>>>
>>> if (!pcms->fw_cfg) {
>>> ACPI_BUILD_DPRINTF("No fw cfg. Bailing out.\n");
>>> @@ -2859,6 +2869,12 @@ void acpi_setup(void)
>>> fw_cfg_add_file(pcms->fw_cfg, ACPI_BUILD_TPMLOG_FILE,
>>> tables.tcpalog->data, acpi_data_len(tables.tcpalog));
>>>
>>> + vmgenid_dev = find_vmgenid_dev();
>>> + if (vmgenid_dev) {
>>> + vmgenid_add_fw_cfg(VMGENID(vmgenid_dev), pcms->fw_cfg,
>>> + tables.vmgenid);
>>> + }
>>> +
>>> if (!pcmc->rsdp_in_ram) {
>>> /*
>>> * Keep for compatibility with old machine types.
>>> diff --git a/include/hw/acpi/acpi_dev_interface.h
>>> b/include/hw/acpi/acpi_dev_interface.h
>>> index 71d3c48..3c2e4e9 100644
>>> --- a/include/hw/acpi/acpi_dev_interface.h
>>> +++ b/include/hw/acpi/acpi_dev_interface.h
>>> @@ -11,6 +11,7 @@ typedef enum {
>>> ACPI_CPU_HOTPLUG_STATUS = 4,
>>> ACPI_MEMORY_HOTPLUG_STATUS = 8,
>>> ACPI_NVDIMM_HOTPLUG_STATUS = 16,
>>> + ACPI_VMGENID_CHANGE_STATUS = 32,
>>> } AcpiEventStatusBits;
>>>
>>> #define TYPE_ACPI_DEVICE_IF "acpi-device-interface"
>>> diff --git a/include/hw/acpi/vmgenid.h b/include/hw/acpi/vmgenid.h
>>> new file mode 100644
>>> index 0000000..db7fa0e
>>> --- /dev/null
>>> +++ b/include/hw/acpi/vmgenid.h
>>> @@ -0,0 +1,35 @@
>>> +#ifndef ACPI_VMGENID_H
>>> +#define ACPI_VMGENID_H
>>> +
>>> +#include "hw/acpi/bios-linker-loader.h"
>>> +#include "hw/qdev.h"
>>> +#include "qemu/uuid.h"
>>> +
>>> +#define VMGENID_DEVICE "vmgenid"
>>> +#define VMGENID_GUID "guid"
>>> +#define VMGENID_GUID_FW_CFG_FILE "etc/vmgenid_guid"
>>> +#define VMGENID_ADDR_FW_CFG_FILE "etc/vmgenid_addr"
>>> +
>>> +#define VMGENID_FW_CFG_SIZE 4096 /* Occupy a page of memory */
>>> +#define VMGENID_GUID_OFFSET 40 /* allow space for
>>> + * OVMF SDT Header Probe Supressor
>>> + */
>>> +
>>> +#define VMGENID(obj) OBJECT_CHECK(VmGenIdState, (obj), VMGENID_DEVICE)
>>> +
>>> +typedef struct VmGenIdState {
>>> + DeviceClass parent_obj;
>>> + QemuUUID guid; /* The 128-bit GUID seen by the guest */
>>> + uint8_t vmgenid_addr_le[8]; /* Address of the GUID (little-endian) */
>>> +} VmGenIdState;
>>> +
>>> +static inline Object *find_vmgenid_dev(void)
>>> +{
>>> + return object_resolve_path_type("", VMGENID_DEVICE, NULL);
>> What will happen if CLI would be:
>> -device vmgenid -device vmgenid
>> As I understand, it should be exclusive single instance device,
>> and there is nothing to prevent second instance of it.
>
> Igor:
>
> I agree with your observation, but I don't think it's a show-stopper
> (not at this point anyway). I think it is similar to the case that I
> raised earlier, about <= 2.6 machine types that have no DMA support in
> fw_cfg, hence WRITE_POINTER cannot work on them.
>
> Both of these cases (i.e., too early machine types, and multiple vmgenid
> devices) come from "pathologic" command lines, and don't prevent the
> intended use of the device (assuming a correct command line). So, I
> think it should be safe to address these questions later, in a followup
> series (for 2.10, likely).
>
> Ben:
>
> Summary:
> - the sizing wart that I mentioned under (1) is innocent; it doesn't
> deserve a repost on its own. If you do a v7, I suggest that you fix it
> up, but I don't insist.
>
> - Personally I'm fine with the rest. I see that Igor made some comments,
> but I feel that a good chunk of those could have been made for v5 just
> the same (example: dc->hotpluggable, object_property_set_description() /
> class properties). I wouldn't like to delay this series any longer.
> Those improvements can be added later, IMO -- but please do work out
> with Igor whether he really wants a v7 for those.
>
> I'm fine with the patch as-is, and I'm also fine with it if Igor's
> comments are addressed:
>
> Reviewed-by: Laszlo Ersek <ler...@redhat.com <mailto:ler...@redhat.com>>
>
> If you do other changes though, please drop my R-b.
>
Thanks so much for all the reviews. I’ve left off your R-b because I ended up
using the ‘src_offset’ argument for write_pointer(). Other than that, all
recommended changes are made.
> ... I'd like to look at the rest of this series a little, and then I'll
> try to come back with test results (with OVMF).
>
> Thanks!
> Laszlo
>
>>
>>
>>> +}
>>> +
>>> +void vmgenid_build_acpi(VmGenIdState *vms, GArray *table_data, GArray
>>> *guid,
>>> + BIOSLinker *linker);
>>> +void vmgenid_add_fw_cfg(VmGenIdState *vms, FWCfgState *s, GArray *guid);
>>> +
>>> +#endif
smime.p7s
Description: S/MIME cryptographic signature
