Patchwork Compile dma only once

login
register
mail settings
Submitter Blue Swirl
Date May 18, 2010, 7:51 p.m.
Message ID <AANLkTimqYUZEwZgLVe8QEDWHFfz2kk2qzQ-IlK332D2K@mail.gmail.com>
Download mbox | patch
Permalink /patch/52921/
State New
Headers show

Comments

Blue Swirl - May 18, 2010, 7:51 p.m.
Use a qemu_irq to request CPU exit.

7 compilations less for the full build.

Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
---
 Makefile.objs                        |    1 +
 Makefile.target                      |    6 +++---
 default-configs/i386-softmmu.mak     |    1 +
 default-configs/mips-softmmu.mak     |    1 +
 default-configs/mips64-softmmu.mak   |    1 +
 default-configs/mips64el-softmmu.mak |    1 +
 default-configs/mipsel-softmmu.mak   |    1 +
 default-configs/ppc-softmmu.mak      |    1 +
 default-configs/ppc64-softmmu.mak    |    1 +
 default-configs/ppcemb-softmmu.mak   |    1 +
 default-configs/x86_64-softmmu.mak   |    1 +
 hw/dma.c                             |   17 ++++++++++-------
 hw/isa.h                             |    2 +-
 hw/mips_jazz.c                       |   13 ++++++++++++-
 hw/mips_malta.c                      |   13 ++++++++++++-
 hw/pc.c                              |   14 +++++++++++++-
 hw/ppc_prep.c                        |   15 ++++++++++++++-
 hw/sun4m.c                           |    6 +++++-
 hw/sun4u.c                           |    6 +++++-
 19 files changed, 85 insertions(+), 17 deletions(-)

                            void *opaque)
Paul Brook - May 28, 2010, 7:34 p.m.
> Use a qemu_irq to request CPU exit.

Needing to request a CPU exit at all is just wrong. See previous discussions 
about how any use of qemu_bh_schedule_idle is fundamentally broken.

Paul
Blue Swirl - May 29, 2010, 8:13 a.m.
On Fri, May 28, 2010 at 7:34 PM, Paul Brook <paul@codesourcery.com> wrote:
>> Use a qemu_irq to request CPU exit.
>
> Needing to request a CPU exit at all is just wrong. See previous discussions
> about how any use of qemu_bh_schedule_idle is fundamentally broken.

I agree for the device case. Is the attached patch then OK?

But what about other uses (with the patch applied):

User emulator signal delivery:
/src/qemu/darwin-user/signal.c:216:        cpu_exit(global_env);
/src/qemu/linux-user/signal.c:507:        cpu_exit(thread_env);

qemu_notify_event():
/src/qemu/cpus.c:286:        cpu_exit(env);
/src/qemu/cpus.c:289:        cpu_exit(next_cpu);
Is that broken too and should be removed?

cpu_signal():
/src/qemu/cpus.c:531:        cpu_exit(cpu_single_env);

vm_stop():
/src/qemu/cpus.c:733:            cpu_exit(cpu_single_env);

KVM IO window exit:
/src/qemu/kvm-all.c:859:            cpu_exit(env);

Some exclusive ARM operation:
/src/qemu/linux-user/main.c:152:            cpu_exit(other);

ARM/m68k semihosting:
/src/qemu/gdbstub.c:2296:    cpu_exit(s->c_cpu);
Paul Brook - May 30, 2010, 12:13 a.m.
> On Fri, May 28, 2010 at 7:34 PM, Paul Brook <paul@codesourcery.com> wrote:
> >> Use a qemu_irq to request CPU exit.
> > 
> > Needing to request a CPU exit at all is just wrong. See previous
> > discussions about how any use of qemu_bh_schedule_idle is fundamentally
> > broken.
> 
> I agree for the device case. Is the attached patch then OK?

No. You can't remove code without understanding why it was there in the first 
place.  I'm pretty sure this will break FDC emulation, or at least make it 
unfeasibly slow.

The underlying problem is that devices (in particular the FDC) don't 
communicate properly with the DMA engine.  Instead they rely on the DMA device 
polling state at poorly defined intervals.  Removing DMA_schedule without 
removing qemu_bh_schedule_idle is almost certainly wrong.

