diff mbox series

[v3,06/10] powerpc/vas: Map paste address only if window is active

Message ID 4ecd08a1b92590d6220920245c23526a68dc531c.camel@linux.ibm.com (mailing list archive)
State Superseded
Headers show
Series powerpc/pseries/vas: NXGZIP support with DLPAR | expand

Commit Message

Haren Myneni Jan. 21, 2022, 7:58 p.m. UTC
The paste address mapping is done with mmap() after the window is
opened with ioctl. But the window can be closed due to lost credit
due to core removal before mmap(). So if the window is not active,
return mmap() failure with -EACCES and expects the user space reissue
mmap() when the window is active or open new window when the credit
is available.

Signed-off-by: Haren Myneni <haren@linux.ibm.com>
---
 arch/powerpc/platforms/book3s/vas-api.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

Comments

Nicholas Piggin Feb. 14, 2022, 3:20 a.m. UTC | #1
Excerpts from Haren Myneni's message of January 22, 2022 5:58 am:
> 
> The paste address mapping is done with mmap() after the window is
> opened with ioctl. But the window can be closed due to lost credit
> due to core removal before mmap(). So if the window is not active,
> return mmap() failure with -EACCES and expects the user space reissue
> mmap() when the window is active or open new window when the credit
> is available.
> 
> Signed-off-by: Haren Myneni <haren@linux.ibm.com>
> ---
>  arch/powerpc/platforms/book3s/vas-api.c | 21 ++++++++++++++++++++-
>  1 file changed, 20 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/platforms/book3s/vas-api.c b/arch/powerpc/platforms/book3s/vas-api.c
> index a63fd48e34a7..2d06bd1b1935 100644
> --- a/arch/powerpc/platforms/book3s/vas-api.c
> +++ b/arch/powerpc/platforms/book3s/vas-api.c
> @@ -379,10 +379,27 @@ static int coproc_mmap(struct file *fp, struct vm_area_struct *vma)
>  		return -EACCES;
>  	}
>  
> +	/*
> +	 * The initial mapping is done after the window is opened
> +	 * with ioctl. But this window might have been closed
> +	 * due to lost credit (core removal on PowerVM) before mmap().

What does "initial mapping" mean?

mapping ~= mmap, in kernel speak.

You will have to differentiate the concepts.

> +	 * So if the window is not active, return mmap() failure
> +	 * with -EACCES and expects the user space reconfigure (mmap)
> +	 * window when it is active again or open new window when
> +	 * the credit is available.
> +	 */
> +	mutex_lock(&txwin->task_ref.mmap_mutex);
> +	if (txwin->status != VAS_WIN_ACTIVE) {
> +		pr_err("%s(): Window is not active\n", __func__);
> +		rc = -EACCES;
> +		goto out;
> +	}
> +
>  	paste_addr = cp_inst->coproc->vops->paste_addr(txwin);
>  	if (!paste_addr) {
>  		pr_err("%s(): Window paste address failed\n", __func__);
> -		return -EINVAL;
> +		rc = -EINVAL;
> +		goto out;
>  	}
>  
>  	pfn = paste_addr >> PAGE_SHIFT;
> @@ -401,6 +418,8 @@ static int coproc_mmap(struct file *fp, struct vm_area_struct *vma)
>  
>  	txwin->task_ref.vma = vma;
>  
> +out:
> +	mutex_unlock(&txwin->task_ref.mmap_mutex);

If the hypervisor can revoke a window at any point with DLPAR, it's not 
clear *why* this is needed. The hypervisor could cause your window to 
close right after this mmap() returns, right? So an explanation for 
exactly what this patch is needed for beyond that would help.

