Re: [PATCH 22/28] KVM: arm64: Add routing for NVHCR_EL2 trap

[Marc Zyngier <maz@kernel.org>]

On Thu, 02 Jul 2026 17:26:07 +0100,
sashiko-bot@kernel.org wrote:
> 
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] The trap routing condition for `NVHCR_EL2` incorrectly uses a logical OR (`MCB`) instead of a logical AND, causing improper trap injection to guest EL2.
> --
> 
> commit 6d634fa5ff4e9b082f4e46e999fa4990ecf38957
> Author: Marc Zyngier <maz@kernel.org>
> 
> KVM: arm64: Add routing for NVHCR_EL2 trap
>     
> NVHCR_EL2 accesses from EL1 are taken to EL2 when HCRX_EL2.NVTGE==0
> and HCR_EL2.NV==1. Describe this in the exception routing tables.
> 
> > diff --git a/arch/arm64/kvm/emulate-nested.c b/arch/arm64/kvm/emulate-nested.c
> > index b7f3d86a94031..f5dc578d8c985 100644
> > --- a/arch/arm64/kvm/emulate-nested.c
> > +++ b/arch/arm64/kvm/emulate-nested.c
> [ ... ]
> > @@ -468,6 +477,7 @@ static const enum cgt_group_id *coarse_control_combo[] = {
> >  					CGT_HCR_TVM, CGT_HCR_TRVM, CGT_HCRX_SCTLR2En),
> >  	MCB(CGT_HCR_TPU_TICAB,		CGT_HCR_TPU, CGT_HCR_TICAB),
> >  	MCB(CGT_HCR_TPU_TOCU,		CGT_HCR_TPU, CGT_HCR_TOCU),
> > +	MCB(CGT_HCR_NV_HCRX_nNVTGE,	CGT_HCR_NV, CGT_HCRX_nNVTGE),
> 
> [Severity: High]
> Does this correctly evaluate the trap condition as an AND?
> 
> The commit message indicates the trap is taken when "HCRX_EL2.NVTGE==0 and
> HCR_EL2.NV==1". However, coarse_control_combo[] uses the MCB() macro, which
> evaluates multiple conditions using a bitwise OR:
> 
> arch/arm64/kvm/emulate-nested.c:__compute_trap_behaviour() {
> 	...
> 		for (int i = 0; cgids[i] != __RESERVED__; i++)
> 			b |= __compute_trap_behaviour(vcpu, cgids[i], b);
> 	...
> }
> 
> Because this ORs the behaviors, the trap will be forwarded if HCR_EL2.NV == 1
> OR HCRX_EL2.NVTGE == 0 evaluates to forward. This would erroneously send the
> trap to EL2 even if normal execution without nested virt is active (NV == 0
> and NVTGE == 0).
> 
> Should this use a __COMPLEX_CONDITIONS__ callback instead to enforce the
> logical AND?

Grmbl. Yes, well spotted.

	M.

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