[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH 6/7] x86/vga: fix mapping of the VGA text buffer
(resending because I seem to have inadvertently corrupted the Cc field) On Thu, Mar 13, 2025 at 07:39:58PM +0000, Andrew Cooper wrote: > On 13/03/2025 3:30 pm, Roger Pau Monne wrote: > > The call to ioremap_wc() in video_init() will always fail, because > > video_init() is called ahead of vm_init_type(), and so the underlying > > __vmap() call will fail to allocate the linear address space. > > > > Fix by reverting to the previous behavior and using the directmap entries > > in the low 1MB. Note the VGA text buffer directmap entries are also > > adjusted to map the VGA text buffer as WC instead of UC-. > > > > Fixes: 81d195c6c0e2 ('x86: introduce ioremap_wc()') > > Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx> > > --- > > xen/arch/x86/boot/x86_64.S | 10 +++++++--- > > xen/arch/x86/include/asm/config.h | 5 +++++ > > xen/drivers/video/vga.c | 11 ++++++++--- > > 3 files changed, 20 insertions(+), 6 deletions(-) > > > > diff --git a/xen/arch/x86/boot/x86_64.S b/xen/arch/x86/boot/x86_64.S > > index 26b9d1c2df9a..07f4bdf46e31 100644 > > --- a/xen/arch/x86/boot/x86_64.S > > +++ b/xen/arch/x86/boot/x86_64.S > > @@ -84,15 +84,19 @@ ENTRY(__high_start) > > /* > > * Mapping of first 2 megabytes of memory. This is mapped with 4kB mappings > > * to avoid type conflicts with fixed-range MTRRs covering the lowest > > megabyte > > - * of physical memory. In any case the VGA hole should be mapped with type > > UC. > > + * of physical memory. VGA hole should be mapped with type UC, with the > > + * exception of the text buffer that uses WC. > > * Uses 1x 4k page. > > */ > > l1_directmap: > > pfn = 0 > > .rept L1_PAGETABLE_ENTRIES > > - /* VGA hole (0xa0000-0xc0000) should be mapped UC-. */ > > - .if pfn >= 0xa0 && pfn < 0xc0 > > + /* VGA hole (0xa0000-0xb8000) should be mapped UC-. */ > > + .if pfn >= 0xa0 && pfn < 0xb8 > > .quad (pfn << PAGE_SHIFT) | __PAGE_HYPERVISOR_UCMINUS | > > _PAGE_GLOBAL | MAP_SMALL_PAGES > > + /* VGA text buffer (0xb80000-0xc0000) should be mapped WC. */ > > + .elseif pfn >= 0xb8 && pfn < 0xc0 > > + .quad (pfn << PAGE_SHIFT) | __PAGE_HYPERVISOR_WC | _PAGE_GLOBAL | > > MAP_SMALL_PAGES > > .else > > .quad (pfn << PAGE_SHIFT) | PAGE_HYPERVISOR_RWX | MAP_SMALL_PAGES > > .endif > > We have to be careful doing this. > > It probably is safe to use WC in the pagetables. We don't start using > the pagetables until after we're sure we're on a 64bit CPU, which means > WC is available. > > However, doing so now means that we need explicit SFENCE's when using > this, even in places like early_error. The IN/OUT instructions do flush > WC buffers, but the UART is written to before the screen, so there's a > chance that you'll lose the final character of the message on the screen. I don't think early_error will ever use this mapping. `vga_text_buffer` contains the address 0xb8000, and AFAICT it's exclusively used with paging disabled (as the multiboot2 efi path explicitly sets vga_text_buffer = 0). The WC mapping created above is on the directmap, so va > DIRECTMAP_VIRT_START. vga_text_puts() might need such SFENCE, but arguably that should be a different patch IMO. Might be best to ask Jan whether this is on purpose? My hypothesis is that the SFENCE might only be needed in video_endboot() and before reboot if Xen crashed ahead of relinquishing the VGA console. Thanks, Roger.
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |