[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCHv4 1/5] fw_cfg: don't map the fw_cfg IO ports in
From: |
Laszlo Ersek |
Subject: |
Re: [Qemu-devel] [PATCHv4 1/5] fw_cfg: don't map the fw_cfg IO ports in fw_cfg_io_realize() |
Date: |
Fri, 16 Jun 2017 22:59:26 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.0 |
On 06/16/17 15:23, Mark Cave-Ayland wrote:
> As indicated by Laszlo it is a QOM bug for the realize() method to actually
> map the device. Set up the IO regions within fw_cfg_io_realize() and defer
> the mapping with sysbus_add_io() to the caller, as already done in
> fw_cfg_init_mem_wide().
>
> This makes the iobase and dma_iobase properties now obsolete so they can be
> removed.
>
> Signed-off-by: Mark Cave-Ayland <address@hidden>
> ---
> hw/nvram/fw_cfg.c | 15 ++++++++-------
> 1 file changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
> index 316fca9..70a0d5f 100644
> --- a/hw/nvram/fw_cfg.c
> +++ b/hw/nvram/fw_cfg.c
> @@ -936,24 +936,30 @@ FWCfgState *fw_cfg_init_io_dma(uint32_t iobase,
> uint32_t dma_iobase,
> AddressSpace *dma_as)
> {
> DeviceState *dev;
> + SysBusDevice *sbd;
> + FWCfgIoState *ios;
> FWCfgState *s;
> uint32_t version = FW_CFG_VERSION;
> bool dma_requested = dma_iobase && dma_as;
>
> dev = qdev_create(NULL, TYPE_FW_CFG_IO);
> - qdev_prop_set_uint32(dev, "iobase", iobase);
> - qdev_prop_set_uint32(dev, "dma_iobase", dma_iobase);
> if (!dma_requested) {
> qdev_prop_set_bit(dev, "dma_enabled", false);
> }
>
> fw_cfg_init1(dev);
> +
> + sbd = SYS_BUS_DEVICE(dev);
> + ios = FW_CFG_IO(dev);
> + sysbus_add_io(sbd, iobase, &ios->comb_iomem);
> +
> s = FW_CFG(dev);
>
> if (s->dma_enabled) {
> /* 64 bits for the address field */
> s->dma_as = dma_as;
> s->dma_addr = 0;
> + sysbus_add_io(sbd, dma_iobase, &s->dma_iomem);
>
> version |= FW_CFG_VERSION_DMA;
> }
> @@ -1059,8 +1065,6 @@ static void fw_cfg_file_slots_allocate(FWCfgState *s,
> Error **errp)
> }
>
> static Property fw_cfg_io_properties[] = {
> - DEFINE_PROP_UINT32("iobase", FWCfgIoState, iobase, -1),
> - DEFINE_PROP_UINT32("dma_iobase", FWCfgIoState, dma_iobase, -1),
> DEFINE_PROP_BOOL("dma_enabled", FWCfgIoState, parent_obj.dma_enabled,
> true),
> DEFINE_PROP_UINT16("x-file-slots", FWCfgIoState, parent_obj.file_slots,
> @@ -1071,7 +1075,6 @@ static Property fw_cfg_io_properties[] = {
> static void fw_cfg_io_realize(DeviceState *dev, Error **errp)
> {
> FWCfgIoState *s = FW_CFG_IO(dev);
> - SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
> Error *local_err = NULL;
>
> fw_cfg_file_slots_allocate(FW_CFG(s), &local_err);
> @@ -1085,13 +1088,11 @@ static void fw_cfg_io_realize(DeviceState *dev, Error
> **errp)
> * of the i/o region used is FW_CFG_CTL_SIZE */
> memory_region_init_io(&s->comb_iomem, OBJECT(s), &fw_cfg_comb_mem_ops,
> FW_CFG(s), "fwcfg", FW_CFG_CTL_SIZE);
> - sysbus_add_io(sbd, s->iobase, &s->comb_iomem);
>
> if (FW_CFG(s)->dma_enabled) {
> memory_region_init_io(&FW_CFG(s)->dma_iomem, OBJECT(s),
> &fw_cfg_dma_mem_ops, FW_CFG(s), "fwcfg.dma",
> sizeof(dma_addr_t));
> - sysbus_add_io(sbd, s->dma_iobase, &FW_CFG(s)->dma_iomem);
> }
> }
>
>
This patch looks good to me, but I think you should also remove the
"FWCfgIoState.iobase" and "FWCfgIoState.dma_iobase" fields altogether;
that is, from the definition of the "FWCfgIoState" structure. The fields
are no longer used after this patch. (And they are irrelevant for
migration too.)
With that update, you can add my
Reviewed-by: Laszlo Ersek <address@hidden>
Thanks
Laszlo
- [Qemu-devel] [PATCHv4 0/5] fw_cfg: qdev-related tidy-ups, Mark Cave-Ayland, 2017/06/16
- [Qemu-devel] [PATCHv4 3/5] fw_cfg: move assert() and linking of fw_cfg device to the machine into instance_init(), Mark Cave-Ayland, 2017/06/16
- [Qemu-devel] [PATCHv4 2/5] fw_cfg: move setting of FW_CFG_VERSION_DMA bit to fw_cfg_init1(), Mark Cave-Ayland, 2017/06/16
- [Qemu-devel] [PATCHv4 1/5] fw_cfg: don't map the fw_cfg IO ports in fw_cfg_io_realize(), Mark Cave-Ayland, 2017/06/16
- Re: [Qemu-devel] [PATCHv4 1/5] fw_cfg: don't map the fw_cfg IO ports in fw_cfg_io_realize(),
Laszlo Ersek <=
- [Qemu-devel] [PATCHv4 5/5] fw_cfg: move QOM type defines and fw_cfg types into fw_cfg.h, Mark Cave-Ayland, 2017/06/16
- [Qemu-devel] [PATCHv4 4/5] fw_cfg: move qdev_init_nofail() from fw_cfg_init1() to callers, Mark Cave-Ayland, 2017/06/16