On Mon, 30 Jan 2017 22:28:41 +0200 "Michael S. Tsirkin" <m...@redhat.com> wrote:
> On Fri, Jan 27, 2017 at 10:43:13AM -0500, Kevin O'Connor wrote: > > On Fri, Jan 27, 2017 at 03:46:33PM +0100, Laszlo Ersek wrote: > > > On 01/27/17 15:18, Kevin O'Connor wrote: > > > > 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. > > > > > > The offset into the fw_cfg file that receives the allocation address is > > > important, that allows the same file to receive several different > > > addresses (for different downloaded blobs), at different offsets. > > > > > > OTOH, asking the firmware to add a constant to the address value before > > > writing it to the fw_cfg file is not necessary, in my opinion. The blob > > > that the firmware allocated and downloaded originates from QEMU to begin > > > with, so QEMU knows its internal structure. > > > > I guess I'm missing why QEMU would want to use the same writable file > > for multiple pointers as well as why it would want support for > > pointers smaller than 8 bytes in size. If it's because it may be > > easier to support an internal QEMU blob of a particular format, then > > adding a src_offset would facilitate that. > > > > However, if it was done so that WRITE_POINTER mimicks ADD_POINTER then > > that's fine too. I'm okay with either format. > > > > -Kevin > > Both reasons :) offset is because it's easier for QEMU not to have to add > more files (e.g. it simplifies cross-version migration if we don't). On one hand offset simplifies since one file could be re-used for several pointers, on the other hand it doesn't make difference wrt migration since offset becomes ABI and has to be maintained in cross-version migration scenario (size of file shouldn't be issue as they are re-sizable now). So we just end-up with offset vs new file versioning. However considering that number of files is limited, offset scales up better. > size is to mimick ADD_POINTER. >