Re: [kvm-unit-tests PATCH v5 3/6] s390x: sie: switch to home space mode before entering SIE

Thomas Huth
2023-07-14T08:31:34Z

On 14/07/2023 10.21, Nico Boehr wrote:
> Quoting Thomas Huth (2023-07-13 10:21:12)
>> On 13/07/2023 10.17, Claudio Imbrenda wrote:
>>> On Thu, 13 Jul 2023 09:28:19 +0200
>>> Thomas Huth <thuth@redhat.com> wrote:
>>>
>>> [...]
>>>
>>>>> + irq_set_dat_mode(IRQ_DAT_ON, AS_PRIM);
>>>>> + psw_mask_clear_bits(PSW_MASK_HOME);
>>>>> +
>>>>> + /* restore the old CR 13 */
>>>>> + lctlg(13, old_cr13);
>>>>
>>>> Wouldn't it be better to always switch to HOME address mode directly in our
>>>> startup code already (where we enable DAT)? Switching back and forth every
>>>> time we enter SIE looks confusing to me ... or is there a reason why we
>>>> should continue to run in primary address mode by default and only switch to
>>>> home mode here?
>>>
>>> the existing tests are written with the assumption that they are
>>> running in primary mode.
>>>
>>> switching back and forth might be confusing, but avoids having to
>>> fix all the tests
>>
>> Which tests are breaking? And why? And how much effort would it be to fix them?
>
> Since you're not the first asking this, I took the time and
> moved^Whacked everything to home space mode:
>
> - all SIE-related tests time out, even when we load CR1 properly before SIE
> entry. Most likely just an oversight and fixable.
> - the skey test encounters an unexpected PGM int with a weird backtrace
> where I couldn't easily figure out what goes wrong
> - edat test fails with a similar looking backtrace
>
> All in all, it is probably fixable, but additional effort.
>
> I think explicitly switching the address space mode gives us additional
> flexibility, since sie() doesn't need to make assumptions about which address
> space we're running in.

Ok, thanks for checking. Then let's go with this patch here for now, but
maybe you could add a more detailed comment in the source code that talks
about the reasons for switching each time instead of only once during startup?

Thanks,
Thomas