Re: Raspberry Pi 3B: /dev/mem mmap GPIO works on earmv7hf but not onaarch64

[Izumi Tsutsui <tsutsui%ceres.dti.ne.jp@localhost>]

I wrote:

> On the other hand, pmap_mmap_flags() (which is undocumented in pmap(9)) 
> exists, at least on aarch64 and x86, and looks like a place where
> mapping attributes for mmap(2) could be decided.

The following changes against aarch64 specific pmap_mmap_flags()
makes "mmap(2) GPIO via /dev/mem" work:

https://github.com/tsutsui/netbsd-src/commit/567111726df3fd5180ac7a81266be9fefa2c6ce4

---
aarch64: fix /dev/mem mmap cookie handling for MMIO devices

On aarch64, /dev/mem mmap(2) commonly returns an mmap cookie
without explicit MD ARM_MMAP_* attribute bits (nflag == 0) because
there is no MD hook in current MI mem(4) implementation.

In that case the mapping may end up as normal cacheable memory,
which breaks MMIO register access.

Adjust aarch64_mmap_flags(mdpgno) to return PMAP_DEV_NP only for
the narrow case where:

- AARCH64_MMAP_* bits are 0
  (note AARCH64_MMAP_WRITEBACK is also 0 so "unspecified" is ambiguous),
- the PA is unmanaged (PHYS_TO_VM_PAGE(pa) == NULL), and
- the PA is within a devmap region (pmap_devmap_find_pa() != NULL).

Explicit cookie attributes (DEVICE, WRITECOMBINE, NOCACHE, and
WRITEBACK against managed or non-devmap memory) are still decoded
as before.
---

diff --git a/sys/arch/aarch64/aarch64/pmap.c b/sys/arch/aarch64/aarch64/pmap.c
index 53063d9dc92f..cff9859d5b8e 100644
--- a/sys/arch/aarch64/aarch64/pmap.c
+++ b/sys/arch/aarch64/aarch64/pmap.c
@@ -2755,6 +2755,59 @@ pmap_is_referenced(struct vm_page *pg)
 	return false;
 }
 
+u_int
+aarch64_mmap_flags(paddr_t mdpgno)
+{
+	u_int nflag, pflag;
+	paddr_t pa;
+
+	/*
+	 * aarch64 arch has 5 memory attributes defined:
+	 *
+	 *  WriteBack      - write back cache
+	 *  WriteThru      - write through cache
+	 *  NoCache        - no cache
+	 *  Device(nGnRE)  - no Gathering, no Reordering, Early write ack
+	 *  Device(nGnRnE) - no Gathering, no Reordering, no Early write ack
+	 *
+	 * but pmap has PMAP_{NOCACHE,WRITE_COMBINE,WRITE_BACK} flags.
+	 */
+
+	nflag = (mdpgno >> AARCH64_MMAP_FLAG_SHIFT) & AARCH64_MMAP_FLAG_MASK;
+
+	/*
+	 * If AARCH64_MMAP_FLAG is 0 (i.e. no way to distinguish explicit
+	 * AARCH64_MMAP_WRITEBACK from "0" as "not specified"),
+	 * the specified page is unmanaged and is within devmap region,
+	 * assume MMIO device mappings.
+	 */
+	pa = pmap_phys_address(mdpgno);
+       	if (nflag == 0 &&
+	    PHYS_TO_VM_PAGE(pa) == NULL &&
+	    pmap_devmap_find_pa(pa, PAGE_SIZE) != NULL) {
+		return PMAP_DEV_NP;
+	}
+
+	switch (nflag) {
+	case AARCH64_MMAP_DEVICE:
+		pflag = PMAP_DEV;
+		break;
+	case AARCH64_MMAP_WRITECOMBINE:
+		pflag = PMAP_WRITE_COMBINE;
+		break;
+	case AARCH64_MMAP_WRITEBACK:
+		pflag = PMAP_WRITE_BACK;
+		break;
+	case AARCH64_MMAP_NOCACHE:
+		pflag = PMAP_NOCACHE;
+		break;
+	default:
+		pflag = PMAP_NOCACHE;
+                break;
+	}
+	return pflag;
+}
+
 /* get pointer to kernel segment L2 or L3 table entry */
 pt_entry_t *
 kvtopte(vaddr_t va)
