On 01/27/17 16:43, 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
I know no specific reason; I just thought this possible generalization
was one of the advantages in Michael's suggestion.
> as well as why it would want support for
> pointers smaller than 8 bytes in size.
Hm, right, good point.
> 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.
That might be the main reason I guess; reading back a bit, Michael wrote
"... a variant of ADD_POINTER".
> I'm okay with either format.
I'd say let's go ahead with Michael's proposal then. Ben, are you okay
with that?
Thanks!
Laszlo