Skip to contentSkip to conversation

blk/scsi/ufs: scatter-gather read support

RFC Urvashi Sharma 2 participants 2 messages Nov 10 – Nov 11

lore
Share & tools
lore
#0 lore raw

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/[email protected]/msg04243.html

#1 lore raw

Hi,

Quoted 48 lines · Urvashi Sharma

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.

Quoted 7 lines

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 ?

Quoted 6 lines
	};

	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.

Quoted 32 lines

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.

Quoted 4 lines

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

Quoted 5 lines

Thanks,
Urvashi

[1] https://www.mail-archive.com/[email protected]/msg04243.html