Adds HBMCtrl by aakahlow · Pull Request #22 · darchr/dramCacheController

aakahlow · 2022-05-05T07:59:40Z

TODO: Add more description into commit messages

This PR adds hbm ctrl and other related interface changes required for HBM ctrl

Also, builds on top of the SimpleMemCtrl and MemCtrl change

powerjg

mem: Add a notion of pseudo channel in memory interface

Some comments on this one.

mem: Add hbm controller

Couple of style issues, but I didn't review closely

mem: move nextBurstAt. and nextReqTime to memory interface

ok

mem: hbm ctrl -- check which scheduler to restart after refresh

Comments to improve.

mem: support for separate tRCD and tCL for reads/writes

OK

mem: some fixes for hbm ctrl

Rebase error? hard to tell what's going on here.

mem: Add support for min reads per switch

OK

mem: Add HBM2 pseudo channel interface configuration

OK

src/mem/mem_interface.hh

src/mem/simple_mem_ctrl.cc

powerjg · 2022-05-06T17:37:43Z

src/mem/simple_mem_ctrl.cc

    writeQueue.resize(p.qos_priorities);

-    dram->setCtrl(this, commandWindow);
+    dram->setCtrl(this, commandWindow, 0);


If default 0, no need to change

powerjg · 2022-05-06T17:44:39Z

src/mem/hbm_ctrl.cc

+    fatal_if(!ch0_int, "Memory controller must have ch0 interface");
+    fatal_if(!ch1_int, "Memory controller must have ch1 interface");


Should we add something to make sure these interfaces are the same type?

src/mem/hbm_ctrl.hh

src/mem/simple_mem_ctrl.hh

src/mem/dram_interface.cc

src/mem/dram_interface.hh

Change-Id: I1822f68aa962c27b0e2de175dda6a0857e78fce4

Change-Id: If0ffefec43a37fb3ba0fe96a69b8a616bd86d1b5

Change-Id: I3c751837ac3bd02d5941a54c23ee6d9660f0ad59

Change-Id: If4ab5274fce6f83a5a12a78e9fcd8bae2195c3bb

Change-Id: Iab46c6c4a19c1919ec817ce950b8475fc68b4777

Change-Id: I113f73337d80487acdcbe2815d0011e6f3116199

Change-Id: Iff8b219cc82b86b4f5c1f2ae92382e242dd4a039

Change-Id: I6f2fa14e28e3c1769710537c6b0ca5ef945bfa6f

Change-Id: I5e3647fb026e7a175cc3e6be87554cd4d7a34927

Change-Id: I66bd138e4773c32c22156b877426fc73a8c593fa

Change-Id: Id34930d7f00250b1a3ddd4a7606d8300279387b8

Change-Id: I0cd62e1969544c46008d9b3f34661de57adaa703

Change-Id: If91f357821029397a60a78822db1c90fc2f17a48

Change-Id: I3775bea45d9028defeb3cfe5856af0366b981241

Change-Id: Ifb9bad040e8a5a9baa4b1dfbfa27823a87bb24c6

Change-Id: Id2f02b7c12632d908d420c05f4acb1ad2ef55166

powerjg

Lots of small style things. One bigger thing that we should sit down and chat about.

src/mem/dram_interface.cc

powerjg · 2022-05-11T16:58:55Z

src/mem/mem_interface.hh

+    /**
+     * pseudo channel number used for HBM modeling
+     */
+    uint8_t channel_num;


Suggested change

uint8_t channel_num;

uint8_t channelNum;

powerjg · 2022-05-11T16:59:23Z

src/mem/mem_interface.hh

    virtual MemPacket* decodePacket(const PacketPtr pkt, Addr pkt_addr,
-                           unsigned int size, bool is_read)
+                                    unsigned int size, bool is_read,
+                                    uint8_t channel)


src/mem/dram_interface.cc

powerjg · 2022-05-11T17:01:07Z

src/mem/dram_interface.cc

 MemPacket*
 DRAMInterface::decodePacket(const PacketPtr pkt, Addr pkt_addr,
-                       unsigned size, bool is_read)
+                       unsigned size, bool is_read, uint8_t channel)


probably should be more verbose and call this the pseudo_channel

Renamed it to pseudo channel everywhere (to avoid any confusion)

src/mem/hbm_ctrl.cc

powerjg · 2022-05-11T17:13:52Z

src/mem/hbm_ctrl.cc

