| Message ID | 20190926101627.23376-2-david@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | s390x/mmu: Implement more facilities | expand |
On 26.09.19 12:16, David Hildenbrand wrote: > This only adds basic support to the DAT translation, but no EDAT2 support > for TCG. E.g., the gdbstub under kvm uses this function, too, to > translate virtual addresses. > > Reviewed-by: Thomas Huth <thuth@redhat.com> > Signed-off-by: David Hildenbrand <david@redhat.com> > --- > target/s390x/mmu_helper.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/target/s390x/mmu_helper.c b/target/s390x/mmu_helper.c > index 6b34c4c7b4..54f54137ec 100644 > --- a/target/s390x/mmu_helper.c > +++ b/target/s390x/mmu_helper.c > @@ -120,6 +120,7 @@ static int mmu_translate_asce(CPUS390XState *env, target_ulong vaddr, > { > const bool edat1 = (env->cregs[0] & CR0_EDAT) && > s390_has_feat(S390_FEAT_EDAT); > + const bool edat2 = edat1 && s390_has_feat(S390_FEAT_EDAT_2); > const int asce_tl = asce & ASCE_TABLE_LENGTH; > const int asce_p = asce & ASCE_PRIVATE_SPACE; > hwaddr gaddr = asce & ASCE_ORIGIN; > @@ -219,9 +220,17 @@ static int mmu_translate_asce(CPUS390XState *env, target_ulong vaddr, > if ((entry & REGION_ENTRY_TT) != REGION_ENTRY_TT_REGION3) { > return PGM_TRANS_SPEC; > } > + if (edat2 && (entry & REGION3_ENTRY_CR) && asce_p) { > + return PGM_TRANS_SPEC; > + } > if (edat1 && (entry & REGION_ENTRY_P)) { > *flags &= ~PAGE_WRITE; > } > + if (edat2 && (entry & REGION3_ENTRY_FC)) { > + *raddr = (entry & REGION3_ENTRY_RFAA) | > + (vaddr & REGION3_ENTRY_RFAA); Messed up (vaddr & ~REGION3_ENTRY_RFAA) it is.
On 26/09/2019 12.18, David Hildenbrand wrote: > On 26.09.19 12:16, David Hildenbrand wrote: >> This only adds basic support to the DAT translation, but no EDAT2 support >> for TCG. E.g., the gdbstub under kvm uses this function, too, to >> translate virtual addresses. >> >> Reviewed-by: Thomas Huth <thuth@redhat.com> >> Signed-off-by: David Hildenbrand <david@redhat.com> >> --- >> target/s390x/mmu_helper.c | 9 +++++++++ >> 1 file changed, 9 insertions(+) >> >> diff --git a/target/s390x/mmu_helper.c b/target/s390x/mmu_helper.c >> index 6b34c4c7b4..54f54137ec 100644 >> --- a/target/s390x/mmu_helper.c >> +++ b/target/s390x/mmu_helper.c >> @@ -120,6 +120,7 @@ static int mmu_translate_asce(CPUS390XState *env, target_ulong vaddr, >> { >> const bool edat1 = (env->cregs[0] & CR0_EDAT) && >> s390_has_feat(S390_FEAT_EDAT); >> + const bool edat2 = edat1 && s390_has_feat(S390_FEAT_EDAT_2); >> const int asce_tl = asce & ASCE_TABLE_LENGTH; >> const int asce_p = asce & ASCE_PRIVATE_SPACE; >> hwaddr gaddr = asce & ASCE_ORIGIN; >> @@ -219,9 +220,17 @@ static int mmu_translate_asce(CPUS390XState *env, target_ulong vaddr, >> if ((entry & REGION_ENTRY_TT) != REGION_ENTRY_TT_REGION3) { >> return PGM_TRANS_SPEC; >> } >> + if (edat2 && (entry & REGION3_ENTRY_CR) && asce_p) { >> + return PGM_TRANS_SPEC; >> + } >> if (edat1 && (entry & REGION_ENTRY_P)) { >> *flags &= ~PAGE_WRITE; >> } >> + if (edat2 && (entry & REGION3_ENTRY_FC)) { >> + *raddr = (entry & REGION3_ENTRY_RFAA) | >> + (vaddr & REGION3_ENTRY_RFAA); > > Messed up > > (vaddr & ~REGION3_ENTRY_RFAA) > > it is. With that fix: Reviewed-by: Thomas Huth <thuth@redhat.com>
On 01.10.19 10:41, Thomas Huth wrote: > On 26/09/2019 12.18, David Hildenbrand wrote: >> On 26.09.19 12:16, David Hildenbrand wrote: >>> This only adds basic support to the DAT translation, but no EDAT2 support >>> for TCG. E.g., the gdbstub under kvm uses this function, too, to >>> translate virtual addresses. >>> >>> Reviewed-by: Thomas Huth <thuth@redhat.com> >>> Signed-off-by: David Hildenbrand <david@redhat.com> >>> --- >>> target/s390x/mmu_helper.c | 9 +++++++++ >>> 1 file changed, 9 insertions(+) >>> >>> diff --git a/target/s390x/mmu_helper.c b/target/s390x/mmu_helper.c >>> index 6b34c4c7b4..54f54137ec 100644 >>> --- a/target/s390x/mmu_helper.c >>> +++ b/target/s390x/mmu_helper.c >>> @@ -120,6 +120,7 @@ static int mmu_translate_asce(CPUS390XState *env, target_ulong vaddr, >>> { >>> const bool edat1 = (env->cregs[0] & CR0_EDAT) && >>> s390_has_feat(S390_FEAT_EDAT); >>> + const bool edat2 = edat1 && s390_has_feat(S390_FEAT_EDAT_2); >>> const int asce_tl = asce & ASCE_TABLE_LENGTH; >>> const int asce_p = asce & ASCE_PRIVATE_SPACE; >>> hwaddr gaddr = asce & ASCE_ORIGIN; >>> @@ -219,9 +220,17 @@ static int mmu_translate_asce(CPUS390XState *env, target_ulong vaddr, >>> if ((entry & REGION_ENTRY_TT) != REGION_ENTRY_TT_REGION3) { >>> return PGM_TRANS_SPEC; >>> } >>> + if (edat2 && (entry & REGION3_ENTRY_CR) && asce_p) { >>> + return PGM_TRANS_SPEC; >>> + } >>> if (edat1 && (entry & REGION_ENTRY_P)) { >>> *flags &= ~PAGE_WRITE; >>> } >>> + if (edat2 && (entry & REGION3_ENTRY_FC)) { >>> + *raddr = (entry & REGION3_ENTRY_RFAA) | >>> + (vaddr & REGION3_ENTRY_RFAA); >> >> Messed up >> >> (vaddr & ~REGION3_ENTRY_RFAA) >> >> it is. > > With that fix: > > Reviewed-by: Thomas Huth <thuth@redhat.com> > BTW, this change explains the different order of checks you mentioned. I now have here: diff --git a/target/s390x/mmu_helper.c b/target/s390x/mmu_helper.c index dc33c63b1d..dcbffb682f 100644 --- a/target/s390x/mmu_helper.c +++ b/target/s390x/mmu_helper.c @@ -120,6 +120,7 @@ static int mmu_translate_asce(CPUS390XState *env, target_ulong vaddr, { const bool edat1 = (env->cregs[0] & CR0_EDAT) && s390_has_feat(S390_FEAT_EDAT); + const bool edat2 = edat1 && s390_has_feat(S390_FEAT_EDAT_2); const int asce_tl = asce & ASCE_TABLE_LENGTH; const int asce_p = asce & ASCE_PRIVATE_SPACE; hwaddr gaddr = asce & ASCE_ORIGIN; @@ -217,6 +218,17 @@ static int mmu_translate_asce(CPUS390XState *env, target_ulong vaddr, if ((entry & REGION_ENTRY_TT) != REGION_ENTRY_TT_REGION3) { return PGM_TRANS_SPEC; } + if (edat2 && (entry & REGION3_ENTRY_CR) && asce_p) { + return PGM_TRANS_SPEC; + } + if (edat2 && (entry & REGION3_ENTRY_FC)) { + if (entry & REGION_ENTRY_P) { + *flags &= ~PAGE_WRITE; + } + *raddr = (entry & REGION3_ENTRY_RFAA) | + (vaddr & ~REGION3_ENTRY_RFAA); + return 0; + } if (VADDR_SEGMENT_TL(vaddr) < (entry & REGION_ENTRY_TF) >> 6 || VADDR_SEGMENT_TL(vaddr) > (entry & REGION_ENTRY_TL)) { return PGM_SEGMENT_TRANS;
On 01/10/2019 10.51, David Hildenbrand wrote: > On 01.10.19 10:41, Thomas Huth wrote: >> On 26/09/2019 12.18, David Hildenbrand wrote: >>> On 26.09.19 12:16, David Hildenbrand wrote: >>>> This only adds basic support to the DAT translation, but no EDAT2 support >>>> for TCG. E.g., the gdbstub under kvm uses this function, too, to >>>> translate virtual addresses. >>>> >>>> Reviewed-by: Thomas Huth <thuth@redhat.com> >>>> Signed-off-by: David Hildenbrand <david@redhat.com> >>>> --- >>>> target/s390x/mmu_helper.c | 9 +++++++++ >>>> 1 file changed, 9 insertions(+) >>>> >>>> diff --git a/target/s390x/mmu_helper.c b/target/s390x/mmu_helper.c >>>> index 6b34c4c7b4..54f54137ec 100644 >>>> --- a/target/s390x/mmu_helper.c >>>> +++ b/target/s390x/mmu_helper.c >>>> @@ -120,6 +120,7 @@ static int mmu_translate_asce(CPUS390XState *env, target_ulong vaddr, >>>> { >>>> const bool edat1 = (env->cregs[0] & CR0_EDAT) && >>>> s390_has_feat(S390_FEAT_EDAT); >>>> + const bool edat2 = edat1 && s390_has_feat(S390_FEAT_EDAT_2); >>>> const int asce_tl = asce & ASCE_TABLE_LENGTH; >>>> const int asce_p = asce & ASCE_PRIVATE_SPACE; >>>> hwaddr gaddr = asce & ASCE_ORIGIN; >>>> @@ -219,9 +220,17 @@ static int mmu_translate_asce(CPUS390XState *env, target_ulong vaddr, >>>> if ((entry & REGION_ENTRY_TT) != REGION_ENTRY_TT_REGION3) { >>>> return PGM_TRANS_SPEC; >>>> } >>>> + if (edat2 && (entry & REGION3_ENTRY_CR) && asce_p) { >>>> + return PGM_TRANS_SPEC; >>>> + } >>>> if (edat1 && (entry & REGION_ENTRY_P)) { >>>> *flags &= ~PAGE_WRITE; >>>> } >>>> + if (edat2 && (entry & REGION3_ENTRY_FC)) { >>>> + *raddr = (entry & REGION3_ENTRY_RFAA) | >>>> + (vaddr & REGION3_ENTRY_RFAA); >>> >>> Messed up >>> >>> (vaddr & ~REGION3_ENTRY_RFAA) >>> >>> it is. >> >> With that fix: >> >> Reviewed-by: Thomas Huth <thuth@redhat.com> >> > > BTW, this change explains the different order of checks you mentioned. I now have here: > > diff --git a/target/s390x/mmu_helper.c b/target/s390x/mmu_helper.c > index dc33c63b1d..dcbffb682f 100644 > --- a/target/s390x/mmu_helper.c > +++ b/target/s390x/mmu_helper.c > @@ -120,6 +120,7 @@ static int mmu_translate_asce(CPUS390XState *env, target_ulong vaddr, > { > const bool edat1 = (env->cregs[0] & CR0_EDAT) && > s390_has_feat(S390_FEAT_EDAT); > + const bool edat2 = edat1 && s390_has_feat(S390_FEAT_EDAT_2); > const int asce_tl = asce & ASCE_TABLE_LENGTH; > const int asce_p = asce & ASCE_PRIVATE_SPACE; > hwaddr gaddr = asce & ASCE_ORIGIN; > @@ -217,6 +218,17 @@ static int mmu_translate_asce(CPUS390XState *env, target_ulong vaddr, > if ((entry & REGION_ENTRY_TT) != REGION_ENTRY_TT_REGION3) { > return PGM_TRANS_SPEC; > } > + if (edat2 && (entry & REGION3_ENTRY_CR) && asce_p) { > + return PGM_TRANS_SPEC; > + } > + if (edat2 && (entry & REGION3_ENTRY_FC)) { > + if (entry & REGION_ENTRY_P) { > + *flags &= ~PAGE_WRITE; > + } > + *raddr = (entry & REGION3_ENTRY_RFAA) | > + (vaddr & ~REGION3_ENTRY_RFAA); > + return 0; > + } > if (VADDR_SEGMENT_TL(vaddr) < (entry & REGION_ENTRY_TF) >> 6 || > VADDR_SEGMENT_TL(vaddr) > (entry & REGION_ENTRY_TL)) { > return PGM_SEGMENT_TRANS; Ah, ok, and the *flags have to be set first, of course. So better keep it the original way round in your other patch. Thomas
On 01.10.19 10:55, Thomas Huth wrote: > On 01/10/2019 10.51, David Hildenbrand wrote: >> On 01.10.19 10:41, Thomas Huth wrote: >>> On 26/09/2019 12.18, David Hildenbrand wrote: >>>> On 26.09.19 12:16, David Hildenbrand wrote: >>>>> This only adds basic support to the DAT translation, but no EDAT2 support >>>>> for TCG. E.g., the gdbstub under kvm uses this function, too, to >>>>> translate virtual addresses. >>>>> >>>>> Reviewed-by: Thomas Huth <thuth@redhat.com> >>>>> Signed-off-by: David Hildenbrand <david@redhat.com> >>>>> --- >>>>> target/s390x/mmu_helper.c | 9 +++++++++ >>>>> 1 file changed, 9 insertions(+) >>>>> >>>>> diff --git a/target/s390x/mmu_helper.c b/target/s390x/mmu_helper.c >>>>> index 6b34c4c7b4..54f54137ec 100644 >>>>> --- a/target/s390x/mmu_helper.c >>>>> +++ b/target/s390x/mmu_helper.c >>>>> @@ -120,6 +120,7 @@ static int mmu_translate_asce(CPUS390XState *env, target_ulong vaddr, >>>>> { >>>>> const bool edat1 = (env->cregs[0] & CR0_EDAT) && >>>>> s390_has_feat(S390_FEAT_EDAT); >>>>> + const bool edat2 = edat1 && s390_has_feat(S390_FEAT_EDAT_2); >>>>> const int asce_tl = asce & ASCE_TABLE_LENGTH; >>>>> const int asce_p = asce & ASCE_PRIVATE_SPACE; >>>>> hwaddr gaddr = asce & ASCE_ORIGIN; >>>>> @@ -219,9 +220,17 @@ static int mmu_translate_asce(CPUS390XState *env, target_ulong vaddr, >>>>> if ((entry & REGION_ENTRY_TT) != REGION_ENTRY_TT_REGION3) { >>>>> return PGM_TRANS_SPEC; >>>>> } >>>>> + if (edat2 && (entry & REGION3_ENTRY_CR) && asce_p) { >>>>> + return PGM_TRANS_SPEC; >>>>> + } >>>>> if (edat1 && (entry & REGION_ENTRY_P)) { >>>>> *flags &= ~PAGE_WRITE; >>>>> } >>>>> + if (edat2 && (entry & REGION3_ENTRY_FC)) { >>>>> + *raddr = (entry & REGION3_ENTRY_RFAA) | >>>>> + (vaddr & REGION3_ENTRY_RFAA); >>>> >>>> Messed up >>>> >>>> (vaddr & ~REGION3_ENTRY_RFAA) >>>> >>>> it is. >>> >>> With that fix: >>> >>> Reviewed-by: Thomas Huth <thuth@redhat.com> >>> >> >> BTW, this change explains the different order of checks you mentioned. I now have here: >> >> diff --git a/target/s390x/mmu_helper.c b/target/s390x/mmu_helper.c >> index dc33c63b1d..dcbffb682f 100644 >> --- a/target/s390x/mmu_helper.c >> +++ b/target/s390x/mmu_helper.c >> @@ -120,6 +120,7 @@ static int mmu_translate_asce(CPUS390XState *env, target_ulong vaddr, >> { >> const bool edat1 = (env->cregs[0] & CR0_EDAT) && >> s390_has_feat(S390_FEAT_EDAT); >> + const bool edat2 = edat1 && s390_has_feat(S390_FEAT_EDAT_2); >> const int asce_tl = asce & ASCE_TABLE_LENGTH; >> const int asce_p = asce & ASCE_PRIVATE_SPACE; >> hwaddr gaddr = asce & ASCE_ORIGIN; >> @@ -217,6 +218,17 @@ static int mmu_translate_asce(CPUS390XState *env, target_ulong vaddr, >> if ((entry & REGION_ENTRY_TT) != REGION_ENTRY_TT_REGION3) { >> return PGM_TRANS_SPEC; >> } >> + if (edat2 && (entry & REGION3_ENTRY_CR) && asce_p) { >> + return PGM_TRANS_SPEC; >> + } >> + if (edat2 && (entry & REGION3_ENTRY_FC)) { >> + if (entry & REGION_ENTRY_P) { >> + *flags &= ~PAGE_WRITE; >> + } >> + *raddr = (entry & REGION3_ENTRY_RFAA) | >> + (vaddr & ~REGION3_ENTRY_RFAA); >> + return 0; >> + } >> if (VADDR_SEGMENT_TL(vaddr) < (entry & REGION_ENTRY_TF) >> 6 || >> VADDR_SEGMENT_TL(vaddr) > (entry & REGION_ENTRY_TL)) { >> return PGM_SEGMENT_TRANS; > > Ah, ok, and the *flags have to be set first, of course. So better keep > it the original way round in your other patch. I can just move it in this patch, then it's clearer why a different order is needed.
diff --git a/target/s390x/mmu_helper.c b/target/s390x/mmu_helper.c index 6b34c4c7b4..54f54137ec 100644 --- a/target/s390x/mmu_helper.c +++ b/target/s390x/mmu_helper.c @@ -120,6 +120,7 @@ static int mmu_translate_asce(CPUS390XState *env, target_ulong vaddr, { const bool edat1 = (env->cregs[0] & CR0_EDAT) && s390_has_feat(S390_FEAT_EDAT); + const bool edat2 = edat1 && s390_has_feat(S390_FEAT_EDAT_2); const int asce_tl = asce & ASCE_TABLE_LENGTH; const int asce_p = asce & ASCE_PRIVATE_SPACE; hwaddr gaddr = asce & ASCE_ORIGIN; @@ -219,9 +220,17 @@ static int mmu_translate_asce(CPUS390XState *env, target_ulong vaddr, if ((entry & REGION_ENTRY_TT) != REGION_ENTRY_TT_REGION3) { return PGM_TRANS_SPEC; } + if (edat2 && (entry & REGION3_ENTRY_CR) && asce_p) { + return PGM_TRANS_SPEC; + } if (edat1 && (entry & REGION_ENTRY_P)) { *flags &= ~PAGE_WRITE; } + if (edat2 && (entry & REGION3_ENTRY_FC)) { + *raddr = (entry & REGION3_ENTRY_RFAA) | + (vaddr & REGION3_ENTRY_RFAA); + return 0; + } if (VADDR_SEGMENT_TL(vaddr) < (entry & REGION_ENTRY_TF) >> 6 || VADDR_SEGMENT_TL(vaddr) > (entry & REGION_ENTRY_TL)) { return PGM_SEGMENT_TRANS;