All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Ahmed S. Darwish" <darwi@linutronix.de>
To: Marcos Del Sol Vives <marcos@orca.pet>
Cc: linux-kernel@vger.kernel.org,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	x86@kernel.org, "H. Peter Anvin" <hpa@zytor.com>,
	Brian Gerst <brgerst@gmail.com>, Uros Bizjak <ubizjak@gmail.com>,
	Ard Biesheuvel <ardb@kernel.org>,
	David Kaplan <david.kaplan@amd.com>, Kees Cook <kees@kernel.org>,
	"Peter Zijlstra (Intel)" <peterz@infradead.org>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	Oleg Nesterov <oleg@redhat.com>, "Xin Li (Intel)" <xin@zytor.com>,
	Sabyrzhan Tasbolatov <snovitoll@gmail.com>
Subject: Re: [PATCH] x86: add hintable NOPs emulation
Date: Wed, 20 Aug 2025 11:14:25 +0200	[thread overview]
Message-ID: <aKWR8e6VUEZEgbkw@lx-t490> (raw)
In-Reply-To: <20250820013452.495481-1-marcos@orca.pet>

Hi Marcos,

On Wed, 20 Aug 2025, Marcos Del Sol Vives wrote:
...
>
> --- a/arch/x86/include/asm/processor.h
> +++ b/arch/x86/include/asm/processor.h
> @@ -499,6 +499,10 @@ struct thread_struct {
>
>  	unsigned int		iopl_warn:1;
>
> +#ifdef CONFIG_X86_HNOP_EMU
> +	unsigned int		hnop_warn:1;
> +#endif
> +
...
> --- a/arch/x86/kernel/process.c
> +++ b/arch/x86/kernel/process.c
> @@ -178,6 +178,9 @@ int copy_thread(struct task_struct *p, const struct kernel_clone_args *args)
>  	p->thread.io_bitmap = NULL;
>  	clear_tsk_thread_flag(p, TIF_IO_BITMAP);
>  	p->thread.iopl_warn = 0;
> +#ifdef CONFIG_X86_HNOP_EMU
> +	p->thread.hnop_warn = 0;
> +#endif
...
> --- a/arch/x86/kernel/traps.c
> +++ b/arch/x86/kernel/traps.c
> +
> +	if (!t->hnop_warn) {
> +		pr_warn_ratelimited("%s[%d] emulating hintable NOP, ip:%lx\n",
> +		       current->comm, task_pid_nr(current), regs->ip);
> +		t->hnop_warn = 1;
> +	}

Can we please remove all this 'hnop_warn' trickery?  Removing it will
simplifiy the code and avoid complicating 'thread_struct' further.

It's just the kernel doing its normal job.

And if the system is full of binaries with hintable NOPs, ratelimiting
will not save you much.  I got hit recently by a 'ratelimited'
correctible error PCI subsystem warning, and it still overflows the log
buffers of my Thinkpad laptop, in just 4 to 5 days :(

>
> static inline void handle_invalid_op(struct pt_regs *regs)
> {
> +#ifdef CONFIG_X86_HNOP_EMU
> +	if (user_mode(regs) && handle_hnop(regs))
> +		return;
> +#endif
> +
>

CPP conditionals within C function code are ugly.  Please do instead:

    static bool handle_hnop(struct pt_regs *regs)
    {
	if (!IS_ENABLED(CONFIG_X86_HNOP_EMU))
		return false;
	...
    }

Thanks for your contribution!

--
Ahmed S. Darwish
Linutronix GmbH

  parent reply	other threads:[~2025-08-20  9:14 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-20  1:34 [PATCH] x86: add hintable NOPs emulation Marcos Del Sol Vives
2025-08-20  9:07 ` Peter Zijlstra
2025-08-21 12:28   ` David Laight
2025-08-21 12:46     ` Peter Zijlstra
2025-08-21 18:40       ` David Laight
2025-08-21 19:46         ` Marcos Del Sol Vives
2025-08-21 15:11     ` Marcos Del Sol Vives
2025-08-20  9:14 ` Ahmed S. Darwish [this message]
2025-08-20  9:33   ` Marcos Del Sol Vives
2025-08-20  9:43     ` Borislav Petkov
2025-08-20  9:51       ` Marcos Del Sol Vives
2025-08-20  9:55         ` Borislav Petkov
2025-08-20 10:01           ` Marcos Del Sol Vives
2025-08-20 10:08             ` Borislav Petkov
2025-08-20 10:21               ` Marcos Del Sol Vives
2025-08-20 10:30                 ` Borislav Petkov
2025-08-21  2:00             ` Kees Cook
2025-08-20 10:11     ` Ahmed S. Darwish
2025-08-20 10:30       ` Ahmed S. Darwish
2025-08-21  1:43 ` H. Peter Anvin
2025-08-21  9:35   ` Marcos Del Sol Vives
2025-08-21  5:02 ` H. Peter Anvin
2025-08-21 12:26 ` David Laight
2025-08-21 12:48   ` Marcos Del Sol Vives
2025-08-21 12:48 ` Peter Zijlstra
2025-08-21 13:45   ` Marcos Del Sol Vives
2025-08-21 13:59     ` Peter Zijlstra
2025-08-22 22:12   ` H. Peter Anvin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aKWR8e6VUEZEgbkw@lx-t490 \
    --to=darwi@linutronix.de \
    --cc=andrew.cooper3@citrix.com \
    --cc=ardb@kernel.org \
    --cc=bp@alien8.de \
    --cc=brgerst@gmail.com \
    --cc=dave.hansen@linux.intel.com \
    --cc=david.kaplan@amd.com \
    --cc=hpa@zytor.com \
    --cc=kees@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=marcos@orca.pet \
    --cc=mingo@redhat.com \
    --cc=oleg@redhat.com \
    --cc=peterz@infradead.org \
    --cc=snovitoll@gmail.com \
    --cc=tglx@linutronix.de \
    --cc=ubizjak@gmail.com \
    --cc=x86@kernel.org \
    --cc=xin@zytor.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.