[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH V7 09/11] vpci: add initial support for virtual PCI bus topology




On 28.07.22 17:26, Jan Beulich wrote:

Hello Jan

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 <oleksandr_andrushchenko@xxxxxxxx>

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.)

Thanks for the clarification.


I am still getting used to the changes which that patch series makes (I didn't write that code).

Asking for suggestion I didn't mean to point an exact place in the code, but rather a subsystem/software layer,

sorry if I was unclear.



@@ -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.


ok, will do



Jan

--
Regards,

Oleksandr Tyshchenko




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.