Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Thu, 28 Jan 2021 12:15:28 -0500
From:   Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
To:     Jianxiong Gao <jxgao@google.com>
Cc:     erdemaktas@google.com, marcorr@google.com, hch@lst.de,
        m.szyprowski@samsung.com, robin.murphy@arm.com,
        gregkh@linuxfoundation.org, saravanak@google.com,
        heikki.krogerus@linux.intel.com, rafael.j.wysocki@intel.com,
        andriy.shevchenko@linux.intel.com, dan.j.williams@intel.com,
        bgolaszewski@baylibre.com, jroedel@suse.de,
        iommu@lists.linux-foundation.org, kbusch@kernel.org, axboe@fb.com,
        sagi@grimberg.me, linux-nvme@lists.infradead.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/3] Add swiotlb offset preserving mapping when
 dma_dma_parameters->page_offset_mask is non zero.
Message-ID: <YBLxMP3sr71BTL+d@Konrads-MacBook-Pro.local>
References: <20210128003829.1892018-1-jxgao@google.com>
 <20210128003829.1892018-3-jxgao@google.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20210128003829.1892018-3-jxgao@google.com>
MIME-Version: 1.0
X-MS-Exchange-MessageSentRepresentingType: 1
Received: from Konrads-MacBook-Pro.local (138.3.200.34) by BY5PR04CA0024.namprd04.prod.outlook.com (2603:10b6:a03:1d0::34) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3805.17 via Frontend Transport; Thu, 28 Jan 2021 17:15:30 +0000
X-MS-PublicTrafficType: Email
X-MS-Office365-Filtering-Correlation-Id: fbffaaeb-c311-4b15-f179-08d8c3b052a0
X-MS-TrafficTypeDiagnostic: BYAPR10MB3736:
X-Microsoft-Antispam-PRVS: <BYAPR10MB3736917BA768CDD48FD13ED089BA9@BYAPR10MB3736.namprd10.prod.outlook.com>
X-MS-Oob-TLC-OOBClassifiers: OLM:7691;
X-MS-Exchange-SenderADCheck: 1
X-Microsoft-Antispam: BCL:0;
X-Microsoft-Antispam-Message-Info: S95JaRRWfuY6hOnzaV4l84Adqh11MnbnkFKdg4imLgk1Zd57X+ou3v4iVMLkKSn0ZFVBFLfbXh3mN8EAPrnWTiA1pX/I/KnYa7RCIjDUaTiUTmbtgpAKVAMdm4cJiBU5C9SAQHDyhqiEHyK01cmpNxHhGu0HOVn1KJuMt5UAL6EQGsTnzNxd88t83kE5xeEiCEd0eIP6QcKJm4jsMrtNXwTOnlJ80D4TREfW57OAj9iHe8icWzg6S9WRNTIj17p6TSkounOczPyNbO5yfGIMyIIVY1R4PJoTFbkI1ScB0NL+vtXYFNTm7/h9O+ECtSj7XAc3w0v8E5444Ra44s7YV1PYvaiMyHYZ9cTJUvpVpdbj2YKpv6FIUIR1HCAIzcENED/47IbL4vWakKtUVFPA6Z/g+6jzHgYoIMrsel94o8vJC0W9X12MJuq8CbVdfhzwv1yAXNhTg1V/LTNPulsUaxzt2l9doUIDchgBQIu+BYr4Tr81QUxBOTzLdZN/sKbYthvDAxQAHK8cm51caAd6Nf3Sd7ONoQH4/y9puRtcv906K6K4ssExrgumJ/MF5fWJ
X-Forefront-Antispam-Report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:BYAPR10MB2999.namprd10.prod.outlook.com;PTR:;CAT:NONE;SFS:(136003)(39860400002)(376002)(396003)(366004)(346002)(8676002)(5660300002)(4326008)(2906002)(86362001)(186003)(8936002)(66556008)(26005)(956004)(6916009)(316002)(6506007)(52116002)(55016002)(9686003)(83380400001)(66476007)(66946007)(478600001)(7696005)(7416002)(16526019)(67856001);DIR:OUT;SFP:1101;
X-MS-Exchange-AntiSpam-MessageData: =?us-ascii?Q?0To3o0A2tjlIMIPcWXAxdp4cbcsvJaq7SjXDVyFCZUssVe5pjB1G0KuWpiaG?=
 =?us-ascii?Q?pQb7+5pPrg6SVYh7w+9r8gt0FaKKYJNq8iMs0JdVdZHuOHsIzQ1ZidEWv9+C?=
 =?us-ascii?Q?wNVLFhOBFKheVRi/FxKLPFB3k5/VbMJhxRPX+HgBAUO+7yQDKLQICKLIAK/H?=
 =?us-ascii?Q?/6TGHihfDzEajebrjLZsxEVBD7fMNPe8X77ORf+PakbVWt0kDCjnJIH2S3t8?=
 =?us-ascii?Q?n6vmIM0VZGqAoACJj5uv3DRmXtoINu8Ct1cKNoxfFlVte+nxLR/jZ5gBYA4K?=
 =?us-ascii?Q?2a6LUFBFOnzF801FnUAb9GdAx61jisr0iL/XM+n+QnBvUW1mdThIpTP5Asx1?=
 =?us-ascii?Q?eWsHHB604she6yQk8P6ELxQg/qEGh8v6zrRNnIln3qQ+m9US/jvYhw9i0Sfg?=
 =?us-ascii?Q?QHQJJ8eNw4gvL9o/wHjQ/exaMJPRa4MQR4RTb2PJ/cqbiOhVb0xgpI0bG0qv?=
 =?us-ascii?Q?yTOAwH0A8cIPv5ZSrqj9ZtHjTfrEQKY21kBg28rMZSF12EfFEib8TcgUkFfZ?=
 =?us-ascii?Q?3WB+zrMUjbbMFhDICRWq6WI5ValZaWrQqn6tpTv2z1IUDEJAkowD17Ae7MXK?=
 =?us-ascii?Q?21kmJZCsoQGVldRx538OUQxts4DZ4JcDgKaWidJ0LDV5Hk8a7mMKHfR3nTV2?=
 =?us-ascii?Q?tUpX0p+JVq1TfjIKSN/yCpXy4F31Gugwdr5cnKTwEXrfpL1lm28IyJ0PBozL?=
 =?us-ascii?Q?a3V4i94RPrLf1te91uVBMidSv9iN/XZjmK3MgKFNWeT+XJtyUtjFLb9uDETB?=
 =?us-ascii?Q?pDF1B4tJDFazM4s4vZOntVGNsZeBkAwHaypLtW0q+RkMbae/gyYGBSv5N5CR?=
 =?us-ascii?Q?6KZhUe1fnbmJ88KIEcA2btmFLEqm73Rl6jwSxSDYhEIZTEP97YPE5oDjgVGD?=
 =?us-ascii?Q?o/cZ8VF1egzYKnXJtUDu1TIRk6NMDw333E4ceDH9/sy9aklkuPpjOuL0Vmtk?=
 =?us-ascii?Q?j3Hs/pNq7KIEj5idApyzQolU6TlQch3MsCY9Gu2bmIlfNMZeeMryRzhZlu0B?=
 =?us-ascii?Q?akTdhxBT/p4jWLFjvfqrOFXc73PKc2BR1lQPv8D27uIokc+VDohi/XEIomal?=
 =?us-ascii?Q?RocKHYgn?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: fbffaaeb-c311-4b15-f179-08d8c3b052a0
