Re: [PATCH 3/3] Enable nested virtualization support in arm64 KVM mode

[Haibo Xu]

On Mon, 22 Mar 2021 at 18:49, Andrew Jones <drjones@redhat.com> wrote:
>
> On Mon, Mar 22, 2021 at 10:07:26AM +0000, Haibo Xu wrote:
> > Add support for arm64 el2 in qemu KVM mode(nested virtualization).
> > This feature is disabled by default, just as that in TCG mode, and
> > can be enabled by "-M virt,accel=kvm,virtualization=on" when starting
> > a VM.
> >
> > Signed-off-by: Haibo Xu <haibo.xu@linaro.org>
> > ---
> >  hw/arm/virt.c        | 11 ++++++++---
> >  target/arm/cpu.h     |  8 ++++++++
> >  target/arm/kvm64.c   | 14 ++++++++++++++
> >  target/arm/kvm_arm.h | 28 ++++++++++++++++++++++++++++
> >  4 files changed, 58 insertions(+), 3 deletions(-)
> >
> > diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> > index aa2bbd14e0..72e60348d5 100644
> > --- a/hw/arm/virt.c
> > +++ b/hw/arm/virt.c
> > @@ -663,6 +663,11 @@ static void create_gic(VirtMachineState *vms)
> >              qdev_prop_set_uint32(vms->gic, "redist-region-count[1]",
> >                  MIN(smp_cpus - redist0_count, redist1_capacity));
> >          }
> > +
> > +        if (kvm_irqchip_in_kernel()) {
> > +            qdev_prop_set_bit(vms->gic, "has-virtualization-extensions",
> > +                              vms->virt);
> > +        }
> >      } else {
> >          if (!kvm_irqchip_in_kernel()) {
> >              qdev_prop_set_bit(vms->gic, "has-virtualization-extensions",
> > @@ -1905,9 +1910,9 @@ static void machvirt_init(MachineState *machine)
> >          exit(1);
> >      }
> >
> > -    if (vms->virt && kvm_enabled()) {
> > -        error_report("mach-virt: KVM does not support providing "
> > -                     "Virtualization extensions to the guest CPU");
> > +    if (vms->virt && kvm_enabled() && !kvm_arm_nested_virt_supported()) {
> > +        error_report("mach-virt: nested virtualization requested, "
> > +                     "but not supported by the host.");
> >          exit(1);
> >      }
> >
> > diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> > index 193a49ec7f..377187152b 100644
> > --- a/target/arm/cpu.h
> > +++ b/target/arm/cpu.h
> > @@ -4182,6 +4182,14 @@ static inline bool isar_feature_aa64_ssbs(const 
> > ARMISARegisters *id)
> >      return FIELD_EX64(id->id_aa64pfr1, ID_AA64PFR1, SSBS) != 0;
> >  }
> >
> > +/*
> > + * Currently we don't differentiate between the ARMv8.3-NV and ARMv8.4-NV.
> > + */
> > +static inline bool isar_feature_aa64_nv(const ARMISARegisters *id)
> > +{
> > +    return FIELD_EX64(id->id_aa64mmfr2, ID_AA64MMFR2, NV) != 0;
> > +}
>
> What calls this function?
>

Sorry for the noise here! This function should be deleted in this patch!

Previously, I wanted to follow the SVE vCPU feature when adding the NV support,
and the above function is supposed to be used for the feature probe.

Thanks,
Haibo

> > +
> >  /*
> >   * Feature tests for "does this exist in either 32-bit or 64-bit?"
> >   */
> > diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
> > index dff85f6db9..2810104dea 100644
> > --- a/target/arm/kvm64.c
> > +++ b/target/arm/kvm64.c
> > @@ -500,6 +500,7 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures 
> > *ahcf)
> >       */
> >      int fdarray[3];
> >      bool sve_supported;
> > +    bool el2_supported;
> >      uint64_t features = 0;
> >      uint64_t t;
> >      int err;
> > @@ -646,6 +647,7 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures 
> > *ahcf)
> >      }
> >
> >      sve_supported = ioctl(fdarray[0], KVM_CHECK_EXTENSION, 
> > KVM_CAP_ARM_SVE) > 0;
> > +    el2_supported = ioctl(fdarray[0], KVM_CHECK_EXTENSION, 
> > KVM_CAP_ARM_EL2) > 0;
> >
> >      kvm_arm_destroy_scratch_host_vcpu(fdarray);
> >
> > @@ -671,6 +673,10 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures 
> > *ahcf)
> >      features |= 1ULL << ARM_FEATURE_PMU;
> >      features |= 1ULL << ARM_FEATURE_GENERIC_TIMER;
> >
> > +    if (el2_supported) {
> > +        features |= 1ULL << ARM_FEATURE_EL2;
> > +    }
> > +
> >      ahcf->features = features;
> >
> >      return true;
> > @@ -721,6 +727,11 @@ bool kvm_arm_steal_time_supported(void)
> >      return kvm_check_extension(kvm_state, KVM_CAP_STEAL_TIME);
> >  }
> >
> > +bool kvm_arm_nested_virt_supported(void)
> > +{
> > +    return kvm_check_extension(kvm_state, KVM_CAP_ARM_EL2);
> > +}
> > +
> >  QEMU_BUILD_BUG_ON(KVM_ARM64_SVE_VQ_MIN != 1);
> >
> >  void kvm_arm_sve_get_vls(CPUState *cs, unsigned long *map)
> > @@ -856,6 +867,9 @@ int kvm_arch_init_vcpu(CPUState *cs)
> >          assert(kvm_arm_sve_supported());
> >          cpu->kvm_init_features[0] |= 1 << KVM_ARM_VCPU_SVE;
> >      }
> > +    if (cpu->has_el2) {
> > +        cpu->kvm_init_features[0] |= 1 << KVM_ARM_VCPU_HAS_EL2;
> > +    }
> >
> >      /* Do KVM_ARM_VCPU_INIT ioctl */
> >      ret = kvm_arm_vcpu_init(cs);
> > diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h
> > index 34f8daa377..da3a3d5920 100644
> > --- a/target/arm/kvm_arm.h
> > +++ b/target/arm/kvm_arm.h
> > @@ -285,6 +285,24 @@ void kvm_arm_steal_time_finalize(ARMCPU *cpu, Error 
> > **errp);
> >   */
> >  bool kvm_arm_steal_time_supported(void);
> >
> > +/**
> > + * kvm_arm_nested_virt_finalize:
> > + * @cpu: ARMCPU for which to finalize nested-virt
> > + * @errp: Pointer to Error* for error propagation
> > + *
> > + * Validate the nested-virt property selection and set its default
> > + * based on KVM support and guest configuration.
> > + */
> > +void kvm_arm_nested_virt_finalize(ARMCPU *cpu, Error **errp);
>
> Where is this function defined? From where is it called?
>

Same reason here!

> > +
> > +/**
> > + * kvm_arm_nested_virt_supported:
> > + *
> > + * Returns: true if KVM can enable nested virtualization
> > + * and false otherwise.
> > + */
> > +bool kvm_arm_nested_virt_supported(void);
> > +
> >  /**
> >   * kvm_arm_aarch32_supported:
> >   *
> > @@ -398,6 +416,11 @@ static inline bool kvm_arm_steal_time_supported(void)
> >      return false;
> >  }
> >
> > +static inline bool kvm_arm_nested_virt_supported(void)
> > +{
> > +    return false;
> > +}
> > +
> >  /*
> >   * These functions should never actually be called without KVM support.
> >   */
> > @@ -441,6 +464,11 @@ static inline void kvm_arm_steal_time_finalize(ARMCPU 
> > *cpu, Error **errp)
> >      g_assert_not_reached();
> >  }
> >
> > +static inline void kvm_arm_nested_virt_finalize(ARMCPU *cpu, Error **errp)
> > +{
> > +    g_assert_not_reached();
> > +}
> > +
> >  static inline void kvm_arm_sve_get_vls(CPUState *cs, unsigned long *map)
> >  {
> >      g_assert_not_reached();
> > --
> > 2.17.1
> >
>
> Thanks,
> drew
>