netdev - RE: [PATCH iwl-next] ice: use DSN instead of PCI BDF for ice

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <MW3PR11MB4681A62C71659C430281A15680D52@MW3PR11MB4681.namprd11.prod.outlook.com>
Date: Fri, 7 Mar 2025 13:42:58 +0000
From: "Temerkhanov, Sergey" <sergey.temerkhanov@...el.com>
To: Jiri Pirko <jiri@...nulli.us>, "Kitszel, Przemyslaw"
	<przemyslaw.kitszel@...el.com>
CC: "intel-wired-lan@...ts.osuosl.org" <intel-wired-lan@...ts.osuosl.org>,
	"Nguyen, Anthony L" <anthony.l.nguyen@...el.com>, "netdev@...r.kernel.org"
	<netdev@...r.kernel.org>, "Keller, Jacob E" <jacob.e.keller@...el.com>, Jakub
 Kicinski <kuba@...nel.org>, "Loktionov, Aleksandr"
	<aleksandr.loktionov@...el.com>, "Kolacinski, Karol"
	<karol.kolacinski@...el.com>, "Nitka, Grzegorz" <grzegorz.nitka@...el.com>,
	"Schmidt, Michal" <mschmidt@...hat.com>
Subject: RE: [PATCH iwl-next] ice: use DSN instead of PCI BDF for ice_adapter
 index

Comments inline

-----Original Message-----
From: Jiri Pirko <jiri@...nulli.us> 
Sent: Friday, March 7, 2025 1:39 PM
To: Kitszel, Przemyslaw <przemyslaw.kitszel@...el.com>
Cc: intel-wired-lan@...ts.osuosl.org; Nguyen, Anthony L <anthony.l.nguyen@...el.com>; netdev@...r.kernel.org; Keller, Jacob E <jacob.e.keller@...el.com>; Jakub Kicinski <kuba@...nel.org>; Loktionov, Aleksandr <aleksandr.loktionov@...el.com>; Kolacinski, Karol <karol.kolacinski@...el.com>; Nitka, Grzegorz <grzegorz.nitka@...el.com>; Schmidt, Michal <mschmidt@...hat.com>; Temerkhanov, Sergey <sergey.temerkhanov@...el.com>
Subject: Re: [PATCH iwl-next] ice: use DSN instead of PCI BDF for ice_adapter index

Thu, Mar 06, 2025 at 10:11:46PM +0100, przemyslaw.kitszel@...el.com wrote:
>Use Device Serial Number instead of PCI bus/device/function for index 
>of struct ice_adapter.
>Functions on the same physical device should point to the very same 
>ice_adapter instance.
>
>This is not only simplification, but also fixes things up when PF is 
>passed to VM (and thus has a random BDF).

It might be worth checking behavior of different hypervisors/VMMs with multifunction PCI devices passthrough.
QEMU, for example, has options to set the BDFs explicitly.

>
>Suggested-by: Jacob Keller <jacob.e.keller@...el.com>
>Suggested-by: Jakub Kicinski <kuba@...nel.org>
>Suggested-by: Jiri Pirko <jiri@...nulli.us>
>Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@...el.com>
>Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@...el.com>

>-struct ice_adapter *ice_adapter_get(const struct pci_dev *pdev); -void 
>ice_adapter_put(const struct pci_dev *pdev);
>+struct ice_adapter *ice_adapter_get(struct pci_dev *pdev); void 
>+ice_adapter_put(struct pci_dev *pdev);
> 
> #endif /* _ICE_ADAPTER_H */
>diff --git a/drivers/net/ethernet/intel/ice/ice_adapter.c 
>b/drivers/net/ethernet/intel/ice/ice_adapter.c
>index 01a08cfd0090..b668339ed0ef 100644
>--- a/drivers/net/ethernet/intel/ice/ice_adapter.c
>+++ b/drivers/net/ethernet/intel/ice/ice_adapter.c
>@@ -1,7 +1,6 @@
> // SPDX-License-Identifier: GPL-2.0-only  // SPDX-FileCopyrightText: 
>Copyright Red Hat
> 
>-#include <linux/bitfield.h>
> #include <linux/cleanup.h>
> #include <linux/mutex.h>
> #include <linux/pci.h>
>@@ -14,29 +13,9 @@
> static DEFINE_XARRAY(ice_adapters);
> static DEFINE_MUTEX(ice_adapters_mutex);
> 
>-/* PCI bus number is 8 bits. Slot is 5 bits. Domain can have the rest. 
>*/ -#define INDEX_FIELD_DOMAIN GENMASK(BITS_PER_LONG - 1, 13)
>-#define INDEX_FIELD_DEV    GENMASK(31, 16)
>-#define INDEX_FIELD_BUS    GENMASK(12, 5)
>-#define INDEX_FIELD_SLOT   GENMASK(4, 0)
>-
>-static unsigned long ice_adapter_index(const struct pci_dev *pdev)
>+static unsigned long ice_adapter_index(struct pci_dev *pdev)
> {
>-	unsigned int domain = pci_domain_nr(pdev->bus);
>-
>-	WARN_ON(domain > FIELD_MAX(INDEX_FIELD_DOMAIN));
>-
>-	switch (pdev->device) {
>-	case ICE_DEV_ID_E825C_BACKPLANE:
>-	case ICE_DEV_ID_E825C_QSFP:
>-	case ICE_DEV_ID_E825C_SFP:
>-	case ICE_DEV_ID_E825C_SGMII:
>-		return FIELD_PREP(INDEX_FIELD_DEV, pdev->device);
>-	default:
>-		return FIELD_PREP(INDEX_FIELD_DOMAIN, domain) |
>-		       FIELD_PREP(INDEX_FIELD_BUS,    pdev->bus->number) |
>-		       FIELD_PREP(INDEX_FIELD_SLOT,   PCI_SLOT(pdev->devfn));
>-	}
>+	return (unsigned long)pci_get_dsn(pdev);

>How do you ensure there is no xarray index collision then you cut the number like this?

It is also probably necessary to check if all devices supported by the driver have DSN capability enabled.

Regards,
Sergey