[PATCH v2 0/6] powerpc/perf/hv_{gpci,24x7}: fixes

[PATCH v2 0/6] powerpc/perf/hv_{gpci,24x7}: fixes

[Cody P Schafer]

 - 24x7 and gpci probing now uses pr_debug() and doesn't pad to 80 characters
 - Catalog access is fixed for LE kernels
 - remove c99 feature sparse doesn't like
 - 1 device attr made static


Cody P Schafer (6):
  powerpc/perf/hv_24x7: probe errors changed to pr_debug(), padding
    fixed
  powerpc/perf/hv_gpci: probe failures use pr_debug(), and padding
    reduced
  powerpc/perf/hv-gpci: make device attr static
  powerpc/perf/hv-24x7: use (unsigned long) not (u32) values when
    calling plpar_hcall_norets()
  powerpc/perf/hv-24x7: remove [static 4096], sparse chokes on it
  powerpc/perf/hv-24x7: catalog version number is be64, not be32

 arch/powerpc/perf/hv-24x7.c | 30 +++++++++++++++++++++---------
 arch/powerpc/perf/hv-gpci.c |  6 +++---
 2 files changed, 24 insertions(+), 12 deletions(-)

-- 
1.9.2

[PATCH v2 1/6] powerpc/perf/hv_24x7: probe errors changed to pr_debug(), padding fixed

[Cody P Schafer]

fixup for "powerpc/perf: Add support for the hv 24x7 interface"

Makes the "not enabled" message less awful (and hides it in most cases).

Signed-off-by: Cody P Schafer <cody@linux.vnet.ibm.com>
---
 arch/powerpc/perf/hv-24x7.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/perf/hv-24x7.c b/arch/powerpc/perf/hv-24x7.c
index 297c9105..f5bca73 100644
--- a/arch/powerpc/perf/hv-24x7.c
+++ b/arch/powerpc/perf/hv-24x7.c
@@ -485,13 +485,13 @@ static int hv_24x7_init(void)
 	struct hv_perf_caps caps;
 
 	if (!firmware_has_feature(FW_FEATURE_LPAR)) {
-		pr_info("not a virtualized system, not enabling\n");
+		pr_debug("not a virtualized system, not enabling\n");
 		return -ENODEV;
 	}
 
 	hret = hv_perf_caps_get(&caps);
 	if (hret) {
-		pr_info("could not obtain capabilities, error 0x%80lx, not enabling\n",
+		pr_debug("could not obtain capabilities, not enabling, rc=%ld\n",
 				hret);
 		return -ENODEV;
 	}
-- 
1.9.2

[PATCH v2 2/6] powerpc/perf/hv_gpci: probe failures use pr_debug(), and padding reduced

[Cody P Schafer]

fixup for "powerpc/perf: Add support for the hv gpci (get performance
counter info) interface".

Makes the "not enabled" message less awful (and hidden unless
debugging).

Signed-off-by: Cody P Schafer <cody@linux.vnet.ibm.com>
---
 arch/powerpc/perf/hv-gpci.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/perf/hv-gpci.c b/arch/powerpc/perf/hv-gpci.c
