[PATCH 0/7] KVM: arm64: Fix missing ESR_ELx.IL in syndrome injection

[PATCH 0/7] KVM: arm64: Fix missing ESR_ELx.IL in syndrome injection

[Fuad Tabba]

Hi folks,

After sashiko caught the missing IL bug [1], I did an audit of all ESR
syndrome construction sites in KVM/arm64 as Marc suggested. This series
is the result of that audit.

The ARM architecture mandates ESR_ELx.IL=1 for several exception
classes regardless of instruction length: EC=Unknown, Instruction
Aborts, Data Aborts with ISV=0, and SError. For FPAC (EC=0x1C), IL
reflects instruction length, but FPAC can only be generated by A64
instructions, so IL must also be 1.

Patch 1 is the bug sashiko found: inject_undef64() in the pKVM hyp (EL2)
path never set IL.
Patch 2 makes the same fix to inject_undef64() in the normal host path,
where IL was derived from the triggering trap's instruction length. No
instruction that reaches undef injection has a 16-bit encoding, so patch
2 has no functional change today.
Patch 3 makes the matching fix to inject_abt64(). Unlike undef injection,
abort injection is reachable from a 16-bit T32 instruction (a 32-bit EL0
task under an AArch64 EL1 guest), so the old code there injects an abort
with IL=0.
Patch 4 fixes the FPAC syndrome constructed during nested ERET
emulation, which did not set IL.
Patches 5-6 fix SError injection in the emulated and nested paths,
neither of which set IL.
Patch 7 fixes a fake ESR used to exit to the host. The host does not
read IL there, so it is not guest-visible.

Based on Linux 7.1-rc7

Cheers,
/fuad

[1] https://lore.kernel.org/all/87pl1t8q24.wl-maz@kernel.org/

Fuad Tabba (7):
  KVM: arm64: Set ESR_ELx.IL for injected undefined exceptions at EL2
  KVM: arm64: Unconditionally set IL for injected undefined exceptions
  KVM: arm64: Unconditionally set IL for injected abort exceptions
  KVM: arm64: Set IL for injected FPAC exceptions during ERET emulation
  KVM: arm64: Set IL for emulated SError injection
  KVM: arm64: Set IL for nested SError injection
  KVM: arm64: Set IL in fake ESR for pKVM memory sharing exit

 arch/arm64/kvm/emulate-nested.c    |  4 ++--
 arch/arm64/kvm/hyp/nvhe/pkvm.c     |  3 ++-
 arch/arm64/kvm/hyp/nvhe/sys_regs.c |  2 +-
 arch/arm64/kvm/inject_fault.c      | 18 +++++-------------
 4 files changed, 10 insertions(+), 17 deletions(-)

-- 
2.54.0.1136.gdb2ca164c4-goog

[PATCH 1/7] KVM: arm64: Set ESR_ELx.IL for injected undefined exceptions at EL2

[Fuad Tabba]

inject_undef64() constructs an ESR with EC=0 (Unknown) but does not set
IL. The architecture mandates IL=1 for EC=0 unconditionally (ARM DDI
0487, ESR_ELx.IL description), so the injected syndrome is one that
conforming hardware cannot produce.

Set ESR_ELx_IL in the constructed syndrome.

Fixes: e5d40a5a97c1 ("KVM: arm64: pkvm: Add a generic synchronous exception injection primitive")
Reported-by: sashiko <sashiko@sashiko.dev>
Signed-off-by: Fuad Tabba <tabba@google.com>
---
 arch/arm64/kvm/hyp/nvhe/sys_regs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/kvm/hyp/nvhe/sys_regs.c b/arch/arm64/kvm/hyp/nvhe/sys_regs.c
