Re: [PATCH 00/40] soc: Convert to platform remove callback returning void

[Joel Stanley]

On Mon, 25 Sept 2023 at 09:55, Uwe Kleine-König
 wrote:
>
> Hello,
>
> this series converts all platform drivers below drivers/soc to use
> .remove_new(). The motivation is to get rid of an integer return code
> that is (mostly) ignored by the platform driver core and error prone on
> the driver side.
>
> See commit 5c5a7680e67b ("platform: Provide a remove callback that
> returns no value") for an extended explanation and the eventual goal.
>
> As there is no single maintainer team for drivers/soc, I suggest the
> individual maintainers to pick up "their" patches.

I'd be happy if Arnd merged the lot at once. Arnd, what do you think?

If that will be too messy then I understand. I have queued the aspeed
ones locally and will push that out if we decide that's the best way
to go.

Thanks for doing this cleanup Uwe.

Cheers,

Joel

Re: [PATCH] powerpc: Hide empty pt_regs at base of the stack

[Joel Stanley]

On Thu, 24 Aug 2023 at 06:42, Michael Ellerman  wrote:
>
> A thread started via eg. user_mode_thread() runs in the kernel to begin
> with and then may later return to userspace. While it's running in the
> kernel it has a pt_regs at the base of its kernel stack, but that
> pt_regs is all zeroes.
>
> If the thread oopses in that state, it leads to an ugly stack trace with
> a big block of zero GPRs, as reported by Joel:
>
>   Kernel panic - not syncing: VFS: Unable to mount root fs on 
> unknown-block(0,0)
>   CPU: 0 PID: 1 Comm: swapper/0 Not tainted 
> 6.5.0-rc7-4-gf7757129e3de-dirty #3
>   Hardware name: IBM PowerNV (emulated by qemu) POWER9 0x4e1200 opal:v7.0 
> PowerNV
>   Call Trace:
>   [c36afb00] [c10dd058] dump_stack_lvl+0x6c/0x9c (unreliable)
>   [c36afb30] [c013c524] panic+0x178/0x424
>   [c36afbd0] [c2005100] mount_root_generic+0x250/0x324
>   [c36afca0] [c20057d0] prepare_namespace+0x2d4/0x344
>   [c36afd20] [c20049c0] kernel_init_freeable+0x358/0x3ac
>   [c36afdf0] [c00111b0] kernel_init+0x30/0x1a0
>   [c36afe50] [c000debc] ret_from_kernel_user_thread+0x14/0x1c
>   --- interrupt: 0 at 0x0
>   NIP:   LR:  CTR: 
>   REGS: c36afe80 TRAP:    Not tainted  
> (6.5.0-rc7-4-gf7757129e3de-dirty)
>   MSR:   <>  CR:   XER: 
>   CFAR:  IRQMASK: 0
>   GPR00:    
>   GPR04:    
>   GPR08:    
>   GPR12:    
>   GPR16:    
>   GPR20:    
>   GPR24:    
>   GPR28:    
>   NIP [] 0x0
>   LR [] 0x0
>   --- interrupt: 0
>
> The all-zero pt_regs looks ugly and conveys no useful information, other
> than its presence. So detect that case and just show the presence of the
> frame by printing the interrupt marker, eg:
>
>   Kernel panic - not syncing: VFS: Unable to mount root fs on 
> unknown-block(0,0)
>   CPU: 0 PID: 1 Comm: swapper/0 Not tainted 
> 6.5.0-rc3-00126-g18e9506562a0-dirty #301
>   Hardware name: IBM pSeries (emulated by qemu) POWER9 (raw) 0x4e1202 
> 0xf05 of:SLOF,HEAD hv:linux,kvm pSeries
>   Call Trace:
>   [c3aabb00] [c1143db8] dump_stack_lvl+0x6c/0x9c (unreliable)
>   [c3aabb30] [c014c624] panic+0x178/0x424
>   [c3aabbd0] [c20050fc] mount_root_generic+0x250/0x324
>   [c3aabca0] [c20057cc] prepare_namespace+0x2d4/0x344
>   [c3aabd20] [c20049bc] kernel_init_freeable+0x358/0x3ac
>   [c3aabdf0] [c00111b0] kernel_init+0x30/0x1a0
>   [c3aabe50] [c000debc] ret_from_kernel_user_thread+0x14/0x1c
>   --- interrupt: 0 at 0x0
>
> To avoid ever suppressing a valid pt_regs make sure the pt_regs has a
> zero MSR and TRAP value, and is located at the very base of the stack.
>
> Reported-by: Joel Stanley 
> Reported-by: Nicholas Piggin 
> Signed-off-by: Michael Ellerman 

Reviewed-by: Joel Stanley 

Thanks for the explanation and the patch.

> ---
>  arch/powerpc/kernel/process.c | 26 +++---
>  1 file changed, 23 insertions(+), 3 deletions(-)
>
> diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
> index b68898ac07e1..392404688cec 100644
> --- a/arch/powerpc/kernel/process.c
> +++ b/arch/powerpc/kernel/process.c
> @@ -2258,6 +2258,22 @@ unsigned long __get_wchan(struct task_struct *p)
> return ret;
>  }
>
> +static bool empty_user_regs(struct pt_regs *regs, struct task_struct *tsk)
> +{
> +   unsigned long stack_page;
> +
> +   // A non-empty pt_regs should never have a zero MSR or TRAP value.
> +   if (regs->msr || regs->trap)
> +   return false;
> +
> +   // Check it sits at the very base of the stack
> +   stack_page = (unsigned long)task_stack_page(tsk);
> +   if ((unsigned long)(regs + 1) != stack_page + THREAD_SIZE)
> +   return false;
> +
> +   return true;
> +}
> +
>  static int kstack_depth_to_print = CONFIG_PRINT_STACK_DEPTH;
>
>  void __no_sanitize_address show_stack(struct task_struct *tsk,
> @@ -

Re: [PATCH 4/4] powerpc/64s: Use POWER10 stsync barrier for wmb()

[Joel Stanley]

On Thu, 24 Aug 2023 at 12:12, Michael Ellerman  wrote:
>
> Michael Ellerman  writes:
> > Michael Ellerman  writes:
> >> "Nicholas Piggin"  writes:
> >>> On Wed Jun 14, 2023 at 3:56 PM AEST, Michael Ellerman wrote:
>  Michael Ellerman  writes:
>  > Nicholas Piggin  writes:
>  >> The most expensive ordering for hwsync to provide is the store-load
>  >> barrier, because all prior stores have to be drained to the caches
>  >> before subsequent instructions can complete.
>  >>
>  >> stsync just orders stores which means it can just be a barrer that
>  >> goes down the store queue and orders draining, and does not prevent
>  >> completion of subsequent instructions. So it should be faster than
>  >> hwsync.
>  >>
>  >> Use stsync for wmb(). Older processors that don't recognise the SC
>  >> field should treat this as hwsync.
>  >
>  > qemu (7.1) emulating ppc64e does not :/
>  >
>  >   mpic: Setting up MPIC " OpenPIC  " version 1.2 at fe004, max 1 
>  > CPUs
>  >   mpic: ISU size: 256, shift: 8, mask: ff
>  >   mpic: Initializing for 256 sources
>  >   Oops: Exception in kernel mode, sig: 4 [#1]
>  ..
>  >
>  > I guess just put it behind an #ifdef 64S.
> 
>  That doesn't work because qemu emulating a G5 also doesn't accept it.
> 
>  So either we need to get qemu updated and wait a while for that to
>  percolate, or do some runtime patching of wmbs in the kernel >_<
> >>>
> >>> Gah, sorry. QEMU really should be ignoring reserved fields in
> >>> instructions :(
> >>
> >> Yeah, it's an annoying discrepancy vs real hardware and the ISA.
> >>
> >>> I guess leave it out for now. Should fix QEMU but we probably also need
> >>> to do patching so as not to break older QEMUs.
> >>
> >> I'll plan to take the first 3 patches, they seem OK as-is.
> >
> > I didn't do that in the end, because patch 2 suffers from the same
>  ^
>  3
> > problem of not working on QEMU.

Did we get a patch to fix this in to Qemu?

Qemu has recently developed a stable tree process, so if we had a
backportable fix we could get it in there too.

Cheers,

Joel

Re: [PATCH v2] powernv/opal-prd: Silence memcpy() run-time false positive warnings

[Joel Stanley]

On Fri, 7 Jul 2023 at 05:11, Mahesh Salgaonkar  wrote:
>
> opal_prd_msg_notifier extracts the opal prd message size from the message
> header and uses it for allocating opal_prd_msg_queue_item that includes
> the correct message size to be copied. However, while running under
> CONFIG_FORTIFY_SOURCE=y, it triggers following run-time warning:
>
> [ 6458.234352] memcpy: detected field-spanning write (size 32) of single 
> field ">msg" at arch/powerpc/platforms/powernv/opal-prd.c:355 (size 4)
> [ 6458.234390] WARNING: CPU: 9 PID: 660 at 
> arch/powerpc/platforms/powernv/opal-prd.c:355 
> opal_prd_msg_notifier+0x174/0x188 [opal_prd]
> [...]
> [ 6458.234709] NIP [c0080e0c0e6c] opal_prd_msg_notifier+0x174/0x188 
> [opal_prd]
> [ 6458.234723] LR [c0080e0c0e68] opal_prd_msg_notifier+0x170/0x188 
> [opal_prd]
> [ 6458.234736] Call Trace:
> [ 6458.234742] [c002acb23c10] [c0080e0c0e68] 
> opal_prd_msg_notifier+0x170/0x188 [opal_prd] (unreliable)
> [ 6458.234759] [c002acb23ca0] [c019ccc0] 
> notifier_call_chain+0xc0/0x1b0
> [ 6458.234774] [c002acb23d00] [c019ceac] 
> atomic_notifier_call_chain+0x2c/0x40
> [ 6458.234788] [c002acb23d20] [c00d69b4] 
> opal_message_notify+0xf4/0x2c0
> [...]
>
> Split the copy to avoid false positive run-time warning.
>
> Reported-by: Aneesh Kumar K.V 
> Signed-off-by: Mahesh Salgaonkar 

I hit this on a box running the Ubuntu 6.2.0-27-generic kernel.

Do we plan on merging this fix?



> ---
> Change from v1:
> - Rework the memcpy to copy message header and rest of the message
>   separately without adding flex array.
> ---
>  arch/powerpc/platforms/powernv/opal-prd.c |4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/platforms/powernv/opal-prd.c 
> b/arch/powerpc/platforms/powernv/opal-prd.c
> index 113bdb151f687..a068cbc4d43c0 100644
> --- a/arch/powerpc/platforms/powernv/opal-prd.c
> +++ b/arch/powerpc/platforms/powernv/opal-prd.c
> @@ -352,7 +352,9 @@ static int opal_prd_msg_notifier(struct notifier_block 
> *nb,
> if (!item)
> return -ENOMEM;
>
> -   memcpy(>msg, msg->params, msg_size);
> +   item->msg = *hdr;
> +   hdr++;
> +   memcpy((char *)>msg + sizeof(*hdr), hdr, msg_size - 
> sizeof(*hdr));
>
> spin_lock_irqsave(_prd_msg_queue_lock, flags);
> list_add_tail(>list, _prd_msg_queue);
>
>

[PATCH] powerpc/config: Disable SLAB_DEBUG_ON in skiroot

[Joel Stanley]

In 5.10 commit 5e84dd547bce ("powerpc/configs/skiroot: Enable some more
hardening options") set SLUB_DEUBG_ON.

When 5.14 came around, commit 792702911f58 ("slub: force on
no_hash_pointers when slub_debug is enabled") print all the
pointers when SLUB_DEUBG_ON is set. This was fine, but in 5.12 commit
5ead723a20e0 ("lib/vsprintf: no_hash_pointers prints all addresses as
unhashed") added the warning at boot.

Disable SLAB_DEBUG_ON as we don't want the nasty warning. We have
CONFIG_EXPERT so SLAB_DEBUG is enabled. We do lose the settings in
DEBUG_DEFAULT_FLAGS, but it's not clear that these should have been
always-on anyway.

Signed-off-by: Joel Stanley 
---
 arch/powerpc/configs/skiroot_defconfig | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/powerpc/configs/skiroot_defconfig 
b/arch/powerpc/configs/skiroot_defconfig
index 71cfb990a74f..8d3eacb50d56 100644
--- a/arch/powerpc/configs/skiroot_defconfig
+++ b/arch/powerpc/configs/skiroot_defconfig
@@ -289,7 +289,6 @@ CONFIG_LIBCRC32C=y
 # CONFIG_XZ_DEC_SPARC is not set
 CONFIG_PRINTK_TIME=y
 CONFIG_MAGIC_SYSRQ=y
-CONFIG_SLUB_DEBUG_ON=y
 CONFIG_SCHED_STACK_END_CHECK=y
 CONFIG_DEBUG_STACKOVERFLOW=y
 CONFIG_PANIC_ON_OOPS=y
-- 
2.40.1

Re: [kvm-unit-tests v4 00/12] powerpc: updates, P10, PNV support

[Joel Stanley]

On Thu, 15 Jun 2023 at 03:02, Nicholas Piggin  wrote:
>
> On Wed Jun 14, 2023 at 11:09 AM AEST, Joel Stanley wrote:
> > On Thu, 8 Jun 2023 at 07:58, Nicholas Piggin  wrote:
> > >
> > > Posting again, a couple of patches were merged and accounted for review
> > > comments from last time.
> >
> > I saw some failures in the spr tests running on a power9 powernv system:
> >
> > $ TESTNAME=sprs TIMEOUT=90s ACCEL= ./powerpc/run powerpc/sprs.elf -smp
> > 1 |grep FAIL
> > FAIL: WORT  ( 895):0xc0deba80 <==> 0x
>
> This is just TCG machine? I'm not sure why WORT fails, AFAIKS it's the
> same on POWER8 and doesn't do anything just a simple register. I think
> on real hardware WORT may not have any bits implemented on POWER9
> though.

Yeah, just the TCG machine. Now that you point it out all of the
failures I reported are for ACCEL=, so they are running in tcg
mode.

run_migration timeout -k 1s --foreground 90s
/usr/bin/qemu-system-ppc64 -nodefaults -machine pseries,accel=tcg
-bios powerpc/boot_rom.bin -display none -serial stdio -kernel
powerpc/sprs.elf -smp 1 -append -w # -initrd /tmp/tmp.61XhbvCGcb


>
> > $ MIGRATION=yes TESTNAME=sprs-migration TIMEOUT=90s ACCEL=
> > ./powerpc/run powerpc/sprs.elf -smp 1 -append '-w' | grep FAIL
> > FAIL: SRR0  (  26):0xcafefacec0debabc <==> 0x00402244
> > FAIL: SRR1  (  27):0xc006409ebab6 <==> 0x80001001
> > FAIL: CTRL  ( 136):0x <==> 0x8001
> > FAIL: WORT  ( 895):0xc0deba80 <==> 0x
> > FAIL: PIR   (1023):0x0010 <==> 0x0049
> >
> > Linux 6.2.0-20-generic
> > QEMU emulator version 7.2.0 (Debian 1:7.2+dfsg-5ubuntu2)
> >
> > On a power8 powernv:
> >
> > MIGRATION=yes TESTNAME=sprs-migration TIMEOUT=90s ACCEL= ./powerpc/run
> > powerpc/sprs.elf -smp 1 -append '-w' |grep FAIL
> > FAIL: SRR0  (  26):0xcafefacec0debabc <==> 0x00402234
> > FAIL: SRR1  (  27):0xc006409ebab6 <==> 0x80001000
> > FAIL: CTRL  ( 136):0x <==> 0x8001
> > FAIL: PIR   (1023):0x0060 <==> 0x0030
>
> Hmm, seems we take some interrupt over migration test that is not
> accounted for (could check the address in SRR0 to see where it is).
> Either need to prevent that interrupt or avoid failing on SRR0/1 on
> this test.
>
> Interesting about CTRL, I wonder if that not migrating correctly.
> PIR looks like a migration issue as well, it can't be changed so
> destination CPU has got a different PIR. I would be inclined to
> leave those as failing to remind us to look into them.
>
> I'll take a look at the others though.
>
> Thanks,
> Nick

Re: [kvm-unit-tests v4 00/12] powerpc: updates, P10, PNV support

[Joel Stanley]

On Thu, 8 Jun 2023 at 07:58, Nicholas Piggin  wrote:
>
> Posting again, a couple of patches were merged and accounted for review
> comments from last time.

I saw some failures in the spr tests running on a power9 powernv system:

$ TESTNAME=sprs TIMEOUT=90s ACCEL= ./powerpc/run powerpc/sprs.elf -smp
1 |grep FAIL
FAIL: WORT  ( 895):0xc0deba80 <==> 0x

$ MIGRATION=yes TESTNAME=sprs-migration TIMEOUT=90s ACCEL=
./powerpc/run powerpc/sprs.elf -smp 1 -append '-w' | grep FAIL
FAIL: SRR0  (  26):0xcafefacec0debabc <==> 0x00402244
FAIL: SRR1  (  27):0xc006409ebab6 <==> 0x80001001
FAIL: CTRL  ( 136):0x <==> 0x8001
FAIL: WORT  ( 895):0xc0deba80 <==> 0x
FAIL: PIR   (1023):0x0010 <==> 0x0049

Linux 6.2.0-20-generic
QEMU emulator version 7.2.0 (Debian 1:7.2+dfsg-5ubuntu2)

On a power8 powernv:

MIGRATION=yes TESTNAME=sprs-migration TIMEOUT=90s ACCEL= ./powerpc/run
powerpc/sprs.elf -smp 1 -append '-w' |grep FAIL
FAIL: SRR0  (  26):0xcafefacec0debabc <==> 0x00402234
FAIL: SRR1  (  27):0xc006409ebab6 <==> 0x80001000
FAIL: CTRL  ( 136):0x <==> 0x8001
FAIL: PIR   (1023):0x0060 <==> 0x0030

Linux 5.4.0-146-generic
QEMU emulator version 4.2.1 (Debian 1:4.2-3ubuntu6.26)

Cheers,

Joel

>
> Thanks,
> Nick
>
> Nicholas Piggin (12):
>   powerpc: Report instruction address and MSR in unhandled exception
> error
>   powerpc: Add some checking to exception handler install
>   powerpc: Abstract H_CEDE calls into a sleep functions
>   powerpc: Add ISA v3.1 (POWER10) support to SPR test
>   powerpc: Extract some common helpers and defines to headers
>   powerpc/sprs: Specify SPRs with data rather than code
>   powerpc/spapr_vpa: Add basic VPA tests
>   powerpc: Expand exception handler vector granularity
>   powerpc: Add support for more interrupts including HV interrupts
>   powerpc: Discover runtime load address dynamically
>   powerpc: Support powernv machine with QEMU TCG
>   powerpc/sprs: Test hypervisor registers on powernv machine
>
>  lib/powerpc/asm/handlers.h  |   2 +-
>  lib/powerpc/asm/hcall.h |   1 +
>  lib/powerpc/asm/ppc_asm.h   |   9 +
>  lib/powerpc/asm/processor.h |  55 ++-
>  lib/powerpc/handlers.c  |  10 +-
>  lib/powerpc/hcall.c |   4 +-
>  lib/powerpc/io.c|  27 +-
>  lib/powerpc/io.h|   6 +
>  lib/powerpc/processor.c |  79 -
>  lib/powerpc/setup.c |   8 +-
>  lib/ppc64/asm/opal.h|  15 +
>  lib/ppc64/asm/vpa.h |  62 
>  lib/ppc64/opal-calls.S  |  46 +++
>  lib/ppc64/opal.c|  74 +
>  powerpc/Makefile.ppc64  |   4 +-
>  powerpc/cstart64.S  | 105 --
>  powerpc/run |  35 +-
>  powerpc/spapr_hcall.c   |   9 +-
>  powerpc/spapr_vpa.c | 172 ++
>  powerpc/sprs.c  | 645 ++--
>  powerpc/tm.c|  20 +-
>  powerpc/unittests.cfg   |   3 +
>  22 files changed, 1133 insertions(+), 258 deletions(-)
>  create mode 100644 lib/ppc64/asm/opal.h
>  create mode 100644 lib/ppc64/asm/vpa.h
>  create mode 100644 lib/ppc64/opal-calls.S
>  create mode 100644 lib/ppc64/opal.c
>  create mode 100644 powerpc/spapr_vpa.c
>
> --
> 2.40.1
>

Re: [PATCH 2/4] powerpc/64s: Add POWER10 store sync mnemonics

[Joel Stanley]

On Fri, 9 Jun 2023 at 10:01, Nicholas Piggin  wrote:
>
> ISA v3.1 introduces new sync types for store ordering.
>
>   stncisync
>   stcisync
>   stsync
>
> Add ppc-opcode defines for these. This changes PPC_RAW_SYNC to take
> L,SC parameters and adds a PPC_RAW_HWSYNC for callers that want the
> plain old sync (aka hwsync).

I checked these against the ISA and they seem correct.

Did you consider changing LWSYNC to be defined in terms of your new
PPC_RAW_SYNC?

Reviewed-by: Joel Stanley .

>
> Signed-off-by: Nicholas Piggin 
> ---
>  arch/powerpc/include/asm/ppc-opcode.h | 19 ++-
>  arch/powerpc/kernel/traps.c   |  2 +-
>  arch/powerpc/lib/feature-fixups.c |  6 +++---
>  arch/powerpc/net/bpf_jit_comp64.c |  2 +-
>  4 files changed, 19 insertions(+), 10 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/ppc-opcode.h 
> b/arch/powerpc/include/asm/ppc-opcode.h
> index ca5a0da7df4e..7bc8bbcd4adb 100644
> --- a/arch/powerpc/include/asm/ppc-opcode.h
> +++ b/arch/powerpc/include/asm/ppc-opcode.h
> @@ -326,6 +326,8 @@
>  #define ___PPC_R(r)(((r) & 0x1) << 16)
>  #define ___PPC_PRS(prs)(((prs) & 0x1) << 17)
>  #define ___PPC_RIC(ric)(((ric) & 0x3) << 18)
> +#define ___PPC_L(l)(((l) & 0x7) << 21)
> +#define ___PPC_SC(sc)  (((sc) & 0x3) << 16)
>  #define __PPC_RA(a)___PPC_RA(__REG_##a)
>  #define __PPC_RA0(a)   ___PPC_RA(__REGA0_##a)
>  #define __PPC_RB(b)___PPC_RB(__REG_##b)
> @@ -378,8 +380,6 @@
>  #define PPC_RAW_LQARX(t, a, b, eh) (0x7c000228 | ___PPC_RT(t) | 
> ___PPC_RA(a) | ___PPC_RB(b) | __PPC_EH(eh))
>  #define PPC_RAW_LDARX(t, a, b, eh) (0x7ca8 | ___PPC_RT(t) | 
> ___PPC_RA(a) | ___PPC_RB(b) | __PPC_EH(eh))
>  #define PPC_RAW_LWARX(t, a, b, eh) (0x7c28 | ___PPC_RT(t) | 
> ___PPC_RA(a) | ___PPC_RB(b) | __PPC_EH(eh))
> -#define PPC_RAW_PHWSYNC(0x7c8004ac)
> -#define PPC_RAW_PLWSYNC(0x7ca004ac)
>  #define PPC_RAW_STQCX(t, a, b) (0x7c00016d | ___PPC_RT(t) | 
> ___PPC_RA(a) | ___PPC_RB(b))
>  #define PPC_RAW_MADDHD(t, a, b, c) (0x1030 | ___PPC_RT(t) | 
> ___PPC_RA(a) | ___PPC_RB(b) | ___PPC_RC(c))
>  #define PPC_RAW_MADDHDU(t, a, b, c)(0x1031 | ___PPC_RT(t) | 
> ___PPC_RA(a) | ___PPC_RB(b) | ___PPC_RC(c))
> @@ -396,6 +396,13 @@
>  #define PPC_RAW_RFCI   (0x4c66)
>  #define PPC_RAW_RFDI   (0x4c4e)
>  #define PPC_RAW_RFMCI  (0x4c4c)
> +#define PPC_RAW_SYNC(l, sc)(0x7c0004ac | ___PPC_L(l) | 
> ___PPC_SC(sc))
> +#define PPC_RAW_HWSYNC()   PPC_RAW_SYNC(0, 0)
> +#define PPC_RAW_STNCISYNC()PPC_RAW_SYNC(1, 1)
> +#define PPC_RAW_STCISYNC() PPC_RAW_SYNC(0, 2)
> +#define PPC_RAW_STSYNC()   PPC_RAW_SYNC(0, 3)
> +#define PPC_RAW_PHWSYNC()  PPC_RAW_SYNC(4, 0)
> +#define PPC_RAW_PLWSYNC()  PPC_RAW_SYNC(5, 0)
>  #define PPC_RAW_TLBILX(t, a, b)(0x7c24 | __PPC_T_TLB(t) 
> |  __PPC_RA0(a) | __PPC_RB(b))
>  #define PPC_RAW_WAIT_v203  (0x7c7c)
>  #define PPC_RAW_WAIT(w, p) (0x7c3c | __PPC_WC(w) | 
> __PPC_PL(p))
> @@ -421,7 +428,6 @@
>  #define PPC_RAW_DCBFPS(a, b)   (0x7cac | ___PPC_RA(a) | 
> ___PPC_RB(b) | (4 << 21))
>  #define PPC_RAW_DCBSTPS(a, b)  (0x7cac | ___PPC_RA(a) | 
> ___PPC_RB(b) | (6 << 21))
>  #define PPC_RAW_SC()   (0x4402)
> -#define PPC_RAW_SYNC() (0x7c0004ac)
>  #define PPC_RAW_ISYNC()(0x4c00012c)
>
>  /*
> @@ -641,8 +647,11 @@
>  #define STBCIX(s, a, b)stringify_in_c(.long 
> PPC_RAW_STBCIX(s, a, b))
>  #define PPC_DCBFPS(a, b)   stringify_in_c(.long PPC_RAW_DCBFPS(a, b))
>  #define PPC_DCBSTPS(a, b)  stringify_in_c(.long PPC_RAW_DCBSTPS(a, b))
> -#define PPC_PHWSYNCstringify_in_c(.long PPC_RAW_PHWSYNC)
> -#define PPC_PLWSYNCstringify_in_c(.long PPC_RAW_PLWSYNC)
> +#define PPC_STNCISYNC  stringify_in_c(.long PPC_RAW_STNCISYNC())
> +#define PPC_STCISYNC   stringify_in_c(.long PPC_RAW_STCISYNC())
> +#define PPC_STSYNC stringify_in_c(.long PPC_RAW_STSYNC())
> +#define PPC_PHWSYNCstringify_in_c(.long PPC_RAW_PHWSYNC())
> +#define PPC_PLWSYNCstringify_in_c(.long PPC_RAW_PLWSYNC())
>  #define STXVD2X(s, a, b)   stringify_in_c(.long PPC_RAW_STXVD2X(s, a, b))
>  #define LXVD2X(s, a, b)stringify_in_c(.long 
> PPC_RAW_LXVD2X(s, a, b))
>  #define MFVRD(a, t)stringify_in_c(.long PPC_RAW_MFVRD(a, t))
> diff --gi

[RFC PATCH 3/3] powernv/pci: Remove last IODA1 defines

[Joel Stanley]

Signed-off-by: Joel Stanley 
---
 arch/powerpc/platforms/powernv/pci-ioda.c | 5 +++--
 arch/powerpc/platforms/powernv/pci.h  | 1 -
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c 
b/arch/powerpc/platforms/powernv/pci-ioda.c
index c2af5a55a434..cb637827bc58 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
@@ -45,7 +45,8 @@
 #include "pci.h"
 #include "../../../../drivers/pci/pci.h"
 
-static const char * const pnv_phb_names[] = { "IODA1", "IODA2", "NPU_OCAPI" };
+/* This array is indexed with enum pnv_phb_type */
+static const char * const pnv_phb_names[] = { "IODA2", "NPU_OCAPI" };
 
 static void pnv_pci_ioda2_set_bypass(struct pnv_ioda_pe *pe, bool enable);
 static void pnv_pci_configure_bus(struct pci_bus *bus);
@@ -359,7 +360,7 @@ static void __init pnv_ioda_parse_m64_window(struct pnv_phb 
*phb)
const __be32 *r;
u64 pci_addr;
 
-   if (phb->type != PNV_PHB_IODA1 && phb->type != PNV_PHB_IODA2) {
+   if (phb->type != PNV_PHB_IODA2) {
pr_info("  Not support M64 window\n");
return;
}
diff --git a/arch/powerpc/platforms/powernv/pci.h 
b/arch/powerpc/platforms/powernv/pci.h
index 3353db882e35..957f2b47a3c0 100644
--- a/arch/powerpc/platforms/powernv/pci.h
+++ b/arch/powerpc/platforms/powernv/pci.h
@@ -10,7 +10,6 @@
 struct pci_dn;
 
 enum pnv_phb_type {
-   PNV_PHB_IODA1,
PNV_PHB_IODA2,
PNV_PHB_NPU_OCAPI,
 };
-- 
2.39.2

[RFC PATCH 2/3] powerpc/pci: Remove MVE code

[Joel Stanley]

With IODA1 support gone the OPAL calls to set MVE are dead code. Remove
them.

TODO: Do we have rules for removing unused OPAL APIs? Should we leave it
in opal.h? opal-call.c?

Signed-off-by: Joel Stanley 
---
 arch/powerpc/include/asm/opal.h|  3 ---
 arch/powerpc/platforms/powernv/opal-call.c |  2 --
 arch/powerpc/platforms/powernv/pci-ioda.c  | 23 +-
 3 files changed, 1 insertion(+), 27 deletions(-)

diff --git a/arch/powerpc/include/asm/opal.h b/arch/powerpc/include/asm/opal.h
index 726125a534de..a9b31cc258fc 100644
--- a/arch/powerpc/include/asm/opal.h
+++ b/arch/powerpc/include/asm/opal.h
@@ -112,9 +112,6 @@ int64_t opal_pci_set_pe(uint64_t phb_id, uint64_t 
pe_number, uint64_t bus_dev_fu
uint8_t pe_action);
 int64_t opal_pci_set_peltv(uint64_t phb_id, uint32_t parent_pe, uint32_t 
child_pe,
   uint8_t state);
-int64_t opal_pci_set_mve(uint64_t phb_id, uint32_t mve_number, uint32_t 
pe_number);
-int64_t opal_pci_set_mve_enable(uint64_t phb_id, uint32_t mve_number,
-   uint32_t state);
 int64_t opal_pci_get_xive_reissue(uint64_t phb_id, uint32_t xive_number,
  uint8_t *p_bit, uint8_t *q_bit);
 int64_t opal_pci_set_xive_reissue(uint64_t phb_id, uint32_t xive_number,
diff --git a/arch/powerpc/platforms/powernv/opal-call.c 
b/arch/powerpc/platforms/powernv/opal-call.c
index f812c74c61e5..021b0ec29e24 100644
--- a/arch/powerpc/platforms/powernv/opal-call.c
+++ b/arch/powerpc/platforms/powernv/opal-call.c
@@ -167,8 +167,6 @@ OPAL_CALL(opal_pci_map_pe_mmio_window,  
OPAL_PCI_MAP_PE_MMIO_WINDOW);
 OPAL_CALL(opal_pci_set_phb_table_memory,   OPAL_PCI_SET_PHB_TABLE_MEMORY);
 OPAL_CALL(opal_pci_set_pe, OPAL_PCI_SET_PE);
 OPAL_CALL(opal_pci_set_peltv,  OPAL_PCI_SET_PELTV);
-OPAL_CALL(opal_pci_set_mve,OPAL_PCI_SET_MVE);
-OPAL_CALL(opal_pci_set_mve_enable, OPAL_PCI_SET_MVE_ENABLE);
 OPAL_CALL(opal_pci_get_xive_reissue,   OPAL_PCI_GET_XIVE_REISSUE);
 OPAL_CALL(opal_pci_set_xive_reissue,   OPAL_PCI_SET_XIVE_REISSUE);
 OPAL_CALL(opal_pci_set_xive_pe,OPAL_PCI_SET_XIVE_PE);
diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c 
b/arch/powerpc/platforms/powernv/pci-ioda.c
index 2c4e842c2749..c2af5a55a434 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
@@ -865,29 +865,8 @@ int pnv_ioda_configure_pe(struct pnv_phb *phb, struct 
pnv_ioda_pe *pe)
for (rid = pe->rid; rid < rid_end; rid++)
phb->ioda.pe_rmap[rid] = pe->pe_number;
 
-   /* Setup one MVTs on IODA1 */
-   if (phb->type != PNV_PHB_IODA1) {
-   pe->mve_number = 0;
-   goto out;
-   }
+   pe->mve_number = 0;
 
-   pe->mve_number = pe->pe_number;
-   rc = opal_pci_set_mve(phb->opal_id, pe->mve_number, pe->pe_number);
-   if (rc != OPAL_SUCCESS) {
-   pe_err(pe, "OPAL error %ld setting up MVE %x\n",
-  rc, pe->mve_number);
-   pe->mve_number = -1;
-   } else {
-   rc = opal_pci_set_mve_enable(phb->opal_id,
-pe->mve_number, OPAL_ENABLE_MVE);
-   if (rc) {
-   pe_err(pe, "OPAL error %ld enabling MVE %x\n",
-  rc, pe->mve_number);
-   pe->mve_number = -1;
-   }
-   }
-
-out:
return 0;
 }
 
-- 
2.39.2

[RFC PATCH 1/3] powernv/pci: Remove ioda1 support

[Joel Stanley]

The final "VPL" Power7 boxes that were used for powernv bringup have
been scrapped, meaning there are no machines with ioda1 left.

This patch removes the obvious unused code.

Signed-off-by: Joel Stanley 
---
 arch/powerpc/platforms/powernv/pci-ioda.c | 448 +-
 arch/powerpc/platforms/powernv/pci.c  |   5 -
 arch/powerpc/platforms/powernv/pci.h  |   4 -
 3 files changed, 2 insertions(+), 455 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c 
b/arch/powerpc/platforms/powernv/pci-ioda.c
index a02e9cdb5b5d..2c4e842c2749 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
@@ -45,10 +45,6 @@
 #include "pci.h"
 #include "../../../../drivers/pci/pci.h"
 
-#define PNV_IODA1_M64_NUM  16  /* Number of M64 BARs   */
-#define PNV_IODA1_M64_SEGS 8   /* Segments per M64 BAR */
-#define PNV_IODA1_DMA32_SEGSIZE0x1000
-
 static const char * const pnv_phb_names[] = { "IODA1", "IODA2", "NPU_OCAPI" };
 
 static void pnv_pci_ioda2_set_bypass(struct pnv_ioda_pe *pe, bool enable);
@@ -280,86 +276,6 @@ static void pnv_ioda_reserve_dev_m64_pe(struct pci_dev 
*pdev,
}
 }
 
