On 28.07.2022 16:16, Oleksandr wrote: > On 27.07.22 13:32, Jan Beulich wrote: >> On 19.07.2022 19:42, Oleksandr Tyshchenko wrote: >>> From: Oleksandr Andrushchenko <[email protected]> >>> >>> Assign SBDF to the PCI devices being passed through with bus 0. >>> The resulting topology is where PCIe devices reside on the bus 0 of the >>> root complex itself (embedded endpoints). >>> This implementation is limited to 32 devices which are allowed on >>> a single PCI bus. >>> >>> Please note, that at the moment only function 0 of a multifunction >>> device can be passed through. >> I've not been able to spot where this restriction is being enforced - >> can you please point me at the respective code? > > Nor have I found the respective code. > > Could you please suggest a place where to put such enforcement (I guess, > this should be present in the toolstack)?
Such check should be in the tool stack primarily to give a sensible error message to the user. Yet the hypervisor needs to check itself nevertheless. You know the code you're adding much better than I do, so I guess I'm a little puzzled by you asking me to suggest a place. (And for the tool stack I guess asking tool stack folks would get you better mileage.) >>> @@ -124,6 +191,7 @@ void vpci_deassign_device(struct pci_dev *pdev) >>> if ( !has_vpci(pdev->domain) ) >>> return; >>> >>> + vpci_remove_virtual_device(pdev); >>> vpci_remove_device(pdev); >>> } >> And other call sites of vpci_remove_device() do not have a need of >> cleaning up guest_sbdf / vpci_dev_assigned_map? > > I am not 100% sure, but it looks like they don't need. On the other > hand, even if they don't need that, doing the cleaning won't be an issue > at all, > > there is a check before cleaning (which will be extended as I proposed > above), so ... > > >> IOW I wonder if it >> wouldn't be better to have vpci_remove_device() do this as well >> (retaining - see my comment on the earlier patch) the simple aliasing >> of vpci_deassign_device() to vpci_remove_device()). > > > ... maybe yes. Shall I do that change? Well - yes please, afaic. Jan
