public inbox for drm-ai-reviews@public-inbox.freedesktop.org
 help / color / mirror / Atom feed
From: John Stultz <jstultz@google.com>
To: Leon Romanovsky <leon@kernel.org>
Cc: Jiri Pirko <jiri@resnulli.us>,
	dri-devel@lists.freedesktop.org,  linaro-mm-sig@lists.linaro.org,
	iommu@lists.linux.dev,  linux-media@vger.kernel.org,
	sumit.semwal@linaro.org,  benjamin.gaignard@collabora.com,
	Brian.Starkey@arm.com, tjmercier@google.com,
	 christian.koenig@amd.com, m.szyprowski@samsung.com,
	robin.murphy@arm.com,  jgg@ziepe.ca, sean.anderson@linux.dev,
	ptesarik@suse.com,  catalin.marinas@arm.com,
	aneesh.kumar@kernel.org, suzuki.poulose@arm.com,
	 steven.price@arm.com, thomas.lendacky@amd.com,
	john.allen@amd.com,  ashish.kalra@amd.com,
	suravee.suthikulpanit@amd.com,  linux-coco@lists.linux.dev
Subject: Re: [PATCH 4/5] dma-buf: heaps: allow heap to specify valid heap flags
Date: Tue, 10 Feb 2026 12:05:28 -0800	[thread overview]
Message-ID: <CANDhNCoaYoe5Ckin9CnZT2LdQJ2K7amBSUS9GBDzbx_1=U9txw@mail.gmail.com> (raw)
In-Reply-To: <20260210124819.GC12887@unreal>

On Tue, Feb 10, 2026 at 4:48 AM Leon Romanovsky <leon@kernel.org> wrote:
> On Tue, Feb 10, 2026 at 10:05:14AM +0100, Jiri Pirko wrote:
> > Mon, Feb 09, 2026 at 09:08:03PM +0100, jstultz@google.com wrote:
> > >On Mon, Feb 9, 2026 at 7:38 AM Jiri Pirko <jiri@resnulli.us> wrote:
> > >>
> > >> From: Jiri Pirko <jiri@nvidia.com>
> > >>
> > >> Currently the flags, which are unused, are validated for all heaps.
> > >> Since the follow-up patch introduces a flag valid for only one of the
> > >> heaps, allow to specify the valid flags per-heap.
> > >
> > >I'm not really in this space anymore, so take my feedback with a grain of salt.
> > >
> > >While the heap allocate flags argument is unused, it was intended to
> > >be used for generic allocation flags that would apply to all or at
> > >least a wide majority of heaps.
> > >
> > >It was definitely not added to allow for per-heap or heap specific
> > >flags (as this patch tries to utilize it). That was the mess we had
> > >with ION driver that we were trying to avoid.
> > >
> > >The intent of dma-buf heaps is to try to abstract all the different
> > >device memory constraints so there only needs to be a [usage] ->
> > >[heap] mapping, and otherwise userland can be generalized so that it
> > >doesn't need to be re-written to work with different devices/memory
> > >types.  Adding heap-specific allocation flags prevents that
> > >generalization.
> > >
> > >So instead of adding heap specific flags, the general advice has been
> > >to add a separate heap name for the flag property.
> >
> > Right, my original idea was to add a separate heap. Then I spotted the
> > flags and seemed like a great fit. Was not aware or the history or
> > original intention. Would be probably good to document it for
> > future generations.
> >
> > So instead of flag, I will add heap named something
> > like "system_cc_decrypted" to implement this.
>
> It is problematic to expose a user‑visible API that depends on a name.
> Such a design limits our ability to extend the functionality in the
> future, should new use cases arise.

Yes, how userland chooses a heap name is an open problem.

 The difficulty is that userland is the only thing that knows what
devices the buffer will be shared (and this knowledge may be
incomplete if userland passes a buffer between processes) with, so it
has to pick.  But the kernel doesn't give it a way to solve the
constraints of what memory types work with what devices. There have
been some proposals for device sysfs directories to have links to heap
types they support, but that also requires every driver to understand
every heap type. And then you get to the fact that performance is what
folks really want, not compatibility and that may require some system
specific knowledge to decide.

