| Message ID | 1495107549-6097-1-git-send-email-thuth@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
> diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c > index f6e5bce..de0ecd4 100644 > --- a/target/s390x/mem_helper.c > +++ b/target/s390x/mem_helper.c > @@ -20,6 +20,7 @@ > > #include "qemu/osdep.h" > #include "cpu.h" > +#include "exec/address-spaces.h" > #include "exec/helper-proto.h" > #include "exec/exec-all.h" > #include "exec/cpu_ldst.h" > @@ -973,6 +974,33 @@ void HELPER(stctl)(CPUS390XState *env, uint32_t r1, uint64_t a2, uint32_t r3) > } > } > > +uint32_t HELPER(testblock)(CPUS390XState *env, uint64_t real_addr) > +{ > + CPUState *cs = CPU(s390_env_get_cpu(env)); > + uint64_t abs_addr; > + int i; > + It is somewhat strange that we set a condition code in case of a program interrupt (I assume that's the magic of the return value?). But maybe setting the CC on program interrupts is perfectly valid. Reading the PoP, I think it is supposed to be like this: - Memory not addressable? -> PGM_ADDRESSING - Memory protected? -> PGM_PROTECTION - Memory not usable ("invalid checking-block-code", using it would result in a machine check) -> CC=1 - Memory usable and cleared -> CC=0 So in essence, I think we should only set the CC if successful, as we are not simulating ram failure. But maybe that's how all these handlers work, and we can't hinder it from setting the CC. > + real_addr = fix_address(env, real_addr); Could it be that fix_address() misses handling for 24 bit mode? (no idea if that is really relevant, just wondering). > + abs_addr = mmu_real2abs(env, real_addr) & TARGET_PAGE_MASK; > + if (!address_space_access_valid(&address_space_memory, abs_addr, > + TARGET_PAGE_SIZE, true)) { > + program_interrupt(env, PGM_ADDRESSING, 4); > + return 1; > + } > + > + /* Check low-address protection */ > + if ((env->cregs[0] & CR0_LOWPROT) != 0 && real_addr < 0x2000) { I would drop the != 0. > + program_interrupt(env, PGM_PROTECTION, 4); > + return 1; > + } > + > + for (i = 0; i < TARGET_PAGE_SIZE; i += 8) { > + stq_phys(cs->as, abs_addr + i, 0); > + } > + > + return 0; > +} Looks good to me!
On 18.05.2017 14:23, David Hildenbrand wrote: > >> diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c >> index f6e5bce..de0ecd4 100644 >> --- a/target/s390x/mem_helper.c >> +++ b/target/s390x/mem_helper.c >> @@ -20,6 +20,7 @@ >> >> #include "qemu/osdep.h" >> #include "cpu.h" >> +#include "exec/address-spaces.h" >> #include "exec/helper-proto.h" >> #include "exec/exec-all.h" >> #include "exec/cpu_ldst.h" >> @@ -973,6 +974,33 @@ void HELPER(stctl)(CPUS390XState *env, uint32_t r1, uint64_t a2, uint32_t r3) >> } >> } >> >> +uint32_t HELPER(testblock)(CPUS390XState *env, uint64_t real_addr) >> +{ >> + CPUState *cs = CPU(s390_env_get_cpu(env)); >> + uint64_t abs_addr; >> + int i; >> + > > It is somewhat strange that we set a condition code in case of a program > interrupt (I assume that's the magic of the return value?). But maybe > setting the CC on program interrupts is perfectly valid. According to the PoP: "[...] The operation is ter- minated on addressing and protection exceptions. If termination occurs, the condition code and the con- tents of bit positions 32-63 of general register 0 are unpredictable in the 24-bit or 31-bit addressing mode, or the condition code and bits 0-63 of the reg- ister are unpredictable in the 64-bit addressing mode." So setting CC=1 seems a valid behavior here ;-) >> + real_addr = fix_address(env, real_addr); > > Could it be that fix_address() misses handling for 24 bit mode? (no idea > if that is really relevant, just wondering). Yes, 24-bit mode is not emulated by QEMU at all, as far as I know... but that's another story. >> + abs_addr = mmu_real2abs(env, real_addr) & TARGET_PAGE_MASK; >> + if (!address_space_access_valid(&address_space_memory, abs_addr, >> + TARGET_PAGE_SIZE, true)) { >> + program_interrupt(env, PGM_ADDRESSING, 4); >> + return 1; >> + } >> + >> + /* Check low-address protection */ >> + if ((env->cregs[0] & CR0_LOWPROT) != 0 && real_addr < 0x2000) { > > I would drop the != 0. Ok, I can do that in case I have to respin the patch. >> + program_interrupt(env, PGM_PROTECTION, 4); >> + return 1; >> + } >> + >> + for (i = 0; i < TARGET_PAGE_SIZE; i += 8) { >> + stq_phys(cs->as, abs_addr + i, 0); >> + } >> + >> + return 0; >> +} > > Looks good to me! Thanks! Thomas
On 18.05.2017 14:59, Thomas Huth wrote: > On 18.05.2017 14:23, David Hildenbrand wrote: >> >>> diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c >>> index f6e5bce..de0ecd4 100644 >>> --- a/target/s390x/mem_helper.c >>> +++ b/target/s390x/mem_helper.c >>> @@ -20,6 +20,7 @@ >>> >>> #include "qemu/osdep.h" >>> #include "cpu.h" >>> +#include "exec/address-spaces.h" >>> #include "exec/helper-proto.h" >>> #include "exec/exec-all.h" >>> #include "exec/cpu_ldst.h" >>> @@ -973,6 +974,33 @@ void HELPER(stctl)(CPUS390XState *env, uint32_t r1, uint64_t a2, uint32_t r3) >>> } >>> } >>> >>> +uint32_t HELPER(testblock)(CPUS390XState *env, uint64_t real_addr) >>> +{ >>> + CPUState *cs = CPU(s390_env_get_cpu(env)); >>> + uint64_t abs_addr; >>> + int i; >>> + >> >> It is somewhat strange that we set a condition code in case of a program >> interrupt (I assume that's the magic of the return value?). But maybe >> setting the CC on program interrupts is perfectly valid. > > According to the PoP: > > "[...] The operation is ter- > minated on addressing and protection exceptions. If > termination occurs, the condition code and the con- > tents of bit positions 32-63 of general register 0 are > unpredictable in the 24-bit or 31-bit addressing > mode, or the condition code and bits 0-63 of the reg- > ister are unpredictable in the 64-bit addressing mode." > > So setting CC=1 seems a valid behavior here ;-) So Reviewed-by: David Hildenbrand <david@redhat.com>
On 2017-05-18 13:39, 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 <thuth@redhat.com> > --- > v2: > - Use DEF_HELPER_FLAGS_2 instead for DEF_HELPER_2 for returning CC value > - Convert real to absolute address > - Added a check for valid RAM page > - Added low-address protection check > > target/s390x/cpu.h | 1 + > target/s390x/helper.h | 1 + > target/s390x/insn-data.def | 2 ++ > target/s390x/mem_helper.c | 28 ++++++++++++++++++++++++++++ > target/s390x/mmu_helper.c | 2 +- > target/s390x/translate.c | 10 ++++++++++ > 6 files changed, 43 insertions(+), 1 deletion(-) > > diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h > index 240b8a5..4f38ba0 100644 > --- a/target/s390x/cpu.h > +++ b/target/s390x/cpu.h > @@ -1082,6 +1082,7 @@ struct sysib_322 { > #define SIGP_ORDER_MASK 0x000000ff > > void load_psw(CPUS390XState *env, uint64_t mask, uint64_t addr); > +target_ulong mmu_real2abs(CPUS390XState *env, target_ulong raddr); > int mmu_translate(CPUS390XState *env, target_ulong vaddr, int rw, uint64_t asc, > target_ulong *raddr, int *flags, bool exc); > int sclp_service_call(CPUS390XState *env, uint64_t sccb, uint32_t code); > diff --git a/target/s390x/helper.h b/target/s390x/helper.h > index 0b70770..1fae191 100644 > --- a/target/s390x/helper.h > +++ b/target/s390x/helper.h > @@ -102,6 +102,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_FLAGS_2(testblock, TCG_CALL_NO_WG, i32, env, i64) As the helper does not read any values from the global, you can even use TCG_CALL_NO_RWG. > 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 55a7c52..cac0f51 100644 > --- a/target/s390x/insn-data.def > +++ b/target/s390x/insn-data.def > @@ -918,6 +918,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 f6e5bce..de0ecd4 100644 > --- a/target/s390x/mem_helper.c > +++ b/target/s390x/mem_helper.c > @@ -20,6 +20,7 @@ > > #include "qemu/osdep.h" > #include "cpu.h" > +#include "exec/address-spaces.h" > #include "exec/helper-proto.h" > #include "exec/exec-all.h" > #include "exec/cpu_ldst.h" > @@ -973,6 +974,33 @@ void HELPER(stctl)(CPUS390XState *env, uint32_t r1, uint64_t a2, uint32_t r3) > } > } > > +uint32_t HELPER(testblock)(CPUS390XState *env, uint64_t real_addr) > +{ > + CPUState *cs = CPU(s390_env_get_cpu(env)); > + uint64_t abs_addr; > + int i; > + > + real_addr = fix_address(env, real_addr); > + abs_addr = mmu_real2abs(env, real_addr) & TARGET_PAGE_MASK; > + if (!address_space_access_valid(&address_space_memory, abs_addr, > + TARGET_PAGE_SIZE, true)) { > + program_interrupt(env, PGM_ADDRESSING, 4); > + return 1; > + } > + > + /* Check low-address protection */ > + if ((env->cregs[0] & CR0_LOWPROT) != 0 && real_addr < 0x2000) { > + program_interrupt(env, PGM_PROTECTION, 4); > + return 1; > + } > + > + for (i = 0; i < TARGET_PAGE_SIZE; i += 8) { > + stq_phys(cs->as, abs_addr + i, 0); > + } > + > + return 0; > +} > + > uint32_t HELPER(tprot)(uint64_t a1, uint64_t a2) > { > /* XXX implement */ > diff --git a/target/s390x/mmu_helper.c b/target/s390x/mmu_helper.c > index b11a027..31eb9ef 100644 > --- a/target/s390x/mmu_helper.c > +++ b/target/s390x/mmu_helper.c > @@ -108,7 +108,7 @@ static void trigger_page_fault(CPUS390XState *env, target_ulong vaddr, > * Translate real address to absolute (= physical) > * address by taking care of the prefix mapping. > */ > -static target_ulong mmu_real2abs(CPUS390XState *env, target_ulong raddr) > +target_ulong mmu_real2abs(CPUS390XState *env, target_ulong raddr) > { > if (raddr < 0x2000) { > return raddr + env->psa; /* Map the lowcore. */ > diff --git a/target/s390x/translate.c b/target/s390x/translate.c > index 4c48c59..61aa2c9 100644 > --- a/target/s390x/translate.c > +++ b/target/s390x/translate.c > @@ -4057,6 +4057,15 @@ static ExitStatus op_tcxb(DisasContext *s, DisasOps *o) > } > > #ifndef CONFIG_USER_ONLY > + > +static ExitStatus op_testblock(DisasContext *s, DisasOps *o) > +{ > + check_privileged(s); You should also call potential_page_fault as the helper can trigger an exception. > + gen_helper_testblock(cc_op, cpu_env, o->in2); > + set_cc_static(s); > + return NO_EXIT; > +} > + > static ExitStatus op_tprot(DisasContext *s, DisasOps *o) > { > potential_page_fault(s); > @@ -4064,6 +4073,7 @@ static ExitStatus op_tprot(DisasContext *s, DisasOps *o) > set_cc_static(s); > return NO_EXIT; > } > + > #endif > > static ExitStatus op_tr(DisasContext *s, DisasOps *o) Besides the two minor things above, it looks all good to me. Thanks for the new version.
On 2017-05-18 14:59, Thomas Huth wrote: > On 18.05.2017 14:23, David Hildenbrand wrote: > > > >> diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c > >> index f6e5bce..de0ecd4 100644 > >> --- a/target/s390x/mem_helper.c > >> +++ b/target/s390x/mem_helper.c > >> @@ -20,6 +20,7 @@ > >> > >> #include "qemu/osdep.h" > >> #include "cpu.h" > >> +#include "exec/address-spaces.h" > >> #include "exec/helper-proto.h" > >> #include "exec/exec-all.h" > >> #include "exec/cpu_ldst.h" > >> @@ -973,6 +974,33 @@ void HELPER(stctl)(CPUS390XState *env, uint32_t r1, uint64_t a2, uint32_t r3) > >> } > >> } > >> > >> +uint32_t HELPER(testblock)(CPUS390XState *env, uint64_t real_addr) > >> +{ > >> + CPUState *cs = CPU(s390_env_get_cpu(env)); > >> + uint64_t abs_addr; > >> + int i; > >> + > > > > It is somewhat strange that we set a condition code in case of a program > > interrupt (I assume that's the magic of the return value?). But maybe > > setting the CC on program interrupts is perfectly valid. > > According to the PoP: > > "[...] The operation is ter- > minated on addressing and protection exceptions. If > termination occurs, the condition code and the con- > tents of bit positions 32-63 of general register 0 are > unpredictable in the 24-bit or 31-bit addressing > mode, or the condition code and bits 0-63 of the reg- > ister are unpredictable in the 64-bit addressing mode." > > So setting CC=1 seems a valid behavior here ;-) Actually program_interrupt will never return, so CC is left unchanged in case of an exception. It's also matches the unpredictable value described in the PoP ;-).
On 18.05.2017 15:20, Aurelien Jarno wrote: > On 2017-05-18 14:59, Thomas Huth wrote: >> On 18.05.2017 14:23, David Hildenbrand wrote: >>> >>>> diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c >>>> index f6e5bce..de0ecd4 100644 >>>> --- a/target/s390x/mem_helper.c >>>> +++ b/target/s390x/mem_helper.c >>>> @@ -20,6 +20,7 @@ >>>> >>>> #include "qemu/osdep.h" >>>> #include "cpu.h" >>>> +#include "exec/address-spaces.h" >>>> #include "exec/helper-proto.h" >>>> #include "exec/exec-all.h" >>>> #include "exec/cpu_ldst.h" >>>> @@ -973,6 +974,33 @@ void HELPER(stctl)(CPUS390XState *env, uint32_t r1, uint64_t a2, uint32_t r3) >>>> } >>>> } >>>> >>>> +uint32_t HELPER(testblock)(CPUS390XState *env, uint64_t real_addr) >>>> +{ >>>> + CPUState *cs = CPU(s390_env_get_cpu(env)); >>>> + uint64_t abs_addr; >>>> + int i; >>>> + >>> >>> It is somewhat strange that we set a condition code in case of a program >>> interrupt (I assume that's the magic of the return value?). But maybe >>> setting the CC on program interrupts is perfectly valid. >> >> According to the PoP: >> >> "[...] The operation is ter- >> minated on addressing and protection exceptions. If >> termination occurs, the condition code and the con- >> tents of bit positions 32-63 of general register 0 are >> unpredictable in the 24-bit or 31-bit addressing >> mode, or the condition code and bits 0-63 of the reg- >> ister are unpredictable in the 64-bit addressing mode." >> >> So setting CC=1 seems a valid behavior here ;-) > > Actually program_interrupt will never return, so CC is left unchanged in > case of an exception. It's also matches the unpredictable value > described in the PoP ;-). Ah, right, program_interrupt() calls cpu_loop_exit() which in turn does the longjump ... makes more sense to me now, thanks for the hint! Thomas
On 18.05.2017 15:20, Aurelien Jarno wrote: > On 2017-05-18 13:39, 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 <thuth@redhat.com> >> --- >> v2: >> - Use DEF_HELPER_FLAGS_2 instead for DEF_HELPER_2 for returning CC value >> - Convert real to absolute address >> - Added a check for valid RAM page >> - Added low-address protection check >> >> target/s390x/cpu.h | 1 + >> target/s390x/helper.h | 1 + >> target/s390x/insn-data.def | 2 ++ >> target/s390x/mem_helper.c | 28 ++++++++++++++++++++++++++++ >> target/s390x/mmu_helper.c | 2 +- >> target/s390x/translate.c | 10 ++++++++++ >> 6 files changed, 43 insertions(+), 1 deletion(-) >> >> diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h >> index 240b8a5..4f38ba0 100644 >> --- a/target/s390x/cpu.h >> +++ b/target/s390x/cpu.h >> @@ -1082,6 +1082,7 @@ struct sysib_322 { >> #define SIGP_ORDER_MASK 0x000000ff >> >> void load_psw(CPUS390XState *env, uint64_t mask, uint64_t addr); >> +target_ulong mmu_real2abs(CPUS390XState *env, target_ulong raddr); >> int mmu_translate(CPUS390XState *env, target_ulong vaddr, int rw, uint64_t asc, >> target_ulong *raddr, int *flags, bool exc); >> int sclp_service_call(CPUS390XState *env, uint64_t sccb, uint32_t code); >> diff --git a/target/s390x/helper.h b/target/s390x/helper.h >> index 0b70770..1fae191 100644 >> --- a/target/s390x/helper.h >> +++ b/target/s390x/helper.h >> @@ -102,6 +102,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_FLAGS_2(testblock, TCG_CALL_NO_WG, i32, env, i64) > > As the helper does not read any values from the global, you can even use > TCG_CALL_NO_RWG. Ok. >> 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 55a7c52..cac0f51 100644 >> --- a/target/s390x/insn-data.def >> +++ b/target/s390x/insn-data.def >> @@ -918,6 +918,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 f6e5bce..de0ecd4 100644 >> --- a/target/s390x/mem_helper.c >> +++ b/target/s390x/mem_helper.c >> @@ -20,6 +20,7 @@ >> >> #include "qemu/osdep.h" >> #include "cpu.h" >> +#include "exec/address-spaces.h" >> #include "exec/helper-proto.h" >> #include "exec/exec-all.h" >> #include "exec/cpu_ldst.h" >> @@ -973,6 +974,33 @@ void HELPER(stctl)(CPUS390XState *env, uint32_t r1, uint64_t a2, uint32_t r3) >> } >> } >> >> +uint32_t HELPER(testblock)(CPUS390XState *env, uint64_t real_addr) >> +{ >> + CPUState *cs = CPU(s390_env_get_cpu(env)); >> + uint64_t abs_addr; >> + int i; >> + >> + real_addr = fix_address(env, real_addr); >> + abs_addr = mmu_real2abs(env, real_addr) & TARGET_PAGE_MASK; >> + if (!address_space_access_valid(&address_space_memory, abs_addr, >> + TARGET_PAGE_SIZE, true)) { >> + program_interrupt(env, PGM_ADDRESSING, 4); >> + return 1; >> + } >> + >> + /* Check low-address protection */ >> + if ((env->cregs[0] & CR0_LOWPROT) != 0 && real_addr < 0x2000) { >> + program_interrupt(env, PGM_PROTECTION, 4); >> + return 1; >> + } >> + >> + for (i = 0; i < TARGET_PAGE_SIZE; i += 8) { >> + stq_phys(cs->as, abs_addr + i, 0); >> + } >> + >> + return 0; >> +} >> + >> uint32_t HELPER(tprot)(uint64_t a1, uint64_t a2) >> { >> /* XXX implement */ >> diff --git a/target/s390x/mmu_helper.c b/target/s390x/mmu_helper.c >> index b11a027..31eb9ef 100644 >> --- a/target/s390x/mmu_helper.c >> +++ b/target/s390x/mmu_helper.c >> @@ -108,7 +108,7 @@ static void trigger_page_fault(CPUS390XState *env, target_ulong vaddr, >> * Translate real address to absolute (= physical) >> * address by taking care of the prefix mapping. >> */ >> -static target_ulong mmu_real2abs(CPUS390XState *env, target_ulong raddr) >> +target_ulong mmu_real2abs(CPUS390XState *env, target_ulong raddr) >> { >> if (raddr < 0x2000) { >> return raddr + env->psa; /* Map the lowcore. */ >> diff --git a/target/s390x/translate.c b/target/s390x/translate.c >> index 4c48c59..61aa2c9 100644 >> --- a/target/s390x/translate.c >> +++ b/target/s390x/translate.c >> @@ -4057,6 +4057,15 @@ static ExitStatus op_tcxb(DisasContext *s, DisasOps *o) >> } >> >> #ifndef CONFIG_USER_ONLY >> + >> +static ExitStatus op_testblock(DisasContext *s, DisasOps *o) >> +{ >> + check_privileged(s); > > You should also call potential_page_fault as the helper can trigger an > exception. Right, that makes sense now, too. I'll send a v3 ... Thanks! Thomas
On 05/18/2017 06:20 AM, Aurelien Jarno wrote: >> +DEF_HELPER_FLAGS_2(testblock, TCG_CALL_NO_WG, i32, env, i64) > As the helper does not read any values from the global, you can even use > TCG_CALL_NO_RWG. > By throwing an exception, we imply a read of all values along the exception path. r~
On 2017-05-18 08:42, Richard Henderson wrote: > On 05/18/2017 06:20 AM, Aurelien Jarno wrote: > > > +DEF_HELPER_FLAGS_2(testblock, TCG_CALL_NO_WG, i32, env, i64) > > As the helper does not read any values from the global, you can even use > > TCG_CALL_NO_RWG. > > > > By throwing an exception, we imply a read of all values along the exception path. > You are indeed correct, sorry about the wrong comment.
diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h index 240b8a5..4f38ba0 100644 --- a/target/s390x/cpu.h +++ b/target/s390x/cpu.h @@ -1082,6 +1082,7 @@ struct sysib_322 { #define SIGP_ORDER_MASK 0x000000ff void load_psw(CPUS390XState *env, uint64_t mask, uint64_t addr); +target_ulong mmu_real2abs(CPUS390XState *env, target_ulong raddr); int mmu_translate(CPUS390XState *env, target_ulong vaddr, int rw, uint64_t asc, target_ulong *raddr, int *flags, bool exc); int sclp_service_call(CPUS390XState *env, uint64_t sccb, uint32_t code); diff --git a/target/s390x/helper.h b/target/s390x/helper.h index 0b70770..1fae191 100644 --- a/target/s390x/helper.h +++ b/target/s390x/helper.h @@ -102,6 +102,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_FLAGS_2(testblock, TCG_CALL_NO_WG, i32, 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 55a7c52..cac0f51 100644 --- a/target/s390x/insn-data.def +++ b/target/s390x/insn-data.def @@ -918,6 +918,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 f6e5bce..de0ecd4 100644 --- a/target/s390x/mem_helper.c +++ b/target/s390x/mem_helper.c @@ -20,6 +20,7 @@ #include "qemu/osdep.h" #include "cpu.h" +#include "exec/address-spaces.h" #include "exec/helper-proto.h" #include "exec/exec-all.h" #include "exec/cpu_ldst.h" @@ -973,6 +974,33 @@ void HELPER(stctl)(CPUS390XState *env, uint32_t r1, uint64_t a2, uint32_t r3) } } +uint32_t HELPER(testblock)(CPUS390XState *env, uint64_t real_addr) +{ + CPUState *cs = CPU(s390_env_get_cpu(env)); + uint64_t abs_addr; + int i; + + real_addr = fix_address(env, real_addr); + abs_addr = mmu_real2abs(env, real_addr) & TARGET_PAGE_MASK; + if (!address_space_access_valid(&address_space_memory, abs_addr, + TARGET_PAGE_SIZE, true)) { + program_interrupt(env, PGM_ADDRESSING, 4); + return 1; + } + + /* Check low-address protection */ + if ((env->cregs[0] & CR0_LOWPROT) != 0 && real_addr < 0x2000) { + program_interrupt(env, PGM_PROTECTION, 4); + return 1; + } + + for (i = 0; i < TARGET_PAGE_SIZE; i += 8) { + stq_phys(cs->as, abs_addr + i, 0); + } + + return 0; +} + uint32_t HELPER(tprot)(uint64_t a1, uint64_t a2) { /* XXX implement */ diff --git a/target/s390x/mmu_helper.c b/target/s390x/mmu_helper.c index b11a027..31eb9ef 100644 --- a/target/s390x/mmu_helper.c +++ b/target/s390x/mmu_helper.c @@ -108,7 +108,7 @@ static void trigger_page_fault(CPUS390XState *env, target_ulong vaddr, * Translate real address to absolute (= physical) * address by taking care of the prefix mapping. */ -static target_ulong mmu_real2abs(CPUS390XState *env, target_ulong raddr) +target_ulong mmu_real2abs(CPUS390XState *env, target_ulong raddr) { if (raddr < 0x2000) { return raddr + env->psa; /* Map the lowcore. */ diff --git a/target/s390x/translate.c b/target/s390x/translate.c index 4c48c59..61aa2c9 100644 --- a/target/s390x/translate.c +++ b/target/s390x/translate.c @@ -4057,6 +4057,15 @@ static ExitStatus op_tcxb(DisasContext *s, DisasOps *o) } #ifndef CONFIG_USER_ONLY + +static ExitStatus op_testblock(DisasContext *s, DisasOps *o) +{ + check_privileged(s); + gen_helper_testblock(cc_op, cpu_env, o->in2); + set_cc_static(s); + return NO_EXIT; +} + static ExitStatus op_tprot(DisasContext *s, DisasOps *o) { potential_page_fault(s); @@ -4064,6 +4073,7 @@ static ExitStatus op_tprot(DisasContext *s, DisasOps *o) set_cc_static(s); return NO_EXIT; } + #endif static ExitStatus op_tr(DisasContext *s, DisasOps *o)
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 <thuth@redhat.com> --- v2: - Use DEF_HELPER_FLAGS_2 instead for DEF_HELPER_2 for returning CC value - Convert real to absolute address - Added a check for valid RAM page - Added low-address protection check target/s390x/cpu.h | 1 + target/s390x/helper.h | 1 + target/s390x/insn-data.def | 2 ++ target/s390x/mem_helper.c | 28 ++++++++++++++++++++++++++++ target/s390x/mmu_helper.c | 2 +- target/s390x/translate.c | 10 ++++++++++ 6 files changed, 43 insertions(+), 1 deletion(-)