On Thu, Jan 26, 2017 at 08:59:04PM +0200, Michael S. Tsirkin wrote: > On Thu, Jan 26, 2017 at 07:25:22PM +0100, Laszlo Ersek wrote: > > On 01/26/17 19:15, Michael S. Tsirkin wrote: > > > On Thu, Jan 26, 2017 at 06:43:22PM +0100, Laszlo Ersek wrote: > > >> On 01/26/17 16:20, Michael S. Tsirkin wrote: > > >>> On Thu, Jan 26, 2017 at 01:48:37AM +0100, Laszlo Ersek wrote: > > >> > > >>>> But, again, I'd like to keep COMMAND_ALLOCATE_RETURN_ADDR 8-byte wide. > > >>> > > >>> > > >>> What is COMMAND_ALLOCATE_RETURN_ADDR? I'm only familiar with > > >>> COMMAND_ALLOCATE. > > >> > > >> It's a new command being introduced in this series, at my suggestion. It > > >> does the exact same thing as COMMAND_ALLOCATE, except once the > > >> allocation / download is carried out by the firmware, the firmware > > >> writes back the allocation address to the fw_cfg file that is named in > > >> an additional field of the COMMAND_ALLOCATE_RETURN_ADDR structure. (This > > >> is how QEMU learns where the blob in GPA space was placed by the > > >> firmware.) The format for this address-receiving fw_cfg file is supposed > > >> to be 8-byte, little endian. > > > > > > I see. I really think it's better as a separate command though. > > > E.g. COMMAND_WRITE_PTR? > > > > Sure, but please provide specifics, otherwise Ben & myself will have a > > hard time divining & implementing your intent :) > > > > Thanks, > > Laszlo > > I would say a variant of ADD_POINTER: > > /* > * COMMAND_WRITE_POINTER - update a writeable file named > * @pointer.dest_file at @pointer.offset, by writing pointer to > * the table originating from @src_file. 1,2,4 or 8 byte > * unsigned write is used depending on @pointer.size. > */ > struct { > char dest_file[BIOS_LINKER_LOADER_FILESZ]; > char src_file[BIOS_LINKER_LOADER_FILESZ]; > uint32_t offset; > uint8_t size; > } pointer; >
I'm okay with this approach. If an offset is going to be added, shouldn't both a source offset and destination offset be used? /* * COMMAND_WRITE_POINTER - update a writeable file named * @pointer.dest_file at @pointer.dest_offset, by writing pointer * plus @pointer.src_offset to the blob originating from * @src_file. 1,2,4 or 8 byte unsigned write is used depending * on @pointer.size. */ struct { char dest_file[BIOS_LINKER_LOADER_FILESZ]; char src_file[BIOS_LINKER_LOADER_FILESZ]; uint32_t src_offset, dest_offset; uint8_t size; } pointer; I doubt the offsets or size is really all that important though. -Kevin