On Wed, 16 Oct 2024 12:05:14 +0200
Heiko Carstens <hca@xxxxxxxxxxxxx> wrote:
> On Tue, Oct 15, 2024 at 06:43:20PM +0200, Claudio Imbrenda wrote:
> > Any program interrupt that happens in the host during the execution of
> > a KVM guest will now short circuit the fault handler and return to KVM
> > immediately. Guest fault handling (including pfault) will happen
> > entirely inside KVM.
> >
> > When sie64a() returns zero, current->thread.gmap_int_code will contain
> > the program interrupt number that caused the exit, or zero if the exit
> > was not caused by a host program interrupt.
> >
> > KVM will now take care of handling all guest faults in vcpu_post_run().
> >
> > Since gmap faults will not be visible by the rest of the kernel, remove
> > GMAP_FAULT, the linux fault handlers for secure execution faults, the
> > exception table entries for the sie instruction, the nop padding after
> > the sie instruction, and all other references to guest faults from the
> > s390 code.
>
> ...
>
> > diff --git a/arch/s390/kernel/traps.c b/arch/s390/kernel/traps.c
> ...
> > @@ -317,9 +318,23 @@ void noinstr __do_pgm_check(struct pt_regs *regs)
> > struct lowcore *lc = get_lowcore();
> > irqentry_state_t state;
> > unsigned int trapnr;
> > + union teid teid = { .val = lc->trans_exc_code };
> >
> > regs->int_code = lc->pgm_int_code;
> > - regs->int_parm_long = lc->trans_exc_code;
> > + regs->int_parm_long = teid.val;
> > +
> > + /*
> > + * In case of a guest fault, short-circuit the fault handler and return.
> > + * This way the sie64a() function will return 0; fault address and
> > + * other relevant bits are saved in current->thread.gmap_teid, and
> > + * the fault number in current->thread.gmap_int_code. KVM will be
> > + * able to use this information to handle the fault.
> > + */
> > + if (test_pt_regs_flag(regs, PIF_GUEST_FAULT) && (teid.as == PSW_BITS_AS_PRIMARY)) {
> > + current->thread.gmap_teid.val = regs->int_parm_long;
> > + current->thread.gmap_int_code = regs->int_code & 0xffff;
> > + return;
> > + }
>
> This check looks suboptimal to me for two reasons:
>
> - if PIF_GUEST_FAULT is set it should never happen that the normal
> exception handling code is executed; it is clearly a bug if that
> would happen, and with the above check this may or may not be
> recognized with a kernel crash, if I'm not mistaken.
>
> - __do_pgm_check() is executed for all program interruptions. This
> includes those interruptions which do not write a teid. Therefore
> the above check may do something unexpected depending on what teid a
> previous program interruption wrote. I think the teid.as check
> should be moved to kvm as well, and only be done for those cases
> where it is known that the teid contains a valid value.
makes sense
I'll move the checks into the kvm part of the code, and trigger a
KVM_BUG if things are wrong
