Re: [PATCH v2 1/2] rust: pci: skip probing VFs if driver doesn't support VFs

["Danilo Krummrich" <dakr@kernel.org>]

On Thu Oct 2, 2025 at 11:14 PM CEST, Danilo Krummrich wrote:
> On 10/2/25 11:04 PM, Jason Gunthorpe wrote:
>> On Thu, Oct 02, 2025 at 09:36:17PM +0200, Danilo Krummrich wrote:
>>> If we want to obtain the driver's private data from a device outside the scope
>>> of bus callbacks, we always need to ensure that the device is guaranteed to be
>>> bound and we also need to prove the type of the private data, since a device
>>> structure can't be generic over its bound driver.
>> 
>> pci_iov_get_pf_drvdata() does both of these things - this is what it
>> is for. Please don't open code it :(
>
> It makes no sense to use it, both of those things will be ensured in a more
> generic way in the base device implementation already (which is what I meant
> with layering).
>
> Both requirements are not specific to PCI, or the specific VF -> PF use-case.
>
> In order to guarantee soundness both of those things have to be guaranteed for
> any access to the driver's private data.

Actually, let me elaborate a bit more on this:

In C when a driver calls dev_get_drvdata() it asserts two things:

  (1) The device is bound to the driver calling this function.
  (2) It casts the returned void * to the correct type.

Obviously, relying on this in Rust would be fundamentally unsafe.

Hence, the idea is to implement Device::drvdata_borrow::<T>(), which takes a
&Device<Bound> as argument, which proves that the device must be bound.

The T must be the driver specific driver type, i.e. the type that implements
e.g. the pci::Driver trait.

With that, Device::drvdata_borrow() can internally check whether the asserted
type T matches the unique type ID that we store in the device in probe().

This way we can prove that the device is actually bound and that we cast to the
correct type.

Furthermore, the returned reference to the driver's private data can't out-live
the lifetime of the given &Device<Bound>, so we're also guaranteed that the
device won't be unbound while we still have a reference to the driver's private
data.

So, when we call pdev.physfn().drvdata_borrow::<NovaCore>() the checks are
included already.

> I will send some patches soon, I think this will make it obvious. :)
>>>> Certain conditions may be workable, some drivers seem to have
>>>> preferences not to call disable, though I think that is wrong :\
>>>
>>> I fully agree! I was told that this is because apparently some PF drivers are
>>> only loaded to enable SR-IOV and then removed to shrink the potential attack
>>> surface. Personally, I think that's slightly paranoid, if the driver would not
>>> do anything else than enable / disable SR-IOV, but I think we can work around
>>> this use-case if people really want it.
>> 
>> I've heard worse reasons than that. If that is the interest I'd
>> suggest they should just use VFIO and leave a userspace stub
>> process..
>
> I'm not sure I follow your proposal, can you elaborate?