libcamera: dma_buf_allocator: Work around lack of file seals in uClibc
diff mbox series

Message ID 20240605083533.30718-1-laurent.pinchart@ideasonboard.com
State Accepted
Headers show
Series
  • libcamera: dma_buf_allocator: Work around lack of file seals in uClibc
Related show

Commit Message

Laurent Pinchart June 5, 2024, 8:35 a.m. UTC 
uClibc doesn't provide the macros defining parameters for the file
sealing API. Define them manually as a work around.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
An alternative would be to disable udmabuf support on such platforms. I
think we can expect someone running libcamera on a uClibc system to be
able to enable DMA heaps.
---
 meson.build                         | 4 ++++
 src/libcamera/dma_buf_allocator.cpp | 5 +++++
 2 files changed, 9 insertions(+)


base-commit: 98071d3109c131820439f61d9380c0bd4cd2119a

Comments

Laurent Pinchart June 5, 2024, 8:41 a.m. UTC | #1 
On Wed, Jun 05, 2024 at 11:35:33AM +0300, Laurent Pinchart wrote:
> uClibc doesn't provide the macros defining parameters for the file
> sealing API. Define them manually as a work around.
> 

Fixes: ea4baaacc325 ("libcamera: DmaBufAllocator: Support allocating from /dev/udmabuf")

> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> An alternative would be to disable udmabuf support on such platforms. I
> think we can expect someone running libcamera on a uClibc system to be
> able to enable DMA heaps.
> ---
>  meson.build                         | 4 ++++
>  src/libcamera/dma_buf_allocator.cpp | 5 +++++
>  2 files changed, 9 insertions(+)
> 
> diff --git a/meson.build b/meson.build
> index 1902ea2fd3ff..0ef4cdaafd76 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -74,6 +74,10 @@ cc = meson.get_compiler('c')
>  cxx = meson.get_compiler('cpp')
>  config_h = configuration_data()
>  
> +if cc.has_header_symbol('fcntl.h', 'F_ADD_SEALS', prefix : '#define _GNU_SOURCE')
> +    config_h.set('HAVE_FILE_SEALS', 1)
> +endif
> +
>  if cc.has_header_symbol('unistd.h', 'issetugid')
>      config_h.set('HAVE_ISSETUGID', 1)
>  endif
> diff --git a/src/libcamera/dma_buf_allocator.cpp b/src/libcamera/dma_buf_allocator.cpp
> index d7d08e188a62..1c39441a3415 100644
> --- a/src/libcamera/dma_buf_allocator.cpp
> +++ b/src/libcamera/dma_buf_allocator.cpp
> @@ -157,6 +157,11 @@ UniqueFD DmaBufAllocator::allocFromUDmaBuf(const char *name, std::size_t size)
>  		return {};
>  	}
>  
> +#if not HAVE_FILE_SEALS
> +#define F_ADD_SEALS	1033
> +#define F_SEAL_SHRINK	0x0002
> +#endif
> +
>  	/* udmabuf dma-buffers *must* have the F_SEAL_SHRINK seal. */
>  	ret = fcntl(memfd.get(), F_ADD_SEALS, F_SEAL_SHRINK);
>  	if (ret < 0) {
> 
> base-commit: 98071d3109c131820439f61d9380c0bd4cd2119a
Kieran Bingham June 5, 2024, 8:45 a.m. UTC | #2 
Quoting Laurent Pinchart (2024-06-05 09:35:33)
> uClibc doesn't provide the macros defining parameters for the file
> sealing API. Define them manually as a work around.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> An alternative would be to disable udmabuf support on such platforms. I
> think we can expect someone running libcamera on a uClibc system to be
> able to enable DMA heaps.

I think this is fine, and keeps our uClibc compile coverage up.

> ---
>  meson.build                         | 4 ++++
>  src/libcamera/dma_buf_allocator.cpp | 5 +++++
>  2 files changed, 9 insertions(+)
> 
> diff --git a/meson.build b/meson.build
> index 1902ea2fd3ff..0ef4cdaafd76 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -74,6 +74,10 @@ cc = meson.get_compiler('c')
>  cxx = meson.get_compiler('cpp')
>  config_h = configuration_data()
>  
> +if cc.has_header_symbol('fcntl.h', 'F_ADD_SEALS', prefix : '#define _GNU_SOURCE')
> +    config_h.set('HAVE_FILE_SEALS', 1)
> +endif
> +
>  if cc.has_header_symbol('unistd.h', 'issetugid')
>      config_h.set('HAVE_ISSETUGID', 1)
>  endif
> diff --git a/src/libcamera/dma_buf_allocator.cpp b/src/libcamera/dma_buf_allocator.cpp
> index d7d08e188a62..1c39441a3415 100644
> --- a/src/libcamera/dma_buf_allocator.cpp
> +++ b/src/libcamera/dma_buf_allocator.cpp
> @@ -157,6 +157,11 @@ UniqueFD DmaBufAllocator::allocFromUDmaBuf(const char *name, std::size_t size)
>                 return {};
>         }
>  
> +#if not HAVE_FILE_SEALS

