All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC] blk/scsi/ufs: scatter-gather read support
@ 2026-11-10 22:52 Urvashi Sharma
  2026-11-11  8:14 ` neil.armstrong
  2026-11-14  4:05 ` [RFC PATCH 0/3] " Urvashi Sharma
  0 siblings, 2 replies; 7+ messages in thread
From: Urvashi Sharma @ 2026-11-10 22:52 UTC (permalink / raw)
  To: u-boot
  Cc: neil.armstrong, bhupesh.linux, n-francis, marek.vasut+renesas,
	iwamatsu, trini

Hi all,

I would like feedback on a design for scatter-gather (SG) reads on SCSI
block devices, with UFS as the first to implement it. This is
a design discussion only; no patches are attached. The struct/function
sketches below exist to make the proposal concrete enough to comment on,
not as a finished implementation.

Problem
=======

blk_read()/blk_dread() take one contiguous destination buffer. The UFS
driver splits that buffer into several 256KB PRDT entries
(prepare_prdt_table() in drivers/ufs/ufs-uclass.c, MAX_PRDT_ENTRY), but
a caller cannot read one LBA range into several discontiguous buffers.
Such a caller has to issue one command per buffer today.

Use cases:

- FIT images: a FIT can bundle several images contiguously on storage,
  each with its own load address. Loading one FIT means pulling several
  images from storage into several different destination buffers,
  which today is one read per image.

- ELF loading: U-Boot has no path today for loading an ELF image
  directly from storage (bootelf expects the image to already be in
  memory); I raised this gap earlier in [1]. If such a loader is added,
  its segments could land at different, non-contiguous memory
  locations per their program headers, and would hit the same
  one-command-per-buffer pattern.

In both cases the data being read is logically one transfer being split
across destinations, but the current path pays for a full command
setup / UPIU exchange / doorbell ring / completion poll cycle per
buffer. SG would let one request cover several destination buffers
instead of one request per buffer.

I do not have measurements yet -- this is still at the design stage.

Proposed design
===============

Rather than extend the existing single-buffer read path in place, this
adds a separate flow for scatter reads that runs alongside the existing
one, with its own error handling at each layer. SG is not inherently
UFS-specific, so the block and SCSI layers below are meant to be
controller-neutral even though UFS is where I would start.

1. Block layer (include/blk.h, drivers/block/blk-uclass.c): a new
   optional op alongside the existing read op in struct blk_ops, plus
   the scatter list type it takes:

	struct sg_entry {
		unsigned int addr;
		unsigned int blk_count;
	};

	struct sg_list {
		struct sg_entry *entries;
		unsigned int sg_count;
	};

	unsigned long (*read_sg)(struct udevice *dev, lbaint_t start,
				  struct sg_list *scatter_gather_list,
				  lbaint_t blk_cnt);

   with blk_sg_read() / blk_sg_dread() wrappers (LBA, scatter list,
   block count), returning -ENOSYS when the driver has no read_sg.

2. SCSI layer (include/scsi.h, drivers/scsi/scsi.c): rather than add a
   parallel set of command-building functions, struct scsi_cmd gains an
   optional

	struct sg_list *scatter_gather_list;

   member. The existing scsi_exec()/command-building path in scsi.c is
   reused unchanged; when scatter_gather_list is non-NULL and the
   normal READ(10)/READ(16) command is built as today with the list
   attached.

3. UFS (drivers/ufs/ufs-uclass.c): if scatter-gather is enabled,
   prepare_prdt_table() grows an SG-aware counterpart:

	static int prepare_sg_prdt(struct ufs_hba *hba,
				    struct scsi_cmd *pccb);

   that builds one PRDT entry per segment, within the existing PRDT
   size and count limits, with the cache flushed and invalidated per
   segment instead of once for one buffer. The existing doorbell-ring
   and completion-wait logic is reused as-is; nothing changes at the
   hardware-facing end of the transfer, and existing single-buffer
   reads are unchanged.

4. Kconfig: the generic part is under SCSI_SG and the UFS part under
   UFS_SG (depends on UFS && SCSI_SG), so other controllers can add SG
   later without pulling in UFS code, and builds that do not select
   either symbol see no change in behavior.

Thanks,
Urvashi

[1] https://www.mail-archive.com/u-boot@lists.u-boot-project.org/msg04243.html

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC] blk/scsi/ufs: scatter-gather read support
  2026-11-10 22:52 [RFC] blk/scsi/ufs: scatter-gather read support Urvashi Sharma
