> On Jan 27, 2017, at 8:12 AM, Laszlo Ersek <ler...@redhat.com> wrote: > > 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? > Yes, this seems reasonable. If I’m interpreting this properly, it’s like ADD_POINTER, except that we extend the write path back to QEMU over DMA. Is that about right?
> Thanks! > Laszlo —Ben
smime.p7s
Description: S/MIME cryptographic signature