[PATCH v1 0/4] KVM: arm64: Fix initialization of trap register values in pKVM

[PATCH v1 0/4] KVM: arm64: Fix initialization of trap register values in pKVM

[Fuad Tabba]

The patch that dropped PAuth instruction/key traps [1] did not
apply to non-protected VMs in protected mode, since as noted [2],
the hypervisor is supposed to handle trap register values in
pKVM. However, in pKVM upstream code the hypervisor does not set
all necessary trap register values in protected mode, and running
non-protected VMs with PAuth enabled in pKVM has been broken
since then [1].

This patch series lets the hypervisor initialize the values of
trap registers for both non-protected and protected VMs,
including setting PAuth traps depending on whether PAuth is
available in the system and configured for the target VCPU.

Based on Linux 6.12-rc3 (8e929cb546ee).

Cheers,
/fuad

[1] Commit 814ad8f96e92 ("KVM: arm64: Drop trapping of PAuth instructions/keys")
[2] Commit 7e814a20f6da ("KVM: arm64: Tidying up PAuth code in KVM")

Fuad Tabba (4):
  KVM: arm64: Move pkvm_vcpu_init_traps() to init_pkvm_hyp_vcpu()
  KVM: arm64: Refactor kvm_vcpu_enable_ptrauth() for hyp use
  KVM: arm64: Initialize the hypervisor's VM state at EL2
  KVM: arm64: Initialize trap register values in hyp in pKVM

 arch/arm64/include/asm/kvm_asm.h              |   1 -
 arch/arm64/include/asm/kvm_emulate.h          |   4 +
 arch/arm64/kvm/arm.c                          |   8 --
 .../arm64/kvm/hyp/include/nvhe/trap_handler.h |   2 -
 arch/arm64/kvm/hyp/nvhe/hyp-main.c            |  12 +-
 arch/arm64/kvm/hyp/nvhe/pkvm.c                | 116 +++++++++++++++++-
 arch/arm64/kvm/reset.c                        |   5 -
 7 files changed, 122 insertions(+), 26 deletions(-)


base-commit: 8e929cb546ee42c9a61d24fae60605e9e3192354
-- 
2.47.0.rc1.288.g06298d1525-goog

[PATCH v1 1/4] KVM: arm64: Move pkvm_vcpu_init_traps() to init_pkvm_hyp_vcpu()

[Fuad Tabba]

Move pkvm_vcpu_init_traps() to the initialization of the
hypervisor's vcpu state in init_pkvm_hyp_vcpu(), and remove the
associated hypercall.

In protected mode, traps need to be initialized whenever a VCPU
is initialized anyway, and not only for protected VMs. This also
saves an unnecessary hypercall.

Signed-off-by: Fuad Tabba <tabba@google.com>
---
 arch/arm64/include/asm/kvm_asm.h               | 1 -
 arch/arm64/kvm/arm.c                           | 8 --------
 arch/arm64/kvm/hyp/include/nvhe/trap_handler.h | 2 --
 arch/arm64/kvm/hyp/nvhe/hyp-main.c             | 8 --------
 arch/arm64/kvm/hyp/nvhe/pkvm.c                 | 4 +++-
 5 files changed, 3 insertions(+), 20 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_asm.h b/arch/arm64/include/asm/kvm_asm.h
index b36a3b6cc011..7f7866d2bfc2 100644
--- a/arch/arm64/include/asm/kvm_asm.h
+++ b/arch/arm64/include/asm/kvm_asm.h
@@ -76,7 +76,6 @@ enum __kvm_host_smccc_func {
 	__KVM_HOST_SMCCC_FUNC___kvm_timer_set_cntvoff,
 	__KVM_HOST_SMCCC_FUNC___vgic_v3_save_vmcr_aprs,
 	__KVM_HOST_SMCCC_FUNC___vgic_v3_restore_vmcr_aprs,
-	__KVM_HOST_SMCCC_FUNC___pkvm_vcpu_init_traps,
 	__KVM_HOST_SMCCC_FUNC___pkvm_init_vm,
 	__KVM_HOST_SMCCC_FUNC___pkvm_init_vcpu,
 	__KVM_HOST_SMCCC_FUNC___pkvm_teardown_vm,
diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index a0d01c46e408..ece934bb4531 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -856,14 +856,6 @@ int kvm_arch_vcpu_run_pid_change(struct kvm_vcpu *vcpu)
 		static_branch_inc(&userspace_irqchip_in_use);
 	}
 
