[libcamera-devel,v3,1/2] libcamera: ipa_module: add IPA shared library module

1238 diff mbox series

Implement a class to wrap around an IPA module shared object.

For now, just load a struct IPAModuleInfo with symbol name
ipaModuleInfo from an IPA module .so shared object.

Also provide a public header file including the struct IPAModuleInfo,
structured such that both C and C++ IPA modules are supported.

Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
---
Changes in v3:
- created public header file for IPAModuleInfo (and for handling C vs
  C++ IPA modules)
- made ELF loading/parsing functions static
- got rid of bitClass_
- other cosmetic changes

Changes in v2:
- renamed LibLoader class to IPAModule
- added documentation
- added logging
- check that bitness of the shared object is the same as libcamera
- moved symbol loading ("init") to the constructor, and added isValid()
- added elfPointer() to prevent segfaults when reading data from mmap
- moved struct IPAModuleInfo out of IPAModule
- rename getIPAModuleInfo() to IPAModuleInfo(), and make it return a
  const reference
- added munmap after the mmap

 include/libcamera/ipa/ipa_module_info.h |  31 +++
 include/libcamera/meson.build           |   1 +
 src/libcamera/include/ipa_module.h      |  35 +++
 src/libcamera/ipa_module.cpp            | 280 ++++++++++++++++++++++++
 src/libcamera/meson.build               |   2 +
 5 files changed, 349 insertions(+)
 create mode 100644 include/libcamera/ipa/ipa_module_info.h
 create mode 100644 src/libcamera/include/ipa_module.h
 create mode 100644 src/libcamera/ipa_module.cpp

On Mon, May 20, 2019 at 08:40:58PM -0400, Paul Elder wrote:
> Implement a class to wrap around an IPA module shared object.
> 
> For now, just load a struct IPAModuleInfo with symbol name
> ipaModuleInfo from an IPA module .so shared object.
> 
> Also provide a public header file including the struct IPAModuleInfo,
> structured such that both C and C++ IPA modules are supported.
> 
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> ---
> Changes in v3:
> - created public header file for IPAModuleInfo (and for handling C vs
>   C++ IPA modules)
> - made ELF loading/parsing functions static
> - got rid of bitClass_
> - other cosmetic changes
> 
> Changes in v2:
> - renamed LibLoader class to IPAModule
> - added documentation
> - added logging
> - check that bitness of the shared object is the same as libcamera
> - moved symbol loading ("init") to the constructor, and added isValid()
> - added elfPointer() to prevent segfaults when reading data from mmap
> - moved struct IPAModuleInfo out of IPAModule
> - rename getIPAModuleInfo() to IPAModuleInfo(), and make it return a
>   const reference
> - added munmap after the mmap
> 
>  include/libcamera/ipa/ipa_module_info.h |  31 +++
>  include/libcamera/meson.build           |   1 +
>  src/libcamera/include/ipa_module.h      |  35 +++
>  src/libcamera/ipa_module.cpp            | 280 ++++++++++++++++++++++++
>  src/libcamera/meson.build               |   2 +
>  5 files changed, 349 insertions(+)
>  create mode 100644 include/libcamera/ipa/ipa_module_info.h
>  create mode 100644 src/libcamera/include/ipa_module.h
>  create mode 100644 src/libcamera/ipa_module.cpp
> 
> diff --git a/include/libcamera/ipa/ipa_module_info.h b/include/libcamera/ipa/ipa_module_info.h
> new file mode 100644
> index 0000000..eae60f6
> --- /dev/null
> +++ b/include/libcamera/ipa/ipa_module_info.h
> @@ -0,0 +1,31 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2019, Google Inc.
> + *
> + * ipa_module.h - Image Processing Algorithm module

s/ipa_module/ipa_module_info/

and a similar change for the description that then follows.

> + */
> +#ifndef __LIBCAMERA_IPA_MODULE_INFO_H__
> +#define __LIBCAMERA_IPA_MODULE_INFO_H__
> +
> +#ifdef __cplusplus
> +namespace libcamera {
> +#endif
> +
> +struct IPAModuleInfo {
> +	char name[256];
> +	unsigned int version;
> +};
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +extern const struct IPAModuleInfo ipaModuleInfo;
> +#ifdef __cplusplus
> +};
> +#endif
> +
> +#ifdef __cplusplus
> +}; /* namespace libcamera */
> +#endif

All this __cplusplus handling isn't neat, but I think we can live with
it for now. We will later decide whether to mandate a C++ ABI for IPA
modules (as in a C-style generator function that returns a pointer to a
C++ object), or stick with a pure C API (as in a structure of C function
pointers). In the former case we can drop __cplusplus from here, as IPAs
would have to be implemented in C++. Let's just keep this in mind.

