| Message ID | 20230712114149.1291580-4-nrb@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | s390x: Add support for running guests without MSO/MSL | expand |
On 12/07/2023 13.41, Nico Boehr wrote: > This is to prepare for running guests without MSO/MSL, which is > currently not possible. > > We already have code in sie64a to setup a guest primary ASCE before > entering SIE, so we can in theory switch to the page tables which > translate gpa to hpa. > > But the host is running in primary space mode already, so changing the > primary ASCE before entering SIE will also affect the host's code and > data. > > To make this switch useful, the host should run in a different address > space mode. Hence, set up and change to home address space mode before > installing the guest ASCE. > > The home space ASCE is just copied over from the primary space ASCE, so > no functional change is intended, also for tests that want to use > MSO/MSL. If a test intends to use a different primary space ASCE, it can > now just set the guest.asce in the save_area. > > Signed-off-by: Nico Boehr <nrb@linux.ibm.com> > --- > lib/s390x/asm/arch_def.h | 1 + > lib/s390x/sie.c | 18 ++++++++++++++++++ > 2 files changed, 19 insertions(+) > > diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h > index 53279572a9ee..65e1cf58c7e7 100644 > --- a/lib/s390x/asm/arch_def.h > +++ b/lib/s390x/asm/arch_def.h > @@ -91,6 +91,7 @@ struct cpu { > #define AS_HOME 3 > > #define PSW_MASK_DAT 0x0400000000000000UL > +#define PSW_MASK_HOME 0x0000C00000000000UL > #define PSW_MASK_IO 0x0200000000000000UL > #define PSW_MASK_EXT 0x0100000000000000UL > #define PSW_MASK_KEY 0x00F0000000000000UL > diff --git a/lib/s390x/sie.c b/lib/s390x/sie.c > index 9241b4b4a512..ffa8ec91a423 100644 > --- a/lib/s390x/sie.c > +++ b/lib/s390x/sie.c > @@ -46,6 +46,8 @@ void sie_handle_validity(struct vm *vm) > > void sie(struct vm *vm) > { > + uint64_t old_cr13; > + > if (vm->sblk->sdf == 2) > memcpy(vm->sblk->pv_grregs, vm->save_area.guest.grs, > sizeof(vm->save_area.guest.grs)); > @@ -53,6 +55,16 @@ void sie(struct vm *vm) > /* Reset icptcode so we don't trip over it below */ > vm->sblk->icptcode = 0; > > + /* set up home address space to match primary space */ > + old_cr13 = stctg(13); > + lctlg(13, stctg(1)); > + > + /* switch to home space so guest tables can be different from host */ > + psw_mask_set_bits(PSW_MASK_HOME); > + > + /* also handle all interruptions in home space while in SIE */ > + irq_set_dat_mode(IRQ_DAT_ON, AS_HOME); > + > while (vm->sblk->icptcode == 0) { > sie64a(vm->sblk, &vm->save_area); > sie_handle_validity(vm); > @@ -60,6 +72,12 @@ void sie(struct vm *vm) > vm->save_area.guest.grs[14] = vm->sblk->gg14; > vm->save_area.guest.grs[15] = vm->sblk->gg15; > > + 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? Thomas
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 > > Thomas > >
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? Thomas
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. It currently does, but that's fixable - in contrast to when we don't switch.
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
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
diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h index 53279572a9ee..65e1cf58c7e7 100644 --- a/lib/s390x/asm/arch_def.h +++ b/lib/s390x/asm/arch_def.h @@ -91,6 +91,7 @@ struct cpu { #define AS_HOME 3 #define PSW_MASK_DAT 0x0400000000000000UL +#define PSW_MASK_HOME 0x0000C00000000000UL #define PSW_MASK_IO 0x0200000000000000UL #define PSW_MASK_EXT 0x0100000000000000UL #define PSW_MASK_KEY 0x00F0000000000000UL diff --git a/lib/s390x/sie.c b/lib/s390x/sie.c index 9241b4b4a512..ffa8ec91a423 100644 --- a/lib/s390x/sie.c +++ b/lib/s390x/sie.c @@ -46,6 +46,8 @@ void sie_handle_validity(struct vm *vm) void sie(struct vm *vm) { + uint64_t old_cr13; + if (vm->sblk->sdf == 2) memcpy(vm->sblk->pv_grregs, vm->save_area.guest.grs, sizeof(vm->save_area.guest.grs)); @@ -53,6 +55,16 @@ void sie(struct vm *vm) /* Reset icptcode so we don't trip over it below */ vm->sblk->icptcode = 0; + /* set up home address space to match primary space */ + old_cr13 = stctg(13); + lctlg(13, stctg(1)); + + /* switch to home space so guest tables can be different from host */ + psw_mask_set_bits(PSW_MASK_HOME); + + /* also handle all interruptions in home space while in SIE */ + irq_set_dat_mode(IRQ_DAT_ON, AS_HOME); + while (vm->sblk->icptcode == 0) { sie64a(vm->sblk, &vm->save_area); sie_handle_validity(vm); @@ -60,6 +72,12 @@ void sie(struct vm *vm) vm->save_area.guest.grs[14] = vm->sblk->gg14; vm->save_area.guest.grs[15] = vm->sblk->gg15; + 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); + if (vm->sblk->sdf == 2) memcpy(vm->save_area.guest.grs, vm->sblk->pv_grregs, sizeof(vm->save_area.guest.grs));
This is to prepare for running guests without MSO/MSL, which is currently not possible. We already have code in sie64a to setup a guest primary ASCE before entering SIE, so we can in theory switch to the page tables which translate gpa to hpa. But the host is running in primary space mode already, so changing the primary ASCE before entering SIE will also affect the host's code and data. To make this switch useful, the host should run in a different address space mode. Hence, set up and change to home address space mode before installing the guest ASCE. The home space ASCE is just copied over from the primary space ASCE, so no functional change is intended, also for tests that want to use MSO/MSL. If a test intends to use a different primary space ASCE, it can now just set the guest.asce in the save_area. Signed-off-by: Nico Boehr <nrb@linux.ibm.com> --- lib/s390x/asm/arch_def.h | 1 + lib/s390x/sie.c | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+)