base,arch: Fixed usage of `bitfield::replaceBits`

`bitfield::replaceBits` has two parameters, `first` and `last`, which
relate to the position of the MSB and the LSB of the bits to be replaced
respectively. Therefore `first` >= `last`. In some areas of the
codebase, this assumption has been flipped with `first` <= `last`. This
caused at least one known error, recorded here:
https://gem5.atlassian.net/browse/GEM5-695. These inconsistencies have
therefore been rectified.

A note has been added to the `bitfield::replaceBits` Doxygen to make
the usage of this function clearer.

Change-Id: Ie75856161d9a5684066430ecbdcc52e04e1e77bf
Issue-on: https://gem5.atlassian.net/browse/GEM5-696
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/31674
Reviewed-by: Bobby R. Bruce <bbruce@ucdavis.edu>
Maintainer: Bobby R. Bruce <bbruce@ucdavis.edu>
Tested-by: kokoro <noreply+kokoro@google.com>

diff --git a/src/arch/mips/isa.cc b/src/arch/mips/isa.cc
index 616876cebab6ff479a3f4516ef1d5956cc6d84af..5fbb6cf06475038b608cbcd98a6d4b481f2e2ba5 100644 (file)
--- a/src/arch/mips/isa.cc
+++ b/src/arch/mips/isa.cc
@@ -184,7 +184,7 @@ ISA::configCP()
 
     // Now, create Write Mask for ProcID register
     RegVal procIDMask = 0; // Read-Only register
-    replaceBits(procIDMask, 0, 32, 0);
+    replaceBits(procIDMask, 32, 0, 0);
     setRegMask(MISCREG_PRID, procIDMask);
 
     // Config
@@ -198,7 +198,7 @@ ISA::configCP()
     setMiscRegNoEffect(MISCREG_CONFIG, cfg);
     // Now, create Write Mask for Config register
     RegVal cfg_Mask = 0x7FFF0007;
-    replaceBits(cfg_Mask, 0, 32, 0);
+    replaceBits(cfg_Mask, 32, 0, 0);
     setRegMask(MISCREG_CONFIG, cfg_Mask);
 
     // Config1
@@ -220,7 +220,7 @@ ISA::configCP()
     setMiscRegNoEffect(MISCREG_CONFIG1, cfg1);
     // Now, create Write Mask for Config register
     RegVal cfg1_Mask = 0; // Read Only Register
-    replaceBits(cfg1_Mask, 0, 32, 0);
+    replaceBits(cfg1_Mask, 32,0 , 0);
     setRegMask(MISCREG_CONFIG1, cfg1_Mask);
 
     // Config2
@@ -237,7 +237,7 @@ ISA::configCP()
     setMiscRegNoEffect(MISCREG_CONFIG2, cfg2);
     // Now, create Write Mask for Config register
     RegVal cfg2_Mask = 0x7000F000; // Read Only Register
-    replaceBits(cfg2_Mask, 0, 32, 0);
+    replaceBits(cfg2_Mask, 32, 0, 0);
     setRegMask(MISCREG_CONFIG2, cfg2_Mask);
 
     // Config3
@@ -253,7 +253,7 @@ ISA::configCP()
     setMiscRegNoEffect(MISCREG_CONFIG3, cfg3);
     // Now, create Write Mask for Config register
     RegVal cfg3_Mask = 0; // Read Only Register
-    replaceBits(cfg3_Mask, 0, 32, 0);
+    replaceBits(cfg3_Mask, 32,0 , 0);
     setRegMask(MISCREG_CONFIG3, cfg3_Mask);
 
     // EBase - CPUNum
@@ -264,7 +264,7 @@ ISA::configCP()
     // Now, create Write Mask for Config register
     RegVal EB_Mask = 0x3FFFF000;// Except Exception Base, the
                                  // entire register is read only
-    replaceBits(EB_Mask, 0, 32, 0);
+    replaceBits(EB_Mask, 32, 0, 0);
     setRegMask(MISCREG_EBASE, EB_Mask);
 
     // SRS Control - HSS (Highest Shadow Set)
@@ -273,7 +273,7 @@ ISA::configCP()
     setMiscRegNoEffect(MISCREG_SRSCTL, scsCtl);
     // Now, create Write Mask for the SRS Ctl register
     RegVal SC_Mask = 0x0000F3C0;
-    replaceBits(SC_Mask, 0, 32, 0);
+    replaceBits(SC_Mask, 32, 0, 0);
     setRegMask(MISCREG_SRSCTL, SC_Mask);
 
     // IntCtl - IPTI, IPPCI
@@ -283,7 +283,7 @@ ISA::configCP()
     setMiscRegNoEffect(MISCREG_INTCTL, intCtl);
     // Now, create Write Mask for the IntCtl register
     RegVal IC_Mask = 0x000003E0;
-    replaceBits(IC_Mask, 0, 32, 0);
+    replaceBits(IC_Mask, 32, 0, 0);
     setRegMask(MISCREG_INTCTL, IC_Mask);
 
     // Watch Hi - M - FIXME (More than 1 Watch register)
@@ -292,7 +292,7 @@ ISA::configCP()
     setMiscRegNoEffect(MISCREG_WATCHHI0, watchHi);
     // Now, create Write Mask for the IntCtl register
     RegVal wh_Mask = 0x7FFF0FFF;
-    replaceBits(wh_Mask, 0, 32, 0);
+    replaceBits(wh_Mask, 32, 0, 0);
     setRegMask(MISCREG_WATCHHI0, wh_Mask);
 
     // Perf Ctr - M - FIXME (More than 1 PerfCnt Pair)
@@ -302,14 +302,14 @@ ISA::configCP()
     setMiscRegNoEffect(MISCREG_PERFCNT0, perfCntCtl);
     // Now, create Write Mask for the IntCtl register
     RegVal pc_Mask = 0x00007FF;
