Re: [PATCH v3 3/4] powerpc/pseries/vas: Add VAS migration handler

[Nicholas Piggin <npiggin@gmail.com>]

Excerpts from Haren Myneni's message of February 20, 2022 6:06 am:
> 
> Since the VAS windows belong to the VAS hardware resource, the
> hypervisor expects the partition to close them on source partition
> and reopen them after the partition migrated on the destination
> machine.
> 
> This handler is called before pseries_suspend() to close these
> windows and again invoked after migration. All active windows
> for both default and QoS types will be closed and mark them
> in-active and reopened after migration with this handler.
> During the migration, the user space receives paste instruction
> failure if it issues copy/paste on these in-active windows.
> 
> Signed-off-by: Haren Myneni <haren@linux.ibm.com>
> ---
>  arch/powerpc/platforms/pseries/mobility.c |  5 ++
>  arch/powerpc/platforms/pseries/vas.c      | 86 +++++++++++++++++++++++
>  arch/powerpc/platforms/pseries/vas.h      |  6 ++
>  3 files changed, 97 insertions(+)
> 
> diff --git a/arch/powerpc/platforms/pseries/mobility.c b/arch/powerpc/platforms/pseries/mobility.c
> index 85033f392c78..70004243e25e 100644
> --- a/arch/powerpc/platforms/pseries/mobility.c
> +++ b/arch/powerpc/platforms/pseries/mobility.c
> @@ -26,6 +26,7 @@
>  #include <asm/machdep.h>
>  #include <asm/rtas.h>
>  #include "pseries.h"
> +#include "vas.h"	/* vas_migration_handler() */
>  #include "../../kernel/cacheinfo.h"
>  
>  static struct kobject *mobility_kobj;
> @@ -669,12 +670,16 @@ static int pseries_migrate_partition(u64 handle)
>  	if (ret)
>  		return ret;
>  
> +	vas_migration_handler(VAS_SUSPEND);

Not sure if there is much point having a "handler" like this that only
takes two operations. vas_migration_begin()/vas_migration_end() is
better isn't it?

Other question is why can't the suspend handler return error and handle
it here?

> +
>  	ret = pseries_suspend(handle);
>  	if (ret == 0)
>  		post_mobility_fixup();
>  	else
>  		pseries_cancel_migration(handle, ret);
>  
> +	vas_migration_handler(VAS_RESUME);
> +
>  	return ret;
>  }
>  
> diff --git a/arch/powerpc/platforms/pseries/vas.c b/arch/powerpc/platforms/pseries/vas.c
> index fbcf311da0ec..df22827969db 100644
> --- a/arch/powerpc/platforms/pseries/vas.c
> +++ b/arch/powerpc/platforms/pseries/vas.c
> @@ -869,6 +869,92 @@ static struct notifier_block pseries_vas_nb = {
>  	.notifier_call = pseries_vas_notifier,
>  };
>  
> +/*
> + * For LPM, all windows have to be closed on the source partition
> + * before migration and reopen them on the destination partition
> + * after migration. So closing windows during suspend and
> + * reopen them during resume.
> + */
> +int vas_migration_handler(int action)
> +{
> +	struct vas_cop_feat_caps *caps;
> +	int old_nr_creds, new_nr_creds = 0;
> +	struct vas_caps *vcaps;
> +	int i, rc = 0;
> +
> +	/*
> +	 * NX-GZIP is not enabled. Nothing to do for migration.
> +	 */
> +	if (!copypaste_feat)
> +		return rc;
> +
> +	mutex_lock(&vas_pseries_mutex);
> +
> +	for (i = 0; i < VAS_MAX_FEAT_TYPE; i++) {
> +		vcaps = &vascaps[i];
> +		caps = &vcaps->caps;
> +		old_nr_creds = atomic_read(&caps->nr_total_credits);
> +
> +		rc = h_query_vas_capabilities(H_QUERY_VAS_CAPABILITIES,
> +					      vcaps->feat,
> +					      (u64)virt_to_phys(&hv_cop_caps));
> +		if (!rc) {
> +			new_nr_creds = be16_to_cpu(hv_cop_caps.target_lpar_creds);
> +			/*
> +			 * Should not happen. But incase print messages, close
> +			 * all windows in the list during suspend and reopen
> +			 * windows based on new lpar_creds on the destination
> +			 * system.
> +			 */
> +			if (old_nr_creds != new_nr_creds) {
> +				pr_err("state(%d): lpar creds: %d HV lpar creds: %d\n",
> +					action, old_nr_creds, new_nr_creds);
> +				pr_err("Used creds: %d, Active creds: %d\n",
> +					atomic_read(&caps->nr_used_credits),
> +					vcaps->nr_open_windows - vcaps->nr_close_wins);

Error messages should have some vague use to the administrator even if 
you don't expect it or they aren't expected to know much.

"VAS NX error during LPM: credit mismatch blah"

Otherwise if it's a Linux or hypervisor bug then make it a WARN_ON, at 
least then by convention the administrator knows that should be reported
and it's (possibly) non-fatal.

Thanks,
Nick

> +			}
> +		} else {
> +			pr_err("state(%d): Get VAS capabilities failed with %d\n",
> +				action, rc);
> +			/*
> +			 * We can not stop migration with the current lpm
> +			 * implementation. So continue closing all windows in
> +			 * the list (during suspend) and return without
> +			 * opening windows (during resume) if VAS capabilities
> +			 * HCALL failed.
> +			 */
> +			if (action == VAS_RESUME)
> +				goto out;
> +		}
> +
> +		switch (action) {
> +		case VAS_SUSPEND:
> +			rc = reconfig_close_windows(vcaps, vcaps->nr_open_windows,
> +							true);
> +			break;
> +		case VAS_RESUME:
> +			atomic_set(&caps->nr_total_credits, new_nr_creds);
> +			rc = reconfig_open_windows(vcaps, new_nr_creds, true);
> +			break;
> +		default:
> +			/* should not happen */
> +			pr_err("Invalid migration action %d\n", action);
> +			rc = -EINVAL;
> +			goto out;
> +		}
> +
> +		/*
> +		 * Ignore errors during suspend and return for resume.
> +		 */
> +		if (rc && (action == VAS_RESUME))
> +			goto out;
> +	}
> +
> +out:
> +	mutex_unlock(&vas_pseries_mutex);
> +	return rc;
> +}
> +
>  static int __init pseries_vas_init(void)
>  {
>  	struct hv_vas_all_caps *hv_caps;
> diff --git a/arch/powerpc/platforms/pseries/vas.h b/arch/powerpc/platforms/pseries/vas.h
> index 4ddb1001a0aa..f7568d8c6ab0 100644
> --- a/arch/powerpc/platforms/pseries/vas.h
> +++ b/arch/powerpc/platforms/pseries/vas.h
> @@ -33,6 +33,11 @@
>  #define VAS_GZIP_QOS_CAPABILITIES	0x56516F73477A6970
>  #define VAS_GZIP_DEFAULT_CAPABILITIES	0x56446566477A6970
>  
> +enum vas_migrate_action {
> +	VAS_SUSPEND,
> +	VAS_RESUME,
> +};
> +
>  /*
>   * Co-processor feature - GZIP QoS windows or GZIP default windows
>   */
> @@ -132,4 +137,5 @@ struct pseries_vas_window {
>  int sysfs_add_vas_caps(struct vas_cop_feat_caps *caps);
>  int vas_reconfig_capabilties(u8 type);
>  int __init sysfs_pseries_vas_init(struct vas_all_caps *vas_caps);
> +int vas_migration_handler(int action);
>  #endif /* _VAS_H */
> -- 
> 2.27.0
> 
> 
>