[PATCH 0/4] target/arm/cpu: Introduce sve_vq_supported bitmap

[PATCH 0/4] target/arm/cpu: Introduce sve_vq_supported bitmap

[Andrew Jones]

While reviewing the new A64FX CPU type it became clear that CPU
types should be able to specify which SVE vector lengths are
supported. This series adds a new bitmap member to ARMCPU and
modifies arm_cpu_sve_finalize() to validate inputs against it.
So far we only need to set the bitmap for the 'max' CPU type
though and, since it supports all vector lengths, we just fill
the whole thing.

This series was inspired by Richard Henderson's suggestion to
replace arm_cpu_sve_finalize's kvm_supported bitmap with something
that could be shared with TCG.

So far I've only lightly tested this. I'll do more testing and
report back later. I'd also be happy to get test results from
others.

Thanks,
drew

Andrew Jones (4):
  target/arm/cpu: Introduce sve_vq_supported bitmap
  target/arm/kvm64: Ensure sve vls map is completely clear
  target/arm/cpu64: Replace kvm_supported with sve_vq_supported
  target/arm/cpu64: Validate sve vector lengths are supported

 target/arm/cpu.h   |   4 ++
 target/arm/cpu64.c | 118 +++++++++++++++++++++------------------------
 target/arm/kvm64.c |   2 +-
 3 files changed, 61 insertions(+), 63 deletions(-)

-- 
2.31.1

[PATCH 1/4] target/arm/cpu: Introduce sve_vq_supported bitmap

[Andrew Jones]

Allow CPUs that support SVE to specify which SVE vector lengths they
support by setting them in this bitmap. Currently only the 'max' and
'host' CPU types supports SVE and 'host' requires KVM which obtains
its supported bitmap from the host. So, we only need to initialize the
bitmap for 'max' with TCG. And, since 'max' should support all SVE
vector lengths we simply fill the bitmap. Future CPU types may have
less trivial maps though.

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 target/arm/cpu.h   | 4 ++++
 target/arm/cpu64.c | 2 ++
 2 files changed, 6 insertions(+)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 9f0a5f84d503..cc645b57421f 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -1019,9 +1019,13 @@ struct ARMCPU {
      * While processing properties during initialization, corresponding
      * sve_vq_init bits are set for bits in sve_vq_map that have been
      * set by properties.
+     *
+     * Bits set in sve_vq_supported represent valid vector lengths for
+     * the CPU type.
      */
     DECLARE_BITMAP(sve_vq_map, ARM_MAX_VQ);
     DECLARE_BITMAP(sve_vq_init, ARM_MAX_VQ);
+    DECLARE_BITMAP(sve_vq_supported, ARM_MAX_VQ);
 
     /* Generic timer counter frequency, in Hz */
     uint64_t gt_cntfrq_hz;
diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
index c690318a9b63..eb9318c83b74 100644
--- a/target/arm/cpu64.c
+++ b/target/arm/cpu64.c
@@ -840,6 +840,8 @@ static void aarch64_max_initfn(Object *obj)
         /* Default to PAUTH on, with the architected algorithm. */
         qdev_property_add_static(DEVICE(obj), &arm_cpu_pauth_property);
         qdev_property_add_static(DEVICE(obj), &arm_cpu_pauth_impdef_property);
+
+        bitmap_fill(cpu->sve_vq_supported, ARM_MAX_VQ);
     }
 
     aarch64_add_sve_properties(obj);
-- 
2.31.1

Re: [PATCH 1/4] target/arm/cpu: Introduce sve_vq_supported bitmap

[Philippe Mathieu-Daudé]

On 8/19/21 9:37 PM, Andrew Jones wrote:
> Allow CPUs that support SVE to specify which SVE vector lengths they
> support by setting them in this bitmap. Currently only the 'max' and
> 'host' CPU types supports SVE and 'host' requires KVM which obtains
> its supported bitmap from the host. So, we only need to initialize the
> bitmap for 'max' with TCG. And, since 'max' should support all SVE
> vector lengths we simply fill the bitmap. Future CPU types may have
> less trivial maps though.
> 
> Signed-off-by: Andrew Jones <drjones@redhat.com>
> ---
>  target/arm/cpu.h   | 4 ++++
>  target/arm/cpu64.c | 2 ++
>  2 files changed, 6 insertions(+)

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

[PATCH 2/4] target/arm/kvm64: Ensure sve vls map is completely clear

[Andrew Jones]

bitmap_clear() only clears the given range. While the given
range should be sufficient in this case we might as well be
100% sure all bits are zeroed by using bitmap_zero().

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 target/arm/kvm64.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
index 59982d470d37..e790d6c9a573 100644
--- a/target/arm/kvm64.c
+++ b/target/arm/kvm64.c
@@ -740,7 +740,7 @@ void kvm_arm_sve_get_vls(CPUState *cs, unsigned long *map)
     uint32_t vq = 0;
     int i, j;
 
-    bitmap_clear(map, 0, ARM_MAX_VQ);
+    bitmap_zero(map, ARM_MAX_VQ);
 
     /*
      * KVM ensures all host CPUs support the same set of vector lengths.
-- 
2.31.1

Re: [PATCH 2/4] target/arm/kvm64: Ensure sve vls map is completely clear

[Philippe Mathieu-Daudé]

On 8/19/21 9:37 PM, Andrew Jones wrote:
> bitmap_clear() only clears the given range. While the given
> range should be sufficient in this case we might as well be
> 100% sure all bits are zeroed by using bitmap_zero().
> 
> Signed-off-by: Andrew Jones <drjones@redhat.com>
> ---
>  target/arm/kvm64.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

[PATCH 3/4] target/arm/cpu64: Replace kvm_supported with sve_vq_supported

[Andrew Jones]

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>
---
 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

Re: [PATCH 3/4] target/arm/cpu64: Replace kvm_supported with sve_vq_supported

[Philippe Mathieu-Daudé]

On 8/19/21 9:37 PM, 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>
> ---
>  target/arm/cpu64.c | 19 +++++++++++--------
>  1 file changed, 11 insertions(+), 8 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

[PATCH 4/4] target/arm/cpu64: Validate sve vector lengths are supported

[Andrew Jones]

Future CPU types may specify which vector lengths are supported.
We can apply nearly the same logic to validate those lengths
as we do for KVM's supported vector lengths. We merge the code
where we can, but unfortunately can't completely merge it because
KVM requires all vector lengths, power-of-two or not, smaller than
the maximum enabled length to also be enabled. The architecture
only requires all the power-of-two lengths, though, so TCG will
only enforce that.

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 target/arm/cpu64.c | 101 ++++++++++++++++++++-------------------------
 1 file changed, 45 insertions(+), 56 deletions(-)

diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
index 557fd4757740..9cb41c442600 100644
--- a/target/arm/cpu64.c
+++ b/target/arm/cpu64.c
@@ -329,35 +329,26 @@ void arm_cpu_sve_finalize(ARMCPU *cpu, Error **errp)
                     break;
                 }
             }
-            max_vq = vq <= ARM_MAX_VQ ? vq - 1 : ARM_MAX_VQ;
-            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);
-                error_append_hint(errp, "Disabling sve%d results in all "
-                                  "vector lengths being disabled.\n",
-                                  vq * 128);
-                error_append_hint(errp, "With SVE enabled, at least one "
-                                  "vector length must be enabled.\n");
-                return;
-            }
         } else {
             /* Disabling a power-of-two disables all larger lengths. */
-            if (test_bit(0, cpu->sve_vq_init)) {
-                error_setg(errp, "cannot disable sve128");
-                error_append_hint(errp, "Disabling sve128 results in all "
-                                  "vector lengths being disabled.\n");
-                error_append_hint(errp, "With SVE enabled, at least one "
-                                  "vector length must be enabled.\n");
-                return;
-            }
-            for (vq = 2; vq <= ARM_MAX_VQ; vq <<= 1) {
+            for (vq = 1; vq <= ARM_MAX_VQ; vq <<= 1) {
                 if (test_bit(vq - 1, cpu->sve_vq_init)) {
                     break;
                 }
             }
-            max_vq = vq <= ARM_MAX_VQ ? vq - 1 : ARM_MAX_VQ;
-            bitmap_complement(cpu->sve_vq_map, cpu->sve_vq_init, max_vq);
+        }
+
+        max_vq = vq <= ARM_MAX_VQ ? vq - 1 : ARM_MAX_VQ;
+        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);
+            error_append_hint(errp, "Disabling sve%d results in all "
+                              "vector lengths being disabled.\n",
+                              vq * 128);
+            error_append_hint(errp, "With SVE enabled, at least one "
+                              "vector length must be enabled.\n");
+            return;
         }
 
         max_vq = find_last_bit(cpu->sve_vq_map, max_vq) + 1;
