[PATCH 1/3] drm/xe/migrate: prevent infinite recursion

[Matthew Auld]

On 30/07/2025 19:11, Summers, Stuart wrote:
> On Wed, 2025-07-30 at 15:33 +0100, Matthew Auld wrote:
>> If the buf + offset is not aligned to XE_CAHELINE_BYTES we fallback
>> to
>> using a bounce buffer. However the bounce buffer here is allocated on
>> the stack, and the only alignment requirement here is that it's
>> naturally aligned to u8, and not XE_CACHELINE_BYTES. If the bounce
>> buffer is also misaligned we then recurse back into the function
>> again,
>> however the new bounce buffer might also not be aligned, and might
>> never
>> be until we eventually blow through the stack, as we keep recursing.
>>
>> Instead of using the stack use kmalloc, which should respect the
>> power-of-two alignment request here. Fixes a kernel panic when
> 
> Should we add a build bug on (or similar) to guarantee that the power-
> of-2 stays here? My understanding is this alignment isn't guaranteed
> otherwise. Of course the chance of the cacheline being a non-power-of-2
> seems low...

Agreed, will add.

> 
>> triggering this path through eudebug.
>>
>> Fixes: 270172f64b11 ("drm/xe: Update xe_ttm_access_memory to use GPU
>> for non-visible access")
>> Signed-off-by: Matthew Auld <matthew.auld at intel.com>
>> Cc: Maciej Patelczyk <maciej.patelczyk at intel.com>
>> Cc: Matthew Brost <matthew.brost at intel.com>
>> ---
>>   drivers/gpu/drm/xe/xe_migrate.c | 31 +++++++++++++++++++------------
>>   1 file changed, 19 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_migrate.c
>> b/drivers/gpu/drm/xe/xe_migrate.c
>> index 3a276e2348a2..b6bdc9bd32b0 100644
>> --- a/drivers/gpu/drm/xe/xe_migrate.c
>> +++ b/drivers/gpu/drm/xe/xe_migrate.c
>> @@ -1987,15 +1987,21 @@ int xe_migrate_access_memory(struct
>> xe_migrate *m, struct xe_bo *bo,
>>          if (!IS_ALIGNED(len, XE_CACHELINE_BYTES) ||
>>              !IS_ALIGNED((unsigned long)buf + offset,
>> XE_CACHELINE_BYTES)) {
>>                  int buf_offset = 0;
>> +               void *bounce;
>> +               int err;
>> +
>> +               bounce = kmalloc(XE_CACHELINE_BYTES, GFP_KERNEL);
>> +               if (!bounce)
>> +                       return -EINVAL;
> 
> ENOMEM?

Oops. Thanks.

> 
> Thanks,
> Stuart
> 
>> +
>> +               xe_assert(xe, IS_ALIGNED((unsigned long)bounce,
>> +                                        XE_CACHELINE_BYTES));
>>   
>>                  /*
>>                   * Less than ideal for large unaligned access but
>> this should be
>>                   * fairly rare, can fixup if this becomes common.
>>                   */
>>                  do {
>> -                       u8 bounce[XE_CACHELINE_BYTES];
>> -                       void *ptr = (void *)bounce;
>> -                       int err;
>>                          int copy_bytes = min_t(int, bytes_left,
>>                                                 XE_CACHELINE_BYTES -
>>                                                 (offset &
>> XE_CACHELINE_MASK));
>> @@ -2004,22 +2010,22 @@ int xe_migrate_access_memory(struct
>> xe_migrate *m, struct xe_bo *bo,
>>                          err = xe_migrate_access_memory(m, bo,
>>                                                         offset &
>>                                                        
>> ~XE_CACHELINE_MASK,
>> -                                                      (void *)ptr,
>> -
>> sizeof(bounce), 0);
>> +                                                      bounce,
>> +
>> XE_CACHELINE_BYTES, 0);
>>                          if (err)
>> -                               return err;
>> +                               break;
>>   
>>                          if (write) {
>> -                               memcpy(ptr + ptr_offset, buf +
>> buf_offset, copy_bytes);
>> +                               memcpy(bounce + ptr_offset, buf +
>> buf_offset, copy_bytes);
>>   
>>                                  err = xe_migrate_access_memory(m, bo,
>>                                                                 offset
>> & ~XE_CACHELINE_MASK,
>> -                                                              (void
>> *)ptr,
>> -
>> sizeof(bounce), write);
>> +
>> bounce,
>> +
>> XE_CACHELINE_BYTES, write);
>>                                  if (err)
>> -                                       return err;
>> +                                       break;
>>                          } else {
>> -                               memcpy(buf + buf_offset, ptr +
>> ptr_offset,
>> +                               memcpy(buf + buf_offset, bounce +
>> ptr_offset,
>>                                         copy_bytes);
>>                          }
>>   
>> @@ -2028,7 +2034,8 @@ int xe_migrate_access_memory(struct xe_migrate
>> *m, struct xe_bo *bo,
>>                          offset += copy_bytes;
>>                  } while (bytes_left);
>>   
>> -               return 0;
>> +               kfree(bounce);
>> +               return err;
>>          }
>>   
>>          dma_addr = xe_migrate_dma_map(xe, buf, len + page_offset,
>> write);
>