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.
