arm pullreq for rc1. All minor bugfixes, except for the sve-default-vector-length

patches, which are somewhere between a bugfix and a new feature.

Merge remote-tracking branch 'remotes/rth-gitlab/tags/pull-tcg-20210726' into staging (2021-07-27 08:35:01 +0100)

https://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20210727

for you to fetch changes up to e229a179a503f2aee43a76888cf12fbdfe8a3749:

hw: aspeed_gpio: Fix memory size (2021-07-27 11:00:00 +0100)

* hw/arm/smmuv3: Check 31st bit to see if CD is valid

* qemu-options.hx: Fix formatting of -machine memory-backend option

* hw: aspeed_gpio: Fix memory size

* hw/arm/nseries: Display hexadecimal value with '0x' prefix

* Add sve-default-vector-length cpu property

* docs: Update path that mentions deprecated.rst

* hw/intc/armv7m_nvic: for v8.1M VECTPENDING hides S exceptions from NS

* hw/intc/armv7m_nvic: Correct size of ICSR.VECTPENDING

* hw/intc/armv7m_nvic: ISCR.ISRPENDING is set for non-enabled pending interrupts

* target/arm: Report M-profile alignment faults correctly to the guest

* target/arm: Add missing 'return's after calling v7m_exception_taken()

* target/arm: Enforce that M-profile SP low 2 bits are always zero

Joe Komlodi (1):

hw/arm/smmuv3: Check 31st bit to see if CD is valid

Joel Stanley (1):

hw: aspeed_gpio: Fix memory size

Mao Zhongyi (1):

docs: Update path that mentions deprecated.rst

Peter Maydell (7):

qemu-options.hx: Fix formatting of -machine memory-backend option

target/arm: Enforce that M-profile SP low 2 bits are always zero

target/arm: Add missing 'return's after calling v7m_exception_taken()

target/arm: Report M-profile alignment faults correctly to the guest

hw/intc/armv7m_nvic: ISCR.ISRPENDING is set for non-enabled pending interrupts

hw/intc/armv7m_nvic: Correct size of ICSR.VECTPENDING

hw/intc/armv7m_nvic: for v8.1M VECTPENDING hides S exceptions from NS

Philippe Mathieu-Daudé (1):

hw/arm/nseries: Display hexadecimal value with '0x' prefix

Richard Henderson (3):

target/arm: Correctly bound length in sve_zcr_get_valid_len

target/arm: Export aarch64_sve_zcr_get_valid_len

target/arm: Add sve-default-vector-length cpu property

From: Joe Komlodi <joe.komlodi@xilinx.com>

The bit to see if a CD is valid is the last bit of the first word of the CD.

Signed-off-by: Joe Komlodi <joe.komlodi@xilinx.com>

Message-id: 1626728232-134665-2-git-send-email-joe.komlodi@xilinx.com

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

hw/arm/smmuv3-internal.h | 2 +-

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

diff --git a/hw/arm/smmuv3-internal.h b/hw/arm/smmuv3-internal.h

--- a/hw/arm/smmuv3-internal.h

+++ b/hw/arm/smmuv3-internal.h

@@ -XXX,XX +XXX,XX @@ static inline int pa_range(STE *ste)

/* CD fields */

-#define CD_VALID(x) extract32((x)->word[0], 30, 1)

+#define CD_VALID(x) extract32((x)->word[0], 31, 1)

#define CD_ASID(x) extract32((x)->word[1], 16, 16)

#define CD_TTB(x, sel) \

({ \

2.20.1

For M-profile, unlike A-profile, the low 2 bits of SP are defined to be

RES0H, which is to say that they must be hardwired to zero so that

guest attempts to write non-zero values to them are ignored.

Implement this behaviour by masking out the low bits:

* for writes to r13 by the gdbstub

* for writes to any of the various flavours of SP via MSR

* for writes to r13 via store_reg() in generated code

Note that all the direct uses of cpu_R[] in translate.c are in places

where the register is definitely not r13 (usually because that has

been checked for as an UNDEFINED or UNPREDICTABLE case and handled as

UNDEF).

All the other writes to regs[13] in C code are either:

* A-profile only code

* writes of values we can guarantee to be aligned, such as

- writes of previous-SP-value plus or minus a 4-aligned constant

- writes of the value in an SP limit register (which we already

enforce to be aligned)

target/arm/gdbstub.c | 4 ++++

target/arm/m_helper.c | 14 ++++++++------

target/arm/translate.c | 3 +++

3 files changed, 15 insertions(+), 6 deletions(-)

@@ -XXX,XX +XXX,XX @@ void store_reg(DisasContext *s, int reg, TCGv_i32 var)

*/

tcg_gen_andi_i32(var, var, s->thumb ? ~1 : ~3);

s->base.is_jmp = DISAS_JUMP;

+ } else if (reg == 13 && arm_dc_feature(s, ARM_FEATURE_M)) {

+ /* For M-profile SP bits [1:0] are always zero */

+ tcg_gen_andi_i32(var, var, ~3);

}

tcg_gen_mov_i32(cpu_R[reg], var);

tcg_temp_free_i32(var);

2.20.1

In do_v7m_exception_exit(), we perform various checks as part of

performing the exception return. If one of these checks fails, the

architecture requires that we take an appropriate exception on the

existing stackframe. We implement this by calling

v7m_exception_taken() to set up to take the new exception, and then

immediately returning from do_v7m_exception_exit() without proceeding

any further with the unstack-and-exception-return process.

In a couple of checks that are new in v8.1M, we forgot the "return"

statement, with the effect that if bad code in the guest tripped over

these checks we would set up to take a UsageFault exception but then

blunder on trying to also unstack and return from the original

exception, with the probable result that the guest would crash.

Add the missing return statements.

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

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

target/arm/m_helper.c | 2 ++

1 file changed, 2 insertions(+)

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

--- a/target/arm/m_helper.c

+++ b/target/arm/m_helper.c

@@ -XXX,XX +XXX,XX @@ static void do_v7m_exception_exit(ARMCPU *cpu)

qemu_log_mask(CPU_LOG_INT, "...taking UsageFault on existing "

"stackframe: NSACR prevents clearing FPU registers\n");

v7m_exception_taken(cpu, excret, true, false);

+ return;

} else if (!cpacr_pass) {

armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_USAGE,

exc_secure);

@@ -XXX,XX +XXX,XX @@ static void do_v7m_exception_exit(ARMCPU *cpu)

qemu_log_mask(CPU_LOG_INT, "...taking UsageFault on existing "

"stackframe: CPACR prevents clearing FPU registers\n");

v7m_exception_taken(cpu, excret, true, false);

+ return;

/* Clear s0..s15, FPSCR and VPR */

2.20.1

The ISCR.ISRPENDING bit is set when an external interrupt is pending.

This is true whether that external interrupt is enabled or not.

This means that we can't use 's->vectpending == 0' as a shortcut to

"ISRPENDING is zero", because s->vectpending indicates only the

highest priority pending enabled interrupt.

Remove the incorrect optimization so that if there is no pending

enabled interrupt we fall through to scanning through the whole

interrupt array.

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

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

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

---

hw/intc/armv7m_nvic.c | 9 ++++-----

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

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

--- a/hw/intc/armv7m_nvic.c

+++ b/hw/intc/armv7m_nvic.c

@@ -XXX,XX +XXX,XX @@ static bool nvic_isrpending(NVICState *s)

