Now that we have an ARMCPU member sve_vq_supported we no longer
need the local kvm_supported bitmap for KVM's supported vector
lengths.

Signed-off-by: Andrew Jones <drjones@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 target/arm/cpu64.c | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
index eb9318c83b74..557fd4757740 100644
--- a/target/arm/cpu64.c
+++ b/target/arm/cpu64.c
@@ -265,14 +265,17 @@ void arm_cpu_sve_finalize(ARMCPU *cpu, Error **errp)
      * any of the above.  Finally, if SVE is not disabled, then at least one
      * vector length must be enabled.
      */
-    DECLARE_BITMAP(kvm_supported, ARM_MAX_VQ);
     DECLARE_BITMAP(tmp, ARM_MAX_VQ);
     uint32_t vq, max_vq = 0;
 
-    /* Collect the set of vector lengths supported by KVM. */
-    bitmap_zero(kvm_supported, ARM_MAX_VQ);
+    /*
+     * CPU models specify a set of supported vector lengths which are
+     * enabled by default.  Attempting to enable any vector length not set
+     * in the supported bitmap results in an error.  When KVM is enabled we
+     * fetch the supported bitmap from the host.
+     */
     if (kvm_enabled() && kvm_arm_sve_supported()) {
-        kvm_arm_sve_get_vls(CPU(cpu), kvm_supported);
+        kvm_arm_sve_get_vls(CPU(cpu), cpu->sve_vq_supported);
     } else if (kvm_enabled()) {
         assert(!cpu_isar_feature(aa64_sve, cpu));
     }
@@ -299,7 +302,7 @@ void arm_cpu_sve_finalize(ARMCPU *cpu, Error **errp)
              * For KVM we have to automatically enable all supported unitialized
              * lengths, even when the smaller lengths are not all powers-of-two.
              */
