LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* RE: [PATCH] powerpc/vdso32: Add missing _restgpr_31_x to fix build failure
From: David Laight @ 2021-03-15 16:38 UTC (permalink / raw)
  To: 'Rasmus Villemoes', Segher Boessenkool, Christophe Leroy
  Cc: Paul Mackerras, linuxppc-dev@lists.ozlabs.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <023afd0c-dc61-5891-5145-5bcdce8227be@prevas.dk>

From: Rasmus Villemoes
> Sent: 15 March 2021 16:24
> 
> On 12/03/2021 03.29, Segher Boessenkool wrote:
> > Hi!
> >
> > On Tue, Mar 09, 2021 at 06:19:30AM +0000, Christophe Leroy wrote:
> >> With some defconfig including CONFIG_CC_OPTIMIZE_FOR_SIZE,
> >> (for instance mvme5100_defconfig and ps3_defconfig), gcc 5
> >> generates a call to _restgpr_31_x.
> >
> >> I don't know if there is a way to tell GCC not to emit that call, because at the end we get more
> instructions than needed.
> >
> > The function is required by the ABI, you need to have it.
> >
> > You get *fewer* insns statically, and that is what -Os is about: reduce
> > the size of the binaries.
> 
> Is there any reason to not just always build the vdso with -O2? It's one
> page/one VMA either way, and the vdso is about making certain system
> calls cheaper, so if unconditional -O2 could save a few cycles compared
> to -Os, why not? (And if, as it seems, there's only one user within the
> DSO of _restgpr_31_x, yes, the overall size of the .text segment
> probably increases slightly).

Sometimes -Os generates such horrid code you really never want to use it.
A classic is on x86 where it replaces 'load register with byte constant'
with 'push byte' 'pop register'.
The code is actually smaller but the execution time is horrid.

There are also cases where -O2 actually generates smaller code.

Although you may need to disable loop unrolling (often dubious at best)
and either force or disable some function inlining.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)


^ permalink raw reply

* Re: [PATCH] powerpc/vdso32: Add missing _restgpr_31_x to fix build failure
From: Rasmus Villemoes @ 2021-03-15 16:23 UTC (permalink / raw)
  To: Segher Boessenkool, Christophe Leroy
  Cc: linuxppc-dev, Paul Mackerras, linux-kernel
In-Reply-To: <20210312022940.GO29191@gate.crashing.org>

On 12/03/2021 03.29, Segher Boessenkool wrote:
> Hi!
> 
> On Tue, Mar 09, 2021 at 06:19:30AM +0000, Christophe Leroy wrote:
>> With some defconfig including CONFIG_CC_OPTIMIZE_FOR_SIZE,
>> (for instance mvme5100_defconfig and ps3_defconfig), gcc 5
>> generates a call to _restgpr_31_x.
> 
>> I don't know if there is a way to tell GCC not to emit that call, because at the end we get more instructions than needed.
> 
> The function is required by the ABI, you need to have it.
> 
> You get *fewer* insns statically, and that is what -Os is about: reduce
> the size of the binaries.

Is there any reason to not just always build the vdso with -O2? It's one
page/one VMA either way, and the vdso is about making certain system
calls cheaper, so if unconditional -O2 could save a few cycles compared
to -Os, why not? (And if, as it seems, there's only one user within the
DSO of _restgpr_31_x, yes, the overall size of the .text segment
probably increases slightly).

Rasmus

^ permalink raw reply

* Re: [PATCH] powerpc/numa: Fix topology_physical_package_id() on pSeries
From: Cédric Le Goater @ 2021-03-15 16:16 UTC (permalink / raw)
  To: Daniel Henrique Barboza, linuxppc-dev
  Cc: Nathan Lynch, Srikar Dronamraju, Greg Kurz, Vasant Hegde,
	David Gibson
In-Reply-To: <bd0c948c-64f8-cf64-6d30-b9167b6a7629@gmail.com>

