| Message ID | 20220517054850.177016-4-richard.henderson@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | target/arm: SME prep patches | expand |
On Tue, 17 May 2022 at 07:00, Richard Henderson <richard.henderson@linaro.org> wrote: > > We don't need to constrain the value set in zcr_el[1], > because it will be done by sve_zcr_len_for_el. > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > target/arm/cpu.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/target/arm/cpu.c b/target/arm/cpu.c > index d2bd74c2ed..0621944167 100644 > --- a/target/arm/cpu.c > +++ b/target/arm/cpu.c > @@ -208,8 +208,7 @@ static void arm_cpu_reset(DeviceState *dev) > CPACR_EL1, ZEN, 3); > /* with reasonable vector length */ > if (cpu_isar_feature(aa64_sve, cpu)) { > - env->vfp.zcr_el[1] = > - aarch64_sve_zcr_get_valid_len(cpu, cpu->sve_default_vq - 1); > + env->vfp.zcr_el[1] = cpu->sve_default_vq - 1; > } Not all the code that looks at the sve vector length goes through sve_zcr_len_for_el(), though. In particular, this is setting up ZCR_EL1 for usermode, and all the code under linux-user/ that wants to know the vector length does it with "env->vfp.zcr_el[1] & 0xf". More generally, it seems to me less confusing for debug purposes if we set zcr_el[1] on reset to a valid value, not to some invalid value that we're relying on being coerced to something else at point of use. Incidentally, do_prctl_set_vl() also sets zcr_el[1] and it doesn't call aarch64_sve_zcr_get_valid_len(). Should it, or is it doing an equivalent check anyway? -- PMM
On 5/19/22 03:40, Peter Maydell wrote: > Not all the code that looks at the sve vector length > goes through sve_zcr_len_for_el(), though. In particular, > this is setting up ZCR_EL1 for usermode, and all > the code under linux-user/ that wants to know the vector > length does it with "env->vfp.zcr_el[1] & 0xf". Oops, yes. Linux-user should be checking ZCR_LEN from env->hflags. > Incidentally, do_prctl_set_vl() also sets zcr_el[1] and > it doesn't call aarch64_sve_zcr_get_valid_len(). Should it, > or is it doing an equivalent check anyway? I think this got missed when we introduced the set of valid lengths -- it's still assuming all lengths less than maximum are valid. I'll add a couple of cleanup patches for this. r~
diff --git a/target/arm/cpu.c b/target/arm/cpu.c index d2bd74c2ed..0621944167 100644 --- a/target/arm/cpu.c +++ b/target/arm/cpu.c @@ -208,8 +208,7 @@ static void arm_cpu_reset(DeviceState *dev) CPACR_EL1, ZEN, 3); /* with reasonable vector length */ if (cpu_isar_feature(aa64_sve, cpu)) { - env->vfp.zcr_el[1] = - aarch64_sve_zcr_get_valid_len(cpu, cpu->sve_default_vq - 1); + env->vfp.zcr_el[1] = cpu->sve_default_vq - 1; } /* * Enable 48-bit address space (TODO: take reserved_va into account).
We don't need to constrain the value set in zcr_el[1], because it will be done by sve_zcr_len_for_el. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- target/arm/cpu.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)