* [PATCH v2] KVM: arm64: Prevent the host from using an smc with imm16 != 0
@ 2026-03-25 11:31 Sebastian Ene
2026-03-25 11:35 ` Vincent Donnefort
2026-03-25 13:28 ` Mark Rutland
0 siblings, 2 replies; 13+ messages in thread
From: Sebastian Ene @ 2026-03-25 11:31 UTC (permalink / raw)
To: kvmarm, linux-arm-kernel, linux-kernel, android-kvm
Cc: catalin.marinas, joey.gouly, mark.rutland, maz, oupton,
sebastianene, suzuki.poulose, tabba, vdonnefort, will, yuzenghui
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;
+ }
+
handle_host_smc(host_ctxt);
break;
case ESR_ELx_EC_IABT_LOW:
--
2.53.0.1018.g2bb0e51243-goog
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v2] KVM: arm64: Prevent the host from using an smc with imm16 != 0 2026-03-25 11:31 [PATCH v2] KVM: arm64: Prevent the host from using an smc with imm16 != 0 Sebastian Ene @ 2026-03-25 11:35 ` Vincent Donnefort 2026-03-25 11:41 ` Sebastian Ene 2026-03-25 11:46 ` Marc Zyngier 2026-03-25 13:28 ` Mark Rutland 1 sibling, 2 replies; 13+ messages in thread From: Vincent Donnefort @ 2026-03-25 11:35 UTC (permalink / raw) To: Sebastian Ene Cc: kvmarm, linux-arm-kernel, linux-kernel, android-kvm, catalin.marinas, joey.gouly, mark.rutland, maz, oupton, suzuki.poulose, tabba, will, yuzenghui 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) > handle_host_smc(host_ctxt); > break; > case ESR_ELx_EC_IABT_LOW: > -- > 2.53.0.1018.g2bb0e51243-goog > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2] KVM: arm64: Prevent the host from using an smc with imm16 != 0 2026-03-25 11:35 ` Vincent Donnefort @ 2026-03-25 11:41 ` Sebastian Ene 2026-03-25 11:58 ` Marc Zyngier 2026-03-25 12:16 ` Fuad Tabba 2026-03-25 11:46 ` Marc Zyngier 1 sibling, 2 replies; 13+ messages in thread From: Sebastian Ene @ 2026-03-25 11:41 UTC (permalink / raw) To: Vincent Donnefort Cc: kvmarm, linux-arm-kernel, linux-kernel, android-kvm, catalin.marinas, joey.gouly, mark.rutland, maz, oupton, suzuki.poulose, tabba, will, yuzenghui 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. > > handle_host_smc(host_ctxt); > > break; > > case ESR_ELx_EC_IABT_LOW: > > -- > > 2.53.0.1018.g2bb0e51243-goog > > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2] KVM: arm64: Prevent the host from using an smc with imm16 != 0 2026-03-25 11:41 ` Sebastian Ene @ 2026-03-25 11:58 ` Marc Zyngier 2026-03-25 12:16 ` Fuad Tabba 1 sibling, 0 replies; 13+ messages in thread From: Marc Zyngier @ 2026-03-25 11:58 UTC (permalink / raw) To: Sebastian Ene Cc: Vincent Donnefort, kvmarm, linux-arm-kernel, linux-kernel, android-kvm, catalin.marinas, joey.gouly, mark.rutland, oupton, suzuki.poulose, tabba, will, yuzenghui On Wed, 25 Mar 2026 11:41:17 +0000, 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. I don't see that obtaining ESR_EL2 is that hard or expensive. Given that everything is in the same compilation unit, the compiler will probably optimise it. But if I have to state the obvious: you are *handling* an SMC instruction. Surely handle_host_smc() is the correct spot for that job? M. -- Without deviation from the norm, progress is not possible. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2] KVM: arm64: Prevent the host from using an smc with imm16 != 0 2026-03-25 11:41 ` Sebastian Ene 2026-03-25 11:58 ` Marc Zyngier @ 2026-03-25 12:16 ` Fuad Tabba 2026-03-25 13:16 ` Sebastian Ene 1 sibling, 1 reply; 13+ messages in thread From: Fuad Tabba @ 2026-03-25 12:16 UTC (permalink / raw) To: Sebastian Ene Cc: Vincent Donnefort, kvmarm, linux-arm-kernel, linux-kernel, android-kvm, catalin.marinas, joey.gouly, mark.rutland, maz, oupton, suzuki.poulose, will, yuzenghui 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? Thanks, /fuad > > > > handle_host_smc(host_ctxt); > > > break; > > > case ESR_ELx_EC_IABT_LOW: > > > -- > > > 2.53.0.1018.g2bb0e51243-goog > > > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2] KVM: arm64: Prevent the host from using an smc with imm16 != 0 2026-03-25 12:16 ` Fuad Tabba @ 2026-03-25 13:16 ` Sebastian Ene 0 siblings, 0 replies; 13+ messages in thread From: Sebastian Ene @ 2026-03-25 13:16 UTC (permalink / raw) To: Fuad Tabba Cc: Vincent Donnefort, kvmarm, linux-arm-kernel, linux-kernel, android-kvm, catalin.marinas, joey.gouly, mark.rutland, maz, oupton, suzuki.poulose, will, yuzenghui 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 > > > > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2] KVM: arm64: Prevent the host from using an smc with imm16 != 0 2026-03-25 11:35 ` Vincent Donnefort 2026-03-25 11:41 ` Sebastian Ene @ 2026-03-25 11:46 ` Marc Zyngier 2026-03-25 12:12 ` Will Deacon 2026-03-25 13:15 ` Sebastian Ene 1 sibling, 2 replies; 13+ messages in thread From: Marc Zyngier @ 2026-03-25 11:46 UTC (permalink / raw) To: Sebastian Ene, Vincent Donnefort Cc: kvmarm, linux-arm-kernel, linux-kernel, android-kvm, catalin.marinas, joey.gouly, mark.rutland, oupton, suzuki.poulose, tabba, will, yuzenghui On Wed, 25 Mar 2026 11:35:18 +0000, Vincent Donnefort <vdonnefort@google.com> 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) Yes, that'd be vastly better. It also begs the question: if you don't want to handle SMCs with a non-zero immediate, why is it OK to do it for HVCs? Thanks, M. -- Without deviation from the norm, progress is not possible. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2] KVM: arm64: Prevent the host from using an smc with imm16 != 0 2026-03-25 11:46 ` Marc Zyngier @ 2026-03-25 12:12 ` Will Deacon 2026-03-25 13:33 ` Mark Rutland 2026-03-25 13:15 ` Sebastian Ene 1 sibling, 1 reply; 13+ messages in thread From: Will Deacon @ 2026-03-25 12:12 UTC (permalink / raw) To: Marc Zyngier Cc: Sebastian Ene, Vincent Donnefort, kvmarm, linux-arm-kernel, linux-kernel, android-kvm, catalin.marinas, joey.gouly, mark.rutland, oupton, suzuki.poulose, tabba, yuzenghui On Wed, Mar 25, 2026 at 11:46:29AM +0000, Marc Zyngier wrote: > On Wed, 25 Mar 2026 11:35:18 +0000, > Vincent Donnefort <vdonnefort@google.com> 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) > > Yes, that'd be vastly better. > > It also begs the question: if you don't want to handle SMCs with a > non-zero immediate, why is it OK to do it for HVCs? I suppose the difference is that the HVC API is a private interface between EL2 and the host. As it stands, EL2 ignores the immediate but we don't have a way to know how EL3 responds to the immediate for an SMC. When proxying an SMC from the host, EL2 therefore has three choices: 1. Ignore the immediate from the host and always use zero when talking to EL3. That's the current behaviour, but it could theoretically lead to problems if EL3 is using the immediate for something. 2. Propagate the immediate from the host. That should work, but it's a bit involved. 3. Reject non-zero immediates (this patch). Will ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2] KVM: arm64: Prevent the host from using an smc with imm16 != 0 2026-03-25 12:12 ` Will Deacon @ 2026-03-25 13:33 ` Mark Rutland 0 siblings, 0 replies; 13+ messages in thread From: Mark Rutland @ 2026-03-25 13:33 UTC (permalink / raw) To: Will Deacon Cc: Marc Zyngier, Sebastian Ene, Vincent Donnefort, kvmarm, linux-arm-kernel, linux-kernel, android-kvm, catalin.marinas, joey.gouly, oupton, suzuki.poulose, tabba, yuzenghui On Wed, Mar 25, 2026 at 12:12:18PM +0000, Will Deacon wrote: > On Wed, Mar 25, 2026 at 11:46:29AM +0000, Marc Zyngier wrote: > > On Wed, 25 Mar 2026 11:35:18 +0000, > > Vincent Donnefort <vdonnefort@google.com> 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. > > It also begs the question: if you don't want to handle SMCs with a > > non-zero immediate, why is it OK to do it for HVCs? > > I suppose the difference is that the HVC API is a private interface > between EL2 and the host. As it stands, EL2 ignores the immediate but we > don't have a way to know how EL3 responds to the immediate for an SMC. > > When proxying an SMC from the host, EL2 therefore has three choices: > > 1. Ignore the immediate from the host and always use zero when talking > to EL3. That's the current behaviour, but it could theoretically > lead to problems if EL3 is using the immediate for something. > > 2. Propagate the immediate from the host. That should work, but it's > a bit involved. > > 3. Reject non-zero immediates (this patch). I think (3) is the only safe option. As described in: https://lore.kernel.org/linux-arm-kernel/acPi5V0DgGcgHNGO@J2N7QTR9R3/ ... regardless of the the conduit (HVS/SMC) used, a non-zero immediate is NOT a legitimate SMCCC call. (1) is not safe, becuase it could change the semantic of the call, and (2) is not safe because the calling convention could differ (and e.g. things might be clobbered that we don't expect). We know that the host *SHOULD* use a zero immediate, and so anything else represents a bug that we will want to catch. Mark. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2] KVM: arm64: Prevent the host from using an smc with imm16 != 0 2026-03-25 11:46 ` Marc Zyngier 2026-03-25 12:12 ` Will Deacon @ 2026-03-25 13:15 ` Sebastian Ene 1 sibling, 0 replies; 13+ messages in thread From: Sebastian Ene @ 2026-03-25 13:15 UTC (permalink / raw) To: Marc Zyngier Cc: Vincent Donnefort, kvmarm, linux-arm-kernel, linux-kernel, android-kvm, catalin.marinas, joey.gouly, mark.rutland, oupton, suzuki.poulose, tabba, will, yuzenghui On Wed, Mar 25, 2026 at 11:46:29AM +0000, Marc Zyngier wrote: > On Wed, 25 Mar 2026 11:35:18 +0000, > Vincent Donnefort <vdonnefort@google.com> 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) > > Yes, that'd be vastly better. > good, I will update the patch to do this. > It also begs the question: if you don't want to handle SMCs with a > non-zero immediate, why is it OK to do it for HVCs? I talked a bit with Will about this before writing it. My understanding is that we don't have to do it for HVCs because the interface with the hypervisor is controlled by us whereas with non-standard SMCs we need at least to tell the host that we are not handling non-zero imm16. > > Thanks, > > M. > > -- > Without deviation from the norm, progress is not possible. Thanks, Sebastian ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2] KVM: arm64: Prevent the host from using an smc with imm16 != 0 2026-03-25 11:31 [PATCH v2] KVM: arm64: Prevent the host from using an smc with imm16 != 0 Sebastian Ene 2026-03-25 11:35 ` Vincent Donnefort @ 2026-03-25 13:28 ` Mark Rutland 2026-03-25 14:33 ` Marc Zyngier 2026-03-25 16:02 ` Sebastian Ene 1 sibling, 2 replies; 13+ messages in thread From: Mark Rutland @ 2026-03-25 13:28 UTC (permalink / raw) To: Sebastian Ene Cc: kvmarm, linux-arm-kernel, linux-kernel, android-kvm, catalin.marinas, joey.gouly, maz, oupton, suzuki.poulose, tabba, vdonnefort, will, yuzenghui 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. 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. > 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; > + } > + > handle_host_smc(host_ctxt); > break; > case ESR_ELx_EC_IABT_LOW: > -- > 2.53.0.1018.g2bb0e51243-goog > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2] KVM: arm64: Prevent the host from using an smc with imm16 != 0 2026-03-25 13:28 ` Mark Rutland @ 2026-03-25 14:33 ` Marc Zyngier 2026-03-25 16:02 ` Sebastian Ene 1 sibling, 0 replies; 13+ messages in thread From: Marc Zyngier @ 2026-03-25 14:33 UTC (permalink / raw) To: Mark Rutland Cc: Sebastian Ene, kvmarm, linux-arm-kernel, linux-kernel, android-kvm, catalin.marinas, joey.gouly, oupton, suzuki.poulose, tabba, vdonnefort, will, yuzenghui On Wed, 25 Mar 2026 13:28:05 +0000, Mark Rutland <mark.rutland@arm.com> 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. > > 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: I disagree. If SMC can be handled at all, then it cannot UNDEF based on the immediate -- the is no provision for that in the architecture. If it can UNDEF, then it must UNDEF always (as if SCR_EL3.SMD == 1). Thanks, M. -- Without deviation from the norm, progress is not possible. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2] KVM: arm64: Prevent the host from using an smc with imm16 != 0 2026-03-25 13:28 ` Mark Rutland 2026-03-25 14:33 ` Marc Zyngier @ 2026-03-25 16:02 ` Sebastian Ene 1 sibling, 0 replies; 13+ messages in thread From: Sebastian Ene @ 2026-03-25 16:02 UTC (permalink / raw) To: Mark Rutland Cc: kvmarm, linux-arm-kernel, linux-kernel, android-kvm, catalin.marinas, joey.gouly, maz, oupton, suzuki.poulose, tabba, vdonnefort, will, yuzenghui 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@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; > > + } > > + > > handle_host_smc(host_ctxt); > > break; > > case ESR_ELx_EC_IABT_LOW: > > -- > > 2.53.0.1018.g2bb0e51243-goog > > ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2026-03-25 16:02 UTC | newest] Thread overview: 13+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-03-25 11:31 [PATCH v2] KVM: arm64: Prevent the host from using an smc with imm16 != 0 Sebastian Ene 2026-03-25 11:35 ` Vincent Donnefort 2026-03-25 11:41 ` Sebastian Ene 2026-03-25 11:58 ` Marc Zyngier 2026-03-25 12:16 ` Fuad Tabba 2026-03-25 13:16 ` Sebastian Ene 2026-03-25 11:46 ` Marc Zyngier 2026-03-25 12:12 ` Will Deacon 2026-03-25 13:33 ` Mark Rutland 2026-03-25 13:15 ` Sebastian Ene 2026-03-25 13:28 ` Mark Rutland 2026-03-25 14:33 ` Marc Zyngier 2026-03-25 16:02 ` Sebastian Ene
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox