Re: [PATCH 4/5] xen: fix handling framebuffer located above 4GB

From: "Jan Beulich" <JBeulich@suse.com>
To: Marek Marczykowski <marmarek@invisiblethingslab.com>
Cc: Juergen Gross <jgross@suse.com>,
	Stefano Stabellini <sstabellini@kernel.org>,
	Wei Liu <wei.liu2@citrix.com>,
	Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
	George Dunlap <George.Dunlap@eu.citrix.com>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	Ian Jackson <Ian.Jackson@eu.citrix.com>, Tim Deegan <tim@xen.org>,
	Julien Grall <julien.grall@arm.com>,
	xen-devel <xen-devel@lists.xenproject.org>,
	Roger Pau Monne <roger.pau@citrix.com>
Subject: Re: [PATCH 4/5] xen: fix handling framebuffer located above 4GB
Date: Wed, 08 May 2019 03:54:45 -0600	[thread overview]
Message-ID: <5CD2A765020000780022CBBC@prv1-mh.provo.novell.com> (raw)
In-Reply-To: <20190507164311.GB1502@mail-itl>

>>> On 07.05.19 at 18:43, <marmarek@invisiblethingslab.com> wrote:
> On Tue, May 07, 2019 at 10:12:13AM -0600, Jan Beulich wrote:
>> >>> On 07.05.19 at 17:38, <marmarek@invisiblethingslab.com> wrote:
>> > What do you think about adding something that could be backported?
>> > Xen is quite insistent on initializing framebuffer, even with
>> > console=com1 or console=none. Which means, there is no workaround for
>> > this problem.
>> 
>> When the system is in a simple text mode the /basevideo option of
>> xen.efi ought to help, but if it's in an LFB-based mode already (which
>> is the case on the systems I have) then indeed I can't see any
>> workaround.
>> 
>> > Maybe, as a first step, a change that abort framebuffer initialization
>> > if lfb_base == 0 (I assume this is never valid value here, right?)?
>> 
>> Yes, that would be an option. But it would help only in your specific
>> case, not if the truncation results in a non-zero value. I guess we'd
>> better avoid filling the structure if we'd truncate the value.
> 
> Yes, I was thinking about setting lfb_base=0 explicitly if it would
> overflow otherwise.
> 
>> But what's wrong with backporting your change as is?
> 
> If this commit would be backported, what value you'd put in that #ifdef?

I'd keep it as is. The field addition happens for 4.13. And as you say ...

> Also, one may argue that ABI changes should not be backported... But
> since there is clear and independent of xen version method of detecting
> it, I don't think this would be a big issue here.

... there's not really any issue with surfacing this also in older
versions.

>> > If not, then at least abort boot when text console is still there
>> > (blexit before efi_exit_boot). Any preference?
>> 
>> What's wrong with the text console still active? Or maybe I'm
>> misunderstandint you make...
> 
> As soon as you call ExitBootServices(), you can't use
> SIMPLE_TEXT_OUTPUT_INTERFACE anymore. Which means if a) framebuffer
> address didn't fit, and b) you called ExitBootServices() already, you
> don't have any means to tell the user what is wrong. Other than serial
> console of course, if you're lucky enough to have one. So the idea was
> to report the problem before ExitBootServices().

Oh, so be "text console" you meant the EFI interface, not a
console in text mode (which we can drive). Failing to boot in
such a case seems worse to me than booting effectively
headless.

Jan

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