| Message ID | 20170529192440.5990-4-aurelien@aurel32.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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).
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>
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 --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; }
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(-)