From: "Hellstrom, Thomas" <thomas.hellstrom@intel.com>
To: "intel-xe@lists.freedesktop.org" <intel-xe@lists.freedesktop.org>,
"Auld, Matthew" <matthew.auld@intel.com>
Cc: "Brost, Matthew" <matthew.brost@intel.com>,
"stable@vger.kernel.org" <stable@vger.kernel.org>
Subject: Re: [PATCH] drm/xe/userptr: properly setup pfn_flags_mask
Date: Wed, 26 Feb 2025 17:27:21 +0000 [thread overview]
Message-ID: <0e7b306641b7c0fe2d683869a0b89452144cadc7.camel@intel.com> (raw)
In-Reply-To: <20250226171707.280978-2-matthew.auld@intel.com>
On Wed, 2025-02-26 at 17:17 +0000, Matthew Auld wrote:
> Currently we just leave it uninitialised, which at first looks
> harmless,
> however we also don't zero out the pfn array, and with pfn_flags_mask
> the idea is to be able set individual flags for a given range of pfn
> or
> completely ignore them, outside of default_flags. So here we end up
> with
> pfn[i] & pfn_flags_mask, and if both are uninitialised we might get
> back
> an unexpected flags value, like asking for read only with
> default_flags,
> but getting back write on top, leading to potentially bogus
> behaviour.
>
> To fix this ensure we zero the pfn_flags_mask, such that hmm only
> considers the default_flags and not also the initial pfn[i] value.
>
> Fixes: 81e058a3e7fd ("drm/xe: Introduce helper to populate userptr")
> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> Cc: Matthew Brost <matthew.brost@intel.com>
> Cc: Thomas Hellström <thomas.hellstrom@intel.com>
> Cc: <stable@vger.kernel.org> # v6.10+
> ---
> drivers/gpu/drm/xe/xe_hmm.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/xe/xe_hmm.c
> b/drivers/gpu/drm/xe/xe_hmm.c
> index 089834467880..8c3cd65fa4b3 100644
> --- a/drivers/gpu/drm/xe/xe_hmm.c
> +++ b/drivers/gpu/drm/xe/xe_hmm.c
> @@ -206,6 +206,7 @@ int xe_hmm_userptr_populate_range(struct
> xe_userptr_vma *uvma,
> goto free_pfns;
> }
>
> + hmm_range.pfn_flags_mask = 0;
> hmm_range.default_flags = flags;
> hmm_range.hmm_pfns = pfns;
> hmm_range.notifier = &userptr->notifier;
Is there a chance we can modify the function to have an initializer for
hmm_range
struct hmm_range hmm_range = {...
That'd also make adding fields to struct hmm_range less fragile.
Either way,
Reviewed-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
next prev parent reply other threads:[~2025-02-26 17:28 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-02-26 17:17 [PATCH] drm/xe/userptr: properly setup pfn_flags_mask Matthew Auld
2025-02-26 17:27 ` Hellstrom, Thomas [this message]
2025-02-27 5:03 ` Upadhyay, Tejas
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=0e7b306641b7c0fe2d683869a0b89452144cadc7.camel@intel.com \
--to=thomas.hellstrom@intel.com \
--cc=intel-xe@lists.freedesktop.org \
--cc=matthew.auld@intel.com \
--cc=matthew.brost@intel.com \
--cc=stable@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.