[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-ppc] [PATCH v2 08/18] hw/nvram/fw_cfg: Move fw_cfg_file_slots_
From: |
Laszlo Ersek |
Subject: |
Re: [Qemu-ppc] [PATCH v2 08/18] hw/nvram/fw_cfg: Move fw_cfg_file_slots_allocate() to common_realize() |
Date: |
Fri, 8 Mar 2019 11:19:12 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 |
On 03/08/19 02:32, Philippe Mathieu-Daudé wrote:
> Each implementation (I/O and MEM) calls fw_cfg_file_slots_allocate()
> then fw_cfg_common_realize().
> Simplify by moving the fw_cfg_file_slots_allocate() call into
> fw_cfg_common_realize() where it belongs.
The patch does more than that, namely:
>
> Signed-off-by: Philippe Mathieu-Daudé <address@hidden>
> ---
> hw/nvram/fw_cfg.c | 17 ++++++++++-------
> 1 file changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
> index 0fb020edce..ca58d279a4 100644
> --- a/hw/nvram/fw_cfg.c
> +++ b/hw/nvram/fw_cfg.c
> @@ -929,19 +929,26 @@ static void fw_cfg_machine_ready(struct Notifier *n,
> void *data)
> qemu_register_reset(fw_cfg_machine_reset, s);
> }
>
> -
> +static void fw_cfg_file_slots_allocate(FWCfgState *s, Error **errp);
>
> static void fw_cfg_common_realize(DeviceState *dev, Error **errp)
> {
> FWCfgState *s = FW_CFG(dev);
> MachineState *machine = MACHINE(qdev_get_machine());
> uint32_t version = FW_CFG_VERSION;
> + Error *local_err = NULL;
>
> if (!fw_cfg_find()) {
> error_setg(errp, "at most one %s device is permitted", TYPE_FW_CFG);
> return;
> }
>
> + fw_cfg_file_slots_allocate(s, &local_err);
> + if (local_err) {
> + error_propagate(errp, local_err);
> + return;
> + }
> +
> fw_cfg_add_bytes(s, FW_CFG_SIGNATURE, (char *)"QEMU", 4);
> fw_cfg_add_bytes(s, FW_CFG_UUID, &qemu_uuid, 16);
> fw_cfg_add_i16(s, FW_CFG_NOGRAPHIC, (uint16_t)!machine->enable_graphics);
> @@ -1108,7 +1115,7 @@ static void fw_cfg_io_realize(DeviceState *dev, Error
> **errp)
> FWCfgIoState *s = FW_CFG_IO(dev);
> Error *local_err = NULL;
>
> - fw_cfg_file_slots_allocate(FW_CFG(s), &local_err);
> + fw_cfg_common_realize(dev, &local_err);
> if (local_err) {
> error_propagate(errp, local_err);
> return;
> @@ -1125,8 +1132,6 @@ static void fw_cfg_io_realize(DeviceState *dev, Error
> **errp)
> &fw_cfg_dma_mem_ops, FW_CFG(s), "fwcfg.dma",
> sizeof(dma_addr_t));
> }
> -
> - fw_cfg_common_realize(dev, errp);
> }
it moves the fw_cfg_common_realize() call from the ends of the IO/MEM
realize functions to their middles; in particular to the point before
memory regions and (when appropriate) MMIO resources are initialized.
In other words, the patch reorders all of the current actions in
fw_cfg_common_realize() against said MemoryRegion & SysBusDevice
resource actions.
Why is this safe? (I'm not claiming it is unsafe, but I'd like to see it
explained in the commit message.)
Note: the fw_cfg_common_realize() you are moving around was introduced
in commit 38f3adc34de8 ("fw_cfg: move qdev_init_nofail() from
fw_cfg_init1() to callers", 2017-07-17), by Mark Cave-Ayland. I see Mark
is CC'd already, so I hope he can comment.
Thanks,
Laszlo
>
> static void fw_cfg_io_class_init(ObjectClass *klass, void *data)
> @@ -1162,7 +1167,7 @@ static void fw_cfg_mem_realize(DeviceState *dev, Error
> **errp)
> const MemoryRegionOps *data_ops = &fw_cfg_data_mem_ops;
> Error *local_err = NULL;
>
> - fw_cfg_file_slots_allocate(FW_CFG(s), &local_err);
> + fw_cfg_common_realize(dev, &local_err);
> if (local_err) {
> error_propagate(errp, local_err);
> return;
> @@ -1189,8 +1194,6 @@ static void fw_cfg_mem_realize(DeviceState *dev, Error
> **errp)
> sizeof(dma_addr_t));
> sysbus_init_mmio(sbd, &FW_CFG(s)->dma_iomem);
> }
> -
> - fw_cfg_common_realize(dev, errp);
> }
>
> static void fw_cfg_mem_class_init(ObjectClass *klass, void *data)
>
- [Qemu-ppc] [PATCH v2 06/18] hw/nvram/fw_cfg: Remove the unnecessary boot_splash_filedata_size, (continued)
- [Qemu-ppc] [PATCH v2 08/18] hw/nvram/fw_cfg: Move fw_cfg_file_slots_allocate() to common_realize(), Philippe Mathieu-Daudé, 2019/03/07
- Re: [Qemu-ppc] [PATCH v2 08/18] hw/nvram/fw_cfg: Move fw_cfg_file_slots_allocate() to common_realize(),
Laszlo Ersek <=
- [Qemu-ppc] [PATCH v2 09/18] hw/nvram/fw_cfg: Free file_slots in common_unrealize(), Philippe Mathieu-Daudé, 2019/03/07
- [Qemu-ppc] [PATCH v2 10/18] hw/nvram/fw_cfg: Add reboot_timeout to FWCfgState, Philippe Mathieu-Daudé, 2019/03/07
- Re: [Qemu-ppc] [PATCH v2 10/18] hw/nvram/fw_cfg: Add reboot_timeout to FWCfgState, Michael S. Tsirkin, 2019/03/08
[Qemu-ppc] [PATCH v2 11/18] hw/nvram/fw_cfg: Add boot_splash.time_le16 to FWCfgState, Philippe Mathieu-Daudé, 2019/03/07
[Qemu-ppc] [PATCH v2 12/18] hw/nvram/fw_cfg: Keep reference of file_data in FWCfgState, Philippe Mathieu-Daudé, 2019/03/07