Re: [dpdk-dev] [PATCH v7] bus/pci: netuio interface for windows

[Tal Shnaiderman <talshn@nvidia.com>]

> Subject: [PATCH v7] bus/pci: netuio interface for windows 
> 
> This patch adds implementations to probe PCI devices bound to netuio with
> the help of "netuio" class device changes.
> Now Windows will support both "netuio" and "net" device class and can set
> kernel driver type based on the device class selection.
> 
> Note: Few definitions and structures have been copied from
> netuio_interface.h file from ("[v5] windows/netuio: add Windows NetUIO
> kernel driver") series and this will be fixed once the exact path for netuio
> source code is known.
> 
> v7 changes:
>         Removed device property key from pci_netuio.c as Numa detection
>         is done only in pci.c file
> v6 changes:
>         - Split up the get_netuio_device_info() function into smaller
>           functions
>         - Moved get numa node id code to get_device_resource_info() in
>           order to work on both RTE_PCI_KDRV_NONE and
> RTE_PCI_KDRV_NIC_UIO
> v5 changes:
>         Changed when netuio driver handle is closed
> v4 changes:
>         Removed 'reserved' member as it is not used
> v3 changes:
>         Removed the casts
> v2 changes:
>         - Moved all netuio specific definitions and functions to
>           pci_netuio.c and pci_netuio.h files
>         - Added a single function call to scan all the devices
> 
> Signed-off-by: John Alexander <John.Alexander@datapath.co.uk>
> Signed-off-by: Pallavi Kadam <pallavi.kadam@intel.com>
> Reviewed-by: Ranjit Menon <ranjit.menon@intel.com>
> ---
>  drivers/bus/pci/meson.build          |   3 +-
>  drivers/bus/pci/windows/pci.c        |  73 +++++++----
>  drivers/bus/pci/windows/pci_netuio.c | 182
> +++++++++++++++++++++++++++  drivers/bus/pci/windows/pci_netuio.h |
> 59 +++++++++
>  4 files changed, 289 insertions(+), 28 deletions(-)  create mode 100644
> drivers/bus/pci/windows/pci_netuio.c
>  create mode 100644 drivers/bus/pci/windows/pci_netuio.h
> 
> diff --git a/drivers/bus/pci/meson.build b/drivers/bus/pci/meson.build index
> b649a17c2..977800495 100644
> --- a/drivers/bus/pci/meson.build
> +++ b/drivers/bus/pci/meson.build
> @@ -18,7 +18,8 @@ if is_freebsd
>         includes += include_directories('bsd')  endif  if is_windows
> -       sources += files('windows/pci.c')
> +       sources += files('windows/pci.c',
> +                       'windows/pci_netuio.c')
>         includes += include_directories('windows')  endif
> 
> diff --git a/drivers/bus/pci/windows/pci.c b/drivers/bus/pci/windows/pci.c
> index 9e5c8fafb..30f218cc4 100644
> --- a/drivers/bus/pci/windows/pci.c
> +++ b/drivers/bus/pci/windows/pci.c
> @@ -4,10 +4,10 @@
>  #include <rte_windows.h>
>  #include <rte_errno.h>
>  #include <rte_log.h>
> -#include <rte_eal_memconfig.h>
>  #include <rte_eal.h>
> 
>  #include "private.h"
> +#include "pci_netuio.h"
> 
>  #include <devpkey.h>
> 
> @@ -209,25 +209,27 @@ get_device_resource_info(HDEVINFO dev_info,
>         DEVPROPTYPE property_type;
>         DWORD numa_node;
>         BOOL  res;
> +       int ret;
> 
>         switch (dev->kdrv) {
>         case RTE_PCI_KDRV_NONE:
> -               /* Get NUMA node using DEVPKEY_Device_Numa_Node */
> -               res = SetupDiGetDevicePropertyW(dev_info, dev_info_data,
> -                       &DEVPKEY_Device_Numa_Node, &property_type,
> -                       (BYTE *)&numa_node, sizeof(numa_node), NULL, 0);
> -               if (!res) {
> -                       RTE_LOG_WIN32_ERR(
> -                               "SetupDiGetDevicePropertyW"
> -                               "(DEVPKEY_Device_Numa_Node)");
> -                       return -1;
> -               }
> -               dev->device.numa_node = numa_node;
>                 /* mem_resource - Unneeded for RTE_PCI_KDRV_NONE */
>                 dev->mem_resource[0].phys_addr = 0;
>                 dev->mem_resource[0].len = 0;
>                 dev->mem_resource[0].addr = NULL;
>                 break;
> +       case RTE_PCI_KDRV_NIC_UIO:
> +               /* get device info from netuio kernel driver */
> +               ret = get_netuio_device_info(dev_info, dev_info_data, dev);
> +               if (ret != 0) {
> +                       RTE_LOG(DEBUG, EAL,
> +                               "Could not retrieve device info for PCI device "
> +                               PCI_PRI_FMT,
> +                               dev->addr.domain, dev->addr.bus,
> +                               dev->addr.devid, dev->addr.function);
> +                       return ret;
> +               }
> +               break;
>         default:
>                 /* kernel driver type is unsupported */
>                 RTE_LOG(DEBUG, EAL,
> @@ -238,6 +240,17 @@ get_device_resource_info(HDEVINFO dev_info,
>                 return -1;
>         }
> 
> +       /* Get NUMA node using DEVPKEY_Device_Numa_Node */
> +       res = SetupDiGetDevicePropertyW(dev_info, dev_info_data,
> +               &DEVPKEY_Device_Numa_Node, &property_type,
> +               (BYTE *)&numa_node, sizeof(numa_node), NULL, 0);
> +       if (!res) {
> +               RTE_LOG_WIN32_ERR("SetupDiGetDevicePropertyW"
> +                       "(DEVPKEY_Device_Numa_Node)");
> +               return -1;
> +       }
> +       dev->device.numa_node = numa_node;
> +
>         return ERROR_SUCCESS;
>  }
> 
> @@ -286,13 +299,14 @@ parse_pci_hardware_id(const char *buf, struct
> rte_pci_id *pci_id)  }
> 
>  static void
> -get_kernel_driver_type(struct rte_pci_device *dev)
> +set_kernel_driver_type(PSP_DEVINFO_DATA device_info_data,
> +       struct rte_pci_device *dev)
>  {
> -       /*
> -        * If another kernel driver is supported the relevant checking
> -        * functions should be here
> -        */
> -       dev->kdrv = RTE_PCI_KDRV_NONE;
> +       /* set kernel driver type based on device class */
> +       if (IsEqualGUID(&(device_info_data->ClassGuid),
> &GUID_DEVCLASS_NETUIO))
> +               dev->kdrv = RTE_PCI_KDRV_NIC_UIO;
> +       else
> +               dev->kdrv = RTE_PCI_KDRV_NONE;
>  }
> 
>  static int
> @@ -335,7 +349,7 @@ pci_scan_one(HDEVINFO dev_info,
> PSP_DEVINFO_DATA device_info_data)
> 
>         pci_name_set(dev);
> 
> -       get_kernel_driver_type(dev);
> +       set_kernel_driver_type(device_info_data, dev);
> 
>         /* get resources */
>         if (get_device_resource_info(dev_info, device_info_data, dev) @@ -
> 391,8 +405,8 @@ rte_pci_scan(void)
>         if (!rte_eal_has_pci())
>                 return 0;
> 
> -       dev_info = SetupDiGetClassDevs(&GUID_DEVCLASS_NET, TEXT("PCI"),
> NULL,
> -                               DIGCF_PRESENT);
> +       dev_info = SetupDiGetClassDevs(NULL, TEXT("PCI"), NULL,
> +               DIGCF_PRESENT | DIGCF_ALLCLASSES);
>         if (dev_info == INVALID_HANDLE_VALUE) {
>                 RTE_LOG_WIN32_ERR("SetupDiGetClassDevs(pci_scan)");
>                 RTE_LOG(ERR, EAL, "Unable to enumerate PCI devices.\n"); @@ -
> 405,12 +419,17 @@ rte_pci_scan(void)
>         while (SetupDiEnumDeviceInfo(dev_info, device_index,
>             &device_info_data)) {
>                 device_index++;
> -               ret = pci_scan_one(dev_info, &device_info_data);
> -               if (ret == ERROR_SUCCESS)
> -                       found_device++;
> -               else if (ret != ERROR_CONTINUE)
> -                       goto end;
> -
> +               /* we only want to enumerate net & netuio class devices */
> +               if (IsEqualGUID(&(device_info_data.ClassGuid),
> +                   &GUID_DEVCLASS_NET) ||
> +                       IsEqualGUID(&(device_info_data.ClassGuid),
> +                           &GUID_DEVCLASS_NETUIO)) {
> +                       ret = pci_scan_one(dev_info, &device_info_data);
> +                       if (ret == ERROR_SUCCESS)
> +                               found_device++;
> +                       else if (ret != ERROR_CONTINUE)
> +                               goto end;
> +               }
>                 memset(&device_info_data, 0, sizeof(SP_DEVINFO_DATA));
>                 device_info_data.cbSize = sizeof(SP_DEVINFO_DATA);
>         }
> diff --git a/drivers/bus/pci/windows/pci_netuio.c
> b/drivers/bus/pci/windows/pci_netuio.c
> new file mode 100644
> index 000000000..670194839
> --- /dev/null
> +++ b/drivers/bus/pci/windows/pci_netuio.c
> @@ -0,0 +1,182 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(c) 2020 Intel Corporation.
> + */
> +
> +#include <rte_windows.h>
> +#include <rte_errno.h>
> +#include <rte_log.h>
> +#include <rte_eal.h>
> +
> +#include "private.h"
> +#include "pci_netuio.h"
> +
> +static int
> +send_ioctl(HANDLE f, DWORD ioctl,
> +       void *in_buf, DWORD in_buf_size, void *out_buf, DWORD
> +out_buf_size) {
> +       BOOL res;
> +       DWORD bytes_ret = 0;
> +
> +       res = DeviceIoControl(f, ioctl, in_buf, in_buf_size,
> +               out_buf, out_buf_size, &bytes_ret, NULL);
> +       if (!res) {
> +               RTE_LOG_WIN32_ERR("DeviceIoControl:IOCTL query failed");
> +               return -1;
> +       }
> +
> +       return ERROR_SUCCESS;
> +}
> +
> +static HDEVINFO
> +get_netuio_device_information_set(HDEVINFO dev_info,
> +       PSP_DEVINFO_DATA dev_info_data)
> +{
> +       BOOL res;
> +       DWORD required_size = 0;
> +       TCHAR dev_instance_id[MAX_DEVICENAME_SZ];
> +       HDEVINFO di_set = INVALID_HANDLE_VALUE;
> +
> +       /* obtain the driver interface for this device */
> +       res = SetupDiGetDeviceInstanceId(dev_info, dev_info_data,
> +               dev_instance_id, sizeof(dev_instance_id), &required_size);
> +       if (!res) {
> +               RTE_LOG_WIN32_ERR("SetupDiGetDeviceInstanceId");
> +               goto end;
> +       }
> +
> +       /* return the device information set for this device */
> +       di_set = SetupDiGetClassDevs(&GUID_DEVINTERFACE_NETUIO,
> +               dev_instance_id, NULL, DIGCF_PRESENT |
> DIGCF_DEVICEINTERFACE);
> +       if (di_set == INVALID_HANDLE_VALUE) {
> +               RTE_LOG_WIN32_ERR("SetupDiGetClassDevs(device information
> set)");
> +               goto end;
> +       }
> +end:
> +       return di_set;
> +}
> +
> +static PSP_DEVICE_INTERFACE_DETAIL_DATA
> +get_netuio_device_interface_detail(HDEVINFO di_set) {
> +       BOOL res;
> +       DWORD required_size = 0;
> +       SP_DEVICE_INTERFACE_DATA  dev_ifx_data = { 0 };
> +       PSP_DEVICE_INTERFACE_DETAIL_DATA dev_ifx_detail = NULL;
> +
> +       dev_ifx_data.cbSize = sizeof(SP_DEVICE_INTERFACE_DATA);
> +
> +       /* enumerate the netUIO interfaces for this device information set */
> +       res = SetupDiEnumDeviceInterfaces(di_set, 0,
> &GUID_DEVINTERFACE_NETUIO,
> +               0, &dev_ifx_data);
> +       if (!res) {
> +               RTE_LOG_WIN32_ERR("SetupDiEnumDeviceInterfaces: no device
> interface");
> +               goto end;
> +       }
> +
> +       /* request and allocate required size for the device interface detail */
> +       required_size = 0;
> +       res = SetupDiGetDeviceInterfaceDetail(di_set, &dev_ifx_data, NULL, 0,
> +               &required_size, NULL);
> +       if (!res) {
> +               /* ERROR_INSUFFICIENT_BUFFER is expected */
> +               if (GetLastError() != ERROR_INSUFFICIENT_BUFFER) {
> +                       RTE_LOG_WIN32_ERR("SetupDiGetDeviceInterfaceDetail");
> +                       goto end;
> +               }
> +       }
> +
> +       dev_ifx_detail = malloc(required_size);
> +       if (!dev_ifx_detail) {
> +               RTE_LOG(ERR, EAL, "Could not allocate memory for dev
> interface.\n");
> +               goto end;
> +       }
> +       dev_ifx_detail->cbSize =
> + sizeof(SP_DEVICE_INTERFACE_DETAIL_DATA);
> +
> +       res = SetupDiGetDeviceInterfaceDetail(di_set, &dev_ifx_data,
> +               dev_ifx_detail, required_size, NULL, NULL);
> +       if (!res) {
> +               RTE_LOG_WIN32_ERR("SetupDiGetDeviceInterfaceDetail");
> +               free(dev_ifx_detail);
> +               dev_ifx_detail = NULL;
> +               goto end;
> +       }
> +
> +end:
> +       return dev_ifx_detail;
> +}
> +
> +/*
> + * get device resource information by sending ioctl to netuio driver
> +*/ int get_netuio_device_info(HDEVINFO dev_info, PSP_DEVINFO_DATA
> +dev_info_data,
> +       struct rte_pci_device *dev)
> +{
> +       int ret = -1;
> +       HDEVINFO di_set = INVALID_HANDLE_VALUE;
> +       PSP_DEVICE_INTERFACE_DETAIL_DATA dev_ifx_detail = NULL;
> +       HANDLE netuio = INVALID_HANDLE_VALUE;
> +       struct device_info hw_info = { 0 };
> +       unsigned int idx;
> +
> +       /* obtain the device information set for this device */
> +       di_set = get_netuio_device_information_set(dev_info, dev_info_data);
> +       if (di_set == INVALID_HANDLE_VALUE)
> +               goto end;
> +
> +       /* obtain the device interface detail for this device */
> +       dev_ifx_detail = get_netuio_device_interface_detail(di_set);
> +       if (!dev_ifx_detail)
> +               goto end;
> +
> +       /* open the kernel driver */
> +       netuio = CreateFile(dev_ifx_detail->DevicePath,
> +               GENERIC_READ | GENERIC_WRITE,
> +               FILE_SHARE_READ | FILE_SHARE_WRITE,
> +               NULL,
> +               OPEN_EXISTING,
> +               FILE_ATTRIBUTE_NORMAL,
> +               NULL);
> +       if (netuio == INVALID_HANDLE_VALUE) {
> +               RTE_LOG_WIN32_ERR("CreateFile");
> +               RTE_LOG(ERR, EAL, "Unable to open driver file \"%s\".\n",
> +                       dev_ifx_detail->DevicePath);
> +               goto end;
> +       }
> +
> +       /* send ioctl to retrieve device information */
> +       if (send_ioctl(netuio, IOCTL_NETUIO_MAP_HW_INTO_USERSPACE,
> NULL, 0,
> +               &hw_info, sizeof(hw_info)) != ERROR_SUCCESS) {
> +               RTE_LOG(ERR, EAL, "Unable to send ioctl to driver.\n");
> +               goto end;
> +       }
> +
> +       /* set relevant values into the dev structure */
> +       for (idx = 0; idx < PCI_MAX_RESOURCE; idx++) {
> +               dev->mem_resource[idx].phys_addr =
> +                   hw_info.hw[idx].phys_addr.QuadPart;
> +               dev->mem_resource[idx].addr =
> +                   hw_info.hw[idx].user_mapped_virt_addr;
> +               dev->mem_resource[idx].len = hw_info.hw[idx].size;
> +       }
> +
> +       ret = ERROR_SUCCESS;
> +end:
> +       if (ret != ERROR_SUCCESS) {
> +               /* Only close the handle to the driver in case of an error.
> +                * Otherwise, we want to keep the handle open. Closing it
> +                * here will cause the driver to unmap all the process-mapped
> +                * values resulting in invalid addresses.
> +                */
> +               if (netuio != INVALID_HANDLE_VALUE)
> +                       CloseHandle(netuio);
> +       }
> +
> +       if (dev_ifx_detail)
> +               free(dev_ifx_detail);
> +
> +       if (di_set != INVALID_HANDLE_VALUE)
> +               SetupDiDestroyDeviceInfoList(di_set);
> +
> +       return ret;
> +}
> diff --git a/drivers/bus/pci/windows/pci_netuio.h
> b/drivers/bus/pci/windows/pci_netuio.h
> new file mode 100644
> index 000000000..9a77806b5
> --- /dev/null
> +++ b/drivers/bus/pci/windows/pci_netuio.h
> @@ -0,0 +1,59 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(c) 2020 Intel Corporation
> + */
> +
> +#ifndef _PCI_NETUIO_H_
> +#define _PCI_NETUIO_H_
> +
> +/* GUID definition for device class netUIO */
> +DEFINE_GUID(GUID_DEVCLASS_NETUIO, 0x78912bc1, 0xcb8e, 0x4b28,
> +       0xa3, 0x29, 0xf3, 0x22, 0xeb, 0xad, 0xbe, 0x0f);
> +
> +/* GUID definition for the netuio device interface */
> +DEFINE_GUID(GUID_DEVINTERFACE_NETUIO, 0x08336f60, 0x0679, 0x4c6c,
> +       0x85, 0xd2, 0xae, 0x7c, 0xed, 0x65, 0xff, 0xf7);
> +
> +/* IOCTL code definitions */
> +#define IOCTL_NETUIO_MAP_HW_INTO_USERSPACE \
> +       CTL_CODE(FILE_DEVICE_NETWORK, 51, METHOD_BUFFERED, \
> +                        FILE_READ_ACCESS | FILE_WRITE_ACCESS)
> +
> +#define  MAX_DEVICENAME_SZ 255
> +
> +#pragma pack(push)
> +#pragma pack(8)
> +struct mem_region {
> +       UINT64 size;  /* memory region size */
> +       LARGE_INTEGER phys_addr;  /* physical address of the memory region
> */
> +       PVOID virt_addr;  /* virtual address of the memory region */
> +       PVOID user_mapped_virt_addr;  /* virtual address of the region
> mapped */
> +                                       /* into user process context */
> +};
> +
> +#define PCI_MAX_BAR 6
> +
> +struct device_info {
> +       struct mem_region hw[PCI_MAX_BAR]; }; #pragma pack(pop)
> +
> +/**
> + * Get device resource information by sending ioctl to netuio driver
> + *
> + * This function is private to EAL.
> + *
> + * @param dev_info
> + *   HDEVINFO handle to device information set
> + * @param dev_info_data
> + *   SP_DEVINFO_DATA structure holding information about this
> enumerated device
> + * @param dev
> + *   PCI device context for this device
> + * @return
> + *   - 0 on success.
> + *   - negative on error.
> + */
> +int
> +get_netuio_device_info(HDEVINFO dev_info, PSP_DEVINFO_DATA
> dev_info_data,
> +       struct rte_pci_device *dev);
> +
> +#endif /* _PCI_NETUIO_H_ */
> --
> 2.18.0.windows.1

Reviewed-by: Tal Shnaiderman <talshn@nvidia.com>