lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20170801181452.GE4186@kudzu.us>
Date:   Tue, 1 Aug 2017 14:14:53 -0400
From:   Jon Mason <jdmason@...zu.us>
To:     Logan Gunthorpe <logang@...tatee.com>
Cc:     linux-ntb@...glegroups.com, linux-pci@...r.kernel.org,
        linux-kernel@...r.kernel.org, Dave Jiang <dave.jiang@...el.com>,
        Allen Hubbe <Allen.Hubbe@....com>,
        Bjorn Helgaas <bhelgaas@...gle.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Kurt Schwemmer <kurt.schwemmer@...rosemi.com>,
        Stephen Bates <sbates@...thlin.com>,
        Serge Semin <fancer.lancer@...il.com>
Subject: Re: [PATCH v3 06/16] ntb: ensure ntb_mw_get_align() is only called
 when the link is up

On Tue, Jul 25, 2017 at 02:57:43PM -0600, Logan Gunthorpe wrote:
> With Switchtec hardware it's impossible to get the alignment parameters
> for a peer's memory window until the peer's driver has configured it's
> windows. Strictly speaking, the link doesn't have to be up for this,
> but the link being up is the only way the client can tell that
> the otherside has been configured.
> 
> This patch converts ntb_transport and ntb_perf to use this function after
> the link goes up. This simplifies these clients slightly because they
> no longer have to store the alignment parameters. It also tweaks
> ntb_tool so that peer_mw_trans will print zero if it is run before
> the link goes up.
> 
> Signed-off-by: Logan Gunthorpe <logang@...tatee.com>

This also looks like a bug fix.  However, given that it is a little more
invasive and no one reporting any problems with it, I'm less apt to
pull it into the bug-fixes branch without anyone compilaining of a
bug (lest I break something in -stable).

Thanks,
Jon

