Gabe Black has submitted this change. (
https://gem5-review.googlesource.com/c/public/gem5/+/34178 )
Change subject: dev: Stop using the OS page size in the IDE controller.
......................................................................
dev: Stop using the OS page size in the IDE controller.
This size was used to break up DMA transactions so that a single
transaction would not cross a page boundary. This was because on Alpha,
there was an actual page table which translated between PCI and DMA
address spaces. On all currently implemented systems, the mapping is
simply to add a scalar offset, so it's not possible for a legal region
of memory to be contiguous in one space but not in the other.
Additionally, if it *was* possible for there to be a mismatch, it was
only coincidence that Alpha used a page table which had the same sized
pages as it normally used. There is no requirement that there even would
be fixed sized pages in the first place.
To avoid this artificial dependency between the IDE controller and the
ISA, this change simply changes the chunk size for DMA accesses to 4K.
That's the page size at least on x86 and probably other architectures,
and will be a pretty close approximation of the previous behavior.
It's possible that even having this chunking in the first place is
unnecessary and functionally useless, but there are some checks which
happen between chunks, and changing how big they are would change the
frequency of those checks. For instance, the controller/disk may not
notice in the same amount of time if a DMA was cancelled somehow.
Change-Id: I1ec840d1f158c3faa31ba0184458b69bf654c252
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/34178
Reviewed-by: Jason Lowe-Power <power...@gmail.com>
Maintainer: Gabe Black <gabebl...@google.com>
Tested-by: kokoro <noreply+kok...@google.com>
---
M src/dev/storage/ide_ctrl.cc
M src/dev/storage/ide_disk.cc
M src/dev/storage/ide_disk.hh
3 files changed, 10 insertions(+), 9 deletions(-)
Approvals:
Jason Lowe-Power: Looks good to me, approved
Gabe Black: Looks good to me, approved
kokoro: Regressions pass
diff --git a/src/dev/storage/ide_ctrl.cc b/src/dev/storage/ide_ctrl.cc
index 47cdd10..5efa42b 100644
--- a/src/dev/storage/ide_ctrl.cc
+++ b/src/dev/storage/ide_ctrl.cc
@@ -120,7 +120,8 @@
panic("IDE controllers support a maximum "
"of 4 devices attached!\n");
}
- params()->disks[i]->setController(this, sys->getPageBytes());
+ // Arbitrarily set the chunk size to 4K.
+ params()->disks[i]->setController(this, 4 * 1024);
}
primary.select(false);
diff --git a/src/dev/storage/ide_disk.cc b/src/dev/storage/ide_disk.cc
index e97e23b..57fa076 100644
--- a/src/dev/storage/ide_disk.cc
+++ b/src/dev/storage/ide_disk.cc
@@ -435,7 +435,7 @@
// clear out the data buffer
memset(dataBuffer, 0, MAX_DMA_SIZE);
dmaReadCG = new ChunkGenerator(curPrd.getBaseAddr(),
- curPrd.getByteCount(), pageBytes);
+ curPrd.getByteCount(), chunkBytes);
}
if (ctrl->dmaPending() || ctrl->drainState() != DrainState::Running) {
@@ -447,7 +447,7 @@
&dmaReadWaitEvent, dataBuffer + dmaReadCG->complete());
dmaReadBytes += dmaReadCG->size();
dmaReadTxs++;
- if (dmaReadCG->size() == pageBytes)
+ if (dmaReadCG->size() == chunkBytes)
dmaReadFullPages++;
dmaReadCG->next();
} else {
@@ -518,7 +518,7 @@
if (!dmaWriteCG) {
// clear out the data buffer
dmaWriteCG = new ChunkGenerator(curPrd.getBaseAddr(),
- curPrd.getByteCount(), pageBytes);
+ curPrd.getByteCount(), chunkBytes);
}
if (ctrl->dmaPending() || ctrl->drainState() != DrainState::Running) {
schedule(dmaWriteWaitEvent, curTick() + DMA_BACKOFF_PERIOD);
@@ -532,7 +532,7 @@
curPrd.getByteCount(), curPrd.getEOT());
dmaWriteBytes += dmaWriteCG->size();
dmaWriteTxs++;
- if (dmaWriteCG->size() == pageBytes)
+ if (dmaWriteCG->size() == chunkBytes)
dmaWriteFullPages++;
dmaWriteCG->next();
} else {
diff --git a/src/dev/storage/ide_disk.hh b/src/dev/storage/ide_disk.hh
index 9f42941..90cbf57 100644
--- a/src/dev/storage/ide_disk.hh
+++ b/src/dev/storage/ide_disk.hh
@@ -239,8 +239,8 @@
DmaState_t dmaState;
/** Dma transaction is a read */
bool dmaRead;
- /** Size of OS pages. */
- Addr pageBytes;
+ /** Size of chunks to DMA. */
+ Addr chunkBytes;
/** PRD table base address */
uint32_t curPrdAddr;
/** PRD entry */
@@ -283,11 +283,11 @@
* @param c The IDE controller
*/
void
- setController(IdeController *c, Addr page_bytes)
+ setController(IdeController *c, Addr chunk_bytes)
{
panic_if(ctrl, "Cannot change the controller once set!\n");
ctrl = c;
- pageBytes = page_bytes;
+ chunkBytes = chunk_bytes;
}
// Device register read/write
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/34178
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: I1ec840d1f158c3faa31ba0184458b69bf654c252
Gerrit-Change-Number: 34178
Gerrit-PatchSet: 4
Gerrit-Owner: Gabe Black <gabebl...@google.com>
Gerrit-Reviewer: Anthony Gutierrez <anthony.gutier...@amd.com>
Gerrit-Reviewer: Bobby R. Bruce <bbr...@ucdavis.edu>
Gerrit-Reviewer: Gabe Black <gabebl...@google.com>
Gerrit-Reviewer: Giacomo Travaglini <giacomo.travagl...@arm.com>
Gerrit-Reviewer: Jason Lowe-Power <power...@gmail.com>
Gerrit-Reviewer: Nikos Nikoleris <nikos.nikole...@arm.com>
Gerrit-Reviewer: kokoro <noreply+kok...@google.com>
Gerrit-MessageType: merged
_______________________________________________
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
