LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [RFC V2] virtio: Add platform specific DMA API translation for virito devices
From: Benjamin Herrenschmidt @ 2018-05-23 22:27 UTC (permalink / raw)
  To: Michael S. Tsirkin, Anshuman Khandual
  Cc: virtualization, linux-kernel, linuxppc-dev, aik, robh, joe,
	elfring, david, jasowang, mpe, hch
In-Reply-To: <20180523213703-mutt-send-email-mst@kernel.org>

On Wed, 2018-05-23 at 21:50 +0300, Michael S. Tsirkin wrote:

> I re-read that discussion and I'm still unclear on the
> original question, since I got several apparently
> conflicting answers.
> 
> I asked:
> 
> 	Why isn't setting VIRTIO_F_IOMMU_PLATFORM on the
> 	hypervisor side sufficient?

I thought I had replied to this...

There are a couple of reasons:

- First qemu doesn't know that the guest will switch to "secure mode"
in advance. There is no difference between a normal and a secure
partition until the partition does the magic UV call to "enter secure
mode" and qemu doesn't see any of it. So who can set the flag here ?

- Second, when using VIRTIO_F_IOMMU_PLATFORM, we also make qemu (or
vhost) go through the emulated MMIO for every access to the guest,
which adds additional overhead.

Cheers,
Ben.

> 
> 
> >  arch/powerpc/include/asm/dma-mapping.h |  6 ++++++
> >  arch/powerpc/platforms/pseries/iommu.c | 11 +++++++++++
> >  drivers/virtio/virtio_ring.c           | 10 ++++++++++
> >  3 files changed, 27 insertions(+)
> > 
> > diff --git a/arch/powerpc/include/asm/dma-mapping.h b/arch/powerpc/include/asm/dma-mapping.h
> > index 8fa3945..056e578 100644
> > --- a/arch/powerpc/include/asm/dma-mapping.h
> > +++ b/arch/powerpc/include/asm/dma-mapping.h
> > @@ -115,4 +115,10 @@ extern u64 __dma_get_required_mask(struct device *dev);
> >  #define ARCH_HAS_DMA_MMAP_COHERENT
> >  
> >  #endif /* __KERNEL__ */
> > +
> > +#define platform_forces_virtio_dma platform_forces_virtio_dma
> > +
> > +struct virtio_device;
> > +
> > +extern bool platform_forces_virtio_dma(struct virtio_device *vdev);
> >  #endif	/* _ASM_DMA_MAPPING_H */
> > diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c
> > index 06f0296..a2ec15a 100644
> > --- a/arch/powerpc/platforms/pseries/iommu.c
> > +++ b/arch/powerpc/platforms/pseries/iommu.c
> > @@ -38,6 +38,7 @@
> >  #include <linux/of.h>
> >  #include <linux/iommu.h>
> >  #include <linux/rculist.h>
> > +#include <linux/virtio.h>
> >  #include <asm/io.h>
> >  #include <asm/prom.h>
> >  #include <asm/rtas.h>
> > @@ -1396,3 +1397,13 @@ static int __init disable_multitce(char *str)
> >  __setup("multitce=", disable_multitce);
> >  
> >  machine_subsys_initcall_sync(pseries, tce_iommu_bus_notifier_init);
> > +
> > +bool platform_forces_virtio_dma(struct virtio_device *vdev)
> > +{
> > +	/*
> > +	 * On protected guest platforms, force virtio core to use DMA
> > +	 * MAP API for all virtio devices. But there can also be some
> > +	 * exceptions for individual devices like virtio balloon.
> > +	 */
> > +	return (of_find_compatible_node(NULL, NULL, "ibm,ultravisor") != NULL);
> > +}
> 
> Isn't this kind of slow?  vring_use_dma_api is on
> data path and supposed to be very fast.
> 
> > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> > index 21d464a..47ea6c3 100644
> > --- a/drivers/virtio/virtio_ring.c
> > +++ b/drivers/virtio/virtio_ring.c
> > @@ -141,8 +141,18 @@ struct vring_virtqueue {
> >   * unconditionally on data path.
> >   */
> >  
> > +#ifndef platform_forces_virtio_dma
> > +static inline bool platform_forces_virtio_dma(struct virtio_device *vdev)
> > +{
> > +	return false;
> > +}
> > +#endif
> > +
> >  static bool vring_use_dma_api(struct virtio_device *vdev)
> >  {
> > +	if (platform_forces_virtio_dma(vdev))
> > +		return true;
> > +
> >  	if (!virtio_has_iommu_quirk(vdev))
> >  		return true;
> >  
> > -- 
> > 2.9.3

^ permalink raw reply

* Re: [PATCH 4/5] pci/hotplug/pnv-php: add missing of_node_put
From: Bjorn Helgaas @ 2018-05-23 21:50 UTC (permalink / raw)
  To: Julia Lawall
  Cc: Benjamin Herrenschmidt, kernel-janitors, Paul Mackerras,
	Michael Ellerman, Bjorn Helgaas, linuxppc-dev, linux-pci,
	linux-kernel
In-Reply-To: <1527102436-13447-5-git-send-email-Julia.Lawall@lip6.fr>

On Wed, May 23, 2018 at 09:07:15PM +0200, Julia Lawall wrote:
> The device node iterators perform an of_node_get on each iteration, so a
> jump out of the loop requires an of_node_put.
> 
> The semantic patch that fixes this problem is as follows
> (http://coccinelle.lip6.fr):
> 
> // <smpl>
> @@
> expression root,e;
> local idexpression child;
> iterator name for_each_child_of_node;
> @@
> 
>  for_each_child_of_node(root, child) {
>    ... when != of_node_put(child)
>        when != e = child
> +  of_node_put(child);
> ?  break;
>    ...
> }
> ... when != child
> // </smpl>
> 
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

Applied to pci/hotplug for v4.18, thanks!

> ---
>  drivers/pci/hotplug/pnv_php.c |    8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/hotplug/pnv_php.c b/drivers/pci/hotplug/pnv_php.c
> index d441006..6c2e8d7 100644
> --- a/drivers/pci/hotplug/pnv_php.c
> +++ b/drivers/pci/hotplug/pnv_php.c
> @@ -220,12 +220,16 @@ static int pnv_php_populate_changeset(struct of_changeset *ocs,
>  
>  	for_each_child_of_node(dn, child) {
>  		ret = of_changeset_attach_node(ocs, child);
> -		if (ret)
> +		if (ret) {
> +			of_node_put(child);
>  			break;
> +		}
>  
>  		ret = pnv_php_populate_changeset(ocs, child);
> -		if (ret)
> +		if (ret) {
> +			of_node_put(child);
>  			break;
> +		}
>  	}
>  
>  	return ret;
> 

^ permalink raw reply

* Re: [PATCH bpf-next v3 10/10] tools: bpftool: add delimiters to multi-function JITed dumps
From: Jakub Kicinski @ 2018-05-23 21:32 UTC (permalink / raw)
  To: Sandipan Das
  Cc: Daniel Borkmann, ast, netdev, linuxppc-dev, mpe, naveen.n.rao,
	Quentin Monnet
In-Reply-To: <7142939b-515e-50ac-bc0b-50444bf9cc97@linux.vnet.ibm.com>

On Wed, 23 May 2018 16:07:40 +0530, Sandipan Das wrote:
>         "name": "bpf_prog_196af774a3477707_F",
>         "insns": [{
>                 "pc": "0x0",
>                 "operation": "nop",
>                 "operands": [null
>                 ]
>             },{
>                 "pc": "0x4",
>                 "operation": "nop",
>                 "operands": [null
>                 ]
>             },{
>                 "pc": "0x8",
>                 "operation": "mflr",
>                 "operands": ["r0"
>                 ]
>             },{
>                 ...
>             }
>         ]
>     }
> ]
> 
> If this is okay, I can send out the next revision with these changes.

Looks good, thank you!

^ permalink raw reply

* Re: [PATCH 07/14] powerpc: Add support for restartable sequences
From: Mathieu Desnoyers @ 2018-05-23 21:29 UTC (permalink / raw)
  To: Boqun Feng
  Cc: Will Deacon, Peter Zijlstra, Paul E. McKenney, Andy Lutomirski,
	Dave Watson, linux-kernel, linux-api, Paul Turner, Andrew Morton,
	Russell King, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	Andrew Hunter, Andi Kleen, Chris Lameter, Ben Maurer, rostedt,
	Josh Triplett, Linus Torvalds, Catalin Marinas, Michael Kerrisk,
	Joel Fernandes, Benjamin Herrenschmidt, Paul Mackerras,
	Michael Ellerman, linuxppc-dev
In-Reply-To: <1928158599.1541.1527106479862.JavaMail.zimbra@efficios.com>

----- On May 23, 2018, at 4:14 PM, Mathieu Desnoyers mathieu.desnoyers@efficios.com wrote:

> ----- On May 20, 2018, at 10:08 AM, Boqun Feng boqun.feng@gmail.com wrote:
> 
>> On Fri, May 18, 2018 at 02:17:17PM -0400, Mathieu Desnoyers wrote:
>>> ----- On May 17, 2018, at 7:50 PM, Boqun Feng boqun.feng@gmail.com wrote:
>>> [...]
>>> >> > I think you're right. So we have to introduce callsite to rseq_syscall()
>>> >> > in syscall path, something like:
>>> >> > 
>>> >> > diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
>>> >> > index 51695608c68b..a25734a96640 100644
>>> >> > --- a/arch/powerpc/kernel/entry_64.S
>>> >> > +++ b/arch/powerpc/kernel/entry_64.S
>>> >> > @@ -222,6 +222,9 @@ system_call_exit:
>>> >> > 	mtmsrd	r11,1
>>> >> > #endif /* CONFIG_PPC_BOOK3E */
>>> >> > 
>>> >> > +	addi    r3,r1,STACK_FRAME_OVERHEAD
>>> >> > +	bl	rseq_syscall
>>> >> > +
>>> >> > 	ld	r9,TI_FLAGS(r12)
>>> >> > 	li	r11,-MAX_ERRNO
>>> >> > 	andi.
>>> >> > 		r0,r9,(_TIF_SYSCALL_DOTRACE|_TIF_SINGLESTEP|_TIF_USER_WORK_MASK|_TIF_PERSYSCALL_MASK)
>>> >> > 
>>> 
>>> By the way, I think this is not the right spot to call rseq_syscall, because
>>> interrupts are disabled. I think we should move this hunk right after
>>> system_call_exit.
>>> 
>> 
>> Good point.
>> 
>>> Would you like to implement and test an updated patch adding those calls for ppc
>>> 32 and 64 ?
>>> 
>> 
>> I'd like to help, but I don't have a handy ppc environment for test...
>> So I made the below patch which has only been build-tested, hope it
>> could be somewhat helpful.
> 
> Hi Boqun,
> 
> I tried your patch in a ppc64 le environment, and it does not survive boot
> with CONFIG_DEBUG_RSEQ=y. init gets killed right away.

The following fixup gets ppc64 to work:

--- a/arch/powerpc/kernel/entry_64.S
+++ b/arch/powerpc/kernel/entry_64.S
@@ -208,6 +208,7 @@ system_call_exit:
        /* Check whether the syscall is issued inside a restartable sequence */
        addi    r3,r1,STACK_FRAME_OVERHEAD
        bl      rseq_syscall
+       ld      r3,RESULT(r1)
 #endif
        /*
         * Disable interrupts so current_thread_info()->flags can't change,

> Moreover, I'm not sure that the r3 register don't contain something worth
> saving before the call on ppc32. Just after there is a "mr" instruction
> which AFAIU takes r3 as input register.

I'll start testing on ppc32 now.

Thanks,

Mathieu

> 
> Can you look into it ?
> 
> Thanks,
> 
> Mathieu
> 
>> 
>> Regards,
>> Boqun
>> 
>> --------------------------------->8
>> Subject: [PATCH] powerpc: Add syscall detection for restartable sequences
>> 
>> Syscalls are not allowed inside restartable sequences, so add a call to
>> rseq_syscall() at the very beginning of system call exiting path for
>> CONFIG_DEBUG_RSEQ=y kernel. This could help us to detect whether there
>> is a syscall issued inside restartable sequences.
>> 
>> Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
>> ---
>> arch/powerpc/kernel/entry_32.S | 5 +++++
>> arch/powerpc/kernel/entry_64.S | 5 +++++
>> 2 files changed, 10 insertions(+)
>> 
>> diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S
>> index eb8d01bae8c6..2f134eebe7ed 100644
>> --- a/arch/powerpc/kernel/entry_32.S
>> +++ b/arch/powerpc/kernel/entry_32.S
>> @@ -365,6 +365,11 @@ syscall_dotrace_cont:
>> 	blrl			/* Call handler */
>> 	.globl	ret_from_syscall
>> ret_from_syscall:
>> +#ifdef CONFIG_DEBUG_RSEQ
>> +	/* Check whether the syscall is issued inside a restartable sequence */
>> +	addi    r3,r1,STACK_FRAME_OVERHEAD
>> +	bl      rseq_syscall
>> +#endif
>> 	mr	r6,r3
>> 	CURRENT_THREAD_INFO(r12, r1)
>> 	/* disable interrupts so current_thread_info()->flags can't change */
>> diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
>> index 2cb5109a7ea3..2e2d59bb45d0 100644
>> --- a/arch/powerpc/kernel/entry_64.S
>> +++ b/arch/powerpc/kernel/entry_64.S
>> @@ -204,6 +204,11 @@ system_call:			/* label this so stack traces look sane */
>>  * This is blacklisted from kprobes further below with _ASM_NOKPROBE_SYMBOL().
>>  */
>> system_call_exit:
>> +#ifdef CONFIG_DEBUG_RSEQ
>> +	/* Check whether the syscall is issued inside a restartable sequence */
>> +	addi    r3,r1,STACK_FRAME_OVERHEAD
>> +	bl      rseq_syscall
>> +#endif
>> 	/*
>> 	 * Disable interrupts so current_thread_info()->flags can't change,
>> 	 * and so that we don't get interrupted after loading SRR0/1.
>> --
>> 2.16.2
> 
> --
> Mathieu Desnoyers
> EfficiOS Inc.
> http://www.efficios.com

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com

^ permalink raw reply

* Re: [PATCH 07/14] powerpc: Add support for restartable sequences
From: Paul E. McKenney @ 2018-05-23 20:46 UTC (permalink / raw)
  To: Mathieu Desnoyers
  Cc: Boqun Feng, Will Deacon, Peter Zijlstra, Andy Lutomirski,
	Dave Watson, linux-kernel, linux-api, Paul Turner, Andrew Morton,
	Russell King, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	Andrew Hunter, Andi Kleen, Chris Lameter, Ben Maurer, rostedt,
	Josh Triplett, Linus Torvalds, Catalin Marinas, Michael Kerrisk,
	Joel Fernandes, Benjamin Herrenschmidt, Paul Mackerras,
	Michael Ellerman, linuxppc-dev
In-Reply-To: <1928158599.1541.1527106479862.JavaMail.zimbra@efficios.com>

On Wed, May 23, 2018 at 04:14:39PM -0400, Mathieu Desnoyers wrote:
> ----- On May 20, 2018, at 10:08 AM, Boqun Feng boqun.feng@gmail.com wrote:
> 
> > On Fri, May 18, 2018 at 02:17:17PM -0400, Mathieu Desnoyers wrote:
> >> ----- On May 17, 2018, at 7:50 PM, Boqun Feng boqun.feng@gmail.com wrote:
> >> [...]
> >> >> > I think you're right. So we have to introduce callsite to rseq_syscall()
> >> >> > in syscall path, something like:
> >> >> > 
> >> >> > diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
> >> >> > index 51695608c68b..a25734a96640 100644
> >> >> > --- a/arch/powerpc/kernel/entry_64.S
> >> >> > +++ b/arch/powerpc/kernel/entry_64.S
> >> >> > @@ -222,6 +222,9 @@ system_call_exit:
> >> >> > 	mtmsrd	r11,1
> >> >> > #endif /* CONFIG_PPC_BOOK3E */
> >> >> > 
> >> >> > +	addi    r3,r1,STACK_FRAME_OVERHEAD
> >> >> > +	bl	rseq_syscall
> >> >> > +
> >> >> > 	ld	r9,TI_FLAGS(r12)
> >> >> > 	li	r11,-MAX_ERRNO
> >> >> > 	andi.
> >> >> > 		r0,r9,(_TIF_SYSCALL_DOTRACE|_TIF_SINGLESTEP|_TIF_USER_WORK_MASK|_TIF_PERSYSCALL_MASK)
> >> >> > 
> >> 
> >> By the way, I think this is not the right spot to call rseq_syscall, because
> >> interrupts are disabled. I think we should move this hunk right after
> >> system_call_exit.
> >> 
> > 
> > Good point.
> > 
> >> Would you like to implement and test an updated patch adding those calls for ppc
> >> 32 and 64 ?
> >> 
> > 
> > I'd like to help, but I don't have a handy ppc environment for test...
> > So I made the below patch which has only been build-tested, hope it
> > could be somewhat helpful.
> 
> Hi Boqun,
> 
> I tried your patch in a ppc64 le environment, and it does not survive boot
> with CONFIG_DEBUG_RSEQ=y. init gets killed right away.
> 
> Moreover, I'm not sure that the r3 register don't contain something worth
> saving before the call on ppc32. Just after there is a "mr" instruction
> which AFAIU takes r3 as input register.
> 
> Can you look into it ?

Hello, Boqun,

You can also request access to a ppc64 environment here:

	http://osuosl.org/services/powerdev/request_hosting/

							Thanx, Paul

> Thanks,
> 
> Mathieu
> 
> > 
> > Regards,
> > Boqun
> > 
> > --------------------------------->8
> > Subject: [PATCH] powerpc: Add syscall detection for restartable sequences
> > 
> > Syscalls are not allowed inside restartable sequences, so add a call to
> > rseq_syscall() at the very beginning of system call exiting path for
> > CONFIG_DEBUG_RSEQ=y kernel. This could help us to detect whether there
> > is a syscall issued inside restartable sequences.
> > 
> > Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
> > ---
> > arch/powerpc/kernel/entry_32.S | 5 +++++
> > arch/powerpc/kernel/entry_64.S | 5 +++++
> > 2 files changed, 10 insertions(+)
> > 
> > diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S
> > index eb8d01bae8c6..2f134eebe7ed 100644
> > --- a/arch/powerpc/kernel/entry_32.S
> > +++ b/arch/powerpc/kernel/entry_32.S
> > @@ -365,6 +365,11 @@ syscall_dotrace_cont:
> > 	blrl			/* Call handler */
> > 	.globl	ret_from_syscall
> > ret_from_syscall:
> > +#ifdef CONFIG_DEBUG_RSEQ
> > +	/* Check whether the syscall is issued inside a restartable sequence */
> > +	addi    r3,r1,STACK_FRAME_OVERHEAD
> > +	bl      rseq_syscall
> > +#endif
> > 	mr	r6,r3
> > 	CURRENT_THREAD_INFO(r12, r1)
> > 	/* disable interrupts so current_thread_info()->flags can't change */
> > diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
> > index 2cb5109a7ea3..2e2d59bb45d0 100644
> > --- a/arch/powerpc/kernel/entry_64.S
> > +++ b/arch/powerpc/kernel/entry_64.S
> > @@ -204,6 +204,11 @@ system_call:			/* label this so stack traces look sane */
> >  * This is blacklisted from kprobes further below with _ASM_NOKPROBE_SYMBOL().
> >  */
> > system_call_exit:
> > +#ifdef CONFIG_DEBUG_RSEQ
> > +	/* Check whether the syscall is issued inside a restartable sequence */
> > +	addi    r3,r1,STACK_FRAME_OVERHEAD
> > +	bl      rseq_syscall
> > +#endif
> > 	/*
> > 	 * Disable interrupts so current_thread_info()->flags can't change,
> > 	 * and so that we don't get interrupted after loading SRR0/1.
> > --
> > 2.16.2
> 
> -- 
> Mathieu Desnoyers
> EfficiOS Inc.
> http://www.efficios.com
> 

^ permalink raw reply

* Re: [PATCH 07/14] powerpc: Add support for restartable sequences
From: Mathieu Desnoyers @ 2018-05-23 20:14 UTC (permalink / raw)
  To: Boqun Feng
  Cc: Will Deacon, Peter Zijlstra, Paul E. McKenney, Andy Lutomirski,
	Dave Watson, linux-kernel, linux-api, Paul Turner, Andrew Morton,
	Russell King, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	Andrew Hunter, Andi Kleen, Chris Lameter, Ben Maurer, rostedt,
	Josh Triplett, Linus Torvalds, Catalin Marinas, Michael Kerrisk,
	Joel Fernandes, Benjamin Herrenschmidt, Paul Mackerras,
	Michael Ellerman, linuxppc-dev
In-Reply-To: <20180520140811.GB1121@tardis>

----- On May 20, 2018, at 10:08 AM, Boqun Feng boqun.feng@gmail.com wrote:

> On Fri, May 18, 2018 at 02:17:17PM -0400, Mathieu Desnoyers wrote:
>> ----- On May 17, 2018, at 7:50 PM, Boqun Feng boqun.feng@gmail.com wrote:
>> [...]
>> >> > I think you're right. So we have to introduce callsite to rseq_syscall()
>> >> > in syscall path, something like:
>> >> > 
>> >> > diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
>> >> > index 51695608c68b..a25734a96640 100644
>> >> > --- a/arch/powerpc/kernel/entry_64.S
>> >> > +++ b/arch/powerpc/kernel/entry_64.S
>> >> > @@ -222,6 +222,9 @@ system_call_exit:
>> >> > 	mtmsrd	r11,1
>> >> > #endif /* CONFIG_PPC_BOOK3E */
>> >> > 
>> >> > +	addi    r3,r1,STACK_FRAME_OVERHEAD
>> >> > +	bl	rseq_syscall
>> >> > +
>> >> > 	ld	r9,TI_FLAGS(r12)
>> >> > 	li	r11,-MAX_ERRNO
>> >> > 	andi.
>> >> > 		r0,r9,(_TIF_SYSCALL_DOTRACE|_TIF_SINGLESTEP|_TIF_USER_WORK_MASK|_TIF_PERSYSCALL_MASK)
>> >> > 
>> 
>> By the way, I think this is not the right spot to call rseq_syscall, because
>> interrupts are disabled. I think we should move this hunk right after
>> system_call_exit.
>> 
> 
> Good point.
> 
>> Would you like to implement and test an updated patch adding those calls for ppc
>> 32 and 64 ?
>> 
> 
> I'd like to help, but I don't have a handy ppc environment for test...
> So I made the below patch which has only been build-tested, hope it
> could be somewhat helpful.

Hi Boqun,

I tried your patch in a ppc64 le environment, and it does not survive boot
with CONFIG_DEBUG_RSEQ=y. init gets killed right away.

Moreover, I'm not sure that the r3 register don't contain something worth
saving before the call on ppc32. Just after there is a "mr" instruction
which AFAIU takes r3 as input register.

Can you look into it ?

Thanks,

Mathieu

> 
> Regards,
> Boqun
> 
> --------------------------------->8
> Subject: [PATCH] powerpc: Add syscall detection for restartable sequences
> 
> Syscalls are not allowed inside restartable sequences, so add a call to
> rseq_syscall() at the very beginning of system call exiting path for
> CONFIG_DEBUG_RSEQ=y kernel. This could help us to detect whether there
> is a syscall issued inside restartable sequences.
> 
> Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
> ---
> arch/powerpc/kernel/entry_32.S | 5 +++++
> arch/powerpc/kernel/entry_64.S | 5 +++++
> 2 files changed, 10 insertions(+)
> 
> diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S
> index eb8d01bae8c6..2f134eebe7ed 100644
> --- a/arch/powerpc/kernel/entry_32.S
> +++ b/arch/powerpc/kernel/entry_32.S
> @@ -365,6 +365,11 @@ syscall_dotrace_cont:
> 	blrl			/* Call handler */
> 	.globl	ret_from_syscall
> ret_from_syscall:
> +#ifdef CONFIG_DEBUG_RSEQ
> +	/* Check whether the syscall is issued inside a restartable sequence */
> +	addi    r3,r1,STACK_FRAME_OVERHEAD
> +	bl      rseq_syscall
> +#endif
> 	mr	r6,r3
> 	CURRENT_THREAD_INFO(r12, r1)
> 	/* disable interrupts so current_thread_info()->flags can't change */
> diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
> index 2cb5109a7ea3..2e2d59bb45d0 100644
> --- a/arch/powerpc/kernel/entry_64.S
> +++ b/arch/powerpc/kernel/entry_64.S
> @@ -204,6 +204,11 @@ system_call:			/* label this so stack traces look sane */
>  * This is blacklisted from kprobes further below with _ASM_NOKPROBE_SYMBOL().
>  */
> system_call_exit:
> +#ifdef CONFIG_DEBUG_RSEQ
> +	/* Check whether the syscall is issued inside a restartable sequence */
> +	addi    r3,r1,STACK_FRAME_OVERHEAD
> +	bl      rseq_syscall
> +#endif
> 	/*
> 	 * Disable interrupts so current_thread_info()->flags can't change,
> 	 * and so that we don't get interrupted after loading SRR0/1.
> --
> 2.16.2

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com

^ permalink raw reply

* [PATCH 4/5] pci/hotplug/pnv-php: add missing of_node_put
From: Julia Lawall @ 2018-05-23 19:07 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: kernel-janitors, Paul Mackerras, Michael Ellerman, Bjorn Helgaas,
	linuxppc-dev, linux-pci, linux-kernel
In-Reply-To: <1527102436-13447-1-git-send-email-Julia.Lawall@lip6.fr>

The device node iterators perform an of_node_get on each iteration, so a
jump out of the loop requires an of_node_put.

The semantic patch that fixes this problem is as follows
(http://coccinelle.lip6.fr):

// <smpl>
@@
expression root,e;
local idexpression child;
iterator name for_each_child_of_node;
@@

 for_each_child_of_node(root, child) {
   ... when != of_node_put(child)
       when != e = child
+  of_node_put(child);
?  break;
   ...
}
... when != child
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
 drivers/pci/hotplug/pnv_php.c |    8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/hotplug/pnv_php.c b/drivers/pci/hotplug/pnv_php.c
index d441006..6c2e8d7 100644
--- a/drivers/pci/hotplug/pnv_php.c
+++ b/drivers/pci/hotplug/pnv_php.c
@@ -220,12 +220,16 @@ static int pnv_php_populate_changeset(struct of_changeset *ocs,
 
 	for_each_child_of_node(dn, child) {
 		ret = of_changeset_attach_node(ocs, child);
-		if (ret)
+		if (ret) {
+			of_node_put(child);
 			break;
+		}
 
 		ret = pnv_php_populate_changeset(ocs, child);
-		if (ret)
+		if (ret) {
+			of_node_put(child);
 			break;
+		}
 	}
 
 	return ret;

^ permalink raw reply related

* [PATCH 0/5] add missing of_node_put
From: Julia Lawall @ 2018-05-23 19:07 UTC (permalink / raw)
  To: linux-pci
  Cc: kernel-janitors, linuxppc-dev, linux-kernel, linux-arm-kernel,
	linux-gpio, dri-devel, linux-rockchip

The device node iterators perform an of_node_get on each iteration, so a
jump out of the loop requires an of_node_put.

---

 drivers/gpu/drm/rockchip/rockchip_lvds.c   |    4 +++-
 drivers/pci/hotplug/pnv_php.c              |    8 ++++++--
 drivers/phy/hisilicon/phy-hisi-inno-usb2.c |    9 +++++++--
 drivers/pinctrl/pinctrl-at91-pio4.c        |    4 +++-
 drivers/soc/ti/knav_dma.c                  |    1 +
 5 files changed, 20 insertions(+), 6 deletions(-)

^ permalink raw reply

* [PATCH] sound: Use octal not symbolic permissions
From: Joe Perches @ 2018-05-23 19:20 UTC (permalink / raw)
  To: Vinod Koul, Clemens Ladisch, Brian Austin, Paul Handrigan,
	Timur Tabi, Nicolin Chen, Xiubo Li, Fabio Estevam,
	Jaroslav Kysela, Takashi Iwai
  Cc: Liam Girdwood, Mark Brown, alsa-devel, linux-kernel, patches,
	linuxppc-dev

Convert the S_<FOO> symbolic permissions to their octal equivalents as
using octal and not symbolic permissions is preferred by many as more
readable.

see: https://lkml.org/lkml/2016/8/2/1945

Done with automated conversion via:
$ ./scripts/checkpatch.pl -f --types=SYMBOLIC_PERMS --fix-inplace <files...>

Miscellanea:

o Wrapped one multi-line call to a single line

Signed-off-by: Joe Perches <joe@perches.com>
---
 sound/core/compress_offload.c             |  2 +-
 sound/core/info.c                         |  6 +++---
 sound/core/init.c                         |  4 ++--
 sound/core/oss/mixer_oss.c                |  2 +-
 sound/core/oss/pcm_oss.c                  |  2 +-
 sound/core/pcm.c                          | 10 +++++-----
 sound/core/pcm_memory.c                   |  2 +-
 sound/drivers/dummy.c                     |  2 +-
 sound/drivers/mts64.c                     |  6 +++---
 sound/drivers/opl4/opl4_proc.c            |  2 +-
 sound/drivers/portman2x4.c                |  6 +++---
 sound/firewire/bebob/bebob_proc.c         |  2 +-
 sound/firewire/dice/dice-proc.c           |  2 +-
 sound/firewire/digi00x/digi00x-proc.c     |  2 +-
 sound/firewire/fireface/ff-proc.c         |  2 +-
 sound/firewire/fireworks/fireworks_proc.c |  2 +-
 sound/firewire/motu/motu-proc.c           |  2 +-
 sound/firewire/oxfw/oxfw-proc.c           |  2 +-
 sound/firewire/tascam/tascam-proc.c       |  2 +-
 sound/isa/msnd/msnd_pinnacle.c            | 32 +++++++++++++++----------------
 sound/pci/ac97/ac97_proc.c                |  4 ++--
 sound/pci/asihpi/asihpi.c                 | 12 ++++++------
 sound/pci/asihpi/hpioctl.c                |  4 ++--
 sound/pci/ca0106/ca0106_proc.c            |  6 +++---
 sound/pci/cs46xx/cs46xx_lib.c             |  2 +-
 sound/pci/cs46xx/dsp_spos.c               | 14 +++++++-------
 sound/pci/cs46xx/dsp_spos_scb_lib.c       |  2 +-
 sound/pci/ctxfi/cttimer.c                 |  2 +-
 sound/pci/ctxfi/xfi.c                     |  4 ++--
 sound/pci/emu10k1/emu10k1x.c              |  2 +-
 sound/pci/emu10k1/emuproc.c               | 22 ++++++++++-----------
 sound/pci/hda/patch_hdmi.c                |  2 +-
 sound/pci/ice1712/pontis.c                |  2 +-
 sound/pci/ice1712/prodigy_hifi.c          |  2 +-
 sound/pci/lola/lola_proc.c                |  2 +-
 sound/pci/pcxhr/pcxhr.c                   |  2 +-
 sound/soc/codecs/cs43130.c                |  8 ++++----
 sound/soc/codecs/wm_adsp.c                | 11 +++++------
 sound/soc/fsl/fsl_ssi_dbg.c               |  2 +-
 sound/sound_core.c                        |  6 +++---
 sound/sparc/dbri.c                        |  2 +-
 41 files changed, 102 insertions(+), 103 deletions(-)

diff --git a/sound/core/compress_offload.c b/sound/core/compress_offload.c
index 4563432badba..4b01a37c836e 100644
--- a/sound/core/compress_offload.c
+++ b/sound/core/compress_offload.c
@@ -1001,7 +1001,7 @@ static int snd_compress_proc_init(struct snd_compr *compr)
 					   compr->card->proc_root);
 	if (!entry)
 		return -ENOMEM;
-	entry->mode = S_IFDIR | S_IRUGO | S_IXUGO;
+	entry->mode = S_IFDIR | 0555;
 	if (snd_info_register(entry) < 0) {
 		snd_info_free_entry(entry);
 		return -ENOMEM;
diff --git a/sound/core/info.c b/sound/core/info.c
index 4b36767af9e1..fe502bc5e6d2 100644
--- a/sound/core/info.c
+++ b/sound/core/info.c
@@ -454,7 +454,7 @@ static struct snd_info_entry *create_subdir(struct module *mod,
 	entry = snd_info_create_module_entry(mod, name, NULL);
 	if (!entry)
 		return NULL;
-	entry->mode = S_IFDIR | S_IRUGO | S_IXUGO;
+	entry->mode = S_IFDIR | 0555;
 	if (snd_info_register(entry) < 0) {
 		snd_info_free_entry(entry);
 		return NULL;
@@ -470,7 +470,7 @@ int __init snd_info_init(void)
 	snd_proc_root = snd_info_create_entry("asound", NULL);
 	if (!snd_proc_root)
 		return -ENOMEM;
-	snd_proc_root->mode = S_IFDIR | S_IRUGO | S_IXUGO;
+	snd_proc_root->mode = S_IFDIR | 0555;
 	snd_proc_root->p = proc_mkdir("asound", NULL);
 	if (!snd_proc_root->p)
 		goto error;
@@ -716,7 +716,7 @@ snd_info_create_entry(const char *name, struct snd_info_entry *parent)
 		kfree(entry);
 		return NULL;
 	}
-	entry->mode = S_IFREG | S_IRUGO;
+	entry->mode = S_IFREG | 0444;
 	entry->content = SNDRV_INFO_CONTENT_TEXT;
 	mutex_init(&entry->access);
 	INIT_LIST_HEAD(&entry->children);
diff --git a/sound/core/init.c b/sound/core/init.c
index 79b4df1c1dc7..4849c611c0fe 100644
--- a/sound/core/init.c
+++ b/sound/core/init.c
@@ -703,7 +703,7 @@ card_id_store_attr(struct device *dev, struct device_attribute *attr,
 	return count;
 }
 
-static DEVICE_ATTR(id, S_IRUGO | S_IWUSR, card_id_show_attr, card_id_store_attr);
+static DEVICE_ATTR(id, 0644, card_id_show_attr, card_id_store_attr);
 
 static ssize_t
 card_number_show_attr(struct device *dev,
@@ -713,7 +713,7 @@ card_number_show_attr(struct device *dev,
 	return scnprintf(buf, PAGE_SIZE, "%i\n", card->number);
 }
 
-static DEVICE_ATTR(number, S_IRUGO, card_number_show_attr, NULL);
+static DEVICE_ATTR(number, 0444, card_number_show_attr, NULL);
 
 static struct attribute *card_dev_attrs[] = {
 	&dev_attr_id.attr,
diff --git a/sound/core/oss/mixer_oss.c b/sound/core/oss/mixer_oss.c
index 379bf486ccc7..64d904bee8bb 100644
--- a/sound/core/oss/mixer_oss.c
+++ b/sound/core/oss/mixer_oss.c
@@ -1247,7 +1247,7 @@ static void snd_mixer_oss_proc_init(struct snd_mixer_oss *mixer)
 	if (! entry)
 		return;
 	entry->content = SNDRV_INFO_CONTENT_TEXT;
-	entry->mode = S_IFREG | S_IRUGO | S_IWUSR;
+	entry->mode = S_IFREG | 0644;
 	entry->c.text.read = snd_mixer_oss_proc_read;
 	entry->c.text.write = snd_mixer_oss_proc_write;
 	entry->private_data = mixer;
diff --git a/sound/core/oss/pcm_oss.c b/sound/core/oss/pcm_oss.c
index 1980f68246cb..905a53c1cde5 100644
--- a/sound/core/oss/pcm_oss.c
+++ b/sound/core/oss/pcm_oss.c
@@ -3045,7 +3045,7 @@ static void snd_pcm_oss_proc_init(struct snd_pcm *pcm)
 			continue;
 		if ((entry = snd_info_create_card_entry(pcm->card, "oss", pstr->proc_root)) != NULL) {
 			entry->content = SNDRV_INFO_CONTENT_TEXT;
-			entry->mode = S_IFREG | S_IRUGO | S_IWUSR;
+			entry->mode = S_IFREG | 0644;
 			entry->c.text.read = snd_pcm_oss_proc_read;
 			entry->c.text.write = snd_pcm_oss_proc_write;
 			entry->private_data = pstr;
diff --git a/sound/core/pcm.c b/sound/core/pcm.c
index 66ac89aad681..c352bfb973cc 100644
--- a/sound/core/pcm.c
+++ b/sound/core/pcm.c
@@ -530,7 +530,7 @@ static int snd_pcm_stream_proc_init(struct snd_pcm_str *pstr)
 					   pcm->card->proc_root);
 	if (!entry)
 		return -ENOMEM;
-	entry->mode = S_IFDIR | S_IRUGO | S_IXUGO;
+	entry->mode = S_IFDIR | 0555;
 	if (snd_info_register(entry) < 0) {
 		snd_info_free_entry(entry);
 		return -ENOMEM;
@@ -552,7 +552,7 @@ static int snd_pcm_stream_proc_init(struct snd_pcm_str *pstr)
 	if (entry) {
 		entry->c.text.read = snd_pcm_xrun_debug_read;
 		entry->c.text.write = snd_pcm_xrun_debug_write;
-		entry->mode |= S_IWUSR;
+		entry->mode |= 0200;
 		entry->private_data = pstr;
 		if (snd_info_register(entry) < 0) {
 			snd_info_free_entry(entry);
@@ -590,7 +590,7 @@ static int snd_pcm_substream_proc_init(struct snd_pcm_substream *substream)
 					   substream->pstr->proc_root);
 	if (!entry)
 		return -ENOMEM;
-	entry->mode = S_IFDIR | S_IRUGO | S_IXUGO;
+	entry->mode = S_IFDIR | 0555;
 	if (snd_info_register(entry) < 0) {
 		snd_info_free_entry(entry);
 		return -ENOMEM;
@@ -647,7 +647,7 @@ static int snd_pcm_substream_proc_init(struct snd_pcm_substream *substream)
 		entry->private_data = substream;
 		entry->c.text.read = NULL;
 		entry->c.text.write = snd_pcm_xrun_injection_write;
-		entry->mode = S_IFREG | S_IWUSR;
+		entry->mode = S_IFREG | 0200;
 		if (snd_info_register(entry) < 0) {
 			snd_info_free_entry(entry);
 			entry = NULL;
@@ -1087,7 +1087,7 @@ static ssize_t show_pcm_class(struct device *dev,
         return snprintf(buf, PAGE_SIZE, "%s\n", str);
 }
 
-static DEVICE_ATTR(pcm_class, S_IRUGO, show_pcm_class, NULL);
+static DEVICE_ATTR(pcm_class, 0444, show_pcm_class, NULL);
 static struct attribute *pcm_dev_attrs[] = {
 	&dev_attr_pcm_class.attr,
 	NULL
diff --git a/sound/core/pcm_memory.c b/sound/core/pcm_memory.c
index ae33e456708c..4b5356a10315 100644
--- a/sound/core/pcm_memory.c
+++ b/sound/core/pcm_memory.c
@@ -201,7 +201,7 @@ static inline void preallocate_info_init(struct snd_pcm_substream *substream)
 	if ((entry = snd_info_create_card_entry(substream->pcm->card, "prealloc", substream->proc_root)) != NULL) {
 		entry->c.text.read = snd_pcm_lib_preallocate_proc_read;
 		entry->c.text.write = snd_pcm_lib_preallocate_proc_write;
-		entry->mode |= S_IWUSR;
+		entry->mode |= 0200;
 		entry->private_data = substream;
 		if (snd_info_register(entry) < 0) {
 			snd_info_free_entry(entry);
diff --git a/sound/drivers/dummy.c b/sound/drivers/dummy.c
index 8fb9a54fe8ba..9af154db530a 100644
--- a/sound/drivers/dummy.c
+++ b/sound/drivers/dummy.c
@@ -1042,7 +1042,7 @@ static void dummy_proc_init(struct snd_dummy *chip)
 	if (!snd_card_proc_new(chip->card, "dummy_pcm", &entry)) {
 		snd_info_set_text_ops(entry, chip, dummy_proc_read);
 		entry->c.text.write = dummy_proc_write;
-		entry->mode |= S_IWUSR;
+		entry->mode |= 0200;
 		entry->private_data = chip;
 	}
 }
diff --git a/sound/drivers/mts64.c b/sound/drivers/mts64.c
index f32e81342247..b68e71ca7abd 100644
--- a/sound/drivers/mts64.c
+++ b/sound/drivers/mts64.c
@@ -41,11 +41,11 @@ static bool enable[SNDRV_CARDS] = SNDRV_DEFAULT_ENABLE_PNP;
 static struct platform_device *platform_devices[SNDRV_CARDS]; 
 static int device_count;
 
-module_param_array(index, int, NULL, S_IRUGO);
+module_param_array(index, int, NULL, 0444);
 MODULE_PARM_DESC(index, "Index value for " CARD_NAME " soundcard.");
-module_param_array(id, charp, NULL, S_IRUGO);
+module_param_array(id, charp, NULL, 0444);
 MODULE_PARM_DESC(id, "ID string for " CARD_NAME " soundcard.");
-module_param_array(enable, bool, NULL, S_IRUGO);
+module_param_array(enable, bool, NULL, 0444);
 MODULE_PARM_DESC(enable, "Enable " CARD_NAME " soundcard.");
 
 MODULE_AUTHOR("Matthias Koenig <mk@phasorlab.de>");
diff --git a/sound/drivers/opl4/opl4_proc.c b/sound/drivers/opl4/opl4_proc.c
index cd2c07fa2ef4..16b24091d799 100644
--- a/sound/drivers/opl4/opl4_proc.c
+++ b/sound/drivers/opl4/opl4_proc.c
@@ -104,7 +104,7 @@ int snd_opl4_create_proc(struct snd_opl4 *opl4)
 	if (entry) {
 		if (opl4->hardware < OPL3_HW_OPL4_ML) {
 			/* OPL4 can access 4 MB external ROM/SRAM */
-			entry->mode |= S_IWUSR;
+			entry->mode |= 0200;
 			entry->size = 4 * 1024 * 1024;
 		} else {
 			/* OPL4-ML has 1 MB internal ROM */
diff --git a/sound/drivers/portman2x4.c b/sound/drivers/portman2x4.c
index ec8a94325ef6..3cdf0a88d71b 100644
--- a/sound/drivers/portman2x4.c
+++ b/sound/drivers/portman2x4.c
@@ -60,11 +60,11 @@ static bool enable[SNDRV_CARDS] = SNDRV_DEFAULT_ENABLE_PNP;
 static struct platform_device *platform_devices[SNDRV_CARDS]; 
 static int device_count;
 
-module_param_array(index, int, NULL, S_IRUGO);
+module_param_array(index, int, NULL, 0444);
 MODULE_PARM_DESC(index, "Index value for " CARD_NAME " soundcard.");
-module_param_array(id, charp, NULL, S_IRUGO);
+module_param_array(id, charp, NULL, 0444);
 MODULE_PARM_DESC(id, "ID string for " CARD_NAME " soundcard.");
-module_param_array(enable, bool, NULL, S_IRUGO);
+module_param_array(enable, bool, NULL, 0444);
 MODULE_PARM_DESC(enable, "Enable " CARD_NAME " soundcard.");
 
 MODULE_AUTHOR("Levent Guendogdu, Tobias Gehrig, Matthias Koenig");
diff --git a/sound/firewire/bebob/bebob_proc.c b/sound/firewire/bebob/bebob_proc.c
index ec24f96794f5..8096891af913 100644
--- a/sound/firewire/bebob/bebob_proc.c
+++ b/sound/firewire/bebob/bebob_proc.c
@@ -183,7 +183,7 @@ void snd_bebob_proc_init(struct snd_bebob *bebob)
 					  bebob->card->proc_root);
 	if (root == NULL)
 		return;
-	root->mode = S_IFDIR | S_IRUGO | S_IXUGO;
+	root->mode = S_IFDIR | 0555;
 	if (snd_info_register(root) < 0) {
 		snd_info_free_entry(root);
 		return;
diff --git a/sound/firewire/dice/dice-proc.c b/sound/firewire/dice/dice-proc.c
index faf8c854d256..bb870fc73f99 100644
--- a/sound/firewire/dice/dice-proc.c
+++ b/sound/firewire/dice/dice-proc.c
@@ -305,7 +305,7 @@ void snd_dice_create_proc(struct snd_dice *dice)
 					  dice->card->proc_root);
 	if (!root)
 		return;
-	root->mode = S_IFDIR | S_IRUGO | S_IXUGO;
+	root->mode = S_IFDIR | 0555;
 	if (snd_info_register(root) < 0) {
 		snd_info_free_entry(root);
 		return;
diff --git a/sound/firewire/digi00x/digi00x-proc.c b/sound/firewire/digi00x/digi00x-proc.c
index a1d601f31165..6996d5a6ff5f 100644
--- a/sound/firewire/digi00x/digi00x-proc.c
+++ b/sound/firewire/digi00x/digi00x-proc.c
@@ -79,7 +79,7 @@ void snd_dg00x_proc_init(struct snd_dg00x *dg00x)
 	if (root == NULL)
 		return;
 
-	root->mode = S_IFDIR | S_IRUGO | S_IXUGO;
+	root->mode = S_IFDIR | 0555;
 	if (snd_info_register(root) < 0) {
 		snd_info_free_entry(root);
 		return;
diff --git a/sound/firewire/fireface/ff-proc.c b/sound/firewire/fireface/ff-proc.c
index 69441d121f71..40ccbfd8ef89 100644
--- a/sound/firewire/fireface/ff-proc.c
+++ b/sound/firewire/fireface/ff-proc.c
@@ -52,7 +52,7 @@ void snd_ff_proc_init(struct snd_ff *ff)
 					  ff->card->proc_root);
 	if (root == NULL)
 		return;
-	root->mode = S_IFDIR | S_IRUGO | S_IXUGO;
+	root->mode = S_IFDIR | 0555;
 	if (snd_info_register(root) < 0) {
 		snd_info_free_entry(root);
 		return;
diff --git a/sound/firewire/fireworks/fireworks_proc.c b/sound/firewire/fireworks/fireworks_proc.c
index 9c21f31b8b21..779ecec5af62 100644
--- a/sound/firewire/fireworks/fireworks_proc.c
+++ b/sound/firewire/fireworks/fireworks_proc.c
@@ -219,7 +219,7 @@ void snd_efw_proc_init(struct snd_efw *efw)
 					  efw->card->proc_root);
 	if (root == NULL)
 		return;
-	root->mode = S_IFDIR | S_IRUGO | S_IXUGO;
+	root->mode = S_IFDIR | 0555;
 	if (snd_info_register(root) < 0) {
 		snd_info_free_entry(root);
 		return;
diff --git a/sound/firewire/motu/motu-proc.c b/sound/firewire/motu/motu-proc.c
index 4edc064999ed..ab6830a6d242 100644
--- a/sound/firewire/motu/motu-proc.c
+++ b/sound/firewire/motu/motu-proc.c
@@ -107,7 +107,7 @@ void snd_motu_proc_init(struct snd_motu *motu)
 					  motu->card->proc_root);
 	if (root == NULL)
 		return;
-	root->mode = S_IFDIR | S_IRUGO | S_IXUGO;
+	root->mode = S_IFDIR | 0555;
 	if (snd_info_register(root) < 0) {
 		snd_info_free_entry(root);
 		return;
diff --git a/sound/firewire/oxfw/oxfw-proc.c b/sound/firewire/oxfw/oxfw-proc.c
index 8ba4f9f262b8..27dac071bc73 100644
--- a/sound/firewire/oxfw/oxfw-proc.c
+++ b/sound/firewire/oxfw/oxfw-proc.c
@@ -103,7 +103,7 @@ void snd_oxfw_proc_init(struct snd_oxfw *oxfw)
 					  oxfw->card->proc_root);
 	if (root == NULL)
 		return;
-	root->mode = S_IFDIR | S_IRUGO | S_IXUGO;
+	root->mode = S_IFDIR | 0555;
 	if (snd_info_register(root) < 0) {
 		snd_info_free_entry(root);
 		return;
diff --git a/sound/firewire/tascam/tascam-proc.c b/sound/firewire/tascam/tascam-proc.c
index bfd4a4c06914..fee3bf32a0da 100644
--- a/sound/firewire/tascam/tascam-proc.c
+++ b/sound/firewire/tascam/tascam-proc.c
@@ -78,7 +78,7 @@ void snd_tscm_proc_init(struct snd_tscm *tscm)
 					  tscm->card->proc_root);
 	if (root == NULL)
 		return;
-	root->mode = S_IFDIR | S_IRUGO | S_IXUGO;
+	root->mode = S_IFDIR | 0555;
 	if (snd_info_register(root) < 0) {
 		snd_info_free_entry(root);
 		return;
diff --git a/sound/isa/msnd/msnd_pinnacle.c b/sound/isa/msnd/msnd_pinnacle.c
index 45e561c425bf..6c584d9b6c42 100644
--- a/sound/isa/msnd/msnd_pinnacle.c
+++ b/sound/isa/msnd/msnd_pinnacle.c
@@ -757,9 +757,9 @@ static int snd_msnd_pinnacle_cfg_reset(int cfg)
 static int index[SNDRV_CARDS] = SNDRV_DEFAULT_IDX;	/* Index 0-MAX */
 static char *id[SNDRV_CARDS] = SNDRV_DEFAULT_STR;	/* ID for this card */
 
-module_param_array(index, int, NULL, S_IRUGO);
+module_param_array(index, int, NULL, 0444);
 MODULE_PARM_DESC(index, "Index value for msnd_pinnacle soundcard.");
-module_param_array(id, charp, NULL, S_IRUGO);
+module_param_array(id, charp, NULL, 0444);
 MODULE_PARM_DESC(id, "ID string for msnd_pinnacle soundcard.");
 
 static long io[SNDRV_CARDS] = SNDRV_DEFAULT_PORT;
@@ -801,22 +801,22 @@ MODULE_LICENSE("GPL");
 MODULE_FIRMWARE(INITCODEFILE);
 MODULE_FIRMWARE(PERMCODEFILE);
 
-module_param_hw_array(io, long, ioport, NULL, S_IRUGO);
+module_param_hw_array(io, long, ioport, NULL, 0444);
 MODULE_PARM_DESC(io, "IO port #");
-module_param_hw_array(irq, int, irq, NULL, S_IRUGO);
-module_param_hw_array(mem, long, iomem, NULL, S_IRUGO);
-module_param_array(write_ndelay, int, NULL, S_IRUGO);
-module_param(calibrate_signal, int, S_IRUGO);
+module_param_hw_array(irq, int, irq, NULL, 0444);
+module_param_hw_array(mem, long, iomem, NULL, 0444);
+module_param_array(write_ndelay, int, NULL, 0444);
+module_param(calibrate_signal, int, 0444);
 #ifndef MSND_CLASSIC
-module_param_array(digital, int, NULL, S_IRUGO);
-module_param_hw_array(cfg, long, ioport, NULL, S_IRUGO);
-module_param_array(reset, int, 0, S_IRUGO);
-module_param_hw_array(mpu_io, long, ioport, NULL, S_IRUGO);
-module_param_hw_array(mpu_irq, int, irq, NULL, S_IRUGO);
-module_param_hw_array(ide_io0, long, ioport, NULL, S_IRUGO);
-module_param_hw_array(ide_io1, long, ioport, NULL, S_IRUGO);
-module_param_hw_array(ide_irq, int, irq, NULL, S_IRUGO);
-module_param_hw_array(joystick_io, long, ioport, NULL, S_IRUGO);
+module_param_array(digital, int, NULL, 0444);
+module_param_hw_array(cfg, long, ioport, NULL, 0444);
+module_param_array(reset, int, 0, 0444);
+module_param_hw_array(mpu_io, long, ioport, NULL, 0444);
+module_param_hw_array(mpu_irq, int, irq, NULL, 0444);
+module_param_hw_array(ide_io0, long, ioport, NULL, 0444);
+module_param_hw_array(ide_io1, long, ioport, NULL, 0444);
+module_param_hw_array(ide_irq, int, irq, NULL, 0444);
+module_param_hw_array(joystick_io, long, ioport, NULL, 0444);
 #endif
 
 
diff --git a/sound/pci/ac97/ac97_proc.c b/sound/pci/ac97/ac97_proc.c
index 6320bf084e47..e120a11c69e8 100644
--- a/sound/pci/ac97/ac97_proc.c
+++ b/sound/pci/ac97/ac97_proc.c
@@ -448,7 +448,7 @@ void snd_ac97_proc_init(struct snd_ac97 * ac97)
 	if ((entry = snd_info_create_card_entry(ac97->bus->card, name, ac97->bus->proc)) != NULL) {
 		snd_info_set_text_ops(entry, ac97, snd_ac97_proc_regs_read);
 #ifdef CONFIG_SND_DEBUG
-		entry->mode |= S_IWUSR;
+		entry->mode |= 0200;
 		entry->c.text.write = snd_ac97_proc_regs_write;
 #endif
 		if (snd_info_register(entry) < 0) {
@@ -474,7 +474,7 @@ void snd_ac97_bus_proc_init(struct snd_ac97_bus * bus)
 
 	sprintf(name, "codec97#%d", bus->num);
 	if ((entry = snd_info_create_card_entry(bus->card, name, bus->card->proc_root)) != NULL) {
-		entry->mode = S_IFDIR | S_IRUGO | S_IXUGO;
+		entry->mode = S_IFDIR | 0555;
 		if (snd_info_register(entry) < 0) {
 			snd_info_free_entry(entry);
 			entry = NULL;
diff --git a/sound/pci/asihpi/asihpi.c b/sound/pci/asihpi/asihpi.c
index 720361455c60..64e0961f93ba 100644
--- a/sound/pci/asihpi/asihpi.c
+++ b/sound/pci/asihpi/asihpi.c
@@ -69,27 +69,27 @@ static char *id[SNDRV_CARDS] = SNDRV_DEFAULT_STR;	/* ID for this card */
 static bool enable[SNDRV_CARDS] = SNDRV_DEFAULT_ENABLE_PNP;
 static bool enable_hpi_hwdep = 1;
 
-module_param_array(index, int, NULL, S_IRUGO);
+module_param_array(index, int, NULL, 0444);
 MODULE_PARM_DESC(index, "ALSA index value for AudioScience soundcard.");
 
-module_param_array(id, charp, NULL, S_IRUGO);
+module_param_array(id, charp, NULL, 0444);
 MODULE_PARM_DESC(id, "ALSA ID string for AudioScience soundcard.");
 
-module_param_array(enable, bool, NULL, S_IRUGO);
+module_param_array(enable, bool, NULL, 0444);
 MODULE_PARM_DESC(enable, "ALSA enable AudioScience soundcard.");
 
-module_param(enable_hpi_hwdep, bool, S_IRUGO|S_IWUSR);
+module_param(enable_hpi_hwdep, bool, 0644);
 MODULE_PARM_DESC(enable_hpi_hwdep,
 		"ALSA enable HPI hwdep for AudioScience soundcard ");
 
 /* identify driver */
 #ifdef KERNEL_ALSA_BUILD
 static char *build_info = "Built using headers from kernel source";
-module_param(build_info, charp, S_IRUGO);
+module_param(build_info, charp, 0444);
 MODULE_PARM_DESC(build_info, "Built using headers from kernel source");
 #else
 static char *build_info = "Built within ALSA source";
-module_param(build_info, charp, S_IRUGO);
+module_param(build_info, charp, 0444);
 MODULE_PARM_DESC(build_info, "Built within ALSA source");
 #endif
 
diff --git a/sound/pci/asihpi/hpioctl.c b/sound/pci/asihpi/hpioctl.c
index b1a2a7ea4172..7d049569012c 100644
--- a/sound/pci/asihpi/hpioctl.c
+++ b/sound/pci/asihpi/hpioctl.c
@@ -46,14 +46,14 @@ MODULE_FIRMWARE("asihpi/dsp8900.bin");
 #endif
 
 static int prealloc_stream_buf;
-module_param(prealloc_stream_buf, int, S_IRUGO);
+module_param(prealloc_stream_buf, int, 0444);
 MODULE_PARM_DESC(prealloc_stream_buf,
 	"Preallocate size for per-adapter stream buffer");
 
 /* Allow the debug level to be changed after module load.
  E.g.   echo 2 > /sys/module/asihpi/parameters/hpiDebugLevel
 */
-module_param(hpi_debug_level, int, S_IRUGO | S_IWUSR);
+module_param(hpi_debug_level, int, 0644);
 MODULE_PARM_DESC(hpi_debug_level, "debug verbosity 0..5");
 
 /* List of adapters found */
diff --git a/sound/pci/ca0106/ca0106_proc.c b/sound/pci/ca0106/ca0106_proc.c
index 9b2b8b38122f..a2c85cc37972 100644
--- a/sound/pci/ca0106/ca0106_proc.c
+++ b/sound/pci/ca0106/ca0106_proc.c
@@ -431,7 +431,7 @@ int snd_ca0106_proc_init(struct snd_ca0106 *emu)
 	if(! snd_card_proc_new(emu->card, "ca0106_reg32", &entry)) {
 		snd_info_set_text_ops(entry, emu, snd_ca0106_proc_reg_read32);
 		entry->c.text.write = snd_ca0106_proc_reg_write32;
-		entry->mode |= S_IWUSR;
+		entry->mode |= 0200;
 	}
 	if(! snd_card_proc_new(emu->card, "ca0106_reg16", &entry))
 		snd_info_set_text_ops(entry, emu, snd_ca0106_proc_reg_read16);
@@ -440,12 +440,12 @@ int snd_ca0106_proc_init(struct snd_ca0106 *emu)
 	if(! snd_card_proc_new(emu->card, "ca0106_regs1", &entry)) {
 		snd_info_set_text_ops(entry, emu, snd_ca0106_proc_reg_read1);
 		entry->c.text.write = snd_ca0106_proc_reg_write;
-		entry->mode |= S_IWUSR;
+		entry->mode |= 0200;
 	}
 	if(! snd_card_proc_new(emu->card, "ca0106_i2c", &entry)) {
 		entry->c.text.write = snd_ca0106_proc_i2c_write;
 		entry->private_data = emu;
-		entry->mode |= S_IWUSR;
+		entry->mode |= 0200;
 	}
 	if(! snd_card_proc_new(emu->card, "ca0106_regs2", &entry)) 
 		snd_info_set_text_ops(entry, emu, snd_ca0106_proc_reg_read2);
diff --git a/sound/pci/cs46xx/cs46xx_lib.c b/sound/pci/cs46xx/cs46xx_lib.c
index 0020fd0efc46..ed1251c5f449 100644
--- a/sound/pci/cs46xx/cs46xx_lib.c
+++ b/sound/pci/cs46xx/cs46xx_lib.c
@@ -2849,7 +2849,7 @@ static int snd_cs46xx_proc_init(struct snd_card *card, struct snd_cs46xx *chip)
 			entry->private_data = chip;
 			entry->c.ops = &snd_cs46xx_proc_io_ops;
 			entry->size = region->size;
-			entry->mode = S_IFREG | S_IRUSR;
+			entry->mode = S_IFREG | 0400;
 		}
 	}
 #ifdef CONFIG_SND_CS46XX_NEW_DSP
diff --git a/sound/pci/cs46xx/dsp_spos.c b/sound/pci/cs46xx/dsp_spos.c
index aa61615288ff..c44eadef64ae 100644
--- a/sound/pci/cs46xx/dsp_spos.c
+++ b/sound/pci/cs46xx/dsp_spos.c
@@ -798,7 +798,7 @@ int cs46xx_dsp_proc_init (struct snd_card *card, struct snd_cs46xx *chip)
 
 	if ((entry = snd_info_create_card_entry(card, "dsp", card->proc_root)) != NULL) {
 		entry->content = SNDRV_INFO_CONTENT_TEXT;
-		entry->mode = S_IFDIR | S_IRUGO | S_IXUGO;
+		entry->mode = S_IFDIR | 0555;
       
 		if (snd_info_register(entry) < 0) {
 			snd_info_free_entry(entry);
@@ -814,7 +814,7 @@ int cs46xx_dsp_proc_init (struct snd_card *card, struct snd_cs46xx *chip)
 	if ((entry = snd_info_create_card_entry(card, "spos_symbols", ins->proc_dsp_dir)) != NULL) {
 		entry->content = SNDRV_INFO_CONTENT_TEXT;
 		entry->private_data = chip;
-		entry->mode = S_IFREG | S_IRUGO | S_IWUSR;
+		entry->mode = S_IFREG | 0644;
 		entry->c.text.read = cs46xx_dsp_proc_symbol_table_read;
 		if (snd_info_register(entry) < 0) {
 			snd_info_free_entry(entry);
@@ -826,7 +826,7 @@ int cs46xx_dsp_proc_init (struct snd_card *card, struct snd_cs46xx *chip)
 	if ((entry = snd_info_create_card_entry(card, "spos_modules", ins->proc_dsp_dir)) != NULL) {
 		entry->content = SNDRV_INFO_CONTENT_TEXT;
 		entry->private_data = chip;
-		entry->mode = S_IFREG | S_IRUGO | S_IWUSR;
+		entry->mode = S_IFREG | 0644;
 		entry->c.text.read = cs46xx_dsp_proc_modules_read;
 		if (snd_info_register(entry) < 0) {
 			snd_info_free_entry(entry);
@@ -838,7 +838,7 @@ int cs46xx_dsp_proc_init (struct snd_card *card, struct snd_cs46xx *chip)
 	if ((entry = snd_info_create_card_entry(card, "parameter", ins->proc_dsp_dir)) != NULL) {
 		entry->content = SNDRV_INFO_CONTENT_TEXT;
 		entry->private_data = chip;
-		entry->mode = S_IFREG | S_IRUGO | S_IWUSR;
+		entry->mode = S_IFREG | 0644;
 		entry->c.text.read = cs46xx_dsp_proc_parameter_dump_read;
 		if (snd_info_register(entry) < 0) {
 			snd_info_free_entry(entry);
@@ -850,7 +850,7 @@ int cs46xx_dsp_proc_init (struct snd_card *card, struct snd_cs46xx *chip)
 	if ((entry = snd_info_create_card_entry(card, "sample", ins->proc_dsp_dir)) != NULL) {
 		entry->content = SNDRV_INFO_CONTENT_TEXT;
 		entry->private_data = chip;
-		entry->mode = S_IFREG | S_IRUGO | S_IWUSR;
+		entry->mode = S_IFREG | 0644;
 		entry->c.text.read = cs46xx_dsp_proc_sample_dump_read;
 		if (snd_info_register(entry) < 0) {
 			snd_info_free_entry(entry);
@@ -862,7 +862,7 @@ int cs46xx_dsp_proc_init (struct snd_card *card, struct snd_cs46xx *chip)
 	if ((entry = snd_info_create_card_entry(card, "task_tree", ins->proc_dsp_dir)) != NULL) {
 		entry->content = SNDRV_INFO_CONTENT_TEXT;
 		entry->private_data = chip;
-		entry->mode = S_IFREG | S_IRUGO | S_IWUSR;
+		entry->mode = S_IFREG | 0644;
 		entry->c.text.read = cs46xx_dsp_proc_task_tree_read;
 		if (snd_info_register(entry) < 0) {
 			snd_info_free_entry(entry);
@@ -874,7 +874,7 @@ int cs46xx_dsp_proc_init (struct snd_card *card, struct snd_cs46xx *chip)
 	if ((entry = snd_info_create_card_entry(card, "scb_info", ins->proc_dsp_dir)) != NULL) {
 		entry->content = SNDRV_INFO_CONTENT_TEXT;
 		entry->private_data = chip;
-		entry->mode = S_IFREG | S_IRUGO | S_IWUSR;
+		entry->mode = S_IFREG | 0644;
 		entry->c.text.read = cs46xx_dsp_proc_scb_read;
 		if (snd_info_register(entry) < 0) {
 			snd_info_free_entry(entry);
diff --git a/sound/pci/cs46xx/dsp_spos_scb_lib.c b/sound/pci/cs46xx/dsp_spos_scb_lib.c
index 7488e1b7a770..abb01ce66983 100644
--- a/sound/pci/cs46xx/dsp_spos_scb_lib.c
+++ b/sound/pci/cs46xx/dsp_spos_scb_lib.c
@@ -271,7 +271,7 @@ void cs46xx_dsp_proc_register_scb_desc (struct snd_cs46xx *chip,
       
 			entry->content = SNDRV_INFO_CONTENT_TEXT;
 			entry->private_data = scb_info;
-			entry->mode = S_IFREG | S_IRUGO | S_IWUSR;
+			entry->mode = S_IFREG | 0644;
       
 			entry->c.text.read = cs46xx_dsp_proc_scb_info_read;
       
diff --git a/sound/pci/ctxfi/cttimer.c b/sound/pci/ctxfi/cttimer.c
index 08e874e9a7f6..2099e9ce441a 100644
--- a/sound/pci/ctxfi/cttimer.c
+++ b/sound/pci/ctxfi/cttimer.c
@@ -17,7 +17,7 @@
 
 static bool use_system_timer;
 MODULE_PARM_DESC(use_system_timer, "Force to use system-timer");
-module_param(use_system_timer, bool, S_IRUGO);
+module_param(use_system_timer, bool, 0444);
 
 struct ct_timer_ops {
 	void (*init)(struct ct_timer_instance *);
diff --git a/sound/pci/ctxfi/xfi.c b/sound/pci/ctxfi/xfi.c
index f2f32779de98..b2874220be1b 100644
--- a/sound/pci/ctxfi/xfi.c
+++ b/sound/pci/ctxfi/xfi.c
@@ -26,9 +26,9 @@ MODULE_SUPPORTED_DEVICE("{{Creative Labs, Sound Blaster X-Fi}");
 static unsigned int reference_rate = 48000;
 static unsigned int multiple = 2;
 MODULE_PARM_DESC(reference_rate, "Reference rate (default=48000)");
-module_param(reference_rate, uint, S_IRUGO);
+module_param(reference_rate, uint, 0444);
 MODULE_PARM_DESC(multiple, "Rate multiplier (default=2)");
-module_param(multiple, uint, S_IRUGO);
+module_param(multiple, uint, 0444);
 
 static int index[SNDRV_CARDS] = SNDRV_DEFAULT_IDX;
 static char *id[SNDRV_CARDS] = SNDRV_DEFAULT_STR;
diff --git a/sound/pci/emu10k1/emu10k1x.c b/sound/pci/emu10k1/emu10k1x.c
index 2c2b12a06177..611589cbdad6 100644
--- a/sound/pci/emu10k1/emu10k1x.c
+++ b/sound/pci/emu10k1/emu10k1x.c
@@ -1070,7 +1070,7 @@ static int snd_emu10k1x_proc_init(struct emu10k1x *emu)
 	if(! snd_card_proc_new(emu->card, "emu10k1x_regs", &entry)) {
 		snd_info_set_text_ops(entry, emu, snd_emu10k1x_proc_reg_read);
 		entry->c.text.write = snd_emu10k1x_proc_reg_write;
-		entry->mode |= S_IWUSR;
+		entry->mode |= 0200;
 		entry->private_data = emu;
 	}
 	
diff --git a/sound/pci/emu10k1/emuproc.c b/sound/pci/emu10k1/emuproc.c
index bde0d1954f56..dc3d3512154f 100644
--- a/sound/pci/emu10k1/emuproc.c
+++ b/sound/pci/emu10k1/emuproc.c
@@ -574,32 +574,32 @@ int snd_emu10k1_proc_init(struct snd_emu10k1 *emu)
 	if (! snd_card_proc_new(emu->card, "io_regs", &entry)) {
 		snd_info_set_text_ops(entry, emu, snd_emu_proc_io_reg_read);
 		entry->c.text.write = snd_emu_proc_io_reg_write;
-		entry->mode |= S_IWUSR;
+		entry->mode |= 0200;
 	}
 	if (! snd_card_proc_new(emu->card, "ptr_regs00a", &entry)) {
 		snd_info_set_text_ops(entry, emu, snd_emu_proc_ptr_reg_read00a);
 		entry->c.text.write = snd_emu_proc_ptr_reg_write00;
-		entry->mode |= S_IWUSR;
+		entry->mode |= 0200;
 	}
 	if (! snd_card_proc_new(emu->card, "ptr_regs00b", &entry)) {
 		snd_info_set_text_ops(entry, emu, snd_emu_proc_ptr_reg_read00b);
 		entry->c.text.write = snd_emu_proc_ptr_reg_write00;
-		entry->mode |= S_IWUSR;
+		entry->mode |= 0200;
 	}
 	if (! snd_card_proc_new(emu->card, "ptr_regs20a", &entry)) {
 		snd_info_set_text_ops(entry, emu, snd_emu_proc_ptr_reg_read20a);
 		entry->c.text.write = snd_emu_proc_ptr_reg_write20;
-		entry->mode |= S_IWUSR;
+		entry->mode |= 0200;
 	}
 	if (! snd_card_proc_new(emu->card, "ptr_regs20b", &entry)) {
 		snd_info_set_text_ops(entry, emu, snd_emu_proc_ptr_reg_read20b);
 		entry->c.text.write = snd_emu_proc_ptr_reg_write20;
-		entry->mode |= S_IWUSR;
+		entry->mode |= 0200;
 	}
 	if (! snd_card_proc_new(emu->card, "ptr_regs20c", &entry)) {
 		snd_info_set_text_ops(entry, emu, snd_emu_proc_ptr_reg_read20c);
 		entry->c.text.write = snd_emu_proc_ptr_reg_write20;
-		entry->mode |= S_IWUSR;
+		entry->mode |= 0200;
 	}
 #endif
 	
@@ -621,35 +621,35 @@ int snd_emu10k1_proc_init(struct snd_emu10k1 *emu)
 	if (! snd_card_proc_new(emu->card, "fx8010_gpr", &entry)) {
 		entry->content = SNDRV_INFO_CONTENT_DATA;
 		entry->private_data = emu;
-		entry->mode = S_IFREG | S_IRUGO /*| S_IWUSR*/;
+		entry->mode = S_IFREG | 0444 /*| S_IWUSR*/;
 		entry->size = emu->audigy ? A_TOTAL_SIZE_GPR : TOTAL_SIZE_GPR;
 		entry->c.ops = &snd_emu10k1_proc_ops_fx8010;
 	}
 	if (! snd_card_proc_new(emu->card, "fx8010_tram_data", &entry)) {
 		entry->content = SNDRV_INFO_CONTENT_DATA;
 		entry->private_data = emu;
-		entry->mode = S_IFREG | S_IRUGO /*| S_IWUSR*/;
+		entry->mode = S_IFREG | 0444 /*| S_IWUSR*/;
 		entry->size = emu->audigy ? A_TOTAL_SIZE_TANKMEM_DATA : TOTAL_SIZE_TANKMEM_DATA ;
 		entry->c.ops = &snd_emu10k1_proc_ops_fx8010;
 	}
 	if (! snd_card_proc_new(emu->card, "fx8010_tram_addr", &entry)) {
 		entry->content = SNDRV_INFO_CONTENT_DATA;
 		entry->private_data = emu;
-		entry->mode = S_IFREG | S_IRUGO /*| S_IWUSR*/;
+		entry->mode = S_IFREG | 0444 /*| S_IWUSR*/;
 		entry->size = emu->audigy ? A_TOTAL_SIZE_TANKMEM_ADDR : TOTAL_SIZE_TANKMEM_ADDR ;
 		entry->c.ops = &snd_emu10k1_proc_ops_fx8010;
 	}
 	if (! snd_card_proc_new(emu->card, "fx8010_code", &entry)) {
 		entry->content = SNDRV_INFO_CONTENT_DATA;
 		entry->private_data = emu;
-		entry->mode = S_IFREG | S_IRUGO /*| S_IWUSR*/;
+		entry->mode = S_IFREG | 0444 /*| S_IWUSR*/;
 		entry->size = emu->audigy ? A_TOTAL_SIZE_CODE : TOTAL_SIZE_CODE;
 		entry->c.ops = &snd_emu10k1_proc_ops_fx8010;
 	}
 	if (! snd_card_proc_new(emu->card, "fx8010_acode", &entry)) {
 		entry->content = SNDRV_INFO_CONTENT_TEXT;
 		entry->private_data = emu;
-		entry->mode = S_IFREG | S_IRUGO /*| S_IWUSR*/;
+		entry->mode = S_IFREG | 0444 /*| S_IWUSR*/;
 		entry->c.text.read = snd_emu10k1_proc_acode_read;
 	}
 	return 0;
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
index 7d7eb1354eee..8840daf9c6a3 100644
--- a/sound/pci/hda/patch_hdmi.c
+++ b/sound/pci/hda/patch_hdmi.c
@@ -510,7 +510,7 @@ static int eld_proc_new(struct hdmi_spec_per_pin *per_pin, int index)
 
 	snd_info_set_text_ops(entry, per_pin, print_eld_info);
 	entry->c.text.write = write_eld_info;
-	entry->mode |= S_IWUSR;
+	entry->mode |= 0200;
 	per_pin->proc_entry = entry;
 
 	return 0;
diff --git a/sound/pci/ice1712/pontis.c b/sound/pci/ice1712/pontis.c
index 5101f40f6fbd..93b8cfc6636f 100644
--- a/sound/pci/ice1712/pontis.c
+++ b/sound/pci/ice1712/pontis.c
@@ -662,7 +662,7 @@ static void wm_proc_init(struct snd_ice1712 *ice)
 	struct snd_info_entry *entry;
 	if (! snd_card_proc_new(ice->card, "wm_codec", &entry)) {
 		snd_info_set_text_ops(entry, ice, wm_proc_regs_read);
-		entry->mode |= S_IWUSR;
+		entry->mode |= 0200;
 		entry->c.text.write = wm_proc_regs_write;
 	}
 }
diff --git a/sound/pci/ice1712/prodigy_hifi.c b/sound/pci/ice1712/prodigy_hifi.c
index 8dabd4d0211d..d7366ade5a25 100644
--- a/sound/pci/ice1712/prodigy_hifi.c
+++ b/sound/pci/ice1712/prodigy_hifi.c
@@ -926,7 +926,7 @@ static void wm_proc_init(struct snd_ice1712 *ice)
 	struct snd_info_entry *entry;
 	if (!snd_card_proc_new(ice->card, "wm_codec", &entry)) {
 		snd_info_set_text_ops(entry, ice, wm_proc_regs_read);
-		entry->mode |= S_IWUSR;
+		entry->mode |= 0200;
 		entry->c.text.write = wm_proc_regs_write;
 	}
 }
diff --git a/sound/pci/lola/lola_proc.c b/sound/pci/lola/lola_proc.c
index c241dc06dd92..904e3c4f4dfe 100644
--- a/sound/pci/lola/lola_proc.c
+++ b/sound/pci/lola/lola_proc.c
@@ -214,7 +214,7 @@ void lola_proc_debug_new(struct lola *chip)
 		snd_info_set_text_ops(entry, chip, lola_proc_codec_read);
 	if (!snd_card_proc_new(chip->card, "codec_rw", &entry)) {
 		snd_info_set_text_ops(entry, chip, lola_proc_codec_rw_read);
-		entry->mode |= S_IWUSR;
+		entry->mode |= 0200;
 		entry->c.text.write = lola_proc_codec_rw_write;
 	}
 	if (!snd_card_proc_new(chip->card, "regs", &entry))
diff --git a/sound/pci/pcxhr/pcxhr.c b/sound/pci/pcxhr/pcxhr.c
index f9ae72f28ddc..e57da4036231 100644
--- a/sound/pci/pcxhr/pcxhr.c
+++ b/sound/pci/pcxhr/pcxhr.c
@@ -1465,7 +1465,7 @@ static void pcxhr_proc_init(struct snd_pcxhr *chip)
 	    !snd_card_proc_new(chip->card, "gpio", &entry)) {
 		snd_info_set_text_ops(entry, chip, pcxhr_proc_gpio_read);
 		entry->c.text.write = pcxhr_proc_gpo_write;
-		entry->mode |= S_IWUSR;
+		entry->mode |= 0200;
 	}
 	if (!snd_card_proc_new(chip->card, "ltc", &entry))
 		snd_info_set_text_ops(entry, chip, pcxhr_proc_ltc);
diff --git a/sound/soc/codecs/cs43130.c b/sound/soc/codecs/cs43130.c
index feca0a672976..80dc42197154 100644
--- a/sound/soc/codecs/cs43130.c
+++ b/sound/soc/codecs/cs43130.c
@@ -1733,10 +1733,10 @@ static ssize_t cs43130_show_ac_r(struct device *dev,
 	return cs43130_show_ac(dev, buf, HP_RIGHT);
 }
 
-static DEVICE_ATTR(hpload_dc_l, S_IRUGO, cs43130_show_dc_l, NULL);
-static DEVICE_ATTR(hpload_dc_r, S_IRUGO, cs43130_show_dc_r, NULL);
-static DEVICE_ATTR(hpload_ac_l, S_IRUGO, cs43130_show_ac_l, NULL);
-static DEVICE_ATTR(hpload_ac_r, S_IRUGO, cs43130_show_ac_r, NULL);
+static DEVICE_ATTR(hpload_dc_l, 0444, cs43130_show_dc_l, NULL);
+static DEVICE_ATTR(hpload_dc_r, 0444, cs43130_show_dc_r, NULL);
+static DEVICE_ATTR(hpload_ac_l, 0444, cs43130_show_ac_l, NULL);
+static DEVICE_ATTR(hpload_ac_r, 0444, cs43130_show_ac_r, NULL);
 
 static struct reg_sequence hp_en_cal_seq[] = {
 	{CS43130_INT_MASK_4, CS43130_INT_MASK_ALL},
diff --git a/sound/soc/codecs/wm_adsp.c b/sound/soc/codecs/wm_adsp.c
index b7b914963c62..2175dccdf388 100644
--- a/sound/soc/codecs/wm_adsp.c
+++ b/sound/soc/codecs/wm_adsp.c
@@ -627,22 +627,21 @@ static void wm_adsp2_init_debugfs(struct wm_adsp *dsp,
 	if (!root)
 		goto err;
 
-	if (!debugfs_create_bool("booted", S_IRUGO, root, &dsp->booted))
+	if (!debugfs_create_bool("booted", 0444, root, &dsp->booted))
 		goto err;
 
-	if (!debugfs_create_bool("running", S_IRUGO, root, &dsp->running))
+	if (!debugfs_create_bool("running", 0444, root, &dsp->running))
 		goto err;
 
-	if (!debugfs_create_x32("fw_id", S_IRUGO, root, &dsp->fw_id))
+	if (!debugfs_create_x32("fw_id", 0444, root, &dsp->fw_id))
 		goto err;
 
-	if (!debugfs_create_x32("fw_version", S_IRUGO, root,
-				&dsp->fw_id_version))
+	if (!debugfs_create_x32("fw_version", 0444, root, &dsp->fw_id_version))
 		goto err;
 
 	for (i = 0; i < ARRAY_SIZE(wm_adsp_debugfs_fops); ++i) {
 		if (!debugfs_create_file(wm_adsp_debugfs_fops[i].name,
-					 S_IRUGO, root, dsp,
+					 0444, root, dsp,
 					 &wm_adsp_debugfs_fops[i].fops))
 			goto err;
 	}
diff --git a/sound/soc/fsl/fsl_ssi_dbg.c b/sound/soc/fsl/fsl_ssi_dbg.c
index 1bacfa24ba7f..1255dfe19eef 100644
--- a/sound/soc/fsl/fsl_ssi_dbg.c
+++ b/sound/soc/fsl/fsl_ssi_dbg.c
@@ -142,7 +142,7 @@ int fsl_ssi_debugfs_create(struct fsl_ssi_dbg *ssi_dbg, struct device *dev)
 	if (!ssi_dbg->dbg_dir)
 		return -ENOMEM;
 
-	ssi_dbg->dbg_stats = debugfs_create_file("stats", S_IRUGO,
+	ssi_dbg->dbg_stats = debugfs_create_file("stats", 0444,
 						 ssi_dbg->dbg_dir, ssi_dbg,
 						 &fsl_ssi_stats_ops);
 	if (!ssi_dbg->dbg_stats) {
diff --git a/sound/sound_core.c b/sound/sound_core.c
index b4efb22db561..40ad000c2e3c 100644
--- a/sound/sound_core.c
+++ b/sound/sound_core.c
@@ -413,7 +413,7 @@ int register_sound_special_device(const struct file_operations *fops, int unit,
 		break;
 	}
 	return sound_insert_unit(&chains[chain], fops, -1, unit, max_unit,
-				 name, S_IRUSR | S_IWUSR, dev);
+				 name, 0600, dev);
 }
  
 EXPORT_SYMBOL(register_sound_special_device);
@@ -440,7 +440,7 @@ EXPORT_SYMBOL(register_sound_special);
 int register_sound_mixer(const struct file_operations *fops, int dev)
 {
 	return sound_insert_unit(&chains[0], fops, dev, 0, 128,
-				 "mixer", S_IRUSR | S_IWUSR, NULL);
+				 "mixer", 0600, NULL);
 }
 
 EXPORT_SYMBOL(register_sound_mixer);
@@ -468,7 +468,7 @@ EXPORT_SYMBOL(register_sound_mixer);
 int register_sound_dsp(const struct file_operations *fops, int dev)
 {
 	return sound_insert_unit(&chains[3], fops, dev, 3, 131,
-				 "dsp", S_IWUSR | S_IRUSR, NULL);
+				 "dsp", 0600, NULL);
 }
 
 EXPORT_SYMBOL(register_sound_dsp);
diff --git a/sound/sparc/dbri.c b/sound/sparc/dbri.c
index f0e713527e91..7609eceba1a2 100644
--- a/sound/sparc/dbri.c
+++ b/sound/sparc/dbri.c
@@ -2518,7 +2518,7 @@ static void snd_dbri_proc(struct snd_card *card)
 #ifdef DBRI_DEBUG
 	if (!snd_card_proc_new(card, "debug", &entry)) {
 		snd_info_set_text_ops(entry, dbri, dbri_debug_read);
-		entry->mode = S_IFREG | S_IRUGO;	/* Readable only. */
+		entry->mode = S_IFREG | 0444;	/* Readable only. */
 	}
 #endif
 }
-- 
2.15.0

^ permalink raw reply related

* Re: [PATCH 1/2] powerpc/xmon: Specify the full format in DUMP() macro
From: Mathieu Malaterre @ 2018-05-23 18:55 UTC (permalink / raw)
  To: Michael Ellerman; +Cc: linuxppc-dev
In-Reply-To: <20180523114837.6980-1-mpe@ellerman.id.au>

On Wed, May 23, 2018 at 1:48 PM, Michael Ellerman <mpe@ellerman.id.au> wrote:
> In dump_one_paca() the DUMP macro unconditionally prepends '#' to the
> printf format specifier. In most cases we're using either 'x' or 'lx'
> etc. and that is OK. But for 'p' and other formats using '#' is
> actually undefined, and once we enable printf() checking for
> xmon_printf() we will get warnings from the compiler.

Much more elegant, thanks.

All %#-* and %-* formatters are correct. The change to 22 is correct
since both SLB_NUM_BOLTED & SLB_CACHE_ENTRIES < 10.

For the series:

Reviewed-by: Mathieu Malaterre <malat@debian.org>

> So just have each usage specify the full format, that way we can omit
> '#' when it's inappropriate.
>
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> ---
>  arch/powerpc/xmon/xmon.c | 102 +++++++++++++++++++++++------------------------
>  1 file changed, 51 insertions(+), 51 deletions(-)
>
> diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
> index 064e70a59b47..a81aa3afd87e 100644
> --- a/arch/powerpc/xmon/xmon.c
> +++ b/arch/powerpc/xmon/xmon.c
> @@ -2349,27 +2349,27 @@ static void dump_one_paca(int cpu)
>         printf(" %-*s = %s\n", 20, "present", cpu_present(cpu) ? "yes" : "no");
>         printf(" %-*s = %s\n", 20, "online", cpu_online(cpu) ? "yes" : "no");
>
> -#define DUMP(paca, name, format) \
> -       printf(" %-*s = %#-*"format"\t(0x%lx)\n", 20, #name, 18, paca->name, \
> +#define DUMP(paca, name, format)                               \
> +       printf(" %-*s = "format"\t(0x%lx)\n", 20, #name, 18, paca->name, \
>                 offsetof(struct paca_struct, name));
>
> -       DUMP(p, lock_token, "x");
> -       DUMP(p, paca_index, "x");
> -       DUMP(p, kernel_toc, "llx");
> -       DUMP(p, kernelbase, "llx");
> -       DUMP(p, kernel_msr, "llx");
> -       DUMP(p, emergency_sp, "px");
> +       DUMP(p, lock_token, "%#-*x");
> +       DUMP(p, paca_index, "%#-*x");
> +       DUMP(p, kernel_toc, "%#-*llx");
> +       DUMP(p, kernelbase, "%#-*llx");
> +       DUMP(p, kernel_msr, "%#-*llx");
> +       DUMP(p, emergency_sp, "%-*px");
>  #ifdef CONFIG_PPC_BOOK3S_64
> -       DUMP(p, nmi_emergency_sp, "px");
> -       DUMP(p, mc_emergency_sp, "px");
> -       DUMP(p, in_nmi, "x");
> -       DUMP(p, in_mce, "x");
> -       DUMP(p, hmi_event_available, "x");
> +       DUMP(p, nmi_emergency_sp, "%-*px");
> +       DUMP(p, mc_emergency_sp, "%-*px");
> +       DUMP(p, in_nmi, "%#-*x");
> +       DUMP(p, in_mce, "%#-*x");
> +       DUMP(p, hmi_event_available, "%#-*x");
>  #endif
> -       DUMP(p, data_offset, "llx");
> -       DUMP(p, hw_cpu_id, "x");
> -       DUMP(p, cpu_start, "x");
> -       DUMP(p, kexec_state, "x");
> +       DUMP(p, data_offset, "%#-*llx");
> +       DUMP(p, hw_cpu_id, "%#-*x");
> +       DUMP(p, cpu_start, "%#-*x");
> +       DUMP(p, kexec_state, "%#-*x");
>  #ifdef CONFIG_PPC_BOOK3S_64
>         for (i = 0; i < SLB_NUM_BOLTED; i++) {
>                 u64 esid, vsid;
> @@ -2385,54 +2385,54 @@ static void dump_one_paca(int cpu)
>                                 i, esid, vsid);
>                 }
>         }
> -       DUMP(p, vmalloc_sllp, "x");
> -       DUMP(p, slb_cache_ptr, "x");
> +       DUMP(p, vmalloc_sllp, "%#-*x");
> +       DUMP(p, slb_cache_ptr, "%#-*x");
>         for (i = 0; i < SLB_CACHE_ENTRIES; i++)
>                 printf(" slb_cache[%d]:        = 0x%016x\n", i, p->slb_cache[i]);
>
> -       DUMP(p, rfi_flush_fallback_area, "px");
> +       DUMP(p, rfi_flush_fallback_area, "%-*px");
>  #endif
> -       DUMP(p, dscr_default, "llx");
> +       DUMP(p, dscr_default, "%#-*llx");
>  #ifdef CONFIG_PPC_BOOK3E
> -       DUMP(p, pgd, "px");
> -       DUMP(p, kernel_pgd, "px");
> -       DUMP(p, tcd_ptr, "px");
> -       DUMP(p, mc_kstack, "px");
> -       DUMP(p, crit_kstack, "px");
> -       DUMP(p, dbg_kstack, "px");
> +       DUMP(p, pgd, "%-*px");
> +       DUMP(p, kernel_pgd, "%-*px");
> +       DUMP(p, tcd_ptr, "%-*px");
> +       DUMP(p, mc_kstack, "%-*px");
> +       DUMP(p, crit_kstack, "%-*px");
> +       DUMP(p, dbg_kstack, "%-*px");
>  #endif
> -       DUMP(p, __current, "px");
> -       DUMP(p, kstack, "llx");
> +       DUMP(p, __current, "%-*px");
> +       DUMP(p, kstack, "%#-*llx");
>         printf(" kstack_base          = 0x%016llx\n", p->kstack & ~(THREAD_SIZE - 1));
> -       DUMP(p, stab_rr, "llx");
> -       DUMP(p, saved_r1, "llx");
> -       DUMP(p, trap_save, "x");
> -       DUMP(p, irq_soft_mask, "x");
> -       DUMP(p, irq_happened, "x");
> -       DUMP(p, io_sync, "x");
> -       DUMP(p, irq_work_pending, "x");
> -       DUMP(p, nap_state_lost, "x");
> -       DUMP(p, sprg_vdso, "llx");
> +       DUMP(p, stab_rr, "%#-*llx");
> +       DUMP(p, saved_r1, "%#-*llx");
> +       DUMP(p, trap_save, "%#-*x");
> +       DUMP(p, irq_soft_mask, "%#-*x");
> +       DUMP(p, irq_happened, "%#-*x");
> +       DUMP(p, io_sync, "%#-*x");
> +       DUMP(p, irq_work_pending, "%#-*x");
> +       DUMP(p, nap_state_lost, "%#-*x");
> +       DUMP(p, sprg_vdso, "%#-*llx");
>
>  #ifdef CONFIG_PPC_TRANSACTIONAL_MEM
> -       DUMP(p, tm_scratch, "llx");
> +       DUMP(p, tm_scratch, "%#-*llx");
>  #endif
>
>  #ifdef CONFIG_PPC_POWERNV
> -       DUMP(p, core_idle_state_ptr, "px");
> -       DUMP(p, thread_idle_state, "x");
> -       DUMP(p, thread_mask, "x");
> -       DUMP(p, subcore_sibling_mask, "x");
> +       DUMP(p, core_idle_state_ptr, "%-*px");
> +       DUMP(p, thread_idle_state, "%#-*x");
> +       DUMP(p, thread_mask, "%#-*x");
> +       DUMP(p, subcore_sibling_mask, "%#-*x");
>  #endif
>
> -       DUMP(p, accounting.utime, "lx");
> -       DUMP(p, accounting.stime, "lx");
> -       DUMP(p, accounting.utime_scaled, "lx");
> -       DUMP(p, accounting.starttime, "lx");
> -       DUMP(p, accounting.starttime_user, "lx");
> -       DUMP(p, accounting.startspurr, "lx");
> -       DUMP(p, accounting.utime_sspurr, "lx");
> -       DUMP(p, accounting.steal_time, "lx");
> +       DUMP(p, accounting.utime, "%#-*lx");
> +       DUMP(p, accounting.stime, "%#-*lx");
> +       DUMP(p, accounting.utime_scaled, "%#-*lx");
> +       DUMP(p, accounting.starttime, "%#-*lx");
> +       DUMP(p, accounting.starttime_user, "%#-*lx");
> +       DUMP(p, accounting.startspurr, "%#-*lx");
> +       DUMP(p, accounting.utime_sspurr, "%#-*lx");
> +       DUMP(p, accounting.steal_time, "%#-*lx");
>  #undef DUMP
>
>         catch_memory_errors = 0;
> --
> 2.14.1
>

^ permalink raw reply

* Re: [RFC V2] virtio: Add platform specific DMA API translation for virito devices
From: Michael S. Tsirkin @ 2018-05-23 18:50 UTC (permalink / raw)
  To: Anshuman Khandual
  Cc: virtualization, linux-kernel, linuxppc-dev, aik, robh, joe,
	elfring, david, jasowang, benh, mpe, hch
In-Reply-To: <20180522063317.20956-1-khandual@linux.vnet.ibm.com>

subj: s/virito/virtio/

On Tue, May 22, 2018 at 12:03:17PM +0530, Anshuman Khandual wrote:
> This adds a hook which a platform can define in order to allow it to
> force the use of the DMA API for all virtio devices even if they don't
> have the VIRTIO_F_IOMMU_PLATFORM flag set.  We want to use this to do
> bounce-buffering of data on the new secure pSeries platform, currently
> under development, where a KVM host cannot access all of the memory
> space of a secure KVM guest.  The host can only access the pages which
> the guest has explicitly requested to be shared with the host, thus
> the virtio implementation in the guest has to copy data to and from
> shared pages.
> 
> With this hook, the platform code in the secure guest can force the
> use of swiotlb for virtio buffers, with a back-end for swiotlb which
> will use a pool of pre-allocated shared pages.  Thus all data being
> sent or received by virtio devices will be copied through pages which
> the host has access to.
> 
> Signed-off-by: Anshuman Khandual <khandual@linux.vnet.ibm.com>
> ---
> Changes in V2:
> 
> The arch callback has been enabled through an weak symbol defintion
> so that it is enabled only for those architectures subscribing to
> this new framework. Clarified the patch description. The primary
> objective for this RFC has been to get an in principle agreement
> on this approach.
> 
> Original V1:
> 
> Original RFC and discussions https://patchwork.kernel.org/patch/10324405/

I re-read that discussion and I'm still unclear on the
original question, since I got several apparently
conflicting answers.

I asked:

	Why isn't setting VIRTIO_F_IOMMU_PLATFORM on the
	hypervisor side sufficient?



>  arch/powerpc/include/asm/dma-mapping.h |  6 ++++++
>  arch/powerpc/platforms/pseries/iommu.c | 11 +++++++++++
>  drivers/virtio/virtio_ring.c           | 10 ++++++++++
>  3 files changed, 27 insertions(+)
> 
> diff --git a/arch/powerpc/include/asm/dma-mapping.h b/arch/powerpc/include/asm/dma-mapping.h
> index 8fa3945..056e578 100644
> --- a/arch/powerpc/include/asm/dma-mapping.h
> +++ b/arch/powerpc/include/asm/dma-mapping.h
> @@ -115,4 +115,10 @@ extern u64 __dma_get_required_mask(struct device *dev);
>  #define ARCH_HAS_DMA_MMAP_COHERENT
>  
>  #endif /* __KERNEL__ */
> +
> +#define platform_forces_virtio_dma platform_forces_virtio_dma
> +
> +struct virtio_device;
> +
> +extern bool platform_forces_virtio_dma(struct virtio_device *vdev);
>  #endif	/* _ASM_DMA_MAPPING_H */
> diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c
> index 06f0296..a2ec15a 100644
> --- a/arch/powerpc/platforms/pseries/iommu.c
> +++ b/arch/powerpc/platforms/pseries/iommu.c
> @@ -38,6 +38,7 @@
>  #include <linux/of.h>
>  #include <linux/iommu.h>
>  #include <linux/rculist.h>
> +#include <linux/virtio.h>
>  #include <asm/io.h>
>  #include <asm/prom.h>
>  #include <asm/rtas.h>
> @@ -1396,3 +1397,13 @@ static int __init disable_multitce(char *str)
>  __setup("multitce=", disable_multitce);
>  
>  machine_subsys_initcall_sync(pseries, tce_iommu_bus_notifier_init);
> +
> +bool platform_forces_virtio_dma(struct virtio_device *vdev)
> +{
> +	/*
> +	 * On protected guest platforms, force virtio core to use DMA
> +	 * MAP API for all virtio devices. But there can also be some
> +	 * exceptions for individual devices like virtio balloon.
> +	 */
> +	return (of_find_compatible_node(NULL, NULL, "ibm,ultravisor") != NULL);
> +}

Isn't this kind of slow?  vring_use_dma_api is on
data path and supposed to be very fast.

> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> index 21d464a..47ea6c3 100644
> --- a/drivers/virtio/virtio_ring.c
> +++ b/drivers/virtio/virtio_ring.c
> @@ -141,8 +141,18 @@ struct vring_virtqueue {
>   * unconditionally on data path.
>   */
>  
> +#ifndef platform_forces_virtio_dma
> +static inline bool platform_forces_virtio_dma(struct virtio_device *vdev)
> +{
> +	return false;
> +}
> +#endif
> +
>  static bool vring_use_dma_api(struct virtio_device *vdev)
>  {
> +	if (platform_forces_virtio_dma(vdev))
> +		return true;
> +
>  	if (!virtio_has_iommu_quirk(vdev))
>  		return true;
>  
> -- 
> 2.9.3

^ permalink raw reply

* [PATCH net-next 7/8] ibmvnic: Set resetting state at earliest possible point
From: Thomas Falcon @ 2018-05-23 18:38 UTC (permalink / raw)
  To: netdev; +Cc: nfont, jallen, linuxppc-dev, Thomas Falcon
In-Reply-To: <1527100682-23099-1-git-send-email-tlfalcon@linux.vnet.ibm.com>

Set device resetting state at the earliest possible point: as soon as a
reset is successfully scheduled. The reset state is toggled off when
all resets have been processed to completion.

Signed-off-by: Thomas Falcon <tlfalcon@linux.vnet.ibm.com>
---
 drivers/net/ethernet/ibm/ibmvnic.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index f26e1f8..ee51deb 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -1919,7 +1919,6 @@ static void __ibmvnic_reset(struct work_struct *work)
 	netdev = adapter->netdev;
 
 	mutex_lock(&adapter->reset_lock);
-	adapter->resetting = true;
 	reset_state = adapter->state;
 
 	rwi = get_next_rwi(adapter);
@@ -1994,7 +1993,7 @@ static int ibmvnic_reset(struct ibmvnic_adapter *adapter,
 	rwi->reset_reason = reason;
 	list_add_tail(&rwi->list, &adapter->rwi_list);
 	mutex_unlock(&adapter->rwi_lock);
-
+	adapter->resetting = true;
 	netdev_dbg(adapter->netdev, "Scheduling reset (reason %d)\n", reason);
 	schedule_work(&adapter->ibmvnic_reset);
 
-- 
2.7.5

^ permalink raw reply related

* [PATCH net-next 8/8] ibmvnic: Introduce hard reset recovery
From: Thomas Falcon @ 2018-05-23 18:38 UTC (permalink / raw)
  To: netdev; +Cc: nfont, jallen, linuxppc-dev, Thomas Falcon
In-Reply-To: <1527100682-23099-1-git-send-email-tlfalcon@linux.vnet.ibm.com>

Introduce a recovery hard reset to handle reset failure as a result of
change of device context following a transport event, such as a
backing device failover or partition migration. These operations reset
the device context to its initial state. If this occurs during a reset,
any initialization commands are likely to fail with an invalid state
error as backing device firmware requests reinitialization.

When this happens, make one more attempt by performing a hard reset,
which frees any resources currently allocated and performs device
initialization. If a transport event occurs during a device reset, a
flag is set which will trigger a new hard reset following the
completionof the current reset event.

Signed-off-by: Thomas Falcon <tlfalcon@linux.vnet.ibm.com>
---
 drivers/net/ethernet/ibm/ibmvnic.c | 101 +++++++++++++++++++++++++++++++++++--
 drivers/net/ethernet/ibm/ibmvnic.h |   1 +
 2 files changed, 98 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index ee51deb..09f8e6b 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -1878,6 +1878,85 @@ static int do_reset(struct ibmvnic_adapter *adapter,
 	return 0;
 }
 
+static int do_hard_reset(struct ibmvnic_adapter *adapter,
+			 struct ibmvnic_rwi *rwi, u32 reset_state)
+{
+	struct net_device *netdev = adapter->netdev;
+	int rc;
+
+	netdev_dbg(adapter->netdev, "Hard resetting driver (%d)\n",
+		   rwi->reset_reason);
+
+	netif_carrier_off(netdev);
+	adapter->reset_reason = rwi->reset_reason;
+
+	ibmvnic_cleanup(netdev);
+	release_resources(adapter);
+	release_sub_crqs(adapter, 0);
+	release_crq_queue(adapter);
+
+	/* remove the closed state so when we call open it appears
+	 * we are coming from the probed state.
+	 */
+	adapter->state = VNIC_PROBED;
+
+	rc = init_crq_queue(adapter);
+	if (rc) {
+		netdev_err(adapter->netdev,
+			   "Couldn't initialize crq. rc=%d\n", rc);
+		return rc;
+	}
+
+	rc = ibmvnic_init(adapter);
+	if (rc)
+		return rc;
+
+	/* If the adapter was in PROBE state prior to the reset,
+	 * exit here.
+	 */
+	if (reset_state == VNIC_PROBED)
+		return 0;
+
+	rc = ibmvnic_login(netdev);
+	if (rc) {
+		adapter->state = VNIC_PROBED;
+		return 0;
+	}
+	/* netif_set_real_num_xx_queues needs to take rtnl lock here
+	 * unless wait_for_reset is set, in which case the rtnl lock
+	 * has already been taken before initializing the reset
+	 */
+	if (!adapter->wait_for_reset) {
+		rtnl_lock();
+		rc = init_resources(adapter);
+		rtnl_unlock();
+	} else {
+		rc = init_resources(adapter);
+	}
+	if (rc)
+		return rc;
+
+	ibmvnic_disable_irqs(adapter);
+	adapter->state = VNIC_CLOSED;
+
+	if (reset_state == VNIC_CLOSED)
+		return 0;
+
+	rc = __ibmvnic_open(netdev);
+	if (rc) {
+		if (list_empty(&adapter->rwi_list))
+			adapter->state = VNIC_CLOSED;
+		else
+			adapter->state = reset_state;
+
+		return 0;
+	}
+
+	netif_carrier_on(netdev);
+
+	return 0;
+}
+
 static struct ibmvnic_rwi *get_next_rwi(struct ibmvnic_adapter *adapter)
 {
 	struct ibmvnic_rwi *rwi;
@@ -1923,9 +2002,15 @@ static void __ibmvnic_reset(struct work_struct *work)
 
 	rwi = get_next_rwi(adapter);
 	while (rwi) {
-		rc = do_reset(adapter, rwi, reset_state);
+		if (adapter->force_reset_recovery) {
+			adapter->force_reset_recovery = false;
+			rc = do_hard_reset(adapter, rwi, reset_state);
+		} else {
+			rc = do_reset(adapter, rwi, reset_state);
+		}
 		kfree(rwi);
-		if (rc && rc != IBMVNIC_INIT_FAILED)
+		if (rc && rc != IBMVNIC_INIT_FAILED &&
+		    !adapter->force_reset_recovery)
 			break;
 
 		rwi = get_next_rwi(adapter);
@@ -1951,9 +2036,9 @@ static void __ibmvnic_reset(struct work_struct *work)
 static int ibmvnic_reset(struct ibmvnic_adapter *adapter,
 			 enum ibmvnic_reset_reason reason)
 {
+	struct list_head *entry, *tmp_entry;
 	struct ibmvnic_rwi *rwi, *tmp;
 	struct net_device *netdev = adapter->netdev;
-	struct list_head *entry;
 	int ret;
 
 	if (adapter->state == VNIC_REMOVING ||
@@ -1989,7 +2074,13 @@ static int ibmvnic_reset(struct ibmvnic_adapter *adapter,
 		ret = ENOMEM;
 		goto err;
 	}
-
+	/* if we just received a transport event,
+	 * flush reset queue and process this reset
+	 */
+	if (adapter->force_reset_recovery && !list_empty(&adapter->rwi_list)) {
+		list_for_each_safe(entry, tmp_entry, &adapter->rwi_list)
+			list_del(entry);
+	}
 	rwi->reset_reason = reason;
 	list_add_tail(&rwi->list, &adapter->rwi_list);
 	mutex_unlock(&adapter->rwi_lock);
@@ -4271,6 +4362,8 @@ static void ibmvnic_handle_crq(union ibmvnic_crq *crq,
 	case IBMVNIC_CRQ_XPORT_EVENT:
 		netif_carrier_off(netdev);
 		adapter->crq.active = false;
+		if (adapter->resetting)
+			adapter->force_reset_recovery = true;
 		if (gen_crq->cmd == IBMVNIC_PARTITION_MIGRATED) {
 			dev_info(dev, "Migrated, re-enabling adapter\n");
 			ibmvnic_reset(adapter, VNIC_RESET_MOBILITY);
diff --git a/drivers/net/ethernet/ibm/ibmvnic.h b/drivers/net/ethernet/ibm/ibmvnic.h
index edfc312..f9fb780 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.h
+++ b/drivers/net/ethernet/ibm/ibmvnic.h
@@ -1109,6 +1109,7 @@ struct ibmvnic_adapter {
 
 	bool mac_change_pending;
 	bool failover_pending;
+	bool force_reset_recovery;
 
 	struct ibmvnic_tunables desired;
 	struct ibmvnic_tunables fallback;
-- 
2.7.5

^ permalink raw reply related

* [PATCH net-next 6/8] ibmvnic: Create separate initialization routine for resets
From: Thomas Falcon @ 2018-05-23 18:38 UTC (permalink / raw)
  To: netdev; +Cc: nfont, jallen, linuxppc-dev, Thomas Falcon
In-Reply-To: <1527100682-23099-1-git-send-email-tlfalcon@linux.vnet.ibm.com>

Instead of having one initialization routine for all cases, create
a separate, simpler function for standard initialization, such as during
device probe. Use the original initialization function to handle
device reset scenarios. The goal of this patch is to avoid having
a single, cluttered init function to handle all possible
scenarios.

Signed-off-by: Thomas Falcon <tlfalcon@linux.vnet.ibm.com>
---
 drivers/net/ethernet/ibm/ibmvnic.c | 48 ++++++++++++++++++++++++++++++++++++--
 1 file changed, 46 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index b1bbd5b..f26e1f8 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -116,6 +116,7 @@ static void send_cap_queries(struct ibmvnic_adapter *adapter);
 static int init_sub_crqs(struct ibmvnic_adapter *);
 static int init_sub_crq_irqs(struct ibmvnic_adapter *adapter);
 static int ibmvnic_init(struct ibmvnic_adapter *);
+static int ibmvnic_reset_init(struct ibmvnic_adapter *);
 static void release_crq_queue(struct ibmvnic_adapter *);
 static int __ibmvnic_set_mac(struct net_device *netdev, struct sockaddr *p);
 static int init_crq_queue(struct ibmvnic_adapter *adapter);
@@ -1807,7 +1808,7 @@ static int do_reset(struct ibmvnic_adapter *adapter,
 			return rc;
 		}
 
-		rc = ibmvnic_init(adapter);
+		rc = ibmvnic_reset_init(adapter);
 		if (rc)
 			return IBMVNIC_INIT_FAILED;
 
@@ -4571,7 +4572,7 @@ static int init_crq_queue(struct ibmvnic_adapter *adapter)
 	return retrc;
 }
 
-static int ibmvnic_init(struct ibmvnic_adapter *adapter)
+static int ibmvnic_reset_init(struct ibmvnic_adapter *adapter)
 {
 	struct device *dev = &adapter->vdev->dev;
 	unsigned long timeout = msecs_to_jiffies(30000);
@@ -4630,6 +4631,49 @@ static int ibmvnic_init(struct ibmvnic_adapter *adapter)
 	return rc;
 }
 
+static int ibmvnic_init(struct ibmvnic_adapter *adapter)
+{
+	struct device *dev = &adapter->vdev->dev;
+	unsigned long timeout = msecs_to_jiffies(30000);
+	int rc;
+
+	adapter->from_passive_init = false;
+
+	init_completion(&adapter->init_done);
+	adapter->init_done_rc = 0;
+	ibmvnic_send_crq_init(adapter);
+	if (!wait_for_completion_timeout(&adapter->init_done, timeout)) {
+		dev_err(dev, "Initialization sequence timed out\n");
+		return -1;
+	}
+
+	if (adapter->init_done_rc) {
+		release_crq_queue(adapter);
+		return adapter->init_done_rc;
+	}
+
+	if (adapter->from_passive_init) {
+		adapter->state = VNIC_OPEN;
+		adapter->from_passive_init = false;
+		return -1;
+	}
+
+	rc = init_sub_crqs(adapter);
+	if (rc) {
+		dev_err(dev, "Initialization of sub crqs failed\n");
+		release_crq_queue(adapter);
+		return rc;
+	}
+
+	rc = init_sub_crq_irqs(adapter);
+	if (rc) {
+		dev_err(dev, "Failed to initialize sub crq irqs\n");
+		release_crq_queue(adapter);
+	}
+
+	return rc;
+}
+
 static struct device_attribute dev_attr_failover;
 
 static int ibmvnic_probe(struct vio_dev *dev, const struct vio_device_id *id)
-- 
2.7.5

^ permalink raw reply related

* [PATCH net-next 5/8] ibmvnic: Handle error case when setting link state
From: Thomas Falcon @ 2018-05-23 18:37 UTC (permalink / raw)
  To: netdev; +Cc: nfont, jallen, linuxppc-dev, Thomas Falcon
In-Reply-To: <1527100682-23099-1-git-send-email-tlfalcon@linux.vnet.ibm.com>

If setting the link state is not successful, print a warning
with the resulting return code and return it to be handled
by the caller.

Signed-off-by: Thomas Falcon <tlfalcon@linux.vnet.ibm.com>
---
 drivers/net/ethernet/ibm/ibmvnic.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index f1f744e..b1bbd5b 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -929,6 +929,10 @@ static int set_link_state(struct ibmvnic_adapter *adapter, u8 link_state)
 			/* Partuial success, delay and re-send */
 			mdelay(1000);
 			resend = true;
+		} else if (adapter->init_done_rc) {
+			netdev_warn(netdev, "Unable to set link state, rc=%d\n",
+				    adapter->init_done_rc);
+			return adapter->init_done_rc;
 		}
 	} while (resend);
 
-- 
2.7.5

^ permalink raw reply related

* [PATCH net-next 4/8] ibmvnic: Return error code if init interrupted by transport event
From: Thomas Falcon @ 2018-05-23 18:37 UTC (permalink / raw)
  To: netdev; +Cc: nfont, jallen, linuxppc-dev, Thomas Falcon
In-Reply-To: <1527100682-23099-1-git-send-email-tlfalcon@linux.vnet.ibm.com>

If device init is interrupted by a failover, set the init return
code so that it can be checked and handled appropriately by the
init routine.

Signed-off-by: Thomas Falcon <tlfalcon@linux.vnet.ibm.com>
---
 drivers/net/ethernet/ibm/ibmvnic.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index 7083519..f1f744e 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -4249,7 +4249,10 @@ static void ibmvnic_handle_crq(union ibmvnic_crq *crq,
 			dev_info(dev, "Partner initialized\n");
 			adapter->from_passive_init = true;
 			adapter->failover_pending = false;
-			complete(&adapter->init_done);
+			if (!completion_done(&adapter->init_done)) {
+				complete(&adapter->init_done);
+				adapter->init_done_rc = -EIO;
+			}
 			ibmvnic_reset(adapter, VNIC_RESET_FAILOVER);
 			break;
 		case IBMVNIC_CRQ_INIT_COMPLETE:
-- 
2.7.5

^ permalink raw reply related

* [PATCH net-next 3/8] ibmvnic: Check CRQ command return codes
From: Thomas Falcon @ 2018-05-23 18:37 UTC (permalink / raw)
  To: netdev; +Cc: nfont, jallen, linuxppc-dev, Thomas Falcon
In-Reply-To: <1527100682-23099-1-git-send-email-tlfalcon@linux.vnet.ibm.com>

Check whether CRQ command is successful before awaiting a response
from the management partition. If the command was not successful, the
driver may hang waiting for a response that will never come.

Signed-off-by: Thomas Falcon <tlfalcon@linux.vnet.ibm.com>
---
 drivers/net/ethernet/ibm/ibmvnic.c | 51 +++++++++++++++++++++++++++-----------
 1 file changed, 37 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index e6a081c..7083519 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -109,8 +109,8 @@ static union sub_crq *ibmvnic_next_scrq(struct ibmvnic_adapter *,
 					struct ibmvnic_sub_crq_queue *);
 static int ibmvnic_poll(struct napi_struct *napi, int data);
 static void send_map_query(struct ibmvnic_adapter *adapter);
-static void send_request_map(struct ibmvnic_adapter *, dma_addr_t, __be32, u8);
-static void send_request_unmap(struct ibmvnic_adapter *, u8);
+static int send_request_map(struct ibmvnic_adapter *, dma_addr_t, __be32, u8);
+static int send_request_unmap(struct ibmvnic_adapter *, u8);
 static int send_login(struct ibmvnic_adapter *adapter);
 static void send_cap_queries(struct ibmvnic_adapter *adapter);
 static int init_sub_crqs(struct ibmvnic_adapter *);
@@ -172,6 +172,7 @@ static int alloc_long_term_buff(struct ibmvnic_adapter *adapter,
 				struct ibmvnic_long_term_buff *ltb, int size)
 {
 	struct device *dev = &adapter->vdev->dev;
+	int rc;
 
 	ltb->size = size;
 	ltb->buff = dma_alloc_coherent(dev, ltb->size, &ltb->addr,
@@ -185,8 +186,12 @@ static int alloc_long_term_buff(struct ibmvnic_adapter *adapter,
 	adapter->map_id++;
 
 	init_completion(&adapter->fw_done);
-	send_request_map(adapter, ltb->addr,
-			 ltb->size, ltb->map_id);
+	rc = send_request_map(adapter, ltb->addr,
+			      ltb->size, ltb->map_id);
+	if (rc) {
+		dma_free_coherent(dev, ltb->size, ltb->buff, ltb->addr);
+		return rc;
+	}
 	wait_for_completion(&adapter->fw_done);
 
 	if (adapter->fw_done_rc) {
@@ -215,10 +220,14 @@ static void free_long_term_buff(struct ibmvnic_adapter *adapter,
 static int reset_long_term_buff(struct ibmvnic_adapter *adapter,
 				struct ibmvnic_long_term_buff *ltb)
 {
+	int rc;
+
 	memset(ltb->buff, 0, ltb->size);
 
 	init_completion(&adapter->fw_done);
-	send_request_map(adapter, ltb->addr, ltb->size, ltb->map_id);
+	rc = send_request_map(adapter, ltb->addr, ltb->size, ltb->map_id);
+	if (rc)
+		return rc;
 	wait_for_completion(&adapter->fw_done);
 
 	if (adapter->fw_done_rc) {
@@ -952,6 +961,7 @@ static int ibmvnic_get_vpd(struct ibmvnic_adapter *adapter)
 	struct device *dev = &adapter->vdev->dev;
 	union ibmvnic_crq crq;
 	int len = 0;
+	int rc;
 
 	if (adapter->vpd->buff)
 		len = adapter->vpd->len;
@@ -959,7 +969,9 @@ static int ibmvnic_get_vpd(struct ibmvnic_adapter *adapter)
 	init_completion(&adapter->fw_done);
 	crq.get_vpd_size.first = IBMVNIC_CRQ_CMD;
 	crq.get_vpd_size.cmd = GET_VPD_SIZE;
-	ibmvnic_send_crq(adapter, &crq);
+	rc = ibmvnic_send_crq(adapter, &crq);
+	if (rc)
+		return rc;
 	wait_for_completion(&adapter->fw_done);
 
 	if (!adapter->vpd->len)
@@ -992,7 +1004,12 @@ static int ibmvnic_get_vpd(struct ibmvnic_adapter *adapter)
 	crq.get_vpd.cmd = GET_VPD;
 	crq.get_vpd.ioba = cpu_to_be32(adapter->vpd->dma_addr);
 	crq.get_vpd.len = cpu_to_be32((u32)adapter->vpd->len);
-	ibmvnic_send_crq(adapter, &crq);
+	rc = ibmvnic_send_crq(adapter, &crq);
+	if (rc) {
+		kfree(adapter->vpd->buff);
+		adapter->vpd->buff = NULL;
+		return rc;
+	}
 	wait_for_completion(&adapter->fw_done);
 
 	return 0;
@@ -1691,6 +1708,7 @@ static int __ibmvnic_set_mac(struct net_device *netdev, struct sockaddr *p)
 	struct ibmvnic_adapter *adapter = netdev_priv(netdev);
 	struct sockaddr *addr = p;
 	union ibmvnic_crq crq;
+	int rc;
 
 	if (!is_valid_ether_addr(addr->sa_data))
 		return -EADDRNOTAVAIL;
@@ -1701,7 +1719,9 @@ static int __ibmvnic_set_mac(struct net_device *netdev, struct sockaddr *p)
 	ether_addr_copy(&crq.change_mac_addr.mac_addr[0], addr->sa_data);
 
 	init_completion(&adapter->fw_done);
-	ibmvnic_send_crq(adapter, &crq);
+	rc = ibmvnic_send_crq(adapter, &crq);
+	if (rc)
+		return rc;
 	wait_for_completion(&adapter->fw_done);
 	/* netdev->dev_addr is changed in handle_change_mac_rsp function */
 	return adapter->fw_done_rc ? -EIO : 0;
@@ -2365,6 +2385,7 @@ static void ibmvnic_get_ethtool_stats(struct net_device *dev,
 	struct ibmvnic_adapter *adapter = netdev_priv(dev);
 	union ibmvnic_crq crq;
 	int i, j;
+	int rc;
 
 	memset(&crq, 0, sizeof(crq));
 	crq.request_statistics.first = IBMVNIC_CRQ_CMD;
@@ -2375,7 +2396,9 @@ static void ibmvnic_get_ethtool_stats(struct net_device *dev,
 
 	/* Wait for data to be written */
 	init_completion(&adapter->stats_done);
-	ibmvnic_send_crq(adapter, &crq);
+	rc = ibmvnic_send_crq(adapter, &crq);
+	if (rc)
+		return;
 	wait_for_completion(&adapter->stats_done);
 
 	for (i = 0; i < ARRAY_SIZE(ibmvnic_stats); i++)
@@ -3377,8 +3400,8 @@ static int send_login(struct ibmvnic_adapter *adapter)
 	return -1;
 }
 
-static void send_request_map(struct ibmvnic_adapter *adapter, dma_addr_t addr,
-			     u32 len, u8 map_id)
+static int send_request_map(struct ibmvnic_adapter *adapter, dma_addr_t addr,
+			    u32 len, u8 map_id)
 {
 	union ibmvnic_crq crq;
 
@@ -3388,10 +3411,10 @@ static void send_request_map(struct ibmvnic_adapter *adapter, dma_addr_t addr,
 	crq.request_map.map_id = map_id;
 	crq.request_map.ioba = cpu_to_be32(addr);
 	crq.request_map.len = cpu_to_be32(len);
-	ibmvnic_send_crq(adapter, &crq);
+	return ibmvnic_send_crq(adapter, &crq);
 }
 
-static void send_request_unmap(struct ibmvnic_adapter *adapter, u8 map_id)
+static int send_request_unmap(struct ibmvnic_adapter *adapter, u8 map_id)
 {
 	union ibmvnic_crq crq;
 
@@ -3399,7 +3422,7 @@ static void send_request_unmap(struct ibmvnic_adapter *adapter, u8 map_id)
 	crq.request_unmap.first = IBMVNIC_CRQ_CMD;
 	crq.request_unmap.cmd = REQUEST_UNMAP;
 	crq.request_unmap.map_id = map_id;
-	ibmvnic_send_crq(adapter, &crq);
+	return ibmvnic_send_crq(adapter, &crq);
 }
 
 static void send_map_query(struct ibmvnic_adapter *adapter)
-- 
2.7.5

^ permalink raw reply related

* [PATCH net-next 2/8] ibmvnic: Introduce active CRQ state
From: Thomas Falcon @ 2018-05-23 18:37 UTC (permalink / raw)
  To: netdev; +Cc: nfont, jallen, linuxppc-dev, Thomas Falcon
In-Reply-To: <1527100682-23099-1-git-send-email-tlfalcon@linux.vnet.ibm.com>

Introduce an "active" state for a IBM vNIC Command-Response Queue. A CRQ
is considered active once it has initialized or linked with its partner by
sending an initialization request and getting a successful response back
from the management partition.  Until this has happened, do not allow CRQ
commands to be sent other than the initialization request.

This change will avoid a protocol error in case of a device transport
event occurring during a initialization. When the driver receives a
transport event notification indicating that the backing hardware
has changed and needs reinitialization, any further commands other
than the initialization handshake with the VIOS management partition
will result in an invalid state error. Instead of sending a command
that will be returned with an error, print a warning and return an
error that will be handled by the caller.

Signed-off-by: Thomas Falcon <tlfalcon@linux.vnet.ibm.com>
---
 drivers/net/ethernet/ibm/ibmvnic.c | 10 ++++++++++
 drivers/net/ethernet/ibm/ibmvnic.h |  1 +
 2 files changed, 11 insertions(+)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index eabc1e4..e6a081c 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -3147,6 +3147,12 @@ static int ibmvnic_send_crq(struct ibmvnic_adapter *adapter,
 		   (unsigned long int)cpu_to_be64(u64_crq[0]),
 		   (unsigned long int)cpu_to_be64(u64_crq[1]));
 
+	if (!adapter->crq.active &&
+	    crq->generic.first != IBMVNIC_CRQ_INIT_CMD) {
+		dev_warn(dev, "Invalid request detected while CRQ is inactive, possible device state change during reset\n");
+		return -EINVAL;
+	}
+
 	/* Make sure the hypervisor sees the complete request */
 	mb();
 
@@ -4225,6 +4231,7 @@ static void ibmvnic_handle_crq(union ibmvnic_crq *crq,
 			break;
 		case IBMVNIC_CRQ_INIT_COMPLETE:
 			dev_info(dev, "Partner initialization complete\n");
+			adapter->crq.active = true;
 			send_version_xchg(adapter);
 			break;
 		default:
@@ -4233,6 +4240,7 @@ static void ibmvnic_handle_crq(union ibmvnic_crq *crq,
 		return;
 	case IBMVNIC_CRQ_XPORT_EVENT:
 		netif_carrier_off(netdev);
+		adapter->crq.active = false;
 		if (gen_crq->cmd == IBMVNIC_PARTITION_MIGRATED) {
 			dev_info(dev, "Migrated, re-enabling adapter\n");
 			ibmvnic_reset(adapter, VNIC_RESET_MOBILITY);
@@ -4420,6 +4428,7 @@ static int ibmvnic_reset_crq(struct ibmvnic_adapter *adapter)
 	/* Clean out the queue */
 	memset(crq->msgs, 0, PAGE_SIZE);
 	crq->cur = 0;
+	crq->active = false;
 
 	/* And re-open it again */
 	rc = plpar_hcall_norets(H_REG_CRQ, vdev->unit_address,
@@ -4454,6 +4463,7 @@ static void release_crq_queue(struct ibmvnic_adapter *adapter)
 			 DMA_BIDIRECTIONAL);
 	free_page((unsigned long)crq->msgs);
 	crq->msgs = NULL;
+	crq->active = false;
 }
 
 static int init_crq_queue(struct ibmvnic_adapter *adapter)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.h b/drivers/net/ethernet/ibm/ibmvnic.h
index 22391e8..edfc312 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.h
+++ b/drivers/net/ethernet/ibm/ibmvnic.h
@@ -865,6 +865,7 @@ struct ibmvnic_crq_queue {
 	int size, cur;
 	dma_addr_t msg_token;
 	spinlock_t lock;
+	bool active;
 };
 
 union sub_crq {
-- 
2.7.5

^ permalink raw reply related

* [PATCH net-next 1/8] ibmvnic: Mark NAPI flag as disabled when released
From: Thomas Falcon @ 2018-05-23 18:37 UTC (permalink / raw)
  To: netdev; +Cc: nfont, jallen, linuxppc-dev, Thomas Falcon
In-Reply-To: <1527100682-23099-1-git-send-email-tlfalcon@linux.vnet.ibm.com>

Set adapter NAPI state as disabled if they are removed. This will allow
them to be enabled again if reallocated in case of a hard reset.

Signed-off-by: Thomas Falcon <tlfalcon@linux.vnet.ibm.com>
---
 drivers/net/ethernet/ibm/ibmvnic.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index 4bb4646..eabc1e4 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -789,6 +789,7 @@ static void release_napi(struct ibmvnic_adapter *adapter)
 	kfree(adapter->napi);
 	adapter->napi = NULL;
 	adapter->num_active_rx_napi = 0;
+	adapter->napi_enabled = false;
 }
 
 static int ibmvnic_login(struct net_device *netdev)
-- 
2.7.5

^ permalink raw reply related

* [PATCH net-next 0/8] ibmvnic: Failover hardening
From: Thomas Falcon @ 2018-05-23 18:37 UTC (permalink / raw)
  To: netdev; +Cc: nfont, jallen, linuxppc-dev, Thomas Falcon

Introduce additional transport event hardening to handle
events during device reset. In the driver's current state,
if a transport event is received during device reset, it can
cause the device to become unresponsive as invalid operations
are processed as the backing device context changes. After
a transport event, the device expects a request to begin the
initialization process. If the driver is still processing
a previously queued device reset in this state, it is likely
to fail as firmware will reject any commands other than the
one to initialize the client driver's Command-Response Queue.

Instead of failing and becoming dormant, the driver will make
one more attempt to recover and continue operation. This is
achieved by setting a state flag, which if true will direct
the driver to clean up all allocated resources and perform
a hard reset in an attempt to bring the driver back to an
operational state.

Thomas Falcon (8):
  ibmvnic: Mark NAPI flag as disabled when released
  ibmvnic: Introduce active CRQ state
  ibmvnic: Check CRQ command return codes
  ibmvnic: Return error code if init interrupted by transport event
  ibmvnic: Handle error case when setting link state
  ibmvnic: Create separate initialization routine for resets
  ibmvnic: Set resetting state at earliest possible point
  ibmvnic: Introduce hard reset recovery

 drivers/net/ethernet/ibm/ibmvnic.c | 223 +++++++++++++++++++++++++++++++++----
 drivers/net/ethernet/ibm/ibmvnic.h |   2 +
 2 files changed, 202 insertions(+), 23 deletions(-)

-- 
2.7.5

^ permalink raw reply

* Re: [PATCH v3] powerpc: Implement csum_ipv6_magic in assembly
From: Segher Boessenkool @ 2018-05-23 18:34 UTC (permalink / raw)
  To: Christophe Leroy
  Cc: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	linux-kernel, linuxppc-dev, netdev
In-Reply-To: <20180522065701.9DE696CCB4@po14934vm.idsi0.si.c-s.fr>

On Tue, May 22, 2018 at 08:57:01AM +0200, Christophe Leroy wrote:
> The generic csum_ipv6_magic() generates a pretty bad result

<snip>

Please try with a more recent compiler, what you used is pretty ancient.
It's not like recent compilers do great on this either, but it's not
*that* bad anymore ;-)

> --- a/arch/powerpc/lib/checksum_32.S
> +++ b/arch/powerpc/lib/checksum_32.S
> @@ -293,3 +293,36 @@ dst_error:
>  	EX_TABLE(51b, dst_error);
>  
>  EXPORT_SYMBOL(csum_partial_copy_generic)
> +
> +/*
> + * static inline __sum16 csum_ipv6_magic(const struct in6_addr *saddr,
> + *				      const struct in6_addr *daddr,
> + *				      __u32 len, __u8 proto, __wsum sum)
> + */
> +
> +_GLOBAL(csum_ipv6_magic)
> +	lwz	r8, 0(r3)
> +	lwz	r9, 4(r3)
> +	lwz	r10, 8(r3)
> +	lwz	r11, 12(r3)
> +	addc	r0, r5, r6
> +	adde	r0, r0, r7
> +	adde	r0, r0, r8
> +	adde	r0, r0, r9
> +	adde	r0, r0, r10
> +	adde	r0, r0, r11
> +	lwz	r8, 0(r4)
> +	lwz	r9, 4(r4)
> +	lwz	r10, 8(r4)
> +	lwz	r11, 12(r4)
> +	adde	r0, r0, r8
> +	adde	r0, r0, r9
> +	adde	r0, r0, r10
> +	adde	r0, r0, r11
> +	addze	r0, r0
> +	rotlwi	r3, r0, 16
> +	add	r3, r0, r3
> +	not	r3, r3
> +	rlwinm	r3, r3, 16, 16, 31
> +	blr
> +EXPORT_SYMBOL(csum_ipv6_magic)

Clustering the loads and carry insns together is pretty much the worst you
can do on most 32-bit CPUs.


Segher

^ permalink raw reply

* Re: [PATCH v2] powerpc/xmon: Also setup debugger hooks when single-stepping
From: Vaibhav Jain @ 2018-05-23 18:21 UTC (permalink / raw)
  To: Michal Suchanek, Benjamin Herrenschmidt, Paul Mackerras,
	Michael Ellerman, Balbir Singh, Nicholas Piggin, Breno Leitao,
	Guilherme G. Piccoli, Michal Suchanek, linuxppc-dev, linux-kernel
In-Reply-To: <20180523180054.9937-1-msuchanek@suse.de>

Michal Suchanek <msuchanek@suse.de> writes:

> When single-stepping kernel code from xmon without a debug hook enabled
> the kernel crashes. This can happen when kernel starts with xmon on
> crash disabled but xmon is entered using sysrq.
>
> Call force_enable_xmon when single-stepping in xmon to install the xmon
> debug hooks.
>
> Fixes: e1368d0c9edb ("powerpc/xmon: Setup debugger hooks when first
> break-point is set")
>
> Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> ---
LGTM,

Reviewed-by: Vaibhav Jain <vaibhav@linux.ibm.com>

-- 
Vaibhav Jain <vaibhav@linux.ibm.com>
Linux Technology Center, IBM India Pvt. Ltd.

^ permalink raw reply

* [PATCH v2] powerpc/xmon: Also setup debugger hooks when single-stepping
From: Michal Suchanek @ 2018-05-23 18:00 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	Balbir Singh, Nicholas Piggin, Breno Leitao, Vaibhav Jain,
	Guilherme G. Piccoli, Michal Suchanek, linuxppc-dev, linux-kernel

When single-stepping kernel code from xmon without a debug hook enabled
the kernel crashes. This can happen when kernel starts with xmon on
crash disabled but xmon is entered using sysrq.

Call force_enable_xmon when single-stepping in xmon to install the xmon
debug hooks.

Fixes: e1368d0c9edb ("powerpc/xmon: Setup debugger hooks when first
break-point is set")

Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
 arch/powerpc/xmon/xmon.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

v2: calling force_enable_xmon in do_step is sufficient

diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index a0842f1ff72c..252df9741d20 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -778,6 +778,16 @@ static int xmon_fault_handler(struct pt_regs *regs)
 	return 0;
 }
 
+/* Force enable xmon if not already enabled */
+static inline void force_enable_xmon(void)
+{
+	/* Enable xmon hooks if needed */
+	if (!xmon_on) {
+		printf("xmon: Enabling debugger hooks\n");
+		xmon_on = 1;
+	}
+}
+
 static struct bpt *at_breakpoint(unsigned long pc)
 {
 	int i;
@@ -1094,6 +1104,7 @@ static int do_step(struct pt_regs *regs)
 	unsigned int instr;
 	int stepped;
 
+	force_enable_xmon();
 	/* check we are in 64-bit kernel mode, translation enabled */
 	if ((regs->msr & (MSR_64BIT|MSR_PR|MSR_IR)) == (MSR_64BIT|MSR_IR)) {
 		if (mread(regs->nip, &instr, 4) == 4) {
@@ -1268,16 +1279,6 @@ static long check_bp_loc(unsigned long addr)
 	return 1;
 }
 
-/* Force enable xmon if not already enabled */
-static inline void force_enable_xmon(void)
-{
-	/* Enable xmon hooks if needed */
-	if (!xmon_on) {
-		printf("xmon: Enabling debugger hooks\n");
-		xmon_on = 1;
-	}
-}
-
 static char *breakpoint_help_string =
     "Breakpoint command usage:\n"
     "b                show breakpoints\n"
-- 
2.13.6

^ permalink raw reply related

* Re: [PATCH] powerpc/xmon: really enable xmon when a breakpoint is set
From: Michal Suchánek @ 2018-05-23 17:58 UTC (permalink / raw)
  To: Vaibhav Jain
  Cc: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	Balbir Singh, Nicholas Piggin, Breno Leitao, Naveen N. Rao,
	Guilherme G. Piccoli, linuxppc-dev, linux-kernel
In-Reply-To: <87bmd8xieu.fsf@vajain21.in.ibm.com>

On Tue, 22 May 2018 12:53:53 +0530
Vaibhav Jain <vaibhav@linux.vnet.ibm.com> wrote:

> Thanks for the patch Michal,
> 
> Michal Suchanek <msuchanek@suse.de> writes:
> 
> > When single-stepping kernel code from xmon without a debug hook
> > enabled the kernel crashes. This can happen when kernel starts with
> > xmon on crash disabled but xmon is entered using sysrq.
> >
> > Commit e1368d0c9edb ("powerpc/xmon: Setup debugger hooks when first
> > break-point is set") adds force_enable_xmon function that prints
> > "xmon: Enabling debugger hooks" but does not enable them.  
> Debugger hooks are enabled just befores debugger() is entered from
> sysrq_handle_xmon(). Thats why force_enable_xmon() simply sets sets
> 'xmon_on=1' and exits.
> 
> The problem you are seeing is probably due to sysrq_handle_xmon()
> clearing the debugger hooks on return from debugger() as xmon_on was
> never set for the 's' xmon command.

Indeed, setting xmon_on is sufficient. Will resend the patch.

Thanks

Michal

> 
> > Add the call to xmon_init to install the debugger hooks in
> > force_enable_xmon and also call force_enable_xmon when
> > single-stepping in xmon.  
> Only calling force_enable_xmon() from do_step() should be suffice as
> on exit from the debugger() the value of xmon_on is checked and if
> required the debugger hooks are kept instead of getting cleared.
> 
> 
> >  arch/powerpc/xmon/xmon.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
> > index a0842f1ff72c..504bd1c3d8b0 100644
> > --- a/arch/powerpc/xmon/xmon.c
> > +++ b/arch/powerpc/xmon/xmon.c  
> 
> 
> > @@ -1275,6 +1279,7 @@ static inline void force_enable_xmon(void)
> >  	if (!xmon_on) {
> >  		printf("xmon: Enabling debugger hooks\n");
> >  		xmon_on = 1;
> > +		xmon_init(1);
> >  	}
> >  }  
> 
> As mentioned above call to force_enable_xmon() is usually done in
> context of sysrq_handle_xmon() which sets the debugger hooks as soon
> as its entered. So I think that this hunk is not really needed.
> 
> 

^ permalink raw reply

* Patch "powerpc/pseries: Use the security flags in pseries_setup_rfi_flush()" has been added to the 4.16-stable tree
From: gregkh @ 2018-05-23 17:23 UTC (permalink / raw)
  To: greg, gregkh, linuxppc-dev, mpe, tglx; +Cc: stable-commits
In-Reply-To: <20180522144125.10345-10-mpe@ellerman.id.au>


This is a note to let you know that I've just added the patch titled

    powerpc/pseries: Use the security flags in pseries_setup_rfi_flush()

to the 4.16-stable tree which can be found at:
    http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
     powerpc-pseries-use-the-security-flags-in-pseries_setup_rfi_flush.patch
and it can be found in the queue-4.16 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@vger.kernel.org> know about it.


>From foo@baz Wed May 23 19:18:22 CEST 2018
From: Michael Ellerman <mpe@ellerman.id.au>
Date: Wed, 23 May 2018 00:41:20 +1000
Subject: powerpc/pseries: Use the security flags in pseries_setup_rfi_flush()
To: greg@kroah.com
Cc: stable@vger.kernel.org, tglx@linutronix.de, linuxppc-dev@ozlabs.org
Message-ID: <20180522144125.10345-10-mpe@ellerman.id.au>

From: Michael Ellerman <mpe@ellerman.id.au>

commit 2e4a16161fcd324b1f9bf6cb6856529f7eaf0689 upstream.

Now that we have the security flags we can simplify the code in
pseries_setup_rfi_flush() because the security flags have pessimistic
defaults.

Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 arch/powerpc/platforms/pseries/setup.c |   27 ++++++++++++---------------
 1 file changed, 12 insertions(+), 15 deletions(-)

--- a/arch/powerpc/platforms/pseries/setup.c
+++ b/arch/powerpc/platforms/pseries/setup.c
@@ -501,30 +501,27 @@ static void pseries_setup_rfi_flush(void
 	bool enable;
 	long rc;
 
-	/* Enable by default */
-	enable = true;
-	types = L1D_FLUSH_FALLBACK;
-
 	rc = plpar_get_cpu_characteristics(&result);
-	if (rc == H_SUCCESS) {
+	if (rc == H_SUCCESS)
 		init_cpu_char_feature_flags(&result);
 
-		if (result.character & H_CPU_CHAR_L1D_FLUSH_TRIG2)
-			types |= L1D_FLUSH_MTTRIG;
-		if (result.character & H_CPU_CHAR_L1D_FLUSH_ORI30)
-			types |= L1D_FLUSH_ORI;
-
-		if ((!(result.behaviour & H_CPU_BEHAV_L1D_FLUSH_PR)) ||
-		    (!(result.behaviour & H_CPU_BEHAV_FAVOUR_SECURITY)))
-			enable = false;
-	}
-
 	/*
 	 * We're the guest so this doesn't apply to us, clear it to simplify
 	 * handling of it elsewhere.
 	 */
 	security_ftr_clear(SEC_FTR_L1D_FLUSH_HV);
 
+	types = L1D_FLUSH_FALLBACK;
+
+	if (security_ftr_enabled(SEC_FTR_L1D_FLUSH_TRIG2))
+		types |= L1D_FLUSH_MTTRIG;
+
+	if (security_ftr_enabled(SEC_FTR_L1D_FLUSH_ORI30))
+		types |= L1D_FLUSH_ORI;
+
+	enable = security_ftr_enabled(SEC_FTR_FAVOUR_SECURITY) && \
+		 security_ftr_enabled(SEC_FTR_L1D_FLUSH_PR);
+
 	setup_rfi_flush(types, enable);
 }
 


Patches currently in stable-queue which might be from mpe@ellerman.id.au are

queue-4.16/powerpc-pseries-fix-clearing-of-security-feature-flags.patch
queue-4.16/powerpc-powernv-set-or-clear-security-feature-flags.patch
queue-4.16/powerpc-64s-move-cpu_show_meltdown.patch
queue-4.16/powerpc-pseries-set-or-clear-security-feature-flags.patch
queue-4.16/powerpc-move-default-security-feature-flags.patch
queue-4.16/powerpc-powernv-use-the-security-flags-in-pnv_setup_rfi_flush.patch
queue-4.16/powerpc-add-security-feature-flags-for-spectre-meltdown.patch
queue-4.16/powerpc-pseries-use-the-security-flags-in-pseries_setup_rfi_flush.patch
queue-4.16/powerpc-64s-enhance-the-information-in-cpu_show_meltdown.patch
queue-4.16/powerpc-rfi-flush-always-enable-fallback-flush-on-pseries.patch
queue-4.16/powerpc-pseries-add-new-h_get_cpu_characteristics-flags.patch
queue-4.16/powerpc-64s-add-support-for-a-store-forwarding-barrier-at-kernel-entry-exit.patch
queue-4.16/powerpc-64s-wire-up-cpu_show_spectre_v1.patch
queue-4.16/powerpc-64s-wire-up-cpu_show_spectre_v2.patch

^ 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