Re: [PATCH v2 3/3] efi: x86: Wire up IBT annotation in memory attributes table

From: Peter Zijlstra <peterz@infradead.org>
To: Dave Hansen <dave.hansen@intel.com>
Cc: Ard Biesheuvel <ardb@kernel.org>,
	linux-efi@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>, Kees Cook <keescook@chromium.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
	Dave Hansen <dave.hansen@linux.intel.com>
Subject: Re: [PATCH v2 3/3] efi: x86: Wire up IBT annotation in memory attributes table
Date: Wed, 8 Feb 2023 21:14:53 +0100	[thread overview]
Message-ID: <Y+QCvSmdrqaJjxoT@hirez.programming.kicks-ass.net> (raw)
In-Reply-To: <ca9dc359-899b-ec69-36e0-17225189da01@intel.com>

On Wed, Feb 08, 2023 at 07:17:15AM -0800, Dave Hansen wrote:
> On 2/6/23 04:49, Ard Biesheuvel wrote:
> > --- a/arch/x86/kernel/apm_32.c
> > +++ b/arch/x86/kernel/apm_32.c
> > @@ -609,7 +609,7 @@ static long __apm_bios_call(void *_call)
> >  
> >  	apm_irq_save(flags);
> >  	firmware_restrict_branch_speculation_start();
> > -	ibt = ibt_save();
> > +	ibt = ibt_save(true);
> 
> My only nit with these is the bare use of 'true'/'false'.  It's
> impossible to tell at the call-site what the 'true' means.  So, if you
> happen to respin these and see a nice way to remedy this I'd appreciate it.

I've often wished for a named argument extention to C, much like named
initializers, such that one can write:

	ibt_save(.disable = true);

Because the thing you mention is very common with boolean arguments, the
what gets lost in the argument name and true/false just isn't very
telling.

But yeah, even if by some miracle all compiler guys were like, YES! and
implemented it tomorrow, we couldn't use it for a good few years anyway
:-/