lkml.org 
[lkml]   [2022]   [Aug]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH v2] Revert "swiotlb: panic if nslabs is too small"
On Wed, Aug 31, 2022 at 4:20 PM Dongli Zhang <dongli.zhang@oracle.com> wrote:
>
> Hi Yu,
>
> As we discussed in the past, the swiotlb panic on purpose

We should not panic() at all, especially on a platform that has been
working well since at least 4.14.

Did you check out this link I previously shared with you?
https://lore.kernel.org/r/CAHk-=wit-DmhMfQErY29JSPjFgebx_Ld+pnerc4J2Ag990WwAA@mail.gmail.com/

> because the
> mips/cavium-octeon/dma-octeon.c requests to allocate only PAGE_SIZE swiotlb
> buffer.

What's wrong with PAGE_SIZE swiotlb?

> This is smaller than IO_TLB_MIN_SLABS.

IO_TLB_MIN_SLABS is an arbitrary number, and it's inherited from IA64.
So does the comment below. What exactly is the rationale behind it?

> 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 }

Thanks for the code snippets. But I failed to see a point.

> 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.

For what specific reason? My current understanding is that you want to
be future/fool-proof. If so, then you got the priority wrong. We need
to make sure currently working systems continue to work first, then
future/fool-proof.

> I will wait for the suggestion from
> the swiotlb maintainer.

Chris is on vacation. I sure can wait.

But it sounds like you are unsure about what to do. If so, it's not
what you claimed "we have already understood everything related to
swiotlb" previously.

> 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);
> + }
> }

Please ask the MIPS/Octeon maintainers to check this first.

\
 
 \ /
  Last update: 2022-09-01 02:25    [W:0.107 / U:0.044 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site