diff mbox series

[v4,5/9] powerpc/vas: Map paste address only if window is active

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

Commit Message

Haren Myneni Feb. 19, 2022, 7:59 p.m. UTC
The paste address mapping is done with mmap() after the window is
opened with ioctl. If the window is closed by OS in the hypervisor
due to DLPAR after this mmap(), the paste instruction returns
failure until the OS reopens this window again. But before mmap(),
DLPAR core removal can happen which causes the corresponding
window in-active. 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 a new window when the credit
is available.

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

Comments

Nicholas Piggin Feb. 23, 2022, 7:11 a.m. UTC | #1
Excerpts from Haren Myneni's message of February 20, 2022 5:59 am:
> 
> The paste address mapping is done with mmap() after the window is
> opened with ioctl. If the window is closed by OS in the hypervisor
> due to DLPAR after this mmap(), the paste instruction returns

I don't think the changelog was improved here.

The window is closed by the OS in response to a DLPAR operation
by the hypervisor? The OS can't be in the hypervisor.


> failure until the OS reopens this window again. But before mmap(),
> DLPAR core removal can happen which causes the corresponding
> window in-active. 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 a new window when the credit
> is available.
> 
> Signed-off-by: Haren Myneni <haren@linux.ibm.com>
> ---
>  arch/powerpc/platforms/book3s/vas-api.c | 20 +++++++++++++++++++-
>  1 file changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/platforms/book3s/vas-api.c b/arch/powerpc/platforms/book3s/vas-api.c
> index f3e421511ea6..eb4489b2b46b 100644
> --- a/arch/powerpc/platforms/book3s/vas-api.c
> +++ b/arch/powerpc/platforms/book3s/vas-api.c
> @@ -496,10 +496,26 @@ static int coproc_mmap(struct file *fp, struct vm_area_struct *vma)
>  		return -EACCES;
>  	}
>  
> +	/*
> +	 * The initial mmap is done after the window is opened
> +	 * with ioctl. But before mmap(), this window can be closed in
> +	 * the hypervisor due to lost credit (core removal on pseries).
> +	 * So if the window is not active, return mmap() failure with
> +	 * -EACCES and expects the user space reissue mmap() 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;
> @@ -519,6 +535,8 @@ static int coproc_mmap(struct file *fp, struct vm_area_struct *vma)
>  	txwin->task_ref.vma = vma;
>  	vma->vm_ops = &vas_vm_ops;
>  
> +out:
> +	mutex_unlock(&txwin->task_ref.mmap_mutex);

Did we have an explanation or what mmap_mutex is protecting? Sorry if 
you explained it and I forgot -- would be good to have a small comment
(what is it protecting against).

Thanks,
Nick
Haren Myneni Feb. 23, 2022, 8:02 a.m. UTC | #2
On Wed, 2022-02-23 at 17:11 +1000, Nicholas Piggin wrote:
> Excerpts from Haren Myneni's message of February 20, 2022 5:59 am:
> > The paste address mapping is done with mmap() after the window is
> > opened with ioctl. If the window is closed by OS in the hypervisor
> > due to DLPAR after this mmap(), the paste instruction returns
> 
> I don't think the changelog was improved here.
> 
> The window is closed by the OS in response to a DLPAR operation
> by the hypervisor? The OS can't be in the hypervisor.

From the user space point of view, this window is inactive. But from
the hypervisor point of view, this window is closed. So my point is the
window is closed by the OS in the hypervisor for the DLPAR event.

> 
> 
> > failure until the OS reopens this window again. But before mmap(),
> > DLPAR core removal can happen which causes the corresponding
> > window in-active. 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 a new window when the credit
> > is available.
> > 
> > Signed-off-by: Haren Myneni <haren@linux.ibm.com>
> > ---
> >  arch/powerpc/platforms/book3s/vas-api.c | 20 +++++++++++++++++++-
> >  1 file changed, 19 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/powerpc/platforms/book3s/vas-api.c
> > b/arch/powerpc/platforms/book3s/vas-api.c
> > index f3e421511ea6..eb4489b2b46b 100644
> > --- a/arch/powerpc/platforms/book3s/vas-api.c
> > +++ b/arch/powerpc/platforms/book3s/vas-api.c
> > @@ -496,10 +496,26 @@ static int coproc_mmap(struct file *fp,
> > struct vm_area_struct *vma)
> >  		return -EACCES;
> >  	}
> >  
> > +	/*
> > +	 * The initial mmap is done after the window is opened
> > +	 * with ioctl. But before mmap(), this window can be closed in
> > +	 * the hypervisor due to lost credit (core removal on pseries).
> > +	 * So if the window is not active, return mmap() failure with
> > +	 * -EACCES and expects the user space reissue mmap() 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;
> > @@ -519,6 +535,8 @@ static int coproc_mmap(struct file *fp, struct
> > vm_area_struct *vma)
> >  	txwin->task_ref.vma = vma;
> >  	vma->vm_ops = &vas_vm_ops;
> >  
> > +out:
> > +	mutex_unlock(&txwin->task_ref.mmap_mutex);
> 
> Did we have an explanation or what mmap_mutex is protecting? Sorry
> if 
> you explained it and I forgot -- would be good to have a small
> comment
> (what is it protecting against).

The comment should be in the struct definition. I will add some comment
in this patch.

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 f3e421511ea6..eb4489b2b46b 100644
--- a/arch/powerpc/platforms/book3s/vas-api.c
+++ b/arch/powerpc/platforms/book3s/vas-api.c
@@ -496,10 +496,26 @@  static int coproc_mmap(struct file *fp, struct vm_area_struct *vma)
 		return -EACCES;
 	}
 
+	/*
+	 * The initial mmap is done after the window is opened
+	 * with ioctl. But before mmap(), this window can be closed in
+	 * the hypervisor due to lost credit (core removal on pseries).
+	 * So if the window is not active, return mmap() failure with
+	 * -EACCES and expects the user space reissue mmap() 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;
@@ -519,6 +535,8 @@  static int coproc_mmap(struct file *fp, struct vm_area_struct *vma)
 	txwin->task_ref.vma = vma;
 	vma->vm_ops = &vas_vm_ops;
 
+out:
+	mutex_unlock(&txwin->task_ref.mmap_mutex);
 	return rc;
 }