All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v5 0/9] pci: multi-function bit fixes
@ 2010-06-23  7:15 Isaku Yamahata
  2010-06-23  7:15 ` [Qemu-devel] [PATCH v5 1/9] pci: use PCI_DEVFN() where appropriate Isaku Yamahata
                   ` (9 more replies)
  0 siblings, 10 replies; 15+ messages in thread
From: Isaku Yamahata @ 2010-06-23  7:15 UTC (permalink / raw)
  To: qemu-devel; +Cc: yu.liu, mst, blauwirbel, yamahata, paul, aurelien

Maing changes v4 -> v5:
- split up/reorder patches for bisectability and testability
- stype fixes.

patch description:

When pci devices are populated as multi-function,
OS can fail to probe function > 0. It's because multi function
bit of header type register in configuration space isn't set,
so OS probes only function 0 skipping function > 0 as optimization.

This patch set make qemu set multi function bit when function > 0 
is populated.

Changes v3 -> v4:
- introduced multifunction property and add validation check to catch
  user operation errors.
- some more clean up.


Isaku Yamahata (9):
  pci: use PCI_DEVFN() where appropriate.
  pci: insert assert that auto-assigned-address function is single
    function device.
  pci: don't overwrite multi functio bit in pci header type.
  pci: remove PCIDeviceInfo::header_type
  qdev: implement qdev_prop_set_bit().
  pci: introduce multifunction property.
  pci: set multifunction property for normal device.
  pci_bridge: make pci bridge aware of pci multi function bit.
  pci: set PCI multi-function bit appropriately.

 hw/ac97.c            |    1 -
 hw/acpi_piix4.c      |    1 -
 hw/apb_pci.c         |   11 ++----
 hw/dec_pci.c         |    4 +-
 hw/grackle_pci.c     |    1 -
 hw/gt64xxx.c         |    2 +-
 hw/ide/cmd646.c      |    1 -
 hw/ide/piix.c        |    1 -
 hw/macio.c           |    1 -
 hw/ne2000.c          |    1 -
 hw/openpic.c         |    1 -
 hw/pci.c             |   97 +++++++++++++++++++++++++++++++++++++++++++-------
 hw/pci.h             |   21 +++++++++--
 hw/pcnet.c           |    1 -
 hw/piix4.c           |    4 +--
 hw/piix_pci.c        |    5 +--
 hw/ppce500_pci.c     |    3 +-
 hw/prep_pci.c        |    1 -
 hw/qdev-properties.c |    5 +++
 hw/qdev.h            |    1 +
 hw/rtl8139.c         |    1 -
 hw/sun4u.c           |    1 -
 hw/unin_pci.c        |   16 +++-----
 hw/usb-uhci.c        |    1 -
 hw/versatile_pci.c   |    2 +-
 hw/vga-pci.c         |    1 -
 hw/virtio-pci.c      |    1 -
 hw/vmware_vga.c      |    1 -
 hw/wdt_i6300esb.c    |    1 -
 29 files changed, 125 insertions(+), 63 deletions(-)

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PATCH v5 1/9] pci: use PCI_DEVFN() where appropriate.
  2010-06-23  7:15 [Qemu-devel] [PATCH v5 0/9] pci: multi-function bit fixes Isaku Yamahata
@ 2010-06-23  7:15 ` Isaku Yamahata
  2010-06-23  9:59   ` [Qemu-devel] " Michael S. Tsirkin
  2010-06-23  7:15 ` [Qemu-devel] [PATCH v5 2/9] pci: insert assert that auto-assigned-address function is single function device Isaku Yamahata
                   ` (8 subsequent siblings)
  9 siblings, 1 reply; 15+ messages in thread
From: Isaku Yamahata @ 2010-06-23  7:15 UTC (permalink / raw)
  To: qemu-devel; +Cc: yu.liu, mst, blauwirbel, yamahata, paul, aurelien

Use PCI_DEVFN() and PCI_FUNC_MAX where appropriate.
This patch make it clear that func = 0.

test:
The following object files with/without this patch are stripped and compared.
They remains same.
  arm-softmmu/versatile_pci.o
  libhw32/ppce500_pci.o
  libhw32/unin_pci.o
  libhw64/ppce500_pci.o
  libhw64/unin_pci.o
  mips-softmmu/gt64xxx.o
  mips64-softmmu/gt64xxx.o
  mips64el-softmmu/gt64xxx.o
  mipsel-softmmu/gt64xxx.o

Cc: Aurelien Jarno <aurelien@aurel32.net>
Cc: Yu Liu <yu.liu@freescale.com>
Cc: Paul Brook <paul@codesourcery.com>
Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>

---
changes v2 -> v3
- split out into PCI_DEVFN() patch and assert patch.
- more test by comparing stripped object.
---
 hw/gt64xxx.c       |    2 +-
 hw/ppce500_pci.c   |    3 ++-
 hw/unin_pci.c      |   12 ++++++------
 hw/versatile_pci.c |    2 +-
 4 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/hw/gt64xxx.c b/hw/gt64xxx.c
index 7691e1d..8e2cf14 100644
--- a/hw/gt64xxx.c
+++ b/hw/gt64xxx.c
@@ -1115,7 +1115,7 @@ PCIBus *pci_gt64120_init(qemu_irq *pic)
 
     s->pci->bus = pci_register_bus(NULL, "pci",
                                    pci_gt64120_set_irq, pci_gt64120_map_irq,
-                                   pic, 144, 4);
+                                   pic, PCI_DEVFN(18, 0), 4);
     s->ISD_handle = cpu_register_io_memory(gt64120_read, gt64120_write, s);
     d = pci_register_device(s->pci->bus, "GT64120 PCI Bus", sizeof(PCIDevice),
                             0, NULL, NULL);
diff --git a/hw/ppce500_pci.c b/hw/ppce500_pci.c
index 336d284..f949fe3 100644
--- a/hw/ppce500_pci.c
+++ b/hw/ppce500_pci.c
@@ -279,7 +279,8 @@ PCIBus *ppce500_pci_init(qemu_irq pci_irqs[4], target_phys_addr_t registers)
     controller->pci_state.bus = pci_register_bus(NULL, "pci",
                                                  mpc85xx_pci_set_irq,
                                                  mpc85xx_pci_map_irq,
-                                                 pci_irqs, 0x88, 4);
+                                                 pci_irqs, PCI_DEVFN(0x11, 0),
+                                                 4);
     d = pci_register_device(controller->pci_state.bus,
                             "host bridge", sizeof(PCIDevice),
                             0, NULL, NULL);
diff --git a/hw/unin_pci.c b/hw/unin_pci.c
index f0a773d..0ecf40f 100644
--- a/hw/unin_pci.c
+++ b/hw/unin_pci.c
@@ -228,10 +228,10 @@ PCIBus *pci_pmac_init(qemu_irq *pic)
     d = FROM_SYSBUS(UNINState, s);
     d->host_state.bus = pci_register_bus(&d->busdev.qdev, "pci",
                                          pci_unin_set_irq, pci_unin_map_irq,
-                                         pic, 11 << 3, 4);
+                                         pic, PCI_DEVFN(11, 0), 4);
 
 #if 0
-    pci_create_simple(d->host_state.bus, 11 << 3, "uni-north");
+    pci_create_simple(d->host_state.bus, PCI_DEVFN(11, 0), "uni-north");
 #endif
 
     sysbus_mmio_map(s, 0, 0xf2800000);
@@ -240,11 +240,11 @@ PCIBus *pci_pmac_init(qemu_irq *pic)
     /* DEC 21154 bridge */
 #if 0
     /* XXX: not activated as PPC BIOS doesn't handle multiple buses properly */
