On Wed, Jul 08, 2026 at 01:28:42PM +0000, Michael Kelley wrote: > From: Yu Zhang <[email protected]> Sent: Wednesday, July 8, 2026 > 6:09 AM > > > > On Wed, Jul 08, 2026 at 02:52:43AM +0000, Michael Kelley wrote: > > > From: Yu Zhang <[email protected]> Sent: Thursday, July 2, > > > 2026 9:05 AM > > > > > > > > From: Easwar Hariharan <[email protected]> > > > > > > > > Hyper-V identifies each PCI pass-thru device by a logical device ID in > > > > its hypercall interface. This ID consists of a per-bus prefix, derived > > > > from the VMBus device instance GUID, combined with the PCI function > > > > number of the endpoint device. > > > > > > > > Add a small registry in hv_common.c that maps a PCI domain number to its > > > > logical device ID prefix. The vPCI bus driver (pci-hyperv) registers the > > > > prefix when a bus is probed and unregisters it when the bus is removed. > > > > Consumers such as the para-virtualized IOMMU driver look up the prefix > > > > by PCI domain number and combine it with the function number to form the > > > > complete logical device ID for hypercalls. > > > > > > > > The prefix construction is shared via hv_build_logical_dev_id_prefix() > > > > so > > > > that pci-hyperv's interrupt retargeting path and the registry use > > > > exactly > > > > the same byte layout. It is derived on demand from the constant > > > > hv_device > > > > instance GUID rather than cached in struct hv_pcibus_device, which is > > > > private to the pci-hyperv module; this keeps the interface narrow and > > > > avoids depending on pci-hyperv internals. > > > > > > > > Co-developed-by: Yu Zhang <[email protected]> > > > > Signed-off-by: Yu Zhang <[email protected]> > > > > Signed-off-by: Easwar Hariharan <[email protected]> > > > > --- > > > > drivers/hv/hv_common.c | 95 +++++++++++++++++++++++++++++ > > > > drivers/pci/controller/pci-hyperv.c | 21 +++++-- > > > > include/asm-generic/mshyperv.h | 13 ++++ > > > > include/linux/hyperv.h | 8 +++ > > > > 4 files changed, 132 insertions(+), 5 deletions(-) > > > > > > > > diff --git a/drivers/hv/hv_common.c b/drivers/hv/hv_common.c > > > > index 6b67ac616789..53493f8d14dc 100644 > > > > --- a/drivers/hv/hv_common.c > > > > +++ b/drivers/hv/hv_common.c > > > > @@ -26,6 +26,8 @@ > > > > #include <linux/kmsg_dump.h> > > > > #include <linux/sizes.h> > > > > #include <linux/slab.h> > > > > +#include <linux/list.h> > > > > +#include <linux/spinlock.h> > > > > #include <linux/dma-map-ops.h> > > > > #include <linux/set_memory.h> > > > > #include <hyperv/hvhdk.h> > > > > @@ -863,3 +865,96 @@ const char *hv_result_to_string(u64 status) > > > > return "Unknown"; > > > > } > > > > EXPORT_SYMBOL_GPL(hv_result_to_string); > > > > + > > > > +#ifdef CONFIG_HYPERV_PVIOMMU > > > > +/* > > > > + * Logical device ID registry shared between the vPCI bus driver > > > > + * (pci-hyperv) and the para-virtualized IOMMU driver. The vPCI driver > > > > + * registers the per-bus logical device ID prefix at bus probe time, > > > > and > > > > + * the pvIOMMU driver looks it up to build the full logical device ID > > > > used > > > > + * in IOMMU hypercalls. > > > > + */ > > > > +struct hv_pci_busdata { > > > > + int pci_domain_nr; > > > > + u32 logical_dev_id_prefix; > > > > + struct list_head list; > > > > +}; > > > > + > > > > +static LIST_HEAD(hv_pci_bus_list); > > > > +static DEFINE_SPINLOCK(hv_pci_bus_lock); > > > > + > > > > +int hv_iommu_register_pci_bus(int pci_domain_nr, u32 > > > > logical_dev_id_prefix) > > > > +{ > > > > + struct hv_pci_busdata *bus, *new; > > > > + int ret = 0; > > > > + > > > > + new = kzalloc_obj(*new, GFP_KERNEL); > > > > + if (!new) > > > > + return -ENOMEM; > > > > + > > > > + spin_lock(&hv_pci_bus_lock); > > > > + list_for_each_entry(bus, &hv_pci_bus_list, list) { > > > > + if (bus->pci_domain_nr != pci_domain_nr) > > > > + continue; > > > > + > > > > + if (bus->logical_dev_id_prefix != > > > > logical_dev_id_prefix) { > > > > + pr_err("stale registration for PCI domain %d > > > > (old prefix 0x%08x, new 0x%08x)\n", > > > > + pci_domain_nr, > > > > bus->logical_dev_id_prefix, > > > > + logical_dev_id_prefix); > > > > + ret = -EEXIST; > > > > + } > > > > + > > > > + goto out_free; > > > > + } > > > > + > > > > + new->pci_domain_nr = pci_domain_nr; > > > > + new->logical_dev_id_prefix = logical_dev_id_prefix; > > > > + list_add(&new->list, &hv_pci_bus_list); > > > > + spin_unlock(&hv_pci_bus_lock); > > > > + return 0; > > > > + > > > > +out_free: > > > > + spin_unlock(&hv_pci_bus_lock); > > > > + kfree(new); > > > > + return ret; > > > > +} > > > > +EXPORT_SYMBOL_FOR_MODULES(hv_iommu_register_pci_bus, "pci-hyperv"); > > > > + > > > > +void hv_iommu_unregister_pci_bus(int pci_domain_nr) > > > > +{ > > > > + struct hv_pci_busdata *bus, *tmp; > > > > + > > > > + spin_lock(&hv_pci_bus_lock); > > > > + list_for_each_entry_safe(bus, tmp, &hv_pci_bus_list, list) { > > > > + if (bus->pci_domain_nr == pci_domain_nr) { > > > > + list_del(&bus->list); > > > > + kfree(bus); > > > > + break; > > > > + } > > > > + } > > > > + spin_unlock(&hv_pci_bus_lock); > > > > +} > > > > +EXPORT_SYMBOL_FOR_MODULES(hv_iommu_unregister_pci_bus, "pci-hyperv"); > > > > + > > > > +/* > > > > + * Look up the logical device ID prefix registered for @pci_domain_nr. > > > > + * Returns 0 on success with *prefix filled in; -ENODEV if no entry is > > > > + * registered for that PCI domain. > > > > + */ > > > > +int hv_iommu_lookup_logical_dev_id(int pci_domain_nr, u32 *prefix) > > > > +{ > > > > + struct hv_pci_busdata *bus; > > > > + int ret = -ENODEV; > > > > + > > > > + spin_lock(&hv_pci_bus_lock); > > > > + list_for_each_entry(bus, &hv_pci_bus_list, list) { > > > > + if (bus->pci_domain_nr == pci_domain_nr) { > > > > + *prefix = bus->logical_dev_id_prefix; > > > > + ret = 0; > > > > + break; > > > > + } > > > > + } > > > > + spin_unlock(&hv_pci_bus_lock); > > > > + return ret; > > > > +} > > > > > > I started thinking about the mechanism here because it's somewhat > > > annoying that it takes 77 lines of code (sans comments) to manage > > > this simple little mapping. I also started thinking about how many entries > > > are likely to be in the mapping. A guest VM probably has fewer than 10 > > > entries unless it has multiple NICs and maybe some GPUs. But this code > > > is also intended to be used by the Linux-as-root code, and I'm thinking > > > that the number of PCI devices managed by the root could easily be a > > > hundred or more if the root is managing a couple dozen VMs on a large > > > physical server. Searching a linked list with 100 or more entries could be > > > a bit slow. > > > > > > If only the guest scenario were needed, you could declare a static > > > array with 64 entries (64 is an arbitrary upper bound), and just search > > > through the array instead of having to allocate memory, deal with > > > allocation failures, and deal with linked lists. But a fixed size array > > > would need to be much bigger for the root scenario, and you would > > > still be doing a linear search. > > > > > > A better alternative to consider is rhashtable, which is an existing > > > Linux kernel facility. Based on what an AI bot generated for me, the > > > code for setting up and using rhashtable is straightforward, and > > > would probably result in far fewer than 77 lines of code. Lookups > > > would also be faster than a linear search, at least for the root case > > > with more than just a few entries. I'd suggest looking at rhashtable > > > to see whether you like how the resulting code comes out for this > > > use case, and whether it really is simpler than a roll-your-own linked > > > list. > > > > > > > Thank you so much for thinking this through, Michael! That is really > > a valid concern. > > > > How about using XArray? It might be more lightweight compared with > > rhashtable. Using pci_domain_nr as the key and prefix as the value. > > Maybe sth. like: > > > > > > ┃ > > static DEFINE_XARRAY(hv_pci_bus_xa); > > > > int hv_iommu_register_pci_bus(int domain_nr, u32 prefix) > > { > > return xa_insert(&hv_pci_bus_xa, domain_nr, > > xa_mk_value(prefix), GFP_KERNEL); > > } > > ┃ > > > > void hv_iommu_unregister_pci_bus(int domain_nr) > > { > > xa_erase(&hv_pci_bus_xa, domain_nr); > > } > > > > int hv_iommu_lookup_logical_dev_id(int domain_nr, u32 *prefix) > > { > > void *entry = xa_load(&hv_pci_bus_xa, domain_nr); > > if (!entry) > > ┃ > > return -ENODEV; > > *prefix = xa_to_value(entry); > > return 0; > > ┃ > > } > > > > xarray is best where the keys are dense or mostly dense integers. In > this use case, the pci_domain_nr keys are essentially random 16-bit > values, which doesn't fit xarray as well. It would work, but wouldn't > be very efficient. See the 2nd paragraph of the documentation here: > > https://www.kernel.org/doc/html/latest/core-api/xarray.html#xarray >
Thanks for pointing out the documentation. You're right. With sparse 16-bit keys, a hash-based data structure makes more sense than a radix-based one. I'll try reworking it with rhashtable and let you know if it doesn't work out. B.R. Yu > Michael