@ 2026-11-11  8:14 ` neil.armstrong
  2026-11-12  2:19   ` Urvashi Sharma
  2026-11-14  4:05 ` [RFC PATCH 0/3] " Urvashi Sharma
  1 sibling, 1 reply; 7+ messages in thread
From: neil.armstrong @ 2026-11-11  8:14 UTC (permalink / raw)
  To: u-boot, bhupesh.linux, n-francis, marek.vasut+renesas, iwamatsu,
	trini

Hi,

On 11/10/26 23:52, Urvashi Sharma wrote:
> Hi all,
> 
> I would like feedback on a design for scatter-gather (SG) reads on SCSI
> block devices, with UFS as the first to implement it. This is
> a design discussion only; no patches are attached. The struct/function
> sketches below exist to make the proposal concrete enough to comment on,
> not as a finished implementation.
> 
> Problem
> =======
> 
> blk_read()/blk_dread() take one contiguous destination buffer. The UFS
> driver splits that buffer into several 256KB PRDT entries
> (prepare_prdt_table() in drivers/ufs/ufs-uclass.c, MAX_PRDT_ENTRY), but
> a caller cannot read one LBA range into several discontiguous buffers.
> Such a caller has to issue one command per buffer today.
> 
> Use cases:
> 
> - FIT images: a FIT can bundle several images contiguously on storage,
>    each with its own load address. Loading one FIT means pulling several
>    images from storage into several different destination buffers,
>    which today is one read per image.
> 
> - ELF loading: U-Boot has no path today for loading an ELF image
>    directly from storage (bootelf expects the image to already be in
>    memory); I raised this gap earlier in [1]. If such a loader is added,
>    its segments could land at different, non-contiguous memory
>    locations per their program headers, and would hit the same
>    one-command-per-buffer pattern.
> 
> In both cases the data being read is logically one transfer being split
> across destinations, but the current path pays for a full command
> setup / UPIU exchange / doorbell ring / completion poll cycle per
> buffer. SG would let one request cover several destination buffers
> instead of one request per buffer.
> 
> I do not have measurements yet -- this is still at the design stage.
> 
> Proposed design
> ===============
> 
> Rather than extend the existing single-buffer read path in place, this
> adds a separate flow for scatter reads that runs alongside the existing
> one, with its own error handling at each layer. SG is not inherently
> UFS-specific, so the block and SCSI layers below are meant to be
> controller-neutral even though UFS is where I would start.
> 
> 1. Block layer (include/blk.h, drivers/block/blk-uclass.c): a new
>     optional op alongside the existing read op in struct blk_ops, plus
>     the scatter list type it takes:
> 
> 	struct sg_entry {
> 		unsigned int addr;
> 		unsigned int blk_count;

Where do you pass the the buffer for this entry ?

> 	};
> 
> 	struct sg_list {
> 		struct sg_entry *entries;
> 		unsigned int sg_count;
> 	};

Maybe passing sg_count directly to read_sg would be simpler and avoid
an intermediate struct, except if you want to pass some flags and extend
the sg read.

> 
> 	unsigned long (*read_sg)(struct udevice *dev, lbaint_t start,
> 				  struct sg_list *scatter_gather_list,
> 				  lbaint_t blk_cnt);
> 
>     with blk_sg_read() / blk_sg_dread() wrappers (LBA, scatter list,
>     block count), returning -ENOSYS when the driver has no read_sg.
> 
> 2. SCSI layer (include/scsi.h, drivers/scsi/scsi.c): rather than add a
>     parallel set of command-building functions, struct scsi_cmd gains an
>     optional
> 
> 	struct sg_list *scatter_gather_list;
> 
>     member. The existing scsi_exec()/command-building path in scsi.c is
>     reused unchanged; when scatter_gather_list is non-NULL and the
>     normal READ(10)/READ(16) command is built as today with the list
>     attached.
> 
> 3. UFS (drivers/ufs/ufs-uclass.c): if scatter-gather is enabled,
>     prepare_prdt_table() grows an SG-aware counterpart:
> 
> 	static int prepare_sg_prdt(struct ufs_hba *hba,
> 				    struct scsi_cmd *pccb);
> 
>     that builds one PRDT entry per segment, within the existing PRDT
>     size and count limits, with the cache flushed and invalidated per
>     segment instead of once for one buffer. The existing doorbell-ring
>     and completion-wait logic is reused as-is; nothing changes at the
>     hardware-facing end of the transfer, and existing single-buffer
>     reads are unchanged.
> 
> 4. Kconfig: the generic part is under SCSI_SG and the UFS part under
>     UFS_SG (depends on UFS && SCSI_SG), so other controllers can add SG
>     later without pulling in UFS code, and builds that do not select
>     either symbol see no change in behavior.

The overall design looks sane, but:
- the naming may be misleaded with the "sg" dma, maybe instead use something
   like "multiblock" ? I'm open to anything, including sg TBH
- could you check if some other interfaces could make use of it, or
   perhaps if a dumb helper could be added that would simply go over the
   sg_list and run single transfers to emulate that so we could have
   a generic usage of sg_read in FIT loading for example

Please send a quick RFC to we can discuss the details.

Thanks,
Neil

> 
> Thanks,
> Urvashi
> 
> [1] https://www.mail-archive.com/u-boot@lists.u-boot-project.org/msg04243.html


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC] blk/scsi/ufs: scatter-gather read support
  2026-11-11  8:14 ` neil.armstrong
