Revert fixes for DEC vendor specific pages and CD-ROM block size changing (#1451)

* Revert "Don't ResizeCache on sector change if no filename is defined (#1438)"

This reverts commit dd9a3296d4.

* Revert "Add ModeSense page 0x25 (DEC special function control page) (#1412)"

This reverts commit 1121b8d9d6.

* Revert "DiskCache needs a size"

This reverts commit 7cc8df271c.

* Revert "Honor sector size change via ModeSelect6 in scsicd (#1406)"

This reverts commit b7f65d33e2.

* Revert "Multiple fixes for ModeSelect (#1405)"

This reverts commit ad5eae93e7.
This commit is contained in:
Daniel Markstedt 2024-04-13 19:40:53 +09:00 committed by GitHub
parent 12bb86db9b
commit a6a8cadf21
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
17 changed files with 16 additions and 173 deletions

View File

@ -696,17 +696,11 @@ uint32_t Disk::GetSectorSizeInBytes() const
void Disk::SetSectorSizeInBytes(uint32_t size_in_bytes) void Disk::SetSectorSizeInBytes(uint32_t size_in_bytes)
{ {
if (!GetSupportedSectorSizes().contains(size_in_bytes)) { if (!GetSupportedSectorSizes().contains(size_in_bytes)) {
throw io_exception("Invalid sector size of " + to_string(size_in_bytes) + " byte(s)"); throw io_exception("Invalid sector size of " + to_string(size_in_bytes) + " byte(s)");
} }
uint64_t current_blocks = GetBlockCount();
uint32_t current_size_shift_count = size_shift_count;
uint64_t current_size = current_blocks << current_size_shift_count;
size_shift_count = CalculateShiftCount(size_in_bytes); size_shift_count = CalculateShiftCount(size_in_bytes);
assert(size_shift_count); assert(size_shift_count);
if ((current_blocks > 0) && (current_size_shift_count > 0)) {
SetBlockCount(current_size >> size_shift_count);
}
} }
uint32_t Disk::GetConfiguredSectorSize() const uint32_t Disk::GetConfiguredSectorSize() const
@ -720,7 +714,7 @@ bool Disk::SetConfiguredSectorSize(uint32_t configured_size)
return false; return false;
} }
configured_sector_size = configured_size; configured_sector_size = configured_size;
return true; return true;
} }

View File

@ -112,7 +112,7 @@ protected:
void SetUpCache(off_t, bool = false); void SetUpCache(off_t, bool = false);
void ResizeCache(const string&, bool); void ResizeCache(const string&, bool);
bool GetRawMode() const { return (cache?cache->GetRawMode():false); }
void SetUpModePages(map<int, vector<byte>>&, int, bool) const override; void SetUpModePages(map<int, vector<byte>>&, int, bool) const override;
void AddErrorPage(map<int, vector<byte>>&, bool) const; void AddErrorPage(map<int, vector<byte>>&, bool) const;
virtual void AddFormatPage(map<int, vector<byte>>&, bool) const; virtual void AddFormatPage(map<int, vector<byte>>&, bool) const;

View File

@ -51,7 +51,6 @@ public:
~DiskCache() = default; ~DiskCache() = default;
void SetRawMode(bool b) { cd_raw = b; } // CD-ROM raw mode setting void SetRawMode(bool b) { cd_raw = b; } // CD-ROM raw mode setting
bool GetRawMode() const { return cd_raw; }
bool Save(); // Save and release all bool Save(); // Save and release all
bool ReadSector(span<uint8_t>, uint32_t); // Sector Read bool ReadSector(span<uint8_t>, uint32_t); // Sector Read

View File

