bus/pci: set intr_handle type for secondary processes
Checks
Commit Message
Invoking rte_pci_read/write_config functions requires device with
a intr_handle type for using VFIO or UIO driver related functions.
Secondary processes rely on primary processes for device initialization
so they do not usually require using these functions. However, some PMDs,
like NFP PMD, require using these functions even for secondary processes.
Signed-off-by: Alejandro Lucero <alejandro.lucero@netronome.com>
---
drivers/bus/pci/linux/pci_vfio.c | 2 ++
drivers/bus/pci/pci_common_uio.c | 6 ++++++
2 files changed, 8 insertions(+)
Comments
On 27-Sep-18 1:30 PM, Alejandro Lucero wrote:
> Invoking rte_pci_read/write_config functions requires device with
> a intr_handle type for using VFIO or UIO driver related functions.
>
> Secondary processes rely on primary processes for device initialization
> so they do not usually require using these functions. However, some PMDs,
> like NFP PMD, require using these functions even for secondary processes.
>
> Signed-off-by: Alejandro Lucero <alejandro.lucero@netronome.com>
> ---
Hi Alejandro,
I’m curious of consequences of setting intr handle to a valid value when
we don’t have an interrupt thread. Something may try to use it (although
I couldn’t find any such usage).
PCI config read really uses intr handle type to discover userspace
driver type – this seems ever so slightly wrong, and looks like
something that should be part of rte_device somewhere, independent of
interrupt types. Do we have any other alternative to do the same thing
(i.e. know what userspace driver is used for a particular PCI device)?
On Thu, Oct 18, 2018 at 5:26 PM Burakov, Anatoly <anatoly.burakov@intel.com>
wrote:
> On 27-Sep-18 1:30 PM, Alejandro Lucero wrote:
> > Invoking rte_pci_read/write_config functions requires device with
> > a intr_handle type for using VFIO or UIO driver related functions.
> >
> > Secondary processes rely on primary processes for device initialization
> > so they do not usually require using these functions. However, some PMDs,
> > like NFP PMD, require using these functions even for secondary processes.
> >
> > Signed-off-by: Alejandro Lucero <alejandro.lucero@netronome.com>
> > ---
>
> Hi Alejandro,
>
> I’m curious of consequences of setting intr handle to a valid value when
> we don’t have an interrupt thread. Something may try to use it (although
> I couldn’t find any such usage).
>
>
The point is secondary processes do not deal with interrupts so I assume
setting the type does not change anything but it allows to use PCI
read/write functions by secondary processes.
> PCI config read really uses intr handle type to discover userspace
> driver type – this seems ever so slightly wrong, and looks like
> something that should be part of rte_device somewhere, independent of
> interrupt types. Do we have any other alternative to do the same thing
> (i.e. know what userspace driver is used for a particular PCI device)?
>
>
I agree current way not being specially good.
Your comment has reminded me there is another way: just using the kdrv
field from the rte_pci_device struct. I have code using that field for
doing a different thing in the NFP PMD depending on the driver in use, UIO
or VFIO. So I think a better patch would be just to modify those pci
functions for using kdrv field instead.
Adding Ferruh in the thread for commenting on this potential change.
> --
> Thanks,
> Anatoly
>
On 18-Oct-18 5:41 PM, Alejandro Lucero wrote:
>
>
> On Thu, Oct 18, 2018 at 5:26 PM Burakov, Anatoly
> <anatoly.burakov@intel.com <mailto:anatoly.burakov@intel.com>> wrote:
>
> On 27-Sep-18 1:30 PM, Alejandro Lucero wrote:
> > Invoking rte_pci_read/write_config functions requires device with
> > a intr_handle type for using VFIO or UIO driver related functions.
> >
> > Secondary processes rely on primary processes for device
> initialization
> > so they do not usually require using these functions. However,
> some PMDs,
> > like NFP PMD, require using these functions even for secondary
> processes.
> >
> > Signed-off-by: Alejandro Lucero <alejandro.lucero@netronome.com
> <mailto:alejandro.lucero@netronome.com>>
> > ---
>
> Hi Alejandro,
>
> I’m curious of consequences of setting intr handle to a valid value
> when
> we don’t have an interrupt thread. Something may try to use it
> (although
> I couldn’t find any such usage).
>
>
> The point is secondary processes do not deal with interrupts so I assume
> setting the type does not change anything but it allows to use PCI
> read/write functions by secondary processes.
>
> PCI config read really uses intr handle type to discover userspace
> driver type – this seems ever so slightly wrong, and looks like
> something that should be part of rte_device somewhere, independent of
> interrupt types. Do we have any other alternative to do the same thing
> (i.e. know what userspace driver is used for a particular PCI device)?
>
>
> I agree current way not being specially good.
>
> Your comment has reminded me there is another way: just using the kdrv
> field from the rte_pci_device struct. I have code using that field for
> doing a different thing in the NFP PMD depending on the driver in use,
> UIO or VFIO. So I think a better patch would be just to modify those pci
> functions for using kdrv field instead.
>
> Adding Ferruh in the thread for commenting on this potential change.
>
I definitely think the way you describe would be a better way to fix
this (i.e. use kdrv in PCI config functions rather than intr handle type).
On Fri, Oct 19, 2018 at 9:02 AM Burakov, Anatoly <anatoly.burakov@intel.com>
wrote:
> On 18-Oct-18 5:41 PM, Alejandro Lucero wrote:
> >
> >
> > On Thu, Oct 18, 2018 at 5:26 PM Burakov, Anatoly
> > <anatoly.burakov@intel.com <mailto:anatoly.burakov@intel.com>> wrote:
> >
> > On 27-Sep-18 1:30 PM, Alejandro Lucero wrote:
> > > Invoking rte_pci_read/write_config functions requires device with
> > > a intr_handle type for using VFIO or UIO driver related functions.
> > >
> > > Secondary processes rely on primary processes for device
> > initialization
> > > so they do not usually require using these functions. However,
> > some PMDs,
> > > like NFP PMD, require using these functions even for secondary
> > processes.
> > >
> > > Signed-off-by: Alejandro Lucero <alejandro.lucero@netronome.com
> > <mailto:alejandro.lucero@netronome.com>>
> > > ---
> >
> > Hi Alejandro,
> >
> > I’m curious of consequences of setting intr handle to a valid value
> > when
> > we don’t have an interrupt thread. Something may try to use it
> > (although
> > I couldn’t find any such usage).
> >
> >
> > The point is secondary processes do not deal with interrupts so I assume
> > setting the type does not change anything but it allows to use PCI
> > read/write functions by secondary processes.
> >
> > PCI config read really uses intr handle type to discover userspace
> > driver type – this seems ever so slightly wrong, and looks like
> > something that should be part of rte_device somewhere, independent of
> > interrupt types. Do we have any other alternative to do the same
> thing
> > (i.e. know what userspace driver is used for a particular PCI
> device)?
> >
> >
> > I agree current way not being specially good.
> >
> > Your comment has reminded me there is another way: just using the kdrv
> > field from the rte_pci_device struct. I have code using that field for
> > doing a different thing in the NFP PMD depending on the driver in use,
> > UIO or VFIO. So I think a better patch would be just to modify those pci
> > functions for using kdrv field instead.
> >
> > Adding Ferruh in the thread for commenting on this potential change.
> >
>
> I definitely think the way you describe would be a better way to fix
> this (i.e. use kdrv in PCI config functions rather than intr handle type).
>
>
I will send another patch then.
Thanks!
> --
> Thanks,
> Anatoly
>
@@ -545,7 +545,9 @@
struct pci_map *maps;
+ /* This is required for using rte_pci_read/write_config */
dev->intr_handle.fd = -1;
+ dev->intr_handle.type = RTE_INTR_HANDLE_VFIO_MSIX;
/* store PCI address string */
snprintf(pci_addr, sizeof(pci_addr), PCI_PRI_FMT,
@@ -31,6 +31,12 @@
struct mapped_pci_res_list *uio_res_list =
RTE_TAILQ_CAST(rte_uio_tailq.head, mapped_pci_res_list);
+ /* This is required for using rte_pci_read/write_config */
+ if (dev->kdrv == RTE_KDRV_IGB_UIO)
+ dev->intr_handle.type = RTE_INTR_HANDLE_UIO;
+ else
+ dev->intr_handle.type = RTE_INTR_HANDLE_UIO_INTX;
+
TAILQ_FOREACH(uio_res, uio_res_list, next) {
/* skip this element if it doesn't match our PCI address */