All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 00/19] OpenPIC refactoring and MSI support
@ 2012-12-08 13:44 Alexander Graf
  2012-12-08 13:44 ` [Qemu-devel] [PATCH 01/19] openpic: Remove unused code Alexander Graf
                   ` (18 more replies)
  0 siblings, 19 replies; 43+ messages in thread
From: Alexander Graf @ 2012-12-08 13:44 UTC (permalink / raw)
  To: qemu-ppc@nongnu.org List; +Cc: qemu-devel qemu-devel

The OpenPIC implemtation has been a sad piece of code in the QEMU tree
for a long time now. It's ugly, doesn't stick to the coding style and
misses out on a few of the recent infrastructure improvements.

This patch set is a first stab at moving the OpenPIC further towards
maintainable code. It cleans up the OpenPIC/MPIC code duplication that
was in there (MPIC is just an implementation of the OpenPIC spec, so the
naming didn't make sense). It doesn't fully clean up everything yet.

While at it, this patch set also adds support for MSI(-X) to the MPC8544
variant of the OpenPIC. I have successfully tested MSI and MSI-X with
ahci and virtio-net.

Please beware that savevm support is broken after this patch set. I
have quite big doubts that saving a ppc vm works at all right now though,
so let's just leave it broken for now and replace it with a new version
that is going to be savevm based later.

Alex

Alexander Graf (19):
  openpic: Remove unused code
  mpic: Unify numbering scheme
  openpic: update to proper memory api
  openpic: combine mpic and openpic src handlers
  openpic: Convert subregions to memory api
  openpic: combine mpic and openpic irq raise functions
  openpic: merge mpic and openpic timer handling
  openpic: combine openpic and mpic reset functions
  openpic: unify memory api subregions
  openpic: remove unused type variable
  openpic: convert simple reg operations to builtin bitops
  openpic: rename openpic_t to OpenPICState
  openpic: remove irq_out
  openpic: convert to qdev
  openpic: make brr1 model specific
  openpic: add Shared MSI support
  PPC: e500: Add MSI support
  PPC: e500: Declare pci bridge as bridge
  MSI-X: Fix endianness

 hw/msix.c         |    6 +-
 hw/openpic.c      | 1369 ++++++++++++++++++-----------------------------------
 hw/openpic.h      |    7 +-
 hw/ppc/e500.c     |   51 ++-
 hw/ppc_newworld.c |   25 +-
 hw/ppce500_pci.c  |    6 +
 6 files changed, 544 insertions(+), 920 deletions(-)

^ permalink raw reply	[flat|nested] 43+ messages in thread

* [Qemu-devel] [PATCH 01/19] openpic: Remove unused code
  2012-12-08 13:44 [Qemu-devel] [PATCH 00/19] OpenPIC refactoring and MSI support Alexander Graf
@ 2012-12-08 13:44 ` Alexander Graf
  2012-12-08 15:12   ` Andreas Färber
  2012-12-08 13:44 ` [Qemu-devel] [PATCH 02/19] mpic: Unify numbering scheme Alexander Graf
                   ` (17 subsequent siblings)
  18 siblings, 1 reply; 43+ messages in thread
From: Alexander Graf @ 2012-12-08 13:44 UTC (permalink / raw)
  To: qemu-ppc@nongnu.org List; +Cc: qemu-devel qemu-devel

The openpic code had a few WIP bits left that nobody reanimated within
the last few years. Remove that code.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 hw/openpic.c |  163 ----------------------------------------------------------
 1 files changed, 0 insertions(+), 163 deletions(-)

diff --git a/hw/openpic.c b/hw/openpic.c
index 8b3784a..b30c853 100644
--- a/hw/openpic.c
+++ b/hw/openpic.c
@@ -46,27 +46,8 @@
 #define DPRINTF(fmt, ...) do { } while (0)
 #endif
 
-#define USE_MPCxxx /* Intel model is broken, for now */
-
-#if defined (USE_INTEL_GW80314)
-/* Intel GW80314 I/O Companion chip */
-
-#define MAX_CPU     4
-#define MAX_IRQ    32
-#define MAX_DBL     4
-#define MAX_MBX     4
-#define MAX_TMR     4
-#define VECTOR_BITS 8
-#define MAX_IPI     4
-
-#define VID (0x00000000)
-
-#elif defined(USE_MPCxxx)
-
 #define MAX_CPU    15
 #define MAX_IRQ   128
-#define MAX_DBL     0
-#define MAX_MBX     0
 #define MAX_TMR     4
 #define VECTOR_BITS 8
 #define MAX_IPI     4
@@ -149,12 +130,6 @@ enum mpic_ide_bits {
     IDR_P0     = 0,
 };
 
-#else
-#error "Please select which OpenPic implementation is to be emulated"
-#endif
-
-#define OPENPIC_PAGE_SIZE 4096
-
 #define BF_WIDTH(_bits_) \
 (((_bits_) + (sizeof(uint32_t) * 8) - 1) / (sizeof(uint32_t) * 8))
 
@@ -250,19 +225,6 @@ typedef struct openpic_t {
         uint32_t ticc;  /* Global timer current count register */
         uint32_t tibc;  /* Global timer base count register */
     } timers[MAX_TMR];
-#if MAX_DBL > 0
-    /* Doorbell registers */
-    uint32_t dar;        /* Doorbell activate register */
-    struct {
-        uint32_t dmr;    /* Doorbell messaging register */
-    } doorbells[MAX_DBL];
-#endif
-#if MAX_MBX > 0
-    /* Mailbox registers */
-    struct {
-        uint32_t mbr;    /* Mailbox register */
-    } mailboxes[MAX_MAILBOXES];
-#endif
     /* IRQ out is used when in bypass mode (not implemented) */
     qemu_irq irq_out;
     int max_irq;
@@ -470,19 +432,6 @@ static void openpic_reset (void *opaque)
         opp->timers[i].ticc = 0x00000000;
         opp->timers[i].tibc = 0x80000000;
     }
-    /* Initialise doorbells */
-#if MAX_DBL > 0
-    opp->dar = 0x00000000;
-    for (i = 0; i < MAX_DBL; i++) {
-        opp->doorbells[i].dmr  = 0x00000000;
-    }
-#endif
-    /* Initialise mailboxes */
-#if MAX_MBX > 0
-    for (i = 0; i < MAX_MBX; i++) { /* ? */
-        opp->mailboxes[i].mbr   = 0x00000000;
-    }
-#endif
     /* Go out of RESET state */
     opp->glbc = 0x00000000;
 }
@@ -518,84 +467,6 @@ static inline void write_IRQreg_ipvp(openpic_t *opp, int n_IRQ, uint32_t val)
             opp->src[n_IRQ].ipvp);
 }
 
-#if 0 // Code provision for Intel model
-#if MAX_DBL > 0
-static uint32_t read_doorbell_register (openpic_t *opp,
-                                        int n_dbl, uint32_t offset)
-{
-    uint32_t retval;
-
-    switch (offset) {
-    case DBL_IPVP_OFFSET:
-        retval = read_IRQreg_ipvp(opp, IRQ_DBL0 + n_dbl);
-        break;
-    case DBL_IDE_OFFSET:
-        retval = read_IRQreg_ide(opp, IRQ_DBL0 + n_dbl);
-        break;
-    case DBL_DMR_OFFSET:
-        retval = opp->doorbells[n_dbl].dmr;
-        break;
-    }
-
-    return retval;
-}
-
-static void write_doorbell_register (penpic_t *opp, int n_dbl,
-                                     uint32_t offset, uint32_t value)
-{
-    switch (offset) {
-    case DBL_IVPR_OFFSET:
-        write_IRQreg_ipvp(opp, IRQ_DBL0 + n_dbl, value);
-        break;
-    case DBL_IDE_OFFSET:
-        write_IRQreg_ide(opp, IRQ_DBL0 + n_dbl, value);
-        break;
-    case DBL_DMR_OFFSET:
-        opp->doorbells[n_dbl].dmr = value;
-        break;
-    }
-}
-#endif
-
-#if MAX_MBX > 0
-static uint32_t read_mailbox_register (openpic_t *opp,
-                                       int n_mbx, uint32_t offset)
-{
-    uint32_t retval;
-
-    switch (offset) {
-    case MBX_MBR_OFFSET:
-        retval = opp->mailboxes[n_mbx].mbr;
-        break;
-    case MBX_IVPR_OFFSET:
-        retval = read_IRQreg_ipvp(opp, IRQ_MBX0 + n_mbx);
-        break;
-    case MBX_DMR_OFFSET:
-        retval = read_IRQreg_ide(opp, IRQ_MBX0 + n_mbx);
-        break;
-    }
-
-    return retval;
-}
-
-static void write_mailbox_register (openpic_t *opp, int n_mbx,
-                                    uint32_t address, uint32_t value)
-{
-    switch (offset) {
-    case MBX_MBR_OFFSET:
-        opp->mailboxes[n_mbx].mbr = value;
-        break;
-    case MBX_IVPR_OFFSET:
-        write_IRQreg_ipvp(opp, IRQ_MBX0 + n_mbx, value);
-        break;
-    case MBX_DMR_OFFSET:
-        write_IRQreg_ide(opp, IRQ_MBX0 + n_mbx, value);
-        break;
-    }
-}
-#endif
-#endif /* 0 : Code provision for Intel model */
-
 static void openpic_gbl_write (void *opaque, hwaddr addr, uint32_t val)
 {
     openpic_t *opp = opaque;
@@ -841,7 +712,6 @@ static void openpic_cpu_write_internal(void *opaque, hwaddr addr,
     dst = &opp->dst[idx];
     addr &= 0xFF0;
     switch (addr) {
-#if MAX_IPI > 0
     case 0x40: /* IPIDR */
     case 0x50:
     case 0x60:
@@ -853,7 +723,6 @@ static void openpic_cpu_write_internal(void *opaque, hwaddr addr,
         openpic_set_irq(opp, opp->irq_ipi0 + idx, 1);
         openpic_set_irq(opp, opp->irq_ipi0 + idx, 0);
         break;
-#endif
     case 0x80: /* PCTP */
         dst->pctp = val & 0x0000000F;
         break;
@@ -1109,20 +978,6 @@ static void openpic_save(QEMUFile* f, void *opaque)
         qemu_put_be32s(f, &opp->timers[i].tibc);
     }
 
-#if MAX_DBL > 0
-    qemu_put_be32s(f, &opp->dar);
-
-    for (i = 0; i < MAX_DBL; i++) {
-        qemu_put_be32s(f, &opp->doorbells[i].dmr);
-    }
-#endif
-
-#if MAX_MBX > 0
-    for (i = 0; i < MAX_MAILBOXES; i++) {
-        qemu_put_be32s(f, &opp->mailboxes[i].mbr);
-    }
-#endif
-
     pci_device_save(&opp->pci_dev, f);
 }
 
@@ -1176,20 +1031,6 @@ static int openpic_load(QEMUFile* f, void *opaque, int version_id)
         qemu_get_be32s(f, &opp->timers[i].tibc);
     }
 
-#if MAX_DBL > 0
-    qemu_get_be32s(f, &opp->dar);
-
-    for (i = 0; i < MAX_DBL; i++) {
-        qemu_get_be32s(f, &opp->doorbells[i].dmr);
-    }
-#endif
-
-#if MAX_MBX > 0
-    for (i = 0; i < MAX_MAILBOXES; i++) {
-        qemu_get_be32s(f, &opp->mailboxes[i].mbr);
-    }
-#endif
-
     return pci_device_load(&opp->pci_dev, f);
 }
 
@@ -1222,11 +1063,7 @@ qemu_irq *openpic_init (MemoryRegion **pmem, int nb_cpus,
     for (; i < OPENPIC_IRQ_TIM0; i++) {
         opp->src[i].type = IRQ_SPECIAL;
     }
-#if MAX_IPI > 0
     m = OPENPIC_IRQ_IPI0;
-#else
-    m = OPENPIC_IRQ_DBL0;
-#endif
     for (; i < m; i++) {
         opp->src[i].type = IRQ_TIMER;
     }
-- 
1.6.0.2

^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [Qemu-devel] [PATCH 02/19] mpic: Unify numbering scheme
  2012-12-08 13:44 [Qemu-devel] [PATCH 00/19] OpenPIC refactoring and MSI support Alexander Graf
  2012-12-08 13:44 ` [Qemu-devel] [PATCH 01/19] openpic: Remove unused code Alexander Graf
@ 2012-12-08 13:44 ` Alexander Graf
  2012-12-10 23:34   ` [Qemu-devel] [Qemu-ppc] " Scott Wood
  2012-12-08 13:44 ` [Qemu-devel] [PATCH 03/19] openpic: update to proper memory api Alexander Graf
                   ` (16 subsequent siblings)
  18 siblings, 1 reply; 43+ messages in thread
From: Alexander Graf @ 2012-12-08 13:44 UTC (permalink / raw)
  To: qemu-ppc@nongnu.org List; +Cc: qemu-devel qemu-devel

MPIC interrupt numbers in Linux (device tree) and in QEMU are different,
because QEMU takes the sparseness of the IRQ number space into account.

Remove that cleverness and instead assume a flat number space. This makes
the code easier to understand, because we are actually aligned with Linux
on the view of our worlds.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 hw/openpic.c  |  287 ++++++++------------------------------------------------
 hw/ppc/e500.c |    4 +-
 2 files changed, 43 insertions(+), 248 deletions(-)

diff --git a/hw/openpic.c b/hw/openpic.c
index b30c853..0d858cc 100644
--- a/hw/openpic.c
+++ b/hw/openpic.c
@@ -47,7 +47,7 @@
 #endif
 
 #define MAX_CPU    15
-#define MAX_IRQ   128
+#define MAX_IRQ   256
 #define MAX_TMR     4
 #define VECTOR_BITS 8
 #define MAX_IPI     4
@@ -82,32 +82,25 @@ enum {
 #define MPIC_MAX_CPU      1
 #define MPIC_MAX_EXT     12
 #define MPIC_MAX_INT     64
-#define MPIC_MAX_MSG      4
-#define MPIC_MAX_MSI      8
-#define MPIC_MAX_TMR      MAX_TMR
-#define MPIC_MAX_IPI      MAX_IPI
-#define MPIC_MAX_IRQ     (MPIC_MAX_EXT + MPIC_MAX_INT + MPIC_MAX_TMR + MPIC_MAX_MSG + MPIC_MAX_MSI + (MPIC_MAX_IPI * MPIC_MAX_CPU))
+#define MPIC_MAX_IRQ     MAX_IRQ
 
 /* Interrupt definitions */
-#define MPIC_EXT_IRQ      0
-#define MPIC_INT_IRQ      (MPIC_EXT_IRQ + MPIC_MAX_EXT)
-#define MPIC_TMR_IRQ      (MPIC_INT_IRQ + MPIC_MAX_INT)
-#define MPIC_MSG_IRQ      (MPIC_TMR_IRQ + MPIC_MAX_TMR)
-#define MPIC_MSI_IRQ      (MPIC_MSG_IRQ + MPIC_MAX_MSG)
-#define MPIC_IPI_IRQ      (MPIC_MSI_IRQ + MPIC_MAX_MSI)
+/* IRQs, accessible through the IRQ region */
+#define MPIC_EXT_IRQ      0x00
+#define MPIC_INT_IRQ      0x10
+#define MPIC_MSG_IRQ      0xb0
+#define MPIC_MSI_IRQ      0xe0
+/* These are available through separate regions, but
+   for simplicity's sake mapped into the same number space */
+#define MPIC_TMR_IRQ      0xf3
+#define MPIC_IPI_IRQ      0xfb
 
 #define MPIC_GLB_REG_START        0x0
 #define MPIC_GLB_REG_SIZE         0x10F0
 #define MPIC_TMR_REG_START        0x10F0
 #define MPIC_TMR_REG_SIZE         0x220
-#define MPIC_EXT_REG_START        0x10000
-#define MPIC_EXT_REG_SIZE         0x180
-#define MPIC_INT_REG_START        0x10200
-#define MPIC_INT_REG_SIZE         0x800
-#define MPIC_MSG_REG_START        0x11600
-#define MPIC_MSG_REG_SIZE         0x100
-#define MPIC_MSI_REG_START        0x11C00
-#define MPIC_MSI_REG_SIZE         0x100
+#define MPIC_IRQ_REG_START        0x10000
+#define MPIC_IRQ_REG_SIZE         (MAX_IRQ * 0x20)
 #define MPIC_CPU_REG_START        0x20000
 #define MPIC_CPU_REG_SIZE         0x100 + ((MAX_CPU - 1) * 0x1000)
 
@@ -1205,193 +1198,44 @@ static uint32_t mpic_timer_read (void *opaque, hwaddr addr)
     return retval;
 }
 
-static void mpic_src_ext_write (void *opaque, hwaddr addr,
-                                uint32_t val)
+static void mpic_src_irq_write(void *opaque, hwaddr addr,
+                               uint64_t val, unsigned len)
 {
     openpic_t *mpp = opaque;
-    int idx = MPIC_EXT_IRQ;
+    int idx = addr / 0x20;
 
-    DPRINTF("%s: addr " TARGET_FMT_plx " <= %08x\n", __func__, addr, val);
-    if (addr & 0xF)
-        return;
-
-    if (addr < MPIC_EXT_REG_SIZE) {
-        idx += (addr & 0xFFF0) >> 5;
-        if (addr & 0x10) {
-            /* EXDE / IFEDE / IEEDE */
-            write_IRQreg_ide(mpp, idx, val);
-        } else {
-            /* EXVP / IFEVP / IEEVP */
-            write_IRQreg_ipvp(mpp, idx, val);
-        }
-    }
-}
-
-static uint32_t mpic_src_ext_read (void *opaque, hwaddr addr)
-{
-    openpic_t *mpp = opaque;
-    uint32_t retval;
-    int idx = MPIC_EXT_IRQ;
-
-    DPRINTF("%s: addr " TARGET_FMT_plx "\n", __func__, addr);
-    retval = 0xFFFFFFFF;
-    if (addr & 0xF)
-        return retval;
-
-    if (addr < MPIC_EXT_REG_SIZE) {
-        idx += (addr & 0xFFF0) >> 5;
-        if (addr & 0x10) {
-            /* EXDE / IFEDE / IEEDE */
-            retval = read_IRQreg_ide(mpp, idx);
-        } else {
-            /* EXVP / IFEVP / IEEVP */
-            retval = read_IRQreg_ipvp(mpp, idx);
-        }
-        DPRINTF("%s: => %08x\n", __func__, retval);
-    }
-
-    return retval;
-}
-
-static void mpic_src_int_write (void *opaque, hwaddr addr,
-                                uint32_t val)
-{
-    openpic_t *mpp = opaque;
-    int idx = MPIC_INT_IRQ;
-
-    DPRINTF("%s: addr " TARGET_FMT_plx " <= %08x\n", __func__, addr, val);
-    if (addr & 0xF)
-        return;
-
-    if (addr < MPIC_INT_REG_SIZE) {
-        idx += (addr & 0xFFF0) >> 5;
-        if (addr & 0x10) {
-            /* EXDE / IFEDE / IEEDE */
-            write_IRQreg_ide(mpp, idx, val);
-        } else {
-            /* EXVP / IFEVP / IEEVP */
-            write_IRQreg_ipvp(mpp, idx, val);
-        }
-    }
-}
-
-static uint32_t mpic_src_int_read (void *opaque, hwaddr addr)
-{
-    openpic_t *mpp = opaque;
-    uint32_t retval;
-    int idx = MPIC_INT_IRQ;
-
-    DPRINTF("%s: addr " TARGET_FMT_plx "\n", __func__, addr);
-    retval = 0xFFFFFFFF;
-    if (addr & 0xF)
-        return retval;
-
-    if (addr < MPIC_INT_REG_SIZE) {
-        idx += (addr & 0xFFF0) >> 5;
-        if (addr & 0x10) {
-            /* EXDE / IFEDE / IEEDE */
-            retval = read_IRQreg_ide(mpp, idx);
-        } else {
-            /* EXVP / IFEVP / IEEVP */
-            retval = read_IRQreg_ipvp(mpp, idx);
-        }
-        DPRINTF("%s: => %08x\n", __func__, retval);
-    }
-
-    return retval;
-}
-
-static void mpic_src_msg_write (void *opaque, hwaddr addr,
-                                uint32_t val)
-{
-    openpic_t *mpp = opaque;
-    int idx = MPIC_MSG_IRQ;
-
-    DPRINTF("%s: addr " TARGET_FMT_plx " <= %08x\n", __func__, addr, val);
+    DPRINTF("%s: addr " TARGET_FMT_plx " <= %08" PRIx64 "\n",
+            __func__, addr, val);
     if (addr & 0xF)
         return;
 
-    if (addr < MPIC_MSG_REG_SIZE) {
-        idx += (addr & 0xFFF0) >> 5;
-        if (addr & 0x10) {
-            /* EXDE / IFEDE / IEEDE */
-            write_IRQreg_ide(mpp, idx, val);
-        } else {
-            /* EXVP / IFEVP / IEEVP */
-            write_IRQreg_ipvp(mpp, idx, val);
-        }
-    }
-}
-
-static uint32_t mpic_src_msg_read (void *opaque, hwaddr addr)
-{
-    openpic_t *mpp = opaque;
-    uint32_t retval;
-    int idx = MPIC_MSG_IRQ;
-
-    DPRINTF("%s: addr " TARGET_FMT_plx "\n", __func__, addr);
-    retval = 0xFFFFFFFF;
-    if (addr & 0xF)
-        return retval;
-
-    if (addr < MPIC_MSG_REG_SIZE) {
-        idx += (addr & 0xFFF0) >> 5;
-        if (addr & 0x10) {
-            /* EXDE / IFEDE / IEEDE */
-            retval = read_IRQreg_ide(mpp, idx);
-        } else {
-            /* EXVP / IFEVP / IEEVP */
-            retval = read_IRQreg_ipvp(mpp, idx);
-        }
-        DPRINTF("%s: => %08x\n", __func__, retval);
+    if (addr & 0x10) {
+        /* EXDE / IFEDE / IEEDE */
+        write_IRQreg_ide(mpp, idx, val);
+    } else {
+        /* EXVP / IFEVP / IEEVP */
+        write_IRQreg_ipvp(mpp, idx, val);
     }
-
-    return retval;
 }
 
-static void mpic_src_msi_write (void *opaque, hwaddr addr,
-                                uint32_t val)
-{
-    openpic_t *mpp = opaque;
-    int idx = MPIC_MSI_IRQ;
-
-    DPRINTF("%s: addr " TARGET_FMT_plx " <= %08x\n", __func__, addr, val);
-    if (addr & 0xF)
-        return;
-
-    if (addr < MPIC_MSI_REG_SIZE) {
-        idx += (addr & 0xFFF0) >> 5;
-        if (addr & 0x10) {
-            /* EXDE / IFEDE / IEEDE */
-            write_IRQreg_ide(mpp, idx, val);
-        } else {
-            /* EXVP / IFEVP / IEEVP */
-            write_IRQreg_ipvp(mpp, idx, val);
-        }
-    }
-}
-static uint32_t mpic_src_msi_read (void *opaque, hwaddr addr)
+static uint64_t mpic_src_irq_read(void *opaque, hwaddr addr, unsigned len)
 {
     openpic_t *mpp = opaque;
     uint32_t retval;
-    int idx = MPIC_MSI_IRQ;
+    int idx = addr / 0x20;
 
     DPRINTF("%s: addr " TARGET_FMT_plx "\n", __func__, addr);
-    retval = 0xFFFFFFFF;
     if (addr & 0xF)
-        return retval;
+        return -1;
 
-    if (addr < MPIC_MSI_REG_SIZE) {
-        idx += (addr & 0xFFF0) >> 5;
-        if (addr & 0x10) {
-            /* EXDE / IFEDE / IEEDE */
-            retval = read_IRQreg_ide(mpp, idx);
-        } else {
-            /* EXVP / IFEVP / IEEVP */
-            retval = read_IRQreg_ipvp(mpp, idx);
-        }
-        DPRINTF("%s: => %08x\n", __func__, retval);
+    if (addr & 0x10) {
+        /* EXDE / IFEDE / IEEDE */
+        retval = read_IRQreg_ide(mpp, idx);
+    } else {
+        /* EXVP / IFEVP / IEEVP */
+        retval = read_IRQreg_ipvp(mpp, idx);
     }
+    DPRINTF("%s: => %08x\n", __func__, retval);
 
     return retval;
 }
@@ -1438,60 +1282,14 @@ static const MemoryRegionOps mpic_cpu_ops = {
     .endianness = DEVICE_BIG_ENDIAN,
 };
 
-static const MemoryRegionOps mpic_ext_ops = {
-    .old_mmio = {
-        .write = { openpic_buggy_write,
-                   openpic_buggy_write,
-                   mpic_src_ext_write,
-        },
-        .read  = { openpic_buggy_read,
-                   openpic_buggy_read,
-                   mpic_src_ext_read,
-        },
-    },
-    .endianness = DEVICE_BIG_ENDIAN,
-};
-
-static const MemoryRegionOps mpic_int_ops = {
-    .old_mmio = {
-        .write = { openpic_buggy_write,
-                   openpic_buggy_write,
-                   mpic_src_int_write,
-        },
-        .read  = { openpic_buggy_read,
-                   openpic_buggy_read,
-                   mpic_src_int_read,
-        },
-    },
-    .endianness = DEVICE_BIG_ENDIAN,
-};
-
-static const MemoryRegionOps mpic_msg_ops = {
-    .old_mmio = {
-        .write = { openpic_buggy_write,
-                   openpic_buggy_write,
-                   mpic_src_msg_write,
-        },
-        .read  = { openpic_buggy_read,
-                   openpic_buggy_read,
-                   mpic_src_msg_read,
-        },
-    },
+static const MemoryRegionOps mpic_irq_ops = {
+    .write = mpic_src_irq_write,
+    .read  = mpic_src_irq_read,
     .endianness = DEVICE_BIG_ENDIAN,
-};
-
-static const MemoryRegionOps mpic_msi_ops = {
-    .old_mmio = {
-        .write = { openpic_buggy_write,
-                   openpic_buggy_write,
-                   mpic_src_msi_write,
-        },
-        .read  = { openpic_buggy_read,
-                   openpic_buggy_read,
-                   mpic_src_msi_read,
-        },
+    .impl = {
+        .min_access_size = 4,
+        .max_access_size = 4,
     },
-    .endianness = DEVICE_BIG_ENDIAN,
 };
 
 qemu_irq *mpic_init (MemoryRegion *address_space, hwaddr base,
@@ -1507,10 +1305,7 @@ qemu_irq *mpic_init (MemoryRegion *address_space, hwaddr base,
     } const list[] = {
         {"glb", &mpic_glb_ops, MPIC_GLB_REG_START, MPIC_GLB_REG_SIZE},
         {"tmr", &mpic_tmr_ops, MPIC_TMR_REG_START, MPIC_TMR_REG_SIZE},
-        {"ext", &mpic_ext_ops, MPIC_EXT_REG_START, MPIC_EXT_REG_SIZE},
-        {"int", &mpic_int_ops, MPIC_INT_REG_START, MPIC_INT_REG_SIZE},
-        {"msg", &mpic_msg_ops, MPIC_MSG_REG_START, MPIC_MSG_REG_SIZE},
-        {"msi", &mpic_msi_ops, MPIC_MSI_REG_START, MPIC_MSI_REG_SIZE},
+        {"irq", &mpic_irq_ops, MPIC_IRQ_REG_START, MPIC_IRQ_REG_SIZE},
         {"cpu", &mpic_cpu_ops, MPIC_CPU_REG_START, MPIC_CPU_REG_SIZE},
     };
 
diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
index 47e2d41..f3e97d8 100644
--- a/hw/ppc/e500.c
+++ b/hw/ppc/e500.c
@@ -502,13 +502,13 @@ void ppce500_init(PPCE500Params *params)
     /* Serial */
     if (serial_hds[0]) {
         serial_mm_init(ccsr_addr_space, MPC8544_SERIAL0_REGS_OFFSET,
-                       0, mpic[12+26], 399193,
+                       0, mpic[42], 399193,
                        serial_hds[0], DEVICE_BIG_ENDIAN);
     }
 
     if (serial_hds[1]) {
         serial_mm_init(ccsr_addr_space, MPC8544_SERIAL1_REGS_OFFSET,
-                       0, mpic[12+26], 399193,
+                       0, mpic[42], 399193,
                        serial_hds[1], DEVICE_BIG_ENDIAN);
     }
 
-- 
1.6.0.2

^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [Qemu-devel] [PATCH 03/19] openpic: update to proper memory api
  2012-12-08 13:44 [Qemu-devel] [PATCH 00/19] OpenPIC refactoring and MSI support Alexander Graf
  2012-12-08 13:44 ` [Qemu-devel] [PATCH 01/19] openpic: Remove unused code Alexander Graf
  2012-12-08 13:44 ` [Qemu-devel] [PATCH 02/19] mpic: Unify numbering scheme Alexander Graf
@ 2012-12-08 13:44 ` Alexander Graf
  2012-12-08 13:44 ` [Qemu-devel] [PATCH 04/19] openpic: combine mpic and openpic src handlers Alexander Graf
                   ` (15 subsequent siblings)
  18 siblings, 0 replies; 43+ messages in thread
From: Alexander Graf @ 2012-12-08 13:44 UTC (permalink / raw)
  To: qemu-ppc@nongnu.org List; +Cc: qemu-devel qemu-devel

The openpic code was still using the old mmio memory api. Convert it to
be a generic memory api user and clean up some code that becomes redundant
that way.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 hw/openpic.c |  138 ++++++++++++++++++++--------------------------------------
 1 files changed, 48 insertions(+), 90 deletions(-)

diff --git a/hw/openpic.c b/hw/openpic.c
index 0d858cc..2776a36 100644
--- a/hw/openpic.c
+++ b/hw/openpic.c
@@ -460,7 +460,8 @@ static inline void write_IRQreg_ipvp(openpic_t *opp, int n_IRQ, uint32_t val)
             opp->src[n_IRQ].ipvp);
 }
 
-static void openpic_gbl_write (void *opaque, hwaddr addr, uint32_t val)
+static void openpic_gbl_write(void *opaque, hwaddr addr, uint64_t val,
+                              unsigned len)
 {
     openpic_t *opp = opaque;
     IRQ_dst_t *dst;
@@ -526,7 +527,7 @@ static void openpic_gbl_write (void *opaque, hwaddr addr, uint32_t val)
     }
 }
 
-static uint32_t openpic_gbl_read (void *opaque, hwaddr addr)
+static uint64_t openpic_gbl_read(void *opaque, hwaddr addr, unsigned len)
 {
     openpic_t *opp = opaque;
     uint32_t retval;
@@ -583,7 +584,8 @@ static uint32_t openpic_gbl_read (void *opaque, hwaddr addr)
     return retval;
 }
 
-static void openpic_timer_write (void *opaque, uint32_t addr, uint32_t val)
+static void openpic_timer_write(void *opaque, hwaddr addr, uint64_t val,
+                                unsigned len)
 {
     openpic_t *opp = opaque;
     int idx;
@@ -614,7 +616,7 @@ static void openpic_timer_write (void *opaque, uint32_t addr, uint32_t val)
     }
 }
 
-static uint32_t openpic_timer_read (void *opaque, uint32_t addr)
+static uint64_t openpic_timer_read(void *opaque, hwaddr addr, unsigned len)
 {
     openpic_t *opp = opaque;
     uint32_t retval;
@@ -647,7 +649,8 @@ static uint32_t openpic_timer_read (void *opaque, uint32_t addr)
     return retval;
 }
 
-static void openpic_src_write (void *opaque, uint32_t addr, uint32_t val)
+static void openpic_src_write(void *opaque, hwaddr addr, uint64_t val,
+                              unsigned len)
 {
     openpic_t *opp = opaque;
     int idx;
@@ -666,7 +669,7 @@ static void openpic_src_write (void *opaque, uint32_t addr, uint32_t val)
     }
 }
 
-static uint32_t openpic_src_read (void *opaque, uint32_t addr)
+static uint64_t openpic_src_read(void *opaque, uint64_t addr, unsigned len)
 {
     openpic_t *opp = opaque;
     uint32_t retval;
@@ -748,7 +751,8 @@ static void openpic_cpu_write_internal(void *opaque, hwaddr addr,
     }
 }
 
-static void openpic_cpu_write(void *opaque, hwaddr addr, uint32_t val)
+static void openpic_cpu_write(void *opaque, hwaddr addr, uint64_t val,
+                              unsigned len)
 {
     openpic_cpu_write_internal(opaque, addr, val, (addr & 0x1f000) >> 12);
 }