Thanks,
Nick
Haren Myneni Feb. 16, 2022, 1:58 a.m. UTC | #2
On Mon, 2022-02-14 at 13:20 +1000, Nicholas Piggin wrote:
> Excerpts from Haren Myneni's message of January 22, 2022 5:58 am:
> > The paste address mapping is done with mmap() after the window is
> > opened with ioctl. But the window can be closed due to lost credit
> > due to core removal before mmap(). So if the window is not active,
> > return mmap() failure with -EACCES and expects the user space
> > reissue
> > mmap() when the window is active or open new window when the credit
> > is available.
> > 
> > Signed-off-by: Haren Myneni <haren@linux.ibm.com>
> > ---
> >  arch/powerpc/platforms/book3s/vas-api.c | 21 ++++++++++++++++++++-
> >  1 file changed, 20 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/powerpc/platforms/book3s/vas-api.c
> > b/arch/powerpc/platforms/book3s/vas-api.c
> > index a63fd48e34a7..2d06bd1b1935 100644
> > --- a/arch/powerpc/platforms/book3s/vas-api.c
> > +++ b/arch/powerpc/platforms/book3s/vas-api.c
> > @@ -379,10 +379,27 @@ static int coproc_mmap(struct file *fp,
> > struct vm_area_struct *vma)
> >  		return -EACCES;
> >  	}
> >  
> > +	/*
> > +	 * The initial mapping is done after the window is opened
> > +	 * with ioctl. But this window might have been closed
> > +	 * due to lost credit (core removal on PowerVM) before mmap().
> 
> What does "initial mapping" mean?
> 
> mapping ~= mmap, in kernel speak.

yes, the initial mapping is done with the actual mmap() call. 
> 
> You will have to differentiate the concepts.
> 
> > +	 * So if the window is not active, return mmap() failure
> > +	 * with -EACCES and expects the user space reconfigure (mmap)
> > +	 * window when it is active again or open new window when
> > +	 * the credit is available.
> > +	 */
> > +	mutex_lock(&txwin->task_ref.mmap_mutex);
> > +	if (txwin->status != VAS_WIN_ACTIVE) {
> > +		pr_err("%s(): Window is not active\n", __func__);
> > +		rc = -EACCES;
> > +		goto out;
> > +	}
> > +
> >  	paste_addr = cp_inst->coproc->vops->paste_addr(txwin);
> >  	if (!paste_addr) {
> >  		pr_err("%s(): Window paste address failed\n",
> > __func__);
> > -		return -EINVAL;
> > +		rc = -EINVAL;
> > +		goto out;
> >  	}
> >  
> >  	pfn = paste_addr >> PAGE_SHIFT;
> > @@ -401,6 +418,8 @@ static int coproc_mmap(struct file *fp, struct
> > vm_area_struct *vma)
> >  
> >  	txwin->task_ref.vma = vma;
> >  
> > +out:
> > +	mutex_unlock(&txwin->task_ref.mmap_mutex);
> 
> If the hypervisor can revoke a window at any point with DLPAR, it's
> not 
> clear *why* this is needed. The hypervisor could cause your window
> to 
> close right after this mmap() returns, right? So an explanation for 
> exactly what this patch is needed for beyond that would help.

Yes, the window can be closed by OS due to DLPAR after the mmap()
returns successfully which is a normal case - paste instruction failure
 until the window is reopened again.

But ths patch is mainly for window open by user space and dlpar happens
before the user space issue mmap().    

I will add more description in the commit log. 

Thanks
Haren

> 
> Thanks,
> Nick
diff mbox series

Patch

diff --git a/arch/powerpc/platforms/book3s/vas-api.c b/arch/powerpc/platforms/book3s/vas-api.c
index a63fd48e34a7..2d06bd1b1935 100644
--- a/arch/powerpc/platforms/book3s/vas-api.c
+++ b/arch/powerpc/platforms/book3s/vas-api.c
@@ -379,10 +379,27 @@  static int coproc_mmap(struct file *fp, struct vm_area_struct *vma)
 		return -EACCES;
 	}
 
+	/*
+	 * The initial mapping is done after the window is opened
+	 * with ioctl. But this window might have been closed
+	 * due to lost credit (core removal on PowerVM) before mmap().
+	 * So if the window is not active, return mmap() failure
+	 * with -EACCES and expects the user space reconfigure (mmap)
+	 * window when it is active again or open new window when
+	 * the credit is available.
+	 */
+	mutex_lock(&txwin->task_ref.mmap_mutex);
+	if (txwin->status != VAS_WIN_ACTIVE) {
+		pr_err("%s(): Window is not active\n", __func__);
+		rc = -EACCES;
+		goto out;
+	}
+
 	paste_addr = cp_inst->coproc->vops->paste_addr(txwin);
 	if (!paste_addr) {
 		pr_err("%s(): Window paste address failed\n", __func__);
-		return -EINVAL;
+		rc = -EINVAL;
+		goto out;
 	}
 
 	pfn = paste_addr >> PAGE_SHIFT;
@@ -401,6 +418,8 @@  static int coproc_mmap(struct file *fp, struct vm_area_struct *vma)
 
 	txwin->task_ref.vma = vma;
 
+out:
+	mutex_unlock(&txwin->task_ref.mmap_mutex);
 	return rc;
 }