| Message ID | 53593D82.1090105@suse.de |
|---|---|
| State | New |
| Headers | show |
Hi Alex, On Thu, 24 Apr 2014 18:36:18 +0200 Alexander Graf <agraf@suse.de> wrote: > > On 24.04.14 17:34, Jens Freimann wrote: > > From: Thomas Huth <thuth@linux.vnet.ibm.com> > > > > 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? Thomas
On 25.04.14 14:15, Thomas Huth wrote: > Hi Alex, > > On Thu, 24 Apr 2014 18:36:18 +0200 > Alexander Graf <agraf@suse.de> wrote: >> On 24.04.14 17:34, Jens Freimann wrote: >>> From: Thomas Huth <thuth@linux.vnet.ibm.com> >>> >>> 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? You go ahead and do one. If you can think of a good way to combine mmu_translate_pte and mmu_translate_seg into a single function I'd be happy to see that too :). With the RO flag identical the only differences left are the page mask (4k is implicit for QEMU's TLB, that's why it's omitted for normal PTEs) and the page fault injection. Do 1MB PTEs fault with a page fault or a segment fault? If they fault with a page fault, it's probably the best to just have one mmu_translate_pte() function with a mask parameter. Alex
On Fri, 25 Apr 2014 14:36:11 +0200 Alexander Graf <agraf@suse.de> wrote: > > On 25.04.14 14:15, Thomas Huth wrote: > > > > On Thu, 24 Apr 2014 18:36:18 +0200 > > Alexander Graf <agraf@suse.de> wrote: [...] > >> 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? > > You go ahead and do one. If you can think of a good way to combine > mmu_translate_pte and mmu_translate_seg into a single function I'd be > happy to see that too :). > > With the RO flag identical the only differences left are the page mask > (4k is implicit for QEMU's TLB, that's why it's omitted for normal PTEs) > and the page fault injection. Do 1MB PTEs fault with a page fault or a > segment fault? If they fault with a page fault, it's probably the best > to just have one mmu_translate_pte() function with a mask parameter. No, you always get segment translation exceptions instead. And there is another difference: The entry-is-invalid bit is at a different location (_PAGE_INVALID vs. _SEGMENT_ENTRY_INV). So I think it's nicer to keep the functions separated instead of adding too much if-statements or functions parameters there, ok? Thomas
On 25.04.14 14:56, Thomas Huth wrote: > On Fri, 25 Apr 2014 14:36:11 +0200 > Alexander Graf <agraf@suse.de> wrote: >> On 25.04.14 14:15, Thomas Huth wrote: >>> On Thu, 24 Apr 2014 18:36:18 +0200 >>> Alexander Graf <agraf@suse.de> wrote: > [...] >>>> 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? >> You go ahead and do one. If you can think of a good way to combine >> mmu_translate_pte and mmu_translate_seg into a single function I'd be >> happy to see that too :). >> >> With the RO flag identical the only differences left are the page mask >> (4k is implicit for QEMU's TLB, that's why it's omitted for normal PTEs) >> and the page fault injection. Do 1MB PTEs fault with a page fault or a >> segment fault? If they fault with a page fault, it's probably the best >> to just have one mmu_translate_pte() function with a mask parameter. > No, you always get segment translation exceptions instead. And there is > another difference: The entry-is-invalid bit is at a different location > (_PAGE_INVALID vs. _SEGMENT_ENTRY_INV). So I think it's nicer to keep > the functions separated instead of adding too much if-statements or > functions parameters there, ok? Yes, I agree. Alex
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? */ + *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,