qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 1/2] hw/intc/grlib_irqmp: add support for extended interrupts
@ 2024-09-21 16:43 Nikita Shushura
  2024-09-21 16:43 ` [PATCH v2 2/2] hw/sparc/leon3: add second uart with extended interrupt usage Nikita Shushura
  2024-09-23 11:20 ` [PATCH v2 1/2] hw/intc/grlib_irqmp: add support for extended interrupts Clément Chigot
  0 siblings, 2 replies; 4+ messages in thread
From: Nikita Shushura @ 2024-09-21 16:43 UTC (permalink / raw)
  To: qemu-devel; +Cc: Nikita Shushura, Clément Chigot, Frederic Konrad

Signed-off-by: Nikita Shushura <me@nikitashushura.com>
---
 hw/intc/grlib_irqmp.c | 69 +++++++++++++++++++++++++++++++------------
 1 file changed, 50 insertions(+), 19 deletions(-)

diff --git a/hw/intc/grlib_irqmp.c b/hw/intc/grlib_irqmp.c
index 37ac63fd80..e9414c054a 100644
--- a/hw/intc/grlib_irqmp.c
+++ b/hw/intc/grlib_irqmp.c
@@ -1,8 +1,6 @@
 /*
  * QEMU GRLIB IRQMP Emulator
  *
- * (Extended interrupt not supported)
- *
  * SPDX-License-Identifier: MIT
  *
  * Copyright (c) 2010-2024 AdaCore
@@ -38,25 +36,29 @@
 #include "qemu/module.h"
 #include "qom/object.h"
 
-#define IRQMP_MAX_CPU 16
-#define IRQMP_REG_SIZE 256      /* Size of memory mapped registers */
+#define IRQMP_MAX_CPU (16)
+#define IRQMP_REG_SIZE (256)    /* Size of memory mapped registers */
 
 /* Memory mapped register offsets */
-#define LEVEL_OFFSET     0x00
-#define PENDING_OFFSET   0x04
-#define FORCE0_OFFSET    0x08
-#define CLEAR_OFFSET     0x0C
-#define MP_STATUS_OFFSET 0x10
-#define BROADCAST_OFFSET 0x14
-#define MASK_OFFSET      0x40
-#define FORCE_OFFSET     0x80
-#define EXTENDED_OFFSET  0xC0
+#define LEVEL_OFFSET     (0x00)
+#define PENDING_OFFSET   (0x04)
+#define FORCE0_OFFSET    (0x08)
+#define CLEAR_OFFSET     (0x0C)
+#define MP_STATUS_OFFSET (0x10)
+#define BROADCAST_OFFSET (0x14)
+#define MASK_OFFSET      (0x40)
+#define FORCE_OFFSET     (0x80)
+#define EXTENDED_OFFSET  (0xC0)
 
 /* Multiprocessor Status Register  */
 #define MP_STATUS_CPU_STATUS_MASK ((1 << IRQMP_MAX_CPU)-2)
-#define MP_STATUS_NCPU_SHIFT      28
+#define MP_STATUS_NCPU_SHIFT      (28)
+#define MP_STATUS_EIRQ_OFFSET     (16)
+
+#define MAX_PILS_STD (16)
+#define MAX_PILS_EXT (32)
 
-#define MAX_PILS 16
+#define DEFAULT_EIRQ (12)
 
 OBJECT_DECLARE_SIMPLE_TYPE(IRQMP, GRLIB_IRQMP)
 
