All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 0/3] Signal-free qemu_cpu_kick for TCG
@ 2015-08-14 13:15 Paolo Bonzini
  2015-08-14 13:15 ` [Qemu-devel] [PATCH 1/3] i8257: rewrite DMA_schedule to avoid hooking into the CPU loop Paolo Bonzini
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Paolo Bonzini @ 2015-08-14 13:15 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell

The first two patches remove most uses of cpu_exit outside the
CPU loop.  The third patch converts qemu_cpu_kick to do memory
accesses from the iothread instead of using a signal.

Paolo

Paolo Bonzini (3):
  i8257: rewrite DMA_schedule to avoid hooking into the CPU loop
  i8257: remove cpu_request_exit irq
  tcg: signal-free qemu_cpu_kick

 cpu-exec.c              | 18 ++++------
 cpus.c                  | 91 ++++++++++++++-----------------------------------
 gdbstub.c               |  2 +-
 hw/block/fdc.c          |  2 +-
 hw/dma/i82374.c         |  5 +--
 hw/dma/i8257.c          | 31 +++++++++--------
 hw/i386/pc.c            | 13 +------
 hw/isa/i82378.c         |  3 +-
 hw/mips/mips_fulong2e.c | 13 +------
 hw/mips/mips_jazz.c     | 13 +------
 hw/mips/mips_malta.c    | 13 +------
 hw/ppc/prep.c           | 11 ------
 hw/ppc/spapr_rtas.c     |  2 +-
 hw/sparc/sun4m.c        |  4 +--
 hw/sparc64/sun4u.c      |  4 +--
 include/hw/isa/isa.h    |  4 +--
 qom/cpu.c               |  2 ++
 17 files changed, 64 insertions(+), 167 deletions(-)

-- 
2.4.3

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [Qemu-devel] [PATCH 1/3] i8257: rewrite DMA_schedule to avoid hooking into the CPU loop
  2015-08-14 13:15 [Qemu-devel] [PATCH 0/3] Signal-free qemu_cpu_kick for TCG Paolo Bonzini
@ 2015-08-14 13:15 ` Paolo Bonzini
  2015-08-14 13:15 ` [Qemu-devel] [PATCH 2/3] i8257: remove cpu_request_exit irq Paolo Bonzini
  2015-08-14 13:15 ` [Qemu-devel] [PATCH 3/3] tcg: signal-free qemu_cpu_kick Paolo Bonzini
  2 siblings, 0 replies; 6+ messages in thread
From: Paolo Bonzini @ 2015-08-14 13:15 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell

The i8257 DMA controller uses an idle bottom half, which by default
does not cause the main loop to exit.  Therefore, the DMA_schedule
function is there to ensure that the CPU relinquishes the iothread
mutex to the iothread.

However, this is not enough since the iothread will call
aio_compute_timeout() and go to sleep again.  In the iothread
world, forcing execution of the idle bottom half is much simpler,
and only requires a call to qemu_notify_event().  Do it, removing
the need for the "cpu_request_exit" pseudo-irq.  The next patch
will remove it.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/block/fdc.c       |  2 +-
 hw/dma/i8257.c       | 18 ++++++++++++------
 hw/sparc/sun4m.c     |  2 +-
 hw/sparc64/sun4u.c   |  2 +-
 include/hw/isa/isa.h |  2 +-
 5 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index 5e1b67e..6686a72 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -1417,7 +1417,7 @@ static void fdctrl_start_transfer(FDCtrl *fdctrl, int direction)
                  * recall us...
                  */
                 DMA_hold_DREQ(fdctrl->dma_chann);
-                DMA_schedule(fdctrl->dma_chann);
+                DMA_schedule();
             } else {
                 /* Start transfer */
                 fdctrl_transfer_handler(fdctrl, fdctrl->dma_chann, 0,
diff --git a/hw/dma/i8257.c b/hw/dma/i8257.c
index a414029..409ba7d 100644
--- a/hw/dma/i8257.c
+++ b/hw/dma/i8257.c
@@ -358,6 +358,7 @@ static void channel_run (int ncont, int ichan)
 }
 
 static QEMUBH *dma_bh;
+static bool dma_bh_scheduled;
 
 static void DMA_run (void)
 {
@@ -390,12 +391,15 @@ static void DMA_run (void)
 
     running = 0;
 out:
-    if (rearm)
+    if (rearm) {
         qemu_bh_schedule_idle(dma_bh);
+        dma_bh_scheduled = true;
+    }
 }
 
 static void DMA_run_bh(void *unused)
 {
+    dma_bh_scheduled = false;
     DMA_run();
 }
 
@@ -458,12 +462,14 @@ int DMA_write_memory (int nchan, void *buf, int pos, int len)
     return len;
 }
 
