[PATCH] [RFC] drm/drm_buddy fails to initialize on 32-bit architectures

From: Luís Mendes
Date: Sun Dec 25 2022 - 14:40:42 EST


Re-sending with the correct linux-kernel mailing list email address.
Sorry for the inconvenience.

The proposed patch fixes the issue and allows amdgpu to work again on
armhf with a AMD RX 550 card, however it may not be the best solution
for the issue, as detailed below.

include/log2.h defined macros rounddown_pow_of_two(...) and
roundup_pow_of_two(...) do not handle 64-bit values on 32-bit
architectures (tested on armv9 armhf machine) causing
drm_buddy_init(...) to fail on BUG_ON with an underflow on the order
value, thus impeding amdgpu to load properly (no GUI).

One option is to modify rounddown_pow_of_two(...) to detect if the
variable takes 32 bits or less and call __rounddown_pow_of_two_u32(u32
n) or if the variable takes more space than 32 bits, then call
__rounddown_pow_of_two_u64(u64 n). This would imply renaming
__rounddown_pow_of_two(unsigne
d long n) to
__rounddown_pow_of_two_u32(u32 n) and add a new function
__rounddown_pow_of_two_u64(u64 n). This would be the most transparent
solution, however there a few complications, and they are:
- that the mm subsystem will fail to link on armhf with an undefined
reference on __aeabi_uldivmod
- there a few drivers that directly call __rounddown_pow_of_two(...)
- that other drivers and subsystems generate warnings

So this alternate solution was devised which avoids touching existing
code paths, and just updates drm_buddy which seems to be the only
driver that is failing, however I am not sure if this is the proper
way to go. So I would like to get a second opinion on this, by those
who know.

/include/linux/log2.h
/drivers/gpu/drm/drm_buddy.c

Signed-off-by: Luís Mendes <luis.p.mendes@xxxxxxxxx>
>8------------------------------------------------------8<
diff -uprN linux-next/drivers/gpu/drm/drm_buddy.c
linux-nextLM/drivers/gpu/drm/drm_buddy.c
--- linux-next/drivers/gpu/drm/drm_buddy.c 2022-12-25
16:29:26.000000000 +0000
+++ linux-nextLM/drivers/gpu/drm/drm_buddy.c 2022-12-25
17:04:32.136007116 +0000
@@ -128,7 +128,7 @@ int drm_buddy_init(struct drm_buddy *mm,
unsigned int order;
u64 root_size;

- root_size = rounddown_pow_of_two(size);
+ root_size = rounddown_pow_of_two_u64(size);
order = ilog2(root_size) - ilog2(chunk_size);

root = drm_block_alloc(mm, NULL, order, offset);
diff -uprN linux-next/include/linux/log2.h linux-nextLM/include/linux/log2.h
--- linux-next/include/linux/log2.h 2022-12-25 16:29:29.000000000 +0000
+++ linux-nextLM/include/linux/log2.h 2022-12-25 17:00:34.319901492 +0000
@@ -58,6 +58,18 @@ unsigned long __roundup_pow_of_two(unsig
}

/**
+ * __roundup_pow_of_two_u64() - round up to nearest power of two
+ * (unsgined 64-bits precision version)
+ * @n: value to round up
+ */
+static inline __attribute__((const))
+u64 __roundup_pow_of_two_u64(u64 n)
+{
+ return 1ULL << fls64(n - 1);
+}
+
+
+/**
* __rounddown_pow_of_two() - round down to nearest power of two
* @n: value to round down
*/
@@ -68,6 +80,17 @@ unsigned long __rounddown_pow_of_two(uns
}

/**
+ * __rounddown_pow_of_two_u64() - round down to nearest power of two
+ * (unsgined 64-bits precision version)
+ * @n: value to round down
+ */
+static inline __attribute__((const))
+u64 __rounddown_pow_of_two_u64(u64 n)
+{
+ return 1ULL << (fls64(n) - 1);
+}
+
+/**
* const_ilog2 - log base 2 of 32-bit or a 64-bit constant unsigned value
* @n: parameter
*
@@ -163,6 +186,7 @@ unsigned long __rounddown_pow_of_two(uns
__ilog2_u64(n) \
)

+
/**
* roundup_pow_of_two - round the given value up to nearest power of two
* @n: parameter
@@ -181,6 +205,25 @@ unsigned long __rounddown_pow_of_two(uns
)

/**
+ * roundup_pow_of_two_u64 - round the given value up to nearest power of two
+ * (unsgined 64-bits precision version)
+ * @n: parameter
+ *
+ * round the given value up to the nearest power of two
+ * - the result is undefined when n == 0
+ * - this can be used to initialise global variables from constant data
+ */
+#define roundup_pow_of_two_u64(n) \
+( \
+ __builtin_constant_p(n) ? ( \
+ ((n) == 1) ? 1 : \
+ (1ULL << (ilog2((n) - 1) + 1)) \
+ ) : \
+ __roundup_pow_of_two_u64(n) \
+ )
+
+
+/**
* rounddown_pow_of_two - round the given value down to nearest power of two
* @n: parameter
*
@@ -195,6 +238,22 @@ unsigned long __rounddown_pow_of_two(uns
__rounddown_pow_of_two(n) \
)

+/**
+ * rounddown_pow_of_two_u64 - round the given value down to nearest
power of two
+ * (unsgined 64-bits precision version)
+ * @n: parameter
+ *
+ * round the given value down to the nearest power of two
+ * - the result is undefined when n == 0
+ * - this can be used to initialise global variables from constant data
+ */
+#define rounddown_pow_of_two_u64(n) \
+( \
+ __builtin_constant_p(n) ? ( \
+ (1ULL << ilog2(n))) : \
+ __rounddown_pow_of_two_u64(n) \
+ )
+
static inline __attribute_const__
int __order_base_2(unsigned long n)
{