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@xxxxxxxxxx> 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
Attachment:
signature.asc
Description: PGP signature
