qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] pci: introduce get_info_quirk callback.
@ 2010-02-12  2:31 Isaku Yamahata
  2010-02-12 12:54 ` [Qemu-devel] " Michael S. Tsirkin
  0 siblings, 1 reply; 5+ messages in thread
From: Isaku Yamahata @ 2010-02-12  2:31 UTC (permalink / raw)
  To: qemu-devel; +Cc: Blue Swirl, Michael S. Tsirkin

This patch fixes 525e05147d5a3bdc08caa422d108c1ef71b584b5
by introducing device specific get_info_quirk callback.
It wrongly assumes that pci host bridge class device has
header type of pci-pci bridge. But this isn't always true.
In fact i440fx pci host bridge has header type of normal device,
hence it breaks i440fx and other pci host bridges.
The right fix is that header type should be checked, instead of device class.

The change set's purpose is to show PBM pci host bridge
info which doesn't conform to PCI specification.
So introduce get_info_quirk callback and use it for PBM.

Cc: Blue Swirl <blauwirbel@gmail.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
---
 hw/apb_pci.c |    1 +
 hw/pci.c     |   79 ++++++++++++++++++++++++++++++++-------------------------
 hw/pci.h     |    3 ++
 3 files changed, 48 insertions(+), 35 deletions(-)

diff --git a/hw/apb_pci.c b/hw/apb_pci.c
index 46d5b0e..a557469 100644
--- a/hw/apb_pci.c
+++ b/hw/apb_pci.c
@@ -479,6 +479,7 @@ static PCIDeviceInfo pbm_pci_host_info = {
     .qdev.name = "pbm",
     .qdev.size = sizeof(PCIDevice),
     .init      = pbm_pci_host_init,
+    .get_info_quirk = pci_bridge_get_info,
     .header_type  = PCI_HEADER_TYPE_BRIDGE,
 };
 
diff --git a/hw/pci.c b/hw/pci.c
index 9ad63dd..1e2bde8 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -1271,10 +1271,44 @@ static QObject *pci_get_regions_list(const PCIDevice *dev)
 
 static QObject *pci_get_devices_list(PCIBus *bus, int bus_num);
 
+void pci_bridge_get_info(PCIDevice *dev, PCIBus *bus, QDict *qdict)
+{
+    QObject *pci_bridge;
+
+    pci_bridge = qobject_from_jsonf("{ 'bus': "
+    "{ 'number': %d, 'secondary': %d, 'subordinate': %d }, "
+    "'io_range': { 'base': %" PRId64 ", 'limit': %" PRId64 "}, "
+    "'memory_range': { 'base': %" PRId64 ", 'limit': %" PRId64 "}, "
+    "'prefetchable_range': { 'base': %" PRId64 ", 'limit': %" PRId64 "} }",
+    dev->config[PCI_PRIMARY_BUS], dev->config[PCI_SECONDARY_BUS],
+    dev->config[PCI_SUBORDINATE_BUS],
+    pci_bridge_get_base(dev, PCI_BASE_ADDRESS_SPACE_IO),
+    pci_bridge_get_limit(dev, PCI_BASE_ADDRESS_SPACE_IO),
+    pci_bridge_get_base(dev, PCI_BASE_ADDRESS_SPACE_MEMORY),
+    pci_bridge_get_limit(dev, PCI_BASE_ADDRESS_SPACE_MEMORY),
+    pci_bridge_get_base(dev, PCI_BASE_ADDRESS_SPACE_MEMORY |
+                        PCI_BASE_ADDRESS_MEM_PREFETCH),
+    pci_bridge_get_limit(dev, PCI_BASE_ADDRESS_SPACE_MEMORY |
+                         PCI_BASE_ADDRESS_MEM_PREFETCH));
+
+    if (dev->config[PCI_SECONDARY_BUS] != 0) {
+        PCIBus *child_bus = pci_find_bus(bus, dev->config[PCI_SECONDARY_BUS]);
+        if (child_bus) {
+            qdict_put_obj(qobject_to_qdict(pci_bridge), "devices",
+                          pci_get_devices_list(child_bus,
+                                               dev->config[PCI_SECONDARY_BUS]));
+        }
+    }
+
+    qdict_put_obj(qdict, "pci_bridge", pci_bridge);
+}
+
 static QObject *pci_get_dev_dict(PCIDevice *dev, PCIBus *bus, int bus_num)
 {
-    int class;
+    PCIDeviceInfo *info = DO_UPCAST(PCIDeviceInfo, qdev, dev->qdev.info);
     QObject *obj;
+    QDict *qdict;
+    uint8_t type;
 
     obj = qobject_from_jsonf("{ 'bus': %d, 'slot': %d, 'function': %d,"                                       "'class_info': %p, 'id': %p, 'regions': %p,"
                               " 'qdev_id': %s }",
@@ -1283,45 +1317,20 @@ static QObject *pci_get_dev_dict(PCIDevice *dev, PCIBus *bus, int bus_num)
                               pci_get_dev_class(dev), pci_get_dev_id(dev),
                               pci_get_regions_list(dev),
                               dev->qdev.id ? dev->qdev.id : "");
+    qdict = qobject_to_qdict(obj);
 
     if (dev->config[PCI_INTERRUPT_PIN] != 0) {
-        QDict *qdict = qobject_to_qdict(obj);
         qdict_put(qdict, "irq", qint_from_int(dev->config[PCI_INTERRUPT_LINE]));
     }
 
-    class = pci_get_word(dev->config + PCI_CLASS_DEVICE);
-    if (class == PCI_CLASS_BRIDGE_HOST || class == PCI_CLASS_BRIDGE_PCI) {
-        QDict *qdict;
-        QObject *pci_bridge;
-
-        pci_bridge = qobject_from_jsonf("{ 'bus': "
-        "{ 'number': %d, 'secondary': %d, 'subordinate': %d }, "
-        "'io_range': { 'base': %" PRId64 ", 'limit': %" PRId64 "}, "
-        "'memory_range': { 'base': %" PRId64 ", 'limit': %" PRId64 "}, "
-        "'prefetchable_range': { 'base': %" PRId64 ", 'limit': %" PRId64 "} }",
-        dev->config[PCI_PRIMARY_BUS], dev->config[PCI_SECONDARY_BUS],
-        dev->config[PCI_SUBORDINATE_BUS],
-        pci_bridge_get_base(dev, PCI_BASE_ADDRESS_SPACE_IO),
-        pci_bridge_get_limit(dev, PCI_BASE_ADDRESS_SPACE_IO),
-        pci_bridge_get_base(dev, PCI_BASE_ADDRESS_SPACE_MEMORY),
-        pci_bridge_get_limit(dev, PCI_BASE_ADDRESS_SPACE_MEMORY),
-        pci_bridge_get_base(dev, PCI_BASE_ADDRESS_SPACE_MEMORY |
-                               PCI_BASE_ADDRESS_MEM_PREFETCH),
-        pci_bridge_get_limit(dev, PCI_BASE_ADDRESS_SPACE_MEMORY |
-                                PCI_BASE_ADDRESS_MEM_PREFETCH));
-
-        if (dev->config[PCI_SECONDARY_BUS] != 0) {
-            PCIBus *child_bus = pci_find_bus(bus, dev->config[PCI_SECONDARY_BUS]);
-
-            if (child_bus) {
-                qdict = qobject_to_qdict(pci_bridge);
-                qdict_put_obj(qdict, "devices",
-                              pci_get_devices_list(child_bus,
-                                                   dev->config[PCI_SECONDARY_BUS]));
-            }
-        }
-        qdict = qobject_to_qdict(obj);
-        qdict_put_obj(qdict, "pci_bridge", pci_bridge);
+    type = dev->config[PCI_HEADER_TYPE] & ~PCI_HEADER_TYPE_MULTI_FUNCTION;
+    if (type == PCI_HEADER_TYPE_BRIDGE) {
+        pci_bridge_get_info(dev, bus, qdict);
+    }
+
+    if (info /* not all pci device aren't converted qdev yet */ &&
+        info->get_info_quirk) {
+        info->get_info_quirk(dev, bus, qdict);
     }
 
     return obj;
diff --git a/hw/pci.h b/hw/pci.h
index 8b511d2..5b87acd 100644
--- a/hw/pci.h
+++ b/hw/pci.h
@@ -80,6 +80,7 @@ typedef uint32_t PCIConfigReadFunc(PCIDevice *pci_dev,
 typedef void PCIMapIORegionFunc(PCIDevice *pci_dev, int region_num,
                                 pcibus_t addr, pcibus_t size, int type);
 typedef int PCIUnregisterFunc(PCIDevice *pci_dev);
+typedef void PCIGetInfoQuirkFunc(PCIDevice *pci_dev, PCIBus *bus, QDict *qdict);
 
 typedef struct PCIIORegion {
     pcibus_t addr; /* current PCI mapping address. -1 means not mapped */
@@ -240,6 +241,7 @@ 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_get_info(PCIDevice *dev, PCIBus *bus, QDict *qdict);
 
 static inline void
 pci_set_byte(uint8_t *config, uint8_t val)
@@ -314,6 +316,7 @@ typedef struct {
     PCIUnregisterFunc *exit;
     PCIConfigReadFunc *config_read;
     PCIConfigWriteFunc *config_write;
+    PCIGetInfoQuirkFunc *get_info_quirk;
 
     /* pci config header type */
     uint8_t header_type;
-- 
1.6.6.1

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

* [Qemu-devel] Re: [PATCH] pci: introduce get_info_quirk callback.
  2010-02-12  2:31 [Qemu-devel] [PATCH] pci: introduce get_info_quirk callback Isaku Yamahata
@ 2010-02-12 12:54 ` Michael S. Tsirkin
  2010-02-12 13:34   ` Isaku Yamahata
  0 siblings, 1 reply; 5+ messages in thread
From: Michael S. Tsirkin @ 2010-02-12 12:54 UTC (permalink / raw)
  To: Isaku Yamahata; +Cc: Blue Swirl, qemu-devel

On Fri, Feb 12, 2010 at 11:31:34AM +0900, Isaku Yamahata wrote:
> This patch fixes 525e05147d5a3bdc08caa422d108c1ef71b584b5
> by introducing device specific get_info_quirk callback.
> It wrongly assumes that pci host bridge class device has
> header type of pci-pci bridge. But this isn't always true.
> In fact i440fx pci host bridge has header type of normal device,
> hence it breaks i440fx and other pci host bridges.
> The right fix is that header type should be checked, instead of device class.
> 
> The change set's purpose is to show PBM pci host bridge
> info which doesn't conform to PCI specification.

So, PBM has header type PCI_HEADER_TYPE_NORMAL
but all config space is in bridge format?

> So introduce get_info_quirk callback and use it for PBM.
> 
> Cc: Blue Swirl <blauwirbel@gmail.com>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
> ---
>  hw/apb_pci.c |    1 +
>  hw/pci.c     |   79 ++++++++++++++++++++++++++++++++-------------------------
>  hw/pci.h     |    3 ++
>  3 files changed, 48 insertions(+), 35 deletions(-)
> 
> diff --git a/hw/apb_pci.c b/hw/apb_pci.c
> index 46d5b0e..a557469 100644
> --- a/hw/apb_pci.c
> +++ b/hw/apb_pci.c
> @@ -479,6 +479,7 @@ static PCIDeviceInfo pbm_pci_host_info = {
>      .qdev.name = "pbm",
>      .qdev.size = sizeof(PCIDevice),
>      .init      = pbm_pci_host_init,
> +    .get_info_quirk = pci_bridge_get_info,
>      .header_type  = PCI_HEADER_TYPE_BRIDGE,
>  };
>  
> diff --git a/hw/pci.c b/hw/pci.c
> index 9ad63dd..1e2bde8 100644
> --- a/hw/pci.c
> +++ b/hw/pci.c
> @@ -1271,10 +1271,44 @@ static QObject *pci_get_regions_list(const PCIDevice *dev)
>  
>  static QObject *pci_get_devices_list(PCIBus *bus, int bus_num);
>  
> +void pci_bridge_get_info(PCIDevice *dev, PCIBus *bus, QDict *qdict)
> +{
> +    QObject *pci_bridge;
> +
> +    pci_bridge = qobject_from_jsonf("{ 'bus': "
> +    "{ 'number': %d, 'secondary': %d, 'subordinate': %d }, "
> +    "'io_range': { 'base': %" PRId64 ", 'limit': %" PRId64 "}, "
> +    "'memory_range': { 'base': %" PRId64 ", 'limit': %" PRId64 "}, "
> +    "'prefetchable_range': { 'base': %" PRId64 ", 'limit': %" PRId64 "} }",
> +    dev->config[PCI_PRIMARY_BUS], dev->config[PCI_SECONDARY_BUS],
> +    dev->config[PCI_SUBORDINATE_BUS],
> +    pci_bridge_get_base(dev, PCI_BASE_ADDRESS_SPACE_IO),
> +    pci_bridge_get_limit(dev, PCI_BASE_ADDRESS_SPACE_IO),
> +    pci_bridge_get_base(dev, PCI_BASE_ADDRESS_SPACE_MEMORY),
> +    pci_bridge_get_limit(dev, PCI_BASE_ADDRESS_SPACE_MEMORY),
> +    pci_bridge_get_base(dev, PCI_BASE_ADDRESS_SPACE_MEMORY |
> +                        PCI_BASE_ADDRESS_MEM_PREFETCH),
> +    pci_bridge_get_limit(dev, PCI_BASE_ADDRESS_SPACE_MEMORY |
> +                         PCI_BASE_ADDRESS_MEM_PREFETCH));
> +
> +    if (dev->config[PCI_SECONDARY_BUS] != 0) {
> +        PCIBus *child_bus = pci_find_bus(bus, dev->config[PCI_SECONDARY_BUS]);
> +        if (child_bus) {
> +            qdict_put_obj(qobject_to_qdict(pci_bridge), "devices",
> +                          pci_get_devices_list(child_bus,
> +                                               dev->config[PCI_SECONDARY_BUS]));
> +        }
> +    }
> +
> +    qdict_put_obj(qdict, "pci_bridge", pci_bridge);
> +}
> +
>  static QObject *pci_get_dev_dict(PCIDevice *dev, PCIBus *bus, int bus_num)
>  {
> -    int class;
> +    PCIDeviceInfo *info = DO_UPCAST(PCIDeviceInfo, qdev, dev->qdev.info);
>      QObject *obj;
> +    QDict *qdict;
> +    uint8_t type;
>  
>      obj = qobject_from_jsonf("{ 'bus': %d, 'slot': %d, 'function': %d,"                                       "'class_info': %p, 'id': %p, 'regions': %p,"
>                                " 'qdev_id': %s }",
> @@ -1283,45 +1317,20 @@ static QObject *pci_get_dev_dict(PCIDevice *dev, PCIBus *bus, int bus_num)
>                                pci_get_dev_class(dev), pci_get_dev_id(dev),
>                                pci_get_regions_list(dev),
>                                dev->qdev.id ? dev->qdev.id : "");
> +    qdict = qobject_to_qdict(obj);
>  
>      if (dev->config[PCI_INTERRUPT_PIN] != 0) {
> -        QDict *qdict = qobject_to_qdict(obj);
>          qdict_put(qdict, "irq", qint_from_int(dev->config[PCI_INTERRUPT_LINE]));
>      }
>  
> -    class = pci_get_word(dev->config + PCI_CLASS_DEVICE);
> -    if (class == PCI_CLASS_BRIDGE_HOST || class == PCI_CLASS_BRIDGE_PCI) {
> -        QDict *qdict;
> -        QObject *pci_bridge;
> -
> -        pci_bridge = qobject_from_jsonf("{ 'bus': "
> -        "{ 'number': %d, 'secondary': %d, 'subordinate': %d }, "
> -        "'io_range': { 'base': %" PRId64 ", 'limit': %" PRId64 "}, "
> -        "'memory_range': { 'base': %" PRId64 ", 'limit': %" PRId64 "}, "
> -        "'prefetchable_range': { 'base': %" PRId64 ", 'limit': %" PRId64 "} }",
> -        dev->config[PCI_PRIMARY_BUS], dev->config[PCI_SECONDARY_BUS],
> -        dev->config[PCI_SUBORDINATE_BUS],
> -        pci_bridge_get_base(dev, PCI_BASE_ADDRESS_SPACE_IO),
> -        pci_bridge_get_limit(dev, PCI_BASE_ADDRESS_SPACE_IO),
> -        pci_bridge_get_base(dev, PCI_BASE_ADDRESS_SPACE_MEMORY),
> -        pci_bridge_get_limit(dev, PCI_BASE_ADDRESS_SPACE_MEMORY),
> -        pci_bridge_get_base(dev, PCI_BASE_ADDRESS_SPACE_MEMORY |
> -                               PCI_BASE_ADDRESS_MEM_PREFETCH),
> -        pci_bridge_get_limit(dev, PCI_BASE_ADDRESS_SPACE_MEMORY |
> -                                PCI_BASE_ADDRESS_MEM_PREFETCH));
> -
> -        if (dev->config[PCI_SECONDARY_BUS] != 0) {
> -            PCIBus *child_bus = pci_find_bus(bus, dev->config[PCI_SECONDARY_BUS]);
> -
> -            if (child_bus) {
> -                qdict = qobject_to_qdict(pci_bridge);
> -                qdict_put_obj(qdict, "devices",
> -                              pci_get_devices_list(child_bus,
> -                                                   dev->config[PCI_SECONDARY_BUS]));
> -            }
> -        }
> -        qdict = qobject_to_qdict(obj);
> -        qdict_put_obj(qdict, "pci_bridge", pci_bridge);
> +    type = dev->config[PCI_HEADER_TYPE] & ~PCI_HEADER_TYPE_MULTI_FUNCTION;
> +    if (type == PCI_HEADER_TYPE_BRIDGE) {
> +        pci_bridge_get_info(dev, bus, qdict);
> +    }
> +
> +    if (info /* not all pci device aren't converted qdev yet */ &&
> +        info->get_info_quirk) {
> +        info->get_info_quirk(dev, bus, qdict);
>      }

this should be:

>      if (info /* not all pci device aren't converted qdev yet */ &&
>          info->get_info_quirk) {
>          info->get_info_quirk(dev, bus, qdict);
>      } else if type == PCI_HEADER_TYPE_BRIDGE) {
>          pci_bridge_get_info(dev, bus, qdict);
       }


>  
>      return obj;
> diff --git a/hw/pci.h b/hw/pci.h
> index 8b511d2..5b87acd 100644
> --- a/hw/pci.h
> +++ b/hw/pci.h
> @@ -80,6 +80,7 @@ typedef uint32_t PCIConfigReadFunc(PCIDevice *pci_dev,
>  typedef void PCIMapIORegionFunc(PCIDevice *pci_dev, int region_num,
>                                  pcibus_t addr, pcibus_t size, int type);
>  typedef int PCIUnregisterFunc(PCIDevice *pci_dev);
> +typedef void PCIGetInfoQuirkFunc(PCIDevice *pci_dev, PCIBus *bus, QDict *qdict);
>  
>  typedef struct PCIIORegion {
>      pcibus_t addr; /* current PCI mapping address. -1 means not mapped */
> @@ -240,6 +241,7 @@ 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_get_info(PCIDevice *dev, PCIBus *bus, QDict *qdict);
>  
>  static inline void
>  pci_set_byte(uint8_t *config, uint8_t val)
> @@ -314,6 +316,7 @@ typedef struct {
>      PCIUnregisterFunc *exit;
>      PCIConfigReadFunc *config_read;
>      PCIConfigWriteFunc *config_write;
> +    PCIGetInfoQuirkFunc *get_info_quirk;
>  
>      /* pci config header type */
>      uint8_t header_type;
> -- 
> 1.6.6.1

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

* [Qemu-devel] Re: [PATCH] pci: introduce get_info_quirk callback.
  2010-02-12 12:54 ` [Qemu-devel] " Michael S. Tsirkin
