On 27.09.19 18:16, Richard Henderson wrote:
> On 9/27/19 4:02 AM, David Hildenbrand wrote:
>> On 26.09.19 18:26, Richard Henderson wrote:
>>> We currently set ilen to AUTO, then overwrite that during
>>> unwinding, then overwrite that for the code access case.
>>>
>>> This can be simplified to setting ilen to our arbitrary
>>> value for the (undefined) code access case, then rely on
>>> unwinding to overwrite that with the correct value for
>>> the data access case.
>>>
>>> Signed-off-by: Richard Henderson <richard.hender...@linaro.org>
>>> ---
>>> target/s390x/excp_helper.c | 23 +++++++----------------
>>> 1 file changed, 7 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/target/s390x/excp_helper.c b/target/s390x/excp_helper.c
>>> index 98a1ee8317..8ce992e639 100644
>>> --- a/target/s390x/excp_helper.c
>>> +++ b/target/s390x/excp_helper.c
>>> @@ -96,7 +96,7 @@ bool s390_cpu_tlb_fill(CPUState *cs, vaddr address, int
>>> size,
>>> {
>>> S390CPU *cpu = S390_CPU(cs);
>>>
>>> - trigger_pgm_exception(&cpu->env, PGM_ADDRESSING, ILEN_AUTO);
>>> + trigger_pgm_exception(&cpu->env, PGM_ADDRESSING, ILEN_UNWIND);
>>
>> Hmm, we always trigger a pgm exceptions, meaning we set
>> cs->exception_index even if we have probe = true. Confused by that.
>
> This is the CONFIG_USER_ONLY version, for which probe is always false.
> Perhaps
> I shouldn't have made the function interface identical, but it did appear to
> make things cleaner for most targets.
>
>>> + trigger_pgm_exception(env, excp, 2);
>>
>> I wonder if it is still worth setting this only conditionally. Most
>> probably not.
>
> I don't see that it would be. I hope the comment is clear about this
> arbitrary
> value is overwritten during unwinding.
It's confusing, but I get it :)
--
Thanks,
David / dhildenb
