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

[Mark Cave-Ayland]

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.

In terms of the patch, is the v9 revision suitable for commit or does
this constitute a NACK?


ATB,

Mark.