@ -114,7 +114,7 @@ void ModePageDevice::ModeSense10() const
EnterDataInPhase(); EnterDataInPhase();
} }
void ModePageDevice::ModeSelect(scsi_command, cdb_t, span<const uint8_t>, int) void ModePageDevice::ModeSelect(scsi_command, cdb_t, span<const uint8_t>, int) const
{ {
// There is no default implementation of MODE SELECT // There is no default implementation of MODE SELECT
throw scsi_exception(sense_key::illegal_request, asc::invalid_command_operation_code); throw scsi_exception(sense_key::illegal_request, asc::invalid_command_operation_code);

View File

@ -23,7 +23,7 @@ public:
bool Init(const param_map&) override; bool Init(const param_map&) override;
virtual void ModeSelect(scsi_defs::scsi_command, cdb_t, span<const uint8_t>, int); virtual void ModeSelect(scsi_defs::scsi_command, cdb_t, span<const uint8_t>, int) const;
protected: protected:

View File

@ -40,8 +40,7 @@ string scsi_command_util::ModeSelect(scsi_command cmd, cdb_t cdb, span<const uin
} }
length -= offset; length -= offset;
// treat zero length as valid bool has_valid_page_code = false;
bool has_valid_page_code = (length == 0);
// Parse the pages // Parse the pages
while (length > 0) { while (length > 0) {
@ -63,10 +62,6 @@ string scsi_command_util::ModeSelect(scsi_command cmd, cdb_t cdb, span<const uin
has_valid_page_code = true; has_valid_page_code = true;
} }
else if (page == 0x01) {
// OpenVMS Alpha 7.3 uses this
has_valid_page_code = true;
}
else { else {
stringstream s; stringstream s;
s << "Unknown MODE SELECT page code: $" << setfill('0') << setw(2) << hex << page; s << "Unknown MODE SELECT page code: $" << setfill('0') << setw(2) << hex << page;
@ -76,7 +71,7 @@ string scsi_command_util::ModeSelect(scsi_command cmd, cdb_t cdb, span<const uin
// Advance to the next page // Advance to the next page
const int size = buf[offset + 1] + 2; const int size = buf[offset + 1] + 2;
length -= size + 1; length -= size;
offset += size; offset += size;
} }

View File

@ -165,33 +165,6 @@ vector<uint8_t> SCSICD::InquiryInternal() const
return HandleInquiry(device_type::cd_rom, scsi_level, true); return HandleInquiry(device_type::cd_rom, scsi_level, true);
} }
void SCSICD::ModeSelect(scsi_command cmd, cdb_t cdb, span<const uint8_t> buf, int length)
{
int sector_size = 1 << GetSectorSizeShiftCount();
int wanted_sector_size;
// skip Block Descriptor
int offset = 4;
// evaluate Mode Parameter Block Descriptor, sector size
wanted_sector_size = scsi_command_util::GetInt16(buf, offset + 6);
if (wanted_sector_size != sector_size) {
LogDebug("Changing sector size from " + to_string(sector_size) + " to " + to_string(wanted_sector_size));
SetSectorSizeInBytes(wanted_sector_size);
ClearTrack();
CreateDataTrack();
FlushCache();
string filename;
if ((filename = GetFilename()) != "") {
// DiskCache fails without a file to compute the cache size
ResizeCache(filename, GetRawMode());
}
}
if (const string result = scsi_command_util::ModeSelect(cmd, cdb, buf, length, sector_size);
!result.empty()) {
LogWarn(result);
}
}
void SCSICD::SetUpModePages(map<int, vector<byte>>& pages, int page, bool changeable) const void SCSICD::SetUpModePages(map<int, vector<byte>>& pages, int page, bool changeable) const
{ {
Disk::SetUpModePages(pages, page, changeable); Disk::SetUpModePages(pages, page, changeable);

View File

@ -34,7 +34,6 @@ public:
vector<uint8_t> InquiryInternal() const override; vector<uint8_t> InquiryInternal() const override;
int Read(span<uint8_t>, uint64_t) override; int Read(span<uint8_t>, uint64_t) override;
void ModeSelect(scsi_defs::scsi_command, cdb_t, span<const uint8_t>, int) override;
protected: protected:

View File

@ -82,7 +82,7 @@ vector<uint8_t> SCSIHD::InquiryInternal() const
return HandleInquiry(device_type::direct_access, scsi_level, IsRemovable()); return HandleInquiry(device_type::direct_access, scsi_level, IsRemovable());
} }
void SCSIHD::ModeSelect(scsi_command cmd, cdb_t cdb, span<const uint8_t> buf, int length) void SCSIHD::ModeSelect(scsi_command cmd, cdb_t cdb, span<const uint8_t> buf, int length) const
{ {
if (const string result = scsi_command_util::ModeSelect(cmd, cdb, buf, length, 1 << GetSectorSizeShiftCount()); if (const string result = scsi_command_util::ModeSelect(cmd, cdb, buf, length, 1 << GetSectorSizeShiftCount());
!result.empty()) { !result.empty()) {
@ -97,32 +97,8 @@ void SCSIHD::AddFormatPage(map<int, vector<byte>>& pages, bool changeable) const
EnrichFormatPage(pages, changeable, 1 << GetSectorSizeShiftCount()); EnrichFormatPage(pages, changeable, 1 << GetSectorSizeShiftCount());
} }
// Page code 37 (25h) - DEC Special Function Control page
void SCSIHD::AddDECSpecialFunctionControlPage(map<int, vector<byte>>& pages, bool changeable) const
{
vector<byte> buf(25);
// No changeable area
if (changeable) {
pages[0x25] = buf;
return;
}
buf[0] = static_cast<byte> (0x25 | 0x80); // page code, high bit set
buf[1] = static_cast<byte> (sizeof(buf) - 1);
buf[2] = static_cast<byte> (0x01); // drive does not auto-start
pages[0x25] = buf;
}
void SCSIHD::AddVendorPage(map<int, vector<byte>>& pages, int page, bool changeable) const void SCSIHD::AddVendorPage(map<int, vector<byte>>& pages, int page, bool changeable) const
{ {
// Page code 0x25: DEC Special Function Control page
if (page == 0x25 || page == 0x3f) {
AddDECSpecialFunctionControlPage(pages, changeable);
}
// Page code 48 // Page code 48
if (page == 0x30 || page == 0x3f) { if (page == 0x30 || page == 0x3f) {
AddAppleVendorModePage(pages, changeable); AddAppleVendorModePage(pages, changeable);

View File

@ -37,10 +37,9 @@ public:
// Commands // Commands
vector<uint8_t> InquiryInternal() const override; vector<uint8_t> InquiryInternal() const override;
void ModeSelect(scsi_defs::scsi_command, cdb_t, span<const uint8_t>, int) override; void ModeSelect(scsi_defs::scsi_command, cdb_t, span<const uint8_t>, int) const override;
void AddFormatPage(map<int, vector<byte>>&, bool) const override; void AddFormatPage(map<int, vector<byte>>&, bool) const override;
void AddDECSpecialFunctionControlPage(map<int, vector<byte>>&, bool) const;
void AddVendorPage(map<int, vector<byte>>&, int, bool) const override; void AddVendorPage(map<int, vector<byte>>&, int, bool) const override;
private: private:

View File

@ -88,7 +88,7 @@ void SCSIMO::AddOptionPage(map<int, vector<byte>>& pages, bool) const
// Do not report update blocks // Do not report update blocks
} }
void SCSIMO::ModeSelect(scsi_command cmd, cdb_t cdb, span<const uint8_t> buf, int length) void SCSIMO::ModeSelect(scsi_command cmd, cdb_t cdb, span<const uint8_t> buf, int length) const
{ {
if (const string result = scsi_command_util::ModeSelect(cmd, cdb, buf, length, 1 << GetSectorSizeShiftCount()); if (const string result = scsi_command_util::ModeSelect(cmd, cdb, buf, length, 1 << GetSectorSizeShiftCount());
!result.empty()) { !result.empty()) {

View File

@ -32,7 +32,7 @@ public:
void Open() override; void Open() override;
vector<uint8_t> InquiryInternal() const override; vector<uint8_t> InquiryInternal() const override;
void ModeSelect(scsi_defs::scsi_command, cdb_t, span<const uint8_t>, int) override; void ModeSelect(scsi_defs::scsi_command, cdb_t, span<const uint8_t>, int) const override;
protected: protected:

View File

@ -360,7 +360,6 @@ class MockSCSIHD : public SCSIHD //NOSONAR Ignore inheritance hierarchy depth in
FRIEND_TEST(ScsiHdTest, FinalizeSetup); FRIEND_TEST(ScsiHdTest, FinalizeSetup);
FRIEND_TEST(ScsiHdTest, GetProductData); FRIEND_TEST(ScsiHdTest, GetProductData);
FRIEND_TEST(ScsiHdTest, SetUpModePages); FRIEND_TEST(ScsiHdTest, SetUpModePages);
FRIEND_TEST(ScsiHdTest, DECSpecialFunctionControlPage);
FRIEND_TEST(ScsiHdTest, GetSectorSizes); FRIEND_TEST(ScsiHdTest, GetSectorSizes);
FRIEND_TEST(ScsiHdTest, ModeSelect); FRIEND_TEST(ScsiHdTest, ModeSelect);
FRIEND_TEST(PiscsiExecutorTest, SetSectorSize); FRIEND_TEST(PiscsiExecutorTest, SetSectorSize);
@ -388,7 +387,6 @@ class MockSCSICD : public SCSICD //NOSONAR Ignore inheritance hierarchy depth in
FRIEND_TEST(ScsiCdTest, GetSectorSizes); FRIEND_TEST(ScsiCdTest, GetSectorSizes);
FRIEND_TEST(ScsiCdTest, SetUpModePages); FRIEND_TEST(ScsiCdTest, SetUpModePages);
FRIEND_TEST(ScsiCdTest, ReadToc); FRIEND_TEST(ScsiCdTest, ReadToc);
FRIEND_TEST(ScsiCdTest, ModeSelect);
using SCSICD::SCSICD; using SCSICD::SCSICD;
}; };

View File

@ -46,11 +46,6 @@ TEST(ScsiCommandUtilTest, ModeSelect6)
Property(&scsi_exception::get_asc, asc::invalid_field_in_parameter_list)))) Property(&scsi_exception::get_asc, asc::invalid_field_in_parameter_list))))
<< "Unsupported page 0 was not rejected"; << "Unsupported page 0 was not rejected";
// Page 1
buf[12] = 0x01;
EXPECT_NO_THROW(ModeSelect(scsi_command::eCmdModeSelect6, cdb, buf, LENGTH, 512))
<< "Page 1 is supported";
// Page 3 (Format Device Page) // Page 3 (Format Device Page)
buf[12] = 0x03; buf[12] = 0x03;
EXPECT_THAT([&] { ModeSelect(scsi_command::eCmdModeSelect6, cdb, buf, LENGTH, 512); }, EXPECT_THAT([&] { ModeSelect(scsi_command::eCmdModeSelect6, cdb, buf, LENGTH, 512); },
@ -67,25 +62,7 @@ TEST(ScsiCommandUtilTest, ModeSelect6)
Property(&scsi_exception::get_asc, asc::invalid_field_in_parameter_list)))) Property(&scsi_exception::get_asc, asc::invalid_field_in_parameter_list))))
<< "Not enough command parameters"; << "Not enough command parameters";
// check length computation EXPECT_FALSE(ModeSelect(scsi_command::eCmdModeSelect6, cdb, buf, LENGTH, 512).empty());
buf[3] = 8;
buf[10] = 2;
buf[12] = 1;
buf[13] = 10;
buf[14] = 0x24;
buf[24] = 0;
EXPECT_NO_THROW(ModeSelect(scsi_command::eCmdModeSelect6, cdb, buf, LENGTH, 512))
<< "Multi-page length computation";
// check length computation
buf[3] = 8;
buf[10] = 12;
buf[12] = 0;
buf[13] = 0;
buf[14] = 0;
buf[24] = 0;
EXPECT_NO_THROW(ModeSelect(scsi_command::eCmdModeSelect6, cdb, buf, 12, 512))
<< "Empty ModeSelect6";
} }
TEST(ScsiCommandUtilTest, ModeSelect10) TEST(ScsiCommandUtilTest, ModeSelect10)
@ -134,6 +111,8 @@ TEST(ScsiCommandUtilTest, ModeSelect10)
Property(&scsi_exception::get_sense_key, sense_key::illegal_request), Property(&scsi_exception::get_sense_key, sense_key::illegal_request),
Property(&scsi_exception::get_asc, asc::invalid_field_in_parameter_list)))) Property(&scsi_exception::get_asc, asc::invalid_field_in_parameter_list))))
<< "Not enough command parameters"; << "Not enough command parameters";
EXPECT_FALSE(ModeSelect(scsi_command::eCmdModeSelect10, cdb, buf, LENGTH, 512).empty());
} }
TEST(ScsiCommandUtilTest, EnrichFormatPage) TEST(ScsiCommandUtilTest, EnrichFormatPage)

