lkml.org 
[lkml]   [2018]   [May]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 08/11] coresight: Add generic TMC sg table framework
On Fri, May 18, 2018 at 05:39:24PM +0100, Suzuki K Poulose wrote:
> This patch introduces a generic sg table data structure and
> associated operations. An SG table can be used to map a set
> of Data pages where the trace data could be stored by the TMC
> ETR. The information about the data pages could be stored in
> different formats, depending on the type of the underlying
> SG mechanism (e.g, TMC ETR SG vs Coresight CATU). The generic
> structure provides book keeping of the pages used for the data
> as well as the table contents. The table should be filled by
> the user of the infrastructure.
>
> A table can be created by specifying the number of data pages
> as well as the number of table pages required to hold the
> pointers, where the latter could be different for different
> types of tables. The pages are mapped in the appropriate dma
> data direction mode (i.e, DMA_TO_DEVICE for table pages
> and DMA_FROM_DEVICE for data pages). The framework can optionally
> accept a set of allocated data pages (e.g, perf ring buffer) and
> map them accordingly. The table and data pages are vmap'ed to allow
> easier access by the drivers. The framework also provides helpers to
> sync the data written to the pages with appropriate directions.
>
> This will be later used by the TMC ETR SG unit and CATU.
>
> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> ---
> Changes since v1:
> - Address code style issues, more comments
> ---
> drivers/hwtracing/coresight/coresight-tmc-etr.c | 290 ++++++++++++++++++++++++
> drivers/hwtracing/coresight/coresight-tmc.h | 50 ++++
> 2 files changed, 340 insertions(+)
>
> diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c
> index 9780798..1e844f8 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
> +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
> @@ -17,9 +17,299 @@
>
> #include <linux/coresight.h>
> #include <linux/dma-mapping.h>
> +#include <linux/slab.h>
> #include "coresight-priv.h"
> #include "coresight-tmc.h"
>
> +/*
> + * tmc_pages_get_offset: Go through all the pages in the tmc_pages
> + * and map the device address @addr to an offset within the virtual
> + * contiguous buffer.
> + */
> +static long
> +tmc_pages_get_offset(struct tmc_pages *tmc_pages, dma_addr_t addr)
> +{
> + int i;
> + dma_addr_t page_start;
> +
> + for (i = 0; i < tmc_pages->nr_pages; i++) {
> + page_start = tmc_pages->daddrs[i];
> + if (addr >= page_start && addr < (page_start + PAGE_SIZE))
> + return i * PAGE_SIZE + (addr - page_start);
> + }
> +
> + return -EINVAL;
> +}
> +
> +/*
> + * tmc_pages_free : Unmap and free the pages used by tmc_pages.
> + * If the pages were not allocated in tmc_pages_alloc(), we would
> + * simply drop the refcount.
> + */
> +static void tmc_pages_free(struct tmc_pages *tmc_pages,
> + struct device *dev, enum dma_data_direction dir)
> +{
> + int i;
> +
> + for (i = 0; i < tmc_pages->nr_pages; i++) {
> + if (tmc_pages->daddrs && tmc_pages->daddrs[i])
> + dma_unmap_page(dev, tmc_pages->daddrs[i],
> + PAGE_SIZE, dir);
> + if (tmc_pages->pages && tmc_pages->pages[i])
> + __free_page(tmc_pages->pages[i]);
> + }
> +
> + kfree(tmc_pages->pages);
> + kfree(tmc_pages->daddrs);
> + tmc_pages->pages = NULL;
> + tmc_pages->daddrs = NULL;
> + tmc_pages->nr_pages = 0;
> +}
> +
> +/*
> + * tmc_pages_alloc : Allocate and map pages for a given @tmc_pages.
> + * If @pages is not NULL, the list of page virtual addresses are
> + * used as the data pages. The pages are then dma_map'ed for @dev
> + * with dma_direction @dir.
> + *
> + * Returns 0 upon success, else the error number.
> + */
> +static int tmc_pages_alloc(struct tmc_pages *tmc_pages,
> + struct device *dev, int node,
> + enum dma_data_direction dir, void **pages)
> +{
> + int i, nr_pages;
> + dma_addr_t paddr;
> + struct page *page;
> +
> + nr_pages = tmc_pages->nr_pages;
> + tmc_pages->daddrs = kcalloc(nr_pages, sizeof(*tmc_pages->daddrs),
> + GFP_KERNEL);
> + if (!tmc_pages->daddrs)
> + return -ENOMEM;
> + tmc_pages->pages = kcalloc(nr_pages, sizeof(*tmc_pages->pages),
> + GFP_KERNEL);
> + if (!tmc_pages->pages) {
> + kfree(tmc_pages->daddrs);
> + tmc_pages->daddrs = NULL;
> + return -ENOMEM;
> + }
> +
> + for (i = 0; i < nr_pages; i++) {
> + if (pages && pages[i]) {
> + page = virt_to_page(pages[i]);
> + /* Hold a refcount on the page */
> + get_page(page);
> + } else {
> + page = alloc_pages_node(node,
> + GFP_KERNEL | __GFP_ZERO, 0);
> + }
> + paddr = dma_map_page(dev, page, 0, PAGE_SIZE, dir);
> + if (dma_mapping_error(dev, paddr))
> + goto err;
> + tmc_pages->daddrs[i] = paddr;
> + tmc_pages->pages[i] = page;
> + }
> + return 0;
> +err:
> + tmc_pages_free(tmc_pages, dev, dir);
> + return -ENOMEM;
> +}
> +
> +static inline dma_addr_t tmc_sg_table_base_paddr(struct tmc_sg_table *sg_table)
> +{
> + if (WARN_ON(!sg_table->data_pages.pages[0]))
> + return 0;
> + return sg_table->table_daddr;
> +}
> +
> +static inline void *tmc_sg_table_base_vaddr(struct tmc_sg_table *sg_table)
> +{
> + if (WARN_ON(!sg_table->data_pages.pages[0]))
> + return NULL;
> + return sg_table->table_vaddr;
> +}

