From: Richard Henderson <richard.henderson@linaro.org>

Use the macro instead of two explicit string literals.

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

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Reviewed-by: Eric Auger <eric.auger@redhat.com>

Message-id: 20230124232059.4017615-1-richard.henderson@linaro.org

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

---

hw/arm/sbsa-ref.c | 3 ++-

hw/arm/virt.c | 2 +-

2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/arm/sbsa-ref.c

+++ b/hw/arm/sbsa-ref.c

@@ -XXX,XX +XXX,XX @@

#include "exec/hwaddr.h"

#include "kvm_arm.h"

#include "hw/arm/boot.h"

+#include "hw/arm/smmuv3.h"

#include "hw/block/flash.h"

#include "hw/boards.h"

#include "hw/ide/internal.h"

@@ -XXX,XX +XXX,XX @@ static void create_smmu(const SBSAMachineState *sms, PCIBus *bus)

DeviceState *dev;

int i;

- dev = qdev_new("arm-smmuv3");

+ dev = qdev_new(TYPE_ARM_SMMUV3);

object_property_set_link(OBJECT(dev), "primary-bus", OBJECT(bus),

&error_abort);

diff --git a/hw/arm/virt.c b/hw/arm/virt.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/arm/virt.c

+++ b/hw/arm/virt.c

@@ -XXX,XX +XXX,XX @@ static void create_smmu(const VirtMachineState *vms,

return;

}

- dev = qdev_new("arm-smmuv3");

+ dev = qdev_new(TYPE_ARM_SMMUV3);

object_property_set_link(OBJECT(dev), "primary-bus", OBJECT(bus),

&error_abort);

--

2.34.1

From: Richard Henderson <richard.henderson@linaro.org>

Conversion to probe_access_full missed applying the page offset.

Cc: qemu-stable@nongnu.org

Reported-by: Sid Manning <sidneym@quicinc.com>

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

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Message-id: 20230126233134.103193-1-richard.henderson@linaro.org

Fixes: f3639a64f602 ("target/arm: Use softmmu tlbs for page table walking")

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

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

---

target/arm/ptw.c | 2 +-

1 file changed, 1 insertion(+), 1 deletion(-)

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

index XXXXXXX..XXXXXXX 100644

--- a/target/arm/ptw.c

+++ b/target/arm/ptw.c

@@ -XXX,XX +XXX,XX @@ static bool S1_ptw_translate(CPUARMState *env, S1Translate *ptw,

if (unlikely(flags & TLB_INVALID_MASK)) {

goto fail;

}

- ptw->out_phys = full->phys_addr;

+ ptw->out_phys = full->phys_addr | (addr & ~TARGET_PAGE_MASK);

ptw->out_rw = full->prot & PAGE_WRITE;

pte_attrs = full->pte_attrs;

pte_secure = full->attrs.secure;

--

2.34.1

From: Evgeny Iakovlev <eiakovlev@linux.microsoft.com>

PL011 can be in either of 2 modes depending guest config: FIFO and

single register. The last mode could be viewed as a 1-element-deep FIFO.

Current code open-codes a bunch of depth-dependent logic. Refactor FIFO

depth handling code to isolate calculating current FIFO depth.

One functional (albeit guest-invisible) side-effect of this change is

that previously we would always increment s->read_pos in UARTDR read

handler even if FIFO was disabled, now we are limiting read_pos to not

exceed FIFO depth (read_pos itself is reset to 0 if user disables FIFO).

Signed-off-by: Evgeny Iakovlev <eiakovlev@linux.microsoft.com>

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

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Message-id: 20230123162304.26254-2-eiakovlev@linux.microsoft.com

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

---

include/hw/char/pl011.h | 5 ++++-

hw/char/pl011.c | 30 ++++++++++++++++++------------

2 files changed, 22 insertions(+), 13 deletions(-)

diff --git a/include/hw/char/pl011.h b/include/hw/char/pl011.h

index XXXXXXX..XXXXXXX 100644

--- a/include/hw/char/pl011.h

+++ b/include/hw/char/pl011.h

@@ -XXX,XX +XXX,XX @@ OBJECT_DECLARE_SIMPLE_TYPE(PL011State, PL011)

/* This shares the same struct (and cast macro) as the base pl011 device */

#define TYPE_PL011_LUMINARY "pl011_luminary"

+/* Depth of UART FIFO in bytes, when FIFO mode is enabled (else depth == 1) */

+#define PL011_FIFO_DEPTH 16

+

struct PL011State {

SysBusDevice parent_obj;

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

uint32_t dmacr;

uint32_t int_enabled;

uint32_t int_level;

- uint32_t read_fifo[16];

+ uint32_t read_fifo[PL011_FIFO_DEPTH];

uint32_t ilpr;

uint32_t ibrd;

uint32_t fbrd;

diff --git a/hw/char/pl011.c b/hw/char/pl011.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/char/pl011.c

+++ b/hw/char/pl011.c

@@ -XXX,XX +XXX,XX @@ static void pl011_update(PL011State *s)

}

}

+static bool pl011_is_fifo_enabled(PL011State *s)

+{

+ return (s->lcr & 0x10) != 0;

+}

+

+static inline unsigned pl011_get_fifo_depth(PL011State *s)

+{

+ /* Note: FIFO depth is expected to be power-of-2 */

+ return pl011_is_fifo_enabled(s) ? PL011_FIFO_DEPTH : 1;

+}

+

static uint64_t pl011_read(void *opaque, hwaddr offset,

unsigned size)

