All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/4] hw/ppc: Convert UIC device to QOM
@ 2021-01-08 17:12 Peter Maydell
  2021-01-08 17:12 ` [PATCH v2 1/4] hw/ppc/sam460ex: Drop use of ppcuic_init() Peter Maydell
                   ` (4 more replies)
  0 siblings, 5 replies; 7+ messages in thread
From: Peter Maydell @ 2021-01-08 17:12 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-ppc, Greg Kurz, David Gibson

This patchseries converts the PPC UIC "Universal Interrupt
Controller" to a QOM device.  My main reason for doing it is that
this fixes a couple of long-standing trivial Coverity issues -- the
current ppcuic_init() function allocates an array of qemu_irqs which
the callers then leak.  (The leak is trivial because it happens once
when QEMU starts.)

The first half of v1 is in master now, so this is just a
respin of the last four patches.

Changes v1->v2:
 * fixed the bug in the sam460ex conversion handling of CINT
   (tested that this now boots AROS OK to a desktop)
 * other minor code style tweaks to patch 1 as per review

thanks
-- PMM

Peter Maydell (4):
  hw/ppc/sam460ex: Drop use of ppcuic_init()
  hw/ppc: Delete unused ppc405cr_init() code
  hw/ppc/ppc405_uc: Drop use of ppcuic_init()
  hw/ppc: Remove unused ppcuic_init()

 hw/ppc/ppc405.h           |   8 +-
 include/hw/intc/ppc-uic.h |   7 +
 include/hw/ppc/ppc4xx.h   |   9 -
 hw/ppc/ppc405_boards.c    |   8 +-
 hw/ppc/ppc405_uc.c        | 415 ++++----------------------------------
 hw/ppc/ppc4xx_devs.c      |  38 ----
 hw/ppc/sam460ex.c         |  69 +++++--
 7 files changed, 107 insertions(+), 447 deletions(-)

-- 
2.20.1



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

* [PATCH v2 1/4] hw/ppc/sam460ex: Drop use of ppcuic_init()
  2021-01-08 17:12 [PATCH v2 0/4] hw/ppc: Convert UIC device to QOM Peter Maydell
@ 2021-01-08 17:12 ` Peter Maydell
  2021-01-08 22:16   ` BALATON Zoltan
  2021-01-08 17:12 ` [PATCH v2 2/4] hw/ppc: Delete unused ppc405cr_init() code Peter Maydell
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 7+ messages in thread
From: Peter Maydell @ 2021-01-08 17:12 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-ppc, Greg Kurz, David Gibson

Switch the sam460ex board to directly creating and configuring the
UIC, rather than doing it via the old ppcuic_init() helper function.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
v1->v2 changes:
 * fix typo in UIC 0 CINT wiring
 * move local var declarations up
 * drop unnecessary TODO comment
 * improve comment about what the input_ints[] array is doing
---
 hw/ppc/sam460ex.c | 69 ++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 53 insertions(+), 16 deletions(-)

diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c
index 14e6583eb0d..45721ad6c73 100644
--- a/hw/ppc/sam460ex.c
+++ b/hw/ppc/sam460ex.c
@@ -39,6 +39,7 @@
 #include "hw/usb/hcd-ehci.h"
 #include "hw/ppc/fdt.h"
 #include "hw/qdev-properties.h"
+#include "hw/intc/ppc-uic.h"
 
 #include <libfdt.h>
 
@@ -281,7 +282,9 @@ static void sam460ex_init(MachineState *machine)
     hwaddr ram_bases[SDRAM_NR_BANKS] = {0};
     hwaddr ram_sizes[SDRAM_NR_BANKS] = {0};
     MemoryRegion *l2cache_ram = g_new(MemoryRegion, 1);
-    qemu_irq *irqs, *uic[4];
+    DeviceState *uic[4];
+    qemu_irq mal_irqs[4];
+    int i;
     PCIBus *pci_bus;
     PowerPCCPU *cpu;
     CPUPPCState *env;
@@ -312,13 +315,38 @@ static void sam460ex_init(MachineState *machine)
     ppc4xx_plb_init(env);
 
     /* interrupt controllers */
-    irqs = g_new0(qemu_irq, PPCUIC_OUTPUT_NB);
-    irqs[PPCUIC_OUTPUT_INT] = ((qemu_irq *)env->irq_inputs)[PPC40x_INPUT_INT];
-    irqs[PPCUIC_OUTPUT_CINT] = ((qemu_irq *)env->irq_inputs)[PPC40x_INPUT_CINT];
-    uic[0] = ppcuic_init(env, irqs, 0xc0, 0, 1);
-    uic[1] = ppcuic_init(env, &uic[0][30], 0xd0, 0, 1);
-    uic[2] = ppcuic_init(env, &uic[0][10], 0xe0, 0, 1);
-    uic[3] = ppcuic_init(env, &uic[0][16], 0xf0, 0, 1);
+    for (i = 0; i < ARRAY_SIZE(uic); i++) {
+        SysBusDevice *sbd;
+        /*
+         * UICs 1, 2 and 3 are cascaded through UIC 0.
+         * input_ints[n] is the interrupt number on UIC 0 which
+         * the INT output of UIC n is connected to. The CINT output
+         * of UIC n connects to input_ints[n] + 1.
+         * The entry in input_ints[] for UIC 0 is ignored, because UIC 0's
+         * INT and CINT outputs are connected to the CPU.
+         */
+        const int input_ints[] = { -1, 30, 10, 16 };
+
+        uic[i] = qdev_new(TYPE_PPC_UIC);
+        sbd = SYS_BUS_DEVICE(uic[i]);
+
+        qdev_prop_set_uint32(uic[i], "dcr-base", 0xc0 + i * 0x10);
+        object_property_set_link(OBJECT(uic[i]), "cpu", OBJECT(cpu),
+                                 &error_fatal);
+        sysbus_realize_and_unref(sbd, &error_fatal);
+
+        if (i == 0) {
+            sysbus_connect_irq(sbd, PPCUIC_OUTPUT_INT,
+                               ((qemu_irq *)env->irq_inputs)[PPC40x_INPUT_INT]);
+            sysbus_connect_irq(sbd, PPCUIC_OUTPUT_CINT,
+                               ((qemu_irq *)env->irq_inputs)[PPC40x_INPUT_CINT]);
+        } else {
+            sysbus_connect_irq(sbd, PPCUIC_OUTPUT_INT,
+                               qdev_get_gpio_in(uic[0], input_ints[i]));
+            sysbus_connect_irq(sbd, PPCUIC_OUTPUT_CINT,
+                               qdev_get_gpio_in(uic[0], input_ints[i] + 1));
+        }
+    }
 
     /* SDRAM controller */
     /* put all RAM on first bank because board has one slot
@@ -331,7 +359,8 @@ static void sam460ex_init(MachineState *machine)
                       ram_bases, ram_sizes, 1);
 
     /* IIC controllers and devices */
-    dev = sysbus_create_simple(TYPE_PPC4xx_I2C, 0x4ef600700, uic[0][2]);
+    dev = sysbus_create_simple(TYPE_PPC4xx_I2C, 0x4ef600700,
+                               qdev_get_gpio_in(uic[0], 2));
     i2c = PPC4xx_I2C(dev)->bus;
     /* SPD EEPROM on RAM module */
     spd_data = spd_data_generate(ram_sizes[0] < 128 * MiB ? DDR : DDR2,
@@ -341,7 +370,8 @@ static void sam460ex_init(MachineState *machine)
     /* RTC */
     i2c_slave_create_simple(i2c, "m41t80", 0x68);
 
-    dev = sysbus_create_simple(TYPE_PPC4xx_I2C, 0x4ef600800, uic[0][3]);
+    dev = sysbus_create_simple(TYPE_PPC4xx_I2C, 0x4ef600800,
+                               qdev_get_gpio_in(uic[0], 3));
 
     /* External bus controller */
     ppc405_ebc_init(env);
@@ -356,7 +386,10 @@ static void sam460ex_init(MachineState *machine)
     ppc4xx_sdr_init(env);
 
     /* MAL */
-    ppc4xx_mal_init(env, 4, 16, &uic[2][3]);
+    for (i = 0; i < ARRAY_SIZE(mal_irqs); i++) {
+        mal_irqs[0] = qdev_get_gpio_in(uic[2], 3 + i);
+    }
+    ppc4xx_mal_init(env, 4, 16, mal_irqs);
 
     /* DMA */
     ppc4xx_dma_init(env, 0x200);
@@ -369,21 +402,23 @@ static void sam460ex_init(MachineState *machine)
     memory_region_add_subregion(address_space_mem, 0x400000000LL, l2cache_ram);
 
     /* USB */
-    sysbus_create_simple(TYPE_PPC4xx_EHCI, 0x4bffd0400, uic[2][29]);
+    sysbus_create_simple(TYPE_PPC4xx_EHCI, 0x4bffd0400,
+                         qdev_get_gpio_in(uic[2], 29));
     dev = qdev_new("sysbus-ohci");
     qdev_prop_set_string(dev, "masterbus", "usb-bus.0");
     qdev_prop_set_uint32(dev, "num-ports", 6);
     sbdev = SYS_BUS_DEVICE(dev);
     sysbus_realize_and_unref(sbdev, &error_fatal);
     sysbus_mmio_map(sbdev, 0, 0x4bffd0000);
-    sysbus_connect_irq(sbdev, 0, uic[2][30]);
+    sysbus_connect_irq(sbdev, 0, qdev_get_gpio_in(uic[2], 30));
     usb_create_simple(usb_bus_find(-1), "usb-kbd");
     usb_create_simple(usb_bus_find(-1), "usb-mouse");
 
     /* PCI bus */
     ppc460ex_pcie_init(env);
     /* All PCI irqs are connected to the same UIC pin (cf. UBoot source) */
-    dev = sysbus_create_simple("ppc440-pcix-host", 0xc0ec00000, uic[1][0]);
+    dev = sysbus_create_simple("ppc440-pcix-host", 0xc0ec00000,
+                               qdev_get_gpio_in(uic[1], 0));
     pci_bus = (PCIBus *)qdev_get_child_bus(dev, "pci.0");
     if (!pci_bus) {
         error_report("couldn't create PCI controller!");
@@ -405,12 +440,14 @@ static void sam460ex_init(MachineState *machine)
     /* SoC has 4 UARTs
      * but board has only one wired and two are present in fdt */
     if (serial_hd(0) != NULL) {
-        serial_mm_init(address_space_mem, 0x4ef600300, 0, uic[1][1],
+        serial_mm_init(address_space_mem, 0x4ef600300, 0,
+                       qdev_get_gpio_in(uic[1], 1),
                        PPC_SERIAL_MM_BAUDBASE, serial_hd(0),
                        DEVICE_BIG_ENDIAN);
     }
     if (serial_hd(1) != NULL) {
-        serial_mm_init(address_space_mem, 0x4ef600400, 0, uic[0][1],
+        serial_mm_init(address_space_mem, 0x4ef600400, 0,
+                       qdev_get_gpio_in(uic[0], 1),
                        PPC_SERIAL_MM_BAUDBASE, serial_hd(1),
                        DEVICE_BIG_ENDIAN);
     }
-- 
2.20.1



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

* [PATCH v2 2/4] hw/ppc: Delete unused ppc405cr_init() code
  2021-01-08 17:12 [PATCH v2 0/4] hw/ppc: Convert UIC device to QOM Peter Maydell
  2021-01-08 17:12 ` [PATCH v2 1/4] hw/ppc/sam460ex: Drop use of ppcuic_init() Peter Maydell
