From: Alexander Graf <agraf@csgraf.de>
To: Peter Maydell <peter.maydell@linaro.org>
Cc: Roman Bolshakov <r.bolshakov@yadro.com>,
qemu-arm@nongnu.org, qemu-devel@nongnu.org,
Cameron Esfahani <dirty@apple.com>,
Ivan Babrou <ivan@cloudflare.com>
Subject: [PATCH v3 1/2] hvf: arm: Use macros for sysreg shift/masking
Date: Wed, 9 Feb 2022 13:41:34 +0100 [thread overview]
Message-ID: <20220209124135.69183-1-agraf@csgraf.de> (raw)
We are parsing the syndrome field for sysregs in multiple places across
the hvf code, but repeat shift/mask operations with hard coded constants
every time. This is an error prone approach and makes it harder to reason
about the correctness of these operations.
Let's introduce macros that allow us to unify the constants used as well
as create new helpers to extract fields from the sysreg value.
Suggested-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Alexander Graf <agraf@csgraf.de>
---
target/arm/hvf/hvf.c | 69 ++++++++++++++++++++++++++++++--------------
1 file changed, 47 insertions(+), 22 deletions(-)
diff --git a/target/arm/hvf/hvf.c b/target/arm/hvf/hvf.c
index 92ad0d29c4..8d0447ab01 100644
--- a/target/arm/hvf/hvf.c
+++ b/target/arm/hvf/hvf.c
@@ -35,9 +35,34 @@
ENCODE_AA64_CP_REG(CP_REG_ARM64_SYSREG_CP, crn, crm, op0, op1, op2)
#define PL1_WRITE_MASK 0x4
+#define SYSREG_OP0_SHIFT 20
+#define SYSREG_OP0_MASK 0x3
+#define SYSREG_OP0(sysreg) ((sysreg >> SYSREG_OP0_SHIFT) & SYSREG_OP0_MASK)
+#define SYSREG_OP1_SHIFT 14
+#define SYSREG_OP1_MASK 0x7
+#define SYSREG_OP1(sysreg) ((sysreg >> SYSREG_OP1_SHIFT) & SYSREG_OP1_MASK)
+#define SYSREG_CRN_SHIFT 10
+#define SYSREG_CRN_MASK 0xf
+#define SYSREG_CRN(sysreg) ((sysreg >> SYSREG_CRN_SHIFT) & SYSREG_CRN_MASK)
+#define SYSREG_CRM_SHIFT 1
+#define SYSREG_CRM_MASK 0xf
+#define SYSREG_CRM(sysreg) ((sysreg >> SYSREG_CRM_SHIFT) & SYSREG_CRM_MASK)
+#define SYSREG_OP2_SHIFT 17
+#define SYSREG_OP2_MASK 0x7
+#define SYSREG_OP2(sysreg) ((sysreg >> SYSREG_OP2_SHIFT) & SYSREG_OP2_MASK)
+
#define SYSREG(op0, op1, crn, crm, op2) \
- ((op0 << 20) | (op2 << 17) | (op1 << 14) | (crn << 10) | (crm << 1))
-#define SYSREG_MASK SYSREG(0x3, 0x7, 0xf, 0xf, 0x7)
+ ((op0 << SYSREG_OP0_SHIFT) | \
+ (op1 << SYSREG_OP1_SHIFT) | \
+ (crn << SYSREG_CRN_SHIFT) | \
+ (crm << SYSREG_CRM_SHIFT) | \
+ (op2 << SYSREG_OP2_SHIFT))
+#define SYSREG_MASK \
+ SYSREG(SYSREG_OP0_MASK, \
+ SYSREG_OP1_MASK, \
+ SYSREG_CRN_MASK, \
+ SYSREG_CRM_MASK, \
+ SYSREG_OP2_MASK)
#define SYSREG_OSLAR_EL1 SYSREG(2, 0, 1, 0, 4)
#define SYSREG_OSLSR_EL1 SYSREG(2, 0, 1, 1, 4)
#define SYSREG_OSDLR_EL1 SYSREG(2, 0, 1, 3, 4)
@@ -783,21 +808,21 @@ static int hvf_sysreg_read(CPUState *cpu, uint32_t reg, uint32_t rt)
default:
cpu_synchronize_state(cpu);
trace_hvf_unhandled_sysreg_read(env->pc, reg,
- (reg >> 20) & 0x3,
- (reg >> 14) & 0x7,
- (reg >> 10) & 0xf,
- (reg >> 1) & 0xf,
- (reg >> 17) & 0x7);
+ SYSREG_OP0(reg),
+ SYSREG_OP1(reg),
+ SYSREG_CRN(reg),
+ SYSREG_CRM(reg),
+ SYSREG_OP2(reg));
hvf_raise_exception(cpu, EXCP_UDEF, syn_uncategorized());
return 1;
}
trace_hvf_sysreg_read(reg,
- (reg >> 20) & 0x3,
- (reg >> 14) & 0x7,
- (reg >> 10) & 0xf,
- (reg >> 1) & 0xf,
- (reg >> 17) & 0x7,
+ SYSREG_OP0(reg),
+ SYSREG_OP1(reg),
+ SYSREG_CRN(reg),
+ SYSREG_CRM(reg),
+ SYSREG_OP2(reg),
val);
hvf_set_reg(cpu, rt, val);
@@ -886,11 +911,11 @@ static int hvf_sysreg_write(CPUState *cpu, uint32_t reg, uint64_t val)
CPUARMState *env = &arm_cpu->env;
trace_hvf_sysreg_write(reg,
- (reg >> 20) & 0x3,
- (reg >> 14) & 0x7,
- (reg >> 10) & 0xf,
- (reg >> 1) & 0xf,
- (reg >> 17) & 0x7,
+ SYSREG_OP0(reg),
+ SYSREG_OP1(reg),
+ SYSREG_CRN(reg),
+ SYSREG_CRM(reg),
+ SYSREG_OP2(reg),
val);
switch (reg) {
@@ -960,11 +985,11 @@ static int hvf_sysreg_write(CPUState *cpu, uint32_t reg, uint64_t val)
default:
cpu_synchronize_state(cpu);
trace_hvf_unhandled_sysreg_write(env->pc, reg,
- (reg >> 20) & 0x3,
- (reg >> 14) & 0x7,
- (reg >> 10) & 0xf,
- (reg >> 1) & 0xf,
- (reg >> 17) & 0x7);
+ SYSREG_OP0(reg),
+ SYSREG_OP1(reg),
+ SYSREG_CRN(reg),
+ SYSREG_CRM(reg),
+ SYSREG_OP2(reg));
hvf_raise_exception(cpu, EXCP_UDEF, syn_uncategorized());
return 1;
}
--
2.32.0 (Apple Git-132)
next reply other threads:[~2022-02-09 12:57 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-02-09 12:41 Alexander Graf [this message]
2022-02-09 12:41 ` [PATCH v3 2/2] hvf: arm: Handle unknown ID registers as RES0 Alexander Graf
2022-02-16 17:32 ` Peter Maydell
2022-02-11 8:13 ` [PATCH v3 1/2] hvf: arm: Use macros for sysreg shift/masking Cameron Esfahani
2022-02-11 9:40 ` Philippe Mathieu-Daudé via
2022-02-16 17:35 ` Peter Maydell
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20220209124135.69183-1-agraf@csgraf.de \
--to=agraf@csgraf.de \
--cc=dirty@apple.com \
--cc=ivan@cloudflare.com \
--cc=peter.maydell@linaro.org \
--cc=qemu-arm@nongnu.org \
--cc=qemu-devel@nongnu.org \
--cc=r.bolshakov@yadro.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).