index 278ba7b..8fee1dc 100644
--- a/arch/powerpc/perf/hv-gpci.c
+++ b/arch/powerpc/perf/hv-gpci.c
@@ -273,13 +273,13 @@ static int hv_gpci_init(void)
 	struct hv_perf_caps caps;
 
 	if (!firmware_has_feature(FW_FEATURE_LPAR)) {
-		pr_info("not a virtualized system, not enabling\n");
+		pr_debug("not a virtualized system, not enabling\n");
 		return -ENODEV;
 	}
 
 	hret = hv_perf_caps_get(&caps);
 	if (hret) {
-		pr_info("could not obtain capabilities, error 0x%80lx, not enabling\n",
+		pr_debug("could not obtain capabilities, not enabling, rc=%ld\n",
 				hret);
 		return -ENODEV;
 	}
-- 
1.9.2

[PATCH v2 3/6] powerpc/perf/hv-gpci: make device attr static

[Cody P Schafer]

Signed-off-by: Cody P Schafer <cody@linux.vnet.ibm.com>
---
 arch/powerpc/perf/hv-gpci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/perf/hv-gpci.c b/arch/powerpc/perf/hv-gpci.c
index 8fee1dc..c9d399a 100644
--- a/arch/powerpc/perf/hv-gpci.c
+++ b/arch/powerpc/perf/hv-gpci.c
@@ -78,7 +78,7 @@ static ssize_t kernel_version_show(struct device *dev,
 	return sprintf(page, "0x%x\n", COUNTER_INFO_VERSION_CURRENT);
 }
 
-DEVICE_ATTR_RO(kernel_version);
+static DEVICE_ATTR_RO(kernel_version);
 HV_CAPS_ATTR(version, "0x%x\n");
 HV_CAPS_ATTR(ga, "%d\n");
 HV_CAPS_ATTR(expanded, "%d\n");
-- 
1.9.2

[PATCH v2 4/6] powerpc/perf/hv-24x7: use (unsigned long) not (u32) values when calling plpar_hcall_norets()

[Cody P Schafer]

Signed-off-by: Cody P Schafer <cody@linux.vnet.ibm.com>
---
 arch/powerpc/perf/hv-24x7.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/perf/hv-24x7.c b/arch/powerpc/perf/hv-24x7.c
index f5bca73..3e8f60a 100644
--- a/arch/powerpc/perf/hv-24x7.c
+++ b/arch/powerpc/perf/hv-24x7.c
@@ -155,16 +155,28 @@ static ssize_t read_offset_data(void *dest, size_t dest_len,
 	return copy_len;
 }
 
-static unsigned long h_get_24x7_catalog_page(char page[static 4096],
-					     u32 version, u32 index)
+static unsigned long h_get_24x7_catalog_page_(unsigned long phys_4096,
+					      unsigned long version,
+					      unsigned long index)
 {
-	WARN_ON(!IS_ALIGNED((unsigned long)page, 4096));
+	pr_devel("h_get_24x7_catalog_page(0x%lx, %lu, %lu)",
+			phys_4096,
+			version,
+			index);
+	WARN_ON(!IS_ALIGNED(phys_4096, 4096));
 	return plpar_hcall_norets(H_GET_24X7_CATALOG_PAGE,
-			virt_to_phys(page),
+			phys_4096,
 			version,
 			index);
 }
 
+static unsigned long h_get_24x7_catalog_page(char page[static 4096],
+					     u32 version, u32 index)
+{
+	return h_get_24x7_catalog_page_(virt_to_phys(page),
+					version, index);
+}
+
 static ssize_t catalog_read(struct file *filp, struct kobject *kobj,
 			    struct bin_attribute *bin_attr, char *buf,
 			    loff_t offset, size_t count)
-- 
1.9.2

[PATCH v2 5/6] powerpc/perf/hv-24x7: remove [static 4096], sparse chokes on it

[Cody P Schafer]

Signed-off-by: Cody P Schafer <cody@linux.vnet.ibm.com>
---
 arch/powerpc/perf/hv-24x7.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/perf/hv-24x7.c b/arch/powerpc/perf/hv-24x7.c
index 3e8f60a..95a67f8 100644
--- a/arch/powerpc/perf/hv-24x7.c
+++ b/arch/powerpc/perf/hv-24x7.c
@@ -170,7 +170,7 @@ static unsigned long h_get_24x7_catalog_page_(unsigned long phys_4096,
 			index);
 }
 
