qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Peter Maydell <peter.maydell@linaro.org>
To: Peter Xu <peterx@redhat.com>
Cc: eric.auger@redhat.com, eric.auger.pro@gmail.com,
	qemu-devel@nongnu.org,  qemu-arm@nongnu.org, mst@redhat.com,
	jasowang@redhat.com, imammedo@redhat.com,
	 alex.williamson@redhat.com, clg@redhat.com, philmd@linaro.org,
	 zhenzhong.duan@intel.com, ddutile@redhat.com
Subject: Re: [PATCH 4/5] hw/arm/smmuv3: Move reset to exit phase
Date: Mon, 10 Feb 2025 14:22:56 +0000	[thread overview]
Message-ID: <CAFEAcA9640s-Eu7PHxJ-Xb5X38n-25pnUG+GxymsAWkJf8YfcA@mail.gmail.com> (raw)
In-Reply-To: <Z6oJzRCt_fJLfkmQ@x1.local>

On Mon, 10 Feb 2025 at 14:14, Peter Xu <peterx@redhat.com> wrote:
>
> On Fri, Feb 07, 2025 at 06:18:50PM +0000, Peter Maydell wrote:
> > On Fri, 7 Feb 2025 at 17:48, Peter Xu <peterx@redhat.com> wrote:
> > >
> > > On Fri, Feb 07, 2025 at 04:58:39PM +0000, Peter Maydell wrote:
> > > > (I wonder if we ought to suggest quiescing outstanding
> > > > DMA in the enter phase? But it's probably easier to fix
> > > > the iommus like this series does than try to get every
> > > > dma-capable pci device to do something different.)
> > >
> > > I wonder if we should provide some generic helper to register vIOMMU reset
> > > callbacks, so that we'll be sure any vIOMMU model impl that will register
> > > at exit() phase only, and do nothing during the initial two phases.  Then
> > > we can put some rich comment on that helper on why.
> > >
> > > Looks like it means the qemu reset model in the future can be a combination
> > > of device tree (which resets depth-first) and the three phases model.  We
> > > will start to use different approach to solve different problems.
> >
> > The tree of QOM devices (i.e. the one based on the qbus buses
> > and rooted at the sysbus) resets depth-first, but it does so in
> > three phases: first we traverse everything doing 'enter'; then
> > we traverse everything doing 'hold'; then we traverse everything
> > doing 'exit'. There *used* to be an awkward mix of some things
> > being three-phase and some not, but we have now got rid of all
> > of those so a system reset does a single three-phase reset run
> > which resets everything.
>
> Right.  Sorry I wasn't very clear before indeed on what I wanted to
> express.
>
> My understanding is the 3 phases reset, even if existed, was not designed
> to order things like vIOMMU and devices that is already described by system
> topology.  That's, IMHO, exactly what QOM topology wanted to achieve right
> now on ordering device resets and the whole depth-first reset method would
> make sense with it.
>
> So from that specific POV, it's a mixture use of both methods on ordering
> of devices to reset now (rather than the order of reset process within a
> same device, provided into 3 phases).  It may not be very intuitive when
> someone reads about the two reset mechanisms, as one would naturally take
> vIOMMU as a root object of any other PCIe devices under root complex, and
> thinking the order should be guaranteed by QOM on reset already.  In
> reality it's not.  So that's the part I wonder if we want to document.

Yeah, I see what you mean. The issue here is that the iommu isn't
actually a parent of the devices that access through it. This is
true both in the "qbus/qdev bus tree" sense (where it is the PCI
controller device that owns the PCI bus that the devices are on)
and also in the QOM tree sense[*] (where usually the iommu and the
PCI controller are both created by the machine or the SoC, I guess?).
Instead iommus are separate devices that control data flow but
aren't in a parent-child relationship with the devices on either
side of that flow. There is a guarantee about reset ordering between
bus parent/child (so the PCI controller resets before it resets
the PCI bus that resets the PCI devices on the bus), but that doesn't
help the iommu.

[*] I have a vague idea that ideally we might want reset to be
QOM-tree based rather than qbus-tree based. But getting there from
here is non-trivial. And maybe what we really want is "objects,
especially SoCs, that create children can define what their reset
tree is, with the default being to reset all the QOM children".
Lots of non-thought-through complexity here ;-)

thanks
-- PMM


  reply	other threads:[~2025-02-10 14:23 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-02-06 14:21 [PATCH 0/5] Fix vIOMMU reset order Eric Auger
2025-02-06 14:21 ` [PATCH 1/5] hw/virtio/virtio-iommu: Migrate to 3-phase reset Eric Auger
2025-02-06 14:21 ` [PATCH 2/5] hw/i386/intel-iommu: " Eric Auger
2025-02-06 14:21 ` [PATCH 3/5] hw/i386/intel_iommu: Tear down address spaces before IOMMU reset Eric Auger
2025-02-17  3:02   ` Duan, Zhenzhong
2025-02-17  7:31     ` Eric Auger
2025-02-06 14:21 ` [PATCH 4/5] hw/arm/smmuv3: Move reset to exit phase Eric Auger
2025-02-07 16:37   ` Peter Maydell
2025-02-07 16:50     ` Eric Auger
2025-02-07 16:58       ` Peter Maydell
2025-02-07 17:47         ` Peter Xu
2025-02-07 18:18           ` Peter Maydell
2025-02-10  8:47             ` Eric Auger
2025-02-10 14:14             ` Peter Xu
2025-02-10 14:22               ` Peter Maydell [this message]
2025-02-12 17:28                 ` Cédric Le Goater
2025-02-10  8:35           ` Eric Auger
2025-02-10 14:18             ` Peter Xu
2025-02-10  8:40         ` Eric Auger
2025-02-06 14:21 ` [PATCH 5/5] hw/vfio/common: Add a trace point in vfio_reset_handler Eric Auger
2025-02-07 17:18   ` Cédric Le Goater
2025-02-07 11:09 ` [PATCH 0/5] Fix vIOMMU reset order Michael S. Tsirkin
2025-02-07 16:40   ` Peter Maydell
2025-02-07 16:52     ` Eric Auger
2025-02-07 16:54 ` Peter Xu
2025-02-07 17:06   ` Peter Maydell
2025-02-07 17:31     ` Peter Xu
2025-02-10  8:45       ` Eric Auger
2025-02-07 17:25   ` Cédric Le Goater
2025-02-10  8:49     ` Eric Auger

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAFEAcA9640s-Eu7PHxJ-Xb5X38n-25pnUG+GxymsAWkJf8YfcA@mail.gmail.com \
    --to=peter.maydell@linaro.org \
    --cc=alex.williamson@redhat.com \
    --cc=clg@redhat.com \
    --cc=ddutile@redhat.com \
    --cc=eric.auger.pro@gmail.com \
    --cc=eric.auger@redhat.com \
    --cc=imammedo@redhat.com \
    --cc=jasowang@redhat.com \
    --cc=mst@redhat.com \
    --cc=peterx@redhat.com \
    --cc=philmd@linaro.org \
    --cc=qemu-arm@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=zhenzhong.duan@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).