Danilo Krummrich
2025-Oct-01 23:55 UTC
[PATCH 0/2] rust: pci: expose is_virtfn() and reject VFs in nova-core
On Thu Oct 2, 2025 at 1:51 AM CEST, John Hubbard wrote:> On 10/1/25 4:47 PM, Joel Fernandes wrote: >>> On Oct 1, 2025, at 7:00?PM, John Hubbard <jhubbard at nvidia.com> wrote: >>> ?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... >>>>> ... >> Here is my opinion and correct me if I missed something: >> >> It feels premature to remove the option of nova-core binding to a VF, since other options to disable auto probing do exist as Jason pointed out. >> >> Taking a parallel with VFIO pass through for instance, the user already has to do some diligence like preventing drivers from binding and then making vfio-pci bind to the device IDs. This case is similar though slightly different, but VFIO setup requires some configuration anyway so will it really improve anything? >> >> I quietly suggest holding on till there is a real need or we are sure nova cannot bind to, or operate on a VF. This > > I'm confident that nova-core cannot properly handle a VF with *today's* code. > There is no expectation at all for a VF to show up--yet. > > Which is why I think it's appropriate to skip it right now.I agree with John. If a driver does not support a certain device, it is not the user's responsibility to prevent probing. Currently nova-core does not support VFs, so it should never get probed for them in the first place.
Joel Fernandes
2025-Oct-02 00:48 UTC
[PATCH 0/2] rust: pci: expose is_virtfn() and reject VFs in nova-core
> On Oct 1, 2025, at 7:56?PM, Danilo Krummrich <dakr at kernel.org> wrote: > > ?On Thu Oct 2, 2025 at 1:51 AM CEST, John Hubbard wrote: >> On 10/1/25 4:47 PM, Joel Fernandes wrote: >>>> On Oct 1, 2025, at 7:00?PM, John Hubbard <jhubbard at nvidia.com> wrote: >>>> ?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... >>>>>> ... >>> Here is my opinion and correct me if I missed something: >>> >>> It feels premature to remove the option of nova-core binding to a VF, since other options to disable auto probing do exist as Jason pointed out. >>> >>> Taking a parallel with VFIO pass through for instance, the user already has to do some diligence like preventing drivers from binding and then making vfio-pci bind to the device IDs. This case is similar though slightly different, but VFIO setup requires some configuration anyway so will it really improve anything? >>> >>> I quietly suggest holding on till there is a real need or we are sure nova cannot bind to, or operate on a VF. This >> >> I'm confident that nova-core cannot properly handle a VF with *today's* code. >> There is no expectation at all for a VF to show up--yet. >> >> Which is why I think it's appropriate to skip it right now. > > I agree with John. > > If a driver does not support a certain device, it is not the user's > responsibility to prevent probing. Currently nova-core does not support VFs, so > it should never get probed for them in the first place.That works for me. If we are doing this, I would also suggest adding a detailed comment preceding the if statement, saying the reason for this is because the VFs share the same device IDs when in reality we have 2 different drivers that handle the different functions. Thanks.
John Hubbard
2025-Oct-02 00:54 UTC
[PATCH 0/2] rust: pci: expose is_virtfn() and reject VFs in nova-core
On 10/1/25 5:48 PM, Joel Fernandes wrote:>> On Oct 1, 2025, at 7:56?PM, Danilo Krummrich <dakr at kernel.org> wrote: >> ?On Thu Oct 2, 2025 at 1:51 AM CEST, John Hubbard wrote: >>> On 10/1/25 4:47 PM, Joel Fernandes wrote: >>>>> On Oct 1, 2025, at 7:00?PM, John Hubbard <jhubbard at nvidia.com> wrote: >>>>> ?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... >>>>>>> ... >> If a driver does not support a certain device, it is not the user's >> responsibility to prevent probing. Currently nova-core does not support VFs, so >> it should never get probed for them in the first place. > > That works for me. If we are doing this, I would also suggest adding a detailed comment preceding the if statement,The nova-core piece that decides this is not an if statement. It's a const. It really is cleaner. :) saying the reason for this is because the VFs share the same device IDs when in reality we have 2 different drivers that handle the different functions.>I've got it passing tests already, I'll add appropriate comments and post it shortly, and let's see what you think. thanks, -- John Hubbard