-	/*
-	 * Initialize traps for protected VMs.
-	 * NOTE: Move to run in EL2 directly, rather than via a hypercall, once
-	 * the code is in place for first run initialization at EL2.
-	 */
-	if (kvm_vm_is_protected(kvm))
-		kvm_call_hyp_nvhe(__pkvm_vcpu_init_traps, vcpu);
-
 	mutex_lock(&kvm->arch.config_lock);
 	set_bit(KVM_ARCH_FLAG_HAS_RAN_ONCE, &kvm->arch.flags);
 	mutex_unlock(&kvm->arch.config_lock);
diff --git a/arch/arm64/kvm/hyp/include/nvhe/trap_handler.h b/arch/arm64/kvm/hyp/include/nvhe/trap_handler.h
index 45a84f0ade04..1e6d995968a1 100644
--- a/arch/arm64/kvm/hyp/include/nvhe/trap_handler.h
+++ b/arch/arm64/kvm/hyp/include/nvhe/trap_handler.h
@@ -15,6 +15,4 @@
 #define DECLARE_REG(type, name, ctxt, reg)	\
 				type name = (type)cpu_reg(ctxt, (reg))
 
-void __pkvm_vcpu_init_traps(struct kvm_vcpu *vcpu);
-
 #endif /* __ARM64_KVM_NVHE_TRAP_HANDLER_H__ */
diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-main.c b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
index fefc89209f9e..1a224d5df207 100644
--- a/arch/arm64/kvm/hyp/nvhe/hyp-main.c
+++ b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
@@ -349,13 +349,6 @@ static void handle___pkvm_prot_finalize(struct kvm_cpu_context *host_ctxt)
 	cpu_reg(host_ctxt, 1) = __pkvm_prot_finalize();
 }
 
