[PATCH 0/5] gfx: improvements when using multiboot2 and EFI + misc

Roger Pau Monne posted 5 patches 1 week, 4 days ago
Test gitlab-ci failed
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/20221123154525.63068-1-roger.pau@citrix.com
xen/arch/x86/boot/head.S          | 15 ++++++++--
xen/arch/x86/efi/efi-boot.h       | 48 +++++++++++++++++++++++++++++--
xen/arch/x86/platform_hypercall.c | 11 +++++++
xen/arch/x86/x86_64/asm-offsets.c |  1 +
xen/common/efi/boot.c             | 25 ++++++++++++++++
xen/drivers/video/vga.c           |  2 +-
xen/include/public/platform.h     |  6 ++++
7 files changed, 103 insertions(+), 5 deletions(-)
[PATCH 0/5] gfx: improvements when using multiboot2 and EFI + misc
Posted by Roger Pau Monne 1 week, 4 days ago
Hello,

The following series contains some fixes and improvements related to
graphics usage when booting Xen.

First patch introduces a new platform hypercall to pass the graphics
console information and mode to a PVH dom0, which doesn't have this
information available as part of the start_info contents.

Further patches fix some shortcomings when using multiboot2, mostly the
ignoring of the console=vga (or lack of) and the vga=gfx- parameters.
It also switches default Xen behaviour from trying to reuse the
currently set GOP mode instead of attempting to set the maximum
supported resolution.

Marek: after this series using console= without the vga option should
result in Xen not attempting to touch the selected GOP mode and the
screen not getting cleared.

Thanks, Roger.

Roger Pau Monne (5):
  x86/platform: introduce hypercall to get initial video console
    settings
  efi: only set a console mode if the current one is invalid
  efi: try to use the currently set GOP mode
  multiboot2: parse console= option when setting GOP mode
  multiboot2: parse vga= option when setting GOP mode

 xen/arch/x86/boot/head.S          | 15 ++++++++--
 xen/arch/x86/efi/efi-boot.h       | 48 +++++++++++++++++++++++++++++--
 xen/arch/x86/platform_hypercall.c | 11 +++++++
 xen/arch/x86/x86_64/asm-offsets.c |  1 +
 xen/common/efi/boot.c             | 25 ++++++++++++++++
 xen/drivers/video/vga.c           |  2 +-
 xen/include/public/platform.h     |  6 ++++
 7 files changed, 103 insertions(+), 5 deletions(-)

-- 
2.37.3
Re: [PATCH 0/5] gfx: improvements when using multiboot2 and EFI + misc
Posted by Marek Marczykowski-Górecki 1 week, 3 days ago
On Wed, Nov 23, 2022 at 04:45:19PM +0100, Roger Pau Monne wrote:
> Marek: after this series using console= without the vga option should
> result in Xen not attempting to touch the selected GOP mode and the
> screen not getting cleared.

Thanks, this seems to work mostly fine.
There is one message printed from setup_efi_pci(): ... ROM ... bytes at ...
I'm not sure what to do about this one (although for Qubes, I can simply
patch it out ;) ).

But to get dom0 display image from BGRT, it seems something else is
needed too. Linux complains "Incorrect checksum in table [BGRT]". The
only relevant google result I get is this: https://support.citrix.com/article/CTX460227/citrix-hypervisor-acpi-warning-incorrect-checksum-in-table-bgrt
It blames firmware. But then, it's suspicious that it's also about Xen.
And also, native Linux on the same hw does not complain about the
checksum. So, I think it's rather Xen to blame...
The table lives in area marked as EfiACPIReclaimMemory in memory map, so
I think it shouldn't be clobbered by Xen, at least in theory. I'll look
into it later. It's getting off-topic for this thread anyway.

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
Re: [PATCH 0/5] gfx: improvements when using multiboot2 and EFI + misc
Posted by Roger Pau Monné 1 week, 3 days ago
On Thu, Nov 24, 2022 at 06:15:15AM +0100, Marek Marczykowski-Górecki wrote:
> On Wed, Nov 23, 2022 at 04:45:19PM +0100, Roger Pau Monne wrote:
> > Marek: after this series using console= without the vga option should
> > result in Xen not attempting to touch the selected GOP mode and the
> > screen not getting cleared.
> 
> Thanks, this seems to work mostly fine.
> There is one message printed from setup_efi_pci(): ... ROM ... bytes at ...
> I'm not sure what to do about this one (although for Qubes, I can simply
> patch it out ;) ).

Hm, I'm unsure.  As a starter they could be gated to debug hypervisor only
builds.  And then I'm unsure whether this information couldn't be
printed later when the console option has been parsed, instead of
printing it from the EFI console interface.

> But to get dom0 display image from BGRT, it seems something else is
> needed too. Linux complains "Incorrect checksum in table [BGRT]". The
> only relevant google result I get is this: https://support.citrix.com/article/CTX460227/citrix-hypervisor-acpi-warning-incorrect-checksum-in-table-bgrt
> It blames firmware. But then, it's suspicious that it's also about Xen.
> And also, native Linux on the same hw does not complain about the
> checksum. So, I think it's rather Xen to blame...
> The table lives in area marked as EfiACPIReclaimMemory in memory map, so
> I think it shouldn't be clobbered by Xen, at least in theory. I'll look
> into it later. It's getting off-topic for this thread anyway.

