Messages in this thread Patch in this message | | | Subject | Re: [PATCH v2] Revert "swiotlb: panic if nslabs is too small" | From | Dongli Zhang <> | Date | Wed, 31 Aug 2022 15:20:37 -0700 |
| |
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 >
| |