[PATCH 0/3] target/arm: Add nested virtualization support

[PATCH 0/3] target/arm: Add nested virtualization support

[Haibo Xu]

This series add support for ARMv8.3/8.4 nested virtualization support
in KVM mode. It's based on Marc Zyngier's kernel KVM patches[1], and
has been tested on a FVP model to run a L2 guest with Qemu. Now the
feature can be enabled by "-M virt,accel=kvm,virtualization=on" when
starting a VM.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git/log/?h=kvm-arm64/nv-5.12-WIP

Haibo Xu (3):
  Update linux header with new arm64 NV macro.
  Enable support for setting KVM vGIC maintenance IRQ
  Enable nested virtualization support in arm64 KVM mode

 hw/arm/virt.c                      | 11 ++++++++---
 hw/intc/arm_gicv3_common.c         |  1 +
 hw/intc/arm_gicv3_kvm.c            | 16 ++++++++++++++++
 include/hw/intc/arm_gicv3_common.h |  1 +
 linux-headers/asm-arm64/kvm.h      |  2 ++
 linux-headers/linux/kvm.h          |  1 +
 target/arm/cpu.h                   |  8 ++++++++
 target/arm/kvm64.c                 | 14 ++++++++++++++
 target/arm/kvm_arm.h               | 28 ++++++++++++++++++++++++++++
 9 files changed, 79 insertions(+), 3 deletions(-)

-- 
2.17.1

[PATCH 1/3] Update linux header with new arm64 NV macro.

[Haibo Xu]

Signed-off-by: Haibo Xu <haibo.xu@linaro.org>
---
 linux-headers/asm-arm64/kvm.h | 2 ++
 linux-headers/linux/kvm.h     | 1 +
 2 files changed, 3 insertions(+)

