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

[Haren Myneni <haren@linux.ibm.com>]

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