Re: [PATCH v3 16/28] KVM: arm64: Split NV-specific exit fixups from the non-NV handling

[Marc Zyngier <maz@kernel.org>]

On Wed, 22 Jul 2026 09:23:03 +0100,
sashiko-bot@kernel.org wrote:
> 
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Removing `inline` from `fixup_guest_exit()` and breaking out `fixup_nv_guest_exit()` exposes them to kprobes, which will cause a fatal host panic if probed.
> --
> 
> --- Patch [16]: [PATCH v3 16/28] KVM: arm64: Split NV-specific exit fixups from the non-NV handling ---
> commit 54c98942c16b88b656ffdb68540aac18e6fe4d3f
> Author: Marc Zyngier <maz@kernel.org>
> KVM: arm64: Split NV-specific exit fixups from the non-NV handling
> 
> This commit moves the nested virtualization (NV) handling of early fixups 
> into its own helper function. This simplifies parsing and prepares the code
> for further changes.
> 
> > diff --git a/arch/arm64/kvm/hyp/vhe/switch.c b/arch/arm64/kvm/hyp/vhe/switch.c
> > index a8da40568a048..53af19212895d 100644
> > --- a/arch/arm64/kvm/hyp/vhe/switch.c
> > +++ b/arch/arm64/kvm/hyp/vhe/switch.c
> > @@ -539,18 +539,15 @@ static const exit_handler_fn hyp_exit_handlers[] = {
> >  	[0x3F]				= kvm_hyp_handle_impdef,
> >  };
> >  
> > -static inline bool fixup_guest_exit(struct kvm_vcpu *vcpu, u64 *exit_code)
> > +static void fixup_nv_guest_exit(struct kvm_vcpu *vcpu)
> 
> [Severity: High]
> Does extracting this function without a NOKPROBE_SYMBOL() annotation expose
> it to kprobes?
> 
> Since this executes on the VM exit path while the KVM hypervisor vectors are
> still loaded, placing a kprobe here would trigger a BRK instruction that traps
> to __kvm_hyp_vector. This cannot handle kprobes and would unconditionally
> call hyp_panic(), crashing the host.
> 
> Should we add NOKPROBE_SYMBOL(fixup_nv_guest_exit) to prevent this?

Maybe, but this is out of scope for this series.

> 
> >  {
> > -	synchronize_vcpu_pstate(vcpu);
> > -
> >  	/*
> >  	 * If we were in HYP context on entry, adjust the PSTATE view
> >  	 * so that the usual helpers work correctly. This enforces our
> >  	 * invariant that the guest's HYP context status is preserved
> >  	 * across a run.
> >  	 */
> > -	if (vcpu_has_nv(vcpu) &&
> > -	    unlikely(host_data_test_flag(VCPU_IN_HYP_CONTEXT))) {
> > +	if (unlikely(host_data_test_flag(VCPU_IN_HYP_CONTEXT))) {
> 
> [ ... ]
> 
> > @@ -567,8 +564,15 @@ static inline bool fixup_guest_exit(struct kvm_vcpu *vcpu, u64 *exit_code)
> >  	}
> >  
> >  	/* Apply extreme paranoia! */
> > -	BUG_ON(vcpu_has_nv(vcpu) &&
> > -	       !!host_data_test_flag(VCPU_IN_HYP_CONTEXT) != is_hyp_ctxt(vcpu));
> > +	BUG_ON(!!host_data_test_flag(VCPU_IN_HYP_CONTEXT) != is_hyp_ctxt(vcpu));
> > +}
> > +
> > +static bool fixup_guest_exit(struct kvm_vcpu *vcpu, u64 *exit_code)
> 
> [Severity: High]
> By removing the inline keyword here, doesn't this also expose
> fixup_guest_exit() to kprobes if the compiler chooses not to inline it?

Marking something inline doesn't guarantee inlining either.

>
> Should this function also receive a NOKPROBE_SYMBOL() annotation to avoid the
> same host panic hazard?

See above.

	M.

-- 
Without deviation from the norm, progress is not possible.