Received-SPF: pass (google.com: domain of linux-kernel+bounces-15553-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Precedence: bulk
MIME-Version: 1.0
References: <20231215122614.5481-1-tzimmermann@suse.de> <20231215122614.5481-3-tzimmermann@suse.de>
 <CAMj1kXHrn-PxpMGnR4VoHv7kHvQYyf8SS9i1irm9Gi_uBseciw@mail.gmail.com> <97f118fc-b38f-4bcc-83d3-4d3c13edf7a0@suse.de>
In-Reply-To: <97f118fc-b38f-4bcc-83d3-4d3c13edf7a0@suse.de>
From: Ard Biesheuvel <ardb@kernel.org>
Date: Wed, 3 Jan 2024 14:11:40 +0100
Message-ID: <CAMj1kXF-1TXYzheS-e_rGKnV+6FrkZe+e2sfCixyUzxSQE7X6w@mail.gmail.com>
Subject: Re: [PATCH v2 2/3] arch/x86: Add <asm/ima-efi.h> for arch_ima_efi_boot_mode
To: Thomas Zimmermann <tzimmermann@suse.de>
Cc: tglx@linutronix.de, mingo@redhat.com, bp@alien8.de, 
	dave.hansen@linux.intel.com, x86@kernel.org, hpa@zytor.com, 
	bhelgaas@google.com, arnd@arndb.de, zohar@linux.ibm.com, 
	dmitry.kasatkin@gmail.com, paul@paul-moore.com, jmorris@namei.org, 
	serge@hallyn.com, javierm@redhat.com, linux-arch@vger.kernel.org, 
	linux-efi@vger.kernel.org, linux-kernel@vger.kernel.org, 
	linux-pci@vger.kernel.org, linux-integrity@vger.kernel.org, 
	linux-security-module@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"

On Tue, 2 Jan 2024 at 15:07, Thomas Zimmermann <tzimmermann@suse.de> wrote:
>
> Hii Ard
>
> Am 19.12.23 um 12:38 schrieb Ard Biesheuvel:
> > Hi Thomas,
> >
> > On Fri, 15 Dec 2023 at 13:26, Thomas Zimmermann <tzimmermann@suse.de> wrote:
> >>
> >> The header file <asm/efi.h> contains the macro arch_ima_efi_boot_mode,
> >> which expands to use struct boot_params from <asm/bootparams.h>. Many
> >> drivers include <linux/efi.h>, but do not use boot parameters. Changes
> >> to bootparam.h or its included headers can easily trigger large,
> >> unnessary rebuilds of the kernel.
> >>
> >> Moving x86's arch_ima_efi_boot_mode to <asm/ima-efi.h> and including
> >> <asm/setup.h> separates that dependency from the rest of the EFI
> >> interfaces. The only user is in ima_efi.c. As the file already declares
> >> a default value for arch_ima_efi_boot_mode, move this define into
> >> asm-generic for all other architectures.
> >>
> >> With arch_ima_efi_boot_mode removed from efi.h, <asm/bootparam.h> can
> >> later be removed from further x86 header files.
> >>
> >
> > Apologies if I missed this in v1 but is the new asm-generic header
> > really necessary? Could we instead turn arch_ima_efi_boot_mode into a
> > function that is a static inline { return unset; } by default, but can
> > be emitted out of line in one of the x86/platform/efi.c source files,
> > where referring to boot_params is fine?
>
> I cannot figure out how to do this without *something* in asm-generic or
> adding if-CONFIG_X86 guards in ima-efi.c.
>
> But I noticed that linux/efi.h already contains 2 or 3 ifdef branches
> for x86. Would it be an option to move this code into asm/efi.h
> (including a header file in asm-generic for the non-x86 variants) and
> add the arch_ima_efi_boot_mode() helper there as well?  At least that
> wouldn't be a header for only a single define.
>

Could we just move the x86 implementation out of line?

So something like this in arch/x86/include/asm/efi.h

enum efi_secureboot_mode x86_ima_efi_boot_mode(void);
#define arch_ima_efi_boot_mode x86_ima_efi_boot_mode()

and an implementation in one of the related .c files:

enum efi_secureboot_mode x86_ima_efi_boot_mode(void)
{
    return boot_params.secure_boot;
}

?