[PATCH 1/9] powerpc/64s: Include cpu header

Current powerpc security.c file is defining functions, as
cpu_show_meltdown(), cpu_show_spectre_v{1,2} and others, that are being
declared at linux/cpu.h header without including the header file that
contains these declarations.

This is being reported by sparse, which thinks that these functions are
static, due to the lack of declaration:

	arch/powerpc/kernel/security.c:105:9: warning: symbol 'cpu_show_meltdown' was not declared. Should it be static?
	arch/powerpc/kernel/security.c:139:9: warning: symbol 'cpu_show_spectre_v1' was not declared. Should it be static?
	arch/powerpc/kernel/security.c:161:9: warning: symbol 'cpu_show_spectre_v2' was not declared. Should it be static?
	arch/powerpc/kernel/security.c:209:6: warning: symbol 'stf_barrier' was not declared. Should it be static?
	arch/powerpc/kernel/security.c:289:9: warning: symbol 'cpu_show_spec_store_bypass' was not declared. Should it be static?

This patch simply includes the proper header (linux/cpu.h) to match
function definition and declaration.

Signed-off-by: Breno Leitao <leitao@debian.org>
---
 arch/powerpc/kernel/security.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/powerpc/kernel/security.c b/arch/powerpc/kernel/security.c
index f6f469fc4073..9703dce36307 100644
--- a/arch/powerpc/kernel/security.c
+++ b/arch/powerpc/kernel/security.c

@@ -4,6 +4,7 @@
 //
 // Copyright 2018, Michael Ellerman, IBM Corporation.
 
+#include <linux/cpu.h>
 #include <linux/kernel.h>
 #include <linux/device.h>
 #include <linux/seq_buf.h>

-- 
2.19.0

Function huge_ptep_set_access_flags() has the 'extern' keyword in the
function definition and also in the function declaration. This causes a
warning in 'sparse' since the 'extern' storage class should be used only on
symbol declarations.

	arch/powerpc/mm/pgtable.c:232:12: warning: function 'huge_ptep_set_access_flags' with external linkage has definition

This patch removes the keyword from the definition part, while keeps it in
the declaration part.

Signed-off-by: Breno Leitao <leitao@debian.org>
---
 arch/powerpc/mm/pgtable.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/mm/pgtable.c b/arch/powerpc/mm/pgtable.c
index d71c7777669c..213933b78077 100644
--- a/arch/powerpc/mm/pgtable.c
+++ b/arch/powerpc/mm/pgtable.c

@@ -229,7 +229,7 @@ int ptep_set_access_flags(struct vm_area_struct *vma, unsigned long address,
 }
 
 #ifdef CONFIG_HUGETLB_PAGE
-extern int huge_ptep_set_access_flags(struct vm_area_struct *vma,
+int huge_ptep_set_access_flags(struct vm_area_struct *vma,
 				      unsigned long addr, pte_t *ptep,
 				      pte_t pte, int dirty)
 {

-- 
2.19.0

Currently sparse is complaining about three issues on the xmon code. Two
storage classes issues and a dereferencing a 'noderef'  pointer. These are
the warnings:

	arch/powerpc/xmon/xmon.c:2783:1: warning: symbol 'dump_log_buf' was not declared. Should it be static?
	arch/powerpc/xmon/xmon.c:2989:6: warning: symbol 'format_pte' was not declared. Should it be static?
	arch/powerpc/xmon/xmon.c:2983:30: warning: dereference of noderef expression

This patch fixes all of them, turning both functions static and
dereferencing a pointer calling rcu_dereference() instead of a
straightforward dereference.

Signed-off-by: Breno Leitao <leitao@debian.org>
---
 arch/powerpc/xmon/xmon.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index 4264aedc7775..5c7187923a72 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c

@@ -2779,7 +2779,7 @@ print_address(unsigned long addr)
 	xmon_print_symbol(addr, "\t# ", "");
 }
 
-void
+static void
 dump_log_buf(void)
 {
 	struct kmsg_dumper dumper = { .active = 1 };

@@ -2980,13 +2980,13 @@ static void show_task(struct task_struct *tsk)
 
 	printf("%px %016lx %6d %6d %c %2d %s\n", tsk,
 		tsk->thread.ksp,
-		tsk->pid, tsk->parent->pid,
+		tsk->pid, rcu_dereference(tsk->parent)->pid,
 		state, task_thread_info(tsk)->cpu,
 		tsk->comm);
 }
 
 #ifdef CONFIG_PPC_BOOK3S_64
-void format_pte(void *ptep, unsigned long pte)
+static void format_pte(void *ptep, unsigned long pte)
 {
 	printf("ptep @ 0x%016lx = 0x%016lx\n", (unsigned long)ptep, pte);
 	printf("Maps physical address = 0x%016lx\n", pte & PTE_RPN_MASK);

-- 
2.19.0

Function scom_map_device() returns data type 'scom_map_t', which is a
typedef for 'void *'. This functions is currently returning NULL and zero,
which causes the following warning by 'sparse':

	arch/powerpc/sysdev/scom.c:63:24: warning: Using plain integer as NULL pointer
	arch/powerpc/sysdev/scom.c:86:24: warning: Using plain integer as NULL pointer

This patch simply replaces zero by NULL.

Signed-off-by: Breno Leitao <leitao@debian.org>
---
 arch/powerpc/sysdev/scom.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/sysdev/scom.c b/arch/powerpc/sysdev/scom.c
index 0f6fd5d04d33..a707b24a7ddb 100644
--- a/arch/powerpc/sysdev/scom.c
+++ b/arch/powerpc/sysdev/scom.c

@@ -60,7 +60,7 @@ scom_map_t scom_map_device(struct device_node *dev, int index)
 	parent = scom_find_parent(dev);
 
 	if (parent == NULL)
-		return 0;
+		return NULL;
 
 	/*
 	 * We support "scom-reg" properties for adding scom registers

@@ -83,7 +83,7 @@ scom_map_t scom_map_device(struct device_node *dev, int index)
 	size >>= 2;
 
 	if (index >= (size / (2*cells)))
-		return 0;
+		return NULL;
 
 	reg = of_read_number(&prop[index * cells * 2], cells);
 	cnt = of_read_number(&prop[index * cells * 2 + cells], cells);

-- 
2.19.0

Functions do_stf_{entry,exit}_barrier_fixups are static but not declared as
such. This was detected by `sparse` tool with the following warning:

	arch/powerpc/lib/feature-fixups.c:121:6: warning: symbol 'do_stf_entry_barrier_fixups' was not declared. Should it be static?

	arch/powerpc/lib/feature-fixups.c:171:6: warning: symbol 'do_stf_exit_barrier_fixups' was not declared. Should it be static?

This patch declares both functions as static, as they are only called by
do_stf_barrier_fixups(), which is in the same source code file.

Signed-off-by: Breno Leitao <leitao@debian.org>
---
 arch/powerpc/lib/feature-fixups.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/lib/feature-fixups.c b/arch/powerpc/lib/feature-fixups.c
index e613b02bb2f0..ca8478503f18 100644
--- a/arch/powerpc/lib/feature-fixups.c
+++ b/arch/powerpc/lib/feature-fixups.c

@@ -118,7 +118,7 @@ void do_feature_fixups(unsigned long value, void *fixup_start, void *fixup_end)
 }
 
 #ifdef CONFIG_PPC_BOOK3S_64
-void do_stf_entry_barrier_fixups(enum stf_barrier_type types)
+static void do_stf_entry_barrier_fixups(enum stf_barrier_type types)
 {
 	unsigned int instrs[3], *dest;
 	long *start, *end;

@@ -168,7 +168,7 @@ void do_stf_entry_barrier_fixups(enum stf_barrier_type types)
 		                                           : "unknown");
 }
 
-void do_stf_exit_barrier_fixups(enum stf_barrier_type types)
+static void do_stf_exit_barrier_fixups(enum stf_barrier_type types)
 {
 	unsigned int instrs[6], *dest;
 	long *start, *end;

-- 
2.19.0

There are three symbols (two variables and a function) that are being used
solely in the same file (imc-pmu.c), thus, these symbols should be static,
but they are not. This was detected by sparse:

	arch/powerpc/perf/imc-pmu.c:31:20: warning: symbol 'nest_imc_refc' was not declared. Should it be static?
	arch/powerpc/perf/imc-pmu.c:37:20: warning: symbol 'core_imc_refc' was not declared. Should it be static?
	arch/powerpc/perf/imc-pmu.c:46:16: warning: symbol 'imc_event_to_pmu' was not declared. Should it be static?

This patch simply adds the 'static' storage-class definition to these
symbols, thus, restricting their usage only in the imc-pmu.c file.

Signed-off-by: Breno Leitao <leitao@debian.org>
---
 arch/powerpc/perf/imc-pmu.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/perf/imc-pmu.c b/arch/powerpc/perf/imc-pmu.c
index 1fafc32b12a0..1c693c7726f7 100644
--- a/arch/powerpc/perf/imc-pmu.c
+++ b/arch/powerpc/perf/imc-pmu.c

@@ -28,13 +28,13 @@ static DEFINE_MUTEX(nest_init_lock);
 static DEFINE_PER_CPU(struct imc_pmu_ref *, local_nest_imc_refc);
 static struct imc_pmu **per_nest_pmu_arr;
 static cpumask_t nest_imc_cpumask;
-struct imc_pmu_ref *nest_imc_refc;
+static struct imc_pmu_ref *nest_imc_refc;
 static int nest_pmus;
 
 /* Core IMC data structures and variables */
 
 static cpumask_t core_imc_cpumask;
-struct imc_pmu_ref *core_imc_refc;
+static struct imc_pmu_ref *core_imc_refc;
 static struct imc_pmu *core_imc_pmu;
 
 /* Thread IMC data structures and variables */

@@ -43,7 +43,7 @@ static DEFINE_PER_CPU(u64 *, thread_imc_mem);
 static struct imc_pmu *thread_imc_pmu;
 static int thread_imc_mem_size;
 
-struct imc_pmu *imc_event_to_pmu(struct perf_event *event)
+static struct imc_pmu *imc_event_to_pmu(struct perf_event *event)
 {
 	return container_of(event->pmu, struct imc_pmu, pmu);
 }

-- 
2.19.0

Sparse tool is showing some warnings on pkeys.c file, mainly related to
storage class identifiers. There are static variables and functions not
declared as such. The same thing happens with an extern function, which
misses the header inclusion.

	arch/powerpc/mm/pkeys.c:14:6: warning: symbol 'pkey_execute_disable_supported' was not declared. Should it be static?
	arch/powerpc/mm/pkeys.c:16:6: warning: symbol 'pkeys_devtree_defined' was not declared. Should it be static?
	arch/powerpc/mm/pkeys.c:19:6: warning: symbol 'pkey_amr_mask' was not declared. Should it be static?
	arch/powerpc/mm/pkeys.c:20:6: warning: symbol 'pkey_iamr_mask' was not declared. Should it be static?
	arch/powerpc/mm/pkeys.c:21:6: warning: symbol 'pkey_uamor_mask' was not declared. Should it be static?
	arch/powerpc/mm/pkeys.c:22:6: warning: symbol 'execute_only_key' was not declared. Should it be static?
	arch/powerpc/mm/pkeys.c:60:5: warning: symbol 'pkey_initialize' was not declared. Should it be static?
	arch/powerpc/mm/pkeys.c:404:6: warning: symbol 'arch_vma_access_permitted' was not declared. Should it be static?

This patch fix al the warning, basically turning all global variables that
are not declared as extern at asm/pkeys.h into static.

It also includes asm/mmu_context.h header, which contains the definition of
arch_vma_access_permitted.

Signed-off-by: Breno Leitao <leitao@debian.org>
---
 arch/powerpc/mm/pkeys.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/arch/powerpc/mm/pkeys.c b/arch/powerpc/mm/pkeys.c
index b271b283c785..04b60a8f6e69 100644
--- a/arch/powerpc/mm/pkeys.c
+++ b/arch/powerpc/mm/pkeys.c

@@ -6,20 +6,21 @@
  */
 
 #include <asm/mman.h>
+#include <asm/mmu_context.h>
 #include <asm/setup.h>
 #include <linux/pkeys.h>
 #include <linux/of_device.h>
 
 DEFINE_STATIC_KEY_TRUE(pkey_disabled);
-bool pkey_execute_disable_supported;
 int  pkeys_total;		/* Total pkeys as per device tree */
-bool pkeys_devtree_defined;	/* pkey property exported by device tree */
 u32  initial_allocation_mask;   /* Bits set for the initially allocated keys */
 u32  reserved_allocation_mask;  /* Bits set for reserved keys */
-u64  pkey_amr_mask;		/* Bits in AMR not to be touched */
-u64  pkey_iamr_mask;		/* Bits in AMR not to be touched */
-u64  pkey_uamor_mask;		/* Bits in UMOR not to be touched */
-int  execute_only_key = 2;
+static bool pkey_execute_disable_supported;
+static bool pkeys_devtree_defined;	/* property exported by device tree */
+static u64 pkey_amr_mask;		/* Bits in AMR not to be touched */
+static u64 pkey_iamr_mask;		/* Bits in AMR not to be touched */
+static u64 pkey_uamor_mask;		/* Bits in UMOR not to be touched */
+static int execute_only_key = 2;
 
 #define AMR_BITS_PER_PKEY 2
 #define AMR_RD_BIT 0x1UL

@@ -57,7 +58,7 @@ static inline bool pkey_mmu_enabled(void)
 		return cpu_has_feature(CPU_FTR_PKEY);
 }
 
-int pkey_initialize(void)
+static int pkey_initialize(void)
 {
 	int os_reserved, i;
 

-- 
2.19.0

Function pci_ers_result_name() is a static function, although not declared
as such. This was detected by sparse in the following warning

	arch/powerpc/kernel/eeh_driver.c:63:12: warning: symbol 'pci_ers_result_name' was not declared. Should it be static?

This patch simply declares the function a static.

Signed-off-by: Breno Leitao <leitao@debian.org>
---
 arch/powerpc/kernel/eeh_driver.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/kernel/eeh_driver.c b/arch/powerpc/kernel/eeh_driver.c
index 67619b4b3f96..c325dedfee73 100644
--- a/arch/powerpc/kernel/eeh_driver.c
+++ b/arch/powerpc/kernel/eeh_driver.c

@@ -60,7 +60,7 @@ static int eeh_result_priority(enum pci_ers_result result)
 	}
 };
 
