On 16 May 2018 at 16:16, Philippe Mathieu-Daudé <f4...@amsat.org> wrote: > Hi Eric, > > On 05/16/2018 03:03 PM, Eric Auger wrote: >> Coverity points out that this can overflow if n > 31, >> because it's only doing 32-bit arithmetic. Let's use 1ULL instead >> of 1. Also the formulae used to compute n can be replaced by >> the level_shift() macro. > > This level_shift() replacement doesn't seems that obvious to me, can you > split it in another patch? > >> >> Reported-by: Peter Maydell <peter.mayd...@linaro.org> >> Signed-off-by: Eric Auger <eric.au...@redhat.com> >> --- >> hw/arm/smmu-common.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c >> index 01c7be8..3c5f724 100644 >> --- a/hw/arm/smmu-common.c >> +++ b/hw/arm/smmu-common.c >> @@ -83,9 +83,9 @@ static inline hwaddr get_table_pte_address(uint64_t pte, >> int granule_sz) >> static inline hwaddr get_block_pte_address(uint64_t pte, int level, >> int granule_sz, uint64_t *bsz) >> { >> - int n = (granule_sz - 3) * (4 - level) + 3; >> + int n = level_shift(level, granule_sz); > > Shouldn't this be level_shift(level + 1, granule_sz)?
No. The two expressions are equivalent, they're just arranged differently: level_shift(lvl, gsz) == gsz + (3 - lvl) * (gsz - 3) == gsz + (4 - lvl) * (gsz - 3) - (gsz - 3) == gsz - gsz + (4 - lvl) * (gsz - 3) + 3 == (gsz - 3) * (4 - lvl) + 3 thanks -- PMM