<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:dt="uuid:C2F41010-65B3-11d1-A29F-00AA00C14882" 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=us-ascii"> <meta name="Generator" content="Microsoft Word 15 (filtered medium)"> <style></style> </head> <body lang="EN-US" link="blue" vlink="purple" style="word-wrap:break-word"> <div class="WordSection1"> <p class="msipheader23c54b6f" style="margin:0in"><span style="font-size:10.0pt;font-family:"Arial",sans-serif;color:green">[Public]</span><o:p></o:p></p> <p class="MsoNormal"><o:p> </o:p></p> <p class="MsoNormal">Hi Christian and Xinhui,<o:p></o:p></p> <p class="MsoNormal"><o:p> </o:p></p> <p class="MsoNormal">Thanks for your suggestion. The cause is I saw data corruption in several proprietary use cases. BUILD_BUG_ON will have build variation per gcc difference?<o:p></o:p></p> <p class="MsoNormal"><o:p> </o:p></p> <p class="MsoNormal">Anyway, WARN_ON is fine to me, and I will send a new patch set soon to address this.<o:p></o:p></p> <div> <p class="MsoNormal"><o:p> </o:p></p> <p class="MsoNormal">Regards,<o:p></o:p></p> <p class="MsoNormal">Guchun<o:p></o:p></p> </div> <p class="MsoNormal"><o:p> </o:p></p> <div> <div style="border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0in 0in 0in"> <p class="MsoNormal"><b>From:</b> Koenig, Christian <Christian.Koenig@amd.com> <br> <b>Sent:</b> Friday, September 10, 2021 2:37 PM<br> <b>To:</b> Pan, Xinhui <Xinhui.Pan@amd.com>; amd-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; Deucher, Alexander <Alexander.Deucher@amd.com>; Chen, Guchun <Guchun.Chen@amd.com><br> <b>Cc:</b> Shi, Leslie <Yuliang.Shi@amd.com><br> <b>Subject:</b> Re: [PATCH] drm/ttm: add a BUG_ON in ttm_set_driver_manager when array bounds<o:p></o:p></p> </div> </div> <p class="MsoNormal"><o:p> </o:p></p> <p class="MsoNormal" style="margin-bottom:12.0pt">Yeah, that's a good point.<br> <br> If build_bug_on() doesn't works for some reason then we at least need to lower this to a WARN_ON.<br> <br> A BUG_ON() is only justified if we prevent strong data corruption with it or note a NULL pointer earlier on or similar.<br> <br> Regards,<br> Christian.<o:p></o:p></p> <div> <p class="MsoNormal">Am 10.09.21 um 06:36 schrieb Pan, Xinhui:<o:p></o:p></p> </div> <blockquote style="margin-top:5.0pt;margin-bottom:5.0pt"> <p style="margin:5.0pt"><span style="font-size:10.0pt;font-family:"Arial",sans-serif;color:blue">[AMD Official Use Only]<o:p></o:p></span></p> <p class="MsoNormal"><o:p> </o:p></p> <div> <div> <p class="MsoNormal"><span style="font-family:"Arial",sans-serif;color:black">looks good to me.<o:p></o:p></span></p> </div> <p class="MsoNormal"><span style="font-family:"Arial",sans-serif;color:black">But maybe build_bug_on works too and more reasonable to detect such wrong usage.<o:p></o:p></span></p> <div class="MsoNormal" align="center" style="text-align:center"> <hr size="1" width="98%" align="center"> </div> <div id="divRplyFwdMsg"> <p class="MsoNormal"><b><span style="color:black">From:</span></b><span style="color:black"> Chen, Guchun <a href="mailto:Guchun.Chen@amd.com"><Guchun.Chen@amd.com></a><br> <b>Sent:</b> Friday, September 10, 2021 12:30:14 PM<br> <b>To:</b> <a href="mailto:amd-gfx@lists.freedesktop.org">amd-gfx@lists.freedesktop.org</a> <a href="mailto:amd-gfx@lists.freedesktop.org"><amd-gfx@lists.freedesktop.org></a>; <a href="mailto:dri-devel@lists.freedesktop.org">dri-devel@lists.freedesktop.org</a> <a href="mailto:dri-devel@lists.freedesktop.org"><dri-devel@lists.freedesktop.org></a>; Koenig, Christian <a href="mailto:Christian.Koenig@amd.com"><Christian.Koenig@amd.com></a>; Pan, Xinhui <a href="mailto:Xinhui.Pan@amd.com"><Xinhui.Pan@amd.com></a>; Deucher, Alexander <a href="mailto:Alexander.Deucher@amd.com"> <Alexander.Deucher@amd.com></a><br> <b>Cc:</b> Chen, Guchun <a href="mailto:Guchun.Chen@amd.com"><Guchun.Chen@amd.com></a>; Shi, Leslie <a href="mailto:Yuliang.Shi@amd.com"><Yuliang.Shi@amd.com></a><br> <b>Subject:</b> [PATCH] drm/ttm: add a BUG_ON in ttm_set_driver_manager when array bounds</span> <o:p></o:p></p> <div> <p class="MsoNormal"> <o:p></o:p></p> </div> </div> <div> <div> <p class="MsoNormal" style="margin-bottom:12.0pt">Vendor will define their own memory types on top of TTM_PL_PRIV,<br> but call ttm_set_driver_manager directly without checking mem_type<br> value when setting up memory manager. So add such check to aware<br> the case when array bounds.<br> <br> Signed-off-by: Leslie Shi <a href="mailto:Yuliang.Shi@amd.com"><Yuliang.Shi@amd.com></a><br> Signed-off-by: Guchun Chen <a href="mailto:guchun.chen@amd.com"><guchun.chen@amd.com></a><br> ---<br> include/drm/ttm/ttm_device.h | 1 +<br> 1 file changed, 1 insertion(+)<br> <br> diff --git a/include/drm/ttm/ttm_device.h b/include/drm/ttm/ttm_device.h<br> index 7a0f561c57ee..24ad76ca8022 100644<br> --- a/include/drm/ttm/ttm_device.h<br> +++ b/include/drm/ttm/ttm_device.h<br> @@ -308,6 +308,7 @@ ttm_manager_type(struct ttm_device *bdev, int mem_type)<br> static inline void ttm_set_driver_manager(struct ttm_device *bdev, int type,<br> struct ttm_resource_manager *manager)<br> {<br> + BUG_ON(type >= TTM_NUM_MEM_TYPES);<br> bdev->man_drv[type] = manager;<br> }<br> <br> -- <br> 2.17.1<o:p></o:p></p> </div> </div> </div> </blockquote> <p class="MsoNormal"><o:p> </o:p></p> </div> </body> </html>