+    auto it = rowBurstTicks.begin();
+    while (it != rowBurstTicks.end()) {
+        auto current_it = it++;


Suggested change

auto it = rowBurstTicks.begin();

while (it != rowBurstTicks.end()) {

auto current_it = it++;

for (auto &it : rowBurstTicks) {

powerjg · 2022-05-11T17:14:13Z

src/mem/hbm_ctrl.cc

+    auto it = colBurstTicks.begin();
+    while (it != colBurstTicks.end())
+    {
+        auto current_it = it++;


Ditto above

powerjg · 2022-05-11T17:14:53Z

src/mem/hbm_ctrl.cc

+    }
+
+    // update the mode
+    isTimingMode = system()->isTimingMode();


Is this not handled in SimpleMemCtrl::drainResume?

Change-Id: Ifd60610fc40aae32383339688fe4a0be1b517254

Change-Id: I3d4a884074014216268861b023d833b2db8b3bdb

Change-Id: I225e9e135274b3cfa53658b841dbaa9a7c10abd2

Change-Id: I928ebfbd79b184deea3d48b668c7c3ce62b9957e

aakahlow force-pushed the hbmctrl-rest branch 3 times, most recently from b0429e0 to 95d4ff2 Compare May 6, 2022 03:49

aakahlow mentioned this pull request May 6, 2022

Adds SimpleMemCtrl and MemCtrl #23

Open

powerjg reviewed May 6, 2022

View reviewed changes

aakahlow added 9 commits May 8, 2022 21:14

mem: Add a notion of pseudo channel in memory interface

7bead27

Change-Id: I1822f68aa962c27b0e2de175dda6a0857e78fce4

mem: Add hbm controller

2339766

Change-Id: If0ffefec43a37fb3ba0fe96a69b8a616bd86d1b5

mem: move nextBurstAt. and nextReqTime to memory interface

2873fac

Change-Id: I3c751837ac3bd02d5941a54c23ee6d9660f0ad59

mem: hbm ctrl -- check which scheduler to restart after refresh

bd18795

Change-Id: If4ab5274fce6f83a5a12a78e9fcd8bae2195c3bb

mem: support for separate tRCD and tCL for reads/writes

e99bb8d

Change-Id: Iab46c6c4a19c1919ec817ce950b8475fc68b4777

mem: some fixes for hbm ctrl

b6f2759

Change-Id: I113f73337d80487acdcbe2815d0011e6f3116199

mem: Add support for min reads per switch

e5ca627

Change-Id: Iff8b219cc82b86b4f5c1f2ae92382e242dd4a039

mem: Add HBM2 pseudo channel interface configuration

e2ae575

Change-Id: I6f2fa14e28e3c1769710537c6b0ca5ef945bfa6f

mem: some fixes after broken rebase

73c366b

Change-Id: I5e3647fb026e7a175cc3e6be87554cd4d7a34927

aakahlow force-pushed the hbmctrl-rest branch from 95d4ff2 to 73c366b Compare May 9, 2022 07:00

aakahlow added 7 commits May 9, 2022 15:50

mem: set channel=0 default in setCtrl

b0d4f6b

Change-Id: I66bd138e4773c32c22156b877426fc73a8c593fa

mem: get rid of use of HBMCtrl type from mem interface

72e8362

Change-Id: Id34930d7f00250b1a3ddd4a7606d8300279387b8

mem: fix copyright

9407b4b

Change-Id: I0cd62e1969544c46008d9b3f34661de57adaa703

mem: hbm ctrl fixes based on comments

29e374a

Change-Id: If91f357821029397a60a78822db1c90fc2f17a48

mem: fix readQueueFull signature

68967cd

Change-Id: I3775bea45d9028defeb3cfe5856af0366b981241

mem: fix indendation

78d4748

Change-Id: Ifb9bad040e8a5a9baa4b1dfbfa27823a87bb24c6

mem: Update style

27ee761

Change-Id: Id2f02b7c12632d908d420c05f4acb1ad2ef55166

powerjg reviewed May 11, 2022

View reviewed changes

aakahlow added 4 commits May 11, 2022 23:28

mem: rename channel num to pseudo channel

eab1cca

Change-Id: Ifd60610fc40aae32383339688fe4a0be1b517254

mem: fix styling issues -- 1

42a538f

Change-Id: I3d4a884074014216268861b023d833b2db8b3bdb

mem: more cleanup

63bcc43

Change-Id: I225e9e135274b3cfa53658b841dbaa9a7c10abd2

mem: mem: fix styling issues -- 2

c329eb4

Change-Id: I928ebfbd79b184deea3d48b668c7c3ce62b9957e

		fatal_if(!ch0_int, "Memory controller must have ch0 interface");
		fatal_if(!ch1_int, "Memory controller must have ch1 interface");

Conversation

aakahlow commented May 5, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

powerjg left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

powerjg left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

aakahlow commented May 5, 2022 •

edited

Loading