LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: POWER9 crash due to STRICT_KERNEL_RWX (WAS: Re: Linux-next POWER9 NULL pointer NIP...)
From: Russell Currey @ 2020-04-17  2:27 UTC (permalink / raw)
  To: Steven Rostedt, Qian Cai; +Cc: linuxppc-dev, LKML, Nicholas Piggin
In-Reply-To: <20200416221753.4e47080a@oasis.local.home>

On Thu, 2020-04-16 at 22:17 -0400, Steven Rostedt wrote:
> On Thu, 16 Apr 2020 21:19:10 -0400
> Qian Cai <cai@lca.pw> wrote:
> 
> > OK, reverted the commit,
> > 
> > c55d7b5e6426 (“powerpc: Remove STRICT_KERNEL_RWX incompatibility
> > with RELOCATABLE”)
> > 
> > or set STRICT_KERNEL_RWX=n fixed the crash below and also mentioned
> > in this thread,
> 
> This may be a symptom and not a cure.

Reverting the patch with the given config will have the same effect as
STRICT_KERNEL_RWX=n.  Not discounting that it could be a bug on the
powerpc side (i.e. relocatable kernels with strict RWX on haven't been
exhaustively tested yet), but we should definitely figure out what's
going on with this bad access first.

> 
> > https://lore.kernel.org/lkml/15AC5B0E-A221-4B8C-9039-FA96B8EF7C88@lca.pw/
> > 
> > [  148.110969][T13115] LTP: starting chown04_16
> > [  148.255048][T13380] kernel tried to execute exec-protected page
> > (c0000000016804ac) - exploit attempt? (uid: 0)
> > [  148.255099][T13380] BUG: Unable to handle kernel instruction
> > fetch
> > [  148.255122][T13380] Faulting instruction address:
> > 0xc0000000016804ac
> > [  148.255136][T13380] Oops: Kernel access of bad area, sig: 11
> > [#1]
> > [  148.255157][T13380] LE PAGE_SIZE=64K MMU=Radix SMP NR_CPUS=256
> > DEBUG_PAGEALLOC NUMA PowerNV
> > [  148.255171][T13380] Modules linked in: loop kvm_hv kvm xfs
> > sd_mod bnx2x mdio ahci tg3 libahci libphy libata firmware_class
> > dm_mirror dm_region_hash dm_log dm_mod
> > [  148.255213][T13380] CPU: 45 PID: 13380 Comm: chown04_16 Tainted:
> > G        W         5.6.0+ #7
> > [  148.255236][T13380] NIP:  c0000000016804ac LR: c00800000fa60408
> > CTR: c0000000016804ac
> > [  148.255250][T13380] REGS: c0000010a6fafa00 TRAP: 0400   Tainted:
> > G        W          (5.6.0+)
> > [  148.255281][T13380] MSR:  9000000010009033
> > <SF,HV,EE,ME,IR,DR,RI,LE>  CR: 84000248  XER: 20040000
> > [  148.255310][T13380] CFAR: c00800000fa66534 IRQMASK: 0 
> > [  148.255310][T13380] GPR00: c000000000973268 c0000010a6fafc90
> > c000000001648200 0000000000000000 
> > [  148.255310][T13380] GPR04: c000000d8a22dc00 c0000010a6fafd30
> > 00000000b5e98331 ffffffff00012c9f 
> > [  148.255310][T13380] GPR08: c000000d8a22dc00 0000000000000000
> > 0000000000000000 c00000000163c520 
> > [  148.255310][T13380] GPR12: c0000000016804ac c000001ffffdad80
> > 0000000000000000 0000000000000000 
> > [  148.255310][T13380] GPR16: 0000000000000000 0000000000000000
> > 0000000000000000 0000000000000000 
> > [  148.255310][T13380] GPR20: 0000000000000000 0000000000000000
> > 0000000000000000 0000000000000000 
> > [  148.255310][T13380] GPR24: 00007fff8f5e2e48 0000000000000000
> > c00800000fa6a488 c0000010a6fafd30 
> > [  148.255310][T13380] GPR28: 0000000000000000 000000007fffffff
> > c00800000fa60400 c000000efd0c6780 
> > [  148.255494][T13380] NIP [c0000000016804ac]
> > sysctl_net_busy_read+0x0/0x4
> 
> The instruction pointer is on sysctl_net_busy_read? Isn't that data
> and
> not code?
> 
> In net/socket.c:
> 
>   #ifdef CONFIG_NET_RX_BUSY_POLL
>   unsigned int sysctl_net_busy_read __read_mostly;
>   unsigned int sysctl_net_busy_poll __read_mostly;
>   #endif
> 
> -- Steve
> 
> 
> > [  148.255516][T13380] LR [c00800000fa60408] find_free_cb+0x8/0x30
> > [loop]
> > [  148.255528][T13380] Call Trace:
> > [  148.255538][T13380] [c0000010a6fafc90] [c0000000009732c0]
> > idr_for_each+0xf0/0x170 (unreliable)
> > [  148.255572][T13380] [c0000010a6fafd10] [c00800000fa626c4]
> > loop_lookup.part.1+0x4c/0xb0 [loop]
> > [  148.255597][T13380] [c0000010a6fafd50] [c00800000fa634d8]
> > loop_control_ioctl+0x120/0x1d0 [loop]
> > [  148.255623][T13380] [c0000010a6fafdb0] [c0000000004ddc08]
> > ksys_ioctl+0xd8/0x130
> > [  148.255636][T13380] [c0000010a6fafe00] [c0000000004ddc88]
> > sys_ioctl+0x28/0x40
> > [  148.255669][T13380] [c0000010a6fafe20] [c00000000000b378]
> > system_call+0x5c/0x68
> > [  148.255699][T13380] Instruction dump:
> > [  148.255718][T13380] XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX
> > XXXXXXXX XXXXXXXX XXXXXXXX 
> > [  148.255744][T13380] XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX
> > XXXXXXXX XXXXXXXX XXXXXXXX 
> > [  148.255772][T13380] ---[ end trace a5894a74208c22ec ]---
> > [  148.576663][T13380] 
> > [  149.576765][T13380] Kernel panic - not syncing: Fatal exception
> > 


^ permalink raw reply

* Re: POWER9 crash due to STRICT_KERNEL_RWX (WAS: Re: Linux-next POWER9 NULL pointer NIP...)
From: Steven Rostedt @ 2020-04-17  2:17 UTC (permalink / raw)
  To: Qian Cai; +Cc: linuxppc-dev, LKML, Nicholas Piggin
In-Reply-To: <69F0448F-CA5B-497D-B8AF-2848175B9477@lca.pw>

On Thu, 16 Apr 2020 21:19:10 -0400
Qian Cai <cai@lca.pw> wrote:

> OK, reverted the commit,
> 
> c55d7b5e6426 (“powerpc: Remove STRICT_KERNEL_RWX incompatibility with RELOCATABLE”)
> 
> or set STRICT_KERNEL_RWX=n fixed the crash below and also mentioned in this thread,

This may be a symptom and not a cure.

> 
> https://lore.kernel.org/lkml/15AC5B0E-A221-4B8C-9039-FA96B8EF7C88@lca.pw/
> 
> [  148.110969][T13115] LTP: starting chown04_16
> [  148.255048][T13380] kernel tried to execute exec-protected page (c0000000016804ac) - exploit attempt? (uid: 0)
> [  148.255099][T13380] BUG: Unable to handle kernel instruction fetch
> [  148.255122][T13380] Faulting instruction address: 0xc0000000016804ac
> [  148.255136][T13380] Oops: Kernel access of bad area, sig: 11 [#1]
> [  148.255157][T13380] LE PAGE_SIZE=64K MMU=Radix SMP NR_CPUS=256 DEBUG_PAGEALLOC NUMA PowerNV
> [  148.255171][T13380] Modules linked in: loop kvm_hv kvm xfs sd_mod bnx2x mdio ahci tg3 libahci libphy libata firmware_class dm_mirror dm_region_hash dm_log dm_mod
> [  148.255213][T13380] CPU: 45 PID: 13380 Comm: chown04_16 Tainted: G        W         5.6.0+ #7
> [  148.255236][T13380] NIP:  c0000000016804ac LR: c00800000fa60408 CTR: c0000000016804ac
> [  148.255250][T13380] REGS: c0000010a6fafa00 TRAP: 0400   Tainted: G        W          (5.6.0+)
> [  148.255281][T13380] MSR:  9000000010009033 <SF,HV,EE,ME,IR,DR,RI,LE>  CR: 84000248  XER: 20040000
> [  148.255310][T13380] CFAR: c00800000fa66534 IRQMASK: 0 
> [  148.255310][T13380] GPR00: c000000000973268 c0000010a6fafc90 c000000001648200 0000000000000000 
> [  148.255310][T13380] GPR04: c000000d8a22dc00 c0000010a6fafd30 00000000b5e98331 ffffffff00012c9f 
> [  148.255310][T13380] GPR08: c000000d8a22dc00 0000000000000000 0000000000000000 c00000000163c520 
> [  148.255310][T13380] GPR12: c0000000016804ac c000001ffffdad80 0000000000000000 0000000000000000 
> [  148.255310][T13380] GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000 
> [  148.255310][T13380] GPR20: 0000000000000000 0000000000000000 0000000000000000 0000000000000000 
> [  148.255310][T13380] GPR24: 00007fff8f5e2e48 0000000000000000 c00800000fa6a488 c0000010a6fafd30 
> [  148.255310][T13380] GPR28: 0000000000000000 000000007fffffff c00800000fa60400 c000000efd0c6780 
> [  148.255494][T13380] NIP [c0000000016804ac] sysctl_net_busy_read+0x0/0x4

The instruction pointer is on sysctl_net_busy_read? Isn't that data and
not code?

In net/socket.c:

  #ifdef CONFIG_NET_RX_BUSY_POLL
  unsigned int sysctl_net_busy_read __read_mostly;
  unsigned int sysctl_net_busy_poll __read_mostly;
  #endif

-- Steve


> [  148.255516][T13380] LR [c00800000fa60408] find_free_cb+0x8/0x30 [loop]
> [  148.255528][T13380] Call Trace:
> [  148.255538][T13380] [c0000010a6fafc90] [c0000000009732c0] idr_for_each+0xf0/0x170 (unreliable)
> [  148.255572][T13380] [c0000010a6fafd10] [c00800000fa626c4] loop_lookup.part.1+0x4c/0xb0 [loop]
> [  148.255597][T13380] [c0000010a6fafd50] [c00800000fa634d8] loop_control_ioctl+0x120/0x1d0 [loop]
> [  148.255623][T13380] [c0000010a6fafdb0] [c0000000004ddc08] ksys_ioctl+0xd8/0x130
> [  148.255636][T13380] [c0000010a6fafe00] [c0000000004ddc88] sys_ioctl+0x28/0x40
> [  148.255669][T13380] [c0000010a6fafe20] [c00000000000b378] system_call+0x5c/0x68
> [  148.255699][T13380] Instruction dump:
> [  148.255718][T13380] XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX 
> [  148.255744][T13380] XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX 
> [  148.255772][T13380] ---[ end trace a5894a74208c22ec ]---
> [  148.576663][T13380] 
> [  149.576765][T13380] Kernel panic - not syncing: Fatal exception
> 


^ permalink raw reply

* Re: [musl] Powerpc Linux 'scv' system call ABI proposal take 2
From: Segher Boessenkool @ 2020-04-17  1:48 UTC (permalink / raw)
  To: Rich Felker
  Cc: libc-alpha, musl, Nicholas Piggin, Florian Weimer, libc-dev,
	linuxppc-dev
In-Reply-To: <20200417003442.GD11469@brightrain.aerifal.cx>

On Thu, Apr 16, 2020 at 08:34:42PM -0400, Rich Felker wrote:
> On Thu, Apr 16, 2020 at 06:02:35PM -0500, Segher Boessenkool wrote:
> > On Thu, Apr 16, 2020 at 08:12:19PM +0200, Florian Weimer wrote:
> > > > I think my choice would be just making the inline syscall be a single
> > > > call insn to an asm source file that out-of-lines the loading of TOC
> > > > pointer and call through it or branch based on hwcap so that it's not
> > > > repeated all over the place.
> > > 
> > > I don't know how problematic control flow out of an inline asm is on
> > > POWER.  But this is basically the -moutline-atomics approach.
> > 
> > Control flow out of inline asm (other than with "asm goto") is not
> > allowed at all, just like on any other target (and will not work in
> > practice, either -- just like on any other target).  But the suggestion
> > was to use actual assembler code, not inline asm?
> 
> Calling it control flow out of inline asm is something of a misnomer.
> The enclosing state is not discarded or altered; the asm statement
> exits normally, reaching the next instruction in the enclosing
> block/function as soon as the call from the asm statement returns,
> with all register/clobber constraints satisfied.

Ah.  That should always Just Work, then -- our ABIs guarantee you can.

> Control flow out of inline asm would be more like longjmp, and it can
> be valid -- for instance, you can implement coroutines this way
> (assuming you switch stack correctly) or do longjmp this way (jumping
> to the location saved by setjmp). But it's not what'd be happening
> here.

Yeah, you cannot do that in C, not without making assumptions about what
machine code the compiler generates.  GCC explicitly disallows it, too:

     'asm' statements may not perform jumps into other 'asm' statements,
     only to the listed GOTOLABELS.  GCC's optimizers do not know about
     other jumps; therefore they cannot take account of them when
     deciding how to optimize.


Segher

^ permalink raw reply

* Re: [PATCH] powerpc/uaccess: Use flexible addressing with __put_user()/__get_user()
From: Segher Boessenkool @ 2020-04-17  1:39 UTC (permalink / raw)
  To: Christophe Leroy; +Cc: linux-kernel, npiggin, Paul Mackerras, linuxppc-dev
In-Reply-To: <1f5a7975-3b32-3a14-e03e-7c875df57aa3@c-s.fr>

Hi!

On Thu, Apr 16, 2020 at 07:50:00AM +0200, Christophe Leroy wrote:
> Le 16/04/2020 à 00:06, Segher Boessenkool a écrit :
> >On Wed, Apr 15, 2020 at 09:20:26AM +0000, Christophe Leroy wrote:
> >>At the time being, __put_user()/__get_user() and friends only use
> >>register indirect with immediate index addressing, with the index
> >>set to 0. Ex:
> >>
> >>	lwz	reg1, 0(reg2)
> >
> >This is called a "D-form" instruction, or sometimes "offset addressing".
> >Don't talk about an "index", it confuses things, because the *other*
> >kind is called "indexed" already, also in the ISA docs!  (X-form, aka
> >indexed addressing, [reg+reg], where D-form does [reg+imm], and both
> >forms can do [reg]).
> 
> In the "Programming Environments Manual for 32-Bit Implementations of 
> the PowerPC™ Architecture", they list the following addressing modes:
> 
> Load and store operations have three categories of effective address 
> generation that depend on the
> operands specified:
> • Register indirect with immediate index mode
> • Register indirect with index mode
> • Register indirect mode

Huh.  I must have pushed all that confusing terminology to the back of
my head :-)

> >%Un on an "m" operand doesn't do much: you need to make it "m<>" if you
> >want pre-modify ("update") insns to be generated.  (You then will want
> >to make sure that operand is used in a way GCC can understand; since it
> >is used only once here, that works fine).
> 
> Ah ? Indeed I got the idea from include/asm/io.h where there is:
> 
> #define DEF_MMIO_IN_D(name, size, insn)				\
> static inline u##size name(const volatile u##size __iomem *addr)	\
> {									\
> 	u##size ret;							\
> 	__asm__ __volatile__("sync;"#insn"%U1%X1 %0,%1;twi 0,%0,0;isync"\
> 		: "=r" (ret) : "m" (*addr) : "memory");			\
> 	return ret;							\
> }
> 
> It should be "m<>" there as well ?

Yes, that will work here.

Long ago, "m" in inline assembler code did the same as "m<>" now does
(and "m" internal in GCC still does).  To get a memory without pre-modify
addressing, you used "es".

Since people kept getting that wrong (it *is* surprising), it was changed
to the current scheme.  But the kernel uses weren't updated (and no one
seems to have missed it).

