linux-kernel - Re: [PATCH 1/2] dma: pl330: improve pl330_tx

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, 25 Nov 2014 13:43:24 +0100
From:	Lars-Peter Clausen <lars@...afoo.de>
To:	Robert Baldyga <r.baldyga@...sung.com>, vinod.koul@...el.com
CC:	dan.j.williams@...el.com, ars@...afoo.de,
	dmaengine@...r.kernel.org, linux-kernel@...r.kernel.org,
	m.szyprowski@...sung.com, l.czerwinski@...sung.com
Subject: Re: [PATCH 1/2] dma: pl330: improve pl330_tx_status() function

On 11/25/2014 01:25 PM, Robert Baldyga wrote:
[...]
>   static enum dma_status
>   pl330_tx_status(struct dma_chan *chan, dma_cookie_t cookie,
>   		 struct dma_tx_state *txstate)
>   {
> -	return dma_cookie_status(chan, cookie, txstate);
> +	enum dma_status ret;
> +	unsigned long flags;
> +	struct dma_pl330_desc *desc;
> +	struct dma_pl330_chan *pch = to_pchan(chan);
> +	unsigned int bytes_transferred;
> +	unsigned int residual;
> +
> +	/* Check in pending list */
> +	spin_lock_irqsave(&pch->lock, flags);
> +	list_for_each_entry(desc, &pch->work_list, node) {
> +		if (desc->txd.cookie == cookie) {
> +			bytes_transferred =
> +				pl330_get_current_xferred_count(pch, desc);
> +			residual =  desc->bytes_requested -
> +				bytes_transferred % desc->bytes_requested;
> +			dma_set_residue(txstate, residual);
> +			ret = desc->status;
> +			spin_unlock_irqrestore(&pch->lock, flags);
> +			return ret;

I don't think this has the correct semantics. The expected behavior is that 
you pass a cookie of a descriptor and tx_status tells you how many bytes for 
the whole descriptor are still left to be transferred. What you implemented 
tells you how many bytes are still left for the current segment of the 
active descriptor. This will only work fine for descriptors that only have 
one segment. But this will definitely break audio playback using the PL330 
where you have more than one segment per descriptor.


> +		}
> +	}
> +	spin_unlock_irqrestore(&pch->lock, flags);
> +
> +	ret = dma_cookie_status(chan, cookie, txstate);
> +	dma_set_residue(txstate,  pch->transfered);
> +

pch->transfered is always 0?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/