Gabe Black has uploaded this change for review. ( https://gem5-review.googlesource.com/c/public/gem5/+/36816 )

Change subject: dev: Convert the IDE controller to use the RegisterBank types.
......................................................................

dev: Convert the IDE controller to use the RegisterBank types.

Also get rid of the "ideConfig" register which does not actually show up
in the spec corresponding to this device's PCI IDs.

Change-Id: Id5d109403f49d956c696371b4d93d26150cc96dc
---
M src/dev/storage/ide_ctrl.cc
M src/dev/storage/ide_ctrl.hh
2 files changed, 113 insertions(+), 166 deletions(-)



diff --git a/src/dev/storage/ide_ctrl.cc b/src/dev/storage/ide_ctrl.cc
index 695e4d8..ca2f098 100644
--- a/src/dev/storage/ide_ctrl.cc
+++ b/src/dev/storage/ide_ctrl.cc
@@ -42,6 +42,7 @@

 #include <string>

+#include "base/cprintf.hh"
 #include "cpu/intr_control.hh"
 #include "debug/IdeCtrl.hh"
 #include "dev/storage/ide_disk.hh"
@@ -61,18 +62,6 @@
     BMIDescTablePtr = 0x4
 };

-// PCI config space registers
-enum ConfRegOffset {
-    PrimaryTiming = 0x40,
-    SecondaryTiming = 0x42,
-    DeviceTiming = 0x44,
-    UDMAControl = 0x48,
-    UDMATiming = 0x4A,
-    IDEConfig = 0x54
-};
-
-static const uint16_t timeRegWithDecodeEn = 0x8000;
-
 IdeController::Channel::Channel(string newName) : _name(newName)
 {
     bmiRegs.reset();
@@ -81,10 +70,9 @@
 }

 IdeController::IdeController(const Params &p)
-    : PciDevice(p), primary(name() + ".primary"),
+    : PciDevice(p), configSpaceRegs(name() + ".config_space_regs"),
+    primary(name() + ".primary"),
     secondary(name() + ".secondary"),
-    primaryTiming(htole(timeRegWithDecodeEn)),
-    secondaryTiming(htole(timeRegWithDecodeEn)),
     ioShift(p.io_shift), ctrlOffset(p.ctrl_offset)
 {

@@ -117,6 +105,26 @@
     secondary.select(false);
 }

