On Fri, 1 Mar 2024 at 21:19, Richard Henderson
<richard.hender...@linaro.org> wrote:
On 3/1/24 08:32, Peter Maydell wrote:
We prefer the FIELD macro over ad-hoc #defines for register bits;
switch CNTHCTL to that style before we add any more bits.
Signed-off-by: Peter Maydell <peter.mayd...@linaro.org>
---
target/arm/internals.h | 19 +++++++++++++++++--
target/arm/helper.c | 9 ++++-----
2 files changed, 21 insertions(+), 7 deletions(-)
diff --git a/target/arm/internals.h b/target/arm/internals.h
index c93acb270cc..6553e414934 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -224,8 +224,23 @@ FIELD(VTCR, SL2, 33, 1)
#define HSTR_TTEE (1 << 16)
#define HSTR_TJDBX (1 << 17)
-#define CNTHCTL_CNTVMASK (1 << 18)
-#define CNTHCTL_CNTPMASK (1 << 19)
+FIELD(CNTHCTL, EL0PCTEN, 0, 1)
+FIELD(CNTHCTL, EL0VCTEN, 1, 1)
+FIELD(CNTHCTL, EVNTEN, 2, 1)
+FIELD(CNTHCTL, EVNTDIR, 3, 1)
...
+FIELD(CNTHCTL, EL0VTEN, 8, 1)
+FIELD(CNTHCTL, EL0PTEN, 9, 1)
+FIELD(CNTHCTL, EL1PCTEN, 10, 1)
+FIELD(CNTHCTL, EL1PTEN, 11, 1)
These bits change definition based on HCR_E2H, which I remembered when I got to
patch 5,
which adds code nearby the existing tests of these bits.
It might be confusing to only provide the E2H versions, without some extra
suffix?
Yeah, bits 8..11 are RES0 if E2H is 0; bits 3 and 2 are the same;
bits 0 and 1 change (to EL1PCTEN and EL1PCEN, so bit 0 when E2H is 0
has the same name as bit 10 when E2H is 1).
I don't see the need to suffix the bits that are only relevant in
one context and RES0 in the other, only the ones where the bit has
the same name but a different location, or where the same bit
number has two names. So:
+/*
+ * Depending on the value of HCR_EL2.E2H, bits 0 and 1
+ * have different bit definitions, and EL1PCTEN might be
+ * bit 0 or bit 10. We use _E2H1 and _E2H0 suffixes to
+ * disambiguate if necessary.
+ */
+FIELD(CNTHCTL, EL0PCTEN_E2H1, 0, 1)
+FIELD(CNTHCTL, EL0VCTEN_E2H1, 1, 1)
+FIELD(CNTHCTL, EL1PCTEN_E2H0, 0, 1)
+FIELD(CNTHCTL, EL1PCEN_E2H0, 1, 1)
+FIELD(CNTHCTL, EVNTEN, 2, 1)
+FIELD(CNTHCTL, EVNTDIR, 3, 1)
+FIELD(CNTHCTL, EVNTI, 4, 4)
+FIELD(CNTHCTL, EL0VTEN, 8, 1)
+FIELD(CNTHCTL, EL0PTEN, 9, 1)
+FIELD(CNTHCTL, EL1PCTEN_E2H1, 10, 1)
+FIELD(CNTHCTL, EL1PTEN, 11, 1)
+FIELD(CNTHCTL, ECV, 12, 1)
+FIELD(CNTHCTL, EL1TVT, 13, 1)
+FIELD(CNTHCTL, EL1TVCT, 14, 1)
+FIELD(CNTHCTL, EL1NVPCT, 15, 1)
+FIELD(CNTHCTL, EL1NVVCT, 16, 1)
+FIELD(CNTHCTL, EVNTIS, 17, 1)
+FIELD(CNTHCTL, CNTVMASK, 18, 1)
+FIELD(CNTHCTL, CNTPMASK, 19, 1)
(and updating the uses in following patches as needed) ?