@@ -393,46 +384,44 @@ void arm_cpu_sve_finalize(ARMCPU *cpu, Error **errp)
     assert(max_vq != 0);
     bitmap_clear(cpu->sve_vq_map, max_vq, ARM_MAX_VQ - max_vq);
 
-    if (kvm_enabled()) {
-        /* Ensure the set of lengths matches what KVM supports. */
-        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)) {
-                if (cpu->sve_max_vq) {
-                    error_setg(errp, "cannot set sve-max-vq=%d",
-                               cpu->sve_max_vq);
-                    error_append_hint(errp, "This KVM host does not support "
-                                      "the vector length %d-bits.\n",
-                                      vq * 128);
-                    error_append_hint(errp, "It may not be possible to use "
-                                      "sve-max-vq with this KVM host. Try "
-                                      "using only sve<N> properties.\n");
-                } else {
-                    error_setg(errp, "cannot enable sve%d", vq * 128);
-                    error_append_hint(errp, "This KVM host does not support "
-                                      "the vector length %d-bits.\n",
-                                      vq * 128);
-                }
+    /* Ensure the set of lengths matches what is supported. */
+    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)) {
+            if (cpu->sve_max_vq) {
+                error_setg(errp, "cannot set sve-max-vq=%d", cpu->sve_max_vq);
+                error_append_hint(errp, "This CPU does not support "
+                                  "the vector length %d-bits.\n", vq * 128);
+                error_append_hint(errp, "It may not be possible to use "
+                                  "sve-max-vq with this CPU. Try "
+                                  "using only sve<N> properties.\n");
             } else {
+                error_setg(errp, "cannot enable sve%d", vq * 128);
+                error_append_hint(errp, "This CPU does not support "
+                                  "the vector length %d-bits.\n", vq * 128);
+            }
+            return;
+        } else {
+            if (kvm_enabled()) {
                 error_setg(errp, "cannot disable sve%d", vq * 128);
                 error_append_hint(errp, "The KVM host requires all "
                                   "supported vector lengths smaller "
                                   "than %d bits to also be enabled.\n",
                                   max_vq * 128);
-            }
-            return;
-        }
-    } else {
-        /* Ensure all required powers-of-two are enabled. */
-        for (vq = pow2floor(max_vq); vq >= 1; vq >>= 1) {
-            if (!test_bit(vq - 1, cpu->sve_vq_map)) {
-                error_setg(errp, "cannot disable sve%d", vq * 128);
-                error_append_hint(errp, "sve%d is required as it "
-                                  "is a power-of-two length smaller than "
-                                  "the maximum, sve%d\n",
-                                  vq * 128, max_vq * 128);
                 return;
+            } else {
+                /* Ensure all required powers-of-two are enabled. */
+                for (vq = pow2floor(max_vq); vq >= 1; vq >>= 1) {
+                    if (!test_bit(vq - 1, cpu->sve_vq_map)) {
+                        error_setg(errp, "cannot disable sve%d", vq * 128);
+                        error_append_hint(errp, "sve%d is required as it "
+                                          "is a power-of-two length smaller "
+                                          " than the maximum, sve%d\n",
+                                          vq * 128, max_vq * 128);
+                        return;
+                    }
+                }
             }
         }
     }
-- 
2.31.1

Re: [PATCH 4/4] target/arm/cpu64: Validate sve vector lengths are supported

[Andrew Jones]

