[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-arm] [Qemu-devel] [PATCH 06/13] target/arm/kvm: max cpu: Enabl
From: |
Andrew Jones |
Subject: |
Re: [Qemu-arm] [Qemu-devel] [PATCH 06/13] target/arm/kvm: max cpu: Enable SVE when available |
Date: |
Wed, 5 Jun 2019 13:04:44 +0200 |
User-agent: |
NeoMutt/20180716 |
On Wed, Jun 05, 2019 at 11:09:56AM +0200, Auger Eric wrote:
> Hi Drew,
>
> On 5/12/19 10:36 AM, Andrew Jones wrote:
> > Enable SVE in the KVM guest when the 'max' cpu type is configured
> > and KVM supports it. KVM SVE requires use of the new finalize
> > vcpu ioctl, so we add that now too.
> >
> > Signed-off-by: Andrew Jones <address@hidden>
> > ---
> > target/arm/cpu64.c | 1 +
> > target/arm/kvm.c | 5 +++++
> > target/arm/kvm64.c | 16 +++++++++++++++-
> > target/arm/kvm_arm.h | 12 ++++++++++++
> > 4 files changed, 33 insertions(+), 1 deletion(-)
> >
> > diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
> > index 228906f26786..6c19ef6837d5 100644
> > --- a/target/arm/cpu64.c
> > +++ b/target/arm/cpu64.c
> > @@ -292,6 +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;
> same line in the !kvm_enabled path. Maybe you can set the sve_max_vq
> field in a subsequent patch and just introduce the finalize and
> capability checking in that patch?
This gets changed in a subsequent patch, so factoring now would
be wasted code motion. I'm not sure the finalize function is worth
its own patch, so I'm inclined to leave this as is.
> > } else {
> > uint64_t t;
> > uint32_t u;
> > diff --git a/target/arm/kvm.c b/target/arm/kvm.c
> > index 599563461264..c51db4229d0f 100644
> > --- a/target/arm/kvm.c
> > +++ b/target/arm/kvm.c
> > @@ -50,6 +50,11 @@ int kvm_arm_vcpu_init(CPUState *cs)
> > return kvm_vcpu_ioctl(cs, KVM_ARM_VCPU_INIT, &init);
> > }
> >
> > +int kvm_arm_vcpu_finalize(CPUState *cs, int feature)
> > +{
> > + return kvm_vcpu_ioctl(cs, KVM_ARM_VCPU_FINALIZE, &feature);
> > +}
> > +
> > void kvm_arm_init_serror_injection(CPUState *cs)
> > {
> > cap_has_inject_serror_esr = kvm_check_extension(cs->kvm_state,
> > diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
> > index 86362f4cd7d0..c2d92df75353 100644
> > --- a/target/arm/kvm64.c
> > +++ b/target/arm/kvm64.c
> > @@ -622,13 +622,20 @@ int kvm_arch_init_vcpu(CPUState *cs)
> > cpu->kvm_init_features[0] |= 1 << KVM_ARM_VCPU_EL1_32BIT;
> > }
> > if (!kvm_check_extension(cs->kvm_state, KVM_CAP_ARM_PMU_V3)) {
> > - cpu->has_pmu = false;
> > + cpu->has_pmu = false;
> nit: maybe document this unrelated indent fix in the commit msg?
It's pretty obvious without extra commentary, IMHO.
> > }
> > if (cpu->has_pmu) {
> > cpu->kvm_init_features[0] |= 1 << KVM_ARM_VCPU_PMU_V3;
> > } else {
> > unset_feature(&env->features, ARM_FEATURE_PMU);
> > }
> > + if (cpu->sve_max_vq) {
> > + if (!kvm_check_extension(cs->kvm_state, KVM_CAP_ARM_SVE)) {
> > + cpu->sve_max_vq = 0;
> > + } else {
> > + cpu->kvm_init_features[0] |= 1 << KVM_ARM_VCPU_SVE;
> > + }
> > + }
> >
> > /* Do KVM_ARM_VCPU_INIT ioctl */
> > ret = kvm_arm_vcpu_init(cs);
> > @@ -636,6 +643,13 @@ int kvm_arch_init_vcpu(CPUState *cs)
> > return ret;
> > }
> >
> > + if (cpu->sve_max_vq) {
> > + ret = kvm_arm_vcpu_finalize(cs, KVM_ARM_VCPU_SVE);
> > + if (ret) {
> > + return ret;
> > + }
> > + }
> > +
> > /*
> > * When KVM is in use, PSCI is emulated in-kernel and not by qemu.
> > * Currently KVM has its own idea about MPIDR assignment, so we
> > diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h
> > index 2a07333c615f..c488ec3ab410 100644
> > --- a/target/arm/kvm_arm.h
> > +++ b/target/arm/kvm_arm.h
> > @@ -27,6 +27,18 @@
> > */
> > int kvm_arm_vcpu_init(CPUState *cs);
> >
> > +/**
> > + * kvm_arm_vcpu_finalize
> > + * @cs: CPUState
> > + * @feature: int
> feature bitmap or bit?
Neither. I can improve this by stating these integers must be one
of the set defined in the "KVM_ARM_VCPU_FINALIZE" section of
kernel doc Documentation/virtual/kvm/api.txt though.
> > + *
> > + * Finalizes the configuration of the specified VCPU feature
> > + * by invoking the KVM_ARM_VCPU_FINALIZE ioctl.
> > + *
> > + * Returns: 0 if success else < 0 error code
> > + */
> > +int kvm_arm_vcpu_finalize(CPUState *cs, int feature);
> > +
> > /**
> > * kvm_arm_register_device:
> > * @mr: memory region for this device
> >
>
Thanks,
drew