[RFC PATCH 0/5] QEMU v7.2.0 aarch64 Nested Virtualization Support

[RFC PATCH 0/5] QEMU v7.2.0 aarch64 Nested Virtualization Support

[Miguel Luis]

This series adds ARMv8.3/8.4 nested virtualization support in KVM mode.
    
To enable nested virtualization for a guest, the host must expose EL2
support via QEMU command line switches:

-machine virt,accel=kvm,virtualization=on

Inspired on Haibo Xu's previous work [0][1], Marc Zyngier's kvmtool branch [2]
and kernel patches [3] on nested virtualization for aarch64, this has been
tested on an Ampere implementation.

This series adapts previous work on top of v7.2.0, it considers comments given
at the time and preserves authorship of the original patches.

[0]: https://lore.kernel.org/qemu-devel/cover.1616052889.git.haibo.xu@linaro.org/
[1]: https://lore.kernel.org/qemu-devel/cover.1617281290.git.haibo.xu@linaro.org/
[2]: https://git.kernel.org/pub/scm/linux/kernel/git/maz/kvmtool.git/log/?h=arm64/nv-5.16
[3]: https://lore.kernel.org/linux-arm-kernel/20230131092504.2880505-1-maz@kernel.org/

Miguel Luis (5):
  linux-headers: [kvm,arm64] add the necessary definitions to match host
    kernel
  hw/intc/gicv3: add support for setting KVM vGIC maintenance IRQ
  target/arm/kvm: add helper to detect EL2 when using KVM
  target/arm: enable feature ARM_FEATURE_EL2 if EL2 is supported
  arm/virt: provide virtualization extensions to the guest

 hw/arm/virt.c                      |  8 +++++++-
 hw/intc/arm_gicv3_common.c         |  1 +
 hw/intc/arm_gicv3_kvm.c            | 25 +++++++++++++++++++++++++
 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                   |  2 +-
 target/arm/kvm64.c                 | 21 +++++++++++++++++++++
 target/arm/kvm_arm.h               | 12 ++++++++++++
 9 files changed, 71 insertions(+), 2 deletions(-)

-- 
2.39.2

[RFC PATCH 1/5] linux-headers: [kvm, arm64] add the necessary definitions to match host kernel

[Miguel Luis]

From: Haibo Xu <haibo.xu@linaro.org>

linux-headers define host properties needed for the VMM to interact with
KVM, so let's include them *while* they're not available yet on linux
upstream since aarch64 nested virtualization is still a work in
progress.

Ref: https://lore.kernel.org/qemu-devel/636b5932e4cf061b6f97516e82d4319c1d29b871.1616052889.git.haibo.xu@linaro.org/

Signed-off-by: Haibo Xu <haibo.xu@linaro.org>
Signed-off-by: Miguel Luis <miguel.luis@oracle.com>
---
 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 4bf2d7246e..7bbcba7418 100644