On Thu, Aug 19, 2021 at 09:37:58PM +0200, Andrew Jones wrote:
> Future CPU types may specify which vector lengths are supported.
> We can apply nearly the same logic to validate those lengths
> as we do for KVM's supported vector lengths. We merge the code
> where we can, but unfortunately can't completely merge it because
> KVM requires all vector lengths, power-of-two or not, smaller than
> the maximum enabled length to also be enabled. The architecture
> only requires all the power-of-two lengths, though, so TCG will
> only enforce that.
> 
> Signed-off-by: Andrew Jones <drjones@redhat.com>
> ---
>  target/arm/cpu64.c | 101 ++++++++++++++++++++-------------------------
>  1 file changed, 45 insertions(+), 56 deletions(-)
> 
> diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
> index 557fd4757740..9cb41c442600 100644
> --- a/target/arm/cpu64.c
> +++ b/target/arm/cpu64.c
> @@ -329,35 +329,26 @@ void arm_cpu_sve_finalize(ARMCPU *cpu, Error **errp)
>                      break;
>                  }
>              }
> -            max_vq = vq <= ARM_MAX_VQ ? vq - 1 : ARM_MAX_VQ;
> -            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);
> -                error_append_hint(errp, "Disabling sve%d results in all "
> -                                  "vector lengths being disabled.\n",
> -                                  vq * 128);
> -                error_append_hint(errp, "With SVE enabled, at least one "
> -                                  "vector length must be enabled.\n");
> -                return;
> -            }
>          } else {
>              /* Disabling a power-of-two disables all larger lengths. */
> -            if (test_bit(0, cpu->sve_vq_init)) {
> -                error_setg(errp, "cannot disable sve128");
> -                error_append_hint(errp, "Disabling sve128 results in all "
> -                                  "vector lengths being disabled.\n");
> -                error_append_hint(errp, "With SVE enabled, at least one "
> -                                  "vector length must be enabled.\n");
> -                return;
> -            }
> -            for (vq = 2; vq <= ARM_MAX_VQ; vq <<= 1) {
> +            for (vq = 1; vq <= ARM_MAX_VQ; vq <<= 1) {
>                  if (test_bit(vq - 1, cpu->sve_vq_init)) {
>                      break;
>                  }
>              }
> -            max_vq = vq <= ARM_MAX_VQ ? vq - 1 : ARM_MAX_VQ;
> -            bitmap_complement(cpu->sve_vq_map, cpu->sve_vq_init, max_vq);
> +        }
> +
> +        max_vq = vq <= ARM_MAX_VQ ? vq - 1 : ARM_MAX_VQ;
> +        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);
> +            error_append_hint(errp, "Disabling sve%d results in all "
> +                              "vector lengths being disabled.\n",
> +                              vq * 128);
> +            error_append_hint(errp, "With SVE enabled, at least one "
> +                              "vector length must be enabled.\n");
> +            return;
>          }
>  
>          max_vq = find_last_bit(cpu->sve_vq_map, max_vq) + 1;
> @@ -393,46 +384,44 @@ void arm_cpu_sve_finalize(ARMCPU *cpu, Error **errp)
>      assert(max_vq != 0);
>      bitmap_clear(cpu->sve_vq_map, max_vq, ARM_MAX_VQ - max_vq);
>  
> -    if (kvm_enabled()) {
> -        /* Ensure the set of lengths matches what KVM supports. */
> -        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)) {
> -                if (cpu->sve_max_vq) {
> -                    error_setg(errp, "cannot set sve-max-vq=%d",
> -                               cpu->sve_max_vq);
> -                    error_append_hint(errp, "This KVM host does not support "
> -                                      "the vector length %d-bits.\n",
> -                                      vq * 128);
> -                    error_append_hint(errp, "It may not be possible to use "
> -                                      "sve-max-vq with this KVM host. Try "
> -                                      "using only sve<N> properties.\n");
> -                } else {
> -                    error_setg(errp, "cannot enable sve%d", vq * 128);
> -                    error_append_hint(errp, "This KVM host does not support "
> -                                      "the vector length %d-bits.\n",
> -                                      vq * 128);
> -                }
> +    /* Ensure the set of lengths matches what is supported. */
> +    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)) {
> +            if (cpu->sve_max_vq) {
> +                error_setg(errp, "cannot set sve-max-vq=%d", cpu->sve_max_vq);
> +                error_append_hint(errp, "This CPU does not support "
> +                                  "the vector length %d-bits.\n", vq * 128);
> +                error_append_hint(errp, "It may not be possible to use "
> +                                  "sve-max-vq with this CPU. Try "
> +                                  "using only sve<N> properties.\n");
>              } else {
> +                error_setg(errp, "cannot enable sve%d", vq * 128);
> +                error_append_hint(errp, "This CPU does not support "
> +                                  "the vector length %d-bits.\n", vq * 128);
> +            }
> +            return;
> +        } else {
> +            if (kvm_enabled()) {
>                  error_setg(errp, "cannot disable sve%d", vq * 128);
>                  error_append_hint(errp, "The KVM host requires all "
>                                    "supported vector lengths smaller "
>                                    "than %d bits to also be enabled.\n",
>                                    max_vq * 128);
> -            }
> -            return;
> -        }
> -    } else {
> -        /* Ensure all required powers-of-two are enabled. */
> -        for (vq = pow2floor(max_vq); vq >= 1; vq >>= 1) {
> -            if (!test_bit(vq - 1, cpu->sve_vq_map)) {
> -                error_setg(errp, "cannot disable sve%d", vq * 128);
> -                error_append_hint(errp, "sve%d is required as it "
> -                                  "is a power-of-two length smaller than "
> -                                  "the maximum, sve%d\n",
> -                                  vq * 128, max_vq * 128);
>                  return;
> +            } else {
> +                /* Ensure all required powers-of-two are enabled. */
> +                for (vq = pow2floor(max_vq); vq >= 1; vq >>= 1) {
> +                    if (!test_bit(vq - 1, cpu->sve_vq_map)) {
> +                        error_setg(errp, "cannot disable sve%d", vq * 128);
> +                        error_append_hint(errp, "sve%d is required as it "
> +                                          "is a power-of-two length smaller "
> +                                          " than the maximum, sve%d\n",

When moving 'than' down to avoid going over 80 characters I somehow added
a leading space, making two spaces between 'smaller' and 'than'. I'll send
a v2 to remove it.

Thanks,
drew

> +                                          vq * 128, max_vq * 128);
> +                        return;
> +                    }
> +                }
>              }
>          }
>      }
> -- 
> 2.31.1
>

