[Qemu-devel] [PATCH] s390: support EDAT-2 in mmu_translate_region

[Qemu-devel] [PATCH] s390: support EDAT-2 in mmu_translate_region

[Ilya Leoshkevich]

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 <iii@linux.ibm.com>
Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 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);
-- 
2.21.0

Re: [Qemu-devel] [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 <iii@linux.ibm.com>
> Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
> ---
>  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 <david@redhat.com>
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 <david@redhat.com>

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

Re: [Qemu-devel] [PATCH] s390: support EDAT-2 in mmu_translate_region

[Ilya Leoshkevich]

> Am 16.07.2019 um 14:41 schrieb David Hildenbrand <david@redhat.com>:
> 
> 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

This is not urgent, I can live with my patch for now.
It’s good to know that proper EDAT-2 support is being worked on.

Thanks!
Ilya

Re: [Qemu-devel] [PATCH] s390: support EDAT-2 in mmu_translate_region

[Cornelia Huck]

On Tue, 16 Jul 2019 14:52:03 +0200
Ilya Leoshkevich <iii@linux.ibm.com> wrote:

> > Am 16.07.2019 um 14:41 schrieb David Hildenbrand <david@redhat.com>:
> > 
> > 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  
> 
> This is not urgent, I can live with my patch for now.
> It’s good to know that proper EDAT-2 support is being worked on.
> 
> Thanks!
> Ilya

Ok, so I will not queue this patch right now (I assume you're fine with
keeping this locally for now?), but wait for David's series for 4.2.

Sounds reasonable?

Re: [Qemu-devel] [PATCH] s390: support EDAT-2 in mmu_translate_region

[Ilya Leoshkevich]

> Am 16.07.2019 um 15:04 schrieb Cornelia Huck <cohuck@redhat.com>:
> 
> On Tue, 16 Jul 2019 14:52:03 +0200
> Ilya Leoshkevich <iii@linux.ibm.com> wrote:
> 
>>> Am 16.07.2019 um 14:41 schrieb David Hildenbrand <david@redhat.com>:
>>> 
>>> 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  
>> 
>> This is not urgent, I can live with my patch for now.
>> It’s good to know that proper EDAT-2 support is being worked on.
>> 
>> Thanks!
>> Ilya
> 
> Ok, so I will not queue this patch right now (I assume you're fine with
> keeping this locally for now?), but wait for David's series for 4.2.
> 
> Sounds reasonable?

Yes, that sounds good.

Re: [Qemu-devel] [PATCH] s390: support EDAT-2 in mmu_translate_region

[Christian Borntraeger]

On 16.07.19 15:04, Cornelia Huck wrote:
> On Tue, 16 Jul 2019 14:52:03 +0200
> Ilya Leoshkevich <iii@linux.ibm.com> wrote:
> 
>>> Am 16.07.2019 um 14:41 schrieb David Hildenbrand <david@redhat.com>:
>>>
>>> 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  
>>
>> This is not urgent, I can live with my patch for now.
>> It’s good to know that proper EDAT-2 support is being worked on.
>>
>> Thanks!
>> Ilya
> 
> Ok, so I will not queue this patch right now (I assume you're fine with
> keeping this locally for now?), but wait for David's series for 4.2.
> 
> Sounds reasonable?

While not complete, Ilyas  patch clearly improves the situation (and it is pretty
similar to the EDAT-1 support). 
I think the question is: are there other instruction that we emulate in qemu via the
page table walker even for KVM? I believe we always go via the kvm memory ioctl for
page table  access via instructions so the patch is not critical for KVM. So unless
we have something I think Connys proposal is reasonable.

Re: [Qemu-devel] [PATCH] s390: support EDAT-2 in mmu_translate_region

[David Hildenbrand]

On 16.07.19 15:11, Christian Borntraeger wrote:
> 
> On 16.07.19 15:04, Cornelia Huck wrote:
>> On Tue, 16 Jul 2019 14:52:03 +0200
>> Ilya Leoshkevich <iii@linux.ibm.com> wrote:
>>
>>>> Am 16.07.2019 um 14:41 schrieb David Hildenbrand <david@redhat.com>:
>>>>
>>>> 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  
>>>
>>> This is not urgent, I can live with my patch for now.
>>> It’s good to know that proper EDAT-2 support is being worked on.
>>>
>>> Thanks!
>>> Ilya
>>
>> Ok, so I will not queue this patch right now (I assume you're fine with
>> keeping this locally for now?), but wait for David's series for 4.2.
>>
>> Sounds reasonable?
> 
> While not complete, Ilyas  patch clearly improves the situation (and it is pretty
> similar to the EDAT-1 support). 
> I think the question is: are there other instruction that we emulate in qemu via the
> page table walker even for KVM? I believe we always go via the kvm memory ioctl for
> page table  access via instructions so the patch is not critical for KVM. So unless
> we have something I think Connys proposal is reasonable.

Yes, I remember we always go via the ioctl. Only debug memory accesses
take this path for KVM.

-- 

Thanks,

David / dhildenb