[PATCH 0/6] W=1 fixes

[PATCH 0/6] W=1 fixes

[Cédric Le Goater]

Hello,

With this small series, I could compile the ppc kernel with W=1. There
are certainly other configs which will need more fixes but it's a good
start. 

The last 2 patches look hacky. Christophe, could you help with these
to find a better place to include the declarations ?

Thanks,

C.

Cédric Le Goater (6):
  powerpc/prom: Introduce early_reserve_mem_old()
  powerpc/pseries/vas: Declare pseries_vas_fault_thread_fn() as static
  KVM: PPC: Book3S PR: Declare kvmppc_handle_exit_pr()
  KVM: PPC: Book3S PR: Remove unused variable
  audit: Declare ppc32_classify_syscall()
  powerpc/compat_sys: Declare syscalls

 arch/powerpc/include/asm/syscalls.h  | 31 +++++++++++++++++++++++
 arch/powerpc/include/asm/unistd.h    |  3 +++
 arch/powerpc/kvm/book3s.h            |  1 +
 arch/powerpc/kernel/audit.c          |  1 -
 arch/powerpc/kernel/prom.c           | 37 +++++++++++++++-------------
 arch/powerpc/kvm/book3s_64_mmu.c     |  3 +--
 arch/powerpc/platforms/pseries/vas.c |  2 +-
 7 files changed, 57 insertions(+), 21 deletions(-)

-- 
2.31.1

[PATCH 1/6] powerpc/prom: Introduce early_reserve_mem_old()

[Cédric Le Goater]

and condition its call with IS_ENABLED(CONFIG_PPC32). This fixes a
compile error with W=1.

arch/powerpc/kernel/prom.c: In function ‘early_reserve_mem’:
arch/powerpc/kernel/prom.c:625:10: error: variable ‘reserve_map’ set but not used [-Werror=unused-but-set-variable]
  __be64 *reserve_map;
          ^~~~~~~~~~~
cc1: all warnings being treated as errors

Cc: Christophe Leroy <christophe.leroy@c-s.fr>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---

 Christophe, I think you had comments on this one ? Yes, I am being a bit lazy.

 arch/powerpc/kernel/prom.c | 37 ++++++++++++++++++++-----------------
 1 file changed, 20 insertions(+), 17 deletions(-)

diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
index f620e04dc9bf..52869d12bc1d 100644
--- a/arch/powerpc/kernel/prom.c
+++ b/arch/powerpc/kernel/prom.c
@@ -621,27 +621,14 @@ static void __init early_reserve_mem_dt(void)
 	}
 }
 
-static void __init early_reserve_mem(void)
+static void __init early_reserve_mem_old(void)
 {
 	__be64 *reserve_map;
 
 	reserve_map = (__be64 *)(((unsigned long)initial_boot_params) +
 			fdt_off_mem_rsvmap(initial_boot_params));
 
-	/* Look for the new "reserved-regions" property in the DT */
-	early_reserve_mem_dt();
-
-#ifdef CONFIG_BLK_DEV_INITRD
-	/* Then reserve the initrd, if any */
-	if (initrd_start && (initrd_end > initrd_start)) {
-		memblock_reserve(ALIGN_DOWN(__pa(initrd_start), PAGE_SIZE),
-			ALIGN(initrd_end, PAGE_SIZE) -
-			ALIGN_DOWN(initrd_start, PAGE_SIZE));
-	}
-#endif /* CONFIG_BLK_DEV_INITRD */
-
-#ifdef CONFIG_PPC32
-	/* 
+	/*
 	 * Handle the case where we might be booting from an old kexec
 	 * image that setup the mem_rsvmap as pairs of 32-bit values
 	 */
@@ -659,9 +646,25 @@ static void __init early_reserve_mem(void)
 			DBG("reserving: %x -> %x\n", base_32, size_32);
 			memblock_reserve(base_32, size_32);
 		}
-		return;
 	}
-#endif
+}
+
+static void __init early_reserve_mem(void)
+{
+	/* Look for the new "reserved-regions" property in the DT */
+	early_reserve_mem_dt();
+
+#ifdef CONFIG_BLK_DEV_INITRD
+	/* Then reserve the initrd, if any */
+	if (initrd_start && (initrd_end > initrd_start)) {
+		memblock_reserve(ALIGN_DOWN(__pa(initrd_start), PAGE_SIZE),
+			ALIGN(initrd_end, PAGE_SIZE) -
+			ALIGN_DOWN(initrd_start, PAGE_SIZE));
+	}
+#endif /* CONFIG_BLK_DEV_INITRD */
+
+	if (IS_ENABLED(CONFIG_PPC32))
+		early_reserve_mem_old();
 }
 
 #ifdef CONFIG_PPC_TRANSACTIONAL_MEM
-- 
2.31.1

Re: [PATCH 1/6] powerpc/prom: Introduce early_reserve_mem_old()

[Christophe Leroy]

Le 19/08/2021 à 14:56, Cédric Le Goater a écrit :
> and condition its call with IS_ENABLED(CONFIG_PPC32). This fixes a
> compile error with W=1.
> 
> arch/powerpc/kernel/prom.c: In function ‘early_reserve_mem’:
> arch/powerpc/kernel/prom.c:625:10: error: variable ‘reserve_map’ set but not used [-Werror=unused-but-set-variable]
>    __be64 *reserve_map;
>            ^~~~~~~~~~~
> cc1: all warnings being treated as errors
> 
> Cc: Christophe Leroy <christophe.leroy@c-s.fr>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
> 
>   Christophe, I think you had comments on this one ? Yes, I am being a bit lazy.


