From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57402) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aJodJ-0007A2-JT for qemu-devel@nongnu.org; Thu, 14 Jan 2016 15:36:38 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aJodE-0005RG-WC for qemu-devel@nongnu.org; Thu, 14 Jan 2016 15:36:37 -0500 Received: from mx1.redhat.com ([209.132.183.28]:49218) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aJodE-0005R0-Oc for qemu-devel@nongnu.org; Thu, 14 Jan 2016 15:36:32 -0500 Message-ID: <1452803789.14628.88.camel@redhat.com> From: Alex Williamson Date: Thu, 14 Jan 2016 13:36:29 -0700 In-Reply-To: References: Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable 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: Cao jin , qemu-devel@nongnu.org Cc: chen.fan.fnst@cn.fujitsu.com, izumi.taku@jp.fujitsu.com, mst@redhat.com On Tue, 2016-01-12 at 10:43 +0800, Cao jin wrote: > From: Chen Fan >=20 > 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. >=20 > Signed-off-by: Chen Fan > --- > =C2=A0hw/pci/pci.c=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0| 13 +++++++++++++ > =C2=A0hw/pci/pci_bridge.c=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0|=C2=A0=C2= =A03 +++ > =C2=A0include/hw/pci/pci.h=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0|=C2=A0=C2=A01 = + > =C2=A0include/hw/pci/pci_bus.h |=C2=A0=C2=A03 +++ > =C2=A04 files changed, 20 insertions(+) >=20 > 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) > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0vmstate_unregister(NULL, &vmstate_pcibus,= bus); > =C2=A0} > =C2=A0 > +void pci_bus_pre_reset(PCIBus *bus, uint32_t seqid) > +{ > +=C2=A0=C2=A0=C2=A0=C2=A0PCIBus *sec; > + > +=C2=A0=C2=A0=C2=A0=C2=A0bus->in_reset =3D true; > +=C2=A0=C2=A0=C2=A0=C2=A0bus->reset_seqid =3D seqid; > + > +=C2=A0=C2=A0=C2=A0=C2=A0QLIST_FOREACH(sec, &bus->child, sibling) { > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0pci_bus_pre_reset(sec,= seqid); > +=C2=A0=C2=A0=C2=A0=C2=A0} > +} > + > =C2=A0static bool pcibus_is_root(PCIBus *bus) > =C2=A0{ > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0return !bus->parent_dev; > @@ -276,6 +288,7 @@ static void pcibus_reset(BusState *qbus) > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0for (i =3D 0; i < bus->nirq; i++) { > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0assert(bus->irq_c= ount[i] =3D=3D 0); > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0} > +=C2=A0=C2=A0=C2=A0=C2=A0bus->in_reset =3D false; > =C2=A0} > =C2=A0 > =C2=A0static void pci_host_bus_register(PCIBus *bus, DeviceState *paren= t) > 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, > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0newctl =3D pci_get_word(d->config + PCI_B= RIDGE_CONTROL); > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0if (~oldctl & newctl & PCI_BRIDGE_CTL_BUS= _RESET) { > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0/* Trigger hot re= set on 0->1 transition. */ > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0uint32_t seqid =3D s->= sec_bus.reset_seqid++; Doesn't this need to come from a global sequence ID? =C2=A0Imagine the ca= se 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. =C2=A0Th= e 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. > + > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0pci_bus_pre_reset(&s->= sec_bus, seqid ? seqid : 1); Does this work? =C2=A0Seems like this would make devices ignore the secon= d bus reset after the VM is instantiated. =C2=A0ie. =C2=A0the first bus res= et seqid is 0, so we call pre_reset with 1, the second time we call it with 1 again. > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0qbus_reset_all(&s= ->sec_bus.qbus); I'd be tempted to call qbus_walk_children() directly, it already has a pre_busfn callback hook. > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0} > =C2=A0} > 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); > =C2=A0void pci_device_set_intx_routing_notifier(PCIDevice *dev, > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0PCIINTxRoutingNotifier > notifier); > =C2=A0void pci_device_reset(PCIDevice *dev); > +void pci_bus_pre_reset(PCIBus *bus, uint32_t seqid); > =C2=A0 > =C2=A0PCIDevice *pci_nic_init_nofail(NICInfo *nd, PCIBus *rootbus, > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0const 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 { > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0int nirq; > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0int *irq_count; > =C2=A0 > +=C2=A0=C2=A0=C2=A0=C2=A0bool in_reset; > +=C2=A0=C2=A0=C2=A0=C2=A0uint32_t reset_seqid; > + > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0NotifierWithReturnList hotplug_notifiers; > =C2=A0}; > =C2=A0