qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v1 0/8] GICv3 LPI and ITS feature implementation
@ 2021-03-23  4:12 Shashi Mallela
  2021-03-23  4:12 ` [PATCH v1 1/8] hw/intc: GICv3 ITS initial framework Shashi Mallela
                   ` (9 more replies)
  0 siblings, 10 replies; 16+ messages in thread
From: Shashi Mallela @ 2021-03-23  4:12 UTC (permalink / raw)
  To: peter.maydell, leif, rad; +Cc: qemu-arm, qemu-devel

This patchset implements qemu device model for enabling physical
LPI support and ITS functionality in GIC as per GICv3 specification.
Both flat table and 2 level tables are implemented.The ITS commands
for adding/deleting ITS table entries,trigerring LPI interrupts are 
implemented.Translated LPI interrupt ids are processed by redistributor
to determine priority and set pending state appropriately before
forwarding the same to cpu interface.
The ITS feature support has been added to sbsa-ref platform as well as
virt platform,wherein the emulated functionality co-exists with kvm
kernel functionality.

Shashi Mallela (8):
  hw/intc: GICv3 ITS initial framework
  hw/intc: GICv3 ITS register definitions added
  hw/intc: GICv3 ITS command queue framework
  hw/intc: GICv3 ITS Command processing
  hw/intc: GICv3 ITS Feature enablement
  hw/intc: GICv3 redistributor ITS processing
  hw/arm/sbsa-ref: add ITS support in SBSA GIC
  hw/arm/virt: add ITS support in virt GIC

 hw/arm/sbsa-ref.c                      |   26 +-
 hw/arm/virt.c                          |   10 +-
 hw/intc/arm_gicv3.c                    |    6 +
 hw/intc/arm_gicv3_common.c             |   16 +
 hw/intc/arm_gicv3_cpuif.c              |   15 +-
 hw/intc/arm_gicv3_dist.c               |   22 +-
 hw/intc/arm_gicv3_its.c                | 1417 ++++++++++++++++++++
 hw/intc/arm_gicv3_its_common.c         |   17 +-
 hw/intc/arm_gicv3_its_kvm.c            |    2 +-
 hw/intc/arm_gicv3_redist.c             |  155 ++-
 hw/intc/gicv3_internal.h               |  176 ++-
 hw/intc/meson.build                    |    1 +
 include/hw/intc/arm_gicv3_common.h     |   14 +
 include/hw/intc/arm_gicv3_its_common.h |   12 +-
 target/arm/kvm_arm.h                   |    4 +-
 15 files changed, 1869 insertions(+), 24 deletions(-)
 create mode 100644 hw/intc/arm_gicv3_its.c

-- 
2.27.0



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

* [PATCH v1 1/8] hw/intc: GICv3 ITS initial framework
  2021-03-23  4:12 [PATCH v1 0/8] GICv3 LPI and ITS feature implementation Shashi Mallela
@ 2021-03-23  4:12 ` Shashi Mallela
  2021-03-25 16:43   ` Alex Bennée
  2021-03-25 17:18   ` Alex Bennée
  2021-03-23  4:12 ` [PATCH v1 2/8] hw/intc: GICv3 ITS register definitions added Shashi Mallela
                   ` (8 subsequent siblings)
  9 siblings, 2 replies; 16+ messages in thread
From: Shashi Mallela @ 2021-03-23  4:12 UTC (permalink / raw)
  To: peter.maydell, leif, rad; +Cc: qemu-arm, qemu-devel

Added register definitions relevant to ITS,implemented overall
ITS device framework with stubs for ITS control and translater
regions read/write,extended ITS common to handle mmio init between
existing kvm device and newer qemu device.

Signed-off-by: Shashi Mallela <shashi.mallela@linaro.org>
---
 hw/intc/arm_gicv3_its.c                | 323 ++++++++++++++++++++
 hw/intc/arm_gicv3_its_common.c         |  17 +-
 hw/intc/arm_gicv3_its_kvm.c            |   2 +-
 hw/intc/gicv3_internal.h               | 173 ++++++++++-
 hw/intc/meson.build                    |   1 +
 include/hw/intc/arm_gicv3_its_common.h |  12 +-
 6 files changed, 520 insertions(+), 8 deletions(-)

diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c
new file mode 100644
index 0000000000..34e49b4d63
--- /dev/null
+++ b/hw/intc/arm_gicv3_its.c
@@ -0,0 +1,323 @@
+/*
+ * ITS emulation for a GICv3-based system
+ *
+ * Copyright Linaro.org 2021
+ *
+ * Authors:
+ *  Shashi Mallela <shashi.mallela@linaro.org>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or (at your
+ * option) any later version.  See the COPYING file in the top-level directory.
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/log.h"
+#include "hw/qdev-properties.h"
+#include "hw/intc/arm_gicv3_its_common.h"
+#include "gicv3_internal.h"
+#include "qom/object.h"
+
+typedef struct GICv3ITSClass GICv3ITSClass;
+/* This is reusing the GICv3ITSState typedef from ARM_GICV3_ITS_COMMON */
+DECLARE_OBJ_CHECKERS(GICv3ITSState, GICv3ITSClass,
+                     ARM_GICV3_ITS, TYPE_ARM_GICV3_ITS)
+
+struct GICv3ITSClass {
+    GICv3ITSCommonClass parent_class;
+    void (*parent_reset)(DeviceState *dev);
+};
+
+static MemTxResult its_trans_writew(GICv3ITSState *s, hwaddr offset,
+                               uint64_t value, MemTxAttrs attrs)
+{
+    MemTxResult result = MEMTX_OK;
+
+    return result;
+}
+
+static MemTxResult its_trans_writel(GICv3ITSState *s, hwaddr offset,
+                               uint64_t value, MemTxAttrs attrs)
+{
+    MemTxResult result = MEMTX_OK;
+
+    return result;
+}
+
+static MemTxResult gicv3_its_trans_write(void *opaque, hwaddr offset,
+                               uint64_t data, unsigned size, MemTxAttrs attrs)
+{
+    GICv3ITSState *s = (GICv3ITSState *)opaque;
+    MemTxResult result;
+
+    switch (size) {
+    case 2:
+        result = its_trans_writew(s, offset, data, attrs);
+        break;
+    case 4:
+        result = its_trans_writel(s, offset, data, attrs);
+        break;
+    default:
+        result = MEMTX_ERROR;
+        break;
+    }
+
+    if (result == MEMTX_ERROR) {
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "%s: invalid guest write at offset " TARGET_FMT_plx
+                      "size %u\n", __func__, offset, size);
+        /*
+         * The spec requires that reserved registers are RAZ/WI;
+         * so use MEMTX_ERROR returns from leaf functions as a way to
+         * trigger the guest-error logging but don't return it to
+         * the caller, or we'll cause a spurious guest data abort.
+         */
+        result = MEMTX_OK;
+    }
+    return result;
+}
+
+static MemTxResult gicv3_its_trans_read(void *opaque, hwaddr offset,
+                              uint64_t *data, unsigned size, MemTxAttrs attrs)
+{
+    qemu_log_mask(LOG_GUEST_ERROR,
+        "%s: Invalid read from transaction register area at offset "
+        TARGET_FMT_plx "\n", __func__, offset);
+    return MEMTX_ERROR;
+}
+
+static MemTxResult its_writeb(GICv3ITSState *s, hwaddr offset,
+                               uint64_t value, MemTxAttrs attrs)
+{
+    qemu_log_mask(LOG_GUEST_ERROR,
+                "%s: unsupported byte write to register at offset "
+                TARGET_FMT_plx "\n", __func__, offset);
+    return MEMTX_ERROR;
+}
+
+static MemTxResult its_readb(GICv3ITSState *s, hwaddr offset,
+                               uint64_t *data, MemTxAttrs attrs)
+{
+    qemu_log_mask(LOG_GUEST_ERROR,
+                "%s: unsupported byte read from register at offset "
+                TARGET_FMT_plx "\n", __func__, offset);
+    return MEMTX_ERROR;
+}
+
+static MemTxResult its_writew(GICv3ITSState *s, hwaddr offset,
+                               uint64_t value, MemTxAttrs attrs)
+{
+    qemu_log_mask(LOG_GUEST_ERROR,
+        "%s: unsupported word write to register at offset "
+        TARGET_FMT_plx "\n", __func__, offset);
+    return MEMTX_ERROR;
+}
+
+static MemTxResult its_readw(GICv3ITSState *s, hwaddr offset,
+                               uint64_t *data, MemTxAttrs attrs)
+{
+    qemu_log_mask(LOG_GUEST_ERROR,
+        "%s: unsupported word read from register at offset "
+        TARGET_FMT_plx "\n", __func__, offset);
+    return MEMTX_ERROR;
+}
+
+static MemTxResult its_writel(GICv3ITSState *s, hwaddr offset,
+                               uint64_t value, MemTxAttrs attrs)
+{
+    MemTxResult result = MEMTX_OK;
+
+    return result;
+}
+
+static MemTxResult its_readl(GICv3ITSState *s, hwaddr offset,
+                               uint64_t *data, MemTxAttrs attrs)
+{
+    MemTxResult result = MEMTX_OK;
+
+    return result;
+}
+
+static MemTxResult its_writell(GICv3ITSState *s, hwaddr offset,
+                               uint64_t value, MemTxAttrs attrs)
+{
+    MemTxResult result = MEMTX_OK;
+
+    return result;
+}
+
+static MemTxResult its_readll(GICv3ITSState *s, hwaddr offset,
+                               uint64_t *data, MemTxAttrs attrs)
+{
+    MemTxResult result = MEMTX_OK;
+
+    return result;
+}
+
+static MemTxResult gicv3_its_read(void *opaque, hwaddr offset, uint64_t *data,
+                              unsigned size, MemTxAttrs attrs)
+{
+    GICv3ITSState *s = (GICv3ITSState *)opaque;
+    MemTxResult result;
+
+    switch (size) {
+    case 1:
+        result = its_readb(s, offset, data, attrs);
+        break;
+    case 2:
+        result = its_readw(s, offset, data, attrs);
+        break;
+    case 4:
+        result = its_readl(s, offset, data, attrs);
+        break;
+    case 8:
+        result = its_readll(s, offset, data, attrs);
+        break;
+    default:
+        result = MEMTX_ERROR;
+        break;
+    }
+
+    if (result == MEMTX_ERROR) {
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "%s: invalid guest read at offset " TARGET_FMT_plx
+                      "size %u\n", __func__, offset, size);
+        /*
+         * The spec requires that reserved registers are RAZ/WI;
+         * so use MEMTX_ERROR returns from leaf functions as a way to
+         * trigger the guest-error logging but don't return it to
+         * the caller, or we'll cause a spurious guest data abort.
+         */
+        result = MEMTX_OK;
+        *data = 0;
+    }
+    return result;
+}
+
+static MemTxResult gicv3_its_write(void *opaque, hwaddr offset, uint64_t data,
+                               unsigned size, MemTxAttrs attrs)
+{
+    GICv3ITSState *s = (GICv3ITSState *)opaque;
+    MemTxResult result;
+
+    switch (size) {
+    case 1:
+        result = its_writeb(s, offset, data, attrs);
+        break;
+    case 2:
+        result = its_writew(s, offset, data, attrs);
+        break;
+    case 4:
+        result = its_writel(s, offset, data, attrs);
+        break;
+    case 8:
+        result = its_writell(s, offset, data, attrs);
+        break;
+    default:
+        result = MEMTX_ERROR;
+        break;
+    }
+
+    if (result == MEMTX_ERROR) {
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "%s: invalid guest write at offset " TARGET_FMT_plx
+                      "size %u\n", __func__, offset, size);
+        /*
+         * The spec requires that reserved registers are RAZ/WI;
+         * so use MEMTX_ERROR returns from leaf functions as a way to
+         * trigger the guest-error logging but don't return it to
+         * the caller, or we'll cause a spurious guest data abort.
+         */
+        result = MEMTX_OK;
+    }
+    return result;
+}
+
+static const MemoryRegionOps gicv3_its_control_ops = {
+    .read_with_attrs = gicv3_its_read,
+    .write_with_attrs = gicv3_its_write,
+    .endianness = DEVICE_NATIVE_ENDIAN,
+};
+
+static const MemoryRegionOps gicv3_its_trans_ops = {
+    .read_with_attrs = gicv3_its_trans_read,
+    .write_with_attrs = gicv3_its_trans_write,
+    .endianness = DEVICE_NATIVE_ENDIAN,
+};
+
+static void gicv3_arm_its_realize(DeviceState *dev, Error **errp)
+{
+    GICv3ITSState *s = ARM_GICV3_ITS_COMMON(dev);
+
+    gicv3_its_init_mmio(s, &gicv3_its_control_ops, &gicv3_its_trans_ops);
+}
+
+static void gicv3_its_reset(DeviceState *dev)
+{
+    GICv3ITSState *s = ARM_GICV3_ITS_COMMON(dev);
+    GICv3ITSClass *c = ARM_GICV3_ITS_GET_CLASS(s);
+
+    if (s->gicv3->cpu->gicr_typer & GICR_TYPER_PLPIS) {
+        c->parent_reset(dev);
+
+        /* set the ITS default features supported */
+        s->typer = GITS_TYPER_PHYSICAL | (GITS_TYPER_ITT_ENTRY_SIZE <<
+                   GITS_TYPER_ITT_ENTRY_OFFSET) | (GITS_TYPER_IDBITS <<
+                   GITS_TYPER_IDBITS_OFFSET) | GITS_TYPER_DEVBITS |
+                   GITS_TYPER_CIL | GITS_TYPER_CIDBITS;
+
+        /*
+         * We claim to be an ARM r0p0 with a zero ProductID.
+         * This is the same as an r0p0 GIC-500.
+         */
+        s->iidr = gicv3_iidr();
+
+        /* Quiescent bit reset to 1 */
+        s->ctlr = (1U << 31);
+
+        /*
+         * setting GITS_BASER0.Type = 0b001 (Device)
+         *         GITS_BASER1.Type = 0b100 (Collection Table)
+         *         GITS_BASER<n>.Type,where n = 3 to 7 are 0b00 (Unimplemented)
+         *         GITS_BASER<0,1>.Page_Size = 64KB
+         * and default translation table entry size to 16 bytes
+         */
+        s->baser[0] = (GITS_ITT_TYPE_DEVICE << GITS_BASER_TYPE_OFFSET) |
+                      (GITS_BASER_PAGESIZE_64K << GITS_BASER_PAGESIZE_OFFSET) |
+                      (GITS_DTE_SIZE << GITS_BASER_ENTRYSIZE_OFFSET);
+        s->baser[1] = (GITS_ITT_TYPE_COLLECTION << GITS_BASER_TYPE_OFFSET) |
+                      (GITS_BASER_PAGESIZE_64K << GITS_BASER_PAGESIZE_OFFSET) |
+                      (GITS_CTE_SIZE << GITS_BASER_ENTRYSIZE_OFFSET);
+    }
+}
+
+static Property gicv3_its_props[] = {
+    DEFINE_PROP_LINK("parent-gicv3", GICv3ITSState, gicv3, "arm-gicv3",
+                     GICv3State *),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void gicv3_its_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    GICv3ITSClass *ic = ARM_GICV3_ITS_CLASS(klass);
+
+    dc->realize = gicv3_arm_its_realize;
+    device_class_set_props(dc, gicv3_its_props);
+    device_class_set_parent_reset(dc, gicv3_its_reset, &ic->parent_reset);
+}
+
+static const TypeInfo gicv3_its_info = {
+    .name = TYPE_ARM_GICV3_ITS,
+    .parent = TYPE_ARM_GICV3_ITS_COMMON,
+    .instance_size = sizeof(GICv3ITSState),
+    .class_init = gicv3_its_class_init,
+    .class_size = sizeof(GICv3ITSClass),
+};
+
+static void gicv3_its_register_types(void)
+{
+    type_register_static(&gicv3_its_info);
+}
+
+type_init(gicv3_its_register_types)
diff --git a/hw/intc/arm_gicv3_its_common.c b/hw/intc/arm_gicv3_its_common.c
index 66c4c6a188..c18fe23fae 100644
--- a/hw/intc/arm_gicv3_its_common.c
+++ b/hw/intc/arm_gicv3_its_common.c
@@ -55,7 +55,9 @@ static const VMStateDescription vmstate_its = {
     .priority = MIG_PRI_GICV3_ITS,
     .fields = (VMStateField[]) {
         VMSTATE_UINT32(ctlr, GICv3ITSState),
+        VMSTATE_UINT32(translater, GICv3ITSState),
         VMSTATE_UINT32(iidr, GICv3ITSState),
+        VMSTATE_UINT64(typer, GICv3ITSState),
         VMSTATE_UINT64(cbaser, GICv3ITSState),
         VMSTATE_UINT64(cwriter, GICv3ITSState),
         VMSTATE_UINT64(creadr, GICv3ITSState),
@@ -99,15 +101,21 @@ static const MemoryRegionOps gicv3_its_trans_ops = {
     .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
-void gicv3_its_init_mmio(GICv3ITSState *s, const MemoryRegionOps *ops)
+void gicv3_its_init_mmio(GICv3ITSState *s, const MemoryRegionOps *ops,
+     const MemoryRegionOps *tops)
 {
     SysBusDevice *sbd = SYS_BUS_DEVICE(s);
 
     memory_region_init_io(&s->iomem_its_cntrl, OBJECT(s), ops, s,
                           "control", ITS_CONTROL_SIZE);
-    memory_region_init_io(&s->iomem_its_translation, OBJECT(s),
-                          &gicv3_its_trans_ops, s,
-                          "translation", ITS_TRANS_SIZE);
+    if (tops == NULL) {
+        memory_region_init_io(&s->iomem_its_translation, OBJECT(s),
+                              &gicv3_its_trans_ops, s,
+                              "translation", ITS_TRANS_SIZE);
+    } else {
+        memory_region_init_io(&s->iomem_its_translation, OBJECT(s),
+                              tops, s, "translation", ITS_TRANS_SIZE);
+    }
 
     /* Our two regions are always adjacent, therefore we now combine them
      * into a single one in order to make our users' life easier.
@@ -130,6 +138,7 @@ static void gicv3_its_common_reset(DeviceState *dev)
     s->cwriter = 0;
     s->creadr = 0;
     s->iidr = 0;
+    s->translater = 0;
     memset(&s->baser, 0, sizeof(s->baser));
 }
 
diff --git a/hw/intc/arm_gicv3_its_kvm.c b/hw/intc/arm_gicv3_its_kvm.c
index b554d2ede0..0b4cbed28b 100644
--- a/hw/intc/arm_gicv3_its_kvm.c
+++ b/hw/intc/arm_gicv3_its_kvm.c
@@ -106,7 +106,7 @@ static void kvm_arm_its_realize(DeviceState *dev, Error **errp)
     kvm_arm_register_device(&s->iomem_its_cntrl, -1, KVM_DEV_ARM_VGIC_GRP_ADDR,
                             KVM_VGIC_ITS_ADDR_TYPE, s->dev_fd, 0);
 
-    gicv3_its_init_mmio(s, NULL);
+    gicv3_its_init_mmio(s, NULL, NULL);
 
     if (!kvm_device_check_attr(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_ITS_REGS,
         GITS_CTLR)) {
diff --git a/hw/intc/gicv3_internal.h b/hw/intc/gicv3_internal.h
index 05303a55c8..7c6bc33e97 100644
--- a/hw/intc/gicv3_internal.h
+++ b/hw/intc/gicv3_internal.h
@@ -67,6 +67,12 @@
 #define GICD_CTLR_E1NWF             (1U << 7)
 #define GICD_CTLR_RWP               (1U << 31)
 
+#define GICD_TYPER_LPIS_OFFSET         17
+#define GICD_TYPER_IDBITS_OFFSET       19
+#define GICD_TYPER_IDBITS_MASK       0x1f
+/* 16 bits EventId */
+#define GICD_TYPER_IDBITS            0xf
+
 /*
  * Redistributor frame offsets from RD_base
  */
@@ -123,14 +129,17 @@
 #define GICR_WAKER_ChildrenAsleep    (1U << 2)
 
 #define GICR_PROPBASER_OUTER_CACHEABILITY_MASK (7ULL << 56)
-#define GICR_PROPBASER_ADDR_MASK               (0xfffffffffULL << 12)
+#define GICR_PROPBASER_ADDR_OFFSET               12
+#define GICR_PROPBASER_ADDR_MASK               ((1ULL << 40) - 1)
 #define GICR_PROPBASER_SHAREABILITY_MASK       (3U << 10)
 #define GICR_PROPBASER_CACHEABILITY_MASK       (7U << 7)
 #define GICR_PROPBASER_IDBITS_MASK             (0x1f)
+#define GICR_PROPBASER_IDBITS_THRESHOLD          0xd
 
 #define GICR_PENDBASER_PTZ                     (1ULL << 62)
 #define GICR_PENDBASER_OUTER_CACHEABILITY_MASK (7ULL << 56)
-#define GICR_PENDBASER_ADDR_MASK               (0xffffffffULL << 16)
+#define GICR_PENDBASER_ADDR_OFFSET               16
+#define GICR_PENDBASER_ADDR_MASK               ((1ULL << 36) - 1)
 #define GICR_PENDBASER_SHAREABILITY_MASK       (3U << 10)
 #define GICR_PENDBASER_CACHEABILITY_MASK       (7U << 7)
 
@@ -239,11 +248,171 @@
 #define ICH_VTR_EL2_PREBITS_SHIFT 26
 #define ICH_VTR_EL2_PRIBITS_SHIFT 29
 
+#define GITS_CTLR_ENABLED             (1U << 0)  /* ITS Enabled or not */
+#define GITS_CTLR_QUIESCENT           (1U << 31) /* Quiescent bit */
+
+#define GITS_BASER_SIZE                     (0xff)
+#define GITS_BASER_PAGESIZE_OFFSET            8
+#define GITS_BASER_PAGESIZE_MASK             0x3
+#define GITS_BASER_SHAREABILITY_OFFSET        10
+#define GITS_BASER_SHAREABILITY_MASK         0x3
+#define GITS_BASER_PHYADDR_OFFSET             12
+#define GITS_BASER_PHYADDR_MASK             ((1ULL << 36) - 1)
+#define GITS_BASER_PHYADDR_OFFSETL_64K        16
+#define GITS_BASER_PHYADDR_MASKL_64K        ((1ULL << 32) - 1)
+#define GITS_BASER_PHYADDR_OFFSETH_64K        48
+#define GITS_BASER_PHYADDR_MASKH_64K        ((1ULL << 4) - 1)
+#define GITS_BASER_ENTRYSIZE_OFFSET           48
+#define GITS_BASER_ENTRYSIZE_MASK           ((1U << 5) - 1)
+#define GITS_BASER_OUTERCACHE_OFFSET          53
+#define GITS_BASER_OUTERCACHE_MASK            0x7
+#define GITS_BASER_TYPE_OFFSET                56
+#define GITS_BASER_TYPE_MASK                  0x7
+#define GITS_BASER_INNERCACHE_OFFSET          59
+#define GITS_BASER_INNERCACHE_MASK            0x7
+#define GITS_BASER_INDIRECT_OFFSET            62
+#define GITS_BASER_INDIRECT_MASK              0x1
+#define GITS_BASER_VALID                      63
+#define GITS_BASER_VALID_MASK                 0x1
+
+#define GITS_BASER_VAL_MASK                 ((0x7ULL << 56) | (0x1fULL << 48))
+
+#define GITS_BASER_PAGESIZE_4K                0
+#define GITS_BASER_PAGESIZE_16K               1
+#define GITS_BASER_PAGESIZE_64K               2
+
+#define GITS_ITT_TYPE_DEVICE                  1ULL
+#define GITS_ITT_TYPE_COLLECTION              4ULL
+
+#define GITS_CBASER_SIZE                    (0xff)
+#define GITS_CBASER_SHAREABILITY_OFFSET       10
+#define GITS_CBASER_SHAREABILITY_MASK        0x3
+#define GITS_CBASER_PHYADDR_OFFSET            12
+#define GITS_CBASER_PHYADDR_MASK       ((1ULL << 40) - 1)
+#define GITS_CBASER_OUTERCACHE_OFFSET         53
+#define GITS_CBASER_OUTERCACHE_MASK          0x7
+#define GITS_CBASER_INNERCACHE_OFFSET         59
+#define GITS_CBASER_INNERCACHE_MASK          0x7
+#define GITS_CBASER_VALID_OFFSET              63
+#define GITS_CBASER_VALID_MASK               0x1
+
+#define GITS_CREADR_STALLED           (1U << 0)
+#define GITS_CREADR_OFFSET             5
+#define GITS_CREADR_OFFSET_MASK       ((1U << 15) - 1)
+
+#define GITS_CWRITER_RETRY            (1U << 0)
+#define GITS_CWRITER_OFFSET             5
+#define GITS_CWRITER_OFFSET_MASK      ((1U << 15) - 1)
+
+#define GITS_TYPER_DEVBITS_OFFSET       13
+#define GITS_TYPER_DEVBITS_MASK        0x1f
+#define GITS_TYPER_IDBITS_OFFSET        8
+#define GITS_TYPER_IDBITS_MASK         0x1f
+#define GITS_TYPER_CIDBITS_OFFSET       32
+#define GITS_TYPER_CIDBITS_MASK        0xf
+#define GITS_TYPER_CIL_OFFSET           36
+#define GITS_TYPER_CIL_MASK             0x1
+#define GITS_TYPER_PTA_OFFSET           19
+#define GITS_TYPER_PTA_MASK             0x1
+#define GITS_TYPER_SEIS_OFFSET          18
+#define GITS_TYPER_SEIS_MASK            0x1
+
+/* Default features advertised by this version of ITS */
+/* Physical LPIs supported */
+#define GITS_TYPER_PHYSICAL           (1U << 0)
+/*
+ * 11 bytes Interrupt translation Table Entry size
+ * Valid = 1 bit,InterruptType = 1 bit,
+ * Size of LPI number space[considering max 24 bits],
+ * Size of LPI number space[considering max 24 bits],
+ * ICID = 16 bits,
+ * vPEID = 16 bits
+ */
+#define GITS_TYPER_ITT_ENTRY_OFFSET     4
+#define GITS_TYPER_ITT_ENTRY_SIZE       0xB
+#define GITS_TYPER_IDBITS_OFFSET        8
+
+/* 16 bits EventId */
+#define GITS_TYPER_IDBITS             GICD_TYPER_IDBITS
+/* 16 bits DeviceId */
+#define GITS_TYPER_DEVBITS            (0xf << 13)
+/* Collection ID Limit indicated by CIDbits(next) */
+#define GITS_TYPER_CIL                (1ULL << 36)
+/* 16 bits CollectionId */
+#define GITS_TYPER_CIDBITS            (0xfULL << 32)
+/*
+ * 8 bytes Device Table Entry size
+ * Valid = 1 bit,ITTAddr = 44 bits,Size = 5 bits
+ */
+#define GITS_DTE_SIZE                 (0x8ULL)
+/*
+ * 8 bytes Collection Table Entry size
+ * Valid = 1 bit,RDBase = 36 bits(considering max RDBASE)
+ */
+#define GITS_CTE_SIZE                 (0x8ULL)
+
 /* Special interrupt IDs */
 #define INTID_SECURE 1020
 #define INTID_NONSECURE 1021
 #define INTID_SPURIOUS 1023
 
+/* ITS Commands */
+#define GITS_CMD_CLEAR            0x04
+#define GITS_CMD_DISCARD          0x0F
+#define GITS_CMD_INT              0x03
+#define GITS_CMD_MAPC             0x09
+#define GITS_CMD_MAPD             0x08
+#define GITS_CMD_MAPI             0x0B
+#define GITS_CMD_MAPTI            0x0A
+#define GITS_CMD_SYNC             0x05
+
+#define GITS_ITT_PAGE_SIZE_0      0x1000
+#define GITS_ITT_PAGE_SIZE_1      0x4000
+#define GITS_ITT_PAGE_SIZE_2      0x10000
+
+#define GITS_CMDQ_ENTRY_SIZE               32
+#define GITS_CMDQ_MAX_ENTRIES_PER_PAGE     128
+#define NUM_BYTES_IN_DW                     8
+
+#define CMD_MASK                  0xff
+
+/* MAPC command fields */
+#define ICID_LEN                  16
+#define ICID_MASK                 ((1U << ICID_LEN) - 1)
+#define RDBASE_LEN                32
+#define RDBASE_OFFSET             16
+#define RDBASE_MASK               ((1ULL << RDBASE_LEN) - 1)
+#define RDBASE_PROCNUM_LEN        16
+#define RDBASE_PROCNUM_MASK       ((1ULL << RDBASE_PROCNUM_LEN) - 1)
+
+#define DEVID_OFFSET              32
+#define DEVID_LEN                 32
+#define DEVID_MASK                ((1ULL << DEVID_LEN) - 1)
+
+/* MAPD command fields */
+#define ITTADDR_LEN               44
+#define ITTADDR_OFFSET            8
+#define ITTADDR_MASK              ((1ULL << ITTADDR_LEN) - 1)
+#define SIZE_MASK                 0x1f
+
+/* MAPI command fields */
+#define EVENTID_MASK              ((1ULL << 32) - 1)
+
+/* MAPTI command fields */
+#define pINTID_OFFSET              32
+#define pINTID_MASK               ((1ULL << 32) - 1)
+
+#define VALID_SHIFT               63
+#define VALID_MASK                0x1
+
+#define L1TABLE_ENTRY_SIZE         8
+
+#define LPI_CTE_ENABLE_OFFSET      0
+#define LPI_CTE_ENABLED          VALID_MASK
+#define LPI_CTE_PRIORITY_OFFSET    2
+#define LPI_CTE_PRIORITY_MASK     ((1U << 6) - 1)
+#define LPI_PRIORITY_MASK         0xfc
+
 /* Functions internal to the emulated GICv3 */
 
 /**
diff --git a/hw/intc/meson.build b/hw/intc/meson.build
index 1c299039f6..53472239f0 100644
--- a/hw/intc/meson.build
+++ b/hw/intc/meson.build
@@ -8,6 +8,7 @@ softmmu_ss.add(when: 'CONFIG_ARM_GIC', if_true: files(
   'arm_gicv3_dist.c',
   'arm_gicv3_its_common.c',
   'arm_gicv3_redist.c',
+  'arm_gicv3_its.c',
 ))
 softmmu_ss.add(when: 'CONFIG_ETRAXFS', if_true: files('etraxfs_pic.c'))
 softmmu_ss.add(when: 'CONFIG_HEATHROW_PIC', if_true: files('heathrow_pic.c'))
diff --git a/include/hw/intc/arm_gicv3_its_common.h b/include/hw/intc/arm_gicv3_its_common.h
index 5a0952b404..08bc5652ed 100644
--- a/include/hw/intc/arm_gicv3_its_common.h
+++ b/include/hw/intc/arm_gicv3_its_common.h
@@ -25,17 +25,24 @@
 #include "hw/intc/arm_gicv3_common.h"
 #include "qom/object.h"
 
+#define TYPE_ARM_GICV3_ITS "arm-gicv3-its"
+
 #define ITS_CONTROL_SIZE 0x10000
 #define ITS_TRANS_SIZE   0x10000
 #define ITS_SIZE         (ITS_CONTROL_SIZE + ITS_TRANS_SIZE)
 
 #define GITS_CTLR        0x0
 #define GITS_IIDR        0x4
+#define GITS_TYPER       0x8
 #define GITS_CBASER      0x80
 #define GITS_CWRITER     0x88
 #define GITS_CREADR      0x90
 #define GITS_BASER       0x100
 
+#define GITS_TRANSLATER  0x0040
+
+#define GITS_PIDR2       0xFFE8
+
 struct GICv3ITSState {
     SysBusDevice parent_obj;
 
@@ -52,6 +59,8 @@ struct GICv3ITSState {
     /* Registers */
     uint32_t ctlr;
     uint32_t iidr;
+    uint32_t translater;
+    uint64_t typer;
     uint64_t cbaser;
     uint64_t cwriter;
     uint64_t creadr;
@@ -62,7 +71,8 @@ struct GICv3ITSState {
 
 typedef struct GICv3ITSState GICv3ITSState;
 
-void gicv3_its_init_mmio(GICv3ITSState *s, const MemoryRegionOps *ops);
+void gicv3_its_init_mmio(GICv3ITSState *s, const MemoryRegionOps *ops,
+                   const MemoryRegionOps *tops);
 
 #define TYPE_ARM_GICV3_ITS_COMMON "arm-gicv3-its-common"
 typedef struct GICv3ITSCommonClass GICv3ITSCommonClass;
-- 
2.27.0



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

* [PATCH v1 2/8] hw/intc: GICv3 ITS register definitions added
  2021-03-23  4:12 [PATCH v1 0/8] GICv3 LPI and ITS feature implementation Shashi Mallela
  2021-03-23  4:12 ` [PATCH v1 1/8] hw/intc: GICv3 ITS initial framework Shashi Mallela
