Linux RDMA and InfiniBand development
 help / color / mirror / Atom feed
* Re: [PATCH v5 00/16] Add Paravirtual RDMA Driver
From: Leon Romanovsky @ 2016-09-26  5:57 UTC (permalink / raw)
  To: Adit Ranadive
  Cc: dledford, linux-rdma, pv-drivers, netdev, linux-pci, jhansen,
	asarwade, georgezhang, bryantan
In-Reply-To: <9f65ab5c-d8c2-e8e3-9334-5d1865a20dc9@vmware.com>

[-- Attachment #1: Type: text/plain, Size: 544 bytes --]

On Sun, Sep 25, 2016 at 10:25:12PM -0700, Adit Ranadive wrote:
> On Sun, Sep 25 2016 at 10:03:52AM +0300, Leon Romanovsky wrote:
> > On Sat, Sep 24, 2016 at 04:21:24PM -0700, Adit Ranadive wrote:
> >
> > <...>
> >
> > >  include/uapi/rdma/pvrdma-abi.h                 |   99 ++
> > >  include/uapi/rdma/pvrdma-uapi.h                |  255 +++++
> >
> > As Jason said, you need a very good reason to split and create number of
> > files per-driver in UAPI folder.
>
> I can move the pvrdma-uapi.h back to the pvrdma driver folder.

Yes, please.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* Re: [PATCH v5 16/16] MAINTAINERS: Update for PVRDMA driver
From: Leon Romanovsky @ 2016-09-26  5:56 UTC (permalink / raw)
  To: Adit Ranadive
  Cc: dledford, linux-rdma, pv-drivers, netdev, linux-pci, jhansen,
	asarwade, georgezhang, bryantan
In-Reply-To: <7ddc5906-6ba8-335e-c0b8-eb60427a1be8@vmware.com>

[-- Attachment #1: Type: text/plain, Size: 1722 bytes --]

On Sun, Sep 25, 2016 at 10:22:02PM -0700, Adit Ranadive wrote:
> On Sun, Sep 25 2016 at 10:30:10AM +0300, Leon Romanovsky wrote:
> > On Sat, Sep 24, 2016 at 04:21:40PM -0700, Adit Ranadive wrote:
> > > Add maintainer info for the PVRDMA driver.
> > >
> > > Reviewed-by: Jorgen Hansen <jhansen@vmware.com>
> > > Reviewed-by: George Zhang <georgezhang@vmware.com>
> > > Reviewed-by: Aditya Sarwade <asarwade@vmware.com>
> > > Reviewed-by: Bryan Tan <bryantan@vmware.com>
> > > Signed-off-by: Adit Ranadive <aditr@vmware.com>
> > > ---
> > > Changes v4->v5:
> > >  - Added pvrdma files to common UAPI folder.
> > > ---
> > >  MAINTAINERS | 9 +++++++++
> > >  1 file changed, 9 insertions(+)
> > >
> > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > index 87e23cd..5023dc0 100644
> > > --- a/MAINTAINERS
> > > +++ b/MAINTAINERS
> > > @@ -12615,6 +12615,15 @@ S:	Maintained
> > >  F:	drivers/scsi/vmw_pvscsi.c
> > >  F:	drivers/scsi/vmw_pvscsi.h
> > >
> > > +VMWARE PVRDMA DRIVER
> > > +M:	Adit Ranadive <aditr@vmware.com>
> > > +M:	VMware PV-Drivers <pv-drivers@vmware.com>
> > > +L:	linux-rdma@vger.kernel.org
> > > +S:	Maintained
> > > +F:	drivers/infiniband/hw/pvrdma/
> > > +F:	include/uapi/rdma/pvrdma-abi.h
> > > +F:	include/uapi/rdma/pvrdma-uapi.h
> >
> > Please remove the last two lines, these files will be maintained by
> > Doug.
> >
> > Thanks
> >
>
> Ok. Based on your recent patch series on export vendor specific ABIs,
> the ABI files were added to be maintained [1] by individual driver owners.
> Is that not the case now?

I based my answer on this response from Doug [1], maybe I'm wrong.

[1] http://marc.info/?l=linux-rdma&m=147464894811998&w=2

>
> [1] http://marc.info/?l=linux-rdma&m=147455473718235&w=2

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* Re: [PATCH TRIVIAL infiniband-diags] ibportstate.c: When not switch, port can be router port
From: ira.weiny @ 2016-09-26  5:47 UTC (permalink / raw)
  To: Hal Rosenstock; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <f44f2a4f-15f7-6ae5-d1fb-7af0a77aeaee-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org>

On Fri, Sep 23, 2016 at 09:17:10AM -0400, Hal Rosenstock wrote:
> 
> Indicate PortInfo is on CA/RT port rather than just CA port when not
> on Switch port. RT is what's used by ibnetdiscover for router nodes.
> 
> Signed-off-by: Hal Rosenstock <hal-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

Thanks Applied,
Ira

> ---
> diff --git a/src/ibportstate.c b/src/ibportstate.c
> index cb47aa9..cfb8be7 100644
> --- a/src/ibportstate.c
> +++ b/src/ibportstate.c
> @@ -522,9 +522,9 @@ int main(int argc, char **argv)
>  		IBEXIT("Can't set M_Key fields on switch port != 0");
>  
>  	if (port_op != QUERY || changed)
> -		printf("Initial %s PortInfo:\n", is_switch ? "Switch" : "CA");
> +		printf("Initial %s PortInfo:\n", is_switch ? "Switch" : "CA/RT");
>  	else
> -		printf("%s PortInfo:\n", is_switch ? "Switch" : "CA");
> +		printf("%s PortInfo:\n", is_switch ? "Switch" : "CA/RT");
>  	espeed_cap = get_port_info(&portid, data, portnum, is_switch);
>  	show_port_info(&portid, data, portnum, espeed_cap, is_switch);
>  	if (is_mlnx_ext_port_info_supported(devid)) {
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH infiniband-diags] ibportstate: Fixed peer port probing when using DR routing
From: ira.weiny @ 2016-09-26  5:34 UTC (permalink / raw)
  To: Hal Rosenstock
  Cc: Knut Omang, linux-rdma-u79uwXL29TY76Z2rM5mHXA, Line Holen,
	Dag Moxnes
In-Reply-To: <01da2b4d-270a-f8c9-3d5f-ec7a4c0b7d22-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org>

On Fri, Sep 23, 2016 at 09:06:45AM -0400, Hal Rosenstock wrote:
> On 9/23/2016 2:42 AM, Knut Omang wrote:
> > From: Dag Moxnes <dag.moxnes-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> 
> Title might be better as:
> ibportstate: Fixed switch peer port probing when using DR routing

Agreed.

> 
> > 
> > When querying a remote port 
> 
> on a switch
> 
> > using ibportportstate,
> 
> Typo: ibportstate
> 
> > queries to the peer port
> > would result in timeouts. The reason for this is that the DR path (or partial
> > DR path was not correctly constructed.
> 
> It looks like there are 2 cases fixed below and it's best to describe
> them. My take on it is:
> 1. local LID (for switch port 0) is not yet configured
> 2. SMI requester port (I think this is what you term remote port above)
> is on local rather than remote switch
> Is that accurate ?
> 
> > Signed-off-by: Dag Moxnes <dag.moxnes-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> > Reviewed-by: Line Holen <line.holen-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> > Signed-off-by: Knut Omang <knut.omang-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> > ---
> >  src/ibportstate.c | 36 +++++++++++++++++++++++++++---------
> >  1 file changed, 27 insertions(+), 9 deletions(-)
> > 
> > diff --git a/src/ibportstate.c b/src/ibportstate.c
> > index cb47aa9..cd0f680 100644
> > --- a/src/ibportstate.c
> > +++ b/src/ibportstate.c
> > @@ -1,6 +1,8 @@
> >  /*
> >   * Copyright (c) 2004-2009 Voltaire Inc.  All rights reserved.
> >   * Copyright (c) 2010,2011 Mellanox Technologies LTD.  All rights reserved.
> > + * Copyright (c) 2011, 2015, Oracle and/or its affiliates. All rights reserved.
> > + *
> >   *
> >   * This software is available to you under a choice of one of two
> >   * licenses.  You may choose to be licensed under the terms of the GNU
> > @@ -655,15 +657,31 @@ int main(int argc, char **argv)
> >  
> >  			/* Setup portid for peer port */
> >  			memcpy(&peerportid, &portid, sizeof(peerportid));
> > -			peerportid.drpath.cnt = 1;
> > -			peerportid.drpath.p[1] = (uint8_t) portnum;
> > -
> > -			/* Set DrSLID to local lid */
> > -			if (resolve_self(ibd_ca, ibd_ca_port, &selfportid,
> > -						&selfport, 0) < 0)
> > -				IBEXIT("could not resolve self");
> > -			peerportid.drpath.drslid = (uint16_t) selfportid.lid;
> > -			peerportid.drpath.drdlid = 0xffff;
> > +			if (portid.lid == 0) {
> > +				peerportid.drpath.cnt = portid.drpath.cnt + 1;

Nit: peerportid.drpath.cnt++;

Ira

> > +				if (peerportid.drpath.cnt == IB_SUBNET_PATH_HOPS_MAX) {
> > +					exit(0);
> 
> Nit: IBEXIT("Too many hops") rather than exit(0)
> 
> > +				}
> > +				peerportid.drpath.p[peerportid.drpath.cnt] = (uint8_t) portnum;
> > +			} else {
> > +				/* Set DrSLID to local lid */
> > +				if (resolve_portid_str(ibd_ca, ibd_ca_port, &portid, argv[0],
> > +						       ibd_dest_type, ibd_sm_id, srcport) < 0)
> > +					IBEXIT("could not resolve self");
> > +
> > +				peerportid.drpath.cnt = 1;
> > +				peerportid.drpath.p[1] = (uint8_t) portnum;
> > +
> > +
> 
> Nit: Eliminate extra blank line
> 
> > +				/* Use partial LID routing if remote switch */
> > +				if ((portid.lid == selfportid.lid)) {
> > +					peerportid.drpath.drslid = 0xffff;
> > +					peerportid.drpath.drdlid = 0xffff;
> > +				} else {
> > +					peerportid.drpath.drslid = selfportid.lid;
> > +					peerportid.drpath.drdlid = 0xffff;
> > +				}
> 
> The following can be factored out of the above if/else:
> 				peerportid.drpath.drdlid = 0xffff;
> 
> > +			}
> >  
> >  			/* Get peer port NodeInfo to obtain peer port number */
> >  			is_peer_switch = get_node_info(&peerportid, data);
> > 
> > base-commit: 2937cf99350a2e423b705e8b8dd10499796a7b41
> > 
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v5 00/16] Add Paravirtual RDMA Driver
From: Adit Ranadive @ 2016-09-26  5:25 UTC (permalink / raw)
  To: Leon Romanovsky
  Cc: dledford, linux-rdma, pv-drivers, netdev, linux-pci, jhansen,
	asarwade, georgezhang, bryantan
In-Reply-To: <20160925070352.GU4088@leon.nu>

On Sun, Sep 25 2016 at 10:03:52AM +0300, Leon Romanovsky wrote:
> On Sat, Sep 24, 2016 at 04:21:24PM -0700, Adit Ranadive wrote:
> 
> <...>
> 
> >  include/uapi/rdma/pvrdma-abi.h                 |   99 ++
> >  include/uapi/rdma/pvrdma-uapi.h                |  255 +++++
> 
> As Jason said, you need a very good reason to split and create number of
> files per-driver in UAPI folder.

I can move the pvrdma-uapi.h back to the pvrdma driver folder.

^ permalink raw reply

* Re: [PATCH v5 16/16] MAINTAINERS: Update for PVRDMA driver
From: Adit Ranadive @ 2016-09-26  5:22 UTC (permalink / raw)
  To: Leon Romanovsky
  Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA,
	pv-drivers-pghWNbHTmq7QT0dZR+AlfA, netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-pci-u79uwXL29TY76Z2rM5mHXA, jhansen-pghWNbHTmq7QT0dZR+AlfA,
	asarwade-pghWNbHTmq7QT0dZR+AlfA,
	georgezhang-pghWNbHTmq7QT0dZR+AlfA,
	bryantan-pghWNbHTmq7QT0dZR+AlfA
In-Reply-To: <20160925073010.GW4088-2ukJVAZIZ/Y@public.gmane.org>

On Sun, Sep 25 2016 at 10:30:10AM +0300, Leon Romanovsky wrote:
> On Sat, Sep 24, 2016 at 04:21:40PM -0700, Adit Ranadive wrote:
> > Add maintainer info for the PVRDMA driver.
> >
> > Reviewed-by: Jorgen Hansen <jhansen-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>
> > Reviewed-by: George Zhang <georgezhang-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>
> > Reviewed-by: Aditya Sarwade <asarwade-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>
> > Reviewed-by: Bryan Tan <bryantan-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>
> > Signed-off-by: Adit Ranadive <aditr-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>
> > ---
> > Changes v4->v5:
> >  - Added pvrdma files to common UAPI folder.
> > ---
> >  MAINTAINERS | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 87e23cd..5023dc0 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -12615,6 +12615,15 @@ S:	Maintained
> >  F:	drivers/scsi/vmw_pvscsi.c
> >  F:	drivers/scsi/vmw_pvscsi.h
> >
> > +VMWARE PVRDMA DRIVER
> > +M:	Adit Ranadive <aditr-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>
> > +M:	VMware PV-Drivers <pv-drivers-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>
> > +L:	linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > +S:	Maintained
> > +F:	drivers/infiniband/hw/pvrdma/
> > +F:	include/uapi/rdma/pvrdma-abi.h
> > +F:	include/uapi/rdma/pvrdma-uapi.h
> 
> Please remove the last two lines, these files will be maintained by
> Doug.
> 
> Thanks
>

Ok. Based on your recent patch series on export vendor specific ABIs,
the ABI files were added to be maintained [1] by individual driver owners. 
Is that not the case now?

[1] http://marc.info/?l=linux-rdma&m=147455473718235&w=2
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v5 13/16] IB/pvrdma: Add the main driver module for PVRDMA
From: Adit Ranadive @ 2016-09-26  5:10 UTC (permalink / raw)
  To: Leon Romanovsky
  Cc: dledford, linux-rdma, pv-drivers, netdev, linux-pci, jhansen,
	asarwade, georgezhang, bryantan
In-Reply-To: <20160925075703.GX4088@leon.nu>

On sun, Sep 25 2016 at 10:57:03AM +0300, Leon Romanovsky wrote:
> On Sat, Sep 24, 2016 at 04:21:37PM -0700, Adit Ranadive wrote:
> > This patch adds the support to register a RDMA device with the kernel RDMA
> > stack as well as a kernel module. This also initializes the underlying
> > virtual PCI device.
> >
> > Reviewed-by: Yuval Shaia <yuval.shaia@oracle.com>
> > Reviewed-by: Jorgen Hansen <jhansen@vmware.com>
> > Reviewed-by: George Zhang <georgezhang@vmware.com>
> > Reviewed-by: Aditya Sarwade <asarwade@vmware.com>
> > Reviewed-by: Bryan Tan <bryantan@vmware.com>
> > Signed-off-by: Adit Ranadive <aditr@vmware.com>

<...>

> > +
> > +#include <linux/errno.h>
> > +#include <linux/inetdevice.h>
> > +#include <linux/init.h>
> > +#include <linux/module.h>
> > +#include <linux/slab.h>
> > +#include <rdma/ib_addr.h>
> > +#include <rdma/ib_smi.h>
> > +#include <rdma/ib_user_verbs.h>
> > +#include <net/addrconf.h>
> > +#include <rdma/pvrdma-abi.h>
> > +
> > +#include "pvrdma.h"
> > +
> > +#define DRV_NAME	"pvrdma"
> > +#define DRV_VERSION	"1.0"
> > +#define DRV_RELDATE	"January 1, 2013"
> > +
> > +static const char pvrdma_version[] =
> > +	DRV_NAME ": PVRDMA InfiniBand driver v"
> > +	DRV_VERSION " (" DRV_RELDATE ")\n";
> 
> This is a good example why driver version and reldate are useless in
> kernel. We are in 2016 and not in 2013. All these data are forgotten to
> update right after the developer copied it from other pre-historic driver
> (very common practice in netdev). It is worth to stop to use this totally
> useless defines.
> 

Sorry, in our case it was the Mellanox mlx4 driver :). Yeah, I can remove
DRV_VERSION and DRV_RELDATE. I'll keep the MODULE_VERSION since we keep
track of our virtual driver versions that way. So when there are any changes
to our driver we typically bump up the version. Not sure if that's a netdev
or misc device practice and if thats followed here.

