On 09/08/2022 16:50, Jan Beulich wrote:
> When passed -1, the function (taking a u16) will look for segment
> 0xffff, which might exist. If it exists, we may find (return) the wrong
> device.
>
> Signed-off-by: Jan Beulich <jbeul...@suse.com>
> ---
> An alternative would be to declare that both functions cannot be called
> with "wildcards" anymore. The last such use went away with f591755823a7
> ("IOMMU/PCI: don't let domain cleanup continue when device de-assignment
> failed") afaict.The way wildcards were used before were always bogus IMO. I suggest we take this opportunity to remove the ability to re-introduce that anti-pattern. > Each time I look at this pair of functions I wonder why we have two > copies of almost the same code (with a curious difference of only one > having ASSERT(pcidevs_locked())). Any opinions on deleting either one, > subsuming its functionality into the other one by allowing the domain > pointer to be NULL to signify "any"? I'm in two minds about this. Because they are the same, they ought to be deduped. Except they're both insane and both want changing to a less silly datastructure, at which point they will be different. It is a total waste to do an O(n) loop over all PCI devices in the system checking for equality to single device (and in the domain case, assignment to the domain). The domain variant should loop over the pci devices in that domain, because it is guaranteed to be a shorter list than all devices. The global lookup probably wants to investigate a more efficient datastructure because I bet this is a hotpath. ~Andrew
