dev: Fix race conditions in IDE device on newer kernels
Newer linux kernels and distros exercise more functionality in the IDE device than previously, exposing 2 races. The first race is the handling of aborted DMA commands would immediately report the device is ready back to the kernel and cause already in flight commands to assert the simulator when they returned and discovered an inconsitent device state. The second race was due to the Status register not being handled correctly, the interrupt status bit would get stuck at 1 and the driver eventually views this as a bad state and logs the condition to the terminal. This patch fixes these two conditions by making the device handle aborted commands gracefully and properly handles clearing the interrupt status bit in the Status register.
This commit is contained in:
parent
fb0496498d
commit
be4aa2b6ba
5 changed files with 120 additions and 16 deletions
|
@ -1,4 +1,16 @@
|
||||||
/*
|
/*
|
||||||
|
* Copyright (c) 2013 ARM Limited
|
||||||
|
* All rights reserved
|
||||||
|
*
|
||||||
|
* The license below extends only to copyright in the software and shall
|
||||||
|
* not be construed as granting a license to any other intellectual
|
||||||
|
* property including but not limited to intellectual property relating
|
||||||
|
* to a hardware implementation of the functionality of the software
|
||||||
|
* licensed hereunder. You may use the software subject to the license
|
||||||
|
* terms below provided that you ensure that this notice is replicated
|
||||||
|
* unmodified and in its entirety in all distributions of the software,
|
||||||
|
* modified or unmodified, in source code or in binary form.
|
||||||
|
*
|
||||||
* Copyright (c) 2004-2005 The Regents of The University of Michigan
|
* Copyright (c) 2004-2005 The Regents of The University of Michigan
|
||||||
* All rights reserved.
|
* All rights reserved.
|
||||||
*
|
*
|
||||||
|
@ -111,11 +123,11 @@ IdeController::IdeController(Params *p)
|
||||||
|
|
||||||
if ((BARAddrs[0] & ~BAR_IO_MASK) && (!legacyIO[0] || ioShift)) {
|
if ((BARAddrs[0] & ~BAR_IO_MASK) && (!legacyIO[0] || ioShift)) {
|
||||||
primary.cmdAddr = BARAddrs[0]; primary.cmdSize = BARSize[0];
|
primary.cmdAddr = BARAddrs[0]; primary.cmdSize = BARSize[0];
|
||||||
primary.ctrlAddr = BARAddrs[1]; primary.ctrlSize = BARAddrs[1];
|
primary.ctrlAddr = BARAddrs[1]; primary.ctrlSize = BARSize[1];
|
||||||
}
|
}
|
||||||
if ((BARAddrs[2] & ~BAR_IO_MASK) && (!legacyIO[2] || ioShift)) {
|
if ((BARAddrs[2] & ~BAR_IO_MASK) && (!legacyIO[2] || ioShift)) {
|
||||||
secondary.cmdAddr = BARAddrs[2]; secondary.cmdSize = BARSize[2];
|
secondary.cmdAddr = BARAddrs[2]; secondary.cmdSize = BARSize[2];
|
||||||
secondary.ctrlAddr = BARAddrs[3]; secondary.ctrlSize = BARAddrs[3];
|
secondary.ctrlAddr = BARAddrs[3]; secondary.ctrlSize = BARSize[3];
|
||||||
}
|
}
|
||||||
|
|
||||||
ioEnabled = (config.command & htole(PCI_CMD_IOSE));
|
ioEnabled = (config.command & htole(PCI_CMD_IOSE));
|
||||||
|
@ -414,14 +426,21 @@ IdeController::Channel::accessBMI(Addr offset,
|
||||||
newVal.active = oldVal.active;
|
newVal.active = oldVal.active;
|
||||||
|
|
||||||
// to reset (set 0) IDEINTS and IDEDMAE, write 1 to each
|
// to reset (set 0) IDEINTS and IDEDMAE, write 1 to each
|
||||||
if (oldVal.intStatus && newVal.intStatus)
|
if ((oldVal.intStatus == 1) && (newVal.intStatus == 1)) {
|
||||||
newVal.intStatus = 0; // clear the interrupt?
|
newVal.intStatus = 0; // clear the interrupt?
|
||||||
else
|
} else {
|
||||||
newVal.intStatus = oldVal.intStatus;
|
// Assigning two bitunion fields to each other does not
|
||||||
if (oldVal.dmaError && newVal.dmaError)
|
// work as intended, so we need to use this temporary variable
|
||||||
|
// to get around the bug.
|
||||||
|
uint8_t tmp = oldVal.intStatus;
|
||||||
|
newVal.intStatus = tmp;
|
||||||
|
}
|
||||||
|
if ((oldVal.dmaError == 1) && (newVal.dmaError == 1)) {
|
||||||
newVal.dmaError = 0;
|
newVal.dmaError = 0;
|
||||||
else
|
} else {
|
||||||
newVal.dmaError = oldVal.dmaError;
|
uint8_t tmp = oldVal.dmaError;
|
||||||
|
newVal.dmaError = tmp;
|
||||||
|
}
|
||||||
|
|
||||||
bmiRegs.status = newVal;
|
bmiRegs.status = newVal;
|
||||||
}
|
}
|
||||||
|
|
|
@ -1,4 +1,16 @@
|
||||||
/*
|
/*
|
||||||
|
* Copyright (c) 2013 ARM Limited
|
||||||
|
* All rights reserved
|
||||||
|
*
|
||||||
|
* The license below extends only to copyright in the software and shall
|
||||||
|
* not be construed as granting a license to any other intellectual
|
||||||
|
* property including but not limited to intellectual property relating
|
||||||
|
* to a hardware implementation of the functionality of the software
|
||||||
|
* licensed hereunder. You may use the software subject to the license
|
||||||
|
* terms below provided that you ensure that this notice is replicated
|
||||||
|
* unmodified and in its entirety in all distributions of the software,
|
||||||
|
* modified or unmodified, in source code or in binary form.
|
||||||
|
*
|
||||||
* Copyright (c) 2004-2005 The Regents of The University of Michigan
|
* Copyright (c) 2004-2005 The Regents of The University of Michigan
|
||||||
* All rights reserved.
|
* All rights reserved.
|
||||||
*
|
*
|
||||||
|
@ -143,6 +155,7 @@ IdeDisk::reset(int id)
|
||||||
drqBytesLeft = 0;
|
drqBytesLeft = 0;
|
||||||
dmaRead = false;
|
dmaRead = false;
|
||||||
intrPending = false;
|
intrPending = false;
|
||||||
|
dmaAborted = false;
|
||||||
|
|
||||||
// set the device state to idle
|
// set the device state to idle
|
||||||
dmaState = Dma_Idle;
|
dmaState = Dma_Idle;
|
||||||
|
@ -319,6 +332,12 @@ IdeDisk::writeControl(const Addr offset, int size, const uint8_t *data)
|
||||||
void
|
void
|
||||||
IdeDisk::doDmaTransfer()
|
IdeDisk::doDmaTransfer()
|
||||||
{
|
{
|
||||||
|
if (dmaAborted) {
|
||||||
|
DPRINTF(IdeDisk, "DMA Aborted before reading PRD entry\n");
|
||||||
|
updateState(ACT_CMD_ERROR);
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
if (dmaState != Dma_Transfer || devState != Transfer_Data_Dma)
|
if (dmaState != Dma_Transfer || devState != Transfer_Data_Dma)
|
||||||
panic("Inconsistent DMA transfer state: dmaState = %d devState = %d\n",
|
panic("Inconsistent DMA transfer state: dmaState = %d devState = %d\n",
|
||||||
dmaState, devState);
|
dmaState, devState);
|
||||||
|
@ -334,6 +353,11 @@ IdeDisk::doDmaTransfer()
|
||||||
void
|
void
|
||||||
IdeDisk::dmaPrdReadDone()
|
IdeDisk::dmaPrdReadDone()
|
||||||
{
|
{
|
||||||
|
if (dmaAborted) {
|
||||||
|
DPRINTF(IdeDisk, "DMA Aborted while reading PRD entry\n");
|
||||||
|
updateState(ACT_CMD_ERROR);
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
DPRINTF(IdeDisk,
|
DPRINTF(IdeDisk,
|
||||||
"PRD: baseAddr:%#x (%#x) byteCount:%d (%d) eot:%#x sector:%d\n",
|
"PRD: baseAddr:%#x (%#x) byteCount:%d (%d) eot:%#x sector:%d\n",
|
||||||
|
@ -396,6 +420,14 @@ IdeDisk::regStats()
|
||||||
void
|
void
|
||||||
IdeDisk::doDmaRead()
|
IdeDisk::doDmaRead()
|
||||||
{
|
{
|
||||||
|
if (dmaAborted) {
|
||||||
|
DPRINTF(IdeDisk, "DMA Aborted in middle of Dma Read\n");
|
||||||
|
if (dmaReadCG)
|
||||||
|
delete dmaReadCG;
|
||||||
|
dmaReadCG = NULL;
|
||||||
|
updateState(ACT_CMD_ERROR);
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
if (!dmaReadCG) {
|
if (!dmaReadCG) {
|
||||||
// clear out the data buffer
|
// clear out the data buffer
|
||||||
|
@ -427,10 +459,8 @@ IdeDisk::doDmaRead()
|
||||||
void
|
void
|
||||||
IdeDisk::dmaReadDone()
|
IdeDisk::dmaReadDone()
|
||||||
{
|
{
|
||||||
|
|
||||||
uint32_t bytesWritten = 0;
|
uint32_t bytesWritten = 0;
|
||||||
|
|
||||||
|
|
||||||
// write the data to the disk image
|
// write the data to the disk image
|
||||||
for (bytesWritten = 0; bytesWritten < curPrd.getByteCount();
|
for (bytesWritten = 0; bytesWritten < curPrd.getByteCount();
|
||||||
bytesWritten += SectorSize) {
|
bytesWritten += SectorSize) {
|
||||||
|
@ -475,7 +505,14 @@ IdeDisk::doDmaDataWrite()
|
||||||
void
|
void
|
||||||
IdeDisk::doDmaWrite()
|
IdeDisk::doDmaWrite()
|
||||||
{
|
{
|
||||||
DPRINTF(IdeDisk, "doDmaWrite: rescheduling\n");
|
if (dmaAborted) {
|
||||||
|
DPRINTF(IdeDisk, "DMA Aborted while doing DMA Write\n");
|
||||||
|
if (dmaWriteCG)
|
||||||
|
delete dmaWriteCG;
|
||||||
|
dmaWriteCG = NULL;
|
||||||
|
updateState(ACT_CMD_ERROR);
|
||||||
|
return;
|
||||||
|
}
|
||||||
if (!dmaWriteCG) {
|
if (!dmaWriteCG) {
|
||||||
// clear out the data buffer
|
// clear out the data buffer
|
||||||
dmaWriteCG = new ChunkGenerator(curPrd.getBaseAddr(),
|
dmaWriteCG = new ChunkGenerator(curPrd.getBaseAddr(),
|
||||||
|
@ -980,7 +1017,10 @@ IdeDisk::updateState(DevAction_t action)
|
||||||
break;
|
break;
|
||||||
|
|
||||||
case Transfer_Data_Dma:
|
case Transfer_Data_Dma:
|
||||||
if (action == ACT_CMD_ERROR || action == ACT_DMA_DONE) {
|
if (action == ACT_CMD_ERROR) {
|
||||||
|
dmaAborted = true;
|
||||||
|
devState = Device_Dma_Abort;
|
||||||
|
} else if (action == ACT_DMA_DONE) {
|
||||||
// clear the BSY bit
|
// clear the BSY bit
|
||||||
setComplete();
|
setComplete();
|
||||||
// set the seek bit
|
// set the seek bit
|
||||||
|
@ -997,6 +1037,25 @@ IdeDisk::updateState(DevAction_t action)
|
||||||
}
|
}
|
||||||
break;
|
break;
|
||||||
|
|
||||||
|
case Device_Dma_Abort:
|
||||||
|
if (action == ACT_CMD_ERROR) {
|
||||||
|
setComplete();
|
||||||
|
status |= STATUS_SEEK_BIT;
|
||||||
|
ctrl->setDmaComplete(this);
|
||||||
|
dmaAborted = false;
|
||||||
|
dmaState = Dma_Idle;
|
||||||
|
|
||||||
|
if (!isIENSet()) {
|
||||||
|
devState = Device_Idle_SI;
|
||||||
|
intrPost();
|
||||||
|
} else {
|
||||||
|
devState = Device_Idle_S;
|
||||||
|
}
|
||||||
|
} else {
|
||||||
|
DPRINTF(IdeDisk, "Disk still busy aborting previous DMA command\n");
|
||||||
|
}
|
||||||
|
break;
|
||||||
|
|
||||||
default:
|
default:
|
||||||
panic("Unknown IDE device state: %#x\n", devState);
|
panic("Unknown IDE device state: %#x\n", devState);
|
||||||
}
|
}
|
||||||
|
@ -1074,6 +1133,7 @@ IdeDisk::serialize(ostream &os)
|
||||||
SERIALIZE_SCALAR(curSector);
|
SERIALIZE_SCALAR(curSector);
|
||||||
SERIALIZE_SCALAR(dmaRead);
|
SERIALIZE_SCALAR(dmaRead);
|
||||||
SERIALIZE_SCALAR(intrPending);
|
SERIALIZE_SCALAR(intrPending);
|
||||||
|
SERIALIZE_SCALAR(dmaAborted);
|
||||||
SERIALIZE_ENUM(devState);
|
SERIALIZE_ENUM(devState);
|
||||||
SERIALIZE_ENUM(dmaState);
|
SERIALIZE_ENUM(dmaState);
|
||||||
SERIALIZE_ARRAY(dataBuffer, MAX_DMA_SIZE);
|
SERIALIZE_ARRAY(dataBuffer, MAX_DMA_SIZE);
|
||||||
|
@ -1126,6 +1186,7 @@ IdeDisk::unserialize(Checkpoint *cp, const string §ion)
|
||||||
UNSERIALIZE_SCALAR(curSector);
|
UNSERIALIZE_SCALAR(curSector);
|
||||||
UNSERIALIZE_SCALAR(dmaRead);
|
UNSERIALIZE_SCALAR(dmaRead);
|
||||||
UNSERIALIZE_SCALAR(intrPending);
|
UNSERIALIZE_SCALAR(intrPending);
|
||||||
|
UNSERIALIZE_SCALAR(dmaAborted);
|
||||||
UNSERIALIZE_ENUM(devState);
|
UNSERIALIZE_ENUM(devState);
|
||||||
UNSERIALIZE_ENUM(dmaState);
|
UNSERIALIZE_ENUM(dmaState);
|
||||||
UNSERIALIZE_ARRAY(dataBuffer, MAX_DMA_SIZE);
|
UNSERIALIZE_ARRAY(dataBuffer, MAX_DMA_SIZE);
|
||||||
|
|
|
@ -1,4 +1,16 @@
|
||||||
/*
|
/*
|
||||||
|
* Copyright (c) 2013 ARM Limited
|
||||||
|
* All rights reserved
|
||||||
|
*
|
||||||
|
* The license below extends only to copyright in the software and shall
|
||||||
|
* not be construed as granting a license to any other intellectual
|
||||||
|
* property including but not limited to intellectual property relating
|
||||||
|
* to a hardware implementation of the functionality of the software
|
||||||
|
* licensed hereunder. You may use the software subject to the license
|
||||||
|
* terms below provided that you ensure that this notice is replicated
|
||||||
|
* unmodified and in its entirety in all distributions of the software,
|
||||||
|
* modified or unmodified, in source code or in binary form.
|
||||||
|
*
|
||||||
* Copyright (c) 2004-2005 The Regents of The University of Michigan
|
* Copyright (c) 2004-2005 The Regents of The University of Michigan
|
||||||
* All rights reserved.
|
* All rights reserved.
|
||||||
*
|
*
|
||||||
|
@ -177,7 +189,8 @@ typedef enum DevState {
|
||||||
|
|
||||||
// DMA protocol
|
// DMA protocol
|
||||||
Prepare_Data_Dma,
|
Prepare_Data_Dma,
|
||||||
Transfer_Data_Dma
|
Transfer_Data_Dma,
|
||||||
|
Device_Dma_Abort
|
||||||
} DevState_t;
|
} DevState_t;
|
||||||
|
|
||||||
typedef enum DmaState {
|
typedef enum DmaState {
|
||||||
|
@ -236,6 +249,8 @@ class IdeDisk : public SimObject
|
||||||
int devID;
|
int devID;
|
||||||
/** Interrupt pending */
|
/** Interrupt pending */
|
||||||
bool intrPending;
|
bool intrPending;
|
||||||
|
/** DMA Aborted */
|
||||||
|
bool dmaAborted;
|
||||||
|
|
||||||
Stats::Scalar dmaReadFullPages;
|
Stats::Scalar dmaReadFullPages;
|
||||||
Stats::Scalar dmaReadBytes;
|
Stats::Scalar dmaReadBytes;
|
||||||
|
|
|
@ -57,7 +57,7 @@ class SimObject;
|
||||||
* SimObject shouldn't cause the version number to increase, only changes to
|
* SimObject shouldn't cause the version number to increase, only changes to
|
||||||
* existing objects such as serializing/unserializing more state, changing sizes
|
* existing objects such as serializing/unserializing more state, changing sizes
|
||||||
* of serialized arrays, etc. */
|
* of serialized arrays, etc. */
|
||||||
static const uint64_t gem5CheckpointVersion = 0x0000000000000006;
|
static const uint64_t gem5CheckpointVersion = 0x0000000000000007;
|
||||||
|
|
||||||
template <class T>
|
template <class T>
|
||||||
void paramOut(std::ostream &os, const std::string &name, const T ¶m);
|
void paramOut(std::ostream &os, const std::string &name, const T ¶m);
|
||||||
|
|
|
@ -1,6 +1,6 @@
|
||||||
#!/usr/bin/env python
|
#!/usr/bin/env python
|
||||||
|
|
||||||
# Copyright (c) 2012 ARM Limited
|
# Copyright (c) 2012-2013 ARM Limited
|
||||||
# All rights reserved
|
# All rights reserved
|
||||||
#
|
#
|
||||||
# The license below extends only to copyright in the software and shall
|
# The license below extends only to copyright in the software and shall
|
||||||
|
@ -209,6 +209,15 @@ def from_5(cpt):
|
||||||
else:
|
else:
|
||||||
print "ISA is not x86"
|
print "ISA is not x86"
|
||||||
|
|
||||||
|
# Version 7 of the checkpoint adds support for the IDE dmaAbort flag
|
||||||
|
def from_6(cpt):
|
||||||
|
# Update IDE disk devices with dmaAborted
|
||||||
|
for sec in cpt.sections():
|
||||||
|
# curSector only exists in IDE devices, so key on that attribute
|
||||||
|
if cpt.has_option(sec, "curSector"):
|
||||||
|
cpt.set(sec, "dmaAborted", "false")
|
||||||
|
|
||||||
|
|
||||||
migrations = []
|
migrations = []
|
||||||
migrations.append(from_0)
|
migrations.append(from_0)
|
||||||
migrations.append(from_1)
|
migrations.append(from_1)
|
||||||
|
@ -216,6 +225,7 @@ migrations.append(from_2)
|
||||||
migrations.append(from_3)
|
migrations.append(from_3)
|
||||||
migrations.append(from_4)
|
migrations.append(from_4)
|
||||||
migrations.append(from_5)
|
migrations.append(from_5)
|
||||||
|
migrations.append(from_6)
|
||||||
|
|
||||||
verbose_print = False
|
verbose_print = False
|
||||||
|
|
||||||
|
@ -274,7 +284,6 @@ def process_file(path, **kwargs):
|
||||||
verboseprint("\t...completed")
|
verboseprint("\t...completed")
|
||||||
cpt.write(file(path, 'w'))
|
cpt.write(file(path, 'w'))
|
||||||
|
|
||||||
|
|
||||||
if __name__ == '__main__':
|
if __name__ == '__main__':
|
||||||
from optparse import OptionParser
|
from optparse import OptionParser
|
||||||
parser = OptionParser("usage: %prog [options] <filename or directory>")
|
parser = OptionParser("usage: %prog [options] <filename or directory>")
|
||||||
|
|
Loading…
Reference in a new issue