@@ -832,96 +836,63 @@ static uint32_t openpic_cpu_read_internal(void *opaque, hwaddr addr,
     return retval;
 }
 
-static uint32_t openpic_cpu_read(void *opaque, hwaddr addr)
+static uint64_t openpic_cpu_read(void *opaque, hwaddr addr, unsigned len)
 {
     return openpic_cpu_read_internal(opaque, addr, (addr & 0x1f000) >> 12);
 }
 
-static void openpic_buggy_write (void *opaque,
-                                 hwaddr addr, uint32_t val)
-{
-    printf("Invalid OPENPIC write access !\n");
-}
-
-static uint32_t openpic_buggy_read (void *opaque, hwaddr addr)
-{
-    printf("Invalid OPENPIC read access !\n");
-
-    return -1;
-}
-
-static void openpic_writel (void *opaque,
-                            hwaddr addr, uint32_t val)
+static void openpic_write(void *opaque, hwaddr addr, uint64_t val,
+                          unsigned len)
 {
     openpic_t *opp = opaque;
 
-    addr &= 0x3FFFF;
     DPRINTF("%s: offset %08x val: %08x\n", __func__, (int)addr, val);
     if (addr < 0x1100) {
         /* Global registers */
-        openpic_gbl_write(opp, addr, val);
+        openpic_gbl_write(opp, addr, val, len);
     } else if (addr < 0x10000) {
         /* Timers registers */
-        openpic_timer_write(opp, addr, val);
+        openpic_timer_write(opp, addr, val, len);
     } else if (addr < 0x20000) {
         /* Source registers */
-        openpic_src_write(opp, addr, val);
+        openpic_src_write(opp, addr, val, len);
     } else {
         /* CPU registers */
-        openpic_cpu_write(opp, addr, val);
+        openpic_cpu_write(opp, addr, val, len);
     }
 }
 
-static uint32_t openpic_readl (void *opaque,hwaddr addr)
+static uint64_t openpic_read(void *opaque, hwaddr addr, unsigned len)
 {
     openpic_t *opp = opaque;
     uint32_t retval;
 
-    addr &= 0x3FFFF;
     DPRINTF("%s: offset %08x\n", __func__, (int)addr);
     if (addr < 0x1100) {
         /* Global registers */
-        retval = openpic_gbl_read(opp, addr);
+        retval = openpic_gbl_read(opp, addr, len);
     } else if (addr < 0x10000) {
         /* Timers registers */
-        retval = openpic_timer_read(opp, addr);
+        retval = openpic_timer_read(opp, addr, len);
     } else if (addr < 0x20000) {
         /* Source registers */
-        retval = openpic_src_read(opp, addr);
+        retval = openpic_src_read(opp, addr, len);
     } else {
         /* CPU registers */
-        retval = openpic_cpu_read(opp, addr);
+        retval = openpic_cpu_read(opp, addr, len);
     }
 
     return retval;
 }
 
-static uint64_t openpic_read(void *opaque, hwaddr addr,
-                             unsigned size)
-{
-    openpic_t *opp = opaque;
-
-    switch (size) {
-    case 4: return openpic_readl(opp, addr);
-    default: return openpic_buggy_read(opp, addr);
-    }
-}
-
-static void openpic_write(void *opaque, hwaddr addr,
-                          uint64_t data, unsigned size)
-{
-    openpic_t *opp = opaque;
-
-    switch (size) {
-    case 4: return openpic_writel(opp, addr, data);
-    default: return openpic_buggy_write(opp, addr, data);
-    }
-}
-
 static const MemoryRegionOps openpic_ops = {
     .read = openpic_read,
     .write = openpic_write,
     .endianness = DEVICE_LITTLE_ENDIAN,
+    .impl = {
+        .min_access_size = 4,
+        .max_access_size = 4,
+    },
 };
 
 static void openpic_save_IRQ_queue(QEMUFile* f, IRQ_queue_t *q)
@@ -1130,7 +1101,8 @@ static void mpic_reset (void *opaque)
     mpp->glbc = 0x00000000;
 }
 
-static void mpic_timer_write (void *opaque, hwaddr addr, uint32_t val)
+static void mpic_timer_write(void *opaque, hwaddr addr, uint64_t val,
+                             unsigned len)
 {
     openpic_t *mpp = opaque;
     int idx, cpu;
@@ -1138,7 +1110,6 @@ static void mpic_timer_write (void *opaque, hwaddr addr, uint32_t val)
     DPRINTF("%s: addr " TARGET_FMT_plx " <= %08x\n", __func__, addr, val);
     if (addr & 0xF)
         return;
-    addr &= 0xFFFF;
     cpu = addr >> 12;
     idx = (addr >> 6) & 0x3;
     switch (addr & 0x30) {
@@ -1163,7 +1134,7 @@ static void mpic_timer_write (void *opaque, hwaddr addr, uint32_t val)
     }
 }
 
