Xiang Chen (1):

hw/arm/smmuv3: Pass the actual perm to returned IOMMUTLBEntry in smmuv3_translate()

docs/system/arm/virt.rst | 5 +-

hw/intc/gicv3_internal.h | 231 ++++++++-

include/hw/arm/virt.h | 19 +-

include/hw/intc/arm_gicv3_common.h | 13 +

include/hw/intc/arm_gicv3_its_common.h | 1 +

target/arm/cpu.h | 59 ++-

target/arm/translate-a32.h | 13 +-

target/arm/translate.h | 17 +-

hw/arm/smmuv3.c | 2 +-

hw/arm/virt.c | 102 +++-

hw/intc/arm_gicv3_common.c | 54 +-

hw/intc/arm_gicv3_cpuif.c | 195 ++++++--

hw/intc/arm_gicv3_dist.c | 7 +-

hw/intc/arm_gicv3_its.c | 876 +++++++++++++++++++++++++++------

hw/intc/arm_gicv3_its_kvm.c | 2 +

hw/intc/arm_gicv3_kvm.c | 5 +

hw/intc/arm_gicv3_redist.c | 480 +++++++++++++++---

linux-user/arm/cpu_loop.c | 2 +-

target/arm/cpu.c | 16 +-

target/arm/helper-a64.c | 4 +-

target/arm/helper.c | 19 +-

target/arm/hvf/hvf.c | 2 +-

target/arm/m_helper.c | 6 +-

target/arm/op_helper.c | 13 -

target/arm/translate-a64.c | 50 +-

target/arm/translate-m-nocp.c | 12 +-

target/arm/translate-neon.c | 21 +-

target/arm/translate-sve.c | 9 +-

target/arm/translate-vfp.c | 76 +--

target/arm/translate.c | 101 ++--

hw/intc/trace-events | 18 +-

31 files changed, 1890 insertions(+), 540 deletions(-)

In commit b6f96009acc we split do_process_its_cmd() from

process_its_cmd(), but forgot the usual blank line between function

definitions. Add it.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

Message-id: 20220408141550.1271295-2-peter.maydell@linaro.org

---

hw/intc/arm_gicv3_its.c | 1 +

1 file changed, 1 insertion(+)

diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/intc/arm_gicv3_its.c

+++ b/hw/intc/arm_gicv3_its.c

@@ -XXX,XX +XXX,XX @@ static ItsCmdResult do_process_its_cmd(GICv3ITSState *s, uint32_t devid,

}

return CMD_CONTINUE;

}

+

static ItsCmdResult process_its_cmd(GICv3ITSState *s, const uint64_t *cmdpkt,

ItsCmdType cmd)

{

--

2.25.1

In the GICv3 code we implicitly rely on there being at least one CPU

and thus at least one redistributor and CPU interface. Sanity-check

that the property the board code sets is not zero.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

Message-id: 20220408141550.1271295-3-peter.maydell@linaro.org

---

hw/intc/arm_gicv3_common.c | 4 ++++

1 file changed, 4 insertions(+)

diff --git a/hw/intc/arm_gicv3_common.c b/hw/intc/arm_gicv3_common.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/intc/arm_gicv3_common.c

+++ b/hw/intc/arm_gicv3_common.c

@@ -XXX,XX +XXX,XX @@ static void arm_gicv3_common_realize(DeviceState *dev, Error **errp)

s->num_irq, GIC_INTERNAL);

return;

}

+ if (s->num_cpu == 0) {

+ error_setg(errp, "num-cpu must be at least 1");

+ return;

+ }

/* ITLinesNumber is represented as (N / 32) - 1, so this is an

* implementation imposed restriction, not an architectural one,

--

2.25.1

Boards using the GICv3 need to configure it with both the total

number of CPUs and also the sizes of all the memory regions which

contain redistributors (one redistributor per CPU). At the moment

the GICv3 checks that the number of CPUs specified is not too many to

fit in the defined redistributor regions, but in fact the code

assumes that the two match exactly. For instance when we set the

GICR_TYPER.Last bit on the final redistributor in each region, we

assume that we don't need to consider the possibility of a region

being only half full of redistributors or even completely empty. We

also assume in gicv3_redist_read() and gicv3_redist_write() that we

can calculate the CPU index from the offset within the MemoryRegion

and that this will always be in range.

Fortunately all the board code sets the redistributor region sizes to

exactly match the CPU count, so this isn't a visible bug. We could

in theory make the GIC code handle non-full redistributor regions, or

have it automatically reduce the provided region sizes to match the

CPU count, but the simplest thing is just to strengthen the error

check and insist that the CPU count and redistributor region size

settings match exactly, since all the board code does that anyway.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

Message-id: 20220408141550.1271295-4-peter.maydell@linaro.org

---

hw/intc/arm_gicv3_common.c | 4 ++--

1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/intc/arm_gicv3_common.c b/hw/intc/arm_gicv3_common.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/intc/arm_gicv3_common.c

+++ b/hw/intc/arm_gicv3_common.c

@@ -XXX,XX +XXX,XX @@ static void arm_gicv3_common_realize(DeviceState *dev, Error **errp)

for (i = 0; i < s->nb_redist_regions; i++) {

rdist_capacity += s->redist_region_count[i];

}

- if (rdist_capacity < s->num_cpu) {

+ if (rdist_capacity != s->num_cpu) {

error_setg(errp, "Capacity of the redist regions(%d) "

- "is less than number of vcpus(%d)",

+ "does not match the number of vcpus(%d)",

rdist_capacity, s->num_cpu);

return;

}

--

2.25.1

We use the common function gicv3_idreg() to supply the CoreSight ID

register values for the GICv3 for the copies of these ID registers in

the distributor, redistributor and ITS register frames. This isn't

quite correct, because while most of the register values are the

same, the PIDR0 value should vary to indicate which of these three

frames it is. (You can see this and also the correct values of these

PIDR0 registers by looking at the GIC-600 or GIC-700 TRMs, for

example.)

Make gicv3_idreg() take an extra argument for the PIDR0 value.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

Message-id: 20220408141550.1271295-5-peter.maydell@linaro.org

---

hw/intc/gicv3_internal.h | 15 +++++++++++++--

hw/intc/arm_gicv3_dist.c | 2 +-

hw/intc/arm_gicv3_its.c | 2 +-

hw/intc/arm_gicv3_redist.c | 2 +-

4 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/hw/intc/gicv3_internal.h b/hw/intc/gicv3_internal.h

index XXXXXXX..XXXXXXX 100644

--- a/hw/intc/gicv3_internal.h

+++ b/hw/intc/gicv3_internal.h

@@ -XXX,XX +XXX,XX @@ static inline uint32_t gicv3_iidr(void)

return 0x43b;

}

-static inline uint32_t gicv3_idreg(int regoffset)

+/* CoreSight PIDR0 values for ARM GICv3 implementations */

+#define GICV3_PIDR0_DIST 0x92

