Re: [PATCH v2] KVM: arm64: Prevent the host from using an smc with imm16 != 0

[Sebastian Ene]

On Wed, Mar 25, 2026 at 01:28:05PM +0000, Mark Rutland wrote:
> On Wed, Mar 25, 2026 at 11:31:38AM +0000, Sebastian Ene wrote:
> > The ARM Service Calling Convention (SMCCC) specifies that the function
> > identifier and parameters should be passed in registers, leaving the
> > 16-bit immediate field of the SMC instruction un-handled.
> 
> That's not quite right; the SMCCC spec says callers must use immediate
> 0.

I'll update the commit message. it is the pKVM that doesn't handle it
(the imm16).

> 
> See https://developer.arm.com/documentation/den0028/h/ section 2.10
> ("SME and HVC immediate value"), which says:
> 
> | • For all compliant calls, an SMC or HVC immediate value of zero must be
> |   used.
> | • Nonzero immediate values in SMC instructions are reserved.
> | • Nonzero immediate values in HVC instructions are designated for use by
> |   hypervisor vendors.
> 
> > Currently, our pKVM handler ignores the immediate value, which could lead
> > to non-compliant software relying on implementation-defined behavior.
> > Enforce the host kernel running under pKVM to use an immediate value
> > of 0 by decoding the ISS from the ESR_EL2 and return a not supported
> > error code back to the caller.
> 
> From my PoV, it'd be fine to turn a non-zero immediate into an UNDEF:
> 
> * For HVC, we can say any non-zero immediate represents a request to KVM to
>   inject an UNDEF.
> 
> * For SMC, the dbehaviour is not defined.
> 
> I am also ok with returning a value in x0, BUT that's stronger than SMCCC
> actually requires.
>
> Mark.

Thanks,
Sebastian

> 
> > Signed-off-by: Sebastian Ene <sebastianene@xxxxxxxxxx>
> > ---
> > v1 -> v2:
> > 
> >  - Dropped injecting an UNDEF and return an error instead
> >    (SMCCC_RET_NOT_SUPPORTED)
> >  - Used the mask ESR_ELx_xVC_IMM_MASK instead of masking with U16_MAX
> >  - Updated the title of the commit message from:
> >    "[PATCH] KVM: arm64: Inject UNDEF when host is executing an
> >     smc with imm16 != 0"
> > ---
> >  arch/arm64/kvm/hyp/nvhe/hyp-main.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-main.c b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> > index e7790097db93..4ffe30fd8707 100644
> > --- a/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> > +++ b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> > @@ -762,6 +762,12 @@ void handle_trap(struct kvm_cpu_context *host_ctxt)
> >  		handle_host_hcall(host_ctxt);
> >  		break;
> >  	case ESR_ELx_EC_SMC64:
> > +		if (ESR_ELx_xVC_IMM_MASK & esr) {
> > +			cpu_reg(host_ctxt, 0) = SMCCC_RET_NOT_SUPPORTED;
> > +			kvm_skip_host_instr();
> > +			break;
> > +		}
> > +
> >  		handle_host_smc(host_ctxt);
> >  		break;
> >  	case ESR_ELx_EC_IABT_LOW:
> > -- 
> > 2.53.0.1018.g2bb0e51243-goog
> >