Re: [PATCH 1/3] drm/xe/oa: Use xe_map layer

["Dixit, Ashutosh" <ashutosh.dixit@intel.com>]

On Tue, 07 Apr 2026 15:49:27 -0700, Umesh Nerlige Ramappa wrote:
>

Hi Umesh,

> On Mon, Apr 06, 2026 at 08:02:17PM -0700, Ashutosh Dixit wrote:
> > @@ -311,30 +317,37 @@ static enum hrtimer_restart xe_oa_poll_check_timer_cb(struct hrtimer *hrtimer)
> >	return HRTIMER_RESTART;
> > }
> >
> > +static inline unsigned long
> > +xe_oa_copy_to_user(void __user *dst, const struct iosys_map *src, size_t src_offset, size_t len)
> > +{
> > +	if (src->is_iomem)
> > +		return copy_to_user(dst, src->vaddr_iomem + src_offset, len);
>
> I think this requires xe_map_memcpy_from for the iomem case.
>
> > +	else
> > +		return copy_to_user(dst, src->vaddr + src_offset, len);
> > +}
> > +

Yes, you have certainly spotted the iffy part of this patch. However, this
situation is not that simple:

* xe_map_memcpy_from or iosys_map_memcpy_from copies from iomem to kernel
  memory (potentially using special accessors for iomem), not to user
  memory

* copy_to_user copies from kernel memory to user memory (using special
  checks), not from iomem

So in the kernel we don't have functions which copy directly from iomem to
user memory. That is, functions like iosys_map_copy_to_user() or
copy_to_user_from_iomem() do not exist, because they would need to combine
iomem access and copy_to_user (likely copy_to_user will need to be
rewritten with iomem accessors, if such a function were to be
implemented). There may be a reason why such functions don't exist in the
kernel.

Another way might be to copy from iomem to kernel memory and from kernel to
user memory (that is have both iosys_map_memcpy_from and copy_to_user
calls), but this would entail an extra copy into a bounce buffer in kernel
memory. Something like this:

static inline unsigned long
xe_oa_copy_to_user(void __user *dst, const struct iosys_map *src, size_t src_offset, size_t len)
{
	void *tmp = kmalloc(len, GFP_KERNEL); // kmalloc or statically allocated tmp buffer

	xe_map_memcpy_from(xe, tmp, src, src_offset, len); // extra copy

	return copy_to_user(dst, tmp, len);
}

In our case, xe_map_memcpy_from or iosys_map_memcpy_from are basically a
memcpy (because vram bar is ioremapped, special accessors for iomem are not
needed). Therefore, to avoid the extra copy, I decided to drop the iomem
access part (since the copy_to_user part cannot be dropped). Hence I end up
with the xe_oa_copy_to_user() above. The function was tested on BMG with OA
buffer in vram and works as expected.

I did think about renaming the above function xe_map_copy_to_user() and
moving it to xe_map.h, so that is possible. But since there are no other
users (and none are expected) at this point of xe_map_copy_to_user() apart
from OA because of this workaround (other observation stream buffers are in
system memory, not vram), I left the function as specific to OA.

Let me know what you think. Thanks.