<...>

> > +static void pvrdma_get_fw_ver_str(struct ib_device *device, char *str,
> > +				  size_t str_len)
> > +{
> > +	struct pvrdma_dev *dev =
> > +		container_of(device, struct pvrdma_dev, ib_dev);
> > +	snprintf(str, str_len, "%d.%d.%d\n",
> > +		 (int) (dev->dsr->caps.fw_ver >> 32),
> > +		 (int) (dev->dsr->caps.fw_ver >> 16) & 0xffff,
> > +		 (int) dev->dsr->caps.fw_ver & 0xffff);
> > +}
> 
> Yuval already pointed it to you.
> Thanks to Ira, we have general function in core to print FW version.
> Please use them.
> 

If you are referring to commit 5fa76c20458518ed6181adddef2e31c5afc0745c
then isnt this is exactly what Ira did for the other providers?
Here, the pvrdma_get_fw_ver_str function is registered as a callback for
the get_dev_fw_str API. Please our pvrdma_register_device function where
we register our callbacks with ib_core:
https://patchwork.kernel.org/patch/9349357/

> > +	dev->ib_dev.get_dev_fw_str = pvrdma_get_fw_ver_str;

^ permalink raw reply

* Re: [infiniband-diags PATCH 4/4] A few typo fixes in the manpages
From: ira.weiny @ 2016-09-26  4:56 UTC (permalink / raw)
  To: Ana Guerrero López; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20160905214910.GA9795-2v28OwtJ5VXkvb5ro2YpKQ@public.gmane.org>

On Mon, Sep 05, 2016 at 11:49:10PM +0200, Ana Guerrero López wrote:
> They have been detected by lintian, the Debian package checker
> 
> Signed-off-by: Ana Guerrero López <ana-kLLpUjk7vcnYtjvyW6yDsg@public.gmane.org>

Thanks Applied,
Ira

> ---
>  doc/rst/iblinkinfo.8.in.rst       | 4 ++--
>  doc/rst/ibqueryerrors.8.in.rst    | 2 +-
>  doc/rst/infiniband-diags.8.in.rst | 4 ++--
>  doc/rst/rdma-ndd.8.in.rst         | 4 ++--
>  4 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/doc/rst/iblinkinfo.8.in.rst b/doc/rst/iblinkinfo.8.in.rst
> index ee2aefb..5f87f37 100644
> --- a/doc/rst/iblinkinfo.8.in.rst
> +++ b/doc/rst/iblinkinfo.8.in.rst
> @@ -57,7 +57,7 @@ The node to start a partial scan can be specified with the following addresses.
>  
>  **--switch, -S <port_guid>** same as "-G". (provided only for backward compatibility)
>  
> -How much of the scan to be printed can be controled with the following.
> +How much of the scan to be printed can be controlled with the following.
>  
>  **--all, -a**
>  Print all nodes found in a partial fabric scan.  Normally a
> @@ -81,7 +81,7 @@ Filter downports indicated in a ibnetdiscover cache.  If a port was previously
>  indicated as down in the specified cache, and is still down, do not output it in the
>  resulting output.  This option may be particularly useful for environments
>  where switches are not fully populated, thus much of the default iblinkinfo
> -info is considered unuseful.  See **ibnetdiscover** for information on caching
> +info is considered useless.  See **ibnetdiscover** for information on caching
>  ibnetdiscover output.
>  
>  
> diff --git a/doc/rst/ibqueryerrors.8.in.rst b/doc/rst/ibqueryerrors.8.in.rst
> index df2901c..0a7992b 100644
> --- a/doc/rst/ibqueryerrors.8.in.rst
> +++ b/doc/rst/ibqueryerrors.8.in.rst
> @@ -63,7 +63,7 @@ using a QoS aware routing engine as it can cause a credit deadlock.
>  
>  **CAUTION** clearing data or error counters will occur regardless of if they
>  are printed or not.  See **--counters** and **--data** for details on
> -controling which counters are printed.
> +controlling which counters are printed.
>  
>  **--details** include receive error and transmit discard details
>  
> diff --git a/doc/rst/infiniband-diags.8.in.rst b/doc/rst/infiniband-diags.8.in.rst
> index 7776a30..be77ce3 100644
> --- a/doc/rst/infiniband-diags.8.in.rst
> +++ b/doc/rst/infiniband-diags.8.in.rst
> @@ -120,8 +120,8 @@ Switch Forwarding Table info
>  
>  	See: ibtracert, ibroute, dump_lfts, dump_mfts, check_lft_balance, ibfindnodesusing
>  
> -Peformance counters
> --------------------
> +Performance counters
> +--------------------
>  
>  	See: ibqueryerrors, perfquery
>  
> diff --git a/doc/rst/rdma-ndd.8.in.rst b/doc/rst/rdma-ndd.8.in.rst
> index 708a5ab..1d126db 100644
> --- a/doc/rst/rdma-ndd.8.in.rst
> +++ b/doc/rst/rdma-ndd.8.in.rst
> @@ -27,7 +27,7 @@ those changes.
>  DETAILS
>  =======
>  
> -Full operation of this deamon requires kernels which support polling of the
> +Full operation of this daemon requires kernels which support polling of the
>  procfs hostname file as well as libudev.
>  
>  If your system does not support either of these features, the daemon will set
> @@ -37,7 +37,7 @@ the Node Descriptions at start up and then sleep forever.
>  Node Description format
>  -----------------------
>  
> -The deamon uses the nd_format configuration option within the ibdiags.conf
> +The daemon uses the nd_format configuration option within the ibdiags.conf
>  file.  %h and %d can be used as wildcards in that string to specify the dynamic
>  use of <hostname> and <device> respectively.
>  
> -- 
> 2.9.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [infiniband-diags PATCH 3/4] Update the shebang to point to /bin/bash
From: ira.weiny @ 2016-09-26  4:56 UTC (permalink / raw)
  To: Ana Guerrero López; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20160905214902.GA9765-2v28OwtJ5VXkvb5ro2YpKQ@public.gmane.org>

On Mon, Sep 05, 2016 at 11:49:02PM +0200, Ana Guerrero López wrote:
> The script contains a few bashims and it won't run in systems such as Debian
> and Ubuntu where /bin/sh is not a symlink to /bin/bash
> 
> Signed-off-by: Ana Guerrero López <ana-kLLpUjk7vcnYtjvyW6yDsg@public.gmane.org>

Thanks Applied,
Ira

>
> ---
>  doc/generate | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/doc/generate b/doc/generate
> index 816b66f..b1339f8 100755
> --- a/doc/generate
> +++ b/doc/generate
> @@ -1,4 +1,4 @@
> -#!/bin/sh
> +#!/bin/bash
>  
>  docdir=`dirname $0`
>  mode=$1
> -- 
> 2.9.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [infiniband-diags PATCH 2/4] Replace INCLUDES with AM_CPPFLAGS
From: ira.weiny @ 2016-09-26  4:56 UTC (permalink / raw)
  To: Ana Guerrero López; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20160905214852.GA9682-2v28OwtJ5VXkvb5ro2YpKQ@public.gmane.org>

On Mon, Sep 05, 2016 at 11:48:52PM +0200, Ana Guerrero López wrote:
> Fixes the following warnings:
> 
> Makefile.am:4: warning: 'INCLUDES' is the old name for 'AM_CPPFLAGS' (or '*_CPPFLAGS')
> 
> libibnetdisc/Makefile.am:4: warning: 'INCLUDES' is the old name for 'AM_CPPFLAGS' (or '*_CPPFLAGS')
> 
> Signed-off-by: Ana Guerrero López <ana-kLLpUjk7vcnYtjvyW6yDsg@public.gmane.org>

Thanks Applied,
Ira

> ---
>  Makefile.am              | 2 +-
>  libibnetdisc/Makefile.am | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/Makefile.am b/Makefile.am
> index 3fea79d..1973702 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -1,7 +1,7 @@
>  ACLOCAL_AMFLAGS = -I config
>  SUBDIRS = libibnetdisc
>  
> -INCLUDES = -I$(top_builddir)/include/ -I$(srcdir)/include -I$(includedir) \
> +AM_CPPFLAGS = -I$(top_builddir)/include/ -I$(srcdir)/include -I$(includedir) \
>  	-I$(includedir)/infiniband -I$(top_srcdir)/libibnetdisc/include
>  
>  if DEBUG
> diff --git a/libibnetdisc/Makefile.am b/libibnetdisc/Makefile.am
> index 2cfcc33..fee13ca 100644
> --- a/libibnetdisc/Makefile.am
> +++ b/libibnetdisc/Makefile.am
> @@ -1,7 +1,7 @@
>  
>  #SUBDIRS = .
>  
> -INCLUDES = -I$(srcdir)/include -I$(includedir) -I$(includedir)/infiniband
> +AM_CPPFLAGS = -I$(srcdir)/include -I$(includedir) -I$(includedir)/infiniband
>  
>  lib_LTLIBRARIES = libibnetdisc.la
>  sbin_PROGRAMS =
> -- 
> 2.9.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [infiniband-diags PATCH 1/4] Update AM_INIT_AUTOMAKE to use subdir-objects
From: ira.weiny @ 2016-09-26  4:56 UTC (permalink / raw)
  To: Ana Guerrero López; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20160905214844.GA9613-2v28OwtJ5VXkvb5ro2YpKQ@public.gmane.org>

On Mon, Sep 05, 2016 at 11:48:44PM +0200, Ana Guerrero López wrote:
> With this option, the objects are placed into the subdirectory of
> the build directory corresponding to the subdirectory of the source file.
> Fixes the following warning:
> 
> Makefile.am:103: warning: source file 'src/ibdiag_common.c' is in a subdirectory,
> Makefile.am:103: but option 'subdir-objects' is disabled
> automake: warning: possible forward-incompatibility.
> automake: At least a source file is in a subdirectory, but the 'subdir-objects'
> automake: automake option hasn't been enabled.  For now, the corresponding output
> automake: object file(s) will be placed in the top-level directory.  However,
> automake: this behaviour will change in future Automake versions: they will
> automake: unconditionally cause object files to be placed in the same subdirectory
> automake: of the corresponding sources.
> automake: You are advised to start using 'subdir-objects' option throughout your
> automake: project, to avoid future incompatibilities.
> 
> Signed-off-by: Ana Guerrero López <ana-kLLpUjk7vcnYtjvyW6yDsg@public.gmane.org>

Thanks Applied,
Ira

> ---
>  configure.ac | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/configure.ac b/configure.ac
> index 623bc4f..75dbb8d 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -5,7 +5,7 @@ AC_INIT(infiniband-diags, 1.6.7, linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org)
>  AC_CONFIG_AUX_DIR(config)
>  AC_CONFIG_MACRO_DIR(config)
>  AM_CONFIG_HEADER(config.h)
> -AM_INIT_AUTOMAKE([foreign])
> +AM_INIT_AUTOMAKE([subdir-objects])
>  
>  AC_SUBST(RELEASE, ${RELEASE:-unknown})
>  AC_SUBST(TARBALL, ${TARBALL:-${PACKAGE}-${VERSION}.tar.gz})
> -- 
> 2.9.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v5 02/16] IB/pvrdma: Add user-level shared functions
From: Adit Ranadive @ 2016-09-26  4:22 UTC (permalink / raw)
  To: Leon Romanovsky
  Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA,
	pv-drivers-pghWNbHTmq7QT0dZR+AlfA, netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-pci-u79uwXL29TY76Z2rM5mHXA, jhansen-pghWNbHTmq7QT0dZR+AlfA,
	asarwade-pghWNbHTmq7QT0dZR+AlfA,
	georgezhang-pghWNbHTmq7QT0dZR+AlfA,
	bryantan-pghWNbHTmq7QT0dZR+AlfA
In-Reply-To: <20160925072624.GV4088-2ukJVAZIZ/Y@public.gmane.org>

On Sun, Sep 25 2016 at 10:26:24AM +0300, Leon Romanovsky wrote:
> > On Sat, Sep 24, 2016 at 04:21:26PM -0700, Adit Ranadive wrote:
> > We share some common structures with the user-level driver. This patch adds
> > those structures and shared functions to traverse the QP/CQ rings.

<...>

> > +
> > +#include <linux/types.h>
> > +
> > +#define PVRDMA_UVERBS_ABI_VERSION	3
> > +#define PVRDMA_BOARD_ID			1
> > +#define PVRDMA_REV_ID			1
> > 
> > Please don't add defines which you are not using in the library and the
> > two above are not in use.
> >

I'll move these to the pvrdma.h file.

<...>

> > diff --git a/include/uapi/rdma/pvrdma-uapi.h b/include/uapi/rdma/pvrdma-uapi.h
> > new file mode 100644
> > index 0000000..430d8a5

<...>

> > +
> > +#ifndef __PVRDMA_UAPI_H__
> > +#define __PVRDMA_UAPI_H__
> > +
> > +#include <linux/types.h>
> > +
> > +#define PVRDMA_VERSION 17
> > 
> > What do you plan to do with this VERSION?
> > How is it related to ABI?
> >

Not related. This is only for the driver to know which APIs to support.
For example, an older driver would still be able to work with a newer
device. I can move this to pvrdma.h as well.

To be honest, I thought I can move this file into the uapi folder since
the structures here are shared with the user-level library. Based on
your comments in this thread and the other ones, I think it makes sense
to move this file back to the pvrdma driver folder and rename it 
(pvrdma_wqe.h?) to avoid confusion. There might still be some duplicate
code (especially the UAR offsets and WQE structs) here and in our
user-level library.

Let me know if that makes sense.