-    pci_create_simple(d->host_state.bus, 12 << 3, "dec-21154");
+    pci_create_simple(d->host_state.bus, PCI_DEVFN(12, 0), "dec-21154");
 #endif
 
     /* Uninorth AGP bus */
-    pci_create_simple(d->host_state.bus, 11 << 3, "uni-north-agp");
+    pci_create_simple(d->host_state.bus, PCI_DEVFN(11, 0), "uni-north-agp");
     dev = qdev_create(NULL, "uni-north-agp");
     qdev_init_nofail(dev);
     s = sysbus_from_qdev(dev);
@@ -254,7 +254,7 @@ PCIBus *pci_pmac_init(qemu_irq *pic)
     /* Uninorth internal bus */
 #if 0
     /* XXX: not needed for now */
-    pci_create_simple(d->host_state.bus, 14 << 3, "uni-north-pci");
+    pci_create_simple(d->host_state.bus, PCI_DEVFN(14, 0), "uni-north-pci");
     dev = qdev_create(NULL, "uni-north-pci");
     qdev_init_nofail(dev);
     s = sysbus_from_qdev(dev);
@@ -280,7 +280,7 @@ PCIBus *pci_pmac_u3_init(qemu_irq *pic)
 
     d->host_state.bus = pci_register_bus(&d->busdev.qdev, "pci",
                                          pci_unin_set_irq, pci_unin_map_irq,
-                                         pic, 11 << 3, 4);
+                                         pic, PCI_DEVFN(11, 0), 4);
 
     sysbus_mmio_map(s, 0, 0xf0800000);
     sysbus_mmio_map(s, 1, 0xf0c00000);
diff --git a/hw/versatile_pci.c b/hw/versatile_pci.c
index 199bc19..a76bdfa 100644
--- a/hw/versatile_pci.c
+++ b/hw/versatile_pci.c
@@ -127,7 +127,7 @@ static int pci_vpb_init(SysBusDevice *dev)
     }
     bus = pci_register_bus(&dev->qdev, "pci",
                            pci_vpb_set_irq, pci_vpb_map_irq, s->irq,
-                           11 << 3, 4);
+                           PCI_DEVFN(11, 0), 4);
 
     /* ??? Register memory space.  */
 