-static unsigned long h_get_24x7_catalog_page(char page[static 4096],
+static unsigned long h_get_24x7_catalog_page(char page[],
 					     u32 version, u32 index)
 {
 	return h_get_24x7_catalog_page_(virt_to_phys(page),
-- 
1.9.2

[PATCH v2 6/6] powerpc/perf/hv-24x7: catalog version number is be64, not be32

[Cody P Schafer]

The catalog version number was changed from a be32 (with proceeding
32bits of padding) to a be64, update the code to treat it as a be64

Signed-off-by: Cody P Schafer <cody@linux.vnet.ibm.com>
---
 arch/powerpc/perf/hv-24x7.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/perf/hv-24x7.c b/arch/powerpc/perf/hv-24x7.c
index 95a67f8..9d4badc 100644
--- a/arch/powerpc/perf/hv-24x7.c
+++ b/arch/powerpc/perf/hv-24x7.c
@@ -171,7 +171,7 @@ static unsigned long h_get_24x7_catalog_page_(unsigned long phys_4096,
 }
 
 static unsigned long h_get_24x7_catalog_page(char page[],
-					     u32 version, u32 index)
+					     u64 version, u32 index)
 {
 	return h_get_24x7_catalog_page_(virt_to_phys(page),
 					version, index);
@@ -185,7 +185,7 @@ static ssize_t catalog_read(struct file *filp, struct kobject *kobj,
 	ssize_t ret = 0;
 	size_t catalog_len = 0, catalog_page_len = 0, page_count = 0;
 	loff_t page_offset = 0;
-	uint32_t catalog_version_num = 0;
+	uint64_t catalog_version_num = 0;
 	void *page = kmem_cache_alloc(hv_page_cache, GFP_USER);
 	struct hv_24x7_catalog_page_0 *page_0 = page;
 	if (!page)
@@ -197,7 +197,7 @@ static ssize_t catalog_read(struct file *filp, struct kobject *kobj,
 		goto e_free;
 	}
 
-	catalog_version_num = be32_to_cpu(page_0->version);
+	catalog_version_num = be64_to_cpu(page_0->version);
 	catalog_page_len = be32_to_cpu(page_0->length);
 	catalog_len = catalog_page_len * 4096;
 
@@ -255,7 +255,7 @@ e_free:								\
 static DEVICE_ATTR_RO(_name)
 
 PAGE_0_ATTR(catalog_version, "%lld\n",
-		(unsigned long long)be32_to_cpu(page_0->version));
+		(unsigned long long)be64_to_cpu(page_0->version));
 PAGE_0_ATTR(catalog_len, "%lld\n",
 		(unsigned long long)be32_to_cpu(page_0->length) * 4096);
 static BIN_ATTR_RO(catalog, 0/* real length varies */);
-- 
1.9.2

Re: [PATCH v2 6/6] powerpc/perf/hv-24x7: catalog version number is be64, not be32

[Benjamin Herrenschmidt]

On Tue, 2014-04-15 at 10:10 -0700, Cody P Schafer wrote:
> The catalog version number was changed from a be32 (with proceeding
> 32bits of padding) to a be64, update the code to treat it as a be64
> 
> Signed-off-by: Cody P Schafer <cody@linux.vnet.ibm.com>
> --

Have you tested this ?

It doesn't build for me:

arch/powerpc/perf/hv-24x7.c: In function 'catalog_read':
arch/powerpc/perf/hv-24x7.c:223:3: error: format '%d' expects argument of type 'int', but argument 2 has type 'uint64_t' [-Werror=format]
cc1: all warnings being treated as errors

I'll fix that up in my tree.

Cheers,
Ben.

>  arch/powerpc/perf/hv-24x7.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/powerpc/perf/hv-24x7.c b/arch/powerpc/perf/hv-24x7.c
> index 95a67f8..9d4badc 100644
> --- a/arch/powerpc/perf/hv-24x7.c
> +++ b/arch/powerpc/perf/hv-24x7.c
> @@ -171,7 +171,7 @@ static unsigned long h_get_24x7_catalog_page_(unsigned long phys_4096,
>  }
>  
>  static unsigned long h_get_24x7_catalog_page(char page[],
> -					     u32 version, u32 index)
> +					     u64 version, u32 index)
>  {
>  	return h_get_24x7_catalog_page_(virt_to_phys(page),
>  					version, index);
> @@ -185,7 +185,7 @@ static ssize_t catalog_read(struct file *filp, struct kobject *kobj,
>  	ssize_t ret = 0;
>  	size_t catalog_len = 0, catalog_page_len = 0, page_count = 0;
>  	loff_t page_offset = 0;
> -	uint32_t catalog_version_num = 0;
> +	uint64_t catalog_version_num = 0;
>  	void *page = kmem_cache_alloc(hv_page_cache, GFP_USER);
>  	struct hv_24x7_catalog_page_0 *page_0 = page;
>  	if (!page)
> @@ -197,7 +197,7 @@ static ssize_t catalog_read(struct file *filp, struct kobject *kobj,
>  		goto e_free;
>  	}
>  
> -	catalog_version_num = be32_to_cpu(page_0->version);
> +	catalog_version_num = be64_to_cpu(page_0->version);
>  	catalog_page_len = be32_to_cpu(page_0->length);
>  	catalog_len = catalog_page_len * 4096;
>  
> @@ -255,7 +255,7 @@ e_free:								\
>  static DEVICE_ATTR_RO(_name)
>  
>  PAGE_0_ATTR(catalog_version, "%lld\n",
> -		(unsigned long long)be32_to_cpu(page_0->version));
> +		(unsigned long long)be64_to_cpu(page_0->version));
>  PAGE_0_ATTR(catalog_len, "%lld\n",
>  		(unsigned long long)be32_to_cpu(page_0->length) * 4096);
>  static BIN_ATTR_RO(catalog, 0/* real length varies */);

Re: [PATCH v2 6/6] powerpc/perf/hv-24x7: catalog version number is be64, not be32

[Cody P Schafer]

On 04/27/2014 09:47 PM, Benjamin Herrenschmidt wrote:
> On Tue, 2014-04-15 at 10:10 -0700, Cody P Schafer wrote:
>> The catalog version number was changed from a be32 (with proceeding
>> 32bits of padding) to a be64, update the code to treat it as a be64
>>
>> Signed-off-by: Cody P Schafer <cody@linux.vnet.ibm.com>
>> --
>
> Have you tested this ?
>
> It doesn't build for me:
>
> arch/powerpc/perf/hv-24x7.c: In function 'catalog_read':
> arch/powerpc/perf/hv-24x7.c:223:3: error: format '%d' expects argument of type 'int', but argument 2 has type 'uint64_t' [-Werror=format]
> cc1: all warnings being treated as errors

I have, and I wasn't initially sure how I managed to miss that 
warning-as-error. On examination: My config (for some reason) has 
CONFIG_PPC_DISABLE_WERROR=y set (probably because it's a variation of a 
distro config). Must have been piping the warnings to a file and 
forgotten to check the file.

> I'll fix that up in my tree.

Thanks.

Re: [PATCH v2 6/6] powerpc/perf/hv-24x7: catalog version number is be64, not be32

[Michael Ellerman]

On Sun, 2014-04-27 at 21:59 -0700, Cody P Schafer wrote:
> On 04/27/2014 09:47 PM, Benjamin Herrenschmidt wrote:
> > On Tue, 2014-04-15 at 10:10 -0700, Cody P Schafer wrote:
> >> The catalog version number was changed from a be32 (with proceeding
> >> 32bits of padding) to a be64, update the code to treat it as a be64
> >>
> >> Signed-off-by: Cody P Schafer <cody@linux.vnet.ibm.com>
> >> --
> >
> > Have you tested this ?
> >
> > It doesn't build for me:
> >
> > arch/powerpc/perf/hv-24x7.c: In function 'catalog_read':
> > arch/powerpc/perf/hv-24x7.c:223:3: error: format '%d' expects argument of type 'int', but argument 2 has type 'uint64_t' [-Werror=format]
> > cc1: all warnings being treated as errors
> 
> I have, and I wasn't initially sure how I managed to miss that 
> warning-as-error. On examination: My config (for some reason) has 
> CONFIG_PPC_DISABLE_WERROR=y set (probably because it's a variation of a 
> distro config).

Please test build with ppc64_defconfig at least.

cheers