The above two functions deal with DMA'able and virtual addresses for the table
page buffer. Yet the test in the WARN_ON is done on the data page array.
Shouldn't this be sg_table->table_pages.pages[0] instead?

If not please add a comment justifying your position so that someone else
looking at the code does't end up thinking the same in a year from now.

> +
> +static inline void *
> +tmc_sg_table_data_vaddr(struct tmc_sg_table *sg_table)
> +{
> + if (WARN_ON(!sg_table->data_pages.nr_pages))
> + return 0;
> + return sg_table->data_vaddr;
> +}

I see that tmc_sg_table_base_vaddr() and tmc_sg_table_data_vaddr() are both
returning the virtual address of the contiguous buffer for table and data
respectively. Yet there is a discrepency in the naming convention. I would
have expected tmc_sg_table_base_vaddr() and tmc_sg_data_base_vaddr() so that
there is a little symmetry between them.


Otherwise looks good to me.

> +
> +static inline long
> +tmc_sg_get_data_page_offset(struct tmc_sg_table *sg_table, dma_addr_t addr)
> +{
> + return tmc_pages_get_offset(&sg_table->data_pages, addr);
> +}
> +
> +static inline void tmc_free_table_pages(struct tmc_sg_table *sg_table)
> +{
> + if (sg_table->table_vaddr)
> + vunmap(sg_table->table_vaddr);
> + tmc_pages_free(&sg_table->table_pages, sg_table->dev, DMA_TO_DEVICE);
> +}
> +
> +static void tmc_free_data_pages(struct tmc_sg_table *sg_table)
> +{
> + if (sg_table->data_vaddr)
> + vunmap(sg_table->data_vaddr);
> + tmc_pages_free(&sg_table->data_pages, sg_table->dev, DMA_FROM_DEVICE);
> +}
> +
> +void tmc_free_sg_table(struct tmc_sg_table *sg_table)
> +{
> + tmc_free_table_pages(sg_table);
> + tmc_free_data_pages(sg_table);
> +}
> +
> +/*
> + * Alloc pages for the table. Since this will be used by the device,
> + * allocate the pages closer to the device (i.e, dev_to_node(dev)
> + * rather than the CPU node).
> + */
> +static int tmc_alloc_table_pages(struct tmc_sg_table *sg_table)
> +{
> + int rc;
> + struct tmc_pages *table_pages = &sg_table->table_pages;
> +
> + rc = tmc_pages_alloc(table_pages, sg_table->dev,
> + dev_to_node(sg_table->dev),
> + DMA_TO_DEVICE, NULL);
> + if (rc)
> + return rc;
> + sg_table->table_vaddr = vmap(table_pages->pages,
> + table_pages->nr_pages,
> + VM_MAP,
> + PAGE_KERNEL);
> + if (!sg_table->table_vaddr)
> + rc = -ENOMEM;
> + else
> + sg_table->table_daddr = table_pages->daddrs[0];
> + return rc;
> +}
> +
> +static int tmc_alloc_data_pages(struct tmc_sg_table *sg_table, void **pages)
> +{
> + int rc;
> +
> + /* Allocate data pages on the node requested by the caller */
> + rc = tmc_pages_alloc(&sg_table->data_pages,
> + sg_table->dev, sg_table->node,
> + DMA_FROM_DEVICE, pages);
> + if (!rc) {
> + sg_table->data_vaddr = vmap(sg_table->data_pages.pages,
> + sg_table->data_pages.nr_pages,
> + VM_MAP,
> + PAGE_KERNEL);
> + if (!sg_table->data_vaddr)
> + rc = -ENOMEM;
> + }
> + return rc;
> +}
> +
> +/*
> + * tmc_alloc_sg_table: Allocate and setup dma pages for the TMC SG table
> + * and data buffers. TMC writes to the data buffers and reads from the SG
> + * Table pages.
> + *
> + * @dev - Device to which page should be DMA mapped.
> + * @node - Numa node for mem allocations
> + * @nr_tpages - Number of pages for the table entries.
> + * @nr_dpages - Number of pages for Data buffer.
> + * @pages - Optional list of virtual address of pages.
> + */
> +struct tmc_sg_table *tmc_alloc_sg_table(struct device *dev,
> + int node,
> + int nr_tpages,
> + int nr_dpages,
> + void **pages)
> +{
> + long rc;
> + struct tmc_sg_table *sg_table;
> +
> + sg_table = kzalloc(sizeof(*sg_table), GFP_KERNEL);
> + if (!sg_table)
> + return ERR_PTR(-ENOMEM);
> + sg_table->data_pages.nr_pages = nr_dpages;
> + sg_table->table_pages.nr_pages = nr_tpages;
> + sg_table->node = node;
> + sg_table->dev = dev;
> +
> + rc = tmc_alloc_data_pages(sg_table, pages);
> + if (!rc)
> + rc = tmc_alloc_table_pages(sg_table);
> + if (rc) {
> + tmc_free_sg_table(sg_table);
> + kfree(sg_table);
> + return ERR_PTR(rc);
> + }
> +
> + return sg_table;
> +}
> +
> +/*
> + * tmc_sg_table_sync_data_range: Sync the data buffer written
> + * by the device from @offset upto a @size bytes.
> + */
> +void tmc_sg_table_sync_data_range(struct tmc_sg_table *table,
> + u64 offset, u64 size)
> +{
> + int i, index, start;
> + int npages = DIV_ROUND_UP(size, PAGE_SIZE);
> + struct device *dev = table->dev;
> + struct tmc_pages *data = &table->data_pages;
> +
> + start = offset >> PAGE_SHIFT;
> + for (i = start; i < (start + npages); i++) {
> + index = i % data->nr_pages;
> + dma_sync_single_for_cpu(dev, data->daddrs[index],
> + PAGE_SIZE, DMA_FROM_DEVICE);
> + }
> +}
> +
> +/* tmc_sg_sync_table: Sync the page table */
> +void tmc_sg_table_sync_table(struct tmc_sg_table *sg_table)
> +{
> + int i;
> + struct device *dev = sg_table->dev;
> + struct tmc_pages *table_pages = &sg_table->table_pages;
> +
> + for (i = 0; i < table_pages->nr_pages; i++)
> + dma_sync_single_for_device(dev, table_pages->daddrs[i],
> + PAGE_SIZE, DMA_TO_DEVICE);
> +}
> +
> +/*
> + * tmc_sg_table_get_data: Get the buffer pointer for data @offset
> + * in the SG buffer. The @bufpp is updated to point to the buffer.
> + * Returns :
> + * the length of linear data available at @offset.
> + * or
> + * <= 0 if no data is available.
> + */
> +ssize_t tmc_sg_table_get_data(struct tmc_sg_table *sg_table,
> + u64 offset, size_t len, char **bufpp)
> +{
> + size_t size;
> + int pg_idx = offset >> PAGE_SHIFT;
> + int pg_offset = offset & (PAGE_SIZE - 1);
> + struct tmc_pages *data_pages = &sg_table->data_pages;
> +
> + size = tmc_sg_table_buf_size(sg_table);
> + if (offset >= size)
> + return -EINVAL;
> +
> + /* Make sure we don't go beyond the end */
> + len = (len < (size - offset)) ? len : size - offset;
> + /* Respect the page boundaries */
> + len = (len < (PAGE_SIZE - pg_offset)) ? len : (PAGE_SIZE - pg_offset);
> + if (len > 0)
> + *bufpp = page_address(data_pages->pages[pg_idx]) + pg_offset;
> + return len;
> +}
> +
> static void tmc_etr_enable_hw(struct tmc_drvdata *drvdata)
> {
> u32 axictl, sts;
> diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h
> index 73f944d..19a765c 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc.h
> +++ b/drivers/hwtracing/coresight/coresight-tmc.h
> @@ -18,6 +18,7 @@
> #ifndef _CORESIGHT_TMC_H
> #define _CORESIGHT_TMC_H
>
> +#include <linux/dma-mapping.h>
> #include <linux/miscdevice.h>
>
> #define TMC_RSZ 0x004
> @@ -171,6 +172,38 @@ struct tmc_drvdata {
> u32 etr_caps;
> };
>
> +/**
> + * struct tmc_pages - Collection of pages used for SG.
> + * @nr_pages: Number of pages in the list.
> + * @daddrs: Array of DMA'able page address.
> + * @pages: Array pages for the buffer.
> + */
> +struct tmc_pages {
> + int nr_pages;
> + dma_addr_t *daddrs;
> + struct page **pages;
> +};
> +
> +/*
> + * struct tmc_sg_table - Generic SG table for TMC
> + * @dev: Device for DMA allocations
> + * @table_vaddr: Contiguous Virtual address for PageTable
> + * @data_vaddr: Contiguous Virtual address for Data Buffer
> + * @table_daddr: DMA address of the PageTable base
> + * @node: Node for Page allocations
> + * @table_pages: List of pages & dma address for Table
> + * @data_pages: List of pages & dma address for Data
> + */
> +struct tmc_sg_table {
> + struct device *dev;
> + void *table_vaddr;
> + void *data_vaddr;
> + dma_addr_t table_daddr;
> + int node;
> + struct tmc_pages table_pages;
> + struct tmc_pages data_pages;
> +};
> +
> /* Generic functions */
> void tmc_wait_for_tmcready(struct tmc_drvdata *drvdata);
> void tmc_flush_and_stop(struct tmc_drvdata *drvdata);
> @@ -226,4 +259,21 @@ static inline bool tmc_etr_has_cap(struct tmc_drvdata *drvdata, u32 cap)
> return !!(drvdata->etr_caps & cap);
> }
>
> +struct tmc_sg_table *tmc_alloc_sg_table(struct device *dev,
> + int node,
> + int nr_tpages,
> + int nr_dpages,
> + void **pages);
> +void tmc_free_sg_table(struct tmc_sg_table *sg_table);
> +void tmc_sg_table_sync_table(struct tmc_sg_table *sg_table);
> +void tmc_sg_table_sync_data_range(struct tmc_sg_table *table,
> + u64 offset, u64 size);
> +ssize_t tmc_sg_table_get_data(struct tmc_sg_table *sg_table,
> + u64 offset, size_t len, char **bufpp);
> +static inline unsigned long
> +tmc_sg_table_buf_size(struct tmc_sg_table *sg_table)
> +{
> + return sg_table->data_pages.nr_pages << PAGE_SHIFT;
> +}
> +
> #endif
> --
> 2.7.4
>

\
 
 \ /
  Last update: 2018-05-23 22:27    [W:0.132 / U:2.380 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site