Nouveau - Oct 2025 - [PATCH 0/2] rust: pci: expose is_virtfn() and reject VFs in nova-core

On Thu Oct 2, 2025 at 12:38 AM CEST, John Hubbard wrote:
> On 10/1/25 6:52 AM, Zhi Wang wrote:
>> On 1.10.2025 13.32, Danilo Krummrich wrote:
>>> On Wed Oct 1, 2025 at 3:22 AM CEST, John Hubbard wrote:
>>>> On 9/30/25 5:29 PM, Alistair Popple wrote:
>>>>> On 2025-10-01 at 08:07 +1000, John Hubbard <jhubbard at
nvidia.com> wrote...
> ...
>>> So, this patch series does not do anything uncommon.
>>>
>>>>> I'm guessing the proposal is to fail the probe()
function in nova-core for
>>>>> the VFs - I'm not sure but does the driver core
continue to try probing other
>>>>> drivers if one fails probe()? It seems like this would be
something best
>>>>> filtered on in the device id table, although I understand
that's not possible
>>>>> today.
>>>
>>> Yes, the driver core keeps going until it finds a driver that
succeeds probing
>>> or no driver is left to probe. (This behavior is also the reason
for the name
>>> probe() in the first place.)
>>>
>>> However, nowadays we ideally know whether a driver fits a device
before probe()
>>> is called, but there are still exceptions; with PCI virtual
functions we've just
>>> hit one of those.
>>>
>>> Theoretically, we could also indicate whether a driver handles
virtual functions
>>> through a boolean in struct pci_driver, which would be a bit more
elegant.
>>>
>>> If you want I can also pick this up with my SR-IOV RFC which will
probably touch
>>> the driver structure as well; I plan to send something in a few
days.
>
> As I mentioned in the other fork of this thread, I do think this is
> a good start. So unless someone disagrees, I'd like to go with this
> series (perhaps with better wording in the commit messages, and maybe
> a better comment above the probe() failure return) for now.
Indicating whether the driver supports VFs through a boolean in struct
pci_driver is about the same effort (well, maybe slightly more), but solves the
problem in a cleaner way since it avoids probe() being called in the first
place. Other existing drivers benefit from that as well.

Forget about the SR-IOV RFC I was talking about; I really just intended to offer
to take care of that. :)

On 10/1/25 3:52 PM, Danilo Krummrich wrote:
> On Thu Oct 2, 2025 at 12:38 AM CEST, John Hubbard wrote:
>> On 10/1/25 6:52 AM, Zhi Wang wrote:
>>> On 1.10.2025 13.32, Danilo Krummrich wrote:
>>>> On Wed Oct 1, 2025 at 3:22 AM CEST, John Hubbard wrote:
>>>>> On 9/30/25 5:29 PM, Alistair Popple wrote:
>>>>>> On 2025-10-01 at 08:07 +1000, John Hubbard <jhubbard
at nvidia.com> wrote...
>> ...
>> As I mentioned in the other fork of this thread, I do think this is
>> a good start. So unless someone disagrees, I'd like to go with this
>> series (perhaps with better wording in the commit messages, and maybe
>> a better comment above the probe() failure return) for now.
> 
> Indicating whether the driver supports VFs through a boolean in struct
> pci_driver is about the same effort (well, maybe slightly more), but solves
the
> problem in a cleaner way since it avoids probe() being called in the first
> place. Other existing drivers benefit from that as well.
Yes, that is cleaner, and like you say, nearly as easy.
> 
> Forget about the SR-IOV RFC I was talking about; I really just intended to
offer
> to take care of that. :)
I can send out a v2 with that "PCI driver bool: supports VFs"
approach,
glad to do that.


thanks,
-- 
John Hubbard

On Thu, Oct 02, 2025 at 12:52:10AM +0200, Danilo Krummrich wrote:
> Indicating whether the driver supports VFs through a boolean in struct
> pci_driver is about the same effort (well, maybe slightly more), but solves
the
> problem in a cleaner way since it avoids probe() being called in the first
> place. Other existing drivers benefit from that as well.
I'm strongly against that idea.

Drivers should not be doing things like this, giving them core code
helpers to do something they should not do is the wrong direction.

I think this patchset should be simply dropped. Novacore should try to
boot on a VF and fail if it isn't setup.

Jason