@ 2010-02-12 13:34   ` Isaku Yamahata
  2010-02-12 20:10     ` Blue Swirl
  0 siblings, 1 reply; 5+ messages in thread
From: Isaku Yamahata @ 2010-02-12 13:34 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: Blue Swirl, qemu-devel

On Fri, Feb 12, 2010 at 02:54:59PM +0200, Michael S. Tsirkin wrote:
> On Fri, Feb 12, 2010 at 11:31:34AM +0900, Isaku Yamahata wrote:
> > This patch fixes 525e05147d5a3bdc08caa422d108c1ef71b584b5
> > by introducing device specific get_info_quirk callback.
> > It wrongly assumes that pci host bridge class device has
> > header type of pci-pci bridge. But this isn't always true.
> > In fact i440fx pci host bridge has header type of normal device,
> > hence it breaks i440fx and other pci host bridges.
> > The right fix is that header type should be checked, instead of device class.
> > 
> > The change set's purpose is to show PBM pci host bridge
> > info which doesn't conform to PCI specification.
> 
> So, PBM has header type PCI_HEADER_TYPE_NORMAL
> but all config space is in bridge format?

Some of registers is in bridge format.
To be honest, I don't know whether all or some.
Blue seems to want bus numbers registers at least.

http://lists.gnu.org/archive/html/qemu-devel/2010-02/msg00519.html
-- 
yamahata

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