-/* request the emulator to transfer a new DMA memory block ASAP */
-void DMA_schedule(int nchan)
+/* request the emulator to transfer a new DMA memory block ASAP (even
+ * if the idle bottom half would not have exited the iothread yet).
+ */
+void DMA_schedule(void)
 {
-    struct dma_cont *d = &dma_controllers[nchan > 3];
-
-    qemu_irq_pulse(*d->cpu_request_exit);
+    if (dma_bh_scheduled) {
+        qemu_notify_event();
+    }
 }
 
 static void dma_reset(void *opaque)
diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
index 68ac4d8..ebaae9d 100644
--- a/hw/sparc/sun4m.c
+++ b/hw/sparc/sun4m.c
@@ -109,7 +109,7 @@ int DMA_write_memory (int nchan, void *buf, int pos, int size)
 }
 void DMA_hold_DREQ (int nchan) {}
 void DMA_release_DREQ (int nchan) {}
-void DMA_schedule(int nchan) {}
+void DMA_schedule(void) {}
 
 void DMA_init(int high_page_enable, qemu_irq *cpu_request_exit)
 {
diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
index 30cfa0e..44eb4eb 100644
--- a/hw/sparc64/sun4u.c
+++ b/hw/sparc64/sun4u.c
@@ -112,7 +112,7 @@ int DMA_write_memory (int nchan, void *buf, int pos, int size)
 }
 void DMA_hold_DREQ (int nchan) {}
 void DMA_release_DREQ (int nchan) {}