A one line comment above saying why they are hardcoded here for context
to readers coming across this would be nice but optional.

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

> +#define F_ADD_SEALS    1033
> +#define F_SEAL_SHRINK  0x0002
> +#endif
> +
>         /* udmabuf dma-buffers *must* have the F_SEAL_SHRINK seal. */
>         ret = fcntl(memfd.get(), F_ADD_SEALS, F_SEAL_SHRINK);
>         if (ret < 0) {
> 
> base-commit: 98071d3109c131820439f61d9380c0bd4cd2119a
> -- 
> Regards,
> 
> Laurent Pinchart
>
Hans de Goede June 5, 2024, 9:08 a.m. UTC | #3 
Hi,

On 6/5/24 10:35 AM, Laurent Pinchart wrote:
> uClibc doesn't provide the macros defining parameters for the file
> sealing API. Define them manually as a work around.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Thanks, patch looks good to me:

Reviewed-by: Hans de Goede <hdegoede@redhat.com>

Regards,

Hans


> ---
> An alternative would be to disable udmabuf support on such platforms. I
> think we can expect someone running libcamera on a uClibc system to be
> able to enable DMA heaps.
> ---
>  meson.build                         | 4 ++++
>  src/libcamera/dma_buf_allocator.cpp | 5 +++++
>  2 files changed, 9 insertions(+)
> 
> diff --git a/meson.build b/meson.build
> index 1902ea2fd3ff..0ef4cdaafd76 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -74,6 +74,10 @@ cc = meson.get_compiler('c')
>  cxx = meson.get_compiler('cpp')
>  config_h = configuration_data()
>  
> +if cc.has_header_symbol('fcntl.h', 'F_ADD_SEALS', prefix : '#define _GNU_SOURCE')
> +    config_h.set('HAVE_FILE_SEALS', 1)
> +endif
> +
>  if cc.has_header_symbol('unistd.h', 'issetugid')
>      config_h.set('HAVE_ISSETUGID', 1)
>  endif
> diff --git a/src/libcamera/dma_buf_allocator.cpp b/src/libcamera/dma_buf_allocator.cpp
> index d7d08e188a62..1c39441a3415 100644
> --- a/src/libcamera/dma_buf_allocator.cpp
> +++ b/src/libcamera/dma_buf_allocator.cpp
> @@ -157,6 +157,11 @@ UniqueFD DmaBufAllocator::allocFromUDmaBuf(const char *name, std::size_t size)
>  		return {};
>  	}
>  
> +#if not HAVE_FILE_SEALS
> +#define F_ADD_SEALS	1033
> +#define F_SEAL_SHRINK	0x0002
> +#endif
> +
>  	/* udmabuf dma-buffers *must* have the F_SEAL_SHRINK seal. */
>  	ret = fcntl(memfd.get(), F_ADD_SEALS, F_SEAL_SHRINK);
>  	if (ret < 0) {
> 
> base-commit: 98071d3109c131820439f61d9380c0bd4cd2119a
Laurent Pinchart June 5, 2024, 9:10 a.m. UTC | #4 
On Wed, Jun 05, 2024 at 09:45:28AM +0100, Kieran Bingham wrote:
> Quoting Laurent Pinchart (2024-06-05 09:35:33)
> > uClibc doesn't provide the macros defining parameters for the file
> > sealing API. Define them manually as a work around.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> > An alternative would be to disable udmabuf support on such platforms. I
> > think we can expect someone running libcamera on a uClibc system to be
> > able to enable DMA heaps.
> 
> I think this is fine, and keeps our uClibc compile coverage up.
> 
> > ---
> >  meson.build                         | 4 ++++
> >  src/libcamera/dma_buf_allocator.cpp | 5 +++++
> >  2 files changed, 9 insertions(+)
> > 
> > diff --git a/meson.build b/meson.build
> > index 1902ea2fd3ff..0ef4cdaafd76 100644
> > --- a/meson.build
> > +++ b/meson.build
> > @@ -74,6 +74,10 @@ cc = meson.get_compiler('c')
> >  cxx = meson.get_compiler('cpp')
> >  config_h = configuration_data()
> >  
> > +if cc.has_header_symbol('fcntl.h', 'F_ADD_SEALS', prefix : '#define _GNU_SOURCE')
> > +    config_h.set('HAVE_FILE_SEALS', 1)
> > +endif
> > +
> >  if cc.has_header_symbol('unistd.h', 'issetugid')
> >      config_h.set('HAVE_ISSETUGID', 1)
> >  endif
> > diff --git a/src/libcamera/dma_buf_allocator.cpp b/src/libcamera/dma_buf_allocator.cpp
> > index d7d08e188a62..1c39441a3415 100644
> > --- a/src/libcamera/dma_buf_allocator.cpp
> > +++ b/src/libcamera/dma_buf_allocator.cpp
> > @@ -157,6 +157,11 @@ UniqueFD DmaBufAllocator::allocFromUDmaBuf(const char *name, std::size_t size)
> >                 return {};
> >         }
> >  
> > +#if not HAVE_FILE_SEALS
> 
> A one line comment above saying why they are hardcoded here for context
> to readers coming across this would be nice but optional.

