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

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



>>> 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.

> 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.

> 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.

Jan


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


 


Rackspace

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