-void DMA_schedule(int nchan) {}
+void DMA_schedule(void) {}
 
 void DMA_init(int high_page_enable, qemu_irq *cpu_request_exit)
 {
diff --git a/include/hw/isa/isa.h b/include/hw/isa/isa.h
index f21ceaa..81b94ea 100644
--- a/include/hw/isa/isa.h
+++ b/include/hw/isa/isa.h
@@ -112,7 +112,7 @@ int DMA_read_memory (int nchan, void *buf, int pos, int size);
 int DMA_write_memory (int nchan, void *buf, int pos, int size);
 void DMA_hold_DREQ (int nchan);
 void DMA_release_DREQ (int nchan);
-void DMA_schedule(int nchan);
+void DMA_schedule(void);
 void DMA_init(int high_page_enable, qemu_irq *cpu_request_exit);
 void DMA_register_channel (int nchan,
                            DMA_transfer_handler transfer_handler,
-- 
2.4.3

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [Qemu-devel] [PATCH 2/3] i8257: remove cpu_request_exit irq
  2015-08-14 13:15 [Qemu-devel] [PATCH 0/3] Signal-free qemu_cpu_kick for TCG Paolo Bonzini
  2015-08-14 13:15 ` [Qemu-devel] [PATCH 1/3] i8257: rewrite DMA_schedule to avoid hooking into the CPU loop Paolo Bonzini
@ 2015-08-14 13:15 ` Paolo Bonzini
  2015-08-14 13:15 ` [Qemu-devel] [PATCH 3/3] tcg: signal-free qemu_cpu_kick Paolo Bonzini
  2 siblings, 0 replies; 6+ messages in thread
From: Paolo Bonzini @ 2015-08-14 13:15 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell

This is unused.  cpu_exit now is almost exclusively an internal function
to the CPU execution loop.  The next patch will change the remaining
occurrences to qemu_cpu_kick, making it truly internal.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/dma/i82374.c         |  5 +----
 hw/dma/i8257.c          | 13 ++++---------
 hw/i386/pc.c            | 13 +------------
 hw/isa/i82378.c         |  3 +--
 hw/mips/mips_fulong2e.c | 13 +------------
 hw/mips/mips_jazz.c     | 13 +------------
 hw/mips/mips_malta.c    | 13 +------------
 hw/ppc/prep.c           | 11 -----------
 hw/sparc/sun4m.c        |  2 +-
 hw/sparc64/sun4u.c      |  2 +-
 include/hw/isa/isa.h    |  2 +-
 11 files changed, 13 insertions(+), 77 deletions(-)

diff --git a/hw/dma/i82374.c b/hw/dma/i82374.c
index b8ad2e6..f630971 100644
--- a/hw/dma/i82374.c
+++ b/hw/dma/i82374.c
@@ -38,7 +38,6 @@ do { fprintf(stderr, "i82374 ERROR: " fmt , ## __VA_ARGS__); } while (0)
 
 typedef struct I82374State {
     uint8_t commands[8];
-    qemu_irq out;
     PortioList port_list;
 } I82374State;
 
@@ -101,7 +100,7 @@ static uint32_t i82374_read_descriptor(void *opaque, uint32_t nport)
 
 static void i82374_realize(I82374State *s, Error **errp)
 {
-    DMA_init(1, &s->out);
+    DMA_init(1);
     memset(s->commands, 0, sizeof(s->commands));
 }
 
@@ -145,8 +144,6 @@ static void i82374_isa_realize(DeviceState *dev, Error **errp)
                     isa->iobase);
 
     i82374_realize(s, errp);
-
-    qdev_init_gpio_out(dev, &s->out, 1);
 }
 
 static Property i82374_properties[] = {
diff --git a/hw/dma/i8257.c b/hw/dma/i8257.c
index 409ba7d..1398424 100644
--- a/hw/dma/i8257.c
+++ b/hw/dma/i8257.c
@@ -59,7 +59,6 @@ static struct dma_cont {
     uint8_t flip_flop;
     int dshift;
     struct dma_regs regs[4];
-    qemu_irq *cpu_request_exit;
     MemoryRegion channel_io;
     MemoryRegion cont_io;
 } dma_controllers[2];
@@ -521,13 +520,11 @@ static const MemoryRegionOps cont_io_ops = {
 
 /* dshift = 0: 8 bit DMA, 1 = 16 bit DMA */
 static void dma_init2(struct dma_cont *d, int base, int dshift,
-                      int page_base, int pageh_base,
-                      qemu_irq *cpu_request_exit)
+                      int page_base, int pageh_base)
 {
     int i;
 
     d->dshift = dshift;
-    d->cpu_request_exit = cpu_request_exit;
 
     memory_region_init_io(&d->channel_io, NULL, &channel_io_ops, d,
                           "dma-chan", 8 << d->dshift);
@@ -591,12 +588,10 @@ static const VMStateDescription vmstate_dma = {
     }
 };
 
-void DMA_init(int high_page_enable, qemu_irq *cpu_request_exit)
+void DMA_init(int high_page_enable)
 {
-    dma_init2(&dma_controllers[0], 0x00, 0, 0x80,
-              high_page_enable ? 0x480 : -1, cpu_request_exit);
-    dma_init2(&dma_controllers[1], 0xc0, 1, 0x88,
-              high_page_enable ? 0x488 : -1, cpu_request_exit);
+    dma_init2(&dma_controllers[0], 0x00, 0, 0x80, high_page_enable ? 0x480 : -1);
+    dma_init2(&dma_controllers[1], 0xc0, 1, 0x88, high_page_enable ? 0x488 : -1);
     vmstate_register (NULL, 0, &vmstate_dma, &dma_controllers[0]);
     vmstate_register (NULL, 1, &vmstate_dma, &dma_controllers[1]);
 
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 7661ea9..c63a308 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1431,15 +1431,6 @@ DeviceState *pc_vga_init(ISABus *isa_bus, PCIBus *pci_bus)
     return dev;
 }
 
-static void cpu_request_exit(void *opaque, int irq, int level)
-{
-    CPUState *cpu = current_cpu;
-
-    if (cpu && level) {
-        cpu_exit(cpu);
-    }
-}
-
 static const MemoryRegionOps ioport80_io_ops = {
     .write = ioport80_write,
     .read = ioport80_read,
@@ -1474,7 +1465,6 @@ void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi,
     qemu_irq rtc_irq = NULL;
     qemu_irq *a20_line;
     ISADevice *i8042, *port92, *vmmouse, *pit = NULL;
-    qemu_irq *cpu_exit_irq;
     MemoryRegion *ioport80_io = g_new(MemoryRegion, 1);
     MemoryRegion *ioportF0_io = g_new(MemoryRegion, 1);
 
@@ -1551,8 +1541,7 @@ void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi,
     port92 = isa_create_simple(isa_bus, "port92");
     port92_init(port92, &a20_line[1]);
 
-    cpu_exit_irq = qemu_allocate_irqs(cpu_request_exit, NULL, 1);
-    DMA_init(0, cpu_exit_irq);
+    DMA_init(0);
 
     for(i = 0; i < MAX_FD; i++) {
         fd[i] = drive_get(IF_FLOPPY, 0, i);
diff --git a/hw/isa/i82378.c b/hw/isa/i82378.c
index fcf97d8..d4c8306 100644
--- a/hw/isa/i82378.c
+++ b/hw/isa/i82378.c
@@ -100,7 +100,6 @@ static void i82378_realize(PCIDevice *pci, Error **errp)
 
     /* 2 82C37 (dma) */
     isa = isa_create_simple(isabus, "i82374");
-    qdev_connect_gpio_out(DEVICE(isa), 0, s->out[1]);
 
     /* timer */
     isa_create_simple(isabus, "mc146818rtc");
@@ -111,7 +110,7 @@ static void i82378_init(Object *obj)
     DeviceState *dev = DEVICE(obj);
     I82378State *s = I82378(obj);
 
-    qdev_init_gpio_out(dev, s->out, 2);
+    qdev_init_gpio_out(dev, s->out, 1);
     qdev_init_gpio_in(dev, i82378_request_pic_irq, 16);
 }
 
diff --git a/hw/mips/mips_fulong2e.c b/hw/mips/mips_fulong2e.c
index dea941a..6d2ea30 100644
--- a/hw/mips/mips_fulong2e.c
+++ b/hw/mips/mips_fulong2e.c
@@ -251,15 +251,6 @@ static void network_init (PCIBus *pci_bus)
     }
 }
 
-static void cpu_request_exit(void *opaque, int irq, int level)
-{
-    CPUState *cpu = current_cpu;
-
-    if (cpu && level) {
-        cpu_exit(cpu);
-    }
-}
-
 static void mips_fulong2e_init(MachineState *machine)
 {
     ram_addr_t ram_size = machine->ram_size;
@@ -274,7 +265,6 @@ static void mips_fulong2e_init(MachineState *machine)
     long bios_size;
     int64_t kernel_entry;
     qemu_irq *i8259;
-    qemu_irq *cpu_exit_irq;
     PCIBus *pci_bus;
     ISABus *isa_bus;
     I2CBus *smbus;
@@ -375,8 +365,7 @@ static void mips_fulong2e_init(MachineState *machine)
 
     /* init other devices */
     pit = pit_init(isa_bus, 0x40, 0, NULL);
-    cpu_exit_irq = qemu_allocate_irqs(cpu_request_exit, NULL, 1);
-    DMA_init(0, cpu_exit_irq);
+    DMA_init(0);
 
     /* Super I/O */
     isa_create_simple(isa_bus, "i8042");
diff --git a/hw/mips/mips_jazz.c b/hw/mips/mips_jazz.c
index 9d60633..3906016 100644
--- a/hw/mips/mips_jazz.c
+++ b/hw/mips/mips_jazz.c
@@ -104,15 +104,6 @@ static const MemoryRegionOps dma_dummy_ops = {
 #define MAGNUM_BIOS_SIZE_MAX 0x7e000
 #define MAGNUM_BIOS_SIZE (BIOS_SIZE < MAGNUM_BIOS_SIZE_MAX ? BIOS_SIZE : MAGNUM_BIOS_SIZE_MAX)
 
-static void cpu_request_exit(void *opaque, int irq, int level)
-{
-    CPUState *cpu = current_cpu;
-
-    if (cpu && level) {
-        cpu_exit(cpu);
-    }
-}
-
 static CPUUnassignedAccess real_do_unassigned_access;
 static void mips_jazz_do_unassigned_access(CPUState *cpu, hwaddr addr,
                                            bool is_write, bool is_exec,
@@ -150,7 +141,6 @@ static void mips_jazz_init(MachineState *machine,
     ISADevice *pit;
     DriveInfo *fds[MAX_FD];
     qemu_irq esp_reset, dma_enable;
-    qemu_irq *cpu_exit_irq;
     MemoryRegion *ram = g_new(MemoryRegion, 1);
     MemoryRegion *bios = g_new(MemoryRegion, 1);
     MemoryRegion *bios2 = g_new(MemoryRegion, 1);
@@ -234,8 +224,7 @@ static void mips_jazz_init(MachineState *machine,
     /* ISA devices */
     i8259 = i8259_init(isa_bus, env->irq[4]);
     isa_bus_irqs(isa_bus, i8259);
-    cpu_exit_irq = qemu_allocate_irqs(cpu_request_exit, NULL, 1);
-    DMA_init(0, cpu_exit_irq);
+    DMA_init(0);
     pit = pit_init(isa_bus, 0x40, 0, NULL);
     pcspk_init(isa_bus, pit);
 
diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
index 3082e75..23b6fc3 100644
--- a/hw/mips/mips_malta.c
+++ b/hw/mips/mips_malta.c
@@ -905,15 +905,6 @@ static void main_cpu_reset(void *opaque)
     }
 }
 
-static void cpu_request_exit(void *opaque, int irq, int level)
-{
-    CPUState *cpu = current_cpu;
-
-    if (cpu && level) {
-        cpu_exit(cpu);
-    }
-}
-
 static
 void mips_malta_init(MachineState *machine)
 {
@@ -939,7 +930,6 @@ void mips_malta_init(MachineState *machine)
     MIPSCPU *cpu;
     CPUMIPSState *env;
     qemu_irq *isa_irq;
-    qemu_irq *cpu_exit_irq;
     int piix4_devfn;
     I2CBus *smbus;
     int i;
@@ -1175,8 +1165,7 @@ void mips_malta_init(MachineState *machine)
     smbus_eeprom_init(smbus, 8, smbus_eeprom_buf, smbus_eeprom_size);
     g_free(smbus_eeprom_buf);
     pit = pit_init(isa_bus, 0x40, 0, NULL);
-    cpu_exit_irq = qemu_allocate_irqs(cpu_request_exit, NULL, 1);
-    DMA_init(0, cpu_exit_irq);
+    DMA_init(0);
 
     /* Super I/O */
     isa_create_simple(isa_bus, "i8042");
diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
index 45b5f62..81f0838 100644
--- a/hw/ppc/prep.c
+++ b/hw/ppc/prep.c
@@ -336,15 +336,6 @@ static uint32_t PREP_io_800_readb (void *opaque, uint32_t addr)
 
 #define NVRAM_SIZE        0x2000
 
-static void cpu_request_exit(void *opaque, int irq, int level)
-{
-    CPUState *cpu = current_cpu;
-
-    if (cpu && level) {
-        cpu_exit(cpu);
-    }
-}
-
 static void ppc_prep_reset(void *opaque)
 {
     PowerPCCPU *cpu = opaque;
@@ -626,8 +617,6 @@ static void ppc_prep_init(MachineState *machine)
     cpu = POWERPC_CPU(first_cpu);
     qdev_connect_gpio_out(&pci->qdev, 0,
                           cpu->env.irq_inputs[PPC6xx_INPUT_INT]);
-    qdev_connect_gpio_out(&pci->qdev, 1,
-                          qemu_allocate_irq(cpu_request_exit, NULL, 0));
     sysbus_connect_irq(&pcihost->busdev, 0, qdev_get_gpio_in(&pci->qdev, 9));
     sysbus_connect_irq(&pcihost->busdev, 1, qdev_get_gpio_in(&pci->qdev, 11));
     sysbus_connect_irq(&pcihost->busdev, 2, qdev_get_gpio_in(&pci->qdev, 9));
diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
index ebaae9d..b5db8b7 100644
--- a/hw/sparc/sun4m.c
+++ b/hw/sparc/sun4m.c
@@ -111,7 +111,7 @@ void DMA_hold_DREQ (int nchan) {}
 void DMA_release_DREQ (int nchan) {}
 void DMA_schedule(void) {}
 
-void DMA_init(int high_page_enable, qemu_irq *cpu_request_exit)
+void DMA_init(int high_page_enable)
 {
 }
 
diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
index 44eb4eb..a887a86 100644
--- a/hw/sparc64/sun4u.c
+++ b/hw/sparc64/sun4u.c
@@ -114,7 +114,7 @@ void DMA_hold_DREQ (int nchan) {}
 void DMA_release_DREQ (int nchan) {}
 void DMA_schedule(void) {}
 
-void DMA_init(int high_page_enable, qemu_irq *cpu_request_exit)
+void DMA_init(int high_page_enable)
 {
 }
 
diff --git a/include/hw/isa/isa.h b/include/hw/isa/isa.h
index 81b94ea..d758b39 100644
--- a/include/hw/isa/isa.h
+++ b/include/hw/isa/isa.h
@@ -113,7 +113,7 @@ int DMA_write_memory (int nchan, void *buf, int pos, int size);
 void DMA_hold_DREQ (int nchan);
 void DMA_release_DREQ (int nchan);
 void DMA_schedule(void);
-void DMA_init(int high_page_enable, qemu_irq *cpu_request_exit);
+void DMA_init(int high_page_enable);
 void DMA_register_channel (int nchan,
                            DMA_transfer_handler transfer_handler,
                            void *opaque);
-- 
2.4.3

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [Qemu-devel] [PATCH 3/3] tcg: signal-free qemu_cpu_kick
  2015-08-14 13:15 [Qemu-devel] [PATCH 0/3] Signal-free qemu_cpu_kick for TCG Paolo Bonzini
  2015-08-14 13:15 ` [Qemu-devel] [PATCH 1/3] i8257: rewrite DMA_schedule to avoid hooking into the CPU loop Paolo Bonzini
  2015-08-14 13:15 ` [Qemu-devel] [PATCH 2/3] i8257: remove cpu_request_exit irq Paolo Bonzini
@ 2015-08-14 13:15 ` Paolo Bonzini
  2015-08-17 18:31   ` Richard Henderson
  2 siblings, 1 reply; 6+ messages in thread
From: Paolo Bonzini @ 2015-08-14 13:15 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell

Signals are slow and do not exist on Win32.  It is not much more
complicated to use memory barriers (which we already need anyway on
Windows!) and set the existing flags in the iothread.

qemu_cpu_kick_thread is not used anymore on TCG, since the TCG thread
is never outside usermode while the CPU is running (not halted).

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 cpu-exec.c          | 18 ++++-------
 cpus.c              | 91 +++++++++++++++--------------------------------------
 gdbstub.c           |  2 +-
 hw/ppc/spapr_rtas.c |  2 +-
 qom/cpu.c           |  2 ++
 5 files changed, 35 insertions(+), 80 deletions(-)

diff --git a/cpu-exec.c b/cpu-exec.c
index 713540f..069c2eb 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -367,19 +367,10 @@ int cpu_exec(CPUState *cpu)
         cpu->halted = 0;
     }
 
-    current_cpu = cpu;
-
-    /* As long as current_cpu is null, up to the assignment just above,
-     * requests by other threads to exit the execution loop are expected to
-     * be issued using the exit_request global. We must make sure that our
-     * evaluation of the global value is performed past the current_cpu
-     * value transition point, which requires a memory barrier as well as
-     * an instruction scheduling constraint on modern architectures.  */
-    smp_mb();
-
+    atomic_mb_set(&current_cpu, cpu);
     rcu_read_lock();
 
-    if (unlikely(exit_request)) {
+    if (unlikely(atomic_mb_read(&exit_request))) {
         cpu->exit_request = 1;
     }
 
@@ -519,8 +510,11 @@ int cpu_exec(CPUState *cpu)
                          * loop. Whatever requested the exit will also
                          * have set something else (eg exit_request or
                          * interrupt_request) which we will handle
-                         * next time around the loop.
+                         * next time around the loop.  But we need to
+                         * ensure tcg_exit_req is read before exit_request
+                         * or interrupt_request.
                          */
+                        smp_rmb();
                         next_tb = 0;
                         break;
                     case TB_EXIT_ICOUNT_EXPIRED:
diff --git a/cpus.c b/cpus.c
index c1e74d9..0aa02a0 100644
--- a/cpus.c
+++ b/cpus.c
@@ -661,14 +661,6 @@ static void cpu_handle_guest_debug(CPUState *cpu)
     cpu->stopped = true;
 }
 
-static void cpu_signal(int sig)
-{
-    if (current_cpu) {
-        cpu_exit(current_cpu);
-    }
-    exit_request = 1;
-}
-
 #ifdef CONFIG_LINUX
 static void sigbus_reraise(void)
 {
@@ -781,29 +773,11 @@ static void qemu_kvm_init_cpu_signals(CPUState *cpu)
     }
 }
 
-static void qemu_tcg_init_cpu_signals(void)
-{
-    sigset_t set;
-    struct sigaction sigact;
-
-    memset(&sigact, 0, sizeof(sigact));
-    sigact.sa_handler = cpu_signal;
-    sigaction(SIG_IPI, &sigact, NULL);
-
-    sigemptyset(&set);
-    sigaddset(&set, SIG_IPI);
-    pthread_sigmask(SIG_UNBLOCK, &set, NULL);
-}
-
 #else /* _WIN32 */
 static void qemu_kvm_init_cpu_signals(CPUState *cpu)
 {
     abort();
 }
-
-static void qemu_tcg_init_cpu_signals(void)
-{
-}
 #endif /* _WIN32 */
 
 static QemuMutex qemu_global_mutex;
@@ -1041,7 +1015,6 @@ static void *qemu_tcg_cpu_thread_fn(void *arg)
     rcu_register_thread();
 
     qemu_mutex_lock_iothread();
-    qemu_tcg_init_cpu_signals();
     qemu_thread_get_self(cpu->thread);
 
     CPU_FOREACH(cpu) {
@@ -1085,61 +1058,45 @@ static void qemu_cpu_kick_thread(CPUState *cpu)
 #ifndef _WIN32
     int err;
 
+    if (cpu->thread_kicked) {
+        return;
+    }
+    cpu->thread_kicked = true;
     err = pthread_kill(cpu->thread->thread, SIG_IPI);
     if (err) {
         fprintf(stderr, "qemu:%s: %s", __func__, strerror(err));
         exit(1);
     }
 #else /* _WIN32 */
-    if (!qemu_cpu_is_self(cpu)) {
-        CONTEXT tcgContext;
-
-        if (SuspendThread(cpu->hThread) == (DWORD)-1) {
-            fprintf(stderr, "qemu:%s: GetLastError:%lu\n", __func__,
-                    GetLastError());
-            exit(1);
-        }
-
-        /* On multi-core systems, we are not sure that the thread is actually
-         * suspended until we can get the context.
-         */
-        tcgContext.ContextFlags = CONTEXT_CONTROL;
-        while (GetThreadContext(cpu->hThread, &tcgContext) != 0) {
-            continue;
-        }
-
-        cpu_signal(0);
-
-        if (ResumeThread(cpu->hThread) == (DWORD)-1) {
-            fprintf(stderr, "qemu:%s: GetLastError:%lu\n", __func__,
-                    GetLastError());
-            exit(1);
-        }
-    }
+    abort();
 #endif
 }
 
 void qemu_cpu_kick(CPUState *cpu)
 {
     qemu_cond_broadcast(cpu->halt_cond);
-    if (!tcg_enabled() && !cpu->thread_kicked) {
+    if (tcg_enabled()) {
+        /* Ensure whatever caused the exit has reached the CPU threads before
+         * writing exit_request.
+         */
+        smp_wmb();
+        exit_request = 1;
+        /* Ignore the CPU argument since all CPUs run in the same thread;
+         * preempt the currently running one.  The memory barriers ensures
+         * that other CPUs will see the request if the current CPU is
+         * preempted.
+         */
+        smp_wmb();
+        cpu_exit(atomic_rcu_read(&current_cpu));
+    } else {
         qemu_cpu_kick_thread(cpu);
-        cpu->thread_kicked = true;
     }
 }
 
 void qemu_cpu_kick_self(void)
 {
-#ifndef _WIN32
     assert(current_cpu);
-
-    if (!current_cpu->thread_kicked) {
-        qemu_cpu_kick_thread(current_cpu);
-        current_cpu->thread_kicked = true;
-    }
-#else
-    abort();
-#endif
+    qemu_cpu_kick_thread(current_cpu);
 }
 
 bool qemu_cpu_is_self(CPUState *cpu)
@@ -1171,7 +1128,7 @@ void qemu_mutex_lock_iothread(void)
         atomic_dec(&iothread_requesting_mutex);
     } else {
         if (qemu_mutex_trylock(&qemu_global_mutex)) {
-            qemu_cpu_kick_thread(first_cpu);
+            qemu_cpu_kick(first_cpu);
             qemu_mutex_lock(&qemu_global_mutex);
         }
         atomic_dec(&iothread_requesting_mutex);
@@ -1440,7 +1397,9 @@ static void tcg_exec_all(void)
             break;
         }
     }
-    exit_request = 0;
+
+    /* Pairs with smp_wmb in qemu_cpu_kick.  */
+    atomic_mb_set(&exit_request, 0);
 }
 
 void list_cpus(FILE *f, fprintf_function cpu_fprintf, const char *optarg)
diff --git a/gdbstub.c b/gdbstub.c
index ffe7e6e..a5a173a 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1362,7 +1362,7 @@ void gdb_do_syscall(gdb_syscall_complete_cb cb, const char *fmt, ...)
        is still in the running state, which can cause packets to be dropped
        and state transition 'T' packets to be sent while the syscall is still
        being processed.  */
-    cpu_exit(s->c_cpu);
+    qemu_cpu_kick(s->c_cpu);
 #endif
 }
 
diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
index 2986f94..9869bc9 100644
--- a/hw/ppc/spapr_rtas.c
+++ b/hw/ppc/spapr_rtas.c
@@ -214,7 +214,7 @@ static void rtas_stop_self(PowerPCCPU *cpu, sPAPRMachineState *spapr,
     CPUPPCState *env = &cpu->env;
 
     cs->halted = 1;
-    cpu_exit(cs);
+    qemu_cpu_kick(cs);
     /*
      * While stopping a CPU, the guest calls H_CPPR which
      * effectively disables interrupts on XICS level.
diff --git a/qom/cpu.c b/qom/cpu.c
index 62f4b5d..02b56f7 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -114,6 +114,8 @@ void cpu_reset_interrupt(CPUState *cpu, int mask)
 void cpu_exit(CPUState *cpu)
 {
     cpu->exit_request = 1;
+    /* Ensure cpu_exec will see the exit request after TCG has exited.  */
+    smp_wmb();
     cpu->tcg_exit_req = 1;
 }
 
-- 
2.4.3

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH 3/3] tcg: signal-free qemu_cpu_kick
  2015-08-14 13:15 ` [Qemu-devel] [PATCH 3/3] tcg: signal-free qemu_cpu_kick Paolo Bonzini
@ 2015-08-17 18:31   ` Richard Henderson
  2015-08-17 20:47     ` Paolo Bonzini
  0 siblings, 1 reply; 6+ messages in thread
From: Richard Henderson @ 2015-08-17 18:31 UTC (permalink / raw)
  To: Paolo Bonzini, qemu-devel; +Cc: peter.maydell

On 08/14/2015 06:15 AM, Paolo Bonzini wrote:
> +    atomic_mb_set(&current_cpu, cpu);
...
> +        cpu_exit(atomic_rcu_read(&current_cpu));

Mixing java and rcu style sync to the same data structure?

> +                         * ensure tcg_exit_req is read before exit_request
> +                         * or interrupt_request.
>                           */
> +                        smp_rmb();
>                          next_tb = 0;

This I don't understand, since we've just read exit_request above, and you're
putting the barrier here?

> +        /* Ensure whatever caused the exit has reached the CPU threads before
> +         * writing exit_request.
> +         */
> +        smp_wmb();
> +        exit_request = 1;
> +        /* Ignore the CPU argument since all CPUs run in the same thread;
> +         * preempt the currently running one.  The memory barriers ensures
> +         * that other CPUs will see the request if the current CPU is
> +         * preempted.
> +         */
> +        smp_wmb();
> +        cpu_exit(atomic_rcu_read(&current_cpu));

...

> +    /* Pairs with smp_wmb in qemu_cpu_kick.  */
> +    atomic_mb_set(&exit_request, 0);
>  }

Bare barriers and java style sync to the same data structure?

>      cpu->exit_request = 1;
> +    /* Ensure cpu_exec will see the exit request after TCG has exited.  */
> +    smp_wmb();
>      cpu->tcg_exit_req = 1;
>  }

Likewise.

I find this mixing highly confusing.  I see no way to prove that it's going to
be right for non-x86.


r~

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH 3/3] tcg: signal-free qemu_cpu_kick
  2015-08-17 18:31   ` Richard Henderson
@ 2015-08-17 20:47     ` Paolo Bonzini
  0 siblings, 0 replies; 6+ messages in thread
From: Paolo Bonzini @ 2015-08-17 20:47 UTC (permalink / raw)
  To: Richard Henderson, qemu-devel; +Cc: peter.maydell



On 17/08/2015 11:31, Richard Henderson wrote:
> On 08/14/2015 06:15 AM, Paolo Bonzini wrote:
>> +    atomic_mb_set(&current_cpu, cpu);
> ...
>> +        cpu_exit(atomic_rcu_read(&current_cpu));
> 
> Mixing java and rcu style sync to the same data structure?

Well, I usually read rcu_read as CONSUME, rcu_set as RELEASE, mb_read as
either ACQUIRE or "SEQ_CST without IRIW" and mb_set as "SEQ_CST without
IRIW".  But you're right that the patch is unreadable.

>> +                         * ensure tcg_exit_req is read before exit_request
>> +                         * or interrupt_request.
>>                           */
>> +                        smp_rmb();
>>                          next_tb = 0;
> 
> This I don't understand, since we've just read exit_request above, and you're
> putting the barrier here?

