On 16.05.2017 15:42, Aurelien Jarno wrote: > On 2017-05-16 11:28, Thomas Huth wrote: >> TEST BLOCK was likely once used to execute basic memory >> tests, but nowadays it's just a (slow) way to clear a page. >> >> Signed-off-by: Thomas Huth <th...@redhat.com> >> --- >> target/s390x/helper.h | 1 + >> target/s390x/insn-data.def | 2 ++ >> target/s390x/mem_helper.c | 12 ++++++++++++ >> target/s390x/translate.c | 10 ++++++++++ >> 4 files changed, 25 insertions(+) >> >> diff --git a/target/s390x/helper.h b/target/s390x/helper.h >> index 9102071..a5a3705 100644 >> --- a/target/s390x/helper.h >> +++ b/target/s390x/helper.h >> @@ -99,6 +99,7 @@ DEF_HELPER_FLAGS_4(lctl, TCG_CALL_NO_WG, void, env, i32, >> i64, i32) >> DEF_HELPER_FLAGS_4(lctlg, TCG_CALL_NO_WG, void, env, i32, i64, i32) >> DEF_HELPER_FLAGS_4(stctl, TCG_CALL_NO_WG, void, env, i32, i64, i32) >> DEF_HELPER_FLAGS_4(stctg, TCG_CALL_NO_WG, void, env, i32, i64, i32) >> +DEF_HELPER_2(testblock, void, env, i64) >> DEF_HELPER_FLAGS_2(tprot, TCG_CALL_NO_RWG, i32, i64, i64) >> DEF_HELPER_FLAGS_2(iske, TCG_CALL_NO_RWG_SE, i64, env, i64) >> DEF_HELPER_FLAGS_3(sske, TCG_CALL_NO_RWG, void, env, i64, i64) >> diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def >> index 075ff59..a6aba54 100644 >> --- a/target/s390x/insn-data.def >> +++ b/target/s390x/insn-data.def >> @@ -912,6 +912,8 @@ >> /* STORE USING REAL ADDRESS */ >> C(0xb246, STURA, RRE, Z, r1_o, r2_o, 0, 0, stura, 0) >> C(0xb925, STURG, RRE, Z, r1_o, r2_o, 0, 0, sturg, 0) >> +/* TEST BLOCK */ >> + C(0xb22c, TB, RRE, Z, 0, r2_o, 0, 0, testblock, 0) >> /* TEST PROTECTION */ >> C(0xe501, TPROT, SSE, Z, la1, a2, 0, 0, tprot, 0) >> >> diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c >> index 675aba2..0349c10 100644 >> --- a/target/s390x/mem_helper.c >> +++ b/target/s390x/mem_helper.c >> @@ -933,6 +933,18 @@ void HELPER(stctl)(CPUS390XState *env, uint32_t r1, >> uint64_t a2, uint32_t r3) >> } >> } >> >> +void HELPER(testblock)(CPUS390XState *env, uint64_t addr) >> +{ >> + CPUState *cs = CPU(s390_env_get_cpu(env)); >> + int i; >> + >> + addr = get_address(env, 0, 0, addr) & ~0xfffULL; > > I guess you want to use ~TARGET_PAGE_MASK here.
Yes, that's better, thanks! >> + for (i = 0; i < TARGET_PAGE_SIZE; i += 8) { >> + stq_phys(cs->as, addr + i, 0); >> + } >> + env->cc_op = 0; >> +} >> + > > From what I understand the resulting condition code should depends if > the block is usable or not. Shouldn't there be a check to see if the > address actually targets the RAM? I just tried it under z/VM and KVM, and in case you specify an address that does not point to valid RAM, you get an addressing exception instead. So the CC=1 case was rather meant for memory blocks of valid RAM which contain uncorrectable bit flip errors or something like that. That does not make much sense for emulation, so CC=0 is the only useful code that we can return here. But of course I've also got to add a check for valid RAM and return an addressing exception here now, too... Thomas