* [Qemu-devel] Re: [PATCH] pci: introduce get_info_quirk callback.
  2010-02-12 13:34   ` Isaku Yamahata
@ 2010-02-12 20:10     ` Blue Swirl
  2010-02-13  8:04       ` Blue Swirl
  0 siblings, 1 reply; 5+ messages in thread
From: Blue Swirl @ 2010-02-12 20:10 UTC (permalink / raw)
  To: Isaku Yamahata; +Cc: qemu-devel, Michael S. Tsirkin

On Fri, Feb 12, 2010 at 3:34 PM, Isaku Yamahata <yamahata@valinux.co.jp> wrote:
> On Fri, Feb 12, 2010 at 02:54:59PM +0200, Michael S. Tsirkin wrote:
>> On Fri, Feb 12, 2010 at 11:31:34AM +0900, Isaku Yamahata wrote:
>> > This patch fixes 525e05147d5a3bdc08caa422d108c1ef71b584b5
>> > by introducing device specific get_info_quirk callback.
>> > It wrongly assumes that pci host bridge class device has
>> > header type of pci-pci bridge. But this isn't always true.
>> > In fact i440fx pci host bridge has header type of normal device,
>> > hence it breaks i440fx and other pci host bridges.
>> > The right fix is that header type should be checked, instead of device class.
>> >
>> > The change set's purpose is to show PBM pci host bridge
>> > info which doesn't conform to PCI specification.
>>
>> So, PBM has header type PCI_HEADER_TYPE_NORMAL
>> but all config space is in bridge format?
>
> Some of registers is in bridge format.
> To be honest, I don't know whether all or some.
> Blue seems to want bus numbers registers at least.
>
> http://lists.gnu.org/archive/html/qemu-devel/2010-02/msg00519.html

I'm not so sure anymore, I'll have to check this in the weekend.
Perhaps there was some other problem with bus numbering or OpenBIOS
PCI programming which resulted in the need to program PRIMARY/
SECONDARY/SUBORDINATE bus values for host bridge. Does it make any
sense to use anything other than bus 0 for PCI bus attached to host
bridge?

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

* [Qemu-devel] Re: [PATCH] pci: introduce get_info_quirk callback.
  2010-02-12 20:10     ` Blue Swirl
