Re: [Intel-wired-lan] [PATCH net-next v3 1/3] ice: add ice_adapter for shared data across PFs on the same NIC

[Marcin Szycik <marcin.szycik@linux.intel.com>]

On 07.03.2024 23:25, Michal Schmidt wrote:
> There is a need for synchronization between ice PFs on the same physical
> adapter.
> 
> Add a "struct ice_adapter" for holding data shared between PFs of the
> same multifunction PCI device. The struct is refcounted - each ice_pf
> holds a reference to it.
> 
> Its first use will be for PTP. I expect it will be useful also to
> improve the ugliness that is ice_prot_id_tbl.
> 
> Signed-off-by: Michal Schmidt <mschmidt@redhat.com>
> ---
>  drivers/net/ethernet/intel/ice/Makefile      |   3 +-
>  drivers/net/ethernet/intel/ice/ice.h         |   2 +
>  drivers/net/ethernet/intel/ice/ice_adapter.c | 107 +++++++++++++++++++
>  drivers/net/ethernet/intel/ice/ice_adapter.h |  22 ++++
>  drivers/net/ethernet/intel/ice/ice_main.c    |   8 ++
>  5 files changed, 141 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/net/ethernet/intel/ice/ice_adapter.c
>  create mode 100644 drivers/net/ethernet/intel/ice/ice_adapter.h
> 
> diff --git a/drivers/net/ethernet/intel/ice/Makefile b/drivers/net/ethernet/intel/ice/Makefile
> index cddd82d4ca0f..4fa09c321440 100644
> --- a/drivers/net/ethernet/intel/ice/Makefile
> +++ b/drivers/net/ethernet/intel/ice/Makefile
> @@ -36,7 +36,8 @@ ice-y := ice_main.o	\
>  	 ice_repr.o	\
>  	 ice_tc_lib.o	\
>  	 ice_fwlog.o	\
> -	 ice_debugfs.o
> +	 ice_debugfs.o  \
> +	 ice_adapter.o
>  ice-$(CONFIG_PCI_IOV) +=	\
>  	ice_sriov.o		\
>  	ice_virtchnl.o		\
> diff --git a/drivers/net/ethernet/intel/ice/ice.h b/drivers/net/ethernet/intel/ice/ice.h
> index 365c03d1c462..1ffecbdd361a 100644
> --- a/drivers/net/ethernet/intel/ice/ice.h
> +++ b/drivers/net/ethernet/intel/ice/ice.h
> @@ -77,6 +77,7 @@
>  #include "ice_gnss.h"
>  #include "ice_irq.h"
>  #include "ice_dpll.h"
> +#include "ice_adapter.h"
>  
>  #define ICE_BAR0		0
>  #define ICE_REQ_DESC_MULTIPLE	32
> @@ -544,6 +545,7 @@ struct ice_agg_node {
>  
>  struct ice_pf {
>  	struct pci_dev *pdev;
> +	struct ice_adapter *adapter;
>  
>  	struct devlink_region *nvm_region;
>  	struct devlink_region *sram_region;
> diff --git a/drivers/net/ethernet/intel/ice/ice_adapter.c b/drivers/net/ethernet/intel/ice/ice_adapter.c
> new file mode 100644
> index 000000000000..6b9eeba6edf7
> --- /dev/null
> +++ b/drivers/net/ethernet/intel/ice/ice_adapter.c
> @@ -0,0 +1,107 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +// SPDX-FileCopyrightText: Copyright Red Hat
> +
> +#include <linux/cleanup.h>
> +#include <linux/mutex.h>
> +#include <linux/pci.h>
> +#include <linux/slab.h>
> +#include <linux/xarray.h>
> +#include "ice_adapter.h"
> +
> +static DEFINE_XARRAY(ice_adapters);
> +
> +static unsigned long ice_adapter_index(const struct pci_dev *pdev)
> +{
> +	unsigned int domain = pci_domain_nr(pdev->bus);
> +
> +	WARN_ON((unsigned long)domain >> (BITS_PER_LONG - 13));
> +	return ((unsigned long)domain << 13) |
> +	       ((unsigned long)pdev->bus->number << 5) |

Magic numbers?

> +	       PCI_SLOT(pdev->devfn);
> +}
> +
> +static struct ice_adapter *ice_adapter_new(void)
> +{
> +	struct ice_adapter *adapter;
> +
> +	adapter = kzalloc(sizeof(*adapter), GFP_KERNEL);
> +	if (!adapter)
> +		return NULL;
> +
> +	refcount_set(&adapter->refcount, 1);
> +
> +	return adapter;
> +}
> +
> +static void ice_adapter_free(struct ice_adapter *adapter)
> +{
> +	kfree(adapter);
> +}
> +
> +DEFINE_FREE(ice_adapter_free, struct ice_adapter*, if (_T) ice_adapter_free(_T))
> +
> +/**
> + * ice_adapter_get - Get a shared ice_adapter structure.
> + * @pdev: Pointer to the pci_dev whose driver is getting the ice_adapter.
> + *
> + * Gets a pointer to a shared ice_adapter structure. Physical functions (PFs)
> + * of the same multi-function PCI device share one ice_adapter structure.
> + * The ice_adapter is reference-counted. The PF driver must use ice_adapter_put
> + * to release its reference.
> + *
> + * Context: Process, may sleep.
> + * Return:  Pointer to ice_adapter on success.
> + *          ERR_PTR() on error. -ENOMEM is the only possible error.

What about ERR_PTR(xa_err(ret))?

> + */
> +struct ice_adapter *ice_adapter_get(const struct pci_dev *pdev)
> +{
> +	struct ice_adapter *ret, __free(ice_adapter_free) *adapter = NULL;
> +	unsigned long index = ice_adapter_index(pdev);
> +
> +	adapter = ice_adapter_new();
> +	if (!adapter)
> +		return ERR_PTR(-ENOMEM);

---8<---

Thanks,
Marcin