Yeah, my comment was to leave thing almost as is, just drop the #ifdef CONFIG_PPC32 and instead put 
something like:

	if (!IS_ENABLED(CONFIG_PPC32))
		return;

> 
>   arch/powerpc/kernel/prom.c | 37 ++++++++++++++++++++-----------------
>   1 file changed, 20 insertions(+), 17 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
> index f620e04dc9bf..52869d12bc1d 100644
> --- a/arch/powerpc/kernel/prom.c
> +++ b/arch/powerpc/kernel/prom.c
> @@ -621,27 +621,14 @@ static void __init early_reserve_mem_dt(void)
>   	}
>   }
>   
> -static void __init early_reserve_mem(void)
> +static void __init early_reserve_mem_old(void)

Why old ? Because ppc32 ?

I think that's more changes than needed.



>   {
>   	__be64 *reserve_map;
>   
>   	reserve_map = (__be64 *)(((unsigned long)initial_boot_params) +
>   			fdt_off_mem_rsvmap(initial_boot_params));
>   
> -	/* Look for the new "reserved-regions" property in the DT */
> -	early_reserve_mem_dt();
> -
> -#ifdef CONFIG_BLK_DEV_INITRD
> -	/* Then reserve the initrd, if any */
> -	if (initrd_start && (initrd_end > initrd_start)) {
> -		memblock_reserve(ALIGN_DOWN(__pa(initrd_start), PAGE_SIZE),
> -			ALIGN(initrd_end, PAGE_SIZE) -
> -			ALIGN_DOWN(initrd_start, PAGE_SIZE));
> -	}
> -#endif /* CONFIG_BLK_DEV_INITRD */
> -
> -#ifdef CONFIG_PPC32
> -	/*
> +	/*
>   	 * Handle the case where we might be booting from an old kexec
>   	 * image that setup the mem_rsvmap as pairs of 32-bit values
>   	 */
> @@ -659,9 +646,25 @@ static void __init early_reserve_mem(void)
>   			DBG("reserving: %x -> %x\n", base_32, size_32);
>   			memblock_reserve(base_32, size_32);
>   		}
> -		return;
>   	}
> -#endif
> +}
> +
> +static void __init early_reserve_mem(void)
> +{
> +	/* Look for the new "reserved-regions" property in the DT */
> +	early_reserve_mem_dt();
> +
> +#ifdef CONFIG_BLK_DEV_INITRD
> +	/* Then reserve the initrd, if any */
> +	if (initrd_start && (initrd_end > initrd_start)) {
> +		memblock_reserve(ALIGN_DOWN(__pa(initrd_start), PAGE_SIZE),
> +			ALIGN(initrd_end, PAGE_SIZE) -
> +			ALIGN_DOWN(initrd_start, PAGE_SIZE));
> +	}
> +#endif /* CONFIG_BLK_DEV_INITRD */
> +
> +	if (IS_ENABLED(CONFIG_PPC32))
> +		early_reserve_mem_old();
>   }
>   
>   #ifdef CONFIG_PPC_TRANSACTIONAL_MEM
> 

Re: [PATCH 1/6] powerpc/prom: Introduce early_reserve_mem_old()

[Michael Ellerman]

Christophe Leroy <christophe.leroy@csgroup.eu> writes:
> Le 19/08/2021 à 14:56, Cédric Le Goater a écrit :
>> and condition its call with IS_ENABLED(CONFIG_PPC32). This fixes a
>> compile error with W=1.
>> 
>> arch/powerpc/kernel/prom.c: In function ‘early_reserve_mem’:
>> arch/powerpc/kernel/prom.c:625:10: error: variable ‘reserve_map’ set but not used [-Werror=unused-but-set-variable]
>>    __be64 *reserve_map;
>>            ^~~~~~~~~~~
>> cc1: all warnings being treated as errors
>> 
>> Cc: Christophe Leroy <christophe.leroy@c-s.fr>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>> 
>>   Christophe, I think you had comments on this one ? Yes, I am being a bit lazy.
>
>
> Yeah, my comment was to leave thing almost as is, just drop the #ifdef CONFIG_PPC32 and instead put 
> something like:
>
> 	if (!IS_ENABLED(CONFIG_PPC32))
> 		return;

Yeah that's cleaner, let's do that.

cheers

Re: [PATCH 1/6] powerpc/prom: Introduce early_reserve_mem_old()

[Cédric Le Goater]

