[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH-4.16 v2] xen/efi: Fix Grub2 boot on arm64



Hi Stefano,

On 09/11/2021 21:52, Stefano Stabellini wrote:
On Tue, 9 Nov 2021, Jan Beulich wrote:
On 09.11.2021 03:11, Stefano Stabellini wrote:
On Mon, 8 Nov 2021, Jan Beulich wrote:
On 05.11.2021 16:33, Stefano Stabellini wrote:
My main concern is performance and resource utilization. With v3 of the
patch get_parent_handle will get called for every module to be loaded.
With dom0less, it could easily get called 10 times or more. Taking a
look at get_parent_handle, the Xen side doesn't seem small and I have
no idea how the EDK2 side looks. I am just worried that it would
actually have an impact on boot times (also depending on the bootloader
implementation).

The biggest part of the function deals with determining the "residual" of
the file name. That part looks to be of no interest at all to
allocate_module_file() (whether that's actually correct I can't tell). I
don't see why this couldn't be made conditional (e.g. by passing in NULL
for "leaf").

I understand the idea of passing NULL instead of "leaf", but I tried
having a look and I can't tell what we would be able to skip in
get_parent_handle.

My bad - I did overlook that dir_handle gets updated even past the
initial loop.

Should we have a global variable to keep the dir_handle open during
dom0less module loading?

If that's contained within Arm-specific code, I (obviously) don't mind.
Otherwise I remain to be convinced.

I think we can do something decent entirely within
xen/arch/arm/efi/efi-boot.h.

Luca, see below as reference; it is untested and incomplete but should
explain the idea better than words. With the below, we only open/close
the handle once for the all dom0less modules.

Looking at the diff below, you open/close dir_handle within efi_check_dt_boot(). This means that only the functions called within the function will effectively used.

At which point, I think this should be an argument of the functions rather than a global function. This will drastically reduce any misuse of the global variable (which BTW the name seems to clash with some of the arguments).

Cheers,

--
Julien Grall



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.