ECC memory semantics for heaps

[Maxime Ripard]

Hi John,

Thanks for your answer

On Wed, Feb 28, 2024 at 08:17:55PM -0800, John Stultz wrote:
> On Wed, Feb 28, 2024 at 7:24 AM Maxime Ripard <mripard at redhat.com> wrote:
> >
> > I'm currently working on a platform that seems to have togglable RAM ECC
> > support. Enabling ECC reduces the memory capacity and memory bandwidth,
> > so while it's a good idea to protect most of the system, it's not worth
> > it for things like framebuffers that won't really be affected by a
> > bitflip.
> >
> > It's currently setup by enabling ECC on the entire memory, and then
> > having a region of memory where ECC is disabled and where we're supposed
> > to allocate from for allocations that don't need it.
> >
> > My first thought to support this was to create a reserved memory region
> > for the !ECC memory, and to create a heap to allocate buffers from that
> > region. That would leave the system protected by ECC, while enabling
> > userspace to be nicer to the system by allocating buffers from the !ECC
> > region if it doesn't need it.
> >
> > However, this creates basically a new combination compared to the one we
> > already have (ie, physically contiguous vs virtually contiguous), and we
> > probably would want to throw in cacheable vs non-cacheable too.
> >
> > If we had to provide new heaps for each variation, we would have 8 heaps
> > (and 6 new ones), which could be fine I guess but would still increase
> > quite a lot the number of heaps we have so far.
> >
> > Is it something that would be a problem? If it is, do you see another
> > way to support those kind of allocations (like providing hints through
> > the ioctl maybe?)?
> 
> So, the dma-buf heaps interface uses chardevs so that we can have a
> lot of flexibility in the types of heaps (and don't have the risk of
> bitmask exhaustion like ION had). So I don't see adding many
> differently named heaps as particularly problematic.

Ok

> That said, if there are truly generic properties (cacheable vs
> non-cachable is maybe one of those) which apply to most heaps, I'm
> open to making use of the flags. But I want to avoid having per-heap
> flags, it really needs to be a generic attribute.
> 
> And I personally don't mind the idea of having things added as heaps
> initially, and potentially upgrading them to flags if needed (allowing
> heap drivers to optionally enumerate the old chardevs behind a config
> option for backwards compatibility).
> 
> How common is the hardware that is going to provide this configurable
> ECC option

In terms of number of SoCs with the feature, it's probably a handful. In
terms of number of units shipped, we're in the fairly common range :)

> and will you really want the option on all of the heap types?

Aside from the cacheable/uncacheable discussion, yes. We could probably
get away with only physically contiguous allocations at the moment
though, I'll double check.

> Will there be any hardware constraint limitations caused by the
> ECC/!ECC flags? (ie: Devices that can't use !ECC allocated buffers?)

My understanding is that there's no device restriction. It will be a
carved out memory so we will need to maintain a separate pool and it
will be limited in size, but that's pretty much the only one afaik.

> If not, I wonder if it would make sense to have something more along
> the lines using a fcntl()  like how F_SEAL_* is used with memfds?
> With some of the discussion around "restricted"/"secure" heaps that
> can change state, I've liked this idea of just allocating dmabufs from
> normal heaps and then using fcntl or something similar to modify
> properties of the buffer that are separate from the type of memory
> that is needed to be allocated to satisfy device constraints.

Sorry, I'm not super familiar with F_SEAL so I don't really follow what
you have in mind here. Do you have any additional resources I could read
to understand better what you're thinking about?

Also, if we were to modify the ECC attributes after the dma-buf has been
allocated by dma-buf, and if the !ECC memory is carved out only, then
wouldn't that mean we would need to reallocate the backing buffer for
that dma-buf?

Thanks!
Maxime
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20240304/db1c8da8/attachment.sig>