Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20230113171125.2846306-1-dionnaglaze@google.com>
In-Reply-To: <20230113171125.2846306-1-dionnaglaze@google.com>
From:   Ard Biesheuvel <ardb@kernel.org>
Date:   Fri, 13 Jan 2023 19:17:11 +0100
Message-ID: <CAMj1kXFFQ6QvbnjUBHhQOu+UxniMR9k+tB7kthBwjudw8tG38A@mail.gmail.com>
Subject: Re: [PATCH] x86/efi: Safely enable unaccepted memory in UEFI
To:     Dionna Glaze <dionnaglaze@google.com>,
        linux-efi <linux-efi@vger.kernel.org>
Cc:     linux-kernel@vger.kernel.org, x86@kernel.org, jiewen.yao@intel.com,
        devel@edk2.groups.io, "Min M. Xu" <min.m.xu@intel.org>,
        Gerd Hoffmann <kraxel@redhat.com>,
        James Bottomley <jejb@linux.ibm.com>,
        Tom Lendacky <Thomas.Lendacky@amd.com>,
        Erdem Aktas <erdemaktas@google.com>,
        "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>,
        Dave Hansen <dave.hansen@linux.intel.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

(cc linux-efi)

On Fri, 13 Jan 2023 at 18:11, Dionna Glaze <dionnaglaze@google.com> wrote:
>
> This patch depends on Kirill A. Shutemov's series
>
> [PATCHv8 00/14] mm, x86/cc: Implement support for unaccepted memory
>
> The UEFI v2.9 specification includes a new memory type to be used in
> environments where the OS must accept memory that is provided from its
> host. Before the introduction of this memory type, all memory was
> accepted eagerly in the firmware. In order for the firmware to safely
> stop accepting memory on the OS's behalf, the OS must affirmatively
> indicate support to the firmware.
>
> Enabling unaccepted memory requires calling a 0-argument enablement
> protocol before ExitBootServices. This call is only made if the kernel
> is compiled with UNACCEPTED_MEMORY=y
>
> The naming of the protocol guid is dependent on the standardization of
> the protocol, which is being discussed. Acceptance is contingent on
> the kernel community's approval.
>
> Cc: Ard Biescheuvel <ardb@kernel.org>
> Cc: "Min M. Xu" <min.m.xu@intel.org>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: James Bottomley <jejb@linux.ibm.com>
> Cc: Tom Lendacky <Thomas.Lendacky@amd.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Erdem Aktas <erdemaktas@google.com>
> Cc: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
> Cc: Dave Hansen <dave.hansen@linux.intel.com>
>
> Signed-off-by: Dionna Glaze <dionnaglaze@google.com>
> ---
>  drivers/firmware/efi/libstub/x86-stub.c | 36 +++++++++++++++++++++++++
>  include/linux/efi.h                     |  1 +
>  2 files changed, 37 insertions(+)
>
> diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c
> index a0bfd31358ba..5e9ebfbb49e6 100644
> --- a/drivers/firmware/efi/libstub/x86-stub.c
> +++ b/drivers/firmware/efi/libstub/x86-stub.c
> @@ -26,6 +26,17 @@ const efi_dxe_services_table_t *efi_dxe_table;
>  u32 image_offset __section(".data");
>  static efi_loaded_image_t *image = NULL;
>
> +union memory_acceptance_protocol {
> +       struct {
> +               efi_status_t (__efiapi *allow_unaccepted_memory)(
> +                       union memory_acceptance_protocol *);
> +       };
> +       struct {
> +               u32 allow_unaccepted_memory;
> +       } mixed_mode;
> +};
> +typedef union memory_acceptance_protocol memory_acceptance_protocol_t;
> +

Please put the typedef first, and use the defined type in the function
prototype, not the union.


>  static efi_status_t
>  preserve_pci_rom_image(efi_pci_io_protocol_t *pci, struct pci_setup_rom **__rom)
>  {
> @@ -310,6 +321,29 @@ setup_memory_protection(unsigned long image_base, unsigned long image_size)
>  #endif
>  }
>
> +
> +static void setup_unaccepted_memory(void)
> +{
> +#ifdef CONFIG_UNACCEPTED_MEMORY

Please drop the #ifdef

> +       efi_guid_t mem_acceptance_proto = EFI_MEMORY_ACCEPTANCE_PROTOCOL_GUID;
> +       memory_acceptance_protocol_t *proto;
> +       efi_status_t status;
> +

... and here, do

if (!IS_ENABLED(CONFIG_UNACCEPTED_MEMORY))
    return;

> +       /*
> +        * Enable unaccepted memory before calling exit boot services in order
> +        * for the UEFI to not accept all memory on EBS.
> +        */
> +       status = efi_bs_call(locate_protocol, &mem_acceptance_proto, NULL,
> +                            (void **)&proto);
> +       if (status != EFI_SUCCESS)
> +               return;
> +
> +       status = efi_call_proto(proto, allow_unaccepted_memory);
> +       if (status != EFI_SUCCESS)
> +               efi_err("Memory acceptance protocol failed\n");
> +#endif
> +}
> +
>  static const efi_char16_t apple[] = L"Apple";
>
>  static void setup_quirks(struct boot_params *boot_params,
> @@ -899,6 +933,8 @@ asmlinkage unsigned long efi_main(efi_handle_t handle,
>
>         setup_quirks(boot_params, bzimage_addr, buffer_end - buffer_start);
>
> +       setup_unaccepted_memory();
> +
>         status = exit_boot(boot_params, handle);
>         if (status != EFI_SUCCESS) {
>                 efi_err("exit_boot() failed!\n");
> diff --git a/include/linux/efi.h b/include/linux/efi.h
> index 4b27519143f5..bfc0e4f2aba5 100644
> --- a/include/linux/efi.h
> +++ b/include/linux/efi.h
> @@ -391,6 +391,7 @@ void efi_native_runtime_setup(void);
>  #define EFI_RT_PROPERTIES_TABLE_GUID           EFI_GUID(0xeb66918a, 0x7eef, 0x402a,  0x84, 0x2e, 0x93, 0x1d, 0x21, 0xc3, 0x8a, 0xe9)
>  #define EFI_DXE_SERVICES_TABLE_GUID            EFI_GUID(0x05ad34ba, 0x6f02, 0x4214,  0x95, 0x2e, 0x4d, 0xa0, 0x39, 0x8e, 0x2b, 0xb9)
>  #define EFI_SMBIOS_PROTOCOL_GUID               EFI_GUID(0x03583ff6, 0xcb36, 0x4940,  0x94, 0x7e, 0xb9, 0xb3, 0x9f, 0x4a, 0xfa, 0xf7)
> +#define EFI_MEMORY_ACCEPTANCE_PROTOCOL_GUID    EFI_GUID(0xc5a010fe, 0x38a7, 0x4531,  0x8a, 0x4a, 0x05, 0x00, 0xd2, 0xfd, 0x16, 0x49)
>
>  #define EFI_IMAGE_SECURITY_DATABASE_GUID       EFI_GUID(0xd719b2cb, 0x3d3a, 0x4596,  0xa3, 0xbc, 0xda, 0xd0, 0x0e, 0x67, 0x65, 0x6f)
>  #define EFI_SHIM_LOCK_GUID                     EFI_GUID(0x605dab50, 0xe046, 0x4300,  0xab, 0xb6, 0x3d, 0xd8, 0x10, 0xdd, 0x8b, 0x23)
> --
> 2.39.0.314.g84b9a713c41-goog
>