diff --git a/cpp/controllers/abstract_controller.h b/cpp/controllers/abstract_controller.h index 1ec70aed..ca262abe 100644 --- a/cpp/controllers/abstract_controller.h +++ b/cpp/controllers/abstract_controller.h @@ -84,6 +84,10 @@ public: scsi_defs::status GetStatus() const { return ctrl.status; } void SetStatus(scsi_defs::status s) { ctrl.status = s; } uint32_t GetLength() const { return ctrl.length; } + void SetLength(uint32_t l) { ctrl.length = l; } + uint32_t GetBlocks() const { return ctrl.blocks; } + void SetBlocks(uint32_t b) { ctrl.blocks = b; } + void DecrementBlocks() { --ctrl.blocks; } bool IsByteTransfer() const { return is_byte_transfer; } void SetByteTransfer(bool); @@ -101,7 +105,6 @@ protected: bool HasValidLength() const { return ctrl.length != 0; } int GetOffset() const { return ctrl.offset; } void ResetOffset() { ctrl.offset = 0; } - void SetLength(uint32_t l) { ctrl.length = l; } void UpdateOffsetAndLength() { ctrl.offset += ctrl.length; ctrl.length = 0; } private: diff --git a/cpp/controllers/scsi_controller.cpp b/cpp/controllers/scsi_controller.cpp index 49e34382..116192fa 100644 --- a/cpp/controllers/scsi_controller.cpp +++ b/cpp/controllers/scsi_controller.cpp @@ -16,8 +16,9 @@ #include "hal/gpiobus.h" #include "hal/systimer.h" #include "rascsi_exceptions.h" -#include "devices/scsi_host_bridge.h" -#include "devices/scsi_daynaport.h" +#include "devices/interfaces/byte_writer.h" +#include "devices/mode_page_device.h" +#include "devices/disk.h" #include "scsi_controller.h" #include #include @@ -197,6 +198,7 @@ void ScsiController::Command() bus->SetIO(false); const int actual_count = bus->CommandHandShake(GetBuffer().data()); + // TODO Try to move GetCommandByteCount() to BUS, so that the controller does not need to know GPIOBUS const int command_byte_count = GPIOBUS::GetCommandByteCount(GetBuffer()[0]); // If not able to receive all, move to the status phase @@ -229,7 +231,7 @@ void ScsiController::Execute() // Initialization for data transfer ResetOffset(); - ctrl.blocks = 1; + SetBlocks(1); execstart = SysTimer::GetTimerLow(); // Discard pending sense data from the previous command if the current command is not REQUEST SENSE @@ -317,7 +319,7 @@ void ScsiController::Status() // Data transfer is 1 byte x 1 block ResetOffset(); SetLength(1); - ctrl.blocks = 1; + SetBlocks(1); GetBuffer()[0] = (BYTE)GetStatus(); return; @@ -367,7 +369,7 @@ void ScsiController::MsgOut() // Data transfer is 1 byte x 1 block ResetOffset(); SetLength(1); - ctrl.blocks = 1; + SetBlocks(1); return; } @@ -492,13 +494,13 @@ void ScsiController::Send() if (HasValidLength()) { LOGTRACE("%s%s", __PRETTY_FUNCTION__, (" Sending handhake with offset " + to_string(GetOffset()) + ", length " - + to_string(ctrl.length)).c_str()) + + to_string(GetLength())).c_str()) // TODO The delay has to be taken from ctrl.unit[lun], but as there are currently no Daynaport drivers for // LUNs other than 0 this work-around works. - if (const int len = bus->SendHandShake(GetBuffer().data() + ctrl.offset, ctrl.length, + if (const int len = bus->SendHandShake(GetBuffer().data() + ctrl.offset, GetLength(), HasDeviceForLun(0) ? GetDeviceForLun(0)->GetSendDelay() : 0); - len != (int)ctrl.length) { + len != (int)GetLength()) { // If you cannot send all, move to status phase Error(sense_key::ABORTED_COMMAND); return; @@ -510,14 +512,14 @@ void ScsiController::Send() } // Block subtraction, result initialization - ctrl.blocks--; + DecrementBlocks(); bool result = true; // Processing after data collection (read/data-in only) - if (IsDataIn() && ctrl.blocks != 0) { + if (IsDataIn() && GetBlocks() != 0) { // set next buffer (set offset, length) result = XferIn(GetBuffer()); - LOGTRACE("%s%s", __PRETTY_FUNCTION__, (" Processing after data collection. Blocks: " + to_string(ctrl.blocks)).c_str()) + LOGTRACE("%s%s", __PRETTY_FUNCTION__, (" Processing after data collection. Blocks: " + to_string(GetBlocks())).c_str()) } // If result FALSE, move to status phase @@ -527,8 +529,8 @@ void ScsiController::Send() } // Continue sending if block !=0 - if (ctrl.blocks != 0){ - LOGTRACE("%s%s", __PRETTY_FUNCTION__, (" Continuing to send. Blocks: " + to_string(ctrl.blocks)).c_str()) + if (GetBlocks() != 0){ + LOGTRACE("%s%s", __PRETTY_FUNCTION__, (" Continuing to send. Blocks: " + to_string(GetBlocks())).c_str()) assert(HasValidLength()); assert(ctrl.offset == 0); return; @@ -562,7 +564,7 @@ void ScsiController::Send() case BUS::phase_t::status: // Message in phase SetLength(1); - ctrl.blocks = 1; + SetBlocks(1); GetBuffer()[0] = (BYTE)ctrl.message; MsgIn(); break; @@ -588,12 +590,11 @@ void ScsiController::Receive() // Length != 0 if received if (HasValidLength()) { - LOGTRACE("%s Length is %d bytes", __PRETTY_FUNCTION__, ctrl.length) + LOGTRACE("%s Length is %d bytes", __PRETTY_FUNCTION__, GetLength()) // If not able to receive all, move to status phase - if (int len = bus->ReceiveHandShake(GetBuffer().data() + GetOffset(), ctrl.length); - len != (int)ctrl.length) { - LOGERROR("%s Not able to receive %d bytes of data, only received %d",__PRETTY_FUNCTION__, ctrl.length, len) + if (int len = bus->ReceiveHandShake(GetBuffer().data() + GetOffset(), GetLength()); len != (int)GetLength()) { + LOGERROR("%s Not able to receive %d bytes of data, only received %d",__PRETTY_FUNCTION__, GetLength(), len) Error(sense_key::ABORTED_COMMAND); return; } @@ -604,14 +605,14 @@ void ScsiController::Receive() } // Block subtraction, result initialization - ctrl.blocks--; + DecrementBlocks(); bool result = true; // Processing after receiving data (by phase) LOGTRACE("%s Phase: %s",__PRETTY_FUNCTION__, BUS::GetPhaseStrRaw(GetPhase())) switch (GetPhase()) { case BUS::phase_t::dataout: - if (ctrl.blocks == 0) { + if (GetBlocks() == 0) { // End with this buffer result = XferOut(false); } else { @@ -637,14 +638,13 @@ void ScsiController::Receive() } // If result FALSE, move to status phase - // TODO Check whether we can raise scsi_exception here in order to simplify the error handling if (!result) { Error(sense_key::ABORTED_COMMAND); return; } - // Continue to receive if block !=0 - if (ctrl.blocks != 0) { + // Continue to receive if block != 0 + if (GetBlocks() != 0) { assert(HasValidLength()); assert(ctrl.offset == 0); return; @@ -693,18 +693,17 @@ void ScsiController::ReceiveBytes() assert(!bus->GetIO()); if (HasValidLength()) { - LOGTRACE("%s Length is %d bytes", __PRETTY_FUNCTION__, ctrl.length) + LOGTRACE("%s Length is %d bytes", __PRETTY_FUNCTION__, GetLength()) // If not able to receive all, move to status phase - if (uint32_t len = bus->ReceiveHandShake(GetBuffer().data() + GetOffset(), ctrl.length); - len != ctrl.length) { + if (uint32_t len = bus->ReceiveHandShake(GetBuffer().data() + GetOffset(), GetLength()); len != GetLength()) { LOGERROR("%s Not able to receive %d bytes of data, only received %d", - __PRETTY_FUNCTION__, ctrl.length, len) + __PRETTY_FUNCTION__, GetLength(), len) Error(sense_key::ABORTED_COMMAND); return; } - bytes_to_transfer = ctrl.length; + bytes_to_transfer = GetLength(); UpdateOffsetAndLength(); @@ -795,7 +794,6 @@ void ScsiController::DataOutNonBlockOriented() case scsi_command::eCmdModeSelect6: case scsi_command::eCmdModeSelect10: { - // TODO Try to get rid of this cast if (auto device = dynamic_pointer_cast(GetDeviceForLun(GetEffectiveLun())); device != nullptr) { device->ModeSelect(GetOpcode(), ctrl.cmd, GetBuffer(), GetOffset()); @@ -867,26 +865,33 @@ bool ScsiController::XferIn(vector& buf) // Data transfer OUT // *If cont=true, reset the offset and length // +// TODO Try to use less casts +// //--------------------------------------------------------------------------- bool ScsiController::XferOutBlockOriented(bool cont) { - auto disk = dynamic_pointer_cast(GetDeviceForLun(GetEffectiveLun())); - if (disk == nullptr) { - return false; - } + auto device = GetDeviceForLun(GetEffectiveLun()); // Limited to write commands switch (GetOpcode()) { case scsi_command::eCmdModeSelect6: case scsi_command::eCmdModeSelect10: + { + auto mode_page_device = dynamic_pointer_cast(device); + if (mode_page_device == nullptr) { + return false; + } + try { - disk->ModeSelect(GetOpcode(), ctrl.cmd, GetBuffer(), GetOffset()); + mode_page_device->ModeSelect(GetOpcode(), ctrl.cmd, GetBuffer(), GetOffset()); } catch(const scsi_exception& e) { Error(e.get_sense_key(), e.get_asc()); return false; } + break; + } case scsi_command::eCmdWrite6: case scsi_command::eCmdWrite10: @@ -895,11 +900,9 @@ bool ScsiController::XferOutBlockOriented(bool cont) case scsi_command::eCmdVerify10: case scsi_command::eCmdVerify16: { - // Special case Write function for bridge - // TODO This class must not know about SCSIBR - if (auto bridge = dynamic_pointer_cast(disk); bridge) { - if (!bridge->WriteBytes(ctrl.cmd, GetBuffer(), ctrl.length)) { - // Write failed + // Special case for SCBR and SCDP + if (auto byte_writer = dynamic_pointer_cast(device); byte_writer) { + if (!byte_writer->WriteBytes(ctrl.cmd, GetBuffer(), GetLength())) { return false; } @@ -907,14 +910,9 @@ bool ScsiController::XferOutBlockOriented(bool cont) break; } - // Special case Write function for DaynaPort - // TODO This class must not know about DaynaPort - if (auto daynaport = dynamic_pointer_cast(disk); daynaport) { - daynaport->WriteBytes(ctrl.cmd, GetBuffer(), 0); - - ResetOffset(); - ctrl.blocks = 0; - break; + auto disk = dynamic_pointer_cast(device); + if (disk == nullptr) { + return false; } try { @@ -923,7 +921,6 @@ bool ScsiController::XferOutBlockOriented(bool cont) catch(const scsi_exception& e) { Error(e.get_sense_key(), e.get_asc()); - // Write failed return false; } @@ -938,7 +935,6 @@ bool ScsiController::XferOutBlockOriented(bool cont) SetLength(disk->WriteCheck(ctrl.next - 1)); } catch(const scsi_exception&) { - // Cannot write return false; } @@ -1006,7 +1002,7 @@ void ScsiController::ParseMessage() // Check only when synchronous transfer is possible if (!scsi.syncenable || scsi.msb[i + 2] != 0x01) { SetLength(1); - ctrl.blocks = 1; + SetBlocks(1); GetBuffer()[0] = 0x07; MsgIn(); return; @@ -1024,7 +1020,7 @@ void ScsiController::ParseMessage() // STDR response message generation SetLength(5); - ctrl.blocks = 1; + SetBlocks(1); GetBuffer()[0] = 0x01; GetBuffer()[1] = 0x03; GetBuffer()[2] = 0x01; @@ -1046,7 +1042,7 @@ void ScsiController::ProcessMessage() // Data transfer is 1 byte x 1 block ResetOffset(); SetLength(1); - ctrl.blocks = 1; + SetBlocks(1); return; } diff --git a/cpp/devices/disk.cpp b/cpp/devices/disk.cpp index c4d40024..74a97c82 100644 --- a/cpp/devices/disk.cpp +++ b/cpp/devices/disk.cpp @@ -110,7 +110,7 @@ void Disk::FormatUnit() void Disk::Read(access_mode mode) { if (uint64_t start; CheckAndGetStartAndCount(start, ctrl->blocks, mode)) { - ctrl->length = Read(ctrl->cmd, controller->GetBuffer(), start); + controller->SetLength(Read(ctrl->cmd, controller->GetBuffer(), start)); LOGTRACE("%s ctrl.length is %d", __PRETTY_FUNCTION__, controller->GetLength()) // Set next block @@ -150,7 +150,7 @@ void Disk::ReadWriteLong16() void Disk::Write(access_mode mode) { if (uint64_t start; CheckAndGetStartAndCount(start, ctrl->blocks, mode)) { - ctrl->length = WriteCheck(start); + controller->SetLength(WriteCheck(start)); // Set next block ctrl->next = start + 1; @@ -172,7 +172,7 @@ void Disk::Verify(access_mode mode) } // Test reading - ctrl->length = Read(ctrl->cmd, controller->GetBuffer(), start); + controller->SetLength(Read(ctrl->cmd, controller->GetBuffer(), start)); // Set next block ctrl->next = start + 1; @@ -245,7 +245,7 @@ void Disk::ReadDefectData10() // The defect list is empty fill_n(controller->GetBuffer().begin(), allocation_length, 0); - ctrl->length = (int)allocation_length; + controller->SetLength((uint32_t)allocation_length); EnterDataInPhase(); } @@ -586,7 +586,7 @@ void Disk::ReadCapacity10() SetInt32(buf, 4, 1 << size_shift_count); // the size - ctrl->length = 8; + controller->SetLength(8); EnterDataInPhase(); } @@ -613,7 +613,7 @@ void Disk::ReadCapacity16() buf[13] = 0; // the size - ctrl->length = 14; + controller->SetLength(14); EnterDataInPhase(); } diff --git a/cpp/devices/interfaces/byte_writer.h b/cpp/devices/interfaces/byte_writer.h new file mode 100644 index 00000000..a8625264 --- /dev/null +++ b/cpp/devices/interfaces/byte_writer.h @@ -0,0 +1,28 @@ +//--------------------------------------------------------------------------- +// +// SCSI Target Emulator RaSCSI Reloaded +// for Raspberry Pi +// +// Copyright (C) 2022 Uwe Seimet +// +// Abstraction for the DaynaPort and the host bridge, which both have methods for writing byte sequences +// +//--------------------------------------------------------------------------- + +#pragma once + +#include "os.h" +#include + +using namespace std; + +class ByteWriter +{ + +public: + + ByteWriter() = default; + virtual ~ByteWriter() = default; + + virtual bool WriteBytes(const vector&, vector&, uint32_t) = 0; +}; diff --git a/cpp/devices/mode_page_device.cpp b/cpp/devices/mode_page_device.cpp index 08f58bc6..85c77402 100644 --- a/cpp/devices/mode_page_device.cpp +++ b/cpp/devices/mode_page_device.cpp @@ -101,14 +101,14 @@ int ModePageDevice::AddModePages(const vector& cdb, vector& buf, int void ModePageDevice::ModeSense6() { - ctrl->length = ModeSense6(ctrl->cmd, controller->GetBuffer()); + controller->SetLength(ModeSense6(ctrl->cmd, controller->GetBuffer())); EnterDataInPhase(); } void ModePageDevice::ModeSense10() { - ctrl->length = ModeSense10(ctrl->cmd, controller->GetBuffer()); + controller->SetLength(ModeSense10(ctrl->cmd, controller->GetBuffer())); EnterDataInPhase(); } @@ -120,7 +120,7 @@ void ModePageDevice::ModeSelect(scsi_command, const vector&, const vectorlength = SaveParametersCheck(ctrl->cmd[4]); + controller->SetLength(SaveParametersCheck(ctrl->cmd[4])); EnterDataOutPhase(); } @@ -129,7 +129,7 @@ void ModePageDevice::ModeSelect10() { const size_t length = min(controller->GetBuffer().size(), (size_t)GetInt16(ctrl->cmd, 7)); - ctrl->length = SaveParametersCheck((int)length); + controller->SetLength(SaveParametersCheck((uint32_t)length)); EnterDataOutPhase(); } diff --git a/cpp/devices/primary_device.cpp b/cpp/devices/primary_device.cpp index d7c03366..c0b69c13 100644 --- a/cpp/devices/primary_device.cpp +++ b/cpp/devices/primary_device.cpp @@ -77,7 +77,7 @@ void PrimaryDevice::Inquiry() const size_t allocation_length = min(buf.size(), (size_t)GetInt16(ctrl->cmd, 3)); memcpy(controller->GetBuffer().data(), buf.data(), allocation_length); - ctrl->length = (uint32_t)allocation_length; + controller->SetLength((uint32_t)allocation_length); // Report if the device does not support the requested LUN if (int lun = controller->GetEffectiveLun(); !controller->HasDeviceForLun(lun)) { @@ -114,7 +114,7 @@ void PrimaryDevice::ReportLuns() size += 8; - ctrl->length = min(allocation_length, size); + controller->SetLength(min(allocation_length, size)); EnterDataInPhase(); } @@ -142,7 +142,7 @@ void PrimaryDevice::RequestSense() const size_t allocation_length = min(buf.size(), (size_t)ctrl->cmd[4]); memcpy(controller->GetBuffer().data(), buf.data(), allocation_length); - ctrl->length = (uint32_t)allocation_length; + controller->SetLength((uint32_t)allocation_length); EnterDataInPhase(); } diff --git a/cpp/devices/scsi_command_util.h b/cpp/devices/scsi_command_util.h index 2d44230f..5a8903c6 100644 --- a/cpp/devices/scsi_command_util.h +++ b/cpp/devices/scsi_command_util.h @@ -11,6 +11,7 @@ #pragma once +#include "os.h" #include "scsi.h" #include #include diff --git a/cpp/devices/scsi_daynaport.cpp b/cpp/devices/scsi_daynaport.cpp index 3567f1ae..0967a99a 100644 --- a/cpp/devices/scsi_daynaport.cpp +++ b/cpp/devices/scsi_daynaport.cpp @@ -32,8 +32,7 @@ using namespace scsi_defs; using namespace scsi_command_util; -// TODO Disk must not be the superclass -SCSIDaynaPort::SCSIDaynaPort(int lun) : Disk(SCDP, lun) +SCSIDaynaPort::SCSIDaynaPort(int lun) : PrimaryDevice(SCDP, lun) { dispatcher.Add(scsi_command::eCmdTestUnitReady, "TestUnitReady", &SCSIDaynaPort::TestUnitReady); dispatcher.Add(scsi_command::eCmdRead6, "Read6", &SCSIDaynaPort::Read6); @@ -48,20 +47,10 @@ SCSIDaynaPort::SCSIDaynaPort(int lun) : Disk(SCDP, lun) SetSendDelay(DAYNAPORT_READ_HEADER_SZ); SupportsParams(true); - // TODO Remove as soon as SCDP is not a subclass of Disk anymore - SetStoppable(false); - // TODO Remove as soon as SCDP is not a subclass of Disk anymore - SupportsFile(false); } bool SCSIDaynaPort::Dispatch(scsi_command cmd) { - // TODO As long as DaynaPort suffers from being a subclass of Disk at least reject MODE SENSE and MODE SELECT - if (cmd == scsi_command::eCmdModeSense6 || cmd == scsi_command::eCmdModeSelect6 || - cmd == scsi_command::eCmdModeSense10 || cmd == scsi_command::eCmdModeSelect10) { - return false; - } - // The superclass class handles the less specific commands return dispatcher.Dispatch(this, cmd) ? true : super::Dispatch(cmd); } @@ -89,11 +78,6 @@ bool SCSIDaynaPort::Init(const unordered_map& params) return true; } -void SCSIDaynaPort::Open() -{ - m_tap.OpenDump(GetFilename().c_str()); -} - vector SCSIDaynaPort::InquiryInternal() const { vector buf = HandleInquiry(device_type::PROCESSOR, scsi_level::SCSI_2, false); @@ -145,8 +129,8 @@ int SCSIDaynaPort::Read(const vector& cdb, vector& buf, uint64_t) const int requested_length = cdb[4]; LOGTRACE("%s Read maximum length %d, (%04X)", __PRETTY_FUNCTION__, requested_length, requested_length) - // At host startup, it will send a READ(6) command with a length of 1. We should - // respond by going into the status mode with a code of 0x02 + // At startup the host may send a READ(6) command with a sector count of 1 to read the root sector. + // We should respond by going into the status mode with a code of 0x02. if (requested_length == 1) { return 0; } @@ -253,17 +237,6 @@ int SCSIDaynaPort::Read(const vector& cdb, vector& buf, uint64_t) return DAYNAPORT_READ_HEADER_SZ; } -int SCSIDaynaPort::WriteCheck(uint64_t) -{ - CheckReady(); - - if (!m_bTapEnable) { - throw scsi_exception(sense_key::UNIT_ATTENTION, asc::MEDIUM_NOT_PRESENT); - } - - return 1; -} - //--------------------------------------------------------------------------- // // Write @@ -282,7 +255,7 @@ int SCSIDaynaPort::WriteCheck(uint64_t) // XX XX ... is the actual packet // //--------------------------------------------------------------------------- -bool SCSIDaynaPort::WriteBytes(const vector& cdb, const vector& buf, uint64_t) +bool SCSIDaynaPort::WriteBytes(const vector& cdb, vector& buf, uint32_t) { const int data_format = cdb[5]; int data_length = GetInt16(cdb, 3); @@ -301,6 +274,8 @@ bool SCSIDaynaPort::WriteBytes(const vector& cdb, const vector& buf, LOGWARN("%s Unknown data format %02X", __PRETTY_FUNCTION__, data_format) } + controller->SetBlocks(0); + return true; } @@ -337,7 +312,7 @@ void SCSIDaynaPort::Read6() { // Get record number and block number const uint32_t record = GetInt24(ctrl->cmd, 1) & 0x1fffff; - ctrl->blocks=1; + controller->SetBlocks(1); // If any commands have a bogus control value, they were probably not // generated by the DaynaPort driver so ignore them @@ -346,10 +321,10 @@ void SCSIDaynaPort::Read6() throw scsi_exception(sense_key::ILLEGAL_REQUEST, asc::INVALID_FIELD_IN_CDB); } - LOGTRACE("%s READ(6) command record=%d blocks=%d", __PRETTY_FUNCTION__, record, ctrl->blocks) + LOGTRACE("%s READ(6) command record=%d blocks=%d", __PRETTY_FUNCTION__, record, controller->GetBlocks()) - ctrl->length = Read(ctrl->cmd, controller->GetBuffer(), record); - LOGTRACE("%s ctrl.length is %d", __PRETTY_FUNCTION__, ctrl->length) + controller->SetLength(Read(ctrl->cmd, controller->GetBuffer(), record)); + LOGTRACE("%s ctrl.length is %d", __PRETTY_FUNCTION__, controller->GetLength()) // Set next block ctrl->next = record + 1; @@ -365,22 +340,23 @@ void SCSIDaynaPort::Write6() const int data_format = ctrl->cmd[5]; if (data_format == 0x00) { - ctrl->length = GetInt16(ctrl->cmd, 3); + controller->SetLength(GetInt16(ctrl->cmd, 3)); } else if (data_format == 0x80) { - ctrl->length = GetInt16(ctrl->cmd, 3) + 8; + controller->SetLength(GetInt16(ctrl->cmd, 3) + 8); } else { LOGWARN("%s Unknown data format $%02X", __PRETTY_FUNCTION__, data_format) } - LOGTRACE("%s length: $%04X (%d) format: $%02X", __PRETTY_FUNCTION__, ctrl->length, ctrl->length, data_format) + LOGTRACE("%s length: $%04X (%d) format: $%02X", __PRETTY_FUNCTION__, controller->GetLength(), + controller->GetLength(), data_format) - if (ctrl->length <= 0) { + if (controller->GetLength() <= 0) { throw scsi_exception(sense_key::ILLEGAL_REQUEST, asc::INVALID_FIELD_IN_CDB); } // Set next block - ctrl->blocks = 1; + controller->SetBlocks(1); ctrl->next = 1; EnterDataOutPhase(); @@ -388,10 +364,10 @@ void SCSIDaynaPort::Write6() void SCSIDaynaPort::RetrieveStatistics() { - ctrl->length = RetrieveStats(ctrl->cmd, controller->GetBuffer()); + controller->SetLength(RetrieveStats(ctrl->cmd, controller->GetBuffer())); // Set next block - ctrl->blocks = 1; + controller->SetBlocks(1); ctrl->next = 1; EnterDataInPhase(); @@ -427,7 +403,7 @@ void SCSIDaynaPort::SetInterfaceMode() { // Check whether this command is telling us to "Set Interface Mode" or "Set MAC Address" - ctrl->length = RetrieveStats(ctrl->cmd, controller->GetBuffer()); + controller->SetLength(RetrieveStats(ctrl->cmd, controller->GetBuffer())); switch(ctrl->cmd[5]){ case CMD_SCSILINK_SETMODE: // TODO Not implemented, do nothing @@ -435,7 +411,7 @@ void SCSIDaynaPort::SetInterfaceMode() break; case CMD_SCSILINK_SETMAC: - ctrl->length = 6; + controller->SetLength(6); EnterDataOutPhase(); break; @@ -455,8 +431,8 @@ void SCSIDaynaPort::SetInterfaceMode() void SCSIDaynaPort::SetMcastAddr() { - ctrl->length = ctrl->cmd[4]; - if (ctrl->length == 0) { + controller->SetLength(ctrl->cmd[4]); + if (controller->GetLength() == 0) { LOGWARN("%s Not supported SetMcastAddr Command %02X", __PRETTY_FUNCTION__, ctrl->cmd[2]) throw scsi_exception(sense_key::ILLEGAL_REQUEST, asc::INVALID_FIELD_IN_CDB); @@ -502,3 +478,4 @@ void SCSIDaynaPort::EnableInterface() EnterStatusPhase(); } + diff --git a/cpp/devices/scsi_daynaport.h b/cpp/devices/scsi_daynaport.h index 11157a42..b6a24f05 100644 --- a/cpp/devices/scsi_daynaport.h +++ b/cpp/devices/scsi_daynaport.h @@ -29,8 +29,8 @@ #pragma once -#include "os.h" -#include "disk.h" +#include "interfaces/byte_writer.h" +#include "primary_device.h" #include "ctapdriver.h" #include #include @@ -41,7 +41,7 @@ // DaynaPort SCSI Link // //=========================================================================== -class SCSIDaynaPort : public Disk +class SCSIDaynaPort : public PrimaryDevice, public ByteWriter { public: @@ -49,19 +49,17 @@ public: ~SCSIDaynaPort() override = default; bool Init(const unordered_map&) override; - void Open() override; // Commands vector InquiryInternal() const override; - int Read(const vector&, vector&, uint64_t) override; - bool WriteBytes(const vector&, const vector&, uint64_t); - int WriteCheck(uint64_t block) override; + int Read(const vector&, vector&, uint64_t); + bool WriteBytes(const vector&, vector&, uint32_t) override; int RetrieveStats(const vector&, vector&) const; void TestUnitReady() override; - void Read6() override; - void Write6() override; + void Read6(); + void Write6(); void RetrieveStatistics(); void SetInterfaceMode(); void SetMcastAddr(); @@ -90,7 +88,7 @@ public: private: - using super = Disk; + using super = PrimaryDevice; Dispatcher dispatcher; diff --git a/cpp/devices/scsi_host_bridge.cpp b/cpp/devices/scsi_host_bridge.cpp index 3a1e7e88..cffced02 100644 --- a/cpp/devices/scsi_host_bridge.cpp +++ b/cpp/devices/scsi_host_bridge.cpp @@ -27,7 +27,7 @@ using namespace std; using namespace scsi_defs; using namespace scsi_command_util; -SCSIBR::SCSIBR(int lun) : Disk(SCBR, lun) +SCSIBR::SCSIBR(int lun) : PrimaryDevice(SCBR, lun) { // Create host file system fs.Reset(); @@ -37,10 +37,6 @@ SCSIBR::SCSIBR(int lun) : Disk(SCBR, lun) dispatcher.Add(scsi_command::eCmdWrite6, "SendMessage10", &SCSIBR::SendMessage10); SupportsParams(true); - // TODO Remove as soon as SCBR is not a subclass of Disk anymore - SetStoppable(false); - // TODO Remove as soon as SCBR is not a subclass of Disk anymore - SupportsFile(false); } bool SCSIBR::Init(const unordered_map& params) @@ -198,7 +194,7 @@ int SCSIBR::GetMessage10(const vector& cdb, vector& buf) return 0; } -bool SCSIBR::WriteBytes(const vector& cdb, vector& buf, uint64_t) +bool SCSIBR::WriteBytes(const vector& cdb, vector& buf, uint32_t) { // Type const int type = cdb[2]; @@ -261,13 +257,13 @@ void SCSIBR::GetMessage10() // Ensure a sufficient buffer size (because it is not a transfer for each block) controller->AllocateBuffer(0x1000000); - ctrl->length = GetMessage10(ctrl->cmd, controller->GetBuffer()); - if (ctrl->length <= 0) { + controller->SetLength(GetMessage10(ctrl->cmd, controller->GetBuffer())); + if (controller->GetLength() <= 0) { throw scsi_exception(sense_key::ILLEGAL_REQUEST, asc::INVALID_FIELD_IN_CDB); } // Set next block - ctrl->blocks = 1; + controller->SetBlocks(1); ctrl->next = 1; EnterDataInPhase(); @@ -282,8 +278,8 @@ void SCSIBR::GetMessage10() //--------------------------------------------------------------------------- void SCSIBR::SendMessage10() { - ctrl->length = GetInt24(ctrl->cmd, 6); - if (ctrl->length <= 0) { + controller->SetLength(GetInt24(ctrl->cmd, 6)); + if (controller->GetLength() <= 0) { throw scsi_exception(sense_key::ILLEGAL_REQUEST, asc::INVALID_FIELD_IN_CDB); } @@ -291,7 +287,7 @@ void SCSIBR::SendMessage10() controller->AllocateBuffer(0x1000000); // Set next block - ctrl->blocks = 1; + controller->SetBlocks(1); ctrl->next = 1; EnterDataOutPhase(); diff --git a/cpp/devices/scsi_host_bridge.h b/cpp/devices/scsi_host_bridge.h index 89d16396..08417ff8 100644 --- a/cpp/devices/scsi_host_bridge.h +++ b/cpp/devices/scsi_host_bridge.h @@ -17,8 +17,8 @@ //--------------------------------------------------------------------------- #pragma once -#include "os.h" -#include "disk.h" +#include "interfaces/byte_writer.h" +#include "primary_device.h" #include "ctapdriver.h" #include "cfilesystem.h" #include @@ -26,7 +26,7 @@ using namespace std; -class SCSIBR : public Disk +class SCSIBR : public PrimaryDevice, public ByteWriter { static constexpr const array bcast_addr = { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff }; @@ -38,20 +38,17 @@ public: bool Init(const unordered_map&) override; bool Dispatch(scsi_command) override; - // TODO Remove as soon as SCSIBR is not a subclass of Disk anymore - void Open() override { super::ValidateFile(); } - // Commands vector InquiryInternal() const override; int GetMessage10(const vector&, vector&); - bool WriteBytes(const vector&, vector&, uint64_t); + bool WriteBytes(const vector&, vector&, uint32_t) override; void TestUnitReady() override; void GetMessage10(); void SendMessage10(); private: - using super = Disk; + using super = PrimaryDevice; Dispatcher dispatcher; diff --git a/cpp/devices/scsi_printer.cpp b/cpp/devices/scsi_printer.cpp index 921206bb..e3612beb 100644 --- a/cpp/devices/scsi_printer.cpp +++ b/cpp/devices/scsi_printer.cpp @@ -109,7 +109,7 @@ void SCSIPrinter::Print() throw scsi_exception(sense_key::ILLEGAL_REQUEST, asc::INVALID_FIELD_IN_CDB); } - ctrl->length = length; + controller->SetLength(length); controller->SetByteTransfer(true); EnterDataOutPhase(); diff --git a/cpp/devices/scsicd.cpp b/cpp/devices/scsicd.cpp index 10188cc7..3e019044 100644 --- a/cpp/devices/scsicd.cpp +++ b/cpp/devices/scsicd.cpp @@ -162,7 +162,7 @@ void SCSICD::CreateDataTrack() void SCSICD::ReadToc() { - ctrl->length = ReadTocInternal(ctrl->cmd, controller->GetBuffer()); + controller->SetLength(ReadTocInternal(ctrl->cmd, controller->GetBuffer())); EnterDataInPhase(); } diff --git a/cpp/hal/gpiobus.cpp b/cpp/hal/gpiobus.cpp index 9d02e36a..0f6868f3 100644 --- a/cpp/hal/gpiobus.cpp +++ b/cpp/hal/gpiobus.cpp @@ -981,7 +981,7 @@ int GPIOBUS::GetCommandByteCount(BYTE opcode) return 16; } else if (opcode == 0xA0) { return 12; - } else if (opcode == 0x05 || (opcode >= 0x20 && opcode <= 0x7D)) { + } else if (opcode >= 0x20 && opcode <= 0x7D) { return 10; } else { return 6; diff --git a/cpp/hal/gpiobus.h b/cpp/hal/gpiobus.h index 39afc819..e3e81bba 100644 --- a/cpp/hal/gpiobus.h +++ b/cpp/hal/gpiobus.h @@ -13,6 +13,7 @@ #include "config.h" #include "scsi.h" +#include "bus.h" #include #ifdef __linux__ @@ -47,7 +48,7 @@ #define GPIO_FUNCTION_TRACE #endif -using namespace std; // NOSONAR Not relevant for rascsi +using namespace std; //--------------------------------------------------------------------------- // diff --git a/cpp/rascsi/rascsi_executor.cpp b/cpp/rascsi/rascsi_executor.cpp index b0da6570..1fb83ebb 100644 --- a/cpp/rascsi/rascsi_executor.cpp +++ b/cpp/rascsi/rascsi_executor.cpp @@ -319,12 +319,11 @@ bool RascsiExecutor::Attach(const CommandContext& context, const PbDeviceDefinit } if (device->SupportsParams() && !device->Init(params)) { - return context.ReturnLocalizedError(LocalizationKey::ERROR_INITIALIZATION, PbDeviceType_Name(type), + return context.ReturnLocalizedError(LocalizationKey::ERROR_INITIALIZATION, PbDeviceType_Name(device->GetType()), to_string(id), to_string(lun)); } - // Remove SupportsFile as soon as Daynaport and bridge do not inherit from Disk anymore - if (storage_device != nullptr && storage_device->SupportsFile()) { + if (storage_device != nullptr) { storage_device->ReserveFile(full_path, id, lun); } @@ -397,7 +396,6 @@ bool RascsiExecutor::Insert(const CommandContext& context, const PbDeviceDefinit bool RascsiExecutor::Detach(const CommandContext& context, shared_ptr device, bool dryRun) const { - cerr << "AAA " << device->GetId() << endl; auto controller = controller_manager.FindController(device->GetId()); if (controller == nullptr) { return context.ReturnLocalizedError(LocalizationKey::ERROR_DETACH); diff --git a/cpp/scsi.h b/cpp/scsi.h index a6e18717..749b9956 100644 --- a/cpp/scsi.h +++ b/cpp/scsi.h @@ -9,9 +9,6 @@ #pragma once -// TODO Remove this include as soon as gpiobus.cpp/h is open for editing (adding the include there) again -#include "bus.h" - namespace scsi_defs { enum class scsi_level : int { SCSI_1_CCS = 1, diff --git a/cpp/test/gpiobus_test.cpp b/cpp/test/gpiobus_test.cpp index 7a48c9c0..faaf920e 100644 --- a/cpp/test/gpiobus_test.cpp +++ b/cpp/test/gpiobus_test.cpp @@ -31,10 +31,6 @@ TEST(GpioBusTest, GetCommandByteCount) EXPECT_EQ(12, GPIOBUS::GetCommandByteCount(0xa0)); opcodes.insert(0xa0); - // TODO Opcode 0x05 must be removed from gpiobus.cpp - EXPECT_EQ(10, GPIOBUS::GetCommandByteCount(0x05)); - opcodes.insert(0x05); - for (int i = 0x20; i <= 0x7d; i++) { EXPECT_EQ(10, GPIOBUS::GetCommandByteCount(i)); opcodes.insert(i); diff --git a/cpp/test/rascsi_executor_test.cpp b/cpp/test/rascsi_executor_test.cpp index 18c55a71..7d2cd5fe 100644 --- a/cpp/test/rascsi_executor_test.cpp +++ b/cpp/test/rascsi_executor_test.cpp @@ -39,10 +39,10 @@ TEST_F(RascsiExecutorTest, ProcessDeviceCmd) const int ID = 3; const int LUN = 0; - shared_ptr bus_ptr = make_shared(); + auto bus = make_shared(); DeviceFactory device_factory; - MockAbstractController controller(bus_ptr, ID); - ControllerManager controller_manager(bus_ptr); + MockAbstractController controller(bus, ID); + ControllerManager controller_manager(bus); RascsiImage rascsi_image; RascsiResponse rascsi_response(device_factory, controller_manager, 32); auto executor = make_shared(rascsi_response, rascsi_image, device_factory, controller_manager); diff --git a/cpp/test/scsi_daynaport_test.cpp b/cpp/test/scsi_daynaport_test.cpp index c7d7c291..586691b6 100644 --- a/cpp/test/scsi_daynaport_test.cpp +++ b/cpp/test/scsi_daynaport_test.cpp @@ -57,15 +57,6 @@ TEST(ScsiDaynaportTest, Read) EXPECT_EQ(0, daynaport->Read(cmd, buf, 0)) << "Trying to read the root sector must fail"; } -TEST(ScsiDaynaportTest, WriteCheck) -{ - vector buf(0); - NiceMock controller(make_shared(), 0); - auto daynaport = dynamic_pointer_cast(CreateDevice(SCDP, controller)); - - EXPECT_THROW(daynaport->WriteCheck(0), scsi_exception); -} - TEST(ScsiDaynaportTest, WriteBytes) { vector buf(0);