Re: [PATCH] drm/radeon: add si tile mode array query

On Mit, 2013-04-03 at 17:22 -0400, j.glisse@gmail.com wrote:

From: Jerome Glisse jglisse@redhat.com

Allow userspace to query for the tile mode array so userspace can properly compute surface pitch and alignment requirement depending on tiling.

Signed-off-by: Jerome Glisse jglisse@redhat.com

[...]

diff --git a/drivers/gpu/drm/radeon/radeon_kms.c b/drivers/gpu/drm/radeon/radeon_kms.c index c75cb2c..8076434 100644 --- a/drivers/gpu/drm/radeon/radeon_kms.c +++ b/drivers/gpu/drm/radeon/radeon_kms.c @@ -176,80 +176,65 @@ int radeon_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp) struct radeon_device *rdev = dev->dev_private; struct drm_radeon_info *info = data; struct radeon_mode_info *minfo = &rdev->mode_info;

•   uint32_t value, *value_ptr;
  

•   uint64_t value64, *value_ptr64;
  

•   uint32_t *value, value_tmp, *value_ptr, value_size;
  

•   uint64_t value64;
    struct drm_crtc *crtc;
    int i, found;
  
  

•   /* TIMESTAMP is a 64-bit value, needs special handling. */
  

•   if (info->request == RADEON_INFO_TIMESTAMP) {
  

•           if (rdev->family >= CHIP_R600) {
  

•                   value_ptr64 = (uint64_t*)((unsigned long)info->value);
  

•                   value64 = radeon_get_gpu_clock_counter(rdev);
  

•                   if (DRM_COPY_TO_USER(value_ptr64, &value64, sizeof(value64))) {
  

•                           DRM_ERROR("copy_to_user %s:%u\n", __func__, __LINE__);
  

•                           return -EFAULT;
  

•                   }
  

•                   return 0;
  

•           } else {
  

•                   DRM_DEBUG_KMS("timestamp is r6xx+ only!\n");
  

•                   return -EINVAL;
  

•           }
  

•   }
  

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

•   if (DRM_COPY_FROM_USER(&value, value_ptr, sizeof(value))) {
  

•           DRM_ERROR("copy_from_user %s:%u\n", __func__, __LINE__);
  

•           return -EFAULT;
  

•   }
  

•   value = &value_tmp;
  

•   value_size = sizeof(uint32_t);
  

[...]

•   case RADEON_INFO_TIMESTAMP:
  

•           if (rdev->family < CHIP_R600) {
  

•                   DRM_DEBUG_KMS("timestamp is r6xx+ only!\n");
  

•                   return -EINVAL;
  

•           }
  

•           value = (uint32_t*)&value64;
  

•           value_size = sizeof(uint64_t);
  

•           value64 = radeon_get_gpu_clock_counter(rdev);
  

•           break;
  

[...]

•   if (DRM_COPY_TO_USER(value_ptr, &value, sizeof(uint32_t))) {
  

•   if (DRM_COPY_TO_USER(value_ptr, value, value_size)) {
            DRM_ERROR("copy_to_user %s:%u\n", __func__, __LINE__);
            return -EFAULT;
    }
  

Are these changes safe wrt strict aliasing? Might be safer to use char* for the second argument passed to DRM_COPY_TO_USER.