@ 2026-11-12  2:19   ` Urvashi Sharma
  0 siblings, 0 replies; 7+ messages in thread
From: Urvashi Sharma @ 2026-11-12  2:19 UTC (permalink / raw)
  To: u-boot, Neil Armstrong
  Cc: bhupesh.linux, n-francis, marek.vasut+renesas, iwamatsu, trini

Hi Neil,

Thanks for the quick feedback.

On Wed, Nov 11, 2026 at 09:14:01AM +0100, Neil Armstrong wrote:
> Where do you pass the the buffer for this entry ?

The addr field in sg_entry is meant to be the destination buffer for
the entry. I'll type it as a real pointer (void *) instead of
unsigned int so that's unambiguous.

> Maybe passing sg_count directly to read_sg would be simpler and avoid
> an intermediate struct, except if you want to pass some flags and extend
> the sg read.

Sure, sg_count as an argument works too. I went with a struct mainly
so that if we need extra params later (flags, alignment hints, etc.),
we can extend it without changing the read_sg() signature or every
caller.

> - the naming may be misleaded with the "sg" dma, maybe instead use something
>    like "multiblock" ? I'm open to anything, including sg TBH

Multi-block works, but "scatter-gather" is pretty established
terminology in storage, so I'd lean toward prefixing instead, e.g.
blk_sg_*, hope that would help to make it less confusing.

> - could you check if some other interfaces could make use of it, or
>    perhaps if a dumb helper could be added that would simply go over the
>    sg_list and run single transfers to emulate that so we could have
>    a generic usage of sg_read in FIT loading for example

I looked at the flow, and it's doable to add a fallback that loops
single-buffer reads when a driver doesn't implement read_sg, so
callers don't need to special-case it.

> Please send a quick RFC to we can discuss the details.

I have a rough implementation working locally, so I'll clean it up and
send it as an RFC patch series.

Thanks,
Urvashi

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [RFC PATCH 0/3] blk/scsi/ufs: scatter-gather read support
  2026-11-10 22:52 [RFC] blk/scsi/ufs: scatter-gather read support Urvashi Sharma
  2026-11-11  8:14 ` neil.armstrong
@ 2026-11-14  4:05 ` Urvashi Sharma
  2026-11-14  4:05   ` [RFC PATCH 1/3] blk: add " Urvashi Sharma
                     ` (2 more replies)
  1 sibling, 3 replies; 7+ messages in thread
From: Urvashi Sharma @ 2026-11-14  4:05 UTC (permalink / raw)
  To: u-boot
  Cc: neil.armstrong, bhupesh.linux, n-francis, marek.vasut+renesas,
	iwamatsu, trini, digant.shah

This implements the scatter-gather read design discussed in [1].

Reading one contiguous block range into several separate buffers
currently takes one read request per buffer, since the block uclass
has no way to pass a list of destination addresses to a driver.
Controllers which accept such a list for a single command, like UFS
with its PRDT, cannot make use of it.

Patch 1 adds the block layer side: an optional read_sg() operation
taking a struct blk_sg_list, and blk_read_sg() to drive it.
blk_read_sg() checks that every entry is a non-zero multiple of the
block size and that the lengths add up to the requested block count,
then either calls the driver or loops blk_read() over the list, so
callers behave the same way regardless of the driver. Patches 2 and 3
implement read_sg() for SCSI and UFS.

Changes from the design posted in [1]:

- addr in struct blk_sg_entry is void * rather than unsigned int,
  and entries carry a byte length rather than a block count.

- The list is still a struct rather than a bare count argument, to
  leave room for flags or alignment hints without changing the
  read_sg() signature or its callers.

- Names are prefixed blk_sg_ / read_sg to keep the storage sense of
  scatter-gather distinct from the DMA one.

- blk_read_sg() falls back to one blk_read() per buffer when the
  driver has no read_sg(), or rejects a list with -ENOSYS or
  -EOPNOTSUPP, instead of returning -ENOSYS to the caller.

- Kconfig symbols are BLK_SCATTER_GATHER_READ and
  UFS_SCATTER_GATHER, as the generic part belongs to the block layer
  rather than SCSI.

There is no in-tree user yet, so both symbols default to off and no
defconfig selects them.

Tested on qemu_arm64 with QEMU's UFS device, reading a block range
into several buffers with blk_read_sg() via UFS_PCI.

[1] https://lore.kernel.org/u-boot/ptmm7jaltdwuyxj5ks26oht2cj6ckp4at6prvra27l4r6idkiz@xzyj2s3spx25/

Urvashi Sharma (3):
  blk: add scatter-gather read support
  scsi: add scatter-gather read support
  ufs: add scatter-gather read support

 drivers/block/Kconfig      | 10 +++++
 drivers/block/blk-uclass.c | 75 ++++++++++++++++++++++++++++++++++++++
 drivers/scsi/scsi.c        | 58 +++++++++++++++++++++++++++++
 drivers/ufs/Kconfig        |  8 ++++
 drivers/ufs/ufs-uclass.c   | 75 ++++++++++++++++++++++++++++++++++++--
 include/blk.h              | 61 +++++++++++++++++++++++++++++++
 include/scsi.h             |  6 +++
 7 files changed, 290 insertions(+), 3 deletions(-)

-- 
2.34.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [RFC PATCH 1/3] blk: add scatter-gather read support
  2026-11-14  4:05 ` [RFC PATCH 0/3] " Urvashi Sharma
