Received: by 2002:a25:1104:0:0:0:0:0 with SMTP id 4csp485494ybr; Fri, 22 May 2020 11:15:03 -0700 (PDT) X-Google-Smtp-Source: ABdhPJxiE/7LeFM/kJ1BAAU1QQABasn/kBtLFI64gYfTOZWMfP1EPBLUpolZQlgAJgOfGdMS8fnx X-Received: by 2002:a50:f402:: with SMTP id r2mr3889144edm.27.1590171302815; Fri, 22 May 2020 11:15:02 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1590171302; cv=none; d=google.com; s=arc-20160816; b=YhzV9d+Z4R6qLaNAGV7CnlDyFIBz/azySYlrIY7xFXeVIS+zAO72EYoWPnfU3SIy95 bBeKf3VjkhgtJbe4prvf2OZBKhxelVTQIdgB3vuvJedaYu6FXN84tkUe9KpgxwNuO+6Z om4xlAjULQXPYv+5oiKVGKWrJeh/daT/rUgB6yYLSkLapeW5B4uFyg3mSVV8pN/13Peq HJN2sY4UGn76uBhMf6rQA4o7Ke6U9b6l0byjWpXQiZSsucZkqTaozTqKCSRYYICCzIq7 UqK2ojKAtDwuaIrIEUgG95qQUzmPnTQvnlgcgTbmWcitsyGSWcjF6ESrsLjwc5H21EMS dcWw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:content-transfer-encoding :content-language:in-reply-to:mime-version:user-agent:date :message-id:from:references:cc:to:subject:dkim-signature; bh=5VJHe7bH8hAm6eUuJu27AVIfqO1GywinPUVd6f11dDs=; b=aQR+Q4vOE86jEW2GANQoZtj3VJP5FkwiPST4leGYap5yIx9e+MdXctQW0r3vpFAl0P fzrDYus6GiwjmUG4Vg9TIGbq1R1aTNmAT/WXwlyeqlDqEG0xZJTitv1VVUhq00QH5y6N v5BEUtv2FQ9B8yhRYRcV69Y55FghBav26zQ+yDzSR6sTsm6cGEA1xVAH36ABScxRpDNN Qeq8tW9XMn/+q5Q2vCx4F9RdB07eKrDVwNnO9sYidKHnClOj/0ow0E68Ih/1RfHpLWxX zTFkHbPA6p0RoK46SoBUqh4NMWhFh7WFn5EYI++CsFaNajyItloXmKTqWIUtDJDv8Uwy yY1A== ARC-Authentication-Results: i=1; mx.google.com; dkim=fail header.i=@xen.org header.s=20200302mail header.b=DWY+wdnf; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id g4si4624566eds.542.2020.05.22.11.14.39; Fri, 22 May 2020 11:15:02 -0700 (PDT) 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; Authentication-Results: mx.google.com; dkim=fail header.i=@xen.org header.s=20200302mail header.b=DWY+wdnf; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1730813AbgEVSLG (ORCPT + 99 others); Fri, 22 May 2020 14:11:06 -0400 Received: from mail.xenproject.org ([104.130.215.37]:58792 "EHLO mail.xenproject.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726373AbgEVSLF (ORCPT ); Fri, 22 May 2020 14:11:05 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=xen.org; s=20200302mail; h=Content-Transfer-Encoding:Content-Type:In-Reply-To: MIME-Version:Date:Message-ID:From:References:Cc:To:Subject:Sender:Reply-To: Content-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender: Resent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=5VJHe7bH8hAm6eUuJu27AVIfqO1GywinPUVd6f11dDs=; b=DWY+wdnfmcr/84yBeVywBch+ZK fTpXAobGMc4xRDwXBwl6sZrqstYLh9PhmX7hH6BpImIMbDqxH0KwBngIa7FzJ54wfi8+uWb/5s0Lc 2zUFol+D+MKCo9/SWg2AWsCLCz2nUacA9ebfwE5fJvSq2hL3jqNbwIifqKzMYPf+uifE=; Received: from xenbits.xenproject.org ([104.239.192.120]) by mail.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1jcC8O-0001Aw-A6; Fri, 22 May 2020 18:11:04 +0000 Received: from 54-240-197-238.amazon.com ([54.240.197.238] helo=a483e7b01a66.ant.amazon.com) by xenbits.xenproject.org with esmtpsa (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.89) (envelope-from ) id 1jcC8O-00043H-2H; Fri, 22 May 2020 18:11:04 +0000 Subject: Re: [PATCH 01/10] swiotlb-xen: use vmalloc_to_page on vmalloc virt addresses To: Stefano Stabellini Cc: jgross@suse.com, boris.ostrovsky@oracle.com, konrad.wilk@oracle.com, xen-devel@lists.xenproject.org, linux-kernel@vger.kernel.org, Stefano Stabellini References: <20200520234520.22563-1-sstabellini@kernel.org> <23e5b6d8-c5d9-b43f-41cd-9d02d8ec0a7f@xen.org> From: Julien Grall Message-ID: Date: Fri, 22 May 2020 19:11:01 +0100 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.14; rv:68.0) Gecko/20100101 Thunderbird/68.8.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-GB Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Stefano, On 22/05/2020 04:54, Stefano Stabellini wrote: > On Thu, 21 May 2020, Julien Grall wrote: >> Hi, >> >> On 21/05/2020 00:45, Stefano Stabellini wrote: >>> From: Boris Ostrovsky >>> >>> Don't just assume that virt_to_page works on all virtual addresses. >>> Instead add a is_vmalloc_addr check and use vmalloc_to_page on vmalloc >>> virt addresses. >> >> Can you provide an example where swiotlb is used with vmalloc()? > > The issue was reported here happening on the Rasperry Pi 4: > https://marc.info/?l=xen-devel&m=158862573216800 Thanks, it would be good if the commit message contains a bit more details. > > If you are asking where in the Linux codebase the vmalloc is happening > specifically, I don't know for sure, my information is limited to the > stack trace that you see in the link (I don't have a Rasperry Pi 4 yet > but I shall have one soon.) Looking at the code there is a comment in xen_swiotlb_alloc_coherent() suggesting that xen_alloc_coherent_pages() may return an ioremap'ped region on Arm. So it feels to me that commit b877ac9815a8fe7e5f6d7fdde3dc34652408840a "xen/swiotlb: remember having called xen_create_contiguous_region()" has always been broken on Arm. As an aside, your commit message also suggests this is an issue for every virtual address used in swiotlb. But only the virt_to_page() call in xen_swiotlb_free_coherent() is modified. Is it intended? If yes, I think you want to update your commit message. > > >>> Signed-off-by: Boris Ostrovsky >>> Signed-off-by: Stefano Stabellini >>> --- >>> drivers/xen/swiotlb-xen.c | 5 ++++- >>> 1 file changed, 4 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c >>> index b6d27762c6f8..a42129cba36e 100644 >>> --- a/drivers/xen/swiotlb-xen.c >>> +++ b/drivers/xen/swiotlb-xen.c >>> @@ -335,6 +335,7 @@ xen_swiotlb_free_coherent(struct device *hwdev, size_t >>> size, void *vaddr, >>> int order = get_order(size); >>> phys_addr_t phys; >>> u64 dma_mask = DMA_BIT_MASK(32); >>> + struct page *pg; >>> if (hwdev && hwdev->coherent_dma_mask) >>> dma_mask = hwdev->coherent_dma_mask; >>> @@ -346,9 +347,11 @@ xen_swiotlb_free_coherent(struct device *hwdev, size_t >>> size, void *vaddr, >>> /* Convert the size to actually allocated. */ >>> size = 1UL << (order + XEN_PAGE_SHIFT); >>> + pg = is_vmalloc_addr(vaddr) ? vmalloc_to_page(vaddr) : >>> + virt_to_page(vaddr); >> >> Common DMA code seems to protect this check with CONFIG_DMA_REMAP. Is it >> something we want to do it here as well? Or is there any other condition where >> vmalloc can happen? > > I can see it in dma_direct_free_pages: > > if (IS_ENABLED(CONFIG_DMA_REMAP) && is_vmalloc_addr(cpu_addr)) > vunmap(cpu_addr); > > I wonder why the common DMA code does that. is_vmalloc_addr should work > regardless of CONFIG_DMA_REMAP. Maybe just for efficiency? is_vmalloc_addr() doesn't looks very expensive (although it is not an inline function). So I am not sure the reason behind it. It might be worth asking the author of the config. Cheers, -- Julien Grall