-static int pnv_ioda1_init_m64(struct pnv_phb *phb)
-{
-   struct resource *r;
-   int index;
-
-   /*
-* There are 16 M64 BARs, each of which has 8 segments. So
-* there are as many M64 segments as the maximum number of
-* PEs, which is 128.
-*/
-   for (index = 0; index < PNV_IODA1_M64_NUM; index++) {
-   unsigned long base, segsz = phb->ioda.m64_segsize;
-   int64_t rc;
-
-   base = phb->ioda.m64_base +
-  index * PNV_IODA1_M64_SEGS * segsz;
-   rc = opal_pci_set_phb_mem_window(phb->opal_id,
-   OPAL_M64_WINDOW_TYPE, index, base, 0,
-   PNV_IODA1_M64_SEGS * segsz);
-   if (rc != OPAL_SUCCESS) {
-   pr_warn("  Error %lld setting M64 PHB#%x-BAR#%d\n",
-   rc, phb->hose->global_number, index);
-   goto fail;
-   }
-
-   rc = opal_pci_phb_mmio_enable(phb->opal_id,
-   OPAL_M64_WINDOW_TYPE, index,
-   OPAL_ENABLE_M64_SPLIT);
-   if (rc != OPAL_SUCCESS) {
-   pr_warn("  Error %lld enabling M64 PHB#%x-BAR#%d\n",
-   rc, phb->hose->global_number, index);
-   goto fail;
-   }
-   }
-
-   for (index = 0; index < phb->ioda.total_pe_num; index++) {
-   int64_t rc;
-
-   /*
-* P7IOC supports M64DT, which helps mapping M64 segment
-* to one particular PE#. However, PHB3 has fixed mapping
-* between M64 segment and PE#. In order to have same logic
-* for P7IOC and PHB3, we enforce fixed mapping between M64
-* segment and PE# on P7IOC.
-*/
-   rc = opal_pci_map_pe_mmio_window(phb->opal_id,
-   index, OPAL_M64_WINDOW_TYPE,
-   index / PNV_IODA1_M64_SEGS,
-   index % PNV_IODA1_M64_SEGS);
-   if (rc != OPAL_SUCCESS) {
-   pr_warn("%s: Error %lld mapping M64 for PHB#%x-PE#%x\n",
-   __func__, rc, phb->hose->global_number,
-   index);
-   goto fail;
-   }
-   }
-
-   /*
-* Exclude the segments for reserved and root bus PE, which
-* are first or last two PEs.
-*/
-   r = >hose->mem_resources[1];
-   if (phb->ioda.reserved_pe_idx == 0)
-   r->start += (2 * phb->ioda.m64_segsize);
-   else if (phb->ioda.reserved_pe_idx == (phb->ioda.total_pe_num - 1))
-   r->end -= (2 * phb->ioda.m64_segsize);
-   else
-   WARN(1, "Wrong reserved PE#%x on PHB#%x\n",
-phb->ioda.reserved_pe_idx, phb->hose->global_number);
-
-   return 0;
-
-fail:
-   for ( ; index >= 0; index--)
-   opal_pci_phb_mmio_enable(phb->opal_id,
-   OPAL_M64_WINDOW_TYPE, index, OPAL_DISABLE_M64);
-
-   return -EIO;
-}
-
 static void pnv_ioda_reserve_m64_pe(struct pci_bus *bus,
unsigned long *pe_bitmap,
bool all)
@@ -518,10 +434,7 @@ static void __init pnv_ioda_parse_m64_window(struct 
pnv_phb *phb)
 * Setup init functions for M64 based on IODA version, IODA3 uses
 * the IODA2 code.
 */