View File

@ -133,56 +133,3 @@ TEST(ScsiCdTest, ReadToc)
// Further testing requires filesystem access // Further testing requires filesystem access
} }
TEST(ScsiCdTest, ModeSelect)
{
MockSCSICD cd(0);
MockSCSICD cd1(0);
vector<int> cmd(6);
vector<uint8_t> buf(255);
// dummy file for DiskCache resize after sector size change
path filename = CreateTempFile(2* 2048);
cd.SetFilename(string(filename));
cd.Open();
EXPECT_EQ(2, cd.GetBlockCount());
cd.SetSectorSizeInBytes(2048);
// PF
cmd[1] = 0x10;
// Length
buf[3] = 0x08;
// 2048 bytes per sector
buf[10] = 0x08;
// Page 3 (Device Format Page)
buf[12] = 0x01;
EXPECT_NO_THROW(cd.ModeSelect(scsi_command::eCmdModeSelect6, cmd, buf, 255)) << "MODE SELECT(6) with sector size 2048 is supported";
// 512 bytes per sector - ModeSelect6
buf[10] = 0x02;
EXPECT_NO_THROW(cd.ModeSelect(scsi_command::eCmdModeSelect6, cmd, buf, 255)) << "MODE SELECT(6) with sector size 512 is supported";
// 2048 bytes per sector - ModeSelect6
buf[10] = 0x08;
EXPECT_NO_THROW(cd.ModeSelect(scsi_command::eCmdModeSelect6, cmd, buf, 255)) << "MODE SELECT(6) with sector size 2048 is supported";
// 512 bytes per sector - ModeSelect10
buf[10] = 0x02;
EXPECT_NO_THROW(cd.ModeSelect(scsi_command::eCmdModeSelect10, cmd, buf, 255)) << "MODE SELECT(10) with sector size 512 is supported";
// unsupported sector size - ModeSelect6
buf[10] = 0x04;
EXPECT_THROW(cd.ModeSelect(scsi_command::eCmdModeSelect6, cmd, buf, 255), io_exception) << "MODE SELECT(6) with sector size 1024 is unsupported";
// sector size not multiple of 512 - ModeSelect6
buf[10] = 0x03;
EXPECT_THROW(cd.ModeSelect(scsi_command::eCmdModeSelect6, cmd, buf, 255), io_exception) << "MODE SELECT(6) with sector size 768 is unsupported";
// cd1 has no dummy file attached, simulating an empty CD drive
cd1.SetSectorSizeInBytes(2048);
// 512 bytes per sector - ModeSelect6
buf[10] = 0x02;
EXPECT_NO_THROW(cd1.ModeSelect(scsi_command::eCmdModeSelect6, cmd, buf, 255)) << "MODE SELECT(6), emtpy drive, with sector size 512 is supported";
}

