xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: Jan Beulich <jbeulich@suse.com>
To: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: xen-devel <xen-devel@lists.xenproject.org>
Subject: Re: [Xen-devel] EFI development issues
Date: Mon, 13 Jan 2020 17:46:30 +0100	[thread overview]
Message-ID: <e32e75f1-08f1-bdff-b347-23293dafb933@suse.com> (raw)
In-Reply-To: <8b795995-4f61-af08-2ead-5a841cb709f0@citrix.com>

On 13.01.2020 17:02, Andrew Cooper wrote:
> My recent boot pagetable changes have caused me to work with the EFI
> build of Xen rather more than previously.
> 
> First, there is a dependency tracking bug in the build system.  Edits to
> xen/arch/x86/efi/efi-boot.h don't cause xen.efi to be regenerated.  From
> what I can tell, the file doesn't even get recompiled, because syntax
> errors even go unnoticed.

Not an issue here, I've just now tried it out. .boot.o.d also
correctly names the file here.

> Second, and the main point of the email.
> 
> The EFI code has some logic which does:
> 
> if ( !base_video )
> {
>     ...
> 
>     if ( best != StdOut->Mode->Mode )
>         StdOut->SetMode(StdOut, best);
> }
> 
> just before printing out the Xen banner.  This has a side effect of
> causing all further use of StdOut/StdErr to cease working, and
> interfering completely with debugging activities.

Interesting, and certainly unintended. Obviously the "normal" output
works (for me at least, with or without serial console, albeit I
don't think I've ever tried headless, i.e. _just_ a serial console),
so it's not exactly clear to me what other "debugging activities" it
may interfere with. A broken StdOut / StdErr protocol implementation
in the firmware?

>  (Waiting for a
> keypress on StdIn however does work, which is how I eventually diagnosed
> that it was an output problem.)  Skipping this logic allows debugging to
> work.

As should then do -basevideo.

> The code appeared in bf6501a62 "x86-64: EFI boot code" and has no
> specific description of what it is doing and/or trying to achieve.

efi_console_set_mode() is simple enough I think: It tries to maximize
screen dimensions. (There's some historical context here, as the
code wasn't written from scratch: Serial consoles often weren't
available when colleagues of mine and I did some of the original EFI
enabling work for a long canceled project. Plus there we had a rather
better (tm) kernel debugger, wanting to utilize as high resolution a
screen as possible to show as much useful information as possible at
any point in time.)

> It is also not entirely clear why it is gated on having a cfg file in
> the first place (c/s ,c38cf865ec8, not that there is adequate context
> for why)

The description of the cited commit is clear enough, isn't it?
This is just the same distinction (just placed differently) for
Arm as that between efi_start() (doing most of this stuff) and
efi_multiboot2() (not doing so, in particular the command line
parsing, and e.g. not even providing a means to suppress the
call to efi_console_set_mode()).

For anything beyond this I have to defer to the Arm folks, who
wanted it this way.

> or why there is a Xen command line argument "-basevideo"
> introduced in the beginning to skip this behaviour.

Traditionally video mode setting had its problems, hence we
anticipated there may be problems also with EFI doing so. As a
result we wanted, from the very beginning, a simply means to
get past any such.

> As a point of reference, I don't see Linux making any SetMode calls.

Well, if I'm not mistaken Xen's support for booting as an EFI
application predates Linux'es by quite a bit, so there was nothing
to reference there. As said, the origin of this code is elsewhere.

> What is the purpose of changing to a different mode?  Certainly as far
> as serial consoles go, sticking with the mode the loader uses certainly
> feels like a safer option.

Does a serial console report a "resolution" in the first place? And
if we were able to (sufficiently easily) tell video from serial
console, how would we deal with the case of StdOut / StdErr being
multiplexed to both?

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

  reply	other threads:[~2020-01-13 16:46 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-13 16:02 [Xen-devel] EFI development issues Andrew Cooper
2020-01-13 16:46 ` Jan Beulich [this message]
2020-01-16 20:28   ` Andrew Cooper
2020-01-17  8:57     ` Jan Beulich
2020-01-17 15:24 ` 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=e32e75f1-08f1-bdff-b347-23293dafb933@suse.com \
    --to=jbeulich@suse.com \
    --cc=andrew.cooper3@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).