[Qemu-devel] [PATCH] target-arm: Handle "extended small page" descriptors correctly

[Peter Maydell]

The old ARMv5-style page table format includes a kind of second level
descriptor named the "extended small page" format, whose primary purpose
is to allow specification of the TEX memory attribute bits on a 4K page.
This exists on ARMv6 and also (as an implementation extension) on XScale
CPUs; it's UNPREDICTABLE on v5.

We were mishandling this in two ways:
 (1) we weren't implementing it for v6 (probably never noticed because
Linux will use the new-style v6 page table format there)
 (2) we were not correctly setting the page_size, which is 4K, not 1K

The latter bug went unnoticed for years because the only thing which
the page_size affects is which TLB entries get flushed when the guest
does a TLB invalidate on an address in the page, and prior to commit
2f0d8631b7 we were doing a full TLB flush very frequently due to Linux's
habit of writing the SCTLR pointlessly a lot.

(We can assume that after commit 2f0d8631b7 the bug went unnoticed
for a year because nobody's actually using the Zaurus/XScale emulation...)

Report the correct page size for these descriptors, and permit them
on ARMv6 CPUs. This fixes a problem where a kernel image for Zaurus
can boot the kernel OK but gets random segfaults when it tries to
run userspace programs.

Signed-off-by: Peter Maydell <address@hidden>
---
This took me something like three weeks to track down, on and off...

 target-arm/helper.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/target-arm/helper.c b/target-arm/helper.c
index f8f8d76..238da3c 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -5227,20 +5227,25 @@ static int get_phys_addr_v5(CPUARMState *env, uint32_t 
address, int access_type,
             ap = (desc >> (4 + ((address >> 9) & 6))) & 3;
             *page_size = 0x1000;
             break;
-        case 3: /* 1k page.  */
+        case 3: /* 1k page, or ARMv6/XScale "extended small (4k) page" */
             if (type == 1) {
-                if (arm_feature(env, ARM_FEATURE_XSCALE)) {
+                /* ARMv6/XScale extended small page format */
+                if (arm_feature(env, ARM_FEATURE_XSCALE)
+                    || arm_feature(env, ARM_FEATURE_V6)) {
                     phys_addr = (desc & 0xfffff000) | (address & 0xfff);
+                    *page_size = 0x1000;
                 } else {
-                    /* Page translation fault.  */
+                    /* UNPREDICTABLE in ARMv5; we choose to take a
+                     * page translation fault.
+                     */
                     code = 7;
                     goto do_fault;
                 }
             } else {
                 phys_addr = (desc & 0xfffffc00) | (address & 0x3ff);
+                *page_size = 0x400;
             }
             ap = (desc >> 4) & 3;
-            *page_size = 0x400;
             break;
         default:
             /* Never happens, but compiler isn't smart enough to tell.  */
-- 
1.9.1