X-MS-Exchange-CrossTenant-AuthSource: BYAPR10MB2999.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 28 Jan 2021 17:15:34.4043
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: Uij+hqOgHlFCfQBDcqSivl2sODR+A+lW7L7fsCcsnov9OYVvN052bkp0FNscjU0oasH+OIeieiEsk8zWY3e7OQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BYAPR10MB3736
X-Proofpoint-Virus-Version: vendor=nai engine=6200 definitions=9878 signatures=668683
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 mlxlogscore=999 mlxscore=0 spamscore=0
 adultscore=0 bulkscore=0 phishscore=0 suspectscore=0 malwarescore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2009150000
 definitions=main-2101280083
X-Proofpoint-Virus-Version: vendor=nai engine=6200 definitions=9878 signatures=668683
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 adultscore=0 impostorscore=0
 phishscore=0 bulkscore=0 priorityscore=1501 mlxlogscore=999
 lowpriorityscore=0 spamscore=0 mlxscore=0 suspectscore=0 malwarescore=0
 clxscore=1011 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2009150000 definitions=main-2101280083
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Wed, Jan 27, 2021 at 04:38:28PM -0800, Jianxiong Gao wrote:
> For devices that need to preserve address offset on mapping through
> swiotlb, this patch adds offset preserving based on page_offset_mask
> and keeps the offset if the mask is non zero. This is needed for
> device drivers like NVMe.

<scratches his head>

Didn't you send this patch like a month ago and someone pointed
out that the right fix would be in the NVMe driver?

Is there an issue with fixing the NVMe driver?

