PATCH: radeondrm x86_64 and android32

PATCH: radeondrm x86_64 and android32

[Sergey Korshunoff]

Android-x86 4.0-r1 (32 bit) have problems with x86_64 kernel when he
trying to use a radeon kms. The following change correct a problem:

drivers/gpu/drm/radeon_kms.c (function radeon_info_ioctl):

- value_ptr = (uint32_t *)((unsigned long)info->value);
+ value_ptr = (uint32_t *)((unsigned)info->value);

Looks like a userspace data in android running under x86_64 is located
above 4 Gb. I don't think so. But after this change android run fine.

Re: PATCH: radeondrm x86_64 and android32

[Christian König]

Am 15.09.2014 um 12:09 schrieb Sergey Korshunoff:
> Android-x86 4.0-r1 (32 bit) have problems with x86_64 kernel when he
> trying to use a radeon kms. The following change correct a problem:
>
> drivers/gpu/drm/radeon_kms.c (function radeon_info_ioctl):
>
> - value_ptr = (uint32_t *)((unsigned long)info->value);
> + value_ptr = (uint32_t *)((unsigned)info->value);
>
> Looks like a userspace data in android running under x86_64 is located
> above 4 Gb. I don't think so. But after this change android run fine.

That's most likely a bug on the userspace side, caused by the upper 
32bits of info->value not initialized properly.

The kernel patch you show above will most likely just break 64bit userspace.

Regards,
Christian.

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

Re: PATCH: radeondrm x86_64 and android32

[Sergey Korshunoff]

