Hi Drew,
On 5/12/19 10:36 AM, Andrew Jones wrote:
> Allow the cpu type 'max' sve-max-vq property to work with kvm
> too. If the property is not specified then the maximum kvm
> supports is used. If it is specified we check that kvm supports
> that exact length or error out if it doesn't.
>
> Signed-off-by: Andrew Jones <drjo...@redhat.com>
> ---
> target/arm/cpu.h | 4 +++
> target/arm/cpu64.c | 7 ++--
> target/arm/kvm64.c | 80 ++++++++++++++++++++++++++++++++++++++++++++--
> 3 files changed, 86 insertions(+), 5 deletions(-)
>
> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> index 733b840a7127..8292d547e8f9 100644
> --- a/target/arm/cpu.h
> +++ b/target/arm/cpu.h
> @@ -3122,6 +3122,10 @@ static inline uint64_t arm_sctlr(CPUARMState *env, int
> el)
> }
> }
>
> +static inline int arm_cpu_fls64(uint64_t v)
> +{
> + return !v ? 0 : 64 - clz64(v);
> +}
>
> /* Return true if the processor is in big-endian mode. */
> static inline bool arm_cpu_data_is_big_endian(CPUARMState *env)
> diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
> index 6c19ef6837d5..3756e7e2a3e5 100644
> --- a/target/arm/cpu64.c
> +++ b/target/arm/cpu64.c
> @@ -292,7 +292,7 @@ static void aarch64_max_initfn(Object *obj)
>
> if (kvm_enabled()) {
> kvm_arm_set_cpu_features_from_host(cpu);
> - cpu->sve_max_vq = ARM_MAX_VQ;
> + cpu->sve_max_vq = -1; /* set in kvm_arch_init_vcpu() */
> } else {
> uint64_t t;
> uint32_t u;
> @@ -374,9 +374,10 @@ static void aarch64_max_initfn(Object *obj)
> #endif
>
> cpu->sve_max_vq = ARM_MAX_VQ;
> - object_property_add(obj, "sve-max-vq", "uint32", cpu_max_get_sve_vq,
> - cpu_max_set_sve_vq, NULL, NULL, &error_fatal);
> }
> +
> + object_property_add(obj, "sve-max-vq", "uint32", cpu_max_get_sve_vq,
> + cpu_max_set_sve_vq, NULL, NULL, &error_fatal);> }
>
> struct ARMCPUInfo {
> diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
> index c2d92df75353..0c666e405357 100644
> --- a/target/arm/kvm64.c
> +++ b/target/arm/kvm64.c
> @@ -446,6 +446,59 @@ void kvm_arm_pmu_set_irq(CPUState *cs, int irq)
> }
> }
>
> +static int kvm_arm_get_sve_vls(CPUState *cs, uint64_t sve_vls[])
> +{
> + struct kvm_one_reg reg = {
> + .id = KVM_REG_ARM64_SVE_VLS,
> + .addr = (uint64_t)&sve_vls[0],
> + };
> + int i, ret;
> +
> + ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, ®);
> + if (ret) {
> + return ret;
> + }
> +
> + ret = 0;
is it mandated?
> + for (i = KVM_ARM64_SVE_VLS_WORDS - 1; i >= 0; --i) {
> + if (sve_vls[i]) {
> + ret = arm_cpu_fls64(sve_vls[i]) + i * 64;
> + break;
> + }
> + }
> +
> + return ret;
> +}
> +
> +static int kvm_arm_set_sve_vls(CPUState *cs, uint64_t sve_vls[], int max_vq)
> +{
> + struct kvm_one_reg reg = {
> + .id = KVM_REG_ARM64_SVE_VLS,
> + .addr = (uint64_t)&sve_vls[0],
> + };
> + int i;
> +
> + for (i = KVM_ARM64_SVE_VLS_WORDS - 1; i >= 0; --i) {
> + if (sve_vls[i]) {
> + int vq = arm_cpu_fls64(sve_vls[i]) + i * 64;
nit: add a line
> + while (vq > max_vq) {
> + sve_vls[i] &= ~BIT_MASK(vq - 1);
Isn't BIT_MASK for 32b. MAKE_64BIT_MASK?
> + vq = arm_cpu_fls64(sve_vls[i]) + i * 64;
> + }
> + if (vq < max_vq) {
I don't really get this check: having vq less than max_vq does not seems
weird. Do you absolutely want vq=max_vq?
> + error_report("sve-max-vq=%d is not a valid length", max_vq);
> + error_printf("next lowest is %d\n", vq);
why mixing error_report/printf?
> + return -EINVAL;
> + }
> + if (vq == max_vq) {
> + break;
> + }
> + }
> + }
> +
> + return kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®);
> +}
> +
> static inline void set_feature(uint64_t *features, int feature)
> {
> *features |= 1ULL << feature;
> @@ -605,7 +658,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;
> }
>
> @@ -631,7 +684,12 @@ int kvm_arch_init_vcpu(CPUState *cs)
> }
> if (cpu->sve_max_vq) {
> if (!kvm_check_extension(cs->kvm_state, KVM_CAP_ARM_SVE)) {
> - cpu->sve_max_vq = 0;
> + if (cpu->sve_max_vq == -1) {> + cpu->sve_max_vq =
> 0;
> + } else {
> + error_report("This KVM host does not support SVE");
> + return -EINVAL;
> + }
> } else {
> cpu->kvm_init_features[0] |= 1 << KVM_ARM_VCPU_SVE;
> }
> @@ -644,6 +702,24 @@ int kvm_arch_init_vcpu(CPUState *cs)
> }
>
> if (cpu->sve_max_vq) {
> + uint64_t sve_vls[KVM_ARM64_SVE_VLS_WORDS];
line
> + ret = kvm_arm_get_sve_vls(cs, sve_vls);
> + if (ret < 0) {
> + return ret;
> + }
> + if (cpu->sve_max_vq == -1) {> + cpu->sve_max_vq = ret;
> + } else if (cpu->sve_max_vq > ret) {
> + error_report("This KVM host does not support SVE vectors "
I would rephrase the error mesg with something like:
This KVM host supports SVE vectors of max VQ=%d whereas requested VQ is %d
> + "of length %d quadwords (%d bytes)",
> + cpu->sve_max_vq, cpu->sve_max_vq * 16);
> + return -EINVAL;
> + } else {
> + ret = kvm_arm_set_sve_vls(cs, sve_vls, cpu->sve_max_vq);
> + if (ret < 0) {
> + return ret;
> + }
> + }
> ret = kvm_arm_vcpu_finalize(cs, KVM_ARM_VCPU_SVE);
> if (ret) {
> return ret;
>
Thanks
Eric
