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

[Daniel Kiper <daniel.kiper@xxxxxxxxxx>]

On Tue, Nov 04, 2014 at 10:46:15AM -0800, Roy Franz wrote:
> 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."

Much better but right now it indirectly suggest that multiboot2 protocol is 
available
on all platforms including x86 which is not true (at least now). Additionally, 
I am
a bit confused about multiboot protocol naming on ARM64. It looks that
http://wiki.xen.org/wiki/Xen_ARM_with_Virtualization_Extensions/Multiboot
calls it multiboot. You say multiboot2. Which one is correct?
Could you enlighten me?

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

Yes, please. Hmmm... Should not we use "simple EFI application loader"
instead of "EFI loader"?

Daniel

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