> +
> +#endif /* __LIBCAMERA_IPA_MODULE_INFO_H__ */
> diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build
> index 83d226a..cb64f0c 100644
> --- a/include/libcamera/meson.build
> +++ b/include/libcamera/meson.build
> @@ -5,6 +5,7 @@ libcamera_api = files([
>      'event_dispatcher.h',
>      'event_notifier.h',
>      'geometry.h',
> +    'ipa/ipa_module_info.h',
>      'libcamera.h',
>      'object.h',
>      'request.h',
> diff --git a/src/libcamera/include/ipa_module.h b/src/libcamera/include/ipa_module.h
> new file mode 100644
> index 0000000..a13ea4a
> --- /dev/null
> +++ b/src/libcamera/include/ipa_module.h
> @@ -0,0 +1,35 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2019, Google Inc.
> + *
> + * ipa_module.h - Image Processing Algorithm module
> + */
> +#ifndef __LIBCAMERA_IPA_MODULE_H__
> +#define __LIBCAMERA_IPA_MODULE_H__
> +
> +#include <libcamera/ipa/ipa_module_info.h>
> +#include <string>
> +
> +namespace libcamera {
> +
> +class IPAModule
> +{
> +public:
> +	explicit IPAModule(const std::string &libPath);
> +
> +	bool isValid() const;
> +
> +	const struct IPAModuleInfo &info() const;
> +
> +private:
> +	struct IPAModuleInfo info_;
> +
> +	std::string libPath_;
> +	bool valid_;
> +
> +	int loadIPAModuleInfo(const char *libPath);
> +};
> +
> +} /* namespace libcamera */
> +
> +#endif /* __LIBCAMERA_IPA_MODULE_H__ */
> diff --git a/src/libcamera/ipa_module.cpp b/src/libcamera/ipa_module.cpp
> new file mode 100644
> index 0000000..acfe1af
> --- /dev/null
> +++ b/src/libcamera/ipa_module.cpp
> @@ -0,0 +1,280 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2019, Google Inc.
> + *
> + * ipa_module.cpp - Image Processing Algorithm module
> + */
> +
> +#include "ipa_module.h"
> +
> +#include <elf.h>
> +#include <errno.h>
> +#include <fcntl.h>
> +#include <string.h>
> +#include <sys/mman.h>
> +#include <sys/stat.h>
> +#include <sys/types.h>
> +#include <unistd.h>
> +
> +#include "log.h"
> +
> +/**
> + * \file ipa_module.h
> + * \brief Image Processing Algorithm module
> + */
> +
> +/**
> + * \file ipa_module_info.h
> + * \brief Image Processing Algorithm module information
> + */
> +
> +namespace libcamera {
> +
> +LOG_DEFINE_CATEGORY(IPAModule)
> +
> +namespace {
> +
> +template<typename T>
> +typename std::remove_extent<T>::type *elfPointer(void *map, off_t offset,
> +						 size_t fileSize)
> +{
> +	size_t size = offset + sizeof(T);
> +	if (size > fileSize || size < sizeof(T))
> +		return nullptr;
> +
> +	return reinterpret_cast<typename std::remove_extent<T>::type *>
> +		(static_cast<char *>(map) + offset);
> +}
> +
> +int elfVerifyIdent(void *map, size_t soSize)
> +{
> +	char *e_ident = elfPointer<char[EI_NIDENT]>(map, 0, soSize);
> +	if (!e_ident)
> +		return -ENOEXEC;
> +
> +	if (e_ident[EI_MAG0] != ELFMAG0 ||
> +	    e_ident[EI_MAG1] != ELFMAG1 ||
> +	    e_ident[EI_MAG2] != ELFMAG2 ||
> +	    e_ident[EI_MAG3] != ELFMAG3 ||
> +	    e_ident[EI_VERSION] != EV_CURRENT)
> +		return -ENOEXEC;
> +
> +	int bitClass = sizeof(unsigned long) == 4 ? ELFCLASS32 : ELFCLASS64;
> +	if (e_ident[EI_CLASS] != bitClass)
> +		return -ENOEXEC;
> +
> +	int a = 1;
> +	unsigned char endianness = *reinterpret_cast<char *>(&a) == 1
> +				 ? ELFDATA2LSB : ELFDATA2MSB;
> +	if (e_ident[EI_DATA] != endianness)
> +		return -ENOEXEC;
> +
> +	return 0;
> +}
> +
> +template<class ElfHeader, class SecHeader, class SymHeader>
> +int elfLoadSymbol(void *dst, size_t size, void *map, size_t soSize,
> +			  const char *symbol)
> +{
> +	ElfHeader *eHdr = elfPointer<ElfHeader>(map, 0, soSize);
> +	if (!eHdr)
> +		return -ENOEXEC;
> +
> +	off_t offset = eHdr->e_shoff + eHdr->e_shentsize * eHdr->e_shstrndx;
> +	SecHeader *sHdr = elfPointer<SecHeader>(map, offset, soSize);
> +	if (!sHdr)
> +		return -ENOEXEC;
> +	off_t shnameoff = sHdr->sh_offset;
> +
> +	/* Locate .dynsym section header. */
> +	SecHeader *dynsym = nullptr;
> +	for (unsigned int i = 0; i < eHdr->e_shnum; i++) {
> +		offset = eHdr->e_shoff + eHdr->e_shentsize * i;
> +		sHdr = elfPointer<SecHeader>(map, offset, soSize);
> +		if (!sHdr)
> +			return -ENOEXEC;
> +
> +		offset = shnameoff + sHdr->sh_name;
> +		char *name = elfPointer<char[8]>(map, offset, soSize);
> +		if (!name)
> +			return -ENOEXEC;
> +
> +		if (sHdr->sh_type == SHT_DYNSYM && !strcmp(name, ".dynsym")) {
> +			dynsym = sHdr;
> +			break;
> +		}
> +	}
> +
> +	if (dynsym == nullptr) {
> +		LOG(IPAModule, Error) << "ELF has no .dynsym section";
> +		return -ENOEXEC;
> +	}
> +
> +	offset = eHdr->e_shoff + eHdr->e_shentsize * dynsym->sh_link;
> +	sHdr = elfPointer<SecHeader>(map, offset, soSize);
> +	if (!sHdr)
> +		return -ENOEXEC;
> +	off_t dynsym_nameoff = sHdr->sh_offset;
> +
> +	/* Locate symbol in the .dynsym section. */
> +	SymHeader *targetSymbol = nullptr;
> +	unsigned int dynsym_num = dynsym->sh_size / dynsym->sh_entsize;
> +	for (unsigned int i = 0; i < dynsym_num; i++) {
> +		offset = dynsym->sh_offset + dynsym->sh_entsize * i;
> +		SymHeader *sym = elfPointer<SymHeader>(map, offset, soSize);
> +		if (!sym)
> +			return -ENOEXEC;
> +
> +		offset = dynsym_nameoff + sym->st_name;
> +		char *name = elfPointer<char>(map, offset, soSize);

You need a char[strlen(symbol)+1] here.

> +		if (!name)
> +			return -ENOEXEC;
> +
> +		if (!strcmp(name, symbol) &&
> +		    sym->st_info & STB_GLOBAL &&
> +		    sym->st_size == size) {

This could hold on two lines instead of three.

> +			targetSymbol = sym;
> +			break;
> +		}
> +	}
> +
> +	if (targetSymbol == nullptr) {
> +		LOG(IPAModule, Error) << "Symbol " << symbol << " not found";
> +		return -ENOEXEC;
> +	}
> +
> +	/* Locate and return data of symbol. */
> +	if (targetSymbol->st_shndx >= eHdr->e_shnum)
> +		return -ENOEXEC;
> +	offset = eHdr->e_shoff + targetSymbol->st_shndx * eHdr->e_shentsize;
> +	sHdr = elfPointer<SecHeader>(map, offset, soSize);
> +	if (!sHdr)
> +		return -ENOEXEC;
> +	offset = sHdr->sh_offset + (targetSymbol->st_value - sHdr->sh_addr);
> +	char *data = elfPointer<char>(map, offset, soSize);

And a char[size] here.

> +	if (!data)
> +		return -ENOEXEC;
> +	/* \todo move out memcpy? */

YOu know my opinion on this :-)

> +	memcpy(dst, data, size);
> +
> +	return 0;
> +}
> +
> +} /* namespace */

namespace libcamera ?

> +
> +/**
> + * \struct IPAModuleInfo
> + * \brief Information of an IPA plugin

Do we standardise on "IPA plugin" to mean "IPA module shared object" ?
If so you should replace "The IPA plugin shared object" with "The IPA
plugin" below. Or should we drop the word plugin and use module through
the whole documentation instead, to avoid having two different terms ?
I'm tempted to go for the latter, with a global s/plugin/module/.

> + *
> + * This structure contains the information of an IPA plugin. It is loaded,
> + * read, and validated before anything else is loaded from the plugin.
> + *
> + * \var IPAModuleInfo::name
> + * \brief The name of the IPA plugin
> + *
> + * \var IPAModuleInfo::version
> + * \brief The version of the IPA plugin
> + *
> + * \todo abi compatability version
> + * \todo pipeline compatability matcher
> + */
> +
> +/**
> + * \class IPAModule
> + * \brief Wrapper around IPA module shared object
> + */
> +
> +/**
> + * \brief Construct an IPAModule instance
> + * \param[in] libPath path to IPA plugin
> + *
> + * Loads the IPAModuleInfo from the IPA plugin at libPath.
> + * The IPA plugin shared object file must be of the same endianness and
> + * bitness as libcamera.
> + *
> + * \todo also load funtions and whatever from the IPA to be used by pipelines

Very formal :-) We'll fix this soon anyway.

> + *
> + * Note that isValid() should be called to check if this loading was successful
> + * or not.

Maybe replace the last sentence with

 * The caller shall call the isValid() method after constructing an
 * IPAModule instance to verify the validity of the IPAModule.

to make it a bit more formal ?

> + */
> +IPAModule::IPAModule(const std::string &libPath)
> +	: libPath_(libPath), valid_(false)
> +{
> +	int ret = loadIPAModuleInfo("ipaModuleInfo");
> +	if (ret < 0)
> +		return;

Maybe

	if (loadIPAModuleInfo("ipaModuleInfo") < 0)
		return;

?

> +
> +	valid_ = true;
> +}
> +
> +int IPAModule::loadIPAModuleInfo(const char *symbol)

Don't pass the symbol name to this function, you can hardcode it
internally.

> +{
> +	int fd = open(libPath_.c_str(), O_RDONLY);
> +	if (fd < 0) {
> +		int ret = -errno;
> +		LOG(IPAModule, Error) << "Failed to open IPA library: "
> +				      << strerror(-ret);
> +		return ret;
> +	}
> +
> +	size_t soSize;
> +	void *map;
> +	struct stat st;
> +	int ret = fstat(fd, &st);
> +	if (ret < 0)
> +		goto close;
> +	soSize = st.st_size;
> +	map = mmap(NULL, soSize, PROT_READ, MAP_PRIVATE, fd, 0);
> +	if (map == MAP_FAILED) {
> +		ret = -errno;
> +		goto close;
> +	}
> +
> +	ret = elfVerifyIdent(map, soSize);
> +	if (ret)
> +		goto unmap;
> +
> +	if (sizeof(unsigned long) == 4)
> +		ret = elfLoadSymbol<Elf32_Ehdr, Elf32_Shdr, Elf32_Sym>
> +				   (&info_, sizeof(info_), map, soSize, symbol);
> +	else
> +		ret = elfLoadSymbol<Elf64_Ehdr, Elf64_Shdr, Elf64_Sym>
> +				   (&info_, sizeof(info_), map, soSize, symbol);
> +	if (ret)
> +		goto unmap;

You can remove those two lines.

> +
> +unmap:
> +	munmap(map, soSize);
> +close:
> +	close(fd);
> +	return ret;
> +}
> +
> +/**
> + * \brief Check if the IPAModule instance is valid
> + *
> + * An IPAModule instance is valid if the IPA Module shared object was
> + * successfully loaded, and the ipaModuleInfo in it was read.

As we will add a load() method that dlopen()s the .so later, I would
write this as

An IPAModuleinstance is valid if the IPA module shared object exists and
the IPA module information it contains was successfully retrieved and
validated.

> + *
> + * \return true if the the IPAModule is  valid, false otherwise

s/is  valid/is valid/

> + */
> +bool IPAModule::isValid() const
> +{
> +	return valid_;
> +}
> +
> +/**
> + * \brief Get the IPA module information

We use Retrieve instead of Get through the livrary.

> + *
> + * The content of the IPA module information is loaded from the module,
> + * and is valid only if the module is valid (as returned by isValid()).
> + * Calling this function on an invalid module is an error.
> + *
> + * \return IPAModuleInfo

\return The IPA module information

> + */
> +const struct IPAModuleInfo &IPAModule::info() const
> +{
> +	return info_;
> +}
> +
> +} /* namespace libcamera */
> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
> index 8796f49..e5b48f2 100644
> --- a/src/libcamera/meson.build
> +++ b/src/libcamera/meson.build
> @@ -10,6 +10,7 @@ libcamera_sources = files([
>      'event_notifier.cpp',
>      'formats.cpp',
>      'geometry.cpp',
> +    'ipa_module.cpp',
>      'log.cpp',
>      'media_device.cpp',
>      'media_object.cpp',
> @@ -31,6 +32,7 @@ libcamera_headers = files([
>      'include/device_enumerator_udev.h',
>      'include/event_dispatcher_poll.h',
>      'include/formats.h',
> +    'include/ipa_module.h',
>      'include/log.h',
>      'include/media_device.h',
>      'include/media_object.h',