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

[Julien Grall <julien@xxxxxxx>]

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