From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48800) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Ymi4T-0004Fi-EI for qemu-devel@nongnu.org; Mon, 27 Apr 2015 08:23:35 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Ymi4O-00036B-Ct for qemu-devel@nongnu.org; Mon, 27 Apr 2015 08:23:33 -0400 Received: from mx1.redhat.com ([209.132.183.28]:54515) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Ymi4O-00035v-3Z for qemu-devel@nongnu.org; Mon, 27 Apr 2015 08:23:28 -0400 Date: Mon, 27 Apr 2015 14:23:21 +0200 From: "Michael S. Tsirkin" Message-ID: <20150427141437-mutt-send-email-mst@redhat.com> References: <1426791181-23831-1-git-send-email-marcel@redhat.com> <1426791181-23831-15-git-send-email-marcel@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1426791181-23831-15-git-send-email-marcel@redhat.com> Subject: Re: [Qemu-devel] [PATCH V6 for-2.3 14/26] hw/pci-host/piix: implement TYPE_PCI_HOST_BRIDGE_SNOOPED interface List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Marcel Apfelbaum Cc: kraxel@redhat.com, quintela@redhat.com, qemu-devel@nongnu.org, agraf@suse.de, alex.williamson@redhat.com, kevin@koconnor.net, hare@suse.de, imammedo@redhat.com, amit.shah@redhat.com, pbonzini@redhat.com, leon.alrae@imgtec.com, aurelien@aurel32.net, rth@twiddle.net On Thu, Mar 19, 2015 at 08:52:49PM +0200, Marcel Apfelbaum wrote: > The PIIX holds a list of snooping host bridges that is populated > by a registration function that shall be used by any host bridge > that needs to use PIIX configuration space to complete their > configuration cycles. > > PIIX monitors acceses to configuration registers and passes them > to the corresponding host bridge if the input bus number is > in its bus range. > > Signed-off-by: Marcel Apfelbaum > --- > hw/pci-host/piix.c | 118 +++++++++++++++++++++++++++++++++++++++++++++- > hw/pci/pci_host.c | 28 +++++++++++ > include/hw/i386/pc.h | 6 +++ > include/hw/pci/pci_host.h | 3 ++ > 4 files changed, 154 insertions(+), 1 deletion(-) > > diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c > index 0033ab4..f512765 100644 > --- a/hw/pci-host/piix.c > +++ b/hw/pci-host/piix.c > @@ -50,6 +50,12 @@ typedef struct I440FXState { > PcPciInfo pci_info; > uint64_t pci_hole64_size; > uint32_t short_root_bus; > + > + /* > + * List of PCI host bridges listening to > + * the same configuration registers. > + */ > + GPtrArray *snooping_hosts; > } I440FXState; > > #define PIIX_NUM_PIC_IRQS 16 /* i8259 * 2 */ > @@ -255,14 +261,80 @@ static void i440fx_pcihost_get_pci_hole64_end(Object *obj, Visitor *v, > visit_type_uint64(v, &w64.end, name, errp); > } > > +static PCIBus *i440fx_find_target_bus(I440FXState *i440fx, uint8_t bus_num) > +{ > + PCIHostState *host = PCI_HOST_BRIDGE(i440fx); > + PCIBus *target_bus = NULL; > + > + if (bus_num == pci_bus_num(host->bus)) { > + target_bus = host->bus; > + } else { > + int i; > + > + for (i = 0; i < i440fx->snooping_hosts->len; i++) { > + PCIHostState *s_host = g_ptr_array_index(i440fx->snooping_hosts, i); > + Range r = pci_host_get_buses_range(s_host); > + > + if (r.begin <= bus_num && r.end >= bus_num) { > + target_bus = s_host->bus; > + } > + extra empty line > + } > + } > + > + return target_bus; > +} > + > +static void i440fx_pcihost_data_write(void *opaque, hwaddr addr, > + uint64_t val, unsigned len) > +{ > + uint32_t config_reg = PCI_HOST_BRIDGE(opaque)->config_reg; > + > + if (config_reg & (1u << 31)) { > + uint8_t bus_num = (config_reg >> 16) & 0xFF; > + I440FXState *i440fx = I440FX_PCI_HOST_BRIDGE(opaque); > + PCIBus *target_bus = i440fx_find_target_bus(i440fx, bus_num); > + > + if (target_bus) { > + pci_data_write(target_bus, config_reg | (addr & 3), val, len); > + } > + } > +} > + > +static uint64_t i440fx_pcihost_data_read(void *opaque, > + hwaddr addr, unsigned len) > +{ > + uint32_t config_reg = PCI_HOST_BRIDGE(opaque)->config_reg; > + > + if (config_reg & (1U << 31)) { > + int bus_num = (config_reg >> 16) & 0xFF; > + I440FXState *i440fx = I440FX_PCI_HOST_BRIDGE(opaque); > + PCIBus *target_bus = i440fx_find_target_bus(i440fx, bus_num); > + > + if (target_bus) { > + return pci_data_read(target_bus, config_reg | (addr & 3), len); > + } > + } > + > + return 0xffffffff; > +} > + > +const MemoryRegionOps i440fx_pcihost_data_le_ops = { > + .read = i440fx_pcihost_data_read, > + .write = i440fx_pcihost_data_write, > + .endianness = DEVICE_LITTLE_ENDIAN, > +}; > + Can't you just add pxb as a child bus? If you do, pci_find_bus_nr will do the right thing more or less automatically, no? > static void i440fx_pcihost_initfn(Object *obj) > { > PCIHostState *s = PCI_HOST_BRIDGE(obj); > I440FXState *d = I440FX_PCI_HOST_BRIDGE(obj); > > + d->snooping_hosts = g_ptr_array_new(); > + > memory_region_init_io(&s->conf_mem, obj, &pci_host_conf_le_ops, s, > "pci-conf-idx", 4); > - memory_region_init_io(&s->data_mem, obj, &pci_host_data_le_ops, s, > + memory_region_init_io(&s->data_mem, obj, &i440fx_pcihost_data_le_ops, s, > "pci-conf-data", 4); > > object_property_add(obj, PCI_HOST_PROP_PCI_HOLE_START, "int", > @@ -400,6 +472,40 @@ PCIBus *find_i440fx(void) > return s ? s->bus : NULL; > } > > +/* > + * Returns 0 on successs, -1 if i440fx host was not > + * found or the bus number is already in use. > + */ > +int i440fx_register_snooping_host(PCIHostState *host) > +{ > + I440FXState *s = OBJECT_CHECK(I440FXState, > + object_resolve_path("/machine/i440fx", NULL), > + TYPE_I440FX_PCI_HOST_BRIDGE); > + uint8_t bus_num = pci_bus_num(host->bus); > + int i; > + > + /* i440fx host not found */ > + if (!s) { > + return -1; > + } > + > + /* 0 is reserved for i440fx host bridge */ > + if (bus_num == 0) { > + return -1; > + } > + > + /* check if the bus number is unique */ > + for (i = 0; i < s->snooping_hosts->len; i++) { > + PCIHostState *s_host = g_ptr_array_index(s->snooping_hosts, i); > + if (bus_num == pci_bus_num(s_host->bus)) { > + return -1; > + } > + } > + > + g_ptr_array_add(s->snooping_hosts, host); > + return 0; > +} > + > /* PIIX3 PCI to ISA bridge */ > static void piix3_set_irq_pic(PIIX3State *piix3, int pic_irq) > { > @@ -742,6 +848,13 @@ static const char *i440fx_pcihost_root_bus_path(PCIHostState *host_bridge, > return "0000:00"; > } > > +static GPtrArray *i440fx_snooping_hosts(PCIHostState *host_bridge) > +{ > + I440FXState *s = I440FX_PCI_HOST_BRIDGE(host_bridge); > + > + return s->snooping_hosts; > +} > + > static Property i440fx_props[] = { > DEFINE_PROP_SIZE(PCI_HOST_PROP_PCI_HOLE64_SIZE, I440FXState, > pci_hole64_size, DEFAULT_PCI_HOLE64_SIZE), > @@ -753,11 +866,13 @@ static void i440fx_pcihost_class_init(ObjectClass *klass, void *data) > { > DeviceClass *dc = DEVICE_CLASS(klass); > PCIHostBridgeClass *hc = PCI_HOST_BRIDGE_CLASS(klass); > + PCIHostBridgeSnoopedClass *sc = TYPE_PCI_HOST_BRIDGE_SNOOPED_CLASS(klass); > > hc->root_bus_path = i440fx_pcihost_root_bus_path; > dc->realize = i440fx_pcihost_realize; > dc->fw_name = "pci"; > dc->props = i440fx_props; > + sc->snooping_hosts = i440fx_snooping_hosts; > } > > static const TypeInfo i440fx_pcihost_info = { > @@ -768,6 +883,7 @@ static const TypeInfo i440fx_pcihost_info = { > .class_init = i440fx_pcihost_class_init, > .interfaces = (InterfaceInfo[]) { > { TYPE_PCI_MAIN_HOST_BRIDGE }, > + { TYPE_PCI_HOST_BRIDGE_SNOOPED }, > { } > } > > diff --git a/hw/pci/pci_host.c b/hw/pci/pci_host.c > index 288e74c..35e462b 100644 > --- a/hw/pci/pci_host.c > +++ b/hw/pci/pci_host.c > @@ -20,6 +20,8 @@ > > #include "hw/pci/pci.h" > #include "hw/pci/pci_host.h" > +#include "hw/pci/pci_bus.h" > +#include "qemu/range.h" > #include "trace.h" > > /* debug PCI */ > @@ -32,6 +34,32 @@ do { printf("pci_host_data: " fmt , ## __VA_ARGS__); } while (0) > #define PCI_DPRINTF(fmt, ...) > #endif > > +Range pci_host_get_buses_range(PCIHostState *host) new api, needs documentation. > +{ > + uint8_t root_bus_num = pci_bus_num(host->bus); > + Range r = {root_bus_num, root_bus_num}; > + int i; > + > + for (i = 0; i < ARRAY_SIZE(host->bus->devices); ++i) { > + PCIDevice *dev = host->bus->devices[i]; > + PCIDeviceClass *pc; > + > + if (!dev) { > + continue; > + } > + > + pc = PCI_DEVICE_GET_CLASS(dev); > + > + if (pc->is_bridge) { > + uint8_t subordinate = dev->config[PCI_SUBORDINATE_BUS]; > + if (subordinate > r.end) { > + r.end = subordinate; > + } > + } > + } > + > + return r; > +} > /* > * PCI address > * bit 16 - 24: bus number > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h > index 1b35168..a918a61 100644 > --- a/include/hw/i386/pc.h > +++ b/include/hw/i386/pc.h > @@ -241,6 +241,12 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state, int *piix_devfn, > MemoryRegion *ram_memory); > > PCIBus *find_i440fx(void); > +/* > + * Register hosts without their own configuration registers > + * to i440fx main PCI host bridge > + */ > +int i440fx_register_snooping_host(PCIHostState *host); > + > /* piix4.c */ > extern PCIDevice *piix4_dev; > int piix4_init(PCIBus *bus, ISABus **isa_bus, int devfn); > diff --git a/include/hw/pci/pci_host.h b/include/hw/pci/pci_host.h > index a041919..dc7ccb2 100644 > --- a/include/hw/pci/pci_host.h > +++ b/include/hw/pci/pci_host.h > @@ -87,6 +87,9 @@ typedef struct PCIHostBridgeSnoopedClass { > GPtrArray *(*snooping_hosts)(PCIHostState *); > } PCIHostBridgeSnoopedClass; > > +/* Returns the buses interval owned by the input host */ > +Range pci_host_get_buses_range(PCIHostState *host); > + > /* common internal helpers for PCI/PCIe hosts, cut off overflows */ > void pci_host_config_write_common(PCIDevice *pci_dev, uint32_t addr, > uint32_t limit, uint32_t val, uint32_t len); > -- > 2.1.0