[PATCH v1] hw/arm/xlnx-zynqmp: 'Or' the QSPI / QSPI DMA IRQs

[PATCH v1] hw/arm/xlnx-zynqmp: 'Or' the QSPI / QSPI DMA IRQs

[Francisco Iglesias]

'Or' the IRQs coming from the QSPI and QSPI DMA models. This is done for
avoiding the situation where one of the models incorrectly deasserts an
interrupt asserted from the other model (which will result in that the IRQ
is lost and will not reach guest SW).

Signed-off-by: Francisco Iglesias <francisco.iglesias@xilinx.com>
---

Hi,

I noted this after receiving a review comment (from Peter Maydell) on a similar
issue on the Versal machine while working on the OSPI series.

Best regards,
Francisco Iglesias


 include/hw/arm/xlnx-zynqmp.h |  2 ++
 hw/arm/xlnx-zynqmp.c         | 14 ++++++++++++--
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/include/hw/arm/xlnx-zynqmp.h b/include/hw/arm/xlnx-zynqmp.h
index 062e637fe4..9424f81c37 100644
--- a/include/hw/arm/xlnx-zynqmp.h
+++ b/include/hw/arm/xlnx-zynqmp.h
@@ -38,6 +38,7 @@
 #include "hw/dma/xlnx_csu_dma.h"
 #include "hw/nvram/xlnx-bbram.h"
 #include "hw/nvram/xlnx-zynqmp-efuse.h"
+#include "hw/or-irq.h"
 
 #define TYPE_XLNX_ZYNQMP "xlnx-zynqmp"
 OBJECT_DECLARE_SIMPLE_TYPE(XlnxZynqMPState, XLNX_ZYNQMP)
@@ -122,6 +123,7 @@ struct XlnxZynqMPState {
     XlnxZDMA gdma[XLNX_ZYNQMP_NUM_GDMA_CH];
     XlnxZDMA adma[XLNX_ZYNQMP_NUM_ADMA_CH];
     XlnxCSUDMA qspi_dma;
+    qemu_or_irq qspi_irq_orgate;
 
     char *boot_cpu;
     ARMCPU *boot_cpu_ptr;
diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
index 1c52a575aa..5fbf38c466 100644
--- a/hw/arm/xlnx-zynqmp.c
+++ b/hw/arm/xlnx-zynqmp.c
@@ -50,6 +50,7 @@
 #define LQSPI_ADDR          0xc0000000
 #define QSPI_IRQ            15
 #define QSPI_DMA_ADDR       0xff0f0800
+#define NUM_QSPI_IRQ_LINES  2
 
 #define DP_ADDR             0xfd4a0000
 #define DP_IRQ              113
@@ -362,6 +363,8 @@ static void xlnx_zynqmp_init(Object *obj)
     }
 
     object_initialize_child(obj, "qspi-dma", &s->qspi_dma, TYPE_XLNX_CSU_DMA);
+    object_initialize_child(obj, "qspi-irq-orgate",
+                            &s->qspi_irq_orgate, TYPE_OR_IRQ);
 }
 
 static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
@@ -709,6 +712,11 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
                            gic_spi[adma_ch_intr[i]]);
     }
 
+    object_property_set_int(OBJECT(&s->qspi_irq_orgate),
+                            "num-lines", NUM_QSPI_IRQ_LINES, &error_fatal);
+    qdev_realize(DEVICE(&s->qspi_irq_orgate), NULL, &error_fatal);
+    qdev_connect_gpio_out(DEVICE(&s->qspi_irq_orgate), 0, gic_spi[QSPI_IRQ]);
+
     if (!object_property_set_link(OBJECT(&s->qspi_dma), "dma",
                                   OBJECT(system_memory), errp)) {
         return;
@@ -718,7 +726,8 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
     }
 
     sysbus_mmio_map(SYS_BUS_DEVICE(&s->qspi_dma), 0, QSPI_DMA_ADDR);
