Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Subject: Re: [PATCH v2] Revert "swiotlb: panic if nslabs is too small"
To:     Yu Zhao <yuzhao@google.com>, Christoph Hellwig <hch@infradead.org>,
        Robin Murphy <robin.murphy@arm.com>,
        Marek Szyprowski <m.szyprowski@samsung.com>
Cc:     iommu@lists.linux.dev, linux-mips@vger.kernel.org,
        linux-kernel@vger.kernel.org, kernel test robot <lkp@intel.com>,
        Dan Carpenter <dan.carpenter@oracle.com>
References: <20220829232934.3277747-1-yuzhao@google.com>
 <20220831063818.3902572-1-yuzhao@google.com>
From:   Dongli Zhang <dongli.zhang@oracle.com>
Message-ID: <747f76e1-a5ec-150c-311e-a60396f6f7ab@oracle.com>
Date:   Wed, 31 Aug 2022 15:20:37 -0700
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.6.1
In-Reply-To: <20220831063818.3902572-1-yuzhao@google.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?bVlVd01Wd1N3NjFFOVFoL0J5TC9vaGx4MlVYNi9hMHVJWVQxeFExY3c0ZzA4?=
 =?utf-8?B?MnB2dkxWZmxxaVF1TlRZb3BHRUduQ0NYTUVnL25HdXpBa2xxOXBBNE96b2xB?=
 =?utf-8?B?dnhORG5uSzh1czV3SG1ndkNCeFVWaWV6QjE0S0ZZSVFtdDRqOHQ4a1dUWDE3?=
 =?utf-8?B?b3NZYmtQNWl4SXhuMk1hSHlnY0ZmNWxXdkxXNE1oQThjNnQ0TjJzS29PRVFS?=
 =?utf-8?B?bzhCUnZnTG91dmVNRnBlbi91M2ludUhtV1hPNzNrN0xwZmpMZmRvOXlrSjFN?=
 =?utf-8?B?RjNCdEdYeWZOK1ZFZ1JxbzFJZE9ZdEdacmMrVkwzc3dOSWxySWlwYVUvWktJ?=
 =?utf-8?B?NnRnbG9BRlVENmJuVWlOMTVMNFZValpabzFHTjBCT1JWNHJBdk1ubkYvaUFS?=
 =?utf-8?B?RjFnanJWRTBvNmhUcHpoMWxZd05FWFVZeWN4OWpjT3ZjMEhuZXRJN2s2czFw?=
 =?utf-8?B?NzhuRkc1TTV0RTUrY05vd1ZvL3hFcWZ1aUdPUHZtWmFoTUF2SDhWeUJKM2x0?=
 =?utf-8?B?R3p1Vkh0KzY1MkdVZlVPUTh2WHQvZGNRTno1ZzFpcVoyRnQ5WStKekhqSXNv?=
 =?utf-8?B?Qko2R0hxVTZJbmtPb290ZlplZ3NUS3dFSzVKTE1ZR0NSUGRvTkwvWXVVUC84?=
 =?utf-8?B?VzcrVFpLOWdMekVMS3BWZWRZdVgxMERWdW1Qc1Z2K29HL1Q2dFd0WXp5Y25D?=
 =?utf-8?B?Q1FEbXplMlI2L1lORXBZZFBPcTZBakF0WVVEclZTZjJvMEIzWnMxL2NQWGRN?=
 =?utf-8?B?N2FKejhCMTZCOEFLWmhyQVRLa1NGWWpveVdjZTBRUGFDQis1THBMOWZ5V3Rx?=
 =?utf-8?B?cEVUOGlmNk4rK3NLM1R0S2c5Vnp3emVMVTVyd0kwUjlDN1BoVWVQeWYwVDdG?=
 =?utf-8?B?Y1pVUHprS1lCeHgva2hVVGdmNktxOU93cGlZYVFrNTBTL24ybDltVmJYTTJL?=
 =?utf-8?B?Y2Z4UldsdzVoTGZnUEcyczY5VVlnUXl6VitVQnd1cWFGTmlIS28yblJaZzJi?=
 =?utf-8?B?NEtPY0hmQU5zZTQ0Tzd0VXhvSUVYeXVUc3I3UGRDMzIyTko0S2JlMzdUN0JR?=
 =?utf-8?B?bGdsZzE0a0pMa3l0V0NHS2dXTlBjL1JoNnVKV0UyZmFJaWZ3Y2pKNFNDcnlI?=
 =?utf-8?B?L0NxUVRXN0JCWERXYS9US2dtUDJobXczV2J5bDc2SWdLZ2w2L0VZQkhvTjFK?=
 =?utf-8?B?Y3h2WEhmVVQrWXB0Rks3UnM5M2JLR1M3TWFqSmhic3NUeDN6R2c2T2lNMGRy?=
 =?utf-8?B?N2VGclZmUHMzY0hqS1FUcnRTNUNoTlZqWENzUU1QWlA5UmhSUXJDb0xteFlh?=
 =?utf-8?B?dCtEOW8yWnFlYWNQeUlCTDM5TWlpMGdjcGh4VktleXZCYlNucGtHTGhpMmNw?=
 =?utf-8?B?TDlZSEpTNzNtb1BJd3FMTUthODhYbkpnZnhQMEJjUndtMWxka0NhaHUrcVRm?=
 =?utf-8?B?ZGVzTlZpZ2ZTYTNSQUtucnd3bENHazkzTTQrSTFjUjZMZzlIelhoYWdIbmF1?=
 =?utf-8?B?dEhWTzFlclYrVnVWSWpiamlNQ05DQ2E1elo5NXlWdE1WKzdDSkdYZDlPZzY4?=
 =?utf-8?B?MnJRcFBpc0xRRU0rTlNVWVpMNXZxMktvVkc3WElvUG95TTQ2a1pmbzBIaWtU?=
 =?utf-8?B?TjNSS2E5TTNncEhGZ3hrRlJVNVZxSkZlRWR1ZzllSWJMRG5zSThqVndYTjhG?=
 =?utf-8?B?cXBGa25wbUt0YUdmUEJCajc5QS9xbFZIRlowclk0TnhKamxXQTQ5L1dsZkVH?=
 =?utf-8?B?UVZBSlU4N3EyTnp4MzdkZzR3RzlTeUFUR21xODhlL1ZHM0RheCs3TVUzZEZ0?=
 =?utf-8?B?WlYybm9zVmEzN1ZVSy9xSHdrWFFYV0g3amMzKzV2Nm9mb3p4cGlJQWovSExR?=
 =?utf-8?B?SjNJajF2ZWRkSHEwMFRBeXh1QzBWa3NYVzg2MmM3TzNjZFl5eXV1a2V6b0ZN?=
 =?utf-8?B?c00va3lQK2RzaUhhYlFVakVxQWxDd2VBOWs3MUMwS2ZwbTQxSXF4c2x5L0M5?=
 =?utf-8?B?c2lkVEdScjZqTHU5MXhWMUNSelBTci91Y0RzUUlRbHI4cnovYVV6ZFlOTDd6?=
 =?utf-8?B?Vm1ieFdQK1ExeWdkaWJ6U1ZkSG82anAzSXFTNmNlZlM2dDhHcnNROGdBWG9C?=
 =?utf-8?B?YVk0ZGJZK2MySHZWei9iSjU1RDRqTXhnVjlyVkFOVUdscVA1L1ZXWkIwQm9N?=
 =?utf-8?Q?eWfDBLmOIVgqc4Qz0mEhzxs=3D?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: c1607f37-9dd0-4ad2-39fd-08da8b9f0a47
