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

Nico Boehr
2023-07-14T10:33:13Z

Quoting Thomas Huth (2023-07-14 10:30:33)
> 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?

OK, how about this?

* Set up home address space to match primary space. Instead of running
* in home space all the time, we switch every time in sie() because:
* - tests that depend on running in primary space mode don't need to be
* touched
* - it avoids regressions in tests
* - switching every time makes it easier to extend this in the future,
* for example to allow tests to run in whatever space they want