Re: [PATCH 07/32] s390/traps: Handle nullifying program checks in get_trap_ip()

[Heiko Carstens]

On Mon, Oct 12, 2026 at 03:01:56PM +0200, Ilya Leoshkevich wrote:
> > @@ -41,15 +42,19 @@ struct pgm_stat {
> >   static DEFINE_PER_CPU_SHARED_ALIGNED(struct pgm_stat, pgm_stat);
> > -static inline void __user *get_trap_ip(struct pt_regs *regs)
> > +static inline unsigned long get_trap_ip(struct pt_regs *regs)
> >   {
> > -	unsigned long address;
> > +	static const unsigned long nullified[2] = { PGM_CHECKS_NULLIFIED };
> 
> Maybe just [], to deal with the possibility of expansion in the future?

That is also intentional since there are quite a few hard assumptions
in the kernel that only interruption codes in the range of 0x0..0x7f are
supported. But I do agree that a hard-coded "2" doesn't look too nice.

I guess a follow clean-up patch could address that.

It is similar to the hard coded 128 here:

static void (*pgm_check_table[128])(struct pt_regs *regs) = {

in traps.c - which is also not nice.

> > @@ -152,7 +157,7 @@ static void illegal_op(struct pt_regs *regs)
> >   	int signal = 0;
> >   	u16 opcode;
> > -	location = get_trap_ip(regs);
> > +	location = (void __user *)get_trap_ip(regs);
> 
> Should location be moved inside the "if"? Now it reads weirdly: we cast
> the pointer to __user before checking for user mode.
> 
> >   	if (user_mode(regs)) {
> >   		if (get_user(opcode, location))

You are suggesting to move the assignment like this:

	if (user_mode(regs)) {
		location = (void __user *)get_trap_ip(regs);
		if (get_user(opcode, location))

? Or something else? Sure, why not. Just let me know what you prefer.