RE: [PATCH v3 2/5] Drivers: hv: Add logical device ID registry for vPCI devices

From: Yu Zhang <[email protected]> Sent: Monday, August 17, 2026 3:38 AM
> 
> On Fri, Aug 14, 2026 at 03:31:27PM +0000, Michael Kelley wrote:
> > From: Yu Zhang <[email protected]> Sent: Tuesday, August 11, 2026 8:50 AM
> > >

[snip]

> > > +#ifdef CONFIG_HYPERV_PVIOMMU
> >
> > Making this code in hv_common.c conditional on HYPERV_PVIOMMU
> > assumes that the only consumer is the pvIOMMU driver. But my
> > understanding is that root partition code is also expected to be a
> > consumer, and I would guess that code is independent of a guest
> > pvIOMMU. So the #ifdef might need to become more complicated
> > to also allow the root partition case. I'm thinking it makes sense to
> > always build this code as part of hv_common.c. Even when building
> > for a guest only with no root partition code, HYPERV_PVIOMMU
> > defaults to HYPERV so the pvIOMMU driver will be built anytime
> > Hyper-V code is included, unless someone goes out of their way to
> > disable it. Getting rid of all the #ifdef'ery would make the code a bit
> > cleaner, and I'm looking for an excuse to do so!
> >
> > I'd also note that CONFIG_HYPERV_PVIOMMU isn't defined
> > until Patch 4 of this series. This patch will still build without the
> > remaining patches, so bisect isn't broken, but it would be with
> > the equivalent of CONFIG_HYPERV_PVIOMMU=n. It's a little bit
> > weird to be using a CONFIG_* value before it is created, but maybe
> > the ordering among the patches is complex and not easily sorted.
> >
> 
> Thank you, Michael!
> 
> That makes sense. The trade-off is that a Hyper-V guest without
> pvIOMMU support will still allocate and populate the registry for
> its PCI buses. If that small overhead is acceptable, building it
> unconditionally does make the design simpler. And we do not need
> to worry about the chicken-and-egg problem when ordering this patch
> and patch 4. :)

To me, the small overhead is acceptable. vPCI add/remove
operations are relatively infrequent, and are not a "fast" path. So
the overhead is of no real consequence.

[snip]

> >
> > The comment above is written as if the pvIOMMU driver is the only
> > consumer, which isn't accurate assuming the root partition use case
> > materializes as expected.  And that also means having "iommu" in the
> > function name is overly specific. Same with the other function names
> > with "iommu".  There's really nothing here that is specific to pvIOMMUs.
> >
> 
> Agreed. Will update the comment.
> 
> For the function names, I would like to avoid using sth. like
> hv_register_pci_bus() since that sounds like to register the
> PCI bus itself. Two options I'm having in mind are:
> 
> 	hv_pci_busdata_register()
> 	hv_pci_busdata_unregister()
> 	hv_pci_busdata_lookup()
> 
> or some more descriptive yet legthy ones, e.g.,
> 
> 	hv_pci_bus_register_logical_dev_id_prefix()
> 	hv_pci_bus_unregister_logical_dev_id_prefix()
> 	hv_pci_bus_lookup_logical_dev_id_prefix()
> 
> Do you have a preference, or any suggestion?

Or maybe just hv_pci_register_dev_id()? To me, it's
better to put the verb ("register") first followed by the
object. "logical_dev_id_prefix" as the object is a bit
too wordy for my preference.  Just "dev_id" or "busdata"
is better, even if neither is fully precise.

[snip]

> 
> > Separately, it seems like the existing calls from the Hyper-V vPCI driver should
> > never duplicate an existing mapping. Your code disallows mapping a domain_nr
> > to a different logical_dev_id_prefix, but silently allows entering a mapping
> > that already exists as an exact match. I wonder if that really should be
> > allowed, as it seems likely such would be followed later by twice removing
> > the duplicate entry, with the second removal failing. Removing a non-existent
> > entry is also silently allowed in the unregister function below, with the same
> > question as to whether that should be allowed.
> >
> 
> Actually, I don't think an exact duplicate registration is a valid case
> either. So how about we use WARN_ONCE() and return -EEXIST when an entry
> for the PCI domain is already registered(regardless it is a exact match
> or not)?
> 
> And for unregister, how about we initialize the result to -ENOENT, do the
> removal if lookup finds the entry, and WARN_ONCE if no entry is really
> removed?

Works for me.

> 
> > > +}
> > > +EXPORT_SYMBOL_FOR_MODULES(hv_iommu_register_pci_bus, "pci-hyperv");
> >
> > This may be overly restrictive if the root partition use case code is in the
> > mshv_root module.
> >
> 
> Well, I'm not sure. IMHO, the register/unregister side is owned by
> pci-hyperv, which creates and removes the Hyper-V PCI bus. The guest
> pvIOMMU or the root driver are only cumsumers of the  lookup.
> 
> So maybe the register/unregister exports should remain restricted to
> pci-hyperv, and if a future module needs the lookup helper, an export
> can be added for the lookup helper by then (we do not need to export
> lookup right now for pvIOMMU, which is built-in)?
> 
> Am I missing anything?

You are correct and didn't miss anything. I didn't fully think it through,
so my comment is bogus. What you propose is fine.

Michael