-    sysbus_connect_irq(SYS_BUS_DEVICE(&s->qspi_dma), 0, gic_spi[QSPI_IRQ]);
+    sysbus_connect_irq(SYS_BUS_DEVICE(&s->qspi_dma), 0,
+                       qdev_get_gpio_in(DEVICE(&s->qspi_irq_orgate), 0));
 
     if (!object_property_set_link(OBJECT(&s->qspi), "stream-connected-dma",
                                   OBJECT(&s->qspi_dma), errp)) {
@@ -729,7 +738,8 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
     }
     sysbus_mmio_map(SYS_BUS_DEVICE(&s->qspi), 0, QSPI_ADDR);
     sysbus_mmio_map(SYS_BUS_DEVICE(&s->qspi), 1, LQSPI_ADDR);
-    sysbus_connect_irq(SYS_BUS_DEVICE(&s->qspi), 0, gic_spi[QSPI_IRQ]);
+    sysbus_connect_irq(SYS_BUS_DEVICE(&s->qspi), 0,
+                       qdev_get_gpio_in(DEVICE(&s->qspi_irq_orgate), 1));
 
     for (i = 0; i < XLNX_ZYNQMP_NUM_QSPI_BUS; i++) {
         g_autofree gchar *bus_name = g_strdup_printf("qspi%d", i);
-- 
2.11.0

Re: [PATCH v1] hw/arm/xlnx-zynqmp: 'Or' the QSPI / QSPI DMA IRQs

[Philippe Mathieu-Daudé via]

On 3/2/22 16:17, Francisco Iglesias wrote:
> 'Or' the IRQs coming from the QSPI and QSPI DMA models. This is done for
> avoiding the situation where one of the models incorrectly deasserts an
> interrupt asserted from the other model (which will result in that the IRQ
> is lost and will not reach guest SW).
> 
> Signed-off-by: Francisco Iglesias <francisco.iglesias@xilinx.com>
> ---
> 
> Hi,
> 
> I noted this after receiving a review comment (from Peter Maydell) on a similar
> issue on the Versal machine while working on the OSPI series.
> 
> Best regards,
> Francisco Iglesias
> 
> 
>   include/hw/arm/xlnx-zynqmp.h |  2 ++
>   hw/arm/xlnx-zynqmp.c         | 14 ++++++++++++--
>   2 files changed, 14 insertions(+), 2 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Re: [PATCH v1] hw/arm/xlnx-zynqmp: 'Or' the QSPI / QSPI DMA IRQs

[Luc Michel]

On 15:17 Thu 03 Feb     , Francisco Iglesias wrote:
> 'Or' the IRQs coming from the QSPI and QSPI DMA models. This is done for
> avoiding the situation where one of the models incorrectly deasserts an
> interrupt asserted from the other model (which will result in that the IRQ
> is lost and will not reach guest SW).
> 
> Signed-off-by: Francisco Iglesias <francisco.iglesias@xilinx.com>

Reviewed-by: Luc Michel <luc@lmichel.fr>

> ---
> 
> Hi,
> 
> I noted this after receiving a review comment (from Peter Maydell) on a similar
> issue on the Versal machine while working on the OSPI series.
> 
> Best regards,
> Francisco Iglesias
> 
> 
>  include/hw/arm/xlnx-zynqmp.h |  2 ++
>  hw/arm/xlnx-zynqmp.c         | 14 ++++++++++++--
>  2 files changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/include/hw/arm/xlnx-zynqmp.h b/include/hw/arm/xlnx-zynqmp.h
> index 062e637fe4..9424f81c37 100644
> --- a/include/hw/arm/xlnx-zynqmp.h
> +++ b/include/hw/arm/xlnx-zynqmp.h
> @@ -38,6 +38,7 @@
>  #include "hw/dma/xlnx_csu_dma.h"
>  #include "hw/nvram/xlnx-bbram.h"
>  #include "hw/nvram/xlnx-zynqmp-efuse.h"
> +#include "hw/or-irq.h"
>  
>  #define TYPE_XLNX_ZYNQMP "xlnx-zynqmp"
>  OBJECT_DECLARE_SIMPLE_TYPE(XlnxZynqMPState, XLNX_ZYNQMP)
> @@ -122,6 +123,7 @@ struct XlnxZynqMPState {
>      XlnxZDMA gdma[XLNX_ZYNQMP_NUM_GDMA_CH];
>      XlnxZDMA adma[XLNX_ZYNQMP_NUM_ADMA_CH];
>      XlnxCSUDMA qspi_dma;
> +    qemu_or_irq qspi_irq_orgate;
>  
>      char *boot_cpu;
>      ARMCPU *boot_cpu_ptr;
> diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
> index 1c52a575aa..5fbf38c466 100644
> --- a/hw/arm/xlnx-zynqmp.c
> +++ b/hw/arm/xlnx-zynqmp.c
> @@ -50,6 +50,7 @@
>  #define LQSPI_ADDR          0xc0000000
>  #define QSPI_IRQ            15
>  #define QSPI_DMA_ADDR       0xff0f0800
> +#define NUM_QSPI_IRQ_LINES  2
>  
>  #define DP_ADDR             0xfd4a0000
>  #define DP_IRQ              113
> @@ -362,6 +363,8 @@ static void xlnx_zynqmp_init(Object *obj)
>      }
>  
>      object_initialize_child(obj, "qspi-dma", &s->qspi_dma, TYPE_XLNX_CSU_DMA);
> +    object_initialize_child(obj, "qspi-irq-orgate",
> +                            &s->qspi_irq_orgate, TYPE_OR_IRQ);
>  }
>  
>  static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
> @@ -709,6 +712,11 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
>                             gic_spi[adma_ch_intr[i]]);
>      }
>  
> +    object_property_set_int(OBJECT(&s->qspi_irq_orgate),
> +                            "num-lines", NUM_QSPI_IRQ_LINES, &error_fatal);
> +    qdev_realize(DEVICE(&s->qspi_irq_orgate), NULL, &error_fatal);
> +    qdev_connect_gpio_out(DEVICE(&s->qspi_irq_orgate), 0, gic_spi[QSPI_IRQ]);
> +
>      if (!object_property_set_link(OBJECT(&s->qspi_dma), "dma",
>                                    OBJECT(system_memory), errp)) {
>          return;
> @@ -718,7 +726,8 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
>      }
>  
>      sysbus_mmio_map(SYS_BUS_DEVICE(&s->qspi_dma), 0, QSPI_DMA_ADDR);
> -    sysbus_connect_irq(SYS_BUS_DEVICE(&s->qspi_dma), 0, gic_spi[QSPI_IRQ]);
> +    sysbus_connect_irq(SYS_BUS_DEVICE(&s->qspi_dma), 0,
> +                       qdev_get_gpio_in(DEVICE(&s->qspi_irq_orgate), 0));
>  
>      if (!object_property_set_link(OBJECT(&s->qspi), "stream-connected-dma",
>                                    OBJECT(&s->qspi_dma), errp)) {
> @@ -729,7 +738,8 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
>      }
>      sysbus_mmio_map(SYS_BUS_DEVICE(&s->qspi), 0, QSPI_ADDR);
>      sysbus_mmio_map(SYS_BUS_DEVICE(&s->qspi), 1, LQSPI_ADDR);
> -    sysbus_connect_irq(SYS_BUS_DEVICE(&s->qspi), 0, gic_spi[QSPI_IRQ]);
> +    sysbus_connect_irq(SYS_BUS_DEVICE(&s->qspi), 0,
> +                       qdev_get_gpio_in(DEVICE(&s->qspi_irq_orgate), 1));
>  
>      for (i = 0; i < XLNX_ZYNQMP_NUM_QSPI_BUS; i++) {
>          g_autofree gchar *bus_name = g_strdup_printf("qspi%d", i);
> -- 
> 2.11.0
> 

-- 

Re: [PATCH v1] hw/arm/xlnx-zynqmp: 'Or' the QSPI / QSPI DMA IRQs

[Peter Maydell]

On Thu, 3 Feb 2022 at 15:17, Francisco Iglesias
<francisco.iglesias@xilinx.com> wrote:
>
> 'Or' the IRQs coming from the QSPI and QSPI DMA models. This is done for
> avoiding the situation where one of the models incorrectly deasserts an
> interrupt asserted from the other model (which will result in that the IRQ
> is lost and will not reach guest SW).
>
> Signed-off-by: Francisco Iglesias <francisco.iglesias@xilinx.com>



Applied to target-arm.next, thanks.

-- PMM