-- 
1.6.6.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PATCH v5 2/9] pci: insert assert that auto-assigned-address function is single function device.
  2010-06-23  7:15 [Qemu-devel] [PATCH v5 0/9] pci: multi-function bit fixes Isaku Yamahata
  2010-06-23  7:15 ` [Qemu-devel] [PATCH v5 1/9] pci: use PCI_DEVFN() where appropriate Isaku Yamahata
@ 2010-06-23  7:15 ` Isaku Yamahata
  2010-06-23 10:11   ` [Qemu-devel] " Michael S. Tsirkin
  2010-06-23  7:15 ` [Qemu-devel] [PATCH v5 3/9] pci: don't overwrite multi functio bit in pci header type Isaku Yamahata
                   ` (7 subsequent siblings)
  9 siblings, 1 reply; 15+ messages in thread
From: Isaku Yamahata @ 2010-06-23  7:15 UTC (permalink / raw)
  To: qemu-devel; +Cc: yu.liu, mst, blauwirbel, yamahata, paul, aurelien

Auto-assigned-address pci function (passing devfn = -1) is always
single function.
This patch adds assert() to guarantee that auto-assigned-address function
is always single function device at function = 0.

Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>

---
changes v2 -> v3
split out into PCI_DEVFN() patch and assert patch.
---
 hw/pci.c |    3 ++-
 hw/pci.h |    1 +
 2 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/hw/pci.c b/hw/pci.c
index ef17eb4..1ba209f 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -224,6 +224,7 @@ void pci_bus_new_inplace(PCIBus *bus, DeviceState *parent,
                          const char *name, int devfn_min)
 {
     qbus_create_inplace(&bus->qbus, &pci_bus_info, parent, name);
+    assert(PCI_FUNC(devfn_min) == 0);
     bus->devfn_min = devfn_min;
     QLIST_INIT(&bus->child);
     vmstate_register(-1, &vmstate_pcibus, bus);
@@ -601,7 +602,7 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
 {
     if (devfn < 0) {
         for(devfn = bus->devfn_min ; devfn < ARRAY_SIZE(bus->devices);
-            devfn += 8) {
+            devfn += PCI_FUNC_MAX) {
             if (!bus->devices[devfn])
                 goto found;
         }
diff --git a/hw/pci.h b/hw/pci.h
index 6a2bc6a..077387d 100644
--- a/hw/pci.h
+++ b/hw/pci.h
@@ -14,6 +14,7 @@
 #define PCI_DEVFN(slot, func)   ((((slot) & 0x1f) << 3) | ((func) & 0x07))
 #define PCI_SLOT(devfn)         (((devfn) >> 3) & 0x1f)
 #define PCI_FUNC(devfn)         ((devfn) & 0x07)
+#define PCI_FUNC_MAX            8
 
 /* Class, Vendor and Device IDs from Linux's pci_ids.h */
 #include "pci_ids.h"
-- 
1.6.6.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PATCH v5 3/9] pci: don't overwrite multi functio bit in pci header type.
  2010-06-23  7:15 [Qemu-devel] [PATCH v5 0/9] pci: multi-function bit fixes Isaku Yamahata
  2010-06-23  7:15 ` [Qemu-devel] [PATCH v5 1/9] pci: use PCI_DEVFN() where appropriate Isaku Yamahata
  2010-06-23  7:15 ` [Qemu-devel] [PATCH v5 2/9] pci: insert assert that auto-assigned-address function is single function device Isaku Yamahata
@ 2010-06-23  7:15 ` Isaku Yamahata
  2010-06-23  7:15 ` [Qemu-devel] [PATCH v5 4/9] pci: remove PCIDeviceInfo::header_type Isaku Yamahata
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 15+ messages in thread
From: Isaku Yamahata @ 2010-06-23  7:15 UTC (permalink / raw)
  To: qemu-devel; +Cc: yu.liu, mst, blauwirbel, yamahata, paul, aurelien

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.

how to test:
run qemu and issue info pci to see whether a device in question is
normal device, not pci-to-pci bridge.
This is handy because guest os isn't required.

tested changes:
The following files are covered by using following commands.
sparc64-softmmu
  apb_pci.c, vga-pci.c, cmd646.c, ne2k_pci.c, sun4u.c
ppc-softmmu
  grackle_pci.c, cmd646.c, ne2k_pci.c, vga-pci.c, macio.c
ppc-softmmu -M mac99
  unin_pci.c(uni-north, uni-north-agp)
ppc64-softmmu
  pci-ohci, ne2k_pci, vga-pci, unin_pci.c(u3-agp)
x86_64-softmmu
  acpi_piix4.c, ide/piix.c, piix_pci.c
  -vga vmware vmware_vga.c
  -watchdog i6300esb wdt_i6300esb.c
  -usb usb-uhci.c
  -sound ac97 ac97.c
  -nic model=rtl8139 rtl8139.c
  -nic model=pcnet pcnet.c
  -balloon virtio virtio-pci.c:

untested changes:
The following changes aren't tested.
prep_pci.c: ppc-softmmu -M prep should cover, but core dumped.
unin_pci.c(uni-north-pci): the caller is commented out.
openpic.c: the caller is commented out in ppc_prep.c

Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>

---
changes v3 -> v4:
- dropped the hunks which will be overwrites by later patch.
  piix4.c piix_pci.c
- moved this patch up in the patch series for bisectability.
chnages v2 -> v3:
- added  record what was tested in the commit message
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/piix_pci.c     |    1 -
 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 -
 20 files changed, 0 insertions(+), 24 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 31c8d70..6dd529f 100644
--- a/hw/apb_pci.c
+++ b/hw/apb_pci.c
@@ -427,8 +427,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 5e63eb5..5e75930 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/piix_pci.c b/hw/piix_pci.c
index 16645cd..7917ae0 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;
 
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 72e2242..441f0a9 100644
--- a/hw/rtl8139.c
+++ b/hw/rtl8139.c
@@ -3361,7 +3361,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 0ecf40f..717ffd1 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 e101fa0..0e25f25 100644
--- a/hw/virtio-pci.c
+++ b/hw/virtio-pci.c
@@ -506,7 +506,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] 15+ messages in thread

* [Qemu-devel] [PATCH v5 4/9] pci: remove PCIDeviceInfo::header_type
  2010-06-23  7:15 [Qemu-devel] [PATCH v5 0/9] pci: multi-function bit fixes Isaku Yamahata
                   ` (2 preceding siblings ...)
  2010-06-23  7:15 ` [Qemu-devel] [PATCH v5 3/9] pci: don't overwrite multi functio bit in pci header type Isaku Yamahata
@ 2010-06-23  7:15 ` Isaku Yamahata
  2010-06-23  7:15 ` [Qemu-devel] [PATCH v5 5/9] qdev: implement qdev_prop_set_bit() Isaku Yamahata
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 15+ messages in thread
From: Isaku Yamahata @ 2010-06-23  7:15 UTC (permalink / raw)
  To: qemu-devel; +Cc: yu.liu, mst, blauwirbel, yamahata, paul, aurelien

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     |   15 ++++++++-------
 hw/pci.h     |    8 ++++++--
 4 files changed, 16 insertions(+), 11 deletions(-)

diff --git a/hw/apb_pci.c b/hw/apb_pci.c
index 6dd529f..69a774d 100644
--- a/hw/apb_pci.c
+++ b/hw/apb_pci.c
@@ -434,7 +434,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 = 1,
 };
 
 static SysBusDeviceInfo pbm_host_info = {
diff --git a/hw/dec_pci.c b/hw/dec_pci.c
index 024c67c..83ce4cd 100644
--- a/hw/dec_pci.c
+++ b/hw/dec_pci.c
@@ -90,7 +90,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  = 1,
 };
 
 static void dec_register_devices(void)
diff --git a/hw/pci.c b/hw/pci.c
index 1ba209f..a14b74d 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -598,7 +598,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);
@@ -620,13 +620,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);
     }
 
@@ -1555,7 +1554,9 @@ static int pci_bridge_initfn(PCIDevice *dev)
     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;
+    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_SEC_STATUS,
                  PCI_STATUS_66MHZ | PCI_STATUS_FAST_BACK);
     return 0;
@@ -1606,7 +1607,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);
@@ -1856,7 +1857,7 @@ static PCIDeviceInfo bridge_info = {
     .init         = pci_bridge_initfn,
     .exit         = pci_bridge_exitfn,
     .config_write = pci_bridge_write_config,
-    .header_type  = PCI_HEADER_TYPE_BRIDGE,
+    .is_bridge    = 1,
     .qdev.props   = (Property[]) {
         DEFINE_PROP_HEX32("vendorid", PCIBridge, vid, 0),
         DEFINE_PROP_HEX32("deviceid", PCIBridge, did, 0),
diff --git a/hw/pci.h b/hw/pci.h
index 077387d..76adc66 100644
--- a/hw/pci.h
+++ b/hw/pci.h
@@ -326,8 +326,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? */
-- 
1.6.6.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PATCH v5 5/9] qdev: implement qdev_prop_set_bit().
  2010-06-23  7:15 [Qemu-devel] [PATCH v5 0/9] pci: multi-function bit fixes Isaku Yamahata
                   ` (3 preceding siblings ...)
  2010-06-23  7:15 ` [Qemu-devel] [PATCH v5 4/9] pci: remove PCIDeviceInfo::header_type Isaku Yamahata
@ 2010-06-23  7:15 ` Isaku Yamahata
  2010-06-23  7:15 ` [Qemu-devel] [PATCH v5 6/9] pci: introduce multifunction property Isaku Yamahata
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 15+ messages in thread
From: Isaku Yamahata @ 2010-06-23  7:15 UTC (permalink / raw)
  To: qemu-devel; +Cc: yu.liu, mst, blauwirbel, yamahata, paul, aurelien

implement qdev_prop_set_bit().

Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
---
 hw/qdev-properties.c |    5 +++++
 hw/qdev.h            |    1 +
 2 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index 5a8739d..20eec76 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -597,6 +597,11 @@ void qdev_prop_set(DeviceState *dev, const char *name, void *src, enum PropertyT
     qdev_prop_cpy(dev, prop, src);
 }
 
+void qdev_prop_set_bit(DeviceState *dev, const char *name, bool value)
+{
+    qdev_prop_set(dev, name, &value, PROP_TYPE_BIT);
+}
+
 void qdev_prop_set_uint8(DeviceState *dev, const char *name, uint8_t value)
 {
     qdev_prop_set(dev, name, &value, PROP_TYPE_UINT8);
diff --git a/hw/qdev.h b/hw/qdev.h
index be5ad67..1de7594 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -266,6 +266,7 @@ void *qdev_get_prop_ptr(DeviceState *dev, Property *prop);
 int qdev_prop_exists(DeviceState *dev, const char *name);
 int qdev_prop_parse(DeviceState *dev, const char *name, const char *value);
 void qdev_prop_set(DeviceState *dev, const char *name, void *src, enum PropertyType type);
+void qdev_prop_set_bit(DeviceState *dev, const char *name, bool value);
 void qdev_prop_set_uint8(DeviceState *dev, const char *name, uint8_t value);
 void qdev_prop_set_uint16(DeviceState *dev, const char *name, uint16_t value);
 void qdev_prop_set_uint32(DeviceState *dev, const char *name, uint32_t value);
-- 
1.6.6.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PATCH v5 6/9] pci: introduce multifunction property.
  2010-06-23  7:15 [Qemu-devel] [PATCH v5 0/9] pci: multi-function bit fixes Isaku Yamahata
                   ` (4 preceding siblings ...)
  2010-06-23  7:15 ` [Qemu-devel] [PATCH v5 5/9] qdev: implement qdev_prop_set_bit() Isaku Yamahata
@ 2010-06-23  7:15 ` Isaku Yamahata
  2010-06-23  7:15 ` [Qemu-devel] [PATCH v5 7/9] pci: set multifunction property for normal device Isaku Yamahata
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 15+ messages in thread
From: Isaku Yamahata @ 2010-06-23  7:15 UTC (permalink / raw)
  To: qemu-devel; +Cc: yu.liu, mst, blauwirbel, yamahata, paul, aurelien

introduce multifunction property.
Also introduce new convenient device creation function which
will be used later.

For bisectability this patch doesn't do anything, but sets the property
resulting in no functional changes.
Actual changes will be introduced by later patch.

Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>

---
changes v4 -> v5:
- split up the patch.
- convert multifunction uint8_t property into bit property
  by introducint QEMU_PCI_CAP_MULTIFUNCTION
- uint8_t mf -> bool multifunction
---
 hw/pci.c |   22 +++++++++++++++++++---
 hw/pci.h |    9 +++++++++
 2 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/hw/pci.c b/hw/pci.c
index a14b74d..b91d9f5 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -67,6 +67,8 @@ static struct BusInfo pci_bus_info = {
         DEFINE_PROP_PCI_DEVFN("addr", PCIDevice, devfn, -1),
         DEFINE_PROP_STRING("romfile", PCIDevice, romfile),
         DEFINE_PROP_UINT32("rombar",  PCIDevice, rom_bar, 1),
+        DEFINE_PROP_BIT("multifunction", PCIDevice, cap_present,
+                        QEMU_PCI_CAP_MULTIFUNCTION_BITNR, false),
         DEFINE_PROP_END_OF_LIST()
     }
 };
@@ -1651,22 +1653,36 @@ void pci_qdev_register_many(PCIDeviceInfo *info)
     }
 }
 
-PCIDevice *pci_create(PCIBus *bus, int devfn, const char *name)
+PCIDevice *pci_create_multifunction(PCIBus *bus, int devfn, bool multifunction,
+                                    const char *name)
 {
     DeviceState *dev;
 
     dev = qdev_create(&bus->qbus, name);
     qdev_prop_set_uint32(dev, "addr", devfn);
+    qdev_prop_set_bit(dev, "multifunction", multifunction);
     return DO_UPCAST(PCIDevice, qdev, dev);
 }
 
-PCIDevice *pci_create_simple(PCIBus *bus, int devfn, const char *name)
+PCIDevice *pci_create_simple_multifunction(PCIBus *bus, int devfn,
+                                           bool multifunction,
+                                           const char *name)
 {
-    PCIDevice *dev = pci_create(bus, devfn, name);
+    PCIDevice *dev = pci_create_multifunction(bus, devfn, multifunction, name);
     qdev_init_nofail(&dev->qdev);
     return dev;
 }
 
+PCIDevice *pci_create(PCIBus *bus, int devfn, const char *name)
+{
+    return pci_create_multifunction(bus, devfn, false, name);
+}
+
+PCIDevice *pci_create_simple(PCIBus *bus, int devfn, const char *name)
+{
+    return pci_create_simple_multifunction(bus, devfn, false, name);
+}
+
 static int pci_find_space(PCIDevice *pdev, uint8_t size)
 {
     int config_size = pci_config_size(pdev);
diff --git a/hw/pci.h b/hw/pci.h
index 76adc66..733a314 100644
--- a/hw/pci.h
+++ b/hw/pci.h
@@ -111,6 +111,10 @@ typedef struct PCIIORegion {
 enum {
     QEMU_PCI_CAP_MSIX = 0x1,
     QEMU_PCI_CAP_EXPRESS = 0x2,
+
+    /* multifunction capable device */
+#define QEMU_PCI_CAP_MULTIFUNCTION_BITNR        2
+    QEMU_PCI_CAP_MULTIFUNCTION = (1 << QEMU_PCI_CAP_MULTIFUNCTION_BITNR),
 };
 
 struct PCIDevice {
@@ -343,6 +347,11 @@ typedef struct {
 void pci_qdev_register(PCIDeviceInfo *info);
 void pci_qdev_register_many(PCIDeviceInfo *info);
 
+PCIDevice *pci_create_multifunction(PCIBus *bus, int devfn, bool multifunction,
+                                    const char *name);
+PCIDevice *pci_create_simple_multifunction(PCIBus *bus, int devfn,
+                                           bool multifunction,
+                                           const char *name);
 PCIDevice *pci_create(PCIBus *bus, int devfn, const char *name);
 PCIDevice *pci_create_simple(PCIBus *bus, int devfn, const char *name);
 
-- 
1.6.6.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PATCH v5 7/9] pci: set multifunction property for normal device.
  2010-06-23  7:15 [Qemu-devel] [PATCH v5 0/9] pci: multi-function bit fixes Isaku Yamahata
                   ` (5 preceding siblings ...)
  2010-06-23  7:15 ` [Qemu-devel] [PATCH v5 6/9] pci: introduce multifunction property Isaku Yamahata
@ 2010-06-23  7:15 ` Isaku Yamahata
  2010-06-23  7:15 ` [Qemu-devel] [PATCH v5 8/9] pci_bridge: make pci bridge aware of pci multi function bit Isaku Yamahata
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 15+ messages in thread
From: Isaku Yamahata @ 2010-06-23  7:15 UTC (permalink / raw)
  To: qemu-devel; +Cc: yu.liu, mst, blauwirbel, yamahata, paul, aurelien

use pci_create_simple_multifunction() for normal device which sets
multifunction bit.
At the moment, only pc_piix.c and mips_malta.c uses multifunction
devices with piix3/4 pci-isa bridge.
And other boards don't populate those devices.

Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>

---
changes v4 -> v5:
- uint8_t mf -> bool multifunction
---
 hw/piix4.c    |    2 +-
 hw/piix_pci.c |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/piix4.c b/hw/piix4.c
index f75951b..2298074 100644
--- a/hw/piix4.c
+++ b/hw/piix4.c
@@ -105,7 +105,7 @@ int piix4_init(PCIBus *bus, int devfn)
 {
     PCIDevice *d;
 
-    d = pci_create_simple(bus, devfn, "PIIX4");
+    d = pci_create_simple_multifunction(bus, devfn, true, "PIIX4");
     return d->devfn;
 }
 
diff --git a/hw/piix_pci.c b/hw/piix_pci.c
index 7917ae0..a3185f7 100644
--- a/hw/piix_pci.c
+++ b/hw/piix_pci.c
@@ -234,7 +234,7 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state, int *piix3_devfn, qemu_irq *
     *pi440fx_state = DO_UPCAST(PCII440FXState, dev, d);
 
     piix3 = DO_UPCAST(PIIX3State, dev,
-                                 pci_create_simple(b, -1, "PIIX3"));
+                      pci_create_simple_multifunction(b, -1, true, "PIIX3"));
     piix3->pic = pic;
     pci_bus_irqs(b, piix3_set_irq, pci_slot_get_pirq, piix3, 4);
     (*pi440fx_state)->piix3 = piix3;
-- 
1.6.6.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PATCH v5 8/9] pci_bridge: make pci bridge aware of pci multi function bit.
  2010-06-23  7:15 [Qemu-devel] [PATCH v5 0/9] pci: multi-function bit fixes Isaku Yamahata
                   ` (6 preceding siblings ...)
  2010-06-23  7:15 ` [Qemu-devel] [PATCH v5 7/9] pci: set multifunction property for normal device Isaku Yamahata
@ 2010-06-23  7:15 ` Isaku Yamahata
  2010-06-23  7:15 ` [Qemu-devel] [PATCH v5 9/9] pci: set PCI multi-function bit appropriately Isaku Yamahata
  2010-07-09  1:44 ` [Qemu-devel] [PATCH v5 0/9] pci: multi-function bit fixes Isaku Yamahata
  9 siblings, 0 replies; 15+ messages in thread
