[PATCH v3] drm/xe/gt: Fix assert in L3 bank mask generation

[Lucas De Marchi]

On Tue, Apr 30, 2024 at 04:39:05PM GMT, Francois Dugast wrote:
>What needs to be asserted is that the pattern fits in the number
>of bits provided by the user in patternbits, otherwise it would
>be truncated when replicated according to the mask, which is
>likely not the intended use of this function.
>The pattern argument is a bitmap so use find_last_bit() instead
>of fls(). The bit position starts at index 0 so remove "or equal"
>from the comparison. XE_MAX_L3_BANK_MASK_BITS would be the
>returned value if the pattern is 0, which can be the case on some
>platforms.
>
>v2: Check the result does not overflow the array (Lucas De Marchi)
>
>Cc: Matt Roper <matthew.d.roper at intel.com>
>Cc: Lucas De Marchi <lucas.demarchi at intel.com>
>Signed-off-by: Francois Dugast <francois.dugast at intel.com>
>---
> drivers/gpu/drm/xe/xe_gt_topology.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/gpu/drm/xe/xe_gt_topology.c b/drivers/gpu/drm/xe/xe_gt_topology.c
>index af841d801a8f..b51f40493123 100644
>--- a/drivers/gpu/drm/xe/xe_gt_topology.c
>+++ b/drivers/gpu/drm/xe/xe_gt_topology.c
>@@ -108,7 +108,9 @@ gen_l3_mask_from_pattern(struct xe_device *xe, xe_l3_bank_mask_t dst,
> {
> 	unsigned long bit;
>
>-	xe_assert(xe, fls(mask) <= patternbits);
>+	xe_assert(xe, find_last_bit(pattern, XE_MAX_L3_BANK_MASK_BITS) < patternbits ||
>+		  bitmap_empty(pattern, XE_MAX_L3_BANK_MASK_BITS));
>+	xe_assert(xe, patternbits * fls(mask) <= XE_MAX_L3_BANK_MASK_BITS);

sorry, I didn't notice before that mask is a long when suggesting
this... just copied from what we had. It seems we need to do:

	xe_assert(xe, !mask || patternbits * (__fls(mask) + 1) <= XE_MAX_L3_BANK_MASK_BITS);

I still think calling this with mask == 0 or pattern == 0 is a problem
the caller should deal with, but if we are dealing with one of them ==
0, we should also deal with the other.

with that,

Reviewed-by: Lucas De Marchi <lucas.demarchi at intel.com>

Lucas De Marchi

> 	for_each_set_bit(bit, &mask, 32) {
> 		xe_l3_bank_mask_t shifted_pattern = {};
>
>-- 
>2.34.1
>