From: Lucas De Marchi <lucas.demarchi@intel.com>
To: Francois Dugast <francois.dugast@intel.com>
Cc: <intel-xe@lists.freedesktop.org>, Matt Roper <matthew.d.roper@intel.com>
Subject: Re: [PATCH v3] drm/xe/gt: Fix assert in L3 bank mask generation
Date: Wed, 1 May 2024 14:15:29 -0500 [thread overview]
Message-ID: <z23x737lhyqilayigva6yptjw5dba6qdrhxn6lvm3dtfagxemk@sgyakeuwhobf> (raw)
In-Reply-To: <20240430163905.7-1-francois.dugast@intel.com>
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@intel.com>
>Cc: Lucas De Marchi <lucas.demarchi@intel.com>
>Signed-off-by: Francois Dugast <francois.dugast@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@intel.com>
Lucas De Marchi
> for_each_set_bit(bit, &mask, 32) {
> xe_l3_bank_mask_t shifted_pattern = {};
>
>--
>2.34.1
>
prev parent reply other threads:[~2024-05-01 19:15 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-30 16:39 [PATCH v3] drm/xe/gt: Fix assert in L3 bank mask generation Francois Dugast
2024-04-30 17:52 ` ✓ CI.Patch_applied: success for drm/xe/gt: Fix assert in L3 bank mask generation (rev3) Patchwork
2024-04-30 17:53 ` ✓ CI.checkpatch: " Patchwork
2024-04-30 17:54 ` ✓ CI.KUnit: " Patchwork
2024-04-30 18:06 ` ✓ CI.Build: " Patchwork
2024-04-30 18:08 ` ✓ CI.Hooks: " Patchwork
2024-04-30 18:10 ` ✓ CI.checksparse: " Patchwork
2024-04-30 18:38 ` ✓ CI.BAT: " Patchwork
2024-04-30 20:34 ` ✗ CI.FULL: failure " Patchwork
2024-05-01 19:15 ` Lucas De Marchi [this message]
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=z23x737lhyqilayigva6yptjw5dba6qdrhxn6lvm3dtfagxemk@sgyakeuwhobf \
--to=lucas.demarchi@intel.com \
--cc=francois.dugast@intel.com \
--cc=intel-xe@lists.freedesktop.org \
--cc=matthew.d.roper@intel.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 read-only IMAP folder(s) and NNTP newsgroup(s).