diff --git a/linux-headers/asm-arm64/kvm.h b/linux-headers/asm-arm64/kvm.h
index b6a0eaa32a..77b995a26c 100644
--- a/linux-headers/asm-arm64/kvm.h
+++ b/linux-headers/asm-arm64/kvm.h
@@ -106,6 +106,7 @@ struct kvm_regs {
 #define KVM_ARM_VCPU_SVE		4 /* enable SVE for this CPU */
 #define KVM_ARM_VCPU_PTRAUTH_ADDRESS	5 /* VCPU uses address authentication */
 #define KVM_ARM_VCPU_PTRAUTH_GENERIC	6 /* VCPU uses generic authentication */
+#define KVM_ARM_VCPU_HAS_EL2		7 /* Support nested virtualization */
 
 struct kvm_vcpu_init {
 	__u32 target;
@@ -334,6 +335,7 @@ struct kvm_vcpu_events {
 #define KVM_DEV_ARM_VGIC_GRP_CPU_SYSREGS 6
 #define KVM_DEV_ARM_VGIC_GRP_LEVEL_INFO  7
 #define KVM_DEV_ARM_VGIC_GRP_ITS_REGS 8
+#define KVM_DEV_ARM_VGIC_GRP_MAINT_IRQ 9
 #define KVM_DEV_ARM_VGIC_LINE_LEVEL_INFO_SHIFT	10
 #define KVM_DEV_ARM_VGIC_LINE_LEVEL_INFO_MASK \
 			(0x3fffffULL << KVM_DEV_ARM_VGIC_LINE_LEVEL_INFO_SHIFT)
diff --git a/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h
index 020b62a619..ce4630c4db 100644
--- a/linux-headers/linux/kvm.h
+++ b/linux-headers/linux/kvm.h
@@ -1056,6 +1056,7 @@ struct kvm_ppc_resize_hpt {
 #define KVM_CAP_ENFORCE_PV_FEATURE_CPUID 190
 #define KVM_CAP_SYS_HYPERV_CPUID 191
 #define KVM_CAP_DIRTY_LOG_RING 192
+#define KVM_CAP_ARM_EL2 193
 
 #ifdef KVM_CAP_IRQ_ROUTING
 
-- 
2.17.1

[PATCH 2/3] Enable support for setting KVM vGIC maintenance IRQ

[Haibo Xu]

Uses the new VGIC KVM device attribute to set the maintenance IRQ.
This is fixed to use IRQ 25(PPI 9), as a platform decision matching
the arm64 SBSA recommendation.

Signed-off-by: Haibo Xu <haibo.xu@linaro.org>
---
 hw/intc/arm_gicv3_common.c         |  1 +
 hw/intc/arm_gicv3_kvm.c            | 16 ++++++++++++++++
 include/hw/intc/arm_gicv3_common.h |  1 +
 3 files changed, 18 insertions(+)

diff --git a/hw/intc/arm_gicv3_common.c b/hw/intc/arm_gicv3_common.c
index 58ef65f589..3ac10c8e61 100644
--- a/hw/intc/arm_gicv3_common.c
+++ b/hw/intc/arm_gicv3_common.c
@@ -495,6 +495,7 @@ static Property arm_gicv3_common_properties[] = {
     DEFINE_PROP_UINT32("num-irq", GICv3State, num_irq, 32),
     DEFINE_PROP_UINT32("revision", GICv3State, revision, 3),
     DEFINE_PROP_BOOL("has-security-extensions", GICv3State, security_extn, 0),
+    DEFINE_PROP_BOOL("has-virtualization-extensions", GICv3State, virt_extn, 0),
     DEFINE_PROP_ARRAY("redist-region-count", GICv3State, nb_redist_regions,
                       redist_region_count, qdev_prop_uint32, uint32_t),
     DEFINE_PROP_END_OF_LIST(),
diff --git a/hw/intc/arm_gicv3_kvm.c b/hw/intc/arm_gicv3_kvm.c
index 65a4c880a3..1e1ca66e2c 100644
--- a/hw/intc/arm_gicv3_kvm.c
+++ b/hw/intc/arm_gicv3_kvm.c
@@ -826,6 +826,22 @@ static void kvm_arm_gicv3_realize(DeviceState *dev, Error **errp)
     kvm_device_access(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_CTRL,
                       KVM_DEV_ARM_VGIC_CTRL_INIT, NULL, true, &error_abort);
 
+    if (s->virt_extn) {
+        bool maint_irq_allowed;
+        uint32_t maint_irq = 25;
+
+        maint_irq_allowed =
+            kvm_device_check_attr(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_MAINT_IRQ, 0);
+        if (!maint_irq_allowed) {
+            error_setg(errp, "VGICv3 setting maintenance IRQ are not "
+                             "supported by this host kernel");
+            return;
+        }
+
+        kvm_device_access(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_MAINT_IRQ,
+                          0, &maint_irq, true, &error_abort);
+    }
+
     kvm_arm_register_device(&s->iomem_dist, -1, KVM_DEV_ARM_VGIC_GRP_ADDR,
                             KVM_VGIC_V3_ADDR_TYPE_DIST, s->dev_fd, 0);
 
diff --git a/include/hw/intc/arm_gicv3_common.h b/include/hw/intc/arm_gicv3_common.h
index 91491a2f66..921ddc2c5f 100644
--- a/include/hw/intc/arm_gicv3_common.h
+++ b/include/hw/intc/arm_gicv3_common.h
@@ -220,6 +220,7 @@ struct GICv3State {
     uint32_t num_irq;
     uint32_t revision;
     bool security_extn;
+    bool virt_extn;
     bool irq_reset_nonsecure;
     bool gicd_no_migration_shift_bug;
 
-- 
2.17.1

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

[Haibo Xu]

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;
+}
+
 /*
  * 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);
+
+/**
+ * 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

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

[Andrew Jones]

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?

> +
>  /*
>   * 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?

> +
> +/**
> + * 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 

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
>

Re: [PATCH 0/3] target/arm: Add nested virtualization support

[Andrea Bolognani]

On Mon, 2021-03-22 at 10:07 +0000, Haibo Xu wrote:
> This series add support for ARMv8.3/8.4 nested virtualization support
> in KVM mode. It's based on Marc Zyngier's kernel KVM patches[1], and
> has been tested on a FVP model to run a L2 guest with Qemu. Now the
> feature can be enabled by "-M virt,accel=kvm,virtualization=on" when
> starting a VM.

Why the need to enable this explicitly? AFAIK, that's not necessary
for any other architecture: on x86, you just need to make sure you're
using '-cpu host' and pass a parameter to the kernel module.

Even assuming this can't be enabled transparently, wouldn't its
availability it be controlled by a CPU feature flag, similar to what
already happens for SVE and PMU, rather than a machine type option?

That would also address the discoverability issue: unless I'm
mistaken (which I very well might be :), with the current
implementation there's no way to tell whether nested KVM will be
usable short of trying and seeing whether QEMU errors out.

-- 
Andrea Bolognani / Red Hat / Virtualization

Re: [PATCH 0/3] target/arm: Add nested virtualization support

[Andrew Jones]

On Mon, Mar 22, 2021 at 04:42:23PM +0100, Andrea Bolognani wrote:
> On Mon, 2021-03-22 at 10:07 +0000, Haibo Xu wrote:
> > This series add support for ARMv8.3/8.4 nested virtualization support
> > in KVM mode. It's based on Marc Zyngier's kernel KVM patches[1], and
> > has been tested on a FVP model to run a L2 guest with Qemu. Now the
> > feature can be enabled by "-M virt,accel=kvm,virtualization=on" when
> > starting a VM.
> 
> Why the need to enable this explicitly? AFAIK, that's not necessary
> for any other architecture: on x86, you just need to make sure you're
> using '-cpu host' and pass a parameter to the kernel module.
> 
> Even assuming this can't be enabled transparently, wouldn't its
> availability it be controlled by a CPU feature flag, similar to what
> already happens for SVE and PMU, rather than a machine type option?

I 100% agree. We should control this feature with a CPU feature property.
NV is a CPU feature, after all. Also, we should add it to the properties
that we can probe in cpu_model_advertised_features[].

Thanks,
drew

> 
> That would also address the discoverability issue: unless I'm
> mistaken (which I very well might be :), with the current
> implementation there's no way to tell whether nested KVM will be
> usable short of trying and seeing whether QEMU errors out.
> 
> -- 
> Andrea Bolognani / Red Hat / Virtualization
> 
> 

Re: [PATCH 0/3] target/arm: Add nested virtualization support

[Haibo Xu]

On Tue, 23 Mar 2021 at 00:32, Andrew Jones <drjones@redhat.com> wrote:
>
> On Mon, Mar 22, 2021 at 04:42:23PM +0100, Andrea Bolognani wrote:
> > On Mon, 2021-03-22 at 10:07 +0000, Haibo Xu wrote:
> > > This series add support for ARMv8.3/8.4 nested virtualization support
> > > in KVM mode. It's based on Marc Zyngier's kernel KVM patches[1], and
> > > has been tested on a FVP model to run a L2 guest with Qemu. Now the
> > > feature can be enabled by "-M virt,accel=kvm,virtualization=on" when
> > > starting a VM.
> >
> > Why the need to enable this explicitly? AFAIK, that's not necessary
> > for any other architecture: on x86, you just need to make sure you're
> > using '-cpu host' and pass a parameter to the kernel module.
> >
> > Even assuming this can't be enabled transparently, wouldn't its
> > availability it be controlled by a CPU feature flag, similar to what
> > already happens for SVE and PMU, rather than a machine type option?
>
> I 100% agree. We should control this feature with a CPU feature property.
> NV is a CPU feature, after all. Also, we should add it to the properties
> that we can probe in cpu_model_advertised_features[].
>

Thanks so much for the comments!

Yes, NV should be a vCPU feature, just as the kernel macro
KVM_ARM_VCPU_HAS_EL2 implies.
To implement it as a VM feature here just want to reuse the codes in
TCG mode which emulate a
guest CPU with virtualization extension support.

I will change the NV in KVM mode to a vCPU feature in  the next version.
@Peter Maydell and @Richard Henderson, shall we change that in TCG mode as well?

Thanks,
Haibo

> Thanks,
> drew
>
> >
> > That would also address the discoverability issue: unless I'm
> > mistaken (which I very well might be :), with the current
> > implementation there's no way to tell whether nested KVM will be
> > usable short of trying and seeing whether QEMU errors out.
> >
> > --
> > Andrea Bolognani / Red Hat / Virtualization
> >
> >
>