Re: [PATCH] KVM: arm64: Preserve AArch32 CP64 registers on rejected reads

[Marc Zyngier <maz@kernel.org>]

On Sat, 01 Aug 2026 16:36:16 +0100,
Karl Mehltretter <kmehltretter@gmail.com> wrote:
> 
> kvm_handle_cp_64() only seeds params.regval for writes. If a CP64 read is
> decoded but rejected, emulate_cp() still returns handled and the caller

s/rejected/UNDEFs/

> writes params.regval back to Rt/Rt2.
> 
> This can happen for write-only GIC SGI registers

Can it? I know we have some code handling this case, but this was for
a (long gone) userspace harness driving this code. On any HW, reading
from WO registers directly results in an UNDEF, without SW
involvement.  Same thing for RO vs writes.

If you know of any case contradicting this statement, please let me
know.

> and for rejected PMU counter reads. In both cases KVM injects an

s/rejected PMU counter reads/read accesses generating an UNDEF/

> UNDEF into the guest, so the MRRC destination registers must remain
> unchanged.

s/destination registers/GPRs/

>
> Instead, the uninitialised regval is copied into the guest registers. With
> stack auto-initialisation this is a deterministic zero or pattern value.
> With CONFIG_INIT_STACK_NONE it may be stale host stack data.
> 
> Match kvm_handle_cp_32() and kvm_handle_sys_reg() by seeding regval from
> the destination registers before emulation.
> 
> Fixes: 6d52f35af10c ("arm64: KVM: add SGI generation register emulation")

This has nothing to do with the handling of a particular register, and
it has *always* been wrong. The correct tag appears to be:

Fixes: 62a89c44954f0 ("arm64: KVM: 32bit handling of coprocessor traps")

> Assisted-by: Claude:claude-opus-5
> Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com>
> ---
> Runtime tested on a Raspberry Pi 400 with a minimal KVM harness running
> an AArch32 guest. On the unpatched 6.1.21-v8+ vendor kernel, the PMCCNTR
> MRRC test took UNDEF with r0=0x00000001/r1=0x00000000 instead of the
> guest's sentinel values. With this patch on v7.2-rc3-278-g38436106b2f5,
> the same test preserved r0=0x12345678/r1=0x9abcdef0.
> 
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -4860,11 +4860,11 @@
>  	/*
>  	 * Make a 64-bit value out of Rt and Rt2. As we use the same trap
>  	 * backends between AArch32 and AArch64, we get away with it.
> +	 *
> +	 * This also makes rejected reads preserve Rt/Rt2.

There is no such thing as "rejected reads" in the architecture (we
only use this word for userspace accesses, not guests). Also, I don't
think this deserves a comment as it aligns the logic with the rest of
the sysreg handling code.

>  	 */
> -	if (params.is_write) {
> -		params.regval = vcpu_get_reg(vcpu, Rt) & 0xffffffff;
> -		params.regval |= vcpu_get_reg(vcpu, Rt2) << 32;
> -	}
> +	params.regval = vcpu_get_reg(vcpu, Rt) & 0xffffffff;
> +	params.regval |= vcpu_get_reg(vcpu, Rt2) << 32;
>  
>  	/*
>  	 * If the table contains a handler, handle the

Other than the nitpicks above, this looks OK to me.

Thanks,

	M.

-- 
Jazz isn't dead. It just smells funny.