Gabe Black has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/57016 )
Change subject: dev: Stop passing the pkt into read|writeConfig.
......................................................................
dev: Stop passing the pkt into read|writeConfig.
The config space offset is not necessarily the masked lower bits of the
address in the original packet. We're already computing the actual
offset, but then we're throwing it away. Instead, pass in the offset,
target (or source) buffer, and size.
Change-Id: Ib01ce1f8e6c40c73d1b14e29ba979370b2ab9e6b
---
M src/dev/amdgpu/amdgpu_device.cc
M src/dev/amdgpu/amdgpu_device.hh
M src/dev/net/i8254xGBe.cc
M src/dev/net/i8254xGBe.hh
M src/dev/net/ns_gige.cc
M src/dev/net/ns_gige.hh
M src/dev/pci/device.cc
M src/dev/pci/device.hh
M src/dev/pci/host.cc
M src/dev/storage/ide_ctrl.cc
M src/dev/storage/ide_ctrl.hh
11 files changed, 123 insertions(+), 104 deletions(-)
diff --git a/src/dev/amdgpu/amdgpu_device.cc
b/src/dev/amdgpu/amdgpu_device.cc
index a4edef4..13c50a1 100644
--- a/src/dev/amdgpu/amdgpu_device.cc
+++ b/src/dev/amdgpu/amdgpu_device.cc
@@ -97,13 +97,12 @@
}
Tick
-AMDGPUDevice::readConfig(PacketPtr pkt)
+AMDGPUDevice::readConfig(Addr offset, void *data, Addr size)
{
- [[maybe_unused]] int offset = pkt->getAddr() & PCI_CONFIG_SIZE;
DPRINTF(AMDGPUDevice, "Read Config: from offset: %#x size: %#x "
"data: %#x\n", offset, pkt->getSize(), config.data[offset]);
- Tick delay = PciDevice::readConfig(pkt);
+ Tick delay = PciDevice::readConfig(offset, data, size);
// Before sending MMIOs the driver sends three interrupts in a row.
// Use this to trigger creating a checkpoint to restore in timing mode.
@@ -125,14 +124,25 @@
}
Tick
-AMDGPUDevice::writeConfig(PacketPtr pkt)
+AMDGPUDevice::writeConfig(Addr offset, const void *data, Addr size)
{
- [[maybe_unused]] int offset = pkt->getAddr() & PCI_CONFIG_SIZE;
- DPRINTF(AMDGPUDevice, "Write Config: from offset: %#x size: %#x "
- "data: %#x\n", offset, pkt->getSize(),
- pkt->getUintX(ByteOrder::little));
+ if (debug::AMDGPUDevice) {
+ uint64_t val = 0;
+ if (size == 1)
+ val = letoh(*(const uint8_t *)data);
+ else if (size == 2)
+ val = letoh(*(const uint16_t *)data);
+ else if (size == 4)
+ val = letoh(*(const uint32_t *)data);
+ else if (size == 8)
+ val = letoh(*(const uint64_t *)data);
+ else
+ panic("Unsupported config write size %d.", size);
+ DPRINTF(AMDGPUDevice, "Write Config: from offset: %#x size: %#x "
+ "data: %#x\n", offset, size, val);
+ }
- return PciDevice::writeConfig(pkt);
+ return PciDevice::writeConfig(offset, data, size);
}
void
diff --git a/src/dev/amdgpu/amdgpu_device.hh
b/src/dev/amdgpu/amdgpu_device.hh
index b97d682..62e40e9 100644
--- a/src/dev/amdgpu/amdgpu_device.hh
+++ b/src/dev/amdgpu/amdgpu_device.hh
@@ -114,8 +114,8 @@
*/
void intrPost();
- Tick writeConfig(PacketPtr pkt) override;
- Tick readConfig(PacketPtr pkt) override;
+ Tick writeConfig(Addr offset, const void *data, Addr size) override;
+ Tick readConfig(Addr offset, void *data, Addr size) override;
Tick read(PacketPtr pkt) override;
Tick write(PacketPtr pkt) override;
diff --git a/src/dev/net/i8254xGBe.cc b/src/dev/net/i8254xGBe.cc
index 1bfa85f..c09e9a8 100644
--- a/src/dev/net/i8254xGBe.cc
+++ b/src/dev/net/i8254xGBe.cc
@@ -148,11 +148,10 @@
}
Tick
-IGbE::writeConfig(PacketPtr pkt)
+IGbE::writeConfig(Addr offset, const void *data, Addr size)
{
- int offset = pkt->getAddr() & PCI_CONFIG_SIZE;
if (offset < PCI_DEVICE_SPECIFIC)
- PciDevice::writeConfig(pkt);
+ PciDevice::writeConfig(offset, data, size);
else
panic("Device specific PCI config space not implemented.\n");
diff --git a/src/dev/net/i8254xGBe.hh b/src/dev/net/i8254xGBe.hh
index e6aa35d..9db4871 100644
--- a/src/dev/net/i8254xGBe.hh
+++ b/src/dev/net/i8254xGBe.hh
@@ -490,7 +490,7 @@
Tick read(PacketPtr pkt) override;
Tick write(PacketPtr pkt) override;
- Tick writeConfig(PacketPtr pkt) override;
+ Tick writeConfig(Addr offset, const void *data, Addr size) override;
bool ethRxPkt(EthPacketPtr packet);
void ethTxDone();
diff --git a/src/dev/net/ns_gige.cc b/src/dev/net/ns_gige.cc
index b8c3705..434948d 100644
--- a/src/dev/net/ns_gige.cc
+++ b/src/dev/net/ns_gige.cc
@@ -148,11 +148,10 @@
* This is to write to the PCI general configuration registers
*/
Tick
-NSGigE::writeConfig(PacketPtr pkt)
+NSGigE::writeConfig(Addr offset, const void *data, Addr size)
{
- int offset = pkt->getAddr() & PCI_CONFIG_SIZE;
if (offset < PCI_DEVICE_SPECIFIC)
- PciDevice::writeConfig(pkt);
+ PciDevice::writeConfig(offset, data, size);
else
panic("Device specific PCI config space not implemented!\n");
@@ -199,7 +198,8 @@
if (daddr > LAST && daddr <= RESERVED) {
panic("Accessing reserved register");
} else if (daddr > RESERVED && daddr <= 0x3FC) {
- return readConfig(pkt);
+ return readConfig(pkt->getAddr() & PCI_CONFIG_SIZE,
+ pkt->getPtr<uint8_t>(), pkt->getSize());
} else if (daddr >= MIB_START && daddr <= MIB_END) {
// don't implement all the MIB's. hopefully the kernel
// doesn't actually DEPEND upon their values
@@ -417,7 +417,8 @@
if (daddr > LAST && daddr <= RESERVED) {
panic("Accessing reserved register");
} else if (daddr > RESERVED && daddr <= 0x3FC) {
- return writeConfig(pkt);
+ return writeConfig(pkt->getAddr() & PCI_CONFIG_SIZE,
+ pkt->getConstPtr<uint8_t>(), pkt->getSize());
} else if (daddr > 0x3FC)
panic("Something is messed up!\n");
diff --git a/src/dev/net/ns_gige.hh b/src/dev/net/ns_gige.hh
index 13a2ec0..d6e8286 100644
--- a/src/dev/net/ns_gige.hh
+++ b/src/dev/net/ns_gige.hh
@@ -339,7 +339,7 @@
Port &getPort(const std::string &if_name,
PortID idx=InvalidPortID) override;
- Tick writeConfig(PacketPtr pkt) override;
+ Tick writeConfig(Addr offset, const void *data, Addr size) override;
Tick read(PacketPtr pkt) override;
Tick write(PacketPtr pkt) override;
diff --git a/src/dev/pci/device.cc b/src/dev/pci/device.cc
index 35a4d4f..f4f33ad 100644
--- a/src/dev/pci/device.cc
+++ b/src/dev/pci/device.cc
@@ -46,6 +46,7 @@
#include <list>
#include <string>
+#include <utility>
#include <vector>
#include "base/inifile.hh"
@@ -209,58 +210,42 @@
}
Tick
-PciDevice::readConfig(PacketPtr pkt)
+PciDevice::readConfig(Addr offset, void *data, Addr size)
{
- int offset = pkt->getAddr() & PCI_CONFIG_SIZE;
-
/* Return 0 for accesses to unimplemented PCI configspace areas */
if (offset >= PCI_DEVICE_SPECIFIC &&
offset < PCI_CONFIG_SIZE) {
warn_once("Device specific PCI config space "
"not implemented for %s!\n", this->name());
- switch (pkt->getSize()) {
- case sizeof(uint8_t):
- pkt->setLE<uint8_t>(0);
- break;
- case sizeof(uint16_t):
- pkt->setLE<uint16_t>(0);
- break;
- case sizeof(uint32_t):
- pkt->setLE<uint32_t>(0);
- break;
- default:
- panic("invalid access size(?) for PCI configspace!\n");
- }
+ uint8_t *bytes = (uint8_t *)data;
+ std::fill(bytes, bytes + size, 0);
} else if (offset > PCI_CONFIG_SIZE) {
panic("Out-of-range access to PCI config space!\n");
}
- switch (pkt->getSize()) {
+ switch (size) {
case sizeof(uint8_t):
- pkt->setLE<uint8_t>(config.data[offset]);
+ std::memcpy(data, &config.data[offset], 1);
DPRINTF(PciDevice,
"readConfig: dev %#x func %#x reg %#x 1 bytes: data = %#x\n",
- _busAddr.dev, _busAddr.func, offset,
- (uint32_t)pkt->getLE<uint8_t>());
+ _busAddr.dev, _busAddr.func, offset, *(uint8_t *)data);
break;
case sizeof(uint16_t):
- pkt->setLE<uint16_t>(*(uint16_t*)&config.data[offset]);
+ std::memcpy(data, &config.data[offset], 2);
+ *(uint16_t *)data = *(uint16_t *)&config.data[offset];
DPRINTF(PciDevice,
"readConfig: dev %#x func %#x reg %#x 2 bytes: data = %#x\n",
- _busAddr.dev, _busAddr.func, offset,
- (uint32_t)pkt->getLE<uint16_t>());
+ _busAddr.dev, _busAddr.func, offset, *(uint16_t *)data);
break;
case sizeof(uint32_t):
- pkt->setLE<uint32_t>(*(uint32_t*)&config.data[offset]);
+ std::memcpy(data, &config.data[offset], 4);
DPRINTF(PciDevice,
"readConfig: dev %#x func %#x reg %#x 4 bytes: data = %#x\n",
- _busAddr.dev, _busAddr.func, offset,
- (uint32_t)pkt->getLE<uint32_t>());
+ _busAddr.dev, _busAddr.func, offset, *(uint32_t *)data);
break;
default:
panic("invalid access size(?) for PCI configspace!\n");
}
- pkt->makeAtomicResponse();
return configDelay;
}
@@ -280,38 +265,28 @@
}
Tick
-PciDevice::writeConfig(PacketPtr pkt)
+PciDevice::writeConfig(Addr offset, const void *data, Addr size)
{
- int offset = pkt->getAddr() & PCI_CONFIG_SIZE;
-
/* No effect if we write to config space that is not implemented*/
if (offset >= PCI_DEVICE_SPECIFIC &&
offset < PCI_CONFIG_SIZE) {
warn_once("Device specific PCI config space "
"not implemented for %s!\n", this->name());
- switch (pkt->getSize()) {
- case sizeof(uint8_t):
- case sizeof(uint16_t):
- case sizeof(uint32_t):
- break;
- default:
- panic("invalid access size(?) for PCI configspace!\n");
- }
} else if (offset > PCI_CONFIG_SIZE) {
panic("Out-of-range access to PCI config space!\n");
}
- switch (pkt->getSize()) {
+ switch (size) {
case sizeof(uint8_t):
switch (offset) {
case PCI0_INTERRUPT_LINE:
- config.interruptLine = pkt->getLE<uint8_t>();
+ config.interruptLine = *(uint8_t *)data;
break;
case PCI_CACHE_LINE_SIZE:
- config.cacheLineSize = pkt->getLE<uint8_t>();
+ config.cacheLineSize = *(uint8_t *)data;
break;
case PCI_LATENCY_TIMER:
- config.latencyTimer = pkt->getLE<uint8_t>();
+ config.latencyTimer = *(uint8_t *)data;
break;
/* Do nothing for these read-only registers */
case PCI0_INTERRUPT_PIN:
@@ -325,29 +300,27 @@
}
DPRINTF(PciDevice,
"writeConfig: dev %#x func %#x reg %#x 1 bytes: data = %#x\n",
- _busAddr.dev, _busAddr.func, offset,
- (uint32_t)pkt->getLE<uint8_t>());
+ _busAddr.dev, _busAddr.func, offset, htole(*(uint8_t *)data));
break;
case sizeof(uint16_t):
switch (offset) {
case PCI_COMMAND:
- config.command = pkt->getLE<uint8_t>();
+ config.command = *(uint16_t *)data;
// IO or memory space may have been enabled/disabled.
pioPort.sendRangeChange();
break;
case PCI_STATUS:
- config.status = pkt->getLE<uint8_t>();
+ config.status = *(uint16_t *)data;
break;
case PCI_CACHE_LINE_SIZE:
- config.cacheLineSize = pkt->getLE<uint8_t>();
+ config.cacheLineSize = *(uint16_t *)data;
break;
default:
panic("writing to a read only register");
}
DPRINTF(PciDevice,
"writeConfig: dev %#x func %#x reg %#x 2 bytes: data = %#x\n",
- _busAddr.dev, _busAddr.func, offset,
- (uint32_t)pkt->getLE<uint16_t>());
+ _busAddr.dev, _busAddr.func, offset, *(uint16_t *)data);
break;
case sizeof(uint32_t):
switch (offset) {
@@ -361,23 +334,26 @@
int num = BAR_NUMBER(offset);
auto *bar = BARs[num];
config.baseAddr[num] =
- htole(bar->write(hostInterface,
pkt->getLE<uint32_t>()));
+ htole(bar->write(hostInterface, letoh(*(uint32_t
*)data)));
pioPort.sendRangeChange();
}
break;
case PCI0_ROM_BASE_ADDR:
- if (letoh(pkt->getLE<uint32_t>()) == 0xfffffffe)
+ {
+ auto val = *(uint32_t *)data;
+ if (letoh(val) == 0xfffffffe)
config.expansionROM = htole((uint32_t)0xffffffff);
else
- config.expansionROM = pkt->getLE<uint32_t>();
+ config.expansionROM = val;
+ }
break;
case PCI_COMMAND:
// This could also clear some of the error bits in the Status
// register. However they should never get set, so lets ignore
// it for now
- config.command = pkt->getLE<uint32_t>();
+ config.command = *(uint32_t *)data;
// IO or memory space may have been enabled/disabled.
pioPort.sendRangeChange();
break;
@@ -387,13 +363,11 @@
}
DPRINTF(PciDevice,
"writeConfig: dev %#x func %#x reg %#x 4 bytes: data = %#x\n",
- _busAddr.dev, _busAddr.func, offset,
- (uint32_t)pkt->getLE<uint32_t>());
+ _busAddr.dev, _busAddr.func, offset, *(uint32_t *)data);
break;
default:
panic("invalid access size(?) for PCI configspace!\n");
}
- pkt->makeAtomicResponse();
return configDelay;
}
diff --git a/src/dev/pci/device.hh b/src/dev/pci/device.hh
index dfa70b5..002c374 100644
--- a/src/dev/pci/device.hh
+++ b/src/dev/pci/device.hh
@@ -335,18 +335,22 @@
* Write to the PCI config space data that is stored locally. This may
be
* overridden by the device but at some point it will eventually call
this
* for normal operations that it does not need to override.
- * @param pkt packet containing the write the offset into config space
+ * @param offset Offset into config space of the write.
+ * @param data Pointer to the data to write.
+ * @param size Number of bytes to write.
*/
- virtual Tick writeConfig(PacketPtr pkt);
+ virtual Tick writeConfig(Addr offset, const void *data, Addr size);
/**
* Read from the PCI config space data that is stored locally. This
may be
* overridden by the device but at some point it will eventually call
this
* for normal operations that it does not need to override.
- * @param pkt packet containing the write the offset into config space
+ * @param offset Offset into config space of the read.
+ * @param data Pointer to the data to read.
+ * @param size Number of bytes to read.
*/
- virtual Tick readConfig(PacketPtr pkt);
+ virtual Tick readConfig(Addr offset, void *data, Addr size);
protected:
PciHost::DeviceInterface hostInterface;
diff --git a/src/dev/pci/host.cc b/src/dev/pci/host.cc
index ce86967..fc859f5 100644
--- a/src/dev/pci/host.cc
+++ b/src/dev/pci/host.cc
@@ -103,7 +103,7 @@
// @todo Remove this after testing
pkt->headerDelay = pkt->payloadDelay = 0;
- return pci_dev->writeConfig(pkt);
+ return pci_dev->writeConfig(offset, pkt->getConstPtr<uint8_t>(), size);
}
Tick
@@ -121,7 +121,7 @@
if (pci_dev) {
// @todo Remove this after testing
pkt->headerDelay = pkt->payloadDelay = 0;
- return pci_dev->readConfig(pkt);
+ return pci_dev->readConfig(offset, pkt_data, size);
} else {
std::fill(pkt_data, pkt_data + size, 0xFF);
return 0;
diff --git a/src/dev/storage/ide_ctrl.cc b/src/dev/storage/ide_ctrl.cc
index 45e6242..ff567d5 100644
--- a/src/dev/storage/ide_ctrl.cc
+++ b/src/dev/storage/ide_ctrl.cc
@@ -155,40 +155,57 @@
}
Tick
-IdeController::readConfig(PacketPtr pkt)
+IdeController::readConfig(Addr offset, void *data, Addr size)
{
- int offset = pkt->getAddr() & PCI_CONFIG_SIZE;
if (offset < PCI_DEVICE_SPECIFIC)
- return PciDevice::readConfig(pkt);
+ return PciDevice::readConfig(offset, data, size);
- size_t size = pkt->getSize();
+ configSpaceRegs.read(offset, data, size);
- configSpaceRegs.read(offset, pkt->getPtr<void>(), size);
+ if (debug::IdeCtrl) {
+ uint64_t val = 0;
+ if (size == 1)
+ val = letoh(*(uint8_t *)data);
+ else if (size == 2)
+ val = letoh(*(uint16_t *)data);
+ else if (size == 4)
+ val = letoh(*(uint32_t *)data);
+ else if (size == 8)
+ val = letoh(*(uint64_t *)data);
+ else
+ panic("Unsupported config read size %d.", size);
+ DPRINTF(IdeCtrl, "PCI read offset: %#x size: %d data: %#x\n",
+ offset, size, val);
+ }
- DPRINTF(IdeCtrl, "PCI read offset: %#x size: %d data: %#x\n", offset,
size,
- pkt->getUintX(ByteOrder::little));
-
- pkt->makeAtomicResponse();
return configDelay;
}
Tick
-IdeController::writeConfig(PacketPtr pkt)
+IdeController::writeConfig(Addr offset, const void *data, Addr size)
{
- int offset = pkt->getAddr() & PCI_CONFIG_SIZE;
-
if (offset < PCI_DEVICE_SPECIFIC)
- return PciDevice::writeConfig(pkt);
+ return PciDevice::writeConfig(offset, data, size);
- size_t size = pkt->getSize();
+ if (debug::IdeCtrl) {
+ uint64_t val = 0;
+ if (size == 1)
+ val = letoh(*(const uint8_t *)data);
+ else if (size == 2)
+ val = letoh(*(const uint16_t *)data);
+ else if (size == 4)
+ val = letoh(*(const uint32_t *)data);
+ else if (size == 8)
+ val = letoh(*(const uint64_t *)data);
+ else
+ panic("Unsupported config write size %d.", size);
+ DPRINTF(IdeCtrl, "PCI write offset: %#x size: %d data: %#x\n",
+ offset, size, val);
+ }
- DPRINTF(IdeCtrl, "PCI write offset: %#x size: %d data: %#x\n",
- offset, size, pkt->getUintX(ByteOrder::little));
+ configSpaceRegs.write(offset, data, size);
- configSpaceRegs.write(offset, pkt->getConstPtr<void>(), size);
-
- pkt->makeAtomicResponse();
return configDelay;
}
diff --git a/src/dev/storage/ide_ctrl.hh b/src/dev/storage/ide_ctrl.hh
index 635c446..53b4486 100644
--- a/src/dev/storage/ide_ctrl.hh
+++ b/src/dev/storage/ide_ctrl.hh
@@ -204,8 +204,8 @@
virtual void postInterrupt(bool is_primary);
virtual void clearInterrupt(bool is_primary);
- Tick writeConfig(PacketPtr pkt) override;
- Tick readConfig(PacketPtr pkt) override;
+ Tick writeConfig(Addr offset, const void *data, Addr size) override;
+ Tick readConfig(Addr offset, void *data, Addr size) override;
Tick read(PacketPtr pkt) override;
Tick write(PacketPtr pkt) override;
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/57016
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: Ib01ce1f8e6c40c73d1b14e29ba979370b2ab9e6b
Gerrit-Change-Number: 57016
Gerrit-PatchSet: 1
Gerrit-Owner: Gabe Black <gabe.bl...@gmail.com>
Gerrit-MessageType: newchange
_______________________________________________
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
