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