linux-kernel - Re: [PATCH 5/5] dmaengine: plx-dma: Implement descriptor submission

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:   Mon, 11 Nov 2019 11:11:26 -0700
From:   Logan Gunthorpe <logang@...tatee.com>
To:     Vinod Koul <vkoul@...nel.org>
Cc:     linux-kernel@...r.kernel.org, dmaengine@...r.kernel.org,
        Dan Williams <dan.j.williams@...el.com>
Subject: Re: [PATCH 5/5] dmaengine: plx-dma: Implement descriptor submission



On 2019-11-09 10:40 a.m., Vinod Koul wrote:
>> +static dma_cookie_t plx_dma_tx_submit(struct dma_async_tx_descriptor *desc)
>> +	__releases(plxdev->ring_lock)
>> +{
>> +	struct plx_dma_dev *plxdev = chan_to_plx_dma_dev(desc->chan);
>> +	struct plx_dma_desc *plxdesc = to_plx_desc(desc);
>> +	dma_cookie_t cookie;
>> +
>> +	cookie = dma_cookie_assign(desc);
>> +
>> +	/*
>> +	 * Ensure the descriptor updates are visible to the dma device
>> +	 * before setting the valid bit.
>> +	 */
>> +	wmb();
>> +
>> +	plxdesc->hw->flags_and_size |= cpu_to_le32(PLX_DESC_FLAG_VALID);
> 
> so where do you store the submitted descriptor?

The descriptors are stored in a ring in memory which the DMA engine
reads. The ring is at (struct plx_dma_dev)->hw_ring. plxdesc->hw is a
pointer to the descriptor's specific entry in the hardware's ring. The
hardware descriptor is populated in plx_dma_prep_memcpy(). Once the
valid flag is set, the hardware owns the descriptor and may start
processing it.

>> +
>> +	spin_unlock_bh(&plxdev->ring_lock);
>> +
>> +	return cookie;
>> +}
>> +
>> +static enum dma_status plx_dma_tx_status(struct dma_chan *chan,
>> +		dma_cookie_t cookie, struct dma_tx_state *txstate)
>> +{
>> +	struct plx_dma_dev *plxdev = chan_to_plx_dma_dev(chan);
>> +	enum dma_status ret;
>> +
>> +	ret = dma_cookie_status(chan, cookie, txstate);
>> +	if (ret == DMA_COMPLETE)
>> +		return ret;
>> +
>> +	plx_dma_process_desc(plxdev);
> 
> why is this done here..? Query of status should not make you process
> something!

When descriptors are submitted without interrupts, something has to
cleanup the completed descriptors and this is the only sensible place to
do that. This is exactly what IOAT does[1] (but with a different name
and a bit more complexity).

>> +
>> +	return dma_cookie_status(chan, cookie, txstate);
>> +}
>> +
>> +static void plx_dma_issue_pending(struct dma_chan *chan)
>> +{
>> +	struct plx_dma_dev *plxdev = chan_to_plx_dma_dev(chan);
>> +
>> +	rcu_read_lock();
>> +	if (!rcu_dereference(plxdev->pdev)) {
>> +		rcu_read_unlock();
>> +		return;
>> +	}
>> +
>> +	/*
>> +	 * Ensure the valid bits are visible before starting the
>> +	 * DMA engine.
>> +	 */
>> +	wmb();
>> +
>> +	writew(PLX_REG_CTRL_START_VAL, plxdev->bar + PLX_REG_CTRL);
> 
> start what? 

The hardware processes entries in the ring and once it reaches the end
of the submitted descriptors, then it simply stops forever. Setting this
bit will start it processing entries again (or, if it is already
running, nothing will happen).

Logan

[1]
https://elixir.bootlin.com/linux/latest/source/drivers/dma/ioat/dma.c#L962