| Message ID | f9b630bb6c67f0facf29e42277ad7d991e660b3c.1329124507.git.mst@redhat.com |
|---|---|
| State | New |
| Headers | show |
Hi, > + /* If we don't specify the name, the bus will be addressed as <id>.0, where > + * id is the parent id. But it seems more natural to address the bus using > + * the parent device name. */ > + if (dev->qdev.id && *dev->qdev.id) { > + br->bus_name = dev->qdev.id; > + } That makes the bridge behave different than everybody else. Not a good idea IMHO. cheers, Gerd
On 02/13/12 10:16, Michael S. Tsirkin wrote: > This adds support for a standard pci to pci bridge, > enabling support for more than 32 PCI devices in the system. > Device hotplug is supported by means of SHPC controller. > For guests with an SHPC driver, this allows robust hotplug > and even hotplug of nested bridges, up to 31 devices > per bridge. This seems to not support 64bit prefetchable memory windows, at least linux doesn't think it does, lspci looks like this: 00:10.0 PCI bridge: Red Hat, Inc. Device 0001 (prog-if 00 [Normal decode]) Physical Slot: 16 Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx- Status: Cap+ 66MHz+ UDF- FastB2B+ ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx- Latency: 0 Region 0: Memory at f6126000 (32-bit, non-prefetchable) [size=256] Bus: primary=00, secondary=01, subordinate=01, sec-latency=0 Memory behind bridge: f6000000-f60fffff Prefetchable memory behind bridge: f8000000-fbffffff Secondary status: 66MHz+ FastB2B+ ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- <SERR- <PERR- BridgeCtl: Parity- SERR- NoISA- VGA- MAbort- >Reset- FastB2B- PriDiscTmr- SecDiscTmr- DiscTmrStat- DiscTmrSERREn- Capabilities: [40] Hot-plug capable Kernel modules: shpchp Intentional? cheers, Gerd
On Fri, Feb 17, 2012 at 02:25:56PM +0100, Gerd Hoffmann wrote: > Hi, > > > + /* If we don't specify the name, the bus will be addressed as <id>.0, where > > + * id is the parent id. But it seems more natural to address the bus using > > + * the parent device name. */ > > + if (dev->qdev.id && *dev->qdev.id) { > > + br->bus_name = dev->qdev.id; > > + } > > That makes the bridge behave different than everybody else. > Not a good idea IMHO. > > cheers, > Gerd Everybody else has names built up according to an undocumented scheme which no one can figure out without reading code, so no one uses them. We need to fix that, but there is, generally, no need for these names so it stayed low priority. With the bridge people must use the id to connect devices to it, so name must be a sane one.
On Sun, Feb 19, 2012 at 04:57:07PM +0200, Michael S. Tsirkin wrote: > On Fri, Feb 17, 2012 at 02:25:56PM +0100, Gerd Hoffmann wrote: > > Hi, > > > > > + /* If we don't specify the name, the bus will be addressed as <id>.0, where > > > + * id is the parent id. But it seems more natural to address the bus using > > > + * the parent device name. */ > > > + if (dev->qdev.id && *dev->qdev.id) { > > > + br->bus_name = dev->qdev.id; > > > + } > > > > That makes the bridge behave different than everybody else. > > Not a good idea IMHO. > > > > cheers, > > Gerd > > Everybody else has names built up according to an undocumented scheme > which no one can figure out without reading code, so no one uses them. > We need to fix that, but there is, generally, no need for these names > so it stayed low priority. > > With the bridge people must use the id to connect devices to it, > so name must be a sane one. I just sent the patch making bus id for bridges follow the value set by the user. That will make the bridge behave in the same way as everybody else :) > -- > MST
On Fri, Feb 17, 2012 at 02:33:42PM +0100, Gerd Hoffmann wrote: > On 02/13/12 10:16, Michael S. Tsirkin wrote: > > This adds support for a standard pci to pci bridge, > > enabling support for more than 32 PCI devices in the system. > > Device hotplug is supported by means of SHPC controller. > > For guests with an SHPC driver, this allows robust hotplug > > and even hotplug of nested bridges, up to 31 devices > > per bridge. > > This seems to not support 64bit prefetchable memory windows, at least > linux doesn't think it does, lspci looks like this: > > 00:10.0 PCI bridge: Red Hat, Inc. Device 0001 (prog-if 00 [Normal decode]) > Physical Slot: 16 > Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- > ParErr- Stepping- SERR- FastB2B- DisINTx- > Status: Cap+ 66MHz+ UDF- FastB2B+ ParErr- DEVSEL=fast >TAbort- > <TAbort- <MAbort- >SERR- <PERR- INTx- > Latency: 0 > Region 0: Memory at f6126000 (32-bit, non-prefetchable) [size=256] > Bus: primary=00, secondary=01, subordinate=01, sec-latency=0 > Memory behind bridge: f6000000-f60fffff > Prefetchable memory behind bridge: f8000000-fbffffff > Secondary status: 66MHz+ FastB2B+ ParErr- DEVSEL=fast >TAbort- > <TAbort- <MAbort- <SERR- <PERR- > BridgeCtl: Parity- SERR- NoISA- VGA- MAbort- >Reset- FastB2B- > PriDiscTmr- SecDiscTmr- DiscTmrStat- DiscTmrSERREn- > Capabilities: [40] Hot-plug capable > Kernel modules: shpchp > > Intentional? > > cheers, > Gerd I'll need to check. v3 I am sending out has this code unchanged. What in the above tells you that 64 bit windows are not supported? BAR0 is 32 bit non prefetcheable. As far as I can see linux driver takes no precautions against access combining that can happen with prefetcheable BARs, so non-prefetcheable seems safer. I could make it 64 bit I guess, I just heard that there is some known bug being worked around in memory region code triggered somehow by 64 bit, and waiting for the dust to settle.
Hi, >> This seems to not support 64bit prefetchable memory windows, at least >> linux doesn't think it does, lspci looks like this: >> >> 00:10.0 PCI bridge: Red Hat, Inc. Device 0001 (prog-if 00 [Normal decode]) [ ... ] >> Memory behind bridge: f6000000-f60fffff >> Prefetchable memory behind bridge: f8000000-fbffffff > What in the above tells you that 64 bit windows are not supported? lspci prints 64bit addresses then, like this: 00:1c.0 PCI bridge: Intel Corporation 82801I (ICH9 Family) PCI Express Port 1 (rev 03) (prog-if 00 [Normal decode]) [ ... ] I/O behind bridge: 00008000-00008fff Memory behind bridge: c0000000-c01fffff Prefetchable memory behind bridge: 00000000c0200000-00000000c03fffff > BAR0 is 32 bit non prefetcheable. As far as I can see linux driver > takes no precautions against access combining that can > happen with prefetcheable BARs, so non-prefetcheable > seems safer. I'm not talking about bar #0, but about the prefetchable memory window for devices behind the bridge. cheers, Gerd
diff --git a/Makefile.objs b/Makefile.objs index 4546477..e89112c 100644 --- a/Makefile.objs +++ b/Makefile.objs @@ -193,7 +193,7 @@ hw-obj-$(CONFIG_VIRTIO) += virtio-console.o hw-obj-y += usb-libhw.o hw-obj-$(CONFIG_VIRTIO_PCI) += virtio-pci.o hw-obj-y += fw_cfg.o -hw-obj-$(CONFIG_PCI) += pci.o pci_bridge.o +hw-obj-$(CONFIG_PCI) += pci.o pci_bridge.o pci_bridge_dev.o hw-obj-$(CONFIG_PCI) += msix.o msi.o hw-obj-$(CONFIG_PCI) += shpc.o hw-obj-$(CONFIG_PCI) += pci_host.o pcie_host.o diff --git a/hw/pci_bridge_dev.c b/hw/pci_bridge_dev.c new file mode 100644 index 0000000..f48cd2d --- /dev/null +++ b/hw/pci_bridge_dev.c @@ -0,0 +1,136 @@ +/* + * Standard PCI Bridge Device + * + * Copyright (c) 2011 Red Hat Inc. Author: Michael S. Tsirkin <mst@redhat.com> + * + * http://www.pcisig.com/specifications/conventional/pci_to_pci_bridge_architecture/ + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, see <http://www.gnu.org/licenses/>. + */ + +#include "pci_bridge.h" +#include "pci_ids.h" +#include "shpc.h" +#include "memory.h" +#include "pci_internals.h" + +#define REDHAT_PCI_VENDOR_ID 0x1b36 +#define PCI_BRIDGE_DEV_VENDOR_ID REDHAT_PCI_VENDOR_ID +#define PCI_BRIDGE_DEV_DEVICE_ID 0x1 + +struct PCIBridgeDev { + PCIBridge bridge; + MemoryRegion bar; +}; +typedef struct PCIBridgeDev PCIBridgeDev; + +/* Mapping mandated by PCI-to-PCI Bridge architecture specification, + * revision 1.2 */ +/* Table 9-1: Interrupt Binding for Devices Behind a Bridge */ +static int pci_bridge_dev_map_irq_fn(PCIDevice *dev, int irq_num) +{ + return (irq_num + PCI_SLOT(dev->devfn)) % PCI_NUM_PINS; +} + +static int pci_bridge_dev_initfn(PCIDevice *dev) +{ + PCIBridge *br = DO_UPCAST(PCIBridge, dev, dev); + PCIBridgeDev *bridge_dev = DO_UPCAST(PCIBridgeDev, bridge, br); + int err; + br->map_irq = pci_bridge_dev_map_irq_fn; + /* If we don't specify the name, the bus will be addressed as <id>.0, where + * id is the parent id. But it seems more natural to address the bus using + * the parent device name. */ + if (dev->qdev.id && *dev->qdev.id) { + br->bus_name = dev->qdev.id; + } + err = pci_bridge_initfn(dev); + if (err) { + goto bridge_error; + } + memory_region_init(&bridge_dev->bar, "shpc-bar", shpc_bar_size(dev)); + err = shpc_init(dev, &br->sec_bus, &bridge_dev->bar, 0); + if (err) { + goto error; + } + /* TODO: spec recommends using 64 bit prefetcheable BAR. + * Check whether that works well. */ + pci_register_bar(dev, 0, PCI_BASE_ADDRESS_SPACE_MEMORY, &bridge_dev->bar); + dev->config[PCI_INTERRUPT_PIN] = 0x1; + return 0; +error: + memory_region_destroy(&bridge_dev->bar); +bridge_error: + return err; +} + +static int pci_bridge_dev_exitfn(PCIDevice *dev) +{ + PCIBridge *br = DO_UPCAST(PCIBridge, dev, dev); + PCIBridgeDev *bridge_dev = DO_UPCAST(PCIBridgeDev, bridge, br); + int ret; + shpc_cleanup(dev); + memory_region_destroy(&bridge_dev->bar); + ret = pci_bridge_exitfn(dev); + assert(!ret); + return 0; +} + +static void pci_bridge_dev_write_config(PCIDevice *d, + uint32_t address, uint32_t val, int len) +{ + pci_bridge_write_config(d, address, val, len); + shpc_cap_write_config(d, address, val, len); +} + +static void qdev_pci_bridge_dev_reset(DeviceState *qdev) +{ + PCIDevice *dev = DO_UPCAST(PCIDevice, qdev, qdev); + pci_bridge_reset(qdev); + shpc_reset(dev); +} + +static void pci_bridge_dev_class_init(ObjectClass *klass, void *data) +{ + DeviceClass *dc = DEVICE_CLASS(klass); + PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); + k->init = pci_bridge_dev_initfn; + k->exit = pci_bridge_dev_exitfn; + k->config_write = pci_bridge_dev_write_config; + k->vendor_id = PCI_BRIDGE_DEV_VENDOR_ID; + k->device_id = PCI_BRIDGE_DEV_DEVICE_ID; + k->class_id = PCI_CLASS_BRIDGE_PCI; + k->is_bridge = 1, + dc->desc = "Standard PCI Bridge"; + dc->reset = qdev_pci_bridge_dev_reset; + /* + * TODO: migration. + * dc->vmsd = + * dc->props = + */ +} + +static TypeInfo pci_bridge_dev_info = { + .name = "pci-bridge", + .parent = TYPE_PCI_DEVICE, + .instance_size = sizeof(PCIBridgeDev), + .class_init = pci_bridge_dev_class_init, +}; + +static void pci_bridge_dev_register(void) +{ + type_register_static(&pci_bridge_dev_info); +} + +device_init(pci_bridge_dev_register);
This adds support for a standard pci to pci bridge, enabling support for more than 32 PCI devices in the system. Device hotplug is supported by means of SHPC controller. For guests with an SHPC driver, this allows robust hotplug and even hotplug of nested bridges, up to 31 devices per bridge. TODO: - chassis capability support - migration support - remove dependency on pci_internals.h Signed-off-by: Michael S. Tsirkin <mst@redhat.com> --- Makefile.objs | 2 +- hw/pci_bridge_dev.c | 136 +++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 137 insertions(+), 1 deletions(-) create mode 100644 hw/pci_bridge_dev.c