> 
> Signed-off-by: Jianxiong Gao <jxgao@google.com>
> ---
>  kernel/dma/swiotlb.c | 25 ++++++++++++++++++++++---
>  1 file changed, 22 insertions(+), 3 deletions(-)
> 
> diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
> index 7c42df6e6100..4cab35f2c9bc 100644
> --- a/kernel/dma/swiotlb.c
> +++ b/kernel/dma/swiotlb.c
> @@ -468,7 +468,7 @@ phys_addr_t swiotlb_tbl_map_single(struct device *hwdev, phys_addr_t orig_addr,
>  	dma_addr_t tbl_dma_addr = phys_to_dma_unencrypted(hwdev, io_tlb_start);
>  	unsigned long flags;
>  	phys_addr_t tlb_addr;
> -	unsigned int nslots, stride, index, wrap;
> +	unsigned int nslots, stride, index, wrap, page_offset_mask, page_offset;
>  	int i;
>  	unsigned long mask;
>  	unsigned long offset_slots;
> @@ -500,12 +500,16 @@ phys_addr_t swiotlb_tbl_map_single(struct device *hwdev, phys_addr_t orig_addr,
>  		    ? ALIGN(mask + 1, 1 << IO_TLB_SHIFT) >> IO_TLB_SHIFT
>  		    : 1UL << (BITS_PER_LONG - IO_TLB_SHIFT);
>  
> +	page_offset_mask = dma_get_page_offset_mask(hwdev);
> +	page_offset = orig_addr & page_offset_mask;
> +	alloc_size += page_offset;
> +
>  	/*
>  	 * For mappings greater than or equal to a page, we limit the stride
>  	 * (and hence alignment) to a page size.
>  	 */
>  	nslots = ALIGN(alloc_size, 1 << IO_TLB_SHIFT) >> IO_TLB_SHIFT;
> -	if (alloc_size >= PAGE_SIZE)
> +	if ((alloc_size >= PAGE_SIZE) || (page_offset_mask > (1 << IO_TLB_SHIFT)))
>  		stride = (1 << (PAGE_SHIFT - IO_TLB_SHIFT));
>  	else
>  		stride = 1;
> @@ -583,6 +587,11 @@ phys_addr_t swiotlb_tbl_map_single(struct device *hwdev, phys_addr_t orig_addr,
>  	 */
>  	for (i = 0; i < nslots; i++)
>  		io_tlb_orig_addr[index+i] = orig_addr + (i << IO_TLB_SHIFT);
> +	/*
> +	 * When keeping the offset of the original data, we need to advance
> +	 * the tlb_addr by the offset of orig_addr.
> +	 */
> +	tlb_addr += page_offset;
>  	if (!(attrs & DMA_ATTR_SKIP_CPU_SYNC) &&
>  	    (dir == DMA_TO_DEVICE || dir == DMA_BIDIRECTIONAL))
>  		swiotlb_bounce(orig_addr, tlb_addr, mapping_size, DMA_TO_DEVICE);
> @@ -598,7 +607,9 @@ void swiotlb_tbl_unmap_single(struct device *hwdev, phys_addr_t tlb_addr,
>  			      enum dma_data_direction dir, unsigned long attrs)
>  {
>  	unsigned long flags;
> -	int i, count, nslots = ALIGN(alloc_size, 1 << IO_TLB_SHIFT) >> IO_TLB_SHIFT;
> +	unsigned int num_page_offset_slabs, page_offset_mask = dma_get_page_offset_mask(hwdev);
> +	int i, count;
> +	int nslots = ALIGN(alloc_size + tlb_addr & page_offset_mask, 1 << IO_TLB_SHIFT) >> IO_TLB_SHIFT;
>  	int index = (tlb_addr - io_tlb_start) >> IO_TLB_SHIFT;
>  	phys_addr_t orig_addr = io_tlb_orig_addr[index];
>  
> @@ -610,6 +621,14 @@ void swiotlb_tbl_unmap_single(struct device *hwdev, phys_addr_t tlb_addr,
>  	    ((dir == DMA_FROM_DEVICE) || (dir == DMA_BIDIRECTIONAL)))
>  		swiotlb_bounce(orig_addr, tlb_addr, mapping_size, DMA_FROM_DEVICE);
>  
> +	/*
> +	 * When dma_get_page_offset_mask is used, we may have padded more slabs
> +	 * when padding exceeds one slab. We need to move index back to the
> +	 * beginning of the padding.
> +	 */
> +	num_page_offset_slabs =  (tlb_addr & page_offset_mask) / (1 << IO_TLB_SHIFT);
> +	index -= num_page_offset_slabs;
> +
>  	/*
>  	 * Return the buffer to the free list by setting the corresponding
>  	 * entries to indicate the number of contiguous entries available.
> -- 
> 2.27.0
>