All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] target/arm: Add nested virtualization support
@ 2021-03-22 10:07 Haibo Xu
  2021-03-22 10:07 ` [PATCH 1/3] Update linux header with new arm64 NV macro Haibo Xu
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Haibo Xu @ 2021-03-22 10:07 UTC (permalink / raw)
  To: qemu-devel, qemu-arm
  Cc: peter.maydell, drjones, richard.henderson, Haibo Xu, pbonzini, philmd

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



^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/3] Update linux header with new arm64 NV macro.
  2021-03-22 10:07 [PATCH 0/3] target/arm: Add nested virtualization support Haibo Xu
@ 2021-03-22 10:07 ` Haibo Xu
  2021-03-22 10:07 ` [PATCH 2/3] Enable support for setting KVM vGIC maintenance IRQ Haibo Xu
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 9+ messages in thread
From: Haibo Xu @ 2021-03-22 10:07 UTC (permalink / raw)
  To: qemu-devel, qemu-arm
  Cc: peter.maydell, drjones, richard.henderson, Haibo Xu, pbonzini, philmd

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



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/3] Enable support for setting KVM vGIC maintenance IRQ
  2021-03-22 10:07 [PATCH 0/3] target/arm: Add nested virtualization support Haibo Xu
  2021-03-22 10:07 ` [PATCH 1/3] Update linux header with new arm64 NV macro Haibo Xu
@ 2021-03-22 10:07 ` Haibo Xu
  2021-03-22 10:07 ` [PATCH 3/3] Enable nested virtualization support in arm64 KVM mode Haibo Xu
  2021-03-22 15:42 ` [PATCH 0/3] target/arm: Add nested virtualization support Andrea Bolognani
  3 siblings, 0 replies; 9+ messages in thread
From: Haibo Xu @ 2021-03-22 10:07 UTC (permalink / raw)
  To: qemu-devel, qemu-arm
  Cc: peter.maydell, drjones, richard.henderson, Haibo Xu, pbonzini, philmd

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



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 3/3] Enable nested virtualization support in arm64 KVM mode
  2021-03-22 10:07 [PATCH 0/3] target/arm: Add nested virtualization support Haibo Xu
  2021-03-22 10:07 ` [PATCH 1/3] Update linux header with new arm64 NV macro Haibo Xu
  2021-03-22 10:07 ` [PATCH 2/3] Enable support for setting KVM vGIC maintenance IRQ Haibo Xu
@ 2021-03-22 10:07 ` Haibo Xu
  2021-03-22 10:48   ` Andrew Jones
  2021-03-22 15:42 ` [PATCH 0/3] target/arm: Add nested virtualization support Andrea Bolognani
  3 siblings, 1 reply; 9+ messages in thread
From: Haibo Xu @ 2021-03-22 10:07 UTC (permalink / raw)
  To: qemu-devel, qemu-arm
  Cc: peter.maydell, drjones, richard.henderson, Haibo Xu, pbonzini, philmd

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



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 3/3] Enable nested virtualization support in arm64 KVM mode
  2021-03-22 10:07 ` [PATCH 3/3] Enable nested virtualization support in arm64 KVM mode Haibo Xu
@ 2021-03-22 10:48   ` Andrew Jones
  2021-03-23  7:08     ` Haibo Xu
  0 siblings, 1 reply; 9+ messages in thread
From: Andrew Jones @ 2021-03-22 10:48 UTC (permalink / raw)
  To: Haibo Xu
  Cc: peter.maydell, richard.henderson, qemu-devel, qemu-arm, pbonzini, philmd

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 



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 0/3] target/arm: Add nested virtualization support
  2021-03-22 10:07 [PATCH 0/3] target/arm: Add nested virtualization support Haibo Xu
                   ` (2 preceding siblings ...)
  2021-03-22 10:07 ` [PATCH 3/3] Enable nested virtualization support in arm64 KVM mode Haibo Xu
@ 2021-03-22 15:42 ` Andrea Bolognani
  2021-03-22 16:32   ` Andrew Jones
  3 siblings, 1 reply; 9+ messages in thread
From: Andrea Bolognani @ 2021-03-22 15:42 UTC (permalink / raw)
  To: Haibo Xu, qemu-devel, qemu-arm
  Cc: peter.maydell, drjones, philmd, richard.henderson, pbonzini

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



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 0/3] target/arm: Add nested virtualization support
  2021-03-22 15:42 ` [PATCH 0/3] target/arm: Add nested virtualization support Andrea Bolognani
@ 2021-03-22 16:32   ` Andrew Jones
  2021-03-23  7:00     ` Haibo Xu
  0 siblings, 1 reply; 9+ messages in thread
From: Andrew Jones @ 2021-03-22 16:32 UTC (permalink / raw)
  To: Andrea Bolognani
  Cc: peter.maydell, richard.henderson, qemu-devel, qemu-arm, Haibo Xu,
	pbonzini, philmd

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
> 
> 



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 0/3] target/arm: Add nested virtualization support
  2021-03-22 16:32   ` Andrew Jones
@ 2021-03-23  7:00     ` Haibo Xu
  0 siblings, 0 replies; 9+ messages in thread
From: Haibo Xu @ 2021-03-23  7:00 UTC (permalink / raw)
  To: Andrew Jones
  Cc: Peter Maydell, Richard Henderson, Andrea Bolognani,
	QEMU Developers, qemu-arm, pbonzini, Philippe Mathieu-Daudé

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
> >
> >
>


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 3/3] Enable nested virtualization support in arm64 KVM mode
  2021-03-22 10:48   ` Andrew Jones
@ 2021-03-23  7:08     ` Haibo Xu
  0 siblings, 0 replies; 9+ messages in thread
From: Haibo Xu @ 2021-03-23  7:08 UTC (permalink / raw)
  To: Andrew Jones
  Cc: Peter Maydell, Richard Henderson, QEMU Developers, qemu-arm,
	pbonzini, Philippe Mathieu-Daudé

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
>


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2021-03-23  7:09 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-22 10:07 [PATCH 0/3] target/arm: Add nested virtualization support Haibo Xu
2021-03-22 10:07 ` [PATCH 1/3] Update linux header with new arm64 NV macro Haibo Xu
2021-03-22 10:07 ` [PATCH 2/3] Enable support for setting KVM vGIC maintenance IRQ Haibo Xu
2021-03-22 10:07 ` [PATCH 3/3] Enable nested virtualization support in arm64 KVM mode Haibo Xu
2021-03-22 10:48   ` Andrew Jones
2021-03-23  7:08     ` Haibo Xu
2021-03-22 15:42 ` [PATCH 0/3] target/arm: Add nested virtualization support Andrea Bolognani
2021-03-22 16:32   ` Andrew Jones
2021-03-23  7:00     ` Haibo Xu

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.