Re: [Xen-devel] [PATCH for-4.5 V2] EFI: Ignore EFI commandline, skip console setup when booted from GRUB

[Roy Franz <roy.franz@xxxxxxxxxx>]

On Tue, Nov 4, 2014 at 3:57 AM, Daniel Kiper <daniel.kiper@xxxxxxxxxx> wrote:
> On Mon, Nov 03, 2014 at 08:37:31PM -0800, Roy Franz wrote:
>> Update EFI code to completely ignore the EFI comnandline when booted from 
>> GRUB.
>> Previusly it was parsed of EFI boot specific options, but these aren't used
>> when booted from GRUB.
>>
>> Don't do EFI console or video configuration when booted by GRUB.  The EFI 
>> boot
>> code does some console and video initialization to support native EFI boot 
>> from
>> the EFI boot manager or EFI shell.  This initlization should not be done when
>> booted using GRUB.
>>
>> Update EFI documentation to indicate that it describes EFI native boot, and
>> does not apply at all when Xen is booted using GRUB.
>>
>> Signed-off-by: Roy Franz <roy.franz@xxxxxxxxxx>
>
> In general Reviewed-by: Daniel Kiper <daniel.kiper@xxxxxxxxxx> but...
>
>> ---
>> This patch implements what I understand to be the desired behavior when 
>> booting
>> an EFI Xen image via GRUB based on the thread last week.  The EFI command 
>> line
>> is not used, and the Xen commandline comes via the multiboot protocol (and
>> in the ARM case the multiboot FDT bindings).  This brings the x86 and arm64
>> GRUB EFI boot cases into alignment in not using the EFI commandline.
>>
>> The current state of the arm64 code takes the Xen commandline from the FDT,
>> but still looks in the EFI commandline for EFI boot specific options.  If
>> unexpected options are passed in the EFI commandline, it will generate
>> "unrecognized option" ouput for all unexpected options.
>>
>> I think this should be considered for 4.5.
>>
>> Changes from v1:
>> * Fixed style, and removed blank line changes
>> * Reviewed scope of variables now that more code is in if ( use_cfg_file ) 
>> block
>>
>>
>>  docs/misc/efi.markdown |   5 ++
>>  xen/common/efi/boot.c  | 240 
>> +++++++++++++++++++++++++------------------------
>>  2 files changed, 128 insertions(+), 117 deletions(-)
>>
>> diff --git a/docs/misc/efi.markdown b/docs/misc/efi.markdown
>> index 5e48aa3..f435ec7 100644
>> --- a/docs/misc/efi.markdown
>> +++ b/docs/misc/efi.markdown
>> @@ -29,6 +29,11 @@ separators will be tried) to be present in the same 
>> directory as the binary.
>>  (To illustrate the name handling, a binary named `xen-4.2-unstable.efi` 
>> would
>>  try `xen-4.2-unstable.cfg`, `xen-4.2.cfg`, `xen-4.cfg`, and `xen.cfg` in
>>  order.) One can override this with a command line option 
>> (`-cfg=<filename>`).
>> +This configuration file and EFI commandline are only used for booting 
>> directly
>> +from EFI firmware, or when using an EFI loader that does not support
>> +the multiboot2 protocol.  When booting using GRUB or another multiboot aware
>> +loader the EFI commandline is ignored and all information is passed from
>> +the loader to Xen using the multiboot protocol.
>
> First you are referring to multiboot2 and later you are talking about
> multiboot. It makes some confusion. Could you put full stop after "...
> using an EFI loader" or rephrase this sentences in another way to make
> them clear?
>
> Daniel
I think all references should be multiboot2.  I do really want
something that is clear
to most readers.

How about:

"This configuration file and EFI commandline are only used for booting directly
from EFI firmware, or when using an EFI loader. When booting using GRUB or
another multiboot2 aware loader the EFI commandline is ignored and all
information
is passed from the loader to Xen using the multiboot protocol."

Would it be helpful to list an example EFI loader, such as gummiboot?

"... or when using an EFI loader (such as gummiboot.)  When booting using GRUB"

gummiboot and other EFI loaders just execute another EFI application,
so as far as Xen
is concerned this would be like booting from the EFI shell.

Thanks,
Roy

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