linux-kernel - Re: [PATCH v2] coresight: tmc-etr: Speed up for bounce buffer in flat mode

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]

Message-ID: <20210712110916.GB704210@leoy-ThinkPad-X240s>
Date:   Mon, 12 Jul 2021 19:09:16 +0800
From:   Leo Yan <leo.yan@...aro.org>
To:     Suzuki K Poulose <suzuki.poulose@....com>
Cc:     Mathieu Poirier <mathieu.poirier@...aro.org>,
        Mike Leach <mike.leach@...aro.org>,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        coresight@...ts.linaro.org, linux-arm-kernel@...ts.infradead.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] coresight: tmc-etr: Speed up for bounce buffer in
 flat mode

Hi Suzuki,

On Mon, Jul 12, 2021 at 10:55:32AM +0100, Suzuki Kuruppassery Poulose wrote:

[...]

> >   static void tmc_etr_sync_flat_buf(struct etr_buf *etr_buf, u64 rrp, u64 rwp)
> >   {
> > +	struct etr_flat_buf *flat_buf = etr_buf->private;
> > +	struct device *real_dev = flat_buf->dev->parent;
> > +
> >   	/*
> >   	 * Adjust the buffer to point to the beginning of the trace data
> >   	 * and update the available trace data.
> > @@ -648,6 +668,28 @@ static void tmc_etr_sync_flat_buf(struct etr_buf *etr_buf, u64 rrp, u64 rwp)
> >   		etr_buf->len = etr_buf->size;
> >   	else
> >   		etr_buf->len = rwp - rrp;
> > +
> > +	if (etr_buf->offset + etr_buf->len > etr_buf->size) {
> > +		int len1, len2;
> > +
> > +		/*
> > +		 * If trace data is wrapped around, sync AUX bounce buffer
> > +		 * for two chunks: "len1" is for the trace date length at
> > +		 * the tail of bounce buffer, and "len2" is the length from
> > +		 * the start of the buffer after wrapping around.
> > +		 */
> > +		len1 = etr_buf->size - etr_buf->offset;
> > +		len2 = etr_buf->len - len1;
> > +		dma_sync_single_for_cpu(real_dev,
> > +					flat_buf->daddr + etr_buf->offset,
> > +					len1, DMA_FROM_DEVICE);
> > +		dma_sync_single_for_cpu(real_dev, flat_buf->daddr,
> > +					len2, DMA_FROM_DEVICE);
> 
> We always start tracing at the beginning of the buffer and the only reason
> why we would get a wrap around, is when the buffer is full.
> So you could as well sync the entire buffer in one go
> 
> 		dma_sync_single_for_cpu(real_dev, flat_buf->daddr,
> 					etr_buf->len, DMA_FROM_DEVICE);

I am doubt why you conclude "always start tracing at the beginning of
the buffer"?  I read the driver but cannot find any code in the driver
to reset rrp and rwp after fetching the trace data, or there have any
implict operation to reset pointers?

Just want to double check for this, in case I miss anything.  Thanks
for the review.

Leo