| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <feb5e4c5-b5c2-e235-c952-70c38e185080@redhat.com>
Date: Mon, 19 Feb 2018 10:18:21 -0800
From: Laura Abbott <labbott@...hat.com>
To: Sumit Semwal <sumit.semwal@...aro.org>, devel@...verdev.osuosl.org,
Todd Kjos <tkjos@...roid.com>, linux-kernel@...r.kernel.org,
dri-devel@...ts.freedesktop.org, Liam Mark <lmark@...eaurora.org>,
linux-kselftest@...r.kernel.org, Shuah Khan <shuah@...nel.org>
Subject: Re: [PATCH 2/2] selftests: ion: Add simple test with the vgem driver
On 02/19/2018 07:31 AM, Daniel Vetter wrote:
> On Thu, Feb 15, 2018 at 05:24:45PM -0800, Laura Abbott wrote:
>> Ion is designed to be a framework used by other clients who perform
>> operations on the buffer. Use the DRM vgem client as a simple consumer.
>> In conjunction with the dma-buf sync ioctls, this tests the full attach/map
>> path for the system heap.
>>
>> Signed-off-by: Laura Abbott <labbott@...hat.com>
>> ---
>> tools/testing/selftests/android/ion/Makefile | 3 +-
>> tools/testing/selftests/android/ion/config | 1 +
>> tools/testing/selftests/android/ion/ionmap_test.c | 136 ++++++++++++++++++++++
>> 3 files changed, 139 insertions(+), 1 deletion(-)
>> create mode 100644 tools/testing/selftests/android/ion/ionmap_test.c
>>
>> diff --git a/tools/testing/selftests/android/ion/Makefile b/tools/testing/selftests/android/ion/Makefile
>> index 96e0c448b39d..d23b6d537d8b 100644
>> --- a/tools/testing/selftests/android/ion/Makefile
>> +++ b/tools/testing/selftests/android/ion/Makefile
>> @@ -2,7 +2,7 @@
>> INCLUDEDIR := -I. -I../../../../../drivers/staging/android/uapi/
>> CFLAGS := $(CFLAGS) $(INCLUDEDIR) -Wall -O2 -g
>>
>> -TEST_GEN_FILES := ionapp_export ionapp_import
>> +TEST_GEN_FILES := ionapp_export ionapp_import ionmap_test
>>
>> all: $(TEST_GEN_FILES)
>>
>> @@ -14,3 +14,4 @@ include ../../lib.mk
>>
>> $(OUTPUT)/ionapp_export: ionapp_export.c ipcsocket.c ionutils.c
>> $(OUTPUT)/ionapp_import: ionapp_import.c ipcsocket.c ionutils.c
>> +$(OUTPUT)/ionmap_test: ionmap_test.c ionutils.c
>> diff --git a/tools/testing/selftests/android/ion/config b/tools/testing/selftests/android/ion/config
>> index 19db6ca9aa2b..b4ad748a9dd9 100644
>> --- a/tools/testing/selftests/android/ion/config
>> +++ b/tools/testing/selftests/android/ion/config
>> @@ -2,3 +2,4 @@ CONFIG_ANDROID=y
>> CONFIG_STAGING=y
>> CONFIG_ION=y
>> CONFIG_ION_SYSTEM_HEAP=y
>> +CONFIG_DRM_VGEM=y
>> diff --git a/tools/testing/selftests/android/ion/ionmap_test.c b/tools/testing/selftests/android/ion/ionmap_test.c
>> new file mode 100644
>> index 000000000000..dab36b06b37d
>> --- /dev/null
>> +++ b/tools/testing/selftests/android/ion/ionmap_test.c
>> @@ -0,0 +1,136 @@
>> +#include <errno.h>
>> +#include <fcntl.h>
>> +#include <stdio.h>
>> +#include <stdint.h>
>> +#include <string.h>
>> +#include <unistd.h>
>> +
>> +#include <sys/ioctl.h>
>> +#include <sys/types.h>
>> +#include <sys/stat.h>
>> +
>> +#include <linux/dma-buf.h>
>> +
>> +#include <drm/drm.h>
>> +
>> +#include "ion.h"
>> +#include "ionutils.h"
>> +
>> +int check_vgem(int fd)
>> +{
>> + drm_version_t version = { 0 };
>> + char name[5];
>> + int ret;
>> +
>> + version.name_len = 4;
>> + version.name = name;
>> +
>> + ret = ioctl(fd, DRM_IOCTL_VERSION, &version);
>> + if (ret)
>> + return 1;
>> +
>> + return strcmp(name, "vgem");
>> +}
>> +
>> +int open_vgem(void)
>> +{
>> + int i, fd;
>> + const char *drmstr = "/dev/dri/card";
>> +
>> + fd = -1;
>> + for (i = 0; i < 16; i++) {
>> + char name[80];
>> +
>> + sprintf(name, "%s%u", drmstr, i);
>> +
>> + fd = open(name, O_RDWR);
>> + if (fd < 0)
>> + continue;
>> +
>> + if (check_vgem(fd)) {
>> + close(fd);
>> + continue;
>> + } else {
>> + break;
>> + }
>> +
>> + }
>> + return fd;
>> +}
>> +
>> +int import_vgem_fd(int vgem_fd, int dma_buf_fd, uint32_t *handle)
>> +{
>> + struct drm_prime_handle import_handle = { 0 };
>> + int ret;
>> +
>> + import_handle.fd = dma_buf_fd;
>> + import_handle.flags = 0;
>> + import_handle.handle = 0;
>> +
>> + ret = ioctl(vgem_fd, DRM_IOCTL_PRIME_FD_TO_HANDLE, &import_handle);
>> + if (ret == 0)
>> + *handle = import_handle.handle;
>> + return ret;
>> +}
>> +
>> +void close_handle(int vgem_fd, uint32_t handle)
>> +{
>> + struct drm_gem_close close = { 0 };
>> +
>> + close.handle = handle;
>> + ioctl(vgem_fd, DRM_IOCTL_GEM_CLOSE, &close);
>> +}
>> +
>> +int main()
>> +{
>> + int ret, vgem_fd;
>> + struct ion_buffer_info info;
>> + uint32_t handle = 0;
>> + struct dma_buf_sync sync = { 0 };
>> +
>> + info.heap_type = ION_HEAP_TYPE_SYSTEM;
>> + info.heap_size = 4096;
>> + info.flag_type = ION_FLAG_CACHED;
>> +
>> + ret = ion_export_buffer_fd(&info);
>> + if (ret < 0) {
>> + printf("ion buffer alloc failed\n");
>> + return -1;
>> + }
>> +
>> + vgem_fd = open_vgem();
>> + if (vgem_fd < 0) {
>> + ret = vgem_fd;
>> + printf("Failed to open vgem\n");
>> + goto out_ion;
>> + }
>> +
>> + ret = import_vgem_fd(vgem_fd, info.buffd, &handle);
>> +
>> + if (ret < 0) {
>> + printf("Failed to import buffer\n");
>> + goto out_vgem;
>> + }
>> +
>> + sync.flags = DMA_BUF_SYNC_START | DMA_BUF_SYNC_RW;
>> + ret = ioctl(info.buffd, DMA_BUF_IOCTL_SYNC, &sync);
>> + if (ret)
>> + printf("sync start failed %d\n", errno);
>> +
>> + memset(info.buffer, 0xff, 4096);
>> +
>> + sync.flags = DMA_BUF_SYNC_END | DMA_BUF_SYNC_RW;
>> + ret = ioctl(info.buffd, DMA_BUF_IOCTL_SYNC, &sync);
>> + if (ret)
>> + printf("sync end failed %d\n", errno);
>
> At least in drm we require that userspace auto-restarts all ioctls when
> they get interrupt. See
>
> https://cgit.freedesktop.org/drm/libdrm/tree/xf86drm.c#n186
>
> not really an issue with vgem (which can't wait for hw or anything else).
> But good to make sure we don't spread bad copypastas.
>
> Actual use of the ioctls looks all good. With the drmIoctl wrapper added
> and used this is:
>
> Reviewed-by: Daniel Vetter <daniel.vetter@...ll.ch>
Thanks for the review. The reason I didn't use the standard drmIoctl was
because I didn't want to introduce a dependency on libdrm. I don't see
an example of another selftest having a dependency on an external
library.
Is adding a dependencies on fairly standard but still external userspace
libraries okay for kernel self tests?
Thanks,
Laura
Powered by blists - more mailing lists