Re: [PATCH v2 5/5] bpf ppc32: Add instructions for atomic_[cmp]xchg
Le 10/06/2022 à 17:55, Hari Bathini a écrit : > This adds two atomic opcodes BPF_XCHG and BPF_CMPXCHG on ppc32, both > of which include the BPF_FETCH flag. The kernel's atomic_cmpxchg > operation fundamentally has 3 operands, but we only have two register > fields. Therefore the operand we compare against (the kernel's API > calls it 'old') is hard-coded to be BPF_REG_R0. Also, kernel's > atomic_cmpxchg returns the previous value at dst_reg + off. JIT the > same for BPF too with return value put in BPF_REG_0. > >BPF_REG_R0 = atomic_cmpxchg(dst_reg + off, BPF_REG_R0, src_reg); > > Signed-off-by: Hari Bathini > --- > > Changes in v2: > * Moved variable declaration to avoid late declaration error on >some compilers. > * Tried to make code readable and compact. > > > arch/powerpc/net/bpf_jit_comp32.c | 25 ++--- > 1 file changed, 22 insertions(+), 3 deletions(-) > > diff --git a/arch/powerpc/net/bpf_jit_comp32.c > b/arch/powerpc/net/bpf_jit_comp32.c > index 28dc6a1a8f2f..43f1c76d48ce 100644 > --- a/arch/powerpc/net/bpf_jit_comp32.c > +++ b/arch/powerpc/net/bpf_jit_comp32.c > @@ -297,6 +297,7 @@ int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, > struct codegen_context * > u32 ax_reg = bpf_to_ppc(BPF_REG_AX); > u32 tmp_reg = bpf_to_ppc(TMP_REG); > u32 size = BPF_SIZE(code); > + u32 save_reg, ret_reg; > s16 off = insn[i].off; > s32 imm = insn[i].imm; > bool func_addr_fixed; > @@ -799,6 +800,9 @@ int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, > struct codegen_context * >* BPF_STX ATOMIC (atomic ops) >*/ > case BPF_STX | BPF_ATOMIC | BPF_W: > + save_reg = _R0; > + ret_reg = src_reg; > + > bpf_set_seen_register(ctx, tmp_reg); > bpf_set_seen_register(ctx, ax_reg); > > @@ -829,6 +833,21 @@ int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, > struct codegen_context * > case BPF_XOR | BPF_FETCH: > EMIT(PPC_RAW_XOR(_R0, _R0, src_reg)); > break; > + case BPF_CMPXCHG: > + /* > + * Return old value in BPF_REG_0 for > BPF_CMPXCHG & > + * in src_reg for other cases. > + */ > + ret_reg = bpf_to_ppc(BPF_REG_0); > + > + /* Compare with old value in BPF_REG_0 */ > + EMIT(PPC_RAW_CMPW(bpf_to_ppc(BPF_REG_0), _R0)); > + /* Don't set if different from old value */ > + PPC_BCC_SHORT(COND_NE, (ctx->idx + 3) * 4); > + fallthrough; > + case BPF_XCHG: > + save_reg = src_reg; I'm a bit lost, when save_reg is src_reg, don't we expect the upper part (ie src_reg - 1) to be explicitely zeroised ? > + break; > default: > pr_err_ratelimited("eBPF filter atomic op code > %02x (@%d) unsupported\n", > code, i); > @@ -836,15 +855,15 @@ int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, > struct codegen_context * > } > > /* store new value */ > - EMIT(PPC_RAW_STWCX(_R0, tmp_reg, dst_reg)); > + EMIT(PPC_RAW_STWCX(save_reg, tmp_reg, dst_reg)); > /* we're done if this succeeded */ > PPC_BCC_SHORT(COND_NE, tmp_idx); > > /* For the BPF_FETCH variant, get old data into src_reg > */ > if (imm & BPF_FETCH) { > - EMIT(PPC_RAW_MR(src_reg, ax_reg)); > + EMIT(PPC_RAW_MR(ret_reg, ax_reg)); > if (!fp->aux->verifier_zext) > - EMIT(PPC_RAW_LI(src_reg_h, 0)); > + EMIT(PPC_RAW_LI(ret_reg - 1, 0)); /* > higher 32-bit */ > } > break; >
Re: [PATCH v2 4/5] bpf ppc32: add support for BPF_ATOMIC bitwise operations
Le 10/06/2022 à 17:55, Hari Bathini a écrit : > Adding instructions for ppc32 for > > atomic_and > atomic_or > atomic_xor > atomic_fetch_add > atomic_fetch_and > atomic_fetch_or > atomic_fetch_xor > > Signed-off-by: Hari Bathini > --- > > Changes in v2: > * Used an additional register (BPF_REG_AX) > - to avoid clobbering src_reg. > - to keep the lwarx reservation as intended. > - to avoid the odd switch/goto construct. Might be a stupid question as I don't know the internals of BPF: Are we sure BPF_REG_AX cannot be the src reg or the dst reg ? > * Zero'ed out the higher 32-bit explicitly when required. > > arch/powerpc/net/bpf_jit_comp32.c | 53 --- > 1 file changed, 41 insertions(+), 12 deletions(-) > > diff --git a/arch/powerpc/net/bpf_jit_comp32.c > b/arch/powerpc/net/bpf_jit_comp32.c > index e46ed1e8c6ca..28dc6a1a8f2f 100644 > --- a/arch/powerpc/net/bpf_jit_comp32.c > +++ b/arch/powerpc/net/bpf_jit_comp32.c > @@ -294,6 +294,7 @@ int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, > struct codegen_context * > u32 dst_reg_h = dst_reg - 1; > u32 src_reg = bpf_to_ppc(insn[i].src_reg); > u32 src_reg_h = src_reg - 1; > + u32 ax_reg = bpf_to_ppc(BPF_REG_AX); > u32 tmp_reg = bpf_to_ppc(TMP_REG); > u32 size = BPF_SIZE(code); > s16 off = insn[i].off; > @@ -798,25 +799,53 @@ int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, > struct codegen_context * >* BPF_STX ATOMIC (atomic ops) >*/ > case BPF_STX | BPF_ATOMIC | BPF_W: > - if (imm != BPF_ADD) { > - pr_err_ratelimited("eBPF filter atomic op code > %02x (@%d) unsupported\n", > -code, i); > - return -ENOTSUPP; > - } > - > - /* *(u32 *)(dst + off) += src */ > - > bpf_set_seen_register(ctx, tmp_reg); > + bpf_set_seen_register(ctx, ax_reg); > + > /* Get offset into TMP_REG */ > EMIT(PPC_RAW_LI(tmp_reg, off)); > + tmp_idx = ctx->idx * 4; > /* load value from memory into r0 */ > EMIT(PPC_RAW_LWARX(_R0, tmp_reg, dst_reg, 0)); > - /* add value from src_reg into this */ > - EMIT(PPC_RAW_ADD(_R0, _R0, src_reg)); > - /* store result back */ > + > + /* Save old value in BPF_REG_AX */ > + if (imm & BPF_FETCH) > + EMIT(PPC_RAW_MR(ax_reg, _R0)); > + > + switch (imm) { > + case BPF_ADD: > + case BPF_ADD | BPF_FETCH: > + EMIT(PPC_RAW_ADD(_R0, _R0, src_reg)); > + break; > + case BPF_AND: > + case BPF_AND | BPF_FETCH: > + EMIT(PPC_RAW_AND(_R0, _R0, src_reg)); > + break; > + case BPF_OR: > + case BPF_OR | BPF_FETCH: > + EMIT(PPC_RAW_OR(_R0, _R0, src_reg)); > + break; > + case BPF_XOR: > + case BPF_XOR | BPF_FETCH: > + EMIT(PPC_RAW_XOR(_R0, _R0, src_reg)); > + break; > + default: > + pr_err_ratelimited("eBPF filter atomic op code > %02x (@%d) unsupported\n", > +code, i); > + return -EOPNOTSUPP; > + } > + > + /* store new value */ > EMIT(PPC_RAW_STWCX(_R0, tmp_reg, dst_reg)); > /* we're done if this succeeded */ > - PPC_BCC_SHORT(COND_NE, (ctx->idx - 3) * 4); > + PPC_BCC_SHORT(COND_NE, tmp_idx); > + > + /* For the BPF_FETCH variant, get old data into src_reg > */ > + if (imm & BPF_FETCH) { > + EMIT(PPC_RAW_MR(src_reg, ax_reg)); > + if (!fp->aux->verifier_zext) > + EMIT(PPC_RAW_LI(src_reg_h, 0)); > + } > break; > > case BPF_STX | BPF_ATOMIC | BPF_DW: /* *(u64 *)(dst + off) += > src */
[PATCH 2/2] powerpc: Move prom_init() out of asm-prototypes.h
This is the end of the work started with commit 76222808fc25 ("powerpc: Move C prototypes out of asm-prototypes.h") Now that asm/machdep.h doesn't include asm/setup.h anymore, there are no conflicts anymore with the function prom_init() defined in drivers/gpu/drm/nouveau/nvkm/subdev/bios/shadowrom.o So we can move it to asm/setup.h Signed-off-by: Christophe Leroy --- arch/powerpc/include/asm/asm-prototypes.h | 6 -- arch/powerpc/include/asm/setup.h | 5 + 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/arch/powerpc/include/asm/asm-prototypes.h b/arch/powerpc/include/asm/asm-prototypes.h index d995c65d18ab..3b2bbc273055 100644 --- a/arch/powerpc/include/asm/asm-prototypes.h +++ b/arch/powerpc/include/asm/asm-prototypes.h @@ -34,12 +34,6 @@ int64_t __opal_call(int64_t a0, int64_t a1, int64_t a2, int64_t a3, int64_t a4, int64_t a5, int64_t a6, int64_t a7, int64_t opcode, uint64_t msr); -/* prom_init (OpenFirmware) */ -unsigned long __init prom_init(unsigned long r3, unsigned long r4, - unsigned long pp, - unsigned long r6, unsigned long r7, - unsigned long kbase); - /* misc runtime */ extern u64 __bswapdi2(u64); extern s64 __lshrdi3(s64, int); diff --git a/arch/powerpc/include/asm/setup.h b/arch/powerpc/include/asm/setup.h index 8fa37ef5da4d..14a0b8af738e 100644 --- a/arch/powerpc/include/asm/setup.h +++ b/arch/powerpc/include/asm/setup.h @@ -85,6 +85,11 @@ void __init machine_init(u64 dt_ptr); void __init early_setup(unsigned long dt_ptr); void early_setup_secondary(void); +/* prom_init (OpenFirmware) */ +unsigned long __init prom_init(unsigned long r3, unsigned long r4, + unsigned long pp, unsigned long r6, + unsigned long r7, unsigned long kbase); + #endif /* !__ASSEMBLY__ */ #endif /* _ASM_POWERPC_SETUP_H */ -- 2.35.3
[PATCH 1/2] powerpc: Don't include asm/setup.h in asm/machdep.h
asm/machdep.h doesn't need asm/setup.h Remove it. Add it directly in files that needs it. Signed-off-by: Christophe Leroy --- arch/powerpc/include/asm/machdep.h | 2 -- arch/powerpc/kernel/pci-common.c | 1 + arch/powerpc/kernel/prom.c | 2 +- arch/powerpc/kernel/prom_init.c| 2 +- arch/powerpc/kexec/core.c | 1 + arch/powerpc/kvm/powerpc.c | 1 + arch/powerpc/mm/mem.c | 1 + arch/powerpc/platforms/pseries/kexec.c | 2 +- arch/powerpc/platforms/pseries/lpar.c | 2 +- arch/powerpc/platforms/pseries/setup.c | 1 + arch/powerpc/sysdev/fsl_pci.c | 1 + drivers/scsi/mesh.c| 2 +- 12 files changed, 11 insertions(+), 7 deletions(-) diff --git a/arch/powerpc/include/asm/machdep.h b/arch/powerpc/include/asm/machdep.h index 358d171ae8e0..613755afa8a9 100644 --- a/arch/powerpc/include/asm/machdep.h +++ b/arch/powerpc/include/asm/machdep.h @@ -8,8 +8,6 @@ #include #include -#include - struct pt_regs; struct pci_bus; struct device_node; diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c index 068410cd54a3..c87999289752 100644 --- a/arch/powerpc/kernel/pci-common.c +++ b/arch/powerpc/kernel/pci-common.c @@ -39,6 +39,7 @@ #include #include #include +#include #include "../../../drivers/pci/pci.h" diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c index feae8509b59c..1066b072db35 100644 --- a/arch/powerpc/kernel/prom.c +++ b/arch/powerpc/kernel/prom.c @@ -44,7 +44,7 @@ #include #include #include -#include +#include #include #include #include diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c index 04694ec423f6..1939683e35ed 100644 --- a/arch/powerpc/kernel/prom_init.c +++ b/arch/powerpc/kernel/prom_init.c @@ -42,7 +42,7 @@ #include #include #include -#include +#include #include #include diff --git a/arch/powerpc/kexec/core.c b/arch/powerpc/kexec/core.c index 7ab4980fe13a..9773dd0640f3 100644 --- a/arch/powerpc/kexec/core.c +++ b/arch/powerpc/kexec/core.c @@ -19,6 +19,7 @@ #include #include #include +#include void machine_kexec_mask_interrupts(void) { unsigned int i; diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c index 191992fcb2c2..fb1490761c87 100644 --- a/arch/powerpc/kvm/powerpc.c +++ b/arch/powerpc/kvm/powerpc.c @@ -33,6 +33,7 @@ #include #endif #include +#include #include "timing.h" #include "irq.h" diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c index 52b77684acda..2cf6748755e1 100644 --- a/arch/powerpc/mm/mem.c +++ b/arch/powerpc/mm/mem.c @@ -25,6 +25,7 @@ #include #include #include +#include #include diff --git a/arch/powerpc/platforms/pseries/kexec.c b/arch/powerpc/platforms/pseries/kexec.c index ab6cdbebb35e..096d09ed89f6 100644 --- a/arch/powerpc/platforms/pseries/kexec.c +++ b/arch/powerpc/platforms/pseries/kexec.c @@ -6,7 +6,7 @@ #include #include -#include +#include #include #include #include diff --git a/arch/powerpc/platforms/pseries/lpar.c b/arch/powerpc/platforms/pseries/lpar.c index 937f9c010b22..e6c117fb6491 100644 --- a/arch/powerpc/platforms/pseries/lpar.c +++ b/arch/powerpc/platforms/pseries/lpar.c @@ -27,7 +27,7 @@ #include #include #include -#include +#include #include #include #include diff --git a/arch/powerpc/platforms/pseries/setup.c b/arch/powerpc/platforms/pseries/setup.c index afb074269b42..e86a749173e6 100644 --- a/arch/powerpc/platforms/pseries/setup.c +++ b/arch/powerpc/platforms/pseries/setup.c @@ -72,6 +72,7 @@ #include #include #include +#include #include "pseries.h" diff --git a/arch/powerpc/sysdev/fsl_pci.c b/arch/powerpc/sysdev/fsl_pci.c index 1011cfea2e32..e8d072e98b66 100644 --- a/arch/powerpc/sysdev/fsl_pci.c +++ b/arch/powerpc/sysdev/fsl_pci.c @@ -38,6 +38,7 @@ #include #include #include +#include #include #include diff --git a/drivers/scsi/mesh.c b/drivers/scsi/mesh.c index 322d3ad38159..a74f0c2c8ba7 100644 --- a/drivers/scsi/mesh.c +++ b/drivers/scsi/mesh.c @@ -38,7 +38,7 @@ #include #include #include -#include +#include #include #include -- 2.35.3
[PATCH 2/4] scsi: cxlflash: Include missing linux/irqdomain.h
powerpc's asm/prom.h brings some headers that it doesn't need itself. Once those headers are removed from asm/prom.h, the following errors occur: CC [M] drivers/scsi/cxlflash/ocxl_hw.o drivers/scsi/cxlflash/ocxl_hw.c: In function 'afu_map_irq': drivers/scsi/cxlflash/ocxl_hw.c:195:16: error: implicit declaration of function 'irq_create_mapping' [-Werror=implicit-function-declaration] 195 | virq = irq_create_mapping(NULL, irq->hwirq); |^~ drivers/scsi/cxlflash/ocxl_hw.c:222:9: error: implicit declaration of function 'irq_dispose_mapping' [-Werror=implicit-function-declaration] 222 | irq_dispose_mapping(virq); | ^~~ drivers/scsi/cxlflash/ocxl_hw.c: In function 'afu_unmap_irq': drivers/scsi/cxlflash/ocxl_hw.c:264:13: error: implicit declaration of function 'irq_find_mapping'; did you mean 'is_cow_mapping'? [-Werror=implicit-function-declaration] 264 | if (irq_find_mapping(NULL, irq->hwirq)) { | ^~~~ | is_cow_mapping cc1: some warnings being treated as errors Fix it by including linux/irqdomain.h Signed-off-by: Christophe Leroy --- drivers/scsi/cxlflash/ocxl_hw.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/scsi/cxlflash/ocxl_hw.c b/drivers/scsi/cxlflash/ocxl_hw.c index 244fc27215dc..631eda2d467e 100644 --- a/drivers/scsi/cxlflash/ocxl_hw.c +++ b/drivers/scsi/cxlflash/ocxl_hw.c @@ -16,6 +16,7 @@ #include #include #include +#include #include #include -- 2.35.3
[PATCH 1/4] video: fbdev: offb: Include missing linux/platform_device.h
A lot of drivers were getting platform and of headers indirectly via headers like asm/pci.h or asm/prom.h Most of them were fixed during 5.19 cycle but a newissue was introduced by commit 52b1b46c39ae ("of: Create platform devices for OF framebuffers") Include missing platform_device.h to allow cleaning asm/pci.h Cc: Thomas Zimmermann Fixes: 52b1b46c39ae ("of: Create platform devices for OF framebuffers") Signed-off-by: Christophe Leroy --- drivers/video/fbdev/offb.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/video/fbdev/offb.c b/drivers/video/fbdev/offb.c index b1acb1ebebe9..91001990e351 100644 --- a/drivers/video/fbdev/offb.c +++ b/drivers/video/fbdev/offb.c @@ -26,6 +26,7 @@ #include #include #include +#include #include #ifdef CONFIG_PPC32 -- 2.35.3
[PATCH 4/4] powerpc: Finally remove unnecessary headers from asm/prom.h
Remove all headers included from asm/prom.h which are not used by asm/prom.h itself. Declare struct device_node and struct property locally to avoid including of.h Signed-off-by: Christophe Leroy --- arch/powerpc/include/asm/prom.h | 12 +++- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/arch/powerpc/include/asm/prom.h b/arch/powerpc/include/asm/prom.h index 5c80152e8f18..93f112133934 100644 --- a/arch/powerpc/include/asm/prom.h +++ b/arch/powerpc/include/asm/prom.h @@ -12,15 +12,9 @@ * Updates for PPC64 by Peter Bergner & David Engebretsen, IBM Corp. */ #include -#include -#include - -/* These includes should be removed once implicit includes are cleaned up. */ -#include -#include -#include -#include -#include + +struct device_node; +struct property; #define OF_DT_BEGIN_NODE 0x1 /* Start of node, full name */ #define OF_DT_END_NODE 0x2 /* End node */ -- 2.35.3
[PATCH 3/4] powerpc: Remove asm/prom.h from asm/mpc52xx.h and asm/pci.h
asm/pci.h and asm/mpc52xx.h don't need asm/prom.h Declare struct device_node locally to avoid including of.h Signed-off-by: Christophe Leroy --- arch/powerpc/include/asm/mpc52xx.h | 3 ++- arch/powerpc/include/asm/pci.h | 1 - 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/powerpc/include/asm/mpc52xx.h b/arch/powerpc/include/asm/mpc52xx.h index ce1e0aabaa64..f721a5c90e20 100644 --- a/arch/powerpc/include/asm/mpc52xx.h +++ b/arch/powerpc/include/asm/mpc52xx.h @@ -15,7 +15,6 @@ #ifndef __ASSEMBLY__ #include -#include #include #endif /* __ASSEMBLY__ */ @@ -268,6 +267,8 @@ struct mpc52xx_intr { #ifndef __ASSEMBLY__ +struct device_node; + /* mpc52xx_common.c */ extern void mpc5200_setup_xlb_arbiter(void); extern void mpc52xx_declare_of_platform_devices(void); diff --git a/arch/powerpc/include/asm/pci.h b/arch/powerpc/include/asm/pci.h index 915d6ee4b40a..0f182074cdb7 100644 --- a/arch/powerpc/include/asm/pci.h +++ b/arch/powerpc/include/asm/pci.h @@ -14,7 +14,6 @@ #include #include -#include #include /* Return values for pci_controller_ops.probe_mode function */ -- 2.35.3
Re: [PATCH v3 3/3] powerpc/pseries: wire up rng during setup_arch
Le 11/06/2022 à 17:10, Jason A. Donenfeld a écrit : > The platform's RNG must be available before random_init() in order to be > useful for initial seeding, which in turn means that it needs to be > called from setup_arch(), rather than from an init call. Fortunately, > each platform already has a setup_arch function pointer, which means > it's easy to wire this up. This commit also removes some noisy log > messages that don't add much. > > Cc: sta...@vger.kernel.org > Cc: Michael Ellerman > Cc: Christophe Leroy > Fixes: a489043f4626 ("powerpc/pseries: Implement arch_get_random_long() based > on H_RANDOM") > Signed-off-by: Jason A. Donenfeld Reviewed-by: Christophe Leroy > --- > arch/powerpc/platforms/pseries/pseries.h | 2 ++ > arch/powerpc/platforms/pseries/rng.c | 11 +++ > arch/powerpc/platforms/pseries/setup.c | 1 + > 3 files changed, 6 insertions(+), 8 deletions(-) > > diff --git a/arch/powerpc/platforms/pseries/pseries.h > b/arch/powerpc/platforms/pseries/pseries.h > index f5c916c839c9..1d75b7742ef0 100644 > --- a/arch/powerpc/platforms/pseries/pseries.h > +++ b/arch/powerpc/platforms/pseries/pseries.h > @@ -122,4 +122,6 @@ void pseries_lpar_read_hblkrm_characteristics(void); > static inline void pseries_lpar_read_hblkrm_characteristics(void) { } > #endif > > +void pseries_rng_init(void); > + > #endif /* _PSERIES_PSERIES_H */ > diff --git a/arch/powerpc/platforms/pseries/rng.c > b/arch/powerpc/platforms/pseries/rng.c > index 6268545947b8..6ddfdeaace9e 100644 > --- a/arch/powerpc/platforms/pseries/rng.c > +++ b/arch/powerpc/platforms/pseries/rng.c > @@ -10,6 +10,7 @@ > #include > #include > #include > +#include "pseries.h" > > > static int pseries_get_random_long(unsigned long *v) > @@ -24,19 +25,13 @@ static int pseries_get_random_long(unsigned long *v) > return 0; > } > > -static __init int rng_init(void) > +void __init pseries_rng_init(void) > { > struct device_node *dn; > > dn = of_find_compatible_node(NULL, NULL, "ibm,random"); > if (!dn) > - return -ENODEV; > - > - pr_info("Registering arch random hook.\n"); > - > + return; > ppc_md.get_random_seed = pseries_get_random_long; > - > of_node_put(dn); > - return 0; > } > -machine_subsys_initcall(pseries, rng_init); > diff --git a/arch/powerpc/platforms/pseries/setup.c > b/arch/powerpc/platforms/pseries/setup.c > index afb074269b42..ee4f1db49515 100644 > --- a/arch/powerpc/platforms/pseries/setup.c > +++ b/arch/powerpc/platforms/pseries/setup.c > @@ -839,6 +839,7 @@ static void __init pSeries_setup_arch(void) > } > > ppc_md.pcibios_root_bridge_prepare = pseries_root_bridge_prepare; > + pseries_rng_init(); > } > > static void pseries_panic(char *str)
Re: [PATCH v3 2/3] powerpc/powernv: wire up rng during setup_arch
Le 11/06/2022 à 17:10, Jason A. Donenfeld a écrit : > The platform's RNG must be available before random_init() in order to be > useful for initial seeding, which in turn means that it needs to be > called from setup_arch(), rather than from an init call. Fortunately, > each platform already has a setup_arch function pointer, which means > it's easy to wire this up. This commit also removes some noisy log > messages that don't add much. > > Cc: sta...@vger.kernel.org > Cc: Michael Ellerman > Cc: Christophe Leroy > Fixes: a4da0d50b2a0 ("powerpc: Implement arch_get_random_long/int() for > powernv") > Signed-off-by: Jason A. Donenfeld Reviewed-by: Christophe Leroy > --- > arch/powerpc/platforms/powernv/powernv.h | 2 ++ > arch/powerpc/platforms/powernv/rng.c | 18 +- > arch/powerpc/platforms/powernv/setup.c | 2 ++ > 3 files changed, 9 insertions(+), 13 deletions(-) > > diff --git a/arch/powerpc/platforms/powernv/powernv.h > b/arch/powerpc/platforms/powernv/powernv.h > index e297bf4abfcb..fd3f5e1eb10b 100644 > --- a/arch/powerpc/platforms/powernv/powernv.h > +++ b/arch/powerpc/platforms/powernv/powernv.h > @@ -42,4 +42,6 @@ ssize_t memcons_copy(struct memcons *mc, char *to, loff_t > pos, size_t count); > u32 __init memcons_get_size(struct memcons *mc); > struct memcons *__init memcons_init(struct device_node *node, const char > *mc_prop_name); > > +void powernv_rng_init(void); > + > #endif /* _POWERNV_H */ > diff --git a/arch/powerpc/platforms/powernv/rng.c > b/arch/powerpc/platforms/powernv/rng.c > index e3d44b36ae98..c86bf097e407 100644 > --- a/arch/powerpc/platforms/powernv/rng.c > +++ b/arch/powerpc/platforms/powernv/rng.c > @@ -17,6 +17,7 @@ > #include > #include > #include > +#include "powernv.h" > > #define DARN_ERR 0xul > > @@ -84,24 +85,20 @@ static int powernv_get_random_darn(unsigned long *v) > return 1; > } > > -static int __init initialise_darn(void) > +static void __init initialise_darn(void) > { > unsigned long val; > int i; > > if (!cpu_has_feature(CPU_FTR_ARCH_300)) > - return -ENODEV; > + return; > > for (i = 0; i < 10; i++) { > if (powernv_get_random_darn(&val)) { > ppc_md.get_random_seed = powernv_get_random_darn; > - return 0; > + return; > } > } > - > - pr_warn("Unable to use DARN for get_random_seed()\n"); > - > - return -EIO; > } > > int powernv_get_random_long(unsigned long *v) > @@ -163,14 +160,12 @@ static __init int rng_create(struct device_node *dn) > > rng_init_per_cpu(rng, dn); > > - pr_info_once("Registering arch random hook.\n"); > - > ppc_md.get_random_seed = powernv_get_random_long; > > return 0; > } > > -static __init int rng_init(void) > +void __init powernv_rng_init(void) > { > struct device_node *dn; > int rc; > @@ -188,7 +183,4 @@ static __init int rng_init(void) > } > > initialise_darn(); > - > - return 0; > } > -machine_subsys_initcall(powernv, rng_init); > diff --git a/arch/powerpc/platforms/powernv/setup.c > b/arch/powerpc/platforms/powernv/setup.c > index 824c3ad7a0fa..a5fcb6796b22 100644 > --- a/arch/powerpc/platforms/powernv/setup.c > +++ b/arch/powerpc/platforms/powernv/setup.c > @@ -203,6 +203,8 @@ static void __init pnv_setup_arch(void) > pnv_check_guarded_cores(); > > /* XXX PMCS */ > + > + powernv_rng_init(); > } > > static void __init pnv_init(void)
Re: [PATCH v3 1/3] powerpc/microwatt: wire up rng during setup_arch
Le 11/06/2022 à 17:10, Jason A. Donenfeld a écrit : > The platform's RNG must be available before random_init() in order to be > useful for initial seeding, which in turn means that it needs to be > called from setup_arch(), rather than from an init call. Fortunately, > each platform already has a setup_arch function pointer, which means > it's easy to wire this up. This commit also removes some noisy log > messages that don't add much. > > Cc: sta...@vger.kernel.org > Cc: Michael Ellerman > Cc: Christophe Leroy > Fixes: c25769fddaec ("powerpc/microwatt: Add support for hardware random > number generator") > Signed-off-by: Jason A. Donenfeld Reviewed-by: Christophe Leroy > --- > arch/powerpc/platforms/microwatt/microwatt.h | 7 +++ > arch/powerpc/platforms/microwatt/rng.c | 10 +++--- > arch/powerpc/platforms/microwatt/setup.c | 8 > 3 files changed, 18 insertions(+), 7 deletions(-) > create mode 100644 arch/powerpc/platforms/microwatt/microwatt.h > > diff --git a/arch/powerpc/platforms/microwatt/microwatt.h > b/arch/powerpc/platforms/microwatt/microwatt.h > new file mode 100644 > index ..335417e95e66 > --- /dev/null > +++ b/arch/powerpc/platforms/microwatt/microwatt.h > @@ -0,0 +1,7 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +#ifndef _MICROWATT_H > +#define _MICROWATT_H > + > +void microwatt_rng_init(void); > + > +#endif /* _MICROWATT_H */ > diff --git a/arch/powerpc/platforms/microwatt/rng.c > b/arch/powerpc/platforms/microwatt/rng.c > index 7bc4d1cbfaf0..8ece87d005c8 100644 > --- a/arch/powerpc/platforms/microwatt/rng.c > +++ b/arch/powerpc/platforms/microwatt/rng.c > @@ -11,6 +11,7 @@ > #include > #include > #include > +#include "microwatt.h" > > #define DARN_ERR 0xul > > @@ -29,7 +30,7 @@ static int microwatt_get_random_darn(unsigned long *v) > return 1; > } > > -static __init int rng_init(void) > +void __init microwatt_rng_init(void) > { > unsigned long val; > int i; > @@ -37,12 +38,7 @@ static __init int rng_init(void) > for (i = 0; i < 10; i++) { > if (microwatt_get_random_darn(&val)) { > ppc_md.get_random_seed = microwatt_get_random_darn; > - return 0; > + return; > } > } > - > - pr_warn("Unable to use DARN for get_random_seed()\n"); > - > - return -EIO; > } > -machine_subsys_initcall(, rng_init); > diff --git a/arch/powerpc/platforms/microwatt/setup.c > b/arch/powerpc/platforms/microwatt/setup.c > index 0b02603bdb74..6b32539395a4 100644 > --- a/arch/powerpc/platforms/microwatt/setup.c > +++ b/arch/powerpc/platforms/microwatt/setup.c > @@ -16,6 +16,8 @@ > #include > #include > > +#include "microwatt.h" > + > static void __init microwatt_init_IRQ(void) > { > xics_init(); > @@ -32,10 +34,16 @@ static int __init microwatt_populate(void) > } > machine_arch_initcall(microwatt, microwatt_populate); > > +static void __init microwatt_setup_arch(void) > +{ > + microwatt_rng_init(); > +} > + > define_machine(microwatt) { > .name = "microwatt", > .probe = microwatt_probe, > .init_IRQ = microwatt_init_IRQ, > + .setup_arch = microwatt_setup_arch, > .progress = udbg_progress, > .calibrate_decr = generic_calibrate_decr, > };
[PATCH v3 3/3] powerpc/pseries: wire up rng during setup_arch
The platform's RNG must be available before random_init() in order to be useful for initial seeding, which in turn means that it needs to be called from setup_arch(), rather than from an init call. Fortunately, each platform already has a setup_arch function pointer, which means it's easy to wire this up. This commit also removes some noisy log messages that don't add much. Cc: sta...@vger.kernel.org Cc: Michael Ellerman Cc: Christophe Leroy Fixes: a489043f4626 ("powerpc/pseries: Implement arch_get_random_long() based on H_RANDOM") Signed-off-by: Jason A. Donenfeld --- arch/powerpc/platforms/pseries/pseries.h | 2 ++ arch/powerpc/platforms/pseries/rng.c | 11 +++ arch/powerpc/platforms/pseries/setup.c | 1 + 3 files changed, 6 insertions(+), 8 deletions(-) diff --git a/arch/powerpc/platforms/pseries/pseries.h b/arch/powerpc/platforms/pseries/pseries.h index f5c916c839c9..1d75b7742ef0 100644 --- a/arch/powerpc/platforms/pseries/pseries.h +++ b/arch/powerpc/platforms/pseries/pseries.h @@ -122,4 +122,6 @@ void pseries_lpar_read_hblkrm_characteristics(void); static inline void pseries_lpar_read_hblkrm_characteristics(void) { } #endif +void pseries_rng_init(void); + #endif /* _PSERIES_PSERIES_H */ diff --git a/arch/powerpc/platforms/pseries/rng.c b/arch/powerpc/platforms/pseries/rng.c index 6268545947b8..6ddfdeaace9e 100644 --- a/arch/powerpc/platforms/pseries/rng.c +++ b/arch/powerpc/platforms/pseries/rng.c @@ -10,6 +10,7 @@ #include #include #include +#include "pseries.h" static int pseries_get_random_long(unsigned long *v) @@ -24,19 +25,13 @@ static int pseries_get_random_long(unsigned long *v) return 0; } -static __init int rng_init(void) +void __init pseries_rng_init(void) { struct device_node *dn; dn = of_find_compatible_node(NULL, NULL, "ibm,random"); if (!dn) - return -ENODEV; - - pr_info("Registering arch random hook.\n"); - + return; ppc_md.get_random_seed = pseries_get_random_long; - of_node_put(dn); - return 0; } -machine_subsys_initcall(pseries, rng_init); diff --git a/arch/powerpc/platforms/pseries/setup.c b/arch/powerpc/platforms/pseries/setup.c index afb074269b42..ee4f1db49515 100644 --- a/arch/powerpc/platforms/pseries/setup.c +++ b/arch/powerpc/platforms/pseries/setup.c @@ -839,6 +839,7 @@ static void __init pSeries_setup_arch(void) } ppc_md.pcibios_root_bridge_prepare = pseries_root_bridge_prepare; + pseries_rng_init(); } static void pseries_panic(char *str) -- 2.35.1
[PATCH v3 2/3] powerpc/powernv: wire up rng during setup_arch
The platform's RNG must be available before random_init() in order to be useful for initial seeding, which in turn means that it needs to be called from setup_arch(), rather than from an init call. Fortunately, each platform already has a setup_arch function pointer, which means it's easy to wire this up. This commit also removes some noisy log messages that don't add much. Cc: sta...@vger.kernel.org Cc: Michael Ellerman Cc: Christophe Leroy Fixes: a4da0d50b2a0 ("powerpc: Implement arch_get_random_long/int() for powernv") Signed-off-by: Jason A. Donenfeld --- arch/powerpc/platforms/powernv/powernv.h | 2 ++ arch/powerpc/platforms/powernv/rng.c | 18 +- arch/powerpc/platforms/powernv/setup.c | 2 ++ 3 files changed, 9 insertions(+), 13 deletions(-) diff --git a/arch/powerpc/platforms/powernv/powernv.h b/arch/powerpc/platforms/powernv/powernv.h index e297bf4abfcb..fd3f5e1eb10b 100644 --- a/arch/powerpc/platforms/powernv/powernv.h +++ b/arch/powerpc/platforms/powernv/powernv.h @@ -42,4 +42,6 @@ ssize_t memcons_copy(struct memcons *mc, char *to, loff_t pos, size_t count); u32 __init memcons_get_size(struct memcons *mc); struct memcons *__init memcons_init(struct device_node *node, const char *mc_prop_name); +void powernv_rng_init(void); + #endif /* _POWERNV_H */ diff --git a/arch/powerpc/platforms/powernv/rng.c b/arch/powerpc/platforms/powernv/rng.c index e3d44b36ae98..c86bf097e407 100644 --- a/arch/powerpc/platforms/powernv/rng.c +++ b/arch/powerpc/platforms/powernv/rng.c @@ -17,6 +17,7 @@ #include #include #include +#include "powernv.h" #define DARN_ERR 0xul @@ -84,24 +85,20 @@ static int powernv_get_random_darn(unsigned long *v) return 1; } -static int __init initialise_darn(void) +static void __init initialise_darn(void) { unsigned long val; int i; if (!cpu_has_feature(CPU_FTR_ARCH_300)) - return -ENODEV; + return; for (i = 0; i < 10; i++) { if (powernv_get_random_darn(&val)) { ppc_md.get_random_seed = powernv_get_random_darn; - return 0; + return; } } - - pr_warn("Unable to use DARN for get_random_seed()\n"); - - return -EIO; } int powernv_get_random_long(unsigned long *v) @@ -163,14 +160,12 @@ static __init int rng_create(struct device_node *dn) rng_init_per_cpu(rng, dn); - pr_info_once("Registering arch random hook.\n"); - ppc_md.get_random_seed = powernv_get_random_long; return 0; } -static __init int rng_init(void) +void __init powernv_rng_init(void) { struct device_node *dn; int rc; @@ -188,7 +183,4 @@ static __init int rng_init(void) } initialise_darn(); - - return 0; } -machine_subsys_initcall(powernv, rng_init); diff --git a/arch/powerpc/platforms/powernv/setup.c b/arch/powerpc/platforms/powernv/setup.c index 824c3ad7a0fa..a5fcb6796b22 100644 --- a/arch/powerpc/platforms/powernv/setup.c +++ b/arch/powerpc/platforms/powernv/setup.c @@ -203,6 +203,8 @@ static void __init pnv_setup_arch(void) pnv_check_guarded_cores(); /* XXX PMCS */ + + powernv_rng_init(); } static void __init pnv_init(void) -- 2.35.1
[PATCH v3 1/3] powerpc/microwatt: wire up rng during setup_arch
The platform's RNG must be available before random_init() in order to be useful for initial seeding, which in turn means that it needs to be called from setup_arch(), rather than from an init call. Fortunately, each platform already has a setup_arch function pointer, which means it's easy to wire this up. This commit also removes some noisy log messages that don't add much. Cc: sta...@vger.kernel.org Cc: Michael Ellerman Cc: Christophe Leroy Fixes: c25769fddaec ("powerpc/microwatt: Add support for hardware random number generator") Signed-off-by: Jason A. Donenfeld --- arch/powerpc/platforms/microwatt/microwatt.h | 7 +++ arch/powerpc/platforms/microwatt/rng.c | 10 +++--- arch/powerpc/platforms/microwatt/setup.c | 8 3 files changed, 18 insertions(+), 7 deletions(-) create mode 100644 arch/powerpc/platforms/microwatt/microwatt.h diff --git a/arch/powerpc/platforms/microwatt/microwatt.h b/arch/powerpc/platforms/microwatt/microwatt.h new file mode 100644 index ..335417e95e66 --- /dev/null +++ b/arch/powerpc/platforms/microwatt/microwatt.h @@ -0,0 +1,7 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef _MICROWATT_H +#define _MICROWATT_H + +void microwatt_rng_init(void); + +#endif /* _MICROWATT_H */ diff --git a/arch/powerpc/platforms/microwatt/rng.c b/arch/powerpc/platforms/microwatt/rng.c index 7bc4d1cbfaf0..8ece87d005c8 100644 --- a/arch/powerpc/platforms/microwatt/rng.c +++ b/arch/powerpc/platforms/microwatt/rng.c @@ -11,6 +11,7 @@ #include #include #include +#include "microwatt.h" #define DARN_ERR 0xul @@ -29,7 +30,7 @@ static int microwatt_get_random_darn(unsigned long *v) return 1; } -static __init int rng_init(void) +void __init microwatt_rng_init(void) { unsigned long val; int i; @@ -37,12 +38,7 @@ static __init int rng_init(void) for (i = 0; i < 10; i++) { if (microwatt_get_random_darn(&val)) { ppc_md.get_random_seed = microwatt_get_random_darn; - return 0; + return; } } - - pr_warn("Unable to use DARN for get_random_seed()\n"); - - return -EIO; } -machine_subsys_initcall(, rng_init); diff --git a/arch/powerpc/platforms/microwatt/setup.c b/arch/powerpc/platforms/microwatt/setup.c index 0b02603bdb74..6b32539395a4 100644 --- a/arch/powerpc/platforms/microwatt/setup.c +++ b/arch/powerpc/platforms/microwatt/setup.c @@ -16,6 +16,8 @@ #include #include +#include "microwatt.h" + static void __init microwatt_init_IRQ(void) { xics_init(); @@ -32,10 +34,16 @@ static int __init microwatt_populate(void) } machine_arch_initcall(microwatt, microwatt_populate); +static void __init microwatt_setup_arch(void) +{ + microwatt_rng_init(); +} + define_machine(microwatt) { .name = "microwatt", .probe = microwatt_probe, .init_IRQ = microwatt_init_IRQ, + .setup_arch = microwatt_setup_arch, .progress = udbg_progress, .calibrate_decr = generic_calibrate_decr, }; -- 2.35.1
[PATCH v3 0/3] powerpc: wire up rng during setup_arch
The platform's RNG must be available before random_init() in order to be useful for initial seeding, which in turn means that it needs to be called from setup_arch(), rather than from an init call. This series wires that up properly on the three platforms that currently initialize the RNG from the wrong place. Cc: Michael Ellerman Cc: Christophe Leroy Jason A. Donenfeld (3): powerpc/microwatt: wire up rng during setup_arch powerpc/powernv: wire up rng during setup_arch powerpc/pseries: wire up rng during setup_arch arch/powerpc/platforms/microwatt/microwatt.h | 7 +++ arch/powerpc/platforms/microwatt/rng.c | 10 +++--- arch/powerpc/platforms/microwatt/setup.c | 8 arch/powerpc/platforms/powernv/powernv.h | 2 ++ arch/powerpc/platforms/powernv/rng.c | 18 +- arch/powerpc/platforms/powernv/setup.c | 2 ++ arch/powerpc/platforms/pseries/pseries.h | 2 ++ arch/powerpc/platforms/pseries/rng.c | 11 +++ arch/powerpc/platforms/pseries/setup.c | 1 + 9 files changed, 33 insertions(+), 28 deletions(-) create mode 100644 arch/powerpc/platforms/microwatt/microwatt.h -- 2.35.1
Re: [PATCH v2 1/3] powerpc/microwatt: wire up rng during setup_arch
Hi Christophe, On Sat, Jun 11, 2022 at 4:49 PM Christophe Leroy wrote: > Then you have: > > arch/powerpc/platforms/powernv/powernv.h > arch/powerpc/platforms/pseries/pseries.h > > and you can add > > arch/powerpc/platforms/microwatt/microwatt.h Oh, terrific, thanks. I'll do that. Jason
Re: [PATCH v2 3/3] powerpc/pseries: wire up rng during setup_arch
Hi Christophe, On Sat, Jun 11, 2022 at 4:45 PM Christophe Leroy wrote: > > There must be a empty line between declarations and code. Ack. > Prototype has to go in a header file Already voiced disagreement about this in the other thread. > and should be pSeries maybe > allthough camelCase in throw up on. All the rng.c functions use pseries_ in lower case, so I'll stick with that, as that's where the function is defined. Jason
Re: [PATCH v2 1/3] powerpc/microwatt: wire up rng during setup_arch
Le 11/06/2022 à 16:46, Jason A. Donenfeld a écrit : > Hi again, > > On Sat, Jun 11, 2022 at 04:41:58PM +0200, Jason A. Donenfeld wrote: >> Hi Christophe, >> >> On Sat, Jun 11, 2022 at 4:40 PM Christophe Leroy >> wrote: +__init void microwatt_rng_init(void); >>> >>> This prototype should be declared in a header file, for instance asm/setup.h >> >> Alright. > > Actually, on second thought, I don't think this part is worth doing. > These are per-platform functions, not powerpc-wide. > Then you have: arch/powerpc/platforms/powernv/powernv.h arch/powerpc/platforms/pseries/pseries.h and you can add arch/powerpc/platforms/microwatt/microwatt.h
Re: [PATCH v2 2/3] powerpc/powernv: wire up rng during setup_arch
Hi Christophe, On Sat, Jun 11, 2022 at 4:42 PM Christophe Leroy wrote: > Same here, the prototype should go in a header file., should be 'void > __init' (and indeed the __init is pointless in the prototype, only > matters in the function definition). I'll change the order, but I don't see a good place for the prototype other than the .c file. It's not a big deal to keep it there. > > Maybe the name should be pnv_rng_init() like the setup arch below. All the rng.c files are powernv_ prefixed, not pnv_ prefixed. Jason
Re: [PATCH v2 1/3] powerpc/microwatt: wire up rng during setup_arch
Hi again, On Sat, Jun 11, 2022 at 04:41:58PM +0200, Jason A. Donenfeld wrote: > Hi Christophe, > > On Sat, Jun 11, 2022 at 4:40 PM Christophe Leroy > wrote: > > > > > > +__init void microwatt_rng_init(void); > > > > This prototype should be declared in a header file, for instance asm/setup.h > > Alright. Actually, on second thought, I don't think this part is worth doing. These are per-platform functions, not powerpc-wide. Jason
Re: [PATCH v2 3/3] powerpc/pseries: wire up rng during setup_arch
Le 11/06/2022 à 12:04, Jason A. Donenfeld a écrit : > The platform's RNG must be available before random_init() in order to be > useful for initial seeding, which in turn means that it needs to be > called from setup_arch(), rather than from an init call. Fortunately, > each platform already has a setup_arch function pointer, which means > it's easy to wire this up for each of the three platforms that have an > RNG. This commit also removes some noisy log messages that don't add > much. > > Cc: sta...@vger.kernel.org > Cc: Michael Ellerman > Cc: Christophe Leroy > Fixes: a489043f4626 ("powerpc/pseries: Implement arch_get_random_long() based > on H_RANDOM") > Signed-off-by: Jason A. Donenfeld > --- > arch/powerpc/platforms/pseries/rng.c | 11 ++- > arch/powerpc/platforms/pseries/setup.c | 3 +++ > 2 files changed, 5 insertions(+), 9 deletions(-) > > diff --git a/arch/powerpc/platforms/pseries/rng.c > b/arch/powerpc/platforms/pseries/rng.c > index 6268545947b8..d39bfce39aa1 100644 > --- a/arch/powerpc/platforms/pseries/rng.c > +++ b/arch/powerpc/platforms/pseries/rng.c > @@ -24,19 +24,12 @@ static int pseries_get_random_long(unsigned long *v) > return 0; > } > > -static __init int rng_init(void) > +__init void pseries_rng_init(void) > { > struct device_node *dn; > - There must be a empty line between declarations and code. > dn = of_find_compatible_node(NULL, NULL, "ibm,random"); > if (!dn) > - return -ENODEV; > - > - pr_info("Registering arch random hook.\n"); > - > + return; > ppc_md.get_random_seed = pseries_get_random_long; > - > of_node_put(dn); > - return 0; > } > -machine_subsys_initcall(pseries, rng_init); > diff --git a/arch/powerpc/platforms/pseries/setup.c > b/arch/powerpc/platforms/pseries/setup.c > index afb074269b42..7f3ee2658163 100644 > --- a/arch/powerpc/platforms/pseries/setup.c > +++ b/arch/powerpc/platforms/pseries/setup.c > @@ -779,6 +779,8 @@ static resource_size_t > pseries_pci_iov_resource_alignment(struct pci_dev *pdev, > } > #endif > > +__init void pseries_rng_init(void); > + Prototype has to go in a header file, and should be pSeries maybe allthough camelCase in throw up on. > static void __init pSeries_setup_arch(void) > { > set_arch_panic_timeout(10, ARCH_PANIC_TIMEOUT); > @@ -839,6 +841,7 @@ static void __init pSeries_setup_arch(void) > } > > ppc_md.pcibios_root_bridge_prepare = pseries_root_bridge_prepare; > + pseries_rng_init(); > } > > static void pseries_panic(char *str)
Re: [PATCH v2 2/3] powerpc/powernv: wire up rng during setup_arch
Le 11/06/2022 à 12:04, Jason A. Donenfeld a écrit : > The platform's RNG must be available before random_init() in order to be > useful for initial seeding, which in turn means that it needs to be > called from setup_arch(), rather than from an init call. Fortunately, > each platform already has a setup_arch function pointer, which means > it's easy to wire this up for each of the three platforms that have an > RNG. This commit also removes some noisy log messages that don't add > much. > > Cc: sta...@vger.kernel.org > Cc: Michael Ellerman > Cc: Christophe Leroy > Fixes: a4da0d50b2a0 ("powerpc: Implement arch_get_random_long/int() for > powernv") > Signed-off-by: Jason A. Donenfeld > --- > arch/powerpc/platforms/powernv/rng.c | 17 - > arch/powerpc/platforms/powernv/setup.c | 4 > 2 files changed, 8 insertions(+), 13 deletions(-) > > diff --git a/arch/powerpc/platforms/powernv/rng.c > b/arch/powerpc/platforms/powernv/rng.c > index e3d44b36ae98..ef24e72a1b69 100644 > --- a/arch/powerpc/platforms/powernv/rng.c > +++ b/arch/powerpc/platforms/powernv/rng.c > @@ -84,24 +84,20 @@ static int powernv_get_random_darn(unsigned long *v) > return 1; > } > > -static int __init initialise_darn(void) > +static void __init initialise_darn(void) > { > unsigned long val; > int i; > > if (!cpu_has_feature(CPU_FTR_ARCH_300)) > - return -ENODEV; > + return; > > for (i = 0; i < 10; i++) { > if (powernv_get_random_darn(&val)) { > ppc_md.get_random_seed = powernv_get_random_darn; > - return 0; > + return; > } > } > - > - pr_warn("Unable to use DARN for get_random_seed()\n"); > - > - return -EIO; > } > > int powernv_get_random_long(unsigned long *v) > @@ -163,14 +159,12 @@ static __init int rng_create(struct device_node *dn) > > rng_init_per_cpu(rng, dn); > > - pr_info_once("Registering arch random hook.\n"); > - > ppc_md.get_random_seed = powernv_get_random_long; > > return 0; > } > > -static __init int rng_init(void) > +__init void powernv_rng_init(void) > { > struct device_node *dn; > int rc; > @@ -188,7 +182,4 @@ static __init int rng_init(void) > } > > initialise_darn(); > - > - return 0; > } > -machine_subsys_initcall(powernv, rng_init); > diff --git a/arch/powerpc/platforms/powernv/setup.c > b/arch/powerpc/platforms/powernv/setup.c > index 824c3ad7a0fa..a0c5217bc5c0 100644 > --- a/arch/powerpc/platforms/powernv/setup.c > +++ b/arch/powerpc/platforms/powernv/setup.c > @@ -184,6 +184,8 @@ static void __init pnv_check_guarded_cores(void) > } > } > > +__init void powernv_rng_init(void); > + Same here, the prototype should go in a header file., should be 'void __init' (and indeed the __init is pointless in the prototype, only matters in the function definition). Maybe the name should be pnv_rng_init() like the setup arch below. > static void __init pnv_setup_arch(void) > { > set_arch_panic_timeout(10, ARCH_PANIC_TIMEOUT); > @@ -203,6 +205,8 @@ static void __init pnv_setup_arch(void) > pnv_check_guarded_cores(); > > /* XXX PMCS */ > + > + powernv_rng_init(); > } > > static void __init pnv_init(void)
Re: [PATCH v2 1/3] powerpc/microwatt: wire up rng during setup_arch
Hi Christophe, On Sat, Jun 11, 2022 at 4:40 PM Christophe Leroy wrote: > > > > +__init void microwatt_rng_init(void); > > This prototype should be declared in a header file, for instance asm/setup.h Alright. > And I think the __init keyword usually goes after the type, so should be > 'void __init'. Indeed I thought so too. It just wasn't like this before, but that doesn't mean I shouldn't fix it. v3 coming right up. Jason
Re: [PATCH v2 1/3] powerpc/microwatt: wire up rng during setup_arch
Le 11/06/2022 à 12:04, Jason A. Donenfeld a écrit : > The platform's RNG must be available before random_init() in order to be > useful for initial seeding, which in turn means that it needs to be > called from setup_arch(), rather than from an init call. Fortunately, > each platform already has a setup_arch function pointer, which means > it's easy to wire this up for each of the three platforms that have an > RNG. This commit also removes some noisy log messages that don't add > much. > > Cc: sta...@vger.kernel.org > Cc: Michael Ellerman > Cc: Christophe Leroy > Fixes: c25769fddaec ("powerpc/microwatt: Add support for hardware random > number generator") > Signed-off-by: Jason A. Donenfeld > --- > arch/powerpc/platforms/microwatt/rng.c | 9 ++--- > arch/powerpc/platforms/microwatt/setup.c | 8 > 2 files changed, 10 insertions(+), 7 deletions(-) > > diff --git a/arch/powerpc/platforms/microwatt/rng.c > b/arch/powerpc/platforms/microwatt/rng.c > index 7bc4d1cbfaf0..d13f656910ad 100644 > --- a/arch/powerpc/platforms/microwatt/rng.c > +++ b/arch/powerpc/platforms/microwatt/rng.c > @@ -29,7 +29,7 @@ static int microwatt_get_random_darn(unsigned long *v) > return 1; > } > > -static __init int rng_init(void) > +__init void microwatt_rng_init(void) > { > unsigned long val; > int i; > @@ -37,12 +37,7 @@ static __init int rng_init(void) > for (i = 0; i < 10; i++) { > if (microwatt_get_random_darn(&val)) { > ppc_md.get_random_seed = microwatt_get_random_darn; > - return 0; > + return; > } > } > - > - pr_warn("Unable to use DARN for get_random_seed()\n"); > - > - return -EIO; > } > -machine_subsys_initcall(, rng_init); > diff --git a/arch/powerpc/platforms/microwatt/setup.c > b/arch/powerpc/platforms/microwatt/setup.c > index 0b02603bdb74..23c996dcc870 100644 > --- a/arch/powerpc/platforms/microwatt/setup.c > +++ b/arch/powerpc/platforms/microwatt/setup.c > @@ -32,10 +32,18 @@ static int __init microwatt_populate(void) > } > machine_arch_initcall(microwatt, microwatt_populate); > > +__init void microwatt_rng_init(void); This prototype should be declared in a header file, for instance asm/setup.h checkpatch.pl returns the following warning: WARNING:AVOID_EXTERNS: externs should be avoided in .c files #59: FILE: arch/powerpc/platforms/microwatt/setup.c:35: +__init void microwatt_rng_init(void); And I think the __init keyword usually goes after the type, so should be 'void __init'. > + > +static void __init microwatt_setup_arch(void) > +{ > + microwatt_rng_init(); > +} > + > define_machine(microwatt) { > .name = "microwatt", > .probe = microwatt_probe, > .init_IRQ = microwatt_init_IRQ, > + .setup_arch = microwatt_setup_arch, > .progress = udbg_progress, > .calibrate_decr = generic_calibrate_decr, > };
[PATCH v4.19] powerpc/32: Fix overread/overwrite of thread_struct via ptrace
commit 8e127846fc97778a5e5c99bca1ce0bbc5ec9 upstream. The ptrace PEEKUSR/POKEUSR (aka PEEKUSER/POKEUSER) API allows a process to read/write registers of another process. To get/set a register, the API takes an index into an imaginary address space called the "USER area", where the registers of the process are laid out in some fashion. The kernel then maps that index to a particular register in its own data structures and gets/sets the value. The API only allows a single machine-word to be read/written at a time. So 4 bytes on 32-bit kernels and 8 bytes on 64-bit kernels. The way floating point registers (FPRs) are addressed is somewhat complicated, because double precision float values are 64-bit even on 32-bit CPUs. That means on 32-bit kernels each FPR occupies two word-sized locations in the USER area. On 64-bit kernels each FPR occupies one word-sized location in the USER area. Internally the kernel stores the FPRs in an array of u64s, or if VSX is enabled, an array of pairs of u64s where one half of each pair stores the FPR. Which half of the pair stores the FPR depends on the kernel's endianness. To handle the different layouts of the FPRs depending on VSX/no-VSX and big/little endian, the TS_FPR() macro was introduced. Unfortunately the TS_FPR() macro does not take into account the fact that the addressing of each FPR differs between 32-bit and 64-bit kernels. It just takes the index into the "USER area" passed from userspace and indexes into the fp_state.fpr array. On 32-bit there are 64 indexes that address FPRs, but only 32 entries in the fp_state.fpr array, meaning the user can read/write 256 bytes past the end of the array. Because the fp_state sits in the middle of the thread_struct there are various fields than can be overwritten, including some pointers. As such it may be exploitable. It has also been observed to cause systems to hang or otherwise misbehave when using gdbserver, and is probably the root cause of this report which could not be easily reproduced: https://lore.kernel.org/linuxppc-dev/dc38afe9-6b78-f3f5-666b-986939e40...@keymile.com/ Rather than trying to make the TS_FPR() macro even more complicated to fix the bug, or add more macros, instead add a special-case for 32-bit kernels. This is more obvious and hopefully avoids a similar bug happening again in future. Note that because 32-bit kernels never have VSX enabled the code doesn't need to consider TS_FPRWIDTH/OFFSET at all. Add a BUILD_BUG_ON() to ensure that 32-bit && VSX is never enabled. Fixes: 87fec0514f61 ("powerpc: PTRACE_PEEKUSR/PTRACE_POKEUSER of FPR registers in little endian builds") Cc: sta...@vger.kernel.org # v3.13+ Reported-by: Ariel Miculas Tested-by: Christophe Leroy Signed-off-by: Michael Ellerman Link: https://lore.kernel.org/r/20220609133245.573565-1-...@ellerman.id.au --- arch/powerpc/kernel/ptrace.c | 18 ++ 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c index e08b32ccf1d9..073117ba75fe 100644 --- a/arch/powerpc/kernel/ptrace.c +++ b/arch/powerpc/kernel/ptrace.c @@ -3007,8 +3007,13 @@ long arch_ptrace(struct task_struct *child, long request, flush_fp_to_thread(child); if (fpidx < (PT_FPSCR - PT_FPR0)) - memcpy(&tmp, &child->thread.TS_FPR(fpidx), - sizeof(long)); + if (IS_ENABLED(CONFIG_PPC32)) { + // On 32-bit the index we are passed refers to 32-bit words + tmp = ((u32 *)child->thread.fp_state.fpr)[fpidx]; + } else { + memcpy(&tmp, &child->thread.TS_FPR(fpidx), + sizeof(long)); + } else tmp = child->thread.fp_state.fpscr; } @@ -3040,8 +3045,13 @@ long arch_ptrace(struct task_struct *child, long request, flush_fp_to_thread(child); if (fpidx < (PT_FPSCR - PT_FPR0)) - memcpy(&child->thread.TS_FPR(fpidx), &data, - sizeof(long)); + if (IS_ENABLED(CONFIG_PPC32)) { + // On 32-bit the index we are passed refers to 32-bit words + ((u32 *)child->thread.fp_state.fpr)[fpidx] = data; + } else { + memcpy(&child->thread.TS_FPR(fpidx), &data, + sizeof(long)); + } else child->thread.fp_state.fpscr = data; ret = 0; -- 2.35.3
Re: [PATCH] powerpc/rng: wire up during setup_arch
Le 11/06/2022 à 11:58, Jason A. Donenfeld a écrit : > Hi Christophe, > > On Sat, Jun 11, 2022 at 09:27:43AM +, Christophe Leroy wrote: >> Le 11/06/2022 à 11:22, Jason A. Donenfeld a écrit : >>> Hi Christophe, >>> >>> On Sat, Jun 11, 2022 at 11:17:23AM +0200, Christophe Leroy wrote: Also, you copied stable. Should you add a Fixes: tag so that we know what it fixes ? >>> >>> I suppose the fixes tag would be whatever introduced those files in the >>> first place, so not all together useful. But if you want something, feel >>> free to append these when applying the commit: >>> >>> Fixes: a4da0d50b2a0 ("powerpc: Implement arch_get_random_long/int() for >>> powernv") >>> Fixes: a489043f4626 ("powerpc/pseries: Implement arch_get_random_long() >>> based on H_RANDOM") >>> Fixes: c25769fddaec ("powerpc/microwatt: Add support for hardware random >>> number generator") >>> >> >> Well it helps knowing on which stable version it applies. >> >> Maybe it would be cleaner to send three patches ? After all they look > > Sounds like irritating paperwork to me. It helps with application to stable. Two of the above commits are in v3.12, the other one appears in v5.13 So having the microwatt in a separate patch should ease. > >> like 3 independant changes with nothing in common at all. > > "Nothing in common"? I don't know about that. I mean no common file that needs to be changed for the three platforms, so it makes it easy. > > Anyway, sure, I'll do that and send a v2 series. > As they are independant it doesn't need to be a series at all. But that's fine if it is a series as well. Christophe
Re: [PATCH] powerpc/rng: wire up during setup_arch
Hey again, On Sat, Jun 11, 2022 at 11:58:23AM +0200, Jason A. Donenfeld wrote: > Anyway, sure, I'll do that and send a v2 series. This is now done here: https://lore.kernel.org/lkml/20220611100447.5066-1-ja...@zx2c4.com/ Jason
[PATCH v2 3/3] powerpc/pseries: wire up rng during setup_arch
The platform's RNG must be available before random_init() in order to be useful for initial seeding, which in turn means that it needs to be called from setup_arch(), rather than from an init call. Fortunately, each platform already has a setup_arch function pointer, which means it's easy to wire this up for each of the three platforms that have an RNG. This commit also removes some noisy log messages that don't add much. Cc: sta...@vger.kernel.org Cc: Michael Ellerman Cc: Christophe Leroy Fixes: a489043f4626 ("powerpc/pseries: Implement arch_get_random_long() based on H_RANDOM") Signed-off-by: Jason A. Donenfeld --- arch/powerpc/platforms/pseries/rng.c | 11 ++- arch/powerpc/platforms/pseries/setup.c | 3 +++ 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/arch/powerpc/platforms/pseries/rng.c b/arch/powerpc/platforms/pseries/rng.c index 6268545947b8..d39bfce39aa1 100644 --- a/arch/powerpc/platforms/pseries/rng.c +++ b/arch/powerpc/platforms/pseries/rng.c @@ -24,19 +24,12 @@ static int pseries_get_random_long(unsigned long *v) return 0; } -static __init int rng_init(void) +__init void pseries_rng_init(void) { struct device_node *dn; - dn = of_find_compatible_node(NULL, NULL, "ibm,random"); if (!dn) - return -ENODEV; - - pr_info("Registering arch random hook.\n"); - + return; ppc_md.get_random_seed = pseries_get_random_long; - of_node_put(dn); - return 0; } -machine_subsys_initcall(pseries, rng_init); diff --git a/arch/powerpc/platforms/pseries/setup.c b/arch/powerpc/platforms/pseries/setup.c index afb074269b42..7f3ee2658163 100644 --- a/arch/powerpc/platforms/pseries/setup.c +++ b/arch/powerpc/platforms/pseries/setup.c @@ -779,6 +779,8 @@ static resource_size_t pseries_pci_iov_resource_alignment(struct pci_dev *pdev, } #endif +__init void pseries_rng_init(void); + static void __init pSeries_setup_arch(void) { set_arch_panic_timeout(10, ARCH_PANIC_TIMEOUT); @@ -839,6 +841,7 @@ static void __init pSeries_setup_arch(void) } ppc_md.pcibios_root_bridge_prepare = pseries_root_bridge_prepare; + pseries_rng_init(); } static void pseries_panic(char *str) -- 2.35.1
[PATCH v2 2/3] powerpc/powernv: wire up rng during setup_arch
The platform's RNG must be available before random_init() in order to be useful for initial seeding, which in turn means that it needs to be called from setup_arch(), rather than from an init call. Fortunately, each platform already has a setup_arch function pointer, which means it's easy to wire this up for each of the three platforms that have an RNG. This commit also removes some noisy log messages that don't add much. Cc: sta...@vger.kernel.org Cc: Michael Ellerman Cc: Christophe Leroy Fixes: a4da0d50b2a0 ("powerpc: Implement arch_get_random_long/int() for powernv") Signed-off-by: Jason A. Donenfeld --- arch/powerpc/platforms/powernv/rng.c | 17 - arch/powerpc/platforms/powernv/setup.c | 4 2 files changed, 8 insertions(+), 13 deletions(-) diff --git a/arch/powerpc/platforms/powernv/rng.c b/arch/powerpc/platforms/powernv/rng.c index e3d44b36ae98..ef24e72a1b69 100644 --- a/arch/powerpc/platforms/powernv/rng.c +++ b/arch/powerpc/platforms/powernv/rng.c @@ -84,24 +84,20 @@ static int powernv_get_random_darn(unsigned long *v) return 1; } -static int __init initialise_darn(void) +static void __init initialise_darn(void) { unsigned long val; int i; if (!cpu_has_feature(CPU_FTR_ARCH_300)) - return -ENODEV; + return; for (i = 0; i < 10; i++) { if (powernv_get_random_darn(&val)) { ppc_md.get_random_seed = powernv_get_random_darn; - return 0; + return; } } - - pr_warn("Unable to use DARN for get_random_seed()\n"); - - return -EIO; } int powernv_get_random_long(unsigned long *v) @@ -163,14 +159,12 @@ static __init int rng_create(struct device_node *dn) rng_init_per_cpu(rng, dn); - pr_info_once("Registering arch random hook.\n"); - ppc_md.get_random_seed = powernv_get_random_long; return 0; } -static __init int rng_init(void) +__init void powernv_rng_init(void) { struct device_node *dn; int rc; @@ -188,7 +182,4 @@ static __init int rng_init(void) } initialise_darn(); - - return 0; } -machine_subsys_initcall(powernv, rng_init); diff --git a/arch/powerpc/platforms/powernv/setup.c b/arch/powerpc/platforms/powernv/setup.c index 824c3ad7a0fa..a0c5217bc5c0 100644 --- a/arch/powerpc/platforms/powernv/setup.c +++ b/arch/powerpc/platforms/powernv/setup.c @@ -184,6 +184,8 @@ static void __init pnv_check_guarded_cores(void) } } +__init void powernv_rng_init(void); + static void __init pnv_setup_arch(void) { set_arch_panic_timeout(10, ARCH_PANIC_TIMEOUT); @@ -203,6 +205,8 @@ static void __init pnv_setup_arch(void) pnv_check_guarded_cores(); /* XXX PMCS */ + + powernv_rng_init(); } static void __init pnv_init(void) -- 2.35.1
[PATCH v2 1/3] powerpc/microwatt: wire up rng during setup_arch
The platform's RNG must be available before random_init() in order to be useful for initial seeding, which in turn means that it needs to be called from setup_arch(), rather than from an init call. Fortunately, each platform already has a setup_arch function pointer, which means it's easy to wire this up for each of the three platforms that have an RNG. This commit also removes some noisy log messages that don't add much. Cc: sta...@vger.kernel.org Cc: Michael Ellerman Cc: Christophe Leroy Fixes: c25769fddaec ("powerpc/microwatt: Add support for hardware random number generator") Signed-off-by: Jason A. Donenfeld --- arch/powerpc/platforms/microwatt/rng.c | 9 ++--- arch/powerpc/platforms/microwatt/setup.c | 8 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/arch/powerpc/platforms/microwatt/rng.c b/arch/powerpc/platforms/microwatt/rng.c index 7bc4d1cbfaf0..d13f656910ad 100644 --- a/arch/powerpc/platforms/microwatt/rng.c +++ b/arch/powerpc/platforms/microwatt/rng.c @@ -29,7 +29,7 @@ static int microwatt_get_random_darn(unsigned long *v) return 1; } -static __init int rng_init(void) +__init void microwatt_rng_init(void) { unsigned long val; int i; @@ -37,12 +37,7 @@ static __init int rng_init(void) for (i = 0; i < 10; i++) { if (microwatt_get_random_darn(&val)) { ppc_md.get_random_seed = microwatt_get_random_darn; - return 0; + return; } } - - pr_warn("Unable to use DARN for get_random_seed()\n"); - - return -EIO; } -machine_subsys_initcall(, rng_init); diff --git a/arch/powerpc/platforms/microwatt/setup.c b/arch/powerpc/platforms/microwatt/setup.c index 0b02603bdb74..23c996dcc870 100644 --- a/arch/powerpc/platforms/microwatt/setup.c +++ b/arch/powerpc/platforms/microwatt/setup.c @@ -32,10 +32,18 @@ static int __init microwatt_populate(void) } machine_arch_initcall(microwatt, microwatt_populate); +__init void microwatt_rng_init(void); + +static void __init microwatt_setup_arch(void) +{ + microwatt_rng_init(); +} + define_machine(microwatt) { .name = "microwatt", .probe = microwatt_probe, .init_IRQ = microwatt_init_IRQ, + .setup_arch = microwatt_setup_arch, .progress = udbg_progress, .calibrate_decr = generic_calibrate_decr, }; -- 2.35.1
[PATCH v2 0/3] powerpc: wire up rng during setup_arch
The platform's RNG must be available before random_init() in order to be useful for initial seeding, which in turn means that it needs to be called from setup_arch(), rather than from an init call. This series wires that up properly on the three platforms that currently initialize the RNG from the wrong place. Jason A. Donenfeld (3): powerpc/microwatt: wire up rng during setup_arch powerpc/powernv: wire up rng during setup_arch powerpc/pseries: wire up rng during setup_arch arch/powerpc/platforms/microwatt/rng.c | 9 ++--- arch/powerpc/platforms/microwatt/setup.c | 8 arch/powerpc/platforms/powernv/rng.c | 17 - arch/powerpc/platforms/powernv/setup.c | 4 arch/powerpc/platforms/pseries/rng.c | 11 ++- arch/powerpc/platforms/pseries/setup.c | 3 +++ 6 files changed, 23 insertions(+), 29 deletions(-) -- 2.35.1
Re: [PATCH] powerpc/rng: wire up during setup_arch
Hi Christophe, On Sat, Jun 11, 2022 at 09:27:43AM +, Christophe Leroy wrote: > Le 11/06/2022 à 11:22, Jason A. Donenfeld a écrit : > > Hi Christophe, > > > > On Sat, Jun 11, 2022 at 11:17:23AM +0200, Christophe Leroy wrote: > >> Also, you copied stable. Should you add a Fixes: tag so that we know > >> what it fixes ? > > > > I suppose the fixes tag would be whatever introduced those files in the > > first place, so not all together useful. But if you want something, feel > > free to append these when applying the commit: > > > > Fixes: a4da0d50b2a0 ("powerpc: Implement arch_get_random_long/int() for > > powernv") > > Fixes: a489043f4626 ("powerpc/pseries: Implement arch_get_random_long() > > based on H_RANDOM") > > Fixes: c25769fddaec ("powerpc/microwatt: Add support for hardware random > > number generator") > > > > Well it helps knowing on which stable version it applies. > > Maybe it would be cleaner to send three patches ? After all they look Sounds like irritating paperwork to me. > like 3 independant changes with nothing in common at all. "Nothing in common"? I don't know about that. Anyway, sure, I'll do that and send a v2 series. Jason
Re: [PATCH] powerpc/rng: wire up during setup_arch
Le 11/06/2022 à 11:22, Jason A. Donenfeld a écrit : > Hi Christophe, > > On Sat, Jun 11, 2022 at 11:17:23AM +0200, Christophe Leroy wrote: >> Also, you copied stable. Should you add a Fixes: tag so that we know >> what it fixes ? > > I suppose the fixes tag would be whatever introduced those files in the > first place, so not all together useful. But if you want something, feel > free to append these when applying the commit: > > Fixes: a4da0d50b2a0 ("powerpc: Implement arch_get_random_long/int() for > powernv") > Fixes: a489043f4626 ("powerpc/pseries: Implement arch_get_random_long() based > on H_RANDOM") > Fixes: c25769fddaec ("powerpc/microwatt: Add support for hardware random > number generator") > Well it helps knowing on which stable version it applies. Maybe it would be cleaner to send three patches ? After all they look like 3 independant changes with nothing in common at all. Christophe
Re: [PATCH] powerpc/rng: wire up during setup_arch
Le 11/06/2022 à 11:20, Jason A. Donenfeld a écrit : > Hi Christophe, > > On Sat, Jun 11, 2022 at 09:16:24AM +, Christophe Leroy wrote: >> Le 11/06/2022 à 10:11, Jason A. Donenfeld a écrit : >>> The platform's RNG must be available before random_init() in order to be >>> useful for initial seeding, which in turn means that it needs to be >>> called from setup_arch(), rather than from an init call. Fortunately, >>> each platform already has a setup_arch function pointer, which means >>> it's easy to wire this up for each of the three platforms that have an >>> RNG. This commit also removes some noisy log messages that don't add >>> much. >> >> Can't we use one of the machine initcalls for that ? >> Like machine_early_initcall() or machine_arch_initcall() ? > > No, unfortunately. I tried this, and it's still too late. This must be > done in setup_arch(). Ok > >> Today it is using machine_subsys_initcall() and you didn't remove it. >> It means rng_init() will be called twice. Is that ok ? > > I did remove the calls to machine_subsys_initcall(). I just double > checked: > > zx2c4@thinkpad ~/Projects/random-linux/arch/powerpc $ rg > machine_subsys_initcall platforms/*/rng.c > zx2c4@thinkpad ~/Projects/random-linux/arch/powerpc $ Oops, I overlooked it, sorry. Christophe
Re: [PATCH] powerpc/rng: wire up during setup_arch
Hi Christophe, On Sat, Jun 11, 2022 at 11:17:23AM +0200, Christophe Leroy wrote: > Also, you copied stable. Should you add a Fixes: tag so that we know > what it fixes ? I suppose the fixes tag would be whatever introduced those files in the first place, so not all together useful. But if you want something, feel free to append these when applying the commit: Fixes: a4da0d50b2a0 ("powerpc: Implement arch_get_random_long/int() for powernv") Fixes: a489043f4626 ("powerpc/pseries: Implement arch_get_random_long() based on H_RANDOM") Fixes: c25769fddaec ("powerpc/microwatt: Add support for hardware random number generator") Jason
Re: [PATCH] powerpc/rng: wire up during setup_arch
Hi Christophe, On Sat, Jun 11, 2022 at 09:16:24AM +, Christophe Leroy wrote: > Le 11/06/2022 à 10:11, Jason A. Donenfeld a écrit : > > The platform's RNG must be available before random_init() in order to be > > useful for initial seeding, which in turn means that it needs to be > > called from setup_arch(), rather than from an init call. Fortunately, > > each platform already has a setup_arch function pointer, which means > > it's easy to wire this up for each of the three platforms that have an > > RNG. This commit also removes some noisy log messages that don't add > > much. > > Can't we use one of the machine initcalls for that ? > Like machine_early_initcall() or machine_arch_initcall() ? No, unfortunately. I tried this, and it's still too late. This must be done in setup_arch(). > Today it is using machine_subsys_initcall() and you didn't remove it. > It means rng_init() will be called twice. Is that ok ? I did remove the calls to machine_subsys_initcall(). I just double checked: zx2c4@thinkpad ~/Projects/random-linux/arch/powerpc $ rg machine_subsys_initcall platforms/*/rng.c zx2c4@thinkpad ~/Projects/random-linux/arch/powerpc $ Jason
Re: [PATCH] powerpc/rng: wire up during setup_arch
Le 11/06/2022 à 11:16, Christophe Leroy a écrit : Le 11/06/2022 à 10:11, Jason A. Donenfeld a écrit : The platform's RNG must be available before random_init() in order to be useful for initial seeding, which in turn means that it needs to be called from setup_arch(), rather than from an init call. Fortunately, each platform already has a setup_arch function pointer, which means it's easy to wire this up for each of the three platforms that have an RNG. This commit also removes some noisy log messages that don't add much. Can't we use one of the machine initcalls for that ? Like machine_early_initcall() or machine_arch_initcall() ? Today it is using machine_subsys_initcall() and you didn't remove it. It means rng_init() will be called twice. Is that ok ? Also, you copied stable. Should you add a Fixes: tag so that we know what it fixes ? Cc: sta...@vger.kernel.org Cc: Michael Ellerman Signed-off-by: Jason A. Donenfeld --- arch/powerpc/platforms/microwatt/rng.c | 9 ++--- arch/powerpc/platforms/microwatt/setup.c | 8 arch/powerpc/platforms/powernv/rng.c | 17 - arch/powerpc/platforms/powernv/setup.c | 4 arch/powerpc/platforms/pseries/rng.c | 11 ++- arch/powerpc/platforms/pseries/setup.c | 3 +++ 6 files changed, 23 insertions(+), 29 deletions(-) diff --git a/arch/powerpc/platforms/microwatt/rng.c b/arch/powerpc/platforms/microwatt/rng.c index 7bc4d1cbfaf0..d13f656910ad 100644 --- a/arch/powerpc/platforms/microwatt/rng.c +++ b/arch/powerpc/platforms/microwatt/rng.c @@ -29,7 +29,7 @@ static int microwatt_get_random_darn(unsigned long *v) return 1; } -static __init int rng_init(void) +__init void microwatt_rng_init(void) { unsigned long val; int i; @@ -37,12 +37,7 @@ static __init int rng_init(void) for (i = 0; i < 10; i++) { if (microwatt_get_random_darn(&val)) { ppc_md.get_random_seed = microwatt_get_random_darn; - return 0; + return; } } - - pr_warn("Unable to use DARN for get_random_seed()\n"); - - return -EIO; } -machine_subsys_initcall(, rng_init); diff --git a/arch/powerpc/platforms/microwatt/setup.c b/arch/powerpc/platforms/microwatt/setup.c index 0b02603bdb74..23c996dcc870 100644 --- a/arch/powerpc/platforms/microwatt/setup.c +++ b/arch/powerpc/platforms/microwatt/setup.c @@ -32,10 +32,18 @@ static int __init microwatt_populate(void) } machine_arch_initcall(microwatt, microwatt_populate); +__init void microwatt_rng_init(void); + +static void __init microwatt_setup_arch(void) +{ + microwatt_rng_init(); +} + define_machine(microwatt) { .name = "microwatt", .probe = microwatt_probe, .init_IRQ = microwatt_init_IRQ, + .setup_arch = microwatt_setup_arch, .progress = udbg_progress, .calibrate_decr = generic_calibrate_decr, }; diff --git a/arch/powerpc/platforms/powernv/rng.c b/arch/powerpc/platforms/powernv/rng.c index e3d44b36ae98..ef24e72a1b69 100644 --- a/arch/powerpc/platforms/powernv/rng.c +++ b/arch/powerpc/platforms/powernv/rng.c @@ -84,24 +84,20 @@ static int powernv_get_random_darn(unsigned long *v) return 1; } -static int __init initialise_darn(void) +static void __init initialise_darn(void) { unsigned long val; int i; if (!cpu_has_feature(CPU_FTR_ARCH_300)) - return -ENODEV; + return; for (i = 0; i < 10; i++) { if (powernv_get_random_darn(&val)) { ppc_md.get_random_seed = powernv_get_random_darn; - return 0; + return; } } - - pr_warn("Unable to use DARN for get_random_seed()\n"); - - return -EIO; } int powernv_get_random_long(unsigned long *v) @@ -163,14 +159,12 @@ static __init int rng_create(struct device_node *dn) rng_init_per_cpu(rng, dn); - pr_info_once("Registering arch random hook.\n"); - ppc_md.get_random_seed = powernv_get_random_long; return 0; } -static __init int rng_init(void) +__init void powernv_rng_init(void) { struct device_node *dn; int rc; @@ -188,7 +182,4 @@ static __init int rng_init(void) } initialise_darn(); - - return 0; } -machine_subsys_initcall(powernv, rng_init); diff --git a/arch/powerpc/platforms/powernv/setup.c b/arch/powerpc/platforms/powernv/setup.c index 824c3ad7a0fa..a0c5217bc5c0 100644 --- a/arch/powerpc/platforms/powernv/setup.c +++ b/arch/powerpc/platforms/powernv/setup.c @@ -184,6 +184,8 @@ static void __init pnv_check_guarded_cores(void) } } +__init void powernv_rng_init(void); + static void __init pnv_setup_arch(void) { set_arch_panic_timeout(10, ARCH_PANIC_TIMEOUT); @@ -203,6 +205,8 @@ static void __init pnv_setup_arch(void) pnv_check_guarded_cores(); /* XXX PMCS */ + + powernv_rng_init(); } static void __init pnv_init(void) diff --git a/arch/powerpc/plat
Re: [PATCH] powerpc/rng: wire up during setup_arch
Le 11/06/2022 à 10:11, Jason A. Donenfeld a écrit : > The platform's RNG must be available before random_init() in order to be > useful for initial seeding, which in turn means that it needs to be > called from setup_arch(), rather than from an init call. Fortunately, > each platform already has a setup_arch function pointer, which means > it's easy to wire this up for each of the three platforms that have an > RNG. This commit also removes some noisy log messages that don't add > much. Can't we use one of the machine initcalls for that ? Like machine_early_initcall() or machine_arch_initcall() ? Today it is using machine_subsys_initcall() and you didn't remove it. It means rng_init() will be called twice. Is that ok ? > > Cc: sta...@vger.kernel.org > Cc: Michael Ellerman > Signed-off-by: Jason A. Donenfeld > --- > arch/powerpc/platforms/microwatt/rng.c | 9 ++--- > arch/powerpc/platforms/microwatt/setup.c | 8 > arch/powerpc/platforms/powernv/rng.c | 17 - > arch/powerpc/platforms/powernv/setup.c | 4 > arch/powerpc/platforms/pseries/rng.c | 11 ++- > arch/powerpc/platforms/pseries/setup.c | 3 +++ > 6 files changed, 23 insertions(+), 29 deletions(-) > > diff --git a/arch/powerpc/platforms/microwatt/rng.c > b/arch/powerpc/platforms/microwatt/rng.c > index 7bc4d1cbfaf0..d13f656910ad 100644 > --- a/arch/powerpc/platforms/microwatt/rng.c > +++ b/arch/powerpc/platforms/microwatt/rng.c > @@ -29,7 +29,7 @@ static int microwatt_get_random_darn(unsigned long *v) > return 1; > } > > -static __init int rng_init(void) > +__init void microwatt_rng_init(void) > { > unsigned long val; > int i; > @@ -37,12 +37,7 @@ static __init int rng_init(void) > for (i = 0; i < 10; i++) { > if (microwatt_get_random_darn(&val)) { > ppc_md.get_random_seed = microwatt_get_random_darn; > - return 0; > + return; > } > } > - > - pr_warn("Unable to use DARN for get_random_seed()\n"); > - > - return -EIO; > } > -machine_subsys_initcall(, rng_init); > diff --git a/arch/powerpc/platforms/microwatt/setup.c > b/arch/powerpc/platforms/microwatt/setup.c > index 0b02603bdb74..23c996dcc870 100644 > --- a/arch/powerpc/platforms/microwatt/setup.c > +++ b/arch/powerpc/platforms/microwatt/setup.c > @@ -32,10 +32,18 @@ static int __init microwatt_populate(void) > } > machine_arch_initcall(microwatt, microwatt_populate); > > +__init void microwatt_rng_init(void); > + > +static void __init microwatt_setup_arch(void) > +{ > + microwatt_rng_init(); > +} > + > define_machine(microwatt) { > .name = "microwatt", > .probe = microwatt_probe, > .init_IRQ = microwatt_init_IRQ, > + .setup_arch = microwatt_setup_arch, > .progress = udbg_progress, > .calibrate_decr = generic_calibrate_decr, > }; > diff --git a/arch/powerpc/platforms/powernv/rng.c > b/arch/powerpc/platforms/powernv/rng.c > index e3d44b36ae98..ef24e72a1b69 100644 > --- a/arch/powerpc/platforms/powernv/rng.c > +++ b/arch/powerpc/platforms/powernv/rng.c > @@ -84,24 +84,20 @@ static int powernv_get_random_darn(unsigned long *v) > return 1; > } > > -static int __init initialise_darn(void) > +static void __init initialise_darn(void) > { > unsigned long val; > int i; > > if (!cpu_has_feature(CPU_FTR_ARCH_300)) > - return -ENODEV; > + return; > > for (i = 0; i < 10; i++) { > if (powernv_get_random_darn(&val)) { > ppc_md.get_random_seed = powernv_get_random_darn; > - return 0; > + return; > } > } > - > - pr_warn("Unable to use DARN for get_random_seed()\n"); > - > - return -EIO; > } > > int powernv_get_random_long(unsigned long *v) > @@ -163,14 +159,12 @@ static __init int rng_create(struct device_node *dn) > > rng_init_per_cpu(rng, dn); > > - pr_info_once("Registering arch random hook.\n"); > - > ppc_md.get_random_seed = powernv_get_random_long; > > return 0; > } > > -static __init int rng_init(void) > +__init void powernv_rng_init(void) > { > struct device_node *dn; > int rc; > @@ -188,7 +182,4 @@ static __init int rng_init(void) > } > > initialise_darn(); > - > - return 0; > } > -machine_subsys_initcall(powernv, rng_init); > diff --git a/arch/powerpc/platforms/powernv/setup.c > b/arch/powerpc/platforms/powernv/setup.c > index 824c3ad7a0fa..a0c5217bc5c0 100644 > --- a/arch/powerpc/platforms/powernv/setup.c > +++ b/arch/powerpc/platforms/powernv/setup.c > @@ -184,6 +184,8 @@ static void __init pnv_check_guarded_cores(void) > } > } > > +__init void powernv_rng_init(void); > + > static void __init pnv_setup
Re: [PATCH] powerpc/ptrace: Fix buffer overflow when handling PTRACE_PEEKUSER and PTRACE_POKEUSER
Le 09/06/2022 à 11:52, Ariel Miculas a écrit : > This fixes the gdbserver issue on PPC32 described here: > Link: > https://linuxppc-dev.ozlabs.narkive.com/C46DRek4/debug-problems-on-ppc-83xx-target-due-to-changed-struct-task-struct > > On PPC32, the user space code considers the floating point to be an > array of unsigned int (32 bits) - the index passed in is based on > this assumption. > > fp_state is a matrix consisting of 32 lines > /* FP and VSX 0-31 register set / > struct thread_fp_state { > u64 fpr[32][TS_FPRWIDTH] attribute((aligned(16))); > u64 fpscr; / Floating point status */ > }; > > On PPC32, PT_FPSCR is defined as: (PT_FPR0 + 2*32 + 1) > > This means the fpr index validation allows a range from 0 to 65, leading > to out-of-bounds array access. This ends up corrupting > threads_struct->state, which holds the state of the task. Thus, threads > incorrectly transition from a running state to a traced state and get > stuck in that state. > > On PPC32 it's ok to assume that TS_FPRWIDTH is 1 because CONFIG_VSX is > PPC64 specific. TS_FPROFFSET can be safely ignored, thus the assumption > that fpr is an array of 32 elements of type u64 holds true. > > Solution taken from arch/powerpc/kernel/ptrace32.c > > Signed-off-by: Ariel Miculas > --- > arch/powerpc/kernel/ptrace/ptrace-fpu.c | 31 +++-- > 1 file changed, 29 insertions(+), 2 deletions(-) > > diff --git a/arch/powerpc/kernel/ptrace/ptrace-fpu.c > b/arch/powerpc/kernel/ptrace/ptrace-fpu.c > index 5dca19361316..93695abbbdfb 100644 > --- a/arch/powerpc/kernel/ptrace/ptrace-fpu.c > +++ b/arch/powerpc/kernel/ptrace/ptrace-fpu.c > @@ -6,9 +6,16 @@ > > #include "ptrace-decl.h" > > +#ifdef CONFIG_PPC32 > +/* Macros to workout the correct index for the FPR in the thread struct */ > +#define FPRNUMBER(i) (((i) - PT_FPR0) >> 1) > +#define FPRHALF(i) (((i) - PT_FPR0) & 1) > +#define FPRINDEX(i) TS_FPRWIDTH * FPRNUMBER(i) * 2 + FPRHALF(i) > +#endif I can't see the benefit of such macros if they are only for PPC32. > + > int ptrace_get_fpr(struct task_struct *child, int index, unsigned long > *data) > { > -#ifdef CONFIG_PPC_FPU_REGS > +#if defined(CONFIG_PPC_FPU_REGS) && !defined(CONFIG_PPC32) > unsigned int fpidx = index - PT_FPR0; > #endif #ifdefs should be avoided as much as possible. > > @@ -17,10 +24,20 @@ int ptrace_get_fpr(struct task_struct *child, int index, > unsigned long *data) > > #ifdef CONFIG_PPC_FPU_REGS > flush_fp_to_thread(child); > +#ifdef CONFIG_PPC32 Here you could use IS_ENABLED(CONFIG_PPC32), it would also avoid the above #ifdef. > + /* > + * the user space code considers the floating point > + * to be an array of unsigned int (32 bits) - the > + * index passed in is based on this assumption. > + */ > + *data = ((unsigned int *)child->thread.fp_state.fpr) > + [FPRINDEX(index)]; if I understand FPRINDEX(index) correctly, at the end we have FPRINDEX(i) == i, so I can't see the point. Michael's patch seems easier to understand. I think if one day we want something common to ppc32 and ppc64, we need to use a new macro similar to TS_FPR() but that properly takes ppc32 into account. Pay attention to not change TS_FPR() as it is used in other places where it is valid for both PPC32 and PPC64. > +#else > if (fpidx < (PT_FPSCR - PT_FPR0)) > memcpy(data, &child->thread.TS_FPR(fpidx), sizeof(long)); > else > *data = child->thread.fp_state.fpscr; > +#endif > #else > *data = 0; > #endif > @@ -30,7 +47,7 @@ int ptrace_get_fpr(struct task_struct *child, int index, > unsigned long *data) > > int ptrace_put_fpr(struct task_struct *child, int index, unsigned long data) > { > -#ifdef CONFIG_PPC_FPU_REGS > +#if defined(CONFIG_PPC_FPU_REGS) && !defined(CONFIG_PPC32) > unsigned int fpidx = index - PT_FPR0; > #endif > > @@ -39,10 +56,20 @@ int ptrace_put_fpr(struct task_struct *child, int index, > unsigned long data) > > #ifdef CONFIG_PPC_FPU_REGS > flush_fp_to_thread(child); > +#ifdef CONFIG_PPC32 > + /* > + * the user space code considers the floating point > + * to be an array of unsigned int (32 bits) - the > + * index passed in is based on this assumption. > + */ > + ((unsigned int *)child->thread.fp_state.fpr) > + [FPRINDEX(index)] = data; > +#else > if (fpidx < (PT_FPSCR - PT_FPR0)) > memcpy(&child->thread.TS_FPR(fpidx), &data, sizeof(long)); > else > child->thread.fp_state.fpscr = data; > +#endif > #endif > > return 0;
Re: [PATCH 1/6] powerpc: Add ZERO_GPRS macros for register clears
Le 10/06/2022 à 05:32, Rohan McLure a écrit : >> On 2 Jun 2022, at 2:00 am, Segher Boessenkool >> wrote: >> >> Hi! >> >> On Wed, Jun 01, 2022 at 03:48:45PM +1000, Rohan McLure wrote: >>> +.macro BINOP_REGS op, rhs, start, end >>> + .Lreg=\start >>> + .rept (\end - \start + 1) >>> + \op .Lreg, \rhs >>> + .Lreg=.Lreg+1 >>> + .endr >>> +.endm >> >> This is for unary operations, not binary operations (there is only one >> item on the RHS). You can in principle put a string "a,b" in the rhs >> parameter, but in practice you need a or b to depend on the loop counter >> as well, so even such trickiness won't do. Make the naming less >> confusing, maybe? Or don't have an unused extra level of abstraction in >> the first place :-) >> >> >> Segher > > Thanks Segher, Christophe for reviewing this. > > Yep I see how having a macro to perform rX = rX <> Y for arbitrary infix <> > and operand > is unlikely to find much use outside of ZERO_GPRS. As I resubmit this patch > series I > will rename it to ZERO_REGS or similar to be more explicitly coupled to > ZERO_GPRS. > > Something like this I was thinking: > > .macro ZERO_REGS start, end > .Lreg=\start > .rept (\end - \start + 1) > li .Lreg, 0 > .Lreg=.Lreg+1 > .endr > .endm > I'd have a preference for using a verb, for instance ZEROISE_REGS or CLEAR_REGS Christophe
[PATCH] powerpc/rng: wire up during setup_arch
The platform's RNG must be available before random_init() in order to be useful for initial seeding, which in turn means that it needs to be called from setup_arch(), rather than from an init call. Fortunately, each platform already has a setup_arch function pointer, which means it's easy to wire this up for each of the three platforms that have an RNG. This commit also removes some noisy log messages that don't add much. Cc: sta...@vger.kernel.org Cc: Michael Ellerman Signed-off-by: Jason A. Donenfeld --- arch/powerpc/platforms/microwatt/rng.c | 9 ++--- arch/powerpc/platforms/microwatt/setup.c | 8 arch/powerpc/platforms/powernv/rng.c | 17 - arch/powerpc/platforms/powernv/setup.c | 4 arch/powerpc/platforms/pseries/rng.c | 11 ++- arch/powerpc/platforms/pseries/setup.c | 3 +++ 6 files changed, 23 insertions(+), 29 deletions(-) diff --git a/arch/powerpc/platforms/microwatt/rng.c b/arch/powerpc/platforms/microwatt/rng.c index 7bc4d1cbfaf0..d13f656910ad 100644 --- a/arch/powerpc/platforms/microwatt/rng.c +++ b/arch/powerpc/platforms/microwatt/rng.c @@ -29,7 +29,7 @@ static int microwatt_get_random_darn(unsigned long *v) return 1; } -static __init int rng_init(void) +__init void microwatt_rng_init(void) { unsigned long val; int i; @@ -37,12 +37,7 @@ static __init int rng_init(void) for (i = 0; i < 10; i++) { if (microwatt_get_random_darn(&val)) { ppc_md.get_random_seed = microwatt_get_random_darn; - return 0; + return; } } - - pr_warn("Unable to use DARN for get_random_seed()\n"); - - return -EIO; } -machine_subsys_initcall(, rng_init); diff --git a/arch/powerpc/platforms/microwatt/setup.c b/arch/powerpc/platforms/microwatt/setup.c index 0b02603bdb74..23c996dcc870 100644 --- a/arch/powerpc/platforms/microwatt/setup.c +++ b/arch/powerpc/platforms/microwatt/setup.c @@ -32,10 +32,18 @@ static int __init microwatt_populate(void) } machine_arch_initcall(microwatt, microwatt_populate); +__init void microwatt_rng_init(void); + +static void __init microwatt_setup_arch(void) +{ + microwatt_rng_init(); +} + define_machine(microwatt) { .name = "microwatt", .probe = microwatt_probe, .init_IRQ = microwatt_init_IRQ, + .setup_arch = microwatt_setup_arch, .progress = udbg_progress, .calibrate_decr = generic_calibrate_decr, }; diff --git a/arch/powerpc/platforms/powernv/rng.c b/arch/powerpc/platforms/powernv/rng.c index e3d44b36ae98..ef24e72a1b69 100644 --- a/arch/powerpc/platforms/powernv/rng.c +++ b/arch/powerpc/platforms/powernv/rng.c @@ -84,24 +84,20 @@ static int powernv_get_random_darn(unsigned long *v) return 1; } -static int __init initialise_darn(void) +static void __init initialise_darn(void) { unsigned long val; int i; if (!cpu_has_feature(CPU_FTR_ARCH_300)) - return -ENODEV; + return; for (i = 0; i < 10; i++) { if (powernv_get_random_darn(&val)) { ppc_md.get_random_seed = powernv_get_random_darn; - return 0; + return; } } - - pr_warn("Unable to use DARN for get_random_seed()\n"); - - return -EIO; } int powernv_get_random_long(unsigned long *v) @@ -163,14 +159,12 @@ static __init int rng_create(struct device_node *dn) rng_init_per_cpu(rng, dn); - pr_info_once("Registering arch random hook.\n"); - ppc_md.get_random_seed = powernv_get_random_long; return 0; } -static __init int rng_init(void) +__init void powernv_rng_init(void) { struct device_node *dn; int rc; @@ -188,7 +182,4 @@ static __init int rng_init(void) } initialise_darn(); - - return 0; } -machine_subsys_initcall(powernv, rng_init); diff --git a/arch/powerpc/platforms/powernv/setup.c b/arch/powerpc/platforms/powernv/setup.c index 824c3ad7a0fa..a0c5217bc5c0 100644 --- a/arch/powerpc/platforms/powernv/setup.c +++ b/arch/powerpc/platforms/powernv/setup.c @@ -184,6 +184,8 @@ static void __init pnv_check_guarded_cores(void) } } +__init void powernv_rng_init(void); + static void __init pnv_setup_arch(void) { set_arch_panic_timeout(10, ARCH_PANIC_TIMEOUT); @@ -203,6 +205,8 @@ static void __init pnv_setup_arch(void) pnv_check_guarded_cores(); /* XXX PMCS */ + + powernv_rng_init(); } static void __init pnv_init(void) diff --git a/arch/powerpc/platforms/pseries/rng.c b/arch/powerpc/platforms/pseries/rng.c index 6268545947b8..d39bfce39aa1 100644 --- a/arch/powerpc/platforms/pseries/rng.c +++ b/arch/powerpc/platforms/pseries/rng.c @@ -24,19 +24,12 @@ static int pseries_get_random_