+#define GICV3_PIDR0_REDIST 0x93

+#define GICV3_PIDR0_ITS 0x94

+

+static inline uint32_t gicv3_idreg(int regoffset, uint8_t pidr0)

{

/* Return the value of the CoreSight ID register at the specified

* offset from the first ID register (as found in the distributor

@@ -XXX,XX +XXX,XX @@ static inline uint32_t gicv3_idreg(int regoffset)

static const uint8_t gicd_ids[] = {

0x44, 0x00, 0x00, 0x00, 0x92, 0xB4, 0x3B, 0x00, 0x0D, 0xF0, 0x05, 0xB1

};

- return gicd_ids[regoffset / 4];

+

+ regoffset /= 4;

+

+ if (regoffset == 4) {

+ return pidr0;

+ }

+ return gicd_ids[regoffset];

}

/**

diff --git a/hw/intc/arm_gicv3_dist.c b/hw/intc/arm_gicv3_dist.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/intc/arm_gicv3_dist.c

+++ b/hw/intc/arm_gicv3_dist.c

@@ -XXX,XX +XXX,XX @@ static bool gicd_readl(GICv3State *s, hwaddr offset,

}

case GICD_IDREGS ... GICD_IDREGS + 0x2f:

/* ID registers */

- *data = gicv3_idreg(offset - GICD_IDREGS);

+ *data = gicv3_idreg(offset - GICD_IDREGS, GICV3_PIDR0_DIST);

return true;

case GICD_SGIR:

/* WO registers, return unknown value */

diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/intc/arm_gicv3_its.c

+++ b/hw/intc/arm_gicv3_its.c

@@ -XXX,XX +XXX,XX @@ static bool its_readl(GICv3ITSState *s, hwaddr offset,

break;

case GITS_IDREGS ... GITS_IDREGS + 0x2f:

/* ID registers */

- *data = gicv3_idreg(offset - GITS_IDREGS);

+ *data = gicv3_idreg(offset - GITS_IDREGS, GICV3_PIDR0_ITS);

break;

case GITS_TYPER:

*data = extract64(s->typer, 0, 32);

diff --git a/hw/intc/arm_gicv3_redist.c b/hw/intc/arm_gicv3_redist.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/intc/arm_gicv3_redist.c

+++ b/hw/intc/arm_gicv3_redist.c

@@ -XXX,XX +XXX,XX @@ static MemTxResult gicr_readl(GICv3CPUState *cs, hwaddr offset,

*data = cs->gicr_nsacr;

return MEMTX_OK;

case GICR_IDREGS ... GICR_IDREGS + 0x2f:

- *data = gicv3_idreg(offset - GICR_IDREGS);

+ *data = gicv3_idreg(offset - GICR_IDREGS, GICV3_PIDR0_REDIST);

return MEMTX_OK;

default:

return MEMTX_ERROR;

--

2.25.1

In a GICv3, it is impossible for the GIC to deliver a VIRQ or VFIQ to

the CPU unless the CPU has EL2, because VIRQ and VFIQ are only

configurable via EL2-only system registers. Moreover, in our

implementation we were only calculating and updating the state of the

VIRQ and VFIQ lines in gicv3_cpuif_virt_irq_fiq_update() when those

EL2 system registers changed. We were therefore able to assert in

arm_cpu_set_irq() that we didn't see a VIRQ or VFIQ line update if

EL2 wasn't present.

This assumption no longer holds with GICv4:

* even if the CPU does not have EL2 the guest is able to cause the

GIC to deliver a virtual LPI by programming the ITS (which is a

silly thing for it to do, but possible)

* because we now need to recalculate the state of the VIRQ and VFIQ

lines in more cases than just "some EL2 GIC sysreg was written",

we will see calls to arm_cpu_set_irq() for "VIRQ is 0, VFIQ is 0"

even if the guest is not using the virtual LPI parts of the ITS

Remove the assertions, and instead simply ignore the state of the

VIRQ and VFIQ lines if the CPU does not have EL2.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

Message-id: 20220408141550.1271295-6-peter.maydell@linaro.org

---

target/arm/cpu.c | 12 ++++++++++--

1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/target/arm/cpu.c b/target/arm/cpu.c

index XXXXXXX..XXXXXXX 100644

--- a/target/arm/cpu.c

+++ b/target/arm/cpu.c

@@ -XXX,XX +XXX,XX @@ static void arm_cpu_set_irq(void *opaque, int irq, int level)

[ARM_CPU_VFIQ] = CPU_INTERRUPT_VFIQ

};

+ if (!arm_feature(env, ARM_FEATURE_EL2) &&

+ (irq == ARM_CPU_VIRQ || irq == ARM_CPU_VFIQ)) {

+ /*

+ * The GIC might tell us about VIRQ and VFIQ state, but if we don't

+ * have EL2 support we don't care. (Unless the guest is doing something

+ * silly this will only be calls saying "level is still 0".)

+ */

+ return;

+ }

+

if (level) {

env->irq_line_state |= mask[irq];

} else {

@@ -XXX,XX +XXX,XX @@ static void arm_cpu_set_irq(void *opaque, int irq, int level)

switch (irq) {

case ARM_CPU_VIRQ:

- assert(arm_feature(env, ARM_FEATURE_EL2));

arm_cpu_update_virq(cpu);

break;

case ARM_CPU_VFIQ:

- assert(arm_feature(env, ARM_FEATURE_EL2));

arm_cpu_update_vfiq(cpu);

break;

case ARM_CPU_IRQ:

--

2.25.1

In process_mapti() we check interrupt IDs to see whether they are

in the valid LPI range. Factor this out into its own utility

function, as we're going to want it elsewhere too for GICv4.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

Message-id: 20220408141550.1271295-7-peter.maydell@linaro.org

---

hw/intc/arm_gicv3_its.c | 10 +++++++---

1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/intc/arm_gicv3_its.c

+++ b/hw/intc/arm_gicv3_its.c

@@ -XXX,XX +XXX,XX @@ typedef enum ItsCmdResult {

CMD_CONTINUE = 1,

} ItsCmdResult;

+static inline bool intid_in_lpi_range(uint32_t id)

+{

+ return id >= GICV3_LPI_INTID_START &&

+ id < (1 << (GICD_TYPER_IDBITS + 1));

+}

+

static uint64_t baser_base_addr(uint64_t value, uint32_t page_sz)

{

uint64_t result = 0;

@@ -XXX,XX +XXX,XX @@ static ItsCmdResult process_mapti(GICv3ITSState *s, const uint64_t *cmdpkt,

uint32_t devid, eventid;

uint32_t pIntid = 0;

uint64_t num_eventids;

- uint32_t num_intids;

uint16_t icid = 0;

DTEntry dte;

ITEntry ite;

@@ -XXX,XX +XXX,XX @@ static ItsCmdResult process_mapti(GICv3ITSState *s, const uint64_t *cmdpkt,

return CMD_STALL;

}

num_eventids = 1ULL << (dte.size + 1);

- num_intids = 1ULL << (GICD_TYPER_IDBITS + 1);

if (icid >= s->ct.num_entries) {

qemu_log_mask(LOG_GUEST_ERROR,

@@ -XXX,XX +XXX,XX @@ static ItsCmdResult process_mapti(GICv3ITSState *s, const uint64_t *cmdpkt,

return CMD_CONTINUE;

}

- if (pIntid < GICV3_LPI_INTID_START || pIntid >= num_intids) {

+ if (!intid_in_lpi_range(pIntid)) {

qemu_log_mask(LOG_GUEST_ERROR,

"%s: invalid interrupt ID 0x%x\n", __func__, pIntid);

return CMD_CONTINUE;

--

2.25.1

The GICv4 defines a new in-guest-memory table for the ITS: this is

the vPE table. Implement the new GITS_BASER2 register which the

guest uses to tell the ITS where the vPE table is located, including

the decode of the register fields into the TableDesc structure which

we do for the GITS_BASER<n> when the guest enables the ITS.

We guard provision of the new register with the its_feature_virtual()

function, which does a check of the GITS_TYPER.Virtual bit which

indicates presence of ITS support for virtual LPIs. Since this bit

is currently always zero, GICv4-specific features will not be

accessible to the guest yet.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

Message-id: 20220408141550.1271295-8-peter.maydell@linaro.org

---

hw/intc/gicv3_internal.h | 16 ++++++++++++++++

include/hw/intc/arm_gicv3_its_common.h | 1 +

hw/intc/arm_gicv3_its.c | 25 +++++++++++++++++++++++++

3 files changed, 42 insertions(+)

diff --git a/hw/intc/gicv3_internal.h b/hw/intc/gicv3_internal.h

index XXXXXXX..XXXXXXX 100644

--- a/hw/intc/gicv3_internal.h

+++ b/hw/intc/gicv3_internal.h

@@ -XXX,XX +XXX,XX @@ FIELD(GITS_CTLR, ENABLED, 0, 1)

FIELD(GITS_CTLR, QUIESCENT, 31, 1)

FIELD(GITS_TYPER, PHYSICAL, 0, 1)

+FIELD(GITS_TYPER, VIRTUAL, 1, 1)

FIELD(GITS_TYPER, ITT_ENTRY_SIZE, 4, 4)

FIELD(GITS_TYPER, IDBITS, 8, 5)

FIELD(GITS_TYPER, DEVBITS, 13, 5)

@@ -XXX,XX +XXX,XX @@ FIELD(GITS_TYPER, CIL, 36, 1)

#define GITS_BASER_PAGESIZE_64K 2

#define GITS_BASER_TYPE_DEVICE 1ULL

+#define GITS_BASER_TYPE_VPE 2ULL

#define GITS_BASER_TYPE_COLLECTION 4ULL

#define GITS_PAGE_SIZE_4K 0x1000

@@ -XXX,XX +XXX,XX @@ FIELD(DTE, ITTADDR, 6, 44)

FIELD(CTE, VALID, 0, 1)

FIELD(CTE, RDBASE, 1, RDBASE_PROCNUM_LENGTH)

+/*

+ * 8 bytes VPE table entry size:

+ * Valid = 1 bit, VPTsize = 5 bits, VPTaddr = 36 bits, RDbase = 16 bits

+ *

+ * Field sizes for Valid and size are mandated; field sizes for RDbase

+ * and VPT_addr are IMPDEF.

+ */

+#define GITS_VPE_SIZE 0x8ULL

+

+FIELD(VTE, VALID, 0, 1)

+FIELD(VTE, VPTSIZE, 1, 5)

+FIELD(VTE, VPTADDR, 6, 36)

+FIELD(VTE, RDBASE, 42, RDBASE_PROCNUM_LENGTH)

+

/* Special interrupt IDs */

#define INTID_SECURE 1020

#define INTID_NONSECURE 1021

diff --git a/include/hw/intc/arm_gicv3_its_common.h b/include/hw/intc/arm_gicv3_its_common.h

index XXXXXXX..XXXXXXX 100644

--- a/include/hw/intc/arm_gicv3_its_common.h

+++ b/include/hw/intc/arm_gicv3_its_common.h

@@ -XXX,XX +XXX,XX @@ struct GICv3ITSState {

TableDesc dt;

TableDesc ct;

+ TableDesc vpet;

CmdQDesc cq;

Error *migration_blocker;

diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/intc/arm_gicv3_its.c

+++ b/hw/intc/arm_gicv3_its.c

@@ -XXX,XX +XXX,XX @@ typedef enum ItsCmdResult {

CMD_CONTINUE = 1,

} ItsCmdResult;

+/* True if the ITS supports the GICv4 virtual LPI feature */

+static bool its_feature_virtual(GICv3ITSState *s)

+{

+ return s->typer & R_GITS_TYPER_VIRTUAL_MASK;

+}

+

static inline bool intid_in_lpi_range(uint32_t id)

{

return id >= GICV3_LPI_INTID_START &&

@@ -XXX,XX +XXX,XX @@ static void extract_table_params(GICv3ITSState *s)

idbits = 16;

}

break;

+ case GITS_BASER_TYPE_VPE:

+ td = &s->vpet;

+ /*

+ * For QEMU vPEIDs are always 16 bits. (GICv4.1 allows an

+ * implementation to implement fewer bits and report this

+ * via GICD_TYPER2.)

+ */

+ idbits = 16;

+ break;

default:

/*

* GITS_BASER<n>.TYPE is read-only, so GITS_BASER_RO_MASK

@@ -XXX,XX +XXX,XX @@ static void gicv3_its_reset(DeviceState *dev)

/*

* setting GITS_BASER0.Type = 0b001 (Device)

* GITS_BASER1.Type = 0b100 (Collection Table)

+ * GITS_BASER2.Type = 0b010 (vPE) for GICv4 and later

* 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

@@ -XXX,XX +XXX,XX @@ static void gicv3_its_reset(DeviceState *dev)

GITS_BASER_PAGESIZE_64K);

s->baser[1] = FIELD_DP64(s->baser[1], GITS_BASER, ENTRYSIZE,

GITS_CTE_SIZE - 1);

+

+ if (its_feature_virtual(s)) {

+ s->baser[2] = FIELD_DP64(s->baser[2], GITS_BASER, TYPE,

+ GITS_BASER_TYPE_VPE);

+ s->baser[2] = FIELD_DP64(s->baser[2], GITS_BASER, PAGESIZE,

+ GITS_BASER_PAGESIZE_64K);

+ s->baser[2] = FIELD_DP64(s->baser[2], GITS_BASER, ENTRYSIZE,

+ GITS_VPE_SIZE - 1);

+ }

}

static void gicv3_its_post_load(GICv3ITSState *s)

--

2.25.1

Implement the GICv4 VMAPI and VMAPTI commands. These write

an interrupt translation table entry that maps (DeviceID,EventID)

to (vPEID,vINTID,doorbell). The only difference between VMAPI

and VMAPTI is that VMAPI assumes vINTID == EventID rather than

both being specified in the command packet.

(This code won't be reachable until we allow the GIC version to be

set to 4. Support for reading this new virtual-interrupt DTE and

handling it correctly will be implemented in a later commit.)

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

Message-id: 20220408141550.1271295-9-peter.maydell@linaro.org

---

hw/intc/gicv3_internal.h | 9 ++++

hw/intc/arm_gicv3_its.c | 91 ++++++++++++++++++++++++++++++++++++++++

hw/intc/trace-events | 2 +

3 files changed, 102 insertions(+)

diff --git a/hw/intc/gicv3_internal.h b/hw/intc/gicv3_internal.h

index XXXXXXX..XXXXXXX 100644

--- a/hw/intc/gicv3_internal.h

+++ b/hw/intc/gicv3_internal.h

@@ -XXX,XX +XXX,XX @@ FIELD(GITS_TYPER, CIL, 36, 1)

#define GITS_CMD_INVALL 0x0D

#define GITS_CMD_MOVALL 0x0E

#define GITS_CMD_DISCARD 0x0F

+#define GITS_CMD_VMAPTI 0x2A

+#define GITS_CMD_VMAPI 0x2B

/* MAPC command fields */

#define ICID_LENGTH 16

@@ -XXX,XX +XXX,XX @@ FIELD(MOVI_0, DEVICEID, 32, 32)

FIELD(MOVI_1, EVENTID, 0, 32)

FIELD(MOVI_2, ICID, 0, 16)

+/* VMAPI, VMAPTI command fields */

+FIELD(VMAPTI_0, DEVICEID, 32, 32)

+FIELD(VMAPTI_1, EVENTID, 0, 32)

+FIELD(VMAPTI_1, VPEID, 32, 16)

+FIELD(VMAPTI_2, VINTID, 0, 32) /* VMAPTI only */

+FIELD(VMAPTI_2, DOORBELL, 32, 32)

+

/*

* 12 bytes Interrupt translation Table Entry size

* as per Table 5.3 in GICv3 spec

diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/intc/arm_gicv3_its.c

+++ b/hw/intc/arm_gicv3_its.c

@@ -XXX,XX +XXX,XX @@ static inline bool intid_in_lpi_range(uint32_t id)

id < (1 << (GICD_TYPER_IDBITS + 1));

}

+static inline bool valid_doorbell(uint32_t id)

+{

+ /* Doorbell fields may be an LPI, or 1023 to mean "no doorbell" */

+ return id == INTID_SPURIOUS || intid_in_lpi_range(id);

+}

+

static uint64_t baser_base_addr(uint64_t value, uint32_t page_sz)

{

uint64_t result = 0;

@@ -XXX,XX +XXX,XX @@ static ItsCmdResult process_mapti(GICv3ITSState *s, const uint64_t *cmdpkt,

return update_ite(s, eventid, &dte, &ite) ? CMD_CONTINUE : CMD_STALL;

}

+static ItsCmdResult process_vmapti(GICv3ITSState *s, const uint64_t *cmdpkt,

+ bool ignore_vintid)

+{

+ uint32_t devid, eventid, vintid, doorbell, vpeid;

+ uint32_t num_eventids;

+ DTEntry dte;

+ ITEntry ite;

+

+ if (!its_feature_virtual(s)) {

+ return CMD_CONTINUE;

+ }

+

+ devid = FIELD_EX64(cmdpkt[0], VMAPTI_0, DEVICEID);

+ eventid = FIELD_EX64(cmdpkt[1], VMAPTI_1, EVENTID);

+ vpeid = FIELD_EX64(cmdpkt[1], VMAPTI_1, VPEID);

+ doorbell = FIELD_EX64(cmdpkt[2], VMAPTI_2, DOORBELL);

+ if (ignore_vintid) {

+ vintid = eventid;

+ trace_gicv3_its_cmd_vmapi(devid, eventid, vpeid, doorbell);

+ } else {

+ vintid = FIELD_EX64(cmdpkt[2], VMAPTI_2, VINTID);

+ trace_gicv3_its_cmd_vmapti(devid, eventid, vpeid, vintid, doorbell);

+ }

+

+ if (devid >= s->dt.num_entries) {

+ qemu_log_mask(LOG_GUEST_ERROR,

+ "%s: invalid DeviceID 0x%x (must be less than 0x%x)\n",

+ __func__, devid, s->dt.num_entries);

+ return CMD_CONTINUE;

+ }

+

+ if (get_dte(s, devid, &dte) != MEMTX_OK) {

+ return CMD_STALL;

+ }

+

+ if (!dte.valid) {

+ qemu_log_mask(LOG_GUEST_ERROR,

+ "%s: no entry in device table for DeviceID 0x%x\n",

+ __func__, devid);

+ return CMD_CONTINUE;

+ }

+

+ num_eventids = 1ULL << (dte.size + 1);

+

+ if (eventid >= num_eventids) {

+ qemu_log_mask(LOG_GUEST_ERROR,

+ "%s: EventID 0x%x too large for DeviceID 0x%x "

+ "(must be less than 0x%x)\n",

+ __func__, eventid, devid, num_eventids);

+ return CMD_CONTINUE;

+ }

+ if (!intid_in_lpi_range(vintid)) {

+ qemu_log_mask(LOG_GUEST_ERROR,

+ "%s: VIntID 0x%x not a valid LPI\n",

+ __func__, vintid);

+ return CMD_CONTINUE;

+ }

+ if (!valid_doorbell(doorbell)) {

+ qemu_log_mask(LOG_GUEST_ERROR,

+ "%s: Doorbell %d not 1023 and not a valid LPI\n",

+ __func__, doorbell);

+ return CMD_CONTINUE;

+ }

+ if (vpeid >= s->vpet.num_entries) {

+ qemu_log_mask(LOG_GUEST_ERROR,

+ "%s: VPEID 0x%x out of range (must be less than 0x%x)\n",

+ __func__, vpeid, s->vpet.num_entries);

+ return CMD_CONTINUE;

+ }

+ /* add ite entry to interrupt translation table */

+ ite.valid = true;

+ ite.inttype = ITE_INTTYPE_VIRTUAL;

+ ite.intid = vintid;

+ ite.icid = 0;

+ ite.doorbell = doorbell;

+ ite.vpeid = vpeid;

+ return update_ite(s, eventid, &dte, &ite) ? CMD_CONTINUE : CMD_STALL;

+}

+

/*

* Update the Collection Table entry for @icid to @cte. Returns true

* on success, false if there was a memory access error.

@@ -XXX,XX +XXX,XX @@ static void process_cmdq(GICv3ITSState *s)

case GITS_CMD_MOVALL:

result = process_movall(s, cmdpkt);

break;

+ case GITS_CMD_VMAPTI:

+ result = process_vmapti(s, cmdpkt, false);

+ break;

+ case GITS_CMD_VMAPI:

+ result = process_vmapti(s, cmdpkt, true);

+ break;

default:

trace_gicv3_its_cmd_unknown(cmd);

break;

diff --git a/hw/intc/trace-events b/hw/intc/trace-events

index XXXXXXX..XXXXXXX 100644

--- a/hw/intc/trace-events

+++ b/hw/intc/trace-events

@@ -XXX,XX +XXX,XX @@ gicv3_its_cmd_mapti(uint32_t devid, uint32_t eventid, uint32_t icid, uint32_t in

gicv3_its_cmd_inv(void) "GICv3 ITS: command INV or INVALL"

gicv3_its_cmd_movall(uint64_t rd1, uint64_t rd2) "GICv3 ITS: command MOVALL RDbase1 0x%" PRIx64 " RDbase2 0x%" PRIx64

gicv3_its_cmd_movi(uint32_t devid, uint32_t eventid, uint32_t icid) "GICv3 ITS: command MOVI DeviceID 0x%x EventID 0x%x ICID 0x%x"

+gicv3_its_cmd_vmapi(uint32_t devid, uint32_t eventid, uint32_t vpeid, uint32_t doorbell) "GICv3 ITS: command VMAPI DeviceID 0x%x EventID 0x%x vPEID 0x%x Dbell_pINTID 0x%x"

+gicv3_its_cmd_vmapti(uint32_t devid, uint32_t eventid, uint32_t vpeid, uint32_t vintid, uint32_t doorbell) "GICv3 ITS: command VMAPI DeviceID 0x%x EventID 0x%x vPEID 0x%x vINTID 0x%x Dbell_pINTID 0x%x"

gicv3_its_cmd_unknown(unsigned cmd) "GICv3 ITS: unknown command 0x%x"

gicv3_its_cte_read(uint32_t icid, int valid, uint32_t rdbase) "GICv3 ITS: Collection Table read for ICID 0x%x: valid %d RDBase 0x%x"

gicv3_its_cte_write(uint32_t icid, int valid, uint32_t rdbase) "GICv3 ITS: Collection Table write for ICID 0x%x: valid %d RDBase 0x%x"

--

2.25.1

Implement the GICv4 VMAPP command, which writes an entry to the vPE

table.

For GICv4.1 this command has extra fields in the command packet

and additional behaviour. We define the 4.1-only fields with the

FIELD macro, but only implement the GICv4.0 version of the command.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

Message-id: 20220408141550.1271295-10-peter.maydell@linaro.org

---

hw/intc/gicv3_internal.h | 12 ++++++

hw/intc/arm_gicv3_its.c | 88 ++++++++++++++++++++++++++++++++++++++++

hw/intc/trace-events | 2 +

3 files changed, 102 insertions(+)

diff --git a/hw/intc/gicv3_internal.h b/hw/intc/gicv3_internal.h

index XXXXXXX..XXXXXXX 100644

--- a/hw/intc/gicv3_internal.h

+++ b/hw/intc/gicv3_internal.h

@@ -XXX,XX +XXX,XX @@ FIELD(GITS_TYPER, CIL, 36, 1)

#define GITS_CMD_INVALL 0x0D

#define GITS_CMD_MOVALL 0x0E

#define GITS_CMD_DISCARD 0x0F

+#define GITS_CMD_VMAPP 0x29

#define GITS_CMD_VMAPTI 0x2A

#define GITS_CMD_VMAPI 0x2B

@@ -XXX,XX +XXX,XX @@ FIELD(VMAPTI_1, VPEID, 32, 16)

FIELD(VMAPTI_2, VINTID, 0, 32) /* VMAPTI only */

FIELD(VMAPTI_2, DOORBELL, 32, 32)

+/* VMAPP command fields */

+FIELD(VMAPP_0, ALLOC, 8, 1) /* GICv4.1 only */

+FIELD(VMAPP_0, PTZ, 9, 1) /* GICv4.1 only */

+FIELD(VMAPP_0, VCONFADDR, 16, 36) /* GICv4.1 only */

+FIELD(VMAPP_1, DEFAULT_DOORBELL, 0, 32) /* GICv4.1 only */

+FIELD(VMAPP_1, VPEID, 32, 16)

+FIELD(VMAPP_2, RDBASE, 16, 36)

+FIELD(VMAPP_2, V, 63, 1)

+FIELD(VMAPP_3, VPTSIZE, 0, 8) /* For GICv4.0, bits [7:6] are RES0 */

+FIELD(VMAPP_3, VPTADDR, 16, 36)

+

/*

* 12 bytes Interrupt translation Table Entry size

* as per Table 5.3 in GICv3 spec

diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/intc/arm_gicv3_its.c

+++ b/hw/intc/arm_gicv3_its.c

@@ -XXX,XX +XXX,XX @@ typedef struct ITEntry {

uint32_t vpeid;

} ITEntry;

+typedef struct VTEntry {

+ bool valid;

+ unsigned vptsize;

+ uint32_t rdbase;

+ uint64_t vptaddr;

+} VTEntry;

/*

* The ITS spec permits a range of CONSTRAINED UNPREDICTABLE options

@@ -XXX,XX +XXX,XX @@ static ItsCmdResult process_movi(GICv3ITSState *s, const uint64_t *cmdpkt)

return update_ite(s, eventid, &dte, &old_ite) ? CMD_CONTINUE : CMD_STALL;

}

+/*

+ * Update the vPE Table entry at index @vpeid with the entry @vte.

+ * Returns true on success, false if there was a memory access error.

+ */

+static bool update_vte(GICv3ITSState *s, uint32_t vpeid, const VTEntry *vte)

+{

+ AddressSpace *as = &s->gicv3->dma_as;

+ uint64_t entry_addr;

+ uint64_t vteval = 0;

+ MemTxResult res = MEMTX_OK;

+

+ trace_gicv3_its_vte_write(vpeid, vte->valid, vte->vptsize, vte->vptaddr,

+ vte->rdbase);

+

+ if (vte->valid) {

+ vteval = FIELD_DP64(vteval, VTE, VALID, 1);

+ vteval = FIELD_DP64(vteval, VTE, VPTSIZE, vte->vptsize);

+ vteval = FIELD_DP64(vteval, VTE, VPTADDR, vte->vptaddr);

+ vteval = FIELD_DP64(vteval, VTE, RDBASE, vte->rdbase);

+ }

+

+ entry_addr = table_entry_addr(s, &s->vpet, vpeid, &res);

+ if (res != MEMTX_OK) {

+ return false;

+ }

+ if (entry_addr == -1) {

+ /* No L2 table for this index: discard write and continue */

+ return true;

+ }

+ address_space_stq_le(as, entry_addr, vteval, MEMTXATTRS_UNSPECIFIED, &res);

+ return res == MEMTX_OK;

+}

+

+static ItsCmdResult process_vmapp(GICv3ITSState *s, const uint64_t *cmdpkt)

+{

+ VTEntry vte;

+ uint32_t vpeid;

+

+ if (!its_feature_virtual(s)) {

+ return CMD_CONTINUE;

+ }

+

+ vpeid = FIELD_EX64(cmdpkt[1], VMAPP_1, VPEID);

+ vte.rdbase = FIELD_EX64(cmdpkt[2], VMAPP_2, RDBASE);

+ vte.valid = FIELD_EX64(cmdpkt[2], VMAPP_2, V);

+ vte.vptsize = FIELD_EX64(cmdpkt[3], VMAPP_3, VPTSIZE);

+ vte.vptaddr = FIELD_EX64(cmdpkt[3], VMAPP_3, VPTADDR);

+

+ trace_gicv3_its_cmd_vmapp(vpeid, vte.rdbase, vte.valid,

+ vte.vptaddr, vte.vptsize);

+

+ /*

+ * For GICv4.0 the VPT_size field is only 5 bits, whereas we

+ * define our field macros to include the full GICv4.1 8 bits.

+ * The range check on VPT_size will catch the cases where

+ * the guest set the RES0-in-GICv4.0 bits [7:6].

+ */

+ if (vte.vptsize > FIELD_EX64(s->typer, GITS_TYPER, IDBITS)) {

+ qemu_log_mask(LOG_GUEST_ERROR,

+ "%s: invalid VPT_size 0x%x\n", __func__, vte.vptsize);

+ return CMD_CONTINUE;

+ }

+

+ if (vte.valid && vte.rdbase >= s->gicv3->num_cpu) {

+ qemu_log_mask(LOG_GUEST_ERROR,

+ "%s: invalid rdbase 0x%x\n", __func__, vte.rdbase);

+ return CMD_CONTINUE;

+ }

+

+ if (vpeid >= s->vpet.num_entries) {

+ qemu_log_mask(LOG_GUEST_ERROR,

+ "%s: VPEID 0x%x out of range (must be less than 0x%x)\n",

+ __func__, vpeid, s->vpet.num_entries);

+ return CMD_CONTINUE;

+ }

+

+ return update_vte(s, vpeid, &vte) ? CMD_CONTINUE : CMD_STALL;

+}

+

/*

* Current implementation blocks until all

* commands are processed

@@ -XXX,XX +XXX,XX @@ static void process_cmdq(GICv3ITSState *s)

case GITS_CMD_VMAPI:

result = process_vmapti(s, cmdpkt, true);

break;

+ case GITS_CMD_VMAPP:

+ result = process_vmapp(s, cmdpkt);

+ break;

default:

trace_gicv3_its_cmd_unknown(cmd);

break;

diff --git a/hw/intc/trace-events b/hw/intc/trace-events

index XXXXXXX..XXXXXXX 100644

--- a/hw/intc/trace-events

+++ b/hw/intc/trace-events

@@ -XXX,XX +XXX,XX @@ gicv3_its_cmd_movall(uint64_t rd1, uint64_t rd2) "GICv3 ITS: command MOVALL RDba

gicv3_its_cmd_movi(uint32_t devid, uint32_t eventid, uint32_t icid) "GICv3 ITS: command MOVI DeviceID 0x%x EventID 0x%x ICID 0x%x"

gicv3_its_cmd_vmapi(uint32_t devid, uint32_t eventid, uint32_t vpeid, uint32_t doorbell) "GICv3 ITS: command VMAPI DeviceID 0x%x EventID 0x%x vPEID 0x%x Dbell_pINTID 0x%x"

gicv3_its_cmd_vmapti(uint32_t devid, uint32_t eventid, uint32_t vpeid, uint32_t vintid, uint32_t doorbell) "GICv3 ITS: command VMAPI DeviceID 0x%x EventID 0x%x vPEID 0x%x vINTID 0x%x Dbell_pINTID 0x%x"

+gicv3_its_cmd_vmapp(uint32_t vpeid, uint64_t rdbase, int valid, uint64_t vptaddr, uint32_t vptsize) "GICv3 ITS: command VMAPP vPEID 0x%x RDbase 0x%" PRIx64 " V %d VPT_addr 0x%" PRIx64 " VPT_size 0x%x"

gicv3_its_cmd_unknown(unsigned cmd) "GICv3 ITS: unknown command 0x%x"

gicv3_its_cte_read(uint32_t icid, int valid, uint32_t rdbase) "GICv3 ITS: Collection Table read for ICID 0x%x: valid %d RDBase 0x%x"

gicv3_its_cte_write(uint32_t icid, int valid, uint32_t rdbase) "GICv3 ITS: Collection Table write for ICID 0x%x: valid %d RDBase 0x%x"

@@ -XXX,XX +XXX,XX @@ gicv3_its_ite_write(uint64_t ittaddr, uint32_t eventid, int valid, int inttype,

gicv3_its_dte_read(uint32_t devid, int valid, uint32_t size, uint64_t ittaddr) "GICv3 ITS: Device Table read for DeviceID 0x%x: valid %d size 0x%x ITTaddr 0x%" PRIx64

gicv3_its_dte_write(uint32_t devid, int valid, uint32_t size, uint64_t ittaddr) "GICv3 ITS: Device Table write for DeviceID 0x%x: valid %d size 0x%x ITTaddr 0x%" PRIx64

gicv3_its_dte_read_fault(uint32_t devid) "GICv3 ITS: Device Table read for DeviceID 0x%x: faulted"

+gicv3_its_vte_write(uint32_t vpeid, int valid, uint32_t vptsize, uint64_t vptaddr, uint32_t rdbase) "GICv3 ITS: vPE Table write for vPEID 0x%x: valid %d VPTsize 0x%x VPTaddr 0x%" PRIx64 " RDbase 0x%x"

# armv7m_nvic.c

nvic_recompute_state(int vectpending, int vectpending_prio, int exception_prio) "NVIC state recomputed: vectpending %d vectpending_prio %d exception_prio %d"

--

2.25.1

In the ItsCmdResult enum, we currently distinguish only CMD_STALL

(failure, stall processing of the command queue) and CMD_CONTINUE

(keep processing the queue), and we use the latter both for "there

was a parameter error, go on to the next command" and "the command

succeeded, go on to the next command". Sometimes we would like to

distinguish those two cases, so add CMD_CONTINUE_OK to the enum to

represent the success situation, and use it in the relevant places.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

Message-id: 20220408141550.1271295-11-peter.maydell@linaro.org

---

hw/intc/arm_gicv3_its.c | 29 ++++++++++++++++-------------

1 file changed, 16 insertions(+), 13 deletions(-)

diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/intc/arm_gicv3_its.c

+++ b/hw/intc/arm_gicv3_its.c

@@ -XXX,XX +XXX,XX @@ typedef struct VTEntry {

* and continue processing.

* The process_* functions which handle individual ITS commands all

* return an ItsCmdResult which tells process_cmdq() whether it should

- * stall or keep going.

+ * stall, keep going because of an error, or keep going because the

+ * command was a success.

*/

typedef enum ItsCmdResult {

CMD_STALL = 0,

CMD_CONTINUE = 1,

+ CMD_CONTINUE_OK = 2,

} ItsCmdResult;

/* True if the ITS supports the GICv4 virtual LPI feature */

@@ -XXX,XX +XXX,XX @@ static ItsCmdResult do_process_its_cmd(GICv3ITSState *s, uint32_t devid,

ITEntry ite = {};

/* remove mapping from interrupt translation table */

ite.valid = false;

- return update_ite(s, eventid, &dte, &ite) ? CMD_CONTINUE : CMD_STALL;

+ return update_ite(s, eventid, &dte, &ite) ? CMD_CONTINUE_OK : CMD_STALL;

}

- return CMD_CONTINUE;

+ return CMD_CONTINUE_OK;

}

static ItsCmdResult process_its_cmd(GICv3ITSState *s, const uint64_t *cmdpkt,

@@ -XXX,XX +XXX,XX @@ static ItsCmdResult process_mapti(GICv3ITSState *s, const uint64_t *cmdpkt,

ite.icid = icid;

ite.doorbell = INTID_SPURIOUS;

ite.vpeid = 0;

- return update_ite(s, eventid, &dte, &ite) ? CMD_CONTINUE : CMD_STALL;

+ return update_ite(s, eventid, &dte, &ite) ? CMD_CONTINUE_OK : CMD_STALL;

}

static ItsCmdResult process_vmapti(GICv3ITSState *s, const uint64_t *cmdpkt,

@@ -XXX,XX +XXX,XX @@ static ItsCmdResult process_vmapti(GICv3ITSState *s, const uint64_t *cmdpkt,

ite.icid = 0;

ite.doorbell = doorbell;

ite.vpeid = vpeid;

- return update_ite(s, eventid, &dte, &ite) ? CMD_CONTINUE : CMD_STALL;

+ return update_ite(s, eventid, &dte, &ite) ? CMD_CONTINUE_OK : CMD_STALL;

}

/*

@@ -XXX,XX +XXX,XX @@ static ItsCmdResult process_mapc(GICv3ITSState *s, const uint64_t *cmdpkt)

return CMD_CONTINUE;

}

- return update_cte(s, icid, &cte) ? CMD_CONTINUE : CMD_STALL;

+ return update_cte(s, icid, &cte) ? CMD_CONTINUE_OK : CMD_STALL;

}

/*

@@ -XXX,XX +XXX,XX @@ static ItsCmdResult process_mapd(GICv3ITSState *s, const uint64_t *cmdpkt)

return CMD_CONTINUE;

}

- return update_dte(s, devid, &dte) ? CMD_CONTINUE : CMD_STALL;

+ return update_dte(s, devid, &dte) ? CMD_CONTINUE_OK : CMD_STALL;

}

static ItsCmdResult process_movall(GICv3ITSState *s, const uint64_t *cmdpkt)

@@ -XXX,XX +XXX,XX @@ static ItsCmdResult process_movall(GICv3ITSState *s, const uint64_t *cmdpkt)

if (rd1 == rd2) {

/* Move to same target must succeed as a no-op */

- return CMD_CONTINUE;

+ return CMD_CONTINUE_OK;

}

/* Move all pending LPIs from redistributor 1 to redistributor 2 */

gicv3_redist_movall_lpis(&s->gicv3->cpu[rd1], &s->gicv3->cpu[rd2]);

- return CMD_CONTINUE;

+ return CMD_CONTINUE_OK;

}

static ItsCmdResult process_movi(GICv3ITSState *s, const uint64_t *cmdpkt)

@@ -XXX,XX +XXX,XX @@ static ItsCmdResult process_movi(GICv3ITSState *s, const uint64_t *cmdpkt)

/* Update the ICID field in the interrupt translation table entry */

old_ite.icid = new_icid;

- return update_ite(s, eventid, &dte, &old_ite) ? CMD_CONTINUE : CMD_STALL;

+ return update_ite(s, eventid, &dte, &old_ite) ? CMD_CONTINUE_OK : CMD_STALL;

}

/*

@@ -XXX,XX +XXX,XX @@ static ItsCmdResult process_vmapp(GICv3ITSState *s, const uint64_t *cmdpkt)

return CMD_CONTINUE;

}

- return update_vte(s, vpeid, &vte) ? CMD_CONTINUE : CMD_STALL;

+ return update_vte(s, vpeid, &vte) ? CMD_CONTINUE_OK : CMD_STALL;

}

/*

@@ -XXX,XX +XXX,XX @@ static void process_cmdq(GICv3ITSState *s)

}

while (wr_offset != rd_offset) {

- ItsCmdResult result = CMD_CONTINUE;

+ ItsCmdResult result = CMD_CONTINUE_OK;

void *hostmem;

hwaddr buflen;

uint64_t cmdpkt[GITS_CMDQ_ENTRY_WORDS];

@@ -XXX,XX +XXX,XX @@ static void process_cmdq(GICv3ITSState *s)

trace_gicv3_its_cmd_unknown(cmd);

break;

}

- if (result == CMD_CONTINUE) {

+ if (result != CMD_STALL) {

+ /* CMD_CONTINUE or CMD_CONTINUE_OK */

rd_offset++;

rd_offset %= s->cq.num_entries;

s->creadr = FIELD_DP64(s->creadr, GITS_CREADR, OFFSET, rd_offset);

--

2.25.1

The operation of finding an interrupt table entry given a (DeviceID,

EventID) pair is necessary in multiple different ITS commands. The

process requires first using the DeviceID as an index into the device

table to find the DTE, and then useng the EventID as an index into

the interrupt table specified by that DTE to find the ITE. We also

need to handle all the possible error cases: indexes out of range,

table memory not readable, table entries not valid.

Factor this out into a separate lookup_ite() function which we

can then call from the places where we were previously open-coding

this sequence. We'll also need this for some of the new GICv4.0

commands.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

Message-id: 20220408141550.1271295-12-peter.maydell@linaro.org

---

hw/intc/arm_gicv3_its.c | 124 +++++++++++++++++++++-------------------

1 file changed, 64 insertions(+), 60 deletions(-)

diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/intc/arm_gicv3_its.c

+++ b/hw/intc/arm_gicv3_its.c

@@ -XXX,XX +XXX,XX @@ out:

return res;

}

+/*

+ * Given a (DeviceID, EventID), look up the corresponding ITE, including

+ * checking for the various invalid-value cases. If we find a valid ITE,

+ * fill in @ite and @dte and return CMD_CONTINUE_OK. Otherwise return

+ * CMD_STALL or CMD_CONTINUE as appropriate (and the contents of @ite

+ * should not be relied on).

+ *

+ * The string @who is purely for the LOG_GUEST_ERROR messages,

+ * and should indicate the name of the calling function or similar.

+ */

+static ItsCmdResult lookup_ite(GICv3ITSState *s, const char *who,

+ uint32_t devid, uint32_t eventid, ITEntry *ite,

+ DTEntry *dte)

+{

+ uint64_t num_eventids;

+

+ if (devid >= s->dt.num_entries) {

+ qemu_log_mask(LOG_GUEST_ERROR,

+ "%s: invalid command attributes: devid %d>=%d",

+ who, devid, s->dt.num_entries);

+ return CMD_CONTINUE;

+ }

+

+ if (get_dte(s, devid, dte) != MEMTX_OK) {

+ return CMD_STALL;

+ }

+ if (!dte->valid) {

+ qemu_log_mask(LOG_GUEST_ERROR,

+ "%s: invalid command attributes: "

+ "invalid dte for %d\n", who, devid);

+ return CMD_CONTINUE;

+ }

+

+ num_eventids = 1ULL << (dte->size + 1);

+ if (eventid >= num_eventids) {

+ qemu_log_mask(LOG_GUEST_ERROR,

+ "%s: invalid command attributes: eventid %d >= %"

+ PRId64 "\n", who, eventid, num_eventids);

+ return CMD_CONTINUE;

+ }

+

+ if (get_ite(s, eventid, dte, ite) != MEMTX_OK) {

+ return CMD_STALL;

+ }

+

+ if (!ite->valid) {

+ qemu_log_mask(LOG_GUEST_ERROR,

+ "%s: invalid command attributes: invalid ITE\n", who);

+ return CMD_CONTINUE;

+ }

+

+ return CMD_CONTINUE_OK;

+}

+

/*

* This function handles the processing of following commands based on

* the ItsCmdType parameter passed:-

@@ -XXX,XX +XXX,XX @@ out:

static ItsCmdResult do_process_its_cmd(GICv3ITSState *s, uint32_t devid,

uint32_t eventid, ItsCmdType cmd)

{

- uint64_t num_eventids;

DTEntry dte;

CTEntry cte;

ITEntry ite;

+ ItsCmdResult cmdres;

- if (devid >= s->dt.num_entries) {

- qemu_log_mask(LOG_GUEST_ERROR,

- "%s: invalid command attributes: devid %d>=%d",

- __func__, devid, s->dt.num_entries);

- return CMD_CONTINUE;

+ cmdres = lookup_ite(s, __func__, devid, eventid, &ite, &dte);

+ if (cmdres != CMD_CONTINUE_OK) {

+ return cmdres;

}

- if (get_dte(s, devid, &dte) != MEMTX_OK) {

- return CMD_STALL;

- }

- if (!dte.valid) {

- qemu_log_mask(LOG_GUEST_ERROR,

- "%s: invalid command attributes: "

- "invalid dte for %d\n", __func__, devid);

- return CMD_CONTINUE;

- }

-

- num_eventids = 1ULL << (dte.size + 1);

- if (eventid >= num_eventids) {

- qemu_log_mask(LOG_GUEST_ERROR,

- "%s: invalid command attributes: eventid %d >= %"

- PRId64 "\n",

- __func__, eventid, num_eventids);

- return CMD_CONTINUE;

- }

-

- if (get_ite(s, eventid, &dte, &ite) != MEMTX_OK) {

- return CMD_STALL;

- }

-

- if (!ite.valid || ite.inttype != ITE_INTTYPE_PHYSICAL) {

+ if (ite.inttype != ITE_INTTYPE_PHYSICAL) {

qemu_log_mask(LOG_GUEST_ERROR,

"%s: invalid command attributes: invalid ITE\n",

__func__);

@@ -XXX,XX +XXX,XX @@ static ItsCmdResult process_movi(GICv3ITSState *s, const uint64_t *cmdpkt)

{

uint32_t devid, eventid;

uint16_t new_icid;

- uint64_t num_eventids;

DTEntry dte;

CTEntry old_cte, new_cte;

ITEntry old_ite;

+ ItsCmdResult cmdres;

devid = FIELD_EX64(cmdpkt[0], MOVI_0, DEVICEID);

eventid = FIELD_EX64(cmdpkt[1], MOVI_1, EVENTID);

@@ -XXX,XX +XXX,XX @@ static ItsCmdResult process_movi(GICv3ITSState *s, const uint64_t *cmdpkt)

trace_gicv3_its_cmd_movi(devid, eventid, new_icid);

- if (devid >= s->dt.num_entries) {

- qemu_log_mask(LOG_GUEST_ERROR,

- "%s: invalid command attributes: devid %d>=%d",

- __func__, devid, s->dt.num_entries);

- return CMD_CONTINUE;

- }

- if (get_dte(s, devid, &dte) != MEMTX_OK) {

- return CMD_STALL;

+ cmdres = lookup_ite(s, __func__, devid, eventid, &old_ite, &dte);

+ if (cmdres != CMD_CONTINUE_OK) {

+ return cmdres;

}

- if (!dte.valid) {

- qemu_log_mask(LOG_GUEST_ERROR,

- "%s: invalid command attributes: "

- "invalid dte for %d\n", __func__, devid);

- return CMD_CONTINUE;

- }

-

- num_eventids = 1ULL << (dte.size + 1);

- if (eventid >= num_eventids) {

- qemu_log_mask(LOG_GUEST_ERROR,

- "%s: invalid command attributes: eventid %d >= %"

- PRId64 "\n",

- __func__, eventid, num_eventids);

- return CMD_CONTINUE;

- }

-

- if (get_ite(s, eventid, &dte, &old_ite) != MEMTX_OK) {

- return CMD_STALL;

- }

-

- if (!old_ite.valid || old_ite.inttype != ITE_INTTYPE_PHYSICAL) {

+ if (old_ite.inttype != ITE_INTTYPE_PHYSICAL) {

qemu_log_mask(LOG_GUEST_ERROR,

"%s: invalid command attributes: invalid ITE\n",

__func__);

--

2.25.1

Factor out the sequence of looking up a CTE from an ICID including

the validity and error checks.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

Message-id: 20220408141550.1271295-13-peter.maydell@linaro.org

---

hw/intc/arm_gicv3_its.c | 109 ++++++++++++++--------------------------

1 file changed, 39 insertions(+), 70 deletions(-)

diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/intc/arm_gicv3_its.c

+++ b/hw/intc/arm_gicv3_its.c

@@ -XXX,XX +XXX,XX @@ static ItsCmdResult lookup_ite(GICv3ITSState *s, const char *who,

return CMD_CONTINUE_OK;

}

+/*

+ * Given an ICID, look up the corresponding CTE, including checking for various

+ * invalid-value cases. If we find a valid CTE, fill in @cte and return

+ * CMD_CONTINUE_OK; otherwise return CMD_STALL or CMD_CONTINUE (and the

+ * contents of @cte should not be relied on).

+ *

+ * The string @who is purely for the LOG_GUEST_ERROR messages,

+ * and should indicate the name of the calling function or similar.

+ */

+static ItsCmdResult lookup_cte(GICv3ITSState *s, const char *who,

+ uint32_t icid, CTEntry *cte)

+{