From: Isaku Yamahata @ 2010-06-23  7:15 UTC (permalink / raw)
  To: qemu-devel; +Cc: yu.liu, mst, blauwirbel, yamahata, paul, aurelien

make pci bridge aware of pci multi function property and let pci generic
code to set the bit.

Cc: Blue Swirl <blauwirbel@gmail.com>
Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>

---
changes v4 -> v5:
- uint8_t mf -> bool multifunction
---
 hw/apb_pci.c |    4 ++--
 hw/dec_pci.c |    2 +-
 hw/pci.c     |    5 +++--
 hw/pci.h     |    3 ++-
 4 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/hw/apb_pci.c b/hw/apb_pci.c
index 69a774d..fd11459 100644
--- a/hw/apb_pci.c
+++ b/hw/apb_pci.c
@@ -350,13 +350,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),
+    *bus2 = pci_bridge_init(d->bus, PCI_DEVFN(1, 0), true,
                             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),
+    *bus3 = pci_bridge_init(d->bus, PCI_DEVFN(1, 1), true,
                             PCI_VENDOR_ID_SUN, PCI_DEVICE_ID_SUN_SIMBA,
                             pci_apb_map_irq,
                             "Advanced PCI Bus secondary bridge 2");
diff --git a/hw/dec_pci.c b/hw/dec_pci.c
index 83ce4cd..ee49d5a 100644
--- a/hw/dec_pci.c
+++ b/hw/dec_pci.c
@@ -55,7 +55,7 @@ PCIBus *pci_dec_21154_init(PCIBus *parent_bus, int devfn)
 
     dev = qdev_create(NULL, "dec-21154");
     qdev_init_nofail(dev);
