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