On Wed, Oct 16, 2019 at 10:54:08AM +0200, Andrew Jones wrote: > Allow cpu 'host' to enable SVE when it's available, unless the > user chooses to disable it with the added 'sve=off' cpu property. > Also give the user the ability to select vector lengths with the > sve<N> properties. We don't adopt 'max' cpu's other sve property, > sve-max-vq, because that property is difficult to use with KVM. > That property assumes all vector lengths in the range from 1 up > to and including the specified maximum length are supported, but > there may be optional lengths not supported by the host in that > range. With KVM one must be more specific when enabling vector > lengths. > > Signed-off-by: Andrew Jones <drjo...@redhat.com> > Reviewed-by: Eric Auger <eric.au...@redhat.com> > Reviewed-by: Richard Henderson <richard.hender...@linaro.org>
This patch works well on aarch64 with SVE machine, thanks! Please feel free to add: Tested-by: Masayoshi Mizuma <m.miz...@jp.fujitsu.com> - Masa > --- > docs/arm-cpu-features.rst | 19 ++++++++++++------- > target/arm/cpu.c | 3 +++ > target/arm/cpu.h | 2 ++ > target/arm/cpu64.c | 33 +++++++++++++++++---------------- > target/arm/kvm64.c | 14 +++++++++++++- > tests/arm-cpu-features.c | 21 +++++++++++---------- > 6 files changed, 58 insertions(+), 34 deletions(-) > > diff --git a/docs/arm-cpu-features.rst b/docs/arm-cpu-features.rst > index bed218d44619..1b367e22e16e 100644 > --- a/docs/arm-cpu-features.rst > +++ b/docs/arm-cpu-features.rst > @@ -272,31 +272,36 @@ SVE CPU Property Examples > > $ qemu-system-aarch64 -M virt -cpu max > > - 3) Only enable the 128-bit vector length:: > + 3) When KVM is enabled, implicitly enable all host CPU supported vector > + lengths with the `host` CPU type:: > + > + $ qemu-system-aarch64 -M virt,accel=kvm -cpu host > + > + 4) Only enable the 128-bit vector length:: > > $ qemu-system-aarch64 -M virt -cpu max,sve128=on > > - 4) Disable the 512-bit vector length and all larger vector lengths, > + 5) Disable the 512-bit vector length and all larger vector lengths, > since 512 is a power-of-two. This results in all the smaller, > uninitialized lengths (128, 256, and 384) defaulting to enabled:: > > $ qemu-system-aarch64 -M virt -cpu max,sve512=off > > - 5) Enable the 128-bit, 256-bit, and 512-bit vector lengths:: > + 6) Enable the 128-bit, 256-bit, and 512-bit vector lengths:: > > $ qemu-system-aarch64 -M virt -cpu max,sve128=on,sve256=on,sve512=on > > - 6) The same as (5), but since the 128-bit and 256-bit vector > + 7) The same as (6), but since the 128-bit and 256-bit vector > lengths are required for the 512-bit vector length to be enabled, > then allow them to be auto-enabled:: > > $ qemu-system-aarch64 -M virt -cpu max,sve512=on > > - 7) Do the same as (6), but by first disabling SVE and then re-enabling it:: > + 8) Do the same as (7), but by first disabling SVE and then re-enabling it:: > > $ qemu-system-aarch64 -M virt -cpu max,sve=off,sve512=on,sve=on > > - 8) Force errors regarding the last vector length:: > + 9) Force errors regarding the last vector length:: > > $ qemu-system-aarch64 -M virt -cpu max,sve128=off > $ qemu-system-aarch64 -M virt -cpu max,sve=off,sve128=off,sve=on > @@ -308,5 +313,5 @@ The examples in "SVE CPU Property Examples" exhibit many > ways to select > vector lengths which developers may find useful in order to avoid overly > verbose command lines. However, the recommended way to select vector > lengths is to explicitly enable each desired length. Therefore only > -example's (1), (3), and (5) exhibit recommended uses of the properties. > +example's (1), (4), and (6) exhibit recommended uses of the properties. > > diff --git a/target/arm/cpu.c b/target/arm/cpu.c > index 48c18871f6ea..0c4465880ddd 100644 > --- a/target/arm/cpu.c > +++ b/target/arm/cpu.c > @@ -2669,6 +2669,9 @@ static void arm_host_initfn(Object *obj) > ARMCPU *cpu = ARM_CPU(obj); > > kvm_arm_set_cpu_features_from_host(cpu); > + if (arm_feature(&cpu->env, ARM_FEATURE_AARCH64)) { > + aarch64_add_sve_properties(obj); > + } > arm_cpu_post_init(obj); > } > > diff --git a/target/arm/cpu.h b/target/arm/cpu.h > index 11162484465a..5b9c3e4cd73d 100644 > --- a/target/arm/cpu.h > +++ b/target/arm/cpu.h > @@ -974,11 +974,13 @@ int aarch64_cpu_gdb_write_register(CPUState *cpu, > uint8_t *buf, int reg); > void aarch64_sve_narrow_vq(CPUARMState *env, unsigned vq); > void aarch64_sve_change_el(CPUARMState *env, int old_el, > int new_el, bool el0_a64); > +void aarch64_add_sve_properties(Object *obj); > #else > static inline void aarch64_sve_narrow_vq(CPUARMState *env, unsigned vq) { } > static inline void aarch64_sve_change_el(CPUARMState *env, int o, > int n, bool a) > { } > +static inline void aarch64_add_sve_properties(Object *obj) { } > #endif > > #if !defined(CONFIG_TCG) > diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c > index c161a146ff0d..68baf0482ffa 100644 > --- a/target/arm/cpu64.c > +++ b/target/arm/cpu64.c > @@ -594,6 +594,21 @@ static void cpu_arm_set_sve(Object *obj, Visitor *v, > const char *name, > cpu->isar.id_aa64pfr0 = t; > } > > +void aarch64_add_sve_properties(Object *obj) > +{ > + uint32_t vq; > + > + object_property_add(obj, "sve", "bool", cpu_arm_get_sve, > + cpu_arm_set_sve, 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); > + } > +} > + > /* -cpu max: if KVM is enabled, like -cpu host (best possible with this > host); > * otherwise, a CPU with as many features enabled as our emulation supports. > * The version of '-cpu max' for qemu-system-arm is defined in cpu.c; > @@ -602,17 +617,11 @@ static void cpu_arm_set_sve(Object *obj, Visitor *v, > const char *name, > static void aarch64_max_initfn(Object *obj) > { > ARMCPU *cpu = ARM_CPU(obj); > - uint32_t vq; > - uint64_t t; > > if (kvm_enabled()) { > kvm_arm_set_cpu_features_from_host(cpu); > - if (kvm_arm_sve_supported(CPU(cpu))) { > - t = cpu->isar.id_aa64pfr0; > - t = FIELD_DP64(t, ID_AA64PFR0, SVE, 1); > - cpu->isar.id_aa64pfr0 = t; > - } > } else { > + uint64_t t; > uint32_t u; > aarch64_a57_initfn(obj); > > @@ -712,17 +721,9 @@ static void aarch64_max_initfn(Object *obj) > #endif > } > > - object_property_add(obj, "sve", "bool", cpu_arm_get_sve, > - cpu_arm_set_sve, NULL, NULL, &error_fatal); > + aarch64_add_sve_properties(obj); > 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 c93bbee425ae..876184b8fe4d 100644 > --- a/target/arm/kvm64.c > +++ b/target/arm/kvm64.c > @@ -488,7 +488,9 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures > *ahcf) > * and then query that CPU for the relevant ID registers. > */ > int fdarray[3]; > + bool sve_supported; > uint64_t features = 0; > + uint64_t t; > int err; > > /* Old kernels may not know about the PREFERRED_TARGET ioctl: however > @@ -578,13 +580,23 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures > *ahcf) > ARM64_SYS_REG(3, 0, 0, 3, 2)); > } > > + sve_supported = ioctl(fdarray[0], KVM_CHECK_EXTENSION, KVM_CAP_ARM_SVE) > > 0; > + > kvm_arm_destroy_scratch_host_vcpu(fdarray); > > if (err < 0) { > return false; > } > > - /* We can assume any KVM supporting CPU is at least a v8 > + /* Add feature bits that can't appear until after VCPU init. */ > + if (sve_supported) { > + t = ahcf->isar.id_aa64pfr0; > + t = FIELD_DP64(t, ID_AA64PFR0, SVE, 1); > + ahcf->isar.id_aa64pfr0 = t; > + } > + > + /* > + * We can assume any KVM supporting CPU is at least a v8 > * with VFPv4+Neon; this in turn implies most of the other > * feature bits. > */ > diff --git a/tests/arm-cpu-features.c b/tests/arm-cpu-features.c > index 61694e2f8e0c..92668efb8f56 100644 > --- a/tests/arm-cpu-features.c > +++ b/tests/arm-cpu-features.c > @@ -356,7 +356,7 @@ static void sve_tests_sve_off_kvm(const void *data) > { > QTestState *qts; > > - qts = qtest_init(MACHINE "-accel kvm -cpu max,sve=off"); > + qts = qtest_init(MACHINE "-accel kvm -cpu host,sve=off"); > > /* > * We don't know if this host supports SVE so we don't > @@ -365,8 +365,8 @@ static void sve_tests_sve_off_kvm(const void *data) > * 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 }"); > + assert_sve_vls(qts, "host", 0, NULL); > + assert_sve_vls(qts, "host", 0, "{ 'sve128': false }"); > > qtest_quit(qts); > } > @@ -433,8 +433,8 @@ static void test_query_cpu_model_expansion_kvm(const void > *data) > "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"); > + assert_has_feature(qts, "host", "sve"); > + resp = do_query_no_props(qts, "host"); > kvm_supports_sve = resp_get_feature(resp, "sve"); > vls = resp_get_sve_vls(resp); > qobject_unref(resp); > @@ -445,7 +445,7 @@ static void test_query_cpu_model_expansion_kvm(const void > *data) > 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); > + assert_sve_vls(qts, "host", vls, "{ %s: true }", max_name); > > /* Get the next supported length smaller than max-vq. */ > vq = 64 - __builtin_clzll(vls & ~BIT_ULL(max_vq - 1)); > @@ -454,7 +454,7 @@ static void test_query_cpu_model_expansion_kvm(const void > *data) > * We have at least one length smaller than max-vq, > * so we can disable max-vq. > */ > - assert_sve_vls(qts, "max", (vls & ~BIT_ULL(max_vq - 1)), > + assert_sve_vls(qts, "host", (vls & ~BIT_ULL(max_vq - 1)), > "{ %s: false }", max_name); > > /* > @@ -464,7 +464,7 @@ static void test_query_cpu_model_expansion_kvm(const void > *data) > */ > sprintf(name, "sve%d", vq * 128); > error = g_strdup_printf("cannot disable %s", name); > - assert_error(qts, "max", error, > + assert_error(qts, "host", error, > "{ %s: true, %s: false }", > max_name, name); > g_free(error); > @@ -477,7 +477,7 @@ static void test_query_cpu_model_expansion_kvm(const void > *data) > 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); > + assert_error(qts, "host", error, "{ %s: false }", name); > g_free(error); > > /* Get an unsupported length. */ > @@ -489,13 +489,14 @@ static void test_query_cpu_model_expansion_kvm(const > void *data) > 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); > + assert_error(qts, "host", error, "{ %s: true }", name); > g_free(error); > } > } else { > g_assert(vls == 0); > } > } else { > + assert_has_not_feature(qts, "host", "sve"); > assert_error(qts, "host", > "'pmu' feature not supported by KVM on this host", > "{ 'pmu': true }"); > -- > 2.21.0 > >