On 3/3/2025 6:26 AM, Philippe Mathieu-Daudé wrote:
Hi Brian and Sid,

On 1/3/25 18:20, Brian Cain wrote:
From: Sid Manning <sidn...@quicinc.com>

Co-authored-by: Matheus Tavares Bernardino <quic_mathb...@quicinc.com>
Co-authored-by: Damien Hedde <damien.he...@dahe.fr>
Signed-off-by: Brian Cain <brian.c...@oss.qualcomm.com>
---
  MAINTAINERS                    |   2 +
  docs/devel/hexagon-l2vic.rst   |  59 +++++
  docs/devel/index-internals.rst |   1 +
  include/hw/intc/l2vic.h        |  37 +++
  hw/intc/l2vic.c                | 417 +++++++++++++++++++++++++++++++++
  hw/intc/Kconfig                |   3 +
  hw/intc/meson.build            |   2 +
  hw/intc/trace-events           |   4 +
  8 files changed, 525 insertions(+)
  create mode 100644 docs/devel/hexagon-l2vic.rst
  create mode 100644 include/hw/intc/l2vic.h
  create mode 100644 hw/intc/l2vic.c


diff --git a/hw/intc/l2vic.c b/hw/intc/l2vic.c
new file mode 100644
index 0000000000..9df6575214
--- /dev/null
+++ b/hw/intc/l2vic.c
@@ -0,0 +1,417 @@
+/*
+ * QEMU L2VIC Interrupt Controller
+ *
+ * Arm PrimeCell PL190 Vector Interrupt Controller was used as a reference. + * Copyright(c) 2020-2025 Qualcomm Innovation Center, Inc. All Rights Reserved.
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "hw/irq.h"
+#include "hw/sysbus.h"
+#include "migration/vmstate.h"
+#include "qemu/log.h"
+#include "qemu/module.h"
+#include "hw/intc/l2vic.h"
+#include "trace.h"
+
+#define L2VICA(s, n) (s[(n) >> 2])
+
+#define TYPE_L2VIC "l2vic"
+#define L2VIC(obj) OBJECT_CHECK(L2VICState, (obj), TYPE_L2VIC)

Why not use OBJECT_DECLARE_SIMPLE_TYPE()?


Will do, thanks.


+
+#define SLICE_MAX (L2VIC_INTERRUPT_MAX / 32)
+
+typedef struct L2VICState {
+    SysBusDevice parent_obj;
+
+    QemuMutex active;
+    MemoryRegion iomem;
+    MemoryRegion fast_iomem;
+    uint32_t level;
+    /*
+     * offset 0:vid group 0 etc, 10 bits in each group
+     * are used:
+     */
+    uint32_t vid_group[4];
+    uint32_t vid0;
+    /* Clear Status of Active Edge interrupt, not used: */
+    uint32_t int_clear[SLICE_MAX] QEMU_ALIGNED(16);
+    /* Enable interrupt source */
+    uint32_t int_enable[SLICE_MAX] QEMU_ALIGNED(16);
+    /* Clear (set to 0) corresponding bit in int_enable */
+    uint32_t int_enable_clear;
+    /* Set (to 1) corresponding bit in int_enable */
+    uint32_t int_enable_set;
+    /* Present for debugging, not used */
+    uint32_t int_pending[SLICE_MAX] QEMU_ALIGNED(16);
+    /* Generate an interrupt */
+    uint32_t int_soft;
+    /* Which enabled interrupt is active */
+    uint32_t int_status[SLICE_MAX] QEMU_ALIGNED(16);
+    /* Edge or Level interrupt */
+    uint32_t int_type[SLICE_MAX] QEMU_ALIGNED(16);
+    /* L2 interrupt group 0-3 0x600-0x7FF */
+    uint32_t int_group_n0[SLICE_MAX] QEMU_ALIGNED(16);
+    uint32_t int_group_n1[SLICE_MAX] QEMU_ALIGNED(16);
+    uint32_t int_group_n2[SLICE_MAX] QEMU_ALIGNED(16);
+    uint32_t int_group_n3[SLICE_MAX] QEMU_ALIGNED(16);
+    qemu_irq irq[8];
+} L2VICState;

OBJECT_DECLARE_SIMPLE_TYPE(L2VICState, L2VIC)


+static inline bool vid_active(L2VICState *s)
+
+{
+    /* scan all 1024 bits in int_status arrary */
+    const int size = sizeof(s->int_status) * CHAR_BIT;
+    const int active_irq = find_first_bit((unsigned long *)s->int_status, size);

Maybe this file could leverage the 32-bit bitops.h API:

$ git grep bit32\( include/qemu/bitops.h
include/qemu/bitops.h:38: * - Bits stored in an array of 'uint32_t': set_bit32(), clear_bit32(), etc include/qemu/bitops.h:270:static inline void set_bit32(long nr, uint32_t *addr) include/qemu/bitops.h:296:static inline void clear_bit32(long nr, uint32_t *addr) include/qemu/bitops.h:322:static inline void change_bit32(long nr, uint32_t *addr) include/qemu/bitops.h:335:static inline int test_and_set_bit32(long nr, uint32_t *addr) include/qemu/bitops.h:350:static inline int test_and_clear_bit32(long nr, uint32_t *addr) include/qemu/bitops.h:365:static inline int test_and_change_bit32(long nr, uint32_t *addr) include/qemu/bitops.h:380:static inline int test_bit32(long nr, const uint32_t *addr)


I think your suggestion is based on the fact the state to hold the interrupt status (et al) were declared as uint32_t arrays. But in fact it might be clearer to take Taylor's suggestion from this thread and use DECLARE_BITMAP() instead?

In which case, perhaps the call to find_first_bit() could remain?


+    return ((active_irq != size)) ? true : false;
+}
+
+static bool l2vic_update(L2VICState *s, int irq)
+{
+    if (vid_active(s)) {
+        return true;
+    }
+
+    bool pending = test_bit(irq, (unsigned long *)s->int_pending);
+    bool enable = test_bit(irq, (unsigned long *)s->int_enable);
+    if (pending && enable) {
+        int vid = get_vid(s, irq);
+        set_bit(irq, (unsigned long *)s->int_status);
+        clear_bit(irq, (unsigned long *)s->int_pending);
+        clear_bit(irq, (unsigned long *)s->int_enable);
+        /* ensure the irq line goes low after going high */
+        s->vid0 = irq;
+        s->vid_group[get_vid(s, irq)] = irq;
+
+        /* already low: now call pulse */
+        /*     pulse: calls qemu_upper() and then qemu_lower()) */
+        qemu_irq_pulse(s->irq[vid + 2]);
+        trace_l2vic_delivered(irq, vid);
+        return true;
+    }
+    return false;
+}


Reply via email to