-    ret = pci_bridge_init(parent_bus, devfn,
+    ret = pci_bridge_init(parent_bus, devfn, false,
                           PCI_VENDOR_ID_DEC, PCI_DEVICE_ID_DEC_21154,
                           dec_map_irq, "DEC 21154 PCI-PCI bridge");
 
diff --git a/hw/pci.c b/hw/pci.c
index b91d9f5..36b3760 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -1572,13 +1572,14 @@ static int pci_bridge_exitfn(PCIDevice *pci_dev)
     return 0;
 }
 
-PCIBus *pci_bridge_init(PCIBus *bus, int devfn, uint16_t vid, uint16_t did,
+PCIBus *pci_bridge_init(PCIBus *bus, int devfn, bool multifunction,
+                        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");
+    dev = pci_create_multifunction(bus, devfn, multifunction, "pci-bridge");
     qdev_prop_set_uint32(&dev->qdev, "vendorid", vid);
     qdev_prop_set_uint32(&dev->qdev, "deviceid", did);
     qdev_init_nofail(&dev->qdev);
diff --git a/hw/pci.h b/hw/pci.h
index 733a314..4164074 100644
--- a/hw/pci.h
+++ b/hw/pci.h
@@ -234,7 +234,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,
+PCIBus *pci_bridge_init(PCIBus *bus, int devfn, bool multifunction,
+                        uint16_t vid, uint16_t did,
                         pci_map_irq_fn map_irq, const char *name);
 PCIDevice *pci_bridge_get_device(PCIBus *bus);
 
-- 
1.6.6.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PATCH v5 9/9] pci: set PCI multi-function bit appropriately.
  2010-06-23  7:15 [Qemu-devel] [PATCH v5 0/9] pci: multi-function bit fixes Isaku Yamahata
                   ` (7 preceding siblings ...)
  2010-06-23  7:15 ` [Qemu-devel] [PATCH v5 8/9] pci_bridge: make pci bridge aware of pci multi function bit Isaku Yamahata
@ 2010-06-23  7:15 ` Isaku Yamahata
  2010-07-09  1:44 ` [Qemu-devel] [PATCH v5 0/9] pci: multi-function bit fixes Isaku Yamahata
  9 siblings, 0 replies; 15+ messages in thread
From: Isaku Yamahata @ 2010-06-23  7:15 UTC (permalink / raw)
  To: qemu-devel
  Cc: yu.liu, mst, Juan Quintela, blauwirbel, yamahata, paul, aurelien

Set PCI multi-function bit according to multifunction property.
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 this patch allows user to set multifunction bit via property
and checks whether multifunction bit is set correctly.

Cc:  Juan Quintela <quintela@redhat.com>
Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>

---
chages v4 -> v5:
- split out the patch
- fix memory leak pointed out by Juan Quintela <quintela@redhat.com>
- convert multifunction uint8_t property into bit property
  by introducint QEMU_PCI_CAP_MULTIFUNCTION
- s/mf/multifunction/g
- minor style fix

changes v3 -> v4:
- introduce multifunction property.

changes v2 -> v3:
- introduce PCI_FUNC_MAX
- more commit log

changes v1 -> v2:
- don't set header type register in configuration space.
---
 hw/apb_pci.c  |    3 ---
 hw/pci.c      |   52 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 hw/piix4.c    |    2 --
 hw/piix_pci.c |    2 --
 4 files changed, 52 insertions(+), 7 deletions(-)

diff --git a/hw/apb_pci.c b/hw/apb_pci.c
index fd11459..0ecac55 100644
--- a/hw/apb_pci.c
+++ b/hw/apb_pci.c
@@ -312,9 +312,6 @@ static void apb_pci_bridge_init(PCIBus *b)
                  PCI_STATUS_FAST_BACK | PCI_STATUS_66MHZ |
                  PCI_STATUS_DEVSEL_MEDIUM);
     pci_set_byte(dev->config + PCI_REVISION_ID, 0x11);
-    pci_set_byte(dev->config + PCI_HEADER_TYPE,
-                 pci_get_byte(dev->config + PCI_HEADER_TYPE) |
-                 PCI_HEADER_TYPE_MULTI_FUNCTION);
 }
 
 PCIBus *pci_apb_init(target_phys_addr_t special_base,
diff --git a/hw/pci.c b/hw/pci.c
index 36b3760..f4bfbea 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -577,6 +577,54 @@ static void pci_init_wmask_bridge(PCIDevice *d)
     pci_set_word(d->wmask + PCI_BRIDGE_CONTROL, 0xffff);
 }
 
+static int pci_init_multifunction(PCIBus *bus, PCIDevice *dev)
+{
+    uint8_t slot = PCI_SLOT(dev->devfn);
+    uint8_t func;
+
+    if (dev->cap_present & QEMU_PCI_CAP_MULTIFUNCTION) {
+        dev->config[PCI_HEADER_TYPE] |= PCI_HEADER_TYPE_MULTI_FUNCTION;
+    }
+
+    /*
+     * multifuction bit is interpreted in two ways as follows.
+     *   - all functions must set the bit to 1.
+     *     Example: Intel X53
+     *   - function 0 must set the bit, but the rest function (> 0)
+     *     is allowed to leave the bit to 0.
+     *     Example: PIIX3(also in qemu), PIIX4(also in qemu), ICH10,
+     *
+     * So OS (at least Linux) checks the bit of only function 0,
+     * and doesn't see the bit of function > 0.
+     *
+     * The below check allows both interpretation.
+     */
+    if (PCI_FUNC(dev->devfn)) {
+        PCIDevice *f0 = bus->devices[PCI_DEVFN(slot, 0)];
+        if (f0 && !(f0->cap_present & QEMU_PCI_CAP_MULTIFUNCTION)) {
+            /* function 0 should set multifunction bit */
+            error_report("PCI: single function device can't be populated "
+                         "in function %x.%x", slot, PCI_FUNC(dev->devfn));
+            return -1;
+        }
+        return 0;
+    }
+
+    if (dev->cap_present & QEMU_PCI_CAP_MULTIFUNCTION) {
+        return 0;
+    }
+    /* function 0 indicates single function, so function > 0 must be NULL */
+    for (func = 1; func < PCI_FUNC_MAX; ++func) {
+        if (bus->devices[PCI_DEVFN(slot, func)]) {
+            error_report("PCI: %x.0 indicates single function, "
+                         "but %x.%x is already populated.",
+                         slot, slot, func);
+            return -1;
+        }
+    }
+    return 0;
+}
+
 static void pci_config_alloc(PCIDevice *pci_dev)
 {
     int config_size = pci_config_size(pci_dev);
@@ -630,6 +678,10 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
     if (is_bridge) {
         pci_init_wmask_bridge(pci_dev);
     }
+    if (pci_init_multifunction(bus, pci_dev)) {
+        pci_config_free(pci_dev);
+        return NULL;
+    }
 
     if (!config_read)
         config_read = pci_default_read_config;
diff --git a/hw/piix4.c b/hw/piix4.c
index 2298074..58e8f70 100644
--- a/hw/piix4.c
+++ b/hw/piix4.c
@@ -93,8 +93,6 @@ 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
 
     piix4_dev = d;
     qemu_register_reset(piix4_reset, d);
diff --git a/hw/piix_pci.c b/hw/piix_pci.c
index a3185f7..8bb4507 100644
--- a/hw/piix_pci.c
+++ b/hw/piix_pci.c
@@ -336,8 +336,6 @@ 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
 
     qemu_register_reset(piix3_reset, d);
     return 0;
-- 
1.6.6.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Qemu-devel] Re: [PATCH v5 1/9] pci: use PCI_DEVFN() where appropriate.
  2010-06-23  7:15 ` [Qemu-devel] [PATCH v5 1/9] pci: use PCI_DEVFN() where appropriate Isaku Yamahata
@ 2010-06-23  9:59   ` Michael S. Tsirkin
  0 siblings, 0 replies; 15+ messages in thread
