xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: Daniel Kiper <daniel.kiper@oracle.com>
To: Jan Beulich <JBeulich@suse.com>
Cc: Juergen Gross <JGross@suse.com>,
	stefano.stabellini@eu.citrix.com, andrew.cooper3@citrix.com,
	cardoe@cardoe.com, pgnet.dev@gmail.com, ning.sun@intel.com,
	david.vrabel@citrix.com, xen-devel@lists.xenproject.org,
	qiaowei.ren@intel.com, richard.l.maliszewski@intel.com,
	gang.wei@intel.com, fu.wei@linaro.org
Subject: Re: [PATCH v3 13/16 - RFC] x86: add multiboot2 protocol support for EFI platforms
Date: Thu, 2 Jun 2016 18:12:52 +0200	[thread overview]
Message-ID: <20160602161252.GT5490@olila.local.net-space.pl> (raw)
In-Reply-To: <57500BB502000078000F0C0E@prv-mh.provo.novell.com>

On Thu, Jun 02, 2016 at 02:34:28AM -0600, Jan Beulich wrote:
> >>> On 01.06.16 at 21:03, <daniel.kiper@oracle.com> wrote:
> > On Fri, May 27, 2016 at 03:02:25AM -0600, Jan Beulich wrote:
> >> >>> On 25.05.16 at 23:02, <daniel.kiper@oracle.com> 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@lists.xen.org
http://lists.xen.org/xen-devel

  reply	other threads:[~2016-06-02 16:13 UTC|newest]