-            bitmap_andnot(tmp, kvm_supported, cpu->sve_vq_init, max_vq);
+            bitmap_andnot(tmp, cpu->sve_vq_supported, cpu->sve_vq_init, max_vq);
             bitmap_or(cpu->sve_vq_map, cpu->sve_vq_map, tmp, max_vq);
         } else {
             /* Propagate enabled bits down through required powers-of-two. */
@@ -322,12 +325,12 @@ void arm_cpu_sve_finalize(ARMCPU *cpu, Error **errp)
             /* Disabling a supported length disables all larger lengths. */
             for (vq = 1; vq <= ARM_MAX_VQ; ++vq) {
                 if (test_bit(vq - 1, cpu->sve_vq_init) &&
-                    test_bit(vq - 1, kvm_supported)) {
+                    test_bit(vq - 1, cpu->sve_vq_supported)) {
                     break;
                 }
             }
             max_vq = vq <= ARM_MAX_VQ ? vq - 1 : ARM_MAX_VQ;
-            bitmap_andnot(cpu->sve_vq_map, kvm_supported,
+            bitmap_andnot(cpu->sve_vq_map, cpu->sve_vq_supported,
                           cpu->sve_vq_init, max_vq);
             if (max_vq == 0 || bitmap_empty(cpu->sve_vq_map, max_vq)) {
                 error_setg(errp, "cannot disable sve%d", vq * 128);
@@ -392,7 +395,7 @@ void arm_cpu_sve_finalize(ARMCPU *cpu, Error **errp)
 
     if (kvm_enabled()) {
         /* Ensure the set of lengths matches what KVM supports. */
-        bitmap_xor(tmp, cpu->sve_vq_map, kvm_supported, max_vq);
+        bitmap_xor(tmp, cpu->sve_vq_map, cpu->sve_vq_supported, max_vq);
         if (!bitmap_empty(tmp, max_vq)) {
             vq = find_last_bit(tmp, max_vq) + 1;
             if (test_bit(vq - 1, cpu->sve_vq_map)) {
-- 
2.31.1

On 8/23/21 9:06 AM, Andrew Jones wrote:
> Now that we have an ARMCPU member sve_vq_supported we no longer
> need the local kvm_supported bitmap for KVM's supported vector
> lengths.
> 
> Signed-off-by: Andrew Jones <drjones@redhat.com>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>   target/arm/cpu64.c | 19 +++++++++++--------
>   1 file changed, 11 insertions(+), 8 deletions(-)
> 
> diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
> index eb9318c83b74..557fd4757740 100644
> --- a/target/arm/cpu64.c
> +++ b/target/arm/cpu64.c
> @@ -265,14 +265,17 @@ void arm_cpu_sve_finalize(ARMCPU *cpu, Error **errp)
>        * any of the above.  Finally, if SVE is not disabled, then at least one
>        * vector length must be enabled.
>        */
> -    DECLARE_BITMAP(kvm_supported, ARM_MAX_VQ);
>       DECLARE_BITMAP(tmp, ARM_MAX_VQ);
>       uint32_t vq, max_vq = 0;
>   
> -    /* Collect the set of vector lengths supported by KVM. */
> -    bitmap_zero(kvm_supported, ARM_MAX_VQ);
> +    /*
> +     * CPU models specify a set of supported vector lengths which are
> +     * enabled by default.  Attempting to enable any vector length not set
> +     * in the supported bitmap results in an error.  When KVM is enabled we
> +     * fetch the supported bitmap from the host.
> +     */
>       if (kvm_enabled() && kvm_arm_sve_supported()) {
> -        kvm_arm_sve_get_vls(CPU(cpu), kvm_supported);
> +        kvm_arm_sve_get_vls(CPU(cpu), cpu->sve_vq_supported);
>       } else if (kvm_enabled()) {
>           assert(!cpu_isar_feature(aa64_sve, cpu));
>       }

I think this whole stanza should now be moved into kvm_arm_get_host_cpu_features, where we 
detect sve and fetch ID_AA64ZFR0_EL1.

As a separate patch, since this one is simply the variable rename.

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

r~

On Mon, Aug 23, 2021 at 10:53:48AM -0700, Richard Henderson wrote:
> On 8/23/21 9:06 AM, Andrew Jones wrote:
> > Now that we have an ARMCPU member sve_vq_supported we no longer
> > need the local kvm_supported bitmap for KVM's supported vector
> > lengths.
> > 
> > Signed-off-by: Andrew Jones <drjones@redhat.com>
> > Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> > ---
> >   target/arm/cpu64.c | 19 +++++++++++--------
> >   1 file changed, 11 insertions(+), 8 deletions(-)
> > 
> > diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
> > index eb9318c83b74..557fd4757740 100644
> > --- a/target/arm/cpu64.c
> > +++ b/target/arm/cpu64.c
> > @@ -265,14 +265,17 @@ void arm_cpu_sve_finalize(ARMCPU *cpu, Error **errp)
> >        * any of the above.  Finally, if SVE is not disabled, then at least one
> >        * vector length must be enabled.
> >        */
> > -    DECLARE_BITMAP(kvm_supported, ARM_MAX_VQ);
> >       DECLARE_BITMAP(tmp, ARM_MAX_VQ);
> >       uint32_t vq, max_vq = 0;
> > -    /* Collect the set of vector lengths supported by KVM. */
> > -    bitmap_zero(kvm_supported, ARM_MAX_VQ);
> > +    /*
> > +     * CPU models specify a set of supported vector lengths which are
> > +     * enabled by default.  Attempting to enable any vector length not set
> > +     * in the supported bitmap results in an error.  When KVM is enabled we
> > +     * fetch the supported bitmap from the host.
> > +     */
> >       if (kvm_enabled() && kvm_arm_sve_supported()) {
> > -        kvm_arm_sve_get_vls(CPU(cpu), kvm_supported);
> > +        kvm_arm_sve_get_vls(CPU(cpu), cpu->sve_vq_supported);
> >       } else if (kvm_enabled()) {
> >           assert(!cpu_isar_feature(aa64_sve, cpu));
> >       }
> 
> I think this whole stanza should now be moved into
> kvm_arm_get_host_cpu_features, where we detect sve and fetch
> ID_AA64ZFR0_EL1.
> 
> As a separate patch, since this one is simply the variable rename.

Good idea. I'll do that for v3.

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

Thanks,
drew 

On Tue, Aug 24, 2021 at 08:28:55AM +0200, Andrew Jones wrote:
> On Mon, Aug 23, 2021 at 10:53:48AM -0700, Richard Henderson wrote:
> > On 8/23/21 9:06 AM, Andrew Jones wrote:
> > > Now that we have an ARMCPU member sve_vq_supported we no longer
> > > need the local kvm_supported bitmap for KVM's supported vector
> > > lengths.
> > > 
> > > Signed-off-by: Andrew Jones <drjones@redhat.com>
> > > Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> > > ---
> > >   target/arm/cpu64.c | 19 +++++++++++--------
> > >   1 file changed, 11 insertions(+), 8 deletions(-)
> > > 
> > > diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
> > > index eb9318c83b74..557fd4757740 100644
> > > --- a/target/arm/cpu64.c
> > > +++ b/target/arm/cpu64.c
> > > @@ -265,14 +265,17 @@ void arm_cpu_sve_finalize(ARMCPU *cpu, Error **errp)
> > >        * any of the above.  Finally, if SVE is not disabled, then at least one
> > >        * vector length must be enabled.
> > >        */
> > > -    DECLARE_BITMAP(kvm_supported, ARM_MAX_VQ);
> > >       DECLARE_BITMAP(tmp, ARM_MAX_VQ);
> > >       uint32_t vq, max_vq = 0;
> > > -    /* Collect the set of vector lengths supported by KVM. */
> > > -    bitmap_zero(kvm_supported, ARM_MAX_VQ);
> > > +    /*
> > > +     * CPU models specify a set of supported vector lengths which are
> > > +     * enabled by default.  Attempting to enable any vector length not set
> > > +     * in the supported bitmap results in an error.  When KVM is enabled we
> > > +     * fetch the supported bitmap from the host.
> > > +     */
> > >       if (kvm_enabled() && kvm_arm_sve_supported()) {
> > > -        kvm_arm_sve_get_vls(CPU(cpu), kvm_supported);
> > > +        kvm_arm_sve_get_vls(CPU(cpu), cpu->sve_vq_supported);
> > >       } else if (kvm_enabled()) {
> > >           assert(!cpu_isar_feature(aa64_sve, cpu));
> > >       }
> > 
> > I think this whole stanza should now be moved into
> > kvm_arm_get_host_cpu_features, where we detect sve and fetch
> > ID_AA64ZFR0_EL1.
> > 
> > As a separate patch, since this one is simply the variable rename.
> 
> Good idea. I'll do that for v3.

Actually, I'll post an independent series for this idea rather than
a v3 with another patch. With enough changes we can avoid several
scratch vcpus, but that's getting too far outside the scope of this
series.

Thanks,
drew