X-MS-Exchange-CrossTenant-AuthSource: BYAPR10MB2663.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 31 Aug 2022 22:20:41.7503
 (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: LqWwvVs10v6U5qQrepERso0VCkv8gndFBrSfSwDZJv1IqEaWuldGhYUdJ6v+qCNOE5INDrtbaZYJ0honrPZ4aw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH0PR10MB4488
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.205,Aquarius:18.0.895,Hydra:6.0.517,FMLib:17.11.122.1
 definitions=2022-08-31_14,2022-08-31_03,2022-06-22_01
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 malwarescore=0 suspectscore=0
 mlxscore=0 spamscore=0 phishscore=0 mlxlogscore=999 adultscore=0
 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2207270000 definitions=main-2208310109
X-Proofpoint-ORIG-GUID: L_KnwqI66TqP1AXyXeUWtojrTw1yMtcW
X-Proofpoint-GUID: L_KnwqI66TqP1AXyXeUWtojrTw1yMtcW
X-Spam-Status: No, score=-2.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_LOW,
        RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE,T_SCC_BODY_TEXT_LINE
        autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hi Yu,

As we discussed in the past, the swiotlb panic on purpose because the
mips/cavium-octeon/dma-octeon.c requests to allocate only PAGE_SIZE swiotlb
buffer. This is smaller than IO_TLB_MIN_SLABS.

The below comments mentioned that IO_TLB_MIN_SLABS is the "Minimum IO TLB size
to bother booting with".

56 /*
57  * Minimum IO TLB size to bother booting with.  Systems with mainly
58  * 64bit capable cards will only lightly use the swiotlb.  If we can't
59  * allocate a contiguous 1MB, we're probably in trouble anyway.
60  */
61 #define IO_TLB_MIN_SLABS ((1<<20) >> IO_TLB_SHIFT)


The arm may create swiotlb conditionally. That is, the swiotlb is not
initialized if (1) CONFIG_ARM_LPAE is not set (line 273), or (2) max_pfn <=
arm_dma_pfn_limit (line 274).

arch/arm/mm/init.c

271 void __init mem_init(void)
272 {
273 #ifdef CONFIG_ARM_LPAE
274         swiotlb_init(max_pfn > arm_dma_pfn_limit, SWIOTLB_VERBOSE);
275 #endif
276
277         set_max_mapnr(pfn_to_page(max_pfn) - mem_map);


On x86, the swiotlb is not initialized if the memory is small (> MAX_DMA32_PFN,
at line 47), or the secure memory is not required.

44 static void __init pci_swiotlb_detect(void)
45 {
46         /* don't initialize swiotlb if iommu=off (no_iommu=1) */
47         if (!no_iommu && max_possible_pfn > MAX_DMA32_PFN)
48                 x86_swiotlb_enable = true;
49
50         /*
51          * Set swiotlb to 1 so that bounce buffers are allocated and used for
52          * devices that can't support DMA to encrypted memory.
53          */
54         if (cc_platform_has(CC_ATTR_HOST_MEM_ENCRYPT))
55                 x86_swiotlb_enable = true;
56
57         /*
58          * Guest with guest memory encryption currently perform all DMA through
59          * bounce buffers as the hypervisor can't access arbitrary VM memory
60          * that is not explicitly shared with it.
61          */
62         if (cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT)) {
63                 x86_swiotlb_enable = true;
64                 x86_swiotlb_flags |= SWIOTLB_FORCE;
65         }
66 }


Regardless whether the current patch will be reverted, unless there is specific
reason (e.g., those PAGE_SIZE will be used), I do not think it is a good idea to
allocate <IO_TLB_MIN_SLABS swiotlb buffer. I will wait for the suggestion from
the swiotlb maintainer.

Since I do not have a mips environment, I am not able to test if the below makes
any trouble in your situation at arch/mips/cavium-octeon/dma-octeon.c.

@@ -234,6 +234,8 @@ void __init plat_swiotlb_setup(void)
                swiotlbsize = 64 * (1<<20);
 #endif

-       swiotlb_adjust_size(swiotlbsize);
-       swiotlb_init(true, SWIOTLB_VERBOSE);
+       if (swiotlbsize != PAGE_SIZE) {
+               swiotlb_adjust_size(swiotlbsize);
+               swiotlb_init(true, SWIOTLB_VERBOSE);
+       }
 }


