| Message ID | 20211211191135.1764649-14-peter.maydell@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | arm gicv3 ITS: Various bug fixes and refactorings | expand |
On 12/11/21 11:11 AM, Peter Maydell wrote: > Use FIELD macros to handle CTEs, rather than ad-hoc mask-and-shift. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > hw/intc/gicv3_internal.h | 3 ++- > hw/intc/arm_gicv3_its.c | 7 ++++--- > 2 files changed, 6 insertions(+), 4 deletions(-) Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
Peter Maydell <peter.maydell@linaro.org> writes: > Use FIELD macros to handle CTEs, rather than ad-hoc mask-and-shift. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > hw/intc/gicv3_internal.h | 3 ++- > hw/intc/arm_gicv3_its.c | 7 ++++--- > 2 files changed, 6 insertions(+), 4 deletions(-) > > diff --git a/hw/intc/gicv3_internal.h b/hw/intc/gicv3_internal.h > index 14e8ef68e02..1eeb99035da 100644 > --- a/hw/intc/gicv3_internal.h > +++ b/hw/intc/gicv3_internal.h > @@ -403,7 +403,8 @@ FIELD(DTE, ITTADDR, 6, 44) > * Valid = 1 bit, RDBase = 16 bits > */ > #define GITS_CTE_SIZE (0x8ULL) > -#define GITS_CTE_RDBASE_PROCNUM_MASK MAKE_64BIT_MASK(1, RDBASE_PROCNUM_LENGTH) > +FIELD(CTE, VALID, 0, 1) > +FIELD(CTE, RDBASE, 1, RDBASE_PROCNUM_LENGTH) > > /* Special interrupt IDs */ > #define INTID_SECURE 1020 > diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c > index d6637229479..ab6ce09dbc2 100644 > --- a/hw/intc/arm_gicv3_its.c > +++ b/hw/intc/arm_gicv3_its.c > @@ -104,7 +104,7 @@ static bool get_cte(GICv3ITSState *s, uint16_t icid, uint64_t *cte, > MEMTXATTRS_UNSPECIFIED, res); > } > > - return (*cte & TABLE_ENTRY_VALID_MASK) != 0; > + return FIELD_EX64(*cte, CTE, VALID); > } > > static bool update_ite(GICv3ITSState *s, uint32_t eventid, uint64_t dte, > @@ -308,7 +308,7 @@ static bool process_its_cmd(GICv3ITSState *s, uint64_t value, uint32_t offset, > * Current implementation only supports rdbase == procnum > * Hence rdbase physical address is ignored > */ > - rdbase = (cte & GITS_CTE_RDBASE_PROCNUM_MASK) >> 1U; > + rdbase = FIELD_EX64(cte, CTE, RDBASE); > > if (rdbase >= s->gicv3->num_cpu) { > return result; > @@ -426,7 +426,8 @@ static bool update_cte(GICv3ITSState *s, uint16_t icid, bool valid, > > if (valid) { > /* add mapping entry to collection table */ > - cte = (valid & TABLE_ENTRY_VALID_MASK) | (rdbase << 1ULL); > + cte = FIELD_DP64(cte, CTE, VALID, 1); > + cte = FIELD_DP64(cte, CTE, RDBASE, rdbase); I almost flagged this until I realised the double deposit are additive and the same as the bare | Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
diff --git a/hw/intc/gicv3_internal.h b/hw/intc/gicv3_internal.h index 14e8ef68e02..1eeb99035da 100644 --- a/hw/intc/gicv3_internal.h +++ b/hw/intc/gicv3_internal.h @@ -403,7 +403,8 @@ FIELD(DTE, ITTADDR, 6, 44) * Valid = 1 bit, RDBase = 16 bits */ #define GITS_CTE_SIZE (0x8ULL) -#define GITS_CTE_RDBASE_PROCNUM_MASK MAKE_64BIT_MASK(1, RDBASE_PROCNUM_LENGTH) +FIELD(CTE, VALID, 0, 1) +FIELD(CTE, RDBASE, 1, RDBASE_PROCNUM_LENGTH) /* Special interrupt IDs */ #define INTID_SECURE 1020 diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c index d6637229479..ab6ce09dbc2 100644 --- a/hw/intc/arm_gicv3_its.c +++ b/hw/intc/arm_gicv3_its.c @@ -104,7 +104,7 @@ static bool get_cte(GICv3ITSState *s, uint16_t icid, uint64_t *cte, MEMTXATTRS_UNSPECIFIED, res); } - return (*cte & TABLE_ENTRY_VALID_MASK) != 0; + return FIELD_EX64(*cte, CTE, VALID); } static bool update_ite(GICv3ITSState *s, uint32_t eventid, uint64_t dte, @@ -308,7 +308,7 @@ static bool process_its_cmd(GICv3ITSState *s, uint64_t value, uint32_t offset, * Current implementation only supports rdbase == procnum * Hence rdbase physical address is ignored */ - rdbase = (cte & GITS_CTE_RDBASE_PROCNUM_MASK) >> 1U; + rdbase = FIELD_EX64(cte, CTE, RDBASE); if (rdbase >= s->gicv3->num_cpu) { return result; @@ -426,7 +426,8 @@ static bool update_cte(GICv3ITSState *s, uint16_t icid, bool valid, if (valid) { /* add mapping entry to collection table */ - cte = (valid & TABLE_ENTRY_VALID_MASK) | (rdbase << 1ULL); + cte = FIELD_DP64(cte, CTE, VALID, 1); + cte = FIELD_DP64(cte, CTE, RDBASE, rdbase); } /*
Use FIELD macros to handle CTEs, rather than ad-hoc mask-and-shift. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/intc/gicv3_internal.h | 3 ++- hw/intc/arm_gicv3_its.c | 7 ++++--- 2 files changed, 6 insertions(+), 4 deletions(-)