Re: [PATCH 0/4] target/arm/cpu: Introduce sve_vq_supported bitmap

[Andrew Jones]

On Thu, Aug 19, 2021 at 09:37:54PM +0200, Andrew Jones wrote:
> While reviewing the new A64FX CPU type it became clear that CPU
> types should be able to specify which SVE vector lengths are
> supported. This series adds a new bitmap member to ARMCPU and
> modifies arm_cpu_sve_finalize() to validate inputs against it.
> So far we only need to set the bitmap for the 'max' CPU type
> though and, since it supports all vector lengths, we just fill
> the whole thing.
> 
> This series was inspired by Richard Henderson's suggestion to
> replace arm_cpu_sve_finalize's kvm_supported bitmap with something
> that could be shared with TCG.
> 
> So far I've only lightly tested this. I'll do more testing and
> report back later. I'd also be happy to get test results from
> others.

I did more testing and it looks good to me except for the extra
space in an error message that I reported in patch 4.

Thanks,
drew

> 
> Thanks,
> drew
> 
> Andrew Jones (4):
>   target/arm/cpu: Introduce sve_vq_supported bitmap
>   target/arm/kvm64: Ensure sve vls map is completely clear
>   target/arm/cpu64: Replace kvm_supported with sve_vq_supported
>   target/arm/cpu64: Validate sve vector lengths are supported
> 
>  target/arm/cpu.h   |   4 ++
>  target/arm/cpu64.c | 118 +++++++++++++++++++++------------------------
>  target/arm/kvm64.c |   2 +-
>  3 files changed, 61 insertions(+), 63 deletions(-)
> 
> -- 
> 2.31.1
>