> > +
> > +#define PVRDMA_UAR_HANDLE_MASK	0x00FFFFFF	/* Bottom 24 bits. */
> > +#define PVRDMA_UAR_QP_OFFSET	0		/* Offset of QP doorbell. */
> > +#define PVRDMA_UAR_QP_SEND	BIT(30)		/* Send bit. */
> > +#define PVRDMA_UAR_QP_RECV	BIT(31)		/* Recv bit. */
> > +#define PVRDMA_UAR_CQ_OFFSET	4		/* Offset of CQ doorbell. */
> > +#define PVRDMA_UAR_CQ_ARM_SOL	BIT(29)		/* Arm solicited bit. */
> > +#define PVRDMA_UAR_CQ_ARM	BIT(30)		/* Arm bit. */
> > +#define PVRDMA_UAR_CQ_POLL	BIT(31)		/* Poll bit. */
> > +#define PVRDMA_INVALID_IDX	-1		/* Invalid index. */
> > 
> > +
> > +/* PVRDMA atomic compare and swap */
> > +struct pvrdma_exp_cmp_swap {
> > 
> > _EXP_ looks very similar to MLNX_OFED naming convention.
> > 

Yes, the operation was based on that. Any concerns? 
I can rename this and the one below.

> > +	__u64 swap_val;
> > +	__u64 compare_val;
> > +	__u64 swap_mask;
> > +	__u64 compare_mask;
> > +};
> > +
> > +/* PVRDMA atomic fetch and add */
> > +struct pvrdma_exp_fetch_add {
> > 
> > The same as above.
> > 
> > +	__u64 add_val;
> > +	__u64 field_boundary;
> > +};
> > +
> > +/* PVRDMA address vector. */
> > +struct pvrdma_av {
> > +	__u32 port_pd;
> > +	__u32 sl_tclass_flowlabel;
> > +	__u8 dgid[16];
> > +	__u8 src_path_bits;
> > +	__u8 gid_index;
> > +	__u8 stat_rate;
> > +	__u8 hop_limit;
> > +	__u8 dmac[6];
> > +	__u8 reserved[6];
> > +};
> > +
> > +/* PVRDMA scatter/gather entry */
> > +struct pvrdma_sge {
> > +	__u64   addr;
> > +	__u32   length;
> > +	__u32   lkey;
> > +};
> > +
> > +/* PVRDMA receive queue work request */
> > +struct pvrdma_rq_wqe_hdr {
> > +	__u64 wr_id;		/* wr id */
> > +	__u32 num_sge;		/* size of s/g array */
> > +	__u32 total_len;	/* reserved */
> > +};
> > +/* Use pvrdma_sge (ib_sge) for receive queue s/g array elements. */
> > +
> > +/* PVRDMA send queue work request */
> > +struct pvrdma_sq_wqe_hdr {
> > +	__u64 wr_id;		/* wr id */
> > +	__u32 num_sge;		/* size of s/g array */
> > +	__u32 total_len;	/* reserved */
> > +	__u32 opcode;		/* operation type */
> > +	__u32 send_flags;	/* wr flags */
> > +	union {
> > +		__u32 imm_data;
> > +		__u32 invalidate_rkey;
> > +	} ex;
> > +	__u32 reserved;
> > +	union {
> > +		struct {
> > +			__u64 remote_addr;
> > +			__u32 rkey;
> > +			__u8 reserved[4];
> > +		} rdma;
> > +		struct {
> > +			__u64 remote_addr;
> > +			__u64 compare_add;
> > +			__u64 swap;
> > +			__u32 rkey;
> > +			__u32 reserved;
> > +		} atomic;
> > +		struct {
> > +			__u64 remote_addr;
> > +			__u32 log_arg_sz;
> > +			__u32 rkey;
> > +			union {
> > +				struct pvrdma_exp_cmp_swap  cmp_swap;
> > +				struct pvrdma_exp_fetch_add fetch_add;
> > +			} wr_data;
> > +		} masked_atomics;
> > +		struct {
> > +			__u64 iova_start;
> > +			__u64 pl_pdir_dma;
> > +			__u32 page_shift;
> > +			__u32 page_list_len;
> > +			__u32 length;
> > +			__u32 access_flags;
> > +			__u32 rkey;
> > +		} fast_reg;
> > +		struct {
> > +			__u32 remote_qpn;
> > +			__u32 remote_qkey;
> > +			struct pvrdma_av av;
> > +		} ud;
> > +	} wr;
> > +};
> > 
> > No, I have half-baked patch series which refactors this structure in kernel.
> > There is no need to put this structure in UAPI.
> >

This is specific to our device.. We do need to enqueue the WQE in this format
for the device to recognize it. This is the same format that the user-level
library will put the WQE in. As I said above, we can move this to the main
pvrdma driver directory if you prefer.

> > +/* Use pvrdma_sge (ib_sge) for send queue s/g array elements. */
> > +
> > +/* Completion queue element. */
> > +struct pvrdma_cqe {
> > +	__u64 wr_id;
> > +	__u64 qp;
> > +	__u32 opcode;
> > +	__u32 status;
> > +	__u32 byte_len;
> > +	__u32 imm_data;
> > +	__u32 src_qp;
> > +	__u32 wc_flags;
> > +	__u32 vendor_err;
> > +	__u16 pkey_index;
> > +	__u16 slid;
> > +	__u8 sl;
> > +	__u8 dlid_path_bits;
> > +	__u8 port_num;
> > +	__u8 smac[6];
> > +	__u8 reserved2[7]; /* Pad to next power of 2 (64). */
> > +};
> > +
> > +struct pvrdma_ring {
> > +	atomic_t prod_tail;	/* Producer tail. */
> > +	atomic_t cons_head;	/* Consumer head. */
> > +};
> > +
> > +struct pvrdma_ring_state {
> > +	struct pvrdma_ring tx;	/* Tx ring. */
> > +	struct pvrdma_ring rx;	/* Rx ring. */
> > +};
> > +
> > +static inline int pvrdma_idx_valid(__u32 idx, __u32 max_elems)
> > +{
> > +	/* Generates fewer instructions than a less-than. */
> > +	return (idx & ~((max_elems << 1) - 1)) == 0;
> > +}
> > +
> > +static inline __s32 pvrdma_idx(atomic_t *var, __u32 max_elems)
> > +{
> > +	const unsigned int idx = atomic_read(var);
> > +
> > +	if (pvrdma_idx_valid(idx, max_elems))
> > +		return idx & (max_elems - 1);
> > +	return PVRDMA_INVALID_IDX;
> > +}
> > +
> > +static inline void pvrdma_idx_ring_inc(atomic_t *var, __u32 max_elems)
> > +{
> > +	__u32 idx = atomic_read(var) + 1;	/* Increment. */
> > 
> > It is definitely different atomic_read than you expect. From my grep
> > searches on my machine, linux kernel doesn't export in standard headers
> > the atomic_* functions and C has their implementation of that functions.
> > 

This would probably change for the user-level library, so no need have this file
in UAPI.

> > +
> > +	idx &= (max_elems << 1) - 1;		/* Modulo size, flip gen. */
> > +	atomic_set(var, idx);
> > +}
> > +
> > +static inline __s32 pvrdma_idx_ring_has_space(const struct pvrdma_ring *r,
> > +					      __u32 max_elems, __u32 *out_tail)
> > +{
> > +	const __u32 tail = atomic_read(&r->prod_tail);
> > +	const __u32 head = atomic_read(&r->cons_head);
> > +
> > +	if (pvrdma_idx_valid(tail, max_elems) &&
> > +	    pvrdma_idx_valid(head, max_elems)) {
> > +		*out_tail = tail & (max_elems - 1);
> > +		return tail != (head ^ max_elems);
> > +	}
> > +	return PVRDMA_INVALID_IDX;
> > +}
> > +
> > +static inline __s32 pvrdma_idx_ring_has_data(const struct pvrdma_ring *r,
> > +					     __u32 max_elems, __u32 *out_head)
> > +{
> > +	const __u32 tail = atomic_read(&r->prod_tail);
> > +	const __u32 head = atomic_read(&r->cons_head);
> > +
> > +	if (pvrdma_idx_valid(tail, max_elems) &&
> > +	    pvrdma_idx_valid(head, max_elems)) {
> > +		*out_head = head & (max_elems - 1);
> > +		return tail != head;
> > +	}
> > +	return PVRDMA_INVALID_IDX;
> > +}
> > +
> > +static inline bool pvrdma_idx_ring_is_valid_idx(const struct pvrdma_ring *r,
> > +						__u32 max_elems, __u32 *idx)
> > +{
> > +	const __u32 tail = atomic_read(&r->prod_tail);
> > +	const __u32 head = atomic_read(&r->cons_head);
> > +
> > +	if (pvrdma_idx_valid(tail, max_elems) &&
> > +	    pvrdma_idx_valid(head, max_elems) &&
> > +	    pvrdma_idx_valid(*idx, max_elems)) {
> > +		if (tail > head && (*idx < tail && *idx >= head))
> > +			return true;
> > +		else if (head > tail && (*idx >= head || *idx < tail))
> > +			return true;
> > +	}
> > +	return false;
> > +}
> > +
> > +#endif /* __PVRDMA_UAPI_H__ */
> > 
> > I suggest completely remove this file from UAPI headers folder.
> > 

I can move this back to the pvrdma driver folder.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH rdma-core 4/5] libocrdma: Move ocrdma's list implementation into common directory
From: Jason Gunthorpe @ 2016-09-25 22:05 UTC (permalink / raw)
  To: Leon Romanovsky
  Cc: Christoph Hellwig, dledford-H+wXaHxf7aLQT0dZR+AlfA,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA, yishaih-VPRAkNaXOzVWk0Htik3J/w
In-Reply-To: <20160925161315.GD4088-2ukJVAZIZ/Y@public.gmane.org>

On Sun, Sep 25, 2016 at 07:13:15PM +0300, Leon Romanovsky wrote:
> On Sun, Sep 25, 2016 at 07:41:21AM -0700, Christoph Hellwig wrote:
> > This implementation was also copy and pasted from the Linux kernel
> > one, including the kernel document comments, and an incorrect license
> > was added to it.  It should be removed just like the Mellanox copy.
> 
> Awesome,
> Any suggestion where can we get list.h compliant with BSD license?
> Is this enough [1] ?
> 
> [1]
> https://github.com/lattera/freebsd/blob/master/sys/ofed/include/linux/list.h

I recommend CCAN's version, it looks like it would be trivial to
convert to.

https://github.com/rustyrussell/ccan/blob/master/ccan/list/list.h

We should also try and use their macros for the other stuff your
migrated, eg their container_of has more static checking, as does the
min/max, and they have a min/max_t implementation.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH rdma-core 4/5] libocrdma: Move ocrdma's list implementation into common directory
From: Bart Van Assche @ 2016-09-25 17:50 UTC (permalink / raw)
  To: Christoph Hellwig, Leon Romanovsky
  Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA,
	jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/,
	yishaih-VPRAkNaXOzVWk0Htik3J/w
In-Reply-To: <20160925162203.GA32434-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>

On 09/25/16 09:22, Christoph Hellwig wrote:
> On Sun, Sep 25, 2016 at 07:13:15PM +0300, Leon Romanovsky wrote:
>> Any suggestion where can we get list.h compliant with BSD license?
>> Is this enough [1] ?
>
> That one looks like someone who knew the Linux one very well spend
> a lot of effort obsfucating it after starting with the Linux version :)
>
> If I were the copyright holder of the file I'd be okay with that given
> that the list implementation is pretty much trivial anyway.
>
> But given that I don't know who owns every little bit of the Linux
> lists.h I'd simply avoid anything looking like it and use something
> like the BSD queue.h instead (which glibc also provides, but in a
> horribly outdated version).

How about using <list> from libstdc++? It is allowed to use libstdc++ 
headers to build non-GPL software. See also 
https://gcc.gnu.org/onlinedocs/libstdc++/manual/license.html for 
libstdc++ license information.

Bart.

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH rdma-core 4/5] libocrdma: Move ocrdma's list implementation into common directory
From: Christoph Hellwig @ 2016-09-25 16:22 UTC (permalink / raw)
  To: Leon Romanovsky
  Cc: Christoph Hellwig, dledford-H+wXaHxf7aLQT0dZR+AlfA,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA,
	jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/,
	yishaih-VPRAkNaXOzVWk0Htik3J/w
In-Reply-To: <20160925161315.GD4088-2ukJVAZIZ/Y@public.gmane.org>

On Sun, Sep 25, 2016 at 07:13:15PM +0300, Leon Romanovsky wrote:
> Any suggestion where can we get list.h compliant with BSD license?
> Is this enough [1] ?

That one looks like someone who knew the Linux one very well spend
a lot of effort obsfucating it after starting with the Linux version :)

If I were the copyright holder of the file I'd be okay with that given
that the list implementation is pretty much trivial anyway.

But given that I don't know who owns every little bit of the Linux
lists.h I'd simply avoid anything looking like it and use something
like the BSD queue.h instead (which glibc also provides, but in a
horribly outdated version).
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH rdma-core 4/5] libocrdma: Move ocrdma's list implementation into common directory
From: Leon Romanovsky @ 2016-09-25 16:13 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA,
	jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/,
	yishaih-VPRAkNaXOzVWk0Htik3J/w
In-Reply-To: <20160925144121.GA12246-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>

[-- Attachment #1: Type: text/plain, Size: 458 bytes --]

On Sun, Sep 25, 2016 at 07:41:21AM -0700, Christoph Hellwig wrote:
> This implementation was also copy and pasted from the Linux kernel
> one, including the kernel document comments, and an incorrect license
> was added to it.  It should be removed just like the Mellanox copy.

Awesome,
Any suggestion where can we get list.h compliant with BSD license?
Is this enough [1] ?

[1]
https://github.com/lattera/freebsd/blob/master/sys/ofed/include/linux/list.h

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* [PATCH 4/4] IB/hfi1: Use EPROM platform configuration read
From: Dennis Dalessandro @ 2016-09-25 14:49 UTC (permalink / raw)
  To: dledford-H+wXaHxf7aLQT0dZR+AlfA
  Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, Easwar Hariharan, Dean Luick
In-Reply-To: <20160925144724.13602.38549.stgit-9QXIwq+3FY+1XWohqUldA0EOCMrvLtNR@public.gmane.org>

From: Dean Luick <dean.luick-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

The driver will now try to read directly from the EPROM as its
first choice for the platform configuration file.

Reviewed-by: Easwar Hariharan <easwar.hariharan-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Dean Luick <dean.luick-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Dennis Dalessandro <dennis.dalessandro-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 drivers/infiniband/hw/hfi1/chip.c     |    9 +++++----
 drivers/infiniband/hw/hfi1/platform.c |   32 +++++++++++++++++++++-----------
 2 files changed, 26 insertions(+), 15 deletions(-)

diff --git a/drivers/infiniband/hw/hfi1/chip.c b/drivers/infiniband/hw/hfi1/chip.c
index 88394cd..9bf5f23 100644
--- a/drivers/infiniband/hw/hfi1/chip.c
+++ b/drivers/infiniband/hw/hfi1/chip.c
@@ -14566,6 +14566,11 @@ struct hfi1_devdata *hfi1_init_dd(struct pci_dev *pdev,
 	if (ret)
 		goto bail_cleanup;
 
+	/* call before get_platform_config(), after init_chip_resources() */
+	ret = eprom_init(dd);
+	if (ret)
+		goto bail_free_rcverr;
+
 	/* Needs to be called before hfi1_firmware_init */
 	get_platform_config(dd);
 
@@ -14686,10 +14691,6 @@ struct hfi1_devdata *hfi1_init_dd(struct pci_dev *pdev,
 	if (ret)
 		goto bail_free_cntrs;
 
-	ret = eprom_init(dd);
-	if (ret)
-		goto bail_free_rcverr;
-
 	goto bail;
 
 bail_free_rcverr:
diff --git a/drivers/infiniband/hw/hfi1/platform.c b/drivers/infiniband/hw/hfi1/platform.c
index 965c8ae..2024331 100644
--- a/drivers/infiniband/hw/hfi1/platform.c
+++ b/drivers/infiniband/hw/hfi1/platform.c
@@ -47,29 +47,39 @@
 
 #include "hfi.h"
 #include "efivar.h"
+#include "eprom.h"
 
 void get_platform_config(struct hfi1_devdata *dd)
 {
 	int ret = 0;
 	unsigned long size = 0;
 	u8 *temp_platform_config = NULL;
+	u32 esize;
+
+	ret = eprom_read_platform_config(dd, (void **)&temp_platform_config,
+					 &esize);
+	if (!ret) {
+		/* success */
+		size = esize;
+		goto success;
+	}
+	/* fail, try EFI variable */
 
 	ret = read_hfi1_efi_var(dd, "configuration", &size,
 				(void **)&temp_platform_config);
-	if (ret) {
-		dd_dev_info(dd,
-			    "%s: Failed to get platform config from UEFI, falling back to request firmware\n",
-			    __func__);
-		/* fall back to request firmware */
-		platform_config_load = 1;
-		goto bail;
-	}
+	if (!ret)
+		goto success;
+
+	dd_dev_info(dd,
+		    "%s: Failed to get platform config from UEFI, falling back to request firmware\n",
+		    __func__);
+	/* fall back to request firmware */
+	platform_config_load = 1;
+	return;
 
+success:
 	dd->platform_config.data = temp_platform_config;
 	dd->platform_config.size = size;
-
-bail:
-	/* exit */;
 }
 
 void free_platform_config(struct hfi1_devdata *dd)

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH 3/4] IB/hfi1: Add ability to read platform config from the EPROM
From: Dennis Dalessandro @ 2016-09-25 14:49 UTC (permalink / raw)
  To: dledford-H+wXaHxf7aLQT0dZR+AlfA
  Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, Easwar Hariharan, Dean Luick
In-Reply-To: <20160925144724.13602.38549.stgit-9QXIwq+3FY+1XWohqUldA0EOCMrvLtNR@public.gmane.org>

From: Dean Luick <dean.luick-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

Add a function to read the platform configuration file from
the EPROM.

Reviewed-by: Easwar Hariharan <easwar.hariharan-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Dean Luick <dean.luick-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Dennis Dalessandro <dennis.dalessandro-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 drivers/infiniband/hw/hfi1/eprom.c |   82 ++++++++++++++++++++++++++++++++++++
 drivers/infiniband/hw/hfi1/eprom.h |    2 +
 2 files changed, 84 insertions(+), 0 deletions(-)

diff --git a/drivers/infiniband/hw/hfi1/eprom.c b/drivers/infiniband/hw/hfi1/eprom.c
index 30ecd94..e70c223 100644
--- a/drivers/infiniband/hw/hfi1/eprom.c
+++ b/drivers/infiniband/hw/hfi1/eprom.c
@@ -203,3 +203,85 @@ int eprom_init(struct hfi1_devdata *dd)
 done_asic:
 	return ret;
 }
+
+/* magic character sequence that trails an image */
+#define IMAGE_TRAIL_MAGIC "egamiAPO"
+
+/*
+ * Read all of partition 1.  The actual file is at the front.  Adjust
+ * the returned size if a trailing image magic is found.
+ */
+static int read_partition_platform_config(struct hfi1_devdata *dd, void **data,
+					  u32 *size)
+{
+	void *buffer;
+	void *p;
+	u32 length;
+	int ret;
+
+	buffer = kmalloc(P1_SIZE, GFP_KERNEL);
+	if (!buffer)
+		return -ENOMEM;
+
+	ret = read_length(dd, P1_START, P1_SIZE, buffer);
+	if (ret) {
+		kfree(buffer);
+		return ret;
+	}
+
+	/* scan for image magic that may trail the actual data */
+	p = strnstr(buffer, IMAGE_TRAIL_MAGIC, P1_SIZE);
+	if (p)
+		length = p - buffer;
+	else
+		length = P1_SIZE;
+
+	*data = buffer;
+	*size = length;
+	return 0;
+}
+
+/*
+ * Read the platform configuration file from the EPROM.
+ *
+ * On success, an allocated buffer containing the data and its size are
+ * returned.  It is up to the caller to free this buffer.
+ *
+ * Return value:
+ *   0	      - success
+ *   -ENXIO   - no EPROM is available
+ *   -EBUSY   - not able to acquire access to the EPROM
+ *   -ENOENT  - no recognizable file written
+ *   -ENOMEM  - buffer could not be allocated
+ */
+int eprom_read_platform_config(struct hfi1_devdata *dd, void **data, u32 *size)
+{
+	u32 directory[EP_PAGE_DWORDS]; /* aligned buffer */
+	int ret;
+
+	if (!dd->eprom_available)
+		return -ENXIO;
+
+	ret = acquire_chip_resource(dd, CR_EPROM, EPROM_TIMEOUT);
+	if (ret)
+		return -EBUSY;
+
+	/* read the last page of P0 for the EPROM format magic */
+	ret = read_length(dd, P1_START - EP_PAGE_SIZE, EP_PAGE_SIZE, directory);
+	if (ret)
+		goto done;
+
+	/* last dword of P0 contains a magic indicator */
+	if (directory[EP_PAGE_DWORDS - 1] == 0) {
+		/* partition format */
+		ret = read_partition_platform_config(dd, data, size);
+		goto done;
+	}
+
+	/* nothing recognized */
+	ret = -ENOENT;
+
+done:
+	release_chip_resource(dd, CR_EPROM);
+	return ret;
+}
diff --git a/drivers/infiniband/hw/hfi1/eprom.h b/drivers/infiniband/hw/hfi1/eprom.h
index b7ac9ce..e774184 100644
--- a/drivers/infiniband/hw/hfi1/eprom.h
+++ b/drivers/infiniband/hw/hfi1/eprom.h
@@ -48,3 +48,5 @@
 struct hfi1_devdata;
 
 int eprom_init(struct hfi1_devdata *dd);
+int eprom_read_platform_config(struct hfi1_devdata *dd, void **buf_ret,
+			       u32 *size_ret);

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH 2/4] IB/hfi1: Change EPROM read for internal use
From: Dennis Dalessandro @ 2016-09-25 14:49 UTC (permalink / raw)
  To: dledford-H+wXaHxf7aLQT0dZR+AlfA
  Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, Easwar Hariharan, Dean Luick