{

@@ -XXX,XX +XXX,XX @@ static uint64_t pl011_read(void *opaque, hwaddr offset,

c = s->read_fifo[s->read_pos];

if (s->read_count > 0) {

s->read_count--;

- if (++s->read_pos == 16)

- s->read_pos = 0;

+ s->read_pos = (s->read_pos + 1) & (pl011_get_fifo_depth(s) - 1);

}

if (s->read_count == 0) {

s->flags |= PL011_FLAG_RXFE;

@@ -XXX,XX +XXX,XX @@ static int pl011_can_receive(void *opaque)

PL011State *s = (PL011State *)opaque;

int r;

- if (s->lcr & 0x10) {

- r = s->read_count < 16;

- } else {

- r = s->read_count < 1;

- }

+ r = s->read_count < pl011_get_fifo_depth(s);

trace_pl011_can_receive(s->lcr, s->read_count, r);

return r;

}

@@ -XXX,XX +XXX,XX @@ static void pl011_put_fifo(void *opaque, uint32_t value)

{

PL011State *s = (PL011State *)opaque;

int slot;

+ unsigned pipe_depth;

- slot = s->read_pos + s->read_count;

- if (slot >= 16)

- slot -= 16;

+ pipe_depth = pl011_get_fifo_depth(s);

+ slot = (s->read_pos + s->read_count) & (pipe_depth - 1);

s->read_fifo[slot] = value;

s->read_count++;

s->flags &= ~PL011_FLAG_RXFE;

trace_pl011_put_fifo(value, s->read_count);

- if (!(s->lcr & 0x10) || s->read_count == 16) {

+ if (s->read_count == pipe_depth) {

trace_pl011_put_fifo_full();

s->flags |= PL011_FLAG_RXFF;

}

@@ -XXX,XX +XXX,XX @@ static const VMStateDescription vmstate_pl011 = {

VMSTATE_UINT32(dmacr, PL011State),

VMSTATE_UINT32(int_enabled, PL011State),

VMSTATE_UINT32(int_level, PL011State),

- VMSTATE_UINT32_ARRAY(read_fifo, PL011State, 16),

+ VMSTATE_UINT32_ARRAY(read_fifo, PL011State, PL011_FIFO_DEPTH),

VMSTATE_UINT32(ilpr, PL011State),

VMSTATE_UINT32(ibrd, PL011State),

VMSTATE_UINT32(fbrd, PL011State),

--

2.34.1

From: Evgeny Iakovlev <eiakovlev@linux.microsoft.com>

Previous change slightly modified the way we handle data writes when

FIFO is disabled. Previously we kept incrementing read_pos and were

storing data at that position, although we only have a

single-register-deep FIFO now. Then we changed it to always store data

at pos 0.

If guest disables FIFO and the proceeds to read data, it will work out

fine, because we still read from current read_pos before setting it to

0.

However, to make code less fragile, introduce a post_load hook for

PL011State and move fixup read FIFO state when FIFO is disabled. Since

we are introducing a post_load hook, also do some sanity checking on

untrusted incoming input state.

Signed-off-by: Evgeny Iakovlev <eiakovlev@linux.microsoft.com>

Message-id: 20230123162304.26254-3-eiakovlev@linux.microsoft.com

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

---

hw/char/pl011.c | 25 +++++++++++++++++++++++++

1 file changed, 25 insertions(+)

diff --git a/hw/char/pl011.c b/hw/char/pl011.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/char/pl011.c

+++ b/hw/char/pl011.c

@@ -XXX,XX +XXX,XX @@ static const VMStateDescription vmstate_pl011_clock = {

}

};

+static int pl011_post_load(void *opaque, int version_id)

+{

+ PL011State* s = opaque;

+

+ /* Sanity-check input state */

+ if (s->read_pos >= ARRAY_SIZE(s->read_fifo) ||

+ s->read_count > ARRAY_SIZE(s->read_fifo)) {

+ return -1;

+ }

+

+ if (!pl011_is_fifo_enabled(s) && s->read_count > 0 && s->read_pos > 0) {

+ /*

+ * Older versions of PL011 didn't ensure that the single

+ * character in the FIFO in FIFO-disabled mode is in

+ * element 0 of the array; convert to follow the current

+ * code's assumptions.

+ */

+ s->read_fifo[0] = s->read_fifo[s->read_pos];

+ s->read_pos = 0;

+ }

+

+ return 0;

+}

+

static const VMStateDescription vmstate_pl011 = {

.name = "pl011",

.version_id = 2,

.minimum_version_id = 2,

+ .post_load = pl011_post_load,

.fields = (VMStateField[]) {

VMSTATE_UINT32(readbuff, PL011State),

VMSTATE_UINT32(flags, PL011State),

--

2.34.1

From: Evgeny Iakovlev <eiakovlev@linux.microsoft.com>

PL011 currently lacks a reset method. Implement it.

Signed-off-by: Evgeny Iakovlev <eiakovlev@linux.microsoft.com>

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

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Message-id: 20230123162304.26254-4-eiakovlev@linux.microsoft.com

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

---

hw/char/pl011.c | 26 +++++++++++++++++++++-----

1 file changed, 21 insertions(+), 5 deletions(-)

diff --git a/hw/char/pl011.c b/hw/char/pl011.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/char/pl011.c

+++ b/hw/char/pl011.c

@@ -XXX,XX +XXX,XX @@ static void pl011_init(Object *obj)

s->clk = qdev_init_clock_in(DEVICE(obj), "clk", pl011_clock_update, s,

ClockUpdate);

- s->read_trigger = 1;

- s->ifl = 0x12;

- s->cr = 0x300;

- s->flags = 0x90;

-

s->id = pl011_id_arm;

}

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

pl011_event, NULL, s, NULL, true);

}

+static void pl011_reset(DeviceState *dev)

+{

+ PL011State *s = PL011(dev);

+

+ s->lcr = 0;

+ s->rsr = 0;

+ s->dmacr = 0;

+ s->int_enabled = 0;

+ s->int_level = 0;

+ s->ilpr = 0;

+ s->ibrd = 0;

+ s->fbrd = 0;

+ s->read_pos = 0;

+ s->read_count = 0;

+ s->read_trigger = 1;

+ s->ifl = 0x12;

+ s->cr = 0x300;

+ s->flags = 0x90;

+}

+

static void pl011_class_init(ObjectClass *oc, void *data)

{

DeviceClass *dc = DEVICE_CLASS(oc);

dc->realize = pl011_realize;

+ dc->reset = pl011_reset;

dc->vmsd = &vmstate_pl011;

device_class_set_props(dc, pl011_properties);

}

--

2.34.1

From: Evgeny Iakovlev <eiakovlev@linux.microsoft.com>

Current FIFO handling code does not reset RXFE/RXFF flags when guest

resets FIFO by writing to UARTLCR register, although internal FIFO state

is reset to 0 read count. Actual guest-visible flag update will happen

only on next data read or write attempt. As a result of that any guest

that expects RXFE flag to be set (and RXFF to be cleared) after resetting

FIFO will never see that happen.

Signed-off-by: Evgeny Iakovlev <eiakovlev@linux.microsoft.com>

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

Message-id: 20230123162304.26254-5-eiakovlev@linux.microsoft.com

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

---

hw/char/pl011.c | 18 +++++++++++++-----

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

diff --git a/hw/char/pl011.c b/hw/char/pl011.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/char/pl011.c

+++ b/hw/char/pl011.c

@@ -XXX,XX +XXX,XX @@ static inline unsigned pl011_get_fifo_depth(PL011State *s)

return pl011_is_fifo_enabled(s) ? PL011_FIFO_DEPTH : 1;

}

+static inline void pl011_reset_fifo(PL011State *s)

+{

+ s->read_count = 0;

+ s->read_pos = 0;

+

+ /* Reset FIFO flags */

+ s->flags &= ~(PL011_FLAG_RXFF | PL011_FLAG_TXFF);

+ s->flags |= PL011_FLAG_RXFE | PL011_FLAG_TXFE;

+}

+

static uint64_t pl011_read(void *opaque, hwaddr offset,

unsigned size)

{

@@ -XXX,XX +XXX,XX @@ static void pl011_write(void *opaque, hwaddr offset,

case 11: /* UARTLCR_H */

/* Reset the FIFO state on FIFO enable or disable */

if ((s->lcr ^ value) & 0x10) {

- s->read_count = 0;

- s->read_pos = 0;

+ pl011_reset_fifo(s);

}

if ((s->lcr ^ value) & 0x1) {

int break_enable = value & 0x1;

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

s->ilpr = 0;

s->ibrd = 0;

s->fbrd = 0;

- s->read_pos = 0;

- s->read_count = 0;

s->read_trigger = 1;

s->ifl = 0x12;

s->cr = 0x300;

- s->flags = 0x90;

+ s->flags = 0;

+ pl011_reset_fifo(s);

}

static void pl011_class_init(ObjectClass *oc, void *data)

--

2.34.1

From: Alexander Graf <agraf@csgraf.de>

We currently only support GICv2 emulation. To also support GICv3, we will

need to pass a few system registers into their respective handler functions.

This patch adds support for HVF to call into the TCG callbacks for GICv3

system register handlers. This is safe because the GICv3 TCG code is generic

as long as we limit ourselves to EL0 and EL1 - which are the only modes

supported by HVF.

To make sure nobody trips over that, we also annotate callbacks that don't

work in HVF mode, such as EL state change hooks.

With GICv3 support in place, we can run with more than 8 vCPUs.

Signed-off-by: Alexander Graf <agraf@csgraf.de>

Message-id: 20230128224459.70676-1-agraf@csgraf.de

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

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

---

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

target/arm/hvf/hvf.c | 151 ++++++++++++++++++++++++++++++++++++

target/arm/hvf/trace-events | 2 +

3 files changed, 168 insertions(+), 1 deletion(-)

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

index XXXXXXX..XXXXXXX 100644

--- a/hw/intc/arm_gicv3_cpuif.c

+++ b/hw/intc/arm_gicv3_cpuif.c

@@ -XXX,XX +XXX,XX @@

#include "hw/irq.h"

#include "cpu.h"

#include "target/arm/cpregs.h"

+#include "sysemu/tcg.h"

+#include "sysemu/qtest.h"

/*

* Special case return value from hppvi_index(); must be larger than

@@ -XXX,XX +XXX,XX @@ void gicv3_init_cpuif(GICv3State *s)

* which case we'd get the wrong value.

* So instead we define the regs with no ri->opaque info, and

* get back to the GICv3CPUState from the CPUARMState.

+ *

+ * These CP regs callbacks can be called from either TCG or HVF code.

*/

define_arm_cp_regs(cpu, gicv3_cpuif_reginfo);

@@ -XXX,XX +XXX,XX @@ void gicv3_init_cpuif(GICv3State *s)

define_arm_cp_regs(cpu, gicv3_cpuif_ich_apxr23_reginfo);

}

}

- arm_register_el_change_hook(cpu, gicv3_cpuif_el_change_hook, cs);

+ if (tcg_enabled() || qtest_enabled()) {

+ /*

+ * We can only trap EL changes with TCG. However the GIC interrupt

+ * state only changes on EL changes involving EL2 or EL3, so for

+ * the non-TCG case this is OK, as EL2 and EL3 can't exist.

+ */

+ arm_register_el_change_hook(cpu, gicv3_cpuif_el_change_hook, cs);

+ } else {

+ assert(!arm_feature(&cpu->env, ARM_FEATURE_EL2));

+ assert(!arm_feature(&cpu->env, ARM_FEATURE_EL3));

+ }

}

}

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

