Re: [Qemu-ppc] [PATCH 2/6] vga: increase priority of 0xa0000 memory region

[Hervé Poussineau]

Le 03/01/2017 à 00:02, David Gibson a écrit :
> On Thu, Dec 29, 2016 at 11:12:12PM +0100, Hervé Poussineau wrote:
> > VGA device registers vram as BAR 0. If this BAR is activated as a very low 
> > address which
> > crosses 0xa0000-0xbffff, low memory region is not accessible anymore.
> >
> > This fixes display on PReP machine if we enable PCI mapping at
> > address 0.
>
> This commit message needs more information.  What exactly is the VGA
> BAR colliding with?  Why does the other thing have higher priority?
> Why is it safe for the VGA BAR to override the other thing?  Why is
> this safe on all platforms?

VGA has (basically) two memory regions:
- the legacy one, from 0xa0000 to 0xbffff
- a memory region describing the whole VRAM, configurable with PCI BAR 0.

In QEMU, mapping PCI at address 0 is not permitted 
(MachineClass->pci_allow_0_address is false by default),
except on arm/virt and ppc/spapr. So, this is usually not a problem as all PCI 
BARs (including video PCI BAR 0) are not mapped at
address 0, and so both memory regions can't collide.

When trying Linux on ppc/40p (introduced later in this patchset), I saw that 
Linux assigns
PCI BAR addresses in the order of PCI devices detection, and VGA PCI BAR 0 ends 
up at address 0.
However, Linux assumes that 0xa0000-0xbffff is still available to drive the VGA 
card in text mode.
Without changing the region priority, VGA output is garbled (as writes directly 
go to VGA memory).
When increasing the region priority, VGA output is restored (as writes go to 
legacy address space).

arm/virt by default doesn't have a PCI VGA card.
ppc/spapr may probably have the same problem as ppc/40p. However, as VGA PCI 
BAR 0 and VGA legacy space
both have a priority of 1, it may probably have a problem. I only need to 
convince Linux to use address 0 for VGA PCI BAR.

Hervé

> > Signed-off-by: Hervé Poussineau <address@hidden>
> > ---
> >  hw/display/vga.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/hw/display/vga.c b/hw/display/vga.c
> > index 2a88b3c..c573f35 100644
> > --- a/hw/display/vga.c
> > +++ b/hw/display/vga.c
> > @@ -2265,7 +2265,7 @@ void vga_init(VGACommonState *s, Object *obj, 
> > MemoryRegion *address_space,
> >      memory_region_add_subregion_overlap(address_space,
> >                                          0x000a0000,
> >                                          vga_io_memory,
> > -                                        1);
> > +                                        2);
> >      memory_region_set_coalescing(vga_io_memory);
> >      if (init_vga_ports) {
> >          portio_list_init(&s->vga_port_list, obj, vga_ports, s, "vga");