--- a/linux-headers/asm-arm64/kvm.h
+++ b/linux-headers/asm-arm64/kvm.h
@@ -108,6 +108,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;
@@ -379,6 +380,7 @@ enum {
 #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 ebdafa576d..bfd1d73988 100644
--- a/linux-headers/linux/kvm.h
+++ b/linux-headers/linux/kvm.h
@@ -1175,6 +1175,7 @@ struct kvm_ppc_resize_hpt {
 #define KVM_CAP_VM_DISABLE_NX_HUGE_PAGES 220
 #define KVM_CAP_S390_ZPCI_OP 221
 #define KVM_CAP_S390_CPU_TOPOLOGY 222
+#define KVM_CAP_ARM_EL2 226
 
 #ifdef KVM_CAP_IRQ_ROUTING
 
-- 
2.39.2

[RFC PATCH 2/5] hw/intc/gicv3: add support for setting KVM vGIC maintenance IRQ

[Miguel Luis]

From: Haibo Xu <haibo.xu@linaro.org>

Use the VGIC maintenance IRQ if VHE is requested. As per the ARM GIC
Architecture Specification for GICv3 and GICv4 Arm strongly recommends that
maintenance interrupts are configured to use INTID 25 matching the
Server Base System Architecture (SBSA) recomendation.

Ref: https://lore.kernel.org/qemu-devel/49a4944e2f148c56938380b981afe154b7a8b7ee.1617281290.git.haibo.xu@linaro.org/

Signed-off-by: Haibo Xu <haibo.xu@linaro.org>
[Miguel Luis: avoid direct usage of helpers (_check_attr(); _access())]
Signed-off-by: Miguel Luis <miguel.luis@oracle.com>
---
 hw/arm/virt.c                      |  5 +++++
 hw/intc/arm_gicv3_common.c         |  1 +
 hw/intc/arm_gicv3_kvm.c            | 25 +++++++++++++++++++++++++
 include/hw/intc/arm_gicv3_common.h |  1 +
 4 files changed, 32 insertions(+)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index b871350856..377181e009 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -759,6 +759,11 @@ static void create_gic(VirtMachineState *vms, MemoryRegion *mem)
             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",
diff --git a/hw/intc/arm_gicv3_common.c b/hw/intc/arm_gicv3_common.c
index 351843db4a..e2a6ff1b49 100644
--- a/hw/intc/arm_gicv3_common.c
+++ b/hw/intc/arm_gicv3_common.c
@@ -563,6 +563,7 @@ static Property arm_gicv3_common_properties[] = {
     DEFINE_PROP_UINT32("revision", GICv3State, revision, 3),
     DEFINE_PROP_BOOL("has-lpi", GICv3State, lpi_enable, 0),
     DEFINE_PROP_BOOL("has-security-extensions", GICv3State, security_extn, 0),
+    DEFINE_PROP_BOOL("has-virtualization-extensions", GICv3State, virt_extn, 0),
     /*
      * Compatibility property: force 8 bits of physical priority, even
      * if the CPU being emulated should have fewer.
diff --git a/hw/intc/arm_gicv3_kvm.c b/hw/intc/arm_gicv3_kvm.c
index 3ca643ecba..ce924753bb 100644
--- a/hw/intc/arm_gicv3_kvm.c
+++ b/hw/intc/arm_gicv3_kvm.c
@@ -22,6 +22,7 @@
 #include "qemu/osdep.h"
 #include "qapi/error.h"
 #include "hw/intc/arm_gicv3_common.h"
+#include "hw/arm/virt.h"
 #include "qemu/error-report.h"
 #include "qemu/module.h"
 #include "sysemu/kvm.h"
@@ -803,6 +804,30 @@ static void kvm_arm_gicv3_realize(DeviceState *dev, Error **errp)
         return;
     }
 
+
+    if (s->virt_extn) {
+        /*
+         * Arm strongly recommends that maintenance interrupts are configured
+         * to use INTID 25. For more information, see Server Base System
+         * Architecture (SBSA)
+         */
+        uint32_t maint_irq = PPI(ARCH_GIC_MAINT_IRQ);
+
+        struct kvm_device_attr kdevattr = {
+            .group = KVM_DEV_ARM_VGIC_GRP_MAINT_IRQ,
+            .addr = (uint64_t)&maint_irq
+        };
+
+        if (!kvm_device_ioctl(s->dev_fd, KVM_GET_DEVICE_ATTR, &kdevattr)) {
+            error_setg(errp, "VGICv3 setting maintenance IRQ are not "
+                            "supported by this host kernel");
+            return;
+        }
+
+        kvm_device_ioctl(s->dev_fd, KVM_SET_DEVICE_ATTR, &kdevattr);
+    }
+
+
     gicv3_init_irqs_and_mmio(s, kvm_arm_gicv3_set_irq, NULL);
 
     for (i = 0; i < s->num_cpu; i++) {
diff --git a/include/hw/intc/arm_gicv3_common.h b/include/hw/intc/arm_gicv3_common.h
index ab5182a28a..91e1c1a45a 100644
--- a/include/hw/intc/arm_gicv3_common.h
+++ b/include/hw/intc/arm_gicv3_common.h
@@ -248,6 +248,7 @@ struct GICv3State {
     uint32_t revision;
     bool lpi_enable;
     bool security_extn;
+    bool virt_extn;
     bool force_8bit_prio;
     bool irq_reset_nonsecure;
     bool gicd_no_migration_shift_bug;
-- 
2.39.2

[RFC PATCH 3/5] target/arm/kvm: add helper to detect EL2 when using KVM

[Miguel Luis]

From: Haibo Xu <haibo.xu@linaro.org>

Introduce query support for KVM_CAP_ARM_EL2.

Ref: https://lore.kernel.org/qemu-devel/65b8771bfecada08bf02c9cf87c2f0f9cdf943b3.1617281290.git.haibo.xu@linaro.org/

Signed-off-by: Haibo Xu <haibo.xu@linaro.org>
Signed-off-by: Miguel Luis <miguel.luis@oracle.com>
---
 target/arm/kvm64.c   |  5 +++++
 target/arm/kvm_arm.h | 12 ++++++++++++
 2 files changed, 17 insertions(+)

diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
index 1197253d12..be8144a2b5 100644
--- a/target/arm/kvm64.c
+++ b/target/arm/kvm64.c
@@ -754,6 +754,11 @@ bool kvm_arm_aarch32_supported(void)
     return kvm_check_extension(kvm_state, KVM_CAP_ARM_EL1_32BIT);
 }
 
+bool kvm_arm_el2_supported(void)
+{
+    return kvm_check_extension(kvm_state, KVM_CAP_ARM_EL2);
+}
+
 bool kvm_arm_sve_supported(void)
 {
     return kvm_check_extension(kvm_state, KVM_CAP_ARM_SVE);
diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h
index 99017b635c..86a0cb4308 100644
--- a/target/arm/kvm_arm.h
+++ b/target/arm/kvm_arm.h
@@ -305,6 +305,13 @@ bool kvm_arm_pmu_supported(void);
  */
 bool kvm_arm_sve_supported(void);
 
+/**
+ * kvm_arm_el2_supported:
+ *
+ * Returns true if KVM can enable EL2 and false otherwise.
+ */
+bool kvm_arm_el2_supported(void);
+
 /**
  * kvm_arm_get_max_vm_ipa_size:
  * @ms: Machine state handle
@@ -395,6 +402,11 @@ static inline bool kvm_arm_steal_time_supported(void)
     return false;
 }
 
+static inline bool kvm_arm_el2_supported(void)
+{
+    return false;
+}
+
 /*
  * These functions should never actually be called without KVM support.
  */
-- 
2.39.2

[RFC PATCH 4/5] target/arm: enable feature ARM_FEATURE_EL2 if EL2 is supported

[Miguel Luis]

From: Haibo Xu <haibo.xu@linaro.org>

KVM_CAP_ARM_EL2 must be supported by the cpu to enable ARM_FEATURE_EL2.
EL2 bits on ID_AA64PFR0 state unsupported on the value 0b0000.

Ref: https://lore.kernel.org/qemu-devel/b7c2626e6c720ccc43e57197dff3dac72d613640.1616052890.git.haibo.xu@linaro.org/

Signed-off-by: Haibo Xu <haibo.xu@linaro.org>
[Miguel Luis: use of ID_AA64PFR0 for cpu features]
Signed-off-by: Miguel Luis <miguel.luis@oracle.com>
---
 target/arm/cpu.h   |  2 +-
 target/arm/kvm64.c | 16 ++++++++++++++++
 2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 9aeed3c848..de2a88b43e 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -3961,7 +3961,7 @@ static inline bool isar_feature_aa64_aa32_el1(const ARMISARegisters *id)
 
 static inline bool isar_feature_aa64_aa32_el2(const ARMISARegisters *id)
 {
-    return FIELD_EX64(id->id_aa64pfr0, ID_AA64PFR0, EL2) >= 2;
+    return FIELD_EX64(id->id_aa64pfr0, ID_AA64PFR0, EL2) != 0;
 }
 
 static inline bool isar_feature_aa64_ras(const ARMISARegisters *id)
diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
index be8144a2b5..f7ebd731aa 100644
--- a/target/arm/kvm64.c
+++ b/target/arm/kvm64.c
@@ -505,6 +505,7 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
      */
     int fdarray[3];
     bool sve_supported;
+    bool el2_supported;
     bool pmu_supported = false;
     uint64_t features = 0;
     int err;
@@ -535,6 +536,14 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
         init.features[0] |= 1 << KVM_ARM_VCPU_SVE;
     }
 
+    /*
+     * Ask for EL2 if supported.
+     */
+    el2_supported = kvm_arm_el2_supported();
+    if (el2_supported) {
+        init.features[0] |= 1 << KVM_ARM_VCPU_HAS_EL2;
+    }
+
     /*
      * Ask for Pointer Authentication if supported, so that we get
      * the unsanitized field values for AA64ISAR1_EL1.
@@ -714,6 +723,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;
@@ -881,6 +894,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_isar_feature(aa64_aa32_el2, cpu)) {
+        cpu->kvm_init_features[0] |= 1 << KVM_ARM_VCPU_HAS_EL2;
+    }
     if (cpu_isar_feature(aa64_pauth, cpu)) {
         cpu->kvm_init_features[0] |= (1 << KVM_ARM_VCPU_PTRAUTH_ADDRESS |
                                       1 << KVM_ARM_VCPU_PTRAUTH_GENERIC);
-- 
2.39.2

[RFC PATCH 5/5] arm/virt: provide virtualization extensions to the guest

[Miguel Luis]

From: Haibo Xu <haibo.xu@linaro.org>

VHE enablement if host supports EL2.

Ref: https://lore.kernel.org/qemu-devel/b7c2626e6c720ccc43e57197dff3dac72d613640.1616052890.git.haibo.xu@linaro.org/

Signed-off-by: Haibo Xu <haibo.xu@linaro.org>
Signed-off-by: Miguel Luis <miguel.luis@oracle.com>
---
 hw/arm/virt.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 377181e009..7103aecf3f 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -2093,7 +2093,8 @@ static void machvirt_init(MachineState *machine)
         exit(1);
     }
 
-    if (vms->virt && (kvm_enabled() || hvf_enabled())) {
+    if (vms->virt && (kvm_enabled() || hvf_enabled())
+        && !kvm_arm_el2_supported()) {
         error_report("mach-virt: %s does not support providing "
                      "Virtualization extensions to the guest CPU",
                      kvm_enabled() ? "KVM" : "HVF");
-- 
2.39.2

Re: [RFC PATCH 1/5] linux-headers: [kvm, arm64] add the necessary definitions to match host kernel

[Cornelia Huck]

On Mon, Feb 27 2023, Miguel Luis <miguel.luis@oracle.com> wrote:

> From: Haibo Xu <haibo.xu@linaro.org>
>
> linux-headers define host properties needed for the VMM to interact with
> KVM, so let's include them *while* they're not available yet on linux
> upstream since aarch64 nested virtualization is still a work in
> progress.
>
> Ref: https://lore.kernel.org/qemu-devel/636b5932e4cf061b6f97516e82d4319c1d29b871.1616052889.git.haibo.xu@linaro.org/
>
> Signed-off-by: Haibo Xu <haibo.xu@linaro.org>
> Signed-off-by: Miguel Luis <miguel.luis@oracle.com>
> ---
>  linux-headers/asm-arm64/kvm.h | 2 ++
>  linux-headers/linux/kvm.h     | 1 +
>  2 files changed, 3 insertions(+)

Can you please mark this explicitly as a placeholder for a proper
headers update? Just so that it doesn't get lost :)

Re: [RFC PATCH 4/5] target/arm: enable feature ARM_FEATURE_EL2 if EL2 is supported

[Richard Henderson]

On 2/27/23 06:37, Miguel Luis wrote:
> From: Haibo Xu <haibo.xu@linaro.org>
> 
> KVM_CAP_ARM_EL2 must be supported by the cpu to enable ARM_FEATURE_EL2.
> EL2 bits on ID_AA64PFR0 state unsupported on the value 0b0000.
> 
> Ref: https://lore.kernel.org/qemu-devel/b7c2626e6c720ccc43e57197dff3dac72d613640.1616052890.git.haibo.xu@linaro.org/
> 
> Signed-off-by: Haibo Xu <haibo.xu@linaro.org>
> [Miguel Luis: use of ID_AA64PFR0 for cpu features]
> Signed-off-by: Miguel Luis <miguel.luis@oracle.com>
> ---
>   target/arm/cpu.h   |  2 +-
>   target/arm/kvm64.c | 16 ++++++++++++++++
>   2 files changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> index 9aeed3c848..de2a88b43e 100644
> --- a/target/arm/cpu.h
> +++ b/target/arm/cpu.h
> @@ -3961,7 +3961,7 @@ static inline bool isar_feature_aa64_aa32_el1(const ARMISARegisters *id)
>   
>   static inline bool isar_feature_aa64_aa32_el2(const ARMISARegisters *id)
>   {
> -    return FIELD_EX64(id->id_aa64pfr0, ID_AA64PFR0, EL2) >= 2;
> +    return FIELD_EX64(id->id_aa64pfr0, ID_AA64PFR0, EL2) != 0;
>   }

No, this predicate is testing if EL2 supports AArch32 more.

> @@ -714,6 +723,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;
> +    }

This is the test you want...

> @@ -881,6 +894,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_isar_feature(aa64_aa32_el2, cpu)) {
> +        cpu->kvm_init_features[0] |= 1 << KVM_ARM_VCPU_HAS_EL2;
> +    }

... here.

While you could add a new isar_feature predicate for EL2 supported in AArch64 mode, the 
feature test is equivalent and good enough, and is more obviously tied to the required KVM 
support.


r~

Re: [RFC PATCH 5/5] arm/virt: provide virtualization extensions to the guest

[Richard Henderson]

On 2/27/23 06:37, Miguel Luis wrote:
> -    if (vms->virt && (kvm_enabled() || hvf_enabled())) {
> +    if (vms->virt && (kvm_enabled() || hvf_enabled())
> +        && !kvm_arm_el2_supported()) {

The ordering of the tests isn't right -- shouldn't test kvm_arm_* for hvf.

     virt && ((kvm && !kvm_el2) || hvf)


r~

Re: [RFC PATCH 3/5] target/arm/kvm: add helper to detect EL2 when using KVM

[Richard Henderson]

On 2/27/23 06:37, Miguel Luis wrote:
> From: Haibo Xu<haibo.xu@linaro.org>
> 
> Introduce query support for KVM_CAP_ARM_EL2.
> 
> Ref:https://lore.kernel.org/qemu-devel/65b8771bfecada08bf02c9cf87c2f0f9cdf943b3.1617281290.git.haibo.xu@linaro.org/
> 
> Signed-off-by: Haibo Xu<haibo.xu@linaro.org>
> Signed-off-by: Miguel Luis<miguel.luis@oracle.com>
> ---
>   target/arm/kvm64.c   |  5 +++++
>   target/arm/kvm_arm.h | 12 ++++++++++++
>   2 files changed, 17 insertions(+)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [RFC PATCH 1/5] linux-headers: [kvm, arm64] add the necessary definitions to match host kernel

[Miguel Luis]

Hi Cornelia,

> On 27 Feb 2023, at 15:49, Cornelia Huck <cohuck@redhat.com> wrote:
> 
> On Mon, Feb 27 2023, Miguel Luis <miguel.luis@oracle.com> wrote:
> 
>> From: Haibo Xu <haibo.xu@linaro.org>
>> 
>> linux-headers define host properties needed for the VMM to interact with
>> KVM, so let's include them *while* they're not available yet on linux
>> upstream since aarch64 nested virtualization is still a work in
>> progress.
>> 
>> Ref: https://lore.kernel.org/qemu-devel/636b5932e4cf061b6f97516e82d4319c1d29b871.1616052889.git.haibo.xu@linaro.org/
>> 
>> Signed-off-by: Haibo Xu <haibo.xu@linaro.org>
>> Signed-off-by: Miguel Luis <miguel.luis@oracle.com>
>> ---
>> linux-headers/asm-arm64/kvm.h | 2 ++
>> linux-headers/linux/kvm.h     | 1 +
>> 2 files changed, 3 insertions(+)
> 
> Can you please mark this explicitly as a placeholder for a proper
> headers update? Just so that it doesn't get lost :)

Sure yes! Thank you.

Miguel

Re: [RFC PATCH 4/5] target/arm: enable feature ARM_FEATURE_EL2 if EL2 is supported

[Miguel Luis]

Hi Richard,

> On 27 Feb 2023, at 18:24, Richard Henderson <richard.henderson@linaro.org> wrote:
> 
> On 2/27/23 06:37, Miguel Luis wrote:
>> From: Haibo Xu <haibo.xu@linaro.org>
>> KVM_CAP_ARM_EL2 must be supported by the cpu to enable ARM_FEATURE_EL2.
>> EL2 bits on ID_AA64PFR0 state unsupported on the value 0b0000.
>> Ref: https://lore.kernel.org/qemu-devel/b7c2626e6c720ccc43e57197dff3dac72d613640.1616052890.git.haibo.xu@linaro.org/
>> Signed-off-by: Haibo Xu <haibo.xu@linaro.org>
>> [Miguel Luis: use of ID_AA64PFR0 for cpu features]
>> Signed-off-by: Miguel Luis <miguel.luis@oracle.com>
>> ---
>>  target/arm/cpu.h   |  2 +-
>>  target/arm/kvm64.c | 16 ++++++++++++++++
>>  2 files changed, 17 insertions(+), 1 deletion(-)
>> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
>> index 9aeed3c848..de2a88b43e 100644
>> --- a/target/arm/cpu.h
>> +++ b/target/arm/cpu.h
>> @@ -3961,7 +3961,7 @@ static inline bool isar_feature_aa64_aa32_el1(const ARMISARegisters *id)
>>    static inline bool isar_feature_aa64_aa32_el2(const ARMISARegisters *id)
>>  {
>> -    return FIELD_EX64(id->id_aa64pfr0, ID_AA64PFR0, EL2) >= 2;
>> +    return FIELD_EX64(id->id_aa64pfr0, ID_AA64PFR0, EL2) != 0;
>>  }
> 
> No, this predicate is testing if EL2 supports AArch32 more.
> 
>> @@ -714,6 +723,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;
>> +    }
> 
> This is the test you want...
> 
>> @@ -881,6 +894,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_isar_feature(aa64_aa32_el2, cpu)) {
>> +        cpu->kvm_init_features[0] |= 1 << KVM_ARM_VCPU_HAS_EL2;
>> +    }
> 
> ... here.
> 
> While you could add a new isar_feature predicate for EL2 supported in AArch64 mode, the feature test is equivalent and good enough, and is more obviously tied to the required KVM support.
> 
> 

Thank you for the explanation. I will take it into consideration on the next version.

Miguel

> r~

Re: [RFC PATCH 5/5] arm/virt: provide virtualization extensions to the guest

[Miguel Luis]

Hi Richard,

> On 27 Feb 2023, at 18:26, Richard Henderson <richard.henderson@linaro.org> wrote:
> 
> On 2/27/23 06:37, Miguel Luis wrote:
>> -    if (vms->virt && (kvm_enabled() || hvf_enabled())) {
>> +    if (vms->virt && (kvm_enabled() || hvf_enabled())
>> +        && !kvm_arm_el2_supported()) {
> 
> The ordering of the tests isn't right -- shouldn't test kvm_arm_* for hvf.
> 
>    virt && ((kvm && !kvm_el2) || hvf)
> 

Agree. It will be fixed on the next version.

Thank you!
Miguel

> 
> r~

Re: [RFC PATCH 2/5] hw/intc/gicv3: add support for setting KVM vGIC maintenance IRQ

[Peter Maydell]

On Mon, 27 Feb 2023 at 16:37, Miguel Luis <miguel.luis@oracle.com> wrote:
>
> From: Haibo Xu <haibo.xu@linaro.org>
>
> Use the VGIC maintenance IRQ if VHE is requested. As per the ARM GIC
> Architecture Specification for GICv3 and GICv4 Arm strongly recommends that
> maintenance interrupts are configured to use INTID 25 matching the
> Server Base System Architecture (SBSA) recomendation.

What does this mean for QEMU, though? If the issue is
"KVM doesn't support the maintenance interrupt being anything
other than INTID 25" then we should say so (and have our code
error out if the board tries to use some other value). If the
issue is "the *host* has to be using the right INTID" then I
would hope that KVM simply doesn't expose the capability if
the host h/w won't let it work correctly. If KVM can happily
use any maintenance interrupt ID that the board model wants,
then we should make that work, rather than hardcoding 25 into
our gicv3 code.

> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index b871350856..377181e009 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -759,6 +759,11 @@ static void create_gic(VirtMachineState *vms, MemoryRegion *mem)
>              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",
> diff --git a/hw/intc/arm_gicv3_common.c b/hw/intc/arm_gicv3_common.c
> index 351843db4a..e2a6ff1b49 100644
> --- a/hw/intc/arm_gicv3_common.c
> +++ b/hw/intc/arm_gicv3_common.c
> @@ -563,6 +563,7 @@ static Property arm_gicv3_common_properties[] = {
>      DEFINE_PROP_UINT32("revision", GICv3State, revision, 3),
>      DEFINE_PROP_BOOL("has-lpi", GICv3State, lpi_enable, 0),
>      DEFINE_PROP_BOOL("has-security-extensions", GICv3State, security_extn, 0),
> +    DEFINE_PROP_BOOL("has-virtualization-extensions", GICv3State, virt_extn, 0),
>      /*
>       * Compatibility property: force 8 bits of physical priority, even
>       * if the CPU being emulated should have fewer.
> diff --git a/hw/intc/arm_gicv3_kvm.c b/hw/intc/arm_gicv3_kvm.c
> index 3ca643ecba..ce924753bb 100644
> --- a/hw/intc/arm_gicv3_kvm.c
> +++ b/hw/intc/arm_gicv3_kvm.c
> @@ -22,6 +22,7 @@
>  #include "qemu/osdep.h"
>  #include "qapi/error.h"
>  #include "hw/intc/arm_gicv3_common.h"
> +#include "hw/arm/virt.h"
>  #include "qemu/error-report.h"
>  #include "qemu/module.h"
>  #include "sysemu/kvm.h"
> @@ -803,6 +804,30 @@ static void kvm_arm_gicv3_realize(DeviceState *dev, Error **errp)
>          return;
>      }
>
> +
> +    if (s->virt_extn) {
> +        /*
> +         * Arm strongly recommends that maintenance interrupts are configured
> +         * to use INTID 25. For more information, see Server Base System
> +         * Architecture (SBSA)
> +         */
> +        uint32_t maint_irq = PPI(ARCH_GIC_MAINT_IRQ);
> +
> +        struct kvm_device_attr kdevattr = {
> +            .group = KVM_DEV_ARM_VGIC_GRP_MAINT_IRQ,
> +            .addr = (uint64_t)&maint_irq
> +        };
> +
> +        if (!kvm_device_ioctl(s->dev_fd, KVM_GET_DEVICE_ATTR, &kdevattr)) {
> +            error_setg(errp, "VGICv3 setting maintenance IRQ are not "
> +                            "supported by this host kernel");
> +            return;
> +        }
> +
> +        kvm_device_ioctl(s->dev_fd, KVM_SET_DEVICE_ATTR, &kdevattr);
> +    }

So if I understand this correctly, the requirement here is basically
"tell the kernel which IRQ line is being used by the board code
for the maintenance interrupt", right? It seems to me that the
straightforward way to do that is to have an integer QOM property on
the GICv3 device like "maintenance-interrupt-id", and make the
board code set it (whether using KVM or not). The TCG implementation
doesn't care, and the KVM implementation can set it up in
kvm_arm_gicv3_realize(). Then you don't need to specifically tell
the GIC device that the guest is using the virtualization extensions.

thanks
-- PMM

Re: [RFC PATCH 2/5] hw/intc/gicv3: add support for setting KVM vGIC maintenance IRQ

[Marc Zyngier]

On Mon, 06 Mar 2023 14:02:33 +0000,
Peter Maydell <peter.maydell@linaro.org> wrote:
> 
> On Mon, 27 Feb 2023 at 16:37, Miguel Luis <miguel.luis@oracle.com> wrote:
> >
> > From: Haibo Xu <haibo.xu@linaro.org>
> >
> > Use the VGIC maintenance IRQ if VHE is requested. As per the ARM GIC
> > Architecture Specification for GICv3 and GICv4 Arm strongly recommends that
> > maintenance interrupts are configured to use INTID 25 matching the
> > Server Base System Architecture (SBSA) recomendation.
> 
> What does this mean for QEMU, though? If the issue is
> "KVM doesn't support the maintenance interrupt being anything
> other than INTID 25" then we should say so (and have our code
> error out if the board tries to use some other value).

No, KVM doesn't give two hoots about the INTID, as long as this is a
PPI that is otherwise unused.

> If the
> issue is "the *host* has to be using the right INTID" then I
> would hope that KVM simply doesn't expose the capability if
> the host h/w won't let it work correctly.

No host maintenance interrupt, no NV. This is specially mandatory as
the L1 guest is in (almost) complete control of the ICH_*_EL2
registers and expects MIs to be delivered.

> If KVM can happily
> use any maintenance interrupt ID that the board model wants,
> then we should make that work, rather than hardcoding 25 into
> our gicv3 code.

+1.

I'd eliminate any reference to SBSA, as it has no bearing on either
KVM nor the QEMU GIC code.

I also question the "if VHE is requested". Not having VHE doesn't
preclude virtualisation. Was that supposed to be "virtualisation
extension" instead?

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

Re: [RFC PATCH 2/5] hw/intc/gicv3: add support for setting KVM vGIC maintenance IRQ

[Miguel Luis]

Hi Peter,

> On 6 Mar 2023, at 13:02, Peter Maydell <peter.maydell@linaro.org> wrote:
> 
> On Mon, 27 Feb 2023 at 16:37, Miguel Luis <miguel.luis@oracle.com> wrote:
>> 
>> From: Haibo Xu <haibo.xu@linaro.org>
>> 
>> Use the VGIC maintenance IRQ if VHE is requested. As per the ARM GIC
>> Architecture Specification for GICv3 and GICv4 Arm strongly recommends that
>> maintenance interrupts are configured to use INTID 25 matching the
>> Server Base System Architecture (SBSA) recomendation.
> 
> What does this mean for QEMU, though? If the issue is
> "KVM doesn't support the maintenance interrupt being anything
> other than INTID 25" then we should say so (and have our code
> error out if the board tries to use some other value).