@ 2021-03-23  4:12 ` Shashi Mallela
  2021-03-25 19:34   ` Alex Bennée
  2021-03-23  4:12 ` [PATCH v1 3/8] hw/intc: GICv3 ITS command queue framework Shashi Mallela
                   ` (7 subsequent siblings)
  9 siblings, 1 reply; 16+ messages in thread
From: Shashi Mallela @ 2021-03-23  4:12 UTC (permalink / raw)
  To: peter.maydell, leif, rad; +Cc: qemu-arm, qemu-devel

Defined descriptors for ITS device table,collection table and ITS
command queue entities.Implemented register read/write functions,
extract ITS table parameters and command queue parameters,extended
gicv3 common to capture qemu address space(which host the ITS table
platform memories required for subsequent ITS processing) and
initialize the same in its device.

Signed-off-by: Shashi Mallela <shashi.mallela@linaro.org>
---
 hw/intc/arm_gicv3_its.c            | 356 ++++++++++++++++++++
 include/hw/intc/arm_gicv3_common.h |   4 +
 2 files changed, 360 insertions(+)

diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c
index 34e49b4d63..4895d32e67 100644
--- a/hw/intc/arm_gicv3_its.c
+++ b/hw/intc/arm_gicv3_its.c
@@ -23,11 +23,179 @@ typedef struct GICv3ITSClass GICv3ITSClass;
 DECLARE_OBJ_CHECKERS(GICv3ITSState, GICv3ITSClass,
                      ARM_GICV3_ITS, TYPE_ARM_GICV3_ITS)
 
+typedef struct {
+    bool valid;
+    bool indirect;
+    uint16_t entry_sz;
+    uint32_t max_entries;
+    uint32_t max_devids;
+    uint64_t base_addr;
+} DevTableDesc;
+
+typedef struct {
+    bool valid;
+    bool indirect;
+    uint16_t entry_sz;
+    uint32_t max_entries;
+    uint32_t max_collids;
+    uint64_t base_addr;
+} CollTableDesc;
+
+typedef struct {
+    bool valid;
+    uint32_t max_entries;
+    uint64_t base_addr;
+} CmdQDesc;
+
 struct GICv3ITSClass {
     GICv3ITSCommonClass parent_class;
     void (*parent_reset)(DeviceState *dev);
+
+    DevTableDesc  dt;
+    CollTableDesc ct;
+    CmdQDesc      cq;
 };
 
+static bool extract_table_params(GICv3ITSState *s, int index)
+{
+    GICv3ITSClass *c = ARM_GICV3_ITS_GET_CLASS(s);
+    uint16_t num_pages = 0;
+    uint8_t  page_sz_type;
+    uint8_t type;
+    uint32_t page_sz = 0;
+    uint64_t value = s->baser[index];
+
+    num_pages = (value & GITS_BASER_SIZE);
+    page_sz_type = (value >> GITS_BASER_PAGESIZE_OFFSET) &
+                        GITS_BASER_PAGESIZE_MASK;
+
+    if (page_sz_type == 0) {
+        page_sz = GITS_ITT_PAGE_SIZE_0;
+    } else if (page_sz_type == 0) {
+        page_sz = GITS_ITT_PAGE_SIZE_1;
+    } else if (page_sz_type == 2) {
+        page_sz = GITS_ITT_PAGE_SIZE_2;
+    } else {
+        return false;
+    }
+
+    type = (value >> GITS_BASER_TYPE_OFFSET) &
+                        GITS_BASER_TYPE_MASK;
+
+    if (type == GITS_ITT_TYPE_DEVICE) {
+        c->dt.valid = (value >> GITS_BASER_VALID) & GITS_BASER_VALID_MASK;
+
+        if (c->dt.valid) {
+            c->dt.indirect = (value >> GITS_BASER_INDIRECT_OFFSET) &
+                                       GITS_BASER_INDIRECT_MASK;
+            c->dt.entry_sz = (value >> GITS_BASER_ENTRYSIZE_OFFSET) &
+                                   GITS_BASER_ENTRYSIZE_MASK;
+
+            if (!c->dt.indirect) {
+                c->dt.max_entries = ((num_pages + 1) * page_sz) /
+                                                       c->dt.entry_sz;
+            } else {
+                c->dt.max_entries = ((((num_pages + 1) * page_sz) /
+                                        L1TABLE_ENTRY_SIZE) *
+                                    (page_sz / c->dt.entry_sz));
+            }
+
+            c->dt.max_devids = (1UL << (((value >> GITS_TYPER_DEVBITS_OFFSET) &
+                                           GITS_TYPER_DEVBITS_MASK) + 1));
+
+            if ((page_sz == GITS_ITT_PAGE_SIZE_0) ||
+                    (page_sz == GITS_ITT_PAGE_SIZE_1)) {
+                c->dt.base_addr = (value >> GITS_BASER_PHYADDR_OFFSET) &
+                                        GITS_BASER_PHYADDR_MASK;
+                c->dt.base_addr <<= GITS_BASER_PHYADDR_OFFSET;
+            } else if (page_sz == GITS_ITT_PAGE_SIZE_2) {
+                c->dt.base_addr = ((value >> GITS_BASER_PHYADDR_OFFSETL_64K) &
+                                   GITS_BASER_PHYADDR_MASKL_64K) <<
+                                     GITS_BASER_PHYADDR_OFFSETL_64K;
+                c->dt.base_addr |= ((value >> GITS_BASER_PHYADDR_OFFSET) &
+                                    GITS_BASER_PHYADDR_MASKH_64K) <<
+                                     GITS_BASER_PHYADDR_OFFSETH_64K;
+            }
+        }
+    } else if (type == GITS_ITT_TYPE_COLLECTION) {
+        c->ct.valid = (value >> GITS_BASER_VALID) & GITS_BASER_VALID_MASK;
+
+        /*
+         * GITS_TYPER.HCC is 0 for this implementation
+         * hence writes are discarded if ct.valid is 0
+         */
+        if (c->ct.valid) {
+            c->ct.indirect = (value >> GITS_BASER_INDIRECT_OFFSET) &
+                                       GITS_BASER_INDIRECT_MASK;
+            c->ct.entry_sz = (value >> GITS_BASER_ENTRYSIZE_OFFSET) &
+                                    GITS_BASER_ENTRYSIZE_MASK;
+
+            if (!c->ct.indirect) {
+                c->ct.max_entries = ((num_pages + 1) * page_sz) /
+                                      c->ct.entry_sz;
+            } else {
+                c->ct.max_entries = ((((num_pages + 1) * page_sz) /
+                                      L1TABLE_ENTRY_SIZE) *
+                                      (page_sz / c->ct.entry_sz));
+            }
+
+            if ((value >> GITS_TYPER_CIL_OFFSET) & GITS_TYPER_CIL_MASK) {
+                c->ct.max_collids = (1UL << (((value >>
+                                               GITS_TYPER_CIDBITS_OFFSET) &
+                                               GITS_TYPER_CIDBITS_MASK) + 1));
+            } else {
+                /* 16-bit CollectionId supported when CIL == 0 */
+                c->ct.max_collids = (1UL << 16);
+            }
+
+            if ((page_sz == GITS_ITT_PAGE_SIZE_0) ||
+                 (page_sz == GITS_ITT_PAGE_SIZE_1)) {
+                c->ct.base_addr = (value >> GITS_BASER_PHYADDR_OFFSET) &
+                                            GITS_BASER_PHYADDR_MASK;
+                c->ct.base_addr <<= GITS_BASER_PHYADDR_OFFSET;
+            } else if (page_sz == GITS_ITT_PAGE_SIZE_2) {
+                c->ct.base_addr = ((value >> GITS_BASER_PHYADDR_OFFSETL_64K) &
+                                   GITS_BASER_PHYADDR_MASKL_64K) <<
+                                    GITS_BASER_PHYADDR_OFFSETL_64K;
+                c->ct.base_addr |= ((value >> GITS_BASER_PHYADDR_OFFSET) &
+                                    GITS_BASER_PHYADDR_MASKH_64K) <<
+                                    GITS_BASER_PHYADDR_OFFSETH_64K;
+            }
+        }
+    } else {
+        /* unsupported ITS table type */
+        qemu_log_mask(LOG_GUEST_ERROR, "%s: Unsupported ITS table type %d",
+                         __func__, type);
+        return false;
+    }
+    return true;
+}
+
+static bool extract_cmdq_params(GICv3ITSState *s)
+{
+    GICv3ITSClass *c = ARM_GICV3_ITS_GET_CLASS(s);
+    uint16_t num_pages = 0;
+    uint64_t value = s->cbaser;
+
+    num_pages = (value & GITS_CBASER_SIZE);
+
+    c->cq.valid = (value >> GITS_CBASER_VALID_OFFSET) &
+                                GITS_CBASER_VALID_MASK;
+
+    if (!num_pages || !c->cq.valid) {
+        return false;
+    }
+
+    if (c->cq.valid) {
+        c->cq.max_entries = ((num_pages + 1) * GITS_ITT_PAGE_SIZE_0) /
+                                                GITS_CMDQ_ENTRY_SIZE;
+        c->cq.base_addr = (value >> GITS_CBASER_PHYADDR_OFFSET) &
+                                    GITS_CBASER_PHYADDR_MASK;
+        c->cq.base_addr <<= GITS_CBASER_PHYADDR_OFFSET;
+    }
+    return true;
+}
+
 static MemTxResult its_trans_writew(GICv3ITSState *s, hwaddr offset,
                                uint64_t value, MemTxAttrs attrs)
 {
@@ -126,7 +294,75 @@ static MemTxResult its_writel(GICv3ITSState *s, hwaddr offset,
                                uint64_t value, MemTxAttrs attrs)
 {
     MemTxResult result = MEMTX_OK;
+    int index;
+    uint64_t temp = 0;
 
+    switch (offset) {
+    case GITS_CTLR:
+        s->ctlr |= (value & ~(s->ctlr));
+        break;
+    case GITS_CBASER:
+        /* GITS_CBASER register becomes RO if ITS is already enabled */
+        if (!(s->ctlr & GITS_CTLR_ENABLED)) {
+            s->cbaser = deposit64(s->cbaser, 0, 32, value);
+            s->creadr = 0;
+        }
+        break;
+    case GITS_CBASER + 4:
+        /* GITS_CBASER register becomes RO if ITS is already enabled */
+        if (!(s->ctlr & GITS_CTLR_ENABLED)) {
+            s->cbaser = deposit64(s->cbaser, 32, 32, value);
+            if (!extract_cmdq_params(s)) {
+                qemu_log_mask(LOG_GUEST_ERROR,
+                       "%s: error extracting GITS_CBASER parameters "
+                       TARGET_FMT_plx "\n", __func__, offset);
+                s->cbaser = 0;
+                result = MEMTX_ERROR;
+            } else {
+                s->creadr = 0;
+            }
+        }
+        break;
+    case GITS_CWRITER:
+        s->cwriter = deposit64(s->cwriter, 0, 32, value);
+        break;
+    case GITS_CWRITER + 4:
+        s->cwriter = deposit64(s->cwriter, 32, 32, value);
+        break;
+    case GITS_BASER ... GITS_BASER + 0x3f:
+        /* GITS_BASERn registers become RO if ITS is already enabled */
+        if (!(s->ctlr & GITS_CTLR_ENABLED)) {
+            index = (offset - GITS_BASER) / 8;
+
+            if (offset & 7) {
+                temp = s->baser[index];
+                temp = deposit64(temp, 32, 32, (value & ~GITS_BASER_VAL_MASK));
+                s->baser[index] |= temp;
+
+                if (!extract_table_params(s, index)) {
+                    qemu_log_mask(LOG_GUEST_ERROR,
+                        "%s: error extracting GITS_BASER parameters "
+                        TARGET_FMT_plx "\n", __func__, offset);
+                    s->baser[index] = 0;
+                    result = MEMTX_ERROR;
+                }
+            } else {
+                s->baser[index] =  deposit64(s->baser[index], 0, 32, value);
+            }
+        }
+        break;
+    case GITS_IIDR:
+    case GITS_TYPER:
+    case GITS_CREADR:
+        /* RO registers, ignore the write */
+        qemu_log_mask(LOG_GUEST_ERROR,
+            "%s: invalid guest write to RO register at offset "
+            TARGET_FMT_plx "\n", __func__, offset);
+        break;
+    default:
+        result = MEMTX_ERROR;
+        break;
+    }
     return result;
 }
 
@@ -134,7 +370,54 @@ static MemTxResult its_readl(GICv3ITSState *s, hwaddr offset,
                                uint64_t *data, MemTxAttrs attrs)
 {
     MemTxResult result = MEMTX_OK;
+    int index;
 
+    switch (offset) {
+    case GITS_CTLR:
+        *data = s->ctlr;
+        break;
+    case GITS_IIDR:
+        *data = s->iidr;
+        break;
+    case GITS_PIDR2:
+        *data = 0x30; /* GICv3 */
+        break;
+    case GITS_TYPER:
+        *data = extract64(s->typer, 0, 32);
+        break;
+    case GITS_TYPER + 4:
+        *data = extract64(s->typer, 32, 32);
+        break;
+    case GITS_CBASER:
+        *data = extract64(s->cbaser, 0, 32);
+        break;
+    case GITS_CBASER + 4:
+        *data = extract64(s->cbaser, 32, 32);
+        break;
+    case GITS_CREADR:
+        *data = extract64(s->creadr, 0, 32);
+        break;
+    case GITS_CREADR + 4:
+        *data = extract64(s->creadr, 32, 32);
+        break;
+    case GITS_CWRITER:
+        *data = extract64(s->cwriter, 0, 32);
+        break;
+    case GITS_CWRITER + 4:
+        *data = extract64(s->cwriter, 32, 32);
+        break;
+    case GITS_BASER ... GITS_BASER + 0x3f:
+        index = (offset - GITS_BASER) / 8;
+        if (offset & 7) {
+            *data = s->baser[index] >> 32;
+        } else {
+            *data = (uint32_t)s->baser[index];
+        }
+        break;
+    default:
+        result = MEMTX_ERROR;
+        break;
+    }
     return result;
 }
 
@@ -142,7 +425,52 @@ static MemTxResult its_writell(GICv3ITSState *s, hwaddr offset,
                                uint64_t value, MemTxAttrs attrs)
 {
     MemTxResult result = MEMTX_OK;
+    int index;
 
+    switch (offset) {
+    case GITS_BASER ... GITS_BASER + 0x3f:
+        /* GITS_BASERn registers become RO if ITS is already enabled */
+        if (!(s->ctlr & GITS_CTLR_ENABLED)) {
+            index = (offset - GITS_BASER) / 8;
+            s->baser[index] |= (value & ~GITS_BASER_VAL_MASK);
+            if (!extract_table_params(s, index)) {
+                qemu_log_mask(LOG_GUEST_ERROR,
+                        "%s: error extracting GITS_BASER parameters "
+                        TARGET_FMT_plx "\n", __func__, offset);
+                s->baser[index] = 0;
+                result = MEMTX_ERROR;
+            }
+        }
+        break;
+    case GITS_CBASER:
+        /* GITS_CBASER register becomes RO if ITS is already enabled */
+        if (!(s->ctlr & GITS_CTLR_ENABLED)) {
+            s->cbaser = value;
+            if (!extract_cmdq_params(s)) {
+                qemu_log_mask(LOG_GUEST_ERROR,
+                       "%s: error extracting GITS_CBASER parameters "
+                       TARGET_FMT_plx "\n", __func__, offset);
+                s->cbaser = 0;
+                result = MEMTX_ERROR;
+            } else {
+                s->creadr = 0;
+            }
+        }
+        break;
+    case GITS_CWRITER:
+        s->cwriter = value;
+        break;
+    case GITS_TYPER:
+    case GITS_CREADR:
+        /* RO register, ignore the write */
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "%s: invalid guest write to RO register at offset "
+                      TARGET_FMT_plx "\n", __func__, offset);
+        break;
+    default:
+        result = MEMTX_ERROR;
+        break;
+    }
     return result;
 }
 
@@ -150,7 +478,29 @@ static MemTxResult its_readll(GICv3ITSState *s, hwaddr offset,
                                uint64_t *data, MemTxAttrs attrs)
 {
     MemTxResult result = MEMTX_OK;
+    int index;
 
+    switch (offset) {
+    case GITS_TYPER:
+        *data = s->typer;
+        break;
+    case GITS_BASER ... GITS_BASER + 0x3f:
+        index = (offset - GITS_BASER) / 8;
+        *data = s->baser[index];
+        break;
+    case GITS_CBASER:
+        *data = s->cbaser;
+        break;
+    case GITS_CREADR:
+        *data = s->creadr;
+        break;
+    case GITS_CWRITER:
+        *data = s->cwriter;
+        break;
+    default:
+        result = MEMTX_ERROR;
+        break;
+    }
     return result;
 }
 
@@ -250,6 +600,9 @@ static void gicv3_arm_its_realize(DeviceState *dev, Error **errp)
     GICv3ITSState *s = ARM_GICV3_ITS_COMMON(dev);
 
     gicv3_its_init_mmio(s, &gicv3_its_control_ops, &gicv3_its_trans_ops);
+
+    address_space_init(&s->gicv3->sysmem_as, s->gicv3->sysmem,
+                        "gicv3-its-sysmem");
 }
 
 static void gicv3_its_reset(DeviceState *dev)