My main objection to you original patch is that it introduces a new API for 
something that is just plain wrong. At minimum it needs a comment documenting 
that this function should never be used for anything - It only exists because 
we're too lazy to fix legacy code.

> But what about other uses (with the patch applied):

I was just referring to device emulation.

qemu_notify_event is also pretty bogus.  It is a horrible hack to workaround 
deficiencies in the network API.

Paul

Patch

diff --git a/Makefile.objs b/Makefile.objs
index acbaf22..3dae341 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -155,6 +155,7 @@  hw-obj-$(CONFIG_USB_UHCI) += usb-uhci.o
 hw-obj-$(CONFIG_FDC) += fdc.o
 hw-obj-$(CONFIG_ACPI) += acpi.o acpi_piix4.o
 hw-obj-$(CONFIG_APM) += pm_smbus.o apm.o
+hw-obj-$(CONFIG_DMA) += dma.o

 # PPC devices
 hw-obj-$(CONFIG_OPENPIC) += openpic.o
diff --git a/Makefile.target b/Makefile.target
index a22484e..f2802e5 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -188,7 +188,7 @@  obj-y += rtl8139.o
 obj-y += e1000.o

 # Hardware support
-obj-i386-y = pckbd.o dma.o
+obj-i386-y = pckbd.o
 obj-i386-y += vga.o
 obj-i386-y += mc146818rtc.o i8259.o pc.o
 obj-i386-y += cirrus_vga.o apic.o ioapic.o piix_pci.o
@@ -199,7 +199,7 @@  obj-i386-y += pc_piix.o

 # shared objects
 obj-ppc-y = ppc.o
-obj-ppc-y += vga.o dma.o
+obj-ppc-y += vga.o
 # PREP target
 obj-ppc-y += pckbd.o i8259.o mc146818rtc.o
 obj-ppc-y += ppc_prep.o
@@ -217,7 +217,7 @@  obj-ppc-$(CONFIG_FDT) += device_tree.o

 obj-mips-y = mips_r4k.o mips_jazz.o mips_malta.o mips_mipssim.o
 obj-mips-y += mips_addr.o mips_timer.o mips_int.o
-obj-mips-y += dma.o vga.o i8259.o
+obj-mips-y += vga.o i8259.o
 obj-mips-y += g364fb.o jazz_led.o
 obj-mips-y += gt64xxx.o pckbd.o mc146818rtc.o
 obj-mips-y += piix4.o cirrus_vga.o
diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak
index 4fbd8aa..f916de2 100644
--- a/default-configs/i386-softmmu.mak
+++ b/default-configs/i386-softmmu.mak
@@ -12,6 +12,7 @@  CONFIG_USB_UHCI=y
 CONFIG_FDC=y
 CONFIG_ACPI=y
 CONFIG_APM=y
+CONFIG_DMA=y
 CONFIG_IDE_CORE=y
 CONFIG_IDE_QDEV=y
 CONFIG_IDE_PCI=y
diff --git a/default-configs/mips-softmmu.mak b/default-configs/mips-softmmu.mak
index fd6fbc1..34b4445 100644
--- a/default-configs/mips-softmmu.mak
+++ b/default-configs/mips-softmmu.mak
@@ -14,6 +14,7 @@  CONFIG_USB_UHCI=y
 CONFIG_FDC=y
 CONFIG_ACPI=y
 CONFIG_APM=y
+CONFIG_DMA=y
 CONFIG_IDE_CORE=y
 CONFIG_IDE_QDEV=y
 CONFIG_IDE_PCI=y
diff --git a/default-configs/mips64-softmmu.mak
b/default-configs/mips64-softmmu.mak
index dfd30db..01b72f5 100644
--- a/default-configs/mips64-softmmu.mak
+++ b/default-configs/mips64-softmmu.mak
@@ -14,6 +14,7 @@  CONFIG_USB_UHCI=y
 CONFIG_FDC=y
 CONFIG_ACPI=y
 CONFIG_APM=y
+CONFIG_DMA=y
 CONFIG_IDE_CORE=y
 CONFIG_IDE_QDEV=y
 CONFIG_IDE_PCI=y
diff --git a/default-configs/mips64el-softmmu.mak
b/default-configs/mips64el-softmmu.mak
index 6fa54a3..2cbc828 100644
--- a/default-configs/mips64el-softmmu.mak
+++ b/default-configs/mips64el-softmmu.mak
@@ -14,6 +14,7 @@  CONFIG_USB_UHCI=y
 CONFIG_FDC=y
 CONFIG_ACPI=y
 CONFIG_APM=y
+CONFIG_DMA=y
 CONFIG_IDE_CORE=y
 CONFIG_IDE_QDEV=y
 CONFIG_IDE_PCI=y
diff --git a/default-configs/mipsel-softmmu.mak
b/default-configs/mipsel-softmmu.mak
index 8203997..4f50089 100644
--- a/default-configs/mipsel-softmmu.mak
+++ b/default-configs/mipsel-softmmu.mak
@@ -14,6 +14,7 @@  CONFIG_USB_UHCI=y
 CONFIG_FDC=y
 CONFIG_ACPI=y
 CONFIG_APM=y
+CONFIG_DMA=y
 CONFIG_IDE_CORE=y
 CONFIG_IDE_QDEV=y
 CONFIG_IDE_PCI=y
diff --git a/default-configs/ppc-softmmu.mak b/default-configs/ppc-softmmu.mak
index 958bf75..59db505 100644
--- a/default-configs/ppc-softmmu.mak
+++ b/default-configs/ppc-softmmu.mak
@@ -9,6 +9,7 @@  CONFIG_VGA_PCI=y
 CONFIG_SERIAL=y
 CONFIG_I8254=y
 CONFIG_FDC=y
+CONFIG_DMA=y
 CONFIG_OPENPIC=y
 CONFIG_PREP_PCI=y
 CONFIG_MACIO=y
diff --git a/default-configs/ppc64-softmmu.mak
b/default-configs/ppc64-softmmu.mak
index 9896662..4b7ceba 100644
--- a/default-configs/ppc64-softmmu.mak
+++ b/default-configs/ppc64-softmmu.mak
@@ -9,6 +9,7 @@  CONFIG_VGA_PCI=y
 CONFIG_SERIAL=y
 CONFIG_I8254=y
 CONFIG_FDC=y
+CONFIG_DMA=y
 CONFIG_OPENPIC=y
 CONFIG_PREP_PCI=y
 CONFIG_MACIO=y
diff --git a/default-configs/ppcemb-softmmu.mak
b/default-configs/ppcemb-softmmu.mak
index 2e41a94..9cae6ea 100644
--- a/default-configs/ppcemb-softmmu.mak
+++ b/default-configs/ppcemb-softmmu.mak
@@ -9,6 +9,7 @@  CONFIG_VGA_PCI=y
 CONFIG_SERIAL=y
 CONFIG_I8254=y
 CONFIG_FDC=y
+CONFIG_DMA=y
 CONFIG_OPENPIC=y
 CONFIG_PREP_PCI=y
 CONFIG_MACIO=y
diff --git a/default-configs/x86_64-softmmu.mak
b/default-configs/x86_64-softmmu.mak
index ffedfdb..155647e 100644
--- a/default-configs/x86_64-softmmu.mak
+++ b/default-configs/x86_64-softmmu.mak
@@ -12,6 +12,7 @@  CONFIG_USB_UHCI=y
 CONFIG_FDC=y
 CONFIG_ACPI=y
 CONFIG_APM=y
+CONFIG_DMA=y
 CONFIG_IDE_CORE=y
 CONFIG_IDE_QDEV=y
 CONFIG_IDE_PCI=y
diff --git a/hw/dma.c b/hw/dma.c
index e5f7af7..5b21521 100644
--- a/hw/dma.c
+++ b/hw/dma.c
@@ -57,6 +57,7 @@  static struct dma_cont {
     uint8_t flip_flop;
     int dshift;
     struct dma_regs regs[4];
+    qemu_irq *cpu_request_exit;
 } dma_controllers[2];

 enum {
@@ -444,9 +445,9 @@  int DMA_write_memory (int nchan, void *buf, int
pos, int len)
 /* request the emulator to transfer a new DMA memory block ASAP */
 void DMA_schedule(int nchan)
 {
-    CPUState *env = cpu_single_env;
-    if (env)
-        cpu_exit(env);
+    struct dma_cont *d = &dma_controllers[nchan > 3];
+
+    qemu_irq_pulse(*d->cpu_request_exit);
 }

 static void dma_reset(void *opaque)
@@ -464,12 +465,14 @@  static int dma_phony_handler (void *opaque, int
nchan, int dma_pos, int dma_len)

 /* 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)
+                      int page_base, int pageh_base,
+                      qemu_irq *cpu_request_exit)
 {
     static const int page_port_list[] = { 0x1, 0x2, 0x3, 0x7 };
     int i;

     d->dshift = dshift;
+    d->cpu_request_exit = cpu_request_exit;
     for (i = 0; i < 8; i++) {
         register_ioport_write (base + (i << dshift), 1, 1, write_chan, d);
         register_ioport_read (base + (i << dshift), 1, 1, read_chan, d);
@@ -539,12 +542,12 @@  static const VMStateDescription vmstate_dma = {
     }
 };

-void DMA_init (int high_page_enable)
+void DMA_init(int high_page_enable, qemu_irq *cpu_request_exit)
 {
     dma_init2(&dma_controllers[0], 0x00, 0, 0x80,
-              high_page_enable ? 0x480 : -1);
+              high_page_enable ? 0x480 : -1, cpu_request_exit);
     dma_init2(&dma_controllers[1], 0xc0, 1, 0x88,
-              high_page_enable ? 0x488 : -1);
+              high_page_enable ? 0x488 : -1, cpu_request_exit);
     vmstate_register (0, &vmstate_dma, &dma_controllers[0]);
     vmstate_register (1, &vmstate_dma, &dma_controllers[1]);

diff --git a/hw/isa.h b/hw/isa.h
index 97f69a2..aaf0272 100644
--- a/hw/isa.h
+++ b/hw/isa.h
@@ -41,7 +41,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_init (int high_page_enable);
+void DMA_init(int high_page_enable, qemu_irq *cpu_request_exit);
 void DMA_register_channel (int nchan,
                            DMA_transfer_handler transfer_handler,
                            void *opaque);
diff --git a/hw/mips_jazz.c b/hw/mips_jazz.c
index 6e0ec8f..ead3a00 100644
--- a/hw/mips_jazz.c
+++ b/hw/mips_jazz.c
@@ -114,6 +114,15 @@  static void audio_init(qemu_irq *pic)
 #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 *env = cpu_single_env;
+
+    if (env && level) {
+        cpu_exit(env);
+    }
+}
+
 static
 void mips_jazz_init (ram_addr_t ram_size,
                      const char *cpu_model,
@@ -130,6 +139,7 @@  void mips_jazz_init (ram_addr_t ram_size,
     PITState *pit;
     DriveInfo *fds[MAX_FD];
     qemu_irq esp_reset;
+    qemu_irq *cpu_exit_irq;
     ram_addr_t ram_offset;
     ram_addr_t bios_offset;

@@ -189,7 +199,8 @@  void mips_jazz_init (ram_addr_t ram_size,
     i8259 = i8259_init(env->irq[4]);
     isa_bus_new(NULL);
     isa_bus_irqs(i8259);
-    DMA_init(0);
+    cpu_exit_irq = qemu_allocate_irqs(cpu_request_exit, NULL, 1);
+    DMA_init(0, cpu_exit_irq);
     pit = pit_init(0x40, i8259[0]);
     pcspk_init(pit);

diff --git a/hw/mips_malta.c b/hw/mips_malta.c
index 792709b..a8f9d15 100644
--- a/hw/mips_malta.c
+++ b/hw/mips_malta.c
@@ -763,6 +763,15 @@  static void main_cpu_reset(void *opaque)
     }
 }

+static void cpu_request_exit(void *opaque, int irq, int level)
+{
+    CPUState *env = cpu_single_env;
+
+    if (env && level) {
+        cpu_exit(env);
+    }
+}
+
 static
 void mips_malta_init (ram_addr_t ram_size,
                       const char *boot_device,
@@ -781,6 +790,7 @@  void mips_malta_init (ram_addr_t ram_size,
     FDCtrl *floppy_controller;
     MaltaFPGAState *malta_fpga;
     qemu_irq *i8259;
+    qemu_irq *cpu_exit_irq;
     int piix4_devfn;
     uint8_t *eeprom_buf;
     i2c_bus *smbus;
@@ -943,7 +953,8 @@  void mips_malta_init (ram_addr_t ram_size,
         qdev_init_nofail(eeprom);
     }
     pit = pit_init(0x40, isa_reserve_irq(0));
-    DMA_init(0);
+    cpu_exit_irq = qemu_allocate_irqs(cpu_request_exit, NULL, 1);
+    DMA_init(0, cpu_exit_irq);

     /* Super I/O */
     isa_dev = isa_create_simple("i8042");
diff --git a/hw/pc.c b/hw/pc.c
index 20dc7fd..ec03038 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -928,6 +928,15 @@  void pc_vga_init(PCIBus *pci_bus)
     }
 }

+static void cpu_request_exit(void *opaque, int irq, int level)
+{
+    CPUState *env = cpu_single_env;
+
+    if (env && level) {
+        cpu_exit(env);
+    }
+}
+
 void pc_basic_device_init(qemu_irq *isa_irq,
                           FDCtrl **floppy_controller,
                           ISADevice **rtc_state)
@@ -935,6 +944,7 @@  void pc_basic_device_init(qemu_irq *isa_irq,
     int i;
     DriveInfo *fd[MAX_FD];
     PITState *pit;
+    qemu_irq *cpu_exit_irq;

     register_ioport_write(0x80, 1, 1, ioport80_write, NULL);

@@ -966,7 +976,9 @@  void pc_basic_device_init(qemu_irq *isa_irq,
     }

     isa_create_simple("i8042");
-    DMA_init(0);
+
+    cpu_exit_irq = qemu_allocate_irqs(cpu_request_exit, NULL, 1);
+    DMA_init(0, cpu_exit_irq);

     for(i = 0; i < MAX_FD; i++) {
         fd[i] = drive_get(IF_FLOPPY, 0, i);
diff --git a/hw/ppc_prep.c b/hw/ppc_prep.c
index 09a9881..16c9950 100644
--- a/hw/ppc_prep.c
+++ b/hw/ppc_prep.c
@@ -547,6 +547,15 @@  static CPUReadMemoryFunc * const PPC_prep_io_read[] = {

 #define NVRAM_SIZE        0x2000

+static void cpu_request_exit(void *opaque, int irq, int level)
+{
+    CPUState *env = cpu_single_env;
+
+    if (env && level) {
+        cpu_exit(env);
+    }
+}
+
 /* PowerPC PREP hardware initialisation */
 static void ppc_prep_init (ram_addr_t ram_size,
                            const char *boot_device,
@@ -565,6 +574,7 @@  static void ppc_prep_init (ram_addr_t ram_size,
     uint32_t kernel_base, kernel_size, initrd_base, initrd_size;
     PCIBus *pci_bus;
     qemu_irq *i8259;
+    qemu_irq *cpu_exit_irq;
     int ppc_boot_device;
     DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
     DriveInfo *fd[MAX_FD];
@@ -719,7 +729,10 @@  static void ppc_prep_init (ram_addr_t ram_size,
 		     hd[2 * i + 1]);
     }
     isa_create_simple("i8042");
-    DMA_init(1);
+
+    cpu_exit_irq = qemu_allocate_irqs(cpu_request_exit, NULL, 1);
+    DMA_init(1, cpu_exit_irq);
+
     //    SB16_init();

     for(i = 0; i < MAX_FD; i++) {
diff --git a/hw/sun4m.c b/hw/sun4m.c
index 9a79120..7ba0f76 100644
--- a/hw/sun4m.c
+++ b/hw/sun4m.c
@@ -152,7 +152,11 @@  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_init (int high_page_enable) {}
+
+void DMA_init(int high_page_enable, qemu_irq *cpu_request_exit)
+{
+}
+
 void DMA_register_channel (int nchan,
                            DMA_transfer_handler transfer_handler,
                            void *opaque)
diff --git a/hw/sun4u.c b/hw/sun4u.c
index 24ea367..e9a1e23 100644
--- a/hw/sun4u.c
+++ b/hw/sun4u.c
@@ -105,7 +105,11 @@  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_init (int high_page_enable) {}
+
+void DMA_init(int high_page_enable, qemu_irq *cpu_request_exit)
+{
+}
+
 void DMA_register_channel (int nchan,
                            DMA_transfer_handler transfer_handler,