index XXXXXXX..XXXXXXX 100644

--- a/target/arm/hvf/hvf.c

+++ b/target/arm/hvf/hvf.c

@@ -XXX,XX +XXX,XX @@

#define SYSREG_PMCCNTR_EL0 SYSREG(3, 3, 9, 13, 0)

#define SYSREG_PMCCFILTR_EL0 SYSREG(3, 3, 14, 15, 7)

+#define SYSREG_ICC_AP0R0_EL1 SYSREG(3, 0, 12, 8, 4)

+#define SYSREG_ICC_AP0R1_EL1 SYSREG(3, 0, 12, 8, 5)

+#define SYSREG_ICC_AP0R2_EL1 SYSREG(3, 0, 12, 8, 6)

+#define SYSREG_ICC_AP0R3_EL1 SYSREG(3, 0, 12, 8, 7)

+#define SYSREG_ICC_AP1R0_EL1 SYSREG(3, 0, 12, 9, 0)

+#define SYSREG_ICC_AP1R1_EL1 SYSREG(3, 0, 12, 9, 1)

+#define SYSREG_ICC_AP1R2_EL1 SYSREG(3, 0, 12, 9, 2)

+#define SYSREG_ICC_AP1R3_EL1 SYSREG(3, 0, 12, 9, 3)

