Re: [PATCH v3 09/10] powerpc/pseries/vas: sysfs interface to export capabilities

[Nicholas Piggin <npiggin@gmail.com>]

Excerpts from Haren Myneni's message of January 22, 2022 6:00 am:
> 
> The hypervisor provides the available VAS GZIP capabilities such
> as default or QoS window type and the target available credits in
> each type. This patch creates sysfs entries and exports the target,
> used and the available credits for each feature.
> 
> This interface can be used by the user space to determine the credits
> usage or to set the target credits in the case of QoS type (for DLPAR).
> 
> /sys/devices/vas/vas0/gzip/def_caps: (default GZIP capabilities)
> 	avail_creds /* Available credits to use */
> 	target_creds /* Total credits available. Can be
> 			 /* changed with DLPAR operation */
> 	used_creds  /* Used credits */
> 
> /sys/devices/vas/vas0/gzip/qos_caps (QoS GZIP capabilities)
> 	avail_creds
> 	target_creds
> 	used_creds

Can we not use these abbreviations in sysfs?

In the code it's one thing (I still don't prefer them but that's up to 
you), but we should be a bit better in external ABIs. Can we also get a 
better description of the difference between available credits and 
target credits? They both appear to be credits available.

default_capabilities
  nr_available_credits
  nr_target_credits
  nr_used_credits

qos_capabilities
  ...

Keeping this description in the tree rather than just in the changelog
would be nice too.

> 
> Signed-off-by: Haren Myneni <haren@linux.ibm.com>
> ---
>  arch/powerpc/platforms/pseries/Makefile    |   2 +-
>  arch/powerpc/platforms/pseries/vas-sysfs.c | 218 +++++++++++++++++++++
>  arch/powerpc/platforms/pseries/vas.c       |   6 +
>  arch/powerpc/platforms/pseries/vas.h       |   6 +
>  4 files changed, 231 insertions(+), 1 deletion(-)
>  create mode 100644 arch/powerpc/platforms/pseries/vas-sysfs.c
> 
> diff --git a/arch/powerpc/platforms/pseries/Makefile b/arch/powerpc/platforms/pseries/Makefile
> index ee60b59024b4..29b522d2c755 100644
> --- a/arch/powerpc/platforms/pseries/Makefile
> +++ b/arch/powerpc/platforms/pseries/Makefile
> @@ -29,6 +29,6 @@ obj-$(CONFIG_PPC_SVM)		+= svm.o
>  obj-$(CONFIG_FA_DUMP)		+= rtas-fadump.o
>  
>  obj-$(CONFIG_SUSPEND)		+= suspend.o
> -obj-$(CONFIG_PPC_VAS)		+= vas.o
> +obj-$(CONFIG_PPC_VAS)		+= vas.o vas-sysfs.o
>  
>  obj-$(CONFIG_ARCH_HAS_CC_PLATFORM)	+= cc_platform.o
> diff --git a/arch/powerpc/platforms/pseries/vas-sysfs.c b/arch/powerpc/platforms/pseries/vas-sysfs.c
> new file mode 100644
> index 000000000000..f7609cdef8f8
> --- /dev/null
> +++ b/arch/powerpc/platforms/pseries/vas-sysfs.c
> @@ -0,0 +1,218 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright 2016-17 IBM Corp.

Seems to have suffered a copy-paste problem.

> +	/*
> +	 * The hypervisor does not expose multiple VAS instances, but can
> +	 * see multiple VAS instances on PowerNV. So create 'vas0' directory
> +	 * on PowerVM.

powernv / pseries when you're talking about the APIs or Linux 
implementation (or you can use OPAL / PAPR for APIs specifically too).

Thanks,
Nick

> +	 */
> +	pseries_vas_kobj = kobject_create_and_add("vas0",
> +					&vas_miscdev.this_device->kobj);
> +	if (!pseries_vas_kobj) {
> +		pr_err("Failed to create VAS sysfs entry\n");
> +		return -ENOMEM;
> +	}
> +
> +	if ((vas_caps->feat_type & VAS_GZIP_QOS_FEAT_BIT) ||
> +		(vas_caps->feat_type & VAS_GZIP_DEF_FEAT_BIT)) {
> +		gzip_caps_kobj = kobject_create_and_add("gzip",
> +						       pseries_vas_kobj);
> +		if (!gzip_caps_kobj) {
> +			pr_err("Failed to create VAS GZIP capability entry\n");
> +			kobject_put(pseries_vas_kobj);
> +			return -ENOMEM;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +#else
> +int sysfs_add_vas_caps(struct vas_cop_feat_caps *caps)
> +{
> +	return 0;
> +}
> +
> +int __init sysfs_pseries_vas_init(struct vas_all_caps *vas_caps)
> +{
> +	return 0;
> +}
> +#endif
> diff --git a/arch/powerpc/platforms/pseries/vas.c b/arch/powerpc/platforms/pseries/vas.c
> index 75ccd0a599ec..32098e4a2786 100644
> --- a/arch/powerpc/platforms/pseries/vas.c
> +++ b/arch/powerpc/platforms/pseries/vas.c
> @@ -560,6 +560,10 @@ static int __init get_vas_capabilities(u8 feat, enum vas_cop_feat_type type,
>  		}
>  	}
>  
> +	rc = sysfs_add_vas_caps(caps);
> +	if (rc)
> +		return rc;
> +
>  	copypaste_feat = true;
>  
>  	return 0;
> @@ -843,6 +847,8 @@ static int __init pseries_vas_init(void)
>  	caps_all.descriptor = be64_to_cpu(hv_caps->descriptor);
>  	caps_all.feat_type = be64_to_cpu(hv_caps->feat_type);
>  
> +	sysfs_pseries_vas_init(&caps_all);
> +
>  	hv_cop_caps = kmalloc(sizeof(*hv_cop_caps), GFP_KERNEL);
>  	if (!hv_cop_caps) {
>  		rc = -ENOMEM;
> diff --git a/arch/powerpc/platforms/pseries/vas.h b/arch/powerpc/platforms/pseries/vas.h
> index 8ce9b84693e8..bc393bd74030 100644
> --- a/arch/powerpc/platforms/pseries/vas.h
> +++ b/arch/powerpc/platforms/pseries/vas.h
> @@ -24,6 +24,9 @@
>  #define VAS_COPY_PASTE_USER_MODE	0x00000001
>  #define VAS_COP_OP_USER_MODE		0x00000010
>  
> +#define VAS_GZIP_QOS_CAPABILITIES	0x56516F73477A6970
> +#define VAS_GZIP_DEFAULT_CAPABILITIES	0x56446566477A6970
> +
>  /*
>   * Co-processor feature - GZIP QoS windows or GZIP default windows
>   */
> @@ -120,4 +123,7 @@ struct pseries_vas_window {
>  	char *name;
>  	int fault_virq;
>  };
> +
> +int sysfs_add_vas_caps(struct vas_cop_feat_caps *caps);
> +int __init sysfs_pseries_vas_init(struct vas_all_caps *vas_caps);
>  #endif /* _VAS_H */
> -- 
> 2.27.0
> 
> 
>