diff mbox series

[PATCH-for-4.2,v1,2/6] s390x/tcg: Rework MMU selection for instruction fetches

Message ID 20190812112737.6652-3-david@redhat.com (mailing list archive)
State New, archived
Headers show
Series s390x/mmu: Storage key reference and change bit handling | expand

Commit Message

David Hildenbrand Aug. 12, 2019, 11:27 a.m. UTC 
Instructions are always fetched from primary address space, except when
in home address mode. Perform the selection directly in cpu_mmu_index().

get_mem_index() is only used to perform data access, instructions are
fetched via cpu_lduw_code(), which translates to cpu_mmu_index(env, true).

We don't care about restricting the access permissions of the TLB
entries anymore, as we no longer enter PRIMARY entries into the
SECONDARY MMU. Cleanup related code a bit.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/cpu.h        |  7 +++++++
 target/s390x/mmu_helper.c | 35 ++++++++++++++---------------------
 2 files changed, 21 insertions(+), 21 deletions(-)

Comments

David Hildenbrand Aug. 12, 2019, 1:37 p.m. UTC | #1 
On 12.08.19 13:27, David Hildenbrand wrote:
> Instructions are always fetched from primary address space, except when
> in home address mode. Perform the selection directly in cpu_mmu_index().
> 
> get_mem_index() is only used to perform data access, instructions are
> fetched via cpu_lduw_code(), which translates to cpu_mmu_index(env, true).
> 
> We don't care about restricting the access permissions of the TLB
> entries anymore, as we no longer enter PRIMARY entries into the
> SECONDARY MMU. Cleanup related code a bit.
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  target/s390x/cpu.h        |  7 +++++++
>  target/s390x/mmu_helper.c | 35 ++++++++++++++---------------------
>  2 files changed, 21 insertions(+), 21 deletions(-)
> 
> diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
> index a606547b4d..c34992bb2e 100644
> --- a/target/s390x/cpu.h
> +++ b/target/s390x/cpu.h
> @@ -332,6 +332,13 @@ static inline int cpu_mmu_index(CPUS390XState *env, bool ifetch)
>          return MMU_REAL_IDX;
>      }
>  
> +    if (ifetch) {
> +        if ((env->psw.mask & PSW_MASK_ASC) == PSW_ASC_HOME) {
> +            return MMU_HOME_IDX;
> +        }
> +        return MMU_PRIMARY_IDX;
> +    }
> +
>      switch (env->psw.mask & PSW_MASK_ASC) {
>      case PSW_ASC_PRIMARY:
>          return MMU_PRIMARY_IDX;
> diff --git a/target/s390x/mmu_helper.c b/target/s390x/mmu_helper.c
> index 6e9c4d6151..2c9bb3acc0 100644
> --- a/target/s390x/mmu_helper.c
> +++ b/target/s390x/mmu_helper.c
> @@ -349,6 +349,7 @@ int mmu_translate(CPUS390XState *env, target_ulong vaddr, int rw, uint64_t asc,
>  {
>      static S390SKeysState *ss;
>      static S390SKeysClass *skeyclass;
> +    uint64_t asce;
>      int r = -1;

I can now stop initializing r.

>      uint8_t key;
>  
> @@ -381,35 +382,21 @@ int mmu_translate(CPUS390XState *env, target_ulong vaddr, int rw, uint64_t asc,
>      if (!(env->psw.mask & PSW_MASK_DAT)) {
>          *raddr = vaddr;
>          r = 0;

and this can go as well.

> -        goto out;
> +        goto nodat;
>      }
>
Cornelia Huck Aug. 13, 2019, 12:52 p.m. UTC | #2 
On Mon, 12 Aug 2019 15:37:39 +0200
David Hildenbrand <david@redhat.com> wrote:

> On 12.08.19 13:27, David Hildenbrand wrote:
> > Instructions are always fetched from primary address space, except when
> > in home address mode. Perform the selection directly in cpu_mmu_index().
> > 
> > get_mem_index() is only used to perform data access, instructions are
> > fetched via cpu_lduw_code(), which translates to cpu_mmu_index(env, true).
> > 
> > We don't care about restricting the access permissions of the TLB
> > entries anymore, as we no longer enter PRIMARY entries into the
> > SECONDARY MMU. Cleanup related code a bit.
> > 
> > Signed-off-by: David Hildenbrand <david@redhat.com>
> > ---
> >  target/s390x/cpu.h        |  7 +++++++
> >  target/s390x/mmu_helper.c | 35 ++++++++++++++---------------------
> >  2 files changed, 21 insertions(+), 21 deletions(-)
> > 
> > diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
> > index a606547b4d..c34992bb2e 100644
> > --- a/target/s390x/cpu.h
> > +++ b/target/s390x/cpu.h
> > @@ -332,6 +332,13 @@ static inline int cpu_mmu_index(CPUS390XState *env, bool ifetch)
> >          return MMU_REAL_IDX;
> >      }
> >  
> > +    if (ifetch) {
> > +        if ((env->psw.mask & PSW_MASK_ASC) == PSW_ASC_HOME) {
> > +            return MMU_HOME_IDX;
> > +        }
> > +        return MMU_PRIMARY_IDX;
> > +    }
> > +
> >      switch (env->psw.mask & PSW_MASK_ASC) {
> >      case PSW_ASC_PRIMARY:
> >          return MMU_PRIMARY_IDX;
> > diff --git a/target/s390x/mmu_helper.c b/target/s390x/mmu_helper.c
> > index 6e9c4d6151..2c9bb3acc0 100644
> > --- a/target/s390x/mmu_helper.c
> > +++ b/target/s390x/mmu_helper.c
> > @@ -349,6 +349,7 @@ int mmu_translate(CPUS390XState *env, target_ulong vaddr, int rw, uint64_t asc,
> >  {
> >      static S390SKeysState *ss;
> >      static S390SKeysClass *skeyclass;
> > +    uint64_t asce;
> >      int r = -1;  
> 
> I can now stop initializing r.
> 
> >      uint8_t key;
> >  
> > @@ -381,35 +382,21 @@ int mmu_translate(CPUS390XState *env, target_ulong vaddr, int rw, uint64_t asc,
> >      if (!(env->psw.mask & PSW_MASK_DAT)) {
> >          *raddr = vaddr;
> >          r = 0;  
> 
> and this can go as well.
> 
> > -        goto out;
> > +        goto nodat;
> >      }
> >    
> 
> 

So, there will be a v2?
David Hildenbrand Aug. 13, 2019, 12:53 p.m. UTC | #3 
On 13.08.19 14:52, Cornelia Huck wrote:
> On Mon, 12 Aug 2019 15:37:39 +0200
> David Hildenbrand <david@redhat.com> wrote:
> 
>> On 12.08.19 13:27, David Hildenbrand wrote:
>>> Instructions are always fetched from primary address space, except when
>>> in home address mode. Perform the selection directly in cpu_mmu_index().
>>>
>>> get_mem_index() is only used to perform data access, instructions are
>>> fetched via cpu_lduw_code(), which translates to cpu_mmu_index(env, true).
>>>
>>> We don't care about restricting the access permissions of the TLB
>>> entries anymore, as we no longer enter PRIMARY entries into the
>>> SECONDARY MMU. Cleanup related code a bit.
>>>
>>> Signed-off-by: David Hildenbrand <david@redhat.com>
>>> ---
>>>  target/s390x/cpu.h        |  7 +++++++
>>>  target/s390x/mmu_helper.c | 35 ++++++++++++++---------------------
>>>  2 files changed, 21 insertions(+), 21 deletions(-)
>>>
>>> diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
>>> index a606547b4d..c34992bb2e 100644
>>> --- a/target/s390x/cpu.h
>>> +++ b/target/s390x/cpu.h
>>> @@ -332,6 +332,13 @@ static inline int cpu_mmu_index(CPUS390XState *env, bool ifetch)
>>>          return MMU_REAL_IDX;
>>>      }
>>>  
>>> +    if (ifetch) {
>>> +        if ((env->psw.mask & PSW_MASK_ASC) == PSW_ASC_HOME) {
>>> +            return MMU_HOME_IDX;
>>> +        }
>>> +        return MMU_PRIMARY_IDX;
>>> +    }
>>> +
>>>      switch (env->psw.mask & PSW_MASK_ASC) {
>>>      case PSW_ASC_PRIMARY:
>>>          return MMU_PRIMARY_IDX;
>>> diff --git a/target/s390x/mmu_helper.c b/target/s390x/mmu_helper.c
>>> index 6e9c4d6151..2c9bb3acc0 100644
>>> --- a/target/s390x/mmu_helper.c
>>> +++ b/target/s390x/mmu_helper.c
>>> @@ -349,6 +349,7 @@ int mmu_translate(CPUS390XState *env, target_ulong vaddr, int rw, uint64_t asc,
>>>  {
>>>      static S390SKeysState *ss;
>>>      static S390SKeysClass *skeyclass;
>>> +    uint64_t asce;
>>>      int r = -1;  
>>
>> I can now stop initializing r.
>>
>>>      uint8_t key;
>>>  
>>> @@ -381,35 +382,21 @@ int mmu_translate(CPUS390XState *env, target_ulong vaddr, int rw, uint64_t asc,
>>>      if (!(env->psw.mask & PSW_MASK_DAT)) {
>>>          *raddr = vaddr;
>>>          r = 0;  
>>
>> and this can go as well.
>>
>>> -        goto out;
>>> +        goto nodat;
>>>      }
>>>    
>>
>>
> 
> So, there will be a v2?

Yes, but waiting for more feedback.
Cornelia Huck Aug. 13, 2019, 1:16 p.m. UTC | #4 
On Mon, 12 Aug 2019 13:27:33 +0200
David Hildenbrand <david@redhat.com> wrote:

> Instructions are always fetched from primary address space, except when
> in home address mode. Perform the selection directly in cpu_mmu_index().
> 
> get_mem_index() is only used to perform data access, instructions are
> fetched via cpu_lduw_code(), which translates to cpu_mmu_index(env, true).
> 
> We don't care about restricting the access permissions of the TLB
> entries anymore, as we no longer enter PRIMARY entries into the
> SECONDARY MMU. Cleanup related code a bit.
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  target/s390x/cpu.h        |  7 +++++++
>  target/s390x/mmu_helper.c | 35 ++++++++++++++---------------------
>  2 files changed, 21 insertions(+), 21 deletions(-)

Looks sane to me; will wait for v2 to give a tag.
diff mbox series

Patch

diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index a606547b4d..c34992bb2e 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -332,6 +332,13 @@  static inline int cpu_mmu_index(CPUS390XState *env, bool ifetch)
         return MMU_REAL_IDX;
     }
 
+    if (ifetch) {
+        if ((env->psw.mask & PSW_MASK_ASC) == PSW_ASC_HOME) {
+            return MMU_HOME_IDX;
+        }
+        return MMU_PRIMARY_IDX;
+    }
+
     switch (env->psw.mask & PSW_MASK_ASC) {
     case PSW_ASC_PRIMARY:
         return MMU_PRIMARY_IDX;
diff --git a/target/s390x/mmu_helper.c b/target/s390x/mmu_helper.c
index 6e9c4d6151..2c9bb3acc0 100644
--- a/target/s390x/mmu_helper.c
+++ b/target/s390x/mmu_helper.c
@@ -349,6 +349,7 @@  int mmu_translate(CPUS390XState *env, target_ulong vaddr, int rw, uint64_t asc,
 {
     static S390SKeysState *ss;
     static S390SKeysClass *skeyclass;
+    uint64_t asce;
     int r = -1;
     uint8_t key;
 
@@ -381,35 +382,21 @@  int mmu_translate(CPUS390XState *env, target_ulong vaddr, int rw, uint64_t asc,
     if (!(env->psw.mask & PSW_MASK_DAT)) {
         *raddr = vaddr;
         r = 0;
-        goto out;
+        goto nodat;
     }
 
     switch (asc) {
     case PSW_ASC_PRIMARY:
         PTE_DPRINTF("%s: asc=primary\n", __func__);
-        r = mmu_translate_asce(env, vaddr, asc, env->cregs[1], raddr, flags,
-                               rw, exc);
+        asce = env->cregs[1];
         break;
     case PSW_ASC_HOME:
         PTE_DPRINTF("%s: asc=home\n", __func__);
-        r = mmu_translate_asce(env, vaddr, asc, env->cregs[13], raddr, flags,
-                               rw, exc);
+        asce = env->cregs[13];
         break;
     case PSW_ASC_SECONDARY:
         PTE_DPRINTF("%s: asc=secondary\n", __func__);
-        /*
-         * Instruction: Primary
-         * Data: Secondary
-         */
-        if (rw == MMU_INST_FETCH) {
-            r = mmu_translate_asce(env, vaddr, PSW_ASC_PRIMARY, env->cregs[1],
-                                   raddr, flags, rw, exc);
-            *flags &= ~(PAGE_READ | PAGE_WRITE);
-        } else {
-            r = mmu_translate_asce(env, vaddr, PSW_ASC_SECONDARY, env->cregs[7],
-                                   raddr, flags, rw, exc);
-            *flags &= ~(PAGE_EXEC);
-        }
+        asce = env->cregs[7];
         break;
     case PSW_ASC_ACCREG:
     default:
@@ -417,11 +404,17 @@  int mmu_translate(CPUS390XState *env, target_ulong vaddr, int rw, uint64_t asc,
         break;
     }
 
- out:
+    /* perform the DAT translation */
+    r = mmu_translate_asce(env, vaddr, asc, asce, raddr, flags, rw, exc);
+    if (r) {
+        return r;
+    }
+
+nodat:
     /* Convert real address -> absolute address */
     *raddr = mmu_real2abs(env, *raddr);
 
-    if (r == 0 && *raddr < ram_size) {
+    if (*raddr < ram_size) {
         if (skeyclass->get_skeys(ss, *raddr / TARGET_PAGE_SIZE, 1, &key)) {
             trace_get_skeys_nonzero(r);
             return 0;
@@ -441,7 +434,7 @@  int mmu_translate(CPUS390XState *env, target_ulong vaddr, int rw, uint64_t asc,
         }
     }
 
-    return r;
+    return 0;
 }
 
 /**