{

int irq;

- /* We can shortcut if the highest priority pending interrupt

- * happens to be external or if there is nothing pending.

+ /*

+ * We can shortcut if the highest priority pending interrupt

+ * happens to be external; if not we need to check the whole

+ * vectors[] array.

*/

if (s->vectpending > NVIC_FIRST_IRQ) {

return true;

}

- if (s->vectpending == 0) {

- return false;

for (irq = NVIC_FIRST_IRQ; irq < s->num_irq; irq++) {

if (s->vectors[irq].pending) {

2.20.1

The VECTPENDING field in the ICSR is 9 bits wide, in bits [20:12] of

the register. We were incorrectly masking it to 8 bits, so it would

report the wrong value if the pending exception was greater than 256.

Fix the bug.

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

hw/intc/armv7m_nvic.c | 2 +-

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

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

--- a/hw/intc/armv7m_nvic.c

+++ b/hw/intc/armv7m_nvic.c

@@ -XXX,XX +XXX,XX @@ static uint32_t nvic_readl(NVICState *s, uint32_t offset, MemTxAttrs attrs)

/* VECTACTIVE */

val = cpu->env.v7m.exception;

/* VECTPENDING */

- val |= (s->vectpending & 0xff) << 12;

+ val |= (s->vectpending & 0x1ff) << 12;

/* ISRPENDING - set if any external IRQ is pending */

if (nvic_isrpending(s)) {

val |= (1 << 22);

2.20.1

In Arm v8.1M the VECTPENDING field in the ICSR has new behaviour: if

the register is accessed NonSecure and the highest priority pending

enabled exception (that would be returned in the VECTPENDING field)

targets Secure, then the VECTPENDING field must read 1 rather than

the exception number of the pending exception. Implement this.

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

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

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

---

hw/intc/armv7m_nvic.c | 31 ++++++++++++++++++++++++-------

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

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

--- a/hw/intc/armv7m_nvic.c

+++ b/hw/intc/armv7m_nvic.c

@@ -XXX,XX +XXX,XX @@ void armv7m_nvic_acknowledge_irq(void *opaque)

nvic_irq_update(s);

+static bool vectpending_targets_secure(NVICState *s)

+ /* Return true if s->vectpending targets Secure state */

+ if (s->vectpending_is_s_banked) {

+ return true;

+ }

+ return !exc_is_banked(s->vectpending) &&

+ exc_targets_secure(s, s->vectpending);

void armv7m_nvic_get_pending_irq_info(void *opaque,

int *pirq, bool *ptargets_secure)

2.20.1

From: Mao Zhongyi <maozhongyi@cmss.chinamobile.com>

Missed in commit f3478392 "docs: Move deprecation, build

and license info out of system/"

Signed-off-by: Mao Zhongyi <maozhongyi@cmss.chinamobile.com>

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

Message-id: 20210723065828.1336760-1-maozhongyi@cmss.chinamobile.com

configure | 2 +-

target/i386/cpu.c | 2 +-

MAINTAINERS | 2 +-

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

diff --git a/configure b/configure

index XXXXXXX..XXXXXXX 100755

--- a/configure

+++ b/configure

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

if test -n "${deprecated_features}"; then

echo "Warning, deprecated features enabled."

- echo "Please see docs/system/deprecated.rst"

+ echo "Please see docs/about/deprecated.rst"

echo " features: ${deprecated_features}"

fi

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

--- a/target/i386/cpu.c

+++ b/target/i386/cpu.c

@@ -XXX,XX +XXX,XX @@ static const X86CPUDefinition builtin_x86_defs[] = {

* none", but this is just for compatibility while libvirt isn't

* adapted to resolve CPU model versions before creating VMs.

* See "Runnability guarantee of CPU models" at

- * docs/system/deprecated.rst.

+ * docs/about/deprecated.rst.

*/

X86CPUVersion default_cpu_version = 1;

diff --git a/MAINTAINERS b/MAINTAINERS

index XXXXXXX..XXXXXXX 100644

--- a/MAINTAINERS

+++ b/MAINTAINERS

@@ -XXX,XX +XXX,XX @@ F: contrib/gitdm/*

Incompatible changes

R: libvir-list@redhat.com

-F: docs/system/deprecated.rst

+F: docs/about/deprecated.rst

Build System

------------

2.20.1

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

Currently, our only caller is sve_zcr_len_for_el, which has

already masked the length extracted from ZCR_ELx, so the

masking done here is a nop. But we will shortly have uses

from other locations, where the length will be unmasked.

Saturate the length to ARM_MAX_VQ instead of truncating to

the low 4 bits.

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

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

@@ -XXX,XX +XXX,XX @@ static uint32_t sve_zcr_get_valid_len(ARMCPU *cpu, uint32_t start_len)

{

uint32_t end_len;

- end_len = start_len &= 0xf;

+ start_len = MIN(start_len, ARM_MAX_VQ - 1);

+ end_len = start_len;

+

if (!test_bit(start_len, cpu->sve_vq_map)) {

end_len = find_last_bit(cpu->sve_vq_map, start_len);

assert(end_len < start_len);

2.20.1

Rename from sve_zcr_get_valid_len and make accessible

from outside of helper.c.

target/arm/internals.h | 10 ++++++++++

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

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

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

--- a/target/arm/internals.h

+++ b/target/arm/internals.h

@@ -XXX,XX +XXX,XX @@ void arm_translate_init(void);

void arm_cpu_synchronize_from_tb(CPUState *cs, const TranslationBlock *tb);

#endif /* CONFIG_TCG */

+/**

+ * aarch64_sve_zcr_get_valid_len:

+ * @cpu: cpu context

+ * @start_len: maximum len to consider

+ *

+ * Return the maximum supported sve vector length <= @start_len.

+ * Note that both @start_len and the return value are in units

+ * of ZCR_ELx.LEN, so the vector bit length is (x + 1) * 128.

+ */

+uint32_t aarch64_sve_zcr_get_valid_len(ARMCPU *cpu, uint32_t start_len);

enum arm_fprounding {

FPROUNDING_TIEEVEN,

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 @@ int sve_exception_el(CPUARMState *env, int el)

return 0;

-static uint32_t sve_zcr_get_valid_len(ARMCPU *cpu, uint32_t start_len)

+uint32_t aarch64_sve_zcr_get_valid_len(ARMCPU *cpu, uint32_t start_len)

{

uint32_t end_len;

@@ -XXX,XX +XXX,XX @@ uint32_t sve_zcr_len_for_el(CPUARMState *env, int el)

zcr_len = MIN(zcr_len, 0xf & (uint32_t)env->vfp.zcr_el[3]);

}

- return sve_zcr_get_valid_len(cpu, zcr_len);

+ return aarch64_sve_zcr_get_valid_len(cpu, zcr_len);

}

static void zcr_write(CPUARMState *env, const ARMCPRegInfo *ri,

2.20.1

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

Mirror the behavour of /proc/sys/abi/sve_default_vector_length

under the real linux kernel. We have no way of passing along

a real default across exec like the kernel can, but this is a

decent way of adjusting the startup vector length of a process.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/482

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

docs/system/arm/cpu-features.rst | 15 ++++++++

target/arm/cpu.h | 5 +++

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

target/arm/cpu64.c | 60 ++++++++++++++++++++++++++++++++

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

diff --git a/docs/system/arm/cpu-features.rst b/docs/system/arm/cpu-features.rst

--- a/docs/system/arm/cpu-features.rst

+++ b/docs/system/arm/cpu-features.rst

@@ -XXX,XX +XXX,XX @@ verbose command lines. However, the recommended way to select vector

lengths is to explicitly enable each desired length. Therefore only

example's (1), (4), and (6) exhibit recommended uses of the properties.

+SVE User-mode Default Vector Length Property

+--------------------------------------------

+

+For qemu-aarch64, the cpu property ``sve-default-vector-length=N`` is

+defined to mirror the Linux kernel parameter file

+``/proc/sys/abi/sve_default_vector_length``. The default length, ``N``,

+is in units of bytes and must be between 16 and 8192.

+If not specified, the default vector length is 64.

+

+If the default length is larger than the maximum vector length enabled,

+the actual vector length will be reduced. Note that the maximum vector

+length supported by QEMU is 256.

+

+If this property is set to ``-1`` then the default vector length

+is set to the maximum possible length.

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

index XXXXXXX..XXXXXXX 100644

--- a/target/arm/cpu.h

+++ b/target/arm/cpu.h

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

/* Used to set the maximum vector length the cpu will support. */

uint32_t sve_max_vq;

+#ifdef CONFIG_USER_ONLY

+ /* Used to set the default vector length at process start. */

+ uint32_t sve_default_vq;

+#endif

+

/*

* In sve_vq_map each set bit is a supported vector length of

* (bit-number + 1) * 16 bytes, i.e. each bit number + 1 is the vector

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_reset(DeviceState *dev)

env->cp15.cpacr_el1 = deposit64(env->cp15.cpacr_el1, 16, 2, 3);

/* with reasonable vector length */

if (cpu_isar_feature(aa64_sve, cpu)) {

- env->vfp.zcr_el[1] = MIN(cpu->sve_max_vq - 1, 3);

+ env->vfp.zcr_el[1] =

+ aarch64_sve_zcr_get_valid_len(cpu, cpu->sve_default_vq - 1);

}

/*

* Enable TBI0 but not TBI1.

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

QLIST_INIT(&cpu->pre_el_change_hooks);

QLIST_INIT(&cpu->el_change_hooks);

-#ifndef CONFIG_USER_ONLY

+#ifdef CONFIG_USER_ONLY

+# ifdef TARGET_AARCH64

+ /*

+ * The linux kernel defaults to 512-bit vectors, when sve is supported.

+ * See documentation for /proc/sys/abi/sve_default_vector_length, and

+ * our corresponding sve-default-vector-length cpu property.

+ */

+ cpu->sve_default_vq = 4;

+# endif

+#else

/* Our inbound IRQ and FIQ lines */

if (kvm_enabled()) {

/* VIRQ and VFIQ are unused with KVM but we add them to maintain

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

index XXXXXXX..XXXXXXX 100644

--- a/target/arm/cpu64.c

+++ b/target/arm/cpu64.c

@@ -XXX,XX +XXX,XX @@ static void cpu_arm_set_sve(Object *obj, bool value, Error **errp)

cpu->isar.id_aa64pfr0 = t;

}

+#ifdef CONFIG_USER_ONLY

+/* Mirror linux /proc/sys/abi/sve_default_vector_length. */

+static void cpu_arm_set_sve_default_vec_len(Object *obj, Visitor *v,

+ const char *name, void *opaque,

+ Error **errp)

+{

+ ARMCPU *cpu = ARM_CPU(obj);

+ int32_t default_len, default_vq, remainder;

+

+ if (!visit_type_int32(v, name, &default_len, errp)) {

+ /* Undocumented, but the kernel allows -1 to indicate "maximum". */

+ if (default_len == -1) {

+ cpu->sve_default_vq = ARM_MAX_VQ;

+ default_vq = default_len / 16;

+ remainder = default_len % 16;

+

+ /*

+ * Note that the 512 max comes from include/uapi/asm/sve_context.h

+ * and is the maximum architectural width of ZCR_ELx.LEN.

+ */

+ if (remainder || default_vq < 1 || default_vq > 512) {

+ error_setg(errp, "cannot set sve-default-vector-length");

+ if (remainder) {

+ error_append_hint(errp, "Vector length not a multiple of 16\n");

+ } else if (default_vq < 1) {

+ error_append_hint(errp, "Vector length smaller than 16\n");

+ } else {

+ error_append_hint(errp, "Vector length larger than %d\n",

+ 512 * 16);

+ }

+ return;

+ }

+

+ cpu->sve_default_vq = default_vq;

+}

+

+static void cpu_arm_get_sve_default_vec_len(Object *obj, Visitor *v,

+ const char *name, void *opaque,

+ Error **errp)

+{

+ ARMCPU *cpu = ARM_CPU(obj);

+ int32_t value = cpu->sve_default_vq * 16;

+

+ visit_type_int32(v, name, &value, errp);

+}

+#endif

+

void aarch64_add_sve_properties(Object *obj)

{

uint32_t vq;

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

object_property_add(obj, name, "bool", cpu_arm_get_sve_vq,

cpu_arm_set_sve_vq, NULL, NULL);

}

+

+#ifdef CONFIG_USER_ONLY

+ /* Mirror linux /proc/sys/abi/sve_default_vector_length. */

+ object_property_add(obj, "sve-default-vector-length", "int32",

+ cpu_arm_get_sve_default_vec_len,

+ cpu_arm_set_sve_default_vec_len, NULL, NULL);

+#endif

}

void arm_cpu_pauth_finalize(ARMCPU *cpu, Error **errp)

2.20.1

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

hw/arm/nseries.c | 2 +-

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

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

--- a/hw/arm/nseries.c

+++ b/hw/arm/nseries.c

@@ -XXX,XX +XXX,XX @@ static uint32_t mipid_txrx(void *opaque, uint32_t cmd, int len)

default:

bad_cmd:

qemu_log_mask(LOG_GUEST_ERROR,

- "%s: unknown command %02x\n", __func__, s->cmd);