Re: [Qemu-devel] [PATCH v4 1/9] ACPI: Add a function for building named qword entries

[Michael S. Tsirkin]

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).
size is to mimick ADD_POINTER.

-- 
MST