Re: [PATCH 13/17] KVM: arm64: Add per-EC entry/exit state marshalling for protected guests

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] The HVC64 entry handler ignores host-requested PC increments and exception injections, causing an infinite trap loop.
- [High] Host-injected exceptions and PC advancements are silently ignored for unhandled Exception Classes, causing infinite trap loops.
- [High] handle_pvm_exit_wfx fails to marshal the WFx timeout register to x0, breaking WFET and WFIT emulation.
--

commit 0b0e245ce37c95597a108d7e1ed3684401ade81b
Author: Fuad Tabba <fuad.tabba@linux.dev>
Subject: KVM: arm64: Add per-EC entry/exit state marshalling for protected guests

This patch moves a protected guest's state between the hyp vCPU and the host
per exception class instead of copying the whole context. It adds per-EC
entry and exit handlers for various traps and routes protected guests
through them to marshal only the necessary state.

> diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-main.c b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> index 5925d35abba8c..461e15cab1b92 100644
> --- a/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> +++ b/arch/arm64/kvm/hyp/nvhe/hyp-main.c

[ ... ]

> -static void __maybe_unused handle_pvm_entry_hvc64(struct pkvm_hyp_vcpu *hyp_vcpu)
> +static void handle_pvm_entry_hvc64(struct pkvm_hyp_vcpu *hyp_vcpu)
> +{
> +	handle_pvm_entry_psci(hyp_vcpu);
> +}

[Severity: High]
Does this code omit handling for host-requested PC increments and exception
injections? 

If the guest executes an HVC instruction, which traps to EL2, and the host
handles the call (e.g., standard PSCI fallback) by setting the INCREMENT_PC
or PENDING_EXCEPTION flags, it appears handle_pvm_entry_hvc64() doesn't
apply them on re-entry. As a result, the guest's PC wouldn't advance and
it could infinitely retry the same HVC.

[ ... ]

> +static void handle_pvm_exit_wfx(struct pkvm_hyp_vcpu *hyp_vcpu)
> +{
> +	hyp_vcpu->host_vcpu->arch.ctxt.regs.pstate =
> +		hyp_vcpu->vcpu.arch.ctxt.regs.pstate & PSR_MODE_MASK;
> +}

[Severity: High]
Can this cause the host to read uninitialized or stale data for WFET and
WFIT timeout values?

When a protected guest traps to EL2 on a WFET/WFIT, pvm_host_esr()
deliberately masks out the RN field (the register index holding the timeout)
from the host's view of the ESR. Because of this, the host's
kvm_handle_wfx() expects the timeout register value to be marshalled into
regs[0]. It looks like handle_pvm_exit_wfx() doesn't copy the guest's RN
register to host_vcpu->arch.ctxt.regs.regs[0] before exiting to the host.

[ ... ]

> @@ -418,10 +638,16 @@ static void flush_hyp_vcpu(struct pkvm_hyp_vcpu *hyp_vcpu)
>  	case ARM_EXCEPTION_IL:
>  		break;
>  	case ARM_EXCEPTION_TRAP:
> -		esr_ec = ESR_ELx_EC(kvm_vcpu_get_esr(&hyp_vcpu->vcpu));
> -		ec_handler = entry_hyp_vm_handlers[esr_ec];
> -		if (ec_handler)
> -			ec_handler(hyp_vcpu);
> +		/* Nothing was marshalled for this trap, see sync_hyp_vcpu(). */
> +		if (ARM_SERROR_PENDING(hyp_vcpu->exit_code))
> +			break;
> +
> +		if (pkvm_hyp_vcpu_is_protected(hyp_vcpu)) {
> +			esr_ec = ESR_ELx_EC(kvm_vcpu_get_esr(&hyp_vcpu->vcpu));
> +			ec_handler = entry_hyp_pvm_handlers[esr_ec];
> +			if (ec_handler)
> +				ec_handler(hyp_vcpu);
> +		}
>  		break;

[Severity: High]
Does this logic silently ignore host-injected exceptions and PC advancements
for unhandled Exception Classes?

If a protected guest executes an unhandled instruction (like SMC64), it
traps to EL2. The host cannot emulate the unknown EC and pends an exception
via the PENDING_EXCEPTION flag. Because there is no specific entry handler
for this EC in entry_hyp_pvm_handlers, ec_handler is NULL and the re-entry
skips entry handling. 

Since the generic fallback was removed, the PENDING_EXCEPTION flag is
ignored, the PC is not advanced, and the guest could hang trying to retry
the failing instruction infinitely.

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831163421.272420-1-fuad.tabba@linux.dev?part=13