In-Reply-To: <20160925144724.13602.38549.stgit-9QXIwq+3FY+1XWohqUldA0EOCMrvLtNR@public.gmane.org>

From: Dean Luick <dean.luick-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

Change the EPROM read to save to a buffer and allow any offset.
Remove missed includes and left over declarations.

Reviewed-by: Easwar Hariharan <easwar.hariharan-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Dean Luick <dean.luick-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Dennis Dalessandro <dennis.dalessandro-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 drivers/infiniband/hw/hfi1/eprom.c    |   78 ++++++++++++++++++++++++++-------
 drivers/infiniband/hw/hfi1/eprom.h    |    2 -
 drivers/infiniband/hw/hfi1/file_ops.c |    1 
 3 files changed, 61 insertions(+), 20 deletions(-)

diff --git a/drivers/infiniband/hw/hfi1/eprom.c b/drivers/infiniband/hw/hfi1/eprom.c
index 5a11bee..30ecd94 100644
--- a/drivers/infiniband/hw/hfi1/eprom.c
+++ b/drivers/infiniband/hw/hfi1/eprom.c
@@ -62,7 +62,8 @@
 
 /* controller page size, in bytes */
 #define EP_PAGE_SIZE 256
-#define EEP_PAGE_MASK (EP_PAGE_SIZE - 1)
+#define EP_PAGE_MASK (EP_PAGE_SIZE - 1)
+#define EP_PAGE_DWORDS (EP_PAGE_SIZE / sizeof(u32))
 
 /* controller commands */
 #define CMD_SHIFT 24
@@ -89,35 +90,78 @@ static void read_page(struct hfi1_devdata *dd, u32 offset, u32 *result)
 	int i;
 
 	write_csr(dd, ASIC_EEP_ADDR_CMD, CMD_READ_DATA(offset));
-	for (i = 0; i < EP_PAGE_SIZE / sizeof(u32); i++)
+	for (i = 0; i < EP_PAGE_DWORDS; i++)
 		result[i] = (u32)read_csr(dd, ASIC_EEP_DATA);
 	write_csr(dd, ASIC_EEP_ADDR_CMD, CMD_NOP); /* close open page */
 }
 
 /*
- * Read length bytes starting at offset.  Copy to user address addr.
+ * Read length bytes starting at offset from the start of the EPROM.
  */
-static int read_length(struct hfi1_devdata *dd, u32 start, u32 len, u64 addr)
+static int read_length(struct hfi1_devdata *dd, u32 start, u32 len, void *dest)
 {
-	u32 offset;
-	u32 buffer[EP_PAGE_SIZE / sizeof(u32)];
-	int ret = 0;
+	u32 buffer[EP_PAGE_DWORDS];
+	u32 end;
+	u32 start_offset;
+	u32 read_start;
+	u32 bytes;
+
+	if (len == 0)
+		return 0;
 
-	/* reject anything not on an EPROM page boundary */
-	if ((start & EEP_PAGE_MASK) || (len & EEP_PAGE_MASK))
+	end = start + len;
+
+	/*
+	 * Make sure the read range is not outside of the controller read
+	 * command address range.  Note that '>' is correct below - the end
+	 * of the range is OK if it stops at the limit, but no higher.
+	 */
+	if (end > (1 << CMD_SHIFT))
 		return -EINVAL;
 
-	for (offset = 0; offset < len; offset += EP_PAGE_SIZE) {
-		read_page(dd, start + offset, buffer);
-		if (copy_to_user((void __user *)(addr + offset),
-				 buffer, EP_PAGE_SIZE)) {
-			ret = -EFAULT;
-			goto done;
+	/* read the first partial page */
+	start_offset = start & EP_PAGE_MASK;
+	if (start_offset) {
+		/* partial starting page */
+
+		/* align and read the page that contains the start */
+		read_start = start & ~EP_PAGE_MASK;
+		read_page(dd, read_start, buffer);
+
+		/* the rest of the page is available data */
+		bytes = EP_PAGE_SIZE - start_offset;
+
+		if (len <= bytes) {
+			/* end is within this page */
+			memcpy(dest, (u8 *)buffer + start_offset, len);
+			return 0;
 		}
+
+		memcpy(dest, (u8 *)buffer + start_offset, bytes);
+
+		start += bytes;
+		len -= bytes;
+		dest += bytes;
 	}
+	/* start is now page aligned */
 
-done:
-	return ret;
+	/* read whole pages */
+	while (len >= EP_PAGE_SIZE) {
+		read_page(dd, start, buffer);
+		memcpy(dest, buffer, EP_PAGE_SIZE);
+
+		start += EP_PAGE_SIZE;
+		len -= EP_PAGE_SIZE;
+		dest += EP_PAGE_SIZE;
+	}
+
+	/* read the last partial page */
+	if (len) {
+		read_page(dd, start, buffer);
+		memcpy(dest, buffer, len);
+	}
+
+	return 0;
 }
 
 /*
diff --git a/drivers/infiniband/hw/hfi1/eprom.h b/drivers/infiniband/hw/hfi1/eprom.h
index d41f0b1..b7ac9ce 100644
--- a/drivers/infiniband/hw/hfi1/eprom.h
+++ b/drivers/infiniband/hw/hfi1/eprom.h
@@ -45,8 +45,6 @@
  *
  */
 
-struct hfi1_cmd;
 struct hfi1_devdata;
 
 int eprom_init(struct hfi1_devdata *dd);
-int handle_eprom_command(struct file *fp, const struct hfi1_cmd *cmd);
diff --git a/drivers/infiniband/hw/hfi1/file_ops.c b/drivers/infiniband/hw/hfi1/file_ops.c
index 13bb6a5..677efa0 100644
--- a/drivers/infiniband/hw/hfi1/file_ops.c
+++ b/drivers/infiniband/hw/hfi1/file_ops.c
@@ -58,7 +58,6 @@
 #include "trace.h"
 #include "user_sdma.h"
 #include "user_exp_rcv.h"
-#include "eprom.h"
 #include "aspm.h"
 #include "mmu_rb.h"
 

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH 1/4] IB/hfi1: Restore EPROM read ability
From: Dennis Dalessandro @ 2016-09-25 14:49 UTC (permalink / raw)
  To: dledford-H+wXaHxf7aLQT0dZR+AlfA
  Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, Easwar Hariharan, Dean Luick
In-Reply-To: <20160925144724.13602.38549.stgit-9QXIwq+3FY+1XWohqUldA0EOCMrvLtNR@public.gmane.org>

From: Dean Luick <dean.luick-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

Partially revert commit d07903174202 ("IB/hfi1: Remove
EPROM functionality from data device"), bringing back
the ability to read from the EPROM.

This code will be used for driver-only acccess to the EPROM.

Reviewed-by: Easwar Hariharan <easwar.hariharan-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Dean Luick <dean.luick-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Dennis Dalessandro <dennis.dalessandro-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 drivers/infiniband/hw/hfi1/eprom.c |   59 ++++++++++++++++++++++++++++++++++++
 1 files changed, 59 insertions(+), 0 deletions(-)

diff --git a/drivers/infiniband/hw/hfi1/eprom.c b/drivers/infiniband/hw/hfi1/eprom.c
index 36b7794..5a11bee 100644
--- a/drivers/infiniband/hw/hfi1/eprom.c
+++ b/drivers/infiniband/hw/hfi1/eprom.c
@@ -49,7 +49,25 @@
 #include "common.h"
 #include "eprom.h"
 
+/*
+ * The EPROM is logically divided into three partitions:
+ *	partition 0: the first 128K, visible from PCI ROM BAR
+ *	partition 1: 4K config file (sector size)
+ *	partition 2: the rest
+ */
+#define P0_SIZE (128 * 1024)
+#define P1_SIZE   (4 * 1024)
+#define P1_START P0_SIZE
+#define P2_START (P0_SIZE + P1_SIZE)
+
+/* controller page size, in bytes */
+#define EP_PAGE_SIZE 256
+#define EEP_PAGE_MASK (EP_PAGE_SIZE - 1)
+
+/* controller commands */
 #define CMD_SHIFT 24
+#define CMD_NOP			    (0)
+#define CMD_READ_DATA(addr)	    ((0x03 << CMD_SHIFT) | addr)
 #define CMD_RELEASE_POWERDOWN_NOID  ((0xab << CMD_SHIFT))
 
 /* controller interface speeds */
@@ -61,6 +79,47 @@
  * Double it for safety.
  */
 #define EPROM_TIMEOUT 80000 /* ms */
+
+/*
+ * Read a 256 byte (64 dword) EPROM page.
+ * All callers have verified the offset is at a page boundary.
+ */
+static void read_page(struct hfi1_devdata *dd, u32 offset, u32 *result)
+{
+	int i;
+
+	write_csr(dd, ASIC_EEP_ADDR_CMD, CMD_READ_DATA(offset));
+	for (i = 0; i < EP_PAGE_SIZE / sizeof(u32); i++)
+		result[i] = (u32)read_csr(dd, ASIC_EEP_DATA);
+	write_csr(dd, ASIC_EEP_ADDR_CMD, CMD_NOP); /* close open page */
+}
+
+/*
+ * Read length bytes starting at offset.  Copy to user address addr.
+ */
+static int read_length(struct hfi1_devdata *dd, u32 start, u32 len, u64 addr)
+{
+	u32 offset;
+	u32 buffer[EP_PAGE_SIZE / sizeof(u32)];
+	int ret = 0;
+
+	/* reject anything not on an EPROM page boundary */
+	if ((start & EEP_PAGE_MASK) || (len & EEP_PAGE_MASK))
+		return -EINVAL;
+
+	for (offset = 0; offset < len; offset += EP_PAGE_SIZE) {
+		read_page(dd, start + offset, buffer);
+		if (copy_to_user((void __user *)(addr + offset),
+				 buffer, EP_PAGE_SIZE)) {
+			ret = -EFAULT;
+			goto done;
+		}
+	}
+
+done:
+	return ret;
+}
+
 /*
  * Initialize the EPROM handler.
  */

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH 0/4] IB/hfi1: Read platform configuration file from EPROM
From: Dennis Dalessandro @ 2016-09-25 14:49 UTC (permalink / raw)
  To: dledford-H+wXaHxf7aLQT0dZR+AlfA
  Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, Easwar Hariharan, Dean Luick

Doug,

This series adds the capability to read configuration information used to init
the link from the EPROM.

It partially undoes some of the work that was removed when we removed the
user access. This is not done though in support of user access. This is only so
the driver can read the EPROM. All user access is still only through resource0.

Series applies on previously submitted patches and should apply cleanly to your
for-4.9 branch. Can also be seen in GitHub at:
https://github.com/ddalessa/kernel/tree/for-4.9

---

Dean Luick (4):
      IB/hfi1: Restore EPROM read ability
      IB/hfi1: Change EPROM read for internal use
      IB/hfi1: Add ability to read platform config from the EPROM
      IB/hfi1: Use EPROM platform configuration read


 drivers/infiniband/hw/hfi1/chip.c     |    9 +-
 drivers/infiniband/hw/hfi1/eprom.c    |  185 +++++++++++++++++++++++++++++++++
 drivers/infiniband/hw/hfi1/eprom.h    |    4 -
 drivers/infiniband/hw/hfi1/file_ops.c |    1 
 drivers/infiniband/hw/hfi1/platform.c |   32 ++++--
 5 files changed, 213 insertions(+), 18 deletions(-)

--
-Denny
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [PATCH 5/5] IB/hfi1: Document new sysfs entries for hfi1 driver
From: Dennis Dalessandro @ 2016-09-25 14:44 UTC (permalink / raw)
  To: dledford-H+wXaHxf7aLQT0dZR+AlfA
  Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, Sebastian Sanchez,
	Jianxin Xiong, Tadeusz Struk
In-Reply-To: <20160925144248.12088.71489.stgit-9QXIwq+3FY+1XWohqUldA0EOCMrvLtNR@public.gmane.org>

From: Tadeusz Struk <tadeusz.struk-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

This patch adds description for the sdma engine related sysfs entries
for the HFI1 driver.

Reviewed-by: Dennis Dalessandro <dennis.dalessandro-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Reviewed-by: Sebastian Sanchez <sebastian.sanchez-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Reviewed-by: Jianxin Xiong <jianxin.xiong-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Tadeusz Struk <tadeusz.struk-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Dennis Dalessandro <dennis.dalessandro-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 Documentation/infiniband/sysfs.txt |   30 ++++++++++++++++++++++++++++++
 1 files changed, 30 insertions(+), 0 deletions(-)

diff --git a/Documentation/infiniband/sysfs.txt b/Documentation/infiniband/sysfs.txt
index 45bcafe..77570d1 100644
--- a/Documentation/infiniband/sysfs.txt
+++ b/Documentation/infiniband/sysfs.txt
@@ -89,6 +89,36 @@ HFI1
    nctxts - number of allowed contexts (PSM2)
    chip_reset - diagnostic (root only)
    boardversion - board version
+
+   sdma<N>/ - one directory per sdma engine (0 - 15)
+	sdma<N>/cpu_list - read-write, list of cpus for user-process to sdma
+			   engine assignment.
+	sdma<N>/vl - read-only, vl the sdma engine maps to.
+
+	The new interface will give the user control on the affinity settings
+	for the hfi1 device.
+	As an example, to set an sdma engine irq affinity and thread affinity
+	of a user processes to use the sdma engine, which is "near" in terms
+	of NUMA configuration, or physical cpu location, the user will do:
+
+	echo "3" > /proc/irq/<N>/smp_affinity_list
+	echo "4-7" > /sys/devices/.../sdma3/cpu_list
+	cat /sys/devices/.../sdma3/vl
+	0
+	echo "8" > /proc/irq/<M>/smp_affinity_list
+	echo "9-12" > /sys/devices/.../sdma4/cpu_list
+	cat /sys/devices/.../sdma4/vl
+	1
+
+	to make sure that when a process runs on cpus 4,5,6, or 7,
+	and uses vl=0, then sdma engine 3 is selected by the driver,
+	and also the interrupt of the sdma engine 3 is steered to cpu 3.
+	Similarly, when a process runs on cpus 9,10,11, or 12 and sets vl=1,
+	then engine 4 will be selected and the irq of the sdma engine 4 is
+	steered to cpu 8.
+	This assumes that in the above N is the irq number of "sdma3",
+	and M is irq number of "sdma4" in the /proc/interrupts file.
+
    ports/1/
           CCMgtA/
                cc_settings_bin - CCA tables used by PSM2

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH 4/5] IB/hfi1: Add new debugfs sdma_cpu_list file
From: Dennis Dalessandro @ 2016-09-25 14:44 UTC (permalink / raw)
  To: dledford-H+wXaHxf7aLQT0dZR+AlfA
  Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, Sebastian Sanchez,
	Jianxin Xiong, Tadeusz Struk
