From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45422) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aLTOZ-0002fb-VK for qemu-devel@nongnu.org; Tue, 19 Jan 2016 05:20:17 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aLTOV-0002Yi-AR for qemu-devel@nongnu.org; Tue, 19 Jan 2016 05:20:15 -0500 Received: from [59.151.112.132] (port=28791 helo=heian.cn.fujitsu.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aLTOU-0002Xc-Kw for qemu-devel@nongnu.org; Tue, 19 Jan 2016 05:20:11 -0500 Message-ID: <569E0CC7.8030304@cn.fujitsu.com> Date: Tue, 19 Jan 2016 18:15:35 +0800 From: Chen Fan MIME-Version: 1.0 References: <1452803789.14628.88.camel@redhat.com> In-Reply-To: <1452803789.14628.88.camel@redhat.com> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v16 10/14] pci: introduce pci bus pre reset List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alex Williamson , Cao jin , qemu-devel@nongnu.org Cc: izumi.taku@jp.fujitsu.com, mst@redhat.com On 01/15/2016 04:36 AM, Alex Williamson wrote: > On Tue, 2016-01-12 at 10:43 +0800, Cao jin wrote: >> From: Chen Fan >> >> avoid repeat bus reset, here introduce a sequence ID for each time >> bus hot reset, so each vfio device could know whether they've already >> been reset for that sequence ID. >> >> Signed-off-by: Chen Fan >> --- >> hw/pci/pci.c | 13 +++++++++++++ >> hw/pci/pci_bridge.c | 3 +++ >> include/hw/pci/pci.h | 1 + >> include/hw/pci/pci_bus.h | 3 +++ >> 4 files changed, 20 insertions(+) >> >> diff --git a/hw/pci/pci.c b/hw/pci/pci.c >> index f6ca6ef..ceb72d5 100644 >> --- a/hw/pci/pci.c >> +++ b/hw/pci/pci.c >> @@ -91,6 +91,18 @@ static void pci_bus_unrealize(BusState *qbus, >> Error **errp) >> vmstate_unregister(NULL, &vmstate_pcibus, bus); >> } >> >> +void pci_bus_pre_reset(PCIBus *bus, uint32_t seqid) >> +{ >> + PCIBus *sec; >> + >> + bus->in_reset = true; >> + bus->reset_seqid = seqid; >> + >> + QLIST_FOREACH(sec, &bus->child, sibling) { >> + pci_bus_pre_reset(sec, seqid); >> + } >> +} >> + >> static bool pcibus_is_root(PCIBus *bus) >> { >> return !bus->parent_dev; >> @@ -276,6 +288,7 @@ static void pcibus_reset(BusState *qbus) >> for (i = 0; i < bus->nirq; i++) { >> assert(bus->irq_count[i] == 0); >> } >> + bus->in_reset = false; >> } >> >> static void pci_host_bus_register(PCIBus *bus, DeviceState *parent) >> diff --git a/hw/pci/pci_bridge.c b/hw/pci/pci_bridge.c >> index 40c97b1..c7f15a1 100644 >> --- a/hw/pci/pci_bridge.c >> +++ b/hw/pci/pci_bridge.c >> @@ -268,6 +268,9 @@ void pci_bridge_write_config(PCIDevice *d, >> newctl = pci_get_word(d->config + PCI_BRIDGE_CONTROL); >> if (~oldctl & newctl & PCI_BRIDGE_CTL_BUS_RESET) { >> /* Trigger hot reset on 0->1 transition. */ >> + uint32_t seqid = s->sec_bus.reset_seqid++; > Doesn't this need to come from a global sequence ID? Imagine the case > of a nested bus, the leaf bus is reset incrementing the sequence ID. > The devices on that bus store that sequence ID as they're reset. The > parent bus is then reset, but all the devices on the leaf bus have > already been reset for that sequence ID and ignore the reset. > >> + >> + pci_bus_pre_reset(&s->sec_bus, seqid ? seqid : 1); > Does this work? Seems like this would make devices ignore the second > bus reset after the VM is instantiated. ie. the first bus reset seqid > is 0, so we call pre_reset with 1, the second time we call it with 1 > again. > >> qbus_reset_all(&s->sec_bus.qbus); > I'd be tempted to call qbus_walk_children() directly, it already has a > pre_busfn callback hook. Hi Alex, this looks like need to change much pci core code, as Michael suggested in 00/14, maybe we should simply the aer implementation. what do you think of that? Thanks, Chen > >> } >> } >> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h >> index 379b6e1..b811279 100644 >> --- a/include/hw/pci/pci.h >> +++ b/include/hw/pci/pci.h >> @@ -381,6 +381,7 @@ void pci_bus_fire_intx_routing_notifier(PCIBus >> *bus); >> void pci_device_set_intx_routing_notifier(PCIDevice *dev, >> PCIINTxRoutingNotifier >> notifier); >> void pci_device_reset(PCIDevice *dev); >> +void pci_bus_pre_reset(PCIBus *bus, uint32_t seqid); >> >> PCIDevice *pci_nic_init_nofail(NICInfo *nd, PCIBus *rootbus, >> const char *default_model, >> diff --git a/include/hw/pci/pci_bus.h b/include/hw/pci/pci_bus.h >> index 7812fa9..dd6aaf1 100644 >> --- a/include/hw/pci/pci_bus.h >> +++ b/include/hw/pci/pci_bus.h >> @@ -40,6 +40,9 @@ struct PCIBus { >> int nirq; >> int *irq_count; >> >> + bool in_reset; >> + uint32_t reset_seqid; >> + >> NotifierWithReturnList hotplug_notifiers; >> }; >> > > > . >