Thread overview: 94+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-04-15 12:33 [PATCH v3 00/16] x86: multiboot2 protocol support Daniel Kiper
2016-04-15 12:33 ` [PATCH v3 01/16] x86/boot: do not create unwind tables Daniel Kiper
2016-04-15 15:45   ` Andrew Cooper
2016-04-15 12:33 ` [PATCH v3 02/16] x86: zero BSS using stosl instead of stosb Daniel Kiper
2016-04-15 13:57   ` Konrad Rzeszutek Wilk
2016-04-15 15:48   ` Andrew Cooper
2016-04-15 12:33 ` [PATCH v3 03/16] x86/boot: call reloc() using cdecl calling convention Daniel Kiper
2016-04-15 15:56   ` Andrew Cooper
2016-06-17  8:41     ` Daniel Kiper
2016-06-17  9:30       ` Jan Beulich
2016-05-24  8:42   ` Jan Beulich
2016-04-15 12:33 ` [PATCH v3 04/16] x86/boot/reloc: create generic alloc and copy functions Daniel Kiper
2016-04-15 12:33 ` [PATCH v3 05/16] x86/boot: use %ecx instead of %eax Daniel Kiper
2016-04-15 12:33 ` [PATCH v3 06/16] x86/boot/reloc: Rename some variables and rearrange code a bit Daniel Kiper
2016-04-15 12:33 ` [PATCH v3 07/16] x86/boot: create *.lnk files with linker script Daniel Kiper
2016-04-15 14:04   ` Konrad Rzeszutek Wilk
2016-05-24  9:05   ` Jan Beulich
2016-05-24 12:28     ` Daniel Kiper
2016-05-24 12:52       ` Jan Beulich
2016-06-17  9:06         ` Daniel Kiper
2016-06-17 10:04           ` Jan Beulich
2016-06-17 10:34             ` Daniel Kiper
2016-04-15 12:33 ` [PATCH v3 08/16] x86: add multiboot2 protocol support Daniel Kiper
2016-05-24 15:46   ` Jan Beulich
2016-05-25 16:34     ` Daniel Kiper
2016-05-26 10:28       ` Andrew Cooper
2016-05-27  8:08         ` Jan Beulich
2016-05-27  8:13           ` Andrew Cooper
2016-05-27  8:24             ` Jan Beulich
2016-05-27  8:11       ` Jan Beulich
2016-04-15 12:33 ` [PATCH v3 09/16] efi: explicitly define efi struct in xen/arch/x86/efi/stub.c Daniel Kiper
2016-05-25  7:03   ` Jan Beulich
2016-05-25 16:45     ` Daniel Kiper
2016-05-27  8:16       ` Jan Beulich
2016-06-01 15:07         ` Daniel Kiper
2016-07-05 18:33         ` Daniel Kiper
2016-07-06  6:55           ` Jan Beulich
2016-07-06 10:27             ` Daniel Kiper
2016-07-06 12:00               ` Jan Beulich
2016-07-06 12:55                 ` Daniel Kiper
2016-04-15 12:33 ` [PATCH v3 10/16] efi: create efi_enabled() Daniel Kiper
2016-05-25  7:20   ` Jan Beulich
2016-05-25 17:15     ` Daniel Kiper
2016-05-26 10:31       ` Andrew Cooper
2016-05-27  8:22       ` Jan Beulich
2016-06-01 15:23         ` Daniel Kiper
2016-06-01 15:41           ` Jan Beulich
2016-06-01 19:28             ` Daniel Kiper
2016-06-02  8:06               ` Jan Beulich
2016-04-15 12:33 ` [PATCH v3 11/16] efi: build xen.gz with EFI code Daniel Kiper
2016-05-25  7:53   ` Jan Beulich
2016-05-25 19:07     ` Daniel Kiper
2016-05-27  8:31       ` Jan Beulich
2016-06-01 15:48         ` Daniel Kiper
2016-06-01 15:58           ` Jan Beulich
2016-06-01 19:39             ` Daniel Kiper
2016-04-15 12:33 ` [PATCH v3 12/16 - RFC] x86/efi: create new early memory allocator Daniel Kiper
2016-05-25  8:39   ` Jan Beulich
2016-05-25 19:48     ` Daniel Kiper
2016-05-27  8:37       ` Jan Beulich
2016-06-01 15:58         ` Daniel Kiper
2016-06-01 16:02           ` Jan Beulich
2016-06-01 19:53             ` Daniel Kiper
2016-06-02  8:11               ` Jan Beulich
2016-06-02 10:43                 ` Daniel Kiper
2016-06-02 11:10                   ` Jan Beulich
2016-06-01 16:01         ` Daniel Kiper
2016-07-05 18:26   ` Daniel Kiper
2016-07-06  7:22     ` Jan Beulich
2016-07-06 11:15       ` Daniel Kiper
2016-04-15 12:33 ` [PATCH v3 13/16 - RFC] x86: add multiboot2 protocol support for EFI platforms Daniel Kiper
2016-05-25  9:32   ` Jan Beulich
2016-05-25 10:29     ` Jan Beulich
2016-05-25 21:02     ` Daniel Kiper
2016-05-27  9:02       ` Jan Beulich
2016-06-01 19:03         ` Daniel Kiper
2016-06-02  8:34           ` Jan Beulich
2016-06-02 16:12             ` Daniel Kiper [this message]
2016-06-03  9:26               ` Jan Beulich
2016-06-03 17:06                 ` Konrad Rzeszutek Wilk
2016-04-15 12:33 ` [PATCH v3 14/16] x86/boot: implement early command line parser in C Daniel Kiper
2016-05-25 10:33   ` Jan Beulich
2016-05-25 21:36     ` Daniel Kiper
2016-05-27  9:33       ` Jan Beulich
2016-06-02  8:15         ` Daniel Kiper
2016-06-02  8:39           ` Jan Beulich
2016-04-15 12:33 ` [PATCH v3 15/16 - RFC] x86: make Xen early boot code relocatable Daniel Kiper
2016-05-25 10:48   ` Jan Beulich
2016-04-15 12:33 ` [PATCH v3 16/16] x86: add multiboot2 protocol support for relocatable images Daniel Kiper
2016-05-25 11:03   ` Jan Beulich
2016-06-01 13:35     ` Daniel Kiper
2016-06-01 14:44       ` Jan Beulich
2016-06-01 19:16         ` Daniel Kiper
2016-06-02  8:41           ` Jan Beulich

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160602161252.GT5490@olila.local.net-space.pl \
    --to=daniel.kiper@oracle.com \
    --cc=JBeulich@suse.com \
    --cc=JGross@suse.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=cardoe@cardoe.com \
    --cc=david.vrabel@citrix.com \
    --cc=fu.wei@linaro.org \
    --cc=gang.wei@intel.com \
    --cc=ning.sun@intel.com \
    --cc=pgnet.dev@gmail.com \
    --cc=qiaowei.ren@intel.com \
    --cc=richard.l.maliszewski@intel.com \
    --cc=stefano.stabellini@eu.citrix.com \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).