| Message ID | 20220527180623.185261-4-richard.henderson@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | target/arm: SME prep patches | expand |
On Fri, 27 May 2022 at 19:07, 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; > } I'm still not a fan of the zcr_el[] value not actually being a valid one. I'd rather we constrained it when we write the value into the field. thanks -- PMM
On 5/31/22 05:15, Peter Maydell wrote: > On Fri, 27 May 2022 at 19:07, 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; >> } > > I'm still not a fan of the zcr_el[] value not actually being > a valid one. I'd rather we constrained it when we write the > value into the field. It is an architecturally valid value, exactly like the kernel might write while probing for supported vector lengths. It will result in this, or the next smaller supported vector size, being selected. r~
On Tue, 31 May 2022 at 15:28, Richard Henderson <richard.henderson@linaro.org> wrote: > > On 5/31/22 05:15, Peter Maydell wrote: > > On Fri, 27 May 2022 at 19:07, 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; > >> } > > > > I'm still not a fan of the zcr_el[] value not actually being > > a valid one. I'd rather we constrained it when we write the > > value into the field. > > It is an architecturally valid value, exactly like the kernel might write while probing > for supported vector lengths. It will result in this, or the next smaller supported > vector size, being selected. Mmm, I guess so (having re-read the ZCR_EL1 docs). -- PMM
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(-)