Comments inline -----Original Message----- From: Jiri Pirko <[email protected]> Sent: Friday, March 7, 2025 1:39 PM To: Kitszel, Przemyslaw <[email protected]> Cc: [email protected]; Nguyen, Anthony L <[email protected]>; [email protected]; Keller, Jacob E <[email protected]>; Jakub Kicinski <[email protected]>; Loktionov, Aleksandr <[email protected]>; Kolacinski, Karol <[email protected]>; Nitka, Grzegorz <[email protected]>; Schmidt, Michal <[email protected]>; Temerkhanov, Sergey <[email protected]> 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, [email protected] 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 <[email protected]> >Suggested-by: Jakub Kicinski <[email protected]> >Suggested-by: Jiri Pirko <[email protected]> >Reviewed-by: Aleksandr Loktionov <[email protected]> >Signed-off-by: Przemek Kitszel <[email protected]> >-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