+void
+IdeController::ConfigSpaceRegs::serialize(CheckpointOut &cp) const
+{
+    SERIALIZE_SCALAR(primaryTiming);
+    SERIALIZE_SCALAR(secondaryTiming);
+    SERIALIZE_SCALAR(deviceTiming);
+    SERIALIZE_SCALAR(udmaControl);
+    SERIALIZE_SCALAR(udmaTiming);
+}
+
+void
+IdeController::ConfigSpaceRegs::unserialize(CheckpointIn &cp)
+{
+    UNSERIALIZE_SCALAR(primaryTiming);
+    UNSERIALIZE_SCALAR(secondaryTiming);
+    UNSERIALIZE_SCALAR(deviceTiming);
+    UNSERIALIZE_SCALAR(udmaControl);
+    UNSERIALIZE_SCALAR(udmaTiming);
+}
+
 bool
 IdeController::isDiskSelected(IdeDisk *diskPtr)
 {
@@ -151,79 +159,34 @@
 IdeController::readConfig(PacketPtr pkt)
 {
     int offset = pkt->getAddr() & PCI_CONFIG_SIZE;
-    if (offset < PCI_DEVICE_SPECIFIC) {
+    if (offset < PCI_DEVICE_SPECIFIC)
         return PciDevice::readConfig(pkt);
-    }

-    switch (pkt->getSize()) {
-      case sizeof(uint8_t):
-        switch (offset) {
-          case DeviceTiming:
-            pkt->setLE<uint8_t>(deviceTiming);
-            break;
-          case UDMAControl:
-            pkt->setLE<uint8_t>(udmaControl);
-            break;
-          case PrimaryTiming + 1:
-            pkt->setLE<uint8_t>(bits(htole(primaryTiming), 15, 8));
-            break;
-          case SecondaryTiming + 1:
-            pkt->setLE<uint8_t>(bits(htole(secondaryTiming), 15, 8));
-            break;
-          case IDEConfig:
-            pkt->setLE<uint8_t>(bits(htole(ideConfig), 7, 0));
-            break;
-          case IDEConfig + 1:
-            pkt->setLE<uint8_t>(bits(htole(ideConfig), 15, 8));
-            break;
-          default:
- panic("Invalid PCI configuration read for size 1 at offset: %#x!\n",
-                    offset);
-        }
- DPRINTF(IdeCtrl, "PCI read offset: %#x size: 1 data: %#x\n", offset,
-                (uint32_t)pkt->getLE<uint8_t>());
+    size_t size = pkt->getSize();
+
+    configSpaceRegs.read(offset, pkt->getPtr<void>(), size);
+
+    std::string data;
+    switch (size) {
+      case 1:
+        data = csprintf("%#x", pkt->getLE<uint8_t>());
         break;
-      case sizeof(uint16_t):
-        switch (offset) {
-          case UDMAControl:
-            pkt->setLE<uint16_t>(udmaControl);
-            break;
-          case PrimaryTiming:
-            pkt->setLE<uint16_t>(primaryTiming);
-            break;
-          case SecondaryTiming:
-            pkt->setLE<uint16_t>(secondaryTiming);
-            break;
-          case UDMATiming:
-            pkt->setLE<uint16_t>(udmaTiming);
-            break;
-          case IDEConfig:
-            pkt->setLE<uint16_t>(ideConfig);
-            break;
-          default:
- panic("Invalid PCI configuration read for size 2 offset: %#x!\n",
-                    offset);
-        }
- DPRINTF(IdeCtrl, "PCI read offset: %#x size: 2 data: %#x\n", offset,
-                (uint32_t)pkt->getLE<uint16_t>());
+      case 2:
+        data = csprintf("%#x", pkt->getLE<uint16_t>());
         break;
-      case sizeof(uint32_t):
-        switch (offset) {
-          case PrimaryTiming:
-            pkt->setLE<uint32_t>(primaryTiming);
-            break;
-          case IDEConfig:
-            pkt->setLE<uint32_t>(ideConfig);
-            break;
-          default:
-            panic("No 32bit reads implemented for this device.");
-        }
- DPRINTF(IdeCtrl, "PCI read offset: %#x size: 4 data: %#x\n", offset,
-                (uint32_t)pkt->getLE<uint32_t>());
+      case 4:
+        data = csprintf("%#x", pkt->getLE<uint32_t>());
+        break;
+      case 8:
+        data = csprintf("%#x", pkt->getLE<uint64_t>());
         break;
       default:
-        panic("invalid access size(?) for PCI configspace!\n");
+        data = "{binary}";
+        break;
     }
+ DPRINTF(IdeCtrl, "PCI read offset: %#x size: %d data: %s\n", offset, size,
+            data);
+
     pkt->makeAtomicResponse();
     return configDelay;
 }
@@ -233,73 +196,37 @@
 IdeController::writeConfig(PacketPtr pkt)
 {
     int offset = pkt->getAddr() & PCI_CONFIG_SIZE;
-    if (offset < PCI_DEVICE_SPECIFIC) {
-        PciDevice::writeConfig(pkt);
-    } else {
-        switch (pkt->getSize()) {
-          case sizeof(uint8_t):
-            switch (offset) {
-              case DeviceTiming:
-                deviceTiming = pkt->getLE<uint8_t>();
-                break;
-              case UDMAControl:
-                udmaControl = pkt->getLE<uint8_t>();
-                break;
-              case IDEConfig:
-                replaceBits(ideConfig, 7, 0, pkt->getLE<uint8_t>());
-                break;
-              case IDEConfig + 1:
-                replaceBits(ideConfig, 15, 8, pkt->getLE<uint8_t>());
-                break;
-              default:
-                panic("Invalid PCI configuration write "
-                        "for size 1 offset: %#x!\n", offset);
-            }
-            DPRINTF(IdeCtrl, "PCI write offset: %#x size: 1 data: %#x\n",
-                    offset, (uint32_t)pkt->getLE<uint8_t>());
-            break;
-          case sizeof(uint16_t):
-            switch (offset) {
-              case UDMAControl:
-                udmaControl = pkt->getLE<uint16_t>();
-                break;
-              case PrimaryTiming:
-                primaryTiming = pkt->getLE<uint16_t>();
-                break;
-              case SecondaryTiming:
-                secondaryTiming = pkt->getLE<uint16_t>();
-                break;
-              case UDMATiming:
-                udmaTiming = pkt->getLE<uint16_t>();
-                break;
-              case IDEConfig:
-                ideConfig = pkt->getLE<uint16_t>();
-                break;
-              default:
-                panic("Invalid PCI configuration write "
-                        "for size 2 offset: %#x!\n",
-                        offset);
-            }
-            DPRINTF(IdeCtrl, "PCI write offset: %#x size: 2 data: %#x\n",
-                    offset, (uint32_t)pkt->getLE<uint16_t>());
-            break;
-          case sizeof(uint32_t):
-            switch (offset) {
-              case PrimaryTiming:
-                primaryTiming = pkt->getLE<uint32_t>();
-                break;
-              case IDEConfig:
-                ideConfig = pkt->getLE<uint32_t>();
-                break;
-              default:
- panic("Write of unimplemented PCI config. register: %x\n", offset);
-            }
-            break;
-          default:
-            panic("invalid access size(?) for PCI configspace!\n");
-        }
-        pkt->makeAtomicResponse();
+
+    if (offset < PCI_DEVICE_SPECIFIC)
+        return PciDevice::writeConfig(pkt);
+
+    size_t size = pkt->getSize();
+
+    std::string data;
+    switch (size) {
+      case 1:
+        data = csprintf("%#x", pkt->getLE<uint8_t>());
+        break;
+      case 2:
+        data = csprintf("%#x", pkt->getLE<uint16_t>());
+        break;
+      case 4:
+        data = csprintf("%#x", pkt->getLE<uint32_t>());
+        break;
+      case 8:
+        data = csprintf("%#x", pkt->getLE<uint64_t>());
+        break;
+      default:
+        data = "{binary}";
+        break;
     }
+
+    DPRINTF(IdeCtrl, "PCI write offset: %#x size: %d data: %#x\n",
+            offset, size, data);
+
+    configSpaceRegs.write(offset, pkt->getConstPtr<void>(), size);
+
+    pkt->makeAtomicResponse();
     return configDelay;
 }

@@ -510,12 +437,7 @@
     secondary.serialize("secondary", cp);

     // Serialize config registers
-    SERIALIZE_SCALAR(primaryTiming);
-    SERIALIZE_SCALAR(secondaryTiming);
-    SERIALIZE_SCALAR(deviceTiming);
-    SERIALIZE_SCALAR(udmaControl);
-    SERIALIZE_SCALAR(udmaTiming);
-    SERIALIZE_SCALAR(ideConfig);
+    configSpaceRegs.serialize(cp);
 }

 void
@@ -543,12 +465,7 @@
     secondary.unserialize("secondary", cp);

     // Unserialize config registers
-    UNSERIALIZE_SCALAR(primaryTiming);
-    UNSERIALIZE_SCALAR(secondaryTiming);
-    UNSERIALIZE_SCALAR(deviceTiming);
-    UNSERIALIZE_SCALAR(udmaControl);
-    UNSERIALIZE_SCALAR(udmaTiming);
-    UNSERIALIZE_SCALAR(ideConfig);
+    configSpaceRegs.unserialize(cp);
 }

 void
diff --git a/src/dev/storage/ide_ctrl.hh b/src/dev/storage/ide_ctrl.hh
index dfce834..ebb21e1 100644
--- a/src/dev/storage/ide_ctrl.hh
+++ b/src/dev/storage/ide_ctrl.hh
@@ -37,6 +37,7 @@
 #include "base/bitunion.hh"
 #include "dev/io_device.hh"
 #include "dev/pci/device.hh"
+#include "dev/reg_bank.hh"
 #include "params/IdeController.hh"

 class IdeDisk;
@@ -62,6 +63,42 @@
         Bitfield<0> startStop;
     EndBitUnion(BMICommandReg)

+    /** Registers used in device specific PCI configuration */
+    class ConfigSpaceRegs : public RegisterBankLE
+    {
+      public:
+        ConfigSpaceRegs(const std::string &name) :
+            RegisterBankLE(name, PCI_DEVICE_SPECIFIC)
+        {
+            // None of these registers are actually hooked up to control
+            // anything, so they have no specially defined behaviors. They
+ // just store values for now, but should presumably do something
+            // in a more accurate model.
+ addRegisters({primaryTiming, secondaryTiming, deviceTiming, raz0,
+                          udmaControl, raz1, udmaTiming, raz2});
+        }
+
+        enum {
+            TimeRegWithDecodeEnabled = 0x8000
+        };
+
+        /* 0x40-0x41 */ Register16 primaryTiming =
+                            {"primary timing", TimeRegWithDecodeEnabled};
+        /* 0x42-0x43 */ Register16 secondaryTiming =
+                            {"secondary timing", TimeRegWithDecodeEnabled};
+        /* 0x44      */ Register8 deviceTiming = {"device timing"};
+        /* 0x45-0x47 */ RegisterRaz raz0 = {"raz0", 3};
+        /* 0x48      */ Register8 udmaControl = {"udma control"};
+        /* 0x49      */ RegisterRaz raz1 = {"raz1", 1};
+        /* 0x4a-0x4b */ Register16 udmaTiming = {"udma timing"};
+ /* 0x4c-... */ RegisterRaz raz2 = {"raz2", PCI_CONFIG_SIZE - 0x4c};
+
+        void serialize(CheckpointOut &cp) const;
+        void unserialize(CheckpointIn &cp);
+    };
+
+    ConfigSpaceRegs configSpaceRegs;
+
     struct Channel
     {
         std::string _name;
@@ -121,13 +158,6 @@
     Channel primary;
     Channel secondary;

-    /** Registers used in device specific PCI configuration */
-    uint16_t primaryTiming, secondaryTiming;
-    uint8_t deviceTiming = 0;
-    uint8_t udmaControl = 0;
-    uint16_t udmaTiming = 0;
-    uint16_t ideConfig = 0;
-
     uint32_t ioShift, ctrlOffset;

     void dispatchAccess(PacketPtr pkt, bool read);

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/36816
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: Id5d109403f49d956c696371b4d93d26150cc96dc
Gerrit-Change-Number: 36816
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

Reply via email to