From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id A75B7A00C5; Fri, 8 May 2020 00:50:08 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id CA9A41DB7A; Fri, 8 May 2020 00:50:07 +0200 (CEST) Received: from mail-lf1-f66.google.com (mail-lf1-f66.google.com [209.85.167.66]) by dpdk.org (Postfix) with ESMTP id 3EC741DB79 for ; Fri, 8 May 2020 00:50:07 +0200 (CEST) Received: by mail-lf1-f66.google.com with SMTP id s9so5894495lfp.1 for ; Thu, 07 May 2020 15:50:07 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=date:from:to:cc:subject:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=ZT98Rn1PKG8nFdLTGYRVbJy0vLnEEPwakJlRVSOtb98=; b=jhyRwUfzVMbxMtTNbDQXN7VBfXQCqJ50yh5I1TC9wa2WeWErRi6K34z3iQBvV6ORBV h6f4x3BovK2j6KpNdjcBr9s134QmtG9WVIFlVjTyQ2JwvkWo0xgh8M7VPEG62UbqX2UW ztGDP+soIobMroFXnwA5xOziMn0SM4f6s4rs6zFr7EvYXO/NBeYlJf7vLk192TupIHh+ LfXFUuwyqVVx4SnMFDhb69NCWaqNcI06H2sAa9GU9yM9OQauaSzafP3oQfnCgPUcEC4h i8h9/+aI2+2nmrCUmmrQiRLKaAmr/5wFWKaU0e4tVLp2Ft1kgKuNcAauc5GsE6GszwDn JJYw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=ZT98Rn1PKG8nFdLTGYRVbJy0vLnEEPwakJlRVSOtb98=; b=qpEgjicRXwCrg/RBsDrHhgJmu5dQP11eI/n3HD1b5m+tKiTJavKeWpH12dRUf62+4i mXB3Ml/FR9swn9vGNO8HoSr8++N8ajP6noE6hbtKB5IhvNLzvLQO6X7uBdBlVyLzzBAn WNL/9i9NP9aEsxNsVrnK9MmQJkF57fahhyb4WI4fMVlORxhr4kp0L/7OziX4roc+dDyF ImidpOBqEzbdZC/Nliauoe52GdFoMHM12mN35eucDIyh0tP09lmlOshFbQCSJth4Ny1D W365xY64RUciyx6zx8e+aFT6Fa//wdRjkCA9DYkILMp+pxQZ9CAKV5gPYzZ9fQHyhZNN WBuA== X-Gm-Message-State: AGi0PuZMDkQpRyK7aj74zgs+ExY174hNsfw22NEhAe8kVBb/DdBedT4A 9kwAblu1hMAy6wXkcP+7n5g= X-Google-Smtp-Source: APiQypJ3WkuTZoplKm9yg9EKT9VRxtUSd+Y/HdcJZdu4vT/PV7y0mn5aHKZsu7dXrmfmF7igtH5w0w== X-Received: by 2002:a05:6512:3081:: with SMTP id z1mr10377558lfd.102.1588891806497; Thu, 07 May 2020 15:50:06 -0700 (PDT) Received: from Sovereign (broadband-37-110-65-23.ip.moscow.rt.ru. [37.110.65.23]) by smtp.gmail.com with ESMTPSA id a2sm4109658ljj.53.2020.05.07.15.50.05 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 07 May 2020 15:50:05 -0700 (PDT) Date: Fri, 8 May 2020 01:50:04 +0300 From: Dmitry Kozlyuk To: talshn@mellanox.com Cc: dev@dpdk.org, thomas@monjalon.net, pallavi.kadam@intel.com, david.marchand@redhat.com, grive@u256.net, ranjit.menon@intel.com, navasile@linux.microsoft.com, anatoly.burakov@intel.com Message-ID: <20200508015004.2510c9e7@Sovereign> In-Reply-To: <20200507121646.624-8-talshn@mellanox.com> References: <20200507121646.624-1-talshn@mellanox.com> <20200507121646.624-8-talshn@mellanox.com> X-Mailer: Claws Mail 3.17.5 (GTK+ 2.24.32; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [dpdk-dev] [PATCH v3 7/7] bus/pci: support Windows with bifurcated drivers X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 2020-05-07 15:16 GMT+0300 talshn@mellanox.com wrote: > From: Tal Shnaiderman >=20 > Uses SetupAPI.h functions to scan PCI tree. > Uses DEVPKEY_Device_Numa_Node to get the PCI NUMA node. > Uses SPDRP_BUSNUMBER and SPDRP_BUSNUMBER to get the BDF. > scanning currently supports types RTE_KDRV_NONE. >=20 > Signed-off-by: Tal Shnaiderman > --- > drivers/bus/pci/windows/pci.c | 225 +++++++++++++++++++++= +++++- > lib/librte_eal/windows/include/rte_windows.h | 1 + > 2 files changed, 224 insertions(+), 2 deletions(-) >=20 > diff --git a/drivers/bus/pci/windows/pci.c b/drivers/bus/pci/windows/pci.c > index b1d34ae11..e8eff4f6f 100644 > --- a/drivers/bus/pci/windows/pci.c > +++ b/drivers/bus/pci/windows/pci.c > @@ -1,14 +1,24 @@ > /* SPDX-License-Identifier: BSD-3-Clause > * Copyright 2020 Mellanox Technologies, Ltd > */ > +#include > #include > #include > - > #include Not used. > #include > =20 > #include "private.h" > =20 > +#include > + > +#define MAX_STR_TOKENS 8 > +#define DEC 10 > +#define HEX 16 Not used. > +/* > + * This code is used to simulate a PCI probe by parsing information in > + * the registry hive for PCI devices. > + */ > + > /* The functions below are not implemented on Windows, > * but need to be defined for compilation purposes > */ > @@ -158,6 +168,184 @@ pci_uio_remap_resource(struct rte_pci_device *dev _= _rte_unused) > */ > return -1; > } > + > +static > +int get_device_pci_address(HDEVINFO hDevInfo, > + PSP_DEVINFO_DATA pDeviceInfoData, struct rte_pci_addr *addr) Result type should be on a separate line (that is, "static int = "). This is also the first from numerous violations of DPDK naming conventions. IMO, Windows EAL should strictly follow them to become "a first-class citiz= en" in DPDK and to attract existing developers. Even if not, style should be at least consistent across each file (now we have snake_case, Hungarian notation, camelCase, and PascalCase mixed). > +{ > + BOOL bResult; > + ULONG bus_num, dev_and_func; > + > + bResult =3D SetupDiGetDeviceRegistryProperty(hDevInfo, pDeviceInfoData, > + SPDRP_BUSNUMBER, NULL, (PBYTE)&bus_num, sizeof(bus_num), NULL); > + if (!bResult) > + goto end; RTE_LOG_WIN32_ERROR("SetupDiGetDeviceRegistryProperty(SPDRP_BUSNUMBER)"), e= tc. would be more helpful in finding exact error cause here and in other places. Since you'll have to save GetLastError() before logging, you can also get r= id of goto and just use plain return. > + > + bResult =3D SetupDiGetDeviceRegistryProperty(hDevInfo, pDeviceInfoData, > + SPDRP_ADDRESS, NULL, (PBYTE)&dev_and_func, sizeof(dev_and_func), > + NULL); > + if (!bResult) > + goto end; > + > + addr->domain =3D 0; > + addr->bus =3D bus_num; > + addr->devid =3D dev_and_func >> 16; > + addr->function =3D dev_and_func & 0xffff; > + return 0; > +end: > + return GetLastError(); > + > +} > + [snip] > + > +static int > +pci_scan_one(HDEVINFO hDevInfo, PSP_DEVINFO_DATA pDeviceInfoData) > +{ > + struct rte_pci_device *dev; > + int ret =3D -1; > + > + dev =3D malloc(sizeof(struct rte_pci_device)); Nitpicking, but sizeof(*dev) would be more concise and robust. > + if (dev =3D=3D NULL) { > + ret =3D -1; > + goto end; > + } > + > + memset(dev, 0, sizeof(*dev)); > + > + char strPCIDeviceInfo[PATH_MAX]; > + BOOL bResult; [1/6] Compiling C object 'drivers/a715181@@rte_bus_pci@sta/bus_pci_windows_pci.c.obj'. ../../../drivers/bus/pci/windows/pci.c: In function =E2=80=98pci_scan_one= =E2=80=99: ../../../drivers/bus/pci/windows/pci.c:284:8: warning: variable =E2=80=98bR= esult=E2=80=99 set but not used [-Wunused-but-set-variable] > + struct rte_pci_addr addr; > + struct rte_pci_id pci_id; > + [snip] > int > rte_pci_scan(void) > { > - return 0; > + DWORD DeviceIndex =3D 0, FoundDevice =3D 0; > + HDEVINFO hDevInfo =3D NULL; > + SP_DEVINFO_DATA DeviceInfoData =3D { 0 }; No need to initialize hDevInfo and DeviceInfoData: first, there's a memset() below and NULL is meaningless for hDevInfo, second, this way you lose compiler warnings in case you really forget to initialize variables. > + int ret =3D -1; > + > + hDevInfo =3D SetupDiGetClassDevs(&GUID_DEVCLASS_NET, NULL, NULL, > + DIGCF_PRESENT); Shouldn't devices be limited to PCI here? > + if (hDevInfo =3D=3D INVALID_HANDLE_VALUE) { Would be helpful to log error code here. > + RTE_LOG(ERR, EAL, "Unable to enumerate PCI devices.\n"); > + goto end; > + } > + > + DeviceInfoData.cbSize =3D sizeof(SP_DEVINFO_DATA); > + DeviceIndex =3D 0; > + > + while (SetupDiEnumDeviceInfo(hDevInfo, DeviceIndex, &DeviceInfoData)) { > + DeviceIndex++; > + ret =3D pci_scan_one(hDevInfo, &DeviceInfoData); > + if (ret =3D=3D ERROR_SUCCESS) > + FoundDevice++; > + else if (ret !=3D ERROR_CONTINUE) > + goto end; > + > + memset(&DeviceInfoData, 0, sizeof(SP_DEVINFO_DATA)); > + DeviceInfoData.cbSize =3D sizeof(SP_DEVINFO_DATA); > + } > + > + RTE_LOG(ERR, EAL, "PCI scan found %lu devices\n", FoundDevice); Why ERR? > + ret =3D (FoundDevice !=3D 0) ? 0 : -1; Zero PCI network devices is not necessarily an error: consider testing with --vdev only, but without explicit --no-pci. Some issues I found when cross-compiling with MinGW-w64 (gcc 9.2.1 "cc (Arch Linux 9.2.1+20200130-2) 9.2.1 20200130"). 1. Missing DEVPKEY_Device_Numa_Node (MinGW-w64 defect): ../../../drivers/bus/pci/windows/pci.c: In function =E2=80=98get_device_res= ource_info=E2=80=99: ../../../drivers/bus/pci/windows/pci.c:213:5: error: =E2=80=98DEVPKEY_Devic= e_Numa_Node=E2=80=99 undeclared (first use in this function) 2. undefined reference to `__emutls_v.per_lcore__rte_errno' (related to TLS issues on Windows). 3. LINK.EXE specific options: FAILED: drivers/librte_bus_pci-20.0.dll=20 /usr/bin/x86_64-w64-mingw32-gcc -o drivers/librte_bus_pci-20.0.dll 'driver= s/a715181@@rte_bus_pci@sha/bus_pci_pci_common.c.obj' 'drivers/a715181@@rte_= bus_pci@sha/bus_pci_pci_params.c.obj' 'drivers/a715181@@rte_bus_pci@sha/bus= _pci_windows_pci.c.obj' -Wl,--allow-shlib-undefined -Wl,-O1 -shared -Wl,--s= tart-group -Wl,--out-implib=3Ddrivers/librte_bus_pci.dll.a -pthread -lm -la= dvapi32 -lsetupapi lib/librte_eal.dll.a lib/librte_kvargs.dll.a lib/librte_= pci.dll.a -Wl,/def:/home/dmitry/src/dpdk/public/build/cross/gcc/drivers/rte= _bus_pci_exports.def '-Wl,/implib:drivers\\librte_pci.dll.a' -lkernel32 -lu= ser32 -lgdi32 -lwinspool -lshell32 -lole32 -loleaut32 -luuid -lcomdlg32 -Wl= ,--end-group /usr/lib/gcc/x86_64-w64-mingw32/9.2.0/../../../../x86_64-w64-mingw32/bin/ld= : cannot find /def:/home/dmitry/src/dpdk/public/build/cross/gcc/drivers/rte= _bus_pci_exports.def: No such file or directory /usr/lib/gcc/x86_64-w64-mingw32/9.2.0/../../../../x86_64-w64-mingw32/bin/ld= : cannot find /implib:drivers\\librte_pci.dll.a: No such file or directory Cumulative fixes diff, feel free to distribute it among your patches: diff --git a/drivers/bus/pci/windows/pci.c b/drivers/bus/pci/windows/pci.c index e8eff4f6f..9568a090a 100644 --- a/drivers/bus/pci/windows/pci.c +++ b/drivers/bus/pci/windows/pci.c @@ -11,6 +11,10 @@ =20 #include =20 +#ifndef DEVPKEY_Device_Numa_Node +DEFINE_DEVPROPKEY(DEVPKEY_Device_Numa_Node, 0x540b947e, 0x8b40, 0x45bc, = 0xa8, 0xa2, 0x6a, 0x0b, 0x89, 0x4c, 0xbd, 0xa2, 3); +#endif + #define MAX_STR_TOKENS 8 #define DEC 10 #define HEX 16 diff --git a/drivers/meson.build b/drivers/meson.build index e11a1cd39..96af28b9c 100644 --- a/drivers/meson.build +++ b/drivers/meson.build @@ -159,7 +159,7 @@ foreach class:dpdk_driver_classes input: version_map, output: '@0@_exports.def'.format(lib_name)) lk_deps =3D [version_map, def_file] - if is_windows + if is_ms_linker lk_args =3D ['-Wl,/def:' + def_file.full_path(), '-Wl,/implib:drivers\\' + implib] else diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_ve= rsion.map index 5d6d3a8c6..74665145b 100644 --- a/lib/librte_eal/rte_eal_version.map +++ b/lib/librte_eal/rte_eal_version.map @@ -391,4 +391,6 @@ EXPERIMENTAL { rte_trace_point_lookup; rte_trace_regexp; rte_trace_save; + + __emutls_v.per_lcore__rte_errno; }; --=20 Dmitry Kozlyuk