+#define SYSREG_ICC_ASGI1R_EL1 SYSREG(3, 0, 12, 11, 6)

+#define SYSREG_ICC_BPR0_EL1 SYSREG(3, 0, 12, 8, 3)

+#define SYSREG_ICC_BPR1_EL1 SYSREG(3, 0, 12, 12, 3)

+#define SYSREG_ICC_CTLR_EL1 SYSREG(3, 0, 12, 12, 4)

+#define SYSREG_ICC_DIR_EL1 SYSREG(3, 0, 12, 11, 1)

+#define SYSREG_ICC_EOIR0_EL1 SYSREG(3, 0, 12, 8, 1)

+#define SYSREG_ICC_EOIR1_EL1 SYSREG(3, 0, 12, 12, 1)

+#define SYSREG_ICC_HPPIR0_EL1 SYSREG(3, 0, 12, 8, 2)

+#define SYSREG_ICC_HPPIR1_EL1 SYSREG(3, 0, 12, 12, 2)

+#define SYSREG_ICC_IAR0_EL1 SYSREG(3, 0, 12, 8, 0)

+#define SYSREG_ICC_IAR1_EL1 SYSREG(3, 0, 12, 12, 0)

+#define SYSREG_ICC_IGRPEN0_EL1 SYSREG(3, 0, 12, 12, 6)

+#define SYSREG_ICC_IGRPEN1_EL1 SYSREG(3, 0, 12, 12, 7)

+#define SYSREG_ICC_PMR_EL1 SYSREG(3, 0, 4, 6, 0)

+#define SYSREG_ICC_RPR_EL1 SYSREG(3, 0, 12, 11, 3)

+#define SYSREG_ICC_SGI0R_EL1 SYSREG(3, 0, 12, 11, 7)

+#define SYSREG_ICC_SGI1R_EL1 SYSREG(3, 0, 12, 11, 5)

+#define SYSREG_ICC_SRE_EL1 SYSREG(3, 0, 12, 12, 5)

+

#define WFX_IS_WFE (1 << 0)

#define TMR_CTL_ENABLE (1 << 0)

@@ -XXX,XX +XXX,XX @@ static bool is_id_sysreg(uint32_t reg)

SYSREG_CRM(reg) < 8;

}

+static uint32_t hvf_reg2cp_reg(uint32_t reg)

+{

+ return ENCODE_AA64_CP_REG(CP_REG_ARM64_SYSREG_CP,

+ (reg >> SYSREG_CRN_SHIFT) & SYSREG_CRN_MASK,

+ (reg >> SYSREG_CRM_SHIFT) & SYSREG_CRM_MASK,

+ (reg >> SYSREG_OP0_SHIFT) & SYSREG_OP0_MASK,

+ (reg >> SYSREG_OP1_SHIFT) & SYSREG_OP1_MASK,

+ (reg >> SYSREG_OP2_SHIFT) & SYSREG_OP2_MASK);

+}

+

+static bool hvf_sysreg_read_cp(CPUState *cpu, uint32_t reg, uint64_t *val)

+{

+ ARMCPU *arm_cpu = ARM_CPU(cpu);

+ CPUARMState *env = &arm_cpu->env;

+ const ARMCPRegInfo *ri;

+

+ ri = get_arm_cp_reginfo(arm_cpu->cp_regs, hvf_reg2cp_reg(reg));

+ if (ri) {

+ if (ri->accessfn) {

+ if (ri->accessfn(env, ri, true) != CP_ACCESS_OK) {

+ return false;

+ }

+ }

+ if (ri->type & ARM_CP_CONST) {

+ *val = ri->resetvalue;

+ } else if (ri->readfn) {

+ *val = ri->readfn(env, ri);

+ } else {

+ *val = CPREG_FIELD64(env, ri);

+ }

+ trace_hvf_vgic_read(ri->name, *val);

+ return true;

+ }

+

+ return false;

+}

+

static int hvf_sysreg_read(CPUState *cpu, uint32_t reg, uint32_t rt)

{

ARMCPU *arm_cpu = ARM_CPU(cpu);

@@ -XXX,XX +XXX,XX @@ static int hvf_sysreg_read(CPUState *cpu, uint32_t reg, uint32_t rt)

case SYSREG_OSDLR_EL1:

/* Dummy register */

break;

+ case SYSREG_ICC_AP0R0_EL1:

+ case SYSREG_ICC_AP0R1_EL1:

+ case SYSREG_ICC_AP0R2_EL1:

+ case SYSREG_ICC_AP0R3_EL1:

+ case SYSREG_ICC_AP1R0_EL1:

+ case SYSREG_ICC_AP1R1_EL1:

+ case SYSREG_ICC_AP1R2_EL1:

+ case SYSREG_ICC_AP1R3_EL1:

+ case SYSREG_ICC_ASGI1R_EL1:

+ case SYSREG_ICC_BPR0_EL1:

+ case SYSREG_ICC_BPR1_EL1:

+ case SYSREG_ICC_DIR_EL1:

+ case SYSREG_ICC_EOIR0_EL1:

+ case SYSREG_ICC_EOIR1_EL1:

+ case SYSREG_ICC_HPPIR0_EL1:

+ case SYSREG_ICC_HPPIR1_EL1:

+ case SYSREG_ICC_IAR0_EL1:

+ case SYSREG_ICC_IAR1_EL1:

+ case SYSREG_ICC_IGRPEN0_EL1:

+ case SYSREG_ICC_IGRPEN1_EL1:

+ case SYSREG_ICC_PMR_EL1:

+ case SYSREG_ICC_SGI0R_EL1:

+ case SYSREG_ICC_SGI1R_EL1:

+ case SYSREG_ICC_SRE_EL1:

+ case SYSREG_ICC_CTLR_EL1:

+ /* Call the TCG sysreg handler. This is only safe for GICv3 regs. */

+ if (!hvf_sysreg_read_cp(cpu, reg, &val)) {

+ hvf_raise_exception(cpu, EXCP_UDEF, syn_uncategorized());

+ }

+ break;

default:

if (is_id_sysreg(reg)) {

/* ID system registers read as RES0 */

@@ -XXX,XX +XXX,XX @@ static void pmswinc_write(CPUARMState *env, uint64_t value)

}

}

