Bobby R. Bruce has submitted this change. (
https://gem5-review.googlesource.com/c/public/gem5/+/31674 )
Change subject: base,arch: Fixed usage of `bitfield::replaceBits`
......................................................................
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 <[email protected]>
Maintainer: Bobby R. Bruce <[email protected]>
Tested-by: kokoro <[email protected]>
---
M src/arch/mips/isa.cc
M src/arch/x86/bios/intelmp.cc
M src/arch/x86/bios/intelmp.hh
M src/base/bitfield.hh
4 files changed, 27 insertions(+), 25 deletions(-)
Approvals:
Bobby R. Bruce: Looks good to me, approved; Looks good to me, approved
kokoro: Regressions pass
diff --git a/src/arch/mips/isa.cc b/src/arch/mips/isa.cc
index 616876c..5fbb6cf 100644
--- a/src/arch/mips/isa.cc
+++ b/src/arch/mips/isa.cc
@@ -184,7 +184,7 @@
// 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 @@
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 @@
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 @@
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 @@
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 @@
// 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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 f7e9994..55088fd 100644
--- a/src/arch/x86/bios/intelmp.cc
+++ b/src/arch/x86/bios/intelmp.cc
@@ -284,9 +284,9 @@
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 fbc5775..2ee400f 100644
--- a/src/arch/x86/bios/intelmp.hh
+++ b/src/arch/x86/bios/intelmp.hh
@@ -239,8 +239,8 @@
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 82c3307..5cde019 100644
--- a/src/base/bitfield.hh
+++ b/src/base/bitfield.hh
@@ -150,7 +150,9 @@
/**
* 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
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/31674
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: Ie75856161d9a5684066430ecbdcc52e04e1e77bf
Gerrit-Change-Number: 31674
Gerrit-PatchSet: 3
Gerrit-Owner: Bobby R. Bruce <[email protected]>
Gerrit-Reviewer: Alec Roelke <[email protected]>
Gerrit-Reviewer: Anthony Gutierrez <[email protected]>
Gerrit-Reviewer: Bobby R. Bruce <[email protected]>
Gerrit-Reviewer: Gabe Black <[email protected]>
Gerrit-Reviewer: Jason Lowe-Power <[email protected]>
Gerrit-Reviewer: kokoro <[email protected]>
Gerrit-MessageType: merged
_______________________________________________
gem5-dev mailing list -- [email protected]
To unsubscribe send an email to [email protected]
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
