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

[Roy Franz <roy.franz@xxxxxxxxxx>]

On Thu, Dec 4, 2014 at 1:35 AM, Jan Beulich <JBeulich@xxxxxxxx> 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.
>
>> 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.

I don't mind a change here, and I agree that it is more like a .c file
than a .h.  If a name change is done, is it worth dropping the "efi-" at
the same time?


>
> Jan
>

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