From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 5FE01ECAAA1 for ; Fri, 28 Oct 2022 16:27:04 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 3247010E86E; Fri, 28 Oct 2022 16:27:03 +0000 (UTC) Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by gabe.freedesktop.org (Postfix) with ESMTPS id 9918310E86E for ; Fri, 28 Oct 2022 16:26:59 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1666974419; x=1698510419; h=message-id:date:mime-version:subject:to:cc:references: from:in-reply-to:content-transfer-encoding; bh=YHFWuoghLuJu9VRgIsP3jwvNl0xh+CXbEtMccja/akA=; b=VcEIAdSEmLCetedjDoIGM/25TXCP7ZspwCqtAVQsv3yR6ipLL/axThhK xI2k7tLdYhX2cx/L8m/staiIouu5iFCGnQFGffEL/9kX2kzvH3vULSogU 8vfwZMMw9FaVQKsD8yZL/frVQOpuskgHOgfIfj5mRP6WRxhTB9I+ZyEMM MktAJXOK46/6g1yP1WM1lnGGdQzAGggp41FRsbfmD8tlYPydnllzGqHsT O32ZeIRybIqibKVYp0vsTaFo33kSxUMbMz0KTQ9p9Ls8pwmqgJfqPxApb KUU0okH4EWlLCrju/+WsirG4tAGx0qdDQcVs3DGRO5iFsWT9AqaBWOWfv g==; X-IronPort-AV: E=McAfee;i="6500,9779,10514"; a="310228327" X-IronPort-AV: E=Sophos;i="5.95,221,1661842800"; d="scan'208";a="310228327" Received: from orsmga001.jf.intel.com ([10.7.209.18]) by orsmga103.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 28 Oct 2022 09:26:59 -0700 X-IronPort-AV: E=McAfee;i="6500,9779,10514"; a="666118056" X-IronPort-AV: E=Sophos;i="5.95,221,1661842800"; d="scan'208";a="666118056" Received: from ahamill-mobl2.ger.corp.intel.com (HELO [10.252.29.35]) ([10.252.29.35]) by orsmga001-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 28 Oct 2022 09:26:57 -0700 Message-ID: Date: Fri, 28 Oct 2022 17:26:55 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Firefox/102.0 Thunderbird/102.4.0 Content-Language: en-GB To: "Ruhl, Michael J" , "intel-gfx@lists.freedesktop.org" References: <20221028155029.494736-1-matthew.auld@intel.com> <20221028155029.494736-2-matthew.auld@intel.com> From: Matthew Auld In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Subject: Re: [Intel-gfx] [PATCH v2 2/4] drm/i915/selftests: exercise GPU access from the importer X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" On 28/10/2022 17:10, Ruhl, Michael J wrote: >> -----Original Message----- >> From: Auld, Matthew >> Sent: Friday, October 28, 2022 11:50 AM >> To: intel-gfx@lists.freedesktop.org >> Cc: Landwerlin, Lionel G ; Tvrtko Ursulin >> ; Ville Syrjälä ; >> Ruhl, Michael J >> Subject: [PATCH v2 2/4] drm/i915/selftests: exercise GPU access from the >> importer >> >> Using PAGE_SIZE here potentially hides issues so bump that to something >> larger. This should also make it possible for iommu to coalesce entries >> for us. With that in place verify we can write from the GPU using the >> importers sg_table, followed by checking that our writes match when read >>from the CPU side. >> >> v2: Switch over to igt_gpu_fill_dw(), which looks to be more widely >> supported than the migrate stuff (at least OOTB). >> >> References: https://gitlab.freedesktop.org/drm/intel/-/issues/7306 >> Signed-off-by: Matthew Auld >> Cc: Lionel Landwerlin >> Cc: Tvrtko Ursulin >> Cc: Ville Syrjälä >> Cc: Michael J. Ruhl >> --- >> .../drm/i915/gem/selftests/i915_gem_dmabuf.c | 79 >> ++++++++++++++++++- >> 1 file changed, 78 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c >> b/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c >> index f2f3cfad807b..e57f9390076c 100644 >> --- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c >> +++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c >> @@ -6,8 +6,12 @@ >> >> #include "i915_drv.h" >> #include "i915_selftest.h" >> +#include "gem/i915_gem_context.h" >> >> +#include "mock_context.h" >> #include "mock_dmabuf.h" >> +#include "igt_gem_utils.h" >> +#include "selftests/mock_drm.h" >> #include "selftests/mock_gem_device.h" >> >> static int igt_dmabuf_export(void *arg) >> @@ -140,6 +144,75 @@ static int >> igt_dmabuf_import_same_driver_lmem(void *arg) >> return err; >> } >> >> +static int verify_access(struct drm_i915_private *i915, >> + struct drm_i915_gem_object *native_obj, >> + struct drm_i915_gem_object *import_obj) >> +{ >> + struct i915_gem_engines_iter it; >> + struct i915_gem_context *ctx; >> + struct intel_context *ce; >> + struct i915_vma *vma; >> + struct file *file; >> + u32 *vaddr; >> + int err = 0, i; >> + >> + file = mock_file(i915); >> + if (IS_ERR(file)) >> + return PTR_ERR(file); >> + >> + ctx = live_context(i915, file); >> + if (IS_ERR(ctx)) { >> + err = PTR_ERR(ctx); >> + goto out_file; >> + } >> + >> + for_each_gem_engine(ce, i915_gem_context_lock_engines(ctx), it) { >> + if (intel_engine_can_store_dword(ce->engine)) >> + break; >> + } >> + i915_gem_context_unlock_engines(ctx); >> + if (!ce) >> + goto out_file; >> + >> + vma = i915_vma_instance(import_obj, ce->vm, NULL); >> + if (IS_ERR(vma)) { >> + err = PTR_ERR(vma); >> + goto out_file; >> + } >> + >> + err = i915_vma_pin(vma, 0, 0, PIN_USER); >> + if (err) >> + goto out_file; >> + >> + err = igt_gpu_fill_dw(ce, vma, 0, >> + vma->size >> PAGE_SHIFT, 0xdeadbeaf); >> + i915_vma_unpin(vma); >> + if (err) >> + goto out_file; >> + >> + err = i915_gem_object_wait(import_obj, 0, >> MAX_SCHEDULE_TIMEOUT); >> + if (err) >> + goto out_file; >> + >> + vaddr = i915_gem_object_pin_map_unlocked(native_obj, >> I915_MAP_WB); >> + if (IS_ERR(vaddr)) { >> + err = PTR_ERR(vaddr); >> + goto out_file; >> + } >> + >> + for (i = 0; i < native_obj->base.size / sizeof(u32); i += PAGE_SIZE / >> sizeof(u32)) { >> + if (vaddr[i] != 0xdeadbeaf) { >> + pr_err("Data mismatch [%d]=%u\n", i, vaddr[i]); >> + err = -EINVAL; >> + goto out_file; >> + } > > Not sure what timing issues are related to this test, but this loop could have > some impact (takes a long time, assuming the object is LMEM). > > Would checking the beginning, middle and end of each page be any less > beneficial than the current check? We are currently just sampling the first dword of each page (that could perhaps be random instead). We could go further and sample a selection of random pages instead, if speed is a concern. Although here the pages must be in system memory. dma-buf in upstream will currently force migrate to system memory. > > Not a suggested change, but just a thought to ponder if the timing becomes > an issue... > > This test looks reasonable to me. > > Reviewed-by: Michael J. Ruhl > > M > >> + } >> + >> +out_file: >> + fput(file); >> + return err; >> +} >> + >> static int igt_dmabuf_import_same_driver(struct drm_i915_private *i915, >> struct intel_memory_region >> **regions, >> unsigned int num_regions) >> @@ -154,7 +227,7 @@ static int igt_dmabuf_import_same_driver(struct >> drm_i915_private *i915, >> >> force_different_devices = true; >> >> - obj = __i915_gem_object_create_user(i915, PAGE_SIZE, >> + obj = __i915_gem_object_create_user(i915, SZ_8M, >> regions, num_regions); >> if (IS_ERR(obj)) { >> pr_err("__i915_gem_object_create_user failed with >> err=%ld\n", >> @@ -206,6 +279,10 @@ static int igt_dmabuf_import_same_driver(struct >> drm_i915_private *i915, >> >> i915_gem_object_unlock(import_obj); >> >> + err = verify_access(i915, obj, import_obj); >> + if (err) >> + goto out_import; >> + >> /* Now try a fake an importer */ >> import_attach = dma_buf_attach(dmabuf, obj->base.dev->dev); >> if (IS_ERR(import_attach)) { >> -- >> 2.37.3 >