@@ -259,6 +612,9 @@ static void gicv3_its_reset(DeviceState *dev)
 
     if (s->gicv3->cpu->gicr_typer & GICR_TYPER_PLPIS) {
         c->parent_reset(dev);
+        memset(&c->dt, 0 , sizeof(c->dt));
+        memset(&c->ct, 0 , sizeof(c->ct));
+        memset(&c->cq, 0 , sizeof(c->cq));
 
         /* set the ITS default features supported */
         s->typer = GITS_TYPER_PHYSICAL | (GITS_TYPER_ITT_ENTRY_SIZE <<
diff --git a/include/hw/intc/arm_gicv3_common.h b/include/hw/intc/arm_gicv3_common.h
index 91491a2f66..b0f2414fa3 100644
--- a/include/hw/intc/arm_gicv3_common.h
+++ b/include/hw/intc/arm_gicv3_common.h
@@ -226,12 +226,16 @@ struct GICv3State {
     int dev_fd; /* kvm device fd if backed by kvm vgic support */
     Error *migration_blocker;
 
+    MemoryRegion *sysmem;
+    AddressSpace sysmem_as;
+
     /* Distributor */
 
     /* for a GIC with the security extensions the NS banked version of this
      * register is just an alias of bit 1 of the S banked version.
      */
     uint32_t gicd_ctlr;
+    uint32_t gicd_typer;
     uint32_t gicd_statusr[2];
     GIC_DECLARE_BITMAP(group);        /* GICD_IGROUPR */
     GIC_DECLARE_BITMAP(grpmod);       /* GICD_IGRPMODR */
-- 
2.27.0



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

* [PATCH v1 3/8] hw/intc: GICv3 ITS command queue framework
  2021-03-23  4:12 [PATCH v1 0/8] GICv3 LPI and ITS feature implementation Shashi Mallela
  2021-03-23  4:12 ` [PATCH v1 1/8] hw/intc: GICv3 ITS initial framework Shashi Mallela
  2021-03-23  4:12 ` [PATCH v1 2/8] hw/intc: GICv3 ITS register definitions added Shashi Mallela
@ 2021-03-23  4:12 ` Shashi Mallela
  2021-03-23  4:12 ` [PATCH v1 4/8] hw/intc: GICv3 ITS Command processing Shashi Mallela
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 16+ messages in thread
From: Shashi Mallela @ 2021-03-23  4:12 UTC (permalink / raw)
  To: peter.maydell, leif, rad; +Cc: qemu-arm, qemu-devel

Added functionality to trigger ITS command queue processing on
write to CWRITE register and process each command queue entry to
identify the command type and handle commands like MAPD,MAPC,SYNC.

Signed-off-by: Shashi Mallela <shashi.mallela@linaro.org>
---
 hw/intc/arm_gicv3_its.c | 362 ++++++++++++++++++++
 1 file changed, 362 insertions(+)

diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c
index 4895d32e67..9b094e1f0a 100644
--- a/hw/intc/arm_gicv3_its.c
+++ b/hw/intc/arm_gicv3_its.c
@@ -56,6 +56,362 @@ struct GICv3ITSClass {
     CmdQDesc      cq;
 };
 
+static MemTxResult process_sync(GICv3ITSState *s, uint32_t offset)
+{
+    GICv3ITSClass *c = ARM_GICV3_ITS_GET_CLASS(s);
+    AddressSpace *as = &s->gicv3->sysmem_as;
+    uint64_t rdbase;
+    uint64_t value;
+    bool pta = false;
+    MemTxResult res = MEMTX_OK;
+
+    offset += NUM_BYTES_IN_DW;
+    offset += NUM_BYTES_IN_DW;
+
+    value = address_space_ldq_le(as, c->cq.base_addr + offset,
+                                     MEMTXATTRS_UNSPECIFIED, &res);
+
+    if ((s->typer >> GITS_TYPER_PTA_OFFSET) & GITS_TYPER_PTA_MASK) {
+        /*
+         * only bits[47:16] are considered instead of bits [51:16]
+         * since with a physical address the target address must be
+         * 64KB aligned
+         */
+        rdbase = (value >> RDBASE_OFFSET) & RDBASE_MASK;
+        pta = true;
+    } else {
+        rdbase = (value >> RDBASE_OFFSET) & RDBASE_PROCNUM_MASK;
+    }
+
+    if (!pta && (rdbase < (s->gicv3->num_cpu))) {
+        /*
+         * Current implementation makes a blocking synchronous call
+         * for every command issued earlier,hence the internal state
+         * is already consistent by the time SYNC command is executed.
+         */
+    }
+
+    offset += NUM_BYTES_IN_DW;
+    return res;
+}
+
+static void update_cte(GICv3ITSState *s, uint16_t icid, uint64_t cte)
+{
+    GICv3ITSClass *c = ARM_GICV3_ITS_GET_CLASS(s);
+    AddressSpace *as = &s->gicv3->sysmem_as;
+    uint64_t value;
+    uint8_t  page_sz_type;
+    uint64_t l2t_addr;
+    bool valid_l2t;
+    uint32_t l2t_id;
+    uint32_t page_sz = 0;
+    uint32_t max_l2_entries;
+
+    if (c->ct.indirect) {
+        /* 2 level table */
+        page_sz_type = (s->baser[0] >>
+                        GITS_BASER_PAGESIZE_OFFSET) &
+                        GITS_BASER_PAGESIZE_MASK;
+
+        if (page_sz_type == 0) {
+            page_sz = GITS_ITT_PAGE_SIZE_0;
+        } else if (page_sz_type == 1) {
+            page_sz = GITS_ITT_PAGE_SIZE_1;
+        } else if (page_sz_type == 2) {
+            page_sz = GITS_ITT_PAGE_SIZE_2;
+        }
+
+        l2t_id = icid / (page_sz / L1TABLE_ENTRY_SIZE);
+
+        value = address_space_ldq_le(as,
+                                     c->ct.base_addr +
+                                     (l2t_id * L1TABLE_ENTRY_SIZE),
+                                     MEMTXATTRS_UNSPECIFIED, NULL);
+
+        valid_l2t = (value >> VALID_SHIFT) & VALID_MASK;
+
+        if (valid_l2t) {
+            max_l2_entries = page_sz / c->ct.entry_sz;
+
+            l2t_addr = (value >> page_sz_type) &
+                        ((1ULL << (51 - page_sz_type)) - 1);
+
+            address_space_write(as, l2t_addr +
+                                 ((icid % max_l2_entries) * GITS_CTE_SIZE),
+                                 MEMTXATTRS_UNSPECIFIED,
+                                 &cte, sizeof(cte));
+        }
+    } else {
+        /* Flat level table */
+        address_space_write(as, c->ct.base_addr + (icid * GITS_CTE_SIZE),
+                            MEMTXATTRS_UNSPECIFIED, &cte,
+                            sizeof(cte));
+    }
+}
+
+static MemTxResult process_mapc(GICv3ITSState *s, uint32_t offset)
+{
+    GICv3ITSClass *c = ARM_GICV3_ITS_GET_CLASS(s);
+    AddressSpace *as = &s->gicv3->sysmem_as;
+    uint16_t icid;
+    uint64_t rdbase;
+    bool valid;
+    bool pta = false;
+    MemTxResult res = MEMTX_OK;
+    uint64_t cte_entry;
+    uint64_t value;
+
+    offset += NUM_BYTES_IN_DW;
+    offset += NUM_BYTES_IN_DW;
+
+    value = address_space_ldq_le(as, c->cq.base_addr + offset,
+                                 MEMTXATTRS_UNSPECIFIED, &res);
+
+    icid = value & ICID_MASK;
+
+    if ((s->typer >> GITS_TYPER_PTA_OFFSET) & GITS_TYPER_PTA_MASK) {
+        /*
+         * only bits[47:16] are considered instead of bits [51:16]
+         * since with a physical address the target address must be
+         * 64KB aligned
+         */
+        rdbase = (value >> RDBASE_OFFSET) & RDBASE_MASK;
+        pta = true;
+    } else {
+        rdbase = (value >> RDBASE_OFFSET) & RDBASE_PROCNUM_MASK;
+    }
+
+    valid = (value >> VALID_SHIFT) & VALID_MASK;
+
+    if (valid) {
+        if ((icid > c->ct.max_collids) || (!pta &&
+                (rdbase > s->gicv3->num_cpu))) {
+            if ((s->typer >> GITS_TYPER_SEIS_OFFSET) &
+                             GITS_TYPER_SEIS_MASK) {
+                /* Generate System Error here if supported */
+            }
+            qemu_log_mask(LOG_GUEST_ERROR,
+                "%s: invalid collection table attributes "
+                "icid %d rdbase %lu\n", __func__, icid, rdbase);
+            /*
+             * in this implementation,in case of error
+             * we ignore this command and move onto the next
+             * command in the queue
+             */
+        } else {
+            if (c->ct.valid) {
+                /* add mapping entry to collection table */
+                cte_entry = (valid & VALID_MASK) |
+                            (pta ? ((rdbase & RDBASE_MASK) << 1ULL) :
+                            ((rdbase & RDBASE_PROCNUM_MASK) << 1ULL));
+
+                update_cte(s, icid, cte_entry);
+            }
+        }
+    } else {
+        if (c->ct.valid) {
+            /* remove mapping entry from collection table */
+            cte_entry = 0;
+
+            update_cte(s, icid, cte_entry);
+        }
+    }
+
+    offset += NUM_BYTES_IN_DW;
+    offset += NUM_BYTES_IN_DW;
+
+    return res;
+}
+
+static void update_dte(GICv3ITSState *s, uint32_t devid, uint64_t dte)
+{
+    GICv3ITSClass *c = ARM_GICV3_ITS_GET_CLASS(s);
+    AddressSpace *as = &s->gicv3->sysmem_as;
+    uint64_t value;
+    uint8_t  page_sz_type;
+    uint64_t l2t_addr;
+    bool valid_l2t;
+    uint32_t l2t_id;
+    uint32_t page_sz = 0;
+    uint32_t max_l2_entries;
+
+    if (c->dt.indirect) {
+        /* 2 level table */
+        page_sz_type = (s->baser[0] >>
+                        GITS_BASER_PAGESIZE_OFFSET) &
+                        GITS_BASER_PAGESIZE_MASK;
+
+        if (page_sz_type == 0) {
+            page_sz = GITS_ITT_PAGE_SIZE_0;
+        } else if (page_sz_type == 1) {
+            page_sz = GITS_ITT_PAGE_SIZE_1;
+        } else if (page_sz_type == 2) {
+            page_sz = GITS_ITT_PAGE_SIZE_2;
+        }
+
+        l2t_id = devid / (page_sz / L1TABLE_ENTRY_SIZE);
+
+        value = address_space_ldq_le(as,
+                                     c->dt.base_addr +
+                                     (l2t_id * L1TABLE_ENTRY_SIZE),
+                                     MEMTXATTRS_UNSPECIFIED, NULL);
+
+        valid_l2t = (value >> VALID_SHIFT) & VALID_MASK;
+
+        if (valid_l2t) {
+            max_l2_entries = page_sz / c->dt.entry_sz;
+
+            l2t_addr = (value >> page_sz_type) &
+                        ((1ULL << (51 - page_sz_type)) - 1);
+
+            address_space_write(as, l2t_addr +
+                                 ((devid % max_l2_entries) * GITS_DTE_SIZE),
+                                 MEMTXATTRS_UNSPECIFIED, &dte, sizeof(dte));
+        }
+    } else {
+        /* Flat level table */
+        address_space_write(as, c->dt.base_addr + (devid * GITS_DTE_SIZE),
+                            MEMTXATTRS_UNSPECIFIED, &dte, sizeof(dte));
+    }
+}
+
+static MemTxResult process_mapd(GICv3ITSState *s, uint64_t value,
+                                 uint32_t offset)
+{
+    GICv3ITSClass *c = ARM_GICV3_ITS_GET_CLASS(s);
+    AddressSpace *as = &s->gicv3->sysmem_as;
+    uint32_t devid;
+    uint8_t size;
+    uint64_t itt_addr;
+    bool valid;
+    MemTxResult res = MEMTX_OK;
+    uint64_t dte_entry = 0;
+
+    devid = (value >> DEVID_OFFSET) & DEVID_MASK;
+
+    offset += NUM_BYTES_IN_DW;
+    value = address_space_ldq_le(as, c->cq.base_addr + offset,
+                                 MEMTXATTRS_UNSPECIFIED, &res);
+    size = (value & SIZE_MASK);
+
+    offset += NUM_BYTES_IN_DW;
+    value = address_space_ldq_le(as, c->cq.base_addr + offset,
+                                 MEMTXATTRS_UNSPECIFIED, &res);
+    itt_addr = (value >> ITTADDR_OFFSET) & ITTADDR_MASK;
+
+    valid = (value >> VALID_SHIFT) & VALID_MASK;
+
+    if (valid) {
+        if ((devid > c->dt.max_devids) ||
+            (size > ((s->typer >> GITS_TYPER_IDBITS_OFFSET) &
+                          GITS_TYPER_IDBITS_MASK))) {
+            if ((s->typer >> GITS_TYPER_SEIS_OFFSET) &
+                             GITS_TYPER_SEIS_MASK) {
+                /* Generate System Error here if supported */
+            }
+            qemu_log_mask(LOG_GUEST_ERROR,
+                "%s: invalid device table attributes "
+                "devid %d or size %d\n", __func__, devid, size);
+            /*
+             * in this implementation,in case of error
+             * we ignore this command and move onto the next
+             * command in the queue
+             */
+        } else {
+            if (c->dt.valid) {
+                /* add mapping entry to device table */
+                dte_entry = (valid & VALID_MASK) |
+                            ((size & SIZE_MASK) << 1U) |
+                            ((itt_addr & ITTADDR_MASK) << 6ULL);
+
+                update_dte(s, devid, dte_entry);
+            }
+        }
+    } else {
+        if (c->dt.valid) {
+            /* remove mapping entry from device table */
+            dte_entry = 0;
+            update_dte(s, devid, dte_entry);
+        }
+    }
+
+    offset += NUM_BYTES_IN_DW;
+    offset += NUM_BYTES_IN_DW;
+
+    return res;
+}
+
+/*
+ * Current implementation blocks until all
+ * commands are processed
+ */
+static MemTxResult process_cmdq(GICv3ITSState *s)
+{
+    GICv3ITSClass *c = ARM_GICV3_ITS_GET_CLASS(s);
+    uint32_t wr_offset = 0;
+    uint32_t rd_offset = 0;
+    uint32_t cq_offset = 0;
+    uint64_t data;
+    AddressSpace *as = &s->gicv3->sysmem_as;
+    MemTxResult res = MEMTX_OK;
+    uint8_t cmd;
+
+    wr_offset = (s->cwriter >> GITS_CWRITER_OFFSET) &
+                             GITS_CWRITER_OFFSET_MASK;
+
+    if (wr_offset > c->cq.max_entries) {
+        qemu_log_mask(LOG_GUEST_ERROR,
+                        "%s: invalid write offset "
+                        "%d\n", __func__, wr_offset);
+        res = MEMTX_ERROR;
+        return res;
+    }
+
+    rd_offset = (s->creadr >> GITS_CREADR_OFFSET) &
+                             GITS_CREADR_OFFSET_MASK;
+
+    while (wr_offset != rd_offset) {
+        cq_offset = (rd_offset * GITS_CMDQ_ENTRY_SIZE);
+        data = address_space_ldq_le(as, c->cq.base_addr + cq_offset,
+                                      MEMTXATTRS_UNSPECIFIED, &res);
+        cmd = (data & CMD_MASK);
+
+        switch (cmd) {
+        case GITS_CMD_INT:
+            break;
+        case GITS_CMD_CLEAR:
+            break;
+        case GITS_CMD_SYNC:
+            res = process_sync(s, cq_offset);
+            break;
+        case GITS_CMD_MAPD:
+            res = process_mapd(s, data, cq_offset);
+            break;
+        case GITS_CMD_MAPC:
+            res = process_mapc(s, cq_offset);
+            break;
+        case GITS_CMD_MAPTI:
+            break;
+        case GITS_CMD_MAPI:
+            break;
+        case GITS_CMD_DISCARD:
+            break;
+        default:
+            break;
+        }
+        if (res == MEMTX_OK) {
+            rd_offset++;
+            rd_offset %= c->cq.max_entries;
+            s->creadr = (rd_offset << GITS_CREADR_OFFSET);
+        } else {
+            qemu_log_mask(LOG_GUEST_ERROR,
+                "%s: %x cmd processing failed!!\n", __func__, cmd);
+            break;
+        }
+    }
+    return res;
+}
+
 static bool extract_table_params(GICv3ITSState *s, int index)
 {
     GICv3ITSClass *c = ARM_GICV3_ITS_GET_CLASS(s);
@@ -325,6 +681,9 @@ static MemTxResult its_writel(GICv3ITSState *s, hwaddr offset,
         break;
     case GITS_CWRITER:
         s->cwriter = deposit64(s->cwriter, 0, 32, value);
+        if ((s->ctlr & GITS_CTLR_ENABLED) && (s->cwriter != s->creadr)) {
+            result = process_cmdq(s);
+        }
         break;
     case GITS_CWRITER + 4:
         s->cwriter = deposit64(s->cwriter, 32, 32, value);
@@ -459,6 +818,9 @@ static MemTxResult its_writell(GICv3ITSState *s, hwaddr offset,
         break;
     case GITS_CWRITER:
         s->cwriter = value;
+        if ((s->ctlr & GITS_CTLR_ENABLED) && (s->cwriter != s->creadr)) {
+            result = process_cmdq(s);
+        }
         break;
     case GITS_TYPER:
     case GITS_CREADR:
-- 
2.27.0



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

* [PATCH v1 4/8] hw/intc: GICv3 ITS Command processing
  2021-03-23  4:12 [PATCH v1 0/8] GICv3 LPI and ITS feature implementation Shashi Mallela
                   ` (2 preceding siblings ...)
  2021-03-23  4:12 ` [PATCH v1 3/8] hw/intc: GICv3 ITS command queue framework Shashi Mallela
@ 2021-03-23  4:12 ` Shashi Mallela
  2021-03-23  4:12 ` [PATCH v1 5/8] hw/intc: GICv3 ITS Feature enablement Shashi Mallela
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 16+ messages in thread
From: Shashi Mallela @ 2021-03-23  4:12 UTC (permalink / raw)
  To: peter.maydell, leif, rad; +Cc: qemu-arm, qemu-devel

Added ITS command queue handling for MAPTI,MAPI commands,handled ITS
translation which triggers an LPI via INT command as well as write
to GITS_TRANSLATER register,defined enum to differentiate between ITS
command interrupt trigger and GITS_TRANSLATER based interrupt trigger.
Each of these commands make use of other functionalities implemented to
get device table entry,collection table entry or interrupt translation
table entry required for their processing.

Signed-off-by: Shashi Mallela <shashi.mallela@linaro.org>
---
 hw/intc/arm_gicv3_its.c            | 371 +++++++++++++++++++-
 include/hw/intc/arm_gicv3_common.h |   2 +
 2 files changed, 372 insertions(+), 1 deletion(-)

diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c
index 9b094e1f0a..de2d179b5e 100644
--- a/hw/intc/arm_gicv3_its.c
+++ b/hw/intc/arm_gicv3_its.c
@@ -56,6 +56,158 @@ struct GICv3ITSClass {
     CmdQDesc      cq;
 };
 
+typedef enum ItsCmdType {
+    NONE = 0, /* internal indication for GITS_TRANSLATER write */
+    CLEAR = 1,
+    DISCARD = 2,
+    INT = 3,
+} ItsCmdType;
+
+static bool get_cte(GICv3ITSState *s, uint16_t icid, uint64_t *cte)
+{
+    GICv3ITSClass *c = ARM_GICV3_ITS_GET_CLASS(s);
+    AddressSpace *as = &s->gicv3->sysmem_as;
+    uint8_t  page_sz_type;
+    uint64_t l2t_addr;
+    uint64_t value;
+    bool valid_l2t;
+    uint32_t l2t_id;
+    uint32_t page_sz = 0;
+    uint32_t max_l2_entries;
+    bool status = false;
+
+    if (c->ct.indirect) {
+        /* 2 level table */
+        page_sz_type = (s->baser[0] >>
+                        GITS_BASER_PAGESIZE_OFFSET) &
+                        GITS_BASER_PAGESIZE_MASK;
+
+        if (page_sz_type == 0) {
+            page_sz = GITS_ITT_PAGE_SIZE_0;
+        } else if (page_sz_type == 1) {
+            page_sz = GITS_ITT_PAGE_SIZE_1;
+        } else if (page_sz_type == 2) {
+            page_sz = GITS_ITT_PAGE_SIZE_2;
+        }
+
+        l2t_id = icid / (page_sz / L1TABLE_ENTRY_SIZE);
+
+        value = address_space_ldq_le(as,
+                                     c->ct.base_addr +
+                                     (l2t_id * L1TABLE_ENTRY_SIZE),
+                                     MEMTXATTRS_UNSPECIFIED, NULL);
+
+        valid_l2t = (value >> VALID_SHIFT) & VALID_MASK;
+
+        if (valid_l2t) {
+            max_l2_entries = page_sz / c->ct.entry_sz;
+
+            l2t_addr = (value >> page_sz_type) &
+                        ((1ULL << (51 - page_sz_type)) - 1);
+
+            address_space_read(as, l2t_addr +
+                                 ((icid % max_l2_entries) * GITS_CTE_SIZE),
+                                 MEMTXATTRS_UNSPECIFIED,
+                                 cte, sizeof(*cte));
+       }
+    } else {
+        /* Flat level table */
+        address_space_read(as, c->ct.base_addr + (icid * GITS_CTE_SIZE),
+                            MEMTXATTRS_UNSPECIFIED, cte,
+                            sizeof(*cte));
+    }
+
+    if (*cte & VALID_MASK) {
+        status = true;
+    }
+
+    return status;
+}
+
+static bool get_ite(GICv3ITSState *s, uint32_t eventid, uint64_t dte,
+                      uint16_t *icid, uint32_t *pIntid)
+{
+    AddressSpace *as = &s->gicv3->sysmem_as;
+    uint8_t buff[GITS_TYPER_ITT_ENTRY_SIZE];
+    uint64_t itt_addr;
+    bool status = false;
+
+    itt_addr = (dte >> 6ULL) & ITTADDR_MASK;
+    itt_addr <<= ITTADDR_OFFSET; /* 256 byte aligned */
+
+    address_space_read(as, itt_addr + (eventid * sizeof(buff)),
+                MEMTXATTRS_UNSPECIFIED, &buff,
+                sizeof(buff));
+
+    if (buff[0] & VALID_MASK) {
+        if ((buff[0] >> 1U) & GITS_TYPER_PHYSICAL) {
+            memcpy(pIntid, &buff[1], 3);
+            memcpy(icid, &buff[7], sizeof(*icid));
+            status = true;
+        }
+    }
+
+    return status;
+}
+
+static uint64_t get_dte(GICv3ITSState *s, uint32_t devid)
+{
+    GICv3ITSClass *c = ARM_GICV3_ITS_GET_CLASS(s);
+    AddressSpace *as = &s->gicv3->sysmem_as;
+    uint8_t  page_sz_type;
+    uint64_t l2t_addr;
+    uint64_t value;
+    bool valid_l2t;
+    uint32_t l2t_id;
+    uint32_t page_sz = 0;
+    uint32_t max_l2_entries;
+
+    if (c->ct.indirect) {
+        /* 2 level table */
+        page_sz_type = (s->baser[0] >>
+                        GITS_BASER_PAGESIZE_OFFSET) &
+                        GITS_BASER_PAGESIZE_MASK;
+
+        if (page_sz_type == 0) {
+            page_sz = GITS_ITT_PAGE_SIZE_0;
+        } else if (page_sz_type == 1) {
+            page_sz = GITS_ITT_PAGE_SIZE_1;
+        } else if (page_sz_type == 2) {
+            page_sz = GITS_ITT_PAGE_SIZE_2;
+        }
+
+        l2t_id = devid / (page_sz / L1TABLE_ENTRY_SIZE);
+
+        value = address_space_ldq_le(as,
+                                     c->dt.base_addr +
+                                     (l2t_id * L1TABLE_ENTRY_SIZE),
+                                     MEMTXATTRS_UNSPECIFIED, NULL);
+
+        valid_l2t = (value >> VALID_SHIFT) & VALID_MASK;
+
+        if (valid_l2t) {
+            max_l2_entries = page_sz / c->dt.entry_sz;
+
+            l2t_addr = (value >> page_sz_type) &
+                        ((1ULL << (51 - page_sz_type)) - 1);
+
+            value = 0;
+            address_space_read(as, l2t_addr +
+                                 ((devid % max_l2_entries) * GITS_DTE_SIZE),
+                                 MEMTXATTRS_UNSPECIFIED,
+                                 &value, sizeof(value));
+        }
+    } else {
+        /* Flat level table */
+        value = 0;
+        address_space_read(as, c->dt.base_addr + (devid * GITS_DTE_SIZE),
+                            MEMTXATTRS_UNSPECIFIED, &value,
+                            sizeof(value));
+    }
+
+    return value;
+}
+
 static MemTxResult process_sync(GICv3ITSState *s, uint32_t offset)
 {
     GICv3ITSClass *c = ARM_GICV3_ITS_GET_CLASS(s);
@@ -95,6 +247,192 @@ static MemTxResult process_sync(GICv3ITSState *s, uint32_t offset)
     return res;
 }
 
+static MemTxResult process_int(GICv3ITSState *s, uint64_t value,
+                                uint32_t offset, ItsCmdType cmd)
+{
+    GICv3ITSClass *c = ARM_GICV3_ITS_GET_CLASS(s);
+    AddressSpace *as = &s->gicv3->sysmem_as;
+    uint32_t devid, eventid;
+    MemTxResult res = MEMTX_OK;
+    bool dte_valid;
+    uint64_t dte = 0;
+    uint32_t max_eventid;
+    uint16_t icid = 0;
+    uint32_t pIntid = 0;
+    bool ite_valid = false;
+    uint64_t cte = 0;
+    bool cte_valid = false;
+    uint8_t buff[GITS_TYPER_ITT_ENTRY_SIZE];
+    uint64_t itt_addr;
+
+    if (cmd == NONE) {
+        devid = offset;
+    } else {
+        devid = (value >> DEVID_OFFSET) & DEVID_MASK;
+
+        offset += NUM_BYTES_IN_DW;
+        value = address_space_ldq_le(as, c->cq.base_addr + offset,
+                                 MEMTXATTRS_UNSPECIFIED, &res);
+    }
+
+    eventid = (value & EVENTID_MASK);
+
+    dte = get_dte(s, devid);
+    dte_valid = dte & VALID_MASK;
+
+    if (dte_valid) {
+        max_eventid = (1UL << (((dte >> 1U) & SIZE_MASK) + 1));
+
+        ite_valid = get_ite(s, eventid, dte, &icid, &pIntid);
+
+        if (ite_valid) {
+            cte_valid = get_cte(s, icid, &cte);
+        }
+    }
+
+    if ((devid > c->dt.max_devids) || !dte_valid || !ite_valid ||
+            !cte_valid || (eventid > max_eventid)) {
+        if ((s->typer >> GITS_TYPER_SEIS_OFFSET) &
+                         GITS_TYPER_SEIS_MASK) {
+            /*
+             * Generate System Error here if supported
+             * for each of the individual error cases
+             */
+        }
+        qemu_log_mask(LOG_GUEST_ERROR,
+            "%s: invalid interrupt translation table attributes "
+            "devid %d or eventid %d\n",
+            __func__, devid, eventid);
+        /*
+         * in this implementation,in case of error
+         * we ignore this command and move onto the next
+         * command in the queue
+         */
+    } else {
+        if ((s->typer >> GITS_TYPER_PTA_OFFSET) & GITS_TYPER_PTA_MASK) {
+            /*
+             * only bits[47:16] are considered instead of bits [51:16]
+             * since with a physical address the target address must be
+             * 64KB aligned
+             */
+
+            /*
+             * Current implementation only supports rdbase == procnum
+             * Hence rdbase physical address is ignored
+             */
+        } else {
+
+            if (cmd == DISCARD) {
+                /* remove mapping from interrupt translation table */
+                memset(buff, 0, sizeof(buff));
+
+                itt_addr = (dte >> 6ULL) & ITTADDR_MASK;
+                itt_addr <<= ITTADDR_OFFSET; /* 256 byte aligned */
+
+                address_space_write(as, itt_addr + (eventid * sizeof(buff)),
+                                    MEMTXATTRS_UNSPECIFIED, &buff,
+                                    sizeof(buff));
+            }
+        }
+    }
+
+    if (cmd != NONE) {
+        offset += NUM_BYTES_IN_DW;
+        offset += NUM_BYTES_IN_DW;
+    }
+
+    return res;
+}
+
+static MemTxResult process_mapti(GICv3ITSState *s, uint64_t value,
+                                    uint32_t offset, bool ignore_pInt)
+{
+    GICv3ITSClass *c = ARM_GICV3_ITS_GET_CLASS(s);
+    AddressSpace *as = &s->gicv3->sysmem_as;
+    uint32_t devid, eventid;
+    uint32_t pIntid = 0;
+    uint32_t max_eventid, max_Intid;
+    bool dte_valid;
+    MemTxResult res = MEMTX_OK;
+    uint16_t icid = 0;
+    uint64_t dte = 0;
+    uint64_t itt_addr;
+    uint8_t buff[GITS_TYPER_ITT_ENTRY_SIZE];
+    uint32_t int_spurious = INTID_SPURIOUS;
+
+    devid = (value >> DEVID_OFFSET) & DEVID_MASK;
+    offset += NUM_BYTES_IN_DW;
+    value = address_space_ldq_le(as, c->cq.base_addr + offset,
+                                 MEMTXATTRS_UNSPECIFIED, &res);
+
+    eventid = (value & EVENTID_MASK);
+
+    if (!ignore_pInt) {
+        pIntid = (value >> pINTID_OFFSET) & pINTID_MASK;
+    }
+
+    offset += NUM_BYTES_IN_DW;
+    value = address_space_ldq_le(as, c->cq.base_addr + offset,
+                                 MEMTXATTRS_UNSPECIFIED, &res);
+
+    icid = value & ICID_MASK;
+
+    dte = get_dte(s, devid);
+    dte_valid = dte & VALID_MASK;
+
+    max_eventid = (1UL << (((dte >> 1U) & SIZE_MASK) + 1));
+
+    if (!ignore_pInt) {
+        max_Intid = (1UL << (((s->typer >> GITS_TYPER_IDBITS_OFFSET) &
+                      GITS_TYPER_IDBITS_MASK) + 1));
+    }
+
+    if ((devid > c->dt.max_devids) || (icid > c->ct.max_collids) ||
+            !dte_valid || (eventid > max_eventid) ||
+            (!ignore_pInt && ((pIntid < GICV3_LPI_INTID_START) ||
+               (pIntid > max_Intid)))) {
+        if ((s->typer >> GITS_TYPER_SEIS_OFFSET) &
+                         GITS_TYPER_SEIS_MASK) {
+            /*
+             * Generate System Error here if supported
+             * for each of the individual error cases
+             */
+        }
+        qemu_log_mask(LOG_GUEST_ERROR,
+            "%s: invalid interrupt translation table attributes "
+            "devid %d or icid %d or eventid %d or pIntid %d\n",
+            __func__, devid, icid, eventid, pIntid);
+        /*
+         * in this implementation,in case of error
+         * we ignore this command and move onto the next
+         * command in the queue
+         */
+    } else {
+        /* add entry to interrupt translation table */
+        memset(buff, 0, sizeof(buff));
+        buff[0] = (dte_valid & VALID_MASK) | (GITS_TYPER_PHYSICAL << 1U);
+        if (ignore_pInt) {
+            memcpy(&buff[1], &eventid, 3);
+        } else {
+            memcpy(&buff[1], &pIntid, 3);
+        }
+        memcpy(&buff[4], &int_spurious, 3);
+        memcpy(&buff[7], &icid, sizeof(icid));
+
+        itt_addr = (dte >> 6ULL) & ITTADDR_MASK;
+        itt_addr <<= ITTADDR_OFFSET; /* 256 byte aligned */
+
+        address_space_write(as, itt_addr + (eventid * sizeof(buff)),
+                    MEMTXATTRS_UNSPECIFIED, &buff,
+                    sizeof(buff));
+    }
+
+    offset += NUM_BYTES_IN_DW;
+    offset += NUM_BYTES_IN_DW;
+
+    return res;
+}
+
 static void update_cte(GICv3ITSState *s, uint16_t icid, uint64_t cte)
 {
     GICv3ITSClass *c = ARM_GICV3_ITS_GET_CLASS(s);
@@ -276,7 +614,7 @@ static void update_dte(GICv3ITSState *s, uint32_t devid, uint64_t dte)
 }
 
 static MemTxResult process_mapd(GICv3ITSState *s, uint64_t value,
-                                 uint32_t offset)
+                                  uint32_t offset)
 {
     GICv3ITSClass *c = ARM_GICV3_ITS_GET_CLASS(s);
     AddressSpace *as = &s->gicv3->sysmem_as;
@@ -378,8 +716,10 @@ static MemTxResult process_cmdq(GICv3ITSState *s)
 
         switch (cmd) {
         case GITS_CMD_INT:
+            res = process_int(s, data, cq_offset, INT);
             break;
         case GITS_CMD_CLEAR:
+            res = process_int(s, data, cq_offset, CLEAR);
             break;
         case GITS_CMD_SYNC:
             res = process_sync(s, cq_offset);
@@ -391,10 +731,13 @@ static MemTxResult process_cmdq(GICv3ITSState *s)
             res = process_mapc(s, cq_offset);
             break;
         case GITS_CMD_MAPTI:
+            res = process_mapti(s, data, cq_offset, false);
             break;
         case GITS_CMD_MAPI:
+            res = process_mapti(s, data, cq_offset, true);
             break;
         case GITS_CMD_DISCARD:
+            res = process_int(s, data, cq_offset, DISCARD);
             break;
         default:
             break;
@@ -556,7 +899,20 @@ static MemTxResult its_trans_writew(GICv3ITSState *s, hwaddr offset,
                                uint64_t value, MemTxAttrs attrs)
 {
     MemTxResult result = MEMTX_OK;
+    uint32_t devid = 0;
 
+    switch (offset) {
+    case GITS_TRANSLATER:
+        if (s->ctlr & GITS_CTLR_ENABLED) {
+            s->translater = (value & 0x0000FFFFU);
+            devid = attrs.requester_id;
+            result = process_int(s, s->translater, devid, NONE);
+        }
+        break;
+    default:
+        result = MEMTX_ERROR;
+        break;
+    }
     return result;
 }
 
@@ -564,7 +920,20 @@ static MemTxResult its_trans_writel(GICv3ITSState *s, hwaddr offset,
                                uint64_t value, MemTxAttrs attrs)
 {
     MemTxResult result = MEMTX_OK;
+    uint32_t devid = 0;
 
+    switch (offset) {
+    case GITS_TRANSLATER:
+        if (s->ctlr & GITS_CTLR_ENABLED) {
+            s->translater = value;
+            devid = attrs.requester_id;
+            result = process_int(s, s->translater, devid, NONE);
+        }
+        break;
+    default:
+        result = MEMTX_ERROR;
+        break;
+    }
     return result;
 }
 
diff --git a/include/hw/intc/arm_gicv3_common.h b/include/hw/intc/arm_gicv3_common.h
index b0f2414fa3..3a710592a9 100644
--- a/include/hw/intc/arm_gicv3_common.h
+++ b/include/hw/intc/arm_gicv3_common.h
@@ -36,6 +36,8 @@
 #define GICV3_MAXIRQ 1020
 #define GICV3_MAXSPI (GICV3_MAXIRQ - GIC_INTERNAL)
 
+#define GICV3_LPI_INTID_START 8192
+
 #define GICV3_REDIST_SIZE 0x20000
 
 /* Number of SGI target-list bits */
-- 
2.27.0



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

* [PATCH v1 5/8] hw/intc: GICv3 ITS Feature enablement
  2021-03-23  4:12 [PATCH v1 0/8] GICv3 LPI and ITS feature implementation Shashi Mallela
                   ` (3 preceding siblings ...)
  2021-03-23  4:12 ` [PATCH v1 4/8] hw/intc: GICv3 ITS Command processing Shashi Mallela
@ 2021-03-23  4:12 ` Shashi Mallela
  2021-03-23  4:12 ` [PATCH v1 6/8] hw/intc: GICv3 redistributor ITS processing Shashi Mallela
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 16+ messages in thread
From: Shashi Mallela @ 2021-03-23  4:12 UTC (permalink / raw)
  To: peter.maydell, leif, rad; +Cc: qemu-arm, qemu-devel

Added properties to enable ITS feature and define qemu system
address space memory in gicv3 common,setup distributor and
redistributor registers to indicate LPI support.

Signed-off-by: Shashi Mallela <shashi.mallela@linaro.org>
---
 hw/intc/arm_gicv3_common.c         | 16 +++++++++++
 hw/intc/arm_gicv3_dist.c           | 22 +++++++++++++--
 hw/intc/arm_gicv3_redist.c         | 29 ++++++++++++++++++--
 include/hw/intc/arm_gicv3_common.h |  8 ++++++
 4 files changed, 70 insertions(+), 5 deletions(-)

diff --git a/hw/intc/arm_gicv3_common.c b/hw/intc/arm_gicv3_common.c
index 58ef65f589..3bfc52f7fa 100644
--- a/hw/intc/arm_gicv3_common.c
+++ b/hw/intc/arm_gicv3_common.c
@@ -156,6 +156,7 @@ static const VMStateDescription vmstate_gicv3_cpu = {
         VMSTATE_UINT32(gicr_waker, GICv3CPUState),
         VMSTATE_UINT64(gicr_propbaser, GICv3CPUState),
         VMSTATE_UINT64(gicr_pendbaser, GICv3CPUState),
+        VMSTATE_BOOL(lpi_outofrange, GICv3CPUState),
         VMSTATE_UINT32(gicr_igroupr0, GICv3CPUState),
         VMSTATE_UINT32(gicr_ienabler0, GICv3CPUState),
         VMSTATE_UINT32(gicr_ipendr0, GICv3CPUState),
@@ -227,6 +228,7 @@ static const VMStateDescription vmstate_gicv3 = {
     .priority = MIG_PRI_GICV3,
     .fields = (VMStateField[]) {
         VMSTATE_UINT32(gicd_ctlr, GICv3State),
+        VMSTATE_UINT32(gicd_typer, GICv3State),
         VMSTATE_UINT32_ARRAY(gicd_statusr, GICv3State, 2),
         VMSTATE_UINT32_ARRAY(group, GICv3State, GICV3_BMP_SIZE),
         VMSTATE_UINT32_ARRAY(grpmod, GICv3State, GICV3_BMP_SIZE),
@@ -381,6 +383,16 @@ static void arm_gicv3_common_realize(DeviceState *dev, Error **errp)
             (1 << 24) |
             (i << 8) |
             (last << 4);
+
+        if (s->lpi_enable) {
+            s->cpu[i].gicr_typer |= GICR_TYPER_PLPIS;
+
+            if (!s->sysmem) {
+                error_setg(errp,
+                    "Redist-ITS: Guest 'sysmem' reference link not set");
+                return;
+            }
+        }
     }
 }
 
@@ -406,6 +418,7 @@ static void arm_gicv3_common_reset(DeviceState *dev)
         cs->gicr_waker = GICR_WAKER_ProcessorSleep | GICR_WAKER_ChildrenAsleep;
         cs->gicr_propbaser = 0;
         cs->gicr_pendbaser = 0;
+        cs->lpi_outofrange = false;
         /* If we're resetting a TZ-aware GIC as if secure firmware
          * had set it up ready to start a kernel in non-secure, we
          * need to set interrupts to group 1 so the kernel can use them.
@@ -494,9 +507,12 @@ static Property arm_gicv3_common_properties[] = {
     DEFINE_PROP_UINT32("num-cpu", GICv3State, num_cpu, 1),
     DEFINE_PROP_UINT32("num-irq", GICv3State, num_irq, 32),
     DEFINE_PROP_UINT32("revision", GICv3State, revision, 3),
+    DEFINE_PROP_BOOL("has-lpi", GICv3State, lpi_enable, 0),
     DEFINE_PROP_BOOL("has-security-extensions", GICv3State, security_extn, 0),
     DEFINE_PROP_ARRAY("redist-region-count", GICv3State, nb_redist_regions,
                       redist_region_count, qdev_prop_uint32, uint32_t),
+    DEFINE_PROP_LINK("sysmem", GICv3State, sysmem, TYPE_MEMORY_REGION,
+                     MemoryRegion *),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/hw/intc/arm_gicv3_dist.c b/hw/intc/arm_gicv3_dist.c
index b65f56f903..96a317a8ef 100644
--- a/hw/intc/arm_gicv3_dist.c
+++ b/hw/intc/arm_gicv3_dist.c
@@ -366,12 +366,15 @@ static MemTxResult gicd_readl(GICv3State *s, hwaddr offset,
         return MEMTX_OK;
     case GICD_TYPER:
     {
+        bool lpi_supported = false;
         /* For this implementation:
          * No1N == 1 (1-of-N SPI interrupts not supported)
          * A3V == 1 (non-zero values of Affinity level 3 supported)
          * IDbits == 0xf (we support 16-bit interrupt identifiers)
          * DVIS == 0 (Direct virtual LPI injection not supported)
-         * LPIS == 0 (LPIs not supported)
+         * LPIS == 1 (LPIs are supported if affinity routing is enabled)
+         * num_LPIs == 0b00000 (bits [15:11],Number of LPIs as indicated
+         *                      by GICD_TYPER.IDbits)
          * MBIS == 0 (message-based SPIs not supported)
          * SecurityExtn == 1 if security extns supported
          * CPUNumber == 0 since for us ARE is always 1
@@ -385,8 +388,23 @@ static MemTxResult gicd_readl(GICv3State *s, hwaddr offset,
          */
         bool sec_extn = !(s->gicd_ctlr & GICD_CTLR_DS);
 
+        /*
+         * With securityextn on,LPIs are supported when affinity routing
+         * is enabled for non-secure state and if off LPIs are supported
+         * when affinity routing is enabled.
+         */
+        if (s->lpi_enable) {
+            if (sec_extn) {
+                lpi_supported = (s->gicd_ctlr & GICD_CTLR_ARE_NS);
+            } else {
+                lpi_supported = (s->gicd_ctlr & GICD_CTLR_ARE);
+            }
+        }
+
         *data = (1 << 25) | (1 << 24) | (sec_extn << 10) |
-            (0xf << 19) | itlinesnumber;
+            (lpi_supported << GICD_TYPER_LPIS_OFFSET) | (GICD_TYPER_IDBITS <<
+            GICD_TYPER_IDBITS_OFFSET) | itlinesnumber;
+        s->gicd_typer = *data;
         return MEMTX_OK;
     }
     case GICD_IIDR:
diff --git a/hw/intc/arm_gicv3_redist.c b/hw/intc/arm_gicv3_redist.c
index 8645220d61..f4d14811ec 100644
--- a/hw/intc/arm_gicv3_redist.c
+++ b/hw/intc/arm_gicv3_redist.c
@@ -248,10 +248,16 @@ static MemTxResult gicr_writel(GICv3CPUState *cs, hwaddr offset,
     case GICR_CTLR:
         /* For our implementation, GICR_TYPER.DPGS is 0 and so all
          * the DPG bits are RAZ/WI. We don't do anything asynchronously,
-         * so UWP and RWP are RAZ/WI. And GICR_TYPER.LPIS is 0 (we don't
-         * implement LPIs) so Enable_LPIs is RES0. So there are no writable
-         * bits for us.
+         * so UWP and RWP are RAZ/WI. GICR_TYPER.LPIS is 1 (we
+         * implement LPIs) so Enable_LPIs is programmable.
          */
+        if (cs->gicr_typer & GICR_TYPER_PLPIS) {
+            if (value & GICR_CTLR_ENABLE_LPIS) {
+                cs->gicr_ctlr |= GICR_CTLR_ENABLE_LPIS;
+            } else {
+                cs->gicr_ctlr &= ~GICR_CTLR_ENABLE_LPIS;
+            }
+        }
         return MEMTX_OK;
     case GICR_STATUSR:
         /* RAZ/WI for our implementation */
@@ -275,6 +281,14 @@ static MemTxResult gicr_writel(GICv3CPUState *cs, hwaddr offset,
         cs->gicr_waker = value;
         return MEMTX_OK;
     case GICR_PROPBASER:
+        if ((value & GICR_PROPBASER_IDBITS_MASK) <
+            GICR_PROPBASER_IDBITS_THRESHOLD) {
+            cs->lpi_outofrange = true;
+        }
+        if ((value & GICR_PROPBASER_IDBITS_MASK) > GICD_TYPER_IDBITS) {
+            value &= ~GICR_PROPBASER_IDBITS_MASK;
+            value |= GICD_TYPER_IDBITS;
+        }
         cs->gicr_propbaser = deposit64(cs->gicr_propbaser, 0, 32, value);
         return MEMTX_OK;
     case GICR_PROPBASER + 4:
@@ -397,6 +411,15 @@ static MemTxResult gicr_writell(GICv3CPUState *cs, hwaddr offset,
 {
     switch (offset) {
     case GICR_PROPBASER:
+        if ((value & GICR_PROPBASER_IDBITS_MASK) <
+            GICR_PROPBASER_IDBITS_THRESHOLD) {
+            cs->lpi_outofrange = true;
+        }
+        if ((value & GICR_PROPBASER_IDBITS_MASK) >
+            GICD_TYPER_IDBITS) {
+            value &= ~GICR_PROPBASER_IDBITS_MASK;
+            value |= GICD_TYPER_IDBITS;
+        }
         cs->gicr_propbaser = value;
         return MEMTX_OK;
     case GICR_PENDBASER:
diff --git a/include/hw/intc/arm_gicv3_common.h b/include/hw/intc/arm_gicv3_common.h
index 3a710592a9..db3989484d 100644
--- a/include/hw/intc/arm_gicv3_common.h
+++ b/include/hw/intc/arm_gicv3_common.h
@@ -175,6 +175,13 @@ struct GICv3CPUState {
     uint32_t gicr_nsacr;
     uint8_t gicr_ipriorityr[GIC_INTERNAL];
 
+    /*
+     * flag to indicate LPIs are out of range
+     * since IDbits from GICR_PROPBASER is less
+     * than 0b1101
+     */
+    bool lpi_outofrange;
+
     /* CPU interface */
     uint64_t icc_sre_el1;
     uint64_t icc_ctlr_el1[2];
@@ -221,6 +228,7 @@ struct GICv3State {
     uint32_t num_cpu;
     uint32_t num_irq;
     uint32_t revision;
+    bool lpi_enable;
     bool security_extn;
     bool irq_reset_nonsecure;
     bool gicd_no_migration_shift_bug;
-- 
2.27.0



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

* [PATCH v1 6/8] hw/intc: GICv3 redistributor ITS processing
  2021-03-23  4:12 [PATCH v1 0/8] GICv3 LPI and ITS feature implementation Shashi Mallela
                   ` (4 preceding siblings ...)
  2021-03-23  4:12 ` [PATCH v1 5/8] hw/intc: GICv3 ITS Feature enablement Shashi Mallela
@ 2021-03-23  4:12 ` Shashi Mallela
  2021-03-23  4:12 ` [PATCH v1 7/8] hw/arm/sbsa-ref: add ITS support in SBSA GIC Shashi Mallela
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 16+ messages in thread
From: Shashi Mallela @ 2021-03-23  4:12 UTC (permalink / raw)
  To: peter.maydell, leif, rad; +Cc: qemu-arm, qemu-devel

Implemented lpi processing at redistributor to get lpi config info
from lpi configuration table,determine priority,set pending state in
lpi pending table and forward the lpi to cpuif.Added logic to invoke
redistributor lpi processing with translated LPI which set/clear LPI
from ITS device as part of ITS INT,CLEAR,DISCARD command and
GITS_TRANSLATER processing.

Signed-off-by: Shashi Mallela <shashi.mallela@linaro.org>
---
 hw/intc/arm_gicv3.c        |   6 +
 hw/intc/arm_gicv3_cpuif.c  |  15 ++-
 hw/intc/arm_gicv3_its.c    |   9 +-
 hw/intc/arm_gicv3_redist.c | 126 ++++++++++++++++++++
 hw/intc/gicv3_internal.h   |   3 +
 5 files changed, 154 insertions(+), 5 deletions(-)

diff --git a/hw/intc/arm_gicv3.c b/hw/intc/arm_gicv3.c
index 66eaa97198..618fa1af95 100644
--- a/hw/intc/arm_gicv3.c
+++ b/hw/intc/arm_gicv3.c
@@ -166,6 +166,12 @@ static void gicv3_redist_update_noirqset(GICv3CPUState *cs)
         cs->hppi.grp = gicv3_irq_group(cs->gic, cs, cs->hppi.irq);
     }
 
+    if (cs->gic->lpi_enable) {
+        if (gicv3_redist_update_lpi(cs)) {
+            seenbetter = true;
+        }
+    }
+
     /* If the best interrupt we just found would preempt whatever
      * was the previous best interrupt before this update, then
      * we know it's definitely the best one now.
diff --git a/hw/intc/arm_gicv3_cpuif.c b/hw/intc/arm_gicv3_cpuif.c
index 43ef1d7a84..c225b80f66 100644
--- a/hw/intc/arm_gicv3_cpuif.c
+++ b/hw/intc/arm_gicv3_cpuif.c
@@ -899,9 +899,14 @@ static void icc_activate_irq(GICv3CPUState *cs, int irq)
         cs->gicr_ipendr0 = deposit32(cs->gicr_ipendr0, irq, 1, 0);
         gicv3_redist_update(cs);
     } else {
-        gicv3_gicd_active_set(cs->gic, irq);
-        gicv3_gicd_pending_clear(cs->gic, irq);
-        gicv3_update(cs->gic, irq, 1);
+        if (irq >= GICV3_LPI_INTID_START) {
+            gicv3_redist_lpi_pending(cs, irq, 0);
+            gicv3_redist_update(cs);
+        } else {
+            gicv3_gicd_active_set(cs->gic, irq);
+            gicv3_gicd_pending_clear(cs->gic, irq);
+            gicv3_update(cs->gic, irq, 1);
+        }
     }
 }
 
@@ -1337,7 +1342,9 @@ static void icc_eoir_write(CPUARMState *env, const ARMCPRegInfo *ri,
          * valid interrupt value read from the Interrupt Acknowledge
          * register" and so this is UNPREDICTABLE. We choose to ignore it.
          */
-        return;
+        if (!(cs->gic->lpi_enable && (irq >= GICV3_LPI_INTID_START))) {
+            return;
+        }
     }
 
     if (icc_highest_active_group(cs) != grp) {
diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c
index de2d179b5e..bb46af92a3 100644
--- a/hw/intc/arm_gicv3_its.c
+++ b/hw/intc/arm_gicv3_its.c
@@ -262,6 +262,7 @@ static MemTxResult process_int(GICv3ITSState *s, uint64_t value,
     bool ite_valid = false;
     uint64_t cte = 0;
     bool cte_valid = false;
+    uint64_t rdbase;
     uint8_t buff[GITS_TYPER_ITT_ENTRY_SIZE];
     uint64_t itt_addr;
 
@@ -315,12 +316,18 @@ static MemTxResult process_int(GICv3ITSState *s, uint64_t value,
              * since with a physical address the target address must be
              * 64KB aligned
              */
-
+            rdbase = (cte >> 1U) & RDBASE_MASK;
             /*
              * Current implementation only supports rdbase == procnum
              * Hence rdbase physical address is ignored
              */
         } else {
+            rdbase = (cte >> 1U) & RDBASE_PROCNUM_MASK;
+            if ((cmd == CLEAR) || (cmd == DISCARD)) {
+                gicv3_redist_process_lpi(&s->gicv3->cpu[rdbase], pIntid, 0);
+            } else {
+                gicv3_redist_process_lpi(&s->gicv3->cpu[rdbase], pIntid, 1);
+            }
 
             if (cmd == DISCARD) {
                 /* remove mapping from interrupt translation table */
diff --git a/hw/intc/arm_gicv3_redist.c b/hw/intc/arm_gicv3_redist.c
index f4d14811ec..dc47ed42d2 100644
--- a/hw/intc/arm_gicv3_redist.c
+++ b/hw/intc/arm_gicv3_redist.c
@@ -549,6 +549,132 @@ MemTxResult gicv3_redist_write(void *opaque, hwaddr offset, uint64_t data,
     return r;
 }
 
+bool gicv3_redist_update_lpi(GICv3CPUState *cs)
+{
+    AddressSpace *as = &cs->gic->sysmem_as;
+    uint64_t lpict_baddr, lpipt_baddr;
+    uint32_t pendt_size = 0;
+    uint8_t lpite;
+    uint8_t prio, pend;
+    int i;
+    bool seenbetter = false;
+
+    if ((!cs->gicr_ctlr & GICR_CTLR_ENABLE_LPIS) || !cs->gicr_propbaser ||
+        !cs->gicr_pendbaser || cs->lpi_outofrange) {
+        return seenbetter;
+    }
+
+    lpict_baddr = (cs->gicr_propbaser >> GICR_PROPBASER_ADDR_OFFSET) &
+                       GICR_PROPBASER_ADDR_MASK;
+    lpict_baddr <<= GICR_PROPBASER_ADDR_OFFSET;
+
+    lpipt_baddr = (cs->gicr_pendbaser >> GICR_PENDBASER_ADDR_OFFSET) &
+                       GICR_PENDBASER_ADDR_MASK;
+    lpipt_baddr <<= GICR_PENDBASER_ADDR_OFFSET;
+
+    /* Determine the highest priority pending interrupt among LPIs */
+    pendt_size = (1UL << ((cs->gicr_propbaser &
+                            GICR_PROPBASER_IDBITS_MASK) - 1));
+
+    for (i = 0; i < pendt_size; i++) {
+        address_space_read(as, lpipt_baddr +
+                (((GICV3_LPI_INTID_START + i) / 8) * sizeof(pend)),
+                MEMTXATTRS_UNSPECIFIED, &pend, sizeof(pend));
+
+        if ((1 << ((GICV3_LPI_INTID_START + i) % 8)) & pend) {
+            address_space_read(as, lpict_baddr + (i * sizeof(lpite)),
+                      MEMTXATTRS_UNSPECIFIED, &lpite, sizeof(lpite));
+
+            prio = ((lpite >> LPI_CTE_PRIORITY_OFFSET) &
+                     LPI_CTE_PRIORITY_MASK);
+            prio &= LPI_PRIORITY_MASK;
+
+            if (prio < cs->hppi.prio) {
+                cs->hppi.irq = GICV3_LPI_INTID_START + i;
+                cs->hppi.prio = prio;
+                /* LPIs are always non-secure Grp1 interrupts */
+                cs->hppi.grp = GICV3_G1NS;
+                seenbetter = true;
+            }
+        }
+    }
+    return seenbetter;
+}
+
+void gicv3_redist_lpi_pending(GICv3CPUState *cs, int irq, int level)
+{
+    AddressSpace *as = &cs->gic->sysmem_as;
+    uint64_t lpipt_baddr;
+    bool ispend = false;
+    uint8_t pend;
+
+    /*
+     * get the bit value corresponding to this irq in the
+     * lpi pending table
+     */
+    lpipt_baddr = (cs->gicr_pendbaser >> GICR_PENDBASER_ADDR_OFFSET) &
+                      GICR_PENDBASER_ADDR_MASK;
+    lpipt_baddr <<= GICR_PENDBASER_ADDR_OFFSET;
+
+    address_space_read(as, lpipt_baddr + ((irq / 8) * sizeof(pend)),
+                         MEMTXATTRS_UNSPECIFIED, &pend, sizeof(pend));
+    ispend = ((pend >> (irq % 8)) & 0x1);
+
+    if (ispend) {
+        if (!level) {
+            /*
+             * clear the pending bit and update the lpi pending table
+             */
+            pend &= ~(1 << (irq % 8));
+
+            address_space_write(as, lpipt_baddr + ((irq / 8) * sizeof(pend)),
+                                 MEMTXATTRS_UNSPECIFIED, &pend, sizeof(pend));
+        }
+    } else {
+        if (level) {
+            /*
+             * if pending bit is not already set for this irq,turn-on the
+             * pending bit and update the lpi pending table
+             */
+            pend |= (1 << (irq % 8));
+
+            address_space_write(as, lpipt_baddr + ((irq / 8) * sizeof(pend)),
+                                 MEMTXATTRS_UNSPECIFIED, &pend, sizeof(pend));
+        }
+    }
+}
+
+void gicv3_redist_process_lpi(GICv3CPUState *cs, int irq, int level)
+{
+    AddressSpace *as = &cs->gic->sysmem_as;
+    uint64_t lpict_baddr;
+    uint8_t lpite;
+
+    if ((!cs->gicr_ctlr & GICR_CTLR_ENABLE_LPIS) || !cs->gicr_propbaser ||
+         !cs->gicr_pendbaser || cs->lpi_outofrange) {
+        return;
+    }
+
+    lpict_baddr = (cs->gicr_propbaser >> GICR_PROPBASER_ADDR_OFFSET) &
+                      GICR_PROPBASER_ADDR_MASK;
+    lpict_baddr <<= GICR_PROPBASER_ADDR_OFFSET;
+
+    /* get the lpi config table entry corresponding to this irq */
+    address_space_read(as, lpict_baddr + ((irq - GICV3_LPI_INTID_START) *
+                        sizeof(lpite)), MEMTXATTRS_UNSPECIFIED,
+                        &lpite, sizeof(lpite));
+
+    /* check if this irq is enabled before proceeding further */
+    if (!(lpite & LPI_CTE_ENABLED)) {
+        return;
+    }
+
+    /* set/clear the pending bit for this irq */
+    gicv3_redist_lpi_pending(cs, irq, level);
+
+    gicv3_redist_update(cs);
+}
+
 void gicv3_redist_set_irq(GICv3CPUState *cs, int irq, int level)
 {
     /* Update redistributor state for a change in an external PPI input line */
diff --git a/hw/intc/gicv3_internal.h b/hw/intc/gicv3_internal.h
index 7c6bc33e97..cbb7810ec8 100644
--- a/hw/intc/gicv3_internal.h
+++ b/hw/intc/gicv3_internal.h
@@ -465,6 +465,9 @@ MemTxResult gicv3_redist_write(void *opaque, hwaddr offset, uint64_t data,
                                unsigned size, MemTxAttrs attrs);
 void gicv3_dist_set_irq(GICv3State *s, int irq, int level);
 void gicv3_redist_set_irq(GICv3CPUState *cs, int irq, int level);
+void gicv3_redist_process_lpi(GICv3CPUState *cs, int irq, int level);
+void gicv3_redist_lpi_pending(GICv3CPUState *cs, int irq, int level);
+bool gicv3_redist_update_lpi(GICv3CPUState *cs);
 void gicv3_redist_send_sgi(GICv3CPUState *cs, int grp, int irq, bool ns);
 void gicv3_init_cpuif(GICv3State *s);
 
-- 
2.27.0



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

* [PATCH v1 7/8] hw/arm/sbsa-ref: add ITS support in SBSA GIC
  2021-03-23  4:12 [PATCH v1 0/8] GICv3 LPI and ITS feature implementation Shashi Mallela
                   ` (5 preceding siblings ...)
  2021-03-23  4:12 ` [PATCH v1 6/8] hw/intc: GICv3 redistributor ITS processing Shashi Mallela
@ 2021-03-23  4:12 ` Shashi Mallela
  2021-03-23  4:12 ` [PATCH v1 8/8] hw/arm/virt: add ITS support in virt GIC Shashi Mallela
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 16+ messages in thread
From: Shashi Mallela @ 2021-03-23  4:12 UTC (permalink / raw)
  To: peter.maydell, leif, rad; +Cc: qemu-arm, qemu-devel

Included creation of ITS as part of SBSA platform GIC
initialization.

Signed-off-by: Shashi Mallela <shashi.mallela@linaro.org>
---
 hw/arm/sbsa-ref.c | 26 +++++++++++++++++---
 1 file changed, 23 insertions(+), 3 deletions(-)

diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
index 88dfb2284c..d05cbcae48 100644
--- a/hw/arm/sbsa-ref.c
+++ b/hw/arm/sbsa-ref.c
@@ -35,7 +35,7 @@
 #include "hw/boards.h"
 #include "hw/ide/internal.h"
 #include "hw/ide/ahci_internal.h"
-#include "hw/intc/arm_gicv3_common.h"
+#include "hw/intc/arm_gicv3_its_common.h"
 #include "hw/loader.h"
 #include "hw/pci-host/gpex.h"
 #include "hw/qdev-properties.h"
@@ -65,6 +65,7 @@ enum {
     SBSA_CPUPERIPHS,
     SBSA_GIC_DIST,
     SBSA_GIC_REDIST,
+    SBSA_GIC_ITS,
     SBSA_SECURE_EC,
     SBSA_GWDT,
     SBSA_GWDT_REFRESH,
@@ -108,6 +109,7 @@ static const MemMapEntry sbsa_ref_memmap[] = {
     [SBSA_CPUPERIPHS] =         { 0x40000000, 0x00040000 },
     [SBSA_GIC_DIST] =           { 0x40060000, 0x00010000 },
     [SBSA_GIC_REDIST] =         { 0x40080000, 0x04000000 },
+    [SBSA_GIC_ITS] =            { 0x44090000, 0x00020000 },
     [SBSA_SECURE_EC] =          { 0x50000000, 0x00001000 },
     [SBSA_GWDT_REFRESH] =       { 0x50010000, 0x00001000 },
     [SBSA_GWDT_CONTROL] =       { 0x50011000, 0x00001000 },
@@ -378,7 +380,20 @@ static void create_secure_ram(SBSAMachineState *sms,
     memory_region_add_subregion(secure_sysmem, base, secram);
 }
 
-static void create_gic(SBSAMachineState *sms)
+static void create_its(SBSAMachineState *sms)
+{
+    DeviceState *dev;
+
+    dev = qdev_new(TYPE_ARM_GICV3_ITS);
+    SysBusDevice *s = SYS_BUS_DEVICE(dev);
+
+    object_property_set_link(OBJECT(dev), "parent-gicv3", OBJECT(sms->gic),
+                             &error_abort);
+    sysbus_realize_and_unref(s, &error_fatal);
+    sysbus_mmio_map(s, 0, sbsa_ref_memmap[SBSA_GIC_ITS].base);
+}
+
+static void create_gic(SBSAMachineState *sms, MemoryRegion *mem)
 {
     unsigned int smp_cpus = MACHINE(sms)->smp.cpus;
     SysBusDevice *gicbusdev;
@@ -405,6 +420,10 @@ static void create_gic(SBSAMachineState *sms)
     qdev_prop_set_uint32(sms->gic, "len-redist-region-count", 1);
     qdev_prop_set_uint32(sms->gic, "redist-region-count[0]", redist0_count);
 
+    object_property_set_link(OBJECT(sms->gic), "sysmem", OBJECT(mem),
+                                 &error_fatal);
+    qdev_prop_set_bit(sms->gic, "has-lpi", true);
+
     gicbusdev = SYS_BUS_DEVICE(sms->gic);
     sysbus_realize_and_unref(gicbusdev, &error_fatal);
     sysbus_mmio_map(gicbusdev, 0, sbsa_ref_memmap[SBSA_GIC_DIST].base);
@@ -451,6 +470,7 @@ static void create_gic(SBSAMachineState *sms)
         sysbus_connect_irq(gicbusdev, i + 3 * smp_cpus,
                            qdev_get_gpio_in(cpudev, ARM_CPU_VFIQ));
     }
+    create_its(sms);
 }
 
 static void create_uart(const SBSAMachineState *sms, int uart,
@@ -763,7 +783,7 @@ static void sbsa_ref_init(MachineState *machine)
 
     create_secure_ram(sms, secure_sysmem);
 
-    create_gic(sms);
+    create_gic(sms, sysmem);
 
     create_uart(sms, SBSA_UART, sysmem, serial_hd(0));
     create_uart(sms, SBSA_SECURE_UART, secure_sysmem, serial_hd(1));
-- 
2.27.0



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

* [PATCH v1 8/8] hw/arm/virt: add ITS support in virt GIC
  2021-03-23  4:12 [PATCH v1 0/8] GICv3 LPI and ITS feature implementation Shashi Mallela
                   ` (6 preceding siblings ...)
  2021-03-23  4:12 ` [PATCH v1 7/8] hw/arm/sbsa-ref: add ITS support in SBSA GIC Shashi Mallela
@ 2021-03-23  4:12 ` Shashi Mallela
  2021-03-25 17:59 ` [PATCH v1 0/8] GICv3 LPI and ITS feature implementation Alex Bennée
  2021-03-25 19:44 ` Alex Bennée
  9 siblings, 0 replies; 16+ messages in thread
From: Shashi Mallela @ 2021-03-23  4:12 UTC (permalink / raw)
  To: peter.maydell, leif, rad; +Cc: qemu-arm, qemu-devel

Included creation of ITS as part of virt platform GIC
initialization.This Emulated ITS model now co-exists with kvm
ITS and is enabled in absence of kvm irq kernel support in a
platform.

Signed-off-by: Shashi Mallela <shashi.mallela@linaro.org>
---
 hw/arm/virt.c        | 10 ++++++++--
 target/arm/kvm_arm.h |  4 ++--
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index aa2bbd14e0..77cf2db90f 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -622,7 +622,7 @@ static void create_v2m(VirtMachineState *vms)
     vms->msi_controller = VIRT_MSI_CTRL_GICV2M;
 }
 
-static void create_gic(VirtMachineState *vms)
+static void create_gic(VirtMachineState *vms, MemoryRegion *mem)
 {
     MachineState *ms = MACHINE(vms);
     /* We create a standalone GIC */
@@ -656,6 +656,12 @@ static void create_gic(VirtMachineState *vms)
                              nb_redist_regions);
         qdev_prop_set_uint32(vms->gic, "redist-region-count[0]", redist0_count);
 
+        if (!kvm_irqchip_in_kernel()) {
+            object_property_set_link(OBJECT(vms->gic), "sysmem", OBJECT(mem),
+                                     &error_fatal);
+            qdev_prop_set_bit(vms->gic, "has-lpi", true);
+        }
+
         if (nb_redist_regions == 2) {
             uint32_t redist1_capacity =
                     vms->memmap[VIRT_HIGH_GIC_REDIST2].size / GICV3_REDIST_SIZE;
@@ -2039,7 +2045,7 @@ static void machvirt_init(MachineState *machine)
 
     virt_flash_fdt(vms, sysmem, secure_sysmem ?: sysmem);
 
-    create_gic(vms);
+    create_gic(vms, sysmem);
 
     virt_cpu_post_init(vms, sysmem);
 
diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h
index 34f8daa377..0613454975 100644
--- a/target/arm/kvm_arm.h
+++ b/target/arm/kvm_arm.h
@@ -525,8 +525,8 @@ static inline const char *its_class_name(void)
         /* KVM implementation requires this capability */
         return kvm_direct_msi_enabled() ? "arm-its-kvm" : NULL;
     } else {
-        /* Software emulation is not implemented yet */
-        return NULL;
+        /* Software emulation based model */
+        return "arm-gicv3-its";
     }
 }
 
-- 
2.27.0



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

* Re: [PATCH v1 1/8] hw/intc: GICv3 ITS initial framework
  2021-03-23  4:12 ` [PATCH v1 1/8] hw/intc: GICv3 ITS initial framework Shashi Mallela
@ 2021-03-25 16:43   ` Alex Bennée
  2021-03-25 17:18   ` Alex Bennée
  1 sibling, 0 replies; 16+ messages in thread
From: Alex Bennée @ 2021-03-25 16:43 UTC (permalink / raw)
  To: Shashi Mallela; +Cc: peter.maydell, leif, qemu-devel, qemu-arm, rad


Shashi Mallela <shashi.mallela@linaro.org> writes:

> Added register definitions relevant to ITS,implemented overall
> ITS device framework with stubs for ITS control and translater
> regions read/write,extended ITS common to handle mmio init between
> existing kvm device and newer qemu device.
>
> Signed-off-by: Shashi Mallela <shashi.mallela@linaro.org>
> ---
>  hw/intc/arm_gicv3_its.c                | 323 ++++++++++++++++++++
>  hw/intc/arm_gicv3_its_common.c         |  17 +-
>  hw/intc/arm_gicv3_its_kvm.c            |   2 +-
>  hw/intc/gicv3_internal.h               | 173 ++++++++++-
>  hw/intc/meson.build                    |   1 +
>  include/hw/intc/arm_gicv3_its_common.h |  12 +-
>  6 files changed, 520 insertions(+), 8 deletions(-)
>
> diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c
> new file mode 100644
> index 0000000000..34e49b4d63
> --- /dev/null
> +++ b/hw/intc/arm_gicv3_its.c
> @@ -0,0 +1,323 @@
> +/*
> + * ITS emulation for a GICv3-based system
> + *
> + * Copyright Linaro.org 2021
> + *
> + * Authors:
> + *  Shashi Mallela <shashi.mallela@linaro.org>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or (at your
> + * option) any later version.  See the COPYING file in the top-level directory.
> + *
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu/log.h"
> +#include "hw/qdev-properties.h"
> +#include "hw/intc/arm_gicv3_its_common.h"
> +#include "gicv3_internal.h"
> +#include "qom/object.h"
> +
> +typedef struct GICv3ITSClass GICv3ITSClass;
> +/* This is reusing the GICv3ITSState typedef from ARM_GICV3_ITS_COMMON */
> +DECLARE_OBJ_CHECKERS(GICv3ITSState, GICv3ITSClass,
> +                     ARM_GICV3_ITS, TYPE_ARM_GICV3_ITS)
> +
> +struct GICv3ITSClass {
> +    GICv3ITSCommonClass parent_class;
> +    void (*parent_reset)(DeviceState *dev);
> +};
> +
> +static MemTxResult its_trans_writew(GICv3ITSState *s, hwaddr offset,
> +                               uint64_t value, MemTxAttrs attrs)
> +{
> +    MemTxResult result = MEMTX_OK;
> +
> +    return result;
> +}
> +
> +static MemTxResult its_trans_writel(GICv3ITSState *s, hwaddr offset,
> +                               uint64_t value, MemTxAttrs attrs)
> +{
> +    MemTxResult result = MEMTX_OK;
> +
> +    return result;
> +}
> +
> +static MemTxResult gicv3_its_trans_write(void *opaque, hwaddr offset,
> +                               uint64_t data, unsigned size, MemTxAttrs attrs)
> +{
> +    GICv3ITSState *s = (GICv3ITSState *)opaque;
> +    MemTxResult result;
> +
> +    switch (size) {
> +    case 2:
> +        result = its_trans_writew(s, offset, data, attrs);
> +        break;
> +    case 4:
> +        result = its_trans_writel(s, offset, data, attrs);
> +        break;
> +    default:
> +        result = MEMTX_ERROR;
> +        break;
> +    }
> +
> +    if (result == MEMTX_ERROR) {
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "%s: invalid guest write at offset " TARGET_FMT_plx
> +                      "size %u\n", __func__, offset, size);
> +        /*
> +         * The spec requires that reserved registers are RAZ/WI;
> +         * so use MEMTX_ERROR returns from leaf functions as a way to
> +         * trigger the guest-error logging but don't return it to
> +         * the caller, or we'll cause a spurious guest data abort.
> +         */
> +        result = MEMTX_OK;
> +    }
> +    return result;
> +}
> +
> +static MemTxResult gicv3_its_trans_read(void *opaque, hwaddr offset,
> +                              uint64_t *data, unsigned size, MemTxAttrs attrs)
> +{
> +    qemu_log_mask(LOG_GUEST_ERROR,
> +        "%s: Invalid read from transaction register area at offset "
> +        TARGET_FMT_plx "\n", __func__, offset);
> +    return MEMTX_ERROR;
> +}
> +
> +static MemTxResult its_writeb(GICv3ITSState *s, hwaddr offset,
> +                               uint64_t value, MemTxAttrs attrs)
> +{
> +    qemu_log_mask(LOG_GUEST_ERROR,
> +                "%s: unsupported byte write to register at offset "
> +                TARGET_FMT_plx "\n", __func__, offset);
> +    return MEMTX_ERROR;

unsupported should be LOG_UNIMP which makes it easier to see where QEMU
is missing something vs the guest doing something nuts.

> +}
> +
> +static MemTxResult its_readb(GICv3ITSState *s, hwaddr offset,
> +                               uint64_t *data, MemTxAttrs attrs)
> +{
> +    qemu_log_mask(LOG_GUEST_ERROR,
> +                "%s: unsupported byte read from register at offset "
> +                TARGET_FMT_plx "\n", __func__, offset);
> +    return MEMTX_ERROR;
> +}
> +
> +static MemTxResult its_writew(GICv3ITSState *s, hwaddr offset,
> +                               uint64_t value, MemTxAttrs attrs)
> +{
> +    qemu_log_mask(LOG_GUEST_ERROR,
> +        "%s: unsupported word write to register at offset "
> +        TARGET_FMT_plx "\n", __func__, offset);
> +    return MEMTX_ERROR;
> +}
> +
> +static MemTxResult its_readw(GICv3ITSState *s, hwaddr offset,
> +                               uint64_t *data, MemTxAttrs attrs)
> +{
> +    qemu_log_mask(LOG_GUEST_ERROR,
> +        "%s: unsupported word read from register at offset "
> +        TARGET_FMT_plx "\n", __func__, offset);
> +    return MEMTX_ERROR;
> +}

ditto as above

> +
> +static MemTxResult its_writel(GICv3ITSState *s, hwaddr offset,
> +                               uint64_t value, MemTxAttrs attrs)
> +{
> +    MemTxResult result = MEMTX_OK;
> +
> +    return result;
> +}
> +
> +static MemTxResult its_readl(GICv3ITSState *s, hwaddr offset,
> +                               uint64_t *data, MemTxAttrs attrs)
> +{
> +    MemTxResult result = MEMTX_OK;
> +
> +    return result;
> +}
> +
> +static MemTxResult its_writell(GICv3ITSState *s, hwaddr offset,
> +                               uint64_t value, MemTxAttrs attrs)
> +{
> +    MemTxResult result = MEMTX_OK;
> +
> +    return result;
> +}
> +
> +static MemTxResult its_readll(GICv3ITSState *s, hwaddr offset,
> +                               uint64_t *data, MemTxAttrs attrs)
> +{
> +    MemTxResult result = MEMTX_OK;
> +
> +    return result;
> +}
> +
> +static MemTxResult gicv3_its_read(void *opaque, hwaddr offset, uint64_t *data,
> +                              unsigned size, MemTxAttrs attrs)
> +{
> +    GICv3ITSState *s = (GICv3ITSState *)opaque;
> +    MemTxResult result;
> +
> +    switch (size) {
> +    case 1:
> +        result = its_readb(s, offset, data, attrs);
> +        break;
> +    case 2:
> +        result = its_readw(s, offset, data, attrs);
> +        break;
> +    case 4:
> +        result = its_readl(s, offset, data, attrs);
> +        break;
> +    case 8:
> +        result = its_readll(s, offset, data, attrs);
> +        break;
> +    default:
> +        result = MEMTX_ERROR;
> +        break;
> +    }
> +
> +    if (result == MEMTX_ERROR) {
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "%s: invalid guest read at offset " TARGET_FMT_plx
> +                      "size %u\n", __func__, offset, size);
> +        /*
> +         * The spec requires that reserved registers are RAZ/WI;
> +         * so use MEMTX_ERROR returns from leaf functions as a way to
> +         * trigger the guest-error logging but don't return it to
> +         * the caller, or we'll cause a spurious guest data abort.
> +         */
> +        result = MEMTX_OK;
> +        *data = 0;
> +    }
> +    return result;
> +}
> +
> +static MemTxResult gicv3_its_write(void *opaque, hwaddr offset, uint64_t data,
> +                               unsigned size, MemTxAttrs attrs)
> +{
> +    GICv3ITSState *s = (GICv3ITSState *)opaque;
> +    MemTxResult result;
> +
> +    switch (size) {
> +    case 1:
> +        result = its_writeb(s, offset, data, attrs);
> +        break;
> +    case 2:
> +        result = its_writew(s, offset, data, attrs);
> +        break;
> +    case 4:
> +        result = its_writel(s, offset, data, attrs);
> +        break;
> +    case 8:
> +        result = its_writell(s, offset, data, attrs);
> +        break;
> +    default:
> +        result = MEMTX_ERROR;
> +        break;
> +    }
> +
> +    if (result == MEMTX_ERROR) {
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "%s: invalid guest write at offset " TARGET_FMT_plx
> +                      "size %u\n", __func__, offset, size);
> +        /*
> +         * The spec requires that reserved registers are RAZ/WI;
> +         * so use MEMTX_ERROR returns from leaf functions as a way to
> +         * trigger the guest-error logging but don't return it to
> +         * the caller, or we'll cause a spurious guest data abort.
> +         */
> +        result = MEMTX_OK;
> +    }
> +    return result;
> +}
> +
> +static const MemoryRegionOps gicv3_its_control_ops = {
> +    .read_with_attrs = gicv3_its_read,
> +    .write_with_attrs = gicv3_its_write,
> +    .endianness = DEVICE_NATIVE_ENDIAN,
> +};
> +
> +static const MemoryRegionOps gicv3_its_trans_ops = {
> +    .read_with_attrs = gicv3_its_trans_read,
> +    .write_with_attrs = gicv3_its_trans_write,
> +    .endianness = DEVICE_NATIVE_ENDIAN,
> +};
> +
> +static void gicv3_arm_its_realize(DeviceState *dev, Error **errp)
> +{
> +    GICv3ITSState *s = ARM_GICV3_ITS_COMMON(dev);
> +
> +    gicv3_its_init_mmio(s, &gicv3_its_control_ops,
> &gicv3_its_trans_ops);

Ahh I got confused later on because we have two local static structures
for &gicv3_its_trans_ops. Could we give them differentiated names
please?

> +}
> +
> +static void gicv3_its_reset(DeviceState *dev)
> +{
> +    GICv3ITSState *s = ARM_GICV3_ITS_COMMON(dev);
> +    GICv3ITSClass *c = ARM_GICV3_ITS_GET_CLASS(s);
> +
> +    if (s->gicv3->cpu->gicr_typer & GICR_TYPER_PLPIS) {
> +        c->parent_reset(dev);
> +
> +        /* set the ITS default features supported */
> +        s->typer = GITS_TYPER_PHYSICAL | (GITS_TYPER_ITT_ENTRY_SIZE <<
> +                   GITS_TYPER_ITT_ENTRY_OFFSET) | (GITS_TYPER_IDBITS <<
> +                   GITS_TYPER_IDBITS_OFFSET) | GITS_TYPER_DEVBITS |
> +                   GITS_TYPER_CIL | GITS_TYPER_CIDBITS;
> +
> +        /*
> +         * We claim to be an ARM r0p0 with a zero ProductID.
> +         * This is the same as an r0p0 GIC-500.
> +         */
> +        s->iidr = gicv3_iidr();
> +
> +        /* Quiescent bit reset to 1 */
> +        s->ctlr = (1U << 31);

I see s->ctlr is set variously with deposit32, manual shift/mask/or and
here with a hard-coded 1 << 31. We generally prefer symbolic names for
register bits. I suspect it might be worth converting the ctlr symbols
to a set of REG32/FIELD descriptors, i.e.:

REG32(GICD_CTLR,                0x0)
    FIELD(GICD_CTLR, EN_GRP0, 0, 1)
    FIELD(GICD_CTLR, EN_GRP1, 1, 1)
    FIELD(GICD_CTLR, RESET,  31, 1)

which would make it:

  s->ctlr = FIELD_DP32(0, GICD_CTLR, RESET, 1);

or

  s->ctlr = (1U << R_GICD_CTLR_RESET_SHIFT);

if the DP32 seems a little over-kill (but probably not). I see bits of
the GIC code have already been converted.

> +
> +        /*
> +         * setting GITS_BASER0.Type = 0b001 (Device)
> +         *         GITS_BASER1.Type = 0b100 (Collection Table)
> +         *         GITS_BASER<n>.Type,where n = 3 to 7 are 0b00 (Unimplemented)
> +         *         GITS_BASER<0,1>.Page_Size = 64KB
> +         * and default translation table entry size to 16 bytes
> +         */
> +        s->baser[0] = (GITS_ITT_TYPE_DEVICE << GITS_BASER_TYPE_OFFSET) |
> +                      (GITS_BASER_PAGESIZE_64K << GITS_BASER_PAGESIZE_OFFSET) |
> +                      (GITS_DTE_SIZE << GITS_BASER_ENTRYSIZE_OFFSET);
> +        s->baser[1] = (GITS_ITT_TYPE_COLLECTION << GITS_BASER_TYPE_OFFSET) |
> +                      (GITS_BASER_PAGESIZE_64K << GITS_BASER_PAGESIZE_OFFSET) |
> +                      (GITS_CTE_SIZE << GITS_BASER_ENTRYSIZE_OFFSET);
> +    }
> +}
> +
> +static Property gicv3_its_props[] = {
> +    DEFINE_PROP_LINK("parent-gicv3", GICv3ITSState, gicv3, "arm-gicv3",
> +                     GICv3State *),
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
> +static void gicv3_its_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +    GICv3ITSClass *ic = ARM_GICV3_ITS_CLASS(klass);
> +
> +    dc->realize = gicv3_arm_its_realize;
> +    device_class_set_props(dc, gicv3_its_props);
> +    device_class_set_parent_reset(dc, gicv3_its_reset, &ic->parent_reset);
> +}
> +
> +static const TypeInfo gicv3_its_info = {
> +    .name = TYPE_ARM_GICV3_ITS,
> +    .parent = TYPE_ARM_GICV3_ITS_COMMON,
> +    .instance_size = sizeof(GICv3ITSState),
> +    .class_init = gicv3_its_class_init,
> +    .class_size = sizeof(GICv3ITSClass),
> +};
> +
> +static void gicv3_its_register_types(void)
> +{
> +    type_register_static(&gicv3_its_info);
> +}
> +
> +type_init(gicv3_its_register_types)
> diff --git a/hw/intc/arm_gicv3_its_common.c b/hw/intc/arm_gicv3_its_common.c
> index 66c4c6a188..c18fe23fae 100644
> --- a/hw/intc/arm_gicv3_its_common.c
> +++ b/hw/intc/arm_gicv3_its_common.c
> @@ -55,7 +55,9 @@ static const VMStateDescription vmstate_its = {
>      .priority = MIG_PRI_GICV3_ITS,
>      .fields = (VMStateField[]) {
>          VMSTATE_UINT32(ctlr, GICv3ITSState),
> +        VMSTATE_UINT32(translater, GICv3ITSState),
>          VMSTATE_UINT32(iidr, GICv3ITSState),
> +        VMSTATE_UINT64(typer, GICv3ITSState),
>          VMSTATE_UINT64(cbaser, GICv3ITSState),
>          VMSTATE_UINT64(cwriter, GICv3ITSState),
>          VMSTATE_UINT64(creadr, GICv3ITSState),

I think you need to bump:

    .version_id = 1,
    .minimum_version_id = 1,

here so we don't attempt to migrate in an incompatible VMstate from a
pre-ITS migration. Are these fields always used now? If not we might
want to jump a few more hoops to preserve compatibility and make the
fields optional.

> @@ -99,15 +101,21 @@ static const MemoryRegionOps gicv3_its_trans_ops = {
>      .endianness = DEVICE_NATIVE_ENDIAN,
>  };
>  
> -void gicv3_its_init_mmio(GICv3ITSState *s, const MemoryRegionOps *ops)
> +void gicv3_its_init_mmio(GICv3ITSState *s, const MemoryRegionOps *ops,
> +     const MemoryRegionOps *tops)
>  {
>      SysBusDevice *sbd = SYS_BUS_DEVICE(s);
>  
>      memory_region_init_io(&s->iomem_its_cntrl, OBJECT(s), ops, s,
>                            "control", ITS_CONTROL_SIZE);
> -    memory_region_init_io(&s->iomem_its_translation, OBJECT(s),
> -                          &gicv3_its_trans_ops, s,
> -                          "translation", ITS_TRANS_SIZE);
> +    if (tops == NULL) {
> +        memory_region_init_io(&s->iomem_its_translation, OBJECT(s),
> +                              &gicv3_its_trans_ops, s,
> +                              "translation", ITS_TRANS_SIZE);
> +    } else {
> +        memory_region_init_io(&s->iomem_its_translation, OBJECT(s),
> +                              tops, s, "translation", ITS_TRANS_SIZE);
> +    }

You might as well short-circuit it one line as it's the only difference,
i.e.:

   memory_region_init_io(&s->iomem_its_translation, OBJECT(s),
                         tops ? tops : &gicv3_its_trans_ops, s,
                         "translation", ITS_TRANS_SIZE);

but see bellow.

>      /* Our two regions are always adjacent, therefore we now combine them
>       * into a single one in order to make our users' life easier.
> @@ -130,6 +138,7 @@ static void gicv3_its_common_reset(DeviceState *dev)
>      s->cwriter = 0;
>      s->creadr = 0;
>      s->iidr = 0;
> +    s->translater = 0;
>      memset(&s->baser, 0, sizeof(s->baser));
>  }
>  
> diff --git a/hw/intc/arm_gicv3_its_kvm.c b/hw/intc/arm_gicv3_its_kvm.c
> index b554d2ede0..0b4cbed28b 100644
> --- a/hw/intc/arm_gicv3_its_kvm.c
> +++ b/hw/intc/arm_gicv3_its_kvm.c
> @@ -106,7 +106,7 @@ static void kvm_arm_its_realize(DeviceState *dev, Error **errp)
>      kvm_arm_register_device(&s->iomem_its_cntrl, -1, KVM_DEV_ARM_VGIC_GRP_ADDR,
>                              KVM_VGIC_ITS_ADDR_TYPE, s->dev_fd, 0);
>  
> -    gicv3_its_init_mmio(s, NULL);
> +    gicv3_its_init_mmio(s, NULL, NULL);
>  
>      if (!kvm_device_check_attr(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_ITS_REGS,
>          GITS_CTLR)) {
> diff --git a/hw/intc/gicv3_internal.h b/hw/intc/gicv3_internal.h
> index 05303a55c8..7c6bc33e97 100644
> --- a/hw/intc/gicv3_internal.h
> +++ b/hw/intc/gicv3_internal.h
> @@ -67,6 +67,12 @@
>  #define GICD_CTLR_E1NWF             (1U << 7)
>  #define GICD_CTLR_RWP               (1U << 31)
>  
> +#define GICD_TYPER_LPIS_OFFSET         17
> +#define GICD_TYPER_IDBITS_OFFSET       19
> +#define GICD_TYPER_IDBITS_MASK       0x1f
> +/* 16 bits EventId */
> +#define GICD_TYPER_IDBITS            0xf
> +
>  /*
>   * Redistributor frame offsets from RD_base
>   */
> @@ -123,14 +129,17 @@
>  #define GICR_WAKER_ChildrenAsleep    (1U << 2)
>  
>  #define GICR_PROPBASER_OUTER_CACHEABILITY_MASK (7ULL << 56)
> -#define GICR_PROPBASER_ADDR_MASK               (0xfffffffffULL << 12)
> +#define GICR_PROPBASER_ADDR_OFFSET               12
> +#define GICR_PROPBASER_ADDR_MASK               ((1ULL << 40) - 1)
>  #define GICR_PROPBASER_SHAREABILITY_MASK       (3U << 10)
>  #define GICR_PROPBASER_CACHEABILITY_MASK       (7U << 7)
>  #define GICR_PROPBASER_IDBITS_MASK             (0x1f)
> +#define GICR_PROPBASER_IDBITS_THRESHOLD          0xd

More candidates for REG/FIELD conversion.

>  
>  #define GICR_PENDBASER_PTZ                     (1ULL << 62)
>  #define GICR_PENDBASER_OUTER_CACHEABILITY_MASK (7ULL << 56)
> -#define GICR_PENDBASER_ADDR_MASK               (0xffffffffULL << 16)
> +#define GICR_PENDBASER_ADDR_OFFSET               16
> +#define GICR_PENDBASER_ADDR_MASK               ((1ULL << 36) - 1)
>  #define GICR_PENDBASER_SHAREABILITY_MASK       (3U << 10)
>  #define GICR_PENDBASER_CACHEABILITY_MASK       (7U << 7)
>  
> @@ -239,11 +248,171 @@
>  #define ICH_VTR_EL2_PREBITS_SHIFT 26
>  #define ICH_VTR_EL2_PRIBITS_SHIFT 29
>  
> +#define GITS_CTLR_ENABLED             (1U << 0)  /* ITS Enabled or not */
> +#define GITS_CTLR_QUIESCENT           (1U << 31) /* Quiescent bit */
> +
> +#define GITS_BASER_SIZE                     (0xff)
> +#define GITS_BASER_PAGESIZE_OFFSET            8
> +#define GITS_BASER_PAGESIZE_MASK             0x3
> +#define GITS_BASER_SHAREABILITY_OFFSET        10
> +#define GITS_BASER_SHAREABILITY_MASK         0x3
> +#define GITS_BASER_PHYADDR_OFFSET             12
> +#define GITS_BASER_PHYADDR_MASK             ((1ULL << 36) - 1)
> +#define GITS_BASER_PHYADDR_OFFSETL_64K        16
> +#define GITS_BASER_PHYADDR_MASKL_64K        ((1ULL << 32) - 1)
> +#define GITS_BASER_PHYADDR_OFFSETH_64K        48
> +#define GITS_BASER_PHYADDR_MASKH_64K        ((1ULL << 4) - 1)
> +#define GITS_BASER_ENTRYSIZE_OFFSET           48
> +#define GITS_BASER_ENTRYSIZE_MASK           ((1U << 5) - 1)
> +#define GITS_BASER_OUTERCACHE_OFFSET          53
> +#define GITS_BASER_OUTERCACHE_MASK            0x7
> +#define GITS_BASER_TYPE_OFFSET                56
> +#define GITS_BASER_TYPE_MASK                  0x7
> +#define GITS_BASER_INNERCACHE_OFFSET          59
> +#define GITS_BASER_INNERCACHE_MASK            0x7
> +#define GITS_BASER_INDIRECT_OFFSET            62
> +#define GITS_BASER_INDIRECT_MASK              0x1
> +#define GITS_BASER_VALID                      63
> +#define GITS_BASER_VALID_MASK                 0x1
> +
> +#define GITS_BASER_VAL_MASK                 ((0x7ULL << 56) | (0x1fULL << 48))
> +
> +#define GITS_BASER_PAGESIZE_4K                0
> +#define GITS_BASER_PAGESIZE_16K               1
> +#define GITS_BASER_PAGESIZE_64K               2
> +
> +#define GITS_ITT_TYPE_DEVICE                  1ULL
> +#define GITS_ITT_TYPE_COLLECTION              4ULL
> +
> +#define GITS_CBASER_SIZE                    (0xff)
> +#define GITS_CBASER_SHAREABILITY_OFFSET       10
> +#define GITS_CBASER_SHAREABILITY_MASK        0x3
> +#define GITS_CBASER_PHYADDR_OFFSET            12
> +#define GITS_CBASER_PHYADDR_MASK       ((1ULL << 40) - 1)
> +#define GITS_CBASER_OUTERCACHE_OFFSET         53
> +#define GITS_CBASER_OUTERCACHE_MASK          0x7
> +#define GITS_CBASER_INNERCACHE_OFFSET         59
> +#define GITS_CBASER_INNERCACHE_MASK          0x7
> +#define GITS_CBASER_VALID_OFFSET              63
> +#define GITS_CBASER_VALID_MASK               0x1
> +
> +#define GITS_CREADR_STALLED           (1U << 0)
> +#define GITS_CREADR_OFFSET             5
> +#define GITS_CREADR_OFFSET_MASK       ((1U << 15) - 1)
> +
> +#define GITS_CWRITER_RETRY            (1U << 0)
> +#define GITS_CWRITER_OFFSET             5
> +#define GITS_CWRITER_OFFSET_MASK      ((1U << 15) - 1)
> +
> +#define GITS_TYPER_DEVBITS_OFFSET       13
> +#define GITS_TYPER_DEVBITS_MASK        0x1f
> +#define GITS_TYPER_IDBITS_OFFSET        8
> +#define GITS_TYPER_IDBITS_MASK         0x1f
> +#define GITS_TYPER_CIDBITS_OFFSET       32
> +#define GITS_TYPER_CIDBITS_MASK        0xf
> +#define GITS_TYPER_CIL_OFFSET           36
> +#define GITS_TYPER_CIL_MASK             0x1
> +#define GITS_TYPER_PTA_OFFSET           19
> +#define GITS_TYPER_PTA_MASK             0x1
> +#define GITS_TYPER_SEIS_OFFSET          18
> +#define GITS_TYPER_SEIS_MASK            0x1

And here.

> +
> +/* Default features advertised by this version of ITS */
> +/* Physical LPIs supported */
> +#define GITS_TYPER_PHYSICAL           (1U << 0)
> +/*
> + * 11 bytes Interrupt translation Table Entry size
> + * Valid = 1 bit,InterruptType = 1 bit,
> + * Size of LPI number space[considering max 24 bits],
> + * Size of LPI number space[considering max 24 bits],
> + * ICID = 16 bits,
> + * vPEID = 16 bits
> + */
> +#define GITS_TYPER_ITT_ENTRY_OFFSET     4
> +#define GITS_TYPER_ITT_ENTRY_SIZE       0xB
> +#define GITS_TYPER_IDBITS_OFFSET        8
> +
> +/* 16 bits EventId */
> +#define GITS_TYPER_IDBITS             GICD_TYPER_IDBITS
> +/* 16 bits DeviceId */
> +#define GITS_TYPER_DEVBITS            (0xf << 13)
> +/* Collection ID Limit indicated by CIDbits(next) */
> +#define GITS_TYPER_CIL                (1ULL << 36)
> +/* 16 bits CollectionId */
> +#define GITS_TYPER_CIDBITS            (0xfULL << 32)
> +/*
> + * 8 bytes Device Table Entry size
> + * Valid = 1 bit,ITTAddr = 44 bits,Size = 5 bits
> + */
> +#define GITS_DTE_SIZE                 (0x8ULL)
> +/*
> + * 8 bytes Collection Table Entry size
> + * Valid = 1 bit,RDBase = 36 bits(considering max RDBASE)
> + */
> +#define GITS_CTE_SIZE                 (0x8ULL)
> +
>  /* Special interrupt IDs */
>  #define INTID_SECURE 1020
>  #define INTID_NONSECURE 1021
>  #define INTID_SPURIOUS 1023
>  
> +/* ITS Commands */
> +#define GITS_CMD_CLEAR            0x04
> +#define GITS_CMD_DISCARD          0x0F
> +#define GITS_CMD_INT              0x03
> +#define GITS_CMD_MAPC             0x09
> +#define GITS_CMD_MAPD             0x08
> +#define GITS_CMD_MAPI             0x0B
> +#define GITS_CMD_MAPTI            0x0A
> +#define GITS_CMD_SYNC             0x05

Although looking at this patch I suspect the definition of registers
should come in when they are first used.

> +
> +#define GITS_ITT_PAGE_SIZE_0      0x1000
> +#define GITS_ITT_PAGE_SIZE_1      0x4000
> +#define GITS_ITT_PAGE_SIZE_2      0x10000
> +
> +#define GITS_CMDQ_ENTRY_SIZE               32
> +#define GITS_CMDQ_MAX_ENTRIES_PER_PAGE     128
> +#define NUM_BYTES_IN_DW                     8
> +
> +#define CMD_MASK                  0xff
> +
> +/* MAPC command fields */
> +#define ICID_LEN                  16
> +#define ICID_MASK                 ((1U << ICID_LEN) - 1)
> +#define RDBASE_LEN                32
> +#define RDBASE_OFFSET             16
> +#define RDBASE_MASK               ((1ULL << RDBASE_LEN) - 1)
> +#define RDBASE_PROCNUM_LEN        16
> +#define RDBASE_PROCNUM_MASK       ((1ULL << RDBASE_PROCNUM_LEN) - 1)
> +
> +#define DEVID_OFFSET              32
> +#define DEVID_LEN                 32
> +#define DEVID_MASK                ((1ULL << DEVID_LEN) - 1)
> +
> +/* MAPD command fields */
> +#define ITTADDR_LEN               44
> +#define ITTADDR_OFFSET            8
> +#define ITTADDR_MASK              ((1ULL << ITTADDR_LEN) - 1)
> +#define SIZE_MASK                 0x1f
> +
> +/* MAPI command fields */
> +#define EVENTID_MASK              ((1ULL << 32) - 1)
> +
> +/* MAPTI command fields */
> +#define pINTID_OFFSET              32
> +#define pINTID_MASK               ((1ULL << 32) - 1)
> +
> +#define VALID_SHIFT               63
> +#define VALID_MASK                0x1
> +
> +#define L1TABLE_ENTRY_SIZE         8
> +
> +#define LPI_CTE_ENABLE_OFFSET      0
> +#define LPI_CTE_ENABLED          VALID_MASK
> +#define LPI_CTE_PRIORITY_OFFSET    2
> +#define LPI_CTE_PRIORITY_MASK     ((1U << 6) - 1)
> +#define LPI_PRIORITY_MASK         0xfc
> +
>  /* Functions internal to the emulated GICv3 */
>  
>  /**
> diff --git a/hw/intc/meson.build b/hw/intc/meson.build
> index 1c299039f6..53472239f0 100644
> --- a/hw/intc/meson.build
> +++ b/hw/intc/meson.build
> @@ -8,6 +8,7 @@ softmmu_ss.add(when: 'CONFIG_ARM_GIC', if_true: files(
>    'arm_gicv3_dist.c',
>    'arm_gicv3_its_common.c',
>    'arm_gicv3_redist.c',
> +  'arm_gicv3_its.c',
>  ))
>  softmmu_ss.add(when: 'CONFIG_ETRAXFS', if_true: files('etraxfs_pic.c'))
>  softmmu_ss.add(when: 'CONFIG_HEATHROW_PIC', if_true: files('heathrow_pic.c'))
> diff --git a/include/hw/intc/arm_gicv3_its_common.h b/include/hw/intc/arm_gicv3_its_common.h
> index 5a0952b404..08bc5652ed 100644
> --- a/include/hw/intc/arm_gicv3_its_common.h
> +++ b/include/hw/intc/arm_gicv3_its_common.h
> @@ -25,17 +25,24 @@
>  #include "hw/intc/arm_gicv3_common.h"
>  #include "qom/object.h"
>  
> +#define TYPE_ARM_GICV3_ITS "arm-gicv3-its"
> +
>  #define ITS_CONTROL_SIZE 0x10000
>  #define ITS_TRANS_SIZE   0x10000
>  #define ITS_SIZE         (ITS_CONTROL_SIZE + ITS_TRANS_SIZE)
>  
>  #define GITS_CTLR        0x0
>  #define GITS_IIDR        0x4
> +#define GITS_TYPER       0x8
>  #define GITS_CBASER      0x80
>  #define GITS_CWRITER     0x88
>  #define GITS_CREADR      0x90
>  #define GITS_BASER       0x100
>  
> +#define GITS_TRANSLATER  0x0040
> +
> +#define GITS_PIDR2       0xFFE8
> +
>  struct GICv3ITSState {
>      SysBusDevice parent_obj;
>  
> @@ -52,6 +59,8 @@ struct GICv3ITSState {
>      /* Registers */
>      uint32_t ctlr;
>      uint32_t iidr;
> +    uint32_t translater;
> +    uint64_t typer;
>      uint64_t cbaser;
>      uint64_t cwriter;
>      uint64_t creadr;
> @@ -62,7 +71,8 @@ struct GICv3ITSState {
>  
>  typedef struct GICv3ITSState GICv3ITSState;
>  
> -void gicv3_its_init_mmio(GICv3ITSState *s, const MemoryRegionOps *ops);
> +void gicv3_its_init_mmio(GICv3ITSState *s, const MemoryRegionOps *ops,
> +                   const MemoryRegionOps *tops);
>  
>  #define TYPE_ARM_GICV3_ITS_COMMON "arm-gicv3-its-common"
>  typedef struct GICv3ITSCommonClass GICv3ITSCommonClass;


-- 
Alex Bennée


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

* Re: [PATCH v1 1/8] hw/intc: GICv3 ITS initial framework
  2021-03-23  4:12 ` [PATCH v1 1/8] hw/intc: GICv3 ITS initial framework Shashi Mallela
  2021-03-25 16:43   ` Alex Bennée
@ 2021-03-25 17:18   ` Alex Bennée
  1 sibling, 0 replies; 16+ messages in thread
From: Alex Bennée @ 2021-03-25 17:18 UTC (permalink / raw)
  To: Shashi Mallela; +Cc: peter.maydell, leif, qemu-devel, qemu-arm, rad


Shashi Mallela <shashi.mallela@linaro.org> writes:

> Added register definitions relevant to ITS,implemented overall
> ITS device framework with stubs for ITS control and translater
> regions read/write,extended ITS common to handle mmio init between
> existing kvm device and newer qemu device.
>
> Signed-off-by: Shashi Mallela <shashi.mallela@linaro.org>
> ---
<snip>
>  
>  typedef struct GICv3ITSState GICv3ITSState;
>

Also could we kernel-doc this function and explain when and why tops
would be NULL.

> -void gicv3_its_init_mmio(GICv3ITSState *s, const MemoryRegionOps *ops);
> +void gicv3_its_init_mmio(GICv3ITSState *s, const MemoryRegionOps *ops,
> +                   const MemoryRegionOps *tops);
>  
>  #define TYPE_ARM_GICV3_ITS_COMMON "arm-gicv3-its-common"
>  typedef struct GICv3ITSCommonClass GICv3ITSCommonClass;


-- 
Alex Bennée


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

* Re: [PATCH v1 0/8] GICv3 LPI and ITS feature implementation
  2021-03-23  4:12 [PATCH v1 0/8] GICv3 LPI and ITS feature implementation Shashi Mallela
                   ` (7 preceding siblings ...)
  2021-03-23  4:12 ` [PATCH v1 8/8] hw/arm/virt: add ITS support in virt GIC Shashi Mallela
@ 2021-03-25 17:59 ` Alex Bennée
  2021-03-25 19:44 ` Alex Bennée
  9 siblings, 0 replies; 16+ messages in thread
From: Alex Bennée @ 2021-03-25 17:59 UTC (permalink / raw)
  To: Shashi Mallela; +Cc: peter.maydell, leif, qemu-devel, qemu-arm, rad

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


Shashi Mallela <shashi.mallela@linaro.org> writes:

> This patchset implements qemu device model for enabling physical
> LPI support and ITS functionality in GIC as per GICv3 specification.
> Both flat table and 2 level tables are implemented.The ITS commands
> for adding/deleting ITS table entries,trigerring LPI interrupts are 
> implemented.Translated LPI interrupt ids are processed by redistributor
> to determine priority and set pending state appropriately before
> forwarding the same to cpu interface.
> The ITS feature support has been added to sbsa-ref platform as well as
> virt platform,wherein the emulated functionality co-exists with kvm
> kernel functionality.

Running the kvm-unit-tests ITS set:

   env QEMU=$HOME/lsrc/qemu.git/builds/arm.all/qemu-system-aarch64 ./run_tests.sh -g its

with a patched unitests.cfg to remove the KVM requirement I get:

  PASS its-introspection (5 tests)
  FAIL its-trigger (6 tests, 1 unexpected failures)
  FAIL its-migration
  FAIL its-pending-migration (1 tests, 1 unexpected failures)
  SKIP its-migrate-unmapped-collection (1 tests, 1 skipped)

The its-migration asserts:

  Now migrate the VM, then press a key to continue...
  INFO: gicv3: its-migration: Migration complete
  INT dev_id=2 event_id=20
  /home/alex/lsrc/tests/kvm-unit-tests.git/lib/arm64/gic-v3-its-cmd.c:192: assert failed: false: INT timeout!
          STACK:

Full logs attached:


[-- Attachment #2: logs from kvm-unit-tests ITS run --]
[-- Type: application/octet-stream, Size: 21325 bytes --]

timeout -k 1s --foreground 90s /home/alex/lsrc/qemu.git/builds/arm.all/qemu-system-aarch64 -nodefaults -machine virt,accel=tcg -cpu cortex-a57 -device virtio-serial-device -device virtconsole,chardev=ctd -chardev testdev,id=ctd -device pci-testdev -display none -serial stdio -kernel arm/gic.flat -smp 8 -machine gic-version=3 -append its-introspection # -initrd /tmp/tmp.irCRYgMeFT
PASS: gicv3: its-introspection: GITS_IIDR is read-only
PASS: gicv3: its-introspection: GITS_TYPER is read-only
PASS: gicv3: its-introspection: ITS supports physical LPIs
INFO: gicv3: its-introspection: vLPI support: no
INFO: gicv3: its-introspection: ITT entry size = 0xc
INFO: gicv3: its-introspection: Bit Count: EventID=16 DeviceId=16 CollId=16
PASS: gicv3: its-introspection: ID spaces
INFO: gicv3: its-introspection: Target address format PE #
PASS: gicv3: its-introspection: detect device and collection BASER
INFO: gicv3: its-introspection: device table entry_size = 0x9
INFO: gicv3: its-introspection: collection table entry_size = 0x9
SUMMARY: 5 tests

run_migration timeout -k 1s --foreground 90s /home/alex/lsrc/qemu.git/builds/arm.all/qemu-system-aarch64 -nodefaults -machine virt,accel=tcg -cpu cortex-a57 -device virtio-serial-device -device virtconsole,chardev=ctd -chardev testdev,id=ctd -device pci-testdev -display none -serial stdio -kernel arm/gic.flat -smp 8 -machine gic-version=3 -append its-migrate-unmapped-collection # -initrd /tmp/tmp.DQwPrbFkd0
qemu-system-aarch64:qemu-system-aarch64: -chardev -chardev socket,id=mon1,path=/tmp/mig-helper-qmp1.yEhVzheBai,server,nowait socket,id=mon2,path=/tmp/mig-helper-qmp2.WIhQnCH5nq,server,nowait: : warning: warning: short-form boolean option 'server' deprecatedshort-form boolean option 'server' deprecated

Please use server=on instead
Please use server=on instead
qemu-system-aarch64:qemu-system-aarch64: -chardev -chardev socket,id=mon1,path=/tmp/mig-helper-qmp1.yEhVzheBai,server,nowait socket,id=mon2,path=/tmp/mig-helper-qmp2.WIhQnCH5nq,server,nowait: : warning: warning: short-form boolean option 'nowait' deprecatedshort-form boolean option 'nowait' deprecated

Please use wait=off instead
Please use wait=off instead
ITS: MAPD devid=2 size = 0x8 itt=0x40440000 valid=1
ITS: MAPD devid=7 size = 0x8 itt=0x40450000 valid=1
MAPC col_id=3 target_addr = 0x30000 valid=1
MAPC col_id=2 target_addr = 0x20000 valid=1
INVALL col_id=2
INVALL col_id=3
MAPTI dev_id=2 event_id=20 -> phys_id=8195, col_id=3
MAPTI dev_id=7 event_id=255 -> phys_id=8196, col_id=2
SKIP: gicv3: its-migrate-unmapped-collection: Skipping test, as this test hangs without the fix. Set ERRATA_8c58be34494b=y to enable.
Now migrate the VM, then press a key to continue...
INFO: gicv3: its-migrate-unmapped-collection: Migration complete
SUMMARY: 1 tests, 1 skipped

run_migration timeout -k 1s --foreground 90s /home/alex/lsrc/qemu.git/builds/arm.all/qemu-system-aarch64 -nodefaults -machine virt,accel=tcg -cpu cortex-a57 -device virtio-serial-device -device virtconsole,chardev=ctd -chardev testdev,id=ctd -device pci-testdev -display none -serial stdio -kernel arm/gic.flat -smp 8 -machine gic-version=3 -append its-migration # -initrd /tmp/tmp.o6yvmr7IoW
qemu-system-aarch64: -chardev socket,id=mon2,path=/tmp/mig-helper-qmp2.26HZNvi5R6,server,nowait: warning: short-form boolean option 'server' deprecated
Please use server=on instead
qemu-system-aarch64: -chardev socket,id=mon2,path=/tmp/mig-helper-qmp2.26HZNvi5R6,server,nowait: warning: short-form boolean option 'nowait' deprecated
Please use wait=off instead
qemu-system-aarch64: -chardev socket,id=mon1,path=/tmp/mig-helper-qmp1.a3pbeaOG0G,server,nowait: warning: short-form boolean option 'server' deprecated
Please use server=on instead
qemu-system-aarch64: -chardev socket,id=mon1,path=/tmp/mig-helper-qmp1.a3pbeaOG0G,server,nowait: warning: short-form boolean option 'nowait' deprecated
Please use wait=off instead
ITS: MAPD devid=2 size = 0x8 itt=0x40440000 valid=1
ITS: MAPD devid=7 size = 0x8 itt=0x40450000 valid=1
MAPC col_id=3 target_addr = 0x30000 valid=1
MAPC col_id=2 target_addr = 0x20000 valid=1
INVALL col_id=2
INVALL col_id=3
MAPTI dev_id=2 event_id=20 -> phys_id=8195, col_id=3
MAPTI dev_id=7 event_id=255 -> phys_id=8196, col_id=2
Now migrate the VM, then press a key to continue...
INFO: gicv3: its-migration: Migration complete
INT dev_id=2 event_id=20
/home/alex/lsrc/tests/kvm-unit-tests.git/lib/arm64/gic-v3-its-cmd.c:192: assert failed: false: INT timeout!
	STACK:

run_migration timeout -k 1s --foreground 90s /home/alex/lsrc/qemu.git/builds/arm.all/qemu-system-aarch64 -nodefaults -machine virt,accel=tcg -cpu cortex-a57 -device virtio-serial-device -device virtconsole,chardev=ctd -chardev testdev,id=ctd -device pci-testdev -display none -serial stdio -kernel arm/gic.flat -smp 8 -machine gic-version=3 -append its-pending-migration # -initrd /tmp/tmp.g0MCehNHE7
qemu-system-aarch64: -chardev socket,id=mon2,path=/tmp/mig-helper-qmp2.LzwzLgvqSJ,server,nowait: warning: short-form boolean option 'server' deprecated
Please use server=on instead
qemu-system-aarch64: -chardev socket,id=mon2,path=/tmp/mig-helper-qmp2.LzwzLgvqSJ,server,nowait: warning: short-form boolean option 'nowait' deprecated
qemu-system-aarch64:Please use wait=off instead
 -chardev socket,id=mon1,path=/tmp/mig-helper-qmp1.HVjOEOLU2L,server,nowait: warning: short-form boolean option 'server' deprecated
Please use server=on instead
qemu-system-aarch64: -chardev socket,id=mon1,path=/tmp/mig-helper-qmp1.HVjOEOLU2L,server,nowait: warning: short-form boolean option 'nowait' deprecated
Please use wait=off instead
ITS: MAPD devid=2 size = 0x8 itt=0x40450000 valid=1
MAPC col_id=7 target_addr = 0x70000 valid=1
MAPC col_id=6 target_addr = 0x60000 valid=1
MAPTI dev_id=2 event_id=0 -> phys_id=8192, col_id=6
MAPTI dev_id=2 event_id=1 -> phys_id=8193, col_id=7
MAPTI dev_id=2 event_id=2 -> phys_id=8194, col_id=6
MAPTI dev_id=2 event_id=3 -> phys_id=8195, col_id=7
MAPTI dev_id=2 event_id=4 -> phys_id=8196, col_id=6
MAPTI dev_id=2 event_id=5 -> phys_id=8197, col_id=7
MAPTI dev_id=2 event_id=6 -> phys_id=8198, col_id=6
MAPTI dev_id=2 event_id=7 -> phys_id=8199, col_id=7
MAPTI dev_id=2 event_id=8 -> phys_id=8200, col_id=6
MAPTI dev_id=2 event_id=9 -> phys_id=8201, col_id=7
MAPTI dev_id=2 event_id=10 -> phys_id=8202, col_id=6
MAPTI dev_id=2 event_id=11 -> phys_id=8203, col_id=7
MAPTI dev_id=2 event_id=12 -> phys_id=8204, col_id=6
MAPTI dev_id=2 event_id=13 -> phys_id=8205, col_id=7
MAPTI dev_id=2 event_id=14 -> phys_id=8206, col_id=6
MAPTI dev_id=2 event_id=15 -> phys_id=8207, col_id=7
MAPTI dev_id=2 event_id=16 -> phys_id=8208, col_id=6
MAPTI dev_id=2 event_id=17 -> phys_id=8209, col_id=7
MAPTI dev_id=2 event_id=18 -> phys_id=8210, col_id=6
MAPTI dev_id=2 event_id=19 -> phys_id=8211, col_id=7
MAPTI dev_id=2 event_id=20 -> phys_id=8212, col_id=6
MAPTI dev_id=2 event_id=21 -> phys_id=8213, col_id=7
MAPTI dev_id=2 event_id=22 -> phys_id=8214, col_id=6
MAPTI dev_id=2 event_id=23 -> phys_id=8215, col_id=7
MAPTI dev_id=2 event_id=24 -> phys_id=8216, col_id=6
MAPTI dev_id=2 event_id=25 -> phys_id=8217, col_id=7
MAPTI dev_id=2 event_id=26 -> phys_id=8218, col_id=6
MAPTI dev_id=2 event_id=27 -> phys_id=8219, col_id=7
MAPTI dev_id=2 event_id=28 -> phys_id=8220, col_id=6
MAPTI dev_id=2 event_id=29 -> phys_id=8221, col_id=7
MAPTI dev_id=2 event_id=30 -> phys_id=8222, col_id=6
MAPTI dev_id=2 event_id=31 -> phys_id=8223, col_id=7
MAPTI dev_id=2 event_id=32 -> phys_id=8224, col_id=6
MAPTI dev_id=2 event_id=33 -> phys_id=8225, col_id=7
MAPTI dev_id=2 event_id=34 -> phys_id=8226, col_id=6
MAPTI dev_id=2 event_id=35 -> phys_id=8227, col_id=7
MAPTI dev_id=2 event_id=36 -> phys_id=8228, col_id=6
MAPTI dev_id=2 event_id=37 -> phys_id=8229, col_id=7
MAPTI dev_id=2 event_id=38 -> phys_id=8230, col_id=6
MAPTI dev_id=2 event_id=39 -> phys_id=8231, col_id=7
MAPTI dev_id=2 event_id=40 -> phys_id=8232, col_id=6
MAPTI dev_id=2 event_id=41 -> phys_id=8233, col_id=7
MAPTI dev_id=2 event_id=42 -> phys_id=8234, col_id=6
MAPTI dev_id=2 event_id=43 -> phys_id=8235, col_id=7
MAPTI dev_id=2 event_id=44 -> phys_id=8236, col_id=6
MAPTI dev_id=2 event_id=45 -> phys_id=8237, col_id=7
MAPTI dev_id=2 event_id=46 -> phys_id=8238, col_id=6
MAPTI dev_id=2 event_id=47 -> phys_id=8239, col_id=7
MAPTI dev_id=2 event_id=48 -> phys_id=8240, col_id=6
MAPTI dev_id=2 event_id=49 -> phys_id=8241, col_id=7
MAPTI dev_id=2 event_id=50 -> phys_id=8242, col_id=6
MAPTI dev_id=2 event_id=51 -> phys_id=8243, col_id=7
MAPTI dev_id=2 event_id=52 -> phys_id=8244, col_id=6
MAPTI dev_id=2 event_id=53 -> phys_id=8245, col_id=7
MAPTI dev_id=2 event_id=54 -> phys_id=8246, col_id=6
MAPTI dev_id=2 event_id=55 -> phys_id=8247, col_id=7
MAPTI dev_id=2 event_id=56 -> phys_id=8248, col_id=6
MAPTI dev_id=2 event_id=57 -> phys_id=8249, col_id=7
MAPTI dev_id=2 event_id=58 -> phys_id=8250, col_id=6
MAPTI dev_id=2 event_id=59 -> phys_id=8251, col_id=7
MAPTI dev_id=2 event_id=60 -> phys_id=8252, col_id=6
MAPTI dev_id=2 event_id=61 -> phys_id=8253, col_id=7
MAPTI dev_id=2 event_id=62 -> phys_id=8254, col_id=6
MAPTI dev_id=2 event_id=63 -> phys_id=8255, col_id=7
MAPTI dev_id=2 event_id=64 -> phys_id=8256, col_id=6
MAPTI dev_id=2 event_id=65 -> phys_id=8257, col_id=7
MAPTI dev_id=2 event_id=66 -> phys_id=8258, col_id=6
MAPTI dev_id=2 event_id=67 -> phys_id=8259, col_id=7
MAPTI dev_id=2 event_id=68 -> phys_id=8260, col_id=6
MAPTI dev_id=2 event_id=69 -> phys_id=8261, col_id=7
MAPTI dev_id=2 event_id=70 -> phys_id=8262, col_id=6
MAPTI dev_id=2 event_id=71 -> phys_id=8263, col_id=7
MAPTI dev_id=2 event_id=72 -> phys_id=8264, col_id=6
MAPTI dev_id=2 event_id=73 -> phys_id=8265, col_id=7
MAPTI dev_id=2 event_id=74 -> phys_id=8266, col_id=6
MAPTI dev_id=2 event_id=75 -> phys_id=8267, col_id=7
MAPTI dev_id=2 event_id=76 -> phys_id=8268, col_id=6
MAPTI dev_id=2 event_id=77 -> phys_id=8269, col_id=7
MAPTI dev_id=2 event_id=78 -> phys_id=8270, col_id=6
MAPTI dev_id=2 event_id=79 -> phys_id=8271, col_id=7
MAPTI dev_id=2 event_id=80 -> phys_id=8272, col_id=6
MAPTI dev_id=2 event_id=81 -> phys_id=8273, col_id=7
MAPTI dev_id=2 event_id=82 -> phys_id=8274, col_id=6
MAPTI dev_id=2 event_id=83 -> phys_id=8275, col_id=7
MAPTI dev_id=2 event_id=84 -> phys_id=8276, col_id=6
MAPTI dev_id=2 event_id=85 -> phys_id=8277, col_id=7
MAPTI dev_id=2 event_id=86 -> phys_id=8278, col_id=6
MAPTI dev_id=2 event_id=87 -> phys_id=8279, col_id=7
MAPTI dev_id=2 event_id=88 -> phys_id=8280, col_id=6
MAPTI dev_id=2 event_id=89 -> phys_id=8281, col_id=7
MAPTI dev_id=2 event_id=90 -> phys_id=8282, col_id=6
MAPTI dev_id=2 event_id=91 -> phys_id=8283, col_id=7
MAPTI dev_id=2 event_id=92 -> phys_id=8284, col_id=6
MAPTI dev_id=2 event_id=93 -> phys_id=8285, col_id=7
MAPTI dev_id=2 event_id=94 -> phys_id=8286, col_id=6
MAPTI dev_id=2 event_id=95 -> phys_id=8287, col_id=7
MAPTI dev_id=2 event_id=96 -> phys_id=8288, col_id=6
MAPTI dev_id=2 event_id=97 -> phys_id=8289, col_id=7
MAPTI dev_id=2 event_id=98 -> phys_id=8290, col_id=6
MAPTI dev_id=2 event_id=99 -> phys_id=8291, col_id=7
MAPTI dev_id=2 event_id=100 -> phys_id=8292, col_id=6
MAPTI dev_id=2 event_id=101 -> phys_id=8293, col_id=7
MAPTI dev_id=2 event_id=102 -> phys_id=8294, col_id=6
MAPTI dev_id=2 event_id=103 -> phys_id=8295, col_id=7
MAPTI dev_id=2 event_id=104 -> phys_id=8296, col_id=6
MAPTI dev_id=2 event_id=105 -> phys_id=8297, col_id=7
MAPTI dev_id=2 event_id=106 -> phys_id=8298, col_id=6
MAPTI dev_id=2 event_id=107 -> phys_id=8299, col_id=7
MAPTI dev_id=2 event_id=108 -> phys_id=8300, col_id=6
MAPTI dev_id=2 event_id=109 -> phys_id=8301, col_id=7
MAPTI dev_id=2 event_id=110 -> phys_id=8302, col_id=6
MAPTI dev_id=2 event_id=111 -> phys_id=8303, col_id=7
MAPTI dev_id=2 event_id=112 -> phys_id=8304, col_id=6
MAPTI dev_id=2 event_id=113 -> phys_id=8305, col_id=7
MAPTI dev_id=2 event_id=114 -> phys_id=8306, col_id=6
MAPTI dev_id=2 event_id=115 -> phys_id=8307, col_id=7
MAPTI dev_id=2 event_id=116 -> phys_id=8308, col_id=6
MAPTI dev_id=2 event_id=117 -> phys_id=8309, col_id=7
MAPTI dev_id=2 event_id=118 -> phys_id=8310, col_id=6
MAPTI dev_id=2 event_id=119 -> phys_id=8311, col_id=7
MAPTI dev_id=2 event_id=120 -> phys_id=8312, col_id=6
MAPTI dev_id=2 event_id=121 -> phys_id=8313, col_id=7
MAPTI dev_id=2 event_id=122 -> phys_id=8314, col_id=6
MAPTI dev_id=2 event_id=123 -> phys_id=8315, col_id=7
MAPTI dev_id=2 event_id=124 -> phys_id=8316, col_id=6
MAPTI dev_id=2 event_id=125 -> phys_id=8317, col_id=7
MAPTI dev_id=2 event_id=126 -> phys_id=8318, col_id=6
MAPTI dev_id=2 event_id=127 -> phys_id=8319, col_id=7
MAPTI dev_id=2 event_id=128 -> phys_id=8320, col_id=6
MAPTI dev_id=2 event_id=129 -> phys_id=8321, col_id=7
MAPTI dev_id=2 event_id=130 -> phys_id=8322, col_id=6
MAPTI dev_id=2 event_id=131 -> phys_id=8323, col_id=7
MAPTI dev_id=2 event_id=132 -> phys_id=8324, col_id=6
MAPTI dev_id=2 event_id=133 -> phys_id=8325, col_id=7
MAPTI dev_id=2 event_id=134 -> phys_id=8326, col_id=6
MAPTI dev_id=2 event_id=135 -> phys_id=8327, col_id=7
MAPTI dev_id=2 event_id=136 -> phys_id=8328, col_id=6
MAPTI dev_id=2 event_id=137 -> phys_id=8329, col_id=7
MAPTI dev_id=2 event_id=138 -> phys_id=8330, col_id=6
MAPTI dev_id=2 event_id=139 -> phys_id=8331, col_id=7
MAPTI dev_id=2 event_id=140 -> phys_id=8332, col_id=6
MAPTI dev_id=2 event_id=141 -> phys_id=8333, col_id=7
MAPTI dev_id=2 event_id=142 -> phys_id=8334, col_id=6
MAPTI dev_id=2 event_id=143 -> phys_id=8335, col_id=7
MAPTI dev_id=2 event_id=144 -> phys_id=8336, col_id=6
MAPTI dev_id=2 event_id=145 -> phys_id=8337, col_id=7
MAPTI dev_id=2 event_id=146 -> phys_id=8338, col_id=6
MAPTI dev_id=2 event_id=147 -> phys_id=8339, col_id=7
MAPTI dev_id=2 event_id=148 -> phys_id=8340, col_id=6
MAPTI dev_id=2 event_id=149 -> phys_id=8341, col_id=7
MAPTI dev_id=2 event_id=150 -> phys_id=8342, col_id=6
MAPTI dev_id=2 event_id=151 -> phys_id=8343, col_id=7
MAPTI dev_id=2 event_id=152 -> phys_id=8344, col_id=6
MAPTI dev_id=2 event_id=153 -> phys_id=8345, col_id=7
MAPTI dev_id=2 event_id=154 -> phys_id=8346, col_id=6
MAPTI dev_id=2 event_id=155 -> phys_id=8347, col_id=7
MAPTI dev_id=2 event_id=156 -> phys_id=8348, col_id=6
MAPTI dev_id=2 event_id=157 -> phys_id=8349, col_id=7
MAPTI dev_id=2 event_id=158 -> phys_id=8350, col_id=6
MAPTI dev_id=2 event_id=159 -> phys_id=8351, col_id=7
MAPTI dev_id=2 event_id=160 -> phys_id=8352, col_id=6
MAPTI dev_id=2 event_id=161 -> phys_id=8353, col_id=7
MAPTI dev_id=2 event_id=162 -> phys_id=8354, col_id=6
MAPTI dev_id=2 event_id=163 -> phys_id=8355, col_id=7
MAPTI dev_id=2 event_id=164 -> phys_id=8356, col_id=6
MAPTI dev_id=2 event_id=165 -> phys_id=8357, col_id=7
MAPTI dev_id=2 event_id=166 -> phys_id=8358, col_id=6
MAPTI dev_id=2 event_id=167 -> phys_id=8359, col_id=7
MAPTI dev_id=2 event_id=168 -> phys_id=8360, col_id=6
MAPTI dev_id=2 event_id=169 -> phys_id=8361, col_id=7
MAPTI dev_id=2 event_id=170 -> phys_id=8362, col_id=6
MAPTI dev_id=2 event_id=171 -> phys_id=8363, col_id=7
MAPTI dev_id=2 event_id=172 -> phys_id=8364, col_id=6
MAPTI dev_id=2 event_id=173 -> phys_id=8365, col_id=7
MAPTI dev_id=2 event_id=174 -> phys_id=8366, col_id=6
MAPTI dev_id=2 event_id=175 -> phys_id=8367, col_id=7
MAPTI dev_id=2 event_id=176 -> phys_id=8368, col_id=6
MAPTI dev_id=2 event_id=177 -> phys_id=8369, col_id=7
MAPTI dev_id=2 event_id=178 -> phys_id=8370, col_id=6
MAPTI dev_id=2 event_id=179 -> phys_id=8371, col_id=7
MAPTI dev_id=2 event_id=180 -> phys_id=8372, col_id=6
MAPTI dev_id=2 event_id=181 -> phys_id=8373, col_id=7
MAPTI dev_id=2 event_id=182 -> phys_id=8374, col_id=6
MAPTI dev_id=2 event_id=183 -> phys_id=8375, col_id=7
MAPTI dev_id=2 event_id=184 -> phys_id=8376, col_id=6
MAPTI dev_id=2 event_id=185 -> phys_id=8377, col_id=7
MAPTI dev_id=2 event_id=186 -> phys_id=8378, col_id=6
MAPTI dev_id=2 event_id=187 -> phys_id=8379, col_id=7
MAPTI dev_id=2 event_id=188 -> phys_id=8380, col_id=6
MAPTI dev_id=2 event_id=189 -> phys_id=8381, col_id=7
MAPTI dev_id=2 event_id=190 -> phys_id=8382, col_id=6
MAPTI dev_id=2 event_id=191 -> phys_id=8383, col_id=7
MAPTI dev_id=2 event_id=192 -> phys_id=8384, col_id=6
MAPTI dev_id=2 event_id=193 -> phys_id=8385, col_id=7
MAPTI dev_id=2 event_id=194 -> phys_id=8386, col_id=6
MAPTI dev_id=2 event_id=195 -> phys_id=8387, col_id=7
MAPTI dev_id=2 event_id=196 -> phys_id=8388, col_id=6
MAPTI dev_id=2 event_id=197 -> phys_id=8389, col_id=7
MAPTI dev_id=2 event_id=198 -> phys_id=8390, col_id=6
MAPTI dev_id=2 event_id=199 -> phys_id=8391, col_id=7
MAPTI dev_id=2 event_id=200 -> phys_id=8392, col_id=6
MAPTI dev_id=2 event_id=201 -> phys_id=8393, col_id=7
MAPTI dev_id=2 event_id=202 -> phys_id=8394, col_id=6
MAPTI dev_id=2 event_id=203 -> phys_id=8395, col_id=7
MAPTI dev_id=2 event_id=204 -> phys_id=8396, col_id=6
MAPTI dev_id=2 event_id=205 -> phys_id=8397, col_id=7
MAPTI dev_id=2 event_id=206 -> phys_id=8398, col_id=6
MAPTI dev_id=2 event_id=207 -> phys_id=8399, col_id=7
MAPTI dev_id=2 event_id=208 -> phys_id=8400, col_id=6
MAPTI dev_id=2 event_id=209 -> phys_id=8401, col_id=7
MAPTI dev_id=2 event_id=210 -> phys_id=8402, col_id=6
MAPTI dev_id=2 event_id=211 -> phys_id=8403, col_id=7
MAPTI dev_id=2 event_id=212 -> phys_id=8404, col_id=6
MAPTI dev_id=2 event_id=213 -> phys_id=8405, col_id=7
MAPTI dev_id=2 event_id=214 -> phys_id=8406, col_id=6
MAPTI dev_id=2 event_id=215 -> phys_id=8407, col_id=7
MAPTI dev_id=2 event_id=216 -> phys_id=8408, col_id=6
MAPTI dev_id=2 event_id=217 -> phys_id=8409, col_id=7
MAPTI dev_id=2 event_id=218 -> phys_id=8410, col_id=6
MAPTI dev_id=2 event_id=219 -> phys_id=8411, col_id=7
MAPTI dev_id=2 event_id=220 -> phys_id=8412, col_id=6
MAPTI dev_id=2 event_id=221 -> phys_id=8413, col_id=7
MAPTI dev_id=2 event_id=222 -> phys_id=8414, col_id=6
MAPTI dev_id=2 event_id=223 -> phys_id=8415, col_id=7
MAPTI dev_id=2 event_id=224 -> phys_id=8416, col_id=6
MAPTI dev_id=2 event_id=225 -> phys_id=8417, col_id=7
MAPTI dev_id=2 event_id=226 -> phys_id=8418, col_id=6
MAPTI dev_id=2 event_id=227 -> phys_id=8419, col_id=7
MAPTI dev_id=2 event_id=228 -> phys_id=8420, col_id=6
MAPTI dev_id=2 event_id=229 -> phys_id=8421, col_id=7
MAPTI dev_id=2 event_id=230 -> phys_id=8422, col_id=6
MAPTI dev_id=2 event_id=231 -> phys_id=8423, col_id=7
MAPTI dev_id=2 event_id=232 -> phys_id=8424, col_id=6
MAPTI dev_id=2 event_id=233 -> phys_id=8425, col_id=7
MAPTI dev_id=2 event_id=234 -> phys_id=8426, col_id=6
MAPTI dev_id=2 event_id=235 -> phys_id=8427, col_id=7
MAPTI dev_id=2 event_id=236 -> phys_id=8428, col_id=6
MAPTI dev_id=2 event_id=237 -> phys_id=8429, col_id=7
MAPTI dev_id=2 event_id=238 -> phys_id=8430, col_id=6
MAPTI dev_id=2 event_id=239 -> phys_id=8431, col_id=7
MAPTI dev_id=2 event_id=240 -> phys_id=8432, col_id=6
MAPTI dev_id=2 event_id=241 -> phys_id=8433, col_id=7
MAPTI dev_id=2 event_id=242 -> phys_id=8434, col_id=6
MAPTI dev_id=2 event_id=243 -> phys_id=8435, col_id=7
MAPTI dev_id=2 event_id=244 -> phys_id=8436, col_id=6
MAPTI dev_id=2 event_id=245 -> phys_id=8437, col_id=7
MAPTI dev_id=2 event_id=246 -> phys_id=8438, col_id=6
MAPTI dev_id=2 event_id=247 -> phys_id=8439, col_id=7
MAPTI dev_id=2 event_id=248 -> phys_id=8440, col_id=6
MAPTI dev_id=2 event_id=249 -> phys_id=8441, col_id=7
MAPTI dev_id=2 event_id=250 -> phys_id=8442, col_id=6
MAPTI dev_id=2 event_id=251 -> phys_id=8443, col_id=7
MAPTI dev_id=2 event_id=252 -> phys_id=8444, col_id=6
MAPTI dev_id=2 event_id=253 -> phys_id=8445, col_id=7
MAPTI dev_id=2 event_id=254 -> phys_id=8446, col_id=6
MAPTI dev_id=2 event_id=255 -> phys_id=8447, col_id=7
INVALL col_id=7
INVALL col_id=6
Now migrate the VM, then press a key to continue...
INFO: gicv3: its-pending-migration: Migration complete
INFO: gicv3: its-pending-migration: expected 128 LPIs on PE #6, 0 observed
FAIL: gicv3: its-pending-migration: 128 LPIs on both PE0 and PE1 after migration
SUMMARY: 1 tests, 1 unexpected failures

timeout -k 1s --foreground 90s /home/alex/lsrc/qemu.git/builds/arm.all/qemu-system-aarch64 -nodefaults -machine virt,accel=tcg -cpu cortex-a57 -device virtio-serial-device -device virtconsole,chardev=ctd -chardev testdev,id=ctd -device pci-testdev -display none -serial stdio -kernel arm/gic.flat -smp 8 -machine gic-version=3 -append its-trigger # -initrd /tmp/tmp.shXaNoBX3p
ITS: MAPD devid=2 size = 0x8 itt=0x40440000 valid=1
ITS: MAPD devid=7 size = 0x8 itt=0x40450000 valid=1
MAPC col_id=3 target_addr = 0x30000 valid=1
MAPC col_id=2 target_addr = 0x20000 valid=1
INVALL col_id=2
INVALL col_id=3
MAPTI dev_id=2 event_id=20 -> phys_id=8195, col_id=3
MAPTI dev_id=7 event_id=255 -> phys_id=8196, col_id=2
INT dev_id=2 event_id=20
PASS: gicv3: its-trigger: int: dev=2, eventid=20  -> lpi= 8195, col=3
INT dev_id=7 event_id=255
PASS: gicv3: its-trigger: int: dev=7, eventid=255 -> lpi= 8196, col=2
INV dev_id=2 event_id=20
INT dev_id=2 event_id=20
PASS: gicv3: its-trigger: inv/invall: dev2/eventid=20 does not trigger any LPI
INT dev_id=2 event_id=20
INFO: gicv3: its-trigger: inv/invall: Unexpected LPI (cpuid=3, intid=8195)
FAIL: gicv3: its-trigger: inv/invall: dev2/eventid=20 still does not trigger any LPI
INVALL col_id=3
INT dev_id=2 event_id=20
PASS: gicv3: its-trigger: inv/invall: dev2/eventid=20 now triggers an LPI
ITS: MAPD devid=2 size = 0x8 itt=0x40440000 valid=0
INT dev_id=2 event_id=20
PASS: gicv3: its-trigger: mapd valid=false: no LPI after device unmap
SUMMARY: 6 tests, 1 unexpected failures

BUILD_HEAD=3054ca26

[-- Attachment #3: Type: text/plain, Size: 23 bytes --]



-- 
Alex Bennée

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

* Re: [PATCH v1 2/8] hw/intc: GICv3 ITS register definitions added
  2021-03-23  4:12 ` [PATCH v1 2/8] hw/intc: GICv3 ITS register definitions added Shashi Mallela
@ 2021-03-25 19:34   ` Alex Bennée
  2021-03-31 16:48     ` shashi.mallela
  0 siblings, 1 reply; 16+ messages in thread
From: Alex Bennée @ 2021-03-25 19:34 UTC (permalink / raw)
  To: Shashi Mallela; +Cc: peter.maydell, leif, qemu-devel, qemu-arm, rad


Shashi Mallela <shashi.mallela@linaro.org> writes:

> Defined descriptors for ITS device table,collection table and ITS
> command queue entities.Implemented register read/write functions,
> extract ITS table parameters and command queue parameters,extended
> gicv3 common to capture qemu address space(which host the ITS table
> platform memories required for subsequent ITS processing) and
> initialize the same in its device.
>
> Signed-off-by: Shashi Mallela <shashi.mallela@linaro.org>
> ---
>  hw/intc/arm_gicv3_its.c            | 356 ++++++++++++++++++++
>  include/hw/intc/arm_gicv3_common.h |   4 +
>  2 files changed, 360 insertions(+)
>
> diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c
> index 34e49b4d63..4895d32e67 100644
> --- a/hw/intc/arm_gicv3_its.c
> +++ b/hw/intc/arm_gicv3_its.c
> @@ -23,11 +23,179 @@ typedef struct GICv3ITSClass GICv3ITSClass;
>  DECLARE_OBJ_CHECKERS(GICv3ITSState, GICv3ITSClass,
>                       ARM_GICV3_ITS, TYPE_ARM_GICV3_ITS)
>  
> +typedef struct {
> +    bool valid;
> +    bool indirect;
> +    uint16_t entry_sz;
> +    uint32_t max_entries;
> +    uint32_t max_devids;
> +    uint64_t base_addr;
> +} DevTableDesc;
> +
> +typedef struct {
> +    bool valid;
> +    bool indirect;
> +    uint16_t entry_sz;
> +    uint32_t max_entries;
> +    uint32_t max_collids;
> +    uint64_t base_addr;
> +} CollTableDesc;
> +
> +typedef struct {
> +    bool valid;
> +    uint32_t max_entries;
> +    uint64_t base_addr;
> +} CmdQDesc;
> +
>  struct GICv3ITSClass {
>      GICv3ITSCommonClass parent_class;
>      void (*parent_reset)(DeviceState *dev);
> +
> +    DevTableDesc  dt;
> +    CollTableDesc ct;
> +    CmdQDesc      cq;
>  };
>  
> +static bool extract_table_params(GICv3ITSState *s, int index)
> +{
> +    GICv3ITSClass *c = ARM_GICV3_ITS_GET_CLASS(s);
> +    uint16_t num_pages = 0;
> +    uint8_t  page_sz_type;
> +    uint8_t type;
> +    uint32_t page_sz = 0;
> +    uint64_t value = s->baser[index];
> +
> +    num_pages = (value & GITS_BASER_SIZE);
> +    page_sz_type = (value >> GITS_BASER_PAGESIZE_OFFSET) &
> +                        GITS_BASER_PAGESIZE_MASK;

So these definitions can come in the header on the same patch where they
are used. Also with the FIELD macros this then becomes:

  num_pages = FIELD_EX64(value, GITS_BASER, SIZE);
  page_sz_type = FIELD_EX64(value, GITS_BASER, PAGESIZE);

etc...

basically anywhere where you are shift and masking you should be using
the FIELD macros (or at the least the extract/deposit functions).

> +
> +    if (page_sz_type == 0) {
> +        page_sz = GITS_ITT_PAGE_SIZE_0;
> +    } else if (page_sz_type == 0) {
> +        page_sz = GITS_ITT_PAGE_SIZE_1;
> +    } else if (page_sz_type == 2) {
> +        page_sz = GITS_ITT_PAGE_SIZE_2;
> +    } else {
> +        return false;
> +    }
> +
> +    type = (value >> GITS_BASER_TYPE_OFFSET) &
> +                        GITS_BASER_TYPE_MASK;
> +
> +    if (type == GITS_ITT_TYPE_DEVICE) {
> +        c->dt.valid = (value >> GITS_BASER_VALID) & GITS_BASER_VALID_MASK;
> +
> +        if (c->dt.valid) {
> +            c->dt.indirect = (value >> GITS_BASER_INDIRECT_OFFSET) &
> +                                       GITS_BASER_INDIRECT_MASK;
> +            c->dt.entry_sz = (value >> GITS_BASER_ENTRYSIZE_OFFSET) &
> +                                   GITS_BASER_ENTRYSIZE_MASK;
> +
> +            if (!c->dt.indirect) {
> +                c->dt.max_entries = ((num_pages + 1) * page_sz) /
> +                                                       c->dt.entry_sz;
> +            } else {
> +                c->dt.max_entries = ((((num_pages + 1) * page_sz) /
> +                                        L1TABLE_ENTRY_SIZE) *
> +                                    (page_sz / c->dt.entry_sz));
> +            }
> +
> +            c->dt.max_devids = (1UL << (((value >> GITS_TYPER_DEVBITS_OFFSET) &
> +                                           GITS_TYPER_DEVBITS_MASK) + 1));
> +
> +            if ((page_sz == GITS_ITT_PAGE_SIZE_0) ||
> +                    (page_sz == GITS_ITT_PAGE_SIZE_1)) {
> +                c->dt.base_addr = (value >> GITS_BASER_PHYADDR_OFFSET) &
> +                                        GITS_BASER_PHYADDR_MASK;
> +                c->dt.base_addr <<= GITS_BASER_PHYADDR_OFFSET;
> +            } else if (page_sz == GITS_ITT_PAGE_SIZE_2) {
> +                c->dt.base_addr = ((value >> GITS_BASER_PHYADDR_OFFSETL_64K) &
> +                                   GITS_BASER_PHYADDR_MASKL_64K) <<
> +                                     GITS_BASER_PHYADDR_OFFSETL_64K;
> +                c->dt.base_addr |= ((value >> GITS_BASER_PHYADDR_OFFSET) &
> +                                    GITS_BASER_PHYADDR_MASKH_64K) <<
> +                                     GITS_BASER_PHYADDR_OFFSETH_64K;
> +            }
> +        }
> +    } else if (type == GITS_ITT_TYPE_COLLECTION) {
> +        c->ct.valid = (value >> GITS_BASER_VALID) & GITS_BASER_VALID_MASK;
> +
> +        /*
> +         * GITS_TYPER.HCC is 0 for this implementation
> +         * hence writes are discarded if ct.valid is 0
> +         */
> +        if (c->ct.valid) {
> +            c->ct.indirect = (value >> GITS_BASER_INDIRECT_OFFSET) &
> +                                       GITS_BASER_INDIRECT_MASK;
> +            c->ct.entry_sz = (value >> GITS_BASER_ENTRYSIZE_OFFSET) &
> +                                    GITS_BASER_ENTRYSIZE_MASK;
> +
> +            if (!c->ct.indirect) {
> +                c->ct.max_entries = ((num_pages + 1) * page_sz) /
> +                                      c->ct.entry_sz;
> +            } else {
> +                c->ct.max_entries = ((((num_pages + 1) * page_sz) /
> +                                      L1TABLE_ENTRY_SIZE) *
> +                                      (page_sz / c->ct.entry_sz));
> +            }
> +
> +            if ((value >> GITS_TYPER_CIL_OFFSET) & GITS_TYPER_CIL_MASK) {
> +                c->ct.max_collids = (1UL << (((value >>
> +                                               GITS_TYPER_CIDBITS_OFFSET) &
> +                                               GITS_TYPER_CIDBITS_MASK) + 1));
> +            } else {
> +                /* 16-bit CollectionId supported when CIL == 0 */
> +                c->ct.max_collids = (1UL << 16);
> +            }
> +
> +            if ((page_sz == GITS_ITT_PAGE_SIZE_0) ||
> +                 (page_sz == GITS_ITT_PAGE_SIZE_1)) {
> +                c->ct.base_addr = (value >> GITS_BASER_PHYADDR_OFFSET) &
> +                                            GITS_BASER_PHYADDR_MASK;
> +                c->ct.base_addr <<= GITS_BASER_PHYADDR_OFFSET;
> +            } else if (page_sz == GITS_ITT_PAGE_SIZE_2) {
> +                c->ct.base_addr = ((value >> GITS_BASER_PHYADDR_OFFSETL_64K) &
> +                                   GITS_BASER_PHYADDR_MASKL_64K) <<
> +                                    GITS_BASER_PHYADDR_OFFSETL_64K;
> +                c->ct.base_addr |= ((value >> GITS_BASER_PHYADDR_OFFSET) &
> +                                    GITS_BASER_PHYADDR_MASKH_64K) <<
> +                                    GITS_BASER_PHYADDR_OFFSETH_64K;
> +            }
> +        }
> +    } else {
> +        /* unsupported ITS table type */
> +        qemu_log_mask(LOG_GUEST_ERROR, "%s: Unsupported ITS table type %d",
> +                         __func__, type);

LOG_UNIMP

> +        return false;
> +    }
> +    return true;
> +}
> +
> +static bool extract_cmdq_params(GICv3ITSState *s)
> +{
> +    GICv3ITSClass *c = ARM_GICV3_ITS_GET_CLASS(s);
> +    uint16_t num_pages = 0;
> +    uint64_t value = s->cbaser;
> +
> +    num_pages = (value & GITS_CBASER_SIZE);
> +
> +    c->cq.valid = (value >> GITS_CBASER_VALID_OFFSET) &
> +                                GITS_CBASER_VALID_MASK;
> +
> +    if (!num_pages || !c->cq.valid) {
> +        return false;
> +    }
> +
> +    if (c->cq.valid) {
> +        c->cq.max_entries = ((num_pages + 1) * GITS_ITT_PAGE_SIZE_0) /
> +                                                GITS_CMDQ_ENTRY_SIZE;
> +        c->cq.base_addr = (value >> GITS_CBASER_PHYADDR_OFFSET) &
> +                                    GITS_CBASER_PHYADDR_MASK;
> +        c->cq.base_addr <<= GITS_CBASER_PHYADDR_OFFSET;
> +    }
> +    return true;
> +}
> +
>  static MemTxResult its_trans_writew(GICv3ITSState *s, hwaddr offset,
>                                 uint64_t value, MemTxAttrs attrs)
>  {
> @@ -126,7 +294,75 @@ static MemTxResult its_writel(GICv3ITSState *s, hwaddr offset,
>                                 uint64_t value, MemTxAttrs attrs)
>  {
>      MemTxResult result = MEMTX_OK;
> +    int index;
> +    uint64_t temp = 0;
>  
> +    switch (offset) {
> +    case GITS_CTLR:
> +        s->ctlr |= (value & ~(s->ctlr));
> +        break;
> +    case GITS_CBASER:
> +        /* GITS_CBASER register becomes RO if ITS is already enabled */
> +        if (!(s->ctlr & GITS_CTLR_ENABLED)) {
> +            s->cbaser = deposit64(s->cbaser, 0, 32, value);
> +            s->creadr = 0;
> +        }
> +        break;
> +    case GITS_CBASER + 4:
> +        /* GITS_CBASER register becomes RO if ITS is already enabled */
> +        if (!(s->ctlr & GITS_CTLR_ENABLED)) {
> +            s->cbaser = deposit64(s->cbaser, 32, 32, value);
> +            if (!extract_cmdq_params(s)) {
> +                qemu_log_mask(LOG_GUEST_ERROR,
> +                       "%s: error extracting GITS_CBASER parameters "
> +                       TARGET_FMT_plx "\n", __func__, offset);
> +                s->cbaser = 0;
> +                result = MEMTX_ERROR;

LOG_UNIMP?

> +            } else {
> +                s->creadr = 0;
> +            }
> +        }
> +        break;
> +    case GITS_CWRITER:
> +        s->cwriter = deposit64(s->cwriter, 0, 32, value);
> +        break;
> +    case GITS_CWRITER + 4:
> +        s->cwriter = deposit64(s->cwriter, 32, 32, value);
> +        break;
> +    case GITS_BASER ... GITS_BASER + 0x3f:
> +        /* GITS_BASERn registers become RO if ITS is already enabled */
> +        if (!(s->ctlr & GITS_CTLR_ENABLED)) {
> +            index = (offset - GITS_BASER) / 8;
> +
> +            if (offset & 7) {
> +                temp = s->baser[index];
> +                temp = deposit64(temp, 32, 32, (value & ~GITS_BASER_VAL_MASK));
> +                s->baser[index] |= temp;
> +
> +                if (!extract_table_params(s, index)) {
> +                    qemu_log_mask(LOG_GUEST_ERROR,
> +                        "%s: error extracting GITS_BASER parameters "
> +                        TARGET_FMT_plx "\n", __func__, offset);

LOG_UNIMP again? 

> +                    s->baser[index] = 0;
> +                    result = MEMTX_ERROR;
> +                }
> +            } else {
> +                s->baser[index] =  deposit64(s->baser[index], 0, 32, value);
> +            }
> +        }
> +        break;
> +    case GITS_IIDR:
> +    case GITS_TYPER:
> +    case GITS_CREADR:
> +        /* RO registers, ignore the write */
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +            "%s: invalid guest write to RO register at offset "
> +            TARGET_FMT_plx "\n", __func__, offset);
> +        break;
> +    default:
> +        result = MEMTX_ERROR;
> +        break;
> +    }
>      return result;
>  }
>  
> @@ -134,7 +370,54 @@ static MemTxResult its_readl(GICv3ITSState *s, hwaddr offset,
>                                 uint64_t *data, MemTxAttrs attrs)
>  {
>      MemTxResult result = MEMTX_OK;
> +    int index;
>  
> +    switch (offset) {
> +    case GITS_CTLR:
> +        *data = s->ctlr;
> +        break;
> +    case GITS_IIDR:
> +        *data = s->iidr;
> +        break;
> +    case GITS_PIDR2:
> +        *data = 0x30; /* GICv3 */
> +        break;
> +    case GITS_TYPER:
> +        *data = extract64(s->typer, 0, 32);
> +        break;
> +    case GITS_TYPER + 4:
> +        *data = extract64(s->typer, 32, 32);
> +        break;
> +    case GITS_CBASER:
> +        *data = extract64(s->cbaser, 0, 32);
> +        break;
> +    case GITS_CBASER + 4:
> +        *data = extract64(s->cbaser, 32, 32);
> +        break;
> +    case GITS_CREADR:
> +        *data = extract64(s->creadr, 0, 32);
> +        break;
> +    case GITS_CREADR + 4:
> +        *data = extract64(s->creadr, 32, 32);
> +        break;
> +    case GITS_CWRITER:
> +        *data = extract64(s->cwriter, 0, 32);
> +        break;
> +    case GITS_CWRITER + 4:
> +        *data = extract64(s->cwriter, 32, 32);
> +        break;
> +    case GITS_BASER ... GITS_BASER + 0x3f:
> +        index = (offset - GITS_BASER) / 8;
> +        if (offset & 7) {
> +            *data = s->baser[index] >> 32;
> +        } else {
> +            *data = (uint32_t)s->baser[index];
> +        }
> +        break;
> +    default:
> +        result = MEMTX_ERROR;
> +        break;
> +    }
>      return result;
>  }
>  
> @@ -142,7 +425,52 @@ static MemTxResult its_writell(GICv3ITSState *s, hwaddr offset,
>                                 uint64_t value, MemTxAttrs attrs)
>  {
>      MemTxResult result = MEMTX_OK;
> +    int index;
>  
> +    switch (offset) {
> +    case GITS_BASER ... GITS_BASER + 0x3f:
> +        /* GITS_BASERn registers become RO if ITS is already enabled */
> +        if (!(s->ctlr & GITS_CTLR_ENABLED)) {
> +            index = (offset - GITS_BASER) / 8;
> +            s->baser[index] |= (value & ~GITS_BASER_VAL_MASK);
> +            if (!extract_table_params(s, index)) {
> +                qemu_log_mask(LOG_GUEST_ERROR,
> +                        "%s: error extracting GITS_BASER parameters "
> +                        TARGET_FMT_plx "\n", __func__, offset);
> +                s->baser[index] = 0;
> +                result = MEMTX_ERROR;
> +            }
> +        }
> +        break;
> +    case GITS_CBASER:
> +        /* GITS_CBASER register becomes RO if ITS is already enabled */
> +        if (!(s->ctlr & GITS_CTLR_ENABLED)) {
> +            s->cbaser = value;
> +            if (!extract_cmdq_params(s)) {
> +                qemu_log_mask(LOG_GUEST_ERROR,
> +                       "%s: error extracting GITS_CBASER parameters "
> +                       TARGET_FMT_plx "\n", __func__, offset);

So are these all LOG_UNIMP or are they a programming failure on our part?

> +                s->cbaser = 0;
> +                result = MEMTX_ERROR;
> +            } else {
> +                s->creadr = 0;
> +            }
> +        }
> +        break;
> +    case GITS_CWRITER:
> +        s->cwriter = value;
> +        break;
> +    case GITS_TYPER:
> +    case GITS_CREADR:
> +        /* RO register, ignore the write */
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "%s: invalid guest write to RO register at offset "
> +                      TARGET_FMT_plx "\n", __func__, offset);
> +        break;
> +    default:
> +        result = MEMTX_ERROR;
> +        break;
> +    }
>      return result;
>  }
>  
> @@ -150,7 +478,29 @@ static MemTxResult its_readll(GICv3ITSState *s, hwaddr offset,
>                                 uint64_t *data, MemTxAttrs attrs)
>  {
>      MemTxResult result = MEMTX_OK;
> +    int index;
>  
> +    switch (offset) {
> +    case GITS_TYPER:
> +        *data = s->typer;
> +        break;
> +    case GITS_BASER ... GITS_BASER + 0x3f:
> +        index = (offset - GITS_BASER) / 8;
> +        *data = s->baser[index];
> +        break;
> +    case GITS_CBASER:
> +        *data = s->cbaser;
> +        break;
> +    case GITS_CREADR:
> +        *data = s->creadr;
> +        break;
> +    case GITS_CWRITER:
> +        *data = s->cwriter;
> +        break;
> +    default:
> +        result = MEMTX_ERROR;
> +        break;
> +    }
>      return result;
>  }
>  
> @@ -250,6 +600,9 @@ static void gicv3_arm_its_realize(DeviceState *dev, Error **errp)
>      GICv3ITSState *s = ARM_GICV3_ITS_COMMON(dev);
>  
>      gicv3_its_init_mmio(s, &gicv3_its_control_ops, &gicv3_its_trans_ops);
> +
> +    address_space_init(&s->gicv3->sysmem_as, s->gicv3->sysmem,
> +                        "gicv3-its-sysmem");
>  }
>  
>  static void gicv3_its_reset(DeviceState *dev)
> @@ -259,6 +612,9 @@ static void gicv3_its_reset(DeviceState *dev)
>  
>      if (s->gicv3->cpu->gicr_typer & GICR_TYPER_PLPIS) {
>          c->parent_reset(dev);
> +        memset(&c->dt, 0 , sizeof(c->dt));
> +        memset(&c->ct, 0 , sizeof(c->ct));
> +        memset(&c->cq, 0 , sizeof(c->cq));
>  
>          /* set the ITS default features supported */
>          s->typer = GITS_TYPER_PHYSICAL | (GITS_TYPER_ITT_ENTRY_SIZE <<
> diff --git a/include/hw/intc/arm_gicv3_common.h b/include/hw/intc/arm_gicv3_common.h
> index 91491a2f66..b0f2414fa3 100644
> --- a/include/hw/intc/arm_gicv3_common.h
> +++ b/include/hw/intc/arm_gicv3_common.h
> @@ -226,12 +226,16 @@ struct GICv3State {
>      int dev_fd; /* kvm device fd if backed by kvm vgic support */
>      Error *migration_blocker;
>  
> +    MemoryRegion *sysmem;
> +    AddressSpace sysmem_as;
> +
>      /* Distributor */
>  
>      /* for a GIC with the security extensions the NS banked version of this
>       * register is just an alias of bit 1 of the S banked version.
>       */
>      uint32_t gicd_ctlr;
> +    uint32_t gicd_typer;
>      uint32_t gicd_statusr[2];
>      GIC_DECLARE_BITMAP(group);        /* GICD_IGROUPR */
>      GIC_DECLARE_BITMAP(grpmod);       /* GICD_IGRPMODR */


-- 
Alex Bennée


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

* Re: [PATCH v1 0/8] GICv3 LPI and ITS feature implementation
  2021-03-23  4:12 [PATCH v1 0/8] GICv3 LPI and ITS feature implementation Shashi Mallela
                   ` (8 preceding siblings ...)
  2021-03-25 17:59 ` [PATCH v1 0/8] GICv3 LPI and ITS feature implementation Alex Bennée
@ 2021-03-25 19:44 ` Alex Bennée
  9 siblings, 0 replies; 16+ messages in thread
From: Alex Bennée @ 2021-03-25 19:44 UTC (permalink / raw)
  To: Shashi Mallela; +Cc: peter.maydell, leif, qemu-devel, qemu-arm, rad


Shashi Mallela <shashi.mallela@linaro.org> writes:

> This patchset implements qemu device model for enabling physical
> LPI support and ITS functionality in GIC as per GICv3 specification.
> Both flat table and 2 level tables are implemented.The ITS commands
> for adding/deleting ITS table entries,trigerring LPI interrupts are 
> implemented.Translated LPI interrupt ids are processed by redistributor
> to determine priority and set pending state appropriately before
> forwarding the same to cpu interface.
> The ITS feature support has been added to sbsa-ref platform as well as
> virt platform,wherein the emulated functionality co-exists with kvm
> kernel functionality.

OK I've finished my first pass. I didn't want to keep repeating myself
with the later patches. So in summary:

  - use REG/FIELD to avoid the manual mask definition/manipulation
  - define registers in the patch that first uses them
  - check LOG_UNIMP vs LOG_GUEST_ERROR (and possibly assert if it
    shouldn't happen)
  - and of course pass the kvm-unit-tests ITS tests ;-) 

Thanks,

>
> Shashi Mallela (8):
>   hw/intc: GICv3 ITS initial framework
>   hw/intc: GICv3 ITS register definitions added
>   hw/intc: GICv3 ITS command queue framework
>   hw/intc: GICv3 ITS Command processing
>   hw/intc: GICv3 ITS Feature enablement
>   hw/intc: GICv3 redistributor ITS processing
>   hw/arm/sbsa-ref: add ITS support in SBSA GIC
>   hw/arm/virt: add ITS support in virt GIC
>
>  hw/arm/sbsa-ref.c                      |   26 +-
>  hw/arm/virt.c                          |   10 +-
>  hw/intc/arm_gicv3.c                    |    6 +
>  hw/intc/arm_gicv3_common.c             |   16 +
>  hw/intc/arm_gicv3_cpuif.c              |   15 +-
>  hw/intc/arm_gicv3_dist.c               |   22 +-
>  hw/intc/arm_gicv3_its.c                | 1417 ++++++++++++++++++++
>  hw/intc/arm_gicv3_its_common.c         |   17 +-
>  hw/intc/arm_gicv3_its_kvm.c            |    2 +-
>  hw/intc/arm_gicv3_redist.c             |  155 ++-
>  hw/intc/gicv3_internal.h               |  176 ++-
>  hw/intc/meson.build                    |    1 +
>  include/hw/intc/arm_gicv3_common.h     |   14 +
>  include/hw/intc/arm_gicv3_its_common.h |   12 +-
>  target/arm/kvm_arm.h                   |    4 +-
>  15 files changed, 1869 insertions(+), 24 deletions(-)
>  create mode 100644 hw/intc/arm_gicv3_its.c


-- 
Alex Bennée


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

* Re: [PATCH v1 2/8] hw/intc: GICv3 ITS register definitions added
  2021-03-25 19:34   ` Alex Bennée
@ 2021-03-31 16:48     ` shashi.mallela
  2021-03-31 23:31       ` Richard Henderson
  0 siblings, 1 reply; 16+ messages in thread
From: shashi.mallela @ 2021-03-31 16:48 UTC (permalink / raw)
  To: Alex Bennée; +Cc: peter.maydell, leif, qemu-devel, qemu-arm, rad

On Thu, 2021-03-25 at 19:34 +0000, Alex Bennée wrote:
> Shashi Mallela <shashi.mallela@linaro.org> writes:
> 
> > Defined descriptors for ITS device table,collection table and ITS
> > command queue entities.Implemented register read/write functions,
> > extract ITS table parameters and command queue parameters,extended
> > gicv3 common to capture qemu address space(which host the ITS table
> > platform memories required for subsequent ITS processing) and
> > initialize the same in its device.
> > 
> > Signed-off-by: Shashi Mallela <shashi.mallela@linaro.org>
> > ---
> >  hw/intc/arm_gicv3_its.c            | 356 ++++++++++++++++++++
> >  include/hw/intc/arm_gicv3_common.h |   4 +
> >  2 files changed, 360 insertions(+)
> > 
> > diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c
> > index 34e49b4d63..4895d32e67 100644
> > --- a/hw/intc/arm_gicv3_its.c
> > +++ b/hw/intc/arm_gicv3_its.c
> > @@ -23,11 +23,179 @@ typedef struct GICv3ITSClass GICv3ITSClass;
> >  DECLARE_OBJ_CHECKERS(GICv3ITSState, GICv3ITSClass,
> >                       ARM_GICV3_ITS, TYPE_ARM_GICV3_ITS)
> >  
> > +typedef struct {
> > +    bool valid;
> > +    bool indirect;
> > +    uint16_t entry_sz;
> > +    uint32_t max_entries;
> > +    uint32_t max_devids;
> > +    uint64_t base_addr;
> > +} DevTableDesc;
> > +
> > +typedef struct {
> > +    bool valid;
> > +    bool indirect;
> > +    uint16_t entry_sz;
> > +    uint32_t max_entries;
> > +    uint32_t max_collids;
> > +    uint64_t base_addr;
> > +} CollTableDesc;
> > +
> > +typedef struct {
> > +    bool valid;
> > +    uint32_t max_entries;
> > +    uint64_t base_addr;
> > +} CmdQDesc;
> > +
> >  struct GICv3ITSClass {
> >      GICv3ITSCommonClass parent_class;
> >      void (*parent_reset)(DeviceState *dev);
> > +
> > +    DevTableDesc  dt;
> > +    CollTableDesc ct;
> > +    CmdQDesc      cq;
> >  };
> >  
> > +static bool extract_table_params(GICv3ITSState *s, int index)
> > +{
> > +    GICv3ITSClass *c = ARM_GICV3_ITS_GET_CLASS(s);
> > +    uint16_t num_pages = 0;
> > +    uint8_t  page_sz_type;
> > +    uint8_t type;
> > +    uint32_t page_sz = 0;
> > +    uint64_t value = s->baser[index];
> > +
> > +    num_pages = (value & GITS_BASER_SIZE);
> > +    page_sz_type = (value >> GITS_BASER_PAGESIZE_OFFSET) &
> > +                        GITS_BASER_PAGESIZE_MASK;
> 
> So these definitions can come in the header on the same patch where
> they
> are used. Also with the FIELD macros this then becomes:
> 
>   num_pages = FIELD_EX64(value, GITS_BASER, SIZE);
>   page_sz_type = FIELD_EX64(value, GITS_BASER, PAGESIZE);
> 
> etc...
> 
> basically anywhere where you are shift and masking you should be
> using
> the FIELD macros (or at the least the extract/deposit functions).
> 
> > +
> > +    if (page_sz_type == 0) {
> > +        page_sz = GITS_ITT_PAGE_SIZE_0;
> > +    } else if (page_sz_type == 0) {
> > +        page_sz = GITS_ITT_PAGE_SIZE_1;
> > +    } else if (page_sz_type == 2) {
> > +        page_sz = GITS_ITT_PAGE_SIZE_2;
> > +    } else {
> > +        return false;
> > +    }
> > +
> > +    type = (value >> GITS_BASER_TYPE_OFFSET) &
> > +                        GITS_BASER_TYPE_MASK;
> > +
> > +    if (type == GITS_ITT_TYPE_DEVICE) {
> > +        c->dt.valid = (value >> GITS_BASER_VALID) &
> > GITS_BASER_VALID_MASK;
> > +
> > +        if (c->dt.valid) {
> > +            c->dt.indirect = (value >> GITS_BASER_INDIRECT_OFFSET)
> > &
> > +                                       GITS_BASER_INDIRECT_MASK;
> > +            c->dt.entry_sz = (value >>
> > GITS_BASER_ENTRYSIZE_OFFSET) &
> > +                                   GITS_BASER_ENTRYSIZE_MASK;
> > +
> > +            if (!c->dt.indirect) {
> > +                c->dt.max_entries = ((num_pages + 1) * page_sz) /
> > +                                                       c-
> > >dt.entry_sz;
> > +            } else {
> > +                c->dt.max_entries = ((((num_pages + 1) * page_sz)
> > /
> > +                                        L1TABLE_ENTRY_SIZE) *
> > +                                    (page_sz / c->dt.entry_sz));
> > +            }
> > +
> > +            c->dt.max_devids = (1UL << (((value >>
> > GITS_TYPER_DEVBITS_OFFSET) &
> > +                                           GITS_TYPER_DEVBITS_MASK
> > ) + 1));
> > +
> > +            if ((page_sz == GITS_ITT_PAGE_SIZE_0) ||
> > +                    (page_sz == GITS_ITT_PAGE_SIZE_1)) {
> > +                c->dt.base_addr = (value >>
> > GITS_BASER_PHYADDR_OFFSET) &
> > +                                        GITS_BASER_PHYADDR_MASK;
> > +                c->dt.base_addr <<= GITS_BASER_PHYADDR_OFFSET;
> > +            } else if (page_sz == GITS_ITT_PAGE_SIZE_2) {
> > +                c->dt.base_addr = ((value >>
> > GITS_BASER_PHYADDR_OFFSETL_64K) &
> > +                                   GITS_BASER_PHYADDR_MASKL_64K)
> > <<
> > +                                     GITS_BASER_PHYADDR_OFFSETL_64
> > K;
> > +                c->dt.base_addr |= ((value >>
> > GITS_BASER_PHYADDR_OFFSET) &
> > +                                    GITS_BASER_PHYADDR_MASKH_64K)
> > <<
> > +                                     GITS_BASER_PHYADDR_OFFSETH_64
> > K;
> > +            }
> > +        }
> > +    } else if (type == GITS_ITT_TYPE_COLLECTION) {
> > +        c->ct.valid = (value >> GITS_BASER_VALID) &
> > GITS_BASER_VALID_MASK;
> > +
> > +        /*
> > +         * GITS_TYPER.HCC is 0 for this implementation
> > +         * hence writes are discarded if ct.valid is 0
> > +         */
> > +        if (c->ct.valid) {
> > +            c->ct.indirect = (value >> GITS_BASER_INDIRECT_OFFSET)
> > &
> > +                                       GITS_BASER_INDIRECT_MASK;
> > +            c->ct.entry_sz = (value >>
> > GITS_BASER_ENTRYSIZE_OFFSET) &
> > +                                    GITS_BASER_ENTRYSIZE_MASK;
> > +
> > +            if (!c->ct.indirect) {
> > +                c->ct.max_entries = ((num_pages + 1) * page_sz) /
> > +                                      c->ct.entry_sz;
> > +            } else {
> > +                c->ct.max_entries = ((((num_pages + 1) * page_sz)
> > /
> > +                                      L1TABLE_ENTRY_SIZE) *
> > +                                      (page_sz / c->ct.entry_sz));
> > +            }
> > +
> > +            if ((value >> GITS_TYPER_CIL_OFFSET) &
> > GITS_TYPER_CIL_MASK) {
> > +                c->ct.max_collids = (1UL << (((value >>
> > +                                               GITS_TYPER_CIDBITS_
> > OFFSET) &
> > +                                               GITS_TYPER_CIDBITS_
> > MASK) + 1));
> > +            } else {
> > +                /* 16-bit CollectionId supported when CIL == 0 */
> > +                c->ct.max_collids = (1UL << 16);
> > +            }
> > +
> > +            if ((page_sz == GITS_ITT_PAGE_SIZE_0) ||
> > +                 (page_sz == GITS_ITT_PAGE_SIZE_1)) {
> > +                c->ct.base_addr = (value >>
> > GITS_BASER_PHYADDR_OFFSET) &
> > +                                            GITS_BASER_PHYADDR_MAS
> > K;
> > +                c->ct.base_addr <<= GITS_BASER_PHYADDR_OFFSET;
> > +            } else if (page_sz == GITS_ITT_PAGE_SIZE_2) {
> > +                c->ct.base_addr = ((value >>
> > GITS_BASER_PHYADDR_OFFSETL_64K) &
> > +                                   GITS_BASER_PHYADDR_MASKL_64K)
> > <<
> > +                                    GITS_BASER_PHYADDR_OFFSETL_64K
> > ;
> > +                c->ct.base_addr |= ((value >>
> > GITS_BASER_PHYADDR_OFFSET) &
> > +                                    GITS_BASER_PHYADDR_MASKH_64K)
> > <<
> > +                                    GITS_BASER_PHYADDR_OFFSETH_64K
> > ;
> > +            }
> > +        }
> > +    } else {
> > +        /* unsupported ITS table type */
> > +        qemu_log_mask(LOG_GUEST_ERROR, "%s: Unsupported ITS table
> > type %d",
> > +                         __func__, type);
> 
> LOG_UNIMP
> this is not LOG_UNIMP but error indication
> 
> > +        return false;
> > +    }
> > +    return true;
> > +}
> > +
> > +static bool extract_cmdq_params(GICv3ITSState *s)
> > +{
> > +    GICv3ITSClass *c = ARM_GICV3_ITS_GET_CLASS(s);
> > +    uint16_t num_pages = 0;
> > +    uint64_t value = s->cbaser;
> > +
> > +    num_pages = (value & GITS_CBASER_SIZE);
> > +
> > +    c->cq.valid = (value >> GITS_CBASER_VALID_OFFSET) &
> > +                                GITS_CBASER_VALID_MASK;
> > +
> > +    if (!num_pages || !c->cq.valid) {
> > +        return false;
> > +    }
> > +
> > +    if (c->cq.valid) {
> > +        c->cq.max_entries = ((num_pages + 1) *
> > GITS_ITT_PAGE_SIZE_0) /
> > +                                                GITS_CMDQ_ENTRY_SI
> > ZE;
> > +        c->cq.base_addr = (value >> GITS_CBASER_PHYADDR_OFFSET) &
> > +                                    GITS_CBASER_PHYADDR_MASK;
> > +        c->cq.base_addr <<= GITS_CBASER_PHYADDR_OFFSET;
> > +    }
> > +    return true;
> > +}
> > +
> >  static MemTxResult its_trans_writew(GICv3ITSState *s, hwaddr
> > offset,
> >                                 uint64_t value, MemTxAttrs attrs)
> >  {
> > @@ -126,7 +294,75 @@ static MemTxResult its_writel(GICv3ITSState
> > *s, hwaddr offset,
> >                                 uint64_t value, MemTxAttrs attrs)
> >  {
> >      MemTxResult result = MEMTX_OK;
> > +    int index;
> > +    uint64_t temp = 0;
> >  
> > +    switch (offset) {
> > +    case GITS_CTLR:
> > +        s->ctlr |= (value & ~(s->ctlr));
> > +        break;
> > +    case GITS_CBASER:
> > +        /* GITS_CBASER register becomes RO if ITS is already
> > enabled */
> > +        if (!(s->ctlr & GITS_CTLR_ENABLED)) {
> > +            s->cbaser = deposit64(s->cbaser, 0, 32, value);
> > +            s->creadr = 0;
> > +        }
> > +        break;
> > +    case GITS_CBASER + 4:
> > +        /* GITS_CBASER register becomes RO if ITS is already
> > enabled */
> > +        if (!(s->ctlr & GITS_CTLR_ENABLED)) {
> > +            s->cbaser = deposit64(s->cbaser, 32, 32, value);
> > +            if (!extract_cmdq_params(s)) {
> > +                qemu_log_mask(LOG_GUEST_ERROR,
> > +                       "%s: error extracting GITS_CBASER
> > parameters "
> > +                       TARGET_FMT_plx "\n", __func__, offset);
> > +                s->cbaser = 0;
> > +                result = MEMTX_ERROR;
> 
> LOG_UNIMP?
> this is not LOG_UNIMP but error indication
> 
> > +            } else {
> > +                s->creadr = 0;
> > +            }
> > +        }
> > +        break;
> > +    case GITS_CWRITER:
> > +        s->cwriter = deposit64(s->cwriter, 0, 32, value);
> > +        break;
> > +    case GITS_CWRITER + 4:
> > +        s->cwriter = deposit64(s->cwriter, 32, 32, value);
> > +        break;
> > +    case GITS_BASER ... GITS_BASER + 0x3f:
> > +        /* GITS_BASERn registers become RO if ITS is already
> > enabled */
> > +        if (!(s->ctlr & GITS_CTLR_ENABLED)) {
> > +            index = (offset - GITS_BASER) / 8;
> > +
> > +            if (offset & 7) {
> > +                temp = s->baser[index];
> > +                temp = deposit64(temp, 32, 32, (value &
> > ~GITS_BASER_VAL_MASK));
> > +                s->baser[index] |= temp;
> > +
> > +                if (!extract_table_params(s, index)) {
> > +                    qemu_log_mask(LOG_GUEST_ERROR,
> > +                        "%s: error extracting GITS_BASER
> > parameters "
> > +                        TARGET_FMT_plx "\n", __func__, offset);
> 
> LOG_UNIMP again?
> this is not LOG_UNIMP but error indication
> 
> > +                    s->baser[index] = 0;
> > +                    result = MEMTX_ERROR;
> > +                }
> > +            } else {
> > +                s->baser[index] =  deposit64(s->baser[index], 0,
> > 32, value);
> > +            }
> > +        }
> > +        break;
> > +    case GITS_IIDR:
> > +    case GITS_TYPER:
> > +    case GITS_CREADR:
> > +        /* RO registers, ignore the write */
> > +        qemu_log_mask(LOG_GUEST_ERROR,
> > +            "%s: invalid guest write to RO register at offset "
> > +            TARGET_FMT_plx "\n", __func__, offset);
> > +        break;
> > +    default:
> > +        result = MEMTX_ERROR;
> > +        break;
> > +    }
> >      return result;
> >  }
> >  
> > @@ -134,7 +370,54 @@ static MemTxResult its_readl(GICv3ITSState *s,
> > hwaddr offset,
> >                                 uint64_t *data, MemTxAttrs attrs)
> >  {
> >      MemTxResult result = MEMTX_OK;
> > +    int index;
> >  
> > +    switch (offset) {
> > +    case GITS_CTLR:
> > +        *data = s->ctlr;
> > +        break;
> > +    case GITS_IIDR:
> > +        *data = s->iidr;
> > +        break;
> > +    case GITS_PIDR2:
> > +        *data = 0x30; /* GICv3 */
> > +        break;
> > +    case GITS_TYPER:
> > +        *data = extract64(s->typer, 0, 32);
> > +        break;
> > +    case GITS_TYPER + 4:
> > +        *data = extract64(s->typer, 32, 32);
> > +        break;
> > +    case GITS_CBASER:
> > +        *data = extract64(s->cbaser, 0, 32);
> > +        break;
> > +    case GITS_CBASER + 4:
> > +        *data = extract64(s->cbaser, 32, 32);
> > +        break;
> > +    case GITS_CREADR:
> > +        *data = extract64(s->creadr, 0, 32);
> > +        break;
> > +    case GITS_CREADR + 4:
> > +        *data = extract64(s->creadr, 32, 32);
> > +        break;
> > +    case GITS_CWRITER:
> > +        *data = extract64(s->cwriter, 0, 32);
> > +        break;
> > +    case GITS_CWRITER + 4:
> > +        *data = extract64(s->cwriter, 32, 32);
> > +        break;
> > +    case GITS_BASER ... GITS_BASER + 0x3f:
> > +        index = (offset - GITS_BASER) / 8;
> > +        if (offset & 7) {
> > +            *data = s->baser[index] >> 32;
> > +        } else {
> > +            *data = (uint32_t)s->baser[index];
> > +        }
> > +        break;
> > +    default:
> > +        result = MEMTX_ERROR;
> > +        break;
> > +    }
> >      return result;
> >  }
> >  
> > @@ -142,7 +425,52 @@ static MemTxResult its_writell(GICv3ITSState
> > *s, hwaddr offset,
> >                                 uint64_t value, MemTxAttrs attrs)
> >  {
> >      MemTxResult result = MEMTX_OK;
> > +    int index;
> >  
> > +    switch (offset) {
> > +    case GITS_BASER ... GITS_BASER + 0x3f:
> > +        /* GITS_BASERn registers become RO if ITS is already
> > enabled */
> > +        if (!(s->ctlr & GITS_CTLR_ENABLED)) {
> > +            index = (offset - GITS_BASER) / 8;
> > +            s->baser[index] |= (value & ~GITS_BASER_VAL_MASK);
> > +            if (!extract_table_params(s, index)) {
> > +                qemu_log_mask(LOG_GUEST_ERROR,
> > +                        "%s: error extracting GITS_BASER
> > parameters "
> > +                        TARGET_FMT_plx "\n", __func__, offset);
> > +                s->baser[index] = 0;
> > +                result = MEMTX_ERROR;
> > +            }
> > +        }
> > +        break;
> > +    case GITS_CBASER:
> > +        /* GITS_CBASER register becomes RO if ITS is already
> > enabled */
> > +        if (!(s->ctlr & GITS_CTLR_ENABLED)) {
> > +            s->cbaser = value;
> > +            if (!extract_cmdq_params(s)) {
> > +                qemu_log_mask(LOG_GUEST_ERROR,
> > +                       "%s: error extracting GITS_CBASER
> > parameters "
> > +                       TARGET_FMT_plx "\n", __func__, offset);
> 
> So are these all LOG_UNIMP or are they a programming failure on our
> part?
they are not LOG_UNIMP but error indications during processing which
could be due to invalid parameters passed from the driver
> 
> > +                s->cbaser = 0;
> > +                result = MEMTX_ERROR;
> > +            } else {
> > +                s->creadr = 0;
> > +            }
> > +        }
> > +        break;
> > +    case GITS_CWRITER:
> > +        s->cwriter = value;
> > +        break;
> > +    case GITS_TYPER:
> > +    case GITS_CREADR:
> > +        /* RO register, ignore the write */
> > +        qemu_log_mask(LOG_GUEST_ERROR,
> > +                      "%s: invalid guest write to RO register at
> > offset "
> > +                      TARGET_FMT_plx "\n", __func__, offset);
> > +        break;
> > +    default:
> > +        result = MEMTX_ERROR;
> > +        break;
> > +    }
> >      return result;
> >  }
> >  
> > @@ -150,7 +478,29 @@ static MemTxResult its_readll(GICv3ITSState
> > *s, hwaddr offset,
> >                                 uint64_t *data, MemTxAttrs attrs)
> >  {
> >      MemTxResult result = MEMTX_OK;
> > +    int index;
> >  
> > +    switch (offset) {
> > +    case GITS_TYPER:
> > +        *data = s->typer;
> > +        break;
> > +    case GITS_BASER ... GITS_BASER + 0x3f:
> > +        index = (offset - GITS_BASER) / 8;
> > +        *data = s->baser[index];
> > +        break;
> > +    case GITS_CBASER:
> > +        *data = s->cbaser;
> > +        break;
> > +    case GITS_CREADR:
> > +        *data = s->creadr;
> > +        break;
> > +    case GITS_CWRITER:
> > +        *data = s->cwriter;
> > +        break;
> > +    default:
> > +        result = MEMTX_ERROR;
> > +        break;
> > +    }
> >      return result;
> >  }
> >  
> > @@ -250,6 +600,9 @@ static void gicv3_arm_its_realize(DeviceState
> > *dev, Error **errp)
> >      GICv3ITSState *s = ARM_GICV3_ITS_COMMON(dev);
> >  
> >      gicv3_its_init_mmio(s, &gicv3_its_control_ops,
> > &gicv3_its_trans_ops);
> > +
> > +    address_space_init(&s->gicv3->sysmem_as, s->gicv3->sysmem,
> > +                        "gicv3-its-sysmem");
> >  }
> >  
> >  static void gicv3_its_reset(DeviceState *dev)
> > @@ -259,6 +612,9 @@ static void gicv3_its_reset(DeviceState *dev)
> >  
> >      if (s->gicv3->cpu->gicr_typer & GICR_TYPER_PLPIS) {
> >          c->parent_reset(dev);
> > +        memset(&c->dt, 0 , sizeof(c->dt));
> > +        memset(&c->ct, 0 , sizeof(c->ct));
> > +        memset(&c->cq, 0 , sizeof(c->cq));
> >  
> >          /* set the ITS default features supported */
> >          s->typer = GITS_TYPER_PHYSICAL |
> > (GITS_TYPER_ITT_ENTRY_SIZE <<
> > diff --git a/include/hw/intc/arm_gicv3_common.h
> > b/include/hw/intc/arm_gicv3_common.h
> > index 91491a2f66..b0f2414fa3 100644
> > --- a/include/hw/intc/arm_gicv3_common.h
> > +++ b/include/hw/intc/arm_gicv3_common.h
> > @@ -226,12 +226,16 @@ struct GICv3State {
> >      int dev_fd; /* kvm device fd if backed by kvm vgic support */
> >      Error *migration_blocker;
> >  
> > +    MemoryRegion *sysmem;
> > +    AddressSpace sysmem_as;
> > +
> >      /* Distributor */
> >  
> >      /* for a GIC with the security extensions the NS banked
> > version of this
> >       * register is just an alias of bit 1 of the S banked version.
> >       */
> >      uint32_t gicd_ctlr;
> > +    uint32_t gicd_typer;
> >      uint32_t gicd_statusr[2];
> >      GIC_DECLARE_BITMAP(group);        /* GICD_IGROUPR */
> >      GIC_DECLARE_BITMAP(grpmod);       /* GICD_IGRPMODR */
> 
> 



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

* Re: [PATCH v1 2/8] hw/intc: GICv3 ITS register definitions added
  2021-03-31 16:48     ` shashi.mallela
@ 2021-03-31 23:31       ` Richard Henderson
  0 siblings, 0 replies; 16+ messages in thread
From: Richard Henderson @ 2021-03-31 23:31 UTC (permalink / raw)
  To: shashi.mallela, Alex Bennée
  Cc: peter.maydell, leif, qemu-devel, qemu-arm, rad

On 3/31/21 9:48 AM, shashi.mallela@linaro.org wrote:
>>> +        if (!(s->ctlr & GITS_CTLR_ENABLED)) {
>>> +            s->cbaser = value;
>>> +            if (!extract_cmdq_params(s)) {
>>> +                qemu_log_mask(LOG_GUEST_ERROR,
>>> +                       "%s: error extracting GITS_CBASER
>>> parameters "
>>> +                       TARGET_FMT_plx "\n", __func__, offset);
>>
>> So are these all LOG_UNIMP or are they a programming failure on our
>> part?
> they are not LOG_UNIMP but error indications during processing which
> could be due to invalid parameters passed from the driver

Then we should re-word the message so that it does not read as if it is a qemu 
problem, but a guest problem.

It is difficult to do this with the code structure you've written, but you 
could use an Error** parameter (include/qabi/error.h) to extract_cmdq_params so 
that you can diagnose exactly what is wrong.


r~


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

end of thread, other threads:[~2021-03-31 23:33 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-23  4:12 [PATCH v1 0/8] GICv3 LPI and ITS feature implementation Shashi Mallela
2021-03-23  4:12 ` [PATCH v1 1/8] hw/intc: GICv3 ITS initial framework Shashi Mallela
2021-03-25 16:43   ` Alex Bennée
2021-03-25 17:18   ` Alex Bennée
2021-03-23  4:12 ` [PATCH v1 2/8] hw/intc: GICv3 ITS register definitions added Shashi Mallela
2021-03-25 19:34   ` Alex Bennée
2021-03-31 16:48     ` shashi.mallela
2021-03-31 23:31       ` Richard Henderson
2021-03-23  4:12 ` [PATCH v1 3/8] hw/intc: GICv3 ITS command queue framework Shashi Mallela
2021-03-23  4:12 ` [PATCH v1 4/8] hw/intc: GICv3 ITS Command processing Shashi Mallela
2021-03-23  4:12 ` [PATCH v1 5/8] hw/intc: GICv3 ITS Feature enablement Shashi Mallela
2021-03-23  4:12 ` [PATCH v1 6/8] hw/intc: GICv3 redistributor ITS processing Shashi Mallela
2021-03-23  4:12 ` [PATCH v1 7/8] hw/arm/sbsa-ref: add ITS support in SBSA GIC Shashi Mallela
2021-03-23  4:12 ` [PATCH v1 8/8] hw/arm/virt: add ITS support in virt GIC Shashi Mallela
2021-03-25 17:59 ` [PATCH v1 0/8] GICv3 LPI and ITS feature implementation Alex Bennée
2021-03-25 19:44 ` Alex Bennée

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).