From dd53d04d8c19515223f86129ca043614e196840e Mon Sep 17 00:00:00 2001 From: Michael Butler Date: Wed, 24 Mar 2021 21:26:02 -0700 Subject: [PATCH 1/2] Improve the structure of NNAPI AIDL Memory Prior to this change, the NN AIDL HAL created a Memory struct analogous to hidl_memory, consisting of a name, size, and native handle. However, this Memory struct is not very structured, and requires both the client and server to agree on how the data should be interpreted. This CL tightens the structure of the Memory representation by introducing Ashmem and MappableFile structs to android.hardware.common in order to hold specific file descriptors representing memory regions. Further, this CL redefines android.hardwre.neuralnetworks's Memory object as a union of the Ashmem, MappableFile, and (existing) HardwareBuffer memory types. This change also adds "com.android.neuralnetworks" to the graphics AIDL HAL's apex_available build rule. Bug: 183118727 Test: mma Change-Id: I32322df676b83597c9e95f13662c322a6a80accc Merged-In: I32322df676b83597c9e95f13662c322a6a80accc (cherry picked from commit 1158c80ff6f24223b8add271945b66f34db78d60) --- .../android/hardware/common/Ashmem.aidl | 39 ++++++++++++++ .../android/hardware/common/MappableFile.aidl | 41 ++++++++++++++ .../android/hardware/common/NativeHandle.aidl | 28 +++++++--- .../aidl/android/hardware/common/Ashmem.aidl | 34 ++++++++++++ .../android/hardware/common/MappableFile.aidl | 53 +++++++++++++++++++ graphics/common/aidl/Android.bp | 1 + neuralnetworks/aidl/Android.bp | 1 + .../hardware/neuralnetworks/Memory.aidl | 8 +-- .../hardware/neuralnetworks/Memory.aidl | 26 ++++++--- 9 files changed, 213 insertions(+), 18 deletions(-) create mode 100644 common/aidl/aidl_api/android.hardware.common/current/android/hardware/common/Ashmem.aidl create mode 100644 common/aidl/aidl_api/android.hardware.common/current/android/hardware/common/MappableFile.aidl create mode 100644 common/aidl/android/hardware/common/Ashmem.aidl create mode 100644 common/aidl/android/hardware/common/MappableFile.aidl diff --git a/common/aidl/aidl_api/android.hardware.common/current/android/hardware/common/Ashmem.aidl b/common/aidl/aidl_api/android.hardware.common/current/android/hardware/common/Ashmem.aidl new file mode 100644 index 0000000000..a4380315c9 --- /dev/null +++ b/common/aidl/aidl_api/android.hardware.common/current/android/hardware/common/Ashmem.aidl @@ -0,0 +1,39 @@ +/* + * Copyright 2021 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +/////////////////////////////////////////////////////////////////////////////// +// THIS FILE IS IMMUTABLE. DO NOT EDIT IN ANY CASE. // +/////////////////////////////////////////////////////////////////////////////// + +// This file is a snapshot of an AIDL file. Do not edit it manually. There are +// two cases: +// 1). this is a frozen version file - do not edit this in any case. +// 2). this is a 'current' file. If you make a backwards compatible change to +// the interface (from the latest frozen version), the build system will +// prompt you to update this file with `m -update-api`. +// +// You must not make a backward incompatible change to any AIDL file built +// with the aidl_interface module type with versions property set. The module +// type is used to build AIDL files in a way that they can be used across +// independently updatable components of the system. If a device is shipped +// with such a backward incompatible change, it has a high risk of breaking +// later when a module using the interface is updated, e.g., Mainline modules. + +package android.hardware.common; +@VintfStability +parcelable Ashmem { + ParcelFileDescriptor fd; + long size; +} diff --git a/common/aidl/aidl_api/android.hardware.common/current/android/hardware/common/MappableFile.aidl b/common/aidl/aidl_api/android.hardware.common/current/android/hardware/common/MappableFile.aidl new file mode 100644 index 0000000000..394ea8ff07 --- /dev/null +++ b/common/aidl/aidl_api/android.hardware.common/current/android/hardware/common/MappableFile.aidl @@ -0,0 +1,41 @@ +/* + * Copyright 2021 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +/////////////////////////////////////////////////////////////////////////////// +// THIS FILE IS IMMUTABLE. DO NOT EDIT IN ANY CASE. // +/////////////////////////////////////////////////////////////////////////////// + +// This file is a snapshot of an AIDL file. Do not edit it manually. There are +// two cases: +// 1). this is a frozen version file - do not edit this in any case. +// 2). this is a 'current' file. If you make a backwards compatible change to +// the interface (from the latest frozen version), the build system will +// prompt you to update this file with `m -update-api`. +// +// You must not make a backward incompatible change to any AIDL file built +// with the aidl_interface module type with versions property set. The module +// type is used to build AIDL files in a way that they can be used across +// independently updatable components of the system. If a device is shipped +// with such a backward incompatible change, it has a high risk of breaking +// later when a module using the interface is updated, e.g., Mainline modules. + +package android.hardware.common; +@VintfStability +parcelable MappableFile { + long length; + int prot; + ParcelFileDescriptor fd; + long offset; +} diff --git a/common/aidl/aidl_api/android.hardware.common/current/android/hardware/common/NativeHandle.aidl b/common/aidl/aidl_api/android.hardware.common/current/android/hardware/common/NativeHandle.aidl index f37b7d506f..2ed5c0b22d 100644 --- a/common/aidl/aidl_api/android.hardware.common/current/android/hardware/common/NativeHandle.aidl +++ b/common/aidl/aidl_api/android.hardware.common/current/android/hardware/common/NativeHandle.aidl @@ -1,14 +1,30 @@ +/* + * Copyright 2019 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ /////////////////////////////////////////////////////////////////////////////// // THIS FILE IS IMMUTABLE. DO NOT EDIT IN ANY CASE. // /////////////////////////////////////////////////////////////////////////////// -// This file is a snapshot of an AIDL interface (or parcelable). Do not try to -// edit this file. It looks like you are doing that because you have modified -// an AIDL interface in a backward-incompatible way, e.g., deleting a function -// from an interface or a field from a parcelable and it broke the build. That -// breakage is intended. +// This file is a snapshot of an AIDL file. Do not edit it manually. There are +// two cases: +// 1). this is a frozen version file - do not edit this in any case. +// 2). this is a 'current' file. If you make a backwards compatible change to +// the interface (from the latest frozen version), the build system will +// prompt you to update this file with `m -update-api`. // -// You must not make a backward incompatible changes to the AIDL files built +// You must not make a backward incompatible change to any AIDL file built // with the aidl_interface module type with versions property set. The module // type is used to build AIDL files in a way that they can be used across // independently updatable components of the system. If a device is shipped diff --git a/common/aidl/android/hardware/common/Ashmem.aidl b/common/aidl/android/hardware/common/Ashmem.aidl new file mode 100644 index 0000000000..8e402668bf --- /dev/null +++ b/common/aidl/android/hardware/common/Ashmem.aidl @@ -0,0 +1,34 @@ +/* + * Copyright 2021 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package android.hardware.common; + +import android.os.ParcelFileDescriptor; + +/** + * Type that holds same memory as the "ashmem" hidl_memory type from HIDL. + */ +@VintfStability +parcelable Ashmem { + /** + * A handle to a memory region. + */ + ParcelFileDescriptor fd; + /** + * Size of the memory region in bytes. + */ + long size; +} diff --git a/common/aidl/android/hardware/common/MappableFile.aidl b/common/aidl/android/hardware/common/MappableFile.aidl new file mode 100644 index 0000000000..a7763eab37 --- /dev/null +++ b/common/aidl/android/hardware/common/MappableFile.aidl @@ -0,0 +1,53 @@ +/* + * Copyright 2021 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package android.hardware.common; + +import android.os.ParcelFileDescriptor; + +/** + * A region of a file that can be mapped into memory. + * + * In Linux, MappableFile may be used with mmap as `MAP_SHARED`. + * + * MappableFile is compatible with ::android::base::MappedFile. + */ +@VintfStability +parcelable MappableFile { + /** + * Length of the mapping region in bytes. + */ + long length; + /** + * The desired memory protection for the mapping. + * + * In Linux, prot is either `PROT_NONE` (indicating that mapped pages may not be accessed) or + * the bitwise OR of one or more of the following flags: + * - `PROT_READ` (indicating that the mapped pages may be read) + * - `PROT_WRITE` (indicating that the mapped pages may be written) + */ + int prot; + /** + * A handle to a mappable file. + */ + ParcelFileDescriptor fd; + /** + * The offset in the file to the beginning of the mapping region in number of bytes. + * + * Note: Some mapping functions require that the offset is aligned to the page size. + */ + long offset; +} diff --git a/graphics/common/aidl/Android.bp b/graphics/common/aidl/Android.bp index 2a46f9dc7f..cadd13cdde 100644 --- a/graphics/common/aidl/Android.bp +++ b/graphics/common/aidl/Android.bp @@ -34,6 +34,7 @@ aidl_interface { apex_available: [ "//apex_available:platform", "com.android.media.swcodec", + "com.android.neuralnetworks", ], min_sdk_version: "29", }, diff --git a/neuralnetworks/aidl/Android.bp b/neuralnetworks/aidl/Android.bp index b1860e2bd0..ebf4654885 100644 --- a/neuralnetworks/aidl/Android.bp +++ b/neuralnetworks/aidl/Android.bp @@ -16,6 +16,7 @@ aidl_interface { stability: "vintf", imports: [ "android.hardware.common", + "android.hardware.graphics.common", ], backend: { java: { diff --git a/neuralnetworks/aidl/aidl_api/android.hardware.neuralnetworks/current/android/hardware/neuralnetworks/Memory.aidl b/neuralnetworks/aidl/aidl_api/android.hardware.neuralnetworks/current/android/hardware/neuralnetworks/Memory.aidl index 8207b25570..37fa102cf4 100644 --- a/neuralnetworks/aidl/aidl_api/android.hardware.neuralnetworks/current/android/hardware/neuralnetworks/Memory.aidl +++ b/neuralnetworks/aidl/aidl_api/android.hardware.neuralnetworks/current/android/hardware/neuralnetworks/Memory.aidl @@ -33,8 +33,8 @@ package android.hardware.neuralnetworks; @VintfStability -parcelable Memory { - android.hardware.common.NativeHandle handle; - long size; - String name; +union Memory { + android.hardware.common.Ashmem ashmem; + android.hardware.common.MappableFile mappableFile; + android.hardware.graphics.common.HardwareBuffer hardwareBuffer; } diff --git a/neuralnetworks/aidl/android/hardware/neuralnetworks/Memory.aidl b/neuralnetworks/aidl/android/hardware/neuralnetworks/Memory.aidl index 870f0aeb08..244ac8752d 100644 --- a/neuralnetworks/aidl/android/hardware/neuralnetworks/Memory.aidl +++ b/neuralnetworks/aidl/android/hardware/neuralnetworks/Memory.aidl @@ -15,16 +15,26 @@ */ package android.hardware.neuralnetworks; -import android.hardware.common.NativeHandle; -import android.os.ParcelFileDescriptor; + +import android.hardware.common.Ashmem; +import android.hardware.common.MappableFile; +import android.hardware.graphics.common.HardwareBuffer; /** - * A type that is used to pass pieces of shared memory between processes. - * The type structure mimics hidl_memory type from HIDL. + * The different types of memory that can be shared across processes. */ @VintfStability -parcelable Memory { - NativeHandle handle; - long size; - String name; +union Memory { + /** + * Ashmem hidl_memory type from HIDL. + */ + Ashmem ashmem; + /** + * File that can be mapped. + */ + MappableFile mappableFile; + /** + * AIDL representation of AHardwareBuffer. + */ + HardwareBuffer hardwareBuffer; } From f03ebd93b9a547538b43014a9be6cbb983676c91 Mon Sep 17 00:00:00 2001 From: Michael Butler Date: Thu, 25 Mar 2021 15:27:38 -0700 Subject: [PATCH 2/2] Update NN utility code and VTS tests with new Memory type This CL fixes the compiler errors that arose of changing the Memory representation of the NN AIDL HAL, and updates the conversion and utility code to work with the new Memory type. This change also makes libaidlcommonsupport available to apex modules at min sdk level 29. Bug: 183118727 Test: mma Test: VtsHalNeuralnetworksTargetTest Change-Id: Ief565473b4d82e0bb43785fc3b8275b16bd26cf6 Merged-In: Ief565473b4d82e0bb43785fc3b8275b16bd26cf6 (cherry picked from commit b0fcb3927d848e9721f05a458b5d6d4d2cb8079d) --- common/support/Android.bp | 5 + neuralnetworks/aidl/utils/Android.bp | 4 + neuralnetworks/aidl/utils/src/Conversions.cpp | 275 ++++++++++-------- neuralnetworks/aidl/utils/src/Utils.cpp | 62 +++- neuralnetworks/aidl/vts/functional/Android.bp | 2 + .../aidl/vts/functional/MemoryDomainTests.cpp | 21 +- .../aidl/vts/functional/ValidateModel.cpp | 16 +- .../utils/common/src/CommonUtils.cpp | 128 ++++++-- 8 files changed, 344 insertions(+), 169 deletions(-) diff --git a/common/support/Android.bp b/common/support/Android.bp index 8aea306dae..730798d840 100644 --- a/common/support/Android.bp +++ b/common/support/Android.bp @@ -18,6 +18,11 @@ cc_library_static { "android.hardware.common-V2-ndk_platform", "libcutils", ], + apex_available: [ + "//apex_available:platform", + "com.android.neuralnetworks", + ], + min_sdk_version: "29", } cc_test { diff --git a/neuralnetworks/aidl/utils/Android.bp b/neuralnetworks/aidl/utils/Android.bp index ad961cfe99..0ccc711ecf 100644 --- a/neuralnetworks/aidl/utils/Android.bp +++ b/neuralnetworks/aidl/utils/Android.bp @@ -31,6 +31,8 @@ cc_library_static { export_include_dirs: ["include"], cflags: ["-Wthread-safety"], static_libs: [ + "android.hardware.graphics.common-V2-ndk_platform", + "libaidlcommonsupport", "libarect", "neuralnetworks_types", "neuralnetworks_utils_hal_common", @@ -51,7 +53,9 @@ cc_test { ], static_libs: [ "android.hardware.common-V2-ndk_platform", + "android.hardware.graphics.common-V2-ndk_platform", "android.hardware.neuralnetworks-V1-ndk_platform", + "libaidlcommonsupport", "libgmock", "libneuralnetworks_common", "neuralnetworks_types", diff --git a/neuralnetworks/aidl/utils/src/Conversions.cpp b/neuralnetworks/aidl/utils/src/Conversions.cpp index d5f7f81663..93ac51c233 100644 --- a/neuralnetworks/aidl/utils/src/Conversions.cpp +++ b/neuralnetworks/aidl/utils/src/Conversions.cpp @@ -16,8 +16,13 @@ #include "Conversions.h" +#include +#include #include +#include +#include #include +#include #include #include #include @@ -125,28 +130,17 @@ struct NativeHandleDeleter { using UniqueNativeHandle = std::unique_ptr; -static GeneralResult nativeHandleFromAidlHandle(const NativeHandle& handle) { - std::vector fds; - fds.reserve(handle.fds.size()); - for (const auto& fd : handle.fds) { - auto duplicatedFd = NN_TRY(dupFd(fd.get())); - fds.emplace_back(duplicatedFd.release()); +GeneralResult nativeHandleFromAidlHandle(const NativeHandle& handle) { + auto nativeHandle = UniqueNativeHandle(dupFromAidl(handle)); + if (nativeHandle.get() == nullptr) { + return NN_ERROR() << "android::dupFromAidl failed to convert the common::NativeHandle to a " + "native_handle_t"; } - - constexpr size_t kIntMax = std::numeric_limits::max(); - CHECK_LE(handle.fds.size(), kIntMax); - CHECK_LE(handle.ints.size(), kIntMax); - native_handle_t* nativeHandle = native_handle_create(static_cast(handle.fds.size()), - static_cast(handle.ints.size())); - if (nativeHandle == nullptr) { - return NN_ERROR() << "Failed to create native_handle"; + if (!std::all_of(nativeHandle->data + 0, nativeHandle->data + nativeHandle->numFds, + [](int fd) { return fd >= 0; })) { + return NN_ERROR() << "android::dupFromAidl returned an invalid native_handle_t"; } - for (size_t i = 0; i < fds.size(); ++i) { - nativeHandle->data[i] = fds[i].release(); - } - std::copy(handle.ints.begin(), handle.ints.end(), &nativeHandle->data[nativeHandle->numFds]); - - return UniqueNativeHandle(nativeHandle); + return nativeHandle; } } // anonymous namespace @@ -353,67 +347,66 @@ GeneralResult unvalidatedConvert(bool measureTiming) { return measureTiming ? MeasureTiming::YES : MeasureTiming::NO; } -static uint32_t roundUpToMultiple(uint32_t value, uint32_t multiple) { - return (value + multiple - 1) / multiple * multiple; -} - GeneralResult unvalidatedConvert(const aidl_hal::Memory& memory) { - VERIFY_NON_NEGATIVE(memory.size) << "Memory size must not be negative"; - if (memory.size > std::numeric_limits::max()) { - return NN_ERROR() << "Memory: size must be <= std::numeric_limits::max()"; - } + using Tag = aidl_hal::Memory::Tag; + switch (memory.getTag()) { + case Tag::ashmem: { + const auto& ashmem = memory.get(); + VERIFY_NON_NEGATIVE(ashmem.size) << "Memory size must not be negative"; + if (ashmem.size > std::numeric_limits::max()) { + return NN_ERROR() << "Memory: size must be <= std::numeric_limits::max()"; + } - if (memory.name != "hardware_buffer_blob") { - return std::make_shared(Memory{ - .handle = NN_TRY(unvalidatedConvertHelper(memory.handle)), - .size = static_cast(memory.size), - .name = memory.name, - }); - } + auto handle = Memory::Ashmem{ + .fd = NN_TRY(dupFd(ashmem.fd.get())), + .size = static_cast(ashmem.size), + }; + return std::make_shared(Memory{.handle = std::move(handle)}); + } + case Tag::mappableFile: { + const auto& mappableFile = memory.get(); + VERIFY_NON_NEGATIVE(mappableFile.length) << "Memory size must not be negative"; + VERIFY_NON_NEGATIVE(mappableFile.offset) << "Memory offset must not be negative"; + if (mappableFile.length > std::numeric_limits::max()) { + return NN_ERROR() << "Memory: size must be <= std::numeric_limits::max()"; + } + if (mappableFile.offset > std::numeric_limits::max()) { + return NN_ERROR() << "Memory: offset must be <= std::numeric_limits::max()"; + } - const auto size = static_cast(memory.size); - const auto format = AHARDWAREBUFFER_FORMAT_BLOB; - const auto usage = AHARDWAREBUFFER_USAGE_CPU_READ_OFTEN | AHARDWAREBUFFER_USAGE_CPU_WRITE_OFTEN; - const uint32_t width = size; - const uint32_t height = 1; // height is always 1 for BLOB mode AHardwareBuffer. - const uint32_t layers = 1; // layers is always 1 for BLOB mode AHardwareBuffer. + const size_t size = static_cast(mappableFile.length); + const int prot = mappableFile.prot; + const int fd = mappableFile.fd.get(); + const size_t offset = static_cast(mappableFile.offset); - const UniqueNativeHandle handle = NN_TRY(nativeHandleFromAidlHandle(memory.handle)); - const native_handle_t* nativeHandle = handle.get(); + return createSharedMemoryFromFd(size, prot, fd, offset); + } + case Tag::hardwareBuffer: { + const auto& hardwareBuffer = memory.get(); - // AHardwareBuffer_createFromHandle() might fail because an allocator - // expects a specific stride value. In that case, we try to guess it by - // aligning the width to small powers of 2. - // TODO(b/174120849): Avoid stride assumptions. - AHardwareBuffer* hardwareBuffer = nullptr; - status_t status = UNKNOWN_ERROR; - for (uint32_t alignment : {1, 4, 32, 64, 128, 2, 8, 16}) { - const uint32_t stride = roundUpToMultiple(width, alignment); - AHardwareBuffer_Desc desc{ - .width = width, - .height = height, - .layers = layers, - .format = format, - .usage = usage, - .stride = stride, - }; - status = AHardwareBuffer_createFromHandle(&desc, nativeHandle, - AHARDWAREBUFFER_CREATE_FROM_HANDLE_METHOD_CLONE, - &hardwareBuffer); - if (status == NO_ERROR) { - break; + const UniqueNativeHandle handle = + NN_TRY(nativeHandleFromAidlHandle(hardwareBuffer.handle)); + const native_handle_t* nativeHandle = handle.get(); + + const AHardwareBuffer_Desc desc{ + .width = static_cast(hardwareBuffer.description.width), + .height = static_cast(hardwareBuffer.description.height), + .layers = static_cast(hardwareBuffer.description.layers), + .format = static_cast(hardwareBuffer.description.format), + .usage = static_cast(hardwareBuffer.description.usage), + .stride = static_cast(hardwareBuffer.description.stride), + }; + AHardwareBuffer* ahwb = nullptr; + const status_t status = AHardwareBuffer_createFromHandle( + &desc, nativeHandle, AHARDWAREBUFFER_CREATE_FROM_HANDLE_METHOD_CLONE, &ahwb); + if (status != NO_ERROR) { + return NN_ERROR() << "createFromHandle failed"; + } + + return createSharedMemoryFromAHWB(ahwb, /*takeOwnership=*/true); } } - if (status != NO_ERROR) { - return NN_ERROR(ErrorStatus::GENERAL_FAILURE) - << "Can't create AHardwareBuffer from handle. Error: " << status; - } - - return std::make_shared(Memory{ - .handle = HardwareBufferHandle(hardwareBuffer, /*takeOwnership=*/true), - .size = static_cast(memory.size), - .name = memory.name, - }); + return NN_ERROR() << "Unrecognized Memory::Tag: " << memory.getTag(); } GeneralResult unvalidatedConvert(const aidl_hal::Timing& timing) { @@ -645,20 +638,95 @@ struct overloaded : Ts... { template overloaded(Ts...)->overloaded; -static nn::GeneralResult aidlHandleFromNativeHandle( - const native_handle_t& handle) { - common::NativeHandle aidlNativeHandle; +nn::GeneralResult aidlHandleFromNativeHandle( + const native_handle_t& nativeHandle) { + auto handle = ::android::dupToAidl(&nativeHandle); + if (!std::all_of(handle.fds.begin(), handle.fds.end(), + [](const ndk::ScopedFileDescriptor& fd) { return fd.get() >= 0; })) { + return NN_ERROR() << "android::dupToAidl returned an invalid common::NativeHandle"; + } + return handle; +} - aidlNativeHandle.fds.reserve(handle.numFds); - for (int i = 0; i < handle.numFds; ++i) { - auto duplicatedFd = NN_TRY(nn::dupFd(handle.data[i])); - aidlNativeHandle.fds.emplace_back(duplicatedFd.release()); +nn::GeneralResult unvalidatedConvert(const nn::Memory::Ashmem& memory) { + if constexpr (std::numeric_limits::max() > std::numeric_limits::max()) { + if (memory.size > std::numeric_limits::max()) { + return ( + NN_ERROR() + << "Memory::Ashmem: size must be <= std::numeric_limits::max()") + . + operator nn::GeneralResult(); + } } - aidlNativeHandle.ints = std::vector(&handle.data[handle.numFds], - &handle.data[handle.numFds + handle.numInts]); + auto fd = NN_TRY(nn::dupFd(memory.fd)); + auto handle = common::Ashmem{ + .fd = ndk::ScopedFileDescriptor(fd.release()), + .size = static_cast(memory.size), + }; + return Memory::make(std::move(handle)); +} - return aidlNativeHandle; +nn::GeneralResult unvalidatedConvert(const nn::Memory::Fd& memory) { + if constexpr (std::numeric_limits::max() > std::numeric_limits::max()) { + if (memory.size > std::numeric_limits::max()) { + return (NN_ERROR() << "Memory::Fd: size must be <= std::numeric_limits::max()") + . + operator nn::GeneralResult(); + } + if (memory.offset > std::numeric_limits::max()) { + return ( + NN_ERROR() + << "Memory::Fd: offset must be <= std::numeric_limits::max()") + . + operator nn::GeneralResult(); + } + } + + auto fd = NN_TRY(nn::dupFd(memory.fd)); + auto handle = common::MappableFile{ + .length = static_cast(memory.size), + .prot = memory.prot, + .fd = ndk::ScopedFileDescriptor(fd.release()), + .offset = static_cast(memory.offset), + }; + return Memory::make(std::move(handle)); +} + +nn::GeneralResult unvalidatedConvert(const nn::Memory::HardwareBuffer& memory) { + const native_handle_t* nativeHandle = AHardwareBuffer_getNativeHandle(memory.handle.get()); + if (nativeHandle == nullptr) { + return (NN_ERROR() << "unvalidatedConvert failed because AHardwareBuffer_getNativeHandle " + "returned nullptr") + . + operator nn::GeneralResult(); + } + + auto handle = NN_TRY(aidlHandleFromNativeHandle(*nativeHandle)); + + AHardwareBuffer_Desc desc; + AHardwareBuffer_describe(memory.handle.get(), &desc); + + const auto description = graphics::common::HardwareBufferDescription{ + .width = static_cast(desc.width), + .height = static_cast(desc.height), + .layers = static_cast(desc.layers), + .format = static_cast(desc.format), + .usage = static_cast(desc.usage), + .stride = static_cast(desc.stride), + }; + + auto hardwareBuffer = graphics::common::HardwareBuffer{ + .description = std::move(description), + .handle = std::move(handle), + }; + return Memory::make(std::move(hardwareBuffer)); +} + +nn::GeneralResult unvalidatedConvert(const nn::Memory::Unknown& /*memory*/) { + return (NN_ERROR() << "Unable to convert Unknown memory type") + . + operator nn::GeneralResult(); } } // namespace @@ -693,41 +761,12 @@ nn::GeneralResult unvalidatedConvert(const nn::SharedHandl } nn::GeneralResult unvalidatedConvert(const nn::SharedMemory& memory) { - CHECK(memory != nullptr); - if (memory->size > std::numeric_limits::max()) { - return NN_ERROR() << "Memory size doesn't fit into int64_t."; + if (memory == nullptr) { + return (NN_ERROR() << "Unable to convert nullptr memory") + . + operator nn::GeneralResult(); } - if (const auto* handle = std::get_if(&memory->handle)) { - return Memory{ - .handle = NN_TRY(unvalidatedConvert(*handle)), - .size = static_cast(memory->size), - .name = memory->name, - }; - } - - const auto* ahwb = std::get(memory->handle).get(); - AHardwareBuffer_Desc bufferDesc; - AHardwareBuffer_describe(ahwb, &bufferDesc); - - if (bufferDesc.format == AHARDWAREBUFFER_FORMAT_BLOB) { - CHECK_EQ(memory->size, bufferDesc.width); - CHECK_EQ(memory->name, "hardware_buffer_blob"); - } else { - CHECK_EQ(memory->size, 0u); - CHECK_EQ(memory->name, "hardware_buffer"); - } - - const native_handle_t* nativeHandle = AHardwareBuffer_getNativeHandle(ahwb); - if (nativeHandle == nullptr) { - return NN_ERROR() << "unvalidatedConvert failed because AHardwareBuffer_getNativeHandle " - "returned nullptr"; - } - - return Memory{ - .handle = NN_TRY(aidlHandleFromNativeHandle(*nativeHandle)), - .size = static_cast(memory->size), - .name = memory->name, - }; + return std::visit([](const auto& x) { return unvalidatedConvert(x); }, memory->handle); } nn::GeneralResult unvalidatedConvert(const nn::ErrorStatus& errorStatus) { diff --git a/neuralnetworks/aidl/utils/src/Utils.cpp b/neuralnetworks/aidl/utils/src/Utils.cpp index 95516c854b..03407be4ce 100644 --- a/neuralnetworks/aidl/utils/src/Utils.cpp +++ b/neuralnetworks/aidl/utils/src/Utils.cpp @@ -16,12 +16,20 @@ #include "Utils.h" +#include +#include +#include +#include #include #include +#include namespace aidl::android::hardware::neuralnetworks::utils { namespace { +nn::GeneralResult clone(const ndk::ScopedFileDescriptor& fd); +using utils::clone; + template nn::GeneralResult> cloneVec(const std::vector& arguments) { std::vector clonedObjects; @@ -37,24 +45,52 @@ nn::GeneralResult> clone(const std::vector& arguments) { return cloneVec(arguments); } +nn::GeneralResult clone(const ndk::ScopedFileDescriptor& fd) { + auto duplicatedFd = NN_TRY(nn::dupFd(fd.get())); + return ndk::ScopedFileDescriptor(duplicatedFd.release()); +} + +nn::GeneralResult clone(const common::NativeHandle& handle) { + return common::NativeHandle{ + .fds = NN_TRY(cloneVec(handle.fds)), + .ints = handle.ints, + }; +} + } // namespace nn::GeneralResult clone(const Memory& memory) { - common::NativeHandle nativeHandle; - nativeHandle.ints = memory.handle.ints; - nativeHandle.fds.reserve(memory.handle.fds.size()); - for (const auto& fd : memory.handle.fds) { - const int newFd = dup(fd.get()); - if (newFd < 0) { - return NN_ERROR() << "Couldn't dup a file descriptor"; + switch (memory.getTag()) { + case Memory::Tag::ashmem: { + const auto& ashmem = memory.get(); + auto handle = common::Ashmem{ + .fd = NN_TRY(clone(ashmem.fd)), + .size = ashmem.size, + }; + return Memory::make(std::move(handle)); + } + case Memory::Tag::mappableFile: { + const auto& memFd = memory.get(); + auto handle = common::MappableFile{ + .length = memFd.length, + .prot = memFd.prot, + .fd = NN_TRY(clone(memFd.fd)), + .offset = memFd.offset, + }; + return Memory::make(std::move(handle)); + } + case Memory::Tag::hardwareBuffer: { + const auto& hardwareBuffer = memory.get(); + auto handle = graphics::common::HardwareBuffer{ + .description = hardwareBuffer.description, + .handle = NN_TRY(clone(hardwareBuffer.handle)), + }; + return Memory::make(std::move(handle)); } - nativeHandle.fds.emplace_back(newFd); } - return Memory{ - .handle = std::move(nativeHandle), - .size = memory.size, - .name = memory.name, - }; + return (NN_ERROR() << "Unrecognized Memory::Tag: " << memory.getTag()) + . + operator nn::GeneralResult(); } nn::GeneralResult clone(const RequestMemoryPool& requestPool) { diff --git a/neuralnetworks/aidl/vts/functional/Android.bp b/neuralnetworks/aidl/vts/functional/Android.bp index 7804c2a765..d5b150a934 100644 --- a/neuralnetworks/aidl/vts/functional/Android.bp +++ b/neuralnetworks/aidl/vts/functional/Android.bp @@ -50,9 +50,11 @@ cc_test { ], static_libs: [ "android.hardware.common-V2-ndk_platform", + "android.hardware.graphics.common-V2-ndk_platform", "android.hardware.neuralnetworks-V1-ndk_platform", "android.hidl.allocator@1.0", "android.hidl.memory@1.0", + "libaidlcommonsupport", "libgmock", "libhidlmemory", "libneuralnetworks_generated_test_harness", diff --git a/neuralnetworks/aidl/vts/functional/MemoryDomainTests.cpp b/neuralnetworks/aidl/vts/functional/MemoryDomainTests.cpp index 596f8ae58e..e8313f19eb 100644 --- a/neuralnetworks/aidl/vts/functional/MemoryDomainTests.cpp +++ b/neuralnetworks/aidl/vts/functional/MemoryDomainTests.cpp @@ -16,6 +16,7 @@ #define LOG_TAG "neuralnetworks_aidl_hal_test" +#include #include #include #include @@ -659,10 +660,26 @@ class MemoryDomainCopyTestBase : public MemoryDomainTestBase { return allocateBuffer(preparedModel, inputIndexes, outputIndexes, {}); } + size_t getSize(const Memory& memory) { + switch (memory.getTag()) { + case Memory::Tag::ashmem: + return memory.get().size; + case Memory::Tag::mappableFile: + return memory.get().length; + case Memory::Tag::hardwareBuffer: { + const auto& hardwareBuffer = memory.get(); + const bool isBlob = + hardwareBuffer.description.format == graphics::common::PixelFormat::BLOB; + return isBlob ? hardwareBuffer.description.width : 0; + } + } + return 0; + } + Memory allocateSharedMemory(uint32_t size) { const auto sharedMemory = nn::createSharedMemory(size).value(); auto memory = utils::convert(sharedMemory).value(); - EXPECT_EQ(memory.size, size); + EXPECT_EQ(getSize(memory), size); return memory; } @@ -690,7 +707,7 @@ class MemoryDomainCopyTestBase : public MemoryDomainTestBase { void initializeDeviceMemory(const std::shared_ptr& buffer) { Memory memory = allocateSharedMemory(kTestOperandDataSize); - ASSERT_EQ(memory.size, kTestOperandDataSize); + ASSERT_EQ(getSize(memory), kTestOperandDataSize); testCopyFrom(buffer, memory, utils::toSigned(kTestOperand.dimensions).value(), ErrorStatus::NONE); } diff --git a/neuralnetworks/aidl/vts/functional/ValidateModel.cpp b/neuralnetworks/aidl/vts/functional/ValidateModel.cpp index 94d3daf6bb..698c054941 100644 --- a/neuralnetworks/aidl/vts/functional/ValidateModel.cpp +++ b/neuralnetworks/aidl/vts/functional/ValidateModel.cpp @@ -259,12 +259,16 @@ template <> size_t sizeForBinder(const Memory& memory) { // This is just a guess. - size_t size = 0; - const NativeHandle& handle = memory.handle; - size += sizeof(decltype(handle.fds)::value_type) * handle.fds.size(); - size += sizeof(decltype(handle.ints)::value_type) * handle.ints.size(); - size += sizeForBinder(memory.name); - size += sizeof(memory); + size_t size = sizeof(Memory); + + // Only hardwareBuffer type memory has dynamic memory that needs to be accounted for (in the + // form of a NativeHandle type). The other other types of memory (MappableFile, Ashmem) use a + // single file descriptor (with metadata) instead. + if (memory.getTag() == Memory::Tag::hardwareBuffer) { + const NativeHandle& handle = memory.get().handle; + size += sizeof(decltype(handle.fds)::value_type) * handle.fds.size(); + size += sizeof(decltype(handle.ints)::value_type) * handle.ints.size(); + } return size; } diff --git a/neuralnetworks/utils/common/src/CommonUtils.cpp b/neuralnetworks/utils/common/src/CommonUtils.cpp index 924ecb2d1b..4d26795d89 100644 --- a/neuralnetworks/utils/common/src/CommonUtils.cpp +++ b/neuralnetworks/utils/common/src/CommonUtils.cpp @@ -89,6 +89,59 @@ void copyPointersToSharedMemory(nn::Model::Subgraph* subgraph, }); } +nn::GeneralResult createNativeHandleFrom(base::unique_fd fd, + const std::vector& ints) { + constexpr size_t kIntMax = std::numeric_limits::max(); + CHECK_LE(ints.size(), kIntMax); + native_handle_t* nativeHandle = native_handle_create(1, static_cast(ints.size())); + if (nativeHandle == nullptr) { + return NN_ERROR() << "Failed to create native_handle"; + } + + nativeHandle->data[0] = fd.release(); + std::copy(ints.begin(), ints.end(), nativeHandle->data + 1); + + hidl_handle handle; + handle.setTo(nativeHandle, /*shouldOwn=*/true); + return handle; +} + +nn::GeneralResult createHidlMemoryFrom(const nn::Memory::Ashmem& memory) { + auto fd = NN_TRY(nn::dupFd(memory.fd)); + auto handle = NN_TRY(createNativeHandleFrom(std::move(fd), {})); + return hidl_memory("ashmem", std::move(handle), memory.size); +} + +nn::GeneralResult createHidlMemoryFrom(const nn::Memory::Fd& memory) { + auto fd = NN_TRY(nn::dupFd(memory.fd)); + + const auto [lowOffsetBits, highOffsetBits] = nn::getIntsFromOffset(memory.offset); + const std::vector ints = {memory.prot, lowOffsetBits, highOffsetBits}; + + auto handle = NN_TRY(createNativeHandleFrom(std::move(fd), ints)); + return hidl_memory("mmap_fd", std::move(handle), memory.size); +} + +nn::GeneralResult createHidlMemoryFrom(const nn::Memory::HardwareBuffer& memory) { + const auto* ahwb = memory.handle.get(); + AHardwareBuffer_Desc bufferDesc; + AHardwareBuffer_describe(ahwb, &bufferDesc); + + const bool isBlob = bufferDesc.format == AHARDWAREBUFFER_FORMAT_BLOB; + const size_t size = isBlob ? bufferDesc.width : 0; + const char* const name = isBlob ? "hardware_buffer_blob" : "hardware_buffer"; + + const native_handle_t* nativeHandle = AHardwareBuffer_getNativeHandle(ahwb); + const hidl_handle hidlHandle(nativeHandle); + hidl_handle copiedHandle(hidlHandle); + + return hidl_memory(name, std::move(copiedHandle), size); +} + +nn::GeneralResult createHidlMemoryFrom(const nn::Memory::Unknown& memory) { + return hidl_memory(memory.name, NN_TRY(hidlHandleFromSharedHandle(memory.handle)), memory.size); +} + } // anonymous namespace nn::Capabilities::OperandPerformanceTable makeQuantized8PerformanceConsistentWithP( @@ -255,27 +308,7 @@ nn::GeneralResult createHidlMemoryFromSharedMemory(const nn::Shared if (memory == nullptr) { return NN_ERROR() << "Memory must be non-empty"; } - if (const auto* handle = std::get_if(&memory->handle)) { - return hidl_memory(memory->name, NN_TRY(hidlHandleFromSharedHandle(*handle)), memory->size); - } - - const auto* ahwb = std::get(memory->handle).get(); - AHardwareBuffer_Desc bufferDesc; - AHardwareBuffer_describe(ahwb, &bufferDesc); - - if (bufferDesc.format == AHARDWAREBUFFER_FORMAT_BLOB) { - CHECK_EQ(memory->size, bufferDesc.width); - CHECK_EQ(memory->name, "hardware_buffer_blob"); - } else { - CHECK_EQ(memory->size, 0u); - CHECK_EQ(memory->name, "hardware_buffer"); - } - - const native_handle_t* nativeHandle = AHardwareBuffer_getNativeHandle(ahwb); - const hidl_handle hidlHandle(nativeHandle); - hidl_handle handle(hidlHandle); - - return hidl_memory(memory->name, std::move(handle), memory->size); + return std::visit([](const auto& x) { return createHidlMemoryFrom(x); }, memory->handle); } static uint32_t roundUpToMultiple(uint32_t value, uint32_t multiple) { @@ -283,14 +316,53 @@ static uint32_t roundUpToMultiple(uint32_t value, uint32_t multiple) { } nn::GeneralResult createSharedMemoryFromHidlMemory(const hidl_memory& memory) { - CHECK_LE(memory.size(), std::numeric_limits::max()); + CHECK_LE(memory.size(), std::numeric_limits::max()); + if (!memory.valid()) { + return NN_ERROR() << "Unable to convert invalid hidl_memory"; + } + + if (memory.name() == "ashmem") { + if (memory.handle()->numFds != 1) { + return NN_ERROR() << "Unable to convert invalid ashmem memory object with " + << memory.handle()->numFds << " numFds, but expected 1"; + } + if (memory.handle()->numInts != 0) { + return NN_ERROR() << "Unable to convert invalid ashmem memory object with " + << memory.handle()->numInts << " numInts, but expected 0"; + } + auto handle = nn::Memory::Ashmem{ + .fd = NN_TRY(nn::dupFd(memory.handle()->data[0])), + .size = static_cast(memory.size()), + }; + return std::make_shared(nn::Memory{.handle = std::move(handle)}); + } + + if (memory.name() == "mmap_fd") { + if (memory.handle()->numFds != 1) { + return NN_ERROR() << "Unable to convert invalid mmap_fd memory object with " + << memory.handle()->numFds << " numFds, but expected 1"; + } + if (memory.handle()->numInts != 3) { + return NN_ERROR() << "Unable to convert invalid mmap_fd memory object with " + << memory.handle()->numInts << " numInts, but expected 3"; + } + + const int fd = memory.handle()->data[0]; + const int prot = memory.handle()->data[1]; + const int lower = memory.handle()->data[2]; + const int higher = memory.handle()->data[3]; + const size_t offset = nn::getOffsetFromInts(lower, higher); + + return nn::createSharedMemoryFromFd(static_cast(memory.size()), prot, fd, offset); + } if (memory.name() != "hardware_buffer_blob") { - return std::make_shared(nn::Memory{ + auto handle = nn::Memory::Unknown{ .handle = NN_TRY(sharedHandleFromNativeHandle(memory.handle())), - .size = static_cast(memory.size()), + .size = static_cast(memory.size()), .name = memory.name(), - }); + }; + return std::make_shared(nn::Memory{.handle = std::move(handle)}); } const auto size = memory.size(); @@ -328,11 +400,7 @@ nn::GeneralResult createSharedMemoryFromHidlMemory(const hidl_ << "Can't create AHardwareBuffer from handle. Error: " << status; } - return std::make_shared(nn::Memory{ - .handle = nn::HardwareBufferHandle(hardwareBuffer, /*takeOwnership=*/true), - .size = static_cast(memory.size()), - .name = memory.name(), - }); + return nn::createSharedMemoryFromAHWB(hardwareBuffer, /*takeOwnership=*/true); } nn::GeneralResult hidlHandleFromSharedHandle(const nn::Handle& handle) {