* [PATCH 0/2] target/arm: Add sve-default-vector-length cpu property @ 2021-07-14 18:06 Richard Henderson 2021-07-14 18:06 ` [PATCH 1/2] target/arm: Export aarch64_sve_zcr_get_valid_len Richard Henderson 2021-07-14 18:06 ` [PATCH 2/2] target/arm: Add sve-default-vector-length cpu property Richard Henderson 0 siblings, 2 replies; 6+ messages in thread From: Richard Henderson @ 2021-07-14 18:06 UTC (permalink / raw) To: qemu-devel; +Cc: peter.maydell This is intended to resolve #482. I'm not sure where to document this. The convenient place would be in docs/system/arm/cpu-features.rst, where all of the rest of the sve documentation lives. But that's explicitly system-only, and this new properly is explicitly user-only. Suggestions? r~ Richard Henderson (2): target/arm: Export aarch64_sve_zcr_get_valid_len target/arm: Add sve-default-vector-length cpu property target/arm/cpu.h | 7 ++++++ target/arm/cpu.c | 14 ++++++++++-- target/arm/cpu64.c | 53 +++++++++++++++++++++++++++++++++++++++++++++ target/arm/helper.c | 8 ++++--- 4 files changed, 77 insertions(+), 5 deletions(-) -- 2.25.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] target/arm: Export aarch64_sve_zcr_get_valid_len 2021-07-14 18:06 [PATCH 0/2] target/arm: Add sve-default-vector-length cpu property Richard Henderson @ 2021-07-14 18:06 ` Richard Henderson 2021-07-16 8:53 ` Peter Maydell 2021-07-14 18:06 ` [PATCH 2/2] target/arm: Add sve-default-vector-length cpu property Richard Henderson 1 sibling, 1 reply; 6+ messages in thread From: Richard Henderson @ 2021-07-14 18:06 UTC (permalink / raw) To: qemu-devel; +Cc: peter.maydell Rename from sve_zcr_get_valid_len and make accessible from outside of helper.c. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- target/arm/cpu.h | 2 ++ target/arm/helper.c | 8 +++++--- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/target/arm/cpu.h b/target/arm/cpu.h index be9a4dceae..52e99344c5 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -1060,6 +1060,8 @@ int arm_cpu_write_elf64_note(WriteCoreDumpFunction f, CPUState *cs, int arm_cpu_write_elf32_note(WriteCoreDumpFunction f, CPUState *cs, int cpuid, void *opaque); +uint32_t aarch64_sve_zcr_get_valid_len(ARMCPU *cpu, uint32_t start_len); + #ifdef TARGET_AARCH64 int aarch64_cpu_gdb_read_register(CPUState *cpu, GByteArray *buf, int reg); int aarch64_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg); diff --git a/target/arm/helper.c b/target/arm/helper.c index 910ace4274..a49067c115 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -6454,11 +6454,13 @@ 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; - 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); @@ -6484,7 +6486,7 @@ 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.25.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] target/arm: Export aarch64_sve_zcr_get_valid_len 2021-07-14 18:06 ` [PATCH 1/2] target/arm: Export aarch64_sve_zcr_get_valid_len Richard Henderson @ 2021-07-16 8:53 ` Peter Maydell 0 siblings, 0 replies; 6+ messages in thread From: Peter Maydell @ 2021-07-16 8:53 UTC (permalink / raw) To: Richard Henderson; +Cc: QEMU Developers On Wed, 14 Jul 2021 at 19:06, Richard Henderson <richard.henderson@linaro.org> wrote: > > Rename from sve_zcr_get_valid_len and make accessible > from outside of helper.c. > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > target/arm/cpu.h | 2 ++ > target/arm/helper.c | 8 +++++--- > 2 files changed, 7 insertions(+), 3 deletions(-) > > diff --git a/target/arm/cpu.h b/target/arm/cpu.h > index be9a4dceae..52e99344c5 100644 > --- a/target/arm/cpu.h > +++ b/target/arm/cpu.h > @@ -1060,6 +1060,8 @@ int arm_cpu_write_elf64_note(WriteCoreDumpFunction f, CPUState *cs, > int arm_cpu_write_elf32_note(WriteCoreDumpFunction f, CPUState *cs, > int cpuid, void *opaque); > > +uint32_t aarch64_sve_zcr_get_valid_len(ARMCPU *cpu, uint32_t start_len); We only need this in cpu.c, I think, so I would favour putting it in internals.h. A brief comment defining its purpose would also be good. > + > #ifdef TARGET_AARCH64 > int aarch64_cpu_gdb_read_register(CPUState *cpu, GByteArray *buf, int reg); > int aarch64_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg); > diff --git a/target/arm/helper.c b/target/arm/helper.c > index 910ace4274..a49067c115 100644 > --- a/target/arm/helper.c > +++ b/target/arm/helper.c > @@ -6454,11 +6454,13 @@ 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; > > - end_len = start_len &= 0xf; > + start_len = MIN(start_len, ARM_MAX_VQ - 1); > + end_len = start_len; > + This seems to also be making a functional change? That should be a separate patch. > 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); > @@ -6484,7 +6486,7 @@ 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); > } thanks -- PMM ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/2] target/arm: Add sve-default-vector-length cpu property 2021-07-14 18:06 [PATCH 0/2] target/arm: Add sve-default-vector-length cpu property Richard Henderson 2021-07-14 18:06 ` [PATCH 1/2] target/arm: Export aarch64_sve_zcr_get_valid_len Richard Henderson @ 2021-07-14 18:06 ` Richard Henderson 2021-07-16 9:14 ` Peter Maydell 1 sibling, 1 reply; 6+ messages in thread From: Richard Henderson @ 2021-07-14 18:06 UTC (permalink / raw) To: qemu-devel; +Cc: peter.maydell 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> --- target/arm/cpu.h | 5 +++++ target/arm/cpu.c | 14 ++++++++++-- target/arm/cpu64.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 70 insertions(+), 2 deletions(-) diff --git a/target/arm/cpu.h b/target/arm/cpu.h index 52e99344c5..ffd82edaef 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -1006,6 +1006,11 @@ 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 9cddfd6a44..b5a2c9eb45 100644 --- a/target/arm/cpu.c +++ b/target/arm/cpu.c @@ -201,7 +201,8 @@ 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. @@ -1051,7 +1052,16 @@ 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 c7a1626bec..0e44a4f154 100644 --- a/target/arm/cpu64.c +++ b/target/arm/cpu64.c @@ -559,6 +559,52 @@ 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); + uint32_t default_len, default_vq, remainder; + + if (!visit_type_uint32(v, name, &default_len, errp)) { + return; + } + + default_vq = default_len / 128; + remainder = default_len % 128; + + /* + * 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 128\n"); + } else if (default_vq < 1) { + error_append_hint(errp, "Vector length smaller than 128\n"); + } else { + error_append_hint(errp, "Vector length larger than 65536\n"); + } + 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); + uint32_t value = cpu->sve_default_vq * 128; + + visit_type_uint32(v, name, &value, errp); +} +#endif + void aarch64_add_sve_properties(Object *obj) { uint32_t vq; @@ -571,6 +617,13 @@ 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", "uint32", + 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.25.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] target/arm: Add sve-default-vector-length cpu property 2021-07-14 18:06 ` [PATCH 2/2] target/arm: Add sve-default-vector-length cpu property Richard Henderson @ 2021-07-16 9:14 ` Peter Maydell 2021-07-23 18:47 ` Richard Henderson 0 siblings, 1 reply; 6+ messages in thread From: Peter Maydell @ 2021-07-16 9:14 UTC (permalink / raw) To: Richard Henderson; +Cc: QEMU Developers On Wed, 14 Jul 2021 at 19:06, Richard Henderson <richard.henderson@linaro.org> wrote: > > 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> > +#ifdef CONFIG_USER_ONLY > +/* Mirror linux /proc/sys/abi/sve_default_vector_length. */ /proc/sys/abi/sve_default_vector_length wants a vector length in bytes, and it looks like we take a length in bits. I assume that's to match other places where the user can specify vector lengths, but we should mention the units we expect and that it's not what the kernel uses. We also don't support the kernel's (undocumented) "-1 means set to the maximum" behaviour -- do we need it, or is that more reasonably achievable by the user via other properties ? Otherwise Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Regarding documentation, yeah, we don't have a great place to put usermode specific CPU properties. I would just put the documentation with the docs of the other properties. Maybe we'll sort out the document structure later (or at least put a link from the usermode doc section into the CPU part of the system-emulation doc section), but the usermode emulation documentation overall is in need of a massive overhaul some day... thanks -- PMM ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] target/arm: Add sve-default-vector-length cpu property 2021-07-16 9:14 ` Peter Maydell @ 2021-07-23 18:47 ` Richard Henderson 0 siblings, 0 replies; 6+ messages in thread From: Richard Henderson @ 2021-07-23 18:47 UTC (permalink / raw) To: Peter Maydell; +Cc: QEMU Developers On 7/15/21 11:14 PM, Peter Maydell wrote: >> +#ifdef CONFIG_USER_ONLY >> +/* Mirror linux /proc/sys/abi/sve_default_vector_length. */ > > /proc/sys/abi/sve_default_vector_length wants a vector length in > bytes, and it looks like we take a length in bits. I assume that's > to match other places where the user can specify vector lengths, > but we should mention the units we expect and that it's not what > the kernel uses. Oops, that wasn't intentional. > We also don't support the kernel's (undocumented) "-1 means set > to the maximum" behaviour -- do we need it, or is that more reasonably > achievable by the user via other properties ? I didn't notice that one either, possibly because it's undocumented. Might as well support that too. r~ ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-07-23 18:48 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-07-14 18:06 [PATCH 0/2] target/arm: Add sve-default-vector-length cpu property Richard Henderson 2021-07-14 18:06 ` [PATCH 1/2] target/arm: Export aarch64_sve_zcr_get_valid_len Richard Henderson 2021-07-16 8:53 ` Peter Maydell 2021-07-14 18:06 ` [PATCH 2/2] target/arm: Add sve-default-vector-length cpu property Richard Henderson 2021-07-16 9:14 ` Peter Maydell 2021-07-23 18:47 ` Richard Henderson
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.