On 02/07/17 13:11, Igor Mammedov wrote: > On Mon, 6 Feb 2017 19:31:24 +0200 > "Michael S. Tsirkin" <m...@redhat.com> wrote: > >> On Mon, Feb 06, 2017 at 09:16:25AM -0800, Ben Warren wrote: >>>>> @@ -257,8 +263,11 @@ void bios_linker_loader_add_pointer(BIOSLinker >>>>> *linker, >>>>> const BiosLinkerFileEntry *source_file = >>>>> bios_linker_find_file(linker, src_file); >>>>> >>>>> - assert(dst_patched_offset < dst_file->blob->len); >>>>> - assert(dst_patched_offset + dst_patched_size <= dst_file->blob->len); >>>>> + /* dst_file need not exist if writing back */ >>>> >>>> Why not? >>> Because WRITE_POINTER can be called without having first called >>> ALLOCATE. In the Vm Generation ID example, there’s no reason for BIOS >>> to allocate memory for the address fw_cfg, since it’s a construct that >>> only matters to QEMU. This is something that was requested by Laszlo. >> >> Well all other commands require you to first allocate. >> How does bios know e.g. which zone to put it in then? > There is no need to know which zone to put it in as file > is writeback only, i.e. there is no need to allocate > RAM for file (shadow it) which won't be read by guest side ever > if I got idea right. > Issue in the patch is combining new command with existing API > bios_linker_loader_add_pointer(), > as you suggested new API function + good description what it does > would be better than reusing.
Agreed on all points (and sorry about the delayed response): - The writeable fw_cfg file that receives the address from the guest firmware need not and should not be downloaded by the guest firmware, into guest memory. Such a blob, in guest memory, is entirely useless to the guest. - I also strongly prefer a separate command structure, and function name, for the new command. (The structure can be shared by a typedef or another sub-structure, but it should have its own separate struct tag / type name.) Thanks! Laszlo > > >> >> I don't like the asymmetry ... Laszlo? > >