In-Reply-To: <20160925144248.12088.71489.stgit-9QXIwq+3FY+1XWohqUldA0EOCMrvLtNR@public.gmane.org>

From: Tadeusz Struk <tadeusz.struk-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

Add a debugfs sdma_cpu_list file that can be used to examine the CPU to
sdma engine assignments for the whole device.

Reviewed-by: Dennis Dalessandro <dennis.dalessandro-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Reviewed-by: Sebastian Sanchez <sebastian.sanchez-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Reviewed-by: Jianxin Xiong <jianxin.xiong-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Tadeusz Struk <tadeusz.struk-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Dennis Dalessandro <dennis.dalessandro-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 drivers/infiniband/hw/hfi1/debugfs.c |   38 ++++++++++++++++++++++++++++++
 drivers/infiniband/hw/hfi1/sdma.c    |   43 ++++++++++++++++++++++++++++++++++
 drivers/infiniband/hw/hfi1/sdma.h    |    2 ++
 3 files changed, 83 insertions(+), 0 deletions(-)

diff --git a/drivers/infiniband/hw/hfi1/debugfs.c b/drivers/infiniband/hw/hfi1/debugfs.c
index 5e9be16..632ba21 100644
--- a/drivers/infiniband/hw/hfi1/debugfs.c
+++ b/drivers/infiniband/hw/hfi1/debugfs.c
@@ -933,6 +933,43 @@ static const struct counter_info port_cntr_ops[] = {
 	DEBUGFS_OPS("asic_flags", asic_flags_read, asic_flags_write),
 };
 
+static void *_sdma_cpu_list_seq_start(struct seq_file *s, loff_t *pos)
+{
+	if (*pos >= num_online_cpus())
+		return NULL;
+
+	return pos;
+}
+
+static void *_sdma_cpu_list_seq_next(struct seq_file *s, void *v, loff_t *pos)
+{
+	++*pos;
+	if (*pos >= num_online_cpus())
+		return NULL;
+
+	return pos;
+}
+
+static void _sdma_cpu_list_seq_stop(struct seq_file *s, void *v)
+{
+	/* nothing allocated */
+}
+
+static int _sdma_cpu_list_seq_show(struct seq_file *s, void *v)
+{
+	struct hfi1_ibdev *ibd = (struct hfi1_ibdev *)s->private;
+	struct hfi1_devdata *dd = dd_from_dev(ibd);
+	loff_t *spos = v;
+	loff_t i = *spos;
+
+	sdma_seqfile_dump_cpu_list(s, dd, (unsigned long)i);
+	return 0;
+}
+
+DEBUGFS_SEQ_FILE_OPS(sdma_cpu_list);
+DEBUGFS_SEQ_FILE_OPEN(sdma_cpu_list)
+DEBUGFS_FILE_OPS(sdma_cpu_list);
+
 void hfi1_dbg_ibdev_init(struct hfi1_ibdev *ibd)
 {
 	char name[sizeof("port0counters") + 1];
@@ -961,6 +998,7 @@ void hfi1_dbg_ibdev_init(struct hfi1_ibdev *ibd)
 	DEBUGFS_SEQ_FILE_CREATE(ctx_stats, ibd->hfi1_ibdev_dbg, ibd);
 	DEBUGFS_SEQ_FILE_CREATE(qp_stats, ibd->hfi1_ibdev_dbg, ibd);
 	DEBUGFS_SEQ_FILE_CREATE(sdes, ibd->hfi1_ibdev_dbg, ibd);
+	DEBUGFS_SEQ_FILE_CREATE(sdma_cpu_list, ibd->hfi1_ibdev_dbg, ibd);
 	/* dev counter files */
 	for (i = 0; i < ARRAY_SIZE(cntr_ops); i++)
 		DEBUGFS_FILE_CREATE(cntr_ops[i].name,
diff --git a/drivers/infiniband/hw/hfi1/sdma.c b/drivers/infiniband/hw/hfi1/sdma.c
index 8cfa960..fd39bca 100644
--- a/drivers/infiniband/hw/hfi1/sdma.c
+++ b/drivers/infiniband/hw/hfi1/sdma.c
@@ -1093,6 +1093,49 @@ static void sdma_rht_free(void *ptr, void *arg)
 	kfree(rht_node);
 }
 
+/**
+ * sdma_seqfile_dump_cpu_list() - debugfs dump the cpu to sdma mappings
+ * @s: seq file
+ * @dd: hfi1_devdata
+ * @cpuid: cpu id
+ *
+ * This routine dumps the process to sde mappings per cpu
+ */
+void sdma_seqfile_dump_cpu_list(struct seq_file *s,
+				struct hfi1_devdata *dd,
+				unsigned long cpuid)
+{
+	struct sdma_rht_node *rht_node;
+	int i, j;
+
+	rht_node = rhashtable_lookup_fast(&dd->sdma_rht, &cpuid,
+					  sdma_rht_params);
+	if (!rht_node)
+		return;
+
+	seq_printf(s, "cpu%3lu: ", cpuid);
+	for (i = 0; i < HFI1_MAX_VLS_SUPPORTED; i++) {
+		if (!rht_node->map[i] || !rht_node->map[i]->ctr)
+			continue;
+
+		seq_printf(s, " vl%d: [", i);
+
+		for (j = 0; j < rht_node->map[i]->ctr; j++) {
+			if (!rht_node->map[i]->sde[j])
+				continue;
+
+			if (j > 0)
+				seq_puts(s, ",");
+
+			seq_printf(s, " sdma%2d",
+				   rht_node->map[i]->sde[j]->this_idx);
+		}
+		seq_puts(s, " ]");
+	}
+
+	seq_puts(s, "\n");
+}
+
 /*
  * Free the indicated map struct
  */
diff --git a/drivers/infiniband/hw/hfi1/sdma.h b/drivers/infiniband/hw/hfi1/sdma.h
index 93025f6..56257ea 100644
--- a/drivers/infiniband/hw/hfi1/sdma.h
+++ b/drivers/infiniband/hw/hfi1/sdma.h
@@ -1068,6 +1068,8 @@ ssize_t sdma_set_cpu_to_sde_map(struct sdma_engine *sde, const char *buf,
 				size_t count);
 int sdma_engine_get_vl(struct sdma_engine *sde);
 void sdma_seqfile_dump_sde(struct seq_file *s, struct sdma_engine *);
+void sdma_seqfile_dump_cpu_list(struct seq_file *s, struct hfi1_devdata *dd,
+				unsigned long cpuid);
 
 #ifdef CONFIG_SDMA_VERBOSITY
 void sdma_dumpstate(struct sdma_engine *);

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH 3/5] IB/hfi1: Add irq affinity notification handler
From: Dennis Dalessandro @ 2016-09-25 14:44 UTC (permalink / raw)
  To: dledford-H+wXaHxf7aLQT0dZR+AlfA
  Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, Sebastian Sanchez,
	Jianxin Xiong, Tadeusz Struk
