Re: [Qemu-devel] [PATCH v3 4/5] Enable fw_cfg DMA interface for ARM

["Marc Marí" <markmb@redhat.com>]

On Sat, 19 Sep 2015 01:10:46 +0200
Laszlo Ersek <lersek@redhat.com> wrote:

> On 09/18/15 22:24, Marc Marí wrote:
> > On Fri, 18 Sep 2015 22:16:46 +0200
> > Laszlo Ersek <lersek@redhat.com> wrote:
> > 
> >> On 09/18/15 10:58, Marc Marí wrote:
> >>> Enable the fw_cfg DMA interface for the ARM virt machine.
> >>>
> >>> Based on Gerd Hoffman's initial implementation.
> >>>
> >>> Signed-off-by: Marc Marí <markmb@redhat.com>
> >>> ---
> >>>  hw/arm/virt.c | 9 +++++----
> >>>  1 file changed, 5 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> >>> index 3568107..47f4ad3 100644
> >>> --- a/hw/arm/virt.c
> >>> +++ b/hw/arm/virt.c
> >>> @@ -113,7 +113,7 @@ static const MemMapEntry a15memmap[] = {
> >>>      [VIRT_GIC_V2M] =            { 0x08020000, 0x00001000 },
> >>>      [VIRT_UART] =               { 0x09000000, 0x00001000 },
> >>>      [VIRT_RTC] =                { 0x09010000, 0x00001000 },
> >>> -    [VIRT_FW_CFG] =             { 0x09020000, 0x0000000a },
> >>> +    [VIRT_FW_CFG] =             { 0x09020000, 0x00000014 },
> >>
> >> Okay, Laszlo is the hateful reviewer. Sorry about that. I'm late,
> >> yes.
> >>
> >> But: this says 0x00000014, ie 20 bytes in decimal. I don't think
> >> that's correct; it should be 0x18 -- 24 bytes in decimal. From
> >> patch #2: "DMA Address address:       Base + 16 (8 bytes)".
> > 
> > It's not your problem if I don't know how to count. So don't
> > apologize :).
> > 
> > And it's better to catch this stupid little mistakes now.
> 
> Got some good news: with those two fixups in place (register block
> size corrected, and dma_enabled set via device property), I could
> test the AAVMF / ArmVirtPkg / <insert your favorite synonym here>
> patches.
> 
> On my APM Mustang, downloading a decompressed kernel (14,475,776
> bytes), a decompressed initrd (18,177,264), and a cmdline (104
> bytes :)), in total 32,653,144 bytes, takes approx. 24 seconds with
> the 8-byte wide MMIO data register. (Yeah, it's *really* slow.)
> 
> Using the DMA interface, the same takes about 52 milliseconds, and
> that still includes one progress message per 1 MB downloaded :)
> 
> It's a factor of approx. 450. Not bad. Not bad. :)

Not bad. Not bad :). In x86 the speedup is high but not so brutal. I'm
really happy that it works so well.

Thanks
Marc

> Thanks
> Laszlo
> 
> 
> > Thanks
> > Marc
> > 
> >> Thanks (and I'm sorry about being late!)
> >> Laszlo
> >>
> >>>      [VIRT_MMIO] =               { 0x0a000000, 0x00000200 },
> >>>      /* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of
> >>> that size */ [VIRT_PLATFORM_BUS] =       { 0x0c000000,
> >>> 0x02000000 }, @@ -651,13 +651,13 @@ static void
> >>> create_flash(const VirtBoardInfo *vbi) g_free(nodename);
> >>>  }
> >>>  
> >>> -static void create_fw_cfg(const VirtBoardInfo *vbi)
> >>> +static void create_fw_cfg(const VirtBoardInfo *vbi, AddressSpace
> >>> *as) {
> >>>      hwaddr base = vbi->memmap[VIRT_FW_CFG].base;
> >>>      hwaddr size = vbi->memmap[VIRT_FW_CFG].size;
> >>>      char *nodename;
> >>>  
> >>> -    fw_cfg_init_mem_wide(base + 8, base, 8, 0, NULL);
> >>> +    fw_cfg_init_mem_wide(base + 8, base, 8, base + 16, as);
> >>>  
> >>>      nodename = g_strdup_printf("/fw-cfg@%" PRIx64, base);
> >>>      qemu_fdt_add_subnode(vbi->fdt, nodename);
> >>> @@ -919,6 +919,7 @@ static void machvirt_init(MachineState
> >>> *machine) 
> >>>      create_fdt(vbi);
> >>>  
> >>> +
> >>>      for (n = 0; n < smp_cpus; n++) {
> >>>          ObjectClass *oc = cpu_class_by_name(TYPE_ARM_CPU,
> >>> cpustr[0]); CPUClass *cc = CPU_CLASS(oc);
> >>> @@ -984,7 +985,7 @@ static void machvirt_init(MachineState
> >>> *machine) */
> >>>      create_virtio_devices(vbi, pic);
> >>>  
> >>> -    create_fw_cfg(vbi);
> >>> +    create_fw_cfg(vbi, &address_space_memory);
> >>>      rom_set_fw(fw_cfg_find());
> >>>  
> >>>      guest_info->smp_cpus = smp_cpus;
> >>>
> >>
> > 
>