@@ -68,6 +70,7 @@ struct IRQMP {
     MemoryRegion iomem;
 
     unsigned int ncpus;
+    unsigned int eirq;
     IRQMPState *state;
     qemu_irq start_signal[IRQMP_MAX_CPU];
     qemu_irq irq[IRQMP_MAX_CPU];
@@ -89,13 +92,26 @@ struct IRQMPState {
 
 static void grlib_irqmp_check_irqs(IRQMPState *state)
 {
-    int i;
+    int i, j;
 
     assert(state != NULL);
     assert(state->parent != NULL);
 
     for (i = 0; i < state->parent->ncpus; i++) {
         uint32_t pend = (state->pending | state->force[i]) & state->mask[i];
+
+        /*
+         * Checks is pending interrupt extended.
+         * If so, sets pending to EIRQ and acknowledges
+         * extended interrupt
+         */
+        for (j = MAX_PILS_STD; j < MAX_PILS_EXT; j++) {
+            if ((pend & (1 << j)) != 0) {
+                pend = (1 << state->parent->eirq);
+                state->extended[i] = (j & 0xffff);
+            }
+        }
+
         uint32_t level0 = pend & ~state->level;
         uint32_t level1 = pend &  state->level;
 
@@ -110,6 +126,10 @@ static void grlib_irqmp_check_irqs(IRQMPState *state)
 static void grlib_irqmp_ack_mask(IRQMPState *state, unsigned int cpu,
                                  uint32_t mask)
 {
+    if ((mask & (1 << state->parent->eirq)) != 0) {
+        mask |= (1 << state->extended[cpu]);
+    }
+
     /* Clear registers */
     state->pending  &= ~mask;
     state->force[cpu] &= ~mask;
@@ -144,7 +164,6 @@ static void grlib_irqmp_set_irq(void *opaque, int irq, int level)
     assert(s         != NULL);
     assert(s->parent != NULL);
 
-
     if (level) {
         trace_grlib_irqmp_set_irq(irq);
 
@@ -278,6 +297,9 @@ static void grlib_irqmp_write(void *opaque, hwaddr addr,
                 state->mpstatus &= ~(1 << i);
             }
         }
+
+        /* Writing EIRQ number */
+        state->mpstatus |= (state->parent->eirq << MP_STATUS_EIRQ_OFFSET);
         return;
 
     case BROADCAST_OFFSET:
@@ -345,7 +367,8 @@ static void grlib_irqmp_reset(DeviceState *d)
     memset(irqmp->state, 0, sizeof *irqmp->state);
     irqmp->state->parent = irqmp;
     irqmp->state->mpstatus = ((irqmp->ncpus - 1) << MP_STATUS_NCPU_SHIFT) |
-        ((1 << irqmp->ncpus) - 2);
+        ((1 << irqmp->ncpus) - 2) |
+        (irqmp->eirq << MP_STATUS_EIRQ_OFFSET);
 }
 
 static void grlib_irqmp_realize(DeviceState *dev, Error **errp)
@@ -359,7 +382,14 @@ static void grlib_irqmp_realize(DeviceState *dev, Error **errp)
         return;
     }
 
-    qdev_init_gpio_in(dev, grlib_irqmp_set_irq, MAX_PILS);
+    if ((!irqmp->eirq) || (irqmp->eirq >= MAX_PILS_STD)) {
+        error_setg(errp, "Invalid eirq properties: "
+                   "%u, must be 0 < eirq < %u.", irqmp->eirq,
+                   MAX_PILS_STD);
+        return;
+    }
+
+    qdev_init_gpio_in(dev, grlib_irqmp_set_irq, MAX_PILS_EXT);
 
     /*
      * Transitionning from 0 to 1 starts the CPUs. The opposite can't
@@ -378,6 +408,7 @@ static void grlib_irqmp_realize(DeviceState *dev, Error **errp)
 
 static Property grlib_irqmp_properties[] = {
     DEFINE_PROP_UINT32("ncpus", IRQMP, ncpus, 1),
+    DEFINE_PROP_UINT32("eirq", IRQMP, eirq, DEFAULT_EIRQ),
     DEFINE_PROP_END_OF_LIST(),
 };
 
-- 
2.46.1




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

* [PATCH v2 2/2] hw/sparc/leon3: add second uart with extended interrupt usage
  2024-09-21 16:43 [PATCH v2 1/2] hw/intc/grlib_irqmp: add support for extended interrupts Nikita Shushura
@ 2024-09-21 16:43 ` Nikita Shushura
  2024-09-23 11:21   ` Clément Chigot
  2024-09-23 11:20 ` [PATCH v2 1/2] hw/intc/grlib_irqmp: add support for extended interrupts Clément Chigot
  1 sibling, 1 reply; 4+ messages in thread
From: Nikita Shushura @ 2024-09-21 16:43 UTC (permalink / raw)
  To: qemu-devel
  Cc: Nikita Shushura, Clément Chigot, Frederic Konrad,
	Mark Cave-Ayland, Artyom Tarasenko

Signed-off-by: Nikita Shushura <me@nikitashushura.com>
---
 hw/sparc/leon3.c | 63 +++++++++++++++++++++++++++++++++++-------------
 1 file changed, 46 insertions(+), 17 deletions(-)

diff --git a/hw/sparc/leon3.c b/hw/sparc/leon3.c
index 6aaa04cb19..c559854e5e 100644
--- a/hw/sparc/leon3.c
+++ b/hw/sparc/leon3.c
@@ -54,10 +54,14 @@
 #define LEON3_PROM_OFFSET    (0x00000000)
 #define LEON3_RAM_OFFSET     (0x40000000)
 
-#define MAX_CPUS  4
+#define MAX_CPUS  (4)
+#define LEON3_EIRQ (12)
 
-#define LEON3_UART_OFFSET  (0x80000100)
-#define LEON3_UART_IRQ     (3)
+#define LEON3_UART0_OFFSET  (0x80000100)
+#define LEON3_UART0_IRQ     (2)
+
+#define LEON3_UART1_OFFSET  (0x80100100)
+#define LEON3_UART1_IRQ     (17)
 
 #define LEON3_IRQMP_OFFSET (0x80000200)
 
@@ -65,7 +69,8 @@
 #define LEON3_TIMER_IRQ    (6)
 #define LEON3_TIMER_COUNT  (2)
 
-#define LEON3_APB_PNP_OFFSET (0x800FF000)
+#define LEON3_APB1_PNP_OFFSET (0x800FF000)
+#define LEON3_APB2_PNP_OFFSET (0x801FF000)
 #define LEON3_AHB_PNP_OFFSET (0xFFFFF000)
 
 typedef struct ResetData {
@@ -122,7 +127,8 @@ static void write_bootloader(void *ptr, hwaddr kernel_addr)
 
     /* Initialize the UARTs                                        */
     /* *UART_CONTROL = UART_RECEIVE_ENABLE | UART_TRANSMIT_ENABLE; */
-    p = gen_store_u32(p, 0x80000108, 3);
+    p = gen_store_u32(p, LEON3_UART0_OFFSET + 0x8, 3);
+    p = gen_store_u32(p, LEON3_UART1_OFFSET + 0x8, 3);
 
     /* Initialize the TIMER 0                                      */
     /* *GPTIMER_SCALER_RELOAD = 40 - 1;                            */
@@ -271,7 +277,8 @@ static void leon3_generic_hw_init(MachineState *machine)
     DeviceState *dev, *irqmpdev;
     int i;
     AHBPnp *ahb_pnp;
-    APBPnp *apb_pnp;
+    APBPnp *apb1_pnp;
+    APBPnp *apb2_pnp;
 
     reset_info = g_malloc0(sizeof(ResetData));
 
@@ -298,10 +305,19 @@ static void leon3_generic_hw_init(MachineState *machine)
                             GRLIB_LEON3_DEV, GRLIB_AHB_MASTER,
                             GRLIB_CPU_AREA);
 
-    apb_pnp = GRLIB_APB_PNP(qdev_new(TYPE_GRLIB_APB_PNP));
-    sysbus_realize_and_unref(SYS_BUS_DEVICE(apb_pnp), &error_fatal);
-    sysbus_mmio_map(SYS_BUS_DEVICE(apb_pnp), 0, LEON3_APB_PNP_OFFSET);
-    grlib_ahb_pnp_add_entry(ahb_pnp, LEON3_APB_PNP_OFFSET, 0xFFF,
+    /* Initialize APB1 */
+    apb1_pnp = GRLIB_APB_PNP(qdev_new(TYPE_GRLIB_APB_PNP));
+    sysbus_realize_and_unref(SYS_BUS_DEVICE(apb1_pnp), &error_fatal);
+    sysbus_mmio_map(SYS_BUS_DEVICE(apb1_pnp), 0, LEON3_APB1_PNP_OFFSET);
+    grlib_ahb_pnp_add_entry(ahb_pnp, LEON3_APB1_PNP_OFFSET, 0xFFF,
+                            GRLIB_VENDOR_GAISLER, GRLIB_APBMST_DEV,
+                            GRLIB_AHB_SLAVE, GRLIB_AHBMEM_AREA);
+
+    /* Initialize APB2 */
+    apb2_pnp = GRLIB_APB_PNP(qdev_new(TYPE_GRLIB_APB_PNP));
+    sysbus_realize_and_unref(SYS_BUS_DEVICE(apb2_pnp), &error_fatal);
+    sysbus_mmio_map(SYS_BUS_DEVICE(apb2_pnp), 0, LEON3_APB2_PNP_OFFSET);
+    grlib_ahb_pnp_add_entry(ahb_pnp, LEON3_APB2_PNP_OFFSET, 0xFFF,
                             GRLIB_VENDOR_GAISLER, GRLIB_APBMST_DEV,
                             GRLIB_AHB_SLAVE, GRLIB_AHBMEM_AREA);
 
@@ -309,6 +325,8 @@ static void leon3_generic_hw_init(MachineState *machine)
     irqmpdev = qdev_new(TYPE_GRLIB_IRQMP);
     object_property_set_int(OBJECT(irqmpdev), "ncpus", machine->smp.cpus,
                             &error_fatal);
+    /*object_property_set_int(OBJECT(irqmpdev), "eirq", LEON3_EIRQ,*/
+    /*                        &error_fatal);*/
     sysbus_realize_and_unref(SYS_BUS_DEVICE(irqmpdev), &error_fatal);
 
     for (i = 0; i < machine->smp.cpus; i++) {
@@ -325,7 +343,7 @@ static void leon3_generic_hw_init(MachineState *machine)
     }
 
     sysbus_mmio_map(SYS_BUS_DEVICE(irqmpdev), 0, LEON3_IRQMP_OFFSET);
-    grlib_apb_pnp_add_entry(apb_pnp, LEON3_IRQMP_OFFSET, 0xFFF,
+    grlib_apb_pnp_add_entry(apb1_pnp, LEON3_IRQMP_OFFSET, 0xFFF,
                             GRLIB_VENDOR_GAISLER, GRLIB_IRQMP_DEV,
                             2, 0, GRLIB_APBIO_AREA);
 
@@ -417,20 +435,31 @@ static void leon3_generic_hw_init(MachineState *machine)
                            qdev_get_gpio_in(irqmpdev, LEON3_TIMER_IRQ + i));
     }
 
-    grlib_apb_pnp_add_entry(apb_pnp, LEON3_TIMER_OFFSET, 0xFFF,
+    grlib_apb_pnp_add_entry(apb1_pnp, LEON3_TIMER_OFFSET, 0xFFF,
                             GRLIB_VENDOR_GAISLER, GRLIB_GPTIMER_DEV,
                             0, LEON3_TIMER_IRQ, GRLIB_APBIO_AREA);
 
-    /* Allocate uart */
+    /* Allocate UART0 */
     dev = qdev_new(TYPE_GRLIB_APB_UART);
     qdev_prop_set_chr(dev, "chrdev", serial_hd(0));
     sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
-    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, LEON3_UART_OFFSET);
+    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, LEON3_UART0_OFFSET);
+    sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0,
+                       qdev_get_gpio_in(irqmpdev, LEON3_UART0_IRQ));
+    grlib_apb_pnp_add_entry(apb1_pnp, LEON3_UART0_OFFSET, 0xFFF,
+                            GRLIB_VENDOR_GAISLER, GRLIB_APBUART_DEV, 1,
+                            LEON3_UART0_IRQ, GRLIB_APBIO_AREA);
+
+    /* Allocate UART1 */
+    dev = qdev_new(TYPE_GRLIB_APB_UART);
+    qdev_prop_set_chr(dev, "chrdev", serial_hd(1));
+    sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
+    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, LEON3_UART1_OFFSET);
     sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0,
