| 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
| ||
|
Message-ID: <6BF4C165-2AA2-49CC-B452-756CD0830129@gmail.com>
Date: Thu, 14 Nov 2019 10:48:38 -0800
From: "Jonathan Lemon" <jonathan.lemon@...il.com>
To: "Lorenzo Bianconi" <lorenzo@...nel.org>
Cc: netdev@...r.kernel.org, lorenzo.bianconi@...hat.com,
davem@...emloft.net, thomas.petazzoni@...tlin.com,
brouer@...hat.com, ilias.apalodimas@...aro.org,
matteo.croce@...hat.com
Subject: Re: [PATCH net-next 2/3] net: page_pool: add the possibility to sync
DMA memory for non-coherent devices
On 10 Nov 2019, at 4:09, Lorenzo Bianconi wrote:
> Introduce the following parameters in order to add the possibility to
> sync
> DMA memory area before putting allocated buffers in the page_pool
> caches:
> - sync: set to 1 if device is non cache-coherent and needs to flush
> DMA
> area
> - offset: DMA address offset where the DMA engine starts copying rx
> data
> - max_len: maximum DMA memory size page_pool is allowed to flush. This
> is currently used in __page_pool_alloc_pages_slow routine when pages
> are allocated from page allocator
> These parameters are supposed to be set by device drivers
>
> Tested-by: Matteo Croce <mcroce@...hat.com>
> Signed-off-by: Lorenzo Bianconi <lorenzo@...nel.org>
> ---
> include/net/page_pool.h | 11 +++++++----
> net/core/page_pool.c | 39 +++++++++++++++++++++++++++++++++------
> 2 files changed, 40 insertions(+), 10 deletions(-)
>
> diff --git a/include/net/page_pool.h b/include/net/page_pool.h
> index 2cbcdbdec254..defbfd90ab46 100644
> --- a/include/net/page_pool.h
> +++ b/include/net/page_pool.h
> @@ -65,6 +65,9 @@ struct page_pool_params {
> int nid; /* Numa node id to allocate from pages from */
> struct device *dev; /* device, for DMA pre-mapping purposes */
> enum dma_data_direction dma_dir; /* DMA mapping direction */
> + unsigned int max_len; /* max DMA sync memory size */
> + unsigned int offset; /* DMA addr offset */
> + u8 sync;
> };
How about using PP_FLAG_DMA_SYNC instead of another flag word?
(then it can also be gated on having DMA_MAP enabled)
--
Jonathan
>
> struct page_pool {
> @@ -150,8 +153,8 @@ static inline void page_pool_destroy(struct
> page_pool *pool)
> }
>
> /* Never call this directly, use helpers below */
> -void __page_pool_put_page(struct page_pool *pool,
> - struct page *page, bool allow_direct);
> +void __page_pool_put_page(struct page_pool *pool, struct page *page,
> + unsigned int dma_sync_size, bool allow_direct);
>
> static inline void page_pool_put_page(struct page_pool *pool,
> struct page *page, bool allow_direct)
> @@ -160,14 +163,14 @@ static inline void page_pool_put_page(struct
> page_pool *pool,
> * allow registering MEM_TYPE_PAGE_POOL, but shield linker.
> */
> #ifdef CONFIG_PAGE_POOL
> - __page_pool_put_page(pool, page, allow_direct);
> + __page_pool_put_page(pool, page, 0, allow_direct);
> #endif
> }
> /* Very limited use-cases allow recycle direct */
> static inline void page_pool_recycle_direct(struct page_pool *pool,
> struct page *page)
> {
> - __page_pool_put_page(pool, page, true);
> + __page_pool_put_page(pool, page, 0, true);
> }
>
> /* API user MUST have disconnected alloc-side (not allowed to call
> diff --git a/net/core/page_pool.c b/net/core/page_pool.c
> index 5bc65587f1c4..af9514c2d15b 100644
> --- a/net/core/page_pool.c
> +++ b/net/core/page_pool.c
> @@ -112,6 +112,17 @@ static struct page *__page_pool_get_cached(struct
> page_pool *pool)
> return page;
> }
>
> +/* Used for non-coherent devices */
> +static void page_pool_dma_sync_for_device(struct page_pool *pool,
> + struct page *page,
> + unsigned int dma_sync_size)
> +{
> + dma_sync_size = min(dma_sync_size, pool->p.max_len);
> + dma_sync_single_range_for_device(pool->p.dev, page->dma_addr,
> + pool->p.offset, dma_sync_size,
> + pool->p.dma_dir);
> +}
> +
> /* slow path */
> noinline
> static struct page *__page_pool_alloc_pages_slow(struct page_pool
> *pool,
> @@ -156,6 +167,10 @@ static struct page
> *__page_pool_alloc_pages_slow(struct page_pool *pool,
> }
> page->dma_addr = dma;
>
> + /* non-coherent devices - flush memory */
> + if (pool->p.sync)
> + page_pool_dma_sync_for_device(pool, page, pool->p.max_len);
> +
> skip_dma_map:
> /* Track how many pages are held 'in-flight' */
> pool->pages_state_hold_cnt++;
> @@ -255,7 +270,8 @@ static void __page_pool_return_page(struct
> page_pool *pool, struct page *page)
> }
>
> static bool __page_pool_recycle_into_ring(struct page_pool *pool,
> - struct page *page)
> + struct page *page,
> + unsigned int dma_sync_size)
> {
> int ret;
> /* BH protection not needed if current is serving softirq */
> @@ -264,6 +280,10 @@ static bool __page_pool_recycle_into_ring(struct
> page_pool *pool,
> else
> ret = ptr_ring_produce_bh(&pool->ring, page);
>
> + /* non-coherent devices - flush memory */
> + if (ret == 0 && pool->p.sync)
> + page_pool_dma_sync_for_device(pool, page, dma_sync_size);
> +
> return (ret == 0) ? true : false;
> }
>
> @@ -273,18 +293,23 @@ static bool __page_pool_recycle_into_ring(struct
> page_pool *pool,
> * Caller must provide appropriate safe context.
> */
> static bool __page_pool_recycle_direct(struct page *page,
> - struct page_pool *pool)
> + struct page_pool *pool,
> + unsigned int dma_sync_size)
> {
> if (unlikely(pool->alloc.count == PP_ALLOC_CACHE_SIZE))
> return false;
>
> /* Caller MUST have verified/know (page_ref_count(page) == 1) */
> pool->alloc.cache[pool->alloc.count++] = page;
> +
> + /* non-coherent devices - flush memory */
> + if (pool->p.sync)
> + page_pool_dma_sync_for_device(pool, page, dma_sync_size);
> return true;
> }
>
> -void __page_pool_put_page(struct page_pool *pool,
> - struct page *page, bool allow_direct)
> +void __page_pool_put_page(struct page_pool *pool, struct page *page,
> + unsigned int dma_sync_size, bool allow_direct)
> {
> /* This allocator is optimized for the XDP mode that uses
> * one-frame-per-page, but have fallbacks that act like the
> @@ -296,10 +321,12 @@ void __page_pool_put_page(struct page_pool
> *pool,
> /* Read barrier done in page_ref_count / READ_ONCE */
>
> if (allow_direct && in_serving_softirq())
> - if (__page_pool_recycle_direct(page, pool))
> + if (__page_pool_recycle_direct(page, pool,
> + dma_sync_size))
> return;
>
> - if (!__page_pool_recycle_into_ring(pool, page)) {
> + if (!__page_pool_recycle_into_ring(pool, page,
> + dma_sync_size)) {
> /* Cache full, fallback to free pages */
> __page_pool_return_page(pool, page);
> }
> --
> 2.21.0
Powered by blists - more mailing lists