LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [RFC PATCH 1/3] powerpc/mm: Introduce temporary mm
From: Christophe Leroy @ 2020-04-18 10:36 UTC (permalink / raw)
  To: Christopher M. Riedl, linuxppc-dev
In-Reply-To: <20200323045205.20314-2-cmr@informatik.wtf>



Le 23/03/2020 à 05:52, Christopher M. Riedl a écrit :
> x86 supports the notion of a temporary mm which restricts access to
> temporary PTEs to a single CPU. A temporary mm is useful for situations
> where a CPU needs to perform sensitive operations (such as patching a
> STRICT_KERNEL_RWX kernel) requiring temporary mappings without exposing
> said mappings to other CPUs. A side benefit is that other CPU TLBs do
> not need to be flushed when the temporary mm is torn down.
> 
> Mappings in the temporary mm can be set in the userspace portion of the
> address-space.
> 
> Interrupts must be disabled while the temporary mm is in use. HW
> breakpoints, which may have been set by userspace as watchpoints on
> addresses now within the temporary mm, are saved and disabled when
> loading the temporary mm. The HW breakpoints are restored when unloading
> the temporary mm. All HW breakpoints are indiscriminately disabled while
> the temporary mm is in use.
> 
> Based on x86 implementation:
> 
> commit cefa929c034e
> ("x86/mm: Introduce temporary mm structs")
> 
> Signed-off-by: Christopher M. Riedl <cmr@informatik.wtf>
> ---
>   arch/powerpc/include/asm/debug.h       |  1 +
>   arch/powerpc/include/asm/mmu_context.h | 56 +++++++++++++++++++++++++-
>   arch/powerpc/kernel/process.c          |  5 +++
>   3 files changed, 61 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/include/asm/debug.h b/arch/powerpc/include/asm/debug.h
> index 7756026b95ca..b945bc16c932 100644
> --- a/arch/powerpc/include/asm/debug.h
> +++ b/arch/powerpc/include/asm/debug.h
> @@ -45,6 +45,7 @@ static inline int debugger_break_match(struct pt_regs *regs) { return 0; }
>   static inline int debugger_fault_handler(struct pt_regs *regs) { return 0; }
>   #endif
>   
> +void __get_breakpoint(struct arch_hw_breakpoint *brk);
>   void __set_breakpoint(struct arch_hw_breakpoint *brk);
>   bool ppc_breakpoint_available(void);
>   #ifdef CONFIG_PPC_ADV_DEBUG_REGS
> diff --git a/arch/powerpc/include/asm/mmu_context.h b/arch/powerpc/include/asm/mmu_context.h
> index 360367c579de..3e6381d04c28 100644
> --- a/arch/powerpc/include/asm/mmu_context.h
> +++ b/arch/powerpc/include/asm/mmu_context.h
> @@ -7,9 +7,10 @@
>   #include <linux/mm.h>
>   #include <linux/sched.h>
>   #include <linux/spinlock.h>
> -#include <asm/mmu.h>	
> +#include <asm/mmu.h>
>   #include <asm/cputable.h>
>   #include <asm/cputhreads.h>
> +#include <asm/hw_breakpoint.h>
>   
>   /*
>    * Most if the context management is out of line
> @@ -270,5 +271,58 @@ static inline int arch_dup_mmap(struct mm_struct *oldmm,
>   	return 0;
>   }
>   
> +struct temp_mm {
> +	struct mm_struct *temp;
> +	struct mm_struct *prev;
> +	bool is_kernel_thread;
> +	struct arch_hw_breakpoint brk;
> +};
> +
> +static inline void init_temp_mm(struct temp_mm *temp_mm, struct mm_struct *mm)
> +{
> +	temp_mm->temp = mm;
> +	temp_mm->prev = NULL;
> +	temp_mm->is_kernel_thread = false;
> +	memset(&temp_mm->brk, 0, sizeof(temp_mm->brk));
> +}
> +
> +static inline void use_temporary_mm(struct temp_mm *temp_mm)
> +{
> +	lockdep_assert_irqs_disabled();
> +
> +	temp_mm->is_kernel_thread = current->mm == NULL;
> +	if (temp_mm->is_kernel_thread)
> +		temp_mm->prev = current->active_mm;
> +	else
> +		temp_mm->prev = current->mm;

Could be:
	temp_mm->prev = current->mm ? : current->active_mm;

> +
> +	/*
> +	 * Hash requires a non-NULL current->mm to allocate a userspace address
> +	 * when handling a page fault. Does not appear to hurt in Radix either.
> +	 */
> +	current->mm = temp_mm->temp;
> +	switch_mm_irqs_off(NULL, temp_mm->temp, current);
> +
> +	if (ppc_breakpoint_available()) {
> +		__get_breakpoint(&temp_mm->brk);
> +		if (temp_mm->brk.type != 0)
> +			hw_breakpoint_disable();
> +	}

There is a series for having more than one breakpoint, see 
https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=170073&state=*

> +}
> +
> +static inline void unuse_temporary_mm(struct temp_mm *temp_mm)
> +{
> +	lockdep_assert_irqs_disabled();
> +
> +	if (temp_mm->is_kernel_thread)
> +		current->mm = NULL;
> +	else
> +		current->mm = temp_mm->prev;

Could be:
	current->mm = !temp_mm->is_kernel_thread ? temp_mm->prev : NULL;

> +	switch_mm_irqs_off(NULL, temp_mm->prev, current);
> +
> +	if (ppc_breakpoint_available() && temp_mm->brk.type != 0)
> +		__set_breakpoint(&temp_mm->brk);
> +}
> +
>   #endif /* __KERNEL__ */
>   #endif /* __ASM_POWERPC_MMU_CONTEXT_H */
> diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
> index fad50db9dcf2..5e5cf33fc358 100644
> --- a/arch/powerpc/kernel/process.c
> +++ b/arch/powerpc/kernel/process.c
> @@ -793,6 +793,11 @@ static inline int set_breakpoint_8xx(struct arch_hw_breakpoint *brk)
>   	return 0;
>   }
>   
> +void __get_breakpoint(struct arch_hw_breakpoint *brk)
> +{
> +	memcpy(brk, this_cpu_ptr(&current_brk), sizeof(*brk));
> +}
> +
>   void __set_breakpoint(struct arch_hw_breakpoint *brk)
>   {
>   	memcpy(this_cpu_ptr(&current_brk), brk, sizeof(*brk));
> 

Christophe

^ permalink raw reply

* Re: [RFC PATCH] powerpc/lib: Fixing use a temporary mm for code patching
From: Christophe Leroy @ 2020-04-18 10:27 UTC (permalink / raw)
  To: Christopher M Riedl; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <1418874364.198277.1586967776509@privateemail.com>



Le 15/04/2020 à 18:22, Christopher M Riedl a écrit :
>> On April 15, 2020 4:12 AM Christophe Leroy <christophe.leroy@c-s.fr> wrote:
>>
>>   
>> Le 15/04/2020 à 07:16, Christopher M Riedl a écrit :
>>>> On March 26, 2020 9:42 AM Christophe Leroy <christophe.leroy@c-s.fr> wrote:
>>>>
>>>>    
>>>> This patch fixes the RFC series identified below.
>>>> It fixes three points:
>>>> - Failure with CONFIG_PPC_KUAP
>>>> - Failure to write do to lack of DIRTY bit set on the 8xx
>>>> - Inadequaly complex WARN post verification
>>>>
>>>> However, it has an impact on the CPU load. Here is the time
>>>> needed on an 8xx to run the ftrace selftests without and
>>>> with this series:
>>>> - Without CONFIG_STRICT_KERNEL_RWX		==> 38 seconds
>>>> - With CONFIG_STRICT_KERNEL_RWX			==> 40 seconds
>>>> - With CONFIG_STRICT_KERNEL_RWX + this series	==> 43 seconds
>>>>
>>>> Link: https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=166003
>>>> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
>>>> ---
>>>>    arch/powerpc/lib/code-patching.c | 5 ++++-
>>>>    1 file changed, 4 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
>>>> index f156132e8975..4ccff427592e 100644
>>>> --- a/arch/powerpc/lib/code-patching.c
>>>> +++ b/arch/powerpc/lib/code-patching.c
>>>> @@ -97,6 +97,7 @@ static int map_patch(const void *addr, struct patch_mapping *patch_mapping)
>>>>    	}
>>>>    
>>>>    	pte = mk_pte(page, pgprot);
>>>> +	pte = pte_mkdirty(pte);
>>>>    	set_pte_at(patching_mm, patching_addr, ptep, pte);
>>>>    
>>>>    	init_temp_mm(&patch_mapping->temp_mm, patching_mm);
>>>> @@ -168,7 +169,9 @@ static int do_patch_instruction(unsigned int *addr, unsigned int instr)
>>>>    			(offset_in_page((unsigned long)addr) /
>>>>    				sizeof(unsigned int));
>>>>    
>>>> +	allow_write_to_user(patch_addr, sizeof(instr));
>>>>    	__patch_instruction(addr, instr, patch_addr);
>>>> +	prevent_write_to_user(patch_addr, sizeof(instr));
>>>>
>>>
>>> On radix we can map the page with PAGE_KERNEL protection which ends up
>>> setting EAA[0] in the radix PTE. This means the KUAP (AMR) protection is
>>> ignored (ISA v3.0b Fig. 35) since we are accessing the page from MSR[PR]=0.
>>>
>>> Can we employ a similar approach on the 8xx? I would prefer *not* to wrap
>>> the __patch_instruction() with the allow_/prevent_write_to_user() KUAP things
>>> because this is a temporary kernel mapping which really isn't userspace in
>>> the usual sense.
>>
>> On the 8xx, that's pretty different.
>>
>> The PTE doesn't control whether a page is user page or a kernel page.
>> The only thing that is set in the PTE is whether a page is linked to a
>> given PID or not.
>> PAGE_KERNEL tells that the page can be addressed with any PID.
>>
>> The user access right is given by a kind of zone, which is in the PGD
>> entry. Every pages above PAGE_OFFSET are defined as belonging to zone 0.
>> Every pages below PAGE_OFFSET are defined as belonging to zone 1.
>>
>> By default, zone 0 can only be accessed by kernel, and zone 1 can only
>> be accessed by user. When kernel wants to access zone 1, it temporarily
>> changes properties of zone 1 to allow both kernel and user accesses.
>>
>> So, if your mapping is below PAGE_OFFSET, it is in zone 1 and kernel
>> must unlock it to access it.
>>
>>
>> And this is more or less the same on hash/32. This is managed by segment
>> registers. One segment register corresponds to a 256Mbytes area. Every
>> pages below PAGE_OFFSET can only be read by default by kernel. Only user
>> can write if the PTE allows it. When the kernel needs to write at an
>> address below PAGE_OFFSET, it must change the segment properties in the
>> corresponding segment register.
>>
>> So, for both cases, if we want to have it local to a task while still
>> allowing kernel access, it means we have to define a new special area
>> between TASK_SIZE and PAGE_OFFSET which belongs to kernel zone.
>>
>> That looks complex to me for a small benefit, especially as 8xx is not
>> SMP and neither are most of the hash/32 targets.
>>
> 
> Agreed. So I guess the solution is to differentiate between radix/non-radix
> and use PAGE_SHARED for non-radix along with the KUAP functions when KUAP
> is enabled. Hmm, I need to think about this some more, especially if it's
> acceptable to temporarily map kernel text as PAGE_SHARED for patching. Do
> you see any obvious problems on 8xx and hash/32 w/ using PAGE_SHARED?

