From bfe5fda8e7ced129716f5741cf7ed2592a338824 Mon Sep 17 00:00:00 2001 From: Anton Blanchard Date: Tue, 6 Jan 2015 21:12:08 +1100 Subject: powernv: Fix OPAL tracepoint code Patch c49f63530bb6 ("powernv: Add OPAL tracepoints") has a spurious store to the stack: ld r12,opal_tracepoint_refcount@toc(r2); \ std r12,32(r1); \ The store was originally used to save the current tracepoint status so the entry and the exit tracepoints were always balanced. In the end I just created a separate path when tracepoints are enabled. The offset on the stack used for this store is not valid for ABIv2 and it causes strange issues. I noticed it because OPAL console input was broken. Fixes: c49f63530bb6 ("powernv: Add OPAL tracepoints") Cc: # v3.17+ Signed-off-by: Anton Blanchard Signed-off-by: Michael Ellerman --- arch/powerpc/platforms/powernv/opal-wrappers.S | 1 - 1 file changed, 1 deletion(-) diff --git a/arch/powerpc/platforms/powernv/opal-wrappers.S b/arch/powerpc/platforms/powernv/opal-wrappers.S index 54eca8b3b288..0509bca5e830 100644 --- a/arch/powerpc/platforms/powernv/opal-wrappers.S +++ b/arch/powerpc/platforms/powernv/opal-wrappers.S @@ -40,7 +40,6 @@ BEGIN_FTR_SECTION; \ b 1f; \ END_FTR_SECTION(0, 1); \ ld r12,opal_tracepoint_refcount@toc(r2); \ - std r12,32(r1); \ cmpdi r12,0; \ bne- LABEL; \ 1: -- cgit v1.2.3 From 0712dc7e73e59d79bcead5d5520acf4e9e917e87 Mon Sep 17 00:00:00 2001 From: Ian Munsie Date: Wed, 7 Jan 2015 16:33:04 +1100 Subject: cxl: Fix issues when unmapping contexts An issue was introduced with "cxl: Unmap MMIO regions when detaching a context" (b123429e6a9e8d03aacf888d23262835f0081448) where closing a context normally could also unmap the problem state area of other contexts currently using the AFU. It was also discovered that after a context's MMIO space had been unmapped it would read 0s when accessing it, whereas the expected behaviour was for the access to fail altogether. In order to address these issues, this patch does two things: - Forced mmap unmapping is only done when we are forcefully detaching all contexts, and not in the normal detach path. Since the normal context close path is tied to the file release any mmaps must have already been released so we don't need to worry in that case. - The mmap path now uses a vm_operations_struct with a fault handler. The fault handler ensures that the context is in started state, otherwise it fails the access attempt with a SIGBUS. Fixes: b123429e6a9e ("cxl: Unmap MMIO regions when detaching a context") Signed-off-by: Ian Munsie Signed-off-by: Michael Ellerman --- drivers/misc/cxl/context.c | 82 +++++++++++++++++++++++++++++++++++----------- drivers/misc/cxl/file.c | 14 ++++---- 2 files changed, 71 insertions(+), 25 deletions(-) diff --git a/drivers/misc/cxl/context.c b/drivers/misc/cxl/context.c index 51fd6b524371..d1b55fe62817 100644 --- a/drivers/misc/cxl/context.c +++ b/drivers/misc/cxl/context.c @@ -100,6 +100,46 @@ int cxl_context_init(struct cxl_context *ctx, struct cxl_afu *afu, bool master, return 0; } +static int cxl_mmap_fault(struct vm_area_struct *vma, struct vm_fault *vmf) +{ + struct cxl_context *ctx = vma->vm_file->private_data; + unsigned long address = (unsigned long)vmf->virtual_address; + u64 area, offset; + + offset = vmf->pgoff << PAGE_SHIFT; + + pr_devel("%s: pe: %i address: 0x%lx offset: 0x%llx\n", + __func__, ctx->pe, address, offset); + + if (ctx->afu->current_mode == CXL_MODE_DEDICATED) { + area = ctx->afu->psn_phys; + if (offset > ctx->afu->adapter->ps_size) + return VM_FAULT_SIGBUS; + } else { + area = ctx->psn_phys; + if (offset > ctx->psn_size) + return VM_FAULT_SIGBUS; + } + + mutex_lock(&ctx->status_mutex); + + if (ctx->status != STARTED) { + mutex_unlock(&ctx->status_mutex); + pr_devel("%s: Context not started, failing problem state access\n", __func__); + return VM_FAULT_SIGBUS; + } + + vm_insert_pfn(vma, address, (area + offset) >> PAGE_SHIFT); + + mutex_unlock(&ctx->status_mutex); + + return VM_FAULT_NOPAGE; +} + +static const struct vm_operations_struct cxl_mmap_vmops = { + .fault = cxl_mmap_fault, +}; + /* * Map a per-context mmio space into the given vma. */ @@ -108,26 +148,25 @@ int cxl_context_iomap(struct cxl_context *ctx, struct vm_area_struct *vma) u64 len = vma->vm_end - vma->vm_start; len = min(len, ctx->psn_size); - if (ctx->afu->current_mode == CXL_MODE_DEDICATED) { - vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot); - return vm_iomap_memory(vma, ctx->afu->psn_phys, ctx->afu->adapter->ps_size); - } + if (ctx->afu->current_mode != CXL_MODE_DEDICATED) { + /* make sure there is a valid per process space for this AFU */ + if ((ctx->master && !ctx->afu->psa) || (!ctx->afu->pp_psa)) { + pr_devel("AFU doesn't support mmio space\n"); + return -EINVAL; + } - /* make sure there is a valid per process space for this AFU */ - if ((ctx->master && !ctx->afu->psa) || (!ctx->afu->pp_psa)) { - pr_devel("AFU doesn't support mmio space\n"); - return -EINVAL; + /* Can't mmap until the AFU is enabled */ + if (!ctx->afu->enabled) + return -EBUSY; } - /* Can't mmap until the AFU is enabled */ - if (!ctx->afu->enabled) - return -EBUSY; - pr_devel("%s: mmio physical: %llx pe: %i master:%i\n", __func__, ctx->psn_phys, ctx->pe , ctx->master); + vma->vm_flags |= VM_IO | VM_PFNMAP; vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot); - return vm_iomap_memory(vma, ctx->psn_phys, len); + vma->vm_ops = &cxl_mmap_vmops; + return 0; } /* @@ -150,12 +189,6 @@ static void __detach_context(struct cxl_context *ctx) afu_release_irqs(ctx); flush_work(&ctx->fault_work); /* Only needed for dedicated process */ wake_up_all(&ctx->wq); - - /* Release Problem State Area mapping */ - mutex_lock(&ctx->mapping_lock); - if (ctx->mapping) - unmap_mapping_range(ctx->mapping, 0, 0, 1); - mutex_unlock(&ctx->mapping_lock); } /* @@ -184,6 +217,17 @@ void cxl_context_detach_all(struct cxl_afu *afu) * created and torn down after the IDR removed */ __detach_context(ctx); + + /* + * We are force detaching - remove any active PSA mappings so + * userspace cannot interfere with the card if it comes back. + * Easiest way to exercise this is to unbind and rebind the + * driver via sysfs while it is in use. + */ + mutex_lock(&ctx->mapping_lock); + if (ctx->mapping) + unmap_mapping_range(ctx->mapping, 0, 0, 1); + mutex_unlock(&ctx->mapping_lock); } mutex_unlock(&afu->contexts_lock); } diff --git a/drivers/misc/cxl/file.c b/drivers/misc/cxl/file.c index e9f2f10dbb37..b15d8113877c 100644 --- a/drivers/misc/cxl/file.c +++ b/drivers/misc/cxl/file.c @@ -140,18 +140,20 @@ static long afu_ioctl_start_work(struct cxl_context *ctx, pr_devel("%s: pe: %i\n", __func__, ctx->pe); - mutex_lock(&ctx->status_mutex); - if (ctx->status != OPENED) { - rc = -EIO; - goto out; - } - + /* Do this outside the status_mutex to avoid a circular dependency with + * the locking in cxl_mmap_fault() */ if (copy_from_user(&work, uwork, sizeof(struct cxl_ioctl_start_work))) { rc = -EFAULT; goto out; } + mutex_lock(&ctx->status_mutex); + if (ctx->status != OPENED) { + rc = -EIO; + goto out; + } + /* * if any of the reserved fields are set or any of the unused * flags are set it's invalid -- cgit v1.2.3 From a87e810f61b49f19bd29ea564b7cd1e92e43d989 Mon Sep 17 00:00:00 2001 From: Michael Ellerman Date: Thu, 8 Jan 2015 15:30:08 +1100 Subject: powerpc: Work around gcc bug in current_thread_info() In commit a3e5b356b3ab "powerpc: Don't use local named register variable in current_thread_info" Anton changed the way we did current_thread_info() to accommodate LLVM, and it was not meant to have any effect elsewhere. Unfortunately it has exposed a gcc bug, where r1 gets copied into another register and then gcc uses that register to restore the toc after a function call, even when that register is volatile and has been clobbered by the function call. We could revert Anton's patch, but it's not clear the original code is safe either, we may just have been lucky. The cleanest solution is to just use the existing CURRENT_THREAD_INFO() asm macro, and call it using inline asm. Segher points out we don't need volatile on the asm, if the result of the shift is unused it's fine for the compiler to elide it. Fixes: a3e5b356b3ab ("powerpc: Don't use local named register variable in current_thread_info") Reported-by: Alexander Graf Signed-off-by: Michael Ellerman --- arch/powerpc/include/asm/thread_info.h | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/arch/powerpc/include/asm/thread_info.h b/arch/powerpc/include/asm/thread_info.h index ebc4f165690a..0be6c681cab1 100644 --- a/arch/powerpc/include/asm/thread_info.h +++ b/arch/powerpc/include/asm/thread_info.h @@ -23,9 +23,9 @@ #define THREAD_SIZE (1 << THREAD_SHIFT) #ifdef CONFIG_PPC64 -#define CURRENT_THREAD_INFO(dest, sp) clrrdi dest, sp, THREAD_SHIFT +#define CURRENT_THREAD_INFO(dest, sp) stringify_in_c(clrrdi dest, sp, THREAD_SHIFT) #else -#define CURRENT_THREAD_INFO(dest, sp) rlwinm dest, sp, 0, 0, 31-THREAD_SHIFT +#define CURRENT_THREAD_INFO(dest, sp) stringify_in_c(rlwinm dest, sp, 0, 0, 31-THREAD_SHIFT) #endif #ifndef __ASSEMBLY__ @@ -71,12 +71,13 @@ struct thread_info { #define THREAD_SIZE_ORDER (THREAD_SHIFT - PAGE_SHIFT) /* how to get the thread information struct from C */ -register unsigned long __current_r1 asm("r1"); static inline struct thread_info *current_thread_info(void) { - /* gcc4, at least, is smart enough to turn this into a single - * rlwinm for ppc32 and clrrdi for ppc64 */ - return (struct thread_info *)(__current_r1 & ~(THREAD_SIZE-1)); + unsigned long val; + + asm (CURRENT_THREAD_INFO(%0,1) : "=r" (val)); + + return (struct thread_info *)val; } #endif /* __ASSEMBLY__ */ -- cgit v1.2.3