index 8c3fbb413a06..9767adf1f73e 100644
--- a/arch/arm64/kvm/hyp/nvhe/sys_regs.c
+++ b/arch/arm64/kvm/hyp/nvhe/sys_regs.c
@@ -278,7 +278,7 @@ static void inject_sync64(struct kvm_vcpu *vcpu, u64 esr)
  */
 static void inject_undef64(struct kvm_vcpu *vcpu)
 {
-	inject_sync64(vcpu, (ESR_ELx_EC_UNKNOWN << ESR_ELx_EC_SHIFT));
+	inject_sync64(vcpu, (ESR_ELx_EC_UNKNOWN << ESR_ELx_EC_SHIFT) | ESR_ELx_IL);
 }
 
 static u64 read_id_reg(const struct kvm_vcpu *vcpu,
-- 
2.54.0.1136.gdb2ca164c4-goog

[PATCH 2/7] KVM: arm64: Unconditionally set IL for injected undefined exceptions

[Fuad Tabba]

inject_undef64() derives IL from the triggering trap's instruction
length (kvm_vcpu_trap_il_is32bit()), but the IL of the injected
exception is fixed by its EC, not by the triggering instruction. The
architecture mandates IL=1 for EC=0 (Unknown) unconditionally, so the
conditional is wrong. The undef-injection paths are not reached from
16-bit instructions, so there is no functional change today, but the
logic should not rely on that.

Set ESR_ELx_IL unconditionally.

Fixes: aa8eff9bfbd5 ("arm64: KVM: fault injection into a guest")
Signed-off-by: Fuad Tabba <tabba@google.com>
---
 arch/arm64/kvm/inject_fault.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/arch/arm64/kvm/inject_fault.c b/arch/arm64/kvm/inject_fault.c
index 89982bd3345f..9dfae1bcdf99 100644
--- a/arch/arm64/kvm/inject_fault.c
+++ b/arch/arm64/kvm/inject_fault.c
@@ -170,14 +170,7 @@ void kvm_inject_sync(struct kvm_vcpu *vcpu, u64 esr)
 
 static void inject_undef64(struct kvm_vcpu *vcpu)
 {
-	u64 esr = (ESR_ELx_EC_UNKNOWN << ESR_ELx_EC_SHIFT);
-
-	/*
-	 * Build an unknown exception, depending on the instruction
-	 * set.
-	 */
-	if (kvm_vcpu_trap_il_is32bit(vcpu))
-		esr |= ESR_ELx_IL;
+	u64 esr = (ESR_ELx_EC_UNKNOWN << ESR_ELx_EC_SHIFT) | ESR_ELx_IL;
 
 	kvm_inject_sync(vcpu, esr);
 }
-- 
2.54.0.1136.gdb2ca164c4-goog

[PATCH 3/7] KVM: arm64: Unconditionally set IL for injected abort exceptions

[Fuad Tabba]

inject_abt64() derives IL from the triggering trap's instruction length
(kvm_vcpu_trap_il_is32bit()), but the IL of the injected abort is fixed
by its EC, not by the triggering instruction. The architecture mandates
IL=1 for Instruction Aborts unconditionally and for Data Aborts with
ISV=0, and this function never sets ISV (the FSC is always EXTABT or
SEA_TTW). For a 16-bit T32 trap (a 32-bit EL0 task under an AArch64 EL1
guest) the trap has IL=0, so the abort is injected with the wrong IL.

Set ESR_ELx_IL unconditionally.

Fixes: aa8eff9bfbd5 ("arm64: KVM: fault injection into a guest")
Signed-off-by: Fuad Tabba <tabba@google.com>
---
 arch/arm64/kvm/inject_fault.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/kvm/inject_fault.c b/arch/arm64/kvm/inject_fault.c
index 9dfae1bcdf99..444d219b0217 100644
--- a/arch/arm64/kvm/inject_fault.c
+++ b/arch/arm64/kvm/inject_fault.c
@@ -138,11 +138,10 @@ static void inject_abt64(struct kvm_vcpu *vcpu, bool is_iabt, unsigned long addr
 		pend_sync_exception(vcpu);
 
 	/*
-	 * Build an {i,d}abort, depending on the level and the
-	 * instruction set. Report an external synchronous abort.
+	 * Build an {i,d}abort, depending on the level.
+	 * Report an external synchronous abort.
 	 */
-	if (kvm_vcpu_trap_il_is32bit(vcpu))
-		esr |= ESR_ELx_IL;
+	esr |= ESR_ELx_IL;
 
 	/*
 	 * Here, the guest runs in AArch64 mode when in EL1. If we get
-- 
2.54.0.1136.gdb2ca164c4-goog

[PATCH 4/7] KVM: arm64: Set IL for injected FPAC exceptions during ERET emulation

[Fuad Tabba]

The FPAC syndrome constructed during nested ERET emulation does not set
IL. For FPAC (EC=0x1C), IL reflects the instruction length. ERET and
its authenticated variants are always A64 32-bit instructions, so IL
must be 1.

Fixes: 213b3d1ea161 ("KVM: arm64: nv: Handle ERETA[AB] instructions")
Signed-off-by: Fuad Tabba <tabba@google.com>
---
 arch/arm64/kvm/emulate-nested.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/kvm/emulate-nested.c b/arch/arm64/kvm/emulate-nested.c
index dba7ced74ca5..4b39363cf891 100644
--- a/arch/arm64/kvm/emulate-nested.c
+++ b/arch/arm64/kvm/emulate-nested.c
@@ -2777,7 +2777,7 @@ void kvm_emulate_nested_eret(struct kvm_vcpu *vcpu)
 		 */
 		if (kvm_has_pauth(vcpu->kvm, FPACCOMBINE) && !(spsr & PSR_IL_BIT)) {
 			esr &= ESR_ELx_ERET_ISS_ERETA;
-			esr |= FIELD_PREP(ESR_ELx_EC_MASK, ESR_ELx_EC_FPAC);
+			esr |= FIELD_PREP(ESR_ELx_EC_MASK, ESR_ELx_EC_FPAC) | ESR_ELx_IL;
 			kvm_inject_nested_sync(vcpu, esr);
 			return;
 		}
-- 
2.54.0.1136.gdb2ca164c4-goog

[PATCH 5/7] KVM: arm64: Set IL for emulated SError injection

[Fuad Tabba]

kvm_inject_serror_esr() constructs an SError syndrome without IL. The
architecture mandates IL=1 for SError unconditionally.

Fixes: f6e2262dfa1a ("KVM: arm64: Populate ESR_ELx.EC for emulated SError injection")
Signed-off-by: Fuad Tabba <tabba@google.com>
---
 arch/arm64/kvm/inject_fault.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/kvm/inject_fault.c b/arch/arm64/kvm/inject_fault.c
index 444d219b0217..d6c4fc16f879 100644
--- a/arch/arm64/kvm/inject_fault.c
+++ b/arch/arm64/kvm/inject_fault.c
@@ -381,7 +381,7 @@ int kvm_inject_serror_esr(struct kvm_vcpu *vcpu, u64 esr)
 	 */
 	if (!serror_is_masked(vcpu)) {
 		pend_serror_exception(vcpu);
-		esr |= FIELD_PREP(ESR_ELx_EC_MASK, ESR_ELx_EC_SERROR);
+		esr |= FIELD_PREP(ESR_ELx_EC_MASK, ESR_ELx_EC_SERROR) | ESR_ELx_IL;
 		vcpu_write_sys_reg(vcpu, esr, exception_esr_elx(vcpu));
 		return 1;
 	}
-- 
2.54.0.1136.gdb2ca164c4-goog

[PATCH 6/7] KVM: arm64: Set IL for nested SError injection

[Fuad Tabba]

kvm_inject_nested_serror() constructs an SError syndrome without IL.
The architecture mandates IL=1 for SError unconditionally.

Fixes: 77ee70a07357 ("KVM: arm64: nv: Honor SError exception routing / masking")
Signed-off-by: Fuad Tabba <tabba@google.com>
---
 arch/arm64/kvm/emulate-nested.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/kvm/emulate-nested.c b/arch/arm64/kvm/emulate-nested.c
index 4b39363cf891..4262d4c17a87 100644
--- a/arch/arm64/kvm/emulate-nested.c
+++ b/arch/arm64/kvm/emulate-nested.c
@@ -2938,6 +2938,6 @@ int kvm_inject_nested_serror(struct kvm_vcpu *vcpu, u64 esr)
 	 * vSError injection. Manually populate EC for an emulated SError
 	 * exception.
 	 */
-	esr |= FIELD_PREP(ESR_ELx_EC_MASK, ESR_ELx_EC_SERROR);
+	esr |= FIELD_PREP(ESR_ELx_EC_MASK, ESR_ELx_EC_SERROR) | ESR_ELx_IL;
 	return kvm_inject_nested(vcpu, esr, except_type_serror);
 }
-- 
2.54.0.1136.gdb2ca164c4-goog

[PATCH 7/7] KVM: arm64: Set IL in fake ESR for pKVM memory sharing exit

[Fuad Tabba]

__pkvm_memshare_page_req() constructs a fake DABT ESR_EL2 to exit to
the host without setting IL. The ESR has ISV=0, so IL must be 1 per the
architecture. The host does not read IL on this path, but the
constructed syndrome should still be architecturally valid.

Set ESR_ELx_IL.

Fixes: 03313efed5e2 ("KVM: arm64: Implement the MEM_SHARE hypercall for protected VMs")
Signed-off-by: Fuad Tabba <tabba@google.com>
---
 arch/arm64/kvm/hyp/nvhe/pkvm.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/kvm/hyp/nvhe/pkvm.c b/arch/arm64/kvm/hyp/nvhe/pkvm.c
index eb1c10120f9f..c982a3a04c37 100644
--- a/arch/arm64/kvm/hyp/nvhe/pkvm.c
+++ b/arch/arm64/kvm/hyp/nvhe/pkvm.c
@@ -1054,7 +1054,8 @@ static u64 __pkvm_memshare_page_req(struct kvm_vcpu *vcpu, u64 ipa)
 
 	/* Fake up a data abort (level 3 translation fault on write) */
 	vcpu->arch.fault.esr_el2 = (ESR_ELx_EC_DABT_LOW << ESR_ELx_EC_SHIFT) |
-				   ESR_ELx_WNR | ESR_ELx_FSC_FAULT |
+				   ESR_ELx_IL | ESR_ELx_WNR |
+				   ESR_ELx_FSC_FAULT |
 				   FIELD_PREP(ESR_ELx_FSC_LEVEL, 3);
 
 	/* Shuffle the IPA around into the HPFAR */
-- 
2.54.0.1136.gdb2ca164c4-goog

Re: [PATCH 6/7] KVM: arm64: Set IL for nested SError injection

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] The constructed SError syndrome (ESR) is silently dropped during nested EL2 exception injection because `ESR_EL2` is never updated.
- [High] When SCTLR2_EL1_EASE routes a Synchronous External abort to an SError, the ESR injected has the wrong EC (IABT/DABT instead of SERROR) and an incorrect ISS format.
--

