Hi Alex,
On Thu, 24 Apr 2014 18:36:18 +0200
Alexander Graf <address@hidden> wrote:
On 24.04.14 17:34, Jens Freimann wrote:
From: Thomas Huth <address@hidden>
With the EDAT-1 facility, the MMU translation can stop at the
segment table already, pointing to a 1 MB block.
...
diff --git a/target-s390x/helper.c b/target-s390x/helper.c
index aa628b8..89dc6e7 100644
--- a/target-s390x/helper.c
+++ b/target-s390x/helper.c
@@ -217,6 +217,10 @@ static int mmu_translate_asce(CPUS390XState *env,
target_ulong vaddr,
offs = (vaddr >> 17) & 0x3ff8;
break;
case _ASCE_TYPE_SEGMENT:
+ if (env && (env->cregs[0] & CR0_EDAT) && (asce & _SEGMENT_ENTRY_FC)) {
+ *raddr = (asce & 0xfffffffffff00000ULL) | (vaddr & 0xfffff);
+ return 0;
+ }
This is missing the page flags.
D'oh, missed that :-(
I also think we should rather align the
code with the PTE handling somehow. This way it gets pretty confusing to
follow. How about something like this (untested)?
I gave it a try ... works fine with two small modifications...
diff --git a/target-s390x/helper.c b/target-s390x/helper.c
index aa628b8..96c1c66 100644
--- a/target-s390x/helper.c
+++ b/target-s390x/helper.c
@@ -170,6 +170,48 @@ static void trigger_page_fault(CPUS390XState *env,
target_ulong vaddr,
trigger_pgm_exception(env, type, ilen);
}
+static int mmu_translate_pte(CPUS390XState *env, target_ulong vaddr,
+ uint64_t asc, uint64_t asce,
+ target_ulong *raddr, int *flags, int rw)
+{
+ if (asce & _PAGE_INVALID) {
+ DPRINTF("%s: PTE=0x%" PRIx64 " invalid\n", __func__, asce);
+ trigger_page_fault(env, vaddr, PGM_PAGE_TRANS, asc, rw);
+ return -1;
+ }
+
+ if (asce & _PAGE_RO) {
+ *flags &= ~PAGE_WRITE;
+ }
+
+ *raddr = asce & _ASCE_ORIGIN;
+
+ PTE_DPRINTF("%s: PTE=0x%" PRIx64 "\n", __func__, asce);
+
+ return 0;
+}
+
+static int mmu_translate_segpte(CPUS390XState *env, target_ulong vaddr,
+ uint64_t asc, uint64_t asce,
+ target_ulong *raddr, int *flags, int rw)
+{
+ if (asce & _SEGMENT_ENTRY_INV) {
+ DPRINTF("%s: SEG=0x%" PRIx64 " invalid\n", __func__, asce);
+ trigger_page_fault(env, vaddr, PGM_SEGMENT_TRANS, asc, rw);
+ return -1;
+ }
+
+ if (asce & _PAGE_RO) { /* XXX is this correct? */
You can remove the XXX comment, the protection bit is the same for
both, page table entries and segment table entries.
+ *flags &= ~PAGE_WRITE;
+ }
+
+ *raddr = (asce & 0xfffffffffff00000ULL) | (vaddr & 0xfffff);
+
+ PTE_DPRINTF("%s: SEG=0x%" PRIx64 "\n", __func__, asce);
+
+ return 0;
+}
+
static int mmu_translate_asce(CPUS390XState *env, target_ulong vaddr,
uint64_t asc, uint64_t asce, int level,
target_ulong *raddr, int *flags, int rw)
@@ -229,28 +271,19 @@ static int mmu_translate_asce(CPUS390XState *env,
target_ulong vaddr,
PTE_DPRINTF("%s: 0x%" PRIx64 " + 0x%" PRIx64 " => 0x%016" PRIx64 "\n",
__func__, origin, offs, new_asce);
- if (level != _ASCE_TYPE_SEGMENT) {
+ } if (level == _ASCE_TYPE_SEGMENT) {
I had to remove the "}" at the beginning of above line.
+ /* 4KB page */
+ return mmu_translate_pte(env, vaddr, asc, new_asce, raddr,
flags, rw);
+ } else if (((level - 4) == _ASCE_TYPE_SEGMENT) &&
+ (env->cregs[0] & CR0_EDAT) &&
+ (asce & _SEGMENT_ENTRY_FC)) {
That's got to be "(new_asce & _SEGMENT_ENTRY_FC)" instead.
+ /* 1MB page */
+ return mmu_translate_segpte(env, vaddr, asc, new_asce, raddr,
flags, rw);
+ } else {
/* yet another region */
return mmu_translate_asce(env, vaddr, asc, new_asce, level -
4, raddr,
flags, rw);
}
-
- /* PTE */
- if (new_asce & _PAGE_INVALID) {
- DPRINTF("%s: PTE=0x%" PRIx64 " invalid\n", __func__, new_asce);
- trigger_page_fault(env, vaddr, PGM_PAGE_TRANS, asc, rw);
- return -1;
- }
-
- if (new_asce & _PAGE_RO) {
- *flags &= ~PAGE_WRITE;
- }
-
- *raddr = new_asce & _ASCE_ORIGIN;
-
- PTE_DPRINTF("%s: PTE=0x%" PRIx64 "\n", __func__, new_asce);
-
- return 0;
}
static int mmu_translate_asc(CPUS390XState *env, target_ulong vaddr,
I'm fine with these changes, too. So how shall we continue? Could you
assemble a complete patch or shall I prepare one?