All of lore.kernel.org
 help / color / mirror / Atom feed
From: Igor Mammedov <imammedo@redhat.com>
To: Peter Xu <peterx@redhat.com>
Cc: "Daniel P . Berrangé" <berrange@redhat.com>,
	"Eduardo Habkost" <ehabkost@redhat.com>,
	"Michael S . Tsirkin" <mst@redhat.com>,
	"Jason Wang" <jasowang@redhat.com>,
	qemu-devel@nongnu.org, "Markus Armbruster" <armbru@redhat.com>,
	"Eric Auger" <eric.auger@redhat.com>,
	"Alex Williamson" <alex.williamson@redhat.com>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Dr . David Alan Gilbert" <dgilbert@redhat.com>
Subject: Re: [PATCH 4/4] vl: Prioritize realizations of devices
Date: Thu, 2 Sep 2021 10:26:16 +0200	[thread overview]
Message-ID: <20210902102616.1b596104@redhat.com> (raw)
In-Reply-To: <YS0rXQXwqKjhr4TA@t490s>

On Mon, 30 Aug 2021 15:02:53 -0400
Peter Xu <peterx@redhat.com> wrote:

> On Thu, Aug 26, 2021 at 09:43:59AM -0400, Peter Xu wrote:
> > > > A simple state machine can track "has IOMMU" state.  It has three states
> > > > "no so far", "yes", and "no", and two events "add IOMMU" and "add device
> > > > that needs to know".  State diagram:
> > > > 
> > > >                           no so far
> > > >                    +--- (start state) ---+
> > > >                    |                     |
> > > >          add IOMMU |                     | add device that
> > > >                    |                     |  needs to know
> > > >                    v                     v
> > > >              +--> yes                    no <--+
> > > >              |     |   add device that   |     |
> > > >              +-----+    needs to know    +-----+
> > > > 
> > > > "Add IOMMU" in state "no" is an error.  
> > > 
> > > question is how we distinguish "device that needs to know"
> > > from device that doesn't need to know, and then recently
> > > added feature 'bypass IOMMU' adds more fun to this.  
> > 
> > Maybe we can start from "no device needs to know"? Then add more into it when
> > the list expands.
> > 
> > vfio-pci should be a natural fit because we're sure it won't break any valid
> > old configurations.  However we may need to be careful on adding more devices,
> > e.g. when some old configuration might work on old binaries, but I'm not sure.  
> 
> Btw, I think this state machine is indeed bringing some complexity on even
> understanding it - it is definitely working but it's not obvious to anyone at
> the first glance, and it's only solving problem for vIOMMU.  E.g., do we need
> yet another state machine for some other ordering constraints?
>
> From that POV, I don't like this solution more than the simple "assign priority
> for device realization" idea..
It seems simple but implicit dependencies are fragile (good or
I'd rather say bad example implicit dependencies is vl.c magic code,
where changing order of initialization can easily break QEMU,
which happens almost every time it's refactored),
and as Markus already mentioned it won't work in QMP case.

What could work for both cases is explicit dependencies,
however it would be hard to describe such dependency in this case,
where device can work both with and without IOMMU depending
on the bus settings it's attached to.

Have you considered another approach, i.e. instead of reordering,
change vfio-pci device model to reconfigure DMA address space
after realize time (ex: at reset time)?




  parent reply	other threads:[~2021-09-02  8:27 UTC|newest]

Thread overview: 52+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-18 19:42 [PATCH 0/4] vl: Prioritize device realizations Peter Xu
2021-08-18 19:42 ` [PATCH 1/4] qdev-monitor: Trace qdev creation Peter Xu
2021-08-18 19:43 ` [PATCH 2/4] qemu-config: Allow in-place sorting of QemuOptsList Peter Xu
2021-08-18 19:43 ` [PATCH 3/4] qdev: Export qdev_get_device_class() Peter Xu
2021-08-18 19:43 ` [PATCH 4/4] vl: Prioritize realizations of devices Peter Xu
2021-08-23 18:49   ` Eduardo Habkost
2021-08-23 19:18     ` Peter Xu
2021-08-23 21:07       ` Eduardo Habkost
2021-08-23 21:31         ` Peter Xu
2021-08-23 21:54           ` Michael S. Tsirkin
2021-08-23 22:51             ` Peter Xu
2021-08-23 21:56           ` Eduardo Habkost
2021-08-23 23:05             ` Peter Xu
2021-08-25  9:39               ` Markus Armbruster
2021-08-25 12:28                 ` Markus Armbruster
2021-08-25 21:50                   ` Peter Xu
2021-08-26  3:50                     ` Peter Xu
2021-08-26  8:01                       ` Markus Armbruster
2021-08-26 11:36                         ` Igor Mammedov
2021-08-26 13:43                           ` Peter Xu
2021-08-30 19:02                             ` Peter Xu
2021-08-31 11:35                               ` Markus Armbruster
2021-09-02  8:26                               ` Igor Mammedov [this message]
2021-09-02 13:45                                 ` Peter Xu
2021-09-02 13:53                                   ` Daniel P. Berrangé
2021-09-02 14:21                                     ` Peter Xu
2021-09-02 14:57                                       ` Markus Armbruster
2021-09-03 15:48                                         ` Peter Xu
2021-09-02 15:06                                       ` Daniel P. Berrangé
2021-09-02 15:26                                   ` Markus Armbruster
2021-09-03 13:00                                   ` Igor Mammedov
2021-09-03 16:03                                     ` Peter Xu
2021-09-06  8:49                                       ` Igor Mammedov
2021-09-02  7:46                             ` Igor Mammedov
2021-08-26  4:57                     ` Markus Armbruster
2021-08-23 22:05       ` Michael S. Tsirkin
2021-08-23 22:36         ` Peter Xu
2021-08-24  2:52           ` Jason Wang
2021-08-24 15:50             ` Peter Xu
2021-08-25  4:23               ` Jason Wang
2021-09-06  9:22                 ` Eric Auger
2021-08-24 16:24         ` David Hildenbrand
2021-08-24 19:52           ` Peter Xu
2021-08-25  8:08             ` David Hildenbrand
2021-08-24  2:51       ` Jason Wang
2021-10-20 13:44 ` [PATCH 0/4] vl: Prioritize device realizations David Hildenbrand
2021-10-20 13:48   ` Daniel P. Berrangé
2021-10-20 13:58     ` David Hildenbrand
2021-10-21  4:20   ` Peter Xu
2021-10-21  7:17     ` David Hildenbrand
2021-10-21  8:00       ` Peter Xu
2021-10-21 16:54         ` David Hildenbrand

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=20210902102616.1b596104@redhat.com \
    --to=imammedo@redhat.com \
    --cc=alex.williamson@redhat.com \
    --cc=armbru@redhat.com \
    --cc=berrange@redhat.com \
    --cc=dgilbert@redhat.com \
    --cc=ehabkost@redhat.com \
    --cc=eric.auger@redhat.com \
    --cc=jasowang@redhat.com \
    --cc=mst@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=peterx@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.