Page MenuHomeFreeBSD

cam/scsi, ufshci: Add Well-known LUN Probing
AbandonedPublic

Authored by jaeyoon on Oct 27 2025, 6:59 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Nov 30, 7:52 AM
Unknown Object (File)
Fri, Nov 28, 3:08 AM
Unknown Object (File)
Sun, Nov 16, 11:51 AM
Unknown Object (File)
Thu, Nov 13, 6:08 PM
Unknown Object (File)
Mon, Nov 3, 11:44 AM
Unknown Object (File)
Mon, Nov 3, 11:43 AM
Unknown Object (File)
Mon, Nov 3, 11:41 AM
Unknown Object (File)
Mon, Nov 3, 11:41 AM
Subscribers

Details

Reviewers
slm
imp
Group Reviewers
cam
Summary

Add well-known lun probing to CAM and register it as a
pass-through peripheral device. The ufshci driver finds
the “UFS device” wlun and registers it.

This patch contains 5 commits.

Sponsored by: Samsung Electronic

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 68855
Build 65738: arc lint + arc unit

Event Timeline

jaeyoon added a reviewer: imp.

I will request a review for each commit individually to make reviewing easier.

This patch is ready for the review!

Just a few questions. I started this review a few weeks ago then it slipped my mind...

sys/cam/cam_xpt_internal.h
174

Is there a reason you can't reuse luns_mtx to protect both of these lists?

sys/cam/scsi/scsi_all.c
9004

You can likely always |= this in w/o the if. The other boolean request different specific bits, but this specifies them.

sys/cam/scsi/scsi_xpt.c
1260

this looks like it might have a formatting issue

1421–1423

you have is_wlun defined above, but then expand it here. Maybe it would be better to use that here too. Unless I'm missing something.

2035–2036

formatting not (copied from the original): we usually don't have a space after sizeof in this construct.

2237

this looks like a formatting mistake. The '*' here should line up with the '*' on the line above.

2243

same.

2246

the old formatting was correct. This seems odd to my eye, though CAM's formatting is a lot less consistent than other parts of the kernel.

sys/dev/ufshci/ufshci_private.h
275

Doesn't qemu report there's no WLUNs? Or does it say it does and then implement it badly.

sys/dev/ufshci/ufshci_sim.c
449

I'm not sure I like the busy-wait here. Is there no way to make this interrupt driven?

467

I don't understand how this works or what it's supposed to accomplish. Can you explain?

Oh, you left a review here. I'll reopen it.

Thanks for the review~!

sys/cam/cam_xpt_internal.h
174

I also considered this, but it's okay to reuse luns_mtx. I removed it.

sys/cam/scsi/scsi_xpt.c
1421–1423

I missed it, thank you!

2246

My VSCode seems to have trouble with CAM's formatting. I'll pay more attention. Thanks!

sys/dev/ufshci/ufshci_private.h
275

QEMU does not have a WLUN and returns an error during WLUN probe. Therefore, unnecessary SCSI command errors are printed. Therefore, I added a quirk to prevent WLUN probe.

ufshci: SCSI command completion error, Status(0x2) Key(0x5), ASC(0x24), ASCQ(0x0)
(probe0:ufshci0:0:0:0): REPORT LUNS. CDB: a0 00 01 00 00 00 00 00 00 10 00 00
(probe0:ufshci0:0:0:0): CAM status: CCB request completed with an error
(probe0:ufshci0:0:0:0): Retrying command, 4 more tries remain
ufshci0: Function(0x0) Invalid response code = 0x1
ufshci: SCSI command completion error, Status(0x2) Key(0x5), ASC(0x24), ASCQ(0x0)
(probe0:ufshci0:0:0:0): REPORT LUNS. CDB: a0 00 01 00 00 00 00 00 00 10 00 00
(probe0:ufshci0:0:0:0): CAM status: CCB request completed with an error
(probe0:ufshci0:0:0:0): Retrying command, 3 more tries remain
ufshci0: Function(0x0) Invalid response code = 0x1
ufshci: SCSI command completion error, Status(0x2) Key(0x5), ASC(0x24), ASCQ(0x0)
(probe0:ufshci0:0:0:0): REPORT LUNS. CDB: a0 00 01 00 00 00 00 00 00 10 00 00
(probe0:ufshci0:0:0:0): CAM status: CCB request completed with an error
(probe0:ufshci0:0:0:0): Retrying command, 2 more tries remain
ufshci0: Function(0x0) Invalid response code = 0x1
ufshci: SCSI command completion error, Status(0x2) Key(0x5), ASC(0x24), ASCQ(0x0)
(probe0:ufshci0:0:0:0): REPORT LUNS. CDB: a0 00 01 00 00 00 00 00 00 10 00 00
(probe0:ufshci0:0:0:0): CAM status: CCB request completed with an error
(probe0:ufshci0:0:0:0): Retrying command, 1 more tries remain
ufshci0: Function(0x0) Invalid response code = 0x1
ufshci: SCSI command completion error, Status(0x2) Key(0x5), ASC(0x24), ASCQ(0x0)
(probe0:ufshci0:0:0:0): REPORT LUNS. CDB: a0 00 01 00 00 00 00 00 00 10 00 00
(probe0:ufshci0:0:0:0): CAM status: CCB request completed with an error
(probe0:ufshci0:0:0:0): Retrying command, 0 more tries remain
ufshci0: Function(0x0) Invalid response code = 0x1
ufshci: SCSI command completion error, Status(0x2) Key(0x5), ASC(0x24), ASCQ(0x0)
(probe0:ufshci0:0:0:0): REPORT LUNS. CDB: a0 00 01 00 00 00 00 00 00 10 00 00
(probe0:ufshci0:0:0:0): CAM status: CCB request completed with an error
(probe0:ufshci0:0:0:0): Error 5, Retries exhausted
ufshci0: Function(0x0) Invalid response code = 0x1
sys/dev/ufshci/ufshci_sim.c
449

How about using pause_sbt()?

467

The purpose is to increase the reference count of periph so that it is not released while sending the SSU command. Am I misunderstanding?
Is it okay not to worry about the reference count here?

jaeyoon marked 4 inline comments as done.
  • cam/scsi: Support well known logical unit
  • ufshci: Enable WLUN scan
  • cam/scsi: Add power condition support to START STOP UNIT
  • ufshci: Add a check for WLUN during driver initialization

Reopen until the current comment is resolved.

OK. I understand adding the quirk. It may make sense for us to handle some of these 'probing' commands better in the future, but we have many others than the ones you've added. As such, lets go wit the the PIM_WLUN flag for now.

In D53375#1233119, @imp wrote:

OK. I understand adding the quirk. It may make sense for us to handle some of these 'probing' commands better in the future, but we have many others than the ones you've added. As such, lets go wit the the PIM_WLUN flag for now.

Thanks for understanding!