linux-kernel - Re: [PATCH RFC v4 09/15] spi: axi-spi-engine: implement offload support

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: <f7fd8929-352a-46e2-8f78-15720ab31b9d@baylibre.com>
Date: Fri, 25 Oct 2024 11:35:42 -0500
From: David Lechner <dlechner@...libre.com>
To: Nuno Sá <noname.nuno@...il.com>,
 Mark Brown <broonie@...nel.org>, Jonathan Cameron <jic23@...nel.org>,
 Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
 Conor Dooley <conor+dt@...nel.org>, Nuno Sá
 <nuno.sa@...log.com>, Uwe Kleine-König <ukleinek@...nel.org>
Cc: Michael Hennerich <Michael.Hennerich@...log.com>,
 Lars-Peter Clausen <lars@...afoo.de>, David Jander <david@...tonic.nl>,
 Martin Sperl <kernel@...tin.sperl.org>, linux-spi@...r.kernel.org,
 devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-iio@...r.kernel.org, linux-pwm@...r.kernel.org
Subject: Re: [PATCH RFC v4 09/15] spi: axi-spi-engine: implement offload
 support

On 10/25/24 8:09 AM, Nuno Sá wrote:
> On Wed, 2024-10-23 at 15:59 -0500, David Lechner wrote:
>> Implement SPI offload support for the AXI SPI Engine. Currently, the
>> hardware only supports triggering offload transfers with a hardware
>> trigger so attempting to use an offload message in the regular SPI
>> message queue will fail. Also, only allows streaming rx data to an
>> external sink, so attempts to use a rx_buf in the offload message will
>> fail.
>>
>> Signed-off-by: David Lechner <dlechner@...libre.com>
>> ---
>>

...

>> +static int spi_engine_offload_prepare(struct spi_message *msg)
>> +{
>> +	struct spi_controller *host = msg->spi->controller;
>> +	struct spi_engine *spi_engine = spi_controller_get_devdata(host);
>> +	struct spi_engine_program *p = msg->opt_state;
>> +	struct spi_engine_offload *priv = msg->offload->priv;
>> +	struct spi_transfer *xfer;
>> +	void __iomem *cmd_addr;
>> +	void __iomem *sdo_addr;
>> +	size_t tx_word_count = 0;
>> +	unsigned int i;
>> +
>> +	if (p->length > spi_engine->offload_ctrl_mem_size)
>> +		return -EINVAL;
>> +
>> +	/* count total number of tx words in message */
>> +	list_for_each_entry(xfer, &msg->transfers, transfer_list) {
>> +		if (!xfer->tx_buf)
>> +			continue;
>> +
>> +		if (xfer->bits_per_word <= 8)
>> +			tx_word_count += xfer->len;
>> +		else if (xfer->bits_per_word <= 16)
>> +			tx_word_count += xfer->len / 2;
>> +		else
>> +			tx_word_count += xfer->len / 4;
>> +	}
>> +
>> +	if (tx_word_count > spi_engine->offload_sdo_mem_size)
>> +		return -EINVAL;
>> +
>> +	if (test_and_set_bit_lock(SPI_ENGINE_OFFLOAD_FLAG_PREPARED, &priv->flags))
>> +		return -EBUSY;
>> +
> 
> This is odd. Any special reason for using this with aquire - release semantics? Can
> optimize() and unoptimize() run concurrently? Because if they can this does not give
> us mutual exclusion and we really need to do what we're doing with kind of stuff :)
> 
> - Nuno Sá
> 
> 

This looks like another leftover from an in-between revision that
didn't get fully cleaned up. I will reconsider what is need here.

But to answer the question, strictly speaking, there isn't anything
to prevent two concurrent calls spi_optimize_message() with different
messages but the same offload instance.