Re: [PATCH 7/8] pci: Add pci_for_each_device_all()

[Peter Xu]

On Fri, Oct 22, 2021 at 04:43:43AM -0400, Michael S. Tsirkin wrote:
> On Fri, Oct 22, 2021 at 10:33:15AM +0800, Peter Xu wrote:
> > Hi, Michael,
> > 
> > On Thu, Oct 21, 2021 at 06:54:59AM -0400, Michael S. Tsirkin wrote:
> > > > +typedef struct {
> > > > +    pci_bus_dev_fn fn;
> > > > +    void *opaque;
> > > > +} pci_bus_dev_args;
> > > 
> > > code style violation. CamelCase for structs pls.
> > 
> > OK.
> > 
> > > > +/* Call 'fn' for each pci device on the system */
> > > > +void pci_for_each_device_all(pci_bus_dev_fn fn, void *opaque);
> > > 
> > > Instead of hacking pci making initialization o(N^2),
> > 
> > Why it's O(N^2)?  One vIOMMU walks O(N), and we only have one vIOMMU, or am 
> > I
> > wrong?
> 
> What I meant is this is O(N) and if called M times will be O(N * M)
> yes your patches only call once so O(N), still we can do better.

I see.

> 
> > > can't we add a variant of object_resolve_path_type ?
> > 
> > Could you elaborate?  Here what we want to do is to make sure there're no
> > specific PCI devices registered, and potentially it can be more than one 
> > type
> > of device in the future.
> > 
> > Thanks,
> 
> All you seem to care about is checking there's no VFIO
> (why - should really be documented in a code comment much more clearly).

Right, Alex asked the same question.  I'll make sure to mention that in the
commit message in the next version.

> Looks like object_resolve_path_type does that with O(1) complexity.
> If we need a variant that checks for multiple types we can add that.

It's still O(N), or am I wrong?  I mean for example there's the loop in
object_resolve_partial_path().

But yeah I can use that too if you prefer, it's just that when we want to
detect more types of pci classes it could be slower iiuc, because we'll need to
call object_resolve_path_type() once for each type.  For pci bus scan it's
always one round because we only have at most one x86 vIOMMU for each guest.

At the meantime, IMHO patch 1-6 are cleanups that should be good even without
patch 7/8. If we prefer object_resolve_path_type() I'd still think it would be
good to propose patch 1-6 separately (with some patch properly squashed as
suggested by reviewers)?

Thanks,

-- 
Peter Xu