I'll add

	/* uClibc doesn't provide the file sealig API. */

> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> 
> > +#define F_ADD_SEALS    1033
> > +#define F_SEAL_SHRINK  0x0002
> > +#endif
> > +
> >         /* udmabuf dma-buffers *must* have the F_SEAL_SHRINK seal. */
> >         ret = fcntl(memfd.get(), F_ADD_SEALS, F_SEAL_SHRINK);
> >         if (ret < 0) {
> > 
> > base-commit: 98071d3109c131820439f61d9380c0bd4cd2119a

Patch
diff mbox series 

diff --git a/meson.build b/meson.build
index 1902ea2fd3ff..0ef4cdaafd76 100644
--- a/meson.build
+++ b/meson.build
@@ -74,6 +74,10 @@  cc = meson.get_compiler('c')
 cxx = meson.get_compiler('cpp')
 config_h = configuration_data()
 
+if cc.has_header_symbol('fcntl.h', 'F_ADD_SEALS', prefix : '#define _GNU_SOURCE')
+    config_h.set('HAVE_FILE_SEALS', 1)
+endif
+
 if cc.has_header_symbol('unistd.h', 'issetugid')
     config_h.set('HAVE_ISSETUGID', 1)
 endif
diff --git a/src/libcamera/dma_buf_allocator.cpp b/src/libcamera/dma_buf_allocator.cpp
index d7d08e188a62..1c39441a3415 100644
--- a/src/libcamera/dma_buf_allocator.cpp
+++ b/src/libcamera/dma_buf_allocator.cpp
@@ -157,6 +157,11 @@  UniqueFD DmaBufAllocator::allocFromUDmaBuf(const char *name, std::size_t size)
 		return {};
 	}
 
+#if not HAVE_FILE_SEALS
+#define F_ADD_SEALS	1033
+#define F_SEAL_SHRINK	0x0002
+#endif
+
 	/* udmabuf dma-buffers *must* have the F_SEAL_SHRINK seal. */
 	ret = fcntl(memfd.get(), F_ADD_SEALS, F_SEAL_SHRINK);
 	if (ret < 0) {