-   if (phb->type == PNV_PH

[RFC PATCH 0/3] powernv/pci: Remove unused IODA1 support

[Joel Stanley]

Oliver mentioned this code was only kept around to support the VPL
Power7 boxes. Now that they are all gone, remove the code.

Build and boot tested in qemu only.

Joel Stanley (3):
  powernv/pci: Remove ioda1 support
  powerpc/pci: Remove MVE code
  powernv/pci: Remove last IODA1 defines

 arch/powerpc/include/asm/opal.h|   3 -
 arch/powerpc/platforms/powernv/opal-call.c |   2 -
 arch/powerpc/platforms/powernv/pci-ioda.c  | 476 +
 arch/powerpc/platforms/powernv/pci.c   |   5 -
 arch/powerpc/platforms/powernv/pci.h   |   5 -
 5 files changed, 6 insertions(+), 485 deletions(-)

-- 
2.39.2

Re: [PATCH] powerpc/crypto: Fix aes-gcm-p10 build when VSX=n

[Joel Stanley]

On Mon, 15 May 2023 at 12:48, Michael Ellerman  wrote:
>
> When VSX is disabled, eg. microwatt_defconfig, the build fails with:
>
>   In function ‘enable_kernel_vsx’,
>   inlined from ‘vsx_begin’ at arch/powerpc/crypto/aes-gcm-p10-glue.c:68:2,
>   inlined from ‘p10_aes_gcm_crypt.constprop’ at 
> arch/powerpc/crypto/aes-gcm-p10-glue.c:244:2:
>   ...
>   arch/powerpc/include/asm/switch_to.h:86:9: note: in expansion of macro 
> ‘BUILD_BUG’
>  86 | BUILD_BUG();
> | ^
>
> Fix it by making the p10-aes-gcm code depend on VSX.
>
> Signed-off-by: Michael Ellerman 

Reviewed-by: Joel Stanley 

> ---
>  arch/powerpc/crypto/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/crypto/Kconfig b/arch/powerpc/crypto/Kconfig
> index 7113f9355165..ad1872518992 100644
> --- a/arch/powerpc/crypto/Kconfig
> +++ b/arch/powerpc/crypto/Kconfig
> @@ -96,7 +96,7 @@ config CRYPTO_AES_PPC_SPE
>
>  config CRYPTO_AES_GCM_P10
> tristate "Stitched AES/GCM acceleration support on P10 or later CPU 
> (PPC)"
> -   depends on PPC64 && CPU_LITTLE_ENDIAN
> +   depends on PPC64 && CPU_LITTLE_ENDIAN && VSX

VSX depends on PPC_BOOK3S_64 but I guess there's no harm in keeping
the PPC64 here?

> select CRYPTO_LIB_AES
> select CRYPTO_ALGAPI
> select CRYPTO_AEAD
> --
> 2.40.1
>

[PATCH] KVM: PPC: Fix documentation for ppc mmu caps

[Joel Stanley]

The documentation mentions KVM_CAP_PPC_RADIX_MMU, but the defines in the
kvm headers spell it KVM_CAP_PPC_MMU_RADIX. Similarly with
KVM_CAP_PPC_MMU_HASH_V3.

Fixes: c92701322711 ("KVM: PPC: Book3S HV: Add userspace interfaces for POWER9 
MMU")
Signed-off-by: Joel Stanley 
---
There is also the copy of the uapi header in tools/ that needs updating,
but that seems to be manually kept in sync so I didn't patch that.
---
 Documentation/virt/kvm/api.rst | 8 
 include/uapi/linux/kvm.h   | 4 ++--
 tools/include/uapi/linux/kvm.h | 2 +-
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
index a5c803f39832..979a5f7b545e 100644
--- a/Documentation/virt/kvm/api.rst
+++ b/Documentation/virt/kvm/api.rst
@@ -4237,7 +4237,7 @@ operating system that uses the PIT for timing (e.g. Linux 
2.4.x).
 4.100 KVM_PPC_CONFIGURE_V3_MMU
 --
 
-:Capability: KVM_CAP_PPC_RADIX_MMU or KVM_CAP_PPC_HASH_MMU_V3
+:Capability: KVM_CAP_PPC_MMU_RADIX or KVM_CAP_PPC_MMU_HASH_V3
 :Architectures: ppc
 :Type: vm ioctl
 :Parameters: struct kvm_ppc_mmuv3_cfg (in)
@@ -4271,7 +4271,7 @@ the Power ISA V3.00, Book III section 5.7.6.1.
 4.101 KVM_PPC_GET_RMMU_INFO
 ---
 
-:Capability: KVM_CAP_PPC_RADIX_MMU
+:Capability: KVM_CAP_PPC_MMU_RADIX
 :Architectures: ppc
 :Type: vm ioctl
 :Parameters: struct kvm_ppc_rmmu_info (out)
@@ -7701,7 +7701,7 @@ capability via KVM_ENABLE_CAP ioctl on the vcpu fd. Note 
that this
 will disable the use of APIC hardware virtualization even if supported
 by the CPU, as it's incompatible with SynIC auto-EOI behavior.
 
-8.3 KVM_CAP_PPC_RADIX_MMU
+8.3 KVM_CAP_PPC_MMU_RADIX
 -
 
 :Architectures: ppc
@@ -7711,7 +7711,7 @@ available, means that the kernel can support guests using 
the
 radix MMU defined in Power ISA V3.00 (as implemented in the POWER9
 processor).
 
-8.4 KVM_CAP_PPC_HASH_MMU_V3
+8.4 KVM_CAP_PPC_MMU_HASH_V3
 ---
 
 :Architectures: ppc
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index d77aef872a0a..69f90989bfc8 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -1533,9 +1533,9 @@ struct kvm_s390_ucas_mapping {
 /* Available with KVM_CAP_SPAPR_RESIZE_HPT */
 #define KVM_PPC_RESIZE_HPT_PREPARE _IOR(KVMIO, 0xad, struct kvm_ppc_resize_hpt)
 #define KVM_PPC_RESIZE_HPT_COMMIT  _IOR(KVMIO, 0xae, struct kvm_ppc_resize_hpt)
-/* Available with KVM_CAP_PPC_RADIX_MMU or KVM_CAP_PPC_HASH_MMU_V3 */
+/* Available with KVM_CAP_PPC_MMU_RADIX or KVM_CAP_PPC_MMU_HASH_V3 */
 #define KVM_PPC_CONFIGURE_V3_MMU  _IOW(KVMIO,  0xaf, struct kvm_ppc_mmuv3_cfg)
-/* Available with KVM_CAP_PPC_RADIX_MMU */
+/* Available with KVM_CAP_PPC_MMU_RADIX */
 #define KVM_PPC_GET_RMMU_INFO_IOW(KVMIO,  0xb0, struct kvm_ppc_rmmu_info)
 /* Available with KVM_CAP_PPC_GET_CPU_CHAR */
 #define KVM_PPC_GET_CPU_CHAR _IOR(KVMIO,  0xb1, struct kvm_ppc_cpu_char)
diff --git a/tools/include/uapi/linux/kvm.h b/tools/include/uapi/linux/kvm.h
index d77aef872a0a..9fb12e04c93d 100644
--- a/tools/include/uapi/linux/kvm.h
+++ b/tools/include/uapi/linux/kvm.h
@@ -1533,7 +1533,7 @@ struct kvm_s390_ucas_mapping {
 /* Available with KVM_CAP_SPAPR_RESIZE_HPT */
 #define KVM_PPC_RESIZE_HPT_PREPARE _IOR(KVMIO, 0xad, struct kvm_ppc_resize_hpt)
 #define KVM_PPC_RESIZE_HPT_COMMIT  _IOR(KVMIO, 0xae, struct kvm_ppc_resize_hpt)
-/* Available with KVM_CAP_PPC_RADIX_MMU or KVM_CAP_PPC_HASH_MMU_V3 */
+/* Available with KVM_CAP_PPC_RADIX_MMU or KVM_CAP_PPC_MMU_HASH_V3 */
 #define KVM_PPC_CONFIGURE_V3_MMU  _IOW(KVMIO,  0xaf, struct kvm_ppc_mmuv3_cfg)
 /* Available with KVM_CAP_PPC_RADIX_MMU */
 #define KVM_PPC_GET_RMMU_INFO_IOW(KVMIO,  0xb0, struct kvm_ppc_rmmu_info)
-- 
2.39.2

Re: [PATCH v2 0/4] KVM: selftests: add powerpc support

[Joel Stanley]

On Sat, 8 Apr 2023 at 04:01, Nicholas Piggin  wrote:
>
> This series adds initial KVM selftests support for powerpc
> (64-bit, BookS, radix MMU).

This means the tests won't work on power8. Perhaps you could add
something like this?

--- a/tools/testing/selftests/kvm/lib/powerpc/processor.c
+++ b/tools/testing/selftests/kvm/lib/powerpc/processor.c
@@ -33,6 +33,8 @@ void virt_arch_pgd_alloc(struct kvm_vm *vm)
vm_paddr_t prtb, pgtb;
size_t pgd_pages;

+   TEST_REQUIRE(kvm_has_cap(KVM_CAP_PPC_MMU_RADIX));
+

>
> Since v1:
> - Update MAINTAINERS KVM PPC entry to include kvm selftests.
> - Fixes and cleanups from Sean's review including new patch 1.
> - Add 4K guest page support requiring new patch 2.
>
> Thanks,
> Nick
>
> Nicholas Piggin (4):
>   KVM: selftests: Move pgd_created check into virt_pgd_alloc
>   KVM: selftests: Add aligned guest physical page allocator
>   KVM: PPC: selftests: add support for powerpc
>   KVM: PPC: selftests: add selftests sanity tests
>
>  MAINTAINERS   |   2 +
>  tools/testing/selftests/kvm/Makefile  |  15 +
>  .../selftests/kvm/include/kvm_util_base.h |  27 ++
>  .../selftests/kvm/include/powerpc/hcall.h |  21 +
>  .../selftests/kvm/include/powerpc/ppc_asm.h   |  32 ++
>  .../selftests/kvm/include/powerpc/processor.h |  33 ++
>  .../selftests/kvm/lib/aarch64/processor.c |   4 -
>  tools/testing/selftests/kvm/lib/guest_modes.c |   3 +
>  tools/testing/selftests/kvm/lib/kvm_util.c|  56 ++-
>  .../selftests/kvm/lib/powerpc/handlers.S  |  93 
>  .../testing/selftests/kvm/lib/powerpc/hcall.c |  45 ++
>  .../selftests/kvm/lib/powerpc/processor.c | 429 ++
>  .../testing/selftests/kvm/lib/powerpc/ucall.c |  30 ++
>  .../selftests/kvm/lib/riscv/processor.c   |   4 -
>  .../selftests/kvm/lib/s390x/processor.c   |   4 -
>  .../selftests/kvm/lib/x86_64/processor.c  |   7 +-
>  tools/testing/selftests/kvm/powerpc/helpers.h |  46 ++
>  .../testing/selftests/kvm/powerpc/null_test.c | 166 +++
>  .../selftests/kvm/powerpc/rtas_hcall.c| 146 ++
>  19 files changed, 1129 insertions(+), 34 deletions(-)
>  create mode 100644 tools/testing/selftests/kvm/include/powerpc/hcall.h
>  create mode 100644 tools/testing/selftests/kvm/include/powerpc/ppc_asm.h
>  create mode 100644 tools/testing/selftests/kvm/include/powerpc/processor.h
>  create mode 100644 tools/testing/selftests/kvm/lib/powerpc/handlers.S
>  create mode 100644 tools/testing/selftests/kvm/lib/powerpc/hcall.c
>  create mode 100644 tools/testing/selftests/kvm/lib/powerpc/processor.c
>  create mode 100644 tools/testing/selftests/kvm/lib/powerpc/ucall.c
>  create mode 100644 tools/testing/selftests/kvm/powerpc/helpers.h
>  create mode 100644 tools/testing/selftests/kvm/powerpc/null_test.c
>  create mode 100644 tools/testing/selftests/kvm/powerpc/rtas_hcall.c
>
> --
> 2.40.0
>

Re: [kvm-unit-tests v3 10/13] powerpc: Add support for more interrupts including HV interrupts

[Joel Stanley]

Hi Nick,

On Mon, 27 Mar 2023 at 12:55, Nicholas Piggin  wrote:
>
> Interrupt vectors were not being populated for all architected
> interrupt types, which could lead to crashes rather than a message for
> unhandled interrupts.
>
> 0x20 sized vectors require some reworking of the code to fit. This
> also adds support for HV / HSRR type interrupts which will be used in
> a later change.
>
> Signed-off-by: Nicholas Piggin 
> ---
>  powerpc/cstart64.S | 79 ++
>  1 file changed, 65 insertions(+), 14 deletions(-)
>
> diff --git a/powerpc/cstart64.S b/powerpc/cstart64.S

> +handler_trampoline:
> +   mfctr   r0
> +   std r0,_CTR(r1)
> +
> +   ld  r0, P_HANDLER(0)
> +   mtctr   r0
> +
> +   /* nip and msr */
> +   mfsrr0  r0

I tried building the tests on a power8 box with binutils 2.34 and gas complains:

powerpc/cstart64.S: Assembler messages:
powerpc/cstart64.S:337: Error: unrecognized opcode: `mfhsrr0'
powerpc/cstart64.S:340: Error: unrecognized opcode: `mfhsrr1'

It appears this mnemonic is only supported for power10 (and were only
added in binutils 2.36):

$ git grep -i mfhsrr
opcodes/ppc-opc.c:{"mfhsrr0",   XSPR(31,339,314), XSPR_MASK, POWER10,
 EXT,{RS}},
opcodes/ppc-opc.c:{"mfhsrr1",   XSPR(31,339,315), XSPR_MASK, POWER10,
 EXT,{RS}},

I replaced it with mfspr and the tests ran fine:

@@ -334,10 +338,10 @@ handler_htrampoline:
mtctr   r0

/* nip and msr */
-   mfhsrr0 r0
+   mfspr   r0, SPRN_HSRR0
std r0, _NIP(r1)

-   mfhsrr1 r0
+   mfspr   r0, SPRN_HSRR1
std r0, _MSR(r1)

Cheers,

Joel

[PATCH] powerpc: Remove duplicate SPRN_HSRR definitions

[Joel Stanley]

There are two copies of these defines. Keep the older ones as they have
associated bit definitions.

Signed-off-by: Joel Stanley 
---
Today I learnt that if you have two copies of the define, but they are
the same value, the compiler won't warn.

 arch/powerpc/include/asm/reg.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
index 1e8b2e04e626..0bf4c506a1eb 100644
--- a/arch/powerpc/include/asm/reg.h
+++ b/arch/powerpc/include/asm/reg.h
@@ -382,8 +382,6 @@
 #define SPRN_HIOR  0x137   /* 970 Hypervisor interrupt offset */
 #define SPRN_RMOR  0x138   /* Real mode offset register */
 #define SPRN_HRMOR 0x139   /* Real mode offset register */
-#define SPRN_HSRR0 0x13A   /* Hypervisor Save/Restore 0 */
-#define SPRN_HSRR1 0x13B   /* Hypervisor Save/Restore 1 */
 #define SPRN_ASDR  0x330   /* Access segment descriptor register */
 #define SPRN_IC0x350   /* Virtual Instruction Count */
 #define SPRN_VTB   0x351   /* Virtual Time Base */
-- 
2.39.2

Re: [PATCH] powerpc: remove checks for binutils older than 2.25

[Joel Stanley]

On Thu, 19 Jan 2023 at 08:24, Masahiro Yamada  wrote:
>
> Commit e4412739472b ("Documentation: raise minimum supported version of
> binutils to 2.25") allows us to remove the checks for old binutils.
>
> There is no more user for ld-ifversion. Remove it as well.

ppc kernels fail to link with 2.27 under some configurations:

 https://github.com/linuxppc/issues/issues/388

We may want to use ld-ifversion to exclude that version.

>
> Signed-off-by: Masahiro Yamada 
> ---
>
>  arch/powerpc/Makefile | 22 +-
>  arch/powerpc/lib/Makefile |  2 +-
>  scripts/Makefile.compiler |  4 
>  3 files changed, 2 insertions(+), 26 deletions(-)
>
> diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
> index dc4cbf0a5ca9..3d265b16c0ae 100644
> --- a/arch/powerpc/Makefile
> +++ b/arch/powerpc/Makefile
> @@ -42,18 +42,13 @@ machine-$(CONFIG_PPC64) += 64
>  machine-$(CONFIG_CPU_LITTLE_ENDIAN) += le
>  UTS_MACHINE := $(subst $(space),,$(machine-y))
>
> -# XXX This needs to be before we override LD below
> -ifdef CONFIG_PPC32
> -KBUILD_LDFLAGS_MODULE += arch/powerpc/lib/crtsavres.o
> -else
> -ifeq ($(call ld-ifversion, -ge, 22500, y),y)
> +ifeq ($(CONFIG_PPC64)$(CONFIG_LD_IS_BFD),yy)
>  # Have the linker provide sfpr if possible.
>  # There is a corresponding test in arch/powerpc/lib/Makefile
>  KBUILD_LDFLAGS_MODULE += --save-restore-funcs
>  else
>  KBUILD_LDFLAGS_MODULE += arch/powerpc/lib/crtsavres.o
>  endif
> -endif
>
>  ifdef CONFIG_CPU_LITTLE_ENDIAN
>  KBUILD_CFLAGS  += -mlittle-endian
> @@ -389,8 +384,6 @@ vdso_prepare: prepare0
> $(build)=arch/powerpc/kernel/vdso 
> include/generated/vdso64-offsets.h)
>  endif
>
> -archprepare: checkbin
> -
>  archheaders:
> $(Q)$(MAKE) $(build)=arch/powerpc/kernel/syscalls all
>
> @@ -405,16 +398,3 @@ else
> $(eval KBUILD_CFLAGS += -mstack-protector-guard-offset=$(shell awk 
> '{if ($$2 == "TASK_CANARY") print $$3;}' include/generated/asm-offsets.h))
>  endif
>  endif
> -
> -PHONY += checkbin
> -# Check toolchain versions:
> -# - gcc-4.6 is the minimum kernel-wide version so nothing required.
> -checkbin:
> -   @if test "x${CONFIG_LD_IS_LLD}" != "xy" -a \
> -   "x$(call ld-ifversion, -le, 22400, y)" = "xy" ; then \
> -   echo -n '*** binutils 2.24 miscompiles weak symbols ' ; \
> -   echo 'in some circumstances.' ; \
> -   echo'*** binutils 2.23 do not define the TOC symbol ' ; \
> -   echo -n '*** Please use a different binutils version.' ; \
> -   false ; \
> -   fi
> diff --git a/arch/powerpc/lib/Makefile b/arch/powerpc/lib/Makefile
> index 4de71cbf6e8e..c53618c34b70 100644
> --- a/arch/powerpc/lib/Makefile
> +++ b/arch/powerpc/lib/Makefile
> @@ -42,7 +42,7 @@ obj-$(CONFIG_FUNCTION_ERROR_INJECTION)+= 
> error-inject.o
>  # 64-bit linker creates .sfpr on demand for final link (vmlinux),
>  # so it is only needed for modules, and only for older linkers which
>  # do not support --save-restore-funcs
> -ifeq ($(call ld-ifversion, -lt, 22500, y),y)
> +ifndef CONFIG_LD_IS_BFD
>  extra-$(CONFIG_PPC64)  += crtsavres.o
>  endif
>
> diff --git a/scripts/Makefile.compiler b/scripts/Makefile.compiler
> index 3d8adfd34af1..ad07a4efc253 100644
> --- a/scripts/Makefile.compiler
> +++ b/scripts/Makefile.compiler
> @@ -72,7 +72,3 @@ clang-min-version = $(call test-ge, 
> $(CONFIG_CLANG_VERSION), $1)
>  # ld-option
>  # Usage: KBUILD_LDFLAGS += $(call ld-option, -X, -Y)
>  ld-option = $(call try-run, $(LD) $(KBUILD_LDFLAGS) $(1) -v,$(1),$(2),$(3))
> -
> -# ld-ifversion
> -# Usage:  $(call ld-ifversion, -ge, 22252, y)
> -ld-ifversion = $(shell [ $(CONFIG_LD_VERSION)0 $(1) $(2)0 ] && echo $(3) || 
> echo $(4))
> --
> 2.34.1
>

Re: [PATCH v6 0/4] Option to build big-endian with ELFv2 ABI

[Joel Stanley]

On Mon, 28 Nov 2022 at 04:16, Nicholas Piggin  wrote:
>
> This is hopefully the final attempt. Luis was happy for the module
> patch to go via the powerpc tree, so I've put the the ELFv2 for big
> endian build patches into the series. Hopefully we can deprecate
> the ELFv1 ABI
>
> Since v5, I cleaned up patch 2 as per Christophe's review. And patch
> 4 I removed the EXPERT depends so it's easier to test. It's marked as
> experimental, but we should soon make it default and try to deprecate
> the v1 ABI so we can eventually remove it.

Reviewed-by: Joel Stanley 

I did some builds and boot tested them in qemu fine.

>
> Thanks,
> Nick
>
> Nicholas Piggin (4):
>   module: add module_elf_check_arch for module-specific checks
>   powerpc/64: Add module check for ELF ABI version
>   powerpc/64: Add big-endian ELFv2 flavour to crypto VMX asm generation
>   powerpc/64: Option to build big-endian with ELFv2 ABI
>
>  arch/powerpc/Kconfig   | 21 +
>  arch/powerpc/kernel/module_64.c| 10 ++
>  arch/powerpc/platforms/Kconfig.cputype |  4 ++--
>  drivers/crypto/vmx/Makefile| 12 +++-
>  drivers/crypto/vmx/ppc-xlate.pl| 10 ++
>  include/linux/moduleloader.h   |  3 +++
>  kernel/module/main.c   | 10 ++
>  7 files changed, 63 insertions(+), 7 deletions(-)
>
> --
> 2.37.2
>

[PATCH] powerpc: Fix 85xx build

[Joel Stanley]

The merge of the kbuild tree dropped the renaming of the FSL_BOOKE
kconfig option.

Fixes: 8afc66e8d43b ("Merge tag 'kbuild-v6.1' of 
git://git.kernel.org/pub/scm/linux/kernel/git/masahiroy/linux-kbuild")
Signed-off-by: Joel Stanley 
---
 arch/powerpc/kernel/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index 68ea30fb373e..ee2d76cb3187 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -121,7 +121,7 @@ obj-$(CONFIG_PPC_BOOK3S_32) += head_book3s_32.o
 obj-$(CONFIG_40x)  += head_40x.o
 obj-$(CONFIG_44x)  += head_44x.o
 obj-$(CONFIG_PPC_8xx)  += head_8xx.o
-obj-$(CONFIG_FSL_BOOKE)+= head_85xx.o
+obj-$(CONFIG_PPC_85xx) += head_85xx.o
 extra-y+= vmlinux.lds
 
 obj-$(CONFIG_RELOCATABLE)  += reloc_$(BITS).o
-- 
2.35.1

[PATCH] powerpc: Implement slightly better 64-bit LE non-VMX memory copy

[Joel Stanley]

From: Paul Mackerras 

At present, on 64-bit little-endian machines, we have the choice of
either a dumb loop that does one byte per iteration, or an optimized
loop using VMX instructions.  On microwatt, we don't have VMX, so
we are stuck with the dumb loop, which is very slow.

This makes the dumb loop a little less dumb.  It now does 16 bytes
per iteration, using 'ld' and 'std' instructions.  If the number of
bytes to copy is not a multiple of 16, the one-byte-per-iteration
loop is used for the last 1--15 bytes.

Signed-off-by: Paul Mackerras 
Signed-off-by: Joel Stanley 
---
 arch/powerpc/lib/memcpy_64.S | 27 +++
 1 file changed, 19 insertions(+), 8 deletions(-)

diff --git a/arch/powerpc/lib/memcpy_64.S b/arch/powerpc/lib/memcpy_64.S
index 016c91e958d8..bed7eb327b25 100644
--- a/arch/powerpc/lib/memcpy_64.S
+++ b/arch/powerpc/lib/memcpy_64.S
@@ -18,7 +18,7 @@
 _GLOBAL_TOC_KASAN(memcpy)
 BEGIN_FTR_SECTION
 #ifdef __LITTLE_ENDIAN__
-   cmpdi   cr7,r5,0
+   clrldi  r6,r5,60
 #else
std r3,-STACKFRAMESIZE+STK_REG(R31)(r1) /* save destination 
pointer for return value */
 #endif
@@ -29,13 +29,24 @@ FTR_SECTION_ELSE
 ALT_FTR_SECTION_END_IFCLR(CPU_FTR_VMX_COPY)
 #ifdef __LITTLE_ENDIAN__
/* dumb little-endian memcpy that will get replaced at runtime */
-   addi r9,r3,-1
-   addi r4,r4,-1
-   beqlr cr7
-   mtctr r5
-1: lbzu r10,1(r4)
-   stbu r10,1(r9)
-   bdnz 1b
+   addir9,r3,-8
+   addir4,r4,-8
+   srdi.   r0,r5,4
+   beq 2f
+   mtctr   r0
+3: ld  r10,8(r4)
+   std r10,8(r9)
+   ldu r10,16(r4)
+   stdur10,16(r9)
+   bdnz3b
+2: cmpwi   r6,0
+   beqlr
+   addir9,r9,7
+   addir4,r4,7
+   mtctr   r6
+1: lbzur10,1(r4)
+   stbur10,1(r9)
+   bdnz1b
blr
 #else
PPC_MTOCRF(0x01,r5)
-- 
2.35.1

[PATCH v2] powerpc/microwatt: Add litesd

[Joel Stanley]

This is the register layout of the litesd peripheral for the fusesoc
based Microwatt SoC.

It requires a description of the system clock, which is hardcoded to
100MHz.

Signed-off-by: Joel Stanley 
---
v2:
 * remove non-removable property
 * Remove status=disabled
 * Add clock
---
 arch/powerpc/boot/dts/microwatt.dts | 22 ++
 1 file changed, 22 insertions(+)

diff --git a/arch/powerpc/boot/dts/microwatt.dts 
b/arch/powerpc/boot/dts/microwatt.dts
index b69db1d275cd..269e930b3b0b 100644
--- a/arch/powerpc/boot/dts/microwatt.dts
+++ b/arch/powerpc/boot/dts/microwatt.dts
@@ -21,6 +21,14 @@ memory@0 {
reg = <0x 0x 0x 0x1000>;
};
 
+   clocks {
+   sys_clk: litex_sys_clk {
+   #clock-cells = <0>;
+   compatible = "fixed-clock";
+   clock-frequency = <1>;
+   };
+   };
+
cpus {
#size-cells = <0x00>;
#address-cells = <0x01>;
@@ -141,6 +149,20 @@ ethernet@802 {
litex,slot-size = <0x800>;
interrupts = <0x11 0x1>;
};
+
+   mmc@804 {
+   compatible = "litex,mmc";
+   reg = <0x8042800 0x800
+   0x8041000 0x800
+   0x8040800 0x800
+   0x8042000 0x800
+   0x8041800 0x800>;
+   reg-names = "phy", "core", "reader", "writer", "irq";
+   bus-width = <4>;
+   interrupts = <0x13 1>;
+   cap-sd-highspeed;
+   clocks = <_clk>;
+   };
};
 
chosen {
-- 
2.35.1

[PATCH] powerpc/microwatt: Add litesd

[Joel Stanley]

This is the register layout of the litesd peripheral for the fusesoc
based Microwatt SoC.

Signed-off-by: Joel Stanley 
---
 arch/powerpc/boot/dts/microwatt.dts | 15 +++
 1 file changed, 15 insertions(+)

diff --git a/arch/powerpc/boot/dts/microwatt.dts 
b/arch/powerpc/boot/dts/microwatt.dts
index b69db1d275cd..0a2e82ca1c79 100644
--- a/arch/powerpc/boot/dts/microwatt.dts
+++ b/arch/powerpc/boot/dts/microwatt.dts
@@ -141,6 +141,21 @@ ethernet@802 {
litex,slot-size = <0x800>;
interrupts = <0x11 0x1>;
};
+
+   mmc@804 {
+   compatible = "litex,mmc";
+   reg = <0x8042800 0x800
+   0x8041000 0x800
+   0x8040800 0x800
+   0x8042000 0x800
+   0x8041800 0x800>;
+   reg-names = "phy", "core", "reader", "writer", "irq";
+   bus-width = <4>;
+   interrupts = <0x13 1>;
+   cap-sd-highspeed;
+   non-removeable;
+   status = "disabled";
+   };
};
 
chosen {
-- 
2.35.1

Re: [PATCH] powerpc/microwatt: Remove unused early debug code

[Joel Stanley]

On Mon, 19 Sept 2022 at 05:28, Michael Ellerman  wrote:
>
> The original microwatt submission[1] included some early debug code for
> using the Microwatt "potato" UART.

The potato is indeed dead.

>
> The series that was eventually merged switched to using a standard UART,
> and so doesn't need any special early debug handling. But some of the
> original code was merged accidentally under the non-existent
> CONFIG_PPC_EARLY_DEBUG_MICROWATT.

The kconfig never got added, so you're right. Using the "legacy serial
console" must be how we get early console on microwatt? I can't quite
work it out.

May or may not be related to https://github.com/linuxppc/issues/issues/413

>
> Drop the unused code.
>
> 1: 
> https://lore.kernel.org/linuxppc-dev/20200509050340.gd1464...@thinks.paulus.ozlabs.org/
>
> Fixes: 48b545b8018d ("powerpc/microwatt: Use standard 16550 UART for console")
> Reported-by: Lukas Bulwahn 
> Signed-off-by: Michael Ellerman 
> ---
>  arch/powerpc/kernel/udbg_16550.c | 39 
>  1 file changed, 39 deletions(-)
>
> diff --git a/arch/powerpc/kernel/udbg_16550.c 
> b/arch/powerpc/kernel/udbg_16550.c
> index d3942de254c6..ddfbc74bf85f 100644
> --- a/arch/powerpc/kernel/udbg_16550.c
> +++ b/arch/powerpc/kernel/udbg_16550.c
> @@ -296,42 +296,3 @@ void __init udbg_init_40x_realmode(void)
>  }
>
>  #endif /* CONFIG_PPC_EARLY_DEBUG_40x */
> -
> -#ifdef CONFIG_PPC_EARLY_DEBUG_MICROWATT
> -
> -#define UDBG_UART_MW_ADDR  ((void __iomem *)0xc0002000)
> -
> -static u8 udbg_uart_in_isa300_rm(unsigned int reg)
> -{
> -   uint64_t msr = mfmsr();
> -   uint8_t  c;
> -
> -   mtmsr(msr & ~(MSR_EE|MSR_DR));
> -   isync();
> -   eieio();
> -   c = __raw_rm_readb(UDBG_UART_MW_ADDR + (reg << 2));
> -   mtmsr(msr);
> -   isync();
> -   return c;
> -}
> -
> -static void udbg_uart_out_isa300_rm(unsigned int reg, u8 val)
> -{
> -   uint64_t msr = mfmsr();
> -
> -   mtmsr(msr & ~(MSR_EE|MSR_DR));
> -   isync();
> -   eieio();
> -   __raw_rm_writeb(val, UDBG_UART_MW_ADDR + (reg << 2));
> -   mtmsr(msr);
> -   isync();
> -}
> -
> -void __init udbg_init_debug_microwatt(void)
> -{
> -   udbg_uart_in = udbg_uart_in_isa300_rm;
> -   udbg_uart_out = udbg_uart_out_isa300_rm;
> -   udbg_use_uart();
> -}
> -
> -#endif /* CONFIG_PPC_EARLY_DEBUG_MICROWATT */
> --
> 2.37.2
>

[PATCH] powerpc/microwatt: Add LiteX MMC driver

[Joel Stanley]

Enable the LiteX MMC device and it's dependency the common clock
framework.

Signed-off-by: Joel Stanley 
---
 arch/powerpc/configs/microwatt_defconfig | 5 +
 1 file changed, 5 insertions(+)

diff --git a/arch/powerpc/configs/microwatt_defconfig 
b/arch/powerpc/configs/microwatt_defconfig
index eff933ebbb9e..ea2dbd778aad 100644
--- a/arch/powerpc/configs/microwatt_defconfig
+++ b/arch/powerpc/configs/microwatt_defconfig
@@ -75,7 +75,12 @@ CONFIG_SPI_BITBANG=y
 CONFIG_SPI_SPIDEV=y
 # CONFIG_HWMON is not set
 # CONFIG_USB_SUPPORT is not set
+CONFIG_MMC=y
+# CONFIG_PWRSEQ_EMMC is not set
+# CONFIG_PWRSEQ_SIMPLE is not set
+CONFIG_MMC_LITEX=y
 # CONFIG_VIRTIO_MENU is not set
+CONFIG_COMMON_CLK=y
 # CONFIG_IOMMU_SUPPORT is not set
 # CONFIG_NVMEM is not set
 CONFIG_EXT4_FS=y
-- 
2.35.1

[PATCH v2] powerpc/perf: Give generic PMU a nice name

[Joel Stanley]

When booting on a machine that uses the compat pmu driver we see this:

 [0.071192] GENERIC_COMPAT performance monitor hardware support registered

Which is a bit shouty. Give it a nicer name.

Signed-off-by: Joel Stanley 
---
v2: Go with ISAv3

 arch/powerpc/perf/generic-compat-pmu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/perf/generic-compat-pmu.c 
b/arch/powerpc/perf/generic-compat-pmu.c
index f3db88aee4dd..16392962c511 100644
--- a/arch/powerpc/perf/generic-compat-pmu.c
+++ b/arch/powerpc/perf/generic-compat-pmu.c
@@ -292,7 +292,7 @@ static int generic_compute_mmcr(u64 event[], int n_ev,
 }
 
 static struct power_pmu generic_compat_pmu = {
-   .name   = "GENERIC_COMPAT",
+   .name   = "ISAv3",
.n_counter  = MAX_PMU_COUNTERS,
.add_fields = ISA207_ADD_FIELDS,
.test_adder = ISA207_TEST_ADDER,
-- 
2.35.1

Re: [PATCH] powerpc/perf: Give generic PMU a nice name

[Joel Stanley]

On Tue, 31 May 2022 at 08:53, Madhavan Srinivasan  wrote:
>
>
> On 5/26/22 12:07 PM, Joel Stanley wrote:
> > When booting on a machine that uses the compat pmu driver we see this:
> >
> >   [0.071192] GENERIC_COMPAT performance monitor hardware support 
> > registered
> Sorry that was my mistake.
> I agree having it as ISAv3 is better.

Okay. The downside of this is it's not as clear that you're using a
fallback driver.

I'll send a v2 with ISAv3

>
> Maddy
>
> >
> > Which is a bit shouty. Give it a nicer name.
> >
> > Signed-off-by: Joel Stanley 
> > ---
> >
> > Other options:
> >
> >   - ISAv3 (because it is relevant for PowerISA 3.0B and beyond, see the
> > comment in init_generic_compat_pmu)
> >
> >   - Generic Compat (same, but less shouty)
> >
> >   arch/powerpc/perf/generic-compat-pmu.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/powerpc/perf/generic-compat-pmu.c 
> > b/arch/powerpc/perf/generic-compat-pmu.c
> > index f3db88aee4dd..5be5a5ebaf42 100644
> > --- a/arch/powerpc/perf/generic-compat-pmu.c
> > +++ b/arch/powerpc/perf/generic-compat-pmu.c
> > @@ -292,7 +292,7 @@ static int generic_compute_mmcr(u64 event[], int n_ev,
> >   }
> >
> >   static struct power_pmu generic_compat_pmu = {
> > - .name   = "GENERIC_COMPAT",
> > + .name   = "Architected",
> >   .n_counter  = MAX_PMU_COUNTERS,
> >   .add_fields = ISA207_ADD_FIELDS,
> >   .test_adder = ISA207_TEST_ADDER,

[PATCH] powerpc/perf: Give generic PMU a nice name

[Joel Stanley]

When booting on a machine that uses the compat pmu driver we see this:

 [0.071192] GENERIC_COMPAT performance monitor hardware support registered

Which is a bit shouty. Give it a nicer name.

Signed-off-by: Joel Stanley 
---

Other options:

 - ISAv3 (because it is relevant for PowerISA 3.0B and beyond, see the
   comment in init_generic_compat_pmu)

 - Generic Compat (same, but less shouty)

 arch/powerpc/perf/generic-compat-pmu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/perf/generic-compat-pmu.c 
b/arch/powerpc/perf/generic-compat-pmu.c
index f3db88aee4dd..5be5a5ebaf42 100644
--- a/arch/powerpc/perf/generic-compat-pmu.c
+++ b/arch/powerpc/perf/generic-compat-pmu.c
@@ -292,7 +292,7 @@ static int generic_compute_mmcr(u64 event[], int n_ev,
 }
 
 static struct power_pmu generic_compat_pmu = {
-   .name   = "GENERIC_COMPAT",
+   .name   = "Architected",
.n_counter  = MAX_PMU_COUNTERS,
.add_fields = ISA207_ADD_FIELDS,
.test_adder = ISA207_TEST_ADDER,
-- 
2.35.1

Re: [PATCH] powerpc/microwatt: Add mmu bits to device tree

[Joel Stanley]

On Fri, 20 May 2022 at 00:06, Nicholas Piggin  wrote:
>
> Excerpts from Joel Stanley's message of May 19, 2022 10:57 pm:
> > In commit 5402e239d09f ("powerpc/64s: Get LPID bit width from device
> > tree") the kernel tried to determine the pid and lpid bits from the
> > device tree. If they are not found, there is a fallback, but Microwatt
> > wasn't covered as has the unusual configuration of being both !HV and bare
> > metal.
> >
> > Set the values in the device tree to avoid having to add a special case.
> > The lpid value is the only one required, but add both for completeness.
> >
> > Fixes: 5402e239d09f ("powerpc/64s: Get LPID bit width from device tree")
> > Signed-off-by: Joel Stanley 
>
> LGTM... does Microwatt actually need 12 lpid bits, or does it just need
> the LPID 0 partition table entry? I wonder if you set it to 1. That's
> a minor nit though.

0, 1, 4, 5, 6, 7 fails. 8, 9, 10, 11, 12 work.

I'll let someone else explain why that is!

Oh, it's because we use it like this:

arch/powerpc/include/asm/book3s/64/mmu.h:#define PATB_SIZE_SHIFT
 (mmu_lpid_bits + 4)

and then

arch/powerpc/mm/book3s64/radix_pgtable.c:
set_ptcr_when_no_uv(__pa(partition_tb) |
arch/powerpc/mm/book3s64/radix_pgtable.c:
 (PATB_SIZE_SHIFT - 12));

 so anything less than 8 will result in a negative value there.

The ISA says (from Figure 22 in 3.1, or 21 in 3.0B):

 Partition Table Size=2**(12+PATS), PATS<= 24.

>From this it's clear that 12 >= mmu_lpid_bits >= 24.

(except there's only 4 bits for PATS in the ISA, so really the maximum is 16?)

What's not clear to me is why we define PATB_SIZE_SHIFT as mmu_lpid_bits + 4.

Cheers,

Joel

>
> Acked-by: Nicholas Piggin 
>
> > ---
> >  arch/powerpc/boot/dts/microwatt.dts | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/arch/powerpc/boot/dts/microwatt.dts 
> > b/arch/powerpc/boot/dts/microwatt.dts
> > index 65b270a90f94..b69db1d275cd 100644
> > --- a/arch/powerpc/boot/dts/microwatt.dts
> > +++ b/arch/powerpc/boot/dts/microwatt.dts
> > @@ -90,6 +90,8 @@ PowerPC,Microwatt@0 {
> >   64-bit;
> >   d-cache-size = <0x1000>;
> >   ibm,chip-id = <0>;
> > + ibm,mmu-lpid-bits = <12>;
> > + ibm,mmu-pid-bits = <20>;
> >   };
> >   };
> >
> > --
> > 2.35.1
> >
> >

[PATCH] powerpc/microwatt: Add mmu bits to device tree

[Joel Stanley]

In commit 5402e239d09f ("powerpc/64s: Get LPID bit width from device
tree") the kernel tried to determine the pid and lpid bits from the
device tree. If they are not found, there is a fallback, but Microwatt
wasn't covered as has the unusual configuration of being both !HV and bare
metal.

Set the values in the device tree to avoid having to add a special case.
The lpid value is the only one required, but add both for completeness.

Fixes: 5402e239d09f ("powerpc/64s: Get LPID bit width from device tree")
Signed-off-by: Joel Stanley 
---
 arch/powerpc/boot/dts/microwatt.dts | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/powerpc/boot/dts/microwatt.dts 
b/arch/powerpc/boot/dts/microwatt.dts
index 65b270a90f94..b69db1d275cd 100644
--- a/arch/powerpc/boot/dts/microwatt.dts
+++ b/arch/powerpc/boot/dts/microwatt.dts
@@ -90,6 +90,8 @@ PowerPC,Microwatt@0 {
64-bit;
d-cache-size = <0x1000>;
ibm,chip-id = <0>;
+   ibm,mmu-lpid-bits = <12>;
+   ibm,mmu-pid-bits = <20>;
};
};
 
-- 
2.35.1

Re: [PATCH] powerpc: fix typos in comments

[Joel Stanley]

On Sat, 30 Apr 2022 at 18:58, Julia Lawall  wrote:
>
> Various spelling mistakes in comments.
> Detected with the help of Coccinelle.
>
> Signed-off-by: Julia Lawall 

I read the patch and it appears that all of the corrections are good.
Thanks for sending it Julia.

I'm not sure that one mega patch is the right way to go or not, so
I'll leave that to mpe.

Reviewed-by: Joel Stanley 

>
> ---
>  arch/powerpc/boot/cuboot-hotfoot.c  |2 +-
>  arch/powerpc/crypto/aes-spe-glue.c  |2 +-
>  arch/powerpc/kernel/cputable.c  |2 +-
>  arch/powerpc/kernel/dawr.c  |2 +-
>  arch/powerpc/kernel/eeh.c   |4 ++--
>  arch/powerpc/kernel/eeh_event.c |2 +-
>  arch/powerpc/kernel/fadump.c|   10 +-
>  arch/powerpc/kernel/module_32.c |2 +-
>  arch/powerpc/kernel/module_64.c |4 ++--
>  arch/powerpc/kernel/pci-common.c|2 +-
>  arch/powerpc/kernel/pci_of_scan.c   |2 +-
>  arch/powerpc/kernel/process.c   |4 ++--
>  arch/powerpc/kernel/prom_init.c |2 +-
>  arch/powerpc/kernel/ptrace/ptrace-view.c|2 +-
>  arch/powerpc/kernel/rtas_flash.c|2 +-
>  arch/powerpc/kernel/setup-common.c  |2 +-
>  arch/powerpc/kernel/signal_64.c |2 +-
>  arch/powerpc/kernel/smp.c   |2 +-
>  arch/powerpc/kernel/time.c  |4 ++--
>  arch/powerpc/kernel/watchdog.c  |2 +-
>  arch/powerpc/kexec/core_64.c|2 +-
>  arch/powerpc/kvm/book3s_64_mmu_hv.c |2 +-
>  arch/powerpc/kvm/book3s_64_vio_hv.c |2 +-
>  arch/powerpc/kvm/book3s_emulate.c   |2 +-
>  arch/powerpc/kvm/book3s_hv_p9_entry.c   |2 +-
>  arch/powerpc/kvm/book3s_hv_uvmem.c  |2 +-
>  arch/powerpc/kvm/book3s_pr.c|2 +-
>  arch/powerpc/kvm/book3s_xics.c  |2 +-
>  arch/powerpc/kvm/book3s_xive.c  |6 +++---
>  arch/powerpc/kvm/e500mc.c   |2 +-
>  arch/powerpc/mm/book3s64/hash_pgtable.c |2 +-
>  arch/powerpc/mm/book3s64/hash_utils.c   |4 ++--
>  arch/powerpc/mm/book3s64/pgtable.c  |2 +-
>  arch/powerpc/mm/book3s64/radix_pgtable.c|2 +-
>  arch/powerpc/mm/book3s64/radix_tlb.c|2 +-
>  arch/powerpc/mm/book3s64/slb.c  |4 ++--
>  arch/powerpc/mm/init_64.c   |4 ++--
>  arch/powerpc/mm/nohash/book3e_hugetlbpage.c |2 +-
>  arch/powerpc/mm/nohash/kaslr_booke.c|2 +-
>  arch/powerpc/mm/pgtable-frag.c  |2 +-
>  arch/powerpc/perf/8xx-pmu.c |2 +-
>  arch/powerpc/perf/core-book3s.c |6 +++---
>  arch/powerpc/perf/imc-pmu.c |4 ++--
>  arch/powerpc/perf/isa207-common.c   |6 +++---
>  arch/powerpc/platforms/512x/clock-commonclk.c   |2 +-
>  arch/powerpc/platforms/512x/mpc512x_shared.c|2 +-
>  arch/powerpc/platforms/52xx/mpc52xx_common.c|2 +-
>  arch/powerpc/platforms/52xx/mpc52xx_gpt.c   |2 +-
>  arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c   |2 +-
>  arch/powerpc/platforms/85xx/mpc85xx_cds.c   |2 +-
>  arch/powerpc/platforms/86xx/gef_ppc9a.c |2 +-
>  arch/powerpc/platforms/86xx/gef_sbc310.c|2 +-
>  arch/powerpc/platforms/86xx/gef_sbc610.c|2 +-
>  arch/powerpc/platforms/book3s/vas-api.c |2 +-
>  arch/powerpc/platforms/cell/cbe_regs.c  |2 +-
>  arch/powerpc/platforms/cell/iommu.c |2 +-
>  arch/powerpc/platforms/cell/spider-pci.c|2 +-
>  arch/powerpc/platforms/cell/spu_manage.c|2 +-
>  arch/powerpc/platforms/powermac/low_i2c.c   |2 +-
>  arch/powerpc/platforms/powernv/eeh-powernv.c|   10 +-
>  arch/powerpc/platforms/powernv/idle.c   |4 ++--
>  arch/powerpc/platforms/powernv/ocxl.c   |2 +-
>  arch/powerpc/platforms/powernv/opal-fadump.c|2 +-
>  arch/powerpc/platforms/powernv/opal-lpc.c   |2 +-
>  arch/powerpc/platforms/powernv/opal-memory-errors.c |2 +-
>  arch/powerpc/platforms/powernv/pci-sriov.c  |2 +-
>  arch/powerpc/platforms/ps3/mm.c

Re: [PATCH] powerpc/boot: Build wrapper for an appropriate CPU

[Joel Stanley]

On Thu, 31 Mar 2022 at 23:46, Segher Boessenkool
 wrote:
>
> On Thu, Mar 31, 2022 at 12:19:52PM -0300, Murilo Opsfelder Araújo wrote:
> > My understanding is that the default cpu type for -mcpu=powerpc64 can
> > change.
>
> Different subtargets (Linux, AIX, Darwin, the various BSDs, bare ELF,
> etc.) have different default CPUs.  It also can be set at configure time
> for most subtargets.
>
> Linux can be built with compilers not targetting *-linux*, so it would
> be best to specify a specific CPU always.
>
> > >I did a little test using my buildroot compiler which has
> > >with-cpu=power10. I used the presence of PCREL relocations as evidence
> > >that it was build for power10.
> > >
> > >$ powerpc64le-buildroot-linux-gnu-gcc -mcpu=power10 -c test.c
> > >$ readelf -r test.o |grep -c PCREL
> > >24
> >
> > It respected the -mcpu=power10 you provided.
>
> Of course.
>
> > And that's my concern.  We're relying on the compiler default cpu type.
>
> That is not the compiler default.  It is the default from who built the
> compiler.  It can vary wildly and unpredictably.
>
> The actual compiler default will not change so easily at all, basically
> only when its subtarget drops support for an older CPU.
>
> > If gcc defaults -mcpu=powerpc64le to power10, you're going to have
> > the same problem again.
>
> That will not happen before power10 is the minimum supported CPU.
> Anything else is madness.

Murilo, does Segher's explanation address your concerns?

I believe the patch I sent fixes the problem that you're worried
about. It should be compatible into the future too.

Cheers,

Joel

>
> > It happens that the power8 default cpu type
> > is compatible to your system by coincidence.
>
> No, power8 is (and always was) the minimum supported CPU type for
> powerpc64le-linux.
>
> > In gcc/config/rs6000/rs6000-cpus.def, they are set to different processors:
> >
> > 254 RS6000_CPU ("powerpc64", PROCESSOR_POWERPC64, MASK_PPC_GFXOPT |
> > MASK_POWERPC64)
> > 255 RS6000_CPU ("powerpc64le", PROCESSOR_POWER8, MASK_POWERPC64 |
> > ISA_2_7_MASKS_SERVER
>
> Those can and will change though, over time.  But -mcpu=powerpc64 (etc.)
> always will mean what the current documentation says it does:
>  '-mcpu=powerpc', '-mcpu=powerpc64', and '-mcpu=powerpc64le' specify
>  pure 32-bit PowerPC (either endian), 64-bit big endian PowerPC and
>  64-bit little endian PowerPC architecture machine types, with an
>  appropriate, generic processor model assumed for scheduling
>  purposes.
>
> > My suggestion was to explicitly set -mcpu=power8 instead of
> > -mcpu=powerpc64le.
>
> That is implied anyway, it is the minimum supported for
> powerpc64le-linux.  Using -mcpu=powerpc64le might schedule better for
> newer CPUs, in the future (but the code will always work on all still
> supported CPUs).
>
>
> Segher

Re: [PATCH v2 1/2] powerpc/powernv: Get L1D flush requirements from device-tree

[Joel Stanley]

On Tue, 5 Apr 2022 at 06:13, Michael Ellerman  wrote:
>
> Joel Stanley  writes:
> > On Mon, 4 Apr 2022 at 10:15, Russell Currey  wrote:
> >>
> >> The device-tree properties no-need-l1d-flush-msr-pr-1-to-0 and
> >> no-need-l1d-flush-kernel-on-user-access are the equivalents of
> >> H_CPU_BEHAV_NO_L1D_FLUSH_ENTRY and H_CPU_BEHAV_NO_L1D_FLUSH_UACCESS
> >> from the H_GET_CPU_CHARACTERISTICS hcall on pseries respectively.
> >>
> >> In commit d02fa40d759f ("powerpc/powernv: Remove POWER9 PVR version
> >> check for entry and uaccess flushes") the condition for disabling the
> >> L1D flush on kernel entry and user access was changed from any non-P9
> >> CPU to only checking P7 and P8.  Without the appropriate device-tree
> >> checks for newer processors on powernv, these flushes are unnecessarily
> >> enabled on those systems.  This patch corrects this.
> >>
> >> Fixes: d02fa40d759f ("powerpc/powernv: Remove POWER9 PVR version check for 
> >> entry and uaccess flushes")
> >> Reported-by: Joel Stanley 
> >> Signed-off-by: Russell Currey 
> >
> > I booted both patches in this series on a power10 powernv machine,
> > applied on top of v5.18-rc1:
> >
> > $ dmesg |grep -i flush
> > [0.00] rfi-flush: fallback displacement flush available
> > [0.00] rfi-flush: patched 12 locations (no flush)
> > [0.00] count-cache-flush: flush disabled.
> > [0.00] link-stack-flush: flush disabled.
> >
> > $ grep . /sys/devices/system/cpu/vulnerabilities/*
> > /sys/devices/system/cpu/vulnerabilities/itlb_multihit:Not affected
> > /sys/devices/system/cpu/vulnerabilities/l1tf:Not affected
> > /sys/devices/system/cpu/vulnerabilities/mds:Not affected
> > /sys/devices/system/cpu/vulnerabilities/meltdown:Not affected
> > /sys/devices/system/cpu/vulnerabilities/spec_store_bypass:Not affected
> > /sys/devices/system/cpu/vulnerabilities/spectre_v1:Mitigation: __user
> > pointer sanitization, ori31 speculation barrier enabled
> > /sys/devices/system/cpu/vulnerabilities/spectre_v2:Mitigation:
> > Software count cache flush (hardware accelerated), Software link stack
> > flush
> > /sys/devices/system/cpu/vulnerabilities/srbds:Not affected
> > /sys/devices/system/cpu/vulnerabilities/tsx_async_abort:Not affected
> >
> > Does that match what we expect?
>
> AFAIK yes. Happy for ruscur to correct me though.
>
> Can you also try running the kernel selftests under
> tools/testing/selftests/powerpc/security/ ?

Here's the results when booted with no_spectrev2 (which I keep on
doing by accident, as this machine has it in it's nvram):

make[1]: Entering directory
'/home/joel/dev/kernels/linus/tools/testing/selftests/powerpc/security'
TAP version 13
1..5
# selftests: powerpc/security: rfi_flush
# test: rfi_flush_test
# tags: git_version:v5.18-rc1-0-g312310928417
# PASS (L1D misses with rfi_flush=0: 63101900 < 9500) [10/10 pass]
# PASS (L1D misses with rfi_flush=1: 196001003 > 19000) [10/10 pass]
# success: rfi_flush_test
ok 1 selftests: powerpc/security: rfi_flush
# selftests: powerpc/security: entry_flush
# test: entry_flush_test
# tags: git_version:v5.18-rc1-0-g312310928417
# PASS (L1D misses with entry_flush=0: 52766044 < 9500) [10/10 pass]
# PASS (L1D misses with entry_flush=1: 196082833 > 19000) [10/10 pass]
# success: entry_flush_test
ok 2 selftests: powerpc/security: entry_flush
# selftests: powerpc/security: uaccess_flush
# test: uaccess_flush_test
# tags: git_version:v5.18-rc1-0-g312310928417
# PASS (L1D misses with uaccess_flush=0: 68646267 < 9500) [10/10 pass]
# PASS (L1D misses with uaccess_flush=1: 194177355 > 19000) [10/10 pass]
# success: uaccess_flush_test
ok 3 selftests: powerpc/security: uaccess_flush
# selftests: powerpc/security: spectre_v2
# test: spectre_v2
# tags: git_version:v5.18-rc1-0-g312310928417
# sysfs reports: 'Vulnerable'
#  PM_BR_PRED_CCACHE: result  0 running/enabled 2090650862
# PM_BR_MPRED_CCACHE: result  1 running/enabled 2090648016
# Miss percent 0 %
# OK - Measured branch prediction rates match reported spectre v2 mitigation.
# success: spectre_v2
ok 4 selftests: powerpc/security: spectre_v2
# selftests: powerpc/security: mitigation-patching.sh
# Spawned threads enabling/disabling mitigations ...
# Waiting for timeout ...
# OK
ok 5 selftests: powerpc/security: mitigation-patching.sh


Here's the same thing without the command line option set:

make[1]: Entering directory
'/home/joel/dev/kernels/linus/tools/testing/selftests/powerpc/security'
TAP version 13
1..5
# selftests: powerpc/security: rfi_flush
# test: rfi_flush_test
# tags: git_version:v5.18-rc1-0-g312310928417
# PASS (L1D misses

Re: [PATCH v2 1/2] powerpc/powernv: Get L1D flush requirements from device-tree

[Joel Stanley]

On Mon, 4 Apr 2022 at 10:15, Russell Currey  wrote:
>
> The device-tree properties no-need-l1d-flush-msr-pr-1-to-0 and
> no-need-l1d-flush-kernel-on-user-access are the equivalents of
> H_CPU_BEHAV_NO_L1D_FLUSH_ENTRY and H_CPU_BEHAV_NO_L1D_FLUSH_UACCESS
> from the H_GET_CPU_CHARACTERISTICS hcall on pseries respectively.
>
> In commit d02fa40d759f ("powerpc/powernv: Remove POWER9 PVR version
> check for entry and uaccess flushes") the condition for disabling the
> L1D flush on kernel entry and user access was changed from any non-P9
> CPU to only checking P7 and P8.  Without the appropriate device-tree
> checks for newer processors on powernv, these flushes are unnecessarily
> enabled on those systems.  This patch corrects this.
>
> Fixes: d02fa40d759f ("powerpc/powernv: Remove POWER9 PVR version check for 
> entry and uaccess flushes")
> Reported-by: Joel Stanley 
> Signed-off-by: Russell Currey 

I booted both patches in this series on a power10 powernv machine,
applied on top of v5.18-rc1:

$ dmesg |grep -i flush
[0.00] rfi-flush: fallback displacement flush available
[0.00] rfi-flush: patched 12 locations (no flush)
[0.00] count-cache-flush: flush disabled.
[0.00] link-stack-flush: flush disabled.

$ grep . /sys/devices/system/cpu/vulnerabilities/*
/sys/devices/system/cpu/vulnerabilities/itlb_multihit:Not affected
/sys/devices/system/cpu/vulnerabilities/l1tf:Not affected
/sys/devices/system/cpu/vulnerabilities/mds:Not affected
/sys/devices/system/cpu/vulnerabilities/meltdown:Not affected
/sys/devices/system/cpu/vulnerabilities/spec_store_bypass:Not affected
/sys/devices/system/cpu/vulnerabilities/spectre_v1:Mitigation: __user
pointer sanitization, ori31 speculation barrier enabled
/sys/devices/system/cpu/vulnerabilities/spectre_v2:Mitigation:
Software count cache flush (hardware accelerated), Software link stack
flush
/sys/devices/system/cpu/vulnerabilities/srbds:Not affected
/sys/devices/system/cpu/vulnerabilities/tsx_async_abort:Not affected

Does that match what we expect?

Cheers,

Joel

> ---
>  arch/powerpc/platforms/powernv/setup.c | 6 ++
>  1 file changed, 6 insertions(+)
>
> diff --git a/arch/powerpc/platforms/powernv/setup.c 
> b/arch/powerpc/platforms/powernv/setup.c
> index 105d889abd51..378f7e5f18d2 100644
> --- a/arch/powerpc/platforms/powernv/setup.c
> +++ b/arch/powerpc/platforms/powernv/setup.c
> @@ -96,6 +96,12 @@ static void __init init_fw_feat_flags(struct device_node 
> *np)
>
> if (fw_feature_is("disabled", "needs-spec-barrier-for-bound-checks", 
> np))
> security_ftr_clear(SEC_FTR_BNDS_CHK_SPEC_BAR);
> +
> +   if (fw_feature_is("enabled", "no-need-l1d-flush-msr-pr-1-to-0", np))
> +   security_ftr_clear(SEC_FTR_L1D_FLUSH_ENTRY);
> +
> +   if (fw_feature_is("enabled", 
> "no-need-l1d-flush-kernel-on-user-access", np))
> +   security_ftr_clear(SEC_FTR_L1D_FLUSH_UACCESS);
>  }
>
>  static void __init pnv_setup_security_mitigations(void)
> --
> 2.35.1
>

Re: [PATCH] powerpc/boot: Build wrapper for an appropriate CPU

[Joel Stanley]

On Thu, 31 Mar 2022 at 02:05, Murilo Opsfelder Araújo
 wrote:
>
> Hi, Joel.
>
> On 3/30/22 08:24, Joel Stanley wrote:
> > Currently the boot wrapper lacks a -mcpu option, so it will be built for
> > the toolchain's default cpu. This is a problem if the toolchain defaults
> > to a cpu with newer instructions.
> >
> > We could wire in TARGET_CPU but instead use the oldest supported option
> > so the wrapper runs anywhere.
> >
> > The GCC documentation stays that -mcpu=powerpc64le will give us a
> > generic 64 bit powerpc machine:
> >
> >   -mcpu=powerpc, -mcpu=powerpc64, and -mcpu=powerpc64le specify pure
> >   32-bit PowerPC (either endian), 64-bit big endian PowerPC and 64-bit
> >   little endian PowerPC architecture machine types, with an appropriate,
> >   generic processor model assumed for scheduling purposes.
> >
> > So do that for each of the three machines.
> >
> > This bug was found when building the kernel with a toolchain that
> > defaulted to powre10, resulting in a pcrel enabled wrapper which fails
> > to link:
> >
> >   arch/powerpc/boot/wrapper.a(crt0.o): in function `p_base':
> >   (.text+0x150): call to `platform_init' lacks nop, can't restore toc; (toc 
> > save/adjust stub)
> >   (.text+0x154): call to `start' lacks nop, can't restore toc; (toc 
> > save/adjust stub)
> >   powerpc64le-buildroot-linux-gnu-ld: final link failed: bad value
> >
> > Even with tha bug worked around the resulting kernel would crash on a
> > power9 box:
> >
> >   $ qemu-system-ppc64 -nographic -nodefaults -M powernv9 -kernel 
> > arch/powerpc/boot/zImage.epapr -serial mon:stdio
> >   [7.069331356,5] INIT: Starting kernel at 0x20010020, fdt at 
> > 0x3068c628 25694 bytes
> >   [7.130374661,3] ***
> >   [7.131072886,3] Fatal Exception 0xe40 at 200101e4MSR 
> > 9001
> >   [7.131290613,3] CFAR : 2001027c MSR  : 9001
> >   [7.131433759,3] SRR0 : 20010050 SRR1 : 9001
> >   [7.13155,3] HSRR0: 200101e4 HSRR1: 9001
> >   [7.131733687,3] DSISR: 0000 DAR  : 
> >   [7.131905162,3] LR   : 20010280 CTR  : 
> >   [7.132068356,3] CR   : 44002004 XER  : 
> >
> > Link: https://github.com/linuxppc/issues/issues/400
> > Signed-off-by: Joel Stanley 
> > ---
> > Tested:
> >
> >   - ppc64le_defconfig
> >   - pseries and powernv qemu, for power8, power9, power10 cpus
> >   - buildroot compiler that defaults to -mcpu=power10 (gcc 10.3.0, ld 
> > 2.36.1)
> >   -  RHEL9 cross compilers (gcc 11.2.1-1, ld 2.35.2-17.el9)
> >
> > All decompressed and made it into the kernel ok.
> >
> > ppc64_defconfig did not work, as we've got a regression when the wrapper
> > is built for big endian. It hasn't worked for zImage.pseries for a long
> > time (at least v4.14), and broke some time between v5.4 and v5.17 for
> > zImage.epapr.
> >
> >   arch/powerpc/boot/Makefile | 8 ++--
> >   1 file changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/powerpc/boot/Makefile b/arch/powerpc/boot/Makefile
> > index 9993c6256ad2..1f5cc401bfc0 100644
> > --- a/arch/powerpc/boot/Makefile
> > +++ b/arch/powerpc/boot/Makefile
> > @@ -38,9 +38,13 @@ BOOTCFLAGS:= -Wall -Wundef -Wstrict-prototypes 
> > -Wno-trigraphs \
> >$(LINUXINCLUDE)
> >
> >   ifdef CONFIG_PPC64_BOOT_WRAPPER
> > -BOOTCFLAGS   += -m64
> > +ifdef CONFIG_CPU_LITTLE_ENDIAN
> > +BOOTCFLAGS   += -m64 -mcpu=powerpc64le
> >   else
> > -BOOTCFLAGS   += -m32
> > +BOOTCFLAGS   += -m64 -mcpu=powerpc64
> > +endif
> > +else
> > +BOOTCFLAGS   += -m32 -mcpu=powerpc
> >   endif
> >
> >   BOOTCFLAGS  += -isystem $(shell $(BOOTCC) -print-file-name=include)
>
> I think it was a fortunate coincidence that the default cpu type of your gcc 
> is
> compatible with your system.  If the distro gcc moves its default to a newer 
> cpu
> type than your system, this bug would happen again.

Perhaps I needed to be clear in my commit message: that's the exact
bug I'm looking to avoid. I have a buildroot toolchain that was built
for -mcpu=power10.

I think you're suggesting the -mcpu=powerpc64 option will change it 's
behavior depending on the default. From my reading of the man page, I
don't think that's true.

I did a little test using my buildroot compiler which has
with-cpu=power10. I used the presence of PCREL

Re: [PATCH] powerpc/boot: Build wrapper for an appropriate CPU

[Joel Stanley]

On Wed, 30 Mar 2022 at 11:33, Christophe Leroy
 wrote:
>
>
>
> Le 30/03/2022 à 13:24, Joel Stanley a écrit :
> > Currently the boot wrapper lacks a -mcpu option, so it will be built for
> > the toolchain's default cpu. This is a problem if the toolchain defaults
> > to a cpu with newer instructions.
> >
> > We could wire in TARGET_CPU but instead use the oldest supported option
> > so the wrapper runs anywhere.
> >
> > The GCC documentation stays that -mcpu=powerpc64le will give us a
> > generic 64 bit powerpc machine:
> >
> >   -mcpu=powerpc, -mcpu=powerpc64, and -mcpu=powerpc64le specify pure
> >   32-bit PowerPC (either endian), 64-bit big endian PowerPC and 64-bit
> >   little endian PowerPC architecture machine types, with an appropriate,
> >   generic processor model assumed for scheduling purposes.
> >
> > So do that for each of the three machines.
> >
> > This bug was found when building the kernel with a toolchain that
> > defaulted to powre10, resulting in a pcrel enabled wrapper which fails
> > to link:
> >
> >   arch/powerpc/boot/wrapper.a(crt0.o): in function `p_base':
> >   (.text+0x150): call to `platform_init' lacks nop, can't restore toc; (toc 
> > save/adjust stub)
> >   (.text+0x154): call to `start' lacks nop, can't restore toc; (toc 
> > save/adjust stub)
> >   powerpc64le-buildroot-linux-gnu-ld: final link failed: bad value
> >
> > Even with tha bug worked around the resulting kernel would crash on a
> > power9 box:
> >
> >   $ qemu-system-ppc64 -nographic -nodefaults -M powernv9 -kernel 
> > arch/powerpc/boot/zImage.epapr -serial mon:stdio
> >   [7.069331356,5] INIT: Starting kernel at 0x20010020, fdt at 
> > 0x3068c628 25694 bytes
> >   [7.130374661,3] ***
> >   [7.131072886,3] Fatal Exception 0xe40 at 200101e4MSR 
> > 9001
> >   [7.131290613,3] CFAR : 2001027c MSR  : 9001
> >   [7.131433759,3] SRR0 : 20010050 SRR1 : 9001
> >   [7.13155,3] HSRR0: 200101e4 HSRR1: 9001
> >   [7.131733687,3] DSISR: 0000 DAR  : 
> >   [7.131905162,3] LR   : 20010280 CTR  : 
> >   [7.132068356,3] CR   : 44002004 XER  : 
> >
> > Link: https://github.com/linuxppc/issues/issues/400
> > Signed-off-by: Joel Stanley 
> > ---
> > Tested:
> >
> >   - ppc64le_defconfig
> >   - pseries and powernv qemu, for power8, power9, power10 cpus
> >   - buildroot compiler that defaults to -mcpu=power10 (gcc 10.3.0, ld 
> > 2.36.1)
> >   -  RHEL9 cross compilers (gcc 11.2.1-1, ld 2.35.2-17.el9)
> >
> > All decompressed and made it into the kernel ok.
> >
> > ppc64_defconfig did not work, as we've got a regression when the wrapper
> > is built for big endian. It hasn't worked for zImage.pseries for a long
> > time (at least v4.14), and broke some time between v5.4 and v5.17 for
> > zImage.epapr.
> >
> >   arch/powerpc/boot/Makefile | 8 ++--
> >   1 file changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/powerpc/boot/Makefile b/arch/powerpc/boot/Makefile
> > index 9993c6256ad2..1f5cc401bfc0 100644
> > --- a/arch/powerpc/boot/Makefile
> > +++ b/arch/powerpc/boot/Makefile
> > @@ -38,9 +38,13 @@ BOOTCFLAGS:= -Wall -Wundef -Wstrict-prototypes 
> > -Wno-trigraphs \
> >$(LINUXINCLUDE)
> >
> >   ifdef CONFIG_PPC64_BOOT_WRAPPER
> > -BOOTCFLAGS   += -m64
> > +ifdef CONFIG_CPU_LITTLE_ENDIAN
> > +BOOTCFLAGS   += -m64 -mcpu=powerpc64le
> >   else
> > -BOOTCFLAGS   += -m32
> > +BOOTCFLAGS   += -m64 -mcpu=powerpc64
> > +endif
> > +else
> > +BOOTCFLAGS   += -m32 -mcpu=powerpc
>
> How does that interracts with the following lines ? Isn't it an issue to
> have two -mcpu ?
>
> arch/powerpc/boot/Makefile:$(obj)/4xx.o: BOOTCFLAGS += -mcpu=405
> arch/powerpc/boot/Makefile:$(obj)/ebony.o: BOOTCFLAGS += -mcpu=440
> arch/powerpc/boot/Makefile:$(obj)/cuboot-hotfoot.o: BOOTCFLAGS += -mcpu=405
> arch/powerpc/boot/Makefile:$(obj)/cuboot-taishan.o: BOOTCFLAGS += -mcpu=440
> arch/powerpc/boot/Makefile:$(obj)/cuboot-katmai.o: BOOTCFLAGS += -mcpu=440
> arch/powerpc/boot/Makefile:$(obj)/cuboot-acadia.o: BOOTCFLAGS += -mcpu=405
> arch/powerpc/boot/Makefile:$(obj)/treeboot-iss4xx.o: BOOTCFLAGS += -mcpu=405
> arch/powerpc/boot/Makefile:$(obj)/treeboot-currituck.o: BOOTCFLAGS +=
> -mcpu=405
> arch/powerpc/boot/Makefile:$(obj)/treeboot-akebono.o: BOOTCFLAGS +=
> -mcpu=405

Good point, I didn't test the other wrappers.

Last one wins as far as -mcpu lines goes, from a quick test. But it
might lead to less confusion if I dropped the -mcpu=powerpc change.

[PATCH] powerpc/boot: Build wrapper for an appropriate CPU

[Joel Stanley]

Currently the boot wrapper lacks a -mcpu option, so it will be built for
the toolchain's default cpu. This is a problem if the toolchain defaults
to a cpu with newer instructions.

We could wire in TARGET_CPU but instead use the oldest supported option
so the wrapper runs anywhere.

The GCC documentation stays that -mcpu=powerpc64le will give us a
generic 64 bit powerpc machine:

 -mcpu=powerpc, -mcpu=powerpc64, and -mcpu=powerpc64le specify pure
 32-bit PowerPC (either endian), 64-bit big endian PowerPC and 64-bit
 little endian PowerPC architecture machine types, with an appropriate,
 generic processor model assumed for scheduling purposes.

So do that for each of the three machines.

This bug was found when building the kernel with a toolchain that
defaulted to powre10, resulting in a pcrel enabled wrapper which fails
to link:

 arch/powerpc/boot/wrapper.a(crt0.o): in function `p_base':
 (.text+0x150): call to `platform_init' lacks nop, can't restore toc; (toc 
save/adjust stub)
 (.text+0x154): call to `start' lacks nop, can't restore toc; (toc save/adjust 
stub)
 powerpc64le-buildroot-linux-gnu-ld: final link failed: bad value

Even with tha bug worked around the resulting kernel would crash on a
power9 box:

 $ qemu-system-ppc64 -nographic -nodefaults -M powernv9 -kernel 
arch/powerpc/boot/zImage.epapr -serial mon:stdio
 [7.069331356,5] INIT: Starting kernel at 0x20010020, fdt at 0x3068c628 
25694 bytes
 [7.130374661,3] ***
 [7.131072886,3] Fatal Exception 0xe40 at 200101e4MSR 
9001
 [7.131290613,3] CFAR : 2001027c MSR  : 9001
 [7.131433759,3] SRR0 : 20010050 SRR1 : 9001
 [7.13155,3] HSRR0: 200101e4 HSRR1: 9001
 [7.131733687,3] DSISR:  DAR  : 
 [7.131905162,3] LR   : 20010280 CTR  : 
 [7.132068356,3] CR   : 44002004 XER  : 

Link: https://github.com/linuxppc/issues/issues/400
Signed-off-by: Joel Stanley 
---
Tested:

 - ppc64le_defconfig
 - pseries and powernv qemu, for power8, power9, power10 cpus
 - buildroot compiler that defaults to -mcpu=power10 (gcc 10.3.0, ld 2.36.1)
 -  RHEL9 cross compilers (gcc 11.2.1-1, ld 2.35.2-17.el9)

All decompressed and made it into the kernel ok.

ppc64_defconfig did not work, as we've got a regression when the wrapper
is built for big endian. It hasn't worked for zImage.pseries for a long
time (at least v4.14), and broke some time between v5.4 and v5.17 for
zImage.epapr.

 arch/powerpc/boot/Makefile | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/boot/Makefile b/arch/powerpc/boot/Makefile
index 9993c6256ad2..1f5cc401bfc0 100644
--- a/arch/powerpc/boot/Makefile
+++ b/arch/powerpc/boot/Makefile
@@ -38,9 +38,13 @@ BOOTCFLAGS:= -Wall -Wundef -Wstrict-prototypes 
-Wno-trigraphs \
 $(LINUXINCLUDE)
 
 ifdef CONFIG_PPC64_BOOT_WRAPPER
-BOOTCFLAGS += -m64
+ifdef CONFIG_CPU_LITTLE_ENDIAN
+BOOTCFLAGS += -m64 -mcpu=powerpc64le
 else
-BOOTCFLAGS += -m32
+BOOTCFLAGS += -m64 -mcpu=powerpc64
+endif
+else
+BOOTCFLAGS += -m32 -mcpu=powerpc
 endif
 
 BOOTCFLAGS += -isystem $(shell $(BOOTCC) -print-file-name=include)
-- 
2.35.1

Re: [PATCH] recordmcount: Support empty section from recent binutils

[Joel Stanley]

On Mon, 29 Nov 2021 at 22:43, Christophe Leroy
 wrote:
>
>
>
> Le 29/11/2021 à 18:43, Steven Rostedt a écrit :
> > On Fri, 26 Nov 2021 08:43:23 +
> > LEROY Christophe  wrote:
> >
> >> Le 24/11/2021 à 15:43, Christophe Leroy a écrit :
> >>> Looks like recent binutils (2.36 and over ?) may empty some section,
> >>> leading to failure like:
> >>>
> >>> Cannot find symbol for section 11: .text.unlikely.
> >>> kernel/kexec_file.o: failed
> >>> make[1]: *** [scripts/Makefile.build:287: kernel/kexec_file.o] Error 1
> >>>
> >>> In order to avoid that, ensure that the section has a content before
> >>> returning it's name in has_rel_mcount().
> >>
> >> This patch doesn't work, on PPC32 I get the following message with this
> >> patch applied:
> >>
> >> [0.00] ftrace: No functions to be traced?
> >>
> >> Without the patch I get:
> >>
> >> [0.00] ftrace: allocating 22381 entries in 66 pages
> >> [0.00] ftrace: allocated 66 pages with 2 groups
> >
> > Because of this report, I have not applied this patch (even though I was
> > about to push it to Linus).
> >
> > I'm pulling it from my queue until this gets resolved.
> >
>
> I have no idea on how to fix that for the moment.
>
> With GCC 10 (binutils 2.36) an objdump -x on kernel/kexec_file.o gives:
>
>  ld  .text.unlikely  .text.unlikely
>   wF .text.unlikely 0038
> .arch_kexec_apply_relocations_add
> 0038  wF .text.unlikely 0038
> .arch_kexec_apply_relocations
>
>
> With GCC 11 (binutils 2.37) the same gives:
>
>   wF .text.unlikely 0038
> .arch_kexec_apply_relocations_add
> 0038  wF .text.unlikely 0038
> .arch_kexec_apply_relocations
>
>
> The problem is that recordmcount drops weak symbols, and it doesn't find
> any non-weak symbol in .text.unlikely
>
> Explication given at
> https://elixir.bootlin.com/linux/v5.16-rc2/source/scripts/recordmcount.h#L506
>
> I have no idea on what to do.

Did you end up finding a solution for this issue?

Cheers,

Joel

Re: No Linux logs when doing `ppc64_cpu --smt=off/8`

[Joel Stanley]

On Thu, 17 Feb 2022 at 01:07, Michael Ellerman  wrote:
>
> Michal Suchánek  writes:
> > On Mon, Feb 14, 2022 at 01:33:24PM +0100, Paul Menzel wrote:

> >> Yes, simple `nproc` suffice, but I was more thinking about, that the Linux
> >> log is often used for debugging and the changes of amount of processing
> >> units might be good to have. `ppc64_cpu --smt=off` or `=8` seems to block
> >> for quite some time, and each thread/processing unit seems to powered
> >> down/on sequentially, so it takes quite some time and it blocks. So 140
> >> messages would indeed be quite noise. No idea how `ppc64_cpu` works, and if
> >> it could log a message at the beginning and end.
> >
> > Yes, it enables/disables threads one by one. AFAICT the kernel cannot know 
> > that
> > ppc64_cpu will enable/disable more threads later, it can either log each
> > or none. Rate limiting would not show the whole picture so it's not
> > great solution either.
>
> Right, ppc64_cpu just uses the sysfs online files, so it's doing them one at a
> time. The kernel has no knowledge that ppc64_cpu is turning all
> secondaries on/off so there's no easy way for the kernel to do a summary
> message.
>
> An easy solution would be for ppc64_cpu to log something via syslog(3).

Another option would be to implement this sysfs knob for powerpc:

 /sys/devices/system/cpu/smt/control

 
https://github.com/torvalds/linux/commit/05736e4ac13c08a4a9b1ef2de26dd31a32cbee57

It currently returns "notimplemented" on power, but it's designed (for
x86) to turn SMT on and off.

With this implemented we could offline all CPUs at once before
returning to userspace. It "just" needs someone with some time to work
on it.

...which I spent the afternoon attempting to do:

 https://lore.kernel.org/linuxppc-dev/20220217070419.351683-1-j...@jms.id.au/

It does what it says on the tin, but there's a few rough edges that
need working out. It won't solve the problem you had of wanting a
message in the kernel logs (I notice that the x86 cpu hotplug code
does print something; I wonder how that goes on a large box). If you
or anyone else wants to run with the idea then free free to use my
patch as a starting point.

Cheers,

Joel

[RFC PATCH] powerpc: Implement hotplug smt control

[Joel Stanley]

x86 added a control for turning SMT on and off in commit 05736e4ac13c
("cpu/hotplug: Provide knobs to control SMT").

Implement this for powerpc as an alternative to the currently method of
iterating through /sys/devices/system/cpu/cpuN/online for every CPU.

  # ppc64_cpu --info
  Core   0:0*1*2*3*4*5*6*7*
  Core   1:8*9*   10*   11*   12*   13*   14*   15*
  # grep . /sys/devices/system/cpu/smt/*
  /sys/devices/system/cpu/smt/active:1
  /sys/devices/system/cpu/smt/control:on
  # echo off > /sys/devices/system/cpu/smt/control
  # ppc64_cpu --info
  Core   0:0*1 2 3 4 5 6 7
  Core   1:8*9101112131415
  # grep . /sys/devices/system/cpu/smt/*
  /sys/devices/system/cpu/smt/active:0
  /sys/devices/system/cpu/smt/control:off

Signed-off-by: Joel Stanley 
---

This is a RFC as there are bugs:

 - Booting with nosmt results in a WARN for every sibling thread

smp: Bringing up secondary CPUs ...
CPU UP failed (-125) CPU 1 state (null) (151)
[ cut here ]
WARNING: CPU: 0 PID: 1 at kernel/cpu.c:711 _cpu_up+0x304/0x310
Modules linked in:
CPU: 0 PID: 1 Comm: swapper/0 Not tainted 
5.17.0-rc4-00055-g2db720040f59-dirty #8
...
NIP [c0136ac4] _cpu_up+0x304/0x310
LR [c0136ac4] _cpu_up+0x304/0x310
Call Trace:
[c8503ab0] [c0136ac4] _cpu_up+0x304/0x310 (unreliable)
[c8503b60] [c0136be4] cpu_up+0x114/0x1b0
[c8503c00] [c0137538] bringup_nonboot_cpus+0xb8/0x110
[c8503c60] [c2031838] smp_init+0x48/0xd4
[c8503cc0] [c20048f4] kernel_init_freeable+0x20c/0x3dc
[c8503da0] [c00127a4] kernel_init+0x34/0x1a0
[c8503e10] [c000cd64] ret_from_kernel_thread+0x5c/0x64
Instruction dump:
1d270028 7cca482a 48089689 6000 7ec4b378 7f03c378 4bffd45d 7ec6b378
7f05c378 7f84e378 3860 4bffe919 <0b03> eac10060 4bfffe64 3c4c0271
---[ end trace  ]---

and then you can get a BUG at runtime if you fiddle with the online
state:

# ppc64_cpu --smt=on
# ppc64_cpu --info
Core   0:0*1*2*3*4*5*6*7*
Core   1:8*9*   10*   11*   12*   13*   14*   15*
# ppc64_cpu --smt=off
[   95.643467][  T203] [ cut here ]
[   95.643556][  T203] kernel BUG at kernel/irq_work.c:235!
[   95.643633][  T203] Oops: Exception in kernel mode, sig: 5 [#1]

 - Using the smt control to turn off SMT means you cannot online those
   CPUs with /sys/devices/system/cpu/cpuN/online (returns EPERM)

# ppc64_cpu --info
Core   0:0*1*2*3*4*5*6*7*
Core   1:8*9*   10*   11*   12*   13*   14*   15*
# echo off > /sys/devices/system/cpu/smt/control
# ppc64_cpu --info
Core   0:0*1 2 3 4 5 6 7
Core   1:8*9101112131415
# grep . /sys/devices/system/cpu/smt/*
/sys/devices/system/cpu/smt/active:0
/sys/devices/system/cpu/smt/control:off
# ppc64_cpu --smt=on
One or more cpus could not be on/offlined

# strace ppc64_cpu --smt=on
openat(AT_FDCWD, "/sys/devices/system/cpu/cpu0/online", O_RDONLY) = 3
newfstatat(3, "", {st_mode=S_IFREG|0644, st_size=65536, ...}, 
AT_EMPTY_PATH) = 0
read(3, "1\n", 8192)= 2
close(3)= 0
openat(AT_FDCWD, "/sys/devices/system/cpu/cpu0/online", O_WRONLY) = 3
write(3, "1", 1)= 1
close(3)= 0
openat(AT_FDCWD, "/sys/devices/system/cpu/cpu1/online", O_WRONLY) = 3
write(3, "1", 1)= -1 EPERM (Operation not permitted)
close(3)= 0
access("/sys/devices/system/cpu/cpu8/online", F_OK) = 0
openat(AT_FDCWD, "/sys/devices/system/cpu/cpu8/online", O_RDONLY) = 3
newfstatat(3, "", {st_mode=S_IFREG|0644, st_size=65536, ...}, 
AT_EMPTY_PATH) = 0
read(3, "1\n", 8192)= 2
close(3)= 0
openat(AT_FDCWD, "/sys/devices/system/cpu/cpu8/online", O_WRONLY) = 3
write(3, "1", 1)= 1
close(3)= 0
openat(AT_FDCWD, "/sys/devices/system/cpu/cpu9/online", O_WRONLY) = 3
write(3, "1", 1)= -1 EPERM (Operation not permitted)
close(3)= 0
write(2, "One or more cpus could not be on"..., 42One or more cpus could 
not be on/offlined

 - Setting smt off with the online interface doesn't update the status
   of the smt control sysfs file (b

Re: [PATCH v2 16/16] powerpc/microwatt: Don't select the hash MMU code

[Joel Stanley]

On Thu, 21 Oct 2021 at 04:04, Nicholas Piggin  wrote:
>
> Microwatt is radix-only, so it does not require hash MMU support.
>
> This saves 20kB compressed dtbImage and 56kB vmlinux size.
>
> Signed-off-by: Nicholas Piggin 
> ---
>  arch/powerpc/configs/microwatt_defconfig | 1 -
>  arch/powerpc/platforms/microwatt/Kconfig | 1 -
>  2 files changed, 2 deletions(-)
>
> diff --git a/arch/powerpc/configs/microwatt_defconfig 
> b/arch/powerpc/configs/microwatt_defconfig
> index 6e62966730d3..7c8eb29d8afe 100644
> --- a/arch/powerpc/configs/microwatt_defconfig
> +++ b/arch/powerpc/configs/microwatt_defconfig
> @@ -27,7 +27,6 @@ CONFIG_PPC_MICROWATT=y
>  # CONFIG_PPC_OF_BOOT_TRAMPOLINE is not set
>  CONFIG_CPU_FREQ=y
>  CONFIG_HZ_100=y
> -# CONFIG_PPC_MEM_KEYS is not set
>  # CONFIG_SECCOMP is not set
>  # CONFIG_MQ_IOSCHED_KYBER is not set
>  # CONFIG_COREDUMP is not set

We still end up with CONFIG_PPC_64S_HASH_MMU=y in the config as it
defaults to y.

We should disable in the defconfig it so your new changes are tested
by that defconfig:

+# CONFIG_PPC_64S_HASH_MMU is not set

I boot tested your series on Microwatt with microwatt_defconfig (with
and without that option set) and ppc64le_defconfig.

Cheers,

Joel

> diff --git a/arch/powerpc/platforms/microwatt/Kconfig 
> b/arch/powerpc/platforms/microwatt/Kconfig
> index 823192e9d38a..5e320f49583a 100644
> --- a/arch/powerpc/platforms/microwatt/Kconfig
> +++ b/arch/powerpc/platforms/microwatt/Kconfig
> @@ -5,7 +5,6 @@ config PPC_MICROWATT
> select PPC_XICS
> select PPC_ICS_NATIVE
> select PPC_ICP_NATIVE
> -   select PPC_HASH_MMU_NATIVE if PPC_64S_HASH_MMU
> select PPC_UDBG_16550
> select ARCH_RANDOM
> help
> --
> 2.23.0
>

[PATCH v2] powerpc/64s: Default to 64K pages for 64 bit book3s

[Joel Stanley]

For 64-bit book3s the default should be 64K as that's what modern CPUs
are designed for.

The following defconfigs already set CONFIG_PPC_64K_PAGES:

 cell_defconfig
 pasemi_defconfig
 powernv_defconfig
 ppc64_defconfig
 pseries_defconfig
 skiroot_defconfig

The have the option removed from the defconfig, as it is now the
default.

The defconfigs that now need to set CONFIG_PPC_4K_PAGES to maintain
their existing behaviour are:

 g5_defconfig
 maple_defconfig
 microwatt_defconfig
 ps3_defconfig

Link: https://github.com/linuxppc/issues/issues/109
Signed-off-by: Joel Stanley 
---
v2: remove unrelated change from microwatt_defconfig

 arch/powerpc/Kconfig | 1 +
 arch/powerpc/configs/cell_defconfig  | 1 -
 arch/powerpc/configs/g5_defconfig| 1 +
 arch/powerpc/configs/maple_defconfig | 1 +
 arch/powerpc/configs/microwatt_defconfig | 1 +
 arch/powerpc/configs/pasemi_defconfig| 1 -
 arch/powerpc/configs/powernv_defconfig   | 1 -
 arch/powerpc/configs/ppc64_defconfig | 1 -
 arch/powerpc/configs/ps3_defconfig   | 1 +
 arch/powerpc/configs/pseries_defconfig   | 1 -
 arch/powerpc/configs/skiroot_defconfig   | 1 -
 11 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 8a584414ef67..e2c220fa91c0 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -708,6 +708,7 @@ config ARCH_MEMORY_PROBE
 
 choice
prompt "Page size"
+   default PPC_64K_PAGES if PPC_BOOK3S_64
default PPC_4K_PAGES
help
  Select the kernel logical page size. Increasing the page size
diff --git a/arch/powerpc/configs/cell_defconfig 
b/arch/powerpc/configs/cell_defconfig
index cc2c0d51f493..7fd9e596ea33 100644
--- a/arch/powerpc/configs/cell_defconfig
+++ b/arch/powerpc/configs/cell_defconfig
@@ -36,7 +36,6 @@ CONFIG_GEN_RTC=y
 CONFIG_BINFMT_MISC=m
 CONFIG_IRQ_ALL_CPUS=y
 CONFIG_NUMA=y
-CONFIG_PPC_64K_PAGES=y
 CONFIG_SCHED_SMT=y
 CONFIG_PCIEPORTBUS=y
 CONFIG_NET=y
diff --git a/arch/powerpc/configs/g5_defconfig 
b/arch/powerpc/configs/g5_defconfig
index 63d611cc160f..9d6212a8b195 100644
--- a/arch/powerpc/configs/g5_defconfig
+++ b/arch/powerpc/configs/g5_defconfig
@@ -26,6 +26,7 @@ CONFIG_CPU_FREQ_PMAC64=y
 CONFIG_GEN_RTC=y
 CONFIG_KEXEC=y
 CONFIG_IRQ_ALL_CPUS=y
+CONFIG_PPC_4K_PAGES=y
 CONFIG_PCI_MSI=y
 CONFIG_NET=y
 CONFIG_PACKET=y
diff --git a/arch/powerpc/configs/maple_defconfig 
b/arch/powerpc/configs/maple_defconfig
index 9424c1e67e1c..c821a97f4a89 100644
--- a/arch/powerpc/configs/maple_defconfig
+++ b/arch/powerpc/configs/maple_defconfig
@@ -25,6 +25,7 @@ CONFIG_UDBG_RTAS_CONSOLE=y
 CONFIG_GEN_RTC=y
 CONFIG_KEXEC=y
 CONFIG_IRQ_ALL_CPUS=y
+CONFIG_PPC_4K_PAGES=y
 CONFIG_PCI_MSI=y
 CONFIG_NET=y
 CONFIG_PACKET=y
diff --git a/arch/powerpc/configs/microwatt_defconfig 
b/arch/powerpc/configs/microwatt_defconfig
index 9465209b8c5b..07d87a4044b2 100644
--- a/arch/powerpc/configs/microwatt_defconfig
+++ b/arch/powerpc/configs/microwatt_defconfig
@@ -26,6 +26,7 @@ CONFIG_PPC_MICROWATT=y
 # CONFIG_PPC_OF_BOOT_TRAMPOLINE is not set
 CONFIG_CPU_FREQ=y
 CONFIG_HZ_100=y
+CONFIG_PPC_4K_PAGES=y
 # CONFIG_PPC_MEM_KEYS is not set
 # CONFIG_SECCOMP is not set
 # CONFIG_MQ_IOSCHED_KYBER is not set
diff --git a/arch/powerpc/configs/pasemi_defconfig 
b/arch/powerpc/configs/pasemi_defconfig
index 78606b7e42df..e00a703581c3 100644
--- a/arch/powerpc/configs/pasemi_defconfig
+++ b/arch/powerpc/configs/pasemi_defconfig
@@ -22,7 +22,6 @@ CONFIG_CPU_FREQ_GOV_POWERSAVE=y
 CONFIG_CPU_FREQ_GOV_USERSPACE=y
 CONFIG_CPU_FREQ_GOV_ONDEMAND=y
 CONFIG_HZ_1000=y
-CONFIG_PPC_64K_PAGES=y
 # CONFIG_SECCOMP is not set
 CONFIG_PCI_MSI=y
 CONFIG_PCCARD=y
diff --git a/arch/powerpc/configs/powernv_defconfig 
b/arch/powerpc/configs/powernv_defconfig
index 8bfeea6c7de7..49f49c263935 100644
--- a/arch/powerpc/configs/powernv_defconfig
+++ b/arch/powerpc/configs/powernv_defconfig
@@ -62,7 +62,6 @@ CONFIG_MEMORY_FAILURE=y
 CONFIG_HWPOISON_INJECT=m
 CONFIG_TRANSPARENT_HUGEPAGE=y
 CONFIG_DEFERRED_STRUCT_PAGE_INIT=y
-CONFIG_PPC_64K_PAGES=y
 CONFIG_SCHED_SMT=y
 CONFIG_PM=y
 CONFIG_HOTPLUG_PCI=y
diff --git a/arch/powerpc/configs/ppc64_defconfig 
b/arch/powerpc/configs/ppc64_defconfig
index 0ad2291337a7..203d0b7f0bb8 100644
--- a/arch/powerpc/configs/ppc64_defconfig
+++ b/arch/powerpc/configs/ppc64_defconfig
@@ -52,7 +52,6 @@ CONFIG_KEXEC_FILE=y
 CONFIG_CRASH_DUMP=y
 CONFIG_FA_DUMP=y
 CONFIG_IRQ_ALL_CPUS=y
-CONFIG_PPC_64K_PAGES=y
 CONFIG_SCHED_SMT=y
 CONFIG_HOTPLUG_PCI=y
 CONFIG_HOTPLUG_PCI_RPA=m
diff --git a/arch/powerpc/configs/ps3_defconfig 
b/arch/powerpc/configs/ps3_defconfig
index f300dcb937cc..7c95fab4b920 100644
--- a/arch/powerpc/configs/ps3_defconfig
+++ b/arch/powerpc/configs/ps3_defconfig
@@ -30,6 +30,7 @@ CONFIG_PS3_LPM=m
 # CONFIG_CORE_DUMP_DEFAULT_ELF_HEADERS is not set
 CONFIG_BINFMT_MISC=y
 CONFIG_KEXEC=y
+CONFIG_PPC_4K_PAGES=y
 # CONFIG_SPARSEMEM_VMEMMAP is not set
 # CONFIG_COMPACTION is not set
 CONFIG_SCHED_SMT=y
diff --

Re: [PATCH] powerpc/64s: Default to 64K pages for 64 bit book3s

[Joel Stanley]

On Thu, 14 Oct 2021 at 07:03, LEROY Christophe
 wrote:
>
>
>
> Le 14/10/2021 à 01:31, Joel Stanley a écrit :
> > For 64-bit book3s the default should be 64K as that's what modern CPUs
> > are designed for.
> >
> > The following defconfigs already set CONFIG_PPC_64K_PAGES:
> >
> >   cell_defconfig
> >   pasemi_defconfig
> >   powernv_defconfig
> >   ppc64_defconfig
> >   pseries_defconfig
> >   skiroot_defconfig
> >
> > The have the option removed from the defconfig, as it is now the
> > default.
> >
> > The defconfigs that now need to set CONFIG_PPC_4K_PAGES to maintain
> > their existing behaviour are:
> >
> >   g5_defconfig
> >   maple_defconfig
> >   microwatt_defconfig
> >   ps3_defconfig
> >
> > Link: https://github.com/linuxppc/issues/issues/109
> > Signed-off-by: Joel Stanley 
> > ---
> >   arch/powerpc/Kconfig | 1 +
> >   arch/powerpc/configs/cell_defconfig  | 1 -
> >   arch/powerpc/configs/g5_defconfig| 1 +
> >   arch/powerpc/configs/maple_defconfig | 1 +
> >   arch/powerpc/configs/microwatt_defconfig | 2 +-
> >   arch/powerpc/configs/pasemi_defconfig| 1 -
> >   arch/powerpc/configs/powernv_defconfig   | 1 -
> >   arch/powerpc/configs/ppc64_defconfig | 1 -
> >   arch/powerpc/configs/ps3_defconfig   | 1 +
> >   arch/powerpc/configs/pseries_defconfig   | 1 -
> >   arch/powerpc/configs/skiroot_defconfig   | 1 -
> >   11 files changed, 5 insertions(+), 7 deletions(-)
> >
>
> > diff --git a/arch/powerpc/configs/microwatt_defconfig 
> > b/arch/powerpc/configs/microwatt_defconfig
> > index 9465209b8c5b..556ec5eec684 100644
> > --- a/arch/powerpc/configs/microwatt_defconfig
> > +++ b/arch/powerpc/configs/microwatt_defconfig
> > @@ -1,7 +1,6 @@
> >   # CONFIG_SWAP is not set
> >   # CONFIG_CROSS_MEMORY_ATTACH is not set
> >   CONFIG_HIGH_RES_TIMERS=y
> > -CONFIG_PREEMPT_VOLUNTARY=y
>
> This seems unrelated.

It is, thanks for catching that.

[PATCH] powerpc/64s: Default to 64K pages for 64 bit book3s

[Joel Stanley]

For 64-bit book3s the default should be 64K as that's what modern CPUs
are designed for.

The following defconfigs already set CONFIG_PPC_64K_PAGES:

 cell_defconfig
 pasemi_defconfig
 powernv_defconfig
 ppc64_defconfig
 pseries_defconfig
 skiroot_defconfig

The have the option removed from the defconfig, as it is now the
default.

The defconfigs that now need to set CONFIG_PPC_4K_PAGES to maintain
their existing behaviour are:

 g5_defconfig
 maple_defconfig
 microwatt_defconfig
 ps3_defconfig

Link: https://github.com/linuxppc/issues/issues/109
Signed-off-by: Joel Stanley 
---
 arch/powerpc/Kconfig | 1 +
 arch/powerpc/configs/cell_defconfig  | 1 -
 arch/powerpc/configs/g5_defconfig| 1 +
 arch/powerpc/configs/maple_defconfig | 1 +
 arch/powerpc/configs/microwatt_defconfig | 2 +-
 arch/powerpc/configs/pasemi_defconfig| 1 -
 arch/powerpc/configs/powernv_defconfig   | 1 -
 arch/powerpc/configs/ppc64_defconfig | 1 -
 arch/powerpc/configs/ps3_defconfig   | 1 +
 arch/powerpc/configs/pseries_defconfig   | 1 -
 arch/powerpc/configs/skiroot_defconfig   | 1 -
 11 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 8a584414ef67..e2c220fa91c0 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -708,6 +708,7 @@ config ARCH_MEMORY_PROBE
 
 choice
prompt "Page size"
+   default PPC_64K_PAGES if PPC_BOOK3S_64
default PPC_4K_PAGES
help
  Select the kernel logical page size. Increasing the page size
diff --git a/arch/powerpc/configs/cell_defconfig 
b/arch/powerpc/configs/cell_defconfig
index cc2c0d51f493..7fd9e596ea33 100644
--- a/arch/powerpc/configs/cell_defconfig
+++ b/arch/powerpc/configs/cell_defconfig
@@ -36,7 +36,6 @@ CONFIG_GEN_RTC=y
 CONFIG_BINFMT_MISC=m
 CONFIG_IRQ_ALL_CPUS=y
 CONFIG_NUMA=y
-CONFIG_PPC_64K_PAGES=y
 CONFIG_SCHED_SMT=y
 CONFIG_PCIEPORTBUS=y
 CONFIG_NET=y
diff --git a/arch/powerpc/configs/g5_defconfig 
b/arch/powerpc/configs/g5_defconfig
index 63d611cc160f..9d6212a8b195 100644
--- a/arch/powerpc/configs/g5_defconfig
+++ b/arch/powerpc/configs/g5_defconfig
@@ -26,6 +26,7 @@ CONFIG_CPU_FREQ_PMAC64=y
 CONFIG_GEN_RTC=y
 CONFIG_KEXEC=y
 CONFIG_IRQ_ALL_CPUS=y
+CONFIG_PPC_4K_PAGES=y
 CONFIG_PCI_MSI=y
 CONFIG_NET=y
 CONFIG_PACKET=y
diff --git a/arch/powerpc/configs/maple_defconfig 
b/arch/powerpc/configs/maple_defconfig
index 9424c1e67e1c..c821a97f4a89 100644
--- a/arch/powerpc/configs/maple_defconfig
+++ b/arch/powerpc/configs/maple_defconfig
@@ -25,6 +25,7 @@ CONFIG_UDBG_RTAS_CONSOLE=y
 CONFIG_GEN_RTC=y
 CONFIG_KEXEC=y
 CONFIG_IRQ_ALL_CPUS=y
+CONFIG_PPC_4K_PAGES=y
 CONFIG_PCI_MSI=y
 CONFIG_NET=y
 CONFIG_PACKET=y
diff --git a/arch/powerpc/configs/microwatt_defconfig 
b/arch/powerpc/configs/microwatt_defconfig
index 9465209b8c5b..556ec5eec684 100644
--- a/arch/powerpc/configs/microwatt_defconfig
+++ b/arch/powerpc/configs/microwatt_defconfig
@@ -1,7 +1,6 @@
 # CONFIG_SWAP is not set
 # CONFIG_CROSS_MEMORY_ATTACH is not set
 CONFIG_HIGH_RES_TIMERS=y
-CONFIG_PREEMPT_VOLUNTARY=y
 CONFIG_TICK_CPU_ACCOUNTING=y
 CONFIG_LOG_BUF_SHIFT=16
 CONFIG_PRINTK_SAFE_LOG_BUF_SHIFT=12
@@ -26,6 +25,7 @@ CONFIG_PPC_MICROWATT=y
 # CONFIG_PPC_OF_BOOT_TRAMPOLINE is not set
 CONFIG_CPU_FREQ=y
 CONFIG_HZ_100=y
+CONFIG_PPC_4K_PAGES=y
 # CONFIG_PPC_MEM_KEYS is not set
 # CONFIG_SECCOMP is not set
 # CONFIG_MQ_IOSCHED_KYBER is not set
diff --git a/arch/powerpc/configs/pasemi_defconfig 
b/arch/powerpc/configs/pasemi_defconfig
index 78606b7e42df..e00a703581c3 100644
--- a/arch/powerpc/configs/pasemi_defconfig
+++ b/arch/powerpc/configs/pasemi_defconfig
@@ -22,7 +22,6 @@ CONFIG_CPU_FREQ_GOV_POWERSAVE=y
 CONFIG_CPU_FREQ_GOV_USERSPACE=y
 CONFIG_CPU_FREQ_GOV_ONDEMAND=y
 CONFIG_HZ_1000=y
-CONFIG_PPC_64K_PAGES=y
 # CONFIG_SECCOMP is not set
 CONFIG_PCI_MSI=y
 CONFIG_PCCARD=y
diff --git a/arch/powerpc/configs/powernv_defconfig 
b/arch/powerpc/configs/powernv_defconfig
index 8bfeea6c7de7..49f49c263935 100644
--- a/arch/powerpc/configs/powernv_defconfig
+++ b/arch/powerpc/configs/powernv_defconfig
@@ -62,7 +62,6 @@ CONFIG_MEMORY_FAILURE=y
 CONFIG_HWPOISON_INJECT=m
 CONFIG_TRANSPARENT_HUGEPAGE=y
 CONFIG_DEFERRED_STRUCT_PAGE_INIT=y
-CONFIG_PPC_64K_PAGES=y
 CONFIG_SCHED_SMT=y
 CONFIG_PM=y
 CONFIG_HOTPLUG_PCI=y
diff --git a/arch/powerpc/configs/ppc64_defconfig 
b/arch/powerpc/configs/ppc64_defconfig
index 0ad2291337a7..203d0b7f0bb8 100644
--- a/arch/powerpc/configs/ppc64_defconfig
+++ b/arch/powerpc/configs/ppc64_defconfig
@@ -52,7 +52,6 @@ CONFIG_KEXEC_FILE=y
 CONFIG_CRASH_DUMP=y
 CONFIG_FA_DUMP=y
 CONFIG_IRQ_ALL_CPUS=y
-CONFIG_PPC_64K_PAGES=y
 CONFIG_SCHED_SMT=y
 CONFIG_HOTPLUG_PCI=y
 CONFIG_HOTPLUG_PCI_RPA=m
diff --git a/arch/powerpc/configs/ps3_defconfig 
b/arch/powerpc/configs/ps3_defconfig
index f300dcb937cc..7c95fab4b920 100644
--- a/arch/powerpc/configs/ps3_defconfig
+++ b/arch/powerpc/configs/ps3_defconfig
@@ -30,6 +30,7 @@ CONFIG_PS3_LPM=m
 # CONFIG_CORE_DUMP_DEFAULT_E

[PATCH v2] powerpc/s64: Clarify that radix lacks DEBUG_PAGEALLOC

[Joel Stanley]

The page_alloc.c code will call into __kernel_map_pages when
DEBUG_PAGEALLOC is configured and enabled.

As the implementation assumes hash, this should crash spectacularly if
not for a bit of luck in __kernel_map_pages. In this function
linear_map_hash_count is always zero, the for loop exits without doing
any damage.

There are no other platforms that determine if they support
debug_pagealloc at runtime. Instead of adding code to mm/page_alloc.c to
do that, this change turns the map/unmap into a noop when in radix
mode and prints a warning once.

Signed-off-by: Joel Stanley 
---
v2: Put __kernel_map_pages in pgtable.h

 arch/powerpc/include/asm/book3s/64/hash.h|  2 ++
 arch/powerpc/include/asm/book3s/64/pgtable.h | 11 +++
 arch/powerpc/include/asm/book3s/64/radix.h   |  3 +++
 arch/powerpc/mm/book3s64/hash_utils.c|  2 +-
 arch/powerpc/mm/book3s64/radix_pgtable.c |  7 +++
 5 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/include/asm/book3s/64/hash.h 
b/arch/powerpc/include/asm/book3s/64/hash.h
index d959b0195ad9..674fe0e890dc 100644
--- a/arch/powerpc/include/asm/book3s/64/hash.h
+++ b/arch/powerpc/include/asm/book3s/64/hash.h
@@ -255,6 +255,8 @@ int hash__create_section_mapping(unsigned long start, 
unsigned long end,
 int nid, pgprot_t prot);
 int hash__remove_section_mapping(unsigned long start, unsigned long end);
 
+void hash__kernel_map_pages(struct page *page, int numpages, int enable);
+
 #endif /* !__ASSEMBLY__ */
 #endif /* __KERNEL__ */
 #endif /* _ASM_POWERPC_BOOK3S_64_HASH_H */
diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h 
b/arch/powerpc/include/asm/book3s/64/pgtable.h
index 5d34a8646f08..265661ded238 100644
--- a/arch/powerpc/include/asm/book3s/64/pgtable.h
+++ b/arch/powerpc/include/asm/book3s/64/pgtable.h
@@ -1101,6 +1101,17 @@ static inline void vmemmap_remove_mapping(unsigned long 
start,
 }
 #endif
 
+#ifdef CONFIG_DEBUG_PAGEALLOC
+static inline void __kernel_map_pages(struct page *page, int numpages, int 
enable)
+{
+   if (radix_enabled()) {
+   radix__kernel_map_pages(page, numpages, enable);
+   return;
+   }
+   hash__kernel_map_pages(page, numpages, enable);
+}
+#endif
+
 static inline pte_t pmd_pte(pmd_t pmd)
 {
return __pte_raw(pmd_raw(pmd));
diff --git a/arch/powerpc/include/asm/book3s/64/radix.h 
b/arch/powerpc/include/asm/book3s/64/radix.h
index 59cab558e2f0..d090d9612348 100644
--- a/arch/powerpc/include/asm/book3s/64/radix.h
+++ b/arch/powerpc/include/asm/book3s/64/radix.h
@@ -316,5 +316,8 @@ int radix__create_section_mapping(unsigned long start, 
unsigned long end,
  int nid, pgprot_t prot);
 int radix__remove_section_mapping(unsigned long start, unsigned long end);
 #endif /* CONFIG_MEMORY_HOTPLUG */
+
+void radix__kernel_map_pages(struct page *page, int numpages, int enable);
+
 #endif /* __ASSEMBLY__ */
 #endif
diff --git a/arch/powerpc/mm/book3s64/hash_utils.c 
b/arch/powerpc/mm/book3s64/hash_utils.c
index c145776d3ae5..cfd45245d009 100644
--- a/arch/powerpc/mm/book3s64/hash_utils.c
+++ b/arch/powerpc/mm/book3s64/hash_utils.c
@@ -1988,7 +1988,7 @@ static void kernel_unmap_linear_page(unsigned long vaddr, 
unsigned long lmi)
 mmu_kernel_ssize, 0);
 }
 
-void __kernel_map_pages(struct page *page, int numpages, int enable)
+void hash__kernel_map_pages(struct page *page, int numpages, int enable)
 {
unsigned long flags, vaddr, lmi;
int i;
diff --git a/arch/powerpc/mm/book3s64/radix_pgtable.c 
b/arch/powerpc/mm/book3s64/radix_pgtable.c
index ae20add7954a..83b33418ad28 100644
--- a/arch/powerpc/mm/book3s64/radix_pgtable.c
+++ b/arch/powerpc/mm/book3s64/radix_pgtable.c
@@ -920,6 +920,13 @@ void __meminit radix__vmemmap_remove_mapping(unsigned long 
start, unsigned long
 #endif
 #endif
 
+#ifdef CONFIG_DEBUG_PAGEALLOC
+void radix__kernel_map_pages(struct page *page, int numpages, int enable)
+{
+pr_warn_once("DEBUG_PAGEALLOC not supported in radix mode\n");
+}
+#endif
+
 #ifdef CONFIG_TRANSPARENT_HUGEPAGE
 
 unsigned long radix__pmd_hugepage_update(struct mm_struct *mm, unsigned long 
addr,
-- 
2.33.0

[PATCH] powerpc/s64: Clarify that radix lacks DEBUG_PAGEALLOC

[Joel Stanley]

The page_alloc.c code will call into __kernel_map_pages when
DEBUG_PAGEALLOC is configured and enabled.

As the implementation assumes hash, this should crash spectacularly if
not for a bit of luck in __kernel_map_pages. In this function
linear_map_hash_count is always zero, the for loop exits without doing
any damage.

There are no other platforms that determine if they support
debug_pagealloc at runtime. Instead of adding code to mm/page_alloc.c to
do that, this change turns the map/unmap into a noop when in radix
mode and prints a warning once.

Signed-off-by: Joel Stanley 
---
I noticed this when I was looking at adding kfence support a while back.
I've put that work aside and jpn has since gotten further than me, but I
think this is a fix worth considering.

 arch/powerpc/include/asm/book3s/64/hash.h |  2 ++
 arch/powerpc/mm/book3s64/hash_utils.c |  2 +-
 arch/powerpc/mm/book3s64/pgtable.c| 12 
 3 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/include/asm/book3s/64/hash.h 
b/arch/powerpc/include/asm/book3s/64/hash.h
index d959b0195ad9..674fe0e890dc 100644
--- a/arch/powerpc/include/asm/book3s/64/hash.h
+++ b/arch/powerpc/include/asm/book3s/64/hash.h
@@ -255,6 +255,8 @@ int hash__create_section_mapping(unsigned long start, 
unsigned long end,
 int nid, pgprot_t prot);
 int hash__remove_section_mapping(unsigned long start, unsigned long end);
 
+void hash__kernel_map_pages(struct page *page, int numpages, int enable);
+
 #endif /* !__ASSEMBLY__ */
 #endif /* __KERNEL__ */
 #endif /* _ASM_POWERPC_BOOK3S_64_HASH_H */
diff --git a/arch/powerpc/mm/book3s64/hash_utils.c 
b/arch/powerpc/mm/book3s64/hash_utils.c
index c145776d3ae5..cfd45245d009 100644
--- a/arch/powerpc/mm/book3s64/hash_utils.c
+++ b/arch/powerpc/mm/book3s64/hash_utils.c
@@ -1988,7 +1988,7 @@ static void kernel_unmap_linear_page(unsigned long vaddr, 
unsigned long lmi)
 mmu_kernel_ssize, 0);
 }
 
-void __kernel_map_pages(struct page *page, int numpages, int enable)
+void hash__kernel_map_pages(struct page *page, int numpages, int enable)
 {
unsigned long flags, vaddr, lmi;
int i;
diff --git a/arch/powerpc/mm/book3s64/pgtable.c 
b/arch/powerpc/mm/book3s64/pgtable.c
index 9e16c7b1a6c5..0aefc272cd03 100644
--- a/arch/powerpc/mm/book3s64/pgtable.c
+++ b/arch/powerpc/mm/book3s64/pgtable.c
@@ -526,3 +526,15 @@ static int __init pgtable_debugfs_setup(void)
return 0;
 }
 arch_initcall(pgtable_debugfs_setup);
+
+#ifdef CONFIG_DEBUG_PAGEALLOC
+void __kernel_map_pages(struct page *page, int numpages, int enable)
+{
+   if (radix_enabled()) {
+   pr_warn_once("DEBUG_PAGEALLOC not supported in radix mode\n");
+   return;
+   }
+
+   hash__kernel_map_pages(page, numpages, enable);
+}
+#endif
-- 
2.33.0

Re: [PATCH] powerpc/boot: Use CONFIG_PPC_POWERNV to compile OPAL support

[Joel Stanley]

On Mon, 11 Oct 2021 at 07:42, Cédric Le Goater  wrote:
>
> CONFIG_PPC64_BOOT_WRAPPER is selected by CPU_LITTLE_ENDIAN which is
> used to compile support for other platforms such as Microwatt. There
> is no need for OPAL calls on these.
>
> Signed-off-by: Cédric Le Goater 

Reviewed-by: Joel Stanley 

> ---
>  arch/powerpc/boot/serial.c | 2 +-
>  arch/powerpc/boot/Makefile | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/boot/serial.c b/arch/powerpc/boot/serial.c
> index 9a19e5905485..54d2522be485 100644
> --- a/arch/powerpc/boot/serial.c
> +++ b/arch/powerpc/boot/serial.c
> @@ -132,7 +132,7 @@ int serial_console_init(void)
> else if (dt_is_compatible(devp, "fsl,mpc5200-psc-uart"))
> rc = mpc5200_psc_console_init(devp, _cd);
>  #endif
> -#ifdef CONFIG_PPC64_BOOT_WRAPPER
> +#ifdef CONFIG_PPC_POWERNV
> else if (dt_is_compatible(devp, "ibm,opal-console-raw"))
> rc = opal_console_init(devp, _cd);
>  #endif
> diff --git a/arch/powerpc/boot/Makefile b/arch/powerpc/boot/Makefile
> index 089ee3ea55c8..9993c6256ad2 100644
> --- a/arch/powerpc/boot/Makefile
> +++ b/arch/powerpc/boot/Makefile
> @@ -123,7 +123,7 @@ src-wlib-y := string.S crt0.S stdio.c decompress.c main.c 
> \
> oflib.c ofconsole.c cuboot.c
>
>  src-wlib-$(CONFIG_PPC_MPC52xx) += mpc52xx-psc.c
> -src-wlib-$(CONFIG_PPC64_BOOT_WRAPPER) += opal-calls.S opal.c
> +src-wlib-$(CONFIG_PPC_POWERNV) += opal-calls.S opal.c
>  ifndef CONFIG_PPC64_BOOT_WRAPPER
>  src-wlib-y += crtsavres.S
>  endif
> --
> 2.31.1
>

Re: [RFC 5/5] ipmi:bt-bmc: Add Microwatt

[Joel Stanley]

Hi Anton,

On Wed, 6 Oct 2021 at 02:12, Anton Blanchard  wrote:
>
> This adds the Microwatt specific bits, including interrupt support.

The series looks good.

I've got a couple of patches on the ipmi list that this will conflict with:

 https://sourceforge.net/p/openipmi/mailman/message/37345391/
 https://lore.kernel.org/all/20210903015314.177987-1-j...@jms.id.au/

If there's no feedback from others I suggest basing your series on top
of those, and sending them to Corey on the ipmi list to merge.

Cheers,

Joel

>
> Signed-off-by: Anton Blanchard 
> ---
>  .../devicetree/bindings/ipmi/ibt-bmc.txt  |  1 +
>  drivers/char/ipmi/Kconfig |  8 ++-
>  drivers/char/ipmi/bt-bmc.c| 69 +++
>  3 files changed, 75 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/ipmi/ibt-bmc.txt 
> b/Documentation/devicetree/bindings/ipmi/ibt-bmc.txt
> index 78ee716a950e..1b661daf0193 100644
> --- a/Documentation/devicetree/bindings/ipmi/ibt-bmc.txt
> +++ b/Documentation/devicetree/bindings/ipmi/ibt-bmc.txt
> @@ -9,6 +9,7 @@ Required properties:
>  - compatible : should be one of
> "aspeed,ast2400-ibt-bmc"
> "aspeed,ast2500-ibt-bmc"
> +   "ibm,microwatt-ibt-bmc"
>  - reg: physical address and size of the registers
>
>  Optional properties:
> diff --git a/drivers/char/ipmi/Kconfig b/drivers/char/ipmi/Kconfig
> index 8b2f0f675e5f..079302f4eef2 100644
> --- a/drivers/char/ipmi/Kconfig
> +++ b/drivers/char/ipmi/Kconfig
> @@ -152,13 +152,15 @@ config IPMI_KCS_BMC_SERIO
>   called kcs_bmc_serio.
>
>  config BT_IPMI_BMC
> -   depends on ARCH_ASPEED || COMPILE_TEST
> +   depends on ARCH_ASPEED || PPC_MICROWATT || COMPILE_TEST
> depends on REGMAP && REGMAP_MMIO && MFD_SYSCON
> tristate "BT IPMI bmc driver"
> help
>   Provides a driver for the BT (Block Transfer) IPMI interface
> - found on Aspeed SOCs (AST2400 and AST2500). The driver
> - implements the BMC side of the BT interface.
> + found on Aspeed SOCs (AST2400 and AST2500) as well as the OpenPOWER
> + LPC peripheral macro at
> + 
> + The driver implements the BMC side of the BT interface.
>
>  config IPMB_DEVICE_INTERFACE
> tristate 'IPMB Interface handler'
> diff --git a/drivers/char/ipmi/bt-bmc.c b/drivers/char/ipmi/bt-bmc.c
> index b48e04405ac4..24327b57c60b 100644
> --- a/drivers/char/ipmi/bt-bmc.c
> +++ b/drivers/char/ipmi/bt-bmc.c
> @@ -41,6 +41,11 @@
>  #define   BT_CR2_IRQ_HBUSY 0x40
>  #define ASPEED_BT_CR3  0xc
>
> +#define MICROWATT_IRQ_MASK 0x0
> +#define MICROWATT_IRQ_STATUS   0x4
> +#define   IRQ_HOST_TO_BMC_ATTN 0x1
> +#define   IRQ_HOST_NOT_BUSY0x2
> +
>  #define BT_CTRL0x10
>  #define   BT_CTRL_B_BUSY   0x80
>  #define   BT_CTRL_H_BUSY   0x40
> @@ -395,6 +400,27 @@ static irqreturn_t aspeed_bt_bmc_irq(int irq, void *arg)
> return IRQ_HANDLED;
>  }
>
> +static irqreturn_t microwatt_bt_bmc_irq(int irq, void *arg)
> +{
> +   struct bt_bmc *bt_bmc = arg;
> +   u32 reg;
> +   int rc;
> +
> +   rc = regmap_read(bt_bmc->map, bt_bmc->offset + MICROWATT_IRQ_STATUS, 
> );
> +   if (rc)
> +   return IRQ_NONE;
> +
> +   /* Interrupt wasn't something we knew about */
> +   if (!(reg & (IRQ_HOST_TO_BMC_ATTN | IRQ_HOST_NOT_BUSY)))
> +   return IRQ_NONE;
> +
> +   /* ack all pending IRQs */
> +   regmap_write(bt_bmc->map, bt_bmc->offset + MICROWATT_IRQ_STATUS, 0);
> +
> +   wake_up(_bmc->queue);
> +   return IRQ_HANDLED;
> +}
> +
>  static int aspeed_bt_bmc_config_irq(struct bt_bmc *bt_bmc,
>  struct platform_device *pdev)
>  {
> @@ -446,6 +472,48 @@ static const struct bt_bmc_ops aspeed_bt_bmc_ops = {
> .enable_bt = aspeed_enable_bt,
>  };
>
> +static int microwatt_bt_bmc_config_irq(struct bt_bmc *bt_bmc,
> +struct platform_device *pdev)
> +{
> +   struct device *dev = >dev;
> +   int rc;
> +
> +   bt_bmc->irq = platform_get_irq_optional(pdev, 0);
> +   if (bt_bmc->irq < 0)
> +   return bt_bmc->irq;
> +
> +   rc = devm_request_irq(dev, bt_bmc->irq, microwatt_bt_bmc_irq, 
> IRQF_SHARED,
> + DEVICE_NAME, bt_bmc);
> +   if (rc < 0) {
> +   dev_warn(dev, "Unable to request IRQ %d\n", bt_bmc->irq);
> +   bt_bmc->irq = rc;
> +   return rc;
> +   }
> +
> +   /*
> +* Configure the hardware to give us an interrupt whenever the H2B
> +* bit is set or the HBUSY bit is cleared.
> +*
> +* H2B will be asserted when the bmc has data for us; HBUSY
> +* will be cleared (along with B2H) when we can write the next
> +* message to the BT buffer
> +*/
> +   rc = 

Re: [PATCH 1/4] crypto: nintendo-aes - add a new AES driver

[Joel Stanley]

On Tue, 21 Sept 2021 at 21:47, Emmanuel Gil Peyrot
 wrote:
>
> This engine implements AES in CBC mode, using 128-bit keys only.  It is
> present on both the Wii and the Wii U, and is apparently identical in
> both consoles.
>
> The hardware is capable of firing an interrupt when the operation is
> done, but this driver currently uses a busy loop, I’m not too sure
> whether it would be preferable to switch, nor how to achieve that.
>
> It also supports a mode where no operation is done, and thus could be
> used as a DMA copy engine, but I don’t know how to expose that to the
> kernel or whether it would even be useful.
>
> In my testing, on a Wii U, this driver reaches 80.7 MiB/s, while the
> aes-generic driver only reaches 30.9 MiB/s, so it is a quite welcome
> speedup.
>
> This driver was written based on reversed documentation, see:
> https://wiibrew.org/wiki/Hardware/AES
>
> Signed-off-by: Emmanuel Gil Peyrot 
> Tested-by: Emmanuel Gil Peyrot   # on Wii U
> ---
>  drivers/crypto/Kconfig|  11 ++
>  drivers/crypto/Makefile   |   1 +
>  drivers/crypto/nintendo-aes.c | 273 ++
>  3 files changed, 285 insertions(+)
>  create mode 100644 drivers/crypto/nintendo-aes.c
>
> diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig
> index 9a4c275a1335..adc94ad7462d 100644
> --- a/drivers/crypto/Kconfig
> +++ b/drivers/crypto/Kconfig
> @@ -871,4 +871,15 @@ config CRYPTO_DEV_SA2UL
>
>  source "drivers/crypto/keembay/Kconfig"
>
> +config CRYPTO_DEV_NINTENDO
> +   tristate "Support for the Nintendo Wii U AES engine"
> +   depends on WII || WIIU || COMPILE_TEST

This current seteup will allow the driver to be compile tested for
non-powerpc, which will fail on the dcbf instructions.

Perhaps use this instead:

   depends on WII || WIIU || (COMPILE_TEST && PPC)

> +   select CRYPTO_AES
> +   help
> + Say 'Y' here to use the Nintendo Wii or Wii U on-board AES
> + engine for the CryptoAPI AES algorithm.
> +
> + To compile this driver as a module, choose M here: the module
> + will be called nintendo-aes.
> +
>  endif # CRYPTO_HW
> diff --git a/drivers/crypto/Makefile b/drivers/crypto/Makefile
> index fa22cb19e242..004dae7bbf39 100644
> --- a/drivers/crypto/Makefile
> +++ b/drivers/crypto/Makefile
> @@ -22,6 +22,7 @@ obj-$(CONFIG_CRYPTO_DEV_MARVELL) += marvell/
>  obj-$(CONFIG_CRYPTO_DEV_MXS_DCP) += mxs-dcp.o
>  obj-$(CONFIG_CRYPTO_DEV_NIAGARA2) += n2_crypto.o
>  n2_crypto-y := n2_core.o n2_asm.o
> +obj-$(CONFIG_CRYPTO_DEV_NINTENDO) += nintendo-aes.o
>  obj-$(CONFIG_CRYPTO_DEV_NX) += nx/
>  obj-$(CONFIG_CRYPTO_DEV_OMAP) += omap-crypto.o
>  obj-$(CONFIG_CRYPTO_DEV_OMAP_AES) += omap-aes-driver.o
> diff --git a/drivers/crypto/nintendo-aes.c b/drivers/crypto/nintendo-aes.c
> new file mode 100644
> index ..79ae77500999
> --- /dev/null
> +++ b/drivers/crypto/nintendo-aes.c
> @@ -0,0 +1,273 @@
> +/*
> + * Copyright (C) 2021 Emmanuel Gil Peyrot 
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 and
> + * only version 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.

The  kernel uses the SDPX header instead of pasting the text.

> +static int
> +do_crypt(const void *src, void *dst, u32 len, u32 flags)
> +{
> +   u32 blocks = ((len >> 4) - 1) & AES_CTRL_BLOCK;
> +   u32 status;
> +   u32 counter = OP_TIMEOUT;
> +   u32 i;
> +
> +   /* Flush out all of src, we can’t know whether any of it is in cache 
> */
> +   for (i = 0; i < len; i += 32)
> +   __asm__("dcbf 0, %0" : : "r" (src + i));
> +   __asm__("sync" : : : "memory");

This could be flush_dcache_range, from asm/cacheflush.h

> +
> +   /* Set the addresses for DMA */
> +   iowrite32be(virt_to_phys((void *)src), base + AES_SRC);
> +   iowrite32be(virt_to_phys(dst), base + AES_DEST);
> +
> +   /* Start the operation */
> +   iowrite32be(flags | blocks, base + AES_CTRL);
> +
> +   /* TODO: figure out how to use interrupts here, this will probably
> +* lower throughput but let the CPU do other things while the AES
> +* engine is doing its work. */
> +   do {
> +   status = ioread32be(base + AES_CTRL);
> +   cpu_relax();
> +   } while ((status & AES_CTRL_EXEC) && --counter);

You could add a msleep in here?

Consider using readl_poll_timeout().

Cheers,

Joel

> +
> +   /* Do we ever get called with dst ≠ src?  If so we have to invalidate
> +* dst in addition to the earlier flush of src. */
> +   if (unlikely(dst != src)) {
> +   for (i = 0; i < len; i += 32)
> +  

Re: [PATCH] mm: Remove HARDENED_USERCOPY_FALLBACK

[Joel Stanley]

On Tue, 21 Sept 2021 at 09:50, Stephen Kitt  wrote:
>
> This has served its purpose and is no longer used. All usercopy
> violations appear to have been handled by now, any remaining
> instances (or new bugs) will cause copies to be rejected.
>
> This isn't a direct revert of commit 2d891fbc3bb6 ("usercopy: Allow
> strict enforcement of whitelists"); since usercopy_fallback is
> effectively 0, the fallback handling is removed too.
>
> This also removes the usercopy_fallback module parameter on
> slab_common.
>
> Link: https://github.com/KSPP/linux/issues/153
> Signed-off-by: Stephen Kitt 
> Suggested-by: Kees Cook 
> ---
>  arch/powerpc/configs/skiroot_defconfig |  1 -

For the defconfig change:

Reviewed-by: Joel Stanley 

Cheers,

Joel

>  include/linux/slab.h   |  2 --
>  mm/slab.c  | 13 -
>  mm/slab_common.c   |  8 
>  mm/slub.c  | 14 --
>  security/Kconfig   | 14 --
>  6 files changed, 52 deletions(-)
>
> diff --git a/arch/powerpc/configs/skiroot_defconfig 
> b/arch/powerpc/configs/skiroot_defconfig
> index b806a5d3a695..c3ba614c973d 100644
> --- a/arch/powerpc/configs/skiroot_defconfig
> +++ b/arch/powerpc/configs/skiroot_defconfig
> @@ -275,7 +275,6 @@ CONFIG_NLS_UTF8=y
>  CONFIG_ENCRYPTED_KEYS=y
>  CONFIG_SECURITY=y
>  CONFIG_HARDENED_USERCOPY=y
> -# CONFIG_HARDENED_USERCOPY_FALLBACK is not set
>  CONFIG_HARDENED_USERCOPY_PAGESPAN=y
>  CONFIG_FORTIFY_SOURCE=y
>  CONFIG_SECURITY_LOCKDOWN_LSM=y
> diff --git a/include/linux/slab.h b/include/linux/slab.h
> index 0c97d788762c..5b21515afae0 100644
> --- a/include/linux/slab.h
> +++ b/include/linux/slab.h
> @@ -142,8 +142,6 @@ struct mem_cgroup;
>  void __init kmem_cache_init(void);
>  bool slab_is_available(void);
>
> -extern bool usercopy_fallback;
> -
>  struct kmem_cache *kmem_cache_create(const char *name, unsigned int size,
> unsigned int align, slab_flags_t flags,
> void (*ctor)(void *));
> diff --git a/mm/slab.c b/mm/slab.c
> index d0f725637663..4d826394ffcb 100644
> --- a/mm/slab.c
> +++ b/mm/slab.c
> @@ -4207,19 +4207,6 @@ void __check_heap_object(const void *ptr, unsigned 
> long n, struct page *page,
> n <= cachep->useroffset - offset + cachep->usersize)
> return;
>
> -   /*
> -* If the copy is still within the allocated object, produce
> -* a warning instead of rejecting the copy. This is intended
> -* to be a temporary method to find any missing usercopy
> -* whitelists.
> -*/
> -   if (usercopy_fallback &&
> -   offset <= cachep->object_size &&
> -   n <= cachep->object_size - offset) {
> -   usercopy_warn("SLAB object", cachep->name, to_user, offset, 
> n);
> -   return;
> -   }
> -
> usercopy_abort("SLAB object", cachep->name, to_user, offset, n);
>  }
>  #endif /* CONFIG_HARDENED_USERCOPY */
> diff --git a/mm/slab_common.c b/mm/slab_common.c
> index a4a571428c51..925b00c1d4e8 100644
> --- a/mm/slab_common.c
> +++ b/mm/slab_common.c
> @@ -37,14 +37,6 @@ LIST_HEAD(slab_caches);
>  DEFINE_MUTEX(slab_mutex);
>  struct kmem_cache *kmem_cache;
>
> -#ifdef CONFIG_HARDENED_USERCOPY
> -bool usercopy_fallback __ro_after_init =
> -   IS_ENABLED(CONFIG_HARDENED_USERCOPY_FALLBACK);
> -module_param(usercopy_fallback, bool, 0400);
> -MODULE_PARM_DESC(usercopy_fallback,
> -   "WARN instead of reject usercopy whitelist violations");
> -#endif
> -
>  static LIST_HEAD(slab_caches_to_rcu_destroy);
>  static void slab_caches_to_rcu_destroy_workfn(struct work_struct *work);
>  static DECLARE_WORK(slab_caches_to_rcu_destroy_work,
> diff --git a/mm/slub.c b/mm/slub.c
> index 3f96e099817a..77f53e76a3c3 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -4125,7 +4125,6 @@ void __check_heap_object(const void *ptr, unsigned long 
> n, struct page *page,
>  {
> struct kmem_cache *s;
> unsigned int offset;
> -   size_t object_size;
> bool is_kfence = is_kfence_address(ptr);
>
> ptr = kasan_reset_tag(ptr);
> @@ -4158,19 +4157,6 @@ void __check_heap_object(const void *ptr, unsigned 
> long n, struct page *page,
> n <= s->useroffset - offset + s->usersize)
> return;
>
> -   /*
> -* If the copy is still within the allocated object, produce
> -* a warning instead of rejecting the copy. This is int

Re: [PATCH trivial v2] powerpc/powernv/dump: Fix typo in comment

[Joel Stanley]

On Tue, 14 Sept 2021 at 14:38, Vasant Hegde
 wrote:
>
> Signed-off-by: Vasant Hegde 

Reviewed-by: Joel Stanley 

> ---
>  arch/powerpc/platforms/powernv/opal-dump.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/platforms/powernv/opal-dump.c 
> b/arch/powerpc/platforms/powernv/opal-dump.c
> index 00c5a59d82d9..717d1d30ade5 100644
> --- a/arch/powerpc/platforms/powernv/opal-dump.c
> +++ b/arch/powerpc/platforms/powernv/opal-dump.c
> @@ -419,7 +419,7 @@ void __init opal_platform_dump_init(void)
> int rc;
> int dump_irq;
>
> -   /* ELOG not supported by firmware */
> +   /* Dump not supported by firmware */
> if (!opal_check_token(OPAL_DUMP_READ))
> return;
>
> --
> 2.31.1
>

Re: [PATCH trivial] powerpc/powernv/dump: Fix typo is comment

[Joel Stanley]

On Tue, 14 Sept 2021 at 10:17, Vasant Hegde
 wrote:
>
> Signed-off-by: Vasant Hegde 

There's a typo in your commit message :)

> ---
>  arch/powerpc/platforms/powernv/opal-dump.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/platforms/powernv/opal-dump.c 
> b/arch/powerpc/platforms/powernv/opal-dump.c
> index 00c5a59d82d9..717d1d30ade5 100644
> --- a/arch/powerpc/platforms/powernv/opal-dump.c
> +++ b/arch/powerpc/platforms/powernv/opal-dump.c
> @@ -419,7 +419,7 @@ void __init opal_platform_dump_init(void)
> int rc;
> int dump_irq;
>
> -   /* ELOG not supported by firmware */
> +   /* Dump not supported by firmware */
> if (!opal_check_token(OPAL_DUMP_READ))
> return;
>
> --
> 2.31.1
>

Re: [PATCH] powerpc/xics: Set the IRQ chip data for the ICS native backend

[Joel Stanley]

On Mon, 13 Sept 2021 at 13:48, Cédric Le Goater  wrote:
>
> The ICS native driver relies on the IRQ chip data to find the struct
> 'ics_native' describing the ICS controller but it was removed by commit
> 248af248a8f4 ("powerpc/xics: Rename the map handler in a check handler").
> Revert this change to fix the Microwatt SoC platform.
>
> Fixes: 248af248a8f4 ("powerpc/xics: Rename the map handler in a check 
> handler")
> Signed-off-by: Cédric Le Goater 

Reviewed-by: Joel Stanley 

Thanks Cédric.

> ---
>  arch/powerpc/sysdev/xics/xics-common.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/sysdev/xics/xics-common.c 
> b/arch/powerpc/sysdev/xics/xics-common.c
> index 5c1a157a83b8..244a727c6ba4 100644
> --- a/arch/powerpc/sysdev/xics/xics-common.c
> +++ b/arch/powerpc/sysdev/xics/xics-common.c
> @@ -348,9 +348,9 @@ static int xics_host_map(struct irq_domain *domain, 
> unsigned int virq,
> if (xics_ics->check(xics_ics, hwirq))
> return -EINVAL;
>
> -   /* No chip data for the XICS domain */
> +   /* Let the ICS be the chip data for the XICS domain. For ICS native */
> irq_domain_set_info(domain, virq, hwirq, xics_ics->chip,
> -   NULL, handle_fasteoi_irq, NULL, NULL);
> +   xics_ics, handle_fasteoi_irq, NULL, NULL);
>
> return 0;
>  }
> --
> 2.31.1
>

[PATCH 3/3] powerpc/configs/microwatt: Enable options for systemd

[Joel Stanley]

When booting with systemd these options are required.

This increases the image by about 50KB, or 2%.

Signed-off-by: Joel Stanley 
---
 arch/powerpc/configs/microwatt_defconfig | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/configs/microwatt_defconfig 
b/arch/powerpc/configs/microwatt_defconfig
index 2f8b1fec9a16..4a4924cd056e 100644
--- a/arch/powerpc/configs/microwatt_defconfig
+++ b/arch/powerpc/configs/microwatt_defconfig
@@ -5,6 +5,7 @@ CONFIG_PREEMPT_VOLUNTARY=y
 CONFIG_TICK_CPU_ACCOUNTING=y
 CONFIG_LOG_BUF_SHIFT=16
 CONFIG_PRINTK_SAFE_LOG_BUF_SHIFT=12
+CONFIG_CGROUPS=y
 CONFIG_BLK_DEV_INITRD=y
 CONFIG_CC_OPTIMIZE_FOR_SIZE=y
 CONFIG_KALLSYMS_ALL=y
@@ -77,8 +78,10 @@ CONFIG_SPI_SPIDEV=y
 CONFIG_EXT4_FS=y
 # CONFIG_FILE_LOCKING is not set
 # CONFIG_DNOTIFY is not set
-# CONFIG_INOTIFY_USER is not set
+CONFIG_AUTOFS_FS=y
+CONFIG_TMPFS=y
 # CONFIG_MISC_FILESYSTEMS is not set
+CONFIG_CRYPTO_SHA256=y
 # CONFIG_CRYPTO_HW is not set
 # CONFIG_XZ_DEC_X86 is not set
 # CONFIG_XZ_DEC_IA64 is not set
-- 
2.33.0

[PATCH 2/3] powerpc/configs/microwattt: Enable Liteeth

[Joel Stanley]

Liteeth is the network device used by Microwatt.

Signed-off-by: Joel Stanley 
---
 arch/powerpc/configs/microwatt_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/powerpc/configs/microwatt_defconfig 
b/arch/powerpc/configs/microwatt_defconfig
index a08b739123da..2f8b1fec9a16 100644
--- a/arch/powerpc/configs/microwatt_defconfig
+++ b/arch/powerpc/configs/microwatt_defconfig
@@ -53,6 +53,7 @@ CONFIG_MTD_SPI_NOR=y
 CONFIG_BLK_DEV_LOOP=y
 CONFIG_BLK_DEV_RAM=y
 CONFIG_NETDEVICES=y
+CONFIG_LITEX_LITEETH=y
 # CONFIG_WLAN is not set
 # CONFIG_INPUT is not set
 # CONFIG_SERIO is not set
-- 
2.33.0

[PATCH 1/3] powerpc/microwatt: Add Ethernet to device tree

[Joel Stanley]

The liteeth network device is used in the Microwatt soc.

Signed-off-by: Joel Stanley 
---
 arch/powerpc/boot/dts/microwatt.dts | 12 
 1 file changed, 12 insertions(+)

diff --git a/arch/powerpc/boot/dts/microwatt.dts 
b/arch/powerpc/boot/dts/microwatt.dts
index 974abbdda249..65b270a90f94 100644
--- a/arch/powerpc/boot/dts/microwatt.dts
+++ b/arch/powerpc/boot/dts/microwatt.dts
@@ -127,6 +127,18 @@ UART0: serial@2000 {
fifo-size = <16>;
interrupts = <0x10 0x1>;
};
+
+   ethernet@802 {
+   compatible = "litex,liteeth";
+   reg = <0x8021000 0x100
+   0x8020800 0x100
+   0x803 0x2000>;
+   reg-names = "mac", "mido", "buffer";
+   litex,rx-slots = <2>;
+   litex,tx-slots = <2>;
+   litex,slot-size = <0x800>;
+   interrupts = <0x11 0x1>;
+   };
};
 
chosen {
-- 
2.33.0

[PATCH 0/3] powerpc/microwatt: Device tree and config updates

[Joel Stanley]

This enables the liteeth network device for microwatt which will be
merged in v5.15.

It also turns on some options so the microwatt defconfig can be used to
boot a userspace with systemd.

Joel Stanley (3):
  powerpc/microwatt: Add Ethernet to device tree
  powerpc/configs/microwattt: Enable Liteeth
  powerpc/configs/microwatt: Enable options for systemd

 arch/powerpc/boot/dts/microwatt.dts  | 12 
 arch/powerpc/configs/microwatt_defconfig |  6 +-
 2 files changed, 17 insertions(+), 1 deletion(-)

-- 
2.33.0

[PATCH v2 3/3] powerpc/configs: Regenerate mpc885_ads_defconfig

[Joel Stanley]

Regenerate atop v5.14-rc6 by doing a make savedefconfig.

The changes a re-ordering except for the following (which are still set
indirectly):

 - CONFIG_DEBUG_KERNEL=y selected by EXPERT

 - CONFIG_PPC_EARLY_DEBUG_CPM_ADDR=0xff002008 which is the default
   setting

Signed-off-by: Joel Stanley 
---
 arch/powerpc/configs/mpc885_ads_defconfig | 46 +++
 1 file changed, 22 insertions(+), 24 deletions(-)

diff --git a/arch/powerpc/configs/mpc885_ads_defconfig 
b/arch/powerpc/configs/mpc885_ads_defconfig
index cd08f9ed2c8d..c74dc76b1d0d 100644
--- a/arch/powerpc/configs/mpc885_ads_defconfig
+++ b/arch/powerpc/configs/mpc885_ads_defconfig
@@ -1,19 +1,30 @@
-CONFIG_PPC_8xx=y
 # CONFIG_SWAP is not set
 CONFIG_SYSVIPC=y
 CONFIG_NO_HZ=y
 CONFIG_HIGH_RES_TIMERS=y
+CONFIG_BPF_JIT=y
+CONFIG_VIRT_CPU_ACCOUNTING_NATIVE=y
 CONFIG_LOG_BUF_SHIFT=14
 CONFIG_EXPERT=y
 # CONFIG_ELF_CORE is not set
 # CONFIG_BASE_FULL is not set
 # CONFIG_FUTEX is not set
+CONFIG_PERF_EVENTS=y
 # CONFIG_VM_EVENT_COUNTERS is not set
-# CONFIG_BLK_DEV_BSG is not set
-CONFIG_PARTITION_ADVANCED=y
+CONFIG_PPC_8xx=y
+CONFIG_8xx_GPIO=y
+CONFIG_SMC_UCODE_PATCH=y
+CONFIG_PIN_TLB=y
 CONFIG_GEN_RTC=y
 CONFIG_HZ_100=y
+CONFIG_MATH_EMULATION=y
+CONFIG_PPC_16K_PAGES=y
+CONFIG_ADVANCED_OPTIONS=y
 # CONFIG_SECCOMP is not set
+CONFIG_STRICT_KERNEL_RWX=y
+CONFIG_MODULES=y
+# CONFIG_BLK_DEV_BSG is not set
+CONFIG_PARTITION_ADVANCED=y
 CONFIG_NET=y
 CONFIG_PACKET=y
 CONFIG_UNIX=y
@@ -46,38 +57,25 @@ CONFIG_DAVICOM_PHY=y
 # CONFIG_LEGACY_PTYS is not set
 CONFIG_SERIAL_CPM=y
 CONFIG_SERIAL_CPM_CONSOLE=y
+CONFIG_SPI=y
+CONFIG_SPI_FSL_SPI=y
 # CONFIG_HWMON is not set
+CONFIG_WATCHDOG=y
+CONFIG_8xxx_WDT=y
 # CONFIG_USB_SUPPORT is not set
 # CONFIG_DNOTIFY is not set
 CONFIG_TMPFS=y
 CONFIG_CRAMFS=y
 CONFIG_NFS_FS=y
 CONFIG_ROOT_NFS=y
+CONFIG_CRYPTO=y
+CONFIG_CRYPTO_DEV_TALITOS=y
 CONFIG_CRC32_SLICEBY4=y
 CONFIG_DEBUG_INFO=y
 CONFIG_MAGIC_SYSRQ=y
-CONFIG_DETECT_HUNG_TASK=y
-CONFIG_PPC_16K_PAGES=y
-CONFIG_DEBUG_KERNEL=y
 CONFIG_DEBUG_FS=y
-CONFIG_PPC_PTDUMP=y
-CONFIG_MODULES=y
-CONFIG_SPI=y
-CONFIG_SPI_FSL_SPI=y
-CONFIG_CRYPTO=y
-CONFIG_CRYPTO_DEV_TALITOS=y
-CONFIG_8xx_GPIO=y
-CONFIG_WATCHDOG=y
-CONFIG_8xxx_WDT=y
-CONFIG_SMC_UCODE_PATCH=y
-CONFIG_ADVANCED_OPTIONS=y
-CONFIG_PIN_TLB=y
-CONFIG_PERF_EVENTS=y
-CONFIG_MATH_EMULATION=y
-CONFIG_VIRT_CPU_ACCOUNTING_NATIVE=y
-CONFIG_STRICT_KERNEL_RWX=y
-CONFIG_BPF_JIT=y
 CONFIG_DEBUG_VM_PGTABLE=y
+CONFIG_DETECT_HUNG_TASK=y
 CONFIG_BDI_SWITCH=y
 CONFIG_PPC_EARLY_DEBUG=y
-CONFIG_PPC_EARLY_DEBUG_CPM_ADDR=0xff002008
+CONFIG_PPC_PTDUMP=y
-- 
2.32.0

[PATCH v2 2/3] powerpc/config: Renable MTD_PHYSMAP_OF

[Joel Stanley]

CONFIG_MTD_PHYSMAP_OF is not longer enabled as it depends on
MTD_PHYSMAP which is not enabled.

This is a regression from commit 642b1e8dbed7 ("mtd: maps: Merge
physmap_of.c into physmap-core.c"), which added the extra dependency.
Add CONFIG_MTD_PHYSMAP=y so this stays in the config, as Christophe said
it is useful for build coverage.

Fixes: 642b1e8dbed7 ("mtd: maps: Merge physmap_of.c into physmap-core.c")
Signed-off-by: Joel Stanley 
---
 arch/powerpc/configs/mpc885_ads_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/powerpc/configs/mpc885_ads_defconfig 
b/arch/powerpc/configs/mpc885_ads_defconfig
index 5cd17adf903f..cd08f9ed2c8d 100644
--- a/arch/powerpc/configs/mpc885_ads_defconfig
+++ b/arch/powerpc/configs/mpc885_ads_defconfig
@@ -33,6 +33,7 @@ CONFIG_MTD_CFI_GEOMETRY=y
 # CONFIG_MTD_CFI_I2 is not set
 CONFIG_MTD_CFI_I4=y
 CONFIG_MTD_CFI_AMDSTD=y
+CONFIG_MTD_PHYSMAP=y
 CONFIG_MTD_PHYSMAP_OF=y
 # CONFIG_BLK_DEV is not set
 CONFIG_NETDEVICES=y
-- 
2.32.0

[PATCH v2 1/3] powerpc/config: Fix IPV6 warning in mpc855_ads

[Joel Stanley]

When building this config there's a warning:

  79:warning: override: reassigning to symbol IPV6

Commit 9a1762a4a4ff ("powerpc/8xx: Update mpc885_ads_defconfig to
improve CI") added CONFIG_IPV6=y, but left '# CONFIG_IPV6 is not set'
in.

IPV6 is default y, so remove both to clean up the build.

Acked-by: Christophe Leroy 
Fixes: 9a1762a4a4ff ("powerpc/8xx: Update mpc885_ads_defconfig to improve CI")
Signed-off-by: Joel Stanley 
---
 arch/powerpc/configs/mpc885_ads_defconfig | 2 --
 1 file changed, 2 deletions(-)

diff --git a/arch/powerpc/configs/mpc885_ads_defconfig 
b/arch/powerpc/configs/mpc885_ads_defconfig
index d21f266cea9a..5cd17adf903f 100644
--- a/arch/powerpc/configs/mpc885_ads_defconfig
+++ b/arch/powerpc/configs/mpc885_ads_defconfig
@@ -21,7 +21,6 @@ CONFIG_INET=y
 CONFIG_IP_MULTICAST=y
 CONFIG_IP_PNP=y
 CONFIG_SYN_COOKIES=y
-# CONFIG_IPV6 is not set
 # CONFIG_FW_LOADER is not set
 CONFIG_MTD=y
 CONFIG_MTD_BLOCK=y
@@ -76,7 +75,6 @@ CONFIG_PERF_EVENTS=y
 CONFIG_MATH_EMULATION=y
 CONFIG_VIRT_CPU_ACCOUNTING_NATIVE=y
 CONFIG_STRICT_KERNEL_RWX=y
-CONFIG_IPV6=y
 CONFIG_BPF_JIT=y
 CONFIG_DEBUG_VM_PGTABLE=y
 CONFIG_BDI_SWITCH=y
-- 
2.32.0

[PATCH v2 0/3] powerpc: mpc855_ads defconfig fixes

[Joel Stanley]

v2: fix typos, split out mtd fix from savedefconfig patch

The first patch fixes a build warning I noticed when testing something
unrelated.

The second fixes a regression where the MTD partition support dropped out
of the config. I have enabled the dependency so this is still part of
the config.

The third patch is the result of doing a make savedefconfig with the
first two patches applied.

Joel Stanley (3):
  powerpc/config: Fix IPV6 warning in mpc855_ads
  powerpc/config: Renable MTD_PHYSMAP_OF
  powerpc/configs: Regenerate mpc885_ads_defconfig

 arch/powerpc/configs/mpc885_ads_defconfig | 49 +++
 1 file changed, 23 insertions(+), 26 deletions(-)

-- 
2.32.0

Re: [PATCH 2/2] powerpc/configs: Regenerate mpc885_ads_defconfig

[Joel Stanley]

On Mon, 16 Aug 2021 at 08:49, Christophe Leroy
 wrote:
>
>
>
> Le 16/08/2021 à 10:31, Joel Stanley a écrit :
> > Regenrate atop v5.14-rc6.
>
> Typos.
>
> You mean you did redo a "make savedefconfig" ?

Yes, I did.

Re: [PATCH 2/3] powerpc: Fix undefined static key

[Joel Stanley]

On Mon, 16 Aug 2021 at 08:39, Christophe Leroy
 wrote:
>
>
>
> Le 16/08/2021 à 10:24, Joel Stanley a écrit :
> > When CONFIG_PPC_BARRIER_NOSPEC=n, security.c is not built leading to a
> > missing definition of uaccess_flush_key.
> >
> >LD  vmlinux.o
> >MODPOST vmlinux.symvers
> >MODINFO modules.builtin.modinfo
> >GEN modules.builtin
> >LD  .tmp_vmlinux.kallsyms1
> > powerpc64le-linux-gnu-ld: arch/powerpc/kernel/align.o:(.toc+0x0): undefined 
> > reference to `uaccess_flush_key'
> > powerpc64le-linux-gnu-ld: arch/powerpc/kernel/signal_64.o:(.toc+0x0): 
> > undefined reference to `uaccess_flush_key'
> > powerpc64le-linux-gnu-ld: arch/powerpc/kernel/process.o:(.toc+0x0): 
> > undefined reference to `uaccess_flush_key'
> > powerpc64le-linux-gnu-ld: arch/powerpc/kernel/traps.o:(.toc+0x0): undefined 
> > reference to `uaccess_flush_key'
> > powerpc64le-linux-gnu-ld: 
> > arch/powerpc/kernel/hw_breakpoint_constraints.o:(.toc+0x0): undefined 
> > reference to `uaccess_flush_key'
> > powerpc64le-linux-gnu-ld: arch/powerpc/kernel/ptrace/ptrace.o:(.toc+0x0): 
> > more undefined references to `uaccess_flush_key' follow
> > make[1]: *** [Makefile:1176: vmlinux] Error 1
> >
> > Hack one in to fix the build.
> >
> > Signed-off-by: Joel Stanley 
> > ---
> >   arch/powerpc/include/asm/security_features.h | 3 +++
> >   1 file changed, 3 insertions(+)
> >
> > diff --git a/arch/powerpc/include/asm/security_features.h 
> > b/arch/powerpc/include/asm/security_features.h
> > index 792eefaf230b..46ade7927a4c 100644
> > --- a/arch/powerpc/include/asm/security_features.h
> > +++ b/arch/powerpc/include/asm/security_features.h
> > @@ -39,6 +39,9 @@ static inline bool security_ftr_enabled(u64 feature)
> >   return !!(powerpc_security_features & feature);
> >   }
> >
> > +#ifndef CONFIG_PPC_BARRIER_NOSPEC
> > +DEFINE_STATIC_KEY_FALSE(uaccess_flush_key);
> > +#endif
>
> It will then be re-defined by each file that includes asm/security_features.h 
> 
>
> You can't use a DEFINE_ in a .h
>
> You have to fix the problem at its source.
>
> Cleanest way I see it to modify asm/book3s/64/kup.h and something like
>
> if (IS_ENABLED(CONFIG_PPC_BARRIER_NOSPEC) && 
> static_branch_unlikely(_flush_key)

This won't work either as there's no declaration for uaccess_flush_key:

arch/powerpc/include/asm/book3s/64/kup.h:411:78: error:
‘uaccess_flush_key’ undeclared (first use in this function)

We could add an extern for it, but that is distasteful as the static
key API suggests using the structs directly is deprecated, and the
macros we're supposed to use perform initialisation.

Could we assume microwatt-like platforms will not gain pkeys support,
and have a stubbed out set of prevent/restore_user_access for systems
that turn off either pkeys or BARRIER_NOSPEC?

Or do we get rid of PPC_BARRIER_NOSPEC as an option, and have machines
rely on disabling it at runtime?

[PATCH 2/2] powerpc/configs: Regenerate mpc885_ads_defconfig

[Joel Stanley]

Regenrate atop v5.14-rc6.

The chagnes are mostly re-ordering, except for the following which fall
out due to dependenacies:

 - CONFIG_DEBUG_KERNEL=y selected by EXPERT

 - CONFIG_PPC_EARLY_DEBUG_CPM_ADDR=0xff002008 which is the default
   setting

CONFIG_MTD_PHYSMAP_OF is not longer enabled, as it depends on
MTD_PHYSMAP which is not enabled. This is a regression from commit
642b1e8dbed7 ("mtd: maps: Merge physmap_of.c into physmap-core.c"),
which added the extra dependency. Add CONFIG_MTD_PHYSMAP=y so this stays
in the config.

Signed-off-by: Joel Stanley 
---
 arch/powerpc/configs/mpc885_ads_defconfig | 47 +++
 1 file changed, 23 insertions(+), 24 deletions(-)

diff --git a/arch/powerpc/configs/mpc885_ads_defconfig 
b/arch/powerpc/configs/mpc885_ads_defconfig
index 5cd17adf903f..c74dc76b1d0d 100644
--- a/arch/powerpc/configs/mpc885_ads_defconfig
+++ b/arch/powerpc/configs/mpc885_ads_defconfig
@@ -1,19 +1,30 @@
-CONFIG_PPC_8xx=y
 # CONFIG_SWAP is not set
 CONFIG_SYSVIPC=y
 CONFIG_NO_HZ=y
 CONFIG_HIGH_RES_TIMERS=y
+CONFIG_BPF_JIT=y
+CONFIG_VIRT_CPU_ACCOUNTING_NATIVE=y
 CONFIG_LOG_BUF_SHIFT=14
 CONFIG_EXPERT=y
 # CONFIG_ELF_CORE is not set
 # CONFIG_BASE_FULL is not set
 # CONFIG_FUTEX is not set
+CONFIG_PERF_EVENTS=y
 # CONFIG_VM_EVENT_COUNTERS is not set
-# CONFIG_BLK_DEV_BSG is not set
-CONFIG_PARTITION_ADVANCED=y
+CONFIG_PPC_8xx=y
+CONFIG_8xx_GPIO=y
+CONFIG_SMC_UCODE_PATCH=y
+CONFIG_PIN_TLB=y
 CONFIG_GEN_RTC=y
 CONFIG_HZ_100=y
+CONFIG_MATH_EMULATION=y
+CONFIG_PPC_16K_PAGES=y
+CONFIG_ADVANCED_OPTIONS=y
 # CONFIG_SECCOMP is not set
+CONFIG_STRICT_KERNEL_RWX=y
+CONFIG_MODULES=y
+# CONFIG_BLK_DEV_BSG is not set
+CONFIG_PARTITION_ADVANCED=y
 CONFIG_NET=y
 CONFIG_PACKET=y
 CONFIG_UNIX=y
@@ -33,6 +44,7 @@ CONFIG_MTD_CFI_GEOMETRY=y
 # CONFIG_MTD_CFI_I2 is not set
 CONFIG_MTD_CFI_I4=y
 CONFIG_MTD_CFI_AMDSTD=y
+CONFIG_MTD_PHYSMAP=y
 CONFIG_MTD_PHYSMAP_OF=y
 # CONFIG_BLK_DEV is not set
 CONFIG_NETDEVICES=y
@@ -45,38 +57,25 @@ CONFIG_DAVICOM_PHY=y
 # CONFIG_LEGACY_PTYS is not set
 CONFIG_SERIAL_CPM=y
 CONFIG_SERIAL_CPM_CONSOLE=y
+CONFIG_SPI=y
+CONFIG_SPI_FSL_SPI=y
 # CONFIG_HWMON is not set
+CONFIG_WATCHDOG=y
+CONFIG_8xxx_WDT=y
 # CONFIG_USB_SUPPORT is not set
 # CONFIG_DNOTIFY is not set
 CONFIG_TMPFS=y
 CONFIG_CRAMFS=y
 CONFIG_NFS_FS=y
 CONFIG_ROOT_NFS=y
+CONFIG_CRYPTO=y
+CONFIG_CRYPTO_DEV_TALITOS=y
 CONFIG_CRC32_SLICEBY4=y
 CONFIG_DEBUG_INFO=y
 CONFIG_MAGIC_SYSRQ=y
-CONFIG_DETECT_HUNG_TASK=y
-CONFIG_PPC_16K_PAGES=y
-CONFIG_DEBUG_KERNEL=y
 CONFIG_DEBUG_FS=y
-CONFIG_PPC_PTDUMP=y
-CONFIG_MODULES=y
-CONFIG_SPI=y
-CONFIG_SPI_FSL_SPI=y
-CONFIG_CRYPTO=y
-CONFIG_CRYPTO_DEV_TALITOS=y
-CONFIG_8xx_GPIO=y
-CONFIG_WATCHDOG=y
-CONFIG_8xxx_WDT=y
-CONFIG_SMC_UCODE_PATCH=y
-CONFIG_ADVANCED_OPTIONS=y
-CONFIG_PIN_TLB=y
-CONFIG_PERF_EVENTS=y
-CONFIG_MATH_EMULATION=y
-CONFIG_VIRT_CPU_ACCOUNTING_NATIVE=y
-CONFIG_STRICT_KERNEL_RWX=y
-CONFIG_BPF_JIT=y
 CONFIG_DEBUG_VM_PGTABLE=y
+CONFIG_DETECT_HUNG_TASK=y
 CONFIG_BDI_SWITCH=y
 CONFIG_PPC_EARLY_DEBUG=y
-CONFIG_PPC_EARLY_DEBUG_CPM_ADDR=0xff002008
+CONFIG_PPC_PTDUMP=y
-- 
2.32.0

[PATCH 1/2] powerpc/config: Fix IPV6 warning in mpc855_ads

[Joel Stanley]

When building this config there's a warning:

  79:warning: override: reassigning to symbol IPV6

Commit 9a1762a4a4ff ("powerpc/8xx: Update mpc885_ads_defconfig to
improve CI") added CONFIG_IPV6=y, but left '# CONFIG_IPV6 is not set'
in.

IPV6 is default y, so remove both to clean up the build.

Signed-off-by: Joel Stanley 
---
 arch/powerpc/configs/mpc885_ads_defconfig | 2 --
 1 file changed, 2 deletions(-)

diff --git a/arch/powerpc/configs/mpc885_ads_defconfig 
b/arch/powerpc/configs/mpc885_ads_defconfig
index d21f266cea9a..5cd17adf903f 100644
--- a/arch/powerpc/configs/mpc885_ads_defconfig
+++ b/arch/powerpc/configs/mpc885_ads_defconfig
@@ -21,7 +21,6 @@ CONFIG_INET=y
 CONFIG_IP_MULTICAST=y
 CONFIG_IP_PNP=y
 CONFIG_SYN_COOKIES=y
-# CONFIG_IPV6 is not set
 # CONFIG_FW_LOADER is not set
 CONFIG_MTD=y
 CONFIG_MTD_BLOCK=y
@@ -76,7 +75,6 @@ CONFIG_PERF_EVENTS=y
 CONFIG_MATH_EMULATION=y
 CONFIG_VIRT_CPU_ACCOUNTING_NATIVE=y
 CONFIG_STRICT_KERNEL_RWX=y
-CONFIG_IPV6=y
 CONFIG_BPF_JIT=y
 CONFIG_DEBUG_VM_PGTABLE=y
 CONFIG_BDI_SWITCH=y
-- 
2.32.0

[PATCH 0/2] powerpc: mpc855_ads defconfig fixes

[Joel Stanley]

The first was a build warning I noticed when testing something
unrelated.

I took a moment to look into it, and came up with the second patch which
updates the defconfig to make it easier to maintain in the future

It also fixes a regression where the MTD partition support dropped out
of the config. Given noone noticed the regression since v4.20 was
released, perhaps it could be left disabled?

Joel Stanley (2):
  powerpc/config: Fix IPV6 warning in mpc855_ads
  powerpc/configs: Regenerate mpc885_ads_defconfig

 arch/powerpc/configs/mpc885_ads_defconfig | 49 +++
 1 file changed, 23 insertions(+), 26 deletions(-)

-- 
2.32.0

[PATCH 3/3] powerpc/microwatt: CPU doesn't (yet) have speculation bugs

[Joel Stanley]

Signed-off-by: Joel Stanley 
---
 arch/powerpc/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 663766fbf505..d5af6667c206 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -279,6 +279,7 @@ config PPC_BARRIER_NOSPEC
bool
default y
depends on PPC_BOOK3S_64 || PPC_FSL_BOOK3E
+   depends on !PPC_MICROWATT
 
 config EARLY_PRINTK
bool
-- 
2.32.0

[PATCH 2/3] powerpc: Fix undefined static key

[Joel Stanley]

When CONFIG_PPC_BARRIER_NOSPEC=n, security.c is not built leading to a
missing definition of uaccess_flush_key.

  LD  vmlinux.o
  MODPOST vmlinux.symvers
  MODINFO modules.builtin.modinfo
  GEN modules.builtin
  LD  .tmp_vmlinux.kallsyms1
powerpc64le-linux-gnu-ld: arch/powerpc/kernel/align.o:(.toc+0x0): undefined 
reference to `uaccess_flush_key'
powerpc64le-linux-gnu-ld: arch/powerpc/kernel/signal_64.o:(.toc+0x0): undefined 
reference to `uaccess_flush_key'
powerpc64le-linux-gnu-ld: arch/powerpc/kernel/process.o:(.toc+0x0): undefined 
reference to `uaccess_flush_key'
powerpc64le-linux-gnu-ld: arch/powerpc/kernel/traps.o:(.toc+0x0): undefined 
reference to `uaccess_flush_key'
powerpc64le-linux-gnu-ld: 
arch/powerpc/kernel/hw_breakpoint_constraints.o:(.toc+0x0): undefined reference 
to `uaccess_flush_key'
powerpc64le-linux-gnu-ld: arch/powerpc/kernel/ptrace/ptrace.o:(.toc+0x0): more 
undefined references to `uaccess_flush_key' follow
make[1]: *** [Makefile:1176: vmlinux] Error 1

Hack one in to fix the build.

Signed-off-by: Joel Stanley 
---
 arch/powerpc/include/asm/security_features.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/powerpc/include/asm/security_features.h 
b/arch/powerpc/include/asm/security_features.h
index 792eefaf230b..46ade7927a4c 100644
--- a/arch/powerpc/include/asm/security_features.h
+++ b/arch/powerpc/include/asm/security_features.h
@@ -39,6 +39,9 @@ static inline bool security_ftr_enabled(u64 feature)
return !!(powerpc_security_features & feature);
 }
 
+#ifndef CONFIG_PPC_BARRIER_NOSPEC
+DEFINE_STATIC_KEY_FALSE(uaccess_flush_key);
+#endif
 
 // Features indicating support for Spectre/Meltdown mitigations
 
-- 
2.32.0

[PATCH 1/3] powerpc/64s: Fix build when !PPC_BARRIER_NOSPEC

[Joel Stanley]

When disabling PPC_BARRIER_NOSPEC the do_barrier_nospec_fixups_range
definition is still included, as well as a stub in asm/setup.h:

../arch/powerpc/lib/feature-fixups.c:502:6: error: redefinition of 
‘do_barrier_nospec_fixu>
  502 | void do_barrier_nospec_fixups_range(bool enable, void *fixup_start, 
void *fixup_en>
  |  ^~
In file included from ../arch/powerpc/lib/feature-fixups.c:23:
../arch/powerpc/include/asm/setup.h:70:20: note: previous definition of 
‘do_barrier_nospec>
   70 | static inline void do_barrier_nospec_fixups_range(bool enable, void 
*start, void *>
  |^~

I assume the intent was to put the just do_barrier_nospec_fixups
behind PPC_BARRIER_NOSPEC and let the compiler drop _range when there
are no users. (There is a caller in module.c, but this is behind
PPC_BARRIER_NOSPEC).

This makes PPC_BOOK3S_64 match how the PPC_FSL_BOOK3E build works.

Signed-off-by: Joel Stanley 
---
 arch/powerpc/include/asm/setup.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/arch/powerpc/include/asm/setup.h b/arch/powerpc/include/asm/setup.h
index 6c1a7d217d1a..71012284c044 100644
--- a/arch/powerpc/include/asm/setup.h
+++ b/arch/powerpc/include/asm/setup.h
@@ -66,8 +66,6 @@ extern bool barrier_nospec_enabled;
 
 #ifdef CONFIG_PPC_BARRIER_NOSPEC
 void do_barrier_nospec_fixups_range(bool enable, void *start, void *end);
-#else
-static inline void do_barrier_nospec_fixups_range(bool enable, void *start, 
void *end) { }
 #endif
 
 #ifdef CONFIG_PPC_FSL_BOOK3E
-- 
2.32.0

[PATCH 0/3] powerpc/64s: Fix PPC_BARRIER_NOSPEC=n

[Joel Stanley]

When disabling PPC_BARRIER_NOSPEC on Microwatt to see if it improved
boot time, I discovered the build was broken (first patch). This got
worse between when I first tried and now (second patch).

The third patch disables PPC_BARRIER_NOSPEC when building for Microwatt.
This one is optional, as it doesn't seem to change boot speed with the
current Microwatt design on an Arty.

Joel Stanley (3):
  powerpc/64s: Fix build when !PPC_BARRIER_NOSPEC
  powerpc: Fix undefined static key
  powerpc/microwatt: CPU doesn't (yet) have speculation bugs

 arch/powerpc/Kconfig | 1 +
 arch/powerpc/include/asm/security_features.h | 3 +++
 arch/powerpc/include/asm/setup.h | 2 --
 3 files changed, 4 insertions(+), 2 deletions(-)

-- 
2.32.0

Re: [PATCH] usb: gadget: fsl: properly remove remnant of MXC support

[Joel Stanley]

On Sat, 12 Jun 2021 at 00:31, Li Yang  wrote:
>
> Commit a390bef7db1f ("usb: gadget: fsl_mxc_udc: Remove the driver")
> didn't remove all the MXC related stuff which can cause build problem
> for LS1021 when enabled again in Kconfig.  This patch remove all the
> remnants.
>
> Signed-off-by: Li Yang 

Reviewed-by: Joel Stanley 

Will you re-submit the kconfig change once this is merged?

Cheers,

Joel

Re: [PATCH 4/4] powerpc/powernv: Remove POWER9 PVR version check for entry and uaccess flushes

[Joel Stanley]

On Tue, 4 May 2021 at 09:16, Nicholas Piggin  wrote:
>
> Excerpts from Joel Stanley's message of May 4, 2021 10:51 am:
> > On Mon, 3 May 2021 at 13:04, Nicholas Piggin  wrote:
> >>
> >> These aren't necessarily POWER9 only, and it's not to say some new
> >> vulnerability may not get discovered on other processors for which
> >> we would like the flexibility of having the workaround enabled by
> >> firmware.
> >>
> >> Remove the restriction that they only apply to POWER9.
> >
> > I was wondering how these worked which led me to reviewing your patch.
> > From what I could see, these are enabled by default (SEC_FTR_DEFAULT
> > in arch/powerpc/include/asm/security_features.h), so unless all
> > non-POWER9 machines have set the "please don't" bit in their firmware
> > this patch will enable the feature for those machines. Is that what
> > you wanted?
>
> Yes. POWER7/8 should be affected (it's similar mechanism that requires
> the meltdown RFI flush, which those processors need).
>
> POWER10 we haven't released a bare metal firmware with the right bits
> yet. Not urgent at the moment but wouldn't hurt to specify them and
> add the Linux code for them.

Thanks for the explanation. This could go in the commit message if you re-spin.

Reviewed-by: Joel Stanley 

Re: [PATCH 4/4] powerpc/powernv: Remove POWER9 PVR version check for entry and uaccess flushes

[Joel Stanley]

On Mon, 3 May 2021 at 13:04, Nicholas Piggin  wrote:
>
> These aren't necessarily POWER9 only, and it's not to say some new
> vulnerability may not get discovered on other processors for which
> we would like the flexibility of having the workaround enabled by
> firmware.
>
> Remove the restriction that they only apply to POWER9.

I was wondering how these worked which led me to reviewing your patch.
>From what I could see, these are enabled by default (SEC_FTR_DEFAULT
in arch/powerpc/include/asm/security_features.h), so unless all
non-POWER9 machines have set the "please don't" bit in their firmware
this patch will enable the feature for those machines. Is that what
you wanted?

>
> Signed-off-by: Nicholas Piggin 
> ---
>  arch/powerpc/platforms/powernv/setup.c | 9 -
>  1 file changed, 9 deletions(-)
>
> diff --git a/arch/powerpc/platforms/powernv/setup.c 
> b/arch/powerpc/platforms/powernv/setup.c
> index a8db3f153063..6ec67223f8c7 100644
> --- a/arch/powerpc/platforms/powernv/setup.c
> +++ b/arch/powerpc/platforms/powernv/setup.c
> @@ -122,15 +122,6 @@ static void pnv_setup_security_mitigations(void)
> type = L1D_FLUSH_ORI;
> }
>
> -   /*
> -* If we are non-Power9 bare metal, we don't need to flush on kernel
> -* entry or after user access: they fix a P9 specific vulnerability.
> -*/
> -   if (!pvr_version_is(PVR_POWER9)) {
> -   security_ftr_clear(SEC_FTR_L1D_FLUSH_ENTRY);
> -   security_ftr_clear(SEC_FTR_L1D_FLUSH_UACCESS);
> -   }
> -
> enable = security_ftr_enabled(SEC_FTR_FAVOUR_SECURITY) && \
>  (security_ftr_enabled(SEC_FTR_L1D_FLUSH_PR)   || \
>   security_ftr_enabled(SEC_FTR_L1D_FLUSH_HV));
> --
> 2.23.0
>

Re: [PATCH] asm-generic: Force inlining of get_order() to work around gcc10 poor decision

[Joel Stanley]

On Sat, 17 Oct 2020 at 15:55, Christophe Leroy
 wrote:
>
> When building mpc885_ads_defconfig with gcc 10.1,
> the function get_order() appears 50 times in vmlinux:
>
> [linux]# ppc-linux-objdump -x vmlinux | grep get_order | wc -l
> 50
>
> [linux]# size vmlinux
>textdata bss dec hex filename
> 3842620  675624  135160 4653404  47015c vmlinux
>
> In the old days, marking a function 'static inline' was forcing
> GCC to inline, but since commit ac7c3e4ff401 ("compiler: enable
> CONFIG_OPTIMIZE_INLINING forcibly") GCC may decide to not inline
> a function.
>
> It looks like GCC 10 is taking poor decisions on this.
>
> get_order() compiles into the following tiny function,
> occupying 20 bytes of text.
>
> 007c :
>   7c:   38 63 ff ff addir3,r3,-1
>   80:   54 63 a3 3e rlwinm  r3,r3,20,12,31
>   84:   7c 63 00 34 cntlzw  r3,r3
>   88:   20 63 00 20 subfic  r3,r3,32
>   8c:   4e 80 00 20 blr
>
> By forcing get_order() to be __always_inline, the size of text is
> reduced by 1940 bytes, that is almost twice the space occupied by
> 50 times get_order()
>
> [linux-powerpc]# size vmlinux
>textdata bss dec hex filename
> 3840680  675588  135176 4651444  46f9b4 vmlinux

I see similar results with GCC 10.2 building for arm32. There are 143
instances of get_order with aspeed_g5_defconfig.

Before:
 9071838 2630138  186468 11888444 b5673c vmlinux
After:
 9069886 2630126  186468 11886480 b55f90 vmlinux

1952 bytes smaller with your patch applied. Did you raise this with
anyone from GCC?

Reviewed-by: Joel Stanley 



> Signed-off-by: Christophe Leroy 
> ---
>  include/asm-generic/getorder.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/asm-generic/getorder.h b/include/asm-generic/getorder.h
> index e9f20b813a69..f2979e3a96b6 100644
> --- a/include/asm-generic/getorder.h
> +++ b/include/asm-generic/getorder.h
> @@ -26,7 +26,7 @@
>   *
>   * The result is undefined if the size is 0.
>   */
> -static inline __attribute_const__ int get_order(unsigned long size)
> +static __always_inline __attribute_const__ int get_order(unsigned long size)
>  {
> if (__builtin_constant_p(size)) {
> if (!size)
> --
> 2.25.0
>

Re: [RFC PATCH next-20200930] treewide: Convert macro and uses of __section(foo) to __section("foo")

[Joel Stanley]

On Thu, 1 Oct 2020 at 20:19, Joe Perches  wrote:
>
> On Thu, 2020-10-01 at 14:39 -0500, Segher Boessenkool wrch/ote:
> > Hi!
> >
> > On Thu, Oct 01, 2020 at 12:15:39PM +0200, Miguel Ojeda wrote:
> > > > So it looks like the best option is to exclude these
> > > > 2 files from conversion.
> > >
> > > Agreed. Nevertheless, is there any reason arch/powerpc/* should not be
> > > compiling cleanly with compiler.h? (CC'ing the rest of the PowerPC
> > > reviewers and ML).
> >
> > You need to #include compiler_types.h to get this #define?
>
> Actually no, you need to add
>
> #include 
>
> to both files and then it builds properly.
>
> Ideally though nothing should include this file directly.

arch/powerpc/boot is the powerpc wrapper, and it's not built with the
same includes or flags as the rest of the kernel. It doesn't include
any of the headers in the top level include/ directory for hysterical
raisins.

The straightforward fix would be to exclude this directory from your script.

Cheers,

Joel

Re: [PATCH] linux: configure CONFIG_I2C_OPAL as in-built.

[Joel Stanley]

On Fri, 25 Sep 2020 at 18:19, Mimi Zohar  wrote:
>
> Hi Nayna,
>
> On Wed, 2020-09-23 at 14:25 -0400, Nayna Jain wrote:
> > Currently, skiroot_defconfig CONFIG_I2C_OPAL is built as a loadable
> > module rather than builtin, even if CONFIG_I2C=y is defined. This
> > results in a delay in the TPM initialization, causing IMA to go into
> > TPM bypass mode. As a result, the IMA measurements are added to the
> > measurement list, but do not extend the TPM. Because of this, it is
> > impossible to verify or attest to the system's integrity, either from
> > skiroot or the target Host OS.
>
> The patch description is good, but perhaps we could provide a bit more
> context before.
>
> The concept of trusted boot requires the measurement to be added to the
> measurement list and extend the TPM, prior to allowing access to the
> file. By allowing access to a file before its measurement is included
> in the measurement list and extended into the TPM PCR, a malicious file
> could potentially prevent its own measurement from being added. As the
> PCRs are tamper proof, measuring and extending the TPM prior to giving
> access to the file, guarantees that all file measurements are included
> in the measurement list, including the malicious file.
>
> IMA needs to be enabled before any files are accessed in order to
> verify a file's integrity and extend the TPM with the file
> measurement.  Queueing file measurements breaks the measure and extend,
> before usage, trusted boot paradigm.
>
> The ima-evm-utils package includes a test for walking the IMA
> measurement list, calculating the expected TPM PCRs, and comparing the
> calculated PCR values with the physical TPM.  Testing is important to
> ensure the TPM is initialized prior to IMA.  Failure to validate the
> IMA measurement list may indicate IMA went into TPM bypass mode, like
> in this case.

Thanks for the explanation Mimi. It's lucky that the TPM drivers can
be loaded early enough!

Should we add something like this to security/integrity/ima/Kconfig?

select I2C_OPAL if PPC_POWERNV

It's generally frowned upon to select user visible symbols, but IMA
does this for the TCG options already.

Cheers,

Joel

>
> thanks,
>
> Mimi
>
> >
> > Reported-by: Mimi Zohar 
> > Signed-off-by: Nayna Jain 
> > ---
> >  openpower/configs/linux/skiroot_defconfig | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/openpower/configs/linux/skiroot_defconfig 
> > b/openpower/configs/linux/skiroot_defconfig
> > index 44309e12..a555adb2 100644
> > --- a/openpower/configs/linux/skiroot_defconfig
> > +++ b/openpower/configs/linux/skiroot_defconfig
> > @@ -216,7 +216,7 @@ CONFIG_I2C=y
> >  CONFIG_I2C_CHARDEV=y
> >  # CONFIG_I2C_HELPER_AUTO is not set
> >  CONFIG_I2C_ALGOBIT=y
> > -CONFIG_I2C_OPAL=m
> > +CONFIG_I2C_OPAL=y
> >  CONFIG_PPS=y
> >  CONFIG_SENSORS_IBMPOWERNV=m
> >  CONFIG_DRM=m
>
>

Re: [PATCH] cpuidle-pseries: Fix CEDE latency conversion from tb to us

[Joel Stanley]

On Tue, 1 Sep 2020 at 14:09, Gautham R. Shenoy  wrote:
>
> From: "Gautham R. Shenoy" 
>
> commit d947fb4c965c ("cpuidle: pseries: Fixup exit latency for
> CEDE(0)") sets the exit latency of CEDE(0) based on the latency values
> of the Extended CEDE states advertised by the platform. The values
> advertised by the platform are in timebase ticks. However the cpuidle
> framework requires the latency values in microseconds.
>
> If the tb-ticks value advertised by the platform correspond to a value
> smaller than 1us, during the conversion from tb-ticks to microseconds,
> in the current code, the result becomes zero. This is incorrect as it
> puts a CEDE state on par with the snooze state.
>
> This patch fixes this by rounding up the result obtained while
> converting the latency value from tb-ticks to microseconds.
>
> Fixes: commit d947fb4c965c ("cpuidle: pseries: Fixup exit latency for
> CEDE(0)")
>
> Signed-off-by: Gautham R. Shenoy 

Reviewed-by: Joel Stanley 

Should you check for the zero case and print a warning?

> ---
>  drivers/cpuidle/cpuidle-pseries.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/cpuidle/cpuidle-pseries.c 
> b/drivers/cpuidle/cpuidle-pseries.c
> index ff6d99e..9043358 100644
> --- a/drivers/cpuidle/cpuidle-pseries.c
> +++ b/drivers/cpuidle/cpuidle-pseries.c
> @@ -361,7 +361,7 @@ static void __init fixup_cede0_latency(void)
> for (i = 0; i < nr_xcede_records; i++) {
> struct xcede_latency_record *record = >records[i];
> u64 latency_tb = be64_to_cpu(record->latency_ticks);
> -   u64 latency_us = tb_to_ns(latency_tb) / NSEC_PER_USEC;
> +   u64 latency_us = DIV_ROUND_UP_ULL(tb_to_ns(latency_tb), 
> NSEC_PER_USEC);
>
> if (latency_us < min_latency_us)
> min_latency_us = latency_us;
> --
> 1.9.4
>

[PATCH v3] powerpc: Warn about use of smt_snooze_delay

[Joel Stanley]

It's not done anything for a long time. Save the percpu variable, and
emit a warning to remind users to not expect it to do anything.

This uses pr_warn_once instead of pr_warn_ratelimit as testing
'ppc64_cpu --smt=off' on a 24 core / 4 SMT system showed the warning to
be noisy, as the online/offline loop is slow.

Fixes: 3fa8cad82b94 ("powerpc/pseries/cpuidle: smt-snooze-delay cleanup.")
Cc: sta...@vger.kernel.org # v3.14
Acked-by: Gautham R. Shenoy 
Signed-off-by: Joel Stanley 
--
v3:
 pr_warn_once instead of pr_warn_ratelimited
 Update meessages with mpe's suggestions
v2:
 Use pr_warn instead of WARN
 Reword and print proccess name with pid in message
 Leave CPU_FTR_SMT test in
---
 arch/powerpc/kernel/sysfs.c | 42 +++--
 1 file changed, 17 insertions(+), 25 deletions(-)

diff --git a/arch/powerpc/kernel/sysfs.c b/arch/powerpc/kernel/sysfs.c
index 46b4ebc33db7..5dea98fa2f93 100644
--- a/arch/powerpc/kernel/sysfs.c
+++ b/arch/powerpc/kernel/sysfs.c
@@ -32,29 +32,27 @@
 
 static DEFINE_PER_CPU(struct cpu, cpu_devices);
 
-/*
- * SMT snooze delay stuff, 64-bit only for now
- */
-
 #ifdef CONFIG_PPC64
 
-/* Time in microseconds we delay before sleeping in the idle loop */
-static DEFINE_PER_CPU(long, smt_snooze_delay) = { 100 };
+/*
+ * Snooze delay has not been hooked up since 3fa8cad82b94 
("powerpc/pseries/cpuidle:
+ * smt-snooze-delay cleanup.") and has been broken even longer. As was 
foretold in
+ * 2014:
+ *
+ *  "ppc64_util currently utilises it. Once we fix ppc64_util, propose to clean
+ *  up the kernel code."
+ *
+ * powerpc-utils stopped using it as of 1.3.8. At some point in the future this
+ * code should be removed.
+ */
 
 static ssize_t store_smt_snooze_delay(struct device *dev,
  struct device_attribute *attr,
  const char *buf,
  size_t count)
 {
-   struct cpu *cpu = container_of(dev, struct cpu, dev);
-   ssize_t ret;
-   long snooze;
-
-   ret = sscanf(buf, "%ld", );
-   if (ret != 1)
-   return -EINVAL;
-
-   per_cpu(smt_snooze_delay, cpu->dev.id) = snooze;
+   pr_warn_once("%s (%d) stored to unsupported smt_snooze_delay, which has 
no effect.\n",
+current->comm, current->pid);
return count;
 }
 
@@ -62,9 +60,9 @@ static ssize_t show_smt_snooze_delay(struct device *dev,
 struct device_attribute *attr,
 char *buf)
 {
-   struct cpu *cpu = container_of(dev, struct cpu, dev);
-
-   return sprintf(buf, "%ld\n", per_cpu(smt_snooze_delay, cpu->dev.id));
+   pr_warn_once("%s (%d) read from unsupported smt_snooze_delay\n",
+current->comm, current->pid);
+   return sprintf(buf, "100\n");
 }
 
 static DEVICE_ATTR(smt_snooze_delay, 0644, show_smt_snooze_delay,
@@ -72,16 +70,10 @@ static DEVICE_ATTR(smt_snooze_delay, 0644, 
show_smt_snooze_delay,
 
 static int __init setup_smt_snooze_delay(char *str)
 {
-   unsigned int cpu;
-   long snooze;
-
if (!cpu_has_feature(CPU_FTR_SMT))
return 1;
 
-   snooze = simple_strtol(str, NULL, 10);
-   for_each_possible_cpu(cpu)
-   per_cpu(smt_snooze_delay, cpu) = snooze;
-
+   pr_warn("smt-snooze-delay command line option has no effect\n");
return 1;
 }
 __setup("smt-snooze-delay=", setup_smt_snooze_delay);
-- 
2.28.0

Re: [PATCH v2] powerpc: Warn about use of smt_snooze_delay

[Joel Stanley]

On Tue, 4 Aug 2020 at 11:59, Michael Ellerman  wrote:
>
> Joel Stanley  writes:
> > It's not done anything for a long time. Save the percpu variable, and
> > emit a warning to remind users to not expect it to do anything.
> >
> > Fixes: 3fa8cad82b94 ("powerpc/pseries/cpuidle: smt-snooze-delay cleanup.")
> > Cc: sta...@vger.kernel.org # v3.14
> > Signed-off-by: Joel Stanley 
> > --
> > v2:
> >  Use pr_warn instead of WARN
> >  Reword and print proccess name with pid in message
> >  Leave CPU_FTR_SMT test in
> >  Add Fixes line
> >
> > mpe, if you don't agree then feel free to drop the cc stable.
> >
> > Testing 'ppc64_cpu --smt=off' on a 24 core / 4 SMT system it's quite noisy
> > as the online/offline loop that ppc64_cpu runs is slow.
>
> Hmm, that is pretty spammy.
>
> I know I suggested the ratelimit, but I was thinking it would print once
> for each ppc64_cpu invocation, not ~30 times.
>
> How about pr_warn_once(), that should still be sufficient for people to
> notice if they're looking for it.

I think that's a reasonable suggestion.

>
> ...
> > diff --git a/arch/powerpc/kernel/sysfs.c b/arch/powerpc/kernel/sysfs.c
> > index 571b3259697e..ba6d4cee19ef 100644
> > --- a/arch/powerpc/kernel/sysfs.c
> > +++ b/arch/powerpc/kernel/sysfs.c
> > @@ -32,29 +32,26 @@
> >
> >  static DEFINE_PER_CPU(struct cpu, cpu_devices);
> >
> > -/*
> > - * SMT snooze delay stuff, 64-bit only for now
> > - */
> > -
> >  #ifdef CONFIG_PPC64
> >
> > -/* Time in microseconds we delay before sleeping in the idle loop */
> > -static DEFINE_PER_CPU(long, smt_snooze_delay) = { 100 };
> > +/*
> > + * Snooze delay has not been hooked up since 3fa8cad82b94 
> > ("powerpc/pseries/cpuidle:
> > + * smt-snooze-delay cleanup.") and has been broken even longer. As was 
> > foretold in
> > + * 2014:
> > + *
> > + *  "ppc64_util currently utilises it. Once we fix ppc64_util, propose to 
> > clean
> > + *  up the kernel code."
> > + *
> > + * At some point in the future this code should be removed.
> > + */
> >
> >  static ssize_t store_smt_snooze_delay(struct device *dev,
> > struct device_attribute *attr,
> > const char *buf,
> > size_t count)
> >  {
> > - struct cpu *cpu = container_of(dev, struct cpu, dev);
> > - ssize_t ret;
> > - long snooze;
> > -
> > - ret = sscanf(buf, "%ld", );
> > - if (ret != 1)
> > - return -EINVAL;
> > -
> > - per_cpu(smt_snooze_delay, cpu->dev.id) = snooze;
> > + pr_warn_ratelimited("%s (%d) used unsupported smt_snooze_delay, this 
> > has no effect\n",
> > + current->comm, current->pid);
>
> Can we make this:
>
> "%s (%d) stored to unsupported smt_snooze_delay, which has no 
> effect.\n",

ack

>
>
> >   return count;
> >  }
> >
> > @@ -62,9 +59,9 @@ static ssize_t show_smt_snooze_delay(struct device *dev,
> >struct device_attribute *attr,
> >char *buf)
> >  {
> > - struct cpu *cpu = container_of(dev, struct cpu, dev);
> > -
> > - return sprintf(buf, "%ld\n", per_cpu(smt_snooze_delay, cpu->dev.id));
> > + pr_warn_ratelimited("%s (%d) used unsupported smt_snooze_delay, this 
> > has no effect\n",
> > + current->comm, current->pid);
>
> It has as much effect as it ever did :)
>
> So maybe:
>
> "%s (%d) read from unsupported smt_snooze_delay.\n",
>
>
> I can do those changes when applying if you like rather than making you
> do a v3.

Yes please! Your suggested changes lgtm.

Cheers,

Joel

Re: [PATCH 5/6] powerpc/powernv/pci: Drop unused parent variable

[Joel Stanley]

On Tue, 4 Aug 2020 at 01:06, Oliver O'Halloran  wrote:
>
> The "parent" variable in pnv_pci_ioda_configure_pe() isn't used for
> anything anymore and can be dropped.
>
> Signed-off-by: Oliver O'Halloran 

Reviewed-by: Joel Stanley 

Re: [PATCH 4/6] powerpc/powernv: Fix spurious kerneldoc warnings in opal-prd.c

[Joel Stanley]

On Tue, 4 Aug 2020 at 01:03, Oliver O'Halloran  wrote:
>
> Comments opening with /** are parsed by kerneldoc and this causes the
> following warning to be printed:
>
> arch/powerpc/platforms/powernv/opal-prd.c:31: warning: cannot 
> understand
> function prototype: 'struct opal_prd_msg_queue_item '
>
> opal_prd_mesg_queue_item is an internal data structure so there's no real
> need for it to be documented at all. Fix up the comment to squash the
> warning.
>
> Signed-off-by: Oliver O'Halloran 

Reviewed-by: Joel Stanley 

Re: [PATCH 3/6] powerpc/powernv: Staticify functions without prototypes

[Joel Stanley]

On Tue, 4 Aug 2020 at 01:01, Oliver O'Halloran  wrote:
>
> There's a few scattered in the powernv platform.
>
> Signed-off-by: Oliver O'Halloran 

> +++ b/arch/powerpc/platforms/powernv/eeh-powernv.c
> @@ -38,7 +38,7 @@
>
>  static int eeh_event_irq = -EINVAL;
>
> -void pnv_pcibios_bus_add_device(struct pci_dev *pdev)
> +static void pnv_pcibios_bus_add_device(struct pci_dev *pdev)
>  {
> dev_dbg(>dev, "EEH: Setting up device\n");
> eeh_probe_device(pdev);

This one could even be deleted as eeh_probe_device has it's own dev_dbg.

Reviewed-by: Joel Stanley 

Re: [PATCH 1/6] powerpc/powernv/smp: Fix spurious DBG() warning

[Joel Stanley]

On Tue, 4 Aug 2020 at 00:57, Oliver O'Halloran  wrote:
>
> When building with W=1 we get the following warning:
>
>  arch/powerpc/platforms/powernv/smp.c: In function ‘pnv_smp_cpu_kill_self’:
>  arch/powerpc/platforms/powernv/smp.c:276:16: error: suggest braces around
> empty body in an ‘if’ statement [-Werror=empty-body]
>276 |  cpu, srr1);
>|^
>  cc1: all warnings being treated as errors
>
> The full context is this block:
>
>  if (srr1 && !generic_check_cpu_restart(cpu))
> DBG("CPU%d Unexpected exit while offline srr1=%lx!\n",
> cpu, srr1);
>
> When building with DEBUG undefined DBG() expands to nothing and GCC emits
> the warning due to the lack of braces around an empty statement.
>
> Signed-off-by: Oliver O'Halloran 
> ---
> We could add the braces too. That might even be better since it's a multi-line
> if block even though it's only a single statement.

Or you could put it all on one line, now that our 120 line overlords
have taken over.

Reviewed-by: Joel Stanley 

Messy:

$ git grep "define DBG(" arch/powerpc/ |grep -v print
arch/powerpc/kernel/crash_dump.c:#define DBG(fmt...)
arch/powerpc/kernel/iommu.c:#define DBG(...)
arch/powerpc/kernel/legacy_serial.c:#define DBG(fmt...) do { } while(0)
arch/powerpc/kernel/prom.c:#define DBG(fmt...)
arch/powerpc/kernel/setup-common.c:#define DBG(fmt...)
arch/powerpc/kernel/setup_32.c:#define DBG(fmt...)
arch/powerpc/kernel/smp.c:#define DBG(fmt...)
arch/powerpc/kernel/vdso.c:#define DBG(fmt...)
arch/powerpc/kvm/book3s_hv_rm_xive.c:#define DBG(fmt...) do { } while(0)
arch/powerpc/mm/book3s64/hash_utils.c:#define DBG(fmt...)
arch/powerpc/platforms/83xx/mpc832x_mds.c:#define DBG(fmt...)
arch/powerpc/platforms/83xx/mpc832x_rdb.c:#define DBG(fmt...)
arch/powerpc/platforms/83xx/mpc836x_mds.c:#define DBG(fmt...)
arch/powerpc/platforms/85xx/mpc85xx_ds.c:#define DBG(fmt, args...)
arch/powerpc/platforms/85xx/mpc85xx_mds.c:#define DBG(fmt...)
arch/powerpc/platforms/85xx/mpc85xx_rdb.c:#define DBG(fmt, args...)
arch/powerpc/platforms/86xx/mpc86xx_hpcn.c:#define DBG(fmt...) do { } while(0)
arch/powerpc/platforms/cell/setup.c:#define DBG(fmt...)
arch/powerpc/platforms/cell/smp.c:#define DBG(fmt...)
arch/powerpc/platforms/embedded6xx/mpc7448_hpc2.c:#define DBG(fmt...)
do { } while(0)
arch/powerpc/platforms/maple/pci.c:#define DBG(x...)
arch/powerpc/platforms/maple/setup.c:#define DBG(fmt...)
arch/powerpc/platforms/maple/time.c:#define DBG(x...)
arch/powerpc/platforms/powermac/bootx_init.c:#define DBG(fmt...) do { } while(0)
arch/powerpc/platforms/powermac/feature.c:#define DBG(fmt...)
arch/powerpc/platforms/powermac/low_i2c.c:#define DBG(x...) do {\
arch/powerpc/platforms/powermac/low_i2c.c:#define DBG(x...)
arch/powerpc/platforms/powermac/nvram.c:#define DBG(x...)
arch/powerpc/platforms/powermac/pci.c:#define DBG(x...)
arch/powerpc/platforms/powermac/pfunc_base.c:#define DBG(fmt...)
arch/powerpc/platforms/powermac/pfunc_core.c:#define DBG(fmt...)
arch/powerpc/platforms/powermac/smp.c:#define DBG(fmt...)
arch/powerpc/platforms/powermac/time.c:#define DBG(x...)
arch/powerpc/platforms/powernv/smp.c:#define DBG(fmt...)
arch/powerpc/sysdev/dart_iommu.c:#define DBG(...)
arch/powerpc/sysdev/ge/ge_pic.c:#define DBG(fmt...) do { } while (0)
arch/powerpc/sysdev/mpic.c:#define DBG(fmt...)
arch/powerpc/sysdev/tsi108_dev.c:#define DBG(fmt...) do { } while(0)
arch/powerpc/sysdev/tsi108_pci.c:#define DBG(x...)


> ---
>  arch/powerpc/platforms/powernv/smp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/platforms/powernv/smp.c 
> b/arch/powerpc/platforms/powernv/smp.c
> index b2ba3e95bda7..bbf361f23ae8 100644
> --- a/arch/powerpc/platforms/powernv/smp.c
> +++ b/arch/powerpc/platforms/powernv/smp.c
> @@ -43,7 +43,7 @@
>  #include 
>  #define DBG(fmt...) udbg_printf(fmt)
>  #else
> -#define DBG(fmt...)
> +#define DBG(fmt...) do { } while (0)
>  #endif
>
>  static void pnv_smp_setup_cpu(int cpu)
> --
> 2.26.2
>

[PATCH v2] powerpc: Warn about use of smt_snooze_delay

[Joel Stanley]

It's not done anything for a long time. Save the percpu variable, and
emit a warning to remind users to not expect it to do anything.

Fixes: 3fa8cad82b94 ("powerpc/pseries/cpuidle: smt-snooze-delay cleanup.")
Cc: sta...@vger.kernel.org # v3.14
Signed-off-by: Joel Stanley 
--
v2:
 Use pr_warn instead of WARN
 Reword and print proccess name with pid in message
 Leave CPU_FTR_SMT test in
 Add Fixes line

mpe, if you don't agree then feel free to drop the cc stable.

Testing 'ppc64_cpu --smt=off' on a 24 core / 4 SMT system it's quite noisy
as the online/offline loop that ppc64_cpu runs is slow.

This could be fixed by open coding pr_warn_ratelimit with the ratelimit
parameters tweaked if someone was concerned. I'll leave that to someone
else as a future enhancement.

[  237.642088][ T1197] ppc64_cpu (1197) used unsupported smt_snooze_delay, this 
has no effect
[  237.642175][ T1197] ppc64_cpu (1197) used unsupported smt_snooze_delay, this 
has no effect
[  237.642261][ T1197] ppc64_cpu (1197) used unsupported smt_snooze_delay, this 
has no effect
[  237.642345][ T1197] ppc64_cpu (1197) used unsupported smt_snooze_delay, this 
has no effect
[  237.642430][ T1197] ppc64_cpu (1197) used unsupported smt_snooze_delay, this 
has no effect
[  237.642516][ T1197] ppc64_cpu (1197) used unsupported smt_snooze_delay, this 
has no effect
[  237.642625][ T1197] ppc64_cpu (1197) used unsupported smt_snooze_delay, this 
has no effect
[  237.642709][ T1197] ppc64_cpu (1197) used unsupported smt_snooze_delay, this 
has no effect
[  237.642793][ T1197] ppc64_cpu (1197) used unsupported smt_snooze_delay, this 
has no effect
[  237.642878][ T1197] ppc64_cpu (1197) used unsupported smt_snooze_delay, this 
has no effect
[  254.264030][ T1197] store_smt_snooze_delay: 14 callbacks suppressed
[  254.264033][ T1197] ppc64_cpu (1197) used unsupported smt_snooze_delay, this 
has no effect
[  254.264048][ T1197] ppc64_cpu (1197) used unsupported smt_snooze_delay, this 
has no effect
[  254.264062][ T1197] ppc64_cpu (1197) used unsupported smt_snooze_delay, this 
has no effect
[  254.264075][ T1197] ppc64_cpu (1197) used unsupported smt_snooze_delay, this 
has no effect
[  254.264089][ T1197] ppc64_cpu (1197) used unsupported smt_snooze_delay, this 
has no effect
[  254.264103][ T1197] ppc64_cpu (1197) used unsupported smt_snooze_delay, this 
has no effect
[  254.264116][ T1197] ppc64_cpu (1197) used unsupported smt_snooze_delay, this 
has no effect
[  254.264130][ T1197] ppc64_cpu (1197) used unsupported smt_snooze_delay, this 
has no effect
[  254.264143][ T1197] ppc64_cpu (1197) used unsupported smt_snooze_delay, this 
has no effect
[  254.264157][ T1197] ppc64_cpu (1197) used unsupported smt_snooze_delay, this 
has no effect

Signed-off-by: Joel Stanley 
---
 arch/powerpc/kernel/sysfs.c | 41 +++--
 1 file changed, 16 insertions(+), 25 deletions(-)

diff --git a/arch/powerpc/kernel/sysfs.c b/arch/powerpc/kernel/sysfs.c
index 571b3259697e..ba6d4cee19ef 100644
--- a/arch/powerpc/kernel/sysfs.c
+++ b/arch/powerpc/kernel/sysfs.c
@@ -32,29 +32,26 @@
 
 static DEFINE_PER_CPU(struct cpu, cpu_devices);
 
-/*
- * SMT snooze delay stuff, 64-bit only for now
- */
-
 #ifdef CONFIG_PPC64
 
-/* Time in microseconds we delay before sleeping in the idle loop */
-static DEFINE_PER_CPU(long, smt_snooze_delay) = { 100 };
+/*
+ * Snooze delay has not been hooked up since 3fa8cad82b94 
("powerpc/pseries/cpuidle:
+ * smt-snooze-delay cleanup.") and has been broken even longer. As was 
foretold in
+ * 2014:
+ *
+ *  "ppc64_util currently utilises it. Once we fix ppc64_util, propose to clean
+ *  up the kernel code."
+ *
+ * At some point in the future this code should be removed.
+ */
 
 static ssize_t store_smt_snooze_delay(struct device *dev,
  struct device_attribute *attr,
  const char *buf,
  size_t count)
 {
-   struct cpu *cpu = container_of(dev, struct cpu, dev);
-   ssize_t ret;
-   long snooze;
-
-   ret = sscanf(buf, "%ld", );
-   if (ret != 1)
-   return -EINVAL;
-
-   per_cpu(smt_snooze_delay, cpu->dev.id) = snooze;
+   pr_warn_ratelimited("%s (%d) used unsupported smt_snooze_delay, this 
has no effect\n",
+   current->comm, current->pid);
return count;
 }
 
@@ -62,9 +59,9 @@ static ssize_t show_smt_snooze_delay(struct device *dev,
 struct device_attribute *attr,
 char *buf)
 {
-   struct cpu *cpu = container_of(dev, struct cpu, dev);
-
-   return sprintf(buf, "%ld\n", per_cpu(smt_snooze_delay, cpu->dev.id));
+   pr_warn_ratelimited("%s (%d) used unsupported smt_snooze_delay, this 
has no effect\n",
+   current->comm, current->pid

Re: [PATCH] powerpc: Warn about use of smt_snooze_delay

[Joel Stanley]

On Mon, 29 Jun 2020 at 10:42, Gautham R Shenoy  wrote:
>
> On Thu, Jun 25, 2020 at 07:33:49PM +0930, Joel Stanley wrote:
> > It's not done anything for a long time. Save the percpu variable, and
> > emit a warning to remind users to not expect it to do anything.
> >
> > Signed-off-by: Joel Stanley 
>
> The only known user of "smt_snooze_delay" is the "ppc64_cpu" which
> uses the presence of this file to assume that the system is SMT
> capable.
>
> Since we have "/sys/devices/system/cpu/smt/" these days, perhaps the
> userspace utility can use that and we can get rid of the file
> altogether ?

I've sent a change to the userspace tool to stop using the file. It
now uses the device tree parsing that was already present to determine
the smt state.

 https://github.com/ibm-power-utilities/powerpc-utils/pull/43

We will want to wait for the userspace tool to propagate through a
release and to distros before we remove the file all together. I agree
it should be removed in the future.

I've got of this patch v2 that changes the message to be:

 pr_warn_ratelimited("%s (%d) used unsupported
smt_snooze_delay, this has no effect\n",
current->comm, current->pid);

I'll send that out today.

Cheers,

Joel

>
> FWIW,
> Acked-by: Gautham R. Shenoy 
> > ---
> >  arch/powerpc/kernel/sysfs.c | 41 +
> >  1 file changed, 14 insertions(+), 27 deletions(-)
> >
> > diff --git a/arch/powerpc/kernel/sysfs.c b/arch/powerpc/kernel/sysfs.c
> > index 571b3259697e..530ae92bc46d 100644
> > --- a/arch/powerpc/kernel/sysfs.c
> > +++ b/arch/powerpc/kernel/sysfs.c
> > @@ -32,29 +32,25 @@
> >
> >  static DEFINE_PER_CPU(struct cpu, cpu_devices);
> >
> > -/*
> > - * SMT snooze delay stuff, 64-bit only for now
> > - */
> > -
> >  #ifdef CONFIG_PPC64
> >
> > -/* Time in microseconds we delay before sleeping in the idle loop */
> > -static DEFINE_PER_CPU(long, smt_snooze_delay) = { 100 };
> > +/*
> > + * Snooze delay has not been hooked up since 3fa8cad82b94 
> > ("powerpc/pseries/cpuidle:
> > + * smt-snooze-delay cleanup.") and has been broken even longer. As was 
> > foretold in
> > + * 2014:
> > + *
> > + *  "ppc64_util currently utilises it. Once we fix ppc64_util, propose to 
> > clean
> > + *  up the kernel code."
> > + *
> > + * At some point in the future this code should be removed.
> > + */
> >
> >  static ssize_t store_smt_snooze_delay(struct device *dev,
> > struct device_attribute *attr,
> > const char *buf,
> > size_t count)
> >  {
> > - struct cpu *cpu = container_of(dev, struct cpu, dev);
> > - ssize_t ret;
> > - long snooze;
> > -
> > - ret = sscanf(buf, "%ld", );
> > - if (ret != 1)
> > - return -EINVAL;
> > -
> > - per_cpu(smt_snooze_delay, cpu->dev.id) = snooze;
> > + WARN_ON_ONCE("smt_snooze_delay sysfs file has no effect\n");
> >   return count;
> >  }
> >
> > @@ -62,9 +58,9 @@ static ssize_t show_smt_snooze_delay(struct device *dev,
> >struct device_attribute *attr,
> >char *buf)
> >  {
> > - struct cpu *cpu = container_of(dev, struct cpu, dev);
> > + WARN_ON_ONCE("smt_snooze_delay sysfs file has no effect\n");
> >
> > - return sprintf(buf, "%ld\n", per_cpu(smt_snooze_delay, cpu->dev.id));
> > + return sprintf(buf, "100\n");
> >  }
> >
> >  static DEVICE_ATTR(smt_snooze_delay, 0644, show_smt_snooze_delay,
> > @@ -72,16 +68,7 @@ static DEVICE_ATTR(smt_snooze_delay, 0644, 
> > show_smt_snooze_delay,
> >
> >  static int __init setup_smt_snooze_delay(char *str)
> >  {
> > - unsigned int cpu;
> > - long snooze;
> > -
> > - if (!cpu_has_feature(CPU_FTR_SMT))
> > - return 1;
> > -
> > - snooze = simple_strtol(str, NULL, 10);
> > - for_each_possible_cpu(cpu)
> > - per_cpu(smt_snooze_delay, cpu) = snooze;
> > -
> > + WARN_ON_ONCE("smt-snooze-delay command line option has no effect\n");
> >   return 1;
> >  }
> >  __setup("smt-snooze-delay=", setup_smt_snooze_delay);
> > --
> > 2.27.0
> >

[PATCH] powerpc: Warn about use of smt_snooze_delay

[Joel Stanley]

It's not done anything for a long time. Save the percpu variable, and
emit a warning to remind users to not expect it to do anything.

Signed-off-by: Joel Stanley 
---
 arch/powerpc/kernel/sysfs.c | 41 +
 1 file changed, 14 insertions(+), 27 deletions(-)

diff --git a/arch/powerpc/kernel/sysfs.c b/arch/powerpc/kernel/sysfs.c
index 571b3259697e..530ae92bc46d 100644
--- a/arch/powerpc/kernel/sysfs.c
+++ b/arch/powerpc/kernel/sysfs.c
@@ -32,29 +32,25 @@
 
 static DEFINE_PER_CPU(struct cpu, cpu_devices);
 
-/*
- * SMT snooze delay stuff, 64-bit only for now
- */
-
 #ifdef CONFIG_PPC64
 
-/* Time in microseconds we delay before sleeping in the idle loop */
-static DEFINE_PER_CPU(long, smt_snooze_delay) = { 100 };
+/*
+ * Snooze delay has not been hooked up since 3fa8cad82b94 
("powerpc/pseries/cpuidle:
+ * smt-snooze-delay cleanup.") and has been broken even longer. As was 
foretold in
+ * 2014:
+ *
+ *  "ppc64_util currently utilises it. Once we fix ppc64_util, propose to clean
+ *  up the kernel code."
+ *
+ * At some point in the future this code should be removed.
+ */
 
 static ssize_t store_smt_snooze_delay(struct device *dev,
  struct device_attribute *attr,
  const char *buf,
  size_t count)
 {
-   struct cpu *cpu = container_of(dev, struct cpu, dev);
-   ssize_t ret;
-   long snooze;
-
-   ret = sscanf(buf, "%ld", );
-   if (ret != 1)
-   return -EINVAL;
-
-   per_cpu(smt_snooze_delay, cpu->dev.id) = snooze;
+   WARN_ON_ONCE("smt_snooze_delay sysfs file has no effect\n");
return count;
 }
 
@@ -62,9 +58,9 @@ static ssize_t show_smt_snooze_delay(struct device *dev,
 struct device_attribute *attr,
 char *buf)
 {
-   struct cpu *cpu = container_of(dev, struct cpu, dev);
+   WARN_ON_ONCE("smt_snooze_delay sysfs file has no effect\n");
 
-   return sprintf(buf, "%ld\n", per_cpu(smt_snooze_delay, cpu->dev.id));
+   return sprintf(buf, "100\n");
 }
 
 static DEVICE_ATTR(smt_snooze_delay, 0644, show_smt_snooze_delay,
@@ -72,16 +68,7 @@ static DEVICE_ATTR(smt_snooze_delay, 0644, 
show_smt_snooze_delay,
 
 static int __init setup_smt_snooze_delay(char *str)
 {
-   unsigned int cpu;
-   long snooze;
-
-   if (!cpu_has_feature(CPU_FTR_SMT))
-   return 1;
-
-   snooze = simple_strtol(str, NULL, 10);
-   for_each_possible_cpu(cpu)
-   per_cpu(smt_snooze_delay, cpu) = snooze;
-
+   WARN_ON_ONCE("smt-snooze-delay command line option has no effect\n");
return 1;
 }
 __setup("smt-snooze-delay=", setup_smt_snooze_delay);
-- 
2.27.0

Re: [PATCH v5 01/13] powerpc: Remove Xilinx PPC405/PPC440 support

[Joel Stanley]

On Fri, 19 Jun 2020 at 11:02, Michael Ellerman  wrote:
>
> Nathan Chancellor  writes:
> >> It's kind of nuts that the zImage points to some arbitrary image
> >> depending on what's configured and the order of things in the Makefile.
> >> But I'm not sure how we make it less nuts without risking breaking
> >> people's existing setups.
> >
> > Hi Michael,
> >
> > For what it's worth, this is squared this away in terms of our CI by
> > just building and booting the uImage directly, rather than implicitly
> > using the zImage:
> >
> > https://github.com/ClangBuiltLinux/continuous-integration/pull/282
> > https://github.com/ClangBuiltLinux/boot-utils/pull/22
>
> Great.
>
> > We were only using the zImage because that is what Joel Stanley intially
> > set us up with when PowerPC 32-bit was added to our CI:
> >
> > https://github.com/ClangBuiltLinux/continuous-integration/pull/100
>
> Ah, so Joel owes us all beers then ;)

Hey, you owe me beers for finding broken machines!

This machine was picked from a vague discussion on an internal chat.
The two requirements were that it would build, and boot in qemu.

If there's a better supported 32 bit machine then we should switch the
CI over. We don't want the Clang CI to be the only user and give the
false impression that someone out there is still booting upstream
kernels on it.

> > Admittedly, we really do not have many PowerPC experts in our
> > organization so we are supporting it on a "best effort" basis, which
> > often involves using whatever knowledge is floating around or can be
> > gained from interactions such as this :) so thank you for that!
>
> No worries. I definitely don't expect you folks to invest much effort in
> powerpc, especially the old 32-bit stuff, so always happy to help debug
> things, and really appreciate the testing you do.

+1

Cheers,

Joel

Re: [PATCH 1/4] powerpc: Add a ppc_inst_as_str() helper

[Joel Stanley]

On Tue, 2 Jun 2020 at 05:31, Jordan Niethe  wrote:
>
> There are quite a few places where instructions are printed, this is
> done using a '%x' format specifier. With the introduction of prefixed
> instructions, this does not work well. Currently in these places,
> ppc_inst_val() is used for the value for %x so only the first word of
> prefixed instructions are printed.
>
> When the instructions are word instructions, only a single word should
> be printed. For prefixed instructions both the prefix and suffix should
> be printed. To accommodate both of these situations, instead of a '%x'
> specifier use '%s' and introduce a helper, __ppc_inst_as_str() which
> returns a char *. The char * __ppc_inst_as_str() returns is buffer that
> is passed to it by the caller.
>
> It is cumbersome to require every caller of __ppc_inst_as_str() to now
> declare a buffer. To make it more convenient to use __ppc_inst_as_str(),
> wrap it in a macro that uses a compound statement to allocate a buffer
> on the caller's stack before calling it.
>
> Signed-off-by: Jordan Niethe 

Reviewed-by: Joel Stanley 

> ---
>  arch/powerpc/include/asm/inst.h  | 19 +++
>  arch/powerpc/kernel/kprobes.c|  2 +-
>  arch/powerpc/kernel/trace/ftrace.c   | 26 +-
>  arch/powerpc/lib/test_emulate_step.c |  4 ++--
>  arch/powerpc/xmon/xmon.c |  2 +-
>  5 files changed, 36 insertions(+), 17 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/inst.h b/arch/powerpc/include/asm/inst.h
> index 45f3ec868258..3df7806e6dc3 100644
> --- a/arch/powerpc/include/asm/inst.h
> +++ b/arch/powerpc/include/asm/inst.h
> @@ -122,6 +122,25 @@ static inline u64 ppc_inst_as_u64(struct ppc_inst x)
>  #endif
>  }
>
> +#define PPC_INST_STR_LEN sizeof("0x 0x")
> +
> +static inline char *__ppc_inst_as_str(char str[PPC_INST_STR_LEN], struct 
> ppc_inst x)
> +{
> +   if (ppc_inst_prefixed(x))
> +   sprintf(str, "0x%08x 0x%08x", ppc_inst_val(x), 
> ppc_inst_suffix(x));
> +   else
> +   sprintf(str, "0x%08x", ppc_inst_val(x));
> +
> +   return str;
> +}
> +
> +#define ppc_inst_as_str(x) \
> +({ \
> +   char __str[PPC_INST_STR_LEN];   \
> +   __ppc_inst_as_str(__str, x);\
> +   __str;  \
> +})
> +
>  int probe_user_read_inst(struct ppc_inst *inst,
>  struct ppc_inst __user *nip);
>
> diff --git a/arch/powerpc/kernel/kprobes.c b/arch/powerpc/kernel/kprobes.c
> index 227510df8c55..d0797171dba3 100644
> --- a/arch/powerpc/kernel/kprobes.c
> +++ b/arch/powerpc/kernel/kprobes.c
> @@ -244,7 +244,7 @@ static int try_to_emulate(struct kprobe *p, struct 
> pt_regs *regs)
>  * So, we should never get here... but, its still
>  * good to catch them, just in case...
>  */
> -   printk("Can't step on instruction %x\n", ppc_inst_val(insn));
> +   printk("Can't step on instruction %s\n", 
> ppc_inst_as_str(insn));
> BUG();
> } else {
> /*
> diff --git a/arch/powerpc/kernel/trace/ftrace.c 
> b/arch/powerpc/kernel/trace/ftrace.c
> index 5e399628f51a..da11a26d8213 100644
> --- a/arch/powerpc/kernel/trace/ftrace.c
> +++ b/arch/powerpc/kernel/trace/ftrace.c
> @@ -73,8 +73,8 @@ ftrace_modify_code(unsigned long ip, struct ppc_inst old, 
> struct ppc_inst new)
>
> /* Make sure it is what we expect it to be */
> if (!ppc_inst_equal(replaced, old)) {
> -   pr_err("%p: replaced (%#x) != old (%#x)",
> -   (void *)ip, ppc_inst_val(replaced), ppc_inst_val(old));
> +   pr_err("%p: replaced (%s) != old (%s)",
> +   (void *)ip, ppc_inst_as_str(replaced), ppc_inst_as_str(old));
> return -EINVAL;
> }
>
> @@ -137,7 +137,7 @@ __ftrace_make_nop(struct module *mod,
>
> /* Make sure that that this is still a 24bit jump */
> if (!is_bl_op(op)) {
> -   pr_err("Not expected bl: opcode is %x\n", ppc_inst_val(op));
> +   pr_err("Not expected bl: opcode is %s\n", 
> ppc_inst_as_str(op));
> return -EINVAL;
> }
>
> @@ -172,8 +172,8 @@ __ftrace_make_nop(struct module *mod,
> /* We expect either a mflr r0, or a std r0, LRSAVE(r1) */
> if (!ppc_inst_equal(op, ppc_inst(PPC_INST_MFLR)) &&
> !ppc_inst_equal(op, ppc_inst(PPC_INST_STD_LR))) {
> -   pr_err("Unexpected instruction %08x around bl

Re: [PATCH RFC 1/4] powerpc/radix: Fix compilation for radix with CONFIG_SMP=n

[Joel Stanley]

On Sat, 9 May 2020 at 07:52, Nicholas Piggin  wrote:
>
> Excerpts from Paul Mackerras's message of May 9, 2020 3:02 pm:
> > This fixes the compile errors we currently get with CONFIG_SMP=n and
> > CONFIG_PPC_RADIX_MMU=y.
>
> Did I already fix this, or does it keep getting broken?! :(
>
> Anyway fine by me if it's required.

You're right, your fix was merged in 5.7-rc1.

Cheers,

Joel

Re: [PATCH 4/5] powerpc: Replace _ALIGN() by ALIGN()

[Joel Stanley]

On Mon, 20 Apr 2020 at 18:39, Christophe Leroy  wrote:
>
> _ALIGN() is specific to powerpc
> ALIGN() is generic and does the same
>
> Replace _ALIGN() by ALIGN()
>
> Signed-off-by: Christophe Leroy 

Reviewed-by: Joel Stanley 

> ---
>  arch/powerpc/include/asm/book3s/32/pgtable.h | 2 +-
>  arch/powerpc/include/asm/nohash/32/pgtable.h | 2 +-
>  arch/powerpc/kernel/prom_init.c  | 8 
>  arch/powerpc/platforms/powermac/bootx_init.c | 4 ++--
>  4 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/book3s/32/pgtable.h 
> b/arch/powerpc/include/asm/book3s/32/pgtable.h
> index 53b5c93eaf5d..0d4bccb4b9f2 100644
> --- a/arch/powerpc/include/asm/book3s/32/pgtable.h
> +++ b/arch/powerpc/include/asm/book3s/32/pgtable.h
> @@ -188,7 +188,7 @@ int map_kernel_page(unsigned long va, phys_addr_t pa, 
> pgprot_t prot);
>   * memory shall not share segments.
>   */
>  #if defined(CONFIG_STRICT_KERNEL_RWX) && defined(CONFIG_MODULES)
> -#define VMALLOC_START ((_ALIGN((long)high_memory, 256L << 20) + 
> VMALLOC_OFFSET) & \
> +#define VMALLOC_START ((ALIGN((long)high_memory, 256L << 20) + 
> VMALLOC_OFFSET) & \
>~(VMALLOC_OFFSET - 1))
>  #else
>  #define VMALLOC_START long)high_memory + VMALLOC_OFFSET) & 
> ~(VMALLOC_OFFSET-1)))
> diff --git a/arch/powerpc/include/asm/nohash/32/pgtable.h 
> b/arch/powerpc/include/asm/nohash/32/pgtable.h
> index 5b4d4c4297e1..4315d40906a0 100644
> --- a/arch/powerpc/include/asm/nohash/32/pgtable.h
> +++ b/arch/powerpc/include/asm/nohash/32/pgtable.h
> @@ -110,7 +110,7 @@ int map_kernel_page(unsigned long va, phys_addr_t pa, 
> pgprot_t prot);
>   */
>  #define VMALLOC_OFFSET (0x100) /* 16M */
>  #ifdef PPC_PIN_SIZE
> -#define VMALLOC_START (((_ALIGN((long)high_memory, PPC_PIN_SIZE) + 
> VMALLOC_OFFSET) & ~(VMALLOC_OFFSET-1)))
> +#define VMALLOC_START (((ALIGN((long)high_memory, PPC_PIN_SIZE) + 
> VMALLOC_OFFSET) & ~(VMALLOC_OFFSET-1)))

Perhaps this once needed to be more flexiable, but now it always
aligns to 256M and then to 16MB.

>  #else
>  #define VMALLOC_START long)high_memory + VMALLOC_OFFSET) & 
> ~(VMALLOC_OFFSET-1)))

This is an open coded align to VMALLOC_OFFSET.

>  #endif
> diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
> index 3a5a7db4564f..e3a9fde51c4f 100644
> --- a/arch/powerpc/kernel/prom_init.c
> +++ b/arch/powerpc/kernel/prom_init.c
> @@ -2426,7 +2426,7 @@ static void __init *make_room(unsigned long *mem_start, 
> unsigned long *mem_end,
>  {
> void *ret;
>
> -   *mem_start = _ALIGN(*mem_start, align);
> +   *mem_start = ALIGN(*mem_start, align);
> while ((*mem_start + needed) > *mem_end) {
> unsigned long room, chunk;
>
> @@ -2562,7 +2562,7 @@ static void __init scan_dt_build_struct(phandle node, 
> unsigned long *mem_start,
> *lp++ = *p;
> }
> *lp = 0;
> -   *mem_start = _ALIGN((unsigned long)lp + 1, 4);
> +   *mem_start = ALIGN((unsigned long)lp + 1, 4);
> }
>
> /* get it again for debugging */
> @@ -2608,7 +2608,7 @@ static void __init scan_dt_build_struct(phandle node, 
> unsigned long *mem_start,
> /* push property content */
> valp = make_room(mem_start, mem_end, l, 4);
> call_prom("getprop", 4, 1, node, pname, valp, l);
> -   *mem_start = _ALIGN(*mem_start, 4);
> +   *mem_start = ALIGN(*mem_start, 4);
>
> if (!prom_strcmp(pname, "phandle"))
> has_phandle = 1;
> @@ -2667,7 +2667,7 @@ static void __init flatten_device_tree(void)
> prom_panic ("couldn't get device tree root\n");
>
> /* Build header and make room for mem rsv map */
> -   mem_start = _ALIGN(mem_start, 4);
> +   mem_start = ALIGN(mem_start, 4);
> hdr = make_room(_start, _end,
> sizeof(struct boot_param_header), 4);
> dt_header_start = (unsigned long)hdr;
> diff --git a/arch/powerpc/platforms/powermac/bootx_init.c 
> b/arch/powerpc/platforms/powermac/bootx_init.c
> index c3374a90952f..9d4ecd292255 100644
> --- a/arch/powerpc/platforms/powermac/bootx_init.c
> +++ b/arch/powerpc/platforms/powermac/bootx_init.c
> @@ -386,7 +386,7 @@ static unsigned long __init bootx_flatten_dt(unsigned 
> long start)
> hdr->dt_strings_size = bootx_dt_strend - bootx_dt_strbase;
>
> /* Build structure */
> -   mem_end = _ALIGN(mem_end, 16);
> +   mem_end

Re: [PATCH 5/5] powerpc: Remove _ALIGN_UP(), _ALIGN_DOWN() and _ALIGN()

[Joel Stanley]

On Mon, 20 Apr 2020 at 18:39, Christophe Leroy  wrote:
>
> These three powerpc macros have been replaced by
> equivalent generic macros and are not used anymore.
>
> Remove them.
>
> Signed-off-by: Christophe Leroy 

Reviewed-By: Joel Stanley 

riscv has a copy of these too that could probably be removed:

arch/riscv/include/asm/page.h:#define _ALIGN_UP(addr, size)
(((addr)+((size)-1))&(~((size)-1)))
arch/riscv/include/asm/page.h:#define _ALIGN(addr, size)
_ALIGN_UP(addr, size)



> ---
>  arch/powerpc/include/asm/page.h | 7 ---
>  1 file changed, 7 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/page.h b/arch/powerpc/include/asm/page.h
> index 3ee8df0f66e0..a63fe6f3a0ff 100644
> --- a/arch/powerpc/include/asm/page.h
> +++ b/arch/powerpc/include/asm/page.h
> @@ -249,13 +249,6 @@ static inline bool pfn_valid(unsigned long pfn)
>  #include 
>  #endif
>
> -/* align addr on a size boundary - adjust address up/down if needed */
> -#define _ALIGN_UP(addr, size)   __ALIGN_KERNEL(addr, size)
> -#define _ALIGN_DOWN(addr, size)((addr)&(~((typeof(addr))(size)-1)))
> -
> -/* align addr on a size boundary - adjust address up if needed */
> -#define _ALIGN(addr,size) _ALIGN_UP(addr,size)
> -
>  /*
>   * Don't compare things with KERNELBASE or PAGE_OFFSET to test for
>   * "kernelness", use is_kernel_addr() - it should do what you want.
> --
> 2.25.0
>

Re: [PATCH 3/5] powerpc: Replace _ALIGN_UP() by ALIGN()

[Joel Stanley]

On Mon, 20 Apr 2020 at 18:39, Christophe Leroy  wrote:
>
> _ALIGN_UP() is specific to powerpc
> ALIGN() is generic and does the same
>
> Replace _ALIGN_UP() by ALIGN()
>
> Signed-off-by: Christophe Leroy 

Reviewed-by: Joel Stanley 

> ---
>  arch/powerpc/include/asm/iommu.h |  4 ++--
>  arch/powerpc/kernel/head_booke.h |  2 +-
>  arch/powerpc/kernel/nvram_64.c   |  4 ++--
>  arch/powerpc/kernel/pci_64.c |  2 +-
>  arch/powerpc/kernel/prom.c   |  4 ++--
>  arch/powerpc/kernel/prom_init.c  |  8 
>  arch/powerpc/kvm/book3s_64_vio_hv.c  |  2 +-
>  arch/powerpc/mm/book3s64/hash_tlb.c  |  2 +-
>  arch/powerpc/mm/book3s64/radix_pgtable.c |  2 +-
>  arch/powerpc/mm/slice.c  |  2 +-
>  arch/powerpc/platforms/cell/iommu.c  |  6 +++---
>  arch/powerpc/platforms/powermac/bootx_init.c | 10 +-
>  arch/powerpc/platforms/powernv/pci-ioda.c|  8 
>  arch/powerpc/platforms/ps3/mm.c  | 16 
>  arch/powerpc/platforms/ps3/setup.c   |  2 +-
>  15 files changed, 37 insertions(+), 37 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/iommu.h 
> b/arch/powerpc/include/asm/iommu.h
> index 350101e11ddb..5032f1593299 100644
> --- a/arch/powerpc/include/asm/iommu.h
> +++ b/arch/powerpc/include/asm/iommu.h
> @@ -22,11 +22,11 @@
>  #define IOMMU_PAGE_SHIFT_4K  12
>  #define IOMMU_PAGE_SIZE_4K   (ASM_CONST(1) << IOMMU_PAGE_SHIFT_4K)
>  #define IOMMU_PAGE_MASK_4K   (~((1 << IOMMU_PAGE_SHIFT_4K) - 1))
> -#define IOMMU_PAGE_ALIGN_4K(addr) _ALIGN_UP(addr, IOMMU_PAGE_SIZE_4K)
> +#define IOMMU_PAGE_ALIGN_4K(addr) ALIGN(addr, IOMMU_PAGE_SIZE_4K)
>
>  #define IOMMU_PAGE_SIZE(tblptr) (ASM_CONST(1) << (tblptr)->it_page_shift)
>  #define IOMMU_PAGE_MASK(tblptr) (~((1 << (tblptr)->it_page_shift) - 1))
> -#define IOMMU_PAGE_ALIGN(addr, tblptr) _ALIGN_UP(addr, 
> IOMMU_PAGE_SIZE(tblptr))
> +#define IOMMU_PAGE_ALIGN(addr, tblptr) ALIGN(addr, IOMMU_PAGE_SIZE(tblptr))
>
>  /* Boot time flags */
>  extern int iommu_is_off;
> diff --git a/arch/powerpc/kernel/head_booke.h 
> b/arch/powerpc/kernel/head_booke.h
> index bd2e5ed8dd50..18f87bf9e32b 100644
> --- a/arch/powerpc/kernel/head_booke.h
> +++ b/arch/powerpc/kernel/head_booke.h
> @@ -534,7 +534,7 @@ struct exception_regs {
>  };
>
>  /* ensure this structure is always sized to a multiple of the stack 
> alignment */
> -#define STACK_EXC_LVL_FRAME_SIZE   _ALIGN_UP(sizeof (struct 
> exception_regs), 16)
> +#define STACK_EXC_LVL_FRAME_SIZE   ALIGN(sizeof (struct exception_regs), 
> 16)
>
>  #endif /* __ASSEMBLY__ */
>  #endif /* __HEAD_BOOKE_H__ */
> diff --git a/arch/powerpc/kernel/nvram_64.c b/arch/powerpc/kernel/nvram_64.c
> index fb4f61096613..314780e8ef78 100644
> --- a/arch/powerpc/kernel/nvram_64.c
> +++ b/arch/powerpc/kernel/nvram_64.c
> @@ -854,8 +854,8 @@ loff_t __init nvram_create_partition(const char *name, 
> int sig,
> BUILD_BUG_ON(NVRAM_BLOCK_LEN != 16);
>
> /* Convert sizes from bytes to blocks */
> -   req_size = _ALIGN_UP(req_size, NVRAM_BLOCK_LEN) / NVRAM_BLOCK_LEN;
> -   min_size = _ALIGN_UP(min_size, NVRAM_BLOCK_LEN) / NVRAM_BLOCK_LEN;
> +   req_size = ALIGN(req_size, NVRAM_BLOCK_LEN) / NVRAM_BLOCK_LEN;
> +   min_size = ALIGN(min_size, NVRAM_BLOCK_LEN) / NVRAM_BLOCK_LEN;
>
> /* If no minimum size specified, make it the same as the
>  * requested size
> diff --git a/arch/powerpc/kernel/pci_64.c b/arch/powerpc/kernel/pci_64.c
> index e5d05af5a9af..ff8e3fbdf663 100644
> --- a/arch/powerpc/kernel/pci_64.c
> +++ b/arch/powerpc/kernel/pci_64.c
> @@ -131,7 +131,7 @@ static int pcibios_map_phb_io_space(struct pci_controller 
> *hose)
> unsigned long io_virt_offset;
>
> phys_page = ALIGN_DOWN(hose->io_base_phys, PAGE_SIZE);
> -   size_page = _ALIGN_UP(hose->pci_io_size, PAGE_SIZE);
> +   size_page = ALIGN(hose->pci_io_size, PAGE_SIZE);
>
> /* Make sure IO area address is clear */
> hose->io_base_alloc = NULL;
> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
> index 10b5d5eafd34..1dcf0e214a22 100644
> --- a/arch/powerpc/kernel/prom.c
> +++ b/arch/powerpc/kernel/prom.c
> @@ -97,7 +97,7 @@ static inline int overlaps_initrd(unsigned long start, 
> unsigned long size)
> return 0;
>
> return  (start + size) > ALIGN_DOWN(initrd_start, PAGE_SIZE) &&
> -   start <= _ALIGN_UP(initrd_end, PAGE_SIZE);
> +   start <= ALIGN(initrd_end, PAGE_SIZE);
>  #else
>

Re: [PATCH 2/5] powerpc: Replace _ALIGN_DOWN() by ALIGN_DOWN()

[Joel Stanley]

On Mon, 20 Apr 2020 at 18:38, Christophe Leroy  wrote:
>
> _ALIGN_DOWN() is specific to powerpc
> ALIGN_DOWN() is generic and does the same
>
> Replace _ALIGN_DOWN() by ALIGN_DOWN()

This one is a bit less obvious. It becomes (leaving the typeof's alone
for clarity):

-((addr)&(~((typeof(addr))(size)-1)))
+addr) - ((size) - 1)) + ((typeof(addr))(size) - 1)) &
~((typeof(addr))(size)-1))

Which I assume the compiler will sort out?

Reviewed-by: Joel Stanley 




>
> Signed-off-by: Christophe Leroy 
> ---
>  arch/powerpc/include/asm/book3s/32/pgtable.h |  2 +-
>  arch/powerpc/include/asm/nohash/32/pgtable.h |  2 +-
>  arch/powerpc/kernel/pci_64.c |  2 +-
>  arch/powerpc/kernel/prom.c   |  6 +++---
>  arch/powerpc/kernel/prom_init.c  |  8 
>  arch/powerpc/mm/book3s64/hash_tlb.c  |  4 ++--
>  arch/powerpc/mm/init_64.c|  4 ++--
>  arch/powerpc/platforms/powernv/opal-fadump.c |  2 +-
>  arch/powerpc/platforms/powernv/pci-ioda.c|  2 +-
>  arch/powerpc/platforms/ps3/mm.c  | 14 +++---
>  arch/powerpc/platforms/pseries/rtas-fadump.c |  2 +-
>  11 files changed, 24 insertions(+), 24 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/book3s/32/pgtable.h 
> b/arch/powerpc/include/asm/book3s/32/pgtable.h
> index 7549393c4c43..53b5c93eaf5d 100644
> --- a/arch/powerpc/include/asm/book3s/32/pgtable.h
> +++ b/arch/powerpc/include/asm/book3s/32/pgtable.h
> @@ -195,7 +195,7 @@ int map_kernel_page(unsigned long va, phys_addr_t pa, 
> pgprot_t prot);
>  #endif
>
>  #ifdef CONFIG_KASAN_VMALLOC
> -#define VMALLOC_END_ALIGN_DOWN(ioremap_bot, PAGE_SIZE << 
> KASAN_SHADOW_SCALE_SHIFT)
> +#define VMALLOC_ENDALIGN_DOWN(ioremap_bot, PAGE_SIZE << 
> KASAN_SHADOW_SCALE_SHIFT)
>  #else
>  #define VMALLOC_ENDioremap_bot
>  #endif
> diff --git a/arch/powerpc/include/asm/nohash/32/pgtable.h 
> b/arch/powerpc/include/asm/nohash/32/pgtable.h
> index b04ba257fddb..5b4d4c4297e1 100644
> --- a/arch/powerpc/include/asm/nohash/32/pgtable.h
> +++ b/arch/powerpc/include/asm/nohash/32/pgtable.h
> @@ -116,7 +116,7 @@ int map_kernel_page(unsigned long va, phys_addr_t pa, 
> pgprot_t prot);
>  #endif
>
>  #ifdef CONFIG_KASAN_VMALLOC
> -#define VMALLOC_END_ALIGN_DOWN(ioremap_bot, PAGE_SIZE << 
> KASAN_SHADOW_SCALE_SHIFT)
> +#define VMALLOC_ENDALIGN_DOWN(ioremap_bot, PAGE_SIZE << 
> KASAN_SHADOW_SCALE_SHIFT)
>  #else
>  #define VMALLOC_ENDioremap_bot
>  #endif
> diff --git a/arch/powerpc/kernel/pci_64.c b/arch/powerpc/kernel/pci_64.c
> index f83d1f69b1dd..e5d05af5a9af 100644
> --- a/arch/powerpc/kernel/pci_64.c
> +++ b/arch/powerpc/kernel/pci_64.c
> @@ -130,7 +130,7 @@ static int pcibios_map_phb_io_space(struct pci_controller 
> *hose)
> unsigned long size_page;
> unsigned long io_virt_offset;
>
> -   phys_page = _ALIGN_DOWN(hose->io_base_phys, PAGE_SIZE);
> +   phys_page = ALIGN_DOWN(hose->io_base_phys, PAGE_SIZE);
> size_page = _ALIGN_UP(hose->pci_io_size, PAGE_SIZE);
>
> /* Make sure IO area address is clear */
> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
> index 6620f37abe73..10b5d5eafd34 100644
> --- a/arch/powerpc/kernel/prom.c
> +++ b/arch/powerpc/kernel/prom.c
> @@ -96,7 +96,7 @@ static inline int overlaps_initrd(unsigned long start, 
> unsigned long size)
> if (!initrd_start)
> return 0;
>
> -   return  (start + size) > _ALIGN_DOWN(initrd_start, PAGE_SIZE) &&
> +   return  (start + size) > ALIGN_DOWN(initrd_start, PAGE_SIZE) &&
> start <= _ALIGN_UP(initrd_end, PAGE_SIZE);
>  #else
> return 0;
> @@ -623,9 +623,9 @@ static void __init early_reserve_mem(void)
>  #ifdef CONFIG_BLK_DEV_INITRD
> /* Then reserve the initrd, if any */
> if (initrd_start && (initrd_end > initrd_start)) {
> -   memblock_reserve(_ALIGN_DOWN(__pa(initrd_start), PAGE_SIZE),
> +   memblock_reserve(ALIGN_DOWN(__pa(initrd_start), PAGE_SIZE),
> _ALIGN_UP(initrd_end, PAGE_SIZE) -
> -   _ALIGN_DOWN(initrd_start, PAGE_SIZE));
> +   ALIGN_DOWN(initrd_start, PAGE_SIZE));
> }
>  #endif /* CONFIG_BLK_DEV_INITRD */
>
> diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
> index 806be751c336..4cf5958eebd4 100644
> --- a/arch/powerpc/kernel/prom_init.c
> +++ b/arch/powerpc/kernel/prom_init.c
> @@ -1500,7 +1500,7 @@ static unsigned long __init alloc_down(unsigned long 
> size, unsigned long align,
>
> 

Re: [PATCH 1/5] drivers/powerpc: Replace _ALIGN_UP() by ALIGN()

[Joel Stanley]

On Mon, 20 Apr 2020 at 18:37, Christophe Leroy  wrote:
>
> _ALIGN_UP() is specific to powerpc
> ALIGN() is generic and does the same
>
> Replace _ALIGN_UP() by ALIGN()
>
> Signed-off-by: Christophe Leroy 

I was curious, so I expanded out the kernel one. Here's the diff:

- (((addr)+((size)-1))&(~((typeof(addr))(size)-1)))
+ (((addr)+((typeof(addr))(size) - 1))&~((typeof(addr))(size)-1))

So it adds a cast, but aside from that it's the same.

Reviewed-by: Joel Stanley 

> ---
>  drivers/ps3/ps3-lpm.c   | 6 +++---
>  drivers/vfio/pci/vfio_pci_nvlink2.c | 2 +-
>  drivers/video/fbdev/ps3fb.c | 4 ++--
>  sound/ppc/snd_ps3.c | 2 +-
>  4 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/ps3/ps3-lpm.c b/drivers/ps3/ps3-lpm.c
> index 83c45659bc9d..064b5884ba13 100644
> --- a/drivers/ps3/ps3-lpm.c
> +++ b/drivers/ps3/ps3-lpm.c
> @@ -1096,8 +1096,8 @@ int ps3_lpm_open(enum ps3_lpm_tb_type tb_type, void 
> *tb_cache,
> lpm_priv->tb_cache_internal = NULL;
> lpm_priv->tb_cache = NULL;
> } else if (tb_cache) {
> -   if (tb_cache != (void *)_ALIGN_UP((unsigned long)tb_cache, 
> 128)
> -   || tb_cache_size != _ALIGN_UP(tb_cache_size, 128)) {
> +   if (tb_cache != (void *)ALIGN((unsigned long)tb_cache, 128)
> +   || tb_cache_size != ALIGN(tb_cache_size, 128)) {
> dev_err(sbd_core(), "%s:%u: unaligned tb_cache\n",
> __func__, __LINE__);
> result = -EINVAL;
> @@ -1116,7 +1116,7 @@ int ps3_lpm_open(enum ps3_lpm_tb_type tb_type, void 
> *tb_cache,
> result = -ENOMEM;
> goto fail_malloc;
> }
> -   lpm_priv->tb_cache = (void *)_ALIGN_UP(
> +   lpm_priv->tb_cache = (void *)ALIGN(
> (unsigned long)lpm_priv->tb_cache_internal, 128);
> }
>
> diff --git a/drivers/vfio/pci/vfio_pci_nvlink2.c 
> b/drivers/vfio/pci/vfio_pci_nvlink2.c
> index ed20d73cc27c..65c61710c0e9 100644
> --- a/drivers/vfio/pci/vfio_pci_nvlink2.c
> +++ b/drivers/vfio/pci/vfio_pci_nvlink2.c
> @@ -67,7 +67,7 @@ static size_t vfio_pci_nvgpu_rw(struct vfio_pci_device 
> *vdev,
>  *
>  * This is not fast path anyway.
>  */
> -   sizealigned = _ALIGN_UP(posoff + count, PAGE_SIZE);
> +   sizealigned = ALIGN(posoff + count, PAGE_SIZE);
> ptr = ioremap_cache(data->gpu_hpa + posaligned, sizealigned);
> if (!ptr)
> return -EFAULT;
> diff --git a/drivers/video/fbdev/ps3fb.c b/drivers/video/fbdev/ps3fb.c
> index 834f63edf700..9df78fb77267 100644
> --- a/drivers/video/fbdev/ps3fb.c
> +++ b/drivers/video/fbdev/ps3fb.c
> @@ -44,7 +44,7 @@
>  #define GPU_CMD_BUF_SIZE   (2 * 1024 * 1024)
>  #define GPU_FB_START   (64 * 1024)
>  #define GPU_IOIF   (0x0d00UL)
> -#define GPU_ALIGN_UP(x)_ALIGN_UP((x), 64)
> +#define GPU_ALIGN_UP(x)ALIGN((x), 64)
>  #define GPU_MAX_LINE_LENGTH(65536 - 64)
>
>  #define GPU_INTR_STATUS_VSYNC_00   /* vsync on 
> head A */
> @@ -1015,7 +1015,7 @@ static int ps3fb_probe(struct ps3_system_bus_device 
> *dev)
> }
>  #endif
>
> -   max_ps3fb_size = _ALIGN_UP(GPU_IOIF, 256*1024*1024) - GPU_IOIF;
> +   max_ps3fb_size = ALIGN(GPU_IOIF, 256*1024*1024) - GPU_IOIF;
> if (ps3fb_videomemory.size > max_ps3fb_size) {
> dev_info(>core, "Limiting ps3fb mem size to %lu bytes\n",
>  max_ps3fb_size);
> diff --git a/sound/ppc/snd_ps3.c b/sound/ppc/snd_ps3.c
> index 6d2a33b8faa0..b8161a08f2ca 100644
> --- a/sound/ppc/snd_ps3.c
> +++ b/sound/ppc/snd_ps3.c
> @@ -926,7 +926,7 @@ static int snd_ps3_driver_probe(struct 
> ps3_system_bus_device *dev)
> PAGE_SHIFT, /* use system page size */
> 0, /* dma type; not used */
> NULL,
> -   _ALIGN_UP(SND_PS3_DMA_REGION_SIZE, PAGE_SIZE));
> +   ALIGN(SND_PS3_DMA_REGION_SIZE, PAGE_SIZE));
> dev->d_region->ioid = PS3_AUDIO_IOID;
>
> ret = ps3_dma_region_create(dev->d_region);
> --
> 2.25.0
>

Re: [PATCH] powerpc/boot: Delete unneeded .globl _zimage_start

[Joel Stanley]

On Wed, 25 Mar 2020 at 05:19, Fangrui Song  wrote:
>
> .globl sets the symbol binding to STB_GLOBAL while .weak sets the
> binding to STB_WEAK. They should not be used together. It is accidetal
> rather then intentional that GNU as let .weak override .globl while
> clang integrated assembler let the last win.
>
> Fixes: cd197ffcf10b "[POWERPC] zImage: Cleanup and improve zImage entry point"
> Fixes: ee9d21b3b358 "powerpc/boot: Ensure _zimage_start is a weak symbol"
> Link: https://github.com/ClangBuiltLinux/linux/issues/937
> Signed-off-by: Fangrui Song 
> Cc: Joel Stanley 
> Cc: Michael Ellerman 
> Cc: Nick Desaulniers 
> Cc: clang-built-li...@googlegroups.com
> ---
>  arch/powerpc/boot/crt0.S | 3 ---
>  1 file changed, 3 deletions(-)
>
> diff --git a/arch/powerpc/boot/crt0.S b/arch/powerpc/boot/crt0.S
> index 92608f34d312..1d83966f5ef6 100644
> --- a/arch/powerpc/boot/crt0.S
> +++ b/arch/powerpc/boot/crt0.S
> @@ -44,9 +44,6 @@ p_end:.long   _end
>  p_pstack:  .long   _platform_stack_top
>  #endif
>
> -   .globl  _zimage_start
> -   /* Clang appears to require the .weak directive to be after the symbol
> -* is defined. See https://bugs.llvm.org/show_bug.cgi?id=38921  */
> .weak   _zimage_start
>  _zimage_start:

Your explanation makes sense to me. I've added Alan to cc for his review.

Reviewed-by: Joel Stanley 

Thanks for the patch.

Cheers,

Joel

> .globl  _zimage_start_lib
> --
> 2.25.1.696.g5e7596f4ac-goog
>

Re: [PATCH v2 07/10] powerpc/configs/skiroot: Enable security features

[Joel Stanley]

On Tue, 21 Jan 2020 at 04:30, Michael Ellerman  wrote:
>
> From: Joel Stanley 
>
> This turns on HARDENED_USERCOPY with HARDENED_USERCOPY_PAGESPAN, and
> FORTIFY_SOURCE.
>
> It also enables SECURITY_LOCKDOWN_LSM with _EARLY and
> LOCK_DOWN_KERNEL_FORCE_INTEGRITY options enabled. This still allows
> xmon to be used in read-only mode.
>
> MODULE_SIG is selected by lockdown, so it is still enabled.
>
> Signed-off-by: Joel Stanley 
> [mpe: Switch to lockdown integrity mode per oohal]
> Signed-off-by: Michael Ellerman 

I did some testing and with change we break kexec. As it's critical
for this kernel to be able to kexec we need to set KEXEC_FILE=y if
we're setting FORCE_INTEGRITY=y.

I've tested your series with that modification made and userspace was
once again able to kexec (with -s).

Cheers,

Joel

> ---
>  arch/powerpc/configs/skiroot_defconfig | 11 ++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
>
> v2: Switch to lockdown integrity mode rather than confidentiality as noticed 
> by
> dja and discussed with jms and oohal.
>
> diff --git a/arch/powerpc/configs/skiroot_defconfig 
> b/arch/powerpc/configs/skiroot_defconfig
> index 24a210fe0049..93b478436a2b 100644
> --- a/arch/powerpc/configs/skiroot_defconfig
> +++ b/arch/powerpc/configs/skiroot_defconfig
> @@ -49,7 +49,6 @@ CONFIG_JUMP_LABEL=y
>  CONFIG_STRICT_KERNEL_RWX=y
>  CONFIG_MODULES=y
>  CONFIG_MODULE_UNLOAD=y
> -CONFIG_MODULE_SIG=y
>  CONFIG_MODULE_SIG_FORCE=y
>  CONFIG_MODULE_SIG_SHA512=y
>  CONFIG_PARTITION_ADVANCED=y
> @@ -272,6 +271,16 @@ CONFIG_NLS_ASCII=y
>  CONFIG_NLS_ISO8859_1=y
>  CONFIG_NLS_UTF8=y
>  CONFIG_ENCRYPTED_KEYS=y
> +CONFIG_SECURITY=y
> +CONFIG_HARDENED_USERCOPY=y
> +# CONFIG_HARDENED_USERCOPY_FALLBACK is not set
> +CONFIG_HARDENED_USERCOPY_PAGESPAN=y
> +CONFIG_FORTIFY_SOURCE=y
> +CONFIG_SECURITY_LOCKDOWN_LSM=y
> +CONFIG_SECURITY_LOCKDOWN_LSM_EARLY=y
> +CONFIG_LOCK_DOWN_KERNEL_FORCE_INTEGRITY=y
> +# CONFIG_INTEGRITY is not set
> +CONFIG_LSM="yama,loadpin,safesetid,integrity"
>  # CONFIG_CRYPTO_HW is not set
>  CONFIG_CRC16=y
>  CONFIG_CRC_ITU_T=y
> --
> 2.21.1
>

Re: [PATCH 7/9] powerpc/configs/skiroot: Enable security features

[Joel Stanley]

On Thu, 16 Jan 2020 at 07:10, Oliver O'Halloran  wrote:
>
> On Thu, Jan 16, 2020 at 4:00 PM Daniel Axtens  wrote:
> >
> > Michael Ellerman  writes:
> >
> > > From: Joel Stanley 
> > >
> > > This turns on HARDENED_USERCOPY with HARDENED_USERCOPY_PAGESPAN, and
> > > FORTIFY_SOURCE.
> > >
> > > It also enables SECURITY_LOCKDOWN_LSM with _EARLY and
> > > LOCK_DOWN_KERNEL_FORCE_CONFIDENTIALITY options enabled.
> > >
> >
> > As I said before, this will disable xmon entirely. If we want to set
> > this, we should compile out xmon. But if we want xmon in read-only mode
> > to be an option, we should pick integrity mode.
> >
> > I don't really mind, because I don't work with skiroot very
> > much. Oliver, Joel, Nayna, you all do stuff around this sort of level -
> > is this a problem for any of you?
>
> Keep it enabled and force INTEGRITY mode. There are some cases where
> xmon is the only method for debugging a crashing skiroot (hello SMC
> BMCs) so I'd rather it remained available. If there's some actual
> security benefit to disabling it entirely then someone should
> articulate that.

Ack.

Re: [PATCH 5/9] powerpc/configs/skiroot: Drop default n CONFIG_CRYPTO_ECHAINIV

[Joel Stanley]

On Thu, 16 Jan 2020 at 01:48, Michael Ellerman  wrote:
>
> It's default n so we don't need to disable it.
>
> Signed-off-by: Michael Ellerman 

Acked-by: Joel Stanley 

Re: [PATCH 4/9] powerpc/configs/skiroot: Drop HID_LOGITECH

[Joel Stanley]

On Thu, 16 Jan 2020 at 01:48, Michael Ellerman  wrote:
>
> Commit bdd08fff4915 ("HID: logitech: Add depends on LEDS_CLASS to
> Logitech Kconfig entry") made HID_LOGITECH depend on LEDS_CLASS which
> we do not enable, meaning we are not actually enabling those drivers
> any more.
>
> The Kconfig help text suggests USB HID compliant Logictech devices
> will continue to work without HID_LOGITECH, so just drop it.
>
> Signed-off-by: Michael Ellerman 

Acked-by: Joel Stanley 

Re: [PATCH 3/9] powerpc/configs: Drop NET_VENDOR_HP which moved to staging

[Joel Stanley]

On Thu, 16 Jan 2020 at 01:48, Michael Ellerman  wrote:
>
> The HP network driver moved to staging in commit 52340b82cf1a ("hp100:
> Move 100BaseVG AnyLAN driver to staging") meaning we don't need to
> disable it any more in our defconfigs.
>
> Signed-off-by: Michael Ellerman 

Acked-by: Joel Stanley