From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752610AbdLKRPB (ORCPT ); Mon, 11 Dec 2017 12:15:01 -0500 Received: from ale.deltatee.com ([207.54.116.67]:33886 "EHLO ale.deltatee.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752321AbdLKROu (ORCPT ); Mon, 11 Dec 2017 12:14:50 -0500 To: Allen Hubbe , linux-ntb@googlegroups.com, linux-kernel@vger.kernel.org Cc: "'Jon Mason'" References: <20171209000217.18366-1-logang@deltatee.com> <20171209000217.18366-2-logang@deltatee.com> <000201d37290$81605eb0$84211c10$@dell.com> From: Logan Gunthorpe Message-ID: Date: Mon, 11 Dec 2017 10:14:48 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 MIME-Version: 1.0 In-Reply-To: <000201d37290$81605eb0$84211c10$@dell.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-SA-Exim-Connect-IP: 172.16.1.162 X-SA-Exim-Rcpt-To: jdmason@kudzu.us, linux-kernel@vger.kernel.org, linux-ntb@googlegroups.com, Allen.Hubbe@dell.com X-SA-Exim-Mail-From: logang@deltatee.com Subject: Re: [PATCH 2/2] ntb_hw_switchtec: Check for alignment of the buffer in mw_set_trans() X-SA-Exim-Version: 4.2.1 (built Tue, 02 Aug 2016 21:08:31 +0000) X-SA-Exim-Scanned: Yes (on ale.deltatee.com) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 11/12/17 07:58 AM, Allen Hubbe wrote: > !IS_ALIGNED(addr, BIT_ULL(xlate_pos)) > Ok, yes, that's much better. I'll change it. Thanks. >> + /* >> + * 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(&sndev->stdev->dev, >> + "ERROR: Memory window address is not aligned to it's size!\n"); > > This would be the only ntb hw driver that prints an error in this situation. The ntb_mw_get_align() should provide enough information to client drivers to determine the alignment requirements before calling ntb_mw_set_trans(). IMO no need to print here, but let's see what others say. mw_get_align doesn't communicate the fact that the buffer has to be aligned by its size. It may also be that all hardware does not have this restriction (ie. if the hardware adds to the base address instead of just replacing the lower bits). There is definitely a need to print this error somewhere as I hit this case and it caused very weird behavior. It was a huge pain to debug. Also, it's a security issue and huge bug if we end up mapping the memory we didn't think we were mapping. I don't think it's a good idea for us to require clients to check this as that requires a number of checks and a client author may forget to add it to their driver. I'd maybe go with a check in ntb_mw_set_trans before calling the driver, but that only makes sense if all hardware has the same requirement. Logan