Re: [GIT PULL] NTB bug fixes for v5.11

[Jon Mason]

On Mon, Jan 4, 2021 at 3:31 AM Dan Carpenter  wrote:
>
> On Sun, Dec 27, 2020 at 09:38:23AM -0800, Linus Torvalds wrote:
> > On Sun, Dec 27, 2020 at 6:16 AM Jon Mason  wrote:
> > >
> > > Wang Qing (1):
> > >   ntb: idt: fix error check in ntb_hw_idt.c
> >
> > So this patch seems to be at least partially triggered by a smatch
> > warning that is a bit questionable.
> >
> > This part:
> >
> >  if (IS_ERR_OR_NULL(dbgfs_topdir)) {
> >  dev_info(>ntb.pdev->dev, "Top DebugFS directory absent");
> > -return PTR_ERR(dbgfs_topdir);
> > +return PTR_ERR_OR_ZERO(dbgfs_topdir);
> >  }
> >
> > works, but is very non-optimal and unnecessary.
> >
> > The thing is, "PTR_ERR()" works just fine on a IS_ERR_OR_NULL pointer.
> > It doesn't work on a _regular_ non-NULL and non-ERR pointer, and will
> > return random garbage for those. But if you've tested for
> > IS_ERR_OR_NULL(), then a regular PTR_ERR() is already fine.
> >
> > And PTR_ERR_OR_ZERO() potentially generates an extraneous pointless
> > tests against zero (to check for the ERR case).
> >
> > A compiler may be able to notice that the PTR_ERR_OR_ZERO() is
> > unnecessary and remove it (because of the IS_ERR_OR_NULL() checks),
> > but in general we should assume compilers are "not stupid" rather than
> > "really smart".
> >
> > So while this patch isn't _wrong_, and I've already pulled it, the
> > fact that apparently some smatch test triggers these pointless and
> > potentially expensive patches is not a good idea.
> >
> > I'm not sure what the smatch tests should be (NULL turns to 0, which
> > may be confusing), but I'm cc'ing Dan in case he has ideas.
> >
>
> The most common bug that this check finds is the other part of that same
> commit 91b8246de859 ("ntb: idt: fix error check in ntb_hw_idt.c"):
>
> /* Allocate the memory for IDT NTB device data */
> ndev = idt_create_dev(pdev, id);
> -   if (IS_ERR_OR_NULL(ndev))
> +   if (IS_ERR(ndev))
> return PTR_ERR(ndev);
>
> idt_create_dev() never returns NULL, but if it did then we don't want
> to return success.
>
> For the debugfs stuff, the caller doesn't check the return value anyway.
> Just make it a void function.  A lot of this debugfs code could be
> simplified.  It's not a bug to pass an error pointer or a NULL dbgfs_topdir
> pointer to debugfs_create_file().  There isn't any benefit in checking
> debugfs_initialized().
>
> diff --git a/drivers/ntb/hw/idt/ntb_hw_idt.c b/drivers/ntb/hw/idt/ntb_hw_idt.c
> index e7a4c2aa8baa..710c17b2a923 100644
> --- a/drivers/ntb/hw/idt/ntb_hw_idt.c
> +++ b/drivers/ntb/hw/idt/ntb_hw_idt.c
> @@ -2504,28 +2504,14 @@ static ssize_t idt_dbgfs_info_read(struct file *filp, 
> char __user *ubuf,
>   *
>   * Return: zero on success, otherwise a negative error number.
>   */
> -static int idt_init_dbgfs(struct idt_ntb_dev *ndev)
> +static void idt_init_dbgfs(struct idt_ntb_dev *ndev)
>  {
> char devname[64];
>
> -   /* If the top directory is not created then do nothing */
> -   if (IS_ERR_OR_NULL(dbgfs_topdir)) {
> -   dev_info(>ntb.pdev->dev, "Top DebugFS directory 
> absent");
> -   return PTR_ERR_OR_ZERO(dbgfs_topdir);
> -   }
> -
> /* Create the info file node */
> snprintf(devname, 64, "info:%s", pci_name(ndev->ntb.pdev));
> ndev->dbgfs_info = debugfs_create_file(devname, 0400, dbgfs_topdir,
> -   ndev, _dbgfs_info_ops);
> -   if (IS_ERR(ndev->dbgfs_info)) {
> -   dev_dbg(>ntb.pdev->dev, "Failed to create DebugFS 
> node");
> -   return PTR_ERR(ndev->dbgfs_info);
> -   }
> -
> -   dev_dbg(>ntb.pdev->dev, "NTB device DebugFS node created");
> -
> -   return 0;
> +  ndev, _dbgfs_info_ops);
>  }
>
>  /*
> @@ -2792,7 +2778,7 @@ static int idt_pci_probe(struct pci_dev *pdev,
> goto err_deinit_isr;
>
> /* Initialize DebugFS info node */
> -   (void)idt_init_dbgfs(ndev);
> +   idt_init_dbgfs(ndev);
>
> /* IDT PCIe-switch NTB driver is finally initialized */
> dev_info(>dev, "IDT NTB device is ready");
> @@ -2904,9 +2890,7 @@ static int __init idt_pci_driver_init(void)
>  {
> pr_info("%s %s\n", NTB_DESC, NTB_VER);
>
> -   /* Create the top DebugFS directory if the FS is initialized */
> - 

[GIT PULL] NTB bug fixes for v5.11

[Jon Mason]

Hello Linus,
Here are a few NTB bug fixes for v5.11.  Please consider pulling them.

Thanks,
Jon

The following changes since commit 3650b228f83adda7e5ee532e2b90429c03f7b9ec:

  Linux 5.10-rc1 (2020-10-25 15:14:11 -0700)

are available in the Git repository at:

  git://github.com/jonmason/ntb tags/ntb-5.11

for you to fetch changes up to 75b6f6487cedd0e4c8e07d68b68b8f85cd352bfe:

  ntb: intel: add Intel NTB LTR vendor support for gen4 NTB (2020-12-06 
18:18:03 -0500)


Big fix for IDT NTB and Intel NTB LTR management support


Dave Jiang (1):
  ntb: intel: add Intel NTB LTR vendor support for gen4 NTB

Wang Qing (1):
  ntb: idt: fix error check in ntb_hw_idt.c

 drivers/ntb/hw/idt/ntb_hw_idt.c|  4 ++--
 drivers/ntb/hw/intel/ntb_hw_gen1.h |  1 +
 drivers/ntb/hw/intel/ntb_hw_gen4.c | 27 ++-
 drivers/ntb/hw/intel/ntb_hw_gen4.h | 15 +++
 4 files changed, 44 insertions(+), 3 deletions(-)

Re: [PATCH] ntb: idt: fix error check in ntb_hw_idt.c

[Jon Mason]

On Tue, Nov 10, 2020 at 09:33:40PM +0300, Serge Semin wrote:
> Hello Wang
> 
> On Fri, Nov 06, 2020 at 05:43:31PM +0800, Wang Qing wrote:
> > idt_create_dev never return NULL and fix smatch warning.
> 
> Thanks for submitting this. For the both changes
> Acked-by: Serge Semin 

Applied to the ntb branch.

Thanks,
Jon

> 
> They are mostly unrelated though. If they weren't trivial I'd have
> suggested to split them up into the dedicated patches. Since they
> aren't I suppose leaving the patch 'as is' is ok, unless the subsystem
> maintainer thinks differently.
> 
> -Sergey
> 
> > 
> > Signed-off-by: Wang Qing 
> > ---
> >  drivers/ntb/hw/idt/ntb_hw_idt.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/ntb/hw/idt/ntb_hw_idt.c 
> > b/drivers/ntb/hw/idt/ntb_hw_idt.c
> > index d54261f..e7a4c2a
> > --- a/drivers/ntb/hw/idt/ntb_hw_idt.c
> > +++ b/drivers/ntb/hw/idt/ntb_hw_idt.c
> > @@ -2511,7 +2511,7 @@ static int idt_init_dbgfs(struct idt_ntb_dev *ndev)
> > /* If the top directory is not created then do nothing */
> > if (IS_ERR_OR_NULL(dbgfs_topdir)) {
> > dev_info(>ntb.pdev->dev, "Top DebugFS directory absent");
> > -   return PTR_ERR(dbgfs_topdir);
> > +   return PTR_ERR_OR_ZERO(dbgfs_topdir);
> > }
> >  
> > /* Create the info file node */
> > @@ -2756,7 +2756,7 @@ static int idt_pci_probe(struct pci_dev *pdev,
> >  
> > /* Allocate the memory for IDT NTB device data */
> > ndev = idt_create_dev(pdev, id);
> > -   if (IS_ERR_OR_NULL(ndev))
> > +   if (IS_ERR(ndev))
> > return PTR_ERR(ndev);
> >  
> > /* Initialize the basic PCI subsystem of the device */
> > -- 
> > 2.7.4
> > 

[GIT PULL] NTB bug fixes for v5.10

[Jon Mason]

Hello Linus,
Here are a few NTB bug fixes for v5.10.  Please consider pulling them.

Thanks,
Jon

The following changes since commit d012a7190fc1fd72ed48911e77ca97ba4521bccd:

  Linux 5.9-rc2 (2020-08-23 14:08:43 -0700)

are available in the Git repository at:

  git://github.com/jonmason/ntb tags/ntb-5.10

for you to fetch changes up to b8e2c8bbdf7778c6e3c65db21ababb1dfa794282:

  NTB: Use struct_size() helper in devm_kzalloc() (2020-08-24 10:58:06 -0400)


Bug fixes for v5.10


Dinghao Liu (1):
  ntb: intel: Fix memleak in intel_ntb_pci_probe

Gustavo A. R. Silva (1):
  NTB: Use struct_size() helper in devm_kzalloc()

Kaige Li (1):
  NTB: hw: amd: fix an issue about leak system resources

 drivers/ntb/hw/amd/ntb_hw_amd.c| 1 +
 drivers/ntb/hw/intel/ntb_hw_gen1.c | 2 +-
 drivers/ntb/test/ntb_msi_test.c| 5 +
 3 files changed, 3 insertions(+), 5 deletions(-)

Re: [PATCH][next] NTB: Use struct_size() helper in devm_kzalloc()

[Jon Mason]

On Fri, Jun 19, 2020 at 01:10:55PM -0600, Logan Gunthorpe wrote:
> 
> 
> On 2020-06-19 11:25 a.m., Gustavo A. R. Silva wrote:
> > Make use of the struct_size() helper instead of an open-coded version
> > in order to avoid any potential type mistakes. Also, remove unnecessary
> > variable _struct_size_.
> > 
> > This code was detected with the help of Coccinelle and, audited and
> > fixed manually.
> > 
> > Addresses-KSPP-ID: https://github.com/KSPP/linux/issues/83
> > Signed-off-by: Gustavo A. R. Silva 
> 
> Cool, I didn't know that existed! Thanks!
> 
> Reviewed-by: Logan Gunthorpe 

Added to the ntb-next branch.

Thanks,
Jon


> 
> > ---
> >  drivers/ntb/test/ntb_msi_test.c | 5 +
> >  1 file changed, 1 insertion(+), 4 deletions(-)
> > 
> > diff --git a/drivers/ntb/test/ntb_msi_test.c 
> > b/drivers/ntb/test/ntb_msi_test.c
> > index 99d826ed9c34..7095ecd6223a 100644
> > --- a/drivers/ntb/test/ntb_msi_test.c
> > +++ b/drivers/ntb/test/ntb_msi_test.c
> > @@ -319,7 +319,6 @@ static void ntb_msit_remove_dbgfs(struct ntb_msit_ctx 
> > *nm)
> >  static int ntb_msit_probe(struct ntb_client *client, struct ntb_dev *ntb)
> >  {
> > struct ntb_msit_ctx *nm;
> > -   size_t struct_size;
> > int peers;
> > int ret;
> >  
> > @@ -352,9 +351,7 @@ static int ntb_msit_probe(struct ntb_client *client, 
> > struct ntb_dev *ntb)
> > return ret;
> > }
> >  
> > -   struct_size = sizeof(*nm) + sizeof(*nm->peers) * peers;
> > -
> > -   nm = devm_kzalloc(>dev, struct_size, GFP_KERNEL);
> > +   nm = devm_kzalloc(>dev, struct_size(nm, peers, peers), GFP_KERNEL);
> > if (!nm)
> > return -ENOMEM;
> >  
> > 
> 
> -- 
> You received this message because you are subscribed to the Google Groups 
> "linux-ntb" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to linux-ntb+unsubscr...@googlegroups.com.
> To view this discussion on the web visit 
> https://groups.google.com/d/msgid/linux-ntb/268330b2-e42b-4d62-1ff0-8462d68e498e%40deltatee.com.

Re: [RESEND] NTB: hw: amd: fix an issue about leak system resources

[Jon Mason]

On Tue, Aug 11, 2020 at 09:59:57AM +0800, Kaige Li wrote:
> The related system resources were not released when pci_set_dma_mask(),
> pci_set_consistent_dma_mask(), or pci_iomap() return error in the
> amd_ntb_init_pci() function. Add pci_release_regions() to fix it.
> 
> Signed-off-by: Kaige Li 

Pulled into my ntb branch.  I added:
Fixes: a1b3695820aa ("NTB: Add support for AMD PCI-Express Non-Transparent 
Bridge")

Thanks,
Jon

> ---
> 
> changed commit massage.
> 
>  drivers/ntb/hw/amd/ntb_hw_amd.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/ntb/hw/amd/ntb_hw_amd.c b/drivers/ntb/hw/amd/ntb_hw_amd.c
> index 88e1db6..71428d8 100644
> --- a/drivers/ntb/hw/amd/ntb_hw_amd.c
> +++ b/drivers/ntb/hw/amd/ntb_hw_amd.c
> @@ -1203,6 +1203,7 @@ static int amd_ntb_init_pci(struct amd_ntb_dev *ndev,
>  
>  err_dma_mask:
>   pci_clear_master(pdev);
> + pci_release_regions(pdev);
>  err_pci_regions:
>   pci_disable_device(pdev);
>  err_pci_enable:
> -- 
> 2.1.0
> 

Re: [PATCH] ntb: intel: Fix memleak in intel_ntb_pci_probe

[Jon Mason]

On Mon, Aug 24, 2020 at 07:37:56AM -0700, Dave Jiang wrote:
> 
> 
> On 8/22/2020 11:55 PM, Dinghao Liu wrote:
> > The default error branch of a series of pdev_is_gen calls
> > should free ndev just like what we've done in these calls.
> > 
> > Signed-off-by: Dinghao Liu 
> 
> Thanks Dinghao
> Acked-by: Dave Jiang 

Added to my ntb branch.
I added 
Fixes: 26bfe3d0b227 ("ntb: intel: Add Icelake (gen4) support for Intel NTB")

Thanks,
Jon


> 
> > ---
> >   drivers/ntb/hw/intel/ntb_hw_gen1.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/ntb/hw/intel/ntb_hw_gen1.c 
> > b/drivers/ntb/hw/intel/ntb_hw_gen1.c
> > index 3185efeab487..093dd20057b9 100644
> > --- a/drivers/ntb/hw/intel/ntb_hw_gen1.c
> > +++ b/drivers/ntb/hw/intel/ntb_hw_gen1.c
> > @@ -1893,7 +1893,7 @@ static int intel_ntb_pci_probe(struct pci_dev *pdev,
> > goto err_init_dev;
> > } else {
> > rc = -EINVAL;
> > -   goto err_ndev;
> > +   goto err_init_pci;
> > }
> > ndev_reset_unsafe_flags(ndev);
> > 

[GIT PULL] NTB patches for v5.8

[Jon Mason]

Hello Linus,
Here are a lot of NTB bug fixes and Intel Icelake for v5.8.  Please
consider pulling them.

Thanks,
Jon



The following changes since commit 8f3d9f354286745c751374f5f1fcafee6b3f3136:

  Linux 5.7-rc1 (2020-04-12 12:35:55 -0700)

are available in the Git repository at:

  git://github.com/jonmason/ntb tags/ntb-5.8

for you to fetch changes up to 2130c0ba69d69bb21f5c52787f2587db00d13d8a:

  NTB: ntb_test: Fix bug when counting remote files (2020-06-05 20:02:09 -0400)


Intel Icelake NTB support, Intel driver bug fixes, and lots of bug fixes
for ntb tests


Dave Jiang (3):
  ntb: intel: Add Icelake (gen4) support for Intel NTB
  ntb: intel: add hw workaround for NTB BAR alignment
  ntb: intel: fix static declaration

Jiasen Lin (1):
  NTB: Fix static check warning in perf_clear_test

Logan Gunthorpe (9):
  ntb: hw: remove the code that sets the DMA mask
  NTB: ntb_tool: reading the link file should not end in a NULL byte
  NTB: Revert the change to use the NTB device dev for DMA allocations
  NTB: Fix the default port and peer numbers for legacy drivers
  NTB: ntb_pingpong: Choose doorbells based on port number
  NTB: perf: Don't require one more memory window than number of peers
  NTB: perf: Fix support for hardware that doesn't have port numbers
  NTB: perf: Fix race condition when run with ntb_test
  NTB: ntb_test: Fix bug when counting remote files

Maciej Grochowski (1):
  include/ntb: Fix typo in ntb_unregister_device description

Sanjay R Mehta (4):
  ntb_perf: pass correct struct device to dma_alloc_coherent
  ntb_tool: pass correct struct device to dma_alloc_coherent
  ntb_perf: increase sleep time from one milli sec to one sec
  ntb_perf: avoid false dma unmap of destination address

Wesley Sheng (1):
  NTB: correct ntb_peer_spad_addr and ntb_peer_spad_read comment typos

 drivers/ntb/core.c  |   9 +-
 drivers/ntb/hw/amd/ntb_hw_amd.c |   4 -
 drivers/ntb/hw/idt/ntb_hw_idt.c |   6 -
 drivers/ntb/hw/intel/Makefile   |   2 +-
 drivers/ntb/hw/intel/ntb_hw_gen1.c  |  49 +--
 drivers/ntb/hw/intel/ntb_hw_gen1.h  |   1 +
 drivers/ntb/hw/intel/ntb_hw_gen3.c  |  13 +-
 drivers/ntb/hw/intel/ntb_hw_gen3.h  |   8 +
 drivers/ntb/hw/intel/ntb_hw_gen4.c  | 552 
 drivers/ntb/hw/intel/ntb_hw_gen4.h  | 100 ++
 drivers/ntb/hw/intel/ntb_hw_intel.h |  12 +
 drivers/ntb/test/ntb_perf.c |  49 +--
 drivers/ntb/test/ntb_pingpong.c |  14 +-
 drivers/ntb/test/ntb_tool.c |   9 +-
 include/linux/ntb.h |   6 +-
 tools/testing/selftests/ntb/ntb_test.sh |   2 +-
 16 files changed, 751 insertions(+), 85 deletions(-)
 create mode 100644 drivers/ntb/hw/intel/ntb_hw_gen4.c
 create mode 100644 drivers/ntb/hw/intel/ntb_hw_gen4.h

Re: [PATCH] NTB: correct ntb_peer_spad_addr and ntb_peer_spad_read comment typos

[Jon Mason]

On Tue, May 26, 2020 at 03:59:43PM +0800, Wesley Sheng wrote:
> The comment for ntb_peer_spad_addr and ntb_peer_spad_read
> incorrectly referred to peer doorbell register and local
> scratchpad register.
> 
> Signed-off-by: Wesley Sheng 

Pulled into the ntb branch

Thanks,
Jon

> ---
>  include/linux/ntb.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/ntb.h b/include/linux/ntb.h
> index 8c13538aeffe..b9b0d805d0f9 100644
> --- a/include/linux/ntb.h
> +++ b/include/linux/ntb.h
> @@ -1351,7 +1351,7 @@ static inline int ntb_spad_write(struct ntb_dev *ntb, 
> int sidx, u32 val)
>   * @sidx:Scratchpad index.
>   * @spad_addr:   OUT - The address of the peer scratchpad register.
>   *
> - * Return the address of the peer doorbell register.  This may be used, for
> + * Return the address of the peer scratchpad register.  This may be used, for
>   * example, by drivers that offload memory copy operations to a dma engine.
>   *
>   * Return: Zero on success, otherwise an error number.
> @@ -1373,7 +1373,7 @@ static inline int ntb_peer_spad_addr(struct ntb_dev 
> *ntb, int pidx, int sidx,
>   *
>   * Read the peer scratchpad register, and return the value.
>   *
> - * Return: The value of the local scratchpad register.
> + * Return: The value of the peer scratchpad register.
>   */
>  static inline u32 ntb_peer_spad_read(struct ntb_dev *ntb, int pidx, int sidx)
>  {
> -- 
> 2.16.2
> 

Re: [PATCH v3 0/5] ntb perf, ntb tool and ntb-hw improvements

[Jon Mason]

On Tue, May 05, 2020 at 11:21:47PM -0500, Sanjay R Mehta wrote:
> v3: 
> - Increased ntb_perf command re-try sleep time
> - avoid false dma unmap of dst address.
> 
> v2: Incorporated improvements suggested by Logan Gunthorpe

Pulled into the ntb branch.

Thanks,
Jon

> 
> Links of the review comments for v3:
> 1. https://lkml.org/lkml/2020/3/11/981
> 2. https://lkml.org/lkml/2020/3/10/1827
> 
> Logan Gunthorpe (1):
>   ntb: hw: remove the code that sets the DMA mask
> 
> Sanjay R Mehta (4):
>   ntb_perf: pass correct struct device to dma_alloc_coherent
>   ntb_tool: pass correct struct device to dma_alloc_coherent
>   ntb_perf: increase sleep time from one milli sec to one sec
>   ntb_perf: avoid false dma unmap of destination address
> 
>  drivers/ntb/hw/amd/ntb_hw_amd.c|  4 
>  drivers/ntb/hw/idt/ntb_hw_idt.c|  6 --
>  drivers/ntb/hw/intel/ntb_hw_gen1.c |  4 
>  drivers/ntb/test/ntb_perf.c| 23 ---
>  drivers/ntb/test/ntb_tool.c|  6 +++---
>  5 files changed, 11 insertions(+), 32 deletions(-)
> 
> -- 
> 2.7.4
> 

[GIT PULL] NTB changes for v5.4

[Jon Mason]

Hello Linus,
Here are a few NTB bug fixes and a new AMD device support for v5.4.  Please 
consider pulling them.

Thanks,
Jon



The following changes since commit 4d856f72c10ecb060868ed10ff1b1453943fc6c8:

  Linux 5.3 (2019-09-15 14:19:32 -0700)

are available in the Git repository at:

  git://github.com/jonmason/ntb tags/ntb-5.4

for you to fetch changes up to 4720101fab62d0453babb0287b58a9c5bf78fb80:

  NTB: fix IDT Kconfig typos/spellos (2019-09-23 17:20:40 -0400)


A few bug fixes and support for new AMD NTB Hardware


Alexander Fomichev (1):
  ntb_hw_switchtec: make ntb_mw_set_trans() work when addr == 0

Colin Ian King (1):
  NTB: ntb_transport: remove redundant assignment to rc

Randy Dunlap (1):
  NTB: fix IDT Kconfig typos/spellos

Sanjay R Mehta (3):
  ntb: point to right memory window index
  ntb_hw_amd: Add a new NTB PCI device ID
  ntb_hw_amd: Add memory window support for new AMD hardware

 drivers/ntb/hw/amd/ntb_hw_amd.c| 22 ++
 drivers/ntb/hw/amd/ntb_hw_amd.h|  8 ++--
 drivers/ntb/hw/idt/Kconfig |  6 +++---
 drivers/ntb/hw/mscc/ntb_hw_switchtec.c |  2 +-
 drivers/ntb/ntb_transport.c|  2 +-
 drivers/ntb/test/ntb_perf.c|  2 +-
 6 files changed, 30 insertions(+), 12 deletions(-)

Re: [PATCH] NTB: fix IDT Kconfig typos/spellos

[Jon Mason]

On Wed, Sep 18, 2019 at 1:58 PM Randy Dunlap  wrote:
>
> From: Randy Dunlap 
>
> Fix typos in drivers/ntb/hw/idt/Kconfig.
> Use consistent spelling and capitalization.
>
> Fixes: bf2a952d31d2 ("NTB: Add IDT 89HPESxNTx PCIe-switches support")
> Signed-off-by: Randy Dunlap 
> Cc: Jon Mason 
> Cc: Dave Jiang 
> Cc: Allen Hubbe 
> Cc: Serge Semin 
> Cc: linux-...@googlegroups.com

Pulled into the ntb branch.

Thanks,
Jon

> ---
>  drivers/ntb/hw/idt/Kconfig |6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> --- lnx-53.orig/drivers/ntb/hw/idt/Kconfig
> +++ lnx-53/drivers/ntb/hw/idt/Kconfig
> @@ -4,11 +4,11 @@ config NTB_IDT
> depends on PCI
> select HWMON
> help
> -This driver supports NTB of cappable IDT PCIe-switches.
> +This driver supports NTB of capable IDT PCIe-switches.
>
>  Some of the pre-initializations must be made before IDT PCIe-switch
> -exposes it NT-functions correctly. It should be done by either proper
> -initialisation of EEPROM connected to master smbus of the switch or
> +exposes its NT-functions correctly. It should be done by either 
> proper
> +initialization of EEPROM connected to master SMbus of the switch or
>  by BIOS using slave-SMBus interface changing corresponding registers
>  value. Evidently it must be done before PCI bus enumeration is
>  finished in Linux kernel.
>
>

Re: [PATCH 2/2] ntb_hw_amd: Add memory window support for new AMD hardware

[Jon Mason]

On Sun, Sep 15, 2019 at 10:08 AM Mehta, Sanju  wrote:
>
> From: Sanjay R Mehta 
>
> The AMD new hardware uses BAR23 and BAR45 as memory windows
> as compared to previos where BAR1, BAR23 and BAR45 is used
> for memory windows.
>
> This patch add support for both AMD hardwares.

I pulled both of these patches into the ntb branch.  I am aiming for a
5.4 pull request this Wednesday.  So, please test if possible.

Thanks,
Jon

>
> Signed-off-by: Sanjay R Mehta 
> ---
>  drivers/ntb/hw/amd/ntb_hw_amd.c | 23 ++-
>  drivers/ntb/hw/amd/ntb_hw_amd.h |  7 ++-
>  2 files changed, 24 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/ntb/hw/amd/ntb_hw_amd.c b/drivers/ntb/hw/amd/ntb_hw_amd.c
> index e9286cf..156c2a1 100644
> --- a/drivers/ntb/hw/amd/ntb_hw_amd.c
> +++ b/drivers/ntb/hw/amd/ntb_hw_amd.c
> @@ -78,7 +78,7 @@ static int ndev_mw_to_bar(struct amd_ntb_dev *ndev, int idx)
> if (idx < 0 || idx > ndev->mw_count)
> return -EINVAL;
>
> -   return 1 << idx;
> +   return ndev->dev_data->mw_idx << idx;
>  }
>
>  static int amd_ntb_mw_count(struct ntb_dev *ntb, int pidx)
> @@ -909,7 +909,7 @@ static int amd_init_ntb(struct amd_ntb_dev *ndev)
>  {
> void __iomem *mmio = ndev->self_mmio;
>
> -   ndev->mw_count = AMD_MW_CNT;
> +   ndev->mw_count = ndev->dev_data->mw_count;
> ndev->spad_count = AMD_SPADS_CNT;
> ndev->db_count = AMD_DB_CNT;
>
> @@ -1069,6 +1069,8 @@ static int amd_ntb_pci_probe(struct pci_dev *pdev,
> goto err_ndev;
> }
>
> +   ndev->dev_data = (struct ntb_dev_data *)id->driver_data;
> +
> ndev_init_struct(ndev, pdev);
>
> rc = amd_ntb_init_pci(ndev, pdev);
> @@ -1123,10 +1125,21 @@ static const struct file_operations 
> amd_ntb_debugfs_info = {
> .read = ndev_debugfs_read,
>  };
>
> +static const struct ntb_dev_data dev_data[] = {
> +   { /* for device 145b */
> +   .mw_count = 3,
> +   .mw_idx = 1,
> +   },
> +   { /* for device 148b */
> +   .mw_count = 2,
> +   .mw_idx = 2,
> +   },
> +};
> +
>  static const struct pci_device_id amd_ntb_pci_tbl[] = {
> -   {PCI_VDEVICE(AMD, 0x145b)},
> -   {PCI_VDEVICE(AMD, 0x148b)},
> -   {0}
> +   { PCI_VDEVICE(AMD, 0x145b), (kernel_ulong_t)_data[0] },
> +   { PCI_VDEVICE(AMD, 0x148b), (kernel_ulong_t)_data[1] },
> +   { 0, }
>  };
>  MODULE_DEVICE_TABLE(pci, amd_ntb_pci_tbl);
>
> diff --git a/drivers/ntb/hw/amd/ntb_hw_amd.h b/drivers/ntb/hw/amd/ntb_hw_amd.h
> index 3aac994..139a307 100644
> --- a/drivers/ntb/hw/amd/ntb_hw_amd.h
> +++ b/drivers/ntb/hw/amd/ntb_hw_amd.h
> @@ -92,7 +92,6 @@ static inline void _write64(u64 val, void __iomem *mmio)
>
>  enum {
> /* AMD NTB Capability */
> -   AMD_MW_CNT  = 3,
> AMD_DB_CNT  = 16,
> AMD_MSIX_VECTOR_CNT = 24,
> AMD_SPADS_CNT   = 16,
> @@ -169,6 +168,11 @@ enum {
> AMD_PEER_OFFSET = 0x400,
>  };
>
> +struct ntb_dev_data {
> +   const unsigned char mw_count;
> +   const unsigned int mw_idx;
> +};
> +
>  struct amd_ntb_dev;
>
>  struct amd_ntb_vec {
> @@ -184,6 +188,7 @@ struct amd_ntb_dev {
> u32 cntl_sta;
> u32 peer_sta;
>
> +   struct ntb_dev_data *dev_data;
> unsigned char mw_count;
> unsigned char spad_count;
> unsigned char db_count;
> --
> 2.7.4
>

Re: [PATCH] NTB: ntb_transport: remove redundant assignment to rc

[Jon Mason]

On Sun, Aug 18, 2019 at 7:53 PM Colin King  wrote:
>
> From: Colin Ian King 
>
> Variable rc is initialized to a value that is never read and it
> is re-assigned later. The initialization is redundant and can be
> removed.

Applied to ntb-next, thanks

> Addresses-Coverity: ("Unused value")
> Signed-off-by: Colin Ian King 
> ---
>  drivers/ntb/ntb_transport.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
> index 40c90ca10729..00a5d5764993 100644
> --- a/drivers/ntb/ntb_transport.c
> +++ b/drivers/ntb/ntb_transport.c
> @@ -292,7 +292,7 @@ static int ntb_transport_bus_match(struct device *dev,
>  static int ntb_transport_bus_probe(struct device *dev)
>  {
> const struct ntb_transport_client *client;
> -   int rc = -EINVAL;
> +   int rc;
>
> get_device(dev);
>
> --
> 2.20.1
>

Re: [PATCH] point to right memory window index

[Jon Mason]

On Wed, Apr 10, 2019 at 11:24 AM Mehta, Sanju  wrote:
>
> Hi All,
>
> Any comments on below patch?

I wasn't cc'ed, so this one missed my inbox.  Applied to ntb-next, thanks.

>
> Thanks & Regards,
> Sanjay Mehta
>
> -Original Message-
> From: Mehta, Sanju 
> Sent: Friday, March 29, 2019 5:03 PM
> To: S-k, Shyam-sundar ; jdma...@kudzu.us; 
> dave.ji...@intel.com; alle...@gmail.com
> Cc: linux-...@googlegroups.com; linux-kernel@vger.kernel.org; Mehta, Sanju 
> 
> Subject: [PATCH] point to right memory window index
>
> From: Sanjay R Mehta 
>
> second parameter of ntb_peer_mw_get_addr is pointing to wrong memory window 
> index by passing "peer gidx" instead of "local gidx".
>
> For ex, "local gidx" value is '0' and "peer gidx" value is '1', then
>
> on peer side ntb_mw_set_trans() api is used as below with gidx pointing to 
> local side gidx which is '0', so memroy window '0' is chosen and XLAT '0'
> will be programmed by peer side.
>
> ntb_mw_set_trans(perf->ntb, peer->pidx, peer->gidx, peer->inbuf_xlat,
> peer->inbuf_size);
>
> Now, on local side ntb_peer_mw_get_addr() is been used as below with gidx 
> pointing to "peer gidx" which is '1', so pointing to memory window '1'
> instead of memory window '0'.
>
> ntb_peer_mw_get_addr(perf->ntb,  peer->gidx, _addr,
> >outbuf_size);
>
> So this patch pass "local gidx" as parameter to ntb_peer_mw_get_addr().
>
> Signed-off-by: Sanjay R Mehta 
> ---
>  drivers/ntb/test/ntb_perf.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/ntb/test/ntb_perf.c b/drivers/ntb/test/ntb_perf.c index 
> c7d1a48..08e18d7 100644
> --- a/drivers/ntb/test/ntb_perf.c
> +++ b/drivers/ntb/test/ntb_perf.c
> @@ -1381,7 +1381,7 @@ static int perf_setup_peer_mw(struct perf_peer *peer)
> int ret;
>
> /* Get outbound MW parameters and map it */
> -   ret = ntb_peer_mw_get_addr(perf->ntb, peer->gidx, _addr,
> +   ret = ntb_peer_mw_get_addr(perf->ntb, perf->gidx, _addr,
>>outbuf_size);
> if (ret)
> return ret;
> --
> 2.7.4
>

[GIT PULL] NTB bug fixes for v5.3

[Jon Mason]

Hello Linus,
Here is a trivial NTB bug fix for v5.3.  Please consider pulling it.

Thanks,
Jon



The following changes since commit e21a712a9685488f5ce80495b37b9fdbe96c230d:

  Linux 5.3-rc3 (2019-08-04 18:40:12 -0700)

are available in the Git repository at:

  git://github.com/jonmason/ntb tags/ntb-5.3-bugfixes

for you to fetch changes up to 49da065f7b1f27be625de65d6d55bdd22ac6b5c2:

  NTB/msi: remove incorrect MODULE defines (2019-08-05 15:42:27 -0400)


Bug fix for NTB MSI kernel compile warning


Logan Gunthorpe (1):
  NTB/msi: remove incorrect MODULE defines

 drivers/ntb/msi.c | 5 -
 1 file changed, 5 deletions(-)

[GIT PULL] NTB changes for v5.3

[Jon Mason]

Hello Linus,
Here are the NTB changes for v5.3.  The big change is adding the virtual
MSI interface for NTB (reviewed and acked by Bjorn).  Also, there are
some bug fixes.  Please consider pulling them.

Thanks,
Jon



The following changes since commit a188339ca5a396acc588e5851ed7e19f66b0ebd9:

  Linux 5.2-rc1 (2019-05-19 15:47:09 -0700)

are available in the Git repository at:

  git://github.com/jonmason/ntb tags/ntb-5.3

for you to fetch changes up to d9c53aa440b332059f7f0ce3f7868ff1dc58c62c:

  NTB: Describe the ntb_msi_test client in the documentation. (2019-06-13 
09:03:12 -0400)


New feature to add support for NTB virtual MSI interrupts, the ability
to test and use this feature in the NTB transport layer.  Also, bug
fixes for the AMD and Switchtec drivers, as well as some general
patches.


Dan Carpenter (2):
  ntb_hw_switchtec: potential shift wrapping bug in 
switchtec_ntb_init_sndev()
  NTB: amd: Silence shift wrapping warning in amd_ntb_db_vector_mask()

Joey Zhang (2):
  ntb_hw_switchtec: Remove redundant steps of switchtec_ntb_reinit_peer() 
function
  ntb_hw_switchtec: Fix setup MW with failure bug

Logan Gunthorpe (11):
  NTB: ntb_transport: Ensure qp->tx_mw_dma_addr is initaliazed
  PCI/MSI: Support allocating virtual MSI interrupts
  PCI/switchtec: Add module parameter to request more interrupts
  NTB: Introduce helper functions to calculate logical port number
  NTB: Introduce functions to calculate multi-port resource index
  NTB: Rename ntb.c to support multiple source files in the module
  NTB: Introduce MSI library
  NTB: Introduce NTB MSI Test Client
  NTB: Add ntb_msi_test support to ntb_test
  NTB: Add MSI interrupt support to ntb_transport
  NTB: Describe the ntb_msi_test client in the documentation.

Sanjay R Mehta (4):
  NTB: ntb_perf: Increased the number of message retries to 1000
  NTB: ntb_perf: Disable NTB link after clearing peer XLAT registers
  NTB: ntb_perf: Clear stale values in doorbell and command SPAD register
  NTB: ntb_hw_amd: set peer limit register

Wesley Sheng (2):
  NTB: correct ntb_dev_ops and ntb_dev comment typos
  ntb_hw_switchtec: Skip unnecessary re-setup of shared memory window for 
crosslink case

YueHaibing (1):
  ntb: intel: Make intel_ntb3_peer_db_addr static

 Documentation/ntb.txt   |  27 ++
 drivers/ntb/Kconfig |  11 +
 drivers/ntb/Makefile|   3 +
 drivers/ntb/{ntb.c => core.c}   |   0
 drivers/ntb/hw/amd/ntb_hw_amd.c |  10 +-
 drivers/ntb/hw/intel/ntb_hw_gen3.c  |   6 +-
 drivers/ntb/hw/mscc/ntb_hw_switchtec.c  |  82 +++---
 drivers/ntb/msi.c   | 415 ++
 drivers/ntb/ntb_transport.c | 170 -
 drivers/ntb/test/Kconfig|   9 +
 drivers/ntb/test/Makefile   |   1 +
 drivers/ntb/test/ntb_msi_test.c | 433 
 drivers/ntb/test/ntb_perf.c |  14 +-
 drivers/pci/msi.c   |  54 +++-
 drivers/pci/switch/switchtec.c  |  12 +-
 include/linux/msi.h |   8 +
 include/linux/ntb.h | 200 ++-
 include/linux/pci.h |   9 +
 tools/testing/selftests/ntb/ntb_test.sh |  54 +++-
 19 files changed, 1458 insertions(+), 60 deletions(-)
 rename drivers/ntb/{ntb.c => core.c} (100%)
 create mode 100644 drivers/ntb/msi.c
 create mode 100644 drivers/ntb/test/ntb_msi_test.c

Re: [PATCH 0/3] Redundant steps removal and bug fix of ntb_hw_switchtec

[Jon Mason]

On Thu, Jun 06, 2019 at 03:09:41PM +0800, Kelvin Cao wrote:
> Hi, Everyone,
> 
> This patch series remove redundant steps and fix one bug of the 
> ntb_hw_switchtec module.
> 
> When a re-initialization is caused by a link event, the driver will
> re-setup the shared memory windows. But at that time, the shared memory
> is still valid, and it's unnecessary to free, reallocate and then
> initialize it again. Remove these redundant steps.
> 
> In case of NTB crosslink topology, the setting of shared memory window
> in the virtual partition doesn't reset on peer's reboot. So skip the
> re-setup of shared memory window for that case.
> 
> Switchtec does not support setting multiple MWs simultaneously. However,
> there's a race condition when a re-initialization is caused by a link 
> event, the driver will re-setup the shared memory window asynchronously
> and this races with the client setting up its memory windows on the 
> link up event. Fix this by ensure do the entire initialization in a work
> queue and signal the client once it's done. 
> 
> Regard,
> Kelvin
> 
> --
> 
> Changed since v1:
>   - It's a second resend of v1

Sorry for the delay.  The series is now in the ntb branch.  We've
missed window for 5.2, but it will be in the 5.3 pull request.

Thanks,
Jon

> --
> 
> Joey Zhang (2):
>   ntb_hw_switchtec: Remove redundant steps of
> switchtec_ntb_reinit_peer() function
>   ntb_hw_switchtec: Fix setup MW with failure bug
> 
> Wesley Sheng (1):
>   ntb_hw_switchtec: Skip unnecessary re-setup of shared memory window
> for crosslink case
> 
>  drivers/ntb/hw/mscc/ntb_hw_switchtec.c | 80 
> +-
>  1 file changed, 49 insertions(+), 31 deletions(-)
> 
> -- 
> 2.7.4
> 

Re: [PATCH] NTB: correct ntb_dev_ops and ntb_dev comment typos

[Jon Mason]

On Tue, Apr 30, 2019 at 06:04:29PM +0800, Wesley Sheng wrote:
> The comment for ntb_dev_ops and ntb_dev incorrectly referred to
> ntb_ctx_ops and ntb_device.
> 
> Signed-off-by: Wesley Sheng 
> Reviewed-by: Logan Gunthorpe 

Sorry for the delay.  The series is now in the ntb branch.  We've
missed window for 5.2, but it will be in the 5.3 pull request.

Thanks,
Jon

> ---
>  include/linux/ntb.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/ntb.h b/include/linux/ntb.h
> index 56a92e3..604abc8 100644
> --- a/include/linux/ntb.h
> +++ b/include/linux/ntb.h
> @@ -205,7 +205,7 @@ static inline int ntb_ctx_ops_is_valid(const struct 
> ntb_ctx_ops *ops)
>  }
>  
>  /**
> - * struct ntb_ctx_ops - ntb device operations
> + * struct ntb_dev_ops - ntb device operations
>   * @port_number: See ntb_port_number().
>   * @peer_port_count: See ntb_peer_port_count().
>   * @peer_port_number:See ntb_peer_port_number().
> @@ -404,7 +404,7 @@ struct ntb_client {
>  #define drv_ntb_client(__drv) container_of((__drv), struct ntb_client, drv)
>  
>  /**
> - * struct ntb_device - ntb device
> + * struct ntb_dev - ntb device
>   * @dev: Linux device object.
>   * @pdev:PCI device entry of the ntb.
>   * @topo:Detected topology of the ntb.
> -- 
> 2.7.4
> 

Re: [PATCH v5 00/10] Support using MSI interrupts in ntb_transport

[Jon Mason]

On Thu, May 23, 2019 at 04:30:50PM -0600, Logan Gunthorpe wrote:
> This is another resend as there has been no feedback since v4.
> Seems Jon has been MIA this past cycle so hopefully he appears on the
> list soon.
> 
> I've addressed the feedback so far and rebased on the latest kernel
> and would like this to be considered for merging this cycle.
> 
> The only outstanding issue I know of is that it still will not work
> with IDT hardware, but ntb_transport doesn't work with IDT hardware
> and there is still no sensible common infrastructure to support
> ntb_peer_mw_set_trans(). Thus, I decline to consider that complication
> in this patchset. However, I'll be happy to review work that adds this
> feature in the future.
> 
> Also, as the port number and resource index stuff is a bit complicated,
> I made a quick out of tree test fixture to ensure it's correct[1]. As
> an excerise I also wrote some test code[2] using the upcomming KUnit
> feature.

Sorry for the delay.  The patch is now in the ntb-next branch.  We've
missed window for 5.2, but it will be in the 5.3 pull request (barring
last minute comments).

Thanks,
Jon

> 
> Logan
> 
> [1] https://repl.it/repls/ExcitingPresentFile
> [2] https://github.com/sbates130272/linux-p2pmem/commits/ntb_kunit
> 
> --
> 
> Changes in v5:
> 
> * Rebased onto v5.2-rc1 (plus the patches in ntb-next)
> 
> --
> 
> Changes in v4:
> 
> * Rebased onto v5.1-rc6 (No changes)
> 
> * Numerous grammar and spelling mistakes spotted by Bjorn
> 
> --
> 
> Changes in v3:
> 
> * Rebased onto v5.1-rc1 (Dropped the first two patches as they have
>   been merged, and cleaned up some minor conflicts in the PCI tree)
> 
> * Added a new patch (#3) to calculate logical port numbers that
>   are port numbers from 0 to (number of ports - 1). This is
>   then used in ntb_peer_resource_idx() to fix the issues brought
>   up by Serge.
> 
> * Fixed missing __iomem and iowrite calls (as noticed by Serge)
> 
> * Added patch 10 which describes ntb_msi_test in the documentation
>   file (as requested by Serge)
> 
> * A couple other minor nits and documentation fixes
> 
> --
> 
> Changes in v2:
> 
> * Cleaned up the changes in intel_irq_remapping.c to make them
>   less confusing and add a comment. (Per discussion with Jacob and
>   Joerg)
> 
> * Fixed a nit from Bjorn and collected his Ack
> 
> * Added a Kconfig dependancy on CONFIG_PCI_MSI for CONFIG_NTB_MSI
>   as the Kbuild robot hit a random config that didn't build
>   without it.
> 
> * Worked in a callback for when the MSI descriptor changes so that
>   the clients can resend the new address and data values to the peer.
>   On my test system this was never necessary, but there may be
>   other platforms where this can occur. I tested this by hacking
>   in a path to rewrite the MSI descriptor when I change the cpu
>   affinity of an IRQ. There's a bit of uncertainty over the latency
>   of the change, but without hardware this can acctually occur on
>   we can't test this. This was the result of a discussion with Dave.
> 
> --
> 
> This patch series adds optional support for using MSI interrupts instead
> of NTB doorbells in ntb_transport. This is desirable seeing doorbells on
> current hardware are quite slow and therefore switching to MSI interrupts
> provides a significant performance gain. On switchtec hardware, a simple
> apples-to-apples comparison shows ntb_netdev/iperf numbers going from
> 3.88Gb/s to 14.1Gb/s when switching to MSI interrupts.
> 
> To do this, a couple changes are required outside of the NTB tree:
> 
> 1) The IOMMU must know to accept MSI requests from aliased bused numbers
> seeing NTB hardware typically sends proxied request IDs through
> additional requester IDs. The first patch in this series adds support
> for the Intel IOMMU. A quirk to add these aliases for switchtec hardware
> was already accepted. See commit ad281ecf1c7d ("PCI: Add DMA alias quirk
> for Microsemi Switchtec NTB") for a description of NTB proxy IDs and why
> this is necessary.
> 
> 2) NTB transport (and other clients) may often need more MSI interrupts
> than the NTB hardware actually advertises support for. However, seeing
> these interrupts will not be triggered by the hardware but through an
> NTB memory window, the hardware does not actually need support or need
> to know about them. Therefore we add the concept of Virtual MSI
> interrupts which are allocated just like any other MSI interrupt but
> are not programmed into the hardware's MSI table. This is done in
> Patch 2 and then made use of in Patch 3.
> 
> The remaining patches in this series add a library for dealing with MSI
> interrupts, a test client and finally support in ntb_transport.
> 
> The series is based off of v5.1-rc6 plus the patches in ntb-next.
> A git repo is available here:
> 
> https://github.com/sbates130272/linux-p2pmem/ ntb_transport_msi_v4
> 
> Thanks,
> 
> Logan
> 
> --
> 
> Logan Gunthorpe (10):
>   PCI/MSI: Support allocating virtual MSI interrupts
>   

Re: [PATCH v2] drivers: ntb: Kconfig: pedantic cleanups

[Jon Mason]

On Wed, Mar 20, 2019 at 9:15 PM Bjorn Helgaas  wrote:
>
> [+cc Jon, Dave, Allen]
>
> On Wed, Mar 06, 2019 at 11:02:54PM +0100, Enrico Weigelt, metux IT consult 
> wrote:
> > Formatting of Kconfig files doesn't look so pretty, so just
> > take damp cloth and clean it up.
>
> Oops, I didn't notice that this was a v2.  I first thought this was a
> 2/2 patch.  Sorry for the noise.  Most of my comments still apply
> here, I think.

I 100% agree with the comments form Bjorn.  Though most of the
comments were from previous versions, but if you are going to change
it then might as well make it right.  :)
Assuming you make the changes, I'll pull it in.

Thanks,
Jon

>
> > Signed-off-by: Enrico Weigelt, metux IT consult 
> > ---
> >  drivers/ntb/Kconfig  | 20 ++--
> >  drivers/ntb/hw/amd/Kconfig   |  4 ++--
> >  drivers/ntb/hw/idt/Kconfig   | 41 -
> >  drivers/ntb/hw/intel/Kconfig |  4 ++--
> >  drivers/ntb/hw/mscc/Kconfig  |  8 
> >  drivers/ntb/test/Kconfig | 26 +-
> >  6 files changed, 51 insertions(+), 52 deletions(-)
> >
> > diff --git a/drivers/ntb/Kconfig b/drivers/ntb/Kconfig
> > index 95944e5..5ce3fdd 100644
> > --- a/drivers/ntb/Kconfig
> > +++ b/drivers/ntb/Kconfig
> > @@ -2,13 +2,13 @@ menuconfig NTB
> >   tristate "Non-Transparent Bridge support"
> >   depends on PCI
> >   help
> > -  The PCI-E Non-transparent bridge hardware is a point-to-point PCI-E 
> > bus
> > -  connecting 2 systems.  When configured, writes to the device's PCI
> > -  mapped memory will be mirrored to a buffer on the remote system.  The
> > -  ntb Linux driver uses this point-to-point communication as a method 
> > to
> > -  transfer data from one system to the other.
> > +   The PCI-E Non-transparent bridge hardware is a point-to-point PCI-E 
> > bus
> > +   connecting 2 systems.  When configured, writes to the device's PCI
> > +   mapped memory will be mirrored to a buffer on the remote system.  
> > The
> > +   ntb Linux driver uses this point-to-point communication as a method 
> > to
> > +   transfer data from one system to the other.
> >
> > -  If unsure, say N.
> > +   If unsure, say N.
> >
> >  if NTB
> >
> > @@ -19,10 +19,10 @@ source "drivers/ntb/test/Kconfig"
> >  config NTB_TRANSPORT
> >   tristate "NTB Transport Client"
> >   help
> > -  This is a transport driver that enables connected systems to exchange
> > -  messages over the ntb hardware.  The transport exposes a queue pair 
> > api
> > -  to client drivers.
> > +   This is a transport driver that enables connected systems to 
> > exchange
> > +   messages over the ntb hardware.  The transport exposes a queue pair 
> > api
> > +   to client drivers.
> >
> > -  If unsure, say N.
> > +   If unsure, say N.
> >
> >  endif # NTB
> > diff --git a/drivers/ntb/hw/amd/Kconfig b/drivers/ntb/hw/amd/Kconfig
> > index cfe903c..9a90f17 100644
> > --- a/drivers/ntb/hw/amd/Kconfig
> > +++ b/drivers/ntb/hw/amd/Kconfig
> > @@ -2,6 +2,6 @@ config NTB_AMD
> >   tristate "AMD Non-Transparent Bridge support"
> >   depends on X86_64
> >   help
> > -  This driver supports AMD NTB on capable Zeppelin hardware.
> > +   This driver supports AMD NTB on capable Zeppelin hardware.
> >
> > -  If unsure, say N.
> > +   If unsure, say N.
> > diff --git a/drivers/ntb/hw/idt/Kconfig b/drivers/ntb/hw/idt/Kconfig
> > index f8948cf..5d106ac 100644
> > --- a/drivers/ntb/hw/idt/Kconfig
> > +++ b/drivers/ntb/hw/idt/Kconfig
> > @@ -3,28 +3,27 @@ config NTB_IDT
> >   depends on PCI
> >   select HWMON
> >   help
> > -  This driver supports NTB of cappable IDT PCIe-switches.
> > +   This driver supports NTB of cappable IDT PCIe-switches.
> >
> > -  Some of the pre-initializations must be made before IDT PCIe-switch
> > -  exposes it NT-functions correctly. It should be done by either proper
> > -  initialisation of EEPROM connected to master smbus of the switch or
> > -  by BIOS using slave-SMBus interface changing corresponding registers
> > -  value. Evidently it must be done before PCI bus enumeration is
> > -  finished in Linux kernel.
> > +   Some of the pre-initializations must be made before IDT PCIe-switch
> > +   exposes it NT-functions correctly. It should be done by either 
> > proper
> > +   initialisation of EEPROM connected to master smbus of the switch or
> > +   by BIOS using slave-SMBus interface changing corresponding registers
> > +   value. Evidently it must be done before PCI bus enumeration is
> > +   finished in Linux kernel.
> >
> > -  First of all partitions must be activated and properly assigned to 
> > all
> > -  the ports with NT-functions intended to be activated (see SWPARTxCTL
> > -  and SWPORTxCTL registers). Then all NT-function BARs must be enabled
> > -  with 

Re: linux-next: Fixes tag needs some work in the ntb tree

[Jon Mason]

On Thu, Mar 21, 2019 at 4:28 PM Stephen Rothwell  wrote:
>
> Hi Jon,
>
> On Thu, 21 Mar 2019 15:53:47 -0400 Jon Mason  wrote:
> >
> > Is there an existing set of tools/scripts I should run to prevent
> > things like this from happening?
>
> I have attached the script I run over all new commits submitted for
> linux-next.

Fantastic!  I'll add a variation of this to my git tree and hopefully
prevent any future issues.

Thanks,
Jon

>
> --
> Cheers,
> Stephen Rothwell

Re: linux-next: Fixes tag needs some work in the ntb tree

[Jon Mason]

On Wed, Mar 20, 2019 at 5:00 PM Logan Gunthorpe  wrote:
>
>
>
> On 2019-03-20 2:58 p.m., Stephen Rothwell wrote:
> > Hi all,
> >
> > In commit
> >
> >   f4d0dd1064b8 ("NTB: ntb_transport: Ensure qp->tx_mw_dma_addr is 
> > initaliazed")
> >
> > Fixes tag
> >
> >   Fixes: c27ccb899219 ("NTB: ntb_transport: Ensure the destination buffer 
> > is mapped for TX DMA")
> >
> > has these problem(s):
> >
> >   - Target SHA1 does not exist
> >
> > Did you mean:
> >
> >   c59666bb32b9 ("NTB: ntb_transport: Ensure the destination buffer is 
> > mapped for TX DMA")
>
> My fault. I sent the fix patch with the fixes tag before the original
> patch was merged upstream expecting them to be squashed. c59666bb32b9 is
> the correct hash in upstream now.

I've corrected the patch in my git branches and repushed.

Is there an existing set of tools/scripts I should run to prevent
things like this from happening?

Thanks,
Jon

>
> Logan
>

Re: [PATCH 1/4] NTB: ntb_perf: Increased the number of message retries to 1000

[Jon Mason]

On Fri, Feb 15, 2019 at 09:20:07AM +, Mehta, Sanju wrote:
> From: Sanjay R Mehta 
> 
> while waiting for the peer ntb_perf to initialize scratchpad
> registers, local side ntb_perf  might have already exhausted the
> maximum number of retries which is currently set to 500. To avoid
> this and to give little more time to the peer ntb_perf for scratchpad
> initialization, increased the number of retries to 1000
> 
> Signed-off-by: Sanjay R Mehta 

Series applied to my ntb branch.

Thanks,
Jon


> ---
>  drivers/ntb/test/ntb_perf.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/ntb/test/ntb_perf.c b/drivers/ntb/test/ntb_perf.c
> index 2a9d6b0..a828d0e 100644
> --- a/drivers/ntb/test/ntb_perf.c
> +++ b/drivers/ntb/test/ntb_perf.c
> @@ -100,7 +100,7 @@ MODULE_DESCRIPTION("PCIe NTB Performance Measurement 
> Tool");
>  #define DMA_TRIES100
>  #define DMA_MDELAY   10
>  
> -#define MSG_TRIES500
> +#define MSG_TRIES1000
>  #define MSG_UDELAY_LOW   1000
>  #define MSG_UDELAY_HIGH  2000
>  
> -- 
> 2.7.4
> 
> -- 
> You received this message because you are subscribed to the Google Groups 
> "linux-ntb" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to linux-ntb+unsubscr...@googlegroups.com.
> To post to this group, send email to linux-...@googlegroups.com.
> To view this discussion on the web visit 
> https://groups.google.com/d/msgid/linux-ntb/155079-27216-1-git-send-email-Sanju.Mehta%40amd.com.
> For more options, visit https://groups.google.com/d/optout.

Re: [PATCH -next] ntb: intel: Make intel_ntb3_peer_db_addr static

[Jon Mason]

On Wed, Mar 20, 2019 at 10:08:12PM +0800, Yue Haibing wrote:
> From: YueHaibing 
> 
> Fix sparse warning:
> 
> drivers/ntb/hw/intel/ntb_hw_gen3.c:535:5: warning:
>  symbol 'intel_ntb3_peer_db_addr' was not declared. Should it be static?
> 
> Signed-off-by: YueHaibing 

Looks good to me.  Added to the ntb branch,

Thanks,
Jon

> ---
>  drivers/ntb/hw/intel/ntb_hw_gen3.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/ntb/hw/intel/ntb_hw_gen3.c 
> b/drivers/ntb/hw/intel/ntb_hw_gen3.c
> index f475b56..c339716 100644
> --- a/drivers/ntb/hw/intel/ntb_hw_gen3.c
> +++ b/drivers/ntb/hw/intel/ntb_hw_gen3.c
> @@ -532,9 +532,9 @@ static int intel_ntb3_mw_set_trans(struct ntb_dev *ntb, 
> int pidx, int idx,
>   return 0;
>  }
>  
> -int intel_ntb3_peer_db_addr(struct ntb_dev *ntb, phys_addr_t *db_addr,
> - resource_size_t *db_size,
> - u64 *db_data, int db_bit)
> +static int intel_ntb3_peer_db_addr(struct ntb_dev *ntb, phys_addr_t *db_addr,
> +resource_size_t *db_size,
> +u64 *db_data, int db_bit)
>  {
>   phys_addr_t db_addr_base;
>   struct intel_ntb_dev *ndev = ntb_ndev(ntb);
> -- 
> 2.7.0
> 
> 

[GIT PULL] NTB bug fixes for v5.1

[Jon Mason]

Hello Linus,
Here are a few NTB fixes for v5.1.  While the tag is new, the patches
have been in my tree (and thus linux-next) for weeks now.  Please
consider pulling them.

Thanks,
Jon



The following changes since commit bfeffd155283772bbe78c6a05dec7c0128ee500c:

  Linux 5.0-rc1 (2019-01-06 17:08:20 -0800)

are available in the Git repository at:

  git://github.com/jonmason/ntb tags/ntb-5.1

for you to fetch changes up to ebb09b33c60c46fd4f7ffa0af9e693eebe765d1b:

  NTB: add new parameter to peer_db_addr() db_bit and db_data (2019-02-13 
11:03:18 -0500)


Fixes for switchtec debugability and mapping table entries, NTB
transport improvements, and a reworking of the peer_db_addr to all for
better abstraction


Joey Zhang (1):
  NTB: ntb_transport: Free MWs in ntb_transport_link_cleanup()

Leonid Ravich (1):
  NTB: add new parameter to peer_db_addr() db_bit and db_data

Logan Gunthorpe (1):
  NTB: ntb_transport: Ensure the destination buffer is mapped for TX DMA

Paul Selles (2):
  ntb_hw_switchtec: debug print 64bit aligned crosslink BAR Numbers
  ntb_hw_switchtec: Added support of >=4G memory windows

Wesley Sheng (1):
  ntb_hw_switchtec: NT req id mapping table register entry number should be 
512

 drivers/ntb/hw/intel/ntb_hw_gen1.c | 25 +++--
 drivers/ntb/hw/intel/ntb_hw_gen1.h |  5 +++--
 drivers/ntb/hw/intel/ntb_hw_gen3.c | 33 -
 drivers/ntb/hw/mscc/ntb_hw_switchtec.c | 20 
 drivers/ntb/ntb_transport.c| 31 +--
 include/linux/ntb.h| 10 +++---
 include/linux/switchtec.h  | 10 +++---
 7 files changed, 113 insertions(+), 21 deletions(-)

Re: [PATCH] NTB: ntb_transport: Free MWs in ntb_transport_link_cleanup()

[Jon Mason]

On Mon, Jan 07, 2019 at 06:16:02PM -0700, Logan Gunthorpe wrote:
> 
> 
> On 06/01/19 08:12 PM, Joey Zhang wrote:
> > If NTB peer host crashes or reboots, the NTB transport link will be
> > down and the MWs of NTB transport will be invalid. But the
> > ntb_transport_link_cleanup() does not free these invalid MWs. When
> > the NTB peer host is recovered later, NTB transport link will be
> > up and the ntb_set_mw() will not reset up MWs. Because the MWs of
> > NTB transport are invalid, the NTB transport will not work.
> > 
> > We can fix it by freeing MWs when NTB transport link is down, then
> > the ntb_set_mw() will reset up MWs when NTB transport link is up.
> > 
> > Signed-off-by: Joey Zhang 
> 
> Looks ok to me.

Added to the ntb branch, thanks!

> Reviewed By: Logan Gunthorpe 

Looks like your missed the '-' ;-)
I took the liberity of adding it

Thanks,
Jon

> 
> > ---
> >  drivers/ntb/ntb_transport.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
> > index 3bfdb45..6e8902d 100644
> > --- a/drivers/ntb/ntb_transport.c
> > +++ b/drivers/ntb/ntb_transport.c
> > @@ -862,6 +862,9 @@ static void ntb_transport_link_cleanup(struct 
> > ntb_transport_ctx *nt)
> > if (!nt->link_is_up)
> > cancel_delayed_work_sync(>link_work);
> >  
> > +   for (i = 0; i < nt->mw_count; i++)
> > +   ntb_free_mw(nt, i);
> > +
> > /* The scratchpad registers keep the values if the remote side
> >  * goes down, blast them now to give them a sane value the next
> >  * time they are accessed
> > 

Re: [PATCH] NTB: ntb_transport: Ensure the destination buffer is mapped for TX DMA

[Jon Mason]

On Fri, Jan 18, 2019 at 05:25:20PM -0700, Dave Jiang wrote:
> 
> 
> On 1/18/19 5:10 PM, Logan Gunthorpe wrote:
> > Presently, when ntb_transport is used with DMA and the IOMMU turned on,
> > it fails with errors from the IOMMU such as:
> > 
> >   DMAR: DRHD: handling fault status reg 202
> >   DMAR: [DMA Write] Request device [00:04.0] fault addr
> > 381fc034 [fault reason 05] PTE Write access is not set
> > 
> > This is because ntb_transport does not map the BAR space with the IOMMU.
> > 
> > To fix this, we map the entire MW region for each QP after we assign
> > the DMA channel. This prevents needing an extra DMA map in the fast
> > path.
> > 
> > Link: 
> > https://lore.kernel.org/linux-pci/499934e7-3734-1aee-37dd-b42a5d2a2...@intel.com/
> > Signed-off-by: Logan Gunthorpe 
> > Cc: Jon Mason 
> > Cc: Dave Jiang 
> > Cc: Allen Hubbe 
> 
> Nice! I actually never encountered this on the Intel NTB with IOMMU on.
> It also could be that the Intel BIOS already took care of it for all
> embedded device BARs on the uncore. Nevertheless it's needed. Thanks!
> 
> Reviewed-by: Dave Jiang 

Added to the ntb branch, thanks!

> 
> > ---
> >  drivers/ntb/ntb_transport.c | 28 ++--
> >  1 file changed, 26 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
> > index 3bfdb4562408..526b65afc16a 100644
> > --- a/drivers/ntb/ntb_transport.c
> > +++ b/drivers/ntb/ntb_transport.c
> > @@ -144,7 +144,9 @@ struct ntb_transport_qp {
> > struct list_head tx_free_q;
> > spinlock_t ntb_tx_free_q_lock;
> > void __iomem *tx_mw;
> > -   dma_addr_t tx_mw_phys;
> > +   phys_addr_t tx_mw_phys;
> > +   size_t tx_mw_size;
> > +   dma_addr_t tx_mw_dma_addr;
> > unsigned int tx_index;
> > unsigned int tx_max_entry;
> > unsigned int tx_max_frame;
> > @@ -1049,6 +1051,7 @@ static int ntb_transport_init_queue(struct 
> > ntb_transport_ctx *nt,
> > tx_size = (unsigned int)mw_size / num_qps_mw;
> > qp_offset = tx_size * (qp_num / mw_count);
> >  
> > +   qp->tx_mw_size = tx_size;
> > qp->tx_mw = nt->mw_vec[mw_num].vbase + qp_offset;
> > if (!qp->tx_mw)
> > return -EINVAL;
> > @@ -1644,7 +1647,7 @@ static int ntb_async_tx_submit(struct 
> > ntb_transport_qp *qp,
> > dma_cookie_t cookie;
> >  
> > device = chan->device;
> > -   dest = qp->tx_mw_phys + qp->tx_max_frame * entry->tx_index;
> > +   dest = qp->tx_mw_dma_addr + qp->tx_max_frame * entry->tx_index;
> > buff_off = (size_t)buf & ~PAGE_MASK;
> > dest_off = (size_t)dest & ~PAGE_MASK;
> >  
> > @@ -1863,6 +1866,18 @@ ntb_transport_create_queue(void *data, struct device 
> > *client_dev,
> > qp->rx_dma_chan = NULL;
> > }
> >  
> > +   if (qp->tx_dma_chan) {
> > +   qp->tx_mw_dma_addr =
> > +   dma_map_resource(qp->tx_dma_chan->device->dev,
> > +qp->tx_mw_phys, qp->tx_mw_size,
> > +DMA_FROM_DEVICE, 0);
> > +   if (dma_mapping_error(qp->tx_dma_chan->device->dev,
> > + qp->tx_mw_dma_addr)) {
> > +   qp->tx_mw_dma_addr = 0;
> > +   goto err1;
> > +   }
> > +   }
> > +
> > dev_dbg(>dev, "Using %s memcpy for TX\n",
> > qp->tx_dma_chan ? "DMA" : "CPU");
> >  
> > @@ -1904,6 +1919,10 @@ ntb_transport_create_queue(void *data, struct device 
> > *client_dev,
> > qp->rx_alloc_entry = 0;
> > while ((entry = ntb_list_rm(>ntb_rx_q_lock, >rx_free_q)))
> > kfree(entry);
> > +   if (qp->tx_mw_dma_addr)
> > +   dma_unmap_resource(qp->tx_dma_chan->device->dev,
> > +  qp->tx_mw_dma_addr, qp->tx_mw_size,
> > +  DMA_FROM_DEVICE, 0);
> > if (qp->tx_dma_chan)
> > dma_release_channel(qp->tx_dma_chan);
> > if (qp->rx_dma_chan)
> > @@ -1945,6 +1964,11 @@ void ntb_transport_free_queue(struct 
> > ntb_transport_qp *qp)
> >  */
> > dma_sync_wait(chan, qp->last_cookie);
> > dmaengine_terminate_all(chan);
> > +
> > +   dma_unmap_resource(chan->device->dev,
> > +  qp->tx_mw_dma_addr, qp->tx_mw_size,
> > +  DMA_FROM_DEVICE, 0);
> > +
> > dma_release_channel(chan);
> > }
> >  
> > 

Re: [PATCH v3 0/8] Fix breakage caused by the NTB multi-port patchset

[Jon Mason]

On Wed, Jan 09, 2019 at 12:22:25PM -0700, Logan Gunthorpe wrote:
> Hey,
> 
> I'm resending this because I've recently found out that the change we
> made to use the NTB struct device in DMA allocations is wrong and
> needs to be reverted. Turns out that, when running with an IOMMU,
> dma_alloc_coherent() will always fail if you pass it the NTB struct
> device. This is because the device has not been assigned an IOMMU
> group and the Intel IOMMU at least expect the devices to be on the PCI
> bus and be able to find a proper bus-dev-fn number through a struct
> pci device. Therefore, we must revert the change and I've changed
> patch 2 to do this and remove the no longer necessary DMA mask
> adjustments.
> 
> I'm not sure if we can get past the impass in getting this series merged:
> I still maintain every patch in this series is necessary to fix a
> regression and there's no way to add port numbers to switchtec in the
> crosslink configuration so it can't be fixed in the other way that was
> suggested.

Given the need for this to get in (as it does fix actual problems) and
the absence of comments on v3, I'm adding this to the ntb-next branch.
If there is anyone that has anything more to say on this, please do so
immediately.  Otherwise, I think this probably needs to be moved to the
ntb branch and go into 5.1-rc1.

Thoughts?

Thanks,
Jon

> 
> Logan
> 
> --
> 
> Changes since v2:
> - Rebased on v5.0-rc1
> - Modify Patch 2 to revert back to using the PCI struct device
>   instead of the NTB struct device in DMA calls
> - Collected Allen's Acks
> - Collected Alexander's Tested-By
> 
> Changes since v1:
> - Rebased onto ntb-next (there was a minor conflict in a recent change
>   to the intel driver)
> - Collected Dave's Ack
> 
> --
> 
> Logan Gunthorpe (8):
>   NTB: ntb_tool: reading the link file should not end in a NULL byte
>   NTB: Revert the change to use the NTB device dev for DMA allocations
>   NTB: Fix the default port and peer numbers for legacy drivers
>   NTB: ntb_pingpong: Choose doorbells based on port number
>   NTB: perf: Don't require one more memory window than number of peers
>   NTB: perf: Fix support for hardware that doesn't have port numbers
>   NTB: perf: Fix race condition when run with ntb_test
>   NTB: ntb_test: Fix bug when counting remote files
> 
>  drivers/ntb/hw/amd/ntb_hw_amd.c |  4 
>  drivers/ntb/hw/idt/ntb_hw_idt.c |  6 -
>  drivers/ntb/hw/intel/ntb_hw_gen1.c  |  4 
>  drivers/ntb/ntb.c   |  9 ++--
>  drivers/ntb/test/ntb_perf.c | 29 +++--
>  drivers/ntb/test/ntb_pingpong.c | 14 +---
>  drivers/ntb/test/ntb_tool.c |  9 
>  tools/testing/selftests/ntb/ntb_test.sh |  2 +-
>  8 files changed, 35 insertions(+), 42 deletions(-)
> 
> --
> 2.19.0

Re: [PATCH v2 0/3] ntb_hw_switchtec: Added support of >=4G memory windows

[Jon Mason]

On Wed, Dec 12, 2018 at 7:01 PM Logan Gunthorpe  wrote:
>
>
>
> On 2018-12-12 4:57 p.m., Jon Mason wrote:
> > On Wed, Dec 12, 2018 at 6:42 PM Logan Gunthorpe  wrote:
> >>
> >>
> >>
> >> On 2018-12-12 4:00 p.m., Jon Mason wrote:
> >>> So, you based your patches on a series of patches not in the
> >>> ntb/ntb-next branch?  Please don't do this.  I see nothing in these
> >>> patches which requires that series, which makes this even more
> >>> unnecessary.  Since these are fairly trivial, I'm taking them and
> >>> pushing to the ntb-next branch to give these more time to be tested
> >>> (due to not being tested on the proper branch).  I would really
> >>> appreciate you testing the ntb-next branch as a sanity check.
> >>
> >> The NTB test tools don't work with switchtec hardware without that patch
> >> set, so there's no way to test the changes without that branch.
> >
> > Then let's get those patches in.  IIRC, I asked you to split up the
> > patch series to be bugfixes and features (or at least reorder the
> > series so I can split it up that way in my branches).  Also, I think
> > Serge had some comments that may/may not need to be addressed.  Could
> > you please reorder and resend (and Serge can comment as needed on the
> > resend)?
>
> I resent a while back and responded to all the feedback. Every patch in
> that series fixes a bug. None of them add features.

Per https://lkml.org/lkml/2018/6/12/552, I asked the series be split
up and the comments to be cleaned-up.  You repushed without addressing
this (or Serge's comments), which caused me to ignore the series.
Again, I'm happy to take it as a single series and split it up on my
end, I just need the patches reordered to have the bugfixes I
specified in the front to allow for this to be easily done.

Alternatively, you can rebase and resend them as-is to the ntb mailing
list and we can rehash it out there.

Thanks,
Jon

>
> Logan
>
> --
> You received this message because you are subscribed to the Google Groups 
> "linux-ntb" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to linux-ntb+unsubscr...@googlegroups.com.
> To post to this group, send email to linux-...@googlegroups.com.
> To view this discussion on the web visit 
> https://groups.google.com/d/msgid/linux-ntb/ff7019a0-bac2-2de2-d06c-d54a82286b90%40deltatee.com.
> For more options, visit https://groups.google.com/d/optout.

Re: [PATCH v2 0/3] ntb_hw_switchtec: Added support of >=4G memory windows

[Jon Mason]

On Wed, Dec 12, 2018 at 6:42 PM Logan Gunthorpe  wrote:
>
>
>
> On 2018-12-12 4:00 p.m., Jon Mason wrote:
> > So, you based your patches on a series of patches not in the
> > ntb/ntb-next branch?  Please don't do this.  I see nothing in these
> > patches which requires that series, which makes this even more
> > unnecessary.  Since these are fairly trivial, I'm taking them and
> > pushing to the ntb-next branch to give these more time to be tested
> > (due to not being tested on the proper branch).  I would really
> > appreciate you testing the ntb-next branch as a sanity check.
>
> The NTB test tools don't work with switchtec hardware without that patch
> set, so there's no way to test the changes without that branch.

Then let's get those patches in.  IIRC, I asked you to split up the
patch series to be bugfixes and features (or at least reorder the
series so I can split it up that way in my branches).  Also, I think
Serge had some comments that may/may not need to be addressed.  Could
you please reorder and resend (and Serge can comment as needed on the
resend)?

Thanks,
Jon


>
> You're right that there is no compile-time dependency.
>
> Logan
>
> --
> You received this message because you are subscribed to the Google Groups 
> "linux-ntb" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to linux-ntb+unsubscr...@googlegroups.com.
> To post to this group, send email to linux-...@googlegroups.com.
> To view this discussion on the web visit 
> https://groups.google.com/d/msgid/linux-ntb/f22b6112-920b-37f1-68fe-f43164a18f05%40deltatee.com.
> For more options, visit https://groups.google.com/d/optout.

Re: [PATCH v2 0/3] ntb_hw_switchtec: Added support of >=4G memory windows

[Jon Mason]

On Thu, Dec 6, 2018 at 1:47 AM Wesley Sheng  wrote:
>
> Hi, Everyone,
>
> This patch series adds support of >=4G memory windows.
>
> Current Switchtec's BAR setup registers are limited to 32bits,
> corresponding to the maximum MW (memory window) size is <4G.
> Increase the MW sizes with the addition of the BAR Setup Extension
> Register for the upper 32bits of a 64bits MW size. This increases the MW
> range to between 4K and 2^63.
>
> Additionally, we've made the following changes:
>
> * debug print 64bit aligned crosslink BAR numbers
> * Fix the array size of NT req id mapping table
>
> Tested with ntb_test.sh successfully based on NTB fixes series from
> Logan Gunthorpe  at
> https://github.com/sbates130272/linux-p2pmem on branch of
> ntb_multiport_fixes

So, you based your patches on a series of patches not in the
ntb/ntb-next branch?  Please don't do this.  I see nothing in these
patches which requires that series, which makes this even more
unnecessary.  Since these are fairly trivial, I'm taking them and
pushing to the ntb-next branch to give these more time to be tested
(due to not being tested on the proper branch).  I would really
appreciate you testing the ntb-next branch as a sanity check.

Thanks,
Jon

>
> Regards,
> Wesley
>
> --
>
> Changed since v1:
>   - Using upper_32_bits() and lower_32_bits() marcos makes it easier
> to read and avoids compiler warning on 32-bit arch
>   - Reorder the patches to make the bug fixes first and add a "Fixes"
> line to the commit messages
>
> --
> Paul Selles (2):
>   ntb_hw_switchtec: debug print 64bit aligned crosslink BAR Numbers
>   ntb_hw_switchtec: Added support of >=4G memory windows
>
> Wesley Sheng (1):
>   ntb_hw_switchtec: NT req id mapping table register entry number should
> be 512
>
>  drivers/ntb/hw/mscc/ntb_hw_switchtec.c | 11 ---
>  include/linux/switchtec.h  | 10 +++---
>  2 files changed, 15 insertions(+), 6 deletions(-)
>
> --
> 2.7.4
>

Re: [PATCH 2/3] ntb_hw_switchtec: Added support of >=4G memory windows

[Jon Mason]

On Wed, Nov 21, 2018 at 9:19 PM Wesley Sheng  wrote:
>
> From: Paul Selles 
>
> Current Switchtec's BAR setup registers are limited to 32bits,
> corresponding to the maximum MW (memory window) size is <4G.
>
> Increase the MW sizes with the addition of the BAR Setup Extension
> Register for the upper 32bits of a 64bits MW size. This increases the MW
> range to between 4K and 2^63.
>
> Reported-by: Boris Glimcher 
> Signed-off-by: Paul Selles 
> Signed-off-by: Wesley Sheng 
> ---
>  drivers/ntb/hw/mscc/ntb_hw_switchtec.c | 9 +++--
>  include/linux/switchtec.h  | 6 +-
>  2 files changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/ntb/hw/mscc/ntb_hw_switchtec.c 
> b/drivers/ntb/hw/mscc/ntb_hw_switchtec.c
> index 9916bc5..32850fb 100644
> --- a/drivers/ntb/hw/mscc/ntb_hw_switchtec.c
> +++ b/drivers/ntb/hw/mscc/ntb_hw_switchtec.c
> @@ -264,6 +264,7 @@ static void switchtec_ntb_mw_clr_direct(struct 
> switchtec_ntb *sndev, int idx)
> ctl_val &= ~NTB_CTRL_BAR_DIR_WIN_EN;
> iowrite32(ctl_val, >bar_entry[bar].ctl);
> iowrite32(0, >bar_entry[bar].win_size);
> +   iowrite32(0, >bar_ext_entry[bar].win_size);
> iowrite64(sndev->self_partition, >bar_entry[bar].xlate_addr);
>  }
>
> @@ -286,7 +287,9 @@ static void switchtec_ntb_mw_set_direct(struct 
> switchtec_ntb *sndev, int idx,
> ctl_val |= NTB_CTRL_BAR_DIR_WIN_EN;
>
> iowrite32(ctl_val, >bar_entry[bar].ctl);
> -   iowrite32(xlate_pos | size, >bar_entry[bar].win_size);
> +   iowrite32(xlate_pos | (size & 0xF000),
> + >bar_entry[bar].win_size);
> +   iowrite32(size >> 32, >bar_ext_entry[bar].win_size);

Thanks for the patches.  Overall the look good.  Per the kbuild email,
size_t is 32bits on 32bit arch.  So, this is going to have compile
warnings on those.  Please address this and resubmit.

Also, patches 1 and 3 are bug fixes.  Please do the following, reorder
the patches to make the bug fixes first and add a "Fixes" line to the
commit messages (see
https://www.kernel.org/doc/html/latest/process/submitting-patches.html#using-reported-by-tested-by-reviewed-by-suggested-by-and-fixes).
This will allow me to split up the series and get the bug fixes into
v4.20 (and the stable trees).

Thanks,
Jon

> iowrite64(sndev->self_partition | addr,
>   >bar_entry[bar].xlate_addr);
>  }
> @@ -1053,7 +1056,9 @@ static int crosslink_setup_mws(struct switchtec_ntb 
> *sndev, int ntb_lut_idx,
> ctl_val |= NTB_CTRL_BAR_DIR_WIN_EN;
>
> iowrite32(ctl_val, >bar_entry[bar].ctl);
> -   iowrite32(xlate_pos | size, >bar_entry[bar].win_size);
> +   iowrite32(xlate_pos | (size & 0xF000),
> + >bar_entry[bar].win_size);
> +   iowrite32(size >> 32, >bar_ext_entry[bar].win_size);
> iowrite64(sndev->peer_partition | addr,
>   >bar_entry[bar].xlate_addr);
> }
> diff --git a/include/linux/switchtec.h b/include/linux/switchtec.h
> index eee0412..1e6e333 100644
> --- a/include/linux/switchtec.h
> +++ b/include/linux/switchtec.h
> @@ -248,7 +248,11 @@ struct ntb_ctrl_regs {
> u32 win_size;
> u64 xlate_addr;
> } bar_entry[6];
> -   u32 reserved2[216];
> +   struct {
> +   u32 win_size;
> +   u32 reserved[3];
> +   } bar_ext_entry[6];
> +   u32 reserved2[192];
> u32 req_id_table[256];
> u32 reserved3[512];
> u64 lut_entry[512];
> --
> 2.7.4
>

Re: [PATCH 2/3] ntb_hw_switchtec: Added support of >=4G memory windows

[Jon Mason]

On Wed, Nov 21, 2018 at 9:19 PM Wesley Sheng  wrote:
>
> From: Paul Selles 
>
> Current Switchtec's BAR setup registers are limited to 32bits,
> corresponding to the maximum MW (memory window) size is <4G.
>
> Increase the MW sizes with the addition of the BAR Setup Extension
> Register for the upper 32bits of a 64bits MW size. This increases the MW
> range to between 4K and 2^63.
>
> Reported-by: Boris Glimcher 
> Signed-off-by: Paul Selles 
> Signed-off-by: Wesley Sheng 
> ---
>  drivers/ntb/hw/mscc/ntb_hw_switchtec.c | 9 +++--
>  include/linux/switchtec.h  | 6 +-
>  2 files changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/ntb/hw/mscc/ntb_hw_switchtec.c 
> b/drivers/ntb/hw/mscc/ntb_hw_switchtec.c
> index 9916bc5..32850fb 100644
> --- a/drivers/ntb/hw/mscc/ntb_hw_switchtec.c
> +++ b/drivers/ntb/hw/mscc/ntb_hw_switchtec.c
> @@ -264,6 +264,7 @@ static void switchtec_ntb_mw_clr_direct(struct 
> switchtec_ntb *sndev, int idx)
> ctl_val &= ~NTB_CTRL_BAR_DIR_WIN_EN;
> iowrite32(ctl_val, >bar_entry[bar].ctl);
> iowrite32(0, >bar_entry[bar].win_size);
> +   iowrite32(0, >bar_ext_entry[bar].win_size);
> iowrite64(sndev->self_partition, >bar_entry[bar].xlate_addr);
>  }
>
> @@ -286,7 +287,9 @@ static void switchtec_ntb_mw_set_direct(struct 
> switchtec_ntb *sndev, int idx,
> ctl_val |= NTB_CTRL_BAR_DIR_WIN_EN;
>
> iowrite32(ctl_val, >bar_entry[bar].ctl);
> -   iowrite32(xlate_pos | size, >bar_entry[bar].win_size);
> +   iowrite32(xlate_pos | (size & 0xF000),
> + >bar_entry[bar].win_size);
> +   iowrite32(size >> 32, >bar_ext_entry[bar].win_size);

Thanks for the patches.  Overall the look good.  Per the kbuild email,
size_t is 32bits on 32bit arch.  So, this is going to have compile
warnings on those.  Please address this and resubmit.

Also, patches 1 and 3 are bug fixes.  Please do the following, reorder
the patches to make the bug fixes first and add a "Fixes" line to the
commit messages (see
https://www.kernel.org/doc/html/latest/process/submitting-patches.html#using-reported-by-tested-by-reviewed-by-suggested-by-and-fixes).
This will allow me to split up the series and get the bug fixes into
v4.20 (and the stable trees).

Thanks,
Jon

> iowrite64(sndev->self_partition | addr,
>   >bar_entry[bar].xlate_addr);
>  }
> @@ -1053,7 +1056,9 @@ static int crosslink_setup_mws(struct switchtec_ntb 
> *sndev, int ntb_lut_idx,
> ctl_val |= NTB_CTRL_BAR_DIR_WIN_EN;
>
> iowrite32(ctl_val, >bar_entry[bar].ctl);
> -   iowrite32(xlate_pos | size, >bar_entry[bar].win_size);
> +   iowrite32(xlate_pos | (size & 0xF000),
> + >bar_entry[bar].win_size);
> +   iowrite32(size >> 32, >bar_ext_entry[bar].win_size);
> iowrite64(sndev->peer_partition | addr,
>   >bar_entry[bar].xlate_addr);
> }
> diff --git a/include/linux/switchtec.h b/include/linux/switchtec.h
> index eee0412..1e6e333 100644
> --- a/include/linux/switchtec.h
> +++ b/include/linux/switchtec.h
> @@ -248,7 +248,11 @@ struct ntb_ctrl_regs {
> u32 win_size;
> u64 xlate_addr;
> } bar_entry[6];
> -   u32 reserved2[216];
> +   struct {
> +   u32 win_size;
> +   u32 reserved[3];
> +   } bar_ext_entry[6];
> +   u32 reserved2[192];
> u32 req_id_table[256];
> u32 reserved3[512];
> u64 lut_entry[512];
> --
> 2.7.4
>

[GIT PULL] NTB patches for v4.20

[Jon Mason]

Hello Linus,
Here are a few NTB patches for v4.20.  Fairly minor changes and bug
fixes.  Please consider pulling them.


Thanks,
Jon

---

The following changes since commit 84df9525b0c27f3ebc2ebb1864fa62a97fdedb7d:

  Linux 4.19 (2018-10-22 07:37:37 +0100)

are available in the Git repository at:

  git://github.com/jonmason/ntb tags/ntb-4.20

for you to fetch changes up to a662315d8ad9e687fe648b6eea9bd35017f565dd:

  ntb: idt: Alter the driver info comments (2018-11-01 10:33:12 -0400)


NTB IDT thermal changes and hook into hwmon, ntb_netdev clean-up of
private struct, and a few bug fixes.


Aaron Sierra (2):
  NTB: transport: Try harder to alloc an aligned MW buffer
  ntb_netdev: Simplify remove with client device drvdata

Dave Jiang (1):
  ntb: intel: fix return value for ndev_vec_mask()

Gustavo A. R. Silva (2):
  NTB: ntb_hw_idt: replace IS_ERR_OR_NULL with regular NULL checks
  ntb: ntb_transport: Mark expected switch fall-throughs

Jon Mason (1):
  ntb_netdev: fix sleep time mismatch

Serge Semin (5):
  ntb: idt: Set PCIe bus address to BARLIMITx
  ntb: idt: Alter temperature read method
  ntb: idt: Add basic hwmon sysfs interface
  ntb: idt: Discard temperature sensor IRQ handler
  ntb: idt: Alter the driver info comments

 drivers/net/ntb_netdev.c   |  30 +---
 drivers/ntb/hw/idt/Kconfig |   5 +-
 drivers/ntb/hw/idt/ntb_hw_idt.c| 327 +++--
 drivers/ntb/hw/idt/ntb_hw_idt.h|  87 +-
 drivers/ntb/hw/intel/ntb_hw_gen1.c |   2 +-
 drivers/ntb/ntb_transport.c|  88 +++---
 6 files changed, 429 insertions(+), 110 deletions(-)

[GIT PULL] NTB patches for v4.20

[Jon Mason]

Hello Linus,
Here are a few NTB patches for v4.20.  Fairly minor changes and bug
fixes.  Please consider pulling them.


Thanks,
Jon

---

The following changes since commit 84df9525b0c27f3ebc2ebb1864fa62a97fdedb7d:

  Linux 4.19 (2018-10-22 07:37:37 +0100)

are available in the Git repository at:

  git://github.com/jonmason/ntb tags/ntb-4.20

for you to fetch changes up to a662315d8ad9e687fe648b6eea9bd35017f565dd:

  ntb: idt: Alter the driver info comments (2018-11-01 10:33:12 -0400)


NTB IDT thermal changes and hook into hwmon, ntb_netdev clean-up of
private struct, and a few bug fixes.


Aaron Sierra (2):
  NTB: transport: Try harder to alloc an aligned MW buffer
  ntb_netdev: Simplify remove with client device drvdata

Dave Jiang (1):
  ntb: intel: fix return value for ndev_vec_mask()

Gustavo A. R. Silva (2):
  NTB: ntb_hw_idt: replace IS_ERR_OR_NULL with regular NULL checks
  ntb: ntb_transport: Mark expected switch fall-throughs

Jon Mason (1):
  ntb_netdev: fix sleep time mismatch

Serge Semin (5):
  ntb: idt: Set PCIe bus address to BARLIMITx
  ntb: idt: Alter temperature read method
  ntb: idt: Add basic hwmon sysfs interface
  ntb: idt: Discard temperature sensor IRQ handler
  ntb: idt: Alter the driver info comments

 drivers/net/ntb_netdev.c   |  30 +---
 drivers/ntb/hw/idt/Kconfig |   5 +-
 drivers/ntb/hw/idt/ntb_hw_idt.c| 327 +++--
 drivers/ntb/hw/idt/ntb_hw_idt.h|  87 +-
 drivers/ntb/hw/intel/ntb_hw_gen1.c |   2 +-
 drivers/ntb/ntb_transport.c|  88 +++---
 6 files changed, 429 insertions(+), 110 deletions(-)

Re: [PATCH v2 0/4] ntb: idt: Add hwmon temperature sensor interface

[Jon Mason]

On Tue, Jul 17, 2018 at 12:24:33PM +0300, Serge Semin wrote:
> IDT PCIe-switches are equipped with an embedded temperature sensor. It
> works within the range [0; 127.5]C with a resolution of 0.5C. It can
> be used to monitor the chip core temperature so to have prevent it from
> possible overheating. It might be very topical for the chip, since it
> gets heated like in hell especially if ASPM isn't enabled.
> 
> Other than the current sampled temperatur, the sensor interface exposes
> history registors with lowest and highest measured temperature, thresholds
> and alarm IRQs enabled/disable bits, ADC/filter settings. The device manual
> states that the switch is able to generate a msi interrupt on PCIe upstreams
> if the temperature crosses one of three configurable thresholds. But in
> practice we discovered that the enable/disable threshold IRQs bits interface
> is very broken (see the third patch commit message), so it can't be used
> to create the hwmon alarm interface. As the result we had to remove the
> already available temperature sensor IRQ handler and disable the corresponding
> interrupt.
> 
> Current version of the driver provides following standard hwmon sysfs
> files: temperature input, lowest and highest measured temperature
> with possibility to reset the history, temperature offset. The rest of the
> nodes can't be safely implemented for the chip due to the described issues.
> 
> Changelog v2:
> - Add "select HWMON" to the NTB_IDT kconfig
> 
> Signed-off-by: Serge Semin 

 was waiting on you to address the kbuild errors, and it appears you
 did already.  Somehow I missed v2 of this series and just found it now.
 Sorry :(

Applied to the ntb-next branch

Thanks,
Jon

> 
> Serge Semin (4):
>   ntb: idt: Alter temperature read method
>   ntb: idt: Add basic hwmon sysfs interface
>   ntb: idt: Discard temperature sensor IRQ handler
>   ntb: idt: Alter the driver info comments
> 
>  drivers/ntb/hw/idt/Kconfig  |   4 +-
>  drivers/ntb/hw/idt/ntb_hw_idt.c | 317 
> ++--
>  drivers/ntb/hw/idt/ntb_hw_idt.h |  87 ++-
>  3 files changed, 353 insertions(+), 55 deletions(-)
> 
> -- 
> 2.12.0
> 

Re: [PATCH v2 0/4] ntb: idt: Add hwmon temperature sensor interface

[Jon Mason]

On Tue, Jul 17, 2018 at 12:24:33PM +0300, Serge Semin wrote:
> IDT PCIe-switches are equipped with an embedded temperature sensor. It
> works within the range [0; 127.5]C with a resolution of 0.5C. It can
> be used to monitor the chip core temperature so to have prevent it from
> possible overheating. It might be very topical for the chip, since it
> gets heated like in hell especially if ASPM isn't enabled.
> 
> Other than the current sampled temperatur, the sensor interface exposes
> history registors with lowest and highest measured temperature, thresholds
> and alarm IRQs enabled/disable bits, ADC/filter settings. The device manual
> states that the switch is able to generate a msi interrupt on PCIe upstreams
> if the temperature crosses one of three configurable thresholds. But in
> practice we discovered that the enable/disable threshold IRQs bits interface
> is very broken (see the third patch commit message), so it can't be used
> to create the hwmon alarm interface. As the result we had to remove the
> already available temperature sensor IRQ handler and disable the corresponding
> interrupt.
> 
> Current version of the driver provides following standard hwmon sysfs
> files: temperature input, lowest and highest measured temperature
> with possibility to reset the history, temperature offset. The rest of the
> nodes can't be safely implemented for the chip due to the described issues.
> 
> Changelog v2:
> - Add "select HWMON" to the NTB_IDT kconfig
> 
> Signed-off-by: Serge Semin 

 was waiting on you to address the kbuild errors, and it appears you
 did already.  Somehow I missed v2 of this series and just found it now.
 Sorry :(

Applied to the ntb-next branch

Thanks,
Jon

> 
> Serge Semin (4):
>   ntb: idt: Alter temperature read method
>   ntb: idt: Add basic hwmon sysfs interface
>   ntb: idt: Discard temperature sensor IRQ handler
>   ntb: idt: Alter the driver info comments
> 
>  drivers/ntb/hw/idt/Kconfig  |   4 +-
>  drivers/ntb/hw/idt/ntb_hw_idt.c | 317 
> ++--
>  drivers/ntb/hw/idt/ntb_hw_idt.h |  87 ++-
>  3 files changed, 353 insertions(+), 55 deletions(-)
> 
> -- 
> 2.12.0
> 

Re: [PATCH 0/4] ntb: idt: Add hwmon temperature sensor interface

[Jon Mason]

On Sat, Jul 14, 2018 at 02:58:30PM +0300, Serge Semin wrote:
> IDT PCIe-switches are equipped with an embedded temperature sensor. It
> works within the range [0; 127.5]C with a resolution of 0.5C. It can
> be used to monitor the chip core temperature so to have prevent it from
> possible overheating. It might be very topical for the chip, since it
> gets heated like in hell especially if ASPM isn't enabled.
> 
> Other than the current sampled temperatur, the sensor interface exposes
> history registors with lowest and highest measured temperature, thresholds
> and alarm IRQs enabled/disable bits, ADC/filter settings. The device manual
> states that the switch is able to generate a msi interrupt on PCIe upstreams
> if the temperature crosses one of three configurable thresholds. But in
> practice we discovered that the enable/disable threshold IRQs bits interface
> is very broken (see the third patch commit message), so it can't be used
> to create the hwmon alarm interface. As the result we had to remove the
> already available temperature sensor IRQ handler and disable the corresponding
> interrupt.
> 
> Current version of the driver provides following standard hwmon sysfs
> files: temperature input, lowest and highest measured temperature
> with possibility to reset the history, temperature offset. The rest of the
> nodes can't be safely implemented for the chip due to the described issues.
> 
> Signed-off-by: Serge Semin 

FYI, I'm waiting on you to correct the kbuild issues.  I have been
assuming you knew this, but given the lack of follow-up patches I am
beginning to this you might believe I'm ignoring them.  I'm not :)

Thanks,
Jon

> 
> Serge Semin (4):
>   ntb: idt: Alter temperature read method
>   ntb: idt: Add basic hwmon sysfs interface
>   ntb: idt: Discard temperature sensor IRQ handler
>   ntb: idt: Alter the driver info comments
> 
>  drivers/ntb/hw/idt/Kconfig  |   4 +-
>  drivers/ntb/hw/idt/ntb_hw_idt.c | 317 
> ++--
>  drivers/ntb/hw/idt/ntb_hw_idt.h |  87 ++-
>  3 files changed, 353 insertions(+), 55 deletions(-)
> 
> -- 
> 2.12.0
> 

Re: [PATCH 0/4] ntb: idt: Add hwmon temperature sensor interface

[Jon Mason]

On Sat, Jul 14, 2018 at 02:58:30PM +0300, Serge Semin wrote:
> IDT PCIe-switches are equipped with an embedded temperature sensor. It
> works within the range [0; 127.5]C with a resolution of 0.5C. It can
> be used to monitor the chip core temperature so to have prevent it from
> possible overheating. It might be very topical for the chip, since it
> gets heated like in hell especially if ASPM isn't enabled.
> 
> Other than the current sampled temperatur, the sensor interface exposes
> history registors with lowest and highest measured temperature, thresholds
> and alarm IRQs enabled/disable bits, ADC/filter settings. The device manual
> states that the switch is able to generate a msi interrupt on PCIe upstreams
> if the temperature crosses one of three configurable thresholds. But in
> practice we discovered that the enable/disable threshold IRQs bits interface
> is very broken (see the third patch commit message), so it can't be used
> to create the hwmon alarm interface. As the result we had to remove the
> already available temperature sensor IRQ handler and disable the corresponding
> interrupt.
> 
> Current version of the driver provides following standard hwmon sysfs
> files: temperature input, lowest and highest measured temperature
> with possibility to reset the history, temperature offset. The rest of the
> nodes can't be safely implemented for the chip due to the described issues.
> 
> Signed-off-by: Serge Semin 

FYI, I'm waiting on you to correct the kbuild issues.  I have been
assuming you knew this, but given the lack of follow-up patches I am
beginning to this you might believe I'm ignoring them.  I'm not :)

Thanks,
Jon

> 
> Serge Semin (4):
>   ntb: idt: Alter temperature read method
>   ntb: idt: Add basic hwmon sysfs interface
>   ntb: idt: Discard temperature sensor IRQ handler
>   ntb: idt: Alter the driver info comments
> 
>  drivers/ntb/hw/idt/Kconfig  |   4 +-
>  drivers/ntb/hw/idt/ntb_hw_idt.c | 317 
> ++--
>  drivers/ntb/hw/idt/ntb_hw_idt.h |  87 ++-
>  3 files changed, 353 insertions(+), 55 deletions(-)
> 
> -- 
> 2.12.0
> 

Re: [PATCH] ntb: ntb_transport: Mark expected switch fall-throughs

[Jon Mason]

On Fri, Oct 05, 2018 at 01:03:01PM -0400, Allen Hubbe wrote:
> On Fri, Oct 5, 2018 at 3:12 AM Gustavo A. R. Silva
>  wrote:
> > In preparation to enabling -Wimplicit-fallthrough, mark switch cases
> > where we are expecting to fall through.
> >
> > Addresses-Coverity-ID: 1373888 ("Missing break in switch")
> > Addresses-Coverity-ID: 1373889 ("Missing break in switch")
> > Signed-off-by: Gustavo A. R. Silva 
> 
> Acked-by: Allen Hubbe 

Applied

Thanks,
Jon

> 
> > ---
> >  drivers/ntb/ntb_transport.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
> > index 9398959..c643b9c 100644
> > --- a/drivers/ntb/ntb_transport.c
> > +++ b/drivers/ntb/ntb_transport.c
> > @@ -1278,6 +1278,7 @@ static void ntb_rx_copy_callback(void *data,
> > case DMA_TRANS_READ_FAILED:
> > case DMA_TRANS_WRITE_FAILED:
> > entry->errors++;
> > +   /* fall through */
> > case DMA_TRANS_ABORTED:
> > {
> > struct ntb_transport_qp *qp = entry->qp;
> > @@ -1533,6 +1534,7 @@ static void ntb_tx_copy_callback(void *data,
> > case DMA_TRANS_READ_FAILED:
> > case DMA_TRANS_WRITE_FAILED:
> > entry->errors++;
> > +   /* fall through */
> > case DMA_TRANS_ABORTED:
> > {
> > void __iomem *offset =
> > --
> > 2.7.4

Re: [PATCH] ntb: ntb_transport: Mark expected switch fall-throughs

[Jon Mason]

On Fri, Oct 05, 2018 at 01:03:01PM -0400, Allen Hubbe wrote:
> On Fri, Oct 5, 2018 at 3:12 AM Gustavo A. R. Silva
>  wrote:
> > In preparation to enabling -Wimplicit-fallthrough, mark switch cases
> > where we are expecting to fall through.
> >
> > Addresses-Coverity-ID: 1373888 ("Missing break in switch")
> > Addresses-Coverity-ID: 1373889 ("Missing break in switch")
> > Signed-off-by: Gustavo A. R. Silva 
> 
> Acked-by: Allen Hubbe 

Applied

Thanks,
Jon

> 
> > ---
> >  drivers/ntb/ntb_transport.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
> > index 9398959..c643b9c 100644
> > --- a/drivers/ntb/ntb_transport.c
> > +++ b/drivers/ntb/ntb_transport.c
> > @@ -1278,6 +1278,7 @@ static void ntb_rx_copy_callback(void *data,
> > case DMA_TRANS_READ_FAILED:
> > case DMA_TRANS_WRITE_FAILED:
> > entry->errors++;
> > +   /* fall through */
> > case DMA_TRANS_ABORTED:
> > {
> > struct ntb_transport_qp *qp = entry->qp;
> > @@ -1533,6 +1534,7 @@ static void ntb_tx_copy_callback(void *data,
> > case DMA_TRANS_READ_FAILED:
> > case DMA_TRANS_WRITE_FAILED:
> > entry->errors++;
> > +   /* fall through */
> > case DMA_TRANS_ABORTED:
> > {
> > void __iomem *offset =
> > --
> > 2.7.4

Re: [PATCH] NTB: ntb_hw_idt: replace IS_ERR_OR_NULL with regular NULL checks

[Jon Mason]

On Mon, Aug 27, 2018 at 05:13:06PM -0500, Gustavo A. R. Silva wrote:
> Both devm_kcalloc() and devm_kzalloc() return NULL on error. They
> never return error pointers.
> 
> The use of IS_ERR_OR_NULL is currently applied to the wrong
> context.
> 
> Fix this by replacing IS_ERR_OR_NULL with regular NULL checks.
> 
> Fixes: bf2a952d31d2 ("NTB: Add IDT 89HPESxNTx PCIe-switches support")
> Signed-off-by: Gustavo A. R. Silva 

Applied to my ntb branch

Thanks,
Jon

> ---
>  drivers/ntb/hw/idt/ntb_hw_idt.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/ntb/hw/idt/ntb_hw_idt.c b/drivers/ntb/hw/idt/ntb_hw_idt.c
> index dbe72f1..a67ef23 100644
> --- a/drivers/ntb/hw/idt/ntb_hw_idt.c
> +++ b/drivers/ntb/hw/idt/ntb_hw_idt.c
> @@ -1105,9 +1105,9 @@ static struct idt_mw_cfg *idt_scan_mws(struct 
> idt_ntb_dev *ndev, int port,
>   }
>  
>   /* Allocate memory for memory window descriptors */
> - ret_mws = devm_kcalloc(>ntb.pdev->dev, *mw_cnt,
> - sizeof(*ret_mws), GFP_KERNEL);
> - if (IS_ERR_OR_NULL(ret_mws))
> + ret_mws = devm_kcalloc(>ntb.pdev->dev, *mw_cnt, sizeof(*ret_mws),
> +GFP_KERNEL);
> + if (!ret_mws)
>   return ERR_PTR(-ENOMEM);
>  
>   /* Copy the info of detected memory windows */
> @@ -2390,7 +2390,7 @@ static struct idt_ntb_dev *idt_create_dev(struct 
> pci_dev *pdev,
>  
>   /* Allocate memory for the IDT PCIe-device descriptor */
>   ndev = devm_kzalloc(>dev, sizeof(*ndev), GFP_KERNEL);
> - if (IS_ERR_OR_NULL(ndev)) {
> + if (!ndev) {
>   dev_err(>dev, "Memory allocation failed for descriptor");
>   return ERR_PTR(-ENOMEM);
>   }
> -- 
> 2.7.4
> 

Re: [PATCH] NTB: ntb_hw_idt: replace IS_ERR_OR_NULL with regular NULL checks

[Jon Mason]

On Mon, Aug 27, 2018 at 05:13:06PM -0500, Gustavo A. R. Silva wrote:
> Both devm_kcalloc() and devm_kzalloc() return NULL on error. They
> never return error pointers.
> 
> The use of IS_ERR_OR_NULL is currently applied to the wrong
> context.
> 
> Fix this by replacing IS_ERR_OR_NULL with regular NULL checks.
> 
> Fixes: bf2a952d31d2 ("NTB: Add IDT 89HPESxNTx PCIe-switches support")
> Signed-off-by: Gustavo A. R. Silva 

Applied to my ntb branch

Thanks,
Jon

> ---
>  drivers/ntb/hw/idt/ntb_hw_idt.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/ntb/hw/idt/ntb_hw_idt.c b/drivers/ntb/hw/idt/ntb_hw_idt.c
> index dbe72f1..a67ef23 100644
> --- a/drivers/ntb/hw/idt/ntb_hw_idt.c
> +++ b/drivers/ntb/hw/idt/ntb_hw_idt.c
> @@ -1105,9 +1105,9 @@ static struct idt_mw_cfg *idt_scan_mws(struct 
> idt_ntb_dev *ndev, int port,
>   }
>  
>   /* Allocate memory for memory window descriptors */
> - ret_mws = devm_kcalloc(>ntb.pdev->dev, *mw_cnt,
> - sizeof(*ret_mws), GFP_KERNEL);
> - if (IS_ERR_OR_NULL(ret_mws))
> + ret_mws = devm_kcalloc(>ntb.pdev->dev, *mw_cnt, sizeof(*ret_mws),
> +GFP_KERNEL);
> + if (!ret_mws)
>   return ERR_PTR(-ENOMEM);
>  
>   /* Copy the info of detected memory windows */
> @@ -2390,7 +2390,7 @@ static struct idt_ntb_dev *idt_create_dev(struct 
> pci_dev *pdev,
>  
>   /* Allocate memory for the IDT PCIe-device descriptor */
>   ndev = devm_kzalloc(>dev, sizeof(*ndev), GFP_KERNEL);
> - if (IS_ERR_OR_NULL(ndev)) {
> + if (!ndev) {
>   dev_err(>dev, "Memory allocation failed for descriptor");
>   return ERR_PTR(-ENOMEM);
>   }
> -- 
> 2.7.4
> 

[PATCH] MAINTAINERS: Remove self from Broadcom SoCs

[Jon Mason]

I'm leaving Broadcom, and will no longer have access to hardware and
documentation necessary to be effective in a maintainership role.

Signed-off-by: Jon Mason 
---
 MAINTAINERS | 2 --
 1 file changed, 2 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 4ece30f15777..d063c605074e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2886,7 +2886,6 @@ F:arch/mips/include/asm/mach-bcm47xx/*
 BROADCOM BCM5301X ARM ARCHITECTURE
 M: Hauke Mehrtens 
 M: Rafał Miłecki 
-M: Jon Mason 
 M: bcm-kernel-feedback-l...@broadcom.com
 L: linux-arm-ker...@lists.infradead.org
 S: Maintained
@@ -3024,7 +3023,6 @@ F:drivers/net/ethernet/broadcom/genet/
 BROADCOM IPROC ARM ARCHITECTURE
 M: Ray Jui 
 M: Scott Branden 
-M: Jon Mason 
 M: bcm-kernel-feedback-l...@broadcom.com
 L: linux-arm-ker...@lists.infradead.org (moderated for non-subscribers)
 T: git git://github.com/broadcom/cygnus-linux.git
-- 
2.7.4

[PATCH] MAINTAINERS: Remove self from Broadcom SoCs

[Jon Mason]

I'm leaving Broadcom, and will no longer have access to hardware and
documentation necessary to be effective in a maintainership role.

Signed-off-by: Jon Mason 
---
 MAINTAINERS | 2 --
 1 file changed, 2 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 4ece30f15777..d063c605074e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2886,7 +2886,6 @@ F:arch/mips/include/asm/mach-bcm47xx/*
 BROADCOM BCM5301X ARM ARCHITECTURE
 M: Hauke Mehrtens 
 M: Rafał Miłecki 
-M: Jon Mason 
 M: bcm-kernel-feedback-l...@broadcom.com
 L: linux-arm-ker...@lists.infradead.org
 S: Maintained
@@ -3024,7 +3023,6 @@ F:drivers/net/ethernet/broadcom/genet/
 BROADCOM IPROC ARM ARCHITECTURE
 M: Ray Jui 
 M: Scott Branden 
-M: Jon Mason 
 M: bcm-kernel-feedback-l...@broadcom.com
 L: linux-arm-ker...@lists.infradead.org (moderated for non-subscribers)
 T: git git://github.com/broadcom/cygnus-linux.git
-- 
2.7.4

Re: [PATCH 3/8] NTB: Fix the default port and peer numbers for legacy drivers

[Jon Mason]

On Fri, Jun 8, 2018 at 8:08 PM, Logan Gunthorpe  wrote:
> When the commit adding ntb_default_port_number() and
> ntb_default_peer_port_number()  entered the kernel there was no
> users of it so it was impossible to tell what the API needed.
>
> When a user finally landed a year later (ntb_pingpong) there were
> more NTB topologies were created and no consideration was considered
> to how other drivers had changed.
>
> Now that there is a user it can be fixed to provide a sensible default
> for the legacy drivers that do not implement ntb_{peer_}port_number().
> Seeing ntb_pingpong doesn't check error codes returning EINVAL was also
> not sensible.
>
> Patches for ntb_pingpong and ntb_perf follow (which are broken
> otherwise) to support hardware that doesn't have port numbers. This is
> important not only to not break support with existing drivers but for
> the cross link topology which, due to its perfect symmetry, cannot
> assign unique port numbers to each side.

This is a very long way of saying "no clients are checking the error
codes, so removing them". :)
I think the history and references to follow-on patches are not
necessary in the commit message and belong more in a 0/X.

This is more of a feature than a bug fix.  Can you break this (and the
pingpong and perf changes caused by this) off into a separate series,
as I'll want to apply this to the ntb-next and not bugfixes branch?

Thanks,
Jon

> Fixes: 1e5301196a88 ("NTB: Add indexed ports NTB API")
> Signed-off-by: Logan Gunthorpe 
> ---
>  drivers/ntb/ntb.c | 9 ++---
>  1 file changed, 2 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/ntb/ntb.c b/drivers/ntb/ntb.c
> index 93f24440d11d..d955a92a095a 100644
> --- a/drivers/ntb/ntb.c
> +++ b/drivers/ntb/ntb.c
> @@ -225,10 +225,8 @@ int ntb_default_port_number(struct ntb_dev *ntb)
> case NTB_TOPO_B2B_DSD:
> return NTB_PORT_SEC_DSD;
> default:
> -   break;
> +   return 0;
> }
> -
> -   return -EINVAL;
>  }
>  EXPORT_SYMBOL(ntb_default_port_number);
>
> @@ -251,10 +249,8 @@ int ntb_default_peer_port_number(struct ntb_dev *ntb, 
> int pidx)
> case NTB_TOPO_B2B_DSD:
> return NTB_PORT_PRI_USD;
> default:
> -   break;
> +   return 0;
> }
> -
> -   return -EINVAL;
>  }
>  EXPORT_SYMBOL(ntb_default_peer_port_number);
>
> @@ -326,4 +322,3 @@ static void __exit ntb_driver_exit(void)
> bus_unregister(_bus);
>  }
>  module_exit(ntb_driver_exit);
> -
> --
> 2.11.0
>

Re: [PATCH 3/8] NTB: Fix the default port and peer numbers for legacy drivers

[Jon Mason]

On Fri, Jun 8, 2018 at 8:08 PM, Logan Gunthorpe  wrote:
> When the commit adding ntb_default_port_number() and
> ntb_default_peer_port_number()  entered the kernel there was no
> users of it so it was impossible to tell what the API needed.
>
> When a user finally landed a year later (ntb_pingpong) there were
> more NTB topologies were created and no consideration was considered
> to how other drivers had changed.
>
> Now that there is a user it can be fixed to provide a sensible default
> for the legacy drivers that do not implement ntb_{peer_}port_number().
> Seeing ntb_pingpong doesn't check error codes returning EINVAL was also
> not sensible.
>
> Patches for ntb_pingpong and ntb_perf follow (which are broken
> otherwise) to support hardware that doesn't have port numbers. This is
> important not only to not break support with existing drivers but for
> the cross link topology which, due to its perfect symmetry, cannot
> assign unique port numbers to each side.

This is a very long way of saying "no clients are checking the error
codes, so removing them". :)
I think the history and references to follow-on patches are not
necessary in the commit message and belong more in a 0/X.

This is more of a feature than a bug fix.  Can you break this (and the
pingpong and perf changes caused by this) off into a separate series,
as I'll want to apply this to the ntb-next and not bugfixes branch?

Thanks,
Jon

> Fixes: 1e5301196a88 ("NTB: Add indexed ports NTB API")
> Signed-off-by: Logan Gunthorpe 
> ---
>  drivers/ntb/ntb.c | 9 ++---
>  1 file changed, 2 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/ntb/ntb.c b/drivers/ntb/ntb.c
> index 93f24440d11d..d955a92a095a 100644
> --- a/drivers/ntb/ntb.c
> +++ b/drivers/ntb/ntb.c
> @@ -225,10 +225,8 @@ int ntb_default_port_number(struct ntb_dev *ntb)
> case NTB_TOPO_B2B_DSD:
> return NTB_PORT_SEC_DSD;
> default:
> -   break;
> +   return 0;
> }
> -
> -   return -EINVAL;
>  }
>  EXPORT_SYMBOL(ntb_default_port_number);
>
> @@ -251,10 +249,8 @@ int ntb_default_peer_port_number(struct ntb_dev *ntb, 
> int pidx)
> case NTB_TOPO_B2B_DSD:
> return NTB_PORT_PRI_USD;
> default:
> -   break;
> +   return 0;
> }
> -
> -   return -EINVAL;
>  }
>  EXPORT_SYMBOL(ntb_default_peer_port_number);
>
> @@ -326,4 +322,3 @@ static void __exit ntb_driver_exit(void)
> bus_unregister(_bus);
>  }
>  module_exit(ntb_driver_exit);
> -
> --
> 2.11.0
>

Re: [PATCH 2/8] NTB: Setup the DMA mask globally for all drivers

[Jon Mason]

On Fri, Jun 8, 2018 at 8:08 PM, Logan Gunthorpe  wrote:
> Commit  417cf39cfea9 ("NTB: Set dma mask and dma coherent mask to NTB
> devices") added code to set the DMA mask for the NTB device
> to each driver individually. However, it neglected to set it for the
> Switchtec driver. So when the monolithic commit 7f46c8b3a552 ("NTB:
> ntb_tool: Add full multi-port NTB API support") started allocating
> DMA memory against the NTB device it broke the Switchtec driver.
>
> Seeing this is setting up a property of the NTB device, it should be
> done by the common NTB code (inside ntb_register_device()) so we can be
> sure it's done properly for all drivers. This avoids each driver needing
> to duplicate the code and helps prevent us from inadvertently breaking
> one of the drivers in the future if we have to make changes in this area.
>
> Fixes: 7f46c8b3a552 ("NTB: ntb_tool: Add full multi-port NTB API support")
> Signed-off-by: Logan Gunthorpe 
> ---
>  drivers/ntb/hw/amd/ntb_hw_amd.c |  4 
>  drivers/ntb/hw/idt/ntb_hw_idt.c |  6 --
>  drivers/ntb/hw/intel/ntb_hw_intel.c |  4 
>  drivers/ntb/ntb.c   | 13 -
>  4 files changed, 12 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/ntb/hw/amd/ntb_hw_amd.c b/drivers/ntb/hw/amd/ntb_hw_amd.c
> index 3cfa46876239..f0788aae05c9 100644
> --- a/drivers/ntb/hw/amd/ntb_hw_amd.c
> +++ b/drivers/ntb/hw/amd/ntb_hw_amd.c
> @@ -1020,10 +1020,6 @@ static int amd_ntb_init_pci(struct amd_ntb_dev *ndev,
> goto err_dma_mask;
> dev_warn(>dev, "Cannot DMA consistent highmem\n");
> }
> -   rc = dma_coerce_mask_and_coherent(>ntb.dev,
> - dma_get_mask(>dev));
> -   if (rc)
> -   goto err_dma_mask;
>
> ndev->self_mmio = pci_iomap(pdev, 0, 0);
> if (!ndev->self_mmio) {
> diff --git a/drivers/ntb/hw/idt/ntb_hw_idt.c b/drivers/ntb/hw/idt/ntb_hw_idt.c
> index 8d98872d0983..1918a2db1c43 100644
> --- a/drivers/ntb/hw/idt/ntb_hw_idt.c
> +++ b/drivers/ntb/hw/idt/ntb_hw_idt.c
> @@ -2447,12 +2447,6 @@ static int idt_init_pci(struct idt_ntb_dev *ndev)
> dev_warn(>dev,
> "Cannot set consistent DMA highmem bit mask\n");
> }
> -   ret = dma_coerce_mask_and_coherent(>ntb.dev,
> -  dma_get_mask(>dev));
> -   if (ret != 0) {
> -   dev_err(>dev, "Failed to set NTB device DMA bit 
> mask\n");
> -   return ret;
> -   }
>
> /*
>  * Enable the device advanced error reporting. It's not critical to
> diff --git a/drivers/ntb/hw/intel/ntb_hw_intel.c 
> b/drivers/ntb/hw/intel/ntb_hw_intel.c
> index 156b45cd4a19..341a3d5baa3f 100644
> --- a/drivers/ntb/hw/intel/ntb_hw_intel.c
> +++ b/drivers/ntb/hw/intel/ntb_hw_intel.c
> @@ -2334,10 +2334,6 @@ static int intel_ntb_init_pci(struct intel_ntb_dev 
> *ndev, struct pci_dev *pdev)
> goto err_dma_mask;
> dev_warn(>dev, "Cannot DMA consistent highmem\n");
> }
> -   rc = dma_coerce_mask_and_coherent(>ntb.dev,
> - dma_get_mask(>dev));
> -   if (rc)
> -   goto err_dma_mask;
>
> ndev->self_mmio = pci_iomap(pdev, 0, 0);
> if (!ndev->self_mmio) {
> diff --git a/drivers/ntb/ntb.c b/drivers/ntb/ntb.c
> index 2581ab724c34..93f24440d11d 100644
> --- a/drivers/ntb/ntb.c
> +++ b/drivers/ntb/ntb.c
> @@ -100,6 +100,8 @@ EXPORT_SYMBOL(ntb_unregister_client);
>
>  int ntb_register_device(struct ntb_dev *ntb)
>  {
> +   int ret;
> +
> if (!ntb)
> return -EINVAL;
> if (!ntb->pdev)
> @@ -120,7 +122,16 @@ int ntb_register_device(struct ntb_dev *ntb)
> ntb->ctx_ops = NULL;
> spin_lock_init(>ctx_lock);
>
> -   return device_register(>dev);
> +   device_initialize(>dev);
> +
> +   ret = dma_coerce_mask_and_coherent(>dev,
> +  dma_get_mask(>pdev->dev));

ntb.c is more of a glue layer, and this is more device specific.
While I like adding it here for more common code, it should probably
reside in the ntb_hw_*.c files to enforce the hw specific code all
reside in that layer.  So, this probably needs to be replaced with a
patch which adds the setting of the mask to the switchtec driver.

Thanks,
Jon


> +   if (ret != 0) {
> +   dev_err(>dev, "Failed to set NTB device DMA bit mask\n");
> +   return ret;
> +   }
> +
> +   return device_add(>dev);
>  }
>  EXPORT_SYMBOL(ntb_register_device);
>
> --
> 2.11.0
>

Re: [PATCH 2/8] NTB: Setup the DMA mask globally for all drivers

[Jon Mason]

On Fri, Jun 8, 2018 at 8:08 PM, Logan Gunthorpe  wrote:
> Commit  417cf39cfea9 ("NTB: Set dma mask and dma coherent mask to NTB
> devices") added code to set the DMA mask for the NTB device
> to each driver individually. However, it neglected to set it for the
> Switchtec driver. So when the monolithic commit 7f46c8b3a552 ("NTB:
> ntb_tool: Add full multi-port NTB API support") started allocating
> DMA memory against the NTB device it broke the Switchtec driver.
>
> Seeing this is setting up a property of the NTB device, it should be
> done by the common NTB code (inside ntb_register_device()) so we can be
> sure it's done properly for all drivers. This avoids each driver needing
> to duplicate the code and helps prevent us from inadvertently breaking
> one of the drivers in the future if we have to make changes in this area.
>
> Fixes: 7f46c8b3a552 ("NTB: ntb_tool: Add full multi-port NTB API support")
> Signed-off-by: Logan Gunthorpe 
> ---
>  drivers/ntb/hw/amd/ntb_hw_amd.c |  4 
>  drivers/ntb/hw/idt/ntb_hw_idt.c |  6 --
>  drivers/ntb/hw/intel/ntb_hw_intel.c |  4 
>  drivers/ntb/ntb.c   | 13 -
>  4 files changed, 12 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/ntb/hw/amd/ntb_hw_amd.c b/drivers/ntb/hw/amd/ntb_hw_amd.c
> index 3cfa46876239..f0788aae05c9 100644
> --- a/drivers/ntb/hw/amd/ntb_hw_amd.c
> +++ b/drivers/ntb/hw/amd/ntb_hw_amd.c
> @@ -1020,10 +1020,6 @@ static int amd_ntb_init_pci(struct amd_ntb_dev *ndev,
> goto err_dma_mask;
> dev_warn(>dev, "Cannot DMA consistent highmem\n");
> }
> -   rc = dma_coerce_mask_and_coherent(>ntb.dev,
> - dma_get_mask(>dev));
> -   if (rc)
> -   goto err_dma_mask;
>
> ndev->self_mmio = pci_iomap(pdev, 0, 0);
> if (!ndev->self_mmio) {
> diff --git a/drivers/ntb/hw/idt/ntb_hw_idt.c b/drivers/ntb/hw/idt/ntb_hw_idt.c
> index 8d98872d0983..1918a2db1c43 100644
> --- a/drivers/ntb/hw/idt/ntb_hw_idt.c
> +++ b/drivers/ntb/hw/idt/ntb_hw_idt.c
> @@ -2447,12 +2447,6 @@ static int idt_init_pci(struct idt_ntb_dev *ndev)
> dev_warn(>dev,
> "Cannot set consistent DMA highmem bit mask\n");
> }
> -   ret = dma_coerce_mask_and_coherent(>ntb.dev,
> -  dma_get_mask(>dev));
> -   if (ret != 0) {
> -   dev_err(>dev, "Failed to set NTB device DMA bit 
> mask\n");
> -   return ret;
> -   }
>
> /*
>  * Enable the device advanced error reporting. It's not critical to
> diff --git a/drivers/ntb/hw/intel/ntb_hw_intel.c 
> b/drivers/ntb/hw/intel/ntb_hw_intel.c
> index 156b45cd4a19..341a3d5baa3f 100644
> --- a/drivers/ntb/hw/intel/ntb_hw_intel.c
> +++ b/drivers/ntb/hw/intel/ntb_hw_intel.c
> @@ -2334,10 +2334,6 @@ static int intel_ntb_init_pci(struct intel_ntb_dev 
> *ndev, struct pci_dev *pdev)
> goto err_dma_mask;
> dev_warn(>dev, "Cannot DMA consistent highmem\n");
> }
> -   rc = dma_coerce_mask_and_coherent(>ntb.dev,
> - dma_get_mask(>dev));
> -   if (rc)
> -   goto err_dma_mask;
>
> ndev->self_mmio = pci_iomap(pdev, 0, 0);
> if (!ndev->self_mmio) {
> diff --git a/drivers/ntb/ntb.c b/drivers/ntb/ntb.c
> index 2581ab724c34..93f24440d11d 100644
> --- a/drivers/ntb/ntb.c
> +++ b/drivers/ntb/ntb.c
> @@ -100,6 +100,8 @@ EXPORT_SYMBOL(ntb_unregister_client);
>
>  int ntb_register_device(struct ntb_dev *ntb)
>  {
> +   int ret;
> +
> if (!ntb)
> return -EINVAL;
> if (!ntb->pdev)
> @@ -120,7 +122,16 @@ int ntb_register_device(struct ntb_dev *ntb)
> ntb->ctx_ops = NULL;
> spin_lock_init(>ctx_lock);
>
> -   return device_register(>dev);
> +   device_initialize(>dev);
> +
> +   ret = dma_coerce_mask_and_coherent(>dev,
> +  dma_get_mask(>pdev->dev));

ntb.c is more of a glue layer, and this is more device specific.
While I like adding it here for more common code, it should probably
reside in the ntb_hw_*.c files to enforce the hw specific code all
reside in that layer.  So, this probably needs to be replaced with a
patch which adds the setting of the mask to the switchtec driver.

Thanks,
Jon


> +   if (ret != 0) {
> +   dev_err(>dev, "Failed to set NTB device DMA bit mask\n");
> +   return ret;
> +   }
> +
> +   return device_add(>dev);
>  }
>  EXPORT_SYMBOL(ntb_register_device);
>
> --
> 2.11.0
>

[GIT PULL] NTB changes for v4.18

[Jon Mason]

Hello Linus,
Here are a few NTB changes for v4.18.  They were tested earlier today by
Dave Jiang, and have been in linux-next for some time.  Please consider
pulling them.

Thanks,
Jon



The following changes since commit 29dcea88779c856c7dc92040a0c01233263101d4:

  Linux 4.17 (2018-06-03 14:15:21 -0700)

are available in the Git repository at:

  git://github.com/jonmason/ntb tags/ntb-4.18

for you to fetch changes up to c9160b69258ef46ab62c27a09decb8fef311e700:

  ntb: ntb_transport: Replace GFP_ATOMIC with GFP_KERNEL in 
ntb_transport_create_queue (2018-06-11 15:20:59 -0400)


Reorg and clean-up of the Intel NTB driver, a trivial comment change,
and changes of GFP_ATOMIC to GFP_KERNEL where appropriate.


Dave Jiang (3):
  ntb: intel: header definitions refactor
  ntb: intel: split out the gen3 code
  ntb: intel: change references of skx to gen3

Jia-Ju Bai (2):
  ntb: ntb_transport: Replace GFP_ATOMIC with GFP_KERNEL in 
ntb_transport_setup_qp_mw
  ntb: ntb_transport: Replace GFP_ATOMIC with GFP_KERNEL in 
ntb_transport_create_queue

Wolfram Sang (1):
  NTB: ntb_hw_idt: fix typo 'can by' to 'can be'

 drivers/ntb/hw/idt/ntb_hw_idt.c|   2 +-
 drivers/ntb/hw/intel/Makefile  |   1 +
 .../ntb/hw/intel/{ntb_hw_intel.c => ntb_hw_gen1.c} | 713 ++---
 drivers/ntb/hw/intel/ntb_hw_gen1.h | 182 ++
 drivers/ntb/hw/intel/ntb_hw_gen3.c | 597 +
 drivers/ntb/hw/intel/ntb_hw_gen3.h | 110 
 drivers/ntb/hw/intel/ntb_hw_intel.h| 203 ++
 drivers/ntb/ntb_transport.c|   6 +-
 8 files changed, 1001 insertions(+), 813 deletions(-)
 rename drivers/ntb/hw/intel/{ntb_hw_intel.c => ntb_hw_gen1.c} (74%)
 create mode 100644 drivers/ntb/hw/intel/ntb_hw_gen1.h
 create mode 100644 drivers/ntb/hw/intel/ntb_hw_gen3.c
 create mode 100644 drivers/ntb/hw/intel/ntb_hw_gen3.h

[GIT PULL] NTB changes for v4.18

[Jon Mason]

Hello Linus,
Here are a few NTB changes for v4.18.  They were tested earlier today by
Dave Jiang, and have been in linux-next for some time.  Please consider
pulling them.

Thanks,
Jon



The following changes since commit 29dcea88779c856c7dc92040a0c01233263101d4:

  Linux 4.17 (2018-06-03 14:15:21 -0700)

are available in the Git repository at:

  git://github.com/jonmason/ntb tags/ntb-4.18

for you to fetch changes up to c9160b69258ef46ab62c27a09decb8fef311e700:

  ntb: ntb_transport: Replace GFP_ATOMIC with GFP_KERNEL in 
ntb_transport_create_queue (2018-06-11 15:20:59 -0400)


Reorg and clean-up of the Intel NTB driver, a trivial comment change,
and changes of GFP_ATOMIC to GFP_KERNEL where appropriate.


Dave Jiang (3):
  ntb: intel: header definitions refactor
  ntb: intel: split out the gen3 code
  ntb: intel: change references of skx to gen3

Jia-Ju Bai (2):
  ntb: ntb_transport: Replace GFP_ATOMIC with GFP_KERNEL in 
ntb_transport_setup_qp_mw
  ntb: ntb_transport: Replace GFP_ATOMIC with GFP_KERNEL in 
ntb_transport_create_queue

Wolfram Sang (1):
  NTB: ntb_hw_idt: fix typo 'can by' to 'can be'

 drivers/ntb/hw/idt/ntb_hw_idt.c|   2 +-
 drivers/ntb/hw/intel/Makefile  |   1 +
 .../ntb/hw/intel/{ntb_hw_intel.c => ntb_hw_gen1.c} | 713 ++---
 drivers/ntb/hw/intel/ntb_hw_gen1.h | 182 ++
 drivers/ntb/hw/intel/ntb_hw_gen3.c | 597 +
 drivers/ntb/hw/intel/ntb_hw_gen3.h | 110 
 drivers/ntb/hw/intel/ntb_hw_intel.h| 203 ++
 drivers/ntb/ntb_transport.c|   6 +-
 8 files changed, 1001 insertions(+), 813 deletions(-)
 rename drivers/ntb/hw/intel/{ntb_hw_intel.c => ntb_hw_gen1.c} (74%)
 create mode 100644 drivers/ntb/hw/intel/ntb_hw_gen1.h
 create mode 100644 drivers/ntb/hw/intel/ntb_hw_gen3.c
 create mode 100644 drivers/ntb/hw/intel/ntb_hw_gen3.h

Re: [PATCH] ntb_transport: use put_device() instead of kfree()

[Jon Mason]

On Fri, Mar 09, 2018 at 04:03:24PM +0530, Arvind Yadav wrote:
> Never directly free @dev after calling device_register(), even
> if it returned an error! Always use put_device() to give up the
> reference initialized.
> 
> Signed-off-by: Arvind Yadav 
> ---
>  drivers/ntb/ntb_transport.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
> index 9878c48..8182a3a 100644
> --- a/drivers/ntb/ntb_transport.c
> +++ b/drivers/ntb/ntb_transport.c
> @@ -393,7 +393,7 @@ int ntb_transport_register_client_dev(char *device_name)
>  
>   rc = device_register(dev);
>   if (rc) {
> - kfree(client_dev);
> + put_device(dev);

Now we are leaking client_dev, which is bigger than just dev.  I think
we are going to need both now.

Thanks,
Jon

>   goto err;
>   }
>  
> -- 
> 1.9.1
> 

Re: [PATCH] ntb_transport: use put_device() instead of kfree()

[Jon Mason]

On Fri, Mar 09, 2018 at 04:03:24PM +0530, Arvind Yadav wrote:
> Never directly free @dev after calling device_register(), even
> if it returned an error! Always use put_device() to give up the
> reference initialized.
> 
> Signed-off-by: Arvind Yadav 
> ---
>  drivers/ntb/ntb_transport.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
> index 9878c48..8182a3a 100644
> --- a/drivers/ntb/ntb_transport.c
> +++ b/drivers/ntb/ntb_transport.c
> @@ -393,7 +393,7 @@ int ntb_transport_register_client_dev(char *device_name)
>  
>   rc = device_register(dev);
>   if (rc) {
> - kfree(client_dev);
> + put_device(dev);

Now we are leaking client_dev, which is bigger than just dev.  I think
we are going to need both now.

Thanks,
Jon

>   goto err;
>   }
>  
> -- 
> 1.9.1
> 

Re: [PATCH 6/9] NTB: ntb_hw_idt: fix typo 'can by' to 'can be'

[Jon Mason]

On Fri, May 11, 2018 at 04:12:36PM +0300, Serge Semin wrote:
> On Sun, May 06, 2018 at 01:23:50PM +0200, Wolfram Sang 
>  wrote:
> > Signed-off-by: Wolfram Sang 
> > ---
> >  drivers/ntb/hw/idt/ntb_hw_idt.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/ntb/hw/idt/ntb_hw_idt.c 
> > b/drivers/ntb/hw/idt/ntb_hw_idt.c
> > index 8d98872d0983b7..dbe72f116017ab 100644
> > --- a/drivers/ntb/hw/idt/ntb_hw_idt.c
> > +++ b/drivers/ntb/hw/idt/ntb_hw_idt.c
> > @@ -1401,7 +1401,7 @@ static int idt_ntb_peer_mw_clear_trans(struct ntb_dev 
> > *ntb, int pidx,
> >   *  5. Doorbell operations
> >   *
> >   *Doorbell functionality of IDT PCIe-switches is pretty unusual. First 
> > of
> > - * all there is global doorbell register which state can by changed by any
> > + * all there is global doorbell register which state can be changed by any
> >   * NT-function of the IDT device in accordance with global permissions. 
> > These
> >   * permissions configs are not supported by NTB API, so it must be done by
> >   * either BIOS or EEPROM settings. In the same way the state of the global
> 
> Acked-by: Serge Semin 

Applied to my ntb-next branch.

Thanks,
Jon


> 
> > -- 
> > 2.11.0
> > 

Re: [PATCH 2/2] ntb: ntb_transport: Replace GFP_ATOMIC with GFP_KERNEL in ntb_transport_create_queue

[Jon Mason]

On Tue, Apr 10, 2018 at 09:17:54PM +0800, Jia-Ju Bai wrote:
> ntb_transport_create_queue() is never called in atomic context.
> 
> ntb_transport_create_queue() is only called by ntb_netdev_probe(),
> which is set as ".probe" in struct ntb_transport_client.
> 
> Despite never getting called from atomic context,
> ntb_transport_create_queue() calls kzalloc_node() with GFP_ATOMIC,
> which does not sleep for allocation.
> GFP_ATOMIC is not necessary and can be replaced with GFP_KERNEL,
> which can sleep and improve the possibility of sucessful allocation.
> 
> This is found by a static analysis tool named DCNS written by myself.
> And I also manually check it
> 
> Signed-off-by: Jia-Ju Bai 

Both patches have been applied to my ntb-next branch.

Thanks,
Jon

> ---
>  drivers/ntb/ntb_transport.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
> index f58d8e3..2c0c8bc 100644
> --- a/drivers/ntb/ntb_transport.c
> +++ b/drivers/ntb/ntb_transport.c
> @@ -1825,7 +1825,7 @@ struct ntb_transport_qp *
>   qp->rx_dma_chan ? "DMA" : "CPU");
>  
>   for (i = 0; i < NTB_QP_DEF_NUM_ENTRIES; i++) {
> - entry = kzalloc_node(sizeof(*entry), GFP_ATOMIC, node);
> + entry = kzalloc_node(sizeof(*entry), GFP_KERNEL, node);
>   if (!entry)
>   goto err1;
>  
> @@ -1836,7 +1836,7 @@ struct ntb_transport_qp *
>   qp->rx_alloc_entry = NTB_QP_DEF_NUM_ENTRIES;
>  
>   for (i = 0; i < qp->tx_max_entry; i++) {
> - entry = kzalloc_node(sizeof(*entry), GFP_ATOMIC, node);
> + entry = kzalloc_node(sizeof(*entry), GFP_KERNEL, node);
>   if (!entry)
>   goto err2;
>  
> -- 
> 1.9.1
> 

Re: [PATCH 2/2] ntb: ntb_transport: Replace GFP_ATOMIC with GFP_KERNEL in ntb_transport_create_queue

[Jon Mason]

On Tue, Apr 10, 2018 at 09:17:54PM +0800, Jia-Ju Bai wrote:
> ntb_transport_create_queue() is never called in atomic context.
> 
> ntb_transport_create_queue() is only called by ntb_netdev_probe(),
> which is set as ".probe" in struct ntb_transport_client.
> 
> Despite never getting called from atomic context,
> ntb_transport_create_queue() calls kzalloc_node() with GFP_ATOMIC,
> which does not sleep for allocation.
> GFP_ATOMIC is not necessary and can be replaced with GFP_KERNEL,
> which can sleep and improve the possibility of sucessful allocation.
> 
> This is found by a static analysis tool named DCNS written by myself.
> And I also manually check it
> 
> Signed-off-by: Jia-Ju Bai 

Both patches have been applied to my ntb-next branch.

Thanks,
Jon

> ---
>  drivers/ntb/ntb_transport.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
> index f58d8e3..2c0c8bc 100644
> --- a/drivers/ntb/ntb_transport.c
> +++ b/drivers/ntb/ntb_transport.c
> @@ -1825,7 +1825,7 @@ struct ntb_transport_qp *
>   qp->rx_dma_chan ? "DMA" : "CPU");
>  
>   for (i = 0; i < NTB_QP_DEF_NUM_ENTRIES; i++) {
> - entry = kzalloc_node(sizeof(*entry), GFP_ATOMIC, node);
> + entry = kzalloc_node(sizeof(*entry), GFP_KERNEL, node);
>   if (!entry)
>   goto err1;
>  
> @@ -1836,7 +1836,7 @@ struct ntb_transport_qp *
>   qp->rx_alloc_entry = NTB_QP_DEF_NUM_ENTRIES;
>  
>   for (i = 0; i < qp->tx_max_entry; i++) {
> - entry = kzalloc_node(sizeof(*entry), GFP_ATOMIC, node);
> + entry = kzalloc_node(sizeof(*entry), GFP_KERNEL, node);
>   if (!entry)
>   goto err2;
>  
> -- 
> 1.9.1
> 

Re: [PATCH 6/9] NTB: ntb_hw_idt: fix typo 'can by' to 'can be'

[Jon Mason]

On Fri, May 11, 2018 at 04:12:36PM +0300, Serge Semin wrote:
> On Sun, May 06, 2018 at 01:23:50PM +0200, Wolfram Sang 
>  wrote:
> > Signed-off-by: Wolfram Sang 
> > ---
> >  drivers/ntb/hw/idt/ntb_hw_idt.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/ntb/hw/idt/ntb_hw_idt.c 
> > b/drivers/ntb/hw/idt/ntb_hw_idt.c
> > index 8d98872d0983b7..dbe72f116017ab 100644
> > --- a/drivers/ntb/hw/idt/ntb_hw_idt.c
> > +++ b/drivers/ntb/hw/idt/ntb_hw_idt.c
> > @@ -1401,7 +1401,7 @@ static int idt_ntb_peer_mw_clear_trans(struct ntb_dev 
> > *ntb, int pidx,
> >   *  5. Doorbell operations
> >   *
> >   *Doorbell functionality of IDT PCIe-switches is pretty unusual. First 
> > of
> > - * all there is global doorbell register which state can by changed by any
> > + * all there is global doorbell register which state can be changed by any
> >   * NT-function of the IDT device in accordance with global permissions. 
> > These
> >   * permissions configs are not supported by NTB API, so it must be done by
> >   * either BIOS or EEPROM settings. In the same way the state of the global
> 
> Acked-by: Serge Semin 

Applied to my ntb-next branch.

Thanks,
Jon


> 
> > -- 
> > 2.11.0
> > 

Re: [PATCH] ARM: dts: NSP: Fix amount of RAM on BCM958625HR

[Jon Mason]

On Mon, Feb 26, 2018 at 8:03 PM, Florian Fainelli <f.faine...@gmail.com> wrote:
> Jon attempted to fix the amount of RAM on the BCM958625HR in commit
> c53beb47f621 ("ARM: dts: NSP: Correct RAM amount for BCM958625HR board")
> but it seems like we tripped over some poorly documented schematics.
>
> The top-level page of the schematics says the board has 2GB, but when
> you end-up scrolling to page 6, you see two chips of 4GiB (512MB) but
> what the bootloader really initializes only 512MB, any attempt to use
> more than that results in data aborts. Fix this again back to 512MB.
>
> Fixes: c53beb47f621 ("ARM: dts: NSP: Correct RAM amount for BCM958625HR 
> board")
> Signed-off-by: Florian Fainelli <f.faine...@gmail.com>

Acked-by: Jon Mason <jon.ma...@broadcom.com>

> ---
>  arch/arm/boot/dts/bcm958625hr.dts | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm/boot/dts/bcm958625hr.dts 
> b/arch/arm/boot/dts/bcm958625hr.dts
> index 6a44b8021702..f0e2008f7490 100644
> --- a/arch/arm/boot/dts/bcm958625hr.dts
> +++ b/arch/arm/boot/dts/bcm958625hr.dts
> @@ -49,7 +49,7 @@
>
> memory {
> device_type = "memory";
> -   reg = <0x6000 0x8000>;
> +   reg = <0x6000 0x2000>;
> };
>
> gpio-restart {
> --
> 2.14.1
>

Re: [PATCH] ARM: dts: NSP: Fix amount of RAM on BCM958625HR

[Jon Mason]

On Mon, Feb 26, 2018 at 8:03 PM, Florian Fainelli  wrote:
> Jon attempted to fix the amount of RAM on the BCM958625HR in commit
> c53beb47f621 ("ARM: dts: NSP: Correct RAM amount for BCM958625HR board")
> but it seems like we tripped over some poorly documented schematics.
>
> The top-level page of the schematics says the board has 2GB, but when
> you end-up scrolling to page 6, you see two chips of 4GiB (512MB) but
> what the bootloader really initializes only 512MB, any attempt to use
> more than that results in data aborts. Fix this again back to 512MB.
>
> Fixes: c53beb47f621 ("ARM: dts: NSP: Correct RAM amount for BCM958625HR 
> board")
> Signed-off-by: Florian Fainelli 

Acked-by: Jon Mason 

> ---
>  arch/arm/boot/dts/bcm958625hr.dts | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm/boot/dts/bcm958625hr.dts 
> b/arch/arm/boot/dts/bcm958625hr.dts
> index 6a44b8021702..f0e2008f7490 100644
> --- a/arch/arm/boot/dts/bcm958625hr.dts
> +++ b/arch/arm/boot/dts/bcm958625hr.dts
> @@ -49,7 +49,7 @@
>
> memory {
> device_type = "memory";
> -   reg = <0x6000 0x8000>;
> +   reg = <0x6000 0x2000>;
> };
>
> gpio-restart {
> --
> 2.14.1
>

[GIT PULL] NTB bug fixes for v4.16

[Jon Mason]

Hello Linus,
Here are a few NTB bug fixes, removal of a driver, and updated to the
NTB tools to take advants of the multiport interface.  They've been in
linux-next for a little while.  Please consider pulling them for 4.16.

Thanks,
Jon



The following changes since commit d8a5b80568a9cb66810e75b182018e9edb68e8ff:

  Linux 4.15 (2018-01-28 13:20:33 -0800)

are available in the Git repository at:

  git://github.com/jonmason/ntb tags/ntb-4.16

for you to fetch changes up to 3b28c987fb9547ca9aac73241d0e281cf646387c:

  NTB: ntb_perf: fix cast to restricted __le32 (2018-01-28 22:17:24 -0500)


Bug fixes galore, removal of the ntb atom driver, and updates to the ntb
tools and tests to support the multi-port interface


Allen Hubbe (1):
  MAINTAINERS: NTB: Update contact info

Arnd Bergmann (2):
  ntb_hw_switchtec: fix logic error
  NTB: ntb_perf: fix printing of resource_size_t

Colin Ian King (2):
  NTB: switchtec_ntb: fix spelling mistake: "peforming" -> "performing"
  NTB: ntb_tool: fix memory leak on 'buf' on error exit path

Dan Carpenter (1):
  ntb_perf: Fix an error code in perf_copy_chunk()

Dave Jiang (1):
  ntb: remove Intel Atom NTB driver support

Doug Meyer (1):
  NTB: ntb_hw_switchtec: Fix peer BAR bug in switchtec_ntb_init_shared_mw

Greg Kroah-Hartman (1):
  ntb: remove unneeded DRIVER_LICENSE #defines

Jon Mason (1):
  NTB: switchtec_ntb: Add new line on appropriate printks

Kelvin Cao (1):
  ntb_hw_switchtec: Allow using Switchtec NTB in multi-partition setups

Logan Gunthorpe (9):
  ntb_hw_switchtec: Keep track of the number of LUT windows used by the 
driver
  ntb_hw_switchtec: Create helper function to setup reserved LUT MWs
  ntb_hw_switchtec: Make switchtec_ntb_init_req_id_table() more general
  ntb_hw_switchtec: Expand PFF CSR registers
  ntb_hw_switchtec: Add initialization code for crosslink
  ntb_hw_switchtec: Crosslink doorbells and messages
  ntb_hw_switchtec: Force down the link before initializing
  ntb_transport: Fix bug with max_mw_size parameter
  ntb_hw_switchtec: Check for alignment of the buffer in mw_set_trans()

Serge Semin (16):
  NTB: Rename NTB messaging API methods
  NTB: Set dma mask and dma coherent mask to NTB devices
  NTB: Fix UB/bug in ntb_mw_get_align()
  NTB: ntb_pp: Add full multi-port NTB API support
  NTB: ntb_tool: Add full multi-port NTB API support
  NTB: ntb_perf: Add full multi-port NTB API support
  NTB: ntb_test: Safely use paths with whitespace
  NTB: ntb_test: Add ntb_tool port tests
  NTB: ntb_test: Update ntb_tool link tests
  NTB: ntb_test: Update ntb_tool DB tests
  NTB: ntb_test: Update ntb_tool Scratchpad tests
  NTB: ntb_test: Add ntb_tool Message tests
  NTB: ntb_test: Update ntb_tool MW tests
  NTB: ntb_test: Update ntb_perf tests
  NTB: ntb_hw_idt: Set NTB_TOPO_SWITCH topology
  NTB: ntb_perf: fix cast to restricted __le32

Wei Yongjun (1):
  ntb_hw_switchtec: Make function switchtec_ntb_remove() static

 MAINTAINERS |2 +-
 drivers/ntb/hw/amd/ntb_hw_amd.c |4 +
 drivers/ntb/hw/idt/ntb_hw_idt.c |   37 +-
 drivers/ntb/hw/intel/ntb_hw_intel.c |  313 +-
 drivers/ntb/hw/intel/ntb_hw_intel.h |   58 -
 drivers/ntb/hw/mscc/ntb_hw_switchtec.c  |  603 --
 drivers/ntb/ntb.c   |4 +-
 drivers/ntb/ntb_transport.c |3 +
 drivers/ntb/test/ntb_perf.c | 1824 --
 drivers/ntb/test/ntb_pingpong.c |  450 +---
 drivers/ntb/test/ntb_tool.c | 1827 +--
 include/linux/ntb.h |   51 +-
 include/linux/switchtec.h   |   23 +-
 tools/testing/selftests/ntb/ntb_test.sh |  307 --
 14 files changed, 3551 insertions(+), 1955 deletions(-)

[GIT PULL] NTB bug fixes for v4.16

[Jon Mason]

Hello Linus,
Here are a few NTB bug fixes, removal of a driver, and updated to the
NTB tools to take advants of the multiport interface.  They've been in
linux-next for a little while.  Please consider pulling them for 4.16.

Thanks,
Jon



The following changes since commit d8a5b80568a9cb66810e75b182018e9edb68e8ff:

  Linux 4.15 (2018-01-28 13:20:33 -0800)

are available in the Git repository at:

  git://github.com/jonmason/ntb tags/ntb-4.16

for you to fetch changes up to 3b28c987fb9547ca9aac73241d0e281cf646387c:

  NTB: ntb_perf: fix cast to restricted __le32 (2018-01-28 22:17:24 -0500)


Bug fixes galore, removal of the ntb atom driver, and updates to the ntb
tools and tests to support the multi-port interface


Allen Hubbe (1):
  MAINTAINERS: NTB: Update contact info

Arnd Bergmann (2):
  ntb_hw_switchtec: fix logic error
  NTB: ntb_perf: fix printing of resource_size_t

Colin Ian King (2):
  NTB: switchtec_ntb: fix spelling mistake: "peforming" -> "performing"
  NTB: ntb_tool: fix memory leak on 'buf' on error exit path

Dan Carpenter (1):
  ntb_perf: Fix an error code in perf_copy_chunk()

Dave Jiang (1):
  ntb: remove Intel Atom NTB driver support

Doug Meyer (1):
  NTB: ntb_hw_switchtec: Fix peer BAR bug in switchtec_ntb_init_shared_mw

Greg Kroah-Hartman (1):
  ntb: remove unneeded DRIVER_LICENSE #defines

Jon Mason (1):
  NTB: switchtec_ntb: Add new line on appropriate printks

Kelvin Cao (1):
  ntb_hw_switchtec: Allow using Switchtec NTB in multi-partition setups

Logan Gunthorpe (9):
  ntb_hw_switchtec: Keep track of the number of LUT windows used by the 
driver
  ntb_hw_switchtec: Create helper function to setup reserved LUT MWs
  ntb_hw_switchtec: Make switchtec_ntb_init_req_id_table() more general
  ntb_hw_switchtec: Expand PFF CSR registers
  ntb_hw_switchtec: Add initialization code for crosslink
  ntb_hw_switchtec: Crosslink doorbells and messages
  ntb_hw_switchtec: Force down the link before initializing
  ntb_transport: Fix bug with max_mw_size parameter
  ntb_hw_switchtec: Check for alignment of the buffer in mw_set_trans()

Serge Semin (16):
  NTB: Rename NTB messaging API methods
  NTB: Set dma mask and dma coherent mask to NTB devices
  NTB: Fix UB/bug in ntb_mw_get_align()
  NTB: ntb_pp: Add full multi-port NTB API support
  NTB: ntb_tool: Add full multi-port NTB API support
  NTB: ntb_perf: Add full multi-port NTB API support
  NTB: ntb_test: Safely use paths with whitespace
  NTB: ntb_test: Add ntb_tool port tests
  NTB: ntb_test: Update ntb_tool link tests
  NTB: ntb_test: Update ntb_tool DB tests
  NTB: ntb_test: Update ntb_tool Scratchpad tests
  NTB: ntb_test: Add ntb_tool Message tests
  NTB: ntb_test: Update ntb_tool MW tests
  NTB: ntb_test: Update ntb_perf tests
  NTB: ntb_hw_idt: Set NTB_TOPO_SWITCH topology
  NTB: ntb_perf: fix cast to restricted __le32

Wei Yongjun (1):
  ntb_hw_switchtec: Make function switchtec_ntb_remove() static

 MAINTAINERS |2 +-
 drivers/ntb/hw/amd/ntb_hw_amd.c |4 +
 drivers/ntb/hw/idt/ntb_hw_idt.c |   37 +-
 drivers/ntb/hw/intel/ntb_hw_intel.c |  313 +-
 drivers/ntb/hw/intel/ntb_hw_intel.h |   58 -
 drivers/ntb/hw/mscc/ntb_hw_switchtec.c  |  603 --
 drivers/ntb/ntb.c   |4 +-
 drivers/ntb/ntb_transport.c |3 +
 drivers/ntb/test/ntb_perf.c | 1824 --
 drivers/ntb/test/ntb_pingpong.c |  450 +---
 drivers/ntb/test/ntb_tool.c | 1827 +--
 include/linux/ntb.h |   51 +-
 include/linux/switchtec.h   |   23 +-
 tools/testing/selftests/ntb/ntb_test.sh |  307 --
 14 files changed, 3551 insertions(+), 1955 deletions(-)

Re: [PATCH v2] NTB: ntb_perf: fix cast to restricted __le32

[Jon Mason]

On Wed, Jan 24, 2018 at 09:07:26AM +0100, Arnd Bergmann wrote:
> On Wed, Jan 24, 2018 at 8:48 AM, Serge Semin  wrote:
> > Sparse is whining about the u32 and __le32 mixed usage in the driver
> >
> > drivers/ntb/test/ntb_perf.c:288:21: warning: cast to restricted __le32
> > drivers/ntb/test/ntb_perf.c:295:37: warning: incorrect type in argument 4 
> > (different base types)
> > drivers/ntb/test/ntb_perf.c:295:37:expected unsigned int [unsigned] 
> > [usertype] val
> > drivers/ntb/test/ntb_perf.c:295:37:got restricted __le32 [usertype] 
> > 
> > ...
> >
> > NTB hardware drivers shall accept CPU-endian data and translate it to
> > the portable formate by internal means, so the explicit conversions
> > are not necessary before Scratchpad/Messages API usage anymore.
> >
> > Fixes: b83003b3fdc1 ("NTB: ntb_perf: Add full multi-port NTB API support")
> > Signed-off-by: Serge Semin 
> 
> Looks good to me,
> 
> Acked-by: Arnd Bergmann 

Applied to my ntb-next branch.  Thanks for all of the help on this.

Thanks,
Jon

Re: [PATCH v2] NTB: ntb_perf: fix cast to restricted __le32

[Jon Mason]

On Wed, Jan 24, 2018 at 09:07:26AM +0100, Arnd Bergmann wrote:
> On Wed, Jan 24, 2018 at 8:48 AM, Serge Semin  wrote:
> > Sparse is whining about the u32 and __le32 mixed usage in the driver
> >
> > drivers/ntb/test/ntb_perf.c:288:21: warning: cast to restricted __le32
> > drivers/ntb/test/ntb_perf.c:295:37: warning: incorrect type in argument 4 
> > (different base types)
> > drivers/ntb/test/ntb_perf.c:295:37:expected unsigned int [unsigned] 
> > [usertype] val
> > drivers/ntb/test/ntb_perf.c:295:37:got restricted __le32 [usertype] 
> > 
> > ...
> >
> > NTB hardware drivers shall accept CPU-endian data and translate it to
> > the portable formate by internal means, so the explicit conversions
> > are not necessary before Scratchpad/Messages API usage anymore.
> >
> > Fixes: b83003b3fdc1 ("NTB: ntb_perf: Add full multi-port NTB API support")
> > Signed-off-by: Serge Semin 
> 
> Looks good to me,
> 
> Acked-by: Arnd Bergmann 

Applied to my ntb-next branch.  Thanks for all of the help on this.

Thanks,
Jon

Re: [PATCH] NTB: ntb_perf: fix cast to restricted __le32

[Jon Mason]

On Fri, Jan 19, 2018 at 10:26:37PM +0100, Arnd Bergmann wrote:
> On Fri, Jan 19, 2018 at 10:03 PM, Serge Semin  wrote:
> >
> > Actually the provided patch is the best solution I could come up with.
> > The thing is, that the methods can't be changed. Those functions are
> > the part of the NTB API methods used by many drivers. So basically they
> > are like pci_{read,write}_config_{byte,word,dword}() methods. We can't
> > change their prototypes only because it's suit some driver. The methods
> > give an access to the NTB device dummy u32-sized registers, nothing
> > else. So endianness is the transmitted data settings in this case.
> >
> > NTB is the technology to interconnect some two systems with possibly
> > different endianness (unlike PCI, which interconnect CPU with LE devices).
> > In this case I'd need to set some agreement up between two systems about
> > the endianness of the exchanged data like host and network types in
> > Linux networking. I've chosen the network data to be little-endian,
> > that's why I needed first to convert them from CPU to le32, then on
> > remote side convert them back from le32 to CPU.
> >
> > If you have any better suggestion how the warning can be fixed, I'd
> > be glad to stick to it.
> 
> I don't think your description matches what you actually do: The
> underlying ntb hardware drivers (amd, idt, intel, mscc) all treat the
> incoming data as CPU-endian and convert it to little-endian on
> the register side, so the framework already assumes that whatever
> you do here uses a little-endian wire-level protocol.
> 
> On a little-endian kernel/CPU, nothing is ever swapped here, neither
> in the ntb_perf front-end nor in the back-ends. On a big-endian
> kernel/CPU, they both swap, so you end up with CPU-endian
> data on the wire, so it should be impossible for a big-endian
> system to talk to a little-endian one. Have you actually tried that
> combination with the current code?

I do not believe anyone has every tried NTB on a big endien system,
let alone tried it with one side LE and the other BE.  To my
knowledge, this has only ever been used on x86 to x86.

> If my interpretation is correct, then the best solution would be to
> completely remove the cpu_to_le32/le32_to_cpu conversions
> from ntb_perf, and just define that it works like any other PCI
> device, exchanging little-endian data.

Yes, this would be the best solution.  Thank you for the insight.

Serge, when you get a chance, please make this change and resumbit.

Thanks,
Jon


> 
> There are two interesting cases to consider though:
> 
> - if someone wants to implement an NTB based protocol
>   using big-endian data on the wire, you probably want to add
>   a ntb_peer_spad_read_be()/ntb_peer_msg_write_be()
>   set of interfaces, to go along with ioread32_be()/iowrite32_be()
>   the same way that ntb_peer_spad_read()/ntb_peer_msg_write()
>   ends up doing ioread32()/iowrite32() with the implied little-endian
>   behavior.
> 
> - memcpy_toio()/memcpy_fromio() and ioread32_rep()/iowrite32_rep
>   importantly do not do any byteswap, they are meant to
>   transfer byte streams.
> 
> Arnd

Re: [PATCH] NTB: ntb_perf: fix cast to restricted __le32

[Jon Mason]

On Fri, Jan 19, 2018 at 10:26:37PM +0100, Arnd Bergmann wrote:
> On Fri, Jan 19, 2018 at 10:03 PM, Serge Semin  wrote:
> >
> > Actually the provided patch is the best solution I could come up with.
> > The thing is, that the methods can't be changed. Those functions are
> > the part of the NTB API methods used by many drivers. So basically they
> > are like pci_{read,write}_config_{byte,word,dword}() methods. We can't
> > change their prototypes only because it's suit some driver. The methods
> > give an access to the NTB device dummy u32-sized registers, nothing
> > else. So endianness is the transmitted data settings in this case.
> >
> > NTB is the technology to interconnect some two systems with possibly
> > different endianness (unlike PCI, which interconnect CPU with LE devices).
> > In this case I'd need to set some agreement up between two systems about
> > the endianness of the exchanged data like host and network types in
> > Linux networking. I've chosen the network data to be little-endian,
> > that's why I needed first to convert them from CPU to le32, then on
> > remote side convert them back from le32 to CPU.
> >
> > If you have any better suggestion how the warning can be fixed, I'd
> > be glad to stick to it.
> 
> I don't think your description matches what you actually do: The
> underlying ntb hardware drivers (amd, idt, intel, mscc) all treat the
> incoming data as CPU-endian and convert it to little-endian on
> the register side, so the framework already assumes that whatever
> you do here uses a little-endian wire-level protocol.
> 
> On a little-endian kernel/CPU, nothing is ever swapped here, neither
> in the ntb_perf front-end nor in the back-ends. On a big-endian
> kernel/CPU, they both swap, so you end up with CPU-endian
> data on the wire, so it should be impossible for a big-endian
> system to talk to a little-endian one. Have you actually tried that
> combination with the current code?

I do not believe anyone has every tried NTB on a big endien system,
let alone tried it with one side LE and the other BE.  To my
knowledge, this has only ever been used on x86 to x86.

> If my interpretation is correct, then the best solution would be to
> completely remove the cpu_to_le32/le32_to_cpu conversions
> from ntb_perf, and just define that it works like any other PCI
> device, exchanging little-endian data.

Yes, this would be the best solution.  Thank you for the insight.

Serge, when you get a chance, please make this change and resumbit.

Thanks,
Jon


> 
> There are two interesting cases to consider though:
> 
> - if someone wants to implement an NTB based protocol
>   using big-endian data on the wire, you probably want to add
>   a ntb_peer_spad_read_be()/ntb_peer_msg_write_be()
>   set of interfaces, to go along with ioread32_be()/iowrite32_be()
>   the same way that ntb_peer_spad_read()/ntb_peer_msg_write()
>   ends up doing ioread32()/iowrite32() with the implied little-endian
>   behavior.
> 
> - memcpy_toio()/memcpy_fromio() and ioread32_rep()/iowrite32_rep
>   importantly do not do any byteswap, they are meant to
>   transfer byte streams.
> 
> Arnd

Re: [PATCH][next] NTB: ntb_tool: fix memory leak on 'buf' on error exit path

[Jon Mason]

On Mon, Jan 22, 2018 at 01:02:39PM +0300, Serge Semin wrote:
> On Mon, Jan 22, 2018 at 09:38:57AM +, Colin King 
>  wrote:
> > From: Colin Ian King 
> > 
> > Currently there is a memory leak on buf when the call to ntb_mw_get_align
> > fails.  Add an exit err label and jump to this so that kfree on buf frees
> > the memory.
> > 
> > Detected by CoverityScan, CID#1464286 ("Resource leak")
> > 
> > Fixes: d637628ce00c ("NTB: ntb_tool: Add full multi-port NTB API support")
> > Signed-off-by: Colin Ian King 
> 
> Good catch, thanks!
> 
> Acked-by: Serge Semin 

Applied to ntb-next

Thanks,
Jon

> 
> > ---
> >  drivers/ntb/test/ntb_tool.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/ntb/test/ntb_tool.c b/drivers/ntb/test/ntb_tool.c
> > index 920fc9b161b0..d592c0ffbd19 100644
> > --- a/drivers/ntb/test/ntb_tool.c
> > +++ b/drivers/ntb/test/ntb_tool.c
> > @@ -659,7 +659,7 @@ static ssize_t tool_mw_trans_read(struct file *filep, 
> > char __user *ubuf,
> > ret = ntb_mw_get_align(inmw->tc->ntb, inmw->pidx, inmw->widx,
> >_align, _align, _max);
> > if (ret)
> > -   return ret;
> > +   goto err;
> >  
> > off += scnprintf(buf + off, buf_size - off,
> >  "Inbound MW \t%d\n",
> > @@ -694,6 +694,8 @@ static ssize_t tool_mw_trans_read(struct file *filep, 
> > char __user *ubuf,
> >  _max);
> >  
> > ret = simple_read_from_buffer(ubuf, size, offp, buf, off);
> > +
> > +err:
> > kfree(buf);
> >  
> > return ret;
> > -- 
> > 2.15.1
> > 

Re: [PATCH][next] NTB: ntb_tool: fix memory leak on 'buf' on error exit path

[Jon Mason]

On Mon, Jan 22, 2018 at 01:02:39PM +0300, Serge Semin wrote:
> On Mon, Jan 22, 2018 at 09:38:57AM +, Colin King 
>  wrote:
> > From: Colin Ian King 
> > 
> > Currently there is a memory leak on buf when the call to ntb_mw_get_align
> > fails.  Add an exit err label and jump to this so that kfree on buf frees
> > the memory.
> > 
> > Detected by CoverityScan, CID#1464286 ("Resource leak")
> > 
> > Fixes: d637628ce00c ("NTB: ntb_tool: Add full multi-port NTB API support")
> > Signed-off-by: Colin Ian King 
> 
> Good catch, thanks!
> 
> Acked-by: Serge Semin 

Applied to ntb-next

Thanks,
Jon

> 
> > ---
> >  drivers/ntb/test/ntb_tool.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/ntb/test/ntb_tool.c b/drivers/ntb/test/ntb_tool.c
> > index 920fc9b161b0..d592c0ffbd19 100644
> > --- a/drivers/ntb/test/ntb_tool.c
> > +++ b/drivers/ntb/test/ntb_tool.c
> > @@ -659,7 +659,7 @@ static ssize_t tool_mw_trans_read(struct file *filep, 
> > char __user *ubuf,
> > ret = ntb_mw_get_align(inmw->tc->ntb, inmw->pidx, inmw->widx,
> >_align, _align, _max);
> > if (ret)
> > -   return ret;
> > +   goto err;
> >  
> > off += scnprintf(buf + off, buf_size - off,
> >  "Inbound MW \t%d\n",
> > @@ -694,6 +694,8 @@ static ssize_t tool_mw_trans_read(struct file *filep, 
> > char __user *ubuf,
> >  _max);
> >  
> > ret = simple_read_from_buffer(ubuf, size, offp, buf, off);
> > +
> > +err:
> > kfree(buf);
> >  
> > return ret;
> > -- 
> > 2.15.1
> > 

Re: [PATCH] NTB: ntb_perf: fix printing of resource_size_t

[Jon Mason]

On Fri, Jan 19, 2018 at 03:55:28PM +0100, Arnd Bergmann wrote:
> On 32-bit architectures, resource_size_t is usually 'unsigned int' or
> 'unsigned long' but not 'unsigned long long', so we get a warning
> about printing the wrong data:
> 
> drivers/ntb/test/ntb_perf.c: In function 'perf_setup_peer_mw':
> drivers/ntb/test/ntb_perf.c:1390:35: error: format '%llx' expects argument of 
> type 'long long unsigned int', but argument 4 has type 'resource_size_t {aka 
> unsigned int}' [-Werror=format=]
> 
> This changes the format string to the special %pa that is already
> used elsewhere in the same file.

Applied to my ntb-next branch.

Thanks,
Jon

> 
> Fixes: b83003b3fdc1 ("NTB: ntb_perf: Add full multi-port NTB API support")
> Signed-off-by: Arnd Bergmann 
> ---
>  drivers/ntb/test/ntb_perf.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/ntb/test/ntb_perf.c b/drivers/ntb/test/ntb_perf.c
> index 8de72f3fba4d..1829a17dd461 100644
> --- a/drivers/ntb/test/ntb_perf.c
> +++ b/drivers/ntb/test/ntb_perf.c
> @@ -1387,8 +1387,8 @@ static int perf_setup_peer_mw(struct perf_peer *peer)
>   if (max_mw_size && peer->outbuf_size > max_mw_size) {
>   peer->outbuf_size = max_mw_size;
>   dev_warn(>perf->ntb->dev,
> - "Peer %d outbuf reduced to %#llx\n", peer->pidx,
> - peer->outbuf_size);
> + "Peer %d outbuf reduced to %pa\n", peer->pidx,
> + >outbuf_size);
>   }
>  
>   return 0;
> -- 
> 2.9.0
> 

Re: [PATCH] NTB: ntb_perf: fix printing of resource_size_t

[Jon Mason]

On Fri, Jan 19, 2018 at 03:55:28PM +0100, Arnd Bergmann wrote:
> On 32-bit architectures, resource_size_t is usually 'unsigned int' or
> 'unsigned long' but not 'unsigned long long', so we get a warning
> about printing the wrong data:
> 
> drivers/ntb/test/ntb_perf.c: In function 'perf_setup_peer_mw':
> drivers/ntb/test/ntb_perf.c:1390:35: error: format '%llx' expects argument of 
> type 'long long unsigned int', but argument 4 has type 'resource_size_t {aka 
> unsigned int}' [-Werror=format=]
> 
> This changes the format string to the special %pa that is already
> used elsewhere in the same file.

Applied to my ntb-next branch.

Thanks,
Jon

> 
> Fixes: b83003b3fdc1 ("NTB: ntb_perf: Add full multi-port NTB API support")
> Signed-off-by: Arnd Bergmann 
> ---
>  drivers/ntb/test/ntb_perf.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/ntb/test/ntb_perf.c b/drivers/ntb/test/ntb_perf.c
> index 8de72f3fba4d..1829a17dd461 100644
> --- a/drivers/ntb/test/ntb_perf.c
> +++ b/drivers/ntb/test/ntb_perf.c
> @@ -1387,8 +1387,8 @@ static int perf_setup_peer_mw(struct perf_peer *peer)
>   if (max_mw_size && peer->outbuf_size > max_mw_size) {
>   peer->outbuf_size = max_mw_size;
>   dev_warn(>perf->ntb->dev,
> - "Peer %d outbuf reduced to %#llx\n", peer->pidx,
> - peer->outbuf_size);
> + "Peer %d outbuf reduced to %pa\n", peer->pidx,
> + >outbuf_size);
>   }
>  
>   return 0;
> -- 
> 2.9.0
> 

Re: [PATCH] [RESEND] ntb_hw_switchtec: fix logic error

[Jon Mason]

On Tue, Jan 16, 2018 at 02:50:51PM +0100, Arnd Bergmann wrote:
> Newer gcc (version 7 and 8 presumably) warn about a statement mixing
> the << operator with logical and:
> 
> drivers/ntb/hw/mscc/ntb_hw_switchtec.c: In function 
> 'switchtec_ntb_init_sndev':
> drivers/ntb/hw/mscc/ntb_hw_switchtec.c:888:24: error: '<<' in boolean 
> context, did you mean '<' ? [-Werror=int-in-bool-context]
> 
> My interpretation here is that the author must have intended a bitmask
> rather than a comparison, so I'm changing the '&&' to '&', which makes
> a lot more sense in the context.
> 
> Fixes: 1b249475275d ("ntb_hw_switchtec: Allow using Switchtec NTB in 
> multi-partition setups")
> Reviewed-by: Logan Gunthorpe 
> Signed-off-by: Arnd Bergmann 
> 
> Originally sent on Dec 9, but this patch never got picked up

Sorry for missing the patch.  Applied to ntb-next.

Thanks,
Jon

> ---
>  drivers/ntb/hw/mscc/ntb_hw_switchtec.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/ntb/hw/mscc/ntb_hw_switchtec.c 
> b/drivers/ntb/hw/mscc/ntb_hw_switchtec.c
> index bcd5b6fb3800..76acb8bba3f2 100644
> --- a/drivers/ntb/hw/mscc/ntb_hw_switchtec.c
> +++ b/drivers/ntb/hw/mscc/ntb_hw_switchtec.c
> @@ -885,7 +885,7 @@ static int switchtec_ntb_init_sndev(struct switchtec_ntb 
> *sndev)
>   }
>  
>   sndev->peer_partition = ffs(tpart_vec) - 1;
> - if (!(part_map && (1 << sndev->peer_partition))) {
> + if (!(part_map & (1 << sndev->peer_partition))) {
>   dev_err(>stdev->dev,
>   "ntb target partition is not NT partition\n");
>   return -ENODEV;
> -- 
> 2.9.0
> 

Re: [PATCH] [RESEND] ntb_hw_switchtec: fix logic error

[Jon Mason]

On Tue, Jan 16, 2018 at 02:50:51PM +0100, Arnd Bergmann wrote:
> Newer gcc (version 7 and 8 presumably) warn about a statement mixing
> the << operator with logical and:
> 
> drivers/ntb/hw/mscc/ntb_hw_switchtec.c: In function 
> 'switchtec_ntb_init_sndev':
> drivers/ntb/hw/mscc/ntb_hw_switchtec.c:888:24: error: '<<' in boolean 
> context, did you mean '<' ? [-Werror=int-in-bool-context]
> 
> My interpretation here is that the author must have intended a bitmask
> rather than a comparison, so I'm changing the '&&' to '&', which makes
> a lot more sense in the context.
> 
> Fixes: 1b249475275d ("ntb_hw_switchtec: Allow using Switchtec NTB in 
> multi-partition setups")
> Reviewed-by: Logan Gunthorpe 
> Signed-off-by: Arnd Bergmann 
> 
> Originally sent on Dec 9, but this patch never got picked up

Sorry for missing the patch.  Applied to ntb-next.

Thanks,
Jon

> ---
>  drivers/ntb/hw/mscc/ntb_hw_switchtec.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/ntb/hw/mscc/ntb_hw_switchtec.c 
> b/drivers/ntb/hw/mscc/ntb_hw_switchtec.c
> index bcd5b6fb3800..76acb8bba3f2 100644
> --- a/drivers/ntb/hw/mscc/ntb_hw_switchtec.c
> +++ b/drivers/ntb/hw/mscc/ntb_hw_switchtec.c
> @@ -885,7 +885,7 @@ static int switchtec_ntb_init_sndev(struct switchtec_ntb 
> *sndev)
>   }
>  
>   sndev->peer_partition = ffs(tpart_vec) - 1;
> - if (!(part_map && (1 << sndev->peer_partition))) {
> + if (!(part_map & (1 << sndev->peer_partition))) {
>   dev_err(>stdev->dev,
>   "ntb target partition is not NT partition\n");
>   return -ENODEV;
> -- 
> 2.9.0
> 

Re: [PATCH v2 2/2] ntb_hw_switchtec: Check for alignment of the buffer in mw_set_trans()

[Jon Mason]

On Mon, Dec 18, 2017 at 11:25:06AM -0700, Logan Gunthorpe wrote:
> With Switchtec hardware, the buffer used for a memory window must be
> aligned to its size (the hardware only replaces the lower bits). In
> certain circumstances dma_alloc_coherent() will not provide a buffer
> that adheres to this requirement like when using the CMA and
> CONFIG_CMA_ALIGNMENT is set lower than the buffer size.
> 
> When we get an unaligned buffer mw_set_trans() should return an error.
> We also log an error so we know the cause of the problem.
> 

Applied to ntb-next.

Thanks,
Jon

> Signed-off-by: Logan Gunthorpe <log...@deltatee.com>
> Cc: Jon Mason <jdma...@kudzu.us>
> ---
> 
> v2: Use IS_ALIGNED macro as suggested by Allen
> 
>  drivers/ntb/hw/mscc/ntb_hw_switchtec.c | 13 +
>  1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/ntb/hw/mscc/ntb_hw_switchtec.c 
> b/drivers/ntb/hw/mscc/ntb_hw_switchtec.c
> index bcd5b6fb3800..6c6f991999b5 100644
> --- a/drivers/ntb/hw/mscc/ntb_hw_switchtec.c
> +++ b/drivers/ntb/hw/mscc/ntb_hw_switchtec.c
> @@ -320,6 +320,19 @@ static int switchtec_ntb_mw_set_trans(struct ntb_dev 
> *ntb, int pidx, int widx,
>   if (xlate_pos < 12)
>   return -EINVAL;
> 
> + if (!IS_ALIGNED(addr, BIT_ULL(xlate_pos))) {
> + /*
> +  * In certain circumstances we can get a buffer that is
> +  * not aligned to its size. (Most of the time
> +  * dma_alloc_coherent ensures this). This can happen when
> +  * using large buffers allocated by the CMA
> +  * (see CMA_CONFIG_ALIGNMENT)
> +  */
> + dev_err(>stdev->dev,
> + "ERROR: Memory window address is not aligned to it's 
> size!\n");
> + return -EINVAL;
> + }
> +
>   rc = switchtec_ntb_part_op(sndev, ctl, NTB_CTRL_PART_OP_LOCK,
>  NTB_CTRL_PART_STATUS_LOCKED);
>   if (rc)
> --
> 2.11.0
> 
> -- 
> You received this message because you are subscribed to the Google Groups 
> "linux-ntb" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to linux-ntb+unsubscr...@googlegroups.com.
> To post to this group, send email to linux-...@googlegroups.com.
> To view this discussion on the web visit 
> https://groups.google.com/d/msgid/linux-ntb/20171218182506.5219-2-logang%40deltatee.com.
> For more options, visit https://groups.google.com/d/optout.

Re: [PATCH v2 2/2] ntb_hw_switchtec: Check for alignment of the buffer in mw_set_trans()

[Jon Mason]

On Mon, Dec 18, 2017 at 11:25:06AM -0700, Logan Gunthorpe wrote:
> With Switchtec hardware, the buffer used for a memory window must be
> aligned to its size (the hardware only replaces the lower bits). In
> certain circumstances dma_alloc_coherent() will not provide a buffer
> that adheres to this requirement like when using the CMA and
> CONFIG_CMA_ALIGNMENT is set lower than the buffer size.
> 
> When we get an unaligned buffer mw_set_trans() should return an error.
> We also log an error so we know the cause of the problem.
> 

Applied to ntb-next.

Thanks,
Jon

> Signed-off-by: Logan Gunthorpe 
> Cc: Jon Mason 
> ---
> 
> v2: Use IS_ALIGNED macro as suggested by Allen
> 
>  drivers/ntb/hw/mscc/ntb_hw_switchtec.c | 13 +
>  1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/ntb/hw/mscc/ntb_hw_switchtec.c 
> b/drivers/ntb/hw/mscc/ntb_hw_switchtec.c
> index bcd5b6fb3800..6c6f991999b5 100644
> --- a/drivers/ntb/hw/mscc/ntb_hw_switchtec.c
> +++ b/drivers/ntb/hw/mscc/ntb_hw_switchtec.c
> @@ -320,6 +320,19 @@ static int switchtec_ntb_mw_set_trans(struct ntb_dev 
> *ntb, int pidx, int widx,
>   if (xlate_pos < 12)
>   return -EINVAL;
> 
> + if (!IS_ALIGNED(addr, BIT_ULL(xlate_pos))) {
> + /*
> +  * In certain circumstances we can get a buffer that is
> +  * not aligned to its size. (Most of the time
> +  * dma_alloc_coherent ensures this). This can happen when
> +  * using large buffers allocated by the CMA
> +  * (see CMA_CONFIG_ALIGNMENT)
> +  */
> + dev_err(>stdev->dev,
> + "ERROR: Memory window address is not aligned to it's 
> size!\n");
> + return -EINVAL;
> + }
> +
>   rc = switchtec_ntb_part_op(sndev, ctl, NTB_CTRL_PART_OP_LOCK,
>  NTB_CTRL_PART_STATUS_LOCKED);
>   if (rc)
> --
> 2.11.0
> 
> -- 
> You received this message because you are subscribed to the Google Groups 
> "linux-ntb" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to linux-ntb+unsubscr...@googlegroups.com.
> To post to this group, send email to linux-...@googlegroups.com.
> To view this discussion on the web visit 
> https://groups.google.com/d/msgid/linux-ntb/20171218182506.5219-2-logang%40deltatee.com.
> For more options, visit https://groups.google.com/d/optout.

Re: [PATCH v2 1/2] ntb_transport: Fix bug with max_mw_size parameter

[Jon Mason]

On Mon, Dec 18, 2017 at 11:25:05AM -0700, Logan Gunthorpe wrote:
> When using the max_mw_size parameter of ntb_transport to limit the size of
> the Memory windows, communication cannot be established and the queues
> freeze.
> 
> This is because the mw_size that's reported to the peer is correctly
> limited but the size used locally is not. So the MW is initialized
> with a buffer smaller than the window but the TX side is using the
> full window. This means the TX side will be writing to a region of the
> window that points nowhere.
> 
> This is easily fixed by applying the same limit to tx_size in
> ntb_transport_init_queue().

Applied to ntb-next.

Thanks,
Jon

> 
> Fixes: e26a5843f7f5 ("NTB: Split ntb_hw_intel and ntb_transport drivers")
> Signed-off-by: Logan Gunthorpe <log...@deltatee.com>
> Acked-by: Allen Hubbe <allen.hu...@dell.com>
> Cc: Jon Mason <jdma...@kudzu.us>
> Cc: Dave Jiang <dave.ji...@intel.com>
> ---
>  drivers/ntb/ntb_transport.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
> index 045e3dd4750e..9878c48826e3 100644
> --- a/drivers/ntb/ntb_transport.c
> +++ b/drivers/ntb/ntb_transport.c
> @@ -1003,6 +1003,9 @@ static int ntb_transport_init_queue(struct 
> ntb_transport_ctx *nt,
>   mw_base = nt->mw_vec[mw_num].phys_addr;
>   mw_size = nt->mw_vec[mw_num].phys_size;
>  
> + if (max_mw_size && mw_size > max_mw_size)
> + mw_size = max_mw_size;
> +
>   tx_size = (unsigned int)mw_size / num_qps_mw;
>   qp_offset = tx_size * (qp_num / mw_count);
>  
> -- 
> 2.11.0
> 
> -- 
> You received this message because you are subscribed to the Google Groups 
> "linux-ntb" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to linux-ntb+unsubscr...@googlegroups.com.
> To post to this group, send email to linux-...@googlegroups.com.
> To view this discussion on the web visit 
> https://groups.google.com/d/msgid/linux-ntb/20171218182506.5219-1-logang%40deltatee.com.
> For more options, visit https://groups.google.com/d/optout.

Re: [PATCH v2 1/2] ntb_transport: Fix bug with max_mw_size parameter

[Jon Mason]

On Mon, Dec 18, 2017 at 11:25:05AM -0700, Logan Gunthorpe wrote:
> When using the max_mw_size parameter of ntb_transport to limit the size of
> the Memory windows, communication cannot be established and the queues
> freeze.
> 
> This is because the mw_size that's reported to the peer is correctly
> limited but the size used locally is not. So the MW is initialized
> with a buffer smaller than the window but the TX side is using the
> full window. This means the TX side will be writing to a region of the
> window that points nowhere.
> 
> This is easily fixed by applying the same limit to tx_size in
> ntb_transport_init_queue().

Applied to ntb-next.

Thanks,
Jon

> 
> Fixes: e26a5843f7f5 ("NTB: Split ntb_hw_intel and ntb_transport drivers")
> Signed-off-by: Logan Gunthorpe 
> Acked-by: Allen Hubbe 
> Cc: Jon Mason 
> Cc: Dave Jiang 
> ---
>  drivers/ntb/ntb_transport.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
> index 045e3dd4750e..9878c48826e3 100644
> --- a/drivers/ntb/ntb_transport.c
> +++ b/drivers/ntb/ntb_transport.c
> @@ -1003,6 +1003,9 @@ static int ntb_transport_init_queue(struct 
> ntb_transport_ctx *nt,
>   mw_base = nt->mw_vec[mw_num].phys_addr;
>   mw_size = nt->mw_vec[mw_num].phys_size;
>  
> + if (max_mw_size && mw_size > max_mw_size)
> + mw_size = max_mw_size;
> +
>   tx_size = (unsigned int)mw_size / num_qps_mw;
>   qp_offset = tx_size * (qp_num / mw_count);
>  
> -- 
> 2.11.0
> 
> -- 
> You received this message because you are subscribed to the Google Groups 
> "linux-ntb" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to linux-ntb+unsubscr...@googlegroups.com.
> To post to this group, send email to linux-...@googlegroups.com.
> To view this discussion on the web visit 
> https://groups.google.com/d/msgid/linux-ntb/20171218182506.5219-1-logang%40deltatee.com.
> For more options, visit https://groups.google.com/d/optout.

Re: [PATCH] MAINTAINERS: NTB: Update contact info

[Jon Mason]

On Tue, Dec 12, 2017 at 09:27:52PM -0500, Allen Hubbe wrote:
> I am no longer employed by Dell EMC.  For the purposes of NTB driver
> development and maintenance, please contact me via my personal email.

Applied to ntb-next.

Thanks,
Jon

> 
> Signed-off-by: Allen Hubbe <alle...@gmail.com>
> Signed-off-by: Allen Hubbe <allen.hu...@emc.com>
> ---
>  MAINTAINERS | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 82ad0eabce4f..cb7344e37fc5 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -9712,7 +9712,7 @@ F:      drivers/ntb/hw/amd/
>  NTB DRIVER CORE
>  M:   Jon Mason <jdma...@kudzu.us>
>  M:   Dave Jiang <dave.ji...@intel.com>
> -M:   Allen Hubbe <allen.hu...@emc.com>
> +M:   Allen Hubbe <alle...@gmail.com>
>  L:   linux-...@googlegroups.com
>  S:   Supported
>  W:   https://github.com/jonmason/ntb/wiki
> -- 
> 2.14.1
> 

Re: [PATCH] MAINTAINERS: NTB: Update contact info

[Jon Mason]

On Tue, Dec 12, 2017 at 09:27:52PM -0500, Allen Hubbe wrote:
> I am no longer employed by Dell EMC.  For the purposes of NTB driver
> development and maintenance, please contact me via my personal email.

Applied to ntb-next.

Thanks,
Jon

> 
> Signed-off-by: Allen Hubbe 
> Signed-off-by: Allen Hubbe 
> ---
>  MAINTAINERS | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 82ad0eabce4f..cb7344e37fc5 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -9712,7 +9712,7 @@ F:  drivers/ntb/hw/amd/
>  NTB DRIVER CORE
>  M:   Jon Mason 
>  M:   Dave Jiang 
> -M:   Allen Hubbe 
> +M:   Allen Hubbe 
>  L:   linux-...@googlegroups.com
>  S:   Supported
>  W:   https://github.com/jonmason/ntb/wiki
> -- 
> 2.14.1
> 

Re: [PATCH v4 00/15] NTB: Add full multi-port API support to the test drivers

[Jon Mason]

On Wed, Dec 06, 2017 at 05:31:51PM +0300, Serge Semin wrote:
> The multi-port NTB API was introduced in kernel 4.13 as well as the
> first driver for the true multi-port devices of IDT PCIe-switches
> series. But the test drivers still were left almost unchanged. Yes,
> they didn't fail being used with new NTB API, but they only worked
> with two-ports NTB devices. This patchset is intended to fix the
> issue by amending the NTB test drivers and script so they would be
> fully compatible with multi-port NTB API.
> 
> Additionally I found a few NTB subsystem issues while developing the
> submitted patches. So they are also fixed in this patchset.
> 
> The patchset is applied on top of the recent Jon Mason's repo state:
> https://github.com/jonmason/ntb
> Particularly the "ntb/ntb-next" branch is used. The last commit hash
> had been:
> commit  ("ntb_hw_switchtec: Force down the link before")

Series applied to ntb-next.

Thanks,
Jon

> 
> Serge Semin (15):
>   NTB: Rename NTB messaging API methods
>   NTB: Set dma mask and dma coherent mask to NTB devices
>   NTB: Fix UB/bug in ntb_mw_get_align()
>   NTB: ntb_pp: Add full multi-port NTB API support
>   NTB: ntb_tool: Add full multi-port NTB API support
>   NTB: ntb_perf: Add full multi-port NTB API support
>   NTB: ntb_test: Safely use paths with whitespace
>   NTB: ntb_test: Add ntb_tool port tests
>   NTB: ntb_test: Update ntb_tool link tests
>   NTB: ntb_test: Update ntb_tool DB tests
>   NTB: ntb_test: Update ntb_tool Scratchpad tests
>   NTB: ntb_test: Add ntb_tool Message tests
>   NTB: ntb_test: Update ntb_tool MW tests
>   NTB: ntb_test: Update ntb_perf tests
>   NTB: ntb_hw_idt: Set NTB_TOPO_SWITCH topology
> 
>  drivers/ntb/hw/amd/ntb_hw_amd.c |4 +
>  drivers/ntb/hw/idt/ntb_hw_idt.c |   37 +-
>  drivers/ntb/hw/intel/ntb_hw_intel.c |4 +
>  drivers/ntb/ntb.c   |1 -
>  drivers/ntb/test/ntb_perf.c | 1820 
> +--
>  drivers/ntb/test/ntb_pingpong.c |  447 +---
>  drivers/ntb/test/ntb_tool.c | 1820 
> +--
>  include/linux/ntb.h |   36 +-
>  tools/testing/selftests/ntb/ntb_test.sh |  307 --
>  9 files changed, 3018 insertions(+), 1458 deletions(-)
> 
> -- 
> 2.12.0
> 

Re: [PATCH v4 00/15] NTB: Add full multi-port API support to the test drivers

[Jon Mason]

On Wed, Dec 06, 2017 at 05:31:51PM +0300, Serge Semin wrote:
> The multi-port NTB API was introduced in kernel 4.13 as well as the
> first driver for the true multi-port devices of IDT PCIe-switches
> series. But the test drivers still were left almost unchanged. Yes,
> they didn't fail being used with new NTB API, but they only worked
> with two-ports NTB devices. This patchset is intended to fix the
> issue by amending the NTB test drivers and script so they would be
> fully compatible with multi-port NTB API.
> 
> Additionally I found a few NTB subsystem issues while developing the
> submitted patches. So they are also fixed in this patchset.
> 
> The patchset is applied on top of the recent Jon Mason's repo state:
> https://github.com/jonmason/ntb
> Particularly the "ntb/ntb-next" branch is used. The last commit hash
> had been:
> commit  ("ntb_hw_switchtec: Force down the link before")

Series applied to ntb-next.

Thanks,
Jon

> 
> Serge Semin (15):
>   NTB: Rename NTB messaging API methods
>   NTB: Set dma mask and dma coherent mask to NTB devices
>   NTB: Fix UB/bug in ntb_mw_get_align()
>   NTB: ntb_pp: Add full multi-port NTB API support
>   NTB: ntb_tool: Add full multi-port NTB API support
>   NTB: ntb_perf: Add full multi-port NTB API support
>   NTB: ntb_test: Safely use paths with whitespace
>   NTB: ntb_test: Add ntb_tool port tests
>   NTB: ntb_test: Update ntb_tool link tests
>   NTB: ntb_test: Update ntb_tool DB tests
>   NTB: ntb_test: Update ntb_tool Scratchpad tests
>   NTB: ntb_test: Add ntb_tool Message tests
>   NTB: ntb_test: Update ntb_tool MW tests
>   NTB: ntb_test: Update ntb_perf tests
>   NTB: ntb_hw_idt: Set NTB_TOPO_SWITCH topology
> 
>  drivers/ntb/hw/amd/ntb_hw_amd.c |4 +
>  drivers/ntb/hw/idt/ntb_hw_idt.c |   37 +-
>  drivers/ntb/hw/intel/ntb_hw_intel.c |4 +
>  drivers/ntb/ntb.c   |1 -
>  drivers/ntb/test/ntb_perf.c | 1820 
> +--
>  drivers/ntb/test/ntb_pingpong.c |  447 +---
>  drivers/ntb/test/ntb_tool.c | 1820 
> +--
>  include/linux/ntb.h |   36 +-
>  tools/testing/selftests/ntb/ntb_test.sh |  307 --
>  9 files changed, 3018 insertions(+), 1458 deletions(-)
> 
> -- 
> 2.12.0
> 

Re: [PATCH 5/7] ntb_hw_switchtec: Expand PFF CSR registers

[Jon Mason]

On Tue, Dec 5, 2017 at 2:40 PM, Logan Gunthorpe <log...@deltatee.com> wrote:
>
>
> On 05/12/17 12:12 PM, Jon Mason wrote:
>>
>> It sucks that we don't already have a struct for PCI config space we
>> can reuse here.  If you find the time, it would be good to add in the
>> future to reduce duplicate code here and in the PCI core.  However,
>> this patch is fine without it.
>
>
> I agree. And believe me I looked for one. I'm not sure if it makes sense to
> add it to general code at this time when I'm the only user.

No problem.

>
> Logan

Re: [PATCH 5/7] ntb_hw_switchtec: Expand PFF CSR registers

[Jon Mason]

On Tue, Dec 5, 2017 at 2:40 PM, Logan Gunthorpe  wrote:
>
>
> On 05/12/17 12:12 PM, Jon Mason wrote:
>>
>> It sucks that we don't already have a struct for PCI config space we
>> can reuse here.  If you find the time, it would be good to add in the
>> future to reduce duplicate code here and in the PCI core.  However,
>> this patch is fine without it.
>
>
> I agree. And believe me I looked for one. I'm not sure if it makes sense to
> add it to general code at this time when I'm the only user.

No problem.

>
> Logan

Re: [PATCH 0/7] Switchtec NTB Crosslink Support

[Jon Mason]

On Wed, Nov 29, 2017 at 12:58 PM, Logan Gunthorpe  wrote:
> Also, I forgot to mention, this patch set is based on today's ntb-next.

All of these look sane to me.  Assuming they apply cleanly, adding to ntb-next.

Thanks,
Jon

>
> Logan
>
>
> On 29/11/17 10:55 AM, Logan Gunthorpe wrote:
>>
>> Hi,
>>
>> This patch series adds support for the Switchtec Crosslink feature.
>> Crosslink is similar to B2B in that it allows two switches to be
>> connected back to back.
>>
>> Two switches can already be connected between two hosts, however with this
>> setup, there would be no symmetry as one switch would handle NTB and the
>> other would just need to be a single partition switch. Our customers are
>> looking for a completely symmetric solution such that a host or switch
>> can be easily replaced without needing to worry about which half was
>> replaced.
>>
>> Crosslink solves this problem by having each of the switches configured
>> with two partitions. This results in a special host-less partition in
>> the middle of the connection which can be managed in such a way as
>> to forward all the traffic and still provide all the same NTB
>> functionality. In this way, both hosts and switches can be identical.
>>
>> Please see Patch 6 for a bit more detailed description of how Crosslink
>> works.
>>
>> Thanks,
>>
>> Logan
>>
>> Kelvin Cao (1):
>>ntb_hw_switchtec: Allow using Switchtec NTB in multi-partition setups
>>
>> Logan Gunthorpe (6):
>>ntb_hw_switchtec: Keep track of the number of LUT windows used by the
>>  driver
>>ntb_hw_switchtec: Create helper function to setup reserved LUT MWs
>>ntb_hw_switchtec: Make switchtec_ntb_init_req_id_table() more general
>>ntb_hw_switchtec: Expand PFF CSR registers
>>ntb_hw_switchtec: Add initialization code for crosslink
>>ntb_hw_switchtec: Crosslink doorbells and messages
>>
>>   drivers/ntb/hw/mscc/ntb_hw_switchtec.c | 494
>> +++--
>>   include/linux/ntb.h|  15 +-
>>   include/linux/switchtec.h  |  23 +-
>>   3 files changed, 435 insertions(+), 97 deletions(-)
>>
>> --
>> 2.11.0
>>
>

Re: [PATCH 0/7] Switchtec NTB Crosslink Support

[Jon Mason]

On Wed, Nov 29, 2017 at 12:58 PM, Logan Gunthorpe  wrote:
> Also, I forgot to mention, this patch set is based on today's ntb-next.

All of these look sane to me.  Assuming they apply cleanly, adding to ntb-next.

Thanks,
Jon

>
> Logan
>
>
> On 29/11/17 10:55 AM, Logan Gunthorpe wrote:
>>
>> Hi,
>>
>> This patch series adds support for the Switchtec Crosslink feature.
>> Crosslink is similar to B2B in that it allows two switches to be
>> connected back to back.
>>
>> Two switches can already be connected between two hosts, however with this
>> setup, there would be no symmetry as one switch would handle NTB and the
>> other would just need to be a single partition switch. Our customers are
>> looking for a completely symmetric solution such that a host or switch
>> can be easily replaced without needing to worry about which half was
>> replaced.
>>
>> Crosslink solves this problem by having each of the switches configured
>> with two partitions. This results in a special host-less partition in
>> the middle of the connection which can be managed in such a way as
>> to forward all the traffic and still provide all the same NTB
>> functionality. In this way, both hosts and switches can be identical.
>>
>> Please see Patch 6 for a bit more detailed description of how Crosslink
>> works.
>>
>> Thanks,
>>
>> Logan
>>
>> Kelvin Cao (1):
>>ntb_hw_switchtec: Allow using Switchtec NTB in multi-partition setups
>>
>> Logan Gunthorpe (6):
>>ntb_hw_switchtec: Keep track of the number of LUT windows used by the
>>  driver
>>ntb_hw_switchtec: Create helper function to setup reserved LUT MWs
>>ntb_hw_switchtec: Make switchtec_ntb_init_req_id_table() more general
>>ntb_hw_switchtec: Expand PFF CSR registers
>>ntb_hw_switchtec: Add initialization code for crosslink
>>ntb_hw_switchtec: Crosslink doorbells and messages
>>
>>   drivers/ntb/hw/mscc/ntb_hw_switchtec.c | 494
>> +++--
>>   include/linux/ntb.h|  15 +-
>>   include/linux/switchtec.h  |  23 +-
>>   3 files changed, 435 insertions(+), 97 deletions(-)
>>
>> --
>> 2.11.0
>>
>

Re: [PATCH 5/7] ntb_hw_switchtec: Expand PFF CSR registers

[Jon Mason]

On Wed, Nov 29, 2017 at 12:55 PM, Logan Gunthorpe  wrote:
> The PFF CSR registers actual mirrors the PCI configuration space
> for all the ports in the switch. Previously, this was not needed by
> the driver but will be used by the crosslink code to enumerate the
> bus in an host-less centre partition.
>
> Signed-off-by: Logan Gunthorpe 
> ---
>  include/linux/switchtec.h | 15 ++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/switchtec.h b/include/linux/switchtec.h
> index d4a7c18b42cf..6d325a7a0c19 100644
> --- a/include/linux/switchtec.h
> +++ b/include/linux/switchtec.h
> @@ -292,7 +292,20 @@ enum {
>  struct pff_csr_regs {
> u16 vendor_id;
> u16 device_id;
> -   u32 pci_cfg_header[15];
> +   u16 pcicmd;
> +   u16 pcists;
> +   u32 pci_class;
> +   u32 pci_opts;
> +   union {
> +   u32 pci_bar[6];
> +   u64 pci_bar64[3];
> +   };
> +   u32 pci_cardbus;
> +   u32 pci_subsystem_id;
> +   u32 pci_expansion_rom;
> +   u32 pci_cap_ptr;
> +   u32 reserved1;
> +   u32 pci_irq;

It sucks that we don't already have a struct for PCI config space we
can reuse here.  If you find the time, it would be good to add in the
future to reduce duplicate code here and in the PCI core.  However,
this patch is fine without it.

Thanks,
Jon


> u32 pci_cap_region[48];
> u32 pcie_cap_region[448];
> u32 indirect_gas_window[128];
> --
> 2.11.0
>

Re: [PATCH 5/7] ntb_hw_switchtec: Expand PFF CSR registers

[Jon Mason]

On Wed, Nov 29, 2017 at 12:55 PM, Logan Gunthorpe  wrote:
> The PFF CSR registers actual mirrors the PCI configuration space
> for all the ports in the switch. Previously, this was not needed by
> the driver but will be used by the crosslink code to enumerate the
> bus in an host-less centre partition.
>
> Signed-off-by: Logan Gunthorpe 
> ---
>  include/linux/switchtec.h | 15 ++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/switchtec.h b/include/linux/switchtec.h
> index d4a7c18b42cf..6d325a7a0c19 100644
> --- a/include/linux/switchtec.h
> +++ b/include/linux/switchtec.h
> @@ -292,7 +292,20 @@ enum {
>  struct pff_csr_regs {
> u16 vendor_id;
> u16 device_id;
> -   u32 pci_cfg_header[15];
> +   u16 pcicmd;
> +   u16 pcists;
> +   u32 pci_class;
> +   u32 pci_opts;
> +   union {
> +   u32 pci_bar[6];
> +   u64 pci_bar64[3];
> +   };
> +   u32 pci_cardbus;
> +   u32 pci_subsystem_id;
> +   u32 pci_expansion_rom;
> +   u32 pci_cap_ptr;
> +   u32 reserved1;
> +   u32 pci_irq;

It sucks that we don't already have a struct for PCI config space we
can reuse here.  If you find the time, it would be good to add in the
future to reduce duplicate code here and in the PCI core.  However,
this patch is fine without it.

Thanks,
Jon


> u32 pci_cap_region[48];
> u32 pcie_cap_region[448];
> u32 indirect_gas_window[128];
> --
> 2.11.0
>

Re: [PATCH v2 10/15] NTB: ntb_test: Update ntb_tool DB tests

[Jon Mason]

On Sun, Dec 3, 2017 at 2:17 PM, Serge Semin  wrote:
> DB interface of ntb_tool driver hasn't been changed much, but
> db_valid_mask DebugFS file has still been added to new ntb_tool
> driver. In this case it's much better to test all valid DB bits
> instead of using the predefined mask, which may be incorrect in
> general.
>
> Signed-off-by: Serge Semin 
> ---
>  tools/testing/selftests/ntb/ntb_test.sh | 29 ++---
>  1 file changed, 18 insertions(+), 11 deletions(-)
>
> diff --git a/tools/testing/selftests/ntb/ntb_test.sh 
> b/tools/testing/selftests/ntb/ntb_test.sh
> index 247458c6d8dc..3d43885ea4b5 100755
> --- a/tools/testing/selftests/ntb/ntb_test.sh
> +++ b/tools/testing/selftests/ntb/ntb_test.sh
> @@ -18,7 +18,6 @@ LIST_DEVS=FALSE
>
>  DEBUGFS=${DEBUGFS-/sys/kernel/debug}
>
> -DB_BITMASK=0x7FFF
>  PERF_RUN_ORDER=32
>  MAX_MW_SIZE=0
>  RUN_DMA_TESTS=
> @@ -39,7 +38,6 @@ function show_help()
> echo "be highly recommended."
> echo
> echo "Options:"
> -   echo "  -b BITMASK  doorbell clear bitmask for ntb_tool"
> echo "  -C  don't cleanup ntb modules on exit"
> echo "  -d  run dma tests"
> echo "  -h  show this help message"
> @@ -56,7 +54,6 @@ function parse_args()
> OPTIND=0
> while getopts "b:Cdhlm:r:p:w:" opt; do
> case "$opt" in
> -   b)  DB_BITMASK=${OPTARG} ;;
> C)  DONT_CLEANUP=1 ;;
> d)  RUN_DMA_TESTS=1 ;;
> h)  show_help; exit 0 ;;
> @@ -215,21 +212,30 @@ function doorbell_test()
>
> echo "Running db tests on: $(basename $LOC) / $(basename $REM)"
>
> -   write_file "c $DB_BITMASK" "$REM/db"
> +   DB_VALID_MASK=$(read_file "$LOC/db_valid_mask")
>
> -   for ((i=1; i <= 8; i++)); do
> -   let DB=$(read_file "$REM/db") || true
> -   if [[ "$DB" != "$EXP" ]]; then
> +   write_file "c $DB_VALID_MASK" "$REM/db"
> +
> +   for ((i = 0; i < 64; i++)); do

I'm guessing this should be a tunable variable, for those systems with
a different number of doorbells.


> +   DB=$(read_file "$REM/db")
> +   if [[ "$DB" -ne "$EXP" ]]; then
> echo "Doorbell doesn't match expected value $EXP " \
>  "in $REM/db" >&2
> exit -1
> fi
>
> -   let "MASK=1 << ($i-1)" || true
> -   let "EXP=$EXP | $MASK" || true
> +   let "MASK = (1 << $i) & $DB_VALID_MASK" || true
> +   let "EXP = $EXP | $MASK" || true
> +
> write_file "s $MASK" "$LOC/peer_db"
> done
>
> +   write_file "c $DB_VALID_MASK" "$REM/db_mask"
> +   write_file $DB_VALID_MASK "$REM/db_event"
> +   write_file "s $DB_VALID_MASK" "$REM/db_mask"
> +
> +   write_file "c $DB_VALID_MASK" "$REM/db"
> +
> echo "  Passed"
>  }
>
> @@ -393,14 +399,15 @@ function ntb_tool_tests()
> write_file "Y" "$LOCAL_PEER_TOOL/link_event"
> write_file "Y" "$REMOTE_PEER_TOOL/link_event"
>
> +   doorbell_test "$LOCAL_TOOL" "$REMOTE_TOOL"
> +   doorbell_test "$REMOTE_TOOL" "$LOCAL_TOOL"
> +
> for PEER_TRANS in $(ls "$LOCAL_TOOL"/peer_trans*); do
> PT=$(basename $PEER_TRANS)
> write_file $MW_SIZE "$LOCAL_TOOL/$PT"
> write_file $MW_SIZE "$REMOTE_TOOL/$PT"
> done
>
> -   doorbell_test "$LOCAL_TOOL" "$REMOTE_TOOL"
> -   doorbell_test "$REMOTE_TOOL" "$LOCAL_TOOL"
> scratchpad_test "$LOCAL_TOOL" "$REMOTE_TOOL"
> scratchpad_test "$REMOTE_TOOL" "$LOCAL_TOOL"
>
> --
> 2.12.0
>

Re: [PATCH v2 10/15] NTB: ntb_test: Update ntb_tool DB tests

[Jon Mason]

On Sun, Dec 3, 2017 at 2:17 PM, Serge Semin  wrote:
> DB interface of ntb_tool driver hasn't been changed much, but
> db_valid_mask DebugFS file has still been added to new ntb_tool
> driver. In this case it's much better to test all valid DB bits
> instead of using the predefined mask, which may be incorrect in
> general.
>
> Signed-off-by: Serge Semin 
> ---
>  tools/testing/selftests/ntb/ntb_test.sh | 29 ++---
>  1 file changed, 18 insertions(+), 11 deletions(-)
>
> diff --git a/tools/testing/selftests/ntb/ntb_test.sh 
> b/tools/testing/selftests/ntb/ntb_test.sh
> index 247458c6d8dc..3d43885ea4b5 100755
> --- a/tools/testing/selftests/ntb/ntb_test.sh
> +++ b/tools/testing/selftests/ntb/ntb_test.sh
> @@ -18,7 +18,6 @@ LIST_DEVS=FALSE
>
>  DEBUGFS=${DEBUGFS-/sys/kernel/debug}
>
> -DB_BITMASK=0x7FFF
>  PERF_RUN_ORDER=32
>  MAX_MW_SIZE=0
>  RUN_DMA_TESTS=
> @@ -39,7 +38,6 @@ function show_help()
> echo "be highly recommended."
> echo
> echo "Options:"
> -   echo "  -b BITMASK  doorbell clear bitmask for ntb_tool"
> echo "  -C  don't cleanup ntb modules on exit"
> echo "  -d  run dma tests"
> echo "  -h  show this help message"
> @@ -56,7 +54,6 @@ function parse_args()
> OPTIND=0
> while getopts "b:Cdhlm:r:p:w:" opt; do
> case "$opt" in
> -   b)  DB_BITMASK=${OPTARG} ;;
> C)  DONT_CLEANUP=1 ;;
> d)  RUN_DMA_TESTS=1 ;;
> h)  show_help; exit 0 ;;
> @@ -215,21 +212,30 @@ function doorbell_test()
>
> echo "Running db tests on: $(basename $LOC) / $(basename $REM)"
>
> -   write_file "c $DB_BITMASK" "$REM/db"
> +   DB_VALID_MASK=$(read_file "$LOC/db_valid_mask")
>
> -   for ((i=1; i <= 8; i++)); do
> -   let DB=$(read_file "$REM/db") || true
> -   if [[ "$DB" != "$EXP" ]]; then
> +   write_file "c $DB_VALID_MASK" "$REM/db"
> +
> +   for ((i = 0; i < 64; i++)); do

I'm guessing this should be a tunable variable, for those systems with
a different number of doorbells.


> +   DB=$(read_file "$REM/db")
> +   if [[ "$DB" -ne "$EXP" ]]; then
> echo "Doorbell doesn't match expected value $EXP " \
>  "in $REM/db" >&2
> exit -1
> fi
>
> -   let "MASK=1 << ($i-1)" || true
> -   let "EXP=$EXP | $MASK" || true
> +   let "MASK = (1 << $i) & $DB_VALID_MASK" || true
> +   let "EXP = $EXP | $MASK" || true
> +
> write_file "s $MASK" "$LOC/peer_db"
> done
>
> +   write_file "c $DB_VALID_MASK" "$REM/db_mask"
> +   write_file $DB_VALID_MASK "$REM/db_event"
> +   write_file "s $DB_VALID_MASK" "$REM/db_mask"
> +
> +   write_file "c $DB_VALID_MASK" "$REM/db"
> +
> echo "  Passed"
>  }
>
> @@ -393,14 +399,15 @@ function ntb_tool_tests()
> write_file "Y" "$LOCAL_PEER_TOOL/link_event"
> write_file "Y" "$REMOTE_PEER_TOOL/link_event"
>
> +   doorbell_test "$LOCAL_TOOL" "$REMOTE_TOOL"
> +   doorbell_test "$REMOTE_TOOL" "$LOCAL_TOOL"
> +
> for PEER_TRANS in $(ls "$LOCAL_TOOL"/peer_trans*); do
> PT=$(basename $PEER_TRANS)
> write_file $MW_SIZE "$LOCAL_TOOL/$PT"
> write_file $MW_SIZE "$REMOTE_TOOL/$PT"
> done
>
> -   doorbell_test "$LOCAL_TOOL" "$REMOTE_TOOL"
> -   doorbell_test "$REMOTE_TOOL" "$LOCAL_TOOL"
> scratchpad_test "$LOCAL_TOOL" "$REMOTE_TOOL"
> scratchpad_test "$REMOTE_TOOL" "$LOCAL_TOOL"
>
> --
> 2.12.0
>

Re: [PATCH v2 09/15] NTB: ntb_test: Update ntb_tool link tests

[Jon Mason]

On Sun, Dec 3, 2017 at 2:17 PM, Serge Semin  wrote:
> Link Up and Down methods are used to change NTB link settings on
> local side only for multi-port devices. Link is considered up
> only if both sides local and peer set it up. Intel/AMD hardware
> acts a bit different by assigning the Primary and Secondary roles,
> so Primary device only is able to change the link state. Such behaviour
> should be reflected in the test code.
>
> Signed-off-by: Serge Semin 
> ---
>  tools/testing/selftests/ntb/ntb_test.sh | 26 +++---
>  1 file changed, 15 insertions(+), 11 deletions(-)
>
> diff --git a/tools/testing/selftests/ntb/ntb_test.sh 
> b/tools/testing/selftests/ntb/ntb_test.sh
> index 541ba70ad640..247458c6d8dc 100755
> --- a/tools/testing/selftests/ntb/ntb_test.sh
> +++ b/tools/testing/selftests/ntb/ntb_test.sh
> @@ -138,6 +138,11 @@ function check_file()
> fi
>  }
>
> +function subdirname()
> +{
> +   echo $(basename $(dirname $1)) 2> /dev/nulle

shouldn't this be /dev/null?

> +}
> +
>  function find_pidx()
>  {
> PORT=$1
> @@ -183,9 +188,9 @@ function link_test()
> REM=$2
> EXP=0
>
> -   echo "Running link tests on: $(basename $LOC) / $(basename $REM)"
> +   echo "Running link tests on: $(subdirname $LOC) / $(subdirname $REM)"
>
> -   if ! write_file "N" "$LOC/link" 2> /dev/null; then
> +   if ! write_file "N" "$LOC/../link" 2> /dev/null; then
> echo "  Unsupported"
> return
> fi
> @@ -193,12 +198,11 @@ function link_test()
> write_file "N" "$LOC/link_event"
>
> if [[ $(read_file "$REM/link") != "N" ]]; then
> -   echo "Expected remote link to be down in $REM/link" >&2
> +   echo "Expected link to be down in $REM/link" >&2
> exit -1
> fi
>
> -   write_file "Y" "$LOC/link"
> -   write_file "Y" "$LOC/link_event"
> +   write_file "Y" "$LOC/../link"
>
> echo "  Passed"
>  }
> @@ -379,15 +383,15 @@ function ntb_tool_tests()
>
> port_test "$LOCAL_TOOL" "$REMOTE_TOOL"
>
> -   write_file "Y" "$LOCAL_TOOL/link_event"
> -   write_file "Y" "$REMOTE_TOOL/link_event"
> +   LOCAL_PEER_TOOL="$LOCAL_TOOL/peer$LOCAL_PIDX"
> +   REMOTE_PEER_TOOL="$REMOTE_TOOL/peer$REMOTE_PIDX"
>
> -   link_test "$LOCAL_TOOL" "$REMOTE_TOOL"
> -   link_test "$REMOTE_TOOL" "$LOCAL_TOOL"
> +   link_test "$LOCAL_PEER_TOOL" "$REMOTE_PEER_TOOL"
> +   link_test "$REMOTE_PEER_TOOL" "$LOCAL_PEER_TOOL"
>
> #Ensure the link is up on both sides before continuing
> -   write_file "Y" "$LOCAL_TOOL/link_event"
> -   write_file "Y" "$REMOTE_TOOL/link_event"
> +   write_file "Y" "$LOCAL_PEER_TOOL/link_event"
> +   write_file "Y" "$REMOTE_PEER_TOOL/link_event"
>
> for PEER_TRANS in $(ls "$LOCAL_TOOL"/peer_trans*); do
> PT=$(basename $PEER_TRANS)
> --
> 2.12.0
>

Re: [PATCH v2 09/15] NTB: ntb_test: Update ntb_tool link tests

[Jon Mason]

On Sun, Dec 3, 2017 at 2:17 PM, Serge Semin  wrote:
> Link Up and Down methods are used to change NTB link settings on
> local side only for multi-port devices. Link is considered up
> only if both sides local and peer set it up. Intel/AMD hardware
> acts a bit different by assigning the Primary and Secondary roles,
> so Primary device only is able to change the link state. Such behaviour
> should be reflected in the test code.
>
> Signed-off-by: Serge Semin 
> ---
>  tools/testing/selftests/ntb/ntb_test.sh | 26 +++---
>  1 file changed, 15 insertions(+), 11 deletions(-)
>
> diff --git a/tools/testing/selftests/ntb/ntb_test.sh 
> b/tools/testing/selftests/ntb/ntb_test.sh
> index 541ba70ad640..247458c6d8dc 100755
> --- a/tools/testing/selftests/ntb/ntb_test.sh
> +++ b/tools/testing/selftests/ntb/ntb_test.sh
> @@ -138,6 +138,11 @@ function check_file()
> fi
>  }
>
> +function subdirname()
> +{
> +   echo $(basename $(dirname $1)) 2> /dev/nulle

shouldn't this be /dev/null?

> +}
> +
>  function find_pidx()
>  {
> PORT=$1
> @@ -183,9 +188,9 @@ function link_test()
> REM=$2
> EXP=0
>
> -   echo "Running link tests on: $(basename $LOC) / $(basename $REM)"
> +   echo "Running link tests on: $(subdirname $LOC) / $(subdirname $REM)"
>
> -   if ! write_file "N" "$LOC/link" 2> /dev/null; then
> +   if ! write_file "N" "$LOC/../link" 2> /dev/null; then
> echo "  Unsupported"
> return
> fi
> @@ -193,12 +198,11 @@ function link_test()
> write_file "N" "$LOC/link_event"
>
> if [[ $(read_file "$REM/link") != "N" ]]; then
> -   echo "Expected remote link to be down in $REM/link" >&2
> +   echo "Expected link to be down in $REM/link" >&2
> exit -1
> fi
>
> -   write_file "Y" "$LOC/link"
> -   write_file "Y" "$LOC/link_event"
> +   write_file "Y" "$LOC/../link"
>
> echo "  Passed"
>  }
> @@ -379,15 +383,15 @@ function ntb_tool_tests()
>
> port_test "$LOCAL_TOOL" "$REMOTE_TOOL"
>
> -   write_file "Y" "$LOCAL_TOOL/link_event"
> -   write_file "Y" "$REMOTE_TOOL/link_event"
> +   LOCAL_PEER_TOOL="$LOCAL_TOOL/peer$LOCAL_PIDX"
> +   REMOTE_PEER_TOOL="$REMOTE_TOOL/peer$REMOTE_PIDX"
>
> -   link_test "$LOCAL_TOOL" "$REMOTE_TOOL"
> -   link_test "$REMOTE_TOOL" "$LOCAL_TOOL"
> +   link_test "$LOCAL_PEER_TOOL" "$REMOTE_PEER_TOOL"
> +   link_test "$REMOTE_PEER_TOOL" "$LOCAL_PEER_TOOL"
>
> #Ensure the link is up on both sides before continuing
> -   write_file "Y" "$LOCAL_TOOL/link_event"
> -   write_file "Y" "$REMOTE_TOOL/link_event"
> +   write_file "Y" "$LOCAL_PEER_TOOL/link_event"
> +   write_file "Y" "$REMOTE_PEER_TOOL/link_event"
>
> for PEER_TRANS in $(ls "$LOCAL_TOOL"/peer_trans*); do
> PT=$(basename $PEER_TRANS)
> --
> 2.12.0
>

Re: [PATCH v2 07/15] NTB: ntb_test: Safely use paths with whitespace

[Jon Mason]

On Sun, Dec 3, 2017 at 2:17 PM, Serge Semin  wrote:
> If some of the variables like LOC/REM or LOCAL_*/REMOTE_* got
> whitespaces, the script may fail with syntax error.
>
> Signed-off-by: Serge Semin 
> Acked-by: Logan Gunthorpe 
> Fixes: a9c59ef77458 ("ntb_test: Add a selftest script for the NTB subsystem")

Looks fine to me

> ---
>  tools/testing/selftests/ntb/ntb_test.sh | 62 
> -
>  1 file changed, 31 insertions(+), 31 deletions(-)
>
> diff --git a/tools/testing/selftests/ntb/ntb_test.sh 
> b/tools/testing/selftests/ntb/ntb_test.sh
> index 5fc7ad359e21..a8647ad891eb 100755
> --- a/tools/testing/selftests/ntb/ntb_test.sh
> +++ b/tools/testing/selftests/ntb/ntb_test.sh
> @@ -87,7 +87,7 @@ set -e
>
>  function _modprobe()
>  {
> -modprobe "$@"
> +   modprobe "$@"
>
> if [[ "$REMOTE_HOST" != "" ]]; then
> ssh "$REMOTE_HOST" modprobe "$@"
> @@ -274,13 +274,13 @@ function pingpong_test()
>
> echo "Running ping pong tests on: $(basename $LOC) / $(basename $REM)"
>
> -   LOC_START=$(read_file $LOC/count)
> -   REM_START=$(read_file $REM/count)
> +   LOC_START=$(read_file "$LOC/count")
> +   REM_START=$(read_file "$REM/count")
>
> sleep 7
>
> -   LOC_END=$(read_file $LOC/count)
> -   REM_END=$(read_file $REM/count)
> +   LOC_END=$(read_file "$LOC/count")
> +   REM_END=$(read_file "$REM/count")
>
> if [[ $LOC_START == $LOC_END ]] || [[ $REM_START == $REM_END ]]; then
> echo "Ping pong counter not incrementing!" >&2
> @@ -304,15 +304,15 @@ function perf_test()
> max_mw_size=$MAX_MW_SIZE use_dma=$USE_DMA
>
> echo "Running local perf test $WITH DMA"
> -   write_file "" $LOCAL_PERF/run
> +   write_file "" "$LOCAL_PERF/run"
> echo -n "  "
> -   read_file $LOCAL_PERF/run
> +   read_file "$LOCAL_PERF/run"
> echo "  Passed"
>
> echo "Running remote perf test $WITH DMA"
> -   write_file "" $REMOTE_PERF/run
> +   write_file "" "$REMOTE_PERF/run"
> echo -n "  "
> -   read_file $REMOTE_PERF/run
> +   read_file "$REMOTE_PERF/run"
> echo "  Passed"
>
> _modprobe -r ntb_perf
> @@ -320,39 +320,39 @@ function perf_test()
>
>  function ntb_tool_tests()
>  {
> -   LOCAL_TOOL=$DEBUGFS/ntb_tool/$LOCAL_DEV
> -   REMOTE_TOOL=$REMOTE_HOST:$DEBUGFS/ntb_tool/$REMOTE_DEV
> +   LOCAL_TOOL="$DEBUGFS/ntb_tool/$LOCAL_DEV"
> +   REMOTE_TOOL="$REMOTE_HOST:$DEBUGFS/ntb_tool/$REMOTE_DEV"
>
> echo "Starting ntb_tool tests..."
>
> _modprobe ntb_tool
>
> -   write_file Y $LOCAL_TOOL/link_event
> -   write_file Y $REMOTE_TOOL/link_event
> +   write_file "Y" "$LOCAL_TOOL/link_event"
> +   write_file "Y" "$REMOTE_TOOL/link_event"
>
> -   link_test $LOCAL_TOOL $REMOTE_TOOL
> -   link_test $REMOTE_TOOL $LOCAL_TOOL
> +   link_test "$LOCAL_TOOL" "$REMOTE_TOOL"
> +   link_test "$REMOTE_TOOL" "$LOCAL_TOOL"
>
> #Ensure the link is up on both sides before continuing
> -   write_file Y $LOCAL_TOOL/link_event
> -   write_file Y $REMOTE_TOOL/link_event
> +   write_file "Y" "$LOCAL_TOOL/link_event"
> +   write_file "Y" "$REMOTE_TOOL/link_event"
>
> -   for PEER_TRANS in $(ls $LOCAL_TOOL/peer_trans*); do
> +   for PEER_TRANS in $(ls "$LOCAL_TOOL"/peer_trans*); do
> PT=$(basename $PEER_TRANS)
> -   write_file $MW_SIZE $LOCAL_TOOL/$PT
> -   write_file $MW_SIZE $REMOTE_TOOL/$PT
> +   write_file $MW_SIZE "$LOCAL_TOOL/$PT"
> +   write_file $MW_SIZE "$REMOTE_TOOL/$PT"
> done
>
> -   doorbell_test $LOCAL_TOOL $REMOTE_TOOL
> -   doorbell_test $REMOTE_TOOL $LOCAL_TOOL
> -   scratchpad_test $LOCAL_TOOL $REMOTE_TOOL
> -   scratchpad_test $REMOTE_TOOL $LOCAL_TOOL
> +   doorbell_test "$LOCAL_TOOL" "$REMOTE_TOOL"
> +   doorbell_test "$REMOTE_TOOL" "$LOCAL_TOOL"
> +   scratchpad_test "$LOCAL_TOOL" "$REMOTE_TOOL"
> +   scratchpad_test "$REMOTE_TOOL" "$LOCAL_TOOL"
>
> -   for MW in $(ls $LOCAL_TOOL/mw*); do
> +   for MW in $(ls "$LOCAL_TOOL"/mw*); do
> MW=$(basename $MW)
>
> -   mw_test $MW $LOCAL_TOOL $REMOTE_TOOL
> -   mw_test $MW $REMOTE_TOOL $LOCAL_TOOL
> +   mw_test $MW "$LOCAL_TOOL" "$REMOTE_TOOL"
> +   mw_test $MW "$REMOTE_TOOL" "$LOCAL_TOOL"
> done
>
> _modprobe -r ntb_tool
> @@ -360,8 +360,8 @@ function ntb_tool_tests()
>
>  function ntb_pingpong_tests()
>  {
> -   LOCAL_PP=$DEBUGFS/ntb_pingpong/$LOCAL_DEV
> -   REMOTE_PP=$REMOTE_HOST:$DEBUGFS/ntb_pingpong/$REMOTE_DEV
> +   LOCAL_PP="$DEBUGFS/ntb_pingpong/$LOCAL_DEV"
> +   REMOTE_PP="$REMOTE_HOST:$DEBUGFS/ntb_pingpong/$REMOTE_DEV"
>
> echo "Starting ntb_pingpong tests..."
>
> @@ -374,8 +374,8 @@ function 

Re: [PATCH v2 07/15] NTB: ntb_test: Safely use paths with whitespace

[Jon Mason]

On Sun, Dec 3, 2017 at 2:17 PM, Serge Semin  wrote:
> If some of the variables like LOC/REM or LOCAL_*/REMOTE_* got
> whitespaces, the script may fail with syntax error.
>
> Signed-off-by: Serge Semin 
> Acked-by: Logan Gunthorpe 
> Fixes: a9c59ef77458 ("ntb_test: Add a selftest script for the NTB subsystem")

Looks fine to me

> ---
>  tools/testing/selftests/ntb/ntb_test.sh | 62 
> -
>  1 file changed, 31 insertions(+), 31 deletions(-)
>
> diff --git a/tools/testing/selftests/ntb/ntb_test.sh 
> b/tools/testing/selftests/ntb/ntb_test.sh
> index 5fc7ad359e21..a8647ad891eb 100755
> --- a/tools/testing/selftests/ntb/ntb_test.sh
> +++ b/tools/testing/selftests/ntb/ntb_test.sh
> @@ -87,7 +87,7 @@ set -e
>
>  function _modprobe()
>  {
> -modprobe "$@"
> +   modprobe "$@"
>
> if [[ "$REMOTE_HOST" != "" ]]; then
> ssh "$REMOTE_HOST" modprobe "$@"
> @@ -274,13 +274,13 @@ function pingpong_test()
>
> echo "Running ping pong tests on: $(basename $LOC) / $(basename $REM)"
>
> -   LOC_START=$(read_file $LOC/count)
> -   REM_START=$(read_file $REM/count)
> +   LOC_START=$(read_file "$LOC/count")
> +   REM_START=$(read_file "$REM/count")
>
> sleep 7
>
> -   LOC_END=$(read_file $LOC/count)
> -   REM_END=$(read_file $REM/count)
> +   LOC_END=$(read_file "$LOC/count")
> +   REM_END=$(read_file "$REM/count")
>
> if [[ $LOC_START == $LOC_END ]] || [[ $REM_START == $REM_END ]]; then
> echo "Ping pong counter not incrementing!" >&2
> @@ -304,15 +304,15 @@ function perf_test()
> max_mw_size=$MAX_MW_SIZE use_dma=$USE_DMA
>
> echo "Running local perf test $WITH DMA"
> -   write_file "" $LOCAL_PERF/run
> +   write_file "" "$LOCAL_PERF/run"
> echo -n "  "
> -   read_file $LOCAL_PERF/run
> +   read_file "$LOCAL_PERF/run"
> echo "  Passed"
>
> echo "Running remote perf test $WITH DMA"
> -   write_file "" $REMOTE_PERF/run
> +   write_file "" "$REMOTE_PERF/run"
> echo -n "  "
> -   read_file $REMOTE_PERF/run
> +   read_file "$REMOTE_PERF/run"
> echo "  Passed"
>
> _modprobe -r ntb_perf
> @@ -320,39 +320,39 @@ function perf_test()
>
>  function ntb_tool_tests()
>  {
> -   LOCAL_TOOL=$DEBUGFS/ntb_tool/$LOCAL_DEV
> -   REMOTE_TOOL=$REMOTE_HOST:$DEBUGFS/ntb_tool/$REMOTE_DEV
> +   LOCAL_TOOL="$DEBUGFS/ntb_tool/$LOCAL_DEV"
> +   REMOTE_TOOL="$REMOTE_HOST:$DEBUGFS/ntb_tool/$REMOTE_DEV"
>
> echo "Starting ntb_tool tests..."
>
> _modprobe ntb_tool
>
> -   write_file Y $LOCAL_TOOL/link_event
> -   write_file Y $REMOTE_TOOL/link_event
> +   write_file "Y" "$LOCAL_TOOL/link_event"
> +   write_file "Y" "$REMOTE_TOOL/link_event"
>
> -   link_test $LOCAL_TOOL $REMOTE_TOOL
> -   link_test $REMOTE_TOOL $LOCAL_TOOL
> +   link_test "$LOCAL_TOOL" "$REMOTE_TOOL"
> +   link_test "$REMOTE_TOOL" "$LOCAL_TOOL"
>
> #Ensure the link is up on both sides before continuing
> -   write_file Y $LOCAL_TOOL/link_event
> -   write_file Y $REMOTE_TOOL/link_event
> +   write_file "Y" "$LOCAL_TOOL/link_event"
> +   write_file "Y" "$REMOTE_TOOL/link_event"
>
> -   for PEER_TRANS in $(ls $LOCAL_TOOL/peer_trans*); do
> +   for PEER_TRANS in $(ls "$LOCAL_TOOL"/peer_trans*); do
> PT=$(basename $PEER_TRANS)
> -   write_file $MW_SIZE $LOCAL_TOOL/$PT
> -   write_file $MW_SIZE $REMOTE_TOOL/$PT
> +   write_file $MW_SIZE "$LOCAL_TOOL/$PT"
> +   write_file $MW_SIZE "$REMOTE_TOOL/$PT"
> done
>
> -   doorbell_test $LOCAL_TOOL $REMOTE_TOOL
> -   doorbell_test $REMOTE_TOOL $LOCAL_TOOL
> -   scratchpad_test $LOCAL_TOOL $REMOTE_TOOL
> -   scratchpad_test $REMOTE_TOOL $LOCAL_TOOL
> +   doorbell_test "$LOCAL_TOOL" "$REMOTE_TOOL"
> +   doorbell_test "$REMOTE_TOOL" "$LOCAL_TOOL"
> +   scratchpad_test "$LOCAL_TOOL" "$REMOTE_TOOL"
> +   scratchpad_test "$REMOTE_TOOL" "$LOCAL_TOOL"
>
> -   for MW in $(ls $LOCAL_TOOL/mw*); do
> +   for MW in $(ls "$LOCAL_TOOL"/mw*); do
> MW=$(basename $MW)
>
> -   mw_test $MW $LOCAL_TOOL $REMOTE_TOOL
> -   mw_test $MW $REMOTE_TOOL $LOCAL_TOOL
> +   mw_test $MW "$LOCAL_TOOL" "$REMOTE_TOOL"
> +   mw_test $MW "$REMOTE_TOOL" "$LOCAL_TOOL"
> done
>
> _modprobe -r ntb_tool
> @@ -360,8 +360,8 @@ function ntb_tool_tests()
>
>  function ntb_pingpong_tests()
>  {
> -   LOCAL_PP=$DEBUGFS/ntb_pingpong/$LOCAL_DEV
> -   REMOTE_PP=$REMOTE_HOST:$DEBUGFS/ntb_pingpong/$REMOTE_DEV
> +   LOCAL_PP="$DEBUGFS/ntb_pingpong/$LOCAL_DEV"
> +   REMOTE_PP="$REMOTE_HOST:$DEBUGFS/ntb_pingpong/$REMOTE_DEV"
>
> echo "Starting ntb_pingpong tests..."
>
> @@ -374,8 +374,8 @@ function ntb_pingpong_tests()
>
>  function ntb_perf_tests()
>  {
> -   

Re: [PATCH v2 06/15] NTB: ntb_perf: Add full multi-port NTB API support

[Jon Mason]

On Sun, Dec 3, 2017 at 2:17 PM, Serge Semin  wrote:
> NTB API has been updated to support multi-port devices like IDT
> 89HPESx series or Microsemi Switchtec. Message registers
> functionality has also been added to new API. In order to keep
> the new hardware and corresponding capabilities well tested, NTB
> performance driver is accordingly altered.
>
> Signed-off-by: Serge Semin 
> ---
>
> Changelog v1:
> - Alter interface in compliance with multi-port API
> - Create Scratchpads and Message compliant NTB link init process
> - Simplify memcpy and DMA-based tests process
> - Limit DebugFS access methods to prevent the driver failure
>
> Changelog v2:
> - Remove driver Author/Description/License macros
> - Replace magic numbers with macros
> - Fix global index calc error for devices with highest port index
> - Fix invalid scratchpad selection in send_msg algorithm
> - Fix bytes order translation in message recv method
> - Use local global index to select an inbound MW
> - Add commands send/recv/exec debug prints
> - Call ntb_mw_get_align when link is supposed to be up
>
>  drivers/ntb/test/ntb_perf.c | 1826 
> +--
>  1 file changed, 1222 insertions(+), 604 deletions(-)
>
> diff --git a/drivers/ntb/test/ntb_perf.c b/drivers/ntb/test/ntb_perf.c
> index 427112cf101a..5ca5cc6f9687 100644
> --- a/drivers/ntb/test/ntb_perf.c
> +++ b/drivers/ntb/test/ntb_perf.c
> @@ -5,6 +5,7 @@
>   *   GPL LICENSE SUMMARY
>   *
>   *   Copyright(c) 2015 Intel Corporation. All rights reserved.
> + *   Copyright(c) 2017 T-Platforms. All Rights Reserved.
>   *
>   *   This program is free software; you can redistribute it and/or modify
>   *   it under the terms of version 2 of the GNU General Public License as
> @@ -13,6 +14,7 @@
>   *   BSD LICENSE
>   *
>   *   Copyright(c) 2015 Intel Corporation. All rights reserved.
> + *   Copyright(c) 2017 T-Platforms. All Rights Reserved.
>   *
>   *   Redistribution and use in source and binary forms, with or without
>   *   modification, are permitted provided that the following conditions
> @@ -40,860 +42,1476 @@
>   *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
>   *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>   *
> - *   PCIe NTB Perf Linux driver
> + * PCIe NTB Perf Linux driver
> + *
> + * Contact Information:
> + * Dave Jiang 
> + * Serge Semin , 
> + */
> +
> +/*
> + * How to use this tool, by example.
> + *
> + * Assuming $DBG_DIR is something like:
> + * '/sys/kernel/debug/ntb_perf/:00:03.0'
> + * Suppose aside from local device there is at least one remote device
> + * connected to NTB with index 0.
> + 
> *-
> + * Eg: install driver with specified chunk/total orders and dma-enabled flag
> + *
> + * root@self# insmod ntb_perf.ko chunk_order=19 total_order=28 use_dma
> + 
> *-
> + * Eg: check NTB ports (index) and MW mapping information
> + *
> + * root@self# cat $DBG_DIR/info
> + 
> *-
> + * Eg: start performance test with peer (index 0) and get the test metrics
> + *
> + * root@self# echo 0 > $DBG_DIR/run
> + * root@self# cat $DBG_DIR/run
>   */
>
>  #include 
>  #include 
>  #include 
> -#include 
> -#include 
> -#include 
> +#include 
> +#include 
>  #include 
> +#include 
>  #include 
> +#include 
>  #include 
> -#include 
> -#include 
> -#include 
>  #include 
>  #include 
> +#include 
> +#include 
> +#include 
>  #include 
> -#include 
>
>  #define DRIVER_NAME"ntb_perf"
> -#define DRIVER_DESCRIPTION "PCIe NTB Performance Measurement Tool"
> -
> -#define DRIVER_LICENSE "Dual BSD/GPL"
> -#define DRIVER_VERSION "1.0"
> -#define DRIVER_AUTHOR  "Dave Jiang "
> -
> -#define PERF_LINK_DOWN_TIMEOUT 10
> -#define PERF_VERSION   0x0001
> -#define MAX_THREADS32
> -#define MAX_TEST_SIZE  SZ_1M
> -#define MAX_SRCS   32
> -#define DMA_OUT_RESOURCE_TOmsecs_to_jiffies(50)
> -#define DMA_RETRIES20
> -#define SZ_4G  (1ULL << 32)
> -#define MAX_SEG_ORDER  20 /* no larger than 1M for kmalloc buffer */
> -#define PIDX   NTB_DEF_PEER_IDX
> -
> -MODULE_LICENSE(DRIVER_LICENSE);
> +#define DRIVER_VERSION "2.0"
> +
> +MODULE_LICENSE("Dual BSD/GPL");
>  MODULE_VERSION(DRIVER_VERSION);
> -MODULE_AUTHOR(DRIVER_AUTHOR);
> -MODULE_DESCRIPTION(DRIVER_DESCRIPTION);
> +MODULE_AUTHOR("Dave Jiang ");
> +MODULE_DESCRIPTION("PCIe NTB Performance Measurement Tool");
>
> -static struct dentry *perf_debugfs_dir;
> +#define MAX_THREADS_CNT32
> +#define 

Re: [PATCH v2 06/15] NTB: ntb_perf: Add full multi-port NTB API support

[Jon Mason]

On Sun, Dec 3, 2017 at 2:17 PM, Serge Semin  wrote:
> NTB API has been updated to support multi-port devices like IDT
> 89HPESx series or Microsemi Switchtec. Message registers
> functionality has also been added to new API. In order to keep
> the new hardware and corresponding capabilities well tested, NTB
> performance driver is accordingly altered.
>
> Signed-off-by: Serge Semin 
> ---
>
> Changelog v1:
> - Alter interface in compliance with multi-port API
> - Create Scratchpads and Message compliant NTB link init process
> - Simplify memcpy and DMA-based tests process
> - Limit DebugFS access methods to prevent the driver failure
>
> Changelog v2:
> - Remove driver Author/Description/License macros
> - Replace magic numbers with macros
> - Fix global index calc error for devices with highest port index
> - Fix invalid scratchpad selection in send_msg algorithm
> - Fix bytes order translation in message recv method
> - Use local global index to select an inbound MW
> - Add commands send/recv/exec debug prints
> - Call ntb_mw_get_align when link is supposed to be up
>
>  drivers/ntb/test/ntb_perf.c | 1826 
> +--
>  1 file changed, 1222 insertions(+), 604 deletions(-)
>
> diff --git a/drivers/ntb/test/ntb_perf.c b/drivers/ntb/test/ntb_perf.c
> index 427112cf101a..5ca5cc6f9687 100644
> --- a/drivers/ntb/test/ntb_perf.c
> +++ b/drivers/ntb/test/ntb_perf.c
> @@ -5,6 +5,7 @@
>   *   GPL LICENSE SUMMARY
>   *
>   *   Copyright(c) 2015 Intel Corporation. All rights reserved.
> + *   Copyright(c) 2017 T-Platforms. All Rights Reserved.
>   *
>   *   This program is free software; you can redistribute it and/or modify
>   *   it under the terms of version 2 of the GNU General Public License as
> @@ -13,6 +14,7 @@
>   *   BSD LICENSE
>   *
>   *   Copyright(c) 2015 Intel Corporation. All rights reserved.
> + *   Copyright(c) 2017 T-Platforms. All Rights Reserved.
>   *
>   *   Redistribution and use in source and binary forms, with or without
>   *   modification, are permitted provided that the following conditions
> @@ -40,860 +42,1476 @@
>   *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
>   *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>   *
> - *   PCIe NTB Perf Linux driver
> + * PCIe NTB Perf Linux driver
> + *
> + * Contact Information:
> + * Dave Jiang 
> + * Serge Semin , 
> + */
> +
> +/*
> + * How to use this tool, by example.
> + *
> + * Assuming $DBG_DIR is something like:
> + * '/sys/kernel/debug/ntb_perf/:00:03.0'
> + * Suppose aside from local device there is at least one remote device
> + * connected to NTB with index 0.
> + 
> *-
> + * Eg: install driver with specified chunk/total orders and dma-enabled flag
> + *
> + * root@self# insmod ntb_perf.ko chunk_order=19 total_order=28 use_dma
> + 
> *-
> + * Eg: check NTB ports (index) and MW mapping information
> + *
> + * root@self# cat $DBG_DIR/info
> + 
> *-
> + * Eg: start performance test with peer (index 0) and get the test metrics
> + *
> + * root@self# echo 0 > $DBG_DIR/run
> + * root@self# cat $DBG_DIR/run
>   */
>
>  #include 
>  #include 
>  #include 
> -#include 
> -#include 
> -#include 
> +#include 
> +#include 
>  #include 
> +#include 
>  #include 
> +#include 
>  #include 
> -#include 
> -#include 
> -#include 
>  #include 
>  #include 
> +#include 
> +#include 
> +#include 
>  #include 
> -#include 
>
>  #define DRIVER_NAME"ntb_perf"
> -#define DRIVER_DESCRIPTION "PCIe NTB Performance Measurement Tool"
> -
> -#define DRIVER_LICENSE "Dual BSD/GPL"
> -#define DRIVER_VERSION "1.0"
> -#define DRIVER_AUTHOR  "Dave Jiang "
> -
> -#define PERF_LINK_DOWN_TIMEOUT 10
> -#define PERF_VERSION   0x0001
> -#define MAX_THREADS32
> -#define MAX_TEST_SIZE  SZ_1M
> -#define MAX_SRCS   32
> -#define DMA_OUT_RESOURCE_TOmsecs_to_jiffies(50)
> -#define DMA_RETRIES20
> -#define SZ_4G  (1ULL << 32)
> -#define MAX_SEG_ORDER  20 /* no larger than 1M for kmalloc buffer */
> -#define PIDX   NTB_DEF_PEER_IDX
> -
> -MODULE_LICENSE(DRIVER_LICENSE);
> +#define DRIVER_VERSION "2.0"
> +
> +MODULE_LICENSE("Dual BSD/GPL");
>  MODULE_VERSION(DRIVER_VERSION);
> -MODULE_AUTHOR(DRIVER_AUTHOR);
> -MODULE_DESCRIPTION(DRIVER_DESCRIPTION);
> +MODULE_AUTHOR("Dave Jiang ");
> +MODULE_DESCRIPTION("PCIe NTB Performance Measurement Tool");
>
> -static struct dentry *perf_debugfs_dir;
> +#define MAX_THREADS_CNT32
> +#define DEF_THREADS_CNT1
> +#define MAX_CHUNK_SIZE SZ_1M
> +#define MAX_CHUNK_ORDER20 /* no larger than 1M */
> +
> +#define DMA_TRIES  

Re: [PATCH v2 03/15] NTB: Fix UB/bug in ntb_mw_get_align()

[Jon Mason]

On Tue, Dec 5, 2017 at 12:56 PM, Serge Semin <fancer.lan...@gmail.com> wrote:
> On Tue, Dec 05, 2017 at 11:52:32AM -0500, Jon Mason <jdma...@kudzu.us> wrote:
>> On Sun, Dec 3, 2017 at 2:17 PM, Serge Semin <fancer.lan...@gmail.com> wrote:
>> > Simple (1 << pidx) operation causes undefined behaviour when
>> > pidx >= 32. It must be casted to u64 to match the actual return
>> > value of ntb_link_is_up() method, so to have all the possible
>> > peer indexes covered and to get rid of undefined behaviour.
>> > Additionally there are special macros in "linux/bitops.h" to perform
>> > the bit-set-shift operations, so it's recommended to have them used
>> > for proper bit setting.
>>
>> This looks good to me, but also seems like a bug fix.  Please comment
>> on if this is not noticed.
>>
>
> The consequences of the bug isn't noticeable at the moment, but potentially
> it can be seen on the devices like Switchtec (when multi-portness is finally
> added). Anyway it's better to fix it now, than to wait for obvious bug.
> Additionally as I said the originator of the code should have used BIT_ULL
> instead of the pure bit shifting.

This is more of a question of whether it should go into -stable and be
outside this patch set.  If this is impossible to ever hit, then
including it here instead of fast tracking it in -stable is fine.

Thanks,
Jon

>
> Thanks,
> -Sergey
>
>> Thanks,
>> Jon
>>
>> >
>> > Signed-off-by: Serge Semin <fancer.lan...@gmail.com>
>> > Reviewed-by: Logan Gunthorpe <log...@deltatee.com>
>> > ---
>> >  include/linux/ntb.h | 2 +-
>> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/include/linux/ntb.h b/include/linux/ntb.h
>> > index c1646f2c6344..488e586fb76c 100644
>> > --- a/include/linux/ntb.h
>> > +++ b/include/linux/ntb.h
>> > @@ -764,7 +764,7 @@ static inline int ntb_mw_get_align(struct ntb_dev 
>> > *ntb, int pidx, int widx,
>> >resource_size_t *size_align,
>> >resource_size_t *size_max)
>> >  {
>> > -   if (!(ntb_link_is_up(ntb, NULL, NULL) & (1 << pidx)))
>> > +   if (!(ntb_link_is_up(ntb, NULL, NULL) & BIT_ULL(pidx)))
>> > return -ENOTCONN;
>> >
>> > return ntb->ops->mw_get_align(ntb, pidx, widx, addr_align, 
>> > size_align,
>> > --
>> > 2.12.0
>> >

Re: [PATCH v2 03/15] NTB: Fix UB/bug in ntb_mw_get_align()

[Jon Mason]

On Tue, Dec 5, 2017 at 12:56 PM, Serge Semin  wrote:
> On Tue, Dec 05, 2017 at 11:52:32AM -0500, Jon Mason  wrote:
>> On Sun, Dec 3, 2017 at 2:17 PM, Serge Semin  wrote:
>> > Simple (1 << pidx) operation causes undefined behaviour when
>> > pidx >= 32. It must be casted to u64 to match the actual return
>> > value of ntb_link_is_up() method, so to have all the possible
>> > peer indexes covered and to get rid of undefined behaviour.
>> > Additionally there are special macros in "linux/bitops.h" to perform
>> > the bit-set-shift operations, so it's recommended to have them used
>> > for proper bit setting.
>>
>> This looks good to me, but also seems like a bug fix.  Please comment
>> on if this is not noticed.
>>
>
> The consequences of the bug isn't noticeable at the moment, but potentially
> it can be seen on the devices like Switchtec (when multi-portness is finally
> added). Anyway it's better to fix it now, than to wait for obvious bug.
> Additionally as I said the originator of the code should have used BIT_ULL
> instead of the pure bit shifting.

This is more of a question of whether it should go into -stable and be
outside this patch set.  If this is impossible to ever hit, then
including it here instead of fast tracking it in -stable is fine.

Thanks,
Jon

>
> Thanks,
> -Sergey
>
>> Thanks,
>> Jon
>>
>> >
>> > Signed-off-by: Serge Semin 
>> > Reviewed-by: Logan Gunthorpe 
>> > ---
>> >  include/linux/ntb.h | 2 +-
>> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/include/linux/ntb.h b/include/linux/ntb.h
>> > index c1646f2c6344..488e586fb76c 100644
>> > --- a/include/linux/ntb.h
>> > +++ b/include/linux/ntb.h
>> > @@ -764,7 +764,7 @@ static inline int ntb_mw_get_align(struct ntb_dev 
>> > *ntb, int pidx, int widx,
>> >resource_size_t *size_align,
>> >resource_size_t *size_max)
>> >  {
>> > -   if (!(ntb_link_is_up(ntb, NULL, NULL) & (1 << pidx)))
>> > +   if (!(ntb_link_is_up(ntb, NULL, NULL) & BIT_ULL(pidx)))
>> > return -ENOTCONN;
>> >
>> > return ntb->ops->mw_get_align(ntb, pidx, widx, addr_align, 
>> > size_align,
>> > --
>> > 2.12.0
>> >

Re: [PATCH v2 01/15] NTB: Rename NTB messaging API methods

[Jon Mason]

On Tue, Dec 5, 2017 at 12:31 PM, Serge Semin <fancer.lan...@gmail.com> wrote:
> On Tue, Dec 05, 2017 at 11:49:10AM -0500, Jon Mason <jdma...@kudzu.us> wrote:
>> On Sun, Dec 3, 2017 at 2:17 PM, Serge Semin <fancer.lan...@gmail.com> wrote:
>> > There is a common methods signature form used over all the NTB API
>> > like functions naming scheme, arguments names and order, etc.
>> > Recently added NTB messaging API IO callbacks were named a bit
>> > different so should be renamed to be in compliance with the rest
>> > of the API. The changes are made in a way so the developers won't
>> > be able to compile their code without being informed by the compiler.
>> >
>> > Signed-off-by: Serge Semin <fancer.lan...@gmail.com>
>> > ---
>> >  drivers/ntb/hw/idt/ntb_hw_idt.c | 27 ---
>> >  include/linux/ntb.h | 34 --
>> >  2 files changed, 28 insertions(+), 33 deletions(-)
>> >
>> > diff --git a/drivers/ntb/hw/idt/ntb_hw_idt.c 
>> > b/drivers/ntb/hw/idt/ntb_hw_idt.c
>> > index 0cd79f367f7c..6fb87c0f0d97 100644
>> > --- a/drivers/ntb/hw/idt/ntb_hw_idt.c
>> > +++ b/drivers/ntb/hw/idt/ntb_hw_idt.c
>> > @@ -1744,20 +1744,19 @@ static int idt_ntb_msg_clear_mask(struct ntb_dev 
>> > *ntb, u64 mask_bits)
>> >   * idt_ntb_msg_read() - read message register with specified index
>> >   * (NTB API callback)
>> >   * @ntb:   NTB device context.
>> > - * @midx:  Message register index
>> >   * @pidx:  OUT - Port index of peer device a message retrieved from
>> > - * @msg:   OUT - Data
>> > + * @midx:  Message register index
>> >   *
>> >   * Read data from the specified message register and source register.
>> >   *
>> > - * Return: zero on success, negative error if invalid argument passed.
>> > + * Return: inbound message register value.
>> >   */
>> > -static int idt_ntb_msg_read(struct ntb_dev *ntb, int midx, int *pidx, u32 
>> > *msg)
>> > +static u32 idt_ntb_msg_read(struct ntb_dev *ntb, int *pidx, int midx)
>> >  {
>> > struct idt_ntb_dev *ndev = to_ndev_ntb(ntb);
>> >
>> > if (midx < 0 || IDT_MSG_CNT <= midx)
>> > -   return -EINVAL;
>> > +   return (u32)-1;
>>
>> Please don't do this.  If this is an error return standard error
>> number.  And why are we casting to an unsigned int now?
>>
>
> We discussed these changes on the v1 series. Additionally I asked similar
> question sometime ago even before the patchset was introduced.
> This patch is made to provide the Message interface similar to the Scratchpad
> one. I didn't introduce anything new or unjustified. As you can see the
> spad_read/peer_spad_read methods return u32 type too. As well as the
> Intel/AMD callbacks. The functions are intentionally made to return FFs
> in case if some of the passed arguments get out from the allowed limits.
> In such circumstances the return value emulates a situation like if user would
> reference an invalid PCIe MMIO address. Since the 32-bits register can in 
> general
> have any value including -errno ones, then returning an error within the NTB 
> API
> would be incorrect. I remember Allen described it this way.
> Nobody argued about it last time. If you think it's incorrect, then it should 
> be
> changed in both Scratchpad and Message register interfaces.

Just because no one said anything before, doesn't mean it's
acceptable.  I think that is the official Linux code review mantra ;-)

Okay, if we are going to do it this way, then return ~0 instead.  At
least that way there is no ugly cast.

>
>> >
>> > /* Retrieve source port index of the message */
>> > if (pidx != NULL) {
>> > @@ -1772,18 +1771,15 @@ static int idt_ntb_msg_read(struct ntb_dev *ntb, 
>> > int midx, int *pidx, u32 *msg)
>> > }
>> >
>> > /* Retrieve data of the corresponding message register */
>> > -   if (msg != NULL)
>> > -   *msg = idt_nt_read(ndev, ntdata_tbl.msgs[midx].in);
>> > -
>> > -   return 0;
>> > +   return idt_nt_read(ndev, ntdata_tbl.msgs[midx].in);
>> >  }
>> >
>> >  /*
>> > - * idt_ntb_msg_write() - write data to the specified message register
>> > - *  (NTB API callback)
>> > + * idt_ntb_peer_msg_write() - write data to the specified message register
>> > + *  

Re: [PATCH v2 01/15] NTB: Rename NTB messaging API methods

[Jon Mason]

On Tue, Dec 5, 2017 at 12:31 PM, Serge Semin  wrote:
> On Tue, Dec 05, 2017 at 11:49:10AM -0500, Jon Mason  wrote:
>> On Sun, Dec 3, 2017 at 2:17 PM, Serge Semin  wrote:
>> > There is a common methods signature form used over all the NTB API
>> > like functions naming scheme, arguments names and order, etc.
>> > Recently added NTB messaging API IO callbacks were named a bit
>> > different so should be renamed to be in compliance with the rest
>> > of the API. The changes are made in a way so the developers won't
>> > be able to compile their code without being informed by the compiler.
>> >
>> > Signed-off-by: Serge Semin 
>> > ---
>> >  drivers/ntb/hw/idt/ntb_hw_idt.c | 27 ---
>> >  include/linux/ntb.h | 34 --
>> >  2 files changed, 28 insertions(+), 33 deletions(-)
>> >
>> > diff --git a/drivers/ntb/hw/idt/ntb_hw_idt.c 
>> > b/drivers/ntb/hw/idt/ntb_hw_idt.c
>> > index 0cd79f367f7c..6fb87c0f0d97 100644
>> > --- a/drivers/ntb/hw/idt/ntb_hw_idt.c
>> > +++ b/drivers/ntb/hw/idt/ntb_hw_idt.c
>> > @@ -1744,20 +1744,19 @@ static int idt_ntb_msg_clear_mask(struct ntb_dev 
>> > *ntb, u64 mask_bits)
>> >   * idt_ntb_msg_read() - read message register with specified index
>> >   * (NTB API callback)
>> >   * @ntb:   NTB device context.
>> > - * @midx:  Message register index
>> >   * @pidx:  OUT - Port index of peer device a message retrieved from
>> > - * @msg:   OUT - Data
>> > + * @midx:  Message register index
>> >   *
>> >   * Read data from the specified message register and source register.
>> >   *
>> > - * Return: zero on success, negative error if invalid argument passed.
>> > + * Return: inbound message register value.
>> >   */
>> > -static int idt_ntb_msg_read(struct ntb_dev *ntb, int midx, int *pidx, u32 
>> > *msg)
>> > +static u32 idt_ntb_msg_read(struct ntb_dev *ntb, int *pidx, int midx)
>> >  {
>> > struct idt_ntb_dev *ndev = to_ndev_ntb(ntb);
>> >
>> > if (midx < 0 || IDT_MSG_CNT <= midx)
>> > -   return -EINVAL;
>> > +   return (u32)-1;
>>
>> Please don't do this.  If this is an error return standard error
>> number.  And why are we casting to an unsigned int now?
>>
>
> We discussed these changes on the v1 series. Additionally I asked similar
> question sometime ago even before the patchset was introduced.
> This patch is made to provide the Message interface similar to the Scratchpad
> one. I didn't introduce anything new or unjustified. As you can see the
> spad_read/peer_spad_read methods return u32 type too. As well as the
> Intel/AMD callbacks. The functions are intentionally made to return FFs
> in case if some of the passed arguments get out from the allowed limits.
> In such circumstances the return value emulates a situation like if user would
> reference an invalid PCIe MMIO address. Since the 32-bits register can in 
> general
> have any value including -errno ones, then returning an error within the NTB 
> API
> would be incorrect. I remember Allen described it this way.
> Nobody argued about it last time. If you think it's incorrect, then it should 
> be
> changed in both Scratchpad and Message register interfaces.

Just because no one said anything before, doesn't mean it's
acceptable.  I think that is the official Linux code review mantra ;-)

Okay, if we are going to do it this way, then return ~0 instead.  At
least that way there is no ugly cast.

>
>> >
>> > /* Retrieve source port index of the message */
>> > if (pidx != NULL) {
>> > @@ -1772,18 +1771,15 @@ static int idt_ntb_msg_read(struct ntb_dev *ntb, 
>> > int midx, int *pidx, u32 *msg)
>> > }
>> >
>> > /* Retrieve data of the corresponding message register */
>> > -   if (msg != NULL)
>> > -   *msg = idt_nt_read(ndev, ntdata_tbl.msgs[midx].in);
>> > -
>> > -   return 0;
>> > +   return idt_nt_read(ndev, ntdata_tbl.msgs[midx].in);
>> >  }
>> >
>> >  /*
>> > - * idt_ntb_msg_write() - write data to the specified message register
>> > - *  (NTB API callback)
>> > + * idt_ntb_peer_msg_write() - write data to the specified message register
>> > + *   (NTB API callback)
>> >   * @ntb:   NTB device contex

Re: [PATCH v2 05/15] NTB: ntb_tool: Add full multi-port NTB API support

[Jon Mason]

On Sun, Dec 3, 2017 at 2:17 PM, Serge Semin  wrote:
> NTB API has been updated to support multi-port devices like IDT
> 89HPESx series or Microsemi Switchtec. Message registers
> functionality has also been added to new API. In order to keep
> the new hardware and corresponding capabilities well tested, NTB
> tool driver is accordingly altered.
>
> Signed-off-by: Serge Semin 
> ---
>
> Changelog v1:
> - Alter interface in compliance with multi-port API
> - Move Message/MW/Port/Link settings to a specific directory
>
> Changelog v2:
> - Remove driver Author/Description/License macros
> - Return error if ntb_mw_get_align called while link is down
> - Add db_valid_mask DebugFS file
> - Add msg_inbits/msg_outbits DebugFS files
>
>  drivers/ntb/test/ntb_tool.c | 1805 
> +--
>  1 file changed, 1228 insertions(+), 577 deletions(-)
>
> diff --git a/drivers/ntb/test/ntb_tool.c b/drivers/ntb/test/ntb_tool.c
> index 91526a986caa..e3fbc5944679 100644
> --- a/drivers/ntb/test/ntb_tool.c
> +++ b/drivers/ntb/test/ntb_tool.c
> @@ -5,6 +5,7 @@
>   *   GPL LICENSE SUMMARY
>   *
>   *   Copyright (C) 2015 EMC Corporation. All Rights Reserved.
> + *   Copyright (C) 2017 T-Platforms All Rights Reserved.
>   *
>   *   This program is free software; you can redistribute it and/or modify
>   *   it under the terms of version 2 of the GNU General Public License as
> @@ -18,6 +19,7 @@
>   *   BSD LICENSE
>   *
>   *   Copyright (C) 2015 EMC Corporation. All Rights Reserved.
> + *   Copyright (C) 2017 T-Platforms All Rights Reserved.
>   *
>   *   Redistribution and use in source and binary forms, with or without
>   *   modification, are permitted provided that the following conditions
> @@ -49,6 +51,7 @@
>   *
>   * Contact Information:
>   * Allen Hubbe 
> + * Serge Semin , 
>   */
>
>  /*
> @@ -56,42 +59,125 @@
>   *
>   * Assuming $DBG_DIR is something like:
>   * '/sys/kernel/debug/ntb_tool/:00:03.0'
> + * Suppose aside from local device there is at least one remote device
> + * connected to NTB with index 0.
> + 
> *-
> + * Eg: check local/peer device information.
>   *
> - * Eg: check if clearing the doorbell mask generates an interrupt.
> + * # Get local device port number
> + * root@self# cat $DBG_DIR/port
>   *
> - * # Check the link status
> - * root@self# cat $DBG_DIR/link
> + * # Check local device functionality
> + * root@self# ls $DBG_DIR
> + * dbmsg1  msg_sts peer4/port
> + * db_event  msg2  peer0/  peer5/spad0
> + * db_mask   msg3  peer1/  peer_db   spad1
> + * link  msg_event peer2/  peer_db_mask  spad2
> + * msg0  msg_mask  peer3/  peer_spad spad3
> + * # As one can see it supports:
> + * # 1) four inbound message registers
> + * # 2) four inbound scratchpads
> + * # 3) up to six peer devices
>   *
> - * # Block until the link is up
> - * root@self# echo Y > $DBG_DIR/link_event
> + * # Check peer device port number
> + * root@self# cat $DBG_DIR/peer0/port
>   *
> - * # Set the doorbell mask
> - * root@self# echo 's 1' > $DBG_DIR/mask
> + * # Check peer device(s) functionality to be used
> + * root@self# ls $DBG_DIR/peer0
> + * link mw_trans0   mw_trans6port
> + * link_event   mw_trans1   mw_trans7spad0
> + * msg0 mw_trans2   peer_mw_trans0   spad1
> + * msg1 mw_trans3   peer_mw_trans1   spad2
> + * msg2 mw_trans4   peer_mw_trans2   spad3
> + * msg3 mw_trans5   peer_mw_trans3
> + * # As one can see we got:
> + * # 1) four outbound message registers
> + * # 2) four outbound scratchpads
> + * # 3) eight inbound memory windows
> + * # 4) four outbound memory windows
> + 
> *-
> + * Eg: NTB link tests
>   *
> - * # Ring the doorbell from the peer
> + * # Set local link up/down
> + * root@self# echo Y > $DBG_DIR/link
> + * root@self# echo N > $DBG_DIR/link
> + *
> + * # Check if link with peer device is up/down:
> + * root@self# cat $DBG_DIR/peer0/link
> + *
> + * # Block until the link is up/down
> + * root@self# echo Y > $DBG_DIR/peer0/link_event
> + * root@self# echo N > $DBG_DIR/peer0/link_event
> + 
> *-
> + * Eg: Doorbell registers tests (some functionality might be absent)
> + *
> + * # Set/clear/get local doorbell
> + * root@self# echo 's 1' > $DBG_DIR/db
> + * root@self# echo 'c 1' > $DBG_DIR/db
> + * root@self# cat  $DBG_DIR/db
> + *
> + * # Set/clear/get local doorbell mask
> + * root@self# echo 's 1' > $DBG_DIR/db_mask
> + * root@self# echo 'c 1' > $DBG_DIR/db_mask
> + * root@self# cat $DBG_DIR/db_mask
> 

Re: [PATCH v2 05/15] NTB: ntb_tool: Add full multi-port NTB API support

[Jon Mason]

On Sun, Dec 3, 2017 at 2:17 PM, Serge Semin  wrote:
> NTB API has been updated to support multi-port devices like IDT
> 89HPESx series or Microsemi Switchtec. Message registers
> functionality has also been added to new API. In order to keep
> the new hardware and corresponding capabilities well tested, NTB
> tool driver is accordingly altered.
>
> Signed-off-by: Serge Semin 
> ---
>
> Changelog v1:
> - Alter interface in compliance with multi-port API
> - Move Message/MW/Port/Link settings to a specific directory
>
> Changelog v2:
> - Remove driver Author/Description/License macros
> - Return error if ntb_mw_get_align called while link is down
> - Add db_valid_mask DebugFS file
> - Add msg_inbits/msg_outbits DebugFS files
>
>  drivers/ntb/test/ntb_tool.c | 1805 
> +--
>  1 file changed, 1228 insertions(+), 577 deletions(-)
>
> diff --git a/drivers/ntb/test/ntb_tool.c b/drivers/ntb/test/ntb_tool.c
> index 91526a986caa..e3fbc5944679 100644
> --- a/drivers/ntb/test/ntb_tool.c
> +++ b/drivers/ntb/test/ntb_tool.c
> @@ -5,6 +5,7 @@
>   *   GPL LICENSE SUMMARY
>   *
>   *   Copyright (C) 2015 EMC Corporation. All Rights Reserved.
> + *   Copyright (C) 2017 T-Platforms All Rights Reserved.
>   *
>   *   This program is free software; you can redistribute it and/or modify
>   *   it under the terms of version 2 of the GNU General Public License as
> @@ -18,6 +19,7 @@
>   *   BSD LICENSE
>   *
>   *   Copyright (C) 2015 EMC Corporation. All Rights Reserved.
> + *   Copyright (C) 2017 T-Platforms All Rights Reserved.
>   *
>   *   Redistribution and use in source and binary forms, with or without
>   *   modification, are permitted provided that the following conditions
> @@ -49,6 +51,7 @@
>   *
>   * Contact Information:
>   * Allen Hubbe 
> + * Serge Semin , 
>   */
>
>  /*
> @@ -56,42 +59,125 @@
>   *
>   * Assuming $DBG_DIR is something like:
>   * '/sys/kernel/debug/ntb_tool/:00:03.0'
> + * Suppose aside from local device there is at least one remote device
> + * connected to NTB with index 0.
> + 
> *-
> + * Eg: check local/peer device information.
>   *
> - * Eg: check if clearing the doorbell mask generates an interrupt.
> + * # Get local device port number
> + * root@self# cat $DBG_DIR/port
>   *
> - * # Check the link status
> - * root@self# cat $DBG_DIR/link
> + * # Check local device functionality
> + * root@self# ls $DBG_DIR
> + * dbmsg1  msg_sts peer4/port
> + * db_event  msg2  peer0/  peer5/spad0
> + * db_mask   msg3  peer1/  peer_db   spad1
> + * link  msg_event peer2/  peer_db_mask  spad2
> + * msg0  msg_mask  peer3/  peer_spad spad3
> + * # As one can see it supports:
> + * # 1) four inbound message registers
> + * # 2) four inbound scratchpads
> + * # 3) up to six peer devices
>   *
> - * # Block until the link is up
> - * root@self# echo Y > $DBG_DIR/link_event
> + * # Check peer device port number
> + * root@self# cat $DBG_DIR/peer0/port
>   *
> - * # Set the doorbell mask
> - * root@self# echo 's 1' > $DBG_DIR/mask
> + * # Check peer device(s) functionality to be used
> + * root@self# ls $DBG_DIR/peer0
> + * link mw_trans0   mw_trans6port
> + * link_event   mw_trans1   mw_trans7spad0
> + * msg0 mw_trans2   peer_mw_trans0   spad1
> + * msg1 mw_trans3   peer_mw_trans1   spad2
> + * msg2 mw_trans4   peer_mw_trans2   spad3
> + * msg3 mw_trans5   peer_mw_trans3
> + * # As one can see we got:
> + * # 1) four outbound message registers
> + * # 2) four outbound scratchpads
> + * # 3) eight inbound memory windows
> + * # 4) four outbound memory windows
> + 
> *-
> + * Eg: NTB link tests
>   *
> - * # Ring the doorbell from the peer
> + * # Set local link up/down
> + * root@self# echo Y > $DBG_DIR/link
> + * root@self# echo N > $DBG_DIR/link
> + *
> + * # Check if link with peer device is up/down:
> + * root@self# cat $DBG_DIR/peer0/link
> + *
> + * # Block until the link is up/down
> + * root@self# echo Y > $DBG_DIR/peer0/link_event
> + * root@self# echo N > $DBG_DIR/peer0/link_event
> + 
> *-
> + * Eg: Doorbell registers tests (some functionality might be absent)
> + *
> + * # Set/clear/get local doorbell
> + * root@self# echo 's 1' > $DBG_DIR/db
> + * root@self# echo 'c 1' > $DBG_DIR/db
> + * root@self# cat  $DBG_DIR/db
> + *
> + * # Set/clear/get local doorbell mask
> + * root@self# echo 's 1' > $DBG_DIR/db_mask
> + * root@self# echo 'c 1' > $DBG_DIR/db_mask
> + * root@self# cat $DBG_DIR/db_mask
> + *
> + * # Ring/clear/get peer doorbell
>   * root@peer# echo 's 1' > $DBG_DIR/peer_db
> + * root@peer# echo 'c 1' >