[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] pci: Teach PCI Bridges about VGA routing
From: |
Alex Williamson |
Subject: |
Re: [Qemu-devel] [PATCH] pci: Teach PCI Bridges about VGA routing |
Date: |
Thu, 28 Feb 2013 13:34:39 -0700 |
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 <address@hidden>
> > ---
> > 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 {