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. (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. (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. 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