Re: [EARLY RFC][PATCH 0/4] ION per heap devices
On Tue, Feb 19, 2019 at 1:25 PM Laura Abbott wrote: > > On 2/15/19 12:24 PM, John Stultz wrote: > > This is a *very early RFC* (it builds, that's all I'll say :) > > but I wanted to share it to get some initial feedback before I > > go down the rabit hole of trying to adapt the Android userland > > code to get this fully validated. > > > > This patchset tries to implement the per-heap devices for ION. > > The main benefit is that it avoids multiplexing heap operations > > through the /dev/ion interface, and allows for each heap to have > > its own permissions/sepolicy rules. > > > > Feedback would be greatly appreciated! > > thanks > > -john > > > > Cc: Laura Abbott > > Cc: Sumit Semwal > > Cc: Liam Mark > > Cc: Brian Starkey > > Cc: Andrew F. Davis > > Cc: Alistair Strachan > > Cc: dri-devel@lists.freedesktop.org > > > > John Stultz (4): > >ion: Add ION_VERSION ioctl > >ion: Initial hack to create per heap devices > >ion: Add HEAP_INFO ioctl to be able to fetch heap type > >ion: Make "legacy" /dev/ion support optional > > > > drivers/staging/android/ion/Kconfig | 7 +++ > > drivers/staging/android/ion/ion-ioctl.c | 80 > > + > > drivers/staging/android/ion/ion.c | 51 - > > drivers/staging/android/ion/ion.h | 6 +++ > > drivers/staging/android/uapi/ion.h | 57 +++ > > 5 files changed, 191 insertions(+), 10 deletions(-) > > > > So it occurs to me if this is going to be a new ABI > all together maybe we should just declare a new allocation ioctl > to be used with it. We can keep the old ioctls around > for legacy use cases and maybe eventually delete them > and just use the new allocation ioctl with the new > split heaps. So... I did add ION_IOC_HEAP_ALLOC in my patchset. Or are you suggesting we use a new ION_IOC_MAGIC value for the new ABI? Or some larger rework? thanks -john ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [EARLY RFC][PATCH 0/4] ION per heap devices
On 2/19/19 1:30 PM, Andrew F. Davis wrote: On 2/19/19 3:25 PM, Laura Abbott wrote: On 2/15/19 12:24 PM, John Stultz wrote: This is a *very early RFC* (it builds, that's all I'll say :) but I wanted to share it to get some initial feedback before I go down the rabit hole of trying to adapt the Android userland code to get this fully validated. This patchset tries to implement the per-heap devices for ION. The main benefit is that it avoids multiplexing heap operations through the /dev/ion interface, and allows for each heap to have its own permissions/sepolicy rules. Feedback would be greatly appreciated! thanks -john Cc: Laura Abbott Cc: Sumit Semwal Cc: Liam Mark Cc: Brian Starkey Cc: Andrew F. Davis Cc: Alistair Strachan Cc: dri-devel@lists.freedesktop.org John Stultz (4): ion: Add ION_VERSION ioctl ion: Initial hack to create per heap devices ion: Add HEAP_INFO ioctl to be able to fetch heap type ion: Make "legacy" /dev/ion support optional drivers/staging/android/ion/Kconfig | 7 +++ drivers/staging/android/ion/ion-ioctl.c | 80 + drivers/staging/android/ion/ion.c | 51 - drivers/staging/android/ion/ion.h | 6 +++ drivers/staging/android/uapi/ion.h | 57 +++ 5 files changed, 191 insertions(+), 10 deletions(-) So it occurs to me if this is going to be a new ABI all together maybe we should just declare a new allocation ioctl to be used with it. We can keep the old ioctls around for legacy use cases and maybe eventually delete them and just use the new allocation ioctl with the new split heaps. Why keep the old ones, this is staging, there are no legacy users (that matter to kernel).. Slowing progress for the sake of backwards compat with staging just slows the de-staging down. I think we just fundamentally disagree here. I don't see keeping legacy users as slowing anything down. We're still getting the new ABI that we actually like and we get the chance to easily go back and test. Having a non broken ABI makes it much easier to do testing and validation and comparison. We can remove the last ABI before we move it out of staging. Thanks, Laura Andrew Thanks, Laura ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [EARLY RFC][PATCH 0/4] ION per heap devices
On 2/19/19 3:25 PM, Laura Abbott wrote: > On 2/15/19 12:24 PM, John Stultz wrote: >> This is a *very early RFC* (it builds, that's all I'll say :) >> but I wanted to share it to get some initial feedback before I >> go down the rabit hole of trying to adapt the Android userland >> code to get this fully validated. >> >> This patchset tries to implement the per-heap devices for ION. >> The main benefit is that it avoids multiplexing heap operations >> through the /dev/ion interface, and allows for each heap to have >> its own permissions/sepolicy rules. >> >> Feedback would be greatly appreciated! >> thanks >> -john >> >> Cc: Laura Abbott >> Cc: Sumit Semwal >> Cc: Liam Mark >> Cc: Brian Starkey >> Cc: Andrew F. Davis >> Cc: Alistair Strachan >> Cc: dri-devel@lists.freedesktop.org >> >> John Stultz (4): >> ion: Add ION_VERSION ioctl >> ion: Initial hack to create per heap devices >> ion: Add HEAP_INFO ioctl to be able to fetch heap type >> ion: Make "legacy" /dev/ion support optional >> >> drivers/staging/android/ion/Kconfig | 7 +++ >> drivers/staging/android/ion/ion-ioctl.c | 80 >> + >> drivers/staging/android/ion/ion.c | 51 - >> drivers/staging/android/ion/ion.h | 6 +++ >> drivers/staging/android/uapi/ion.h | 57 +++ >> 5 files changed, 191 insertions(+), 10 deletions(-) >> > > So it occurs to me if this is going to be a new ABI > all together maybe we should just declare a new allocation ioctl > to be used with it. We can keep the old ioctls around > for legacy use cases and maybe eventually delete them > and just use the new allocation ioctl with the new > split heaps. > Why keep the old ones, this is staging, there are no legacy users (that matter to kernel).. Slowing progress for the sake of backwards compat with staging just slows the de-staging down. Andrew > Thanks, > Laura ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [EARLY RFC][PATCH 0/4] ION per heap devices
On 2/15/19 12:24 PM, John Stultz wrote: This is a *very early RFC* (it builds, that's all I'll say :) but I wanted to share it to get some initial feedback before I go down the rabit hole of trying to adapt the Android userland code to get this fully validated. This patchset tries to implement the per-heap devices for ION. The main benefit is that it avoids multiplexing heap operations through the /dev/ion interface, and allows for each heap to have its own permissions/sepolicy rules. Feedback would be greatly appreciated! thanks -john Cc: Laura Abbott Cc: Sumit Semwal Cc: Liam Mark Cc: Brian Starkey Cc: Andrew F. Davis Cc: Alistair Strachan Cc: dri-devel@lists.freedesktop.org John Stultz (4): ion: Add ION_VERSION ioctl ion: Initial hack to create per heap devices ion: Add HEAP_INFO ioctl to be able to fetch heap type ion: Make "legacy" /dev/ion support optional drivers/staging/android/ion/Kconfig | 7 +++ drivers/staging/android/ion/ion-ioctl.c | 80 + drivers/staging/android/ion/ion.c | 51 - drivers/staging/android/ion/ion.h | 6 +++ drivers/staging/android/uapi/ion.h | 57 +++ 5 files changed, 191 insertions(+), 10 deletions(-) So it occurs to me if this is going to be a new ABI all together maybe we should just declare a new allocation ioctl to be used with it. We can keep the old ioctls around for legacy use cases and maybe eventually delete them and just use the new allocation ioctl with the new split heaps. Thanks, Laura ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [EARLY RFC][PATCH 0/4] ION per heap devices
On 2/19/19 9:21 AM, John Stultz wrote: On Mon, Feb 18, 2019 at 3:51 AM Brian Starkey wrote: On Fri, Feb 15, 2019 at 12:24:08PM -0800, John Stultz wrote: This is a *very early RFC* (it builds, that's all I'll say :) but I wanted to share it to get some initial feedback before I go down the rabit hole of trying to adapt the Android userland code to get this fully validated. This patchset tries to implement the per-heap devices for ION. The main benefit is that it avoids multiplexing heap operations through the /dev/ion interface, and allows for each heap to have its own permissions/sepolicy rules. In general, I've always thought that having a device node per-heap is a bit messy for userspace. Multiplexing through the single node doesn't seem like an insurmountable problem, but the point about Hrm. I guess for me having a custom enumeration interface over ioctl seems less ideal compared to a directory list. permissions/sepolicy is reasonably compelling if it's a real requirement. What would be the reasons for that? Its a bit second hand for me, so hopefully I don't have it wrong. I've cc'ed some additional google folks and Benjamin for extra context, but my sense of it is that you may have some less-trusted code that we're fine with allocating system heap dma-bufs, but don't want to to be giving access to more limited heaps like carveout or cma, or more potentially security troubling heaps like system-contig. thanks -john Yes, the discussion was always based on being able to set separate security policy for each heap. It's not clear to me how strong a requirement is these days or if there's other options to enforce the same thing. Thanks, Laura ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [EARLY RFC][PATCH 0/4] ION per heap devices
On Mon, Feb 18, 2019 at 3:51 AM Brian Starkey wrote: > On Fri, Feb 15, 2019 at 12:24:08PM -0800, John Stultz wrote: > > This is a *very early RFC* (it builds, that's all I'll say :) > > but I wanted to share it to get some initial feedback before I > > go down the rabit hole of trying to adapt the Android userland > > code to get this fully validated. > > > > This patchset tries to implement the per-heap devices for ION. > > The main benefit is that it avoids multiplexing heap operations > > through the /dev/ion interface, and allows for each heap to have > > its own permissions/sepolicy rules. > > In general, I've always thought that having a device node per-heap is > a bit messy for userspace. Multiplexing through the single node > doesn't seem like an insurmountable problem, but the point about Hrm. I guess for me having a custom enumeration interface over ioctl seems less ideal compared to a directory list. > permissions/sepolicy is reasonably compelling if it's a real > requirement. What would be the reasons for that? Its a bit second hand for me, so hopefully I don't have it wrong. I've cc'ed some additional google folks and Benjamin for extra context, but my sense of it is that you may have some less-trusted code that we're fine with allocating system heap dma-bufs, but don't want to to be giving access to more limited heaps like carveout or cma, or more potentially security troubling heaps like system-contig. thanks -john ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [EARLY RFC][PATCH 0/4] ION per heap devices
Hi John, On Fri, Feb 15, 2019 at 12:24:08PM -0800, John Stultz wrote: > This is a *very early RFC* (it builds, that's all I'll say :) > but I wanted to share it to get some initial feedback before I > go down the rabit hole of trying to adapt the Android userland > code to get this fully validated. > > This patchset tries to implement the per-heap devices for ION. > The main benefit is that it avoids multiplexing heap operations > through the /dev/ion interface, and allows for each heap to have > its own permissions/sepolicy rules. In general, I've always thought that having a device node per-heap is a bit messy for userspace. Multiplexing through the single node doesn't seem like an insurmountable problem, but the point about permissions/sepolicy is reasonably compelling if it's a real requirement. What would be the reasons for that? Thanks, -Brian > > Feedback would be greatly appreciated! > thanks > -john > > Cc: Laura Abbott > Cc: Sumit Semwal > Cc: Liam Mark > Cc: Brian Starkey > Cc: Andrew F. Davis > Cc: Alistair Strachan > Cc: dri-devel@lists.freedesktop.org > > John Stultz (4): > ion: Add ION_VERSION ioctl > ion: Initial hack to create per heap devices > ion: Add HEAP_INFO ioctl to be able to fetch heap type > ion: Make "legacy" /dev/ion support optional > > drivers/staging/android/ion/Kconfig | 7 +++ > drivers/staging/android/ion/ion-ioctl.c | 80 > + > drivers/staging/android/ion/ion.c | 51 - > drivers/staging/android/ion/ion.h | 6 +++ > drivers/staging/android/uapi/ion.h | 57 +++ > 5 files changed, 191 insertions(+), 10 deletions(-) > > -- > 2.7.4 > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
