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 BD744CD8CB9 for ; Thu, 11 Jun 2026 07:42:07 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 6CCFE10ED38; Thu, 11 Jun 2026 07:42:07 +0000 (UTC) Authentication-Results: gabe.freedesktop.org; dkim=pass (2048-bit key; unprotected) header.d=intel.com header.i=@intel.com header.b="Lu395VLF"; dkim-atps=neutral Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.10]) by gabe.freedesktop.org (Postfix) with ESMTPS id E6B2F10ED38 for ; Thu, 11 Jun 2026 07:42:05 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1781163726; x=1812699726; h=message-id:subject:from:to:cc:date:in-reply-to: references:content-transfer-encoding:mime-version; bh=dC7XbPRHdtYyZWD8pglFV4x2zPqSP+KIr1sSOce949M=; b=Lu395VLFbWklHT+dq4031JKvOiX5ufe8UPo77ft6cPkCS8p9dJDLoYjg cq1iBgBZNj9qjiQF47u4degbUammXwZ5SjrHZ1dNo7rYzymbVF2SdCWWa 8ATdeIv3NwrM3HLcpISHmbBaIuoJGqs4td2G+aBSHZnVjzbE4UJCQaF8i tkC1r85tyRCPqoC3gpp0wMpVbz6s5LGbzE5VBhCHBpadMq9oL3beLYZx9 X89zopYFB33wixMcdkHsnX0Tukb8EFF5itXNGWD3aNOY8JkDASXKlS8+a QQqVxtwGamkREk0t9+vXOIaLx3sMecyBJm35yU5ynvzSvjeYeh13WSWnj A==; X-CSE-ConnectionGUID: lIgWams5QgSFWIM5Adn+Xw== X-CSE-MsgGUID: Yg1NT5gpSSqzVzNhMJt8rw== X-IronPort-AV: E=McAfee;i="6800,10657,11813"; a="93359518" X-IronPort-AV: E=Sophos;i="6.24,198,1774335600"; d="scan'208";a="93359518" Received: from orviesa010.jf.intel.com ([10.64.159.150]) by fmvoesa104.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 11 Jun 2026 00:42:06 -0700 X-CSE-ConnectionGUID: wSMwlVmUTr2Npf7z1M+fBw== X-CSE-MsgGUID: kX52surZSSeZv+lRXlriGQ== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.24,198,1774335600"; d="scan'208";a="245525730" Received: from amilburn-desk.amilburn-desk (HELO [10.245.244.169]) ([10.245.244.169]) by orviesa010-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 11 Jun 2026 00:42:04 -0700 Message-ID: Subject: Re: [RFC PATCH 1/2] drm/gpusvm: Add a DMA-mapping accounting callback From: Thomas =?ISO-8859-1?Q?Hellstr=F6m?= To: Matthew Brost Cc: intel-xe@lists.freedesktop.org Date: Thu, 11 Jun 2026 09:42:01 +0200 In-Reply-To: References: <20260610133451.8930-1-thomas.hellstrom@linux.intel.com> Organization: Intel Sweden AB, Registration Number: 556189-6027 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable User-Agent: Evolution 3.58.3 (3.58.3-1.fc43) MIME-Version: 1.0 X-BeenThere: intel-xe@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel Xe graphics driver List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-xe-bounces@lists.freedesktop.org Sender: "Intel-xe" On Wed, 2026-06-10 at 08:37 -0700, Matthew Brost wrote: > On Wed, Jun 10, 2026 at 03:34:50PM +0200, Thomas Hellstr=C3=B6m wrote: > > Drivers that use the GPU SVM core to DMA-map system memory for GPU > > access may want to keep track of how many pages they have mapped, > > for > > example to expose statistics or to detect leaks. Doing this > > accounting > > in the driver around drm_gpusvm_get_pages() / > > drm_gpusvm_unmap_pages() > > is error prone: the number, order and kind of the dma_addr[] > > entries can > > change between map and unmap (migration, partial unmaps, the iova > > vs > > non-iova paths), which makes symmetric accounting hard to get > > right. > >=20 > > Add an optional @dma_map_account callback to struct drm_gpusvm_ops > > and > > invoke it once per &drm_pagemap_addr entry at the exact points > > where the > > entry is DMA-mapped (sign =3D=3D +1) in drm_gpusvm_get_pages() and > > unmapped > > (sign =3D=3D -1) in __drm_gpusvm_unmap_pages(). Since the map error > > path and > > every unmap/free path funnel through __drm_gpusvm_unmap_pages() for > > the > > entries that were actually mapped, the accounting is symmetric by > > construction. The callback receives the full &drm_pagemap_addr so > > the > > driver can use the order and the proto field to distinguish system > > memory mappings from device interconnect mappings. > >=20 > > The callback must also be usable by the core drm_gpusvm_pages-only > > API > > (mm =3D=3D NULL), as used e.g. for userptr-style mappings. Relax > > drm_gpusvm_init() to allow such users to pass a restricted @ops > > that > > only implements the page-level hooks; the full-SVM hooks > > (@invalidate) > > and the chunk configuration must still be unset in that mode. > >=20 > > Assisted-by: GitHub_Copilot:claude-opus-4.8 > > Signed-off-by: Thomas Hellstr=C3=B6m > > --- > > =C2=A0drivers/gpu/drm/drm_gpusvm.c | 17 +++++++++++++++-- >=20 > I believe this makes sense and is useful for debugging/profiling =E2=80= =94 > the > same applies to the next patch. >=20 > I do have a question, though: drm_pagemap also performs DMA mapping > for > migrations. Should we include those in the accounting as well, or was > there a reason this was intentionally omitted? SVM migration was initially intentionally omitted, because the primary intention here is to debug UMD size-aligning's influence on the number of 2M pages we get. But for completeness before we merge this we might want to add the transient migration entries as well. We might also want to add similar stats for the PPGTT PTE sizes. Thanks, Thomas >=20 > Matt >=20 > > =C2=A0include/drm/drm_gpusvm.h=C2=A0=C2=A0=C2=A0=C2=A0 | 19 +++++++++++= ++++++++ > > =C2=A02 files changed, 34 insertions(+), 2 deletions(-) > >=20 > > diff --git a/drivers/gpu/drm/drm_gpusvm.c > > b/drivers/gpu/drm/drm_gpusvm.c > > index 958cb605aedd..c18f15c77e94 100644 > > --- a/drivers/gpu/drm/drm_gpusvm.c > > +++ b/drivers/gpu/drm/drm_gpusvm.c > > @@ -393,8 +393,15 @@ int drm_gpusvm_init(struct drm_gpusvm *gpusvm, > > =C2=A0 return -EINVAL; > > =C2=A0 mmgrab(mm); > > =C2=A0 } else { > > - /* No full SVM mode, only core drm_gpusvm_pages > > API. */ > > - if (ops || num_chunks || mm_range || > > notifier_size) > > + /* > > + * No full SVM mode, only core drm_gpusvm_pages > > API. A > > + * restricted @ops that only implements the page- > > level hooks > > + * (e.g. @dma_map_account) is allowed; the full- > > SVM hooks must > > + * not be set. > > + */ > > + if (num_chunks || mm_range || notifier_size) > > + return -EINVAL; > > + if (ops && ops->invalidate) > > =C2=A0 return -EINVAL; > > =C2=A0 } > > =C2=A0 > > @@ -1162,6 +1169,8 @@ static void __drm_gpusvm_unmap_pages(struct > > drm_gpusvm *gpusvm, > > =C2=A0 else if (dpagemap && dpagemap->ops- > > >device_unmap) > > =C2=A0 dpagemap->ops- > > >device_unmap(dpagemap, > > =C2=A0 =C2=A0=C2=A0=C2=A0 dev, > > addr); > > + if (gpusvm->ops && gpusvm->ops- > > >dma_map_account) > > + gpusvm->ops- > > >dma_map_account(gpusvm, addr, -1); > > =C2=A0 i +=3D 1 << addr->order; > > =C2=A0 } > > =C2=A0 > > @@ -1584,6 +1593,10 @@ int drm_gpusvm_get_pages(struct drm_gpusvm > > *gpusvm, > > =C2=A0 (addr, DRM_INTERCONNECT_SYSTEM, > > order, > > =C2=A0 dma_dir); > > =C2=A0 } > > + if (gpusvm->ops && gpusvm->ops->dma_map_account) > > + gpusvm->ops->dma_map_account(gpusvm, > > + =C2=A0=C2=A0=C2=A0=C2=A0 &svm_pages- > > >dma_addr[j], > > + =C2=A0=C2=A0=C2=A0=C2=A0 1); > > =C2=A0 i +=3D 1 << order; > > =C2=A0 num_dma_mapped =3D i; > > =C2=A0 flags.has_dma_mapping =3D true; > > diff --git a/include/drm/drm_gpusvm.h b/include/drm/drm_gpusvm.h > > index 8a4d7134a9a7..87ad2dcaa7c4 100644 > > --- a/include/drm/drm_gpusvm.h > > +++ b/include/drm/drm_gpusvm.h > > @@ -75,6 +75,25 @@ struct drm_gpusvm_ops { > > =C2=A0 void (*invalidate)(struct drm_gpusvm *gpusvm, > > =C2=A0 =C2=A0=C2=A0 struct drm_gpusvm_notifier *notifier, > > =C2=A0 =C2=A0=C2=A0 const struct mmu_notifier_range > > *mmu_range); > > + > > + /** > > + * @dma_map_account: Account a DMA-mapping change > > (optional) > > + * @gpusvm: Pointer to the GPU SVM > > + * @addr: The address descriptor of the chunk being > > (un)mapped > > + * @sign: +1 when @addr has just been DMA-mapped, -1 when > > it is being > > + *=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 unmapped > > + * > > + * Called once per &drm_pagemap_addr entry, when the entry > > is > > + * DMA-mapped by drm_gpusvm_get_pages() (@sign =3D=3D +1) and > > when it is > > + * unmapped (@sign =3D=3D -1), so the driver can keep > > symmetric accounting > > + * of the pages it has mapped for GPU access. The @addr- > > >proto field > > + * can be used to distinguish system-memory mappings from > > device > > + * interconnect mappings. May be provided in both full SVM > > mode and the > > + * core drm_gpusvm_pages-only mode. > > + */ > > + void (*dma_map_account)(struct drm_gpusvm *gpusvm, > > + const struct drm_pagemap_addr > > *addr, > > + int sign); > > =C2=A0}; > > =C2=A0 > > =C2=A0/** > > --=20 > > 2.54.0 > >=20