From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from dpdk.org (dpdk.org [92.243.14.124])
	by inbox.dpdk.org (Postfix) with ESMTP id 926C5A00BE;
	Wed, 29 Apr 2020 02:01:55 +0200 (CEST)
Received: from [92.243.14.124] (localhost [127.0.0.1])
	by dpdk.org (Postfix) with ESMTP id 70CFC1D672;
	Wed, 29 Apr 2020 02:01:55 +0200 (CEST)
Received: from mail-lf1-f68.google.com (mail-lf1-f68.google.com
 [209.85.167.68]) by dpdk.org (Postfix) with ESMTP id D21161D671
 for <dev@dpdk.org>; Wed, 29 Apr 2020 02:01:54 +0200 (CEST)
Received: by mail-lf1-f68.google.com with SMTP id f8so124814lfe.12
 for <dev@dpdk.org>; Tue, 28 Apr 2020 17:01:54 -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=FaytoY55nV0B5kZPCgM1MMZRJsaPq6tRm/pmV+xAZac=;
 b=j0YLlBIsWmiaon+CBpnbUFBTrdLa/WgTChYfyX8CDtzFBi9AHKEkFI2qP/FyJZGkro
 QPhHyAJE6xYv7sHI/ZbM1nT5rw7QMEEIMM8zcoFE1A/eoLvJOHTkYUdCK+0shUXWM/BG
 2UjJGrX9jISAZmRJnoOxvnLN1aCNBUpG1UOHIX+/9kkdet0vZfDtKOqRf4tcByTBLItK
 qx+4UR2+/Mt+jDrIuEYS6G4CYwOuzA4X9p77szoyAoD3Uadq3RG+ZdaeQzK1Fi0uBYac
 koBVDCP2OfGGRkJXm43gbF+N30/+CWSrVxN4gdE46MpOSnCa+GsrUWU2mNTG23+nnnFV
 nWgg==
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=FaytoY55nV0B5kZPCgM1MMZRJsaPq6tRm/pmV+xAZac=;
 b=TDlL0e6fzzKBJJM/j+LEUczMcghvci73huZSLAtamOevGtA30muUpjcDidYZ7rqnNn
 5lsTeyZeBbG+wUdITjFWr4/IQqOcWCwX0KZYnPFON4OCdYtgJnL8Q0VwagglvsBs5m0D
 9GEawwU/QU5mc5u/SENpY/9vSq7xgOLAdHVwPzmB74Cj+TJyRx1eLMWxEERHNBF950W3
 ic+rh+Z1AyezD90XO6XoQkmO2DFLTGMlNt8RqKDjd0ZFqZjwZIWlTpwYoG5tMM/pxTR6
 iW17Eo5/pw7wEGTJby+fVRX12wSatvQnfNlOjHArsq1awfycfGYYtJaf6+HswKHgNeti
 PgBw==
X-Gm-Message-State: AGi0PuZXhy9tUY1BKWLwkZDSs7/pi3wVH+D574+d5ft3/QLKf9zrnN82
 KMsa8Xv4ygScHsP7Hy9yxUc=
X-Google-Smtp-Source: APiQypIy9jPM1fOhCxWzn0WY8KiWOaf36pw3ce62BpjDurbI2HNIo/6SiXSIh6kJiHZwAnfDjhBIig==
X-Received: by 2002:ac2:5c4e:: with SMTP id s14mr21058469lfp.77.1588118514345; 
 Tue, 28 Apr 2020 17:01:54 -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 w24sm790109lfe.58.2020.04.28.17.01.53
 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);
 Tue, 28 Apr 2020 17:01:53 -0700 (PDT)
Date: Wed, 29 Apr 2020 03:01:52 +0300
From: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
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
Message-ID: <20200429030152.0ebaae32@Sovereign>
In-Reply-To: <20200428091111.13416-8-talshn@mellanox.com>
References: <20200428091111.13416-1-talshn@mellanox.com>
 <20200428091111.13416-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=US-ASCII