From the previous work I wondered where did the 25 would come from and I'm in
total agreement that this needs a better and meaningful commit message.

> If the
> issue is "the *host* has to be using the right INTID" then I
> would hope that KVM simply doesn't expose the capability if
> the host h/w won't let it work correctly. If KVM can happily
> use any maintenance interrupt ID that the board model wants,
> then we should make that work, rather than hardcoding 25 into
> our gicv3 code.
> 

I agree.

>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>> index b871350856..377181e009 100644
>> --- a/hw/arm/virt.c
>> +++ b/hw/arm/virt.c
>> @@ -759,6 +759,11 @@ static void create_gic(VirtMachineState *vms, MemoryRegion *mem)
>>             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",
>> diff --git a/hw/intc/arm_gicv3_common.c b/hw/intc/arm_gicv3_common.c
>> index 351843db4a..e2a6ff1b49 100644
>> --- a/hw/intc/arm_gicv3_common.c
>> +++ b/hw/intc/arm_gicv3_common.c
>> @@ -563,6 +563,7 @@ static Property arm_gicv3_common_properties[] = {
>>     DEFINE_PROP_UINT32("revision", GICv3State, revision, 3),
>>     DEFINE_PROP_BOOL("has-lpi", GICv3State, lpi_enable, 0),
>>     DEFINE_PROP_BOOL("has-security-extensions", GICv3State, security_extn, 0),
>> +    DEFINE_PROP_BOOL("has-virtualization-extensions", GICv3State, virt_extn, 0),
>>     /*
>>      * Compatibility property: force 8 bits of physical priority, even
>>      * if the CPU being emulated should have fewer.
>> diff --git a/hw/intc/arm_gicv3_kvm.c b/hw/intc/arm_gicv3_kvm.c
>> index 3ca643ecba..ce924753bb 100644
>> --- a/hw/intc/arm_gicv3_kvm.c
>> +++ b/hw/intc/arm_gicv3_kvm.c
>> @@ -22,6 +22,7 @@
>> #include "qemu/osdep.h"
>> #include "qapi/error.h"
>> #include "hw/intc/arm_gicv3_common.h"
>> +#include "hw/arm/virt.h"
>> #include "qemu/error-report.h"
>> #include "qemu/module.h"
>> #include "sysemu/kvm.h"
>> @@ -803,6 +804,30 @@ static void kvm_arm_gicv3_realize(DeviceState *dev, Error **errp)
>>         return;
>>     }
>> 
>> +
>> +    if (s->virt_extn) {
>> +        /*
>> +         * Arm strongly recommends that maintenance interrupts are configured
>> +         * to use INTID 25. For more information, see Server Base System
>> +         * Architecture (SBSA)
>> +         */
>> +        uint32_t maint_irq = PPI(ARCH_GIC_MAINT_IRQ);
>> +
>> +        struct kvm_device_attr kdevattr = {
>> +            .group = KVM_DEV_ARM_VGIC_GRP_MAINT_IRQ,
>> +            .addr = (uint64_t)&maint_irq
>> +        };
>> +
>> +        if (!kvm_device_ioctl(s->dev_fd, KVM_GET_DEVICE_ATTR, &kdevattr)) {
>> +            error_setg(errp, "VGICv3 setting maintenance IRQ are not "
>> +                            "supported by this host kernel");
>> +            return;
>> +        }
>> +
>> +        kvm_device_ioctl(s->dev_fd, KVM_SET_DEVICE_ATTR, &kdevattr);
>> +    }
> 
> So if I understand this correctly, the requirement here is basically
> "tell the kernel which IRQ line is being used by the board code
> for the maintenance interrupt", right?

From the previous statement I understood it would be better to consider the
board code. So, yes.

> It seems to me that the
> straightforward way to do that is to have an integer QOM property on
> the GICv3 device like "maintenance-interrupt-id", and make the
> board code set it (whether using KVM or not).

Thanks, I’ll look into it.

> The TCG implementation
> doesn't care, and the KVM implementation can set it up in
> kvm_arm_gicv3_realize(). Then you don't need to specifically tell
> the GIC device that the guest is using the virtualization extensions.
> 

Yes, that seems better suited.

Thank you,
Miguel

> thanks
> -- PMM

Re: [RFC PATCH 2/5] hw/intc/gicv3: add support for setting KVM vGIC maintenance IRQ

[Miguel Luis]

Hi Marc,

> On 6 Mar 2023, at 13:32, Marc Zyngier <maz@kernel.org> wrote:
> 
> On Mon, 06 Mar 2023 14:02:33 +0000,
> Peter Maydell <peter.maydell@linaro.org> wrote:
>> 
>> On Mon, 27 Feb 2023 at 16:37, Miguel Luis <miguel.luis@oracle.com> wrote:
>>> 
>>> From: Haibo Xu <haibo.xu@linaro.org>
>>> 
>>> Use the VGIC maintenance IRQ if VHE is requested. As per the ARM GIC
>>> Architecture Specification for GICv3 and GICv4 Arm strongly recommends that
>>> maintenance interrupts are configured to use INTID 25 matching the
>>> Server Base System Architecture (SBSA) recomendation.
>> 
>> What does this mean for QEMU, though? If the issue is
>> "KVM doesn't support the maintenance interrupt being anything
>> other than INTID 25" then we should say so (and have our code
>> error out if the board tries to use some other value).
> 
> No, KVM doesn't give two hoots about the INTID, as long as this is a
> PPI that is otherwise unused.
> 
>> If the
>> issue is "the *host* has to be using the right INTID" then I
>> would hope that KVM simply doesn't expose the capability if
>> the host h/w won't let it work correctly.
> 
> No host maintenance interrupt, no NV. This is specially mandatory as
> the L1 guest is in (almost) complete control of the ICH_*_EL2
> registers and expects MIs to be delivered.
> 
>> If KVM can happily
>> use any maintenance interrupt ID that the board model wants,
>> then we should make that work, rather than hardcoding 25 into
>> our gicv3 code.
> 
> +1.
> 
> I'd eliminate any reference to SBSA, as it has no bearing on either
> KVM nor the QEMU GIC code.
> 
> I also question the "if VHE is requested". Not having VHE doesn't
> preclude virtualisation. Was that supposed to be "virtualisation
> extension" instead?
> 

s/VHE/virtualization extension/

I’ve noted it has been a recurring confusion on my part. Will fix. :)

Thank you,
Miguel

> Thanks,
> 
> M.
> 
> -- 
> Without deviation from the norm, progress is not possible.

Re: [RFC PATCH 4/5] target/arm: enable feature ARM_FEATURE_EL2 if EL2 is supported

[Eric Auger]

Hi Miguel,

On 2/27/23 17:37, Miguel Luis wrote:
> From: Haibo Xu <haibo.xu@linaro.org>
> 
> KVM_CAP_ARM_EL2 must be supported by the cpu to enable ARM_FEATURE_EL2.
> EL2 bits on ID_AA64PFR0 state unsupported on the value 0b0000.
> 
> Ref: https://lore.kernel.org/qemu-devel/b7c2626e6c720ccc43e57197dff3dac72d613640.1616052890.git.haibo.xu@linaro.org/
> 
> Signed-off-by: Haibo Xu <haibo.xu@linaro.org>
> [Miguel Luis: use of ID_AA64PFR0 for cpu features]
> Signed-off-by: Miguel Luis <miguel.luis@oracle.com>
> ---
>  target/arm/cpu.h   |  2 +-
>  target/arm/kvm64.c | 16 ++++++++++++++++
>  2 files changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> index 9aeed3c848..de2a88b43e 100644
> --- a/target/arm/cpu.h
> +++ b/target/arm/cpu.h
> @@ -3961,7 +3961,7 @@ static inline bool isar_feature_aa64_aa32_el1(const ARMISARegisters *id)
>  
>  static inline bool isar_feature_aa64_aa32_el2(const ARMISARegisters *id)
>  {
> -    return FIELD_EX64(id->id_aa64pfr0, ID_AA64PFR0, EL2) >= 2;
> +    return FIELD_EX64(id->id_aa64pfr0, ID_AA64PFR0, EL2) != 0;
>  }
>  
>  static inline bool isar_feature_aa64_ras(const ARMISARegisters *id)
> diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
> index be8144a2b5..f7ebd731aa 100644
> --- a/target/arm/kvm64.c
> +++ b/target/arm/kvm64.c
> @@ -505,6 +505,7 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
>       */
>      int fdarray[3];
>      bool sve_supported;
> +    bool el2_supported;
>      bool pmu_supported = false;
>      uint64_t features = 0;
>      int err;
> @@ -535,6 +536,14 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
>          init.features[0] |= 1 << KVM_ARM_VCPU_SVE;
>      }
>  
> +    /*
> +     * Ask for EL2 if supported.
> +     */
> +    el2_supported = kvm_arm_el2_supported();
> +    if (el2_supported) {
> +        init.features[0] |= 1 << KVM_ARM_VCPU_HAS_EL2;
This doesn't work if your host both supports SVE & NV.

The error output by qemu is not straightforward

qemu-system-aarch64: can't apply global host-arm-cpu.sve=off: Property
'host-arm-cpu.sve' not found

The problem is that we create a scratch VM with a CPU featuring both SVE
and NV and this fails at kernel level, I think on vcpu reset.

The trouble is that we do that even if sve=off at qemu level. So I think
this is a more generic issue related to the way we validate host cpu
features.

Thanks

Eric


> +    }
> +
>      /*
>       * Ask for Pointer Authentication if supported, so that we get
>       * the unsanitized field values for AA64ISAR1_EL1.
> @@ -714,6 +723,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;
> @@ -881,6 +894,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_isar_feature(aa64_aa32_el2, cpu)) {
> +        cpu->kvm_init_features[0] |= 1 << KVM_ARM_VCPU_HAS_EL2;
> +    }
>      if (cpu_isar_feature(aa64_pauth, cpu)) {
>          cpu->kvm_init_features[0] |= (1 << KVM_ARM_VCPU_PTRAUTH_ADDRESS |
>                                        1 << KVM_ARM_VCPU_PTRAUTH_GENERIC);

Re: [RFC PATCH 4/5] target/arm: enable feature ARM_FEATURE_EL2 if EL2 is supported

[Miguel Luis]

Hi Eric,

Thanks in advance for your comment.

> On 6 Jul 2023, at 08:16, Eric Auger <eauger@redhat.com> wrote:
> 
> Hi Miguel,
> 
> On 2/27/23 17:37, Miguel Luis wrote:
>> From: Haibo Xu <haibo.xu@linaro.org>
>> 
>> KVM_CAP_ARM_EL2 must be supported by the cpu to enable ARM_FEATURE_EL2.
>> EL2 bits on ID_AA64PFR0 state unsupported on the value 0b0000.
>> 
>> Ref: https://lore.kernel.org/qemu-devel/b7c2626e6c720ccc43e57197dff3dac72d613640.1616052890.git.haibo.xu@linaro.org/
>> 
>> Signed-off-by: Haibo Xu <haibo.xu@linaro.org>
>> [Miguel Luis: use of ID_AA64PFR0 for cpu features]
>> Signed-off-by: Miguel Luis <miguel.luis@oracle.com>
>> ---
>> target/arm/cpu.h   |  2 +-
>> target/arm/kvm64.c | 16 ++++++++++++++++
>> 2 files changed, 17 insertions(+), 1 deletion(-)
>> 
>> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
>> index 9aeed3c848..de2a88b43e 100644
>> --- a/target/arm/cpu.h
>> +++ b/target/arm/cpu.h
>> @@ -3961,7 +3961,7 @@ static inline bool isar_feature_aa64_aa32_el1(const ARMISARegisters *id)
>> 
>> static inline bool isar_feature_aa64_aa32_el2(const ARMISARegisters *id)
>> {
>> -    return FIELD_EX64(id->id_aa64pfr0, ID_AA64PFR0, EL2) >= 2;
>> +    return FIELD_EX64(id->id_aa64pfr0, ID_AA64PFR0, EL2) != 0;
>> }
>> 
>> static inline bool isar_feature_aa64_ras(const ARMISARegisters *id)
>> diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
>> index be8144a2b5..f7ebd731aa 100644
>> --- a/target/arm/kvm64.c
>> +++ b/target/arm/kvm64.c
>> @@ -505,6 +505,7 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
>>      */
>>     int fdarray[3];
>>     bool sve_supported;
>> +    bool el2_supported;
>>     bool pmu_supported = false;
>>     uint64_t features = 0;
>>     int err;
>> @@ -535,6 +536,14 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
>>         init.features[0] |= 1 << KVM_ARM_VCPU_SVE;
>>     }
>> 
>> +    /*
>> +     * Ask for EL2 if supported.
>> +     */
>> +    el2_supported = kvm_arm_el2_supported();
>> +    if (el2_supported) {
>> +        init.features[0] |= 1 << KVM_ARM_VCPU_HAS_EL2;
> This doesn't work if your host both supports SVE & NV.
> 

May I ask if this prevented your L1 or L2 guest from booting? I’ve addressed all
the previous comments on the thread for the new RFC version and this topic is
what I’m currently addressing.

So far the L1 guest booted successfully but not the L2 guest.

> The error output by qemu is not straightforward
> 
> qemu-system-aarch64: can't apply global host-arm-cpu.sve=off: Property
> 'host-arm-cpu.sve' not found
> 
> The problem is that we create a scratch VM with a CPU featuring both SVE
> and NV and this fails at kernel level, I think on vcpu reset.
> 
> The trouble is that we do that even if sve=off at qemu level. So I think
> this is a more generic issue related to the way we validate host cpu
> features.
> 

OK, I’m having a look into that.

Thank you

Miguel

> Thanks
> 
> Eric
> 
> 
>> +    }
>> +
>>     /*
>>      * Ask for Pointer Authentication if supported, so that we get
>>      * the unsanitized field values for AA64ISAR1_EL1.
>> @@ -714,6 +723,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;
>> @@ -881,6 +894,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_isar_feature(aa64_aa32_el2, cpu)) {
>> +        cpu->kvm_init_features[0] |= 1 << KVM_ARM_VCPU_HAS_EL2;
>> +    }
>>     if (cpu_isar_feature(aa64_pauth, cpu)) {
>>         cpu->kvm_init_features[0] |= (1 << KVM_ARM_VCPU_PTRAUTH_ADDRESS |
>>                                       1 << KVM_ARM_VCPU_PTRAUTH_GENERIC);

Re: [RFC PATCH 0/5] QEMU v7.2.0 aarch64 Nested Virtualization Support

[Eric Auger]

Hi Miguel,

On 2/27/23 17:37, Miguel Luis wrote:
> This series adds ARMv8.3/8.4 nested virtualization support in KVM mode.
>     
> To enable nested virtualization for a guest, the host must expose EL2
> support via QEMU command line switches:
> 
> -machine virt,accel=kvm,virtualization=on
> 
> Inspired on Haibo Xu's previous work [0][1], Marc Zyngier's kvmtool branch [2]
> and kernel patches [3] on nested virtualization for aarch64, this has been
> tested on an Ampere implementation.
> 
> This series adapts previous work on top of v7.2.0, it considers comments given
> at the time and preserves authorship of the original patches.
> 
> [0]: https://lore.kernel.org/qemu-devel/cover.1616052889.git.haibo.xu@linaro.org/
> [1]: https://lore.kernel.org/qemu-devel/cover.1617281290.git.haibo.xu@linaro.org/
> [2]: https://git.kernel.org/pub/scm/linux/kernel/git/maz/kvmtool.git/log/?h=arm64/nv-5.16
> [3]: https://lore.kernel.org/linux-arm-kernel/20230131092504.2880505-1-maz@kernel.org/

I rebased the series on top of v8.2. I was able to boot some L2 guests
with it, although it still does not work with guests featuring edk2.

Do you plan to send a respin or may I do?

Thanks

Eric
> 
> Miguel Luis (5):
>   linux-headers: [kvm,arm64] add the necessary definitions to match host
>     kernel
>   hw/intc/gicv3: add support for setting KVM vGIC maintenance IRQ
>   target/arm/kvm: add helper to detect EL2 when using KVM
>   target/arm: enable feature ARM_FEATURE_EL2 if EL2 is supported
>   arm/virt: provide virtualization extensions to the guest
> 
>  hw/arm/virt.c                      |  8 +++++++-
>  hw/intc/arm_gicv3_common.c         |  1 +
>  hw/intc/arm_gicv3_kvm.c            | 25 +++++++++++++++++++++++++
>  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                   |  2 +-
>  target/arm/kvm64.c                 | 21 +++++++++++++++++++++
>  target/arm/kvm_arm.h               | 12 ++++++++++++
>  9 files changed, 71 insertions(+), 2 deletions(-)
> 

Re: [RFC PATCH 0/5] QEMU v7.2.0 aarch64 Nested Virtualization Support

[Miguel Luis]

Hi Eric,

> On 8 Feb 2024, at 15:55, Eric Auger <eauger@redhat.com> wrote:
> 
> Hi Miguel,
> 
> On 2/27/23 17:37, Miguel Luis wrote:
>> This series adds ARMv8.3/8.4 nested virtualization support in KVM mode.
>> 
>> To enable nested virtualization for a guest, the host must expose EL2
>> support via QEMU command line switches:
>> 
>> -machine virt,accel=kvm,virtualization=on
>> 
>> Inspired on Haibo Xu's previous work [0][1], Marc Zyngier's kvmtool branch [2]
>> and kernel patches [3] on nested virtualization for aarch64, this has been
>> tested on an Ampere implementation.
>> 
>> This series adapts previous work on top of v7.2.0, it considers comments given
>> at the time and preserves authorship of the original patches.
>> 
>> [0]: https://lore.kernel.org/qemu-devel/cover.1616052889.git.haibo.xu@linaro.org/
>> [1]: https://lore.kernel.org/qemu-devel/cover.1617281290.git.haibo.xu@linaro.org/
>> [2]: https://git.kernel.org/pub/scm/linux/kernel/git/maz/kvmtool.git/log/?h=arm64/nv-5.16
>> [3]: https://lore.kernel.org/linux-arm-kernel/20230131092504.2880505-1-maz@kernel.org/
> 
> I rebased the series on top of v8.2. I was able to boot some L2 guests
> with it, although it still does not work with guests featuring edk2.
> 
> Do you plan to send a respin or may I do?
> 

I do not have a short-term respin planned.
Please, feel free to do.

Thanks

Miguel

> Thanks
> 
> Eric
>> 
>> Miguel Luis (5):
>>  linux-headers: [kvm,arm64] add the necessary definitions to match host
>>    kernel
>>  hw/intc/gicv3: add support for setting KVM vGIC maintenance IRQ
>>  target/arm/kvm: add helper to detect EL2 when using KVM
>>  target/arm: enable feature ARM_FEATURE_EL2 if EL2 is supported
>>  arm/virt: provide virtualization extensions to the guest
>> 
>> hw/arm/virt.c                      |  8 +++++++-
>> hw/intc/arm_gicv3_common.c         |  1 +
>> hw/intc/arm_gicv3_kvm.c            | 25 +++++++++++++++++++++++++
>> 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                   |  2 +-
>> target/arm/kvm64.c                 | 21 +++++++++++++++++++++
>> target/arm/kvm_arm.h               | 12 ++++++++++++
>> 9 files changed, 71 insertions(+), 2 deletions(-)

Re: [RFC PATCH 0/5] QEMU v7.2.0 aarch64 Nested Virtualization Support

[Eric Auger]

Hi Miguel,

On 2/8/24 18:33, Miguel Luis wrote:
> Hi Eric,
> 
>> On 8 Feb 2024, at 15:55, Eric Auger <eauger@redhat.com> wrote:
>>
>> Hi Miguel,
>>
>> On 2/27/23 17:37, Miguel Luis wrote:
>>> This series adds ARMv8.3/8.4 nested virtualization support in KVM mode.
>>>
>>> To enable nested virtualization for a guest, the host must expose EL2
>>> support via QEMU command line switches:
>>>
>>> -machine virt,accel=kvm,virtualization=on
>>>
>>> Inspired on Haibo Xu's previous work [0][1], Marc Zyngier's kvmtool branch [2]
>>> and kernel patches [3] on nested virtualization for aarch64, this has been
>>> tested on an Ampere implementation.
>>>
>>> This series adapts previous work on top of v7.2.0, it considers comments given
>>> at the time and preserves authorship of the original patches.
>>>
>>> [0]: https://lore.kernel.org/qemu-devel/cover.1616052889.git.haibo.xu@linaro.org/
>>> [1]: https://lore.kernel.org/qemu-devel/cover.1617281290.git.haibo.xu@linaro.org/
>>> [2]: https://git.kernel.org/pub/scm/linux/kernel/git/maz/kvmtool.git/log/?h=arm64/nv-5.16
>>> [3]: https://lore.kernel.org/linux-arm-kernel/20230131092504.2880505-1-maz@kernel.org/
>>
>> I rebased the series on top of v8.2. I was able to boot some L2 guests
>> with it, although it still does not work with guests featuring edk2.
>>
>> Do you plan to send a respin or may I do?
>>
> 
> I do not have a short-term respin planned.
> Please, feel free to do.

OK thanks. I will send it tomorrow.

Eric
> 
> Thanks
> 
> Miguel
> 
>> Thanks
>>
>> Eric
>>>
>>> Miguel Luis (5):
>>>  linux-headers: [kvm,arm64] add the necessary definitions to match host
>>>    kernel
>>>  hw/intc/gicv3: add support for setting KVM vGIC maintenance IRQ
>>>  target/arm/kvm: add helper to detect EL2 when using KVM
>>>  target/arm: enable feature ARM_FEATURE_EL2 if EL2 is supported
>>>  arm/virt: provide virtualization extensions to the guest
>>>
>>> hw/arm/virt.c                      |  8 +++++++-
>>> hw/intc/arm_gicv3_common.c         |  1 +
>>> hw/intc/arm_gicv3_kvm.c            | 25 +++++++++++++++++++++++++
>>> 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                   |  2 +-
>>> target/arm/kvm64.c                 | 21 +++++++++++++++++++++
>>> target/arm/kvm_arm.h               | 12 ++++++++++++
>>> 9 files changed, 71 insertions(+), 2 deletions(-)
> 
>