Chris Morgan reported UBSAN errors in panfrost and tracked them down to
the size computation in lock_region. This calculation is overcomplicated
(cargo culted from kbase) and can be simplified with kernel helpers and
some mathematical identities. The first patch in the series rewrites the
calculation in a form avoiding undefined behaviour; Chris confirms it
placates UBSAN.
While researching this function, I noticed a pair of other potential
bugs: Bifrost can lock more than 4GiB at a time, but must lock at least
32KiB at a time. The latter patches in the series handle these cases.
In review of v1 of this series, Steven pointed out a fourth potential
bug: rounding down the iova can truncate the lock region. v2 adds a new
patch for this case.
The size computation was unit-tested in userspace. Relevant code below,
just missing some copypaste definitions for fls64/clamp/etc:
#define MIN_LOCK (1ULL << 12)
#define MAX_LOCK (1ULL << 48)
struct {
uint64_t size;
uint8_t encoded;
} tests[] = {
/* Clamping */
{ 0, 11 },
{ 1, 11 },
{ 2, 11 },
{ 4095, 11 },
/* Power of two */
{ 4096, 11 },
/* Round up */
{ 4097, 12 },
{ 8192, 12 },
{ 16384, 13 },
{ 16385, 14 },
/* Maximum */
{ ~0ULL, 47 },
};
static uint8_t region_width(uint64_t size)
{
size = clamp(size, MIN_LOCK, MAX_LOCK);
return fls64(size - 1) - 1;
}
int main(int argc, char **argv)
{
for (unsigned i = 0; i < ARRAY_SIZE(tests); ++i) {
uint64_t test = tests[i].size;
uint8_t expected = tests[i].encoded;
uint8_t actual = region_width(test);
assert(expected == actual);
}
}
Changes in v2:
* New patch for non-aligned lock addresses
* Commit message improvements.
* Add Steven's tags.
Alyssa Rosenzweig (4):
drm/panfrost: Simplify lock_region calculation
drm/panfrost: Use u64 for size in lock_region
drm/panfrost: Clamp lock region to Bifrost minimum
drm/panfrost: Handle non-aligned lock addresses
drivers/gpu/drm/panfrost/panfrost_mmu.c | 32 ++++++++++--------------
drivers/gpu/drm/panfrost/panfrost_regs.h | 2 ++
2 files changed, 15 insertions(+), 19 deletions(-)
--
2.30.2