Content-Transfer-Encoding: 7bit
Subject: Re: [dpdk-dev] [PATCH v2 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 <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

On 2020-04-28 12:11 GMT+0300 talshn@mellanox.com wrote:
[snip]
> +	switch (dev->kdrv) {
> +	case RTE_KDRV_NONE:
> +		/* Get NUMA node using DEVPKEY_Device_Numa_Node */
> +		bResult = SetupDiGetDevicePropertyW(hDevInfo, pDeviceInfoData,
> +			&DEVPKEY_Device_Numa_Node, &uPropertyType,
> +			(BYTE *)&uNumaNode, sizeof(uNumaNode), NULL, 0);
> +		if (!bResult) {
> +			ret = GetLastError();
> +			goto end;
> +		}
> +		dev->device.numa_node = uNumaNode;

Note: NUMA node != socket ID, but this field is used as socket ID by PMDs.
I suggest adding Windows-only EAL API to do the translation.

[snip]
> +	/* We now have the various hw IDs in tokens in the str[] array */
> +	/* Isolate the numerical IDs - '_' as the delimiter */
> +	if (parse_hardware_ids(str[0], strlen(str[0]), vendorID, NULL))
> +		goto end;
> +
> +	if (parse_hardware_ids(str[1], strlen(str[1]), deviceID, NULL))
> +		goto end;
> +
> +	if (parse_hardware_ids(str[2], strlen(str[2]), subdeviceID,
> +		subvendorID)) {
> +		goto end;
> +	}

You could avoid hand-written string parsing by using sscanf(), because
hardware ID formats are fixed, limited, and documented:

	https://docs.microsoft.com/en-us/windows-hardware/drivers/install/identifiers-for-pci-devices

[snip]
> +
> +	bResult = SetupDiGetDeviceRegistryPropertyA(hDevInfo, pDeviceInfoData,
> +		SPDRP_LOCATION_INFORMATION, NULL, (BYTE *)&strPCIDeviceInfo,
> +		sizeof(strPCIDeviceInfo), NULL);

> +
> +	/* Some devices don't have location information - simply return 0 */
> +	/* Also, if we don't find 'PCI' in the description, we'll skip it */
> +	if (!bResult) {
> +		ret = (GetLastError() == ERROR_INVALID_DATA) ? ERROR_CONTINUE
> +			: -1;
> +		goto end;
> +	} else if (!strstr(strPCIDeviceInfo, "PCI")) {
> +		ret = ERROR_CONTINUE;
> +		goto end;
> +	}

You could get PCI address without parsing strings with SPDRP_BUSNUMBER and
SPDRP_ADDRESS.

> +
> +	struct rte_pci_addr addr;
> +
> +	if (parse_pci_addr_format((const char *)&strPCIDeviceInfo,
> +			sizeof(strPCIDeviceInfo), &addr) != 0) {
> +		ret = -1;
> +		goto end;
> +	}
> +
> +	dev->addr.domain = addr.domain;
> +	dev->addr.bus = addr.bus;
> +	dev->addr.devid = addr.devid;
> +	dev->addr.function = addr.function;

Why not dev->addr = addr?

[snip]
> diff --git a/lib/librte_eal/rte_eal_exports.def b/lib/librte_eal/rte_eal_exports.def
> index edbb6b277..7a0468a02 100644
> --- a/lib/librte_eal/rte_eal_exports.def
> +++ b/lib/librte_eal/rte_eal_exports.def
> @@ -18,6 +18,7 @@ EXPORTS
>  	rte_eal_tailq_lookup
>  	rte_eal_tailq_register
>  	rte_eal_using_phys_addrs
> +	rte_find_numerical_value
>  	rte_free
>  	rte_log
>  	rte_malloc

This belongs to the patch that added the API, doesn't it?

-- 
Dmitry Kozlyuk