Cleaned up dependencies on controller manager (#964)

* Cleaned up dependencies on controller manager

* Removed global fields

* Simplified setting up RascsiResponse and RascsiExecutor

* Got rid of remaining raw pointers

* Use references instead of pointers

* Improved encapsulation

cpp/controllers/abstract_controller.cpp

@@ -117,7 +117,7 @@ bool AbstractController::AddDevice(shared_ptr<PrimaryDevice> device)
 	}
 
 	luns[device->GetLun()] = device;
-	device->SetController(this);
+	device->SetController(shared_from_this());
 
 	return true;
 }

cpp/controllers/abstract_controller.h

@@ -14,6 +14,7 @@
 #include "scsi.h"
 #include "hal/bus.h"
 #include "phase_handler.h"
+#include "controller_manager.h"
 #include <unordered_set>
 #include <unordered_map>
 #include <vector>
@@ -23,7 +24,7 @@ using namespace std;
 
 class PrimaryDevice;
 
-class AbstractController : public PhaseHandler
+class AbstractController : public PhaseHandler, public enable_shared_from_this<AbstractController>
 {
 public:
 
@@ -34,7 +35,8 @@ public:
 		RESTART_PI
 	};
 
-	AbstractController(shared_ptr<BUS> bus, int target_id, int max_luns) : target_id(target_id), bus(bus), max_luns(max_luns) {}
+	AbstractController(shared_ptr<ControllerManager> controller_manager, int target_id, int max_luns)
+		: controller_manager(controller_manager), target_id(target_id), max_luns(max_luns) {}
 	~AbstractController() override = default;
 
 	virtual void Error(scsi_defs::sense_key, scsi_defs::asc = scsi_defs::asc::NO_ADDITIONAL_SENSE_INFORMATION,
@@ -82,7 +84,8 @@ public:
 
 protected:
 
-	inline shared_ptr<BUS> GetBus() const { return bus; }
+	shared_ptr<ControllerManager> GetControllerManager() const { return controller_manager.lock(); }
+	inline BUS& GetBus() const { return controller_manager.lock()->GetBus(); }
 
 	scsi_defs::scsi_command GetOpcode() const { return static_cast<scsi_defs::scsi_command>(ctrl.cmd[0]); }
 	int GetLun() const { return (ctrl.cmd[1] >> 5) & 0x07; }
@@ -116,13 +119,13 @@ private:
 
 	ctrl_t ctrl = {};
 
+	weak_ptr<ControllerManager> controller_manager;
+
 	// Logical units of this controller mapped to their LUN numbers
 	unordered_map<int, shared_ptr<PrimaryDevice>> luns;
 
 	int target_id;
 
-	shared_ptr<BUS> bus;
-
 	int max_luns;
 
 	bool is_byte_transfer = false;

cpp/controllers/controller_manager.cpp

@@ -23,7 +23,7 @@ bool ControllerManager::AttachToScsiController(int id, shared_ptr<PrimaryDevice>
 
 	// If there is no LUN yet the first LUN must be LUN 0
 	if (device->GetLun() == 0) {
-		controller = make_shared<ScsiController>(bus, id);
+		controller = make_shared<ScsiController>(shared_from_this(), id);
 
 		if (controller->AddDevice(device)) {
 			controllers[id] = controller;
@@ -74,13 +74,6 @@ void ControllerManager::DeleteAllControllers()
 	controllers.clear();
 }
 
-void ControllerManager::ResetAllControllers() const
-{
-	for (const auto& [id, controller] : controllers) {
-		controller->Reset();
-	}
-}
-
 shared_ptr<PrimaryDevice> ControllerManager::GetDeviceByIdAndLun(int id, int lun) const
 {
 	if (const auto& controller = FindController(id); controller != nullptr) {

cpp/controllers/controller_manager.h

@@ -21,26 +21,26 @@ class BUS;
 class AbstractController;
 class PrimaryDevice;
 
-class ControllerManager
+class ControllerManager : public enable_shared_from_this<ControllerManager>
 {
-	shared_ptr<BUS> bus;
+	BUS& bus;
 
 	unordered_map<int, shared_ptr<AbstractController>> controllers;
 
 public:
 
-	explicit ControllerManager(shared_ptr<BUS> bus) : bus(bus) {}
+	explicit ControllerManager(BUS& bus) : bus(bus) {}
 	~ControllerManager() = default;
 
 	// Maximum number of controller devices
 	static const int DEVICE_MAX = 8;
 
+	inline BUS& GetBus() const { return bus; }
 	bool AttachToScsiController(int, shared_ptr<PrimaryDevice>);
 	bool DeleteController(shared_ptr<AbstractController>);
 	shared_ptr<AbstractController> IdentifyController(int) const;
 	shared_ptr<AbstractController> FindController(int) const;
 	unordered_set<shared_ptr<PrimaryDevice>> GetAllDevices() const;
 	void DeleteAllControllers();
-	void ResetAllControllers() const;
 	shared_ptr<PrimaryDevice> GetDeviceByIdAndLun(int, int) const;
 };

cpp/controllers/scsi_controller.cpp

@@ -28,7 +28,8 @@
 
 using namespace scsi_defs;
 
-ScsiController::ScsiController(shared_ptr<BUS> bus, int target_id) : AbstractController(bus, target_id, LUN_MAX)
+ScsiController::ScsiController(shared_ptr<ControllerManager> controller_manager, int target_id)
+	: AbstractController(controller_manager, target_id, LUN_MAX)
 {
 	// The initial buffer size will default to either the default buffer size OR
 	// the size of an Ethernet message, whichever is larger.
@@ -51,18 +52,14 @@ void ScsiController::Reset()
 
 BUS::phase_t ScsiController::Process(int id)
 {
-	// Get bus information
-	GetBus()->Acquire();
+	GetBus().Acquire();
 
-	// Check to see if the reset signal was asserted
-	if (GetBus()->GetRST()) {
+	if (GetBus().GetRST()) {
 		LOGWARN("RESET signal received!")
 
-		// Reset the controller
 		Reset();
 
-		// Reset the bus
-		GetBus()->Reset();
+		GetBus().Reset();
 
 		return GetPhase();
 	}
@@ -84,7 +81,7 @@ BUS::phase_t ScsiController::Process(int id)
 		LOGERROR("%s Unhandled SCSI error, resetting controller and bus and entering bus free phase", __PRETTY_FUNCTION__)
 
 		Reset();
-		GetBus()->Reset();
+		GetBus().Reset();
 
 		BusFree();
 	}
@@ -99,11 +96,11 @@ void ScsiController::BusFree()
 
 		SetPhase(BUS::phase_t::busfree);
 
-		GetBus()->SetREQ(false);
-		GetBus()->SetMSG(false);
-		GetBus()->SetCD(false);
-		GetBus()->SetIO(false);
-		GetBus()->SetBSY(false);
+		GetBus().SetREQ(false);
+		GetBus().SetMSG(false);
+		GetBus().SetCD(false);
+		GetBus().SetIO(false);
+		GetBus().SetBSY(false);
 
 		// Initialize status and message
 		SetStatus(status::GOOD);
@@ -116,6 +113,13 @@ void ScsiController::BusFree()
 
 		SetByteTransfer(false);
 
+		if (shutdown_mode != rascsi_shutdown_mode::NONE) {
+			// Prepare the shutdown by flushing all caches
+			for (const auto& device : GetControllerManager()->GetAllDevices()) {
+				device->FlushCache();
+			}
+		}
+
 		// When the bus is free RaSCSI or the Pi may be shut down.
 		// This code has to be executed in the bus free phase and thus has to be located in the controller.
 		switch(shutdown_mode) {
@@ -146,7 +150,7 @@ void ScsiController::BusFree()
 	}
 
 	// Move to selection phase
-	if (GetBus()->GetSEL() && !GetBus()->GetBSY()) {
+	if (GetBus().GetSEL() && !GetBus().GetBSY()) {
 		Selection();
 	}
 }
@@ -155,7 +159,7 @@ void ScsiController::Selection()
 {
 	if (!IsSelection()) {
 		// A different device controller was selected
-		if (int id = 1 << GetTargetId(); (static_cast<int>(GetBus()->GetDAT()) & id) == 0) {
+		if (int id = 1 << GetTargetId(); (static_cast<int>(GetBus().GetDAT()) & id) == 0) {
 			return;
 		}
 
@@ -169,14 +173,14 @@ void ScsiController::Selection()
 		SetPhase(BUS::phase_t::selection);
 
 		// Raise BSY and respond
-		GetBus()->SetBSY(true);
+		GetBus().SetBSY(true);
 		return;
 	}
 
 	// Selection completed
-	if (!GetBus()->GetSEL() && GetBus()->GetBSY()) {
+	if (!GetBus().GetSEL() && GetBus().GetBSY()) {
 		// Message out phase if ATN=1, otherwise command phase
-		if (GetBus()->GetATN()) {
+		if (GetBus().GetATN()) {
 			MsgOut();
 		} else {
 			Command();
@@ -191,11 +195,11 @@ void ScsiController::Command()
 
 		SetPhase(BUS::phase_t::command);
 
-		GetBus()->SetMSG(false);
-		GetBus()->SetCD(true);
-		GetBus()->SetIO(false);
+		GetBus().SetMSG(false);
+		GetBus().SetCD(true);
+		GetBus().SetIO(false);
 
-		const int actual_count = GetBus()->CommandHandShake(GetBuffer().data());
+		const int actual_count = GetBus().CommandHandShake(GetBuffer().data());
 		const int command_byte_count = BUS::GetCommandByteCount(GetBuffer()[0]);
 
 		// If not able to receive all, move to the status phase
@@ -295,9 +299,9 @@ void ScsiController::Status()
 		SetPhase(BUS::phase_t::status);
 
 		// Signal line operated by the target
-		GetBus()->SetMSG(false);
-		GetBus()->SetCD(true);
-		GetBus()->SetIO(true);
+		GetBus().SetMSG(false);
+		GetBus().SetCD(true);
+		GetBus().SetIO(true);
 
 		// Data transfer is 1 byte x 1 block
 		ResetOffset();
@@ -318,9 +322,9 @@ void ScsiController::MsgIn()
 
 		SetPhase(BUS::phase_t::msgin);
 
-		GetBus()->SetMSG(true);
-		GetBus()->SetCD(true);
-		GetBus()->SetIO(true);
+		GetBus().SetMSG(true);
+		GetBus().SetCD(true);
+		GetBus().SetIO(true);
 
 		ResetOffset();
 		return;
@@ -345,9 +349,9 @@ void ScsiController::MsgOut()
 
 		SetPhase(BUS::phase_t::msgout);
 
-		GetBus()->SetMSG(true);
-		GetBus()->SetCD(true);
-		GetBus()->SetIO(false);
+		GetBus().SetMSG(true);
+		GetBus().SetCD(true);
+		GetBus().SetIO(false);
 
 		// Data transfer is 1 byte x 1 block
 		ResetOffset();
@@ -378,9 +382,9 @@ void ScsiController::DataIn()
 
 		SetPhase(BUS::phase_t::datain);
 
-		GetBus()->SetMSG(false);
-		GetBus()->SetCD(false);
-		GetBus()->SetIO(true);
+		GetBus().SetMSG(false);
+		GetBus().SetCD(false);
+		GetBus().SetIO(true);
 
 		ResetOffset();
 
@@ -409,9 +413,9 @@ void ScsiController::DataOut()
 		SetPhase(BUS::phase_t::dataout);
 
 		// Signal line operated by the target
-		GetBus()->SetMSG(false);
-		GetBus()->SetCD(false);
-		GetBus()->SetIO(false);
+		GetBus().SetMSG(false);
+		GetBus().SetCD(false);
+		GetBus().SetIO(false);
 
 		ResetOffset();
 		return;
@@ -423,12 +427,12 @@ void ScsiController::DataOut()
 void ScsiController::Error(sense_key sense_key, asc asc, status status)
 {
 	// Get bus information
-	GetBus()->Acquire();
+	GetBus().Acquire();
 
 	// Reset check
-	if (GetBus()->GetRST()) {
+	if (GetBus().GetRST()) {
 		Reset();
-		GetBus()->Reset();
+		GetBus().Reset();
 
 		return;
 	}
@@ -472,8 +476,8 @@ void ScsiController::Error(sense_key sense_key, asc asc, status status)
 
 void ScsiController::Send()
 {
-	assert(!GetBus()->GetREQ());
-	assert(GetBus()->GetIO());
+	assert(!GetBus().GetREQ());
+	assert(GetBus().GetIO());
 
 	if (HasValidLength()) {
 		LOGTRACE("%s%s", __PRETTY_FUNCTION__, (" Sending handhake with offset " + to_string(GetOffset()) + ", length "
@@ -481,7 +485,7 @@ void ScsiController::Send()
 
 		// The delay should be taken from the respective LUN, but as there are no Daynaport drivers for
 		// LUNs other than 0 this work-around works.
-		if (const int len = GetBus()->SendHandShake(GetBuffer().data() + GetOffset(), GetLength(),
+		if (const int len = GetBus().SendHandShake(GetBuffer().data() + GetOffset(), GetLength(),
 				HasDeviceForLun(0) ? GetDeviceForLun(0)->GetSendDelay() : 0);
 			len != static_cast<int>(GetLength())) {
 			// If you cannot send all, move to status phase
@@ -568,15 +572,15 @@ void ScsiController::Receive()
 	LOGTRACE("%s",__PRETTY_FUNCTION__)
 
 	// REQ is low
-	assert(!GetBus()->GetREQ());
-	assert(!GetBus()->GetIO());
+	assert(!GetBus().GetREQ());
+	assert(!GetBus().GetIO());
 
 	// Length != 0 if received
 	if (HasValidLength()) {
 		LOGTRACE("%s Length is %d bytes", __PRETTY_FUNCTION__, GetLength())
 
 		// If not able to receive all, move to status phase
-		if (int len = GetBus()->ReceiveHandShake(GetBuffer().data() + GetOffset(), GetLength());
+		if (int len = GetBus().ReceiveHandShake(GetBuffer().data() + GetOffset(), GetLength());
 			len != static_cast<int>(GetLength())) {
 			LOGERROR("%s Not able to receive %d bytes of data, only received %d",__PRETTY_FUNCTION__, GetLength(), len)
 			Error(sense_key::ABORTED_COMMAND);
@@ -673,14 +677,14 @@ bool ScsiController::XferMsg(int msg)
 
 void ScsiController::ReceiveBytes()
 {
-	assert(!GetBus()->GetREQ());
-	assert(!GetBus()->GetIO());
+	assert(!GetBus().GetREQ());
+	assert(!GetBus().GetIO());
 
 	if (HasValidLength()) {
 		LOGTRACE("%s Length is %d bytes", __PRETTY_FUNCTION__, GetLength())
 
 		// If not able to receive all, move to status phase
-		if (uint32_t len = GetBus()->ReceiveHandShake(GetBuffer().data() + GetOffset(), GetLength()); len != GetLength()) {
+		if (uint32_t len = GetBus().ReceiveHandShake(GetBuffer().data() + GetOffset(), GetLength()); len != GetLength()) {
 			LOGERROR("%s Not able to receive %d bytes of data, only received %d",
 					__PRETTY_FUNCTION__, GetLength(), len)
 			Error(sense_key::ABORTED_COMMAND);
@@ -1014,7 +1018,7 @@ void ScsiController::ParseMessage()
 void ScsiController::ProcessMessage()
 {
 	// Continue message out phase as long as ATN keeps asserting
-	if (GetBus()->GetATN()) {
+	if (GetBus().GetATN()) {
 		// Data transfer is 1 byte x 1 block
 		ResetOffset();
 		SetLength(1);

cpp/controllers/scsi_controller.h

@@ -14,6 +14,7 @@
 
 #pragma once
 
+#include "controller_manager.h"
 #include "abstract_controller.h"
 #include "scsi.h"
 #include <array>
@@ -53,7 +54,7 @@ public:
 	// Maximum number of logical units
 	static inline const int LUN_MAX = 32;
 
-	ScsiController(shared_ptr<BUS>, int);
+	explicit ScsiController(shared_ptr<ControllerManager>, int);
 	~ScsiController() override = default;
 
 	void Reset() override;