@ 2010-02-13  8:04       ` Blue Swirl
  0 siblings, 0 replies; 5+ messages in thread
From: Blue Swirl @ 2010-02-13  8:04 UTC (permalink / raw)
  To: Isaku Yamahata; +Cc: qemu-devel, Michael S. Tsirkin

On Fri, Feb 12, 2010 at 10:10 PM, Blue Swirl <blauwirbel@gmail.com> wrote:
> On Fri, Feb 12, 2010 at 3:34 PM, Isaku Yamahata <yamahata@valinux.co.jp> wrote:
>> On Fri, Feb 12, 2010 at 02:54:59PM +0200, Michael S. Tsirkin wrote:
>>> On Fri, Feb 12, 2010 at 11:31:34AM +0900, Isaku Yamahata wrote:
>>> > This patch fixes 525e05147d5a3bdc08caa422d108c1ef71b584b5
>>> > by introducing device specific get_info_quirk callback.
>>> > It wrongly assumes that pci host bridge class device has
>>> > header type of pci-pci bridge. But this isn't always true.
>>> > In fact i440fx pci host bridge has header type of normal device,
>>> > hence it breaks i440fx and other pci host bridges.
>>> > The right fix is that header type should be checked, instead of device class.
>>> >
>>> > The change set's purpose is to show PBM pci host bridge
>>> > info which doesn't conform to PCI specification.
>>>
>>> So, PBM has header type PCI_HEADER_TYPE_NORMAL
>>> but all config space is in bridge format?
>>
>> Some of registers is in bridge format.
>> To be honest, I don't know whether all or some.
>> Blue seems to want bus numbers registers at least.
>>
>> http://lists.gnu.org/archive/html/qemu-devel/2010-02/msg00519.html
>
> I'm not so sure anymore, I'll have to check this in the weekend.
> Perhaps there was some other problem with bus numbering or OpenBIOS
> PCI programming which resulted in the need to program PRIMARY/
> SECONDARY/SUBORDINATE bus values for host bridge. Does it make any
> sense to use anything other than bus 0 for PCI bus attached to host
> bridge?

I found the bug in my new PCI probing code,
525e05147d5a3bdc08caa422d108c1ef71b584b5 was not correct after all.
There is no need to configure host brige bus values.

So I'll apply Isaku's earlier fix that removes the bogus host bridge
data from 'info pci'.

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

end of thread, other threads:[~2010-02-13  8:04 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-02-12  2:31 [Qemu-devel] [PATCH] pci: introduce get_info_quirk callback Isaku Yamahata
2010-02-12 12:54 ` [Qemu-devel] " Michael S. Tsirkin
2010-02-12 13:34   ` Isaku Yamahata
2010-02-12 20:10     ` Blue Swirl
2010-02-13  8:04       ` Blue Swirl

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).