@ 2026-11-14  4:05   ` Urvashi Sharma
  2026-11-14  4:05   ` [RFC PATCH 2/3] scsi: " Urvashi Sharma
  2026-11-14  4:05   ` [RFC PATCH 3/3] ufs: " Urvashi Sharma
  2 siblings, 0 replies; 7+ messages in thread
From: Urvashi Sharma @ 2026-11-14  4:05 UTC (permalink / raw)
  To: u-boot
  Cc: neil.armstrong, bhupesh.linux, n-francis, marek.vasut+renesas,
	iwamatsu, trini, digant.shah

Reading a contiguous range of blocks into several separate buffers
currently requires one read request per buffer, since the block uclass
has no way to pass a list of destination addresses to a driver.

Add an optional read_sg() block operation which takes a list of
destination buffers, described by struct blk_sg_list, and a
blk_read_sg() wrapper to drive it. The wrapper checks that every entry
is a non-zero multiple of the block size and that the lengths add up
to the requested block count, so drivers can rely on the list being
sane.

Drivers which do not implement read_sg(), or which reject a list with
-ENOSYS or -EOPNOTSUPP, fall back to one blk_read() per buffer.

The feature is guarded by CONFIG_BLK_SCATTER_GATHER_READ.

Signed-off-by: Urvashi Sharma <urvashi.sharma@oss.qualcomm.com>
---
 drivers/block/Kconfig      | 10 +++++
 drivers/block/blk-uclass.c | 75 ++++++++++++++++++++++++++++++++++++++
 include/blk.h              | 61 +++++++++++++++++++++++++++++++
 3 files changed, 146 insertions(+)

diff --git a/drivers/block/Kconfig b/drivers/block/Kconfig
index adf338ab00c..93844f5017a 100644
--- a/drivers/block/Kconfig
+++ b/drivers/block/Kconfig
@@ -82,6 +82,16 @@ config BLKMAP
             boundary. A common example is a filesystem image embedded in an FIT
             image.
 
+config BLK_SCATTER_GATHER_READ
+	bool "Support scatter-gather reads on block devices"
+	depends on BLK
+	help
+	  Add blk_read_sg(), which reads a contiguous range of blocks into a
+	  list of separate buffers. Each buffer length must be a multiple of
+	  the block size. Drivers which implement the read_sg() operation can
+	  read the whole list with a single request, otherwise each buffer is
+	  read separately.
+
 config SPL_BLOCK_CACHE
 	bool "Use block device cache in SPL"
 	depends on SPL_BLK
diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c
index 73c24fd9176..024727431aa 100644
--- a/drivers/block/blk-uclass.c
+++ b/drivers/block/blk-uclass.c
@@ -467,6 +467,73 @@ long blk_read(struct udevice *dev, lbaint_t start, lbaint_t blkcnt, void *buf)
 	return blks_read;
 }
 
+#if IS_ENABLED(CONFIG_BLK_SCATTER_GATHER_READ)
+static long blk_read_sg_fallback(struct udevice *dev, lbaint_t start,
+				 struct blk_sg_list *sgl)
+{
+	struct blk_desc *desc = dev_get_uclass_plat(dev);
+	lbaint_t blkcnt;
+	unsigned int i;
+	long ret;
+
+	for (i = 0; i < sgl->count; i++) {
+		blkcnt = sgl->entries[i].len / desc->blksz;
+		ret = blk_read(dev, start, blkcnt, sgl->entries[i].addr);
+		if (ret != blkcnt)
+			return ret < 0 ? ret : -EIO;
+		start += blkcnt;
+	}
+
+	return 0;
+}
+
+long blk_read_sg(struct udevice *dev, lbaint_t start, lbaint_t blkcnt,
+		 struct blk_sg_list *sgl)
+{
+	struct blk_desc *desc = dev_get_uclass_plat(dev);
+	const struct blk_ops *ops = blk_get_ops(dev);
+	struct blk_sg_entry *ent;
+	lbaint_t cnt = 0;
+	unsigned int i;
+	long ret;
+
+	if (!blkcnt)
+		return 0;
+	if (!sgl || !sgl->count)
+		return -EINVAL;
+
+	for (i = 0; i < sgl->count; i++) {
+		ent = &sgl->entries[i];
+		if (!ent->len || ent->len % desc->blksz)
+			return -EINVAL;
+		cnt += ent->len / desc->blksz;
+	}
+	if (cnt != blkcnt)
+		return -EINVAL;
+
+	if (ops->read_sg) {
+		ulong blks_read = ops->read_sg(dev, start, blkcnt, sgl);
+
+		if (blks_read == blkcnt)
+			return blkcnt;
+
+		if (!IS_ERR_VALUE(blks_read))
+			return -EIO;
+
+		ret = blks_read;
+		if (ret != -ENOSYS && ret != -EOPNOTSUPP)
+			return ret;
+	}
+
+	/* Fall back to one read per buffer */
+	ret = blk_read_sg_fallback(dev, start, sgl);
+	if (ret)
+		return ret;
+
+	return blkcnt;
+}
+#endif	/* CONFIG_BLK_SCATTER_GATHER_READ */
+
 long blk_write(struct udevice *dev, lbaint_t start, lbaint_t blkcnt,
 	       const void *buf)
 {
@@ -520,6 +587,14 @@ ulong blk_dread(struct blk_desc *desc, lbaint_t start, lbaint_t blkcnt,
 	return blk_read(desc->bdev, start, blkcnt, buffer);
 }
 
+#if IS_ENABLED(CONFIG_BLK_SCATTER_GATHER_READ)
+ulong blk_dread_sg(struct blk_desc *desc, lbaint_t start, lbaint_t blkcnt,
+		   struct blk_sg_list *sgl)
+{
+	return blk_read_sg(desc->bdev, start, blkcnt, sgl);
+}
+#endif	/* CONFIG_BLK_SCATTER_GATHER_READ */
+
 ulong blk_dwrite(struct blk_desc *desc, lbaint_t start, lbaint_t blkcnt,
 		 const void *buffer)
 {
diff --git a/include/blk.h b/include/blk.h
index 8d1b70cabd3..d5176077300 100644
--- a/include/blk.h
+++ b/include/blk.h
@@ -199,6 +199,28 @@ static inline void blkcache_free(void) {}
 
 struct udevice;
 
+/**
+ * struct blk_sg_entry - destination buffer for a scatter-gather read
+ *
+ * @addr: Destination buffer address
+ * @len: Buffer length in bytes, a multiple of the block size
+ */
+struct blk_sg_entry {
+	void *addr;
+	size_t len;
+};
+
+/**
+ * struct blk_sg_list - list of scatter-gather buffers, filled in order
+ *
+ * @entries: Array of buffers
+ * @count: Number of entries in @entries
+ */
+struct blk_sg_list {
+	struct blk_sg_entry *entries;
+	unsigned int count;
+};
+
 /* Operations on block devices */
 struct blk_ops {
 	/**
@@ -214,6 +236,25 @@ struct blk_ops {
 	unsigned long (*read)(struct udevice *dev, lbaint_t start,
 			      lbaint_t blkcnt, void *buffer);
 
+#if IS_ENABLED(CONFIG_BLK_SCATTER_GATHER_READ)
+	/**
+	 * read_sg() - read from a block device into scatter-gather buffers
+	 *
+	 * This optional callback reads into the whole list with a single
+	 * request. If it is not provided, or returns -ENOSYS or -EOPNOTSUPP,
+	 * each buffer is read separately with read().
+	 *
+	 * @dev:	Device to read from
+	 * @start:	Start block number to read (0=first)
+	 * @blkcnt:	Number of blocks to read
+	 * @sgl:	List of buffers to read into
+	 * @return number of blocks read, or -ve error number (see the
+	 * IS_ERR_VALUE() macro
+	 */
+	unsigned long (*read_sg)(struct udevice *dev, lbaint_t start,
+				 lbaint_t blkcnt, struct blk_sg_list *sgl);
+#endif	/* CONFIG_BLK_SCATTER_GATHER_READ */
+
 	/**
 	 * write() - write to a block device
 	 *
@@ -287,6 +328,8 @@ struct blk_ops {
  */
 unsigned long blk_dread(struct blk_desc *block_dev, lbaint_t start,
 			lbaint_t blkcnt, void *buffer);
+unsigned long blk_dread_sg(struct blk_desc *block_dev, lbaint_t start,
+			   lbaint_t blkcnt, struct blk_sg_list *sgl);
 unsigned long blk_dwrite(struct blk_desc *block_dev, lbaint_t start,
 			 lbaint_t blkcnt, const void *buffer);
 unsigned long blk_derase(struct blk_desc *block_dev, lbaint_t start,
@@ -307,6 +350,24 @@ unsigned long blk_derase(struct blk_desc *block_dev, lbaint_t start,
 long blk_read(struct udevice *dev, lbaint_t start, lbaint_t blkcnt,
 	      void *buffer);
 
+/**
+ * blk_read_sg() - Read from a block device into scatter-gather buffers
+ *
+ * Each buffer length must be a multiple of the block size and the lengths
+ * must add up to @blkcnt. If the driver cannot read into the buffers
+ * directly, each buffer is read separately using blk_read().
+ *
+ * @dev: Device to read from
+ * @start: Start block for the read
+ * @blkcnt: Number of blocks to read
+ * @sgl: List of buffers to read into
+ * @return @blkcnt on success, or -ve on error (-EINVAL if the list is
+ * invalid, -EIO if the device read only part of the data). This never
+ * returns 0 unless @blkcnt is 0
+ */
+long blk_read_sg(struct udevice *dev, lbaint_t start, lbaint_t blkcnt,
+		 struct blk_sg_list *sgl);
+
 /**
  * blk_write() - Write to a block device
  *
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [RFC PATCH 2/3] scsi: add scatter-gather read support
  2026-11-14  4:05 ` [RFC PATCH 0/3] " Urvashi Sharma
  2026-11-14  4:05   ` [RFC PATCH 1/3] blk: add " Urvashi Sharma
@ 2026-11-14  4:05   ` Urvashi Sharma
  2026-11-14  4:05   ` [RFC PATCH 3/3] ufs: " Urvashi Sharma
  2 siblings, 0 replies; 7+ messages in thread
From: Urvashi Sharma @ 2026-11-14  4:05 UTC (permalink / raw)
  To: u-boot
  Cc: neil.armstrong, bhupesh.linux, n-francis, marek.vasut+renesas,
	iwamatsu, trini, digant.shah

Implement the block uclass read_sg() operation for SCSI devices, so a
list of destination buffers can be filled by a single READ command
rather than one command per buffer.

struct scsi_cmd gains an sgl member which is used in place of pdata
when set, leaving the existing single-buffer path untouched. Whether a
controller can handle such a request is not something the SCSI layer
can tell on its own, so struct scsi_plat gains an sg_supported flag
for the parent driver to set; scsi_read_sg() returns -ENOSYS when it
is clear and the block uclass then falls back to single reads.

The request is also rejected with -EOPNOTSUPP when it exceeds the
controller limit from max_bytes_per_req, or when the block count does
not fit the 16-bit transfer length of READ(10) and READ(16) is not
available.

Signed-off-by: Urvashi Sharma <urvashi.sharma@oss.qualcomm.com>
---
 drivers/scsi/scsi.c | 58 +++++++++++++++++++++++++++++++++++++++++++++
 include/scsi.h      |  6 +++++
 2 files changed, 64 insertions(+)

diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
index 50e7d749921..39c6456e863 100644
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -232,6 +232,61 @@ static ulong scsi_read(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
 	return blkcnt;
 }
 
+#if IS_ENABLED(CONFIG_BLK_SCATTER_GATHER_READ)
+static ulong scsi_read_sg(struct udevice *dev, lbaint_t blknr,
+			  lbaint_t blkcnt, struct blk_sg_list *sgl)
+{
+	struct blk_desc *block_dev = dev_get_uclass_plat(dev);
+	struct udevice *bdev = dev->parent;
+	struct scsi_plat *uc_plat = dev_get_uclass_plat(bdev);
+	struct scsi_cmd *pccb = (struct scsi_cmd *)&tempccb;
+	lbaint_t max_blks;
+	int ret;
+
+	if (!uc_plat->sg_supported)
+		return -ENOSYS;
+
+	if (uc_plat->max_bytes_per_req)
+		max_blks = uc_plat->max_bytes_per_req / block_dev->blksz;
+	else
+		max_blks = SCSI_MAX_BLK;
+	if (blkcnt > max_blks)
+		return -EOPNOTSUPP;
+
+	/* READ(10) only has a 16-bit transfer length */
+	if (blkcnt > SCSI_MAX_BLK &&
+	    (!IS_ENABLED(CONFIG_SYS_64BIT_LBA) || blknr <= SCSI_LBA48_READ))
+		return -EOPNOTSUPP;
+
+	/* Setup device */
+	pccb->target = block_dev->target;
+	pccb->lun = block_dev->lun;
+	pccb->pdata = NULL;
+	pccb->datalen = block_dev->blksz * blkcnt;
+	pccb->dma_dir = DMA_FROM_DEVICE;
+	pccb->sgl = sgl;
+
+	debug("\n%s: dev %d startblk " LBAF ", blkcnt " LBAF " sgcount %u\n",
+	      __func__, block_dev->devnum, blknr, blkcnt, sgl->count);
+#ifdef CONFIG_SYS_64BIT_LBA
+	if (blknr > SCSI_LBA48_READ)
+		scsi_setup_read16(pccb, blknr, blkcnt);
+	else
+#endif
+		scsi_setup_read_ext(pccb, blknr, blkcnt);
+
+	ret = scsi_exec(bdev, pccb);
+	pccb->sgl = NULL;
+	if (ret) {
+		if (ret != -EOPNOTSUPP)
+			scsi_print_error(pccb);
+		return ret;
+	}
+
+	return blkcnt;
+}
+#endif	/* CONFIG_BLK_SCATTER_GATHER_READ */
+
 /*******************************************************************************
  * scsi_write
  */
@@ -675,6 +730,9 @@ int scsi_scan(bool verbose)
 
 static const struct blk_ops scsi_blk_ops = {
 	.read	= scsi_read,
+#if IS_ENABLED(CONFIG_BLK_SCATTER_GATHER_READ)
+	.read_sg	= scsi_read_sg,
+#endif	/* CONFIG_BLK_SCATTER_GATHER_READ */
 	.write	= scsi_write,
 	.erase  = scsi_erase,
 #if IS_ENABLED(CONFIG_BOUNCE_BUFFER)
diff --git a/include/scsi.h b/include/scsi.h
index 2520a8b8fe6..06a44d04fdc 100644
--- a/include/scsi.h
+++ b/include/scsi.h
@@ -13,6 +13,8 @@
 
 struct udevice;
 
+struct blk_sg_list;
+
 /**
  * struct scsi_cmd - information about a SCSI command to be processed
  *
@@ -33,6 +35,7 @@ struct udevice;
  * @trans_bytes: tranfered bytes
  * @priv: Private value
  * @dma_dir: Direction of data structure
+ * @sgl: Scatter-gather list, used instead of @pdata when set
  */
 struct scsi_cmd {
 	unsigned char cmd[16];
@@ -54,6 +57,7 @@ struct scsi_cmd {
 
 	unsigned int priv;
 	enum dma_data_direction dma_dir;
+	struct blk_sg_list *sgl;
 };
 
 /*-----------------------------------------------------------
@@ -269,12 +273,14 @@ struct __packed scsi_write10_req {
  * @max_lun: Maximum number of logical units
  * @max_id: Maximum number of target ids
  * @max_bytes_per_req: Maximum number of bytes per read/write request
+ * @sg_supported: true if the controller supports scatter-gather reads
  */
 struct scsi_plat {
 	unsigned long base;
 	unsigned long max_lun;
 	unsigned long max_id;
 	unsigned long max_bytes_per_req;
+	bool sg_supported;
 };
 
 /* Operations for SCSI */
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [RFC PATCH 3/3] ufs: add scatter-gather read support
  2026-11-14  4:05 ` [RFC PATCH 0/3] " Urvashi Sharma
  2026-11-14  4:05   ` [RFC PATCH 1/3] blk: add " Urvashi Sharma
  2026-11-14  4:05   ` [RFC PATCH 2/3] scsi: " Urvashi Sharma
@ 2026-11-14  4:05   ` Urvashi Sharma
  2 siblings, 0 replies; 7+ messages in thread
From: Urvashi Sharma @ 2026-11-14  4:05 UTC (permalink / raw)
  To: u-boot
  Cc: neil.armstrong, bhupesh.linux, n-francis, marek.vasut+renesas,
	iwamatsu, trini, digant.shah

UFS describes the data buffers for a command with a PRDT, which can
already hold more than one entry. So far only a single contiguous
buffer is ever described, which means reading a block range into
several buffers needs one command per buffer.

Build the PRDT from the scatter-gather list when the command carries
one. Each buffer contributes one or more entries, long buffers are
split as before, and the list is rejected with -EOPNOTSUPP if it
needs more entries than the PRDT holds, or if a buffer lies above the
32-bit boundary on controllers with
UFSHCD_QUIRK_BROKEN_64BIT_ADDRESS. In both cases the block uclass
falls back to reading one buffer at a time.

Cache maintenance follows the list rather than the single pdata
range: each buffer is flushed before the command is sent and
invalidated once it completes. Commands without a list keep using the
existing path. The capability is reported to the SCSI layer in
ufshcd_probe() so it only issues such commands when they can be
handled.

Signed-off-by: Urvashi Sharma <urvashi.sharma@oss.qualcomm.com>
---
 drivers/ufs/Kconfig      |  8 +++++
 drivers/ufs/ufs-uclass.c | 75 ++++++++++++++++++++++++++++++++++++++--
 2 files changed, 80 insertions(+), 3 deletions(-)

diff --git a/drivers/ufs/Kconfig b/drivers/ufs/Kconfig
index 49472933de3..df6502807c0 100644
--- a/drivers/ufs/Kconfig
+++ b/drivers/ufs/Kconfig
@@ -8,6 +8,14 @@ config UFS
 	  This selects support for Universal Flash Subsystem (UFS).
 	  Say Y here if you want UFS Support.
 
+config UFS_SCATTER_GATHER
+	bool "Scatter-gather read support for UFS"
+	depends on UFS && BLK_SCATTER_GATHER_READ
+	help
+	  Build one PRDT entry per destination buffer so that blk_read_sg()
+	  reads into all buffers with a single UFS command. Without this,
+	  blk_read_sg() falls back to one read per buffer.
+
 config UFS_AMD_VERSAL2
 	bool "AMD Versal Gen 2 UFS controller platform driver"
 	depends on UFS && ZYNQMP_FIRMWARE
diff --git a/drivers/ufs/ufs-uclass.c b/drivers/ufs/ufs-uclass.c
index 6a51f337e47..2d5efbd7db1 100644
--- a/drivers/ufs/ufs-uclass.c
+++ b/drivers/ufs/ufs-uclass.c
@@ -778,6 +778,20 @@ static void ufshcd_cache_invalidate(void *addr, unsigned long size)
 	invalidate_dcache_range(start_addr, end_addr);
 }
 
+/**
+ * ufshcd_sg_cache_invalidate - Invalidate cache for scatter-gather buffers
+ *
+ * Invalidate cache for each buffer in the list.
+ */
+static void ufshcd_sg_cache_invalidate(struct blk_sg_list *sgl)
+{
+	unsigned int i;
+
+	for (i = 0; i < sgl->count; i++)
+		ufshcd_cache_invalidate(sgl->entries[i].addr,
+					sgl->entries[i].len);
+}
+
 /**
  * ufshcd_prepare_req_desc_hdr() - Fills the requests header
  * descriptor according to request
@@ -1656,6 +1670,50 @@ static void prepare_prdt_table(struct ufs_hba *hba, struct scsi_cmd *pccb)
 	ufshcd_cache_flush(req_desc, sizeof(*req_desc));
 }
 
+static int prepare_sg_prdt_table(struct ufs_hba *hba, struct scsi_cmd *pccb)
+{
+	struct utp_transfer_req_desc *req_desc = hba->utrdl;
+	struct ufshcd_sg_entry *prd_table = hba->ucd_prdt_ptr;
+	struct blk_sg_list *sgl = pccb->sgl;
+	int table_length = 0;
+	ulong len, chunk;
+	unsigned int i;
+	u8 *buf;
+
+	for (i = 0; i < sgl->count; i++) {
+		buf = sgl->entries[i].addr;
+		len = sgl->entries[i].len;
+
+		if ((hba->quirks & UFSHCD_QUIRK_BROKEN_64BIT_ADDRESS) &&
+		    upper_32_bits((uintptr_t)buf + len - 1)) {
+			dev_dbg(hba->dev, "Buffer above 32bit boundary %p\n",
+				buf);
+			return -EOPNOTSUPP;
+		}
+
+		ufshcd_cache_flush(buf, len);
+
+		while (len) {
+			if (table_length == MAX_BUFF) {
+				dev_dbg(hba->dev, "Too many PRDT entries\n");
+				return -EOPNOTSUPP;
+			}
+
+			chunk = min_t(ulong, len, MAX_PRDT_ENTRY);
+			prepare_prdt_desc(hba, &prd_table[table_length++], buf,
+					  chunk - 1);
+			buf += chunk;
+			len -= chunk;
+		}
+	}
+
+	req_desc->prd_table_length = cpu_to_le16(table_length);
+	ufshcd_cache_flush(prd_table, sizeof(*prd_table) * table_length);
+	ufshcd_cache_flush(req_desc, sizeof(*req_desc));
+
+	return 0;
+}
+
 static int ufs_scsi_exec(struct udevice *scsi_dev, struct scsi_cmd *pccb)
 {
 	struct ufs_hba *hba = dev_get_uclass_priv(scsi_dev->parent);
@@ -1665,13 +1723,23 @@ static int ufs_scsi_exec(struct udevice *scsi_dev, struct scsi_cmd *pccb)
 
 	ufshcd_prepare_req_desc_hdr(hba, &upiu_flags, pccb->dma_dir);
 	ufshcd_prepare_utp_scsi_cmd_upiu(hba, pccb, upiu_flags);
-	prepare_prdt_table(hba, pccb);
 
-	ufshcd_cache_flush(pccb->pdata, pccb->datalen);
+	if (IS_ENABLED(CONFIG_UFS_SCATTER_GATHER) && pccb->sgl) {
+		int ret = prepare_sg_prdt_table(hba, pccb);
+
+		if (ret)
+			return ret;
+	} else {
+		prepare_prdt_table(hba, pccb);
+		ufshcd_cache_flush(pccb->pdata, pccb->datalen);
+	}
 
 	ufshcd_send_command(hba, TASK_TAG);
 
-	ufshcd_cache_invalidate(pccb->pdata, pccb->datalen);
+	if (IS_ENABLED(CONFIG_UFS_SCATTER_GATHER) && pccb->sgl)
+		ufshcd_sg_cache_invalidate(pccb->sgl);
+	else
+		ufshcd_cache_invalidate(pccb->pdata, pccb->datalen);
 
 	ocs = ufshcd_get_tr_ocs(hba);
 	switch (ocs) {
@@ -2189,6 +2257,7 @@ int ufshcd_probe(struct udevice *ufs_dev, struct ufs_hba_ops *hba_ops)
 	scsi_plat->max_id = UFSHCD_MAX_ID;
 	scsi_plat->max_lun = UFS_MAX_LUNS;
 	scsi_plat->max_bytes_per_req = UFS_MAX_BYTES;
+	scsi_plat->sg_supported = IS_ENABLED(CONFIG_UFS_SCATTER_GATHER);
 
 	hba->dev = ufs_dev;
 	hba->ops = hba_ops;
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-11-14  4:06 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-10 22:52 [RFC] blk/scsi/ufs: scatter-gather read support Urvashi Sharma
2026-11-11  8:14 ` neil.armstrong
2026-11-12  2:19   ` Urvashi Sharma
2026-11-14  4:05 ` [RFC PATCH 0/3] " Urvashi Sharma
2026-11-14  4:05   ` [RFC PATCH 1/3] blk: add " Urvashi Sharma
2026-11-14  4:05   ` [RFC PATCH 2/3] scsi: " Urvashi Sharma
2026-11-14  4:05   ` [RFC PATCH 3/3] ufs: " Urvashi Sharma

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.