[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v4 8/9] target/arm/cpu64: max cpu: Support sve properties wit
|
From: |
Andrew Jones |
|
Subject: |
Re: [PATCH v4 8/9] target/arm/cpu64: max cpu: Support sve properties with KVM |
|
Date: |
Thu, 26 Sep 2019 10:41:17 +0200 |
|
User-agent: |
NeoMutt/20180716 |
On Thu, Sep 26, 2019 at 08:52:55AM +0200, Auger Eric wrote:
> Hi Drew,
>
> On 9/24/19 1:31 PM, Andrew Jones wrote:
> > Extend the SVE vq map initialization and validation with KVM's
> > supported vector lengths when KVM is enabled. In order to determine
> > and select supported lengths we add two new KVM functions for getting
> > and setting the KVM_REG_ARM64_SVE_VLS pseudo-register.
> >
> > This patch has been co-authored with Richard Henderson, who reworked
> > the target/arm/cpu64.c changes in order to push all the validation and
> > auto-enabling/disabling steps into the finalizer, resulting in a nice
> > LOC reduction.
> >
> > Signed-off-by: Andrew Jones <address@hidden>
> > ---
> > docs/arm-cpu-features.rst | 36 +++++---
> > target/arm/cpu64.c | 167 +++++++++++++++++++++++++++++---------
> > target/arm/kvm64.c | 100 ++++++++++++++++++++++-
> > target/arm/kvm_arm.h | 12 +++
> > tests/arm-cpu-features.c | 105 +++++++++++++++++++++++-
> > 5 files changed, 368 insertions(+), 52 deletions(-)
> >
> > diff --git a/docs/arm-cpu-features.rst b/docs/arm-cpu-features.rst
> > index 1262fddc6201..939366f959cf 100644
> > --- a/docs/arm-cpu-features.rst
> > +++ b/docs/arm-cpu-features.rst
> > @@ -188,10 +188,17 @@ SVE CPU Property Dependencies and Constraints
> >
> > 1) At least one vector length must be enabled when `sve` is enabled.
> >
> > - 2) If a vector length `N` is enabled, then all power-of-two vector
> > - lengths smaller than `N` must also be enabled. E.g. if `sve512`
> > - is enabled, then `sve128` and `sve256` must also be enabled,
> > - but `sve384` is not required.
> > + 2) If a vector length `N` is enabled, then, when KVM is enabled, all
> > + smaller, host supported vector lengths must also be enabled. If
> > + KVM is not enabled, then only all the smaller, power-of-two vector
> > + lengths must be enabled. E.g. with KVM if the host supports all
> > + vector lengths up to 512-bits (128, 256, 384, 512), then if
> > + `sve512` is enabled, `sve128`, `sve256`, and `sve384` must also
> > + be enabled. Without KVM, `sve384` would not be required.
> > +
> > + 3) If KVM is enabled then only vector lengths that the host CPU type
> > + support may be enabled. If SVE is not supported by the host, then
> > + no `sve*` properties may be enabled.
> >
> > SVE CPU Property Parsing Semantics
> > ----------------------------------
> > @@ -210,20 +217,29 @@ SVE CPU Property Parsing Semantics
> > disable the last enabled vector length (see constraint (1) of "SVE
> > CPU Property Dependencies and Constraints").
> >
> > - 4) If one or more `sve<N>` CPU properties are set `off`, but no `sve<N>`,
> > + 4) When KVM is enabled, if the host does not support SVE, then an error
> > + is generated when attempting to enable any `sve*` properties.
> > +
> > + 5) When KVM is enabled, if the host does support SVE, then an error is
> > + generated when attempting to enable any vector lengths not supported
> > + by the host.
> > +
> > + 6) If one or more `sve<N>` CPU properties are set `off`, but no `sve<N>`,
> > CPU properties are set `on`, then the specified vector lengths are
> > disabled but the default for any unspecified lengths remains enabled.
> > - Disabling a power-of-two vector length also disables all vector
> > - lengths larger than the power-of-two length (see constraint (2) of
> > - "SVE CPU Property Dependencies and Constraints").
> > + When KVM is not enabled, disabling a power-of-two vector length also
> > + disables all vector lengths larger than the power-of-two length.
> > + When KVM is enabled, then disabling any supported vector length also
> > + disables all larger vector lengths (see constraint (2) of "SVE CPU
> > + Property Dependencies and Constraints").
> >
> > - 5) If one or more `sve<N>` CPU properties are set to `on`, then they
> > + 7) If one or more `sve<N>` CPU properties are set to `on`, then they
> > are enabled and all unspecified lengths default to disabled, except
> > for the required lengths per constraint (2) of "SVE CPU Property
> > Dependencies and Constraints", which will even be auto-enabled if
> > they were not explicitly enabled.
> >
> > - 6) If SVE was disabled (`sve=off`), allowing all vector lengths to be
> > + 8) If SVE was disabled (`sve=off`), allowing all vector lengths to be
> > explicitly disabled (i.e. avoiding the error specified in (3) of
> > "SVE CPU Property Parsing Semantics"), then if later an `sve=on` is
> > provided an error will be generated. To avoid this error, one must
> > diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
> > index b7eff4e1e107..18dd5e24ec61 100644
> > --- a/target/arm/cpu64.c
> > +++ b/target/arm/cpu64.c
> > @@ -273,9 +273,18 @@ 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);
> > + if (kvm_enabled() && kvm_arm_sve_supported(CPU(cpu))) {
> > + kvm_arm_sve_get_vls(CPU(cpu), kvm_supported);
> > + } else if (kvm_enabled()) {
> > + assert(!cpu_isar_feature(aa64_sve, cpu));
> why not set an error and propagate it instead?
This should never happen. We shouldn't be here if KVM is enabled and SVE
isn't supported. The question is how defensive do we want QEMU code?
We could just drop the check altogether if we don't want the assert, but
I'd rather keep it.
> > + }
> > +
> > /*
> > * Process explicit sve<N> properties.
> > * From the properties, sve_vq_map<N> implies sve_vq_init<N>.
> > @@ -293,10 +302,19 @@ void arm_cpu_sve_finalize(ARMCPU *cpu, Error **errp)
> > return;
> > }
> >
> > - /* Propagate enabled bits down through required powers-of-two. */
> > - for (vq = pow2floor(max_vq); vq >= 1; vq >>= 1) {
> > - if (!test_bit(vq - 1, cpu->sve_vq_init)) {
> > - set_bit(vq - 1, cpu->sve_vq_map);
> > + if (kvm_enabled()) {
> > + /*
> > + * 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_or(cpu->sve_vq_map, cpu->sve_vq_map, tmp, max_vq);
> > + } else {
> > + /* Propagate enabled bits down through required powers-of-two.
> > */
> > + for (vq = pow2floor(max_vq); vq >= 1; vq >>= 1) {
> > + if (!test_bit(vq - 1, cpu->sve_vq_init)) {
> > + set_bit(vq - 1, cpu->sve_vq_map);
> > + }
> > }
> > }
> > } else if (cpu->sve_max_vq == 0) {
> > @@ -308,23 +326,46 @@ void arm_cpu_sve_finalize(ARMCPU *cpu, Error **errp)
> > return;
> > }
> >
> > - /* 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) {
> > - if (test_bit(vq - 1, cpu->sve_vq_init)) {
> > - break;
> > + if (kvm_enabled()) {
> > + /* 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)) {
> > + break;
> > + }
> > + }
> > + max_vq = vq <= ARM_MAX_VQ ? vq - 1 : ARM_MAX_VQ;
> > + bitmap_andnot(cpu->sve_vq_map, kvm_supported,
> > + cpu->sve_vq_init, max_vq);
> > + if (max_vq == 0 || bitmap_empty(cpu->sve_vq_map, max_vq)) {
> > + vq = find_next_bit(kvm_supported, ARM_MAX_VQ, 0) + 1;
> > + error_setg(errp, "cannot disable sve%d", vq * 128);
> isn't the one disabled max_vq? Do you really need to re-compute vq?
We're not looking for max_vq with find_next_bit(), we're looking for
"min_vq". Here we're checking that we always have at least one vector
length enabled. You can't disable the minimum, because that's the last
one.
> > + 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) {
> > + 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_complement(cpu->sve_vq_map, cpu->sve_vq_init, max_vq);
> > max_vq = find_last_bit(cpu->sve_vq_map, max_vq) + 1;
> > }
> >
> > @@ -358,16 +399,48 @@ 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);
> >
> > - /* 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);
> > + if (kvm_enabled()) {
> > + /* Ensure the set of lengths matches what KVM supports. */
> > + bitmap_xor(tmp, cpu->sve_vq_map, kvm_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);
> > + }
> > + } else {
> > + error_setg(errp, "cannot disable sve%d", vq * 128);
> I am not sure about the above message. what is the vq value supposed to
> be? For instance if we previously entered the
> "if (!bitmap_empty(cpu->sve_vq_map, ARM_MAX_VQ)) {" path
'vq' here should be enabled because it's a dependency for the max-vq.
If the user had enabled it or left it uninitialized (allowing it to
be auto-enabled) then we wouldn't be here. To get here the user must
have explicitly disabled it. Hence the 'cannot disable' message.
> > + 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;
> > + }
> > + }
> > }
> >
> > /* From now on sve_max_vq is the actual maximum supported length. */
> > @@ -411,13 +484,22 @@ static void cpu_max_set_sve_max_vq(Object *obj,
> > Visitor *v, const char *name,
> > Error *err = NULL;
> >
> > visit_type_uint32(v, name, &cpu->sve_max_vq, &err);
> > + if (err) {
> > + error_propagate(errp, err);
> > + return;
> > + }
> >
> > - if (!err && (cpu->sve_max_vq == 0 || cpu->sve_max_vq > ARM_MAX_VQ)) {
> > - error_setg(&err, "unsupported SVE vector length");
> > - error_append_hint(&err, "Valid sve-max-vq in range [1-%d]\n",
> > + if (kvm_enabled() && !kvm_arm_sve_supported(CPU(cpu))) {
> > + error_setg(errp, "cannot set sve-max-vq");
> > + error_append_hint(errp, "SVE not supported by KVM on this host\n");
> > + return;
> > + }
> > +
> > + if (cpu->sve_max_vq == 0 || cpu->sve_max_vq > ARM_MAX_VQ) {
> > + error_setg(errp, "unsupported SVE vector length");
> > + error_append_hint(errp, "Valid sve-max-vq in range [1-%d]\n",
> > ARM_MAX_VQ);
> > }
> > - error_propagate(errp, err);
> > }
> >
> > static void cpu_arm_get_sve_vq(Object *obj, Visitor *v, const char *name,
> > @@ -450,6 +532,12 @@ static void cpu_arm_set_sve_vq(Object *obj, Visitor
> > *v, const char *name,
> > return;
> > }
> >
> > + if (value && kvm_enabled() && !kvm_arm_sve_supported(CPU(cpu))) {
> > + error_setg(errp, "cannot enable %s", name);
> > + error_append_hint(errp, "SVE not supported by KVM on this host\n");
> > + return;
> > + }
> > +
> > if (value) {
> > set_bit(vq - 1, cpu->sve_vq_map);
> > } else {
> > @@ -607,20 +695,19 @@ static void aarch64_max_initfn(Object *obj)
> > cpu->ctr = 0x80038003; /* 32 byte I and D cacheline size, VIPT
> > icache */
> > cpu->dcz_blocksize = 7; /* 512 bytes */
> > #endif
> > -
> > - object_property_add(obj, "sve-max-vq", "uint32",
> > cpu_max_get_sve_max_vq,
> > - cpu_max_set_sve_max_vq, NULL, NULL,
> > &error_fatal);
> > -
> > - for (vq = 1; vq <= ARM_MAX_VQ; ++vq) {
> > - char name[8];
> > - sprintf(name, "sve%d", vq * 128);
> > - object_property_add(obj, name, "bool", cpu_arm_get_sve_vq,
> > - cpu_arm_set_sve_vq, NULL, NULL,
> > &error_fatal);
> > - }
> > }
> >
> > object_property_add(obj, "sve", "bool", cpu_arm_get_sve,
> > cpu_arm_set_sve, NULL, NULL, &error_fatal);
> > + object_property_add(obj, "sve-max-vq", "uint32",
> > cpu_max_get_sve_max_vq,
> > + cpu_max_set_sve_max_vq, NULL, NULL, &error_fatal);
> > +
> > + for (vq = 1; vq <= ARM_MAX_VQ; ++vq) {
> > + char name[8];
> > + sprintf(name, "sve%d", vq * 128);
> > + object_property_add(obj, name, "bool", cpu_arm_get_sve_vq,
> > + cpu_arm_set_sve_vq, NULL, NULL, &error_fatal);
> > + }
> > }
> >
> > struct ARMCPUInfo {
> > diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
> > index f96649ae0349..cff4217a8469 100644
> > --- a/target/arm/kvm64.c
> > +++ b/target/arm/kvm64.c
> > @@ -613,6 +613,100 @@ bool kvm_arm_sve_supported(CPUState *cpu)
> > return kvm_check_extension(s, KVM_CAP_ARM_SVE);
> > }
> >
> > +QEMU_BUILD_BUG_ON(KVM_ARM64_SVE_VQ_MIN != 1);
> > +
> > +void kvm_arm_sve_get_vls(CPUState *cs, unsigned long *map)
> > +{
> > + /* Only call this function if kvm_arm_sve_supported() returns true. */
> > + static uint64_t vls[KVM_ARM64_SVE_VLS_WORDS];
> > + static bool probed;
> > + uint32_t vq = 0;
> > + int i, j;
> > +
> > + bitmap_clear(map, 0, ARM_MAX_VQ);
> > +
> > + /*
> > + * KVM ensures all host CPUs support the same set of vector lengths.
> > + * So we only need to create the scratch VCPUs once and then cache
> > + * the results.
> > + */
> > + if (!probed) {
> > + struct kvm_vcpu_init init = {
> > + .target = -1,
> > + .features[0] = (1 << KVM_ARM_VCPU_SVE),
> > + };
> > + struct kvm_one_reg reg = {
> > + .id = KVM_REG_ARM64_SVE_VLS,
> > + .addr = (uint64_t)&vls[0],
> > + };
> > + int fdarray[3], ret;
> > +
> > + probed = true;
> > +
> > + if (!kvm_arm_create_scratch_host_vcpu(NULL, fdarray, &init)) {
> > + error_report("failed to create scratch VCPU with SVE enabled");
> > + abort();
> > + }
> > + ret = ioctl(fdarray[2], KVM_GET_ONE_REG, ®);
> > + kvm_arm_destroy_scratch_host_vcpu(fdarray);
> > + if (ret) {
> > + error_report("failed to get KVM_REG_ARM64_SVE_VLS: %s",
> > + strerror(errno));
> > + abort();
> > + }
> > +
> > + for (i = KVM_ARM64_SVE_VLS_WORDS - 1; i >= 0; --i) {
> > + if (vls[i]) {
> > + vq = 64 - clz64(vls[i]) + i * 64;
> > + break;
> > + }
> > + }
> > + if (vq > ARM_MAX_VQ) {
> > + warn_report("KVM supports vector lengths larger than "
> > + "QEMU can enable");
> > + }
> > + }
> > +
> > + for (i = 0; i < KVM_ARM64_SVE_VLS_WORDS; ++i) {
> > + if (!vls[i]) {
> > + continue;
> > + }
> > + for (j = 1; j <= 64; ++j) {
> > + vq = j + i * 64;
> > + if (vq > ARM_MAX_VQ) {
> > + return;
> > + }
> > + if (vls[i] & (1UL << (j - 1))) {
> > + set_bit(vq - 1, map);
> > + }
> > + }
> > + }
> > +}
> > +
> > +static int kvm_arm_sve_set_vls(CPUState *cs)
> > +{
> > + uint64_t vls[KVM_ARM64_SVE_VLS_WORDS] = {0};
> > + struct kvm_one_reg reg = {
> > + .id = KVM_REG_ARM64_SVE_VLS,
> > + .addr = (uint64_t)&vls[0],
> > + };
> > + ARMCPU *cpu = ARM_CPU(cs);
> > + uint32_t vq;
> > + int i, j;
> > +
> > + assert(cpu->sve_max_vq <= KVM_ARM64_SVE_VQ_MAX);
> > +
> > + for (vq = 1; vq <= cpu->sve_max_vq; ++vq) {
> > + if (test_bit(vq - 1, cpu->sve_vq_map)) {
> > + i = (vq - 1) / 64;
> > + j = (vq - 1) % 64;
> > + vls[i] |= 1UL << j;
> > + }
> > + }
> > +
> > + return kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®);
> > +}
> > +
> > #define ARM_CPU_ID_MPIDR 3, 0, 0, 0, 5
> >
> > int kvm_arch_init_vcpu(CPUState *cs)
> > @@ -624,7 +718,7 @@ int kvm_arch_init_vcpu(CPUState *cs)
> >
> > if (cpu->kvm_target == QEMU_KVM_ARM_TARGET_NONE ||
> > !object_dynamic_cast(OBJECT(cpu), TYPE_AARCH64_CPU)) {
> > - fprintf(stderr, "KVM is not supported for this guest CPU type\n");
> > + error_report("KVM is not supported for this guest CPU type");
> > return -EINVAL;
> > }
> >
> > @@ -660,6 +754,10 @@ int kvm_arch_init_vcpu(CPUState *cs)
> > }
> >
> > if (cpu_isar_feature(aa64_sve, cpu)) {
> > + ret = kvm_arm_sve_set_vls(cs);
> > + if (ret) {
> > + return ret;
> > + }
> > ret = kvm_arm_vcpu_finalize(cs, KVM_ARM_VCPU_SVE);
> > if (ret) {
> > return ret;
> > diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h
> > index 1151877f97ea..a1cc6513f72b 100644
> > --- a/target/arm/kvm_arm.h
> > +++ b/target/arm/kvm_arm.h
> > @@ -212,6 +212,17 @@ typedef struct ARMHostCPUFeatures {
> > */
> > bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf);
> >
> > +/**
> > + * kvm_arm_sve_get_vls:
> > + * @cs: CPUState
> > + * @map: bitmap to fill in
> > + *
> > + * Get all the SVE vector lengths supported by the KVM host, setting
> > + * the bits corresponding to their length in quadwords minus one
> > + * (vq - 1) in @map up to ARM_MAX_VQ.
> > + */
> > +void kvm_arm_sve_get_vls(CPUState *cs, unsigned long *map);
> > +
> > /**
> > * kvm_arm_set_cpu_features_from_host:
> > * @cpu: ARMCPU to set the features for
> > @@ -315,6 +326,7 @@ static inline int kvm_arm_vgic_probe(void)
> > static inline void kvm_arm_pmu_set_irq(CPUState *cs, int irq) {}
> > static inline void kvm_arm_pmu_init(CPUState *cs) {}
> >
> > +static inline void kvm_arm_sve_get_vls(CPUState *cs, unsigned long *map) {}
> > #endif
> >
> > static inline const char *gic_class_name(void)
> > diff --git a/tests/arm-cpu-features.c b/tests/arm-cpu-features.c
> > index d50f98cb6aea..a0129aebf409 100644
> > --- a/tests/arm-cpu-features.c
> > +++ b/tests/arm-cpu-features.c
> > @@ -117,6 +117,17 @@ static QDict *resp_get_props(QDict *resp)
> > return qdict;
> > }
> >
> > +static bool resp_get_feature(QDict *resp, const char *feature)
> > +{
> > + QDict *props;
> > +
> > + g_assert(resp);
> > + g_assert(resp_has_props(resp));
> > + props = resp_get_props(resp);
> > + g_assert(qdict_get(props, feature));
> > + return qdict_get_bool(props, feature);
> > +}
> > +
> > #define assert_has_feature(qts, cpu_type, feature) \
> > ({ \
> > QDict *_resp = do_query_no_props(qts, cpu_type); \
> > @@ -336,6 +347,25 @@ static void sve_tests_sve_off(const void *data)
> > qtest_quit(qts);
> > }
> >
> > +static void sve_tests_sve_off_kvm(const void *data)
> > +{
> > + QTestState *qts;
> > +
> > + qts = qtest_init(MACHINE "-accel kvm -cpu max,sve=off");
> > +
> > + /*
> > + * We don't know if this host supports SVE so we don't
> > + * attempt to test enabling anything. We only test that
> > + * everything is disabled (as it should be with sve=off)
> > + * and that using sve<N>=off to explicitly disable vector
> > + * lengths is OK too.
> > + */
> > + assert_sve_vls(qts, "max", 0, NULL);
> > + assert_sve_vls(qts, "max", 0, "{ 'sve128': false }");
> > +
> > + qtest_quit(qts);
> > +}
> > +
> > static void test_query_cpu_model_expansion(const void *data)
> > {
> > QTestState *qts;
> > @@ -385,12 +415,81 @@ static void test_query_cpu_model_expansion_kvm(const
> > void *data)
> > assert_has_feature(qts, "host", "pmu");
> >
> > if (g_str_equal(qtest_get_arch(), "aarch64")) {
> > + bool kvm_supports_sve;
> > + char max_name[8], name[8];
> > + uint32_t max_vq, vq;
> > + uint64_t vls;
> > + QDict *resp;
> > + char *error;
> > +
> > assert_has_feature(qts, "host", "aarch64");
> > - assert_has_feature(qts, "max", "sve");
> >
> > assert_error(qts, "cortex-a15",
> > "We cannot guarantee the CPU type 'cortex-a15' works "
> > "with KVM on this host", NULL);
> > +
> > + assert_has_feature(qts, "max", "sve");
> > + resp = do_query_no_props(qts, "max");
> > + kvm_supports_sve = resp_get_feature(resp, "sve");
> > + vls = resp_get_sve_vls(resp);
> > + qobject_unref(resp);
> > +
> > + if (kvm_supports_sve) {
> > + g_assert(vls != 0);
> > + max_vq = 64 - __builtin_clzll(vls);
> > + sprintf(max_name, "sve%d", max_vq * 128);
> > +
> > + /* Enabling a supported length is of course fine. */
> > + assert_sve_vls(qts, "max", vls, "{ %s: true }", max_name);
> > +
> > + /* Get the next supported length smaller than max-vq. */
> > + vq = 64 - __builtin_clzll(vls & ~BIT(max_vq - 1));
> > + if (vq) {
> > + /*
> > + * We have at least one length smaller than max-vq,
> > + * so we can disable max-vq.
> > + */
> > + assert_sve_vls(qts, "max", (vls & ~BIT(max_vq - 1)),
> > + "{ %s: false }", max_name);
> > +
> > + /*
> > + * Smaller, supported vector lengths cannot be disabled
> > + * unless all larger, supported vector lengths are also
> > + * disabled.
> > + */
> > + sprintf(name, "sve%d", vq * 128);
> > + error = g_strdup_printf("cannot disable %s", name);
> > + assert_error(qts, "max", error,
> > + "{ %s: true, %s: false }",
> > + max_name, name);
> > + g_free(error);
> > + }
> > +
> > + /*
> > + * The smallest, supported vector length is required, because
> > + * we need at least one vector length enabled.
> > + */
> > + vq = __builtin_ffsll(vls);
> > + sprintf(name, "sve%d", vq * 128);
> > + error = g_strdup_printf("cannot disable %s", name);
> > + assert_error(qts, "max", error, "{ %s: false }", name);
> > + g_free(error);
> > +
> > + /* Get an unsupported length. */
> > + for (vq = 1; vq <= max_vq; ++vq) {
> > + if (!(vls & BIT(vq - 1))) {
> > + break;
> > + }
> > + }
> > + if (vq <= SVE_MAX_VQ) {
> > + sprintf(name, "sve%d", vq * 128);
> > + error = g_strdup_printf("cannot enable %s", name);
> > + assert_error(qts, "max", error, "{ %s: true }", name);
> > + g_free(error);
> > + }
> > + } else {
> > + g_assert(vls == 0);
> > + }
> > } else {
> > assert_error(qts, "host",
> > "'pmu' feature not supported by KVM on this host",
> > @@ -427,6 +526,10 @@ int main(int argc, char **argv)
> > if (kvm_available) {
> > qtest_add_data_func("/arm/kvm/query-cpu-model-expansion",
> > NULL, test_query_cpu_model_expansion_kvm);
> > + if (g_str_equal(qtest_get_arch(), "aarch64")) {
> > +
> > qtest_add_data_func("/arm/kvm/query-cpu-model-expansion/sve-off",
> > + NULL, sve_tests_sve_off_kvm);
> > + }
> > }
> >
> > return g_test_run();
> >
> Besides,
> Reviewed-by: Eric Auger <address@hidden>
>
>
Thanks!
drew
- Re: [PATCH v4 4/9] target/arm/cpu64: max cpu: Introduce sve<N> properties, (continued)