diff mbox

[v2,2/7] ioapic: convert to qdev

Message ID AANLkTilsy1NLu09wxskkHf9Bkb-O2rWpc02t-zBpo8Cw@mail.gmail.com
State New
Headers show

Commit Message

Blue Swirl June 12, 2010, 9:14 p.m. UTC 
Convert to qdev.

Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
---
 hw/apic.h    |    2 --
 hw/ioapic.c  |   45 ++++++++++++++++++++++++++++++---------------
 hw/pc.h      |    4 +++-
 hw/pc_piix.c |   19 ++++++++++++++++++-
 4 files changed, 51 insertions(+), 19 deletions(-)

Comments

Markus Armbruster June 14, 2010, 9:33 a.m. UTC | #1 
Blue Swirl <blauwirbel@gmail.com> writes:

> Convert to qdev.
>
> Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
> ---
>  hw/apic.h    |    2 --
>  hw/ioapic.c  |   45 ++++++++++++++++++++++++++++++---------------
>  hw/pc.h      |    4 +++-
>  hw/pc_piix.c |   19 ++++++++++++++++++-
>  4 files changed, 51 insertions(+), 19 deletions(-)
>
> diff --git a/hw/apic.h b/hw/apic.h
> index e1954f4..dc41400 100644
> --- a/hw/apic.h
> +++ b/hw/apic.h
> @@ -1,7 +1,6 @@
>  #ifndef APIC_H
>  #define APIC_H
>
> -typedef struct IOAPICState IOAPICState;
>  void apic_deliver_irq(uint8_t dest, uint8_t dest_mode,
>                               uint8_t delivery_mode,
>                               uint8_t vector_num, uint8_t polarity,
> @@ -10,7 +9,6 @@ int apic_init(CPUState *env);
>  int apic_accept_pic_intr(CPUState *env);
>  void apic_deliver_pic_intr(CPUState *env, int level);
>  int apic_get_interrupt(CPUState *env);
> -qemu_irq *ioapic_init(void);
>  void apic_reset_irq_delivered(void);
>  int apic_get_irq_delivered(void);
>
> diff --git a/hw/ioapic.c b/hw/ioapic.c
> index e3f8a46..0336dbd 100644
> --- a/hw/ioapic.c
> +++ b/hw/ioapic.c
> @@ -25,6 +25,7 @@
>  #include "apic.h"
>  #include "qemu-timer.h"
>  #include "host-utils.h"
> +#include "sysbus.h"
>
>  //#define DEBUG_IOAPIC
>
> @@ -35,7 +36,6 @@
>  #define DPRINTF(fmt, ...)
>  #endif
>
> -#define IOAPIC_NUM_PINS			0x18
>  #define IOAPIC_LVT_MASKED 		(1<<16)
>
>  #define IOAPIC_TRIGGER_EDGE		0
> @@ -50,7 +50,10 @@
>  #define IOAPIC_DM_SIPI			0x5
>  #define IOAPIC_DM_EXTINT		0x7
>
> +typedef struct IOAPICState IOAPICState;
> +
>  struct IOAPICState {
> +    SysBusDevice busdev;
>      uint8_t id;
>      uint8_t ioregsel;
>
> @@ -209,12 +212,14 @@ static const VMStateDescription vmstate_ioapic = {
>      }
>  };
>
> -static void ioapic_reset(void *opaque)
> +static void ioapic_reset(DeviceState *d)
>  {
> -    IOAPICState *s = opaque;
> +    IOAPICState *s = container_of(d, IOAPICState, busdev.qdev);

DO_UPCAST()?  I hate it, but it's what the other devices do...

[...]
Blue Swirl June 14, 2010, 5:41 p.m. UTC | #2 
On Mon, Jun 14, 2010 at 9:33 AM, Markus Armbruster <armbru@redhat.com> wrote:
> Blue Swirl <blauwirbel@gmail.com> writes:
>
>> Convert to qdev.
>>
>> Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
>> ---
>>  hw/apic.h    |    2 --
>>  hw/ioapic.c  |   45 ++++++++++++++++++++++++++++++---------------
>>  hw/pc.h      |    4 +++-
>>  hw/pc_piix.c |   19 ++++++++++++++++++-
>>  4 files changed, 51 insertions(+), 19 deletions(-)
>>
>> diff --git a/hw/apic.h b/hw/apic.h
>> index e1954f4..dc41400 100644
>> --- a/hw/apic.h
>> +++ b/hw/apic.h
>> @@ -1,7 +1,6 @@
>>  #ifndef APIC_H
>>  #define APIC_H
>>
>> -typedef struct IOAPICState IOAPICState;
>>  void apic_deliver_irq(uint8_t dest, uint8_t dest_mode,
>>                               uint8_t delivery_mode,
>>                               uint8_t vector_num, uint8_t polarity,
>> @@ -10,7 +9,6 @@ int apic_init(CPUState *env);
>>  int apic_accept_pic_intr(CPUState *env);
>>  void apic_deliver_pic_intr(CPUState *env, int level);
>>  int apic_get_interrupt(CPUState *env);
>> -qemu_irq *ioapic_init(void);
>>  void apic_reset_irq_delivered(void);
>>  int apic_get_irq_delivered(void);
>>
>> diff --git a/hw/ioapic.c b/hw/ioapic.c
>> index e3f8a46..0336dbd 100644
>> --- a/hw/ioapic.c
>> +++ b/hw/ioapic.c
>> @@ -25,6 +25,7 @@
>>  #include "apic.h"
>>  #include "qemu-timer.h"
>>  #include "host-utils.h"
>> +#include "sysbus.h"
>>
>>  //#define DEBUG_IOAPIC
>>
>> @@ -35,7 +36,6 @@
>>  #define DPRINTF(fmt, ...)
>>  #endif
>>
>> -#define IOAPIC_NUM_PINS                      0x18
>>  #define IOAPIC_LVT_MASKED            (1<<16)
>>
>>  #define IOAPIC_TRIGGER_EDGE          0
>> @@ -50,7 +50,10 @@
>>  #define IOAPIC_DM_SIPI                       0x5
>>  #define IOAPIC_DM_EXTINT             0x7
>>
>> +typedef struct IOAPICState IOAPICState;
>> +
>>  struct IOAPICState {
>> +    SysBusDevice busdev;
>>      uint8_t id;
>>      uint8_t ioregsel;
>>
>> @@ -209,12 +212,14 @@ static const VMStateDescription vmstate_ioapic = {
>>      }
>>  };
>>
>> -static void ioapic_reset(void *opaque)
>> +static void ioapic_reset(DeviceState *d)
>>  {
>> -    IOAPICState *s = opaque;
>> +    IOAPICState *s = container_of(d, IOAPICState, busdev.qdev);
>
> DO_UPCAST()?  I hate it, but it's what the other devices do...

Both are used:
grep container_of hw/*.[ch]|wc -l
81
grep DO_UPCAST hw/*.[ch]|wc -l
246
Paul Brook June 15, 2010, 12:25 a.m. UTC | #3 
> >> -static void ioapic_reset(void *opaque)
> >> +static void ioapic_reset(DeviceState *d)
> >>  {
> >> -    IOAPICState *s = opaque;
> >> +    IOAPICState *s = container_of(d, IOAPICState, busdev.qdev);
> > 
> > DO_UPCAST()?  I hate it, but it's what the other devices do...
> 
> Both are used:
> grep container_of hw/*.[ch]|wc -l
> 81
> grep DO_UPCAST hw/*.[ch]|wc -l
> 246

Which is appropriate depends on the context.

DO_UPCAST is appropriate when we're dealing with polymorphic types. In this 
case IOAPICState is derived from DeviceState (via SysBusDevice). There is an 
implicit assumption that these are all the same object, so DO_UPCAST is 
approprate.

container_of is approriate when we're embedding one object in annother, but 
without any inheritance relationship between the two.

Paul
diff mbox

Patch

diff --git a/hw/apic.h b/hw/apic.h
index e1954f4..dc41400 100644
--- a/hw/apic.h
+++ b/hw/apic.h
@@ -1,7 +1,6 @@ 
 #ifndef APIC_H
 #define APIC_H

-typedef struct IOAPICState IOAPICState;
 void apic_deliver_irq(uint8_t dest, uint8_t dest_mode,
                              uint8_t delivery_mode,
                              uint8_t vector_num, uint8_t polarity,
@@ -10,7 +9,6 @@  int apic_init(CPUState *env);
 int apic_accept_pic_intr(CPUState *env);
 void apic_deliver_pic_intr(CPUState *env, int level);
 int apic_get_interrupt(CPUState *env);
-qemu_irq *ioapic_init(void);
 void apic_reset_irq_delivered(void);
 int apic_get_irq_delivered(void);

diff --git a/hw/ioapic.c b/hw/ioapic.c
index e3f8a46..0336dbd 100644
--- a/hw/ioapic.c
+++ b/hw/ioapic.c
@@ -25,6 +25,7 @@ 
 #include "apic.h"
 #include "qemu-timer.h"
 #include "host-utils.h"
+#include "sysbus.h"

 //#define DEBUG_IOAPIC

@@ -35,7 +36,6 @@ 
 #define DPRINTF(fmt, ...)
 #endif

-#define IOAPIC_NUM_PINS			0x18
 #define IOAPIC_LVT_MASKED 		(1<<16)

 #define IOAPIC_TRIGGER_EDGE		0
@@ -50,7 +50,10 @@ 
 #define IOAPIC_DM_SIPI			0x5
 #define IOAPIC_DM_EXTINT		0x7

+typedef struct IOAPICState IOAPICState;
+
 struct IOAPICState {
+    SysBusDevice busdev;
     uint8_t id;
     uint8_t ioregsel;

@@ -209,12 +212,14 @@  static const VMStateDescription vmstate_ioapic = {
     }
 };

-static void ioapic_reset(void *opaque)
+static void ioapic_reset(DeviceState *d)
 {
-    IOAPICState *s = opaque;
+    IOAPICState *s = container_of(d, IOAPICState, busdev.qdev);
     int i;

-    memset(s, 0, sizeof(*s));
+    s->id = 0;
+    s->ioregsel = 0;
+    s->irr = 0;
     for(i = 0; i < IOAPIC_NUM_PINS; i++)
         s->ioredtbl[i] = 1 << 16; /* mask LVT */
 }
@@ -231,22 +236,32 @@  static CPUWriteMemoryFunc * const ioapic_mem_write[3] = {
     ioapic_mem_writel,
 };

-qemu_irq *ioapic_init(void)
+static int ioapic_init1(SysBusDevice *dev)
 {
-    IOAPICState *s;
-    qemu_irq *irq;
+    IOAPICState *s = FROM_SYSBUS(IOAPICState, dev);
     int io_memory;

-    s = qemu_mallocz(sizeof(IOAPICState));
-    ioapic_reset(s);
-
     io_memory = cpu_register_io_memory(ioapic_mem_read,
                                        ioapic_mem_write, s);
-    cpu_register_physical_memory(0xfec00000, 0x1000, io_memory);
+    sysbus_init_mmio(dev, 0x1000, io_memory);

-    vmstate_register(0, &vmstate_ioapic, s);
-    qemu_register_reset(ioapic_reset, s);
-    irq = qemu_allocate_irqs(ioapic_set_irq, s, IOAPIC_NUM_PINS);
+    qdev_init_gpio_in(&dev->qdev, ioapic_set_irq, IOAPIC_NUM_PINS);

-    return irq;
+    return 0;
 }
+
+static SysBusDeviceInfo ioapic_info = {
+    .init = ioapic_init1,
+    .qdev.name = "ioapic",
+    .qdev.size = sizeof(IOAPICState),
+    .qdev.vmsd = &vmstate_ioapic,
+    .qdev.reset = ioapic_reset,
+    .qdev.no_user = 1,
+};
+
+static void ioapic_register_devices(void)
+{
+    sysbus_register_withprop(&ioapic_info);
+}
+
+device_init(ioapic_register_devices)
diff --git a/hw/pc.h b/hw/pc.h
index 0e52933..ccfd7ad 100644
--- a/hw/pc.h
+++ b/hw/pc.h
@@ -39,9 +39,11 @@  void pic_info(Monitor *mon);
 void irq_info(Monitor *mon);

 /* ISA */
+#define IOAPIC_NUM_PINS 0x18
+
 typedef struct isa_irq_state {
     qemu_irq *i8259;
-    qemu_irq *ioapic;
+    qemu_irq ioapic[IOAPIC_NUM_PINS];
 } IsaIrqState;

 void isa_irq_handler(void *opaque, int n, int level);
diff --git a/hw/pc_piix.c b/hw/pc_piix.c
index 70f563a..852727e 100644
--- a/hw/pc_piix.c
+++ b/hw/pc_piix.c
@@ -32,6 +32,7 @@ 
 #include "boards.h"
 #include "ide.h"
 #include "kvm.h"
+#include "sysbus.h"

 #define MAX_IDE_BUS 2

@@ -39,6 +40,22 @@  static const int ide_iobase[MAX_IDE_BUS] = { 0x1f0, 0x170 };
 static const int ide_iobase2[MAX_IDE_BUS] = { 0x3f6, 0x376 };
 static const int ide_irq[MAX_IDE_BUS] = { 14, 15 };

+static void ioapic_init(IsaIrqState *isa_irq_state)
+{
+    DeviceState *dev;
+    SysBusDevice *d;
+    unsigned int i;
+
+    dev = qdev_create(NULL, "ioapic");
+    qdev_init_nofail(dev);
+    d = sysbus_from_qdev(dev);
+    sysbus_mmio_map(d, 0, 0xfec00000);
+
+    for (i = 0; i < IOAPIC_NUM_PINS; i++) {
+        isa_irq_state->ioapic[i] = qdev_get_gpio_in(dev, i);
+    }
+}
+
 /* PC hardware initialisation */
 static void pc_init1(ram_addr_t ram_size,
                      const char *boot_device,
@@ -76,7 +93,7 @@  static void pc_init1(ram_addr_t ram_size,
     isa_irq_state = qemu_mallocz(sizeof(*isa_irq_state));
     isa_irq_state->i8259 = i8259;
     if (pci_enabled) {
-        isa_irq_state->ioapic = ioapic_init();
+        ioapic_init(isa_irq_state);
     }
     isa_irq = qemu_allocate_irqs(isa_irq_handler, isa_irq_state, 24);