commit 09fd06d56ab3236a0e7af03e0d026ed6725ad6d6
Author: Fuad Tabba <tabba@google.com>

KVM: arm64: Set IL for nested SError injection

This patch modifies the emulated SError injection to unconditionally
set the IL (Instruction Length) bit in the generated ESR, as mandated
by the architecture.

> diff --git a/arch/arm64/kvm/emulate-nested.c b/arch/arm64/kvm/emulate-nested.c
> index 4b39363cf8911..4262d4c17a877 100644
> --- a/arch/arm64/kvm/emulate-nested.c
> +++ b/arch/arm64/kvm/emulate-nested.c
> @@ -2938,6 +2938,6 @@ int kvm_inject_nested_serror(struct kvm_vcpu *vcpu, u64 esr)
>  	 * vSError injection. Manually populate EC for an emulated SError
>  	 * exception.
>  	 */
> -	esr |= FIELD_PREP(ESR_ELx_EC_MASK, ESR_ELx_EC_SERROR);
> +	esr |= FIELD_PREP(ESR_ELx_EC_MASK, ESR_ELx_EC_SERROR) | ESR_ELx_IL;
>  	return kvm_inject_nested(vcpu, esr, except_type_serror);

[Severity: High]
This isn't a bug introduced by this patch, but does the carefully constructed
esr value actually get written to the guest's ESR_EL2?

