Re: [PATCH v2 3/7] powerpc: use task_pid_nr() for TID allocation
On 25/04/18 07:12, Sukadev Bhattiprolu wrote: Yes. Like with PIDR, was trying to assign TIDR initially to all threads. But since only a subset of threads need/use TIDR, we can assign the value later (when set_thread_tidr() is called). So we should be able to use task_pid_nr() then. OK. Alastair has also confirmed with me that truncating the pid to a u16 should be safe, so therefore: Reviewed-by: Andrew Donnellan <andrew.donnel...@au1.ibm.com> -- Andrew Donnellan OzLabs, ADL Canberra andrew.donnel...@au1.ibm.com IBM Australia Limited -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 5/7] ocxl: Expose the thread_id needed for wait on p9
On 18/04/18 11:08, Alastair D'Silva wrote:
From: Alastair D'Silva <alast...@d-silva.org>
In order to successfully issue as_notify, an AFU needs to know the TID
to notify, which in turn means that this information should be
available in userspace so it can be communicated to the AFU.
Signed-off-by: Alastair D'Silva <alast...@d-silva.org>
nitpicks below
Acked-by: Andrew Donnellan <andrew.donnel...@au1.ibm.com>
---
drivers/misc/ocxl/context.c | 5 +++-
drivers/misc/ocxl/file.c | 53 +++
drivers/misc/ocxl/link.c | 36 ++
drivers/misc/ocxl/ocxl_internal.h | 1 +
include/misc/ocxl.h | 9 +++
include/uapi/misc/ocxl.h | 10
6 files changed, 113 insertions(+), 1 deletion(-)
diff --git a/drivers/misc/ocxl/context.c b/drivers/misc/ocxl/context.c
index 909e8807824a..95f74623113e 100644
--- a/drivers/misc/ocxl/context.c
+++ b/drivers/misc/ocxl/context.c
@@ -34,6 +34,8 @@ int ocxl_context_init(struct ocxl_context *ctx, struct
ocxl_afu *afu,
mutex_init(>xsl_error_lock);
mutex_init(>irq_lock);
idr_init(>irq_idr);
+ ctx->tidr = 0;
+
/*
* Keep a reference on the AFU to make sure it's valid for the
* duration of the life of the context
@@ -65,6 +67,7 @@ int ocxl_context_attach(struct ocxl_context *ctx, u64 amr)
{
int rc;
+ // Locks both status & tidr
mutex_lock(>status_mutex);
if (ctx->status != OPENED) {
rc = -EIO;
@@ -72,7 +75,7 @@ int ocxl_context_attach(struct ocxl_context *ctx, u64 amr)
}
rc = ocxl_link_add_pe(ctx->afu->fn->link, ctx->pasid,
- current->mm->context.id, 0, amr, current->mm,
+ current->mm->context.id, ctx->tidr, amr, current->mm,
xsl_fault_error, ctx);
if (rc)
goto out;
diff --git a/drivers/misc/ocxl/file.c b/drivers/misc/ocxl/file.c
index 038509e5d031..eb409a469f21 100644
--- a/drivers/misc/ocxl/file.c
+++ b/drivers/misc/ocxl/file.c
@@ -5,6 +5,8 @@
#include
#include
#include
+#include
+#include
#include "ocxl_internal.h"
@@ -123,11 +125,55 @@ static long afu_ioctl_get_metadata(struct ocxl_context *ctx,
return 0;
}
+#ifdef CONFIG_PPC64
+static long afu_ioctl_enable_p9_wait(struct ocxl_context *ctx,
+ struct ocxl_ioctl_p9_wait __user *uarg)
+{
+ struct ocxl_ioctl_p9_wait arg;
+
+ memset(, 0, sizeof(arg));
+
+ if (cpu_has_feature(CPU_FTR_P9_TIDR)) {
+ enum ocxl_context_status status;
+
+ // Locks both status & tidr
+ mutex_lock(>status_mutex);
+ if (!ctx->tidr) {
+ if (set_thread_tidr(current))
+ return -ENOENT;
+
+ ctx->tidr = current->thread.tidr;
+ }
+
+ status = ctx->status;
+ mutex_unlock(>status_mutex);
+
+ if (status == ATTACHED) {
+ int rc;
+ struct link *link = ctx->afu->fn->link;
Declarations at the top
+
+ rc = ocxl_link_update_pe(link, ctx->pasid, ctx->tidr);
+ if (rc)
+ return rc;
+ }
+
+ arg.thread_id = ctx->tidr;
+ } else
+ return -ENOENT;
+
+ if (copy_to_user(uarg, , sizeof(arg)))
+ return -EFAULT;
+
+ return 0;
+}
+#endif
+
#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" : \
+ x == OCXL_IOCTL_ENABLE_P9_WAIT ? "ENABLE_P9_WAIT" : \
"UNKNOWN")
static long afu_ioctl(struct file *file, unsigned int cmd,
@@ -186,6 +232,13 @@ static long afu_ioctl(struct file *file, unsigned int cmd,
(struct ocxl_ioctl_metadata __user *) args);
break;
+#ifdef CONFIG_PPC64
+ case OCXL_IOCTL_ENABLE_P9_WAIT:
+ rc = afu_ioctl_enable_p9_wait(ctx,
+ (struct ocxl_ioctl_p9_wait __user *) args);
+ break;
+#endif
+
default:
rc = -EINVAL;
}
diff --git a/drivers/misc/ocxl/link.c b/drivers/misc/ocxl/link.c
index 656e8610eec2..88876ae8f330 100644
--- a/drivers/misc/ocxl/link.c
+++ b/drivers/misc/ocxl/link.c
@@ -544,6
Re: [PATCH v2 6/7] ocxl: Add an IOCTL so userspace knows what CPU features are available
On 18/04/18 11:08, Alastair D'Silva wrote: From: Alastair D'Silva <alast...@d-silva.org> In order for a userspace AFU driver to call the Power9 specific OCXL_IOCTL_ENABLE_P9_WAIT, it needs to verify that it can actually make that call. Signed-off-by: Alastair D'Silva <alast...@d-silva.org> Looks good to me Acked-by: Andrew Donnellan <andrew.donnel...@au1.ibm.com> --- Documentation/accelerators/ocxl.rst | 1 - drivers/misc/ocxl/file.c| 25 + include/uapi/misc/ocxl.h| 4 3 files changed, 29 insertions(+), 1 deletion(-) diff --git a/Documentation/accelerators/ocxl.rst b/Documentation/accelerators/ocxl.rst index ddcc58d01cfb..7904adcc07fd 100644 --- a/Documentation/accelerators/ocxl.rst +++ b/Documentation/accelerators/ocxl.rst @@ -157,7 +157,6 @@ 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. - This is stray -- Andrew Donnellan OzLabs, ADL Canberra andrew.donnel...@au1.ibm.com IBM Australia Limited -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 7/7] ocxl: Document new OCXL IOCTLs
On 18/04/18 11:08, Alastair D'Silva wrote: From: Alastair D'Silva <alast...@d-silva.org> Signed-off-by: Alastair D'Silva <alast...@d-silva.org> This looks better. Acked-by: Andrew Donnellan <andrew.donnel...@au1.ibm.com> --- Documentation/accelerators/ocxl.rst | 11 +++ 1 file changed, 11 insertions(+) diff --git a/Documentation/accelerators/ocxl.rst b/Documentation/accelerators/ocxl.rst index 7904adcc07fd..3b8d3b99795c 100644 --- a/Documentation/accelerators/ocxl.rst +++ b/Documentation/accelerators/ocxl.rst @@ -157,6 +157,17 @@ 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. +OCXL_IOCTL_ENABLE_P9_WAIT: + + Allows the AFU to wake a userspace thread executing 'wait'. Returns + information to userspace to allow it to configure the AFU. Note that + this is only available on Power 9. Nitpicking time, if you do a v3 you should stay on brand and call it POWER9. :D + +OCXL_IOCTL_GET_FEATURES: + + Reports on which CPU features that affect OpenCAPI are usable from + userspace. + mmap ---- -- Andrew Donnellan OzLabs, ADL Canberra andrew.donnel...@au1.ibm.com IBM Australia Limited -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/7] powerpc: Use TIDR CPU feature to control TIDR allocation
On 18/04/18 11:08, Alastair D'Silva wrote: From: Alastair D'Silva <alast...@d-silva.org> Switch the use of TIDR on it's CPU feature, rather than assuming it is available based on architecture. Signed-off-by: Alastair D'Silva <alast...@d-silva.org> Reviewed-by: Andrew Donnellan <andrew.donnel...@au1.ibm.com> -- Andrew Donnellan OzLabs, ADL Canberra andrew.donnel...@au1.ibm.com IBM Australia Limited -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/7] powerpc: Add TIDR CPU feature for Power9
On 18/04/18 11:08, Alastair D'Silva wrote: From: Alastair D'Silva <alast...@d-silva.org> This patch adds a CPU feature bit to show whether the CPU has the TIDR register available, enabling as_notify/wait in userspace. Signed-off-by: Alastair D'Silva <alast...@d-silva.org> Per my previous email: Reviewed-by: Andrew Donnellan <andrew.donnel...@au1.ibm.com> -- Andrew Donnellan OzLabs, ADL Canberra andrew.donnel...@au1.ibm.com IBM Australia Limited -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/7] ocxl: Rename pnv_ocxl_spa_remove_pe to clarify it's action
On 17/04/18 12:09, Alastair D'Silva wrote: From: Alastair D'Silva <alast...@d-silva.org> The function removes the process element from NPU cache. Signed-off-by: Alastair D'Silva <alast...@d-silva.org> Hmm, personally I'd suggest pnv_ocxl_spa_clear_cache() because it's just a wrapper around the OPAL call of a similar name. But I don't feel strongly about this at all, so: Acked-by: Andrew Donnellan <andrew.donnel...@au1.ibm.com> -- Andrew Donnellan OzLabs, ADL Canberra andrew.donnel...@au1.ibm.com IBM Australia Limited -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/7] powerpc: Use TIDR CPU feature to control TIDR allocation
On 17/04/18 12:09, Alastair D'Silva wrote:
From: Alastair D'Silva <alast...@d-silva.org>
Switch the use of TIDR on it's CPU feature, rather than assuming it
is available based on architecture.
Signed-off-by: Alastair D'Silva <alast...@d-silva.org>
There's a use of TIDR in restore_sprs() that's behind the ARCH_300 flag
as well, ideally it should never trigger in the !P9_TIDR case, but you
might want to update that too for clarity?
---
arch/powerpc/kernel/process.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 1237f13fed51..a3e0a3e06d5a 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -1570,7 +1570,7 @@ void clear_thread_tidr(struct task_struct *t)
if (!t->thread.tidr)
return;
- if (!cpu_has_feature(CPU_FTR_ARCH_300)) {
+ if (!cpu_has_feature(CPU_FTR_P9_TIDR)) {
WARN_ON_ONCE(1);
return;
}
@@ -1593,7 +1593,7 @@ int set_thread_tidr(struct task_struct *t)
{
int rc;
- if (!cpu_has_feature(CPU_FTR_ARCH_300))
+ if (!cpu_has_feature(CPU_FTR_P9_TIDR))
return -EINVAL;
if (t != current)
--
Andrew Donnellan OzLabs, ADL Canberra
andrew.donnel...@au1.ibm.com IBM Australia Limited
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/7] powerpc: Add TIDR CPU feature for Power9
On 17/04/18 12:09, Alastair D'Silva wrote: diff --git a/arch/powerpc/include/asm/switch_to.h b/arch/powerpc/include/asm/switch_to.h index be8c9fa23983..5b03d8a82409 100644 --- a/arch/powerpc/include/asm/switch_to.h +++ b/arch/powerpc/include/asm/switch_to.h @@ -94,6 +94,5 @@ static inline void clear_task_ebb(struct task_struct *t) extern int set_thread_uses_vas(void); extern int set_thread_tidr(struct task_struct *t); -extern void clear_thread_tidr(struct task_struct *t); This hunk looks like it really belongs in patch 3. Apart from that, I'm not really familiar with the CPU features code but nothing seems overly wrong... Reviewed-by: Andrew Donnellan <andrew.donnel...@au1.ibm.com> -- Andrew Donnellan OzLabs, ADL Canberra andrew.donnel...@au1.ibm.com IBM Australia Limited -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 7/7] ocxl: Document new OCXL IOCTLs
On 17/04/18 12:09, Alastair D'Silva wrote: From: Alastair D'Silva <alast...@d-silva.org> Signed-off-by: Alastair D'Silva <alast...@d-silva.org> --- Documentation/accelerators/ocxl.rst | 10 ++ 1 file changed, 10 insertions(+) diff --git a/Documentation/accelerators/ocxl.rst b/Documentation/accelerators/ocxl.rst index ddcc58d01cfb..144595a80a1c 100644 --- a/Documentation/accelerators/ocxl.rst +++ b/Documentation/accelerators/ocxl.rst @@ -157,6 +157,16 @@ 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. +OCXL_IOCTL_ENABLE_P9_WAIT: + + Allows the AFU to wake a userspace thread executing 'wait'. Returns + information to userspace to allow it to configure the AFU. Note that this is only available on POWER9. + +OCXL_IOCTL_GET_PLATFORM: + + Notifies userspace as to the platform the kernel believes we are on, + which may differ from what userspace believes. Also reports on which CPU + features which are usable from userspace. The first sentence here doesn't seem to relate to anything that GET_PLATFORM actually does - afaict you're just passing flags which I suppose imply what the correct platform is, but really they're just feature flags? -- Andrew Donnellan OzLabs, ADL Canberra andrew.donnel...@au1.ibm.com IBM Australia Limited -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 02/11] ABI: fix some syntax issues at the ABI database
On 13/04/17 20:08, Mauro Carvalho Chehab wrote: diff --git a/Documentation/ABI/testing/sysfs-class-cxl b/Documentation/ABI/testing/sysfs-class-cxl index 640f65e79ef1..d0b32452dfe1 100644 --- a/Documentation/ABI/testing/sysfs-class-cxl +++ b/Documentation/ABI/testing/sysfs-class-cxl @@ -1,6 +1,6 @@ -Note: Attributes that are shared between devices are stored in the directory -pointed to by the symlink device/. -Example: The real path of the attribute /sys/class/cxl/afu0.0s/irqs_max is +Please notice that attributes that are shared between devices are stored in +the directory pointed to by the symlink device/. +For example, the real path of the attribute /sys/class/cxl/afu0.0s/irqs_max is /sys/class/cxl/afu0.0s/device/irqs_max, i.e. /sys/class/cxl/afu0.0/irqs_max. If you end up respinning this, I'd prefer "Note that" or "Please note that" rather than "Please notice". Otherwise, for cxl: Reviewed-by: Andrew Donnellan <andrew.donnel...@au1.ibm.com> -- Andrew Donnellan OzLabs, ADL Canberra andrew.donnel...@au1.ibm.com IBM Australia Limited -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
