Re: [Qemu-devel] [PATCH v5 01/11] fw_cfg: hard separation between the MMIO and I/O port mappings

[Paolo Bonzini <pbonzini@redhat.com>]

On 18/12/2014 13:58, Peter Maydell wrote:
> On 18 December 2014 at 11:18, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>
>>
>> On 17/12/2014 22:10, Laszlo Ersek wrote:
>>> +static Property fw_cfg_mem_properties[] = {
>>> +    DEFINE_PROP_UINT64("ctl_addr", FWCfgMemState, ctl_addr, -1),
>>> +    DEFINE_PROP_UINT64("data_addr", FWCfgMemState, data_addr, -1),
>>> +    DEFINE_PROP_END_OF_LIST(),
>>> +};
>>> +
>>> +static void fw_cfg_mem_realize(DeviceState *dev, Error **errp)
>>> +{
>>> +    FWCfgMemState *s = FW_CFG_MEM(dev);
>>> +    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
>>> +
>>> +    memory_region_init_io(&s->ctl_iomem, OBJECT(s), &fw_cfg_ctl_mem_ops,
>>> +                          FW_CFG(s), "fwcfg.ctl", FW_CFG_SIZE);
>>> +    sysbus_init_mmio(sbd, &s->ctl_iomem);
>>> +    sysbus_mmio_map(sbd, 0, s->ctl_addr);
>>> +
>>> +    memory_region_init_io(&s->data_iomem, OBJECT(s), &fw_cfg_data_mem_ops,
>>> +                          FW_CFG(s), "fwcfg.data", FW_CFG_DATA_SIZE);
>>> +    sysbus_init_mmio(sbd, &s->data_iomem);
>>> +    sysbus_mmio_map(sbd, 1, s->data_addr);
>>> +}
>>
>> Strictly speaking sysbus_mmio_map should be called by the caller, as in
>> the old fw_cfg_init---which lets you drop the properties too.
> 
> I'd really rather we got this correct -- mmio_map in a realize
> function is wrong and if we merge this I'll have to immediately
> write a bug fix patch to move it out again...

Laszlo is on vacation, I'll prepare a fixup patch tomorrow.

Paolo