@ 2021-01-08 17:12 ` Peter Maydell
  2021-01-08 17:12 ` [PATCH v2 3/4] hw/ppc/ppc405_uc: Drop use of ppcuic_init() Peter Maydell
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 7+ messages in thread
From: Peter Maydell @ 2021-01-08 17:12 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-ppc, Greg Kurz, David Gibson

The function ppc405cr_init() has apparently been unused since it was
added in commit 8ecc7913525ecb in 2007.

Remove this dead code, so we don't have to convert it away from using
ppcuic_init().

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/ppc/ppc405.h    |   6 -
 hw/ppc/ppc405_uc.c | 345 ---------------------------------------------
 2 files changed, 351 deletions(-)

diff --git a/hw/ppc/ppc405.h b/hw/ppc/ppc405.h
index 7ed25cfa1bf..e6c702f7e0d 100644
--- a/hw/ppc/ppc405.h
+++ b/hw/ppc/ppc405.h
@@ -62,12 +62,6 @@ ram_addr_t ppc405_set_bootinfo (CPUPPCState *env, ppc4xx_bd_info_t *bd,
 void ppc4xx_plb_init(CPUPPCState *env);
 void ppc405_ebc_init(CPUPPCState *env);
 
-CPUPPCState *ppc405cr_init(MemoryRegion *address_space_mem,
-                        MemoryRegion ram_memories[4],
-                        hwaddr ram_bases[4],
-                        hwaddr ram_sizes[4],
-                        uint32_t sysclk, qemu_irq **picp,
-                        int do_init);
 CPUPPCState *ppc405ep_init(MemoryRegion *address_space_mem,
                         MemoryRegion ram_memories[2],
                         hwaddr ram_bases[2],
diff --git a/hw/ppc/ppc405_uc.c b/hw/ppc/ppc405_uc.c
index 381720aced9..3e191ae4af5 100644
--- a/hw/ppc/ppc405_uc.c
+++ b/hw/ppc/ppc405_uc.c
@@ -1155,351 +1155,6 @@ static void ppc4xx_gpt_init(hwaddr base, qemu_irq irqs[5])
     qemu_register_reset(ppc4xx_gpt_reset, gpt);
 }
 
-/*****************************************************************************/
-/* PowerPC 405CR */
-enum {
-    PPC405CR_CPC0_PLLMR  = 0x0B0,
-    PPC405CR_CPC0_CR0    = 0x0B1,
-    PPC405CR_CPC0_CR1    = 0x0B2,
-    PPC405CR_CPC0_PSR    = 0x0B4,
-    PPC405CR_CPC0_JTAGID = 0x0B5,
-    PPC405CR_CPC0_ER     = 0x0B9,
-    PPC405CR_CPC0_FR     = 0x0BA,
-    PPC405CR_CPC0_SR     = 0x0BB,
-};
-
-enum {
-    PPC405CR_CPU_CLK   = 0,
-    PPC405CR_TMR_CLK   = 1,
-    PPC405CR_PLB_CLK   = 2,
-    PPC405CR_SDRAM_CLK = 3,
-    PPC405CR_OPB_CLK   = 4,
-    PPC405CR_EXT_CLK   = 5,
-    PPC405CR_UART_CLK  = 6,
-    PPC405CR_CLK_NB    = 7,
-};
-
-typedef struct ppc405cr_cpc_t ppc405cr_cpc_t;
-struct ppc405cr_cpc_t {
-    clk_setup_t clk_setup[PPC405CR_CLK_NB];
-    uint32_t sysclk;
-    uint32_t psr;
-    uint32_t cr0;
-    uint32_t cr1;
-    uint32_t jtagid;
-    uint32_t pllmr;
-    uint32_t er;
-    uint32_t fr;
-};
-
-static void ppc405cr_clk_setup (ppc405cr_cpc_t *cpc)
-{
-    uint64_t VCO_out, PLL_out;
-    uint32_t CPU_clk, TMR_clk, SDRAM_clk, PLB_clk, OPB_clk, EXT_clk, UART_clk;
-    int M, D0, D1, D2;
-
-    D0 = ((cpc->pllmr >> 26) & 0x3) + 1; /* CBDV */
-    if (cpc->pllmr & 0x80000000) {
-        D1 = (((cpc->pllmr >> 20) - 1) & 0xF) + 1; /* FBDV */
-        D2 = 8 - ((cpc->pllmr >> 16) & 0x7); /* FWDVA */
-        M = D0 * D1 * D2;
-        VCO_out = (uint64_t)cpc->sysclk * M;
-        if (VCO_out < 400000000 || VCO_out > 800000000) {
-            /* PLL cannot lock */
-            cpc->pllmr &= ~0x80000000;
-            goto bypass_pll;
-        }
-        PLL_out = VCO_out / D2;
-    } else {
-        /* Bypass PLL */
-    bypass_pll:
-        M = D0;
-        PLL_out = (uint64_t)cpc->sysclk * M;
-    }
-    CPU_clk = PLL_out;
-    if (cpc->cr1 & 0x00800000)
-        TMR_clk = cpc->sysclk; /* Should have a separate clock */
-    else
-        TMR_clk = CPU_clk;
-    PLB_clk = CPU_clk / D0;
-    SDRAM_clk = PLB_clk;
-    D0 = ((cpc->pllmr >> 10) & 0x3) + 1;
-    OPB_clk = PLB_clk / D0;
-    D0 = ((cpc->pllmr >> 24) & 0x3) + 2;
-    EXT_clk = PLB_clk / D0;
-    D0 = ((cpc->cr0 >> 1) & 0x1F) + 1;
-    UART_clk = CPU_clk / D0;
-    /* Setup CPU clocks */
-    clk_setup(&cpc->clk_setup[PPC405CR_CPU_CLK], CPU_clk);
-    /* Setup time-base clock */
-    clk_setup(&cpc->clk_setup[PPC405CR_TMR_CLK], TMR_clk);
-    /* Setup PLB clock */
-    clk_setup(&cpc->clk_setup[PPC405CR_PLB_CLK], PLB_clk);
-    /* Setup SDRAM clock */
-    clk_setup(&cpc->clk_setup[PPC405CR_SDRAM_CLK], SDRAM_clk);
-    /* Setup OPB clock */
-    clk_setup(&cpc->clk_setup[PPC405CR_OPB_CLK], OPB_clk);
-    /* Setup external clock */
-    clk_setup(&cpc->clk_setup[PPC405CR_EXT_CLK], EXT_clk);
-    /* Setup UART clock */
-    clk_setup(&cpc->clk_setup[PPC405CR_UART_CLK], UART_clk);
-}
-
-static uint32_t dcr_read_crcpc (void *opaque, int dcrn)
-{
-    ppc405cr_cpc_t *cpc;
-    uint32_t ret;
-
-    cpc = opaque;
-    switch (dcrn) {
-    case PPC405CR_CPC0_PLLMR:
-        ret = cpc->pllmr;
-        break;
-    case PPC405CR_CPC0_CR0:
-        ret = cpc->cr0;
-        break;
-    case PPC405CR_CPC0_CR1:
-        ret = cpc->cr1;
-        break;
-    case PPC405CR_CPC0_PSR:
-        ret = cpc->psr;
-        break;
-    case PPC405CR_CPC0_JTAGID:
-        ret = cpc->jtagid;
-        break;
-    case PPC405CR_CPC0_ER:
-        ret = cpc->er;
-        break;
-    case PPC405CR_CPC0_FR:
-        ret = cpc->fr;
-        break;
-    case PPC405CR_CPC0_SR:
-        ret = ~(cpc->er | cpc->fr) & 0xFFFF0000;
-        break;
-    default:
-        /* Avoid gcc warning */
-        ret = 0;
-        break;
-    }
-
-    return ret;
-}
-
-static void dcr_write_crcpc (void *opaque, int dcrn, uint32_t val)
-{
-    ppc405cr_cpc_t *cpc;
-
-    cpc = opaque;
-    switch (dcrn) {
-    case PPC405CR_CPC0_PLLMR:
-        cpc->pllmr = val & 0xFFF77C3F;
-        break;
-    case PPC405CR_CPC0_CR0:
-        cpc->cr0 = val & 0x0FFFFFFE;
-        break;
-    case PPC405CR_CPC0_CR1:
-        cpc->cr1 = val & 0x00800000;
-        break;
-    case PPC405CR_CPC0_PSR:
-        /* Read-only */
-        break;
-    case PPC405CR_CPC0_JTAGID:
-        /* Read-only */
-        break;
-    case PPC405CR_CPC0_ER:
-        cpc->er = val & 0xBFFC0000;
-        break;
-    case PPC405CR_CPC0_FR:
-        cpc->fr = val & 0xBFFC0000;
-        break;
-    case PPC405CR_CPC0_SR:
-        /* Read-only */
-        break;
-    }
-}
-
-static void ppc405cr_cpc_reset (void *opaque)
-{
-    ppc405cr_cpc_t *cpc;
-    int D;
-
-    cpc = opaque;
-    /* Compute PLLMR value from PSR settings */
-    cpc->pllmr = 0x80000000;
-    /* PFWD */
-    switch ((cpc->psr >> 30) & 3) {
-    case 0:
-        /* Bypass */
-        cpc->pllmr &= ~0x80000000;
-        break;
-    case 1:
-        /* Divide by 3 */
-        cpc->pllmr |= 5 << 16;
-        break;
-    case 2:
-        /* Divide by 4 */
-        cpc->pllmr |= 4 << 16;
-        break;
-    case 3:
-        /* Divide by 6 */
-        cpc->pllmr |= 2 << 16;
-        break;
-    }
-    /* PFBD */
-    D = (cpc->psr >> 28) & 3;
-    cpc->pllmr |= (D + 1) << 20;
-    /* PT   */
-    D = (cpc->psr >> 25) & 7;
-    switch (D) {
-    case 0x2:
-        cpc->pllmr |= 0x13;
-        break;
-    case 0x4:
-        cpc->pllmr |= 0x15;
-        break;
-    case 0x5:
-        cpc->pllmr |= 0x16;
-        break;
-    default:
-        break;
-    }
-    /* PDC  */
-    D = (cpc->psr >> 23) & 3;
-    cpc->pllmr |= D << 26;
-    /* ODP  */
-    D = (cpc->psr >> 21) & 3;
-    cpc->pllmr |= D << 10;
-    /* EBPD */
-    D = (cpc->psr >> 17) & 3;
-    cpc->pllmr |= D << 24;
-    cpc->cr0 = 0x0000003C;
-    cpc->cr1 = 0x2B0D8800;
-    cpc->er = 0x00000000;
-    cpc->fr = 0x00000000;
-    ppc405cr_clk_setup(cpc);
-}
-
-static void ppc405cr_clk_init (ppc405cr_cpc_t *cpc)
-{
-    int D;
-
-    /* XXX: this should be read from IO pins */
-    cpc->psr = 0x00000000; /* 8 bits ROM */
-    /* PFWD */
-    D = 0x2; /* Divide by 4 */
-    cpc->psr |= D << 30;
-    /* PFBD */
-    D = 0x1; /* Divide by 2 */
-    cpc->psr |= D << 28;
-    /* PDC */
-    D = 0x1; /* Divide by 2 */
-    cpc->psr |= D << 23;
-    /* PT */
-    D = 0x5; /* M = 16 */
-    cpc->psr |= D << 25;
-    /* ODP */
-    D = 0x1; /* Divide by 2 */
-    cpc->psr |= D << 21;
-    /* EBDP */
-    D = 0x2; /* Divide by 4 */
-    cpc->psr |= D << 17;
-}
-
-static void ppc405cr_cpc_init (CPUPPCState *env, clk_setup_t clk_setup[7],
-                               uint32_t sysclk)
-{
-    ppc405cr_cpc_t *cpc;
-
-    cpc = g_malloc0(sizeof(ppc405cr_cpc_t));
-    memcpy(cpc->clk_setup, clk_setup,
-           PPC405CR_CLK_NB * sizeof(clk_setup_t));
-    cpc->sysclk = sysclk;
-    cpc->jtagid = 0x42051049;
-    ppc_dcr_register(env, PPC405CR_CPC0_PSR, cpc,
-                     &dcr_read_crcpc, &dcr_write_crcpc);
-    ppc_dcr_register(env, PPC405CR_CPC0_CR0, cpc,
-                     &dcr_read_crcpc, &dcr_write_crcpc);
-    ppc_dcr_register(env, PPC405CR_CPC0_CR1, cpc,
-                     &dcr_read_crcpc, &dcr_write_crcpc);
-    ppc_dcr_register(env, PPC405CR_CPC0_JTAGID, cpc,
-                     &dcr_read_crcpc, &dcr_write_crcpc);
-    ppc_dcr_register(env, PPC405CR_CPC0_PLLMR, cpc,
-                     &dcr_read_crcpc, &dcr_write_crcpc);
-    ppc_dcr_register(env, PPC405CR_CPC0_ER, cpc,
-                     &dcr_read_crcpc, &dcr_write_crcpc);
-    ppc_dcr_register(env, PPC405CR_CPC0_FR, cpc,
-                     &dcr_read_crcpc, &dcr_write_crcpc);
-    ppc_dcr_register(env, PPC405CR_CPC0_SR, cpc,
-                     &dcr_read_crcpc, &dcr_write_crcpc);
-    ppc405cr_clk_init(cpc);
-    qemu_register_reset(ppc405cr_cpc_reset, cpc);
-}
-
-CPUPPCState *ppc405cr_init(MemoryRegion *address_space_mem,
-                        MemoryRegion ram_memories[4],
-                        hwaddr ram_bases[4],
-                        hwaddr ram_sizes[4],
-                        uint32_t sysclk, qemu_irq **picp,
-                        int do_init)
-{
-    clk_setup_t clk_setup[PPC405CR_CLK_NB];
-    qemu_irq dma_irqs[4];
-    PowerPCCPU *cpu;
-    CPUPPCState *env;
-    qemu_irq *pic, *irqs;
-
-    memset(clk_setup, 0, sizeof(clk_setup));
-    cpu = ppc4xx_init(POWERPC_CPU_TYPE_NAME("405crc"),
-                      &clk_setup[PPC405CR_CPU_CLK],
-                      &clk_setup[PPC405CR_TMR_CLK], sysclk);
-    env = &cpu->env;
-    /* Memory mapped devices registers */
-    /* PLB arbitrer */
-    ppc4xx_plb_init(env);
-    /* PLB to OPB bridge */
-    ppc4xx_pob_init(env);
-    /* OBP arbitrer */
-    ppc4xx_opba_init(0xef600600);
-    /* Universal interrupt controller */
-    irqs = g_new0(qemu_irq, PPCUIC_OUTPUT_NB);
-    irqs[PPCUIC_OUTPUT_INT] =
-        ((qemu_irq *)env->irq_inputs)[PPC40x_INPUT_INT];
-    irqs[PPCUIC_OUTPUT_CINT] =
-        ((qemu_irq *)env->irq_inputs)[PPC40x_INPUT_CINT];
-    pic = ppcuic_init(env, irqs, 0x0C0, 0, 1);
-    *picp = pic;
-    /* SDRAM controller */
-    ppc4xx_sdram_init(env, pic[14], 1, ram_memories,
-                      ram_bases, ram_sizes, do_init);
-    /* External bus controller */
-    ppc405_ebc_init(env);
-    /* DMA controller */
-    dma_irqs[0] = pic[26];
-    dma_irqs[1] = pic[25];
-    dma_irqs[2] = pic[24];
-    dma_irqs[3] = pic[23];
-    ppc405_dma_init(env, dma_irqs);
-    /* Serial ports */
-    if (serial_hd(0) != NULL) {
-        serial_mm_init(address_space_mem, 0xef600300, 0, pic[0],
-                       PPC_SERIAL_MM_BAUDBASE, serial_hd(0),
-                       DEVICE_BIG_ENDIAN);
-    }
-    if (serial_hd(1) != NULL) {
-        serial_mm_init(address_space_mem, 0xef600400, 0, pic[1],
-                       PPC_SERIAL_MM_BAUDBASE, serial_hd(1),
-                       DEVICE_BIG_ENDIAN);
-    }
-    /* IIC controller */
-    sysbus_create_simple(TYPE_PPC4xx_I2C, 0xef600500, pic[2]);
-    /* GPIO */
-    ppc405_gpio_init(0xef600700);
-    /* CPU control */
-    ppc405cr_cpc_init(env, clk_setup, sysclk);
-
-    return env;
-}
-
 /*****************************************************************************/
 /* PowerPC 405EP */
 /* CPU control */
-- 
2.20.1



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

* [PATCH v2 3/4] hw/ppc/ppc405_uc: Drop use of ppcuic_init()
  2021-01-08 17:12 [PATCH v2 0/4] hw/ppc: Convert UIC device to QOM Peter Maydell
  2021-01-08 17:12 ` [PATCH v2 1/4] hw/ppc/sam460ex: Drop use of ppcuic_init() Peter Maydell
  2021-01-08 17:12 ` [PATCH v2 2/4] hw/ppc: Delete unused ppc405cr_init() code Peter Maydell
@ 2021-01-08 17:12 ` Peter Maydell
  2021-01-08 17:12 ` [PATCH v2 4/4] hw/ppc: Remove unused ppcuic_init() Peter Maydell
  2021-01-11  9:23 ` [PATCH v2 0/4] hw/ppc: Convert UIC device to QOM David Gibson
  4 siblings, 0 replies; 7+ messages in thread
From: Peter Maydell @ 2021-01-08 17:12 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-ppc, Greg Kurz, David Gibson

Switch the ppc405_uc boards to directly creating and configuring the
UIC, rather than doing it via the old ppcuic_init() helper function.

We retain the API feature of ppc405ep_init() where it passes back
something allowing the callers to wire up devices to the UIC if
they need to, even though neither of the callsites currently makes
use of this ability -- instead of passing back the qemu_irq array
we pass back the UIC DeviceState.

This fixes a trivial Coverity-detected memory leak where
we were leaking the array of IRQs returned by ppcuic_init().

Fixes: Coverity CID 1421922
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/ppc/ppc405.h        |  2 +-
 hw/ppc/ppc405_boards.c |  8 ++---
 hw/ppc/ppc405_uc.c     | 70 +++++++++++++++++++++++++-----------------
 3 files changed, 47 insertions(+), 33 deletions(-)

diff --git a/hw/ppc/ppc405.h b/hw/ppc/ppc405.h
index e6c702f7e0d..c58f739886a 100644
--- a/hw/ppc/ppc405.h
+++ b/hw/ppc/ppc405.h
@@ -66,7 +66,7 @@ CPUPPCState *ppc405ep_init(MemoryRegion *address_space_mem,
                         MemoryRegion ram_memories[2],
                         hwaddr ram_bases[2],
                         hwaddr ram_sizes[2],
-                        uint32_t sysclk, qemu_irq **picp,
+                        uint32_t sysclk, DeviceState **uicdev,
                         int do_init);
 
 #endif /* PPC405_H */
diff --git a/hw/ppc/ppc405_boards.c b/hw/ppc/ppc405_boards.c
index b7249f21cf2..8f77887fb18 100644
--- a/hw/ppc/ppc405_boards.c
+++ b/hw/ppc/ppc405_boards.c
@@ -151,7 +151,6 @@ static void ref405ep_init(MachineState *machine)
     CPUPPCState *env;
     DeviceState *dev;
     SysBusDevice *s;
-    qemu_irq *pic;
     MemoryRegion *bios;
     MemoryRegion *sram = g_new(MemoryRegion, 1);
     ram_addr_t bdloc;
@@ -167,6 +166,7 @@ static void ref405ep_init(MachineState *machine)
     int len;
     DriveInfo *dinfo;
     MemoryRegion *sysmem = get_system_memory();
+    DeviceState *uicdev;
 
     if (machine->ram_size != mc->default_ram_size) {
         char *sz = size_to_str(mc->default_ram_size);
@@ -184,7 +184,7 @@ static void ref405ep_init(MachineState *machine)
     ram_bases[1] = 0x00000000;
     ram_sizes[1] = 0x00000000;
     env = ppc405ep_init(sysmem, ram_memories, ram_bases, ram_sizes,
-                        33333333, &pic, kernel_filename == NULL ? 0 : 1);
+                        33333333, &uicdev, kernel_filename == NULL ? 0 : 1);
     /* allocate SRAM */
     sram_size = 512 * KiB;
     memory_region_init_ram(sram, NULL, "ef405ep.sram", sram_size,
@@ -429,7 +429,6 @@ static void taihu_405ep_init(MachineState *machine)
     const char *kernel_filename = machine->kernel_filename;
     const char *initrd_filename = machine->initrd_filename;
     char *filename;
-    qemu_irq *pic;
     MemoryRegion *sysmem = get_system_memory();
     MemoryRegion *bios;
     MemoryRegion *ram_memories = g_new(MemoryRegion, 2);
@@ -440,6 +439,7 @@ static void taihu_405ep_init(MachineState *machine)
     int linux_boot;
     int fl_idx;
     DriveInfo *dinfo;
+    DeviceState *uicdev;
 
     if (machine->ram_size != mc->default_ram_size) {
         char *sz = size_to_str(mc->default_ram_size);
@@ -459,7 +459,7 @@ static void taihu_405ep_init(MachineState *machine)
                              "taihu_405ep.ram-1", machine->ram, ram_bases[1],
                              ram_sizes[1]);
     ppc405ep_init(sysmem, ram_memories, ram_bases, ram_sizes,
-                  33333333, &pic, kernel_filename == NULL ? 0 : 1);
+                  33333333, &uicdev, kernel_filename == NULL ? 0 : 1);
     /* allocate and load BIOS */
     fl_idx = 0;
 #if defined(USE_FLASH_BIOS)
diff --git a/hw/ppc/ppc405_uc.c b/hw/ppc/ppc405_uc.c
index 3e191ae4af5..fe047074a17 100644
--- a/hw/ppc/ppc405_uc.c
+++ b/hw/ppc/ppc405_uc.c
@@ -36,6 +36,9 @@
 #include "sysemu/sysemu.h"
 #include "qemu/log.h"
 #include "exec/address-spaces.h"
+#include "hw/intc/ppc-uic.h"
+#include "hw/qdev-properties.h"
+#include "qapi/error.h"
 
 //#define DEBUG_OPBA
 //#define DEBUG_SDRAM
@@ -1446,14 +1449,15 @@ CPUPPCState *ppc405ep_init(MemoryRegion *address_space_mem,
                         MemoryRegion ram_memories[2],
                         hwaddr ram_bases[2],
                         hwaddr ram_sizes[2],
-                        uint32_t sysclk, qemu_irq **picp,
+                        uint32_t sysclk, DeviceState **uicdevp,
                         int do_init)
 {
     clk_setup_t clk_setup[PPC405EP_CLK_NB], tlb_clk_setup;
     qemu_irq dma_irqs[4], gpt_irqs[5], mal_irqs[4];
     PowerPCCPU *cpu;
     CPUPPCState *env;
-    qemu_irq *pic, *irqs;
+    DeviceState *uicdev;
+    SysBusDevice *uicsbd;
 
     memset(clk_setup, 0, sizeof(clk_setup));
     /* init CPUs */
@@ -1474,59 +1478,69 @@ CPUPPCState *ppc405ep_init(MemoryRegion *address_space_mem,
     /* Initialize timers */
     ppc_booke_timers_init(cpu, sysclk, 0);
     /* Universal interrupt controller */
-    irqs = g_new0(qemu_irq, PPCUIC_OUTPUT_NB);
-    irqs[PPCUIC_OUTPUT_INT] =
-        ((qemu_irq *)env->irq_inputs)[PPC40x_INPUT_INT];
-    irqs[PPCUIC_OUTPUT_CINT] =
-        ((qemu_irq *)env->irq_inputs)[PPC40x_INPUT_CINT];
-    pic = ppcuic_init(env, irqs, 0x0C0, 0, 1);
-    *picp = pic;
+    uicdev = qdev_new(TYPE_PPC_UIC);
+    uicsbd = SYS_BUS_DEVICE(uicdev);
+
+    object_property_set_link(OBJECT(uicdev), "cpu", OBJECT(cpu),
+                             &error_fatal);
+    sysbus_realize_and_unref(uicsbd, &error_fatal);
+
+    sysbus_connect_irq(uicsbd, PPCUIC_OUTPUT_INT,
+                       ((qemu_irq *)env->irq_inputs)[PPC40x_INPUT_INT]);
+    sysbus_connect_irq(uicsbd, PPCUIC_OUTPUT_CINT,
+                       ((qemu_irq *)env->irq_inputs)[PPC40x_INPUT_CINT]);
+
+    *uicdevp = uicdev;
+
     /* SDRAM controller */
         /* XXX 405EP has no ECC interrupt */
-    ppc4xx_sdram_init(env, pic[17], 2, ram_memories,
+    ppc4xx_sdram_init(env, qdev_get_gpio_in(uicdev, 17), 2, ram_memories,
                       ram_bases, ram_sizes, do_init);
     /* External bus controller */
     ppc405_ebc_init(env);
     /* DMA controller */
-    dma_irqs[0] = pic[5];
-    dma_irqs[1] = pic[6];
-    dma_irqs[2] = pic[7];
-    dma_irqs[3] = pic[8];
+    dma_irqs[0] = qdev_get_gpio_in(uicdev, 5);
+    dma_irqs[1] = qdev_get_gpio_in(uicdev, 6);
+    dma_irqs[2] = qdev_get_gpio_in(uicdev, 7);
+    dma_irqs[3] = qdev_get_gpio_in(uicdev, 8);
     ppc405_dma_init(env, dma_irqs);
     /* IIC controller */
-    sysbus_create_simple(TYPE_PPC4xx_I2C, 0xef600500, pic[2]);
+    sysbus_create_simple(TYPE_PPC4xx_I2C, 0xef600500,
+                         qdev_get_gpio_in(uicdev, 2));
     /* GPIO */
     ppc405_gpio_init(0xef600700);
     /* Serial ports */
     if (serial_hd(0) != NULL) {
-        serial_mm_init(address_space_mem, 0xef600300, 0, pic[0],
+        serial_mm_init(address_space_mem, 0xef600300, 0,
+                       qdev_get_gpio_in(uicdev, 0),
                        PPC_SERIAL_MM_BAUDBASE, serial_hd(0),
                        DEVICE_BIG_ENDIAN);
     }
     if (serial_hd(1) != NULL) {
-        serial_mm_init(address_space_mem, 0xef600400, 0, pic[1],
+        serial_mm_init(address_space_mem, 0xef600400, 0,
+                       qdev_get_gpio_in(uicdev, 1),
                        PPC_SERIAL_MM_BAUDBASE, serial_hd(1),
                        DEVICE_BIG_ENDIAN);
     }
     /* OCM */
     ppc405_ocm_init(env);
     /* GPT */
-    gpt_irqs[0] = pic[19];
-    gpt_irqs[1] = pic[20];
-    gpt_irqs[2] = pic[21];
-    gpt_irqs[3] = pic[22];
-    gpt_irqs[4] = pic[23];
+    gpt_irqs[0] = qdev_get_gpio_in(uicdev, 19);
+    gpt_irqs[1] = qdev_get_gpio_in(uicdev, 20);
+    gpt_irqs[2] = qdev_get_gpio_in(uicdev, 21);
+    gpt_irqs[3] = qdev_get_gpio_in(uicdev, 22);
+    gpt_irqs[4] = qdev_get_gpio_in(uicdev, 23);
     ppc4xx_gpt_init(0xef600000, gpt_irqs);
     /* PCI */
-    /* Uses pic[3], pic[16], pic[18] */
+    /* Uses UIC IRQs 3, 16, 18 */
     /* MAL */
-    mal_irqs[0] = pic[11];
-    mal_irqs[1] = pic[12];
-    mal_irqs[2] = pic[13];
-    mal_irqs[3] = pic[14];
+    mal_irqs[0] = qdev_get_gpio_in(uicdev, 11);
+    mal_irqs[1] = qdev_get_gpio_in(uicdev, 12);
+    mal_irqs[2] = qdev_get_gpio_in(uicdev, 13);
+    mal_irqs[3] = qdev_get_gpio_in(uicdev, 14);
     ppc4xx_mal_init(env, 4, 2, mal_irqs);
     /* Ethernet */
-    /* Uses pic[9], pic[15], pic[17] */
+    /* Uses UIC IRQs 9, 15, 17 */
     /* CPU control */
     ppc405ep_cpc_init(env, clk_setup, sysclk);
 
-- 
2.20.1



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

* [PATCH v2 4/4] hw/ppc: Remove unused ppcuic_init()
  2021-01-08 17:12 [PATCH v2 0/4] hw/ppc: Convert UIC device to QOM Peter Maydell
                   ` (2 preceding siblings ...)
  2021-01-08 17:12 ` [PATCH v2 3/4] hw/ppc/ppc405_uc: Drop use of ppcuic_init() Peter Maydell
@ 2021-01-08 17:12 ` Peter Maydell
  2021-01-11  9:23 ` [PATCH v2 0/4] hw/ppc: Convert UIC device to QOM David Gibson
  4 siblings, 0 replies; 7+ messages in thread
From: Peter Maydell @ 2021-01-08 17:12 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-ppc, Greg Kurz, David Gibson

Now we've converted all the callsites to directly create the QOM UIC
device themselves, the ppcuic_init() function is unused and can be
removed. The enum defining PPCUIC symbolic constants can be moved
to the ppc-uic.h header where it more naturally belongs.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
Tested-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 include/hw/intc/ppc-uic.h |  7 +++++++
 include/hw/ppc/ppc4xx.h   |  9 ---------
 hw/ppc/ppc4xx_devs.c      | 38 --------------------------------------
 3 files changed, 7 insertions(+), 47 deletions(-)

diff --git a/include/hw/intc/ppc-uic.h b/include/hw/intc/ppc-uic.h
index e614e2ffd80..22dd5e5ac2c 100644
--- a/include/hw/intc/ppc-uic.h
+++ b/include/hw/intc/ppc-uic.h
@@ -47,6 +47,13 @@ OBJECT_DECLARE_SIMPLE_TYPE(PPCUIC, PPC_UIC)
 
 #define UIC_MAX_IRQ 32
 
+/* Symbolic constants for the sysbus IRQ outputs */
+enum {
+    PPCUIC_OUTPUT_INT = 0,
+    PPCUIC_OUTPUT_CINT = 1,
+    PPCUIC_OUTPUT_NB,
+};
+
 struct PPCUIC {
     /*< private >*/
     SysBusDevice parent_obj;
diff --git a/include/hw/ppc/ppc4xx.h b/include/hw/ppc/ppc4xx.h
index cc19c8da5be..980f964b5a9 100644
--- a/include/hw/ppc/ppc4xx.h
+++ b/include/hw/ppc/ppc4xx.h
@@ -33,15 +33,6 @@ PowerPCCPU *ppc4xx_init(const char *cpu_model,
                         clk_setup_t *cpu_clk, clk_setup_t *tb_clk,
                         uint32_t sysclk);
 
-/* PowerPC 4xx universal interrupt controller */
-enum {
-    PPCUIC_OUTPUT_INT = 0,
-    PPCUIC_OUTPUT_CINT = 1,
-    PPCUIC_OUTPUT_NB,
-};
-qemu_irq *ppcuic_init (CPUPPCState *env, qemu_irq *irqs,
-                       uint32_t dcr_base, int has_ssr, int has_vr);
-
 void ppc4xx_sdram_banks(MemoryRegion *ram, int nr_banks,
                         MemoryRegion ram_memories[],
                         hwaddr ram_bases[], hwaddr ram_sizes[],
diff --git a/hw/ppc/ppc4xx_devs.c b/hw/ppc/ppc4xx_devs.c
index ffe4cf43e88..fe9d4f7155e 100644
--- a/hw/ppc/ppc4xx_devs.c
+++ b/hw/ppc/ppc4xx_devs.c
@@ -77,44 +77,6 @@ PowerPCCPU *ppc4xx_init(const char *cpu_type,
     return cpu;
 }
 
-/*****************************************************************************/
-/* "Universal" Interrupt controller */
-
-qemu_irq *ppcuic_init (CPUPPCState *env, qemu_irq *irqs,
-                       uint32_t dcr_base, int has_ssr, int has_vr)
-{
-    DeviceState *uicdev = qdev_new(TYPE_PPC_UIC);
-    SysBusDevice *uicsbd = SYS_BUS_DEVICE(uicdev);
-    qemu_irq *uic_irqs;
-    int i;
-
-    qdev_prop_set_uint32(uicdev, "dcr-base", dcr_base);
-    qdev_prop_set_bit(uicdev, "use-vectors", has_vr);
-    object_property_set_link(OBJECT(uicdev), "cpu", OBJECT(env_cpu(env)),
-                             &error_fatal);
-    sysbus_realize_and_unref(uicsbd, &error_fatal);
-
-    sysbus_connect_irq(uicsbd, PPCUIC_OUTPUT_INT, irqs[PPCUIC_OUTPUT_INT]);
-    sysbus_connect_irq(uicsbd, PPCUIC_OUTPUT_CINT, irqs[PPCUIC_OUTPUT_CINT]);
-
-    /*
-     * Return an allocated array of the UIC's input IRQ lines.
-     * This is an ugly temporary API to retain compatibility with
-     * the ppcuic_init() interface from the pre-QOM-conversion UIC.
-     * None of the callers free this array, so it is leaked -- but
-     * so was the array allocated by qemu_allocate_irqs() in the
-     * old code.
-     *
-     * The callers should just instantiate the UIC and wire it up
-     * themselves rather than passing qemu_irq* in and out of this function.
-     */
-    uic_irqs = g_new0(qemu_irq, UIC_MAX_IRQ);
-    for (i = 0; i < UIC_MAX_IRQ; i++) {
-        uic_irqs[i] = qdev_get_gpio_in(uicdev, i);
-    }
-    return uic_irqs;
-}
-
 /*****************************************************************************/
 /* SDRAM controller */
 typedef struct ppc4xx_sdram_t ppc4xx_sdram_t;
-- 
2.20.1



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

* Re: [PATCH v2 1/4] hw/ppc/sam460ex: Drop use of ppcuic_init()
  2021-01-08 17:12 ` [PATCH v2 1/4] hw/ppc/sam460ex: Drop use of ppcuic_init() Peter Maydell
@ 2021-01-08 22:16   ` BALATON Zoltan
  0 siblings, 0 replies; 7+ messages in thread
From: BALATON Zoltan @ 2021-01-08 22:16 UTC (permalink / raw)
  To: Peter Maydell; +Cc: Greg Kurz, qemu-ppc, qemu-devel, David Gibson

On Fri, 8 Jan 2021, Peter Maydell wrote:
> Switch the sam460ex board to directly creating and configuring the
> UIC, rather than doing it via the old ppcuic_init() helper function.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu>

Regards,
BALATON Zoltan

> ---
> v1->v2 changes:
> * fix typo in UIC 0 CINT wiring
> * move local var declarations up
> * drop unnecessary TODO comment
> * improve comment about what the input_ints[] array is doing
> ---
> hw/ppc/sam460ex.c | 69 ++++++++++++++++++++++++++++++++++++-----------
> 1 file changed, 53 insertions(+), 16 deletions(-)
>
> diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c
> index 14e6583eb0d..45721ad6c73 100644
> --- a/hw/ppc/sam460ex.c
> +++ b/hw/ppc/sam460ex.c
> @@ -39,6 +39,7 @@
> #include "hw/usb/hcd-ehci.h"
> #include "hw/ppc/fdt.h"
> #include "hw/qdev-properties.h"
> +#include "hw/intc/ppc-uic.h"
>
> #include <libfdt.h>
>
> @@ -281,7 +282,9 @@ static void sam460ex_init(MachineState *machine)
>     hwaddr ram_bases[SDRAM_NR_BANKS] = {0};
>     hwaddr ram_sizes[SDRAM_NR_BANKS] = {0};
>     MemoryRegion *l2cache_ram = g_new(MemoryRegion, 1);
> -    qemu_irq *irqs, *uic[4];
> +    DeviceState *uic[4];
> +    qemu_irq mal_irqs[4];
> +    int i;
>     PCIBus *pci_bus;
>     PowerPCCPU *cpu;
>     CPUPPCState *env;
> @@ -312,13 +315,38 @@ static void sam460ex_init(MachineState *machine)
>     ppc4xx_plb_init(env);
>
>     /* interrupt controllers */
> -    irqs = g_new0(qemu_irq, PPCUIC_OUTPUT_NB);
> -    irqs[PPCUIC_OUTPUT_INT] = ((qemu_irq *)env->irq_inputs)[PPC40x_INPUT_INT];
> -    irqs[PPCUIC_OUTPUT_CINT] = ((qemu_irq *)env->irq_inputs)[PPC40x_INPUT_CINT];
> -    uic[0] = ppcuic_init(env, irqs, 0xc0, 0, 1);
> -    uic[1] = ppcuic_init(env, &uic[0][30], 0xd0, 0, 1);
> -    uic[2] = ppcuic_init(env, &uic[0][10], 0xe0, 0, 1);
> -    uic[3] = ppcuic_init(env, &uic[0][16], 0xf0, 0, 1);
> +    for (i = 0; i < ARRAY_SIZE(uic); i++) {
> +        SysBusDevice *sbd;
> +        /*
> +         * UICs 1, 2 and 3 are cascaded through UIC 0.
> +         * input_ints[n] is the interrupt number on UIC 0 which
> +         * the INT output of UIC n is connected to. The CINT output
> +         * of UIC n connects to input_ints[n] + 1.
> +         * The entry in input_ints[] for UIC 0 is ignored, because UIC 0's
> +         * INT and CINT outputs are connected to the CPU.
> +         */
> +        const int input_ints[] = { -1, 30, 10, 16 };
> +
> +        uic[i] = qdev_new(TYPE_PPC_UIC);
> +        sbd = SYS_BUS_DEVICE(uic[i]);
> +
> +        qdev_prop_set_uint32(uic[i], "dcr-base", 0xc0 + i * 0x10);
> +        object_property_set_link(OBJECT(uic[i]), "cpu", OBJECT(cpu),
> +                                 &error_fatal);
> +        sysbus_realize_and_unref(sbd, &error_fatal);
> +
> +        if (i == 0) {
> +            sysbus_connect_irq(sbd, PPCUIC_OUTPUT_INT,
> +                               ((qemu_irq *)env->irq_inputs)[PPC40x_INPUT_INT]);
> +            sysbus_connect_irq(sbd, PPCUIC_OUTPUT_CINT,
> +                               ((qemu_irq *)env->irq_inputs)[PPC40x_INPUT_CINT]);
> +        } else {
> +            sysbus_connect_irq(sbd, PPCUIC_OUTPUT_INT,
> +                               qdev_get_gpio_in(uic[0], input_ints[i]));
> +            sysbus_connect_irq(sbd, PPCUIC_OUTPUT_CINT,
> +                               qdev_get_gpio_in(uic[0], input_ints[i] + 1));
> +        }
> +    }
>
>     /* SDRAM controller */
>     /* put all RAM on first bank because board has one slot
> @@ -331,7 +359,8 @@ static void sam460ex_init(MachineState *machine)
>                       ram_bases, ram_sizes, 1);
>
>     /* IIC controllers and devices */
> -    dev = sysbus_create_simple(TYPE_PPC4xx_I2C, 0x4ef600700, uic[0][2]);
> +    dev = sysbus_create_simple(TYPE_PPC4xx_I2C, 0x4ef600700,
> +                               qdev_get_gpio_in(uic[0], 2));
>     i2c = PPC4xx_I2C(dev)->bus;
>     /* SPD EEPROM on RAM module */
>     spd_data = spd_data_generate(ram_sizes[0] < 128 * MiB ? DDR : DDR2,
> @@ -341,7 +370,8 @@ static void sam460ex_init(MachineState *machine)
>     /* RTC */
>     i2c_slave_create_simple(i2c, "m41t80", 0x68);
>
> -    dev = sysbus_create_simple(TYPE_PPC4xx_I2C, 0x4ef600800, uic[0][3]);
> +    dev = sysbus_create_simple(TYPE_PPC4xx_I2C, 0x4ef600800,
> +                               qdev_get_gpio_in(uic[0], 3));
>
>     /* External bus controller */
>     ppc405_ebc_init(env);
> @@ -356,7 +386,10 @@ static void sam460ex_init(MachineState *machine)
>     ppc4xx_sdr_init(env);
>
>     /* MAL */
> -    ppc4xx_mal_init(env, 4, 16, &uic[2][3]);
> +    for (i = 0; i < ARRAY_SIZE(mal_irqs); i++) {
> +        mal_irqs[0] = qdev_get_gpio_in(uic[2], 3 + i);
> +    }
> +    ppc4xx_mal_init(env, 4, 16, mal_irqs);
>
>     /* DMA */
>     ppc4xx_dma_init(env, 0x200);
> @@ -369,21 +402,23 @@ static void sam460ex_init(MachineState *machine)
>     memory_region_add_subregion(address_space_mem, 0x400000000LL, l2cache_ram);
>
>     /* USB */
> -    sysbus_create_simple(TYPE_PPC4xx_EHCI, 0x4bffd0400, uic[2][29]);
> +    sysbus_create_simple(TYPE_PPC4xx_EHCI, 0x4bffd0400,
> +                         qdev_get_gpio_in(uic[2], 29));
>     dev = qdev_new("sysbus-ohci");
>     qdev_prop_set_string(dev, "masterbus", "usb-bus.0");
>     qdev_prop_set_uint32(dev, "num-ports", 6);
>     sbdev = SYS_BUS_DEVICE(dev);
>     sysbus_realize_and_unref(sbdev, &error_fatal);
>     sysbus_mmio_map(sbdev, 0, 0x4bffd0000);
> -    sysbus_connect_irq(sbdev, 0, uic[2][30]);
> +    sysbus_connect_irq(sbdev, 0, qdev_get_gpio_in(uic[2], 30));
>     usb_create_simple(usb_bus_find(-1), "usb-kbd");
>     usb_create_simple(usb_bus_find(-1), "usb-mouse");
>
>     /* PCI bus */
>     ppc460ex_pcie_init(env);
>     /* All PCI irqs are connected to the same UIC pin (cf. UBoot source) */
> -    dev = sysbus_create_simple("ppc440-pcix-host", 0xc0ec00000, uic[1][0]);
> +    dev = sysbus_create_simple("ppc440-pcix-host", 0xc0ec00000,
> +                               qdev_get_gpio_in(uic[1], 0));
>     pci_bus = (PCIBus *)qdev_get_child_bus(dev, "pci.0");
>     if (!pci_bus) {
>         error_report("couldn't create PCI controller!");
> @@ -405,12 +440,14 @@ static void sam460ex_init(MachineState *machine)
>     /* SoC has 4 UARTs
>      * but board has only one wired and two are present in fdt */
>     if (serial_hd(0) != NULL) {
> -        serial_mm_init(address_space_mem, 0x4ef600300, 0, uic[1][1],
> +        serial_mm_init(address_space_mem, 0x4ef600300, 0,
> +                       qdev_get_gpio_in(uic[1], 1),
>                        PPC_SERIAL_MM_BAUDBASE, serial_hd(0),
>                        DEVICE_BIG_ENDIAN);
>     }
>     if (serial_hd(1) != NULL) {
> -        serial_mm_init(address_space_mem, 0x4ef600400, 0, uic[0][1],
> +        serial_mm_init(address_space_mem, 0x4ef600400, 0,
> +                       qdev_get_gpio_in(uic[0], 1),
>                        PPC_SERIAL_MM_BAUDBASE, serial_hd(1),
>                        DEVICE_BIG_ENDIAN);
>     }
>


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

* Re: [PATCH v2 0/4] hw/ppc: Convert UIC device to QOM
  2021-01-08 17:12 [PATCH v2 0/4] hw/ppc: Convert UIC device to QOM Peter Maydell
                   ` (3 preceding siblings ...)
  2021-01-08 17:12 ` [PATCH v2 4/4] hw/ppc: Remove unused ppcuic_init() Peter Maydell
@ 2021-01-11  9:23 ` David Gibson
  4 siblings, 0 replies; 7+ messages in thread
From: David Gibson @ 2021-01-11  9:23 UTC (permalink / raw)
  To: Peter Maydell; +Cc: qemu-ppc, qemu-devel, Greg Kurz

[-- Attachment #1: Type: text/plain, Size: 967 bytes --]

On Fri, Jan 08, 2021 at 05:12:08PM +0000, Peter Maydell wrote:
> This patchseries converts the PPC UIC "Universal Interrupt
> Controller" to a QOM device.  My main reason for doing it is that
> this fixes a couple of long-standing trivial Coverity issues -- the
> current ppcuic_init() function allocates an array of qemu_irqs which
> the callers then leak.  (The leak is trivial because it happens once
> when QEMU starts.)
> 
> The first half of v1 is in master now, so this is just a
> respin of the last four patches.
> 
> Changes v1->v2:
>  * fixed the bug in the sam460ex conversion handling of CINT
>    (tested that this now boots AROS OK to a desktop)
>  * other minor code style tweaks to patch 1 as per review

Applied to ppc-for-6.0, thanks.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

end of thread, other threads:[~2021-01-11 11:06 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-08 17:12 [PATCH v2 0/4] hw/ppc: Convert UIC device to QOM Peter Maydell
2021-01-08 17:12 ` [PATCH v2 1/4] hw/ppc/sam460ex: Drop use of ppcuic_init() Peter Maydell
2021-01-08 22:16   ` BALATON Zoltan
2021-01-08 17:12 ` [PATCH v2 2/4] hw/ppc: Delete unused ppc405cr_init() code Peter Maydell
2021-01-08 17:12 ` [PATCH v2 3/4] hw/ppc/ppc405_uc: Drop use of ppcuic_init() Peter Maydell
2021-01-08 17:12 ` [PATCH v2 4/4] hw/ppc: Remove unused ppcuic_init() Peter Maydell
2021-01-11  9:23 ` [PATCH v2 0/4] hw/ppc: Convert UIC device to QOM David Gibson

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.