Re: [qemu-s390x] [PATCH] s390: support EDAT-2 in mmu_translate_region

[David Hildenbrand]

On 16.07.19 14:34, Ilya Leoshkevich wrote:
> When debugging s390 linux kernel with qemu kvm gdbstub, dumping memory
> contents at addresses in range 0x80000000-0x100000000 results in an
> error or all zeroes being returned.
> 
> The problem appears to be that linux puts 2G page at that location,
> which qemu currently does not know about.
> 
> Check FC bit of Region-Third-Table Entry in mmu_translate_region, just
> like it's already done for FC bit of Segment-Table Entry in
> mmu_translate_segment.
> 
> Signed-off-by: Ilya Leoshkevich <address@hidden>
> Acked-by: Christian Borntraeger <address@hidden>
> ---
>  target/s390x/cpu.h        | 1 +
>  target/s390x/mmu_helper.c | 8 ++++++++
>  2 files changed, 9 insertions(+)
> 
> diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
> index a606547b4d..947553386f 100644
> --- a/target/s390x/cpu.h
> +++ b/target/s390x/cpu.h
> @@ -548,6 +548,7 @@ QEMU_BUILD_BUG_ON(sizeof(SysIB) != 4096);
>  #define ASCE_TABLE_LENGTH     0x03        /* region table length             
>  */
>  
>  #define REGION_ENTRY_ORIGIN   (~0xfffULL) /* region/segment table origin    
> */
> +#define REGION_ENTRY_FC       0x400       /* region format control          
> */
>  #define REGION_ENTRY_RO       0x200       /* region/segment protection bit  
> */
>  #define REGION_ENTRY_TF       0xc0        /* region/segment table offset    
> */
>  #define REGION_ENTRY_INV      0x20        /* invalid region table entry     
> */
> diff --git a/target/s390x/mmu_helper.c b/target/s390x/mmu_helper.c
> index 6e9c4d6151..76cf920cd2 100644
> --- a/target/s390x/mmu_helper.c
> +++ b/target/s390x/mmu_helper.c
> @@ -242,6 +242,14 @@ static int mmu_translate_region(CPUS390XState *env, 
> target_ulong vaddr,
>          return -1;
>      }
>  
> +    if (level == ASCE_TYPE_REGION3
> +        && (new_entry & REGION_ENTRY_FC) && (env->cregs[0] & CR0_EDAT)) {
> +        /* Decode EDAT-2 region frame absolute address (2GB page) */
> +        *raddr = (new_entry & 0xffffffff80000000ULL) | (vaddr & 0x7fffffff);
> +        PTE_DPRINTF("%s: REG=0x%" PRIx64 "\n", __func__, new_entry);
> +        return 0;
> +    }
> +
>      if (level == ASCE_TYPE_SEGMENT) {
>          return mmu_translate_segment(env, vaddr, asc, new_entry, raddr, 
> flags,
>                                       rw, exc);
> 

I have a patch series lying around that rewrites the whole mmu code in a 
non-recusrive
fasion and implements a set of features. There, I have


commit b3ae14d99a648fec3e503efa2f547886d40ab8c1
Author: David Hildenbrand <address@hidden>
Date:   Mon Jan 15 00:04:07 2018 +0100

    s390x/mmu: add EDAT2 translation support
    
    This only adds basic support to the MMU, but no EDAT2 support  for TCG
    guests.
    
    Signed-off-by: David Hildenbrand <address@hidden>

diff --git a/target/s390x/mmu_helper.c b/target/s390x/mmu_helper.c
index a294cd16f1..72025c4437 100644
--- a/target/s390x/mmu_helper.c
+++ b/target/s390x/mmu_helper.c
@@ -139,6 +139,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;
     uintptr_t ptr = asce & _ASCE_ORIGIN;
@@ -234,9 +235,16 @@ 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;
+            return 0;
+        }
         if (VADDR_SEGMENT_TL(vaddr) < (entry & REGION_ENTRY_TF) >> 6 ||
             VADDR_SEGMENT_TL(vaddr) > (entry & REGION_ENTRY_TL)) {
             return PGM_SEGMENT_TRANS;


So I think this patch is at least missing something.

How urgent is this? If this can wait, I can polish and send my series I have 
here
instead, which also implents
- IEP support
- access-exception-fetch/store-indication facility
- ESOP-1, ESOP-2

-- 

Thanks,

David / dhildenb