> >>  #else /* __powerpc64__ */
> >>  #define __put_user_asm2(x, addr, err)				\
> >>  	__asm__ __volatile__(					\
> >>-		"1:	stw %1,0(%2)\n"				\
> >>-		"2:	stw %1+1,4(%2)\n"			\
> >>+		"1:	stw%U2%X2 %1,%2\n"			\
> >>+		"2:	stw%U2%X2 %L1,%L2\n"			\
> >>  		"3:\n"						\
> >>  		".section .fixup,\"ax\"\n"			\
> >>  		"4:	li %0,%3\n"				\
> >>@@ -140,7 +140,7 @@ extern long __put_user_bad(void);
> >>  		EX_TABLE(1b, 4b)				\
> >>  		EX_TABLE(2b, 4b)				\
> >>  		: "=r" (err)					\
> >>-		: "r" (x), "b" (addr), "i" (-EFAULT), "0" (err))
> >>+		: "r" (x), "m" (*addr), "i" (-EFAULT), "0" (err))
> >
> >Here, it doesn't work.  You don't want two consecutive update insns in
> >any case.  Easiest is to just not use "m<>", and then, don't use %Un
> >(which won't do anything, but it is confusing).
> 
> Can't we leave the Un on the second stw ?

That cannot work.  You can use it on only the *first* though :-)

And this doesn't work on LE I think?  (But the asm doesn't anyway?)

Or, you can decide this is all way too tricky, and not worth it.


Segher

^ permalink raw reply

* Re: [PATCH kernel v2 0/7] powerpc/powenv/ioda: Allow huge DMA window at 4GB
From: Russell Currey @ 2020-04-17  1:26 UTC (permalink / raw)
  To: Oliver O'Halloran, Alexey Kardashevskiy
  Cc: KVM list, Fabiano Rosas, Alistair Popple, kvm-ppc, linuxppc-dev,
	David Gibson
In-Reply-To: <CAOSf1CHgUsJ7jGokg6QD6cEDr4-o5hnyyyjRZ=YijsRY3T1sYA@mail.gmail.com>

On Thu, 2020-04-16 at 12:53 +1000, Oliver O'Halloran wrote:
> On Thu, Apr 16, 2020 at 12:34 PM Oliver O'Halloran <oohall@gmail.com>
> wrote:
> > On Thu, Apr 16, 2020 at 11:27 AM Alexey Kardashevskiy <
> > aik@ozlabs.ru> wrote:
> > > Anyone? Is it totally useless or wrong approach? Thanks,
> > 
> > I wouldn't say it's either, but I still hate it.
> > 
> > The 4GB mode being per-PHB makes it difficult to use unless we
> > force
> > that mode on 100% of the time which I'd prefer not to do. Ideally
> > devices that actually support 64bit addressing (which is most of
> > them)
> > should be able to use no-translate mode when possible since a) It's
> > faster, and b) It frees up room in the TCE cache devices that
> > actually
> > need them. I know you've done some testing with 100G NICs and found
> > the overhead was fine, but IMO that's a bad test since it's pretty
> > much the best-case scenario since all the devices on the PHB are in
> > the same PE. The PHB's TCE cache only hits when the TCE matches the
> > DMA bus address and the PE number for the device so in a multi-PE
> > environment there's a lot of potential for TCE cache trashing. If
> > there was one or two PEs under that PHB it's probably not going to
> > matter, but if you have an NVMe rack with 20 drives it starts to
> > look
> > a bit ugly.
> > 
> > That all said, it might be worth doing this anyway since we
> > probably
> > want the software infrastructure in place to take advantage of it.
> > Maybe expand the command line parameters to allow it to be enabled
> > on
> > a per-PHB basis rather than globally.
> 
> Since we're on the topic
> 
> I've been thinking the real issue we have is that we're trying to
> pick
> an "optimal" IOMMU config at a point where we don't have enough
> information to work out what's actually optimal. The IOMMU config is
> done on a per-PE basis, but since PEs may contain devices with
> different DMA masks (looking at you wierd AMD audio function) we're
> always going to have to pick something conservative as the default
> config for TVE#0 (64k, no bypass mapping) since the driver will tell
> us what the device actually supports long after the IOMMU
> configuation
> is done. What we really want is to be able to have separate IOMMU
> contexts for each device, or at the very least a separate context for
> the crippled devices.
> 
> We could allow a per-device IOMMU context by extending the Master /
> Slave PE thing to cover DMA in addition to MMIO. Right now we only
> use
> slave PEs when a device's MMIO BARs extend over multiple m64
> segments.
> When that happens an MMIO error causes the PHB to freezes the PE
> corresponding to one of those segments, but not any of the others. To
> present a single "PE" to the EEH core we check the freeze status of
> each of the slave PEs when the EEH core does a PE status check and if
> any of them are frozen, we freeze the rest of them too. When a driver
> sets a limited DMA mask we could move that device to a seperate slave
> PE so that it has it's own IOMMU context taylored to its DMA
> addressing limits.
> 
> Thoughts?

For what it's worth this sounds like a good idea to me, it just sounds
tricky to implement.  You're adding another layer of complexity on top
of EEH (well, making things look simple to the EEH core and doing your
own freezing on top of it) in addition to the DMA handling.

If it works then great, just has a high potential to become a new bug
haven.

> 
> Oliver


^ permalink raw reply

* POWER9 crash due to STRICT_KERNEL_RWX (WAS: Re: Linux-next POWER9 NULL pointer NIP...)
From: Qian Cai @ 2020-04-17  1:19 UTC (permalink / raw)
  To: Michael Ellerman, Russell Currey
  Cc: Steven Rostedt, linuxppc-dev, LKML, Nicholas Piggin
In-Reply-To: <161662E3-5D9C-4C15-919C-CFEFE4CC35CB@lca.pw>

OK, reverted the commit,

c55d7b5e6426 (“powerpc: Remove STRICT_KERNEL_RWX incompatibility with RELOCATABLE”)

or set STRICT_KERNEL_RWX=n fixed the crash below and also mentioned in this thread,

https://lore.kernel.org/lkml/15AC5B0E-A221-4B8C-9039-FA96B8EF7C88@lca.pw/

[  148.110969][T13115] LTP: starting chown04_16
[  148.255048][T13380] kernel tried to execute exec-protected page (c0000000016804ac) - exploit attempt? (uid: 0)
[  148.255099][T13380] BUG: Unable to handle kernel instruction fetch
[  148.255122][T13380] Faulting instruction address: 0xc0000000016804ac
[  148.255136][T13380] Oops: Kernel access of bad area, sig: 11 [#1]
[  148.255157][T13380] LE PAGE_SIZE=64K MMU=Radix SMP NR_CPUS=256 DEBUG_PAGEALLOC NUMA PowerNV
[  148.255171][T13380] Modules linked in: loop kvm_hv kvm xfs sd_mod bnx2x mdio ahci tg3 libahci libphy libata firmware_class dm_mirror dm_region_hash dm_log dm_mod
[  148.255213][T13380] CPU: 45 PID: 13380 Comm: chown04_16 Tainted: G        W         5.6.0+ #7
[  148.255236][T13380] NIP:  c0000000016804ac LR: c00800000fa60408 CTR: c0000000016804ac
[  148.255250][T13380] REGS: c0000010a6fafa00 TRAP: 0400   Tainted: G        W          (5.6.0+)
[  148.255281][T13380] MSR:  9000000010009033 <SF,HV,EE,ME,IR,DR,RI,LE>  CR: 84000248  XER: 20040000
[  148.255310][T13380] CFAR: c00800000fa66534 IRQMASK: 0 
[  148.255310][T13380] GPR00: c000000000973268 c0000010a6fafc90 c000000001648200 0000000000000000 
[  148.255310][T13380] GPR04: c000000d8a22dc00 c0000010a6fafd30 00000000b5e98331 ffffffff00012c9f 
[  148.255310][T13380] GPR08: c000000d8a22dc00 0000000000000000 0000000000000000 c00000000163c520 
[  148.255310][T13380] GPR12: c0000000016804ac c000001ffffdad80 0000000000000000 0000000000000000 
[  148.255310][T13380] GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000 
[  148.255310][T13380] GPR20: 0000000000000000 0000000000000000 0000000000000000 0000000000000000 
[  148.255310][T13380] GPR24: 00007fff8f5e2e48 0000000000000000 c00800000fa6a488 c0000010a6fafd30 
[  148.255310][T13380] GPR28: 0000000000000000 000000007fffffff c00800000fa60400 c000000efd0c6780 
[  148.255494][T13380] NIP [c0000000016804ac] sysctl_net_busy_read+0x0/0x4
[  148.255516][T13380] LR [c00800000fa60408] find_free_cb+0x8/0x30 [loop]
[  148.255528][T13380] Call Trace:
[  148.255538][T13380] [c0000010a6fafc90] [c0000000009732c0] idr_for_each+0xf0/0x170 (unreliable)
[  148.255572][T13380] [c0000010a6fafd10] [c00800000fa626c4] loop_lookup.part.1+0x4c/0xb0 [loop]
[  148.255597][T13380] [c0000010a6fafd50] [c00800000fa634d8] loop_control_ioctl+0x120/0x1d0 [loop]
[  148.255623][T13380] [c0000010a6fafdb0] [c0000000004ddc08] ksys_ioctl+0xd8/0x130
[  148.255636][T13380] [c0000010a6fafe00] [c0000000004ddc88] sys_ioctl+0x28/0x40
[  148.255669][T13380] [c0000010a6fafe20] [c00000000000b378] system_call+0x5c/0x68
[  148.255699][T13380] Instruction dump:
[  148.255718][T13380] XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX 
[  148.255744][T13380] XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX 
[  148.255772][T13380] ---[ end trace a5894a74208c22ec ]---
[  148.576663][T13380] 
[  149.576765][T13380] Kernel panic - not syncing: Fatal exception



^ permalink raw reply

* Re: [PATCH] target/ppc: Fix mtmsr(d) L=1 variant that loses interrupts
From: David Gibson @ 2020-04-17  0:40 UTC (permalink / raw)
  To: Nicholas Piggin
  Cc: qemu-devel, qemu-stable, Cédric Le Goater, qemu-ppc,
	Nathan Chancellor, linuxppc-dev
In-Reply-To: <20200414111131.465560-1-npiggin@gmail.com>

[-- Attachment #1: Type: text/plain, Size: 6769 bytes --]

On Tue, Apr 14, 2020 at 09:11:31PM +1000, Nicholas Piggin wrote:
65;5803;1c> If mtmsr L=1 sets MSR[EE] while there is a maskable exception pending,
> it does not cause an interrupt. This causes the test case to hang:
> 
> https://lists.gnu.org/archive/html/qemu-ppc/2019-10/msg00826.html
> 
> More recently, Linux reduced the occurance of operations (e.g., rfi)
> which stop translation and allow pending interrupts to be processed.
> This started causing hangs in Linux boot in long-running kernel tests,
> running with '-d int' shows the decrementer stops firing despite DEC
> wrapping and MSR[EE]=1.
> 
> https://lists.ozlabs.org/pipermail/linuxppc-dev/2020-April/208301.html
> 
> The cause is the broken mtmsr L=1 behaviour, which is contrary to the
> architecture. From Power ISA v3.0B, p.977, Move To Machine State Register,
> Programming Note states:
> 
>     If MSR[EE]=0 and an External, Decrementer, or Performance Monitor
>     exception is pending, executing an mtmsrd instruction that sets
>     MSR[EE] to 1 will cause the interrupt to occur before the next
>     instruction is executed, if no higher priority exception exists
> 
> Fix this by handling L=1 exactly the same way as L=0, modulo the MSR
> bits altered.
> 
> The confusion arises from L=0 being "context synchronizing" whereas L=1
> is "execution synchronizing", which is a weaker semantic. However this
> is not a relaxation of the requirement that these exceptions cause
> interrupts when MSR[EE]=1 (e.g., when mtmsr executes to completion as
> TCG is doing here), rather it specifies how a pipelined processor can
> have multiple instructions in flight where one may influence how another
> behaves.
> 
> Cc: qemu-stable@nongnu.org
> Reported-by: Anton Blanchard <anton@ozlabs.org>
> Reported-by: Nathan Chancellor <natechancellor@gmail.com>
> Tested-by: Nathan Chancellor <natechancellor@gmail.com>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> Thanks very much to Nathan for reporting and testing it, I added his
> Tested-by tag despite a more polished patch, as the the basics are 
> still the same (and still fixes his test case here).
> 
> This bug possibly goes back to early v2.04 / mtmsrd L=1 support around
> 2007, and the code has been changed several times since then so may
> require some backporting.
> 
> 32-bit / mtmsr untested at the moment, I don't have an environment
> handy.
> 
>  target/ppc/translate.c | 46 +++++++++++++++++++++++++-----------------
>  1 file changed, 27 insertions(+), 19 deletions(-)

Applied to ppc-for-5.0.

> 
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index b207fb5386..9959259dba 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -4361,30 +4361,34 @@ static void gen_mtmsrd(DisasContext *ctx)
>      CHK_SV;
>  
>  #if !defined(CONFIG_USER_ONLY)
> +    if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
> +        gen_io_start();
> +    }
>      if (ctx->opcode & 0x00010000) {
> -        /* Special form that does not need any synchronisation */
> +        /* L=1 form only updates EE and RI */
>          TCGv t0 = tcg_temp_new();
> +        TCGv t1 = tcg_temp_new();
>          tcg_gen_andi_tl(t0, cpu_gpr[rS(ctx->opcode)],
>                          (1 << MSR_RI) | (1 << MSR_EE));
> -        tcg_gen_andi_tl(cpu_msr, cpu_msr,
> +        tcg_gen_andi_tl(t1, cpu_msr,
>                          ~(target_ulong)((1 << MSR_RI) | (1 << MSR_EE)));
> -        tcg_gen_or_tl(cpu_msr, cpu_msr, t0);
> +        tcg_gen_or_tl(t1, t1, t0);
> +
> +        gen_helper_store_msr(cpu_env, t1);
>          tcg_temp_free(t0);
> +        tcg_temp_free(t1);
> +
>      } else {
>          /*
>           * XXX: we need to update nip before the store if we enter
>           *      power saving mode, we will exit the loop directly from
>           *      ppc_store_msr
>           */
> -        if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
> -            gen_io_start();
> -        }
>          gen_update_nip(ctx, ctx->base.pc_next);
>          gen_helper_store_msr(cpu_env, cpu_gpr[rS(ctx->opcode)]);
> -        /* Must stop the translation as machine state (may have) changed */
> -        /* Note that mtmsr is not always defined as context-synchronizing */
> -        gen_stop_exception(ctx);
>      }
> +    /* Must stop the translation as machine state (may have) changed */
> +    gen_stop_exception(ctx);
>  #endif /* !defined(CONFIG_USER_ONLY) */
>  }
>  #endif /* defined(TARGET_PPC64) */
> @@ -4394,15 +4398,23 @@ static void gen_mtmsr(DisasContext *ctx)
>      CHK_SV;
>  
>  #if !defined(CONFIG_USER_ONLY)
> -   if (ctx->opcode & 0x00010000) {
> -        /* Special form that does not need any synchronisation */
> +    if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
> +        gen_io_start();
> +    }
> +    if (ctx->opcode & 0x00010000) {
> +        /* L=1 form only updates EE and RI */
>          TCGv t0 = tcg_temp_new();
> +        TCGv t1 = tcg_temp_new();
>          tcg_gen_andi_tl(t0, cpu_gpr[rS(ctx->opcode)],
>                          (1 << MSR_RI) | (1 << MSR_EE));
> -        tcg_gen_andi_tl(cpu_msr, cpu_msr,
> +        tcg_gen_andi_tl(t1, cpu_msr,
>                          ~(target_ulong)((1 << MSR_RI) | (1 << MSR_EE)));
> -        tcg_gen_or_tl(cpu_msr, cpu_msr, t0);
> +        tcg_gen_or_tl(t1, t1, t0);
> +
> +        gen_helper_store_msr(cpu_env, t1);
>          tcg_temp_free(t0);
> +        tcg_temp_free(t1);
> +
>      } else {
>          TCGv msr = tcg_temp_new();
>  
> @@ -4411,9 +4423,6 @@ static void gen_mtmsr(DisasContext *ctx)
>           *      power saving mode, we will exit the loop directly from
>           *      ppc_store_msr
>           */
> -        if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
> -            gen_io_start();
> -        }
>          gen_update_nip(ctx, ctx->base.pc_next);
>  #if defined(TARGET_PPC64)
>          tcg_gen_deposit_tl(msr, cpu_msr, cpu_gpr[rS(ctx->opcode)], 0, 32);
> @@ -4422,10 +4431,9 @@ static void gen_mtmsr(DisasContext *ctx)
>  #endif
>          gen_helper_store_msr(cpu_env, msr);
>          tcg_temp_free(msr);
> -        /* Must stop the translation as machine state (may have) changed */
> -        /* Note that mtmsr is not always defined as context-synchronizing */
> -        gen_stop_exception(ctx);
>      }
> +    /* Must stop the translation as machine state (may have) changed */
> +    gen_stop_exception(ctx);
>  #endif
>  }
>  

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: [PATCH] KVM: PPC: Book3S HV: Handle non-present PTEs in page fault functions
From: David Gibson @ 2020-04-17  0:47 UTC (permalink / raw)
  To: Cédric Le Goater; +Cc: kvm, groug, kvm-ppc, linux-kernel, linuxppc-dev
