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-ayl...@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