Re: [Qemu-devel] [PATCH-for-4.2 v2 2/6] s390x/tcg: Rework MMU selection for instruction fetches

[David Hildenbrand]

On 15.08.19 17:43, Cornelia Huck wrote:
> On Wed, 14 Aug 2019 09:23:51 +0200
> David Hildenbrand <address@hidden> wrote:
> 
>> Instructions are always fetched from primary address space, except when
>> in home address mode. Perform the selection directly in cpu_mmu_index().
>>
>> get_mem_index() is only used to perform data access, instructions are
>> fetched via cpu_lduw_code(), which translates to cpu_mmu_index(env, true).
>>
>> We don't care about restricting the access permissions of the TLB
>> entries anymore, as we no longer enter PRIMARY entries into the
>> SECONDARY MMU. Cleanup related code a bit.
>>
>> Signed-off-by: David Hildenbrand <address@hidden>
>> ---
>>  target/s390x/cpu.h        |  7 +++++++
>>  target/s390x/mmu_helper.c | 38 +++++++++++++++-----------------------
>>  2 files changed, 22 insertions(+), 23 deletions(-)
>>
> 
> (...)
> 
>> diff --git a/target/s390x/mmu_helper.c b/target/s390x/mmu_helper.c
>> index 6e9c4d6151..c34e8d2021 100644
>> --- a/target/s390x/mmu_helper.c
>> +++ b/target/s390x/mmu_helper.c
>> @@ -349,8 +349,9 @@ int mmu_translate(CPUS390XState *env, target_ulong 
>> vaddr, int rw, uint64_t asc,
>>  {
>>      static S390SKeysState *ss;
>>      static S390SKeysClass *skeyclass;
>> -    int r = -1;
>> +    uint64_t asce;
>>      uint8_t key;
>> +    int r;
>>  
>>      if (unlikely(!ss)) {
>>          ss = s390_get_skeys_device();
>> @@ -380,36 +381,21 @@ int mmu_translate(CPUS390XState *env, target_ulong 
>> vaddr, int rw, uint64_t asc,
>>  
>>      if (!(env->psw.mask & PSW_MASK_DAT)) {
>>          *raddr = vaddr;
>> -        r = 0;
>> -        goto out;
>> +        goto nodat;
>>      }
>>  
>>      switch (asc) {
>>      case PSW_ASC_PRIMARY:
>>          PTE_DPRINTF("%s: asc=primary\n", __func__);
>> -        r = mmu_translate_asce(env, vaddr, asc, env->cregs[1], raddr, flags,
>> -                               rw, exc);
>> +        asce = env->cregs[1];
>>          break;
>>      case PSW_ASC_HOME:
>>          PTE_DPRINTF("%s: asc=home\n", __func__);
>> -        r = mmu_translate_asce(env, vaddr, asc, env->cregs[13], raddr, 
>> flags,
>> -                               rw, exc);
>> +        asce = env->cregs[13];
>>          break;
>>      case PSW_ASC_SECONDARY:
>>          PTE_DPRINTF("%s: asc=secondary\n", __func__);
>> -        /*
>> -         * Instruction: Primary
>> -         * Data: Secondary
>> -         */
>> -        if (rw == MMU_INST_FETCH) {
>> -            r = mmu_translate_asce(env, vaddr, PSW_ASC_PRIMARY, 
>> env->cregs[1],
>> -                                   raddr, flags, rw, exc);
>> -            *flags &= ~(PAGE_READ | PAGE_WRITE);
>> -        } else {
>> -            r = mmu_translate_asce(env, vaddr, PSW_ASC_SECONDARY, 
>> env->cregs[7],
>> -                                   raddr, flags, rw, exc);
>> -            *flags &= ~(PAGE_EXEC);
>> -        }
>> +        asce = env->cregs[7];
>>          break;
>>      case PSW_ASC_ACCREG:
>>      default:
>> @@ -417,11 +403,17 @@ int mmu_translate(CPUS390XState *env, target_ulong 
>> vaddr, int rw, uint64_t asc,
>>          break;
>>      }
>>  
>> - out:
>> +    /* perform the DAT translation */
>> +    r = mmu_translate_asce(env, vaddr, asc, asce, raddr, flags, rw, exc);
>> +    if (r) {
>> +        return r;
>> +    }
>> +
>> +nodat:
>>      /* Convert real address -> absolute address */
>>      *raddr = mmu_real2abs(env, *raddr);
>>  
>> -    if (r == 0 && *raddr < ram_size) {
>> +    if (*raddr < ram_size) {
>>          if (skeyclass->get_skeys(ss, *raddr / TARGET_PAGE_SIZE, 1, &key)) {
>>              trace_get_skeys_nonzero(r);
> 
> I think you might up here with an uninitialized r before patch 4?

Right, will reshuffle. Thanks!

-- 

Thanks,

David / dhildenb