linux-kernel - Re: [PATCH net-next v10 1/6] page_pool: fragment API support for 32-bit arch with 64-bit DMA

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 03 Oct 2023 09:45:56 +0200
From:   Paolo Abeni <pabeni@...hat.com>
To:     Yunsheng Lin <linyunsheng@...wei.com>, davem@...emloft.net,
        kuba@...nel.org
Cc:     netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
        Lorenzo Bianconi <lorenzo@...nel.org>,
        Alexander Duyck <alexander.duyck@...il.com>,
        Liang Chen <liangchen.linux@...il.com>,
        Alexander Lobakin <aleksander.lobakin@...el.com>,
        Guillaume Tucker <guillaume.tucker@...labora.com>,
        Matthew Wilcox <willy@...radead.org>,
        Linux-MM <linux-mm@...ck.org>,
        Jesper Dangaard Brouer <hawk@...nel.org>,
        Ilias Apalodimas <ilias.apalodimas@...aro.org>,
        Eric Dumazet <edumazet@...gle.com>
Subject: Re: [PATCH net-next v10 1/6] page_pool: fragment API support for
 32-bit arch with 64-bit DMA

On Fri, 2023-09-22 at 17:11 +0800, Yunsheng Lin wrote:
> Currently page_pool_alloc_frag() is not supported in 32-bit
> arch with 64-bit DMA because of the overlap issue between
> pp_frag_count and dma_addr_upper in 'struct page' for those
> arches, which seems to be quite common, see [1], which means
> driver may need to handle it when using fragment API.
> 
> It is assumed that the combination of the above arch with an
> address space >16TB does not exist, as all those arches have
> 64b equivalent, it seems logical to use the 64b version for a
> system with a large address space. It is also assumed that dma
> address is page aligned when we are dma mapping a page aligned
> buffer, see [2].
> 
> That means we're storing 12 bits of 0 at the lower end for a
> dma address, we can reuse those bits for the above arches to
> support 32b+12b, which is 16TB of memory.
> 
> If we make a wrong assumption, a warning is emitted so that
> user can report to us.
> 
> 1. https://lore.kernel.org/all/20211117075652.58299-1-linyunsheng@huawei.com/
> 2. https://lore.kernel.org/all/20230818145145.4b357c89@kernel.org/
> 
> Signed-off-by: Jakub Kicinski <kuba@...nel.org>
> Signed-off-by: Yunsheng Lin <linyunsheng@...wei.com>
> CC: Lorenzo Bianconi <lorenzo@...nel.org>
> CC: Alexander Duyck <alexander.duyck@...il.com>
> CC: Liang Chen <liangchen.linux@...il.com>
> CC: Alexander Lobakin <aleksander.lobakin@...el.com>
> CC: Guillaume Tucker <guillaume.tucker@...labora.com>
> CC: Matthew Wilcox <willy@...radead.org>
> CC: Linux-MM <linux-mm@...ck.org>
> ---
>  include/linux/mm_types.h        | 13 +------------
>  include/net/page_pool/helpers.h | 20 ++++++++++++++------
>  net/core/page_pool.c            | 14 +++++++++-----
>  3 files changed, 24 insertions(+), 23 deletions(-)
> 
> diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> index 36c5b43999e6..74b49c4c7a52 100644
> --- a/include/linux/mm_types.h
> +++ b/include/linux/mm_types.h
> @@ -125,18 +125,7 @@ struct page {
>  			struct page_pool *pp;
>  			unsigned long _pp_mapping_pad;
>  			unsigned long dma_addr;
> -			union {
> -				/**
> -				 * dma_addr_upper: might require a 64-bit
> -				 * value on 32-bit architectures.
> -				 */
> -				unsigned long dma_addr_upper;
> -				/**
> -				 * For frag page support, not supported in
> -				 * 32-bit architectures with 64-bit DMA.
> -				 */
> -				atomic_long_t pp_frag_count;
> -			};
> +			atomic_long_t pp_frag_count;
>  		};
>  		struct {	/* Tail pages of compound page */
>  			unsigned long compound_head;	/* Bit zero is set */

As noted by Jesper, since this is touching the super-critcal struct
page, an explicit ack from the mm people is required.

@Matthew: could you please have a look?

I think it would be nice also an explicit ack from Jesper and/or Ilias.

Cheers,

Paolo