| Message ID | 1414995190-2267-1-git-send-email-roy.franz@linaro.org |
|---|---|
| State | New |
| Headers | show |
>>> On 03.11.14 at 07:13, <roy.franz@linaro.org> wrote: > 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. Right, but ... > 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. ... why is this? > + if ( use_cfg_file ) > { > EFI_FILE_HANDLE dir_handle; > + size = 0; Coding style (missing blank line between declaration(s) and statement(s). Plus - did you check whether some of the so far function wide variables (e.g. gop) could be moved into this more narrow scope? > } > } > } > - > efi_arch_edd(); > > /* XXX Collect EDID info. */ Please don't. Jan
On Mon, Nov 3, 2014 at 1:33 AM, Jan Beulich <JBeulich@suse.com> wrote: >>>> On 03.11.14 at 07:13, <roy.franz@linaro.org> wrote: >> 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. > > Right, but ... > >> 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. > > ... why is this? The EFI boot code did this before any of the arm64 changes, and that behavior is unchanged. The actual message is "WARNING: Unknown command line option" I was simply trying to explain the current behavior regarding the EFI commandline. > >> + if ( use_cfg_file ) >> { >> EFI_FILE_HANDLE dir_handle; >> + size = 0; > > Coding style (missing blank line between declaration(s) and > statement(s). Plus - did you check whether some of the so far > function wide variables (e.g. gop) could be moved into this more > narrow scope? I'll fix the style. I had not reviewed scope, but now have. There are only a few variables I can reduce in scope, which I have done in V2 of the patch which I will post shortly. The gop variable and a few others cannot be moved without further code reorganization. The graphics mode is set later in efi_start() if gop is !null (line 1035) I don't see any reason this code couldn't be moved to be in the if ( use_cfg_file ) block, but given that this patch is very late in the release cycle, I'm opting to try to keep this patch minimal. If you would prefer me to move this code and variables, I am happy to do a v3 with these changes. > >> } >> } >> } >> - >> efi_arch_edd(); >> >> /* XXX Collect EDID info. */ > > Please don't. removed. > > Jan >
>>> On 04.11.14 at 05:31, <roy.franz@linaro.org> wrote: > On Mon, Nov 3, 2014 at 1:33 AM, Jan Beulich <JBeulich@suse.com> wrote: >>>>> On 03.11.14 at 07:13, <roy.franz@linaro.org> wrote: >>> 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. >> >> Right, but ... >> >>> 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. >> >> ... why is this? > > The EFI boot code did this before any of the arm64 changes, and that > behavior is unchanged. > The actual message is "WARNING: Unknown command line option" > > I was simply trying to explain the current behavior regarding the EFI > commandline. Argh, I should have stripped that second sentence from the quoted context, as the question was regarding the apparently special ARM64 behavior you describe. >>> + if ( use_cfg_file ) >>> { >>> EFI_FILE_HANDLE dir_handle; >>> + size = 0; >> >> Coding style (missing blank line between declaration(s) and >> statement(s). Plus - did you check whether some of the so far >> function wide variables (e.g. gop) could be moved into this more >> narrow scope? > > I'll fix the style. I had not reviewed scope, but now have. There > are only a few > variables I can reduce in scope, which I have done in V2 of the patch > which I will post shortly. > The gop variable and a few others cannot be moved without further code > reorganization. The graphics > mode is set later in efi_start() if gop is !null (line 1035) > I don't see any reason this code couldn't be moved to be in the if ( > use_cfg_file ) block, but given that > this patch is very late in the release cycle, I'm opting to try to > keep this patch minimal. If you would prefer > me to move this code and variables, I am happy to do a v3 with these > changes. No, I'm perfectly fine with your decision not to do a larger re-org. Jan
On Mon, Nov 3, 2014 at 11:49 PM, Jan Beulich <JBeulich@suse.com> wrote: >>>> On 04.11.14 at 05:31, <roy.franz@linaro.org> wrote: >> On Mon, Nov 3, 2014 at 1:33 AM, Jan Beulich <JBeulich@suse.com> wrote: >>>>>> On 03.11.14 at 07:13, <roy.franz@linaro.org> wrote: >>>> 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. >>> >>> Right, but ... >>> >>>> 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. >>> >>> ... why is this? >> >> The EFI boot code did this before any of the arm64 changes, and that >> behavior is unchanged. >> The actual message is "WARNING: Unknown command line option" >> >> I was simply trying to explain the current behavior regarding the EFI >> commandline. > > Argh, I should have stripped that second sentence from the quoted > context, as the question was regarding the apparently special ARM64 > behavior you describe. >>>> 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. OK, I'll try address the above. This behavior is the "no config file case", which only exists on arm64 - x86 always uses a config file when booted as an EFI application, since for x86 GRUB does execute Xen as en EFI application when using multiboot2. The case of being executed as an EFI application, loaded by GRUB and provided module information via the multiboot2 (in FDT) protocol is unique to arm64. All the code is common code, but since efi_arch_use_config_file() is always true for x86 the case described only applies to arm64. When I added the support for arm64 GRUB boot and the no config file case, I limited the no config file case to not having a config file, but left console mode setting, and the "-basevideo" option that goes with that. Those changes were narrowly focused on removing the need for the config file. I think that the result of this patch, which is to have the EFI boot code only do essential boot setup that doesn't need options (and hence no EFI command line at all) is a better way to go. > >>>> + if ( use_cfg_file ) >>>> { >>>> EFI_FILE_HANDLE dir_handle; >>>> + size = 0; >>> >>> Coding style (missing blank line between declaration(s) and >>> statement(s). Plus - did you check whether some of the so far >>> function wide variables (e.g. gop) could be moved into this more >>> narrow scope? >> >> I'll fix the style. I had not reviewed scope, but now have. There >> are only a few >> variables I can reduce in scope, which I have done in V2 of the patch >> which I will post shortly. >> The gop variable and a few others cannot be moved without further code >> reorganization. The graphics >> mode is set later in efi_start() if gop is !null (line 1035) >> I don't see any reason this code couldn't be moved to be in the if ( >> use_cfg_file ) block, but given that >> this patch is very late in the release cycle, I'm opting to try to >> keep this patch minimal. If you would prefer >> me to move this code and variables, I am happy to do a v3 with these >> changes. > > No, I'm perfectly fine with your decision not to do a larger re-org. > > Jan >
On Tue, Nov 04, 2014 at 10:39:53AM -0800, Roy Franz wrote: > On Mon, Nov 3, 2014 at 11:49 PM, Jan Beulich <JBeulich@suse.com> wrote: > >>>> On 04.11.14 at 05:31, <roy.franz@linaro.org> wrote: > >> On Mon, Nov 3, 2014 at 1:33 AM, Jan Beulich <JBeulich@suse.com> wrote: > >>>>>> On 03.11.14 at 07:13, <roy.franz@linaro.org> wrote: > >>>> 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. > >>> > >>> Right, but ... > >>> > >>>> 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. > >>> > >>> ... why is this? > >> > >> The EFI boot code did this before any of the arm64 changes, and that > >> behavior is unchanged. > >> The actual message is "WARNING: Unknown command line option" > >> > >> I was simply trying to explain the current behavior regarding the EFI > >> commandline. > > > > Argh, I should have stripped that second sentence from the quoted > > context, as the question was regarding the apparently special ARM64 > > behavior you describe. > > >>>> 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. > OK, I'll try address the above. This behavior is the "no config file > case", which only > exists on arm64 - x86 always uses a config file when booted as an EFI > application, since > for x86 GRUB does execute Xen as en EFI application when using multiboot2. I have a feeling that you wrote that by mistake. As I said in my earlier emails multiboot2 on x86 EFI platform will not load EFI application as an EFI application. > The case of being executed as an EFI application, loaded by GRUB and provided > module information via the multiboot2 (in FDT) protocol is unique to > arm64. All the code > is common code, but since efi_arch_use_config_file() is always true > for x86 the case > described only applies to arm64. Right now it is true. Probably it will change after introducing multiboot2 protocol on x86. Daniel
On Tue, Nov 4, 2014 at 1:17 PM, Daniel Kiper <daniel.kiper@oracle.com> wrote: > On Tue, Nov 04, 2014 at 10:39:53AM -0800, Roy Franz wrote: >> On Mon, Nov 3, 2014 at 11:49 PM, Jan Beulich <JBeulich@suse.com> wrote: >> >>>> On 04.11.14 at 05:31, <roy.franz@linaro.org> wrote: >> >> On Mon, Nov 3, 2014 at 1:33 AM, Jan Beulich <JBeulich@suse.com> wrote: >> >>>>>> On 03.11.14 at 07:13, <roy.franz@linaro.org> wrote: >> >>>> 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. >> >>> >> >>> Right, but ... >> >>> >> >>>> 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. >> >>> >> >>> ... why is this? >> >> >> >> The EFI boot code did this before any of the arm64 changes, and that >> >> behavior is unchanged. >> >> The actual message is "WARNING: Unknown command line option" >> >> >> >> I was simply trying to explain the current behavior regarding the EFI >> >> commandline. >> > >> > Argh, I should have stripped that second sentence from the quoted >> > context, as the question was regarding the apparently special ARM64 >> > behavior you describe. >> >> >>>> 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. >> OK, I'll try address the above. This behavior is the "no config file >> case", which only >> exists on arm64 - x86 always uses a config file when booted as an EFI >> application, since >> for x86 GRUB does execute Xen as en EFI application when using multiboot2. > > I have a feeling that you wrote that by mistake. As I said in my earlier emails > multiboot2 on x86 EFI platform will not load EFI application as an EFI application. Grr.. meant "does not execute" of course. I do get that now :) > >> The case of being executed as an EFI application, loaded by GRUB and provided >> module information via the multiboot2 (in FDT) protocol is unique to >> arm64. All the code >> is common code, but since efi_arch_use_config_file() is always true >> for x86 the case >> described only applies to arm64. > > Right now it is true. Probably it will change after introducing > multiboot2 protocol on x86. So you expect to be using the efi_start() function mostly as is? > > Daniel
On Tue, Nov 04, 2014 at 02:36:06PM -0800, Roy Franz wrote: > On Tue, Nov 4, 2014 at 1:17 PM, Daniel Kiper <daniel.kiper@oracle.com> wrote: > > On Tue, Nov 04, 2014 at 10:39:53AM -0800, Roy Franz wrote: [...] > >> The case of being executed as an EFI application, loaded by GRUB and provided > >> module information via the multiboot2 (in FDT) protocol is unique to > >> arm64. All the code > >> is common code, but since efi_arch_use_config_file() is always true > >> for x86 the case > >> described only applies to arm64. > > > > Right now it is true. Probably it will change after introducing > > multiboot2 protocol on x86. > > So you expect to be using the efi_start() function mostly as is? More or less. I am working on it right now. Daniel
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. The configuration file consists of one or more sections headed by a section name enclosed in square brackets, with individual values specified in each diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c index 4257341..26e3cc8 100644 --- a/xen/common/efi/boot.c +++ b/xen/common/efi/boot.c @@ -705,6 +705,7 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable) union string section = { NULL }, name; bool_t base_video = 0; char *option_str; + bool_t use_cfg_file; efi_ih = ImageHandle; efi_bs = SystemTable->BootServices; @@ -717,6 +718,7 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable) StdOut = SystemTable->ConOut; StdErr = SystemTable->StdErr ?: StdOut; + use_cfg_file = efi_arch_use_config_file(SystemTable); status = efi_bs->HandleProtocol(ImageHandle, &loaded_image_guid, (void **)&loaded_image); @@ -725,67 +727,70 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable) efi_arch_load_addr_check(loaded_image); - argc = get_argv(0, NULL, loaded_image->LoadOptions, - loaded_image->LoadOptionsSize, NULL); - if ( argc > 0 && - efi_bs->AllocatePool(EfiLoaderData, - (argc + 1) * sizeof(*argv) + - loaded_image->LoadOptionsSize, - (void **)&argv) == EFI_SUCCESS ) - get_argv(argc, argv, loaded_image->LoadOptions, - loaded_image->LoadOptionsSize, &options); - else - argc = 0; - for ( i = 1; i < argc; ++i ) + if ( use_cfg_file ) { - CHAR16 *ptr = argv[i]; - - if ( !ptr ) - break; - if ( *ptr == L'/' || *ptr == L'-' ) + argc = get_argv(0, NULL, loaded_image->LoadOptions, + loaded_image->LoadOptionsSize, NULL); + if ( argc > 0 && + efi_bs->AllocatePool(EfiLoaderData, + (argc + 1) * sizeof(*argv) + + loaded_image->LoadOptionsSize, + (void **)&argv) == EFI_SUCCESS ) + get_argv(argc, argv, loaded_image->LoadOptions, + loaded_image->LoadOptionsSize, &options); + else + argc = 0; + for ( i = 1; i < argc; ++i ) { - if ( wstrcmp(ptr + 1, L"basevideo") == 0 ) - base_video = 1; - else if ( wstrncmp(ptr + 1, L"cfg=", 4) == 0 ) - cfg_file_name = ptr + 5; - else if ( i + 1 < argc && wstrcmp(ptr + 1, L"cfg") == 0 ) - cfg_file_name = argv[++i]; - else if ( wstrcmp(ptr + 1, L"help") == 0 || - (ptr[1] == L'?' && !ptr[2]) ) + CHAR16 *ptr = argv[i]; + + if ( !ptr ) + break; + if ( *ptr == L'/' || *ptr == L'-' ) { - PrintStr(L"Xen EFI Loader options:\r\n"); - PrintStr(L"-basevideo retain current video mode\r\n"); - PrintStr(L"-cfg=<file> specify configuration file\r\n"); - PrintStr(L"-help, -? display this help\r\n"); - blexit(NULL); + if ( wstrcmp(ptr + 1, L"basevideo") == 0 ) + base_video = 1; + else if ( wstrncmp(ptr + 1, L"cfg=", 4) == 0 ) + cfg_file_name = ptr + 5; + else if ( i + 1 < argc && wstrcmp(ptr + 1, L"cfg") == 0 ) + cfg_file_name = argv[++i]; + else if ( wstrcmp(ptr + 1, L"help") == 0 || + (ptr[1] == L'?' && !ptr[2]) ) + { + PrintStr(L"Xen EFI Loader options:\r\n"); + PrintStr(L"-basevideo retain current video mode\r\n"); + PrintStr(L"-cfg=<file> specify configuration file\r\n"); + PrintStr(L"-help, -? display this help\r\n"); + blexit(NULL); + } + else + { + PrintStr(L"WARNING: Unknown command line option '"); + PrintStr(ptr); + PrintStr(L"' ignored\r\n"); + } } else - { - PrintStr(L"WARNING: Unknown command line option '"); - PrintStr(ptr); - PrintStr(L"' ignored\r\n"); - } + section.w = ptr; } - else - section.w = ptr; - } - if ( !base_video ) - { - unsigned int best; - - for ( i = 0, size = 0, best = StdOut->Mode->Mode; - i < StdOut->Mode->MaxMode; ++i ) + if ( !base_video ) { - if ( StdOut->QueryMode(StdOut, i, &cols, &rows) == EFI_SUCCESS && - cols * rows > size ) + unsigned int best; + + for ( i = 0, size = 0, best = StdOut->Mode->Mode; + i < StdOut->Mode->MaxMode; ++i ) { - size = cols * rows; - best = i; + if ( StdOut->QueryMode(StdOut, i, &cols, &rows) == EFI_SUCCESS && + cols * rows > size ) + { + size = cols * rows; + best = i; + } } + if ( best != StdOut->Mode->Mode ) + StdOut->SetMode(StdOut, best); } - if ( best != StdOut->Mode->Mode ) - StdOut->SetMode(StdOut, best); } PrintStr(L"Xen " __stringify(XEN_VERSION) "." __stringify(XEN_SUBVERSION) @@ -793,37 +798,37 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable) efi_arch_relocate_image(0); - if ( StdOut->QueryMode(StdOut, StdOut->Mode->Mode, - &cols, &rows) == EFI_SUCCESS ) - efi_arch_console_init(cols, rows); - - size = 0; - status = efi_bs->LocateHandle(ByProtocol, &gop_guid, NULL, &size, NULL); - if ( status == EFI_BUFFER_TOO_SMALL ) - status = efi_bs->AllocatePool(EfiLoaderData, size, (void **)&handles); - if ( !EFI_ERROR(status) ) - status = efi_bs->LocateHandle(ByProtocol, &gop_guid, NULL, &size, - handles); - if ( EFI_ERROR(status) ) - size = 0; - for ( i = 0; i < size / sizeof(*handles); ++i ) - { - status = efi_bs->HandleProtocol(handles[i], &gop_guid, (void **)&gop); - if ( EFI_ERROR(status) ) - continue; - status = gop->QueryMode(gop, gop->Mode->Mode, &info_size, &mode_info); - if ( !EFI_ERROR(status) ) - break; - } - if ( handles ) - efi_bs->FreePool(handles); - if ( EFI_ERROR(status) ) - gop = NULL; - - cols = rows = depth = 0; - if ( efi_arch_use_config_file(SystemTable) ) + if ( use_cfg_file ) { EFI_FILE_HANDLE dir_handle; + size = 0; + cols = rows = depth = 0; + + if ( StdOut->QueryMode(StdOut, StdOut->Mode->Mode, + &cols, &rows) == EFI_SUCCESS ) + efi_arch_console_init(cols, rows); + + status = efi_bs->LocateHandle(ByProtocol, &gop_guid, NULL, &size, NULL); + if ( status == EFI_BUFFER_TOO_SMALL ) + status = efi_bs->AllocatePool(EfiLoaderData, size, (void **)&handles); + if ( !EFI_ERROR(status) ) + status = efi_bs->LocateHandle(ByProtocol, &gop_guid, NULL, &size, + handles); + if ( EFI_ERROR(status) ) + size = 0; + for ( i = 0; i < size / sizeof(*handles); ++i ) + { + status = efi_bs->HandleProtocol(handles[i], &gop_guid, (void **)&gop); + if ( EFI_ERROR(status) ) + continue; + status = gop->QueryMode(gop, gop->Mode->Mode, &info_size, &mode_info); + if ( !EFI_ERROR(status) ) + break; + } + if ( handles ) + efi_bs->FreePool(handles); + if ( EFI_ERROR(status) ) + gop = NULL; /* Get the file system interface. */ dir_handle = get_parent_handle(loaded_image, &file_name); @@ -931,49 +936,47 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable) dir_handle->Close(dir_handle); - } - - if ( gop && !base_video ) - { - for ( i = size = 0; i < gop->Mode->MaxMode; ++i ) + if ( gop && !base_video ) { - unsigned int bpp = 0; - - status = gop->QueryMode(gop, i, &info_size, &mode_info); - if ( EFI_ERROR(status) ) - continue; - switch ( mode_info->PixelFormat ) + for ( i = size = 0; i < gop->Mode->MaxMode; ++i ) { - case PixelBitMask: - bpp = hweight32(mode_info->PixelInformation.RedMask | - mode_info->PixelInformation.GreenMask | - mode_info->PixelInformation.BlueMask); - break; - case PixelRedGreenBlueReserved8BitPerColor: - case PixelBlueGreenRedReserved8BitPerColor: - bpp = 24; - break; - default: - continue; - } - if ( cols == mode_info->HorizontalResolution && - rows == mode_info->VerticalResolution && - (!depth || bpp == depth) ) - { - gop_mode = i; - break; - } - if ( !cols && !rows && - mode_info->HorizontalResolution * - mode_info->VerticalResolution > size ) - { - size = mode_info->HorizontalResolution * - mode_info->VerticalResolution; - gop_mode = i; + unsigned int bpp = 0; + + status = gop->QueryMode(gop, i, &info_size, &mode_info); + if ( EFI_ERROR(status) ) + continue; + switch ( mode_info->PixelFormat ) + { + case PixelBitMask: + bpp = hweight32(mode_info->PixelInformation.RedMask | + mode_info->PixelInformation.GreenMask | + mode_info->PixelInformation.BlueMask); + break; + case PixelRedGreenBlueReserved8BitPerColor: + case PixelBlueGreenRedReserved8BitPerColor: + bpp = 24; + break; + default: + continue; + } + if ( cols == mode_info->HorizontalResolution && + rows == mode_info->VerticalResolution && + (!depth || bpp == depth) ) + { + gop_mode = i; + break; + } + if ( !cols && !rows && + mode_info->HorizontalResolution * + mode_info->VerticalResolution > size ) + { + size = mode_info->HorizontalResolution * + mode_info->VerticalResolution; + gop_mode = i; + } } } } - efi_arch_edd(); /* XXX Collect EDID info. */
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@linaro.org> --- 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. docs/misc/efi.markdown | 5 ++ xen/common/efi/boot.c | 237 +++++++++++++++++++++++++------------------------ 2 files changed, 125 insertions(+), 117 deletions(-)