Re: [qemu-s390x] [PATCH v1 1/2] s390x/tcg: implement TEST PROTECTION

[David Hildenbrand]

On 19.12.2017 09:20, Thomas Huth wrote:
> On 18.12.2017 23:46, David Hildenbrand wrote:
>> Linux uses TEST PROTECTION to sense for available memory locations.
>>
>> Let's implement what we can for now (just as for the other instructions,
>> excluding AR mode and special protection mechanisms).
>>
>> Signed-off-by: David Hildenbrand <address@hidden>
>> ---
>>  target/s390x/helper.h     |  2 +-
>>  target/s390x/mem_helper.c | 41 +++++++++++++++++++++++++++++++++++++++--
>>  target/s390x/translate.c  |  2 +-
>>  3 files changed, 41 insertions(+), 4 deletions(-)
>>
>> diff --git a/target/s390x/helper.h b/target/s390x/helper.h
>> index 2f17b62d3d..26c1b07b44 100644
>> --- a/target/s390x/helper.h
>> +++ b/target/s390x/helper.h
>> @@ -137,7 +137,7 @@ 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_3(tprot, TCG_CALL_NO_RWG, i32, env, 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)
>>  DEF_HELPER_FLAGS_2(rrbe, TCG_CALL_NO_RWG, i32, env, i64)
>> diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
>> index 2625d843b3..359e446c6f 100644
>> --- a/target/s390x/mem_helper.c
>> +++ b/target/s390x/mem_helper.c
>> @@ -1717,9 +1717,46 @@ uint32_t HELPER(testblock)(CPUS390XState *env, 
>> uint64_t real_addr)
>>      return 0;
>>  }
>>  
>> -uint32_t HELPER(tprot)(uint64_t a1, uint64_t a2)
>> +uint32_t HELPER(tprot)(CPUS390XState *env, uint64_t a1, uint64_t a2)
>>  {
>> -    /* XXX implement */
>> +    S390CPU *cpu = s390_env_get_cpu(env);
>> +    CPUState *cs = CPU(cpu);
>> +
>> +    /*
>> +     * TODO: we currently don't handle all access protection types
>> +     * (including access-list and key-controlled) as well as AR mode.
>> +     */
> 
> Maybe add some "if (a2 & 0xf0) qemu_log_mask(LOG_UNIMPL, ...)" so that
> we've got a chance to detect such conditions later?
> 

In general I would agree, however as such acess-list + storage key stuff
is missing literally all over the place, I don't think it is very helpful.

Once (if ever) we implement it, we will have to go through the whole PoP
either way and fixup all functions.

Thanks!

-- 

Thanks,

David / dhildenb