> ---
>  drivers/ntb/ntb_transport.c | 20 ++++++++++----------
>  drivers/ntb/test/ntb_perf.c | 18 +++++++++---------
>  drivers/ntb/test/ntb_tool.c |  6 +++---
>  3 files changed, 22 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
> index b29558ddfe95..7ed745aaa213 100644
> --- a/drivers/ntb/ntb_transport.c
> +++ b/drivers/ntb/ntb_transport.c
> @@ -191,8 +191,6 @@ struct ntb_transport_qp {
>  struct ntb_transport_mw {
>  	phys_addr_t phys_addr;
>  	resource_size_t phys_size;
> -	resource_size_t xlat_align;
> -	resource_size_t xlat_align_size;
>  	void __iomem *vbase;
>  	size_t xlat_size;
>  	size_t buff_size;
> @@ -687,13 +685,20 @@ static int ntb_set_mw(struct ntb_transport_ctx *nt, int num_mw,
>  	struct ntb_transport_mw *mw = &nt->mw_vec[num_mw];
>  	struct pci_dev *pdev = nt->ndev->pdev;
>  	size_t xlat_size, buff_size;
> +	resource_size_t xlat_align;
> +	resource_size_t xlat_align_size;
>  	int rc;
>  
>  	if (!size)
>  		return -EINVAL;
>  
> -	xlat_size = round_up(size, mw->xlat_align_size);
> -	buff_size = round_up(size, mw->xlat_align);
> +	rc = ntb_mw_get_align(nt->ndev, PIDX, num_mw, &xlat_align,
> +			      &xlat_align_size, NULL);
> +	if (rc)
> +		return rc;
> +
> +	xlat_size = round_up(size, xlat_align_size);
> +	buff_size = round_up(size, xlat_align);
>  
>  	/* No need to re-setup */
>  	if (mw->xlat_size == xlat_size)
> @@ -722,7 +727,7 @@ static int ntb_set_mw(struct ntb_transport_ctx *nt, int num_mw,
>  	 * is a requirement of the hardware. It is recommended to setup CMA
>  	 * for BAR sizes equal or greater than 4MB.
>  	 */
> -	if (!IS_ALIGNED(mw->dma_addr, mw->xlat_align)) {
> +	if (!IS_ALIGNED(mw->dma_addr, xlat_align)) {
>  		dev_err(&pdev->dev, "DMA memory %pad is not aligned\n",
>  			&mw->dma_addr);
>  		ntb_free_mw(nt, num_mw);
> @@ -1106,11 +1111,6 @@ static int ntb_transport_probe(struct ntb_client *self, struct ntb_dev *ndev)
>  	for (i = 0; i < mw_count; i++) {
>  		mw = &nt->mw_vec[i];
>  
> -		rc = ntb_mw_get_align(ndev, PIDX, i, &mw->xlat_align,
> -				      &mw->xlat_align_size, NULL);
> -		if (rc)
> -			goto err1;
> -
>  		rc = ntb_peer_mw_get_addr(ndev, i, &mw->phys_addr,
>  					  &mw->phys_size);
>  		if (rc)
> diff --git a/drivers/ntb/test/ntb_perf.c b/drivers/ntb/test/ntb_perf.c
> index 759f772fa00c..427112cf101a 100644
> --- a/drivers/ntb/test/ntb_perf.c
> +++ b/drivers/ntb/test/ntb_perf.c
> @@ -108,8 +108,6 @@ MODULE_PARM_DESC(on_node, "Run threads only on NTB device node (default: true)")
>  struct perf_mw {
>  	phys_addr_t	phys_addr;
>  	resource_size_t	phys_size;
> -	resource_size_t	xlat_align;
> -	resource_size_t	xlat_align_size;
>  	void __iomem	*vbase;
>  	size_t		xlat_size;
>  	size_t		buf_size;
> @@ -472,13 +470,20 @@ static int perf_set_mw(struct perf_ctx *perf, resource_size_t size)
>  {
>  	struct perf_mw *mw = &perf->mw;
>  	size_t xlat_size, buf_size;
> +	resource_size_t	xlat_align;
> +	resource_size_t	xlat_align_size;
>  	int rc;
>  
>  	if (!size)
>  		return -EINVAL;
>  
> -	xlat_size = round_up(size, mw->xlat_align_size);
> -	buf_size = round_up(size, mw->xlat_align);
> +	rc = ntb_mw_get_align(perf->ntb, PIDX, 0, &xlat_align,
> +			      &xlat_align_size, NULL);
> +	if (rc)
> +		return rc;
> +
> +	xlat_size = round_up(size, xlat_align_size);
> +	buf_size = round_up(size, xlat_align);
>  
>  	if (mw->xlat_size == xlat_size)
>  		return 0;
> @@ -567,11 +572,6 @@ static int perf_setup_mw(struct ntb_dev *ntb, struct perf_ctx *perf)
>  
>  	mw = &perf->mw;
>  
> -	rc = ntb_mw_get_align(ntb, PIDX, 0, &mw->xlat_align,
> -			      &mw->xlat_align_size, NULL);
> -	if (rc)
> -		return rc;
> -
>  	rc = ntb_peer_mw_get_addr(ntb, 0, &mw->phys_addr, &mw->phys_size);
>  	if (rc)
>  		return rc;
> diff --git a/drivers/ntb/test/ntb_tool.c b/drivers/ntb/test/ntb_tool.c
> index a69815c45ce6..91526a986caa 100644
> --- a/drivers/ntb/test/ntb_tool.c
> +++ b/drivers/ntb/test/ntb_tool.c
> @@ -753,9 +753,9 @@ static ssize_t tool_peer_mw_trans_read(struct file *filep,
>  
>  	phys_addr_t base;
>  	resource_size_t mw_size;
> -	resource_size_t align_addr;
> -	resource_size_t align_size;
> -	resource_size_t max_size;
> +	resource_size_t align_addr = 0;
> +	resource_size_t align_size = 0;
> +	resource_size_t max_size = 0;
>  
>  	buf_size = min_t(size_t, size, 512);
>  
> -- 
> 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+unsubscribe@...glegroups.com.
> To post to this group, send email to linux-ntb@...glegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/linux-ntb/20170725205753.4735-7-logang%40deltatee.com.
> For more options, visit https://groups.google.com/d/optout.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