diff mbox

[v2,03/29] target/s390x: implement local-TLB-clearing in IPTE

Message ID 20170529192440.5990-4-aurelien@aurel32.net
State New
Headers show

Commit Message

Aurelien Jarno May 29, 2017, 7:24 p.m. UTC
And at the same time make IPTE SMP aware.

Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 target/s390x/helper.h     |  2 +-
 target/s390x/mem_helper.c | 19 ++++++++++++-------
 target/s390x/translate.c  |  6 +++++-
 3 files changed, 18 insertions(+), 9 deletions(-)

Comments

Thomas Huth May 30, 2017, 9:01 a.m. UTC | #1
On 29.05.2017 21:24, Aurelien Jarno wrote:
> And at the same time make IPTE SMP aware.
> 
> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
> ---
>  target/s390x/helper.h     |  2 +-
>  target/s390x/mem_helper.c | 19 ++++++++++++-------
>  target/s390x/translate.c  |  6 +++++-
>  3 files changed, 18 insertions(+), 9 deletions(-)
[...]
> @@ -1092,13 +1091,19 @@ void HELPER(ipte)(CPUS390XState *env, uint64_t pto, uint64_t vaddr)
>  
>      /* XXX we exploit the fact that Linux passes the exact virtual
>         address here - it's not obliged to! */
> -    tlb_flush_page(cs, page);
> +    /* XXX: the LC bit should be considered as 0 if the local-TLB-clearing
> +       facility is not installed.  */

That should be easy, I think:

    if (!s390_has_feat(S390_FEAT_LOCAL_TLB_CLEARING)) {
        m4 = 0;
    }

> +    if (m4 & 1) {
> +        tlb_flush_page(cs, page);
> +    } else {
> +        tlb_flush_page_all_cpus_synced(cs, page);
> +    }
>  
>      /* XXX 31-bit hack */
> -    if (page & 0x80000000) {
> -        tlb_flush_page(cs, page & ~0x80000000);
> +    if (m4 & 1) {
> +        tlb_flush_page(cs, page ^ 0x80000000);
>      } else {
> -        tlb_flush_page(cs, page | 0x80000000);
> +        tlb_flush_page_all_cpus_synced(cs, page ^ 0x80000000);
>      }
>  }

 Thomas
Aurelien Jarno May 30, 2017, 11:18 a.m. UTC | #2
On 2017-05-30 11:01, Thomas Huth wrote:
> On 29.05.2017 21:24, Aurelien Jarno wrote:
> > And at the same time make IPTE SMP aware.
> > 
> > Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
> > ---
> >  target/s390x/helper.h     |  2 +-
> >  target/s390x/mem_helper.c | 19 ++++++++++++-------
> >  target/s390x/translate.c  |  6 +++++-
> >  3 files changed, 18 insertions(+), 9 deletions(-)
> [...]
> > @@ -1092,13 +1091,19 @@ void HELPER(ipte)(CPUS390XState *env, uint64_t pto, uint64_t vaddr)
> >  
> >      /* XXX we exploit the fact that Linux passes the exact virtual
> >         address here - it's not obliged to! */
> > -    tlb_flush_page(cs, page);
> > +    /* XXX: the LC bit should be considered as 0 if the local-TLB-clearing
> > +       facility is not installed.  */
> 
> That should be easy, I think:
> 
>     if (!s390_has_feat(S390_FEAT_LOCAL_TLB_CLEARING)) {
>         m4 = 0;
>     }

I agree, it should be easy to implement. The reason I haven't implemented
it is that we currently don't have a good model for the TCG CPU, so we
don't have a way to easily enable it or not (though that is changing a
bit with your later patches). In practice we expose supported features
through STFL/SFTLE, but we don't do any check to see if the features or
instructions are enable or not.

In addition this feature is not fully implemented, it should also be
done for the IDTE instruction, which we don't implement (i have a local
patch for that, but not yet ready).
Thomas Huth May 30, 2017, 12:03 p.m. UTC | #3
On 30.05.2017 13:18, Aurelien Jarno wrote:
> On 2017-05-30 11:01, Thomas Huth wrote:
>> On 29.05.2017 21:24, Aurelien Jarno wrote:
>>> And at the same time make IPTE SMP aware.
>>>
>>> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
>>> ---
>>>  target/s390x/helper.h     |  2 +-
>>>  target/s390x/mem_helper.c | 19 ++++++++++++-------
>>>  target/s390x/translate.c  |  6 +++++-
>>>  3 files changed, 18 insertions(+), 9 deletions(-)
>> [...]
>>> @@ -1092,13 +1091,19 @@ void HELPER(ipte)(CPUS390XState *env, uint64_t pto, uint64_t vaddr)
>>>  
>>>      /* XXX we exploit the fact that Linux passes the exact virtual
>>>         address here - it's not obliged to! */
>>> -    tlb_flush_page(cs, page);
>>> +    /* XXX: the LC bit should be considered as 0 if the local-TLB-clearing
>>> +       facility is not installed.  */
>>
>> That should be easy, I think:
>>
>>     if (!s390_has_feat(S390_FEAT_LOCAL_TLB_CLEARING)) {
>>         m4 = 0;
>>     }
> 
> I agree, it should be easy to implement. The reason I haven't implemented
> it is that we currently don't have a good model for the TCG CPU, so we
> don't have a way to easily enable it or not (though that is changing a
> bit with your later patches). In practice we expose supported features
> through STFL/SFTLE, but we don't do any check to see if the features or
> instructions are enable or not.

OK, fair, ... and your patch clearly marks this spot with an XXX, so we
can revise this later. So:

Reviewed-by: Thomas Huth <thuth@redhat.com>
Richard Henderson May 30, 2017, 7:59 p.m. UTC | #4
On 05/29/2017 12:24 PM, Aurelien Jarno wrote:
> And at the same time make IPTE SMP aware.
> 
> Signed-off-by: Aurelien Jarno<aurelien@aurel32.net>
> ---
>   target/s390x/helper.h     |  2 +-
>   target/s390x/mem_helper.c | 19 ++++++++++++-------
>   target/s390x/translate.c  |  6 +++++-
>   3 files changed, 18 insertions(+), 9 deletions(-)

Reviewed-by: Richard Henderson <rth@twiddle.net>


r~
diff mbox

Patch

diff --git a/target/s390x/helper.h b/target/s390x/helper.h
index cc451c70a6..3f5a05d43b 100644
--- a/target/s390x/helper.h
+++ b/target/s390x/helper.h
@@ -111,7 +111,7 @@  DEF_HELPER_4(mvcs, i32, env, i64, i64, i64)
 DEF_HELPER_4(mvcp, i32, env, i64, i64, i64)
 DEF_HELPER_4(sigp, i32, env, i64, i32, i64)
 DEF_HELPER_FLAGS_2(sacf, TCG_CALL_NO_WG, void, env, i64)
-DEF_HELPER_FLAGS_3(ipte, TCG_CALL_NO_RWG, void, env, i64, i64)
+DEF_HELPER_FLAGS_4(ipte, TCG_CALL_NO_RWG, void, env, i64, i64, i32)
 DEF_HELPER_FLAGS_1(ptlb, TCG_CALL_NO_RWG, void, env)
 DEF_HELPER_FLAGS_1(purge, TCG_CALL_NO_RWG, void, env)
 DEF_HELPER_2(lra, i64, env, i64)
diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index 0ebd65d9ab..9fbe7c9ef9 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -1073,14 +1073,13 @@  uint32_t HELPER(mvcp)(CPUS390XState *env, uint64_t l, uint64_t a1, uint64_t a2)
 }
 
 /* invalidate pte */
-void HELPER(ipte)(CPUS390XState *env, uint64_t pto, uint64_t vaddr)
+void HELPER(ipte)(CPUS390XState *env, uint64_t pto, uint64_t vaddr,
+                  uint32_t m4)
 {
     CPUState *cs = CPU(s390_env_get_cpu(env));
     uint64_t page = vaddr & TARGET_PAGE_MASK;
     uint64_t pte_addr, pte;
 
-    /* XXX broadcast to other CPUs */
-
     /* Compute the page table entry address */
     pte_addr = (pto & _SEGMENT_ENTRY_ORIGIN);
     pte_addr += (vaddr & _VADDR_PX) >> 9;
@@ -1092,13 +1091,19 @@  void HELPER(ipte)(CPUS390XState *env, uint64_t pto, uint64_t vaddr)
 
     /* XXX we exploit the fact that Linux passes the exact virtual
        address here - it's not obliged to! */
-    tlb_flush_page(cs, page);
+    /* XXX: the LC bit should be considered as 0 if the local-TLB-clearing
+       facility is not installed.  */
+    if (m4 & 1) {
+        tlb_flush_page(cs, page);
+    } else {
+        tlb_flush_page_all_cpus_synced(cs, page);
+    }
 
     /* XXX 31-bit hack */
-    if (page & 0x80000000) {
-        tlb_flush_page(cs, page & ~0x80000000);
+    if (m4 & 1) {
+        tlb_flush_page(cs, page ^ 0x80000000);
     } else {
-        tlb_flush_page(cs, page | 0x80000000);
+        tlb_flush_page_all_cpus_synced(cs, page ^ 0x80000000);
     }
 }
 
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index f7598184a6..f160b62c19 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -2352,8 +2352,12 @@  static ExitStatus op_ipm(DisasContext *s, DisasOps *o)
 #ifndef CONFIG_USER_ONLY
 static ExitStatus op_ipte(DisasContext *s, DisasOps *o)
 {
+    TCGv_i32 m4;
+
     check_privileged(s);
-    gen_helper_ipte(cpu_env, o->in1, o->in2);
+    m4 = tcg_const_i32(get_field(s->fields, m4));
+    gen_helper_ipte(cpu_env, o->in1, o->in2, m4);
+    tcg_temp_free_i32(m4);
     return NO_EXIT;
 }