diff mbox

[7/7] CXL: Unmap MMIO regions when detaching a context

Message ID 1418026681-14787-7-git-send-email-imunsie@au.ibm.com (mailing list archive)
State Accepted
Commit b123429e6a9e8d03aacf888d23262835f0081448
Delegated to: Michael Ellerman
Headers show

Commit Message

Ian Munsie Dec. 8, 2014, 8:18 a.m. UTC
From: Ian Munsie <imunsie@au1.ibm.com>

If we need to force detach a context (e.g. due to EEH or simply force
unbinding the driver) we should prevent the userspace contexts from
being able to access the Problem State Area MMIO region further, which
they may have mapped with mmap().

This patch unmaps any mapped MMIO regions when detaching a userspace
context.

Signed-off-by: Ian Munsie <imunsie@au1.ibm.com>
---
 drivers/misc/cxl/context.c | 11 ++++++++++-
 drivers/misc/cxl/cxl.h     |  7 ++++++-
 drivers/misc/cxl/file.c    |  6 +++++-
 3 files changed, 21 insertions(+), 3 deletions(-)

Comments

Ian Munsie Dec. 9, 2014, 5:41 a.m. UTC | #1
This one should go to stable - this was the first bug uncovered after
fixing the sleep while atomic and force unbinding the driver.

Cheers,
-Ian

Excerpts from Ian Munsie's message of 2014-12-08 19:18:01 +1100:
> From: Ian Munsie <imunsie@au1.ibm.com>
> 
> If we need to force detach a context (e.g. due to EEH or simply force
> unbinding the driver) we should prevent the userspace contexts from
> being able to access the Problem State Area MMIO region further, which
> they may have mapped with mmap().
> 
> This patch unmaps any mapped MMIO regions when detaching a userspace
> context.
> 
> Signed-off-by: Ian Munsie <imunsie@au1.ibm.com>
> ---
>  drivers/misc/cxl/context.c | 11 ++++++++++-
>  drivers/misc/cxl/cxl.h     |  7 ++++++-
>  drivers/misc/cxl/file.c    |  6 +++++-
>  3 files changed, 21 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/misc/cxl/context.c b/drivers/misc/cxl/context.c
> index 4aa31a3..51fd6b5 100644
> --- a/drivers/misc/cxl/context.c
> +++ b/drivers/misc/cxl/context.c
> @@ -34,7 +34,8 @@ struct cxl_context *cxl_context_alloc(void)
>  /*
>   * Initialises a CXL context.
>   */
> -int cxl_context_init(struct cxl_context *ctx, struct cxl_afu *afu, bool master)
> +int cxl_context_init(struct cxl_context *ctx, struct cxl_afu *afu, bool master,
> +             struct address_space *mapping)
>  {
>      int i;
>  
> @@ -42,6 +43,8 @@ int cxl_context_init(struct cxl_context *ctx, struct cxl_afu *afu, bool master)
>      ctx->afu = afu;
>      ctx->master = master;
>      ctx->pid = NULL; /* Set in start work ioctl */
> +    mutex_init(&ctx->mapping_lock);
> +    ctx->mapping = mapping;
>  
>      /*
>       * Allocate the segment table before we put it in the IDR so that we
> @@ -147,6 +150,12 @@ 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);
>  }
>  
>  /*
> diff --git a/drivers/misc/cxl/cxl.h b/drivers/misc/cxl/cxl.h
> index c1f8aa6..0df0438 100644
> --- a/drivers/misc/cxl/cxl.h
> +++ b/drivers/misc/cxl/cxl.h
> @@ -405,6 +405,10 @@ struct cxl_context {
>      phys_addr_t psn_phys;
>      u64 psn_size;
>  
> +    /* Used to unmap any mmaps when force detaching */
> +    struct address_space *mapping;
> +    struct mutex mapping_lock;
> +
>      spinlock_t sste_lock; /* Protects segment table entries */
>      struct cxl_sste *sstp;
>      u64 sstp0, sstp1;
> @@ -606,7 +610,8 @@ int cxl_alloc_sst(struct cxl_context *ctx);
>  void init_cxl_native(void);
>  
>  struct cxl_context *cxl_context_alloc(void);
> -int cxl_context_init(struct cxl_context *ctx, struct cxl_afu *afu, bool master);
> +int cxl_context_init(struct cxl_context *ctx, struct cxl_afu *afu, bool master,
> +             struct address_space *mapping);
>  void cxl_context_free(struct cxl_context *ctx);
>  int cxl_context_iomap(struct cxl_context *ctx, struct vm_area_struct *vma);
>  
> diff --git a/drivers/misc/cxl/file.c b/drivers/misc/cxl/file.c
> index 2e067a5..b09be44 100644
> --- a/drivers/misc/cxl/file.c
> +++ b/drivers/misc/cxl/file.c
> @@ -77,7 +77,7 @@ static int __afu_open(struct inode *inode, struct file *file, bool master)
>          goto err_put_afu;
>      }
>  
> -    if ((rc = cxl_context_init(ctx, afu, master)))
> +    if ((rc = cxl_context_init(ctx, afu, master, inode->i_mapping)))
>          goto err_put_afu;
>  
>      pr_devel("afu_open pe: %i\n", ctx->pe);
> @@ -113,6 +113,10 @@ static int afu_release(struct inode *inode, struct file *file)
>           __func__, ctx->pe);
>      cxl_context_detach(ctx);
>  
> +    mutex_lock(&ctx->mapping_lock);
> +    ctx->mapping = NULL;
> +    mutex_unlock(&ctx->mapping_lock);
> +
>      put_device(&ctx->afu->dev);
>  
>      /*
Ian Munsie Dec. 15, 2014, 3:32 a.m. UTC | #2
Excerpts from Ian Munsie's message of 2014-12-08 19:18:01 +1100:
> From: Ian Munsie <imunsie@au1.ibm.com>
> 
> If we need to force detach a context (e.g. due to EEH or simply force
> unbinding the driver) we should prevent the userspace contexts from
> being able to access the Problem State Area MMIO region further, which
> they may have mapped with mmap().
> 
> This patch unmaps any mapped MMIO regions when detaching a userspace
> context.

Might want to drop this one for now - Philippe found that it sometimes
causes an issue when multiple contexts are using the afu. Seems that
unmap_mapping_range() unmapped a bit more than I expected.

Cheers,
-Ian
Michael Ellerman Dec. 15, 2014, 11:32 p.m. UTC | #3
On Mon, 2014-12-15 at 14:32 +1100, Ian Munsie wrote:
> Excerpts from Ian Munsie's message of 2014-12-08 19:18:01 +1100:
> > From: Ian Munsie <imunsie@au1.ibm.com>
> > 
> > If we need to force detach a context (e.g. due to EEH or simply force
> > unbinding the driver) we should prevent the userspace contexts from
> > being able to access the Problem State Area MMIO region further, which
> > they may have mapped with mmap().
> > 
> > This patch unmaps any mapped MMIO regions when detaching a userspace
> > context.
> 
> Might want to drop this one for now - Philippe found that it sometimes
> causes an issue when multiple contexts are using the afu. Seems that
> unmap_mapping_range() unmapped a bit more than I expected.

Sorry, it's already in next.

https://git.kernel.org/cgit/linux/kernel/git/mpe/linux.git/commit/?h=next&id=b123429e6a9e8d03aacf888d23262835f0081448

Send me a revert or a fixup.

cheers
diff mbox

Patch

diff --git a/drivers/misc/cxl/context.c b/drivers/misc/cxl/context.c
index 4aa31a3..51fd6b5 100644
--- a/drivers/misc/cxl/context.c
+++ b/drivers/misc/cxl/context.c
@@ -34,7 +34,8 @@  struct cxl_context *cxl_context_alloc(void)
 /*
  * Initialises a CXL context.
  */