On 8/19/21 4:42 PM, Christophe Leroy wrote:
> 
> 
> Le 19/08/2021 à 14:56, Cédric Le Goater a écrit :
>> and condition its call with IS_ENABLED(CONFIG_PPC32). This fixes a
>> compile error with W=1.
>>
>> arch/powerpc/kernel/prom.c: In function ‘early_reserve_mem’:
>> arch/powerpc/kernel/prom.c:625:10: error: variable ‘reserve_map’ set but not used [-Werror=unused-but-set-variable]
>>    __be64 *reserve_map;
>>            ^~~~~~~~~~~
>> cc1: all warnings being treated as errors
>>
>> Cc: Christophe Leroy <christophe.leroy@c-s.fr>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>>
>>   Christophe, I think you had comments on this one ? Yes, I am being a bit lazy.
> 
> 
> Yeah, my comment was to leave thing almost as is, just drop the #ifdef CONFIG_PPC32 and instead put something like:
> 
>     if (!IS_ENABLED(CONFIG_PPC32))
>         return;
> 
>>
>>   arch/powerpc/kernel/prom.c | 37 ++++++++++++++++++++-----------------
>>   1 file changed, 20 insertions(+), 17 deletions(-)
>>
>> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
>> index f620e04dc9bf..52869d12bc1d 100644
>> --- a/arch/powerpc/kernel/prom.c
>> +++ b/arch/powerpc/kernel/prom.c
>> @@ -621,27 +621,14 @@ static void __init early_reserve_mem_dt(void)
>>       }
>>   }
>>   -static void __init early_reserve_mem(void)
>> +static void __init early_reserve_mem_old(void)
> 
> Why old ? Because ppc32 ?

No. because there is message a bit below saying :

		DBG("Found old 32-bit reserve map\n");

> I think that's more changes than needed.

OK. np. I will use your suggestion.

Thanks,

C. 

 
> 
> 
>>   {
>>       __be64 *reserve_map;
>>         reserve_map = (__be64 *)(((unsigned long)initial_boot_params) +
>>               fdt_off_mem_rsvmap(initial_boot_params));
>>   -    /* Look for the new "reserved-regions" property in the DT */
>> -    early_reserve_mem_dt();
>> -
>> -#ifdef CONFIG_BLK_DEV_INITRD
>> -    /* Then reserve the initrd, if any */
>> -    if (initrd_start && (initrd_end > initrd_start)) {
>> -        memblock_reserve(ALIGN_DOWN(__pa(initrd_start), PAGE_SIZE),
>> -            ALIGN(initrd_end, PAGE_SIZE) -
>> -            ALIGN_DOWN(initrd_start, PAGE_SIZE));
>> -    }
>> -#endif /* CONFIG_BLK_DEV_INITRD */
>> -
>> -#ifdef CONFIG_PPC32
>> -    /*
>> +    /*
>>        * Handle the case where we might be booting from an old kexec
>>        * image that setup the mem_rsvmap as pairs of 32-bit values
>>        */
>> @@ -659,9 +646,25 @@ static void __init early_reserve_mem(void)
>>               DBG("reserving: %x -> %x\n", base_32, size_32);
>>               memblock_reserve(base_32, size_32);
>>           }
>> -        return;
>>       }
>> -#endif
>> +}
>> +
>> +static void __init early_reserve_mem(void)
>> +{
>> +    /* Look for the new "reserved-regions" property in the DT */
>> +    early_reserve_mem_dt();
>> +
>> +#ifdef CONFIG_BLK_DEV_INITRD
>> +    /* Then reserve the initrd, if any */
>> +    if (initrd_start && (initrd_end > initrd_start)) {
>> +        memblock_reserve(ALIGN_DOWN(__pa(initrd_start), PAGE_SIZE),
>> +            ALIGN(initrd_end, PAGE_SIZE) -
>> +            ALIGN_DOWN(initrd_start, PAGE_SIZE));
>> +    }
>> +#endif /* CONFIG_BLK_DEV_INITRD */
>> +
>> +    if (IS_ENABLED(CONFIG_PPC32))
>> +        early_reserve_mem_old();
>>   }
>>     #ifdef CONFIG_PPC_TRANSACTIONAL_MEM
>>

[PATCH 2/6] powerpc/pseries/vas: Declare pseries_vas_fault_thread_fn() as static

[Cédric Le Goater]

This fixes a compile error with W=1.

Fixes: 6d0aaf5e0de0 ("powerpc/pseries/vas: Setup IRQ and fault handling")
Cc: Haren Myneni <haren@linux.ibm.com>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 arch/powerpc/platforms/pseries/vas.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/platforms/pseries/vas.c b/arch/powerpc/platforms/pseries/vas.c
index b5c1cf1bc64d..b043e3936d21 100644
--- a/arch/powerpc/platforms/pseries/vas.c
+++ b/arch/powerpc/platforms/pseries/vas.c
@@ -184,7 +184,7 @@ static int h_get_nx_fault(u32 winid, u64 buffer)
  * Note: The hypervisor forwards an interrupt for each fault request.
  *	So one fault CRB to process for each H_GET_NX_FAULT hcall.
  */