-static void handle___pkvm_vcpu_init_traps(struct kvm_cpu_context *host_ctxt)
-{
-	DECLARE_REG(struct kvm_vcpu *, vcpu, host_ctxt, 1);
-
-	__pkvm_vcpu_init_traps(kern_hyp_va(vcpu));
-}
-
 static void handle___pkvm_init_vm(struct kvm_cpu_context *host_ctxt)
 {
 	DECLARE_REG(struct kvm *, host_kvm, host_ctxt, 1);
@@ -411,7 +404,6 @@ static const hcall_t host_hcall[] = {
 	HANDLE_FUNC(__kvm_timer_set_cntvoff),
 	HANDLE_FUNC(__vgic_v3_save_vmcr_aprs),
 	HANDLE_FUNC(__vgic_v3_restore_vmcr_aprs),
-	HANDLE_FUNC(__pkvm_vcpu_init_traps),
 	HANDLE_FUNC(__pkvm_init_vm),
 	HANDLE_FUNC(__pkvm_init_vcpu),
 	HANDLE_FUNC(__pkvm_teardown_vm),
diff --git a/arch/arm64/kvm/hyp/nvhe/pkvm.c b/arch/arm64/kvm/hyp/nvhe/pkvm.c
index 077d4098548d..869955e551a0 100644
--- a/arch/arm64/kvm/hyp/nvhe/pkvm.c
+++ b/arch/arm64/kvm/hyp/nvhe/pkvm.c
@@ -204,7 +204,7 @@ static void pvm_init_trap_regs(struct kvm_vcpu *vcpu)
 /*
  * Initialize trap register values in protected mode.
  */
-void __pkvm_vcpu_init_traps(struct kvm_vcpu *vcpu)
+static void pkvm_vcpu_init_traps(struct kvm_vcpu *vcpu)
 {
 	pvm_init_trap_regs(vcpu);
 	pvm_init_traps_aa64pfr0(vcpu);
@@ -335,6 +335,8 @@ static int init_pkvm_hyp_vcpu(struct pkvm_hyp_vcpu *hyp_vcpu,
 
 	hyp_vcpu->vcpu.arch.hw_mmu = &hyp_vm->kvm.arch.mmu;
 	hyp_vcpu->vcpu.arch.cflags = READ_ONCE(host_vcpu->arch.cflags);
+
+	pkvm_vcpu_init_traps(&hyp_vcpu->vcpu);
 done:
 	if (ret)
 		unpin_host_vcpu(host_vcpu);
-- 
2.47.0.rc1.288.g06298d1525-goog

Re: [PATCH v1 1/4] KVM: arm64: Move pkvm_vcpu_init_traps() to init_pkvm_hyp_vcpu()

[Oliver Upton]

On Mon, Oct 14, 2024 at 11:24:09AM +0100, Fuad Tabba wrote:
> Move pkvm_vcpu_init_traps() to the initialization of the
> hypervisor's vcpu state in init_pkvm_hyp_vcpu(), and remove the
> associated hypercall.
> 
> In protected mode, traps need to be initialized whenever a VCPU
> is initialized anyway, and not only for protected VMs. This also
> saves an unnecessary hypercall.
> 
> Signed-off-by: Fuad Tabba <tabba@google.com>
> ---
>  arch/arm64/include/asm/kvm_asm.h               | 1 -
>  arch/arm64/kvm/arm.c                           | 8 --------
>  arch/arm64/kvm/hyp/include/nvhe/trap_handler.h | 2 --
>  arch/arm64/kvm/hyp/nvhe/hyp-main.c             | 8 --------
>  arch/arm64/kvm/hyp/nvhe/pkvm.c                 | 4 +++-
>  5 files changed, 3 insertions(+), 20 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/kvm_asm.h b/arch/arm64/include/asm/kvm_asm.h
> index b36a3b6cc011..7f7866d2bfc2 100644
> --- a/arch/arm64/include/asm/kvm_asm.h
> +++ b/arch/arm64/include/asm/kvm_asm.h
> @@ -76,7 +76,6 @@ enum __kvm_host_smccc_func {
>  	__KVM_HOST_SMCCC_FUNC___kvm_timer_set_cntvoff,
>  	__KVM_HOST_SMCCC_FUNC___vgic_v3_save_vmcr_aprs,
>  	__KVM_HOST_SMCCC_FUNC___vgic_v3_restore_vmcr_aprs,
> -	__KVM_HOST_SMCCC_FUNC___pkvm_vcpu_init_traps,
>  	__KVM_HOST_SMCCC_FUNC___pkvm_init_vm,
>  	__KVM_HOST_SMCCC_FUNC___pkvm_init_vcpu,
>  	__KVM_HOST_SMCCC_FUNC___pkvm_teardown_vm,
> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> index a0d01c46e408..ece934bb4531 100644
> --- a/arch/arm64/kvm/arm.c
> +++ b/arch/arm64/kvm/arm.c
> @@ -856,14 +856,6 @@ int kvm_arch_vcpu_run_pid_change(struct kvm_vcpu *vcpu)
>  		static_branch_inc(&userspace_irqchip_in_use);
>  	}
>  
> -	/*
> -	 * Initialize traps for protected VMs.
> -	 * NOTE: Move to run in EL2 directly, rather than via a hypercall, once
> -	 * the code is in place for first run initialization at EL2.
> -	 */
> -	if (kvm_vm_is_protected(kvm))
> -		kvm_call_hyp_nvhe(__pkvm_vcpu_init_traps, vcpu);
> -

Can we instead get rid of this hypercall by implementing the one-time
late initialization as the comment suggests?

It'd be great if the feature configuration for non-protected and
protected VMs is done in a similar way, and that is userspace changes to
the ID registers before KVM_RUN.

So we need this sort of 'late' initialization for that behavior to work.

-- 
Thanks,
Oliver

Re: [PATCH v1 1/4] KVM: arm64: Move pkvm_vcpu_init_traps() to init_pkvm_hyp_vcpu()

[Fuad Tabba]

Hi Oliver,

On Tue, 15 Oct 2024 at 08:37, Oliver Upton <oliver.upton@linux.dev> wrote:
>
> On Mon, Oct 14, 2024 at 11:24:09AM +0100, Fuad Tabba wrote:
> > Move pkvm_vcpu_init_traps() to the initialization of the
> > hypervisor's vcpu state in init_pkvm_hyp_vcpu(), and remove the
> > associated hypercall.
> >
> > In protected mode, traps need to be initialized whenever a VCPU
> > is initialized anyway, and not only for protected VMs. This also
> > saves an unnecessary hypercall.
> >
> > Signed-off-by: Fuad Tabba <tabba@google.com>
> > ---
> >  arch/arm64/include/asm/kvm_asm.h               | 1 -
> >  arch/arm64/kvm/arm.c                           | 8 --------
> >  arch/arm64/kvm/hyp/include/nvhe/trap_handler.h | 2 --
> >  arch/arm64/kvm/hyp/nvhe/hyp-main.c             | 8 --------
> >  arch/arm64/kvm/hyp/nvhe/pkvm.c                 | 4 +++-
> >  5 files changed, 3 insertions(+), 20 deletions(-)
> >
> > diff --git a/arch/arm64/include/asm/kvm_asm.h b/arch/arm64/include/asm/kvm_asm.h
> > index b36a3b6cc011..7f7866d2bfc2 100644
> > --- a/arch/arm64/include/asm/kvm_asm.h
> > +++ b/arch/arm64/include/asm/kvm_asm.h
> > @@ -76,7 +76,6 @@ enum __kvm_host_smccc_func {
> >       __KVM_HOST_SMCCC_FUNC___kvm_timer_set_cntvoff,
> >       __KVM_HOST_SMCCC_FUNC___vgic_v3_save_vmcr_aprs,
> >       __KVM_HOST_SMCCC_FUNC___vgic_v3_restore_vmcr_aprs,
> > -     __KVM_HOST_SMCCC_FUNC___pkvm_vcpu_init_traps,
> >       __KVM_HOST_SMCCC_FUNC___pkvm_init_vm,
> >       __KVM_HOST_SMCCC_FUNC___pkvm_init_vcpu,
> >       __KVM_HOST_SMCCC_FUNC___pkvm_teardown_vm,
> > diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> > index a0d01c46e408..ece934bb4531 100644
> > --- a/arch/arm64/kvm/arm.c
> > +++ b/arch/arm64/kvm/arm.c
> > @@ -856,14 +856,6 @@ int kvm_arch_vcpu_run_pid_change(struct kvm_vcpu *vcpu)
> >               static_branch_inc(&userspace_irqchip_in_use);
> >       }
> >
> > -     /*
> > -      * Initialize traps for protected VMs.
> > -      * NOTE: Move to run in EL2 directly, rather than via a hypercall, once
> > -      * the code is in place for first run initialization at EL2.
> > -      */
> > -     if (kvm_vm_is_protected(kvm))
> > -             kvm_call_hyp_nvhe(__pkvm_vcpu_init_traps, vcpu);
> > -
>
> Can we instead get rid of this hypercall by implementing the one-time
> late initialization as the comment suggests?

I'm not sure I understand. I thought this is what I'm doing in this patch...

> It'd be great if the feature configuration for non-protected and
> protected VMs is done in a similar way, and that is userspace changes to
> the ID registers before KVM_RUN.
>
> So we need this sort of 'late' initialization for that behavior to work.

I think this is related to me now quite understanding the first
comment, and by extension not sure I quite get this one.

Sorry about that :)
/fuad

>
> --
> Thanks,
> Oliver

Re: [PATCH v1 1/4] KVM: arm64: Move pkvm_vcpu_init_traps() to init_pkvm_hyp_vcpu()

[Oliver Upton]

On Tue, Oct 15, 2024 at 09:22:47AM +0100, Fuad Tabba wrote:
> Hi Oliver,
> 
> On Tue, 15 Oct 2024 at 08:37, Oliver Upton <oliver.upton@linux.dev> wrote:
> > Can we instead get rid of this hypercall by implementing the one-time
> > late initialization as the comment suggests?
> 
> I'm not sure I understand. I thought this is what I'm doing in this patch...

You're more than welcome to say when I'm being a damn idiot, such as
right now :)

I've been staring at too much initialization code and forgot that the
hypervisor's view of the VM is created late and not at 'vCPU
initialization' i.e. KVM_ARM_VCPU_INIT.

> > It'd be great if the feature configuration for non-protected and
> > protected VMs is done in a similar way, and that is userspace changes to
> > the ID registers before KVM_RUN.
> >
> > So we need this sort of 'late' initialization for that behavior to work.
> 
> I think this is related to me now quite understanding the first
> comment, and by extension not sure I quite get this one.

Sorry for the noise.

-- 
Thanks,
Oliver

Re: [PATCH v1 1/4] KVM: arm64: Move pkvm_vcpu_init_traps() to init_pkvm_hyp_vcpu()

[Fuad Tabba]

On Tue, 15 Oct 2024 at 09:30, Oliver Upton <oliver.upton@linux.dev> wrote:
>
> On Tue, Oct 15, 2024 at 09:22:47AM +0100, Fuad Tabba wrote:
> > Hi Oliver,
> >
> > On Tue, 15 Oct 2024 at 08:37, Oliver Upton <oliver.upton@linux.dev> wrote:
> > > Can we instead get rid of this hypercall by implementing the one-time
> > > late initialization as the comment suggests?
> >
> > I'm not sure I understand. I thought this is what I'm doing in this patch...
>
> You're more than welcome to say when I'm being a damn idiot, such as
> right now :)
>
> I've been staring at too much initialization code and forgot that the
> hypervisor's view of the VM is created late and not at 'vCPU
> initialization' i.e. KVM_ARM_VCPU_INIT.

No worries, just making sure we're on the same page :)

I'll give it a day to see if there are other comments, and respin this
based on yours (and any others I might get).

Thanks!
/fuad

>
> > > It'd be great if the feature configuration for non-protected and
> > > protected VMs is done in a similar way, and that is userspace changes to
> > > the ID registers before KVM_RUN.
> > >
> > > So we need this sort of 'late' initialization for that behavior to work.
> >
> > I think this is related to me now quite understanding the first
> > comment, and by extension not sure I quite get this one.
>
> Sorry for the noise.
>
> --
> Thanks,
> Oliver

[PATCH v1 2/4] KVM: arm64: Refactor kvm_vcpu_enable_ptrauth() for hyp use

[Fuad Tabba]

Move kvm_vcpu_enable_ptrauth() to a shared header to be used by
hypervisor code in protected mode.

No functional change intended.

Signed-off-by: Fuad Tabba <tabba@google.com>
---
 arch/arm64/include/asm/kvm_emulate.h | 4 ++++
 arch/arm64/kvm/reset.c               | 5 -----
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
index a601a9305b10..9a68befbeecf 100644
--- a/arch/arm64/include/asm/kvm_emulate.h
+++ b/arch/arm64/include/asm/kvm_emulate.h
@@ -693,4 +693,8 @@ static inline bool guest_hyp_sve_traps_enabled(const struct kvm_vcpu *vcpu)
 	return __guest_hyp_cptr_xen_trap_enabled(vcpu, ZEN);
 }
 
+static inline void kvm_vcpu_enable_ptrauth(struct kvm_vcpu *vcpu)
+{
+	vcpu_set_flag(vcpu, GUEST_HAS_PTRAUTH);
+}
 #endif /* __ARM64_KVM_EMULATE_H__ */
diff --git a/arch/arm64/kvm/reset.c b/arch/arm64/kvm/reset.c
index 0b0ae5ae7bc2..470524b31951 100644
--- a/arch/arm64/kvm/reset.c
+++ b/arch/arm64/kvm/reset.c
@@ -167,11 +167,6 @@ static void kvm_vcpu_reset_sve(struct kvm_vcpu *vcpu)
 		memset(vcpu->arch.sve_state, 0, vcpu_sve_state_size(vcpu));
 }
 
-static void kvm_vcpu_enable_ptrauth(struct kvm_vcpu *vcpu)
-{
-	vcpu_set_flag(vcpu, GUEST_HAS_PTRAUTH);
-}
-
 /**
  * kvm_reset_vcpu - sets core registers and sys_regs to reset value
  * @vcpu: The VCPU pointer
-- 
2.47.0.rc1.288.g06298d1525-goog

[PATCH v1 3/4] KVM: arm64: Initialize the hypervisor's VM state at EL2

[Fuad Tabba]

Do not trust the state of the VM as provided by the host when
initializing the hypervisor's view of the VM sate. Initialize it
instead at EL2 to a known good and safe state, as pKVM already
does with hypervisor VCPU states.

Signed-off-by: Fuad Tabba <tabba@google.com>
---
 arch/arm64/kvm/hyp/nvhe/pkvm.c | 77 ++++++++++++++++++++++++++++++++++
 1 file changed, 77 insertions(+)

diff --git a/arch/arm64/kvm/hyp/nvhe/pkvm.c b/arch/arm64/kvm/hyp/nvhe/pkvm.c
index 869955e551a0..954df57a935f 100644
--- a/arch/arm64/kvm/hyp/nvhe/pkvm.c
+++ b/arch/arm64/kvm/hyp/nvhe/pkvm.c
@@ -6,6 +6,9 @@
 
 #include <linux/kvm_host.h>
 #include <linux/mm.h>
+
+#include <asm/kvm_emulate.h>
+
 #include <nvhe/fixed_config.h>
 #include <nvhe/mem_protect.h>
 #include <nvhe/memory.h>
@@ -289,6 +292,65 @@ void pkvm_put_hyp_vcpu(struct pkvm_hyp_vcpu *hyp_vcpu)
 	hyp_spin_unlock(&vm_table_lock);
 }
 
+static void pkvm_init_features_from_host(struct pkvm_hyp_vm *hyp_vm, const struct kvm *host_kvm)
+{
+	struct kvm *kvm = &hyp_vm->kvm;
+	DECLARE_BITMAP(allowed_features, KVM_VCPU_MAX_FEATURES);
+
+	/* No restrictions for non-protected VMs. */
+	if (!kvm_vm_is_protected(kvm)) {
+		bitmap_copy(kvm->arch.vcpu_features,
+			    host_kvm->arch.vcpu_features,
+			    KVM_VCPU_MAX_FEATURES);
+		return;
+	}
+
+	bitmap_zero(allowed_features, KVM_VCPU_MAX_FEATURES);
+
+	/*
+	 * For protected VMs, always allow:
+	 * - CPU starting in poweroff state
+	 * - PSCI v0.2
+	 */
+	set_bit(KVM_ARM_VCPU_POWER_OFF, allowed_features);
+	set_bit(KVM_ARM_VCPU_PSCI_0_2, allowed_features);
+
+	/*
+	 * Check if remaining features are allowed:
+	 * - Performance Monitoring
+	 * - Scalable Vectors
+	 * - Pointer Authentication
+	 */
+	if (FIELD_GET(ARM64_FEATURE_MASK(ID_AA64DFR0_EL1_PMUVer), PVM_ID_AA64DFR0_ALLOW))
+		set_bit(KVM_ARM_VCPU_PMU_V3, allowed_features);
+
+	if (FIELD_GET(ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_SVE), PVM_ID_AA64PFR0_ALLOW))
+		set_bit(KVM_ARM_VCPU_SVE, allowed_features);
+
+	if (FIELD_GET(ARM64_FEATURE_MASK(ID_AA64ISAR1_EL1_API), PVM_ID_AA64ISAR1_RESTRICT_UNSIGNED) &&
+	    FIELD_GET(ARM64_FEATURE_MASK(ID_AA64ISAR1_EL1_APA), PVM_ID_AA64ISAR1_RESTRICT_UNSIGNED))
+		set_bit(KVM_ARM_VCPU_PTRAUTH_ADDRESS, allowed_features);
+
+	if (FIELD_GET(ARM64_FEATURE_MASK(ID_AA64ISAR1_EL1_GPI), PVM_ID_AA64ISAR1_ALLOW) &&
+	    FIELD_GET(ARM64_FEATURE_MASK(ID_AA64ISAR1_EL1_GPA), PVM_ID_AA64ISAR1_ALLOW))
+		set_bit(KVM_ARM_VCPU_PTRAUTH_GENERIC, allowed_features);
+
+	bitmap_and(kvm->arch.vcpu_features, host_kvm->arch.vcpu_features,
+		   allowed_features, KVM_VCPU_MAX_FEATURES);
+}
+
+static void pkvm_vcpu_init_ptrauth(struct pkvm_hyp_vcpu *hyp_vcpu)
+{
+	struct kvm_vcpu *vcpu = &hyp_vcpu->vcpu;
+
+	if (vcpu_has_feature(vcpu, KVM_ARM_VCPU_PTRAUTH_ADDRESS) ||
+	    vcpu_has_feature(vcpu, KVM_ARM_VCPU_PTRAUTH_GENERIC)) {
+		kvm_vcpu_enable_ptrauth(vcpu);
+	} else {
+		vcpu_clear_flag(&hyp_vcpu->vcpu, GUEST_HAS_PTRAUTH);
+	}
+}
+
 static void unpin_host_vcpu(struct kvm_vcpu *host_vcpu)
 {
 	if (host_vcpu)
@@ -310,6 +372,18 @@ static void init_pkvm_hyp_vm(struct kvm *host_kvm, struct pkvm_hyp_vm *hyp_vm,
 	hyp_vm->host_kvm = host_kvm;
 	hyp_vm->kvm.created_vcpus = nr_vcpus;
 	hyp_vm->kvm.arch.mmu.vtcr = host_mmu.arch.mmu.vtcr;
+	hyp_vm->kvm.arch.pkvm.enabled = READ_ONCE(host_kvm->arch.pkvm.enabled);
+	pkvm_init_features_from_host(hyp_vm, host_kvm);
+}
+
+static void pkvm_vcpu_init_sve(struct pkvm_hyp_vcpu *hyp_vcpu, struct kvm_vcpu *host_vcpu)
+{
+	struct kvm_vcpu *vcpu = &hyp_vcpu->vcpu;
+
+	if (!vcpu_has_feature(vcpu, KVM_ARM_VCPU_SVE)) {
+		vcpu_clear_flag(vcpu, GUEST_HAS_SVE);
+		vcpu_clear_flag(vcpu, VCPU_SVE_FINALIZED);
+	}
 }
 
 static int init_pkvm_hyp_vcpu(struct pkvm_hyp_vcpu *hyp_vcpu,
@@ -335,7 +409,10 @@ static int init_pkvm_hyp_vcpu(struct pkvm_hyp_vcpu *hyp_vcpu,
 
 	hyp_vcpu->vcpu.arch.hw_mmu = &hyp_vm->kvm.arch.mmu;
 	hyp_vcpu->vcpu.arch.cflags = READ_ONCE(host_vcpu->arch.cflags);
+	hyp_vcpu->vcpu.arch.mp_state.mp_state = KVM_MP_STATE_STOPPED;
 
+	pkvm_vcpu_init_sve(hyp_vcpu, host_vcpu);
+	pkvm_vcpu_init_ptrauth(hyp_vcpu);
 	pkvm_vcpu_init_traps(&hyp_vcpu->vcpu);
 done:
 	if (ret)
-- 
2.47.0.rc1.288.g06298d1525-goog

[PATCH v1 4/4] KVM: arm64: Initialize trap register values in hyp in pKVM

[Fuad Tabba]

Handle the initialization of trap registers at the hypervisor in
pKVM, even for non-protected guests. The host is not trusted with
the values of the trap registers, regardless of the VM type.
Therefore, when switching between the host and the guests, only
flush the required bits of the trap registers from the host's
view that are required for hyp-host communication.

Reported-by: Will Deacon <will@kernel.org>
Fixes: 814ad8f96e92 ("KVM: arm64: Drop trapping of PAuth instructions/keys")
Signed-off-by: Fuad Tabba <tabba@google.com>
---
 arch/arm64/kvm/hyp/nvhe/hyp-main.c |  4 +++-
 arch/arm64/kvm/hyp/nvhe/pkvm.c     | 35 ++++++++++++++++++++++++++++++
 2 files changed, 38 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-main.c b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
index 1a224d5df207..6aa0b13d86e5 100644
--- a/arch/arm64/kvm/hyp/nvhe/hyp-main.c
+++ b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
@@ -105,8 +105,10 @@ static void flush_hyp_vcpu(struct pkvm_hyp_vcpu *hyp_vcpu)
 
 	hyp_vcpu->vcpu.arch.hw_mmu	= host_vcpu->arch.hw_mmu;
 
-	hyp_vcpu->vcpu.arch.hcr_el2	= host_vcpu->arch.hcr_el2;
 	hyp_vcpu->vcpu.arch.mdcr_el2	= host_vcpu->arch.mdcr_el2;
+	hyp_vcpu->vcpu.arch.hcr_el2 &= ~(HCR_TWI | HCR_TWE);
+	hyp_vcpu->vcpu.arch.hcr_el2 |= READ_ONCE(host_vcpu->arch.hcr_el2) &
+						 (HCR_TWI | HCR_TWE);
 
 	hyp_vcpu->vcpu.arch.iflags	= host_vcpu->arch.iflags;
 
diff --git a/arch/arm64/kvm/hyp/nvhe/pkvm.c b/arch/arm64/kvm/hyp/nvhe/pkvm.c
index 954df57a935f..01616c39a810 100644
--- a/arch/arm64/kvm/hyp/nvhe/pkvm.c
+++ b/arch/arm64/kvm/hyp/nvhe/pkvm.c
@@ -204,11 +204,46 @@ static void pvm_init_trap_regs(struct kvm_vcpu *vcpu)
 	}
 }
 
+static void pkvm_vcpu_reset_hcr(struct kvm_vcpu *vcpu)
+{
+	vcpu->arch.hcr_el2 = HCR_GUEST_FLAGS;
+
+	if (has_hvhe())
+		vcpu->arch.hcr_el2 |= HCR_E2H;
+
+	if (cpus_have_final_cap(ARM64_HAS_RAS_EXTN)) {
+		/* route synchronous external abort exceptions to EL2 */
+		vcpu->arch.hcr_el2 |= HCR_TEA;
+		/* trap error record accesses */
+		vcpu->arch.hcr_el2 |= HCR_TERR;
+	}
+
+	if (cpus_have_final_cap(ARM64_HAS_STAGE2_FWB))
+		vcpu->arch.hcr_el2 |= HCR_FWB;
+
+	if (cpus_have_final_cap(ARM64_HAS_EVT) &&
+	    !cpus_have_final_cap(ARM64_MISMATCHED_CACHE_TYPE))
+		vcpu->arch.hcr_el2 |= HCR_TID4;
+	else
+		vcpu->arch.hcr_el2 |= HCR_TID2;
+
+	if (vcpu_has_ptrauth(vcpu))
+		vcpu->arch.hcr_el2 |= (HCR_API | HCR_APK);
+}
+
 /*
  * Initialize trap register values in protected mode.
  */
 static void pkvm_vcpu_init_traps(struct kvm_vcpu *vcpu)
 {
+	vcpu->arch.cptr_el2 = kvm_get_reset_cptr_el2(vcpu);
+	vcpu->arch.mdcr_el2 = 0;
+
+	pkvm_vcpu_reset_hcr(vcpu);
+
+	if ((!vcpu_is_protected(vcpu)))
+		return;
+
 	pvm_init_trap_regs(vcpu);
 	pvm_init_traps_aa64pfr0(vcpu);
 	pvm_init_traps_aa64pfr1(vcpu);
-- 
2.47.0.rc1.288.g06298d1525-goog

Re: [PATCH v1 4/4] KVM: arm64: Initialize trap register values in hyp in pKVM

[Oliver Upton]

On Mon, Oct 14, 2024 at 11:24:12AM +0100, Fuad Tabba wrote:
> Handle the initialization of trap registers at the hypervisor in
> pKVM, even for non-protected guests. The host is not trusted with
> the values of the trap registers, regardless of the VM type.
> Therefore, when switching between the host and the guests, only
> flush the required bits of the trap registers from the host's
> view that are required for hyp-host communication.

It'd be a bit more precise to just mention that the host is allowed to
configure TWI and TWE still for opportunistic scheduling, as neither
affects the protection of VMs or the hypervisor.

>  	pvm_init_trap_regs(vcpu);
>  	pvm_init_traps_aa64pfr0(vcpu);
>  	pvm_init_traps_aa64pfr1(vcpu);

aside: it'd be great to reorganize all of these traps in terms of the
affected trap register and not the feature register.

But that can happen later.

-- 
Thanks,
Oliver

Re: [PATCH v1 4/4] KVM: arm64: Initialize trap register values in hyp in pKVM

[Fuad Tabba]

Hi Oliver,

On Tue, 15 Oct 2024 at 08:57, Oliver Upton <oliver.upton@linux.dev> wrote:
>
> On Mon, Oct 14, 2024 at 11:24:12AM +0100, Fuad Tabba wrote:
> > Handle the initialization of trap registers at the hypervisor in
> > pKVM, even for non-protected guests. The host is not trusted with
> > the values of the trap registers, regardless of the VM type.
> > Therefore, when switching between the host and the guests, only
> > flush the required bits of the trap registers from the host's
> > view that are required for hyp-host communication.
>
> It'd be a bit more precise to just mention that the host is allowed to
> configure TWI and TWE still for opportunistic scheduling, as neither
> affects the protection of VMs or the hypervisor.

Sure, will do.

>
> >       pvm_init_trap_regs(vcpu);
> >       pvm_init_traps_aa64pfr0(vcpu);
> >       pvm_init_traps_aa64pfr1(vcpu);
>
> aside: it'd be great to reorganize all of these traps in terms of the
> affected trap register and not the feature register.
>
> But that can happen later.

I do have patches for that, but I thought I'll start with these first
since they pave the way to those.

Thanks,
/fuad

> --
> Thanks,
> Oliver

Re: [PATCH v1 4/4] KVM: arm64: Initialize trap register values in hyp in pKVM

[Oliver Upton]

On Tue, Oct 15, 2024 at 09:19:03AM +0100, Fuad Tabba wrote:
> Hi Oliver,
> 
> On Tue, 15 Oct 2024 at 08:57, Oliver Upton <oliver.upton@linux.dev> wrote:
> >
> > On Mon, Oct 14, 2024 at 11:24:12AM +0100, Fuad Tabba wrote:
> > > Handle the initialization of trap registers at the hypervisor in
> > > pKVM, even for non-protected guests. The host is not trusted with
> > > the values of the trap registers, regardless of the VM type.
> > > Therefore, when switching between the host and the guests, only
> > > flush the required bits of the trap registers from the host's
> > > view that are required for hyp-host communication.
> >
> > It'd be a bit more precise to just mention that the host is allowed to
> > configure TWI and TWE still for opportunistic scheduling, as neither
> > affects the protection of VMs or the hypervisor.
> 
> Sure, will do.
> 
> >
> > >       pvm_init_trap_regs(vcpu);
> > >       pvm_init_traps_aa64pfr0(vcpu);
> > >       pvm_init_traps_aa64pfr1(vcpu);
> >
> > aside: it'd be great to reorganize all of these traps in terms of the
> > affected trap register and not the feature register.
> >
> > But that can happen later.
> 
> I do have patches for that, but I thought I'll start with these first
> since they pave the way to those.

Entirely reasonable, as you're fixing a legitimate issue here.

-- 
Thanks,
Oliver