TTM Usage review needed fixes

Ohad has made a review of the TTM- and memory management usage: Review comments are:

Below I’ll summarize comments/questions I had, please note that some comments are on code not directly related but within the same flow of TTM:

1. In try_add_system (or the other variants), I know that for now we’re good regarding space allocated in places, but isn’t it worth to having some assertion on c (in index) to avoid out-of-bound access?

2. Maybe rename xe_bo_get_sg -> xe_bo_sg as get implied refcount?

3. In xe_bo_move we have trace_printk, shouldn’t we avoid such prints in production?

4. In xe_bo_restore_kernel, if the restore fails, is it OK to leave BO in the present list? If so, is it because the resume failure will handle it?

5. In struct xe_bo, I don’t think preferred_mem_type is being used.

6. When initializing XA flags to xe->usm.asid_to_vm in the function xe_device_create you are using the flag XA_FLAGS_ALLOC1. Later this XA is used for cyclic allocation. But the Doc says that for Cyc alloc one shouldn’t use ALLOC1. Am I missing something? Is it still OK in this case?

7. Also, for the same XA, in the function xe_vm_create_ioctl, note that, according to the documentation return value of 1 isn’t a failure but merely an indication of a wrap. Are you prohibiting wrap on purpose?

8. Minor one: in xe_exec_ioctl can’t we use num_syncs as the iterator instead of i?

9. Looking at xe_ttm_vram_mgr_free_sgt it seems this function has no relation with TTM/manager. Maybe it belongs in another place?

10. In xe_vm_create, when shifting xe->info.va_bits, wouldn’t it be better to use BIT_ULL?

11. In xe_vma_op_execute, maybe get the VMA in each case and call once to __xe_vma_op_execute?

12. In struct xe_vm, maybe add doc to the flags (XE_VM_FLAG_64K et. al)? 13. Side note: I know that it may not be the scope of the review, but I couldn’t ignore the fact that the XE code is using a lot of inverted logic (e.g., if (!cond)… else).

Regards, Ohad