Re: [Qemu-devel] [RFC 4/7] enable fw_cfg dma for arm virt

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

On Tue, 21 Jul 2015 18:04:31 +0100
Peter Maydell <peter.maydell@linaro.org> wrote:

> On 21 July 2015 at 17:03, Marc Marí <markmb@redhat.com> wrote:
> > From: Gerd Hoffmann <kraxel@redhat.com>
> >
> > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> > ---
> >  docs/specs/fw_cfg.txt |  6 ++++++
> >  hw/arm/virt.c         | 11 ++++++++---
> >  2 files changed, 14 insertions(+), 3 deletions(-)
> >
> > diff --git a/docs/specs/fw_cfg.txt b/docs/specs/fw_cfg.txt
> > index 64d9192..eac83a1 100644
> > --- a/docs/specs/fw_cfg.txt
> > +++ b/docs/specs/fw_cfg.txt
> > @@ -167,6 +167,12 @@ If a partial transfer happened before an error
> > occured the address and length registers indicate how much data has
> > been transfered successfully.
> >
> > +== Register Locations ==
> > +
> > +=== arm ===
> > +
> > +classic fw_cfg base + 16
> > +
> >  = Host-side API =
> >
> >  The following functions are available to the QEMU programmer for
> > adding diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> > index 374660c..d05f6a4 100644
> > --- a/hw/arm/virt.c
> > +++ b/hw/arm/virt.c
> > @@ -606,13 +606,13 @@ static void create_flash(const VirtBoardInfo
> > *vbi) g_free(nodename);
> >  }
> >
> > -static void create_fw_cfg(const VirtBoardInfo *vbi)
> > +static void create_fw_cfg(AddressSpace *as, const VirtBoardInfo
> > *vbi) {
> >      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, 16, as);
> >
> >      nodename = g_strdup_printf("/fw-cfg@%" PRIx64, base);
> >      qemu_fdt_add_subnode(vbi->fdt, nodename);
> 
> Don't we need to also update the device tree (and the spec for
> that!) to indicate that we're providing a with-DMA fw-cfg
> device ?
> 
> > @@ -800,6 +800,7 @@ static void machvirt_init(MachineState *machine)
> >      VirtGuestInfoState *guest_info_state = g_malloc0(sizeof
> > *guest_info_state); VirtGuestInfo *guest_info =
> > &guest_info_state->info; char **cpustr;
> > +    AddressSpace *as = NULL;
> >
> >      if (!cpu_model) {
> >          cpu_model = "cortex-a15";
> > @@ -837,6 +838,10 @@ static void machvirt_init(MachineState
> > *machine) }
> >          cpuobj = object_new(object_class_get_name(oc));
> >
> > +        if (!as) {
> > +            as = CPU(cpuobj)->as;
> > +        }
> 
> Is this really the right AddressSpace to use? Fishing
> around in the CPU object seems a bit dubious...
> 

I have not touched this part, I touched the x86 part, but the same
comment applies in both places:

I don't know much about how and why address spaces are used in QEMU, so
I'm not really sure which address space should I use for this purpose.
I'd appreciate some help in the issue.

Thanks
Marc