+static bool hvf_sysreg_write_cp(CPUState *cpu, uint32_t reg, uint64_t val)

+{

+ ARMCPU *arm_cpu = ARM_CPU(cpu);

+ CPUARMState *env = &arm_cpu->env;

+ const ARMCPRegInfo *ri;

+

+ ri = get_arm_cp_reginfo(arm_cpu->cp_regs, hvf_reg2cp_reg(reg));

+

+ if (ri) {

+ if (ri->accessfn) {

+ if (ri->accessfn(env, ri, false) != CP_ACCESS_OK) {

+ return false;

+ }

+ }

+ if (ri->writefn) {

+ ri->writefn(env, ri, val);

+ } else {

+ CPREG_FIELD64(env, ri) = val;

+ }

+

+ trace_hvf_vgic_write(ri->name, val);

+ return true;

+ }

+

+ return false;

+}

+

static int hvf_sysreg_write(CPUState *cpu, uint32_t reg, uint64_t val)

{

ARMCPU *arm_cpu = ARM_CPU(cpu);

@@ -XXX,XX +XXX,XX @@ static int hvf_sysreg_write(CPUState *cpu, uint32_t reg, uint64_t val)

case SYSREG_OSDLR_EL1:

/* Dummy register */

break;

+ case SYSREG_ICC_AP0R0_EL1:

+ case SYSREG_ICC_AP0R1_EL1:

+ case SYSREG_ICC_AP0R2_EL1:

+ case SYSREG_ICC_AP0R3_EL1:

+ case SYSREG_ICC_AP1R0_EL1:

+ case SYSREG_ICC_AP1R1_EL1:

+ case SYSREG_ICC_AP1R2_EL1:

+ case SYSREG_ICC_AP1R3_EL1:

+ case SYSREG_ICC_ASGI1R_EL1:

+ case SYSREG_ICC_BPR0_EL1:

+ case SYSREG_ICC_BPR1_EL1:

+ case SYSREG_ICC_CTLR_EL1:

+ case SYSREG_ICC_DIR_EL1:

+ case SYSREG_ICC_EOIR0_EL1:

+ case SYSREG_ICC_EOIR1_EL1:

+ case SYSREG_ICC_HPPIR0_EL1:

+ case SYSREG_ICC_HPPIR1_EL1:

+ case SYSREG_ICC_IAR0_EL1:

+ case SYSREG_ICC_IAR1_EL1:

+ case SYSREG_ICC_IGRPEN0_EL1:

+ case SYSREG_ICC_IGRPEN1_EL1:

+ case SYSREG_ICC_PMR_EL1:

+ case SYSREG_ICC_SGI0R_EL1:

+ case SYSREG_ICC_SGI1R_EL1:

+ case SYSREG_ICC_SRE_EL1:

+ /* Call the TCG sysreg handler. This is only safe for GICv3 regs. */

+ if (!hvf_sysreg_write_cp(cpu, reg, val)) {

+ hvf_raise_exception(cpu, EXCP_UDEF, syn_uncategorized());

+ }

+ break;

default:

cpu_synchronize_state(cpu);

trace_hvf_unhandled_sysreg_write(env->pc, reg,

diff --git a/target/arm/hvf/trace-events b/target/arm/hvf/trace-events

index XXXXXXX..XXXXXXX 100644

--- a/target/arm/hvf/trace-events

+++ b/target/arm/hvf/trace-events

@@ -XXX,XX +XXX,XX @@ hvf_unknown_hvc(uint64_t x0) "unknown HVC! 0x%016"PRIx64

hvf_unknown_smc(uint64_t x0) "unknown SMC! 0x%016"PRIx64

hvf_exit(uint64_t syndrome, uint32_t ec, uint64_t pc) "exit: 0x%"PRIx64" [ec=0x%x pc=0x%"PRIx64"]"

hvf_psci_call(uint64_t x0, uint64_t x1, uint64_t x2, uint64_t x3, uint32_t cpuid) "PSCI Call x0=0x%016"PRIx64" x1=0x%016"PRIx64" x2=0x%016"PRIx64" x3=0x%016"PRIx64" cpu=0x%x"

+hvf_vgic_write(const char *name, uint64_t val) "vgic write to %s [val=0x%016"PRIx64"]"

+hvf_vgic_read(const char *name, uint64_t val) "vgic read from %s [val=0x%016"PRIx64"]"

--

2.34.1

From: Alexander Graf <agraf@csgraf.de>

Up to now, the finalize_gic_version() code open coded what is essentially

a support bitmap match between host/emulation environment and desired

target GIC type.

This open coding leads to undesirable side effects. For example, a VM with

KVM and -smp 10 will automatically choose GICv3 while the same command

line with TCG will stay on GICv2 and fail the launch.

This patch combines the TCG and KVM matching code paths by making

everything a 2 pass process. First, we determine which GIC versions the

current environment is able to support, then we go through a single

state machine to determine which target GIC mode that means for us.

After this patch, the only user noticable changes should be consolidated

error messages as well as TCG -M virt supporting -smp > 8 automatically.

Signed-off-by: Alexander Graf <agraf@csgraf.de>

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

Reviewed-by: Cornelia Huck <cohuck@redhat.com>

Reviewed-by: Zenghui Yu <yuzenghui@huawei.com>

Message-id: 20221223090107.98888-2-agraf@csgraf.de

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

---

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

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

2 files changed, 112 insertions(+), 101 deletions(-)

diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h

index XXXXXXX..XXXXXXX 100644

--- a/include/hw/arm/virt.h

+++ b/include/hw/arm/virt.h

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

} VirtMSIControllerType;

typedef enum VirtGICType {

- VIRT_GIC_VERSION_MAX,

- VIRT_GIC_VERSION_HOST,

- VIRT_GIC_VERSION_2,

- VIRT_GIC_VERSION_3,

- VIRT_GIC_VERSION_4,

+ VIRT_GIC_VERSION_MAX = 0,

+ VIRT_GIC_VERSION_HOST = 1,

+ /* The concrete GIC values have to match the GIC version number */

+ VIRT_GIC_VERSION_2 = 2,

+ VIRT_GIC_VERSION_3 = 3,

+ VIRT_GIC_VERSION_4 = 4,

VIRT_GIC_VERSION_NOSEL,

} VirtGICType;

+#define VIRT_GIC_VERSION_2_MASK BIT(VIRT_GIC_VERSION_2)

+#define VIRT_GIC_VERSION_3_MASK BIT(VIRT_GIC_VERSION_3)

+#define VIRT_GIC_VERSION_4_MASK BIT(VIRT_GIC_VERSION_4)

+

struct VirtMachineClass {

MachineClass parent;

bool disallow_affinity_adjustment;

diff --git a/hw/arm/virt.c b/hw/arm/virt.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/arm/virt.c

+++ b/hw/arm/virt.c

@@ -XXX,XX +XXX,XX @@ static void virt_set_memmap(VirtMachineState *vms, int pa_bits)

}

}

