[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aJOhnHCUR0Af4XJt@linux.dev>
Date: Wed, 6 Aug 2025 11:40:28 -0700
From: Oliver Upton <oliver.upton@...ux.dev>
To: Volodymyr Babchuk <Volodymyr_Babchuk@...m.com>
Cc: "linux-arm-kernel@...ts.infradead.org" <linux-arm-kernel@...ts.infradead.org>,
"kvmarm@...ts.linux.dev" <kvmarm@...ts.linux.dev>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
Marc Zyngier <maz@...nel.org>, Joey Gouly <joey.gouly@....com>,
Suzuki K Poulose <suzuki.poulose@....com>,
Zenghui Yu <yuzenghui@...wei.com>,
Catalin Marinas <catalin.marinas@....com>,
Will Deacon <will@...nel.org>
Subject: Re: [PATCH v1 2/2] KVM: arm64: nv: update CPU register PAR_EL1 after
'at s*'
On Wed, Aug 06, 2025 at 02:17:55PM +0000, Volodymyr Babchuk wrote:
> Previously this code update only vCPU's in-memory value, which is good,
> but not enough, as there will be no context switch after exiting
> exception handler, so in-memory value will not get into actual
> register.
>
> It worked good enough for VHE guests because KVM code tried fast path,
> which of course updated real PAR_EL1.
>
> Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@...m.com>
> ---
> arch/arm64/kvm/sys_regs.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index 7b8a0a6f26468..ab2b5e261d312 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -3463,6 +3463,9 @@ static bool handle_at_s1e2(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
>
> __kvm_at_s1e2(vcpu, op, p->regval);
>
> + /* No context switch happened, so we need to update PAR_EL1 manually */
> + write_sysreg(vcpu_read_sys_reg(vcpu, PAR_EL1), par_el1);
> +
Ok, this had me thoroughly confused for a moment. The bug is actually in
kvm_write_sys_reg() which is supposed to update the sysreg value when
things are loaded on the CPU. __kvm_at_s1e2() is doing the right thing
by calling this accessor.
For registers like PAR_EL1 that don't have an EL2->EL1 mapping we assume
they belong to the EL1 context and thus are in-memory when in a hyp
context. TPIDR(RO)_EL0 is similarly affected.
This is a bit of an ugly hack, but something like the following should
get things working if you're able to test it:
diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index ad2548477257..32f8d1de8f1a 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -149,6 +149,22 @@ static bool get_el2_to_el1_mapping(unsigned int reg,
}
}
+/*
+ * Special-cased registers that do not have an ELx mapping and are always
+ * loaded on the CPU.
+ */
+static bool reg_has_elx_mapping(int reg)
+{
+ switch (reg) {
+ case TPIDR_EL0:
+ case TPIDRRO_EL0:
+ case PAR_EL1:
+ return false;
+ default:
+ return true;
+ }
+}
+
u64 vcpu_read_sys_reg(const struct kvm_vcpu *vcpu, int reg)
{
u64 val = 0x8badf00d8badf00d;
@@ -158,6 +174,9 @@ u64 vcpu_read_sys_reg(const struct kvm_vcpu *vcpu, int reg)
if (!vcpu_get_flag(vcpu, SYSREGS_ON_CPU))
goto memory_read;
+ if (!reg_has_elx_mapping(reg))
+ goto sysreg_read;
+
if (unlikely(get_el2_to_el1_mapping(reg, &el1r, &xlate))) {
if (!is_hyp_ctxt(vcpu))
goto memory_read;
@@ -204,6 +223,7 @@ u64 vcpu_read_sys_reg(const struct kvm_vcpu *vcpu, int reg)
if (unlikely(is_hyp_ctxt(vcpu)))
goto memory_read;
+sysreg_read:
if (__vcpu_read_sys_reg_from_cpu(reg, &val))
return val;
@@ -219,6 +239,9 @@ void vcpu_write_sys_reg(struct kvm_vcpu *vcpu, u64 val, int reg)
if (!vcpu_get_flag(vcpu, SYSREGS_ON_CPU))
goto memory_write;
+ if (!reg_has_elx_mapping(reg))
+ goto sysreg_write;
+
if (unlikely(get_el2_to_el1_mapping(reg, &el1r, &xlate))) {
if (!is_hyp_ctxt(vcpu))
goto memory_write;
@@ -259,6 +282,7 @@ void vcpu_write_sys_reg(struct kvm_vcpu *vcpu, u64 val, int reg)
if (unlikely(is_hyp_ctxt(vcpu)))
goto memory_write;
+sysreg_write:
if (__vcpu_write_sys_reg_to_cpu(val, reg))
return;
--
Thanks,
Oliver
Powered by blists - more mailing lists