[Qemu-devel] Re: [PATCH v2 0/6] qdev reset refactoring and pci bus reset

[Isaku Yamahata]

On Tue, Nov 23, 2010 at 08:10:26PM +0200, Michael S. Tsirkin wrote:
> On Tue, Nov 23, 2010 at 12:53:12AM +0200, Michael S. Tsirkin wrote:
> > On Mon, Nov 22, 2010 at 07:43:37PM +0900, Isaku Yamahata wrote:
> > > On Mon, Nov 22, 2010 at 09:54:02AM +0200, Michael S. Tsirkin wrote:
> > > > On Fri, Nov 19, 2010 at 06:55:57PM +0900, Isaku Yamahata wrote:
> > > > > Here is v2. I updated the comments, and dropped the pci qdev reset 
> > > > > patch.
> > > > > 
> > > > > Patch description:
> > > > > The goal of this patch series is to implement secondary bus reset
> > > > > emulation in pci-to-pci bridge.
> > > > > At first, this patch series refactors qdev reset,
> > > > > and then cleans up pci bus reset. Lastly implements pci bridge control
> > > > > secondary bus reset bit.
> > > > > 
> > > > > This patch series is for pci bus reset, which is ported
> > > > > from the following repo.
> > > > > git://repo.or.cz/qemu/aliguori.git qdev-refactor
> > > > 
> > > > I've put the series on my pci branch, tweaking patches 5 and 6 in the
> > > > process.  Out of time to compile-tested only for now.
> > > 
> > > Thank you. The tweaking looks good.
> > > Do you still want me to send another patch to add a comment on RST#?
> > 
> > OK, I got response, and yes, we need to fix code to avoid claiming
> > transactions by devices on the secondary bus while secondary bus is in
> > RST# for both pci and express.
> 
> I guess the following fixes it?

Almost. The following if claouse also needs modification.

commit 120fe782a198b787e13a9d4681afae8dc66dee50
Author: "Michael S. Tsirkin" <address@hidden>
Date:   Wed Nov 24 11:34:59 2010 +0900

    pci: don't access bus while it's reset
    
    Devices on a bus should not respond while RST#
    is asserted, which is controlled by PCI_BRIDGE_CTL_BUS_RESET.
    Simply skip such a bus when propagating the configuration cycle.
    
    Signed-off-by: Michael S. Tsirkin <address@hidden>
    Signed-off-by: Isaku Yamahata <address@hidden>

diff --git a/hw/pci.c b/hw/pci.c
index d02f980..e7df612 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -1554,7 +1554,9 @@ PCIBus *pci_find_bus(PCIBus *bus, int bus_num)
 
     /* try child bus */
     if (!bus->parent_dev /* host pci bridge */ ||
-        (bus->parent_dev->config[PCI_SECONDARY_BUS] < bus_num &&
+        (!(pci_get_word(bus->parent_dev->config + PCI_BRIDGE_CONTROL) &
+           PCI_BRIDGE_CTL_BUS_RESET) /* Don't walk the bus if it's reset. */ &&
+         bus->parent_dev->config[PCI_SECONDARY_BUS] < bus_num &&
          bus_num <= bus->parent_dev->config[PCI_SUBORDINATE_BUS])) {
         for (; bus; bus = sec) {
             QLIST_FOREACH(sec, &bus->child, sibling) {
@@ -1562,8 +1564,11 @@ PCIBus *pci_find_bus(PCIBus *bus, int bus_num)
                 if (sec->parent_dev->config[PCI_SECONDARY_BUS] == bus_num) {
                     return sec;
                 }
-                if (sec->parent_dev->config[PCI_SECONDARY_BUS] < bus_num &&
-                    bus_num <= sec->parent_dev->config[PCI_SUBORDINATE_BUS]) {
+                if (pci_get_word(sec->parent_dev->config +
+                                 PCI_BRIDGE_CONTROL) & PCI_BRIDGE_CTL_BUS_RESET
+                    /* Don't walk the bus if it's reset. */ ||
+                    bus_num < sec->parent_dev->config[PCI_SECONDARY_BUS] ||
+                    sec->parent_dev->config[PCI_SUBORDINATE_BUS] < bus_num) {
                     break;
                 }
             }

-- 
yamahata