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

[Roy Franz <roy.franz@xxxxxxxxxx>]

On Mon, Nov 3, 2014 at 1:33 AM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
>>>> On 03.11.14 at 07:13, <roy.franz@xxxxxxxxxx> 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
>

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