Looking at kvm_inject_nested(), it passes the esr_el2 argument down:

kvm_inject_nested(vcpu, esr_el2, type) {
    ...
    kvm_inject_el2_exception(vcpu, esr_el2, type);
    ...
}

However, in kvm_inject_el2_exception():

static void kvm_inject_el2_exception(...) {
    ...
    switch (type) {
    ...
    case except_type_serror:
        kvm_pend_exception(vcpu, EXCEPT_AA64_EL2_SERR);
        break;
    ...
}

Since except_type_serror only pends the exception and doesn't explicitly
write the esr_el2 argument into ESR_EL2 (unlike except_type_sync), does the
nested hypervisor observe a stale or uninitialized syndrome when it takes
the SError?

[Severity: High]
This is a pre-existing issue, but since this code deals with fixing SError
syndromes, what happens when SCTLR2_ELx.EASE is enabled and a Synchronous
External abort is routed to an SError?

Looking at kvm_inject_nested_sea() in the same file:

int kvm_inject_nested_sea(struct kvm_vcpu *vcpu) {
    ...
    u64 esr = FIELD_PREP(ESR_ELx_EC_MASK,
                         iabt ? ESR_ELx_EC_IABT_LOW : ESR_ELx_EC_DABT_LOW);
    ...
    if (__vcpu_sys_reg(vcpu, SCTLR2_EL2) & SCTLR2_EL1_EASE)
        return kvm_inject_nested(vcpu, esr, except_type_serror);
    ...
}

Does this result in an SError being injected with a Data/Instruction Abort
EC? According to the ARM Architecture Reference Manual (D8.2.5), shouldn't
an SError always have its ESR_ELx.EC field set to 0b101111 (SError)?

A similar pattern appears in inject_abt64() for EL1, where a DABT/IABT
syndrome is constructed and actively written to exception_esr_elx(vcpu)
even though an SError exception was pended. Would this incorrect EC format
confuse the guest's exception handlers?

>  }
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260614163336.3490925-1-tabba@google.com?part=6

Re: [PATCH 4/7] KVM: arm64: Set IL for injected FPAC exceptions during ERET emulation

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Incorrect Emulation of Illegal Exception Return corrupts ELR_EL2 and may cause spurious Instruction Aborts
--

commit 56bc216de713f5bf0e45ff5ad97580cbdc7eeb66
Author: Fuad Tabba <tabba@google.com>

KVM: arm64: Set IL for injected FPAC exceptions during ERET emulation

This commit fixes the FPAC syndrome constructed during nested ERET
emulation to include the IL bit. It ensures that the IL bit is set to 1
for ERET and its authenticated variants, which are always 32-bit A64
instructions.

> diff --git a/arch/arm64/kvm/emulate-nested.c b/arch/arm64/kvm/emulate-nested.c
> --- a/arch/arm64/kvm/emulate-nested.c
> +++ b/arch/arm64/kvm/emulate-nested.c
> @@ -2777,7 +2777,7 @@ void kvm_emulate_nested_eret(struct kvm_vcpu *vcpu)
>  		 */
>  		if (kvm_has_pauth(vcpu->kvm, FPACCOMBINE) && !(spsr & PSR_IL_BIT)) {
>  			esr &= ESR_ELx_ERET_ISS_ERETA;
> -			esr |= FIELD_PREP(ESR_ELx_EC_MASK, ESR_ELx_EC_FPAC);
> +			esr |= FIELD_PREP(ESR_ELx_EC_MASK, ESR_ELx_EC_FPAC) | ESR_ELx_IL;
>  			kvm_inject_nested_sync(vcpu, esr);
>  			return;
>  		}
>  	}
>  
>  	preempt_disable();
>  	vcpu_set_flag(vcpu, IN_NESTED_ERET);
>  	kvm_arch_vcpu_put(vcpu);
>  
>  	if (!esr_iss_is_eretax(esr))
>  		elr = __vcpu_sys_reg(vcpu, ELR_EL2);
>  
>  	trace_kvm_nested_eret(vcpu, elr, spsr);
>  
>  	*vcpu_pc(vcpu) = elr;
>  	*vcpu_cpsr(vcpu) = spsr;

[Severity: High]
This is a pre-existing issue, but does setting the PC to the target address
here cause issues when an illegal exception return is detected?

