This commit is contained in:
Eric Helgeson
2026-01-12 22:07:54 -06:00
parent c6e1c5c6fa
commit c3a70d1beb
3 changed files with 43 additions and 16 deletions
+9 -12
View File
@@ -134,6 +134,8 @@ TEST(DiskTest, Seek6)
// Only bits 4-0 of byte 1 are part of the 21-bit LBA.
// Bug was introduced in commit 255a6e1 (Oct 2022) when GetInt24() replaced proper masking.
// Reference: SCSI-2 spec Table 122 (READ(6)), Table 144 (WRITE(6)), Table 138 (SEEK(6))
// Note: We use SEEK(6) instead of READ(6) because both use the same LBA parsing logic
// (CheckAndGetStartAndCount with RW6/SEEK6 mode) but SEEK doesn't require disk cache.
TEST(DiskTest, Read6LbaLunMasking)
{
auto [controller, disk] = CreateDisk();
@@ -150,11 +152,10 @@ TEST(DiskTest, Read6LbaLunMasking)
controller->SetCmdByte(1, 0x00);
controller->SetCmdByte(2, 0x01);
controller->SetCmdByte(3, 0x00);
controller->SetCmdByte(4, 0); // Transfer length 0 means no actual read, just validation
// This should succeed - LBA 0x100 is within capacity
EXPECT_CALL(*controller, Status);
disk->Dispatch(scsi_command::eCmdRead6);
disk->Dispatch(scsi_command::eCmdSeek6);
EXPECT_EQ(status::good, controller->GetStatus());
// Test case 2: LUN=3 (bits 7-5 = 0x60), LBA=0x000100
@@ -165,13 +166,12 @@ TEST(DiskTest, Read6LbaLunMasking)
controller->SetCmdByte(1, 0x60); // LUN=3 in bits 7-5
controller->SetCmdByte(2, 0x01);
controller->SetCmdByte(3, 0x00);
controller->SetCmdByte(4, 0);
// This should succeed if LUN bits are properly masked
EXPECT_CALL(*controller, Status);
disk->Dispatch(scsi_command::eCmdRead6);
disk->Dispatch(scsi_command::eCmdSeek6);
EXPECT_EQ(status::good, controller->GetStatus())
<< "READ(6) with LUN bits in byte 1 should mask them out per SCSI spec";
<< "SEEK(6) with LUN bits in byte 1 should mask them out per SCSI spec";
// Test case 3: LUN=7 (maximum, bits 7-5 = 0xE0), LBA=0x000100
// Byte 1: 0xE0 | 0x00 = 0xE0
@@ -180,24 +180,21 @@ TEST(DiskTest, Read6LbaLunMasking)
controller->SetCmdByte(1, 0xE0); // LUN=7 in bits 7-5
controller->SetCmdByte(2, 0x01);
controller->SetCmdByte(3, 0x00);
controller->SetCmdByte(4, 0);
EXPECT_CALL(*controller, Status);
disk->Dispatch(scsi_command::eCmdRead6);
disk->Dispatch(scsi_command::eCmdSeek6);
EXPECT_EQ(status::good, controller->GetStatus())
<< "READ(6) with maximum LUN bits should still work";
<< "SEEK(6) with maximum LUN bits should still work";
// Test case 4: Verify the LBA bits in byte 1 (bits 4-0) ARE used
// LUN=0, LBA=0x010000 (65536) - should fail because it exceeds 512 blocks
controller->SetCmdByte(1, 0x01); // LBA bit 16 set
controller->SetCmdByte(2, 0x00);
controller->SetCmdByte(3, 0x00);
controller->SetCmdByte(4, 0);
EXPECT_THAT([&] { d->Dispatch(scsi_command::eCmdRead6); }, Throws<scsi_exception>(AllOf(
EXPECT_THAT([&] { d->Dispatch(scsi_command::eCmdSeek6); }, Throws<scsi_exception>(AllOf(
Property(&scsi_exception::get_sense_key, sense_key::illegal_request),
Property(&scsi_exception::get_asc, asc::lba_out_of_range))))
<< "READ(6) to LBA 0x10000 should fail for 512-block medium";
<< "SEEK(6) to LBA 0x10000 should fail for 512-block medium";
}
// Same test for SEEK(6) command
+3
View File
@@ -21,6 +21,9 @@ nix build .#piscsi-aarch64-debug
# Native build (for local testing)
nix build
# Run unit tests
nix build .#test
```
## Development
+31 -4
View File
@@ -16,7 +16,7 @@
pkgsAarch64 = pkgs.pkgsCross.aarch64-multiplatform;
# Common build function for PiSCSI
mkPiscsi = { stdenv, protobuf, libpcap, spdlog, fmt, abseil-cpp, connectType ? "FULLSPEC", debug ? false }:
mkPiscsi = { stdenv, protobuf, libpcap, spdlog, fmt, abseil-cpp, gtest, connectType ? "FULLSPEC", debug ? false, withTests ? false }:
stdenv.mkDerivation {
pname = "piscsi";
version = "24.10.01";
@@ -41,8 +41,13 @@
spdlog
fmt
abseil-cpp
] ++ pkgs.lib.optionals withTests [
gtest
];
# Tests can be run manually; some fail in sandbox (no network) or segfault
doCheck = false;
makeFlags = [
"CONNECT_TYPE=${connectType}"
] ++ pkgs.lib.optionals debug [
@@ -64,6 +69,7 @@
# Build targets
targets="bin/piscsi bin/scsictl bin/scsimon bin/scsiloop"
${pkgs.lib.optionalString (connectType == "FULLSPEC") ''targets="$targets bin/scsidump"''}
${pkgs.lib.optionalString withTests ''targets="$targets bin/piscsi_test"''}
# Get abseil libs needed by protobuf
ABSL_LIBS=$(pkg-config --libs protobuf 2>/dev/null | grep -oE '\-labsl[^ ]+' | tr '\n' ' ')
@@ -78,6 +84,12 @@
runHook postBuild
'';
checkPhase = pkgs.lib.optionalString withTests ''
runHook preCheck
./bin/piscsi_test
runHook postCheck
'';
installPhase = ''
runHook preInstall
@@ -111,6 +123,7 @@
spdlog = crossPkgs.spdlog;
fmt = crossPkgs.fmt;
abseil-cpp = crossPkgs.abseil-cpp;
gtest = crossPkgs.gtest;
inherit connectType;
};
@@ -124,6 +137,19 @@
spdlog = pkgs.spdlog;
fmt = pkgs.fmt;
abseil-cpp = pkgs.abseil-cpp;
gtest = pkgs.gtest;
};
# Native build with tests
test = mkPiscsi {
stdenv = pkgs.stdenv;
protobuf = pkgs.protobuf;
libpcap = pkgs.libpcap;
spdlog = pkgs.spdlog;
fmt = pkgs.fmt;
abseil-cpp = pkgs.abseil-cpp;
gtest = pkgs.gtest;
withTests = true;
};
# Cross-compiled for Raspberry Pi 2/3/Zero W (ARMv7)
@@ -142,6 +168,7 @@
spdlog = pkgsArmv7.spdlog;
fmt = pkgsArmv7.fmt;
abseil-cpp = pkgsArmv7.abseil-cpp;
gtest = pkgsArmv7.gtest;
connectType = "FULLSPEC";
debug = true;
};
@@ -153,6 +180,7 @@
spdlog = pkgsAarch64.spdlog;
fmt = pkgsAarch64.fmt;
abseil-cpp = pkgsAarch64.abseil-cpp;
gtest = pkgsAarch64.gtest;
connectType = "FULLSPEC";
debug = true;
};
@@ -180,7 +208,6 @@
# Testing
gtest
gmock
];
shellHook = ''
@@ -198,8 +225,8 @@
echo " .#piscsi-armv7-debug - ARMv7 debug build"
echo " .#piscsi-aarch64-debug - AArch64 debug build"
echo ""
echo "Manual build from cpp/:"
echo " make all CONNECT_TYPE=FULLSPEC"
echo "Run tests:"
echo " nix build .#test"
echo ""
'';
};