Re: [qemu-s390x] [PATCH-for-4.2 v1 1/9] s390x/mmu: Better ASC selection in s390_cpu_get_phys_page_debug()

[David Hildenbrand]

On 08.08.19 14:57, Cornelia Huck wrote:
> On Mon,  5 Aug 2019 17:29:39 +0200
> David Hildenbrand <address@hidden> wrote:
> 
>> Let's select the ASC before calling the function and use MMU_DATA_LOAD.
>> This is a preparation to:
>> - Remove the ASC magic depending on the access mode from mmu_translate
>> - Implement IEP support, where we could run into access exceptions
> 
> 'IEP' was instruction execution protection?
> 
>>   trying to fetch instructions
>>
>> Signed-off-by: David Hildenbrand <address@hidden>
>> ---
>>  target/s390x/helper.c | 10 +++++++++-
>>  1 file changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/target/s390x/helper.c b/target/s390x/helper.c
>> index 13ae9909ad..08166558a0 100644
>> --- a/target/s390x/helper.c
>> +++ b/target/s390x/helper.c
>> @@ -58,7 +58,15 @@ hwaddr s390_cpu_get_phys_page_debug(CPUState *cs, vaddr 
>> vaddr)
>>          vaddr &= 0x7fffffff;
>>      }
>>  
>> -    if (mmu_translate(env, vaddr, MMU_INST_FETCH, asc, &raddr, &prot, 
>> false)) {
>> +    /*
>> +     * We want to read the code, however, not run into access exceptions
>> +     * (especially, IEP).
>> +     */
>> +    if (asc != PSW_ASC_HOME) {
>> +        asc = PSW_ASC_PRIMARY;
>> +    }
> 
> Previously, if we'd go in here specifying access register mode, we'd
> hw_error() out. Now, that would be rewritten to using primary. Could
> that be a problem, or do we filter out access register mode even
> earlier?

As this is just a debug interface it's not an issue (it actually makes
sure that we really read code and not data). Any guest that would be
trying to read data while in AR mode would immediately crash.

> 
> (As an aside, I'm not sure the guest crashing qemu by specifying access
> register mode was a good idea. Or do we get to slap the guest before
> that happens?)

I guess there isn't too much we can do - continue running the guest in a
wrong mode would lead to data corruption :/ And as it's core ISA, we
cannot really forbid switching to it. The only solution is to implement
AR mode :D

-- 

Thanks,

David / dhildenb