From mboxrd@z Thu Jan  1 00:00:00 1970
From: David Vrabel <david.vrabel@citrix.com>
Subject: Re: [PATCH v2 2/6] xen/PMU: Sysfs interface for setting
	Xen PMU mode
Date: Tue, 10 Jun 2014 14:48:32 +0100
Message-ID: <53970CB0.6030700@citrix.com>
References: <1402076686-26586-1-git-send-email-boris.ostrovsky@oracle.com>
	<1402076686-26586-3-git-send-email-boris.ostrovsky@oracle.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <1402076686-26586-3-git-send-email-boris.ostrovsky@oracle.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Boris Ostrovsky <boris.ostrovsky@oracle.com>, konrad.wilk@oracle.com
Cc: kevin.tian@intel.com, dietmar.hahn@ts.fujitsu.com, JBeulich@suse.com, xen-devel@lists.xen.org
List-Id: xen-devel@lists.xenproject.org

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