View File

@ -18,12 +18,11 @@ using namespace filesystem;
void ScsiHdNecTest_SetUpModePages(map<int, vector<byte>>& pages) void ScsiHdNecTest_SetUpModePages(map<int, vector<byte>>& pages)
{ {
EXPECT_EQ(6, pages.size()) << "Unexpected number of mode pages"; EXPECT_EQ(5, pages.size()) << "Unexpected number of mode pages";
EXPECT_EQ(12, pages[1].size()); EXPECT_EQ(12, pages[1].size());
EXPECT_EQ(24, pages[3].size()); EXPECT_EQ(24, pages[3].size());
EXPECT_EQ(20, pages[4].size()); EXPECT_EQ(20, pages[4].size());
EXPECT_EQ(12, pages[8].size()); EXPECT_EQ(12, pages[8].size());
EXPECT_EQ(25, pages[37].size());
EXPECT_EQ(30, pages[48].size()); EXPECT_EQ(30, pages[48].size());
} }

View File

@ -13,12 +13,11 @@
void ScsiHdTest_SetUpModePages(map<int, vector<byte>>& pages) void ScsiHdTest_SetUpModePages(map<int, vector<byte>>& pages)
{ {
EXPECT_EQ(6, pages.size()) << "Unexpected number of mode pages"; EXPECT_EQ(5, pages.size()) << "Unexpected number of mode pages";
EXPECT_EQ(12, pages[1].size()); EXPECT_EQ(12, pages[1].size());
EXPECT_EQ(24, pages[3].size()); EXPECT_EQ(24, pages[3].size());
EXPECT_EQ(24, pages[4].size()); EXPECT_EQ(24, pages[4].size());
EXPECT_EQ(12, pages[8].size()); EXPECT_EQ(12, pages[8].size());
EXPECT_EQ(25, pages[37].size());
EXPECT_EQ(30, pages[48].size()); EXPECT_EQ(30, pages[48].size());
} }
@ -102,20 +101,6 @@ TEST(ScsiHdTest, SetUpModePages)
ScsiHdTest_SetUpModePages(pages); ScsiHdTest_SetUpModePages(pages);
} }
TEST(ScsiHdTest, DECSpecialFunctionControlPage)
{
map<int, vector<byte>> pages;
vector<byte> buf;
MockSCSIHD hd(0, false);
EXPECT_NO_THROW(hd.SetUpModePages(pages, 0x25, false)) << "MODE SENSE(6) DEC unique page is supported";
EXPECT_NE(pages.end(), pages.find(0x25));
buf = pages[0x25];
EXPECT_EQ(static_cast<byte> (0x25 | 0x80), buf[0]);
EXPECT_EQ(static_cast<byte> (0x17), buf[1]);
EXPECT_EQ(static_cast<byte> (0x01), buf[2]);
}
TEST(ScsiHdTest, ModeSelect) TEST(ScsiHdTest, ModeSelect)
{ {
MockSCSIHD hd({ 512 }); MockSCSIHD hd({ 512 });