patches.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
From: Sasha Levin <sashal@kernel.org>
To: patches@lists.linux.dev, stable@vger.kernel.org
Cc: Fuad Tabba <tabba@google.com>,
	Mark Rutland <mark.rutland@arm.com>,
	Will Deacon <will@kernel.org>, Sasha Levin <sashal@kernel.org>
Subject: [PATCH AUTOSEL 6.17-6.12] arm64: sysreg: Correct sign definitions for EIESB and DoubleLock
Date: Thu,  2 Oct 2025 11:29:49 -0400	[thread overview]
Message-ID: <20251002153025.2209281-2-sashal@kernel.org> (raw)
In-Reply-To: <20251002153025.2209281-1-sashal@kernel.org>

From: Fuad Tabba <tabba@google.com>

[ Upstream commit f4d4ebc84995178273740f3e601e97fdefc561d2 ]

The `ID_AA64MMFR4_EL1.EIESB` field, is an unsigned enumeration, but was
incorrectly defined as a `SignedEnum` when introduced in commit
cfc680bb04c5 ("arm64: sysreg: Add layout for ID_AA64MMFR4_EL1"). This is
corrected to `UnsignedEnum`.

Conversely, the `ID_AA64DFR0_EL1.DoubleLock` field, is a signed
enumeration, but was incorrectly defined as an `UnsignedEnum`. This is
corrected to `SignedEnum`, which wasn't correctly set when annotated as
such in commit ad16d4cf0b4f ("arm64/sysreg: Initial unsigned annotations
for ID registers").

Signed-off-by: Fuad Tabba <tabba@google.com>
Acked-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Will Deacon <will@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

Based on my comprehensive analysis, here is my determination:

## Backport Status: **YES**

## Extensive Analysis

### Overview
This commit corrects sign definitions for two ARM64 system register
fields:
1. **ID_AA64MMFR4_EL1.EIESB** (bits 7:4): Incorrectly defined as
   `SignedEnum`, should be `UnsignedEnum`
2. **ID_AA64DFR0_EL1.DoubleLock** (bits 39:36): Incorrectly defined as
   `UnsignedEnum`, should be `SignedEnum`

### Technical Deep Dive

#### How Sign Definitions Matter

The `arch/arm64/tools/gen-sysreg.awk` script processes these definitions
(lines 277-301) to generate `_SIGNED` macros:
- `SignedEnum` → `#define REG_FIELD_SIGNED true`
- `UnsignedEnum` → `#define REG_FIELD_SIGNED false`

These macros control critical behavior in
`arch/arm64/include/asm/kvm_host.h:1541-1544`:
```c
#define kvm_cmp_feat(kvm, id, fld, op, limit)
    (id##_##fld##_SIGNED ?
     kvm_cmp_feat_signed(kvm, id, fld, op, limit) :
     kvm_cmp_feat_unsigned(kvm, id, fld, op, limit))
```

The signed vs unsigned distinction affects:
- **Field extraction**: Signed fields get sign-extended via
  `sign_extend64()` (line 1529)
- **Comparison values**: Signed limits get sign-extended (line 1517)
- **Feature detection logic**: Used by `kvm_has_feat()` macro for
  capability checks

#### ARM Architecture Context

Following ARM conventions:
- **Signed enumerations**: Use `0b1111` to represent "Not Implemented"
  (interpreted as -1 in 4-bit signed)
- **Unsigned enumerations**: Count up from 0, with higher values
  indicating more capabilities

**DoubleLock** (should be signed):
- Values: `0b0000` (IMP), `0b1111` (NI)
- Pattern matches other signed fields like MTPMU, FP, AdvSIMD (all using
  0b1111=NI)
- The 0b1111=-1 pattern indicates "feature not present"

**EIESB** (should be unsigned):
- Values: `0b0000` (NI), `0b0001` (ToEL3), `0b0010` (ToELx), `0b1111`
  (ANY)
- Here 0b1111=15 means "applies to ANY exception level" (maximum
  capability)
- This is an ascending enumeration, not a "not implemented" marker

### Active Bug Impact

#### DoubleLock Bug (CRITICAL)

The incorrect definition causes **real bugs** in
`arch/arm64/kvm/hyp/nvhe/pkvm.c:109` (added December 16, 2024, commit
0401f7e76d707):

```c
if (!kvm_has_feat(kvm, ID_AA64DFR0_EL1, DoubleLock, IMP))
    val |= MDCR_EL2_TDOSA;
```

**Bug behavior** (with incorrect unsigned definition):
- When DoubleLock = `0b1111` (not implemented)
- Incorrectly interpreted as unsigned 15
- Check: `15 >= 0` (IMP) → **TRUE** (wrong!)
- Result: **Does NOT set MDCR_EL2_TDOSA trap**
- Impact: **Incorrect hypervisor behavior** - fails to trap debug
  operations that should be trapped

**Correct behavior** (after fix):
- When DoubleLock = `0b1111` (not implemented)
- Correctly interpreted as signed -1
- Check: `-1 >= 0` (IMP) → **FALSE** (correct!)
- Result: **Correctly sets MDCR_EL2_TDOSA trap**

#### EIESB Bug (LATENT)

Not actively used in feature detection yet, but the incorrect definition
would cause failures if code checks `kvm_has_feat(kvm, ID_AA64MMFR4_EL1,
EIESB, ToELx)`:
- Wrong: `-1 >= 2` → FALSE (incorrectly thinks feature unsupported)
- Right: `15 >= 2` → TRUE (correctly detects ANY exception level
  support)

### Code References

Key usage locations:
- **DoubleLock**: `arch/arm64/kvm/hyp/nvhe/pkvm.c:109` - Active bug in
  KVM trap configuration
- **DoubleLock**: `arch/arm64/kvm/config.c` - Feature configuration
- **DoubleLock**: `arch/arm64/kvm/sys_regs.c` - System register field
  preparation
- **Sign logic**: `arch/arm64/kernel/cpufeature.c:191-208` - FTR_BITS
  macros use sign field

### Commit History

- **Jan 31, 2023** (ad16d4cf0b4f): DoubleLock incorrectly marked as
  unsigned
- **Jan 22, 2024** (cfc680bb04c5): EIESB incorrectly introduced as
  signed
- **Dec 16, 2024** (0401f7e76d707): KVM code starts using DoubleLock -
  **bug becomes active**
- **Aug 29, 2025** (f4d4ebc84995): This fix corrects both sign
  definitions
- Acked by Mark Rutland (ARM maintainer)
- No reverts or follow-up fixes found

### Backport Justification

✅ **Fixes important bug**: Active DoubleLock bug causes incorrect KVM
trap configuration
✅ **Small and contained**: Only changes two type annotations in metadata
file
✅ **No architectural changes**: Pure correctness fix
✅ **Minimal regression risk**: Aligns with ARM architecture
specifications
✅ **Affects critical subsystem**: KVM hypervisor trap configuration
✅ **Clean backport**: Changes apply to stable kernel versions
✅ **Maintainer approved**: Acked-by from ARM maintainer Mark Rutland

### Affected Kernel Versions

Should backport to stable kernels containing:
1. The incorrect definitions (since 6.3+ for DoubleLock, 6.8+ for EIESB)
2. **Especially critical** for kernels with the KVM usage code (6.13+)

This commit fixes incorrect metadata that causes real runtime bugs in
ARM64 virtualization code.

 arch/arm64/tools/sysreg | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/tools/sysreg b/arch/arm64/tools/sysreg
index 696ab1f32a674..2a37d4c26d870 100644
--- a/arch/arm64/tools/sysreg
+++ b/arch/arm64/tools/sysreg
@@ -1693,7 +1693,7 @@ UnsignedEnum	43:40	TraceFilt
 	0b0000	NI
 	0b0001	IMP
 EndEnum
-UnsignedEnum	39:36	DoubleLock
+SignedEnum	39:36	DoubleLock
 	0b0000	IMP
 	0b1111	NI
 EndEnum
@@ -2409,7 +2409,7 @@ UnsignedEnum	11:8	ASID2
 	0b0000	NI
 	0b0001	IMP
 EndEnum
-SignedEnum	7:4	EIESB
+UnsignedEnum	7:4	EIESB
 	0b0000	NI
 	0b0001	ToEL3
 	0b0010	ToELx
-- 
2.51.0


  reply	other threads:[~2025-10-02 15:30 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-02 15:29 [PATCH AUTOSEL 6.17-5.4] hfs: fix KMSAN uninit-value issue in hfs_find_set_zero_bits() Sasha Levin
2025-10-02 15:29 ` Sasha Levin [this message]
2025-10-02 15:29 ` [PATCH AUTOSEL 6.17-5.4] hfs: clear offset and space out of valid records in b-tree node Sasha Levin
2025-10-02 15:29 ` [PATCH AUTOSEL 6.17-5.4] hfsplus: return EIO when type of hidden directory mismatch in hfsplus_fill_super() Sasha Levin
2025-10-02 15:29 ` [PATCH AUTOSEL 6.17-6.1] powerpc/32: Remove PAGE_KERNEL_TEXT to fix startup failure Sasha Levin
2025-10-02 15:29 ` [PATCH AUTOSEL 6.17-5.4] m68k: bitops: Fix find_*_bit() signatures Sasha Levin
2025-10-02 15:29 ` [PATCH AUTOSEL 6.17] smb: client: make use of ib_wc_status_msg() and skip IB_WC_WR_FLUSH_ERR logging Sasha Levin
2025-10-02 15:29 ` [PATCH AUTOSEL 6.17-6.16] arm64: realm: ioremap: Allow mapping memory as encrypted Sasha Levin
2025-10-02 16:43   ` Suzuki K Poulose
2025-10-21 15:38     ` Sasha Levin
2025-10-02 15:29 ` [PATCH AUTOSEL 6.17-6.12] gfs2: Fix unlikely race in gdlm_put_lock Sasha Levin
2025-10-02 15:29 ` [PATCH AUTOSEL 6.17-6.1] smb: server: let smb_direct_flush_send_list() invalidate a remote key first Sasha Levin
2025-10-02 15:29 ` [PATCH AUTOSEL 6.17-5.15] nios2: ensure that memblock.current_limit is set when setting pfn limits Sasha Levin
2025-10-02 15:29 ` [PATCH AUTOSEL 6.17-6.12] s390/mm: Use __GFP_ACCOUNT for user page table allocations Sasha Levin
2025-10-02 15:30 ` [PATCH AUTOSEL 6.17-6.16] riscv: mm: Return intended SATP mode for noXlvl options Sasha Levin
2025-10-02 15:30 ` [PATCH AUTOSEL 6.17-6.16] gfs2: Fix LM_FLAG_TRY* logic in add_to_queue Sasha Levin
2025-10-02 15:30 ` [PATCH AUTOSEL 6.17-6.16] dlm: move to rinfo for all middle conversion cases Sasha Levin
2025-10-02 15:30 ` [PATCH AUTOSEL 6.17-5.4] hfsplus: fix KMSAN uninit-value issue in hfsplus_delete_cat() Sasha Levin
2025-10-02 15:30 ` [PATCH AUTOSEL 6.17-5.4] exec: Fix incorrect type for ret Sasha Levin
2025-10-02 15:30 ` [PATCH AUTOSEL 6.17-5.4] hfsplus: fix KMSAN uninit-value issue in __hfsplus_ext_cache_extent() Sasha Levin
2025-10-02 15:30 ` [PATCH AUTOSEL 6.17-6.1] lkdtm: fortify: Fix potential NULL dereference on kmalloc failure Sasha Levin
2025-10-02 15:30 ` [PATCH AUTOSEL 6.17-6.16] riscv: mm: Use mmu-type from FDT to limit SATP mode Sasha Levin
2025-10-02 15:30 ` [PATCH AUTOSEL 6.17-6.6] Unbreak 'make tools/*' for user-space targets Sasha Levin
2025-10-02 15:30 ` [PATCH AUTOSEL 6.17-5.4] hfs: make proper initalization of struct hfs_find_data Sasha Levin
2025-10-02 15:30 ` [PATCH AUTOSEL 6.17-5.4] hfsplus: fix slab-out-of-bounds read in hfsplus_strcasecmp() Sasha Levin
2025-10-02 15:30 ` [PATCH AUTOSEL 6.17-6.16] riscv: cpufeature: add validation for zfa, zfh and zfhmin Sasha Levin
2025-10-02 15:30 ` [PATCH AUTOSEL 6.17-6.12] PCI: Test for bit underflow in pcie_set_readrq() Sasha Levin
2025-10-02 15:30 ` [PATCH AUTOSEL 6.17-6.16] s390/pkey: Forward keygenflags to ep11_unwrapkey Sasha Levin
2025-10-02 15:30 ` [PATCH AUTOSEL 6.17-6.6] drivers/perf: hisi: Relax the event ID check in the framework Sasha Levin
2025-10-02 15:30 ` [PATCH AUTOSEL 6.17-5.4] hfs: validate record offset in hfsplus_bmap_alloc Sasha Levin
2025-10-02 15:30 ` [PATCH AUTOSEL 6.17] smb: client: limit the range of info->receive_credit_target Sasha Levin
2025-10-02 15:30 ` [PATCH AUTOSEL 6.17-5.4] dlm: check for defined force value in dlm_lockspace_release Sasha Levin
2025-10-02 15:30 ` [PATCH AUTOSEL 6.17-6.12] binfmt_elf: preserve original ELF e_flags for core dumps Sasha Levin
2025-10-02 15:58   ` Kees Cook
2025-10-02 15:30 ` [PATCH AUTOSEL 6.17-6.16] arm64: errata: Apply workarounds for Neoverse-V3AE Sasha Levin
2025-10-02 15:30 ` [PATCH AUTOSEL 6.17-6.16] smb: client: queue post_recv_credits_work also if the peer raises the credit target Sasha Levin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20251002153025.2209281-2-sashal@kernel.org \
    --to=sashal@kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=patches@lists.linux.dev \
    --cc=stable@vger.kernel.org \
    --cc=tabba@google.com \
    --cc=will@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).