[PATCH] KVM: arm64: Add an ISB after mapping a hyp fixmap slot

[PATCH] KVM: arm64: Add an ISB after mapping a hyp fixmap slot

[Xuheng Li via B4 Relay]

From: Xuheng Li <xuheng@cs.columbia.edu>

fixmap_map_slot() installs a valid PTE for the CPU's fixmap slot (or
the fixblock slot) and issues a DSB ISHST before returning the slot's
VA, which the caller then accesses.  It does not issue an ISB.

The DSB guarantees that the PTE write has completed, but without a
context synchronization event the architecture does not guarantee that
a later access on the same CPU uses the new entry: its translation may
have been performed speculatively while the slot was still invalid, in
which case the access takes a translation fault.  At EL2 that fault
ends in hyp_panic().  The clear side, fixmap_clear_slot(), already ends
with an ISB.

A litmus test of the same shape (map the slot, DSB ISHST, load through
it) faulted on a Cortex-A72 in 2 out of 100M runs.  Arm's herd7 model
with -variant vmsa also allows the fault, and forbids it once the ISB
is added.

Add the missing ISB.  This covers both hyp_fixmap_map() and
hyp_fixblock_map(), which share fixmap_map_slot().

Fixes: aa6948f82f0b ("KVM: arm64: Add per-cpu fixmap infrastructure at EL2")
Cc: stable@vger.kernel.org
Signed-off-by: Xuheng Li <xuheng@cs.columbia.edu>
---
 arch/arm64/kvm/hyp/nvhe/mm.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/kvm/hyp/nvhe/mm.c b/arch/arm64/kvm/hyp/nvhe/mm.c
index b95d98cfe..83b74c522 100644
--- a/arch/arm64/kvm/hyp/nvhe/mm.c
+++ b/arch/arm64/kvm/hyp/nvhe/mm.c
@@ -316,6 +316,7 @@ static void *fixmap_map_slot(struct hyp_fixmap_slot *slot, phys_addr_t phys)
 	pte |= kvm_phys_to_pte(phys) | KVM_PTE_VALID;
 	WRITE_ONCE(*ptep, pte);
 	dsb(ishst);
+	isb();
 
 	return (void *)slot->addr;
 }

---
base-commit: c339f4010dea8b0d754f84ace70cd1b089878ef8
change-id: 20261028-fixmap-isb-cc9d765e37bc

Best regards,
-- 
Xuheng Li <xuheng@cs.columbia.edu>

Re: [PATCH] KVM: arm64: Add an ISB after mapping a hyp fixmap slot

[Fuad Tabba]

Hi Xuheng,

On Wed, 28 Oct 2026 23:02:10 +0000, Xuheng Li <xuheng@cs.columbia.edu> wrote:
[...]
> A litmus test of the same shape (map the slot, DSB ISHST, load through
> it) faulted on a Cortex-A72 in 2 out of 100M runs.  Arm's herd7 model
> with -variant vmsa also allows the fault, and forbids it once the ISB
> is added.

nit: it would help to have the litmus test itself in the commit message,
or as a commit note, so the herd7 claim can be checked. Here's the one I
ran (herd7 7.58, -variant vmsa): "Sometimes" as below, "Never" with an
ISB after the DSB, and "Never" without the ISB under -variant vmsa,ETS2.

	AArch64 fixmap-noisb
	{
	 [PTE(x)]=(oa:PA(x), valid:0);
	 0:X1=PTE(x); 0:X2=(oa:PA(x)); 0:X3=x;
	}
	 P0           ;
	 STR X2,[X1]  ;
	 DSB ISHST    ;
	 LDR W4,[X3]  ;
	exists (fault(P0,x))

FWIW, the result matches the ARM ARM (B2.3: without FEAT_ETS2, a DSB ST
doesn't order an earlier store before a later translation table walk),
and emit_pte_barriers() in arch/arm64/include/asm/pgtable.h has the same
dsb(ishst) plus isb() after making a kernel pte valid, for the same
reason.

[...]

Reviewed-by: Fuad Tabba <fuad.tabba@linux.dev>

Tested in protected mode on QEMU, booting a protected and a
non-protected guest, and on an Apple M4 (nested virtualization),
booting a protected guest. No regressions.

Tested-by: Fuad Tabba <fuad.tabba@linux.dev>

Cheers,
/fuad

Re: [PATCH] KVM: arm64: Add an ISB after mapping a hyp fixmap slot

[Oliver Upton]

On Thu, Oct 29, 2026 at 08:54:14AM +0000, Fuad Tabba wrote:
> Hi Xuheng,
> 
> On Wed, 28 Oct 2026 23:02:10 +0000, Xuheng Li <xuheng@cs.columbia.edu> wrote:
> [...]
> > A litmus test of the same shape (map the slot, DSB ISHST, load through
> > it) faulted on a Cortex-A72 in 2 out of 100M runs.  Arm's herd7 model
> > with -variant vmsa also allows the fault, and forbids it once the ISB
> > is added.
> 
> nit: it would help to have the litmus test itself in the commit message,
> or as a commit note, so the herd7 claim can be checked. Here's the one I
> ran (herd7 7.58, -variant vmsa): "Sometimes" as below, "Never" with an
> ISB after the DSB, and "Never" without the ISB under -variant vmsa,ETS2.

Maybe I've been conditioned at this point but doing DSB; ISB to consume a
translation after setting it up seems pretty straightforward. Especially
since we don't care about ETS in the kernel at this point.

IOW, I'm not sure what additional detail the litmus test buys us.

Anyway:

Reviewed-by: Oliver Upton <oupton@kernel.org>

Thanks,
Oliver

> 	AArch64 fixmap-noisb
> 	{
> 	 [PTE(x)]=(oa:PA(x), valid:0);
> 	 0:X1=PTE(x); 0:X2=(oa:PA(x)); 0:X3=x;
> 	}
> 	 P0           ;
> 	 STR X2,[X1]  ;
> 	 DSB ISHST    ;
> 	 LDR W4,[X3]  ;
> 	exists (fault(P0,x))
> 
> FWIW, the result matches the ARM ARM (B2.3: without FEAT_ETS2, a DSB ST
> doesn't order an earlier store before a later translation table walk),
> and emit_pte_barriers() in arch/arm64/include/asm/pgtable.h has the same
> dsb(ishst) plus isb() after making a kernel pte valid, for the same
> reason.
> 
> [...]
> 
> Reviewed-by: Fuad Tabba <fuad.tabba@linux.dev>
> 
> Tested in protected mode on QEMU, booting a protected and a
> non-protected guest, and on an Apple M4 (nested virtualization),
> booting a protected guest. No regressions.
> 
> Tested-by: Fuad Tabba <fuad.tabba@linux.dev>
> 
> Cheers,
> /fuad