Re: [PATCH v2 2/6] xen/PMU: Sysfs interface for setting Xen PMU mode

[David Vrabel <david.vrabel@citrix.com>]

On 06/06/14 18:44, Boris Ostrovsky wrote:
> Set Xen's PMU mode via /sys/hypervisor/pmu/pmu_mode. Add XENPMU hypercall.

sysfs files need documentation in Documentaton/ABI/

> --- a/drivers/xen/sys-hypervisor.c
> +++ b/drivers/xen/sys-hypervisor.c
> @@ -20,6 +20,7 @@
>  #include <xen/xenbus.h>
>  #include <xen/interface/xen.h>
>  #include <xen/interface/version.h>
> +#include <xen/interface/xenpmu.h>
>  
>  #define HYPERVISOR_ATTR_RO(_name) \
>  static struct hyp_sysfs_attr  _name##_attr = __ATTR_RO(_name)
> @@ -368,6 +369,116 @@ static void xen_properties_destroy(void)
>  	sysfs_remove_group(hypervisor_kobj, &xen_properties_group);
>  }
>  
> +struct pmu_mode {
> +	const char *name;
> +	uint32_t mode;
> +};
> +
> +struct pmu_mode pmu_modes[] = {
> +	{"enable", XENPMU_MODE_ON},
> +	{"priv_enable", XENPMU_MODE_PRIV},
> +	{"disable", 0}
> +};


If there a better, more description set of options here?  How about:

"self", "all", "none"?

> +
> +static ssize_t pmu_mode_store(struct hyp_sysfs_attr *attr,
> +			      const char *buffer, size_t len)
> +{
> +	int ret;
> +	struct xen_pmu_params xp;
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(pmu_modes); i++) {
> +		if (!strncmp(buffer, pmu_modes[i].name,
> +			     strlen(pmu_modes[i].name))) {

I prefer if (strncmp(...) == 0) as I find the use of the not operator
for a positive match confusing.

David