From 2c9b15cab12c21e32dffb67c5e18f3dc407ca224 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Thu, 6 Jun 2013 05:41:28 -0400 Subject: memory: add owner argument to initialization functions Signed-off-by: Paolo Bonzini --- hw/display/qxl.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'hw/display/qxl.c') diff --git a/hw/display/qxl.c b/hw/display/qxl.c index 937a402b2e..bcfcce4a75 100644 --- a/hw/display/qxl.c +++ b/hw/display/qxl.c @@ -1981,18 +1981,18 @@ static int qxl_init_common(PCIQXLDevice *qxl) pci_set_byte(&config[PCI_INTERRUPT_PIN], 1); qxl->rom_size = qxl_rom_size(); - memory_region_init_ram(&qxl->rom_bar, "qxl.vrom", qxl->rom_size); + memory_region_init_ram(&qxl->rom_bar, NULL, "qxl.vrom", qxl->rom_size); vmstate_register_ram(&qxl->rom_bar, &qxl->pci.qdev); init_qxl_rom(qxl); init_qxl_ram(qxl); qxl->guest_surfaces.cmds = g_new0(QXLPHYSICAL, qxl->ssd.num_surfaces); - memory_region_init_ram(&qxl->vram_bar, "qxl.vram", qxl->vram_size); + memory_region_init_ram(&qxl->vram_bar, NULL, "qxl.vram", qxl->vram_size); vmstate_register_ram(&qxl->vram_bar, &qxl->pci.qdev); - memory_region_init_alias(&qxl->vram32_bar, "qxl.vram32", &qxl->vram_bar, + memory_region_init_alias(&qxl->vram32_bar, NULL, "qxl.vram32", &qxl->vram_bar, 0, qxl->vram32_size); - memory_region_init_io(&qxl->io_bar, &qxl_io_ops, qxl, + memory_region_init_io(&qxl->io_bar, NULL, &qxl_io_ops, qxl, "qxl-ioports", io_size); if (qxl->id == 0) { vga_dirty_log_start(&qxl->vga); @@ -2093,7 +2093,7 @@ static int qxl_init_secondary(PCIDevice *dev) qxl->id = device_id++; qxl_init_ramsize(qxl); - memory_region_init_ram(&qxl->vga.vram, "qxl.vgavram", qxl->vga.vram_size); + memory_region_init_ram(&qxl->vga.vram, NULL, "qxl.vgavram", qxl->vga.vram_size); vmstate_register_ram(&qxl->vga.vram, &qxl->pci.qdev); qxl->vga.vram_ptr = memory_region_get_ram_ptr(&qxl->vga.vram); qxl->vga.con = graphic_console_init(DEVICE(dev), &qxl_ops, qxl); -- cgit 1.4.1 From 712f0cc777dc8abc1f43b8e2a5e65ab3ae563cbd Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Thu, 6 Jun 2013 21:21:13 -0400 Subject: vga: pass owner to vga_init Signed-off-by: Paolo Bonzini --- hw/display/qxl.c | 3 ++- hw/display/vga-pci.c | 3 ++- hw/display/vga.c | 2 +- hw/display/vga_int.h | 2 +- hw/display/vmware_vga.c | 2 +- 5 files changed, 7 insertions(+), 5 deletions(-) (limited to 'hw/display/qxl.c') diff --git a/hw/display/qxl.c b/hw/display/qxl.c index bcfcce4a75..5754fdc5fd 100644 --- a/hw/display/qxl.c +++ b/hw/display/qxl.c @@ -2068,7 +2068,8 @@ static int qxl_init_primary(PCIDevice *dev) qxl_init_ramsize(qxl); vga->vram_size_mb = qxl->vga.vram_size >> 20; vga_common_init(vga); - vga_init(vga, pci_address_space(dev), pci_address_space_io(dev), false); + vga_init(vga, OBJECT(dev), + pci_address_space(dev), pci_address_space_io(dev), false); portio_list_init(qxl_vga_port_list, qxl_vga_portio_list, vga, "vga"); portio_list_add(qxl_vga_port_list, pci_address_space_io(dev), 0x3b0); diff --git a/hw/display/vga-pci.c b/hw/display/vga-pci.c index 23d5b5654a..4716230d7e 100644 --- a/hw/display/vga-pci.c +++ b/hw/display/vga-pci.c @@ -148,7 +148,8 @@ static int pci_std_vga_initfn(PCIDevice *dev) /* vga + console init */ vga_common_init(s); - vga_init(s, pci_address_space(dev), pci_address_space_io(dev), true); + vga_init(s, OBJECT(dev), pci_address_space(dev), pci_address_space_io(dev), + true); s->con = graphic_console_init(DEVICE(dev), s->hw_ops, s); diff --git a/hw/display/vga.c b/hw/display/vga.c index 21e37633f3..24b8b45614 100644 --- a/hw/display/vga.c +++ b/hw/display/vga.c @@ -2350,7 +2350,7 @@ MemoryRegion *vga_init_io(VGACommonState *s, return vga_mem; } -void vga_init(VGACommonState *s, MemoryRegion *address_space, +void vga_init(VGACommonState *s, Object *obj, MemoryRegion *address_space, MemoryRegion *address_space_io, bool init_vga_ports) { MemoryRegion *vga_io_memory; diff --git a/hw/display/vga_int.h b/hw/display/vga_int.h index 66f9f3ceed..accc9f52e8 100644 --- a/hw/display/vga_int.h +++ b/hw/display/vga_int.h @@ -178,7 +178,7 @@ static inline int c6_to_8(int v) } void vga_common_init(VGACommonState *s); -void vga_init(VGACommonState *s, MemoryRegion *address_space, +void vga_init(VGACommonState *s, Object *obj, MemoryRegion *address_space, MemoryRegion *address_space_io, bool init_vga_ports); MemoryRegion *vga_init_io(VGACommonState *s, const MemoryRegionPortio **vga_ports, diff --git a/hw/display/vmware_vga.c b/hw/display/vmware_vga.c index 05be1fa614..358bcc01cf 100644 --- a/hw/display/vmware_vga.c +++ b/hw/display/vmware_vga.c @@ -1199,7 +1199,7 @@ static void vmsvga_init(DeviceState *dev, struct vmsvga_state_s *s, s->fifo_ptr = memory_region_get_ram_ptr(&s->fifo_ram); vga_common_init(&s->vga); - vga_init(&s->vga, address_space, io, true); + vga_init(&s->vga, OBJECT(dev), address_space, io, true); vmstate_register(NULL, 0, &vmstate_vga_common, &s->vga); s->new_depth = 32; } -- cgit 1.4.1 From 270327feb2535f74379030e028b96b57fd60ca39 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Thu, 6 Jun 2013 21:21:13 -0400 Subject: vga: pass owner to vga_common_init Signed-off-by: Paolo Bonzini --- hw/display/cirrus_vga.c | 4 ++-- hw/display/qxl.c | 2 +- hw/display/vga-isa-mm.c | 2 +- hw/display/vga-isa.c | 2 +- hw/display/vga-pci.c | 2 +- hw/display/vga.c | 4 ++-- hw/display/vga_int.h | 2 +- hw/display/vmware_vga.c | 2 +- 8 files changed, 10 insertions(+), 10 deletions(-) (limited to 'hw/display/qxl.c') diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c index 00ac9a383c..80510acd47 100644 --- a/hw/display/cirrus_vga.c +++ b/hw/display/cirrus_vga.c @@ -2912,7 +2912,7 @@ static void isa_cirrus_vga_realizefn(DeviceState *dev, Error **errp) ISACirrusVGAState *d = ISA_CIRRUS_VGA(dev); VGACommonState *s = &d->cirrus_vga.vga; - vga_common_init(s); + vga_common_init(s, OBJECT(dev)); cirrus_init_common(&d->cirrus_vga, CIRRUS_ID_CLGD5430, 0, isa_address_space(isadev), isa_address_space_io(isadev)); @@ -2958,7 +2958,7 @@ static int pci_cirrus_vga_initfn(PCIDevice *dev) int16_t device_id = pc->device_id; /* setup VGA */ - vga_common_init(&s->vga); + vga_common_init(&s->vga, OBJECT(dev)); cirrus_init_common(s, device_id, 1, pci_address_space(dev), pci_address_space_io(dev)); s->vga.con = graphic_console_init(DEVICE(dev), s->vga.hw_ops, &s->vga); diff --git a/hw/display/qxl.c b/hw/display/qxl.c index 5754fdc5fd..59925ec4e5 100644 --- a/hw/display/qxl.c +++ b/hw/display/qxl.c @@ -2067,7 +2067,7 @@ static int qxl_init_primary(PCIDevice *dev) qxl->id = 0; qxl_init_ramsize(qxl); vga->vram_size_mb = qxl->vga.vram_size >> 20; - vga_common_init(vga); + vga_common_init(vga, OBJECT(dev)); vga_init(vga, OBJECT(dev), pci_address_space(dev), pci_address_space_io(dev), false); portio_list_init(qxl_vga_port_list, qxl_vga_portio_list, vga, "vga"); diff --git a/hw/display/vga-isa-mm.c b/hw/display/vga-isa-mm.c index 95707a814e..695c128cb7 100644 --- a/hw/display/vga-isa-mm.c +++ b/hw/display/vga-isa-mm.c @@ -132,7 +132,7 @@ int isa_vga_mm_init(hwaddr vram_base, s = g_malloc0(sizeof(*s)); s->vga.vram_size_mb = VGA_RAM_SIZE >> 20; - vga_common_init(&s->vga); + vga_common_init(&s->vga, NULL); vga_mm_init(s, vram_base, ctrl_base, it_shift, address_space); s->vga.con = graphic_console_init(NULL, s->vga.hw_ops, s); diff --git a/hw/display/vga-isa.c b/hw/display/vga-isa.c index d1691a9f9b..fbf6db5765 100644 --- a/hw/display/vga-isa.c +++ b/hw/display/vga-isa.c @@ -56,7 +56,7 @@ static void vga_isa_realizefn(DeviceState *dev, Error **errp) MemoryRegion *vga_io_memory; const MemoryRegionPortio *vga_ports, *vbe_ports; - vga_common_init(s); + vga_common_init(s, OBJECT(dev)); s->legacy_address_space = isa_address_space(isadev); vga_io_memory = vga_init_io(s, &vga_ports, &vbe_ports); isa_register_portio_list(isadev, 0x3b0, vga_ports, s, "vga"); diff --git a/hw/display/vga-pci.c b/hw/display/vga-pci.c index 4716230d7e..62652bd477 100644 --- a/hw/display/vga-pci.c +++ b/hw/display/vga-pci.c @@ -147,7 +147,7 @@ static int pci_std_vga_initfn(PCIDevice *dev) VGACommonState *s = &d->vga; /* vga + console init */ - vga_common_init(s); + vga_common_init(s, OBJECT(dev)); vga_init(s, OBJECT(dev), pci_address_space(dev), pci_address_space_io(dev), true); diff --git a/hw/display/vga.c b/hw/display/vga.c index 24b8b45614..d3e0f33066 100644 --- a/hw/display/vga.c +++ b/hw/display/vga.c @@ -2256,7 +2256,7 @@ static const GraphicHwOps vga_ops = { .text_update = vga_update_text, }; -void vga_common_init(VGACommonState *s) +void vga_common_init(VGACommonState *s, Object *obj) { int i, j, v, b; @@ -2292,7 +2292,7 @@ void vga_common_init(VGACommonState *s) s->vram_size_mb = s->vram_size >> 20; s->is_vbe_vmstate = 1; - memory_region_init_ram(&s->vram, NULL, "vga.vram", s->vram_size); + memory_region_init_ram(&s->vram, obj, "vga.vram", s->vram_size); vmstate_register_ram_global(&s->vram); xen_register_framebuffer(&s->vram); s->vram_ptr = memory_region_get_ram_ptr(&s->vram); diff --git a/hw/display/vga_int.h b/hw/display/vga_int.h index accc9f52e8..bac366cfd7 100644 --- a/hw/display/vga_int.h +++ b/hw/display/vga_int.h @@ -177,7 +177,7 @@ static inline int c6_to_8(int v) return (v << 2) | (b << 1) | b; } -void vga_common_init(VGACommonState *s); +void vga_common_init(VGACommonState *s, Object *obj); void vga_init(VGACommonState *s, Object *obj, MemoryRegion *address_space, MemoryRegion *address_space_io, bool init_vga_ports); MemoryRegion *vga_init_io(VGACommonState *s, diff --git a/hw/display/vmware_vga.c b/hw/display/vmware_vga.c index 358bcc01cf..ce97870c17 100644 --- a/hw/display/vmware_vga.c +++ b/hw/display/vmware_vga.c @@ -1198,7 +1198,7 @@ static void vmsvga_init(DeviceState *dev, struct vmsvga_state_s *s, vmstate_register_ram_global(&s->fifo_ram); s->fifo_ptr = memory_region_get_ram_ptr(&s->fifo_ram); - vga_common_init(&s->vga); + vga_common_init(&s->vga, OBJECT(dev)); vga_init(&s->vga, OBJECT(dev), address_space, io, true); vmstate_register(NULL, 0, &vmstate_vga_common, &s->vga); s->new_depth = 32; -- cgit 1.4.1 From db10ca9057b11222408f708d5d99a3888eca4feb Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Thu, 6 Jun 2013 21:19:53 -0400 Subject: piolist: add owner argument to initialization functions and pass devices Signed-off-by: Paolo Bonzini --- hw/audio/adlib.c | 2 +- hw/display/qxl.c | 3 ++- hw/display/vga.c | 4 ++-- hw/dma/i82374.c | 2 +- hw/isa/isa-bus.c | 2 +- hw/isa/vt82c686.c | 2 +- hw/ppc/prep.c | 2 +- hw/watchdog/wdt_ib700.c | 2 +- include/exec/ioport.h | 4 +++- ioport.c | 6 ++++-- 10 files changed, 17 insertions(+), 12 deletions(-) (limited to 'hw/display/qxl.c') diff --git a/hw/audio/adlib.c b/hw/audio/adlib.c index 8b9b81e65f..f72e6ee372 100644 --- a/hw/audio/adlib.c +++ b/hw/audio/adlib.c @@ -349,7 +349,7 @@ static void adlib_realizefn (DeviceState *dev, Error **errp) adlib_portio_list[1].offset = s->port; adlib_portio_list[2].offset = s->port + 8; - portio_list_init (port_list, adlib_portio_list, s, "adlib"); + portio_list_init (port_list, OBJECT(s), adlib_portio_list, s, "adlib"); portio_list_add (port_list, isa_address_space_io(&s->parent_obj), 0); } diff --git a/hw/display/qxl.c b/hw/display/qxl.c index 59925ec4e5..3804fe292b 100644 --- a/hw/display/qxl.c +++ b/hw/display/qxl.c @@ -2070,7 +2070,8 @@ static int qxl_init_primary(PCIDevice *dev) vga_common_init(vga, OBJECT(dev)); vga_init(vga, OBJECT(dev), pci_address_space(dev), pci_address_space_io(dev), false); - portio_list_init(qxl_vga_port_list, qxl_vga_portio_list, vga, "vga"); + portio_list_init(qxl_vga_port_list, OBJECT(dev), qxl_vga_portio_list, + vga, "vga"); portio_list_add(qxl_vga_port_list, pci_address_space_io(dev), 0x3b0); vga->con = graphic_console_init(DEVICE(dev), &qxl_ops, qxl); diff --git a/hw/display/vga.c b/hw/display/vga.c index 1657356415..06f44a808c 100644 --- a/hw/display/vga.c +++ b/hw/display/vga.c @@ -2372,11 +2372,11 @@ void vga_init(VGACommonState *s, Object *obj, MemoryRegion *address_space, 1); memory_region_set_coalescing(vga_io_memory); if (init_vga_ports) { - portio_list_init(vga_port_list, vga_ports, s, "vga"); + portio_list_init(vga_port_list, obj, vga_ports, s, "vga"); portio_list_add(vga_port_list, address_space_io, 0x3b0); } if (vbe_ports) { - portio_list_init(vbe_port_list, vbe_ports, s, "vbe"); + portio_list_init(vbe_port_list, obj, vbe_ports, s, "vbe"); portio_list_add(vbe_port_list, address_space_io, 0x1ce); } } diff --git a/hw/dma/i82374.c b/hw/dma/i82374.c index ecda5cb192..a5b891f968 100644 --- a/hw/dma/i82374.c +++ b/hw/dma/i82374.c @@ -139,7 +139,7 @@ static void i82374_isa_realize(DeviceState *dev, Error **errp) I82374State *s = &isa->state; PortioList *port_list = g_new(PortioList, 1); - portio_list_init(port_list, i82374_portio_list, s, "i82374"); + portio_list_init(port_list, OBJECT(isa), i82374_portio_list, s, "i82374"); portio_list_add(port_list, isa_address_space_io(&isa->parent_obj), isa->iobase); diff --git a/hw/isa/isa-bus.c b/hw/isa/isa-bus.c index 136d17ede0..cfd610c681 100644 --- a/hw/isa/isa-bus.c +++ b/hw/isa/isa-bus.c @@ -115,7 +115,7 @@ void isa_register_portio_list(ISADevice *dev, uint16_t start, actually handled e.g. the FDC device. */ isa_init_ioport(dev, start); - portio_list_init(piolist, pio_start, opaque, name); + portio_list_init(piolist, OBJECT(dev), pio_start, opaque, name); portio_list_add(piolist, isabus->address_space_io, start); } diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c index 1f63197d82..19c887d338 100644 --- a/hw/isa/vt82c686.c +++ b/hw/isa/vt82c686.c @@ -443,7 +443,7 @@ static int vt82c686b_initfn(PCIDevice *d) } } - memory_region_init_io(&vt82c->superio, NULL, &superio_ops, + memory_region_init_io(&vt82c->superio, OBJECT(d), &superio_ops, &vt82c->superio_conf, "superio", 2); memory_region_set_enabled(&vt82c->superio, false); /* The floppy also uses 0x3f0 and 0x3f1. diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c index 899aa301ae..0081c701ab 100644 --- a/hw/ppc/prep.c +++ b/hw/ppc/prep.c @@ -653,7 +653,7 @@ static void ppc_prep_init(QEMUMachineInitArgs *args) sysctrl->reset_irq = first_cpu->irq_inputs[PPC6xx_INPUT_HRESET]; - portio_list_init(port_list, prep_portio_list, sysctrl, "prep"); + portio_list_init(port_list, NULL, prep_portio_list, sysctrl, "prep"); portio_list_add(port_list, get_system_io(), 0x0); /* PowerPC control and status register group */ diff --git a/hw/watchdog/wdt_ib700.c b/hw/watchdog/wdt_ib700.c index 597a923294..c78855444c 100644 --- a/hw/watchdog/wdt_ib700.c +++ b/hw/watchdog/wdt_ib700.c @@ -112,7 +112,7 @@ static void wdt_ib700_realize(DeviceState *dev, Error **errp) s->timer = qemu_new_timer_ns(vm_clock, ib700_timer_expired, s); - portio_list_init(port_list, wdt_portio_list, s, "ib700"); + portio_list_init(port_list, OBJECT(s), wdt_portio_list, s, "ib700"); portio_list_add(port_list, isa_address_space_io(&s->parent_obj), 0); } diff --git a/include/exec/ioport.h b/include/exec/ioport.h index 6fb237d03e..bdd4e964eb 100644 --- a/include/exec/ioport.h +++ b/include/exec/ioport.h @@ -25,6 +25,7 @@ #define IOPORT_H #include "qemu-common.h" +#include "qom/object.h" #include "exec/memory.h" typedef uint32_t pio_addr_t; @@ -53,6 +54,7 @@ uint32_t cpu_inl(pio_addr_t addr); typedef struct PortioList { const struct MemoryRegionPortio *ports; + Object *owner; struct MemoryRegion *address_space; unsigned nr; struct MemoryRegion **regions; @@ -60,7 +62,7 @@ typedef struct PortioList { const char *name; } PortioList; -void portio_list_init(PortioList *piolist, +void portio_list_init(PortioList *piolist, Object *owner, const struct MemoryRegionPortio *callbacks, void *opaque, const char *name); void portio_list_destroy(PortioList *piolist); diff --git a/ioport.c b/ioport.c index 3a6a73dd98..79b7f1ae38 100644 --- a/ioport.c +++ b/ioport.c @@ -106,6 +106,7 @@ uint32_t cpu_inl(pio_addr_t addr) } void portio_list_init(PortioList *piolist, + Object *owner, const MemoryRegionPortio *callbacks, void *opaque, const char *name) { @@ -120,6 +121,7 @@ void portio_list_init(PortioList *piolist, piolist->regions = g_new0(MemoryRegion *, n); piolist->address_space = NULL; piolist->opaque = opaque; + piolist->owner = owner; piolist->name = name; } @@ -211,8 +213,8 @@ static void portio_list_add_1(PortioList *piolist, * Use an alias so that the callback is called with an absolute address, * rather than an offset relative to to start + off_low. */ - memory_region_init_io(&mrpio->mr, NULL, &portio_ops, mrpio, piolist->name, - off_high - off_low); + memory_region_init_io(&mrpio->mr, piolist->owner, &portio_ops, mrpio, + piolist->name, off_high - off_low); memory_region_add_subregion(piolist->address_space, start + off_low, &mrpio->mr); piolist->regions[piolist->nr] = &mrpio->mr; -- cgit 1.4.1 From 3eadad551d3d5901b75f8c53dbd57b9bec2f2b01 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Thu, 6 Jun 2013 21:25:08 -0400 Subject: hw/d*: pass owner to memory_region_init* functions Signed-off-by: Paolo Bonzini --- hw/display/cirrus_vga.c | 2 +- hw/display/exynos4210_fimd.c | 2 +- hw/display/jazz_led.c | 2 +- hw/display/milkymist-tmu2.c | 2 +- hw/display/milkymist-vgafb.c | 2 +- hw/display/pl110.c | 2 +- hw/display/qxl.c | 15 +++++++++------ hw/display/tcx.c | 18 ++++++++++-------- hw/dma/pl080.c | 2 +- hw/dma/pl330.c | 3 ++- hw/dma/puv3_dma.c | 2 +- hw/dma/pxa2xx_dma.c | 2 +- hw/dma/sparc32_dma.c | 3 ++- hw/dma/sun4m_iommu.c | 2 +- hw/dma/xilinx_axidma.c | 2 +- 15 files changed, 34 insertions(+), 27 deletions(-) (limited to 'hw/display/qxl.c') diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c index 3d579e2f94..a440575def 100644 --- a/hw/display/cirrus_vga.c +++ b/hw/display/cirrus_vga.c @@ -2967,7 +2967,7 @@ static int pci_cirrus_vga_initfn(PCIDevice *dev) /* setup PCI */ - memory_region_init(&s->pci_bar, NULL, "cirrus-pci-bar0", 0x2000000); + memory_region_init(&s->pci_bar, OBJECT(dev), "cirrus-pci-bar0", 0x2000000); /* XXX: add byte swapping apertures */ memory_region_add_subregion(&s->pci_bar, 0, &s->cirrus_linear_io); diff --git a/hw/display/exynos4210_fimd.c b/hw/display/exynos4210_fimd.c index b520f524f7..eb168eaf11 100644 --- a/hw/display/exynos4210_fimd.c +++ b/hw/display/exynos4210_fimd.c @@ -1908,7 +1908,7 @@ static int exynos4210_fimd_init(SysBusDevice *dev) sysbus_init_irq(dev, &s->irq[1]); sysbus_init_irq(dev, &s->irq[2]); - memory_region_init_io(&s->iomem, NULL, &exynos4210_fimd_mmio_ops, s, + memory_region_init_io(&s->iomem, OBJECT(s), &exynos4210_fimd_mmio_ops, s, "exynos4210.fimd", FIMD_REGS_SIZE); sysbus_init_mmio(dev, &s->iomem); s->console = graphic_console_init(DEVICE(dev), &exynos4210_fimd_ops, s); diff --git a/hw/display/jazz_led.c b/hw/display/jazz_led.c index e6a508ef89..7f82037d99 100644 --- a/hw/display/jazz_led.c +++ b/hw/display/jazz_led.c @@ -264,7 +264,7 @@ static int jazz_led_init(SysBusDevice *dev) { LedState *s = FROM_SYSBUS(LedState, dev); - memory_region_init_io(&s->iomem, NULL, &led_ops, s, "led", 1); + memory_region_init_io(&s->iomem, OBJECT(s), &led_ops, s, "led", 1); sysbus_init_mmio(dev, &s->iomem); s->con = graphic_console_init(DEVICE(dev), &jazz_led_ops, s); diff --git a/hw/display/milkymist-tmu2.c b/hw/display/milkymist-tmu2.c index 31da796d48..efda0824fe 100644 --- a/hw/display/milkymist-tmu2.c +++ b/hw/display/milkymist-tmu2.c @@ -447,7 +447,7 @@ static int milkymist_tmu2_init(SysBusDevice *dev) sysbus_init_irq(dev, &s->irq); - memory_region_init_io(&s->regs_region, NULL, &tmu2_mmio_ops, s, + memory_region_init_io(&s->regs_region, OBJECT(s), &tmu2_mmio_ops, s, "milkymist-tmu2", R_MAX * 4); sysbus_init_mmio(dev, &s->regs_region); diff --git a/hw/display/milkymist-vgafb.c b/hw/display/milkymist-vgafb.c index b1f9c168fa..870b339581 100644 --- a/hw/display/milkymist-vgafb.c +++ b/hw/display/milkymist-vgafb.c @@ -279,7 +279,7 @@ static int milkymist_vgafb_init(SysBusDevice *dev) { MilkymistVgafbState *s = FROM_SYSBUS(typeof(*s), dev); - memory_region_init_io(&s->regs_region, NULL, &vgafb_mmio_ops, s, + memory_region_init_io(&s->regs_region, OBJECT(s), &vgafb_mmio_ops, s, "milkymist-vgafb", R_MAX * 4); sysbus_init_mmio(dev, &s->regs_region); diff --git a/hw/display/pl110.c b/hw/display/pl110.c index 15c2f38baa..60afcf39e1 100644 --- a/hw/display/pl110.c +++ b/hw/display/pl110.c @@ -453,7 +453,7 @@ static int pl110_init(SysBusDevice *dev) { pl110_state *s = FROM_SYSBUS(pl110_state, dev); - memory_region_init_io(&s->iomem, NULL, &pl110_ops, s, "pl110", 0x1000); + memory_region_init_io(&s->iomem, OBJECT(s), &pl110_ops, s, "pl110", 0x1000); sysbus_init_mmio(dev, &s->iomem); sysbus_init_irq(dev, &s->irq); qdev_init_gpio_in(&s->busdev.qdev, pl110_mux_ctrl_set, 1); diff --git a/hw/display/qxl.c b/hw/display/qxl.c index 3804fe292b..3862d7aafc 100644 --- a/hw/display/qxl.c +++ b/hw/display/qxl.c @@ -1981,18 +1981,20 @@ static int qxl_init_common(PCIQXLDevice *qxl) pci_set_byte(&config[PCI_INTERRUPT_PIN], 1); qxl->rom_size = qxl_rom_size(); - memory_region_init_ram(&qxl->rom_bar, NULL, "qxl.vrom", qxl->rom_size); + memory_region_init_ram(&qxl->rom_bar, OBJECT(qxl), "qxl.vrom", + qxl->rom_size); vmstate_register_ram(&qxl->rom_bar, &qxl->pci.qdev); init_qxl_rom(qxl); init_qxl_ram(qxl); qxl->guest_surfaces.cmds = g_new0(QXLPHYSICAL, qxl->ssd.num_surfaces); - memory_region_init_ram(&qxl->vram_bar, NULL, "qxl.vram", qxl->vram_size); + memory_region_init_ram(&qxl->vram_bar, OBJECT(qxl), "qxl.vram", + qxl->vram_size); vmstate_register_ram(&qxl->vram_bar, &qxl->pci.qdev); - memory_region_init_alias(&qxl->vram32_bar, NULL, "qxl.vram32", &qxl->vram_bar, - 0, qxl->vram32_size); + memory_region_init_alias(&qxl->vram32_bar, OBJECT(qxl), "qxl.vram32", + &qxl->vram_bar, 0, qxl->vram32_size); - memory_region_init_io(&qxl->io_bar, NULL, &qxl_io_ops, qxl, + memory_region_init_io(&qxl->io_bar, OBJECT(qxl), &qxl_io_ops, qxl, "qxl-ioports", io_size); if (qxl->id == 0) { vga_dirty_log_start(&qxl->vga); @@ -2095,7 +2097,8 @@ static int qxl_init_secondary(PCIDevice *dev) qxl->id = device_id++; qxl_init_ramsize(qxl); - memory_region_init_ram(&qxl->vga.vram, NULL, "qxl.vgavram", qxl->vga.vram_size); + memory_region_init_ram(&qxl->vga.vram, OBJECT(dev), "qxl.vgavram", + qxl->vga.vram_size); vmstate_register_ram(&qxl->vga.vram, &qxl->pci.qdev); qxl->vga.vram_ptr = memory_region_get_ram_ptr(&qxl->vga.vram); qxl->vga.con = graphic_console_init(DEVICE(dev), &qxl_ops, qxl); diff --git a/hw/display/tcx.c b/hw/display/tcx.c index 24b5478f1b..9fd48b5f8b 100644 --- a/hw/display/tcx.c +++ b/hw/display/tcx.c @@ -528,7 +528,7 @@ static int tcx_init1(SysBusDevice *dev) int size; uint8_t *vram_base; - memory_region_init_ram(&s->vram_mem, NULL, "tcx.vram", + memory_region_init_ram(&s->vram_mem, OBJECT(s), "tcx.vram", s->vram_size * (1 + 4 + 4)); vmstate_register_ram_global(&s->vram_mem); vram_base = memory_region_get_ram_ptr(&s->vram_mem); @@ -536,21 +536,23 @@ static int tcx_init1(SysBusDevice *dev) /* 8-bit plane */ s->vram = vram_base; size = s->vram_size; - memory_region_init_alias(&s->vram_8bit, NULL, "tcx.vram.8bit", + memory_region_init_alias(&s->vram_8bit, OBJECT(s), "tcx.vram.8bit", &s->vram_mem, vram_offset, size); sysbus_init_mmio(dev, &s->vram_8bit); vram_offset += size; vram_base += size; /* DAC */ - memory_region_init_io(&s->dac, NULL, &tcx_dac_ops, s, "tcx.dac", TCX_DAC_NREGS); + memory_region_init_io(&s->dac, OBJECT(s), &tcx_dac_ops, s, + "tcx.dac", TCX_DAC_NREGS); sysbus_init_mmio(dev, &s->dac); /* TEC (dummy) */ - memory_region_init_io(&s->tec, NULL, &dummy_ops, s, "tcx.tec", TCX_TEC_NREGS); + memory_region_init_io(&s->tec, OBJECT(s), &dummy_ops, s, + "tcx.tec", TCX_TEC_NREGS); sysbus_init_mmio(dev, &s->tec); /* THC: NetBSD writes here even with 8-bit display: dummy */ - memory_region_init_io(&s->thc24, NULL, &dummy_ops, s, "tcx.thc24", + memory_region_init_io(&s->thc24, OBJECT(s), &dummy_ops, s, "tcx.thc24", TCX_THC_NREGS_24); sysbus_init_mmio(dev, &s->thc24); @@ -559,7 +561,7 @@ static int tcx_init1(SysBusDevice *dev) size = s->vram_size * 4; s->vram24 = (uint32_t *)vram_base; s->vram24_offset = vram_offset; - memory_region_init_alias(&s->vram_24bit, NULL, "tcx.vram.24bit", + memory_region_init_alias(&s->vram_24bit, OBJECT(s), "tcx.vram.24bit", &s->vram_mem, vram_offset, size); sysbus_init_mmio(dev, &s->vram_24bit); vram_offset += size; @@ -569,14 +571,14 @@ static int tcx_init1(SysBusDevice *dev) size = s->vram_size * 4; s->cplane = (uint32_t *)vram_base; s->cplane_offset = vram_offset; - memory_region_init_alias(&s->vram_cplane, NULL, "tcx.vram.cplane", + memory_region_init_alias(&s->vram_cplane, OBJECT(s), "tcx.vram.cplane", &s->vram_mem, vram_offset, size); sysbus_init_mmio(dev, &s->vram_cplane); s->con = graphic_console_init(DEVICE(dev), &tcx24_ops, s); } else { /* THC 8 bit (dummy) */ - memory_region_init_io(&s->thc8, NULL, &dummy_ops, s, "tcx.thc8", + memory_region_init_io(&s->thc8, OBJECT(s), &dummy_ops, s, "tcx.thc8", TCX_THC_NREGS_8); sysbus_init_mmio(dev, &s->thc8); diff --git a/hw/dma/pl080.c b/hw/dma/pl080.c index 588c7b2571..7937c3ecf0 100644 --- a/hw/dma/pl080.c +++ b/hw/dma/pl080.c @@ -359,7 +359,7 @@ static int pl08x_init(SysBusDevice *dev, int nchannels) { pl080_state *s = FROM_SYSBUS(pl080_state, dev); - memory_region_init_io(&s->iomem, NULL, &pl080_ops, s, "pl080", 0x1000); + memory_region_init_io(&s->iomem, OBJECT(s), &pl080_ops, s, "pl080", 0x1000); sysbus_init_mmio(dev, &s->iomem); sysbus_init_irq(dev, &s->irq); s->nchannels = nchannels; diff --git a/hw/dma/pl330.c b/hw/dma/pl330.c index b366f8acea..ddcc4135d7 100644 --- a/hw/dma/pl330.c +++ b/hw/dma/pl330.c @@ -1528,7 +1528,8 @@ static void pl330_realize(DeviceState *dev, Error **errp) PL330State *s = PL330(dev); sysbus_init_irq(SYS_BUS_DEVICE(dev), &s->irq_abort); - memory_region_init_io(&s->iomem, NULL, &pl330_ops, s, "dma", PL330_IOMEM_SIZE); + memory_region_init_io(&s->iomem, OBJECT(s), &pl330_ops, s, + "dma", PL330_IOMEM_SIZE); sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->iomem); s->timer = qemu_new_timer_ns(vm_clock, pl330_exec_cycle_timer, s); diff --git a/hw/dma/puv3_dma.c b/hw/dma/puv3_dma.c index 787e9a1c4d..36004ae48a 100644 --- a/hw/dma/puv3_dma.c +++ b/hw/dma/puv3_dma.c @@ -80,7 +80,7 @@ static int puv3_dma_init(SysBusDevice *dev) s->reg_CFG[i] = 0x0; } - memory_region_init_io(&s->iomem, NULL, &puv3_dma_ops, s, "puv3_dma", + memory_region_init_io(&s->iomem, OBJECT(s), &puv3_dma_ops, s, "puv3_dma", PUV3_REGS_OFFSET); sysbus_init_mmio(dev, &s->iomem); diff --git a/hw/dma/pxa2xx_dma.c b/hw/dma/pxa2xx_dma.c index 43e2dc3b22..bc7bf4c5e0 100644 --- a/hw/dma/pxa2xx_dma.c +++ b/hw/dma/pxa2xx_dma.c @@ -465,7 +465,7 @@ static int pxa2xx_dma_init(SysBusDevice *dev) qdev_init_gpio_in(&dev->qdev, pxa2xx_dma_request, PXA2XX_DMA_NUM_REQUESTS); - memory_region_init_io(&s->iomem, NULL, &pxa2xx_dma_ops, s, + memory_region_init_io(&s->iomem, OBJECT(s), &pxa2xx_dma_ops, s, "pxa2xx.dma", 0x00010000); sysbus_init_mmio(dev, &s->iomem); sysbus_init_irq(dev, &s->irq); diff --git a/hw/dma/sparc32_dma.c b/hw/dma/sparc32_dma.c index 4abb024647..be6275fea5 100644 --- a/hw/dma/sparc32_dma.c +++ b/hw/dma/sparc32_dma.c @@ -274,7 +274,8 @@ static int sparc32_dma_init1(SysBusDevice *dev) sysbus_init_irq(dev, &s->irq); reg_size = s->is_ledma ? DMA_ETH_SIZE : DMA_SIZE; - memory_region_init_io(&s->iomem, NULL, &dma_mem_ops, s, "dma", reg_size); + memory_region_init_io(&s->iomem, OBJECT(s), &dma_mem_ops, s, + "dma", reg_size); sysbus_init_mmio(dev, &s->iomem); qdev_init_gpio_in(&dev->qdev, dma_set_irq, 1); diff --git a/hw/dma/sun4m_iommu.c b/hw/dma/sun4m_iommu.c index eafb99130f..edb93f36ac 100644 --- a/hw/dma/sun4m_iommu.c +++ b/hw/dma/sun4m_iommu.c @@ -349,7 +349,7 @@ static int iommu_init1(SysBusDevice *dev) sysbus_init_irq(dev, &s->irq); - memory_region_init_io(&s->iomem, NULL, &iommu_mem_ops, s, "iommu", + memory_region_init_io(&s->iomem, OBJECT(s), &iommu_mem_ops, s, "iommu", IOMMU_NREGS * sizeof(uint32_t)); sysbus_init_mmio(dev, &s->iomem); diff --git a/hw/dma/xilinx_axidma.c b/hw/dma/xilinx_axidma.c index bdce6351bb..a48e3baa99 100644 --- a/hw/dma/xilinx_axidma.c +++ b/hw/dma/xilinx_axidma.c @@ -590,7 +590,7 @@ static void xilinx_axidma_init(Object *obj) sysbus_init_irq(sbd, &s->streams[0].irq); sysbus_init_irq(sbd, &s->streams[1].irq); - memory_region_init_io(&s->iomem, NULL, &axidma_ops, s, + memory_region_init_io(&s->iomem, obj, &axidma_ops, s, "xlnx.axi-dma", R_MAX * 4 * 2); sysbus_init_mmio(sbd, &s->iomem); } -- cgit 1.4.1 From 5444e768ee1abe6e021bece19a9a932351f88c88 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 13 May 2013 13:29:47 +0200 Subject: add a header file for atomic operations We're already using them in several places, but __sync builtins are just too ugly to type, and do not provide seqcst load/store operations. Reviewed-by: Richard Henderson Signed-off-by: Paolo Bonzini --- docs/atomics.txt | 352 +++++++++++++++++++++++++++++++++++++++++++++++ hw/display/qxl.c | 3 +- hw/virtio/vhost.c | 9 +- include/qemu/atomic.h | 198 +++++++++++++++++++++----- migration.c | 3 +- tests/test-thread-pool.c | 8 +- 6 files changed, 529 insertions(+), 44 deletions(-) create mode 100644 docs/atomics.txt (limited to 'hw/display/qxl.c') diff --git a/docs/atomics.txt b/docs/atomics.txt new file mode 100644 index 0000000000..6f2997bc65 --- /dev/null +++ b/docs/atomics.txt @@ -0,0 +1,352 @@ +CPUs perform independent memory operations effectively in random order. +but this can be a problem for CPU-CPU interaction (including interactions +between QEMU and the guest). Multi-threaded programs use various tools +to instruct the compiler and the CPU to restrict the order to something +that is consistent with the expectations of the programmer. + +The most basic tool is locking. Mutexes, condition variables and +semaphores are used in QEMU, and should be the default approach to +synchronization. Anything else is considerably harder, but it's +also justified more often than one would like. The two tools that +are provided by qemu/atomic.h are memory barriers and atomic operations. + +Macros defined by qemu/atomic.h fall in three camps: + +- compiler barriers: barrier(); + +- weak atomic access and manual memory barriers: atomic_read(), + atomic_set(), smp_rmb(), smp_wmb(), smp_mb(), smp_read_barrier_depends(); + +- sequentially consistent atomic access: everything else. + + +COMPILER MEMORY BARRIER +======================= + +barrier() prevents the compiler from moving the memory accesses either +side of it to the other side. The compiler barrier has no direct effect +on the CPU, which may then reorder things however it wishes. + +barrier() is mostly used within qemu/atomic.h itself. On some +architectures, CPU guarantees are strong enough that blocking compiler +optimizations already ensures the correct order of execution. In this +case, qemu/atomic.h will reduce stronger memory barriers to simple +compiler barriers. + +Still, barrier() can be useful when writing code that can be interrupted +by signal handlers. + + +SEQUENTIALLY CONSISTENT ATOMIC ACCESS +===================================== + +Most of the operations in the qemu/atomic.h header ensure *sequential +consistency*, where "the result of any execution is the same as if the +operations of all the processors were executed in some sequential order, +and the operations of each individual processor appear in this sequence +in the order specified by its program". + +qemu/atomic.h provides the following set of atomic read-modify-write +operations: + + void atomic_inc(ptr) + void atomic_dec(ptr) + void atomic_add(ptr, val) + void atomic_sub(ptr, val) + void atomic_and(ptr, val) + void atomic_or(ptr, val) + + typeof(*ptr) atomic_fetch_inc(ptr) + typeof(*ptr) atomic_fetch_dec(ptr) + typeof(*ptr) atomic_fetch_add(ptr, val) + typeof(*ptr) atomic_fetch_sub(ptr, val) + typeof(*ptr) atomic_fetch_and(ptr, val) + typeof(*ptr) atomic_fetch_or(ptr, val) + typeof(*ptr) atomic_xchg(ptr, val + typeof(*ptr) atomic_cmpxchg(ptr, old, new) + +all of which return the old value of *ptr. These operations are +polymorphic; they operate on any type that is as wide as an int. + +Sequentially consistent loads and stores can be done using: + + atomic_fetch_add(ptr, 0) for loads + atomic_xchg(ptr, val) for stores + +However, they are quite expensive on some platforms, notably POWER and +ARM. Therefore, qemu/atomic.h provides two primitives with slightly +weaker constraints: + + typeof(*ptr) atomic_mb_read(ptr) + void atomic_mb_set(ptr, val) + +The semantics of these primitives map to Java volatile variables, +and are strongly related to memory barriers as used in the Linux +kernel (see below). + +As long as you use atomic_mb_read and atomic_mb_set, accesses cannot +be reordered with each other, and it is also not possible to reorder +"normal" accesses around them. + +However, and this is the important difference between +atomic_mb_read/atomic_mb_set and sequential consistency, it is important +for both threads to access the same volatile variable. It is not the +case that everything visible to thread A when it writes volatile field f +becomes visible to thread B after it reads volatile field g. The store +and load have to "match" (i.e., be performed on the same volatile +field) to achieve the right semantics. + + +These operations operate on any type that is as wide as an int or smaller. + + +WEAK ATOMIC ACCESS AND MANUAL MEMORY BARRIERS +============================================= + +Compared to sequentially consistent atomic access, programming with +weaker consistency models can be considerably more complicated. +In general, if the algorithm you are writing includes both writes +and reads on the same side, it is generally simpler to use sequentially +consistent primitives. + +When using this model, variables are accessed with atomic_read() and +atomic_set(), and restrictions to the ordering of accesses is enforced +using the smp_rmb(), smp_wmb(), smp_mb() and smp_read_barrier_depends() +memory barriers. + +atomic_read() and atomic_set() prevents the compiler from using +optimizations that might otherwise optimize accesses out of existence +on the one hand, or that might create unsolicited accesses on the other. +In general this should not have any effect, because the same compiler +barriers are already implied by memory barriers. However, it is useful +to do so, because it tells readers which variables are shared with +other threads, and which are local to the current thread or protected +by other, more mundane means. + +Memory barriers control the order of references to shared memory. +They come in four kinds: + +- smp_rmb() guarantees that all the LOAD operations specified before + the barrier will appear to happen before all the LOAD operations + specified after the barrier with respect to the other components of + the system. + + In other words, smp_rmb() puts a partial ordering on loads, but is not + required to have any effect on stores. + +- smp_wmb() guarantees that all the STORE operations specified before + the barrier will appear to happen before all the STORE operations + specified after the barrier with respect to the other components of + the system. + + In other words, smp_wmb() puts a partial ordering on stores, but is not + required to have any effect on loads. + +- smp_mb() guarantees that all the LOAD and STORE operations specified + before the barrier will appear to happen before all the LOAD and + STORE operations specified after the barrier with respect to the other + components of the system. + + smp_mb() puts a partial ordering on both loads and stores. It is + stronger than both a read and a write memory barrier; it implies both + smp_rmb() and smp_wmb(), but it also prevents STOREs coming before the + barrier from overtaking LOADs coming after the barrier and vice versa. + +- smp_read_barrier_depends() is a weaker kind of read barrier. On + most processors, whenever two loads are performed such that the + second depends on the result of the first (e.g., the first load + retrieves the address to which the second load will be directed), + the processor will guarantee that the first LOAD will appear to happen + before the second with respect to the other components of the system. + However, this is not always true---for example, it was not true on + Alpha processors. Whenever this kind of access happens to shared + memory (that is not protected by a lock), a read barrier is needed, + and smp_read_barrier_depends() can be used instead of smp_rmb(). + + Note that the first load really has to have a _data_ dependency and not + a control dependency. If the address for the second load is dependent + on the first load, but the dependency is through a conditional rather + than actually loading the address itself, then it's a _control_ + dependency and a full read barrier or better is required. + + +This is the set of barriers that is required *between* two atomic_read() +and atomic_set() operations to achieve sequential consistency: + + | 2nd operation | + |-----------------------------------------| + 1st operation | (after last) | atomic_read | atomic_set | + ---------------+--------------+-------------+------------| + (before first) | | none | smp_wmb() | + ---------------+--------------+-------------+------------| + atomic_read | smp_rmb() | smp_rmb()* | ** | + ---------------+--------------+-------------+------------| + atomic_set | none | smp_mb()*** | smp_wmb() | + ---------------+--------------+-------------+------------| + + * Or smp_read_barrier_depends(). + + ** This requires a load-store barrier. How to achieve this varies + depending on the machine, but in practice smp_rmb()+smp_wmb() + should have the desired effect. For example, on PowerPC the + lwsync instruction is a combined load-load, load-store and + store-store barrier. + + *** This requires a store-load barrier. On most machines, the only + way to achieve this is a full barrier. + + +You can see that the two possible definitions of atomic_mb_read() +and atomic_mb_set() are the following: + + 1) atomic_mb_read(p) = atomic_read(p); smp_rmb() + atomic_mb_set(p, v) = smp_wmb(); atomic_set(p, v); smp_mb() + + 2) atomic_mb_read(p) = smp_mb() atomic_read(p); smp_rmb() + atomic_mb_set(p, v) = smp_wmb(); atomic_set(p, v); + +Usually the former is used, because smp_mb() is expensive and a program +normally has more reads than writes. Therefore it makes more sense to +make atomic_mb_set() the more expensive operation. + +There are two common cases in which atomic_mb_read and atomic_mb_set +generate too many memory barriers, and thus it can be useful to manually +place barriers instead: + +- when a data structure has one thread that is always a writer + and one thread that is always a reader, manual placement of + memory barriers makes the write side faster. Furthermore, + correctness is easy to check for in this case using the "pairing" + trick that is explained below: + + thread 1 thread 1 + ------------------------- ------------------------ + (other writes) + smp_wmb() + atomic_mb_set(&a, x) atomic_set(&a, x) + smp_wmb() + atomic_mb_set(&b, y) atomic_set(&b, y) + + => + thread 2 thread 2 + ------------------------- ------------------------ + y = atomic_mb_read(&b) y = atomic_read(&b) + smp_rmb() + x = atomic_mb_read(&a) x = atomic_read(&a) + smp_rmb() + +- sometimes, a thread is accessing many variables that are otherwise + unrelated to each other (for example because, apart from the current + thread, exactly one other thread will read or write each of these + variables). In this case, it is possible to "hoist" the implicit + barriers provided by atomic_mb_read() and atomic_mb_set() outside + a loop. For example, the above definition atomic_mb_read() gives + the following transformation: + + n = 0; n = 0; + for (i = 0; i < 10; i++) => for (i = 0; i < 10; i++) + n += atomic_mb_read(&a[i]); n += atomic_read(&a[i]); + smp_rmb(); + + Similarly, atomic_mb_set() can be transformed as follows: + smp_mb(): + + smp_wmb(); + for (i = 0; i < 10; i++) => for (i = 0; i < 10; i++) + atomic_mb_set(&a[i], false); atomic_set(&a[i], false); + smp_mb(); + + +The two tricks can be combined. In this case, splitting a loop in +two lets you hoist the barriers out of the loops _and_ eliminate the +expensive smp_mb(): + + smp_wmb(); + for (i = 0; i < 10; i++) { => for (i = 0; i < 10; i++) + atomic_mb_set(&a[i], false); atomic_set(&a[i], false); + atomic_mb_set(&b[i], false); smb_wmb(); + } for (i = 0; i < 10; i++) + atomic_set(&a[i], false); + smp_mb(); + + The other thread can still use atomic_mb_read()/atomic_mb_set() + + +Memory barrier pairing +---------------------- + +A useful rule of thumb is that memory barriers should always, or almost +always, be paired with another barrier. In the case of QEMU, however, +note that the other barrier may actually be in a driver that runs in +the guest! + +For the purposes of pairing, smp_read_barrier_depends() and smp_rmb() +both count as read barriers. A read barriers shall pair with a write +barrier or a full barrier; a write barrier shall pair with a read +barrier or a full barrier. A full barrier can pair with anything. +For example: + + thread 1 thread 2 + =============== =============== + a = 1; + smp_wmb(); + b = 2; x = b; + smp_rmb(); + y = a; + +Note that the "writing" thread are accessing the variables in the +opposite order as the "reading" thread. This is expected: stores +before the write barrier will normally match the loads after the +read barrier, and vice versa. The same is true for more than 2 +access and for data dependency barriers: + + thread 1 thread 2 + =============== =============== + b[2] = 1; + smp_wmb(); + x->i = 2; + smp_wmb(); + a = x; x = a; + smp_read_barrier_depends(); + y = x->i; + smp_read_barrier_depends(); + z = b[y]; + +smp_wmb() also pairs with atomic_mb_read(), and smp_rmb() also pairs +with atomic_mb_set(). + + +COMPARISON WITH LINUX KERNEL MEMORY BARRIERS +============================================ + +Here is a list of differences between Linux kernel atomic operations +and memory barriers, and the equivalents in QEMU: + +- atomic operations in Linux are always on a 32-bit int type and + use a boxed atomic_t type; atomic operations in QEMU are polymorphic + and use normal C types. + +- atomic_read and atomic_set in Linux give no guarantee at all; + atomic_read and atomic_set in QEMU include a compiler barrier + (similar to the ACCESS_ONCE macro in Linux). + +- most atomic read-modify-write operations in Linux return void; + in QEMU, all of them return the old value of the variable. + +- different atomic read-modify-write operations in Linux imply + a different set of memory barriers; in QEMU, all of them enforce + sequential consistency, which means they imply full memory barriers + before and after the operation. + +- Linux does not have an equivalent of atomic_mb_read() and + atomic_mb_set(). In particular, note that set_mb() is a little + weaker than atomic_mb_set(). + + +SOURCES +======= + +* Documentation/memory-barriers.txt from the Linux kernel + +* "The JSR-133 Cookbook for Compiler Writers", available at + http://g.oswego.edu/dl/jmm/cookbook.html diff --git a/hw/display/qxl.c b/hw/display/qxl.c index 3862d7aafc..ddefa0668a 100644 --- a/hw/display/qxl.c +++ b/hw/display/qxl.c @@ -23,6 +23,7 @@ #include "qemu-common.h" #include "qemu/timer.h" #include "qemu/queue.h" +#include "qemu/atomic.h" #include "monitor/monitor.h" #include "sysemu/sysemu.h" #include "trace.h" @@ -1726,7 +1727,7 @@ static void qxl_send_events(PCIQXLDevice *d, uint32_t events) trace_qxl_send_events_vm_stopped(d->id, events); return; } - old_pending = __sync_fetch_and_or(&d->ram->int_pending, le_events); + old_pending = atomic_fetch_or(&d->ram->int_pending, le_events); if ((old_pending & le_events) == le_events) { return; } diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c index 96ab62517a..8f6ab130ee 100644 --- a/hw/virtio/vhost.c +++ b/hw/virtio/vhost.c @@ -16,6 +16,7 @@ #include #include "hw/virtio/vhost.h" #include "hw/hw.h" +#include "qemu/atomic.h" #include "qemu/range.h" #include #include "exec/address-spaces.h" @@ -47,11 +48,9 @@ static void vhost_dev_sync_region(struct vhost_dev *dev, addr += VHOST_LOG_CHUNK; continue; } - /* Data must be read atomically. We don't really - * need the barrier semantics of __sync - * builtins, but it's easier to use them than - * roll our own. */ - log = __sync_fetch_and_and(from, 0); + /* Data must be read atomically. We don't really need barrier semantics + * but it's easier to use atomic_* than roll our own. */ + log = atomic_xchg(from, 0); while ((bit = sizeof(log) > sizeof(int) ? ffsll(log) : ffs(log))) { hwaddr page_addr; diff --git a/include/qemu/atomic.h b/include/qemu/atomic.h index 10becb6101..0aa8913301 100644 --- a/include/qemu/atomic.h +++ b/include/qemu/atomic.h @@ -1,68 +1,202 @@ -#ifndef __QEMU_BARRIER_H -#define __QEMU_BARRIER_H 1 +/* + * Simple interface for atomic operations. + * + * Copyright (C) 2013 Red Hat, Inc. + * + * Author: Paolo Bonzini + * + * This work is licensed under the terms of the GNU GPL, version 2 or later. + * See the COPYING file in the top-level directory. + * + */ -/* Compiler barrier */ -#define barrier() asm volatile("" ::: "memory") +#ifndef __QEMU_ATOMIC_H +#define __QEMU_ATOMIC_H 1 -#if defined(__i386__) +#include "qemu/compiler.h" -#include "qemu/compiler.h" /* QEMU_GNUC_PREREQ */ +/* For C11 atomic ops */ -/* - * Because of the strongly ordered x86 storage model, wmb() and rmb() are nops - * on x86(well, a compiler barrier only). Well, at least as long as - * qemu doesn't do accesses to write-combining memory or non-temporal - * load/stores from C code. - */ -#define smp_wmb() barrier() -#define smp_rmb() barrier() +/* Compiler barrier */ +#define barrier() ({ asm volatile("" ::: "memory"); (void)0; }) + +#ifndef __ATOMIC_RELAXED /* - * We use GCC builtin if it's available, as that can use - * mfence on 32 bit as well, e.g. if built with -march=pentium-m. - * However, on i386, there seem to be known bugs as recently as 4.3. - * */ -#if QEMU_GNUC_PREREQ(4, 4) -#define smp_mb() __sync_synchronize() + * We use GCC builtin if it's available, as that can use mfence on + * 32-bit as well, e.g. if built with -march=pentium-m. However, on + * i386 the spec is buggy, and the implementation followed it until + * 4.3 (http://gcc.gnu.org/bugzilla/show_bug.cgi?id=36793). + */ +#if defined(__i386__) || defined(__x86_64__) +#if !QEMU_GNUC_PREREQ(4, 4) +#if defined __x86_64__ +#define smp_mb() ({ asm volatile("mfence" ::: "memory"); (void)0; }) #else -#define smp_mb() asm volatile("lock; addl $0,0(%%esp) " ::: "memory") +#define smp_mb() ({ asm volatile("lock; addl $0,0(%%esp) " ::: "memory"); (void)0; }) +#endif +#endif +#endif + + +#ifdef __alpha__ +#define smp_read_barrier_depends() asm volatile("mb":::"memory") #endif -#elif defined(__x86_64__) +#if defined(__i386__) || defined(__x86_64__) || defined(__s390x__) +/* + * Because of the strongly ordered storage model, wmb() and rmb() are nops + * here (a compiler barrier only). QEMU doesn't do accesses to write-combining + * qemu memory or non-temporal load/stores from C code. + */ #define smp_wmb() barrier() #define smp_rmb() barrier() -#define smp_mb() asm volatile("mfence" ::: "memory") + +/* + * __sync_lock_test_and_set() is documented to be an acquire barrier only, + * but it is a full barrier at the hardware level. Add a compiler barrier + * to make it a full barrier also at the compiler level. + */ +#define atomic_xchg(ptr, i) (barrier(), __sync_lock_test_and_set(ptr, i)) + +/* + * Load/store with Java volatile semantics. + */ +#define atomic_mb_set(ptr, i) ((void)atomic_xchg(ptr, i)) #elif defined(_ARCH_PPC) /* * We use an eieio() for wmb() on powerpc. This assumes we don't * need to order cacheable and non-cacheable stores with respect to - * each other + * each other. + * + * smp_mb has the same problem as on x86 for not-very-new GCC + * (http://patchwork.ozlabs.org/patch/126184/, Nov 2011). */ -#define smp_wmb() asm volatile("eieio" ::: "memory") - +#define smp_wmb() ({ asm volatile("eieio" ::: "memory"); (void)0; }) #if defined(__powerpc64__) -#define smp_rmb() asm volatile("lwsync" ::: "memory") +#define smp_rmb() ({ asm volatile("lwsync" ::: "memory"); (void)0; }) #else -#define smp_rmb() asm volatile("sync" ::: "memory") +#define smp_rmb() ({ asm volatile("sync" ::: "memory"); (void)0; }) #endif +#define smp_mb() ({ asm volatile("sync" ::: "memory"); (void)0; }) -#define smp_mb() asm volatile("sync" ::: "memory") +#endif /* _ARCH_PPC */ -#else +#endif /* C11 atomics */ /* * For (host) platforms we don't have explicit barrier definitions * for, we use the gcc __sync_synchronize() primitive to generate a * full barrier. This should be safe on all platforms, though it may - * be overkill for wmb() and rmb(). + * be overkill for smp_wmb() and smp_rmb(). */ +#ifndef smp_mb +#define smp_mb() __sync_synchronize() +#endif + +#ifndef smp_wmb +#ifdef __ATOMIC_RELEASE +#define smp_wmb() __atomic_thread_fence(__ATOMIC_RELEASE) +#else #define smp_wmb() __sync_synchronize() -#define smp_mb() __sync_synchronize() +#endif +#endif + +#ifndef smp_rmb +#ifdef __ATOMIC_ACQUIRE +#define smp_rmb() __atomic_thread_fence(__ATOMIC_ACQUIRE) +#else #define smp_rmb() __sync_synchronize() +#endif +#endif + +#ifndef smp_read_barrier_depends +#ifdef __ATOMIC_CONSUME +#define smp_read_barrier_depends() __atomic_thread_fence(__ATOMIC_CONSUME) +#else +#define smp_read_barrier_depends() barrier() +#endif +#endif +#ifndef atomic_read +#define atomic_read(ptr) (*(__typeof__(*ptr) *volatile) (ptr)) #endif +#ifndef atomic_set +#define atomic_set(ptr, i) ((*(__typeof__(*ptr) *volatile) (ptr)) = (i)) +#endif + +/* These have the same semantics as Java volatile variables. + * See http://gee.cs.oswego.edu/dl/jmm/cookbook.html: + * "1. Issue a StoreStore barrier (wmb) before each volatile store." + * 2. Issue a StoreLoad barrier after each volatile store. + * Note that you could instead issue one before each volatile load, but + * this would be slower for typical programs using volatiles in which + * reads greatly outnumber writes. Alternatively, if available, you + * can implement volatile store as an atomic instruction (for example + * XCHG on x86) and omit the barrier. This may be more efficient if + * atomic instructions are cheaper than StoreLoad barriers. + * 3. Issue LoadLoad and LoadStore barriers after each volatile load." + * + * If you prefer to think in terms of "pairing" of memory barriers, + * an atomic_mb_read pairs with an atomic_mb_set. + * + * And for the few ia64 lovers that exist, an atomic_mb_read is a ld.acq, + * while an atomic_mb_set is a st.rel followed by a memory barrier. + * + * These are a bit weaker than __atomic_load/store with __ATOMIC_SEQ_CST + * (see docs/atomics.txt), and I'm not sure that __ATOMIC_ACQ_REL is enough. + * Just always use the barriers manually by the rules above. + */ +#ifndef atomic_mb_read +#define atomic_mb_read(ptr) ({ \ + typeof(*ptr) _val = atomic_read(ptr); \ + smp_rmb(); \ + _val; \ +}) +#endif + +#ifndef atomic_mb_set +#define atomic_mb_set(ptr, i) do { \ + smp_wmb(); \ + atomic_set(ptr, i); \ + smp_mb(); \ +} while (0) +#endif + +#ifndef atomic_xchg +#ifdef __ATOMIC_SEQ_CST +#define atomic_xchg(ptr, i) ({ \ + typeof(*ptr) _new = (i), _old; \ + __atomic_exchange(ptr, &_new, &_old, __ATOMIC_SEQ_CST); \ + _old; \ +}) +#elif defined __clang__ +#define atomic_xchg(ptr, i) __sync_exchange(ptr, i) +#else +/* __sync_lock_test_and_set() is documented to be an acquire barrier only. */ +#define atomic_xchg(ptr, i) (smp_mb(), __sync_lock_test_and_set(ptr, i)) +#endif +#endif + +/* Provide shorter names for GCC atomic builtins. */ +#define atomic_fetch_inc(ptr) __sync_fetch_and_add(ptr, 1) +#define atomic_fetch_dec(ptr) __sync_fetch_and_add(ptr, -1) +#define atomic_fetch_add __sync_fetch_and_add +#define atomic_fetch_sub __sync_fetch_and_sub +#define atomic_fetch_and __sync_fetch_and_and +#define atomic_fetch_or __sync_fetch_and_or +#define atomic_cmpxchg __sync_val_compare_and_swap + +/* And even shorter names that return void. */ +#define atomic_inc(ptr) ((void) __sync_fetch_and_add(ptr, 1)) +#define atomic_dec(ptr) ((void) __sync_fetch_and_add(ptr, -1)) +#define atomic_add(ptr, n) ((void) __sync_fetch_and_add(ptr, n)) +#define atomic_sub(ptr, n) ((void) __sync_fetch_and_sub(ptr, n)) +#define atomic_and(ptr, n) ((void) __sync_fetch_and_and(ptr, n)) +#define atomic_or(ptr, n) ((void) __sync_fetch_and_or(ptr, n)) + #endif diff --git a/migration.c b/migration.c index a704d48669..635a7e7a08 100644 --- a/migration.c +++ b/migration.c @@ -293,8 +293,7 @@ static void migrate_fd_cleanup(void *opaque) static void migrate_finish_set_state(MigrationState *s, int new_state) { - if (__sync_val_compare_and_swap(&s->state, MIG_STATE_ACTIVE, - new_state) == new_state) { + if (atomic_cmpxchg(&s->state, MIG_STATE_ACTIVE, new_state) == new_state) { trace_migrate_set_state(new_state); } } diff --git a/tests/test-thread-pool.c b/tests/test-thread-pool.c index 22915aac10..b62338f375 100644 --- a/tests/test-thread-pool.c +++ b/tests/test-thread-pool.c @@ -17,15 +17,15 @@ typedef struct { static int worker_cb(void *opaque) { WorkerTestData *data = opaque; - return __sync_fetch_and_add(&data->n, 1); + return atomic_fetch_inc(&data->n); } static int long_cb(void *opaque) { WorkerTestData *data = opaque; - __sync_fetch_and_add(&data->n, 1); + atomic_inc(&data->n); g_usleep(2000000); - __sync_fetch_and_add(&data->n, 1); + atomic_inc(&data->n); return 0; } @@ -169,7 +169,7 @@ static void test_cancel(void) /* Cancel the jobs that haven't been started yet. */ num_canceled = 0; for (i = 0; i < 100; i++) { - if (__sync_val_compare_and_swap(&data[i].n, 0, 3) == 0) { + if (atomic_cmpxchg(&data[i].n, 0, 3) == 0) { data[i].ret = -ECANCELED; bdrv_aio_cancel(data[i].aiocb); active--; -- cgit 1.4.1