[Intel-xe] Review of GPUVM API usage in xe driver

[Dafna Hirschfeld]

Hi, sending to the mailing list. I will review the patchset that refactor the VM bind

Thanks,
Dafna

From: Brost, Matthew <matthew.brost at intel.com>
Sent: Saturday, 4 November 2023 0:51
To: Vivi, Rodrigo <rodrigo.vivi at intel.com>; Hellstrom, Thomas <thomas.hellstrom at intel.com>; Dafna Hirschfeld <dhirschfeld at habana.ai>; Strano, Luis <luis.strano at intel.com>; Dugast, Francois <francois.dugast at intel.com>
Cc: Oded Gabbay <ogabbay at habana.ai>; daniel.vetter at ffwll.ch; Dani Liberman <dliberman at habana.ai>
Subject: RE: Review of GPUVM API usage in xe driver

Just a heads up, a lot of this code is being rewritten…
https://patchwork.freedesktop.org/series/125608/

Our time might be better spent trying to get this series pushed through.

Matt

From: Vivi, Rodrigo <rodrigo.vivi at intel.com<mailto:rodrigo.vivi at intel.com>>
Sent: Friday, November 3, 2023 2:54 PM
To: Brost, Matthew <matthew.brost at intel.com<mailto:matthew.brost at intel.com>>; Hellstrom, Thomas <thomas.hellstrom at intel.com<mailto:thomas.hellstrom at intel.com>>; Hirschfeld, Dafna (Habana) <dhirschfeld at habana.ai<mailto:dhirschfeld at habana.ai>>; Strano, Luis <luis.strano at intel.com<mailto:luis.strano at intel.com>>; Dugast, Francois <francois.dugast at intel.com<mailto:francois.dugast at intel.com>>
Cc: Gabbay, Oded (Habana) <ogabbay at habana.ai<mailto:ogabbay at habana.ai>>; daniel.vetter at ffwll.ch<mailto:daniel.vetter at ffwll.ch>; Liberman, Dani (Habana) <dliberman at habana.ai<mailto:dliberman at habana.ai>>
Subject: Re: Review of GPUVM API usage in xe driver

Cc: Strano

Dafna, thank you so much for the review.
Could you please sent that out to intel-xe at lists.freedesktop.org<mailto:intel-xe at lists.freedesktop.org> as well?

Thanks,
Rodrigo.

On Wed, 2023-11-01 at 08:37 +0000, Dafna Hirschfeld wrote:
As part of the upstream review of the Xe driver, I have reviewed the code related to the usage of GPUVM API. The following is a list of issues I have found

1. The driver uses the flag DRM_GPUVA_SPARSE to indicate null operation (what in habanalabs we call RAZWI) but the gpuvm doc says that this flag is used to support Vulkan 'Sparse Resources' which is not the same.

2. In the function 'vm_bind_ioctl_ops_create', there is access to fields inside a union in 'struct xe_vma_op'. The access is done according to the bind operation from userspace  "XE_VM_BIND_OP_*" which is wrong. It should be done according to the gpuvm op type:   DRM_GPUVA_OP_*

3. In function 'vm_bind_ioctl_ops_unwind' the operation 'i++' should be replaced by 'i--'

4. In the function xe_vma_op_execute, in the case of 'remap' operation, it passes to __xe_vma_op_execute, one of 'unmap'/'prev'/'next' vma, which then passes it to function 'op_execute'. But in 'remap' case in 'op_execute', it executes vm_bind/unbind operation on one or more of unmap/prev/next vmas. So if for example both prev and next exist, it will try to bind both but will call  "xe_vm_prepare_vma" only on one of the vma.

Regards,
Dafna




-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/intel-xe/attachments/20231106/6880efcb/attachment-0001.htm>