Gabe Black has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/55523 )
Change subject: dev: Don't implement the ATAPI_IDENTIFY_DEVICE command.
......................................................................
dev: Don't implement the ATAPI_IDENTIFY_DEVICE command.
This command is one of two that should be implemented by ATAPI devices.
An ATAPI device are essentially optical devices which use SCSI commands
which are transported over ATA using two special commands, a PACKET
command which actually sends the SCSI commands, and an IDENTIFY command
which is basically the same as the ATA IDENTIFY command but which is
only implemented on ATAPI devices. In order to determine if the device
connected to an IDE controller is an optical drive or a regular ATA hard
drive, software can send the ATAPI_IDENTIFY_DEVICE command and see if
gets an appropriate response.
In gem5, this command was originally not implemented by the IDE disk
model. If a driver attempted to send it, the gem5 model would panic and
kill the simulation. To fix that problem, that command was added to the
list of supported commands and just made a synonym for the ATA IDENTIFY
command since they have essentially the same response.
Unfortunately, this makes all ATA devices look like ATAPI devices, which
is not what we have implemented.
Instead, when we get this command, what we *should* do, as far as I can
tell by reading this:
http://users.utcluj.ro/~baruch/media/siee/labor/ATA-Interface.pdf
is to set the ERR bit in the status register, and then set the ABRT bit
in the error register to indicate that the command was not implemented.
I've attempted to implement that into the model with this change by
setting those bits as described, and then setting the "action" member to
be ACT_CMD_ERROR. I think that action is there primarily to support
cancelled transfers, but it seems like it has the desired effect(?).
Since the error bits are not really explicitly set or managed by the
model in most cases, this change also adds a little bit of code at the
top of startCommand which clears them to zero. These bits are supposed
to "contain the status of the last command executed", and if we're
starting a new command, the error bits no longer apply.
I'm confident that conceptually this is how the ATAPI_IDENTIFY_DEVICE
command should behave in our model, at least unless we decide to
implement real ATAPI models which actually accept SCSI commands, etc.
I'm less confident that I've modified the model to actually implement
that behavior, but as far as I can tell it doesn't seem to have broken
anything, and now SeaBIOS no longer things our disk model is a CDROM
drive.
Change-Id: I2c0840e279e9caa89c21a4e7cbdbcaf6bccd92ac
---
M src/dev/storage/ide_disk.cc
1 file changed, 69 insertions(+), 2 deletions(-)
diff --git a/src/dev/storage/ide_disk.cc b/src/dev/storage/ide_disk.cc
index a7185e4..ceb3fd7 100644
--- a/src/dev/storage/ide_disk.cc
+++ b/src/dev/storage/ide_disk.cc
@@ -49,6 +49,7 @@
#include <deque>
#include <string>
+#include "base/bitfield.hh"
#include "base/chunk_generator.hh"
#include "base/compiler.hh"
#include "base/cprintf.hh" // csprintf
@@ -615,6 +616,10 @@
uint32_t size = 0;
dmaRead = false;
+ // Clear any existing errors.
+ replaceBits(status, 0, 0);
+ replaceBits(cmdReg.error, 2, 0);
+
// Decode commands
switch (cmdReg.command) {
// Supported non-data commands
@@ -644,12 +649,19 @@
// Supported PIO data-in commands
case WDCC_IDENTIFY:
- case ATAPI_IDENTIFY_DEVICE:
cmdBytes = cmdBytesLeft = sizeof(struct ataparams);
devState = Prepare_Data_In;
action = ACT_DATA_READY;
break;
+ case ATAPI_IDENTIFY_DEVICE:
+ // We're not an ATAPI device, so this command isn't implemented.
+ devState = Command_Execution;
+ action = ACT_CMD_ERROR;
+ replaceBits(cmdReg.error, 2, 1);
+ replaceBits(status, 0, 1);
+ break;
+
case WDCC_READMULTI:
case WDCC_READ:
if (!(cmdReg.drive & DRIVE_LBA_BIT))
@@ -810,7 +822,7 @@
break;
case Command_Execution:
- if (action == ACT_CMD_COMPLETE) {
+ if (action == ACT_CMD_ERROR || action == ACT_CMD_COMPLETE) {
// clear the BSY bit
setComplete();
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/55523
To unsubscribe, or for help writing mail filters, visit
https://gem5-review.googlesource.com/settings
Gerrit-Project: public/gem5
Gerrit-Branch: develop
Gerrit-Change-Id: I2c0840e279e9caa89c21a4e7cbdbcaf6bccd92ac
Gerrit-Change-Number: 55523
Gerrit-PatchSet: 1
Gerrit-Owner: Gabe Black <[email protected]>
Gerrit-MessageType: newchange
_______________________________________________
gem5-dev mailing list -- [email protected]
To unsubscribe send an email to [email protected]
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
