* [Qemu-devel] [PATCH 01/10] pci_bridge: split out pci bridge code into pci_bridge.c from pci.c
2010-06-17 6:15 [Qemu-devel] [PATCH 00/10] pci: pci to pci bridge clean up and enhancement Isaku Yamahata
@ 2010-06-17 6:15 ` Isaku Yamahata
2010-06-17 6:15 ` [Qemu-devel] [PATCH 02/10] qdev: export qdev_reset() for later use Isaku Yamahata
` (9 subsequent siblings)
10 siblings, 0 replies; 28+ messages in thread
From: Isaku Yamahata @ 2010-06-17 6:15 UTC (permalink / raw)
To: qemu-devel
Cc: jan.kiszka, mst, allen.m.kay, blauwirbel, yamahata, kraxel,
stefano.stabellini, jean.guyader
Move pci bridge related code into pci_bridge.c from pci.c
for further enhancement. pci.c is big enough now, so split it out.
In fact, some of pci bridge functions stays in pci.c because
it accesses to PCIBus member. Unstatic the accessor functions
and use them from pci_bridge.c
Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
---
Makefile.objs | 2 +-
hw/apb_pci.c | 1 +
hw/dec_pci.c | 1 +
hw/pci.c | 104 ++++++---------------------------------------------
hw/pci.h | 8 +++-
hw/pci_bridge.c | 112 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
hw/pci_bridge.h | 37 ++++++++++++++++++
7 files changed, 170 insertions(+), 95 deletions(-)
create mode 100644 hw/pci_bridge.c
create mode 100644 hw/pci_bridge.h
diff --git a/Makefile.objs b/Makefile.objs
index 2bfb6d1..5c37e5c 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -139,7 +139,7 @@ user-obj-y += cutils.o cache-utils.o
hw-obj-y =
hw-obj-y += vl.o loader.o
hw-obj-y += virtio.o virtio-console.o
-hw-obj-y += fw_cfg.o pci.o pci_host.o pcie_host.o
+hw-obj-y += fw_cfg.o pci.o pci_host.o pcie_host.o pci_bridge.o
hw-obj-y += watchdog.o
hw-obj-$(CONFIG_ISA_MMIO) += isa_mmio.o
hw-obj-$(CONFIG_ECC) += ecc.o
diff --git a/hw/apb_pci.c b/hw/apb_pci.c
index 31c8d70..c11d9b5 100644
--- a/hw/apb_pci.c
+++ b/hw/apb_pci.c
@@ -29,6 +29,7 @@
#include "sysbus.h"
#include "pci.h"
#include "pci_host.h"
+#include "pci_bridge.h"
#include "rwhandler.h"
#include "apb_pci.h"
#include "sysemu.h"
diff --git a/hw/dec_pci.c b/hw/dec_pci.c
index 024c67c..b2759dd 100644
--- a/hw/dec_pci.c
+++ b/hw/dec_pci.c
@@ -27,6 +27,7 @@
#include "sysbus.h"
#include "pci.h"
#include "pci_host.h"
+#include "pci_bridge.h"
/* debug DEC */
//#define DEBUG_DEC
diff --git a/hw/pci.c b/hw/pci.c
index 3777c1c..9ba62eb 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -277,24 +277,28 @@ PCIBus *pci_register_bus(DeviceState *parent, const char *name,
return bus;
}
-static void pci_register_secondary_bus(PCIBus *parent,
- PCIBus *bus,
- PCIDevice *dev,
- pci_map_irq_fn map_irq,
- const char *name)
+PCIBus *pci_register_secondary_bus(PCIBus *parent,
+ PCIDevice *dev,
+ pci_map_irq_fn map_irq,
+ const char *name)
{
- qbus_create_inplace(&bus->qbus, &pci_bus_info, &dev->qdev, name);
+ PCIBus *bus;
+ bus = pci_bus_new(&dev->qdev, name, 0);
+
bus->map_irq = map_irq;
bus->parent_dev = dev;
QLIST_INIT(&bus->child);
QLIST_INSERT_HEAD(&parent->child, bus, sibling);
+
+ return bus;
}
-static void pci_unregister_secondary_bus(PCIBus *bus)
+void pci_unregister_secondary_bus(PCIBus *bus)
{
assert(QLIST_EMPTY(&bus->child));
QLIST_REMOVE(bus, sibling);
+ qbus_free(&bus->qbus);
}
int pci_bus_num(PCIBus *s)
@@ -1466,20 +1470,12 @@ PCIDevice *pci_nic_init_nofail(NICInfo *nd, const char *default_model,
return res;
}
-typedef struct {
- PCIDevice dev;
- PCIBus bus;
- uint32_t vid;
- uint32_t did;
-} PCIBridge;
-
-
static void pci_bridge_update_mappings_fn(PCIBus *b, PCIDevice *d)
{
pci_update_mappings(d);
}
-static void pci_bridge_update_mappings(PCIBus *b)
+void pci_bridge_update_mappings(PCIBus *b)
{
PCIBus *child;
@@ -1490,21 +1486,6 @@ static void pci_bridge_update_mappings(PCIBus *b)
}
}
-static void pci_bridge_write_config(PCIDevice *d,
- uint32_t address, uint32_t val, int len)
-{
- pci_default_write_config(d, address, val, len);
-
- if (/* io base/limit */
- ranges_overlap(address, len, PCI_IO_BASE, 2) ||
-
- /* memory base/limit, prefetchable base/limit and
- io base/limit upper 16 */
- ranges_overlap(address, len, PCI_MEMORY_BASE, 20)) {
- pci_bridge_update_mappings(d->bus);
- }
-}
-
PCIBus *pci_find_bus(PCIBus *bus, int bus_num)
{
PCIBus *sec;
@@ -1548,46 +1529,6 @@ PCIDevice *pci_find_device(PCIBus *bus, int bus_num, int slot, int function)
return bus->devices[PCI_DEVFN(slot, function)];
}
-static int pci_bridge_initfn(PCIDevice *dev)
-{
- PCIBridge *s = DO_UPCAST(PCIBridge, dev, dev);
-
- pci_config_set_vendor_id(s->dev.config, s->vid);
- pci_config_set_device_id(s->dev.config, s->did);
-
- pci_set_word(dev->config + PCI_STATUS,
- PCI_STATUS_66MHZ | PCI_STATUS_FAST_BACK);
- pci_config_set_class(dev->config, PCI_CLASS_BRIDGE_PCI);
- dev->config[PCI_HEADER_TYPE] = PCI_HEADER_TYPE_BRIDGE;
- pci_set_word(dev->config + PCI_SEC_STATUS,
- PCI_STATUS_66MHZ | PCI_STATUS_FAST_BACK);
- return 0;
-}
-
-static int pci_bridge_exitfn(PCIDevice *pci_dev)
-{
- PCIBridge *s = DO_UPCAST(PCIBridge, dev, pci_dev);
- PCIBus *bus = &s->bus;
- pci_unregister_secondary_bus(bus);
- return 0;
-}
-
-PCIBus *pci_bridge_init(PCIBus *bus, int devfn, uint16_t vid, uint16_t did,
- pci_map_irq_fn map_irq, const char *name)
-{
- PCIDevice *dev;
- PCIBridge *s;
-
- dev = pci_create(bus, devfn, "pci-bridge");
- qdev_prop_set_uint32(&dev->qdev, "vendorid", vid);
- qdev_prop_set_uint32(&dev->qdev, "deviceid", did);
- qdev_init_nofail(&dev->qdev);
-
- s = DO_UPCAST(PCIBridge, dev, dev);
- pci_register_secondary_bus(bus, &s->bus, &s->dev, map_irq, name);
- return &s->bus;
-}
-
PCIDevice *pci_bridge_get_device(PCIBus *bus)
{
return bus->parent_dev;
@@ -1855,24 +1796,3 @@ static void pcibus_dev_print(Monitor *mon, DeviceState *dev, int indent)
r->addr, r->addr + r->size - 1);
}
}
-
-static PCIDeviceInfo bridge_info = {
- .qdev.name = "pci-bridge",
- .qdev.size = sizeof(PCIBridge),
- .init = pci_bridge_initfn,
- .exit = pci_bridge_exitfn,
- .config_write = pci_bridge_write_config,
- .header_type = PCI_HEADER_TYPE_BRIDGE,
- .qdev.props = (Property[]) {
- DEFINE_PROP_HEX32("vendorid", PCIBridge, vid, 0),
- DEFINE_PROP_HEX32("deviceid", PCIBridge, did, 0),
- DEFINE_PROP_END_OF_LIST(),
- }
-};
-
-static void pci_register_devices(void)
-{
- pci_qdev_register(&bridge_info);
-}
-
-device_init(pci_register_devices)
diff --git a/hw/pci.h b/hw/pci.h
index 3a15bd4..f6e2551 100644
--- a/hw/pci.h
+++ b/hw/pci.h
@@ -208,6 +208,11 @@ void pci_bus_hotplug(PCIBus *bus, pci_hotplug_fn hotplug, DeviceState *dev);
PCIBus *pci_register_bus(DeviceState *parent, const char *name,
pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
void *irq_opaque, int devfn_min, int nirq);
+PCIBus *pci_register_secondary_bus(PCIBus *parent,
+ PCIDevice *dev,
+ pci_map_irq_fn map_irq,
+ const char *name);
+void pci_unregister_secondary_bus(PCIBus *bus);
void pci_bus_set_mem_base(PCIBus *bus, target_phys_addr_t base);
@@ -228,9 +233,8 @@ int pci_read_devaddr(Monitor *mon, const char *addr, int *domp, int *busp,
void do_pci_info_print(Monitor *mon, const QObject *data);
void do_pci_info(Monitor *mon, QObject **ret_data);
-PCIBus *pci_bridge_init(PCIBus *bus, int devfn, uint16_t vid, uint16_t did,
- pci_map_irq_fn map_irq, const char *name);
PCIDevice *pci_bridge_get_device(PCIBus *bus);
+void pci_bridge_update_mappings(PCIBus *b);
static inline void
pci_set_byte(uint8_t *config, uint8_t val)
diff --git a/hw/pci_bridge.c b/hw/pci_bridge.c
new file mode 100644
index 0000000..bf746f1
--- /dev/null
+++ b/hw/pci_bridge.c
@@ -0,0 +1,112 @@
+/*
+ * QEMU PCI bus manager
+ *
+ * Copyright (c) 2004 Fabrice Bellard
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+/*
+ * split out from pci.c
+ * Copyright (c) 2010 Isaku Yamahata <yamahata at valinux co jp>
+ * VA Linux Systems Japan K.K.
+ */
+
+#include "pci_bridge.h"
+
+typedef struct {
+ PCIDevice dev;
+ PCIBus *bus;
+ uint32_t vid;
+ uint32_t did;
+} PCIBridge;
+
+static void pci_bridge_write_config(PCIDevice *d,
+ uint32_t address, uint32_t val, int len)
+{
+ pci_default_write_config(d, address, val, len);
+
+ if (/* io base/limit */
+ ranges_overlap(address, len, PCI_IO_BASE, 2) ||
+
+ /* memory base/limit, prefetchable base/limit and
+ io base/limit upper 16 */
+ ranges_overlap(address, len, PCI_MEMORY_BASE, 20)) {
+ pci_bridge_update_mappings(d->bus);
+ }
+}
+
+static int pci_bridge_initfn(PCIDevice *dev)
+{
+ PCIBridge *s = DO_UPCAST(PCIBridge, dev, dev);
+
+ pci_config_set_vendor_id(s->dev.config, s->vid);
+ pci_config_set_device_id(s->dev.config, s->did);
+
+ pci_set_word(dev->config + PCI_STATUS,
+ PCI_STATUS_66MHZ | PCI_STATUS_FAST_BACK);
+ pci_config_set_class(dev->config, PCI_CLASS_BRIDGE_PCI);
+ dev->config[PCI_HEADER_TYPE] = PCI_HEADER_TYPE_BRIDGE;
+ pci_set_word(dev->config + PCI_SEC_STATUS,
+ PCI_STATUS_66MHZ | PCI_STATUS_FAST_BACK);
+ return 0;
+}
+
+static int pci_bridge_exitfn(PCIDevice *pci_dev)
+{
+ PCIBridge *s = DO_UPCAST(PCIBridge, dev, pci_dev);
+ pci_unregister_secondary_bus(s->bus);
+ return 0;
+}
+
+PCIBus *pci_bridge_init(PCIBus *bus, int devfn, uint16_t vid, uint16_t did,
+ pci_map_irq_fn map_irq, const char *name)
+{
+ PCIDevice *dev;
+ PCIBridge *s;
+
+ dev = pci_create(bus, devfn, "pci-bridge");
+ qdev_prop_set_uint32(&dev->qdev, "vendorid", vid);
+ qdev_prop_set_uint32(&dev->qdev, "deviceid", did);
+ qdev_init_nofail(&dev->qdev);
+
+ s = DO_UPCAST(PCIBridge, dev, dev);
+ s->bus = pci_register_secondary_bus(bus, &s->dev, map_irq, name);
+ return s->bus;
+}
+
+static PCIDeviceInfo bridge_info = {
+ .qdev.name = "pci-bridge",
+ .qdev.size = sizeof(PCIBridge),
+ .init = pci_bridge_initfn,
+ .exit = pci_bridge_exitfn,
+ .config_write = pci_bridge_write_config,
+ .header_type = PCI_HEADER_TYPE_BRIDGE,
+ .qdev.props = (Property[]) {
+ DEFINE_PROP_HEX32("vendorid", PCIBridge, vid, 0),
+ DEFINE_PROP_HEX32("deviceid", PCIBridge, did, 0),
+ DEFINE_PROP_END_OF_LIST(),
+ }
+};
+
+static void pci_register_devices(void)
+{
+ pci_qdev_register(&bridge_info);
+}
+
+device_init(pci_register_devices)
diff --git a/hw/pci_bridge.h b/hw/pci_bridge.h
new file mode 100644
index 0000000..1d46abf
--- /dev/null
+++ b/hw/pci_bridge.h
@@ -0,0 +1,37 @@
+/*
+ * 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, write to the Free Software
+ * Foundation, 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ *
+ * Copyright (c) 2009 Isaku Yamahata <yamahata at valinux co jp>
+ * VA Linux Systems Japan K.K.
+ *
+ */
+
+#ifndef QEMU_PCI_BRIDGE_H
+#define QEMU_PCI_BRIDGE_H
+
+#include "pci.h"
+
+PCIBus *pci_bridge_init(PCIBus *bus, int devfn, uint16_t vid, uint16_t did,
+ pci_map_irq_fn map_irq, const char *name);
+
+#endif /* QEMU_PCI_BRIDGE_H */
+/*
+ * Local variables:
+ * c-indent-level: 4
+ * c-basic-offset: 4
+ * tab-width: 8
+ * indent-tab-mode: nil
+ * End:
+ */
--
1.6.6.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [Qemu-devel] [PATCH 02/10] qdev: export qdev_reset() for later use.
2010-06-17 6:15 [Qemu-devel] [PATCH 00/10] pci: pci to pci bridge clean up and enhancement Isaku Yamahata
2010-06-17 6:15 ` [Qemu-devel] [PATCH 01/10] pci_bridge: split out pci bridge code into pci_bridge.c from pci.c Isaku Yamahata
@ 2010-06-17 6:15 ` Isaku Yamahata
2010-06-17 7:01 ` Markus Armbruster
2010-06-17 10:05 ` [Qemu-devel] " Michael S. Tsirkin
2010-06-17 6:15 ` [Qemu-devel] [PATCH 03/10] pci: fix pci_bus_reset() with 64bit BAR and several clean ups Isaku Yamahata
` (8 subsequent siblings)
10 siblings, 2 replies; 28+ messages in thread
From: Isaku Yamahata @ 2010-06-17 6:15 UTC (permalink / raw)
To: qemu-devel
Cc: jan.kiszka, mst, allen.m.kay, blauwirbel, yamahata, kraxel,
stefano.stabellini, jean.guyader
export qdev_reset() for later use.
Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
---
hw/qdev.c | 13 +++++++++----
hw/qdev.h | 1 +
2 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/hw/qdev.c b/hw/qdev.c
index 61f999c..378f842 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -256,13 +256,18 @@ DeviceState *qdev_device_add(QemuOpts *opts)
return qdev;
}
-static void qdev_reset(void *opaque)
+void qdev_reset(DeviceState *dev)
{
- DeviceState *dev = opaque;
if (dev->info->reset)
dev->info->reset(dev);
}
+static void qdev_reset_fn(void *opaque)
+{
+ DeviceState *dev = opaque;
+ qdev_reset(dev);
+}
+
/* Initialize a device. Device properties should be set before calling
this function. IRQs and MMIO regions should be connected/mapped after
calling this function.
@@ -278,7 +283,7 @@ int qdev_init(DeviceState *dev)
qdev_free(dev);
return rc;
}
- qemu_register_reset(qdev_reset, dev);
+ qemu_register_reset(qdev_reset_fn, dev);
if (dev->info->vmsd) {
vmstate_register_with_alias_id(-1, dev->info->vmsd, dev,
dev->instance_id_alias,
@@ -348,7 +353,7 @@ void qdev_free(DeviceState *dev)
if (dev->opts)
qemu_opts_del(dev->opts);
}
- qemu_unregister_reset(qdev_reset, dev);
+ qemu_unregister_reset(qdev_reset_fn, dev);
QLIST_REMOVE(dev, sibling);
for (prop = dev->info->props; prop && prop->name; prop++) {
if (prop->info->free) {
diff --git a/hw/qdev.h b/hw/qdev.h
index be5ad67..5fbdebf 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -113,6 +113,7 @@ typedef struct GlobalProperty {
DeviceState *qdev_create(BusState *bus, const char *name);
int qdev_device_help(QemuOpts *opts);
DeviceState *qdev_device_add(QemuOpts *opts);
+void qdev_reset(DeviceState *dev);
int qdev_init(DeviceState *dev) QEMU_WARN_UNUSED_RESULT;
void qdev_init_nofail(DeviceState *dev);
void qdev_set_legacy_instance_id(DeviceState *dev, int alias_id,
--
1.6.6.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [Qemu-devel] [PATCH 02/10] qdev: export qdev_reset() for later use.
2010-06-17 6:15 ` [Qemu-devel] [PATCH 02/10] qdev: export qdev_reset() for later use Isaku Yamahata
@ 2010-06-17 7:01 ` Markus Armbruster
2010-06-17 10:05 ` [Qemu-devel] " Michael S. Tsirkin
1 sibling, 0 replies; 28+ messages in thread
From: Markus Armbruster @ 2010-06-17 7:01 UTC (permalink / raw)
To: Isaku Yamahata
Cc: mst, jan.kiszka, stefano.stabellini, allen.m.kay, qemu-devel,
blauwirbel, kraxel, jean.guyader
Isaku Yamahata <yamahata@valinux.co.jp> writes:
> export qdev_reset() for later use.
>
> Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
> ---
> hw/qdev.c | 13 +++++++++----
> hw/qdev.h | 1 +
> 2 files changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/hw/qdev.c b/hw/qdev.c
> index 61f999c..378f842 100644
> --- a/hw/qdev.c
> +++ b/hw/qdev.c
> @@ -256,13 +256,18 @@ DeviceState *qdev_device_add(QemuOpts *opts)
> return qdev;
> }
>
> -static void qdev_reset(void *opaque)
> +void qdev_reset(DeviceState *dev)
> {
> - DeviceState *dev = opaque;
> if (dev->info->reset)
> dev->info->reset(dev);
> }
>
> +static void qdev_reset_fn(void *opaque)
> +{
> + DeviceState *dev = opaque;
> + qdev_reset(dev);
> +}
> +
Nitpick: why the local variable?
[...]
^ permalink raw reply [flat|nested] 28+ messages in thread* [Qemu-devel] Re: [PATCH 02/10] qdev: export qdev_reset() for later use.
2010-06-17 6:15 ` [Qemu-devel] [PATCH 02/10] qdev: export qdev_reset() for later use Isaku Yamahata
2010-06-17 7:01 ` Markus Armbruster
@ 2010-06-17 10:05 ` Michael S. Tsirkin
1 sibling, 0 replies; 28+ messages in thread
From: Michael S. Tsirkin @ 2010-06-17 10:05 UTC (permalink / raw)
To: Isaku Yamahata
Cc: stefano.stabellini, jan.kiszka, allen.m.kay, qemu-devel,
blauwirbel, kraxel, jean.guyader
On Thu, Jun 17, 2010 at 03:15:44PM +0900, Isaku Yamahata wrote:
> export qdev_reset() for later use.
>
> Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
> ---
> hw/qdev.c | 13 +++++++++----
> hw/qdev.h | 1 +
> 2 files changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/hw/qdev.c b/hw/qdev.c
> index 61f999c..378f842 100644
> --- a/hw/qdev.c
> +++ b/hw/qdev.c
> @@ -256,13 +256,18 @@ DeviceState *qdev_device_add(QemuOpts *opts)
> return qdev;
> }
>
> -static void qdev_reset(void *opaque)
> +void qdev_reset(DeviceState *dev)
What does this API do? Yes, I see that it invokes
the reset callback internally. But what does it do
that the caller wants? After all, the callback
gets invoked on reset directly.
> {
> - DeviceState *dev = opaque;
> if (dev->info->reset)
> dev->info->reset(dev);
> }
>
> +static void qdev_reset_fn(void *opaque)
> +{
> + DeviceState *dev = opaque;
> + qdev_reset(dev);
> +}
> +
> /* Initialize a device. Device properties should be set before calling
> this function. IRQs and MMIO regions should be connected/mapped after
> calling this function.
> @@ -278,7 +283,7 @@ int qdev_init(DeviceState *dev)
> qdev_free(dev);
> return rc;
> }
> - qemu_register_reset(qdev_reset, dev);
> + qemu_register_reset(qdev_reset_fn, dev);
> if (dev->info->vmsd) {
> vmstate_register_with_alias_id(-1, dev->info->vmsd, dev,
> dev->instance_id_alias,
> @@ -348,7 +353,7 @@ void qdev_free(DeviceState *dev)
> if (dev->opts)
> qemu_opts_del(dev->opts);
> }
> - qemu_unregister_reset(qdev_reset, dev);
> + qemu_unregister_reset(qdev_reset_fn, dev);
> QLIST_REMOVE(dev, sibling);
> for (prop = dev->info->props; prop && prop->name; prop++) {
> if (prop->info->free) {
> diff --git a/hw/qdev.h b/hw/qdev.h
> index be5ad67..5fbdebf 100644
> --- a/hw/qdev.h
> +++ b/hw/qdev.h
> @@ -113,6 +113,7 @@ typedef struct GlobalProperty {
> DeviceState *qdev_create(BusState *bus, const char *name);
> int qdev_device_help(QemuOpts *opts);
> DeviceState *qdev_device_add(QemuOpts *opts);
> +void qdev_reset(DeviceState *dev);
> int qdev_init(DeviceState *dev) QEMU_WARN_UNUSED_RESULT;
> void qdev_init_nofail(DeviceState *dev);
> void qdev_set_legacy_instance_id(DeviceState *dev, int alias_id,
> --
> 1.6.6.1
^ permalink raw reply [flat|nested] 28+ messages in thread
* [Qemu-devel] [PATCH 03/10] pci: fix pci_bus_reset() with 64bit BAR and several clean ups.
2010-06-17 6:15 [Qemu-devel] [PATCH 00/10] pci: pci to pci bridge clean up and enhancement Isaku Yamahata
2010-06-17 6:15 ` [Qemu-devel] [PATCH 01/10] pci_bridge: split out pci bridge code into pci_bridge.c from pci.c Isaku Yamahata
2010-06-17 6:15 ` [Qemu-devel] [PATCH 02/10] qdev: export qdev_reset() for later use Isaku Yamahata
@ 2010-06-17 6:15 ` Isaku Yamahata
2010-06-17 10:58 ` [Qemu-devel] " Michael S. Tsirkin
2010-06-17 6:15 ` [Qemu-devel] [PATCH 04/10] pci_bridge: introduce pci bridge layer Isaku Yamahata
` (7 subsequent siblings)
10 siblings, 1 reply; 28+ messages in thread
From: Isaku Yamahata @ 2010-06-17 6:15 UTC (permalink / raw)
To: qemu-devel
Cc: jan.kiszka, mst, allen.m.kay, blauwirbel, yamahata, kraxel,
stefano.stabellini, jean.guyader
fix pci_device_reset() with 64bit BAR.
export pci_bus_reset(), pci_device_reset() and two helper functions
for later use. And several clean ups.
Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
---
hw/pci.c | 44 ++++++++++++++++++++++++++++++++++++--------
hw/pci.h | 5 +++++
2 files changed, 41 insertions(+), 8 deletions(-)
diff --git a/hw/pci.c b/hw/pci.c
index 9ba62eb..87f5e6c 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -144,28 +144,50 @@ static void pci_update_irq_status(PCIDevice *dev)
}
}
-static void pci_device_reset(PCIDevice *dev)
+void pci_device_reset_default(PCIDevice *dev)
{
int r;
dev->irq_state = 0;
pci_update_irq_status(dev);
- dev->config[PCI_COMMAND] &= ~(PCI_COMMAND_IO | PCI_COMMAND_MEMORY |
- PCI_COMMAND_MASTER);
+ pci_set_word(dev->config + PCI_COMMAND,
+ pci_get_word(dev->config + PCI_COMMAND) &
+ ~(PCI_COMMAND_IO | PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER));
dev->config[PCI_CACHE_LINE_SIZE] = 0x0;
dev->config[PCI_INTERRUPT_LINE] = 0x0;
for (r = 0; r < PCI_NUM_REGIONS; ++r) {
- if (!dev->io_regions[r].size) {
+ PCIIORegion *region = &dev->io_regions[r];
+ if (!region->size) {
continue;
}
- pci_set_long(dev->config + pci_bar(dev, r), dev->io_regions[r].type);
+
+ if (!(region->type & PCI_BASE_ADDRESS_SPACE_IO) &&
+ region->type & PCI_BASE_ADDRESS_MEM_TYPE_64) {
+ pci_set_quad(dev->config + pci_bar(dev, r), region->type);
+ } else {
+ pci_set_long(dev->config + pci_bar(dev, r), region->type);
+ }
}
pci_update_mappings(dev);
}
-static void pci_bus_reset(void *opaque)
+void pci_device_reset(PCIDevice *dev)
+{
+ if (!dev->qdev.info) {
+ /* for not qdevified device */
+ pci_device_reset_default(dev);
+ return;
+ }
+
+ qdev_reset(&dev->qdev);
+
+ /* TODO: make DeviceInfo::reset call
+ pci_device_reset_default() itself. */
+ pci_device_reset_default(dev);
+}
+
+void pci_bus_reset(PCIBus *bus)
{
- PCIBus *bus = opaque;
int i;
for (i = 0; i < bus->nirq; i++) {
@@ -178,6 +200,12 @@ static void pci_bus_reset(void *opaque)
}
}
+static void pci_bus_resetfn(void *opaque)
+{
+ PCIBus *bus = opaque;
+ pci_bus_reset(bus);
+}
+
static void pci_host_bus_register(int domain, PCIBus *bus)
{
struct PCIHostBus *host;
@@ -231,7 +259,7 @@ void pci_bus_new_inplace(PCIBus *bus, DeviceState *parent,
pci_host_bus_register(0, bus); /* for now only pci domain 0 is supported */
vmstate_register(-1, &vmstate_pcibus, bus);
- qemu_register_reset(pci_bus_reset, bus);
+ qemu_register_reset(pci_bus_resetfn, bus);
}
PCIBus *pci_bus_new(DeviceState *parent, const char *name, int devfn_min)
diff --git a/hw/pci.h b/hw/pci.h
index f6e2551..2a2c8ef 100644
--- a/hw/pci.h
+++ b/hw/pci.h
@@ -199,6 +199,11 @@ int pci_device_load(PCIDevice *s, QEMUFile *f);
typedef void (*pci_set_irq_fn)(void *opaque, int irq_num, int level);
typedef int (*pci_map_irq_fn)(PCIDevice *pci_dev, int irq_num);
typedef int (*pci_hotplug_fn)(DeviceState *qdev, PCIDevice *pci_dev, int state);
+
+void pci_device_reset_default(PCIDevice *dev);
+void pci_device_reset(PCIDevice *dev);
+void pci_bus_reset(PCIBus *bus);
+
void pci_bus_new_inplace(PCIBus *bus, DeviceState *parent,
const char *name, int devfn_min);
PCIBus *pci_bus_new(DeviceState *parent, const char *name, int devfn_min);
--
1.6.6.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [Qemu-devel] Re: [PATCH 03/10] pci: fix pci_bus_reset() with 64bit BAR and several clean ups.
2010-06-17 6:15 ` [Qemu-devel] [PATCH 03/10] pci: fix pci_bus_reset() with 64bit BAR and several clean ups Isaku Yamahata
@ 2010-06-17 10:58 ` Michael S. Tsirkin
0 siblings, 0 replies; 28+ messages in thread
From: Michael S. Tsirkin @ 2010-06-17 10:58 UTC (permalink / raw)
To: Isaku Yamahata
Cc: stefano.stabellini, jan.kiszka, allen.m.kay, qemu-devel,
blauwirbel, kraxel, jean.guyader
On Thu, Jun 17, 2010 at 03:15:45PM +0900, Isaku Yamahata wrote:
> fix pci_device_reset() with 64bit BAR.
> export pci_bus_reset(), pci_device_reset() and two helper functions
> for later use. And several clean ups.
>
> Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
> ---
> hw/pci.c | 44 ++++++++++++++++++++++++++++++++++++--------
> hw/pci.h | 5 +++++
> 2 files changed, 41 insertions(+), 8 deletions(-)
>
> diff --git a/hw/pci.c b/hw/pci.c
> index 9ba62eb..87f5e6c 100644
> --- a/hw/pci.c
> +++ b/hw/pci.c
> @@ -144,28 +144,50 @@ static void pci_update_irq_status(PCIDevice *dev)
> }
> }
>
> -static void pci_device_reset(PCIDevice *dev)
> +void pci_device_reset_default(PCIDevice *dev)
> {
> int r;
>
> dev->irq_state = 0;
> pci_update_irq_status(dev);
> - dev->config[PCI_COMMAND] &= ~(PCI_COMMAND_IO | PCI_COMMAND_MEMORY |
> - PCI_COMMAND_MASTER);
> + pci_set_word(dev->config + PCI_COMMAND,
> + pci_get_word(dev->config + PCI_COMMAND) &
> + ~(PCI_COMMAND_IO | PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER));
> dev->config[PCI_CACHE_LINE_SIZE] = 0x0;
> dev->config[PCI_INTERRUPT_LINE] = 0x0;
> for (r = 0; r < PCI_NUM_REGIONS; ++r) {
> - if (!dev->io_regions[r].size) {
> + PCIIORegion *region = &dev->io_regions[r];
> + if (!region->size) {
> continue;
> }
> - pci_set_long(dev->config + pci_bar(dev, r), dev->io_regions[r].type);
> +
> + if (!(region->type & PCI_BASE_ADDRESS_SPACE_IO) &&
> + region->type & PCI_BASE_ADDRESS_MEM_TYPE_64) {
> + pci_set_quad(dev->config + pci_bar(dev, r), region->type);
> + } else {
> + pci_set_long(dev->config + pci_bar(dev, r), region->type);
> + }
> }
> pci_update_mappings(dev);
> }
>
I applied the first hunk. Looking at it
made me notice that we don't clear interrupt disable
bit on reset, and we really should as it is read/write.
Rather than duplicating code, we should just use wmask.
I ended up with this:
commit b82d3876099c4f1fd009082f052e3bac7e3062e7
Author: Isaku Yamahata <yamahata@valinux.co.jp>
Date: Thu Jun 17 15:15:45 2010 +0900
pci: fix pci_device_reset
Clear interrupt disable bit on reset, according to PCI spec.
Fix pci_device_reset() with 64bit BAR.
Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
diff --git a/hw/pci.c b/hw/pci.c
index 7787005..de33745 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -150,15 +150,24 @@ static void pci_device_reset(PCIDevice *dev)
dev->irq_state = 0;
pci_update_irq_status(dev);
- dev->config[PCI_COMMAND] &= ~(PCI_COMMAND_IO | PCI_COMMAND_MEMORY |
- PCI_COMMAND_MASTER);
+ /* Clear all writeable bits */
+ pci_set_word(dev->config + PCI_COMMAND,
+ pci_get_word(dev->config + PCI_COMMAND) &
+ ~pci_get_word(dev->wmask + PCI_COMMAND));
dev->config[PCI_CACHE_LINE_SIZE] = 0x0;
dev->config[PCI_INTERRUPT_LINE] = 0x0;
for (r = 0; r < PCI_NUM_REGIONS; ++r) {
- if (!dev->io_regions[r].size) {
+ PCIIORegion *region = &dev->io_regions[r];
+ if (!region->size) {
continue;
}
- pci_set_long(dev->config + pci_bar(dev, r), dev->io_regions[r].type);
+
+ if (!(region->type & PCI_BASE_ADDRESS_SPACE_IO) &&
+ region->type & PCI_BASE_ADDRESS_MEM_TYPE_64) {
+ pci_set_quad(dev->config + pci_bar(dev, r), region->type);
+ } else {
+ pci_set_long(dev->config + pci_bar(dev, r), region->type);
+ }
}
pci_update_mappings(dev);
}
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [Qemu-devel] [PATCH 04/10] pci_bridge: introduce pci bridge layer.
2010-06-17 6:15 [Qemu-devel] [PATCH 00/10] pci: pci to pci bridge clean up and enhancement Isaku Yamahata
` (2 preceding siblings ...)
2010-06-17 6:15 ` [Qemu-devel] [PATCH 03/10] pci: fix pci_bus_reset() with 64bit BAR and several clean ups Isaku Yamahata
@ 2010-06-17 6:15 ` Isaku Yamahata
2010-06-17 9:52 ` [Qemu-devel] " Michael S. Tsirkin
2010-06-17 6:15 ` [Qemu-devel] [PATCH 05/10] pci bridge: add helper function for ssvid capability Isaku Yamahata
` (6 subsequent siblings)
10 siblings, 1 reply; 28+ messages in thread
From: Isaku Yamahata @ 2010-06-17 6:15 UTC (permalink / raw)
To: qemu-devel
Cc: jan.kiszka, mst, allen.m.kay, blauwirbel, yamahata, kraxel,
stefano.stabellini, jean.guyader
introduce pci bridge layer.
export pci_bridge_write_config() for generic use.
support device reset and bus reset of bridge control.
convert apb bridge and dec p2p bridge to use new pci bridge layer.
save/restore is supported as a side effect.
This might be a bit over engineering, but this is also preparation
for pci express root/upstream/downstream port.
Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
---
hw/apb_pci.c | 38 +++++++++-----
hw/dec_pci.c | 28 +++++++---
hw/pci_bridge.c | 146 +++++++++++++++++++++++++++++++++++++------------------
hw/pci_bridge.h | 35 ++++++++++++-
qemu-common.h | 1 +
5 files changed, 177 insertions(+), 71 deletions(-)
diff --git a/hw/apb_pci.c b/hw/apb_pci.c
index c11d9b5..cb9051b 100644
--- a/hw/apb_pci.c
+++ b/hw/apb_pci.c
@@ -31,6 +31,7 @@
#include "pci_host.h"
#include "pci_bridge.h"
#include "rwhandler.h"
+#include "pci_bridge.h"
#include "apb_pci.h"
#include "sysemu.h"
@@ -294,9 +295,12 @@ static void pci_apb_set_irq(void *opaque, int irq_num, int level)
}
}
-static void apb_pci_bridge_init(PCIBus *b)
+static int apb_pci_bridge_init(PCIBridge *br)
{
- PCIDevice *dev = pci_bridge_get_device(b);
+ PCIDevice *dev = &br->dev;
+
+ pci_config_set_vendor_id(dev->config, PCI_VENDOR_ID_SUN);
+ pci_config_set_device_id(dev->config, PCI_DEVICE_ID_SUN_SIMBA);
/*
* command register:
@@ -316,6 +320,8 @@ static void apb_pci_bridge_init(PCIBus *b)
pci_set_byte(dev->config + PCI_HEADER_TYPE,
pci_get_byte(dev->config + PCI_HEADER_TYPE) |
PCI_HEADER_TYPE_MULTI_FUNCTION);
+
+ return 0;
}
PCIBus *pci_apb_init(target_phys_addr_t special_base,
@@ -326,6 +332,7 @@ PCIBus *pci_apb_init(target_phys_addr_t special_base,
SysBusDevice *s;
APBState *d;
unsigned int i;
+ PCIBridge *br;
/* Ultrasparc PBM main bus */
dev = qdev_create(NULL, "pbm");
@@ -351,17 +358,13 @@ PCIBus *pci_apb_init(target_phys_addr_t special_base,
pci_create_simple(d->bus, 0, "pbm");
/* APB secondary busses */
- *bus2 = pci_bridge_init(d->bus, PCI_DEVFN(1, 0),
- PCI_VENDOR_ID_SUN, PCI_DEVICE_ID_SUN_SIMBA,
- pci_apb_map_irq,
- "Advanced PCI Bus secondary bridge 1");
- apb_pci_bridge_init(*bus2);
-
- *bus3 = pci_bridge_init(d->bus, PCI_DEVFN(1, 1),
- PCI_VENDOR_ID_SUN, PCI_DEVICE_ID_SUN_SIMBA,
- pci_apb_map_irq,
- "Advanced PCI Bus secondary bridge 2");
- apb_pci_bridge_init(*bus3);
+ br = pci_bridge_create_simple(d->bus, PCI_DEVFN(1, 0), "pbm-bridge",
+ "Advanced PCI Bus secondary bridge 1");
+ *bus2 = pci_bridge_get_sec_bus(br);
+
+ br = pci_bridge_create_simple(d->bus, PCI_DEVFN(1, 1), "pbm-bridge",
+ "Advanced PCI Bus secondary bridge 2");
+ *bus3 = pci_bridge_get_sec_bus(br);
return d->bus;
}
@@ -446,10 +449,19 @@ static SysBusDeviceInfo pbm_host_info = {
.qdev.reset = pci_pbm_reset,
.init = pci_pbm_init_device,
};
+
+static PCIBridgeInfo pbm_pci_bridge_info = {
+ .pci.qdev.name = "pbm-bridge",
+ .pci.qdev.vmsd = &vmstate_pci_device,
+ .init = apb_pci_bridge_init,
+ .map_irq = pci_apb_map_irq,
+};
+
static void pbm_register_devices(void)
{
sysbus_register_withprop(&pbm_host_info);
pci_qdev_register(&pbm_pci_host_info);
+ pci_bridge_qdev_register(&pbm_pci_bridge_info);
}
device_init(pbm_register_devices)
diff --git a/hw/dec_pci.c b/hw/dec_pci.c
index b2759dd..45b5c28 100644
--- a/hw/dec_pci.c
+++ b/hw/dec_pci.c
@@ -49,18 +49,27 @@ static int dec_map_irq(PCIDevice *pci_dev, int irq_num)
return irq_num;
}
-PCIBus *pci_dec_21154_init(PCIBus *parent_bus, int devfn)
+static int dec_21154_initfn(PCIBridge *br)
{
- DeviceState *dev;
- PCIBus *ret;
+ pci_config_set_vendor_id(br->dev.config, PCI_VENDOR_ID_DEC);
+ pci_config_set_device_id(br->dev.config, PCI_DEVICE_ID_DEC_21154);
+ return 0;
+}
- dev = qdev_create(NULL, "dec-21154");
- qdev_init_nofail(dev);
- ret = pci_bridge_init(parent_bus, devfn,
- PCI_VENDOR_ID_DEC, PCI_DEVICE_ID_DEC_21154,
- dec_map_irq, "DEC 21154 PCI-PCI bridge");
+static PCIBridgeInfo dec_21154_pci_bridge_info = {
+ .pci.qdev.name = "dec-21154-p2p-bridge",
+ .pci.qdev.desc = "DEC 21154 PCI-PCI bridge",
+ .pci.qdev.vmsd = &vmstate_pci_device,
+ .init = dec_21154_initfn,
+ .map_irq = dec_map_irq,
+};
- return ret;
+PCIBus *pci_dec_21154_init(PCIBus *parent_bus, int devfn)
+{
+ PCIBridge *br;
+ br = pci_bridge_create_simple(parent_bus, devfn, "dec-21154-p2p-bridge",
+ "DEC 21154 PCI-PCI bridge");
+ return pci_bridge_get_sec_bus(br);
}
static int pci_dec_21154_init_device(SysBusDevice *dev)
@@ -99,6 +108,7 @@ static void dec_register_devices(void)
sysbus_register_dev("dec-21154", sizeof(DECState),
pci_dec_21154_init_device);
pci_qdev_register(&dec_21154_pci_host_info);
+ pci_bridge_qdev_register(&dec_21154_pci_bridge_info);
}
device_init(dec_register_devices)
diff --git a/hw/pci_bridge.c b/hw/pci_bridge.c
index bf746f1..43c21d4 100644
--- a/hw/pci_bridge.c
+++ b/hw/pci_bridge.c
@@ -29,16 +29,10 @@
#include "pci_bridge.h"
-typedef struct {
- PCIDevice dev;
- PCIBus *bus;
- uint32_t vid;
- uint32_t did;
-} PCIBridge;
-
-static void pci_bridge_write_config(PCIDevice *d,
- uint32_t address, uint32_t val, int len)
+void pci_bridge_write_config(PCIDevice *d,
+ uint32_t address, uint32_t val, int len)
{
+ uint16_t bridge_control = pci_get_word(d->config + PCI_BRIDGE_CONTROL);
pci_default_write_config(d, address, val, len);
if (/* io base/limit */
@@ -49,64 +43,122 @@ static void pci_bridge_write_config(PCIDevice *d,
ranges_overlap(address, len, PCI_MEMORY_BASE, 20)) {
pci_bridge_update_mappings(d->bus);
}
+
+ if (ranges_overlap(address, len, PCI_BRIDGE_CONTROL, 2)) {
+ uint16_t new = pci_get_word(d->config + PCI_BRIDGE_CONTROL);
+ if (!(bridge_control & PCI_BRIDGE_CTL_BUS_RESET) &&
+ (new & PCI_BRIDGE_CTL_BUS_RESET)) {
+ /* 0 -> 1 */
+ PCIBridge *br = DO_UPCAST(PCIBridge, dev, d);
+ pci_bus_reset(br->bus);
+ }
+ }
}
-static int pci_bridge_initfn(PCIDevice *dev)
+void pci_bridge_reset_reg(PCIDevice *dev)
{
- PCIBridge *s = DO_UPCAST(PCIBridge, dev, dev);
+ uint8_t *conf = dev->config;
+
+ conf[PCI_PRIMARY_BUS] = 0;
+ conf[PCI_SECONDARY_BUS] = 0;
+ conf[PCI_SUBORDINATE_BUS] = 0;
+ conf[PCI_SEC_LATENCY_TIMER] = 0;
- pci_config_set_vendor_id(s->dev.config, s->vid);
- pci_config_set_device_id(s->dev.config, s->did);
+ conf[PCI_IO_BASE] = 0;
+ conf[PCI_IO_LIMIT] = 0;
+ pci_set_word(conf + PCI_MEMORY_BASE, 0);
+ pci_set_word(conf + PCI_MEMORY_LIMIT, 0);
+ pci_set_word(conf + PCI_PREF_MEMORY_BASE, 0);
+ pci_set_word(conf + PCI_PREF_MEMORY_LIMIT, 0);
+ pci_set_word(conf + PCI_PREF_BASE_UPPER32, 0);
+ pci_set_word(conf + PCI_PREF_LIMIT_UPPER32, 0);
+
+ pci_set_word(conf + PCI_BRIDGE_CONTROL, 0);
+}
+
+void pci_bridge_reset(DeviceState *qdev)
+{
+ PCIDevice *dev = DO_UPCAST(PCIDevice, qdev, qdev);
+ PCIBridge *br = DO_UPCAST(PCIBridge, dev, dev);
+
+ pci_bus_reset(br->bus);
+ pci_bridge_reset_reg(dev);
+ pci_device_reset_default(dev);
+}
+
+static int pci_bridge_initfn(PCIDevice *dev)
+{
+ PCIBridge *br = DO_UPCAST(PCIBridge, dev, dev);
+ PCIDeviceInfo *pci_info = DO_UPCAST(PCIDeviceInfo, qdev, dev->qdev.info);
+ PCIBridgeInfo *info = DO_UPCAST(PCIBridgeInfo, pci, pci_info);
+ int rc = 0;
pci_set_word(dev->config + PCI_STATUS,
PCI_STATUS_66MHZ | PCI_STATUS_FAST_BACK);
pci_config_set_class(dev->config, PCI_CLASS_BRIDGE_PCI);
- dev->config[PCI_HEADER_TYPE] = PCI_HEADER_TYPE_BRIDGE;
pci_set_word(dev->config + PCI_SEC_STATUS,
PCI_STATUS_66MHZ | PCI_STATUS_FAST_BACK);
- return 0;
+
+ br->bus = pci_register_secondary_bus(dev->bus, dev,
+ info->map_irq, br->bus_name);
+ if (!br->bus) {
+ return -1;
+ }
+ if (info->init) {
+ rc = info->init(br);
+ }
+ return rc;
}
-static int pci_bridge_exitfn(PCIDevice *pci_dev)
+static int pci_bridge_exitfn(PCIDevice *dev)
{
- PCIBridge *s = DO_UPCAST(PCIBridge, dev, pci_dev);
- pci_unregister_secondary_bus(s->bus);
+ PCIBridge *br = DO_UPCAST(PCIBridge, dev, dev);
+ PCIDeviceInfo *pci_info = DO_UPCAST(PCIDeviceInfo, qdev, dev->qdev.info);
+ PCIBridgeInfo *info = DO_UPCAST(PCIBridgeInfo, pci, pci_info);
+
+ if (info->exit) {
+ int rc = info->exit(br);
+ if (rc){
+ return rc;
+ }
+ }
+ pci_unregister_secondary_bus(br->bus);
return 0;
}
-PCIBus *pci_bridge_init(PCIBus *bus, int devfn, uint16_t vid, uint16_t did,
- pci_map_irq_fn map_irq, const char *name)
+void pci_bridge_qdev_register(PCIBridgeInfo *info)
{
- PCIDevice *dev;
- PCIBridge *s;
-
- dev = pci_create(bus, devfn, "pci-bridge");
- qdev_prop_set_uint32(&dev->qdev, "vendorid", vid);
- qdev_prop_set_uint32(&dev->qdev, "deviceid", did);
- qdev_init_nofail(&dev->qdev);
-
- s = DO_UPCAST(PCIBridge, dev, dev);
- s->bus = pci_register_secondary_bus(bus, &s->dev, map_irq, name);
- return s->bus;
+ if (!info->pci.qdev.size) {
+ info->pci.qdev.size = sizeof(PCIBridge);
+ }
+ info->pci.init = pci_bridge_initfn;
+ info->pci.exit = pci_bridge_exitfn;
+ info->pci.header_type = PCI_HEADER_TYPE_BRIDGE;
+ if (!info->pci.config_write) {
+ info->pci.config_write = pci_bridge_write_config;
+ }
+ if (!info->pci.qdev.reset) {
+ info->pci.qdev.reset = pci_bridge_reset;
+ }
+ pci_qdev_register(&info->pci);
}
-static PCIDeviceInfo bridge_info = {
- .qdev.name = "pci-bridge",
- .qdev.size = sizeof(PCIBridge),
- .init = pci_bridge_initfn,
- .exit = pci_bridge_exitfn,
- .config_write = pci_bridge_write_config,
- .header_type = PCI_HEADER_TYPE_BRIDGE,
- .qdev.props = (Property[]) {
- DEFINE_PROP_HEX32("vendorid", PCIBridge, vid, 0),
- DEFINE_PROP_HEX32("deviceid", PCIBridge, did, 0),
- DEFINE_PROP_END_OF_LIST(),
- }
-};
+PCIBridge *pci_bridge_create(PCIBus *bus, int devfn, const char *name)
+{
+ PCIDevice *dev = pci_create(bus, devfn, name);
+ return DO_UPCAST(PCIBridge, dev, dev);
+}
-static void pci_register_devices(void)
+void pci_bridge_set_bus_name(PCIBridge *br, const char *bus_name)
{
- pci_qdev_register(&bridge_info);
+ br->bus_name = bus_name;
}
-device_init(pci_register_devices)
+PCIBridge *pci_bridge_create_simple(PCIBus *bus, int devfn,
+ const char *name, const char *bus_name)
+{
+ PCIBridge *br = pci_bridge_create(bus, devfn, name);
+ pci_bridge_set_bus_name(br, bus_name);
+ qdev_init_nofail(&br->dev.qdev);
+ return br;
+}
diff --git a/hw/pci_bridge.h b/hw/pci_bridge.h
index 1d46abf..2747e7f 100644
--- a/hw/pci_bridge.h
+++ b/hw/pci_bridge.h
@@ -23,8 +23,39 @@
#include "pci.h"
-PCIBus *pci_bridge_init(PCIBus *bus, int devfn, uint16_t vid, uint16_t did,
- pci_map_irq_fn map_irq, const char *name);
+struct PCIBridge {
+ PCIDevice dev;
+
+ /* private member */
+ PCIBus *bus;
+ const char *bus_name;
+};
+
+static inline PCIBus *pci_bridge_get_sec_bus(PCIBridge *br)
+{
+ return br->bus;
+}
+
+void pci_bridge_write_config(PCIDevice *d,
+ uint32_t address, uint32_t val, int len);
+void pci_bridge_reset_reg(PCIDevice *dev);
+void pci_bridge_reset(DeviceState *qdev);
+
+typedef int (*pci_bridge_qdev_initfn)(PCIBridge *br);
+typedef int (*pci_bridge_qdev_exitfn)(PCIBridge *br);
+typedef struct
+{
+ PCIDeviceInfo pci;
+ pci_bridge_qdev_initfn init;
+ pci_bridge_qdev_exitfn exit;
+ pci_map_irq_fn map_irq;
+} PCIBridgeInfo;
+
+void pci_bridge_qdev_register(PCIBridgeInfo *info);
+PCIBridge *pci_bridge_create(PCIBus *bus, int devfn, const char *name);
+void pci_bridge_set_bus_name(PCIBridge *br, const char *bus_name);
+PCIBridge *pci_bridge_create_simple(PCIBus *bus, int devfn,
+ const char *name, const char *bus_name);
#endif /* QEMU_PCI_BRIDGE_H */
/*
diff --git a/qemu-common.h b/qemu-common.h
index d133f35..8257311 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -221,6 +221,7 @@ typedef struct PCIHostState PCIHostState;
typedef struct PCIExpressHost PCIExpressHost;
typedef struct PCIBus PCIBus;
typedef struct PCIDevice PCIDevice;
+typedef struct PCIBridge PCIBridge;
typedef struct SerialState SerialState;
typedef struct IRQState *qemu_irq;
typedef struct PCMCIACardState PCMCIACardState;
--
1.6.6.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [Qemu-devel] Re: [PATCH 04/10] pci_bridge: introduce pci bridge layer.
2010-06-17 6:15 ` [Qemu-devel] [PATCH 04/10] pci_bridge: introduce pci bridge layer Isaku Yamahata
@ 2010-06-17 9:52 ` Michael S. Tsirkin
0 siblings, 0 replies; 28+ messages in thread
From: Michael S. Tsirkin @ 2010-06-17 9:52 UTC (permalink / raw)
To: Isaku Yamahata
Cc: stefano.stabellini, jan.kiszka, allen.m.kay, qemu-devel,
blauwirbel, kraxel, jean.guyader
On Thu, Jun 17, 2010 at 03:15:46PM +0900, Isaku Yamahata wrote:
> introduce pci bridge layer.
> export pci_bridge_write_config() for generic use.
> support device reset and bus reset of bridge control.
> convert apb bridge and dec p2p bridge to use new pci bridge layer.
> save/restore is supported as a side effect.
>
> This might be a bit over engineering, but this is also preparation
> for pci express root/upstream/downstream port.
>
> Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
Well, preparations are easier to judge with patches that use them.
> ---
> hw/apb_pci.c | 38 +++++++++-----
> hw/dec_pci.c | 28 +++++++---
> hw/pci_bridge.c | 146 +++++++++++++++++++++++++++++++++++++------------------
> hw/pci_bridge.h | 35 ++++++++++++-
> qemu-common.h | 1 +
> 5 files changed, 177 insertions(+), 71 deletions(-)
>
> diff --git a/hw/apb_pci.c b/hw/apb_pci.c
> index c11d9b5..cb9051b 100644
> --- a/hw/apb_pci.c
> +++ b/hw/apb_pci.c
> @@ -31,6 +31,7 @@
> #include "pci_host.h"
> #include "pci_bridge.h"
> #include "rwhandler.h"
> +#include "pci_bridge.h"
> #include "apb_pci.h"
> #include "sysemu.h"
>
> @@ -294,9 +295,12 @@ static void pci_apb_set_irq(void *opaque, int irq_num, int level)
> }
> }
>
> -static void apb_pci_bridge_init(PCIBus *b)
> +static int apb_pci_bridge_init(PCIBridge *br)
> {
> - PCIDevice *dev = pci_bridge_get_device(b);
> + PCIDevice *dev = &br->dev;
> +
> + pci_config_set_vendor_id(dev->config, PCI_VENDOR_ID_SUN);
> + pci_config_set_device_id(dev->config, PCI_DEVICE_ID_SUN_SIMBA);
>
> /*
> * command register:
> @@ -316,6 +320,8 @@ static void apb_pci_bridge_init(PCIBus *b)
> pci_set_byte(dev->config + PCI_HEADER_TYPE,
> pci_get_byte(dev->config + PCI_HEADER_TYPE) |
> PCI_HEADER_TYPE_MULTI_FUNCTION);
> +
> + return 0;
> }
>
> PCIBus *pci_apb_init(target_phys_addr_t special_base,
> @@ -326,6 +332,7 @@ PCIBus *pci_apb_init(target_phys_addr_t special_base,
> SysBusDevice *s;
> APBState *d;
> unsigned int i;
> + PCIBridge *br;
>
> /* Ultrasparc PBM main bus */
> dev = qdev_create(NULL, "pbm");
> @@ -351,17 +358,13 @@ PCIBus *pci_apb_init(target_phys_addr_t special_base,
> pci_create_simple(d->bus, 0, "pbm");
>
> /* APB secondary busses */
> - *bus2 = pci_bridge_init(d->bus, PCI_DEVFN(1, 0),
> - PCI_VENDOR_ID_SUN, PCI_DEVICE_ID_SUN_SIMBA,
> - pci_apb_map_irq,
> - "Advanced PCI Bus secondary bridge 1");
> - apb_pci_bridge_init(*bus2);
> -
> - *bus3 = pci_bridge_init(d->bus, PCI_DEVFN(1, 1),
> - PCI_VENDOR_ID_SUN, PCI_DEVICE_ID_SUN_SIMBA,
> - pci_apb_map_irq,
> - "Advanced PCI Bus secondary bridge 2");
> - apb_pci_bridge_init(*bus3);
> + br = pci_bridge_create_simple(d->bus, PCI_DEVFN(1, 0), "pbm-bridge",
> + "Advanced PCI Bus secondary bridge 1");
> + *bus2 = pci_bridge_get_sec_bus(br);
> +
> + br = pci_bridge_create_simple(d->bus, PCI_DEVFN(1, 1), "pbm-bridge",
> + "Advanced PCI Bus secondary bridge 2");
> + *bus3 = pci_bridge_get_sec_bus(br);
>
> return d->bus;
> }
> @@ -446,10 +449,19 @@ static SysBusDeviceInfo pbm_host_info = {
> .qdev.reset = pci_pbm_reset,
> .init = pci_pbm_init_device,
> };
> +
> +static PCIBridgeInfo pbm_pci_bridge_info = {
> + .pci.qdev.name = "pbm-bridge",
> + .pci.qdev.vmsd = &vmstate_pci_device,
> + .init = apb_pci_bridge_init,
> + .map_irq = pci_apb_map_irq,
> +};
> +
> static void pbm_register_devices(void)
> {
> sysbus_register_withprop(&pbm_host_info);
> pci_qdev_register(&pbm_pci_host_info);
> + pci_bridge_qdev_register(&pbm_pci_bridge_info);
> }
>
> device_init(pbm_register_devices)
> diff --git a/hw/dec_pci.c b/hw/dec_pci.c
> index b2759dd..45b5c28 100644
> --- a/hw/dec_pci.c
> +++ b/hw/dec_pci.c
> @@ -49,18 +49,27 @@ static int dec_map_irq(PCIDevice *pci_dev, int irq_num)
> return irq_num;
> }
>
> -PCIBus *pci_dec_21154_init(PCIBus *parent_bus, int devfn)
> +static int dec_21154_initfn(PCIBridge *br)
> {
> - DeviceState *dev;
> - PCIBus *ret;
> + pci_config_set_vendor_id(br->dev.config, PCI_VENDOR_ID_DEC);
> + pci_config_set_device_id(br->dev.config, PCI_DEVICE_ID_DEC_21154);
> + return 0;
> +}
>
> - dev = qdev_create(NULL, "dec-21154");
> - qdev_init_nofail(dev);
> - ret = pci_bridge_init(parent_bus, devfn,
> - PCI_VENDOR_ID_DEC, PCI_DEVICE_ID_DEC_21154,
> - dec_map_irq, "DEC 21154 PCI-PCI bridge");
> +static PCIBridgeInfo dec_21154_pci_bridge_info = {
> + .pci.qdev.name = "dec-21154-p2p-bridge",
> + .pci.qdev.desc = "DEC 21154 PCI-PCI bridge",
> + .pci.qdev.vmsd = &vmstate_pci_device,
> + .init = dec_21154_initfn,
> + .map_irq = dec_map_irq,
> +};
>
> - return ret;
> +PCIBus *pci_dec_21154_init(PCIBus *parent_bus, int devfn)
> +{
> + PCIBridge *br;
> + br = pci_bridge_create_simple(parent_bus, devfn, "dec-21154-p2p-bridge",
> + "DEC 21154 PCI-PCI bridge");
> + return pci_bridge_get_sec_bus(br);
> }
>
> static int pci_dec_21154_init_device(SysBusDevice *dev)
> @@ -99,6 +108,7 @@ static void dec_register_devices(void)
> sysbus_register_dev("dec-21154", sizeof(DECState),
> pci_dec_21154_init_device);
> pci_qdev_register(&dec_21154_pci_host_info);
> + pci_bridge_qdev_register(&dec_21154_pci_bridge_info);
> }
>
> device_init(dec_register_devices)
Please document the APIs. For example, what is
pci_bridge_create_simple and how does it differ
from pci_bridge_create?
> diff --git a/hw/pci_bridge.c b/hw/pci_bridge.c
> index bf746f1..43c21d4 100644
> --- a/hw/pci_bridge.c
> +++ b/hw/pci_bridge.c
> @@ -29,16 +29,10 @@
>
> #include "pci_bridge.h"
>
> -typedef struct {
> - PCIDevice dev;
> - PCIBus *bus;
> - uint32_t vid;
> - uint32_t did;
> -} PCIBridge;
> -
> -static void pci_bridge_write_config(PCIDevice *d,
> - uint32_t address, uint32_t val, int len)
> +void pci_bridge_write_config(PCIDevice *d,
> + uint32_t address, uint32_t val, int len)
> {
> + uint16_t bridge_control = pci_get_word(d->config + PCI_BRIDGE_CONTROL);
> pci_default_write_config(d, address, val, len);
>
> if (/* io base/limit */
> @@ -49,64 +43,122 @@ static void pci_bridge_write_config(PCIDevice *d,
> ranges_overlap(address, len, PCI_MEMORY_BASE, 20)) {
> pci_bridge_update_mappings(d->bus);
> }
> +
> + if (ranges_overlap(address, len, PCI_BRIDGE_CONTROL, 2)) {
> + uint16_t new = pci_get_word(d->config + PCI_BRIDGE_CONTROL);
> + if (!(bridge_control & PCI_BRIDGE_CTL_BUS_RESET) &&
> + (new & PCI_BRIDGE_CTL_BUS_RESET)) {
> + /* 0 -> 1 */
> + PCIBridge *br = DO_UPCAST(PCIBridge, dev, d);
> + pci_bus_reset(br->bus);
> + }
> + }
> }
>
> -static int pci_bridge_initfn(PCIDevice *dev)
> +void pci_bridge_reset_reg(PCIDevice *dev)
> {
> - PCIBridge *s = DO_UPCAST(PCIBridge, dev, dev);
> + uint8_t *conf = dev->config;
> +
> + conf[PCI_PRIMARY_BUS] = 0;
> + conf[PCI_SECONDARY_BUS] = 0;
> + conf[PCI_SUBORDINATE_BUS] = 0;
> + conf[PCI_SEC_LATENCY_TIMER] = 0;
>
> - pci_config_set_vendor_id(s->dev.config, s->vid);
> - pci_config_set_device_id(s->dev.config, s->did);
> + conf[PCI_IO_BASE] = 0;
> + conf[PCI_IO_LIMIT] = 0;
> + pci_set_word(conf + PCI_MEMORY_BASE, 0);
> + pci_set_word(conf + PCI_MEMORY_LIMIT, 0);
> + pci_set_word(conf + PCI_PREF_MEMORY_BASE, 0);
> + pci_set_word(conf + PCI_PREF_MEMORY_LIMIT, 0);
> + pci_set_word(conf + PCI_PREF_BASE_UPPER32, 0);
> + pci_set_word(conf + PCI_PREF_LIMIT_UPPER32, 0);
> +
> + pci_set_word(conf + PCI_BRIDGE_CONTROL, 0);
> +}
> +
> +void pci_bridge_reset(DeviceState *qdev)
> +{
> + PCIDevice *dev = DO_UPCAST(PCIDevice, qdev, qdev);
> + PCIBridge *br = DO_UPCAST(PCIBridge, dev, dev);
> +
> + pci_bus_reset(br->bus);
> + pci_bridge_reset_reg(dev);
> + pci_device_reset_default(dev);
> +}
> +
> +static int pci_bridge_initfn(PCIDevice *dev)
> +{
> + PCIBridge *br = DO_UPCAST(PCIBridge, dev, dev);
> + PCIDeviceInfo *pci_info = DO_UPCAST(PCIDeviceInfo, qdev, dev->qdev.info);
> + PCIBridgeInfo *info = DO_UPCAST(PCIBridgeInfo, pci, pci_info);
> + int rc = 0;
>
> pci_set_word(dev->config + PCI_STATUS,
> PCI_STATUS_66MHZ | PCI_STATUS_FAST_BACK);
> pci_config_set_class(dev->config, PCI_CLASS_BRIDGE_PCI);
> - dev->config[PCI_HEADER_TYPE] = PCI_HEADER_TYPE_BRIDGE;
> pci_set_word(dev->config + PCI_SEC_STATUS,
> PCI_STATUS_66MHZ | PCI_STATUS_FAST_BACK);
> - return 0;
> +
> + br->bus = pci_register_secondary_bus(dev->bus, dev,
> + info->map_irq, br->bus_name);
> + if (!br->bus) {
> + return -1;
> + }
> + if (info->init) {
> + rc = info->init(br);
> + }
> + return rc;
> }
>
> -static int pci_bridge_exitfn(PCIDevice *pci_dev)
> +static int pci_bridge_exitfn(PCIDevice *dev)
> {
> - PCIBridge *s = DO_UPCAST(PCIBridge, dev, pci_dev);
> - pci_unregister_secondary_bus(s->bus);
> + PCIBridge *br = DO_UPCAST(PCIBridge, dev, dev);
> + PCIDeviceInfo *pci_info = DO_UPCAST(PCIDeviceInfo, qdev, dev->qdev.info);
> + PCIBridgeInfo *info = DO_UPCAST(PCIBridgeInfo, pci, pci_info);
> +
> + if (info->exit) {
> + int rc = info->exit(br);
> + if (rc){
> + return rc;
> + }
> + }
> + pci_unregister_secondary_bus(br->bus);
> return 0;
> }
>
> -PCIBus *pci_bridge_init(PCIBus *bus, int devfn, uint16_t vid, uint16_t did,
> - pci_map_irq_fn map_irq, const char *name)
> +void pci_bridge_qdev_register(PCIBridgeInfo *info)
> {
> - PCIDevice *dev;
> - PCIBridge *s;
> -
> - dev = pci_create(bus, devfn, "pci-bridge");
> - qdev_prop_set_uint32(&dev->qdev, "vendorid", vid);
> - qdev_prop_set_uint32(&dev->qdev, "deviceid", did);
> - qdev_init_nofail(&dev->qdev);
> -
> - s = DO_UPCAST(PCIBridge, dev, dev);
> - s->bus = pci_register_secondary_bus(bus, &s->dev, map_irq, name);
> - return s->bus;
> + if (!info->pci.qdev.size) {
> + info->pci.qdev.size = sizeof(PCIBridge);
> + }
> + info->pci.init = pci_bridge_initfn;
> + info->pci.exit = pci_bridge_exitfn;
> + info->pci.header_type = PCI_HEADER_TYPE_BRIDGE;
> + if (!info->pci.config_write) {
> + info->pci.config_write = pci_bridge_write_config;
> + }
> + if (!info->pci.qdev.reset) {
> + info->pci.qdev.reset = pci_bridge_reset;
> + }
> + pci_qdev_register(&info->pci);
> }
>
> -static PCIDeviceInfo bridge_info = {
> - .qdev.name = "pci-bridge",
> - .qdev.size = sizeof(PCIBridge),
> - .init = pci_bridge_initfn,
> - .exit = pci_bridge_exitfn,
> - .config_write = pci_bridge_write_config,
> - .header_type = PCI_HEADER_TYPE_BRIDGE,
> - .qdev.props = (Property[]) {
> - DEFINE_PROP_HEX32("vendorid", PCIBridge, vid, 0),
> - DEFINE_PROP_HEX32("deviceid", PCIBridge, did, 0),
> - DEFINE_PROP_END_OF_LIST(),
> - }
> -};
> +PCIBridge *pci_bridge_create(PCIBus *bus, int devfn, const char *name)
> +{
> + PCIDevice *dev = pci_create(bus, devfn, name);
> + return DO_UPCAST(PCIBridge, dev, dev);
> +}
>
> -static void pci_register_devices(void)
> +void pci_bridge_set_bus_name(PCIBridge *br, const char *bus_name)
> {
> - pci_qdev_register(&bridge_info);
> + br->bus_name = bus_name;
> }
>
> -device_init(pci_register_devices)
> +PCIBridge *pci_bridge_create_simple(PCIBus *bus, int devfn,
> + const char *name, const char *bus_name)
> +{
> + PCIBridge *br = pci_bridge_create(bus, devfn, name);
> + pci_bridge_set_bus_name(br, bus_name);
> + qdev_init_nofail(&br->dev.qdev);
> + return br;
> +}
> diff --git a/hw/pci_bridge.h b/hw/pci_bridge.h
> index 1d46abf..2747e7f 100644
> --- a/hw/pci_bridge.h
> +++ b/hw/pci_bridge.h
> @@ -23,8 +23,39 @@
>
> #include "pci.h"
>
> -PCIBus *pci_bridge_init(PCIBus *bus, int devfn, uint16_t vid, uint16_t did,
> - pci_map_irq_fn map_irq, const char *name);
> +struct PCIBridge {
> + PCIDevice dev;
> +
> + /* private member */
> + PCIBus *bus;
> + const char *bus_name;
> +};
> +
> +static inline PCIBus *pci_bridge_get_sec_bus(PCIBridge *br)
> +{
> + return br->bus;
> +}
> +
> +void pci_bridge_write_config(PCIDevice *d,
> + uint32_t address, uint32_t val, int len);
> +void pci_bridge_reset_reg(PCIDevice *dev);
> +void pci_bridge_reset(DeviceState *qdev);
> +
> +typedef int (*pci_bridge_qdev_initfn)(PCIBridge *br);
> +typedef int (*pci_bridge_qdev_exitfn)(PCIBridge *br);
> +typedef struct
struct PCIBridgeInfo, so we can later forward declare if we need to.
> +{
> + PCIDeviceInfo pci;
> + pci_bridge_qdev_initfn init;
> + pci_bridge_qdev_exitfn exit;
> + pci_map_irq_fn map_irq;
> +} PCIBridgeInfo;
> +
> +void pci_bridge_qdev_register(PCIBridgeInfo *info);
> +PCIBridge *pci_bridge_create(PCIBus *bus, int devfn, const char *name);
> +void pci_bridge_set_bus_name(PCIBridge *br, const char *bus_name);
> +PCIBridge *pci_bridge_create_simple(PCIBus *bus, int devfn,
> + const char *name, const char *bus_name);
>
> #endif /* QEMU_PCI_BRIDGE_H */
> /*
> diff --git a/qemu-common.h b/qemu-common.h
> index d133f35..8257311 100644
> --- a/qemu-common.h
> +++ b/qemu-common.h
> @@ -221,6 +221,7 @@ typedef struct PCIHostState PCIHostState;
> typedef struct PCIExpressHost PCIExpressHost;
> typedef struct PCIBus PCIBus;
> typedef struct PCIDevice PCIDevice;
> +typedef struct PCIBridge PCIBridge;
> typedef struct SerialState SerialState;
> typedef struct IRQState *qemu_irq;
> typedef struct PCMCIACardState PCMCIACardState;
> --
> 1.6.6.1
^ permalink raw reply [flat|nested] 28+ messages in thread
* [Qemu-devel] [PATCH 05/10] pci bridge: add helper function for ssvid capability.
2010-06-17 6:15 [Qemu-devel] [PATCH 00/10] pci: pci to pci bridge clean up and enhancement Isaku Yamahata
` (3 preceding siblings ...)
2010-06-17 6:15 ` [Qemu-devel] [PATCH 04/10] pci_bridge: introduce pci bridge layer Isaku Yamahata
@ 2010-06-17 6:15 ` Isaku Yamahata
2010-06-17 10:01 ` [Qemu-devel] " Michael S. Tsirkin
2010-06-17 6:15 ` [Qemu-devel] [PATCH 06/10] pci: eliminate work around in pci_device_reset() Isaku Yamahata
` (5 subsequent siblings)
10 siblings, 1 reply; 28+ messages in thread
From: Isaku Yamahata @ 2010-06-17 6:15 UTC (permalink / raw)
To: qemu-devel
Cc: jan.kiszka, mst, allen.m.kay, blauwirbel, yamahata, kraxel,
stefano.stabellini, jean.guyader
helper function to add ssvid capability.
Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
---
hw/pci_bridge.c | 20 ++++++++++++++++++++
hw/pci_bridge.h | 3 +++
2 files changed, 23 insertions(+), 0 deletions(-)
diff --git a/hw/pci_bridge.c b/hw/pci_bridge.c
index 43c21d4..1397a11 100644
--- a/hw/pci_bridge.c
+++ b/hw/pci_bridge.c
@@ -29,6 +29,26 @@
#include "pci_bridge.h"
+/* PCI bridge subsystem vendor ID helper functions */
+#define PCI_SSVID_SIZEOF 8
+#define PCI_SSVID_SVID 4
+#define PCI_SSVID_SSID 6
+
+int pci_bridge_ssvid_init(PCIDevice *dev, uint8_t offset,
+ uint16_t svid, uint16_t ssid)
+{
+ int pos;
+ pos = pci_add_capability_at_offset(dev, PCI_CAP_ID_SSVID,
+ offset, PCI_SSVID_SIZEOF);
+ if (pos < 0) {
+ return pos;
+ }
+
+ pci_set_word(dev->config + pos + PCI_SSVID_SVID, svid);
+ pci_set_word(dev->config + pos + PCI_SSVID_SSID, ssid);
+ return pos;
+}
+
void pci_bridge_write_config(PCIDevice *d,
uint32_t address, uint32_t val, int len)
{
diff --git a/hw/pci_bridge.h b/hw/pci_bridge.h
index 2747e7f..a1f160b 100644
--- a/hw/pci_bridge.h
+++ b/hw/pci_bridge.h
@@ -23,6 +23,9 @@
#include "pci.h"
+int pci_bridge_ssvid_init(PCIDevice *dev, uint8_t offset,
+ uint16_t svid, uint16_t ssid);
+
struct PCIBridge {
PCIDevice dev;
--
1.6.6.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [Qemu-devel] Re: [PATCH 05/10] pci bridge: add helper function for ssvid capability.
2010-06-17 6:15 ` [Qemu-devel] [PATCH 05/10] pci bridge: add helper function for ssvid capability Isaku Yamahata
@ 2010-06-17 10:01 ` Michael S. Tsirkin
0 siblings, 0 replies; 28+ messages in thread
From: Michael S. Tsirkin @ 2010-06-17 10:01 UTC (permalink / raw)
To: Isaku Yamahata
Cc: stefano.stabellini, jan.kiszka, allen.m.kay, qemu-devel,
blauwirbel, kraxel, jean.guyader
On Thu, Jun 17, 2010 at 03:15:47PM +0900, Isaku Yamahata wrote:
> helper function to add ssvid capability.
>
> Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
But .. this is unused?
> ---
> hw/pci_bridge.c | 20 ++++++++++++++++++++
> hw/pci_bridge.h | 3 +++
> 2 files changed, 23 insertions(+), 0 deletions(-)
>
> diff --git a/hw/pci_bridge.c b/hw/pci_bridge.c
> index 43c21d4..1397a11 100644
> --- a/hw/pci_bridge.c
> +++ b/hw/pci_bridge.c
> @@ -29,6 +29,26 @@
>
> #include "pci_bridge.h"
>
> +/* PCI bridge subsystem vendor ID helper functions */
> +#define PCI_SSVID_SIZEOF 8
> +#define PCI_SSVID_SVID 4
> +#define PCI_SSVID_SSID 6
> +
> +int pci_bridge_ssvid_init(PCIDevice *dev, uint8_t offset,
> + uint16_t svid, uint16_t ssid)
> +{
> + int pos;
> + pos = pci_add_capability_at_offset(dev, PCI_CAP_ID_SSVID,
> + offset, PCI_SSVID_SIZEOF);
> + if (pos < 0) {
> + return pos;
> + }
> +
> + pci_set_word(dev->config + pos + PCI_SSVID_SVID, svid);
> + pci_set_word(dev->config + pos + PCI_SSVID_SSID, ssid);
> + return pos;
> +}
> +
> void pci_bridge_write_config(PCIDevice *d,
> uint32_t address, uint32_t val, int len)
> {
> diff --git a/hw/pci_bridge.h b/hw/pci_bridge.h
> index 2747e7f..a1f160b 100644
> --- a/hw/pci_bridge.h
> +++ b/hw/pci_bridge.h
> @@ -23,6 +23,9 @@
>
> #include "pci.h"
>
> +int pci_bridge_ssvid_init(PCIDevice *dev, uint8_t offset,
> + uint16_t svid, uint16_t ssid);
> +
> struct PCIBridge {
> PCIDevice dev;
>
> --
> 1.6.6.1
^ permalink raw reply [flat|nested] 28+ messages in thread
* [Qemu-devel] [PATCH 06/10] pci: eliminate work around in pci_device_reset().
2010-06-17 6:15 [Qemu-devel] [PATCH 00/10] pci: pci to pci bridge clean up and enhancement Isaku Yamahata
` (4 preceding siblings ...)
2010-06-17 6:15 ` [Qemu-devel] [PATCH 05/10] pci bridge: add helper function for ssvid capability Isaku Yamahata
@ 2010-06-17 6:15 ` Isaku Yamahata
2010-06-17 6:15 ` [Qemu-devel] [PATCH 07/10] pci: fix pci domain registering Isaku Yamahata
` (4 subsequent siblings)
10 siblings, 0 replies; 28+ messages in thread
From: Isaku Yamahata @ 2010-06-17 6:15 UTC (permalink / raw)
To: qemu-devel
Cc: jan.kiszka, mst, allen.m.kay, blauwirbel, yamahata, kraxel,
stefano.stabellini, jean.guyader
Eliminate work around in pci_device_reset() by
making each pci reset function to call pci_device_reset_default().
If a driver reset function isn't specified, set it to pci default reset
function.
Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
---
hw/e1000.c | 1 +
hw/lsi53c895a.c | 2 ++
hw/pci.c | 12 ++++++++----
hw/pcnet.c | 1 +
hw/rtl8139.c | 2 ++
hw/virtio-pci.c | 1 +
6 files changed, 15 insertions(+), 4 deletions(-)
diff --git a/hw/e1000.c b/hw/e1000.c
index 0da65f9..448a743 100644
--- a/hw/e1000.c
+++ b/hw/e1000.c
@@ -1069,6 +1069,7 @@ static void e1000_reset(void *opaque)
memmove(d->mac_reg, mac_reg_init, sizeof mac_reg_init);
d->rxbuf_min_shift = 1;
memset(&d->tx, 0, sizeof d->tx);
+ pci_device_reset_default(&d->dev);
}
static NetClientInfo net_e1000_info = {
diff --git a/hw/lsi53c895a.c b/hw/lsi53c895a.c
index f5a91ba..68723e3 100644
--- a/hw/lsi53c895a.c
+++ b/hw/lsi53c895a.c
@@ -358,6 +358,8 @@ static void lsi_soft_reset(LSIState *s)
qemu_free(s->current);
s->current = NULL;
}
+
+ pci_device_reset_default(&s->dev);
}
static int lsi_dma_40bit(LSIState *s)
diff --git a/hw/pci.c b/hw/pci.c
index 87f5e6c..5dee102 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -171,6 +171,11 @@ void pci_device_reset_default(PCIDevice *dev)
pci_update_mappings(dev);
}
+static void pci_device_reset_default_fn(DeviceState *qdev)
+{
+ pci_device_reset_default(DO_UPCAST(PCIDevice, qdev, qdev));
+}
+
void pci_device_reset(PCIDevice *dev)
{
if (!dev->qdev.info) {
@@ -180,10 +185,6 @@ void pci_device_reset(PCIDevice *dev)
}
qdev_reset(&dev->qdev);
-
- /* TODO: make DeviceInfo::reset call
- pci_device_reset_default() itself. */
- pci_device_reset_default(dev);
}
void pci_bus_reset(PCIBus *bus)
@@ -1614,6 +1615,9 @@ void pci_qdev_register(PCIDeviceInfo *info)
info->qdev.unplug = pci_unplug_device;
info->qdev.exit = pci_unregister_device;
info->qdev.bus_info = &pci_bus_info;
+ if (!info->qdev.reset) {
+ info->qdev.reset = pci_device_reset_default_fn;
+ }
qdev_register(&info->qdev);
}
diff --git a/hw/pcnet.c b/hw/pcnet.c
index 5e63eb5..c894d13 100644
--- a/hw/pcnet.c
+++ b/hw/pcnet.c
@@ -2036,6 +2036,7 @@ static void pci_reset(DeviceState *dev)
PCIPCNetState *d = DO_UPCAST(PCIPCNetState, pci_dev.qdev, dev);
pcnet_h_reset(&d->state);
+ pci_device_reset_default(&d->pci_dev);
}
static PCIDeviceInfo pcnet_info = {
diff --git a/hw/rtl8139.c b/hw/rtl8139.c
index 72e2242..bfa7cde 100644
--- a/hw/rtl8139.c
+++ b/hw/rtl8139.c
@@ -1260,6 +1260,8 @@ static void rtl8139_reset(DeviceState *d)
/* reset tally counters */
RTL8139TallyCounters_clear(&s->tally_counters);
+
+ pci_device_reset_default(&s->dev);
}
static void RTL8139TallyCounters_clear(RTL8139TallyCounters* counters)
diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
index e101fa0..ea8ea6a 100644
--- a/hw/virtio-pci.c
+++ b/hw/virtio-pci.c
@@ -184,6 +184,7 @@ static void virtio_pci_reset(DeviceState *d)
virtio_reset(proxy->vdev);
msix_reset(&proxy->pci_dev);
proxy->bugs = 0;
+ pci_device_reset_default(&proxy->pci_dev);
}
static void virtio_ioport_write(void *opaque, uint32_t addr, uint32_t val)
--
1.6.6.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [Qemu-devel] [PATCH 07/10] pci: fix pci domain registering.
2010-06-17 6:15 [Qemu-devel] [PATCH 00/10] pci: pci to pci bridge clean up and enhancement Isaku Yamahata
` (5 preceding siblings ...)
2010-06-17 6:15 ` [Qemu-devel] [PATCH 06/10] pci: eliminate work around in pci_device_reset() Isaku Yamahata
@ 2010-06-17 6:15 ` Isaku Yamahata
2010-06-17 6:15 ` [Qemu-devel] [PATCH 08/10] pci: remove PCIDeviceInfo::header_type Isaku Yamahata
` (3 subsequent siblings)
10 siblings, 0 replies; 28+ messages in thread
From: Isaku Yamahata @ 2010-06-17 6:15 UTC (permalink / raw)
To: qemu-devel
Cc: jan.kiszka, mst, allen.m.kay, blauwirbel, yamahata, kraxel,
stefano.stabellini, jean.guyader
Only pci host bus must be registered as root bus.
Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
---
hw/pci.c | 8 ++------
hw/pci.h | 1 +
hw/piix_pci.c | 1 +
3 files changed, 4 insertions(+), 6 deletions(-)
diff --git a/hw/pci.c b/hw/pci.c
index 5dee102..162dcd4 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -207,7 +207,7 @@ static void pci_bus_resetfn(void *opaque)
pci_bus_reset(bus);
}
-static void pci_host_bus_register(int domain, PCIBus *bus)
+void pci_host_bus_register(int domain, PCIBus *bus)
{
struct PCIHostBus *host;
host = qemu_mallocz(sizeof(*host));
@@ -254,11 +254,7 @@ void pci_bus_new_inplace(PCIBus *bus, DeviceState *parent,
{
qbus_create_inplace(&bus->qbus, &pci_bus_info, parent, name);
bus->devfn_min = devfn_min;
-
- /* host bridge */
QLIST_INIT(&bus->child);
- pci_host_bus_register(0, bus); /* for now only pci domain 0 is supported */
-
vmstate_register(-1, &vmstate_pcibus, bus);
qemu_register_reset(pci_bus_resetfn, bus);
}
@@ -302,6 +298,7 @@ PCIBus *pci_register_bus(DeviceState *parent, const char *name,
PCIBus *bus;
bus = pci_bus_new(parent, name, devfn_min);
+ pci_host_bus_register(0, bus); /* for now only pci domain 0 is supported */
pci_bus_irqs(bus, set_irq, map_irq, irq_opaque, nirq);
return bus;
}
@@ -317,7 +314,6 @@ PCIBus *pci_register_secondary_bus(PCIBus *parent,
bus->map_irq = map_irq;
bus->parent_dev = dev;
- QLIST_INIT(&bus->child);
QLIST_INSERT_HEAD(&parent->child, bus, sibling);
return bus;
diff --git a/hw/pci.h b/hw/pci.h
index 2a2c8ef..10a63e8 100644
--- a/hw/pci.h
+++ b/hw/pci.h
@@ -227,6 +227,7 @@ PCIDevice *pci_nic_init_nofail(NICInfo *nd, const char *default_model,
const char *default_devaddr);
int pci_bus_num(PCIBus *s);
void pci_for_each_device(PCIBus *bus, int bus_num, void (*fn)(PCIBus *bus, PCIDevice *d));
+void pci_host_bus_register(int domain, PCIBus *bus);
PCIBus *pci_find_root_bus(int domain);
int pci_find_domain(const PCIBus *bus);
PCIBus *pci_find_bus(PCIBus *bus, int bus_num);
diff --git a/hw/piix_pci.c b/hw/piix_pci.c
index d14d05e..16645cd 100644
--- a/hw/piix_pci.c
+++ b/hw/piix_pci.c
@@ -227,6 +227,7 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state, int *piix3_devfn, qemu_irq *
dev = qdev_create(NULL, "i440FX-pcihost");
s = FROM_SYSBUS(I440FXState, sysbus_from_qdev(dev));
b = pci_bus_new(&s->busdev.qdev, NULL, 0);
+ pci_host_bus_register(0, b); /* pci domain 0 */
s->bus = b;
qdev_init_nofail(dev);
--
1.6.6.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [Qemu-devel] [PATCH 08/10] pci: remove PCIDeviceInfo::header_type
2010-06-17 6:15 [Qemu-devel] [PATCH 00/10] pci: pci to pci bridge clean up and enhancement Isaku Yamahata
` (6 preceding siblings ...)
2010-06-17 6:15 ` [Qemu-devel] [PATCH 07/10] pci: fix pci domain registering Isaku Yamahata
@ 2010-06-17 6:15 ` Isaku Yamahata
2010-06-17 6:15 ` [Qemu-devel] [PATCH 09/10] pci: set PCI multi-function bit appropriately Isaku Yamahata
` (2 subsequent siblings)
10 siblings, 0 replies; 28+ messages in thread
From: Isaku Yamahata @ 2010-06-17 6:15 UTC (permalink / raw)
To: qemu-devel
Cc: jan.kiszka, mst, allen.m.kay, blauwirbel, yamahata, kraxel,
stefano.stabellini, jean.guyader
replace PCIDeviceInfo::header_type with is_bridge
as suggested by Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
---
hw/apb_pci.c | 2 +-
hw/dec_pci.c | 2 +-
hw/pci.c | 9 ++++-----
hw/pci.h | 8 ++++++--
hw/pci_bridge.c | 6 +++++-
5 files changed, 17 insertions(+), 10 deletions(-)
diff --git a/hw/apb_pci.c b/hw/apb_pci.c
index cb9051b..a1c17b9 100644
--- a/hw/apb_pci.c
+++ b/hw/apb_pci.c
@@ -440,7 +440,7 @@ static PCIDeviceInfo pbm_pci_host_info = {
.qdev.name = "pbm",
.qdev.size = sizeof(PCIDevice),
.init = pbm_pci_host_init,
- .header_type = PCI_HEADER_TYPE_BRIDGE,
+ .is_bridge = true,
};
static SysBusDeviceInfo pbm_host_info = {
diff --git a/hw/dec_pci.c b/hw/dec_pci.c
index 45b5c28..9311c6f 100644
--- a/hw/dec_pci.c
+++ b/hw/dec_pci.c
@@ -100,7 +100,7 @@ static PCIDeviceInfo dec_21154_pci_host_info = {
.qdev.name = "dec-21154",
.qdev.size = sizeof(PCIDevice),
.init = dec_21154_pci_host_init,
- .header_type = PCI_HEADER_TYPE_BRIDGE,
+ .is_bridge = true,
};
static void dec_register_devices(void)
diff --git a/hw/pci.c b/hw/pci.c
index 162dcd4..5316aa5 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -630,7 +630,7 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
const char *name, int devfn,
PCIConfigReadFunc *config_read,
PCIConfigWriteFunc *config_write,
- uint8_t header_type)
+ bool is_bridge)
{
if (devfn < 0) {
for(devfn = bus->devfn_min ; devfn < ARRAY_SIZE(bus->devices);
@@ -652,13 +652,12 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
pci_dev->irq_state = 0;
pci_config_alloc(pci_dev);
- header_type &= ~PCI_HEADER_TYPE_MULTI_FUNCTION;
- if (header_type == PCI_HEADER_TYPE_NORMAL) {
+ if (!is_bridge) {
pci_set_default_subsystem_id(pci_dev);
}
pci_init_cmask(pci_dev);
pci_init_wmask(pci_dev);
- if (header_type == PCI_HEADER_TYPE_BRIDGE) {
+ if (is_bridge) {
pci_init_wmask_bridge(pci_dev);
}
@@ -1575,7 +1574,7 @@ static int pci_qdev_init(DeviceState *qdev, DeviceInfo *base)
devfn = pci_dev->devfn;
pci_dev = do_pci_register_device(pci_dev, bus, base->name, devfn,
info->config_read, info->config_write,
- info->header_type);
+ info->is_bridge);
if (pci_dev == NULL)
return -1;
rc = info->init(pci_dev);
diff --git a/hw/pci.h b/hw/pci.h
index 10a63e8..ef06b27 100644
--- a/hw/pci.h
+++ b/hw/pci.h
@@ -334,8 +334,12 @@ typedef struct {
PCIConfigReadFunc *config_read;
PCIConfigWriteFunc *config_write;
- /* pci config header type */
- uint8_t header_type;
+ /*
+ * pci-to-pci bridge or normal device.
+ * This doesn't mean pci host switch.
+ * When card bus bridge is supported, this would be enhanced.
+ */
+ int is_bridge;
/* pcie stuff */
int is_express; /* is this device pci express? */
diff --git a/hw/pci_bridge.c b/hw/pci_bridge.c
index 1397a11..736a3db 100644
--- a/hw/pci_bridge.c
+++ b/hw/pci_bridge.c
@@ -113,6 +113,10 @@ static int pci_bridge_initfn(PCIDevice *dev)
PCIBridgeInfo *info = DO_UPCAST(PCIBridgeInfo, pci, pci_info);
int rc = 0;
+ dev->config[PCI_HEADER_TYPE] =
+ (dev->config[PCI_HEADER_TYPE] & PCI_HEADER_TYPE_MULTI_FUNCTION) |
+ PCI_HEADER_TYPE_BRIDGE;
+
pci_set_word(dev->config + PCI_STATUS,
PCI_STATUS_66MHZ | PCI_STATUS_FAST_BACK);
pci_config_set_class(dev->config, PCI_CLASS_BRIDGE_PCI);
@@ -153,7 +157,7 @@ void pci_bridge_qdev_register(PCIBridgeInfo *info)
}
info->pci.init = pci_bridge_initfn;
info->pci.exit = pci_bridge_exitfn;
- info->pci.header_type = PCI_HEADER_TYPE_BRIDGE;
+ info->pci.is_bridge = true;
if (!info->pci.config_write) {
info->pci.config_write = pci_bridge_write_config;
}
--
1.6.6.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [Qemu-devel] [PATCH 09/10] pci: set PCI multi-function bit appropriately.
2010-06-17 6:15 [Qemu-devel] [PATCH 00/10] pci: pci to pci bridge clean up and enhancement Isaku Yamahata
` (7 preceding siblings ...)
2010-06-17 6:15 ` [Qemu-devel] [PATCH 08/10] pci: remove PCIDeviceInfo::header_type Isaku Yamahata
@ 2010-06-17 6:15 ` Isaku Yamahata
2010-06-17 9:37 ` [Qemu-devel] " Michael S. Tsirkin
2010-06-17 6:15 ` [Qemu-devel] [PATCH 10/10] pci: don't overwrite multi functio bit in pci header type Isaku Yamahata
2010-06-17 10:02 ` [Qemu-devel] Re: [PATCH 00/10] pci: pci to pci bridge clean up and enhancement Michael S. Tsirkin
10 siblings, 1 reply; 28+ messages in thread
From: Isaku Yamahata @ 2010-06-17 6:15 UTC (permalink / raw)
To: qemu-devel
Cc: jan.kiszka, mst, allen.m.kay, blauwirbel, yamahata, kraxel,
stefano.stabellini, jean.guyader
set PCI multi-function bit appropriately.
Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
---
changes v1 -> v2:
don't set header type register in configuration space.
---
hw/pci.c | 25 +++++++++++++++++++++++++
1 files changed, 25 insertions(+), 0 deletions(-)
diff --git a/hw/pci.c b/hw/pci.c
index 5316aa5..ee391dc 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -607,6 +607,30 @@ static void pci_init_wmask_bridge(PCIDevice *d)
pci_set_word(d->wmask + PCI_BRIDGE_CONTROL, 0xffff);
}
+static void pci_init_multifunction(PCIBus *bus, PCIDevice *dev)
+{
+ uint8_t slot = PCI_SLOT(dev->devfn);
+ uint8_t func_max = 8;
+ uint8_t func;
+
+ for (func = 0; func < func_max; ++func) {
+ if (bus->devices[PCI_DEVFN(slot, func)]) {
+ break;
+ }
+ }
+ if (func == func_max) {
+ return;
+ }
+
+ for (func = 0; func < func_max; ++func) {
+ if (bus->devices[PCI_DEVFN(slot, func)]) {
+ bus->devices[PCI_DEVFN(slot, func)]->config[PCI_HEADER_TYPE] |=
+ PCI_HEADER_TYPE_MULTI_FUNCTION;
+ }
+ }
+ dev->config[PCI_HEADER_TYPE] |= PCI_HEADER_TYPE_MULTI_FUNCTION;
+}
+
static void pci_config_alloc(PCIDevice *pci_dev)
{
int config_size = pci_config_size(pci_dev);
@@ -660,6 +684,7 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
if (is_bridge) {
pci_init_wmask_bridge(pci_dev);
}
+ pci_init_multifunction(bus, pci_dev);
if (!config_read)
config_read = pci_default_read_config;
--
1.6.6.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [Qemu-devel] Re: [PATCH 09/10] pci: set PCI multi-function bit appropriately.
2010-06-17 6:15 ` [Qemu-devel] [PATCH 09/10] pci: set PCI multi-function bit appropriately Isaku Yamahata
@ 2010-06-17 9:37 ` Michael S. Tsirkin
2010-06-18 2:40 ` Isaku Yamahata
0 siblings, 1 reply; 28+ messages in thread
From: Michael S. Tsirkin @ 2010-06-17 9:37 UTC (permalink / raw)
To: Isaku Yamahata
Cc: stefano.stabellini, jan.kiszka, allen.m.kay, qemu-devel,
blauwirbel, kraxel, jean.guyader
On Thu, Jun 17, 2010 at 03:15:51PM +0900, Isaku Yamahata wrote:
> set PCI multi-function bit appropriately.
>
> Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
>
> ---
> changes v1 -> v2:
> don't set header type register in configuration space.
> ---
> hw/pci.c | 25 +++++++++++++++++++++++++
> 1 files changed, 25 insertions(+), 0 deletions(-)
>
> diff --git a/hw/pci.c b/hw/pci.c
> index 5316aa5..ee391dc 100644
> --- a/hw/pci.c
> +++ b/hw/pci.c
> @@ -607,6 +607,30 @@ static void pci_init_wmask_bridge(PCIDevice *d)
> pci_set_word(d->wmask + PCI_BRIDGE_CONTROL, 0xffff);
> }
>
> +static void pci_init_multifunction(PCIBus *bus, PCIDevice *dev)
> +{
> + uint8_t slot = PCI_SLOT(dev->devfn);
> + uint8_t func_max = 8;
enum or define would be better
> + uint8_t func;
If I understand correctly what this does, it goes over
other functions of the same device, and sets the MULTI_FUNCTION bit
for them if there's more than one function.
Instead, why don't we just set PCI_HEADER_TYPE_MULTI_FUNCTION
in relevant devices?
> +
> + for (func = 0; func < func_max; ++func) {
> + if (bus->devices[PCI_DEVFN(slot, func)]) {
> + break;
> + }
> + }
> + if (func == func_max) {
> + return;
> + }
> +
The above only works if the function is called before
device is added to bus.
> + for (func = 0; func < func_max; ++func) {
> + if (bus->devices[PCI_DEVFN(slot, func)]) {
> + bus->devices[PCI_DEVFN(slot, func)]->config[PCI_HEADER_TYPE] |=
> + PCI_HEADER_TYPE_MULTI_FUNCTION;
> + }
> + }
> + dev->config[PCI_HEADER_TYPE] |= PCI_HEADER_TYPE_MULTI_FUNCTION;
Isn't the bit set above already?
> +}
> +
> static void pci_config_alloc(PCIDevice *pci_dev)
> {
> int config_size = pci_config_size(pci_dev);
> @@ -660,6 +684,7 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
> if (is_bridge) {
> pci_init_wmask_bridge(pci_dev);
> }
> + pci_init_multifunction(bus, pci_dev);
>
> if (!config_read)
> config_read = pci_default_read_config;
> --
> 1.6.6.1
^ permalink raw reply [flat|nested] 28+ messages in thread* [Qemu-devel] Re: [PATCH 09/10] pci: set PCI multi-function bit appropriately.
2010-06-17 9:37 ` [Qemu-devel] " Michael S. Tsirkin
@ 2010-06-18 2:40 ` Isaku Yamahata
2010-06-18 12:44 ` Michael S. Tsirkin
0 siblings, 1 reply; 28+ messages in thread
From: Isaku Yamahata @ 2010-06-18 2:40 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: stefano.stabellini, jan.kiszka, allen.m.kay, qemu-devel,
blauwirbel, kraxel, jean.guyader
On Thu, Jun 17, 2010 at 12:37:21PM +0300, Michael S. Tsirkin wrote:
> On Thu, Jun 17, 2010 at 03:15:51PM +0900, Isaku Yamahata wrote:
> > set PCI multi-function bit appropriately.
> >
> > Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
> >
> > ---
> > changes v1 -> v2:
> > don't set header type register in configuration space.
> > ---
> > hw/pci.c | 25 +++++++++++++++++++++++++
> > 1 files changed, 25 insertions(+), 0 deletions(-)
> >
> > diff --git a/hw/pci.c b/hw/pci.c
> > index 5316aa5..ee391dc 100644
> > --- a/hw/pci.c
> > +++ b/hw/pci.c
> > @@ -607,6 +607,30 @@ static void pci_init_wmask_bridge(PCIDevice *d)
> > pci_set_word(d->wmask + PCI_BRIDGE_CONTROL, 0xffff);
> > }
> >
> > +static void pci_init_multifunction(PCIBus *bus, PCIDevice *dev)
> > +{
> > + uint8_t slot = PCI_SLOT(dev->devfn);
> > + uint8_t func_max = 8;
>
> enum or define would be better
Will fix.
> > + uint8_t func;
>
> If I understand correctly what this does, it goes over
> other functions of the same device, and sets the MULTI_FUNCTION bit
> for them if there's more than one function.
> Instead, why don't we just set PCI_HEADER_TYPE_MULTI_FUNCTION
> in relevant devices?
pci address, devfn ,is exported to users as addr property,
so users can populate pci function(PCIDevice in qemu)
at arbitrary devfn.
It means each function(PCIDevice) don't know whether pci device
(PCIDevice[8]) is multi function or not.
So I chose to handle it in pci generic layer.
It can be argued that it's user operation fault and that
the missing part is validation checks to catch such user errors.
But I prefer more flexible and more user friendly way.
> > +
> > + for (func = 0; func < func_max; ++func) {
> > + if (bus->devices[PCI_DEVFN(slot, func)]) {
> > + break;
> > + }
> > + }
> > + if (func == func_max) {
> > + return;
> > + }
> > +
>
> The above only works if the function is called before
> device is added to bus.
That's right. This function is called before bus->devices[devfn] = dev.
>
> > + for (func = 0; func < func_max; ++func) {
> > + if (bus->devices[PCI_DEVFN(slot, func)]) {
> > + bus->devices[PCI_DEVFN(slot, func)]->config[PCI_HEADER_TYPE] |=
> > + PCI_HEADER_TYPE_MULTI_FUNCTION;
> > + }
> > + }
> > + dev->config[PCI_HEADER_TYPE] |= PCI_HEADER_TYPE_MULTI_FUNCTION;
>
> Isn't the bit set above already?
>
> > +}
> > +
> > static void pci_config_alloc(PCIDevice *pci_dev)
> > {
> > int config_size = pci_config_size(pci_dev);
> > @@ -660,6 +684,7 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
> > if (is_bridge) {
> > pci_init_wmask_bridge(pci_dev);
> > }
> > + pci_init_multifunction(bus, pci_dev);
> >
> > if (!config_read)
> > config_read = pci_default_read_config;
> > --
> > 1.6.6.1
>
--
yamahata
^ permalink raw reply [flat|nested] 28+ messages in thread* [Qemu-devel] Re: [PATCH 09/10] pci: set PCI multi-function bit appropriately.
2010-06-18 2:40 ` Isaku Yamahata
@ 2010-06-18 12:44 ` Michael S. Tsirkin
2010-06-18 13:38 ` Isaku Yamahata
0 siblings, 1 reply; 28+ messages in thread
From: Michael S. Tsirkin @ 2010-06-18 12:44 UTC (permalink / raw)
To: Isaku Yamahata
Cc: stefano.stabellini, jan.kiszka, allen.m.kay, qemu-devel,
blauwirbel, kraxel, jean.guyader
On Fri, Jun 18, 2010 at 11:40:57AM +0900, Isaku Yamahata wrote:
> On Thu, Jun 17, 2010 at 12:37:21PM +0300, Michael S. Tsirkin wrote:
> > On Thu, Jun 17, 2010 at 03:15:51PM +0900, Isaku Yamahata wrote:
> > > set PCI multi-function bit appropriately.
> > >
> > > Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
> > >
> > > ---
> > > changes v1 -> v2:
> > > don't set header type register in configuration space.
> > > ---
> > > hw/pci.c | 25 +++++++++++++++++++++++++
> > > 1 files changed, 25 insertions(+), 0 deletions(-)
> > >
> > > diff --git a/hw/pci.c b/hw/pci.c
> > > index 5316aa5..ee391dc 100644
> > > --- a/hw/pci.c
> > > +++ b/hw/pci.c
> > > @@ -607,6 +607,30 @@ static void pci_init_wmask_bridge(PCIDevice *d)
> > > pci_set_word(d->wmask + PCI_BRIDGE_CONTROL, 0xffff);
> > > }
> > >
> > > +static void pci_init_multifunction(PCIBus *bus, PCIDevice *dev)
> > > +{
> > > + uint8_t slot = PCI_SLOT(dev->devfn);
> > > + uint8_t func_max = 8;
> >
> > enum or define would be better
>
> Will fix.
>
>
> > > + uint8_t func;
> >
> > If I understand correctly what this does, it goes over
> > other functions of the same device, and sets the MULTI_FUNCTION bit
> > for them if there's more than one function.
> > Instead, why don't we just set PCI_HEADER_TYPE_MULTI_FUNCTION
> > in relevant devices?
>
> pci address, devfn ,is exported to users as addr property,
> so users can populate pci function(PCIDevice in qemu)
> at arbitrary devfn.
> It means each function(PCIDevice) don't know whether pci device
> (PCIDevice[8]) is multi function or not.
> So I chose to handle it in pci generic layer.
>
> It can be argued that it's user operation fault and that
> the missing part is validation checks to catch such user errors.
Exactly. Another part that is missing is a way to hotplug
a multifunction device.
OTOH I think that hotplug of separate functions has no chance to work,
so users are better off getting an error.
> But I prefer more flexible and more user friendly way.
I think that most users would only add many functions
to a device as a result of an error.
If we really want the ability to put unrelated devices
as functions in a single one, let's just add
a 'multifunction' qdev property, and validate that
it is set appropriately.
>
> > > +
> > > + for (func = 0; func < func_max; ++func) {
> > > + if (bus->devices[PCI_DEVFN(slot, func)]) {
> > > + break;
> > > + }
> > > + }
> > > + if (func == func_max) {
> > > + return;
> > > + }
> > > +
> >
> > The above only works if the function is called before
> > device is added to bus.
>
> That's right. This function is called before bus->devices[devfn] = dev.
This needs a comment.
> >
> > > + for (func = 0; func < func_max; ++func) {
> > > + if (bus->devices[PCI_DEVFN(slot, func)]) {
> > > + bus->devices[PCI_DEVFN(slot, func)]->config[PCI_HEADER_TYPE] |=
> > > + PCI_HEADER_TYPE_MULTI_FUNCTION;
> > > + }
> > > + }
> > > + dev->config[PCI_HEADER_TYPE] |= PCI_HEADER_TYPE_MULTI_FUNCTION;
> >
> > Isn't the bit set above already?
> >
> > > +}
> > > +
> > > static void pci_config_alloc(PCIDevice *pci_dev)
> > > {
> > > int config_size = pci_config_size(pci_dev);
> > > @@ -660,6 +684,7 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
> > > if (is_bridge) {
> > > pci_init_wmask_bridge(pci_dev);
> > > }
> > > + pci_init_multifunction(bus, pci_dev);
> > >
> > > if (!config_read)
> > > config_read = pci_default_read_config;
> > > --
> > > 1.6.6.1
> >
>
> --
> yamahata
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [Qemu-devel] Re: [PATCH 09/10] pci: set PCI multi-function bit appropriately.
2010-06-18 12:44 ` Michael S. Tsirkin
@ 2010-06-18 13:38 ` Isaku Yamahata
2010-06-18 14:59 ` Michael S. Tsirkin
` (2 more replies)
0 siblings, 3 replies; 28+ messages in thread
From: Isaku Yamahata @ 2010-06-18 13:38 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: stefano.stabellini, jan.kiszka, allen.m.kay, qemu-devel,
blauwirbel, kraxel, jean.guyader
On Fri, Jun 18, 2010 at 03:44:04PM +0300, Michael S. Tsirkin wrote:
> > > If I understand correctly what this does, it goes over
> > > other functions of the same device, and sets the MULTI_FUNCTION bit
> > > for them if there's more than one function.
> > > Instead, why don't we just set PCI_HEADER_TYPE_MULTI_FUNCTION
> > > in relevant devices?
> >
> > pci address, devfn ,is exported to users as addr property,
> > so users can populate pci function(PCIDevice in qemu)
> > at arbitrary devfn.
> > It means each function(PCIDevice) don't know whether pci device
> > (PCIDevice[8]) is multi function or not.
> > So I chose to handle it in pci generic layer.
> >
> > It can be argued that it's user operation fault and that
> > the missing part is validation checks to catch such user errors.
>
> Exactly. Another part that is missing is a way to hotplug
> a multifunction device.
Yes, multi function hot plug is also on my wish list.
> OTOH I think that hotplug of separate functions has no chance to work,
> so users are better off getting an error.
>
> > But I prefer more flexible and more user friendly way.
>
> I think that most users would only add many functions
> to a device as a result of an error.
>
> If we really want the ability to put unrelated devices
> as functions in a single one, let's just add
> a 'multifunction' qdev property, and validate that
> it is set appropriately.
I think "unrelated" is policy. There is no obvious way to determine
which functions can be in a same device.
For example, popular chipset contains isa bridge, ide controller,
usb controller, sound and modem in a single device as functions.
It's up to hardware designer policy which functions are grouped into
a device.
So qemu should be able to populate any function in a device,
and leave such policy check to higher level tool like libvirt/virt-manager.
--
yamahata
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [Qemu-devel] Re: [PATCH 09/10] pci: set PCI multi-function bit appropriately.
2010-06-18 13:38 ` Isaku Yamahata
@ 2010-06-18 14:59 ` Michael S. Tsirkin
2010-06-18 15:22 ` Jamie Lokier
2010-06-20 10:03 ` Michael S. Tsirkin
2 siblings, 0 replies; 28+ messages in thread
From: Michael S. Tsirkin @ 2010-06-18 14:59 UTC (permalink / raw)
To: Isaku Yamahata
Cc: stefano.stabellini, jan.kiszka, allen.m.kay, qemu-devel,
blauwirbel, kraxel, jean.guyader
On Fri, Jun 18, 2010 at 10:38:20PM +0900, Isaku Yamahata wrote:
> On Fri, Jun 18, 2010 at 03:44:04PM +0300, Michael S. Tsirkin wrote:
> > > > If I understand correctly what this does, it goes over
> > > > other functions of the same device, and sets the MULTI_FUNCTION bit
> > > > for them if there's more than one function.
> > > > Instead, why don't we just set PCI_HEADER_TYPE_MULTI_FUNCTION
> > > > in relevant devices?
> > >
> > > pci address, devfn ,is exported to users as addr property,
> > > so users can populate pci function(PCIDevice in qemu)
> > > at arbitrary devfn.
> > > It means each function(PCIDevice) don't know whether pci device
> > > (PCIDevice[8]) is multi function or not.
> > > So I chose to handle it in pci generic layer.
> > >
> > > It can be argued that it's user operation fault and that
> > > the missing part is validation checks to catch such user errors.
> >
> > Exactly. Another part that is missing is a way to hotplug
> > a multifunction device.
>
> Yes, multi function hot plug is also on my wish list.
>
>
> > OTOH I think that hotplug of separate functions has no chance to work,
> > so users are better off getting an error.
> >
> > > But I prefer more flexible and more user friendly way.
> >
> > I think that most users would only add many functions
> > to a device as a result of an error.
> >
> > If we really want the ability to put unrelated devices
> > as functions in a single one, let's just add
> > a 'multifunction' qdev property, and validate that
> > it is set appropriately.
>
> I think "unrelated" is policy. There is no obvious way to determine
> which functions can be in a same device.
> For example, popular chipset contains isa bridge, ide controller,
> usb controller, sound and modem in a single device as functions.
> It's up to hardware designer policy which functions are grouped into
> a device.
>
> So qemu should be able to populate any function in a device,
> and leave such policy check to higher level tool like libvirt/virt-manager.
Fine. But tweaking one device's config when another one is added does not
lead to robust code. Let's just require that users declare devices as
multifunction, and verify this.
--
MST
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Qemu-devel] Re: [PATCH 09/10] pci: set PCI multi-function bit appropriately.
2010-06-18 13:38 ` Isaku Yamahata
2010-06-18 14:59 ` Michael S. Tsirkin
@ 2010-06-18 15:22 ` Jamie Lokier
2010-06-20 10:03 ` Michael S. Tsirkin
2 siblings, 0 replies; 28+ messages in thread
From: Jamie Lokier @ 2010-06-18 15:22 UTC (permalink / raw)
To: Isaku Yamahata
Cc: stefano.stabellini, jan.kiszka, Michael S. Tsirkin, allen.m.kay,
qemu-devel, blauwirbel, kraxel, jean.guyader
Isaku Yamahata wrote:
> On Fri, Jun 18, 2010 at 03:44:04PM +0300, Michael S. Tsirkin wrote:
> > If we really want the ability to put unrelated devices
> > as functions in a single one, let's just add
> > a 'multifunction' qdev property, and validate that
> > it is set appropriately.
>
> I think "unrelated" is policy. There is no obvious way to determine
> which functions can be in a same device.
> For example, popular chipset contains isa bridge, ide controller,
> usb controller, sound and modem in a single device as functions.
> It's up to hardware designer policy which functions are grouped into
> a device.
In hardware terms, quad-port ethernet controllers often present
themselves as a PCI bridge with four independent PCI ethernet
controllers, so they work with standard drivers. Even though all four
are in a single chip. Some USB devices do the same, presenting a
small bulk storage device to ship windows drivers [:roll-eyes: ;-)]
alongside the actual device.
-- Jamie
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Qemu-devel] Re: [PATCH 09/10] pci: set PCI multi-function bit appropriately.
2010-06-18 13:38 ` Isaku Yamahata
2010-06-18 14:59 ` Michael S. Tsirkin
2010-06-18 15:22 ` Jamie Lokier
@ 2010-06-20 10:03 ` Michael S. Tsirkin
2 siblings, 0 replies; 28+ messages in thread
From: Michael S. Tsirkin @ 2010-06-20 10:03 UTC (permalink / raw)
To: Isaku Yamahata
Cc: stefano.stabellini, jan.kiszka, allen.m.kay, qemu-devel,
blauwirbel, kraxel, jean.guyader
On Fri, Jun 18, 2010 at 10:38:20PM +0900, Isaku Yamahata wrote:
> On Fri, Jun 18, 2010 at 03:44:04PM +0300, Michael S. Tsirkin wrote:
> > > > If I understand correctly what this does, it goes over
> > > > other functions of the same device, and sets the MULTI_FUNCTION bit
> > > > for them if there's more than one function.
> > > > Instead, why don't we just set PCI_HEADER_TYPE_MULTI_FUNCTION
> > > > in relevant devices?
> > >
> > > pci address, devfn ,is exported to users as addr property,
> > > so users can populate pci function(PCIDevice in qemu)
> > > at arbitrary devfn.
> > > It means each function(PCIDevice) don't know whether pci device
> > > (PCIDevice[8]) is multi function or not.
> > > So I chose to handle it in pci generic layer.
> > >
> > > It can be argued that it's user operation fault and that
> > > the missing part is validation checks to catch such user errors.
> >
> > Exactly. Another part that is missing is a way to hotplug
> > a multifunction device.
>
> Yes, multi function hot plug is also on my wish list.
>
>
> > OTOH I think that hotplug of separate functions has no chance to work,
> > so users are better off getting an error.
> >
> > > But I prefer more flexible and more user friendly way.
> >
> > I think that most users would only add many functions
> > to a device as a result of an error.
> >
> > If we really want the ability to put unrelated devices
> > as functions in a single one, let's just add
> > a 'multifunction' qdev property, and validate that
> > it is set appropriately.
>
> I think "unrelated" is policy. There is no obvious way to determine
> which functions can be in a same device.
> For example, popular chipset contains isa bridge, ide controller,
> usb controller, sound and modem in a single device as functions.
> It's up to hardware designer policy which functions are grouped into
> a device.
>
> So qemu should be able to populate any function in a device,
> and leave such policy check to higher level tool like libvirt/virt-manager.
Fine. But let's keep it simple: ask user what multifunction bit is,
and not tweak header for device A when device B is added.
> --
> yamahata
^ permalink raw reply [flat|nested] 28+ messages in thread
* [Qemu-devel] [PATCH 10/10] pci: don't overwrite multi functio bit in pci header type.
2010-06-17 6:15 [Qemu-devel] [PATCH 00/10] pci: pci to pci bridge clean up and enhancement Isaku Yamahata
` (8 preceding siblings ...)
2010-06-17 6:15 ` [Qemu-devel] [PATCH 09/10] pci: set PCI multi-function bit appropriately Isaku Yamahata
@ 2010-06-17 6:15 ` Isaku Yamahata
2010-06-17 9:41 ` [Qemu-devel] " Michael S. Tsirkin
2010-06-17 10:02 ` [Qemu-devel] Re: [PATCH 00/10] pci: pci to pci bridge clean up and enhancement Michael S. Tsirkin
10 siblings, 1 reply; 28+ messages in thread
From: Isaku Yamahata @ 2010-06-17 6:15 UTC (permalink / raw)
To: qemu-devel
Cc: jan.kiszka, mst, allen.m.kay, blauwirbel, yamahata, kraxel,
stefano.stabellini, jean.guyader
Don't overwrite pci header type.
Otherwise, multi function bit which pci_init_header_type() sets
appropriately is lost.
Anyway PCI_HEADER_TYPE_NORMAL is zero, so it is unnecessary to zero
which is already zero cleared.
Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
---
changes v1 -> v2:
- set header type of bridge type in pci_bridge_initfn().
- dropped ugly hunk in apb_pci.c.
---
hw/ac97.c | 1 -
hw/acpi_piix4.c | 1 -
hw/apb_pci.c | 2 --
hw/grackle_pci.c | 1 -
hw/ide/cmd646.c | 1 -
hw/ide/piix.c | 1 -
hw/macio.c | 1 -
hw/ne2000.c | 1 -
hw/openpic.c | 1 -
hw/pcnet.c | 1 -
hw/piix4.c | 3 +--
hw/piix_pci.c | 4 +---
hw/prep_pci.c | 1 -
hw/rtl8139.c | 1 -
hw/sun4u.c | 1 -
hw/unin_pci.c | 4 ----
hw/usb-uhci.c | 1 -
hw/vga-pci.c | 1 -
hw/virtio-pci.c | 1 -
hw/vmware_vga.c | 1 -
hw/wdt_i6300esb.c | 1 -
21 files changed, 2 insertions(+), 28 deletions(-)
diff --git a/hw/ac97.c b/hw/ac97.c
index 4319bc8..d71072d 100644
--- a/hw/ac97.c
+++ b/hw/ac97.c
@@ -1295,7 +1295,6 @@ static int ac97_initfn (PCIDevice *dev)
c[PCI_REVISION_ID] = 0x01; /* rid revision ro */
c[PCI_CLASS_PROG] = 0x00; /* pi programming interface ro */
pci_config_set_class (c, PCI_CLASS_MULTIMEDIA_AUDIO); /* ro */
- c[PCI_HEADER_TYPE] = PCI_HEADER_TYPE_NORMAL; /* headtyp header type ro */
/* TODO set when bar is registered. no need to override. */
/* nabmar native audio mixer base address rw */
diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c
index 8d1a628..bfa1d9a 100644
--- a/hw/acpi_piix4.c
+++ b/hw/acpi_piix4.c
@@ -369,7 +369,6 @@ static int piix4_pm_initfn(PCIDevice *dev)
pci_conf[0x08] = 0x03; // revision number
pci_conf[0x09] = 0x00;
pci_config_set_class(pci_conf, PCI_CLASS_BRIDGE_OTHER);
- pci_conf[PCI_HEADER_TYPE] = PCI_HEADER_TYPE_NORMAL; // header_type
pci_conf[0x3d] = 0x01; // interrupt pin 1
pci_conf[0x40] = 0x01; /* PM io base read only bit */
diff --git a/hw/apb_pci.c b/hw/apb_pci.c
index a1c17b9..3b8eda3 100644
--- a/hw/apb_pci.c
+++ b/hw/apb_pci.c
@@ -431,8 +431,6 @@ static int pbm_pci_host_init(PCIDevice *d)
PCI_STATUS_FAST_BACK | PCI_STATUS_66MHZ |
PCI_STATUS_DEVSEL_MEDIUM);
pci_config_set_class(d->config, PCI_CLASS_BRIDGE_HOST);
- pci_set_byte(d->config + PCI_HEADER_TYPE,
- PCI_HEADER_TYPE_NORMAL);
return 0;
}
diff --git a/hw/grackle_pci.c b/hw/grackle_pci.c
index aa0c51b..b3a5f54 100644
--- a/hw/grackle_pci.c
+++ b/hw/grackle_pci.c
@@ -126,7 +126,6 @@ static int grackle_pci_host_init(PCIDevice *d)
d->config[0x08] = 0x00; // revision
d->config[0x09] = 0x01;
pci_config_set_class(d->config, PCI_CLASS_BRIDGE_HOST);
- d->config[PCI_HEADER_TYPE] = PCI_HEADER_TYPE_NORMAL; // header_type
return 0;
}
diff --git a/hw/ide/cmd646.c b/hw/ide/cmd646.c
index 559147f..756ee81 100644
--- a/hw/ide/cmd646.c
+++ b/hw/ide/cmd646.c
@@ -240,7 +240,6 @@ static int pci_cmd646_ide_initfn(PCIDevice *dev)
pci_conf[PCI_CLASS_PROG] = 0x8f;
pci_config_set_class(pci_conf, PCI_CLASS_STORAGE_IDE);
- pci_conf[PCI_HEADER_TYPE] = PCI_HEADER_TYPE_NORMAL; // header_type
pci_conf[0x51] = 0x04; // enable IDE0
if (d->secondary) {
diff --git a/hw/ide/piix.c b/hw/ide/piix.c
index dad6e86..8817915 100644
--- a/hw/ide/piix.c
+++ b/hw/ide/piix.c
@@ -122,7 +122,6 @@ static int pci_piix_ide_initfn(PCIIDEState *d)
pci_conf[PCI_CLASS_PROG] = 0x80; // legacy ATA mode
pci_config_set_class(pci_conf, PCI_CLASS_STORAGE_IDE);
- pci_conf[PCI_HEADER_TYPE] = PCI_HEADER_TYPE_NORMAL; // header_type
qemu_register_reset(piix3_reset, d);
diff --git a/hw/macio.c b/hw/macio.c
index e92e82a..789ca55 100644
--- a/hw/macio.c
+++ b/hw/macio.c
@@ -110,7 +110,6 @@ void macio_init (PCIBus *bus, int device_id, int is_oldworld, int pic_mem_index,
pci_config_set_vendor_id(d->config, PCI_VENDOR_ID_APPLE);
pci_config_set_device_id(d->config, device_id);
pci_config_set_class(d->config, PCI_CLASS_OTHERS << 8);
- d->config[PCI_HEADER_TYPE] = PCI_HEADER_TYPE_NORMAL; // header_type
d->config[0x3d] = 0x01; // interrupt on pin 1
diff --git a/hw/ne2000.c b/hw/ne2000.c
index 78fe14f..126e7cf 100644
--- a/hw/ne2000.c
+++ b/hw/ne2000.c
@@ -723,7 +723,6 @@ static int pci_ne2000_init(PCIDevice *pci_dev)
pci_config_set_vendor_id(pci_conf, PCI_VENDOR_ID_REALTEK);
pci_config_set_device_id(pci_conf, PCI_DEVICE_ID_REALTEK_8029);
pci_config_set_class(pci_conf, PCI_CLASS_NETWORK_ETHERNET);
- pci_conf[PCI_HEADER_TYPE] = PCI_HEADER_TYPE_NORMAL; // header_type
/* TODO: RST# value should be 0. PCI spec 6.2.4 */
pci_conf[PCI_INTERRUPT_PIN] = 1; // interrupt pin 0
diff --git a/hw/openpic.c b/hw/openpic.c
index ac21993..2bbf787 100644
--- a/hw/openpic.c
+++ b/hw/openpic.c
@@ -1194,7 +1194,6 @@ qemu_irq *openpic_init (PCIBus *bus, int *pmem_index, int nb_cpus,
pci_config_set_vendor_id(pci_conf, PCI_VENDOR_ID_IBM);
pci_config_set_device_id(pci_conf, PCI_DEVICE_ID_IBM_OPENPIC2);
pci_config_set_class(pci_conf, PCI_CLASS_SYSTEM_OTHER); // FIXME?
- pci_conf[PCI_HEADER_TYPE] = PCI_HEADER_TYPE_NORMAL; // header_type
pci_conf[0x3d] = 0x00; // no interrupt pin
/* Register I/O spaces */
diff --git a/hw/pcnet.c b/hw/pcnet.c
index c894d13..e172510 100644
--- a/hw/pcnet.c
+++ b/hw/pcnet.c
@@ -1990,7 +1990,6 @@ static int pci_pcnet_init(PCIDevice *pci_dev)
/* TODO: 0 is the default anyway, no need to set it. */
pci_conf[PCI_CLASS_PROG] = 0x00;
pci_config_set_class(pci_conf, PCI_CLASS_NETWORK_ETHERNET);
- pci_conf[PCI_HEADER_TYPE] = PCI_HEADER_TYPE_NORMAL; // header_type
/* TODO: not necessary, is set when BAR is registered. */
pci_set_long(pci_conf + PCI_BASE_ADDRESS_0, PCI_BASE_ADDRESS_SPACE_IO);
diff --git a/hw/piix4.c b/hw/piix4.c
index f75951b..03926a7 100644
--- a/hw/piix4.c
+++ b/hw/piix4.c
@@ -93,8 +93,7 @@ static int piix4_initfn(PCIDevice *d)
pci_config_set_vendor_id(pci_conf, PCI_VENDOR_ID_INTEL);
pci_config_set_device_id(pci_conf, PCI_DEVICE_ID_INTEL_82371AB_0); // 82371AB/EB/MB PIIX4 PCI-to-ISA bridge
pci_config_set_class(pci_conf, PCI_CLASS_BRIDGE_ISA);
- pci_conf[PCI_HEADER_TYPE] =
- PCI_HEADER_TYPE_NORMAL | PCI_HEADER_TYPE_MULTI_FUNCTION; // header_type = PCI_multifunction, generic
+ pci_conf[PCI_HEADER_TYPE] |= PCI_HEADER_TYPE_MULTI_FUNCTION;
piix4_dev = d;
qemu_register_reset(piix4_reset, d);
diff --git a/hw/piix_pci.c b/hw/piix_pci.c
index 16645cd..0b0c92b 100644
--- a/hw/piix_pci.c
+++ b/hw/piix_pci.c
@@ -208,7 +208,6 @@ static int i440fx_initfn(PCIDevice *dev)
pci_config_set_device_id(d->dev.config, PCI_DEVICE_ID_INTEL_82441);
d->dev.config[0x08] = 0x02; // revision
pci_config_set_class(d->dev.config, PCI_CLASS_BRIDGE_HOST);
- d->dev.config[PCI_HEADER_TYPE] = PCI_HEADER_TYPE_NORMAL; // header_type
d->dev.config[I440FX_SMRAM] = 0x02;
@@ -337,8 +336,7 @@ static int piix3_initfn(PCIDevice *dev)
pci_config_set_vendor_id(pci_conf, PCI_VENDOR_ID_INTEL);
pci_config_set_device_id(pci_conf, PCI_DEVICE_ID_INTEL_82371SB_0); // 82371SB PIIX3 PCI-to-ISA bridge (Step A1)
pci_config_set_class(pci_conf, PCI_CLASS_BRIDGE_ISA);
- pci_conf[PCI_HEADER_TYPE] =
- PCI_HEADER_TYPE_NORMAL | PCI_HEADER_TYPE_MULTI_FUNCTION; // header_type = PCI_multifunction, generic
+ pci_conf[PCI_HEADER_TYPE] |= PCI_HEADER_TYPE_MULTI_FUNCTION;
qemu_register_reset(piix3_reset, d);
return 0;
diff --git a/hw/prep_pci.c b/hw/prep_pci.c
index 144fde0..0c2afe9 100644
--- a/hw/prep_pci.c
+++ b/hw/prep_pci.c
@@ -137,7 +137,6 @@ PCIBus *pci_prep_init(qemu_irq *pic)
pci_config_set_class(d->config, PCI_CLASS_BRIDGE_HOST);
d->config[0x0C] = 0x08; // cache_line_size
d->config[0x0D] = 0x10; // latency_timer
- d->config[PCI_HEADER_TYPE] = PCI_HEADER_TYPE_NORMAL; // header_type
d->config[0x34] = 0x00; // capabilities_pointer
return s->bus;
diff --git a/hw/rtl8139.c b/hw/rtl8139.c
index bfa7cde..83e5db9 100644
--- a/hw/rtl8139.c
+++ b/hw/rtl8139.c
@@ -3363,7 +3363,6 @@ static int pci_rtl8139_init(PCIDevice *dev)
pci_conf[PCI_COMMAND] = PCI_COMMAND_IO | PCI_COMMAND_MASTER;
pci_conf[PCI_REVISION_ID] = RTL8139_PCI_REVID; /* >=0x20 is for 8139C+ */
pci_config_set_class(pci_conf, PCI_CLASS_NETWORK_ETHERNET);
- pci_conf[PCI_HEADER_TYPE] = PCI_HEADER_TYPE_NORMAL;
/* TODO: value should be 0 at RST# */
pci_conf[PCI_INTERRUPT_PIN] = 1; /* interrupt pin 0 */
/* TODO: start of capability list, but no capability
diff --git a/hw/sun4u.c b/hw/sun4u.c
index 40b5f1f..cf5a8c4 100644
--- a/hw/sun4u.c
+++ b/hw/sun4u.c
@@ -562,7 +562,6 @@ pci_ebus_init1(PCIDevice *s)
s->config[0x09] = 0x00; // programming i/f
pci_config_set_class(s->config, PCI_CLASS_BRIDGE_OTHER);
s->config[0x0D] = 0x0a; // latency_timer
- s->config[PCI_HEADER_TYPE] = PCI_HEADER_TYPE_NORMAL; // header_type
pci_register_bar(s, 0, 0x1000000, PCI_BASE_ADDRESS_SPACE_MEMORY,
ebus_mmio_mapfunc);
diff --git a/hw/unin_pci.c b/hw/unin_pci.c
index f0a773d..7b1c94b 100644
--- a/hw/unin_pci.c
+++ b/hw/unin_pci.c
@@ -298,7 +298,6 @@ static int unin_main_pci_host_init(PCIDevice *d)
pci_config_set_class(d->config, PCI_CLASS_BRIDGE_HOST);
d->config[0x0C] = 0x08; // cache_line_size
d->config[0x0D] = 0x10; // latency_timer
- d->config[PCI_HEADER_TYPE] = PCI_HEADER_TYPE_NORMAL; // header_type
d->config[0x34] = 0x00; // capabilities_pointer
return 0;
}
@@ -311,7 +310,6 @@ static int unin_agp_pci_host_init(PCIDevice *d)
pci_config_set_class(d->config, PCI_CLASS_BRIDGE_HOST);
d->config[0x0C] = 0x08; // cache_line_size
d->config[0x0D] = 0x10; // latency_timer
- d->config[PCI_HEADER_TYPE] = PCI_HEADER_TYPE_NORMAL; // header_type
// d->config[0x34] = 0x80; // capabilities_pointer
return 0;
}
@@ -327,7 +325,6 @@ static int u3_agp_pci_host_init(PCIDevice *d)
d->config[0x0C] = 0x08;
/* latency timer */
d->config[0x0D] = 0x10;
- d->config[PCI_HEADER_TYPE] = PCI_HEADER_TYPE_NORMAL;
return 0;
}
@@ -339,7 +336,6 @@ static int unin_internal_pci_host_init(PCIDevice *d)
pci_config_set_class(d->config, PCI_CLASS_BRIDGE_HOST);
d->config[0x0C] = 0x08; // cache_line_size
d->config[0x0D] = 0x10; // latency_timer
- d->config[PCI_HEADER_TYPE] = PCI_HEADER_TYPE_NORMAL; // header_type
d->config[0x34] = 0x00; // capabilities_pointer
return 0;
}
diff --git a/hw/usb-uhci.c b/hw/usb-uhci.c
index 624d55b..058bf59 100644
--- a/hw/usb-uhci.c
+++ b/hw/usb-uhci.c
@@ -1108,7 +1108,6 @@ static int usb_uhci_common_initfn(UHCIState *s)
pci_conf[PCI_REVISION_ID] = 0x01; // revision number
pci_conf[PCI_CLASS_PROG] = 0x00;
pci_config_set_class(pci_conf, PCI_CLASS_SERIAL_USB);
- pci_conf[PCI_HEADER_TYPE] = PCI_HEADER_TYPE_NORMAL; // header_type
/* TODO: reset value should be 0. */
pci_conf[PCI_INTERRUPT_PIN] = 4; // interrupt pin 3
pci_conf[0x60] = 0x10; // release number
diff --git a/hw/vga-pci.c b/hw/vga-pci.c
index eef78ed..2315f70 100644
--- a/hw/vga-pci.c
+++ b/hw/vga-pci.c
@@ -90,7 +90,6 @@ static int pci_vga_initfn(PCIDevice *dev)
pci_config_set_vendor_id(pci_conf, PCI_VENDOR_ID_QEMU);
pci_config_set_device_id(pci_conf, PCI_DEVICE_ID_QEMU_VGA);
pci_config_set_class(pci_conf, PCI_CLASS_DISPLAY_VGA);
- pci_conf[PCI_HEADER_TYPE] = PCI_HEADER_TYPE_NORMAL; // header_type
/* XXX: VGA_RAM_SIZE must be a power of two */
pci_register_bar(&d->dev, 0, VGA_RAM_SIZE,
diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
index ea8ea6a..4c02c28 100644
--- a/hw/virtio-pci.c
+++ b/hw/virtio-pci.c
@@ -507,7 +507,6 @@ static void virtio_init_pci(VirtIOPCIProxy *proxy, VirtIODevice *vdev,
config[0x09] = pif;
pci_config_set_class(config, class_code);
- config[PCI_HEADER_TYPE] = PCI_HEADER_TYPE_NORMAL;
config[0x2c] = vendor & 0xFF;
config[0x2d] = (vendor >> 8) & 0xFF;
diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c
index bf2a699..38fe976 100644
--- a/hw/vmware_vga.c
+++ b/hw/vmware_vga.c
@@ -1246,7 +1246,6 @@ static int pci_vmsvga_initfn(PCIDevice *dev)
pci_config_set_class(s->card.config, PCI_CLASS_DISPLAY_VGA);
s->card.config[PCI_CACHE_LINE_SIZE] = 0x08; /* Cache line size */
s->card.config[PCI_LATENCY_TIMER] = 0x40; /* Latency timer */
- s->card.config[PCI_HEADER_TYPE] = PCI_HEADER_TYPE_NORMAL;
s->card.config[PCI_SUBSYSTEM_VENDOR_ID] = PCI_VENDOR_ID_VMWARE & 0xff;
s->card.config[PCI_SUBSYSTEM_VENDOR_ID + 1] = PCI_VENDOR_ID_VMWARE >> 8;
s->card.config[PCI_SUBSYSTEM_ID] = SVGA_PCI_DEVICE_ID & 0xff;
diff --git a/hw/wdt_i6300esb.c b/hw/wdt_i6300esb.c
index be0e89e..46e1df8 100644
--- a/hw/wdt_i6300esb.c
+++ b/hw/wdt_i6300esb.c
@@ -411,7 +411,6 @@ static int i6300esb_init(PCIDevice *dev)
pci_config_set_vendor_id(pci_conf, PCI_VENDOR_ID_INTEL);
pci_config_set_device_id(pci_conf, PCI_DEVICE_ID_INTEL_ESB_9);
pci_config_set_class(pci_conf, PCI_CLASS_SYSTEM_OTHER);
- pci_conf[PCI_HEADER_TYPE] = 0x00;
pci_register_bar(&d->dev, 0, 0x10,
PCI_BASE_ADDRESS_SPACE_MEMORY, i6300esb_map);
--
1.6.6.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [Qemu-devel] Re: [PATCH 10/10] pci: don't overwrite multi functio bit in pci header type.
2010-06-17 6:15 ` [Qemu-devel] [PATCH 10/10] pci: don't overwrite multi functio bit in pci header type Isaku Yamahata
@ 2010-06-17 9:41 ` Michael S. Tsirkin
0 siblings, 0 replies; 28+ messages in thread
From: Michael S. Tsirkin @ 2010-06-17 9:41 UTC (permalink / raw)
To: Isaku Yamahata
Cc: stefano.stabellini, jan.kiszka, allen.m.kay, qemu-devel,
blauwirbel, kraxel, jean.guyader
On Thu, Jun 17, 2010 at 03:15:52PM +0900, Isaku Yamahata wrote:
> Don't overwrite pci header type.
> Otherwise, multi function bit which pci_init_header_type() sets
> appropriately is lost.
> Anyway PCI_HEADER_TYPE_NORMAL is zero, so it is unnecessary to zero
> which is already zero cleared.
>
> Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
All this churn will need quite a bit of testing.
Please record what was tested in the commit message.
If we are doing it, let's clean other registers which
sets registers to default values?
> ---
> changes v1 -> v2:
> - set header type of bridge type in pci_bridge_initfn().
> - dropped ugly hunk in apb_pci.c.
> ---
> hw/ac97.c | 1 -
> hw/acpi_piix4.c | 1 -
> hw/apb_pci.c | 2 --
> hw/grackle_pci.c | 1 -
> hw/ide/cmd646.c | 1 -
> hw/ide/piix.c | 1 -
> hw/macio.c | 1 -
> hw/ne2000.c | 1 -
> hw/openpic.c | 1 -
> hw/pcnet.c | 1 -
> hw/piix4.c | 3 +--
> hw/piix_pci.c | 4 +---
> hw/prep_pci.c | 1 -
> hw/rtl8139.c | 1 -
> hw/sun4u.c | 1 -
> hw/unin_pci.c | 4 ----
> hw/usb-uhci.c | 1 -
> hw/vga-pci.c | 1 -
> hw/virtio-pci.c | 1 -
> hw/vmware_vga.c | 1 -
> hw/wdt_i6300esb.c | 1 -
> 21 files changed, 2 insertions(+), 28 deletions(-)
>
> diff --git a/hw/ac97.c b/hw/ac97.c
> index 4319bc8..d71072d 100644
> --- a/hw/ac97.c
> +++ b/hw/ac97.c
> @@ -1295,7 +1295,6 @@ static int ac97_initfn (PCIDevice *dev)
> c[PCI_REVISION_ID] = 0x01; /* rid revision ro */
> c[PCI_CLASS_PROG] = 0x00; /* pi programming interface ro */
> pci_config_set_class (c, PCI_CLASS_MULTIMEDIA_AUDIO); /* ro */
> - c[PCI_HEADER_TYPE] = PCI_HEADER_TYPE_NORMAL; /* headtyp header type ro */
>
> /* TODO set when bar is registered. no need to override. */
> /* nabmar native audio mixer base address rw */
> diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c
> index 8d1a628..bfa1d9a 100644
> --- a/hw/acpi_piix4.c
> +++ b/hw/acpi_piix4.c
> @@ -369,7 +369,6 @@ static int piix4_pm_initfn(PCIDevice *dev)
> pci_conf[0x08] = 0x03; // revision number
> pci_conf[0x09] = 0x00;
> pci_config_set_class(pci_conf, PCI_CLASS_BRIDGE_OTHER);
> - pci_conf[PCI_HEADER_TYPE] = PCI_HEADER_TYPE_NORMAL; // header_type
> pci_conf[0x3d] = 0x01; // interrupt pin 1
>
> pci_conf[0x40] = 0x01; /* PM io base read only bit */
> diff --git a/hw/apb_pci.c b/hw/apb_pci.c
> index a1c17b9..3b8eda3 100644
> --- a/hw/apb_pci.c
> +++ b/hw/apb_pci.c
> @@ -431,8 +431,6 @@ static int pbm_pci_host_init(PCIDevice *d)
> PCI_STATUS_FAST_BACK | PCI_STATUS_66MHZ |
> PCI_STATUS_DEVSEL_MEDIUM);
> pci_config_set_class(d->config, PCI_CLASS_BRIDGE_HOST);
> - pci_set_byte(d->config + PCI_HEADER_TYPE,
> - PCI_HEADER_TYPE_NORMAL);
> return 0;
> }
>
> diff --git a/hw/grackle_pci.c b/hw/grackle_pci.c
> index aa0c51b..b3a5f54 100644
> --- a/hw/grackle_pci.c
> +++ b/hw/grackle_pci.c
> @@ -126,7 +126,6 @@ static int grackle_pci_host_init(PCIDevice *d)
> d->config[0x08] = 0x00; // revision
> d->config[0x09] = 0x01;
> pci_config_set_class(d->config, PCI_CLASS_BRIDGE_HOST);
> - d->config[PCI_HEADER_TYPE] = PCI_HEADER_TYPE_NORMAL; // header_type
> return 0;
> }
>
> diff --git a/hw/ide/cmd646.c b/hw/ide/cmd646.c
> index 559147f..756ee81 100644
> --- a/hw/ide/cmd646.c
> +++ b/hw/ide/cmd646.c
> @@ -240,7 +240,6 @@ static int pci_cmd646_ide_initfn(PCIDevice *dev)
> pci_conf[PCI_CLASS_PROG] = 0x8f;
>
> pci_config_set_class(pci_conf, PCI_CLASS_STORAGE_IDE);
> - pci_conf[PCI_HEADER_TYPE] = PCI_HEADER_TYPE_NORMAL; // header_type
>
> pci_conf[0x51] = 0x04; // enable IDE0
> if (d->secondary) {
> diff --git a/hw/ide/piix.c b/hw/ide/piix.c
> index dad6e86..8817915 100644
> --- a/hw/ide/piix.c
> +++ b/hw/ide/piix.c
> @@ -122,7 +122,6 @@ static int pci_piix_ide_initfn(PCIIDEState *d)
>
> pci_conf[PCI_CLASS_PROG] = 0x80; // legacy ATA mode
> pci_config_set_class(pci_conf, PCI_CLASS_STORAGE_IDE);
> - pci_conf[PCI_HEADER_TYPE] = PCI_HEADER_TYPE_NORMAL; // header_type
>
> qemu_register_reset(piix3_reset, d);
>
> diff --git a/hw/macio.c b/hw/macio.c
> index e92e82a..789ca55 100644
> --- a/hw/macio.c
> +++ b/hw/macio.c
> @@ -110,7 +110,6 @@ void macio_init (PCIBus *bus, int device_id, int is_oldworld, int pic_mem_index,
> pci_config_set_vendor_id(d->config, PCI_VENDOR_ID_APPLE);
> pci_config_set_device_id(d->config, device_id);
> pci_config_set_class(d->config, PCI_CLASS_OTHERS << 8);
> - d->config[PCI_HEADER_TYPE] = PCI_HEADER_TYPE_NORMAL; // header_type
>
> d->config[0x3d] = 0x01; // interrupt on pin 1
>
> diff --git a/hw/ne2000.c b/hw/ne2000.c
> index 78fe14f..126e7cf 100644
> --- a/hw/ne2000.c
> +++ b/hw/ne2000.c
> @@ -723,7 +723,6 @@ static int pci_ne2000_init(PCIDevice *pci_dev)
> pci_config_set_vendor_id(pci_conf, PCI_VENDOR_ID_REALTEK);
> pci_config_set_device_id(pci_conf, PCI_DEVICE_ID_REALTEK_8029);
> pci_config_set_class(pci_conf, PCI_CLASS_NETWORK_ETHERNET);
> - pci_conf[PCI_HEADER_TYPE] = PCI_HEADER_TYPE_NORMAL; // header_type
> /* TODO: RST# value should be 0. PCI spec 6.2.4 */
> pci_conf[PCI_INTERRUPT_PIN] = 1; // interrupt pin 0
>
> diff --git a/hw/openpic.c b/hw/openpic.c
> index ac21993..2bbf787 100644
> --- a/hw/openpic.c
> +++ b/hw/openpic.c
> @@ -1194,7 +1194,6 @@ qemu_irq *openpic_init (PCIBus *bus, int *pmem_index, int nb_cpus,
> pci_config_set_vendor_id(pci_conf, PCI_VENDOR_ID_IBM);
> pci_config_set_device_id(pci_conf, PCI_DEVICE_ID_IBM_OPENPIC2);
> pci_config_set_class(pci_conf, PCI_CLASS_SYSTEM_OTHER); // FIXME?
> - pci_conf[PCI_HEADER_TYPE] = PCI_HEADER_TYPE_NORMAL; // header_type
> pci_conf[0x3d] = 0x00; // no interrupt pin
>
> /* Register I/O spaces */
> diff --git a/hw/pcnet.c b/hw/pcnet.c
> index c894d13..e172510 100644
> --- a/hw/pcnet.c
> +++ b/hw/pcnet.c
> @@ -1990,7 +1990,6 @@ static int pci_pcnet_init(PCIDevice *pci_dev)
> /* TODO: 0 is the default anyway, no need to set it. */
> pci_conf[PCI_CLASS_PROG] = 0x00;
> pci_config_set_class(pci_conf, PCI_CLASS_NETWORK_ETHERNET);
> - pci_conf[PCI_HEADER_TYPE] = PCI_HEADER_TYPE_NORMAL; // header_type
>
> /* TODO: not necessary, is set when BAR is registered. */
> pci_set_long(pci_conf + PCI_BASE_ADDRESS_0, PCI_BASE_ADDRESS_SPACE_IO);
> diff --git a/hw/piix4.c b/hw/piix4.c
> index f75951b..03926a7 100644
> --- a/hw/piix4.c
> +++ b/hw/piix4.c
> @@ -93,8 +93,7 @@ static int piix4_initfn(PCIDevice *d)
> pci_config_set_vendor_id(pci_conf, PCI_VENDOR_ID_INTEL);
> pci_config_set_device_id(pci_conf, PCI_DEVICE_ID_INTEL_82371AB_0); // 82371AB/EB/MB PIIX4 PCI-to-ISA bridge
> pci_config_set_class(pci_conf, PCI_CLASS_BRIDGE_ISA);
> - pci_conf[PCI_HEADER_TYPE] =
> - PCI_HEADER_TYPE_NORMAL | PCI_HEADER_TYPE_MULTI_FUNCTION; // header_type = PCI_multifunction, generic
> + pci_conf[PCI_HEADER_TYPE] |= PCI_HEADER_TYPE_MULTI_FUNCTION;
>
> piix4_dev = d;
> qemu_register_reset(piix4_reset, d);
> diff --git a/hw/piix_pci.c b/hw/piix_pci.c
> index 16645cd..0b0c92b 100644
> --- a/hw/piix_pci.c
> +++ b/hw/piix_pci.c
> @@ -208,7 +208,6 @@ static int i440fx_initfn(PCIDevice *dev)
> pci_config_set_device_id(d->dev.config, PCI_DEVICE_ID_INTEL_82441);
> d->dev.config[0x08] = 0x02; // revision
> pci_config_set_class(d->dev.config, PCI_CLASS_BRIDGE_HOST);
> - d->dev.config[PCI_HEADER_TYPE] = PCI_HEADER_TYPE_NORMAL; // header_type
>
> d->dev.config[I440FX_SMRAM] = 0x02;
>
> @@ -337,8 +336,7 @@ static int piix3_initfn(PCIDevice *dev)
> pci_config_set_vendor_id(pci_conf, PCI_VENDOR_ID_INTEL);
> pci_config_set_device_id(pci_conf, PCI_DEVICE_ID_INTEL_82371SB_0); // 82371SB PIIX3 PCI-to-ISA bridge (Step A1)
> pci_config_set_class(pci_conf, PCI_CLASS_BRIDGE_ISA);
> - pci_conf[PCI_HEADER_TYPE] =
> - PCI_HEADER_TYPE_NORMAL | PCI_HEADER_TYPE_MULTI_FUNCTION; // header_type = PCI_multifunction, generic
> + pci_conf[PCI_HEADER_TYPE] |= PCI_HEADER_TYPE_MULTI_FUNCTION;
>
> qemu_register_reset(piix3_reset, d);
> return 0;
> diff --git a/hw/prep_pci.c b/hw/prep_pci.c
> index 144fde0..0c2afe9 100644
> --- a/hw/prep_pci.c
> +++ b/hw/prep_pci.c
> @@ -137,7 +137,6 @@ PCIBus *pci_prep_init(qemu_irq *pic)
> pci_config_set_class(d->config, PCI_CLASS_BRIDGE_HOST);
> d->config[0x0C] = 0x08; // cache_line_size
> d->config[0x0D] = 0x10; // latency_timer
> - d->config[PCI_HEADER_TYPE] = PCI_HEADER_TYPE_NORMAL; // header_type
> d->config[0x34] = 0x00; // capabilities_pointer
>
> return s->bus;
> diff --git a/hw/rtl8139.c b/hw/rtl8139.c
> index bfa7cde..83e5db9 100644
> --- a/hw/rtl8139.c
> +++ b/hw/rtl8139.c
> @@ -3363,7 +3363,6 @@ static int pci_rtl8139_init(PCIDevice *dev)
> pci_conf[PCI_COMMAND] = PCI_COMMAND_IO | PCI_COMMAND_MASTER;
> pci_conf[PCI_REVISION_ID] = RTL8139_PCI_REVID; /* >=0x20 is for 8139C+ */
> pci_config_set_class(pci_conf, PCI_CLASS_NETWORK_ETHERNET);
> - pci_conf[PCI_HEADER_TYPE] = PCI_HEADER_TYPE_NORMAL;
> /* TODO: value should be 0 at RST# */
> pci_conf[PCI_INTERRUPT_PIN] = 1; /* interrupt pin 0 */
> /* TODO: start of capability list, but no capability
> diff --git a/hw/sun4u.c b/hw/sun4u.c
> index 40b5f1f..cf5a8c4 100644
> --- a/hw/sun4u.c
> +++ b/hw/sun4u.c
> @@ -562,7 +562,6 @@ pci_ebus_init1(PCIDevice *s)
> s->config[0x09] = 0x00; // programming i/f
> pci_config_set_class(s->config, PCI_CLASS_BRIDGE_OTHER);
> s->config[0x0D] = 0x0a; // latency_timer
> - s->config[PCI_HEADER_TYPE] = PCI_HEADER_TYPE_NORMAL; // header_type
>
> pci_register_bar(s, 0, 0x1000000, PCI_BASE_ADDRESS_SPACE_MEMORY,
> ebus_mmio_mapfunc);
> diff --git a/hw/unin_pci.c b/hw/unin_pci.c
> index f0a773d..7b1c94b 100644
> --- a/hw/unin_pci.c
> +++ b/hw/unin_pci.c
> @@ -298,7 +298,6 @@ static int unin_main_pci_host_init(PCIDevice *d)
> pci_config_set_class(d->config, PCI_CLASS_BRIDGE_HOST);
> d->config[0x0C] = 0x08; // cache_line_size
> d->config[0x0D] = 0x10; // latency_timer
> - d->config[PCI_HEADER_TYPE] = PCI_HEADER_TYPE_NORMAL; // header_type
> d->config[0x34] = 0x00; // capabilities_pointer
> return 0;
> }
> @@ -311,7 +310,6 @@ static int unin_agp_pci_host_init(PCIDevice *d)
> pci_config_set_class(d->config, PCI_CLASS_BRIDGE_HOST);
> d->config[0x0C] = 0x08; // cache_line_size
> d->config[0x0D] = 0x10; // latency_timer
> - d->config[PCI_HEADER_TYPE] = PCI_HEADER_TYPE_NORMAL; // header_type
> // d->config[0x34] = 0x80; // capabilities_pointer
> return 0;
> }
> @@ -327,7 +325,6 @@ static int u3_agp_pci_host_init(PCIDevice *d)
> d->config[0x0C] = 0x08;
> /* latency timer */
> d->config[0x0D] = 0x10;
> - d->config[PCI_HEADER_TYPE] = PCI_HEADER_TYPE_NORMAL;
> return 0;
> }
>
> @@ -339,7 +336,6 @@ static int unin_internal_pci_host_init(PCIDevice *d)
> pci_config_set_class(d->config, PCI_CLASS_BRIDGE_HOST);
> d->config[0x0C] = 0x08; // cache_line_size
> d->config[0x0D] = 0x10; // latency_timer
> - d->config[PCI_HEADER_TYPE] = PCI_HEADER_TYPE_NORMAL; // header_type
> d->config[0x34] = 0x00; // capabilities_pointer
> return 0;
> }
> diff --git a/hw/usb-uhci.c b/hw/usb-uhci.c
> index 624d55b..058bf59 100644
> --- a/hw/usb-uhci.c
> +++ b/hw/usb-uhci.c
> @@ -1108,7 +1108,6 @@ static int usb_uhci_common_initfn(UHCIState *s)
> pci_conf[PCI_REVISION_ID] = 0x01; // revision number
> pci_conf[PCI_CLASS_PROG] = 0x00;
> pci_config_set_class(pci_conf, PCI_CLASS_SERIAL_USB);
> - pci_conf[PCI_HEADER_TYPE] = PCI_HEADER_TYPE_NORMAL; // header_type
> /* TODO: reset value should be 0. */
> pci_conf[PCI_INTERRUPT_PIN] = 4; // interrupt pin 3
> pci_conf[0x60] = 0x10; // release number
> diff --git a/hw/vga-pci.c b/hw/vga-pci.c
> index eef78ed..2315f70 100644
> --- a/hw/vga-pci.c
> +++ b/hw/vga-pci.c
> @@ -90,7 +90,6 @@ static int pci_vga_initfn(PCIDevice *dev)
> pci_config_set_vendor_id(pci_conf, PCI_VENDOR_ID_QEMU);
> pci_config_set_device_id(pci_conf, PCI_DEVICE_ID_QEMU_VGA);
> pci_config_set_class(pci_conf, PCI_CLASS_DISPLAY_VGA);
> - pci_conf[PCI_HEADER_TYPE] = PCI_HEADER_TYPE_NORMAL; // header_type
>
> /* XXX: VGA_RAM_SIZE must be a power of two */
> pci_register_bar(&d->dev, 0, VGA_RAM_SIZE,
> diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
> index ea8ea6a..4c02c28 100644
> --- a/hw/virtio-pci.c
> +++ b/hw/virtio-pci.c
> @@ -507,7 +507,6 @@ static void virtio_init_pci(VirtIOPCIProxy *proxy, VirtIODevice *vdev,
>
> config[0x09] = pif;
> pci_config_set_class(config, class_code);
> - config[PCI_HEADER_TYPE] = PCI_HEADER_TYPE_NORMAL;
>
> config[0x2c] = vendor & 0xFF;
> config[0x2d] = (vendor >> 8) & 0xFF;
> diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c
> index bf2a699..38fe976 100644
> --- a/hw/vmware_vga.c
> +++ b/hw/vmware_vga.c
> @@ -1246,7 +1246,6 @@ static int pci_vmsvga_initfn(PCIDevice *dev)
> pci_config_set_class(s->card.config, PCI_CLASS_DISPLAY_VGA);
> s->card.config[PCI_CACHE_LINE_SIZE] = 0x08; /* Cache line size */
> s->card.config[PCI_LATENCY_TIMER] = 0x40; /* Latency timer */
> - s->card.config[PCI_HEADER_TYPE] = PCI_HEADER_TYPE_NORMAL;
> s->card.config[PCI_SUBSYSTEM_VENDOR_ID] = PCI_VENDOR_ID_VMWARE & 0xff;
> s->card.config[PCI_SUBSYSTEM_VENDOR_ID + 1] = PCI_VENDOR_ID_VMWARE >> 8;
> s->card.config[PCI_SUBSYSTEM_ID] = SVGA_PCI_DEVICE_ID & 0xff;
> diff --git a/hw/wdt_i6300esb.c b/hw/wdt_i6300esb.c
> index be0e89e..46e1df8 100644
> --- a/hw/wdt_i6300esb.c
> +++ b/hw/wdt_i6300esb.c
> @@ -411,7 +411,6 @@ static int i6300esb_init(PCIDevice *dev)
> pci_config_set_vendor_id(pci_conf, PCI_VENDOR_ID_INTEL);
> pci_config_set_device_id(pci_conf, PCI_DEVICE_ID_INTEL_ESB_9);
> pci_config_set_class(pci_conf, PCI_CLASS_SYSTEM_OTHER);
> - pci_conf[PCI_HEADER_TYPE] = 0x00;
>
> pci_register_bar(&d->dev, 0, 0x10,
> PCI_BASE_ADDRESS_SPACE_MEMORY, i6300esb_map);
> --
> 1.6.6.1
^ permalink raw reply [flat|nested] 28+ messages in thread
* [Qemu-devel] Re: [PATCH 00/10] pci: pci to pci bridge clean up and enhancement
2010-06-17 6:15 [Qemu-devel] [PATCH 00/10] pci: pci to pci bridge clean up and enhancement Isaku Yamahata
` (9 preceding siblings ...)
2010-06-17 6:15 ` [Qemu-devel] [PATCH 10/10] pci: don't overwrite multi functio bit in pci header type Isaku Yamahata
@ 2010-06-17 10:02 ` Michael S. Tsirkin
2010-06-17 11:57 ` Michael S. Tsirkin
10 siblings, 1 reply; 28+ messages in thread
From: Michael S. Tsirkin @ 2010-06-17 10:02 UTC (permalink / raw)
To: Isaku Yamahata
Cc: stefano.stabellini, jan.kiszka, allen.m.kay, qemu-devel,
blauwirbel, kraxel, jean.guyader
On Thu, Jun 17, 2010 at 03:15:42PM +0900, Isaku Yamahata wrote:
> This patch series cleans up pci to pci bridge layer by introducing
> pci bridge layer. and some bug fixes.
> Although pci bridge implementation would belong to pci.c,
> I split it out into pci_bridge.c because pci.c is already big enough.
>
> This might seem over engineering, but it's also a preparation for
> pci express root/upstream/downstream port emulators.
> Those express ports are similar, but different from each other.
> So new pci bridge layer helps here.
> Once this patch series is merged, the express ports patch will follow.
A separate patchset with just bugfixes and cleanups, would be easier to
merge.
For example, forcing all devices to call pci_reset_default
in their reset routines does not look like a good cleanup:
the less boilerplate, the better IMO. New APIs seem undocumented
and it is not obvious what they do. For example, what does
qdev_reset do? Adding more callbacks
also does not make me very happy, they are hard to follow
and to debug. Maybe it would be better to look at the bridge layer when
we see how it helps pci express. But it would be even better IMO to avoid
layers, and just export some common functions that can be reused,
without forcing all devices to go through them.
> Isaku Yamahata (10):
> pci_bridge: split out pci bridge code into pci_bridge.c from pci.c
> qdev: export qdev_reset() for later use.
> pci: fix pci_bus_reset() with 64bit BAR and several clean ups.
> pci_bridge: introduce pci bridge layer.
> pci bridge: add helper function for ssvid capability.
> pci: eliminate work around in pci_device_reset().
> pci: fix pci domain registering.
> pci: remove PCIDeviceInfo::header_type
> pci: set PCI multi-function bit appropriately.
> pci: don't overwrite multi functio bit in pci header type.
>
> Makefile.objs | 2 +-
> hw/ac97.c | 1 -
> hw/acpi_piix4.c | 1 -
> hw/apb_pci.c | 43 ++++++++-----
> hw/dec_pci.c | 31 ++++++---
> hw/e1000.c | 1 +
> hw/grackle_pci.c | 1 -
> hw/ide/cmd646.c | 1 -
> hw/ide/piix.c | 1 -
> hw/lsi53c895a.c | 2 +
> hw/macio.c | 1 -
> hw/ne2000.c | 1 -
> hw/openpic.c | 1 -
> hw/pci.c | 194 +++++++++++++++++++++++------------------------------
> hw/pci.h | 22 +++++-
> hw/pci_bridge.c | 188 +++++++++++++++++++++++++++++++++++++++++++++++++++
> hw/pci_bridge.h | 71 +++++++++++++++++++
> hw/pcnet.c | 2 +-
> hw/piix4.c | 3 +-
> hw/piix_pci.c | 5 +-
> hw/prep_pci.c | 1 -
> hw/qdev.c | 13 +++-
> hw/qdev.h | 1 +
> hw/rtl8139.c | 3 +-
> hw/sun4u.c | 1 -
> hw/unin_pci.c | 4 -
> hw/usb-uhci.c | 1 -
> hw/vga-pci.c | 1 -
> hw/virtio-pci.c | 2 +-
> hw/vmware_vga.c | 1 -
> hw/wdt_i6300esb.c | 1 -
> qemu-common.h | 1 +
> 32 files changed, 430 insertions(+), 172 deletions(-)
> create mode 100644 hw/pci_bridge.c
> create mode 100644 hw/pci_bridge.h
^ permalink raw reply [flat|nested] 28+ messages in thread* [Qemu-devel] Re: [PATCH 00/10] pci: pci to pci bridge clean up and enhancement
2010-06-17 10:02 ` [Qemu-devel] Re: [PATCH 00/10] pci: pci to pci bridge clean up and enhancement Michael S. Tsirkin
@ 2010-06-17 11:57 ` Michael S. Tsirkin
2010-06-18 3:26 ` Isaku Yamahata
0 siblings, 1 reply; 28+ messages in thread
From: Michael S. Tsirkin @ 2010-06-17 11:57 UTC (permalink / raw)
To: Isaku Yamahata
Cc: stefano.stabellini, jan.kiszka, allen.m.kay, qemu-devel,
blauwirbel, kraxel, jean.guyader
On Thu, Jun 17, 2010 at 01:02:43PM +0300, Michael S. Tsirkin wrote:
> For example, forcing all devices to call pci_reset_default
> in their reset routines does not look like a good cleanup:
> the less boilerplate, the better IMO.
One thing that we need to address, is devices
which need to enable memory+master on init.
They should probably also enable this on reset.
One approach that was discussed several times
would be to call cleanup and then init again.
I expect this would be enough to get rid of reset
callbacks in most devices.
--
MST
^ permalink raw reply [flat|nested] 28+ messages in thread
* [Qemu-devel] Re: [PATCH 00/10] pci: pci to pci bridge clean up and enhancement
2010-06-17 11:57 ` Michael S. Tsirkin
@ 2010-06-18 3:26 ` Isaku Yamahata
2010-06-18 12:46 ` Michael S. Tsirkin
0 siblings, 1 reply; 28+ messages in thread
From: Isaku Yamahata @ 2010-06-18 3:26 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: stefano.stabellini, jan.kiszka, allen.m.kay, qemu-devel,
blauwirbel, kraxel, jean.guyader
On Thu, Jun 17, 2010 at 02:57:16PM +0300, Michael S. Tsirkin wrote:
> On Thu, Jun 17, 2010 at 01:02:43PM +0300, Michael S. Tsirkin wrote:
> > For example, forcing all devices to call pci_reset_default
> > in their reset routines does not look like a good cleanup:
> > the less boilerplate, the better IMO.
>
> One thing that we need to address, is devices
> which need to enable memory+master on init.
> They should probably also enable this on reset.
Isn't it BIOS/firmware that initializes those bits?
And seabios does so.
The pci spec explicitly says that they are 0 after RST#.
> One approach that was discussed several times
> would be to call cleanup and then init again.
> I expect this would be enough to get rid of reset
> callbacks in most devices.
Oh, some devices set those bits on init.
So cleanup + init again might be easier than
sorting out initialization/reset of each devices.
thanks,
>
> --
> MST
>
--
yamahata
^ permalink raw reply [flat|nested] 28+ messages in thread
* [Qemu-devel] Re: [PATCH 00/10] pci: pci to pci bridge clean up and enhancement
2010-06-18 3:26 ` Isaku Yamahata
@ 2010-06-18 12:46 ` Michael S. Tsirkin
0 siblings, 0 replies; 28+ messages in thread
From: Michael S. Tsirkin @ 2010-06-18 12:46 UTC (permalink / raw)
To: Isaku Yamahata
Cc: stefano.stabellini, jan.kiszka, allen.m.kay, qemu-devel,
blauwirbel, kraxel, jean.guyader
On Fri, Jun 18, 2010 at 12:26:10PM +0900, Isaku Yamahata wrote:
> On Thu, Jun 17, 2010 at 02:57:16PM +0300, Michael S. Tsirkin wrote:
> > On Thu, Jun 17, 2010 at 01:02:43PM +0300, Michael S. Tsirkin wrote:
> > > For example, forcing all devices to call pci_reset_default
> > > in their reset routines does not look like a good cleanup:
> > > the less boilerplate, the better IMO.
> >
> > One thing that we need to address, is devices
> > which need to enable memory+master on init.
> > They should probably also enable this on reset.
>
> Isn't it BIOS/firmware that initializes those bits?
memory yes, bus master no.
> And seabios does so.
> The pci spec explicitly says that they are 0 after RST#.
Mostly these are bugs in emulation, but could be bugs
in devices that we emulate (PBM).
> > One approach that was discussed several times
> > would be to call cleanup and then init again.
> > I expect this would be enough to get rid of reset
> > callbacks in most devices.
>
> Oh, some devices set those bits on init.
> So cleanup + init again might be easier than
> sorting out initialization/reset of each devices.
Exactly.
> thanks,
> >
> > --
> > MST
> >
>
> --
> yamahata
^ permalink raw reply [flat|nested] 28+ messages in thread