On Wed, Aug 11 2021, David Hildenbrand <da...@redhat.com> wrote: > Let's replace the ram_size check by a proper physical address space > check (for example, to prepare for memory hotplug), trigger addressing > exceptions and trace the return value of the storage key getter/setter. > > Provide an helper mmu_absolute_addr_valid() to be used in other context > soon. Always test for "read" instead of "write" as we are not actually > modifying the page itself. > > Signed-off-by: David Hildenbrand <da...@redhat.com> > --- > target/s390x/helper.h | 6 +++--- > target/s390x/mmu_helper.c | 8 ++++++++ > target/s390x/s390x-internal.h | 1 + > target/s390x/tcg/mem_helper.c | 36 ++++++++++++++++++++++------------- > 4 files changed, 35 insertions(+), 16 deletions(-) >
> diff --git a/target/s390x/tcg/mem_helper.c b/target/s390x/tcg/mem_helper.c > index dd506d8d17..90ac82fdcc 100644 > --- a/target/s390x/tcg/mem_helper.c > +++ b/target/s390x/tcg/mem_helper.c > @@ -28,6 +28,7 @@ > #include "qemu/int128.h" > #include "qemu/atomic128.h" > #include "tcg/tcg.h" > +#include "trace.h" > > #if !defined(CONFIG_USER_ONLY) > #include "hw/s390x/storage-keys.h" > @@ -2171,15 +2172,15 @@ uint32_t HELPER(tprot)(CPUS390XState *env, uint64_t > a1, uint64_t a2) > /* insert storage key extended */ > uint64_t HELPER(iske)(CPUS390XState *env, uint64_t r2) > { > - MachineState *ms = MACHINE(qdev_get_machine()); > static S390SKeysState *ss; > static S390SKeysClass *skeyclass; > uint64_t addr = wrap_address(env, r2); > uint8_t key; > + int rc; > > addr = mmu_real2abs(env, addr); > - if (addr > ms->ram_size) { > - return 0; > + if (!mmu_absolute_addr_valid(addr, false)) { > + trigger_pgm_exception(env, PGM_ADDRESSING); Don't you need s390_program_interrupt() instead? > } > > if (unlikely(!ss)) {