If kvm_check_illegal_exception_return() previously detected an illegal return
and set PSR_IL_BIT in spsr, kvm_emulate_nested_eret() unconditionally
advances the guest PC to the ERET target address (elr) and returns to the
guest, relying on hardware to generate the Illegal Execution State exception.

Wouldn't the hardware then save the target address into the guest's ELR_EL2
instead of the faulting ERET address, corrupting the architectural state?

Additionally, since the hardware fetches the instruction at the target address
using the guest EL2 translation regime before execution, an unmapped or
unexecutable target address could trigger an Instruction Abort instead of the
required Illegal Execution State exception, per ARM ARM D1.8.2.

Could this result in the guest hypervisor receiving a spurious Instruction
Abort for an EL1/EL0 address?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260614163336.3490925-1-tabba@google.com?part=4

Re: [PATCH 5/7] KVM: arm64: Set IL for emulated SError injection

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The patch series attempts to fix SError syndrome IL bits, but the constructed ESR is silently dropped when injecting a nested SError because `kvm_inject_el2_exception()` ignores the `esr_el2` argument.
--

commit 3adb0abe8b63984396e6d2ad4fb963bab5512c79
Author: Fuad Tabba <tabba@google.com>
Subject: KVM: arm64: Set IL for emulated SError injection

This commit updates kvm_inject_serror_esr to include the IL bit when
constructing an SError syndrome. It fixes an architectural compliance issue
where IL must be 1 for SErrors unconditionally.

