From: Andrew Jones <andrew.jones@linux.dev>
To: Nikos Nikoleris <nikos.nikoleris@arm.com>
Cc: "Paluri, PavanKumar" <papaluri@amd.com>,
kvm@vger.kernel.org, kvmarm@lists.linux.dev,
alexandru.elisei@arm.com, eric.auger@redhat.com,
shahuang@redhat.com, pbonzini@redhat.com, thuth@redhat.com
Subject: Re: [kvm-unit-tests PATCH v3 08/18] arm64: efi: Improve device tree discovery
Date: Tue, 26 Mar 2024 10:03:58 +0100 [thread overview]
Message-ID: <20240326-f023a9bdf7ccaeb8844c5197@orel> (raw)
In-Reply-To: <b7ca796b-8883-4048-b441-fd5c5bdc4d52@arm.com>
On Mon, Mar 25, 2024 at 09:59:00PM +0000, Nikos Nikoleris wrote:
> On 25/03/2024 16:24, Paluri, PavanKumar wrote:
> > Hi,
> >
> > On 3/5/2024 10:46 AM, Andrew Jones wrote:
> > > Check the device tree GUID when the environment variable is missing,
> > > which allows directly loading the unit test with QEMU's '-kernel'
> > > command line parameter, which is much faster than putting the test
> > > in the EFI file system and then running it from the UEFI shell.
> > >
> > > Reviewed-by: Nikos Nikoleris <nikos.nikoleris@arm.com>
> > > Signed-off-by: Andrew Jones <andrew.jones@linux.dev>
> > > ---
> > > lib/efi.c | 19 ++++++++++++-------
> > > lib/linux/efi.h | 2 ++
> > > 2 files changed, 14 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/lib/efi.c b/lib/efi.c
> > > index d94f0fa16fc0..4d1126b4a64e 100644
> > > --- a/lib/efi.c
> > > +++ b/lib/efi.c
> > > @@ -6,13 +6,13 @@
> > > *
> > > * SPDX-License-Identifier: LGPL-2.0-or-later
> > > */
> > > -
> > > -#include "efi.h"
> > > +#include <libcflat.h>
> > > #include <argv.h>
> > > -#include <stdlib.h>
> > > #include <ctype.h>
> > > -#include <libcflat.h>
> > > +#include <stdlib.h>
> > > #include <asm/setup.h>
> > > +#include "efi.h"
> > > +#include "libfdt/libfdt.h"
> > > /* From lib/argv.c */
> > > extern int __argc, __envc;
> > > @@ -288,13 +288,18 @@ static void *efi_get_fdt(efi_handle_t handle, struct efi_loaded_image_64 *image)
> > > efi_char16_t var[] = ENV_VARNAME_DTBFILE;
> > > efi_char16_t *val;
> > > void *fdt = NULL;
> > > - int fdtsize;
> > > + int fdtsize = 0;
> > > val = efi_get_var(handle, image, var);
> > > - if (val)
> > > + if (val) {
> > > efi_load_image(handle, image, &fdt, &fdtsize, val);
> > > + if (fdtsize == 0)
> > > + return NULL;
> > > + } else if (efi_get_system_config_table(DEVICE_TREE_GUID, &fdt) != EFI_SUCCESS) {
> > > + return NULL;
> > > + }
> > > - return fdt;
> > > + return fdt_check_header(fdt) == 0 ? fdt : NULL;
> >
> > The call to fdt_check_header() seems to be breaking x86 based UEFI
> > tests. I have tested it with .x86/efi/run ./x86/smptest.efi
>
> I am not familiar with the x86 boot process but I would have thought that
> the efi shell variable "fdtfile" is not set and as a result val would be
> NULL. Then efi_get_system_config_table(DEVICE_TREE_GUID, &fdt) would return
> EFI_NOT_FOUND and efi_get_fdt would return NULL without executing the line
> fdt_check_header(fdt).
I suppose there could be a table (maybe empty?) with the DEVICE_TREE_GUID
guid? Anyway, we should probably just #ifdef out the function since x86
kvm-unit-tests doesn't link with libfdt and the only reason it can compile
with an undefined reference to fdt_check_header() is because we create
.efi files through shared libraries.
Thanks,
drew
>
> Thanks,
>
> Nikos
>
> >
> > Thanks,
> > Pavan
> > > }
> > > efi_status_t efi_main(efi_handle_t handle, efi_system_table_t *sys_tab)
> > > diff --git a/lib/linux/efi.h b/lib/linux/efi.h
> > > index 410f0b1a0da1..92d798f79767 100644
> > > --- a/lib/linux/efi.h
> > > +++ b/lib/linux/efi.h
> > > @@ -66,6 +66,8 @@ typedef guid_t efi_guid_t;
> > > #define ACPI_TABLE_GUID EFI_GUID(0xeb9d2d30, 0x2d88, 0x11d3, 0x9a, 0x16, 0x00, 0x90, 0x27, 0x3f, 0xc1, 0x4d)
> > > #define ACPI_20_TABLE_GUID EFI_GUID(0x8868e871, 0xe4f1, 0x11d3, 0xbc, 0x22, 0x00, 0x80, 0xc7, 0x3c, 0x88, 0x81)
> > > +#define DEVICE_TREE_GUID EFI_GUID(0xb1b621d5, 0xf19c, 0x41a5, 0x83, 0x0b, 0xd9, 0x15, 0x2c, 0x69, 0xaa, 0xe0)
> > > +
> > > #define LOADED_IMAGE_PROTOCOL_GUID EFI_GUID(0x5b1b31a1, 0x9562, 0x11d2, 0x8e, 0x3f, 0x00, 0xa0, 0xc9, 0x69, 0x72, 0x3b)
> > > typedef struct {
next prev parent reply other threads:[~2024-03-26 9:04 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-05 16:46 [kvm-unit-tests PATCH v3 00/18] arm64: EFI improvements Andrew Jones
2024-03-05 16:46 ` [kvm-unit-tests PATCH v3 01/18] runtime: Update MAX_SMP probe Andrew Jones
2024-03-07 2:29 ` Shaoqin Huang
2024-03-05 16:46 ` [kvm-unit-tests PATCH v3 02/18] runtime: Add yet another 'no kernel' error message Andrew Jones
2024-03-07 2:39 ` Shaoqin Huang
2024-03-05 16:46 ` [kvm-unit-tests PATCH v3 03/18] arm64: efi: Don't create dummy test Andrew Jones
2024-03-07 3:37 ` Shaoqin Huang
2024-03-05 16:46 ` [kvm-unit-tests PATCH v3 04/18] arm64: efi: Make running tests on EFI can be parallel Andrew Jones
2024-03-05 16:46 ` [kvm-unit-tests PATCH v3 05/18] arm64: efi: Remove redundant dtb generation Andrew Jones
2024-03-05 16:46 ` [kvm-unit-tests PATCH v3 06/18] arm64: efi: Move run code into a function Andrew Jones
2024-03-05 16:46 ` [kvm-unit-tests PATCH v3 07/18] arm64: efi: Remove EFI_USE_DTB Andrew Jones
2024-03-05 16:46 ` [kvm-unit-tests PATCH v3 08/18] arm64: efi: Improve device tree discovery Andrew Jones
2024-03-25 16:24 ` Paluri, PavanKumar
2024-03-25 21:59 ` Nikos Nikoleris
2024-03-26 9:03 ` Andrew Jones [this message]
2024-03-05 16:46 ` [kvm-unit-tests PATCH v3 09/18] lib/efi: Add support for loading the initrd Andrew Jones
2024-03-05 16:46 ` [kvm-unit-tests PATCH v3 10/18] arm64: efi: Allow running tests directly Andrew Jones
2024-03-05 16:46 ` [kvm-unit-tests PATCH v3 11/18] arm/arm64: Factor out some initial setup Andrew Jones
2024-03-05 16:46 ` [kvm-unit-tests PATCH v3 12/18] arm/arm64: Factor out allocator init from mem_init Andrew Jones
2024-03-05 16:46 ` [kvm-unit-tests PATCH v3 13/18] arm64: Simplify efi_mem_init Andrew Jones
2024-03-05 16:46 ` [kvm-unit-tests PATCH v3 14/18] arm64: Add memregions_efi_init Andrew Jones
2024-03-05 16:46 ` [kvm-unit-tests PATCH v3 15/18] arm64: efi: Don't map reserved regions Andrew Jones
2024-03-05 16:46 ` [kvm-unit-tests PATCH v3 16/18] arm64: efi: Fix _start returns from failed _relocate Andrew Jones
2024-03-05 16:46 ` [kvm-unit-tests PATCH v3 17/18] arm64: efi: Switch to our own stack Andrew Jones
2024-03-05 16:46 ` [kvm-unit-tests PATCH v3 18/18] arm64: efi: Add gitlab CI Andrew Jones
2024-03-18 16:47 ` [kvm-unit-tests PATCH v3 00/18] arm64: EFI improvements Andrew Jones
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240326-f023a9bdf7ccaeb8844c5197@orel \
--to=andrew.jones@linux.dev \
--cc=alexandru.elisei@arm.com \
--cc=eric.auger@redhat.com \
--cc=kvm@vger.kernel.org \
--cc=kvmarm@lists.linux.dev \
--cc=nikos.nikoleris@arm.com \
--cc=papaluri@amd.com \
--cc=pbonzini@redhat.com \
--cc=shahuang@redhat.com \
--cc=thuth@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).