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

[Sebastian Ene <sebastianene@google.com>]

On Wed, Mar 25, 2026 at 12:16:17PM +0000, Fuad Tabba wrote:
> On Wed, 25 Mar 2026 at 11:41, Sebastian Ene <sebastianene@google.com> wrote:
> >
> > On Wed, Mar 25, 2026 at 11:35:18AM +0000, Vincent Donnefort 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.
> > > > 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.
> > > >
> > > > Signed-off-by: Sebastian Ene <sebastianene@google.com>
> > > > ---
> > > > 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;
> > > > +           }
> > > > +
> > >
> > > I wonder if it isn't better to move that into handle_host_smc() as this is part
> > > of how we handle the SMC after all? (and it calls that kvm_skip_host_instr()
> > > already)
> > >
> >
> > I was thinking of doing that as well but I prefer this since we don't
> > have to look again at the esr in the callee.
> 
> This is a non-trivial amount of code to have in a switch. Can you
> please put it in a helper?


Hey Fuad,

I will move it as part of the handle_host_smc(..) as suggested by other
comment.

> 
> Thanks,
> /fuad
> 

Thanks,
Sebastian

> >
> > > >             handle_host_smc(host_ctxt);
> > > >             break;
> > > >     case ESR_ELx_EC_IABT_LOW:
> > > > --
> > > > 2.53.0.1018.g2bb0e51243-goog
> > > >