-irqreturn_t pseries_vas_fault_thread_fn(int irq, void *data)
+static irqreturn_t pseries_vas_fault_thread_fn(int irq, void *data)
 {
 	struct pseries_vas_window *txwin = data;
 	struct coprocessor_request_block crb;
-- 
2.31.1

[PATCH 3/6] KVM: PPC: Book3S PR: Declare kvmppc_handle_exit_pr()

[Cédric Le Goater]

This fixes a compile error with W=1.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 arch/powerpc/kvm/book3s.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/powerpc/kvm/book3s.h b/arch/powerpc/kvm/book3s.h
index 740e51def5a5..c08f93b7f523 100644
--- a/arch/powerpc/kvm/book3s.h
+++ b/arch/powerpc/kvm/book3s.h
@@ -24,6 +24,7 @@ extern int kvmppc_core_emulate_mfspr_pr(struct kvm_vcpu *vcpu,
 					int sprn, ulong *spr_val);
 extern int kvmppc_book3s_init_pr(void);
 extern void kvmppc_book3s_exit_pr(void);
+extern int kvmppc_handle_exit_pr(struct kvm_vcpu *vcpu, unsigned int exit_nr);
 
 #ifdef CONFIG_PPC_TRANSACTIONAL_MEM
 extern void kvmppc_emulate_tabort(struct kvm_vcpu *vcpu, int ra_val);
-- 
2.31.1

Re: [PATCH 3/6] KVM: PPC: Book3S PR: Declare kvmppc_handle_exit_pr()

[Christophe Leroy]

Le 19/08/2021 à 14:56, Cédric Le Goater a écrit :
> This fixes a compile error with W=1.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>   arch/powerpc/kvm/book3s.h | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/arch/powerpc/kvm/book3s.h b/arch/powerpc/kvm/book3s.h
> index 740e51def5a5..c08f93b7f523 100644
> --- a/arch/powerpc/kvm/book3s.h
> +++ b/arch/powerpc/kvm/book3s.h
> @@ -24,6 +24,7 @@ extern int kvmppc_core_emulate_mfspr_pr(struct kvm_vcpu *vcpu,
>   					int sprn, ulong *spr_val);
>   extern int kvmppc_book3s_init_pr(void);
>   extern void kvmppc_book3s_exit_pr(void);
> +extern int kvmppc_handle_exit_pr(struct kvm_vcpu *vcpu, unsigned int exit_nr);

Don't add new 'extern' keywords, they are useless and pointless for functions prototypes.

>   
>   #ifdef CONFIG_PPC_TRANSACTIONAL_MEM
>   extern void kvmppc_emulate_tabort(struct kvm_vcpu *vcpu, int ra_val);
> 

Re: [PATCH 3/6] KVM: PPC: Book3S PR: Declare kvmppc_handle_exit_pr()

[Michael Ellerman]

Christophe Leroy <christophe.leroy@csgroup.eu> writes:
> Le 19/08/2021 à 14:56, Cédric Le Goater a écrit :
>> This fixes a compile error with W=1.
>> 
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>>   arch/powerpc/kvm/book3s.h | 1 +
>>   1 file changed, 1 insertion(+)
>> 
>> diff --git a/arch/powerpc/kvm/book3s.h b/arch/powerpc/kvm/book3s.h
>> index 740e51def5a5..c08f93b7f523 100644
>> --- a/arch/powerpc/kvm/book3s.h
>> +++ b/arch/powerpc/kvm/book3s.h
>> @@ -24,6 +24,7 @@ extern int kvmppc_core_emulate_mfspr_pr(struct kvm_vcpu *vcpu,
>>   					int sprn, ulong *spr_val);
>>   extern int kvmppc_book3s_init_pr(void);
>>   extern void kvmppc_book3s_exit_pr(void);
>> +extern int kvmppc_handle_exit_pr(struct kvm_vcpu *vcpu, unsigned int exit_nr);
>
> Don't add new 'extern' keywords, they are useless and pointless for functions prototypes.

I dropped it when applying.

cheers

[PATCH 4/6] KVM: PPC: Book3S PR: Remove unused variable

[Cédric Le Goater]

This fixes a compile error with W=1.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---

 May be, this was sent already ? 

 arch/powerpc/kvm/book3s_64_mmu.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/powerpc/kvm/book3s_64_mmu.c b/arch/powerpc/kvm/book3s_64_mmu.c
index 26b8b27a3755..feee40cb2ba1 100644
--- a/arch/powerpc/kvm/book3s_64_mmu.c
+++ b/arch/powerpc/kvm/book3s_64_mmu.c
@@ -196,7 +196,7 @@ static int kvmppc_mmu_book3s_64_xlate(struct kvm_vcpu *vcpu, gva_t eaddr,
 	hva_t ptegp;
 	u64 pteg[16];
 	u64 avpn = 0;
-	u64 v, r;
+	u64 r;
 	u64 v_val, v_mask;
 	u64 eaddr_mask;
 	int i;
@@ -285,7 +285,6 @@ static int kvmppc_mmu_book3s_64_xlate(struct kvm_vcpu *vcpu, gva_t eaddr,
 		goto do_second;
 	}
 
-	v = be64_to_cpu(pteg[i]);
 	r = be64_to_cpu(pteg[i+1]);
 	pp = (r & HPTE_R_PP) | key;
 	if (r & HPTE_R_PP0)
-- 
2.31.1

[PATCH 5/6] audit: Declare ppc32_classify_syscall()

[Cédric Le Goater]

This fixes a compile error with W=1.

Cc: Christophe Leroy <christophe.leroy@c-s.fr>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---

 I don't think this is correct. Which file could we use ? 

 arch/powerpc/include/asm/unistd.h | 3 +++
 arch/powerpc/kernel/audit.c       | 1 -
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/include/asm/unistd.h b/arch/powerpc/include/asm/unistd.h
index b541c690a31c..d9025a7e973c 100644
--- a/arch/powerpc/include/asm/unistd.h
+++ b/arch/powerpc/include/asm/unistd.h
@@ -47,6 +47,9 @@
 #define __ARCH_WANT_SYS_UTIME
 #define __ARCH_WANT_SYS_NEWFSTATAT
 #define __ARCH_WANT_COMPAT_SYS_SENDFILE
+#ifdef CONFIG_AUDIT
+extern int ppc32_classify_syscall(unsigned int syscall);
+#endif
 #endif
 #define __ARCH_WANT_SYS_FORK
 #define __ARCH_WANT_SYS_VFORK
diff --git a/arch/powerpc/kernel/audit.c b/arch/powerpc/kernel/audit.c
index a2dddd7f3d09..c3c6c6a1069b 100644
--- a/arch/powerpc/kernel/audit.c
+++ b/arch/powerpc/kernel/audit.c
@@ -41,7 +41,6 @@ int audit_classify_arch(int arch)
 int audit_classify_syscall(int abi, unsigned syscall)
 {
 #ifdef CONFIG_PPC64
-	extern int ppc32_classify_syscall(unsigned);
 	if (abi == AUDIT_ARCH_PPC)
 		return ppc32_classify_syscall(syscall);
 #endif
-- 
2.31.1

Re: [PATCH 5/6] audit: Declare ppc32_classify_syscall()

[Christophe Leroy]

Le 19/08/2021 à 14:56, Cédric Le Goater a écrit :
> This fixes a compile error with W=1.
> 
> Cc: Christophe Leroy <christophe.leroy@c-s.fr>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
> 
>   I don't think this is correct. Which file could we use ?

I think you can completely remove ppc32_classify_syscall(), and instead add the following in the 
default case in audit_classify_syscall():

  	default:
+		if (IS_ENABLED(CONFIG_PPC64) && abi == AUDIT_ARCH_PPC)
+			return 1;
  		return 0;


> 
>   arch/powerpc/include/asm/unistd.h | 3 +++
>   arch/powerpc/kernel/audit.c       | 1 -
>   2 files changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/include/asm/unistd.h b/arch/powerpc/include/asm/unistd.h
> index b541c690a31c..d9025a7e973c 100644
> --- a/arch/powerpc/include/asm/unistd.h
> +++ b/arch/powerpc/include/asm/unistd.h
> @@ -47,6 +47,9 @@
>   #define __ARCH_WANT_SYS_UTIME
>   #define __ARCH_WANT_SYS_NEWFSTATAT
>   #define __ARCH_WANT_COMPAT_SYS_SENDFILE
> +#ifdef CONFIG_AUDIT
> +extern int ppc32_classify_syscall(unsigned int syscall);
> +#endif
>   #endif
>   #define __ARCH_WANT_SYS_FORK
>   #define __ARCH_WANT_SYS_VFORK
> diff --git a/arch/powerpc/kernel/audit.c b/arch/powerpc/kernel/audit.c
> index a2dddd7f3d09..c3c6c6a1069b 100644
> --- a/arch/powerpc/kernel/audit.c
> +++ b/arch/powerpc/kernel/audit.c
> @@ -41,7 +41,6 @@ int audit_classify_arch(int arch)
>   int audit_classify_syscall(int abi, unsigned syscall)
>   {
>   #ifdef CONFIG_PPC64
> -	extern int ppc32_classify_syscall(unsigned);
>   	if (abi == AUDIT_ARCH_PPC)
>   		return ppc32_classify_syscall(syscall);
>   #endif
> 

Re: [PATCH 5/6] audit: Declare ppc32_classify_syscall()

[Christophe Leroy]

Le 19/08/2021 à 16:56, Christophe Leroy a écrit :
> 
> 
> Le 19/08/2021 à 14:56, Cédric Le Goater a écrit :
>> This fixes a compile error with W=1.
>>
>> Cc: Christophe Leroy <christophe.leroy@c-s.fr>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>>
>>   I don't think this is correct. Which file could we use ?
> 
> I think you can completely remove ppc32_classify_syscall(), and instead add the following in the 
> default case in audit_classify_syscall():
> 
>       default:
> +        if (IS_ENABLED(CONFIG_PPC64) && abi == AUDIT_ARCH_PPC)
> +            return 1;
>           return 0;
> 

After looking more in details, in fact I think we should convert powerpc to 
CONFIG_AUDIT_ARCH_COMPAT_GENERIC

> 
>>
>>   arch/powerpc/include/asm/unistd.h | 3 +++
>>   arch/powerpc/kernel/audit.c       | 1 -
>>   2 files changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/powerpc/include/asm/unistd.h b/arch/powerpc/include/asm/unistd.h
>> index b541c690a31c..d9025a7e973c 100644
>> --- a/arch/powerpc/include/asm/unistd.h
>> +++ b/arch/powerpc/include/asm/unistd.h
>> @@ -47,6 +47,9 @@
>>   #define __ARCH_WANT_SYS_UTIME
>>   #define __ARCH_WANT_SYS_NEWFSTATAT
>>   #define __ARCH_WANT_COMPAT_SYS_SENDFILE
>> +#ifdef CONFIG_AUDIT
>> +extern int ppc32_classify_syscall(unsigned int syscall);
>> +#endif
>>   #endif
>>   #define __ARCH_WANT_SYS_FORK
>>   #define __ARCH_WANT_SYS_VFORK
>> diff --git a/arch/powerpc/kernel/audit.c b/arch/powerpc/kernel/audit.c
>> index a2dddd7f3d09..c3c6c6a1069b 100644
>> --- a/arch/powerpc/kernel/audit.c
>> +++ b/arch/powerpc/kernel/audit.c
>> @@ -41,7 +41,6 @@ int audit_classify_arch(int arch)
>>   int audit_classify_syscall(int abi, unsigned syscall)
>>   {
>>   #ifdef CONFIG_PPC64
>> -    extern int ppc32_classify_syscall(unsigned);
>>       if (abi == AUDIT_ARCH_PPC)
>>           return ppc32_classify_syscall(syscall);
>>   #endif
>>

Re: [PATCH 5/6] audit: Declare ppc32_classify_syscall()

[Christophe Leroy]

Le 19/08/2021 à 16:56, Christophe Leroy a écrit :
> 
> 
> Le 19/08/2021 à 14:56, Cédric Le Goater a écrit :
>> This fixes a compile error with W=1.
>>
>> Cc: Christophe Leroy <christophe.leroy@c-s.fr>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>>
>>   I don't think this is correct. Which file could we use ?
> 
> I think you can completely remove ppc32_classify_syscall(), and instead add the following in the 
> default case in audit_classify_syscall():
> 
>       default:
> +        if (IS_ENABLED(CONFIG_PPC64) && abi == AUDIT_ARCH_PPC)
> +            return 1;
>           return 0;
> 

Forget that comment, it was crazy, because PPC32 and PPC64 use different syscall numbers.

By the way, I have submitted a patch to completely remove this stuff, see 
https://patchwork.ozlabs.org/project/linuxppc-dev/patch/dc14509a28a993738b1325211f412be72a4f9b1e.1629701132.git.christophe.leroy@csgroup.eu/

> 
>>
>>   arch/powerpc/include/asm/unistd.h | 3 +++
>>   arch/powerpc/kernel/audit.c       | 1 -
>>   2 files changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/powerpc/include/asm/unistd.h b/arch/powerpc/include/asm/unistd.h
>> index b541c690a31c..d9025a7e973c 100644
>> --- a/arch/powerpc/include/asm/unistd.h
>> +++ b/arch/powerpc/include/asm/unistd.h
>> @@ -47,6 +47,9 @@
>>   #define __ARCH_WANT_SYS_UTIME
>>   #define __ARCH_WANT_SYS_NEWFSTATAT
>>   #define __ARCH_WANT_COMPAT_SYS_SENDFILE
>> +#ifdef CONFIG_AUDIT
>> +extern int ppc32_classify_syscall(unsigned int syscall);
>> +#endif
>>   #endif
>>   #define __ARCH_WANT_SYS_FORK
>>   #define __ARCH_WANT_SYS_VFORK
>> diff --git a/arch/powerpc/kernel/audit.c b/arch/powerpc/kernel/audit.c
>> index a2dddd7f3d09..c3c6c6a1069b 100644
>> --- a/arch/powerpc/kernel/audit.c
>> +++ b/arch/powerpc/kernel/audit.c
>> @@ -41,7 +41,6 @@ int audit_classify_arch(int arch)
>>   int audit_classify_syscall(int abi, unsigned syscall)
>>   {
>>   #ifdef CONFIG_PPC64
>> -    extern int ppc32_classify_syscall(unsigned);
>>       if (abi == AUDIT_ARCH_PPC)
>>           return ppc32_classify_syscall(syscall);
>>   #endif
>>

Re: [PATCH 5/6] audit: Declare ppc32_classify_syscall()

[Cédric Le Goater]

On 8/23/21 10:28 AM, Christophe Leroy wrote:
> 
> 
> Le 19/08/2021 à 16:56, Christophe Leroy a écrit :
>>
>>
>> Le 19/08/2021 à 14:56, Cédric Le Goater a écrit :
>>> This fixes a compile error with W=1.
>>>
>>> Cc: Christophe Leroy <christophe.leroy@c-s.fr>
>>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>>> ---
>>>
>>>   I don't think this is correct. Which file could we use ?
>>
>> I think you can completely remove ppc32_classify_syscall(), and instead add the following in the default case in audit_classify_syscall():
>>
>>       default:
>> +        if (IS_ENABLED(CONFIG_PPC64) && abi == AUDIT_ARCH_PPC)
>> +            return 1;
>>           return 0;
>>
> 
> Forget that comment, it was crazy, because PPC32 and PPC64 use different syscall numbers.
> 
> By the way, I have submitted a patch to completely remove this stuff, see https://patchwork.ozlabs.org/project/linuxppc-dev/patch/dc14509a28a993738b1325211f412be72a4f9b1e.1629701132.git.christophe.leroy@csgroup.eu/

Nice ! Let's merge that instead.

Thanks,

C. 


> 
>>
>>>
>>>   arch/powerpc/include/asm/unistd.h | 3 +++
>>>   arch/powerpc/kernel/audit.c       | 1 -
>>>   2 files changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/powerpc/include/asm/unistd.h b/arch/powerpc/include/asm/unistd.h
>>> index b541c690a31c..d9025a7e973c 100644
>>> --- a/arch/powerpc/include/asm/unistd.h
>>> +++ b/arch/powerpc/include/asm/unistd.h
>>> @@ -47,6 +47,9 @@
>>>   #define __ARCH_WANT_SYS_UTIME
>>>   #define __ARCH_WANT_SYS_NEWFSTATAT
>>>   #define __ARCH_WANT_COMPAT_SYS_SENDFILE
>>> +#ifdef CONFIG_AUDIT
>>> +extern int ppc32_classify_syscall(unsigned int syscall);
>>> +#endif
>>>   #endif
>>>   #define __ARCH_WANT_SYS_FORK
>>>   #define __ARCH_WANT_SYS_VFORK
>>> diff --git a/arch/powerpc/kernel/audit.c b/arch/powerpc/kernel/audit.c
>>> index a2dddd7f3d09..c3c6c6a1069b 100644
>>> --- a/arch/powerpc/kernel/audit.c
>>> +++ b/arch/powerpc/kernel/audit.c
>>> @@ -41,7 +41,6 @@ int audit_classify_arch(int arch)
>>>   int audit_classify_syscall(int abi, unsigned syscall)
>>>   {
>>>   #ifdef CONFIG_PPC64
>>> -    extern int ppc32_classify_syscall(unsigned);
>>>       if (abi == AUDIT_ARCH_PPC)
>>>           return ppc32_classify_syscall(syscall);
>>>   #endif
>>>

[PATCH 6/6] powerpc/compat_sys: Declare syscalls

[Cédric Le Goater]

This fixes a compile error with W=1.

Cc: Christophe Leroy <christophe.leroy@c-s.fr>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 arch/powerpc/include/asm/syscalls.h | 31 +++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/arch/powerpc/include/asm/syscalls.h b/arch/powerpc/include/asm/syscalls.h
index 398171fdcd9f..1d5f2abaa38a 100644
--- a/arch/powerpc/include/asm/syscalls.h
+++ b/arch/powerpc/include/asm/syscalls.h
@@ -6,6 +6,7 @@
 #include <linux/compiler.h>
 #include <linux/linkage.h>
 #include <linux/types.h>
+#include <linux/compat.h>
 
 struct rtas_args;
 
@@ -18,5 +19,35 @@ asmlinkage long sys_mmap2(unsigned long addr, size_t len,
 asmlinkage long ppc64_personality(unsigned long personality);
 asmlinkage long sys_rtas(struct rtas_args __user *uargs);
 
+#ifdef CONFIG_COMPAT
+unsigned long compat_sys_mmap2(unsigned long addr, size_t len,
+			       unsigned long prot, unsigned long flags,
+			       unsigned long fd, unsigned long pgoff);
+
+compat_ssize_t compat_sys_pread64(unsigned int fd, char __user *ubuf, compat_size_t count,
+				  u32 reg6, u32 pos1, u32 pos2);
+
+compat_ssize_t compat_sys_pwrite64(unsigned int fd, const char __user *ubuf, compat_size_t count,
+				   u32 reg6, u32 pos1, u32 pos2);
+
+compat_ssize_t compat_sys_readahead(int fd, u32 r4, u32 offset1, u32 offset2, u32 count);
+
+asmlinkage int compat_sys_truncate64(const char __user *path, u32 reg4,
+				     unsigned long len1, unsigned long len2);
+
+asmlinkage long compat_sys_fallocate(int fd, int mode, u32 offset1, u32 offset2,
+				     u32 len1, u32 len2);
+
+asmlinkage int compat_sys_ftruncate64(unsigned int fd, u32 reg4, unsigned long len1,
+				      unsigned long len2);
+
+long ppc32_fadvise64(int fd, u32 unused, u32 offset1, u32 offset2,
+		     size_t len, int advice);
+
+asmlinkage long compat_sys_sync_file_range2(int fd, unsigned int flags,
+					    unsigned int offset1, unsigned int offset2,
+					    unsigned int nbytes1, unsigned int nbytes2);
+#endif
+
 #endif /* __KERNEL__ */
 #endif /* __ASM_POWERPC_SYSCALLS_H */
-- 
2.31.1

Re: [PATCH 6/6] powerpc/compat_sys: Declare syscalls

[Michael Ellerman]

Cédric Le Goater <clg@kaod.org> writes:
> This fixes a compile error with W=1.
>
> Cc: Christophe Leroy <christophe.leroy@c-s.fr>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>  arch/powerpc/include/asm/syscalls.h | 31 +++++++++++++++++++++++++++++
>  1 file changed, 31 insertions(+)
>
> diff --git a/arch/powerpc/include/asm/syscalls.h b/arch/powerpc/include/asm/syscalls.h
> index 398171fdcd9f..1d5f2abaa38a 100644
> --- a/arch/powerpc/include/asm/syscalls.h
> +++ b/arch/powerpc/include/asm/syscalls.h
> @@ -6,6 +6,7 @@
>  #include <linux/compiler.h>
>  #include <linux/linkage.h>
>  #include <linux/types.h>
> +#include <linux/compat.h>
>  
>  struct rtas_args;
>  
> @@ -18,5 +19,35 @@ asmlinkage long sys_mmap2(unsigned long addr, size_t len,
>  asmlinkage long ppc64_personality(unsigned long personality);
>  asmlinkage long sys_rtas(struct rtas_args __user *uargs);
>  
> +#ifdef CONFIG_COMPAT
> +unsigned long compat_sys_mmap2(unsigned long addr, size_t len,
> +			       unsigned long prot, unsigned long flags,
> +			       unsigned long fd, unsigned long pgoff);
> +
> +compat_ssize_t compat_sys_pread64(unsigned int fd, char __user *ubuf, compat_size_t count,
> +				  u32 reg6, u32 pos1, u32 pos2);
> +
> +compat_ssize_t compat_sys_pwrite64(unsigned int fd, const char __user *ubuf, compat_size_t count,
> +				   u32 reg6, u32 pos1, u32 pos2);
> +
> +compat_ssize_t compat_sys_readahead(int fd, u32 r4, u32 offset1, u32 offset2, u32 count);
> +
> +asmlinkage int compat_sys_truncate64(const char __user *path, u32 reg4,
> +				     unsigned long len1, unsigned long len2);

asmlinkage does nothing, ie. it's an empty macro. Let's not add new uses
of it.

cheers

Re: [PATCH 6/6] powerpc/compat_sys: Declare syscalls

[Christophe Leroy]

Le 19/08/2021 à 14:56, Cédric Le Goater a écrit :
> This fixes a compile error with W=1.
> 
> Cc: Christophe Leroy <christophe.leroy@c-s.fr>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>


Instead of doing that, we should use COMPAT_SYSCALL_DEFINEx macros in 
the function definitions.

Thanks
Christophe

> ---
>   arch/powerpc/include/asm/syscalls.h | 31 +++++++++++++++++++++++++++++
>   1 file changed, 31 insertions(+)
> 
> diff --git a/arch/powerpc/include/asm/syscalls.h b/arch/powerpc/include/asm/syscalls.h
> index 398171fdcd9f..1d5f2abaa38a 100644
> --- a/arch/powerpc/include/asm/syscalls.h
> +++ b/arch/powerpc/include/asm/syscalls.h
> @@ -6,6 +6,7 @@
>   #include <linux/compiler.h>
>   #include <linux/linkage.h>
>   #include <linux/types.h>
> +#include <linux/compat.h>
>   
>   struct rtas_args;
>   
> @@ -18,5 +19,35 @@ asmlinkage long sys_mmap2(unsigned long addr, size_t len,
>   asmlinkage long ppc64_personality(unsigned long personality);
>   asmlinkage long sys_rtas(struct rtas_args __user *uargs);
>   
> +#ifdef CONFIG_COMPAT
> +unsigned long compat_sys_mmap2(unsigned long addr, size_t len,
> +			       unsigned long prot, unsigned long flags,
> +			       unsigned long fd, unsigned long pgoff);
> +
> +compat_ssize_t compat_sys_pread64(unsigned int fd, char __user *ubuf, compat_size_t count,
> +				  u32 reg6, u32 pos1, u32 pos2);
> +
> +compat_ssize_t compat_sys_pwrite64(unsigned int fd, const char __user *ubuf, compat_size_t count,
> +				   u32 reg6, u32 pos1, u32 pos2);
> +
> +compat_ssize_t compat_sys_readahead(int fd, u32 r4, u32 offset1, u32 offset2, u32 count);
> +
> +asmlinkage int compat_sys_truncate64(const char __user *path, u32 reg4,
> +				     unsigned long len1, unsigned long len2);
> +
> +asmlinkage long compat_sys_fallocate(int fd, int mode, u32 offset1, u32 offset2,
> +				     u32 len1, u32 len2);
> +
> +asmlinkage int compat_sys_ftruncate64(unsigned int fd, u32 reg4, unsigned long len1,
> +				      unsigned long len2);
> +
> +long ppc32_fadvise64(int fd, u32 unused, u32 offset1, u32 offset2,
> +		     size_t len, int advice);
> +
> +asmlinkage long compat_sys_sync_file_range2(int fd, unsigned int flags,
> +					    unsigned int offset1, unsigned int offset2,
> +					    unsigned int nbytes1, unsigned int nbytes2);
> +#endif
> +
>   #endif /* __KERNEL__ */
>   #endif /* __ASM_POWERPC_SYSCALLS_H */

Re: [PATCH 0/6] W=1 fixes

[Michael Ellerman]

On Thu, 19 Aug 2021 14:56:50 +0200, Cédric Le Goater wrote:
> With this small series, I could compile the ppc kernel with W=1. There
> are certainly other configs which will need more fixes but it's a good
> start.
> 
> The last 2 patches look hacky. Christophe, could you help with these
> to find a better place to include the declarations ?
> 
> [...]

Patches 2-4 applied to powerpc/next.

[2/6] powerpc/pseries/vas: Declare pseries_vas_fault_thread_fn() as static
      https://git.kernel.org/powerpc/c/4cb266074aa17e9cafed3a92e9f43b161516569f
[3/6] KVM: PPC: Book3S PR: Declare kvmppc_handle_exit_pr()
      https://git.kernel.org/powerpc/c/cb53a93e33e108bfec00a13cd12696e1c0ccc8b6
[4/6] KVM: PPC: Book3S PR: Remove unused variable
      https://git.kernel.org/powerpc/c/b352ddae7b2ccd2cb29f495ca0a7c9b6848b623f

cheers