On 3/15/21 4:12 PM, Daniel Henrique Barboza wrote:
> 
> 
> On 3/12/21 11:31 AM, Cédric Le Goater wrote:
>> Initial commit 15863ff3b8da ("powerpc: Make chip-id information
>> available to userspace") introduce a cpu_to_chip_id() routine for the
>> PowerNV platform using the "ibm,chip-id" property to query the chip id
>> of a CPU. But PAPR does not specify such a property and the node id
>> query is broken.
>>
>> Use cpu_to_node() instead which guarantees to have a correct value on
>> all platforms, PowerNV an pSeries.
> 
> It is worth mentioning that that this patch will change how
> topology_physical_package_id() represents in a QEMU guest. Right now, ibm,chip-id
> in QEMU is matching the socket-id. After this patch, topology_physical_package_id()
> will now match the NUMA id of the CPU.

yes. I should have added some more background. 

LPARs are impacted by the use of ibm,chip-id because the property 
does not exist under PowerVM and the topology-id in sysfs is always
-1 even if NUMA nodes are defined.

Under QEMU/KVM, ibm,chip-id is badly calculated when using uncommon 
SMT configuration. This leads to a bogus topology-id value being 
exported in sysfs.

The use of cpu_to_node() guarantees to have a correct NUMA node id 
under both environments QEMU/KVM and PowerVM.

On the PowerNV platform, the numa node id returned by cpu_to_node() 
is computed from the "ibm,associativity" property of the CPU. Its 
value is built from the OPAL chip id and is equivalent to ibm,chip-id. 

May be I should rephrase the commit log in a v2 ?

C.

 
> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> Tested-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> 
>>
>> Cc: Nathan Lynch <nathanl@linux.ibm.com>
>> Cc: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
>> Cc: Vasant Hegde <hegdevasant@linux.vnet.ibm.com>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>>   arch/powerpc/include/asm/topology.h | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/powerpc/include/asm/topology.h b/arch/powerpc/include/asm/topology.h
>> index 3beeb030cd78..887c42a4e43d 100644
>> --- a/arch/powerpc/include/asm/topology.h
>> +++ b/arch/powerpc/include/asm/topology.h
>> @@ -123,7 +123,7 @@ static inline int cpu_to_coregroup_id(int cpu)
>>   #ifdef CONFIG_PPC64
>>   #include <asm/smp.h>
>>   -#define topology_physical_package_id(cpu)    (cpu_to_chip_id(cpu))
>> +#define topology_physical_package_id(cpu)    (cpu_to_node(cpu))
>>     #define topology_sibling_cpumask(cpu)    (per_cpu(cpu_sibling_map, cpu))
>>   #define topology_core_cpumask(cpu)    (cpu_cpu_mask(cpu))
>>


^ permalink raw reply

* Re: [PATCH] powerpc/numa: Fix topology_physical_package_id() on pSeries
From: Daniel Henrique Barboza @ 2021-03-15 15:12 UTC (permalink / raw)
  To: Cédric Le Goater, linuxppc-dev
  Cc: Nathan Lynch, Srikar Dronamraju, Greg Kurz, Vasant Hegde,
	David Gibson
In-Reply-To: <20210312143154.3181109-1-clg@kaod.org>



On 3/12/21 11:31 AM, Cédric Le Goater wrote:
> Initial commit 15863ff3b8da ("powerpc: Make chip-id information
> available to userspace") introduce a cpu_to_chip_id() routine for the
> PowerNV platform using the "ibm,chip-id" property to query the chip id
> of a CPU. But PAPR does not specify such a property and the node id
> query is broken.
> 
> Use cpu_to_node() instead which guarantees to have a correct value on
> all platforms, PowerNV an pSeries.

It is worth mentioning that that this patch will change how
topology_physical_package_id() represents in a QEMU guest. Right now, ibm,chip-id
in QEMU is matching the socket-id. After this patch, topology_physical_package_id()
will now match the NUMA id of the CPU.



Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Tested-by: Daniel Henrique Barboza <danielhb413@gmail.com>

> 
> Cc: Nathan Lynch <nathanl@linux.ibm.com>
> Cc: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
> Cc: Vasant Hegde <hegdevasant@linux.vnet.ibm.com>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>   arch/powerpc/include/asm/topology.h | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/include/asm/topology.h b/arch/powerpc/include/asm/topology.h
> index 3beeb030cd78..887c42a4e43d 100644
> --- a/arch/powerpc/include/asm/topology.h
> +++ b/arch/powerpc/include/asm/topology.h
> @@ -123,7 +123,7 @@ static inline int cpu_to_coregroup_id(int cpu)
>   #ifdef CONFIG_PPC64
>   #include <asm/smp.h>
>   
> -#define topology_physical_package_id(cpu)	(cpu_to_chip_id(cpu))
> +#define topology_physical_package_id(cpu)	(cpu_to_node(cpu))
>   
>   #define topology_sibling_cpumask(cpu)	(per_cpu(cpu_sibling_map, cpu))
>   #define topology_core_cpumask(cpu)	(cpu_cpu_mask(cpu))
> 

^ permalink raw reply

* [PATCH] powerpc/mm: Remove unneeded #ifdef CONFIG_PPC_MEM_KEYS
From: Christophe Leroy @ 2021-03-15 14:52 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: linuxppc-dev, linux-kernel

In fault.c, #ifdef CONFIG_PPC_MEM_KEYS is not needed because all
functions are always defined, and arch_vma_access_permitted()
always returns true when CONFIG_PPC_MEM_KEYS is not defined so
access_pkey_error() will return false so bad_access_pkey()
will never be called.

Include linux/pkeys.h to get a definition of vma_pkeys() for
bad_access_pkey().

Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
 arch/powerpc/mm/fault.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c
index bb368257b55c..f1b3f5922f90 100644
--- a/arch/powerpc/mm/fault.c
+++ b/arch/powerpc/mm/fault.c
@@ -32,6 +32,7 @@
 #include <linux/context_tracking.h>
 #include <linux/hugetlb.h>
 #include <linux/uaccess.h>
+#include <linux/pkeys.h>
 
 #include <asm/firmware.h>
 #include <asm/interrupt.h>
@@ -87,7 +88,6 @@ static noinline int bad_area(struct pt_regs *regs, unsigned long address)
 	return __bad_area(regs, address, SEGV_MAPERR);
 }
 
-#ifdef CONFIG_PPC_MEM_KEYS
 static noinline int bad_access_pkey(struct pt_regs *regs, unsigned long address,
 				    struct vm_area_struct *vma)
 {
@@ -127,7 +127,6 @@ static noinline int bad_access_pkey(struct pt_regs *regs, unsigned long address,
 
 	return 0;
 }
-#endif
 
 static noinline int bad_access(struct pt_regs *regs, unsigned long address)
 {
@@ -234,7 +233,6 @@ static bool bad_kernel_fault(struct pt_regs *regs, unsigned long error_code,
 	return false;
 }
 
-#ifdef CONFIG_PPC_MEM_KEYS
 static bool access_pkey_error(bool is_write, bool is_exec, bool is_pkey,
 			      struct vm_area_struct *vma)
 {
@@ -248,7 +246,6 @@ static bool access_pkey_error(bool is_write, bool is_exec, bool is_pkey,
 
 	return false;
 }
-#endif
 
 static bool access_error(bool is_write, bool is_exec, struct vm_area_struct *vma)
 {
@@ -492,11 +489,9 @@ static int ___do_page_fault(struct pt_regs *regs, unsigned long address,
 			return bad_area(regs, address);
 	}
 
-#ifdef CONFIG_PPC_MEM_KEYS
 	if (unlikely(access_pkey_error(is_write, is_exec,
 				       (error_code & DSISR_KEYFAULT), vma)))
 		return bad_access_pkey(regs, address, vma);
-#endif /* CONFIG_PPC_MEM_KEYS */
 
 	if (unlikely(access_error(is_write, is_exec, vma)))
 		return bad_access(regs, address);
-- 
2.25.0


^ permalink raw reply related

* Re: [PATCH 07/10] powerpc/64e/interrupt: handle bad_page_fault in C
From: Christophe Leroy @ 2021-03-15 14:07 UTC (permalink / raw)
  To: Nicholas Piggin, linuxppc-dev; +Cc: Scott Wood
In-Reply-To: <20210315031716.3940350-8-npiggin@gmail.com>



Le 15/03/2021 à 04:17, Nicholas Piggin a écrit :
> With non-volatile registers saved on interrupt, bad_page_fault
> can now be called by do_page_fault.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>   arch/powerpc/kernel/exceptions-64e.S | 6 ------
>   arch/powerpc/mm/fault.c              | 5 +----
>   2 files changed, 1 insertion(+), 10 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/exceptions-64e.S b/arch/powerpc/kernel/exceptions-64e.S
> index a059ab3542c2..b08c84e0fa56 100644
> --- a/arch/powerpc/kernel/exceptions-64e.S
> +++ b/arch/powerpc/kernel/exceptions-64e.S
> @@ -937,12 +937,6 @@ storage_fault_common:
>   	ld	r14,PACA_EXGEN+EX_R14(r13)
>   	ld	r15,PACA_EXGEN+EX_R15(r13)
>   	bl	do_page_fault
> -	cmpdi	r3,0
> -	bne-	1f
> -	b	interrupt_return
> -	mr	r4,r3
> -	addi	r3,r1,STACK_FRAME_OVERHEAD
> -	bl	__bad_page_fault

Then __bad_page_fault() can be static now.

>   	b	interrupt_return
>   
>   /*
> diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c
> index 2e54bac99a22..44833660b21d 100644
> --- a/arch/powerpc/mm/fault.c
> +++ b/arch/powerpc/mm/fault.c
> @@ -552,12 +552,9 @@ static long __do_page_fault(struct pt_regs *regs)
>   	if (likely(entry)) {
>   		instruction_pointer_set(regs, extable_fixup(entry));
>   		return 0;
> -	} else if (!IS_ENABLED(CONFIG_PPC_BOOK3E_64)) {
> +	} else {
>   		__bad_page_fault(regs, err);
>   		return 0;
> -	} else {
> -		/* 32 and 64e handle the bad page fault in asm */
> -		return err;
>   	}
>   }
>   NOKPROBE_SYMBOL(__do_page_fault);
> 

^ permalink raw reply

* Re: [PATCH] powerpc: Fix missing prototype problems for "arch/powerpc/kernel/setup_64.c"
From: Cédric Le Goater @ 2021-03-15 13:14 UTC (permalink / raw)
  To: Christophe Leroy, heying (H), mpe, benh, paulus, npiggin, dja,
	akpm, rppt, aneesh.kumar
  Cc: johnny.chenyi, linuxppc-dev
In-Reply-To: <fe0638cc-28be-2e5c-9e94-ab3ff7666164@csgroup.eu>

On 3/15/21 2:01 PM, Christophe Leroy wrote:
> 
> 
> Le 15/03/2021 à 13:57, Cédric Le Goater a écrit :
>> On 3/15/21 1:48 PM, heying (H) wrote:
>>>
>>> 在 2021/3/15 20:17, Christophe Leroy 写道:
>>>> You subject doesn't match the content of the patch.
>>> OK. I'll adapt that.
>>>>
>>>> Le 15/03/2021 à 13:04, He Ying a écrit :
>>>>> The variables 'uaccess_fulsh' and 'entry_flush' are not referenced
>>>>> outside the file. So define them as static to avoid the warnings.
>>>>>
>>>>> And add a prototype for the function 'panic_smp_self_stop' for the
>>>>> same purpose.
>>>>>
>>>>> Sparse also warns that 'rfi_flush' should be static. However, it's
>>>>> referenced outside the file.
>>>>
>>>> To clear that warning, you have to include asm/security_features.h, rfi_flush is declared there.
>>> Do you mean that I should include this header in arch/powerpc/kernel/setup_64.c?
>>
>> yes.
>>
>>>>
>>>>>
>>>>> The warnings about the file reported by sparse are as follows:
>>>>> arch/powerpc/kernel/setup_64.c:422:6: warning: symbol 'panic_smp_self_stop' was not declared. Should it be static?
>>>>> arch/powerpc/kernel/setup_64.c:951:6: warning: symbol 'rfi_flush' was not declared. Should it be static?
>>>>> arch/powerpc/kernel/setup_64.c:952:6: warning: symbol 'entry_flush' was not declared. Should it be static?
>>>>> arch/powerpc/kernel/setup_64.c:953:6: warning: symbol 'uaccess_flush' was not declared. Should it be static?
>>>>>
>>>>> Reported-by: Hulk Robot <hulkci@huawei.com>
>>>>> Signed-off-by: He Ying <heying24@huawei.com>
>>>>> ---
>>>>>    arch/powerpc/kernel/setup_64.c | 6 ++++--
>>>>>    1 file changed, 4 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c
>>>>> index 560ed8b975e7..603aacd8527b 100644
>>>>> --- a/arch/powerpc/kernel/setup_64.c
>>>>> +++ b/arch/powerpc/kernel/setup_64.c
>>>>> @@ -71,6 +71,8 @@
>>>>>      #include "setup.h"
>>>>>    +extern void panic_smp_self_stop(void);
>>>>> +
>>>>
>>>> For function prototypes 'extern' is unneeded and deprecated.
>>>>
>>>> And function prototypes should go in an header file.
>>>>
>>>> panic_smp_self_stop() is called from kernel/panic.c , it should be declared in one of the generic linux header files I think.
>>> Yes, you're right. But I have no idea which header it should be declared in. May I have your suggestions?
>>
>> arch/powerpc/include/asm/bug.h looks like a good place.
> 
> Why declaring it in a powerpc header ?
> 
> It's a weak function defined in core part of kernel (kernel/panic.c).
> 
> I think it should go in a common header, just like for instance arch_thaw_secondary_cpus_begin()

Indeed. include/linux/smp.h is a better place for a common routine.

C.


^ permalink raw reply

* Re: [PATCH v3 28/32] powerpc/64s: interrupt implement exit logic in C
From: Christophe Leroy @ 2021-03-15 13:41 UTC (permalink / raw)
  To: Nicholas Piggin, linuxppc-dev; +Cc: Michal Suchanek
In-Reply-To: <20200225173541.1549955-29-npiggin@gmail.com>



Le 25/02/2020 à 18:35, Nicholas Piggin a écrit :
> Implement the bulk of interrupt return logic in C. The asm return code
> must handle a few cases: restoring full GPRs, and emulating stack store.
> 
> The stack store emulation is significantly simplfied, rather than creating
> a new return frame and switching to that before performing the store, it
> uses the PACA to keep a scratch register around to perform thestore.
> 
> The asm return code is moved into 64e for now. The new logic has made
> allowance for 64e, but I don't have a full environment that works well
> to test it, and even booting in emulated qemu is not great for stress
> testing. 64e shouldn't be too far off working with this, given a bit
> more testing and auditing of the logic.
> 
> This is slightly faster on a POWER9 (page fault speed increases about
> 1.1%), probably due to reduced mtmsrd.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> ---

...

> +notrace unsigned long interrupt_exit_user_prepare(struct pt_regs *regs, unsigned long msr)
> +{

...

> +
> +#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
> +	local_paca->tm_scratch = regs->msr;
> +#endif

Could we define a helper for that in asm/tm.h, that voids when CONFIG_PPC_TRANSACTIONAL_MEM is not 
selected ?

> +
> +	kuap_check_amr();
> +
> +	account_cpu_user_exit();
> +
> +	return ret;
> +}
> +
> +void unrecoverable_exception(struct pt_regs *regs);
> +void preempt_schedule_irq(void);
> +
> +notrace unsigned long interrupt_exit_kernel_prepare(struct pt_regs *regs, unsigned long msr)
> +{
> +

...

> +#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
> +	local_paca->tm_scratch = regs->msr;
> +#endif
> +
> +	/*
> +	 * We don't need to restore AMR on the way back to userspace for KUAP.
> +	 * The value of AMR only matters while we're in the kernel.
> +	 */
> +	kuap_restore_amr(regs);
> +
> +	if (unlikely(*ti_flagsp & _TIF_EMULATE_STACK_STORE)) {
> +		clear_bits(_TIF_EMULATE_STACK_STORE, ti_flagsp);
> +		return 1;
> +	}
> +	return 0;
> +}
> +#endif

Christophe

^ permalink raw reply

* Re: [PATCH] powerpc/vdso32: Add missing _restgpr_31_x to fix build failure
From: Michael Ellerman @ 2021-03-15 13:31 UTC (permalink / raw)
  To: Paul Mackerras, Benjamin Herrenschmidt, Michael Ellerman, segher,
	Christophe Leroy
  Cc: linuxppc-dev, linux-kernel
In-Reply-To: <a7aa198a88bcd33c6e35e99f70f86c7b7f2f9440.1615270757.git.christophe.leroy@csgroup.eu>

On Tue, 9 Mar 2021 06:19:30 +0000 (UTC), Christophe Leroy wrote:
> With some defconfig including CONFIG_CC_OPTIMIZE_FOR_SIZE,
> (for instance mvme5100_defconfig and ps3_defconfig), gcc 5
> generates a call to _restgpr_31_x.
> 
> Until recently it went unnoticed, but
> commit 42ed6d56ade2 ("powerpc/vdso: Block R_PPC_REL24 relocations")
> made it rise to the surface.
> 
> [...]

Applied to powerpc/fixes.

[1/1] powerpc/vdso32: Add missing _restgpr_31_x to fix build failure
      https://git.kernel.org/powerpc/c/08c18b63d9656e0389087d1956d2b37fd7019172

cheers

^ permalink raw reply

* Re: [PATCH] powerpc: Force inlining of cpu_has_feature() to avoid build failure
From: Michael Ellerman @ 2021-03-15 13:31 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Michael Ellerman, Paul Mackerras,
	Christophe Leroy
  Cc: linuxppc-dev, linux-kernel
In-Reply-To: <b231dfa040ce4cc37f702f5c3a595fdeabfe0462.1615378209.git.christophe.leroy@csgroup.eu>

On Wed, 10 Mar 2021 12:10:34 +0000 (UTC), Christophe Leroy wrote:
> The code relies on constant folding of cpu_has_feature() based
> on possible and always true values as defined per
> CPU_FTRS_ALWAYS and CPU_FTRS_POSSIBLE.
> 
> Build failure is encountered with for instance
> book3e_all_defconfig on kisskb in the AMDGPU driver which uses
> cpu_has_feature(CPU_FTR_VSX_COMP) to decide whether calling
> kernel_enable_vsx() or not.
> 
> [...]

Applied to powerpc/fixes.

[1/1] powerpc: Force inlining of cpu_has_feature() to avoid build failure
      https://git.kernel.org/powerpc/c/eed5fae00593ab9d261a0c1ffc1bdb786a87a55a

cheers

^ permalink raw reply

* Re: [PATCH 03/10] powerpc/64e/interrupt: use new interrupt return
From: Christophe Leroy @ 2021-03-15 13:30 UTC (permalink / raw)
  To: Nicholas Piggin, linuxppc-dev; +Cc: Scott Wood
In-Reply-To: <20210315031716.3940350-4-npiggin@gmail.com>



Le 15/03/2021 à 04:17, Nicholas Piggin a écrit :
> Update the new C and asm interrupt return code to account for 64e
> specifics, switch over to use it.
> 
> The now-unused old ret_from_except code, that was moved to 64e after the
> 64s conversion, is removed.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>   arch/powerpc/include/asm/asm-prototypes.h |   2 -
>   arch/powerpc/kernel/entry_64.S            |   9 +-
>   arch/powerpc/kernel/exceptions-64e.S      | 321 ++--------------------
>   arch/powerpc/kernel/interrupt.c           |  27 +-
>   arch/powerpc/kernel/irq.c                 |  76 -----
>   5 files changed, 56 insertions(+), 379 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/interrupt.c b/arch/powerpc/kernel/interrupt.c
> index fbabb49888d3..ae7b058b2970 100644
> --- a/arch/powerpc/kernel/interrupt.c
> +++ b/arch/powerpc/kernel/interrupt.c
> @@ -235,6 +235,10 @@ static notrace void booke_load_dbcr0(void)
>   #endif
>   }
>   
> +/* temporary hack for context tracking, removed in later patch */
> +#include <linux/sched/debug.h>
> +asmlinkage __visible void __sched schedule_user(void);
> +
>   /*
>    * This should be called after a syscall returns, with r3 the return value
>    * from the syscall. If this function returns non-zero, the system call
> @@ -292,7 +296,11 @@ notrace unsigned long syscall_exit_prepare(unsigned long r3,
>   	while (unlikely(ti_flags & (_TIF_USER_WORK_MASK & ~_TIF_RESTORE_TM))) {
>   		local_irq_enable();
>   		if (ti_flags & _TIF_NEED_RESCHED) {
> +#ifdef CONFIG_PPC_BOOK3E_64
> +			schedule_user();
> +#else
>   			schedule();
> +#endif
>   		} else {
>   			/*
>   			 * SIGPENDING must restore signal handler function
> @@ -360,7 +368,6 @@ notrace unsigned long syscall_exit_prepare(unsigned long r3,
>   	return ret;
>   }
>   
> -#ifndef CONFIG_PPC_BOOK3E_64 /* BOOK3E not yet using this */
>   notrace unsigned long interrupt_exit_user_prepare(struct pt_regs *regs, unsigned long msr)
>   {
>   	unsigned long ti_flags;
> @@ -372,7 +379,9 @@ notrace unsigned long interrupt_exit_user_prepare(struct pt_regs *regs, unsigned
>   	BUG_ON(!(regs->msr & MSR_PR));
>   	BUG_ON(!FULL_REGS(regs));
>   	BUG_ON(arch_irq_disabled_regs(regs));
> +#ifdef CONFIG_PPC_BOOK3S_64

Shouldn't this go away in patch 6 as well ?
Or is that needed at all ? In syscall_exit_prepare() it is not ifdefed .

>   	CT_WARN_ON(ct_state() == CONTEXT_USER);
> +#endif
>   
>   	/*
>   	 * We don't need to restore AMR on the way back to userspace for KUAP.

^ permalink raw reply

* Re: Build regressions/improvements in v5.12-rc3
From: Christophe Leroy @ 2021-03-15 13:07 UTC (permalink / raw)
  To: Geert Uytterhoeven; +Cc: linuxppc-dev, Linux Kernel Mailing List
In-Reply-To: <CAMuHMdUMjN9TW-ggAgOtj3V36kzNCfoG5o-Bcj=Lk9diJciS=g@mail.gmail.com>



Le 15/03/2021 à 12:02, Geert Uytterhoeven a écrit :
> Hi Christophe,
> 
> On Mon, Mar 15, 2021 at 11:55 AM Christophe Leroy
> <christophe.leroy@csgroup.eu> wrote:
>> Le 15/03/2021 à 11:49, Geert Uytterhoeven a écrit :
>>> On Mon, Mar 15, 2021 at 11:46 AM Geert Uytterhoeven
>>> <geert@linux-m68k.org> wrote:
>>>> JFYI, when comparing v5.12-rc3[1] to v5.12-rc2[3], the summaries are:
>>>>     - build errors: +2/-2
>>>
>>>> 2 error regressions:
>>>>     + /kisskb/src/include/linux/compiler_types.h: error: call to '__compiletime_assert_248' declared with attribute error: BUILD_BUG failed:  => 320:38
>>>>     + /kisskb/src/include/linux/compiler_types.h: error: call to '__compiletime_assert_249' declared with attribute error: BUILD_BUG failed:  => 320:38
>>>
>>> powerpc-gcc4.9/ppc64_book3e_allmodconfig
>>>
>>> So we traded implicit declaration errors:
>>>
>>>     - /kisskb/src/drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.c:
>>> error: implicit declaration of function 'disable_kernel_vsx'
>>> [-Werror=implicit-function-declaration]: 674:2 =>
>>>     - /kisskb/src/drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.c:
>>> error: implicit declaration of function 'enable_kernel_vsx'
>>> [-Werror=implicit-function-declaration]: 638:2 =>
>>>
>>> for compile-time assertions.
>>>
>>
>> You are missing https://github.com/linuxppc/linux/commit/eed5fae00593ab9d261a0c1ffc1bdb786a87a55a
> 
> Which is not part of v5.12-rc3.
> 

Yes, one step at a time :)

When I did the first fix, I tested it with GCC 10 and it worked.

It is only afterwards that I saw the problem in kisskb with gcc 4.9, hence the second fix.

Christophe

^ permalink raw reply

* Re: [PATCH v6 19/22] powerpc/book3s64/hash/kuap: Enable kuap on hash
From: Christophe Leroy @ 2021-03-15 13:02 UTC (permalink / raw)
  To: Aneesh Kumar K.V, linuxppc-dev, mpe; +Cc: Sandipan Das
In-Reply-To: <cc0338af-da92-f6d5-c0fb-aca67a2cccc5@linux.ibm.com>



Le 15/03/2021 à 13:59, Aneesh Kumar K.V a écrit :
> On 3/15/21 5:36 PM, Christophe Leroy wrote:
>>
>>
>> Le 25/11/2020 à 06:16, Aneesh Kumar K.V a écrit :
>>> Reviewed-by: Sandipan Das <sandipan@linux.ibm.com>
>>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
>>
>> PPC_HAVE_KUAP is only selected on book3s/64 when PPC_RADIX_MMU is selected. Is that normal ?
>>
> 
> 
> I guess we missed fixing that with this patch. How about

Yes, looks good to me.

> 
> modified   arch/powerpc/platforms/Kconfig.cputype
> @@ -103,6 +103,8 @@ config PPC_BOOK3S_64
>       select ARCH_SUPPORTS_NUMA_BALANCING
>       select IRQ_WORK
>       select PPC_MM_SLICES
> +    select PPC_HAVE_KUEP
> +    select PPC_HAVE_KUAP
> 
>   config PPC_BOOK3E_64
>       bool "Embedded processors"
> @@ -365,8 +367,6 @@ config PPC_RADIX_MMU
>       bool "Radix MMU Support"
>       depends on PPC_BOOK3S_64
>       select ARCH_HAS_GIGANTIC_PAGE
> -    select PPC_HAVE_KUEP
> -    select PPC_HAVE_KUAP
>       default y
>       help
>         Enable support for the Power ISA 3.0 Radix style MMU. Currently this
> 
> 
> 
> 
> 
> -aneesh

^ permalink raw reply

* Re: [PATCH] powerpc: Fix missing prototype problems for "arch/powerpc/kernel/setup_64.c"
From: Christophe Leroy @ 2021-03-15 13:01 UTC (permalink / raw)
  To: Cédric Le Goater, heying (H), mpe, benh, paulus, npiggin,
	dja, akpm, rppt, aneesh.kumar
  Cc: johnny.chenyi, linuxppc-dev
In-Reply-To: <c6af9455-9a04-c93e-ac7c-3c7d6a56953a@kaod.org>



Le 15/03/2021 à 13:57, Cédric Le Goater a écrit :
> On 3/15/21 1:48 PM, heying (H) wrote:
>>
>> 在 2021/3/15 20:17, Christophe Leroy 写道:
>>> You subject doesn't match the content of the patch.
>> OK. I'll adapt that.
>>>
>>> Le 15/03/2021 à 13:04, He Ying a écrit :
>>>> The variables 'uaccess_fulsh' and 'entry_flush' are not referenced
>>>> outside the file. So define them as static to avoid the warnings.
>>>>
>>>> And add a prototype for the function 'panic_smp_self_stop' for the
>>>> same purpose.
>>>>
>>>> Sparse also warns that 'rfi_flush' should be static. However, it's
>>>> referenced outside the file.
>>>
>>> To clear that warning, you have to include asm/security_features.h, rfi_flush is declared there.
>> Do you mean that I should include this header in arch/powerpc/kernel/setup_64.c?
> 
> yes.
> 
>>>
>>>>
>>>> The warnings about the file reported by sparse are as follows:
>>>> arch/powerpc/kernel/setup_64.c:422:6: warning: symbol 'panic_smp_self_stop' was not declared. Should it be static?
>>>> arch/powerpc/kernel/setup_64.c:951:6: warning: symbol 'rfi_flush' was not declared. Should it be static?
>>>> arch/powerpc/kernel/setup_64.c:952:6: warning: symbol 'entry_flush' was not declared. Should it be static?
>>>> arch/powerpc/kernel/setup_64.c:953:6: warning: symbol 'uaccess_flush' was not declared. Should it be static?
>>>>
>>>> Reported-by: Hulk Robot <hulkci@huawei.com>
>>>> Signed-off-by: He Ying <heying24@huawei.com>
>>>> ---
>>>>    arch/powerpc/kernel/setup_64.c | 6 ++++--
>>>>    1 file changed, 4 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c
>>>> index 560ed8b975e7..603aacd8527b 100644
>>>> --- a/arch/powerpc/kernel/setup_64.c
>>>> +++ b/arch/powerpc/kernel/setup_64.c
>>>> @@ -71,6 +71,8 @@
>>>>      #include "setup.h"
>>>>    +extern void panic_smp_self_stop(void);
>>>> +
>>>
>>> For function prototypes 'extern' is unneeded and deprecated.
>>>
>>> And function prototypes should go in an header file.
>>>
>>> panic_smp_self_stop() is called from kernel/panic.c , it should be declared in one of the generic linux header files I think.
>> Yes, you're right. But I have no idea which header it should be declared in. May I have your suggestions?
> 
> arch/powerpc/include/asm/bug.h looks like a good place.

Why declaring it in a powerpc header ?

It's a weak function defined in core part of kernel (kernel/panic.c).

I think it should go in a common header, just like for instance arch_thaw_secondary_cpus_begin()

> 
> C.
> 
>>>
>>>>    int spinning_secondaries;
>>>>    u64 ppc64_pft_size;
>>>>    @@ -949,8 +951,8 @@ static bool no_rfi_flush;
>>>>    static bool no_entry_flush;
>>>>    static bool no_uaccess_flush;
>>>>    bool rfi_flush;
>>>> -bool entry_flush;
>>>> -bool uaccess_flush;
>>>> +static bool entry_flush;
>>>> +static bool uaccess_flush;
>>>>    DEFINE_STATIC_KEY_FALSE(uaccess_flush_key);
>>>>    EXPORT_SYMBOL(uaccess_flush_key);
>>>>
>>> .

^ permalink raw reply

* Re: [PATCH v6 19/22] powerpc/book3s64/hash/kuap: Enable kuap on hash
From: Aneesh Kumar K.V @ 2021-03-15 12:59 UTC (permalink / raw)
  To: Christophe Leroy, linuxppc-dev, mpe; +Cc: Sandipan Das
In-Reply-To: <2898f50c-aa42-5842-9d29-7272c7da5d00@csgroup.eu>

On 3/15/21 5:36 PM, Christophe Leroy wrote:
> 
> 
> Le 25/11/2020 à 06:16, Aneesh Kumar K.V a écrit :
>> Reviewed-by: Sandipan Das <sandipan@linux.ibm.com>
>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> 
> PPC_HAVE_KUAP is only selected on book3s/64 when PPC_RADIX_MMU is 
> selected. Is that normal ?
> 


I guess we missed fixing that with this patch. How about

modified   arch/powerpc/platforms/Kconfig.cputype
@@ -103,6 +103,8 @@ config PPC_BOOK3S_64
  	select ARCH_SUPPORTS_NUMA_BALANCING
  	select IRQ_WORK
  	select PPC_MM_SLICES
+	select PPC_HAVE_KUEP
+	select PPC_HAVE_KUAP

  config PPC_BOOK3E_64
  	bool "Embedded processors"
@@ -365,8 +367,6 @@ config PPC_RADIX_MMU
  	bool "Radix MMU Support"
  	depends on PPC_BOOK3S_64
  	select ARCH_HAS_GIGANTIC_PAGE
-	select PPC_HAVE_KUEP
-	select PPC_HAVE_KUAP
  	default y
  	help
  	  Enable support for the Power ISA 3.0 Radix style MMU. Currently this





-aneesh

^ permalink raw reply

* Re: [PATCH] powerpc: Fix missing prototype problems for "arch/powerpc/kernel/setup_64.c"
From: Christophe Leroy @ 2021-03-15 12:57 UTC (permalink / raw)
  To: heying (H), mpe, benh, paulus, npiggin, dja, akpm, rppt,
	aneesh.kumar, clg
  Cc: johnny.chenyi, linuxppc-dev
In-Reply-To: <ddd03e30-9e54-66cd-8917-6f620557b795@huawei.com>



Le 15/03/2021 à 13:48, heying (H) a écrit :
> 
> 在 2021/3/15 20:17, Christophe Leroy 写道:
>> You subject doesn't match the content of the patch.
> OK. I'll adapt that.
>>
>> Le 15/03/2021 à 13:04, He Ying a écrit :
>>> The variables 'uaccess_fulsh' and 'entry_flush' are not referenced
>>> outside the file. So define them as static to avoid the warnings.
>>>
>>> And add a prototype for the function 'panic_smp_self_stop' for the
>>> same purpose.
>>>
>>> Sparse also warns that 'rfi_flush' should be static. However, it's
>>> referenced outside the file.
>>
>> To clear that warning, you have to include asm/security_features.h, rfi_flush is declared there.
> Do you mean that I should include this header in arch/powerpc/kernel/setup_64.c?

Yes

>>
>>>
>>> The warnings about the file reported by sparse are as follows:
>>> arch/powerpc/kernel/setup_64.c:422:6: warning: symbol 'panic_smp_self_stop' was not declared. 
>>> Should it be static?
>>> arch/powerpc/kernel/setup_64.c:951:6: warning: symbol 'rfi_flush' was not declared. Should it be 
>>> static?
>>> arch/powerpc/kernel/setup_64.c:952:6: warning: symbol 'entry_flush' was not declared. Should it 
>>> be static?
>>> arch/powerpc/kernel/setup_64.c:953:6: warning: symbol 'uaccess_flush' was not declared. Should it 
>>> be static?
>>>
>>> Reported-by: Hulk Robot <hulkci@huawei.com>
>>> Signed-off-by: He Ying <heying24@huawei.com>
>>> ---
>>>   arch/powerpc/kernel/setup_64.c | 6 ++++--
>>>   1 file changed, 4 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c
>>> index 560ed8b975e7..603aacd8527b 100644
>>> --- a/arch/powerpc/kernel/setup_64.c
>>> +++ b/arch/powerpc/kernel/setup_64.c
>>> @@ -71,6 +71,8 @@
>>>     #include "setup.h"
>>>   +extern void panic_smp_self_stop(void);
>>> +
>>
>> For function prototypes 'extern' is unneeded and deprecated.
>>
>> And function prototypes should go in an header file.
>>
>> panic_smp_self_stop() is called from kernel/panic.c , it should be declared in one of the generic 
>> linux header files I think.
> Yes, you're right. But I have no idea which header it should be declared in. May I have your 
> suggestions?

Maybe include/linux/smp.h ?

>>
>>>   int spinning_secondaries;
>>>   u64 ppc64_pft_size;
>>>   @@ -949,8 +951,8 @@ static bool no_rfi_flush;
>>>   static bool no_entry_flush;
>>>   static bool no_uaccess_flush;
>>>   bool rfi_flush;
>>> -bool entry_flush;
>>> -bool uaccess_flush;
>>> +static bool entry_flush;
>>> +static bool uaccess_flush;
>>>   DEFINE_STATIC_KEY_FALSE(uaccess_flush_key);
>>>   EXPORT_SYMBOL(uaccess_flush_key);
>>>
>> .

^ permalink raw reply

* Re: [PATCH] powerpc: Fix missing prototype problems for "arch/powerpc/kernel/setup_64.c"
From: Cédric Le Goater @ 2021-03-15 12:57 UTC (permalink / raw)
  To: heying (H), Christophe Leroy, mpe, benh, paulus, npiggin, dja,
	akpm, rppt, aneesh.kumar
  Cc: johnny.chenyi, linuxppc-dev
In-Reply-To: <ddd03e30-9e54-66cd-8917-6f620557b795@huawei.com>

On 3/15/21 1:48 PM, heying (H) wrote:
> 
> 在 2021/3/15 20:17, Christophe Leroy 写道:
>> You subject doesn't match the content of the patch.
> OK. I'll adapt that.
>>
>> Le 15/03/2021 à 13:04, He Ying a écrit :
>>> The variables 'uaccess_fulsh' and 'entry_flush' are not referenced
>>> outside the file. So define them as static to avoid the warnings.
>>>
>>> And add a prototype for the function 'panic_smp_self_stop' for the
>>> same purpose.
>>>
>>> Sparse also warns that 'rfi_flush' should be static. However, it's
>>> referenced outside the file.
>>
>> To clear that warning, you have to include asm/security_features.h, rfi_flush is declared there.
> Do you mean that I should include this header in arch/powerpc/kernel/setup_64.c?

yes.

>>
>>>
>>> The warnings about the file reported by sparse are as follows:
>>> arch/powerpc/kernel/setup_64.c:422:6: warning: symbol 'panic_smp_self_stop' was not declared. Should it be static?
>>> arch/powerpc/kernel/setup_64.c:951:6: warning: symbol 'rfi_flush' was not declared. Should it be static?
>>> arch/powerpc/kernel/setup_64.c:952:6: warning: symbol 'entry_flush' was not declared. Should it be static?
>>> arch/powerpc/kernel/setup_64.c:953:6: warning: symbol 'uaccess_flush' was not declared. Should it be static?
>>>
>>> Reported-by: Hulk Robot <hulkci@huawei.com>
>>> Signed-off-by: He Ying <heying24@huawei.com>
>>> ---
>>>   arch/powerpc/kernel/setup_64.c | 6 ++++--
>>>   1 file changed, 4 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c
>>> index 560ed8b975e7..603aacd8527b 100644
>>> --- a/arch/powerpc/kernel/setup_64.c
>>> +++ b/arch/powerpc/kernel/setup_64.c
>>> @@ -71,6 +71,8 @@
>>>     #include "setup.h"
>>>   +extern void panic_smp_self_stop(void);
>>> +
>>
>> For function prototypes 'extern' is unneeded and deprecated.
>>
>> And function prototypes should go in an header file.
>>
>> panic_smp_self_stop() is called from kernel/panic.c , it should be declared in one of the generic linux header files I think.
> Yes, you're right. But I have no idea which header it should be declared in. May I have your suggestions?

arch/powerpc/include/asm/bug.h looks like a good place.

C.

>>
>>>   int spinning_secondaries;
>>>   u64 ppc64_pft_size;
>>>   @@ -949,8 +951,8 @@ static bool no_rfi_flush;
>>>   static bool no_entry_flush;
>>>   static bool no_uaccess_flush;
>>>   bool rfi_flush;
>>> -bool entry_flush;
>>> -bool uaccess_flush;
>>> +static bool entry_flush;
>>> +static bool uaccess_flush;
>>>   DEFINE_STATIC_KEY_FALSE(uaccess_flush_key);
>>>   EXPORT_SYMBOL(uaccess_flush_key);
>>>
>> .


^ permalink raw reply

* Re: [PATCH] powerpc: Fix missing prototype problems for "arch/powerpc/kernel/setup_64.c"
From: heying (H) @ 2021-03-15 12:48 UTC (permalink / raw)
  To: Christophe Leroy, mpe, benh, paulus, npiggin, dja, akpm, rppt,
	aneesh.kumar, clg
  Cc: johnny.chenyi, linuxppc-dev
In-Reply-To: <6eb1925c-a3a1-f062-29da-3a7fa946505c@csgroup.eu>


在 2021/3/15 20:17, Christophe Leroy 写道:
> You subject doesn't match the content of the patch.
OK. I'll adapt that.
>
> Le 15/03/2021 à 13:04, He Ying a écrit :
>> The variables 'uaccess_fulsh' and 'entry_flush' are not referenced
>> outside the file. So define them as static to avoid the warnings.
>>
>> And add a prototype for the function 'panic_smp_self_stop' for the
>> same purpose.
>>
>> Sparse also warns that 'rfi_flush' should be static. However, it's
>> referenced outside the file.
>
> To clear that warning, you have to include asm/security_features.h, 
> rfi_flush is declared there.
Do you mean that I should include this header in 
arch/powerpc/kernel/setup_64.c?
>
>>
>> The warnings about the file reported by sparse are as follows:
>> arch/powerpc/kernel/setup_64.c:422:6: warning: symbol 
>> 'panic_smp_self_stop' was not declared. Should it be static?
>> arch/powerpc/kernel/setup_64.c:951:6: warning: symbol 'rfi_flush' was 
>> not declared. Should it be static?
>> arch/powerpc/kernel/setup_64.c:952:6: warning: symbol 'entry_flush' 
>> was not declared. Should it be static?
>> arch/powerpc/kernel/setup_64.c:953:6: warning: symbol 'uaccess_flush' 
>> was not declared. Should it be static?
>>
>> Reported-by: Hulk Robot <hulkci@huawei.com>
>> Signed-off-by: He Ying <heying24@huawei.com>
>> ---
>>   arch/powerpc/kernel/setup_64.c | 6 ++++--
>>   1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/powerpc/kernel/setup_64.c 
>> b/arch/powerpc/kernel/setup_64.c
>> index 560ed8b975e7..603aacd8527b 100644
>> --- a/arch/powerpc/kernel/setup_64.c
>> +++ b/arch/powerpc/kernel/setup_64.c
>> @@ -71,6 +71,8 @@
>>     #include "setup.h"
>>   +extern void panic_smp_self_stop(void);
>> +
>
> For function prototypes 'extern' is unneeded and deprecated.
>
> And function prototypes should go in an header file.
>
> panic_smp_self_stop() is called from kernel/panic.c , it should be 
> declared in one of the generic linux header files I think.
Yes, you're right. But I have no idea which header it should be declared 
in. May I have your suggestions?
>
>>   int spinning_secondaries;
>>   u64 ppc64_pft_size;
>>   @@ -949,8 +951,8 @@ static bool no_rfi_flush;
>>   static bool no_entry_flush;
>>   static bool no_uaccess_flush;
>>   bool rfi_flush;
>> -bool entry_flush;
>> -bool uaccess_flush;
>> +static bool entry_flush;
>> +static bool uaccess_flush;
>>   DEFINE_STATIC_KEY_FALSE(uaccess_flush_key);
>>   EXPORT_SYMBOL(uaccess_flush_key);
>>
> .

^ permalink raw reply

* Re: [PATCH] powerpc: Fix missing prototype problems for "arch/powerpc/kernel/setup_64.c"
From: Christophe Leroy @ 2021-03-15 12:17 UTC (permalink / raw)
  To: He Ying, mpe, benh, paulus, npiggin, dja, akpm, rppt,
	aneesh.kumar, clg
  Cc: johnny.chenyi, linuxppc-dev
In-Reply-To: <20210315120444.215905-1-heying24@huawei.com>

You subject doesn't match the content of the patch.

Le 15/03/2021 à 13:04, He Ying a écrit :
> The variables 'uaccess_fulsh' and 'entry_flush' are not referenced
> outside the file. So define them as static to avoid the warnings.
> 
> And add a prototype for the function 'panic_smp_self_stop' for the
> same purpose.
> 
> Sparse also warns that 'rfi_flush' should be static. However, it's
> referenced outside the file.

To clear that warning, you have to include asm/security_features.h, rfi_flush is declared there.

> 
> The warnings about the file reported by sparse are as follows:
> arch/powerpc/kernel/setup_64.c:422:6: warning: symbol 'panic_smp_self_stop' was not declared. Should it be static?
> arch/powerpc/kernel/setup_64.c:951:6: warning: symbol 'rfi_flush' was not declared. Should it be static?
> arch/powerpc/kernel/setup_64.c:952:6: warning: symbol 'entry_flush' was not declared. Should it be static?
> arch/powerpc/kernel/setup_64.c:953:6: warning: symbol 'uaccess_flush' was not declared. Should it be static?
> 
> Reported-by: Hulk Robot <hulkci@huawei.com>
> Signed-off-by: He Ying <heying24@huawei.com>
> ---
>   arch/powerpc/kernel/setup_64.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c
> index 560ed8b975e7..603aacd8527b 100644
> --- a/arch/powerpc/kernel/setup_64.c
> +++ b/arch/powerpc/kernel/setup_64.c
> @@ -71,6 +71,8 @@
>   
>   #include "setup.h"
>   
> +extern void panic_smp_self_stop(void);
> +

For function prototypes 'extern' is unneeded and deprecated.

And function prototypes should go in an header file.

panic_smp_self_stop() is called from kernel/panic.c , it should be declared in one of the generic 
linux header files I think.

>   int spinning_secondaries;
>   u64 ppc64_pft_size;
>   
> @@ -949,8 +951,8 @@ static bool no_rfi_flush;
>   static bool no_entry_flush;
>   static bool no_uaccess_flush;
>   bool rfi_flush;
> -bool entry_flush;
> -bool uaccess_flush;
> +static bool entry_flush;
> +static bool uaccess_flush;
>   DEFINE_STATIC_KEY_FALSE(uaccess_flush_key);
>   EXPORT_SYMBOL(uaccess_flush_key);
>   
> 

^ permalink raw reply

* Re: [PATCH v6 19/22] powerpc/book3s64/hash/kuap: Enable kuap on hash
From: Christophe Leroy @ 2021-03-15 12:06 UTC (permalink / raw)
  To: Aneesh Kumar K.V, linuxppc-dev, mpe; +Cc: Sandipan Das
In-Reply-To: <20201125051634.509286-20-aneesh.kumar@linux.ibm.com>



Le 25/11/2020 à 06:16, Aneesh Kumar K.V a écrit :
> Reviewed-by: Sandipan Das <sandipan@linux.ibm.com>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>

PPC_HAVE_KUAP is only selected on book3s/64 when PPC_RADIX_MMU is selected. Is that normal ?


> ---
>   arch/powerpc/mm/book3s64/pkeys.c | 7 ++++++-
>   1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/mm/book3s64/pkeys.c b/arch/powerpc/mm/book3s64/pkeys.c
> index f747d66cc87d..84f8664ffc47 100644
> --- a/arch/powerpc/mm/book3s64/pkeys.c
> +++ b/arch/powerpc/mm/book3s64/pkeys.c
> @@ -257,7 +257,12 @@ void __init setup_kuep(bool disabled)
>   #ifdef CONFIG_PPC_KUAP
>   void __init setup_kuap(bool disabled)
>   {
> -	if (disabled || !early_radix_enabled())
> +	if (disabled)
> +		return;
> +	/*
> +	 * On hash if PKEY feature is not enabled, disable KUAP too.
> +	 */
> +	if (!early_radix_enabled() && !early_mmu_has_feature(MMU_FTR_PKEY))
>   		return;
>   
>   	if (smp_processor_id() == boot_cpuid) {
> 

^ permalink raw reply

* [PATCH] powerpc: Fix missing prototype problems for "arch/powerpc/kernel/setup_64.c"
From: He Ying @ 2021-03-15 12:04 UTC (permalink / raw)
  To: mpe, benh, paulus, npiggin, dja, akpm, rppt, aneesh.kumar, clg
  Cc: johnny.chenyi, linuxppc-dev

The variables 'uaccess_fulsh' and 'entry_flush' are not referenced
outside the file. So define them as static to avoid the warnings.

And add a prototype for the function 'panic_smp_self_stop' for the
same purpose.

Sparse also warns that 'rfi_flush' should be static. However, it's
referenced outside the file.

The warnings about the file reported by sparse are as follows:
arch/powerpc/kernel/setup_64.c:422:6: warning: symbol 'panic_smp_self_stop' was not declared. Should it be static?
arch/powerpc/kernel/setup_64.c:951:6: warning: symbol 'rfi_flush' was not declared. Should it be static?
arch/powerpc/kernel/setup_64.c:952:6: warning: symbol 'entry_flush' was not declared. Should it be static?
arch/powerpc/kernel/setup_64.c:953:6: warning: symbol 'uaccess_flush' was not declared. Should it be static?

Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: He Ying <heying24@huawei.com>
---
 arch/powerpc/kernel/setup_64.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c
index 560ed8b975e7..603aacd8527b 100644
--- a/arch/powerpc/kernel/setup_64.c
+++ b/arch/powerpc/kernel/setup_64.c
@@ -71,6 +71,8 @@
 
 #include "setup.h"
 
+extern void panic_smp_self_stop(void);
+
 int spinning_secondaries;
 u64 ppc64_pft_size;
 
@@ -949,8 +951,8 @@ static bool no_rfi_flush;
 static bool no_entry_flush;
 static bool no_uaccess_flush;
 bool rfi_flush;
-bool entry_flush;
-bool uaccess_flush;
+static bool entry_flush;
+static bool uaccess_flush;
 DEFINE_STATIC_KEY_FALSE(uaccess_flush_key);
 EXPORT_SYMBOL(uaccess_flush_key);
 
-- 
2.17.1


^ permalink raw reply related

* [PATCH] powerpc/math: Fix missing __user qualifier for get_user() and other sparse warnings
From: Christophe Leroy @ 2021-03-15 12:00 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: linuxppc-dev, linux-kernel

Sparse reports the following problems:

arch/powerpc/math-emu/math.c:228:21: warning: Using plain integer as NULL pointer
arch/powerpc/math-emu/math.c:228:31: warning: Using plain integer as NULL pointer
arch/powerpc/math-emu/math.c:228:41: warning: Using plain integer as NULL pointer
arch/powerpc/math-emu/math.c:228:51: warning: Using plain integer as NULL pointer
arch/powerpc/math-emu/math.c:237:13: warning: incorrect type in initializer (different address spaces)
arch/powerpc/math-emu/math.c:237:13:    expected unsigned int [noderef] __user *_gu_addr
arch/powerpc/math-emu/math.c:237:13:    got unsigned int [usertype] *
arch/powerpc/math-emu/math.c:226:1: warning: symbol 'do_mathemu' was not declared. Should it be static?

Add missing __user qualifier when casting pointer used in get_user()

Use NULL instead of 0 to initialise opX local variables.

Add a prototype for do_mathemu() (Added in processor.h like sparc)

Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
 arch/powerpc/include/asm/processor.h | 2 ++
 arch/powerpc/kernel/traps.c          | 1 -
 arch/powerpc/math-emu/math.c         | 4 ++--
 3 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/include/asm/processor.h b/arch/powerpc/include/asm/processor.h
index 8acc3590c971..f21834a1d32e 100644
--- a/arch/powerpc/include/asm/processor.h
+++ b/arch/powerpc/include/asm/processor.h
@@ -417,6 +417,8 @@ extern int fix_alignment(struct pt_regs *);
 #define NET_IP_ALIGN	0
 #endif
 
+int do_mathemu(struct pt_regs *regs);
+
 #endif /* __KERNEL__ */
 #endif /* __ASSEMBLY__ */
 #endif /* _ASM_POWERPC_PROCESSOR_H */
diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
index d615cd7ebfae..3cafbde7a51c 100644
--- a/arch/powerpc/kernel/traps.c
+++ b/arch/powerpc/kernel/traps.c
@@ -1406,7 +1406,6 @@ int is_valid_bugaddr(unsigned long addr)
 static int emulate_math(struct pt_regs *regs)
 {
 	int ret;
-	extern int do_mathemu(struct pt_regs *regs);
 
 	ret = do_mathemu(regs);
 	if (ret >= 0)
diff --git a/arch/powerpc/math-emu/math.c b/arch/powerpc/math-emu/math.c
index 30b4b69c6941..327165f26ca6 100644
--- a/arch/powerpc/math-emu/math.c
+++ b/arch/powerpc/math-emu/math.c
@@ -225,7 +225,7 @@ record_exception(struct pt_regs *regs, int eflag)
 int
 do_mathemu(struct pt_regs *regs)
 {
-	void *op0 = 0, *op1 = 0, *op2 = 0, *op3 = 0;
+	void *op0 = NULL, *op1 = NULL, *op2 = NULL, *op3 = NULL;
 	unsigned long pc = regs->nip;
 	signed short sdisp;
 	u32 insn = 0;
@@ -234,7 +234,7 @@ do_mathemu(struct pt_regs *regs)
 	int type = 0;
 	int eflag, trap;
 
-	if (get_user(insn, (u32 *)pc))
+	if (get_user(insn, (u32 __user *)pc))
 		return -EFAULT;
 
 	switch (insn >> 26) {
-- 
2.25.0


^ permalink raw reply related

* [PATCH v2 0/6] Speedup mremap on ppc64
From: Aneesh Kumar K.V @ 2021-03-15 11:38 UTC (permalink / raw)
  To: linux-mm, akpm; +Cc: peterz, kaleshsingh, Aneesh Kumar K.V, joel, linuxppc-dev

This patchset enables MOVE_PMD/MOVE_PUD support on power. This requires
the platform to support updating higher-level page tables without
updating page table entries. This also needs to invalidate the Page Walk
Cache on architecture supporting the same.

The patchset does that by switching the page table update to use mmu gather
interface instead of direct tlb flush. mmu gather allows the architecture
to manage page walk cache invalidate separately.

Changes from V1:
* Rebase to recent upstream
* Fix build issues with tlb_gather_mmu changes


Aneesh Kumar K.V (6):
  selftest/mremap_test: Update the test to handle pagesize other than 4K
  selftest/mremap_test: Avoid crash with static build
  mm/mremap: Use pmd/pud_poplulate to update page table entries
  mm/mremap: Use mmu gather interface instead of flush_tlb_range
  mm/mremap: Allow arch runtime override
  powerpc/mm: Enable move pmd/pud

 arch/arc/include/asm/tlb.h               |   5 +
 arch/arm64/include/asm/tlb.h             |   6 ++
 arch/powerpc/include/asm/tlb.h           |   6 ++
 arch/powerpc/platforms/Kconfig.cputype   |   2 +
 arch/x86/include/asm/tlb.h               |   5 +
 mm/mremap.c                              |  54 +++++++++--
 tools/testing/selftests/vm/mremap_test.c | 118 ++++++++++++-----------
 7 files changed, 133 insertions(+), 63 deletions(-)

-- 
2.29.2


^ permalink raw reply

* [PATCH v2 6/6] powerpc/mm: Enable move pmd/pud
From: Aneesh Kumar K.V @ 2021-03-15 11:38 UTC (permalink / raw)
  To: linux-mm, akpm; +Cc: peterz, kaleshsingh, Aneesh Kumar K.V, joel, linuxppc-dev
In-Reply-To: <20210315113824.270796-1-aneesh.kumar@linux.ibm.com>

mremap HAVE_MOVE_PMD/PUD optimization time comparison for 1GB region:
1GB mremap - Source PTE-aligned, Destination PTE-aligned
        mremap time:      1114318ns
1GB mremap - Source PMD-aligned, Destination PMD-aligned
        mremap time:      1097715ns
1GB mremap - Source PUD-aligned, Destination PUD-aligned
        mremap time:        26851ns

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
 arch/powerpc/platforms/Kconfig.cputype | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/powerpc/platforms/Kconfig.cputype b/arch/powerpc/platforms/Kconfig.cputype
index 3ce907523b1e..2e666e569fdf 100644
--- a/arch/powerpc/platforms/Kconfig.cputype
+++ b/arch/powerpc/platforms/Kconfig.cputype
@@ -97,6 +97,8 @@ config PPC_BOOK3S_64
 	select PPC_HAVE_PMU_SUPPORT
 	select SYS_SUPPORTS_HUGETLBFS
 	select HAVE_ARCH_TRANSPARENT_HUGEPAGE
+	select HAVE_MOVE_PMD
+	select HAVE_MOVE_PUD
 	select ARCH_ENABLE_THP_MIGRATION if TRANSPARENT_HUGEPAGE
 	select ARCH_SUPPORTS_NUMA_BALANCING
 	select IRQ_WORK
-- 
2.29.2


^ permalink raw reply related

* [PATCH v2 2/6] selftest/mremap_test: Avoid crash with static build
From: Aneesh Kumar K.V @ 2021-03-15 11:38 UTC (permalink / raw)
  To: linux-mm, akpm; +Cc: peterz, kaleshsingh, Aneesh Kumar K.V, joel, linuxppc-dev
In-Reply-To: <20210315113824.270796-1-aneesh.kumar@linux.ibm.com>

With a large mmap map size, we can overlap with the text area and using
MAP_FIXED results in unmapping that area. Switch to MAP_FIXED_NOREPLACE
and handle the EEXIST error.

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
 tools/testing/selftests/vm/mremap_test.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/vm/mremap_test.c b/tools/testing/selftests/vm/mremap_test.c
index c9a5461eb786..0624d1bd71b5 100644
--- a/tools/testing/selftests/vm/mremap_test.c
+++ b/tools/testing/selftests/vm/mremap_test.c
@@ -75,9 +75,10 @@ static void *get_source_mapping(struct config c)
 retry:
 	addr += c.src_alignment;
 	src_addr = mmap((void *) addr, c.region_size, PROT_READ | PROT_WRITE,
-			MAP_FIXED | MAP_ANONYMOUS | MAP_SHARED, -1, 0);
+			MAP_FIXED_NOREPLACE | MAP_ANONYMOUS | MAP_SHARED,
+			-1, 0);
 	if (src_addr == MAP_FAILED) {
-		if (errno == EPERM)
+		if (errno == EPERM || errno == EEXIST)
 			goto retry;
 		goto error;
 	}
-- 
2.29.2


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox