Re: [Xen-devel] A few EFI code questions

[Daniel Kiper <daniel.kiper@xxxxxxxxxx>]

On Thu, Dec 04, 2014 at 09:35:01AM +0000, Jan Beulich wrote:
> >>> On 03.12.14 at 22:02, <daniel.kiper@xxxxxxxxxx> wrote:
> > Hey,
> >
> > 1) Why is there in EFI code so many functions (e.g. efi_start(),
> >    efi_arch_edd(), ...) with local variables declared as a static?
> >    Though some of them have also regular local variables. I do not
> >    why it was decided that some of them must be the static and
> >    some of do not. It is a bit confusing. As I can see there is
> >    only one place which have to have local static (place_string()).
> >    Other seems to me as thing to save space on the stack but I do
> >    not think we need that. According to UEFI spec there will be
> >    "128 KiB or more of available stack space" when system runs in
> >    boot services mode. It is a lot of space. So, I think we can
> >    safely convert most of local static variables to normal local
> >    variables. Am I right?
>
> No. Consider what code results when you e.g. make an EFI_GUID
> instance non-static.

It could be quite big...

> > 2) I am going to add EDID support to EFI code. Should it be x86
> >    specific code or common one? As I can see EDID is defined as
> >    part of GOP so I think that EDID code should be placed in
> >    xen/common/efi/boot.c.
>
> Yes.

Granted!

> > 3) Should not we change xen/arch/*/efi/efi-boot.h to
> >    xen/arch/*/efi/efi-boot.c? efi-boot.h contains more
> >    code than definitions, declarations and short static
> >    functions. So, I think that it is more regular *.c file
> >    than header file.
>
> That's a matter of taste - I'd probably have made it .c too, but
> didn't mind it being .h as done by Roy (presumably on the basis
> that #include directives are preferred to have .h files as their
> operands). The only thing I regret is that I didn't ask for the
> pointless efi- prefix to be dropped.

As I can see a few people people agree to some extent with my suggestion.
Great! Sadly if we wish .c file than simple boot.c (as Jan suggested we can
drop efi- prefix) conflicts with exiting boot.c link. Is efi-boot.c OK?
Or maybe boot-arch.c? boot.h is OK for sure. Which one do you prefer?
Do you have better ideas?

Daniel

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel