LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH -next] ocxl: remove set but not used variables 'tid' and 'lpid'
From: Frederic Barrat @ 2019-04-03 13:58 UTC (permalink / raw)
  To: Yue Haibing, andrew.donnellan, arnd, gregkh; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <20190329154456.27152-1-yuehaibing@huawei.com>



Le 29/03/2019 à 16:44, Yue Haibing a écrit :
> From: YueHaibing <yuehaibing@huawei.com>
> 
> Fixes gcc '-Wunused-but-set-variable' warning:
> 
> drivers/misc/ocxl/link.c: In function 'xsl_fault_handler':
> drivers/misc/ocxl/link.c:187:17: warning: variable 'tid' set but not used [-Wunused-but-set-variable]
> drivers/misc/ocxl/link.c:187:6: warning: variable 'lpid' set but not used [-Wunused-but-set-variable]
> 
> They are never used and can be removed.
> 
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
> ---

Acked-by: Frederic Barrat <fbarrat@linux.ibm.com>


>   drivers/misc/ocxl/link.c | 4 +---
>   1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/misc/ocxl/link.c b/drivers/misc/ocxl/link.c
> index d50b861..3be07e9 100644
> --- a/drivers/misc/ocxl/link.c
> +++ b/drivers/misc/ocxl/link.c
> @@ -184,7 +184,7 @@ static irqreturn_t xsl_fault_handler(int irq, void *data)
>   	u64 dsisr, dar, pe_handle;
>   	struct pe_data *pe_data;
>   	struct ocxl_process_element *pe;
> -	int lpid, pid, tid;
> +	int pid;
>   	bool schedule = false;
>   
>   	read_irq(spa, &dsisr, &dar, &pe_handle);
> @@ -192,9 +192,7 @@ static irqreturn_t xsl_fault_handler(int irq, void *data)
>   
>   	WARN_ON(pe_handle > SPA_PE_MASK);
>   	pe = spa->spa_mem + pe_handle;
> -	lpid = be32_to_cpu(pe->lpid);
>   	pid = be32_to_cpu(pe->pid);
> -	tid = be32_to_cpu(pe->tid);
>   	/* We could be reading all null values here if the PE is being
>   	 * removed while an interrupt kicks in. It's not supposed to
>   	 * happen if the driver notified the AFU to terminate the
> 


^ permalink raw reply

* Re: [PATCH 2/2] arch: add pidfd and io_uring syscalls everywhere
From: Jens Axboe @ 2019-04-03 13:49 UTC (permalink / raw)
  To: Will Deacon, Michael Ellerman
  Cc: Rich Felker, linux-ia64, linux-sh, Heiko Carstens, linux-mips,
	James E . J . Bottomley, Max Filippov, Paul Mackerras, sparclinux,
	linux-s390, Helge Deller, Russell King, Geert Uytterhoeven,
	Catalin Marinas, James Hogan, Firoz Khan, Matt Turner, Fenghua Yu,
	Arnd Bergmann, linux-m68k, Ivan Kokshaysky, linux-arm-kernel,
	Richard Henderson, Michal Simek, Tony Luck, linux-parisc,
	linux-kernel, Ralf Baechle, Paul Burton, linux-alpha,
	Martin Schwidefsky, Andrew Morton, linuxppc-dev, David S . Miller
In-Reply-To: <20190403111134.GA7159@fuggles.cambridge.arm.com>

On 4/3/19 5:11 AM, Will Deacon wrote:
> Hi Michael,
> 
> On Wed, Apr 03, 2019 at 01:47:50PM +1100, Michael Ellerman wrote:
>> Arnd Bergmann <arnd@arndb.de> writes:
>>> diff --git a/arch/powerpc/kernel/syscalls/syscall.tbl b/arch/powerpc/kernel/syscalls/syscall.tbl
>>> index b18abb0c3dae..00f5a63c8d9a 100644
>>> --- a/arch/powerpc/kernel/syscalls/syscall.tbl
>>> +++ b/arch/powerpc/kernel/syscalls/syscall.tbl
>>> @@ -505,3 +505,7 @@
>>>  421	32	rt_sigtimedwait_time64		sys_rt_sigtimedwait		compat_sys_rt_sigtimedwait_time64
>>>  422	32	futex_time64			sys_futex			sys_futex
>>>  423	32	sched_rr_get_interval_time64	sys_sched_rr_get_interval	sys_sched_rr_get_interval
>>> +424	common	pidfd_send_signal		sys_pidfd_send_signal
>>> +425	common	io_uring_setup			sys_io_uring_setup
>>> +426	common	io_uring_enter			sys_io_uring_enter
>>> +427	common	io_uring_register		sys_io_uring_register
>>
>> Acked-by: Michael Ellerman <mpe@ellerman.id.au> (powerpc)
>>
>> Lightly tested.
>>
>> The pidfd_test selftest passes.
> 
> That reports pass for me too, although it fails to unshare the pid ns, which I
> assume is benign.
> 
>> Ran the io_uring example from fio, which prints lots of:
> 
> How did you invoke that? I had a play with the tests in:

It's t/io_uring from the fio repo:

git://git.kernel.dk/fio

and you just run it ala:

# make t/io_uring
# t/io_uring /dev/some_device

>   git://git.kernel.dk/liburing
> 
> but I quickly ran into the kernel oops below.
> 
> Will
> 
> --->8
> 
> will@autoplooker:~/liburing/test$ ./io_uring_register 
> RELIMIT_MEMLOCK: 67108864 (67108864)
> [   35.477875] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000070
> [   35.478969] Mem abort info:
> [   35.479296]   ESR = 0x96000004
> [   35.479785]   Exception class = DABT (current EL), IL = 32 bits
> [   35.480528]   SET = 0, FnV = 0
> [   35.480980]   EA = 0, S1PTW = 0
> [   35.481345] Data abort info:
> [   35.481680]   ISV = 0, ISS = 0x00000004
> [   35.482267]   CM = 0, WnR = 0
> [   35.482618] user pgtable: 4k pages, 48-bit VAs, pgdp = (____ptrval____)
> [   35.483486] [0000000000000070] pgd=0000000000000000
> [   35.484041] Internal error: Oops: 96000004 [#1] PREEMPT SMP
> [   35.484788] Modules linked in:
> [   35.485311] CPU: 113 PID: 3973 Comm: io_uring_regist Not tainted 5.1.0-rc3-00012-g40b114779944 #1
> [   35.486712] Hardware name: linux,dummy-virt (DT)
> [   35.487450] pstate: 20400005 (nzCv daif +PAN -UAO)
> [   35.488228] pc : link_pwq+0x10/0x60
> [   35.488794] lr : apply_wqattrs_commit+0xe0/0x118
> [   35.489550] sp : ffff000017e2bbc0

Huh, this looks odd, it's crashing inside the wq setup.


-- 
Jens Axboe


^ permalink raw reply

* Re: [PATCH 0/4] Enabling secure boot on PowerNV systems
From: Michael Ellerman @ 2019-04-03 13:21 UTC (permalink / raw)
  To: Claudio Carvalho, linuxppc-dev, linux-efi, linux-integrity,
	linux-kernel
  Cc: Ard Biesheuvel, Nayna Jain, Claudio Carvalho, Matthew Garret,
	Paul Mackerras, Jeremy Kerr
In-Reply-To: <20190402181505.25037-1-cclaudio@linux.ibm.com>

Hi Claudio,

Thanks for posting this.

Claudio Carvalho <cclaudio@linux.ibm.com> writes:
> This patch set is part of a series that implements secure boot on
> PowerNV systems.
>
> In order to verify the OS kernel on PowerNV, secure boot requires X.509
> certificates trusted by the platform, the secure boot modes, and several
> other pieces of information. These are stored in secure variables
> controlled by OPAL, also known as OPAL secure variables.
>
> This patch set adds the following features:
>
> 1. Enable efivarfs by selecting CONFIG_EFI in the CONFIG_OPAL_SECVAR
>    introduced in this patch set. With CONFIG_EFIVAR_FS, userspace tools can
>    be used to manage the secure variables.
> 2. Add support for OPAL secure variables by overwriting the EFI hooks
>    (get_variable, get_next_variable, set_variable and query_variable_info)
>    with OPAL call wrappers. There is probably a better way to add this
>    support, for example, we are investigating if we could register the
>    efivar_operations rather than overwriting the EFI hooks. In this patch
>    set, CONFIG_OPAL_SECVAR selects CONFIG_EFI. If, instead, we registered
>    efivar_operations, CONFIG_EFIVAR_FS would need to depend on
>    CONFIG_EFI|| CONFIG_OPAL_SECVAR. Comments or suggestions on the
>    preferred technique would be greatly appreciated.

I am *very* reluctant to start selecting CONFIG_EFI on powerpc.

Simply because we don't actually have EFI, and I worry we're going to
both break assumptions in the EFI code as well as impose requirements on
the powerpc code that aren't really necessary.

So I'd definitely prefer we go the route of enabling efivarfs with an
alternate backend.

Better still would be a generic secure variable interface as Matt
suggests, if the userspace tools can be relatively easily adapted to use
that interface.

cheers

^ permalink raw reply

* Re: [PATCH] powerpc/xmon: add read-only mode
From: Christopher M Riedl @ 2019-04-03 13:15 UTC (permalink / raw)
  To: Christophe Leroy, linuxppc-dev; +Cc: andonnel
In-Reply-To: <ee41455c-0ef8-2b5b-2f61-16bcf5f45d3f@c-s.fr>

> On April 3, 2019 at 12:15 AM Christophe Leroy <christophe.leroy@c-s.fr> wrote:
> 
> 
> 
> 
> Le 03/04/2019 à 05:38, Christopher M Riedl a écrit :
> >> On March 29, 2019 at 3:41 AM Christophe Leroy <christophe.leroy@c-s.fr> wrote:
> >>
> >>
> >>
> >>
> >> Le 29/03/2019 à 05:21, cmr a écrit :
> >>> Operations which write to memory should be restricted on secure systems
> >>> and optionally to avoid self-destructive behaviors.
> >>>
> >>> Add a config option, XMON_RO, to control default xmon behavior along
> >>> with kernel cmdline options xmon=ro and xmon=rw for explicit control.
> >>> The default is to enable read-only mode.
> >>>
> >>> The following xmon operations are affected:
> >>> memops:
> >>> 	disable memmove
> >>> 	disable memset
> >>> memex:
> >>> 	no-op'd mwrite
> >>> super_regs:
> >>> 	no-op'd write_spr
> >>> bpt_cmds:
> >>> 	disable
> >>> proc_call:
> >>> 	disable
> >>>
> >>> Signed-off-by: cmr <cmr@informatik.wtf>
> >>
> >> A Fully qualified name should be used.
> > 
> > What do you mean by fully-qualified here? PPC_XMON_RO? (PPC_)XMON_READONLY?
> 
> I mean it should be
> 
> Signed-off-by: Christopher M Riedl <cmr@informatik.wtf>
> 
> instead of
> 
> Signed-off-by: cmr <cmr@informatik.wtf>
> 

Hehe, thanks :)

> > 
> >>
> >>> ---
> >>>    arch/powerpc/Kconfig.debug |  7 +++++++
> >>>    arch/powerpc/xmon/xmon.c   | 24 ++++++++++++++++++++++++
> >>>    2 files changed, 31 insertions(+)
> >>>
> >>> diff --git a/arch/powerpc/Kconfig.debug b/arch/powerpc/Kconfig.debug
> >>> index 4e00cb0a5464..33cc01adf4cb 100644
> >>> --- a/arch/powerpc/Kconfig.debug
> >>> +++ b/arch/powerpc/Kconfig.debug
> >>> @@ -117,6 +117,13 @@ config XMON_DISASSEMBLY
> >>>    	  to say Y here, unless you're building for a memory-constrained
> >>>    	  system.
> >>>    
> >>> +config XMON_RO
> >>> +	bool "Set xmon read-only mode"
> >>> +	depends on XMON
> >>> +	default y
> >>
> >> Should it really be always default y ?
> >> I would set default 'y' only when some security options are also set.
> >>
> > 
> > This is a good point, I based this on an internal Slack suggestion but giving this more thought, disabling read-only mode by default makes more sense. I'm not sure what security options could be set though?
> > 
> 
> Maybe starting with CONFIG_STRICT_KERNEL_RWX
> 
> Another point that may also be addressed by your patch is the definition 
> of PAGE_KERNEL_TEXT:
> 
> #if defined(CONFIG_KGDB) || defined(CONFIG_XMON) || 
> defined(CONFIG_BDI_SWITCH) ||\
> 	defined(CONFIG_KPROBES) || defined(CONFIG_DYNAMIC_FTRACE)
> #define PAGE_KERNEL_TEXT	PAGE_KERNEL_X
> #else
> #define PAGE_KERNEL_TEXT	PAGE_KERNEL_ROX
> #endif
> 
> The above let me think that it would be better if you add a config 
> XMON_RW instead of XMON_RO, with default !STRICT_KERNEL_RWX
> 
> Christophe

Thanks! I like that a lot better, this, along with your other suggestions
in the initial review, will be in the next version.

^ permalink raw reply

* Re: [PATCH] powerpc/xmon: add read-only mode
From: Christopher M Riedl @ 2019-04-03 13:02 UTC (permalink / raw)
  To: Andrew Donnellan, linuxppc-dev
In-Reply-To: <81765ce2-af34-6776-a5f6-f69d265cd3c2@au1.ibm.com>


> On March 29, 2019 at 12:49 AM Andrew Donnellan <andrew.donnellan@au1.ibm.com> wrote:
> 
> 
> On 29/3/19 3:21 pm, cmr wrote:
> > Operations which write to memory should be restricted on secure systems
> > and optionally to avoid self-destructive behaviors.
> 
> For reference:
>   - https://github.com/linuxppc/issues/issues/219
>   - https://github.com/linuxppc/issues/issues/232
> 
> Perhaps clarify what is meant here by "secure systems".
> 
> Otherwise commit message looks good.
> 

I will reword this for the next patch to reflect the verbiage in the referenced
github issue -- ie. Secure Boot and not violating secure boot integrity by using xmon.

> 
> > ---
> >   arch/powerpc/Kconfig.debug |  7 +++++++
> >   arch/powerpc/xmon/xmon.c   | 24 ++++++++++++++++++++++++
> >   2 files changed, 31 insertions(+)
> > 
> > diff --git a/arch/powerpc/Kconfig.debug b/arch/powerpc/Kconfig.debug
> > index 4e00cb0a5464..33cc01adf4cb 100644
> > --- a/arch/powerpc/Kconfig.debug
> > +++ b/arch/powerpc/Kconfig.debug
> > @@ -117,6 +117,13 @@ config XMON_DISASSEMBLY
> >   	  to say Y here, unless you're building for a memory-constrained
> >   	  system.
> >   
> > +config XMON_RO
> > +	bool "Set xmon read-only mode"
> > +	depends on XMON
> > +	default y
> > +	help
> > +	  Disable state- and memory-altering write operations in xmon.
> 
> The meaning of this option is a bit unclear.
> 
>  From the code - it looks like what this option actually does is enable 
> RO mode *by default*. In which case it should probably be called 
> XMON_RO_DEFAULT and the description should note that RW mode can still 
> be enabled via a cmdline option.
>

Based on Christophe's feedback the default will change for this option in the
next patch. I will also add the cmdline options to the description for clarity.

>
> > +
> >   config DEBUGGER
> >   	bool
> >   	depends on KGDB || XMON
> > diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
> > index a0f44f992360..c13ee73cdfd4 100644
> > --- a/arch/powerpc/xmon/xmon.c
> > +++ b/arch/powerpc/xmon/xmon.c
> > @@ -80,6 +80,7 @@ static int set_indicator_token = RTAS_UNKNOWN_SERVICE;
> >   #endif
> >   static unsigned long in_xmon __read_mostly = 0;
> >   static int xmon_on = IS_ENABLED(CONFIG_XMON_DEFAULT);
> > +static int xmon_ro = IS_ENABLED(CONFIG_XMON_RO);
> >   
> >   static unsigned long adrs;
> >   static int size = 1;
> > @@ -1042,6 +1043,8 @@ cmds(struct pt_regs *excp)
> >   			set_lpp_cmd();
> >   			break;
> >   		case 'b':
> > +			if (xmon_ro == 1)
> > +				break;
> 
> For all these cases - it would be much better to print an error message 
> somewhere when we abort due to read-only mode.
> 

I included print messages initially but then thought about how xmon is intended
for "power" users. I can add print statements to avoid confusion and frustration
since the operations are just "silently" dropped -- *if* that aligns with xmon's
"philosophy".

^ permalink raw reply

* Re: [PATCH 0/6] convert locked_vm from unsigned long to atomic64_t
From: Steven Sistare @ 2019-04-03 12:51 UTC (permalink / raw)
  To: Daniel Jordan
  Cc: Davidlohr Bueso, linux_lkml_grp, kvm, Alan Tull,
	Alexey Kardashevskiy, linux-fpga, linux-kernel, kvm-ppc, linux-mm,
	Alex Williamson, Moritz Fischer, akpm, linuxppc-dev,
	Christoph Lameter, Wu Hao
In-Reply-To: <20190402204158.27582-1-daniel.m.jordan@oracle.com>

On 4/2/2019 4:41 PM, Daniel Jordan wrote:
> Hi,
> 
> From patch 1:
> 
>   Taking and dropping mmap_sem to modify a single counter, locked_vm, is
>   overkill when the counter could be synchronized separately.
>   
>   Make mmap_sem a little less coarse by changing locked_vm to an atomic,
>   the 64-bit variety to avoid issues with overflow on 32-bit systems.
> 
> This is a more conservative alternative to [1] with no user-visible
> effects.  Thanks to Alexey Kardashevskiy for pointing out the racy
> atomics and to Alex Williamson, Christoph Lameter, Ira Weiny, and Jason
> Gunthorpe for their comments on [1].
> 
> Davidlohr Bueso recently did a similar conversion for pinned_vm[2].
> 
> Testing
>  1. passes LTP mlock[all], munlock[all], fork, mmap, and mremap tests in an
>     x86 kvm guest
>  2. a VFIO-enabled x86 kvm guest shows the same VmLck in
>     /proc/pid/status before and after this change
>  3. cross-compiles on powerpc
> 
> The series is based on v5.1-rc3.  Please consider for 5.2.
> 
> Daniel
> 
> [1] https://lore.kernel.org/linux-mm/20190211224437.25267-1-daniel.m.jordan@oracle.com/
> [2] https://lore.kernel.org/linux-mm/20190206175920.31082-1-dave@stgolabs.net/
> 
> Daniel Jordan (6):
>   mm: change locked_vm's type from unsigned long to atomic64_t
>   vfio/type1: drop mmap_sem now that locked_vm is atomic
>   vfio/spapr_tce: drop mmap_sem now that locked_vm is atomic
>   fpga/dlf/afu: drop mmap_sem now that locked_vm is atomic
>   powerpc/mmu: drop mmap_sem now that locked_vm is atomic
>   kvm/book3s: drop mmap_sem now that locked_vm is atomic
> 
>  arch/powerpc/kvm/book3s_64_vio.c    | 34 ++++++++++--------------
>  arch/powerpc/mm/mmu_context_iommu.c | 28 +++++++++-----------
>  drivers/fpga/dfl-afu-dma-region.c   | 40 ++++++++++++-----------------
>  drivers/vfio/vfio_iommu_spapr_tce.c | 37 ++++++++++++--------------
>  drivers/vfio/vfio_iommu_type1.c     | 31 +++++++++-------------
>  fs/proc/task_mmu.c                  |  2 +-
>  include/linux/mm_types.h            |  2 +-
>  kernel/fork.c                       |  2 +-
>  mm/debug.c                          |  5 ++--
>  mm/mlock.c                          |  4 +--
>  mm/mmap.c                           | 18 ++++++-------
>  mm/mremap.c                         |  6 ++---
>  12 files changed, 89 insertions(+), 120 deletions(-)
> 
> base-commit: 79a3aaa7b82e3106be97842dedfd8429248896e6

Hi Daniel,
  You could clean all 6 patches up nicely with a common subroutine that
increases locked_vm subject to the rlimit.  Pass a bool arg that is true if
the  limit should be enforced, !dma->lock_cap for one call site, and
!capable(CAP_IPC_LOCK) for the rest.  Push the warnings and debug statements
to the subroutine as well.  One patch could refactor, and a second could
change the locking method.

- Steve

^ permalink raw reply

* Re: [PATCH v3 5/5] Lib: sort.h: remove the size argument from the swap function
From: Andy Shevchenko @ 2019-04-03 11:33 UTC (permalink / raw)
  To: Andrey Abramov
  Cc: peterz, Dave Chinner, Rasmus Villemoes, Linux Kernel Mailing List,
	yamada.masahiro, paulus, hpa, jslaby, sfr, darrick.wong, richard,
	mark, x86, mingo, naveen.n.rao, linux-snps-arc, jannh, bp, jlbec,
	jpoimboe, tglx, George Spelvin, dedekind1, vgupta, ard.biesheuvel,
	adrian.hunter, gregkh, linux-mtd, Morton Andrew, linuxppc-dev,
	ocfs2-devel
In-Reply-To: <793311554238525@sas1-87f9feb8d943.qloud-c.yandex.net>

On Tue, Apr 02, 2019 at 11:55:25PM +0300, Andrey Abramov wrote:
> Removes size argument from the swap function because:
> 	1) It wasn't used.
> 	2) Custom swap function knows what kind of objects it swaps,
> 		so it already knows their sizes.
> 
> Signed-off-by: Andrey Abramov <st5pub@yandex.ru>
> Reviewed by: George Spelvin <lkml@sdf.org>

FWIW,
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

> ---
>  arch/x86/kernel/unwind_orc.c | 2 +-
>  include/linux/sort.h         | 2 +-
>  kernel/jump_label.c          | 2 +-
>  lib/extable.c                | 2 +-
>  lib/sort.c                   | 7 +++----
>  5 files changed, 7 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/x86/kernel/unwind_orc.c b/arch/x86/kernel/unwind_orc.c
> index 89be1be1790c..dc410b567189 100644
> --- a/arch/x86/kernel/unwind_orc.c
> +++ b/arch/x86/kernel/unwind_orc.c
> @@ -176,7 +176,7 @@ static struct orc_entry *orc_find(unsigned long ip)
>  	return orc_ftrace_find(ip);
>  }
>  
> -static void orc_sort_swap(void *_a, void *_b, int size)
> +static void orc_sort_swap(void *_a, void *_b)
>  {
>  	struct orc_entry *orc_a, *orc_b;
>  	struct orc_entry orc_tmp;
> diff --git a/include/linux/sort.h b/include/linux/sort.h
> index 2b99a5dd073d..13bb4635b5f1 100644
> --- a/include/linux/sort.h
> +++ b/include/linux/sort.h
> @@ -6,6 +6,6 @@
>  
>  void sort(void *base, size_t num, size_t size,
>  	  int (*cmp)(const void *, const void *),
> -	  void (*swap)(void *, void *, int));
> +	  void (*swap)(void *, void *));
>  
>  #endif
> diff --git a/kernel/jump_label.c b/kernel/jump_label.c
> index bad96b476eb6..6b1187b8a060 100644
> --- a/kernel/jump_label.c
> +++ b/kernel/jump_label.c
> @@ -45,7 +45,7 @@ static int jump_label_cmp(const void *a, const void *b)
>  	return 0;
>  }
>  
> -static void jump_label_swap(void *a, void *b, int size)
> +static void jump_label_swap(void *a, void *b)
>  {
>  	long delta = (unsigned long)a - (unsigned long)b;
>  	struct jump_entry *jea = a;
> diff --git a/lib/extable.c b/lib/extable.c
> index f54996fdd0b8..0515a94538ca 100644
> --- a/lib/extable.c
> +++ b/lib/extable.c
> @@ -28,7 +28,7 @@ static inline unsigned long ex_to_insn(const struct exception_table_entry *x)
>  #ifndef ARCH_HAS_RELATIVE_EXTABLE
>  #define swap_ex		NULL
>  #else
> -static void swap_ex(void *a, void *b, int size)
> +static void swap_ex(void *a, void *b)
>  {
>  	struct exception_table_entry *x = a, *y = b, tmp;
>  	int delta = b - a;
> diff --git a/lib/sort.c b/lib/sort.c
> index 50855ea8c262..8704750e6bde 100644
> --- a/lib/sort.c
> +++ b/lib/sort.c
> @@ -114,7 +114,7 @@ static void swap_bytes(void *a, void *b, size_t n)
>  	} while (n);
>  }
>  
> -typedef void (*swap_func_t)(void *a, void *b, int size);
> +typedef void (*swap_func_t)(void *a, void *b);
>  
>  /*
>   * The values are arbitrary as long as they can't be confused with
> @@ -138,7 +138,7 @@ static void do_swap(void *a, void *b, size_t size, swap_func_t swap_func)
>  	else if (swap_func == SWAP_BYTES)
>  		swap_bytes(a, b, size);
>  	else
> -		swap_func(a, b, (int)size);
> +		swap_func(a, b);
>  }
>  
>  /**
> @@ -186,8 +186,7 @@ static size_t parent(size_t i, unsigned int lsbit, size_t size)
>   * it less suitable for kernel use.
>   */
>  void sort(void *base, size_t num, size_t size,
> -	  int (*cmp_func)(const void *, const void *),
> -	  void (*swap_func)(void *, void *, int size))
> +	  int (*cmp_func)(const void *, const void *), swap_func_t swap_func)
>  {
>  	/* pre-scale counters for performance */
>  	size_t n = num * size, a = (num/2) * size;
> -- 
> 2.21.0
> 
> 

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply

* Re: [PATCH 2/2] arch: add pidfd and io_uring syscalls everywhere
From: Will Deacon @ 2019-04-03 11:11 UTC (permalink / raw)
  To: Michael Ellerman, axboe
  Cc: Rich Felker, linux-ia64, linux-sh, Heiko Carstens, linux-mips,
	James E . J . Bottomley, Max Filippov, Paul Mackerras, sparclinux,
	linux-s390, Helge Deller, Russell King, Geert Uytterhoeven,
	Catalin Marinas, James Hogan, Firoz Khan, Matt Turner, Fenghua Yu,
	Arnd Bergmann, linux-m68k, Ivan Kokshaysky, linux-arm-kernel,
	Richard Henderson, Michal Simek, Tony Luck, linux-parisc,
	linux-kernel, Ralf Baechle, Paul Burton, linux-alpha,
	Martin Schwidefsky, Andrew Morton, linuxppc-dev, David S . Miller
In-Reply-To: <87tvff24a1.fsf@concordia.ellerman.id.au>

Hi Michael,

On Wed, Apr 03, 2019 at 01:47:50PM +1100, Michael Ellerman wrote:
> Arnd Bergmann <arnd@arndb.de> writes:
> > diff --git a/arch/powerpc/kernel/syscalls/syscall.tbl b/arch/powerpc/kernel/syscalls/syscall.tbl
> > index b18abb0c3dae..00f5a63c8d9a 100644
> > --- a/arch/powerpc/kernel/syscalls/syscall.tbl
> > +++ b/arch/powerpc/kernel/syscalls/syscall.tbl
> > @@ -505,3 +505,7 @@
> >  421	32	rt_sigtimedwait_time64		sys_rt_sigtimedwait		compat_sys_rt_sigtimedwait_time64
> >  422	32	futex_time64			sys_futex			sys_futex
> >  423	32	sched_rr_get_interval_time64	sys_sched_rr_get_interval	sys_sched_rr_get_interval
> > +424	common	pidfd_send_signal		sys_pidfd_send_signal
> > +425	common	io_uring_setup			sys_io_uring_setup
> > +426	common	io_uring_enter			sys_io_uring_enter
> > +427	common	io_uring_register		sys_io_uring_register
> 
> Acked-by: Michael Ellerman <mpe@ellerman.id.au> (powerpc)
> 
> Lightly tested.
> 
> The pidfd_test selftest passes.

That reports pass for me too, although it fails to unshare the pid ns, which I
assume is benign.

> Ran the io_uring example from fio, which prints lots of:

How did you invoke that? I had a play with the tests in:

  git://git.kernel.dk/liburing

but I quickly ran into the kernel oops below.

Will

--->8

will@autoplooker:~/liburing/test$ ./io_uring_register 
RELIMIT_MEMLOCK: 67108864 (67108864)
[   35.477875] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000070
[   35.478969] Mem abort info:
[   35.479296]   ESR = 0x96000004
[   35.479785]   Exception class = DABT (current EL), IL = 32 bits
[   35.480528]   SET = 0, FnV = 0
[   35.480980]   EA = 0, S1PTW = 0
[   35.481345] Data abort info:
[   35.481680]   ISV = 0, ISS = 0x00000004
[   35.482267]   CM = 0, WnR = 0
[   35.482618] user pgtable: 4k pages, 48-bit VAs, pgdp = (____ptrval____)
[   35.483486] [0000000000000070] pgd=0000000000000000
[   35.484041] Internal error: Oops: 96000004 [#1] PREEMPT SMP
[   35.484788] Modules linked in:
[   35.485311] CPU: 113 PID: 3973 Comm: io_uring_regist Not tainted 5.1.0-rc3-00012-g40b114779944 #1
[   35.486712] Hardware name: linux,dummy-virt (DT)
[   35.487450] pstate: 20400005 (nzCv daif +PAN -UAO)
[   35.488228] pc : link_pwq+0x10/0x60
[   35.488794] lr : apply_wqattrs_commit+0xe0/0x118
[   35.489550] sp : ffff000017e2bbc0
[   35.490088] x29: ffff000017e2bbc0 x28: ffff8004b9118000 
[   35.490939] x27: 0000000000000000 x26: ffff8004c21c4200 
[   35.491786] x25: 0000000000000004 x24: ffff00001123e1b0 
[   35.492640] x23: ffff8004c5390000 x22: ffff8004bb440500 
[   35.493502] x21: ffff8004bb440500 x20: 0000000000000070 
[   35.494355] x19: 0000000000000022 x18: 0000000000000000 
[   35.495202] x17: 0000000000000000 x16: 0000000000000000 
[   35.496054] x15: 0000000000000000 x14: ffff7e0012e8a240 
[   35.496910] x13: 00004a73a5e663e2 x12: 0000000000000000 
[   35.497764] x11: 0000000000000001 x10: 0000000000000070 
[   35.498611] x9 : ffff8004cb49d610 x8 : 00000000ffffffff 
[   35.499462] x7 : ffff8004c4ff9c70 x6 : ffff8004cb49ccb0 
[   35.500308] x5 : ffff8004c66cc4c0 x4 : 0000000000000001 
[   35.501173] x3 : 0000000000000000 x2 : 0000000000000040 
[   35.502019] x1 : 0000000000000004 x0 : 0000000000000000 
[   35.502872] Process io_uring_regist (pid: 3973, stack limit = 0x(____ptrval____))
[   35.504052] Call trace:
[   35.504463]  link_pwq+0x10/0x60
[   35.504987]  apply_wqattrs_commit+0xe0/0x118
[   35.505681]  apply_workqueue_attrs_locked+0x3c/0x80
[   35.506460]  apply_workqueue_attrs+0x3c/0x60
[   35.507152]  alloc_workqueue+0x264/0x430
[   35.507786]  io_uring_setup+0x478/0x6a8
[   35.508414]  __arm64_sys_io_uring_setup+0x18/0x20
[   35.509183]  el0_svc_common+0x80/0xf0
[   35.509786]  el0_svc_handler+0x2c/0x80
[   35.510393]  el0_svc+0x8/0xc
[   35.510873] Code: a9bd7bfd 910003fd a90153f3 9101c014 (f9403802) 
[   35.511843] ---[ end trace 0a53e45ee26def4c ]---
Segmentation fault

^ permalink raw reply

* [PATCH V2] ASoC: fsl_esai: Support synchronous mode
From: S.j. Wang @ 2019-04-03 10:07 UTC (permalink / raw)
  To: timur@kernel.org, nicoleotsuka@gmail.com, Xiubo.Lee@gmail.com,
	festevam@gmail.com, broonie@kernel.org,
	alsa-devel@alsa-project.org
  Cc: linuxppc-dev@lists.ozlabs.org

In ESAI synchronous mode, the clock is generated by Tx, So
we should always set registers of Tx which relate with the
bit clock and frame clock generation (TCCR, TCR, ECR), even
there is only Rx is working.

Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
---
changes in v2
- refine the patch according Nicolin's comments
- merge plain setting.

 sound/soc/fsl/fsl_esai.c | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/sound/soc/fsl/fsl_esai.c b/sound/soc/fsl/fsl_esai.c
index 3623aa9a6f2e..f2e7b27f8447 100644
--- a/sound/soc/fsl/fsl_esai.c
+++ b/sound/soc/fsl/fsl_esai.c
@@ -218,7 +218,7 @@ static int fsl_esai_set_dai_sysclk(struct snd_soc_dai *dai, int clk_id,
 {
 	struct fsl_esai *esai_priv = snd_soc_dai_get_drvdata(dai);
 	struct clk *clksrc = esai_priv->extalclk;
-	bool tx = clk_id <= ESAI_HCKT_EXTAL;
+	bool tx = (clk_id <= ESAI_HCKT_EXTAL || esai_priv->synchronous);
 	bool in = dir == SND_SOC_CLOCK_IN;
 	u32 ratio, ecr = 0;
 	unsigned long clk_rate;
@@ -253,7 +253,7 @@ static int fsl_esai_set_dai_sysclk(struct snd_soc_dai *dai, int clk_id,
 		ecr |= ESAI_ECR_ETI;
 		/* fall through */
 	case ESAI_HCKR_EXTAL:
-		ecr |= ESAI_ECR_ERI;
+		ecr |= esai_priv->synchronous ? ESAI_ECR_ETI : ESAI_ECR_ERI;
 		break;
 	default:
 		return -EINVAL;
@@ -537,10 +537,18 @@ static int fsl_esai_hw_params(struct snd_pcm_substream *substream,
 
 	bclk = params_rate(params) * slot_width * esai_priv->slots;
 
-	ret = fsl_esai_set_bclk(dai, tx, bclk);
+	ret = fsl_esai_set_bclk(dai, esai_priv->synchronous || tx, bclk);
 	if (ret)
 		return ret;
 
+	mask = ESAI_xCR_xSWS_MASK;
+	val = ESAI_xCR_xSWS(slot_width, width);
+
+	regmap_update_bits(esai_priv->regmap, REG_ESAI_xCR(tx), mask, val);
+	/* Recording in synchronous mode needs to set TCR also */
+	if (!tx && esai_priv->synchronous)
+		regmap_update_bits(esai_priv->regmap, REG_ESAI_TCR, mask, val);
+
 	/* Use Normal mode to support monaural audio */
 	regmap_update_bits(esai_priv->regmap, REG_ESAI_xCR(tx),
 			   ESAI_xCR_xMOD_MASK, params_channels(params) > 1 ?
@@ -556,10 +564,9 @@ static int fsl_esai_hw_params(struct snd_pcm_substream *substream,
 
 	regmap_update_bits(esai_priv->regmap, REG_ESAI_xFCR(tx), mask, val);
 
-	mask = ESAI_xCR_xSWS_MASK | (tx ? ESAI_xCR_PADC : 0);
-	val = ESAI_xCR_xSWS(slot_width, width) | (tx ? ESAI_xCR_PADC : 0);
-
-	regmap_update_bits(esai_priv->regmap, REG_ESAI_xCR(tx), mask, val);
+	if (tx)
+		regmap_update_bits(esai_priv->regmap, REG_ESAI_TCR,
+					ESAI_xCR_PADC, ESAI_xCR_PADC);
 
 	/* Remove ESAI personal reset by configuring ESAI_PCRC and ESAI_PRRC */
 	regmap_update_bits(esai_priv->regmap, REG_ESAI_PRRC,
-- 
1.9.1


^ permalink raw reply related

* Re: [PATCH 3/5] s390: Fix vDSO clock_getres()
From: Thomas Gleixner @ 2019-04-03 10:06 UTC (permalink / raw)
  To: Martin Schwidefsky
  Cc: linux-arch, linux-s390, Shuah Khan, Vincent Chen, Arnd Bergmann,
	Heiko Carstens, Catalin Marinas, Will Deacon, Paul Mackerras,
	Vincenzo Frascino, linuxppc-dev, linux-arm-kernel, Greentime Hu
In-Reply-To: <20190403113844.334a32d9@mschwideX1>

On Wed, 3 Apr 2019, Martin Schwidefsky wrote:

> On Mon,  1 Apr 2019 12:51:50 +0100
> Vincenzo Frascino <vincenzo.frascino@arm.com> wrote:
> 
> > clock_getres in the vDSO library has to preserve the same behaviour
> > of posix_get_hrtimer_res().
> > 
> > In particular, posix_get_hrtimer_res() does:
> >     sec = 0;
> >     ns = hrtimer_resolution;
> > and hrtimer_resolution depends on the enablement of the high
> > resolution timers that can happen either at compile or at run time.
> > 
> > Fix the s390 vdso implementation of clock_getres keeping a copy of
> > hrtimer_resolution in vdso data and using that directly.
> > 
> > Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
> > Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
> > Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
> > ---
> >  arch/s390/include/asm/vdso.h           |  1 +
> >  arch/s390/kernel/asm-offsets.c         |  2 +-
> >  arch/s390/kernel/time.c                |  1 +
> >  arch/s390/kernel/vdso32/clock_getres.S | 17 ++++++++++++-----
> >  arch/s390/kernel/vdso64/clock_getres.S | 15 ++++++++++-----
> >  5 files changed, 25 insertions(+), 11 deletions(-)
> 
> I tried this patch and in principle this works. In that regard
> Acked-by: Martin Schwidefsky <schwidefsky@de.ibm.com>
> 
> But I wonder if the loop to check the update counter is really
> necessary. The hrtimer_resolution value can only changes once with
> the first call to hrtimer_switch_to_hres(). With the TOD clock
> as the only clock available on s390 we always have the ability
> to do hrtimer. It then all depends on the highres=[on|off] kernel
> parameter what value we get with clock_getres().

Yes, it's not changing after boot anymore.

Thanks,

	tglx

^ permalink raw reply

* RE: [PATCH] ASoC: fsl_esai: Support synchronous mode
From: S.j. Wang @ 2019-04-03 10:03 UTC (permalink / raw)
  To: Nicolin Chen
  Cc: alsa-devel@alsa-project.org, timur@kernel.org,
	Xiubo.Lee@gmail.com, festevam@gmail.com, broonie@kernel.org,
	linuxppc-dev@lists.ozlabs.org
In-Reply-To: <20190402200115.GA22168@Asurada-Nvidia.nvidia.com>

Hi

> 
> > > On Mon, Apr 01, 2019 at 11:39:10AM +0000, S.j. Wang wrote:
> > > > In ESAI synchronous mode, the clock is generated by Tx, So we
> > > > should always set registers of Tx which relate with the bit clock
> > > > and frame clock generation (TCCR, TCR, ECR), even there is only Rx is
> working.
> > > >
> > > > Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> > > > ---
> > > >  sound/soc/fsl/fsl_esai.c | 28 +++++++++++++++++++++++++++-
> > > >  1 file changed, 27 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/sound/soc/fsl/fsl_esai.c b/sound/soc/fsl/fsl_esai.c
> > > > index
> > > > 3623aa9a6f2e..d9fcddd55c02 100644
> > > > --- a/sound/soc/fsl/fsl_esai.c
> > > > +++ b/sound/soc/fsl/fsl_esai.c
> > > > @@ -230,6 +230,21 @@ static int fsl_esai_set_dai_sysclk(struct
> > > snd_soc_dai *dai, int clk_id,
> > > >  		return -EINVAL;
> > > >  	}
> > > >
> > > > +	if (esai_priv->synchronous && !tx) {
> > > > +		switch (clk_id) {
> > > > +		case ESAI_HCKR_FSYS:
> > > > +			fsl_esai_set_dai_sysclk(dai, ESAI_HCKT_FSYS,
> > > > +								freq, dir);
> > > > +			break;
> > > > +		case ESAI_HCKR_EXTAL:
> > > > +			fsl_esai_set_dai_sysclk(dai, ESAI_HCKT_EXTAL,
> > > > +								freq, dir);
> > >
> > > Not sure why you call set_dai_sysclk inside set_dai_sysclk again. It
> > > feels very confusing to do so, especially without a comments.
> >
> > For sync mode, only RX is enabled,  the register of tx should be set,
> > so call the Set_dai_sysclk again.
> 
> Yea, I understood that. But why not just replace RX with TX on the register-
> writing level? Do we need to set both TCCR and RCCR? Your change in
> hw_params() only sets TCCR inside fsl_esai_set_bclk(), so we probably only
> need to change TCCR for recordings running in sync mode, right?
> 
> From the commit message, it feels like that only the clock-related fields in
> the TX registers need to be set. Things like calculation and setting the
> direction of HCKx pin don't need to run again.
> 
> > > > @@ -537,10 +552,21 @@ static int fsl_esai_hw_params(struct
> > > > snd_pcm_substream *substream,
> > > >
> > > >  	bclk = params_rate(params) * slot_width * esai_priv->slots;
> > > >
> > > > -	ret = fsl_esai_set_bclk(dai, tx, bclk);
> > > > +	ret = fsl_esai_set_bclk(dai, esai_priv->synchronous ? true : tx,
> > > > +bclk);
> > > >  	if (ret)
> > > >  		return ret;
> > > >
> > > > +	if (esai_priv->synchronous && !tx) {
> > > > +		/* Use Normal mode to support monaural audio */
> > > > +		regmap_update_bits(esai_priv->regmap, REG_ESAI_TCR,
> > > > +			   ESAI_xCR_xMOD_MASK,
> > > params_channels(params) > 1 ?
> > > > +			   ESAI_xCR_xMOD_NETWORK : 0);
> > > > +
> > > > +		mask = ESAI_xCR_xSWS_MASK | ESAI_xCR_PADC;
> > > > +		val = ESAI_xCR_xSWS(slot_width, width) | ESAI_xCR_PADC;
> > > > +		regmap_update_bits(esai_priv->regmap, REG_ESAI_TCR,
> > > mask, val);
> > > > +	}
> > >
> > > Does synchronous mode require to set both TCR and RCR? or just TCR?
> 
> > Both TCR and RCR.  RCR will be set in normal flow.
> 
> OK. Settings both xCRs makes sense. Would you please try this:
> 
> ===============
> @@ -537,14 +552,20 @@ static int fsl_esai_hw_params(struct
> snd_pcm_substream *substream,
> 
>         bclk = params_rate(params) * slot_width * esai_priv->slots;
> 
> -       ret = fsl_esai_set_bclk(dai, tx, bclk);
> +       /* Synchronous mode uses TX clock generator */
> +       ret = fsl_esai_set_bclk(dai, esai_priv->synchronous || tx,
> + bclk);
>         if (ret)
>                 return ret;
> 
> +       mask = ESAI_xCR_xMOD_MASK | ESAI_xCR_xSWS_MASK;
> +       val = ESAI_xCR_xSWS(slot_width, width);
>         /* Use Normal mode to support monaural audio */
> -       regmap_update_bits(esai_priv->regmap, REG_ESAI_xCR(tx),
> -                          ESAI_xCR_xMOD_MASK, params_channels(params) > 1 ?
> -                          ESAI_xCR_xMOD_NETWORK : 0);
> +       val |= params_channels(params) > 1 ? ESAI_xCR_xMOD_NETWORK : 0;
> +
> +       regmap_update_bits(esai_priv->regmap, REG_ESAI_xCR(tx), mask, val);
> +       /* Recording in synchronous mode needs to set TCR also */
> +       if (!tx && esai_priv->synchronous)
> +               regmap_update_bits(esai_priv->regmap, REG_ESAI_TCR,
> + mask, val);
> 
>         regmap_update_bits(esai_priv->regmap, REG_ESAI_xFCR(tx),
>                            ESAI_xFCR_xFR_MASK, ESAI_xFCR_xFR); @@ -556,10
> +577,10 @@ static int fsl_esai_hw_params(struct snd_pcm_substream
> *substream,
> 
>         regmap_update_bits(esai_priv->regmap, REG_ESAI_xFCR(tx), mask, val);
> 
> -       mask = ESAI_xCR_xSWS_MASK | (tx ? ESAI_xCR_PADC : 0);
> -       val = ESAI_xCR_xSWS(slot_width, width) | (tx ? ESAI_xCR_PADC : 0);
> -
> -       regmap_update_bits(esai_priv->regmap, REG_ESAI_xCR(tx), mask, val);
> +       /* Only TCR has padding bit and needs to be set for synchronous
> mode */
> +       if (tx || esai_priv->synchronous)
> +               regmap_update_bits(esai_priv->regmap, REG_ESAI_TCR,
> +                                  ESAI_xCR_PADC, ESAI_xCR_PADC);
> 
>         /* Remove ESAI personal reset by configuring ESAI_PCRC and
> ESAI_PRRC */
>         regmap_update_bits(esai_priv->regmap, REG_ESAI_PRRC,
> ===============
> 
> It merges those plain settings so we won't need to change both sides if
> there's a modification of xCR settings in the future.

Thanks for reviewing,  will send v2.  And the ESAI_xCR_PADC, ESAI_xCR_xMOD_NETWORK
Is not needed in sync mode.


^ permalink raw reply

* Re: [PATCH 3/5] s390: Fix vDSO clock_getres()
From: Martin Schwidefsky @ 2019-04-03  9:38 UTC (permalink / raw)
  To: Vincenzo Frascino
  Cc: linux-arch, linux-s390, Shuah Khan, Arnd Bergmann, Heiko Carstens,
	Catalin Marinas, Will Deacon, Paul Mackerras, Thomas Gleixner,
	Vincent Chen, linuxppc-dev, linux-arm-kernel, Greentime Hu
In-Reply-To: <20190401115152.32751-4-vincenzo.frascino@arm.com>

On Mon,  1 Apr 2019 12:51:50 +0100
Vincenzo Frascino <vincenzo.frascino@arm.com> wrote:

> clock_getres in the vDSO library has to preserve the same behaviour
> of posix_get_hrtimer_res().
> 
> In particular, posix_get_hrtimer_res() does:
>     sec = 0;
>     ns = hrtimer_resolution;
> and hrtimer_resolution depends on the enablement of the high
> resolution timers that can happen either at compile or at run time.
> 
> Fix the s390 vdso implementation of clock_getres keeping a copy of
> hrtimer_resolution in vdso data and using that directly.
> 
> Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
> Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
> Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
> ---
>  arch/s390/include/asm/vdso.h           |  1 +
>  arch/s390/kernel/asm-offsets.c         |  2 +-
>  arch/s390/kernel/time.c                |  1 +
>  arch/s390/kernel/vdso32/clock_getres.S | 17 ++++++++++++-----
>  arch/s390/kernel/vdso64/clock_getres.S | 15 ++++++++++-----
>  5 files changed, 25 insertions(+), 11 deletions(-)

I tried this patch and in principle this works. In that regard
Acked-by: Martin Schwidefsky <schwidefsky@de.ibm.com>

But I wonder if the loop to check the update counter is really
necessary. The hrtimer_resolution value can only changes once with
the first call to hrtimer_switch_to_hres(). With the TOD clock
as the only clock available on s390 we always have the ability
to do hrtimer. It then all depends on the highres=[on|off] kernel
parameter what value we get with clock_getres().

-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.


^ permalink raw reply

* Re: [PATCH -next] ibmvnic: remove set but not used variable 'netdev'
From: Mukesh Ojha @ 2019-04-03  9:06 UTC (permalink / raw)
  To: Yue Haibing, davem, benh, paulus, mpe, tlfalcon, jallen
  Cc: netdev, linuxppc-dev, linux-kernel
In-Reply-To: <20190403075409.29384-1-yuehaibing@huawei.com>


On 4/3/2019 1:24 PM, Yue Haibing wrote:
> From: YueHaibing <yuehaibing@huawei.com>
>
> Fixes gcc '-Wunused-but-set-variable' warning:
>
> drivers/net/ethernet/ibm/ibmvnic.c: In function '__ibmvnic_reset':
> drivers/net/ethernet/ibm/ibmvnic.c:1971:21: warning: variable 'netdev' set but not used [-Wunused-but-set-variable]
>
> It's never used since introduction in
> commit ed651a10875f ("ibmvnic: Updated reset handling")
>
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
Reviewed-by: Mukesh Ojha <mojha@codeaurora.org>

Cheers,
-Mukesh
> ---
>   drivers/net/ethernet/ibm/ibmvnic.c | 2 --
>   1 file changed, 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
> index 25b8e04..20c4e08 100644
> --- a/drivers/net/ethernet/ibm/ibmvnic.c
> +++ b/drivers/net/ethernet/ibm/ibmvnic.c
> @@ -1968,13 +1968,11 @@ static void __ibmvnic_reset(struct work_struct *work)
>   {
>   	struct ibmvnic_rwi *rwi;
>   	struct ibmvnic_adapter *adapter;
> -	struct net_device *netdev;
>   	bool we_lock_rtnl = false;
>   	u32 reset_state;
>   	int rc = 0;
>   
>   	adapter = container_of(work, struct ibmvnic_adapter, ibmvnic_reset);
> -	netdev = adapter->netdev;
>   
>   	/* netif_set_real_num_xx_queues needs to take rtnl lock here
>   	 * unless wait_for_reset is set, in which case the rtnl lock

^ permalink raw reply

* [PATCH -next] ibmvnic: remove set but not used variable 'netdev'
From: Yue Haibing @ 2019-04-03  7:54 UTC (permalink / raw)
  To: davem, benh, paulus, mpe, tlfalcon, jallen
  Cc: netdev, YueHaibing, linuxppc-dev, linux-kernel

From: YueHaibing <yuehaibing@huawei.com>

Fixes gcc '-Wunused-but-set-variable' warning:

drivers/net/ethernet/ibm/ibmvnic.c: In function '__ibmvnic_reset':
drivers/net/ethernet/ibm/ibmvnic.c:1971:21: warning: variable 'netdev' set but not used [-Wunused-but-set-variable]

It's never used since introduction in
commit ed651a10875f ("ibmvnic: Updated reset handling")

Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
 drivers/net/ethernet/ibm/ibmvnic.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index 25b8e04..20c4e08 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -1968,13 +1968,11 @@ static void __ibmvnic_reset(struct work_struct *work)
 {
 	struct ibmvnic_rwi *rwi;
 	struct ibmvnic_adapter *adapter;
-	struct net_device *netdev;
 	bool we_lock_rtnl = false;
 	u32 reset_state;
 	int rc = 0;
 
 	adapter = container_of(work, struct ibmvnic_adapter, ibmvnic_reset);
-	netdev = adapter->netdev;
 
 	/* netif_set_real_num_xx_queues needs to take rtnl lock here
 	 * unless wait_for_reset is set, in which case the rtnl lock
-- 
2.7.0



^ permalink raw reply related

* Re: [PATCH] powerpc: config: skiroot: Add (back) MLX5 ethernet support
From: Mathieu Malaterre @ 2019-04-03  7:12 UTC (permalink / raw)
  To: Joel Stanley; +Cc: Carol L Soto, linuxppc-dev, Stewart Smith
In-Reply-To: <20190403004926.6702-1-joel@jms.id.au>

On Wed, Apr 3, 2019 at 2:51 AM Joel Stanley <joel@jms.id.au> wrote:
>
> It turns out that some defconfig changes and kernel config option
> changes meant we accidentally dropped Ethernet support for Mellanox CLX5
> cards.
>
> Reported-by: Carol L Soto <clsoto@us.ibm.com>
> Suggested-by: Carol L Soto <clsoto@us.ibm.com>
> Signed-off-by: Stewart Smith <stewart@linux.ibm.com>
> Signed-off-by: Joel Stanley <joel@jms.id.au>

Fixes: cbc39809a398 ("powerpc/configs: Update skiroot defconfig")

?

> ---
>  arch/powerpc/configs/skiroot_defconfig | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/arch/powerpc/configs/skiroot_defconfig b/arch/powerpc/configs/skiroot_defconfig
> index 5ba131c30f6b..6038b9347d9e 100644
> --- a/arch/powerpc/configs/skiroot_defconfig
> +++ b/arch/powerpc/configs/skiroot_defconfig
> @@ -163,6 +163,8 @@ CONFIG_S2IO=m
>  CONFIG_MLX4_EN=m
>  # CONFIG_MLX4_CORE_GEN2 is not set
>  CONFIG_MLX5_CORE=m
> +CONFIG_MLX5_CORE_EN=y
> +# CONFIG_MLX5_EN_RXNFC is not set
>  # CONFIG_NET_VENDOR_MICREL is not set
>  # CONFIG_NET_VENDOR_MICROSEMI is not set
>  CONFIG_MYRI10GE=m
> --
> 2.20.1
>

^ permalink raw reply

* [PATCH] powerpc/mm: Fix build error with FLATMEM book3s64 config
From: Aneesh Kumar K.V @ 2019-04-03  6:05 UTC (permalink / raw)
  To: npiggin, benh, paulus, mpe, Christophe Leroy
  Cc: Aneesh Kumar K.V, linuxppc-dev

The current value of MAX_PHYSMEM_BITS cannot work with 32 bit configs.
We used to have MAX_PHYSMEM_BITS not defined without SPARSEMEM and 32
bit configs never expected a value to be set for MAX_PHYSMEM_BITS.

Dependent code such as zsmalloc derived the right values based on other
fields. Instead of finding a value that works with different configs,
use new values only for book3s_64. For 64 bit booke, use the definition
of MAX_PHYSMEM_BITS as per commit a7df61a0e2b6 ("[PATCH] ppc64: Increase sparsemem defaults")
That change was done in 2005 and hopefully will work with book3e 64.

Fixes: 8bc086899816 ("powerpc/mm: Only define MAX_PHYSMEM_BITS in SPARSEMEM configurations")
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
 arch/powerpc/include/asm/book3s/64/mmu.h | 15 +++++++++++++++
 arch/powerpc/include/asm/mmu.h           | 15 ---------------
 arch/powerpc/include/asm/nohash/64/mmu.h |  2 ++
 3 files changed, 17 insertions(+), 15 deletions(-)

diff --git a/arch/powerpc/include/asm/book3s/64/mmu.h b/arch/powerpc/include/asm/book3s/64/mmu.h
index 1ceee000c18d..a809bdd77322 100644
--- a/arch/powerpc/include/asm/book3s/64/mmu.h
+++ b/arch/powerpc/include/asm/book3s/64/mmu.h
@@ -35,6 +35,21 @@ typedef pte_t *pgtable_t;
 
 #endif /* __ASSEMBLY__ */
 
+/*
+ * If we store section details in page->flags we can't increase the MAX_PHYSMEM_BITS
+ * if we increase SECTIONS_WIDTH we will not store node details in page->flags and
+ * page_to_nid does a page->section->node lookup
+ * Hence only increase for VMEMMAP. Further depending on SPARSEMEM_EXTREME reduce
+ * memory requirements with large number of sections.
+ * 51 bits is the max physical real address on POWER9
+ */
+#if defined(CONFIG_SPARSEMEM_VMEMMAP) && defined(CONFIG_SPARSEMEM_EXTREME) &&  \
+	defined(CONFIG_PPC_64K_PAGES)
+#define MAX_PHYSMEM_BITS 51
+#else
+#define MAX_PHYSMEM_BITS 46
+#endif
+
 /* 64-bit classic hash table MMU */
 #include <asm/book3s/64/mmu-hash.h>
 
diff --git a/arch/powerpc/include/asm/mmu.h b/arch/powerpc/include/asm/mmu.h
index 598cdcdd1355..78d53c4396ac 100644
--- a/arch/powerpc/include/asm/mmu.h
+++ b/arch/powerpc/include/asm/mmu.h
@@ -341,21 +341,6 @@ static inline bool strict_kernel_rwx_enabled(void)
  */
 #define MMU_PAGE_COUNT	16
 
-/*
- * If we store section details in page->flags we can't increase the MAX_PHYSMEM_BITS
- * if we increase SECTIONS_WIDTH we will not store node details in page->flags and
- * page_to_nid does a page->section->node lookup
- * Hence only increase for VMEMMAP. Further depending on SPARSEMEM_EXTREME reduce
- * memory requirements with large number of sections.
- * 51 bits is the max physical real address on POWER9
- */
-#if defined(CONFIG_SPARSEMEM_VMEMMAP) && defined(CONFIG_SPARSEMEM_EXTREME) &&	\
-	defined (CONFIG_PPC_64K_PAGES)
-#define MAX_PHYSMEM_BITS        51
-#elif defined(CONFIG_SPARSEMEM)
-#define MAX_PHYSMEM_BITS        46
-#endif
-
 #ifdef CONFIG_PPC_BOOK3S_64
 #include <asm/book3s/64/mmu.h>
 #else /* CONFIG_PPC_BOOK3S_64 */
diff --git a/arch/powerpc/include/asm/nohash/64/mmu.h b/arch/powerpc/include/asm/nohash/64/mmu.h
index e6585480dfc4..81cf30c370e5 100644
--- a/arch/powerpc/include/asm/nohash/64/mmu.h
+++ b/arch/powerpc/include/asm/nohash/64/mmu.h
@@ -2,6 +2,8 @@
 #ifndef _ASM_POWERPC_NOHASH_64_MMU_H_
 #define _ASM_POWERPC_NOHASH_64_MMU_H_
 
+#define MAX_PHYSMEM_BITS        44
+
 /* Freescale Book-E software loaded TLB or Book-3e (ISA 2.06+) MMU */
 #include <asm/nohash/mmu-book3e.h>
 
-- 
2.20.1


^ permalink raw reply related

* Re: VLC doesn't play videos anymore since the PowerPC fixes 5.1-3
From: Christophe Leroy @ 2019-04-03  5:05 UTC (permalink / raw)
  To: Christian Zigotzky, linuxppc-dev
In-Reply-To: <19FEC425-987D-4B95-AEFD-FF7C05990242@xenosoft.de>

Le 03/04/2019 à 05:52, Christian Zigotzky a écrit :
> Please test VLC with the RC3 of kernel 5.1.
> 
> The removing of the PowerPC fixes 5.1-3 has solved the VLC issue. Another user has already confirmed that [1]. This isn’t an April Fool‘s. ;-)

Could you bisect to identify the guilty commit ?

Thanks
Christophe

> 
> Thanks
> 
> [1] http://forum.hyperion-entertainment.com/viewtopic.php?f=58&t=4256&start=20#p47561
> 

^ permalink raw reply

* Re: [PATCH 5/6] powerpc/mmu: drop mmap_sem now that locked_vm is atomic
From: Christophe Leroy @ 2019-04-03  4:58 UTC (permalink / raw)
  To: Daniel Jordan, akpm
  Cc: Davidlohr Bueso, Alexey Kardashevskiy, linux-kernel, linux-mm,
	Paul Mackerras, Christoph Lameter, linuxppc-dev
In-Reply-To: <20190402204158.27582-6-daniel.m.jordan@oracle.com>



Le 02/04/2019 à 22:41, Daniel Jordan a écrit :
> With locked_vm now an atomic, there is no need to take mmap_sem as
> writer.  Delete and refactor accordingly.

Could you please detail the change ? It looks like this is not the only 
change. I'm wondering what the consequences are.

Before we did:
- lock
- calculate future value
- check the future value is acceptable
- update value if future value acceptable
- return error if future value non acceptable
- unlock

Now we do:
- atomic update with future (possibly too high) value
- check the new value is acceptable
- atomic update back with older value if new value not acceptable and 
return error

So if a concurrent action wants to increase locked_vm with an acceptable 
step while another one has temporarily set it too high, it will now fail.

I think we should keep the previous approach and do a cmpxchg after 
validating the new value.

Christophe

> 
> Signed-off-by: Daniel Jordan <daniel.m.jordan@oracle.com>
> Cc: Alexey Kardashevskiy <aik@ozlabs.ru>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Christoph Lameter <cl@linux.com>
> Cc: Davidlohr Bueso <dave@stgolabs.net>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: <linux-mm@kvack.org>
> Cc: <linuxppc-dev@lists.ozlabs.org>
> Cc: <linux-kernel@vger.kernel.org>
> ---
>   arch/powerpc/mm/mmu_context_iommu.c | 27 +++++++++++----------------
>   1 file changed, 11 insertions(+), 16 deletions(-)
> 
> diff --git a/arch/powerpc/mm/mmu_context_iommu.c b/arch/powerpc/mm/mmu_context_iommu.c
> index 8038ac24a312..a4ef22b67c07 100644
> --- a/arch/powerpc/mm/mmu_context_iommu.c
> +++ b/arch/powerpc/mm/mmu_context_iommu.c
> @@ -54,34 +54,29 @@ struct mm_iommu_table_group_mem_t {
>   static long mm_iommu_adjust_locked_vm(struct mm_struct *mm,
>   		unsigned long npages, bool incr)
>   {
> -	long ret = 0, locked, lock_limit;
> +	long ret = 0;
> +	unsigned long lock_limit;
>   	s64 locked_vm;
>   
>   	if (!npages)
>   		return 0;
>   
> -	down_write(&mm->mmap_sem);
> -	locked_vm = atomic64_read(&mm->locked_vm);
>   	if (incr) {
> -		locked = locked_vm + npages;
>   		lock_limit = rlimit(RLIMIT_MEMLOCK) >> PAGE_SHIFT;
> -		if (locked > lock_limit && !capable(CAP_IPC_LOCK))
> +		locked_vm = atomic64_add_return(npages, &mm->locked_vm);
> +		if (locked_vm > lock_limit && !capable(CAP_IPC_LOCK)) {
>   			ret = -ENOMEM;
> -		else
> -			atomic64_add(npages, &mm->locked_vm);
> +			atomic64_sub(npages, &mm->locked_vm);
> +		}
>   	} else {
> -		if (WARN_ON_ONCE(npages > locked_vm))
> -			npages = locked_vm;
> -		atomic64_sub(npages, &mm->locked_vm);
> +		locked_vm = atomic64_sub_return(npages, &mm->locked_vm);
> +		WARN_ON_ONCE(locked_vm < 0);
>   	}
>   
> -	pr_debug("[%d] RLIMIT_MEMLOCK HASH64 %c%ld %ld/%ld\n",
> -			current ? current->pid : 0,
> -			incr ? '+' : '-',
> -			npages << PAGE_SHIFT,
> -			atomic64_read(&mm->locked_vm) << PAGE_SHIFT,
> +	pr_debug("[%d] RLIMIT_MEMLOCK HASH64 %c%lu %lld/%lu\n",
> +			current ? current->pid : 0, incr ? '+' : '-',
> +			npages << PAGE_SHIFT, locked_vm << PAGE_SHIFT,
>   			rlimit(RLIMIT_MEMLOCK));
> -	up_write(&mm->mmap_sem);
>   
>   	return ret;
>   }
> 

^ permalink raw reply

* Re: [PATCH 1/6] mm: change locked_vm's type from unsigned long to atomic64_t
From: Christophe Leroy @ 2019-04-03  4:46 UTC (permalink / raw)
  To: Daniel Jordan, akpm
  Cc: Davidlohr Bueso, kvm, Alan Tull, Alexey Kardashevskiy, linux-fpga,
	linux-kernel, kvm-ppc, linux-mm, Alex Williamson, Moritz Fischer,
	Christoph Lameter, linuxppc-dev, Wu Hao
In-Reply-To: <20190402204158.27582-2-daniel.m.jordan@oracle.com>



Le 02/04/2019 à 22:41, Daniel Jordan a écrit :
> Taking and dropping mmap_sem to modify a single counter, locked_vm, is
> overkill when the counter could be synchronized separately.
> 
> Make mmap_sem a little less coarse by changing locked_vm to an atomic,
> the 64-bit variety to avoid issues with overflow on 32-bit systems.

Can you elaborate on the above ? Previously it was 'unsigned long', what 
were the issues ? If there was such issues, shouldn't there be a first 
patch moving it from unsigned long to u64 before this atomic64_t change 
? Or at least it should be clearly explain here what the issues are and 
how switching to a 64 bit counter fixes them.

Christophe

> 
> Signed-off-by: Daniel Jordan <daniel.m.jordan@oracle.com>
> Cc: Alan Tull <atull@kernel.org>
> Cc: Alexey Kardashevskiy <aik@ozlabs.ru>
> Cc: Alex Williamson <alex.williamson@redhat.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Christoph Lameter <cl@linux.com>
> Cc: Davidlohr Bueso <dave@stgolabs.net>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Moritz Fischer <mdf@kernel.org>
> Cc: Paul Mackerras <paulus@ozlabs.org>
> Cc: Wu Hao <hao.wu@intel.com>
> Cc: <linux-mm@kvack.org>
> Cc: <kvm@vger.kernel.org>
> Cc: <kvm-ppc@vger.kernel.org>
> Cc: <linuxppc-dev@lists.ozlabs.org>
> Cc: <linux-fpga@vger.kernel.org>
> Cc: <linux-kernel@vger.kernel.org>
> ---
>   arch/powerpc/kvm/book3s_64_vio.c    | 14 ++++++++------
>   arch/powerpc/mm/mmu_context_iommu.c | 15 ++++++++-------
>   drivers/fpga/dfl-afu-dma-region.c   | 18 ++++++++++--------
>   drivers/vfio/vfio_iommu_spapr_tce.c | 17 +++++++++--------
>   drivers/vfio/vfio_iommu_type1.c     | 10 ++++++----
>   fs/proc/task_mmu.c                  |  2 +-
>   include/linux/mm_types.h            |  2 +-
>   kernel/fork.c                       |  2 +-
>   mm/debug.c                          |  5 +++--
>   mm/mlock.c                          |  4 ++--
>   mm/mmap.c                           | 18 +++++++++---------
>   mm/mremap.c                         |  6 +++---
>   12 files changed, 61 insertions(+), 52 deletions(-)
> 
> diff --git a/arch/powerpc/kvm/book3s_64_vio.c b/arch/powerpc/kvm/book3s_64_vio.c
> index f02b04973710..e7fdb6d10eeb 100644
> --- a/arch/powerpc/kvm/book3s_64_vio.c
> +++ b/arch/powerpc/kvm/book3s_64_vio.c
> @@ -59,32 +59,34 @@ static unsigned long kvmppc_stt_pages(unsigned long tce_pages)
>   static long kvmppc_account_memlimit(unsigned long stt_pages, bool inc)
>   {
>   	long ret = 0;
> +	s64 locked_vm;
>   
>   	if (!current || !current->mm)
>   		return ret; /* process exited */
>   
>   	down_write(&current->mm->mmap_sem);
>   
> +	locked_vm = atomic64_read(&current->mm->locked_vm);
>   	if (inc) {
>   		unsigned long locked, lock_limit;
>   
> -		locked = current->mm->locked_vm + stt_pages;
> +		locked = locked_vm + stt_pages;
>   		lock_limit = rlimit(RLIMIT_MEMLOCK) >> PAGE_SHIFT;
>   		if (locked > lock_limit && !capable(CAP_IPC_LOCK))
>   			ret = -ENOMEM;
>   		else
> -			current->mm->locked_vm += stt_pages;
> +			atomic64_add(stt_pages, &current->mm->locked_vm);
>   	} else {
> -		if (WARN_ON_ONCE(stt_pages > current->mm->locked_vm))
> -			stt_pages = current->mm->locked_vm;
> +		if (WARN_ON_ONCE(stt_pages > locked_vm))
> +			stt_pages = locked_vm;
>   
> -		current->mm->locked_vm -= stt_pages;
> +		atomic64_sub(stt_pages, &current->mm->locked_vm);
>   	}
>   
>   	pr_debug("[%d] RLIMIT_MEMLOCK KVM %c%ld %ld/%ld%s\n", current->pid,
>   			inc ? '+' : '-',
>   			stt_pages << PAGE_SHIFT,
> -			current->mm->locked_vm << PAGE_SHIFT,
> +			atomic64_read(&current->mm->locked_vm) << PAGE_SHIFT,
>   			rlimit(RLIMIT_MEMLOCK),
>   			ret ? " - exceeded" : "");
>   
> diff --git a/arch/powerpc/mm/mmu_context_iommu.c b/arch/powerpc/mm/mmu_context_iommu.c
> index e7a9c4f6bfca..8038ac24a312 100644
> --- a/arch/powerpc/mm/mmu_context_iommu.c
> +++ b/arch/powerpc/mm/mmu_context_iommu.c
> @@ -55,30 +55,31 @@ static long mm_iommu_adjust_locked_vm(struct mm_struct *mm,
>   		unsigned long npages, bool incr)
>   {
>   	long ret = 0, locked, lock_limit;
> +	s64 locked_vm;
>   
>   	if (!npages)
>   		return 0;
>   
>   	down_write(&mm->mmap_sem);
> -
> +	locked_vm = atomic64_read(&mm->locked_vm);
>   	if (incr) {
> -		locked = mm->locked_vm + npages;
> +		locked = locked_vm + npages;
>   		lock_limit = rlimit(RLIMIT_MEMLOCK) >> PAGE_SHIFT;
>   		if (locked > lock_limit && !capable(CAP_IPC_LOCK))
>   			ret = -ENOMEM;
>   		else
> -			mm->locked_vm += npages;
> +			atomic64_add(npages, &mm->locked_vm);
>   	} else {
> -		if (WARN_ON_ONCE(npages > mm->locked_vm))
> -			npages = mm->locked_vm;
> -		mm->locked_vm -= npages;
> +		if (WARN_ON_ONCE(npages > locked_vm))
> +			npages = locked_vm;
> +		atomic64_sub(npages, &mm->locked_vm);
>   	}
>   
>   	pr_debug("[%d] RLIMIT_MEMLOCK HASH64 %c%ld %ld/%ld\n",
>   			current ? current->pid : 0,
>   			incr ? '+' : '-',
>   			npages << PAGE_SHIFT,
> -			mm->locked_vm << PAGE_SHIFT,
> +			atomic64_read(&mm->locked_vm) << PAGE_SHIFT,
>   			rlimit(RLIMIT_MEMLOCK));
>   	up_write(&mm->mmap_sem);
>   
> diff --git a/drivers/fpga/dfl-afu-dma-region.c b/drivers/fpga/dfl-afu-dma-region.c
> index e18a786fc943..08132fd9b6b7 100644
> --- a/drivers/fpga/dfl-afu-dma-region.c
> +++ b/drivers/fpga/dfl-afu-dma-region.c
> @@ -45,6 +45,7 @@ void afu_dma_region_init(struct dfl_feature_platform_data *pdata)
>   static int afu_dma_adjust_locked_vm(struct device *dev, long npages, bool incr)
>   {
>   	unsigned long locked, lock_limit;
> +	s64 locked_vm;
>   	int ret = 0;
>   
>   	/* the task is exiting. */
> @@ -53,24 +54,25 @@ static int afu_dma_adjust_locked_vm(struct device *dev, long npages, bool incr)
>   
>   	down_write(&current->mm->mmap_sem);
>   
> +	locked_vm = atomic64_read(&current->mm->locked_vm);
>   	if (incr) {
> -		locked = current->mm->locked_vm + npages;
> +		locked = locked_vm + npages;
>   		lock_limit = rlimit(RLIMIT_MEMLOCK) >> PAGE_SHIFT;
>   
>   		if (locked > lock_limit && !capable(CAP_IPC_LOCK))
>   			ret = -ENOMEM;
>   		else
> -			current->mm->locked_vm += npages;
> +			atomic64_add(npages, &current->mm->locked_vm);
>   	} else {
> -		if (WARN_ON_ONCE(npages > current->mm->locked_vm))
> -			npages = current->mm->locked_vm;
> -		current->mm->locked_vm -= npages;
> +		if (WARN_ON_ONCE(npages > locked_vm))
> +			npages = locked_vm;
> +		atomic64_sub(npages, &current->mm->locked_vm);
>   	}
>   
> -	dev_dbg(dev, "[%d] RLIMIT_MEMLOCK %c%ld %ld/%ld%s\n", current->pid,
> +	dev_dbg(dev, "[%d] RLIMIT_MEMLOCK %c%ld %lld/%lu%s\n", current->pid,
>   		incr ? '+' : '-', npages << PAGE_SHIFT,
> -		current->mm->locked_vm << PAGE_SHIFT, rlimit(RLIMIT_MEMLOCK),
> -		ret ? "- exceeded" : "");
> +		(s64)atomic64_read(&current->mm->locked_vm) << PAGE_SHIFT,
> +		rlimit(RLIMIT_MEMLOCK), ret ? "- exceeded" : "");
>   
>   	up_write(&current->mm->mmap_sem);
>   
> diff --git a/drivers/vfio/vfio_iommu_spapr_tce.c b/drivers/vfio/vfio_iommu_spapr_tce.c
> index 8dbb270998f4..e7d787e5d839 100644
> --- a/drivers/vfio/vfio_iommu_spapr_tce.c
> +++ b/drivers/vfio/vfio_iommu_spapr_tce.c
> @@ -36,7 +36,8 @@ static void tce_iommu_detach_group(void *iommu_data,
>   
>   static long try_increment_locked_vm(struct mm_struct *mm, long npages)
>   {
> -	long ret = 0, locked, lock_limit;
> +	long ret = 0, lock_limit;
> +	s64 locked;
>   
>   	if (WARN_ON_ONCE(!mm))
>   		return -EPERM;
> @@ -45,16 +46,16 @@ static long try_increment_locked_vm(struct mm_struct *mm, long npages)
>   		return 0;
>   
>   	down_write(&mm->mmap_sem);
> -	locked = mm->locked_vm + npages;
> +	locked = atomic64_read(&mm->locked_vm) + npages;
>   	lock_limit = rlimit(RLIMIT_MEMLOCK) >> PAGE_SHIFT;
>   	if (locked > lock_limit && !capable(CAP_IPC_LOCK))
>   		ret = -ENOMEM;
>   	else
> -		mm->locked_vm += npages;
> +		atomic64_add(npages, &mm->locked_vm);
>   
>   	pr_debug("[%d] RLIMIT_MEMLOCK +%ld %ld/%ld%s\n", current->pid,
>   			npages << PAGE_SHIFT,
> -			mm->locked_vm << PAGE_SHIFT,
> +			atomic64_read(&mm->locked_vm) << PAGE_SHIFT,
>   			rlimit(RLIMIT_MEMLOCK),
>   			ret ? " - exceeded" : "");
>   
> @@ -69,12 +70,12 @@ static void decrement_locked_vm(struct mm_struct *mm, long npages)
>   		return;
>   
>   	down_write(&mm->mmap_sem);
> -	if (WARN_ON_ONCE(npages > mm->locked_vm))
> -		npages = mm->locked_vm;
> -	mm->locked_vm -= npages;
> +	if (WARN_ON_ONCE(npages > atomic64_read(&mm->locked_vm)))
> +		npages = atomic64_read(&mm->locked_vm);
> +	atomic64_sub(npages, &mm->locked_vm);
>   	pr_debug("[%d] RLIMIT_MEMLOCK -%ld %ld/%ld\n", current->pid,
>   			npages << PAGE_SHIFT,
> -			mm->locked_vm << PAGE_SHIFT,
> +			atomic64_read(&mm->locked_vm) << PAGE_SHIFT,
>   			rlimit(RLIMIT_MEMLOCK));
>   	up_write(&mm->mmap_sem);
>   }
> diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
> index 73652e21efec..5b2878697286 100644
> --- a/drivers/vfio/vfio_iommu_type1.c
> +++ b/drivers/vfio/vfio_iommu_type1.c
> @@ -270,18 +270,19 @@ static int vfio_lock_acct(struct vfio_dma *dma, long npage, bool async)
>   	if (!ret) {
>   		if (npage > 0) {
>   			if (!dma->lock_cap) {
> +				s64 locked_vm = atomic64_read(&mm->locked_vm);
>   				unsigned long limit;
>   
>   				limit = task_rlimit(dma->task,
>   						RLIMIT_MEMLOCK) >> PAGE_SHIFT;
>   
> -				if (mm->locked_vm + npage > limit)
> +				if (locked_vm + npage > limit)
>   					ret = -ENOMEM;
>   			}
>   		}
>   
>   		if (!ret)
> -			mm->locked_vm += npage;
> +			atomic64_add(npage, &mm->locked_vm);
>   
>   		up_write(&mm->mmap_sem);
>   	}
> @@ -401,6 +402,7 @@ static long vfio_pin_pages_remote(struct vfio_dma *dma, unsigned long vaddr,
>   	long ret, pinned = 0, lock_acct = 0;
>   	bool rsvd;
>   	dma_addr_t iova = vaddr - dma->vaddr + dma->iova;
> +	atomic64_t *locked_vm = &current->mm->locked_vm;
>   
>   	/* This code path is only user initiated */
>   	if (!current->mm)
> @@ -418,7 +420,7 @@ static long vfio_pin_pages_remote(struct vfio_dma *dma, unsigned long vaddr,
>   	 * pages are already counted against the user.
>   	 */
>   	if (!rsvd && !vfio_find_vpfn(dma, iova)) {
> -		if (!dma->lock_cap && current->mm->locked_vm + 1 > limit) {
> +		if (!dma->lock_cap && atomic64_read(locked_vm) + 1 > limit) {
>   			put_pfn(*pfn_base, dma->prot);
>   			pr_warn("%s: RLIMIT_MEMLOCK (%ld) exceeded\n", __func__,
>   					limit << PAGE_SHIFT);
> @@ -445,7 +447,7 @@ static long vfio_pin_pages_remote(struct vfio_dma *dma, unsigned long vaddr,
>   
>   		if (!rsvd && !vfio_find_vpfn(dma, iova)) {
>   			if (!dma->lock_cap &&
> -			    current->mm->locked_vm + lock_acct + 1 > limit) {
> +			    atomic64_read(locked_vm) + lock_acct + 1 > limit) {
>   				put_pfn(pfn, dma->prot);
>   				pr_warn("%s: RLIMIT_MEMLOCK (%ld) exceeded\n",
>   					__func__, limit << PAGE_SHIFT);
> diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
> index 92a91e7816d8..61da4b24d0e0 100644
> --- a/fs/proc/task_mmu.c
> +++ b/fs/proc/task_mmu.c
> @@ -58,7 +58,7 @@ void task_mem(struct seq_file *m, struct mm_struct *mm)
>   	swap = get_mm_counter(mm, MM_SWAPENTS);
>   	SEQ_PUT_DEC("VmPeak:\t", hiwater_vm);
>   	SEQ_PUT_DEC(" kB\nVmSize:\t", total_vm);
> -	SEQ_PUT_DEC(" kB\nVmLck:\t", mm->locked_vm);
> +	SEQ_PUT_DEC(" kB\nVmLck:\t", atomic64_read(&mm->locked_vm));
>   	SEQ_PUT_DEC(" kB\nVmPin:\t", atomic64_read(&mm->pinned_vm));
>   	SEQ_PUT_DEC(" kB\nVmHWM:\t", hiwater_rss);
>   	SEQ_PUT_DEC(" kB\nVmRSS:\t", total_rss);
> diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> index 7eade9132f02..5059b99a0827 100644
> --- a/include/linux/mm_types.h
> +++ b/include/linux/mm_types.h
> @@ -410,7 +410,7 @@ struct mm_struct {
>   		unsigned long hiwater_vm;  /* High-water virtual memory usage */
>   
>   		unsigned long total_vm;	   /* Total pages mapped */
> -		unsigned long locked_vm;   /* Pages that have PG_mlocked set */
> +		atomic64_t    locked_vm;   /* Pages that have PG_mlocked set */
>   		atomic64_t    pinned_vm;   /* Refcount permanently increased */
>   		unsigned long data_vm;	   /* VM_WRITE & ~VM_SHARED & ~VM_STACK */
>   		unsigned long exec_vm;	   /* VM_EXEC & ~VM_WRITE & ~VM_STACK */
> diff --git a/kernel/fork.c b/kernel/fork.c
> index 9dcd18aa210b..56be8cdc7b4a 100644
> --- a/kernel/fork.c
> +++ b/kernel/fork.c
> @@ -979,7 +979,7 @@ static struct mm_struct *mm_init(struct mm_struct *mm, struct task_struct *p,
>   	mm->core_state = NULL;
>   	mm_pgtables_bytes_init(mm);
>   	mm->map_count = 0;
> -	mm->locked_vm = 0;
> +	atomic64_set(&mm->locked_vm, 0);
>   	atomic64_set(&mm->pinned_vm, 0);
>   	memset(&mm->rss_stat, 0, sizeof(mm->rss_stat));
>   	spin_lock_init(&mm->page_table_lock);
> diff --git a/mm/debug.c b/mm/debug.c
> index eee9c221280c..b9cd71927d3c 100644
> --- a/mm/debug.c
> +++ b/mm/debug.c
> @@ -136,7 +136,7 @@ void dump_mm(const struct mm_struct *mm)
>   #endif
>   		"mmap_base %lu mmap_legacy_base %lu highest_vm_end %lu\n"
>   		"pgd %px mm_users %d mm_count %d pgtables_bytes %lu map_count %d\n"
> -		"hiwater_rss %lx hiwater_vm %lx total_vm %lx locked_vm %lx\n"
> +		"hiwater_rss %lx hiwater_vm %lx total_vm %lx locked_vm %llx\n"
>   		"pinned_vm %llx data_vm %lx exec_vm %lx stack_vm %lx\n"
>   		"start_code %lx end_code %lx start_data %lx end_data %lx\n"
>   		"start_brk %lx brk %lx start_stack %lx\n"
> @@ -167,7 +167,8 @@ void dump_mm(const struct mm_struct *mm)
>   		atomic_read(&mm->mm_count),
>   		mm_pgtables_bytes(mm),
>   		mm->map_count,
> -		mm->hiwater_rss, mm->hiwater_vm, mm->total_vm, mm->locked_vm,
> +		mm->hiwater_rss, mm->hiwater_vm, mm->total_vm,
> +		(u64)atomic64_read(&mm->locked_vm),
>   		(u64)atomic64_read(&mm->pinned_vm),
>   		mm->data_vm, mm->exec_vm, mm->stack_vm,
>   		mm->start_code, mm->end_code, mm->start_data, mm->end_data,
> diff --git a/mm/mlock.c b/mm/mlock.c
> index 080f3b36415b..e492a155c51a 100644
> --- a/mm/mlock.c
> +++ b/mm/mlock.c
> @@ -562,7 +562,7 @@ static int mlock_fixup(struct vm_area_struct *vma, struct vm_area_struct **prev,
>   		nr_pages = -nr_pages;
>   	else if (old_flags & VM_LOCKED)
>   		nr_pages = 0;
> -	mm->locked_vm += nr_pages;
> +	atomic64_add(nr_pages, &mm->locked_vm);
>   
>   	/*
>   	 * vm_flags is protected by the mmap_sem held in write mode.
> @@ -687,7 +687,7 @@ static __must_check int do_mlock(unsigned long start, size_t len, vm_flags_t fla
>   	if (down_write_killable(&current->mm->mmap_sem))
>   		return -EINTR;
>   
> -	locked += current->mm->locked_vm;
> +	locked += atomic64_read(&current->mm->locked_vm);
>   	if ((locked > lock_limit) && (!capable(CAP_IPC_LOCK))) {
>   		/*
>   		 * It is possible that the regions requested intersect with
> diff --git a/mm/mmap.c b/mm/mmap.c
> index 41eb48d9b527..03576c1d530c 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -1339,7 +1339,7 @@ static inline int mlock_future_check(struct mm_struct *mm,
>   	/*  mlock MCL_FUTURE? */
>   	if (flags & VM_LOCKED) {
>   		locked = len >> PAGE_SHIFT;
> -		locked += mm->locked_vm;
> +		locked += atomic64_read(&mm->locked_vm);
>   		lock_limit = rlimit(RLIMIT_MEMLOCK);
>   		lock_limit >>= PAGE_SHIFT;
>   		if (locked > lock_limit && !capable(CAP_IPC_LOCK))
> @@ -1825,7 +1825,7 @@ unsigned long mmap_region(struct file *file, unsigned long addr,
>   					vma == get_gate_vma(current->mm))
>   			vma->vm_flags &= VM_LOCKED_CLEAR_MASK;
>   		else
> -			mm->locked_vm += (len >> PAGE_SHIFT);
> +			atomic64_add(len >> PAGE_SHIFT, &mm->locked_vm);
>   	}
>   
>   	if (file)
> @@ -2301,7 +2301,7 @@ static int acct_stack_growth(struct vm_area_struct *vma,
>   	if (vma->vm_flags & VM_LOCKED) {
>   		unsigned long locked;
>   		unsigned long limit;
> -		locked = mm->locked_vm + grow;
> +		locked = atomic64_read(&mm->locked_vm) + grow;
>   		limit = rlimit(RLIMIT_MEMLOCK);
>   		limit >>= PAGE_SHIFT;
>   		if (locked > limit && !capable(CAP_IPC_LOCK))
> @@ -2395,7 +2395,7 @@ int expand_upwards(struct vm_area_struct *vma, unsigned long address)
>   				 */
>   				spin_lock(&mm->page_table_lock);
>   				if (vma->vm_flags & VM_LOCKED)
> -					mm->locked_vm += grow;
> +					atomic64_add(grow, &mm->locked_vm);
>   				vm_stat_account(mm, vma->vm_flags, grow);
>   				anon_vma_interval_tree_pre_update_vma(vma);
>   				vma->vm_end = address;
> @@ -2475,7 +2475,7 @@ int expand_downwards(struct vm_area_struct *vma,
>   				 */
>   				spin_lock(&mm->page_table_lock);
>   				if (vma->vm_flags & VM_LOCKED)
> -					mm->locked_vm += grow;
> +					atomic64_add(grow, &mm->locked_vm);
>   				vm_stat_account(mm, vma->vm_flags, grow);
>   				anon_vma_interval_tree_pre_update_vma(vma);
>   				vma->vm_start = address;
> @@ -2796,11 +2796,11 @@ int __do_munmap(struct mm_struct *mm, unsigned long start, size_t len,
>   	/*
>   	 * unlock any mlock()ed ranges before detaching vmas
>   	 */
> -	if (mm->locked_vm) {
> +	if (atomic64_read(&mm->locked_vm)) {
>   		struct vm_area_struct *tmp = vma;
>   		while (tmp && tmp->vm_start < end) {
>   			if (tmp->vm_flags & VM_LOCKED) {
> -				mm->locked_vm -= vma_pages(tmp);
> +				atomic64_sub(vma_pages(tmp), &mm->locked_vm);
>   				munlock_vma_pages_all(tmp);
>   			}
>   
> @@ -3043,7 +3043,7 @@ static int do_brk_flags(unsigned long addr, unsigned long len, unsigned long fla
>   	mm->total_vm += len >> PAGE_SHIFT;
>   	mm->data_vm += len >> PAGE_SHIFT;
>   	if (flags & VM_LOCKED)
> -		mm->locked_vm += (len >> PAGE_SHIFT);
> +		atomic64_add(len >> PAGE_SHIFT, &mm->locked_vm);
>   	vma->vm_flags |= VM_SOFTDIRTY;
>   	return 0;
>   }
> @@ -3115,7 +3115,7 @@ void exit_mmap(struct mm_struct *mm)
>   		up_write(&mm->mmap_sem);
>   	}
>   
> -	if (mm->locked_vm) {
> +	if (atomic64_read(&mm->locked_vm)) {
>   		vma = mm->mmap;
>   		while (vma) {
>   			if (vma->vm_flags & VM_LOCKED)
> diff --git a/mm/mremap.c b/mm/mremap.c
> index e3edef6b7a12..9a4046bb2875 100644
> --- a/mm/mremap.c
> +++ b/mm/mremap.c
> @@ -422,7 +422,7 @@ static unsigned long move_vma(struct vm_area_struct *vma,
>   	}
>   
>   	if (vm_flags & VM_LOCKED) {
> -		mm->locked_vm += new_len >> PAGE_SHIFT;
> +		atomic64_add(new_len >> PAGE_SHIFT, &mm->locked_vm);
>   		*locked = true;
>   	}
>   
> @@ -473,7 +473,7 @@ static struct vm_area_struct *vma_to_resize(unsigned long addr,
>   
>   	if (vma->vm_flags & VM_LOCKED) {
>   		unsigned long locked, lock_limit;
> -		locked = mm->locked_vm << PAGE_SHIFT;
> +		locked = atomic64_read(&mm->locked_vm) << PAGE_SHIFT;
>   		lock_limit = rlimit(RLIMIT_MEMLOCK);
>   		locked += new_len - old_len;
>   		if (locked > lock_limit && !capable(CAP_IPC_LOCK))
> @@ -679,7 +679,7 @@ SYSCALL_DEFINE5(mremap, unsigned long, addr, unsigned long, old_len,
>   
>   			vm_stat_account(mm, vma->vm_flags, pages);
>   			if (vma->vm_flags & VM_LOCKED) {
> -				mm->locked_vm += pages;
> +				atomic64_add(pages, &mm->locked_vm);
>   				locked = true;
>   				new_addr = addr;
>   			}
> 

^ permalink raw reply

* Re: [PATCH stable v4.14 13/32] powerpc/fsl: Add barrier_nospec implementation for NXP PowerPC Book3E
From: Joakim Tjernlund @ 2019-04-03  4:38 UTC (permalink / raw)
  To: mpe@ellerman.id.au, stable@vger.kernel.org,
	gregkh@linuxfoundation.org
  Cc: diana.craciun@nxp.com, linuxppc-dev@ozlabs.org, msuchanek@suse.de
In-Reply-To: <87zhp729jz.fsf@concordia.ellerman.id.au>

On Wed, 2019-04-03 at 11:53 +1100, Michael Ellerman wrote:
> 
> Joakim Tjernlund <Joakim.Tjernlund@infinera.com> writes:
> > On Tue, 2019-04-02 at 17:19 +1100, Michael Ellerman wrote:
> > > Joakim Tjernlund <Joakim.Tjernlund@infinera.com> writes:
> ...
> > > > Can I compile it away?
> > > 
> > > You can't actually, but you can disable it at runtime with
> > > "nospectre_v1" on the kernel command line.
> > > 
> > > We could make it a user selectable compile time option if you really
> > > want it to be.
> > 
> > I think yes. Considering that these patches are fairly untested and the impact
> > in the wild unknown. Requiring systems to change their boot config over night is
> > too fast.
> 
> OK. Just to be clear, you're actually using 4.14 on an NXP board and
> would actually use this option? I don't want to add another option just
> for a theoretical use case.

Correct, we use 4.14 on several custom boards using NXP CPUs and would appreciate
if I could control spectre with a build switch.

Thanks a lot!
               Jocke

^ permalink raw reply

* Re: [PATCH] powerpc/xmon: add read-only mode
From: Christophe Leroy @ 2019-04-03  4:15 UTC (permalink / raw)
  To: Christopher M Riedl, linuxppc-dev; +Cc: andonnel
In-Reply-To: <184679822.6025.1554262730223@privateemail.com>



Le 03/04/2019 à 05:38, Christopher M Riedl a écrit :
>> On March 29, 2019 at 3:41 AM Christophe Leroy <christophe.leroy@c-s.fr> wrote:
>>
>>
>>
>>
>> Le 29/03/2019 à 05:21, cmr a écrit :
>>> Operations which write to memory should be restricted on secure systems
>>> and optionally to avoid self-destructive behaviors.
>>>
>>> Add a config option, XMON_RO, to control default xmon behavior along
>>> with kernel cmdline options xmon=ro and xmon=rw for explicit control.
>>> The default is to enable read-only mode.
>>>
>>> The following xmon operations are affected:
>>> memops:
>>> 	disable memmove
>>> 	disable memset
>>> memex:
>>> 	no-op'd mwrite
>>> super_regs:
>>> 	no-op'd write_spr
>>> bpt_cmds:
>>> 	disable
>>> proc_call:
>>> 	disable
>>>
>>> Signed-off-by: cmr <cmr@informatik.wtf>
>>
>> A Fully qualified name should be used.
> 
> What do you mean by fully-qualified here? PPC_XMON_RO? (PPC_)XMON_READONLY?

I mean it should be

Signed-off-by: Christopher M Riedl <cmr@informatik.wtf>

instead of

Signed-off-by: cmr <cmr@informatik.wtf>

> 
>>
>>> ---
>>>    arch/powerpc/Kconfig.debug |  7 +++++++
>>>    arch/powerpc/xmon/xmon.c   | 24 ++++++++++++++++++++++++
>>>    2 files changed, 31 insertions(+)
>>>
>>> diff --git a/arch/powerpc/Kconfig.debug b/arch/powerpc/Kconfig.debug
>>> index 4e00cb0a5464..33cc01adf4cb 100644
>>> --- a/arch/powerpc/Kconfig.debug
>>> +++ b/arch/powerpc/Kconfig.debug
>>> @@ -117,6 +117,13 @@ config XMON_DISASSEMBLY
>>>    	  to say Y here, unless you're building for a memory-constrained
>>>    	  system.
>>>    
>>> +config XMON_RO
>>> +	bool "Set xmon read-only mode"
>>> +	depends on XMON
>>> +	default y
>>
>> Should it really be always default y ?
>> I would set default 'y' only when some security options are also set.
>>
> 
> This is a good point, I based this on an internal Slack suggestion but giving this more thought, disabling read-only mode by default makes more sense. I'm not sure what security options could be set though?
> 

Maybe starting with CONFIG_STRICT_KERNEL_RWX

Another point that may also be addressed by your patch is the definition 
of PAGE_KERNEL_TEXT:

#if defined(CONFIG_KGDB) || defined(CONFIG_XMON) || 
defined(CONFIG_BDI_SWITCH) ||\
	defined(CONFIG_KPROBES) || defined(CONFIG_DYNAMIC_FTRACE)
#define PAGE_KERNEL_TEXT	PAGE_KERNEL_X
#else
#define PAGE_KERNEL_TEXT	PAGE_KERNEL_ROX
#endif

The above let me think that it would be better if you add a config 
XMON_RW instead of XMON_RO, with default !STRICT_KERNEL_RWX

Christophe

^ permalink raw reply

* [PATCH kernel v2 1/2] powerpc/mm_iommu: Fix potential deadlock
From: Alexey Kardashevskiy @ 2019-04-03  4:12 UTC (permalink / raw)
  To: linuxppc-dev
  Cc: Alexey Kardashevskiy, Aneesh Kumar K.V, kvm-ppc, David Gibson
In-Reply-To: <20190403041233.57619-1-aik@ozlabs.ru>

Currently mm_iommu_do_alloc() is called in 2 cases:
- VFIO_IOMMU_SPAPR_REGISTER_MEMORY ioctl() for normal memory:
	this locks &mem_list_mutex and then locks mm::mmap_sem
	several times when adjusting locked_vm or pinning pages;
- vfio_pci_nvgpu_regops::mmap() for GPU memory:
	this is called with mm::mmap_sem held already and it locks
	&mem_list_mutex.

So one can craft a userspace program to do special ioctl and mmap in
2 threads concurrently and cause a deadlock which lockdep warns about
(below).

We did not hit this yet because QEMU constructs the machine in a single
thread.

This moves the overlap check next to where the new entry is added and
reduces the amount of time spent with &mem_list_mutex held.

This moves locked_vm adjustment from under &mem_list_mutex.

This relies on mm_iommu_adjust_locked_vm() doing nothing when entries==0.

This is one of the lockdep warnings:
======================================================
WARNING: possible circular locking dependency detected
5.1.0-rc2-le_nv2_aikATfstn1-p1 #363 Not tainted
------------------------------------------------------
qemu-system-ppc/8038 is trying to acquire lock:
000000002ec6c453 (mem_list_mutex){+.+.}, at: mm_iommu_do_alloc+0x70/0x490

but task is already holding lock:
00000000fd7da97f (&mm->mmap_sem){++++}, at: vm_mmap_pgoff+0xf0/0x160

which lock already depends on the new lock.

the existing dependency chain (in reverse order) is:

-> #1 (&mm->mmap_sem){++++}:
       lock_acquire+0xf8/0x260
       down_write+0x44/0xa0
       mm_iommu_adjust_locked_vm.part.1+0x4c/0x190
       mm_iommu_do_alloc+0x310/0x490
       tce_iommu_ioctl.part.9+0xb84/0x1150 [vfio_iommu_spapr_tce]
       vfio_fops_unl_ioctl+0x94/0x430 [vfio]
       do_vfs_ioctl+0xe4/0x930
       ksys_ioctl+0xc4/0x110
       sys_ioctl+0x28/0x80
       system_call+0x5c/0x70

-> #0 (mem_list_mutex){+.+.}:
       __lock_acquire+0x1484/0x1900
       lock_acquire+0xf8/0x260
       __mutex_lock+0x88/0xa70
       mm_iommu_do_alloc+0x70/0x490
       vfio_pci_nvgpu_mmap+0xc0/0x130 [vfio_pci]
       vfio_pci_mmap+0x198/0x2a0 [vfio_pci]
       vfio_device_fops_mmap+0x44/0x70 [vfio]
       mmap_region+0x5d4/0x770
       do_mmap+0x42c/0x650
       vm_mmap_pgoff+0x124/0x160
       ksys_mmap_pgoff+0xdc/0x2f0
       sys_mmap+0x40/0x80
       system_call+0x5c/0x70

other info that might help us debug this:

 Possible unsafe locking scenario:

       CPU0                    CPU1
       ----                    ----
  lock(&mm->mmap_sem);
                               lock(mem_list_mutex);
                               lock(&mm->mmap_sem);
  lock(mem_list_mutex);

 *** DEADLOCK ***

1 lock held by qemu-system-ppc/8038:
 #0: 00000000fd7da97f (&mm->mmap_sem){++++}, at: vm_mmap_pgoff+0xf0/0x160

Fixes: c10c21efa4bc ("powerpc/vfio/iommu/kvm: Do not pin device memory", 2018-12-19)
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
 arch/powerpc/mm/mmu_context_iommu.c | 75 +++++++++++++++--------------
 1 file changed, 39 insertions(+), 36 deletions(-)

diff --git a/arch/powerpc/mm/mmu_context_iommu.c b/arch/powerpc/mm/mmu_context_iommu.c
index e7a9c4f6bfca..9d9be850f8c2 100644
--- a/arch/powerpc/mm/mmu_context_iommu.c
+++ b/arch/powerpc/mm/mmu_context_iommu.c
@@ -95,28 +95,14 @@ static long mm_iommu_do_alloc(struct mm_struct *mm, unsigned long ua,
 			      unsigned long entries, unsigned long dev_hpa,
 			      struct mm_iommu_table_group_mem_t **pmem)
 {
-	struct mm_iommu_table_group_mem_t *mem;
-	long i, ret, locked_entries = 0;
+	struct mm_iommu_table_group_mem_t *mem, *mem2;
+	long i, ret, locked_entries = 0, pinned = 0;
 	unsigned int pageshift;
 
-	mutex_lock(&mem_list_mutex);
-
-	list_for_each_entry_rcu(mem, &mm->context.iommu_group_mem_list,
-			next) {
-		/* Overlap? */
-		if ((mem->ua < (ua + (entries << PAGE_SHIFT))) &&
-				(ua < (mem->ua +
-				       (mem->entries << PAGE_SHIFT)))) {
-			ret = -EINVAL;
-			goto unlock_exit;
-		}
-
-	}
-
 	if (dev_hpa == MM_IOMMU_TABLE_INVALID_HPA) {
 		ret = mm_iommu_adjust_locked_vm(mm, entries, true);
 		if (ret)
-			goto unlock_exit;
+			return ret;
 
 		locked_entries = entries;
 	}
@@ -150,15 +136,10 @@ static long mm_iommu_do_alloc(struct mm_struct *mm, unsigned long ua,
 	down_read(&mm->mmap_sem);
 	ret = get_user_pages_longterm(ua, entries, FOLL_WRITE, mem->hpages, NULL);
 	up_read(&mm->mmap_sem);
+	pinned = ret > 0 ? ret : 0;
 	if (ret != entries) {
-		/* free the reference taken */
-		for (i = 0; i < ret; i++)
-			put_page(mem->hpages[i]);
-
-		vfree(mem->hpas);
-		kfree(mem);
 		ret = -EFAULT;
-		goto unlock_exit;
+		goto free_exit;
 	}
 
 	pageshift = PAGE_SHIFT;
@@ -183,21 +164,43 @@ static long mm_iommu_do_alloc(struct mm_struct *mm, unsigned long ua,
 	}
 
 good_exit:
-	ret = 0;
 	atomic64_set(&mem->mapped, 1);
 	mem->used = 1;
 	mem->ua = ua;
 	mem->entries = entries;
-	*pmem = mem;
+
+	mutex_lock(&mem_list_mutex);
+
+	list_for_each_entry_rcu(mem2, &mm->context.iommu_group_mem_list, next) {
+		/* Overlap? */
+		if ((mem2->ua < (ua + (entries << PAGE_SHIFT))) &&
+				(ua < (mem2->ua +
+				       (mem2->entries << PAGE_SHIFT)))) {
+			ret = -EINVAL;
+			mutex_unlock(&mem_list_mutex);
+			goto free_exit;
+		}
+	}
 
 	list_add_rcu(&mem->next, &mm->context.iommu_group_mem_list);
 
-unlock_exit:
-	if (locked_entries && ret)
-		mm_iommu_adjust_locked_vm(mm, locked_entries, false);
-
 	mutex_unlock(&mem_list_mutex);
 
+	*pmem = mem;
+
+	return 0;
+
+free_exit:
+	/* free the reference taken */
+	for (i = 0; i < pinned; i++)
+		put_page(mem->hpages[i]);
+
+	vfree(mem->hpas);
+	kfree(mem);
+
+unlock_exit:
+	mm_iommu_adjust_locked_vm(mm, locked_entries, false);
+
 	return ret;
 }
 
@@ -266,7 +269,7 @@ static void mm_iommu_release(struct mm_iommu_table_group_mem_t *mem)
 long mm_iommu_put(struct mm_struct *mm, struct mm_iommu_table_group_mem_t *mem)
 {
 	long ret = 0;
-	unsigned long entries, dev_hpa;
+	unsigned long unlock_entries = 0;
 
 	mutex_lock(&mem_list_mutex);
 
@@ -287,17 +290,17 @@ long mm_iommu_put(struct mm_struct *mm, struct mm_iommu_table_group_mem_t *mem)
 		goto unlock_exit;
 	}
 
+	if (mem->dev_hpa == MM_IOMMU_TABLE_INVALID_HPA)
+		unlock_entries = mem->entries;
+
 	/* @mapped became 0 so now mappings are disabled, release the region */
-	entries = mem->entries;
-	dev_hpa = mem->dev_hpa;
 	mm_iommu_release(mem);
 
-	if (dev_hpa == MM_IOMMU_TABLE_INVALID_HPA)
-		mm_iommu_adjust_locked_vm(mm, entries, false);
-
 unlock_exit:
 	mutex_unlock(&mem_list_mutex);
 
+	mm_iommu_adjust_locked_vm(mm, unlock_entries, false);
+
 	return ret;
 }
 EXPORT_SYMBOL_GPL(mm_iommu_put);
-- 
2.17.1


^ permalink raw reply related

* [PATCH kernel v2 2/2] powerpc/mm_iommu: Allow pinning large regions
From: Alexey Kardashevskiy @ 2019-04-03  4:12 UTC (permalink / raw)
  To: linuxppc-dev
  Cc: Alexey Kardashevskiy, Aneesh Kumar K.V, kvm-ppc, David Gibson
In-Reply-To: <20190403041233.57619-1-aik@ozlabs.ru>

When called with vmas_arg==NULL, get_user_pages_longterm() allocates
an array of nr_pages*8 which can easily get greater that the max order,
for example, registering memory for a 256GB guest does this and fails
in __alloc_pages_nodemask().

This adds a loop over chunks of entries to fit the max order limit.

Fixes: 678e174c4c16 ("powerpc/mm/iommu: allow migration of cma allocated pages during mm_iommu_do_alloc", 2019-03-05)
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
 arch/powerpc/mm/mmu_context_iommu.c | 24 ++++++++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/mm/mmu_context_iommu.c b/arch/powerpc/mm/mmu_context_iommu.c
index 9d9be850f8c2..8330f135294f 100644
--- a/arch/powerpc/mm/mmu_context_iommu.c
+++ b/arch/powerpc/mm/mmu_context_iommu.c
@@ -98,6 +98,7 @@ static long mm_iommu_do_alloc(struct mm_struct *mm, unsigned long ua,
 	struct mm_iommu_table_group_mem_t *mem, *mem2;
 	long i, ret, locked_entries = 0, pinned = 0;
 	unsigned int pageshift;
+	unsigned long entry, chunk;
 
 	if (dev_hpa == MM_IOMMU_TABLE_INVALID_HPA) {
 		ret = mm_iommu_adjust_locked_vm(mm, entries, true);
@@ -134,11 +135,26 @@ static long mm_iommu_do_alloc(struct mm_struct *mm, unsigned long ua,
 	}
 
 	down_read(&mm->mmap_sem);
-	ret = get_user_pages_longterm(ua, entries, FOLL_WRITE, mem->hpages, NULL);
+	chunk = (1UL << (PAGE_SHIFT + MAX_ORDER - 1)) /
+			sizeof(struct vm_area_struct *);
+	chunk = min(chunk, entries);
+	for (entry = 0; entry < entries; entry += chunk) {
+		unsigned long n = min(entries - entry, chunk);
+
+		ret = get_user_pages_longterm(ua + (entry << PAGE_SHIFT), n,
+				FOLL_WRITE, mem->hpages + entry, NULL);
+		if (ret == n) {
+			pinned += n;
+			continue;
+		}
+		if (ret > 0)
+			pinned += ret;
+		break;
+	}
 	up_read(&mm->mmap_sem);
-	pinned = ret > 0 ? ret : 0;
-	if (ret != entries) {
-		ret = -EFAULT;
+	if (pinned != entries) {
+		if (!ret)
+			ret = -EFAULT;
 		goto free_exit;
 	}
 
-- 
2.17.1


^ permalink raw reply related

* [PATCH kernel v2 0/2] powerpc/mm_iommu: Fixes
From: Alexey Kardashevskiy @ 2019-04-03  4:12 UTC (permalink / raw)
  To: linuxppc-dev
  Cc: Alexey Kardashevskiy, Aneesh Kumar K.V, kvm-ppc, David Gibson

The patches do independent things but touch exact same code so
the order in which they should apply matters.

This supercedes:
[PATCH kernel] powerpc/mm_iommu: Allow pinning large regions
[PATCH kernel 1/2] powerpc/mm_iommu: Prepare for less locking
[PATCH kernel 2/2] powerpc/mm_iommu: Fix potential deadlock


This is based on sha1
5e7a8ca31926 Linus Torvalds "Merge branch 'work.aio' of git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs".

Please comment. Thanks.



Alexey Kardashevskiy (2):
  powerpc/mm_iommu: Fix potential deadlock
  powerpc/mm_iommu: Allow pinning large regions

 arch/powerpc/mm/mmu_context_iommu.c | 97 +++++++++++++++++------------
 1 file changed, 58 insertions(+), 39 deletions(-)

-- 
2.17.1


^ permalink raw reply

* VLC doesn't play videos anymore since the PowerPC fixes 5.1-3
From: Christian Zigotzky @ 2019-04-03  3:52 UTC (permalink / raw)
  To: linuxppc-dev

Please test VLC with the RC3 of kernel 5.1.

The removing of the PowerPC fixes 5.1-3 has solved the VLC issue. Another user has already confirmed that [1]. This isn’t an April Fool‘s. ;-)

Thanks

[1] http://forum.hyperion-entertainment.com/viewtopic.php?f=58&t=4256&start=20#p47561

^ permalink raw reply


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