-const char *pci_ers_result_name(enum pci_ers_result result)
+static const char *pci_ers_result_name(enum pci_ers_result result)
 {
 	switch (result) {
 	case PCI_ERS_RESULT_NONE:

-- 
2.19.0

Sparse shows that xive_do_source_eoi() file is defined without any
declaration, thus, it should be a static function.

	arch/powerpc/sysdev/xive/common.c:312:6: warning: symbol 'xive_do_source_eoi' was not declared. Should it be static?

This patch simply turns this symbol into static.

Signed-off-by: Breno Leitao <leitao@debian.org>
---
 arch/powerpc/sysdev/xive/common.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/sysdev/xive/common.c b/arch/powerpc/sysdev/xive/common.c
index 959a2a62f233..6a6178357529 100644
--- a/arch/powerpc/sysdev/xive/common.c
+++ b/arch/powerpc/sysdev/xive/common.c

@@ -309,7 +309,7 @@ static void xive_do_queue_eoi(struct xive_cpu *xc)
  * EOI an interrupt at the source. There are several methods
  * to do this depending on the HW version and source type
  */
-void xive_do_source_eoi(u32 hw_irq, struct xive_irq_data *xd)
+static void xive_do_source_eoi(u32 hw_irq, struct xive_irq_data *xd)
 {
 	/* If the XIVE supports the new "store EOI facility, use it */
 	if (xd->flags & XIVE_IRQ_FLAG_STORE_EOI)

-- 
2.19.0

Breno Leitao [off-list ref] a écrit :

Function huge_ptep_set_access_flags() has the 'extern' keyword in the
function definition and also in the function declaration. This causes a
warning in 'sparse' since the 'extern' storage class should be used only on
symbol declarations.

	arch/powerpc/mm/pgtable.c:232:12: warning: function  
'huge_ptep_set_access_flags' with external linkage has definition

This patch removes the keyword from the definition part, while keeps it in
the declaration part.

I think checkpatch also says that extern should be avoided in declarations.
Can you remove both ?

Christophe

hi Christophe,

On 10/23/2018 12:38 PM, LEROY Christophe wrote:

Breno Leitao [off-list ref] a écrit :

quoted

This patch removes the keyword from the definition part, while keeps
it in
the declaration part.

I think checkpatch also says that extern should be avoided in declarations.

Thanks for the review. I tried to look at this complain, but I didn't see
this behavior on checkpatch.pl from kernel 4.19. I created a commit that adds
a new extern prototype and checked the patch. Take a look:

# git show

	commit 720cd4ee7bf3c0607eaea79e209b719bac79508e
	Author: Breno Leitao [off-list ref]
	Date:   Wed Oct 24 10:31:54 2018 -0400

	powerpc/mm: New test function

	New test function.

	Signed-off-by: Breno Leitao [off-list ref]

	diff --git a/arch/powerpc/include/asm/hugetlb.h
	b/arch/powerpc/include/asm/hugetlb.h
	index 2d00cc530083..4a348e42cab6 100644
	--- a/arch/powerpc/include/asm/hugetlb.h
	+++ b/arch/powerpc/include/asm/hugetlb.h
	@@ -167,6 +167,8 @@ extern int huge_ptep_set_access_flags(struct vm_area_struct *vma,
				      unsigned long addr, pte_t *ptep,
				      pte_t pte, int dirty);

	+extern int test(int foo);
	+
	static inline pte_t huge_ptep_get(pte_t *ptep)
	{
		return *ptep;

	diff --git a/mm/hugetlb.c b/mm/hugetlb.c
	index 5c390f5a5207..2e8f5f77f7f6 100644
	--- a/mm/hugetlb.c
	+++ b/mm/hugetlb.c
	@@ -3204,6 +3204,11 @@ static void set_huge_ptep_writable(struct
	vm_area_struct *vma,
		update_mmu_cache(vma, address, ptep);
	}

	+int test(int foo)
	+{
	+	return foo;
	+}
	+
	bool is_hugetlb_entry_migration(pte_t pte)
	{
	swp_entry_t swp;


# scripts/checkpatch.pl -g HEAD

	total: 0 errors, 0 warnings, 19 lines checked

	Commit 720cd4ee7bf3 ("powerpc/mm: New test function") has no obvious
	style problems and is ready for submission.

Breno Leitao [off-list ref] a écrit :

hi Christophe,

On 10/23/2018 12:38 PM, LEROY Christophe wrote:

quoted

Breno Leitao [off-list ref] a écrit :

quoted

This patch removes the keyword from the definition part, while keeps
it in
the declaration part.

I think checkpatch also says that extern should be avoided in declarations.

Thanks for the review. I tried to look at this complain, but I didn't see
this behavior on checkpatch.pl from kernel 4.19. I created a commit that adds
a new extern prototype and checked the patch. Take a look:

Use option --strict with checkpatch.pl

Christophe

# git show

	commit 720cd4ee7bf3c0607eaea79e209b719bac79508e
	Author: Breno Leitao [off-list ref]
	Date:   Wed Oct 24 10:31:54 2018 -0400

	powerpc/mm: New test function

	New test function.

	Signed-off-by: Breno Leitao [off-list ref]

	diff --git a/arch/powerpc/include/asm/hugetlb.h
	b/arch/powerpc/include/asm/hugetlb.h
	index 2d00cc530083..4a348e42cab6 100644
	--- a/arch/powerpc/include/asm/hugetlb.h
	+++ b/arch/powerpc/include/asm/hugetlb.h
	@@ -167,6 +167,8 @@ extern int huge_ptep_set_access_flags(struct  
vm_area_struct *vma,
				      unsigned long addr, pte_t *ptep,
				      pte_t pte, int dirty);

	+extern int test(int foo);
	+
	static inline pte_t huge_ptep_get(pte_t *ptep)
	{
		return *ptep;

	diff --git a/mm/hugetlb.c b/mm/hugetlb.c
	index 5c390f5a5207..2e8f5f77f7f6 100644
	--- a/mm/hugetlb.c
	+++ b/mm/hugetlb.c
	@@ -3204,6 +3204,11 @@ static void set_huge_ptep_writable(struct
	vm_area_struct *vma,
		update_mmu_cache(vma, address, ptep);
	}

	+int test(int foo)
	+{
	+	return foo;
	+}
	+
	bool is_hugetlb_entry_migration(pte_t pte)
	{
	swp_entry_t swp;


# scripts/checkpatch.pl -g HEAD

	total: 0 errors, 0 warnings, 19 lines checked

	Commit 720cd4ee7bf3 ("powerpc/mm: New test function") has no obvious
	style problems and is ready for submission.

Hi Christophe,

On 10/24/18 12:12 PM, LEROY Christophe wrote:

Breno Leitao [off-list ref] a écrit :

quoted

hi Christophe,

On 10/23/2018 12:38 PM, LEROY Christophe wrote:

quoted

Breno Leitao [off-list ref] a écrit :

quoted

This patch removes the keyword from the definition part, while keeps
it in
the declaration part.

I think checkpatch also says that extern should be avoided in
declarations.

Thanks for the review. I tried to look at this complain, but I didn't see
this behavior on checkpatch.pl from kernel 4.19. I created a commit
that adds
a new extern prototype and checked the patch. Take a look:

Use option --strict with checkpatch.pl

Thanks. I fixed this patch and resent a v2 with this fix.

Michael,

If you happen to accept this series, please ignore this patch (3/9) and
use a v2 as its replacement:

https://patchwork.ozlabs.org/patch/991444/

On Mon, 2018-10-22 at 14:54:12 UTC, Breno Leitao wrote:

Current powerpc security.c file is defining functions, as
cpu_show_meltdown(), cpu_show_spectre_v{1,2} and others, that are being
declared at linux/cpu.h header without including the header file that
contains these declarations.

This is being reported by sparse, which thinks that these functions are
static, due to the lack of declaration:

	arch/powerpc/kernel/security.c:105:9: warning: symbol 'cpu_show_meltdown' was not declared. Should it be static?
	arch/powerpc/kernel/security.c:139:9: warning: symbol 'cpu_show_spectre_v1' was not declared. Should it be static?
	arch/powerpc/kernel/security.c:161:9: warning: symbol 'cpu_show_spectre_v2' was not declared. Should it be static?
	arch/powerpc/kernel/security.c:209:6: warning: symbol 'stf_barrier' was not declared. Should it be static?
	arch/powerpc/kernel/security.c:289:9: warning: symbol 'cpu_show_spec_store_bypass' was not declared. Should it be static?

This patch simply includes the proper header (linux/cpu.h) to match
function definition and declaration.

Signed-off-by: Breno Leitao <leitao@debian.org>

Series applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/42e2acde1237878462b028f5a27d9c

cheers