linux-kernel - Re: [PATCH 1/5] spi: add spi_optimize

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: <92e7e0acf6d8746a07729924982acbfea777c468.camel@gmail.com>
Date: Tue, 13 Feb 2024 10:53:56 +0100
From: Nuno Sá <noname.nuno@...il.com>
To: David Lechner <dlechner@...libre.com>, Mark Brown <broonie@...nel.org>
Cc: Martin Sperl <kernel@...tin.sperl.org>, David Jander
 <david@...tonic.nl>,  Jonathan Cameron <jic23@...nel.org>, Michael
 Hennerich <michael.hennerich@...log.com>, Nuno Sá
 <nuno.sa@...log.com>, Alain Volmat <alain.volmat@...s.st.com>, Maxime
 Coquelin <mcoquelin.stm32@...il.com>, Alexandre Torgue
 <alexandre.torgue@...s.st.com>,  linux-spi@...r.kernel.org,
 linux-kernel@...r.kernel.org,  linux-stm32@...md-mailman.stormreply.com,
 linux-arm-kernel@...ts.infradead.org,  linux-iio@...r.kernel.org
Subject: Re: [PATCH 1/5] spi: add spi_optimize_message() APIs

On Mon, 2024-02-12 at 17:26 -0600, David Lechner wrote:
> This adds a new spi_optimize_message() function that can be used to
> optimize SPI messages that are used more than once. Peripheral drivers
> that use the same message multiple times can use this API to perform SPI
> message validation and controller-specific optimizations once and then
> reuse the message while avoiding the overhead of revalidating the
> message on each spi_(a)sync() call.
> 
> Internally, the SPI core will also call this function for each message
> if the peripheral driver did not explicitly call it. This is done to so
> that controller drivers don't have to have multiple code paths for
> optimized and non-optimized messages.
> 
> A hook is provided for controller drivers to perform controller-specific
> optimizations.
> 
> Suggested-by: Martin Sperl <kernel@...tin.sperl.org>
> Link:
> https://lore.kernel.org/linux-spi/39DEC004-10A1-47EF-9D77-276188D2580C@martin.sperl.org/
> Signed-off-by: David Lechner <dlechner@...libre.com>
> ---
>  drivers/spi/spi.c       | 145 ++++++++++++++++++++++++++++++++++++++++++++++-
> -
>  include/linux/spi/spi.h |  19 +++++++
>  2 files changed, 160 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
> index c2b10e2c75f0..5bac215d7009 100644
> --- a/drivers/spi/spi.c
> +++ b/drivers/spi/spi.c
> @@ -2106,6 +2106,41 @@ struct spi_message *spi_get_next_queued_message(struct
> spi_controller *ctlr)
>  }
>  EXPORT_SYMBOL_GPL(spi_get_next_queued_message);
>  
> +/**
> + * __spi_unoptimize_message - shared implementation of
> spi_unoptimize_message()
> + *                            and spi_maybe_unoptimize_message()
> + * @msg: the message to unoptimize
> + *
> + * Periperhal drivers should use spi_unoptimize_message() and callers inside
> + * core should use spi_maybe_unoptimize_message() rather than calling this
> + * function directly.
> + *
> + * It is not valid to call this on a message that is not currently optimized.
> + */
> +static void __spi_unoptimize_message(struct spi_message *msg)
> +{
> +	struct spi_controller *ctlr = msg->spi->controller;
> +
> +	if (ctlr->unoptimize_message)
> +		ctlr->unoptimize_message(msg);
> +
> +	msg->optimized = false;
> +	msg->opt_state = NULL;
> +}
> +
> +/**
> + * spi_maybe_unoptimize_message - unoptimize msg not managed by a peripheral
> + * @msg: the message to unoptimize
> + *
> + * This function is used to unoptimize a message if and only if it was
> + * optimized by the core (via spi_maybe_optimize_message()).
> + */
> +static void spi_maybe_unoptimize_message(struct spi_message *msg)
> +{
> +	if (!msg->pre_optimized && msg->optimized)
> +		__spi_unoptimize_message(msg);
> +}
> +
>  /**
>   * spi_finalize_current_message() - the current message is complete
>   * @ctlr: the controller to return the message to
> @@ -2153,6 +2188,8 @@ void spi_finalize_current_message(struct spi_controller
> *ctlr)
>  
>  	mesg->prepared = false;
>  
> +	spi_maybe_unoptimize_message(mesg);
> +
>  	WRITE_ONCE(ctlr->cur_msg_incomplete, false);
>  	smp_mb(); /* See __spi_pump_transfer_message()... */
>  	if (READ_ONCE(ctlr->cur_msg_need_completion))
> @@ -4194,6 +4231,99 @@ static int __spi_validate(struct spi_device *spi,
> struct spi_message *message)
>  	return 0;
>  }
>  
> +/**
> + * __spi_optimize_message - shared implementation for spi_optimize_message()
> + *                          and spi_maybe_optimize_message()
> + * @spi: the device that will be used for the message
> + * @msg: the message to optimize
> + * @pre_optimized: whether the message is considered pre-optimized or not
> + *
> + * Peripheral drivers will call spi_optimize_message() and the spi core will
> + * call spi_maybe_optimize_message() instead of calling this directly.
> + *
> + * It is not valid to call this on a message that has already been optimized.
> + *
> + * Return: zero on success, else a negative error code
> + */
> +static int __spi_optimize_message(struct spi_device *spi,
> +				  struct spi_message *msg,
> +				  bool pre_optimized)
> +{
> +	struct spi_controller *ctlr = spi->controller;
> +	int ret;
> +
> +	ret = __spi_validate(spi, msg);
> +	if (ret)
> +		return ret;
> +
> +	if (ctlr->optimize_message) {
> +		ret = ctlr->optimize_message(msg);
> +		if (ret)
> +			return ret;
> +	}

Not really sure what are the spi core guarantees or what controllers should be
expecting but I'll still ask :). Do we need to care about locking in here?
Mainly on the controller callback? For spi device related data I guess it's up
to the peripheral driver not to do anything weird or to properly protect the spi
message?

- Nuno Sá