+static VirtGICType finalize_gic_version_do(const char *accel_name,

+ VirtGICType gic_version,

+ int gics_supported,

+ unsigned int max_cpus)

+{

+ /* Convert host/max/nosel to GIC version number */

+ switch (gic_version) {

+ case VIRT_GIC_VERSION_HOST:

+ if (!kvm_enabled()) {

+ error_report("gic-version=host requires KVM");

+ exit(1);

+ }

+

+ /* For KVM, gic-version=host means gic-version=max */

+ return finalize_gic_version_do(accel_name, VIRT_GIC_VERSION_MAX,

+ gics_supported, max_cpus);

+ case VIRT_GIC_VERSION_MAX:

+ if (gics_supported & VIRT_GIC_VERSION_4_MASK) {

+ gic_version = VIRT_GIC_VERSION_4;

+ } else if (gics_supported & VIRT_GIC_VERSION_3_MASK) {

+ gic_version = VIRT_GIC_VERSION_3;

+ } else {

+ gic_version = VIRT_GIC_VERSION_2;

+ }

+ break;

+ case VIRT_GIC_VERSION_NOSEL:

+ if ((gics_supported & VIRT_GIC_VERSION_2_MASK) &&

+ max_cpus <= GIC_NCPU) {

+ gic_version = VIRT_GIC_VERSION_2;

+ } else if (gics_supported & VIRT_GIC_VERSION_3_MASK) {

+ /*

+ * in case the host does not support v2 emulation or

+ * the end-user requested more than 8 VCPUs we now default

+ * to v3. In any case defaulting to v2 would be broken.

+ */

+ gic_version = VIRT_GIC_VERSION_3;

+ } else if (max_cpus > GIC_NCPU) {

+ error_report("%s only supports GICv2 emulation but more than 8 "

+ "vcpus are requested", accel_name);

+ exit(1);

+ }

+ break;

+ case VIRT_GIC_VERSION_2:

+ case VIRT_GIC_VERSION_3:

+ case VIRT_GIC_VERSION_4:

+ break;

+ }

+

+ /* Check chosen version is effectively supported */

+ switch (gic_version) {

+ case VIRT_GIC_VERSION_2:

+ if (!(gics_supported & VIRT_GIC_VERSION_2_MASK)) {

+ error_report("%s does not support GICv2 emulation", accel_name);

+ exit(1);

+ }

+ break;

+ case VIRT_GIC_VERSION_3:

+ if (!(gics_supported & VIRT_GIC_VERSION_3_MASK)) {

+ error_report("%s does not support GICv3 emulation", accel_name);

+ exit(1);

+ }

+ break;

+ case VIRT_GIC_VERSION_4:

+ if (!(gics_supported & VIRT_GIC_VERSION_4_MASK)) {

+ error_report("%s does not support GICv4 emulation, is virtualization=on?",

+ accel_name);

+ exit(1);

+ }

+ break;

+ default:

+ error_report("logic error in finalize_gic_version");

+ exit(1);

+ break;

+ }

+

+ return gic_version;

+}

+

/*

* finalize_gic_version - Determines the final gic_version

* according to the gic-version property

@@ -XXX,XX +XXX,XX @@ static void virt_set_memmap(VirtMachineState *vms, int pa_bits)

*/

static void finalize_gic_version(VirtMachineState *vms)

