Xen-Devel Archive on lore.kernel.org
 help / color / Atom feed
From: Andrew Cooper <andrew.cooper3@citrix.com>
To: Jan Beulich <jbeulich@suse.com>
Cc: xen-devel <xen-devel@lists.xenproject.org>
Subject: Re: [Xen-devel] EFI development issues
Date: Thu, 16 Jan 2020 20:28:21 +0000
Message-ID: <9a0fb4ab-831d-f293-7fa1-b969a1571690@citrix.com> (raw)
In-Reply-To: <e32e75f1-08f1-bdff-b347-23293dafb933@suse.com>

On 13/01/2020 16:46, Jan Beulich wrote:
> 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.

My .boot.o.d looks plausible as well, but there is clearly an issue
somewhere.  It is completely reliable on a Debian system with default
make and toolchain.

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

I have an HTML5 virtual KVM to the box.  Looking at it as well as the
serial (which is ultimately the same IPMI controller), the VGA screen
does expand its dimensions a little and show the debugging which is
missing on the serial.

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

That is another observation.  It is very confusing having command line
parameters to xen.efi which are different to command line parameters to
Xen.  The parameters to xen.efi aren't documented anywhere, as far as I
can tell.

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

It states the changes being made.  It doesn't explain why this is a
helpful change to make.

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

Hmm.  This is need of abstracting out, so we don't have two totally
different ways of only doing a subset of EFI setup when actually started
via multiboot2.

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

I wasn't going for heritage.  Simply for "what do other things in a
similar position to Xen do".

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

How would I find out?

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

In my case they are multiplexed, and all work fine before set_mode() call.

After set_mode(), VGA still works, while serial sees nothing further on
StdOut/Stderr (pre-existing lines are still in place).

StdIn still works for both as well.  I had inserted a wait-for-keypress,
in case there was a buffering issue causing the StdOut content not to be
emitted before the triple fault, and ended up with no debugging on the
serial, and a system which triple faulted once it had seen a keypress.

~Andrew

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

  reply index

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-13 16:02 Andrew Cooper
2020-01-13 16:46 ` Jan Beulich
2020-01-16 20:28   ` Andrew Cooper [this message]
2020-01-17  8:57     ` Jan Beulich
2020-01-17 15:24 ` Jan Beulich

Reply instructions:

You may reply publically 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=9a0fb4ab-831d-f293-7fa1-b969a1571690@citrix.com \
    --to=andrew.cooper3@citrix.com \
    --cc=jbeulich@suse.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

Xen-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/xen-devel/0 xen-devel/git/0.git
	git clone --mirror https://lore.kernel.org/xen-devel/1 xen-devel/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 xen-devel xen-devel/ https://lore.kernel.org/xen-devel \
		xen-devel@lists.xenproject.org xen-devel@lists.xen.org
	public-inbox-index xen-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.xenproject.lists.xen-devel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git