[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCHv9 3/3] fw_cfg: move QOM type defines and fw_cfg
From: |
Igor Mammedov |
Subject: |
Re: [Qemu-devel] [PATCHv9 3/3] fw_cfg: move QOM type defines and fw_cfg types into fw_cfg.h |
Date: |
Mon, 17 Jul 2017 13:03:11 +0200 |
On Sun, 16 Jul 2017 20:12:13 +0100
Mark Cave-Ayland <address@hidden> wrote:
> On 14/07/17 19:56, Eduardo Habkost wrote:
>
> >>> Why do you need the full struct declaration to be exposed in the
> >>> header?
> >>
> >> Different board code wants to hook up "comb_iomem" manually to different
> >> address spaces, so they need to access the field directly. This is the
> >> ultimate goal of the entire exercise, IIRC.
>
> Yes, that is correct (and I believe is mentioned in the cover letter, too).
>
> >>> The memory regions are supposed to be visible as QOM
> >>> children to the fw_cfg device, already.
> >>
> >> I don't understand this. How else can board code work with "comb_iomem"
> >> than described above? If there is a way, I agree it would be preferable.
> >
> > object_resolve_path_component(fw_cfg, "fwcfg[0]") and
> > object_resolve_path_component(fw_cfg, "fwcfg.dma[0]") should
> > return fw_cfg->comb_iomem and fw_cfg->dma_iomem, respectively.
> >
> > I don't know why those names were chosen, though. Probably it's
> > a good idea to call object_property_add_child() manually with
> > more appropriate names inside the fw_cfg code instead of letting
> > memory_region_init() pick the child name.
>
> That's interesting. I did a grep of the codebase for
> object_resolve_path_component and struggled to find an instance where it
> was being used to provide access to a MemoryRegion. Even if it's an
> available feature, it's certainly not one that is widely known about.
Agreed,
we haven't used suggested approach before and I don't see
benefits it will bring in fwcfg case.
Eduardo,
Could you just apply v9, which seems to be good enough and
does the intended job before 2.10 goes into soft-freeze?
we always could do object_resolve_path_component() on top
if it makes sense.
>
> In terms of the patch, is the v9 revision suitable for commit or does
> this constitute a NACK?
>
>
> ATB,
>
> Mark.
- [Qemu-devel] [PATCHv9 1/3] fw_cfg: switch fw_cfg_find() to locate the fw_cfg device by type rather than path, (continued)