* [PATCH v3 0/4] KVM: arm64: pKVM fixes
@ 2025-12-16 10:30 Alexandru Elisei
2025-12-16 10:30 ` [PATCH v3 1/4] KVM: arm64: Copy FGT traps to unprotected pKVM VCPU on VCPU load Alexandru Elisei
` (5 more replies)
0 siblings, 6 replies; 7+ messages in thread
From: Alexandru Elisei @ 2025-12-16 10:30 UTC (permalink / raw)
To: maz, oupton, joey.gouly, suzuki.poulose, yuzenghui, qperret, will,
tabba, linux-arm-kernel, kvmarm
v2 can be found at [1].
Similar to [2], when trying to run an unprotected VM on FVP with S1PIE enabled
and kvm-arm.mode=protected, writes to PIRE0_EL1 made by the guest in
__cpu_setup() are trapped by KVM and the BUG_ON(!r->access) is hit. That's
because HFGWTR_EL2.nPIRE0_EL1 is an inverse polarity trap and the FGT values for
the unprotected pKVM VCPU weren't being propagated from kvm_arch_vcpu_load().
Couldn't figure out how to run a protected pKVM VM, so that's untested.
Changelog v2->v3:
* Gathered Reviewed-by tags, thanks!
* In patch #2, KVM now calls bad_trap() instead of KVM_BUG(). Also changed patch
subject to match.
[1] https://lore.kernel.org/kvmarm/20251215114409.212512-1-alexandru.elisei@arm.com/
[2] https://lore.kernel.org/all/20251112102853.47759-1-alexandru.elisei@arm.com/
Alexandru Elisei (4):
KVM: arm64: Copy FGT traps to unprotected pKVM VCPU on VCPU load
KVM: arm64: Inject UNDEF for a register trap without accessor
KVM: arm64: Remove extra argument for __pvkm_host_{share,unshare}_hyp()
KVM: arm64: Remove unused parameter in synchronize_vcpu_pstate()
arch/arm64/kvm/hyp/include/hyp/switch.h | 2 +-
arch/arm64/kvm/hyp/nvhe/hyp-main.c | 3 +++
arch/arm64/kvm/hyp/nvhe/pkvm.c | 1 -
arch/arm64/kvm/hyp/nvhe/switch.c | 2 +-
arch/arm64/kvm/hyp/vhe/switch.c | 2 +-
arch/arm64/kvm/mmu.c | 4 ++--
arch/arm64/kvm/sys_regs.c | 5 ++++-
7 files changed, 12 insertions(+), 7 deletions(-)
base-commit: 8f0b4cce4481fb22653697cced8d0d04027cb1e8
--
2.52.0
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v3 1/4] KVM: arm64: Copy FGT traps to unprotected pKVM VCPU on VCPU load
2025-12-16 10:30 [PATCH v3 0/4] KVM: arm64: pKVM fixes Alexandru Elisei
@ 2025-12-16 10:30 ` Alexandru Elisei
2025-12-16 10:30 ` [PATCH v3 2/4] KVM: arm64: Inject UNDEF for a register trap without accessor Alexandru Elisei
` (4 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Alexandru Elisei @ 2025-12-16 10:30 UTC (permalink / raw)
To: maz, oupton, joey.gouly, suzuki.poulose, yuzenghui, qperret, will,
tabba, linux-arm-kernel, kvmarm
Commit fb10ddf35c1c ("KVM: arm64: Compute per-vCPU FGTs at vcpu_load()")
introduced per-VCPU FGT traps. For an unprotected pKVM VCPU, the untrusted
host FGT configuration is copied in pkvm_vcpu_init_traps(), which is called
from __pkvm_init_vcpu(). __pkvm_init_vcpu() is called once per VCPU (when
the VCPU is first run) which means that the uninitialized, zero, values for
the FGT registers end up being used for the entire lifetime of the VCPU.
This causes both unwanted traps (for the inverse polarity trap bits) and
the guest being allowed to access registers it shouldn't.
Fix it by copying the FGT traps for unprotected pKVM VCPUs when the
untrusted host loads the VCPU.
Fixes: fb10ddf35c1c ("KVM: arm64: Compute per-vCPU FGTs at vcpu_load()")
Acked-by: Will Deacon <will@kernel.org>
Tested-by: Fuad Tabba <tabba@google.com>
Reviewed-by: Fuad Tabba <tabba@google.com>
Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
arch/arm64/kvm/hyp/nvhe/hyp-main.c | 3 +++
arch/arm64/kvm/hyp/nvhe/pkvm.c | 1 -
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-main.c b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
index a7c689152f68..8ffbbce5e2ed 100644
--- a/arch/arm64/kvm/hyp/nvhe/hyp-main.c
+++ b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
@@ -180,6 +180,9 @@ static void handle___pkvm_vcpu_load(struct kvm_cpu_context *host_ctxt)
/* Propagate WFx trapping flags */
hyp_vcpu->vcpu.arch.hcr_el2 &= ~(HCR_TWE | HCR_TWI);
hyp_vcpu->vcpu.arch.hcr_el2 |= hcr_el2 & (HCR_TWE | HCR_TWI);
+ } else {
+ memcpy(&hyp_vcpu->vcpu.arch.fgt, hyp_vcpu->host_vcpu->arch.fgt,
+ sizeof(hyp_vcpu->vcpu.arch.fgt));
}
}
diff --git a/arch/arm64/kvm/hyp/nvhe/pkvm.c b/arch/arm64/kvm/hyp/nvhe/pkvm.c
index 8911338961c5..12b2acfbcfd1 100644
--- a/arch/arm64/kvm/hyp/nvhe/pkvm.c
+++ b/arch/arm64/kvm/hyp/nvhe/pkvm.c
@@ -172,7 +172,6 @@ static int pkvm_vcpu_init_traps(struct pkvm_hyp_vcpu *hyp_vcpu)
/* Trust the host for non-protected vcpu features. */
vcpu->arch.hcrx_el2 = host_vcpu->arch.hcrx_el2;
- memcpy(vcpu->arch.fgt, host_vcpu->arch.fgt, sizeof(vcpu->arch.fgt));
return 0;
}
--
2.52.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v3 2/4] KVM: arm64: Inject UNDEF for a register trap without accessor
2025-12-16 10:30 [PATCH v3 0/4] KVM: arm64: pKVM fixes Alexandru Elisei
2025-12-16 10:30 ` [PATCH v3 1/4] KVM: arm64: Copy FGT traps to unprotected pKVM VCPU on VCPU load Alexandru Elisei
@ 2025-12-16 10:30 ` Alexandru Elisei
2025-12-16 10:30 ` [PATCH v3 3/4] KVM: arm64: Remove extra argument for __pvkm_host_{share,unshare}_hyp() Alexandru Elisei
` (3 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Alexandru Elisei @ 2025-12-16 10:30 UTC (permalink / raw)
To: maz, oupton, joey.gouly, suzuki.poulose, yuzenghui, qperret, will,
tabba, linux-arm-kernel, kvmarm
Configuring a register trap without specifying an accessor function is
abviously a bug. Instead of calling die() when that happens, let's be a
bit more helpful and print the register encoding. Also inject an
undefined instruction exception in the guest, similar to other unhandled
register accesses.
Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
arch/arm64/kvm/sys_regs.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index c8fd7c6a12a1..88a57ca36d96 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -4668,7 +4668,10 @@ static void perform_access(struct kvm_vcpu *vcpu,
* that we don't know how to handle. This certainly qualifies
* as a gross bug that should be fixed right away.
*/
- BUG_ON(!r->access);
+ if (!r->access) {
+ bad_trap(vcpu, params, r, "register access");
+ return;
+ }
/* Skip instruction if instructed so */
if (likely(r->access(vcpu, params, r)))
--
2.52.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v3 3/4] KVM: arm64: Remove extra argument for __pvkm_host_{share,unshare}_hyp()
2025-12-16 10:30 [PATCH v3 0/4] KVM: arm64: pKVM fixes Alexandru Elisei
2025-12-16 10:30 ` [PATCH v3 1/4] KVM: arm64: Copy FGT traps to unprotected pKVM VCPU on VCPU load Alexandru Elisei
2025-12-16 10:30 ` [PATCH v3 2/4] KVM: arm64: Inject UNDEF for a register trap without accessor Alexandru Elisei
@ 2025-12-16 10:30 ` Alexandru Elisei
2025-12-16 10:30 ` [PATCH v3 4/4] KVM: arm64: Remove unused parameter in synchronize_vcpu_pstate() Alexandru Elisei
` (2 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Alexandru Elisei @ 2025-12-16 10:30 UTC (permalink / raw)
To: maz, oupton, joey.gouly, suzuki.poulose, yuzenghui, qperret, will,
tabba, linux-arm-kernel, kvmarm
__pvkm_host_share_hyp() and __pkvm_host_unshare_hyp() both have one
parameter, the pfn, not two. Even though correctness isn't impacted because
the SMCCC handlers pass the first argument and ignore the second one, let's
call the functions with the proper number of arguments.
Reviewed-by: Fuad Tabba <tabba@google.com>
Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
arch/arm64/kvm/mmu.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
index 48d7c372a4cd..124404eb208d 100644
--- a/arch/arm64/kvm/mmu.c
+++ b/arch/arm64/kvm/mmu.c
@@ -497,7 +497,7 @@ static int share_pfn_hyp(u64 pfn)
this->count = 1;
rb_link_node(&this->node, parent, node);
rb_insert_color(&this->node, &hyp_shared_pfns);
- ret = kvm_call_hyp_nvhe(__pkvm_host_share_hyp, pfn, 1);
+ ret = kvm_call_hyp_nvhe(__pkvm_host_share_hyp, pfn);
unlock:
mutex_unlock(&hyp_shared_pfns_lock);
@@ -523,7 +523,7 @@ static int unshare_pfn_hyp(u64 pfn)
rb_erase(&this->node, &hyp_shared_pfns);
kfree(this);
- ret = kvm_call_hyp_nvhe(__pkvm_host_unshare_hyp, pfn, 1);
+ ret = kvm_call_hyp_nvhe(__pkvm_host_unshare_hyp, pfn);
unlock:
mutex_unlock(&hyp_shared_pfns_lock);
--
2.52.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v3 4/4] KVM: arm64: Remove unused parameter in synchronize_vcpu_pstate()
2025-12-16 10:30 [PATCH v3 0/4] KVM: arm64: pKVM fixes Alexandru Elisei
` (2 preceding siblings ...)
2025-12-16 10:30 ` [PATCH v3 3/4] KVM: arm64: Remove extra argument for __pvkm_host_{share,unshare}_hyp() Alexandru Elisei
@ 2025-12-16 10:30 ` Alexandru Elisei
2025-12-16 10:43 ` [PATCH v3 0/4] KVM: arm64: pKVM fixes Fuad Tabba
2025-12-17 9:18 ` Marc Zyngier
5 siblings, 0 replies; 7+ messages in thread
From: Alexandru Elisei @ 2025-12-16 10:30 UTC (permalink / raw)
To: maz, oupton, joey.gouly, suzuki.poulose, yuzenghui, qperret, will,
tabba, linux-arm-kernel, kvmarm
synchronize_vcpu_pstate() doesn't make use of the reference to exit_code,
remove the parameter.
Reviewed-by: Fuad Tabba <tabba@google.com>
Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
arch/arm64/kvm/hyp/include/hyp/switch.h | 2 +-
arch/arm64/kvm/hyp/nvhe/switch.c | 2 +-
arch/arm64/kvm/hyp/vhe/switch.c | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/arm64/kvm/hyp/include/hyp/switch.h b/arch/arm64/kvm/hyp/include/hyp/switch.h
index c5d5e5b86eaf..afecbdd3c1e9 100644
--- a/arch/arm64/kvm/hyp/include/hyp/switch.h
+++ b/arch/arm64/kvm/hyp/include/hyp/switch.h
@@ -854,7 +854,7 @@ static inline bool kvm_hyp_handle_exit(struct kvm_vcpu *vcpu, u64 *exit_code,
return false;
}
-static inline void synchronize_vcpu_pstate(struct kvm_vcpu *vcpu, u64 *exit_code)
+static inline void synchronize_vcpu_pstate(struct kvm_vcpu *vcpu)
{
/*
* Check for the conditions of Cortex-A510's #2077057. When these occur
diff --git a/arch/arm64/kvm/hyp/nvhe/switch.c b/arch/arm64/kvm/hyp/nvhe/switch.c
index d3b9ec8a7c28..779089e42681 100644
--- a/arch/arm64/kvm/hyp/nvhe/switch.c
+++ b/arch/arm64/kvm/hyp/nvhe/switch.c
@@ -211,7 +211,7 @@ static inline bool fixup_guest_exit(struct kvm_vcpu *vcpu, u64 *exit_code)
{
const exit_handler_fn *handlers = kvm_get_exit_handler_array(vcpu);
- synchronize_vcpu_pstate(vcpu, exit_code);
+ synchronize_vcpu_pstate(vcpu);
/*
* Some guests (e.g., protected VMs) are not be allowed to run in
diff --git a/arch/arm64/kvm/hyp/vhe/switch.c b/arch/arm64/kvm/hyp/vhe/switch.c
index 9984c492305a..9db3f11a4754 100644
--- a/arch/arm64/kvm/hyp/vhe/switch.c
+++ b/arch/arm64/kvm/hyp/vhe/switch.c
@@ -536,7 +536,7 @@ static const exit_handler_fn hyp_exit_handlers[] = {
static inline bool fixup_guest_exit(struct kvm_vcpu *vcpu, u64 *exit_code)
{
- synchronize_vcpu_pstate(vcpu, exit_code);
+ synchronize_vcpu_pstate(vcpu);
/*
* If we were in HYP context on entry, adjust the PSTATE view
--
2.52.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v3 0/4] KVM: arm64: pKVM fixes
2025-12-16 10:30 [PATCH v3 0/4] KVM: arm64: pKVM fixes Alexandru Elisei
` (3 preceding siblings ...)
2025-12-16 10:30 ` [PATCH v3 4/4] KVM: arm64: Remove unused parameter in synchronize_vcpu_pstate() Alexandru Elisei
@ 2025-12-16 10:43 ` Fuad Tabba
2025-12-17 9:18 ` Marc Zyngier
5 siblings, 0 replies; 7+ messages in thread
From: Fuad Tabba @ 2025-12-16 10:43 UTC (permalink / raw)
To: Alexandru Elisei
Cc: maz, oupton, joey.gouly, suzuki.poulose, yuzenghui, qperret, will,
linux-arm-kernel, kvmarm
Hi Alexandru,
On Tue, 16 Dec 2025 at 10:31, Alexandru Elisei <alexandru.elisei@arm.com> wrote:
>
> v2 can be found at [1].
>
> Similar to [2], when trying to run an unprotected VM on FVP with S1PIE enabled
> and kvm-arm.mode=protected, writes to PIRE0_EL1 made by the guest in
> __cpu_setup() are trapped by KVM and the BUG_ON(!r->access) is hit. That's
> because HFGWTR_EL2.nPIRE0_EL1 is an inverse polarity trap and the FGT values for
> the unprotected pKVM VCPU weren't being propagated from kvm_arch_vcpu_load().
>
> Couldn't figure out how to run a protected pKVM VM, so that's untested.
Hopefully this will change soon [1]!
I applied this to the pKVM/Android stack, and tested it with protected
VMs as well as non-protected VMs.
For the series:
Tested-by: Fuad Tabba <tabba@google.com>
Reviewed-by: Fuad Tabba <tabba@google.com>
Cheers,
/fuad
[1] https://lore.kernel.org/all/aTMPn0dBbSVUwwJ1@willie-the-truck/
>
> Changelog v2->v3:
> * Gathered Reviewed-by tags, thanks!
> * In patch #2, KVM now calls bad_trap() instead of KVM_BUG(). Also changed patch
> subject to match.
>
> [1] https://lore.kernel.org/kvmarm/20251215114409.212512-1-alexandru.elisei@arm.com/
> [2] https://lore.kernel.org/all/20251112102853.47759-1-alexandru.elisei@arm.com/
>
> Alexandru Elisei (4):
> KVM: arm64: Copy FGT traps to unprotected pKVM VCPU on VCPU load
> KVM: arm64: Inject UNDEF for a register trap without accessor
> KVM: arm64: Remove extra argument for __pvkm_host_{share,unshare}_hyp()
> KVM: arm64: Remove unused parameter in synchronize_vcpu_pstate()
>
> arch/arm64/kvm/hyp/include/hyp/switch.h | 2 +-
> arch/arm64/kvm/hyp/nvhe/hyp-main.c | 3 +++
> arch/arm64/kvm/hyp/nvhe/pkvm.c | 1 -
> arch/arm64/kvm/hyp/nvhe/switch.c | 2 +-
> arch/arm64/kvm/hyp/vhe/switch.c | 2 +-
> arch/arm64/kvm/mmu.c | 4 ++--
> arch/arm64/kvm/sys_regs.c | 5 ++++-
> 7 files changed, 12 insertions(+), 7 deletions(-)
>
>
> base-commit: 8f0b4cce4481fb22653697cced8d0d04027cb1e8
> --
> 2.52.0
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3 0/4] KVM: arm64: pKVM fixes
2025-12-16 10:30 [PATCH v3 0/4] KVM: arm64: pKVM fixes Alexandru Elisei
` (4 preceding siblings ...)
2025-12-16 10:43 ` [PATCH v3 0/4] KVM: arm64: pKVM fixes Fuad Tabba
@ 2025-12-17 9:18 ` Marc Zyngier
5 siblings, 0 replies; 7+ messages in thread
From: Marc Zyngier @ 2025-12-17 9:18 UTC (permalink / raw)
To: Alexandru Elisei
Cc: oupton, joey.gouly, suzuki.poulose, yuzenghui, qperret, will,
tabba, linux-arm-kernel, kvmarm
On Tue, 16 Dec 2025 10:30:49 +0000,
Alexandru Elisei <alexandru.elisei@arm.com> wrote:
>
> v2 can be found at [1].
>
> Similar to [2], when trying to run an unprotected VM on FVP with S1PIE enabled
> and kvm-arm.mode=protected, writes to PIRE0_EL1 made by the guest in
> __cpu_setup() are trapped by KVM and the BUG_ON(!r->access) is hit. That's
> because HFGWTR_EL2.nPIRE0_EL1 is an inverse polarity trap and the FGT values for
> the unprotected pKVM VCPU weren't being propagated from kvm_arch_vcpu_load().
>
> Couldn't figure out how to run a protected pKVM VM, so that's untested.
Reviewed-by: Marc Zyngier <maz@kernel.org>
M.
--
Without deviation from the norm, progress is not possible.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2025-12-17 9:18 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-16 10:30 [PATCH v3 0/4] KVM: arm64: pKVM fixes Alexandru Elisei
2025-12-16 10:30 ` [PATCH v3 1/4] KVM: arm64: Copy FGT traps to unprotected pKVM VCPU on VCPU load Alexandru Elisei
2025-12-16 10:30 ` [PATCH v3 2/4] KVM: arm64: Inject UNDEF for a register trap without accessor Alexandru Elisei
2025-12-16 10:30 ` [PATCH v3 3/4] KVM: arm64: Remove extra argument for __pvkm_host_{share,unshare}_hyp() Alexandru Elisei
2025-12-16 10:30 ` [PATCH v3 4/4] KVM: arm64: Remove unused parameter in synchronize_vcpu_pstate() Alexandru Elisei
2025-12-16 10:43 ` [PATCH v3 0/4] KVM: arm64: pKVM fixes Fuad Tabba
2025-12-17 9:18 ` Marc Zyngier
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).