* [PATCH v12 17/22] selftests/vm: associate key on a mapped page and detect access violation
From: Ram Pai @ 2018-02-22 1:55 UTC (permalink / raw)
To: shuahkh, linux-kselftest
Cc: mpe, linuxppc-dev, linux-mm, x86, linux-arch, linux-doc,
linux-kernel, mingo, akpm, dave.hansen, benh, paulus, khandual,
aneesh.kumar, bsingharora, hbabu, mhocko, bauerman, ebiederm,
linuxram, arnd
In-Reply-To: <1519264541-7621-1-git-send-email-linuxram@us.ibm.com>
detect access-violation on a page to which access-disabled
key is associated much after the page is mapped.
cc: Dave Hansen <dave.hansen@intel.com>
cc: Florian Weimer <fweimer@redhat.com>
Signed-off-by: Ram Pai <linuxram@us.ibm.com>
---
tools/testing/selftests/vm/protection_keys.c | 19 +++++++++++++++++++
1 files changed, 19 insertions(+), 0 deletions(-)
diff --git a/tools/testing/selftests/vm/protection_keys.c b/tools/testing/selftests/vm/protection_keys.c
index aae6771..dba9162 100644
--- a/tools/testing/selftests/vm/protection_keys.c
+++ b/tools/testing/selftests/vm/protection_keys.c
@@ -1016,6 +1016,24 @@ void test_read_of_access_disabled_region(int *ptr, u16 pkey)
dprintf1("*ptr: %d\n", ptr_contents);
expected_pkey_fault(pkey);
}
+
+void test_read_of_access_disabled_region_with_page_already_mapped(int *ptr,
+ u16 pkey)
+{
+ int ptr_contents;
+
+ dprintf1("disabling access to PKEY[%02d], doing read @ %p\n",
+ pkey, ptr);
+ ptr_contents = read_ptr(ptr);
+ dprintf1("reading ptr before disabling the read : %d\n",
+ ptr_contents);
+ rdpkey_reg();
+ pkey_access_deny(pkey);
+ ptr_contents = read_ptr(ptr);
+ dprintf1("*ptr: %d\n", ptr_contents);
+ expected_pkey_fault(pkey);
+}
+
void test_write_of_write_disabled_region(int *ptr, u16 pkey)
{
dprintf1("disabling write access to PKEY[%02d], doing write\n", pkey);
@@ -1310,6 +1328,7 @@ void test_mprotect_pkey_on_unsupported_cpu(int *ptr, u16 pkey)
void (*pkey_tests[])(int *ptr, u16 pkey) = {
test_read_of_write_disabled_region,
test_read_of_access_disabled_region,
+ test_read_of_access_disabled_region_with_page_already_mapped,
test_write_of_write_disabled_region,
test_write_of_access_disabled_region,
test_kernel_write_of_access_disabled_region,
--
1.7.1
^ permalink raw reply related
* [PATCH v12 18/22] selftests/vm: associate key on a mapped page and detect write violation
From: Ram Pai @ 2018-02-22 1:55 UTC (permalink / raw)
To: shuahkh, linux-kselftest
Cc: mpe, linuxppc-dev, linux-mm, x86, linux-arch, linux-doc,
linux-kernel, mingo, akpm, dave.hansen, benh, paulus, khandual,
aneesh.kumar, bsingharora, hbabu, mhocko, bauerman, ebiederm,
linuxram, arnd
In-Reply-To: <1519264541-7621-1-git-send-email-linuxram@us.ibm.com>
detect write-violation on a page to which write-disabled
key is associated much after the page is mapped.
cc: Dave Hansen <dave.hansen@intel.com>
cc: Florian Weimer <fweimer@redhat.com>
Signed-off-by: Ram Pai <linuxram@us.ibm.com>
---
tools/testing/selftests/vm/protection_keys.c | 12 ++++++++++++
1 files changed, 12 insertions(+), 0 deletions(-)
diff --git a/tools/testing/selftests/vm/protection_keys.c b/tools/testing/selftests/vm/protection_keys.c
index dba9162..b685e26 100644
--- a/tools/testing/selftests/vm/protection_keys.c
+++ b/tools/testing/selftests/vm/protection_keys.c
@@ -1034,6 +1034,17 @@ void test_read_of_access_disabled_region_with_page_already_mapped(int *ptr,
expected_pkey_fault(pkey);
}
+void test_write_of_write_disabled_region_with_page_already_mapped(int *ptr,
+ u16 pkey)
+{
+ *ptr = __LINE__;
+ dprintf1("disabling write access; after accessing the page, "
+ "to PKEY[%02d], doing write\n", pkey);
+ pkey_write_deny(pkey);
+ *ptr = __LINE__;
+ expected_pkey_fault(pkey);
+}
+
void test_write_of_write_disabled_region(int *ptr, u16 pkey)
{
dprintf1("disabling write access to PKEY[%02d], doing write\n", pkey);
@@ -1330,6 +1341,7 @@ void test_mprotect_pkey_on_unsupported_cpu(int *ptr, u16 pkey)
test_read_of_access_disabled_region,
test_read_of_access_disabled_region_with_page_already_mapped,
test_write_of_write_disabled_region,
+ test_write_of_write_disabled_region_with_page_already_mapped,
test_write_of_access_disabled_region,
test_kernel_write_of_access_disabled_region,
test_kernel_write_of_write_disabled_region,
--
1.7.1
^ permalink raw reply related
* [PATCH v12 19/22] selftests/vm: detect write violation on a mapped access-denied-key page
From: Ram Pai @ 2018-02-22 1:55 UTC (permalink / raw)
To: shuahkh, linux-kselftest
Cc: mpe, linuxppc-dev, linux-mm, x86, linux-arch, linux-doc,
linux-kernel, mingo, akpm, dave.hansen, benh, paulus, khandual,
aneesh.kumar, bsingharora, hbabu, mhocko, bauerman, ebiederm,
linuxram, arnd
In-Reply-To: <1519264541-7621-1-git-send-email-linuxram@us.ibm.com>
detect write-violation on a page to which access-disabled
key is associated much after the page is mapped.
cc: Dave Hansen <dave.hansen@intel.com>
cc: Florian Weimer <fweimer@redhat.com>
Signed-off-by: Ram Pai <linuxram@us.ibm.com>
---
tools/testing/selftests/vm/protection_keys.c | 13 +++++++++++++
1 files changed, 13 insertions(+), 0 deletions(-)
diff --git a/tools/testing/selftests/vm/protection_keys.c b/tools/testing/selftests/vm/protection_keys.c
index b685e26..437ee74 100644
--- a/tools/testing/selftests/vm/protection_keys.c
+++ b/tools/testing/selftests/vm/protection_keys.c
@@ -1059,6 +1059,18 @@ void test_write_of_access_disabled_region(int *ptr, u16 pkey)
*ptr = __LINE__;
expected_pkey_fault(pkey);
}
+
+void test_write_of_access_disabled_region_with_page_already_mapped(int *ptr,
+ u16 pkey)
+{
+ *ptr = __LINE__;
+ dprintf1("disabling access; after accessing the page, "
+ " to PKEY[%02d], doing write\n", pkey);
+ pkey_access_deny(pkey);
+ *ptr = __LINE__;
+ expected_pkey_fault(pkey);
+}
+
void test_kernel_write_of_access_disabled_region(int *ptr, u16 pkey)
{
int ret;
@@ -1343,6 +1355,7 @@ void test_mprotect_pkey_on_unsupported_cpu(int *ptr, u16 pkey)
test_write_of_write_disabled_region,
test_write_of_write_disabled_region_with_page_already_mapped,
test_write_of_access_disabled_region,
+ test_write_of_access_disabled_region_with_page_already_mapped,
test_kernel_write_of_access_disabled_region,
test_kernel_write_of_write_disabled_region,
test_kernel_gup_of_access_disabled_region,
--
1.7.1
^ permalink raw reply related
* [PATCH v12 20/22] selftests/vm: testcases must restore pkey-permissions
From: Ram Pai @ 2018-02-22 1:55 UTC (permalink / raw)
To: shuahkh, linux-kselftest
Cc: mpe, linuxppc-dev, linux-mm, x86, linux-arch, linux-doc,
linux-kernel, mingo, akpm, dave.hansen, benh, paulus, khandual,
aneesh.kumar, bsingharora, hbabu, mhocko, bauerman, ebiederm,
linuxram, arnd
In-Reply-To: <1519264541-7621-1-git-send-email-linuxram@us.ibm.com>
Generally the signal handler restores the state of the pkey register
before returning. However there are times when the read/write operation
can legitamely fail without invoking the signal handler. Eg: A
sys_read() operaton to a write-protected page should be disallowed. In
such a case the state of the pkey register is not restored to its
original state. The test case is responsible for restoring the key
register state to its original value.
cc: Dave Hansen <dave.hansen@intel.com>
cc: Florian Weimer <fweimer@redhat.com>
Signed-off-by: Ram Pai <linuxram@us.ibm.com>
---
tools/testing/selftests/vm/protection_keys.c | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/tools/testing/selftests/vm/protection_keys.c b/tools/testing/selftests/vm/protection_keys.c
index 437ee74..42c068a 100644
--- a/tools/testing/selftests/vm/protection_keys.c
+++ b/tools/testing/selftests/vm/protection_keys.c
@@ -1003,6 +1003,7 @@ void test_read_of_write_disabled_region(int *ptr, u16 pkey)
ptr_contents = read_ptr(ptr);
dprintf1("*ptr: %d\n", ptr_contents);
dprintf1("\n");
+ pkey_write_allow(pkey);
}
void test_read_of_access_disabled_region(int *ptr, u16 pkey)
{
@@ -1082,6 +1083,7 @@ void test_kernel_write_of_access_disabled_region(int *ptr, u16 pkey)
ret = read(test_fd, ptr, 1);
dprintf1("read ret: %d\n", ret);
pkey_assert(ret);
+ pkey_access_allow(pkey);
}
void test_kernel_write_of_write_disabled_region(int *ptr, u16 pkey)
{
@@ -1094,6 +1096,7 @@ void test_kernel_write_of_write_disabled_region(int *ptr, u16 pkey)
if (ret < 0 && (DEBUG_LEVEL > 0))
perror("verbose read result (OK for this to be bad)");
pkey_assert(ret);
+ pkey_write_allow(pkey);
}
void test_kernel_gup_of_access_disabled_region(int *ptr, u16 pkey)
@@ -1113,6 +1116,7 @@ void test_kernel_gup_of_access_disabled_region(int *ptr, u16 pkey)
vmsplice_ret = vmsplice(pipe_fds[1], &iov, 1, SPLICE_F_GIFT);
dprintf1("vmsplice() ret: %d\n", vmsplice_ret);
pkey_assert(vmsplice_ret == -1);
+ pkey_access_allow(pkey);
close(pipe_fds[0]);
close(pipe_fds[1]);
@@ -1133,6 +1137,7 @@ void test_kernel_gup_write_to_write_disabled_region(int *ptr, u16 pkey)
if (DEBUG_LEVEL > 0)
perror("futex");
dprintf1("futex() ret: %d\n", futex_ret);
+ pkey_write_allow(pkey);
}
/* Assumes that all pkeys other than 'pkey' are unallocated */
--
1.7.1
^ permalink raw reply related
* [PATCH v12 21/22] selftests/vm: sub-page allocator
From: Ram Pai @ 2018-02-22 1:55 UTC (permalink / raw)
To: shuahkh, linux-kselftest
Cc: mpe, linuxppc-dev, linux-mm, x86, linux-arch, linux-doc,
linux-kernel, mingo, akpm, dave.hansen, benh, paulus, khandual,
aneesh.kumar, bsingharora, hbabu, mhocko, bauerman, ebiederm,
linuxram, arnd
In-Reply-To: <1519264541-7621-1-git-send-email-linuxram@us.ibm.com>
introduce a new allocator that allocates 4k hardware-pages to back
64k linux-page. This allocator is only applicable on powerpc.
cc: Dave Hansen <dave.hansen@intel.com>
cc: Florian Weimer <fweimer@redhat.com>
Signed-off-by: Ram Pai <linuxram@us.ibm.com>
---
tools/testing/selftests/vm/protection_keys.c | 30 ++++++++++++++++++++++++++
1 files changed, 30 insertions(+), 0 deletions(-)
diff --git a/tools/testing/selftests/vm/protection_keys.c b/tools/testing/selftests/vm/protection_keys.c
index 42c068a..1b06e59 100644
--- a/tools/testing/selftests/vm/protection_keys.c
+++ b/tools/testing/selftests/vm/protection_keys.c
@@ -766,6 +766,35 @@ void free_pkey_malloc(void *ptr)
return ptr;
}
+void *malloc_pkey_with_mprotect_subpage(long size, int prot, u16 pkey)
+{
+#ifdef __powerpc64__
+ void *ptr;
+ int ret;
+
+ dprintf1("doing %s(size=%ld, prot=0x%x, pkey=%d)\n", __func__,
+ size, prot, pkey);
+ pkey_assert(pkey < NR_PKEYS);
+ ptr = mmap(NULL, size, prot, MAP_ANONYMOUS|MAP_PRIVATE, -1, 0);
+ pkey_assert(ptr != (void *)-1);
+
+ ret = syscall(__NR_subpage_prot, ptr, size, NULL);
+ if (ret) {
+ perror("subpage_perm");
+ return PTR_ERR_ENOTSUP;
+ }
+
+ ret = mprotect_pkey((void *)ptr, PAGE_SIZE, prot, pkey);
+ pkey_assert(!ret);
+ record_pkey_malloc(ptr, size);
+
+ dprintf1("%s() for pkey %d @ %p\n", __func__, pkey, ptr);
+ return ptr;
+#else /* __powerpc64__ */
+ return PTR_ERR_ENOTSUP;
+#endif /* __powerpc64__ */
+}
+
void *malloc_pkey_anon_huge(long size, int prot, u16 pkey)
{
int ret;
@@ -888,6 +917,7 @@ void setup_hugetlbfs(void)
void *(*pkey_malloc[])(long size, int prot, u16 pkey) = {
malloc_pkey_with_mprotect,
+ malloc_pkey_with_mprotect_subpage,
malloc_pkey_anon_huge,
malloc_pkey_hugetlb
/* can not do direct with the pkey_mprotect() API:
--
1.7.1
^ permalink raw reply related
* [PATCH v12 22/22] selftests/vm: Fix deadlock in protection_keys.c
From: Ram Pai @ 2018-02-22 1:55 UTC (permalink / raw)
To: shuahkh, linux-kselftest
Cc: mpe, linuxppc-dev, linux-mm, x86, linux-arch, linux-doc,
linux-kernel, mingo, akpm, dave.hansen, benh, paulus, khandual,
aneesh.kumar, bsingharora, hbabu, mhocko, bauerman, ebiederm,
linuxram, arnd
In-Reply-To: <1519264541-7621-1-git-send-email-linuxram@us.ibm.com>
From: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
The sig_chld() handler calls dprintf2() taking care of setting
dprint_in_signal so that sigsafe_printf() won't call printf().
Unfortunately, this precaution is is negated by dprintf_level(), which
has a call to fflush().
This function acquires a lock, which means that if the signal interrupts an
ongoing fflush() the process will deadlock. At least on powerpc this is
easy to trigger, resulting in the following backtrace when attaching to the
frozen process:
(gdb) bt
#0 0x00007fff9f96c7d8 in __lll_lock_wait_private () from /lib64/power8/libc.so.6
#1 0x00007fff9f8cba4c in _IO_flush_all_lockp () from /lib64/power8/libc.so.6
#2 0x00007fff9f8cbd1c in __GI__IO_flush_all () from /lib64/power8/libc.so.6
#3 0x00007fff9f8b7424 in fflush () from /lib64/power8/libc.so.6
#4 0x00000000100504f8 in sig_chld (x=17) at protection_keys.c:283
#5 <signal handler called>
#6 0x00007fff9f8cb8ac in _IO_flush_all_lockp () from /lib64/power8/libc.so.6
#7 0x00007fff9f8cbd1c in __GI__IO_flush_all () from /lib64/power8/libc.so.6
#8 0x00007fff9f8b7424 in fflush () from /lib64/power8/libc.so.6
#9 0x0000000010050b50 in pkey_get (pkey=7, flags=0) at protection_keys.c:379
#10 0x0000000010050dc0 in pkey_disable_set (pkey=7, flags=2) at protection_keys.c:423
#11 0x0000000010051414 in pkey_write_deny (pkey=7) at protection_keys.c:486
#12 0x00000000100556bc in test_ptrace_of_child (ptr=0x7fff9f7f0000, pkey=7) at protection_keys.c:1288
#13 0x0000000010055f60 in run_tests_once () at protection_keys.c:1414
#14 0x00000000100561a4 in main () at protection_keys.c:1459
The fix is to refrain from calling fflush() when inside a signal handler.
The output may not be as pretty but at least the testcase will be able to
move on.
cc: Dave Hansen <dave.hansen@intel.com>
cc: Florian Weimer <fweimer@redhat.com>
Signed-off-by: Ram Pai <linuxram@us.ibm.com>
Signed-off-by: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
tools/testing/selftests/vm/pkey-helpers.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
---
tools/testing/selftests/vm/pkey-helpers.h | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/tools/testing/selftests/vm/pkey-helpers.h b/tools/testing/selftests/vm/pkey-helpers.h
index 67f9b0f..7240598 100644
--- a/tools/testing/selftests/vm/pkey-helpers.h
+++ b/tools/testing/selftests/vm/pkey-helpers.h
@@ -128,7 +128,8 @@ static inline void sigsafe_printf(const char *format, ...)
#define dprintf_level(level, args...) do { \
if (level <= DEBUG_LEVEL) \
sigsafe_printf(args); \
- fflush(NULL); \
+ if (!dprint_in_signal) \
+ fflush(NULL); \
} while (0)
#define dprintf0(args...) dprintf_level(0, args)
#define dprintf1(args...) dprintf_level(1, args)
--
1.7.1
^ permalink raw reply related
* [PATCH v2 0/2] Add get_metadata IOCTL to share OCXL information to userspace
From: Alastair D'Silva @ 2018-02-22 3:03 UTC (permalink / raw)
To: linuxppc-dev, linux-kernel
Cc: frederic.barrat, andrew.donnellan, arnd, gregkh, alastair,
Alastair D'Silva
From: Alastair D'Silva <alastair@d-silva.org>
This series adds a new IOCTL for OpenCAPI, which exposes adapter
metadata to userspace.
---
v1->v2:
- Document the new IOCTL
Alastair D'Silva (2):
ocxl: Add get_metadata IOCTL to share OCXL information to userspace
ocxl: Document the OCXL_IOCTL_GET_METADATA IOCTL
Documentation/accelerators/ocxl.rst | 5 +++++
drivers/misc/ocxl/file.c | 27 +++++++++++++++++++++++++++
include/uapi/misc/ocxl.h | 22 ++++++++++++++++++++++
3 files changed, 54 insertions(+)
--
2.14.3
^ permalink raw reply
* [PATCH v2 2/2] ocxl: Document the OCXL_IOCTL_GET_METADATA IOCTL
From: Alastair D'Silva @ 2018-02-22 3:03 UTC (permalink / raw)
To: linuxppc-dev, linux-kernel
Cc: frederic.barrat, andrew.donnellan, arnd, gregkh, alastair,
Alastair D'Silva
In-Reply-To: <20180222030334.14060-1-alastair@au1.ibm.com>
From: Alastair D'Silva <alastair@d-silva.org>
Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
---
Documentation/accelerators/ocxl.rst | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/Documentation/accelerators/ocxl.rst b/Documentation/accelerators/ocxl.rst
index 4f7af841d935..ddcc58d01cfb 100644
--- a/Documentation/accelerators/ocxl.rst
+++ b/Documentation/accelerators/ocxl.rst
@@ -152,6 +152,11 @@ OCXL_IOCTL_IRQ_SET_FD:
Associate an event fd to an AFU interrupt so that the user process
can be notified when the AFU sends an interrupt.
+OCXL_IOCTL_GET_METADATA:
+
+ Obtains configuration information from the card, such at the size of
+ MMIO areas, the AFU version, and the PASID for the current context.
+
mmap
----
--
2.14.3
^ permalink raw reply related
* [PATCH v2 1/2] ocxl: Add get_metadata IOCTL to share OCXL information to userspace
From: Alastair D'Silva @ 2018-02-22 3:03 UTC (permalink / raw)
To: linuxppc-dev, linux-kernel
Cc: frederic.barrat, andrew.donnellan, arnd, gregkh, alastair,
Alastair D'Silva
In-Reply-To: <20180222030334.14060-1-alastair@au1.ibm.com>
From: Alastair D'Silva <alastair@d-silva.org>
Some required information is not exposed to userspace currently (eg. the
PASID), pass this information back, along with other information which
is currently communicated via sysfs, which saves some parsing effort in
userspace.
Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
---
drivers/misc/ocxl/file.c | 27 +++++++++++++++++++++++++++
include/uapi/misc/ocxl.h | 22 ++++++++++++++++++++++
2 files changed, 49 insertions(+)
diff --git a/drivers/misc/ocxl/file.c b/drivers/misc/ocxl/file.c
index d9aa407db06a..11514a8444e5 100644
--- a/drivers/misc/ocxl/file.c
+++ b/drivers/misc/ocxl/file.c
@@ -102,10 +102,32 @@ static long afu_ioctl_attach(struct ocxl_context *ctx,
return rc;
}
+static long afu_ioctl_get_metadata(struct ocxl_context *ctx,
+ struct ocxl_ioctl_get_metadata __user *uarg)
+{
+ struct ocxl_ioctl_get_metadata arg;
+
+ memset(&arg, 0, sizeof(arg));
+
+ arg.version = 0;
+
+ arg.afu_version_major = ctx->afu->config.version_major;
+ arg.afu_version_minor = ctx->afu->config.version_minor;
+ arg.pasid = ctx->pasid;
+ arg.pp_mmio_size = ctx->afu->config.pp_mmio_stride;
+ arg.global_mmio_size = ctx->afu->config.global_mmio_size;
+
+ if (copy_to_user(uarg, &arg, sizeof(arg)))
+ return -EFAULT;
+
+ return 0;
+}
+
#define CMD_STR(x) (x == OCXL_IOCTL_ATTACH ? "ATTACH" : \
x == OCXL_IOCTL_IRQ_ALLOC ? "IRQ_ALLOC" : \
x == OCXL_IOCTL_IRQ_FREE ? "IRQ_FREE" : \
x == OCXL_IOCTL_IRQ_SET_FD ? "IRQ_SET_FD" : \
+ x == OCXL_IOCTL_GET_METADATA ? "GET_METADATA" : \
"UNKNOWN")
static long afu_ioctl(struct file *file, unsigned int cmd,
@@ -157,6 +179,11 @@ static long afu_ioctl(struct file *file, unsigned int cmd,
irq_fd.eventfd);
break;
+ case OCXL_IOCTL_GET_METADATA:
+ rc = afu_ioctl_get_metadata(ctx,
+ (struct ocxl_ioctl_get_metadata __user *) args);
+ break;
+
default:
rc = -EINVAL;
}
diff --git a/include/uapi/misc/ocxl.h b/include/uapi/misc/ocxl.h
index 4b0b0b756f3e..16e1f48ce280 100644
--- a/include/uapi/misc/ocxl.h
+++ b/include/uapi/misc/ocxl.h
@@ -32,6 +32,27 @@ struct ocxl_ioctl_attach {
__u64 reserved3;
};
+/*
+ * Version contains the version of the struct.
+ * Versions will always be backwards compatible, that is, new versions will not
+ * alter existing fields
+ */
+struct ocxl_ioctl_get_metadata {
+ __u16 version;
+
+ // Version 0 fields
+ __u8 afu_version_major;
+ __u8 afu_version_minor;
+ __u32 pasid;
+
+ __u64 pp_mmio_size;
+ __u64 global_mmio_size;
+
+ // End version 0 fields
+
+ __u64 reserved[13]; // Total of 16*u64
+};
+
struct ocxl_ioctl_irq_fd {
__u64 irq_offset;
__s32 eventfd;
@@ -45,5 +66,6 @@ struct ocxl_ioctl_irq_fd {
#define OCXL_IOCTL_IRQ_ALLOC _IOR(OCXL_MAGIC, 0x11, __u64)
#define OCXL_IOCTL_IRQ_FREE _IOW(OCXL_MAGIC, 0x12, __u64)
#define OCXL_IOCTL_IRQ_SET_FD _IOW(OCXL_MAGIC, 0x13, struct ocxl_ioctl_irq_fd)
+#define OCXL_IOCTL_GET_METADATA _IOR(OCXL_MAGIC, 0x14, struct ocxl_ioctl_get_metadata)
#endif /* _UAPI_MISC_OCXL_H */
--
2.14.3
^ permalink raw reply related
* Re: [PATCH] ocxl: Add get_metadata IOCTL to share OCXL information to userspace
From: Michael Ellerman @ 2018-02-22 3:19 UTC (permalink / raw)
To: Alastair D'Silva, Balbir Singh
Cc: Arnd Bergmann, frederic.barrat, Greg KH,
linux-kernel@vger.kernel.org, linuxppc-dev, Andrew Donnellan
In-Reply-To: <1519255934.2867.3.camel@au1.ibm.com>
"Alastair D'Silva" <alastair@au1.ibm.com> writes:
> On Wed, 2018-02-21 at 17:43 +1100, Balbir Singh wrote:
>> On Wed, Feb 21, 2018 at 3:57 PM, Alastair D'Silva <alastair@au1.ibm.c
>> om> wrote:
>> > From: Alastair D'Silva <alastair@d-silva.org>
>> >
...
>> > diff --git a/include/uapi/misc/ocxl.h b/include/uapi/misc/ocxl.h
>> > index 4b0b0b756f3e..16e1f48ce280 100644
>> > --- a/include/uapi/misc/ocxl.h
>> > +++ b/include/uapi/misc/ocxl.h
>> > @@ -32,6 +32,27 @@ struct ocxl_ioctl_attach {
>> > __u64 reserved3;
>> > };
>> >
>> > +/*
>> > + * Version contains the version of the struct.
>> > + * Versions will always be backwards compatible, that is, new
>> > versions will not
>> > + * alter existing fields
>> > + */
>> > +struct ocxl_ioctl_get_metadata {
>>
>> This sounds more like a function name, do we need it to be
>> _get_metdata?
>>
>
> It pretty much is a function, it returns to userspace metadata about
> the descriptor being operated on.
It's not a function, it's a struct?
Outside of "management English" "get" is a verb, so using it in the name
of the struct is confusing, it should be a noun phrase.
cheers
^ permalink raw reply
* Re: [PATCH 00/23] kconfig: move compiler capability tests to Kconfig
From: Michael Ellerman @ 2018-02-22 3:22 UTC (permalink / raw)
To: Masahiro Yamada, Arnd Bergmann
Cc: Rich Felker, Kernel Hardening, X86 ML, Paul Mackerras,
H. Peter Anvin, sparclinux, Sam Ravnborg, Yoshinori Sato,
Jonathan Corbet, Richard Weinberger, Linux-sh list,
Linus Torvalds, Ingo Molnar, Emese Revfy, Kees Cook, uml-devel,
Linux Kbuild mailing list, Peter Oberparleiter, Jeff Dike,
user-mode-linux-user, Thomas Gleixner, Michal Marek,
Ulf Magnusson, Greg Kroah-Hartman, Randy Dunlap,
open list:DOCUMENTATION, Linux Kernel Mailing List, linuxppc-dev,
David S. Miller
In-Reply-To: <CAK7LNAR3OMh9Q9ZfaBq=FpSJ-+DT5-RH_20ohV-iu34pX9hFKw@mail.gmail.com>
Masahiro Yamada <yamada.masahiro@socionext.com> writes:
>
<snip>
>
> (Case 3)
> Compiler flag -foo is sensitive to endian-ness.
>
>
> config CC_NEEDS_BIG_ENDIAN
> def_bool $(cc-option -mbig-endian) && CPU_BIG_ENDIAN
>
> config CC_NEEDS_LITTLE_ENDIAN
> def_bool $(cc-option -mlittle-endian) && CPU_LITTLE_ENDIAN
>
> config CC_HAS_FOO
> bool
> default $(cc-option -mbig-endian -foo) if CC_NEEDS_BIG_ENDIAN
> default $(cc-option -mlittle-endian -foo) if CC_NEEDS_LITTLE_ENDIAN
> default $(cc-option -foo)
We may do something like this on powerpc, where we have 32/64-bit and
big/little endian (on 64-bit) and then some ABI options that we
set/unset depending on endian.
The above looks like it could work though.
cheers
^ permalink raw reply
* Re: [PATCH] bpf, powerpc: fix jit for seccomp_data access
From: Michael Ellerman @ 2018-02-22 3:34 UTC (permalink / raw)
To: Daniel Borkmann, Mark Lord, Naveen N. Rao, Alexei Starovoitov,
Kees Cook, Andy Lutomirski, Will Drewry
Cc: linuxppc-dev
In-Reply-To: <03bbd3d6-3f67-795a-afa3-2feee367991a@iogearbox.net>
Daniel Borkmann <daniel@iogearbox.net> writes:
> On 02/21/2018 01:33 AM, Michael Ellerman wrote:
>> Mark Lord <mlord@pobox.com> writes:
>>
>>> I am using SECCOMP to filter syscalls on a ppc32 platform,
>>> and noticed that the JIT compiler was failing on the BPF
>>> even though the interpreter was working fine.
>>>
>>> The issue was that the compiler was missing one of the instructions
>>> used by SECCOMP, so here is a patch to enable JIT for that instruction.
>>>
>>> Signed-Off-By: Mark Lord <mlord@pobox.com>
>>
>> Thanks.
>>
>> What is the failure mode without this patch? I assume when you have the
>> JIT enabled the seccomp filter fails to load entirely? Or do we have
>> logic to fall back to the interpreter?
>
> The logic for all JITs is that once a BPF insn cannot be JITed then it falls
> back to BPF interpreter. Here, since ppc32 is cBPF the path is: cBPF insn ->
> cBPF JIT -> fail -> migrate cBPF to eBPF -> run in eBPF interpreter. In the
> case where interpreter is compiled out (CONFIG_BPF_JIT_ALWAYS_ON), then the
> filter is rejected.
OK thanks.
>> Either way we should probably back port to stable. I assume this has
>> been broken since we originally added 32-bit support, so:
>
> Note that arm32 before it was converted to be an eBPF JIT (eBPF JITs do handle
> seccomp BPF in any case) was the only cBPF JIT that had it 'JIT-able', so
> currently, other cBPF ones like sparc32 or mips32 don't translate it either.
> Meaning, it would be nice to have as feature; I wouldn't necessarily frame
> it as a bug though (or at least a stable-urgent one, imho, but I leave that
> to you, of course).
OK, I'll just add the Fixes tag for our reference and we can always back
port it to stable later if we want to.
cheers
^ permalink raw reply
* Re: [PATCH] bpf, powerpc: fix jit for seccomp_data access
From: Michael Ellerman @ 2018-02-22 3:38 UTC (permalink / raw)
To: Naveen N. Rao, Mark Lord
Cc: Alexei Starovoitov, Daniel Borkmann, Kees Cook, linuxppc-dev,
Andy Lutomirski, Will Drewry
In-Reply-To: <1519221778.inx201t3s9.naveen@linux.ibm.com>
"Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> writes:
> Mark Lord wrote:
>> On 18-02-21 07:52 AM, Mark Lord wrote:
>>> On 18-02-21 03:35 AM, Naveen N. Rao wrote:
>> ..
>>>> Looks good to me, but I am not able to apply this patch. There seems to be whitespace damage.
>>>
>>> Here (attached) is a clean copy.
>>
>> Again, this time with the commit message included!
>
> Thanks. However...
> I am able to apply this using 'patch', but not with 'git am' since the
> headers are missing. FWIW, the usual workflow is to make the changes and
> commit it into your repository using 'git commit' and then use 'git
> format-patch' to generate a patch file that you can then post.
>
> I'll defer to Michael on whether he is ok to process this as it is.
The main thing is that it's caught by patchwork[1], otherwise I tend to
miss it. In this case the initial patch was caught by patchwork, so
that's fine.
I fixed up the white space and other issues before applying.
In general I'm happy to do that for new/infrequent committers, not so
much for people who send lots of patches and/or are paid to do so :)
cheers
1: http://patchwork.ozlabs.org/patch/875890/
^ permalink raw reply
* Re: [PATCH] ocxl: Add get_metadata IOCTL to share OCXL information to userspace
From: Balbir Singh @ 2018-02-22 3:41 UTC (permalink / raw)
To: Alastair D'Silva
Cc: linuxppc-dev, linux-kernel@vger.kernel.org, Arnd Bergmann,
frederic.barrat, Greg KH, Andrew Donnellan
In-Reply-To: <1519255934.2867.3.camel@au1.ibm.com>
On Thu, Feb 22, 2018 at 10:32 AM, Alastair D'Silva <alastair@au1.ibm.com> wrote:
>
> On Wed, 2018-02-21 at 17:43 +1100, Balbir Singh wrote:
>> On Wed, Feb 21, 2018 at 3:57 PM, Alastair D'Silva <alastair@au1.ibm.c
>> om> wrote:
>> > From: Alastair D'Silva <alastair@d-silva.org>
>> >
>> > Some required information is not exposed to userspace currently
>> > (eg. the
>> > PASID), pass this information back, along with other information
>> > which
>> > is currently communicated via sysfs, which saves some parsing
>> > effort in
>> > userspace.
>> >
>> > Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
>> > ---
>> > drivers/misc/ocxl/file.c | 27 +++++++++++++++++++++++++++
>> > include/uapi/misc/ocxl.h | 22 ++++++++++++++++++++++
>> > 2 files changed, 49 insertions(+)
>> >
>> > diff --git a/drivers/misc/ocxl/file.c b/drivers/misc/ocxl/file.c
>> > index d9aa407db06a..11514a8444e5 100644
>> > --- a/drivers/misc/ocxl/file.c
>> > +++ b/drivers/misc/ocxl/file.c
>> > @@ -102,10 +102,32 @@ static long afu_ioctl_attach(struct
>> > ocxl_context *ctx,
>> > return rc;
>> > }
>> >
>> > +static long afu_ioctl_get_metadata(struct ocxl_context *ctx,
>> > + struct ocxl_ioctl_get_metadata __user *uarg)
>>
>> Why do we call this metadata? Isn't this an afu_descriptor?
>>
>
> It's metadata for the descriptor.
I meant metadata is too generic, could we have other types of metadata in OCXL?
>
>> > +{
>> > + struct ocxl_ioctl_get_metadata arg;
>> > +
>> > + memset(&arg, 0, sizeof(arg));
>> > +
>> > + arg.version = 0;
>>
>> Does it make sense to have version 0? Even if does, you can afford
>> to skip initialization due to the memset above. I prefer that
>> versions
>> start with 1
>>
>
> Setting it to 0 is for the reader, not the compiler. I'm not clear on
> the benefit of starting the version at 1, could you clarify?
How do I distinguish between version number never set and 0?
>
>> > +
>> > + arg.afu_version_major = ctx->afu->config.version_major;
>> > + arg.afu_version_minor = ctx->afu->config.version_minor;
>> > + arg.pasid = ctx->pasid;
>> > + arg.pp_mmio_size = ctx->afu->config.pp_mmio_stride;
>> > + arg.global_mmio_size = ctx->afu->config.global_mmio_size;
>> > +
>> > + if (copy_to_user(uarg, &arg, sizeof(arg)))
>> > + return -EFAULT;
>> > +
>> > + return 0;
>> > +}
>> > +
>> > #define CMD_STR(x) (x == OCXL_IOCTL_ATTACH ? "ATTACH"
>> > : \
>> > x == OCXL_IOCTL_IRQ_ALLOC ? "IRQ_ALLOC"
>> > : \
>> > x == OCXL_IOCTL_IRQ_FREE ? "IRQ_FREE"
>> > : \
>> > x == OCXL_IOCTL_IRQ_SET_FD ? "IRQ_SET_FD"
>> > : \
>> > + x == OCXL_IOCTL_GET_METADATA ?
>> > "GET_METADATA" : \
>> > "UNKNOWN")
>> >
>> > static long afu_ioctl(struct file *file, unsigned int cmd,
>> > @@ -157,6 +179,11 @@ static long afu_ioctl(struct file *file,
>> > unsigned int cmd,
>> > irq_fd.eventfd);
>> > break;
>> >
>> > + case OCXL_IOCTL_GET_METADATA:
>> > + rc = afu_ioctl_get_metadata(ctx,
>> > + (struct ocxl_ioctl_get_metadata
>> > __user *) args);
>> > + break;
>> > +
>> > default:
>> > rc = -EINVAL;
>> > }
>> > diff --git a/include/uapi/misc/ocxl.h b/include/uapi/misc/ocxl.h
>> > index 4b0b0b756f3e..16e1f48ce280 100644
>> > --- a/include/uapi/misc/ocxl.h
>> > +++ b/include/uapi/misc/ocxl.h
>> > @@ -32,6 +32,27 @@ struct ocxl_ioctl_attach {
>> > __u64 reserved3;
>> > };
>> >
>> > +/*
>> > + * Version contains the version of the struct.
>> > + * Versions will always be backwards compatible, that is, new
>> > versions will not
>> > + * alter existing fields
>> > + */
>> > +struct ocxl_ioctl_get_metadata {
>>
>> This sounds more like a function name, do we need it to be
>> _get_metdata?
>>
>
> It pretty much is a function, it returns to userspace metadata about
> the descriptor being operated on.
>
It has a verb indicating action
>> > + __u16 version;
>> > +
>> > + // Version 0 fields
>> > + __u8 afu_version_major;
>> > + __u8 afu_version_minor;
>> > + __u32 pasid;
>> > +
>> > + __u64 pp_mmio_size;
>> > + __u64 global_mmio_size;
>> > +
>>
>> Should we document the fields? pp_ stands for per process, but is not
>> very clear at first look. Why do we care to return only the size,
>> what
>> about lpc size?
>>
>
> Yes, I would rather call it per_pasid_mmio_size, but consistency with
> the rest of the driver (& exposed sysfs entries) is also important.
>
Balbir Singh.
^ permalink raw reply
* Re: [PATCH] ocxl: Add get_metadata IOCTL to share OCXL information to userspace
From: Balbir Singh @ 2018-02-22 3:46 UTC (permalink / raw)
To: Frederic Barrat
Cc: Alastair D'Silva, Arnd Bergmann, frederic.barrat, Greg KH,
linux-kernel@vger.kernel.org, linuxppc-dev, Andrew Donnellan,
Alastair D'Silva
In-Reply-To: <c6017f9f-67f0-10eb-1eed-527c9c5298ba@linux.vnet.ibm.com>
On Wed, Feb 21, 2018 at 10:25 PM, Frederic Barrat
<fbarrat@linux.vnet.ibm.com> wrote:
>
>
> Le 21/02/2018 =C3=A0 07:43, Balbir Singh a =C3=A9crit :
>>
>> On Wed, Feb 21, 2018 at 3:57 PM, Alastair D'Silva <alastair@au1.ibm.com>
>> wrote:
>>>
>>> From: Alastair D'Silva <alastair@d-silva.org>
>>>
>>> Some required information is not exposed to userspace currently (eg. th=
e
>>> PASID), pass this information back, along with other information which
>>> is currently communicated via sysfs, which saves some parsing effort in
>>> userspace.
>>>
>>> Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
>>> ---
>>> drivers/misc/ocxl/file.c | 27 +++++++++++++++++++++++++++
>>> include/uapi/misc/ocxl.h | 22 ++++++++++++++++++++++
>>> 2 files changed, 49 insertions(+)
>>>
>>> diff --git a/drivers/misc/ocxl/file.c b/drivers/misc/ocxl/file.c
>>> index d9aa407db06a..11514a8444e5 100644
>>> --- a/drivers/misc/ocxl/file.c
>>> +++ b/drivers/misc/ocxl/file.c
>>> @@ -102,10 +102,32 @@ static long afu_ioctl_attach(struct ocxl_context
>>> *ctx,
>>> return rc;
>>> }
>>>
>>> +static long afu_ioctl_get_metadata(struct ocxl_context *ctx,
>>> + struct ocxl_ioctl_get_metadata __user *uarg)
>>
>>
>> Why do we call this metadata? Isn't this an afu_descriptor?
>>
>>> +{
>>> + struct ocxl_ioctl_get_metadata arg;
>>> +
>>> + memset(&arg, 0, sizeof(arg));
>>> +
>>> + arg.version =3D 0;
>>
>>
>> Does it make sense to have version 0? Even if does, you can afford
>> to skip initialization due to the memset above. I prefer that versions
>> start with 1
>>
>>> +
>>> + arg.afu_version_major =3D ctx->afu->config.version_major;
>>> + arg.afu_version_minor =3D ctx->afu->config.version_minor;
>>> + arg.pasid =3D ctx->pasid;
>>> + arg.pp_mmio_size =3D ctx->afu->config.pp_mmio_stride;
>>> + arg.global_mmio_size =3D ctx->afu->config.global_mmio_size;
>>> +
>>> + if (copy_to_user(uarg, &arg, sizeof(arg)))
>>> + return -EFAULT;
>>> +
>>> + return 0;
>>> +}
>>> +
>>> #define CMD_STR(x) (x =3D=3D OCXL_IOCTL_ATTACH ? "ATTACH" :
>>> \
>>> x =3D=3D OCXL_IOCTL_IRQ_ALLOC ? "IRQ_ALLOC" :
>>> \
>>> x =3D=3D OCXL_IOCTL_IRQ_FREE ? "IRQ_FREE" :
>>> \
>>> x =3D=3D OCXL_IOCTL_IRQ_SET_FD ? "IRQ_SET_FD" =
:
>>> \
>>> + x =3D=3D OCXL_IOCTL_GET_METADATA ? "GET_METADAT=
A" : \
>>> "UNKNOWN")
>>>
>>> static long afu_ioctl(struct file *file, unsigned int cmd,
>>> @@ -157,6 +179,11 @@ static long afu_ioctl(struct file *file, unsigned
>>> int cmd,
>>> irq_fd.eventfd);
>>> break;
>>>
>>> + case OCXL_IOCTL_GET_METADATA:
>>> + rc =3D afu_ioctl_get_metadata(ctx,
>>> + (struct ocxl_ioctl_get_metadata __user =
*)
>>> args);
>>> + break;
>>> +
>>> default:
>>> rc =3D -EINVAL;
>>> }
>>> diff --git a/include/uapi/misc/ocxl.h b/include/uapi/misc/ocxl.h
>>> index 4b0b0b756f3e..16e1f48ce280 100644
>>> --- a/include/uapi/misc/ocxl.h
>>> +++ b/include/uapi/misc/ocxl.h
>>> @@ -32,6 +32,27 @@ struct ocxl_ioctl_attach {
>>> __u64 reserved3;
>>> };
>>>
>>> +/*
>>> + * Version contains the version of the struct.
>>> + * Versions will always be backwards compatible, that is, new versions
>>> will not
>>> + * alter existing fields
>>> + */
>>> +struct ocxl_ioctl_get_metadata {
>>
>>
>> This sounds more like a function name, do we need it to be _get_metdata?
>>
>>> + __u16 version;
>>> +
>>> + // Version 0 fields
>>> + __u8 afu_version_major;
>>> + __u8 afu_version_minor;
>>> + __u32 pasid;
>>> +
>>> + __u64 pp_mmio_size;
>>> + __u64 global_mmio_size;
>>> +
>>
>>
>> Should we document the fields? pp_ stands for per process, but is not
>> very clear at first look. Why do we care to return only the size, what
>> about lpc size?
>
>
> My bad, I forgot to mention it before. There's a somewhat high-level
> description which needs updating in:
> Documentation/accelerators/ocxl.rst
Thanks, that's helpful
>
> It doesn't go down to the level of the structure members, but at least al=
l
> ioctl commands should have a brief description.
>
> lpc_size could be added. It's currently useless to the library, but doesn=
't
> hurt. The one which was giving me troubles on a previous version of this
> patch was the lpc numa node ID, since that was experimental code and felt
> out of place considering what's been upstreamed in skiboot and linux so f=
ar.
>
Yeah, I think metadata will evolve for a while till it settle's down.
Since ocxl_ioctl_get_metadata is exposed via uapi, a newer program
calling an older kernel will never work, since the size of that struct
will always be larger than what the OS supports and our copy_to_user()
will fail. The other option is for the user program to try all
possible versions till one succeeds, that is bad as well. I think
there are a few ways around it, if we care about this combination.
Balbir Singh.
^ permalink raw reply
* Re: [PATCH] ocxl: Add get_metadata IOCTL to share OCXL information to userspace
From: Andrew Donnellan @ 2018-02-22 3:47 UTC (permalink / raw)
To: Balbir Singh, Alastair D'Silva
Cc: linuxppc-dev, linux-kernel@vger.kernel.org, Arnd Bergmann,
frederic.barrat, Greg KH
In-Reply-To: <CAKTCnzmmUGPNk1ggpWYQjiDLcAR_c7cqdZyPU+6rKGC6AcNW7A@mail.gmail.com>
On 22/02/18 14:41, Balbir Singh wrote:
>> Setting it to 0 is for the reader, not the compiler. I'm not clear on
>> the benefit of starting the version at 1, could you clarify?
>
> How do I distinguish between version number never set and 0?
The ioctl won't exist without a version number set.
--
Andrew Donnellan OzLabs, ADL Canberra
andrew.donnellan@au1.ibm.com IBM Australia Limited
^ permalink raw reply
* Re: [PATCH] ocxl: Add get_metadata IOCTL to share OCXL information to userspace
From: Alastair D'Silva @ 2018-02-22 3:48 UTC (permalink / raw)
To: Balbir Singh
Cc: linuxppc-dev, linux-kernel@vger.kernel.org, Arnd Bergmann,
frederic.barrat, Greg KH, Andrew Donnellan
In-Reply-To: <CAKTCnzmmUGPNk1ggpWYQjiDLcAR_c7cqdZyPU+6rKGC6AcNW7A@mail.gmail.com>
On Thu, 2018-02-22 at 14:41 +1100, Balbir Singh wrote:
> On Thu, Feb 22, 2018 at 10:32 AM, Alastair D'Silva <alastair@au1.ibm.
> com> wrote:
> >
> > On Wed, 2018-02-21 at 17:43 +1100, Balbir Singh wrote:
> > > On Wed, Feb 21, 2018 at 3:57 PM, Alastair D'Silva <alastair@au1.i
> > > bm.c
> > > om> wrote:
> > > > From: Alastair D'Silva <alastair@d-silva.org>
> > > >
> > > > Some required information is not exposed to userspace currently
> > > > (eg. the
> > > > PASID), pass this information back, along with other
> > > > information
> > > > which
> > > > is currently communicated via sysfs, which saves some parsing
> > > > effort in
> > > > userspace.
> > > >
> > > > Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> > > > ---
> > > > drivers/misc/ocxl/file.c | 27 +++++++++++++++++++++++++++
> > > > include/uapi/misc/ocxl.h | 22 ++++++++++++++++++++++
> > > > 2 files changed, 49 insertions(+)
> > > >
> > > > diff --git a/drivers/misc/ocxl/file.c
> > > > b/drivers/misc/ocxl/file.c
> > > > index d9aa407db06a..11514a8444e5 100644
> > > > --- a/drivers/misc/ocxl/file.c
> > > > +++ b/drivers/misc/ocxl/file.c
> > > > @@ -102,10 +102,32 @@ static long afu_ioctl_attach(struct
> > > > ocxl_context *ctx,
> > > > return rc;
> > > > }
> > > >
> > > > +static long afu_ioctl_get_metadata(struct ocxl_context *ctx,
> > > > + struct ocxl_ioctl_get_metadata __user *uarg)
> > >
> > > Why do we call this metadata? Isn't this an afu_descriptor?
> > >
> >
> > It's metadata for the descriptor.
>
> I meant metadata is too generic, could we have other types of
> metadata in OCXL?
>
I don't believe so, we would instead expand the scope of this IOCTL
using version & space available from the reserved fields.
> >
> > > > +{
> > > > + struct ocxl_ioctl_get_metadata arg;
> > > > +
> > > > + memset(&arg, 0, sizeof(arg));
> > > > +
> > > > + arg.version = 0;
> > >
> > > Does it make sense to have version 0? Even if does, you can
> > > afford
> > > to skip initialization due to the memset above. I prefer that
> > > versions
> > > start with 1
> > >
> >
> > Setting it to 0 is for the reader, not the compiler. I'm not clear
> > on
> > the benefit of starting the version at 1, could you clarify?
>
> How do I distinguish between version number never set and 0?
>
The version number is always set. If the IOCTL doesn't exist, the ioctl
call will error instead.
> >
> > > > +
> > > > + arg.afu_version_major = ctx->afu->config.version_major;
> > > > + arg.afu_version_minor = ctx->afu->config.version_minor;
> > > > + arg.pasid = ctx->pasid;
> > > > + arg.pp_mmio_size = ctx->afu->config.pp_mmio_stride;
> > > > + arg.global_mmio_size = ctx->afu-
> > > > >config.global_mmio_size;
> > > > +
> > > > + if (copy_to_user(uarg, &arg, sizeof(arg)))
> > > > + return -EFAULT;
> > > > +
> > > > + return 0;
> > > > +}
> > > > +
> > > > #define CMD_STR(x) (x == OCXL_IOCTL_ATTACH ? "ATTACH"
> > > > : \
> > > > x == OCXL_IOCTL_IRQ_ALLOC ? "IRQ_ALLOC"
> > > > : \
> > > > x == OCXL_IOCTL_IRQ_FREE ? "IRQ_FREE"
> > > > : \
> > > > x == OCXL_IOCTL_IRQ_SET_FD ?
> > > > "IRQ_SET_FD"
> > > > : \
> > > > + x == OCXL_IOCTL_GET_METADATA ?
> > > > "GET_METADATA" : \
> > > > "UNKNOWN")
> > > >
> > > > static long afu_ioctl(struct file *file, unsigned int cmd,
> > > > @@ -157,6 +179,11 @@ static long afu_ioctl(struct file *file,
> > > > unsigned int cmd,
> > > > irq_fd.eventfd);
> > > > break;
> > > >
> > > > + case OCXL_IOCTL_GET_METADATA:
> > > > + rc = afu_ioctl_get_metadata(ctx,
> > > > + (struct ocxl_ioctl_get_metadata
> > > > __user *) args);
> > > > + break;
> > > > +
> > > > default:
> > > > rc = -EINVAL;
> > > > }
> > > > diff --git a/include/uapi/misc/ocxl.h
> > > > b/include/uapi/misc/ocxl.h
> > > > index 4b0b0b756f3e..16e1f48ce280 100644
> > > > --- a/include/uapi/misc/ocxl.h
> > > > +++ b/include/uapi/misc/ocxl.h
> > > > @@ -32,6 +32,27 @@ struct ocxl_ioctl_attach {
> > > > __u64 reserved3;
> > > > };
> > > >
> > > > +/*
> > > > + * Version contains the version of the struct.
> > > > + * Versions will always be backwards compatible, that is, new
> > > > versions will not
> > > > + * alter existing fields
> > > > + */
> > > > +struct ocxl_ioctl_get_metadata {
> > >
> > > This sounds more like a function name, do we need it to be
> > > _get_metdata?
> > >
> >
> > It pretty much is a function, it returns to userspace metadata
> > about
> > the descriptor being operated on.
> >
>
> It has a verb indicating action
I misunderstood, I had named the struct to match the IOCTL, but that
isn't necessary. I'll update it in the next patch.
--
Alastair D'Silva
Open Source Developer
Linux Technology Centre, IBM Australia
mob: 0423 762 819
^ permalink raw reply
* Re: [PATCH] ocxl: Add get_metadata IOCTL to share OCXL information to userspace
From: Alastair D'Silva @ 2018-02-22 3:51 UTC (permalink / raw)
To: Balbir Singh, Frederic Barrat
Cc: Arnd Bergmann, frederic.barrat, Greg KH,
linux-kernel@vger.kernel.org, linuxppc-dev, Andrew Donnellan
In-Reply-To: <CAKTCnzkwwmbU4vzrmzW=eNnr2QW+rj=OUcAdOVxz+qKGC=9Q9A@mail.gmail.com>
On Thu, 2018-02-22 at 14:46 +1100, Balbir Singh wrote:
<snip>
> lpc_size could be added. It's currently useless to the library, but
> > doesn't
> > hurt. The one which was giving me troubles on a previous version of
> > this
> > patch was the lpc numa node ID, since that was experimental code
> > and felt
> > out of place considering what's been upstreamed in skiboot and
> > linux so far.
> >
>
> Yeah, I think metadata will evolve for a while till it settle's down.
> Since ocxl_ioctl_get_metadata is exposed via uapi, a newer program
> calling an older kernel will never work, since the size of that
> struct
> will always be larger than what the OS supports and our
> copy_to_user()
> will fail. The other option is for the user program to try all
> possible versions till one succeeds, that is bad as well. I think
> there are a few ways around it, if we care about this combination.
>
> Balbir Singh.
>
We have a number of reserved members at the end of the struct which can
be re-purposed for future information (with a corresponding bump of the
version number).
--
Alastair D'Silva
Open Source Developer
Linux Technology Centre, IBM Australia
mob: 0423 762 819
^ permalink raw reply
* [PATCH v3 0/2] Add get_metadata IOCTL to share OCXL information to userspace
From: Alastair D'Silva @ 2018-02-22 4:17 UTC (permalink / raw)
To: linuxppc-dev, linux-kernel
Cc: frederic.barrat, andrew.donnellan, arnd, gregkh, alastair,
Alastair D'Silva
In-Reply-To: <20180222030334.14060-1-alastair@au1.ibm.com>
From: Alastair D'Silva <alastair@d-silva.org>
This series adds a new IOCTL for OpenCAPI, which exposes adapter
metadata to userspace.
---
v1->v2:
- Document the new IOCTL
v2->v3:
- Rename metadata struct
- Document a couple of struct members that are not immediately obvious
Alastair D'Silva (2):
ocxl: Add get_metadata IOCTL to share OCXL information to userspace
ocxl: Document the OCXL_IOCTL_GET_METADATA IOCTL
Documentation/accelerators/ocxl.rst | 5 +++++
drivers/misc/ocxl/file.c | 27 +++++++++++++++++++++++++++
include/uapi/misc/ocxl.h | 17 +++++++++++++++++
3 files changed, 49 insertions(+)
--
2.14.3
^ permalink raw reply
* [PATCH v3 2/2] ocxl: Document the OCXL_IOCTL_GET_METADATA IOCTL
From: Alastair D'Silva @ 2018-02-22 4:17 UTC (permalink / raw)
To: linuxppc-dev, linux-kernel
Cc: frederic.barrat, andrew.donnellan, arnd, gregkh, alastair,
Alastair D'Silva
In-Reply-To: <20180222041739.27899-1-alastair@au1.ibm.com>
From: Alastair D'Silva <alastair@d-silva.org>
Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
---
Documentation/accelerators/ocxl.rst | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/Documentation/accelerators/ocxl.rst b/Documentation/accelerators/ocxl.rst
index 4f7af841d935..ddcc58d01cfb 100644
--- a/Documentation/accelerators/ocxl.rst
+++ b/Documentation/accelerators/ocxl.rst
@@ -152,6 +152,11 @@ OCXL_IOCTL_IRQ_SET_FD:
Associate an event fd to an AFU interrupt so that the user process
can be notified when the AFU sends an interrupt.
+OCXL_IOCTL_GET_METADATA:
+
+ Obtains configuration information from the card, such at the size of
+ MMIO areas, the AFU version, and the PASID for the current context.
+
mmap
----
--
2.14.3
^ permalink raw reply related
* [PATCH v3 1/2] ocxl: Add get_metadata IOCTL to share OCXL information to userspace
From: Alastair D'Silva @ 2018-02-22 4:17 UTC (permalink / raw)
To: linuxppc-dev, linux-kernel
Cc: frederic.barrat, andrew.donnellan, arnd, gregkh, alastair,
Alastair D'Silva
In-Reply-To: <20180222041739.27899-1-alastair@au1.ibm.com>
From: Alastair D'Silva <alastair@d-silva.org>
Some required information is not exposed to userspace currently (eg. the
PASID), pass this information back, along with other information which
is currently communicated via sysfs, which saves some parsing effort in
userspace.
Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
---
drivers/misc/ocxl/file.c | 27 +++++++++++++++++++++++++++
include/uapi/misc/ocxl.h | 17 +++++++++++++++++
2 files changed, 44 insertions(+)
diff --git a/drivers/misc/ocxl/file.c b/drivers/misc/ocxl/file.c
index d9aa407db06a..90df1be5ef3f 100644
--- a/drivers/misc/ocxl/file.c
+++ b/drivers/misc/ocxl/file.c
@@ -102,10 +102,32 @@ static long afu_ioctl_attach(struct ocxl_context *ctx,
return rc;
}
+static long afu_ioctl_get_metadata(struct ocxl_context *ctx,
+ struct ocxl_ioctl_metadata __user *uarg)
+{
+ struct ocxl_ioctl_metadata arg;
+
+ memset(&arg, 0, sizeof(arg));
+
+ arg.version = 0;
+
+ arg.afu_version_major = ctx->afu->config.version_major;
+ arg.afu_version_minor = ctx->afu->config.version_minor;
+ arg.pasid = ctx->pasid;
+ arg.pp_mmio_size = ctx->afu->config.pp_mmio_stride;
+ arg.global_mmio_size = ctx->afu->config.global_mmio_size;
+
+ if (copy_to_user(uarg, &arg, sizeof(arg)))
+ return -EFAULT;
+
+ return 0;
+}
+
#define CMD_STR(x) (x == OCXL_IOCTL_ATTACH ? "ATTACH" : \
x == OCXL_IOCTL_IRQ_ALLOC ? "IRQ_ALLOC" : \
x == OCXL_IOCTL_IRQ_FREE ? "IRQ_FREE" : \
x == OCXL_IOCTL_IRQ_SET_FD ? "IRQ_SET_FD" : \
+ x == OCXL_IOCTL_GET_METADATA ? "GET_METADATA" : \
"UNKNOWN")
static long afu_ioctl(struct file *file, unsigned int cmd,
@@ -157,6 +179,11 @@ static long afu_ioctl(struct file *file, unsigned int cmd,
irq_fd.eventfd);
break;
+ case OCXL_IOCTL_GET_METADATA:
+ rc = afu_ioctl_get_metadata(ctx,
+ (struct ocxl_ioctl_metadata __user *) args);
+ break;
+
default:
rc = -EINVAL;
}
diff --git a/include/uapi/misc/ocxl.h b/include/uapi/misc/ocxl.h
index 4b0b0b756f3e..0af83d80fb3e 100644
--- a/include/uapi/misc/ocxl.h
+++ b/include/uapi/misc/ocxl.h
@@ -32,6 +32,22 @@ struct ocxl_ioctl_attach {
__u64 reserved3;
};
+struct ocxl_ioctl_metadata {
+ __u16 version; // struct version, always backwards compatible
+
+ // Version 0 fields
+ __u8 afu_version_major;
+ __u8 afu_version_minor;
+ __u32 pasid; // PASID assigned to the current context
+
+ __u64 pp_mmio_size; // Per PASID MMIO size
+ __u64 global_mmio_size;
+
+ // End version 0 fields
+
+ __u64 reserved[13]; // Total of 16*u64
+};
+
struct ocxl_ioctl_irq_fd {
__u64 irq_offset;
__s32 eventfd;
@@ -45,5 +61,6 @@ struct ocxl_ioctl_irq_fd {
#define OCXL_IOCTL_IRQ_ALLOC _IOR(OCXL_MAGIC, 0x11, __u64)
#define OCXL_IOCTL_IRQ_FREE _IOW(OCXL_MAGIC, 0x12, __u64)
#define OCXL_IOCTL_IRQ_SET_FD _IOW(OCXL_MAGIC, 0x13, struct ocxl_ioctl_irq_fd)
+#define OCXL_IOCTL_GET_METADATA _IOR(OCXL_MAGIC, 0x14, struct ocxl_ioctl_metadata)
#endif /* _UAPI_MISC_OCXL_H */
--
2.14.3
^ permalink raw reply related
* Re: [PATCH 21/23] gcc-plugins: move GCC version check for PowerPC to Kconfig
From: Andrew Donnellan @ 2018-02-22 5:04 UTC (permalink / raw)
To: Masahiro Yamada, linux-kbuild, Linus Torvalds
Cc: Michal Marek, Ulf Magnusson, Kees Cook, Arnd Bergmann,
kernel-hardening, Greg Kroah-Hartman, Randy Dunlap, linux-kernel,
Emese Revfy, Paul Mackerras, linuxppc-dev, Sam Ravnborg
In-Reply-To: <1518806331-7101-22-git-send-email-yamada.masahiro@socionext.com>
On 17/02/18 05:38, Masahiro Yamada wrote:
> For PowerPC, GCC 5.2 is the requirement for GCC plugins. Move the
> version check to Kconfig, and remove the ugly checker.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
The old checker was as non-ugly as I could make it. :)
Acked-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
--
Andrew Donnellan OzLabs, ADL Canberra
andrew.donnellan@au1.ibm.com IBM Australia Limited
^ permalink raw reply
* Re: [PATCH v3 1/2] ocxl: Add get_metadata IOCTL to share OCXL information to userspace
From: Andrew Donnellan @ 2018-02-22 5:21 UTC (permalink / raw)
To: Alastair D'Silva, linuxppc-dev, linux-kernel
Cc: frederic.barrat, arnd, gregkh, Alastair D'Silva
In-Reply-To: <20180222041739.27899-2-alastair@au1.ibm.com>
On 22/02/18 15:17, Alastair D'Silva wrote:
> From: Alastair D'Silva <alastair@d-silva.org>
>
> Some required information is not exposed to userspace currently (eg. the
> PASID), pass this information back, along with other information which
> is currently communicated via sysfs, which saves some parsing effort in
> userspace.
>
> Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
Acked-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
--
Andrew Donnellan OzLabs, ADL Canberra
andrew.donnellan@au1.ibm.com IBM Australia Limited
^ permalink raw reply
* Re: [PATCH v3 2/2] ocxl: Document the OCXL_IOCTL_GET_METADATA IOCTL
From: Andrew Donnellan @ 2018-02-22 5:21 UTC (permalink / raw)
To: Alastair D'Silva, linuxppc-dev, linux-kernel
Cc: frederic.barrat, arnd, gregkh, Alastair D'Silva
In-Reply-To: <20180222041739.27899-3-alastair@au1.ibm.com>
On 22/02/18 15:17, Alastair D'Silva wrote:
> From: Alastair D'Silva <alastair@d-silva.org>
>
> Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
Acked-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
--
Andrew Donnellan OzLabs, ADL Canberra
andrew.donnellan@au1.ibm.com IBM Australia Limited
^ permalink raw reply
* Re: [PATCH] ocxl: Add get_metadata IOCTL to share OCXL information to userspace
From: Balbir Singh @ 2018-02-22 5:32 UTC (permalink / raw)
To: Alastair D'Silva
Cc: Frederic Barrat, Arnd Bergmann, frederic.barrat, Greg KH,
linux-kernel@vger.kernel.org, linuxppc-dev, Andrew Donnellan
In-Reply-To: <1519271512.2867.14.camel@au1.ibm.com>
On Thu, Feb 22, 2018 at 2:51 PM, Alastair D'Silva <alastair@au1.ibm.com> wrote:
> On Thu, 2018-02-22 at 14:46 +1100, Balbir Singh wrote:
> <snip>
>> lpc_size could be added. It's currently useless to the library, but
>> > doesn't
>> > hurt. The one which was giving me troubles on a previous version of
>> > this
>> > patch was the lpc numa node ID, since that was experimental code
>> > and felt
>> > out of place considering what's been upstreamed in skiboot and
>> > linux so far.
>> >
>>
>> Yeah, I think metadata will evolve for a while till it settle's down.
>> Since ocxl_ioctl_get_metadata is exposed via uapi, a newer program
>> calling an older kernel will never work, since the size of that
>> struct
>> will always be larger than what the OS supports and our
>> copy_to_user()
>> will fail. The other option is for the user program to try all
>> possible versions till one succeeds, that is bad as well. I think
>> there are a few ways around it, if we care about this combination.
>>
>> Balbir Singh.
>>
>
> We have a number of reserved members at the end of the struct which can
> be re-purposed for future information (with a corresponding bump of the
> version number).
Good point, agreed
Balbir Singh.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox