Skip to contentSkip to conversation

ioapic: Convert to memory API

PATCH In review Jan Kiszka 3 participants 5 messages Oct 16, 2011 – Oct 17, 2011

lore
Share & tools
lore

Jan Kiszka

author
#0 lore raw

PATCH 1/1 ioapic: Convert to memory API

From: Jan Kiszka <[email protected]>

Dispatching byte and word accesses like dwords looks strange, but let's just convert mechanically.

Signed-off-by Jan Kiszka <[email protected]>
---
 hw/ioapic.c |   24 +++++++++---------------
 1 files changed, 9 insertions(+), 15 deletions(-)
hw/ioapic.c +9 -15
index 61991d7..5786b86 100644
@@ -86,6 +86,7 @@ typedef struct IOAPICState IOAPICState;
 
 struct IOAPICState {
     SysBusDevice busdev;
+    MemoryRegion io_memory;
     uint8_t id;
     uint8_t ioregsel;
     uint32_t irr;

@@ -309,32 +310,25 @@ static void ioapic_reset(DeviceState *d)
     }
 }
 
-static CPUReadMemoryFunc * const ioapic_mem_read[3] = {
-    ioapic_mem_readl,
-    ioapic_mem_readl,
-    ioapic_mem_readl,
-};
-
-static CPUWriteMemoryFunc * const ioapic_mem_write[3] = {
-    ioapic_mem_writel,
-    ioapic_mem_writel,
-    ioapic_mem_writel,
+static const MemoryRegionOps ioapic_io_ops = {
+    .old_mmio = {
+        .read = { ioapic_mem_readl, ioapic_mem_readl, ioapic_mem_readl, },
+        .write = { ioapic_mem_writel, ioapic_mem_writel, ioapic_mem_writel, },
+    },
+    .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
 static int ioapic_init1(SysBusDevice *dev)
 {
     IOAPICState *s = FROM_SYSBUS(IOAPICState, dev);
-    int io_memory;
     static int ioapic_no;
 
     if (ioapic_no >= MAX_IOAPICS) {
         return -1;
     }
 
-    io_memory = cpu_register_io_memory(ioapic_mem_read,
-                                       ioapic_mem_write, s,
-                                       DEVICE_NATIVE_ENDIAN);
-    sysbus_init_mmio(dev, 0x1000, io_memory);
+    memory_region_init_io(&s->io_memory, &ioapic_io_ops, s, "ioapic", 0x1000);
+    sysbus_init_mmio_region(dev, &s->io_memory);
 
     qdev_init_gpio_in(&dev->qdev, ioapic_set_irq, IOAPIC_NUM_PINS);
Signature · 1 line
1.7.3.4
#1 lore raw
Quoted 13 lines · Jan Kiszka

From: Jan Kiszka <[email protected]>

Dispatching byte and word accesses like dwords looks strange, but let's just convert mechanically.

hw/ioapic.c +0 -7
-static CPUReadMemoryFunc * const ioapic_mem_read[3] = {
-    ioapic_mem_readl,
-    ioapic_mem_readl,
-    ioapic_mem_readl,
-};
-
-static CPUWriteMemoryFunc * const ioapic_mem_write[3] = {

hw/ioapic.c static void ioapic_reset(DeviceState *d) · line 319

Quoted 10 lines
hw/ioapic.c +6 -3
-    ioapic_mem_writel,
-    ioapic_mem_writel,
-    ioapic_mem_writel,
+static const MemoryRegionOps ioapic_io_ops = {
+    .old_mmio = {
+        .read = { ioapic_mem_readl, ioapic_mem_readl, ioapic_mem_readl, },
+        .write = { ioapic_mem_writel, ioapic_mem_writel, ioapic_mem_writel, },
+    },
+    .endianness = DEVICE_NATIVE_ENDIAN,
 };

Why use old_mmio? Use the ordinary .read and .write, and ignore the size parameter.

Signature · 1 line
error compiling committee.c: too many arguments to function
#2 lore raw

PATCH v2 1/2 ioapic: Convert to memory API

This maintains the old imprecise access size handling.

Signed-off-by Jan Kiszka <[email protected]>

---

Changes in v2:
- use new-style handlers

 hw/ioapic.c |   28 +++++++++++-----------------
 1 files changed, 11 insertions(+), 17 deletions(-)
hw/ioapic.c +11 -17
index 61991d7..56b1612 100644
@@ -86,6 +86,7 @@ typedef struct IOAPICState IOAPICState;
 
 struct IOAPICState {
     SysBusDevice busdev;
+    MemoryRegion io_memory;
     uint8_t id;
     uint8_t ioregsel;
     uint32_t irr;

@@ -195,7 +196,8 @@ void ioapic_eoi_broadcast(int vector)
     }
 }
 
-static uint32_t ioapic_mem_readl(void *opaque, target_phys_addr_t addr)
+static uint64_t
+ioapic_mem_read(void *opaque, target_phys_addr_t addr, unsigned int size)
 {
     IOAPICState *s = opaque;
     int index;

@@ -234,7 +236,8 @@ static uint32_t ioapic_mem_readl(void *opaque, target_phys_addr_t addr)
 }
 
 static void
-ioapic_mem_writel(void *opaque, target_phys_addr_t addr, uint32_t val)
+ioapic_mem_write(void *opaque, target_phys_addr_t addr, uint64_t val,
+                 unsigned int size)
 {
     IOAPICState *s = opaque;
     int index;

@@ -309,32 +312,23 @@ static void ioapic_reset(DeviceState *d)
     }
 }
 
-static CPUReadMemoryFunc * const ioapic_mem_read[3] = {
-    ioapic_mem_readl,
-    ioapic_mem_readl,
-    ioapic_mem_readl,
-};
-
-static CPUWriteMemoryFunc * const ioapic_mem_write[3] = {
-    ioapic_mem_writel,
-    ioapic_mem_writel,
-    ioapic_mem_writel,
+static const MemoryRegionOps ioapic_io_ops = {
+    .read = ioapic_mem_read,
+    .write = ioapic_mem_write,
+    .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
 static int ioapic_init1(SysBusDevice *dev)
 {
     IOAPICState *s = FROM_SYSBUS(IOAPICState, dev);
-    int io_memory;
     static int ioapic_no;
 
     if (ioapic_no >= MAX_IOAPICS) {
         return -1;
     }
 
-    io_memory = cpu_register_io_memory(ioapic_mem_read,
-                                       ioapic_mem_write, s,
-                                       DEVICE_NATIVE_ENDIAN);
-    sysbus_init_mmio(dev, 0x1000, io_memory);
+    memory_region_init_io(&s->io_memory, &ioapic_io_ops, s, "ioapic", 0x1000);
+    sysbus_init_mmio_region(dev, &s->io_memory);
 
     qdev_init_gpio_in(&dev->qdev, ioapic_set_irq, IOAPIC_NUM_PINS);
Signature · 1 line
1.7.3.4
#4 lore raw

PATCH 2/2 ioapic: Reject non-dword accesses to IOWIN register

In reply to Avi Kivity

Aligns the model with the spec.

Signed-off-by Jan Kiszka <[email protected]>
---
 hw/ioapic.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)
hw/ioapic.c +6 -0
index 56b1612..eb75766 100644
@@ -208,6 +208,9 @@ ioapic_mem_read(void *opaque, target_phys_addr_t addr, unsigned int size)
         val = s->ioregsel;
         break;
     case IOAPIC_IOWIN:
+        if (size != 4) {
+            break;
+        }
         switch (s->ioregsel) {
         case IOAPIC_REG_ID:
             val = s->id << IOAPIC_ID_SHIFT;

@@ -247,6 +250,9 @@ ioapic_mem_write(void *opaque, target_phys_addr_t addr, uint64_t val,
         s->ioregsel = val;
         break;
     case IOAPIC_IOWIN:
+        if (size != 4) {
+            break;
+        }
         DPRINTF("write: %08x = %08x\n", s->ioregsel, val);
         switch (s->ioregsel) {
         case IOAPIC_REG_ID:
Signature · 1 line
1.7.3.4
Applied
#3 lore raw
In reply to Jan Kiszka
Quoted 2 lines · Jan Kiszka

This maintains the old imprecise access size handling.

Thanks, applied both to memory/queue.

Signature · 1 line
error compiling committee.c: too many arguments to function