Re: [PATCH v3] x86/vga: fix mapping of the VGA text buffer

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Thu, Mar 20, 2025 at 04:25:25PM +0100, Jan Beulich wrote:
> On 20.03.2025 16:11, 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 use __va() for the VGA text
> > buffer, as it's below the 1MB boundary, and thus always mapped in the
> > directmap.
> > 
> > Fixes: 81d195c6c0e2 ('x86: introduce ioremap_wc()')
> > Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> 
> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

Thanks.

> Nevertheless a remark:
> 
> > --- a/xen/drivers/video/vga.c
> > +++ b/xen/drivers/video/vga.c
> > @@ -71,7 +71,7 @@ void __init video_init(void)
> >      {
> >      case XEN_VGATYPE_TEXT_MODE_3:
> >          if ( page_is_ram_type(paddr_to_pfn(0xB8000), 
> > RAM_TYPE_CONVENTIONAL) ||
> > -             ((video = ioremap_wc(0xB8000, 0x8000)) == NULL) )
> > +             ((video = __va(0xB8000)) == NULL) )
> 
> Without having a good suggestion, find such dependencies upon the low
> 1Mb always being mapped (in case we wanted to revisit this, for example)
> is going to be moderately hard. It might be good to somehow mark them.

Hm, if we where to change this, we should likely go over all the
usages of ioremap() and __va() to find them?

I assume you are fine with this going in as-is.

Thanks, Roger.