Removed wrong inheritance of SCDP and SCBR from Disk class (#948)

* Fixed TODOs, updated SCBR and SCDP

* Introduced ByteWriter interface

* Use accessors instead of directly accessing length/block fields
This commit is contained in:
Uwe Seimet
2022-10-29 18:10:00 +02:00
committed by GitHub
parent c1f63c6745
commit 6e35577368
20 changed files with 148 additions and 169 deletions

View File

@@ -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 <sstream>
#include <iomanip>
@@ -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<ModePageDevice>(GetDeviceForLun(GetEffectiveLun()));
device != nullptr) {
device->ModeSelect(GetOpcode(), ctrl.cmd, GetBuffer(), GetOffset());
@@ -867,26 +865,33 @@ bool ScsiController::XferIn(vector<BYTE>& 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<Disk>(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<ModePageDevice>(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<SCSIBR>(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<ByteWriter>(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<SCSIDaynaPort>(disk); daynaport) {
daynaport->WriteBytes(ctrl.cmd, GetBuffer(), 0);
ResetOffset();
ctrl.blocks = 0;
break;
auto disk = dynamic_pointer_cast<Disk>(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;
}