Re: [PATCH v3 02/10] powerpc/pseries/vas: Add notifier for DLPAR core removal/add

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

On Mon, 2022-02-14 at 12:27 +1000, Nicholas Piggin wrote:
> Excerpts from Haren Myneni's message of January 22, 2022 5:54 am:
> > The hypervisor assigns credits for each LPAR based on number of
> > cores configured in that system. So expects to release credits
> > (means windows) when the core is removed. This patch adds notifier
> > for core removal/add so that the OS closes windows if the system
> > looses credits due to core removal and reopen windows when the
> > credits available later.
> 
> This could be improved. As far as I can tell,
> 
>  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.
> 
> Or can you only re-use credits that you previously lost?

yes, reopen windows / re-use credits when the previously lost credits
are available. 
> 
> > Signed-off-by: Haren Myneni <haren@linux.ibm.com>
> > ---
> >  arch/powerpc/platforms/pseries/vas.c | 37
> > ++++++++++++++++++++++++++++
> >  1 file changed, 37 insertions(+)
> > 
> > diff --git a/arch/powerpc/platforms/pseries/vas.c
> > b/arch/powerpc/platforms/pseries/vas.c
> > index c0737379cc7b..d2c8292bfb33 100644
> > --- a/arch/powerpc/platforms/pseries/vas.c
> > +++ b/arch/powerpc/platforms/pseries/vas.c
> > @@ -538,6 +538,39 @@ static int __init get_vas_capabilities(u8
> > feat, enum vas_cop_feat_type type,
> >  	return 0;
> >  }
> >  
> > +/*
> > + * Total number of default credits available (target_credits)
> > + * in LPAR depends on number of cores configured. It varies based
> > on
> > + * whether processors are in shared mode or dedicated mode.
> > + * Get the notifier when CPU configuration is changed with DLPAR
> > + * operation so that get the new target_credits (vas default
> > capabilities)
> > + * and then update the existing windows usage if needed.
> > + */
> > +static int pseries_vas_notifier(struct notifier_block *nb,
> > +				unsigned long action, void *data)
> > +{
> > +	struct of_reconfig_data *rd = data;
> > +	struct device_node *dn = rd->dn;
> > +	const __be32 *intserv = NULL;
> > +	int len, rc = 0;
> > +
> > +	if ((action == OF_RECONFIG_ATTACH_NODE) ||
> > +		(action == OF_RECONFIG_DETACH_NODE))
> 
> I suppose the OF notifier is the way to do it (cc Nathan).

Using notifier here. registering notifier
with of_reconfig_notifier_register() as in other places (hotplug-
cpu.c pseries_smp_notifier())

> 
> Could this patch be folded in with where it acually does something?
> It 
> makes it easier to review and understand how the notifier is used.

Added this notifier as a seperate patch to make it smaller. Sure, I can
include this patch in 'Add reconfig_close/open_windows() patch'. 
> 
> 
> > +		intserv = of_get_property(dn, "ibm,ppc-interrupt-
> > server#s",
> > +					  &len);
> > +	/*
> > +	 * Processor config is not changed
> > +	 */
> > +	if (!intserv)
> > +		return NOTIFY_OK;
> > +
> > +	return rc;
> > +}
> > +
> > +static struct notifier_block pseries_vas_nb = {
> > +	.notifier_call = pseries_vas_notifier,
> > +};
> > +
> >  static int __init pseries_vas_init(void)
> >  {
> >  	struct hv_vas_cop_feat_caps *hv_cop_caps;
> > @@ -591,6 +624,10 @@ static int __init pseries_vas_init(void)
> >  			goto out_cop;
> >  	}
> >  
> > +	/* Processors can be added/removed only on LPAR */
> 
> What does this comment mean? DLPAR?

I will remve it, basically trying to say that this notifier is called
when core is removed / added. 

Thanks
haren

> 
> Thanks,
> Nick
> 
> > +	if (copypaste_feat && firmware_has_feature(FW_FEATURE_LPAR))
> > +		of_reconfig_notifier_register(&pseries_vas_nb);
> > +
> >  	pr_info("GZIP feature is available\n");
> >  
> >  out_cop:
> > -- 
> > 2.27.0
> > 
> > 
> >