| Message ID | 20220110063030.12957-1-quic_jackp@quicinc.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2] usb: gadget: f_mass_storage: Make CD-ROM emulation work with Mac OS-X | expand |
On Sun, Jan 09, 2022 at 10:30:30PM -0800, Jack Pham wrote: > From: Roger Quadros <roger.quadros@nokia.com> > > Mac OS-X expects CD-ROM TOC in raw format (i.e. format:2). It also > sends the READ_TOC CDB in old style SFF8020i format. i.e. 2 format bits > are encoded in MSBs of CDB byte 9. > > This patch will enable CD-ROM emulation to work with Mac OS-X. Tested on > Mac OS X v10.6.3. > > Signed-off-by: Roger Quadros <roger.quadros@nokia.com> > Signed-off-by: Jack Pham <quic_jackp@quicinc.com> > --- > v2: Removed Change-Id tag. > > drivers/usb/gadget/function/f_mass_storage.c | 73 +++++++++++++++++++++++----- > 1 file changed, 61 insertions(+), 12 deletions(-) > > diff --git a/drivers/usb/gadget/function/f_mass_storage.c b/drivers/usb/gadget/function/f_mass_storage.c > index 73a28f8..1f7f4dd6 100644 > --- a/drivers/usb/gadget/function/f_mass_storage.c > +++ b/drivers/usb/gadget/function/f_mass_storage.c > @@ -1188,6 +1188,8 @@ static int do_read_toc(struct fsg_common *common, struct fsg_buffhd *bh) > int msf = common->cmnd[1] & 0x02; > int start_track = common->cmnd[6]; > u8 *buf = (u8 *)bh->buf; > + u8 format; > + int i, len; > > if ((common->cmnd[1] & ~0x02) != 0 || /* Mask away MSF */ > start_track > 1) { > @@ -1195,18 +1197,65 @@ static int do_read_toc(struct fsg_common *common, struct fsg_buffhd *bh) > return -EINVAL; > } > > - memset(buf, 0, 20); > - buf[1] = (20-2); /* TOC data length */ > - buf[2] = 1; /* First track number */ > - buf[3] = 1; /* Last track number */ > - buf[5] = 0x16; /* Data track, copying allowed */ > - buf[6] = 0x01; /* Only track is number 1 */ > - store_cdrom_address(&buf[8], msf, 0); > + format = common->cmnd[2] & 0xf; Hmmm. According to this part later on: > @@ -1933,7 +1982,7 @@ static int do_scsi_command(struct fsg_common *common) > common->data_size_from_cmnd = > get_unaligned_be16(&common->cmnd[7]); > reply = check_command(common, 10, DATA_DIR_TO_HOST, > - (7<<6) | (1<<1), 1, > + (0xf<<6) | (1<<1), 1, > "READ TOC"); common->cmnd[2] can never be anything other than 0. So this computation and the test immediately below are pointless -- unless you change the argument to check_command(). Alan Stern > + /* > + * Check if CDB is old style SFF-8020i > + * i.e. format is in 2 MSBs of byte 9 > + * Mac OS-X host sends us this. > + */ > + if (format == 0) > + format = (common->cmnd[9] >> 6) & 0x3; > + > + switch (format) { > + case 0: > + /* Formatted TOC */ > + len = 4 + 2*8; /* 4 byte header + 2 descriptors */ > + memset(buf, 0, len); > + len -= 2; /* TOC Length excludes length field */ > + buf[1] = len; /* TOC data length */ > + buf[2] = 1; /* First track number */ > + buf[3] = 1; /* Last track number */ > + buf[5] = 0x16; /* Data track, copying allowed */ > + buf[6] = 0x01; /* Only track is number 1 */ > + store_cdrom_address(&buf[8], msf, 0); > + > + buf[13] = 0x16; /* Lead-out track is data */ > + buf[14] = 0xAA; /* Lead-out track number */ > + store_cdrom_address(&buf[16], msf, curlun->num_sectors); > + return len; > + > + case 2: > + /* Raw TOC */ > + len = 4 + 3*11; /* 4 byte header + 3 descriptors */ > + memset(buf, 0, len); /* Header + A0, A1 & A2 descriptors */ > + len -= 2; /* TOC Length excludes length field */ > + buf[1] = len; /* TOC data length */ > + buf[2] = 1; /* First complete session */ > + buf[3] = 1; /* Last complete session */ > + > + buf += 4; > + /* fill in A0, A1 and A2 points */ > + for (i = 0; i < 3; i++) { > + buf[0] = 1; /* Session number */ > + buf[1] = 0x16; /* Data track, copying allowed */ > + /* 2 - Track number 0 -> TOC */ > + buf[3] = 0xA0 + i; /* A0, A1, A2 point */ > + /* 4, 5, 6 - Min, sec, frame is zero */ > + buf[8] = 1; /* Pmin: last track number */ > + buf += 11; /* go to next track descriptor */ > + } > + buf -= 11; /* go back to A2 descriptor */ > + > + /* For A2, 7, 8, 9, 10 - zero, Pmin, Psec, Pframe of Lead out */ > + store_cdrom_address(&buf[7], msf, curlun->num_sectors); > > - buf[13] = 0x16; /* Lead-out track is data */ > - buf[14] = 0xAA; /* Lead-out track number */ > - store_cdrom_address(&buf[16], msf, curlun->num_sectors); > - return 20; > + return len; > + > + default: > + /* Multi-session, PMA, ATIP, CD-TEXT not supported/required */ > + curlun->sense_data = SS_INVALID_FIELD_IN_CDB; > + return -EINVAL; > + } > } > > static int do_mode_sense(struct fsg_common *common, struct fsg_buffhd *bh) > @@ -1933,7 +1982,7 @@ static int do_scsi_command(struct fsg_common *common) > common->data_size_from_cmnd = > get_unaligned_be16(&common->cmnd[7]); > reply = check_command(common, 10, DATA_DIR_TO_HOST, > - (7<<6) | (1<<1), 1, > + (0xf<<6) | (1<<1), 1, > "READ TOC"); > if (reply == 0) > reply = do_read_toc(common, bh); > -- > 2.7.4 >
On Wed, Jan 12, 2022 at 09:22:53PM -0800, Jack Pham wrote: > Hi Alan, > > On Wed, Jan 12, 2022 at 03:05:35PM -0500, Alan Stern wrote: > > On Sun, Jan 09, 2022 at 10:30:30PM -0800, Jack Pham wrote: > > > From: Roger Quadros <roger.quadros@nokia.com> > > > > > > Mac OS-X expects CD-ROM TOC in raw format (i.e. format:2). It also > > > sends the READ_TOC CDB in old style SFF8020i format. i.e. 2 format bits > > > are encoded in MSBs of CDB byte 9. > > > > > > This patch will enable CD-ROM emulation to work with Mac OS-X. Tested on > > > Mac OS X v10.6.3. > > > > > > Signed-off-by: Roger Quadros <roger.quadros@nokia.com> > > > Signed-off-by: Jack Pham <quic_jackp@quicinc.com> > > > --- > > > v2: Removed Change-Id tag. > > > > > > drivers/usb/gadget/function/f_mass_storage.c | 73 +++++++++++++++++++++++----- > > > 1 file changed, 61 insertions(+), 12 deletions(-) > > > > > > diff --git a/drivers/usb/gadget/function/f_mass_storage.c b/drivers/usb/gadget/function/f_mass_storage.c > > > index 73a28f8..1f7f4dd6 100644 > > > --- a/drivers/usb/gadget/function/f_mass_storage.c > > > +++ b/drivers/usb/gadget/function/f_mass_storage.c > > > @@ -1188,6 +1188,8 @@ static int do_read_toc(struct fsg_common *common, struct fsg_buffhd *bh) > > > int msf = common->cmnd[1] & 0x02; > > > int start_track = common->cmnd[6]; > > > u8 *buf = (u8 *)bh->buf; > > > + u8 format; > > > + int i, len; > > > > > > if ((common->cmnd[1] & ~0x02) != 0 || /* Mask away MSF */ > > > start_track > 1) { > > > @@ -1195,18 +1197,65 @@ static int do_read_toc(struct fsg_common *common, struct fsg_buffhd *bh) > > > return -EINVAL; > > > } > > > > > > - memset(buf, 0, 20); > > > - buf[1] = (20-2); /* TOC data length */ > > > - buf[2] = 1; /* First track number */ > > > - buf[3] = 1; /* Last track number */ > > > - buf[5] = 0x16; /* Data track, copying allowed */ > > > - buf[6] = 0x01; /* Only track is number 1 */ > > > - store_cdrom_address(&buf[8], msf, 0); > > > + format = common->cmnd[2] & 0xf; > > > > Hmmm. According to this part later on: > > > > > @@ -1933,7 +1982,7 @@ static int do_scsi_command(struct fsg_common *common) > > > common->data_size_from_cmnd = > > > get_unaligned_be16(&common->cmnd[7]); > > > reply = check_command(common, 10, DATA_DIR_TO_HOST, > > > - (7<<6) | (1<<1), 1, > > > + (0xf<<6) | (1<<1), 1, > > > "READ TOC"); > > > > common->cmnd[2] can never be anything other than 0. > > Ah, that is true. So to allow for cmnd[2] (as well as cmnd[9] as > intended by the patch) to be non-zero, the "mask" argument should rather > be: > > (0xf<<6) | (3<<1) Indeed. > In other words a bitmask of 0x3c6, corresponding to command data bytes > 1, 2, 6, 7, 8 and 9. Is my understanding correct? Yes, that's right. > > So this computation and the test immediately below are pointless -- > > unless you change the argument to check_command(). > > If you are referring to the "if (format == 0)" check, then I believe you > are right. > > > > + /* > > > + * Check if CDB is old style SFF-8020i > > > + * i.e. format is in 2 MSBs of byte 9 > > > + * Mac OS-X host sends us this. > > > + */ > > > + if (format == 0) > > > + format = (common->cmnd[9] >> 6) & 0x3; > > It seems this is the gist of the patch. Without changing the mask > parameter to check_command() above, we know we can only reach here if > format = common->cmnd[2] is 0. However this snippet is then reassigning > format from the upper bits of the 9th byte which could be non-zero, at > least in the case of MacOS. > > So this patch does seem a bit incomplete as you point out and maybe > updating the mask as above should help to allow both fields to determine > the format for any non-zero TOC type. > > But I was trying to confirm these details from the SFF-8020i spec as > mentioned in the original comment above. I was only able to find a > draft copy [1] from a web search which stated: > > "Format field definition: When Format in Byte 2 is zero, then > Byte 9 is used. Other values for this field are reserved for > definition in MMC." > > Note: The Format field in Byte 9 is a vendor-specific area and > will be removed in subsequent versions of this specification. > Functionality is moving to Byte 2." > > However when trying to look up the latest official release of SFF-8020 > the SNIA website [2] lists it as having been expired and incorporated > into the SCSI MMC specification. Consequently, I haven't yet been able > to look further into the SCSI MMC spec itself as it seems it is only > available for a fee from the ANSI/INCITS website [3]. I'm hoping you or > maybe somebody on this list might have more knowledge on these details. I thought I had a copy of the old SFF8020i spec somewhere, but now I can't find it. :-( > So I'm left wondering whether the Format field in Byte 9 is even > standardized, or if it is a remnant of an older or possibly > draft/non-final specification. Yet we clearly have a host that is > relying on this behavior, so there is utility in this patch. > > FWIW, here are the raw bytes of the READ TOC request transaction as > issued by the MacOS host, obtained from a bus analyzer trace, which this > patch is purportedly fixing: > > 55 53 42 43 19 00 00 00 FE FF 00 00 80 00 0A 43 > ^^ > cmnd[0] i.e. OpCode > 02 00 00 00 00 00 FF FE 80 00 00 00 00 00 00 > ^^ ^^ > cmnd[2]==0 || > cmnd[9], upper 2 bits == 0x2 Well, there are really just two things we need to worry about: The driver must continue to work properly on all the systems that are using it now. The driver should be able to work with Mac OS-X. I think if you simply change the mask value then we'll be okay. Alan Stern > Thanks, > Jack > > [1] https://www.bswd.com/sff8020i.pdf > [2] https://www.snia.org/technology-communities/sff/specifications > [3] https://webstore.ansi.org/standards/incits/ansiincits4302007
diff --git a/drivers/usb/gadget/function/f_mass_storage.c b/drivers/usb/gadget/function/f_mass_storage.c index 73a28f8..1f7f4dd6 100644 --- a/drivers/usb/gadget/function/f_mass_storage.c +++ b/drivers/usb/gadget/function/f_mass_storage.c @@ -1188,6 +1188,8 @@ static int do_read_toc(struct fsg_common *common, struct fsg_buffhd *bh) int msf = common->cmnd[1] & 0x02; int start_track = common->cmnd[6]; u8 *buf = (u8 *)bh->buf; + u8 format; + int i, len; if ((common->cmnd[1] & ~0x02) != 0 || /* Mask away MSF */ start_track > 1) { @@ -1195,18 +1197,65 @@ static int do_read_toc(struct fsg_common *common, struct fsg_buffhd *bh) return -EINVAL; } - memset(buf, 0, 20); - buf[1] = (20-2); /* TOC data length */ - buf[2] = 1; /* First track number */ - buf[3] = 1; /* Last track number */ - buf[5] = 0x16; /* Data track, copying allowed */ - buf[6] = 0x01; /* Only track is number 1 */ - store_cdrom_address(&buf[8], msf, 0); + format = common->cmnd[2] & 0xf; + /* + * Check if CDB is old style SFF-8020i + * i.e. format is in 2 MSBs of byte 9 + * Mac OS-X host sends us this. + */ + if (format == 0) + format = (common->cmnd[9] >> 6) & 0x3; + + switch (format) { + case 0: + /* Formatted TOC */ + len = 4 + 2*8; /* 4 byte header + 2 descriptors */ + memset(buf, 0, len); + len -= 2; /* TOC Length excludes length field */ + buf[1] = len; /* TOC data length */ + buf[2] = 1; /* First track number */ + buf[3] = 1; /* Last track number */ + buf[5] = 0x16; /* Data track, copying allowed */ + buf[6] = 0x01; /* Only track is number 1 */ + store_cdrom_address(&buf[8], msf, 0); + + buf[13] = 0x16; /* Lead-out track is data */ + buf[14] = 0xAA; /* Lead-out track number */ + store_cdrom_address(&buf[16], msf, curlun->num_sectors); + return len; + + case 2: + /* Raw TOC */ + len = 4 + 3*11; /* 4 byte header + 3 descriptors */ + memset(buf, 0, len); /* Header + A0, A1 & A2 descriptors */ + len -= 2; /* TOC Length excludes length field */ + buf[1] = len; /* TOC data length */ + buf[2] = 1; /* First complete session */ + buf[3] = 1; /* Last complete session */ + + buf += 4; + /* fill in A0, A1 and A2 points */ + for (i = 0; i < 3; i++) { + buf[0] = 1; /* Session number */ + buf[1] = 0x16; /* Data track, copying allowed */ + /* 2 - Track number 0 -> TOC */ + buf[3] = 0xA0 + i; /* A0, A1, A2 point */ + /* 4, 5, 6 - Min, sec, frame is zero */ + buf[8] = 1; /* Pmin: last track number */ + buf += 11; /* go to next track descriptor */ + } + buf -= 11; /* go back to A2 descriptor */ + + /* For A2, 7, 8, 9, 10 - zero, Pmin, Psec, Pframe of Lead out */ + store_cdrom_address(&buf[7], msf, curlun->num_sectors); - buf[13] = 0x16; /* Lead-out track is data */ - buf[14] = 0xAA; /* Lead-out track number */ - store_cdrom_address(&buf[16], msf, curlun->num_sectors); - return 20; + return len; + + default: + /* Multi-session, PMA, ATIP, CD-TEXT not supported/required */ + curlun->sense_data = SS_INVALID_FIELD_IN_CDB; + return -EINVAL; + } } static int do_mode_sense(struct fsg_common *common, struct fsg_buffhd *bh) @@ -1933,7 +1982,7 @@ static int do_scsi_command(struct fsg_common *common) common->data_size_from_cmnd = get_unaligned_be16(&common->cmnd[7]); reply = check_command(common, 10, DATA_DIR_TO_HOST, - (7<<6) | (1<<1), 1, + (0xf<<6) | (1<<1), 1, "READ TOC"); if (reply == 0) reply = do_read_toc(common, bh);