On Tue, 21 Jul 2015 18:04:31 +0100
Peter Maydell <peter.mayd...@linaro.org> wrote:
> On 21 July 2015 at 17:03, Marc Marí <mar...@redhat.com> wrote:
> > From: Gerd Hoffmann <kra...@redhat.com>
> >
> > Signed-off-by: Gerd Hoffmann <kra...@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