-int cxl_context_init(struct cxl_context *ctx, struct cxl_afu *afu, bool master)
+int cxl_context_init(struct cxl_context *ctx, struct cxl_afu *afu, bool master,
+		     struct address_space *mapping)
 {
 	int i;
 
@@ -42,6 +43,8 @@  int cxl_context_init(struct cxl_context *ctx, struct cxl_afu *afu, bool master)
 	ctx->afu = afu;
 	ctx->master = master;
 	ctx->pid = NULL; /* Set in start work ioctl */
+	mutex_init(&ctx->mapping_lock);
+	ctx->mapping = mapping;
 
 	/*
 	 * Allocate the segment table before we put it in the IDR so that we
@@ -147,6 +150,12 @@  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);
 }
 
 /*
diff --git a/drivers/misc/cxl/cxl.h b/drivers/misc/cxl/cxl.h
index c1f8aa6..0df0438 100644
--- a/drivers/misc/cxl/cxl.h
+++ b/drivers/misc/cxl/cxl.h
@@ -405,6 +405,10 @@  struct cxl_context {
 	phys_addr_t psn_phys;
 	u64 psn_size;
 
+	/* Used to unmap any mmaps when force detaching */
+	struct address_space *mapping;
+	struct mutex mapping_lock;
+
 	spinlock_t sste_lock; /* Protects segment table entries */
 	struct cxl_sste *sstp;
 	u64 sstp0, sstp1;
@@ -606,7 +610,8 @@  int cxl_alloc_sst(struct cxl_context *ctx);
 void init_cxl_native(void);
 
 struct cxl_context *cxl_context_alloc(void);
-int cxl_context_init(struct cxl_context *ctx, struct cxl_afu *afu, bool master);
+int cxl_context_init(struct cxl_context *ctx, struct cxl_afu *afu, bool master,
+		     struct address_space *mapping);
 void cxl_context_free(struct cxl_context *ctx);
 int cxl_context_iomap(struct cxl_context *ctx, struct vm_area_struct *vma);
 
diff --git a/drivers/misc/cxl/file.c b/drivers/misc/cxl/file.c
index 2e067a5..b09be44 100644
--- a/drivers/misc/cxl/file.c
+++ b/drivers/misc/cxl/file.c
@@ -77,7 +77,7 @@  static int __afu_open(struct inode *inode, struct file *file, bool master)
 		goto err_put_afu;
 	}
 
-	if ((rc = cxl_context_init(ctx, afu, master)))
+	if ((rc = cxl_context_init(ctx, afu, master, inode->i_mapping)))
 		goto err_put_afu;
 
 	pr_devel("afu_open pe: %i\n", ctx->pe);
@@ -113,6 +113,10 @@  static int afu_release(struct inode *inode, struct file *file)
 		 __func__, ctx->pe);
 	cxl_context_detach(ctx);
 
+	mutex_lock(&ctx->mapping_lock);
+	ctx->mapping = NULL;
+	mutex_unlock(&ctx->mapping_lock);
+
 	put_device(&ctx->afu->dev);
 
 	/*