From nobody Thu Feb 20 18:04:29 2025 X-Original-To: freebsd-hackers@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4YzLkg70Yyz5pDbw for ; Thu, 20 Feb 2025 18:04:43 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Received: from mail-pj1-x102b.google.com (mail-pj1-x102b.google.com [IPv6:2607:f8b0:4864:20::102b]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "smtp.gmail.com", Issuer "WR4" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4YzLkf4Vgvz3H6B for ; Thu, 20 Feb 2025 18:04:42 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Authentication-Results: mx1.freebsd.org; dkim=pass header.d=bsdimp-com.20230601.gappssmtp.com header.s=20230601 header.b=141Agg5G; dmarc=none; spf=none (mx1.freebsd.org: domain of wlosh@bsdimp.com has no SPF policy when checking 2607:f8b0:4864:20::102b) smtp.mailfrom=wlosh@bsdimp.com Received: by mail-pj1-x102b.google.com with SMTP id 98e67ed59e1d1-2fa5af6d743so2034164a91.3 for ; Thu, 20 Feb 2025 10:04:42 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bsdimp-com.20230601.gappssmtp.com; s=20230601; t=1740074681; x=1740679481; darn=freebsd.org; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=2grrSKsvhAU6momNHGHLIkbgcVMuwEyCZlLL95VxiqE=; b=141Agg5G2kmyjSLYz+yLsQQknGR1914f3YCva1D+nubahF16YbRrXIVIrRVASta7cE Nkj/oxr5m/Ln8hqXo3boOsyKvX/a0j9d0fH9ZDj60vJQ0JKX9gcWYCNifw0qwdJmj854 EiQvvWSOjt1SRjTXdo59EhZG6V7clTpwalyBfjFO+DE8Guv+fwSPhzA4+0jlnkZLF2K6 dCyoowOcfXB0Hru7yB2XEOnE/+2q6bW9mdsZpPy8p1ihW3NSyzbm7YPjuebD5Pl/WITm AnvYfSy0EbL07+jK5DmkNMy2v5r2qNGhTCbmf49/0igMQiC3FZ3FLMWV5ElnDtaOfoB7 uEFg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1740074681; x=1740679481; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=2grrSKsvhAU6momNHGHLIkbgcVMuwEyCZlLL95VxiqE=; b=iTdyqs82SekYdxE+/qj1cAWKUkyOh1Sca2NoZmfcSiz6Ep6fgQS5wGXYALsaTznC1M njx4BkQMhnWOFhXj/QBWeIFxv/kqAlCAYTkIXfempEItYZryPG4nMxLY5ntX61R/sYAU BCR351EBWcZ6IGi3saP+Qs1WokhoDXjcIx7ukFUI3S676GmOyjzuk3H48as8DlsRDvuJ jKqNqjFGeEl8fD1UYlMnew1SR4e4mGi7rZ+xqKyRrdySmN/RVGeXBye6iK/i3QlS+JGg SPXEXY6+FsW2KHGIqov4cmq2ePSf23o9s1Api4h8F8jxSV6LErAZDp8XdMIpO80D6ITD v8Hg== X-Gm-Message-State: AOJu0Yz4n9FHE5jGuXKLjcdATbceEFY9b5ni62uVnmaaGTfg6ihsYytk kXiW0gHzzLQSe18nAreJ3vnZ3VPs457Qg+XKeb8xH9YkPEVp9eLroRYv4oxGNdlQBSASJHFwBj3 EzRpLvKwbwmnpN1zRhh4/xfz/ow0qacqd7a/PrYVeYOqBjUibh48= X-Gm-Gg: ASbGnctR7F3JJ6BINyQWvkWPg5/bKF7w2rflgI55Hnk7DtanG0r7jICnlkWoJKarc6D kAXeMRIEAal3gXkR8gPhUVOH3w8JcyGrJZxWBpmKQOI7Chy3eGBfKolJUc4I5BNNyfqZTA/bV X-Google-Smtp-Source: AGHT+IE7aZt0N3PBY9iNZjILgcpmZXzAn197WkzLn1XQFFY78vZpHSq8qi8z2hpp6XhozPBKpui3x+h61UkAWIIidWI= X-Received: by 2002:a17:90a:d60b:b0:2ee:94d1:7a9d with SMTP id 98e67ed59e1d1-2fce7b3df74mr101279a91.32.1740074681062; Thu, 20 Feb 2025 10:04:41 -0800 (PST) List-Id: Technical discussions relating to FreeBSD List-Archive: https://lists.freebsd.org/archives/freebsd-hackers List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-freebsd-hackers@FreeBSD.org MIME-Version: 1.0 References: <20250117045346epcms2p61099ea651d3ca3f00e3134dd00e6a9ac@epcms2p6> <20250121002602epcms2p6067990e98b8143ff596d90af0ec54492@epcms2p6> <20250123024207epcms2p1782d2ff74077c1f215fbdb0ae5f3aa09@epcms2p1> In-Reply-To: <20250123024207epcms2p1782d2ff74077c1f215fbdb0ae5f3aa09@epcms2p1> From: Warner Losh Date: Thu, 20 Feb 2025 11:04:29 -0700 X-Gm-Features: AWEUYZkUjYQQ8jRR7jqaIsWOxGQTPFL0S9xZXP951rh-70ZxJPd70aO6xkXhE24 Message-ID: Subject: Re: Universal Flash Storage Driver Proposal To: j_yoon.choi@samsung.com Cc: "freebsd-hackers@FreeBSD.org" Content-Type: multipart/alternative; boundary="0000000000003f717d062e96b556" X-Spamd-Result: default: False [-2.68 / 15.00]; NEURAL_HAM_MEDIUM(-1.00)[-1.000]; NEURAL_HAM_LONG(-1.00)[-1.000]; NEURAL_HAM_SHORT(-0.68)[-0.685]; FORGED_SENDER(0.30)[imp@bsdimp.com,wlosh@bsdimp.com]; R_DKIM_ALLOW(-0.20)[bsdimp-com.20230601.gappssmtp.com:s=20230601]; MIME_GOOD(-0.10)[multipart/alternative,text/plain]; TO_DN_EQ_ADDR_SOME(0.00)[]; ASN(0.00)[asn:15169, ipnet:2607:f8b0::/32, country:US]; R_SPF_NA(0.00)[no SPF record]; RCVD_COUNT_ONE(0.00)[1]; MISSING_XM_UA(0.00)[]; MIME_TRACE(0.00)[0:+,1:+,2:~]; DMARC_NA(0.00)[bsdimp.com]; RCPT_COUNT_TWO(0.00)[2]; MLMMJ_DEST(0.00)[freebsd-hackers@freebsd.org]; FROM_NEQ_ENVFROM(0.00)[imp@bsdimp.com,wlosh@bsdimp.com]; FROM_HAS_DN(0.00)[]; RCVD_IN_DNSWL_NONE(0.00)[2607:f8b0:4864:20::102b:from]; TO_MATCH_ENVRCPT_SOME(0.00)[]; PREVIOUSLY_DELIVERED(0.00)[freebsd-hackers@freebsd.org]; RCVD_TLS_LAST(0.00)[]; ARC_NA(0.00)[]; DKIM_TRACE(0.00)[bsdimp-com.20230601.gappssmtp.com:+] X-Rspamd-Queue-Id: 4YzLkf4Vgvz3H6B X-Spamd-Bar: -- --0000000000003f717d062e96b556 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Hey sorry for taking a while to reply... On Wed, Jan 22, 2025 at 7:42=E2=80=AFPM Jaeyoon Choi wrote: > > > Generally, yes. However, looking at umass, you'll see that different > kinds of USB > > thumb drives have different subsets of SCSI that are supported. For > example, RBC > > is a reduced subset of commands, and umass filters things so that we > have the right > > length (only READ(10) and WRITE(10) are supported for RBC, for example)= . > I'd image > > there'd be a similar list for UFS and you'd need to arrange for scsi_da > to only generate > > those. It looks like some kind of quirk info is going on to force this, > but I've not puzzled > > through all the layers to find it. umass also does some minor CDB > rewriting for things > > like ATAPI that are close. UFI also has a different subset that's > supported. > > Thanks for letting me know, I will study the scsi_da code more. > > > Right now that's all handled by the SIM setting cpi.hba_misc |=3D > PIM_NO_6_BYTE for > > the CAM_PATH_INQ xpt request. You may need to create a PIM_NO_10_BYTE i= f > > only the 12 byte variants are supported, or find some other way to > signal to scsi_da that > > it must choose a different way to generate commands (if say a single bi= t > isn't enough, > > or it turns into too much of a specialty quirk flag) > > There are three restrictions that I found in UFS spec (MODE SENSE(6), > READ(12) > and WRITE(12)), > I think adding the following two flags to cpi.hba_misc should be enough: > - PIM_NO_12_BYTE > - PIM_NO_6_BYTE_MODE_SENSE > But I'll take a look at the UFS spec documentation to see if there are mo= re > restrictions needed. > I've been studying UMASS and have a different idea. I'm starting to not like the PIM flags now that I've read it. There's about a dozen commands that different USB flash devices can't do in various ways. I've started to convert umass to just fail the command as illegal when a bad command is sent to it. I've also been modifying the da, cd, etc drivers to notice the illegal commands and modify its behavior. It isn't a new idea: the periph drivers have been doing it to cope with drives that don't support READ(6) commands. It's recently (in the last 5 years) expanded to dealing with other commands like SYNCHRONIZE CACHE, MODE SENSE. The idea is to allow the device to reject the command as illegal. In the cases of reduced SCSI command sets, like RBC, UFS, etc the SIM should reject known bad commands at the SIM layer because history has shown that some of the lower-quality-but-still-working-enough drives can hang when unexpected commands are sent to them. Not supporting MODE SENSE(6) likely is going to be a somewhat larger change= . > Though having said that, I kinda think > > we can retire all the 6-byte command support: we don't need it on > anything modern and > > it really is there for the pre-scsi-1 SASI drives that we don't > support[*]. We may also want > > to rework the number of special cases we have in scsi_da.c as well. > There's a number > > of quirks that either go away with no-6-byte commands, or that can be > guessed w/o a loud > > message to the user (which is why we avoid sending them with the quirk, > though some > > of that avoidance is because some early drives hung). > > I don't know if this helps, but Fuchsia OS has decided not to support > READ(6)/WRITE(6) commands. > (Fuchsia's SCSI layer is still in the early stages of development) > - > https://fuchsia-review.googlesource.com/c/fuchsia/+/932832/comments/72826= c28_c48b071d?tab=3Dcomments Yes. Once upon a time, there was limited space for CDBs in HBAs. So it made a lot of sense to send the smallest CDB possible to the SIM for that HBA. Those days were in the 90s and early 2000s. We have a few hundred devices that are quirked to not use 6-byte commands, so I think that's a wise decision. > So the SCSI protocol likely can still be used, but some adjustments might > be needed > > to the SCSI transport (though the distinction between the two can get > rather confused at > > times). Eg, I think eventually you'll find you'll want a XPORT_UFS to > sit along side > > XPORT_FC, XPORT_SPI, XPORT_SAS, etc. I think the protocol will be > PROTO_SCSI > > for everything. Each of the transports can and do report slightly > different things in handled > > by XPT_GET_TRAN_SETTINGS and XPT_SET_TRAN_SETTINGS messages. > > I was confusing SCSI protocol and SCSI transport. Thanks for the > clarification. > I agree with you that we should add XPORT_UFS. I will study it more and > share > my design. > > > I hope my advice is useful. Also, although I've given talks on this > topic, you should double > > check anything I said in those talks or that I say here. (a) It will > help you learn CAM better > > and (b) some of these details I only think I know and understand, but > I'm missing something > > subtle (if I knew which points, I'd say :). > > > > Good luck with everything, and don't hesitate to ask if you have > questions, want some early > > code review, design advice, etc. > > I was confused and your advice cleared up a lot of my questions. > I will definitely check out your talks again. > > Here are a few questions that come to mind: > 1. i'm trying to take the overall structure similar to NVMe, is that okay= ? > (nvme_controller, nvme_namespace, nvme_qpair, nvme_request, > nvme_tracker, > etc.) > For the UFS HBA device, I think that's a good approach. Most devices these days have tried to de-couple request submission and request completion. While our NVME driver may have some odd quirks in it around these concepts due to early NVMe drives falling short of the ideal, I think it's a good model to follow. > 2. is there any tool or test I can try to test the block device while I'm > developing, I'm curious to know how you are testing it. > Test how? I'm not sure I understand this question well enough to answer, so maybe a few examples will help me focus an answer. > FYI, I'll be out of the office for 5 weeks starting in February, so I'll > probably start development in mid-March. > Perhaps I can ask more detailed questions at that time. > Excellent. This should be waiting for you when you return in a couple of weeks. I hadn't read to the end to know you'd be out, or I would have tried harder to send this sooner (work has been busy). In the mean time, I'll see if I can locate a copy of the UFS standard or a reasonable summary. The ones at jedec.org are a bit too expensive for me to buy on my own. I've found excerpts of it datasheets at best, whic= h may suffice for my review needs. Warner --0000000000003f717d062e96b556 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hey sorry for taking a while to reply...<= /div>
On Wed, Jan 22, 2025 at 7:42=E2=80=AFPM Jaeyoon Choi &= lt;j_yoon.choi@samsung.com&g= t; wrote:
=C2=A0=
> Generally, yes. However, looking at umass, you'll see that differe= nt kinds of USB
> thumb drives have different subsets of SCSI that are supported. For ex= ample, RBC
> is a reduced subset of commands, and umass filters things so that we h= ave the right
> length (only READ(10) and WRITE(10) are supported for RBC, for example= ). I'd image
> there'd be a similar list for UFS and you'd need to arrange fo= r scsi_da to only generate
> those. It looks like some kind of quirk info is going on to force this= , but I've not puzzled
> through all the layers to find it. umass also does some minor CDB rewr= iting for things
> like ATAPI that are close. UFI also has a different subset that's = supported.

Thanks for letting me know, I will study the scsi_da code more.

> Right now that's all handled by the SIM setting cpi.hba_misc |=3D = PIM_NO_6_BYTE for
> the CAM_PATH_INQ xpt request. You may need to create a PIM_NO_10_BYTE = if
> only the 12 byte variants are supported, or find some other way to sig= nal to scsi_da that
> it must choose a different way to generate commands (if say a single b= it isn't enough,
> or it turns into too much of a specialty quirk flag)

There are three restrictions that I found in UFS spec (MODE SENSE(6), READ(= 12)
and WRITE(12)),
I think adding the following two flags to cpi.hba_misc should be enough: - PIM_NO_12_BYTE
- PIM_NO_6_BYTE_MODE_SENSE
But I'll take a look at the UFS spec documentation to see if there are = more
restrictions needed.

I've been stud= ying UMASS and have a different idea. I'm starting to not like the PIM = flags
now that I've read it.

There&#= 39;s about a dozen commands that different USB flash devices can't do i= n various ways.
I've started to convert umass to just fail th= e command as illegal when a bad command is
sent to it. I've a= lso been modifying the da, cd, etc drivers to notice the illegal commands a= nd
modify its behavior. It isn't a new idea: the periph drive= rs have been doing it to cope with
drives that don't support = READ(6) commands. It's recently (in the last 5 years) expanded to
=
dealing with other commands like SYNCHRONIZE CACHE, MODE SENSE.
<= div>
The idea is to allow the device to reject the command as= illegal. In the cases of reduced SCSI
command sets, like RBC, UF= S, etc the SIM should reject known bad commands at the SIM
layer = because history has shown that some of the lower-quality-but-still-working-= enough drives
can hang when unexpected commands are sent to them.=

Not supporting MODE SENSE(6) likely is going to b= e a somewhat larger change.

> Though having said that, I kinda think
> we can retire all the 6-byte command support: we don't need it on = anything modern and
> it really is there for the pre-scsi-1 SASI drives that we don't su= pport[*]. We may also want
> to rework the number of special cases we have in scsi_da.c as well. Th= ere's a number
> of quirks that either go away with no-6-byte commands, or that can be = guessed w/o a loud
> message to the user (which is why we avoid sending them with the quirk= , though some
> of that avoidance is because some early drives hung).=C2=A0

I don't know if this helps, but Fuchsia OS has decided not to support READ(6)/WRITE(6) commands.
(Fuchsia's SCSI layer is still in the early stages of development)
- https://fuchsia-review.googlesource.com/c/fuchsia/+/932832/comments/72826= c28_c48b071d?tab=3Dcomments

Yes. Once u= pon a time, there was limited space for CDBs in HBAs. So it made a lot of
sense to send the smallest CDB possible to the SIM for that HBA. T= hose days were in the
90s and early 2000s. We have a few hundred = devices that are quirked to not use 6-byte
commands, so I think t= hat's a wise decision.

> So the SCSI protocol likely can still be used, but some adjustments mi= ght be needed
> to the SCSI transport (though the distinction between the two can get = rather confused at
> times).=C2=A0 Eg, I think eventually you'll find you'll want a= XPORT_UFS to sit along side
> XPORT_FC, XPORT_SPI, XPORT_SAS, etc. I think the protocol will be PROT= O_SCSI
> for everything. Each of the transports can and do report slightly diff= erent things in handled
> by XPT_GET_TRAN_SETTINGS and XPT_SET_TRAN_SETTINGS messages.

I was confusing SCSI protocol and SCSI transport. Thanks for the clarificat= ion.
I agree with you that we should add XPORT_UFS. I will study it more and sha= re
my design.

> I hope my advice is useful. Also, although I've given talks on thi= s topic, you should double
> check anything I said in those talks or that I say here. (a) It will h= elp you learn CAM better
> and (b) some of these details I only think I know and understand, but = I'm missing something
> subtle (if I knew which points, I'd say :).
>
> Good luck with everything, and don't hesitate to ask if you have q= uestions, want some early
> code review, design advice, etc.

I was confused and your advice cleared up a lot of my questions.
I will definitely check out your talks again.

Here are a few questions that come to mind:
1. i'm trying to take the overall structure similar to NVMe, is that ok= ay?
=C2=A0 =C2=A0(nvme_controller, nvme_namespace, nvme_qpair, nvme_request, nv= me_tracker,
=C2=A0 =C2=A0etc.)

For the UFS HBA devi= ce, I think that's a good approach. Most devices these
days h= ave tried to de-couple request submission and request completion. While
our NVME driver may have some odd quirks in it around these concepts= due to
early NVMe drives falling short of the ideal, I think it&= #39;s a good model to follow.
=C2=A0
2. is there any tool or test I can try to test the block device while I'= ;m
=C2=A0 =C2=A0developing, I'm curious to know how you are testing it.

Test how? I'm not sure I understand t= his question well enough to answer,
so maybe a few examples will = help me focus an answer.
=C2=A0
FYI, I'll be out of the office for 5 weeks starting in February, so I&#= 39;ll
probably start development in mid-March.
Perhaps I can ask more detailed questions at that time.

Excellent. This should be waiting for you when you return = in a couple
of weeks. I hadn't read to the end to know you= 9;d be out, or I would have
tried harder to send this sooner (wor= k has been busy).=C2=A0

In the mean time, I'll= see if I can locate a copy of the UFS standard
or a reasonable s= ummary. The ones at jedec.org are a bit to= o expensive
for me to buy on my own. I've found excerpts of i= t datasheets at best, which
may suffice for my review needs.

Warner
--0000000000003f717d062e96b556--