See commit 89238ef7797023f318f82f4f9dddef59c435b8bd.  I wonder whether
the BGRT image region is marked as EFI_MEMORY_RUNTIME, I will have to
check on my system.

Thanks, Roger.

Re: [PATCH 0/5] gfx: improvements when using multiboot2 and EFI + misc
Posted by Roger Pau Monné 1 week, 3 days ago
On Thu, Nov 24, 2022 at 09:59:25AM +0100, Roger Pau Monné wrote:
> On Thu, Nov 24, 2022 at 06:15:15AM +0100, Marek Marczykowski-Górecki wrote:
> > But to get dom0 display image from BGRT, it seems something else is
> > needed too. Linux complains "Incorrect checksum in table [BGRT]". The
> > only relevant google result I get is this: https://support.citrix.com/article/CTX460227/citrix-hypervisor-acpi-warning-incorrect-checksum-in-table-bgrt
> > It blames firmware. But then, it's suspicious that it's also about Xen.
> > And also, native Linux on the same hw does not complain about the
> > checksum. So, I think it's rather Xen to blame...
> > The table lives in area marked as EfiACPIReclaimMemory in memory map, so
> > I think it shouldn't be clobbered by Xen, at least in theory. I'll look
> > into it later. It's getting off-topic for this thread anyway.
> 
> See commit 89238ef7797023f318f82f4f9dddef59c435b8bd.  I wonder whether
> the BGRT image region is marked as EFI_MEMORY_RUNTIME, I will have to
> check on my system.

Just checked on my system, and the BGRT image is placed in a
EfiBootServicesData section with no EFI_MEMORY_RUNTIME attribute.

To fix this we would need to change efi_arch_process_memory_map() so
it takes the BGRT image address into account and marks the region
where it's placed as reserved.  I'm not aware of anyway to get such
address from EFI data, so we would likely need to parse the BGRT in
efi_arch_process_memory_map().

Thanks, Roger.

Re: [PATCH 0/5] gfx: improvements when using multiboot2 and EFI + misc
Posted by Marek Marczykowski-Górecki 1 week, 3 days ago
On Thu, Nov 24, 2022 at 10:56:33AM +0100, Roger Pau Monné wrote:
> On Thu, Nov 24, 2022 at 09:59:25AM +0100, Roger Pau Monné wrote:
> > On Thu, Nov 24, 2022 at 06:15:15AM +0100, Marek Marczykowski-Górecki wrote:
> > > But to get dom0 display image from BGRT, it seems something else is
> > > needed too. Linux complains "Incorrect checksum in table [BGRT]". The
> > > only relevant google result I get is this: https://support.citrix.com/article/CTX460227/citrix-hypervisor-acpi-warning-incorrect-checksum-in-table-bgrt
> > > It blames firmware. But then, it's suspicious that it's also about Xen.
> > > And also, native Linux on the same hw does not complain about the
> > > checksum. So, I think it's rather Xen to blame...
> > > The table lives in area marked as EfiACPIReclaimMemory in memory map, so
> > > I think it shouldn't be clobbered by Xen, at least in theory. I'll look
> > > into it later. It's getting off-topic for this thread anyway.
> > 
> > See commit 89238ef7797023f318f82f4f9dddef59c435b8bd.  I wonder whether
> > the BGRT image region is marked as EFI_MEMORY_RUNTIME, I will have to
> > check on my system.
> 
> Just checked on my system, and the BGRT image is placed in a
> EfiBootServicesData section with no EFI_MEMORY_RUNTIME attribute.

Right, while the BGRT table itself is in EfiACPIReclaimMemory, the image
it points to lives in EfiBootServicesData. And no EFI_MEMORY_RUNTIME
attribute there either.

> To fix this we would need to change efi_arch_process_memory_map() so
> it takes the BGRT image address into account and marks the region
> where it's placed as reserved.  I'm not aware of anyway to get such
> address from EFI data, so we would likely need to parse the BGRT in
> efi_arch_process_memory_map().

Since Xen has code to do that already, moving it earlier shouldn't be
too much issue. Can `acpi_boot_table_init()` be called that early? And
then, it sounds very similar to the issue we have with the ESRT table.

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
Re: [PATCH 0/5] gfx: improvements when using multiboot2 and EFI + misc
Posted by Jan Beulich 1 week, 3 days ago
On 24.11.2022 06:15, Marek Marczykowski-Górecki wrote:
> On Wed, Nov 23, 2022 at 04:45:19PM +0100, Roger Pau Monne wrote:
>> Marek: after this series using console= without the vga option should
>> result in Xen not attempting to touch the selected GOP mode and the
>> screen not getting cleared.
> 
> Thanks, this seems to work mostly fine.
> There is one message printed from setup_efi_pci(): ... ROM ... bytes at ...
> I'm not sure what to do about this one (although for Qubes, I can simply
> patch it out ;) ).

What's wrong with that message? It's not directly related to gfx devices
anyway; it merely happens to be the case that gfx devices are the most
common ones to come with a ROM.

Jan