Re: [Xen-devel] [PATCH v15 05/10] x86: add multiboot2 protocol support for EFI platforms

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 16.02.17 at 22:49, <daniel.kiper@xxxxxxxxxx> wrote:
> On Thu, Feb 16, 2017 at 02:29:45AM -0700, Jan Beulich wrote:
>> >>> On 15.02.17 at 22:53, <daniel.kiper@xxxxxxxxxx> wrote:
>> > On Wed, Feb 15, 2017 at 03:22:02AM -0700, Jan Beulich wrote:
>> >> >>> On 14.02.17 at 19:38, <daniel.kiper@xxxxxxxxxx> wrote:
>> >> > --- a/xen/arch/x86/boot/head.S
>> >> > +++ b/xen/arch/x86/boot/head.S
>> >> > @@ -394,10 +394,18 @@ __start:
>> >> >
>> >> >          /* EFI IA-32 platforms are not supported. */
>> >> >          cmpl    $MULTIBOOT2_TAG_TYPE_EFI32,MB2_tag_type(%ecx)
>> >> > +        /*
>> >> > +         * Here we should zap vga_text_buffer. However, we can disable
>> >> > +         * VGA updates in simpler and more reliable way later.
>> >> > +         */
>> >> >          je      .Lmb2_efi_ia_32
>> >> >
>> >> >          /* Bootloader shutdown EFI x64 boot services. */
>> >> >          cmpl    $MULTIBOOT2_TAG_TYPE_EFI64,MB2_tag_type(%ecx)
>> >> > +        /*
>> >> > +         * Here we should zap vga_text_buffer. However, we can disable
>> >> > +         * VGA updates in simpler and more reliable way later.
>> >> > +         */
>> >> >          je      .Lmb2_no_bs
>> >>
>> >> I'm afraid I don't view these comments as helpful in understanding
>> >> the whole situation. That's partly because I don't follow both the
>> >> "simpler" and "more reliable" parts (using just the information here,
>> >
>> > OK, I will clarify it.
>> >
>> >> i.e. leaving aside what you've given as explanation earlier, albeit I
>> >> don't think that was fully clarifying things either), and partly
>> >> because I continue to think that the explanation should go where
>> >> the labels are (which is what I had meant to suggest with my
>> >> comment placement in reply to v14). Nor does the adjustment
>> >
>> > OK.
>> >
>> >> above help (me) understand the correctness of the dual use of
>> >> .Lmb2_no_bs.
>> >
>> > What do you mean by "dual use of .Lmb2_no_bs."? I would like to be sure.
>>
>> As said in v14 review, it's being jumped to from two rather different
>> places, and hence the VGA aspect isn't obviously the same for both.
> 
> OK, I will try to clarify. If a bootloader called us using __efi64_mb2_start
> we are sure that we are running on EFI platform and there is no VGA there.
> It means that we can safely zap vga_text_buffer unconditionally in first steps
> (we do that in second instruction). Then we do not need to take care about
> that in case of error. And one of these errors is lack of 
> MULTIBOOT2_TAG_TYPE_EFI_BS
> tag. It means that EFI boot services are shutdown. So, we are in black hole.

Well, see - this is one of my problems with the overall approach here.
Running on EFI in no way means there _is_ no VGA there, it only
means there _may not be_ any VGA. With boot services shut down
and without serial console you have no way of informing the user, so
making an attempt at writing something to VGA may still be helpful.
In the worst case the memory writes go no-where, to RAM, or to an
unrelated device (both of the latter rather unlikely on x86). Granted
there's the second problem of you perhaps not knowing the video
mode, and hence having a hard time producing output that's also
readable.

> We have to inform user about that and halt the system. And that is why we
> jump to .Lmb2_no_bs here.
> 
> On the other hand if the bootloader called us using start label then in most
> cases we are running on legacy BIOS platforms. However, if the bootloader also
> provided MULTIBOOT2_TAG_TYPE_EFI64 tag here then we are sure that we are 
> running
> on EFI platform and EFI boot services are shutdown. This happens when we are
> loaded by old boot loader which does not understand 
> MULTIBOOT2_HEADER_TAG_EFI_BS
> and MULTIBOOT2_HEADER_TAG_ENTRY_ADDRESS_EFI64 tags. So, as above we can jump
> to .Lmb2_no_bs here too.

However, when the boot loader invoked our start label, can't we be
sure there is VGA (at least as much or as little as there would be on
non-EFI platforms, where headless systems certainly also exist)? I
don't think the boot loader can reasonably invoke our legacy entry
point with the system in a state that's not legacy compatible (which,
among other things, means if there is VGA, then it would be in
traditional 80x25 text mode). Hence this second use of the label
ought to avoid suppressing the VGA output attempt in any case.

> I hope that helps.

The above aside, yes, it does. An abbreviated variant of this is
what I would hope to have attached as comment to the error
handling labels, to namely help readers understand why some of
them inhibit VGA output while others don't.

Jan

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