-static uint32_t mpic_timer_read (void *opaque, hwaddr addr)
+static uint64_t mpic_timer_read(void *opaque, hwaddr addr, unsigned len)
 {
     openpic_t *mpp = opaque;
     uint32_t retval;
@@ -1173,7 +1144,6 @@ static uint32_t mpic_timer_read (void *opaque, hwaddr addr)
     retval = 0xFFFFFFFF;
     if (addr & 0xF)
         return retval;
-    addr &= 0xFFFF;
     cpu = addr >> 12;
     idx = (addr >> 6) & 0x3;
     switch (addr & 0x30) {
@@ -1241,45 +1211,33 @@ static uint64_t mpic_src_irq_read(void *opaque, hwaddr addr, unsigned len)
 }
 
 static const MemoryRegionOps mpic_glb_ops = {
-    .old_mmio = {
-        .write = { openpic_buggy_write,
-                   openpic_buggy_write,
-                   openpic_gbl_write,
-        },
-        .read  = { openpic_buggy_read,
-                   openpic_buggy_read,
-                   openpic_gbl_read,
-        },
-    },
+    .write = openpic_gbl_write,
+    .read  = openpic_gbl_read,
     .endianness = DEVICE_BIG_ENDIAN,
+    .impl = {
+        .min_access_size = 4,
+        .max_access_size = 4,
+    },
 };
 
 static const MemoryRegionOps mpic_tmr_ops = {
-    .old_mmio = {
-        .write = { openpic_buggy_write,
-                   openpic_buggy_write,
-                   mpic_timer_write,
-        },
-        .read  = { openpic_buggy_read,
-                   openpic_buggy_read,
-                   mpic_timer_read,
-        },
-    },
+    .write = mpic_timer_write,
+    .read  = mpic_timer_read,
     .endianness = DEVICE_BIG_ENDIAN,
+    .impl = {
+        .min_access_size = 4,
+        .max_access_size = 4,
+    },
 };
 
 static const MemoryRegionOps mpic_cpu_ops = {
-    .old_mmio = {
-        .write = { openpic_buggy_write,
-                   openpic_buggy_write,
-                   openpic_cpu_write,
-        },
-        .read  = { openpic_buggy_read,
-                   openpic_buggy_read,
-                   openpic_cpu_read,
-        },
-    },
+    .write = openpic_cpu_write,
+    .read  = openpic_cpu_read,
     .endianness = DEVICE_BIG_ENDIAN,
+    .impl = {
+        .min_access_size = 4,
+        .max_access_size = 4,
+    },
 };
 
 static const MemoryRegionOps mpic_irq_ops = {
-- 
1.6.0.2

^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [Qemu-devel] [PATCH 04/19] openpic: combine mpic and openpic src handlers
  2012-12-08 13:44 [Qemu-devel] [PATCH 00/19] OpenPIC refactoring and MSI support Alexander Graf
                   ` (2 preceding siblings ...)
  2012-12-08 13:44 ` [Qemu-devel] [PATCH 03/19] openpic: update to proper memory api Alexander Graf
@ 2012-12-08 13:44 ` Alexander Graf
  2012-12-08 13:44 ` [Qemu-devel] [PATCH 05/19] openpic: Convert subregions to memory api Alexander Graf
                   ` (14 subsequent siblings)
  18 siblings, 0 replies; 43+ messages in thread
From: Alexander Graf @ 2012-12-08 13:44 UTC (permalink / raw)
  To: qemu-ppc@nongnu.org List; +Cc: qemu-devel qemu-devel

The MPIC source irq handler suddenly became identical to the standard
OpenPIC source irq handler. Combine them into the same function.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 hw/openpic.c |   52 +++++-----------------------------------------------
 1 files changed, 5 insertions(+), 47 deletions(-)

diff --git a/hw/openpic.c b/hw/openpic.c
index 2776a36..0d65b71 100644
--- a/hw/openpic.c
+++ b/hw/openpic.c
@@ -99,8 +99,8 @@ enum {
 #define MPIC_GLB_REG_SIZE         0x10F0
 #define MPIC_TMR_REG_START        0x10F0
 #define MPIC_TMR_REG_SIZE         0x220
-#define MPIC_IRQ_REG_START        0x10000
-#define MPIC_IRQ_REG_SIZE         (MAX_IRQ * 0x20)
+#define MPIC_SRC_REG_START        0x10000
+#define MPIC_SRC_REG_SIZE         (MAX_IRQ * 0x20)
 #define MPIC_CPU_REG_START        0x20000
 #define MPIC_CPU_REG_SIZE         0x100 + ((MAX_CPU - 1) * 0x1000)
 
@@ -1168,48 +1168,6 @@ static uint64_t mpic_timer_read(void *opaque, hwaddr addr, unsigned len)
     return retval;
 }
 
-static void mpic_src_irq_write(void *opaque, hwaddr addr,
-                               uint64_t val, unsigned len)
-{
-    openpic_t *mpp = opaque;
-    int idx = addr / 0x20;
-
-    DPRINTF("%s: addr " TARGET_FMT_plx " <= %08" PRIx64 "\n",
-            __func__, addr, val);
-    if (addr & 0xF)
-        return;
-
-    if (addr & 0x10) {
-        /* EXDE / IFEDE / IEEDE */
-        write_IRQreg_ide(mpp, idx, val);
-    } else {
-        /* EXVP / IFEVP / IEEVP */
-        write_IRQreg_ipvp(mpp, idx, val);
-    }
-}
-
-static uint64_t mpic_src_irq_read(void *opaque, hwaddr addr, unsigned len)
-{
-    openpic_t *mpp = opaque;
-    uint32_t retval;
-    int idx = addr / 0x20;
-
-    DPRINTF("%s: addr " TARGET_FMT_plx "\n", __func__, addr);
-    if (addr & 0xF)
-        return -1;
-
-    if (addr & 0x10) {
-        /* EXDE / IFEDE / IEEDE */
-        retval = read_IRQreg_ide(mpp, idx);
-    } else {
-        /* EXVP / IFEVP / IEEVP */
-        retval = read_IRQreg_ipvp(mpp, idx);
-    }
-    DPRINTF("%s: => %08x\n", __func__, retval);
-
-    return retval;
-}
-
 static const MemoryRegionOps mpic_glb_ops = {
     .write = openpic_gbl_write,
     .read  = openpic_gbl_read,
@@ -1241,8 +1199,8 @@ static const MemoryRegionOps mpic_cpu_ops = {
 };
 
 static const MemoryRegionOps mpic_irq_ops = {
-    .write = mpic_src_irq_write,
-    .read  = mpic_src_irq_read,
+    .write = openpic_src_write,
+    .read  = openpic_src_read,
     .endianness = DEVICE_BIG_ENDIAN,
     .impl = {
         .min_access_size = 4,
@@ -1263,7 +1221,7 @@ qemu_irq *mpic_init (MemoryRegion *address_space, hwaddr base,
     } const list[] = {
         {"glb", &mpic_glb_ops, MPIC_GLB_REG_START, MPIC_GLB_REG_SIZE},
         {"tmr", &mpic_tmr_ops, MPIC_TMR_REG_START, MPIC_TMR_REG_SIZE},
-        {"irq", &mpic_irq_ops, MPIC_IRQ_REG_START, MPIC_IRQ_REG_SIZE},
+        {"src", &mpic_irq_ops, MPIC_SRC_REG_START, MPIC_SRC_REG_SIZE},
         {"cpu", &mpic_cpu_ops, MPIC_CPU_REG_START, MPIC_CPU_REG_SIZE},
     };
 
-- 
1.6.0.2

^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [Qemu-devel] [PATCH 05/19] openpic: Convert subregions to memory api
  2012-12-08 13:44 [Qemu-devel] [PATCH 00/19] OpenPIC refactoring and MSI support Alexander Graf
                   ` (3 preceding siblings ...)
  2012-12-08 13:44 ` [Qemu-devel] [PATCH 04/19] openpic: combine mpic and openpic src handlers Alexander Graf
@ 2012-12-08 13:44 ` Alexander Graf
  2012-12-08 13:44 ` [Qemu-devel] [PATCH 06/19] openpic: combine mpic and openpic irq raise functions Alexander Graf
                   ` (13 subsequent siblings)
  18 siblings, 0 replies; 43+ messages in thread
From: Alexander Graf @ 2012-12-08 13:44 UTC (permalink / raw)
  To: qemu-ppc@nongnu.org List; +Cc: qemu-devel qemu-devel

The "openpic" controller is currently using one big region and does
subregion dispatching manually. Move this to the memory api.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 hw/openpic.c |  106 +++++++++++++++++++++++++++++++++------------------------
 1 files changed, 61 insertions(+), 45 deletions(-)

diff --git a/hw/openpic.c b/hw/openpic.c
index 0d65b71..29caa20 100644
--- a/hw/openpic.c
+++ b/hw/openpic.c
@@ -78,6 +78,15 @@ enum {
 #define OPENPIC_IRQ_MBX0   (OPENPIC_IRQ_DBL0 + OPENPIC_MAX_DBL) /* First mailbox IRQ */
 #endif
 
+#define OPENPIC_GLB_REG_START        0x0
+#define OPENPIC_GLB_REG_SIZE         0x10F0
+#define OPENPIC_TMR_REG_START        0x10F0
+#define OPENPIC_TMR_REG_SIZE         0x220
+#define OPENPIC_SRC_REG_START        0x10000
+#define OPENPIC_SRC_REG_SIZE         (MAX_IRQ * 0x20)
+#define OPENPIC_CPU_REG_START        0x20000
+#define OPENPIC_CPU_REG_SIZE         0x100 + ((MAX_CPU - 1) * 0x1000)
+
 /* MPIC */
 #define MPIC_MAX_CPU      1
 #define MPIC_MAX_EXT     12
@@ -841,53 +850,39 @@ static uint64_t openpic_cpu_read(void *opaque, hwaddr addr, unsigned len)
     return openpic_cpu_read_internal(opaque, addr, (addr & 0x1f000) >> 12);
 }
 
-static void openpic_write(void *opaque, hwaddr addr, uint64_t val,
-                          unsigned len)
-{
-    openpic_t *opp = opaque;
-
-    DPRINTF("%s: offset %08x val: %08x\n", __func__, (int)addr, val);
-    if (addr < 0x1100) {
-        /* Global registers */
-        openpic_gbl_write(opp, addr, val, len);
-    } else if (addr < 0x10000) {
-        /* Timers registers */
-        openpic_timer_write(opp, addr, val, len);
-    } else if (addr < 0x20000) {
-        /* Source registers */
-        openpic_src_write(opp, addr, val, len);
-    } else {
-        /* CPU registers */
-        openpic_cpu_write(opp, addr, val, len);
-    }
-}
-
-static uint64_t openpic_read(void *opaque, hwaddr addr, unsigned len)
-{
-    openpic_t *opp = opaque;
-    uint32_t retval;
+static const MemoryRegionOps openpic_glb_ops = {
+    .write = openpic_gbl_write,
+    .read  = openpic_gbl_read,
+    .endianness = DEVICE_LITTLE_ENDIAN,
+    .impl = {
+        .min_access_size = 4,
+        .max_access_size = 4,
+    },
+};
 
-    DPRINTF("%s: offset %08x\n", __func__, (int)addr);
-    if (addr < 0x1100) {
-        /* Global registers */
-        retval = openpic_gbl_read(opp, addr, len);
-    } else if (addr < 0x10000) {
-        /* Timers registers */
-        retval = openpic_timer_read(opp, addr, len);
-    } else if (addr < 0x20000) {
-        /* Source registers */
-        retval = openpic_src_read(opp, addr, len);
-    } else {
-        /* CPU registers */
-        retval = openpic_cpu_read(opp, addr, len);
-    }
+static const MemoryRegionOps openpic_tmr_ops = {
+    .write = openpic_timer_write,
+    .read  = openpic_timer_read,
+    .endianness = DEVICE_LITTLE_ENDIAN,
+    .impl = {
+        .min_access_size = 4,
+        .max_access_size = 4,
+    },
+};
 
-    return retval;
-}
+static const MemoryRegionOps openpic_cpu_ops = {
+    .write = openpic_cpu_write,
+    .read  = openpic_cpu_read,
+    .endianness = DEVICE_LITTLE_ENDIAN,
+    .impl = {
+        .min_access_size = 4,
+        .max_access_size = 4,
+    },
+};
 
-static const MemoryRegionOps openpic_ops = {
-    .read = openpic_read,
-    .write = openpic_write,
+static const MemoryRegionOps openpic_src_ops = {
+    .write = openpic_src_write,
+    .read  = openpic_src_read,
     .endianness = DEVICE_LITTLE_ENDIAN,
     .impl = {
         .min_access_size = 4,
@@ -1008,12 +1003,33 @@ qemu_irq *openpic_init (MemoryRegion **pmem, int nb_cpus,
 {
     openpic_t *opp;
     int i, m;
+    struct {
+        const char             *name;
+        MemoryRegionOps const  *ops;
+        hwaddr      start_addr;
+        ram_addr_t              size;
+    } const list[] = {
+        {"glb", &openpic_glb_ops, OPENPIC_GLB_REG_START, OPENPIC_GLB_REG_SIZE},
+        {"tmr", &openpic_tmr_ops, OPENPIC_TMR_REG_START, OPENPIC_TMR_REG_SIZE},
+        {"src", &openpic_src_ops, OPENPIC_SRC_REG_START, OPENPIC_SRC_REG_SIZE},
+        {"cpu", &openpic_cpu_ops, OPENPIC_CPU_REG_START, OPENPIC_CPU_REG_SIZE},
+    };
 
     /* XXX: for now, only one CPU is supported */
     if (nb_cpus != 1)
         return NULL;
     opp = g_malloc0(sizeof(openpic_t));
-    memory_region_init_io(&opp->mem, &openpic_ops, opp, "openpic", 0x40000);
+
+    memory_region_init(&opp->mem, "openpic", 0x40000);
+
+    for (i = 0; i < ARRAY_SIZE(list); i++) {
+
+        memory_region_init_io(&opp->sub_io_mem[i], list[i].ops, opp,
+                              list[i].name, list[i].size);
+
+        memory_region_add_subregion(&opp->mem, list[i].start_addr,
+                                    &opp->sub_io_mem[i]);
+    }
 
     //    isu_base &= 0xFFFC0000;
     opp->nb_cpus = nb_cpus;
-- 
1.6.0.2

^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [Qemu-devel] [PATCH 06/19] openpic: combine mpic and openpic irq raise functions
  2012-12-08 13:44 [Qemu-devel] [PATCH 00/19] OpenPIC refactoring and MSI support Alexander Graf
                   ` (4 preceding siblings ...)
  2012-12-08 13:44 ` [Qemu-devel] [PATCH 05/19] openpic: Convert subregions to memory api Alexander Graf
@ 2012-12-08 13:44 ` Alexander Graf
  2012-12-08 13:44 ` [Qemu-devel] [PATCH 07/19] openpic: merge mpic and openpic timer handling Alexander Graf
                   ` (12 subsequent siblings)
  18 siblings, 0 replies; 43+ messages in thread
From: Alexander Graf @ 2012-12-08 13:44 UTC (permalink / raw)
  To: qemu-ppc@nongnu.org List; +Cc: qemu-devel qemu-devel

The IRQ raise mechanisms of the OpenPIC and MPIC controllers is identical,
just that the MPIC one can also raise critical interrupts.

Combine those two and check for critical raise capability during runtime.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 hw/openpic.c |   34 ++++++++++++++++------------------
 hw/openpic.h |    3 +++
 2 files changed, 19 insertions(+), 18 deletions(-)

diff --git a/hw/openpic.c b/hw/openpic.c
index 29caa20..021bcea 100644
--- a/hw/openpic.c
+++ b/hw/openpic.c
@@ -206,6 +206,9 @@ typedef struct openpic_t {
     PCIDevice pci_dev;
     MemoryRegion mem;
 
+    /* Behavior control */
+    uint32_t flags;
+
     /* Sub-regions */
     MemoryRegion sub_io_mem[7];
 
@@ -233,9 +236,10 @@ typedef struct openpic_t {
     int irq_ipi0;
     int irq_tim0;
     void (*reset) (void *);
-    void (*irq_raise) (struct openpic_t *, int, IRQ_src_t *);
 } openpic_t;
 
+static void openpic_irq_raise(openpic_t *opp, int n_CPU, IRQ_src_t *src);
+
 static inline void IRQ_setbit (IRQ_queue_t *q, int n_IRQ)
 {
     set_bit(q->queue, n_IRQ);
@@ -320,7 +324,7 @@ static void IRQ_local_pipe (openpic_t *opp, int n_CPU, int n_IRQ)
         return;
     }
     DPRINTF("Raise OpenPIC INT output cpu %d irq %d\n", n_CPU, n_IRQ);
-    opp->irq_raise(opp, n_CPU, src);
+    openpic_irq_raise(opp, n_CPU, src);
 }
 
 /* update pic state because registers for n_IRQ have changed value */
@@ -752,7 +756,7 @@ static void openpic_cpu_write_internal(void *opaque, hwaddr addr,
              IPVP_PRIORITY(src->ipvp) > dst->servicing.priority)) {
             DPRINTF("Raise OpenPIC INT output cpu %d irq %d\n",
                     idx, n_IRQ);
-            opp->irq_raise(opp, idx, src);
+            openpic_irq_raise(opp, idx, src);
         }
         break;
     default:
@@ -995,7 +999,13 @@ static int openpic_load(QEMUFile* f, void *opaque, int version_id)
 
 static void openpic_irq_raise(openpic_t *opp, int n_CPU, IRQ_src_t *src)
 {
-    qemu_irq_raise(opp->dst[n_CPU].irqs[OPENPIC_OUTPUT_INT]);
+    int n_ci = IDR_CI0 - n_CPU;
+
+    if ((opp->flags & OPENPIC_FLAG_IDE_CRIT) && test_bit(&src->ide, n_ci)) {
+        qemu_irq_raise(opp->dst[n_CPU].irqs[OPENPIC_OUTPUT_CINT]);
+    } else {
+        qemu_irq_raise(opp->dst[n_CPU].irqs[OPENPIC_OUTPUT_INT]);
+    }
 }
 
 qemu_irq *openpic_init (MemoryRegion **pmem, int nb_cpus,
@@ -1058,7 +1068,6 @@ qemu_irq *openpic_init (MemoryRegion **pmem, int nb_cpus,
                     openpic_save, openpic_load, opp);
     qemu_register_reset(openpic_reset, opp);
 
-    opp->irq_raise = openpic_irq_raise;
     opp->reset = openpic_reset;
 
     if (pmem)
@@ -1067,18 +1076,6 @@ qemu_irq *openpic_init (MemoryRegion **pmem, int nb_cpus,
     return qemu_allocate_irqs(openpic_set_irq, opp, opp->max_irq);
 }
 
-static void mpic_irq_raise(openpic_t *mpp, int n_CPU, IRQ_src_t *src)
-{
-    int n_ci = IDR_CI0 - n_CPU;
-
-    if(test_bit(&src->ide, n_ci)) {
-        qemu_irq_raise(mpp->dst[n_CPU].irqs[OPENPIC_OUTPUT_CINT]);
-    }
-    else {
-        qemu_irq_raise(mpp->dst[n_CPU].irqs[OPENPIC_OUTPUT_INT]);
-    }
-}
-
 static void mpic_reset (void *opaque)
 {
     openpic_t *mpp = (openpic_t *)opaque;
@@ -1264,7 +1261,8 @@ qemu_irq *mpic_init (MemoryRegion *address_space, hwaddr base,
         mpp->dst[i].irqs = irqs[i];
     mpp->irq_out = irq_out;
 
-    mpp->irq_raise = mpic_irq_raise;
+    /* Enable critical interrupt support */
+    mpp->flags |= OPENPIC_FLAG_IDE_CRIT;
     mpp->reset = mpic_reset;
 
     register_savevm(NULL, "mpic", 0, 2, openpic_save, openpic_load, mpp);
diff --git a/hw/openpic.h b/hw/openpic.h
index f50a1e4..1232d10 100644
--- a/hw/openpic.h
+++ b/hw/openpic.h
@@ -11,6 +11,9 @@ enum {
     OPENPIC_OUTPUT_NB,
 };
 
+/* OpenPIC capability flags */
+#define OPENPIC_FLAG_IDE_CRIT    (1 << 0)
+
 qemu_irq *openpic_init (MemoryRegion **pmem, int nb_cpus,
                         qemu_irq **irqs, qemu_irq irq_out);
 qemu_irq *mpic_init (MemoryRegion *address_space, hwaddr base,
-- 
1.6.0.2

^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [Qemu-devel] [PATCH 07/19] openpic: merge mpic and openpic timer handling
  2012-12-08 13:44 [Qemu-devel] [PATCH 00/19] OpenPIC refactoring and MSI support Alexander Graf
                   ` (5 preceding siblings ...)
  2012-12-08 13:44 ` [Qemu-devel] [PATCH 06/19] openpic: combine mpic and openpic irq raise functions Alexander Graf
@ 2012-12-08 13:44 ` Alexander Graf
  2012-12-08 13:44 ` [Qemu-devel] [PATCH 08/19] openpic: combine openpic and mpic reset functions Alexander Graf
                   ` (11 subsequent siblings)
  18 siblings, 0 replies; 43+ messages in thread
From: Alexander Graf @ 2012-12-08 13:44 UTC (permalink / raw)
  To: qemu-ppc@nongnu.org List; +Cc: qemu-devel qemu-devel

The openpic and mpic timer handling code is basically the same.
Merge them.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 hw/openpic.c |  131 ++++++++++++++--------------------------------------------
 1 files changed, 31 insertions(+), 100 deletions(-)

diff --git a/hw/openpic.c b/hw/openpic.c
index 021bcea..b413bba 100644
--- a/hw/openpic.c
+++ b/hw/openpic.c
@@ -194,7 +194,6 @@ enum IPVP_bits {
 #define IPVP_VECTOR(_ipvpr_)   ((_ipvpr_) & IPVP_VECTOR_MASK)
 
 typedef struct IRQ_dst_t {
-    uint32_t tfrr;
     uint32_t pctp; /* CPU current task priority */
     uint32_t pcsr; /* CPU sensitivity register */
     IRQ_queue_t raised;
@@ -532,9 +531,6 @@ static void openpic_gbl_write(void *opaque, hwaddr addr, uint64_t val,
     case 0x10E0: /* SPVE */
         opp->spve = val & 0x000000FF;
         break;
-    case 0x10F0: /* TIFR */
-        opp->tifr = val;
-        break;
     default:
         break;
     }
@@ -586,9 +582,6 @@ static uint64_t openpic_gbl_read(void *opaque, hwaddr addr, unsigned len)
     case 0x10E0: /* SPVE */
         retval = opp->spve;
         break;
-    case 0x10F0: /* TIFR */
-        retval = opp->tifr;
-        break;
     default:
         break;
     }
@@ -606,24 +599,28 @@ static void openpic_timer_write(void *opaque, hwaddr addr, uint64_t val,
     DPRINTF("%s: addr %08x <= %08x\n", __func__, addr, val);
     if (addr & 0xF)
         return;
-    addr -= 0x10;
-    addr &= 0xFFFF;
-    idx = (addr & 0xFFF0) >> 6;
+    idx = (addr >> 6) & 0x3;
     addr = addr & 0x30;
-    switch (addr) {
-    case 0x00: /* TICC */
+
+    if (addr == 0x0) {
+        /* TIFR (TFRR) */
+        opp->tifr = val;
+        return;
+    }
+    switch (addr & 0x30) {
+    case 0x00: /* TICC (GTCCR) */
         break;
-    case 0x10: /* TIBC */
+    case 0x10: /* TIBC (GTBCR) */
         if ((opp->timers[idx].ticc & 0x80000000) != 0 &&
             (val & 0x80000000) == 0 &&
             (opp->timers[idx].tibc & 0x80000000) != 0)
             opp->timers[idx].ticc &= ~0x80000000;
         opp->timers[idx].tibc = val;
         break;
-    case 0x20: /* TIVP */
+    case 0x20: /* TIVP (GTIVPR) */
         write_IRQreg_ipvp(opp, opp->irq_tim0 + idx, val);
         break;
-    case 0x30: /* TIDE */
+    case 0x30: /* TIDE (GTIDR) */
         write_IRQreg_ide(opp, opp->irq_tim0 + idx, val);
         break;
     }
@@ -632,31 +629,35 @@ static void openpic_timer_write(void *opaque, hwaddr addr, uint64_t val,
 static uint64_t openpic_timer_read(void *opaque, hwaddr addr, unsigned len)
 {
     openpic_t *opp = opaque;
-    uint32_t retval;
+    uint32_t retval = -1;
     int idx;
 
     DPRINTF("%s: addr %08x\n", __func__, addr);
-    retval = 0xFFFFFFFF;
-    if (addr & 0xF)
-        return retval;
-    addr -= 0x10;
-    addr &= 0xFFFF;
-    idx = (addr & 0xFFF0) >> 6;
-    addr = addr & 0x30;
-    switch (addr) {
-    case 0x00: /* TICC */
+    if (addr & 0xF) {
+        goto out;
+    }
+    idx = (addr >> 6) & 0x3;
+    if (addr == 0x0) {
+        /* TIFR (TFRR) */
+        retval = opp->tifr;
+        goto out;
+    }
+    switch (addr & 0x30) {
+    case 0x00: /* TICC (GTCCR) */
         retval = opp->timers[idx].ticc;
         break;
-    case 0x10: /* TIBC */
+    case 0x10: /* TIBC (GTBCR) */
         retval = opp->timers[idx].tibc;
         break;
-    case 0x20: /* TIPV */
+    case 0x20: /* TIPV (TIPV) */
         retval = read_IRQreg_ipvp(opp, opp->irq_tim0 + idx);
         break;
-    case 0x30: /* TIDE */
+    case 0x30: /* TIDE (TIDR) */
         retval = read_IRQreg_ide(opp, opp->irq_tim0 + idx);
         break;
     }
+
+out:
     DPRINTF("%s: => %08x\n", __func__, retval);
 
     return retval;
@@ -929,7 +930,6 @@ static void openpic_save(QEMUFile* f, void *opaque)
     qemu_put_sbe32s(f, &opp->nb_cpus);
 
     for (i = 0; i < opp->nb_cpus; i++) {
-        qemu_put_be32s(f, &opp->dst[i].tfrr);
         qemu_put_be32s(f, &opp->dst[i].pctp);
         qemu_put_be32s(f, &opp->dst[i].pcsr);
         openpic_save_IRQ_queue(f, &opp->dst[i].raised);
@@ -982,7 +982,6 @@ static int openpic_load(QEMUFile* f, void *opaque, int version_id)
     qemu_get_sbe32s(f, &opp->nb_cpus);
 
     for (i = 0; i < opp->nb_cpus; i++) {
-        qemu_get_be32s(f, &opp->dst[i].tfrr);
         qemu_get_be32s(f, &opp->dst[i].pctp);
         qemu_get_be32s(f, &opp->dst[i].pcsr);
         openpic_load_IRQ_queue(f, &opp->dst[i].raised);
@@ -1099,7 +1098,6 @@ static void mpic_reset (void *opaque)
     /* Initialise IRQ destinations */
     for (i = 0; i < MAX_CPU; i++) {
         mpp->dst[i].pctp      = 0x0000000F;
-        mpp->dst[i].tfrr      = 0x00000000;
         memset(&mpp->dst[i].raised, 0, sizeof(IRQ_queue_t));
         mpp->dst[i].raised.next = -1;
         memset(&mpp->dst[i].servicing, 0, sizeof(IRQ_queue_t));
@@ -1114,73 +1112,6 @@ static void mpic_reset (void *opaque)
     mpp->glbc = 0x00000000;
 }
 
-static void mpic_timer_write(void *opaque, hwaddr addr, uint64_t val,
-                             unsigned len)
-{
-    openpic_t *mpp = opaque;
-    int idx, cpu;
-
-    DPRINTF("%s: addr " TARGET_FMT_plx " <= %08x\n", __func__, addr, val);
-    if (addr & 0xF)
-        return;
-    cpu = addr >> 12;
-    idx = (addr >> 6) & 0x3;
-    switch (addr & 0x30) {
-    case 0x00: /* gtccr */
-        break;
-    case 0x10: /* gtbcr */
-        if ((mpp->timers[idx].ticc & 0x80000000) != 0 &&
-            (val & 0x80000000) == 0 &&
-            (mpp->timers[idx].tibc & 0x80000000) != 0)
-            mpp->timers[idx].ticc &= ~0x80000000;
-        mpp->timers[idx].tibc = val;
-        break;
-    case 0x20: /* GTIVPR */
-        write_IRQreg_ipvp(mpp, MPIC_TMR_IRQ + idx, val);
-        break;
-    case 0x30: /* GTIDR & TFRR */
-        if ((addr & 0xF0) == 0xF0)
-            mpp->dst[cpu].tfrr = val;
-        else
-            write_IRQreg_ide(mpp, MPIC_TMR_IRQ + idx, val);
-        break;
-    }
-}
-
-static uint64_t mpic_timer_read(void *opaque, hwaddr addr, unsigned len)
-{
-    openpic_t *mpp = opaque;
-    uint32_t retval;
-    int idx, cpu;
-
-    DPRINTF("%s: addr " TARGET_FMT_plx "\n", __func__, addr);
-    retval = 0xFFFFFFFF;
-    if (addr & 0xF)
-        return retval;
-    cpu = addr >> 12;
-    idx = (addr >> 6) & 0x3;
-    switch (addr & 0x30) {
-    case 0x00: /* gtccr */
-        retval = mpp->timers[idx].ticc;
-        break;
-    case 0x10: /* gtbcr */
-        retval = mpp->timers[idx].tibc;
-        break;
-    case 0x20: /* TIPV */
-        retval = read_IRQreg_ipvp(mpp, MPIC_TMR_IRQ + idx);
-        break;
-    case 0x30: /* TIDR */
-        if ((addr &0xF0) == 0XF0)
-            retval = mpp->dst[cpu].tfrr;
-        else
-            retval = read_IRQreg_ide(mpp, MPIC_TMR_IRQ + idx);
-        break;
-    }
-    DPRINTF("%s: => %08x\n", __func__, retval);
-
-    return retval;
-}
-
 static const MemoryRegionOps mpic_glb_ops = {
     .write = openpic_gbl_write,
     .read  = openpic_gbl_read,
@@ -1192,8 +1123,8 @@ static const MemoryRegionOps mpic_glb_ops = {
 };
 
 static const MemoryRegionOps mpic_tmr_ops = {
-    .write = mpic_timer_write,
-    .read  = mpic_timer_read,
+    .write = openpic_timer_write,
+    .read  = openpic_timer_read,
     .endianness = DEVICE_BIG_ENDIAN,
     .impl = {
         .min_access_size = 4,
-- 
1.6.0.2

^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [Qemu-devel] [PATCH 08/19] openpic: combine openpic and mpic reset functions
  2012-12-08 13:44 [Qemu-devel] [PATCH 00/19] OpenPIC refactoring and MSI support Alexander Graf
                   ` (6 preceding siblings ...)
  2012-12-08 13:44 ` [Qemu-devel] [PATCH 07/19] openpic: merge mpic and openpic timer handling Alexander Graf
@ 2012-12-08 13:44 ` Alexander Graf
  2012-12-08 13:44 ` [Qemu-devel] [PATCH 09/19] openpic: unify memory api subregions Alexander Graf
                   ` (10 subsequent siblings)
  18 siblings, 0 replies; 43+ messages in thread
From: Alexander Graf @ 2012-12-08 13:44 UTC (permalink / raw)
  To: qemu-ppc@nongnu.org List; +Cc: qemu-devel qemu-devel

The openpic and mpic reset handlers are almost identical. Combine
them and extract the differences into state variables.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 hw/openpic.c |  103 +++++++++++++++++++++++----------------------------------
 1 files changed, 42 insertions(+), 61 deletions(-)

diff --git a/hw/openpic.c b/hw/openpic.c
index b413bba..6a852ed 100644
--- a/hw/openpic.c
+++ b/hw/openpic.c
@@ -52,7 +52,6 @@
 #define VECTOR_BITS 8
 #define MAX_IPI     4
 #define VID         0x03 /* MPIC version ID */
-#define VENI        0x00000000 /* Vendor ID */
 
 enum {
     IRQ_IPVP = 0,
@@ -124,6 +123,14 @@ enum {
 #define FSL_BRR1_IPMJ (0x00 << 8) /* 8 bit IP major number */
 #define FSL_BRR1_IPMN 0x00 /* 8 bit IP minor number */
 
+#define FREP_NIRQ_SHIFT   16
+#define FREP_NCPU_SHIFT    8
+#define FREP_VID_SHIFT     0
+
+#define VID_REVISION_1_2   2
+
+#define VENI_GENERIC      0x00000000 /* Generic Vendor ID */
+
 enum mpic_ide_bits {
     IDR_EP     = 31,
     IDR_CI0     = 30,
@@ -207,6 +214,13 @@ typedef struct openpic_t {
 
     /* Behavior control */
     uint32_t flags;
+    uint32_t nb_irqs;
+    uint32_t vid;
+    uint32_t veni; /* Vendor identification register */
+    uint32_t spve_mask;
+    uint32_t tifr_reset;
+    uint32_t ipvp_reset;
+    uint32_t ide_reset;
 
     /* Sub-regions */
     MemoryRegion sub_io_mem[7];
@@ -214,8 +228,6 @@ typedef struct openpic_t {
     /* Global registers */
     uint32_t frep; /* Feature reporting register */
     uint32_t glbc; /* Global configuration register  */
-    uint32_t micr; /* MPIC interrupt configuration register */
-    uint32_t veni; /* Vendor identification register */
     uint32_t pint; /* Processor initialization register */
     uint32_t spve; /* Spurious vector register */
     uint32_t tifr; /* Timer frequency reporting register */
@@ -234,7 +246,6 @@ typedef struct openpic_t {
     int max_irq;
     int irq_ipi0;
     int irq_tim0;
-    void (*reset) (void *);
 } openpic_t;
 
 static void openpic_irq_raise(openpic_t *opp, int n_CPU, IRQ_src_t *src);
@@ -411,17 +422,17 @@ static void openpic_reset (void *opaque)
 
     opp->glbc = 0x80000000;
     /* Initialise controller registers */
-    opp->frep = ((OPENPIC_EXT_IRQ - 1) << 16) | ((MAX_CPU - 1) << 8) | VID;
-    opp->veni = VENI;
+    opp->frep = ((opp->nb_irqs -1) << FREP_NIRQ_SHIFT) |
+                ((opp->nb_cpus -1) << FREP_NCPU_SHIFT) |
+                (opp->vid << FREP_VID_SHIFT);
+
     opp->pint = 0x00000000;
-    opp->spve = 0x000000FF;
-    opp->tifr = 0x003F7A00;
-    /* ? */
-    opp->micr = 0x00000000;
+    opp->spve = -1 & opp->spve_mask;
+    opp->tifr = opp->tifr_reset;
     /* Initialise IRQ sources */
     for (i = 0; i < opp->max_irq; i++) {
-        opp->src[i].ipvp = 0xA0000000;
-        opp->src[i].ide  = 0x00000000;
+        opp->src[i].ipvp = opp->ipvp_reset;
+        opp->src[i].ide  = opp->ide_reset;
     }
     /* Initialise IRQ destinations */
     for (i = 0; i < MAX_CPU; i++) {
@@ -498,9 +509,9 @@ static void openpic_gbl_write(void *opaque, hwaddr addr, uint64_t val,
     case 0x1000: /* FREP */
         break;
     case 0x1020: /* GLBC */
-        if (val & 0x80000000 && opp->reset)
-            opp->reset(opp);
-        opp->glbc = val & ~0x80000000;
+        if (val & 0x80000000) {
+            openpic_reset(opp);
+        }
         break;
     case 0x1080: /* VENI */
         break;
@@ -529,7 +540,7 @@ static void openpic_gbl_write(void *opaque, hwaddr addr, uint64_t val,
         }
         break;
     case 0x10E0: /* SPVE */
-        opp->spve = val & 0x000000FF;
+        opp->spve = val & opp->spve_mask;
         break;
     default:
         break;
@@ -911,9 +922,7 @@ static void openpic_save(QEMUFile* f, void *opaque)
     openpic_t *opp = (openpic_t *)opaque;
     unsigned int i;
 
-    qemu_put_be32s(f, &opp->frep);
     qemu_put_be32s(f, &opp->glbc);
-    qemu_put_be32s(f, &opp->micr);
     qemu_put_be32s(f, &opp->veni);
     qemu_put_be32s(f, &opp->pint);
     qemu_put_be32s(f, &opp->spve);
@@ -963,9 +972,7 @@ static int openpic_load(QEMUFile* f, void *opaque, int version_id)
     if (version_id != 1)
         return -EINVAL;
 
-    qemu_get_be32s(f, &opp->frep);
     qemu_get_be32s(f, &opp->glbc);
-    qemu_get_be32s(f, &opp->micr);
     qemu_get_be32s(f, &opp->veni);
     qemu_get_be32s(f, &opp->pint);
     qemu_get_be32s(f, &opp->spve);
@@ -1042,6 +1049,11 @@ qemu_irq *openpic_init (MemoryRegion **pmem, int nb_cpus,
 
     //    isu_base &= 0xFFFC0000;
     opp->nb_cpus = nb_cpus;
+    opp->nb_irqs = OPENPIC_EXT_IRQ;
+    opp->vid = VID;
+    opp->veni = VENI_GENERIC;
+    opp->spve_mask = 0xFF;
+    opp->tifr_reset = 0x003F7A00;
     opp->max_irq = OPENPIC_MAX_IRQ;
     opp->irq_ipi0 = OPENPIC_IRQ_IPI0;
     opp->irq_tim0 = OPENPIC_IRQ_TIM0;
@@ -1067,51 +1079,12 @@ qemu_irq *openpic_init (MemoryRegion **pmem, int nb_cpus,
                     openpic_save, openpic_load, opp);
     qemu_register_reset(openpic_reset, opp);
 
-    opp->reset = openpic_reset;
-
     if (pmem)
         *pmem = &opp->mem;
 
     return qemu_allocate_irqs(openpic_set_irq, opp, opp->max_irq);
 }
 
-static void mpic_reset (void *opaque)
-{
-    openpic_t *mpp = (openpic_t *)opaque;
-    int i;
-
-    mpp->glbc = 0x80000000;
-    /* Initialise controller registers */
-    mpp->frep = 0x004f0002 | ((mpp->nb_cpus - 1) << 8);
-    mpp->veni = VENI;
-    mpp->pint = 0x00000000;
-    mpp->spve = 0x0000FFFF;
-    /* Initialise IRQ sources */
-    for (i = 0; i < mpp->max_irq; i++) {
-        mpp->src[i].ipvp = 0x80800000;
-        mpp->src[i].ide  = 0x00000001;
-    }
-    /* Set IDE for IPIs to 0 so we don't get spurious interrupts */
-    for (i = mpp->irq_ipi0; i < (mpp->irq_ipi0 + MAX_IPI); i++) {
-        mpp->src[i].ide = 0;
-    }
-    /* Initialise IRQ destinations */
-    for (i = 0; i < MAX_CPU; i++) {
-        mpp->dst[i].pctp      = 0x0000000F;
-        memset(&mpp->dst[i].raised, 0, sizeof(IRQ_queue_t));
-        mpp->dst[i].raised.next = -1;
-        memset(&mpp->dst[i].servicing, 0, sizeof(IRQ_queue_t));
-        mpp->dst[i].servicing.next = -1;
-    }
-    /* Initialise timers */
-    for (i = 0; i < MAX_TMR; i++) {
-        mpp->timers[i].ticc = 0x00000000;
-        mpp->timers[i].tibc = 0x80000000;
-    }
-    /* Go out of RESET state */
-    mpp->glbc = 0x00000000;
-}
-
 static const MemoryRegionOps mpic_glb_ops = {
     .write = openpic_gbl_write,
     .read  = openpic_gbl_read,
@@ -1184,6 +1157,15 @@ qemu_irq *mpic_init (MemoryRegion *address_space, hwaddr base,
     }
 
     mpp->nb_cpus = nb_cpus;
+    /* 12 external sources, 48 internal sources , 4 timer sources,
+       4 IPI sources, 4 messaging sources, and 8 Shared MSI sources */
+    mpp->nb_irqs = 80;
+    mpp->vid = VID_REVISION_1_2;
+    mpp->veni = VENI_GENERIC;
+    mpp->spve_mask = 0xFFFF;
+    mpp->tifr_reset = 0x00000000;
+    mpp->ipvp_reset = 0x80000000;
+    mpp->ide_reset = 0x00000001;
     mpp->max_irq = MPIC_MAX_IRQ;
     mpp->irq_ipi0 = MPIC_IPI_IRQ;
     mpp->irq_tim0 = MPIC_TMR_IRQ;
@@ -1194,10 +1176,9 @@ qemu_irq *mpic_init (MemoryRegion *address_space, hwaddr base,
 
     /* Enable critical interrupt support */
     mpp->flags |= OPENPIC_FLAG_IDE_CRIT;
-    mpp->reset = mpic_reset;
 
     register_savevm(NULL, "mpic", 0, 2, openpic_save, openpic_load, mpp);
-    qemu_register_reset(mpic_reset, mpp);
+    qemu_register_reset(openpic_reset, mpp);
 
     return qemu_allocate_irqs(openpic_set_irq, mpp, mpp->max_irq);
 }
-- 
1.6.0.2

^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [Qemu-devel] [PATCH 09/19] openpic: unify memory api subregions
  2012-12-08 13:44 [Qemu-devel] [PATCH 00/19] OpenPIC refactoring and MSI support Alexander Graf
                   ` (7 preceding siblings ...)
  2012-12-08 13:44 ` [Qemu-devel] [PATCH 08/19] openpic: combine openpic and mpic reset functions Alexander Graf
@ 2012-12-08 13:44 ` Alexander Graf
  2012-12-08 13:44 ` [Qemu-devel] [PATCH 10/19] openpic: remove unused type variable Alexander Graf
                   ` (9 subsequent siblings)
  18 siblings, 0 replies; 43+ messages in thread
From: Alexander Graf @ 2012-12-08 13:44 UTC (permalink / raw)
  To: qemu-ppc@nongnu.org List; +Cc: qemu-devel qemu-devel

The only difference between the "openpic" and "mpic" memory api subregion
descriptors is the endianness. Unify them as openpic accessors with explicit
endianness markers in their names.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 hw/openpic.c |  108 ++++++++++++++++++++++++++++++----------------------------
 1 files changed, 56 insertions(+), 52 deletions(-)

diff --git a/hw/openpic.c b/hw/openpic.c
index 6a852ed..e4ef23d 100644
--- a/hw/openpic.c
+++ b/hw/openpic.c
@@ -866,7 +866,7 @@ static uint64_t openpic_cpu_read(void *opaque, hwaddr addr, unsigned len)
     return openpic_cpu_read_internal(opaque, addr, (addr & 0x1f000) >> 12);
 }
 
-static const MemoryRegionOps openpic_glb_ops = {
+static const MemoryRegionOps openpic_glb_ops_le = {
     .write = openpic_gbl_write,
     .read  = openpic_gbl_read,
     .endianness = DEVICE_LITTLE_ENDIAN,
@@ -876,7 +876,17 @@ static const MemoryRegionOps openpic_glb_ops = {
     },
 };
 
-static const MemoryRegionOps openpic_tmr_ops = {
+static const MemoryRegionOps openpic_glb_ops_be = {
+    .write = openpic_gbl_write,
+    .read  = openpic_gbl_read,
+    .endianness = DEVICE_BIG_ENDIAN,
+    .impl = {
+        .min_access_size = 4,
+        .max_access_size = 4,
+    },
+};
+
+static const MemoryRegionOps openpic_tmr_ops_le = {
     .write = openpic_timer_write,
     .read  = openpic_timer_read,
     .endianness = DEVICE_LITTLE_ENDIAN,
@@ -886,7 +896,17 @@ static const MemoryRegionOps openpic_tmr_ops = {
     },
 };
 
-static const MemoryRegionOps openpic_cpu_ops = {
+static const MemoryRegionOps openpic_tmr_ops_be = {
+    .write = openpic_timer_write,
+    .read  = openpic_timer_read,
+    .endianness = DEVICE_BIG_ENDIAN,
+    .impl = {
+        .min_access_size = 4,
+        .max_access_size = 4,
+    },
+};
+
+static const MemoryRegionOps openpic_cpu_ops_le = {
     .write = openpic_cpu_write,
     .read  = openpic_cpu_read,
     .endianness = DEVICE_LITTLE_ENDIAN,
@@ -896,7 +916,17 @@ static const MemoryRegionOps openpic_cpu_ops = {
     },
 };
 
-static const MemoryRegionOps openpic_src_ops = {
+static const MemoryRegionOps openpic_cpu_ops_be = {
+    .write = openpic_cpu_write,
+    .read  = openpic_cpu_read,
+    .endianness = DEVICE_BIG_ENDIAN,
+    .impl = {
+        .min_access_size = 4,
+        .max_access_size = 4,
+    },
+};
+
+static const MemoryRegionOps openpic_src_ops_le = {
     .write = openpic_src_write,
     .read  = openpic_src_read,
     .endianness = DEVICE_LITTLE_ENDIAN,
@@ -906,6 +936,16 @@ static const MemoryRegionOps openpic_src_ops = {
     },
 };
 
+static const MemoryRegionOps openpic_src_ops_be = {
+    .write = openpic_src_write,
+    .read  = openpic_src_read,
+    .endianness = DEVICE_BIG_ENDIAN,
+    .impl = {
+        .min_access_size = 4,
+        .max_access_size = 4,
+    },
+};
+
 static void openpic_save_IRQ_queue(QEMUFile* f, IRQ_queue_t *q)
 {
     unsigned int i;
@@ -1025,10 +1065,14 @@ qemu_irq *openpic_init (MemoryRegion **pmem, int nb_cpus,
         hwaddr      start_addr;
         ram_addr_t              size;
     } const list[] = {
-        {"glb", &openpic_glb_ops, OPENPIC_GLB_REG_START, OPENPIC_GLB_REG_SIZE},
-        {"tmr", &openpic_tmr_ops, OPENPIC_TMR_REG_START, OPENPIC_TMR_REG_SIZE},
-        {"src", &openpic_src_ops, OPENPIC_SRC_REG_START, OPENPIC_SRC_REG_SIZE},
-        {"cpu", &openpic_cpu_ops, OPENPIC_CPU_REG_START, OPENPIC_CPU_REG_SIZE},
+        {"glb", &openpic_glb_ops_le, OPENPIC_GLB_REG_START,
+                                     OPENPIC_GLB_REG_SIZE},
+        {"tmr", &openpic_tmr_ops_le, OPENPIC_TMR_REG_START,
+                                     OPENPIC_TMR_REG_SIZE},
+        {"src", &openpic_src_ops_le, OPENPIC_SRC_REG_START,
+                                     OPENPIC_SRC_REG_SIZE},
+        {"cpu", &openpic_cpu_ops_le, OPENPIC_CPU_REG_START,
+                                     OPENPIC_CPU_REG_SIZE},
     };
 
     /* XXX: for now, only one CPU is supported */
@@ -1085,46 +1129,6 @@ qemu_irq *openpic_init (MemoryRegion **pmem, int nb_cpus,
     return qemu_allocate_irqs(openpic_set_irq, opp, opp->max_irq);
 }
 
-static const MemoryRegionOps mpic_glb_ops = {
-    .write = openpic_gbl_write,
-    .read  = openpic_gbl_read,
-    .endianness = DEVICE_BIG_ENDIAN,
-    .impl = {
-        .min_access_size = 4,
-        .max_access_size = 4,
-    },
-};
-
-static const MemoryRegionOps mpic_tmr_ops = {
-    .write = openpic_timer_write,
-    .read  = openpic_timer_read,
-    .endianness = DEVICE_BIG_ENDIAN,
-    .impl = {
-        .min_access_size = 4,
-        .max_access_size = 4,
-    },
-};
-
-static const MemoryRegionOps mpic_cpu_ops = {
-    .write = openpic_cpu_write,
-    .read  = openpic_cpu_read,
-    .endianness = DEVICE_BIG_ENDIAN,
-    .impl = {
-        .min_access_size = 4,
-        .max_access_size = 4,
-    },
-};
-
-static const MemoryRegionOps mpic_irq_ops = {
-    .write = openpic_src_write,
-    .read  = openpic_src_read,
-    .endianness = DEVICE_BIG_ENDIAN,
-    .impl = {
-        .min_access_size = 4,
-        .max_access_size = 4,
-    },
-};
-
 qemu_irq *mpic_init (MemoryRegion *address_space, hwaddr base,
                      int nb_cpus, qemu_irq **irqs, qemu_irq irq_out)
 {
@@ -1136,10 +1140,10 @@ qemu_irq *mpic_init (MemoryRegion *address_space, hwaddr base,
         hwaddr      start_addr;
         ram_addr_t              size;
     } const list[] = {
-        {"glb", &mpic_glb_ops, MPIC_GLB_REG_START, MPIC_GLB_REG_SIZE},
-        {"tmr", &mpic_tmr_ops, MPIC_TMR_REG_START, MPIC_TMR_REG_SIZE},
-        {"src", &mpic_irq_ops, MPIC_SRC_REG_START, MPIC_SRC_REG_SIZE},
-        {"cpu", &mpic_cpu_ops, MPIC_CPU_REG_START, MPIC_CPU_REG_SIZE},
+        {"glb", &openpic_glb_ops_be, MPIC_GLB_REG_START, MPIC_GLB_REG_SIZE},
+        {"tmr", &openpic_tmr_ops_be, MPIC_TMR_REG_START, MPIC_TMR_REG_SIZE},
+        {"src", &openpic_src_ops_be, MPIC_SRC_REG_START, MPIC_SRC_REG_SIZE},
+        {"cpu", &openpic_cpu_ops_be, MPIC_CPU_REG_START, MPIC_CPU_REG_SIZE},
     };
 
     mpp = g_malloc0(sizeof(openpic_t));
-- 
1.6.0.2

^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [Qemu-devel] [PATCH 10/19] openpic: remove unused type variable
  2012-12-08 13:44 [Qemu-devel] [PATCH 00/19] OpenPIC refactoring and MSI support Alexander Graf
                   ` (8 preceding siblings ...)
  2012-12-08 13:44 ` [Qemu-devel] [PATCH 09/19] openpic: unify memory api subregions Alexander Graf
@ 2012-12-08 13:44 ` Alexander Graf
  2012-12-10 23:42   ` [Qemu-devel] [Qemu-ppc] " Scott Wood
  2012-12-08 13:44 ` [Qemu-devel] [PATCH 11/19] openpic: convert simple reg operations to builtin bitops Alexander Graf
                   ` (8 subsequent siblings)
  18 siblings, 1 reply; 43+ messages in thread
From: Alexander Graf @ 2012-12-08 13:44 UTC (permalink / raw)
  To: qemu-ppc@nongnu.org List; +Cc: qemu-devel qemu-devel

The openpic source irqs are carrying around a type indicator that
is never accessed by anything. Remove it.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 hw/openpic.c |   27 ++-------------------------
 1 files changed, 2 insertions(+), 25 deletions(-)

diff --git a/hw/openpic.c b/hw/openpic.c
index e4ef23d..d252b2b 100644
--- a/hw/openpic.c
+++ b/hw/openpic.c
@@ -167,13 +167,6 @@ static uint32_t openpic_cpu_read_internal(void *opaque, hwaddr addr,
 static void openpic_cpu_write_internal(void *opaque, hwaddr addr,
                                        uint32_t val, int idx);
 
-enum {
-    IRQ_EXTERNAL = 0x01,
-    IRQ_INTERNAL = 0x02,
-    IRQ_TIMER    = 0x04,
-    IRQ_SPECIAL  = 0x08,
-};
-
 typedef struct IRQ_queue_t {
     uint32_t queue[BF_WIDTH(MAX_IRQ)];
     int next;
@@ -183,7 +176,6 @@ typedef struct IRQ_queue_t {
 typedef struct IRQ_src_t {
     uint32_t ipvp;  /* IRQ vector/priority register */
     uint32_t ide;   /* IRQ destination register */
-    int type;
     int last_cpu;
     int pending;    /* TRUE if IRQ is pending */
 } IRQ_src_t;
@@ -971,7 +963,6 @@ static void openpic_save(QEMUFile* f, void *opaque)
     for (i = 0; i < opp->max_irq; i++) {
         qemu_put_be32s(f, &opp->src[i].ipvp);
         qemu_put_be32s(f, &opp->src[i].ide);
-        qemu_put_sbe32s(f, &opp->src[i].type);
         qemu_put_sbe32s(f, &opp->src[i].last_cpu);
         qemu_put_sbe32s(f, &opp->src[i].pending);
     }
@@ -1021,7 +1012,6 @@ static int openpic_load(QEMUFile* f, void *opaque, int version_id)
     for (i = 0; i < opp->max_irq; i++) {
         qemu_get_be32s(f, &opp->src[i].ipvp);
         qemu_get_be32s(f, &opp->src[i].ide);
-        qemu_get_sbe32s(f, &opp->src[i].type);
         qemu_get_sbe32s(f, &opp->src[i].last_cpu);
         qemu_get_sbe32s(f, &opp->src[i].pending);
     }
@@ -1058,7 +1048,7 @@ qemu_irq *openpic_init (MemoryRegion **pmem, int nb_cpus,
                         qemu_irq **irqs, qemu_irq irq_out)
 {
     openpic_t *opp;
-    int i, m;
+    int i;
     struct {
         const char             *name;
         MemoryRegionOps const  *ops;
@@ -1101,20 +1091,7 @@ qemu_irq *openpic_init (MemoryRegion **pmem, int nb_cpus,
     opp->max_irq = OPENPIC_MAX_IRQ;
     opp->irq_ipi0 = OPENPIC_IRQ_IPI0;
     opp->irq_tim0 = OPENPIC_IRQ_TIM0;
-    /* Set IRQ types */
-    for (i = 0; i < OPENPIC_EXT_IRQ; i++) {
-        opp->src[i].type = IRQ_EXTERNAL;
-    }
-    for (; i < OPENPIC_IRQ_TIM0; i++) {
-        opp->src[i].type = IRQ_SPECIAL;
-    }
-    m = OPENPIC_IRQ_IPI0;
-    for (; i < m; i++) {
-        opp->src[i].type = IRQ_TIMER;
-    }
-    for (; i < OPENPIC_MAX_IRQ; i++) {
-        opp->src[i].type = IRQ_INTERNAL;
-    }
+
     for (i = 0; i < nb_cpus; i++)
         opp->dst[i].irqs = irqs[i];
     opp->irq_out = irq_out;
-- 
1.6.0.2

^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [Qemu-devel] [PATCH 11/19] openpic: convert simple reg operations to builtin bitops
  2012-12-08 13:44 [Qemu-devel] [PATCH 00/19] OpenPIC refactoring and MSI support Alexander Graf
                   ` (9 preceding siblings ...)
  2012-12-08 13:44 ` [Qemu-devel] [PATCH 10/19] openpic: remove unused type variable Alexander Graf
@ 2012-12-08 13:44 ` Alexander Graf
  2012-12-08 13:44 ` [Qemu-devel] [PATCH 12/19] openpic: rename openpic_t to OpenPICState Alexander Graf
                   ` (7 subsequent siblings)
  18 siblings, 0 replies; 43+ messages in thread
From: Alexander Graf @ 2012-12-08 13:44 UTC (permalink / raw)
  To: qemu-ppc@nongnu.org List; +Cc: qemu-devel qemu-devel

The openpic code has its own bitmap code to access bits inside of a
bitmap. However, that is overkill when we simply want to check for a
bit inside of a uint32_t.

So instead, let's use normal bit masks and C builtin shifts and ands.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 hw/openpic.c |   67 +++++++++++++++++++++++++++++++--------------------------
 1 files changed, 36 insertions(+), 31 deletions(-)

diff --git a/hw/openpic.c b/hw/openpic.c
index d252b2b..bf6135d 100644
--- a/hw/openpic.c
+++ b/hw/openpic.c
@@ -131,13 +131,12 @@ enum {
 
 #define VENI_GENERIC      0x00000000 /* Generic Vendor ID */
 
-enum mpic_ide_bits {
-    IDR_EP     = 31,
-    IDR_CI0     = 30,
-    IDR_CI1     = 29,
-    IDR_P1     = 1,
-    IDR_P0     = 0,
-};
+#define IDR_EP_SHIFT      31
+#define IDR_EP_MASK       (1 << IDR_EP_SHIFT)
+#define IDR_CI0_SHIFT     30
+#define IDR_CI1_SHIFT     29
+#define IDR_P1_SHIFT      1
+#define IDR_P0_SHIFT      0
 
 #define BF_WIDTH(_bits_) \
 (((_bits_) + (sizeof(uint32_t) * 8) - 1) / (sizeof(uint32_t) * 8))
@@ -180,13 +179,17 @@ typedef struct IRQ_src_t {
     int pending;    /* TRUE if IRQ is pending */
 } IRQ_src_t;
 
-enum IPVP_bits {
-    IPVP_MASK     = 31,
-    IPVP_ACTIVITY = 30,
-    IPVP_MODE     = 29,
-    IPVP_POLARITY = 23,
-    IPVP_SENSE    = 22,
-};
+#define IPVP_MASK_SHIFT       31
+#define IPVP_MASK_MASK        (1 << IPVP_MASK_SHIFT)
+#define IPVP_ACTIVITY_SHIFT   30
+#define IPVP_ACTIVITY_MASK    (1 << IPVP_ACTIVITY_SHIFT)
+#define IPVP_MODE_SHIFT       29
+#define IPVP_MODE_MASK        (1 << IPVP_MODE_SHIFT)
+#define IPVP_POLARITY_SHIFT   23
+#define IPVP_POLARITY_MASK    (1 << IPVP_POLARITY_SHIFT)
+#define IPVP_SENSE_SHIFT      22
+#define IPVP_SENSE_MASK       (1 << IPVP_SENSE_SHIFT)
+
 #define IPVP_PRIORITY_MASK     (0x1F << 16)
 #define IPVP_PRIORITY(_ipvpr_) ((int)(((_ipvpr_) & IPVP_PRIORITY_MASK) >> 16))
 #define IPVP_VECTOR_MASK       ((1 << VECTOR_BITS) - 1)
@@ -309,7 +312,7 @@ static void IRQ_local_pipe (openpic_t *opp, int n_CPU, int n_IRQ)
                 __func__, n_IRQ, n_CPU);
         return;
     }
-    set_bit(&src->ipvp, IPVP_ACTIVITY);
+    src->ipvp |= IPVP_ACTIVITY_MASK;
     IRQ_setbit(&dst->raised, n_IRQ);
     if (priority < dst->raised.priority) {
         /* An higher priority IRQ is already raised */
@@ -342,7 +345,7 @@ static void openpic_update_irq(openpic_t *opp, int n_IRQ)
         DPRINTF("%s: IRQ %d is not pending\n", __func__, n_IRQ);
         return;
     }
-    if (test_bit(&src->ipvp, IPVP_MASK)) {
+    if (src->ipvp & IPVP_MASK_MASK) {
         /* Interrupt source is disabled */
         DPRINTF("%s: IRQ %d is disabled\n", __func__, n_IRQ);
         return;
@@ -352,7 +355,7 @@ static void openpic_update_irq(openpic_t *opp, int n_IRQ)
         DPRINTF("%s: IRQ %d has 0 priority\n", __func__, n_IRQ);
         return;
     }
-    if (test_bit(&src->ipvp, IPVP_ACTIVITY)) {
+    if (src->ipvp & IPVP_ACTIVITY_MASK) {
         /* IRQ already active */
         DPRINTF("%s: IRQ %d is already active\n", __func__, n_IRQ);
         return;
@@ -366,18 +369,19 @@ static void openpic_update_irq(openpic_t *opp, int n_IRQ)
     if (src->ide == (1 << src->last_cpu)) {
         /* Only one CPU is allowed to receive this IRQ */
         IRQ_local_pipe(opp, src->last_cpu, n_IRQ);
-    } else if (!test_bit(&src->ipvp, IPVP_MODE)) {
+    } else if (!(src->ipvp & IPVP_MODE_MASK)) {
         /* Directed delivery mode */
         for (i = 0; i < opp->nb_cpus; i++) {
-            if (test_bit(&src->ide, i))
+            if (src->ide & (1 << i)) {
                 IRQ_local_pipe(opp, i, n_IRQ);
+            }
         }
     } else {
         /* Distributed delivery mode */
         for (i = src->last_cpu + 1; i != src->last_cpu; i++) {
             if (i == opp->nb_cpus)
                 i = 0;
-            if (test_bit(&src->ide, i)) {
+            if (src->ide & (1 << i)) {
                 IRQ_local_pipe(opp, i, n_IRQ);
                 src->last_cpu = i;
                 break;
@@ -394,11 +398,12 @@ static void openpic_set_irq(void *opaque, int n_IRQ, int level)
     src = &opp->src[n_IRQ];
     DPRINTF("openpic: set irq %d = %d ipvp=%08x\n",
             n_IRQ, level, src->ipvp);
-    if (test_bit(&src->ipvp, IPVP_SENSE)) {
+    if (src->ipvp & IPVP_SENSE_MASK) {
         /* level-sensitive irq */
         src->pending = level;
-        if (!level)
-            reset_bit(&src->ipvp, IPVP_ACTIVITY);
+        if (!level) {
+            src->ipvp &= ~IPVP_ACTIVITY_MASK;
+        }
     } else {
         /* edge-sensitive irq */
         if (level)
@@ -809,13 +814,13 @@ static uint32_t openpic_cpu_read_internal(void *opaque, hwaddr addr,
             retval = IPVP_VECTOR(opp->spve);
         } else {
             src = &opp->src[n_IRQ];
-            if (!test_bit(&src->ipvp, IPVP_ACTIVITY) ||
+            if (!(src->ipvp & IPVP_ACTIVITY_MASK) ||
                 !(IPVP_PRIORITY(src->ipvp) > dst->pctp)) {
                 /* - Spurious level-sensitive IRQ
                  * - Priorities has been changed
                  *   and the pending IRQ isn't allowed anymore
                  */
-                reset_bit(&src->ipvp, IPVP_ACTIVITY);
+                src->ipvp &= ~IPVP_ACTIVITY_MASK;
                 retval = IPVP_VECTOR(opp->spve);
             } else {
                 /* IRQ enter servicing state */
@@ -824,20 +829,20 @@ static uint32_t openpic_cpu_read_internal(void *opaque, hwaddr addr,
             }
             IRQ_resetbit(&dst->raised, n_IRQ);
             dst->raised.next = -1;
-            if (!test_bit(&src->ipvp, IPVP_SENSE)) {
+            if (!(src->ipvp & IPVP_SENSE_MASK)) {
                 /* edge-sensitive IRQ */
-                reset_bit(&src->ipvp, IPVP_ACTIVITY);
+                src->ipvp &= ~IPVP_ACTIVITY_MASK;
                 src->pending = 0;
             }
 
             if ((n_IRQ >= opp->irq_ipi0) &&  (n_IRQ < (opp->irq_ipi0 + MAX_IPI))) {
                 src->ide &= ~(1 << idx);
-                if (src->ide && !test_bit(&src->ipvp, IPVP_SENSE)) {
+                if (src->ide && !(src->ipvp & IPVP_SENSE_MASK)) {
                     /* trigger on CPUs that didn't know about it yet */
                     openpic_set_irq(opp, n_IRQ, 1);
                     openpic_set_irq(opp, n_IRQ, 0);
                     /* if all CPUs knew about it, set active bit again */
-                    set_bit(&src->ipvp, IPVP_ACTIVITY);
+                    src->ipvp |= IPVP_ACTIVITY_MASK;
                 }
             }
         }
@@ -1035,9 +1040,9 @@ static int openpic_load(QEMUFile* f, void *opaque, int version_id)
 
 static void openpic_irq_raise(openpic_t *opp, int n_CPU, IRQ_src_t *src)
 {
-    int n_ci = IDR_CI0 - n_CPU;
+    int n_ci = IDR_CI0_SHIFT - n_CPU;
 
-    if ((opp->flags & OPENPIC_FLAG_IDE_CRIT) && test_bit(&src->ide, n_ci)) {
+    if ((opp->flags & OPENPIC_FLAG_IDE_CRIT) && (src->ide & (1 << n_ci))) {
         qemu_irq_raise(opp->dst[n_CPU].irqs[OPENPIC_OUTPUT_CINT]);
     } else {
         qemu_irq_raise(opp->dst[n_CPU].irqs[OPENPIC_OUTPUT_INT]);
-- 
1.6.0.2

^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [Qemu-devel] [PATCH 12/19] openpic: rename openpic_t to OpenPICState
  2012-12-08 13:44 [Qemu-devel] [PATCH 00/19] OpenPIC refactoring and MSI support Alexander Graf
                   ` (10 preceding siblings ...)
  2012-12-08 13:44 ` [Qemu-devel] [PATCH 11/19] openpic: convert simple reg operations to builtin bitops Alexander Graf
@ 2012-12-08 13:44 ` Alexander Graf
  2012-12-08 13:44 ` [Qemu-devel] [PATCH 13/19] openpic: remove irq_out Alexander Graf
                   ` (6 subsequent siblings)
  18 siblings, 0 replies; 43+ messages in thread
From: Alexander Graf @ 2012-12-08 13:44 UTC (permalink / raw)
  To: qemu-ppc@nongnu.org List; +Cc: qemu-devel qemu-devel

Rename the openpic_t struct to OpenPICState, so it adheres better to
the current coding style rules.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 hw/openpic.c |   68 +++++++++++++++++++++++++++++-----------------------------
 1 files changed, 34 insertions(+), 34 deletions(-)

diff --git a/hw/openpic.c b/hw/openpic.c
index bf6135d..f3e53b6 100644
--- a/hw/openpic.c
+++ b/hw/openpic.c
@@ -203,7 +203,7 @@ typedef struct IRQ_dst_t {
     qemu_irq *irqs;
 } IRQ_dst_t;
 
-typedef struct openpic_t {
+typedef struct OpenPICState {
     PCIDevice pci_dev;
     MemoryRegion mem;
 
@@ -241,9 +241,9 @@ typedef struct openpic_t {
     int max_irq;
     int irq_ipi0;
     int irq_tim0;
-} openpic_t;
+} OpenPICState;
 
-static void openpic_irq_raise(openpic_t *opp, int n_CPU, IRQ_src_t *src);
+static void openpic_irq_raise(OpenPICState *opp, int n_CPU, IRQ_src_t *src);
 
 static inline void IRQ_setbit (IRQ_queue_t *q, int n_IRQ)
 {
@@ -260,7 +260,7 @@ static inline int IRQ_testbit (IRQ_queue_t *q, int n_IRQ)
     return test_bit(q->queue, n_IRQ);
 }
 
-static void IRQ_check (openpic_t *opp, IRQ_queue_t *q)
+static void IRQ_check (OpenPICState *opp, IRQ_queue_t *q)
 {
     int next, i;
     int priority;
@@ -281,7 +281,7 @@ static void IRQ_check (openpic_t *opp, IRQ_queue_t *q)
     q->priority = priority;
 }
 
-static int IRQ_get_next (openpic_t *opp, IRQ_queue_t *q)
+static int IRQ_get_next (OpenPICState *opp, IRQ_queue_t *q)
 {
     if (q->next == -1) {
         /* XXX: optimize */
@@ -291,7 +291,7 @@ static int IRQ_get_next (openpic_t *opp, IRQ_queue_t *q)
     return q->next;
 }
 
-static void IRQ_local_pipe (openpic_t *opp, int n_CPU, int n_IRQ)
+static void IRQ_local_pipe (OpenPICState *opp, int n_CPU, int n_IRQ)
 {
     IRQ_dst_t *dst;
     IRQ_src_t *src;
@@ -333,7 +333,7 @@ static void IRQ_local_pipe (openpic_t *opp, int n_CPU, int n_IRQ)
 }
 
 /* update pic state because registers for n_IRQ have changed value */
-static void openpic_update_irq(openpic_t *opp, int n_IRQ)
+static void openpic_update_irq(OpenPICState *opp, int n_IRQ)
 {
     IRQ_src_t *src;
     int i;
@@ -392,7 +392,7 @@ static void openpic_update_irq(openpic_t *opp, int n_IRQ)
 
 static void openpic_set_irq(void *opaque, int n_IRQ, int level)
 {
-    openpic_t *opp = opaque;
+    OpenPICState *opp = opaque;
     IRQ_src_t *src;
 
     src = &opp->src[n_IRQ];
@@ -414,7 +414,7 @@ static void openpic_set_irq(void *opaque, int n_IRQ, int level)
 
 static void openpic_reset (void *opaque)
 {
-    openpic_t *opp = (openpic_t *)opaque;
+    OpenPICState *opp = (OpenPICState *)opaque;
     int i;
 
     opp->glbc = 0x80000000;
@@ -449,17 +449,17 @@ static void openpic_reset (void *opaque)
     opp->glbc = 0x00000000;
 }
 
-static inline uint32_t read_IRQreg_ide(openpic_t *opp, int n_IRQ)
+static inline uint32_t read_IRQreg_ide(OpenPICState *opp, int n_IRQ)
 {
     return opp->src[n_IRQ].ide;
 }
 
-static inline uint32_t read_IRQreg_ipvp(openpic_t *opp, int n_IRQ)
+static inline uint32_t read_IRQreg_ipvp(OpenPICState *opp, int n_IRQ)
 {
     return opp->src[n_IRQ].ipvp;
 }
 
-static inline void write_IRQreg_ide(openpic_t *opp, int n_IRQ, uint32_t val)
+static inline void write_IRQreg_ide(OpenPICState *opp, int n_IRQ, uint32_t val)
 {
     uint32_t tmp;
 
@@ -469,7 +469,7 @@ static inline void write_IRQreg_ide(openpic_t *opp, int n_IRQ, uint32_t val)
     DPRINTF("Set IDE %d to 0x%08x\n", n_IRQ, opp->src[n_IRQ].ide);
 }
 
-static inline void write_IRQreg_ipvp(openpic_t *opp, int n_IRQ, uint32_t val)
+static inline void write_IRQreg_ipvp(OpenPICState *opp, int n_IRQ, uint32_t val)
 {
     /* NOTE: not fully accurate for special IRQs, but simple and sufficient */
     /* ACTIVITY bit is read-only */
@@ -483,7 +483,7 @@ static inline void write_IRQreg_ipvp(openpic_t *opp, int n_IRQ, uint32_t val)
 static void openpic_gbl_write(void *opaque, hwaddr addr, uint64_t val,
                               unsigned len)
 {
-    openpic_t *opp = opaque;
+    OpenPICState *opp = opaque;
     IRQ_dst_t *dst;
     int idx;
 
@@ -546,7 +546,7 @@ static void openpic_gbl_write(void *opaque, hwaddr addr, uint64_t val,
 
 static uint64_t openpic_gbl_read(void *opaque, hwaddr addr, unsigned len)
 {
-    openpic_t *opp = opaque;
+    OpenPICState *opp = opaque;
     uint32_t retval;
 
     DPRINTF("%s: addr " TARGET_FMT_plx "\n", __func__, addr);
@@ -598,10 +598,10 @@ static uint64_t openpic_gbl_read(void *opaque, hwaddr addr, unsigned len)
     return retval;
 }
 
-static void openpic_timer_write(void *opaque, hwaddr addr, uint64_t val,
+static void openpic_tmr_write(void *opaque, hwaddr addr, uint64_t val,
                                 unsigned len)
 {
-    openpic_t *opp = opaque;
+    OpenPICState *opp = opaque;
     int idx;
 
     DPRINTF("%s: addr %08x <= %08x\n", __func__, addr, val);
@@ -634,9 +634,9 @@ static void openpic_timer_write(void *opaque, hwaddr addr, uint64_t val,
     }
 }
 
-static uint64_t openpic_timer_read(void *opaque, hwaddr addr, unsigned len)
+static uint64_t openpic_tmr_read(void *opaque, hwaddr addr, unsigned len)
 {
-    openpic_t *opp = opaque;
+    OpenPICState *opp = opaque;
     uint32_t retval = -1;
     int idx;
 
@@ -674,7 +674,7 @@ out:
 static void openpic_src_write(void *opaque, hwaddr addr, uint64_t val,
                               unsigned len)
 {
-    openpic_t *opp = opaque;
+    OpenPICState *opp = opaque;
     int idx;
 
     DPRINTF("%s: addr %08x <= %08x\n", __func__, addr, val);
@@ -693,7 +693,7 @@ static void openpic_src_write(void *opaque, hwaddr addr, uint64_t val,
 
 static uint64_t openpic_src_read(void *opaque, uint64_t addr, unsigned len)
 {
-    openpic_t *opp = opaque;
+    OpenPICState *opp = opaque;
     uint32_t retval;
     int idx;
 
@@ -718,7 +718,7 @@ static uint64_t openpic_src_read(void *opaque, uint64_t addr, unsigned len)
 static void openpic_cpu_write_internal(void *opaque, hwaddr addr,
                                        uint32_t val, int idx)
 {
-    openpic_t *opp = opaque;
+    OpenPICState *opp = opaque;
     IRQ_src_t *src;
     IRQ_dst_t *dst;
     int s_IRQ, n_IRQ;
@@ -782,7 +782,7 @@ static void openpic_cpu_write(void *opaque, hwaddr addr, uint64_t val,
 static uint32_t openpic_cpu_read_internal(void *opaque, hwaddr addr,
                                           int idx)
 {
-    openpic_t *opp = opaque;
+    OpenPICState *opp = opaque;
     IRQ_src_t *src;
     IRQ_dst_t *dst;
     uint32_t retval;
@@ -884,8 +884,8 @@ static const MemoryRegionOps openpic_glb_ops_be = {
 };
 
 static const MemoryRegionOps openpic_tmr_ops_le = {
-    .write = openpic_timer_write,
-    .read  = openpic_timer_read,
+    .write = openpic_tmr_write,
+    .read  = openpic_tmr_read,
     .endianness = DEVICE_LITTLE_ENDIAN,
     .impl = {
         .min_access_size = 4,
@@ -894,8 +894,8 @@ static const MemoryRegionOps openpic_tmr_ops_le = {
 };
 
 static const MemoryRegionOps openpic_tmr_ops_be = {
-    .write = openpic_timer_write,
-    .read  = openpic_timer_read,
+    .write = openpic_tmr_write,
+    .read  = openpic_tmr_read,
     .endianness = DEVICE_BIG_ENDIAN,
     .impl = {
         .min_access_size = 4,
@@ -956,7 +956,7 @@ static void openpic_save_IRQ_queue(QEMUFile* f, IRQ_queue_t *q)
 
 static void openpic_save(QEMUFile* f, void *opaque)
 {
-    openpic_t *opp = (openpic_t *)opaque;
+    OpenPICState *opp = (OpenPICState *)opaque;
     unsigned int i;
 
     qemu_put_be32s(f, &opp->glbc);
@@ -1002,7 +1002,7 @@ static void openpic_load_IRQ_queue(QEMUFile* f, IRQ_queue_t *q)
 
 static int openpic_load(QEMUFile* f, void *opaque, int version_id)
 {
-    openpic_t *opp = (openpic_t *)opaque;
+    OpenPICState *opp = (OpenPICState *)opaque;
     unsigned int i;
 
     if (version_id != 1)
@@ -1038,7 +1038,7 @@ static int openpic_load(QEMUFile* f, void *opaque, int version_id)
     return pci_device_load(&opp->pci_dev, f);
 }
 
-static void openpic_irq_raise(openpic_t *opp, int n_CPU, IRQ_src_t *src)
+static void openpic_irq_raise(OpenPICState *opp, int n_CPU, IRQ_src_t *src)
 {
     int n_ci = IDR_CI0_SHIFT - n_CPU;
 
@@ -1052,7 +1052,7 @@ static void openpic_irq_raise(openpic_t *opp, int n_CPU, IRQ_src_t *src)
 qemu_irq *openpic_init (MemoryRegion **pmem, int nb_cpus,
                         qemu_irq **irqs, qemu_irq irq_out)
 {
-    openpic_t *opp;
+    OpenPICState *opp;
     int i;
     struct {
         const char             *name;
@@ -1073,7 +1073,7 @@ qemu_irq *openpic_init (MemoryRegion **pmem, int nb_cpus,
     /* XXX: for now, only one CPU is supported */
     if (nb_cpus != 1)
         return NULL;
-    opp = g_malloc0(sizeof(openpic_t));
+    opp = g_malloc0(sizeof(OpenPICState));
 
     memory_region_init(&opp->mem, "openpic", 0x40000);
 
@@ -1114,7 +1114,7 @@ qemu_irq *openpic_init (MemoryRegion **pmem, int nb_cpus,
 qemu_irq *mpic_init (MemoryRegion *address_space, hwaddr base,
                      int nb_cpus, qemu_irq **irqs, qemu_irq irq_out)
 {
-    openpic_t    *mpp;
+    OpenPICState    *mpp;
     int           i;
     struct {
         const char             *name;
@@ -1128,7 +1128,7 @@ qemu_irq *mpic_init (MemoryRegion *address_space, hwaddr base,
         {"cpu", &openpic_cpu_ops_be, MPIC_CPU_REG_START, MPIC_CPU_REG_SIZE},
     };
 
-    mpp = g_malloc0(sizeof(openpic_t));
+    mpp = g_malloc0(sizeof(OpenPICState));
 
     memory_region_init(&mpp->mem, "mpic", 0x40000);
     memory_region_add_subregion(address_space, base, &mpp->mem);
-- 
1.6.0.2

^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [Qemu-devel] [PATCH 13/19] openpic: remove irq_out
  2012-12-08 13:44 [Qemu-devel] [PATCH 00/19] OpenPIC refactoring and MSI support Alexander Graf
                   ` (11 preceding siblings ...)
  2012-12-08 13:44 ` [Qemu-devel] [PATCH 12/19] openpic: rename openpic_t to OpenPICState Alexander Graf
@ 2012-12-08 13:44 ` Alexander Graf
  2012-12-08 13:44 ` [Qemu-devel] [PATCH 14/19] openpic: convert to qdev Alexander Graf
                   ` (5 subsequent siblings)
  18 siblings, 0 replies; 43+ messages in thread
From: Alexander Graf @ 2012-12-08 13:44 UTC (permalink / raw)
  To: qemu-ppc@nongnu.org List; +Cc: qemu-devel qemu-devel

The current openpic emulation contains half-ready code for bypass mode.
Remove it, so that when someone wants to finish it they can start from a
clean state.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 hw/openpic.c      |    8 ++------
 hw/openpic.h      |    4 ++--
 hw/ppc/e500.c     |    2 +-
 hw/ppc_newworld.c |    2 +-
 4 files changed, 6 insertions(+), 10 deletions(-)

diff --git a/hw/openpic.c b/hw/openpic.c
index f3e53b6..321c6d4 100644
--- a/hw/openpic.c
+++ b/hw/openpic.c
@@ -236,8 +236,6 @@ typedef struct OpenPICState {
         uint32_t ticc;  /* Global timer current count register */
         uint32_t tibc;  /* Global timer base count register */
     } timers[MAX_TMR];
-    /* IRQ out is used when in bypass mode (not implemented) */
-    qemu_irq irq_out;
     int max_irq;
     int irq_ipi0;
     int irq_tim0;
@@ -1050,7 +1048,7 @@ static void openpic_irq_raise(OpenPICState *opp, int n_CPU, IRQ_src_t *src)
 }
 
 qemu_irq *openpic_init (MemoryRegion **pmem, int nb_cpus,
-                        qemu_irq **irqs, qemu_irq irq_out)
+                        qemu_irq **irqs)
 {
     OpenPICState *opp;
     int i;
@@ -1099,7 +1097,6 @@ qemu_irq *openpic_init (MemoryRegion **pmem, int nb_cpus,
 
     for (i = 0; i < nb_cpus; i++)
         opp->dst[i].irqs = irqs[i];
-    opp->irq_out = irq_out;
 
     register_savevm(&opp->pci_dev.qdev, "openpic", 0, 2,
                     openpic_save, openpic_load, opp);
@@ -1112,7 +1109,7 @@ qemu_irq *openpic_init (MemoryRegion **pmem, int nb_cpus,
 }
 
 qemu_irq *mpic_init (MemoryRegion *address_space, hwaddr base,
-                     int nb_cpus, qemu_irq **irqs, qemu_irq irq_out)
+                     int nb_cpus, qemu_irq **irqs)
 {
     OpenPICState    *mpp;
     int           i;
@@ -1158,7 +1155,6 @@ qemu_irq *mpic_init (MemoryRegion *address_space, hwaddr base,
 
     for (i = 0; i < nb_cpus; i++)
         mpp->dst[i].irqs = irqs[i];
-    mpp->irq_out = irq_out;
 
     /* Enable critical interrupt support */
     mpp->flags |= OPENPIC_FLAG_IDE_CRIT;
diff --git a/hw/openpic.h b/hw/openpic.h
index 1232d10..8a68f20 100644
--- a/hw/openpic.h
+++ b/hw/openpic.h
@@ -15,7 +15,7 @@ enum {
 #define OPENPIC_FLAG_IDE_CRIT    (1 << 0)
 
 qemu_irq *openpic_init (MemoryRegion **pmem, int nb_cpus,
-                        qemu_irq **irqs, qemu_irq irq_out);
+                        qemu_irq **irqs);
 qemu_irq *mpic_init (MemoryRegion *address_space, hwaddr base,
-                     int nb_cpus, qemu_irq **irqs, qemu_irq irq_out);
+                     int nb_cpus, qemu_irq **irqs);
 #endif /* __OPENPIC_H__ */
diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
index f3e97d8..3f6d58c 100644
--- a/hw/ppc/e500.c
+++ b/hw/ppc/e500.c
@@ -493,7 +493,7 @@ void ppce500_init(PPCE500Params *params)
 
     /* MPIC */
     mpic = mpic_init(ccsr_addr_space, MPC8544_MPIC_REGS_OFFSET,
-                     smp_cpus, irqs, NULL);
+                     smp_cpus, irqs);
 
     if (!mpic) {
         cpu_abort(env, "MPIC failed to initialize\n");
diff --git a/hw/ppc_newworld.c b/hw/ppc_newworld.c
index 664747e..b9c2cd8 100644
--- a/hw/ppc_newworld.c
+++ b/hw/ppc_newworld.c
@@ -320,7 +320,7 @@ static void ppc_core99_init(QEMUMachineInitArgs *args)
             exit(1);
         }
     }
-    pic = openpic_init(&pic_mem, smp_cpus, openpic_irqs, NULL);
+    pic = openpic_init(&pic_mem, smp_cpus, openpic_irqs);
     if (PPC_INPUT(env) == PPC_FLAGS_INPUT_970) {
         /* 970 gets a U3 bus */
         pci_bus = pci_pmac_u3_init(pic, get_system_memory(), get_system_io());
-- 
1.6.0.2

^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [Qemu-devel] [PATCH 14/19] openpic: convert to qdev
  2012-12-08 13:44 [Qemu-devel] [PATCH 00/19] OpenPIC refactoring and MSI support Alexander Graf
                   ` (12 preceding siblings ...)
  2012-12-08 13:44 ` [Qemu-devel] [PATCH 13/19] openpic: remove irq_out Alexander Graf
@ 2012-12-08 13:44 ` Alexander Graf
  2012-12-10 23:47   ` Scott Wood
  2012-12-08 13:44 ` [Qemu-devel] [PATCH 15/19] openpic: make brr1 model specific Alexander Graf
                   ` (4 subsequent siblings)
  18 siblings, 1 reply; 43+ messages in thread
From: Alexander Graf @ 2012-12-08 13:44 UTC (permalink / raw)
  To: qemu-ppc@nongnu.org List; +Cc: qemu-devel qemu-devel

This patch converts the OpenPIC device to qdev. Along the way it
renames the "openpic" target to "raven" and the "mpic" target to
"mpc8544", to better reflect the actual models they implement.

This way we have a generic OpenPIC device now that can handle
different flavors of the OpenPIC specification.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 hw/openpic.c      |  278 ++++++++++++++++++++++++++---------------------------
 hw/openpic.h      |    8 +-
 hw/ppc/e500.c     |   24 ++++-
 hw/ppc_newworld.c |   25 +++++-
 4 files changed, 180 insertions(+), 155 deletions(-)

diff --git a/hw/openpic.c b/hw/openpic.c
index 321c6d4..6aac4b8 100644
--- a/hw/openpic.c
+++ b/hw/openpic.c
@@ -37,6 +37,7 @@
 #include "ppc_mac.h"
 #include "pci.h"
 #include "openpic.h"
+#include "sysbus.h"
 
 //#define DEBUG_OPENPIC
 
@@ -53,30 +54,10 @@
 #define MAX_IPI     4
 #define VID         0x03 /* MPIC version ID */
 
-enum {
-    IRQ_IPVP = 0,
-    IRQ_IDE,
-};
-
-/* OpenPIC */
-#define OPENPIC_MAX_CPU      2
-#define OPENPIC_MAX_IRQ     64
-#define OPENPIC_EXT_IRQ     48
-#define OPENPIC_MAX_TMR      MAX_TMR
-#define OPENPIC_MAX_IPI      MAX_IPI
-
-/* Interrupt definitions */
-#define OPENPIC_IRQ_FE     (OPENPIC_EXT_IRQ)     /* Internal functional IRQ */
-#define OPENPIC_IRQ_ERR    (OPENPIC_EXT_IRQ + 1) /* Error IRQ */
-#define OPENPIC_IRQ_TIM0   (OPENPIC_EXT_IRQ + 2) /* First timer IRQ */
-#if OPENPIC_MAX_IPI > 0
-#define OPENPIC_IRQ_IPI0   (OPENPIC_IRQ_TIM0 + OPENPIC_MAX_TMR) /* First IPI IRQ */
-#define OPENPIC_IRQ_DBL0   (OPENPIC_IRQ_IPI0 + (OPENPIC_MAX_CPU * OPENPIC_MAX_IPI)) /* First doorbell IRQ */
-#else
-#define OPENPIC_IRQ_DBL0   (OPENPIC_IRQ_TIM0 + OPENPIC_MAX_TMR) /* First doorbell IRQ */
-#define OPENPIC_IRQ_MBX0   (OPENPIC_IRQ_DBL0 + OPENPIC_MAX_DBL) /* First mailbox IRQ */
-#endif
+/* OpenPIC capability flags */
+#define OPENPIC_FLAG_IDE_CRIT     (1 << 0)
 
+/* OpenPIC address map */
 #define OPENPIC_GLB_REG_START        0x0
 #define OPENPIC_GLB_REG_SIZE         0x10F0
 #define OPENPIC_TMR_REG_START        0x10F0
@@ -86,31 +67,37 @@ enum {
 #define OPENPIC_CPU_REG_START        0x20000
 #define OPENPIC_CPU_REG_SIZE         0x100 + ((MAX_CPU - 1) * 0x1000)
 
-/* MPIC */
-#define MPIC_MAX_CPU      1
-#define MPIC_MAX_EXT     12
-#define MPIC_MAX_INT     64
-#define MPIC_MAX_IRQ     MAX_IRQ
+/* Raven */
+#define RAVEN_MAX_CPU      2
+#define RAVEN_MAX_EXT     48
+#define RAVEN_MAX_IRQ     64
+#define RAVEN_MAX_TMR      MAX_TMR
+#define RAVEN_MAX_IPI      MAX_IPI
+
+/* Interrupt definitions */
+#define RAVEN_FE_IRQ     (RAVEN_MAX_EXT)     /* Internal functional IRQ */
+#define RAVEN_ERR_IRQ    (RAVEN_MAX_EXT + 1) /* Error IRQ */
+#define RAVEN_TMR_IRQ    (RAVEN_MAX_EXT + 2) /* First timer IRQ */
+#define RAVEN_IPI_IRQ    (RAVEN_TMR_IRQ + RAVEN_MAX_TMR) /* First IPI IRQ */
+/* First doorbell IRQ */
+#define RAVEN_DBL_IRQ    (RAVEN_IPI_IRQ + (RAVEN_MAX_CPU * RAVEN_MAX_IPI))
+
+/* MPC8544 */
+#define MPC8544_MAX_CPU      1
+#define MPC8544_MAX_EXT     12
+#define MPC8544_MAX_INT     64
+#define MPC8544_MAX_IRQ     MAX_IRQ
 
 /* Interrupt definitions */
 /* IRQs, accessible through the IRQ region */
-#define MPIC_EXT_IRQ      0x00
-#define MPIC_INT_IRQ      0x10
-#define MPIC_MSG_IRQ      0xb0
-#define MPIC_MSI_IRQ      0xe0
+#define MPC8544_EXT_IRQ      0x00
+#define MPC8544_INT_IRQ      0x10
+#define MPC8544_MSG_IRQ      0xb0
+#define MPC8544_MSI_IRQ      0xe0
 /* These are available through separate regions, but
    for simplicity's sake mapped into the same number space */
-#define MPIC_TMR_IRQ      0xf3
-#define MPIC_IPI_IRQ      0xfb
-
-#define MPIC_GLB_REG_START        0x0
-#define MPIC_GLB_REG_SIZE         0x10F0
-#define MPIC_TMR_REG_START        0x10F0
-#define MPIC_TMR_REG_SIZE         0x220
-#define MPIC_SRC_REG_START        0x10000
-#define MPIC_SRC_REG_SIZE         (MAX_IRQ * 0x20)
-#define MPIC_CPU_REG_START        0x20000
-#define MPIC_CPU_REG_SIZE         0x100 + ((MAX_CPU - 1) * 0x1000)
+#define MPC8544_TMR_IRQ      0xf3
+#define MPC8544_IPI_IRQ      0xfb
 
 /*
  * Block Revision Register1 (BRR1): QEMU does not fully emulate
@@ -128,6 +115,7 @@ enum {
 #define FREP_VID_SHIFT     0
 
 #define VID_REVISION_1_2   2
+#define VID_REVISION_1_3   3
 
 #define VENI_GENERIC      0x00000000 /* Generic Vendor ID */
 
@@ -204,10 +192,11 @@ typedef struct IRQ_dst_t {
 } IRQ_dst_t;
 
 typedef struct OpenPICState {
-    PCIDevice pci_dev;
+    SysBusDevice busdev;
     MemoryRegion mem;
 
     /* Behavior control */
+    uint32_t model;
     uint32_t flags;
     uint32_t nb_irqs;
     uint32_t vid;
@@ -230,15 +219,15 @@ typedef struct OpenPICState {
     IRQ_src_t src[MAX_IRQ];
     /* Local registers per output pin */
     IRQ_dst_t dst[MAX_CPU];
-    int nb_cpus;
+    uint32_t nb_cpus;
     /* Timer registers */
     struct {
         uint32_t ticc;  /* Global timer current count register */
         uint32_t tibc;  /* Global timer base count register */
     } timers[MAX_TMR];
-    int max_irq;
-    int irq_ipi0;
-    int irq_tim0;
+    uint32_t max_irq;
+    uint32_t irq_ipi0;
+    uint32_t irq_tim0;
 } OpenPICState;
 
 static void openpic_irq_raise(OpenPICState *opp, int n_CPU, IRQ_src_t *src);
@@ -410,9 +399,9 @@ static void openpic_set_irq(void *opaque, int n_IRQ, int level)
     openpic_update_irq(opp, n_IRQ);
 }
 
-static void openpic_reset (void *opaque)
+static void openpic_reset(DeviceState *d)
 {
-    OpenPICState *opp = (OpenPICState *)opaque;
+    OpenPICState *opp = FROM_SYSBUS(typeof (*opp), sysbus_from_qdev(d));
     int i;
 
     opp->glbc = 0x80000000;
@@ -505,7 +494,7 @@ static void openpic_gbl_write(void *opaque, hwaddr addr, uint64_t val,
         break;
     case 0x1020: /* GLBC */
         if (val & 0x80000000) {
-            openpic_reset(opp);
+            openpic_reset(&opp->busdev.qdev);
         }
         break;
     case 0x1080: /* VENI */
@@ -970,7 +959,7 @@ static void openpic_save(QEMUFile* f, void *opaque)
         qemu_put_sbe32s(f, &opp->src[i].pending);
     }
 
-    qemu_put_sbe32s(f, &opp->nb_cpus);
+    qemu_put_be32s(f, &opp->nb_cpus);
 
     for (i = 0; i < opp->nb_cpus; i++) {
         qemu_put_be32s(f, &opp->dst[i].pctp);
@@ -983,8 +972,6 @@ static void openpic_save(QEMUFile* f, void *opaque)
         qemu_put_be32s(f, &opp->timers[i].ticc);
         qemu_put_be32s(f, &opp->timers[i].tibc);
     }
-
-    pci_device_save(&opp->pci_dev, f);
 }
 
 static void openpic_load_IRQ_queue(QEMUFile* f, IRQ_queue_t *q)
@@ -1019,7 +1006,7 @@ static int openpic_load(QEMUFile* f, void *opaque, int version_id)
         qemu_get_sbe32s(f, &opp->src[i].pending);
     }
 
-    qemu_get_sbe32s(f, &opp->nb_cpus);
+    qemu_get_be32s(f, &opp->nb_cpus);
 
     for (i = 0; i < opp->nb_cpus; i++) {
         qemu_get_be32s(f, &opp->dst[i].pctp);
@@ -1033,7 +1020,7 @@ static int openpic_load(QEMUFile* f, void *opaque, int version_id)
         qemu_get_be32s(f, &opp->timers[i].tibc);
     }
 
-    return pci_device_load(&opp->pci_dev, f);
+    return 0;
 }
 
 static void openpic_irq_raise(OpenPICState *opp, int n_CPU, IRQ_src_t *src)
@@ -1047,17 +1034,18 @@ static void openpic_irq_raise(OpenPICState *opp, int n_CPU, IRQ_src_t *src)
     }
 }
 
-qemu_irq *openpic_init (MemoryRegion **pmem, int nb_cpus,
-                        qemu_irq **irqs)
+struct memreg {
+    const char             *name;
+    MemoryRegionOps const  *ops;
+    hwaddr      start_addr;
+    ram_addr_t              size;
+};
+
+static int openpic_init(SysBusDevice *dev)
 {
-    OpenPICState *opp;
-    int i;
-    struct {
-        const char             *name;
-        MemoryRegionOps const  *ops;
-        hwaddr      start_addr;
-        ram_addr_t              size;
-    } const list[] = {
+    OpenPICState *opp = FROM_SYSBUS(typeof (*opp), dev);
+    int i, j;
+    const struct memreg list_le[] = {
         {"glb", &openpic_glb_ops_le, OPENPIC_GLB_REG_START,
                                      OPENPIC_GLB_REG_SIZE},
         {"tmr", &openpic_tmr_ops_le, OPENPIC_TMR_REG_START,
@@ -1067,16 +1055,57 @@ qemu_irq *openpic_init (MemoryRegion **pmem, int nb_cpus,
         {"cpu", &openpic_cpu_ops_le, OPENPIC_CPU_REG_START,
                                      OPENPIC_CPU_REG_SIZE},
     };
+    const struct memreg list_be[] = {
+        {"glb", &openpic_glb_ops_be, OPENPIC_GLB_REG_START,
+                                     OPENPIC_GLB_REG_SIZE},
+        {"tmr", &openpic_tmr_ops_be, OPENPIC_TMR_REG_START,
+                                     OPENPIC_TMR_REG_SIZE},
+        {"src", &openpic_src_ops_be, OPENPIC_SRC_REG_START,
+                                     OPENPIC_SRC_REG_SIZE},
+        {"cpu", &openpic_cpu_ops_be, OPENPIC_CPU_REG_START,
+                                     OPENPIC_CPU_REG_SIZE},
+    };
+    struct memreg const *list;
 
-    /* XXX: for now, only one CPU is supported */
-    if (nb_cpus != 1)
-        return NULL;
-    opp = g_malloc0(sizeof(OpenPICState));
+    switch (opp->model) {
+    case OPENPIC_MODEL_MPC8544:
+    default:
+        opp->flags |= OPENPIC_FLAG_IDE_CRIT;
+        opp->nb_irqs = 80;
+        opp->vid = VID_REVISION_1_2;
+        opp->veni = VENI_GENERIC;
+        opp->spve_mask = 0xFFFF;
+        opp->tifr_reset = 0x00000000;
+        opp->ipvp_reset = 0x80000000;
+        opp->ide_reset = 0x00000001;
+        opp->max_irq = MPC8544_MAX_IRQ;
+        opp->irq_ipi0 = MPC8544_IPI_IRQ;
+        opp->irq_tim0 = MPC8544_TMR_IRQ;
+        list = list_be;
+        break;
+    case OPENPIC_MODEL_RAVEN:
+        opp->nb_irqs = RAVEN_MAX_EXT;
+        opp->vid = VID_REVISION_1_3;
+        opp->veni = VENI_GENERIC;
+        opp->spve_mask = 0xFF;
+        opp->tifr_reset = 0x003F7A00;
+        opp->ipvp_reset = 0xA0000000;
+        opp->ide_reset = 0x00000000;
+        opp->max_irq = RAVEN_MAX_IRQ;
+        opp->irq_ipi0 = RAVEN_IPI_IRQ;
+        opp->irq_tim0 = RAVEN_TMR_IRQ;
+        list = list_le;
+
+        /* Only UP supported today */
+        if (opp->nb_cpus != 1) {
+            return -EINVAL;
+        }
+        break;
+    }
 
     memory_region_init(&opp->mem, "openpic", 0x40000);
 
-    for (i = 0; i < ARRAY_SIZE(list); i++) {
-
+    for (i = 0; i < ARRAY_SIZE(list_le); i++) {
         memory_region_init_io(&opp->sub_io_mem[i], list[i].ops, opp,
                               list[i].name, list[i].size);
 
@@ -1084,83 +1113,48 @@ qemu_irq *openpic_init (MemoryRegion **pmem, int nb_cpus,
                                     &opp->sub_io_mem[i]);
     }
 
-    //    isu_base &= 0xFFFC0000;
-    opp->nb_cpus = nb_cpus;
-    opp->nb_irqs = OPENPIC_EXT_IRQ;
-    opp->vid = VID;
-    opp->veni = VENI_GENERIC;
-    opp->spve_mask = 0xFF;
-    opp->tifr_reset = 0x003F7A00;
-    opp->max_irq = OPENPIC_MAX_IRQ;
-    opp->irq_ipi0 = OPENPIC_IRQ_IPI0;
-    opp->irq_tim0 = OPENPIC_IRQ_TIM0;
-
-    for (i = 0; i < nb_cpus; i++)
-        opp->dst[i].irqs = irqs[i];
-
-    register_savevm(&opp->pci_dev.qdev, "openpic", 0, 2,
+    for (i = 0; i < opp->nb_cpus; i++) {
+        opp->dst[i].irqs = g_new(qemu_irq, OPENPIC_OUTPUT_NB);
+        for (j = 0; j < OPENPIC_OUTPUT_NB; j++) {
+            sysbus_init_irq(dev, &opp->dst[i].irqs[j]);
+        }
+    }
+
+    register_savevm(&opp->busdev.qdev, "openpic", 0, 2,
                     openpic_save, openpic_load, opp);
-    qemu_register_reset(openpic_reset, opp);
 
-    if (pmem)
-        *pmem = &opp->mem;
+    sysbus_init_mmio(dev, &opp->mem);
+    qdev_init_gpio_in(&dev->qdev, openpic_set_irq, opp->max_irq);
 
-    return qemu_allocate_irqs(openpic_set_irq, opp, opp->max_irq);
+    return 0;
 }
 
-qemu_irq *mpic_init (MemoryRegion *address_space, hwaddr base,
-                     int nb_cpus, qemu_irq **irqs)
-{
-    OpenPICState    *mpp;
-    int           i;
-    struct {
-        const char             *name;
-        MemoryRegionOps const  *ops;
-        hwaddr      start_addr;
-        ram_addr_t              size;
-    } const list[] = {
-        {"glb", &openpic_glb_ops_be, MPIC_GLB_REG_START, MPIC_GLB_REG_SIZE},
-        {"tmr", &openpic_tmr_ops_be, MPIC_TMR_REG_START, MPIC_TMR_REG_SIZE},
-        {"src", &openpic_src_ops_be, MPIC_SRC_REG_START, MPIC_SRC_REG_SIZE},
-        {"cpu", &openpic_cpu_ops_be, MPIC_CPU_REG_START, MPIC_CPU_REG_SIZE},
-    };
-
-    mpp = g_malloc0(sizeof(OpenPICState));
-
-    memory_region_init(&mpp->mem, "mpic", 0x40000);
-    memory_region_add_subregion(address_space, base, &mpp->mem);
+static Property openpic_properties[] = {
+    DEFINE_PROP_UINT32("model", OpenPICState, model, OPENPIC_MODEL_MPC8544),
+    DEFINE_PROP_UINT32("nb_cpus", OpenPICState, nb_cpus, 1),
+    DEFINE_PROP_END_OF_LIST(),
+};
 
-    for (i = 0; i < sizeof(list)/sizeof(list[0]); i++) {
+static void openpic_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
 
-        memory_region_init_io(&mpp->sub_io_mem[i], list[i].ops, mpp,
-                              list[i].name, list[i].size);
+    k->init = openpic_init;
+    dc->props = openpic_properties;
+    dc->reset = openpic_reset;
+}
 
-        memory_region_add_subregion(&mpp->mem, list[i].start_addr,
-                                    &mpp->sub_io_mem[i]);
-    }
+static TypeInfo openpic_info = {
+    .name          = "openpic",
+    .parent        = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(OpenPICState),
+    .class_init    = openpic_class_init,
+};
 
-    mpp->nb_cpus = nb_cpus;
-    /* 12 external sources, 48 internal sources , 4 timer sources,
-       4 IPI sources, 4 messaging sources, and 8 Shared MSI sources */
-    mpp->nb_irqs = 80;
-    mpp->vid = VID_REVISION_1_2;
-    mpp->veni = VENI_GENERIC;
-    mpp->spve_mask = 0xFFFF;
-    mpp->tifr_reset = 0x00000000;
-    mpp->ipvp_reset = 0x80000000;
-    mpp->ide_reset = 0x00000001;
-    mpp->max_irq = MPIC_MAX_IRQ;
-    mpp->irq_ipi0 = MPIC_IPI_IRQ;
-    mpp->irq_tim0 = MPIC_TMR_IRQ;
-
-    for (i = 0; i < nb_cpus; i++)
-        mpp->dst[i].irqs = irqs[i];
-
-    /* Enable critical interrupt support */
-    mpp->flags |= OPENPIC_FLAG_IDE_CRIT;
-
-    register_savevm(NULL, "mpic", 0, 2, openpic_save, openpic_load, mpp);
-    qemu_register_reset(openpic_reset, mpp);
-
-    return qemu_allocate_irqs(openpic_set_irq, mpp, mpp->max_irq);
+static void openpic_register_types(void)
+{
+    type_register_static(&openpic_info);
 }
+
+type_init(openpic_register_types)
diff --git a/hw/openpic.h b/hw/openpic.h
index 8a68f20..824609c 100644
--- a/hw/openpic.h
+++ b/hw/openpic.h
@@ -11,11 +11,7 @@ enum {
     OPENPIC_OUTPUT_NB,
 };
 
-/* OpenPIC capability flags */
-#define OPENPIC_FLAG_IDE_CRIT    (1 << 0)
+#define OPENPIC_MODEL_RAVEN       0
+#define OPENPIC_MODEL_MPC8544     1
 
-qemu_irq *openpic_init (MemoryRegion **pmem, int nb_cpus,
-                        qemu_irq **irqs);
-qemu_irq *mpic_init (MemoryRegion *address_space, hwaddr base,
-                     int nb_cpus, qemu_irq **irqs);
 #endif /* __OPENPIC_H__ */
diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
index 3f6d58c..f9893ad 100644
--- a/hw/ppc/e500.c
+++ b/hw/ppc/e500.c
@@ -418,7 +418,7 @@ void ppce500_init(PPCE500Params *params)
     target_ulong dt_base = 0;
     target_ulong initrd_base = 0;
     target_long initrd_size=0;
-    int i=0;
+    int i = 0, j, k;
     unsigned int pci_irq_nrs[4] = {1, 2, 3, 4};
     qemu_irq **irqs, *mpic;
     DeviceState *dev;
@@ -492,13 +492,27 @@ void ppce500_init(PPCE500Params *params)
                                 ccsr_addr_space);
 
     /* MPIC */
-    mpic = mpic_init(ccsr_addr_space, MPC8544_MPIC_REGS_OFFSET,
-                     smp_cpus, irqs);
+    mpic = g_new(qemu_irq, 256);
+    dev = qdev_create(NULL, "openpic");
+    qdev_prop_set_uint32(dev, "nb_cpus", smp_cpus);
+    qdev_prop_set_uint32(dev, "model", OPENPIC_MODEL_MPC8544);
+    qdev_init_nofail(dev);
+    s = sysbus_from_qdev(dev);
+
+    k = 0;
+    for (i = 0; i < smp_cpus; i++) {
+        for (j = 0; j < OPENPIC_OUTPUT_NB; j++) {
+            sysbus_connect_irq(s, k++, irqs[i][j]);
+        }
+    }
 
-    if (!mpic) {
-        cpu_abort(env, "MPIC failed to initialize\n");
+    for (i = 0; i < 256; i++) {
+        mpic[i] = qdev_get_gpio_in(dev, i);
     }
 
+    memory_region_add_subregion(ccsr_addr_space, MPC8544_MPIC_REGS_OFFSET,
+                                s->mmio[0].memory);
+
     /* Serial */
     if (serial_hds[0]) {
         serial_mm_init(ccsr_addr_space, MPC8544_SERIAL0_REGS_OFFSET,
diff --git a/hw/ppc_newworld.c b/hw/ppc_newworld.c
index b9c2cd8..8c2114e 100644
--- a/hw/ppc_newworld.c
+++ b/hw/ppc_newworld.c
@@ -67,6 +67,7 @@
 #include "hw/usb.h"
 #include "blockdev.h"
 #include "exec-memory.h"
+#include "sysbus.h"
 
 #define MAX_IDE_BUS 2
 #define CFG_ADDR 0xf0000510
@@ -141,7 +142,7 @@ static void ppc_core99_init(QEMUMachineInitArgs *args)
     char *filename;
     qemu_irq *pic, **openpic_irqs;
     MemoryRegion *unin_memory = g_new(MemoryRegion, 1);
-    int linux_boot, i;
+    int linux_boot, i, j, k;
     MemoryRegion *ram = g_new(MemoryRegion, 1), *bios = g_new(MemoryRegion, 1);
     hwaddr kernel_base, initrd_base, cmdline_base = 0;
     long kernel_size, initrd_size;
@@ -156,6 +157,8 @@ static void ppc_core99_init(QEMUMachineInitArgs *args)
     void *fw_cfg;
     void *dbdma;
     int machine_arch;
+    SysBusDevice *s;
+    DeviceState *dev;
 
     linux_boot = (kernel_filename != NULL);
 
@@ -320,7 +323,25 @@ static void ppc_core99_init(QEMUMachineInitArgs *args)
             exit(1);
         }
     }
-    pic = openpic_init(&pic_mem, smp_cpus, openpic_irqs);
+
+    pic = g_new(qemu_irq, 64);
+
+    dev = qdev_create(NULL, "openpic");
+    qdev_prop_set_uint32(dev, "model", OPENPIC_MODEL_RAVEN);
+    qdev_init_nofail(dev);
+    s = sysbus_from_qdev(dev);
+    pic_mem = s->mmio[0].memory;
+    k = 0;
+    for (i = 0; i < smp_cpus; i++) {
+        for (j = 0; j < OPENPIC_OUTPUT_NB; j++) {
+            sysbus_connect_irq(s, k++, openpic_irqs[i][j]);
+        }
+    }
+
+    for (i = 0; i < 64; i++) {
+        pic[i] = qdev_get_gpio_in(dev, i);
+    }
+
     if (PPC_INPUT(env) == PPC_FLAGS_INPUT_970) {
         /* 970 gets a U3 bus */
         pci_bus = pci_pmac_u3_init(pic, get_system_memory(), get_system_io());
-- 
1.6.0.2

^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [Qemu-devel] [PATCH 15/19] openpic: make brr1 model specific
  2012-12-08 13:44 [Qemu-devel] [PATCH 00/19] OpenPIC refactoring and MSI support Alexander Graf
                   ` (13 preceding siblings ...)
  2012-12-08 13:44 ` [Qemu-devel] [PATCH 14/19] openpic: convert to qdev Alexander Graf
@ 2012-12-08 13:44 ` Alexander Graf
  2012-12-08 13:44 ` [Qemu-devel] [PATCH 16/19] openpic: add Shared MSI support Alexander Graf
                   ` (3 subsequent siblings)
  18 siblings, 0 replies; 43+ messages in thread
From: Alexander Graf @ 2012-12-08 13:44 UTC (permalink / raw)
  To: qemu-ppc@nongnu.org List; +Cc: qemu-devel qemu-devel

Now that we can properly distinguish between openpic model differences,
let's move brr1 out of the raven code path.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 hw/openpic.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/hw/openpic.c b/hw/openpic.c
index 6aac4b8..f2f152f 100644
--- a/hw/openpic.c
+++ b/hw/openpic.c
@@ -205,6 +205,7 @@ typedef struct OpenPICState {
     uint32_t tifr_reset;
     uint32_t ipvp_reset;
     uint32_t ide_reset;
+    uint32_t brr1;
 
     /* Sub-regions */
     MemoryRegion sub_io_mem[7];
@@ -783,7 +784,7 @@ static uint32_t openpic_cpu_read_internal(void *opaque, hwaddr addr,
     addr &= 0xFF0;
     switch (addr) {
     case 0x00: /* Block Revision Register1 (BRR1) */
-        retval = FSL_BRR1_IPID | FSL_BRR1_IPMJ | FSL_BRR1_IPMN;
+        retval = opp->brr1;
         break;
     case 0x80: /* PCTP */
         retval = dst->pctp;
@@ -1081,6 +1082,7 @@ static int openpic_init(SysBusDevice *dev)
         opp->max_irq = MPC8544_MAX_IRQ;
         opp->irq_ipi0 = MPC8544_IPI_IRQ;
         opp->irq_tim0 = MPC8544_TMR_IRQ;
+        opp->brr1 = FSL_BRR1_IPID | FSL_BRR1_IPMJ | FSL_BRR1_IPMN;
         list = list_be;
         break;
     case OPENPIC_MODEL_RAVEN:
@@ -1094,6 +1096,7 @@ static int openpic_init(SysBusDevice *dev)
         opp->max_irq = RAVEN_MAX_IRQ;
         opp->irq_ipi0 = RAVEN_IPI_IRQ;
         opp->irq_tim0 = RAVEN_TMR_IRQ;
+        opp->brr1 = -1;
         list = list_le;
 
         /* Only UP supported today */
-- 
1.6.0.2

^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [Qemu-devel] [PATCH 16/19] openpic: add Shared MSI support
  2012-12-08 13:44 [Qemu-devel] [PATCH 00/19] OpenPIC refactoring and MSI support Alexander Graf
                   ` (14 preceding siblings ...)
  2012-12-08 13:44 ` [Qemu-devel] [PATCH 15/19] openpic: make brr1 model specific Alexander Graf
@ 2012-12-08 13:44 ` Alexander Graf
  2012-12-11  0:36   ` [Qemu-devel] [Qemu-ppc] " Scott Wood
  2012-12-08 13:44 ` [Qemu-devel] [PATCH 17/19] PPC: e500: Add " Alexander Graf
                   ` (2 subsequent siblings)
  18 siblings, 1 reply; 43+ messages in thread
From: Alexander Graf @ 2012-12-08 13:44 UTC (permalink / raw)
  To: qemu-ppc@nongnu.org List; +Cc: qemu-devel qemu-devel

The OpenPIC allows MSI access through shared MSI registers. Implement
them for the MPC8544 MPIC, so we can support MSIs.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 hw/openpic.c |  150 ++++++++++++++++++++++++++++++++++++++++++++++++++--------
 1 files changed, 130 insertions(+), 20 deletions(-)

diff --git a/hw/openpic.c b/hw/openpic.c
index f2f152f..f71d668 100644
--- a/hw/openpic.c
+++ b/hw/openpic.c
@@ -38,6 +38,7 @@
 #include "pci.h"
 #include "openpic.h"
 #include "sysbus.h"
+#include "msi.h"
 
 //#define DEBUG_OPENPIC
 
@@ -52,6 +53,7 @@
 #define MAX_TMR     4
 #define VECTOR_BITS 8
 #define MAX_IPI     4
+#define MAX_MSI     8
 #define VID         0x03 /* MPIC version ID */
 
 /* OpenPIC capability flags */
@@ -62,6 +64,8 @@
 #define OPENPIC_GLB_REG_SIZE         0x10F0
 #define OPENPIC_TMR_REG_START        0x10F0
 #define OPENPIC_TMR_REG_SIZE         0x220
+#define OPENPIC_MSI_REG_START        0x1600
+#define OPENPIC_MSI_REG_SIZE         0x200
 #define OPENPIC_SRC_REG_START        0x10000
 #define OPENPIC_SRC_REG_SIZE         (MAX_IRQ * 0x20)
 #define OPENPIC_CPU_REG_START        0x20000
@@ -126,6 +130,12 @@
 #define IDR_P1_SHIFT      1
 #define IDR_P0_SHIFT      0
 
+#define MSIIR_OFFSET       0x140
+#define MSIIR_SRS_SHIFT    29
+#define MSIIR_SRS_MASK     (0x7 << MSIIR_SRS_SHIFT)
+#define MSIIR_IBS_SHIFT    24
+#define MSIIR_IBS_MASK     (0x1f << MSIIR_IBS_SHIFT)
+
 #define BF_WIDTH(_bits_) \
 (((_bits_) + (sizeof(uint32_t) * 8) - 1) / (sizeof(uint32_t) * 8))
 
@@ -208,7 +218,7 @@ typedef struct OpenPICState {
     uint32_t brr1;
 
     /* Sub-regions */
-    MemoryRegion sub_io_mem[7];
+    MemoryRegion sub_io_mem[5];
 
     /* Global registers */
     uint32_t frep; /* Feature reporting register */
@@ -226,9 +236,14 @@ typedef struct OpenPICState {
         uint32_t ticc;  /* Global timer current count register */
         uint32_t tibc;  /* Global timer base count register */
     } timers[MAX_TMR];
+    /* Shared MSI registers */
+    struct {
+        uint32_t msir;   /* Shared Message Signaled Interrupt Register */
+    } msi[MAX_MSI];
     uint32_t max_irq;
     uint32_t irq_ipi0;
     uint32_t irq_tim0;
+    uint32_t irq_msi;
 } OpenPICState;
 
 static void openpic_irq_raise(OpenPICState *opp, int n_CPU, IRQ_src_t *src);
@@ -703,6 +718,68 @@ static uint64_t openpic_src_read(void *opaque, uint64_t addr, unsigned len)
     return retval;
 }
 
+static void openpic_msi_write(void *opaque, hwaddr addr, uint64_t val,
+                              unsigned size)
+{
+    OpenPICState *opp = opaque;
+    int idx = opp->irq_msi;
+    int srs, ibs;
+
+    DPRINTF("%s: addr " TARGET_FMT_plx " <= %08x\n", __func__, addr, val);
+    if (addr & 0xF) {
+        return;
+    }
+
+    switch (addr) {
+    case MSIIR_OFFSET:
+        srs = val >> MSIIR_SRS_SHIFT;
+        idx += srs;
+        ibs = (val & MSIIR_IBS_MASK) >> MSIIR_IBS_SHIFT;
+        opp->msi[srs].msir |= 1 << ibs;
+        openpic_set_irq(opp, idx, 1);
+        break;
+    default:
+        /* most registers are read-only, thus ignored */
+        break;
+    }
+}
+
+static uint64_t openpic_msi_read(void *opaque, hwaddr addr, unsigned size)
+{
+    OpenPICState *opp = opaque;
+    uint64_t r = 0;
+    int i, srs;
+
+    DPRINTF("%s: addr " TARGET_FMT_plx "\n", __func__, addr);
+    if (addr & 0xF) {
+        return -1;
+    }
+
+    srs = addr >> 4;
+
+    switch (addr) {
+    case 0x00:
+    case 0x10:
+    case 0x20:
+    case 0x30:
+    case 0x40:
+    case 0x50:
+    case 0x60:
+    case 0x70: /* MSIRs */
+        r = opp->msi[srs].msir;
+        /* Clear on read */
+        opp->msi[srs].msir = 0;
+        break;
+    case 0x120: /* MSISR */
+        for (i = 0; i < MAX_MSI; i++) {
+            r |= (opp->msi[i].msir ? 1 : 0) << i;
+        }
+        break;
+    }
+
+    return r;
+}
+
 static void openpic_cpu_write_internal(void *opaque, hwaddr addr,
                                        uint32_t val, int idx)
 {
@@ -931,6 +1008,26 @@ static const MemoryRegionOps openpic_src_ops_be = {
     },
 };
 
+static const MemoryRegionOps openpic_msi_ops_le = {
+    .read = openpic_msi_read,
+    .write = openpic_msi_write,
+    .endianness = DEVICE_LITTLE_ENDIAN,
+    .impl = {
+        .min_access_size = 4,
+        .max_access_size = 4,
+    },
+};
+
+static const MemoryRegionOps openpic_msi_ops_be = {
+    .read = openpic_msi_read,
+    .write = openpic_msi_write,
+    .endianness = DEVICE_BIG_ENDIAN,
+    .impl = {
+        .min_access_size = 4,
+        .max_access_size = 4,
+    },
+};
+
 static void openpic_save_IRQ_queue(QEMUFile* f, IRQ_queue_t *q)
 {
     unsigned int i;
@@ -1038,6 +1135,7 @@ static void openpic_irq_raise(OpenPICState *opp, int n_CPU, IRQ_src_t *src)
 struct memreg {
     const char             *name;
     MemoryRegionOps const  *ops;
+    bool                   map;
     hwaddr      start_addr;
     ram_addr_t              size;
 };
@@ -1046,27 +1144,31 @@ static int openpic_init(SysBusDevice *dev)
 {
     OpenPICState *opp = FROM_SYSBUS(typeof (*opp), dev);
     int i, j;
-    const struct memreg list_le[] = {
-        {"glb", &openpic_glb_ops_le, OPENPIC_GLB_REG_START,
-                                     OPENPIC_GLB_REG_SIZE},
-        {"tmr", &openpic_tmr_ops_le, OPENPIC_TMR_REG_START,
-                                     OPENPIC_TMR_REG_SIZE},
-        {"src", &openpic_src_ops_le, OPENPIC_SRC_REG_START,
-                                     OPENPIC_SRC_REG_SIZE},
-        {"cpu", &openpic_cpu_ops_le, OPENPIC_CPU_REG_START,
-                                     OPENPIC_CPU_REG_SIZE},
+    struct memreg list_le[] = {
+        {"glb", &openpic_glb_ops_le, true,
+                OPENPIC_GLB_REG_START, OPENPIC_GLB_REG_SIZE},
+        {"tmr", &openpic_tmr_ops_le, true,
+                OPENPIC_TMR_REG_START, OPENPIC_TMR_REG_SIZE},
+        {"msi", &openpic_msi_ops_le, true,
+                OPENPIC_MSI_REG_START, OPENPIC_MSI_REG_SIZE},
+        {"src", &openpic_src_ops_le, true,
+                OPENPIC_SRC_REG_START, OPENPIC_SRC_REG_SIZE},
+        {"cpu", &openpic_cpu_ops_le, true,
+                OPENPIC_CPU_REG_START, OPENPIC_CPU_REG_SIZE},
     };
-    const struct memreg list_be[] = {
-        {"glb", &openpic_glb_ops_be, OPENPIC_GLB_REG_START,
-                                     OPENPIC_GLB_REG_SIZE},
-        {"tmr", &openpic_tmr_ops_be, OPENPIC_TMR_REG_START,
-                                     OPENPIC_TMR_REG_SIZE},
-        {"src", &openpic_src_ops_be, OPENPIC_SRC_REG_START,
-                                     OPENPIC_SRC_REG_SIZE},
-        {"cpu", &openpic_cpu_ops_be, OPENPIC_CPU_REG_START,
-                                     OPENPIC_CPU_REG_SIZE},
+    struct memreg list_be[] = {
+        {"glb", &openpic_glb_ops_be, true,
+                OPENPIC_GLB_REG_START, OPENPIC_GLB_REG_SIZE},
+        {"tmr", &openpic_tmr_ops_be, true,
+                OPENPIC_TMR_REG_START, OPENPIC_TMR_REG_SIZE},
+        {"msi", &openpic_msi_ops_be, true,
+                OPENPIC_MSI_REG_START, OPENPIC_MSI_REG_SIZE},
+        {"src", &openpic_src_ops_be, true,
+                OPENPIC_SRC_REG_START, OPENPIC_SRC_REG_SIZE},
+        {"cpu", &openpic_cpu_ops_be, true,
+                OPENPIC_CPU_REG_START, OPENPIC_CPU_REG_SIZE},
     };
-    struct memreg const *list;
+    struct memreg *list;
 
     switch (opp->model) {
     case OPENPIC_MODEL_MPC8544:
@@ -1082,7 +1184,9 @@ static int openpic_init(SysBusDevice *dev)
         opp->max_irq = MPC8544_MAX_IRQ;
         opp->irq_ipi0 = MPC8544_IPI_IRQ;
         opp->irq_tim0 = MPC8544_TMR_IRQ;
+        opp->irq_msi = MPC8544_MSI_IRQ;
         opp->brr1 = FSL_BRR1_IPID | FSL_BRR1_IPMJ | FSL_BRR1_IPMN;
+        msi_supported = true;
         list = list_be;
         break;
     case OPENPIC_MODEL_RAVEN:
@@ -1098,6 +1202,8 @@ static int openpic_init(SysBusDevice *dev)
         opp->irq_tim0 = RAVEN_TMR_IRQ;
         opp->brr1 = -1;
         list = list_le;
+        /* Don't map MSI region */
+        list[2].map = false;
 
         /* Only UP supported today */
         if (opp->nb_cpus != 1) {
@@ -1109,6 +1215,10 @@ static int openpic_init(SysBusDevice *dev)
     memory_region_init(&opp->mem, "openpic", 0x40000);
 
     for (i = 0; i < ARRAY_SIZE(list_le); i++) {
+        if (!list[i].map) {
+            continue;
+        }
+
         memory_region_init_io(&opp->sub_io_mem[i], list[i].ops, opp,
                               list[i].name, list[i].size);
 
-- 
1.6.0.2

^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [Qemu-devel] [PATCH 17/19] PPC: e500: Add MSI support
  2012-12-08 13:44 [Qemu-devel] [PATCH 00/19] OpenPIC refactoring and MSI support Alexander Graf
                   ` (15 preceding siblings ...)
  2012-12-08 13:44 ` [Qemu-devel] [PATCH 16/19] openpic: add Shared MSI support Alexander Graf
@ 2012-12-08 13:44 ` Alexander Graf
  2012-12-08 13:44 ` [Qemu-devel] [PATCH 18/19] PPC: e500: Declare pci bridge as bridge Alexander Graf
  2012-12-08 13:44 ` [Qemu-devel] [PATCH 19/19] MSI-X: Fix endianness Alexander Graf
  18 siblings, 0 replies; 43+ messages in thread
From: Alexander Graf @ 2012-12-08 13:44 UTC (permalink / raw)
  To: qemu-ppc@nongnu.org List; +Cc: qemu-devel qemu-devel

Now that our interrupt controller supports MSIs, let's expose that feature
to the guest through the device tree!

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 hw/ppc/e500.c |   23 +++++++++++++++++++++++
 1 files changed, 23 insertions(+), 0 deletions(-)

diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
index f9893ad..4942908 100644
--- a/hw/ppc/e500.c
+++ b/hw/ppc/e500.c
@@ -48,6 +48,7 @@
 #define MPC8544_CCSRBAR_BASE       0xE0000000ULL
 #define MPC8544_CCSRBAR_SIZE       0x00100000ULL
 #define MPC8544_MPIC_REGS_OFFSET   0x40000ULL
+#define MPC8544_MSI_REGS_OFFSET   0x41600ULL
 #define MPC8544_SERIAL0_REGS_OFFSET 0x4500ULL
 #define MPC8544_SERIAL1_REGS_OFFSET 0x4600ULL
 #define MPC8544_PCI_REGS_OFFSET    0x8000ULL
@@ -127,8 +128,10 @@ static int ppce500_load_device_tree(CPUPPCState *env,
     char soc[128];
     char mpic[128];
     uint32_t mpic_ph;
+    uint32_t msi_ph;
     char gutil[128];
     char pci[128];
+    char msi[128];
     uint32_t pci_map[7 * 8];
     uint32_t pci_ranges[14] =
         {
@@ -300,6 +303,25 @@ static int ppce500_load_device_tree(CPUPPCState *env,
     qemu_devtree_setprop_cells(fdt, gutil, "reg", MPC8544_UTIL_OFFSET, 0x1000);
     qemu_devtree_setprop(fdt, gutil, "fsl,has-rstcr", NULL, 0);
 
+    snprintf(msi, sizeof(msi), "/%s/msi@%llx", soc, MPC8544_MSI_REGS_OFFSET);
+    qemu_devtree_add_subnode(fdt, msi);
+    qemu_devtree_setprop_string(fdt, msi, "compatible", "fsl,mpic-msi");
+    qemu_devtree_setprop_cells(fdt, msi, "reg", MPC8544_MSI_REGS_OFFSET, 0x200);
+    msi_ph = qemu_devtree_alloc_phandle(fdt);
+    qemu_devtree_setprop_cells(fdt, msi, "msi-available-ranges", 0x0, 0x100);
+    qemu_devtree_setprop_phandle(fdt, msi, "interrupt-parent", mpic);
+    qemu_devtree_setprop_cells(fdt, msi, "interrupts",
+        0xe0, 0x0,
+        0xe1, 0x0,
+        0xe2, 0x0,
+        0xe3, 0x0,
+        0xe4, 0x0,
+        0xe5, 0x0,
+        0xe6, 0x0,
+        0xe7, 0x0);
+    qemu_devtree_setprop_cell(fdt, msi, "phandle", msi_ph);
+    qemu_devtree_setprop_cell(fdt, msi, "linux,phandle", msi_ph);
+
     snprintf(pci, sizeof(pci), "/pci@%llx", MPC8544_PCI_REGS_BASE);
     qemu_devtree_add_subnode(fdt, pci);
     qemu_devtree_setprop_cell(fdt, pci, "cell-index", 0);
@@ -315,6 +337,7 @@ static int ppce500_load_device_tree(CPUPPCState *env,
     for (i = 0; i < 14; i++) {
         pci_ranges[i] = cpu_to_be32(pci_ranges[i]);
     }
+    qemu_devtree_setprop_cell(fdt, pci, "fsl,msi", msi_ph);
     qemu_devtree_setprop(fdt, pci, "ranges", pci_ranges, sizeof(pci_ranges));
     qemu_devtree_setprop_cells(fdt, pci, "reg", MPC8544_PCI_REGS_BASE >> 32,
                                MPC8544_PCI_REGS_BASE, 0, 0x1000);
-- 
1.6.0.2

^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [Qemu-devel] [PATCH 18/19] PPC: e500: Declare pci bridge as bridge
  2012-12-08 13:44 [Qemu-devel] [PATCH 00/19] OpenPIC refactoring and MSI support Alexander Graf
                   ` (16 preceding siblings ...)
  2012-12-08 13:44 ` [Qemu-devel] [PATCH 17/19] PPC: e500: Add " Alexander Graf
@ 2012-12-08 13:44 ` Alexander Graf
  2012-12-08 13:44 ` [Qemu-devel] [PATCH 19/19] MSI-X: Fix endianness Alexander Graf
  18 siblings, 0 replies; 43+ messages in thread
From: Alexander Graf @ 2012-12-08 13:44 UTC (permalink / raw)
  To: qemu-ppc@nongnu.org List; +Cc: qemu-devel qemu-devel

The new PCI host bridge device needs to identify itself as PCI host bridge.
Declare it as such.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 hw/ppce500_pci.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/hw/ppce500_pci.c b/hw/ppce500_pci.c
index 54c72b4..e534341 100644
--- a/hw/ppce500_pci.c
+++ b/hw/ppce500_pci.c
@@ -330,9 +330,15 @@ static int e500_pcihost_bridge_initfn(PCIDevice *d)
     PPCE500CCSRState *ccsr = CCSR(container_get(qdev_get_machine(),
                                   "/e500-ccsr"));
 
+    pci_config_set_class(d->config, PCI_CLASS_BRIDGE_PCI);
+    d->config[PCI_HEADER_TYPE] =
+        (d->config[PCI_HEADER_TYPE] & PCI_HEADER_TYPE_MULTI_FUNCTION) |
+        PCI_HEADER_TYPE_BRIDGE;
+
     memory_region_init_alias(&b->bar0, "e500-pci-bar0", &ccsr->ccsr_space,
                              0, int128_get64(ccsr->ccsr_space.size));
     pci_register_bar(d, 0, PCI_BASE_ADDRESS_SPACE_MEMORY, &b->bar0);
+
     return 0;
 }
 
-- 
1.6.0.2

^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [Qemu-devel] [PATCH 19/19] MSI-X: Fix endianness
  2012-12-08 13:44 [Qemu-devel] [PATCH 00/19] OpenPIC refactoring and MSI support Alexander Graf
                   ` (17 preceding siblings ...)
  2012-12-08 13:44 ` [Qemu-devel] [PATCH 18/19] PPC: e500: Declare pci bridge as bridge Alexander Graf
@ 2012-12-08 13:44 ` Alexander Graf
  2012-12-08 22:41   ` Michael S. Tsirkin
  18 siblings, 1 reply; 43+ messages in thread
From: Alexander Graf @ 2012-12-08 13:44 UTC (permalink / raw)
  To: qemu-ppc@nongnu.org List; +Cc: qemu-devel qemu-devel, Michael S. Tsirkin

The MSI-X vector tables are usually stored in little endian in memory,
so let's mark the accessors as such.

This fixes MSI-X on e500 for me.

Signed-off-by: Alexander Graf <agraf@suse.de>
CC: Michael S. Tsirkin <mst@redhat.com>
---
 hw/msix.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/hw/msix.c b/hw/msix.c
index 136ef09..b57ae60 100644
--- a/hw/msix.c
+++ b/hw/msix.c
@@ -180,8 +180,7 @@ static void msix_table_mmio_write(void *opaque, hwaddr addr,
 static const MemoryRegionOps msix_table_mmio_ops = {
     .read = msix_table_mmio_read,
     .write = msix_table_mmio_write,
-    /* TODO: MSIX should be LITTLE_ENDIAN. */
-    .endianness = DEVICE_NATIVE_ENDIAN,
+    .endianness = DEVICE_LITTLE_ENDIAN,
     .valid = {
         .min_access_size = 4,
         .max_access_size = 4,
@@ -198,8 +197,7 @@ static uint64_t msix_pba_mmio_read(void *opaque, hwaddr addr,
 
 static const MemoryRegionOps msix_pba_mmio_ops = {
     .read = msix_pba_mmio_read,
-    /* TODO: MSIX should be LITTLE_ENDIAN. */
-    .endianness = DEVICE_NATIVE_ENDIAN,
+    .endianness = DEVICE_LITTLE_ENDIAN,
     .valid = {
         .min_access_size = 4,
         .max_access_size = 4,
-- 
1.6.0.2

^ permalink raw reply related	[flat|nested] 43+ messages in thread

* Re: [Qemu-devel] [PATCH 01/19] openpic: Remove unused code
  2012-12-08 13:44 ` [Qemu-devel] [PATCH 01/19] openpic: Remove unused code Alexander Graf
@ 2012-12-08 15:12   ` Andreas Färber
  2012-12-08 15:14     ` Alexander Graf
  2012-12-08 17:06     ` Hervé Poussineau
  0 siblings, 2 replies; 43+ messages in thread
From: Andreas Färber @ 2012-12-08 15:12 UTC (permalink / raw)
  To: Alexander Graf
  Cc: Hervé Poussineau, qemu-ppc@nongnu.org List, qemu-devel qemu-devel

Am 08.12.2012 14:44, schrieb Alexander Graf:
> The openpic code had a few WIP bits left that nobody reanimated within
> the last few years. Remove that code.
> 
> Signed-off-by: Alexander Graf <agraf@suse.de>

Reviewed-by: Andreas Färber <afaerber@suse.de>

I'm not aware of any GW80314 users, CC'ing Hervé just to be sure.
The BeBox has an i82378 (sitting on its MPC105), using i8259 PICs.

But even if there were something using them, this is most definitely the
wrong way to model variations nowadays. :)

Alex, did you find out what was broken about it?

Andreas

> ---
>  hw/openpic.c |  163 ----------------------------------------------------------
>  1 files changed, 0 insertions(+), 163 deletions(-)
> 
> diff --git a/hw/openpic.c b/hw/openpic.c
> index 8b3784a..b30c853 100644
> --- a/hw/openpic.c
> +++ b/hw/openpic.c
> @@ -46,27 +46,8 @@
>  #define DPRINTF(fmt, ...) do { } while (0)
>  #endif
>  
> -#define USE_MPCxxx /* Intel model is broken, for now */
> -
> -#if defined (USE_INTEL_GW80314)
> -/* Intel GW80314 I/O Companion chip */
> -
> -#define MAX_CPU     4
> -#define MAX_IRQ    32
> -#define MAX_DBL     4
> -#define MAX_MBX     4
> -#define MAX_TMR     4
> -#define VECTOR_BITS 8
> -#define MAX_IPI     4
> -
> -#define VID (0x00000000)
> -
> -#elif defined(USE_MPCxxx)
> -
>  #define MAX_CPU    15
>  #define MAX_IRQ   128
> -#define MAX_DBL     0
> -#define MAX_MBX     0
>  #define MAX_TMR     4
>  #define VECTOR_BITS 8
>  #define MAX_IPI     4
> @@ -149,12 +130,6 @@ enum mpic_ide_bits {
>      IDR_P0     = 0,
>  };
>  
> -#else
> -#error "Please select which OpenPic implementation is to be emulated"
> -#endif
> -
> -#define OPENPIC_PAGE_SIZE 4096
> -
>  #define BF_WIDTH(_bits_) \
>  (((_bits_) + (sizeof(uint32_t) * 8) - 1) / (sizeof(uint32_t) * 8))
>  
> @@ -250,19 +225,6 @@ typedef struct openpic_t {
>          uint32_t ticc;  /* Global timer current count register */
>          uint32_t tibc;  /* Global timer base count register */
>      } timers[MAX_TMR];
> -#if MAX_DBL > 0
> -    /* Doorbell registers */
> -    uint32_t dar;        /* Doorbell activate register */
> -    struct {
> -        uint32_t dmr;    /* Doorbell messaging register */
> -    } doorbells[MAX_DBL];
> -#endif
> -#if MAX_MBX > 0
> -    /* Mailbox registers */
> -    struct {
> -        uint32_t mbr;    /* Mailbox register */
> -    } mailboxes[MAX_MAILBOXES];
> -#endif
>      /* IRQ out is used when in bypass mode (not implemented) */
>      qemu_irq irq_out;
>      int max_irq;
> @@ -470,19 +432,6 @@ static void openpic_reset (void *opaque)
>          opp->timers[i].ticc = 0x00000000;
>          opp->timers[i].tibc = 0x80000000;
>      }
> -    /* Initialise doorbells */
> -#if MAX_DBL > 0
> -    opp->dar = 0x00000000;
> -    for (i = 0; i < MAX_DBL; i++) {
> -        opp->doorbells[i].dmr  = 0x00000000;
> -    }
> -#endif
> -    /* Initialise mailboxes */
> -#if MAX_MBX > 0
> -    for (i = 0; i < MAX_MBX; i++) { /* ? */
> -        opp->mailboxes[i].mbr   = 0x00000000;
> -    }
> -#endif
>      /* Go out of RESET state */
>      opp->glbc = 0x00000000;
>  }
> @@ -518,84 +467,6 @@ static inline void write_IRQreg_ipvp(openpic_t *opp, int n_IRQ, uint32_t val)
>              opp->src[n_IRQ].ipvp);
>  }
>  
> -#if 0 // Code provision for Intel model
> -#if MAX_DBL > 0
> -static uint32_t read_doorbell_register (openpic_t *opp,
> -                                        int n_dbl, uint32_t offset)
> -{
> -    uint32_t retval;
> -
> -    switch (offset) {
> -    case DBL_IPVP_OFFSET:
> -        retval = read_IRQreg_ipvp(opp, IRQ_DBL0 + n_dbl);
> -        break;
> -    case DBL_IDE_OFFSET:
> -        retval = read_IRQreg_ide(opp, IRQ_DBL0 + n_dbl);
> -        break;
> -    case DBL_DMR_OFFSET:
> -        retval = opp->doorbells[n_dbl].dmr;
> -        break;
> -    }
> -
> -    return retval;
> -}
> -
> -static void write_doorbell_register (penpic_t *opp, int n_dbl,
> -                                     uint32_t offset, uint32_t value)
> -{
> -    switch (offset) {
> -    case DBL_IVPR_OFFSET:
> -        write_IRQreg_ipvp(opp, IRQ_DBL0 + n_dbl, value);
> -        break;
> -    case DBL_IDE_OFFSET:
> -        write_IRQreg_ide(opp, IRQ_DBL0 + n_dbl, value);
> -        break;
> -    case DBL_DMR_OFFSET:
> -        opp->doorbells[n_dbl].dmr = value;
> -        break;
> -    }
> -}
> -#endif
> -
> -#if MAX_MBX > 0
> -static uint32_t read_mailbox_register (openpic_t *opp,
> -                                       int n_mbx, uint32_t offset)
> -{
> -    uint32_t retval;
> -
> -    switch (offset) {
> -    case MBX_MBR_OFFSET:
> -        retval = opp->mailboxes[n_mbx].mbr;
> -        break;
> -    case MBX_IVPR_OFFSET:
> -        retval = read_IRQreg_ipvp(opp, IRQ_MBX0 + n_mbx);
> -        break;
> -    case MBX_DMR_OFFSET:
> -        retval = read_IRQreg_ide(opp, IRQ_MBX0 + n_mbx);
> -        break;
> -    }
> -
> -    return retval;
> -}
> -
> -static void write_mailbox_register (openpic_t *opp, int n_mbx,
> -                                    uint32_t address, uint32_t value)
> -{
> -    switch (offset) {
> -    case MBX_MBR_OFFSET:
> -        opp->mailboxes[n_mbx].mbr = value;
> -        break;
> -    case MBX_IVPR_OFFSET:
> -        write_IRQreg_ipvp(opp, IRQ_MBX0 + n_mbx, value);
> -        break;
> -    case MBX_DMR_OFFSET:
> -        write_IRQreg_ide(opp, IRQ_MBX0 + n_mbx, value);
> -        break;
> -    }
> -}
> -#endif
> -#endif /* 0 : Code provision for Intel model */
> -
>  static void openpic_gbl_write (void *opaque, hwaddr addr, uint32_t val)
>  {
>      openpic_t *opp = opaque;
> @@ -841,7 +712,6 @@ static void openpic_cpu_write_internal(void *opaque, hwaddr addr,
>      dst = &opp->dst[idx];
>      addr &= 0xFF0;
>      switch (addr) {
> -#if MAX_IPI > 0
>      case 0x40: /* IPIDR */
>      case 0x50:
>      case 0x60:
> @@ -853,7 +723,6 @@ static void openpic_cpu_write_internal(void *opaque, hwaddr addr,
>          openpic_set_irq(opp, opp->irq_ipi0 + idx, 1);
>          openpic_set_irq(opp, opp->irq_ipi0 + idx, 0);
>          break;
> -#endif
>      case 0x80: /* PCTP */
>          dst->pctp = val & 0x0000000F;
>          break;
> @@ -1109,20 +978,6 @@ static void openpic_save(QEMUFile* f, void *opaque)
>          qemu_put_be32s(f, &opp->timers[i].tibc);
>      }
>  
> -#if MAX_DBL > 0
> -    qemu_put_be32s(f, &opp->dar);
> -
> -    for (i = 0; i < MAX_DBL; i++) {
> -        qemu_put_be32s(f, &opp->doorbells[i].dmr);
> -    }
> -#endif
> -
> -#if MAX_MBX > 0
> -    for (i = 0; i < MAX_MAILBOXES; i++) {
> -        qemu_put_be32s(f, &opp->mailboxes[i].mbr);
> -    }
> -#endif
> -
>      pci_device_save(&opp->pci_dev, f);
>  }
>  
> @@ -1176,20 +1031,6 @@ static int openpic_load(QEMUFile* f, void *opaque, int version_id)
>          qemu_get_be32s(f, &opp->timers[i].tibc);
>      }
>  
> -#if MAX_DBL > 0
> -    qemu_get_be32s(f, &opp->dar);
> -
> -    for (i = 0; i < MAX_DBL; i++) {
> -        qemu_get_be32s(f, &opp->doorbells[i].dmr);
> -    }
> -#endif
> -
> -#if MAX_MBX > 0
> -    for (i = 0; i < MAX_MAILBOXES; i++) {
> -        qemu_get_be32s(f, &opp->mailboxes[i].mbr);
> -    }
> -#endif
> -
>      return pci_device_load(&opp->pci_dev, f);
>  }
>  
> @@ -1222,11 +1063,7 @@ qemu_irq *openpic_init (MemoryRegion **pmem, int nb_cpus,
>      for (; i < OPENPIC_IRQ_TIM0; i++) {
>          opp->src[i].type = IRQ_SPECIAL;
>      }
> -#if MAX_IPI > 0
>      m = OPENPIC_IRQ_IPI0;
> -#else
> -    m = OPENPIC_IRQ_DBL0;
> -#endif
>      for (; i < m; i++) {
>          opp->src[i].type = IRQ_TIMER;
>      }
> 


-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [Qemu-devel] [PATCH 01/19] openpic: Remove unused code
  2012-12-08 15:12   ` Andreas Färber
@ 2012-12-08 15:14     ` Alexander Graf
  2012-12-08 17:06     ` Hervé Poussineau
  1 sibling, 0 replies; 43+ messages in thread
From: Alexander Graf @ 2012-12-08 15:14 UTC (permalink / raw)
  To: Andreas Färber
  Cc: Hervé Poussineau, qemu-ppc@nongnu.org List, qemu-devel qemu-devel


On 08.12.2012, at 16:12, Andreas Färber wrote:

> Am 08.12.2012 14:44, schrieb Alexander Graf:
>> The openpic code had a few WIP bits left that nobody reanimated within
>> the last few years. Remove that code.
>> 
>> Signed-off-by: Alexander Graf <agraf@suse.de>
> 
> Reviewed-by: Andreas Färber <afaerber@suse.de>
> 
> I'm not aware of any GW80314 users, CC'ing Hervé just to be sure.
> The BeBox has an i82378 (sitting on its MPC105), using i8259 PICs.
> 
> But even if there were something using them, this is most definitely the
> wrong way to model variations nowadays. :)
> 
> Alex, did you find out what was broken about it?

I don't even know what model was supposed to ever use this. IIUC most of that code was for the Intel OpenPIC flavor (whichever one that is). So it's really more about untestable code rather than it being broken.

I'd say if anyone wants to introduce a board that requires these bits, I'm more than happy to see them added back in in a properly modeled way :)


Alex

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [Qemu-devel] [PATCH 01/19] openpic: Remove unused code
  2012-12-08 15:12   ` Andreas Färber
  2012-12-08 15:14     ` Alexander Graf
@ 2012-12-08 17:06     ` Hervé Poussineau
  1 sibling, 0 replies; 43+ messages in thread
From: Hervé Poussineau @ 2012-12-08 17:06 UTC (permalink / raw)
  To: Andreas Färber
  Cc: qemu-ppc@nongnu.org List, Alexander Graf, qemu-devel qemu-devel

Andreas Färber a écrit :
> Am 08.12.2012 14:44, schrieb Alexander Graf:
>> The openpic code had a few WIP bits left that nobody reanimated within
>> the last few years. Remove that code.
>>
>> Signed-off-by: Alexander Graf <agraf@suse.de>
> 
> Reviewed-by: Andreas Färber <afaerber@suse.de>
> 
> I'm not aware of any GW80314 users, CC'ing Hervé just to be sure.
> The BeBox has an i82378 (sitting on its MPC105), using i8259 PICs.
> 
> But even if there were something using them, this is most definitely the
> wrong way to model variations nowadays. :)
> 
> Alex, did you find out what was broken about it?
> 
> Andreas
> 

Acked-by: Hervé Poussineau <hpoussin@reactos.org>

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [Qemu-devel] [PATCH 19/19] MSI-X: Fix endianness
  2012-12-08 13:44 ` [Qemu-devel] [PATCH 19/19] MSI-X: Fix endianness Alexander Graf
@ 2012-12-08 22:41   ` Michael S. Tsirkin
  0 siblings, 0 replies; 43+ messages in thread
From: Michael S. Tsirkin @ 2012-12-08 22:41 UTC (permalink / raw)
  To: Alexander Graf; +Cc: qemu-ppc@nongnu.org List, qemu-devel qemu-devel

On Sat, Dec 08, 2012 at 02:44:42PM +0100, Alexander Graf wrote:
> The MSI-X vector tables are usually stored in little endian in memory,
> so let's mark the accessors as such.
> 
> This fixes MSI-X on e500 for me.
> 
> Signed-off-by: Alexander Graf <agraf@suse.de>
> CC: Michael S. Tsirkin <mst@redhat.com>

Acked-by: Michael S. Tsirkin <mst@redhat.com>

> ---
>  hw/msix.c |    6 ++----
>  1 files changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/msix.c b/hw/msix.c
> index 136ef09..b57ae60 100644
> --- a/hw/msix.c
> +++ b/hw/msix.c
> @@ -180,8 +180,7 @@ static void msix_table_mmio_write(void *opaque, hwaddr addr,
>  static const MemoryRegionOps msix_table_mmio_ops = {
>      .read = msix_table_mmio_read,
>      .write = msix_table_mmio_write,
> -    /* TODO: MSIX should be LITTLE_ENDIAN. */
> -    .endianness = DEVICE_NATIVE_ENDIAN,
> +    .endianness = DEVICE_LITTLE_ENDIAN,
>      .valid = {
>          .min_access_size = 4,
>          .max_access_size = 4,
> @@ -198,8 +197,7 @@ static uint64_t msix_pba_mmio_read(void *opaque, hwaddr addr,
>  
>  static const MemoryRegionOps msix_pba_mmio_ops = {
>      .read = msix_pba_mmio_read,
> -    /* TODO: MSIX should be LITTLE_ENDIAN. */
> -    .endianness = DEVICE_NATIVE_ENDIAN,
> +    .endianness = DEVICE_LITTLE_ENDIAN,
>      .valid = {
>          .min_access_size = 4,
>          .max_access_size = 4,
> -- 
> 1.6.0.2

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [Qemu-devel] [Qemu-ppc] [PATCH 02/19] mpic: Unify numbering scheme
  2012-12-08 13:44 ` [Qemu-devel] [PATCH 02/19] mpic: Unify numbering scheme Alexander Graf
@ 2012-12-10 23:34   ` Scott Wood
  2012-12-10 23:40     ` Scott Wood
  2012-12-11  8:14     ` Alexander Graf
  0 siblings, 2 replies; 43+ messages in thread
From: Scott Wood @ 2012-12-10 23:34 UTC (permalink / raw)
  To: Alexander Graf; +Cc: qemu-ppc@nongnu.org List, qemu-devel qemu-devel

On 12/08/2012 07:44:25 AM, Alexander Graf wrote:
> MPIC interrupt numbers in Linux (device tree) and in QEMU are  
> different,
> because QEMU takes the sparseness of the IRQ number space into  
> account.
> 
> Remove that cleverness and instead assume a flat number space. This  
> makes
> the code easier to understand, because we are actually aligned with  
> Linux
> on the view of our worlds.
> 
> Signed-off-by: Alexander Graf <agraf@suse.de>
> ---
>  hw/openpic.c  |  287  
> ++++++++------------------------------------------------
>  hw/ppc/e500.c |    4 +-
>  2 files changed, 43 insertions(+), 248 deletions(-)

Thanks!

This also helps accommodate Linux's (and probably other OSes') MPIC
driver, which on boot will initialize all vectors, whether the actual
interrupt is valid or not.  Currently QEMU rejects those accesses as
illegal MMIO; we only survive it now because we don't yet have support  
for
injecting a machine check on e500.

> diff --git a/hw/openpic.c b/hw/openpic.c
> index b30c853..0d858cc 100644
> --- a/hw/openpic.c
> +++ b/hw/openpic.c
> @@ -47,7 +47,7 @@
>  #endif
> 
>  #define MAX_CPU    15
> -#define MAX_IRQ   128
> +#define MAX_IRQ   256
>  #define MAX_TMR     4
>  #define VECTOR_BITS 8
>  #define MAX_IPI     4
> @@ -82,32 +82,25 @@ enum {
>  #define MPIC_MAX_CPU      1
>  #define MPIC_MAX_EXT     12
>  #define MPIC_MAX_INT     64
> -#define MPIC_MAX_MSG      4
> -#define MPIC_MAX_MSI      8
> -#define MPIC_MAX_TMR      MAX_TMR
> -#define MPIC_MAX_IPI      MAX_IPI
> -#define MPIC_MAX_IRQ     (MPIC_MAX_EXT + MPIC_MAX_INT + MPIC_MAX_TMR  
> + MPIC_MAX_MSG + MPIC_MAX_MSI + (MPIC_MAX_IPI * MPIC_MAX_CPU))
> +#define MPIC_MAX_IRQ     MAX_IRQ
> 
>  /* Interrupt definitions */
> -#define MPIC_EXT_IRQ      0
> -#define MPIC_INT_IRQ      (MPIC_EXT_IRQ + MPIC_MAX_EXT)
> -#define MPIC_TMR_IRQ      (MPIC_INT_IRQ + MPIC_MAX_INT)
> -#define MPIC_MSG_IRQ      (MPIC_TMR_IRQ + MPIC_MAX_TMR)
> -#define MPIC_MSI_IRQ      (MPIC_MSG_IRQ + MPIC_MAX_MSG)
> -#define MPIC_IPI_IRQ      (MPIC_MSI_IRQ + MPIC_MAX_MSI)
> +/* IRQs, accessible through the IRQ region */
> +#define MPIC_EXT_IRQ      0x00
> +#define MPIC_INT_IRQ      0x10
> +#define MPIC_MSG_IRQ      0xb0
> +#define MPIC_MSI_IRQ      0xe0

Where are MPIC_EXT/INT/MSG/MSI_IRQ used now?  Note that these are
specific to Freescale's MPIC.

> +/* These are available through separate regions, but
> +   for simplicity's sake mapped into the same number space */
> +#define MPIC_TMR_IRQ      0xf3
> +#define MPIC_IPI_IRQ      0xfb

Please don't do this, or at least choose different numbers.  0xf3 is a
valid MSI on p4080 (not to mention T4240 which goes beyond 256).

Again, what uses these defines?  Is it something later in the series?

-Scott

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [Qemu-devel] [Qemu-ppc] [PATCH 02/19] mpic: Unify numbering scheme
  2012-12-10 23:34   ` [Qemu-devel] [Qemu-ppc] " Scott Wood
@ 2012-12-10 23:40     ` Scott Wood
  2012-12-11  8:14     ` Alexander Graf
  1 sibling, 0 replies; 43+ messages in thread
From: Scott Wood @ 2012-12-10 23:40 UTC (permalink / raw)
  To: Scott Wood
  Cc: qemu-ppc@nongnu.org List, Alexander Graf, qemu-devel qemu-devel

On 12/10/2012 05:34:19 PM, Scott Wood wrote:
> On 12/08/2012 07:44:25 AM, Alexander Graf wrote:
>>  /* Interrupt definitions */
>> -#define MPIC_EXT_IRQ      0
>> -#define MPIC_INT_IRQ      (MPIC_EXT_IRQ + MPIC_MAX_EXT)
>> -#define MPIC_TMR_IRQ      (MPIC_INT_IRQ + MPIC_MAX_INT)
>> -#define MPIC_MSG_IRQ      (MPIC_TMR_IRQ + MPIC_MAX_TMR)
>> -#define MPIC_MSI_IRQ      (MPIC_MSG_IRQ + MPIC_MAX_MSG)
>> -#define MPIC_IPI_IRQ      (MPIC_MSI_IRQ + MPIC_MAX_MSI)
>> +/* IRQs, accessible through the IRQ region */
>> +#define MPIC_EXT_IRQ      0x00
>> +#define MPIC_INT_IRQ      0x10
>> +#define MPIC_MSG_IRQ      0xb0
>> +#define MPIC_MSI_IRQ      0xe0
> 
> Where are MPIC_EXT/INT/MSG/MSI_IRQ used now?  Note that these are
> specific to Freescale's MPIC.
> 
>> +/* These are available through separate regions, but
>> +   for simplicity's sake mapped into the same number space */
>> +#define MPIC_TMR_IRQ      0xf3
>> +#define MPIC_IPI_IRQ      0xfb
> 
> Please don't do this, or at least choose different numbers.  0xf3 is a
> valid MSI on p4080 (not to mention T4240 which goes beyond 256).
> 
> Again, what uses these defines?  Is it something later in the series?

OK, the TMR/IPI are used by existing code that this patch doesn't  
remove, but I think that's not the case with EXT/INT/MSG/MSI.

-Scott

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [Qemu-devel] [Qemu-ppc] [PATCH 10/19] openpic: remove unused type variable
  2012-12-08 13:44 ` [Qemu-devel] [PATCH 10/19] openpic: remove unused type variable Alexander Graf
@ 2012-12-10 23:42   ` Scott Wood
  2012-12-11  8:17     ` Alexander Graf
  0 siblings, 1 reply; 43+ messages in thread
From: Scott Wood @ 2012-12-10 23:42 UTC (permalink / raw)
  To: Alexander Graf; +Cc: qemu-ppc@nongnu.org List, qemu-devel qemu-devel

On 12/08/2012 07:44:33 AM, Alexander Graf wrote:
> The openpic source irqs are carrying around a type indicator that
> is never accessed by anything. Remove it.
> 
> Signed-off-by: Alexander Graf <agraf@suse.de>
> ---
>  hw/openpic.c |   27 ++-------------------------
>  1 files changed, 2 insertions(+), 25 deletions(-)
> 
> diff --git a/hw/openpic.c b/hw/openpic.c
> index e4ef23d..d252b2b 100644
> --- a/hw/openpic.c
> +++ b/hw/openpic.c
> @@ -167,13 +167,6 @@ static uint32_t openpic_cpu_read_internal(void  
> *opaque, hwaddr addr,
>  static void openpic_cpu_write_internal(void *opaque, hwaddr addr,
>                                         uint32_t val, int idx);
> 
> -enum {
> -    IRQ_EXTERNAL = 0x01,
> -    IRQ_INTERNAL = 0x02,
> -    IRQ_TIMER    = 0x04,
> -    IRQ_SPECIAL  = 0x08,
> -};

We may want to distinguish based on something like this in the future  
-- for example, internal interrupts on FSL MPIC don't have a "sense"  
bit.

-Scott

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [Qemu-devel] [PATCH 14/19] openpic: convert to qdev
  2012-12-08 13:44 ` [Qemu-devel] [PATCH 14/19] openpic: convert to qdev Alexander Graf
@ 2012-12-10 23:47   ` Scott Wood
  2012-12-11  8:25     ` Alexander Graf
  0 siblings, 1 reply; 43+ messages in thread
From: Scott Wood @ 2012-12-10 23:47 UTC (permalink / raw)
  To: Alexander Graf; +Cc: qemu-ppc@nongnu.org List, qemu-devel qemu-devel

On 12/08/2012 07:44:37 AM, Alexander Graf wrote:
> This patch converts the OpenPIC device to qdev. Along the way it
> renames the "openpic" target to "raven" and the "mpic" target to
> "mpc8544", to better reflect the actual models they implement.
> 
> This way we have a generic OpenPIC device now that can handle
> different flavors of the OpenPIC specification.

I'd rather not see the expansion of "mpc8544" hardcoding, especially  
since it's not really modelling an MPIC as found in an mpc8544, but  
rather half-implementing a generic Freescale MPIC.  Can we just call it  
a Freescale MPIC, and then work on parameterizing it properly (starting  
with MPIC version)?

-Scott

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [Qemu-devel] [Qemu-ppc] [PATCH 16/19] openpic: add Shared MSI support
  2012-12-08 13:44 ` [Qemu-devel] [PATCH 16/19] openpic: add Shared MSI support Alexander Graf
@ 2012-12-11  0:36   ` Scott Wood
  2012-12-11  8:10     ` Alexander Graf
  0 siblings, 1 reply; 43+ messages in thread
From: Scott Wood @ 2012-12-11  0:36 UTC (permalink / raw)
  To: Alexander Graf; +Cc: qemu-ppc@nongnu.org List, qemu-devel qemu-devel

On 12/08/2012 07:44:39 AM, Alexander Graf wrote:
> The OpenPIC allows MSI access through shared MSI registers. Implement
> them for the MPC8544 MPIC, so we can support MSIs.
> 
> Signed-off-by: Alexander Graf <agraf@suse.de>
> ---
>  hw/openpic.c |  150  
> ++++++++++++++++++++++++++++++++++++++++++++++++++--------
>  1 files changed, 130 insertions(+), 20 deletions(-)
> 
> diff --git a/hw/openpic.c b/hw/openpic.c
> index f2f152f..f71d668 100644
> --- a/hw/openpic.c
> +++ b/hw/openpic.c
> @@ -38,6 +38,7 @@
>  #include "pci.h"
>  #include "openpic.h"
>  #include "sysbus.h"
> +#include "msi.h"
> 
>  //#define DEBUG_OPENPIC
> 
> @@ -52,6 +53,7 @@
>  #define MAX_TMR     4
>  #define VECTOR_BITS 8
>  #define MAX_IPI     4
> +#define MAX_MSI     8
>  #define VID         0x03 /* MPIC version ID */
> 
>  /* OpenPIC capability flags */
> @@ -62,6 +64,8 @@
>  #define OPENPIC_GLB_REG_SIZE         0x10F0
>  #define OPENPIC_TMR_REG_START        0x10F0
>  #define OPENPIC_TMR_REG_SIZE         0x220
> +#define OPENPIC_MSI_REG_START        0x1600
> +#define OPENPIC_MSI_REG_SIZE         0x200
>  #define OPENPIC_SRC_REG_START        0x10000
>  #define OPENPIC_SRC_REG_SIZE         (MAX_IRQ * 0x20)
>  #define OPENPIC_CPU_REG_START        0x20000
> @@ -126,6 +130,12 @@
>  #define IDR_P1_SHIFT      1
>  #define IDR_P0_SHIFT      0
> 
> +#define MSIIR_OFFSET       0x140
> +#define MSIIR_SRS_SHIFT    29
> +#define MSIIR_SRS_MASK     (0x7 << MSIIR_SRS_SHIFT)
> +#define MSIIR_IBS_SHIFT    24
> +#define MSIIR_IBS_MASK     (0x1f << MSIIR_IBS_SHIFT)

FWIW, if you want to model newer MPICs such as on p4080, they have  
multiple banks of MSIs, so you may not want to hardcode one bank.

-Scott

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [Qemu-devel] [Qemu-ppc] [PATCH 16/19] openpic: add Shared MSI support
  2012-12-11  0:36   ` [Qemu-devel] [Qemu-ppc] " Scott Wood
@ 2012-12-11  8:10     ` Alexander Graf
  2012-12-11 17:35       ` Scott Wood
  0 siblings, 1 reply; 43+ messages in thread
From: Alexander Graf @ 2012-12-11  8:10 UTC (permalink / raw)
  To: Scott Wood; +Cc: qemu-ppc@nongnu.org List, qemu-devel qemu-devel



On 11.12.2012, at 01:36, Scott Wood <scottwood@freescale.com> wrote:

> On 12/08/2012 07:44:39 AM, Alexander Graf wrote:
>> The OpenPIC allows MSI access through shared MSI registers. Implement
>> them for the MPC8544 MPIC, so we can support MSIs.
>> Signed-off-by: Alexander Graf <agraf@suse.de>
>> ---
>> hw/openpic.c |  150 ++++++++++++++++++++++++++++++++++++++++++++++++++--------
>> 1 files changed, 130 insertions(+), 20 deletions(-)
>> diff --git a/hw/openpic.c b/hw/openpic.c
>> index f2f152f..f71d668 100644
>> --- a/hw/openpic.c
>> +++ b/hw/openpic.c
>> @@ -38,6 +38,7 @@
>> #include "pci.h"
>> #include "openpic.h"
>> #include "sysbus.h"
>> +#include "msi.h"
>> //#define DEBUG_OPENPIC
>> @@ -52,6 +53,7 @@
>> #define MAX_TMR     4
>> #define VECTOR_BITS 8
>> #define MAX_IPI     4
>> +#define MAX_MSI     8
>> #define VID         0x03 /* MPIC version ID */
>> /* OpenPIC capability flags */
>> @@ -62,6 +64,8 @@
>> #define OPENPIC_GLB_REG_SIZE         0x10F0
>> #define OPENPIC_TMR_REG_START        0x10F0
>> #define OPENPIC_TMR_REG_SIZE         0x220
>> +#define OPENPIC_MSI_REG_START        0x1600
>> +#define OPENPIC_MSI_REG_SIZE         0x200
>> #define OPENPIC_SRC_REG_START        0x10000
>> #define OPENPIC_SRC_REG_SIZE         (MAX_IRQ * 0x20)
>> #define OPENPIC_CPU_REG_START        0x20000
>> @@ -126,6 +130,12 @@
>> #define IDR_P1_SHIFT      1
>> #define IDR_P0_SHIFT      0
>> +#define MSIIR_OFFSET       0x140
>> +#define MSIIR_SRS_SHIFT    29
>> +#define MSIIR_SRS_MASK     (0x7 << MSIIR_SRS_SHIFT)
>> +#define MSIIR_IBS_SHIFT    24
>> +#define MSIIR_IBS_MASK     (0x1f << MSIIR_IBS_SHIFT)
> 
> FWIW, if you want to model newer MPICs such as on p4080, they have multiple banks of MSIs, so you may not want to hardcode one bank.

The OpenPIC code was suffering a lot from attempts to generalize different implementations without implementing them.

If we want to add support for p4080 MPICs later, we add a new model to the emulation and make the nr of msi banks a parameter, like the patch set does for all the other raven/mpc8544 differences. That way we don't get into the current mess of a halfway accurate emulation unless we really want it.


Alex

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [Qemu-devel] [Qemu-ppc] [PATCH 02/19] mpic: Unify numbering scheme
  2012-12-10 23:34   ` [Qemu-devel] [Qemu-ppc] " Scott Wood
  2012-12-10 23:40     ` Scott Wood
@ 2012-12-11  8:14     ` Alexander Graf
  2012-12-11 17:39       ` Scott Wood
  1 sibling, 1 reply; 43+ messages in thread
From: Alexander Graf @ 2012-12-11  8:14 UTC (permalink / raw)
  To: Scott Wood; +Cc: qemu-ppc@nongnu.org List, qemu-devel qemu-devel



On 11.12.2012, at 00:34, Scott Wood <scottwood@freescale.com> wrote:

> On 12/08/2012 07:44:25 AM, Alexander Graf wrote:
>> MPIC interrupt numbers in Linux (device tree) and in QEMU are different,
>> because QEMU takes the sparseness of the IRQ number space into account.
>> Remove that cleverness and instead assume a flat number space. This makes
>> the code easier to understand, because we are actually aligned with Linux
>> on the view of our worlds.
>> Signed-off-by: Alexander Graf <agraf@suse.de>
>> ---
>> hw/openpic.c  |  287 ++++++++------------------------------------------------
>> hw/ppc/e500.c |    4 +-
>> 2 files changed, 43 insertions(+), 248 deletions(-)
> 
> Thanks!
> 
> This also helps accommodate Linux's (and probably other OSes') MPIC
> driver, which on boot will initialize all vectors, whether the actual
> interrupt is valid or not.  Currently QEMU rejects those accesses as
> illegal MMIO; we only survive it now because we don't yet have support for
> injecting a machine check on e500.
> 
>> diff --git a/hw/openpic.c b/hw/openpic.c
>> index b30c853..0d858cc 100644
>> --- a/hw/openpic.c
>> +++ b/hw/openpic.c
>> @@ -47,7 +47,7 @@
>> #endif
>> #define MAX_CPU    15
>> -#define MAX_IRQ   128
>> +#define MAX_IRQ   256
>> #define MAX_TMR     4
>> #define VECTOR_BITS 8
>> #define MAX_IPI     4
>> @@ -82,32 +82,25 @@ enum {
>> #define MPIC_MAX_CPU      1
>> #define MPIC_MAX_EXT     12
>> #define MPIC_MAX_INT     64
>> -#define MPIC_MAX_MSG      4
>> -#define MPIC_MAX_MSI      8
>> -#define MPIC_MAX_TMR      MAX_TMR
>> -#define MPIC_MAX_IPI      MAX_IPI
>> -#define MPIC_MAX_IRQ     (MPIC_MAX_EXT + MPIC_MAX_INT + MPIC_MAX_TMR + MPIC_MAX_MSG + MPIC_MAX_MSI + (MPIC_MAX_IPI * MPIC_MAX_CPU))
>> +#define MPIC_MAX_IRQ     MAX_IRQ
>> /* Interrupt definitions */
>> -#define MPIC_EXT_IRQ      0
>> -#define MPIC_INT_IRQ      (MPIC_EXT_IRQ + MPIC_MAX_EXT)
>> -#define MPIC_TMR_IRQ      (MPIC_INT_IRQ + MPIC_MAX_INT)
>> -#define MPIC_MSG_IRQ      (MPIC_TMR_IRQ + MPIC_MAX_TMR)
>> -#define MPIC_MSI_IRQ      (MPIC_MSG_IRQ + MPIC_MAX_MSG)
>> -#define MPIC_IPI_IRQ      (MPIC_MSI_IRQ + MPIC_MAX_MSI)
>> +/* IRQs, accessible through the IRQ region */
>> +#define MPIC_EXT_IRQ      0x00
>> +#define MPIC_INT_IRQ      0x10
>> +#define MPIC_MSG_IRQ      0xb0
>> +#define MPIC_MSI_IRQ      0xe0
> 
> Where are MPIC_EXT/INT/MSG/MSI_IRQ used now?  Note that these are
> specific to Freescale's MPIC.
> 
>> +/* These are available through separate regions, but
>> +   for simplicity's sake mapped into the same number space */
>> +#define MPIC_TMR_IRQ      0xf3
>> +#define MPIC_IPI_IRQ      0xfb
> 
> Please don't do this, or at least choose different numbers.  0xf3 is a
> valid MSI on p4080 (not to mention T4240 which goes beyond 256).

Ah, that's where I was wondering myself too. I copied the above logic from Linux, which maps tmr and ipi to max_irq-x. But I agree that it sounds off.

I'll rework this one again to distinguish between src and internal interrupt lines.


Alex

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [Qemu-devel] [Qemu-ppc] [PATCH 10/19] openpic: remove unused type variable
  2012-12-10 23:42   ` [Qemu-devel] [Qemu-ppc] " Scott Wood
@ 2012-12-11  8:17     ` Alexander Graf
  0 siblings, 0 replies; 43+ messages in thread
From: Alexander Graf @ 2012-12-11  8:17 UTC (permalink / raw)
  To: Scott Wood; +Cc: qemu-ppc@nongnu.org List, qemu-devel qemu-devel



On 11.12.2012, at 00:42, Scott Wood <scottwood@freescale.com> wrote:

> On 12/08/2012 07:44:33 AM, Alexander Graf wrote:
>> The openpic source irqs are carrying around a type indicator that
>> is never accessed by anything. Remove it.
>> Signed-off-by: Alexander Graf <agraf@suse.de>
>> ---
>> hw/openpic.c |   27 ++-------------------------
>> 1 files changed, 2 insertions(+), 25 deletions(-)
>> diff --git a/hw/openpic.c b/hw/openpic.c
>> index e4ef23d..d252b2b 100644
>> --- a/hw/openpic.c
>> +++ b/hw/openpic.c
>> @@ -167,13 +167,6 @@ static uint32_t openpic_cpu_read_internal(void *opaque, hwaddr addr,
>> static void openpic_cpu_write_internal(void *opaque, hwaddr addr,
>>                                        uint32_t val, int idx);
>> -enum {
>> -    IRQ_EXTERNAL = 0x01,
>> -    IRQ_INTERNAL = 0x02,
>> -    IRQ_TIMER    = 0x04,
>> -    IRQ_SPECIAL  = 0x08,
>> -};
> 
> We may want to distinguish based on something like this in the future -- for example, internal interrupts on FSL MPIC don't have a "sense" bit.

I would rather like to give irq lines flags than types then. An FSL MPIC could set a 'has no sense bit' flag in the irq line which means the sense bit is always masked out when written to.


Alex

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [Qemu-devel] [PATCH 14/19] openpic: convert to qdev
  2012-12-10 23:47   ` Scott Wood
@ 2012-12-11  8:25     ` Alexander Graf
  2012-12-11 17:47       ` Scott Wood
  0 siblings, 1 reply; 43+ messages in thread
From: Alexander Graf @ 2012-12-11  8:25 UTC (permalink / raw)
  To: Scott Wood; +Cc: qemu-ppc@nongnu.org List, qemu-devel qemu-devel



On 11.12.2012, at 00:47, Scott Wood <scottwood@freescale.com> wrote:

> On 12/08/2012 07:44:37 AM, Alexander Graf wrote:
>> This patch converts the OpenPIC device to qdev. Along the way it
>> renames the "openpic" target to "raven" and the "mpic" target to
>> "mpc8544", to better reflect the actual models they implement.
>> This way we have a generic OpenPIC device now that can handle
>> different flavors of the OpenPIC specification.
> 
> I'd rather not see the expansion of "mpc8544" hardcoding, especially since it's not really modelling an MPIC as found in an mpc8544, but rather half-implementing a generic Freescale MPIC.  

That's what we've been doing wrong all the time now. We shouldn't implement a half-generic fsl mpic, because we will never get to a point where we're accurate enough or flexible enough for both definitions.

If we want a pv style generic mpic (for -M e500), let's add such an mpic to the model list and make that one be really generic. But the MPIC in -M mpc8544ds should behave exactly like an mpc8544 mpic. Whenever we fail to do so, we better fix the emulation to be accurate ;)

> Can we just call it a Freescale MPIC, and then work on parameterizing it properly (starting with MPIC version)?

We can add a generic fsl mpic if you like. We can also extract commonalities between the generic fsl mpic and the mpc8544 mpic into a function inside openpic.c.

The MPIC version is a parameter after this patch set, but it's an internal parameter. My first version of the qdevification was setting all the openpic parameters from the machine file, so that you could assemble whatever openpic you like by setting the currect parameters.

That approach is overkill though. We are working in an open source world, where you can always just modify openpic.c, addd your model and be happy :)

Thanks a lot for the review btw!


Alex

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [Qemu-devel] [Qemu-ppc] [PATCH 16/19] openpic: add Shared MSI support
  2012-12-11  8:10     ` Alexander Graf
@ 2012-12-11 17:35       ` Scott Wood
  2012-12-12  0:53         ` Alexander Graf
  0 siblings, 1 reply; 43+ messages in thread
From: Scott Wood @ 2012-12-11 17:35 UTC (permalink / raw)
  To: Alexander Graf; +Cc: qemu-ppc@nongnu.org List, qemu-devel qemu-devel

On 12/11/2012 02:10:14 AM, Alexander Graf wrote:
> 
> 
> On 11.12.2012, at 01:36, Scott Wood <scottwood@freescale.com> wrote:
> 
> > On 12/08/2012 07:44:39 AM, Alexander Graf wrote:
> >> The OpenPIC allows MSI access through shared MSI registers.  
> Implement
> >> them for the MPC8544 MPIC, so we can support MSIs.
> >> Signed-off-by: Alexander Graf <agraf@suse.de>
> >> ---
> >> hw/openpic.c |  150  
> ++++++++++++++++++++++++++++++++++++++++++++++++++--------
> >> 1 files changed, 130 insertions(+), 20 deletions(-)
> >> diff --git a/hw/openpic.c b/hw/openpic.c
> >> index f2f152f..f71d668 100644
> >> --- a/hw/openpic.c
> >> +++ b/hw/openpic.c
> >> @@ -38,6 +38,7 @@
> >> #include "pci.h"
> >> #include "openpic.h"
> >> #include "sysbus.h"
> >> +#include "msi.h"
> >> //#define DEBUG_OPENPIC
> >> @@ -52,6 +53,7 @@
> >> #define MAX_TMR     4
> >> #define VECTOR_BITS 8
> >> #define MAX_IPI     4
> >> +#define MAX_MSI     8
> >> #define VID         0x03 /* MPIC version ID */
> >> /* OpenPIC capability flags */
> >> @@ -62,6 +64,8 @@
> >> #define OPENPIC_GLB_REG_SIZE         0x10F0
> >> #define OPENPIC_TMR_REG_START        0x10F0
> >> #define OPENPIC_TMR_REG_SIZE         0x220
> >> +#define OPENPIC_MSI_REG_START        0x1600
> >> +#define OPENPIC_MSI_REG_SIZE         0x200
> >> #define OPENPIC_SRC_REG_START        0x10000
> >> #define OPENPIC_SRC_REG_SIZE         (MAX_IRQ * 0x20)
> >> #define OPENPIC_CPU_REG_START        0x20000
> >> @@ -126,6 +130,12 @@
> >> #define IDR_P1_SHIFT      1
> >> #define IDR_P0_SHIFT      0
> >> +#define MSIIR_OFFSET       0x140
> >> +#define MSIIR_SRS_SHIFT    29
> >> +#define MSIIR_SRS_MASK     (0x7 << MSIIR_SRS_SHIFT)
> >> +#define MSIIR_IBS_SHIFT    24
> >> +#define MSIIR_IBS_MASK     (0x1f << MSIIR_IBS_SHIFT)
> >
> > FWIW, if you want to model newer MPICs such as on p4080, they have  
> multiple banks of MSIs, so you may not want to hardcode one bank.
> 
> The OpenPIC code was suffering a lot from attempts to generalize  
> different implementations without implementing them.
> 
> If we want to add support for p4080 MPICs later, we add a new model  
> to the emulation and make the nr of msi banks a parameter, like the  
> patch set does for all the other raven/mpc8544 differences. That way  
> we don't get into the current mess of a halfway accurate emulation  
> unless we really want it.

So because the old code made a mess of it, we're saying "abstraction is  
bad" in general?

All I'm saying is this should be done with a runtime data structure (of  
which there could be more than one) rather than #defines.

-Scott

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [Qemu-devel] [Qemu-ppc] [PATCH 02/19] mpic: Unify numbering scheme
  2012-12-11  8:14     ` Alexander Graf
@ 2012-12-11 17:39       ` Scott Wood
  0 siblings, 0 replies; 43+ messages in thread
From: Scott Wood @ 2012-12-11 17:39 UTC (permalink / raw)
  To: Alexander Graf; +Cc: qemu-ppc@nongnu.org List, qemu-devel qemu-devel

On 12/11/2012 02:14:42 AM, Alexander Graf wrote:
> 
> 
> On 11.12.2012, at 00:34, Scott Wood <scottwood@freescale.com> wrote:
> 
> > On 12/08/2012 07:44:25 AM, Alexander Graf wrote:
> >> +/* These are available through separate regions, but
> >> +   for simplicity's sake mapped into the same number space */
> >> +#define MPIC_TMR_IRQ      0xf3
> >> +#define MPIC_IPI_IRQ      0xfb
> >
> > Please don't do this, or at least choose different numbers.  0xf3  
> is a
> > valid MSI on p4080 (not to mention T4240 which goes beyond 256).
> 
> Ah, that's where I was wondering myself too. I copied the above logic  
> from Linux, which maps tmr and ipi to max_irq-x. But I agree that it  
> sounds off.

Yeah, Linux was buggy and has since been fixed (specifically, we turned  
on MPIC_LARGE_VECTORS so that max_irq is large enough to not conflict).

-Scott

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [Qemu-devel] [PATCH 14/19] openpic: convert to qdev
  2012-12-11  8:25     ` Alexander Graf
@ 2012-12-11 17:47       ` Scott Wood
  2012-12-12  0:56         ` Alexander Graf
  0 siblings, 1 reply; 43+ messages in thread
From: Scott Wood @ 2012-12-11 17:47 UTC (permalink / raw)
  To: Alexander Graf; +Cc: qemu-ppc@nongnu.org List, qemu-devel qemu-devel

On 12/11/2012 02:25:31 AM, Alexander Graf wrote:
> 
> 
> On 11.12.2012, at 00:47, Scott Wood <scottwood@freescale.com> wrote:
> 
> > On 12/08/2012 07:44:37 AM, Alexander Graf wrote:
> >> This patch converts the OpenPIC device to qdev. Along the way it
> >> renames the "openpic" target to "raven" and the "mpic" target to
> >> "mpc8544", to better reflect the actual models they implement.
> >> This way we have a generic OpenPIC device now that can handle
> >> different flavors of the OpenPIC specification.
> >
> > I'd rather not see the expansion of "mpc8544" hardcoding,  
> especially since it's not really modelling an MPIC as found in an  
> mpc8544, but rather half-implementing a generic Freescale MPIC.
> 
> That's what we've been doing wrong all the time now. We shouldn't  
> implement a half-generic fsl mpic, because we will never get to a  
> point where we're accurate enough or flexible enough for both  
> definitions.

What do you mean by "both"?  AFAICT we'll always have a half-generic  
MPIC because we'll probably never get it looking 100% like any  
particular chip.

> If we want a pv style generic mpic (for -M e500), let's add such an  
> mpic to the model list and make that one be really generic. But the  
> MPIC in -M mpc8544ds should behave exactly like an mpc8544 mpic.  
> Whenever we fail to do so, we better fix the emulation to be accurate  
> ;)

What behaviors would "mpc8544" specify that "fsl mpic v2.0" would not?

-Scott

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [Qemu-devel] [Qemu-ppc] [PATCH 16/19] openpic: add Shared MSI support
  2012-12-11 17:35       ` Scott Wood
@ 2012-12-12  0:53         ` Alexander Graf
  2012-12-12  1:42           ` Scott Wood
  0 siblings, 1 reply; 43+ messages in thread
From: Alexander Graf @ 2012-12-12  0:53 UTC (permalink / raw)
  To: Scott Wood; +Cc: qemu-ppc@nongnu.org List, qemu-devel qemu-devel


On 11.12.2012, at 18:35, Scott Wood wrote:

> On 12/11/2012 02:10:14 AM, Alexander Graf wrote:
>> On 11.12.2012, at 01:36, Scott Wood <scottwood@freescale.com> wrote:
>> > On 12/08/2012 07:44:39 AM, Alexander Graf wrote:
>> >> The OpenPIC allows MSI access through shared MSI registers. Implement
>> >> them for the MPC8544 MPIC, so we can support MSIs.
>> >> Signed-off-by: Alexander Graf <agraf@suse.de>
>> >> ---
>> >> hw/openpic.c |  150 ++++++++++++++++++++++++++++++++++++++++++++++++++--------
>> >> 1 files changed, 130 insertions(+), 20 deletions(-)
>> >> diff --git a/hw/openpic.c b/hw/openpic.c
>> >> index f2f152f..f71d668 100644
>> >> --- a/hw/openpic.c
>> >> +++ b/hw/openpic.c
>> >> @@ -38,6 +38,7 @@
>> >> #include "pci.h"
>> >> #include "openpic.h"
>> >> #include "sysbus.h"
>> >> +#include "msi.h"
>> >> //#define DEBUG_OPENPIC
>> >> @@ -52,6 +53,7 @@
>> >> #define MAX_TMR     4
>> >> #define VECTOR_BITS 8
>> >> #define MAX_IPI     4
>> >> +#define MAX_MSI     8
>> >> #define VID         0x03 /* MPIC version ID */
>> >> /* OpenPIC capability flags */
>> >> @@ -62,6 +64,8 @@
>> >> #define OPENPIC_GLB_REG_SIZE         0x10F0
>> >> #define OPENPIC_TMR_REG_START        0x10F0
>> >> #define OPENPIC_TMR_REG_SIZE         0x220
>> >> +#define OPENPIC_MSI_REG_START        0x1600
>> >> +#define OPENPIC_MSI_REG_SIZE         0x200
>> >> #define OPENPIC_SRC_REG_START        0x10000
>> >> #define OPENPIC_SRC_REG_SIZE         (MAX_IRQ * 0x20)
>> >> #define OPENPIC_CPU_REG_START        0x20000
>> >> @@ -126,6 +130,12 @@
>> >> #define IDR_P1_SHIFT      1
>> >> #define IDR_P0_SHIFT      0
>> >> +#define MSIIR_OFFSET       0x140
>> >> +#define MSIIR_SRS_SHIFT    29
>> >> +#define MSIIR_SRS_MASK     (0x7 << MSIIR_SRS_SHIFT)
>> >> +#define MSIIR_IBS_SHIFT    24
>> >> +#define MSIIR_IBS_MASK     (0x1f << MSIIR_IBS_SHIFT)
>> >
>> > FWIW, if you want to model newer MPICs such as on p4080, they have multiple banks of MSIs, so you may not want to hardcode one bank.
>> The OpenPIC code was suffering a lot from attempts to generalize different implementations without implementing them.
>> If we want to add support for p4080 MPICs later, we add a new model to the emulation and make the nr of msi banks a parameter, like the patch set does for all the other raven/mpc8544 differences. That way we don't get into the current mess of a halfway accurate emulation unless we really want it.
> 
> So because the old code made a mess of it, we're saying "abstraction is bad" in general?

No, because the old code messed up abstraction, I would like to move to a non-abstract level and then start abstracting at the correct layer. What that correct layer is is still up for discussion :).

> All I'm saying is this should be done with a runtime data structure (of which there could be more than one) rather than #defines.

Yeah, and my argument was that this should be introduced as part of a new model. But if it makes you happy I can of course also make it generic right now, without a user, which means we can't even test whether the generalization works, which means we might screw it up again ;).


Alex

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [Qemu-devel] [PATCH 14/19] openpic: convert to qdev
  2012-12-11 17:47       ` Scott Wood
@ 2012-12-12  0:56         ` Alexander Graf
  2012-12-12  1:38           ` Scott Wood
  0 siblings, 1 reply; 43+ messages in thread
From: Alexander Graf @ 2012-12-12  0:56 UTC (permalink / raw)
  To: Scott Wood; +Cc: qemu-ppc@nongnu.org List, qemu-devel qemu-devel


On 11.12.2012, at 18:47, Scott Wood wrote:

> On 12/11/2012 02:25:31 AM, Alexander Graf wrote:
>> On 11.12.2012, at 00:47, Scott Wood <scottwood@freescale.com> wrote:
>> > On 12/08/2012 07:44:37 AM, Alexander Graf wrote:
>> >> This patch converts the OpenPIC device to qdev. Along the way it
>> >> renames the "openpic" target to "raven" and the "mpic" target to
>> >> "mpc8544", to better reflect the actual models they implement.
>> >> This way we have a generic OpenPIC device now that can handle
>> >> different flavors of the OpenPIC specification.
>> >
>> > I'd rather not see the expansion of "mpc8544" hardcoding, especially since it's not really modelling an MPIC as found in an mpc8544, but rather half-implementing a generic Freescale MPIC.
>> That's what we've been doing wrong all the time now. We shouldn't implement a half-generic fsl mpic, because we will never get to a point where we're accurate enough or flexible enough for both definitions.
> 
> What do you mean by "both"?  AFAICT we'll always have a half-generic MPIC because we'll probably never get it looking 100% like any particular chip.

We should be though. That's the whole point I'm making here.

> 
>> If we want a pv style generic mpic (for -M e500), let's add such an mpic to the model list and make that one be really generic. But the MPIC in -M mpc8544ds should behave exactly like an mpc8544 mpic. Whenever we fail to do so, we better fix the emulation to be accurate ;)
> 
> What behaviors would "mpc8544" specify that "fsl mpic v2.0" would not?

I don't know. If you say that mpc8544 == "fsl mpic v2.0" I'm more than happy to rename what we have. Simply calling it "MPIC" was definitely wrong, so I want with the one where I'm actually sure that what I'm implementing is correct, because I have the spec in front of me.

My general approach to this problem would be that we for example get a p4080 board once. Once we get that, we want a p4080 MPIC. Then you'd sit down and model the p4080 MPIC. You realize that it's identical to the mpc8544 MPIC. So you either choose to instantiate an MPC8544 MPIC or you rename the model name to "fsl mpic v2.0".

If you can assure me today that they will be identical, I'm more than happy to change the name today already :).


Alex

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [Qemu-devel] [PATCH 14/19] openpic: convert to qdev
  2012-12-12  0:56         ` Alexander Graf
@ 2012-12-12  1:38           ` Scott Wood
  2012-12-12 10:37             ` Alexander Graf
  0 siblings, 1 reply; 43+ messages in thread
From: Scott Wood @ 2012-12-12  1:38 UTC (permalink / raw)
  To: Alexander Graf; +Cc: qemu-ppc@nongnu.org List, qemu-devel qemu-devel

On 12/11/2012 06:56:56 PM, Alexander Graf wrote:
> 
> On 11.12.2012, at 18:47, Scott Wood wrote:
> 
> > On 12/11/2012 02:25:31 AM, Alexander Graf wrote:
> >> If we want a pv style generic mpic (for -M e500), let's add such  
> an mpic to the model list and make that one be really generic. But  
> the MPIC in -M mpc8544ds should behave exactly like an mpc8544 mpic.  
> Whenever we fail to do so, we better fix the emulation to be accurate  
> ;)
> >
> > What behaviors would "mpc8544" specify that "fsl mpic v2.0" would  
> not?
> 
> I don't know. If you say that mpc8544 == "fsl mpic v2.0" I'm more  
> than happy to rename what we have. Simply calling it "MPIC" was  
> definitely wrong, so I want with the one where I'm actually sure that  
> what I'm implementing is correct, because I have the spec in front of  
> me.
> 
> My general approach to this problem would be that we for example get  
> a p4080 board once. Once we get that, we want a p4080 MPIC. Then  
> you'd sit down and model the p4080 MPIC. You realize that it's  
> identical to the mpc8544 MPIC. So you either choose to instantiate an  
> MPC8544 MPIC or you rename the model name to "fsl mpic v2.0".

p4080 would be "fsl mpic v4.2" -- unless you want to model an older  
revision of p4080 in which case it could be v4.0 or v4.1.  Note that  
this example shows that the chip name can be even less specific than  
the block version number.

> If you can assure me today that they will be identical, I'm more than  
> happy to change the name today already :).

"fsl mpic v2.0" describes the MPIC that was integrated into the  
mpc8544, as well as several other chips.  In general you can look at  
versioned SoC blocks as if they were a separate chip, except for  
integration parameters, which should be qdev parameters.  The only  
integration parameters I can think of for MPIC are the number of CPUs  
-- we already deviate from mpc8544 there to allow SMP -- and number of  
interrupt sources, for which we can safely just implement the maximum,  
or make it a qdev parameter if we really care about matching what  
hardware reports in FRR[NIRQ] (this number is actually rather useless  
to software the way Freescale implemented it).

-Scott

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [Qemu-devel] [Qemu-ppc] [PATCH 16/19] openpic: add Shared MSI support
  2012-12-12  0:53         ` Alexander Graf
@ 2012-12-12  1:42           ` Scott Wood
  2012-12-12 11:12             ` Alexander Graf
  0 siblings, 1 reply; 43+ messages in thread
From: Scott Wood @ 2012-12-12  1:42 UTC (permalink / raw)
  To: Alexander Graf; +Cc: qemu-ppc@nongnu.org List, qemu-devel qemu-devel

On 12/11/2012 06:53:44 PM, Alexander Graf wrote:
> 
> On 11.12.2012, at 18:35, Scott Wood wrote:
> 
> > On 12/11/2012 02:10:14 AM, Alexander Graf wrote:
> >> On 11.12.2012, at 01:36, Scott Wood <scottwood@freescale.com>  
> wrote:
> >> > On 12/08/2012 07:44:39 AM, Alexander Graf wrote:
> >> >> The OpenPIC allows MSI access through shared MSI registers.  
> Implement
> >> >> them for the MPC8544 MPIC, so we can support MSIs.
> >> >> Signed-off-by: Alexander Graf <agraf@suse.de>
> >> >> ---
> >> >> hw/openpic.c |  150  
> ++++++++++++++++++++++++++++++++++++++++++++++++++--------
> >> >> 1 files changed, 130 insertions(+), 20 deletions(-)
> >> >> diff --git a/hw/openpic.c b/hw/openpic.c
> >> >> index f2f152f..f71d668 100644
> >> >> --- a/hw/openpic.c
> >> >> +++ b/hw/openpic.c
> >> >> @@ -38,6 +38,7 @@
> >> >> #include "pci.h"
> >> >> #include "openpic.h"
> >> >> #include "sysbus.h"
> >> >> +#include "msi.h"
> >> >> //#define DEBUG_OPENPIC
> >> >> @@ -52,6 +53,7 @@
> >> >> #define MAX_TMR     4
> >> >> #define VECTOR_BITS 8
> >> >> #define MAX_IPI     4
> >> >> +#define MAX_MSI     8
> >> >> #define VID         0x03 /* MPIC version ID */
> >> >> /* OpenPIC capability flags */
> >> >> @@ -62,6 +64,8 @@
> >> >> #define OPENPIC_GLB_REG_SIZE         0x10F0
> >> >> #define OPENPIC_TMR_REG_START        0x10F0
> >> >> #define OPENPIC_TMR_REG_SIZE         0x220
> >> >> +#define OPENPIC_MSI_REG_START        0x1600
> >> >> +#define OPENPIC_MSI_REG_SIZE         0x200
> >> >> #define OPENPIC_SRC_REG_START        0x10000
> >> >> #define OPENPIC_SRC_REG_SIZE         (MAX_IRQ * 0x20)
> >> >> #define OPENPIC_CPU_REG_START        0x20000
> >> >> @@ -126,6 +130,12 @@
> >> >> #define IDR_P1_SHIFT      1
> >> >> #define IDR_P0_SHIFT      0
> >> >> +#define MSIIR_OFFSET       0x140
> >> >> +#define MSIIR_SRS_SHIFT    29
> >> >> +#define MSIIR_SRS_MASK     (0x7 << MSIIR_SRS_SHIFT)
> >> >> +#define MSIIR_IBS_SHIFT    24
> >> >> +#define MSIIR_IBS_MASK     (0x1f << MSIIR_IBS_SHIFT)
> >> >
> >> > FWIW, if you want to model newer MPICs such as on p4080, they  
> have multiple banks of MSIs, so you may not want to hardcode one bank.
> >> The OpenPIC code was suffering a lot from attempts to generalize  
> different implementations without implementing them.
> >> If we want to add support for p4080 MPICs later, we add a new  
> model to the emulation and make the nr of msi banks a parameter, like  
> the patch set does for all the other raven/mpc8544 differences. That  
> way we don't get into the current mess of a halfway accurate  
> emulation unless we really want it.
> >
> > So because the old code made a mess of it, we're saying  
> "abstraction is bad" in general?
> 
> No, because the old code messed up abstraction, I would like to move  
> to a non-abstract level and then start abstracting at the correct  
> layer. What that correct layer is is still up for discussion :).
> 
> > All I'm saying is this should be done with a runtime data structure  
> (of which there could be more than one) rather than #defines.
> 
> Yeah, and my argument was that this should be introduced as part of a  
> new model. But if it makes you happy I can of course also make it  
> generic right now, without a user, which means we can't even test  
> whether the generalization works, which means we might screw it up  
> again ;).

It would have a user.  It just wouldn't have multiple users, or a user  
that instantiates multiple banks.  This feels like writing a kernel  
driver that can only handle one instance of a device, just because  
that's all your SoC happens to have at the moment.  Nobody objects to a  
driver having its state in a struct, or the driver maintaining a list  
of said structs, just because you don't have hardware to test the case  
where there are multiple instances.

-Scott

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [Qemu-devel] [PATCH 14/19] openpic: convert to qdev
  2012-12-12  1:38           ` Scott Wood
@ 2012-12-12 10:37             ` Alexander Graf
  0 siblings, 0 replies; 43+ messages in thread
From: Alexander Graf @ 2012-12-12 10:37 UTC (permalink / raw)
  To: Scott Wood; +Cc: qemu-ppc@nongnu.org List, qemu-devel qemu-devel


On 12.12.2012, at 02:38, Scott Wood wrote:

> On 12/11/2012 06:56:56 PM, Alexander Graf wrote:
>> On 11.12.2012, at 18:47, Scott Wood wrote:
>> > On 12/11/2012 02:25:31 AM, Alexander Graf wrote:
>> >> If we want a pv style generic mpic (for -M e500), let's add such an mpic to the model list and make that one be really generic. But the MPIC in -M mpc8544ds should behave exactly like an mpc8544 mpic. Whenever we fail to do so, we better fix the emulation to be accurate ;)
>> >
>> > What behaviors would "mpc8544" specify that "fsl mpic v2.0" would not?
>> I don't know. If you say that mpc8544 == "fsl mpic v2.0" I'm more than happy to rename what we have. Simply calling it "MPIC" was definitely wrong, so I want with the one where I'm actually sure that what I'm implementing is correct, because I have the spec in front of me.
>> My general approach to this problem would be that we for example get a p4080 board once. Once we get that, we want a p4080 MPIC. Then you'd sit down and model the p4080 MPIC. You realize that it's identical to the mpc8544 MPIC. So you either choose to instantiate an MPC8544 MPIC or you rename the model name to "fsl mpic v2.0".
> 
> p4080 would be "fsl mpic v4.2" -- unless you want to model an older revision of p4080 in which case it could be v4.0 or v4.1.  Note that this example shows that the chip name can be even less specific than the block version number.

Ok, I'll rename it to "FSL_MPIC_20" then :).

> 
>> If you can assure me today that they will be identical, I'm more than happy to change the name today already :).
> 
> "fsl mpic v2.0" describes the MPIC that was integrated into the mpc8544, as well as several other chips.  In general you can look at versioned SoC blocks as if they were a separate chip, except for integration parameters, which should be qdev parameters.  The only integration parameters I can think of for MPIC are the number of CPUs -- we already deviate from mpc8544 there to allow SMP -- and number of interrupt sources, for which we can safely just implement the maximum, or make it a qdev parameter if we really care about matching what hardware reports in FRR[NIRQ] (this number is actually rather useless to software the way Freescale implemented it).

Sounds great :).


Alex

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [Qemu-devel] [Qemu-ppc] [PATCH 16/19] openpic: add Shared MSI support
  2012-12-12  1:42           ` Scott Wood
@ 2012-12-12 11:12             ` Alexander Graf
  0 siblings, 0 replies; 43+ messages in thread
From: Alexander Graf @ 2012-12-12 11:12 UTC (permalink / raw)
  To: Scott Wood; +Cc: qemu-ppc@nongnu.org List, qemu-devel qemu-devel


On 12.12.2012, at 02:42, Scott Wood wrote:

> On 12/11/2012 06:53:44 PM, Alexander Graf wrote:
>> On 11.12.2012, at 18:35, Scott Wood wrote:
>> > On 12/11/2012 02:10:14 AM, Alexander Graf wrote:
>> >> On 11.12.2012, at 01:36, Scott Wood <scottwood@freescale.com> wrote:
>> >> > On 12/08/2012 07:44:39 AM, Alexander Graf wrote:
>> >> >> The OpenPIC allows MSI access through shared MSI registers. Implement
>> >> >> them for the MPC8544 MPIC, so we can support MSIs.
>> >> >> Signed-off-by: Alexander Graf <agraf@suse.de>
>> >> >> ---
>> >> >> hw/openpic.c |  150 ++++++++++++++++++++++++++++++++++++++++++++++++++--------
>> >> >> 1 files changed, 130 insertions(+), 20 deletions(-)
>> >> >> diff --git a/hw/openpic.c b/hw/openpic.c
>> >> >> index f2f152f..f71d668 100644
>> >> >> --- a/hw/openpic.c
>> >> >> +++ b/hw/openpic.c
>> >> >> @@ -38,6 +38,7 @@
>> >> >> #include "pci.h"
>> >> >> #include "openpic.h"
>> >> >> #include "sysbus.h"
>> >> >> +#include "msi.h"
>> >> >> //#define DEBUG_OPENPIC
>> >> >> @@ -52,6 +53,7 @@
>> >> >> #define MAX_TMR     4
>> >> >> #define VECTOR_BITS 8
>> >> >> #define MAX_IPI     4
>> >> >> +#define MAX_MSI     8
>> >> >> #define VID         0x03 /* MPIC version ID */
>> >> >> /* OpenPIC capability flags */
>> >> >> @@ -62,6 +64,8 @@
>> >> >> #define OPENPIC_GLB_REG_SIZE         0x10F0
>> >> >> #define OPENPIC_TMR_REG_START        0x10F0
>> >> >> #define OPENPIC_TMR_REG_SIZE         0x220
>> >> >> +#define OPENPIC_MSI_REG_START        0x1600
>> >> >> +#define OPENPIC_MSI_REG_SIZE         0x200
>> >> >> #define OPENPIC_SRC_REG_START        0x10000
>> >> >> #define OPENPIC_SRC_REG_SIZE         (MAX_IRQ * 0x20)
>> >> >> #define OPENPIC_CPU_REG_START        0x20000
>> >> >> @@ -126,6 +130,12 @@
>> >> >> #define IDR_P1_SHIFT      1
>> >> >> #define IDR_P0_SHIFT      0
>> >> >> +#define MSIIR_OFFSET       0x140
>> >> >> +#define MSIIR_SRS_SHIFT    29
>> >> >> +#define MSIIR_SRS_MASK     (0x7 << MSIIR_SRS_SHIFT)
>> >> >> +#define MSIIR_IBS_SHIFT    24
>> >> >> +#define MSIIR_IBS_MASK     (0x1f << MSIIR_IBS_SHIFT)
>> >> >
>> >> > FWIW, if you want to model newer MPICs such as on p4080, they have multiple banks of MSIs, so you may not want to hardcode one bank.
>> >> The OpenPIC code was suffering a lot from attempts to generalize different implementations without implementing them.
>> >> If we want to add support for p4080 MPICs later, we add a new model to the emulation and make the nr of msi banks a parameter, like the patch set does for all the other raven/mpc8544 differences. That way we don't get into the current mess of a halfway accurate emulation unless we really want it.
>> >
>> > So because the old code made a mess of it, we're saying "abstraction is bad" in general?
>> No, because the old code messed up abstraction, I would like to move to a non-abstract level and then start abstracting at the correct layer. What that correct layer is is still up for discussion :).
>> > All I'm saying is this should be done with a runtime data structure (of which there could be more than one) rather than #defines.
>> Yeah, and my argument was that this should be introduced as part of a new model. But if it makes you happy I can of course also make it generic right now, without a user, which means we can't even test whether the generalization works, which means we might screw it up again ;).
> 
> It would have a user.  It just wouldn't have multiple users, or a user that instantiates multiple banks.  This feels like writing a kernel driver that can only handle one instance of a device, just because that's all your SoC happens to have at the moment.  Nobody objects to a driver having its state in a struct, or the driver maintaining a list of said structs, just because you don't have hardware to test the case where there are multiple instances.

Looking at this a bit more, I am getting more and more reluctant to make this dynamic with the current openpic models we have.

To get multiple MSI banks cleanly working, we'd need to make a number of fields in the state struct dynamically allocated (msir backing, sub_io_mem). We'd also need to find a way to identify which bank a callback happens at, which the memory api hides from us today. So we again would probably need to allocate some container struct that we can pass as opaque which tells us the region id we're in.

And all of the pitfalls above would be something you could only spot using code review, not by actual execution tests, because we would have to hardcode msi_banks to 1 for fsl_mpic_20. So every time we assume cur_msi_bank == 0 the code would "just work".

I would really much rather like to postpone that generalization on to the point in time we start implementing more than 1 msi bank. Then we can be sure that the code we're doing generically actually works.


Alex

^ permalink raw reply	[flat|nested] 43+ messages in thread

* [Qemu-devel] [PATCH 02/19] mpic: Unify numbering scheme
  2012-12-12 14:12 [Qemu-devel] [PATCH 00/19] OpenPIC refactoring and MSI support v2 Alexander Graf
@ 2012-12-12 14:12 ` Alexander Graf
  0 siblings, 0 replies; 43+ messages in thread
From: Alexander Graf @ 2012-12-12 14:12 UTC (permalink / raw)
  To: qemu-ppc@nongnu.org List; +Cc: qemu-devel qemu-devel

MPIC interrupt numbers in Linux (device tree) and in QEMU are different,
because QEMU takes the sparseness of the IRQ number space into account.

Remove that cleverness and instead assume a flat number space. This makes
the code easier to understand, because we are actually aligned with Linux
on the view of our worlds.

Signed-off-by: Alexander Graf <agraf@suse.de>

---

v1 -> v2:

  - don't overlap internal vectors on src irqs
---
 hw/openpic.c  |  290 +++++++++------------------------------------------------
 hw/ppc/e500.c |    4 +-
 2 files changed, 45 insertions(+), 249 deletions(-)

diff --git a/hw/openpic.c b/hw/openpic.c
index b30c853..122ce76 100644
--- a/hw/openpic.c
+++ b/hw/openpic.c
@@ -46,11 +46,12 @@
 #define DPRINTF(fmt, ...) do { } while (0)
 #endif
 
-#define MAX_CPU    15
-#define MAX_IRQ   128
+#define MAX_CPU     15
+#define MAX_SRC     256
 #define MAX_TMR     4
 #define VECTOR_BITS 8
 #define MAX_IPI     4
+#define MAX_IRQ     (MAX_SRC + MAX_IPI + MAX_TMR)
 #define VID         0x03 /* MPIC version ID */
 #define VENI        0x00000000 /* Vendor ID */
 
@@ -82,32 +83,25 @@ enum {
 #define MPIC_MAX_CPU      1
 #define MPIC_MAX_EXT     12
 #define MPIC_MAX_INT     64
-#define MPIC_MAX_MSG      4
-#define MPIC_MAX_MSI      8
-#define MPIC_MAX_TMR      MAX_TMR
-#define MPIC_MAX_IPI      MAX_IPI
-#define MPIC_MAX_IRQ     (MPIC_MAX_EXT + MPIC_MAX_INT + MPIC_MAX_TMR + MPIC_MAX_MSG + MPIC_MAX_MSI + (MPIC_MAX_IPI * MPIC_MAX_CPU))
+#define MPIC_MAX_IRQ     MAX_IRQ
 
 /* Interrupt definitions */
-#define MPIC_EXT_IRQ      0
-#define MPIC_INT_IRQ      (MPIC_EXT_IRQ + MPIC_MAX_EXT)
-#define MPIC_TMR_IRQ      (MPIC_INT_IRQ + MPIC_MAX_INT)
-#define MPIC_MSG_IRQ      (MPIC_TMR_IRQ + MPIC_MAX_TMR)
-#define MPIC_MSI_IRQ      (MPIC_MSG_IRQ + MPIC_MAX_MSG)
-#define MPIC_IPI_IRQ      (MPIC_MSI_IRQ + MPIC_MAX_MSI)
+/* IRQs, accessible through the IRQ region */
+#define MPIC_EXT_IRQ      0x00
+#define MPIC_INT_IRQ      0x10
+#define MPIC_MSG_IRQ      0xb0
+#define MPIC_MSI_IRQ      0xe0
+/* These are available through separate regions, but
+   for simplicity's sake mapped into the same number space */
+#define MPIC_TMR_IRQ      0x100
+#define MPIC_IPI_IRQ      0x104
 
 #define MPIC_GLB_REG_START        0x0
 #define MPIC_GLB_REG_SIZE         0x10F0
 #define MPIC_TMR_REG_START        0x10F0
 #define MPIC_TMR_REG_SIZE         0x220
-#define MPIC_EXT_REG_START        0x10000
-#define MPIC_EXT_REG_SIZE         0x180
-#define MPIC_INT_REG_START        0x10200
-#define MPIC_INT_REG_SIZE         0x800
-#define MPIC_MSG_REG_START        0x11600
-#define MPIC_MSG_REG_SIZE         0x100
-#define MPIC_MSI_REG_START        0x11C00
-#define MPIC_MSI_REG_SIZE         0x100
+#define MPIC_IRQ_REG_START        0x10000
+#define MPIC_IRQ_REG_SIZE         (MAX_SRC * 0x20)
 #define MPIC_CPU_REG_START        0x20000
 #define MPIC_CPU_REG_SIZE         0x100 + ((MAX_CPU - 1) * 0x1000)
 
@@ -1205,193 +1199,44 @@ static uint32_t mpic_timer_read (void *opaque, hwaddr addr)
     return retval;
 }
 
-static void mpic_src_ext_write (void *opaque, hwaddr addr,
-                                uint32_t val)
+static void mpic_src_irq_write(void *opaque, hwaddr addr,
+                               uint64_t val, unsigned len)
 {
     openpic_t *mpp = opaque;
-    int idx = MPIC_EXT_IRQ;
+    int idx = addr / 0x20;
 
-    DPRINTF("%s: addr " TARGET_FMT_plx " <= %08x\n", __func__, addr, val);
-    if (addr & 0xF)
-        return;
-
-    if (addr < MPIC_EXT_REG_SIZE) {
-        idx += (addr & 0xFFF0) >> 5;
-        if (addr & 0x10) {
-            /* EXDE / IFEDE / IEEDE */
-            write_IRQreg_ide(mpp, idx, val);
-        } else {
-            /* EXVP / IFEVP / IEEVP */
-            write_IRQreg_ipvp(mpp, idx, val);
-        }
-    }
-}
-
-static uint32_t mpic_src_ext_read (void *opaque, hwaddr addr)
-{
-    openpic_t *mpp = opaque;
-    uint32_t retval;
-    int idx = MPIC_EXT_IRQ;
-
-    DPRINTF("%s: addr " TARGET_FMT_plx "\n", __func__, addr);
-    retval = 0xFFFFFFFF;
-    if (addr & 0xF)
-        return retval;
-
-    if (addr < MPIC_EXT_REG_SIZE) {
-        idx += (addr & 0xFFF0) >> 5;
-        if (addr & 0x10) {
-            /* EXDE / IFEDE / IEEDE */
-            retval = read_IRQreg_ide(mpp, idx);
-        } else {
-            /* EXVP / IFEVP / IEEVP */
-            retval = read_IRQreg_ipvp(mpp, idx);
-        }
-        DPRINTF("%s: => %08x\n", __func__, retval);
-    }
-
-    return retval;
-}
-
-static void mpic_src_int_write (void *opaque, hwaddr addr,
-                                uint32_t val)
-{
-    openpic_t *mpp = opaque;
-    int idx = MPIC_INT_IRQ;
-
-    DPRINTF("%s: addr " TARGET_FMT_plx " <= %08x\n", __func__, addr, val);
-    if (addr & 0xF)
-        return;
-
-    if (addr < MPIC_INT_REG_SIZE) {
-        idx += (addr & 0xFFF0) >> 5;
-        if (addr & 0x10) {
-            /* EXDE / IFEDE / IEEDE */
-            write_IRQreg_ide(mpp, idx, val);
-        } else {
-            /* EXVP / IFEVP / IEEVP */
-            write_IRQreg_ipvp(mpp, idx, val);
-        }
-    }
-}
-
-static uint32_t mpic_src_int_read (void *opaque, hwaddr addr)
-{
-    openpic_t *mpp = opaque;
-    uint32_t retval;
-    int idx = MPIC_INT_IRQ;
-
-    DPRINTF("%s: addr " TARGET_FMT_plx "\n", __func__, addr);
-    retval = 0xFFFFFFFF;
-    if (addr & 0xF)
-        return retval;
-
-    if (addr < MPIC_INT_REG_SIZE) {
-        idx += (addr & 0xFFF0) >> 5;
-        if (addr & 0x10) {
-            /* EXDE / IFEDE / IEEDE */
-            retval = read_IRQreg_ide(mpp, idx);
-        } else {
-            /* EXVP / IFEVP / IEEVP */
-            retval = read_IRQreg_ipvp(mpp, idx);
-        }
-        DPRINTF("%s: => %08x\n", __func__, retval);
-    }
-
-    return retval;
-}
-
-static void mpic_src_msg_write (void *opaque, hwaddr addr,
-                                uint32_t val)
-{
-    openpic_t *mpp = opaque;
-    int idx = MPIC_MSG_IRQ;
-
-    DPRINTF("%s: addr " TARGET_FMT_plx " <= %08x\n", __func__, addr, val);
+    DPRINTF("%s: addr " TARGET_FMT_plx " <= %08" PRIx64 "\n",
+            __func__, addr, val);
     if (addr & 0xF)
         return;
 
-    if (addr < MPIC_MSG_REG_SIZE) {
-        idx += (addr & 0xFFF0) >> 5;
-        if (addr & 0x10) {
-            /* EXDE / IFEDE / IEEDE */
-            write_IRQreg_ide(mpp, idx, val);
-        } else {
-            /* EXVP / IFEVP / IEEVP */
-            write_IRQreg_ipvp(mpp, idx, val);
-        }
-    }
-}
-
-static uint32_t mpic_src_msg_read (void *opaque, hwaddr addr)
-{
-    openpic_t *mpp = opaque;
-    uint32_t retval;
-    int idx = MPIC_MSG_IRQ;
-
-    DPRINTF("%s: addr " TARGET_FMT_plx "\n", __func__, addr);
-    retval = 0xFFFFFFFF;
-    if (addr & 0xF)
-        return retval;
-
-    if (addr < MPIC_MSG_REG_SIZE) {
-        idx += (addr & 0xFFF0) >> 5;
-        if (addr & 0x10) {
-            /* EXDE / IFEDE / IEEDE */
-            retval = read_IRQreg_ide(mpp, idx);
-        } else {
-            /* EXVP / IFEVP / IEEVP */
-            retval = read_IRQreg_ipvp(mpp, idx);
-        }
-        DPRINTF("%s: => %08x\n", __func__, retval);
+    if (addr & 0x10) {
+        /* EXDE / IFEDE / IEEDE */
+        write_IRQreg_ide(mpp, idx, val);
+    } else {
+        /* EXVP / IFEVP / IEEVP */
+        write_IRQreg_ipvp(mpp, idx, val);
     }
-
-    return retval;
 }
 
-static void mpic_src_msi_write (void *opaque, hwaddr addr,
-                                uint32_t val)
-{
-    openpic_t *mpp = opaque;
-    int idx = MPIC_MSI_IRQ;
-
-    DPRINTF("%s: addr " TARGET_FMT_plx " <= %08x\n", __func__, addr, val);
-    if (addr & 0xF)
-        return;
-
-    if (addr < MPIC_MSI_REG_SIZE) {
-        idx += (addr & 0xFFF0) >> 5;
-        if (addr & 0x10) {
-            /* EXDE / IFEDE / IEEDE */
-            write_IRQreg_ide(mpp, idx, val);
-        } else {
-            /* EXVP / IFEVP / IEEVP */
-            write_IRQreg_ipvp(mpp, idx, val);
-        }
-    }
-}
-static uint32_t mpic_src_msi_read (void *opaque, hwaddr addr)
+static uint64_t mpic_src_irq_read(void *opaque, hwaddr addr, unsigned len)
 {
     openpic_t *mpp = opaque;
     uint32_t retval;
-    int idx = MPIC_MSI_IRQ;
+    int idx = addr / 0x20;
 
     DPRINTF("%s: addr " TARGET_FMT_plx "\n", __func__, addr);
-    retval = 0xFFFFFFFF;
     if (addr & 0xF)
-        return retval;
+        return -1;
 
-    if (addr < MPIC_MSI_REG_SIZE) {
-        idx += (addr & 0xFFF0) >> 5;
-        if (addr & 0x10) {
-            /* EXDE / IFEDE / IEEDE */
-            retval = read_IRQreg_ide(mpp, idx);
-        } else {
-            /* EXVP / IFEVP / IEEVP */
-            retval = read_IRQreg_ipvp(mpp, idx);
-        }
-        DPRINTF("%s: => %08x\n", __func__, retval);
+    if (addr & 0x10) {
+        /* EXDE / IFEDE / IEEDE */
+        retval = read_IRQreg_ide(mpp, idx);
+    } else {
+        /* EXVP / IFEVP / IEEVP */
+        retval = read_IRQreg_ipvp(mpp, idx);
     }
+    DPRINTF("%s: => %08x\n", __func__, retval);
 
     return retval;
 }
@@ -1438,60 +1283,14 @@ static const MemoryRegionOps mpic_cpu_ops = {
     .endianness = DEVICE_BIG_ENDIAN,
 };
 
-static const MemoryRegionOps mpic_ext_ops = {
-    .old_mmio = {
-        .write = { openpic_buggy_write,
-                   openpic_buggy_write,
-                   mpic_src_ext_write,
-        },
-        .read  = { openpic_buggy_read,
-                   openpic_buggy_read,
-                   mpic_src_ext_read,
-        },
-    },
-    .endianness = DEVICE_BIG_ENDIAN,
-};
-
-static const MemoryRegionOps mpic_int_ops = {
-    .old_mmio = {
-        .write = { openpic_buggy_write,
-                   openpic_buggy_write,
-                   mpic_src_int_write,
-        },
-        .read  = { openpic_buggy_read,
-                   openpic_buggy_read,
-                   mpic_src_int_read,
-        },
-    },
-    .endianness = DEVICE_BIG_ENDIAN,
-};
-
-static const MemoryRegionOps mpic_msg_ops = {
-    .old_mmio = {
-        .write = { openpic_buggy_write,
-                   openpic_buggy_write,
-                   mpic_src_msg_write,
-        },
-        .read  = { openpic_buggy_read,
-                   openpic_buggy_read,
-                   mpic_src_msg_read,
-        },
-    },
+static const MemoryRegionOps mpic_irq_ops = {
+    .write = mpic_src_irq_write,
+    .read  = mpic_src_irq_read,
     .endianness = DEVICE_BIG_ENDIAN,
-};
-
-static const MemoryRegionOps mpic_msi_ops = {
-    .old_mmio = {
-        .write = { openpic_buggy_write,
-                   openpic_buggy_write,
-                   mpic_src_msi_write,
-        },
-        .read  = { openpic_buggy_read,
-                   openpic_buggy_read,
-                   mpic_src_msi_read,
-        },
+    .impl = {
+        .min_access_size = 4,
+        .max_access_size = 4,
     },
-    .endianness = DEVICE_BIG_ENDIAN,
 };
 
 qemu_irq *mpic_init (MemoryRegion *address_space, hwaddr base,
@@ -1507,10 +1306,7 @@ qemu_irq *mpic_init (MemoryRegion *address_space, hwaddr base,
     } const list[] = {
         {"glb", &mpic_glb_ops, MPIC_GLB_REG_START, MPIC_GLB_REG_SIZE},
         {"tmr", &mpic_tmr_ops, MPIC_TMR_REG_START, MPIC_TMR_REG_SIZE},
-        {"ext", &mpic_ext_ops, MPIC_EXT_REG_START, MPIC_EXT_REG_SIZE},
-        {"int", &mpic_int_ops, MPIC_INT_REG_START, MPIC_INT_REG_SIZE},
-        {"msg", &mpic_msg_ops, MPIC_MSG_REG_START, MPIC_MSG_REG_SIZE},
-        {"msi", &mpic_msi_ops, MPIC_MSI_REG_START, MPIC_MSI_REG_SIZE},
+        {"irq", &mpic_irq_ops, MPIC_IRQ_REG_START, MPIC_IRQ_REG_SIZE},
         {"cpu", &mpic_cpu_ops, MPIC_CPU_REG_START, MPIC_CPU_REG_SIZE},
     };
 
diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
index 47e2d41..f3e97d8 100644
--- a/hw/ppc/e500.c
+++ b/hw/ppc/e500.c
@@ -502,13 +502,13 @@ void ppce500_init(PPCE500Params *params)
     /* Serial */
     if (serial_hds[0]) {
         serial_mm_init(ccsr_addr_space, MPC8544_SERIAL0_REGS_OFFSET,
-                       0, mpic[12+26], 399193,
+                       0, mpic[42], 399193,
                        serial_hds[0], DEVICE_BIG_ENDIAN);
     }
 
     if (serial_hds[1]) {
         serial_mm_init(ccsr_addr_space, MPC8544_SERIAL1_REGS_OFFSET,
-                       0, mpic[12+26], 399193,
+                       0, mpic[42], 399193,
                        serial_hds[1], DEVICE_BIG_ENDIAN);
     }
 
-- 
1.6.0.2

^ permalink raw reply related	[flat|nested] 43+ messages in thread

end of thread, other threads:[~2012-12-12 14:12 UTC | newest]

Thread overview: 43+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-12-08 13:44 [Qemu-devel] [PATCH 00/19] OpenPIC refactoring and MSI support Alexander Graf
2012-12-08 13:44 ` [Qemu-devel] [PATCH 01/19] openpic: Remove unused code Alexander Graf
2012-12-08 15:12   ` Andreas Färber
2012-12-08 15:14     ` Alexander Graf
2012-12-08 17:06     ` Hervé Poussineau
2012-12-08 13:44 ` [Qemu-devel] [PATCH 02/19] mpic: Unify numbering scheme Alexander Graf
2012-12-10 23:34   ` [Qemu-devel] [Qemu-ppc] " Scott Wood
2012-12-10 23:40     ` Scott Wood
2012-12-11  8:14     ` Alexander Graf
2012-12-11 17:39       ` Scott Wood
2012-12-08 13:44 ` [Qemu-devel] [PATCH 03/19] openpic: update to proper memory api Alexander Graf
2012-12-08 13:44 ` [Qemu-devel] [PATCH 04/19] openpic: combine mpic and openpic src handlers Alexander Graf
2012-12-08 13:44 ` [Qemu-devel] [PATCH 05/19] openpic: Convert subregions to memory api Alexander Graf
2012-12-08 13:44 ` [Qemu-devel] [PATCH 06/19] openpic: combine mpic and openpic irq raise functions Alexander Graf
2012-12-08 13:44 ` [Qemu-devel] [PATCH 07/19] openpic: merge mpic and openpic timer handling Alexander Graf
2012-12-08 13:44 ` [Qemu-devel] [PATCH 08/19] openpic: combine openpic and mpic reset functions Alexander Graf
2012-12-08 13:44 ` [Qemu-devel] [PATCH 09/19] openpic: unify memory api subregions Alexander Graf
2012-12-08 13:44 ` [Qemu-devel] [PATCH 10/19] openpic: remove unused type variable Alexander Graf
2012-12-10 23:42   ` [Qemu-devel] [Qemu-ppc] " Scott Wood
2012-12-11  8:17     ` Alexander Graf
2012-12-08 13:44 ` [Qemu-devel] [PATCH 11/19] openpic: convert simple reg operations to builtin bitops Alexander Graf
2012-12-08 13:44 ` [Qemu-devel] [PATCH 12/19] openpic: rename openpic_t to OpenPICState Alexander Graf
2012-12-08 13:44 ` [Qemu-devel] [PATCH 13/19] openpic: remove irq_out Alexander Graf
2012-12-08 13:44 ` [Qemu-devel] [PATCH 14/19] openpic: convert to qdev Alexander Graf
2012-12-10 23:47   ` Scott Wood
2012-12-11  8:25     ` Alexander Graf
2012-12-11 17:47       ` Scott Wood
2012-12-12  0:56         ` Alexander Graf
2012-12-12  1:38           ` Scott Wood
2012-12-12 10:37             ` Alexander Graf
2012-12-08 13:44 ` [Qemu-devel] [PATCH 15/19] openpic: make brr1 model specific Alexander Graf
2012-12-08 13:44 ` [Qemu-devel] [PATCH 16/19] openpic: add Shared MSI support Alexander Graf
2012-12-11  0:36   ` [Qemu-devel] [Qemu-ppc] " Scott Wood
2012-12-11  8:10     ` Alexander Graf
2012-12-11 17:35       ` Scott Wood
2012-12-12  0:53         ` Alexander Graf
2012-12-12  1:42           ` Scott Wood
2012-12-12 11:12             ` Alexander Graf
2012-12-08 13:44 ` [Qemu-devel] [PATCH 17/19] PPC: e500: Add " Alexander Graf
2012-12-08 13:44 ` [Qemu-devel] [PATCH 18/19] PPC: e500: Declare pci bridge as bridge Alexander Graf
2012-12-08 13:44 ` [Qemu-devel] [PATCH 19/19] MSI-X: Fix endianness Alexander Graf
2012-12-08 22:41   ` Michael S. Tsirkin
2012-12-12 14:12 [Qemu-devel] [PATCH 00/19] OpenPIC refactoring and MSI support v2 Alexander Graf
2012-12-12 14:12 ` [Qemu-devel] [PATCH 02/19] mpic: Unify numbering scheme Alexander Graf

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.