[PATCH v3 0/2] powerpc rng cleanups

[Christophe Leroy]

Le 30/06/2022 à 18:46, Jason A. Donenfeld a écrit :
> 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?

When DR is set you are in virtual mode
When DR is unset you are in real mode

Extract from documentation:

DR Data address translation
0 Data address translation is disabled.
1 Data address translation is enabled.


Christophe