netdev - 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, 3 Oct 2023 12:40:13 +0300
From: Ilias Apalodimas <ilias.apalodimas@...aro.org>
To: Paolo Abeni <pabeni@...hat.com>
Cc: Yunsheng Lin <linyunsheng@...wei.com>, davem@...emloft.net, kuba@...nel.org, 
	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>, 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

Hi Paolo

On Tue, 3 Oct 2023 at 10:46, Paolo Abeni <pabeni@...hat.com> wrote:
>
> 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.

I am trying! Unfortunately, it's this time of the year when I have to
travel a lot.  I'll be back in 15 days from now and I will be able to
have a look

Thanks and sorry for the delay
/Ilias

>
> Cheers,
>
> Paolo
>