| Message ID | 20170927170027.8539-4-david@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | s390x/tcg: LAP support using immediate TLB invalidation | expand |
On 09/27/2017 10:00 AM, David Hildenbrand wrote: > Using virtual memory access is wrong and will soon include low-address > protection checks, which is to be bypassed for STFL. > > This was originally part of a bigger STFL(E) refactoring. > > Signed-off-by: David Hildenbrand <david@redhat.com> > --- > target/s390x/helper.h | 2 +- > target/s390x/misc_helper.c | 7 ++++++- > 2 files changed, 7 insertions(+), 2 deletions(-) Need to sort this patch first, so that the series is bisectable. > DEF_HELPER_FLAGS_2(sfpc, TCG_CALL_NO_RWG, void, env, i64) > DEF_HELPER_FLAGS_2(sfas, TCG_CALL_NO_WG, void, env, i64) > DEF_HELPER_FLAGS_1(popcnt, TCG_CALL_NO_RWG_SE, i64, i64) > -DEF_HELPER_FLAGS_1(stfl, TCG_CALL_NO_RWG, void, env) > DEF_HELPER_2(stfle, i32, env, i64) > DEF_HELPER_FLAGS_2(lpq, TCG_CALL_NO_WG, i64, env, i64) > DEF_HELPER_FLAGS_4(stpq, TCG_CALL_NO_WG, void, env, i64, i64, i64) > @@ -153,6 +152,7 @@ DEF_HELPER_FLAGS_3(sturg, TCG_CALL_NO_WG, void, env, i64, i64) > DEF_HELPER_1(per_check_exception, void, env) > DEF_HELPER_FLAGS_3(per_branch, TCG_CALL_NO_RWG, void, env, i64, i64) > DEF_HELPER_FLAGS_2(per_ifetch, TCG_CALL_NO_RWG, void, env, i64) > +DEF_HELPER_FLAGS_1(stfl, TCG_CALL_NO_RWG, void, env) > Why? Otherwise, Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
On 27.09.2017 19:52, Richard Henderson wrote: > On 09/27/2017 10:00 AM, David Hildenbrand wrote: >> Using virtual memory access is wrong and will soon include low-address >> protection checks, which is to be bypassed for STFL. >> >> This was originally part of a bigger STFL(E) refactoring. >> >> Signed-off-by: David Hildenbrand <david@redhat.com> >> --- >> target/s390x/helper.h | 2 +- >> target/s390x/misc_helper.c | 7 ++++++- >> 2 files changed, 7 insertions(+), 2 deletions(-) > > Need to sort this patch first, so that the series is bisectable. Right, this should become #2. > >> DEF_HELPER_FLAGS_2(sfpc, TCG_CALL_NO_RWG, void, env, i64) >> DEF_HELPER_FLAGS_2(sfas, TCG_CALL_NO_WG, void, env, i64) >> DEF_HELPER_FLAGS_1(popcnt, TCG_CALL_NO_RWG_SE, i64, i64) >> -DEF_HELPER_FLAGS_1(stfl, TCG_CALL_NO_RWG, void, env) >> DEF_HELPER_2(stfle, i32, env, i64) >> DEF_HELPER_FLAGS_2(lpq, TCG_CALL_NO_WG, i64, env, i64) >> DEF_HELPER_FLAGS_4(stpq, TCG_CALL_NO_WG, void, env, i64, i64, i64) >> @@ -153,6 +152,7 @@ DEF_HELPER_FLAGS_3(sturg, TCG_CALL_NO_WG, void, env, i64, i64) >> DEF_HELPER_1(per_check_exception, void, env) >> DEF_HELPER_FLAGS_3(per_branch, TCG_CALL_NO_RWG, void, env, i64, i64) >> DEF_HELPER_FLAGS_2(per_ifetch, TCG_CALL_NO_RWG, void, env, i64) >> +DEF_HELPER_FLAGS_1(stfl, TCG_CALL_NO_RWG, void, env) >> > > Why? Otherwise, struct LowCore is only available for !CONFIG_USER_ONLY. Therefore I also have to move the helper declaration into !CONFIG_USER_ONLY. Thanks! > > Reviewed-by: Richard Henderson <richard.henderson@linaro.org> > > > r~ >
On 27.09.2017 20:46, David Hildenbrand wrote: > On 27.09.2017 19:52, Richard Henderson wrote: >> On 09/27/2017 10:00 AM, David Hildenbrand wrote: >>> Using virtual memory access is wrong and will soon include low-address >>> protection checks, which is to be bypassed for STFL. >>> >>> This was originally part of a bigger STFL(E) refactoring. >>> >>> Signed-off-by: David Hildenbrand <david@redhat.com> >>> --- >>> target/s390x/helper.h | 2 +- >>> target/s390x/misc_helper.c | 7 ++++++- >>> 2 files changed, 7 insertions(+), 2 deletions(-) >> >> Need to sort this patch first, so that the series is bisectable. > > Right, this should become #2. > >> >>> DEF_HELPER_FLAGS_2(sfpc, TCG_CALL_NO_RWG, void, env, i64) >>> DEF_HELPER_FLAGS_2(sfas, TCG_CALL_NO_WG, void, env, i64) >>> DEF_HELPER_FLAGS_1(popcnt, TCG_CALL_NO_RWG_SE, i64, i64) >>> -DEF_HELPER_FLAGS_1(stfl, TCG_CALL_NO_RWG, void, env) >>> DEF_HELPER_2(stfle, i32, env, i64) >>> DEF_HELPER_FLAGS_2(lpq, TCG_CALL_NO_WG, i64, env, i64) >>> DEF_HELPER_FLAGS_4(stpq, TCG_CALL_NO_WG, void, env, i64, i64, i64) >>> @@ -153,6 +152,7 @@ DEF_HELPER_FLAGS_3(sturg, TCG_CALL_NO_WG, void, env, i64, i64) >>> DEF_HELPER_1(per_check_exception, void, env) >>> DEF_HELPER_FLAGS_3(per_branch, TCG_CALL_NO_RWG, void, env, i64, i64) >>> DEF_HELPER_FLAGS_2(per_ifetch, TCG_CALL_NO_RWG, void, env, i64) >>> +DEF_HELPER_FLAGS_1(stfl, TCG_CALL_NO_RWG, void, env) >>> >> >> Why? Otherwise, > > struct LowCore is only available for !CONFIG_USER_ONLY. Therefore I also > have to move the helper declaration into !CONFIG_USER_ONLY. You should mention that in the patch description, that it is a privileged instruction and thus you also mark it here accordingly. With that update: Reviewed-by: Thomas Huth <thuth@redhat.com>
diff --git a/target/s390x/helper.h b/target/s390x/helper.h index 75ba04fc15..52c2963baa 100644 --- a/target/s390x/helper.h +++ b/target/s390x/helper.h @@ -104,7 +104,6 @@ DEF_HELPER_FLAGS_5(calc_cc, TCG_CALL_NO_RWG_SE, i32, env, i32, i64, i64, i64) DEF_HELPER_FLAGS_2(sfpc, TCG_CALL_NO_RWG, void, env, i64) DEF_HELPER_FLAGS_2(sfas, TCG_CALL_NO_WG, void, env, i64) DEF_HELPER_FLAGS_1(popcnt, TCG_CALL_NO_RWG_SE, i64, i64) -DEF_HELPER_FLAGS_1(stfl, TCG_CALL_NO_RWG, void, env) DEF_HELPER_2(stfle, i32, env, i64) DEF_HELPER_FLAGS_2(lpq, TCG_CALL_NO_WG, i64, env, i64) DEF_HELPER_FLAGS_4(stpq, TCG_CALL_NO_WG, void, env, i64, i64, i64) @@ -153,6 +152,7 @@ DEF_HELPER_FLAGS_3(sturg, TCG_CALL_NO_WG, void, env, i64, i64) DEF_HELPER_1(per_check_exception, void, env) DEF_HELPER_FLAGS_3(per_branch, TCG_CALL_NO_RWG, void, env, i64, i64) DEF_HELPER_FLAGS_2(per_ifetch, TCG_CALL_NO_RWG, void, env, i64) +DEF_HELPER_FLAGS_1(stfl, TCG_CALL_NO_RWG, void, env) DEF_HELPER_2(xsch, void, env, i64) DEF_HELPER_2(csch, void, env, i64) diff --git a/target/s390x/misc_helper.c b/target/s390x/misc_helper.c index 293fc8428a..0b93381188 100644 --- a/target/s390x/misc_helper.c +++ b/target/s390x/misc_helper.c @@ -541,13 +541,18 @@ static unsigned do_stfle(CPUS390XState *env, uint64_t words[MAX_STFL_WORDS]) return max_bit / 64; } +#ifndef CONFIG_USER_ONLY void HELPER(stfl)(CPUS390XState *env) { uint64_t words[MAX_STFL_WORDS]; + LowCore *lowcore; + lowcore = cpu_map_lowcore(env); do_stfle(env, words); - cpu_stl_data(env, 200, words[0] >> 32); + lowcore->stfl_fac_list = cpu_to_be32(words[0] >> 32); + cpu_unmap_lowcore(lowcore); } +#endif uint32_t HELPER(stfle)(CPUS390XState *env, uint64_t addr) {
Using virtual memory access is wrong and will soon include low-address protection checks, which is to be bypassed for STFL. This was originally part of a bigger STFL(E) refactoring. Signed-off-by: David Hildenbrand <david@redhat.com> --- target/s390x/helper.h | 2 +- target/s390x/misc_helper.c | 7 ++++++- 2 files changed, 7 insertions(+), 2 deletions(-)