The working solution right now is to have the system provide a  [use]
-> [heap] mapping for a specific system.

I think of this as similar to the vfs and /etc/fstab. So /home/ might
be /dev/sdb1 on one device or dev/sda1 on another.  You need some
system specific configuration.

In Android, this mapping is done by Gralloc, so buffers are requested
for a use and then Gralloc decides which heap to allocated from.

Unfortunately there doesn't seem to be a similar standard convention
elsewhere.  And I'll admit even then the enumeration of uses/pipelines
in some general form is also difficult problem (and is somewhat more
bounded for Android).

thanks
-john


  reply	other threads:[~2026-02-10 20:05 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-09 15:38 [PATCH 0/5] dma-buf: heaps: system: add an option to allocate explicitly decrypted memory Jiri Pirko
2026-02-09 15:38 ` [PATCH 1/5] dma-mapping: avoid random addr value print out on error path Jiri Pirko
2026-02-11  6:59   ` Claude review: " Claude Code Review Bot
2026-02-12 11:03   ` [PATCH 1/5] " Marek Szyprowski
2026-02-12 12:52     ` Jiri Pirko
2026-02-09 15:38 ` [PATCH 2/5] dma-mapping: introduce DMA_ATTR_CC_DECRYPTED for pre-decrypted memory Jiri Pirko
2026-02-11  6:59   ` Claude review: " Claude Code Review Bot
2026-02-09 15:38 ` [PATCH 3/5] dma-buf: heaps: use designated initializer for exp_info Jiri Pirko
2026-02-11  6:59   ` Claude review: " Claude Code Review Bot
2026-02-09 15:38 ` [PATCH 4/5] dma-buf: heaps: allow heap to specify valid heap flags Jiri Pirko
2026-02-09 20:08   ` John Stultz
2026-02-10  0:29     ` Jason Gunthorpe
2026-02-10  9:14       ` Jiri Pirko
2026-02-10 12:43         ` Jason Gunthorpe
2026-02-10 14:49           ` Jiri Pirko
2026-02-10 14:54             ` Jason Gunthorpe
2026-02-10  9:05     ` Jiri Pirko
2026-02-10 12:48       ` Leon Romanovsky
2026-02-10 20:05         ` John Stultz [this message]
2026-02-11  6:59   ` Claude review: " Claude Code Review Bot
2026-02-09 15:38 ` [PATCH 5/5] dma-buf: heaps: system: add an option to allocate explicitly decrypted memory Jiri Pirko
2026-02-10 12:02   ` kernel test robot
2026-02-10 18:03   ` kernel test robot
2026-02-11  6:59   ` Claude review: " Claude Code Review Bot
2026-02-11  6:59 ` Claude Code Review Bot

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='CANDhNCoaYoe5Ckin9CnZT2LdQJ2K7amBSUS9GBDzbx_1=U9txw@mail.gmail.com' \
    --to=jstultz@google.com \
    --cc=Brian.Starkey@arm.com \
    --cc=aneesh.kumar@kernel.org \
    --cc=ashish.kalra@amd.com \
    --cc=benjamin.gaignard@collabora.com \
    --cc=catalin.marinas@arm.com \
    --cc=christian.koenig@amd.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=iommu@lists.linux.dev \
    --cc=jgg@ziepe.ca \
    --cc=jiri@resnulli.us \
    --cc=john.allen@amd.com \
    --cc=leon@kernel.org \
    --cc=linaro-mm-sig@lists.linaro.org \
    --cc=linux-coco@lists.linux.dev \
    --cc=linux-media@vger.kernel.org \
    --cc=m.szyprowski@samsung.com \
    --cc=ptesarik@suse.com \
    --cc=robin.murphy@arm.com \
    --cc=sean.anderson@linux.dev \
    --cc=steven.price@arm.com \
    --cc=sumit.semwal@linaro.org \
    --cc=suravee.suthikulpanit@amd.com \
    --cc=suzuki.poulose@arm.com \
    --cc=thomas.lendacky@amd.com \
    --cc=tjmercier@google.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox