BlueSCSI / BlueSCSI-v2

Open source, open hardware, SCSI emulator using the Pi Pico PR2040
https://bluescsi.com
GNU General Public License v3.0
237 stars 25 forks source link

Wrong SCSI error code and wrong peripheral qualifier for a non-existing LUN with INQUIRY command #127

Closed uweseimet closed 7 months ago

uweseimet commented 7 months ago

I just flashed and tested the 2024-02-22 firmware, and unfortunately there is still something wrong with the LUN handling. The issue is similar to https://github.com/BlueSCSI/BlueSCSI-v2/issues/112, but this time INQUIRY is affected:

s2pexec>-i 0 -c 12:00:00:00:30:00
00000000  00:00:02:02:1f:00:00:18:51:55:41:4e:54:55:4d:20  '........QUANTUM '
00000010  42:6c:75:65:53:43:53:49:20:50:69:63:6f:20:20:20  'BlueSCSI Pico   '
00000020  31:2e:30:20:00:00:00:00:00:00:00:00:00:00:00:00  '1.0 ............'
s2pexec>-i 0:1 -c 12:00:00:00:30:00
[18:08:08.498] [warning] Device reported CHECK CONDITION (status code $02)
Error: ILLEGAL REQUEST (Sense Key $05), LOGICAL UNIT NOT SUPPORTED (ASC $25), ASCQ $00

The same test with a real drive (and emulated SCSI2Pi/PiSCSI devices) does not report an error:

s2pexec>-i 5 -c 12:00:00:00:30:00
00000000  05:80:02:02:33:00:00:18:50:4c:45:58:54:4f:52:20  '....3...PLEXTOR '
00000010  43:44:2d:52:4f:4d:20:50:58:2d:34:30:54:53:20:20  'CD-ROM PX-40TS  '
00000020  31:2e:31:31:30:32:2f:32:31:2f:30:30:20:30:31:3a  '1.1102/21/00 01:'
s2pexec>-i 5:1 -c 12:00:00:00:30:00
00000000  7f:80:02:02:33:00:00:18:50:4c:45:58:54:4f:52:20  '....3...PLEXTOR '
00000010  43:44:2d:52:4f:4d:20:50:58:2d:34:30:54:53:20:20  'CD-ROM PX-40TS  '
00000020  31:2e:31:31:30:32:2f:32:31:2f:30:30:20:30:31:3a  '1.1102/21/00 01:'

Instead, as specified for INQUIRY in section 8.2.5.1 of the SCSI-2 specification, the non-existence of the LUN is signalled with $7f in the peripheral qualifier (first byte of the response):

The peripheral qualifier and peripheral device-type fields identify the
device currently connected to the logical unit.  If the target is not
capable of supporting a device on this logical unit, this field shall be
set to 7Fh (peripheral qualifier set to 011b and peripheral device type
set to 1Fh).  The peripheral qualifier is defined in table 46 and the
peripheral device type is defined in table 47.

I think (but cannot test it anymore) this was not wrong with the previous firmware. Regarding INQUIRY in general, it was designed to more or less never fail, but to always be available to collect vital product data.

By the way, it would be nice if BlueSCSI was reporting the firmware version similar to how the Plextor drive does it. That would make checking the logfile obsolete if you are not sure, for instance, whether flashing was successful.

erichelgeson commented 7 months ago

I just flashed and tested the 2024-02-22 firmware, and unfortunately there is still something wrong with the LUN handling.

The PR was a few weeks ago. Once I push the fix for this can you commit to testing it before I merge to avoid back and forth on LUNs?

I think (but cannot test it anymore) this was not wrong with the previous firmware. Regarding INQUIRY in general, it was designed to more or less never fail, but to always be available to collect vital product data.

All binaries are still there - you could flash back to the previous release.

By the way, it would be nice if BlueSCSI was reporting the firmware version similar to how the Plextor drive does it. That would make checking the logfile obsolete if you are not sure, for instance, whether flashing was successful.

That appears to be in theVendor-specific section? Do you know if that is the same response format for SCSI-1? It appears it can be for "Private Use" so it should be fine.

Please don't get me wrong - I do appreciate you raising issues, but I'll need you to verify they are addressed before commiting fixes to main.

uweseimet commented 7 months ago

Screenshot_20240224_000359 Jugding from section 7.1.3 of the X3.131-1986 specification I'd say that for the vendor-specific section the same rules apply as for SCSI-2.

Verifyng a fix is fine for me. I guess that you are going to provide a binary file to flash, so that I can test any upcoming change? Am I right that downgrading or upgrading is essentially the same, i.e. any valid binary I provide will simply be flashed?

From my perspective it is not that important whether the previous release also had this bug or not. It's just that I am quite sure that my test suite did not report it with the previous release, this is why I mentioned it.

erichelgeson commented 7 months ago

Every push has the binaries built (see the little green checkmark in github on the commit above your message) click it and you can see the build details -> Summary -> Under Artifacts: "BlueSCSI Binaries"

Here's the direct link: https://github.com/BlueSCSI/BlueSCSI-v2/actions/runs/8025983300

Yes you can flash older or newer binaries without issue.

uweseimet commented 7 months ago

Sounds good. I will run a test tomorrow.

uweseimet commented 7 months ago

Thank you for the quick fix. My test suite is happy now and does not report any errors.