[-- Attachment #1: Type: text/plain, Size: 1319 bytes --]

Hi Christian!
Yes, it is. Android-x86 4.0-r1 dos'nt fill right a value fild of the
structture drm_radeon_info_t: high bits are a random value. But I
wrote a compat function for this ioctl which clears this bits only for
32-bit applications. This patch is against a 3.10 kernel.

2014-09-15 15:54 GMT+04:00, Christian König <deathsimple@vodafone.de>:
> Am 15.09.2014 um 12:09 schrieb Sergey Korshunoff:
>> Android-x86 4.0-r1 (32 bit) have problems with x86_64 kernel when he
>> trying to use a radeon kms. The following change correct a problem:
>>
>> drivers/gpu/drm/radeon_kms.c (function radeon_info_ioctl):
>>
>> - value_ptr = (uint32_t *)((unsigned long)info->value);
>> + value_ptr = (uint32_t *)((unsigned)info->value);
>>
>> Looks like a userspace data in android running under x86_64 is located
>> above 4 Gb. I don't think so. But after this change android run fine.
>
> That's most likely a bug on the userspace side, caused by the upper
> 32bits of info->value not initialized properly.
>
> The kernel patch you show above will most likely just break 64bit
> userspace.
>
> Regards,
> Christian.
>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
>

[-- Attachment #2: 12-radeon-info-compat.patch --]
[-- Type: application/octet-stream, Size: 2589 bytes --]

diff -urN linux-3.10.0-123.6.3.el7.i686.old/drivers/gpu/drm/radeon/radeon_drv.c linux-3.10.0-123.6.3.el7.i686/drivers/gpu/drm/radeon/radeon_drv.c
--- linux-3.10.0-123.6.3.el7.i686.old/drivers/gpu/drm/radeon/radeon_drv.c	2014-09-15 19:00:17.000000000 +0400
+++ linux-3.10.0-123.6.3.el7.i686/drivers/gpu/drm/radeon/radeon_drv.c	2014-09-15 19:01:11.000000000 +0400
@@ -125,7 +125,7 @@
 void radeon_gem_prime_unpin(struct drm_gem_object *obj);
 void *radeon_gem_prime_vmap(struct drm_gem_object *obj);
 void radeon_gem_prime_vunmap(struct drm_gem_object *obj, void *vaddr);
-extern long radeon_kms_compat_ioctl(struct file *filp, unsigned int cmd,
+extern long radeon_compat_ioctl(struct file *filp, unsigned int cmd,
 				    unsigned long arg);
 
 #if defined(CONFIG_DEBUG_FS)
@@ -380,7 +380,7 @@
 	.poll = drm_poll,
 	.read = drm_read,
 #ifdef CONFIG_COMPAT
-	.compat_ioctl = radeon_kms_compat_ioctl,
+	.compat_ioctl = radeon_compat_ioctl,
 #endif
 };
 
diff -urN linux-3.10.0-123.6.3.el7.i686.old/drivers/gpu/drm/radeon/radeon_ioc32.c linux-3.10.0-123.6.3.el7.i686/drivers/gpu/drm/radeon/radeon_ioc32.c
--- linux-3.10.0-123.6.3.el7.i686.old/drivers/gpu/drm/radeon/radeon_ioc32.c	2014-09-15 19:00:17.000000000 +0400
+++ linux-3.10.0-123.6.3.el7.i686/drivers/gpu/drm/radeon/radeon_ioc32.c	2014-09-15 18:53:59.000000000 +0400
@@ -368,6 +368,14 @@
 #define compat_radeon_cp_setparam NULL
 #endif /* X86_64 || IA64 */
 
+static int compat_radeon_info(struct file *file, unsigned int cmd,
+				     unsigned long arg)
+{
+	struct drm_radeon_info __user *info = (void __user *)arg;
+	info->value &= 0xFFFFFFFF;
+	return drm_ioctl(file, DRM_IOCTL_RADEON_INFO, arg);
+}
+
 static drm_ioctl_compat_t *radeon_compat_ioctls[] = {
 	[DRM_RADEON_CP_INIT] = compat_radeon_cp_init,
 	[DRM_RADEON_CLEAR] = compat_radeon_cp_clear,
@@ -379,6 +387,7 @@
 	[DRM_RADEON_SETPARAM] = compat_radeon_cp_setparam,
 	[DRM_RADEON_ALLOC] = compat_radeon_mem_alloc,
 	[DRM_RADEON_IRQ_EMIT] = compat_radeon_irq_emit,
+	[DRM_RADEON_INFO] = compat_radeon_info,
 };
 
 /**
@@ -390,6 +399,7 @@
  * \param arg user argument.
  * \return zero on success or negative number on failure.
  */
+
 long radeon_compat_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 {
 	unsigned int nr = DRM_IOCTL_NR(cmd);
@@ -409,16 +419,3 @@
 
 	return ret;
 }
-
-long radeon_kms_compat_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
-{
-	unsigned int nr = DRM_IOCTL_NR(cmd);
-	int ret;
-
-	if (nr < DRM_COMMAND_BASE)
-		return drm_compat_ioctl(filp, cmd, arg);
-
-	ret = drm_ioctl(filp, cmd, arg);
-
-	return ret;
-}

[-- Attachment #3: Type: text/plain, Size: 159 bytes --]

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

Re: PATCH: radeondrm x86_64 and android32

[Christian König]

Hi Sergey,

that probably works, but a compat function is the wrong approach here.

The definition of the field was always 64bit and when userspace fails to 
properly set the upper 32bits than userspace needs to get fixed, not the 
kernel.

Can you try to figure out where the random bits in the upper 32bits come 
from?

Regards,
Christian.

Am 15.09.2014 um 17:43 schrieb Sergey Korshunoff:
> Hi Christian!
> Yes, it is. Android-x86 4.0-r1 dos'nt fill right a value fild of the
> structture drm_radeon_info_t: high bits are a random value. But I
> wrote a compat function for this ioctl which clears this bits only for
> 32-bit applications. This patch is against a 3.10 kernel.
>
> 2014-09-15 15:54 GMT+04:00, Christian König <deathsimple@vodafone.de>:
>> Am 15.09.2014 um 12:09 schrieb Sergey Korshunoff:
>>> Android-x86 4.0-r1 (32 bit) have problems with x86_64 kernel when he
>>> trying to use a radeon kms. The following change correct a problem:
>>>
>>> drivers/gpu/drm/radeon_kms.c (function radeon_info_ioctl):
>>>
>>> - value_ptr = (uint32_t *)((unsigned long)info->value);
>>> + value_ptr = (uint32_t *)((unsigned)info->value);
>>>
>>> Looks like a userspace data in android running under x86_64 is located
>>> above 4 Gb. I don't think so. But after this change android run fine.
>> That's most likely a bug on the userspace side, caused by the upper
>> 32bits of info->value not initialized properly.
>>
>> The kernel patch you show above will most likely just break 64bit
>> userspace.
>>
>> Regards,
>> Christian.
>>
>>> _______________________________________________
>>> dri-devel mailing list
>>> dri-devel@lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>>

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

Re: PATCH: radeondrm x86_64 and android32

[Sergey Korshunoff]

> The definition of the field was always 64bit and when userspace fails to properly set the
> upper 32bits than userspace needs to get fixed, not the kernel.

Userspace fails to properly set the upper 32bits and a 32 bit kernel
don't check the upper 32 bits :-)

> Can you try to figure out where the random bits in the upper 32bits come from?

I don't think so. I use android-x86 4.0-r1 image taken from
http://www.android-x86.org
Thank's a lot.