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: <CADEbmW1hG_et13ZSMcX8R-6FFjaEPUvnUveeg8_dEic0QqLS+Q@mail.gmail.com>
Date: Fri, 8 Mar 2024 14:35:49 +0100
From: Michal Schmidt <mschmidt@...hat.com>
To: Marcin Szycik <marcin.szycik@...ux.intel.com>
Cc: intel-wired-lan@...ts.osuosl.org, Jiri Pirko <jiri@...nulli.us>, 
	netdev@...r.kernel.org, Arkadiusz Kubalewski <arkadiusz.kubalewski@...el.com>, 
	Karol Kolacinski <karol.kolacinski@...el.com>, Jacob Keller <jacob.e.keller@...el.com>, 
	Jakub Kicinski <kuba@...nel.org>, Przemek Kitszel <przemyslaw.kitszel@...el.com>
Subject: Re: [Intel-wired-lan] [PATCH net-next v3 1/3] ice: add ice_adapter
 for shared data across PFs on the same NIC

On Fri, Mar 8, 2024 at 11:57 AM Marcin Szycik
<marcin.szycik@...ux.intel.com> wrote:
> 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@...hat.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?

5 bits for the slot number, 8 bits for the bus number. 5+18=13.
I did not find any existing definitions for this purpose in pci.h, but
I can add some local macros.

> > +            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))?

The Xarray call can fail with -EINVAL or -ENOMEM. The -EINVAL would be
the result only if I'd attempt to insert an unaligned pointer, which
I'm not doing, so that leaves -ENOMEM as the only possible error.

> > + */
> > +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
>


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