| Message ID | 1497097821-32754-4-git-send-email-mark.cave-ayland@ilande.co.uk |
|---|---|
| State | New |
| Headers | show |
On 06/10/2017 09:30 AM, Mark Cave-Ayland wrote: > The dma_enabled property enables us to set the FW_CFG_ID version > accordingly. > > Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > --- > hw/nvram/fw_cfg.c | 16 +++++++--------- > 1 file changed, 7 insertions(+), 9 deletions(-) > > diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c > index 1313bfd..f7b78a9 100644 > --- a/hw/nvram/fw_cfg.c > +++ b/hw/nvram/fw_cfg.c > @@ -914,12 +914,19 @@ static void fw_cfg_init1(DeviceState *dev) > { > FWCfgState *s = FW_CFG(dev); > MachineState *machine = MACHINE(qdev_get_machine()); > + uint32_t version = FW_CFG_VERSION; > > assert(!object_resolve_path(FW_CFG_PATH, NULL)); > > object_property_add_child(OBJECT(machine), FW_CFG_NAME, OBJECT(s), NULL); > > fw_cfg_add_bytes(s, FW_CFG_SIGNATURE, (char *)"QEMU", 4); > + > + if (s->dma_enabled) { > + version |= FW_CFG_VERSION_DMA; > + } > + > + fw_cfg_add_i32(s, FW_CFG_ID, version); > fw_cfg_add_bytes(s, FW_CFG_UUID, &qemu_uuid, 16); > fw_cfg_add_i16(s, FW_CFG_NOGRAPHIC, (uint16_t)!machine->enable_graphics); > fw_cfg_add_i16(s, FW_CFG_BOOT_MENU, (uint16_t)boot_menu); > @@ -935,7 +942,6 @@ FWCfgState *fw_cfg_init_io_dma(uint32_t iobase, uint32_t dma_iobase, > { > DeviceState *dev; > FWCfgState *s; > - uint32_t version = FW_CFG_VERSION; > bool dma_requested = dma_iobase && dma_as; > > dev = qdev_create(NULL, TYPE_FW_CFG_IO); > @@ -954,12 +960,8 @@ FWCfgState *fw_cfg_init_io_dma(uint32_t iobase, uint32_t dma_iobase, > /* 64 bits for the address field */ > s->dma_as = dma_as; > s->dma_addr = 0; > - > - version |= FW_CFG_VERSION_DMA; > } > > - fw_cfg_add_i32(s, FW_CFG_ID, version); > - > return s; > } > > @@ -975,7 +977,6 @@ FWCfgState *fw_cfg_init_mem_wide(hwaddr ctl_addr, > DeviceState *dev; > SysBusDevice *sbd; > FWCfgState *s; > - uint32_t version = FW_CFG_VERSION; > bool dma_requested = dma_addr && dma_as; > > dev = qdev_create(NULL, TYPE_FW_CFG_MEM); > @@ -997,11 +998,8 @@ FWCfgState *fw_cfg_init_mem_wide(hwaddr ctl_addr, > s->dma_as = dma_as; > s->dma_addr = 0; > sysbus_mmio_map(sbd, 2, dma_addr); > - version |= FW_CFG_VERSION_DMA; > } > > - fw_cfg_add_i32(s, FW_CFG_ID, version); > - > return s; > } > >
On Sat, 10 Jun 2017 13:30:18 +0100 Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> wrote: > The dma_enabled property enables us to set the FW_CFG_ID version > accordingly. it might be not safe as patch reorders FW_CFG_ID entry. (I recall there were patches that sorted fwcfg entries, but it's worth checking that it won't break migration) CCing Gerd, maybe he would recall how it works. > > Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> > --- > hw/nvram/fw_cfg.c | 16 +++++++--------- > 1 file changed, 7 insertions(+), 9 deletions(-) > > diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c > index 1313bfd..f7b78a9 100644 > --- a/hw/nvram/fw_cfg.c > +++ b/hw/nvram/fw_cfg.c > @@ -914,12 +914,19 @@ static void fw_cfg_init1(DeviceState *dev) > { > FWCfgState *s = FW_CFG(dev); > MachineState *machine = MACHINE(qdev_get_machine()); > + uint32_t version = FW_CFG_VERSION; > > assert(!object_resolve_path(FW_CFG_PATH, NULL)); > > object_property_add_child(OBJECT(machine), FW_CFG_NAME, OBJECT(s), NULL); > > fw_cfg_add_bytes(s, FW_CFG_SIGNATURE, (char *)"QEMU", 4); > + > + if (s->dma_enabled) { > + version |= FW_CFG_VERSION_DMA; > + } > + > + fw_cfg_add_i32(s, FW_CFG_ID, version); > fw_cfg_add_bytes(s, FW_CFG_UUID, &qemu_uuid, 16); > fw_cfg_add_i16(s, FW_CFG_NOGRAPHIC, (uint16_t)!machine->enable_graphics); > fw_cfg_add_i16(s, FW_CFG_BOOT_MENU, (uint16_t)boot_menu); > @@ -935,7 +942,6 @@ FWCfgState *fw_cfg_init_io_dma(uint32_t iobase, uint32_t dma_iobase, > { > DeviceState *dev; > FWCfgState *s; > - uint32_t version = FW_CFG_VERSION; > bool dma_requested = dma_iobase && dma_as; > > dev = qdev_create(NULL, TYPE_FW_CFG_IO); > @@ -954,12 +960,8 @@ FWCfgState *fw_cfg_init_io_dma(uint32_t iobase, uint32_t dma_iobase, > /* 64 bits for the address field */ > s->dma_as = dma_as; > s->dma_addr = 0; > - > - version |= FW_CFG_VERSION_DMA; > } > > - fw_cfg_add_i32(s, FW_CFG_ID, version); > - > return s; > } > > @@ -975,7 +977,6 @@ FWCfgState *fw_cfg_init_mem_wide(hwaddr ctl_addr, > DeviceState *dev; > SysBusDevice *sbd; > FWCfgState *s; > - uint32_t version = FW_CFG_VERSION; > bool dma_requested = dma_addr && dma_as; > > dev = qdev_create(NULL, TYPE_FW_CFG_MEM); > @@ -997,11 +998,8 @@ FWCfgState *fw_cfg_init_mem_wide(hwaddr ctl_addr, > s->dma_as = dma_as; > s->dma_addr = 0; > sysbus_mmio_map(sbd, 2, dma_addr); > - version |= FW_CFG_VERSION_DMA; > } > > - fw_cfg_add_i32(s, FW_CFG_ID, version); > - > return s; > } >
On 12/06/17 12:37, Igor Mammedov wrote: > On Sat, 10 Jun 2017 13:30:18 +0100 > Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> wrote: > >> The dma_enabled property enables us to set the FW_CFG_ID version >> accordingly. > it might be not safe as patch reorders FW_CFG_ID entry. > > (I recall there were patches that sorted fwcfg entries, > but it's worth checking that it won't break migration) > > CCing Gerd, maybe he would recall how it works. Ah so the order in fw_cfg needs to be preserved? (my assumption was that the slot-id also specified the order). I'll have a look and see if I can fix this with a simple re-ordering. ATB, Mark.
On Mon, 2017-06-12 at 13:37 +0200, Igor Mammedov wrote: > On Sat, 10 Jun 2017 13:30:18 +0100 > Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> wrote: > > > The dma_enabled property enables us to set the FW_CFG_ID version > > accordingly. > > it might be not safe as patch reorders FW_CFG_ID entry. > > (I recall there were patches that sorted fwcfg entries, > but it's worth checking that it won't break migration) > > CCing Gerd, maybe he would recall how it works. http://git.qemu.org/?p=qemu.git;a=blob;f=hw/nvram/fw_cfg.c;h=316fca9bc1 dadf8777ad9a9c238381702022bc56;hb=HEAD#l723 So, should be safe to shuffle around stuff. cheers, Gerd
diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c index 1313bfd..f7b78a9 100644 --- a/hw/nvram/fw_cfg.c +++ b/hw/nvram/fw_cfg.c @@ -914,12 +914,19 @@ static void fw_cfg_init1(DeviceState *dev) { FWCfgState *s = FW_CFG(dev); MachineState *machine = MACHINE(qdev_get_machine()); + uint32_t version = FW_CFG_VERSION; assert(!object_resolve_path(FW_CFG_PATH, NULL)); object_property_add_child(OBJECT(machine), FW_CFG_NAME, OBJECT(s), NULL); fw_cfg_add_bytes(s, FW_CFG_SIGNATURE, (char *)"QEMU", 4); + + if (s->dma_enabled) { + version |= FW_CFG_VERSION_DMA; + } + + fw_cfg_add_i32(s, FW_CFG_ID, version); fw_cfg_add_bytes(s, FW_CFG_UUID, &qemu_uuid, 16); fw_cfg_add_i16(s, FW_CFG_NOGRAPHIC, (uint16_t)!machine->enable_graphics); fw_cfg_add_i16(s, FW_CFG_BOOT_MENU, (uint16_t)boot_menu); @@ -935,7 +942,6 @@ FWCfgState *fw_cfg_init_io_dma(uint32_t iobase, uint32_t dma_iobase, { DeviceState *dev; FWCfgState *s; - uint32_t version = FW_CFG_VERSION; bool dma_requested = dma_iobase && dma_as; dev = qdev_create(NULL, TYPE_FW_CFG_IO); @@ -954,12 +960,8 @@ FWCfgState *fw_cfg_init_io_dma(uint32_t iobase, uint32_t dma_iobase, /* 64 bits for the address field */ s->dma_as = dma_as; s->dma_addr = 0; - - version |= FW_CFG_VERSION_DMA; } - fw_cfg_add_i32(s, FW_CFG_ID, version); - return s; } @@ -975,7 +977,6 @@ FWCfgState *fw_cfg_init_mem_wide(hwaddr ctl_addr, DeviceState *dev; SysBusDevice *sbd; FWCfgState *s; - uint32_t version = FW_CFG_VERSION; bool dma_requested = dma_addr && dma_as; dev = qdev_create(NULL, TYPE_FW_CFG_MEM); @@ -997,11 +998,8 @@ FWCfgState *fw_cfg_init_mem_wide(hwaddr ctl_addr, s->dma_as = dma_as; s->dma_addr = 0; sysbus_mmio_map(sbd, 2, dma_addr); - version |= FW_CFG_VERSION_DMA; } - fw_cfg_add_i32(s, FW_CFG_ID, version); - return s; }
The dma_enabled property enables us to set the FW_CFG_ID version accordingly. Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> --- hw/nvram/fw_cfg.c | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-)