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:   Wed, 23 Dec 2020 12:58:38 -0800
From:   Ram Pai <linuxram@us.ibm.com>
To:     Thiago Jung Bauermann <bauerman@linux.ibm.com>
Cc:     linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org,
        Michael Ellerman <mpe@ellerman.id.au>,
        Satheesh Rajendran <sathnaga@linux.vnet.ibm.com>
Subject: Re: [PATCH] powerpc/mm: Limit allocation of SWIOTLB on server
 machines
Message-ID: <20201223205838.GA4102@ram-ibm-com.ibm.com>
Reply-To: Ram Pai <linuxram@us.ibm.com>
References: <20201218062103.76102-1-bauerman@linux.ibm.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20201218062103.76102-1-bauerman@linux.ibm.com>
User-Agent: Mutt/1.5.21 (2010-09-15)
Precedence: bulk

On Fri, Dec 18, 2020 at 03:21:03AM -0300, Thiago Jung Bauermann wrote:
> On server-class POWER machines, we don't need the SWIOTLB unless we're a
> secure VM. Nevertheless, if CONFIG_SWIOTLB is enabled we unconditionally
> allocate it.
> 
> In most cases this is harmless, but on a few machine configurations (e.g.,
> POWER9 powernv systems with 4 GB area reserved for crashdump kernel) it can
> happen that memblock can't find a 64 MB chunk of memory for the SWIOTLB and
> fails with a scary-looking WARN_ONCE:
> 
>  ------------[ cut here ]------------
>  memblock: bottom-up allocation failed, memory hotremove may be affected
>  WARNING: CPU: 0 PID: 0 at mm/memblock.c:332 memblock_find_in_range_node+0x328/0x340
>  Modules linked in:
>  CPU: 0 PID: 0 Comm: swapper Not tainted 5.10.0-rc2-orig+ #6
>  NIP:  c000000000442f38 LR: c000000000442f34 CTR: c0000000001e0080
>  REGS: c000000001def900 TRAP: 0700   Not tainted  (5.10.0-rc2-orig+)
>  MSR:  9000000002021033 <SF,HV,VEC,ME,IR,DR,RI,LE>  CR: 28022222  XER: 20040000
>  CFAR: c00000000014b7b4 IRQMASK: 1
>  GPR00: c000000000442f34 c000000001defba0 c000000001deff00 0000000000000047
>  GPR04: 00000000ffff7fff c000000001def828 c000000001def820 0000000000000000
>  GPR08: 0000001ffc3e0000 c000000001b75478 c000000001b75478 0000000000000001
>  GPR12: 0000000000002000 c000000002030000 0000000000000000 0000000000000000
>  GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000002030000
>  GPR20: 0000000000000000 0000000000010000 0000000000010000 c000000001defc10
>  GPR24: c000000001defc08 c000000001c91868 c000000001defc18 c000000001c91890
>  GPR28: 0000000000000000 ffffffffffffffff 0000000004000000 00000000ffffffff
>  NIP [c000000000442f38] memblock_find_in_range_node+0x328/0x340
>  LR [c000000000442f34] memblock_find_in_range_node+0x324/0x340
>  Call Trace:
>  [c000000001defba0] [c000000000442f34] memblock_find_in_range_node+0x324/0x340 (unreliable)
>  [c000000001defc90] [c0000000015ac088] memblock_alloc_range_nid+0xec/0x1b0
>  [c000000001defd40] [c0000000015ac1f8] memblock_alloc_internal+0xac/0x110
>  [c000000001defda0] [c0000000015ac4d0] memblock_alloc_try_nid+0x94/0xcc
>  [c000000001defe30] [c00000000159c3c8] swiotlb_init+0x78/0x104
>  [c000000001defea0] [c00000000158378c] mem_init+0x4c/0x98
>  [c000000001defec0] [c00000000157457c] start_kernel+0x714/0xac8
>  [c000000001deff90] [c00000000000d244] start_here_common+0x1c/0x58
>  Instruction dump:
>  2c230000 4182ffd4 ea610088 ea810090 4bfffe84 39200001 3d42fff4 3c62ff60
>  3863c560 992a8bfc 4bd0881d 60000000 <0fe00000> ea610088 4bfffd94 60000000
>  random: get_random_bytes called from __warn+0x128/0x184 with crng_init=0
>  ---[ end trace 0000000000000000 ]---
>  software IO TLB: Cannot allocate buffer
> 
> Unless this is a secure VM the message can actually be ignored, because the
> SWIOTLB isn't needed. Therefore, let's avoid the SWIOTLB in those cases.

The above warn_on is conveying a genuine warning. Should it be silenced?

> 
> Fixes: eae9eec476d1 ("powerpc/pseries/svm: Allocate SWIOTLB buffer anywhere in memory")
> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
> ---
>  arch/powerpc/mm/mem.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
> index afab328d0887..3af991844145 100644
> --- a/arch/powerpc/mm/mem.c
> +++ b/arch/powerpc/mm/mem.c
> @@ -300,7 +300,8 @@ void __init mem_init(void)
>  	memblock_set_bottom_up(true);
>  	if (is_secure_guest())
>  		svm_swiotlb_init();
> -	else
> +	/* Server machines don't need SWIOTLB if they're not secure guests. */
> +	else if (!machine_is(pseries) && !machine_is(powernv))

I can see powernv never needing SWIOTLB. But, pseries guests, I am not
so sure.

RP