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

[Marek Marczykowski <marmarek@xxxxxxxxxxxxxxxxxxxxxx>]

On Wed, May 08, 2019 at 03:54:45AM -0600, Jan Beulich wrote:
> >>> On 07.05.19 at 18:43, <marmarek@xxxxxxxxxxxxxxxxxxxxxx> wrote:
> > On Tue, May 07, 2019 at 10:12:13AM -0600, Jan Beulich wrote:
> >> >>> On 07.05.19 at 17:38, <marmarek@xxxxxxxxxxxxxxxxxxxxxx> 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.

You mean to keep it without #ifdef then? I'm not following... If you add
#ifdef __XEN_INTERFACE_VERSION__ >= 0x00040d00 there, the field won't be
available in Xen < 4.13. Which effectively means the patch can't be
backported as it won't compile with Xen < 4.13. Note also that this
structure is the place that Xen use to keep that information internally
(xen_vga_console_info is another name for dom0_vga_console_info), it
isn't only about passing this information to dom0.

Maybe add #ifdef __XEN_INTERFACE_VERSION__ >= 0x00040a00, as the oldest
fully supported version? This will mitigate one of the issues with the
lack of #ifdef (potential conflict with gbl_caps with
__XEN_INTERFACE_VERSION__ < 0x00030206).

Or use some #if meaning Xen interface >= 4.13, or Xen internal build?

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

Yes, if the alternative is booting headless, then indeed it's better
than refusing to boot with a message. But if the alternative is a
mysterious crash without any message...

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

Attachment: signature.asc
Description: PGP signature

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