Re: [kvm-unit-tests RFC PATCH 1/3] x86 EFI: Bypass call to fdt_check_header()

From: "Paluri, PavanKumar" <papaluri@amd.com>
To: Andrew Jones <andrew.jones@linux.dev>
Cc: kvm@vger.kernel.org, pbonzini@redhat.com,
	nikos.nikoleris@arm.com, thomas.lendacky@amd.com,
	michael.roth@amd.com, amit.shah@amd.com
Subject: Re: [kvm-unit-tests RFC PATCH 1/3] x86 EFI: Bypass call to fdt_check_header()
Date: Tue, 26 Mar 2024 10:30:19 -0500	[thread overview]
Message-ID: <391fc273-0385-eaca-5c07-277ceaf1815a@amd.com> (raw)
In-Reply-To: <20240326-663042e3295513fb8814f80d@orel>



On 3/26/2024 3:51 AM, Andrew Jones wrote:
> On Mon, Mar 25, 2024 at 04:36:21PM -0500, Pavan Kumar Paluri wrote:
>> Issuing a call to fdt_check_header() prevents running any of x86 UEFI
>> enabled tests. Bypass this call for x86 in order to enable UEFI
>> supported tests for KUT x86 arch.
> 
> Ouch! Sorry about that. I think I prefer something like below, though.
> 
> Thanks,
> drew
> 
> diff --git a/lib/efi.c b/lib/efi.c
> index 5314eaa81e66..335b66d26092 100644
> --- a/lib/efi.c
> +++ b/lib/efi.c
> @@ -312,6 +312,7 @@ static void* efi_get_var(efi_handle_t handle, struct efi_loaded_image_64 *image,
>         return val;
>  }
>  
> +#if defined(__aarch64__) || defined(__riscv)

I just realized, I had to move this line all the way upto efi_get_var in
order to avoid the following compilation warnings/errors (for x86 build):

lib/efi.c:299:14: error: ‘efi_get_var’ defined but not used
[-Werror=unused-function]
  299 | static void* efi_get_var(efi_handle_t handle, struct
efi_loaded_image_64 *image, efi_char16_t *var)
      |              ^~~~~~~~~~~
lib/efi.c:210:13: error: ‘efi_load_image’ defined but not used
[-Werror=unused-function]
  210 | static void efi_load_image(efi_handle_t handle, struct
efi_loaded_image_64 *image, void **data,
      |             ^~~~~~~~~~~~~~
cc1: all warnings being treated as errors

Diff after applying the change below:

diff --git a/lib/efi.c b/lib/efi.c
index 5314eaa81e66..8a74a22834a4 100644
--- a/lib/efi.c
+++ b/lib/efi.c
@@ -204,6 +204,7 @@ static char *efi_convert_cmdline(struct
efi_loaded_image_64 *image, int *cmd_lin
        return (char *)cmdline_addr;
 }

+#if defined(__aarch64__) || defined(__riscv)
 /*
  * Open the file and read it into a buffer.
  */
@@ -330,6 +331,12 @@ static void *efi_get_fdt(efi_handle_t handle,
struct efi_loaded_image_64 *image)

        return fdt_check_header(fdt) == 0 ? fdt : NULL;
 }
+#else
+static void *efi_get_fdt(efi_handle_t handle, struct
efi_loaded_image_64 *image)
+{
+       return NULL;
+}
+#endif

 static const struct {
        struct efi_vendor_dev_path      vendor;


>  static void *efi_get_fdt(efi_handle_t handle, struct efi_loaded_image_64 *image)
>  {
>         efi_char16_t var[] = ENV_VARNAME_DTBFILE;
> @@ -330,6 +331,12 @@ static void *efi_get_fdt(efi_handle_t handle, struct efi_loaded_image_64 *image)
>  
>         return fdt_check_header(fdt) == 0 ? fdt : NULL;
>  }
> +#else
> +static void *efi_get_fdt(efi_handle_t handle, struct efi_loaded_image_64 *image)
> +{
> +       return NULL;
> +}
> +#endif
>  
>  static const struct {
>         struct efi_vendor_dev_path      vendor;
> 

Thanks,
Pavan