No it shouldn't be a problem AFAICS, except maybe the CPU overhead it 
brings as I mentioned previously (ftrace selftests going from 40 to 43 
seconds ie 8% overhead.

> 
> I don't necessarily want to drop the local mm patching idea for non-radix
> platforms since that means we would have to maintain two implementations.
> 

What's the problem with RADIX, why can't PAGE_SHARED be used on radix ?

Christophe

^ permalink raw reply

* How to use "y" constraint in GCC inline powerpc assembly ?
From: Christophe Leroy @ 2020-04-18  8:28 UTC (permalink / raw)
  To: Segher Boessenkool; +Cc: linuxppc-dev@lists.ozlabs.org

Hi Segher,

I'd like to use cr instead of gpr to return error condition from 
__get_user().

I saw in GCC doc 
(https://gcc.gnu.org/onlinedocs/gcc/Machine-Constraints.html) that it is 
possible to use "y" as constraint to refer to "Any condition register 
field, cr0…cr7".

I tried the test below, but it fails with "error: impossible register 
constraint in 'asm'"

How does "y" has to be used ?

int test(char *p)
{
	struct {
		int r:1;
	} res = {0};

	asm("crnot %0 * 4 + eq, %0 * 4 + eq": "=&y"(res.r));

	if (res.r)
		return -14;
	return 0;
}

Christophe

^ permalink raw reply

* Re: [PATCH 8/8] exec: open code copy_string_kernel
From: Christophe Leroy @ 2020-04-18  8:15 UTC (permalink / raw)
  To: Christoph Hellwig, Andrew Morton, Alexander Viro
  Cc: Arnd Bergmann, linux-kernel, Eric W . Biederman, linux-fsdevel,
	linuxppc-dev, Jeremy Kerr
In-Reply-To: <20200414070142.288696-9-hch@lst.de>



Le 14/04/2020 à 09:01, Christoph Hellwig a écrit :
> Currently copy_string_kernel is just a wrapper around copy_strings that
> simplifies the calling conventions and uses set_fs to allow passing a
> kernel pointer.  But due to the fact the we only need to handle a single
> kernel argument pointer, the logic can be sigificantly simplified while
> getting rid of the set_fs.


Instead of duplicating almost identical code, can you write a function 
that takes whether the source is from user or from kernel, then you just 
do things like:

	if (from_user)
		len = strnlen_user(str, MAX_ARG_STRLEN);
	else
		len = strnlen(str, MAX_ARG_STRLEN);


	if (from_user)
		copy_from_user(kaddr+offset, str, bytes_to_copy);
	else
		memcpy(kaddr+offset, str, bytes_to_copy);

> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>   fs/exec.c | 43 ++++++++++++++++++++++++++++++++++---------
>   1 file changed, 34 insertions(+), 9 deletions(-)
> 
> diff --git a/fs/exec.c b/fs/exec.c
> index b2a77d5acede..ea90af1fb236 100644
> --- a/fs/exec.c
> +++ b/fs/exec.c
> @@ -592,17 +592,42 @@ static int copy_strings(int argc, struct user_arg_ptr argv,
>    */
>   int copy_string_kernel(const char *arg, struct linux_binprm *bprm)
>   {
> -	int r;
> -	mm_segment_t oldfs = get_fs();
> -	struct user_arg_ptr argv = {
> -		.ptr.native = (const char __user *const  __user *)&arg,
> -	};
> +	int len = strnlen(arg, MAX_ARG_STRLEN) + 1 /* terminating NUL */;
> +	unsigned long pos = bprm->p;
> +
> +	if (len == 0)
> +		return -EFAULT;
> +	if (!valid_arg_len(bprm, len))
> +		return -E2BIG;
> +
> +	/* We're going to work our way backwards. */
> +	arg += len;
> +	bprm->p -= len;
> +	if (IS_ENABLED(CONFIG_MMU) && bprm->p < bprm->argmin)
> +		return -E2BIG;
> +
> +	while (len > 0) {
> +		unsigned int bytes_to_copy = min_t(unsigned int, len,
> +				min_not_zero(offset_in_page(pos), PAGE_SIZE));
> +		struct page *page;
> +		char *kaddr;
>   
> -	set_fs(KERNEL_DS);
> -	r = copy_strings(1, argv, bprm);
> -	set_fs(oldfs);
> +		pos -= bytes_to_copy;
> +		arg -= bytes_to_copy;
> +		len -= bytes_to_copy;
>   
> -	return r;
> +		page = get_arg_page(bprm, pos, 1);
> +		if (!page)
> +			return -E2BIG;
> +		kaddr = kmap_atomic(page);
> +		flush_arg_page(bprm, pos & PAGE_MASK, page);
> +		memcpy(kaddr + offset_in_page(pos), arg, bytes_to_copy);
> +		flush_kernel_dcache_page(page);
> +		kunmap_atomic(kaddr);
> +		put_arg_page(page);
> +	}
> +
> +	return 0;
>   }
>   EXPORT_SYMBOL(copy_string_kernel);
>   
> 

Christophe

^ permalink raw reply

* Re: [PATCH 1/2] signal: Factor copy_siginfo_to_external32 from copy_siginfo_to_user32
From: Christophe Leroy @ 2020-04-18  8:05 UTC (permalink / raw)
  To: Eric W. Biederman, Christoph Hellwig
  Cc: Arnd Bergmann, x86, linux-kernel, Alexander Viro, linux-fsdevel,
	Andrew Morton, linuxppc-dev, Jeremy Kerr
In-Reply-To: <87k12dakfx.fsf_-_@x220.int.ebiederm.org>



Le 17/04/2020 à 23:09, Eric W. Biederman a écrit :
> 
> To remove the use of set_fs in the coredump code there needs to be a
> way to convert a kernel siginfo to a userspace compat siginfo.
> 
> Call that function copy_siginfo_to_compat and factor it out of
> copy_siginfo_to_user32.

I find it a pitty to do that.

The existing function could have been easily converted to using 
user_access_begin() + user_access_end() and use unsafe_put_user() to 
copy to userspace to avoid copying through a temporary structure on the 
stack.

With your change, it becomes impossible to do that.

Is that really an issue to use that set_fs() in the coredump code ?

Christophe

> 
> The existence of x32 complicates this code.  On x32 SIGCHLD uses 64bit
> times for utime and stime.  As only SIGCHLD is affected and SIGCHLD
> never causes a coredump I have avoided handling that case.
> 
> Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
> ---
>   include/linux/compat.h |   1 +
>   kernel/signal.c        | 108 +++++++++++++++++++++++------------------
>   2 files changed, 63 insertions(+), 46 deletions(-)
> 
> diff --git a/include/linux/compat.h b/include/linux/compat.h
> index 0480ba4db592..4962b254e550 100644
> --- a/include/linux/compat.h
> +++ b/include/linux/compat.h
> @@ -402,6 +402,7 @@ long compat_get_bitmap(unsigned long *mask, const compat_ulong_t __user *umask,
>   		       unsigned long bitmap_size);
>   long compat_put_bitmap(compat_ulong_t __user *umask, unsigned long *mask,
>   		       unsigned long bitmap_size);
> +void copy_siginfo_to_external32(struct compat_siginfo *to, const struct kernel_siginfo *from);
>   int copy_siginfo_from_user32(kernel_siginfo_t *to, const struct compat_siginfo __user *from);
>   int copy_siginfo_to_user32(struct compat_siginfo __user *to, const kernel_siginfo_t *from);
>   int get_compat_sigevent(struct sigevent *event,
> diff --git a/kernel/signal.c b/kernel/signal.c
> index e58a6c619824..578f196898cb 100644
> --- a/kernel/signal.c
> +++ b/kernel/signal.c
> @@ -3235,90 +3235,106 @@ int copy_siginfo_from_user(kernel_siginfo_t *to, const siginfo_t __user *from)
>   }
>   
>   #ifdef CONFIG_COMPAT
> -int copy_siginfo_to_user32(struct compat_siginfo __user *to,
> -			   const struct kernel_siginfo *from)
> -#if defined(CONFIG_X86_X32_ABI) || defined(CONFIG_IA32_EMULATION)
> +void copy_siginfo_to_external32(struct compat_siginfo *to,
> +				const struct kernel_siginfo *from)
>   {
> -	return __copy_siginfo_to_user32(to, from, in_x32_syscall());
> -}
> -int __copy_siginfo_to_user32(struct compat_siginfo __user *to,
> -			     const struct kernel_siginfo *from, bool x32_ABI)
> -#endif
> -{
> -	struct compat_siginfo new;
> -	memset(&new, 0, sizeof(new));
> +	/*
> +	 * This function does not work properly for SIGCHLD on x32,
> +	 * but it does not need to as SIGCHLD never causes a coredump.
> +	 */
> +	memset(to, 0, sizeof(*to));
>   
> -	new.si_signo = from->si_signo;
> -	new.si_errno = from->si_errno;
> -	new.si_code  = from->si_code;
> +	to->si_signo = from->si_signo;
> +	to->si_errno = from->si_errno;
> +	to->si_code  = from->si_code;
>   	switch(siginfo_layout(from->si_signo, from->si_code)) {
>   	case SIL_KILL:
> -		new.si_pid = from->si_pid;
> -		new.si_uid = from->si_uid;
> +		to->si_pid = from->si_pid;
> +		to->si_uid = from->si_uid;
>   		break;
>   	case SIL_TIMER:
> -		new.si_tid     = from->si_tid;
> -		new.si_overrun = from->si_overrun;
> -		new.si_int     = from->si_int;
> +		to->si_tid     = from->si_tid;
> +		to->si_overrun = from->si_overrun;
> +		to->si_int     = from->si_int;
>   		break;
>   	case SIL_POLL:
> -		new.si_band = from->si_band;
> -		new.si_fd   = from->si_fd;
> +		to->si_band = from->si_band;
> +		to->si_fd   = from->si_fd;
>   		break;
>   	case SIL_FAULT:
> -		new.si_addr = ptr_to_compat(from->si_addr);
> +		to->si_addr = ptr_to_compat(from->si_addr);
>   #ifdef __ARCH_SI_TRAPNO
> -		new.si_trapno = from->si_trapno;
> +		to->si_trapno = from->si_trapno;
>   #endif
>   		break;
>   	case SIL_FAULT_MCEERR:
> -		new.si_addr = ptr_to_compat(from->si_addr);
> +		to->si_addr = ptr_to_compat(from->si_addr);
>   #ifdef __ARCH_SI_TRAPNO
> -		new.si_trapno = from->si_trapno;
> +		to->si_trapno = from->si_trapno;
>   #endif
> -		new.si_addr_lsb = from->si_addr_lsb;
> +		to->si_addr_lsb = from->si_addr_lsb;
>   		break;
>   	case SIL_FAULT_BNDERR:
> -		new.si_addr = ptr_to_compat(from->si_addr);
> +		to->si_addr = ptr_to_compat(from->si_addr);
>   #ifdef __ARCH_SI_TRAPNO
> -		new.si_trapno = from->si_trapno;
> +		to->si_trapno = from->si_trapno;
>   #endif
> -		new.si_lower = ptr_to_compat(from->si_lower);
> -		new.si_upper = ptr_to_compat(from->si_upper);
> +		to->si_lower = ptr_to_compat(from->si_lower);
> +		to->si_upper = ptr_to_compat(from->si_upper);
>   		break;
>   	case SIL_FAULT_PKUERR:
> -		new.si_addr = ptr_to_compat(from->si_addr);
> +		to->si_addr = ptr_to_compat(from->si_addr);
>   #ifdef __ARCH_SI_TRAPNO
> -		new.si_trapno = from->si_trapno;
> +		to->si_trapno = from->si_trapno;
>   #endif
> -		new.si_pkey = from->si_pkey;
> +		to->si_pkey = from->si_pkey;
>   		break;
>   	case SIL_CHLD:
> -		new.si_pid    = from->si_pid;
> -		new.si_uid    = from->si_uid;
> -		new.si_status = from->si_status;
> +		to->si_pid    = from->si_pid;
> +		to->si_uid    = from->si_uid;
> +		to->si_status = from->si_status;
> +		to->si_utime = from->si_utime;
> +		to->si_stime = from->si_stime;
>   #ifdef CONFIG_X86_X32_ABI
>   		if (x32_ABI) {
> -			new._sifields._sigchld_x32._utime = from->si_utime;
> -			new._sifields._sigchld_x32._stime = from->si_stime;
> +			to->_sifields._sigchld_x32._utime = from->si_utime;
> +			to->_sifields._sigchld_x32._stime = from->si_stime;
>   		} else
>   #endif
>   		{
> -			new.si_utime = from->si_utime;
> -			new.si_stime = from->si_stime;
>   		}
>   		break;
>   	case SIL_RT:
> -		new.si_pid = from->si_pid;
> -		new.si_uid = from->si_uid;
> -		new.si_int = from->si_int;
> +		to->si_pid = from->si_pid;
> +		to->si_uid = from->si_uid;
> +		to->si_int = from->si_int;
>   		break;
>   	case SIL_SYS:
> -		new.si_call_addr = ptr_to_compat(from->si_call_addr);
> -		new.si_syscall   = from->si_syscall;
> -		new.si_arch      = from->si_arch;
> +		to->si_call_addr = ptr_to_compat(from->si_call_addr);
> +		to->si_syscall   = from->si_syscall;
> +		to->si_arch      = from->si_arch;
>   		break;
>   	}
> +}
> +
> +int copy_siginfo_to_user32(struct compat_siginfo __user *to,
> +			   const struct kernel_siginfo *from)
> +#if defined(CONFIG_X86_X32_ABI) || defined(CONFIG_IA32_EMULATION)
> +{
> +	return __copy_siginfo_to_user32(to, from, in_x32_syscall());
> +}
> +int __copy_siginfo_to_user32(struct compat_siginfo __user *to,
> +			     const struct kernel_siginfo *from, bool x32_ABI)
> +#endif
> +{
> +	struct compat_siginfo new;
> +	copy_siginfo_to_external32(&new, from);
> +#ifdef CONFIG_X86_X32_ABI
> +	if (x32_ABI && from->si_signo == SIGCHLD) {
> +		new._sifields._sigchld_x32._utime = from->si_utime;
> +		new._sifields._sigchld_x32._stime = from->si_stime;
> +	}
> +#endif
>   
>   	if (copy_to_user(to, &new, sizeof(struct compat_siginfo)))
>   		return -EFAULT;
> 

^ permalink raw reply

* [PATCH] soc: fsl: guts: remove unneeded semicolon in fsl_soc_die_match()
From: Jason Yan @ 2020-04-18  8:18 UTC (permalink / raw)
  To: leoyang.li, tglx, linuxppc-dev, linux-arm-kernel, linux-kernel
  Cc: Hulk Robot, Jason Yan

Fix the following coccicheck warning:

drivers/soc/fsl/guts.c:120:2-3: Unneeded semicolon

Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: Jason Yan <yanaijie@huawei.com>
---
 drivers/soc/fsl/guts.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/soc/fsl/guts.c b/drivers/soc/fsl/guts.c
index 34810f9bb2ee..d5e9a5f2c087 100644
--- a/drivers/soc/fsl/guts.c
+++ b/drivers/soc/fsl/guts.c
@@ -117,7 +117,7 @@ static const struct fsl_soc_die_attr *fsl_soc_die_match(
 		if (matches->svr == (svr & matches->mask))
 			return matches;
 		matches++;
-	};
+	}
 	return NULL;
 }
 
-- 
2.21.1


^ permalink raw reply related

* Re: [PATCH] powerpc/pseries: Make vio and ibmebus initcalls pseries specific
From: Tyrel Datwyler @ 2020-04-18  1:04 UTC (permalink / raw)
  To: Oliver O'Halloran, linuxppc-dev
In-Reply-To: <20200417040749.25800-1-oohall@gmail.com>

On 4/16/20 9:07 PM, Oliver O'Halloran wrote:
> The vio and ibmebus buses are used for pseries specific paravirtualised
> devices and currently they're initialised by the generic initcall types.
> This is mostly fine, but it can result in some nuisance errors in dmesg
> when booting on PowerNV on some OSes, e.g.
> 
> [    2.984439] synth uevent: /devices/vio: failed to send uevent
> [    2.984442] vio vio: uevent: failed to send synthetic uevent
> [   17.968551] synth uevent: /devices/vio: failed to send uevent
> [   17.968554] vio vio: uevent: failed to send synthetic uevent
> 
> We don't see anything similar for the ibmebus because that depends on
> !CONFIG_LITTLE_ENDIAN.
> 
> This patch squashes those by switching to using machine_*_initcall() so the bus
> type is only registered when the kernel is running on a pseries machine.
> 
> Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
> ---

Reviewed-by: Tyrel Datwyler <tyreld@linux.ibm.com>

^ permalink raw reply

* Re: [RFC PATCH] powerpc/64/signal: balance return predictor stack in signal trampoline
From: Alan Modra @ 2020-04-17 23:40 UTC (permalink / raw)
  To: Nicholas Piggin; +Cc: linuxppc-dev, Anton Blanchard
In-Reply-To: <20200417091747.316707-1-npiggin@gmail.com>

On Fri, Apr 17, 2020 at 07:17:47PM +1000, Nicholas Piggin wrote:
> I don't know much about dwarf, gdb still seems to recognize the signal
> frame and unwind properly if I break inside a signal handler.

Yes, the dwarf unwind info still looks good.  The commented out dwarf
near the end of sigtramp.S should probably go.  At least if you really
can't take an async signal in the trampoline (a kernel question, not
anything to do with gcc support of async signals as the comment
wrongly says).  If you *can* take an async signal at some point past
the trampoline addi, then delete the comment and uncomment the code.
Note that the advance_loc there bitrotted ever since the nop was added
before the trampoline, so you'd need to change that to an advance_loc
that moves from .Lsigrt_start to immediately after the addi, ie. 0x42.

-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply

* Re: [PATCH] drivers: uio: new driver uio_fsl_85xx_cache_sram
From: Scott Wood @ 2020-04-17 23:02 UTC (permalink / raw)
  To: Wang Wenhu, gregkh, linux-kernel, linuxppc-dev; +Cc: kernel, robh
In-Reply-To: <20200417172130.14287-1-wenhu.wang@vivo.com>

On Fri, 2020-04-17 at 10:21 -0700, Wang Wenhu wrote:
> Implements a new uio driver for freescale 85xx platforms to access
> the Cache-Sram form user level. It is extremely helpful for the user
> space applications that require high performance memory accesses.
> 
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Christophe Leroy <christophe.leroy@c-s.fr>
> Cc: Scott Wood <oss@buserror.net>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: linuxppc-dev@lists.ozlabs.org
> Signed-off-by: Wang Wenhu <wenhu.wang@vivo.com>
> ---
>  drivers/uio/Kconfig                   |   8 +
>  drivers/uio/Makefile                  |   1 +
>  drivers/uio/uio_fsl_85xx_cache_sram.c | 407 ++++++++++++++++++++++++++
>  3 files changed, 416 insertions(+)
>  create mode 100644 drivers/uio/uio_fsl_85xx_cache_sram.c

NACK, we don't need two copies of this code in the kernel.  Please just wait a
bit and I'll send a patch to have the existing driver expose a dynamic
allocation interface to userspace.

-Scott



^ permalink raw reply

* Re: [PATCH v4,4/4] drivers: uio: new driver for fsl_85xx_cache_sram
From: Scott Wood @ 2020-04-17 22:50 UTC (permalink / raw)
  To: 王文虎
  Cc: Rob Herring, Greg KH, linux-kernel, kernel, linuxppc-dev
In-Reply-To: <APIAAAABCKquIIhDNOkcHqp9.3.1587132987353.Hmail.wenhu.wang@vivo.com>

On Fri, 2020-04-17 at 22:16 +0800, 王文虎 wrote:
> > On Fri, 2020-04-17 at 09:42 +0200, Greg KH wrote:>> On Thu, Apr 16, 2020
> > at 11:58:29PM -0500, Scott Wood wrote:
> > > > On Fri, 2020-04-17 at 10:31 +0800, 王文虎 wrote:
> > > > > Sounds it is. And does the modification below fit well?
> > > > > ---
> > > > > -static const struct of_device_id uio_mpc85xx_l2ctlr_of_match[] = {
> > > > > -       {       .compatible = "uio,mpc85xx-cache-sram", },
> > > > > -       {},
> > > > > +#ifdef CONFIG_OF
> > > > > +static struct of_device_id uio_fsl_85xx_cache_sram_of_match[] = {
> > > > > +       { /* This is filled with module_parm */ },
> > > > > +       { /* Sentinel */ },
> > > > >  };
> > > > > +MODULE_DEVICE_TABLE(of, uio_fsl_85xx_cache_sram_of_match);
> > > > > +module_param_string(of_id,
> > > > > uio_fsl_85xx_cache_sram_of_match[0].compatible,
> > > > > +                           sizeof(uio_fsl_85xx_cache_sram_of_match[
> > > > > 0].c
> > > > > ompa
> > > > > tible), 0);
> > > > > +MODULE_PARM_DESC(of_id, "platform device id to be handled by cache-
> > > > > sram-
> > > > > uio");
> > > > > +#endif
> > > > 
> > > > No.  The point is that you wouldn't be configuring this with the
> > > > device
> > > > tree
> > > > at all.
> > > 
> > > Wait, why not?  Don't force people to use module parameters, that is
> > > crazy.  DT describes the hardware involved, if someone wants to bind to
> > > a specific range of memory, as described by DT, why can't they do so?
> > 
> > Yes, DT describes the hardware, and as I've said a couple times already,
> > this
> > isn't hardware description.
> > 
> > I'm not forcing people to use module parameters.  That was a least-effort
> > suggestion to avoid abusing the DT.  I later said I'd try to come up with
> > a
> > patch that allocates regions dynamically (and most likely doesn't use UIO
> > at
> > all).
> > 
> > > I can understand not liking the name "uio" in a dt tree, but there's no
> > > reason that DT can not describe what a driver binds to here.
> > 
> > The DT already describes this hardware, and there is already code that
> > binds
> > to it.  This patch is trying to add a second node for it with
> > configuration.
> > 
> 
> Hi, Scott, Greg,
> Seems like no balance here. How about I implement a driver of uio including
> the l2ctrl and cache_sram related implementations?
> And this way, the driver would be a hardware level driver and targeted for
> uio.

No, duplicating the code makes no sense whatsoever.  Please just wait a bit
and I'll send a patch to have the existing driver expose a dynamic allocation
interface to userspace.

-Scott



^ permalink raw reply

* Re: remove set_fs calls from the exec and coredump code v2
From: Eric W. Biederman @ 2020-04-17 22:41 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Arnd Bergmann, linux-kernel, Alexander Viro, linux-fsdevel,
	Andrew Morton, linuxppc-dev, Jeremy Kerr
In-Reply-To: <20200414070142.288696-1-hch@lst.de>

Christoph Hellwig <hch@lst.de> writes:

> Hi all,
>
> this series gets rid of playing with the address limit in the exec and
> coredump code.  Most of this was fairly trivial, the biggest changes are
> those to the spufs coredump code.
>
> Changes since v1:
>  - properly spell NUL
>  - properly handle the compat siginfo case in ELF coredumps

Quick question is exec from a kernel thread within the scope of what you
are looking at?

There is a set_fs(USER_DS) in flush_old_exec whose sole purpose appears
to be to allow exec from kernel threads.  Where the kernel threads
run with set_fs(KERNEL_DS) until they call exec.

Eric

^ permalink raw reply

* [PATCH 2/2] signal: Remove the set_fs in binfmt_elf.c:fill_siginfo_note
From: Eric W. Biederman @ 2020-04-17 21:09 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Arnd Bergmann, x86, linux-kernel, Alexander Viro, linux-fsdevel,
	Andrew Morton, linuxppc-dev, Jeremy Kerr
In-Reply-To: <87pnc5akhk.fsf@x220.int.ebiederm.org>


The code in binfmt_elf.c is differnt from the rest of the code that
processes siginfo, as it sends siginfo from a kernel buffer to a file
rather than from kernel memory to userspace buffers.  To remove it's
use of set_fs the code needs some different siginfo helpers.

Add the helper copy_siginfo_to_external to copy from the kernel's
internal siginfo layout to a buffer in the siginfo layout that
userspace expects.

Modify fill_siginfo_note to use copy_siginfo_to_external instead of
set_fs and copy_siginfo_to_user.

Update compat_binfmt_elf.c to use the previously added
copy_siginfo_to_external32 to handle the compat case.

Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
 fs/binfmt_elf.c        | 5 +----
 fs/compat_binfmt_elf.c | 2 +-
 include/linux/signal.h | 7 +++++++
 3 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index 13f25e241ac4..a1f57e20c3cf 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -1556,10 +1556,7 @@ static void fill_auxv_note(struct memelfnote *note, struct mm_struct *mm)
 static void fill_siginfo_note(struct memelfnote *note, user_siginfo_t *csigdata,
 		const kernel_siginfo_t *siginfo)
 {
-	mm_segment_t old_fs = get_fs();
-	set_fs(KERNEL_DS);
-	copy_siginfo_to_user((user_siginfo_t __user *) csigdata, siginfo);
-	set_fs(old_fs);
+	copy_siginfo_to_external(csigdata, siginfo);
 	fill_note(note, "CORE", NT_SIGINFO, sizeof(*csigdata), csigdata);
 }
 
diff --git a/fs/compat_binfmt_elf.c b/fs/compat_binfmt_elf.c
index aaad4ca1217e..fa0e24e1b726 100644
--- a/fs/compat_binfmt_elf.c
+++ b/fs/compat_binfmt_elf.c
@@ -39,7 +39,7 @@
  */
 #define user_long_t		compat_long_t
 #define user_siginfo_t		compat_siginfo_t
-#define copy_siginfo_to_user	copy_siginfo_to_user32
+#define copy_siginfo_to_external	copy_siginfo_to_external32
 
 /*
  * The machine-dependent core note format types are defined in elfcore-compat.h,
diff --git a/include/linux/signal.h b/include/linux/signal.h
index 05bacd2ab135..c1796321cadb 100644
--- a/include/linux/signal.h
+++ b/include/linux/signal.h
@@ -24,6 +24,13 @@ static inline void clear_siginfo(kernel_siginfo_t *info)
 
 #define SI_EXPANSION_SIZE (sizeof(struct siginfo) - sizeof(struct kernel_siginfo))
 
+static inline void copy_siginfo_to_external(siginfo_t *to,
+					    const kernel_siginfo_t *from)
+{
+	memcpy(to, from, sizeof(*from));
+	memset(((char *)to) + sizeof(struct kernel_siginfo), 0, SI_EXPANSION_SIZE);
+}
+
 int copy_siginfo_to_user(siginfo_t __user *to, const kernel_siginfo_t *from);
 int copy_siginfo_from_user(kernel_siginfo_t *to, const siginfo_t __user *from);
 
-- 
2.25.0


^ permalink raw reply related

* [PATCH 1/2] signal: Factor copy_siginfo_to_external32 from copy_siginfo_to_user32
From: Eric W. Biederman @ 2020-04-17 21:09 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Arnd Bergmann, x86, linux-kernel, Alexander Viro, linux-fsdevel,
	Andrew Morton, linuxppc-dev, Jeremy Kerr
In-Reply-To: <87pnc5akhk.fsf@x220.int.ebiederm.org>


To remove the use of set_fs in the coredump code there needs to be a
way to convert a kernel siginfo to a userspace compat siginfo.

Call that function copy_siginfo_to_compat and factor it out of
copy_siginfo_to_user32.

The existence of x32 complicates this code.  On x32 SIGCHLD uses 64bit
times for utime and stime.  As only SIGCHLD is affected and SIGCHLD
never causes a coredump I have avoided handling that case.

Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
 include/linux/compat.h |   1 +
 kernel/signal.c        | 108 +++++++++++++++++++++++------------------
 2 files changed, 63 insertions(+), 46 deletions(-)

diff --git a/include/linux/compat.h b/include/linux/compat.h
index 0480ba4db592..4962b254e550 100644
--- a/include/linux/compat.h
+++ b/include/linux/compat.h
@@ -402,6 +402,7 @@ long compat_get_bitmap(unsigned long *mask, const compat_ulong_t __user *umask,
 		       unsigned long bitmap_size);
 long compat_put_bitmap(compat_ulong_t __user *umask, unsigned long *mask,
 		       unsigned long bitmap_size);
+void copy_siginfo_to_external32(struct compat_siginfo *to, const struct kernel_siginfo *from);
 int copy_siginfo_from_user32(kernel_siginfo_t *to, const struct compat_siginfo __user *from);
 int copy_siginfo_to_user32(struct compat_siginfo __user *to, const kernel_siginfo_t *from);
 int get_compat_sigevent(struct sigevent *event,
diff --git a/kernel/signal.c b/kernel/signal.c
index e58a6c619824..578f196898cb 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -3235,90 +3235,106 @@ int copy_siginfo_from_user(kernel_siginfo_t *to, const siginfo_t __user *from)
 }
 
 #ifdef CONFIG_COMPAT
-int copy_siginfo_to_user32(struct compat_siginfo __user *to,
-			   const struct kernel_siginfo *from)
-#if defined(CONFIG_X86_X32_ABI) || defined(CONFIG_IA32_EMULATION)
+void copy_siginfo_to_external32(struct compat_siginfo *to,
+				const struct kernel_siginfo *from)
 {
-	return __copy_siginfo_to_user32(to, from, in_x32_syscall());
-}
-int __copy_siginfo_to_user32(struct compat_siginfo __user *to,
-			     const struct kernel_siginfo *from, bool x32_ABI)
-#endif
-{
-	struct compat_siginfo new;
-	memset(&new, 0, sizeof(new));
+	/*
+	 * This function does not work properly for SIGCHLD on x32,
+	 * but it does not need to as SIGCHLD never causes a coredump.
+	 */
+	memset(to, 0, sizeof(*to));
 
-	new.si_signo = from->si_signo;
-	new.si_errno = from->si_errno;
-	new.si_code  = from->si_code;
+	to->si_signo = from->si_signo;
+	to->si_errno = from->si_errno;
+	to->si_code  = from->si_code;
 	switch(siginfo_layout(from->si_signo, from->si_code)) {
 	case SIL_KILL:
-		new.si_pid = from->si_pid;
-		new.si_uid = from->si_uid;
+		to->si_pid = from->si_pid;
+		to->si_uid = from->si_uid;
 		break;
 	case SIL_TIMER:
-		new.si_tid     = from->si_tid;
-		new.si_overrun = from->si_overrun;
-		new.si_int     = from->si_int;
+		to->si_tid     = from->si_tid;
+		to->si_overrun = from->si_overrun;
+		to->si_int     = from->si_int;
 		break;
 	case SIL_POLL:
-		new.si_band = from->si_band;
-		new.si_fd   = from->si_fd;
+		to->si_band = from->si_band;
+		to->si_fd   = from->si_fd;
 		break;
 	case SIL_FAULT:
-		new.si_addr = ptr_to_compat(from->si_addr);
+		to->si_addr = ptr_to_compat(from->si_addr);
 #ifdef __ARCH_SI_TRAPNO
-		new.si_trapno = from->si_trapno;
+		to->si_trapno = from->si_trapno;
 #endif
 		break;
 	case SIL_FAULT_MCEERR:
-		new.si_addr = ptr_to_compat(from->si_addr);
+		to->si_addr = ptr_to_compat(from->si_addr);
 #ifdef __ARCH_SI_TRAPNO
-		new.si_trapno = from->si_trapno;
+		to->si_trapno = from->si_trapno;
 #endif
-		new.si_addr_lsb = from->si_addr_lsb;
+		to->si_addr_lsb = from->si_addr_lsb;
 		break;
 	case SIL_FAULT_BNDERR:
-		new.si_addr = ptr_to_compat(from->si_addr);
+		to->si_addr = ptr_to_compat(from->si_addr);
 #ifdef __ARCH_SI_TRAPNO
-		new.si_trapno = from->si_trapno;
+		to->si_trapno = from->si_trapno;
 #endif
-		new.si_lower = ptr_to_compat(from->si_lower);
-		new.si_upper = ptr_to_compat(from->si_upper);
+		to->si_lower = ptr_to_compat(from->si_lower);
+		to->si_upper = ptr_to_compat(from->si_upper);
 		break;
 	case SIL_FAULT_PKUERR:
-		new.si_addr = ptr_to_compat(from->si_addr);
+		to->si_addr = ptr_to_compat(from->si_addr);
 #ifdef __ARCH_SI_TRAPNO
-		new.si_trapno = from->si_trapno;
+		to->si_trapno = from->si_trapno;
 #endif
-		new.si_pkey = from->si_pkey;
+		to->si_pkey = from->si_pkey;
 		break;
 	case SIL_CHLD:
-		new.si_pid    = from->si_pid;
-		new.si_uid    = from->si_uid;
-		new.si_status = from->si_status;
+		to->si_pid    = from->si_pid;
+		to->si_uid    = from->si_uid;
+		to->si_status = from->si_status;
+		to->si_utime = from->si_utime;
+		to->si_stime = from->si_stime;
 #ifdef CONFIG_X86_X32_ABI
 		if (x32_ABI) {
-			new._sifields._sigchld_x32._utime = from->si_utime;
-			new._sifields._sigchld_x32._stime = from->si_stime;
+			to->_sifields._sigchld_x32._utime = from->si_utime;
+			to->_sifields._sigchld_x32._stime = from->si_stime;
 		} else
 #endif
 		{
-			new.si_utime = from->si_utime;
-			new.si_stime = from->si_stime;
 		}
 		break;
 	case SIL_RT:
-		new.si_pid = from->si_pid;
-		new.si_uid = from->si_uid;
-		new.si_int = from->si_int;
+		to->si_pid = from->si_pid;
+		to->si_uid = from->si_uid;
+		to->si_int = from->si_int;
 		break;
 	case SIL_SYS:
-		new.si_call_addr = ptr_to_compat(from->si_call_addr);
-		new.si_syscall   = from->si_syscall;
-		new.si_arch      = from->si_arch;
+		to->si_call_addr = ptr_to_compat(from->si_call_addr);
+		to->si_syscall   = from->si_syscall;
+		to->si_arch      = from->si_arch;
 		break;
 	}
+}
+
+int copy_siginfo_to_user32(struct compat_siginfo __user *to,
+			   const struct kernel_siginfo *from)
+#if defined(CONFIG_X86_X32_ABI) || defined(CONFIG_IA32_EMULATION)
+{
+	return __copy_siginfo_to_user32(to, from, in_x32_syscall());
+}
+int __copy_siginfo_to_user32(struct compat_siginfo __user *to,
+			     const struct kernel_siginfo *from, bool x32_ABI)
+#endif
+{
+	struct compat_siginfo new;
+	copy_siginfo_to_external32(&new, from);
+#ifdef CONFIG_X86_X32_ABI
+	if (x32_ABI && from->si_signo == SIGCHLD) {
+		new._sifields._sigchld_x32._utime = from->si_utime;
+		new._sifields._sigchld_x32._stime = from->si_stime;
+	}
+#endif
 
 	if (copy_to_user(to, &new, sizeof(struct compat_siginfo)))
 		return -EFAULT;
-- 
2.25.0


^ permalink raw reply related

* Re: [PATCH 2/8] signal: clean up __copy_siginfo_to_user32
From: Eric W. Biederman @ 2020-04-17 21:08 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Arnd Bergmann, x86, linux-kernel, Alexander Viro, linux-fsdevel,
	Andrew Morton, linuxppc-dev, Jeremy Kerr
In-Reply-To: <20200414070142.288696-3-hch@lst.de>

Christoph Hellwig <hch@lst.de> writes:

> Instead of an architecture specific calling convention in common code
> just pass a flags argument with architecture specific values.

This bothers me because it makes all architectures pay for the sins of
x32.  Further it starts burying the details of the what is happening in
architecture specific helpers.  Hiding the fact that there is only
one niche architecture that does anything weird.

I am very sensitive to hiding away signal handling details right now
because way to much of the signal handling code got hidden in
architecture specific files and was quite buggy because as a result.

My general sense is putting all of the weird details up front and center
in kernel/signal.c is the only way for this code will be looked at
and successfully maintained.

How about these patches to solve set_fs with binfmt_elf instead:

Eric W. Biederman (2):
      signal: Factor copy_siginfo_to_external32 from copy_siginfo_to_user32
      signal: Remove the set_fs in binfmt_elf.c:fill_siginfo_note

 fs/binfmt_elf.c        |   5 +----
 fs/compat_binfmt_elf.c |   2 +-
 include/linux/compat.h |   1 +
 include/linux/signal.h |   7 +++++++
 kernel/signal.c        | 108 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++----------------------------------------------

Eric

^ permalink raw reply

* Re: [PATCH 4/8] binfmt_elf: open code copy_siginfo_to_user to kernelspace buffer
From: Arnd Bergmann @ 2020-04-17 20:06 UTC (permalink / raw)
  To: Eric W. Biederman
  Cc: the arch/x86 maintainers, linux-kernel@vger.kernel.org,
	Jeremy Kerr, Linux FS-devel Mailing List, Andrew Morton,
	linuxppc-dev, Christoph Hellwig, Alexander Viro
In-Reply-To: <87o8rqc7az.fsf@x220.int.ebiederm.org>

On Fri, Apr 17, 2020 at 8:13 PM Eric W. Biederman <ebiederm@xmission.com> wrote:
>
> Christoph Hellwig <hch@lst.de> writes:
>
> > On Wed, Apr 15, 2020 at 10:20:11AM +0200, Arnd Bergmann wrote:
> >> > I'd rather keep it out of this series and to
> >> > an interested party.  Then again x32 doesn't seem to have a whole lot
> >> > of interested parties..
> >>
> >> Fine with me. It's on my mental list of things that we want to kill off
> >> eventually as soon as the remaining users stop replying to questions
> >> about it.
> >>
> >> In fact I should really turn that into a properly maintained list in
> >> Documentation/... that contains any options that someone has
> >> asked about removing in the past, along with the reasons for keeping
> >> it around and a time at which we should ask about it again.
> >
> > To the newly added x86 maintainers:  Arnd brought up the point that
> > elf_core_dump writes the ABI siginfo format into the core dump. That
> > format differs for i386 vs x32.  Is there any good way to find out
> > which is the right format when are not in a syscall?
> >
> > As far a I can tell x32 vs i386 just seems to be based around what
> > syscall table was used for the current syscall, but core dumps aren't
> > always in syscall context.
>
> I don't think this matters.  The i386 and x32 signal structures
> only differ for SIGCHLD.  The SIGCHLD signal does cause coredumps.
> So as long as we get the 32bit vs 64bit distinct correct all should be
> well.

Ok, makes sense. Thanks for taking a look into this!

      Arnd

^ permalink raw reply

* [PATCH v3 0/4] Clean up hugetlb boot command line processing
From: Mike Kravetz @ 2020-04-17 18:50 UTC (permalink / raw)
  To: linux-mm, linux-kernel, linux-arm-kernel, linuxppc-dev,
	linux-riscv, linux-s390, sparclinux, linux-doc
  Cc: Dave Hansen, Heiko Carstens, Peter Xu, Paul Mackerras,
	Will Deacon, Mina Almasry, Jonathan Corbet, Christian Borntraeger,
	Ingo Molnar, Catalin Marinas, Longpeng, Albert Ou, Vasily Gorbik,
	Paul Walmsley, Thomas Gleixner, Nitesh Narayan Lal, Randy Dunlap,
	Palmer Dabbelt, Andrew Morton, David S . Miller, Mike Kravetz

v3 -
   Used weak attribute method of defining arch_hugetlb_valid_size.
     This eliminates changes to arch specific hugetlb.h files (Peter)
   Updated documentation (Peter, Randy)
   Fixed handling of implicitly specified gigantic page preallocation
     in existing code and removed documentation of such.  There is now
     no difference between handling of gigantic and non-gigantic pages.
     (Peter, Nitesh).
     This requires the most review as there is a small change to
     undocumented behavior.  See patch 4 commit message for details.
   Added Acks and Reviews (Mina, Peter)

v2 -
   Fix build errors with patch 1 (Will)
   Change arch_hugetlb_valid_size arg to unsigned long and remove
     irrelevant 'extern' keyword (Christophe)
   Documentation and other misc changes (Randy, Christophe, Mina)
   Do not process command line options if !hugepages_supported()
     (Dave, but it sounds like we may want to additional changes to
      hugepages_supported() for x86?  If that is needed I would prefer
      a separate patch.)

Longpeng(Mike) reported a weird message from hugetlb command line processing
and proposed a solution [1].  While the proposed patch does address the
specific issue, there are other related issues in command line processing.
As hugetlbfs evolved, updates to command line processing have been made to
meet immediate needs and not necessarily in a coordinated manner.  The result
is that some processing is done in arch specific code, some is done in arch
independent code and coordination is problematic.  Semantics can vary between
architectures.

The patch series does the following:
- Define arch specific arch_hugetlb_valid_size routine used to validate
  passed huge page sizes.
- Move hugepagesz= command line parsing out of arch specific code and into
  an arch independent routine.
- Clean up command line processing to follow desired semantics and
  document those semantics.

[1] https://lore.kernel.org/linux-mm/20200305033014.1152-1-longpeng2@huawei.com

Mike Kravetz (4):
  hugetlbfs: add arch_hugetlb_valid_size
  hugetlbfs: move hugepagesz= parsing to arch independent code
  hugetlbfs: remove hugetlb_add_hstate() warning for existing hstate
  hugetlbfs: clean up command line processing

 .../admin-guide/kernel-parameters.txt         |  40 ++--
 Documentation/admin-guide/mm/hugetlbpage.rst  |  35 ++++
 arch/arm64/mm/hugetlbpage.c                   |  30 +--
 arch/powerpc/mm/hugetlbpage.c                 |  30 +--
 arch/riscv/mm/hugetlbpage.c                   |  24 +--
 arch/s390/mm/hugetlbpage.c                    |  24 +--
 arch/sparc/mm/init_64.c                       |  43 +---
 arch/x86/mm/hugetlbpage.c                     |  23 +--
 include/linux/hugetlb.h                       |   2 +-
 mm/hugetlb.c                                  | 190 +++++++++++++++---
 10 files changed, 271 insertions(+), 170 deletions(-)

-- 
2.25.2


^ permalink raw reply

* [PATCH v3 4/4] hugetlbfs: clean up command line processing
From: Mike Kravetz @ 2020-04-17 18:50 UTC (permalink / raw)
  To: linux-mm, linux-kernel, linux-arm-kernel, linuxppc-dev,
	linux-riscv, linux-s390, sparclinux, linux-doc
  Cc: Dave Hansen, Heiko Carstens, Peter Xu, Paul Mackerras,
	Will Deacon, Mina Almasry, Jonathan Corbet, Christian Borntraeger,
	Ingo Molnar, Catalin Marinas, Longpeng, Albert Ou, Vasily Gorbik,
	Paul Walmsley, Thomas Gleixner, Nitesh Narayan Lal, Randy Dunlap,
	Palmer Dabbelt, Andrew Morton, David S . Miller, Mike Kravetz
In-Reply-To: <20200417185049.275845-1-mike.kravetz@oracle.com>

With all hugetlb page processing done in a single file clean up code.
- Make code match desired semantics
  - Update documentation with semantics
- Make all warnings and errors messages start with 'HugeTLB:'.
- Consistently name command line parsing routines.
- Check for hugepages_supported() before processing parameters.
- Add comments to code
  - Describe some of the subtle interactions
  - Describe semantics of command line arguments

This patch also fixes issues with implicitly setting the number of
gigantic huge pages to preallocate.  Previously on X86 command line,
        hugepages=2 default_hugepagesz=1G
would result in zero 1G pages being preallocated and,
        # grep HugePages_Total /proc/meminfo
        HugePages_Total:       0
        # sysctl -a | grep nr_hugepages
        vm.nr_hugepages = 2
        vm.nr_hugepages_mempolicy = 2
        # cat /proc/sys/vm/nr_hugepages
        2
After this patch 2 gigantic pages will be preallocated and all the
proc, sysfs, sysctl and meminfo files will accurately reflect this.

To address the issue with gigantic pages, a small change in behavior
was made to command line processing.  Previously the command line,
        hugepages=128 default_hugepagesz=2M hugepagesz=2M hugepages=256
would result in the allocation of 256 2M huge pages.  The value 128
would be ignored without any warning.  After this patch, 128 2M pages
will be allocated and a warning message will be displayed indicating
the value of 256 is ignored.  This change in behavior is required
because allocation of implicitly specified gigantic pages must be done
when the default_hugepagesz= is encountered for gigantic pages.
Previously the code waited until later in the boot process (hugetlb_init),
to allocate pages of default size.  However the bootmem allocator required
for gigantic allocations is not available at this time.

Signed-off-by: Mike Kravetz <mike.kravetz@oracle.com>
---
 .../admin-guide/kernel-parameters.txt         |  40 +++--
 Documentation/admin-guide/mm/hugetlbpage.rst  |  35 ++++
 mm/hugetlb.c                                  | 159 ++++++++++++++----
 3 files changed, 190 insertions(+), 44 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index f2a93c8679e8..8cd78cc87a1c 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -834,12 +834,15 @@
 			See also Documentation/networking/decnet.txt.
 
 	default_hugepagesz=
-			[same as hugepagesz=] The size of the default
-			HugeTLB page size. This is the size represented by
-			the legacy /proc/ hugepages APIs, used for SHM, and
-			default size when mounting hugetlbfs filesystems.
-			Defaults to the default architecture's huge page size
-			if not specified.
+			[HW] The size of the default HugeTLB page. This is
+			the size represented by the legacy /proc/ hugepages
+			APIs.  In addition, this is the default hugetlb size
+			used for shmget(), mmap() and mounting hugetlbfs
+			filesystems.  If not specified, defaults to the
+			architecture's default huge page size.  Huge page
+			sizes are architecture dependent.  See also
+			Documentation/admin-guide/mm/hugetlbpage.rst.
+			Format: size[KMG]
 
 	deferred_probe_timeout=
 			[KNL] Debugging option to set a timeout in seconds for
@@ -1479,13 +1482,24 @@
 			hugepages using the cma allocator. If enabled, the
 			boot-time allocation of gigantic hugepages is skipped.
 
-	hugepages=	[HW,X86-32,IA-64] HugeTLB pages to allocate at boot.
-	hugepagesz=	[HW,IA-64,PPC,X86-64] The size of the HugeTLB pages.
-			On x86-64 and powerpc, this option can be specified
-			multiple times interleaved with hugepages= to reserve
-			huge pages of different sizes. Valid pages sizes on
-			x86-64 are 2M (when the CPU supports "pse") and 1G
-			(when the CPU supports the "pdpe1gb" cpuinfo flag).
+	hugepages=	[HW] Number of HugeTLB pages to allocate at boot.
+			If this follows hugepagesz (below), it specifies
+			the number of pages of hugepagesz to be allocated.
+			If this is the first HugeTLB parameter on the command
+			line, it specifies the number of pages to allocate for
+			the default huge page size.  See also
+			Documentation/admin-guide/mm/hugetlbpage.rst.
+			Format: <integer>
+
+	hugepagesz=
+			[HW] The size of the HugeTLB pages.  This is used in
+			conjunction with hugepages (above) to allocate huge
+			pages of a specific size at boot.  The pair
+			hugepagesz=X hugepages=Y can be specified once for
+			each supported huge page size. Huge page sizes are
+			architecture dependent.  See also
+			Documentation/admin-guide/mm/hugetlbpage.rst.
+			Format: size[KMG]
 
 	hung_task_panic=
 			[KNL] Should the hung task detector generate panics.
diff --git a/Documentation/admin-guide/mm/hugetlbpage.rst b/Documentation/admin-guide/mm/hugetlbpage.rst
index 1cc0bc78d10e..5026e58826e2 100644
--- a/Documentation/admin-guide/mm/hugetlbpage.rst
+++ b/Documentation/admin-guide/mm/hugetlbpage.rst
@@ -100,6 +100,41 @@ with a huge page size selection parameter "hugepagesz=<size>".  <size> must
 be specified in bytes with optional scale suffix [kKmMgG].  The default huge
 page size may be selected with the "default_hugepagesz=<size>" boot parameter.
 
+Hugetlb boot command line parameter semantics
+hugepagesz - Specify a huge page size.  Used in conjunction with hugepages
+	parameter to preallocate a number of huge pages of the specified
+	size.  Hence, hugepagesz and hugepages are typically specified in
+	pairs such as:
+		hugepagesz=2M hugepages=512
+	hugepagesz can only be specified once on the command line for a
+	specific huge page size.  Valid huge page sizes are architecture
+	dependent.
+hugepages - Specify the number of huge pages to preallocate.  This typically
+	follows a valid hugepagesz or default_hugepagesz parameter.  However,
+	if hugepages is the first or only hugetlb command line parameter it
+	implicitly specifies the number of huge pages of default size to
+	allocate.  If the number of huge pages of default size is implicitly
+	specified, it can not be overwritten by a hugepagesz,hugepages
+	parameter pair for the default size.
+	For example, on an architecture with 2M default huge page size:
+		hugepages=256 hugepagesz=2M hugepages=512
+	will result in 256 2M huge pages being allocated and a warning message
+	indicating that the hugepages=512 parameter is ignored.  If a hugepages
+	parameter is preceded by an invalid hugepagesz parameter, it will
+	be ignored.
+default_hugepagesz - Specify the default huge page size.  This parameter can
+	only be specified once on the command line.  default_hugepagesz can
+	optionally be followed by the hugepages parameter to preallocate a
+	specific number of huge pages of default size.  The number of default
+	sized huge pages to preallocate can also be implicitly specified as
+	mentioned in the hugepages section above.  Therefore, on an
+	architecture with 2M default huge page size:
+		hugepages=256
+		default_hugepagesz=2M hugepages=256
+		hugepages=256 default_hugepagesz=2M
+	will all result in 256 2M huge pages being allocated.  Valid default
+	huge page size is architecture dependent.
+
 When multiple huge page sizes are supported, ``/proc/sys/vm/nr_hugepages``
 indicates the current number of pre-allocated huge pages of the default size.
 Thus, one can use the following command to dynamically allocate/deallocate
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 0e6eb755ae94..e272ebc2b5cb 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -59,8 +59,8 @@ __initdata LIST_HEAD(huge_boot_pages);
 /* for command line parsing */
 static struct hstate * __initdata parsed_hstate;
 static unsigned long __initdata default_hstate_max_huge_pages;
-static unsigned long __initdata default_hstate_size;
 static bool __initdata parsed_valid_hugepagesz = true;
+static bool __initdata parsed_default_hugepagesz;
 
 /*
  * Protects updates to hugepage_freelists, hugepage_activelist, nr_huge_pages,
@@ -3060,7 +3060,7 @@ static void __init hugetlb_sysfs_init(void)
 		err = hugetlb_sysfs_add_hstate(h, hugepages_kobj,
 					 hstate_kobjs, &hstate_attr_group);
 		if (err)
-			pr_err("Hugetlb: Unable to add hstate %s", h->name);
+			pr_err("HugeTLB: Unable to add hstate %s", h->name);
 	}
 }
 
@@ -3164,7 +3164,7 @@ static void hugetlb_register_node(struct node *node)
 						nhs->hstate_kobjs,
 						&per_node_hstate_attr_group);
 		if (err) {
-			pr_err("Hugetlb: Unable to add hstate %s for node %d\n",
+			pr_err("HugeTLB: Unable to add hstate %s for node %d\n",
 				h->name, node->dev.id);
 			hugetlb_unregister_node(node);
 			break;
@@ -3215,19 +3215,35 @@ static int __init hugetlb_init(void)
 	if (!hugepages_supported())
 		return 0;
 
-	if (!size_to_hstate(default_hstate_size)) {
-		if (default_hstate_size != 0) {
-			pr_err("HugeTLB: unsupported default_hugepagesz %lu. Reverting to %lu\n",
-			       default_hstate_size, HPAGE_SIZE);
+	/*
+	 * Make sure HPAGE_SIZE (HUGETLB_PAGE_ORDER) hstate exists.  Some
+	 * architectures depend on setup being done here.
+	 */
+	hugetlb_add_hstate(HUGETLB_PAGE_ORDER);
+	if (!parsed_default_hugepagesz) {
+		/*
+		 * If we did not parse a default huge page size, set
+		 * default_hstate_idx to HPAGE_SIZE hstate. And, if the
+		 * number of huge pages for this default size was implicitly
+		 * specified, set that here as well.
+		 * Note that the implicit setting will overwrite an explicit
+		 * setting.  A warning will be printed in this case.
+		 */
+		default_hstate_idx = hstate_index(size_to_hstate(HPAGE_SIZE));
+		if (default_hstate_max_huge_pages) {
+			if (default_hstate.max_huge_pages) {
+				char buf[32];
+
+				string_get_size(huge_page_size(&default_hstate),
+					1, STRING_UNITS_2, buf, 32);
+				pr_warn("HugeTLB: Ignoring hugepages=%lu associated with %s page size\n",
+					default_hstate.max_huge_pages, buf);
+				pr_warn("HugeTLB: Using hugepages=%lu for number of default huge pages\n",
+					default_hstate_max_huge_pages);
+			}
+			default_hstate.max_huge_pages =
+				default_hstate_max_huge_pages;
 		}
-
-		default_hstate_size = HPAGE_SIZE;
-		hugetlb_add_hstate(HUGETLB_PAGE_ORDER);
-	}
-	default_hstate_idx = hstate_index(size_to_hstate(default_hstate_size));
-	if (default_hstate_max_huge_pages) {
-		if (!default_hstate.max_huge_pages)
-			default_hstate.max_huge_pages = default_hstate_max_huge_pages;
 	}
 
 	hugetlb_cma_check();
@@ -3287,20 +3303,34 @@ void __init hugetlb_add_hstate(unsigned int order)
 	parsed_hstate = h;
 }
 
-static int __init hugetlb_nrpages_setup(char *s)
+/*
+ * hugepages command line processing
+ * hugepages normally follows a valid hugepagsz or default_hugepagsz
+ * specification.  If not, ignore the hugepages value.  hugepages can also
+ * be the first huge page command line  option in which case it implicitly
+ * specifies the number of huge pages for the default size.
+ */
+static int __init hugepages_setup(char *s)
 {
 	unsigned long *mhp;
 	static unsigned long *last_mhp;
 
+	if (!hugepages_supported()) {
+		pr_warn("HugeTLB: huge pages not supported, ignoring hugepages = %s\n", s);
+		return 0;
+	}
+
 	if (!parsed_valid_hugepagesz) {
-		pr_warn("hugepages = %s preceded by "
-			"an unsupported hugepagesz, ignoring\n", s);
+		pr_warn("HugeTLB: hugepages=%s does not follow a valid hugepagesz, ignoring\n", s);
 		parsed_valid_hugepagesz = true;
-		return 1;
+		return 0;
 	}
+
 	/*
-	 * !hugetlb_max_hstate means we haven't parsed a hugepagesz= parameter yet,
-	 * so this hugepages= parameter goes to the "default hstate".
+	 * !hugetlb_max_hstate means we haven't parsed a hugepagesz= parameter
+	 * yet, so this hugepages= parameter goes to the "default hstate".
+	 * Otherwise, it goes with the previously parsed hugepagesz or
+	 * default_hugepagesz.
 	 */
 	else if (!hugetlb_max_hstate)
 		mhp = &default_hstate_max_huge_pages;
@@ -3308,8 +3338,8 @@ static int __init hugetlb_nrpages_setup(char *s)
 		mhp = &parsed_hstate->max_huge_pages;
 
 	if (mhp == last_mhp) {
-		pr_warn("hugepages= specified twice without interleaving hugepagesz=, ignoring\n");
-		return 1;
+		pr_warn("HugeTLB: hugepages= specified twice without interleaving hugepagesz=, ignoring hugepages=%s\n", s);
+		return 0;
 	}
 
 	if (sscanf(s, "%lu", mhp) <= 0)
@@ -3327,42 +3357,109 @@ static int __init hugetlb_nrpages_setup(char *s)
 
 	return 1;
 }
-__setup("hugepages=", hugetlb_nrpages_setup);
+__setup("hugepages=", hugepages_setup);
 
+/*
+ * hugepagesz command line processing
+ * A specific huge page size can only be specified once with hugepagesz.
+ * hugepagesz is followed by hugepages on the command line.  The global
+ * variable 'parsed_valid_hugepagesz' is used to determine if prior
+ * hugepagesz argument was valid.
+ */
 static int __init hugepagesz_setup(char *s)
 {
 	unsigned long size;
+	struct hstate *h;
+
+	parsed_valid_hugepagesz = false;
+	if (!hugepages_supported()) {
+		pr_warn("HugeTLB: huge pages not supported, ignoring hugepagesz = %s\n", s);
+		return 0;
+	}
 
 	size = (unsigned long)memparse(s, NULL);
 
 	if (!arch_hugetlb_valid_size(size)) {
-		parsed_valid_hugepagesz = false;
-		pr_err("HugeTLB: unsupported hugepagesz %s\n", s);
+		pr_err("HugeTLB: unsupported hugepagesz=%s\n", s);
 		return 0;
 	}
 
-	if (size_to_hstate(size)) {
-		pr_warn("HugeTLB: hugepagesz %s specified twice, ignoring\n", s);
-		return 0;
+	h = size_to_hstate(size);
+	if (h) {
+		/*
+		 * hstate for this size already exists.  This is normally
+		 * an error, but is allowed if the existing hstate is the
+		 * default hstate.  More specifically, it is only allowed if
+		 * the number of huge pages for the default hstate was not
+		 * previously specified.
+		 */
+		if (!parsed_default_hugepagesz ||  h != &default_hstate ||
+		    default_hstate.max_huge_pages) {
+			pr_warn("HugeTLB: hugepagesz=%s specified twice, ignoring\n", s);
+			return 0;
+		}
+
+		/*
+		 * No need to call hugetlb_add_hstate() as hstate already
+		 * exists.  But, do set parsed_hstate so that a following
+		 * hugepages= parameter will be applied to this hstate.
+		 */
+		parsed_hstate = h;
+		parsed_valid_hugepagesz = true;
+		return 1;
 	}
 
 	hugetlb_add_hstate(ilog2(size) - PAGE_SHIFT);
+	parsed_valid_hugepagesz = true;
 	return 1;
 }
 __setup("hugepagesz=", hugepagesz_setup);
 
+/*
+ * default_hugepagesz command line input
+ * Only one instance of default_hugepagesz allowed on command line.
+ */
 static int __init default_hugepagesz_setup(char *s)
 {
 	unsigned long size;
 
+	parsed_valid_hugepagesz = false;
+	if (!hugepages_supported()) {
+		pr_warn("HugeTLB: huge pages not supported, ignoring default_hugepagesz = %s\n", s);
+		return 0;
+	}
+
+	if (parsed_default_hugepagesz) {
+		pr_err("HugeTLB: default_hugepagesz previously specified, ignoring %s\n", s);
+		return 0;
+	}
+
 	size = (unsigned long)memparse(s, NULL);
 
 	if (!arch_hugetlb_valid_size(size)) {
-		pr_err("HugeTLB: unsupported default_hugepagesz %s\n", s);
+		pr_err("HugeTLB: unsupported default_hugepagesz=%s\n", s);
 		return 0;
 	}
 
-	default_hstate_size = size;
+	hugetlb_add_hstate(ilog2(size) - PAGE_SHIFT);
+	parsed_valid_hugepagesz = true;
+	parsed_default_hugepagesz = true;
+	default_hstate_idx = hstate_index(size_to_hstate(size));
+
+	/*
+	 * The number of default huge pages (for this size) could have been
+	 * specified as the first hugetlb parameter: hugepages=X.  If so,
+	 * then default_hstate_max_huge_pages is set.  If the default huge
+	 * page size is gigantic (>= MAX_ORDER), then the pages must be
+	 * allocated here from bootmem allocator.
+	 */
+	if (default_hstate_max_huge_pages) {
+		default_hstate.max_huge_pages = default_hstate_max_huge_pages;
+		if (hstate_is_gigantic(&default_hstate))
+			hugetlb_hstate_alloc_pages(&default_hstate);
+		default_hstate_max_huge_pages = 0;
+	}
+
 	return 1;
 }
 __setup("default_hugepagesz=", default_hugepagesz_setup);
-- 
2.25.2


^ permalink raw reply related

* [PATCH v3 1/4] hugetlbfs: add arch_hugetlb_valid_size
From: Mike Kravetz @ 2020-04-17 18:50 UTC (permalink / raw)
  To: linux-mm, linux-kernel, linux-arm-kernel, linuxppc-dev,
	linux-riscv, linux-s390, sparclinux, linux-doc
  Cc: Dave Hansen, Heiko Carstens, Peter Xu, Paul Mackerras,
	Will Deacon, Mina Almasry, Jonathan Corbet, Christian Borntraeger,
	Ingo Molnar, Catalin Marinas, Longpeng, Albert Ou, Vasily Gorbik,
	Paul Walmsley, Thomas Gleixner, Nitesh Narayan Lal, Randy Dunlap,
	Palmer Dabbelt, Andrew Morton, David S . Miller, Mike Kravetz
In-Reply-To: <20200417185049.275845-1-mike.kravetz@oracle.com>

The architecture independent routine hugetlb_default_setup sets up
the default huge pages size.  It has no way to verify if the passed
value is valid, so it accepts it and attempts to validate at a later
time.  This requires undocumented cooperation between the arch specific
and arch independent code.

For architectures that support more than one huge page size, provide
a routine arch_hugetlb_valid_size to validate a huge page size.
hugetlb_default_setup can use this to validate passed values.

arch_hugetlb_valid_size will also be used in a subsequent patch to
move processing of the "hugepagesz=" in arch specific code to a common
routine in arch independent code.

Signed-off-by: Mike Kravetz <mike.kravetz@oracle.com>
---
 arch/arm64/mm/hugetlbpage.c   | 17 +++++++++++++----
 arch/powerpc/mm/hugetlbpage.c | 20 +++++++++++++-------
 arch/riscv/mm/hugetlbpage.c   | 26 +++++++++++++++++---------
 arch/s390/mm/hugetlbpage.c    | 16 ++++++++++++----
 arch/sparc/mm/init_64.c       | 24 ++++++++++++++++--------
 arch/x86/mm/hugetlbpage.c     | 17 +++++++++++++----
 include/linux/hugetlb.h       |  1 +
 mm/hugetlb.c                  | 21 ++++++++++++++++++---
 8 files changed, 103 insertions(+), 39 deletions(-)

diff --git a/arch/arm64/mm/hugetlbpage.c b/arch/arm64/mm/hugetlbpage.c
index bbeb6a5a6ba6..069b96ee2aec 100644
--- a/arch/arm64/mm/hugetlbpage.c
+++ b/arch/arm64/mm/hugetlbpage.c
@@ -462,17 +462,26 @@ static int __init hugetlbpage_init(void)
 }
 arch_initcall(hugetlbpage_init);
 
-static __init int setup_hugepagesz(char *opt)
+bool __init arch_hugetlb_valid_size(unsigned long size)
 {
-	unsigned long ps = memparse(opt, &opt);
-
-	switch (ps) {
+	switch (size) {
 #ifdef CONFIG_ARM64_4K_PAGES
 	case PUD_SIZE:
 #endif
 	case CONT_PMD_SIZE:
 	case PMD_SIZE:
 	case CONT_PTE_SIZE:
+		return true;
+	}
+
+	return false;
+}
+
+static __init int setup_hugepagesz(char *opt)
+{
+	unsigned long ps = memparse(opt, &opt);
+
+	if (arch_hugetlb_valid_size(ps)) {
 		add_huge_page_size(ps);
 		return 1;
 	}
diff --git a/arch/powerpc/mm/hugetlbpage.c b/arch/powerpc/mm/hugetlbpage.c
index 33b3461d91e8..de54d2a37830 100644
--- a/arch/powerpc/mm/hugetlbpage.c
+++ b/arch/powerpc/mm/hugetlbpage.c
@@ -558,7 +558,7 @@ unsigned long vma_mmu_pagesize(struct vm_area_struct *vma)
 	return vma_kernel_pagesize(vma);
 }
 
-static int __init add_huge_page_size(unsigned long long size)
+bool __init arch_hugetlb_valid_size(unsigned long size)
 {
 	int shift = __ffs(size);
 	int mmu_psize;
@@ -566,20 +566,26 @@ static int __init add_huge_page_size(unsigned long long size)
 	/* Check that it is a page size supported by the hardware and
 	 * that it fits within pagetable and slice limits. */
 	if (size <= PAGE_SIZE || !is_power_of_2(size))
-		return -EINVAL;
+		return false;
 
 	mmu_psize = check_and_get_huge_psize(shift);
 	if (mmu_psize < 0)
-		return -EINVAL;
+		return false;
 
 	BUG_ON(mmu_psize_defs[mmu_psize].shift != shift);
 
-	/* Return if huge page size has already been setup */
-	if (size_to_hstate(size))
-		return 0;
+	return true;
+}
 
-	hugetlb_add_hstate(shift - PAGE_SHIFT);
+static int __init add_huge_page_size(unsigned long long size)
+{
+	int shift = __ffs(size);
+
+	if (!arch_hugetlb_valid_size((unsigned long)size))
+		return -EINVAL;
 
+	if (!size_to_hstate(size))
+		hugetlb_add_hstate(shift - PAGE_SHIFT);
 	return 0;
 }
 
diff --git a/arch/riscv/mm/hugetlbpage.c b/arch/riscv/mm/hugetlbpage.c
index a6189ed36c5f..da1f516bc451 100644
--- a/arch/riscv/mm/hugetlbpage.c
+++ b/arch/riscv/mm/hugetlbpage.c
@@ -12,21 +12,29 @@ int pmd_huge(pmd_t pmd)
 	return pmd_leaf(pmd);
 }
 
+bool __init arch_hugetlb_valid_size(unsigned long size)
+{
+	if (size == HPAGE_SIZE)
+		return true;
+	else if (IS_ENABLED(CONFIG_64BIT) && size == PUD_SIZE)
+		return true;
+	else
+		return false;
+}
+
 static __init int setup_hugepagesz(char *opt)
 {
 	unsigned long ps = memparse(opt, &opt);
 
-	if (ps == HPAGE_SIZE) {
-		hugetlb_add_hstate(HPAGE_SHIFT - PAGE_SHIFT);
-	} else if (IS_ENABLED(CONFIG_64BIT) && ps == PUD_SIZE) {
-		hugetlb_add_hstate(PUD_SHIFT - PAGE_SHIFT);
-	} else {
-		hugetlb_bad_size();
-		pr_err("hugepagesz: Unsupported page size %lu M\n", ps >> 20);
-		return 0;
+	if (arch_hugetlb_valid_size(ps)) {
+		hugetlb_add_hstate(ilog2(ps) - PAGE_SHIFT);
+		return 1;
 	}
 
-	return 1;
+	hugetlb_bad_size();
+	pr_err("hugepagesz: Unsupported page size %lu M\n", ps >> 20);
+	return 0;
+
 }
 __setup("hugepagesz=", setup_hugepagesz);
 
diff --git a/arch/s390/mm/hugetlbpage.c b/arch/s390/mm/hugetlbpage.c
index f01daddcbc5e..ac25b207624c 100644
--- a/arch/s390/mm/hugetlbpage.c
+++ b/arch/s390/mm/hugetlbpage.c
@@ -251,16 +251,24 @@ follow_huge_pud(struct mm_struct *mm, unsigned long address,
 	return pud_page(*pud) + ((address & ~PUD_MASK) >> PAGE_SHIFT);
 }
 
+bool __init arch_hugetlb_valid_size(unsigned long size)
+{
+	if (MACHINE_HAS_EDAT1 && size == PMD_SIZE)
+		return true;
+	else if (MACHINE_HAS_EDAT2 && size == PUD_SIZE)
+		return true;
+	else
+		return false;
+}
+
 static __init int setup_hugepagesz(char *opt)
 {
 	unsigned long size;
 	char *string = opt;
 
 	size = memparse(opt, &opt);
-	if (MACHINE_HAS_EDAT1 && size == PMD_SIZE) {
-		hugetlb_add_hstate(PMD_SHIFT - PAGE_SHIFT);
-	} else if (MACHINE_HAS_EDAT2 && size == PUD_SIZE) {
-		hugetlb_add_hstate(PUD_SHIFT - PAGE_SHIFT);
+	if (arch_hugetlb_valid_size(size)) {
+		hugetlb_add_hstate(ilog2(size) - PAGE_SHIFT);
 	} else {
 		hugetlb_bad_size();
 		pr_err("hugepagesz= specifies an unsupported page size %s\n",
diff --git a/arch/sparc/mm/init_64.c b/arch/sparc/mm/init_64.c
index 1cf0d666dea3..2bfe8e22b706 100644
--- a/arch/sparc/mm/init_64.c
+++ b/arch/sparc/mm/init_64.c
@@ -360,16 +360,11 @@ static void __init pud_huge_patch(void)
 	__asm__ __volatile__("flush %0" : : "r" (addr));
 }
 
-static int __init setup_hugepagesz(char *string)
+bool __init arch_hugetlb_valid_size(unsigned long size)
 {
-	unsigned long long hugepage_size;
-	unsigned int hugepage_shift;
+	unsigned int hugepage_shift = ilog2(size);
 	unsigned short hv_pgsz_idx;
 	unsigned int hv_pgsz_mask;
-	int rc = 0;
-
-	hugepage_size = memparse(string, &string);
-	hugepage_shift = ilog2(hugepage_size);
 
 	switch (hugepage_shift) {
 	case HPAGE_16GB_SHIFT:
@@ -397,7 +392,20 @@ static int __init setup_hugepagesz(char *string)
 		hv_pgsz_mask = 0;
 	}
 
-	if ((hv_pgsz_mask & cpu_pgsz_mask) == 0U) {
+	if ((hv_pgsz_mask & cpu_pgsz_mask) == 0U)
+		return false;
+
+	return true;
+}
+
+static int __init setup_hugepagesz(char *string)
+{
+	unsigned long long hugepage_size;
+	int rc = 0;
+
+	hugepage_size = memparse(string, &string);
+
+	if (!arch_hugetlb_valid_size((unsigned long)hugepage_size)) {
 		hugetlb_bad_size();
 		pr_err("hugepagesz=%llu not supported by MMU.\n",
 			hugepage_size);
diff --git a/arch/x86/mm/hugetlbpage.c b/arch/x86/mm/hugetlbpage.c
index 5bfd5aef5378..1c4372bfe782 100644
--- a/arch/x86/mm/hugetlbpage.c
+++ b/arch/x86/mm/hugetlbpage.c
@@ -181,13 +181,22 @@ hugetlb_get_unmapped_area(struct file *file, unsigned long addr,
 #endif /* CONFIG_HUGETLB_PAGE */
 
 #ifdef CONFIG_X86_64
+bool __init arch_hugetlb_valid_size(unsigned long size)
+{
+	if (size == PMD_SIZE)
+		return true;
+	else if (size == PUD_SIZE && boot_cpu_has(X86_FEATURE_GBPAGES))
+		return true;
+	else
+		return false;
+}
+
 static __init int setup_hugepagesz(char *opt)
 {
 	unsigned long ps = memparse(opt, &opt);
-	if (ps == PMD_SIZE) {
-		hugetlb_add_hstate(PMD_SHIFT - PAGE_SHIFT);
-	} else if (ps == PUD_SIZE && boot_cpu_has(X86_FEATURE_GBPAGES)) {
-		hugetlb_add_hstate(PUD_SHIFT - PAGE_SHIFT);
+
+	if (arch_hugetlb_valid_size(ps)) {
+		hugetlb_add_hstate(ilog2(ps) - PAGE_SHIFT);
 	} else {
 		hugetlb_bad_size();
 		printk(KERN_ERR "hugepagesz: Unsupported page size %lu M\n",
diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
index 43a1cef8f0f1..2eb15f5ab01e 100644
--- a/include/linux/hugetlb.h
+++ b/include/linux/hugetlb.h
@@ -521,6 +521,7 @@ int __init alloc_bootmem_huge_page(struct hstate *h);
 
 void __init hugetlb_bad_size(void);
 void __init hugetlb_add_hstate(unsigned order);
+bool __init arch_hugetlb_valid_size(unsigned long size);
 struct hstate *size_to_hstate(unsigned long size);
 
 #ifndef HUGE_MAX_HSTATE
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index cd459155d28a..871ac1681f37 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -3256,6 +3256,12 @@ static int __init hugetlb_init(void)
 }
 subsys_initcall(hugetlb_init);
 
+/* Overwritten by architectures with more huge page sizes */
+bool __init __attribute((weak)) arch_hugetlb_valid_size(unsigned long size)
+{
+	return size == HPAGE_SIZE;
+}
+
 /* Should be called on processing a hugepagesz=... option */
 void __init hugetlb_bad_size(void)
 {
@@ -3331,12 +3337,21 @@ static int __init hugetlb_nrpages_setup(char *s)
 }
 __setup("hugepages=", hugetlb_nrpages_setup);
 
-static int __init hugetlb_default_setup(char *s)
+static int __init default_hugepagesz_setup(char *s)
 {
-	default_hstate_size = memparse(s, &s);
+	unsigned long size;
+
+	size = (unsigned long)memparse(s, NULL);
+
+	if (!arch_hugetlb_valid_size(size)) {
+		pr_err("HugeTLB: unsupported default_hugepagesz %s\n", s);
+		return 0;
+	}
+
+	default_hstate_size = size;
 	return 1;
 }
-__setup("default_hugepagesz=", hugetlb_default_setup);
+__setup("default_hugepagesz=", default_hugepagesz_setup);
 
 static unsigned int cpuset_mems_nr(unsigned int *array)
 {
-- 
2.25.2


^ permalink raw reply related

* [PATCH v3 3/4] hugetlbfs: remove hugetlb_add_hstate() warning for existing hstate
From: Mike Kravetz @ 2020-04-17 18:50 UTC (permalink / raw)
  To: linux-mm, linux-kernel, linux-arm-kernel, linuxppc-dev,
	linux-riscv, linux-s390, sparclinux, linux-doc
  Cc: Dave Hansen, Heiko Carstens, Peter Xu, Paul Mackerras,
	Will Deacon, Mina Almasry, Jonathan Corbet, Christian Borntraeger,
	Ingo Molnar, Catalin Marinas, Longpeng, Albert Ou, Vasily Gorbik,
	Paul Walmsley, Thomas Gleixner, Nitesh Narayan Lal, Randy Dunlap,
	Palmer Dabbelt, Andrew Morton, David S . Miller, Mike Kravetz
In-Reply-To: <20200417185049.275845-1-mike.kravetz@oracle.com>

The routine hugetlb_add_hstate prints a warning if the hstate already
exists.  This was originally done as part of kernel command line
parsing.  If 'hugepagesz=' was specified more than once, the warning
	pr_warn("hugepagesz= specified twice, ignoring\n");
would be printed.

Some architectures want to enable all huge page sizes.  They would
call hugetlb_add_hstate for all supported sizes.  However, this was
done after command line processing and as a result hstates could have
already been created for some sizes.  To make sure no warning were
printed, there would often be code like:
	if (!size_to_hstate(size)
		hugetlb_add_hstate(ilog2(size) - PAGE_SHIFT)

The only time we want to print the warning is as the result of command
line processing.  So, remove the warning from hugetlb_add_hstate and
add it to the single arch independent routine processing "hugepagesz=".
After this, calls to size_to_hstate() in arch specific code can be
removed and hugetlb_add_hstate can be called without worrying about
warning messages.

Signed-off-by: Mike Kravetz <mike.kravetz@oracle.com>
Acked-by: Mina Almasry <almasrymina@google.com>
---
 arch/arm64/mm/hugetlbpage.c   | 16 ++++------------
 arch/powerpc/mm/hugetlbpage.c |  3 +--
 arch/riscv/mm/hugetlbpage.c   |  2 +-
 arch/sparc/mm/init_64.c       | 19 ++++---------------
 arch/x86/mm/hugetlbpage.c     |  2 +-
 mm/hugetlb.c                  |  9 ++++++---
 6 files changed, 17 insertions(+), 34 deletions(-)

diff --git a/arch/arm64/mm/hugetlbpage.c b/arch/arm64/mm/hugetlbpage.c
index f706b821aba6..21fa98b51e00 100644
--- a/arch/arm64/mm/hugetlbpage.c
+++ b/arch/arm64/mm/hugetlbpage.c
@@ -441,22 +441,14 @@ void huge_ptep_clear_flush(struct vm_area_struct *vma,
 	clear_flush(vma->vm_mm, addr, ptep, pgsize, ncontig);
 }
 
-static void __init add_huge_page_size(unsigned long size)
-{
-	if (size_to_hstate(size))
-		return;
-
-	hugetlb_add_hstate(ilog2(size) - PAGE_SHIFT);
-}
-
 static int __init hugetlbpage_init(void)
 {
 #ifdef CONFIG_ARM64_4K_PAGES
-	add_huge_page_size(PUD_SIZE);
+	hugetlb_add_hstate(PUD_SHIFT - PAGE_SHIFT);
 #endif
-	add_huge_page_size(CONT_PMD_SIZE);
-	add_huge_page_size(PMD_SIZE);
-	add_huge_page_size(CONT_PTE_SIZE);
+	hugetlb_add_hstate(CONT_PMD_SHIFT - PAGE_SHIFT);
+	hugetlb_add_hstate(PMD_SHIFT - PAGE_SHIFT);
+	hugetlb_add_hstate(CONT_PTE_SHIFT - PAGE_SHIFT);
 
 	return 0;
 }
diff --git a/arch/powerpc/mm/hugetlbpage.c b/arch/powerpc/mm/hugetlbpage.c
index 2c3fa0a7787b..4d5ed1093615 100644
--- a/arch/powerpc/mm/hugetlbpage.c
+++ b/arch/powerpc/mm/hugetlbpage.c
@@ -584,8 +584,7 @@ static int __init add_huge_page_size(unsigned long long size)
 	if (!arch_hugetlb_valid_size((unsigned long)size))
 		return -EINVAL;
 
-	if (!size_to_hstate(size))
-		hugetlb_add_hstate(shift - PAGE_SHIFT);
+	hugetlb_add_hstate(shift - PAGE_SHIFT);
 	return 0;
 }
 
diff --git a/arch/riscv/mm/hugetlbpage.c b/arch/riscv/mm/hugetlbpage.c
index 4e5d7e9f0eef..932dadfdca54 100644
--- a/arch/riscv/mm/hugetlbpage.c
+++ b/arch/riscv/mm/hugetlbpage.c
@@ -26,7 +26,7 @@ bool __init arch_hugetlb_valid_size(unsigned long size)
 static __init int gigantic_pages_init(void)
 {
 	/* With CONTIG_ALLOC, we can allocate gigantic pages at runtime */
-	if (IS_ENABLED(CONFIG_64BIT) && !size_to_hstate(1UL << PUD_SHIFT))
+	if (IS_ENABLED(CONFIG_64BIT))
 		hugetlb_add_hstate(PUD_SHIFT - PAGE_SHIFT);
 	return 0;
 }
diff --git a/arch/sparc/mm/init_64.c b/arch/sparc/mm/init_64.c
index 4618f96fd30f..ae819a16d07a 100644
--- a/arch/sparc/mm/init_64.c
+++ b/arch/sparc/mm/init_64.c
@@ -325,23 +325,12 @@ static void __update_mmu_tsb_insert(struct mm_struct *mm, unsigned long tsb_inde
 }
 
 #ifdef CONFIG_HUGETLB_PAGE
-static void __init add_huge_page_size(unsigned long size)
-{
-	unsigned int order;
-
-	if (size_to_hstate(size))
-		return;
-
-	order = ilog2(size) - PAGE_SHIFT;
-	hugetlb_add_hstate(order);
-}
-
 static int __init hugetlbpage_init(void)
 {
-	add_huge_page_size(1UL << HPAGE_64K_SHIFT);
-	add_huge_page_size(1UL << HPAGE_SHIFT);
-	add_huge_page_size(1UL << HPAGE_256MB_SHIFT);
-	add_huge_page_size(1UL << HPAGE_2GB_SHIFT);
+	hugetlb_add_hstate(HPAGE_64K_SHIFT - PAGE_SHIFT);
+	hugetlb_add_hstate(HPAGE_SHIFT - PAGE_SHIFT);
+	hugetlb_add_hstate(HPAGE_256MB_SHIFT - PAGE_SHIFT);
+	hugetlb_add_hstate(HPAGE_2GB_SHIFT - PAGE_SHIFT);
 
 	return 0;
 }
diff --git a/arch/x86/mm/hugetlbpage.c b/arch/x86/mm/hugetlbpage.c
index 937d640a89e3..cf5781142716 100644
--- a/arch/x86/mm/hugetlbpage.c
+++ b/arch/x86/mm/hugetlbpage.c
@@ -195,7 +195,7 @@ bool __init arch_hugetlb_valid_size(unsigned long size)
 static __init int gigantic_pages_init(void)
 {
 	/* With compaction or CMA we can allocate gigantic pages at runtime */
-	if (boot_cpu_has(X86_FEATURE_GBPAGES) && !size_to_hstate(1UL << PUD_SHIFT))
+	if (boot_cpu_has(X86_FEATURE_GBPAGES))
 		hugetlb_add_hstate(PUD_SHIFT - PAGE_SHIFT);
 	return 0;
 }
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index b2d276408cec..0e6eb755ae94 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -3222,8 +3222,7 @@ static int __init hugetlb_init(void)
 		}
 
 		default_hstate_size = HPAGE_SIZE;
-		if (!size_to_hstate(default_hstate_size))
-			hugetlb_add_hstate(HUGETLB_PAGE_ORDER);
+		hugetlb_add_hstate(HUGETLB_PAGE_ORDER);
 	}
 	default_hstate_idx = hstate_index(size_to_hstate(default_hstate_size));
 	if (default_hstate_max_huge_pages) {
@@ -3268,7 +3267,6 @@ void __init hugetlb_add_hstate(unsigned int order)
 	unsigned long i;
 
 	if (size_to_hstate(PAGE_SIZE << order)) {
-		pr_warn("hugepagesz= specified twice, ignoring\n");
 		return;
 	}
 	BUG_ON(hugetlb_max_hstate >= HUGE_MAX_HSTATE);
@@ -3343,6 +3341,11 @@ static int __init hugepagesz_setup(char *s)
 		return 0;
 	}
 
+	if (size_to_hstate(size)) {
+		pr_warn("HugeTLB: hugepagesz %s specified twice, ignoring\n", s);
+		return 0;
+	}
+
 	hugetlb_add_hstate(ilog2(size) - PAGE_SHIFT);
 	return 1;
 }
-- 
2.25.2


^ permalink raw reply related

* [PATCH v3 2/4] hugetlbfs: move hugepagesz= parsing to arch independent code
From: Mike Kravetz @ 2020-04-17 18:50 UTC (permalink / raw)
  To: linux-mm, linux-kernel, linux-arm-kernel, linuxppc-dev,
	linux-riscv, linux-s390, sparclinux, linux-doc
  Cc: Dave Hansen, Heiko Carstens, Peter Xu, Paul Mackerras,
	Will Deacon, Mina Almasry, Jonathan Corbet, Christian Borntraeger,
	Ingo Molnar, Catalin Marinas, Longpeng, Albert Ou, Vasily Gorbik,
	Paul Walmsley, Thomas Gleixner, Nitesh Narayan Lal, Randy Dunlap,
	Palmer Dabbelt, Andrew Morton, David S . Miller, Mike Kravetz
In-Reply-To: <20200417185049.275845-1-mike.kravetz@oracle.com>

Now that architectures provide arch_hugetlb_valid_size(), parsing
of "hugepagesz=" can be done in architecture independent code.
Create a single routine to handle hugepagesz= parsing and remove
all arch specific routines.  We can also remove the interface
hugetlb_bad_size() as this is no longer used outside arch independent
code.

This also provides consistent behavior of hugetlbfs command line
options.  The hugepagesz= option should only be specified once for
a specific size, but some architectures allow multiple instances.
This appears to be more of an oversight when code was added by some
architectures to set up ALL huge pages sizes.

Signed-off-by: Mike Kravetz <mike.kravetz@oracle.com>
Acked-by: Mina Almasry <almasrymina@google.com>
Reviewed-by: Peter Xu <peterx@redhat.com>
---
 arch/arm64/mm/hugetlbpage.c   | 15 ---------------
 arch/powerpc/mm/hugetlbpage.c | 15 ---------------
 arch/riscv/mm/hugetlbpage.c   | 16 ----------------
 arch/s390/mm/hugetlbpage.c    | 18 ------------------
 arch/sparc/mm/init_64.c       | 22 ----------------------
 arch/x86/mm/hugetlbpage.c     | 16 ----------------
 include/linux/hugetlb.h       |  1 -
 mm/hugetlb.c                  | 23 +++++++++++++++++------
 8 files changed, 17 insertions(+), 109 deletions(-)

diff --git a/arch/arm64/mm/hugetlbpage.c b/arch/arm64/mm/hugetlbpage.c
index 069b96ee2aec..f706b821aba6 100644
--- a/arch/arm64/mm/hugetlbpage.c
+++ b/arch/arm64/mm/hugetlbpage.c
@@ -476,18 +476,3 @@ bool __init arch_hugetlb_valid_size(unsigned long size)
 
 	return false;
 }
-
-static __init int setup_hugepagesz(char *opt)
-{
-	unsigned long ps = memparse(opt, &opt);
-
-	if (arch_hugetlb_valid_size(ps)) {
-		add_huge_page_size(ps);
-		return 1;
-	}
-
-	hugetlb_bad_size();
-	pr_err("hugepagesz: Unsupported page size %lu K\n", ps >> 10);
-	return 0;
-}
-__setup("hugepagesz=", setup_hugepagesz);
diff --git a/arch/powerpc/mm/hugetlbpage.c b/arch/powerpc/mm/hugetlbpage.c
index de54d2a37830..2c3fa0a7787b 100644
--- a/arch/powerpc/mm/hugetlbpage.c
+++ b/arch/powerpc/mm/hugetlbpage.c
@@ -589,21 +589,6 @@ static int __init add_huge_page_size(unsigned long long size)
 	return 0;
 }
 
-static int __init hugepage_setup_sz(char *str)
-{
-	unsigned long long size;
-
-	size = memparse(str, &str);
-
-	if (add_huge_page_size(size) != 0) {
-		hugetlb_bad_size();
-		pr_err("Invalid huge page size specified(%llu)\n", size);
-	}
-
-	return 1;
-}
-__setup("hugepagesz=", hugepage_setup_sz);
-
 static int __init hugetlbpage_init(void)
 {
 	bool configured = false;
diff --git a/arch/riscv/mm/hugetlbpage.c b/arch/riscv/mm/hugetlbpage.c
index da1f516bc451..4e5d7e9f0eef 100644
--- a/arch/riscv/mm/hugetlbpage.c
+++ b/arch/riscv/mm/hugetlbpage.c
@@ -22,22 +22,6 @@ bool __init arch_hugetlb_valid_size(unsigned long size)
 		return false;
 }
 
-static __init int setup_hugepagesz(char *opt)
-{
-	unsigned long ps = memparse(opt, &opt);
-
-	if (arch_hugetlb_valid_size(ps)) {
-		hugetlb_add_hstate(ilog2(ps) - PAGE_SHIFT);
-		return 1;
-	}
-
-	hugetlb_bad_size();
-	pr_err("hugepagesz: Unsupported page size %lu M\n", ps >> 20);
-	return 0;
-
-}
-__setup("hugepagesz=", setup_hugepagesz);
-
 #ifdef CONFIG_CONTIG_ALLOC
 static __init int gigantic_pages_init(void)
 {
diff --git a/arch/s390/mm/hugetlbpage.c b/arch/s390/mm/hugetlbpage.c
index ac25b207624c..242dfc0d462d 100644
--- a/arch/s390/mm/hugetlbpage.c
+++ b/arch/s390/mm/hugetlbpage.c
@@ -261,24 +261,6 @@ bool __init arch_hugetlb_valid_size(unsigned long size)
 		return false;
 }
 
-static __init int setup_hugepagesz(char *opt)
-{
-	unsigned long size;
-	char *string = opt;
-
-	size = memparse(opt, &opt);
-	if (arch_hugetlb_valid_size(size)) {
-		hugetlb_add_hstate(ilog2(size) - PAGE_SHIFT);
-	} else {
-		hugetlb_bad_size();
-		pr_err("hugepagesz= specifies an unsupported page size %s\n",
-			string);
-		return 0;
-	}
-	return 1;
-}
-__setup("hugepagesz=", setup_hugepagesz);
-
 static unsigned long hugetlb_get_unmapped_area_bottomup(struct file *file,
 		unsigned long addr, unsigned long len,
 		unsigned long pgoff, unsigned long flags)
diff --git a/arch/sparc/mm/init_64.c b/arch/sparc/mm/init_64.c
index 2bfe8e22b706..4618f96fd30f 100644
--- a/arch/sparc/mm/init_64.c
+++ b/arch/sparc/mm/init_64.c
@@ -397,28 +397,6 @@ bool __init arch_hugetlb_valid_size(unsigned long size)
 
 	return true;
 }
-
-static int __init setup_hugepagesz(char *string)
-{
-	unsigned long long hugepage_size;
-	int rc = 0;
-
-	hugepage_size = memparse(string, &string);
-
-	if (!arch_hugetlb_valid_size((unsigned long)hugepage_size)) {
-		hugetlb_bad_size();
-		pr_err("hugepagesz=%llu not supported by MMU.\n",
-			hugepage_size);
-		goto out;
-	}
-
-	add_huge_page_size(hugepage_size);
-	rc = 1;
-
-out:
-	return rc;
-}
-__setup("hugepagesz=", setup_hugepagesz);
 #endif	/* CONFIG_HUGETLB_PAGE */
 
 void update_mmu_cache(struct vm_area_struct *vma, unsigned long address, pte_t *ptep)
diff --git a/arch/x86/mm/hugetlbpage.c b/arch/x86/mm/hugetlbpage.c
index 1c4372bfe782..937d640a89e3 100644
--- a/arch/x86/mm/hugetlbpage.c
+++ b/arch/x86/mm/hugetlbpage.c
@@ -191,22 +191,6 @@ bool __init arch_hugetlb_valid_size(unsigned long size)
 		return false;
 }
 
-static __init int setup_hugepagesz(char *opt)
-{
-	unsigned long ps = memparse(opt, &opt);
-
-	if (arch_hugetlb_valid_size(ps)) {
-		hugetlb_add_hstate(ilog2(ps) - PAGE_SHIFT);
-	} else {
-		hugetlb_bad_size();
-		printk(KERN_ERR "hugepagesz: Unsupported page size %lu M\n",
-			ps >> 20);
-		return 0;
-	}
-	return 1;
-}
-__setup("hugepagesz=", setup_hugepagesz);
-
 #ifdef CONFIG_CONTIG_ALLOC
 static __init int gigantic_pages_init(void)
 {
diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
index 2eb15f5ab01e..0c13706054ef 100644
--- a/include/linux/hugetlb.h
+++ b/include/linux/hugetlb.h
@@ -519,7 +519,6 @@ int huge_add_to_page_cache(struct page *page, struct address_space *mapping,
 int __init __alloc_bootmem_huge_page(struct hstate *h);
 int __init alloc_bootmem_huge_page(struct hstate *h);
 
-void __init hugetlb_bad_size(void);
 void __init hugetlb_add_hstate(unsigned order);
 bool __init arch_hugetlb_valid_size(unsigned long size);
 struct hstate *size_to_hstate(unsigned long size);
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 871ac1681f37..b2d276408cec 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -3262,12 +3262,6 @@ bool __init __attribute((weak)) arch_hugetlb_valid_size(unsigned long size)
 	return size == HPAGE_SIZE;
 }
 
-/* Should be called on processing a hugepagesz=... option */
-void __init hugetlb_bad_size(void)
-{
-	parsed_valid_hugepagesz = false;
-}
-
 void __init hugetlb_add_hstate(unsigned int order)
 {
 	struct hstate *h;
@@ -3337,6 +3331,23 @@ static int __init hugetlb_nrpages_setup(char *s)
 }
 __setup("hugepages=", hugetlb_nrpages_setup);
 
+static int __init hugepagesz_setup(char *s)
+{
+	unsigned long size;
+
+	size = (unsigned long)memparse(s, NULL);
+
+	if (!arch_hugetlb_valid_size(size)) {
+		parsed_valid_hugepagesz = false;
+		pr_err("HugeTLB: unsupported hugepagesz %s\n", s);
+		return 0;
+	}
+
+	hugetlb_add_hstate(ilog2(size) - PAGE_SHIFT);
+	return 1;
+}
+__setup("hugepagesz=", hugepagesz_setup);
+
 static int __init default_hugepagesz_setup(char *s)
 {
 	unsigned long size;
-- 
2.25.2


^ permalink raw reply related

* Re: [PATCH 4/8] binfmt_elf: open code copy_siginfo_to_user to kernelspace buffer
From: Eric W. Biederman @ 2020-04-17 18:10 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Arnd Bergmann, x86, linux-kernel@vger.kernel.org, Alexander Viro,
	Linux FS-devel Mailing List, Andrew Morton, linuxppc-dev,
	Jeremy Kerr
In-Reply-To: <20200417132714.GA6401@lst.de>

Christoph Hellwig <hch@lst.de> writes:

> On Wed, Apr 15, 2020 at 10:20:11AM +0200, Arnd Bergmann wrote:
>> > I'd rather keep it out of this series and to
>> > an interested party.  Then again x32 doesn't seem to have a whole lot
>> > of interested parties..
>> 
>> Fine with me. It's on my mental list of things that we want to kill off
>> eventually as soon as the remaining users stop replying to questions
>> about it.
>> 
>> In fact I should really turn that into a properly maintained list in
>> Documentation/... that contains any options that someone has
>> asked about removing in the past, along with the reasons for keeping
>> it around and a time at which we should ask about it again.
>
> To the newly added x86 maintainers:  Arnd brought up the point that
> elf_core_dump writes the ABI siginfo format into the core dump. That
> format differs for i386 vs x32.  Is there any good way to find out
> which is the right format when are not in a syscall?
>
> As far a I can tell x32 vs i386 just seems to be based around what
> syscall table was used for the current syscall, but core dumps aren't
> always in syscall context.

I don't think this matters.  The i386 and x32 signal structures
only differ for SIGCHLD.  The SIGCHLD signal does cause coredumps.
So as long as we get the 32bit vs 64bit distinct correct all should be
well.

Eric




^ permalink raw reply

* Re: [PATCH] drivers: uio: new driver uio_fsl_85xx_cache_sram
From: Randy Dunlap @ 2020-04-17 17:39 UTC (permalink / raw)
  To: Wang Wenhu, gregkh, linux-kernel, linuxppc-dev; +Cc: oss, robh, kernel
In-Reply-To: <20200417172130.14287-1-wenhu.wang@vivo.com>

On 4/17/20 10:21 AM, Wang Wenhu wrote:
> diff --git a/drivers/uio/Kconfig b/drivers/uio/Kconfig
> index 202ee81cfc2b..f6e6ec0089c0 100644
> --- a/drivers/uio/Kconfig
> +++ b/drivers/uio/Kconfig
> @@ -105,6 +105,14 @@ config UIO_NETX
>  	  To compile this driver as a module, choose M here; the module
>  	  will be called uio_netx.
>  
> +config UIO_FSL_85XX_CACHE_SRAM
> +	tristate "Freescale MPC85xx Cache-Sram driver"
> +	depends on FSL_SOC_BOOKE && PPC32 && !FSL_85XX_CACHE_SRAM
> +	help
> +	  Generic driver for accessing the Cache-Sram form user level. This

	                                              from

and SRAM would be better than Sram IMO. (2 places)

> +	  is extremely helpful for some user-space applications that require
> +	  high performance memory accesses.
> +
>  config UIO_FSL_ELBC_GPCM
>  	tristate "eLBC/GPCM driver"
>  	depends on FSL_LBC

thanks.
-- 
~Randy


^ permalink raw reply

* [PATCH] drivers: uio: new driver uio_fsl_85xx_cache_sram
From: Wang Wenhu @ 2020-04-17 17:21 UTC (permalink / raw)
  To: gregkh, linux-kernel, linuxppc-dev; +Cc: robh, oss, kernel, Wang Wenhu

Implements a new uio driver for freescale 85xx platforms to access
the Cache-Sram form user level. It is extremely helpful for the user
space applications that require high performance memory accesses.

Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Christophe Leroy <christophe.leroy@c-s.fr>
Cc: Scott Wood <oss@buserror.net>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: Wang Wenhu <wenhu.wang@vivo.com>
---
 drivers/uio/Kconfig                   |   8 +
 drivers/uio/Makefile                  |   1 +
 drivers/uio/uio_fsl_85xx_cache_sram.c | 407 ++++++++++++++++++++++++++
 3 files changed, 416 insertions(+)
 create mode 100644 drivers/uio/uio_fsl_85xx_cache_sram.c

diff --git a/drivers/uio/Kconfig b/drivers/uio/Kconfig
index 202ee81cfc2b..f6e6ec0089c0 100644
--- a/drivers/uio/Kconfig
+++ b/drivers/uio/Kconfig
@@ -105,6 +105,14 @@ config UIO_NETX
 	  To compile this driver as a module, choose M here; the module
 	  will be called uio_netx.
 
+config UIO_FSL_85XX_CACHE_SRAM
+	tristate "Freescale MPC85xx Cache-Sram driver"
+	depends on FSL_SOC_BOOKE && PPC32 && !FSL_85XX_CACHE_SRAM
+	help
+	  Generic driver for accessing the Cache-Sram form user level. This
+	  is extremely helpful for some user-space applications that require
+	  high performance memory accesses.
+
 config UIO_FSL_ELBC_GPCM
 	tristate "eLBC/GPCM driver"
 	depends on FSL_LBC
diff --git a/drivers/uio/Makefile b/drivers/uio/Makefile
index c285dd2a4539..be2056cffc21 100644
--- a/drivers/uio/Makefile
+++ b/drivers/uio/Makefile
@@ -10,4 +10,5 @@ obj-$(CONFIG_UIO_NETX)	+= uio_netx.o
 obj-$(CONFIG_UIO_PRUSS)         += uio_pruss.o
 obj-$(CONFIG_UIO_MF624)         += uio_mf624.o
 obj-$(CONFIG_UIO_FSL_ELBC_GPCM)	+= uio_fsl_elbc_gpcm.o
+obj-$(CONFIG_UIO_FSL_85XX_CACHE_SRAM)	+= uio_fsl_85xx_cache_sram.o
 obj-$(CONFIG_UIO_HV_GENERIC)	+= uio_hv_generic.o
diff --git a/drivers/uio/uio_fsl_85xx_cache_sram.c b/drivers/uio/uio_fsl_85xx_cache_sram.c
new file mode 100644
index 000000000000..312ca38e93e1
--- /dev/null
+++ b/drivers/uio/uio_fsl_85xx_cache_sram.c
@@ -0,0 +1,407 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2020 Vivo Communication Technology Co. Ltd.
+ * Copyright (C) 2020 Wang Wenhu <wenhu.wang@vivo.com>
+ * All rights reserved.
+ */
+
+#include <linux/platform_device.h>
+#include <linux/uio_driver.h>
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/of_address.h>
+
+#define DRIVER_NAME	"uio_fsl_85xx_cache_sram"
+#define UIO_INFO_VER	"devicetree,pseudo"
+
+#define MAX_SRAM_UIO_INFOS	5
+
+#define L2CR_L2FI		0x40000000	/* L2 flash invalidate */
+#define L2CR_SRAM_FULL		0x00010000	/* L2SRAM full size */
+#define L2CR_SRAM_HALF		0x00020000	/* L2SRAM half size */
+#define L2CR_SRAM_QUART		0x00040000	/* L2SRAM one quarter size */
+#define L2CR_SRAM_EIGHTH	0x00060000	/* L2SRAM one eighth size */
+
+#define L2SRAM_BAR_MSK_LO18	0xFFFFC000	/* Lower 18 bits */
+#define L2SRAM_BARE_MSK_HI4	0x0000000F	/* Upper 4 bits */
+
+enum cache_sram_lock_ways {
+	LOCK_WAYS_ZERO,
+	LOCK_WAYS_EIGHTH,
+	LOCK_WAYS_TWO_EIGHTH,
+	LOCK_WAYS_HALF = 4,
+	LOCK_WAYS_FULL = 8,
+};
+
+struct mpc85xx_l2ctlr {
+	u32	ctl;		/* 0x000 - L2 control */
+	u8	res1[0xC];
+	u32	ewar0;		/* 0x010 - External write address 0 */
+	u32	ewarea0;	/* 0x014 - External write address extended 0 */
+	u32	ewcr0;		/* 0x018 - External write ctrl */
+	u8	res2[4];
+	u32	ewar1;		/* 0x020 - External write address 1 */
+	u32	ewarea1;	/* 0x024 - External write address extended 1 */
+	u32	ewcr1;		/* 0x028 - External write ctrl 1 */
+	u8	res3[4];
+	u32	ewar2;		/* 0x030 - External write address 2 */
+	u32	ewarea2;	/* 0x034 - External write address extended 2 */
+	u32	ewcr2;		/* 0x038 - External write ctrl 2 */
+	u8	res4[4];
+	u32	ewar3;		/* 0x040 - External write address 3 */
+	u32	ewarea3;	/* 0x044 - External write address extended 3 */
+	u32	ewcr3;		/* 0x048 - External write ctrl 3 */
+	u8	res5[0xB4];
+	u32	srbar0;		/* 0x100 - SRAM base address 0 */
+	u32	srbarea0;	/* 0x104 - SRAM base addr reg ext address 0 */
+	u32	srbar1;		/* 0x108 - SRAM base address 1 */
+	u32	srbarea1;	/* 0x10C - SRAM base addr reg ext address 1 */
+	u8	res6[0xCF0];
+	u32	errinjhi;	/* 0xE00 - Error injection mask high */
+	u32	errinjlo;	/* 0xE04 - Error injection mask low */
+	u32	errinjctl;	/* 0xE08 - Error injection tag/ecc control */
+	u8	res7[0x14];
+	u32	captdatahi;	/* 0xE20 - Error data high capture */
+	u32	captdatalo;	/* 0xE24 - Error data low capture */
+	u32	captecc;	/* 0xE28 - Error syndrome */
+	u8	res8[0x14];
+	u32	errdet;		/* 0xE40 - Error detect */
+	u32	errdis;		/* 0xE44 - Error disable */
+	u32	errinten;	/* 0xE48 - Error interrupt enable */
+	u32	errattr;	/* 0xE4c - Error attribute capture */
+	u32	erradrrl;	/* 0xE50 - Error address capture low */
+	u32	erradrrh;	/* 0xE54 - Error address capture high */
+	u32	errctl;		/* 0xE58 - Error control */
+	u8	res9[0x1A4];
+};
+
+/**
+ * struct uio_cache_sram - controller for cache-sram and uio devices
+ *
+ * @base_phys:	physical address of cache-sram
+ * @base_virt:	mapped virtual address of cache-sram
+ * @size:	size of the sram could be used by user
+ * @alloced:	size of the sram allocated while initiating uio_infos
+ * @l2cache_size: total size of the cache-sram
+ * @l2ctlr:	address of the l2-controller
+ * @info_count:	count of the uio devices(info) for the cache-sram
+ * @uio_infos:	address array of the uio devices(info)
+ */
+struct uio_cache_sram {
+	phys_addr_t base_phys;
+	void *base_virt;
+	unsigned int size;
+	unsigned int alloced;
+
+	unsigned int l2cache_size;
+
+	struct mpc85xx_l2ctlr __iomem *l2ctlr;
+
+	unsigned int info_count;
+	struct uio_info *uio_infos[MAX_SRAM_UIO_INFOS];
+};
+
+static int of_init_cache_sram(struct device_node *node,
+			      struct uio_cache_sram *cache_sram)
+{
+	const __be32 *cell;
+
+	if (of_property_read_u32(node, "cache-size",
+				&cache_sram->l2cache_size)) {
+		pr_err("%pOF: missing cache-size property\n", node);
+		return -EINVAL;
+	}
+
+	cell = of_get_property(node, "sram-range", NULL);
+	if (!cell) {
+		pr_err("%pOF: missing sram-range property\n", node);
+		return -EINVAL;
+	}
+
+	cache_sram->base_phys = of_read_number(cell, of_n_addr_cells(node));
+	cache_sram->size = of_read_number(cell + of_n_addr_cells(node),
+					  of_n_size_cells(node));
+
+	return 0;
+}
+
+static void l2ctrl_init(struct uio_cache_sram *cache_sram)
+{
+	struct mpc85xx_l2ctlr *l2ctlr = cache_sram->l2ctlr;
+
+	/* Write bits[0-17] to srbar0 */
+	out_be32(&l2ctlr->srbar0,
+		 lower_32_bits(cache_sram->base_phys) & L2SRAM_BAR_MSK_LO18);
+
+	/* Write bits[18-21] to srbare0 */
+#ifdef CONFIG_PHYS_64BIT
+	out_be32(&l2ctlr->srbarea0,
+		 upper_32_bits(cache_sram->base_phys) & L2SRAM_BARE_MSK_HI4);
+#endif
+
+	clrsetbits_be32(&l2ctlr->ctl, L2CR_L2E, L2CR_L2FI);
+
+	switch (LOCK_WAYS_FULL * cache_sram->size / cache_sram->l2cache_size) {
+	case LOCK_WAYS_EIGHTH:
+		setbits32(&l2ctlr->ctl,
+			  L2CR_L2E | L2CR_L2FI | L2CR_SRAM_EIGHTH);
+		break;
+
+	case LOCK_WAYS_TWO_EIGHTH:
+		setbits32(&l2ctlr->ctl,
+			  L2CR_L2E | L2CR_L2FI | L2CR_SRAM_QUART);
+		break;
+
+	case LOCK_WAYS_HALF:
+		setbits32(&l2ctlr->ctl,
+			  L2CR_L2E | L2CR_L2FI | L2CR_SRAM_HALF);
+		break;
+
+	case LOCK_WAYS_FULL:
+	default:
+		setbits32(&l2ctlr->ctl,
+			  L2CR_L2E | L2CR_L2FI | L2CR_SRAM_FULL);
+		break;
+	}
+	eieio();
+}
+
+static int uio_cache_sram_init(struct platform_device *pdev,
+			       struct uio_cache_sram *cache_sram)
+{
+	struct device_node *node = pdev->dev.of_node;
+	unsigned int rem;
+	unsigned char ways;
+	int ret;
+
+	ret = of_init_cache_sram(node, cache_sram);
+	if (ret)
+		return ret;
+
+	rem = cache_sram->l2cache_size % cache_sram->size;
+	ways = LOCK_WAYS_FULL * cache_sram->size / cache_sram->l2cache_size;
+	if (rem || (ways & (ways - 1))) {
+		dev_err(&pdev->dev, "Illegal cache-size in command line\n");
+		return -EINVAL;
+	}
+
+	cache_sram->l2ctlr = of_iomap(pdev->dev.of_node, 0);
+	if (!cache_sram->l2ctlr) {
+		dev_err(&pdev->dev, "error can't map l2-controller\n");
+		return -EINVAL;
+	}
+
+	l2ctrl_init(cache_sram);
+
+	if (!request_mem_region(cache_sram->base_phys, cache_sram->size,
+				"fsl_85xx_cache_sram")) {
+		dev_err(&pdev->dev, "%pOF: request memory failed\n",
+				pdev->dev.of_node);
+		ret = -ENXIO;
+		goto out_unmap;
+	}
+
+	cache_sram->base_virt = ioremap_coherent(cache_sram->base_phys,
+						 cache_sram->size);
+	if (!cache_sram->base_virt) {
+		dev_err(&pdev->dev, "%pOF: ioremap_coherent failed\n",
+			pdev->dev.of_node);
+		ret = -ENOMEM;
+		goto out_release;
+	}
+
+	return 0;
+out_release:
+	release_mem_region(cache_sram->base_phys, cache_sram->size);
+out_unmap:
+	iounmap(cache_sram->l2ctlr);
+	return ret;
+}
+
+static int uio_cache_sram_destroy(struct uio_cache_sram *cache_sram)
+{
+	iounmap(cache_sram->l2ctlr);
+	iounmap(cache_sram->base_virt);
+	release_mem_region(cache_sram->base_phys, cache_sram->size);
+
+	return 0;
+}
+
+static void uio_info_free_internal(struct uio_info *info)
+{
+	int i;
+
+	for (i = 0; i < MAX_UIO_MAPS; i++) {
+		struct uio_mem *uiomem = &info->mem[i];
+
+		if (uiomem->internal_addr)
+			memset(uiomem, 0, sizeof(*uiomem));
+	}
+}
+
+void uio_infos_unregister(struct uio_cache_sram *cache_sram)
+{
+	int i;
+
+	for (i = 0; i < cache_sram->info_count; i++) {
+		uio_unregister_device(cache_sram->uio_infos[i]);
+		uio_info_free_internal(cache_sram->uio_infos[i]);
+		cache_sram->uio_infos[i] = NULL;
+	}
+
+	cache_sram->info_count = 0;
+}
+
+static int uio_fsl_85xx_cache_sram_probe(struct platform_device *pdev)
+{
+	struct device_node *parent = pdev->dev.of_node;
+	struct device_node *node = NULL;
+	struct uio_cache_sram *cache_sram;
+	struct uio_info *info;
+	struct uio_mem *uiomem;
+	const unsigned int *p;
+	struct property *prop;
+	const char *dt_name;
+	u32 size;
+	int ret;
+
+	cache_sram = devm_kzalloc(&pdev->dev, sizeof(*cache_sram), GFP_KERNEL);
+	if (!cache_sram)
+		return -ENOMEM;
+
+	ret = uio_cache_sram_init(pdev, cache_sram);
+	if (ret)
+		return ret;
+
+	for_each_child_of_node(parent, node) {
+		char buf[24];
+		int map_idx = 0;
+
+		/* alloc uio_info for one uio device */
+		info = devm_kzalloc(&pdev->dev, sizeof(*info), GFP_KERNEL);
+		if (!info)
+			return -ENOMEM;
+
+		/* get optional uio name */
+		if (of_property_read_string(parent, "uio_name", &dt_name)) {
+			sprintf(buf, "uio-sram%d", cache_sram->info_count);
+			dt_name = buf;
+		}
+
+		info->name = devm_kstrdup(&pdev->dev, dt_name, GFP_KERNEL);
+		if (!info->name)
+			return -ENOMEM;
+
+		of_property_for_each_u32(node, "uiomaps", prop, p, size) {
+			char name[10];
+
+			/* size should not be less than 2 */
+			if (size < 2) {
+				pr_err("size %x less than 2\n", size);
+				return -EINVAL;
+			}
+
+			/* size should be 2^n aligned */
+			if (size != roundup_pow_of_two(size)) {
+				pr_err("size %x is not 2^n algiend\n", size);
+				return -EINVAL;
+			}
+
+			if (cache_sram->alloced + size > cache_sram->size) {
+				pr_err("size %x too big\n", size);
+				return -EINVAL;
+			}
+
+			uiomem = &info->mem[map_idx];
+			uiomem->addr = cache_sram->base_phys +
+				       cache_sram->alloced;
+			uiomem->internal_addr = cache_sram->base_virt +
+						cache_sram->alloced;
+			uiomem->size = size;
+			uiomem->memtype = UIO_MEM_PHYS;
+
+			cache_sram->alloced += size;
+
+			sprintf(name, "mem%d", map_idx);
+			uiomem->name = devm_kstrdup(&pdev->dev, name,
+						    GFP_KERNEL);
+
+			map_idx++;
+			if (map_idx >= MAX_UIO_MAPS) {
+				dev_warn(&pdev->dev, "more than %d uio-maps for device.\n",
+					 MAX_UIO_MAPS);
+				break;
+			}
+		}
+
+		if (map_idx == 0) {
+			dev_err(&pdev->dev, "error no valid uio-map configuration found\n");
+			ret = -EINVAL;
+			goto err_out;
+		}
+
+		info->version = UIO_INFO_VER;
+
+		/* register uio device */
+		if (uio_register_device(&pdev->dev, info)) {
+			dev_err(&pdev->dev, "error uio,cache-sram registration failed\n");
+			ret = -ENODEV;
+			goto err_out;
+		}
+
+		cache_sram->uio_infos[cache_sram->info_count] = info;
+		cache_sram->info_count++;
+
+		if (cache_sram->info_count >= MAX_SRAM_UIO_INFOS) {
+			dev_warn(&pdev->dev, "more than %d uio_info devices.\n",
+				 MAX_SRAM_UIO_INFOS);
+			break;
+		}
+	}
+
+	platform_set_drvdata(pdev, cache_sram);
+
+	return 0;
+err_out:
+	uio_infos_unregister(cache_sram);
+	return ret;
+}
+
+static int uio_fsl_85xx_cache_sram_remove(struct platform_device *pdev)
+{
+	struct uio_cache_sram *cache_sram = platform_get_drvdata(pdev);
+
+	uio_infos_unregister(cache_sram);
+
+	uio_cache_sram_destroy(cache_sram);
+
+	return 0;
+}
+
+#ifdef CONFIG_OF
+static struct of_device_id uio_fsl_85xx_cache_sram_of_match[] = {
+	{ /* This is filled with module_parm */ },
+	{ /* Sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, uio_fsl_85xx_cache_sram_of_match);
+
+module_param_string(of_id, uio_fsl_85xx_cache_sram_of_match[0].compatible,
+		    sizeof(uio_fsl_85xx_cache_sram_of_match[0].compatible), 0);
+MODULE_PARM_DESC(of_id, "platform device id to be handled by cache-sram-uio");
+#endif
+
+static struct platform_driver uio_fsl_85xx_cache_sram = {
+	.probe = uio_fsl_85xx_cache_sram_probe,
+	.remove = uio_fsl_85xx_cache_sram_remove,
+	.driver = {
+		.name = DRIVER_NAME,
+		.of_match_table	= of_match_ptr(uio_fsl_85xx_cache_sram_of_match),
+	},
+};
+
+module_platform_driver(uio_fsl_85xx_cache_sram);
+
+MODULE_AUTHOR("Wang Wenhu <wenhu.wang@vivo.com>");
+MODULE_DESCRIPTION("Freescale MPC85xx Cache-Sram UIO Platform Driver");
+MODULE_ALIAS("platform:" DRIVER_NAME);
+MODULE_LICENSE("GPL v2");
-- 
2.17.1


^ permalink raw reply related

* Re: ppc64 early slub caches have zero random value
From: Vlastimil Babka @ 2020-04-17 17:19 UTC (permalink / raw)
  To: Michal Suchánek, linuxppc-dev, linux-mm; +Cc: Kees Cook
In-Reply-To: <20200417165304.GF25468@kitsune.suse.cz>

On 4/17/20 6:53 PM, Michal Suchánek wrote:
> Hello,

Hi, thanks for reproducing on latest upstream!

> instrumenting the kernel with the following patch
> 
> ---
>  mm/slub.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/mm/slub.c b/mm/slub.c
> index d6787bbe0248..d40995d5f8ff 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -3633,6 +3633,7 @@ static int kmem_cache_open(struct kmem_cache *s, slab_flags_t flags)
>  	s->flags = kmem_cache_flags(s->size, flags, s->name, s->ctor);
>  #ifdef CONFIG_SLAB_FREELIST_HARDENED
>  	s->random = get_random_long();
> +	pr_notice("Creating cache %s with s->random=%ld\n", s->name, s->random);
>  #endif
>  
>  	if (!calculate_sizes(s, -1))
> 
> I get:
> 
> [    0.000000] random: get_random_u64 called from kmem_cache_open+0x3c/0x5b0
with crng_init=0
> [    0.000000] Creating cache kmem_cache_node with s->random=0
> [    0.000000] Creating cache kmem_cache with s->random=0
> [    0.000000] Creating cache kmalloc-8 with s->random=0
> [    0.000000] Creating cache kmalloc-16 with s->random=0
> [    0.000000] Creating cache kmalloc-32 with s->random=0
> [    0.000000] Creating cache kmalloc-64 with s->random=0
> [    0.000000] Creating cache kmalloc-96 with s->random=0
> [    0.000000] Creating cache kmalloc-128 with s->random=0
> [    0.000000] Creating cache kmalloc-192 with s->random=-682532147323126958
> 
> The earliest caches created invariably end up with s->random of zero.

It seems that reliably it's the first 8 calls get_random_u64(), which sounds
more like some off-by-X bug than a genuine lack entropy that would become fixed
in the meanwhile?

> This is a problem for crash which does not recognize these as randomized
> and fails to read them. While this can be addressed in crash is it
> intended to create caches with zero random value in the kernel?

Definitely not. The question is more likely what guarantees we have with
crng_init=0. Probably we can't expect cryptographically strong randomness, but
zeroes still do look like a bug to me?

> This is broken at least in the 5.4~5.7 range but it is not clear if this
> ever worked. All examples of earlier kernels I have at hand use slab mm.
> 
> Thanks
> 
> Michal
> 


^ permalink raw reply

* [PATCH v4 2/2] powerpc/uaccess: Implement unsafe_copy_to_user() as a simple loop
From: Christophe Leroy @ 2020-04-17 17:08 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, npiggin,
	segher
  Cc: linuxppc-dev, linux-kernel
In-Reply-To: <23e680624680a9a5405f4b88740d2596d4b17c26.1587143308.git.christophe.leroy@c-s.fr>

At the time being, unsafe_copy_to_user() is based on
raw_copy_to_user() which calls __copy_tofrom_user().

__copy_tofrom_user() is a big optimised function to copy big amount
of data. It aligns destinations to cache line in order to use
dcbz instruction.

Today unsafe_copy_to_user() is called only from filldir().
It is used to mainly copy small amount of data like filenames,
so __copy_tofrom_user() is not fit.

Also, unsafe_copy_to_user() is used within user_access_begin/end
sections. In those section, it is preferable to not call functions.

Rewrite unsafe_copy_to_user() as a macro that uses __put_user_goto().
We first perform a loop of long, then we finish with necessary
complements.

unsafe_copy_to_user() might be used in the near future to copy
fixed-size data, like pt_regs structs during signal processing.
Having it as a macro allows GCC to optimise it for instead when
it knows the size in advance, it can unloop loops, drop complements
when the size is a multiple of longs, etc ...

Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
v4: new
---
 arch/powerpc/include/asm/uaccess.h | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h
index 9365b59495a2..b24fbe75f9ce 100644
--- a/arch/powerpc/include/asm/uaccess.h
+++ b/arch/powerpc/include/asm/uaccess.h
@@ -514,7 +514,26 @@ static __must_check inline bool user_access_begin(const void __user *ptr, size_t
 #define unsafe_op_wrap(op, err) do { if (unlikely(op)) goto err; } while (0)
 #define unsafe_get_user(x, p, e) unsafe_op_wrap(__get_user_allowed(x, p), e)
 #define unsafe_put_user(x, p, e) __put_user_goto(x, p, e)
+
 #define unsafe_copy_to_user(d, s, l, e) \
-	unsafe_op_wrap(raw_copy_to_user_allowed(d, s, l), e)
+do {									\
+	u8 __user *_dst = (u8 __user *)(d);				\
+	const u8 *_src = (const u8 *)(s);				\
+	size_t _len = (l);						\
+	int _i;								\
+									\
+	for (_i = 0; _i < (_len & ~(sizeof(long) - 1)); _i += sizeof(long))		\
+		__put_user_goto(*(long*)(_src + _i), (long __user *)(_dst + _i), e);\
+	if (IS_ENABLED(CONFIG_PPC64) && (_len & 4)) {			\
+		__put_user_goto(*(u32*)(_src + _i), (u32 __user *)(_dst + _i), e);	\
+		_i += 4;						\
+	}								\
+	if (_len & 2) {							\
+		__put_user_goto(*(u16*)(_src + _i), (u16 __user *)(_dst + _i), e);	\
+		_i += 2;						\
+	}								\
+	if (_len & 1) \
+		__put_user_goto(*(u8*)(_src + _i), (u8 __user *)(_dst + _i), e);\
+} while (0)
 
 #endif	/* _ARCH_POWERPC_UACCESS_H */
-- 
2.25.0


^ 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