|
From: | Greg Bellows |
Subject: | Re: [Qemu-devel] [PATCH 2/2] target-arm: Add checks that cpreg raw accesses are handled |
Date: | Mon, 19 Jan 2015 12:05:53 -0600 |
Add assertion checking when cpreg structures are registered that they
either forbid raw-access attempts or at least make an attempt at
handling them. Also add an assert in the raw-accessor-of-last-resort,
to avoid silently doing a read or write from offset zero, which is
actually AArch32 CPU register r0.
Signed-off-by: Peter Maydell <address@hidden>
---
target-arm/helper.c | 30 ++++++++++++++++++++++++++++++
1 file changed, 30 insertions(+)
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 18f04b2..39c0ad9 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -119,6 +119,7 @@ static int aarch64_fpu_gdb_set_reg(CPUARMState *env, uint8_t *buf, int reg)
static uint64_t raw_read(CPUARMState *env, const ARMCPRegInfo *ri)
{
+ assert(ri->fieldoffset);
if (cpreg_field_is_64bit(ri)) {
return CPREG_FIELD64(env, ri);
} else {
@@ -129,6 +130,7 @@ static uint64_t raw_read(CPUARMState *env, const ARMCPRegInfo *ri)
static void raw_write(CPUARMState *env, const ARMCPRegInfo *ri,
uint64_t value)
{
+ assert(ri->fieldoffset);
if (cpreg_field_is_64bit(ri)) {
CPREG_FIELD64(env, ri) = value;
} else {
@@ -174,6 +176,26 @@ static void write_raw_cp_reg(CPUARMState *env, const ARMCPRegInfo *ri,
}
}
+static bool raw_accessors_invalid(const ARMCPRegInfo *ri)
+{
+ /* Return true if the regdef would cause an assertion if you called
+ *read_raw_cp_reg() or write_raw_cp_reg() on it (ie if it is a
+ * program bug for it not to have the NO_RAW flag).
+ * NB that returning false here doesn't necessarily mean that calling
+ * read/write_raw_cp_reg() is safe, because we can't distinguish "has
+ * read/write access functions which are safe for raw use" from "has
+ * read/write access functions which have side effects but has forgotten
+ * to provide raw access functions".
+ * The tests here line up with the conditions in read/write_raw_cp_reg()
+ * and assertions in raw_read()/raw_write().
+ */
+ if (ri->type & ARM_CP_CONST) {
+ return true;
+ }
+ return (ri->raw_writefn || ri->writefn || ri->fieldoffset) &&
+ (ri->raw_readfn || ri->readfn || ri->fieldoffset);
+}
+
bool write_cpustate_to_list(ARMCPU *cpu)
{
/* Write the coprocessor state from cpu->env to the (index,value) list. */
@@ -3509,6 +3531,14 @@ static void add_cpreg_to_hashtable(ARMCPU *cpu, const ARMCPRegInfo *r,
r2->type |= ARM_CP_ALIAS;
}
+ /* Check that raw accesses are either forbidden or handled. Note that
+ * we can't assert this earlier because the setup of fieldoffset for
+ * banked registers has to be done first.
+ */
+ if (!(r2->type & ARM_CP_NO_RAW)) {
+ assert(!raw_accessors_invalid(r2));
+ }
+
/* Overriding of an existing definition must be explicitly
* requested.
*/
--
1.9.1
[Prev in Thread] | Current Thread | [Next in Thread] |