Thanks Laszlo! > On Jan 19, 2017, at 1:25 AM, Laszlo Ersek <ler...@redhat.com> wrote: > > On 01/19/17 08:09, Ben Warren wrote: >> >>> On Jan 18, 2017, at 4:02 PM, Ben Warren <b...@skyportsystems.com> >>> wrote: >>> >>> Hi Michael, >>>> On Jan 17, 2017, at 9:45 AM, Michael S. Tsirkin <m...@redhat.com> >>>> 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