> diff --git a/arch/arm64/kvm/inject_fault.c b/arch/arm64/kvm/inject_fault.c
> index 444d219b02174..d6c4fc16f8795 100644
> --- a/arch/arm64/kvm/inject_fault.c
> +++ b/arch/arm64/kvm/inject_fault.c
> @@ -381,7 +381,7 @@ int kvm_inject_serror_esr(struct kvm_vcpu *vcpu, u64 esr)
>  	 */
>  	if (!serror_is_masked(vcpu)) {
>  		pend_serror_exception(vcpu);
> -		esr |= FIELD_PREP(ESR_ELx_EC_MASK, ESR_ELx_EC_SERROR);
> +		esr |= FIELD_PREP(ESR_ELx_EC_MASK, ESR_ELx_EC_SERROR) | ESR_ELx_IL;

[Severity: High]
This is a pre-existing issue, but while we are ensuring the SError syndrome
is correctly populated here, does the nested SError injection path drop the
syndrome entirely?

When injecting a nested SError, kvm_inject_nested_serror() passes the
constructed syndrome down to kvm_inject_el2_exception(). However, looking at
kvm_inject_el2_exception() in arch/arm64/kvm/emulate-nested.c:

arch/arm64/kvm/emulate-nested.c:kvm_inject_el2_exception() {
        ...
        switch (except_type) {
        case except_type_sync:
                kvm_pend_exception(vcpu, EXCEPT_AA64_EL2_SYNC);
                vcpu_write_sys_reg(vcpu, esr_el2, ESR_EL2);
                break;
        case except_type_serror:
                kvm_pend_exception(vcpu, EXCEPT_AA64_EL2_SERR);
                break;
        ...
}

Unlike the except_type_sync case, the except_type_serror case does not call
vcpu_write_sys_reg() to write esr_el2 into ESR_EL2.

Will this cause the guest hypervisor to observe a stale ESR_EL2 value when
it takes the SError, effectively rendering the syndrome fixes ineffective for
nested guests?

>  		vcpu_write_sys_reg(vcpu, esr, exception_esr_elx(vcpu));
>  		return 1;
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260614163336.3490925-1-tabba@google.com?part=5

Re: [PATCH 6/7] KVM: arm64: Set IL for nested SError injection

[Oliver Upton]

On Sun, Jun 14, 2026 at 04:44:16PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] The constructed SError syndrome (ESR) is silently dropped during nested EL2 exception injection because `ESR_EL2` is never updated.
> - [High] When SCTLR2_EL1_EASE routes a Synchronous External abort to an SError, the ESR injected has the wrong EC (IABT/DABT instead of SERROR) and an incorrect ISS format.
> --
> 
> commit 09fd06d56ab3236a0e7af03e0d026ed6725ad6d6
> Author: Fuad Tabba <tabba@google.com>
> 
> KVM: arm64: Set IL for nested SError injection
> 
> This patch modifies the emulated SError injection to unconditionally
> set the IL (Instruction Length) bit in the generated ESR, as mandated
> by the architecture.
> 
> > diff --git a/arch/arm64/kvm/emulate-nested.c b/arch/arm64/kvm/emulate-nested.c
> > index 4b39363cf8911..4262d4c17a877 100644
> > --- a/arch/arm64/kvm/emulate-nested.c
> > +++ b/arch/arm64/kvm/emulate-nested.c
> > @@ -2938,6 +2938,6 @@ int kvm_inject_nested_serror(struct kvm_vcpu *vcpu, u64 esr)
> >  	 * vSError injection. Manually populate EC for an emulated SError
> >  	 * exception.
> >  	 */
> > -	esr |= FIELD_PREP(ESR_ELx_EC_MASK, ESR_ELx_EC_SERROR);
> > +	esr |= FIELD_PREP(ESR_ELx_EC_MASK, ESR_ELx_EC_SERROR) | ESR_ELx_IL;
> >  	return kvm_inject_nested(vcpu, esr, except_type_serror);
> 
> [Severity: High]
> This isn't a bug introduced by this patch, but does the carefully constructed
> esr value actually get written to the guest's ESR_EL2?
> 
> Looking at kvm_inject_nested(), it passes the esr_el2 argument down:
> 
> kvm_inject_nested(vcpu, esr_el2, type) {
>     ...
>     kvm_inject_el2_exception(vcpu, esr_el2, type);
>     ...
> }
> 
> However, in kvm_inject_el2_exception():
> 
> static void kvm_inject_el2_exception(...) {
>     ...
>     switch (type) {
>     ...
>     case except_type_serror:
>         kvm_pend_exception(vcpu, EXCEPT_AA64_EL2_SERR);
>         break;
>     ...
> }
> 
> Since except_type_serror only pends the exception and doesn't explicitly
> write the esr_el2 argument into ESR_EL2 (unlike except_type_sync), does the
> nested hypervisor observe a stale or uninitialized syndrome when it takes
> the SError?

Oops, this definitely needs fixing :) OTOH...

> [Severity: High]
> This is a pre-existing issue, but since this code deals with fixing SError
> syndromes, what happens when SCTLR2_ELx.EASE is enabled and a Synchronous
> External abort is routed to an SError?
> 
> Looking at kvm_inject_nested_sea() in the same file:
> 
> int kvm_inject_nested_sea(struct kvm_vcpu *vcpu) {
>     ...
>     u64 esr = FIELD_PREP(ESR_ELx_EC_MASK,
>                          iabt ? ESR_ELx_EC_IABT_LOW : ESR_ELx_EC_DABT_LOW);
>     ...
>     if (__vcpu_sys_reg(vcpu, SCTLR2_EL2) & SCTLR2_EL1_EASE)
>         return kvm_inject_nested(vcpu, esr, except_type_serror);
>     ...
> }
> 
> Does this result in an SError being injected with a Data/Instruction Abort
> EC? According to the ARM Architecture Reference Manual (D8.2.5), shouldn't
> an SError always have its ESR_ELx.EC field set to 0b101111 (SError)?
> 
> A similar pattern appears in inject_abt64() for EL1, where a DABT/IABT
> syndrome is constructed and actively written to exception_esr_elx(vcpu)
> even though an SError exception was pended. Would this incorrect EC format
> confuse the guest's exception handlers?

This is confidently wrong, down to citations of the Arm ARM.

While I couldn't find an explicit rule to cite in my cursory search of
the manual, the definition of the bit describes this as "External Abort
to SError Vector". On top of that, AArch64.DataAbort() just changes the
vector offset (but not the EC) based on EASE.

My understanding is that the exception syndrome of an external abort is
preserved despite taking the SError vector.

Thanks,
Oliver

Re: [PATCH 4/7] KVM: arm64: Set IL for injected FPAC exceptions during ERET emulation

[Fuad Tabba]

On Sun, 14 Jun 2026 at 17:45, <sashiko-bot@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>
> Pre-existing issues:
> - [High] Incorrect Emulation of Illegal Exception Return corrupts ELR_EL2 and may cause spurious Instruction Aborts
> --
>
> commit 56bc216de713f5bf0e45ff5ad97580cbdc7eeb66
> Author: Fuad Tabba <tabba@google.com>
>
> KVM: arm64: Set IL for injected FPAC exceptions during ERET emulation
>
> This commit fixes the FPAC syndrome constructed during nested ERET
> emulation to include the IL bit. It ensures that the IL bit is set to 1
> for ERET and its authenticated variants, which are always 32-bit A64
> instructions.
>
> > diff --git a/arch/arm64/kvm/emulate-nested.c b/arch/arm64/kvm/emulate-nested.c
> > --- a/arch/arm64/kvm/emulate-nested.c
> > +++ b/arch/arm64/kvm/emulate-nested.c
> > @@ -2777,7 +2777,7 @@ void kvm_emulate_nested_eret(struct kvm_vcpu *vcpu)
> >                */
> >               if (kvm_has_pauth(vcpu->kvm, FPACCOMBINE) && !(spsr & PSR_IL_BIT)) {
> >                       esr &= ESR_ELx_ERET_ISS_ERETA;
> > -                     esr |= FIELD_PREP(ESR_ELx_EC_MASK, ESR_ELx_EC_FPAC);
> > +                     esr |= FIELD_PREP(ESR_ELx_EC_MASK, ESR_ELx_EC_FPAC) | ESR_ELx_IL;
> >                       kvm_inject_nested_sync(vcpu, esr);
> >                       return;
> >               }
> >       }
> >
> >       preempt_disable();
> >       vcpu_set_flag(vcpu, IN_NESTED_ERET);
> >       kvm_arch_vcpu_put(vcpu);
> >
> >       if (!esr_iss_is_eretax(esr))
> >               elr = __vcpu_sys_reg(vcpu, ELR_EL2);
> >
> >       trace_kvm_nested_eret(vcpu, elr, spsr);
> >
> >       *vcpu_pc(vcpu) = elr;
> >       *vcpu_cpsr(vcpu) = spsr;
>
> [Severity: High]
> This is a pre-existing issue, but does setting the PC to the target address
> here cause issues when an illegal exception return is detected?
>
> If kvm_check_illegal_exception_return() previously detected an illegal return
> and set PSR_IL_BIT in spsr, kvm_emulate_nested_eret() unconditionally
> advances the guest PC to the ERET target address (elr) and returns to the
> guest, relying on hardware to generate the Illegal Execution State exception.
>
> Wouldn't the hardware then save the target address into the guest's ELR_EL2
> instead of the faulting ERET address, corrupting the architectural state?
>
> Additionally, since the hardware fetches the instruction at the target address
> using the guest EL2 translation regime before execution, an unmapped or
> unexecutable target address could trigger an Instruction Abort instead of the
> required Illegal Execution State exception, per ARM ARM D1.8.2.
>
> Could this result in the guest hypervisor receiving a spurious Instruction
> Abort for an EL1/EL0 address?

Not a bug. On an illegal exception return, the Arm ARM specifies that
PC is set to ELR_ELx unconditionally. The subsequent Illegal Execution
State exception is taken at the target PC, so ELR_ELx is saved with
the target address. This is architecturally correct.

Similarly, an Instruction Abort at an unmapped target would take
priority over the Illegal Execution State exception, which is also
correct behavior.

One thing I noticed while looking at this:
kvm_check_illegal_exception_return() preserves {N,Z,C,V,D,A,I,F} from
the current PSTATE, but the Arm ARM says these should come from
SPSR_ELx on an illegal exception return (EL and SP are unchanged, but
the flags and masks are taken from SPSR). PAN and ALLINT should also
come from SPSR_ELx if their respective features are implemented.

Marc, is the current behavior intentional, or should we be
constructing PSTATE from the original SPSR_EL2 for those fields?

Cheers,
/fuad

>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260614163336.3490925-1-tabba@google.com?part=4

Re: [PATCH 5/7] KVM: arm64: Set IL for emulated SError injection

[Fuad Tabba]

On Sun, 14 Jun 2026 at 17:46, <sashiko-bot@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>
> Pre-existing issues:
> - [High] The patch series attempts to fix SError syndrome IL bits, but the constructed ESR is silently dropped when injecting a nested SError because `kvm_inject_el2_exception()` ignores the `esr_el2` argument.
> --
>
> commit 3adb0abe8b63984396e6d2ad4fb963bab5512c79
> Author: Fuad Tabba <tabba@google.com>
> Subject: KVM: arm64: Set IL for emulated SError injection
>
> This commit updates kvm_inject_serror_esr to include the IL bit when
> constructing an SError syndrome. It fixes an architectural compliance issue
> where IL must be 1 for SErrors unconditionally.
>
> > diff --git a/arch/arm64/kvm/inject_fault.c b/arch/arm64/kvm/inject_fault.c
> > index 444d219b02174..d6c4fc16f8795 100644
> > --- a/arch/arm64/kvm/inject_fault.c
> > +++ b/arch/arm64/kvm/inject_fault.c
> > @@ -381,7 +381,7 @@ int kvm_inject_serror_esr(struct kvm_vcpu *vcpu, u64 esr)
> >        */
> >       if (!serror_is_masked(vcpu)) {
> >               pend_serror_exception(vcpu);
> > -             esr |= FIELD_PREP(ESR_ELx_EC_MASK, ESR_ELx_EC_SERROR);
> > +             esr |= FIELD_PREP(ESR_ELx_EC_MASK, ESR_ELx_EC_SERROR) | ESR_ELx_IL;
>
> [Severity: High]
> This is a pre-existing issue, but while we are ensuring the SError syndrome
> is correctly populated here, does the nested SError injection path drop the
> syndrome entirely?
>
> When injecting a nested SError, kvm_inject_nested_serror() passes the
> constructed syndrome down to kvm_inject_el2_exception(). However, looking at
> kvm_inject_el2_exception() in arch/arm64/kvm/emulate-nested.c:
>
> arch/arm64/kvm/emulate-nested.c:kvm_inject_el2_exception() {
>         ...
>         switch (except_type) {
>         case except_type_sync:
>                 kvm_pend_exception(vcpu, EXCEPT_AA64_EL2_SYNC);
>                 vcpu_write_sys_reg(vcpu, esr_el2, ESR_EL2);
>                 break;
>         case except_type_serror:
>                 kvm_pend_exception(vcpu, EXCEPT_AA64_EL2_SERR);
>                 break;
>         ...
> }
>
> Unlike the except_type_sync case, the except_type_serror case does not call
> vcpu_write_sys_reg() to write esr_el2 into ESR_EL2.
>
> Will this cause the guest hypervisor to observe a stale ESR_EL2 value when
> it takes the SError, effectively rendering the syndrome fixes ineffective for
> nested guests?

Yes : https://lore.kernel.org/all/ai-DsLydd7ry0RpS@kernel.org/

>
> >               vcpu_write_sys_reg(vcpu, esr, exception_esr_elx(vcpu));
> >               return 1;
> >       }
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260614163336.3490925-1-tabba@google.com?part=5

Re: [PATCH 6/7] KVM: arm64: Set IL for nested SError injection

[Fuad Tabba]

On Mon, 15 Jun 2026 at 05:46, Oliver Upton <oupton@kernel.org> wrote:
>
> On Sun, Jun 14, 2026 at 04:44:16PM +0000, sashiko-bot@kernel.org wrote:
> > Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> >
> > Pre-existing issues:
> > - [High] The constructed SError syndrome (ESR) is silently dropped during nested EL2 exception injection because `ESR_EL2` is never updated.
> > - [High] When SCTLR2_EL1_EASE routes a Synchronous External abort to an SError, the ESR injected has the wrong EC (IABT/DABT instead of SERROR) and an incorrect ISS format.
> > --
> >
> > commit 09fd06d56ab3236a0e7af03e0d026ed6725ad6d6
> > Author: Fuad Tabba <tabba@google.com>
> >
> > KVM: arm64: Set IL for nested SError injection
> >
> > This patch modifies the emulated SError injection to unconditionally
> > set the IL (Instruction Length) bit in the generated ESR, as mandated
> > by the architecture.
> >
> > > diff --git a/arch/arm64/kvm/emulate-nested.c b/arch/arm64/kvm/emulate-nested.c
> > > index 4b39363cf8911..4262d4c17a877 100644
> > > --- a/arch/arm64/kvm/emulate-nested.c
> > > +++ b/arch/arm64/kvm/emulate-nested.c
> > > @@ -2938,6 +2938,6 @@ int kvm_inject_nested_serror(struct kvm_vcpu *vcpu, u64 esr)
> > >      * vSError injection. Manually populate EC for an emulated SError
> > >      * exception.
> > >      */
> > > -   esr |= FIELD_PREP(ESR_ELx_EC_MASK, ESR_ELx_EC_SERROR);
> > > +   esr |= FIELD_PREP(ESR_ELx_EC_MASK, ESR_ELx_EC_SERROR) | ESR_ELx_IL;
> > >     return kvm_inject_nested(vcpu, esr, except_type_serror);
> >
> > [Severity: High]
> > This isn't a bug introduced by this patch, but does the carefully constructed
> > esr value actually get written to the guest's ESR_EL2?
> >
> > Looking at kvm_inject_nested(), it passes the esr_el2 argument down:
> >
> > kvm_inject_nested(vcpu, esr_el2, type) {
> >     ...
> >     kvm_inject_el2_exception(vcpu, esr_el2, type);
> >     ...
> > }
> >
> > However, in kvm_inject_el2_exception():
> >
> > static void kvm_inject_el2_exception(...) {
> >     ...
> >     switch (type) {
> >     ...
> >     case except_type_serror:
> >         kvm_pend_exception(vcpu, EXCEPT_AA64_EL2_SERR);
> >         break;
> >     ...
> > }
> >
> > Since except_type_serror only pends the exception and doesn't explicitly
> > write the esr_el2 argument into ESR_EL2 (unlike except_type_sync), does the
> > nested hypervisor observe a stale or uninitialized syndrome when it takes
> > the SError?
>
> Oops, this definitely needs fixing :) OTOH...

https://lore.kernel.org/all/20260615131116.390977-1-tabba@google.com/

Done :)