In-Reply-To: <a4e1bf29-af52-232e-d0d2-06206fa05fbe@kaod.org>

[-- Attachment #1: Type: text/plain, Size: 4724 bytes --]

On Thu, Apr 16, 2020 at 10:07:49AM +0200, Cédric Le Goater wrote:
> On 4/16/20 7:03 AM, Paul Mackerras wrote:
> > Since cd758a9b57ee "KVM: PPC: Book3S HV: Use __gfn_to_pfn_memslot in HPT
> > page fault handler", it's been possible in fairly rare circumstances to
> > load a non-present PTE in kvmppc_book3s_hv_page_fault() when running a
> > guest on a POWER8 host.
> > 
> > Because that case wasn't checked for, we could misinterpret the non-present
> > PTE as being a cache-inhibited PTE.  That could mismatch with the
> > corresponding hash PTE, which would cause the function to fail with -EFAULT
> > a little further down.  That would propagate up to the KVM_RUN ioctl()
> > generally causing the KVM userspace (usually qemu) to fall over.
> > 
> > This addresses the problem by catching that case and returning to the guest
> > instead, letting it fault again, and retrying the whole page fault from
> > the beginning.
> > 
> > For completeness, this fixes the radix page fault handler in the same
> > way.  For radix this didn't cause any obvious misbehaviour, because we
> > ended up putting the non-present PTE into the guest's partition-scoped
> > page tables, leading immediately to another hypervisor data/instruction
> > storage interrupt, which would go through the page fault path again
> > and fix things up.
> > 
> > Fixes: cd758a9b57ee "KVM: PPC: Book3S HV: Use __gfn_to_pfn_memslot in HPT page fault handler"
> > Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1820402
> > Reported-by: David Gibson <david@gibson.dropbear.id.au>
> > Signed-off-by: Paul Mackerras <paulus@ozlabs.org>
> 
> I didn't see the reported issue with the current 5.7-rc1. Anyhow I gave
> this patch a try on a P8 host and a P9 host with a radix guest and a hash 
> guest (using rhel6). Passthrough is fine also.
> 
> Tested-by: Cédric Le Goater <clg@kaod.org>
> 
> The code looks correct,
> 
> Reviewed-by: Cédric Le Goater <clg@kaod.org>

I ran my test case overnight with this patch for over 1000 iterations,
without any apparent problems so

Tested-by: David Gibson <david@gibson.dropbear.id.au>

> 
> Thanks,
> 
> C. 
> 
> 
> > ---
> > This is a reworked version of the patch David Gibson sent recently,
> > with the fix applied to the radix case as well. The commit message
> > is mostly stolen from David's patch.
> > 
> >  arch/powerpc/kvm/book3s_64_mmu_hv.c    | 9 +++++----
> >  arch/powerpc/kvm/book3s_64_mmu_radix.c | 9 +++++----
> >  2 files changed, 10 insertions(+), 8 deletions(-)
> > 
> > diff --git a/arch/powerpc/kvm/book3s_64_mmu_hv.c b/arch/powerpc/kvm/book3s_64_mmu_hv.c
> > index 3aecec8..20b7dce 100644
> > --- a/arch/powerpc/kvm/book3s_64_mmu_hv.c
> > +++ b/arch/powerpc/kvm/book3s_64_mmu_hv.c
> > @@ -604,18 +604,19 @@ int kvmppc_book3s_hv_page_fault(struct kvm_run *run, struct kvm_vcpu *vcpu,
> >  	 */
> >  	local_irq_disable();
> >  	ptep = __find_linux_pte(vcpu->arch.pgdir, hva, NULL, &shift);
> > +	pte = __pte(0);
> > +	if (ptep)
> > +		pte = *ptep;
> > +	local_irq_enable();
> >  	/*
> >  	 * If the PTE disappeared temporarily due to a THP
> >  	 * collapse, just return and let the guest try again.
> >  	 */
> > -	if (!ptep) {
> > -		local_irq_enable();
> > +	if (!pte_present(pte)) {
> >  		if (page)
> >  			put_page(page);
> >  		return RESUME_GUEST;
> >  	}
> > -	pte = *ptep;
> > -	local_irq_enable();
> >  	hpa = pte_pfn(pte) << PAGE_SHIFT;
> >  	pte_size = PAGE_SIZE;
> >  	if (shift)
> > diff --git a/arch/powerpc/kvm/book3s_64_mmu_radix.c b/arch/powerpc/kvm/book3s_64_mmu_radix.c
> > index 134fbc1..7bf94ba 100644
> > --- a/arch/powerpc/kvm/book3s_64_mmu_radix.c
> > +++ b/arch/powerpc/kvm/book3s_64_mmu_radix.c
> > @@ -815,18 +815,19 @@ int kvmppc_book3s_instantiate_page(struct kvm_vcpu *vcpu,
> >  	 */
> >  	local_irq_disable();
> >  	ptep = __find_linux_pte(vcpu->arch.pgdir, hva, NULL, &shift);
> > +	pte = __pte(0);
> > +	if (ptep)
> > +		pte = *ptep;
> > +	local_irq_enable();
> >  	/*
> >  	 * If the PTE disappeared temporarily due to a THP
> >  	 * collapse, just return and let the guest try again.
> >  	 */
> > -	if (!ptep) {
> > -		local_irq_enable();
> > +	if (!pte_present(pte)) {
> >  		if (page)
> >  			put_page(page);
> >  		return RESUME_GUEST;
> >  	}
> > -	pte = *ptep;
> > -	local_irq_enable();
> >  
> >  	/* If we're logging dirty pages, always map single pages */
> >  	large_enable = !(memslot->flags & KVM_MEM_LOG_DIRTY_PAGES);
> > 
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: [RFC PATCH 2/3] powerpc/lib: Initialize a temporary mm for code patching
From: Michael Ellerman @ 2020-04-17  0:57 UTC (permalink / raw)
  To: Christophe Leroy, Christopher M Riedl, linuxppc-dev
In-Reply-To: <8ff6d279-fdd9-4e4d-b4e0-f5c5cba480a4@c-s.fr>

Christophe Leroy <christophe.leroy@c-s.fr> writes:
> Le 31/03/2020 à 05:19, Christopher M Riedl a écrit :
>>> On March 24, 2020 11:10 AM Christophe Leroy <christophe.leroy@c-s.fr> wrote:
>>> Le 23/03/2020 à 05:52, Christopher M. Riedl a écrit :
>>>> When code patching a STRICT_KERNEL_RWX kernel the page containing the
>>>> address to be patched is temporarily mapped with permissive memory
>>>> protections. Currently, a per-cpu vmalloc patch area is used for this
>>>> purpose. While the patch area is per-cpu, the temporary page mapping is
>>>> inserted into the kernel page tables for the duration of the patching.
>>>> The mapping is exposed to CPUs other than the patching CPU - this is
>>>> undesirable from a hardening perspective.
>>>>
>>>> Use the `poking_init` init hook to prepare a temporary mm and patching
>>>> address. Initialize the temporary mm by copying the init mm. Choose a
>>>> randomized patching address inside the temporary mm userspace address
>>>> portion. The next patch uses the temporary mm and patching address for
>>>> code patching.
>>>>
>>>> Based on x86 implementation:
>>>>
>>>> commit 4fc19708b165
>>>> ("x86/alternatives: Initialize temporary mm for patching")
>>>>
>>>> Signed-off-by: Christopher M. Riedl <cmr@informatik.wtf>
>>>> ---
>>>>    arch/powerpc/lib/code-patching.c | 26 ++++++++++++++++++++++++++
>>>>    1 file changed, 26 insertions(+)
>>>>
>>>> diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
>>>> index 3345f039a876..18b88ecfc5a8 100644
>>>> --- a/arch/powerpc/lib/code-patching.c
>>>> +++ b/arch/powerpc/lib/code-patching.c
>>>> @@ -11,6 +11,8 @@
>>>>    #include <linux/cpuhotplug.h>
>>>>    #include <linux/slab.h>
>>>>    #include <linux/uaccess.h>
>>>> +#include <linux/sched/task.h>
>>>> +#include <linux/random.h>
>>>>    
>>>>    #include <asm/pgtable.h>
>>>>    #include <asm/tlbflush.h>
>>>> @@ -39,6 +41,30 @@ int raw_patch_instruction(unsigned int *addr, unsigned int instr)
>>>>    }
>>>>    
>>>>    #ifdef CONFIG_STRICT_KERNEL_RWX
>>>> +
>>>> +__ro_after_init struct mm_struct *patching_mm;
>>>> +__ro_after_init unsigned long patching_addr;
>>>
>>> Can we make those those static ?
>>>
>> 
>> Yes, makes sense to me.
>> 
>>>> +
>>>> +void __init poking_init(void)
>>>> +{
>>>> +	spinlock_t *ptl; /* for protecting pte table */
>>>> +	pte_t *ptep;
>>>> +
>>>> +	patching_mm = copy_init_mm();
>>>> +	BUG_ON(!patching_mm);
>>>
>>> Does it needs to be a BUG_ON() ? Can't we fail gracefully with just a
>>> WARN_ON ?
>>>
>> 
>> I'm not sure what failing gracefully means here? The main reason this could
>> fail is if there is not enough memory to allocate the patching_mm. The
>> previous implementation had this justification for BUG_ON():
>
> But the system can continue running just fine after this failure.
> Only the things that make use of code patching will fail (ftrace, kgdb, ...)

That's probably true of ftrace, but we can't fail patching for jump
labels (static keys).

See:

void arch_jump_label_transform(struct jump_entry *entry,
			       enum jump_label_type type)
{
	u32 *addr = (u32 *)(unsigned long)entry->code;

	if (type == JUMP_LABEL_JMP)
		patch_branch(addr, entry->target, 0);
	else
		patch_instruction(addr, PPC_INST_NOP);
}

cheers

^ permalink raw reply

* Re: [musl] Powerpc Linux 'scv' system call ABI proposal take 2
From: Rich Felker @ 2020-04-17  0:34 UTC (permalink / raw)
  To: Segher Boessenkool
  Cc: libc-alpha, musl, Nicholas Piggin, Florian Weimer, libc-dev,
	linuxppc-dev
In-Reply-To: <20200416230235.GG26902@gate.crashing.org>

On Thu, Apr 16, 2020 at 06:02:35PM -0500, Segher Boessenkool wrote:
> On Thu, Apr 16, 2020 at 08:12:19PM +0200, Florian Weimer wrote:
> > > I think my choice would be just making the inline syscall be a single
> > > call insn to an asm source file that out-of-lines the loading of TOC
> > > pointer and call through it or branch based on hwcap so that it's not
> > > repeated all over the place.
> > 
> > I don't know how problematic control flow out of an inline asm is on
> > POWER.  But this is basically the -moutline-atomics approach.
> 
> Control flow out of inline asm (other than with "asm goto") is not
> allowed at all, just like on any other target (and will not work in
> practice, either -- just like on any other target).  But the suggestion
> was to use actual assembler code, not inline asm?

Calling it control flow out of inline asm is something of a misnomer.
The enclosing state is not discarded or altered; the asm statement
exits normally, reaching the next instruction in the enclosing
block/function as soon as the call from the asm statement returns,
with all register/clobber constraints satisfied.

Control flow out of inline asm would be more like longjmp, and it can
be valid -- for instance, you can implement coroutines this way
(assuming you switch stack correctly) or do longjmp this way (jumping
to the location saved by setjmp). But it's not what'd be happening
here.

Rich

^ permalink raw reply

* Re: [musl] Powerpc Linux 'scv' system call ABI proposal take 2
From: Segher Boessenkool @ 2020-04-16 23:02 UTC (permalink / raw)
  To: Florian Weimer
  Cc: Rich Felker, libc-alpha, musl, Nicholas Piggin, libc-dev,
	linuxppc-dev
In-Reply-To: <87ftd3e1vg.fsf@mid.deneb.enyo.de>

On Thu, Apr 16, 2020 at 08:12:19PM +0200, Florian Weimer wrote:
> > I think my choice would be just making the inline syscall be a single
> > call insn to an asm source file that out-of-lines the loading of TOC
> > pointer and call through it or branch based on hwcap so that it's not
> > repeated all over the place.
> 
> I don't know how problematic control flow out of an inline asm is on
> POWER.  But this is basically the -moutline-atomics approach.

Control flow out of inline asm (other than with "asm goto") is not
allowed at all, just like on any other target (and will not work in
practice, either -- just like on any other target).  But the suggestion
was to use actual assembler code, not inline asm?


Segher

^ permalink raw reply

* Re: [PATCH V2] vhost: do not enable VHOST_MENU by default
From: Michael S. Tsirkin @ 2020-04-16 22:55 UTC (permalink / raw)
  To: Jason Wang
  Cc: linux-s390, tsbogend, gor, kvm, linux-kernel, heiko.carstens,
	linux-mips, virtualization, borntraeger, geert, netdev, paulus,
	linuxppc-dev
In-Reply-To: <20200415024356.23751-1-jasowang@redhat.com>

On Wed, Apr 15, 2020 at 10:43:56AM +0800, Jason Wang wrote:
> We try to keep the defconfig untouched after decoupling CONFIG_VHOST
> out of CONFIG_VIRTUALIZATION in commit 20c384f1ea1a
> ("vhost: refine vhost and vringh kconfig") by enabling VHOST_MENU by
> default. Then the defconfigs can keep enabling CONFIG_VHOST_NET
> without the caring of CONFIG_VHOST.
> 
> But this will leave a "CONFIG_VHOST_MENU=y" in all defconfigs and even
> for the ones that doesn't want vhost. So it actually shifts the
> burdens to the maintainers of all other to add "CONFIG_VHOST_MENU is
> not set". So this patch tries to enable CONFIG_VHOST explicitly in
> defconfigs that enables CONFIG_VHOST_NET and CONFIG_VHOST_VSOCK.
> 
> Acked-by: Christian Borntraeger <borntraeger@de.ibm.com> (s390)
> Acked-by: Michael Ellerman <mpe@ellerman.id.au> (powerpc)
> Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
> Cc: Vasily Gorbik <gor@linux.ibm.com>
> Cc: Christian Borntraeger <borntraeger@de.ibm.com>
> Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
> Signed-off-by: Jason Wang <jasowang@redhat.com>

I rebased this on top of OABI fix since that
seems more orgent to fix.
Pushed to my vhost branch pls take a look and
if possible test.
Thanks!

> ---
> Change since V1:
> - depends on EVENTFD for VHOST
> ---
>  arch/mips/configs/malta_kvm_defconfig  |  1 +
>  arch/powerpc/configs/powernv_defconfig |  1 +
>  arch/powerpc/configs/ppc64_defconfig   |  1 +
>  arch/powerpc/configs/pseries_defconfig |  1 +
>  arch/s390/configs/debug_defconfig      |  1 +
>  arch/s390/configs/defconfig            |  1 +
>  drivers/vhost/Kconfig                  | 26 +++++++++-----------------
>  7 files changed, 15 insertions(+), 17 deletions(-)
> 
> diff --git a/arch/mips/configs/malta_kvm_defconfig b/arch/mips/configs/malta_kvm_defconfig
> index 8ef612552a19..06f0c7a0ca87 100644
> --- a/arch/mips/configs/malta_kvm_defconfig
> +++ b/arch/mips/configs/malta_kvm_defconfig
> @@ -18,6 +18,7 @@ CONFIG_PCI=y
>  CONFIG_VIRTUALIZATION=y
>  CONFIG_KVM=m
>  CONFIG_KVM_MIPS_DEBUG_COP0_COUNTERS=y
> +CONFIG_VHOST=m
>  CONFIG_VHOST_NET=m
>  CONFIG_MODULES=y
>  CONFIG_MODULE_UNLOAD=y
> diff --git a/arch/powerpc/configs/powernv_defconfig b/arch/powerpc/configs/powernv_defconfig
> index 71749377d164..404245b4594d 100644
> --- a/arch/powerpc/configs/powernv_defconfig
> +++ b/arch/powerpc/configs/powernv_defconfig
> @@ -346,5 +346,6 @@ CONFIG_CRYPTO_DEV_VMX=y
>  CONFIG_VIRTUALIZATION=y
>  CONFIG_KVM_BOOK3S_64=m
>  CONFIG_KVM_BOOK3S_64_HV=m
> +CONFIG_VHOST=m
>  CONFIG_VHOST_NET=m
>  CONFIG_PRINTK_TIME=y
> diff --git a/arch/powerpc/configs/ppc64_defconfig b/arch/powerpc/configs/ppc64_defconfig
> index 7e68cb222c7b..4599fc7be285 100644
> --- a/arch/powerpc/configs/ppc64_defconfig
> +++ b/arch/powerpc/configs/ppc64_defconfig
> @@ -61,6 +61,7 @@ CONFIG_ELECTRA_CF=y
>  CONFIG_VIRTUALIZATION=y
>  CONFIG_KVM_BOOK3S_64=m
>  CONFIG_KVM_BOOK3S_64_HV=m
> +CONFIG_VHOST=m
>  CONFIG_VHOST_NET=m
>  CONFIG_OPROFILE=m
>  CONFIG_KPROBES=y
> diff --git a/arch/powerpc/configs/pseries_defconfig b/arch/powerpc/configs/pseries_defconfig
> index 6b68109e248f..4cad3901b5de 100644
> --- a/arch/powerpc/configs/pseries_defconfig
> +++ b/arch/powerpc/configs/pseries_defconfig
> @@ -321,5 +321,6 @@ CONFIG_CRYPTO_DEV_VMX=y
>  CONFIG_VIRTUALIZATION=y
>  CONFIG_KVM_BOOK3S_64=m
>  CONFIG_KVM_BOOK3S_64_HV=m
> +CONFIG_VHOST=m
>  CONFIG_VHOST_NET=m
>  CONFIG_PRINTK_TIME=y
> diff --git a/arch/s390/configs/debug_defconfig b/arch/s390/configs/debug_defconfig
> index 0c86ba19fa2b..6ec6e69630d1 100644
> --- a/arch/s390/configs/debug_defconfig
> +++ b/arch/s390/configs/debug_defconfig
> @@ -57,6 +57,7 @@ CONFIG_PROTECTED_VIRTUALIZATION_GUEST=y
>  CONFIG_CMM=m
>  CONFIG_APPLDATA_BASE=y
>  CONFIG_KVM=m
> +CONFIG_VHOST=m
>  CONFIG_VHOST_NET=m
>  CONFIG_VHOST_VSOCK=m
>  CONFIG_OPROFILE=m
> diff --git a/arch/s390/configs/defconfig b/arch/s390/configs/defconfig
> index 6b27d861a9a3..d1b3bf83d687 100644
> --- a/arch/s390/configs/defconfig
> +++ b/arch/s390/configs/defconfig
> @@ -57,6 +57,7 @@ CONFIG_PROTECTED_VIRTUALIZATION_GUEST=y
>  CONFIG_CMM=m
>  CONFIG_APPLDATA_BASE=y
>  CONFIG_KVM=m
> +CONFIG_VHOST=m
>  CONFIG_VHOST_NET=m
>  CONFIG_VHOST_VSOCK=m
>  CONFIG_OPROFILE=m
> diff --git a/drivers/vhost/Kconfig b/drivers/vhost/Kconfig
> index e79cbbdfea45..29f171a53d8a 100644
> --- a/drivers/vhost/Kconfig
> +++ b/drivers/vhost/Kconfig
> @@ -12,23 +12,19 @@ config VHOST_RING
>  	  This option is selected by any driver which needs to access
>  	  the host side of a virtio ring.
>  
> -config VHOST
> -	tristate
> +menuconfig VHOST
> +	tristate "Vhost Devices"
> +	depends on EVENTFD
>  	select VHOST_IOTLB
>  	help
> -	  This option is selected by any driver which needs to access
> -	  the core of vhost.
> +	  Enable option to support host kernel or hardware accelerator
> +	  for virtio device.
>  
> -menuconfig VHOST_MENU
> -	bool "VHOST drivers"
> -	default y
> -
> -if VHOST_MENU
> +if VHOST
>  
>  config VHOST_NET
>  	tristate "Host kernel accelerator for virtio net"
> -	depends on NET && EVENTFD && (TUN || !TUN) && (TAP || !TAP)
> -	select VHOST
> +	depends on NET && (TUN || !TUN) && (TAP || !TAP)
>  	---help---
>  	  This kernel module can be loaded in host kernel to accelerate
>  	  guest networking with virtio_net. Not to be confused with virtio_net
> @@ -39,8 +35,7 @@ config VHOST_NET
>  
>  config VHOST_SCSI
>  	tristate "VHOST_SCSI TCM fabric driver"
> -	depends on TARGET_CORE && EVENTFD
> -	select VHOST
> +	depends on TARGET_CORE
>  	default n
>  	---help---
>  	Say M here to enable the vhost_scsi TCM fabric module
> @@ -48,8 +43,7 @@ config VHOST_SCSI
>  
>  config VHOST_VSOCK
>  	tristate "vhost virtio-vsock driver"
> -	depends on VSOCKETS && EVENTFD
> -	select VHOST
> +	depends on VSOCKETS
>  	select VIRTIO_VSOCKETS_COMMON
>  	default n
>  	---help---
> @@ -62,8 +56,6 @@ config VHOST_VSOCK
>  
>  config VHOST_VDPA
>  	tristate "Vhost driver for vDPA-based backend"
> -	depends on EVENTFD
> -	select VHOST
>  	depends on VDPA
>  	help
>  	  This kernel module can be loaded in host kernel to accelerate
> -- 
> 2.20.1


^ permalink raw reply

* [PATCH v3] powerpc/setup_64: Set cache-line-size based on cache-block-size
From: Chris Packham @ 2020-04-16 22:19 UTC (permalink / raw)
  To: mpe, benh, christophe.leroy, tglx, paulus, cai, oss
  Cc: Chris Packham, linuxppc-dev, linux-kernel

If {i,d}-cache-block-size is set and {i,d}-cache-line-size is not, use
the block-size value for both. Per the devicetree spec cache-line-size
is only needed if it differs from the block size.

Originally the code would fallback from block size to line size. An
error message was printed if both properties were missing.

Later the code was refactored to use clearer names and logic but it
inadvertently made line size a required property. This caused the
default values to be used and in turn leads to Power9 systems using the
wrong size.

Fixes: bd067f83b084 ("powerpc/64: Fix naming of cache block vs. cache line")
Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---
It looks as though the bsizep = lsizep is not required per the spec but it's
probably safer to retain it.

Changes in v3:
- Rebase against 5.7.0-rc1
- Add Fixes tag
- Add more information to commit message

Changes in v2:
- Scott pointed out that u-boot should be filling in the cache properties
  (which it does). But it does not specify a cache-line-size because it
  provides a cache-block-size and the spec says you don't have to if they are
  the same. So the error is in the parsing not in the devicetree itself.

 arch/powerpc/kernel/setup_64.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c
index 438a9befce41..8105010b0e76 100644
--- a/arch/powerpc/kernel/setup_64.c
+++ b/arch/powerpc/kernel/setup_64.c
@@ -534,6 +534,8 @@ static bool __init parse_cache_info(struct device_node *np,
 	lsizep = of_get_property(np, propnames[3], NULL);
 	if (bsizep == NULL)
 		bsizep = lsizep;
+	if (lsizep == NULL)
+		lsizep = bsizep;
 	if (lsizep != NULL)
 		lsize = be32_to_cpu(*lsizep);
 	if (bsizep != NULL)
-- 
2.25.1


^ permalink raw reply related

* Re: [Bug 206203] kmemleak reports various leaks in drivers/of/unittest.c
From: Frank Rowand @ 2020-04-16 21:52 UTC (permalink / raw)
  To: Michael Ellerman, Rob Herring; +Cc: devicetree, linuxppc-dev
In-Reply-To: <a717b335-de6c-46eb-1daa-ae5054664e93@gmail.com>

On 4/15/20 10:27 PM, Frank Rowand wrote:
> On 4/8/20 10:22 AM, Frank Rowand wrote:
>> Hi Michael,
>>
>> On 4/7/20 10:13 PM, Michael Ellerman wrote:
>>> bugzilla-daemon@bugzilla.kernel.org writes:
>>>> https://bugzilla.kernel.org/show_bug.cgi?id=206203
>>>>
>>>> Erhard F. (erhard_f@mailbox.org) changed:
>>>>
>>>>            What    |Removed                     |Added
>>>> ----------------------------------------------------------------------------
>>>>  Attachment #286801|0                           |1
>>>>         is obsolete|                            |
>>>>
>>>> --- Comment #10 from Erhard F. (erhard_f@mailbox.org) ---
>>>> Created attachment 288189
>>>>   --> https://bugzilla.kernel.org/attachment.cgi?id=288189&action=edit
>>>> kmemleak output (kernel 5.6.2, Talos II)
>>>
>>> These are all in or triggered by the of unittest code AFAICS.
>>> Content of the log reproduced below.
>>>
>>> Frank/Rob, are these memory leaks expected?
>>
>> Thanks for the report.  I'll look at each one.
> 
> Only one of the leaks was expected.  I have patches to fix the
> unexpected leaks and to remove the expected leak so that the
> kmemleak report of it will not have to be checked again.
> 
> I expect to send the patch series tomorrow (Thursday).

The patches for the memory leaks that I saw on an ARM board are at:

   https://lore.kernel.org/r/1587073370-25963-1-git-send-email-frowand.list@gmail.com

-Frank

< snip >

^ permalink raw reply

* [PATCH] PCI: Use of_node_name_eq for node name comparisons
From: Rob Herring @ 2020-04-16 21:51 UTC (permalink / raw)
  To: Bjorn Helgaas; +Cc: linux-pci, Paul Mackerras, linuxppc-dev

Convert string compares of DT node names to use of_node_name_eq helper
instead. This removes direct access to the node name pointer.

Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-pci@vger.kernel.org
Signed-off-by: Rob Herring <robh@kernel.org>
---
 drivers/pci/hotplug/rpaphp_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/hotplug/rpaphp_core.c b/drivers/pci/hotplug/rpaphp_core.c
index 6504869efabc..9887c9de08c3 100644
--- a/drivers/pci/hotplug/rpaphp_core.c
+++ b/drivers/pci/hotplug/rpaphp_core.c
@@ -435,7 +435,7 @@ static int rpaphp_drc_add_slot(struct device_node *dn)
  */
 int rpaphp_add_slot(struct device_node *dn)
 {
-	if (!dn->name || strcmp(dn->name, "pci"))
+	if (!of_node_name_eq(dn, "pci"))
 		return 0;
 
 	if (of_find_property(dn, "ibm,drc-info", NULL))
-- 
2.20.1


^ permalink raw reply related

* Re: [PATCH v4,4/4] drivers: uio: new driver for fsl_85xx_cache_sram
From: Rob Herring @ 2020-04-16 21:35 UTC (permalink / raw)
  To: Scott Wood; +Cc: gregkh, linux-kernel, kernel, Wang Wenhu, linuxppc-dev
In-Reply-To: <16f8fa2d26d88f22ed05e9870709c2fd5c3960cf.camel@buserror.net>

On Thu, Apr 16, 2020 at 02:59:36PM -0500, Scott Wood wrote:
> On Thu, 2020-04-16 at 08:35 -0700, Wang Wenhu wrote:
> > +#define UIO_INFO_VER	"devicetree,pseudo"
> 
> What does this mean?  Changing a number into a non-obvious string (Why
> "pseudo"?  Why does the UIO user care that the config came from the device
> tree?) just to avoid setting off Greg's version number autoresponse isn't
> really helping anything.
> 
> > +static const struct of_device_id uio_mpc85xx_l2ctlr_of_match[] = {
> > +	{	.compatible = "uio,mpc85xx-cache-sram",	},

Form is <vendor>,<device> and "uio" is not a vendor (and never will be).

> > +	{},
> > +};
> > +
> > +static struct platform_driver uio_fsl_85xx_cache_sram = {
> > +	.probe = uio_fsl_85xx_cache_sram_probe,
> > +	.remove = uio_fsl_85xx_cache_sram_remove,
> > +	.driver = {
> > +		.name = DRIVER_NAME,
> > +		.owner = THIS_MODULE,
> > +		.of_match_table	= uio_mpc85xx_l2ctlr_of_match,
> > +	},
> > +};
> 
> Greg's comment notwithstanding, I really don't think this belongs in the
> device tree (and if I do get overruled on that point, it at least needs a
> binding document).  Let me try to come up with a patch for dynamic allocation.

Agreed. "UIO" bindings have long been rejected.

Rob

^ permalink raw reply

* Re: [PATCH v2] powerpc/setup_64: Set cache-line-size based on cache-block-size
From: Chris Packham @ 2020-04-16 21:28 UTC (permalink / raw)
  To: christophe.leroy@c-s.fr, paulus@samba.org, mpe@ellerman.id.au,
	benh@kernel.crashing.org, oss@buserror.net, tglx@linutronix.de
  Cc: Hamish Martin, linuxppc-dev@lists.ozlabs.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <87tv1jirlj.fsf@mpe.ellerman.id.au>

On Thu, 2020-04-16 at 21:43 +1000, Michael Ellerman wrote:
> Chris Packham <Chris.Packham@alliedtelesis.co.nz> writes:
> > Hi All,
> > 
> > On Wed, 2020-03-25 at 16:18 +1300, Chris Packham wrote:
> > > If {i,d}-cache-block-size is set and {i,d}-cache-line-size is
> > > not,
> > > use
> > > the block-size value for both. Per the devicetree spec cache-
> > > line-
> > > size
> > > is only needed if it differs from the block size.
> > > 
> > > Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> > > ---
> > > It looks as though the bsizep = lsizep is not required per the
> > > spec
> > > but it's
> > > probably safer to retain it.
> > > 
> > > Changes in v2:
> > > - Scott pointed out that u-boot should be filling in the cache
> > > properties
> > >   (which it does). But it does not specify a cache-line-size
> > > because
> > > it
> > >   provides a cache-block-size and the spec says you don't have to
> > > if
> > > they are
> > >   the same. So the error is in the parsing not in the devicetree
> > > itself.
> > > 
> > 
> > Ping? This thread went kind of quiet.
> 
> I replied in the other thread:
> 
>   
> https://lore.kernel.org/linuxppc-dev/87369xx99u.fsf@mpe.ellerman.id.au/
> 
> But then the merge window happened which is a busy time.
> 

Yeah I figured that was the case.

> What I'd really like is a v3 that incorporates the info I wrote in
> the
> other thread and a Fixes tag.
> 
> If you feel like doing that, that would be great. Otherwise I'll do
> it
> tomorrow.

I'll rebase against Linus's tree and have a go a adding some more words
to the commit message.

> 
> cheers

^ permalink raw reply

* Re: [PATCH] PCI/DPC: Allow Non-ACPI Native ports to use DPC
From: Derrick, Jonathan @ 2020-04-16 20:50 UTC (permalink / raw)
  To: sathyanarayanan.kuppuswamy@linux.intel.com, helgaas@kernel.org
  Cc: bhelgaas@google.com, fred@fredlawl.com, sbobroff@linux.ibm.com,
	linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org,
	kbusch@kernel.org, oohall@gmail.com, olof@lixom.net,
	linuxppc-dev@lists.ozlabs.org
In-Reply-To: <4c2bf639-5510-fb17-2f02-73e7016d8f70@linux.intel.com>

On Thu, 2020-04-16 at 13:32 -0700, Kuppuswamy, Sathyanarayanan wrote:
> Hi,
> 
> On 4/16/20 12:59 PM, Jon Derrick wrote:
> > Some platforms have a mix of ports whose capabilities can be negotiated
> > by _OSC, and some ports which are not described by ACPI and instead
> > managed by Native drivers. The existing Firmware-First HEST model can
> > incorrectly tag these Native, Non-ACPI ports as Firmware-First capable
> > ports by advertising the HEST Global flag and specifying the type and
> > class (aer_hest_parse).
> > 
> > This ultimately can lead to bad situations if the BIOS or port firmware
> > leaves DPC preconfigured and the Linux DPC driver is unable to bind to
> > the port to handle DPC events.
> > 
> > This patch adds the check for Native DPC in the port's host bridge in
> > order to allow DPC services to bind to the port.
> > 
> > Signed-off-by: Jon Derrick <jonathan.derrick@intel.com>
> > ---
> >   drivers/pci/pcie/dpc.c | 4 +++-
> >   1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/pci/pcie/dpc.c b/drivers/pci/pcie/dpc.c
> > index 7621704..a1e355d 100644
> > --- a/drivers/pci/pcie/dpc.c
> > +++ b/drivers/pci/pcie/dpc.c
> > @@ -281,10 +281,12 @@ static int dpc_probe(struct pcie_device *dev)
> >   {
> >   	struct pci_dev *pdev = dev->port;
> >   	struct device *device = &dev->device;
> > +	struct pci_host_bridge *host = pci_find_host_bridge(pdev->bus);
> >   	int status;
> >   	u16 ctl, cap;
> >   
> > -	if (pcie_aer_get_firmware_first(pdev) && !pcie_ports_dpc_native)
> > +	if (pcie_aer_get_firmware_first(pdev) && !pcie_ports_dpc_native &&
> For other PCIe services, this check is added in 
> get_port_device_capability().
> Why not add it there for DPC as well ?

Sure. Looking at this, it seems like it needs some more de-tangling to
fit into my model.

> > +	    !host->native_dpc)
> >   		return -ENOTSUPP;
> >   
> >   	status = devm_request_threaded_irq(device, dev->irq, dpc_irq,
> > 

^ permalink raw reply

* Re: [PATCH 11/29] mm: only allow page table mappings for built-in zsmalloc
From: Minchan Kim @ 2020-04-16 20:43 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: linux-hyperv, sergey.senozhatsky.work, David Airlie, dri-devel,
	linux-mm, K. Y. Srinivasan, Sumit Semwal, linux-arch, linux-s390,
	Wei Liu, Stephen Hemminger, x86, Peter Zijlstra, Laura Abbott,
	Nitin Gupta, Daniel Vetter, Haiyang Zhang, linaro-mm-sig, bpf,
	linux-arm-kernel, Robin Murphy, linux-kernel, iommu, Sakari Ailus,
	Andrew Morton, linuxppc-dev
In-Reply-To: <20200414131348.444715-12-hch@lst.de>

On Tue, Apr 14, 2020 at 03:13:30PM +0200, Christoph Hellwig wrote:
> This allows to unexport map_vm_area and unmap_kernel_range, which are
> rather deep internal and should not be available to modules, as they for
> example allow fine grained control of mapping permissions, and also
> allow splitting the setup of a vmalloc area and the actual mapping and
> thus expose vmalloc internals.
> 
> zsmalloc is typically built-in and continues to work (just like the
> percpu-vm code using a similar patter), while modular zsmalloc also
> continues to work, but must use copies.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Acked-by: Minchan Kim <minchan@kernel.org>

Thanks!

^ permalink raw reply

* Re: [PATCH 10/28] mm: only allow page table mappings for built-in zsmalloc
From: Minchan Kim @ 2020-04-16 20:37 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: linux-hyperv, David Airlie, dri-devel, linux-mm, K. Y. Srinivasan,
	Sumit Semwal, linux-arch, linux-s390, Wei Liu, Stephen Hemminger,
	x86, Peter Zijlstra, Laura Abbott, Nitin Gupta, Daniel Vetter,
	Haiyang Zhang, linaro-mm-sig, bpf, linux-arm-kernel, Robin Murphy,
	linux-kernel, Sergey Senozhatsky, iommu, Sakari Ailus,
	Andrew Morton, linuxppc-dev
In-Reply-To: <20200411072052.GA31242@lst.de>

Hi Christoph,


Sorry for the late.

On Sat, Apr 11, 2020 at 09:20:52AM +0200, Christoph Hellwig wrote:
> Hi Minchan,
> 
> On Fri, Apr 10, 2020 at 04:11:36PM -0700, Minchan Kim wrote:
> > It doesn't mean we couldn't use zsmalloc as module any longer. It means
> > we couldn't use zsmalloc as module with pgtable mapping whcih was little
> > bit faster on microbenchmark in some architecutre(However, I usually temped
> > to remove it since it had several problems). However, we could still use
> > zsmalloc as module as copy way instead of pgtable mapping. Thus, if someone
> > really want to rollback the feature, they should provide reasonable reason
> > why it doesn't work for them. "A little fast" wouldn't be enough to exports
> > deep internal to the module.
> 
> do you have any data how much faster it is on arm (and does that include
> arm64 as well)?  Besides the exports which were my prime concern,

https://github.com/sjenning/zsmapbench

I need to recall the memory. IIRC, it was almost 30% faster at that time
in ARM so was not trivial at that time. However, it was story from
several years ago.

> zsmalloc with pgtable mappings also is the only user of map_kernel_range
> outside of vmalloc.c, if it really is another code base for tiny
> improvements we could mark map_kernel_range or in fact remove it entirely
> and open code it in the remaining callers.

I alsh have temped to remove it. Let me have time to revist it in this
chance.

Thanks.

^ permalink raw reply

* Re: [PATCH] PCI/DPC: Allow Non-ACPI Native ports to use DPC
From: Kuppuswamy, Sathyanarayanan @ 2020-04-16 20:32 UTC (permalink / raw)
  To: Jon Derrick, Bjorn Helgaas
  Cc: Frederick Lawler, Sam Bobroff, linux-pci, linux-kernel,
	Keith Busch, Olof Johansson, Oliver O'Halloran, Bjorn Helgaas,
	linuxppc-dev
In-Reply-To: <1587067157-2291-1-git-send-email-jonathan.derrick@intel.com>

Hi,

On 4/16/20 12:59 PM, Jon Derrick wrote:
> Some platforms have a mix of ports whose capabilities can be negotiated
> by _OSC, and some ports which are not described by ACPI and instead
> managed by Native drivers. The existing Firmware-First HEST model can
> incorrectly tag these Native, Non-ACPI ports as Firmware-First capable
> ports by advertising the HEST Global flag and specifying the type and
> class (aer_hest_parse).
> 
> This ultimately can lead to bad situations if the BIOS or port firmware
> leaves DPC preconfigured and the Linux DPC driver is unable to bind to
> the port to handle DPC events.
> 
> This patch adds the check for Native DPC in the port's host bridge in
> order to allow DPC services to bind to the port.
> 
> Signed-off-by: Jon Derrick <jonathan.derrick@intel.com>
> ---
>   drivers/pci/pcie/dpc.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/pcie/dpc.c b/drivers/pci/pcie/dpc.c
> index 7621704..a1e355d 100644
> --- a/drivers/pci/pcie/dpc.c
> +++ b/drivers/pci/pcie/dpc.c
> @@ -281,10 +281,12 @@ static int dpc_probe(struct pcie_device *dev)
>   {
>   	struct pci_dev *pdev = dev->port;
>   	struct device *device = &dev->device;
> +	struct pci_host_bridge *host = pci_find_host_bridge(pdev->bus);
>   	int status;
>   	u16 ctl, cap;
>   
> -	if (pcie_aer_get_firmware_first(pdev) && !pcie_ports_dpc_native)
> +	if (pcie_aer_get_firmware_first(pdev) && !pcie_ports_dpc_native &&
For other PCIe services, this check is added in 
get_port_device_capability().
Why not add it there for DPC as well ?
> +	    !host->native_dpc)
>   		return -ENOTSUPP;
>   
>   	status = devm_request_threaded_irq(device, dev->irq, dpc_irq,
> 

^ permalink raw reply

* Re: [musl] Powerpc Linux 'scv' system call ABI proposal take 2
From: Florian Weimer @ 2020-04-16 20:18 UTC (permalink / raw)
  To: Nicholas Piggin via Libc-alpha
  Cc: libc-dev, Rich Felker, linuxppc-dev, Nicholas Piggin, musl
In-Reply-To: <1587004907.ioxh0bxsln.astroid@bobo.none>

* Nicholas Piggin via Libc-alpha:

> We may or may not be getting a new ABI that will use instructions not 
> supported by old processors.
>
> https://sourceware.org/legacy-ml/binutils/2019-05/msg00331.html
>
> Current ABI continues to work of course and be the default for some 
> time, but building for new one would give some opportunity to drop
> such support for old procs, at least for glibc.

If I recall correctly, during last year's GNU Tools Cauldron, I think
it was pretty clear that this was only to be used for intra-DSO ABIs,
not cross-DSO optimization.  Relocatable object files have an ABI,
too, of course, so that's why there's a ABI documentation needed.

For cross-DSO optimization, the link editor would look at the DSO
being linked in, check if it uses the -mfuture ABI, and apply some
shortcuts.  But at that point, if the DSO is swapped back to a version
built without -mfuture, it no longer works with those newly linked
binaries against the -mfuture version.  Such a thing is a clear ABI
bump, and based what I remember from Cauldron, that is not the plan
here.

(I don't have any insider knowledge—I just don't want people to read
this think: gosh, yet another POWER ABI bump.  But the PCREL stuff
*is* exciting!)

^ permalink raw reply

* [PATCH] PCI/DPC: Allow Non-ACPI Native ports to use DPC
From: Jon Derrick @ 2020-04-16 19:59 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Kuppuswamy Sathyanarayanan, Frederick Lawler, Sam Bobroff,
	linux-pci, linux-kernel, Keith Busch, Olof Johansson,
	Oliver O'Halloran, Bjorn Helgaas, linuxppc-dev, Jon Derrick

Some platforms have a mix of ports whose capabilities can be negotiated
by _OSC, and some ports which are not described by ACPI and instead
managed by Native drivers. The existing Firmware-First HEST model can
incorrectly tag these Native, Non-ACPI ports as Firmware-First capable
ports by advertising the HEST Global flag and specifying the type and
class (aer_hest_parse).

This ultimately can lead to bad situations if the BIOS or port firmware
leaves DPC preconfigured and the Linux DPC driver is unable to bind to
the port to handle DPC events.

This patch adds the check for Native DPC in the port's host bridge in
order to allow DPC services to bind to the port.

Signed-off-by: Jon Derrick <jonathan.derrick@intel.com>
---
 drivers/pci/pcie/dpc.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/pcie/dpc.c b/drivers/pci/pcie/dpc.c
index 7621704..a1e355d 100644
--- a/drivers/pci/pcie/dpc.c
+++ b/drivers/pci/pcie/dpc.c
@@ -281,10 +281,12 @@ static int dpc_probe(struct pcie_device *dev)
 {
 	struct pci_dev *pdev = dev->port;
 	struct device *device = &dev->device;
+	struct pci_host_bridge *host = pci_find_host_bridge(pdev->bus);
 	int status;
 	u16 ctl, cap;
 
-	if (pcie_aer_get_firmware_first(pdev) && !pcie_ports_dpc_native)
+	if (pcie_aer_get_firmware_first(pdev) && !pcie_ports_dpc_native &&
+	    !host->native_dpc)
 		return -ENOTSUPP;
 
 	status = devm_request_threaded_irq(device, dev->irq, dpc_irq,
-- 
1.8.3.1


^ permalink raw reply related

* Re: [PATCH v4,4/4] drivers: uio: new driver for fsl_85xx_cache_sram
From: Scott Wood @ 2020-04-16 19:59 UTC (permalink / raw)
  To: Wang Wenhu, gregkh, linux-kernel, christophe.leroy, linuxppc-dev; +Cc: kernel
In-Reply-To: <20200416153537.23736-5-wenhu.wang@vivo.com>

On Thu, 2020-04-16 at 08:35 -0700, Wang Wenhu wrote:
> +#define UIO_INFO_VER	"devicetree,pseudo"

What does this mean?  Changing a number into a non-obvious string (Why
"pseudo"?  Why does the UIO user care that the config came from the device
tree?) just to avoid setting off Greg's version number autoresponse isn't
really helping anything.

> +static const struct of_device_id uio_mpc85xx_l2ctlr_of_match[] = {
> +	{	.compatible = "uio,mpc85xx-cache-sram",	},
> +	{},
> +};
> +
> +static struct platform_driver uio_fsl_85xx_cache_sram = {
> +	.probe = uio_fsl_85xx_cache_sram_probe,
> +	.remove = uio_fsl_85xx_cache_sram_remove,
> +	.driver = {
> +		.name = DRIVER_NAME,
> +		.owner = THIS_MODULE,
> +		.of_match_table	= uio_mpc85xx_l2ctlr_of_match,
> +	},
> +};

Greg's comment notwithstanding, I really don't think this belongs in the
device tree (and if I do get overruled on that point, it at least needs a
binding document).  Let me try to come up with a patch for dynamic allocation.

-Scott



^ permalink raw reply

* Re: [PATCH v2,5/5] drivers: uio: new driver for fsl_85xx_cache_sram
From: Scott Wood @ 2020-04-16 19:40 UTC (permalink / raw)
  To: Greg KH; +Cc: linux-kernel, kernel, Wang Wenhu, linuxppc-dev
In-Reply-To: <20200416063002.GA299193@kroah.com>

On Thu, 2020-04-16 at 08:30 +0200, Greg KH wrote:
> On Wed, Apr 15, 2020 at 02:26:55PM -0500, Scott Wood wrote:
> > Instead, have module parameters that take the sizes and alignments you'd
> > like
> > to allocate and expose to userspace.  Better still would be some sort of
> > dynamic allocation (e.g. open a fd, ioctl to set the requested
> > size/alignment,
> > if it succeeds you can mmap it, and when the fd is closed the region is
> > freed).
> 
> No module parameters please, this is not the 1990's.
> 
> Use device tree, that is what it is there for.

Since when is the device tree for indicating desired allocations?  This is not
hardware description.

If module parameters are unacceptable, then I'd suggest dynamic allocation as
described above.

-Scott



^ 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