In-Reply-To: <20160925144248.12088.71489.stgit-9QXIwq+3FY+1XWohqUldA0EOCMrvLtNR@public.gmane.org>

From: Tadeusz Struk <tadeusz.struk-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

This patch adds an irq affinity notification handler.
When a user changes interrupt affinity settings for an sdma engine,
the driver needs to make changes to its internal sde structures and
also update the affinity_hint.

Reviewed-by: Dennis Dalessandro <dennis.dalessandro-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Reviewed-by: Sebastian Sanchez <sebastian.sanchez-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Reviewed-by: Jianxin Xiong <jianxin.xiong-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Tadeusz Struk <tadeusz.struk-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Dennis Dalessandro <dennis.dalessandro-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 drivers/infiniband/hw/hfi1/affinity.c |  112 ++++++++++++++++++++++++++++++---
 drivers/infiniband/hw/hfi1/hfi.h      |    1 
 2 files changed, 103 insertions(+), 10 deletions(-)

diff --git a/drivers/infiniband/hw/hfi1/affinity.c b/drivers/infiniband/hw/hfi1/affinity.c
index 17c805a..a26a9a0 100644
--- a/drivers/infiniband/hw/hfi1/affinity.c
+++ b/drivers/infiniband/hw/hfi1/affinity.c
@@ -47,6 +47,7 @@
 #include <linux/topology.h>
 #include <linux/cpumask.h>
 #include <linux/module.h>
+#include <linux/interrupt.h>
 
 #include "hfi.h"
 #include "affinity.h"
@@ -309,6 +310,101 @@ int hfi1_dev_affinity_init(struct hfi1_devdata *dd)
 }
 
 /*
+ * Function updates the irq affinity hint for msix after it has been changed
+ * by the user using the /proc/irq interface. This function only accepts
+ * one cpu in the mask.
+ */
+static void hfi1_update_sdma_affinity(struct hfi1_msix_entry *msix, int cpu)
+{
+	struct sdma_engine *sde = msix->arg;
+	struct hfi1_devdata *dd = sde->dd;
+	struct hfi1_affinity_node *entry;
+	struct cpu_mask_set *set;
+	int i, old_cpu;
+
+	if (cpu > num_online_cpus() || cpu == sde->cpu)
+		return;
+
+	mutex_lock(&node_affinity.lock);
+	entry = node_affinity_lookup(dd->node);
+	if (!entry)
+		goto unlock;
+
+	old_cpu = sde->cpu;
+	sde->cpu = cpu;
+	cpumask_clear(&msix->mask);
+	cpumask_set_cpu(cpu, &msix->mask);
+	dd_dev_dbg(dd, "IRQ vector: %u, type %s engine %u -> cpu: %d\n",
+		   msix->msix.vector, irq_type_names[msix->type],
+		   sde->this_idx, cpu);
+	irq_set_affinity_hint(msix->msix.vector, &msix->mask);
+
+	/*
+	 * Set the new cpu in the hfi1_affinity_node and clean
+	 * the old cpu if it is not used by any other IRQ
+	 */
+	set = &entry->def_intr;
+	cpumask_set_cpu(cpu, &set->mask);
+	cpumask_set_cpu(cpu, &set->used);
+	for (i = 0; i < dd->num_msix_entries; i++) {
+		struct hfi1_msix_entry *other_msix;
+
+		other_msix = &dd->msix_entries[i];
+		if (other_msix->type != IRQ_SDMA || other_msix == msix)
+			continue;
+
+		if (cpumask_test_cpu(old_cpu, &other_msix->mask))
+			goto unlock;
+	}
+	cpumask_clear_cpu(old_cpu, &set->mask);
+	cpumask_clear_cpu(old_cpu, &set->used);
+unlock:
+	mutex_unlock(&node_affinity.lock);
+}
+
+static void hfi1_irq_notifier_notify(struct irq_affinity_notify *notify,
+				     const cpumask_t *mask)
+{
+	int cpu = cpumask_first(mask);
+	struct hfi1_msix_entry *msix = container_of(notify,
+						    struct hfi1_msix_entry,
+						    notify);
+
+	/* Only one CPU configuration supported currently */
+	hfi1_update_sdma_affinity(msix, cpu);
+}
+
+static void hfi1_irq_notifier_release(struct kref *ref)
+{
+	/*
+	 * This is required by affinity notifier. We don't have anything to
+	 * free here.
+	 */
+}
+
+static void hfi1_setup_sdma_notifier(struct hfi1_msix_entry *msix)
+{
+	struct irq_affinity_notify *notify = &msix->notify;
+
+	notify->irq = msix->msix.vector;
+	notify->notify = hfi1_irq_notifier_notify;
+	notify->release = hfi1_irq_notifier_release;
+
+	if (irq_set_affinity_notifier(notify->irq, notify))
+		pr_err("Failed to register sdma irq affinity notifier for irq %d\n",
+		       notify->irq);
+}
+
+static void hfi1_cleanup_sdma_notifier(struct hfi1_msix_entry *msix)
+{
+	struct irq_affinity_notify *notify = &msix->notify;
+
+	if (irq_set_affinity_notifier(notify->irq, NULL))
+		pr_err("Failed to cleanup sdma irq affinity notifier for irq %d\n",
+		       notify->irq);
+}
+
+/*
  * Function sets the irq affinity for msix.
  * It *must* be called with node_affinity.lock held.
  */
@@ -374,22 +470,17 @@ static int get_irq_affinity(struct hfi1_devdata *dd,
 		cpumask_set_cpu(cpu, &set->used);
 	}
 
-	switch (msix->type) {
-	case IRQ_SDMA:
-		sde->cpu = cpu;
-		break;
-	case IRQ_GENERAL:
-	case IRQ_RCVCTXT:
-	case IRQ_OTHER:
-		break;
-	}
-
 	cpumask_set_cpu(cpu, &msix->mask);
 	dd_dev_info(dd, "IRQ vector: %u, type %s %s -> cpu: %d\n",
 		    msix->msix.vector, irq_type_names[msix->type],
 		    extra, cpu);
 	irq_set_affinity_hint(msix->msix.vector, &msix->mask);
 
+	if (msix->type == IRQ_SDMA) {
+		sde->cpu = cpu;
+		hfi1_setup_sdma_notifier(msix);
+	}
+
 	free_cpumask_var(diff);
 	return 0;
 }
@@ -417,6 +508,7 @@ void hfi1_put_irq_affinity(struct hfi1_devdata *dd,
 	switch (msix->type) {
 	case IRQ_SDMA:
 		set = &entry->def_intr;
+		hfi1_cleanup_sdma_notifier(msix);
 		break;
 	case IRQ_GENERAL:
 		/* Don't do accounting for general contexts */
diff --git a/drivers/infiniband/hw/hfi1/hfi.h b/drivers/infiniband/hw/hfi1/hfi.h
index 59f69fa..7eef11b 100644
--- a/drivers/infiniband/hw/hfi1/hfi.h
+++ b/drivers/infiniband/hw/hfi1/hfi.h
@@ -531,6 +531,7 @@ struct hfi1_msix_entry {
 	void *arg;
 	char name[MAX_NAME_SIZE];
 	cpumask_t mask;
+	struct irq_affinity_notify notify;
 };
 
 /* per-SL CCA information */

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH 2/5] IB/hfi1: Add a new VL sysfs attribute for sdma engines
From: Dennis Dalessandro @ 2016-09-25 14:44 UTC (permalink / raw)
  To: dledford-H+wXaHxf7aLQT0dZR+AlfA
  Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, Sebastian Sanchez,
	Jianxin Xiong, Tadeusz Struk
In-Reply-To: <20160925144248.12088.71489.stgit-9QXIwq+3FY+1XWohqUldA0EOCMrvLtNR@public.gmane.org>

From: Tadeusz Struk <tadeusz.struk-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

This patch adds a read-only "VL" attribute for the sysfs entry of each
sdma engine. It will allow the user to check VL to sdma engine mappings.

Reviewed-by: Dennis Dalessandro <dennis.dalessandro-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Reviewed-by: Sebastian Sanchez <sebastian.sanchez-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Reviewed-by: Jianxin Xiong <jianxin.xiong-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Tadeusz Struk <tadeusz.struk-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Dennis Dalessandro <dennis.dalessandro-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 drivers/infiniband/hw/hfi1/sysfs.c |   15 ++++++++++++++-
 1 files changed, 14 insertions(+), 1 deletions(-)

diff --git a/drivers/infiniband/hw/hfi1/sysfs.c b/drivers/infiniband/hw/hfi1/sysfs.c
index 836eea5..edba224 100644
--- a/drivers/infiniband/hw/hfi1/sysfs.c
+++ b/drivers/infiniband/hw/hfi1/sysfs.c
@@ -826,12 +826,25 @@ static ssize_t sde_store_cpu_to_sde_map(struct sdma_engine *sde,
 	return sdma_set_cpu_to_sde_map(sde, buf, count);
 }
 
+static ssize_t sde_show_vl(struct sdma_engine *sde, char *buf)
+{
+	int vl;
+
+	vl = sdma_engine_get_vl(sde);
+	if (vl < 0)
+		return vl;
+
+	return snprintf(buf, PAGE_SIZE, "%d\n", vl);
+}
+
 static SDE_ATTR(cpu_list, S_IWUSR | S_IRUGO,
 		sde_show_cpu_to_sde_map,
 		sde_store_cpu_to_sde_map);
+static SDE_ATTR(vl, S_IRUGO, sde_show_vl, NULL);
 
 static struct sde_attribute *sde_attribs[] = {
-	&sde_attr_cpu_list
+	&sde_attr_cpu_list,
+	&sde_attr_vl
 };
 
 /*

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox