Patchwork pci: Teach PCI Bridges about VGA routing

login
register
mail settings
Submitter Alex Williamson
Date Feb. 28, 2013, 7 p.m.
Message ID <20130228185911.24342.74091.stgit@bling.home>
Download mbox | patch
Permalink /patch/224160/
State New
Headers show

Comments

Alex Williamson - Feb. 28, 2013, 7 p.m.
Each PCI Bridge has a set of implied VGA regions that are enabled
when the VGA bit is set in the bridge control register.  This allows
VGA devices behind bridges.

Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---
 hw/pci/pci_bridge.c |   47 ++++++++++++++++++++++++++++++++++++++++++++---
 hw/pci/pci_bus.h    |   15 +++++++++++++++
 2 files changed, 59 insertions(+), 3 deletions(-)
Michael S. Tsirkin - Feb. 28, 2013, 8:26 p.m.
On Thu, Feb 28, 2013 at 12:00:03PM -0700, Alex Williamson wrote:
> Each PCI Bridge has a set of implied VGA regions that are enabled
> when the VGA bit is set in the bridge control register.  This allows
> VGA devices behind bridges.
> 
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> ---
>  hw/pci/pci_bridge.c |   47 ++++++++++++++++++++++++++++++++++++++++++++---
>  hw/pci/pci_bus.h    |   15 +++++++++++++++
>  2 files changed, 59 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/pci/pci_bridge.c b/hw/pci/pci_bridge.c
> index 995842a..42cbfd1 100644
> --- a/hw/pci/pci_bridge.c
> +++ b/hw/pci/pci_bridge.c
> @@ -151,6 +151,37 @@ static void pci_bridge_init_alias(PCIBridge *bridge, MemoryRegion *alias,
>      memory_region_add_subregion_overlap(parent_space, base, alias, 1);
>  }
>  
> +static void pci_bridge_init_vga_aliases(PCIBridge *br, PCIBus *parent,
> +                                        PCIBridgeVgaWindows *vga)
> +{
> +    uint16_t cmd = pci_get_word(br->dev.config + PCI_COMMAND);
> +    uint8_t brctl = pci_get_byte(br->dev.config + PCI_BRIDGE_CONTROL);
> +
> +    memory_region_init_alias(&vga->alias_io_lo, "pci_bridge_vga_io_lo",
> +                             &br->address_space_io, 0x3b0, 0xc);
> +    memory_region_add_subregion_overlap(parent->address_space_io, 0x3b0,
> +                                        &vga->alias_io_lo, 1);
> +
> +    memory_region_init_alias(&vga->alias_io_hi, "pci_bridge_vga_io_hi",
> +                             &br->address_space_io, 0x3c0, 0x20);
> +    memory_region_add_subregion_overlap(parent->address_space_io, 0x3c0,
> +                                        &vga->alias_io_hi, 1);
> +
> +    if (!(cmd & PCI_COMMAND_IO) || !(brctl & PCI_BRIDGE_CTL_VGA)) {
> +        memory_region_set_enabled(&vga->alias_io_lo, false);
> +        memory_region_set_enabled(&vga->alias_io_hi, false);
> +    }
> +
> +    memory_region_init_alias(&vga->alias_mem, "pci_bridge_vga_mem",
> +                             &br->address_space_mem, 0xa0000, 0x20000);
> +    memory_region_add_subregion_overlap(parent->address_space_mem, 0xa0000,
> +                                        &vga->alias_mem, 1);
> +
> +    if (!(cmd & PCI_COMMAND_MEMORY) || !(brctl & PCI_BRIDGE_CTL_VGA)) {
> +        memory_region_set_enabled(&vga->alias_mem, false);
> +    }
> +}
> +
>  static PCIBridgeWindows *pci_bridge_region_init(PCIBridge *br)
>  {
>      PCIBus *parent = br->dev.bus;
> @@ -175,7 +206,8 @@ static PCIBridgeWindows *pci_bridge_region_init(PCIBridge *br)
>                            &br->address_space_io,
>                            parent->address_space_io,
>                            cmd & PCI_COMMAND_IO);
> -   /* TODO: optinal VGA and VGA palette snooping support. */
> +
> +    pci_bridge_init_vga_aliases(br, parent, &w->vga);
>  
>      return w;
>  }
> @@ -187,6 +219,9 @@ static void pci_bridge_region_del(PCIBridge *br, PCIBridgeWindows *w)
>      memory_region_del_subregion(parent->address_space_io, &w->alias_io);
>      memory_region_del_subregion(parent->address_space_mem, &w->alias_mem);
>      memory_region_del_subregion(parent->address_space_mem, &w->alias_pref_mem);
> +    memory_region_del_subregion(parent->address_space_io, &w->vga.alias_io_lo);
> +    memory_region_del_subregion(parent->address_space_io, &w->vga.alias_io_hi);
> +    memory_region_del_subregion(parent->address_space_mem, &w->vga.alias_mem);
>  }
>  
>  static void pci_bridge_region_cleanup(PCIBridge *br, PCIBridgeWindows *w)
> @@ -194,6 +229,9 @@ static void pci_bridge_region_cleanup(PCIBridge *br, PCIBridgeWindows *w)
>      memory_region_destroy(&w->alias_io);
>      memory_region_destroy(&w->alias_mem);
>      memory_region_destroy(&w->alias_pref_mem);
> +    memory_region_destroy(&w->vga.alias_io_lo);
> +    memory_region_destroy(&w->vga.alias_io_hi);
> +    memory_region_destroy(&w->vga.alias_mem);
>      g_free(w);
>  }
>  
> @@ -227,7 +265,10 @@ void pci_bridge_write_config(PCIDevice *d,
>  
>          /* memory base/limit, prefetchable base/limit and
>             io base/limit upper 16 */
> -        ranges_overlap(address, len, PCI_MEMORY_BASE, 20)) {
> +        ranges_overlap(address, len, PCI_MEMORY_BASE, 20) ||
> +
> +        /* vga enable */
> +        ranges_overlap(address, len, PCI_BRIDGE_CONTROL, 1)) {
>          pci_bridge_update_mappings(s);
>      }
>  
> @@ -294,7 +335,7 @@ void pci_bridge_reset(DeviceState *qdev)
>      pci_set_long(conf + PCI_PREF_BASE_UPPER32, 0);
>      pci_set_long(conf + PCI_PREF_LIMIT_UPPER32, 0);
>  
> -    pci_set_word(conf + PCI_BRIDGE_CONTROL, 0);
> +    pci_set_byte(conf + PCI_BRIDGE_CONTROL, 0);

Hang on, you are leaving PCI_BRIDGE_CTL_VGA read-only?
Shouldn't it be writeable?
And then I'd say disable everything on reset and then
re-enable for the default bridge.

>  }
>  
>  /* default qdev initialization function for PCI-to-PCI bridge */
> diff --git a/hw/pci/pci_bus.h b/hw/pci/pci_bus.h
> index f905b9e..60d5378 100644
> --- a/hw/pci/pci_bus.h
> +++ b/hw/pci/pci_bus.h
> @@ -36,6 +36,20 @@ struct PCIBus {
>      int *irq_count;
>  };
>  
> +typedef struct PCIBridgeVgaWindows PCIBridgeVgaWindows;
> +
> +/*
> + * When bridge control VGA forwarding is enabled, bridges will provide
> + * positive decode on the PCI VGA defined I/O port and MMIO ranges noted
> + * below.  When set, forwarding is only qualified on the I/O and memory
> + * enable bits in the bridge command register.
> + */
> +struct PCIBridgeVgaWindows {
> +    MemoryRegion alias_io_lo; /* I/O ports 0x3b0 - 0x3bb */
> +    MemoryRegion alias_io_hi; /* I/O ports 0x3c0 - 0x3df */
> +    MemoryRegion alias_mem; /* MMIO 0xa0000 - 0xbffff */
> +};
> +
>  typedef struct PCIBridgeWindows PCIBridgeWindows;
>  
>  /*
> @@ -47,6 +61,7 @@ struct PCIBridgeWindows {
>      MemoryRegion alias_pref_mem;
>      MemoryRegion alias_mem;
>      MemoryRegion alias_io;
> +    PCIBridgeVgaWindows vga;
>  };
>  
>  struct PCIBridge {
Alex Williamson - Feb. 28, 2013, 8:34 p.m.
On Thu, 2013-02-28 at 22:26 +0200, Michael S. Tsirkin wrote:
> On Thu, Feb 28, 2013 at 12:00:03PM -0700, Alex Williamson wrote:
> > Each PCI Bridge has a set of implied VGA regions that are enabled
> > when the VGA bit is set in the bridge control register.  This allows
> > VGA devices behind bridges.
> > 
> > Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> > ---
> >  hw/pci/pci_bridge.c |   47 ++++++++++++++++++++++++++++++++++++++++++++---
> >  hw/pci/pci_bus.h    |   15 +++++++++++++++
> >  2 files changed, 59 insertions(+), 3 deletions(-)
> > 
> > diff --git a/hw/pci/pci_bridge.c b/hw/pci/pci_bridge.c
> > index 995842a..42cbfd1 100644
> > --- a/hw/pci/pci_bridge.c
> > +++ b/hw/pci/pci_bridge.c
> > @@ -151,6 +151,37 @@ static void pci_bridge_init_alias(PCIBridge *bridge, MemoryRegion *alias,
> >      memory_region_add_subregion_overlap(parent_space, base, alias, 1);
> >  }
> >  
> > +static void pci_bridge_init_vga_aliases(PCIBridge *br, PCIBus *parent,
> > +                                        PCIBridgeVgaWindows *vga)
> > +{
> > +    uint16_t cmd = pci_get_word(br->dev.config + PCI_COMMAND);
> > +    uint8_t brctl = pci_get_byte(br->dev.config + PCI_BRIDGE_CONTROL);
> > +
> > +    memory_region_init_alias(&vga->alias_io_lo, "pci_bridge_vga_io_lo",
> > +                             &br->address_space_io, 0x3b0, 0xc);
> > +    memory_region_add_subregion_overlap(parent->address_space_io, 0x3b0,
> > +                                        &vga->alias_io_lo, 1);
> > +
> > +    memory_region_init_alias(&vga->alias_io_hi, "pci_bridge_vga_io_hi",
> > +                             &br->address_space_io, 0x3c0, 0x20);
> > +    memory_region_add_subregion_overlap(parent->address_space_io, 0x3c0,
> > +                                        &vga->alias_io_hi, 1);
> > +
> > +    if (!(cmd & PCI_COMMAND_IO) || !(brctl & PCI_BRIDGE_CTL_VGA)) {
> > +        memory_region_set_enabled(&vga->alias_io_lo, false);
> > +        memory_region_set_enabled(&vga->alias_io_hi, false);
> > +    }
> > +
> > +    memory_region_init_alias(&vga->alias_mem, "pci_bridge_vga_mem",
> > +                             &br->address_space_mem, 0xa0000, 0x20000);
> > +    memory_region_add_subregion_overlap(parent->address_space_mem, 0xa0000,
> > +                                        &vga->alias_mem, 1);
> > +
> > +    if (!(cmd & PCI_COMMAND_MEMORY) || !(brctl & PCI_BRIDGE_CTL_VGA)) {
> > +        memory_region_set_enabled(&vga->alias_mem, false);
> > +    }
> > +}
> > +
> >  static PCIBridgeWindows *pci_bridge_region_init(PCIBridge *br)
> >  {
> >      PCIBus *parent = br->dev.bus;
> > @@ -175,7 +206,8 @@ static PCIBridgeWindows *pci_bridge_region_init(PCIBridge *br)
> >                            &br->address_space_io,
> >                            parent->address_space_io,
> >                            cmd & PCI_COMMAND_IO);
> > -   /* TODO: optinal VGA and VGA palette snooping support. */
> > +
> > +    pci_bridge_init_vga_aliases(br, parent, &w->vga);
> >  
> >      return w;
> >  }
> > @@ -187,6 +219,9 @@ static void pci_bridge_region_del(PCIBridge *br, PCIBridgeWindows *w)
> >      memory_region_del_subregion(parent->address_space_io, &w->alias_io);
> >      memory_region_del_subregion(parent->address_space_mem, &w->alias_mem);
> >      memory_region_del_subregion(parent->address_space_mem, &w->alias_pref_mem);
> > +    memory_region_del_subregion(parent->address_space_io, &w->vga.alias_io_lo);
> > +    memory_region_del_subregion(parent->address_space_io, &w->vga.alias_io_hi);
> > +    memory_region_del_subregion(parent->address_space_mem, &w->vga.alias_mem);
> >  }
> >  
> >  static void pci_bridge_region_cleanup(PCIBridge *br, PCIBridgeWindows *w)
> > @@ -194,6 +229,9 @@ static void pci_bridge_region_cleanup(PCIBridge *br, PCIBridgeWindows *w)
> >      memory_region_destroy(&w->alias_io);
> >      memory_region_destroy(&w->alias_mem);
> >      memory_region_destroy(&w->alias_pref_mem);
> > +    memory_region_destroy(&w->vga.alias_io_lo);
> > +    memory_region_destroy(&w->vga.alias_io_hi);
> > +    memory_region_destroy(&w->vga.alias_mem);
> >      g_free(w);
> >  }
> >  
> > @@ -227,7 +265,10 @@ void pci_bridge_write_config(PCIDevice *d,
> >  
> >          /* memory base/limit, prefetchable base/limit and
> >             io base/limit upper 16 */
> > -        ranges_overlap(address, len, PCI_MEMORY_BASE, 20)) {
> > +        ranges_overlap(address, len, PCI_MEMORY_BASE, 20) ||
> > +
> > +        /* vga enable */
> > +        ranges_overlap(address, len, PCI_BRIDGE_CONTROL, 1)) {
> >          pci_bridge_update_mappings(s);
> >      }
> >  
> > @@ -294,7 +335,7 @@ void pci_bridge_reset(DeviceState *qdev)
> >      pci_set_long(conf + PCI_PREF_BASE_UPPER32, 0);
> >      pci_set_long(conf + PCI_PREF_LIMIT_UPPER32, 0);
> >  
> > -    pci_set_word(conf + PCI_BRIDGE_CONTROL, 0);
> > +    pci_set_byte(conf + PCI_BRIDGE_CONTROL, 0);
> 
> Hang on, you are leaving PCI_BRIDGE_CTL_VGA read-only?
> Shouldn't it be writeable?

It's writable for me, I've got SeaBIOS enabling it.  Both
pci_init_mask_bridge and pcie_port_init_reg are marking it writable in
wmask.

> And then I'd say disable everything on reset and then
> re-enable for the default bridge.

The above chunk here is the disable on reset, we were just incorrectly
setting a word instead of a byte.  I just sent a patch to SeaBIOS that
will enable a path to the first VGA device if the platform doesn't
already provide it with one.  Thanks,

Alex

> >  }
> >  
> >  /* default qdev initialization function for PCI-to-PCI bridge */
> > diff --git a/hw/pci/pci_bus.h b/hw/pci/pci_bus.h
> > index f905b9e..60d5378 100644
> > --- a/hw/pci/pci_bus.h
> > +++ b/hw/pci/pci_bus.h
> > @@ -36,6 +36,20 @@ struct PCIBus {
> >      int *irq_count;
> >  };
> >  
> > +typedef struct PCIBridgeVgaWindows PCIBridgeVgaWindows;
> > +
> > +/*
> > + * When bridge control VGA forwarding is enabled, bridges will provide
> > + * positive decode on the PCI VGA defined I/O port and MMIO ranges noted
> > + * below.  When set, forwarding is only qualified on the I/O and memory
> > + * enable bits in the bridge command register.
> > + */
> > +struct PCIBridgeVgaWindows {
> > +    MemoryRegion alias_io_lo; /* I/O ports 0x3b0 - 0x3bb */
> > +    MemoryRegion alias_io_hi; /* I/O ports 0x3c0 - 0x3df */
> > +    MemoryRegion alias_mem; /* MMIO 0xa0000 - 0xbffff */
> > +};
> > +
> >  typedef struct PCIBridgeWindows PCIBridgeWindows;
> >  
> >  /*
> > @@ -47,6 +61,7 @@ struct PCIBridgeWindows {
> >      MemoryRegion alias_pref_mem;
> >      MemoryRegion alias_mem;
> >      MemoryRegion alias_io;
> > +    PCIBridgeVgaWindows vga;
> >  };
> >  
> >  struct PCIBridge {
Michael S. Tsirkin - Feb. 28, 2013, 9:48 p.m.
On Thu, Feb 28, 2013 at 01:34:39PM -0700, Alex Williamson wrote:
> On Thu, 2013-02-28 at 22:26 +0200, Michael S. Tsirkin wrote:
> > On Thu, Feb 28, 2013 at 12:00:03PM -0700, Alex Williamson wrote:
> > > Each PCI Bridge has a set of implied VGA regions that are enabled
> > > when the VGA bit is set in the bridge control register.  This allows
> > > VGA devices behind bridges.
> > > 
> > > Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> > > ---
> > >  hw/pci/pci_bridge.c |   47 ++++++++++++++++++++++++++++++++++++++++++++---
> > >  hw/pci/pci_bus.h    |   15 +++++++++++++++
> > >  2 files changed, 59 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/hw/pci/pci_bridge.c b/hw/pci/pci_bridge.c
> > > index 995842a..42cbfd1 100644
> > > --- a/hw/pci/pci_bridge.c
> > > +++ b/hw/pci/pci_bridge.c
> > > @@ -151,6 +151,37 @@ static void pci_bridge_init_alias(PCIBridge *bridge, MemoryRegion *alias,
> > >      memory_region_add_subregion_overlap(parent_space, base, alias, 1);
> > >  }
> > >  
> > > +static void pci_bridge_init_vga_aliases(PCIBridge *br, PCIBus *parent,
> > > +                                        PCIBridgeVgaWindows *vga)
> > > +{
> > > +    uint16_t cmd = pci_get_word(br->dev.config + PCI_COMMAND);
> > > +    uint8_t brctl = pci_get_byte(br->dev.config + PCI_BRIDGE_CONTROL);
> > > +
> > > +    memory_region_init_alias(&vga->alias_io_lo, "pci_bridge_vga_io_lo",
> > > +                             &br->address_space_io, 0x3b0, 0xc);
> > > +    memory_region_add_subregion_overlap(parent->address_space_io, 0x3b0,
> > > +                                        &vga->alias_io_lo, 1);
> > > +
> > > +    memory_region_init_alias(&vga->alias_io_hi, "pci_bridge_vga_io_hi",
> > > +                             &br->address_space_io, 0x3c0, 0x20);
> > > +    memory_region_add_subregion_overlap(parent->address_space_io, 0x3c0,
> > > +                                        &vga->alias_io_hi, 1);
> > > +
> > > +    if (!(cmd & PCI_COMMAND_IO) || !(brctl & PCI_BRIDGE_CTL_VGA)) {
> > > +        memory_region_set_enabled(&vga->alias_io_lo, false);
> > > +        memory_region_set_enabled(&vga->alias_io_hi, false);
> > > +    }
> > > +
> > > +    memory_region_init_alias(&vga->alias_mem, "pci_bridge_vga_mem",
> > > +                             &br->address_space_mem, 0xa0000, 0x20000);
> > > +    memory_region_add_subregion_overlap(parent->address_space_mem, 0xa0000,
> > > +                                        &vga->alias_mem, 1);
> > > +
> > > +    if (!(cmd & PCI_COMMAND_MEMORY) || !(brctl & PCI_BRIDGE_CTL_VGA)) {
> > > +        memory_region_set_enabled(&vga->alias_mem, false);
> > > +    }
> > > +}
> > > +
> > >  static PCIBridgeWindows *pci_bridge_region_init(PCIBridge *br)
> > >  {
> > >      PCIBus *parent = br->dev.bus;
> > > @@ -175,7 +206,8 @@ static PCIBridgeWindows *pci_bridge_region_init(PCIBridge *br)
> > >                            &br->address_space_io,
> > >                            parent->address_space_io,
> > >                            cmd & PCI_COMMAND_IO);
> > > -   /* TODO: optinal VGA and VGA palette snooping support. */
> > > +
> > > +    pci_bridge_init_vga_aliases(br, parent, &w->vga);
> > >  
> > >      return w;
> > >  }
> > > @@ -187,6 +219,9 @@ static void pci_bridge_region_del(PCIBridge *br, PCIBridgeWindows *w)
> > >      memory_region_del_subregion(parent->address_space_io, &w->alias_io);
> > >      memory_region_del_subregion(parent->address_space_mem, &w->alias_mem);
> > >      memory_region_del_subregion(parent->address_space_mem, &w->alias_pref_mem);
> > > +    memory_region_del_subregion(parent->address_space_io, &w->vga.alias_io_lo);
> > > +    memory_region_del_subregion(parent->address_space_io, &w->vga.alias_io_hi);
> > > +    memory_region_del_subregion(parent->address_space_mem, &w->vga.alias_mem);
> > >  }
> > >  
> > >  static void pci_bridge_region_cleanup(PCIBridge *br, PCIBridgeWindows *w)
> > > @@ -194,6 +229,9 @@ static void pci_bridge_region_cleanup(PCIBridge *br, PCIBridgeWindows *w)
> > >      memory_region_destroy(&w->alias_io);
> > >      memory_region_destroy(&w->alias_mem);
> > >      memory_region_destroy(&w->alias_pref_mem);
> > > +    memory_region_destroy(&w->vga.alias_io_lo);
> > > +    memory_region_destroy(&w->vga.alias_io_hi);
> > > +    memory_region_destroy(&w->vga.alias_mem);
> > >      g_free(w);
> > >  }
> > >  
> > > @@ -227,7 +265,10 @@ void pci_bridge_write_config(PCIDevice *d,
> > >  
> > >          /* memory base/limit, prefetchable base/limit and
> > >             io base/limit upper 16 */
> > > -        ranges_overlap(address, len, PCI_MEMORY_BASE, 20)) {
> > > +        ranges_overlap(address, len, PCI_MEMORY_BASE, 20) ||
> > > +
> > > +        /* vga enable */
> > > +        ranges_overlap(address, len, PCI_BRIDGE_CONTROL, 1)) {
> > >          pci_bridge_update_mappings(s);
> > >      }
> > >  
> > > @@ -294,7 +335,7 @@ void pci_bridge_reset(DeviceState *qdev)
> > >      pci_set_long(conf + PCI_PREF_BASE_UPPER32, 0);
> > >      pci_set_long(conf + PCI_PREF_LIMIT_UPPER32, 0);
> > >  
> > > -    pci_set_word(conf + PCI_BRIDGE_CONTROL, 0);
> > > +    pci_set_byte(conf + PCI_BRIDGE_CONTROL, 0);
> > 
> > Hang on, you are leaving PCI_BRIDGE_CTL_VGA read-only?
> > Shouldn't it be writeable?
> 
> It's writable for me, I've got SeaBIOS enabling it.  Both
> pci_init_mask_bridge and pcie_port_init_reg are marking it writable in
> wmask.
> 
> > And then I'd say disable everything on reset and then
> > re-enable for the default bridge.
> 
> The above chunk here is the disable on reset, we were just incorrectly
> setting a word instead of a byte.

Just re-checked the spec, bridge-control register is a 16 bit one.
In any case I'd like such unrelated fixes be noted in the commit log
please.

> I just sent a patch to SeaBIOS that
> will enable a path to the first VGA device if the platform doesn't
> already provide it with one.  Thanks,
> 
> Alex
> 
> > >  }
> > >  
> > >  /* default qdev initialization function for PCI-to-PCI bridge */
> > > diff --git a/hw/pci/pci_bus.h b/hw/pci/pci_bus.h
> > > index f905b9e..60d5378 100644
> > > --- a/hw/pci/pci_bus.h
> > > +++ b/hw/pci/pci_bus.h
> > > @@ -36,6 +36,20 @@ struct PCIBus {
> > >      int *irq_count;
> > >  };
> > >  
> > > +typedef struct PCIBridgeVgaWindows PCIBridgeVgaWindows;
> > > +
> > > +/*
> > > + * When bridge control VGA forwarding is enabled, bridges will provide
> > > + * positive decode on the PCI VGA defined I/O port and MMIO ranges noted
> > > + * below.  When set, forwarding is only qualified on the I/O and memory
> > > + * enable bits in the bridge command register.
> > > + */
> > > +struct PCIBridgeVgaWindows {
> > > +    MemoryRegion alias_io_lo; /* I/O ports 0x3b0 - 0x3bb */
> > > +    MemoryRegion alias_io_hi; /* I/O ports 0x3c0 - 0x3df */
> > > +    MemoryRegion alias_mem; /* MMIO 0xa0000 - 0xbffff */
> > > +};
> > > +
> > >  typedef struct PCIBridgeWindows PCIBridgeWindows;
> > >  
> > >  /*
> > > @@ -47,6 +61,7 @@ struct PCIBridgeWindows {
> > >      MemoryRegion alias_pref_mem;
> > >      MemoryRegion alias_mem;
> > >      MemoryRegion alias_io;
> > > +    PCIBridgeVgaWindows vga;
> > >  };
> > >  
> > >  struct PCIBridge {
> 
>

Patch

diff --git a/hw/pci/pci_bridge.c b/hw/pci/pci_bridge.c
index 995842a..42cbfd1 100644
--- a/hw/pci/pci_bridge.c
+++ b/hw/pci/pci_bridge.c
@@ -151,6 +151,37 @@  static void pci_bridge_init_alias(PCIBridge *bridge, MemoryRegion *alias,
     memory_region_add_subregion_overlap(parent_space, base, alias, 1);
 }
 
+static void pci_bridge_init_vga_aliases(PCIBridge *br, PCIBus *parent,
+                                        PCIBridgeVgaWindows *vga)
+{
+    uint16_t cmd = pci_get_word(br->dev.config + PCI_COMMAND);
+    uint8_t brctl = pci_get_byte(br->dev.config + PCI_BRIDGE_CONTROL);
+
+    memory_region_init_alias(&vga->alias_io_lo, "pci_bridge_vga_io_lo",
+                             &br->address_space_io, 0x3b0, 0xc);
+    memory_region_add_subregion_overlap(parent->address_space_io, 0x3b0,
+                                        &vga->alias_io_lo, 1);
+
+    memory_region_init_alias(&vga->alias_io_hi, "pci_bridge_vga_io_hi",
+                             &br->address_space_io, 0x3c0, 0x20);
+    memory_region_add_subregion_overlap(parent->address_space_io, 0x3c0,
+                                        &vga->alias_io_hi, 1);
+
+    if (!(cmd & PCI_COMMAND_IO) || !(brctl & PCI_BRIDGE_CTL_VGA)) {
+        memory_region_set_enabled(&vga->alias_io_lo, false);
+        memory_region_set_enabled(&vga->alias_io_hi, false);
+    }
+
+    memory_region_init_alias(&vga->alias_mem, "pci_bridge_vga_mem",
+                             &br->address_space_mem, 0xa0000, 0x20000);
+    memory_region_add_subregion_overlap(parent->address_space_mem, 0xa0000,
+                                        &vga->alias_mem, 1);
+
+    if (!(cmd & PCI_COMMAND_MEMORY) || !(brctl & PCI_BRIDGE_CTL_VGA)) {
+        memory_region_set_enabled(&vga->alias_mem, false);
+    }
+}
+
 static PCIBridgeWindows *pci_bridge_region_init(PCIBridge *br)
 {
     PCIBus *parent = br->dev.bus;
@@ -175,7 +206,8 @@  static PCIBridgeWindows *pci_bridge_region_init(PCIBridge *br)
                           &br->address_space_io,
                           parent->address_space_io,
                           cmd & PCI_COMMAND_IO);
-   /* TODO: optinal VGA and VGA palette snooping support. */
+
+    pci_bridge_init_vga_aliases(br, parent, &w->vga);
 
     return w;
 }
@@ -187,6 +219,9 @@  static void pci_bridge_region_del(PCIBridge *br, PCIBridgeWindows *w)
     memory_region_del_subregion(parent->address_space_io, &w->alias_io);
     memory_region_del_subregion(parent->address_space_mem, &w->alias_mem);
     memory_region_del_subregion(parent->address_space_mem, &w->alias_pref_mem);
+    memory_region_del_subregion(parent->address_space_io, &w->vga.alias_io_lo);
+    memory_region_del_subregion(parent->address_space_io, &w->vga.alias_io_hi);
+    memory_region_del_subregion(parent->address_space_mem, &w->vga.alias_mem);
 }
 
 static void pci_bridge_region_cleanup(PCIBridge *br, PCIBridgeWindows *w)
@@ -194,6 +229,9 @@  static void pci_bridge_region_cleanup(PCIBridge *br, PCIBridgeWindows *w)
     memory_region_destroy(&w->alias_io);
     memory_region_destroy(&w->alias_mem);
     memory_region_destroy(&w->alias_pref_mem);
+    memory_region_destroy(&w->vga.alias_io_lo);
+    memory_region_destroy(&w->vga.alias_io_hi);
+    memory_region_destroy(&w->vga.alias_mem);
     g_free(w);
 }
 
@@ -227,7 +265,10 @@  void pci_bridge_write_config(PCIDevice *d,
 
         /* memory base/limit, prefetchable base/limit and
            io base/limit upper 16 */
-        ranges_overlap(address, len, PCI_MEMORY_BASE, 20)) {
+        ranges_overlap(address, len, PCI_MEMORY_BASE, 20) ||
+
+        /* vga enable */
+        ranges_overlap(address, len, PCI_BRIDGE_CONTROL, 1)) {
         pci_bridge_update_mappings(s);
     }
 
@@ -294,7 +335,7 @@  void pci_bridge_reset(DeviceState *qdev)
     pci_set_long(conf + PCI_PREF_BASE_UPPER32, 0);
     pci_set_long(conf + PCI_PREF_LIMIT_UPPER32, 0);
 
-    pci_set_word(conf + PCI_BRIDGE_CONTROL, 0);
+    pci_set_byte(conf + PCI_BRIDGE_CONTROL, 0);
 }
 
 /* default qdev initialization function for PCI-to-PCI bridge */
diff --git a/hw/pci/pci_bus.h b/hw/pci/pci_bus.h
index f905b9e..60d5378 100644
--- a/hw/pci/pci_bus.h
+++ b/hw/pci/pci_bus.h
@@ -36,6 +36,20 @@  struct PCIBus {
     int *irq_count;
 };
 
+typedef struct PCIBridgeVgaWindows PCIBridgeVgaWindows;
+
+/*
+ * When bridge control VGA forwarding is enabled, bridges will provide
+ * positive decode on the PCI VGA defined I/O port and MMIO ranges noted
+ * below.  When set, forwarding is only qualified on the I/O and memory
+ * enable bits in the bridge command register.
+ */
+struct PCIBridgeVgaWindows {
+    MemoryRegion alias_io_lo; /* I/O ports 0x3b0 - 0x3bb */
+    MemoryRegion alias_io_hi; /* I/O ports 0x3c0 - 0x3df */
+    MemoryRegion alias_mem; /* MMIO 0xa0000 - 0xbffff */
+};
+
 typedef struct PCIBridgeWindows PCIBridgeWindows;
 
 /*
@@ -47,6 +61,7 @@  struct PCIBridgeWindows {
     MemoryRegion alias_pref_mem;
     MemoryRegion alias_mem;
     MemoryRegion alias_io;
+    PCIBridgeVgaWindows vga;
 };
 
 struct PCIBridge {