<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
<meta name="Generator" content="Microsoft Word 15 (filtered medium)">
<style><!--
/* Font Definitions */
@font-face
{font-family:"Cambria Math";
panose-1:2 4 5 3 5 4 6 3 2 4;}
@font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0cm;
font-size:11.0pt;
font-family:"Calibri",sans-serif;}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:#0563C1;
text-decoration:underline;}
span.EmailStyle20
{mso-style-type:personal-reply;
font-family:"Calibri",sans-serif;
color:windowtext;}
.MsoChpDefault
{mso-style-type:export-only;
font-size:10.0pt;
mso-ligatures:none;}
@page WordSection1
{size:612.0pt 792.0pt;
margin:72.0pt 72.0pt 72.0pt 72.0pt;}
div.WordSection1
{page:WordSection1;}
--></style><!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]-->
</head>
<body lang="en-IL" link="#0563C1" vlink="#954F72" style="word-wrap:break-word">
<div class="WordSection1">
<p class="MsoNormal"><span lang="EN-US" style="mso-fareast-language:EN-US">Hi, sending to the mailing list. I will review the patchset that refactor the VM bind<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US" style="mso-fareast-language:EN-US"><o:p> </o:p></span></p>
<p class="MsoNormal"><span lang="EN-US" style="mso-fareast-language:EN-US">Thanks,<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US" style="mso-fareast-language:EN-US">Dafna<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="en-IL" style="mso-fareast-language:EN-US"><o:p> </o:p></span></p>
<div>
<div style="border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0cm 0cm 0cm">
<p class="MsoNormal"><b><span lang="EN-US">From:</span></b><span lang="EN-US"> Brost, Matthew <matthew.brost@intel.com>
<br>
<b>Sent:</b> Saturday, 4 November 2023 0:51<br>
<b>To:</b> Vivi, Rodrigo <rodrigo.vivi@intel.com>; Hellstrom, Thomas <thomas.hellstrom@intel.com>; Dafna Hirschfeld <dhirschfeld@habana.ai>; Strano, Luis <luis.strano@intel.com>; Dugast, Francois <francois.dugast@intel.com><br>
<b>Cc:</b> Oded Gabbay <ogabbay@habana.ai>; daniel.vetter@ffwll.ch; Dani Liberman <dliberman@habana.ai><br>
<b>Subject:</b> RE: Review of GPUVM API usage in xe driver<o:p></o:p></span></p>
</div>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal"><span lang="EN-US">Just a heads up, a lot of this code is being rewritten…<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US"><a href="https://patchwork.freedesktop.org/series/125608/">https://patchwork.freedesktop.org/series/125608/</a><o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US"><o:p> </o:p></span></p>
<p class="MsoNormal"><span lang="EN-US">Our time might be better spent trying to get this series pushed through.<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US"><o:p> </o:p></span></p>
<p class="MsoNormal"><span lang="EN-US">Matt<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US"><o:p> </o:p></span></p>
<div>
<div style="border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0cm 0cm 0cm">
<p class="MsoNormal"><b><span lang="EN-US">From:</span></b><span lang="EN-US"> Vivi, Rodrigo <<a href="mailto:rodrigo.vivi@intel.com">rodrigo.vivi@intel.com</a>>
<br>
<b>Sent:</b> Friday, November 3, 2023 2:54 PM<br>
<b>To:</b> Brost, Matthew <<a href="mailto:matthew.brost@intel.com">matthew.brost@intel.com</a>>; Hellstrom, Thomas <<a href="mailto:thomas.hellstrom@intel.com">thomas.hellstrom@intel.com</a>>; Hirschfeld, Dafna (Habana) <<a href="mailto:dhirschfeld@habana.ai">dhirschfeld@habana.ai</a>>;
Strano, Luis <<a href="mailto:luis.strano@intel.com">luis.strano@intel.com</a>>; Dugast, Francois <<a href="mailto:francois.dugast@intel.com">francois.dugast@intel.com</a>><br>
<b>Cc:</b> Gabbay, Oded (Habana) <<a href="mailto:ogabbay@habana.ai">ogabbay@habana.ai</a>>;
<a href="mailto:daniel.vetter@ffwll.ch">daniel.vetter@ffwll.ch</a>; Liberman, Dani (Habana) <<a href="mailto:dliberman@habana.ai">dliberman@habana.ai</a>><br>
<b>Subject:</b> Re: Review of GPUVM API usage in xe driver<o:p></o:p></span></p>
</div>
</div>
<p class="MsoNormal"><span lang="EN-US"><o:p> </o:p></span></p>
<div>
<p class="MsoNormal"><span lang="EN-US">Cc: Strano<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US"><o:p> </o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US">Dafna, thank you so much for the review.<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US">Could you please sent that out to <a href="mailto:intel-xe@lists.freedesktop.org">
intel-xe@lists.freedesktop.org</a> as well?<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US"><o:p> </o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US">Thanks,<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US">Rodrigo.<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US"><o:p> </o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US">On Wed, 2023-11-01 at 08:37 +0000, Dafna Hirschfeld wrote:<o:p></o:p></span></p>
</div>
<blockquote style="border:none;border-left:solid #729FCF 1.5pt;padding:0cm 0cm 0cm 6.0pt;margin-left:4.8pt;margin-top:5.0pt;margin-right:0cm;margin-bottom:5.0pt">
<p class="MsoNormal"><span lang="EN-US">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<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US"> <o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US">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.<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US"> <o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US">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_* <o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US"> <o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US">3. In function 'vm_bind_ioctl_ops_unwind' the operation 'i++' should be replaced by 'i--' <o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US"> <o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US">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.<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US"> <o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US">Regards,<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US">Dafna<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US"> <o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US"> <o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US"> <o:p></o:p></span></p>
</blockquote>
<div>
<p class="MsoNormal"><span lang="EN-US"><o:p> </o:p></span></p>
</div>
</div>
</body>
</html>