On 18 December 2014 at 11:18, Paolo Bonzini <pbonz...@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... -- PMM