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

[Daniel Kiper <daniel.kiper@xxxxxxxxxx>]

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

I hope that helps.

Daniel

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