diff --git a/sys/arch/aarch64/include/pmap.h b/sys/arch/aarch64/include/pmap.h
index 3721dcac4dfe..6103d40e479b 100644
--- a/sys/arch/aarch64/include/pmap.h
+++ b/sys/arch/aarch64/include/pmap.h
@@ -189,41 +189,7 @@ paddr_t vtophys(vaddr_t);
 #define	PMAP_DEV_NP			0x40000000 /* kenter_pa */
 #define	PMAP_DEV_MASK			(PMAP_DEV | PMAP_DEV_NP)
 
-static inline u_int
-aarch64_mmap_flags(paddr_t mdpgno)
-{
-	u_int nflag, pflag;
-
-	/*
-	 * aarch64 arch has 5 memory attributes defined:
-	 *
-	 *  WriteBack      - write back cache
-	 *  WriteThru      - write through cache
-	 *  NoCache        - no cache
-	 *  Device(nGnRE)  - no Gathering, no Reordering, Early write ack
-	 *  Device(nGnRnE) - no Gathering, no Reordering, no Early write ack
-	 *
-	 * but pmap has PMAP_{NOCACHE,WRITE_COMBINE,WRITE_BACK} flags.
-	 */
-
-	nflag = (mdpgno >> AARCH64_MMAP_FLAG_SHIFT) & AARCH64_MMAP_FLAG_MASK;
-	switch (nflag) {
-	case AARCH64_MMAP_DEVICE:
-		pflag = PMAP_DEV;
-		break;
-	case AARCH64_MMAP_WRITECOMBINE:
-		pflag = PMAP_WRITE_COMBINE;
-		break;
-	case AARCH64_MMAP_WRITEBACK:
-		pflag = PMAP_WRITE_BACK;
-		break;
-	case AARCH64_MMAP_NOCACHE:
-	default:
-		pflag = PMAP_NOCACHE;
-		break;
-	}
-	return pflag;
-}
+u_int   aarch64_mmap_flags(paddr_t);
 
 #define pmap_phys_address(ppn)		aarch64_ptob((ppn) & ~ARM_MMAP_MASK)
 #define pmap_mmap_flags(ppn)		aarch64_mmap_flags((ppn))

---
(moved from pmap.h to pmap.c because it's a bit complicated to use
 PHYS_TO_VM_PAGE() (requires <uvm/uvm_page.h>) in an inline function)


The following additional change is just for readability:

https://github.com/tsutsui/netbsd-src/commit/0abc3913ad412488a74c3360bc951d16be034e6d

---
aarch64/pmap: explicitly strip ARM_MMAP_* bits in pmap_phys_address()

ARM_MMAP_* bits are placed higher bits in mdpgno so they will implicitly
be dropped after aarch64_ptob() (i.e. << PGSHIFT) op, but it's
a bit confusing for readers to depend on implementation details.
---

diff --git a/sys/arch/aarch64/include/pmap.h b/sys/arch/aarch64/include/pmap.h
index 52f909696cf8..3721dcac4dfe 100644
--- a/sys/arch/aarch64/include/pmap.h
+++ b/sys/arch/aarch64/include/pmap.h
@@ -225,7 +225,7 @@ aarch64_mmap_flags(paddr_t mdpgno)
 	return pflag;
 }
 
-#define pmap_phys_address(pa)		aarch64_ptob((pa))
+#define pmap_phys_address(ppn)		aarch64_ptob((ppn) & ~ARM_MMAP_MASK)
 #define pmap_mmap_flags(ppn)		aarch64_mmap_flags((ppn))
 
 void pmap_bootstrap(vaddr_t, vaddr_t);


---
Izumi Tsutsui