{

+ const char *accel_name = current_accel_name();

unsigned int max_cpus = MACHINE(vms)->smp.max_cpus;

+ int gics_supported = 0;

- if (kvm_enabled()) {

- int probe_bitmap;

+ /* Determine which GIC versions the current environment supports */

+ if (kvm_enabled() && kvm_irqchip_in_kernel()) {

+ int probe_bitmap = kvm_arm_vgic_probe();

- if (!kvm_irqchip_in_kernel()) {

- switch (vms->gic_version) {

- case VIRT_GIC_VERSION_HOST:

- warn_report(

- "gic-version=host not relevant with kernel-irqchip=off "

- "as only userspace GICv2 is supported. Using v2 ...");

- return;

- case VIRT_GIC_VERSION_MAX:

- case VIRT_GIC_VERSION_NOSEL:

- vms->gic_version = VIRT_GIC_VERSION_2;

- return;

- case VIRT_GIC_VERSION_2:

- return;

- case VIRT_GIC_VERSION_3:

- error_report(

- "gic-version=3 is not supported with kernel-irqchip=off");

- exit(1);

- case VIRT_GIC_VERSION_4:

- error_report(

- "gic-version=4 is not supported with kernel-irqchip=off");

- exit(1);

- }

- }

-

- probe_bitmap = kvm_arm_vgic_probe();

if (!probe_bitmap) {

error_report("Unable to determine GIC version supported by host");

exit(1);

}

- switch (vms->gic_version) {

- case VIRT_GIC_VERSION_HOST:

- case VIRT_GIC_VERSION_MAX:

- if (probe_bitmap & KVM_ARM_VGIC_V3) {

- vms->gic_version = VIRT_GIC_VERSION_3;

- } else {

- vms->gic_version = VIRT_GIC_VERSION_2;

- }

- return;

- case VIRT_GIC_VERSION_NOSEL:

- if ((probe_bitmap & KVM_ARM_VGIC_V2) && max_cpus <= GIC_NCPU) {

- vms->gic_version = VIRT_GIC_VERSION_2;

- } else if (probe_bitmap & KVM_ARM_VGIC_V3) {

- /*

- * in case the host does not support v2 in-kernel emulation or

- * the end-user requested more than 8 VCPUs we now default

- * to v3. In any case defaulting to v2 would be broken.

- */

- vms->gic_version = VIRT_GIC_VERSION_3;

- } else if (max_cpus > GIC_NCPU) {

- error_report("host only supports in-kernel GICv2 emulation "

- "but more than 8 vcpus are requested");

- exit(1);

- }

- break;

- case VIRT_GIC_VERSION_2:

- case VIRT_GIC_VERSION_3:

- break;

- case VIRT_GIC_VERSION_4:

- error_report("gic-version=4 is not supported with KVM");

- exit(1);

+ if (probe_bitmap & KVM_ARM_VGIC_V2) {

+ gics_supported |= VIRT_GIC_VERSION_2_MASK;

}

-

- /* Check chosen version is effectively supported by the host */

- if (vms->gic_version == VIRT_GIC_VERSION_2 &&

- !(probe_bitmap & KVM_ARM_VGIC_V2)) {

- error_report("host does not support in-kernel GICv2 emulation");

- exit(1);

- } else if (vms->gic_version == VIRT_GIC_VERSION_3 &&

- !(probe_bitmap & KVM_ARM_VGIC_V3)) {

- error_report("host does not support in-kernel GICv3 emulation");

- exit(1);

+ if (probe_bitmap & KVM_ARM_VGIC_V3) {

+ gics_supported |= VIRT_GIC_VERSION_3_MASK;

}

- return;

- }

-

- /* TCG mode */

- switch (vms->gic_version) {

- case VIRT_GIC_VERSION_NOSEL:

- vms->gic_version = VIRT_GIC_VERSION_2;

- break;

- case VIRT_GIC_VERSION_MAX:

+ } else if (kvm_enabled() && !kvm_irqchip_in_kernel()) {

+ /* KVM w/o kernel irqchip can only deal with GICv2 */

+ gics_supported |= VIRT_GIC_VERSION_2_MASK;

+ accel_name = "KVM with kernel-irqchip=off";

+ } else {

+ gics_supported |= VIRT_GIC_VERSION_2_MASK;

if (module_object_class_by_name("arm-gicv3")) {

- /* CONFIG_ARM_GICV3_TCG was set */

+ gics_supported |= VIRT_GIC_VERSION_3_MASK;

if (vms->virt) {

/* GICv4 only makes sense if CPU has EL2 */

- vms->gic_version = VIRT_GIC_VERSION_4;

- } else {

- vms->gic_version = VIRT_GIC_VERSION_3;

+ gics_supported |= VIRT_GIC_VERSION_4_MASK;

}

- } else {

- vms->gic_version = VIRT_GIC_VERSION_2;

}

- break;

- case VIRT_GIC_VERSION_HOST:

- error_report("gic-version=host requires KVM");

- exit(1);

- case VIRT_GIC_VERSION_4:

- if (!vms->virt) {

- error_report("gic-version=4 requires virtualization enabled");

- exit(1);

- }

- break;

- case VIRT_GIC_VERSION_2:

- case VIRT_GIC_VERSION_3:

- break;

}

+

+ /*

+ * Then convert helpers like host/max to concrete GIC versions and ensure

+ * the desired version is supported

+ */

+ vms->gic_version = finalize_gic_version_do(accel_name, vms->gic_version,

+ gics_supported, max_cpus);

}

/*

--

2.34.1

From: Alexander Graf <agraf@csgraf.de>

Let's explicitly list out all accelerators that we support when trying to

determine the supported set of GIC versions. KVM was already separate, so

the only missing one is HVF which simply reuses all of TCG's emulation

code and thus has the same compatibility matrix.

Signed-off-by: Alexander Graf <agraf@csgraf.de>

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Reviewed-by: Cornelia Huck <cohuck@redhat.com>

Reviewed-by: Zenghui Yu <yuzenghui@huawei.com>

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

Message-id: 20221223090107.98888-3-agraf@csgraf.de

[PMM: Added qtest to the list of accelerators]

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

---

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

1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/arm/virt.c

+++ b/hw/arm/virt.c

@@ -XXX,XX +XXX,XX @@

#include "sysemu/numa.h"

#include "sysemu/runstate.h"

#include "sysemu/tpm.h"

+#include "sysemu/tcg.h"

#include "sysemu/kvm.h"

#include "sysemu/hvf.h"

+#include "sysemu/qtest.h"

#include "hw/loader.h"

#include "qapi/error.h"

#include "qemu/bitops.h"

@@ -XXX,XX +XXX,XX @@ static void finalize_gic_version(VirtMachineState *vms)

/* KVM w/o kernel irqchip can only deal with GICv2 */

gics_supported |= VIRT_GIC_VERSION_2_MASK;

accel_name = "KVM with kernel-irqchip=off";

- } else {

+ } else if (tcg_enabled() || hvf_enabled() || qtest_enabled()) {

gics_supported |= VIRT_GIC_VERSION_2_MASK;

if (module_object_class_by_name("arm-gicv3")) {

gics_supported |= VIRT_GIC_VERSION_3_MASK;

@@ -XXX,XX +XXX,XX @@ static void finalize_gic_version(VirtMachineState *vms)

gics_supported |= VIRT_GIC_VERSION_4_MASK;

}

}

+ } else {

+ error_report("Unsupported accelerator, can not determine GIC support");

+ exit(1);

}

/*

--

2.34.1

From: Marcin Juszkiewicz <marcin.juszkiewicz@linaro.org>

Cortex-A76 supports 40bits of address space. sbsa-ref's memory

starts above this limit.

Signed-off-by: Marcin Juszkiewicz <marcin.juszkiewicz@linaro.org>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

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

Message-id: 20230126114416.2447685-1-marcin.juszkiewicz@linaro.org

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

---

hw/arm/sbsa-ref.c | 1 -

1 file changed, 1 deletion(-)

diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/arm/sbsa-ref.c

+++ b/hw/arm/sbsa-ref.c

@@ -XXX,XX +XXX,XX @@ static const int sbsa_ref_irqmap[] = {

static const char * const valid_cpus[] = {

ARM_CPU_TYPE_NAME("cortex-a57"),

ARM_CPU_TYPE_NAME("cortex-a72"),

- ARM_CPU_TYPE_NAME("cortex-a76"),

ARM_CPU_TYPE_NAME("neoverse-n1"),

ARM_CPU_TYPE_NAME("max"),

};

--

2.34.1

The encodings 0,0,C7,C9,0 and 0,0,C7,C9,1 are AT SP1E1RP and AT

S1E1WP, but our ARMCPRegInfo definitions for them incorrectly name

them AT S1E1R and AT S1E1W (which are entirely different

instructions). Fix the names.

(This has no guest-visible effect as the names are for debug purposes

only.)

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

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

Tested-by: Fuad Tabba <tabba@google.com>

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

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

---

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

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

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

index XXXXXXX..XXXXXXX 100644

--- a/target/arm/helper.c

+++ b/target/arm/helper.c

@@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo vhe_reginfo[] = {

#ifndef CONFIG_USER_ONLY

static const ARMCPRegInfo ats1e1_reginfo[] = {

- { .name = "AT_S1E1R", .state = ARM_CP_STATE_AA64,

+ { .name = "AT_S1E1RP", .state = ARM_CP_STATE_AA64,

.opc0 = 1, .opc1 = 0, .crn = 7, .crm = 9, .opc2 = 0,

.access = PL1_W, .type = ARM_CP_NO_RAW | ARM_CP_RAISES_EXC,

.writefn = ats_write64 },

- { .name = "AT_S1E1W", .state = ARM_CP_STATE_AA64,

+ { .name = "AT_S1E1WP", .state = ARM_CP_STATE_AA64,

.opc0 = 1, .opc1 = 0, .crn = 7, .crm = 9, .opc2 = 1,

.access = PL1_W, .type = ARM_CP_NO_RAW | ARM_CP_RAISES_EXC,

.writefn = ats_write64 },

--

2.34.1

The AArch32 ATS12NSO* address translation operations are supposed to

trap to either EL2 or EL3 if they're executed at Secure EL1 (which

can only happen if EL3 is AArch64). We implement this, but we got

the syndrome value wrong: like other traps to EL2 or EL3 on an

AArch32 cpreg access, they should report the 0x3 syndrome, not the

0x0 'uncategorized' syndrome. This is clear in the access pseudocode

for these instructions.

Fix the syndrome value for these operations by correcting the

returned value from the ats_access() function.

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

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

Tested-by: Fuad Tabba <tabba@google.com>

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

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

---

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

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

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

index XXXXXXX..XXXXXXX 100644

--- a/target/arm/helper.c

+++ b/target/arm/helper.c

@@ -XXX,XX +XXX,XX @@ static CPAccessResult ats_access(CPUARMState *env, const ARMCPRegInfo *ri,

if (arm_current_el(env) == 1) {

if (arm_is_secure_below_el3(env)) {

if (env->cp15.scr_el3 & SCR_EEL2) {

- return CP_ACCESS_TRAP_UNCATEGORIZED_EL2;

+ return CP_ACCESS_TRAP_EL2;

}

- return CP_ACCESS_TRAP_UNCATEGORIZED_EL3;

+ return CP_ACCESS_TRAP_EL3;

}

return CP_ACCESS_TRAP_UNCATEGORIZED;

}

--

2.34.1

We added the CPAccessResult values CP_ACCESS_TRAP_UNCATEGORIZED_EL2

and CP_ACCESS_TRAP_UNCATEGORIZED_EL3 purely in order to use them in

the ats_access() function, but doing so was incorrect (a bug fixed in

a previous commit). There aren't any cases where we want an access

function to be able to request a trap to EL2 or EL3 with a zero

syndrome value, so remove these enum values.

As well as cleaning up dead code, the motivation here is that

we'd like to implement fine-grained-trap handling in

helper_access_check_cp_reg(). Although the fine-grained traps

to EL2 are always lower priority than trap-to-same-EL and

higher priority than trap-to-EL3, they are in the middle of

various other kinds of trap-to-EL2. Knowing that a trap-to-EL2

must always for us have the same syndrome (ie that an access

function will return CP_ACCESS_TRAP_EL2 and there is no other

kind of trap-to-EL2 enum value) means we don't have to try

to choose which of the two syndrome values to report if the

access would trap to EL2 both for the fine-grained-trap and

because the access function requires it.

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

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

Tested-by: Fuad Tabba <tabba@google.com>

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

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

---

target/arm/cpregs.h | 4 ++--

target/arm/op_helper.c | 2 ++

2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/target/arm/cpregs.h b/target/arm/cpregs.h

index XXXXXXX..XXXXXXX 100644

--- a/target/arm/cpregs.h