Re: [LKP] [PATCH] drm/radeon: Try to init amdkfd only if 64 bit kernel

From: Oded Gabbay
Date: Sun Dec 28 2014 - 04:05:58 EST



On 12/25/2014 02:31 PM, Christian KÃnig wrote:
Am 22.12.2014 um 20:18 schrieb Oded Gabbay:

On 12/22/2014 09:00 PM, Andi Kleen wrote:
On Mon, Dec 22, 2014 at 10:49:40AM -0800, Andi Kleen wrote:
On Mon, Dec 22, 2014 at 11:58:43AM -0500, Alex Deucher wrote:
On Mon, Dec 22, 2014 at 6:11 AM, Oded Gabbay <oded.gabbay@xxxxxxx> wrote:
amdkfd driver can be compiled only in 64-bit kernel. Therefore, there is no
point in trying to initialize amdkfd in 32-bit kernel.

In addition, in case of specific configuration of 32-bit kernel, no
modules and
random kernel base, the symbol_request function doesn't work as expected - It
doesn't return NULL if the symbol doesn't exists. That makes the kernel
panic.
Therefore, the as amdkfd doesn't compile in 32-bit kernel, the best way is
just
to return false immediately.

Signed-off-by: Oded Gabbay <oded.gabbay@xxxxxxx>
Reviewed-by: Alex Deucher <alexander.deucher@xxxxxxx>
Sorry but the patch is just bogus. X-bit only code is usually
a very bad sign for the code. This is not windows programing after all.
Hi Andi,

Strange, I have never programmed for Windows in my life (except maybe in a
few courses during my degree) :)
Even if you wanted to do a 64bit only driver -- which
you probably don't -- the standard way would be to exclude
it in Kconfig.
So amdkfd actually *only* supports 64bit user processes, because AMD's HSA
stack on Linux supports *only* 64bit user processes. So, yes, I definitely
want to do a 64bit only driver.
If you look at kfd_open(), it fails the open of /dev/kfd if the process is
32bit.
In addition, in Kconfig of amdkfd, it is written:
"depends on DRM_RADEON && AMD_IOMMU_V2 && X86_64"

The problem here is that there is code in radeon, which is a driver that can
compile in 32bit, which tries to load amdkfd. I didn't see a point in trying
to load a driver which can't be compiled in 32bit.

Well in this case couldn't we make the code in radeon depend on whether or not
the KFD driver is compiled in or not instead of checking the system architecture?

Regards,
Christian.

If we are going down that path, we need something like:

bool radeon_kfd_init(void)
{
#if defined(CONFIG_HSA_AMD_MODULE)
current code (symbol request and call to symbol)
#elif defined(CONFIG_HSA_AMD)
direct call to kgd2kfd_init
#else
return false;
#endif
}

Now, the original concept of the symbol_request call was to prevent writing something like the above pseudo-code, but because symbol_request is not currently working in all cases, I think that this is a good band-aid as any.

Oded




Please root-cause why symbol_request doesn't work on 32bit
and fix it properly.
I didn't say it doesn't always work.
The actual thing that doesn't work is the define symbol_get and only in a
specific case of 32bit kernel AND CONFIG_MODULES is unset AND
CONFIG_RANDOMIZE_BASE is set.
The define in that case is:
#define symbol_get(x) ({ extern typeof(x) x __attribute__((weak)); &(x); })

Why it doesn't work (doesn't return NULL when symbol doesn't exists) ?
I don't know, probably because of some elf/makefile/c language magic. I'm
not that big of an expert on those issues, and I wanted to provide a fix for
this problem during the -rc stages. If someone can help me solving the root
cause, I would be more than happy.

Oded
+rusty.
And also with correct email.

-Andi

_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/