Page MenuHomeFreeBSD

powerpc/radix: fix double page offset in mmu_radix_sync_icache()
Needs ReviewPublic

Authored by pkubaj on Mon, Sep 21, 8:58 AM.
Tags
None
Referenced Files
F172899436: D59870.id187320.diff
Mon, Sep 21, 10:49 PM
F172877573: D59870.diff
Mon, Sep 21, 7:10 PM
F172850385: D59870.diff
Mon, Sep 21, 2:07 PM
F172849112: D59870.id187320.diff
Mon, Sep 21, 1:52 PM
Subscribers

Details

Reviewers
None
Group Reviewers
PowerPC
Summary

mmu_radix_sync_icache() adds the offset of va within its page to the
physical address it gets from mmu_radix_extract_locked(). That address
already includes the offset - the extract routines return the physical
address of the byte, not of the frame - so the offset is counted twice
and __syncicache() is handed frame + 2 * offset.

The hash MMU counterpart, moea64_sync_icache(), has to add the offset
because PVO_PADDR() yields only the frame. Here the addition is wrong.

Unless va sits at the very start of a page, the cache lines that get
synced are therefore not the ones that were modified. This has gone
unnoticed because POWER9 keeps its instruction cache coherent with
stores in hardware: a thread spinning on an instruction sees another
CPU's store to it within microseconds even when nothing is synced at
all. The wrong address is not harmless, though. Once the offset
reaches half a page the sync lands in the following physical page, and
if the page being synced is the last one of a physical memory region
that address may not be covered by the direct map, in which case the
kernel takes a data storage interrupt.

Both users of this method pass addresses at arbitrary page offsets:
ppc_instr_emulate() syncs the faulting srr0 before retrying an illegal
instruction, and proc_rwmem() syncs after writing to another process,
which is how ptrace(2) and the DTrace pid provider install breakpoints.

Drop the addition.

Fixes: 6f0b2a235a13 ("powerpc/pmap: Add pmap_sync_icache() for radix pmap")

MFC after: 1 week

Test Plan

POWER9 pseries guest (radix MMU, 512 GB RAM), GENERIC64LE built natively
from main at 42c69445ca33, with and without this change.

RAM on that machine ends at 0x8000000000, and so does the direct map.
A small test program maps the last page of RAM (0x7ffffff000) read-only
and executable through /dev/mem, drops privileges and jumps to an
all-zero word that is already present in that page; nothing is written.
The first illegal instruction trap at a given srr0 makes
ppc_instr_emulate() call pmap_sync_icache(curpmap, srr0, 4), and the
retry then ends in SIGILL.

kernelpage offset 0x030page offset 0x808
unfixedSIGILLpanic
fixedSIGILLSIGILL (same for 0x80c and 0x840)

At offset 0x030 the doubled offset still falls inside the page, so the
unfixed kernel survives. At 0x808 it syncs physical address
0x8000000010, which is past the end of RAM:

fatal kernel trap:
   exception       = 0x300 (data storage interrupt)
   virtual address = 0xc000008000000000
   dsisr           = 0x40000000
   srr0            = 0xc000000000fa8f20 (0xfa8f20)
   lr              = 0xc000000000f6e900 (0xf6e900)
      pid = 97568, comm = syncicache_dsi
panic: data storage interrupt trap
kernel DSI read trap @ 0xc000008000000000 by __syncicache+0x40
at mmu_radix_sync_icache+0xb8
at ppc_instr_emulate+0x1a4
at trap+0x42c
user PGM trap by 0x810036808

The instruction cache coherence mentioned above was measured on the same
machine: a thread spinning on "li r3,1; blr" in a private page sees
another CPU's store of "li r3,2" within 0-2 microseconds, 100 out of 100
times, when the store goes through a shared mapping and nothing is
synced at all. Writes through /proc/<pid>/mem and PT_WRITE_I at page
offsets 0, 0x40, 0x100, 0x400 and 0x7c0 behave the same on the unfixed
and the fixed kernel.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 77131
Build 74014: arc lint + arc unit