-    replaceBits(pc_Mask, 0, 32, 0);
+    replaceBits(pc_Mask, 32, 0, 0);
     setRegMask(MISCREG_PERFCNT0, pc_Mask);
 
     // Random
     setMiscRegNoEffect(MISCREG_CP0_RANDOM, 63);
     // Now, create Write Mask for the IntCtl register
     RegVal random_Mask = 0;
-    replaceBits(random_Mask, 0, 32, 0);
+    replaceBits(random_Mask, 32, 0, 0);
     setRegMask(MISCREG_CP0_RANDOM, random_Mask);
 
     // PageGrain
@@ -318,7 +318,7 @@ ISA::configCP()
     setMiscRegNoEffect(MISCREG_PAGEGRAIN, pageGrain);
     // Now, create Write Mask for the IntCtl register
     RegVal pg_Mask = 0x10000000;
-    replaceBits(pg_Mask, 0, 32, 0);
+    replaceBits(pg_Mask, 32, 0, 0);
     setRegMask(MISCREG_PAGEGRAIN, pg_Mask);
 
     // Status
@@ -337,7 +337,7 @@ ISA::configCP()
     setMiscRegNoEffect(MISCREG_STATUS, status);
     // Now, create Write Mask for the Status register
     RegVal stat_Mask = 0xFF78FF17;
-    replaceBits(stat_Mask, 0, 32, 0);
+    replaceBits(stat_Mask, 32, 0, 0);
     setRegMask(MISCREG_STATUS, stat_Mask);
 
 
@@ -381,29 +381,29 @@ ISA::configCP()
     RegVal mask = 0x7FFFFFFF;
 
     // Now, create Write Mask for the Index register
-    replaceBits(mask, 0, 32, 0);
+    replaceBits(mask, 32, 0, 0);
     setRegMask(MISCREG_INDEX, mask);
 
     mask = 0x3FFFFFFF;
-    replaceBits(mask, 0, 32, 0);
+    replaceBits(mask, 32, 0, 0);
     setRegMask(MISCREG_ENTRYLO0, mask);
     setRegMask(MISCREG_ENTRYLO1, mask);
 
     mask = 0xFF800000;
-    replaceBits(mask, 0, 32, 0);
+    replaceBits(mask, 32, 0, 0);
     setRegMask(MISCREG_CONTEXT, mask);
 
     mask = 0x1FFFF800;
-    replaceBits(mask, 0, 32, 0);
+    replaceBits(mask, 32, 0, 0);
     setRegMask(MISCREG_PAGEMASK, mask);
 
     mask = 0x0;
-    replaceBits(mask, 0, 32, 0);
+    replaceBits(mask, 32, 0, 0);
     setRegMask(MISCREG_BADVADDR, mask);
     setRegMask(MISCREG_LLADDR, mask);
 
     mask = 0x08C00300;
-    replaceBits(mask, 0, 32, 0);
+    replaceBits(mask, 32, 0, 0);
     setRegMask(MISCREG_CAUSE, mask);
 
 }

diff --git a/src/arch/x86/bios/intelmp.cc b/src/arch/x86/bios/intelmp.cc
index f7e99949be499372c2a9d46e2a883360c5caee79..55088fd803e7b0cf0601d6997ec621d4506bf199 100644 (file)
--- a/src/arch/x86/bios/intelmp.cc
+++ b/src/arch/x86/bios/intelmp.cc
@@ -284,9 +284,9 @@ X86ISA::IntelMP::Processor::Processor(Params * p) : BaseConfigEntry(p, 0),
     if (p->bootstrap)
         cpuFlags |= (1 << 1);
 
-    replaceBits(cpuSignature, 0, 3, p->stepping);
-    replaceBits(cpuSignature, 4, 7, p->model);
-    replaceBits(cpuSignature, 8, 11, p->family);
+    replaceBits(cpuSignature, 3, 0, p->stepping);
+    replaceBits(cpuSignature, 7, 4, p->model);
+    replaceBits(cpuSignature, 11, 8, p->family);
 }
 
 X86ISA::IntelMP::Processor *

diff --git a/src/arch/x86/bios/intelmp.hh b/src/arch/x86/bios/intelmp.hh
index fbc5775ee0c58e611afa4a04f097ab28668add3d..2ee400f4c22b8723e1acac496902cd60d4635e4c 100644 (file)
--- a/src/arch/x86/bios/intelmp.hh
+++ b/src/arch/x86/bios/intelmp.hh
@@ -239,8 +239,8 @@ class IntAssignment : public BaseConfigEntry
         sourceBusID(_sourceBusID), sourceBusIRQ(_sourceBusIRQ),
         destApicID(_destApicID), destApicIntIn(_destApicIntIn)
     {
-        replaceBits(flags, 0, 1, polarity);
-        replaceBits(flags, 2, 3, trigger);
+        replaceBits(flags, 1, 0, polarity);
+        replaceBits(flags, 3, 2, trigger);
     }
 };
 

diff --git a/src/base/bitfield.hh b/src/base/bitfield.hh
index 82c33071b666516c709baafcb706c9fc9948490a..5cde01963264f4b4cba756a33b6ddb9682b78fff 100644 (file)
--- a/src/base/bitfield.hh
+++ b/src/base/bitfield.hh
@@ -150,7 +150,9 @@ insertBits(T val, int bit, B bit_val)
 
 /**
  * A convenience function to replace bits first to last of val with bit_val
- * in place.
+ * in place. It is functionally equivalent to insertBits.
+ *
+ * \note "first" is the MSB and "last" is the LSB. "first" >= "last"
  */
 template <class T, class B>
 inline