linux-kernel - Re: [PATCH] dmaengine: bcm2835: Fix polling for completion of DMA with interrupts masked.

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] [day] [month] [year] [list]

Message-ID: <87eg8867ma.fsf@eliezer.anholt.net>
Date:	Tue, 07 Jun 2016 13:56:13 -0700
From:	Eric Anholt <eric@...olt.net>
To:	Vinod Koul <vinod.koul@...el.com>
Cc:	linux-rpi-kernel@...ts.infradead.org,
	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
	Stephen Warren <swarren@...dotorg.org>,
	Lee Jones <lee@...nel.org>, dmaengine@...r.kernel.org
Subject: Re: [PATCH] dmaengine: bcm2835: Fix polling for completion of DMA with interrupts masked.

Vinod Koul <vinod.koul@...el.com> writes:

> On Mon, Jun 06, 2016 at 11:10:38PM -0700, Eric Anholt wrote:
>> >> >> -	if (ret == DMA_COMPLETE || !txstate)
>> >> >> +	if (ret == DMA_COMPLETE)
>> >> >
>> >> > Why do you change this? txstate can be NULL, so no point calculating reside
>> >> > for those cases
>> >> 
>> >> The point was to go into the "Calculate where we're at in our current
>> >> DMA (if the current DMA is the one we're asking about status for)" path,
>> >> so that we could note when the DMA is complete even when there's no
>> >> txstate passed in.
>> >
>> > Can you explain what you mean by current DMA!
>> >
>> > The claulation is always done for 'descriptor' represnted by the cookie. So
>> > it doesnt not matter...!
>> 
>> By current I mean the current descriptor that has been submitted to the
>> hardware, in bcm2835_chan->desc.
>
> As I said, you calculate for the descriptor respresnted by cookie and
> not the one getting processed!

I believe I'm calculating the state for the descriptor being processed
only in the case where the cookie being asked about is for the
descriptor being processed.  I'm confused what your objection is, so I'm
going to annotate what I think is going on in the function so maybe you
can point to where I've got something wrong.

	spin_lock_irqsave(&c->vc.lock, flags);
	vd = vchan_find_desc(&c->vc, cookie);
	if (vd) {
		txstate->residue =
			bcm2835_dma_desc_size(to_bcm2835_dma_desc(&vd->tx));

I believe this is the case for "the descriptor for the cookie being
asked about hasn't been pulled off the list and submitted to the
hardware yet"

	} else if (c->desc && c->desc->vd.tx.cookie == cookie) {
		struct bcm2835_desc *d = c->desc;
		dma_addr_t pos;

		if (d->dir == DMA_MEM_TO_DEV)
			pos = readl(c->chan_base + BCM2835_DMA_SOURCE_AD);
		else if (d->dir == DMA_DEV_TO_MEM)
			pos = readl(c->chan_base + BCM2835_DMA_DEST_AD);
		else
			pos = 0;

		txstate->residue = bcm2835_dma_desc_size_pos(d, pos);

Here the descriptor for the cookie is currently being processed by the
hardware.  It might also be done but hasn't had its done interrupt
handled yet, so this is the case where I want to effectively do the done
IRQ's work of updating the status.

	} else {
		txstate->residue = 0;

Here, it's not in the hardware and it's not queued to be submitted to
the hardware, so it must be done.

	}

What am I missing?


Download attachment "signature.asc" of type "application/pgp-signature" (819 bytes)