[PATCH v3 0/2] powerpc rng cleanups

[Jason A. Donenfeld]

Hi Sachin, Michael,

On Thu, Jun 30, 2022 at 6:12 PM Sachin Sant <sachinp at linux.ibm.com> wrote:
> > On 30-Jun-2022, at 7:31 PM, Jason A. Donenfeld <Jason at zx2c4.com> wrote:
> >
> > These are two small cleanups for -next.
> >
> > I'm sending this v3 because very likely
> > https://lore.kernel.org/all/20220630121654.1939181-1-Jason@zx2c4.com/
> > will land first, in which case this needs a small adjustment.
> >
> > Jason A. Donenfeld (2):
> >  powerpc/powernv: rename remaining rng powernv_ functions to pnv_
> >  powerpc/kvm: don't crash on missing rng, and use darn
> >
> > arch/powerpc/include/asm/archrandom.h |  7 +--
> > arch/powerpc/kvm/book3s_hv_builtin.c  |  7 +--
> > arch/powerpc/platforms/powernv/rng.c  | 65 ++++++++++-----------------
> > drivers/char/hw_random/powernv-rng.c  |  2 +-
> > 4 files changed, 30 insertions(+), 51 deletions(-)
> >
>
> I tried these 2 patches + previous one (to fix kobject warning) on
> top of 5.19.0-rc4-next-20220630 next on a Power8 server.
>
> 5.19.0-rc4-next-20220630 +
> powerpc/powernv: delay rng of node creation until later in boot +
> powerpc/powernv: rename remaining rng powernv_ functions to pnv_ +
> powerpc/kvm: don't crash on missing rng, and use darn
>
> Unfortunately it fails to boot with following crash
>
> [    0.000000] ftrace: allocated 13 pages with 3 groups
> [    0.000000] trace event string verifier disabled
> [    0.000000] rcu: Hierarchical RCU implementation.
> [    0.000000] rcu:     RCU restricting CPUs from NR_CPUS=2048 to nr_cpu_ids=80.
> [    0.000000]  Rude variant of Tasks RCU enabled.
> [    0.000000] rcu: RCU calculated value of scheduler-enlistment delay is 10 jiffies.
> [    0.000000] rcu: Adjusting geometry for rcu_fanout_leaf=16, nr_cpu_ids=80
> [    0.000000] NR_IRQS: 512, nr_irqs: 512, preallocated irqs: 16
> [    0.000000] ICS OPAL backend registered
> [    0.000000] rcu: srcu_init: Setting srcu_struct sizes based on contention.
> [    0.000001] clocksource: timebase: mask: 0xffffffffffffffff max_cycles: 0x761537d007, max_idle_ns: 440795202126 ns
> [    0.000182] clocksource: timebase mult[1f40000] shift[24] registered
> [    0.001905] BUG: Unable to handle kernel data access on read at 0x3ffff40000000
> [    0.002032] Faulting instruction address: 0xc0000000000d7990
> [    0.002132] Oops: Kernel access of bad area, sig: 7 [#1]
> [    0.002226] LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA PowerNV
> [    0.002338] Modules linked in:
> [    0.002396] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.19.0-rc4-next-20220630-dirty #20
> [    0.002539] NIP:  c0000000000d7990 LR: c00000000201fa74 CTR: c0000000000d7960
> [    0.002663] REGS: c000000002a0fa60 TRAP: 0300   Not tainted  (5.19.0-rc4-next-20220630-dirty)
> [    0.002812] MSR:  9000000002001033 <SF,HV,VEC,ME,IR,DR,RI,LE>  CR: 44000228  XER: 20000000
> [    0.002979] CFAR: c00000000201fa70 DAR: 0003ffff40000000 DSISR: 00000002 IRQMASK: 1
> [    0.002979] GPR00: c00000000201fa74 c000000002a0fd00 c000000002a12000 c000000002a0fe90
> [    0.002979] GPR04: 0000000000000001 0000000000000000 c000000000deb578 000000000000006f
> [    0.002979] GPR08: 0000000000000000 0003ffff40000000 c000000007040080 0000000000000000
> [    0.002979] GPR12: c0000000000d7960 c000000002d00000 0000000000000003 0000000000000000
> [    0.002979] GPR16: 0000000000000000 0000000000000000 0000000000000278 c000000002a4dfe0
> [    0.002979] GPR20: c000000002a52238 c000000002a52820 c0000000000d7960 c000000000fe6c50
> [    0.002979] GPR24: 0000000000000001 c000000002a0fe90 c000000000fe6c40 c000000002141ff8
> [    0.002979] GPR28: 0000000000000800 c0000000070400a0 c000000002ab0150 0000000000000000
> [    0.004279] NIP [c0000000000d7990] pnv_get_random_long+0x30/0xd0
> [    0.004390] LR [c00000000201fa74] pnv_get_random_long_early+0x268/0x2d0
> [    0.004509] Call Trace:
> [    0.004553] [c000000002a0fd00] [c00000000201fa4c] pnv_get_random_long_early+0x240/0x2d0 (unreliable)
> [    0.004718] [c000000002a0fe20] [c000000002060d5c] random_init+0xc0/0x214
> [    0.004844] [c000000002a0fec0] [c0000000020048c0] start_kernel+0x990/0xbf8
> [    0.004972] [c000000002a0ff90] [c00000000000d878] start_here_common+0x1c/0x24
> [    0.005102] Instruction dump:
> [    0.005156] 3c4c0294 3842a6a0 7c0802a6 60000000 7d2000a6 71290010 41820048 e94d0030
> [    0.005309] 3d22ff73 3929fff8 7d4a482a e92a0008 <7d204eea> e8ea0010 7d2803f4 7ce94a78
> [    0.005465] ---[ end trace 0000000000000000 ]---
> [    0.005545]
> [    1.005574] Kernel panic - not syncing: Fatal exception
> [    1.005671] Rebooting in 10 seconds..
>
> Reverting powerpc/kvm: don't crash on missing rng, and use darn helps to boot
> the server successfully.

Huh! Thanks for testing that so fast.

That commit has this block in it:

+       if (mfmsr() & MSR_DR) {
+               rng = raw_cpu_read(pnv_rng);
+               *v = rng_whiten(rng, __raw_rm_readq(rng->regs_real));

The idea was that `mfmsr() & MSR_DR` would be true if we're in real
mode. But I don't actually know what that's doing; mpe suggested it to
me. Is it possible the condition is inverted and I should have done
`!(mfmsr() & MSR_DR)`? Or maybe there's a better flag to check than
the DR one?

Jason