Re: [Xen-devel] [PATCH v3 13/16 - RFC] x86: add multiboot2 protocol support for EFI platforms

[Daniel Kiper <daniel.kiper@xxxxxxxxxx>]

On Thu, Jun 02, 2016 at 02:34:28AM -0600, Jan Beulich wrote:
> >>> On 01.06.16 at 21:03, <daniel.kiper@xxxxxxxxxx> wrote:
> > On Fri, May 27, 2016 at 03:02:25AM -0600, Jan Beulich wrote:
> >> >>> On 25.05.16 at 23:02, <daniel.kiper@xxxxxxxxxx> wrote:
> >> > On Wed, May 25, 2016 at 03:32:37AM -0600, Jan Beulich wrote:
> >> >> isn't going to be very helpful in the field, I'm afraid. Even more so
> >> >> that there's no guarantee for a UART to be at port 0x3f8. That's
> >> >
> >> > Right but we do not have big choice here at very early boot stage... 
> >> > :-(((
> >>
> >> Excuse me? You're running on EFI, so you have full infrastructure
> >> available to you. As much as in a normal BIOS scenario (and on a
> >> half way normal system) we can assume a text mode screen with
> >> video memory mapped at B8000, under EFI we can assume output
> >> capabilities (whichever the system owner set up in the firmware
> >> setup).
> >
> > Potentially we can do that for bad_cpu only. However, does it pays?
> > I suppose that most, if not all, platforms with UEFI have CPUs with
> > X86_FEATURE_LM.
>
> I'm not sure about that, keeping various Atoms in mind.

OK.

> > Sadly, It it not feasible for not_multiboot and mb2_too_old. Former
> > means that we were loaded by unknown boot loader and interface is
> > not defined.
>
> Hence we may at least assume accessing VGA memory won't do
> much bad.

Well, we are in black hole here but I am not sure that even in that
situation we should blindly poke VGA memory region. Especially on
EFI platforms behavior can be at least undefined.

> > Hence, we are not able to get anything from EFI. Latter
> > means that we were booted via older multiboot2 version which shutdown
> > boot services.
>
> Hmm, that's certainly one of the possibilities, yes. I wonder then
> whether we wouldn't better do away with all of those output
> attempts then.

This is one option. However, IMO, not nice. Maybe we should stay with
serial plus VGA on legacy BIOS platforms. Serials looks quite well
defined even on EFI platforms. However, if it is not available x86
out instruction should not do any harm. Though if we wish to increase
chance of reaching interested parties I would send error messages to
0x3f8 and 0x2f8 at once. AFAICT, on many machines usually at least one
if not both standard/legacy serial ports are available.

[...]

> >> >> > +        /*
> >> >> > +         * Multiboot2 information address is 32-bit,
> >> >> > +         * so, zero higher half of %rbx.
> >> >> > +         */
> >> >> > +        mov     %ebx,%ebx
> >> >>
> >> >> Wait, no - that's a protocol bug then. We're being entered in 64-bit
> >> >> mode here, so registers should be in 64-bit clean state.
> >> >
> >> > You mean higher half cleared. Right? This is not guaranteed.
> >>
> >> Hence me saying "that's a protocol bug then".
> >
> > Why? Protocol strictly says that "this is not guaranteed".
> > What is the problem with that? Potentially loader can set
> > %rbx higher half to e.g. 0 but I do not think it is needed.
>
> A 64-bit interface shouldn't specify values to live only in halves
> of registers, in my opinion. Remember that the architecture
> guarantees high halves to get zeroed for 32-bit writes to
> registers, so I don't even see any complication for the provider
> side of the interface (which necessarily runs in 64-bit mode).

OK, you convinced me. I will update GRUB2 patches.

> >> > Please check this:
> >> > http://lists.gnu.org/archive/html/grub-devel/2016-03/msg00304.html
> >>
> >> Other than the description of the patch I can't see anything like that,
> >> in particular
> >> - no occurrence of "ebx" in any of the added or changed code
> >> - MULTIBOOT_EFI_MBI_REGISTER getting #define-d as rbx
> >
> > Please check multiboot2 spec, section 3.2, Machine state. It is not
> > freshest one (I am working on EFI updates right now) but I hope that it,
> > together with patch comment, will shed some light.
>
> May I refer you back to you, in another patch, adding just two
> architecture defines for MB2: i386 and MIPS? That's a pretty
> clear indication that there can't be much consistency to be
> expected when talk comes to x86-64 (which most definitely is
> not i386).
>
> >> >> > @@ -170,12 +313,19 @@ multiboot2_proto:
> >> >> >          jne     1f
> >> >> >
> >> >> >          mov     MB2_mem_lower(%ecx),%edx
> >> >> > -        jmp     trampoline_setup
> >> >> > +        jmp     trampoline_bios_setup
> >> >> >
> >> >> >  1:
> >> >> > +        /* EFI mode is not supported via legacy BIOS path. */
> >> >> > +        cmpl    $MULTIBOOT2_TAG_TYPE_EFI32,MB2_tag_type(%ecx)
> >> >> > +        je      mb2_too_old
> >> >> > +
> >> >> > +        cmpl    $MULTIBOOT2_TAG_TYPE_EFI64,MB2_tag_type(%ecx)
> >> >> > +        je      mb2_too_old
> >> >>
> >> >> According to the comment we're on the legacy BIOS boot path
> >> >> here, yet at mb2_too_old you assume no text mode VGA. I.e. I'm
> >> >> now even confused about the output handling there.
> >> >
> >> > It is correct. Old GRUB2 (or other legacy multiboot2 compatible boot
> >> > loader) which runs on EFI platform and does not have my features will
> >> > jump into that path. And we do not support that, so, we should fail
> >> > in the best possible way here.
> >> >
> >> > Your comment suggest that code comment should be improved and
> >> > phrased precisely. I will do that.
> >>
> >> Not necessarily: First of all you didn't clarify what video mode we're
> >> in in that old-grub2 case. Do we have text mode? If so, output should
> >> not be avoided. And if we're in a graphical mode without any vga=
> >> option that grub2 may have chosen to interpret, that would smell like
> >> a bug in grub2.
> >
> > Here boot services are dead. They were shutdown by GRUB2 (or other
> > legacy boot loader). So, we do not have simple access to GOP or
> > anything like that. Am I missing something?
>
> How are boot services coming into the picture here? As the code
> comment still visible above says, we're on the legacy boot path
> here. And legacy boot, from all I know, implies a text mode on
> the primary VGA (if any).

Old multiboot2 protocol (without my features) used one path for legacy
BIOS and EFI platforms. If we are here then we are sure that:
  - we are running on EFI platform,
  - we were loaded by old multiboot2 protocol,
  - EFI boot services are shutdown,
  - there is no VGA here.

Daniel

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