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: <20240206-axi-spi-engine-round-2-1-v1-1-ea6eeb60f4fb@baylibre.com>
Date: Tue,  6 Feb 2024 14:31:27 -0600
From: David Lechner <dlechner@...libre.com>
To: linux-spi@...r.kernel.org
Cc: David Lechner <dlechner@...libre.com>,
	Mark Brown <broonie@...nel.org>,
	Michael Hennerich <michael.hennerich@...log.com>,
	Nuno Sá <nuno.sa@...log.com>,
	linux-kernel@...r.kernel.org
Subject: [PATCH 1/2] spi: axi-spi-engine: remove use of ida for sync id

Profiling has shown that ida_alloc_range() accounts for about 10% of the
time spent in spi_sync() when using the AXI SPI Engine controller. This
call is used to create a unique id for each SPI message to match to an
IRQ when the message is complete.

Since the core SPI code serializes messages in a message queue, we can
only have one message in flight at a time, namely host->cur_msg. This
means that we can use a fixed value instead of a unique id for each
message since there can never be more than one message pending at a
time.

This patch removes the use of ida for the sync id and replaces it with a
constant value. This simplifies the driver and improves performance.

Signed-off-by: David Lechner <dlechner@...libre.com>
---
 drivers/spi/spi-axi-spi-engine.c | 27 ++++++---------------------
 1 file changed, 6 insertions(+), 21 deletions(-)

diff --git a/drivers/spi/spi-axi-spi-engine.c b/drivers/spi/spi-axi-spi-engine.c
index 6b0c72bf3395..9cc602075c17 100644
--- a/drivers/spi/spi-axi-spi-engine.c
+++ b/drivers/spi/spi-axi-spi-engine.c
@@ -57,6 +57,9 @@
 #define SPI_ENGINE_TRANSFER_WRITE		0x1
 #define SPI_ENGINE_TRANSFER_READ		0x2
 
+/* Arbitrary sync ID for use by host->cur_msg */
+#define AXI_SPI_ENGINE_CUR_MSG_SYNC_ID		0x1
+
 #define SPI_ENGINE_CMD(inst, arg1, arg2) \
 	(((inst) << 12) | ((arg1) << 8) | (arg2))
 
@@ -98,8 +101,6 @@ struct spi_engine_message_state {
 	unsigned int rx_length;
 	/** @rx_buf: Bytes not yet written to the RX FIFO. */
 	uint8_t *rx_buf;
-	/** @sync_id: ID to correlate SYNC interrupts with this message. */
-	u8 sync_id;
 };
 
 struct spi_engine {
@@ -109,7 +110,6 @@ struct spi_engine {
 	spinlock_t lock;
 
 	void __iomem *base;
-	struct ida sync_ida;
 	struct timer_list watchdog_timer;
 	struct spi_controller *controller;
 
@@ -483,9 +483,7 @@ static irqreturn_t spi_engine_irq(int irq, void *devid)
 	}
 
 	if (pending & SPI_ENGINE_INT_SYNC && msg) {
-		struct spi_engine_message_state *st = msg->state;
-
-		if (completed_id == st->sync_id) {
+		if (completed_id == AXI_SPI_ENGINE_CUR_MSG_SYNC_ID) {
 			if (timer_delete_sync(&spi_engine->watchdog_timer)) {
 				msg->status = 0;
 				msg->actual_length = msg->frame_length;
@@ -510,10 +508,8 @@ static int spi_engine_prepare_message(struct spi_controller *host,
 				      struct spi_message *msg)
 {
 	struct spi_engine_program p_dry, *p;
-	struct spi_engine *spi_engine = spi_controller_get_devdata(host);
 	struct spi_engine_message_state *st;
 	size_t size;
-	int ret;
 
 	st = kzalloc(sizeof(*st), GFP_KERNEL);
 	if (!st)
@@ -531,18 +527,10 @@ static int spi_engine_prepare_message(struct spi_controller *host,
 		return -ENOMEM;
 	}
 
-	ret = ida_alloc_range(&spi_engine->sync_ida, 0, U8_MAX, GFP_KERNEL);
-	if (ret < 0) {
-		kfree(p);
-		kfree(st);
-		return ret;
-	}
-
-	st->sync_id = ret;
-
 	spi_engine_compile_message(msg, false, p);
 
-	spi_engine_program_add_cmd(p, false, SPI_ENGINE_CMD_SYNC(st->sync_id));
+	spi_engine_program_add_cmd(p, false, SPI_ENGINE_CMD_SYNC(
+						AXI_SPI_ENGINE_CUR_MSG_SYNC_ID));
 
 	st->p = p;
 	st->cmd_buf = p->instructions;
@@ -555,10 +543,8 @@ static int spi_engine_prepare_message(struct spi_controller *host,
 static int spi_engine_unprepare_message(struct spi_controller *host,
 					struct spi_message *msg)
 {
-	struct spi_engine *spi_engine = spi_controller_get_devdata(host);
 	struct spi_engine_message_state *st = msg->state;
 
-	ida_free(&spi_engine->sync_ida, st->sync_id);
 	kfree(st->p);
 	kfree(st);
 
@@ -640,7 +626,6 @@ static int spi_engine_probe(struct platform_device *pdev)
 	spi_engine = spi_controller_get_devdata(host);
 
 	spin_lock_init(&spi_engine->lock);
-	ida_init(&spi_engine->sync_ida);
 	timer_setup(&spi_engine->watchdog_timer, spi_engine_timeout, TIMER_IRQSAFE);
 	spi_engine->controller = host;
 

-- 
2.43.0


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