/fuad


>
> > [Severity: High]
> > This is a pre-existing issue, but since this code deals with fixing SError
> > syndromes, what happens when SCTLR2_ELx.EASE is enabled and a Synchronous
> > External abort is routed to an SError?
> >
> > Looking at kvm_inject_nested_sea() in the same file:
> >
> > int kvm_inject_nested_sea(struct kvm_vcpu *vcpu) {
> >     ...
> >     u64 esr = FIELD_PREP(ESR_ELx_EC_MASK,
> >                          iabt ? ESR_ELx_EC_IABT_LOW : ESR_ELx_EC_DABT_LOW);
> >     ...
> >     if (__vcpu_sys_reg(vcpu, SCTLR2_EL2) & SCTLR2_EL1_EASE)
> >         return kvm_inject_nested(vcpu, esr, except_type_serror);
> >     ...
> > }
> >
> > Does this result in an SError being injected with a Data/Instruction Abort
> > EC? According to the ARM Architecture Reference Manual (D8.2.5), shouldn't
> > an SError always have its ESR_ELx.EC field set to 0b101111 (SError)?
> >
> > A similar pattern appears in inject_abt64() for EL1, where a DABT/IABT
> > syndrome is constructed and actively written to exception_esr_elx(vcpu)
> > even though an SError exception was pended. Would this incorrect EC format
> > confuse the guest's exception handlers?
>
> This is confidently wrong, down to citations of the Arm ARM.
>
> While I couldn't find an explicit rule to cite in my cursory search of
> the manual, the definition of the bit describes this as "External Abort
> to SError Vector". On top of that, AArch64.DataAbort() just changes the
> vector offset (but not the EC) based on EASE.
>
> My understanding is that the exception syndrome of an external abort is
> preserved despite taking the SError vector.
>
> Thanks,
> Oliver