Skip to content

Commit b5fab34

Browse files
Alyssa RosenzweigSteven Price
authored andcommitted
drm/panfrost: Simplify lock_region calculation
In lock_region, simplify the calculation of the region_width parameter. This field is the size, but encoded as ceil(log2(size)) - 1. ceil(log2(size)) may be computed directly as fls(size - 1). However, we want to use the 64-bit versions as the amount to lock can exceed 32-bits. This avoids undefined (and completely wrong) behaviour when locking all memory (size ~0). In this case, the old code would "round up" ~0 to the nearest page, overflowing to 0. Since fls(0) == 0, this would calculate a region width of 10 + 0 = 10. But then the code would shift by (region_width - 11) = -1. As shifting by a negative number is undefined, UBSAN flags the bug. Of course, even if it were defined the behaviour is wrong, instead of locking all memory almost none would get locked. The new form of the calculation corrects this special case and avoids the undefined behaviour. Signed-off-by: Alyssa Rosenzweig <[email protected]> Reported-and-tested-by: Chris Morgan <[email protected]> Fixes: f3ba912 ("drm/panfrost: Add initial panfrost driver") Cc: <[email protected]> Reviewed-by: Steven Price <[email protected]> Reviewed-by: Rob Herring <[email protected]> Signed-off-by: Steven Price <[email protected]> Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
1 parent d03a493 commit b5fab34

File tree

1 file changed

+5
-14
lines changed

1 file changed

+5
-14
lines changed

drivers/gpu/drm/panfrost/panfrost_mmu.c

Lines changed: 5 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -62,21 +62,12 @@ static void lock_region(struct panfrost_device *pfdev, u32 as_nr,
6262
{
6363
u8 region_width;
6464
u64 region = iova & PAGE_MASK;
65-
/*
66-
* fls returns:
67-
* 1 .. 32
68-
*
69-
* 10 + fls(num_pages)
70-
* results in the range (11 .. 42)
71-
*/
72-
73-
size = round_up(size, PAGE_SIZE);
7465

75-
region_width = 10 + fls(size >> PAGE_SHIFT);
76-
if ((size >> PAGE_SHIFT) != (1ul << (region_width - 11))) {
77-
/* not pow2, so must go up to the next pow2 */
78-
region_width += 1;
79-
}
66+
/* The size is encoded as ceil(log2) minus(1), which may be calculated
67+
* with fls. The size must be clamped to hardware bounds.
68+
*/
69+
size = max_t(u64, size, PAGE_SIZE);
70+
region_width = fls64(size - 1) - 1;
8071
region |= region_width;
8172

8273
/* Lock the region that needs to be updated */

0 commit comments

Comments
 (0)