[PATCH v1 0/5] s390x/mmu: Implement more facilities

[PATCH v1 0/5] s390x/mmu: Implement more facilities

[David Hildenbrand]

This is the follow up of:
    [PATCH-for-4.2 v1 0/9] s390x: MMU changes and extensions
Without the general MMU rework. It's based on:
    [PATCH v2 0/7] s390x/mmu: DAT translation rewrite

This series adds adds EDAT2 MMU support, and implements/indicates related
facilities (ESOP-1, ESOP-2, IEP, ...) for TCG. The QEMU CPU model is
updated.

IEP under QEMU TCG seems to work just fine, when eabling it via the "max"
CPU model - via kvm unit tests:
    t460s: ~/git/kvm-unit-tests master $ ./s390x-run s390x/iep.elf -cpu max
    [...]
    PASS: iep: iep protection: Program interrupt: expected(4) == received(4)
    SUMMARY: 1 tests

    EXIT: STATUS=1

Changes since "[PATCH-for-4.2 v1 0/9] s390x: MMU changes and extensions":
- "s390x/mmu: Add EDAT2 translation support"
-- Fix vaddr offset within 2GB page
- "s390x/mmu: Implement ESOP-2 and ..."
-- Squashed two patches as requested.
-- Also set ilen to "2" in case of MMU_INST_FETCH on mmu_translate_real
- "s390x/mmu: Implement Instruction-Execution-Protection Facility"
-- Make sure s390_cpu_get_phys_page_debug() doesn't choke on IEP
- "s390x/cpumodel: Add new TCG features to QEMU cpu model"
-- Add comment "features introduced after the z13"

Cc: Ilya Leoshkevich <iii@linux.ibm.com>

David Hildenbrand (5):
  s390x/mmu: Add EDAT2 translation support
  s390x/mmu: Implement ESOP-2 and
    access-exception-fetch/store-indication facility
  s390x/mmu: Implement Instruction-Execution-Protection Facility
  s390x/cpumodel: Prepare for changes of QEMU model
  s390x/cpumodel: Add new TCG features to QEMU cpu model

 hw/s390x/s390-virtio-ccw.c  |  2 ++
 target/s390x/cpu.h          |  1 +
 target/s390x/gen-features.c | 11 +++++++++-
 target/s390x/helper.c       |  6 +++++-
 target/s390x/mmu_helper.c   | 42 +++++++++++++++++++++++++++++++++++--
 5 files changed, 58 insertions(+), 4 deletions(-)

-- 
2.21.0

[PATCH v1 1/5] s390x/mmu: Add EDAT2 translation support

[David Hildenbrand]

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);
+            return 0;
+        }
         if (VADDR_SEGMENT_TL(vaddr) < (entry & REGION_ENTRY_TF) >> 6 ||
             VADDR_SEGMENT_TL(vaddr) > (entry & REGION_ENTRY_TL)) {
             return PGM_SEGMENT_TRANS;
-- 
2.21.0

[PATCH v1 2/5] s390x/mmu: Implement ESOP-2 and access-exception-fetch/store-indication facility

[David Hildenbrand]

We already implement ESOP-1. For ESOP-2, we only have to indicate all
protection exceptions properly. Due to EDAT-1, we already indicate DAT
exceptions properly. We don't trigger KCP/ALCP/IEP exceptions yet.

So all we have to do is set the TEID (TEC) to the right values
(bit 56, 60, 61) in case of LAP.

We don't have any side-effects (e.g., no guarded-storage facility),
therefore, bit 64 of the TEID (TEC) is always 0.

We always have to indicate whether it is a fetch or a store for all access
exceptions. This is only missing for LAP exceptions.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/mmu_helper.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/target/s390x/mmu_helper.c b/target/s390x/mmu_helper.c
index 54f54137ec..8abc5d31d8 100644
--- a/target/s390x/mmu_helper.c
+++ b/target/s390x/mmu_helper.c
@@ -384,7 +384,9 @@ int mmu_translate(CPUS390XState *env, target_ulong vaddr, int rw, uint64_t asc,
         *flags |= PAGE_WRITE_INV;
         if (is_low_address(vaddr) && rw == MMU_DATA_STORE) {
             if (exc) {
-                trigger_access_exception(env, PGM_PROTECTION, ILEN_AUTO, 0);
+                /* LAP sets bit 56 */
+                tec |= 0x80;
+                trigger_access_exception(env, PGM_PROTECTION, ilen, tec);
             }
             return -EACCES;
         }
@@ -540,6 +542,10 @@ void s390_cpu_virt_mem_handle_exc(S390CPU *cpu, uintptr_t ra)
 int mmu_translate_real(CPUS390XState *env, target_ulong raddr, int rw,
                        target_ulong *addr, int *flags)
 {
+    /* Code accesses have an undefined ilc, let's use 2 bytes. */
+    const int ilen = (rw == MMU_INST_FETCH) ? 2 : ILEN_AUTO;
+    uint64_t tec = (raddr & TARGET_PAGE_MASK) |
+                   (rw == MMU_DATA_STORE ? FS_WRITE : FS_READ);
     const bool lowprot_enabled = env->cregs[0] & CR0_LOWPROT;
 
     *flags = PAGE_READ | PAGE_WRITE | PAGE_EXEC;
@@ -547,7 +553,9 @@ int mmu_translate_real(CPUS390XState *env, target_ulong raddr, int rw,
         /* see comment in mmu_translate() how this works */
         *flags |= PAGE_WRITE_INV;
         if (is_low_address(raddr) && rw == MMU_DATA_STORE) {
-            trigger_access_exception(env, PGM_PROTECTION, ILEN_AUTO, 0);
+            /* LAP sets bit 56 */
+            tec |= 0x80;
+            trigger_access_exception(env, PGM_PROTECTION, ilen, tec);
             return -EACCES;
         }
     }
-- 
2.21.0

[PATCH v1 3/5] s390x/mmu: Implement Instruction-Execution-Protection Facility

[David Hildenbrand]

IEP support in the mmu is fairly easy. Set the right permissions for TLB
entries and properly report an exception.

Make sure to handle EDAT-2 by setting bit 56/60/61 of the TEID (TEC) to
the right values.

Let's keep s390_cpu_get_phys_page_debug() working even if IEP is
active. Switch MMU_DATA_LOAD - this has no other effects any more as the
ASC to be used is now fully selected outside of mmu_translate().

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/cpu.h        |  1 +
 target/s390x/helper.c     |  6 +++++-
 target/s390x/mmu_helper.c | 21 +++++++++++++++++++++
 3 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index 690b94c8ea..6eb7c07013 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -315,6 +315,7 @@ extern const VMStateDescription vmstate_s390_cpu;
 #define CR0_EDAT                0x0000000000800000ULL
 #define CR0_AFP                 0x0000000000040000ULL
 #define CR0_VECTOR              0x0000000000020000ULL
+#define CR0_IEP                 0x0000000000100000ULL
 #define CR0_EMERGENCY_SIGNAL_SC 0x0000000000004000ULL
 #define CR0_EXTERNAL_CALL_SC    0x0000000000002000ULL
 #define CR0_CKC_SC              0x0000000000000800ULL
diff --git a/target/s390x/helper.c b/target/s390x/helper.c
index 948c0398d4..bf503b56ee 100644
--- a/target/s390x/helper.c
+++ b/target/s390x/helper.c
@@ -63,7 +63,11 @@ hwaddr s390_cpu_get_phys_page_debug(CPUState *cs, vaddr vaddr)
         asc = PSW_ASC_PRIMARY;
     }
 
-    if (mmu_translate(env, vaddr, MMU_INST_FETCH, asc, &raddr, &prot, false)) {
+    /*
+     * We want to read code even if IEP is active. Use MMU_DATA_LOAD instead
+     * of MMU_INST_FETCH.
+     */
+    if (mmu_translate(env, vaddr, MMU_DATA_LOAD, asc, &raddr, &prot, false)) {
         return -1;
     }
     return raddr;
diff --git a/target/s390x/mmu_helper.c b/target/s390x/mmu_helper.c
index 8abc5d31d8..ff8b077f82 100644
--- a/target/s390x/mmu_helper.c
+++ b/target/s390x/mmu_helper.c
@@ -121,6 +121,8 @@ 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 bool iep = (env->cregs[0] & CR0_IEP) &&
+                     s390_has_feat(S390_FEAT_INSTRUCTION_EXEC_PROT);
     const int asce_tl = asce & ASCE_TABLE_LENGTH;
     const int asce_p = asce & ASCE_PRIVATE_SPACE;
     hwaddr gaddr = asce & ASCE_ORIGIN;
@@ -227,6 +229,9 @@ static int mmu_translate_asce(CPUS390XState *env, target_ulong vaddr,
             *flags &= ~PAGE_WRITE;
         }
         if (edat2 && (entry & REGION3_ENTRY_FC)) {
+            if (iep && (entry & REGION3_ENTRY_IEP)) {
+                *flags &= ~PAGE_EXEC;
+            }
             *raddr = (entry & REGION3_ENTRY_RFAA) |
                      (vaddr & REGION3_ENTRY_RFAA);
             return 0;
@@ -254,6 +259,9 @@ static int mmu_translate_asce(CPUS390XState *env, target_ulong vaddr,
             *flags &= ~PAGE_WRITE;
         }
         if (edat1 && (entry & SEGMENT_ENTRY_FC)) {
+            if (iep && (entry & SEGMENT_ENTRY_IEP)) {
+                *flags &= ~PAGE_EXEC;
+            }
             *raddr = (entry & SEGMENT_ENTRY_SFAA) |
                      (vaddr & ~SEGMENT_ENTRY_SFAA);
             return 0;
@@ -276,6 +284,9 @@ static int mmu_translate_asce(CPUS390XState *env, target_ulong vaddr,
     if (entry & PAGE_ENTRY_P) {
         *flags &= ~PAGE_WRITE;
     }
+    if (iep && (entry & PAGE_ENTRY_IEP)) {
+        *flags &= ~PAGE_EXEC;
+    }
 
     *raddr = entry & TARGET_PAGE_MASK;
     return 0;
@@ -434,6 +445,16 @@ int mmu_translate(CPUS390XState *env, target_ulong vaddr, int rw, uint64_t asc,
         return -1;
     }
 
+    /* check for Instruction-Execution-Protection */
+    if (unlikely(rw == MMU_INST_FETCH && !(*flags & PAGE_EXEC))) {
+        if (exc) {
+            /* IEP sets bit 56 and 61 */
+            tec |= 0x84;
+            trigger_access_exception(env, PGM_PROTECTION, ilen, tec);
+        }
+        return -1;
+    }
+
 nodat:
     /* Convert real address -> absolute address */
     *raddr = mmu_real2abs(env, *raddr);
-- 
2.21.0

[PATCH v1 4/5] s390x/cpumodel: Prepare for changes of QEMU model

[David Hildenbrand]

Setup the 4.1 compatibility model so we can add new features to the
LATEST model.

Reviewed-by: Cornelia Huck <cohuck@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 hw/s390x/s390-virtio-ccw.c  | 2 ++
 target/s390x/gen-features.c | 6 +++++-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 8bfb6684cb..a213f529ec 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -674,7 +674,9 @@ DEFINE_CCW_MACHINE(4_2, "4.2", true);
 
 static void ccw_machine_4_1_instance_options(MachineState *machine)
 {
+    static const S390FeatInit qemu_cpu_feat = { S390_FEAT_LIST_QEMU_V4_1 };
     ccw_machine_4_2_instance_options(machine);
+    s390_set_qemu_cpu_model(0x2964, 13, 2, qemu_cpu_feat);
 }
 
 static void ccw_machine_4_1_class_options(MachineClass *mc)
diff --git a/target/s390x/gen-features.c b/target/s390x/gen-features.c
index 49a650ac52..7e82f2f004 100644
--- a/target/s390x/gen-features.c
+++ b/target/s390x/gen-features.c
@@ -698,11 +698,14 @@ static uint16_t qemu_V4_0[] = {
     S390_FEAT_ZPCI,
 };
 
-static uint16_t qemu_LATEST[] = {
+static uint16_t qemu_V4_1[] = {
     S390_FEAT_STFLE_53,
     S390_FEAT_VECTOR,
 };
 
+static uint16_t qemu_LATEST[] = {
+};
+
 /* add all new definitions before this point */
 static uint16_t qemu_MAX[] = {
     /* generates a dependency warning, leave it out for now */
@@ -824,6 +827,7 @@ static FeatGroupDefSpec QemuFeatDef[] = {
     QEMU_FEAT_INITIALIZER(V2_11),
     QEMU_FEAT_INITIALIZER(V3_1),
     QEMU_FEAT_INITIALIZER(V4_0),
+    QEMU_FEAT_INITIALIZER(V4_1),
     QEMU_FEAT_INITIALIZER(LATEST),
     QEMU_FEAT_INITIALIZER(MAX),
 };
-- 
2.21.0

[PATCH v1 5/5] s390x/cpumodel: Add new TCG features to QEMU cpu model

[David Hildenbrand]

We now implement a bunch of new facilities we can properly indicate.

ESOP-1/ESOP-2 handling is discussed in the PoP Chafter 3-15
("Suppression on Protection"). The "Basic suppression-on-protection (SOP)
facility" is a core part of z/Architecture without a facility
indication. ESOP-2 is indicated by ESOP-1 + Side-effect facility
("ESOP-2"). Besides ESOP-2, the side-effect facility is only relevant for
the guarded-storage facility (we don't implement).

S390_ESOP:
- We indicate DAT exeptions by setting bit 61 of the TEID (TEC) to 1 and
  bit 60 to zero. We don't trigger ALCP exceptions yet. Also, we set
  bit 0-51 and bit 62/63 to the right values.
S390_ACCESS_EXCEPTION_FS_INDICATION:
- The TEID (TEC) properly indicates in bit 52/53 on any access if it was
  a fetch or a store
S390_SIDE_EFFECT_ACCESS_ESOP2:
- We have no side-effect accesses (esp., we don't implement the
  guarded-storage faciliy), we correctly set bit 64 of the TEID (TEC) to
  0 (no side-effect).
- ESOP2: We properly set bit 56, 60, 61 in the TEID (TEC) to indicate the
  type of protection. We don't trigger KCP/ALCP exceptions yet.
S390_INSTRUCTION_EXEC_PROT:
- The MMU properly detects and indicates the exception on instruction fetches
- Protected TLB entries will never get PAGE_EXEC set.

There is no need to fake the abscence of any of the facilities - without
the facilities, some bits of the TEID (TEC) are simply unpredictable.

As IEP was added with z14 and we currently implement a z13, add it to
the MAX model instead.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/gen-features.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/target/s390x/gen-features.c b/target/s390x/gen-features.c
index 7e82f2f004..6278845b12 100644
--- a/target/s390x/gen-features.c
+++ b/target/s390x/gen-features.c
@@ -704,12 +704,17 @@ static uint16_t qemu_V4_1[] = {
 };
 
 static uint16_t qemu_LATEST[] = {
+    S390_FEAT_ACCESS_EXCEPTION_FS_INDICATION,
+    S390_FEAT_SIDE_EFFECT_ACCESS_ESOP2,
+    S390_FEAT_ESOP,
 };
 
 /* add all new definitions before this point */
 static uint16_t qemu_MAX[] = {
     /* generates a dependency warning, leave it out for now */
     S390_FEAT_MSA_EXT_5,
+    /* features introduced after the z13 */
+    S390_FEAT_INSTRUCTION_EXEC_PROT,
 };
 
 /****** END FEATURE DEFS ******/
-- 
2.21.0

Re: [PATCH v1 1/5] s390x/mmu: Add EDAT2 translation support

[David Hildenbrand]

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.


-- 

Thanks,

David / dhildenb

Re: [PATCH v1 2/5] s390x/mmu: Implement ESOP-2 and access-exception-fetch/store-indication facility

[David Hildenbrand]

On 26.09.19 12:16, David Hildenbrand wrote:
> We already implement ESOP-1. For ESOP-2, we only have to indicate all
> protection exceptions properly. Due to EDAT-1, we already indicate DAT
> exceptions properly. We don't trigger KCP/ALCP/IEP exceptions yet.
> 
> So all we have to do is set the TEID (TEC) to the right values
> (bit 56, 60, 61) in case of LAP.
> 
> We don't have any side-effects (e.g., no guarded-storage facility),
> therefore, bit 64 of the TEID (TEC) is always 0.
> 
> We always have to indicate whether it is a fetch or a store for all access
> exceptions. This is only missing for LAP exceptions.
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  target/s390x/mmu_helper.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/target/s390x/mmu_helper.c b/target/s390x/mmu_helper.c
> index 54f54137ec..8abc5d31d8 100644
> --- a/target/s390x/mmu_helper.c
> +++ b/target/s390x/mmu_helper.c
> @@ -384,7 +384,9 @@ int mmu_translate(CPUS390XState *env, target_ulong vaddr, int rw, uint64_t asc,
>          *flags |= PAGE_WRITE_INV;
>          if (is_low_address(vaddr) && rw == MMU_DATA_STORE) {
>              if (exc) {
> -                trigger_access_exception(env, PGM_PROTECTION, ILEN_AUTO, 0);
> +                /* LAP sets bit 56 */
> +                tec |= 0x80;
> +                trigger_access_exception(env, PGM_PROTECTION, ilen, tec);
>              }
>              return -EACCES;
>          }
> @@ -540,6 +542,10 @@ void s390_cpu_virt_mem_handle_exc(S390CPU *cpu, uintptr_t ra)
>  int mmu_translate_real(CPUS390XState *env, target_ulong raddr, int rw,
>                         target_ulong *addr, int *flags)
>  {
> +    /* Code accesses have an undefined ilc, let's use 2 bytes. */
> +    const int ilen = (rw == MMU_INST_FETCH) ? 2 : ILEN_AUTO;
> +    uint64_t tec = (raddr & TARGET_PAGE_MASK) |
> +                   (rw == MMU_DATA_STORE ? FS_WRITE : FS_READ);
>      const bool lowprot_enabled = env->cregs[0] & CR0_LOWPROT;
>  
>      *flags = PAGE_READ | PAGE_WRITE | PAGE_EXEC;
> @@ -547,7 +553,9 @@ int mmu_translate_real(CPUS390XState *env, target_ulong raddr, int rw,
>          /* see comment in mmu_translate() how this works */
>          *flags |= PAGE_WRITE_INV;
>          if (is_low_address(raddr) && rw == MMU_DATA_STORE) {
> -            trigger_access_exception(env, PGM_PROTECTION, ILEN_AUTO, 0);
> +            /* LAP sets bit 56 */
> +            tec |= 0x80;
> +            trigger_access_exception(env, PGM_PROTECTION, ilen, tec);

I'll drop the ilen change from this patch again as it is handled
properly when filling the TLB and Richard is about to rework that either
way.

-- 

Thanks,

David / dhildenb

Re: [PATCH v1 1/5] s390x/mmu: Add EDAT2 translation support

[Thomas Huth]

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>

Re: [PATCH v1 2/5] s390x/mmu: Implement ESOP-2 and access-exception-fetch/store-indication facility

[Thomas Huth]

On 27/09/2019 14.30, David Hildenbrand wrote:
> On 26.09.19 12:16, David Hildenbrand wrote:
>> We already implement ESOP-1. For ESOP-2, we only have to indicate all
>> protection exceptions properly. Due to EDAT-1, we already indicate DAT
>> exceptions properly. We don't trigger KCP/ALCP/IEP exceptions yet.
>>
>> So all we have to do is set the TEID (TEC) to the right values
>> (bit 56, 60, 61) in case of LAP.
>>
>> We don't have any side-effects (e.g., no guarded-storage facility),
>> therefore, bit 64 of the TEID (TEC) is always 0.
>>
>> We always have to indicate whether it is a fetch or a store for all access
>> exceptions. This is only missing for LAP exceptions.
>>
>> Signed-off-by: David Hildenbrand <david@redhat.com>
>> ---
>>  target/s390x/mmu_helper.c | 12 ++++++++++--
>>  1 file changed, 10 insertions(+), 2 deletions(-)
>>
>> diff --git a/target/s390x/mmu_helper.c b/target/s390x/mmu_helper.c
>> index 54f54137ec..8abc5d31d8 100644
>> --- a/target/s390x/mmu_helper.c
>> +++ b/target/s390x/mmu_helper.c
>> @@ -384,7 +384,9 @@ int mmu_translate(CPUS390XState *env, target_ulong vaddr, int rw, uint64_t asc,
>>          *flags |= PAGE_WRITE_INV;
>>          if (is_low_address(vaddr) && rw == MMU_DATA_STORE) {
>>              if (exc) {
>> -                trigger_access_exception(env, PGM_PROTECTION, ILEN_AUTO, 0);
>> +                /* LAP sets bit 56 */
>> +                tec |= 0x80;
>> +                trigger_access_exception(env, PGM_PROTECTION, ilen, tec);
>>              }
>>              return -EACCES;
>>          }
>> @@ -540,6 +542,10 @@ void s390_cpu_virt_mem_handle_exc(S390CPU *cpu, uintptr_t ra)
>>  int mmu_translate_real(CPUS390XState *env, target_ulong raddr, int rw,
>>                         target_ulong *addr, int *flags)
>>  {
>> +    /* Code accesses have an undefined ilc, let's use 2 bytes. */
>> +    const int ilen = (rw == MMU_INST_FETCH) ? 2 : ILEN_AUTO;
>> +    uint64_t tec = (raddr & TARGET_PAGE_MASK) |
>> +                   (rw == MMU_DATA_STORE ? FS_WRITE : FS_READ);
>>      const bool lowprot_enabled = env->cregs[0] & CR0_LOWPROT;
>>  
>>      *flags = PAGE_READ | PAGE_WRITE | PAGE_EXEC;
>> @@ -547,7 +553,9 @@ int mmu_translate_real(CPUS390XState *env, target_ulong raddr, int rw,
>>          /* see comment in mmu_translate() how this works */
>>          *flags |= PAGE_WRITE_INV;
>>          if (is_low_address(raddr) && rw == MMU_DATA_STORE) {
>> -            trigger_access_exception(env, PGM_PROTECTION, ILEN_AUTO, 0);
>> +            /* LAP sets bit 56 */
>> +            tec |= 0x80;
>> +            trigger_access_exception(env, PGM_PROTECTION, ilen, tec);
> 
> I'll drop the ilen change from this patch again as it is handled
> properly when filling the TLB and Richard is about to rework that either
> way.
> 

Acked-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH v1 1/5] s390x/mmu: Add EDAT2 translation support

[David Hildenbrand]

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;


-- 

Thanks,

David / dhildenb

Re: [PATCH v1 1/5] s390x/mmu: Add EDAT2 translation support

[Thomas Huth]

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

Re: [PATCH v1 1/5] s390x/mmu: Add EDAT2 translation support

[David Hildenbrand]

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.

-- 

Thanks,

David / dhildenb

Re: [PATCH v1 3/5] s390x/mmu: Implement Instruction-Execution-Protection Facility

[Thomas Huth]

On 26/09/2019 12.16, David Hildenbrand wrote:
> IEP support in the mmu is fairly easy. Set the right permissions for TLB
> entries and properly report an exception.
> 
> Make sure to handle EDAT-2 by setting bit 56/60/61 of the TEID (TEC) to
> the right values.
> 
> Let's keep s390_cpu_get_phys_page_debug() working even if IEP is
> active. Switch MMU_DATA_LOAD - this has no other effects any more as the
> ASC to be used is now fully selected outside of mmu_translate().
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  target/s390x/cpu.h        |  1 +
>  target/s390x/helper.c     |  6 +++++-
>  target/s390x/mmu_helper.c | 21 +++++++++++++++++++++
>  3 files changed, 27 insertions(+), 1 deletion(-)

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH v1 0/5] s390x/mmu: Implement more facilities

[David Hildenbrand]

On 26.09.19 12:16, David Hildenbrand wrote:
> This is the follow up of:
>     [PATCH-for-4.2 v1 0/9] s390x: MMU changes and extensions
> Without the general MMU rework. It's based on:
>     [PATCH v2 0/7] s390x/mmu: DAT translation rewrite
> 
> This series adds adds EDAT2 MMU support, and implements/indicates related
> facilities (ESOP-1, ESOP-2, IEP, ...) for TCG. The QEMU CPU model is
> updated.
> 
> IEP under QEMU TCG seems to work just fine, when eabling it via the "max"
> CPU model - via kvm unit tests:
>     t460s: ~/git/kvm-unit-tests master $ ./s390x-run s390x/iep.elf -cpu max
>     [...]
>     PASS: iep: iep protection: Program interrupt: expected(4) == received(4)
>     SUMMARY: 1 tests
> 
>     EXIT: STATUS=1
> 
> Changes since "[PATCH-for-4.2 v1 0/9] s390x: MMU changes and extensions":
> - "s390x/mmu: Add EDAT2 translation support"
> -- Fix vaddr offset within 2GB page
> - "s390x/mmu: Implement ESOP-2 and ..."
> -- Squashed two patches as requested.
> -- Also set ilen to "2" in case of MMU_INST_FETCH on mmu_translate_real
> - "s390x/mmu: Implement Instruction-Execution-Protection Facility"
> -- Make sure s390_cpu_get_phys_page_debug() doesn't choke on IEP
> - "s390x/cpumodel: Add new TCG features to QEMU cpu model"
> -- Add comment "features introduced after the z13"
> 
> Cc: Ilya Leoshkevich <iii@linux.ibm.com>
> 
> David Hildenbrand (5):
>   s390x/mmu: Add EDAT2 translation support
>   s390x/mmu: Implement ESOP-2 and
>     access-exception-fetch/store-indication facility
>   s390x/mmu: Implement Instruction-Execution-Protection Facility
>   s390x/cpumodel: Prepare for changes of QEMU model
>   s390x/cpumodel: Add new TCG features to QEMU cpu model
> 
>  hw/s390x/s390-virtio-ccw.c  |  2 ++
>  target/s390x/cpu.h          |  1 +
>  target/s390x/gen-features.c | 11 +++++++++-
>  target/s390x/helper.c       |  6 +++++-
>  target/s390x/mmu_helper.c   | 42 +++++++++++++++++++++++++++++++++++--
>  5 files changed, 58 insertions(+), 4 deletions(-)
> 

@Christian (@Conny) if I can get an ACK on the last patch, I can send
this directly upstream.

-- 

Thanks,

David / dhildenb

Re: [PATCH v1 5/5] s390x/cpumodel: Add new TCG features to QEMU cpu model

[Cornelia Huck]

On Thu, 26 Sep 2019 12:16:27 +0200
David Hildenbrand <david@redhat.com> wrote:

> We now implement a bunch of new facilities we can properly indicate.
> 
> ESOP-1/ESOP-2 handling is discussed in the PoP Chafter 3-15
> ("Suppression on Protection"). The "Basic suppression-on-protection (SOP)
> facility" is a core part of z/Architecture without a facility
> indication. ESOP-2 is indicated by ESOP-1 + Side-effect facility
> ("ESOP-2"). Besides ESOP-2, the side-effect facility is only relevant for
> the guarded-storage facility (we don't implement).
> 
> S390_ESOP:
> - We indicate DAT exeptions by setting bit 61 of the TEID (TEC) to 1 and
>   bit 60 to zero. We don't trigger ALCP exceptions yet. Also, we set
>   bit 0-51 and bit 62/63 to the right values.
> S390_ACCESS_EXCEPTION_FS_INDICATION:
> - The TEID (TEC) properly indicates in bit 52/53 on any access if it was
>   a fetch or a store
> S390_SIDE_EFFECT_ACCESS_ESOP2:
> - We have no side-effect accesses (esp., we don't implement the
>   guarded-storage faciliy), we correctly set bit 64 of the TEID (TEC) to
>   0 (no side-effect).
> - ESOP2: We properly set bit 56, 60, 61 in the TEID (TEC) to indicate the
>   type of protection. We don't trigger KCP/ALCP exceptions yet.
> S390_INSTRUCTION_EXEC_PROT:
> - The MMU properly detects and indicates the exception on instruction fetches
> - Protected TLB entries will never get PAGE_EXEC set.
> 
> There is no need to fake the abscence of any of the facilities - without
> the facilities, some bits of the TEID (TEC) are simply unpredictable.
> 
> As IEP was added with z14 and we currently implement a z13, add it to
> the MAX model instead.
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  target/s390x/gen-features.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/target/s390x/gen-features.c b/target/s390x/gen-features.c
> index 7e82f2f004..6278845b12 100644
> --- a/target/s390x/gen-features.c
> +++ b/target/s390x/gen-features.c
> @@ -704,12 +704,17 @@ static uint16_t qemu_V4_1[] = {
>  };
>  
>  static uint16_t qemu_LATEST[] = {
> +    S390_FEAT_ACCESS_EXCEPTION_FS_INDICATION,
> +    S390_FEAT_SIDE_EFFECT_ACCESS_ESOP2,
> +    S390_FEAT_ESOP,
>  };
>  
>  /* add all new definitions before this point */
>  static uint16_t qemu_MAX[] = {
>      /* generates a dependency warning, leave it out for now */
>      S390_FEAT_MSA_EXT_5,
> +    /* features introduced after the z13 */
> +    S390_FEAT_INSTRUCTION_EXEC_PROT,
>  };
>  
>  /****** END FEATURE DEFS ******/

Acked-by: Cornelia Huck <cohuck@redhat.com>

Re: [PATCH v1 0/5] s390x/mmu: Implement more facilities

[Cornelia Huck]

On Fri, 4 Oct 2019 15:23:32 +0200
David Hildenbrand <david@redhat.com> wrote:

> On 26.09.19 12:16, David Hildenbrand wrote:
> > This is the follow up of:
> >     [PATCH-for-4.2 v1 0/9] s390x: MMU changes and extensions
> > Without the general MMU rework. It's based on:
> >     [PATCH v2 0/7] s390x/mmu: DAT translation rewrite
> > 
> > This series adds adds EDAT2 MMU support, and implements/indicates related
> > facilities (ESOP-1, ESOP-2, IEP, ...) for TCG. The QEMU CPU model is
> > updated.
> > 
> > IEP under QEMU TCG seems to work just fine, when eabling it via the "max"
> > CPU model - via kvm unit tests:
> >     t460s: ~/git/kvm-unit-tests master $ ./s390x-run s390x/iep.elf -cpu max
> >     [...]
> >     PASS: iep: iep protection: Program interrupt: expected(4) == received(4)
> >     SUMMARY: 1 tests
> > 
> >     EXIT: STATUS=1
> > 
> > Changes since "[PATCH-for-4.2 v1 0/9] s390x: MMU changes and extensions":
> > - "s390x/mmu: Add EDAT2 translation support"
> > -- Fix vaddr offset within 2GB page
> > - "s390x/mmu: Implement ESOP-2 and ..."
> > -- Squashed two patches as requested.
> > -- Also set ilen to "2" in case of MMU_INST_FETCH on mmu_translate_real
> > - "s390x/mmu: Implement Instruction-Execution-Protection Facility"
> > -- Make sure s390_cpu_get_phys_page_debug() doesn't choke on IEP
> > - "s390x/cpumodel: Add new TCG features to QEMU cpu model"
> > -- Add comment "features introduced after the z13"
> > 
> > Cc: Ilya Leoshkevich <iii@linux.ibm.com>
> > 
> > David Hildenbrand (5):
> >   s390x/mmu: Add EDAT2 translation support
> >   s390x/mmu: Implement ESOP-2 and
> >     access-exception-fetch/store-indication facility
> >   s390x/mmu: Implement Instruction-Execution-Protection Facility
> >   s390x/cpumodel: Prepare for changes of QEMU model
> >   s390x/cpumodel: Add new TCG features to QEMU cpu model
> > 
> >  hw/s390x/s390-virtio-ccw.c  |  2 ++
> >  target/s390x/cpu.h          |  1 +
> >  target/s390x/gen-features.c | 11 +++++++++-
> >  target/s390x/helper.c       |  6 +++++-
> >  target/s390x/mmu_helper.c   | 42 +++++++++++++++++++++++++++++++++++--
> >  5 files changed, 58 insertions(+), 4 deletions(-)
> >   
> 
> @Christian (@Conny) if I can get an ACK on the last patch, I can send
> this directly upstream.
> 

No objections from my side, I won't get around to reviewing it in
detail anyway.

Re: [PATCH v1 0/5] s390x/mmu: Implement more facilities

[David Hildenbrand]

On 07.10.19 19:02, Cornelia Huck wrote:
> On Fri, 4 Oct 2019 15:23:32 +0200
> David Hildenbrand <david@redhat.com> wrote:
> 
>> On 26.09.19 12:16, David Hildenbrand wrote:
>>> This is the follow up of:
>>>     [PATCH-for-4.2 v1 0/9] s390x: MMU changes and extensions
>>> Without the general MMU rework. It's based on:
>>>     [PATCH v2 0/7] s390x/mmu: DAT translation rewrite
>>>
>>> This series adds adds EDAT2 MMU support, and implements/indicates related
>>> facilities (ESOP-1, ESOP-2, IEP, ...) for TCG. The QEMU CPU model is
>>> updated.
>>>
>>> IEP under QEMU TCG seems to work just fine, when eabling it via the "max"
>>> CPU model - via kvm unit tests:
>>>     t460s: ~/git/kvm-unit-tests master $ ./s390x-run s390x/iep.elf -cpu max
>>>     [...]
>>>     PASS: iep: iep protection: Program interrupt: expected(4) == received(4)
>>>     SUMMARY: 1 tests
>>>
>>>     EXIT: STATUS=1
>>>
>>> Changes since "[PATCH-for-4.2 v1 0/9] s390x: MMU changes and extensions":
>>> - "s390x/mmu: Add EDAT2 translation support"
>>> -- Fix vaddr offset within 2GB page
>>> - "s390x/mmu: Implement ESOP-2 and ..."
>>> -- Squashed two patches as requested.
>>> -- Also set ilen to "2" in case of MMU_INST_FETCH on mmu_translate_real
>>> - "s390x/mmu: Implement Instruction-Execution-Protection Facility"
>>> -- Make sure s390_cpu_get_phys_page_debug() doesn't choke on IEP
>>> - "s390x/cpumodel: Add new TCG features to QEMU cpu model"
>>> -- Add comment "features introduced after the z13"
>>>
>>> Cc: Ilya Leoshkevich <iii@linux.ibm.com>
>>>
>>> David Hildenbrand (5):
>>>   s390x/mmu: Add EDAT2 translation support
>>>   s390x/mmu: Implement ESOP-2 and
>>>     access-exception-fetch/store-indication facility
>>>   s390x/mmu: Implement Instruction-Execution-Protection Facility
>>>   s390x/cpumodel: Prepare for changes of QEMU model
>>>   s390x/cpumodel: Add new TCG features to QEMU cpu model
>>>
>>>  hw/s390x/s390-virtio-ccw.c  |  2 ++
>>>  target/s390x/cpu.h          |  1 +
>>>  target/s390x/gen-features.c | 11 +++++++++-
>>>  target/s390x/helper.c       |  6 +++++-
>>>  target/s390x/mmu_helper.c   | 42 +++++++++++++++++++++++++++++++++++--
>>>  5 files changed, 58 insertions(+), 4 deletions(-)
>>>   
>>
>> @Christian (@Conny) if I can get an ACK on the last patch, I can send
>> this directly upstream.
>>
> 
> No objections from my side, I won't get around to reviewing it in
> detail anyway.
> 

Okay, thanks. I'll queue and send this soon.

-- 

Thanks,

David / dhildenb