From: Michael S. Tsirkin @ 2010-06-23  9:59 UTC (permalink / raw)
  To: Isaku Yamahata; +Cc: blauwirbel, yu.liu, qemu-devel, aurelien, paul

On Wed, Jun 23, 2010 at 04:15:25PM +0900, Isaku Yamahata wrote:
> Use PCI_DEVFN() and PCI_FUNC_MAX where appropriate.
> This patch make it clear that func = 0.
> 
> test:
> The following object files with/without this patch are stripped and compared.
> They remains same.
>   arm-softmmu/versatile_pci.o
>   libhw32/ppce500_pci.o
>   libhw32/unin_pci.o
>   libhw64/ppce500_pci.o
>   libhw64/unin_pci.o
>   mips-softmmu/gt64xxx.o
>   mips64-softmmu/gt64xxx.o
>   mips64el-softmmu/gt64xxx.o
>   mipsel-softmmu/gt64xxx.o
> 
> Cc: Aurelien Jarno <aurelien@aurel32.net>
> Cc: Yu Liu <yu.liu@freescale.com>
> Cc: Paul Brook <paul@codesourcery.com>
> Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>

Applied, thanks!

> ---
> changes v2 -> v3
> - split out into PCI_DEVFN() patch and assert patch.
> - more test by comparing stripped object.
> ---
>  hw/gt64xxx.c       |    2 +-
>  hw/ppce500_pci.c   |    3 ++-
>  hw/unin_pci.c      |   12 ++++++------
>  hw/versatile_pci.c |    2 +-
>  4 files changed, 10 insertions(+), 9 deletions(-)
> 
> diff --git a/hw/gt64xxx.c b/hw/gt64xxx.c
> index 7691e1d..8e2cf14 100644
> --- a/hw/gt64xxx.c
> +++ b/hw/gt64xxx.c
> @@ -1115,7 +1115,7 @@ PCIBus *pci_gt64120_init(qemu_irq *pic)
>  
>      s->pci->bus = pci_register_bus(NULL, "pci",
>                                     pci_gt64120_set_irq, pci_gt64120_map_irq,
> -                                   pic, 144, 4);
> +                                   pic, PCI_DEVFN(18, 0), 4);
>      s->ISD_handle = cpu_register_io_memory(gt64120_read, gt64120_write, s);
>      d = pci_register_device(s->pci->bus, "GT64120 PCI Bus", sizeof(PCIDevice),
>                              0, NULL, NULL);
> diff --git a/hw/ppce500_pci.c b/hw/ppce500_pci.c
> index 336d284..f949fe3 100644
> --- a/hw/ppce500_pci.c
> +++ b/hw/ppce500_pci.c
> @@ -279,7 +279,8 @@ PCIBus *ppce500_pci_init(qemu_irq pci_irqs[4], target_phys_addr_t registers)
>      controller->pci_state.bus = pci_register_bus(NULL, "pci",
>                                                   mpc85xx_pci_set_irq,
>                                                   mpc85xx_pci_map_irq,
> -                                                 pci_irqs, 0x88, 4);
> +                                                 pci_irqs, PCI_DEVFN(0x11, 0),
> +                                                 4);
>      d = pci_register_device(controller->pci_state.bus,
>                              "host bridge", sizeof(PCIDevice),
>                              0, NULL, NULL);
> diff --git a/hw/unin_pci.c b/hw/unin_pci.c
> index f0a773d..0ecf40f 100644
> --- a/hw/unin_pci.c
> +++ b/hw/unin_pci.c
> @@ -228,10 +228,10 @@ PCIBus *pci_pmac_init(qemu_irq *pic)
>      d = FROM_SYSBUS(UNINState, s);
>      d->host_state.bus = pci_register_bus(&d->busdev.qdev, "pci",
>                                           pci_unin_set_irq, pci_unin_map_irq,
> -                                         pic, 11 << 3, 4);
> +                                         pic, PCI_DEVFN(11, 0), 4);
>  
>  #if 0
> -    pci_create_simple(d->host_state.bus, 11 << 3, "uni-north");
> +    pci_create_simple(d->host_state.bus, PCI_DEVFN(11, 0), "uni-north");
>  #endif
>  
>      sysbus_mmio_map(s, 0, 0xf2800000);
> @@ -240,11 +240,11 @@ PCIBus *pci_pmac_init(qemu_irq *pic)
>      /* DEC 21154 bridge */
>  #if 0
>      /* XXX: not activated as PPC BIOS doesn't handle multiple buses properly */
> -    pci_create_simple(d->host_state.bus, 12 << 3, "dec-21154");
> +    pci_create_simple(d->host_state.bus, PCI_DEVFN(12, 0), "dec-21154");
>  #endif
>  
>      /* Uninorth AGP bus */
> -    pci_create_simple(d->host_state.bus, 11 << 3, "uni-north-agp");
> +    pci_create_simple(d->host_state.bus, PCI_DEVFN(11, 0), "uni-north-agp");
>      dev = qdev_create(NULL, "uni-north-agp");
>      qdev_init_nofail(dev);
>      s = sysbus_from_qdev(dev);
> @@ -254,7 +254,7 @@ PCIBus *pci_pmac_init(qemu_irq *pic)
>      /* Uninorth internal bus */
>  #if 0
>      /* XXX: not needed for now */
> -    pci_create_simple(d->host_state.bus, 14 << 3, "uni-north-pci");
> +    pci_create_simple(d->host_state.bus, PCI_DEVFN(14, 0), "uni-north-pci");
>      dev = qdev_create(NULL, "uni-north-pci");
>      qdev_init_nofail(dev);
>      s = sysbus_from_qdev(dev);
> @@ -280,7 +280,7 @@ PCIBus *pci_pmac_u3_init(qemu_irq *pic)
>  
>      d->host_state.bus = pci_register_bus(&d->busdev.qdev, "pci",
>                                           pci_unin_set_irq, pci_unin_map_irq,
> -                                         pic, 11 << 3, 4);
> +                                         pic, PCI_DEVFN(11, 0), 4);
>  
>      sysbus_mmio_map(s, 0, 0xf0800000);
>      sysbus_mmio_map(s, 1, 0xf0c00000);
> diff --git a/hw/versatile_pci.c b/hw/versatile_pci.c
> index 199bc19..a76bdfa 100644
> --- a/hw/versatile_pci.c
> +++ b/hw/versatile_pci.c
> @@ -127,7 +127,7 @@ static int pci_vpb_init(SysBusDevice *dev)
>      }
>      bus = pci_register_bus(&dev->qdev, "pci",
>                             pci_vpb_set_irq, pci_vpb_map_irq, s->irq,
> -                           11 << 3, 4);
> +                           PCI_DEVFN(11, 0), 4);
>  
>      /* ??? Register memory space.  */
>  
> -- 
> 1.6.6.1

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [Qemu-devel] Re: [PATCH v5 2/9] pci: insert assert that auto-assigned-address function is single function device.
  2010-06-23  7:15 ` [Qemu-devel] [PATCH v5 2/9] pci: insert assert that auto-assigned-address function is single function device Isaku Yamahata
@ 2010-06-23 10:11   ` Michael S. Tsirkin
  2010-06-23 23:42     ` Isaku Yamahata
  0 siblings, 1 reply; 15+ messages in thread
From: Michael S. Tsirkin @ 2010-06-23 10:11 UTC (permalink / raw)
  To: Isaku Yamahata; +Cc: blauwirbel, yu.liu, qemu-devel, aurelien, paul

On Wed, Jun 23, 2010 at 04:15:26PM +0900, Isaku Yamahata wrote:
> Auto-assigned-address pci function (passing devfn = -1) is always
> single function.
> This patch adds assert() to guarantee that auto-assigned-address function
> is always single function device at function = 0.
> 
> Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>

So this means we must pass PCI_DEVFN(x, 0) to pci_register_bus?

> ---
> changes v2 -> v3
> split out into PCI_DEVFN() patch and assert patch.
> ---
>  hw/pci.c |    3 ++-
>  hw/pci.h |    1 +
>  2 files changed, 3 insertions(+), 1 deletions(-)
> 
> diff --git a/hw/pci.c b/hw/pci.c
> index ef17eb4..1ba209f 100644
> --- a/hw/pci.c
> +++ b/hw/pci.c
> @@ -224,6 +224,7 @@ void pci_bus_new_inplace(PCIBus *bus, DeviceState *parent,
>                           const char *name, int devfn_min)
>  {
>      qbus_create_inplace(&bus->qbus, &pci_bus_info, parent, name);
> +    assert(PCI_FUNC(devfn_min) == 0);
>      bus->devfn_min = devfn_min;
>      QLIST_INIT(&bus->child);
>      vmstate_register(-1, &vmstate_pcibus, bus);
> @@ -601,7 +602,7 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
>  {
>      if (devfn < 0) {
>          for(devfn = bus->devfn_min ; devfn < ARRAY_SIZE(bus->devices);
> -            devfn += 8) {
> +            devfn += PCI_FUNC_MAX) {
>              if (!bus->devices[devfn])
>                  goto found;
>          }
> diff --git a/hw/pci.h b/hw/pci.h
> index 6a2bc6a..077387d 100644
> --- a/hw/pci.h
> +++ b/hw/pci.h
> @@ -14,6 +14,7 @@
>  #define PCI_DEVFN(slot, func)   ((((slot) & 0x1f) << 3) | ((func) & 0x07))
>  #define PCI_SLOT(devfn)         (((devfn) >> 3) & 0x1f)
>  #define PCI_FUNC(devfn)         ((devfn) & 0x07)
> +#define PCI_FUNC_MAX            8
>  
>  /* Class, Vendor and Device IDs from Linux's pci_ids.h */
>  #include "pci_ids.h"
> -- 
> 1.6.6.1

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [Qemu-devel] Re: [PATCH v5 2/9] pci: insert assert that auto-assigned-address function is single function device.
  2010-06-23 10:11   ` [Qemu-devel] " Michael S. Tsirkin
@ 2010-06-23 23:42     ` Isaku Yamahata
  0 siblings, 0 replies; 15+ messages in thread
From: Isaku Yamahata @ 2010-06-23 23:42 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: blauwirbel, yu.liu, qemu-devel, aurelien, paul

On Wed, Jun 23, 2010 at 01:11:00PM +0300, Michael S. Tsirkin wrote:
> On Wed, Jun 23, 2010 at 04:15:26PM +0900, Isaku Yamahata wrote:
> > Auto-assigned-address pci function (passing devfn = -1) is always
> > single function.
> > This patch adds assert() to guarantee that auto-assigned-address function
> > is always single function device at function = 0.
> > 
> > Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
> 
> So this means we must pass PCI_DEVFN(x, 0) to pci_register_bus?

Yes.
devfn_min is used only for devfn auto assignment.
By checking the commit logs, it seems that the changeset
which introduced devfn_min easily chose devfn than dev.


---
commit 30468f786c127fc027d84c0aec6155e3e59475bb
Author: bellard <bellard@c046a42c-6fe2-441c-8c8c-71466251a162>
Date:   Mon Jun 21 19:45:35 2004 +0000

    added PCI bus - added IRQ support for PowerPC bridges - suppressed PREP PCI 
bios init

---
commit 502a53952d574717bdb626b651b16cadacab46f4
Author: pbrook <pbrook@c046a42c-6fe2-441c-8c8c-71466251a162>
Date:   Sat May 13 16:11:23 2006 +0000

    Rearrange PCI host emulation code.
    Add ARM PCI emulation.



> 
> > ---
> > changes v2 -> v3
> > split out into PCI_DEVFN() patch and assert patch.
> > ---
> >  hw/pci.c |    3 ++-
> >  hw/pci.h |    1 +
> >  2 files changed, 3 insertions(+), 1 deletions(-)
> > 
> > diff --git a/hw/pci.c b/hw/pci.c
> > index ef17eb4..1ba209f 100644
> > --- a/hw/pci.c
> > +++ b/hw/pci.c
> > @@ -224,6 +224,7 @@ void pci_bus_new_inplace(PCIBus *bus, DeviceState *parent,
> >                           const char *name, int devfn_min)
> >  {
> >      qbus_create_inplace(&bus->qbus, &pci_bus_info, parent, name);
> > +    assert(PCI_FUNC(devfn_min) == 0);
> >      bus->devfn_min = devfn_min;
> >      QLIST_INIT(&bus->child);
> >      vmstate_register(-1, &vmstate_pcibus, bus);
> > @@ -601,7 +602,7 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
> >  {
> >      if (devfn < 0) {
> >          for(devfn = bus->devfn_min ; devfn < ARRAY_SIZE(bus->devices);
> > -            devfn += 8) {
> > +            devfn += PCI_FUNC_MAX) {
> >              if (!bus->devices[devfn])
> >                  goto found;
> >          }
> > diff --git a/hw/pci.h b/hw/pci.h
> > index 6a2bc6a..077387d 100644
> > --- a/hw/pci.h
> > +++ b/hw/pci.h
> > @@ -14,6 +14,7 @@
> >  #define PCI_DEVFN(slot, func)   ((((slot) & 0x1f) << 3) | ((func) & 0x07))
> >  #define PCI_SLOT(devfn)         (((devfn) >> 3) & 0x1f)
> >  #define PCI_FUNC(devfn)         ((devfn) & 0x07)
> > +#define PCI_FUNC_MAX            8
> >  
> >  /* Class, Vendor and Device IDs from Linux's pci_ids.h */
> >  #include "pci_ids.h"
> > -- 
> > 1.6.6.1
> 

-- 
yamahata

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [PATCH v5 0/9] pci: multi-function bit fixes
  2010-06-23  7:15 [Qemu-devel] [PATCH v5 0/9] pci: multi-function bit fixes Isaku Yamahata
                   ` (8 preceding siblings ...)
  2010-06-23  7:15 ` [Qemu-devel] [PATCH v5 9/9] pci: set PCI multi-function bit appropriately Isaku Yamahata
@ 2010-07-09  1:44 ` Isaku Yamahata
  2010-07-11 19:26   ` Blue Swirl
  9 siblings, 1 reply; 15+ messages in thread
From: Isaku Yamahata @ 2010-07-09  1:44 UTC (permalink / raw)
  To: qemu-devel; +Cc: blauwirbel, yu.liu, paul, aurelien, mst

Ping?
Since this is bug fix patches, I'd like to include them
into the next release.

On Wed, Jun 23, 2010 at 04:15:24PM +0900, Isaku Yamahata wrote:
> Maing changes v4 -> v5:
> - split up/reorder patches for bisectability and testability
> - stype fixes.
> 
> patch description:
> 
> When pci devices are populated as multi-function,
> OS can fail to probe function > 0. It's because multi function
> bit of header type register in configuration space isn't set,
> so OS probes only function 0 skipping function > 0 as optimization.
> 
> This patch set make qemu set multi function bit when function > 0 
> is populated.
> 
> Changes v3 -> v4:
> - introduced multifunction property and add validation check to catch
>   user operation errors.
> - some more clean up.
> 
> 
> Isaku Yamahata (9):
>   pci: use PCI_DEVFN() where appropriate.
>   pci: insert assert that auto-assigned-address function is single
>     function device.
>   pci: don't overwrite multi functio bit in pci header type.
>   pci: remove PCIDeviceInfo::header_type
>   qdev: implement qdev_prop_set_bit().
>   pci: introduce multifunction property.
>   pci: set multifunction property for normal device.
>   pci_bridge: make pci bridge aware of pci multi function bit.
>   pci: set PCI multi-function bit appropriately.
> 
>  hw/ac97.c            |    1 -
>  hw/acpi_piix4.c      |    1 -
>  hw/apb_pci.c         |   11 ++----
>  hw/dec_pci.c         |    4 +-
>  hw/grackle_pci.c     |    1 -
>  hw/gt64xxx.c         |    2 +-
>  hw/ide/cmd646.c      |    1 -
>  hw/ide/piix.c        |    1 -
>  hw/macio.c           |    1 -
>  hw/ne2000.c          |    1 -
>  hw/openpic.c         |    1 -
>  hw/pci.c             |   97 +++++++++++++++++++++++++++++++++++++++++++-------
>  hw/pci.h             |   21 +++++++++--
>  hw/pcnet.c           |    1 -
>  hw/piix4.c           |    4 +--
>  hw/piix_pci.c        |    5 +--
>  hw/ppce500_pci.c     |    3 +-
>  hw/prep_pci.c        |    1 -
>  hw/qdev-properties.c |    5 +++
>  hw/qdev.h            |    1 +
>  hw/rtl8139.c         |    1 -
>  hw/sun4u.c           |    1 -
>  hw/unin_pci.c        |   16 +++-----
>  hw/usb-uhci.c        |    1 -
>  hw/versatile_pci.c   |    2 +-
>  hw/vga-pci.c         |    1 -
>  hw/virtio-pci.c      |    1 -
>  hw/vmware_vga.c      |    1 -
>  hw/wdt_i6300esb.c    |    1 -
>  29 files changed, 125 insertions(+), 63 deletions(-)
> 
> 

-- 
yamahata

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [PATCH v5 0/9] pci: multi-function bit fixes
  2010-07-09  1:44 ` [Qemu-devel] [PATCH v5 0/9] pci: multi-function bit fixes Isaku Yamahata
@ 2010-07-11 19:26   ` Blue Swirl
  0 siblings, 0 replies; 15+ messages in thread
From: Blue Swirl @ 2010-07-11 19:26 UTC (permalink / raw)
  To: Isaku Yamahata; +Cc: paul, yu.liu, qemu-devel, aurelien, mst

Thanks, applied all.

On Fri, Jul 9, 2010 at 1:44 AM, Isaku Yamahata <yamahata@valinux.co.jp> wrote:
> Ping?
> Since this is bug fix patches, I'd like to include them
> into the next release.
>
> On Wed, Jun 23, 2010 at 04:15:24PM +0900, Isaku Yamahata wrote:
>> Maing changes v4 -> v5:
>> - split up/reorder patches for bisectability and testability
>> - stype fixes.
>>
>> patch description:
>>
>> When pci devices are populated as multi-function,
>> OS can fail to probe function > 0. It's because multi function
>> bit of header type register in configuration space isn't set,
>> so OS probes only function 0 skipping function > 0 as optimization.
>>
>> This patch set make qemu set multi function bit when function > 0
>> is populated.
>>
>> Changes v3 -> v4:
>> - introduced multifunction property and add validation check to catch
>>   user operation errors.
>> - some more clean up.
>>
>>
>> Isaku Yamahata (9):
>>   pci: use PCI_DEVFN() where appropriate.
>>   pci: insert assert that auto-assigned-address function is single
>>     function device.
>>   pci: don't overwrite multi functio bit in pci header type.
>>   pci: remove PCIDeviceInfo::header_type
>>   qdev: implement qdev_prop_set_bit().
>>   pci: introduce multifunction property.
>>   pci: set multifunction property for normal device.
>>   pci_bridge: make pci bridge aware of pci multi function bit.
>>   pci: set PCI multi-function bit appropriately.
>>
>>  hw/ac97.c            |    1 -
>>  hw/acpi_piix4.c      |    1 -
>>  hw/apb_pci.c         |   11 ++----
>>  hw/dec_pci.c         |    4 +-
>>  hw/grackle_pci.c     |    1 -
>>  hw/gt64xxx.c         |    2 +-
>>  hw/ide/cmd646.c      |    1 -
>>  hw/ide/piix.c        |    1 -
>>  hw/macio.c           |    1 -
>>  hw/ne2000.c          |    1 -
>>  hw/openpic.c         |    1 -
>>  hw/pci.c             |   97 +++++++++++++++++++++++++++++++++++++++++++-------
>>  hw/pci.h             |   21 +++++++++--
>>  hw/pcnet.c           |    1 -
>>  hw/piix4.c           |    4 +--
>>  hw/piix_pci.c        |    5 +--
>>  hw/ppce500_pci.c     |    3 +-
>>  hw/prep_pci.c        |    1 -
>>  hw/qdev-properties.c |    5 +++
>>  hw/qdev.h            |    1 +
>>  hw/rtl8139.c         |    1 -
>>  hw/sun4u.c           |    1 -
>>  hw/unin_pci.c        |   16 +++-----
>>  hw/usb-uhci.c        |    1 -
>>  hw/versatile_pci.c   |    2 +-
>>  hw/vga-pci.c         |    1 -
>>  hw/virtio-pci.c      |    1 -
>>  hw/vmware_vga.c      |    1 -
>>  hw/wdt_i6300esb.c    |    1 -
>>  29 files changed, 125 insertions(+), 63 deletions(-)
>>
>>
>
> --
> yamahata
>

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2010-07-11 19:26 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-06-23  7:15 [Qemu-devel] [PATCH v5 0/9] pci: multi-function bit fixes Isaku Yamahata
2010-06-23  7:15 ` [Qemu-devel] [PATCH v5 1/9] pci: use PCI_DEVFN() where appropriate Isaku Yamahata
2010-06-23  9:59   ` [Qemu-devel] " Michael S. Tsirkin
2010-06-23  7:15 ` [Qemu-devel] [PATCH v5 2/9] pci: insert assert that auto-assigned-address function is single function device Isaku Yamahata
2010-06-23 10:11   ` [Qemu-devel] " Michael S. Tsirkin
2010-06-23 23:42     ` Isaku Yamahata
2010-06-23  7:15 ` [Qemu-devel] [PATCH v5 3/9] pci: don't overwrite multi functio bit in pci header type Isaku Yamahata
2010-06-23  7:15 ` [Qemu-devel] [PATCH v5 4/9] pci: remove PCIDeviceInfo::header_type Isaku Yamahata
2010-06-23  7:15 ` [Qemu-devel] [PATCH v5 5/9] qdev: implement qdev_prop_set_bit() Isaku Yamahata
2010-06-23  7:15 ` [Qemu-devel] [PATCH v5 6/9] pci: introduce multifunction property Isaku Yamahata
2010-06-23  7:15 ` [Qemu-devel] [PATCH v5 7/9] pci: set multifunction property for normal device Isaku Yamahata
2010-06-23  7:15 ` [Qemu-devel] [PATCH v5 8/9] pci_bridge: make pci bridge aware of pci multi function bit Isaku Yamahata
2010-06-23  7:15 ` [Qemu-devel] [PATCH v5 9/9] pci: set PCI multi-function bit appropriately Isaku Yamahata
2010-07-09  1:44 ` [Qemu-devel] [PATCH v5 0/9] pci: multi-function bit fixes Isaku Yamahata
2010-07-11 19:26   ` Blue Swirl

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.