-                       qdev_get_gpio_in(irqmpdev, LEON3_UART_IRQ));
-    grlib_apb_pnp_add_entry(apb_pnp, LEON3_UART_OFFSET, 0xFFF,
+                       qdev_get_gpio_in(irqmpdev, LEON3_UART1_IRQ));
+    grlib_apb_pnp_add_entry(apb1_pnp, LEON3_UART1_OFFSET, 0xFFF,
                             GRLIB_VENDOR_GAISLER, GRLIB_APBUART_DEV, 1,
-                            LEON3_UART_IRQ, GRLIB_APBIO_AREA);
+                            LEON3_UART1_IRQ, GRLIB_APBIO_AREA);
 }
 
 static void leon3_generic_machine_init(MachineClass *mc)
-- 
2.46.1




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

* Re: [PATCH v2 1/2] hw/intc/grlib_irqmp: add support for extended interrupts
  2024-09-21 16:43 [PATCH v2 1/2] hw/intc/grlib_irqmp: add support for extended interrupts Nikita Shushura
  2024-09-21 16:43 ` [PATCH v2 2/2] hw/sparc/leon3: add second uart with extended interrupt usage Nikita Shushura
@ 2024-09-23 11:20 ` Clément Chigot
  1 sibling, 0 replies; 4+ messages in thread
