Re: [Qemu-devel] Virtual Machine Generation ID

[Ben Warren]

Thanks Laszlo!
> On Jan 19, 2017, at 1:25 AM, Laszlo Ersek <address@hidden> wrote:
> 
> On 01/19/17 08:09, Ben Warren wrote:
>> 
>>> On Jan 18, 2017, at 4:02 PM, Ben Warren <address@hidden>
>>> wrote:
>>> 
>>> Hi Michael,
>>>> On Jan 17, 2017, at 9:45 AM, Michael S. Tsirkin <address@hidden>
>>>> wrote:
>>>> 
>>>> On Mon, Jan 16, 2017 at 10:57:42AM -0800, Ben Warren wrote:
>>>>> I think we have a misunderstanding here.  I'm storing the VM
>>>>> Generation ID __data__ (a GUID) in a fw_cfg blob, not the address.
>>>> 
>>>> Yes, I think I gathered this much from the discussion. This is what
>>>> I'm saying - don't. Have guest loader reserve guest memory and write
>>>> the address into a fw cfg blob, and have qemu write the id at that
>>>> address. This way you can update guest memory at any time.
>>>> 
>>> So I've gone down the path of creating a writeable fw_cfg blob to
>>> hold the VGID address, but it doesn't seem to be getting updated.
>>> 
>>> Here's the code I've added:
>>> 
>>> #define VMGENID_FW_CFG_FILE      "etc/vmgenid"
>>> #define VMGENID_FW_CFG_ADDR_FILE      "etc/vmgenid_addr"
>>> 
>>> // Create writeable fw_cfg blob, vas->vgia is a GArray of size 8 and 
>>> element size 1
>>> fw_cfg_add_file_callback(s, VMGENID_FW_CFG_ADDR_FILE, NULL, NULL, 
>>> vms->vgia->data, 8, false);
>>> 
>>> // Request BIOS to allocate memory for the read-only DATA file:
>>> bios_linker_loader_alloc(linker, VMGENID_FW_CFG_FILE, guid, 0,false);
>>> 
>>> // Request BIOS to allocate memory for the writeable ADDRESS file:
>>> bios_linker_loader_alloc(linker, VMGENID_FW_CFG_ADDR_FILE, s->vgia, 0, 
>>> false);
>>> 
>>> // Request BIOS to write the address of the DATA file into the ADDRESS file:
>>> bios_linker_loader_add_pointer(linker, VMGENID_FW_CFG_ADDR_FILE, 0, 
>>> sizeof(uint64_t), VMGENID_FW_CFG_FILE, 0);
>>> 
>>> I've instrumented SeaBIOS and see the requests being made and memcpy
>>> to the file happening, but don't see any changes in QEMU in the
>>> memory pointed to by VMGENID_FW_CFG_ADDR_FILE.  Is this how writeable
>>> fw_cfg is supposed to work?
>>> 
>> Ed explained it to me and I think I get it now.  I realize that you
>> and Igor have explained this throughout the e-mail chain, but I'm a
>> little bit slower.
>> 
>> Anyway, is this understanding correct?  BIOS is in fact patching
>> fw_cfg data, but it's in a copy of the fw_cfg file that is in guest
>> space.  In order to get this to work we would need to add a new
>> command to the linker/loader protocol to write back changes to QEMU
>> after patching, and of course implement the change in BIOS and UEFI
>> projects.
> 
> (1) It's not enough to create just the "etc/vmgenid_addr" file; the
> "etc/vmgenid" file must exist too, so that the firmware can download it.
> 
I do have that file too, just didn’t show it.
> (2) I don't understand why you need the ADD_POINTER command here. I
> think it's unnecessary.
> 
> (3) The ALLOCATE command for "etc/vmgenid_addr" is also unnecessary.
> 
For both of these, I was working within the confines of the existing API, which 
we now know is inadequate.
> (4) A new ALLOCATE_RETURN_ADDR command type should be introduced, with
> the following contents:
> 
>        struct {
>            char file[BIOS_LINKER_LOADER_FILESZ];
>            uint32_t align;
>            uint8_t zone;
>            char addr_file[BIOS_LINKER_LOADER_FILESZ];
>        } alloc_return_addr;
> 
> The first three fields have identical meanings to those of the current
> ALLOCATE command. The last field instructs the firmware as to what
> fw_cfg file to write the 8-byte physical address back to, in little
> endian byte order, of the actual allocation.
> 
> (5) The linker/loader script should then use one command in total,
> namely ALLOCATE_RETURN_ADDR, with
> 
>  file = etc/vmgenid
>  addr_file = etc/vmgenid_addr
> 
> This will allow both SeaBIOS and OVMF to do the right thing.
> 
> In summary:
> - create the read-only "etc/vmgenid" fw_cfg file
> - create the writeable "etc/vmgenid_addr" fw_cfg file
> - use one ALLOCATE_RETURN_ADDR command, and nothing else.
> 
> I hereby volunteer to write the OVMF patch for the ALLOCATE_RETURN_ADDR
> command type.
> 
Sounds great.  We use SeaBIOS so I’ll try to do the same there.

So, just to make sure everything’s covered: this new mechanism will allow QEMU 
to have the guest allocate an arbitrary blob of memory (in the form of a fw_cfg 
file), and will get an offset within guest memory in return (via another fw_cfg 
file).  It can then translate the guest offset into a host address and update 
the blob at will.  Is this correct, because that’s what we need for VM 
Generation ID?

> If someone is interested, I'll remind us that EFI_ACPI_TABLE_PROTOCOL
> installs *copies* of ACPI tables, out of the fw_cfg blobs that were
> downloaded. Therefore OVMF tracks all ADD_POINTER commands that point
> into fw_cfg blobs, and if an fw_cfg blob is not pointed-to by *any*
> ADD_POINTER command, or else *all* ADD_POINTER commands that point into
> it point to ACPI table headers, then OVMF will ultimately free the
> originally downloaded fw_cfg blob. If there is at least one referring
> ADD_POINTER command that points to non-ACPI-table data within the blob,
> then OVMF marks the blob to be preserved permanently, in AcpiNVS type
> memory.
> 
> By introducing the above ALLOCATE_RETURN_ADDR command type, OVMF can do
> two things:
> 
> (a) immediately mark the blob for permanent preservation (i.e., it won't
>    be released after the script processing is complete),
> (b) write the actual allocation address back to the appropriate fw_cfg
>    file.
> 
> For SeaBIOS, only (b) matters -- because it doesn't install *copies* of
> ACPI tables; it rather keeps everything in place, as originally
> allocated, and it just links things together --, but
> ALLOCATE_RETURN_ADDR as proposed above should be suitable for SeaBIOS
> just as well.
> 
> Thanks
> Laszlo
—Ben

smime.p7s
Description: S/MIME cryptographic signature