Re: [PATCH v4 6/9] powerpc/pseries/vas: Close windows with DLPAR core removal

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

On Wed, 2022-02-23 at 17:23 +1000, Nicholas Piggin wrote:
> Excerpts from Haren Myneni's message of February 20, 2022 6:00 am:
> > The hypervisor assigns vas credits (windows) for each LPAR based
> > on the number of cores configured in that system. The OS is
> > expected to release credits when cores are removed, and may
> > allocate more when cores are added. So there is a possibility of
> > using excessive credits (windows) in the LPAR and the hypervisor
> > expects the system to close the excessive windows so that NX load
> > can be equally distributed across all LPARs in the system.
> > 
> > When the OS closes the excessive windows in the hypervisor,
> > it sets the window status in-active and invalidates window
> > virtual address mapping. The user space receives paste instruction
> > failure if any NX requests are issued on the in-active window.
> 
> Thanks for adding this paragraph. Then presumably userspace can
> update their windows and be able to re-try with an available open
> window?
 
yes, the user space should be able to manage with the available open
windows or fall back to SW compression if can. Added this comment in
the fault hanlder patch.
> 
> in-active can be one word, not hyphenated.
> 
> 
> > This patch also adds the notifier for core removal/add to close
> > windows in the hypervisor if the system lost credits (core
> > removal) and reopen windows in the hypervisor when the previously
> > lost credits are available.
> > 
> > Signed-off-by: Haren Myneni <haren@linux.ibm.com>
> > ---
> >  arch/powerpc/include/asm/vas.h       |   2 +
> >  arch/powerpc/platforms/pseries/vas.c | 207
> > +++++++++++++++++++++++++--
> >  arch/powerpc/platforms/pseries/vas.h |   3 +
> >  3 files changed, 204 insertions(+), 8 deletions(-)
> > 
> > diff --git a/arch/powerpc/include/asm/vas.h
> > b/arch/powerpc/include/asm/vas.h
> > index 27251af18c65..6baf7b9ffed4 100644
> > --- a/arch/powerpc/include/asm/vas.h
> > +++ b/arch/powerpc/include/asm/vas.h
> > @@ -34,6 +34,8 @@
> >   */
> >  #define VAS_WIN_ACTIVE		0x0	/* Used in platform
> > independent */
> >  					/* vas mmap() */
> > +/* Window is closed in the hypervisor due to lost credit */
> > +#define VAS_WIN_NO_CRED_CLOSE	0x00000001
> 
> I thought we were getting a different status for software
> status vs status rturned by hypervisor?

vas_window->status is only for Linux status bits which are used for
active, DLPAR close or migration. We do not need status returned from
hypervisor right now. In case if needed in future, hv_status will be
added in pseries_vas_window. 

> 
> > diff --git a/arch/powerpc/platforms/pseries/vas.h
> > b/arch/powerpc/platforms/pseries/vas.h
> > index 2872532ed72a..701363cfd7c1 100644
> > --- a/arch/powerpc/platforms/pseries/vas.h
> > +++ b/arch/powerpc/platforms/pseries/vas.h
> > @@ -83,6 +83,9 @@ struct vas_cop_feat_caps {
> >  struct vas_caps {
> >  	struct vas_cop_feat_caps caps;
> >  	struct list_head list;	/* List of open windows */
> > +	int nr_close_wins;	/* closed windows in the hypervisor for
> > DLPAR */
> > +	int nr_open_windows;	/* Number of successful open
> > windows */
> > +	u8 feat;		/* Feature type */
> >  };
> 
> Still not entirely sold on the idea that nr_open_windows is a feature
> or capability, but if the code works out easier this way, sometimes
> these little hacks are reasonable.

nr_close_wins / nr_open_windows - not a capability or feature, but
these are used to track active windows and needed for DLPAR / migration
- Means total number of open windows and the actual number of windows
closed in the hypervisor. hence I did not add these elements in
 vas_cop_feat_caps struct.

Thanks
Haren

> 
> Thanks,
> Nick