* [PATCH v2 0/4] KVM: arm64: pKVM fixes
@ 2025-12-15 11:44 Alexandru Elisei
2025-12-15 11:44 ` [PATCH v2 1/4] KVM: arm64: Copy FGT traps to unprotected pKVM VCPU on VCPU load Alexandru Elisei
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: Alexandru Elisei @ 2025-12-15 11:44 UTC (permalink / raw)
To: maz, oupton, joey.gouly, suzuki.poulose, yuzenghui, qperret, will,
tabba, linux-arm-kernel, kvmarm
v1 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.
New in this iteration is #2 ("KVM: arm64: Print register encoding if there's no
accessor") because I realized that the first thing I did when I hit the
BUG_ON(!r->access) (for this series and for the fix at [2]) was to print the
encoding for the register. Thought that it might be more useful in the future
to do this by default.
Patch #4 ("KVM: arm64: Remove unused parameter in synchronize_vcpu_pstate()") is
also new - found it by code inspection while debugging something unrelated.
Changelog
=========
v1->v2:
* Rebased on top of v6.19-rc1.
* Added tags to patch #1 ("KVM: arm64: Copy FGT traps to unprotected pKVM
VCPU on VCPU load") - thanks!
* Removed local variables 'hyp_kvm_vcpu' and 'host_vcpu' in patch #1.
* Patch #2 and #4 are new.
* Patch #3 ("KVM: arm64: Remove extra argument for
__pvkm_host_{share,unshare}_hyp()") now properly fixes
__pkvm_host_unshare_hyp().
[1] https://lore.kernel.org/kvmarm/20251210132102.137631-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: Print register encoding if there's no 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 | 8 +++++++-
7 files changed, 15 insertions(+), 7 deletions(-)
base-commit: 8f0b4cce4481fb22653697cced8d0d04027cb1e8
--
2.52.0
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH v2 1/4] KVM: arm64: Copy FGT traps to unprotected pKVM VCPU on VCPU load 2025-12-15 11:44 [PATCH v2 0/4] KVM: arm64: pKVM fixes Alexandru Elisei @ 2025-12-15 11:44 ` Alexandru Elisei 2025-12-15 11:44 ` [PATCH v2 2/4] KVM: arm64: Print register encoding if there's no accessor Alexandru Elisei ` (2 subsequent siblings) 3 siblings, 0 replies; 10+ messages in thread From: Alexandru Elisei @ 2025-12-15 11:44 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] 10+ messages in thread
* [PATCH v2 2/4] KVM: arm64: Print register encoding if there's no accessor 2025-12-15 11:44 [PATCH v2 0/4] KVM: arm64: pKVM fixes Alexandru Elisei 2025-12-15 11:44 ` [PATCH v2 1/4] KVM: arm64: Copy FGT traps to unprotected pKVM VCPU on VCPU load Alexandru Elisei @ 2025-12-15 11:44 ` Alexandru Elisei 2025-12-15 13:58 ` Marc Zyngier 2025-12-15 11:44 ` [PATCH v2 3/4] KVM: arm64: Remove extra argument for __pvkm_host_{share,unshare}_hyp() Alexandru Elisei 2025-12-15 11:44 ` [PATCH v2 4/4] KVM: arm64: Remove unused parameter in synchronize_vcpu_pstate() Alexandru Elisei 3 siblings, 1 reply; 10+ messages in thread From: Alexandru Elisei @ 2025-12-15 11:44 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 and kill the virtual machine instead. Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com> --- arch/arm64/kvm/sys_regs.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c index c8fd7c6a12a1..d669f6fef177 100644 --- a/arch/arm64/kvm/sys_regs.c +++ b/arch/arm64/kvm/sys_regs.c @@ -4668,7 +4668,13 @@ 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) { + KVM_BUG(1, vcpu->kvm, + "Unexpected access to register: { Op0(%2u), Op1(%2u), CRn(%2u), CRm(%2u), Op2(%2u) } (%s)", + params->Op0, params->Op1, params->CRn, params->CRm, params->Op2, + str_write_read(params->is_write)); + return; + } /* Skip instruction if instructed so */ if (likely(r->access(vcpu, params, r))) -- 2.52.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/4] KVM: arm64: Print register encoding if there's no accessor 2025-12-15 11:44 ` [PATCH v2 2/4] KVM: arm64: Print register encoding if there's no accessor Alexandru Elisei @ 2025-12-15 13:58 ` Marc Zyngier 2025-12-15 15:17 ` Alexandru Elisei 0 siblings, 1 reply; 10+ messages in thread From: Marc Zyngier @ 2025-12-15 13:58 UTC (permalink / raw) To: Alexandru Elisei Cc: oupton, joey.gouly, suzuki.poulose, yuzenghui, qperret, will, tabba, linux-arm-kernel, kvmarm On Mon, 15 Dec 2025 11:44:07 +0000, Alexandru Elisei <alexandru.elisei@arm.com> wrote: > > 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 and kill the virtual machine > instead. > > Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com> > --- > arch/arm64/kvm/sys_regs.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c > index c8fd7c6a12a1..d669f6fef177 100644 > --- a/arch/arm64/kvm/sys_regs.c > +++ b/arch/arm64/kvm/sys_regs.c > @@ -4668,7 +4668,13 @@ 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) { > + KVM_BUG(1, vcpu->kvm, > + "Unexpected access to register: { Op0(%2u), Op1(%2u), CRn(%2u), CRm(%2u), Op2(%2u) } (%s)", > + params->Op0, params->Op1, params->CRn, params->CRm, params->Op2, > + str_write_read(params->is_write)); > + return; > + } Why not writing if (KVM_BUG(!r>access, ...)) return; instead? And you could reuse the format that's already defined in print_sys_reg_msg(). You could instead consider injecting an UNDEF in the guest, which I find more palatable than this "vm_bugged" stuff. It would at least be consistent with the "register does not exist in the sysreg table" approach that we already have. Thanks, M. -- Without deviation from the norm, progress is not possible. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/4] KVM: arm64: Print register encoding if there's no accessor 2025-12-15 13:58 ` Marc Zyngier @ 2025-12-15 15:17 ` Alexandru Elisei 2025-12-15 15:39 ` Marc Zyngier 0 siblings, 1 reply; 10+ messages in thread From: Alexandru Elisei @ 2025-12-15 15:17 UTC (permalink / raw) To: Marc Zyngier Cc: oupton, joey.gouly, suzuki.poulose, yuzenghui, qperret, will, tabba, linux-arm-kernel, kvmarm Hi Marc, On Mon, Dec 15, 2025 at 01:58:40PM +0000, Marc Zyngier wrote: > On Mon, 15 Dec 2025 11:44:07 +0000, > Alexandru Elisei <alexandru.elisei@arm.com> wrote: > > > > 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 and kill the virtual machine > > instead. > > > > Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com> > > --- > > arch/arm64/kvm/sys_regs.c | 8 +++++++- > > 1 file changed, 7 insertions(+), 1 deletion(-) > > > > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c > > index c8fd7c6a12a1..d669f6fef177 100644 > > --- a/arch/arm64/kvm/sys_regs.c > > +++ b/arch/arm64/kvm/sys_regs.c > > @@ -4668,7 +4668,13 @@ 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) { > > + KVM_BUG(1, vcpu->kvm, > > + "Unexpected access to register: { Op0(%2u), Op1(%2u), CRn(%2u), CRm(%2u), Op2(%2u) } (%s)", > > + params->Op0, params->Op1, params->CRn, params->CRm, params->Op2, > > + str_write_read(params->is_write)); > > + return; > > + } > > Why not writing > > if (KVM_BUG(!r>access, ...)) > return; > > instead? And you could reuse the format that's already defined in > print_sys_reg_msg(). > > You could instead consider injecting an UNDEF in the guest, which I Injecting an undefined instruction exception early in the boot process can lead to an infinite loop of undefined instruction exceptions in the guest. This is what happens for the unhandled PIRE0_EL1 trap that the previous patch fixes. The guest access takes place in __cpu_setup(), when there are no vectors installed. If you're ok with this, I can move forward with the below: diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c index c8fd7c6a12a1..516072417649 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, "Unexpected register access"); + return; + } /* Skip instruction if instructed so */ if (likely(r->access(vcpu, params, r))) And this is what KVM prints (the register encoding is outside the trace snippet, if you think that would make a difference to the end user): [ 4926.964472] ------------[ cut here ]------------ [ 4926.964529] Unexpected Unexpected register access [ 4926.964713] WARNING: arch/arm64/kvm/sys_regs.c:64 at bad_trap.isra.0+0x70/0x7c, CPU#5: kvm-vcpu-0/179 [ 4926.965014] Modules linked in: [ 4926.965097] CPU: 5 UID: 0 PID: 179 Comm: kvm-vcpu-0 Not tainted 6.19.0-rc1-dirty #85 PREEMPT [ 4926.965240] Hardware name: FVP Base RevC (DT) [ 4926.965316] pstate: 63402005 (nZCv daif +PAN -UAO +TCO +DIT -SSBS BTYPE=--) [ 4926.965442] pc : bad_trap.isra.0+0x70/0x7c [ 4926.965564] lr : bad_trap.isra.0+0x70/0x7c [ 4926.965687] sp : ffff800080be39f0 [ 4926.965760] x29: ffff800080be39f0 x28: ffff0008031e2dc0 x27: 0000000000000000 [ 4926.965943] x26: 0000000000000000 x25: 0000000000000000 x24: 000000000000ae80 [ 4926.966124] x23: ffff000800b78048 x22: ffff00080164c9c0 x21: ffff0008031e2dc0 [ 4926.966314] x20: ffff000800b78000 x19: ffff000800b78000 x18: ffffffffff00b468 [ 4926.966506] x17: 0000000000000000 x16: 0000000000000000 x15: ffffa991688ee120 [ 4926.966689] x14: fffffffffe00b467 x13: 0000000000001408 x12: 0000000000001470 [ 4926.966874] x11: 0000000000000058 x10: 0000000000000018 x9 : ffff000875334bc0 [ 4926.967058] x8 : 0000000002bfffa8 x7 : 0000000000000081 x6 : ffff000877f34bc0 [ 4926.967241] x5 : 0000000000000081 x4 : ffffa991674022a8 x3 : 0000000000000001 [ 4926.967423] x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff0008031e2dc0 [ 4926.967605] Call trace: [ 4926.967664] bad_trap.isra.0+0x70/0x7c (P) [ 4926.967805] perform_access+0xcc/0xd8 [ 4926.967943] kvm_handle_sys_reg+0x100/0x170 [ 4926.968068] handle_exit+0x58/0x168 [ 4926.968187] kvm_arch_vcpu_ioctl_run+0x2f8/0x900 [ 4926.968336] kvm_vcpu_ioctl+0x168/0xa18 [ 4926.968479] __arm64_sys_ioctl+0x2ec/0xaf4 [ 4926.968599] invoke_syscall.constprop.0+0x48/0xc8 [ 4926.968758] do_el0_svc+0x3c/0xb8 [ 4926.968903] el0_svc+0x3c/0x160 [ 4926.969014] el0t_64_sync_handler+0xa0/0xe4 [ 4926.969137] el0t_64_sync+0x198/0x19c [ 4926.969255] ---[ end trace 0000000000000000 ]--- [ 4926.969427] kvm [166]: { Op0( 3), Op1( 0), CRn(10), CRm( 2), Op2( 2), func_write }, Thanks, Alex ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/4] KVM: arm64: Print register encoding if there's no accessor 2025-12-15 15:17 ` Alexandru Elisei @ 2025-12-15 15:39 ` Marc Zyngier 0 siblings, 0 replies; 10+ messages in thread From: Marc Zyngier @ 2025-12-15 15:39 UTC (permalink / raw) To: Alexandru Elisei Cc: oupton, joey.gouly, suzuki.poulose, yuzenghui, qperret, will, tabba, linux-arm-kernel, kvmarm On Mon, 15 Dec 2025 15:17:27 +0000, Alexandru Elisei <alexandru.elisei@arm.com> wrote: > > Hi Marc, > > On Mon, Dec 15, 2025 at 01:58:40PM +0000, Marc Zyngier wrote: > > On Mon, 15 Dec 2025 11:44:07 +0000, > > Alexandru Elisei <alexandru.elisei@arm.com> wrote: > > > > > > 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 and kill the virtual machine > > > instead. > > > > > > Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com> > > > --- > > > arch/arm64/kvm/sys_regs.c | 8 +++++++- > > > 1 file changed, 7 insertions(+), 1 deletion(-) > > > > > > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c > > > index c8fd7c6a12a1..d669f6fef177 100644 > > > --- a/arch/arm64/kvm/sys_regs.c > > > +++ b/arch/arm64/kvm/sys_regs.c > > > @@ -4668,7 +4668,13 @@ 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) { > > > + KVM_BUG(1, vcpu->kvm, > > > + "Unexpected access to register: { Op0(%2u), Op1(%2u), CRn(%2u), CRm(%2u), Op2(%2u) } (%s)", > > > + params->Op0, params->Op1, params->CRn, params->CRm, params->Op2, > > > + str_write_read(params->is_write)); > > > + return; > > > + } > > > > Why not writing > > > > if (KVM_BUG(!r>access, ...)) > > return; > > > > instead? And you could reuse the format that's already defined in > > print_sys_reg_msg(). > > > > You could instead consider injecting an UNDEF in the guest, which I > > Injecting an undefined instruction exception early in the boot process can lead > to an infinite loop of undefined instruction exceptions in the guest. This is > what happens for the unhandled PIRE0_EL1 trap that the previous patch fixes. The > guest access takes place in __cpu_setup(), when there are no vectors > installed. *Linux* crashes in this way. Who cares about Linux? It's a matter of consistency. When KVM unexpectedly traps something, we inject an UNDEF *today*. Why should the lack of an access callback be treated any differently? What material difference does it make to the guest? Someone who wants to understand what is happening in their guest can attach a debugger to it and find out. > > If you're ok with this, I can move forward with the below: > > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c > index c8fd7c6a12a1..516072417649 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, "Unexpected register access"); > + return; > + } > > /* Skip instruction if instructed so */ > if (likely(r->access(vcpu, params, r))) > > And this is what KVM prints (the register encoding is outside the trace snippet, > if you think that would make a difference to the end user): > > [ 4926.964472] ------------[ cut here ]------------ > [ 4926.964529] Unexpected Unexpected register access ^^^^^^^^^^^^^^^^^^^^^ You can clean-up the message... > [ 4926.964713] WARNING: arch/arm64/kvm/sys_regs.c:64 at bad_trap.isra.0+0x70/0x7c, CPU#5: kvm-vcpu-0/179 > [ 4926.965014] Modules linked in: > [ 4926.965097] CPU: 5 UID: 0 PID: 179 Comm: kvm-vcpu-0 Not tainted 6.19.0-rc1-dirty #85 PREEMPT > [ 4926.965240] Hardware name: FVP Base RevC (DT) > [ 4926.965316] pstate: 63402005 (nZCv daif +PAN -UAO +TCO +DIT -SSBS BTYPE=--) > [ 4926.965442] pc : bad_trap.isra.0+0x70/0x7c > [ 4926.965564] lr : bad_trap.isra.0+0x70/0x7c > [ 4926.965687] sp : ffff800080be39f0 > [ 4926.965760] x29: ffff800080be39f0 x28: ffff0008031e2dc0 x27: 0000000000000000 > [ 4926.965943] x26: 0000000000000000 x25: 0000000000000000 x24: 000000000000ae80 > [ 4926.966124] x23: ffff000800b78048 x22: ffff00080164c9c0 x21: ffff0008031e2dc0 > [ 4926.966314] x20: ffff000800b78000 x19: ffff000800b78000 x18: ffffffffff00b468 > [ 4926.966506] x17: 0000000000000000 x16: 0000000000000000 x15: ffffa991688ee120 > [ 4926.966689] x14: fffffffffe00b467 x13: 0000000000001408 x12: 0000000000001470 > [ 4926.966874] x11: 0000000000000058 x10: 0000000000000018 x9 : ffff000875334bc0 > [ 4926.967058] x8 : 0000000002bfffa8 x7 : 0000000000000081 x6 : ffff000877f34bc0 > [ 4926.967241] x5 : 0000000000000081 x4 : ffffa991674022a8 x3 : 0000000000000001 > [ 4926.967423] x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff0008031e2dc0 > [ 4926.967605] Call trace: > [ 4926.967664] bad_trap.isra.0+0x70/0x7c (P) > [ 4926.967805] perform_access+0xcc/0xd8 > [ 4926.967943] kvm_handle_sys_reg+0x100/0x170 > [ 4926.968068] handle_exit+0x58/0x168 > [ 4926.968187] kvm_arch_vcpu_ioctl_run+0x2f8/0x900 > [ 4926.968336] kvm_vcpu_ioctl+0x168/0xa18 > [ 4926.968479] __arm64_sys_ioctl+0x2ec/0xaf4 > [ 4926.968599] invoke_syscall.constprop.0+0x48/0xc8 > [ 4926.968758] do_el0_svc+0x3c/0xb8 > [ 4926.968903] el0_svc+0x3c/0x160 > [ 4926.969014] el0t_64_sync_handler+0xa0/0xe4 > [ 4926.969137] el0t_64_sync+0x198/0x19c > [ 4926.969255] ---[ end trace 0000000000000000 ]--- > [ 4926.969427] kvm [166]: { Op0( 3), Op1( 0), CRn(10), CRm( 2), Op2( 2), func_write }, Works for me. The only small snag is that you only ever get one of these, irrespective of the register. Not a big deal. Thanks, M. -- Without deviation from the norm, progress is not possible. ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 3/4] KVM: arm64: Remove extra argument for __pvkm_host_{share,unshare}_hyp() 2025-12-15 11:44 [PATCH v2 0/4] KVM: arm64: pKVM fixes Alexandru Elisei 2025-12-15 11:44 ` [PATCH v2 1/4] KVM: arm64: Copy FGT traps to unprotected pKVM VCPU on VCPU load Alexandru Elisei 2025-12-15 11:44 ` [PATCH v2 2/4] KVM: arm64: Print register encoding if there's no accessor Alexandru Elisei @ 2025-12-15 11:44 ` Alexandru Elisei 2025-12-15 13:43 ` Fuad Tabba 2025-12-15 11:44 ` [PATCH v2 4/4] KVM: arm64: Remove unused parameter in synchronize_vcpu_pstate() Alexandru Elisei 3 siblings, 1 reply; 10+ messages in thread From: Alexandru Elisei @ 2025-12-15 11:44 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. 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] 10+ messages in thread
* Re: [PATCH v2 3/4] KVM: arm64: Remove extra argument for __pvkm_host_{share,unshare}_hyp() 2025-12-15 11:44 ` [PATCH v2 3/4] KVM: arm64: Remove extra argument for __pvkm_host_{share,unshare}_hyp() Alexandru Elisei @ 2025-12-15 13:43 ` Fuad Tabba 0 siblings, 0 replies; 10+ messages in thread From: Fuad Tabba @ 2025-12-15 13:43 UTC (permalink / raw) To: Alexandru Elisei Cc: maz, oupton, joey.gouly, suzuki.poulose, yuzenghui, qperret, will, linux-arm-kernel, kvmarm On Mon, 15 Dec 2025 at 11:44, Alexandru Elisei <alexandru.elisei@arm.com> wrote: > > __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. > > Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com> > --- Reviewed-by: Fuad Tabba <tabba@google.com> Cheers, /fuad > 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 [flat|nested] 10+ messages in thread
* [PATCH v2 4/4] KVM: arm64: Remove unused parameter in synchronize_vcpu_pstate() 2025-12-15 11:44 [PATCH v2 0/4] KVM: arm64: pKVM fixes Alexandru Elisei ` (2 preceding siblings ...) 2025-12-15 11:44 ` [PATCH v2 3/4] KVM: arm64: Remove extra argument for __pvkm_host_{share,unshare}_hyp() Alexandru Elisei @ 2025-12-15 11:44 ` Alexandru Elisei 2025-12-15 13:42 ` Fuad Tabba 3 siblings, 1 reply; 10+ messages in thread From: Alexandru Elisei @ 2025-12-15 11:44 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. 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] 10+ messages in thread
* Re: [PATCH v2 4/4] KVM: arm64: Remove unused parameter in synchronize_vcpu_pstate() 2025-12-15 11:44 ` [PATCH v2 4/4] KVM: arm64: Remove unused parameter in synchronize_vcpu_pstate() Alexandru Elisei @ 2025-12-15 13:42 ` Fuad Tabba 0 siblings, 0 replies; 10+ messages in thread From: Fuad Tabba @ 2025-12-15 13:42 UTC (permalink / raw) To: Alexandru Elisei Cc: maz, oupton, joey.gouly, suzuki.poulose, yuzenghui, qperret, will, linux-arm-kernel, kvmarm On Mon, 15 Dec 2025 at 11:44, Alexandru Elisei <alexandru.elisei@arm.com> wrote: > > synchronize_vcpu_pstate() doesn't make use of the reference to exit_code, > remove the parameter. > > Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com> > --- Reviewed-by: Fuad Tabba <tabba@google.com> Cheers, /fuad > 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 [flat|nested] 10+ messages in thread
end of thread, other threads:[~2025-12-15 15:40 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-15 11:44 [PATCH v2 0/4] KVM: arm64: pKVM fixes Alexandru Elisei
2025-12-15 11:44 ` [PATCH v2 1/4] KVM: arm64: Copy FGT traps to unprotected pKVM VCPU on VCPU load Alexandru Elisei
2025-12-15 11:44 ` [PATCH v2 2/4] KVM: arm64: Print register encoding if there's no accessor Alexandru Elisei
2025-12-15 13:58 ` Marc Zyngier
2025-12-15 15:17 ` Alexandru Elisei
2025-12-15 15:39 ` Marc Zyngier
2025-12-15 11:44 ` [PATCH v2 3/4] KVM: arm64: Remove extra argument for __pvkm_host_{share,unshare}_hyp() Alexandru Elisei
2025-12-15 13:43 ` Fuad Tabba
2025-12-15 11:44 ` [PATCH v2 4/4] KVM: arm64: Remove unused parameter in synchronize_vcpu_pstate() Alexandru Elisei
2025-12-15 13:42 ` Fuad Tabba
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).