Thank you very much!

Dongli Zhang

On 8/30/22 11:38 PM, Yu Zhao wrote:
> This reverts commit 0bf28fc40d89b1a3e00d1b79473bad4e9ca20ad1.
> 
> Reasons:
>   1. new panic()s shouldn't be added [1].
>   2. It does no "cleanup" but breaks MIPS [2].
> 
> v2: properly solved the conflict [3] with
> commit 20347fca71a38 ("swiotlb: split up the global swiotlb lock")
> Reported-by: kernel test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> 
> [1] https://urldefense.com/v3/__https://lore.kernel.org/r/CAHk-=wit-DmhMfQErY29JSPjFgebx_Ld*pnerc4J2Ag990WwAA@mail.gmail.com/__;Kw!!ACWV5N9M2RV99hQ!PPVATbHVDT6TZ4sqoj5G6vfAJGPAEz-Lmp9njTsM2PPYPQqCP6aq5RF8FDmrXDlSzxJmTUUSgOW3yjKDtg$  
> [2] https://urldefense.com/v3/__https://lore.kernel.org/r/20220820012031.1285979-1-yuzhao@google.com/__;!!ACWV5N9M2RV99hQ!PPVATbHVDT6TZ4sqoj5G6vfAJGPAEz-Lmp9njTsM2PPYPQqCP6aq5RF8FDmrXDlSzxJmTUUSgOXQRsYjKQ$  
> [3] https://urldefense.com/v3/__https://lore.kernel.org/r/202208310701.LKr1WDCh-lkp@intel.com/__;!!ACWV5N9M2RV99hQ!PPVATbHVDT6TZ4sqoj5G6vfAJGPAEz-Lmp9njTsM2PPYPQqCP6aq5RF8FDmrXDlSzxJmTUUSgOW_tjcVMA$  
> 
> Fixes: 0bf28fc40d89b ("swiotlb: panic if nslabs is too small")
> Signed-off-by: Yu Zhao <yuzhao@google.com>
> ---
>  kernel/dma/swiotlb.c | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
> index c5a9190b218f..dd8863987e0c 100644
> --- a/kernel/dma/swiotlb.c
> +++ b/kernel/dma/swiotlb.c
> @@ -326,9 +326,6 @@ void __init swiotlb_init_remap(bool addressing_limit, unsigned int flags,
>  		swiotlb_adjust_nareas(num_possible_cpus());
>  
>  	nslabs = default_nslabs;
> -	if (nslabs < IO_TLB_MIN_SLABS)
> -		panic("%s: nslabs = %lu too small\n", __func__, nslabs);
> -
>  	/*
>  	 * By default allocate the bounce buffer memory from low memory, but
>  	 * allow to pick a location everywhere for hypervisors with guest
> @@ -341,8 +338,7 @@ void __init swiotlb_init_remap(bool addressing_limit, unsigned int flags,
>  	else
>  		tlb = memblock_alloc_low(bytes, PAGE_SIZE);
>  	if (!tlb) {
> -		pr_warn("%s: Failed to allocate %zu bytes tlb structure\n",
> -			__func__, bytes);
> +		pr_warn("%s: failed to allocate tlb structure\n", __func__);
>  		return;
>  	}
>  
> 
> base-commit: dcf8e5633e2e69ad60b730ab5905608b756a032f
>