On 3/27/2024 3:08 AM, Andrew Jones wrote:
> On Tue, Mar 26, 2024 at 12:33:57PM -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 and also calls to
>> efi_load_image(), efi_grow_buffer(), efi_get_var() in order to enable
>> UEFI supported tests for KUT x86 arch.
>>
>> Fixes: 9632ce446b8f ("arm64: efi: Improve device tree discovery")
>> Signed-off-by: Pavan Kumar Paluri <papaluri@xxxxxxx>
>> ---
>> lib/efi.c | 7 +++++++
>> 1 file changed, 7 insertions(+)
>>
>> 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;
>> --
>> 2.34.1
>>
>
> It's a pity that the suggestion I made (which I obviously hadn't even
> compile tested...) isn't sufficient, because what I was going for was
> a way to annotate that specifically efi_get_fdt() was for architectures
> which use a DT (and link libfdt). It's not as nice to indicate that
> the other functions also don't apply to x86 (x86 doesn't use them
> now, but there's no reason it couldn't). With that in mind, I think I
> like your original patch better, but with a tweak to ensure we don't
> generate the undefined reference to fdt_check_header(),
>
> diff --git a/lib/efi.c b/lib/efi.c
> index 5314eaa81e66..dfbadea60411 100644
> --- a/lib/efi.c
> +++ b/lib/efi.c
> @@ -328,7 +328,12 @@ static void *efi_get_fdt(efi_handle_t handle, struct efi_loaded_image_64 *image)
> return NULL;
> }
>
> +#ifdef __x86_64__
> + /* x86 is ACPI-only */
> + return NULL;
> +#else
> return fdt_check_header(fdt) == 0 ? fdt : NULL;
> +#endif
> }
>
> static const struct {
>
> That said, I could go either way, since it also makes sense to not
> compile code for x86 that it doesn't currently use. So, unless you
> need to respin for other reasons,
>
Tested the above diff as well. Both work for x86. So maybe it is better
to not compile code that x86 currently does not use for now and later
make this change when x86 makes use of efi_load_image(), etc.. ? I am
fine with either ways.
> Reviewed-by: Andrew Jones <andrew.jones@xxxxxxxxx>
>
Thanks,
Pavan
> Thanks,
> drew