If we see cpu->exit_request == 1, we exit.  In that case,
cpu->tcg_exit_req doesn't matter.

Here we saw cpu->exit_request == 0 and then got TB_EXIT_REQUESTED.
Because of TB_EXIT_REQUESTED we know cpu->tcg_exit_req is 1; the
smp_rmb() ensures that cpu->exit_request will be read as 1 on the next
iteration.

Paolo

>> +        /* Ensure whatever caused the exit has reached the CPU threads before
>> +         * writing exit_request.
>> +         */
>> +        smp_wmb();
>> +        exit_request = 1;
>> +        /* Ignore the CPU argument since all CPUs run in the same thread;
>> +         * preempt the currently running one.  The memory barriers ensures
>> +         * that other CPUs will see the request if the current CPU is
>> +         * preempted.
>> +         */
>> +        smp_wmb();
>> +        cpu_exit(atomic_rcu_read(&current_cpu));
> 
> ...
> 
>> +    /* Pairs with smp_wmb in qemu_cpu_kick.  */
>> +    atomic_mb_set(&exit_request, 0);
>>  }
> 
> Bare barriers and java style sync to the same data structure?
> 
>>      cpu->exit_request = 1;
>> +    /* Ensure cpu_exec will see the exit request after TCG has exited.  */
>> +    smp_wmb();
>>      cpu->tcg_exit_req = 1;
>>  }
> 
> Likewise.
> 
> I find this mixing highly confusing.  I see no way to prove that it's going to
> be right for non-x86.
> 
> 
> r~
> 
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-08-17 20:48 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-08-14 13:15 [Qemu-devel] [PATCH 0/3] Signal-free qemu_cpu_kick for TCG Paolo Bonzini
2015-08-14 13:15 ` [Qemu-devel] [PATCH 1/3] i8257: rewrite DMA_schedule to avoid hooking into the CPU loop Paolo Bonzini
2015-08-14 13:15 ` [Qemu-devel] [PATCH 2/3] i8257: remove cpu_request_exit irq Paolo Bonzini
2015-08-14 13:15 ` [Qemu-devel] [PATCH 3/3] tcg: signal-free qemu_cpu_kick Paolo Bonzini
2015-08-17 18:31   ` Richard Henderson
2015-08-17 20:47     ` Paolo Bonzini

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.