From: Clément Chigot @ 2024-09-23 11:20 UTC (permalink / raw)
  To: me; +Cc: qemu-devel, Frederic Konrad

On Sat, Sep 21, 2024 at 6:43 PM Nikita Shushura <me@nikitashushura.com> wrote:
>
> Signed-off-by: Nikita Shushura <me@nikitashushura.com>

Additionally to inlined comments:
 - there are a few "extended (not supported)" you can now remove.
 - I think the extended part in "grlib_irqmp_write" is still wrong,
the extended register being read-only.

> ---
>  hw/intc/grlib_irqmp.c | 69 +++++++++++++++++++++++++++++++------------
>  1 file changed, 50 insertions(+), 19 deletions(-)
>
> diff --git a/hw/intc/grlib_irqmp.c b/hw/intc/grlib_irqmp.c
> index 37ac63fd80..e9414c054a 100644
> --- a/hw/intc/grlib_irqmp.c
> +++ b/hw/intc/grlib_irqmp.c
> @@ -1,8 +1,6 @@
>  /*
>   * QEMU GRLIB IRQMP Emulator
>   *
> - * (Extended interrupt not supported)
> - *
>   * SPDX-License-Identifier: MIT
>   *
>   * Copyright (c) 2010-2024 AdaCore
> @@ -38,25 +36,29 @@
>  #include "qemu/module.h"
>  #include "qom/object.h"
>
> -#define IRQMP_MAX_CPU 16
> -#define IRQMP_REG_SIZE 256      /* Size of memory mapped registers */
> +#define IRQMP_MAX_CPU (16)
> +#define IRQMP_REG_SIZE (256)    /* Size of memory mapped registers */
>
>  /* Memory mapped register offsets */
> -#define LEVEL_OFFSET     0x00
> -#define PENDING_OFFSET   0x04
> -#define FORCE0_OFFSET    0x08
> -#define CLEAR_OFFSET     0x0C
> -#define MP_STATUS_OFFSET 0x10
> -#define BROADCAST_OFFSET 0x14
> -#define MASK_OFFSET      0x40
> -#define FORCE_OFFSET     0x80
> -#define EXTENDED_OFFSET  0xC0
> +#define LEVEL_OFFSET     (0x00)
> +#define PENDING_OFFSET   (0x04)
> +#define FORCE0_OFFSET    (0x08)
> +#define CLEAR_OFFSET     (0x0C)
> +#define MP_STATUS_OFFSET (0x10)
> +#define BROADCAST_OFFSET (0x14)
> +#define MASK_OFFSET      (0x40)
> +#define FORCE_OFFSET     (0x80)
> +#define EXTENDED_OFFSET  (0xC0)
>
>  /* Multiprocessor Status Register  */
>  #define MP_STATUS_CPU_STATUS_MASK ((1 << IRQMP_MAX_CPU)-2)
> -#define MP_STATUS_NCPU_SHIFT      28
> +#define MP_STATUS_NCPU_SHIFT      (28)
> +#define MP_STATUS_EIRQ_OFFSET     (16)
> +
> +#define MAX_PILS_STD (16)
> +#define MAX_PILS_EXT (32)
>
> -#define MAX_PILS 16
> +#define DEFAULT_EIRQ (12)
>
>  OBJECT_DECLARE_SIMPLE_TYPE(IRQMP, GRLIB_IRQMP)
>
> @@ -68,6 +70,7 @@ struct IRQMP {
>      MemoryRegion iomem;
>
>      unsigned int ncpus;
> +    unsigned int eirq;
>      IRQMPState *state;
>      qemu_irq start_signal[IRQMP_MAX_CPU];
>      qemu_irq irq[IRQMP_MAX_CPU];
> @@ -89,13 +92,26 @@ struct IRQMPState {
>
>  static void grlib_irqmp_check_irqs(IRQMPState *state)
>  {
> -    int i;
> +    int i, j;
>
>      assert(state != NULL);
>      assert(state->parent != NULL);
>
>      for (i = 0; i < state->parent->ncpus; i++) {
>          uint32_t pend = (state->pending | state->force[i]) & state->mask[i];
> +
> +        /*
> +         * Checks is pending interrupt extended.

s/is/if

> +         * If so, sets pending to EIRQ and acknowledges
> +         * extended interrupt
> +         */
> +        for (j = MAX_PILS_STD; j < MAX_PILS_EXT; j++) {
> +            if ((pend & (1 << j)) != 0) {
> +                pend = (1 << state->parent->eirq);
> +                state->extended[i] = (j & 0xffff);

You're writing 1 bit too far. It should be ((j>>1) & 0xf).
Moreover, what's happening when two extended interrupts are both
pending ? Here, only the last one is taken into account

> +            }
> +        }
> +
>          uint32_t level0 = pend & ~state->level;
>          uint32_t level1 = pend &  state->level;
>
> @@ -110,6 +126,10 @@ static void grlib_irqmp_check_irqs(IRQMPState *state)
>  static void grlib_irqmp_ack_mask(IRQMPState *state, unsigned int cpu,
>                                   uint32_t mask)
>  {
> +    if ((mask & (1 << state->parent->eirq)) != 0) {
> +        mask |= (1 << state->extended[cpu]);
> +    }
> +
>      /* Clear registers */
>      state->pending  &= ~mask;
>      state->force[cpu] &= ~mask;
> @@ -144,7 +164,6 @@ static void grlib_irqmp_set_irq(void *opaque, int irq, int level)
>      assert(s         != NULL);
>      assert(s->parent != NULL);
>
> -
>      if (level) {
>          trace_grlib_irqmp_set_irq(irq);
>
> @@ -278,6 +297,9 @@ static void grlib_irqmp_write(void *opaque, hwaddr addr,
>                  state->mpstatus &= ~(1 << i);
>              }
>          }
> +
> +        /* Writing EIRQ number */
> +        state->mpstatus |= (state->parent->eirq << MP_STATUS_EIRQ_OFFSET);
>          return;
>
>      case BROADCAST_OFFSET:
> @@ -345,7 +367,8 @@ static void grlib_irqmp_reset(DeviceState *d)
>      memset(irqmp->state, 0, sizeof *irqmp->state);
>      irqmp->state->parent = irqmp;
>      irqmp->state->mpstatus = ((irqmp->ncpus - 1) << MP_STATUS_NCPU_SHIFT) |
> -        ((1 << irqmp->ncpus) - 2);
> +        ((1 << irqmp->ncpus) - 2) |
> +        (irqmp->eirq << MP_STATUS_EIRQ_OFFSET);
>  }
>
>  static void grlib_irqmp_realize(DeviceState *dev, Error **errp)
> @@ -359,7 +382,14 @@ static void grlib_irqmp_realize(DeviceState *dev, Error **errp)
>          return;
>      }
>
> -    qdev_init_gpio_in(dev, grlib_irqmp_set_irq, MAX_PILS);
> +    if ((!irqmp->eirq) || (irqmp->eirq >= MAX_PILS_STD)) {
> +        error_setg(errp, "Invalid eirq properties: "
> +                   "%u, must be 0 < eirq < %u.", irqmp->eirq,
> +                   MAX_PILS_STD);
> +        return;
> +    }
> +
> +    qdev_init_gpio_in(dev, grlib_irqmp_set_irq, MAX_PILS_EXT);
>
>      /*
>       * Transitionning from 0 to 1 starts the CPUs. The opposite can't
> @@ -378,6 +408,7 @@ static void grlib_irqmp_realize(DeviceState *dev, Error **errp)
>
>  static Property grlib_irqmp_properties[] = {
>      DEFINE_PROP_UINT32("ncpus", IRQMP, ncpus, 1),
> +    DEFINE_PROP_UINT32("eirq", IRQMP, eirq, DEFAULT_EIRQ),
>      DEFINE_PROP_END_OF_LIST(),
>  };
>
> --
> 2.46.1
>
>


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

* Re: [PATCH v2 2/2] hw/sparc/leon3: add second uart with extended interrupt usage
  2024-09-21 16:43 ` [PATCH v2 2/2] hw/sparc/leon3: add second uart with extended interrupt usage Nikita Shushura
@ 2024-09-23 11:21   ` Clément Chigot
  0 siblings, 0 replies; 4+ messages in thread
From: Clément Chigot @ 2024-09-23 11:21 UTC (permalink / raw)
  To: me; +Cc: qemu-devel, Frederic Konrad, Mark Cave-Ayland, Artyom Tarasenko

On Sat, Sep 21, 2024 at 6:43 PM Nikita Shushura <me@nikitashushura.com> wrote:
>
> Signed-off-by: Nikita Shushura <me@nikitashushura.com>

Mostly OK just a few comments.

> ---
>  hw/sparc/leon3.c | 63 +++++++++++++++++++++++++++++++++++-------------
>  1 file changed, 46 insertions(+), 17 deletions(-)
>
> diff --git a/hw/sparc/leon3.c b/hw/sparc/leon3.c
> index 6aaa04cb19..c559854e5e 100644
> --- a/hw/sparc/leon3.c
> +++ b/hw/sparc/leon3.c
> @@ -54,10 +54,14 @@
>  #define LEON3_PROM_OFFSET    (0x00000000)
>  #define LEON3_RAM_OFFSET     (0x40000000)
>
> -#define MAX_CPUS  4
> +#define MAX_CPUS  (4)
> +#define LEON3_EIRQ (12)
>
> -#define LEON3_UART_OFFSET  (0x80000100)
> -#define LEON3_UART_IRQ     (3)
> +#define LEON3_UART0_OFFSET  (0x80000100)
> +#define LEON3_UART0_IRQ     (2)
> +
> +#define LEON3_UART1_OFFSET  (0x80100100)
> +#define LEON3_UART1_IRQ     (17)
>
>  #define LEON3_IRQMP_OFFSET (0x80000200)
>
> @@ -65,7 +69,8 @@
>  #define LEON3_TIMER_IRQ    (6)
>  #define LEON3_TIMER_COUNT  (2)
>
> -#define LEON3_APB_PNP_OFFSET (0x800FF000)
> +#define LEON3_APB1_PNP_OFFSET (0x800FF000)
> +#define LEON3_APB2_PNP_OFFSET (0x801FF000)
>  #define LEON3_AHB_PNP_OFFSET (0xFFFFF000)
>
>  typedef struct ResetData {
> @@ -122,7 +127,8 @@ static void write_bootloader(void *ptr, hwaddr kernel_addr)
>
>      /* Initialize the UARTs                                        */
>      /* *UART_CONTROL = UART_RECEIVE_ENABLE | UART_TRANSMIT_ENABLE; */
> -    p = gen_store_u32(p, 0x80000108, 3);
> +    p = gen_store_u32(p, LEON3_UART0_OFFSET + 0x8, 3);
> +    p = gen_store_u32(p, LEON3_UART1_OFFSET + 0x8, 3);
>
>      /* Initialize the TIMER 0                                      */
>      /* *GPTIMER_SCALER_RELOAD = 40 - 1;                            */
> @@ -271,7 +277,8 @@ static void leon3_generic_hw_init(MachineState *machine)
>      DeviceState *dev, *irqmpdev;
>      int i;
>      AHBPnp *ahb_pnp;
> -    APBPnp *apb_pnp;
> +    APBPnp *apb1_pnp;
> +    APBPnp *apb2_pnp;
>
>      reset_info = g_malloc0(sizeof(ResetData));
>
> @@ -298,10 +305,19 @@ static void leon3_generic_hw_init(MachineState *machine)
>                              GRLIB_LEON3_DEV, GRLIB_AHB_MASTER,
>                              GRLIB_CPU_AREA);
>
> -    apb_pnp = GRLIB_APB_PNP(qdev_new(TYPE_GRLIB_APB_PNP));
> -    sysbus_realize_and_unref(SYS_BUS_DEVICE(apb_pnp), &error_fatal);
> -    sysbus_mmio_map(SYS_BUS_DEVICE(apb_pnp), 0, LEON3_APB_PNP_OFFSET);
> -    grlib_ahb_pnp_add_entry(ahb_pnp, LEON3_APB_PNP_OFFSET, 0xFFF,
> +    /* Initialize APB1 */
> +    apb1_pnp = GRLIB_APB_PNP(qdev_new(TYPE_GRLIB_APB_PNP));
> +    sysbus_realize_and_unref(SYS_BUS_DEVICE(apb1_pnp), &error_fatal);
> +    sysbus_mmio_map(SYS_BUS_DEVICE(apb1_pnp), 0, LEON3_APB1_PNP_OFFSET);
> +    grlib_ahb_pnp_add_entry(ahb_pnp, LEON3_APB1_PNP_OFFSET, 0xFFF,
> +                            GRLIB_VENDOR_GAISLER, GRLIB_APBMST_DEV,
> +                            GRLIB_AHB_SLAVE, GRLIB_AHBMEM_AREA);
> +
> +    /* Initialize APB2 */
> +    apb2_pnp = GRLIB_APB_PNP(qdev_new(TYPE_GRLIB_APB_PNP));
> +    sysbus_realize_and_unref(SYS_BUS_DEVICE(apb2_pnp), &error_fatal);
> +    sysbus_mmio_map(SYS_BUS_DEVICE(apb2_pnp), 0, LEON3_APB2_PNP_OFFSET);
> +    grlib_ahb_pnp_add_entry(ahb_pnp, LEON3_APB2_PNP_OFFSET, 0xFFF,
>                              GRLIB_VENDOR_GAISLER, GRLIB_APBMST_DEV,
>                              GRLIB_AHB_SLAVE, GRLIB_AHBMEM_AREA);

You could embed that within a loop.

> @@ -309,6 +325,8 @@ static void leon3_generic_hw_init(MachineState *machine)
>      irqmpdev = qdev_new(TYPE_GRLIB_IRQMP);
>      object_property_set_int(OBJECT(irqmpdev), "ncpus", machine->smp.cpus,
>                              &error_fatal);
> +    /*object_property_set_int(OBJECT(irqmpdev), "eirq", LEON3_EIRQ,*/
> +    /*                        &error_fatal);*/

Why is this commented ?

>      sysbus_realize_and_unref(SYS_BUS_DEVICE(irqmpdev), &error_fatal);
>
>      for (i = 0; i < machine->smp.cpus; i++) {
> @@ -325,7 +343,7 @@ static void leon3_generic_hw_init(MachineState *machine)
>      }
>
>      sysbus_mmio_map(SYS_BUS_DEVICE(irqmpdev), 0, LEON3_IRQMP_OFFSET);
> -    grlib_apb_pnp_add_entry(apb_pnp, LEON3_IRQMP_OFFSET, 0xFFF,
> +    grlib_apb_pnp_add_entry(apb1_pnp, LEON3_IRQMP_OFFSET, 0xFFF,
>                              GRLIB_VENDOR_GAISLER, GRLIB_IRQMP_DEV,
>                              2, 0, GRLIB_APBIO_AREA);
>
> @@ -417,20 +435,31 @@ static void leon3_generic_hw_init(MachineState *machine)
>                             qdev_get_gpio_in(irqmpdev, LEON3_TIMER_IRQ + i));
>      }
>
> -    grlib_apb_pnp_add_entry(apb_pnp, LEON3_TIMER_OFFSET, 0xFFF,
> +    grlib_apb_pnp_add_entry(apb1_pnp, LEON3_TIMER_OFFSET, 0xFFF,
>                              GRLIB_VENDOR_GAISLER, GRLIB_GPTIMER_DEV,
>                              0, LEON3_TIMER_IRQ, GRLIB_APBIO_AREA);
>
> -    /* Allocate uart */
> +    /* Allocate UART0 */
>      dev = qdev_new(TYPE_GRLIB_APB_UART);
>      qdev_prop_set_chr(dev, "chrdev", serial_hd(0));
>      sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
> -    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, LEON3_UART_OFFSET);
> +    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, LEON3_UART0_OFFSET);
> +    sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0,
> +                       qdev_get_gpio_in(irqmpdev, LEON3_UART0_IRQ));
> +    grlib_apb_pnp_add_entry(apb1_pnp, LEON3_UART0_OFFSET, 0xFFF,
> +                            GRLIB_VENDOR_GAISLER, GRLIB_APBUART_DEV, 1,
> +                            LEON3_UART0_IRQ, GRLIB_APBIO_AREA);
> +
> +    /* Allocate UART1 */
> +    dev = qdev_new(TYPE_GRLIB_APB_UART);
> +    qdev_prop_set_chr(dev, "chrdev", serial_hd(1));
> +    sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
> +    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, LEON3_UART1_OFFSET);
>      sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0,
> -                       qdev_get_gpio_in(irqmpdev, LEON3_UART_IRQ));
> -    grlib_apb_pnp_add_entry(apb_pnp, LEON3_UART_OFFSET, 0xFFF,
> +                       qdev_get_gpio_in(irqmpdev, LEON3_UART1_IRQ));
> +    grlib_apb_pnp_add_entry(apb1_pnp, LEON3_UART1_OFFSET, 0xFFF,

Should this be apb2 here ?
Moreover, this is pretty similar to UART0. Thus, this would be better
to include within a loop with just a few adjustments for UART0. This
would also have the benefits to ease adding other UARTs, at some
point.


>                              GRLIB_VENDOR_GAISLER, GRLIB_APBUART_DEV, 1,
> -                            LEON3_UART_IRQ, GRLIB_APBIO_AREA);
> +                            LEON3_UART1_IRQ, GRLIB_APBIO_AREA);
>  }
>
>  static void leon3_generic_machine_init(MachineClass *mc)
> --
> 2.46.1
>
>


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

end of thread, other threads:[~2024-09-23 11:22 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-09-21 16:43 [PATCH v2 1/2] hw/intc/grlib_irqmp: add support for extended interrupts Nikita Shushura
2024-09-21 16:43 ` [PATCH v2 2/2] hw/sparc/leon3: add second uart with extended interrupt usage Nikita Shushura
2024-09-23 11:21   ` Clément Chigot
2024-09-23 11:20 ` [PATCH v2 1/2] hw/intc/grlib_irqmp: add support for extended interrupts Clément Chigot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).