Re: [Qemu-devel] [PATCHv9 3/3] fw_cfg: move QOM type defines and fw_cfg types into fw_cfg.h

[Eduardo Habkost <ehabkost@redhat.com>]

On Mon, Jul 17, 2017 at 01:03:11PM +0200, Igor Mammedov wrote:
> On Sun, 16 Jul 2017 20:12:13 +0100
> Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> 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.

I was hoping Michael would apply it (as fw_cfg is currently
closer to PC code than to i386 or machine core).  But I can apply
it if he didn't queue it yet.

-- 
Eduardo