LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH V3] sched/rt, powerpc: Prepare for PREEMPT_RT
From: Michael Ellerman @ 2020-11-16 12:46 UTC (permalink / raw)
  To: Wang Qing, Benjamin Herrenschmidt, Paul Mackerras,
	Christophe Leroy, Nicholas Piggin, Jordan Niethe, Alistair Popple,
	Wang Qing, Aneesh Kumar K.V, Peter Zijlstra, Greg Kroah-Hartman,
	linuxppc-dev, linux-kernel
In-Reply-To: <1604998411-16116-1-git-send-email-wangqing@vivo.com>

Wang Qing <wangqing@vivo.com> writes:
> PREEMPT_RT is a separate preemption model, CONFIG_PREEMPT will
>  be disabled when CONFIG_PREEMPT_RT is enabled,  so we need
> to add CONFIG_PREEMPT_RT output to __die().
>
> Signed-off-by: Wang Qing <wangqing@vivo.com>

Something fairly similar was posted previously.

That time I said:

  I don't think there's any point adding the "_RT" to the __die() output
  until/if we ever start supporting PREEMPT_RT.

  https://lore.kernel.org/linuxppc-dev/87d0ext4q3.fsf@mpe.ellerman.id.au/
   

And I think I still feel the same way. It's not clear powerpc will ever
support PREEMPT_RT, so this would just be confusing to people. And
potentially someone will then send a patch to remove it as dead code.

cheers



> diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
> index 5006dcb..dec7b81
> --- a/arch/powerpc/kernel/traps.c
> +++ b/arch/powerpc/kernel/traps.c
> @@ -262,10 +262,11 @@ static int __die(const char *str, struct pt_regs *regs, long err)
>  {
>  	printk("Oops: %s, sig: %ld [#%d]\n", str, err, ++die_counter);
>  
> -	printk("%s PAGE_SIZE=%luK%s%s%s%s%s%s %s\n",
> +	printk("%s PAGE_SIZE=%luK%s%s%s%s%s%s%s %s\n",
>  	       IS_ENABLED(CONFIG_CPU_LITTLE_ENDIAN) ? "LE" : "BE",
>  	       PAGE_SIZE / 1024, get_mmu_str(),
>  	       IS_ENABLED(CONFIG_PREEMPT) ? " PREEMPT" : "",
> +	       IS_ENABLED(CONFIG_PREEMPT_RT) ? " PREEMPT_RT" : "",
>  	       IS_ENABLED(CONFIG_SMP) ? " SMP" : "",
>  	       IS_ENABLED(CONFIG_SMP) ? (" NR_CPUS=" __stringify(NR_CPUS)) : "",
>  	       debug_pagealloc_enabled() ? " DEBUG_PAGEALLOC" : "",
> -- 
> 2.7.4

^ permalink raw reply

* Re: [PATCH] KVM: PPC: Book3S HV: XIVE: Fix possible oops when accessing ESB page
From: Michael Ellerman @ 2020-11-16 12:29 UTC (permalink / raw)
  To: Cédric Le Goater, Paul Mackerras
  Cc: kvm, Gustavo Romero, Greg Kurz, kvm-ppc, linuxppc-dev,
	David Gibson
In-Reply-To: <1270ada4-e2a9-6a1a-52a9-b5c3479c05ea@kaod.org>

Cédric Le Goater <clg@kaod.org> writes:
> On 11/6/20 4:19 AM, Michael Ellerman wrote:
>> Cédric Le Goater <clg@kaod.org> writes:
>>> When accessing the ESB page of a source interrupt, the fault handler
>>> will retrieve the page address from the XIVE interrupt 'xive_irq_data'
>>> structure. If the associated KVM XIVE interrupt is not valid, that is
>>> not allocated at the HW level for some reason, the fault handler will
>>> dereference a NULL pointer leading to the oops below :
>>>
>>>     WARNING: CPU: 40 PID: 59101 at arch/powerpc/kvm/book3s_xive_native.c:259 xive_native_esb_fault+0xe4/0x240 [kvm]
>>>     CPU: 40 PID: 59101 Comm: qemu-system-ppc Kdump: loaded Tainted: G        W        --------- -  - 4.18.0-240.el8.ppc64le #1
>>>     NIP:  c00800000e949fac LR: c00000000044b164 CTR: c00800000e949ec8
>>>     REGS: c000001f69617840 TRAP: 0700   Tainted: G        W        --------- -  -  (4.18.0-240.el8.ppc64le)
>>>     MSR:  9000000000029033 <SF,HV,EE,ME,IR,DR,RI,LE>  CR: 44044282  XER: 00000000
>>>     CFAR: c00000000044b160 IRQMASK: 0
>>>     GPR00: c00000000044b164 c000001f69617ac0 c00800000e96e000 c000001f69617c10
>>>     GPR04: 05faa2b21e000080 0000000000000000 0000000000000005 ffffffffffffffff
>>>     GPR08: 0000000000000000 0000000000000001 0000000000000000 0000000000000001
>>>     GPR12: c00800000e949ec8 c000001ffffd3400 0000000000000000 0000000000000000
>>>     GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>>>     GPR20: 0000000000000000 0000000000000000 c000001f5c065160 c000000001c76f90
>>>     GPR24: c000001f06f20000 c000001f5c065100 0000000000000008 c000001f0eb98c78
>>>     GPR28: c000001dcab40000 c000001dcab403d8 c000001f69617c10 0000000000000011
>>>     NIP [c00800000e949fac] xive_native_esb_fault+0xe4/0x240 [kvm]
>>>     LR [c00000000044b164] __do_fault+0x64/0x220
>>>     Call Trace:
>>>     [c000001f69617ac0] [0000000137a5dc20] 0x137a5dc20 (unreliable)
>>>     [c000001f69617b50] [c00000000044b164] __do_fault+0x64/0x220
>>>     [c000001f69617b90] [c000000000453838] do_fault+0x218/0x930
>>>     [c000001f69617bf0] [c000000000456f50] __handle_mm_fault+0x350/0xdf0
>>>     [c000001f69617cd0] [c000000000457b1c] handle_mm_fault+0x12c/0x310
>>>     [c000001f69617d10] [c00000000007ef44] __do_page_fault+0x264/0xbb0
>>>     [c000001f69617df0] [c00000000007f8c8] do_page_fault+0x38/0xd0
>>>     [c000001f69617e30] [c00000000000a714] handle_page_fault+0x18/0x38
>>>     Instruction dump:
>>>     40c2fff0 7c2004ac 2fa90000 409e0118 73e90001 41820080 e8bd0008 7c2004ac
>>>     7ca90074 39400000 915c0000 7929d182 <0b090000> 2fa50000 419e0080 e89e0018
>>>     ---[ end trace 66c6ff034c53f64f ]---
>>>     xive-kvm: xive_native_esb_fault: accessing invalid ESB page for source 8 !
>>>
>>> Fix that by checking the validity of the KVM XIVE interrupt structure.
>>>
>>> Reported-by: Greg Kurz <groug@kaod.org>
>>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> 
>> Fixes ?
>
> Ah yes :/  
>
> Cc: stable@vger.kernel.org # v5.2+
> Fixes: 6520ca64cde7 ("KVM: PPC: Book3S HV: XIVE: Add a mapping for the source ESB pages")
>
> Since my provider changed its imap servers, my email filters are really screwed 
> up and I miss emails. 
>
> Sorry about that,

No worries.

It doesn't look like Paul has grabbed this, so I'll take it.

cheers

^ permalink raw reply

* Re: [PATCH] powerpc/pseries/hotplug-cpu: Fix memleak when cpus node not exist
From: Michael Ellerman @ 2020-11-16 12:23 UTC (permalink / raw)
  To: Tyrel Datwyler, Nathan Lynch, Zhang Xiaoxu; +Cc: linuxppc-dev, paulus, groug
In-Reply-To: <a00f4a44-6f38-306f-a24f-4e3565deefc2@linux.ibm.com>

Tyrel Datwyler <tyreld@linux.ibm.com> writes:
> On 11/10/20 6:08 AM, Nathan Lynch wrote:
>> Zhang Xiaoxu <zhangxiaoxu5@huawei.com> writes:
>>> From: zhangxiaoxu <zhangxiaoxu5@huawei.com>
>>>
>>> If the cpus nodes not exist, we lost to free the 'cpu_drcs', which
>>> will leak memory.
>>>
>>> Fixes: a0ff72f9f5a7 ("powerpc/pseries/hotplug-cpu: Remove double free in error path")
>>> Reported-by: Hulk Robot <hulkci@huawei.com>
>>> Signed-off-by: zhangxiaoxu <zhangxiaoxu5@huawei.com>
>>> ---
>>>  arch/powerpc/platforms/pseries/hotplug-cpu.c | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/arch/powerpc/platforms/pseries/hotplug-cpu.c b/arch/powerpc/platforms/pseries/hotplug-cpu.c
>>> index f2837e33bf5d..4bb1c9f2bb11 100644
>>> --- a/arch/powerpc/platforms/pseries/hotplug-cpu.c
>>> +++ b/arch/powerpc/platforms/pseries/hotplug-cpu.c
>>> @@ -743,6 +743,7 @@ static int dlpar_cpu_add_by_count(u32 cpus_to_add)
>>>  	parent = of_find_node_by_path("/cpus");
>>>  	if (!parent) {
>>>  		pr_warn("Could not find CPU root node in device tree\n");
>>> +		kfree(cpu_drcs);
>>>  		return -1;
>>>  	}
>> 
>> Thanks for finding this.
>> 
>> a0ff72f9f5a7 ("powerpc/pseries/hotplug-cpu: Remove double free in error
>> path") was posted in Sept 2019 but was not applied until July 2020:
>> 
>> https://lore.kernel.org/linuxppc-dev/20190919231633.1344-1-nathanl@linux.ibm.com/
>> 
>> Here is that change as posted; note the function context is
>> find_dlpar_cpus_to_add(), not dlpar_cpu_add_by_count():
>> 
>> --- a/arch/powerpc/platforms/pseries/hotplug-cpu.c
>> +++ b/arch/powerpc/platforms/pseries/hotplug-cpu.c
>> @@ -726,7 +726,6 @@ static int find_dlpar_cpus_to_add(u32 *cpu_drcs, u32 cpus_to_add)
>>  	parent = of_find_node_by_path("/cpus");
>>  	if (!parent) {
>>  		pr_warn("Could not find CPU root node in device tree\n");
>> -		kfree(cpu_drcs);
>>  		return -1;
>>  	}
>> 
>> Meanwhile b015f6bc9547dbc056edde7177c7868ca8629c4c ("powerpc/pseries: Add
>> cpu DLPAR support for drc-info property") was posted in Nov 2019 and
>> committed a few days later:
>> 
>> https://lore.kernel.org/linux-pci/1573449697-5448-4-git-send-email-tyreld@linux.ibm.com/
>> 
>> This change reorganized the same code, removing
>> find_dlpar_cpus_to_add(), and it had the effect of fixing the same
>> issue.
>> 
>> However git apparently allowed the older change to still apply on top of
>> this (changing a function different from the one in the original
>> patch!), leading to a real bug.
>
> Yikes, not sure how that happened without either the committer massaging the
> patch to apply, or the line location and context matching exactly.

git-am won't apply it, but patch does. I often have to fall back to
using patch when things don't apply, so that's presumably what happened
here. I try to manually check the result is correct but I obviously
didn't do a good job here.

cheers

^ permalink raw reply

* [PATCH] powerpc: Drop -me200 addition to build flags
From: Michael Ellerman @ 2020-11-16 12:09 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: ndesaulniers, linux-kernel, oss, natechancellor

Currently a build with CONFIG_E200=y will fail with:

  Error: invalid switch -me200
  Error: unrecognized option -me200

Upstream binutils has never supported an -me200 option. Presumably it
was supported at some point by either a fork or Freescale internal
binutils.

We can't support code that we can't even build test, so drop the
addition of -me200 to the build flags, so we can at least build with
CONFIG_E200=y.

Reported-by: Németh Márton <nm127@freemail.hu>
Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---

More discussion: https://lore.kernel.org/lkml/202011131146.g8dPLQDD-lkp@intel.com
---
 arch/powerpc/Makefile | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
index a4d56f0a41d9..16b8336f91dd 100644
--- a/arch/powerpc/Makefile
+++ b/arch/powerpc/Makefile
@@ -248,7 +248,6 @@ KBUILD_CFLAGS		+= $(call cc-option,-mno-string)
 cpu-as-$(CONFIG_40x)		+= -Wa,-m405
 cpu-as-$(CONFIG_44x)		+= -Wa,-m440
 cpu-as-$(CONFIG_ALTIVEC)	+= $(call as-option,-Wa$(comma)-maltivec)
-cpu-as-$(CONFIG_E200)		+= -Wa,-me200
 cpu-as-$(CONFIG_E500)		+= -Wa,-me500
 
 # When using '-many -mpower4' gas will first try and find a matching power4
-- 
2.25.1


^ permalink raw reply related

* Re: [PATCH] powerpc/powernv/memtrace: Fake non-memblock aligned sized traces
From: Michael Ellerman @ 2020-11-16 12:02 UTC (permalink / raw)
  To: Jordan Niethe, linuxppc-dev; +Cc: Jordan Niethe, mikey
In-Reply-To: <20201111055524.2458-1-jniethe5@gmail.com>

Jordan Niethe <jniethe5@gmail.com> writes:
> The hardware trace macros which use the memory provided by memtrace are
> able to use trace sizes as small as 16MB. Only memblock aligned values
> can be removed from each NUMA node by writing that value to
> memtrace/enable in debugfs.  This means setting up, say, a 16MB trace is
> not possible.  To allow such a trace size, instead align whatever value
> is written to memtrace/enable to the memblock size for the purpose of
> removing it from each NUMA node but report the written value from
> memtrace/enable and memtrace/x/size in debugfs.

Why does it matter if the size that's removed is larger than the size
that was requested?

Is it about constraining the size of the trace? If so that seems like it
should be the job of the tracing tools, not the kernel.

cheers

^ permalink raw reply

* Re: [PATCH 2/3] Revert "lib: Revert use of fallthrough pseudo-keyword in lib/"
From: kernel test robot @ 2020-11-16 11:49 UTC (permalink / raw)
  To: Nick Desaulniers, Gustavo A . R . Silva, Nathan Chancellor,
	Miguel Ojeda, Michael Ellerman
  Cc: kbuild-all, Nick Desaulniers, linux-kernel, clang-built-linux,
	Paul Mackerras, linuxppc-dev
In-Reply-To: <20201116043532.4032932-3-ndesaulniers@google.com>

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

Hi Nick,

I love your patch! Perhaps something to improve:

[auto build test WARNING on powerpc/next]
[also build test WARNING on linus/master v5.10-rc4 next-20201116]
[cannot apply to pmladek/for-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Nick-Desaulniers/PPC-Fix-Wimplicit-fallthrough-for-clang/20201116-123803
base:   https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: x86_64-randconfig-m001-20201115 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

smatch warnings:
lib/zstd/huf_compress.c:559 HUF_compress1X_usingCTable() warn: inconsistent indenting

vim +559 lib/zstd/huf_compress.c

   529	
   530	#define HUF_FLUSHBITS_1(stream)                                            \
   531		if (sizeof((stream)->bitContainer) * 8 < HUF_TABLELOG_MAX * 2 + 7) \
   532		HUF_FLUSHBITS(stream)
   533	
   534	#define HUF_FLUSHBITS_2(stream)                                            \
   535		if (sizeof((stream)->bitContainer) * 8 < HUF_TABLELOG_MAX * 4 + 7) \
   536		HUF_FLUSHBITS(stream)
   537	
   538	size_t HUF_compress1X_usingCTable(void *dst, size_t dstSize, const void *src, size_t srcSize, const HUF_CElt *CTable)
   539	{
   540		const BYTE *ip = (const BYTE *)src;
   541		BYTE *const ostart = (BYTE *)dst;
   542		BYTE *const oend = ostart + dstSize;
   543		BYTE *op = ostart;
   544		size_t n;
   545		BIT_CStream_t bitC;
   546	
   547		/* init */
   548		if (dstSize < 8)
   549			return 0; /* not enough space to compress */
   550		{
   551			size_t const initErr = BIT_initCStream(&bitC, op, oend - op);
   552			if (HUF_isError(initErr))
   553				return 0;
   554		}
   555	
   556		n = srcSize & ~3; /* join to mod 4 */
   557		switch (srcSize & 3) {
   558		case 3: HUF_encodeSymbol(&bitC, ip[n + 2], CTable); HUF_FLUSHBITS_2(&bitC);
 > 559			fallthrough;
   560		case 2: HUF_encodeSymbol(&bitC, ip[n + 1], CTable); HUF_FLUSHBITS_1(&bitC);
   561			fallthrough;
   562		case 1: HUF_encodeSymbol(&bitC, ip[n + 0], CTable); HUF_FLUSHBITS(&bitC);
   563		case 0:
   564		default:;
   565		}
   566	
   567		for (; n > 0; n -= 4) { /* note : n&3==0 at this stage */
   568			HUF_encodeSymbol(&bitC, ip[n - 1], CTable);
   569			HUF_FLUSHBITS_1(&bitC);
   570			HUF_encodeSymbol(&bitC, ip[n - 2], CTable);
   571			HUF_FLUSHBITS_2(&bitC);
   572			HUF_encodeSymbol(&bitC, ip[n - 3], CTable);
   573			HUF_FLUSHBITS_1(&bitC);
   574			HUF_encodeSymbol(&bitC, ip[n - 4], CTable);
   575			HUF_FLUSHBITS(&bitC);
   576		}
   577	
   578		return BIT_closeCStream(&bitC);
   579	}
   580	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 31321 bytes --]

^ permalink raw reply

* Re: [PATCH 3/3] powerpc: fix -Wimplicit-fallthrough
From: Miguel Ojeda @ 2020-11-16 11:33 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: Gustavo A . R . Silva, linux-kernel, clang-built-linux,
	Paul Mackerras, Nathan Chancellor, linuxppc-dev
In-Reply-To: <20201116043532.4032932-4-ndesaulniers@google.com>

On Mon, Nov 16, 2020 at 5:35 AM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> The "fallthrough" pseudo-keyword was added as a portable way to denote
> intentional fallthrough. Clang will still warn on cases where there is a
> fallthrough to an immediate break. Add explicit breaks for those cases.
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/236
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>

It makes things clearer having a `break` added, so I like that warning.

Reviewed-by: Miguel Ojeda <ojeda@kernel.org>

Cheers,
Miguel

^ permalink raw reply

* Re: [PATCH 2/3] Revert "lib: Revert use of fallthrough pseudo-keyword in lib/"
From: Miguel Ojeda @ 2020-11-16 11:26 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: Gustavo A . R . Silva, linux-kernel, clang-built-linux,
	Paul Mackerras, Nathan Chancellor, linuxppc-dev
In-Reply-To: <20201116043532.4032932-3-ndesaulniers@google.com>

On Mon, Nov 16, 2020 at 5:35 AM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> This reverts commit 6a9dc5fd6170 ("lib: Revert use of fallthrough
> pseudo-keyword in lib/")
>
> Now that we can build arch/powerpc/boot/ free of -Wimplicit-fallthrough,
> re-enable these fixes for lib/.
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/236
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>

Looks fine on visual inspection:

Reviewed-by: Miguel Ojeda <ojeda@kernel.org>

Cheers,
Miguel

^ permalink raw reply

* Re: [PATCH 1/3] powerpc: boot: include compiler_attributes.h
From: Miguel Ojeda @ 2020-11-16 11:24 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: Gustavo A . R . Silva, linux-kernel, clang-built-linux,
	Paul Mackerras, Nathan Chancellor, linuxppc-dev
In-Reply-To: <20201116043532.4032932-2-ndesaulniers@google.com>

On Mon, Nov 16, 2020 at 5:35 AM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> It was also noted in 6a9dc5fd6170 that we could -D__KERNEL__ and
> -include compiler_types.h like the main kernel does, though testing that
> produces a whole sea of warnings to cleanup. This approach is minimally
> invasive.

I would add a comment noting this as a reminder -- it also helps to
entice a cleanup.

Acked-by: Miguel Ojeda <ojeda@kernel.org>

Cheers,
Miguel

^ permalink raw reply

* Re: [PATCH 2/3] Revert "lib: Revert use of fallthrough pseudo-keyword in lib/"
From: Miguel Ojeda @ 2020-11-16 11:18 UTC (permalink / raw)
  To: Gustavo A. R. Silva
  Cc: Nick Desaulniers, linux-kernel, clang-built-linux, Paul Mackerras,
	Nathan Chancellor, linuxppc-dev
In-Reply-To: <20201116062639.GB7265@embeddedor>

On Mon, Nov 16, 2020 at 7:26 AM Gustavo A. R. Silva
<gustavoars@kernel.org> wrote:
>
> Reviewed-by: Gustavo A. R. Silva <gustavoars@kernel.com>

.org :-)

Cheers,
Miguel

^ permalink raw reply

* Re: [PATCH 3/3] powerpc: fix -Wimplicit-fallthrough
From: Gustavo A. R. Silva @ 2020-11-16  6:28 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: clang-built-linux, linux-kernel, Miguel Ojeda, Paul Mackerras,
	Nathan Chancellor, linuxppc-dev
In-Reply-To: <20201116043532.4032932-4-ndesaulniers@google.com>

On Sun, Nov 15, 2020 at 08:35:32PM -0800, Nick Desaulniers wrote:
> The "fallthrough" pseudo-keyword was added as a portable way to denote
> intentional fallthrough. Clang will still warn on cases where there is a
> fallthrough to an immediate break. Add explicit breaks for those cases.
> 
> Link: https://github.com/ClangBuiltLinux/linux/issues/236
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>

Reviewed-by: Gustavo A. R. Silva <gustavoars@kernel.org>

Thanks
--
Gustavo

> ---
>  arch/powerpc/kernel/prom_init.c | 1 +
>  arch/powerpc/kernel/uprobes.c   | 1 +
>  arch/powerpc/perf/imc-pmu.c     | 1 +
>  3 files changed, 3 insertions(+)
> 
> diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
> index 38ae5933d917..e9d4eb6144e1 100644
> --- a/arch/powerpc/kernel/prom_init.c
> +++ b/arch/powerpc/kernel/prom_init.c
> @@ -355,6 +355,7 @@ static int __init prom_strtobool(const char *s, bool *res)
>  		default:
>  			break;
>  		}
> +		break;
>  	default:
>  		break;
>  	}
> diff --git a/arch/powerpc/kernel/uprobes.c b/arch/powerpc/kernel/uprobes.c
> index d200e7df7167..e8a63713e655 100644
> --- a/arch/powerpc/kernel/uprobes.c
> +++ b/arch/powerpc/kernel/uprobes.c
> @@ -141,6 +141,7 @@ int arch_uprobe_exception_notify(struct notifier_block *self,
>  	case DIE_SSTEP:
>  		if (uprobe_post_sstep_notifier(regs))
>  			return NOTIFY_STOP;
> +		break;
>  	default:
>  		break;
>  	}
> diff --git a/arch/powerpc/perf/imc-pmu.c b/arch/powerpc/perf/imc-pmu.c
> index 7b25548ec42b..e106909ff9c3 100644
> --- a/arch/powerpc/perf/imc-pmu.c
> +++ b/arch/powerpc/perf/imc-pmu.c
> @@ -1500,6 +1500,7 @@ static int update_pmu_ops(struct imc_pmu *pmu)
>  		pmu->pmu.stop = trace_imc_event_stop;
>  		pmu->pmu.read = trace_imc_event_read;
>  		pmu->attr_groups[IMC_FORMAT_ATTR] = &trace_imc_format_group;
> +		break;
>  	default:
>  		break;
>  	}
> -- 
> 2.29.2.299.gdc1121823c-goog
> 

^ permalink raw reply

* Re: [PATCH 2/3] Revert "lib: Revert use of fallthrough pseudo-keyword in lib/"
From: Gustavo A. R. Silva @ 2020-11-16  6:26 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: clang-built-linux, linux-kernel, Miguel Ojeda, Paul Mackerras,
	Nathan Chancellor, linuxppc-dev
In-Reply-To: <20201116043532.4032932-3-ndesaulniers@google.com>

On Sun, Nov 15, 2020 at 08:35:31PM -0800, Nick Desaulniers wrote:
> This reverts commit 6a9dc5fd6170 ("lib: Revert use of fallthrough
> pseudo-keyword in lib/")
> 
> Now that we can build arch/powerpc/boot/ free of -Wimplicit-fallthrough,
> re-enable these fixes for lib/.
> 
> Link: https://github.com/ClangBuiltLinux/linux/issues/236
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>

Reviewed-by: Gustavo A. R. Silva <gustavoars@kernel.com>

Thanks
--
Gustavo

> ---
>  lib/asn1_decoder.c      |  4 ++--
>  lib/assoc_array.c       |  2 +-
>  lib/bootconfig.c        |  4 ++--
>  lib/cmdline.c           | 10 +++++-----
>  lib/dim/net_dim.c       |  2 +-
>  lib/dim/rdma_dim.c      |  4 ++--
>  lib/glob.c              |  2 +-
>  lib/siphash.c           | 36 ++++++++++++++++++------------------
>  lib/ts_fsm.c            |  2 +-
>  lib/vsprintf.c          | 14 +++++++-------
>  lib/xz/xz_dec_lzma2.c   |  4 ++--
>  lib/xz/xz_dec_stream.c  | 16 ++++++++--------
>  lib/zstd/bitstream.h    | 10 +++++-----
>  lib/zstd/compress.c     |  2 +-
>  lib/zstd/decompress.c   | 12 ++++++------
>  lib/zstd/huf_compress.c |  4 ++--
>  16 files changed, 64 insertions(+), 64 deletions(-)
> 
> diff --git a/lib/asn1_decoder.c b/lib/asn1_decoder.c
> index 58f72b25f8e9..13da529e2e72 100644
> --- a/lib/asn1_decoder.c
> +++ b/lib/asn1_decoder.c
> @@ -381,7 +381,7 @@ int asn1_ber_decoder(const struct asn1_decoder *decoder,
>  	case ASN1_OP_END_SET_ACT:
>  		if (unlikely(!(flags & FLAG_MATCHED)))
>  			goto tag_mismatch;
> -		/* fall through */
> +		fallthrough;
>  
>  	case ASN1_OP_END_SEQ:
>  	case ASN1_OP_END_SET_OF:
> @@ -448,7 +448,7 @@ int asn1_ber_decoder(const struct asn1_decoder *decoder,
>  			pc += asn1_op_lengths[op];
>  			goto next_op;
>  		}
> -		/* fall through */
> +		fallthrough;
>  
>  	case ASN1_OP_ACT:
>  		ret = actions[machine[pc + 1]](context, hdr, tag, data + tdp, len);
> diff --git a/lib/assoc_array.c b/lib/assoc_array.c
> index 6f4bcf524554..04c98799c3ba 100644
> --- a/lib/assoc_array.c
> +++ b/lib/assoc_array.c
> @@ -1113,7 +1113,7 @@ struct assoc_array_edit *assoc_array_delete(struct assoc_array *array,
>  						index_key))
>  				goto found_leaf;
>  		}
> -		/* fall through */
> +		fallthrough;
>  	case assoc_array_walk_tree_empty:
>  	case assoc_array_walk_found_wrong_shortcut:
>  	default:
> diff --git a/lib/bootconfig.c b/lib/bootconfig.c
> index 649ed44f199c..9f8c70a98fcf 100644
> --- a/lib/bootconfig.c
> +++ b/lib/bootconfig.c
> @@ -827,7 +827,7 @@ int __init xbc_init(char *buf, const char **emsg, int *epos)
>  							q - 2);
>  				break;
>  			}
> -			/* fall through */
> +			fallthrough;
>  		case '=':
>  			ret = xbc_parse_kv(&p, q, c);
>  			break;
> @@ -836,7 +836,7 @@ int __init xbc_init(char *buf, const char **emsg, int *epos)
>  			break;
>  		case '#':
>  			q = skip_comment(q);
> -			/* fall through */
> +			fallthrough;
>  		case ';':
>  		case '\n':
>  			ret = xbc_parse_key(&p, q);
> diff --git a/lib/cmdline.c b/lib/cmdline.c
> index 9e186234edc0..46f2cb4ce6d1 100644
> --- a/lib/cmdline.c
> +++ b/lib/cmdline.c
> @@ -144,23 +144,23 @@ unsigned long long memparse(const char *ptr, char **retptr)
>  	case 'E':
>  	case 'e':
>  		ret <<= 10;
> -		/* fall through */
> +		fallthrough;
>  	case 'P':
>  	case 'p':
>  		ret <<= 10;
> -		/* fall through */
> +		fallthrough;
>  	case 'T':
>  	case 't':
>  		ret <<= 10;
> -		/* fall through */
> +		fallthrough;
>  	case 'G':
>  	case 'g':
>  		ret <<= 10;
> -		/* fall through */
> +		fallthrough;
>  	case 'M':
>  	case 'm':
>  		ret <<= 10;
> -		/* fall through */
> +		fallthrough;
>  	case 'K':
>  	case 'k':
>  		ret <<= 10;
> diff --git a/lib/dim/net_dim.c b/lib/dim/net_dim.c
> index a4db51c21266..06811d866775 100644
> --- a/lib/dim/net_dim.c
> +++ b/lib/dim/net_dim.c
> @@ -233,7 +233,7 @@ void net_dim(struct dim *dim, struct dim_sample end_sample)
>  			schedule_work(&dim->work);
>  			break;
>  		}
> -		/* fall through */
> +		fallthrough;
>  	case DIM_START_MEASURE:
>  		dim_update_sample(end_sample.event_ctr, end_sample.pkt_ctr,
>  				  end_sample.byte_ctr, &dim->start_sample);
> diff --git a/lib/dim/rdma_dim.c b/lib/dim/rdma_dim.c
> index f7e26c7b4749..15462d54758d 100644
> --- a/lib/dim/rdma_dim.c
> +++ b/lib/dim/rdma_dim.c
> @@ -59,7 +59,7 @@ static bool rdma_dim_decision(struct dim_stats *curr_stats, struct dim *dim)
>  			break;
>  		case DIM_STATS_WORSE:
>  			dim_turn(dim);
> -			/* fall through */
> +			fallthrough;
>  		case DIM_STATS_BETTER:
>  			step_res = rdma_dim_step(dim);
>  			if (step_res == DIM_ON_EDGE)
> @@ -94,7 +94,7 @@ void rdma_dim(struct dim *dim, u64 completions)
>  			schedule_work(&dim->work);
>  			break;
>  		}
> -		/* fall through */
> +		fallthrough;
>  	case DIM_START_MEASURE:
>  		dim->state = DIM_MEASURE_IN_PROGRESS;
>  		dim_update_sample_with_comps(curr_sample->event_ctr, 0, 0,
> diff --git a/lib/glob.c b/lib/glob.c
> index 52e3ed7e4a9b..85ecbda45cd8 100644
> --- a/lib/glob.c
> +++ b/lib/glob.c
> @@ -102,7 +102,7 @@ bool __pure glob_match(char const *pat, char const *str)
>  			break;
>  		case '\\':
>  			d = *pat++;
> -			/* fall through */
> +			fallthrough;
>  		default:	/* Literal character */
>  literal:
>  			if (c == d) {
> diff --git a/lib/siphash.c b/lib/siphash.c
> index c47bb6ff2149..a90112ee72a1 100644
> --- a/lib/siphash.c
> +++ b/lib/siphash.c
> @@ -68,11 +68,11 @@ u64 __siphash_aligned(const void *data, size_t len, const siphash_key_t *key)
>  						  bytemask_from_count(left)));
>  #else
>  	switch (left) {
> -	case 7: b |= ((u64)end[6]) << 48; /* fall through */
> -	case 6: b |= ((u64)end[5]) << 40; /* fall through */
> -	case 5: b |= ((u64)end[4]) << 32; /* fall through */
> +	case 7: b |= ((u64)end[6]) << 48; fallthrough;
> +	case 6: b |= ((u64)end[5]) << 40; fallthrough;
> +	case 5: b |= ((u64)end[4]) << 32; fallthrough;
>  	case 4: b |= le32_to_cpup(data); break;
> -	case 3: b |= ((u64)end[2]) << 16; /* fall through */
> +	case 3: b |= ((u64)end[2]) << 16; fallthrough;
>  	case 2: b |= le16_to_cpup(data); break;
>  	case 1: b |= end[0];
>  	}
> @@ -101,11 +101,11 @@ u64 __siphash_unaligned(const void *data, size_t len, const siphash_key_t *key)
>  						  bytemask_from_count(left)));
>  #else
>  	switch (left) {
> -	case 7: b |= ((u64)end[6]) << 48; /* fall through */
> -	case 6: b |= ((u64)end[5]) << 40; /* fall through */
> -	case 5: b |= ((u64)end[4]) << 32; /* fall through */
> +	case 7: b |= ((u64)end[6]) << 48; fallthrough;
> +	case 6: b |= ((u64)end[5]) << 40; fallthrough;
> +	case 5: b |= ((u64)end[4]) << 32; fallthrough;
>  	case 4: b |= get_unaligned_le32(end); break;
> -	case 3: b |= ((u64)end[2]) << 16; /* fall through */
> +	case 3: b |= ((u64)end[2]) << 16; fallthrough;
>  	case 2: b |= get_unaligned_le16(end); break;
>  	case 1: b |= end[0];
>  	}
> @@ -268,11 +268,11 @@ u32 __hsiphash_aligned(const void *data, size_t len, const hsiphash_key_t *key)
>  						  bytemask_from_count(left)));
>  #else
>  	switch (left) {
> -	case 7: b |= ((u64)end[6]) << 48; /* fall through */
> -	case 6: b |= ((u64)end[5]) << 40; /* fall through */
> -	case 5: b |= ((u64)end[4]) << 32; /* fall through */
> +	case 7: b |= ((u64)end[6]) << 48; fallthrough;
> +	case 6: b |= ((u64)end[5]) << 40; fallthrough;
> +	case 5: b |= ((u64)end[4]) << 32; fallthrough;
>  	case 4: b |= le32_to_cpup(data); break;
> -	case 3: b |= ((u64)end[2]) << 16; /* fall through */
> +	case 3: b |= ((u64)end[2]) << 16; fallthrough;
>  	case 2: b |= le16_to_cpup(data); break;
>  	case 1: b |= end[0];
>  	}
> @@ -301,11 +301,11 @@ u32 __hsiphash_unaligned(const void *data, size_t len,
>  						  bytemask_from_count(left)));
>  #else
>  	switch (left) {
> -	case 7: b |= ((u64)end[6]) << 48; /* fall through */
> -	case 6: b |= ((u64)end[5]) << 40; /* fall through */
> -	case 5: b |= ((u64)end[4]) << 32; /* fall through */
> +	case 7: b |= ((u64)end[6]) << 48; fallthrough;
> +	case 6: b |= ((u64)end[5]) << 40; fallthrough;
> +	case 5: b |= ((u64)end[4]) << 32; fallthrough;
>  	case 4: b |= get_unaligned_le32(end); break;
> -	case 3: b |= ((u64)end[2]) << 16; /* fall through */
> +	case 3: b |= ((u64)end[2]) << 16; fallthrough;
>  	case 2: b |= get_unaligned_le16(end); break;
>  	case 1: b |= end[0];
>  	}
> @@ -431,7 +431,7 @@ u32 __hsiphash_aligned(const void *data, size_t len, const hsiphash_key_t *key)
>  		v0 ^= m;
>  	}
>  	switch (left) {
> -	case 3: b |= ((u32)end[2]) << 16; /* fall through */
> +	case 3: b |= ((u32)end[2]) << 16; fallthrough;
>  	case 2: b |= le16_to_cpup(data); break;
>  	case 1: b |= end[0];
>  	}
> @@ -454,7 +454,7 @@ u32 __hsiphash_unaligned(const void *data, size_t len,
>  		v0 ^= m;
>  	}
>  	switch (left) {
> -	case 3: b |= ((u32)end[2]) << 16; /* fall through */
> +	case 3: b |= ((u32)end[2]) << 16; fallthrough;
>  	case 2: b |= get_unaligned_le16(end); break;
>  	case 1: b |= end[0];
>  	}
> diff --git a/lib/ts_fsm.c b/lib/ts_fsm.c
> index ab749ec10ab5..64fd9015ad80 100644
> --- a/lib/ts_fsm.c
> +++ b/lib/ts_fsm.c
> @@ -193,7 +193,7 @@ static unsigned int fsm_find(struct ts_config *conf, struct ts_state *state)
>  				TOKEN_MISMATCH();
>  
>  			block_idx++;
> -			/* fall through */
> +			fallthrough;
>  
>  		case TS_FSM_ANY:
>  			if (next == NULL)
> diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> index 14c9a6af1b23..d3c5c16f391c 100644
> --- a/lib/vsprintf.c
> +++ b/lib/vsprintf.c
> @@ -1265,7 +1265,7 @@ char *mac_address_string(char *buf, char *end, u8 *addr,
>  
>  	case 'R':
>  		reversed = true;
> -		/* fall through */
> +		fallthrough;
>  
>  	default:
>  		separator = ':';
> @@ -1682,7 +1682,7 @@ char *uuid_string(char *buf, char *end, const u8 *addr,
>  	switch (*(++fmt)) {
>  	case 'L':
>  		uc = true;
> -		/* fall through */
> +		fallthrough;
>  	case 'l':
>  		index = guid_index;
>  		break;
> @@ -2219,7 +2219,7 @@ char *pointer(const char *fmt, char *buf, char *end, void *ptr,
>  	case 'S':
>  	case 's':
>  		ptr = dereference_symbol_descriptor(ptr);
> -		/* fall through */
> +		fallthrough;
>  	case 'B':
>  		return symbol_string(buf, end, ptr, spec, fmt);
>  	case 'R':
> @@ -2450,7 +2450,7 @@ int format_decode(const char *fmt, struct printf_spec *spec)
>  
>  	case 'x':
>  		spec->flags |= SMALL;
> -		/* fall through */
> +		fallthrough;
>  
>  	case 'X':
>  		spec->base = 16;
> @@ -2468,7 +2468,7 @@ int format_decode(const char *fmt, struct printf_spec *spec)
>  		 * utility, treat it as any other invalid or
>  		 * unsupported format specifier.
>  		 */
> -		/* fall through */
> +		fallthrough;
>  
>  	default:
>  		WARN_ONCE(1, "Please remove unsupported %%%c in format string\n", *fmt);
> @@ -3411,10 +3411,10 @@ int vsscanf(const char *buf, const char *fmt, va_list args)
>  			break;
>  		case 'i':
>  			base = 0;
> -			/* fall through */
> +			fallthrough;
>  		case 'd':
>  			is_sign = true;
> -			/* fall through */
> +			fallthrough;
>  		case 'u':
>  			break;
>  		case '%':
> diff --git a/lib/xz/xz_dec_lzma2.c b/lib/xz/xz_dec_lzma2.c
> index 65a1aad8c223..ca2603abee08 100644
> --- a/lib/xz/xz_dec_lzma2.c
> +++ b/lib/xz/xz_dec_lzma2.c
> @@ -1043,7 +1043,7 @@ XZ_EXTERN enum xz_ret xz_dec_lzma2_run(struct xz_dec_lzma2 *s,
>  
>  			s->lzma2.sequence = SEQ_LZMA_PREPARE;
>  
> -			/* fall through */
> +			fallthrough;
>  
>  		case SEQ_LZMA_PREPARE:
>  			if (s->lzma2.compressed < RC_INIT_BYTES)
> @@ -1055,7 +1055,7 @@ XZ_EXTERN enum xz_ret xz_dec_lzma2_run(struct xz_dec_lzma2 *s,
>  			s->lzma2.compressed -= RC_INIT_BYTES;
>  			s->lzma2.sequence = SEQ_LZMA_RUN;
>  
> -			/* fall through */
> +			fallthrough;
>  
>  		case SEQ_LZMA_RUN:
>  			/*
> diff --git a/lib/xz/xz_dec_stream.c b/lib/xz/xz_dec_stream.c
> index 32ab2a08b7cb..fea86deaaa01 100644
> --- a/lib/xz/xz_dec_stream.c
> +++ b/lib/xz/xz_dec_stream.c
> @@ -583,7 +583,7 @@ static enum xz_ret dec_main(struct xz_dec *s, struct xz_buf *b)
>  			if (ret != XZ_OK)
>  				return ret;
>  
> -			/* fall through */
> +			fallthrough;
>  
>  		case SEQ_BLOCK_START:
>  			/* We need one byte of input to continue. */
> @@ -608,7 +608,7 @@ static enum xz_ret dec_main(struct xz_dec *s, struct xz_buf *b)
>  			s->temp.pos = 0;
>  			s->sequence = SEQ_BLOCK_HEADER;
>  
> -			/* fall through */
> +			fallthrough;
>  
>  		case SEQ_BLOCK_HEADER:
>  			if (!fill_temp(s, b))
> @@ -620,7 +620,7 @@ static enum xz_ret dec_main(struct xz_dec *s, struct xz_buf *b)
>  
>  			s->sequence = SEQ_BLOCK_UNCOMPRESS;
>  
> -			/* fall through */
> +			fallthrough;
>  
>  		case SEQ_BLOCK_UNCOMPRESS:
>  			ret = dec_block(s, b);
> @@ -629,7 +629,7 @@ static enum xz_ret dec_main(struct xz_dec *s, struct xz_buf *b)
>  
>  			s->sequence = SEQ_BLOCK_PADDING;
>  
> -			/* fall through */
> +			fallthrough;
>  
>  		case SEQ_BLOCK_PADDING:
>  			/*
> @@ -651,7 +651,7 @@ static enum xz_ret dec_main(struct xz_dec *s, struct xz_buf *b)
>  
>  			s->sequence = SEQ_BLOCK_CHECK;
>  
> -			/* fall through */
> +			fallthrough;
>  
>  		case SEQ_BLOCK_CHECK:
>  			if (s->check_type == XZ_CHECK_CRC32) {
> @@ -675,7 +675,7 @@ static enum xz_ret dec_main(struct xz_dec *s, struct xz_buf *b)
>  
>  			s->sequence = SEQ_INDEX_PADDING;
>  
> -			/* fall through */
> +			fallthrough;
>  
>  		case SEQ_INDEX_PADDING:
>  			while ((s->index.size + (b->in_pos - s->in_start))
> @@ -699,7 +699,7 @@ static enum xz_ret dec_main(struct xz_dec *s, struct xz_buf *b)
>  
>  			s->sequence = SEQ_INDEX_CRC32;
>  
> -			/* fall through */
> +			fallthrough;
>  
>  		case SEQ_INDEX_CRC32:
>  			ret = crc32_validate(s, b);
> @@ -709,7 +709,7 @@ static enum xz_ret dec_main(struct xz_dec *s, struct xz_buf *b)
>  			s->temp.size = STREAM_HEADER_SIZE;
>  			s->sequence = SEQ_STREAM_FOOTER;
>  
> -			/* fall through */
> +			fallthrough;
>  
>  		case SEQ_STREAM_FOOTER:
>  			if (!fill_temp(s, b))
> diff --git a/lib/zstd/bitstream.h b/lib/zstd/bitstream.h
> index 3a49784d5c61..7c65c66e41fd 100644
> --- a/lib/zstd/bitstream.h
> +++ b/lib/zstd/bitstream.h
> @@ -259,15 +259,15 @@ ZSTD_STATIC size_t BIT_initDStream(BIT_DStream_t *bitD, const void *srcBuffer, s
>  		bitD->bitContainer = *(const BYTE *)(bitD->start);
>  		switch (srcSize) {
>  		case 7: bitD->bitContainer += (size_t)(((const BYTE *)(srcBuffer))[6]) << (sizeof(bitD->bitContainer) * 8 - 16);
> -			/* fall through */
> +			fallthrough;
>  		case 6: bitD->bitContainer += (size_t)(((const BYTE *)(srcBuffer))[5]) << (sizeof(bitD->bitContainer) * 8 - 24);
> -			/* fall through */
> +			fallthrough;
>  		case 5: bitD->bitContainer += (size_t)(((const BYTE *)(srcBuffer))[4]) << (sizeof(bitD->bitContainer) * 8 - 32);
> -			/* fall through */
> +			fallthrough;
>  		case 4: bitD->bitContainer += (size_t)(((const BYTE *)(srcBuffer))[3]) << 24;
> -			/* fall through */
> +			fallthrough;
>  		case 3: bitD->bitContainer += (size_t)(((const BYTE *)(srcBuffer))[2]) << 16;
> -			/* fall through */
> +			fallthrough;
>  		case 2: bitD->bitContainer += (size_t)(((const BYTE *)(srcBuffer))[1]) << 8;
>  		default:;
>  		}
> diff --git a/lib/zstd/compress.c b/lib/zstd/compress.c
> index 5e0b67003e55..b080264ed3ad 100644
> --- a/lib/zstd/compress.c
> +++ b/lib/zstd/compress.c
> @@ -3182,7 +3182,7 @@ static size_t ZSTD_compressStream_generic(ZSTD_CStream *zcs, void *dst, size_t *
>  				zcs->outBuffFlushedSize = 0;
>  				zcs->stage = zcss_flush; /* pass-through to flush stage */
>  			}
> -			/* fall through */
> +			fallthrough;
>  
>  		case zcss_flush: {
>  			size_t const toFlush = zcs->outBuffContentSize - zcs->outBuffFlushedSize;
> diff --git a/lib/zstd/decompress.c b/lib/zstd/decompress.c
> index db6761ea4deb..66cd487a326a 100644
> --- a/lib/zstd/decompress.c
> +++ b/lib/zstd/decompress.c
> @@ -442,7 +442,7 @@ size_t ZSTD_decodeLiteralsBlock(ZSTD_DCtx *dctx, const void *src, size_t srcSize
>  		case set_repeat:
>  			if (dctx->litEntropy == 0)
>  				return ERROR(dictionary_corrupted);
> -			/* fall through */
> +			fallthrough;
>  		case set_compressed:
>  			if (srcSize < 5)
>  				return ERROR(corruption_detected); /* srcSize >= MIN_CBLOCK_SIZE == 3; here we need up to 5 for case 3 */
> @@ -1768,7 +1768,7 @@ size_t ZSTD_decompressContinue(ZSTD_DCtx *dctx, void *dst, size_t dstCapacity, c
>  			return 0;
>  		}
>  		dctx->expected = 0; /* not necessary to copy more */
> -		/* fall through */
> +		fallthrough;
>  
>  	case ZSTDds_decodeFrameHeader:
>  		memcpy(dctx->headerBuffer + ZSTD_frameHeaderSize_prefix, src, dctx->expected);
> @@ -2309,7 +2309,7 @@ size_t ZSTD_decompressStream(ZSTD_DStream *zds, ZSTD_outBuffer *output, ZSTD_inB
>  		switch (zds->stage) {
>  		case zdss_init:
>  			ZSTD_resetDStream(zds); /* transparent reset on starting decoding a new frame */
> -			/* fall through */
> +			fallthrough;
>  
>  		case zdss_loadHeader: {
>  			size_t const hSize = ZSTD_getFrameParams(&zds->fParams, zds->headerBuffer, zds->lhSize);
> @@ -2376,7 +2376,7 @@ size_t ZSTD_decompressStream(ZSTD_DStream *zds, ZSTD_outBuffer *output, ZSTD_inB
>  			}
>  			zds->stage = zdss_read;
>  		}
> -			/* fall through */
> +			fallthrough;
>  
>  		case zdss_read: {
>  			size_t const neededInSize = ZSTD_nextSrcSizeToDecompress(zds->dctx);
> @@ -2405,7 +2405,7 @@ size_t ZSTD_decompressStream(ZSTD_DStream *zds, ZSTD_outBuffer *output, ZSTD_inB
>  			zds->stage = zdss_load;
>  			/* pass-through */
>  		}
> -			/* fall through */
> +			fallthrough;
>  
>  		case zdss_load: {
>  			size_t const neededInSize = ZSTD_nextSrcSizeToDecompress(zds->dctx);
> @@ -2438,7 +2438,7 @@ size_t ZSTD_decompressStream(ZSTD_DStream *zds, ZSTD_outBuffer *output, ZSTD_inB
>  				/* pass-through */
>  			}
>  		}
> -			/* fall through */
> +			fallthrough;
>  
>  		case zdss_flush: {
>  			size_t const toFlushSize = zds->outEnd - zds->outStart;
> diff --git a/lib/zstd/huf_compress.c b/lib/zstd/huf_compress.c
> index e727812d12aa..08b4ae80aed4 100644
> --- a/lib/zstd/huf_compress.c
> +++ b/lib/zstd/huf_compress.c
> @@ -556,9 +556,9 @@ size_t HUF_compress1X_usingCTable(void *dst, size_t dstSize, const void *src, si
>  	n = srcSize & ~3; /* join to mod 4 */
>  	switch (srcSize & 3) {
>  	case 3: HUF_encodeSymbol(&bitC, ip[n + 2], CTable); HUF_FLUSHBITS_2(&bitC);
> -		/* fall through */
> +		fallthrough;
>  	case 2: HUF_encodeSymbol(&bitC, ip[n + 1], CTable); HUF_FLUSHBITS_1(&bitC);
> -		/* fall through */
> +		fallthrough;
>  	case 1: HUF_encodeSymbol(&bitC, ip[n + 0], CTable); HUF_FLUSHBITS(&bitC);
>  	case 0:
>  	default:;
> -- 
> 2.29.2.299.gdc1121823c-goog
> 

^ permalink raw reply

* Re: [PATCH 1/3] powerpc: boot: include compiler_attributes.h
From: Gustavo A. R. Silva @ 2020-11-16  6:25 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: clang-built-linux, linux-kernel, Miguel Ojeda, Paul Mackerras,
	Nathan Chancellor, linuxppc-dev
In-Reply-To: <20201116043532.4032932-2-ndesaulniers@google.com>

On Sun, Nov 15, 2020 at 08:35:30PM -0800, Nick Desaulniers wrote:
> The kernel uses `-include` to include include/linux/compiler_types.h
> into all translation units (see scripts/Makefile.lib), which #includes
> compiler_attributes.h.
> 
> arch/powerpc/boot/ uses different compiler flags from the rest of the
> kernel. As such, it doesn't contain the definitions from these headers,
> and redefines a few that it needs.
> 
> For the purpose of enabling -Wimplicit-fallthrough for ppc, include
> compiler_types.h via `-include`.
> 
> Link: https://github.com/ClangBuiltLinux/linux/issues/236
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>

Acked-by: Gustavo A. R. Silva <gustavoars@kernel.org>

Thanks, Nick.
--
Gustavo

> ---
> We could just `#include "include/linux/compiler_types.h"` in the few .c
> sources used from lib/ (there are proper header guards in
> compiler_types.h).
> 
> It was also noted in 6a9dc5fd6170 that we could -D__KERNEL__ and
> -include compiler_types.h like the main kernel does, though testing that
> produces a whole sea of warnings to cleanup. This approach is minimally
> invasive.
> 
>  arch/powerpc/boot/Makefile     | 1 +
>  arch/powerpc/boot/decompress.c | 1 -
>  2 files changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/boot/Makefile b/arch/powerpc/boot/Makefile
> index f8ce6d2dde7b..1659963a8f1d 100644
> --- a/arch/powerpc/boot/Makefile
> +++ b/arch/powerpc/boot/Makefile
> @@ -31,6 +31,7 @@ endif
>  BOOTCFLAGS    := -Wall -Wundef -Wstrict-prototypes -Wno-trigraphs \
>  		 -fno-strict-aliasing -O2 -msoft-float -mno-altivec -mno-vsx \
>  		 -pipe -fomit-frame-pointer -fno-builtin -fPIC -nostdinc \
> +		 -include $(srctree)/include/linux/compiler_attributes.h \
>  		 $(LINUXINCLUDE)
>  
>  ifdef CONFIG_PPC64_BOOT_WRAPPER
> diff --git a/arch/powerpc/boot/decompress.c b/arch/powerpc/boot/decompress.c
> index 8bf39ef7d2df..6098b879ac97 100644
> --- a/arch/powerpc/boot/decompress.c
> +++ b/arch/powerpc/boot/decompress.c
> @@ -21,7 +21,6 @@
>  
>  #define STATIC static
>  #define INIT
> -#define __always_inline inline
>  
>  /*
>   * The build process will copy the required zlib source files and headers
> -- 
> 2.29.2.299.gdc1121823c-goog
> 

^ permalink raw reply

* Re: [PATCH kernel] vfio_pci_nvlink2: Do not attempt NPU2 setup on old P8's NPU
From: Michael Ellerman @ 2020-11-16  6:20 UTC (permalink / raw)
  To: Alexey Kardashevskiy, Andrew Donnellan, linuxppc-dev
  Cc: Leonardo Augusto Guimaraes Garcia, Alex Williamson, kvm,
	David Gibson
In-Reply-To: <1f2be6b0-d53a-aa58-9c4f-d55a6a5b1c79@ozlabs.ru>

Alexey Kardashevskiy <aik@ozlabs.ru> writes:
> On 13/11/2020 16:30, Andrew Donnellan wrote:
>> On 13/11/20 4:06 pm, Alexey Kardashevskiy wrote:
>>> We execute certain NPU2 setup code (such as mapping an LPID to a device
>>> in NPU2) unconditionally if an Nvlink bridge is detected. However this
>>> cannot succeed on P8+ machines as the init helpers return an error other
>>> than ENODEV which means the device is there is and setup failed so
>>> vfio_pci_enable() fails and pass through is not possible.
>>>
>>> This changes the two NPU2 related init helpers to return -ENODEV if
>>> there is no "memory-region" device tree property as this is
>>> the distinction between NPU and NPU2.
>>>
>>> Fixes: 7f92891778df ("vfio_pci: Add NVIDIA GV100GL [Tesla V100 SXM2] 
>>> subdriver")
>>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>> 
>> Should this be Cc: stable?
>
> This depends on whether P8+ + NVLink was ever a  product (hi Leonardo) 
> and had actual customers who still rely on upstream kernels to work as 
> after many years only the last week I heard form some Redhat test 
> engineer that it does not work. May be cc: stable...

I don't think it really matters if it was a product or not. Upstream is
never a product anyway.

If the fix is simple and unlikely to introduce a regression, and would
potentially save someone having to debug the problem again, then it
should get backported to stable.

You should also clarify what you mean by "P8+", it won't be clear to
most readers if you mean "Power 8 and/or later" or specifically Naples /
Power8 NVL.

cheers

^ permalink raw reply

* Re: Error: invalid switch -me200
From: Michael Ellerman @ 2020-11-16  6:06 UTC (permalink / raw)
  To: Christophe Leroy, Segher Boessenkool, Nick Desaulniers,
	mihai.caraman
  Cc: Arnd Bergmann, kbuild-all, Brian Cain,
	Fāng-ruì Sòng, linuxppc-dev, Masahiro Yamada, LKML,
	clang-built-linux, Nathan Chancellor, Linus Torvalds,
	kernel test robot
In-Reply-To: <14e9ce2b-1a83-5353-44c7-b0709796c70e@csgroup.eu>

Christophe Leroy <christophe.leroy@csgroup.eu> writes:
> Le 14/11/2020 à 01:20, Segher Boessenkool a écrit :
>> On Fri, Nov 13, 2020 at 12:14:18PM -0800, Nick Desaulniers wrote:
>>>>>> Error: invalid switch -me200
>>>>>> Error: unrecognized option -me200
>>>>>
>>>>> 251 cpu-as-$(CONFIG_E200)   += -Wa,-me200
>>>>>
>>>>> Are those all broken configs, or is Kconfig messed up such that
>>>>> randconfig can select these when it should not?
>>>>
>>>> Hmmm, looks like this flag does not exist in mainline binutils? There is
>>>> a thread in 2010 about this that Segher commented on:
>>>>
>>>> https://lore.kernel.org/linuxppc-dev/9859E645-954D-4D07-8003-FFCD2391AB6E@kernel.crashing.org/
>>>>
>>>> Guess this config should be eliminated?
>> 
>> The help text for this config options says that e200 is used in 55xx,
>> and there *is* an -me5500 GAS flag (which probably does this same
>> thing, too).  But is any of this tested, or useful, or wanted?
>> 
>> Maybe Christophe knows, cc:ed.
>> 
>
> I don't have much clue on this.

Me either.

> But I see on wikipedia that e5500 is a 64 bits powerpc (https://en.wikipedia.org/wiki/PowerPC_e5500)
>
> What I see is that NXP seems to provide a GCC version that includes aditionnal cpu (e200z0 e200z2 
> e200z3 e200z4 e200z6 e200z7):
>
> valid arguments to '-mcpu=' are: 401 403 405 405fp 440 440fp 464 464fp 476 476fp 505 601 602 603 
> 603e 604 604e 620 630 740 7400 7450 750 801 821 823 8540 8548 860 970 G3 G4 G5 a2 cell e200z0 e200z2 
> e200z3 e200z4 e200z6 e200z7 e300c2 e300c3 e500mc e500mc64 e5500 e6500 ec603e native power3 power4 
> power5 power5+ power6 power6x power7 power8 powerpc powerpc64 powerpc64le rs64 titan "
>
> https://community.nxp.com/t5/MPC5xxx/GCC-generating-not-implemented-instructions/m-p/845049
>
> Apparently based on binutils 2.28
>
> https://www.nxp.com/docs/en/release-note/S32DS-POWER-v1-2-RN.pdf
>
> But that's not exactly -me200 though.
>
> Now, I can't see any defconfig that selects CONFIG_E200, so is that worth keeping it in the kernel 
> at all ?

There was a commit in 2014 that suggests it worked at least to some
extent then:

  3477e71d5319 ("powerpc/booke: Restrict SPE exception handlers to e200/e500 cores")


Presumably there was a non-upstream toolchain where it was supported?

AFAICS the kernel builds OK with just the cpu-as modification removed:

diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
index a4d56f0a41d9..16b8336f91dd 100644
--- a/arch/powerpc/Makefile
+++ b/arch/powerpc/Makefile
@@ -248,7 +248,6 @@ KBUILD_CFLAGS               += $(call cc-option,-mno-string)
 cpu-as-$(CONFIG_40x)           += -Wa,-m405
 cpu-as-$(CONFIG_44x)           += -Wa,-m440
 cpu-as-$(CONFIG_ALTIVEC)       += $(call as-option,-Wa$(comma)-maltivec)
-cpu-as-$(CONFIG_E200)          += -Wa,-me200
 cpu-as-$(CONFIG_E500)          += -Wa,-me500

 # When using '-many -mpower4' gas will first try and find a matching power4


So that seems like the obvious fix for now.

I tried booting the resulting kernel in qemu, but I get:

  $ qemu-system-ppc -M none -cpu e200 -kernel build\~/vmlinux
  Error: Trying to register SPR 574 (23e) twice !


Which is not related AFAIK and indicates the qemu support is broken.

Unless we hear from someone that they're using mainline on an e200 then
it seems like it's a candidate for removal.

cheers

^ permalink raw reply related

* [PATCH 3/3] powerpc: fix -Wimplicit-fallthrough
From: Nick Desaulniers @ 2020-11-16  4:35 UTC (permalink / raw)
  To: Gustavo A . R . Silva, Nathan Chancellor, Miguel Ojeda,
	Michael Ellerman
  Cc: Nick Desaulniers, linux-kernel, clang-built-linux, Paul Mackerras,
	linuxppc-dev
In-Reply-To: <20201116043532.4032932-1-ndesaulniers@google.com>

The "fallthrough" pseudo-keyword was added as a portable way to denote
intentional fallthrough. Clang will still warn on cases where there is a
fallthrough to an immediate break. Add explicit breaks for those cases.

Link: https://github.com/ClangBuiltLinux/linux/issues/236
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
---
 arch/powerpc/kernel/prom_init.c | 1 +
 arch/powerpc/kernel/uprobes.c   | 1 +
 arch/powerpc/perf/imc-pmu.c     | 1 +
 3 files changed, 3 insertions(+)

diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
index 38ae5933d917..e9d4eb6144e1 100644
--- a/arch/powerpc/kernel/prom_init.c
+++ b/arch/powerpc/kernel/prom_init.c
@@ -355,6 +355,7 @@ static int __init prom_strtobool(const char *s, bool *res)
 		default:
 			break;
 		}
+		break;
 	default:
 		break;
 	}
diff --git a/arch/powerpc/kernel/uprobes.c b/arch/powerpc/kernel/uprobes.c
index d200e7df7167..e8a63713e655 100644
--- a/arch/powerpc/kernel/uprobes.c
+++ b/arch/powerpc/kernel/uprobes.c
@@ -141,6 +141,7 @@ int arch_uprobe_exception_notify(struct notifier_block *self,
 	case DIE_SSTEP:
 		if (uprobe_post_sstep_notifier(regs))
 			return NOTIFY_STOP;
+		break;
 	default:
 		break;
 	}
diff --git a/arch/powerpc/perf/imc-pmu.c b/arch/powerpc/perf/imc-pmu.c
index 7b25548ec42b..e106909ff9c3 100644
--- a/arch/powerpc/perf/imc-pmu.c
+++ b/arch/powerpc/perf/imc-pmu.c
@@ -1500,6 +1500,7 @@ static int update_pmu_ops(struct imc_pmu *pmu)
 		pmu->pmu.stop = trace_imc_event_stop;
 		pmu->pmu.read = trace_imc_event_read;
 		pmu->attr_groups[IMC_FORMAT_ATTR] = &trace_imc_format_group;
+		break;
 	default:
 		break;
 	}
-- 
2.29.2.299.gdc1121823c-goog


^ permalink raw reply related

* [PATCH 1/3] powerpc: boot: include compiler_attributes.h
From: Nick Desaulniers @ 2020-11-16  4:35 UTC (permalink / raw)
  To: Gustavo A . R . Silva, Nathan Chancellor, Miguel Ojeda,
	Michael Ellerman
  Cc: Nick Desaulniers, linux-kernel, clang-built-linux, Paul Mackerras,
	linuxppc-dev
In-Reply-To: <20201116043532.4032932-1-ndesaulniers@google.com>

The kernel uses `-include` to include include/linux/compiler_types.h
into all translation units (see scripts/Makefile.lib), which #includes
compiler_attributes.h.

arch/powerpc/boot/ uses different compiler flags from the rest of the
kernel. As such, it doesn't contain the definitions from these headers,
and redefines a few that it needs.

For the purpose of enabling -Wimplicit-fallthrough for ppc, include
compiler_types.h via `-include`.

Link: https://github.com/ClangBuiltLinux/linux/issues/236
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
---
We could just `#include "include/linux/compiler_types.h"` in the few .c
sources used from lib/ (there are proper header guards in
compiler_types.h).

It was also noted in 6a9dc5fd6170 that we could -D__KERNEL__ and
-include compiler_types.h like the main kernel does, though testing that
produces a whole sea of warnings to cleanup. This approach is minimally
invasive.

 arch/powerpc/boot/Makefile     | 1 +
 arch/powerpc/boot/decompress.c | 1 -
 2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/boot/Makefile b/arch/powerpc/boot/Makefile
index f8ce6d2dde7b..1659963a8f1d 100644
--- a/arch/powerpc/boot/Makefile
+++ b/arch/powerpc/boot/Makefile
@@ -31,6 +31,7 @@ endif
 BOOTCFLAGS    := -Wall -Wundef -Wstrict-prototypes -Wno-trigraphs \
 		 -fno-strict-aliasing -O2 -msoft-float -mno-altivec -mno-vsx \
 		 -pipe -fomit-frame-pointer -fno-builtin -fPIC -nostdinc \
+		 -include $(srctree)/include/linux/compiler_attributes.h \
 		 $(LINUXINCLUDE)
 
 ifdef CONFIG_PPC64_BOOT_WRAPPER
diff --git a/arch/powerpc/boot/decompress.c b/arch/powerpc/boot/decompress.c
index 8bf39ef7d2df..6098b879ac97 100644
--- a/arch/powerpc/boot/decompress.c
+++ b/arch/powerpc/boot/decompress.c
@@ -21,7 +21,6 @@
 
 #define STATIC static
 #define INIT
-#define __always_inline inline
 
 /*
  * The build process will copy the required zlib source files and headers
-- 
2.29.2.299.gdc1121823c-goog


^ permalink raw reply related

* [PATCH 2/3] Revert "lib: Revert use of fallthrough pseudo-keyword in lib/"
From: Nick Desaulniers @ 2020-11-16  4:35 UTC (permalink / raw)
  To: Gustavo A . R . Silva, Nathan Chancellor, Miguel Ojeda,
	Michael Ellerman
  Cc: Nick Desaulniers, linux-kernel, clang-built-linux, Paul Mackerras,
	linuxppc-dev
In-Reply-To: <20201116043532.4032932-1-ndesaulniers@google.com>

This reverts commit 6a9dc5fd6170 ("lib: Revert use of fallthrough
pseudo-keyword in lib/")

Now that we can build arch/powerpc/boot/ free of -Wimplicit-fallthrough,
re-enable these fixes for lib/.

Link: https://github.com/ClangBuiltLinux/linux/issues/236
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
---
 lib/asn1_decoder.c      |  4 ++--
 lib/assoc_array.c       |  2 +-
 lib/bootconfig.c        |  4 ++--
 lib/cmdline.c           | 10 +++++-----
 lib/dim/net_dim.c       |  2 +-
 lib/dim/rdma_dim.c      |  4 ++--
 lib/glob.c              |  2 +-
 lib/siphash.c           | 36 ++++++++++++++++++------------------
 lib/ts_fsm.c            |  2 +-
 lib/vsprintf.c          | 14 +++++++-------
 lib/xz/xz_dec_lzma2.c   |  4 ++--
 lib/xz/xz_dec_stream.c  | 16 ++++++++--------
 lib/zstd/bitstream.h    | 10 +++++-----
 lib/zstd/compress.c     |  2 +-
 lib/zstd/decompress.c   | 12 ++++++------
 lib/zstd/huf_compress.c |  4 ++--
 16 files changed, 64 insertions(+), 64 deletions(-)

diff --git a/lib/asn1_decoder.c b/lib/asn1_decoder.c
index 58f72b25f8e9..13da529e2e72 100644
--- a/lib/asn1_decoder.c
+++ b/lib/asn1_decoder.c
@@ -381,7 +381,7 @@ int asn1_ber_decoder(const struct asn1_decoder *decoder,
 	case ASN1_OP_END_SET_ACT:
 		if (unlikely(!(flags & FLAG_MATCHED)))
 			goto tag_mismatch;
-		/* fall through */
+		fallthrough;
 
 	case ASN1_OP_END_SEQ:
 	case ASN1_OP_END_SET_OF:
@@ -448,7 +448,7 @@ int asn1_ber_decoder(const struct asn1_decoder *decoder,
 			pc += asn1_op_lengths[op];
 			goto next_op;
 		}
-		/* fall through */
+		fallthrough;
 
 	case ASN1_OP_ACT:
 		ret = actions[machine[pc + 1]](context, hdr, tag, data + tdp, len);
diff --git a/lib/assoc_array.c b/lib/assoc_array.c
index 6f4bcf524554..04c98799c3ba 100644
--- a/lib/assoc_array.c
+++ b/lib/assoc_array.c
@@ -1113,7 +1113,7 @@ struct assoc_array_edit *assoc_array_delete(struct assoc_array *array,
 						index_key))
 				goto found_leaf;
 		}
-		/* fall through */
+		fallthrough;
 	case assoc_array_walk_tree_empty:
 	case assoc_array_walk_found_wrong_shortcut:
 	default:
diff --git a/lib/bootconfig.c b/lib/bootconfig.c
index 649ed44f199c..9f8c70a98fcf 100644
--- a/lib/bootconfig.c
+++ b/lib/bootconfig.c
@@ -827,7 +827,7 @@ int __init xbc_init(char *buf, const char **emsg, int *epos)
 							q - 2);
 				break;
 			}
-			/* fall through */
+			fallthrough;
 		case '=':
 			ret = xbc_parse_kv(&p, q, c);
 			break;
@@ -836,7 +836,7 @@ int __init xbc_init(char *buf, const char **emsg, int *epos)
 			break;
 		case '#':
 			q = skip_comment(q);
-			/* fall through */
+			fallthrough;
 		case ';':
 		case '\n':
 			ret = xbc_parse_key(&p, q);
diff --git a/lib/cmdline.c b/lib/cmdline.c
index 9e186234edc0..46f2cb4ce6d1 100644
--- a/lib/cmdline.c
+++ b/lib/cmdline.c
@@ -144,23 +144,23 @@ unsigned long long memparse(const char *ptr, char **retptr)
 	case 'E':
 	case 'e':
 		ret <<= 10;
-		/* fall through */
+		fallthrough;
 	case 'P':
 	case 'p':
 		ret <<= 10;
-		/* fall through */
+		fallthrough;
 	case 'T':
 	case 't':
 		ret <<= 10;
-		/* fall through */
+		fallthrough;
 	case 'G':
 	case 'g':
 		ret <<= 10;
-		/* fall through */
+		fallthrough;
 	case 'M':
 	case 'm':
 		ret <<= 10;
-		/* fall through */
+		fallthrough;
 	case 'K':
 	case 'k':
 		ret <<= 10;
diff --git a/lib/dim/net_dim.c b/lib/dim/net_dim.c
index a4db51c21266..06811d866775 100644
--- a/lib/dim/net_dim.c
+++ b/lib/dim/net_dim.c
@@ -233,7 +233,7 @@ void net_dim(struct dim *dim, struct dim_sample end_sample)
 			schedule_work(&dim->work);
 			break;
 		}
-		/* fall through */
+		fallthrough;
 	case DIM_START_MEASURE:
 		dim_update_sample(end_sample.event_ctr, end_sample.pkt_ctr,
 				  end_sample.byte_ctr, &dim->start_sample);
diff --git a/lib/dim/rdma_dim.c b/lib/dim/rdma_dim.c
index f7e26c7b4749..15462d54758d 100644
--- a/lib/dim/rdma_dim.c
+++ b/lib/dim/rdma_dim.c
@@ -59,7 +59,7 @@ static bool rdma_dim_decision(struct dim_stats *curr_stats, struct dim *dim)
 			break;
 		case DIM_STATS_WORSE:
 			dim_turn(dim);
-			/* fall through */
+			fallthrough;
 		case DIM_STATS_BETTER:
 			step_res = rdma_dim_step(dim);
 			if (step_res == DIM_ON_EDGE)
@@ -94,7 +94,7 @@ void rdma_dim(struct dim *dim, u64 completions)
 			schedule_work(&dim->work);
 			break;
 		}
-		/* fall through */
+		fallthrough;
 	case DIM_START_MEASURE:
 		dim->state = DIM_MEASURE_IN_PROGRESS;
 		dim_update_sample_with_comps(curr_sample->event_ctr, 0, 0,
diff --git a/lib/glob.c b/lib/glob.c
index 52e3ed7e4a9b..85ecbda45cd8 100644
--- a/lib/glob.c
+++ b/lib/glob.c
@@ -102,7 +102,7 @@ bool __pure glob_match(char const *pat, char const *str)
 			break;
 		case '\\':
 			d = *pat++;
-			/* fall through */
+			fallthrough;
 		default:	/* Literal character */
 literal:
 			if (c == d) {
diff --git a/lib/siphash.c b/lib/siphash.c
index c47bb6ff2149..a90112ee72a1 100644
--- a/lib/siphash.c
+++ b/lib/siphash.c
@@ -68,11 +68,11 @@ u64 __siphash_aligned(const void *data, size_t len, const siphash_key_t *key)
 						  bytemask_from_count(left)));
 #else
 	switch (left) {
-	case 7: b |= ((u64)end[6]) << 48; /* fall through */
-	case 6: b |= ((u64)end[5]) << 40; /* fall through */
-	case 5: b |= ((u64)end[4]) << 32; /* fall through */
+	case 7: b |= ((u64)end[6]) << 48; fallthrough;
+	case 6: b |= ((u64)end[5]) << 40; fallthrough;
+	case 5: b |= ((u64)end[4]) << 32; fallthrough;
 	case 4: b |= le32_to_cpup(data); break;
-	case 3: b |= ((u64)end[2]) << 16; /* fall through */
+	case 3: b |= ((u64)end[2]) << 16; fallthrough;
 	case 2: b |= le16_to_cpup(data); break;
 	case 1: b |= end[0];
 	}
@@ -101,11 +101,11 @@ u64 __siphash_unaligned(const void *data, size_t len, const siphash_key_t *key)
 						  bytemask_from_count(left)));
 #else
 	switch (left) {
-	case 7: b |= ((u64)end[6]) << 48; /* fall through */
-	case 6: b |= ((u64)end[5]) << 40; /* fall through */
-	case 5: b |= ((u64)end[4]) << 32; /* fall through */
+	case 7: b |= ((u64)end[6]) << 48; fallthrough;
+	case 6: b |= ((u64)end[5]) << 40; fallthrough;
+	case 5: b |= ((u64)end[4]) << 32; fallthrough;
 	case 4: b |= get_unaligned_le32(end); break;
-	case 3: b |= ((u64)end[2]) << 16; /* fall through */
+	case 3: b |= ((u64)end[2]) << 16; fallthrough;
 	case 2: b |= get_unaligned_le16(end); break;
 	case 1: b |= end[0];
 	}
@@ -268,11 +268,11 @@ u32 __hsiphash_aligned(const void *data, size_t len, const hsiphash_key_t *key)
 						  bytemask_from_count(left)));
 #else
 	switch (left) {
-	case 7: b |= ((u64)end[6]) << 48; /* fall through */
-	case 6: b |= ((u64)end[5]) << 40; /* fall through */
-	case 5: b |= ((u64)end[4]) << 32; /* fall through */
+	case 7: b |= ((u64)end[6]) << 48; fallthrough;
+	case 6: b |= ((u64)end[5]) << 40; fallthrough;
+	case 5: b |= ((u64)end[4]) << 32; fallthrough;
 	case 4: b |= le32_to_cpup(data); break;
-	case 3: b |= ((u64)end[2]) << 16; /* fall through */
+	case 3: b |= ((u64)end[2]) << 16; fallthrough;
 	case 2: b |= le16_to_cpup(data); break;
 	case 1: b |= end[0];
 	}
@@ -301,11 +301,11 @@ u32 __hsiphash_unaligned(const void *data, size_t len,
 						  bytemask_from_count(left)));
 #else
 	switch (left) {
-	case 7: b |= ((u64)end[6]) << 48; /* fall through */
-	case 6: b |= ((u64)end[5]) << 40; /* fall through */
-	case 5: b |= ((u64)end[4]) << 32; /* fall through */
+	case 7: b |= ((u64)end[6]) << 48; fallthrough;
+	case 6: b |= ((u64)end[5]) << 40; fallthrough;
+	case 5: b |= ((u64)end[4]) << 32; fallthrough;
 	case 4: b |= get_unaligned_le32(end); break;
-	case 3: b |= ((u64)end[2]) << 16; /* fall through */
+	case 3: b |= ((u64)end[2]) << 16; fallthrough;
 	case 2: b |= get_unaligned_le16(end); break;
 	case 1: b |= end[0];
 	}
@@ -431,7 +431,7 @@ u32 __hsiphash_aligned(const void *data, size_t len, const hsiphash_key_t *key)
 		v0 ^= m;
 	}
 	switch (left) {
-	case 3: b |= ((u32)end[2]) << 16; /* fall through */
+	case 3: b |= ((u32)end[2]) << 16; fallthrough;
 	case 2: b |= le16_to_cpup(data); break;
 	case 1: b |= end[0];
 	}
@@ -454,7 +454,7 @@ u32 __hsiphash_unaligned(const void *data, size_t len,
 		v0 ^= m;
 	}
 	switch (left) {
-	case 3: b |= ((u32)end[2]) << 16; /* fall through */
+	case 3: b |= ((u32)end[2]) << 16; fallthrough;
 	case 2: b |= get_unaligned_le16(end); break;
 	case 1: b |= end[0];
 	}
diff --git a/lib/ts_fsm.c b/lib/ts_fsm.c
index ab749ec10ab5..64fd9015ad80 100644
--- a/lib/ts_fsm.c
+++ b/lib/ts_fsm.c
@@ -193,7 +193,7 @@ static unsigned int fsm_find(struct ts_config *conf, struct ts_state *state)
 				TOKEN_MISMATCH();
 
 			block_idx++;
-			/* fall through */
+			fallthrough;
 
 		case TS_FSM_ANY:
 			if (next == NULL)
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 14c9a6af1b23..d3c5c16f391c 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -1265,7 +1265,7 @@ char *mac_address_string(char *buf, char *end, u8 *addr,
 
 	case 'R':
 		reversed = true;
-		/* fall through */
+		fallthrough;
 
 	default:
 		separator = ':';
@@ -1682,7 +1682,7 @@ char *uuid_string(char *buf, char *end, const u8 *addr,
 	switch (*(++fmt)) {
 	case 'L':
 		uc = true;
-		/* fall through */
+		fallthrough;
 	case 'l':
 		index = guid_index;
 		break;
@@ -2219,7 +2219,7 @@ char *pointer(const char *fmt, char *buf, char *end, void *ptr,
 	case 'S':
 	case 's':
 		ptr = dereference_symbol_descriptor(ptr);
-		/* fall through */
+		fallthrough;
 	case 'B':
 		return symbol_string(buf, end, ptr, spec, fmt);
 	case 'R':
@@ -2450,7 +2450,7 @@ int format_decode(const char *fmt, struct printf_spec *spec)
 
 	case 'x':
 		spec->flags |= SMALL;
-		/* fall through */
+		fallthrough;
 
 	case 'X':
 		spec->base = 16;
@@ -2468,7 +2468,7 @@ int format_decode(const char *fmt, struct printf_spec *spec)
 		 * utility, treat it as any other invalid or
 		 * unsupported format specifier.
 		 */
-		/* fall through */
+		fallthrough;
 
 	default:
 		WARN_ONCE(1, "Please remove unsupported %%%c in format string\n", *fmt);
@@ -3411,10 +3411,10 @@ int vsscanf(const char *buf, const char *fmt, va_list args)
 			break;
 		case 'i':
 			base = 0;
-			/* fall through */
+			fallthrough;
 		case 'd':
 			is_sign = true;
-			/* fall through */
+			fallthrough;
 		case 'u':
 			break;
 		case '%':
diff --git a/lib/xz/xz_dec_lzma2.c b/lib/xz/xz_dec_lzma2.c
index 65a1aad8c223..ca2603abee08 100644
--- a/lib/xz/xz_dec_lzma2.c
+++ b/lib/xz/xz_dec_lzma2.c
@@ -1043,7 +1043,7 @@ XZ_EXTERN enum xz_ret xz_dec_lzma2_run(struct xz_dec_lzma2 *s,
 
 			s->lzma2.sequence = SEQ_LZMA_PREPARE;
 
-			/* fall through */
+			fallthrough;
 
 		case SEQ_LZMA_PREPARE:
 			if (s->lzma2.compressed < RC_INIT_BYTES)
@@ -1055,7 +1055,7 @@ XZ_EXTERN enum xz_ret xz_dec_lzma2_run(struct xz_dec_lzma2 *s,
 			s->lzma2.compressed -= RC_INIT_BYTES;
 			s->lzma2.sequence = SEQ_LZMA_RUN;
 
-			/* fall through */
+			fallthrough;
 
 		case SEQ_LZMA_RUN:
 			/*
diff --git a/lib/xz/xz_dec_stream.c b/lib/xz/xz_dec_stream.c
index 32ab2a08b7cb..fea86deaaa01 100644
--- a/lib/xz/xz_dec_stream.c
+++ b/lib/xz/xz_dec_stream.c
@@ -583,7 +583,7 @@ static enum xz_ret dec_main(struct xz_dec *s, struct xz_buf *b)
 			if (ret != XZ_OK)
 				return ret;
 
-			/* fall through */
+			fallthrough;
 
 		case SEQ_BLOCK_START:
 			/* We need one byte of input to continue. */
@@ -608,7 +608,7 @@ static enum xz_ret dec_main(struct xz_dec *s, struct xz_buf *b)
 			s->temp.pos = 0;
 			s->sequence = SEQ_BLOCK_HEADER;
 
-			/* fall through */
+			fallthrough;
 
 		case SEQ_BLOCK_HEADER:
 			if (!fill_temp(s, b))
@@ -620,7 +620,7 @@ static enum xz_ret dec_main(struct xz_dec *s, struct xz_buf *b)
 
 			s->sequence = SEQ_BLOCK_UNCOMPRESS;
 
-			/* fall through */
+			fallthrough;
 
 		case SEQ_BLOCK_UNCOMPRESS:
 			ret = dec_block(s, b);
@@ -629,7 +629,7 @@ static enum xz_ret dec_main(struct xz_dec *s, struct xz_buf *b)
 
 			s->sequence = SEQ_BLOCK_PADDING;
 
-			/* fall through */
+			fallthrough;
 
 		case SEQ_BLOCK_PADDING:
 			/*
@@ -651,7 +651,7 @@ static enum xz_ret dec_main(struct xz_dec *s, struct xz_buf *b)
 
 			s->sequence = SEQ_BLOCK_CHECK;
 
-			/* fall through */
+			fallthrough;
 
 		case SEQ_BLOCK_CHECK:
 			if (s->check_type == XZ_CHECK_CRC32) {
@@ -675,7 +675,7 @@ static enum xz_ret dec_main(struct xz_dec *s, struct xz_buf *b)
 
 			s->sequence = SEQ_INDEX_PADDING;
 
-			/* fall through */
+			fallthrough;
 
 		case SEQ_INDEX_PADDING:
 			while ((s->index.size + (b->in_pos - s->in_start))
@@ -699,7 +699,7 @@ static enum xz_ret dec_main(struct xz_dec *s, struct xz_buf *b)
 
 			s->sequence = SEQ_INDEX_CRC32;
 
-			/* fall through */
+			fallthrough;
 
 		case SEQ_INDEX_CRC32:
 			ret = crc32_validate(s, b);
@@ -709,7 +709,7 @@ static enum xz_ret dec_main(struct xz_dec *s, struct xz_buf *b)
 			s->temp.size = STREAM_HEADER_SIZE;
 			s->sequence = SEQ_STREAM_FOOTER;
 
-			/* fall through */
+			fallthrough;
 
 		case SEQ_STREAM_FOOTER:
 			if (!fill_temp(s, b))
diff --git a/lib/zstd/bitstream.h b/lib/zstd/bitstream.h
index 3a49784d5c61..7c65c66e41fd 100644
--- a/lib/zstd/bitstream.h
+++ b/lib/zstd/bitstream.h
@@ -259,15 +259,15 @@ ZSTD_STATIC size_t BIT_initDStream(BIT_DStream_t *bitD, const void *srcBuffer, s
 		bitD->bitContainer = *(const BYTE *)(bitD->start);
 		switch (srcSize) {
 		case 7: bitD->bitContainer += (size_t)(((const BYTE *)(srcBuffer))[6]) << (sizeof(bitD->bitContainer) * 8 - 16);
-			/* fall through */
+			fallthrough;
 		case 6: bitD->bitContainer += (size_t)(((const BYTE *)(srcBuffer))[5]) << (sizeof(bitD->bitContainer) * 8 - 24);
-			/* fall through */
+			fallthrough;
 		case 5: bitD->bitContainer += (size_t)(((const BYTE *)(srcBuffer))[4]) << (sizeof(bitD->bitContainer) * 8 - 32);
-			/* fall through */
+			fallthrough;
 		case 4: bitD->bitContainer += (size_t)(((const BYTE *)(srcBuffer))[3]) << 24;
-			/* fall through */
+			fallthrough;
 		case 3: bitD->bitContainer += (size_t)(((const BYTE *)(srcBuffer))[2]) << 16;
-			/* fall through */
+			fallthrough;
 		case 2: bitD->bitContainer += (size_t)(((const BYTE *)(srcBuffer))[1]) << 8;
 		default:;
 		}
diff --git a/lib/zstd/compress.c b/lib/zstd/compress.c
index 5e0b67003e55..b080264ed3ad 100644
--- a/lib/zstd/compress.c
+++ b/lib/zstd/compress.c
@@ -3182,7 +3182,7 @@ static size_t ZSTD_compressStream_generic(ZSTD_CStream *zcs, void *dst, size_t *
 				zcs->outBuffFlushedSize = 0;
 				zcs->stage = zcss_flush; /* pass-through to flush stage */
 			}
-			/* fall through */
+			fallthrough;
 
 		case zcss_flush: {
 			size_t const toFlush = zcs->outBuffContentSize - zcs->outBuffFlushedSize;
diff --git a/lib/zstd/decompress.c b/lib/zstd/decompress.c
index db6761ea4deb..66cd487a326a 100644
--- a/lib/zstd/decompress.c
+++ b/lib/zstd/decompress.c
@@ -442,7 +442,7 @@ size_t ZSTD_decodeLiteralsBlock(ZSTD_DCtx *dctx, const void *src, size_t srcSize
 		case set_repeat:
 			if (dctx->litEntropy == 0)
 				return ERROR(dictionary_corrupted);
-			/* fall through */
+			fallthrough;
 		case set_compressed:
 			if (srcSize < 5)
 				return ERROR(corruption_detected); /* srcSize >= MIN_CBLOCK_SIZE == 3; here we need up to 5 for case 3 */
@@ -1768,7 +1768,7 @@ size_t ZSTD_decompressContinue(ZSTD_DCtx *dctx, void *dst, size_t dstCapacity, c
 			return 0;
 		}
 		dctx->expected = 0; /* not necessary to copy more */
-		/* fall through */
+		fallthrough;
 
 	case ZSTDds_decodeFrameHeader:
 		memcpy(dctx->headerBuffer + ZSTD_frameHeaderSize_prefix, src, dctx->expected);
@@ -2309,7 +2309,7 @@ size_t ZSTD_decompressStream(ZSTD_DStream *zds, ZSTD_outBuffer *output, ZSTD_inB
 		switch (zds->stage) {
 		case zdss_init:
 			ZSTD_resetDStream(zds); /* transparent reset on starting decoding a new frame */
-			/* fall through */
+			fallthrough;
 
 		case zdss_loadHeader: {
 			size_t const hSize = ZSTD_getFrameParams(&zds->fParams, zds->headerBuffer, zds->lhSize);
@@ -2376,7 +2376,7 @@ size_t ZSTD_decompressStream(ZSTD_DStream *zds, ZSTD_outBuffer *output, ZSTD_inB
 			}
 			zds->stage = zdss_read;
 		}
-			/* fall through */
+			fallthrough;
 
 		case zdss_read: {
 			size_t const neededInSize = ZSTD_nextSrcSizeToDecompress(zds->dctx);
@@ -2405,7 +2405,7 @@ size_t ZSTD_decompressStream(ZSTD_DStream *zds, ZSTD_outBuffer *output, ZSTD_inB
 			zds->stage = zdss_load;
 			/* pass-through */
 		}
-			/* fall through */
+			fallthrough;
 
 		case zdss_load: {
 			size_t const neededInSize = ZSTD_nextSrcSizeToDecompress(zds->dctx);
@@ -2438,7 +2438,7 @@ size_t ZSTD_decompressStream(ZSTD_DStream *zds, ZSTD_outBuffer *output, ZSTD_inB
 				/* pass-through */
 			}
 		}
-			/* fall through */
+			fallthrough;
 
 		case zdss_flush: {
 			size_t const toFlushSize = zds->outEnd - zds->outStart;
diff --git a/lib/zstd/huf_compress.c b/lib/zstd/huf_compress.c
index e727812d12aa..08b4ae80aed4 100644
--- a/lib/zstd/huf_compress.c
+++ b/lib/zstd/huf_compress.c
@@ -556,9 +556,9 @@ size_t HUF_compress1X_usingCTable(void *dst, size_t dstSize, const void *src, si
 	n = srcSize & ~3; /* join to mod 4 */
 	switch (srcSize & 3) {
 	case 3: HUF_encodeSymbol(&bitC, ip[n + 2], CTable); HUF_FLUSHBITS_2(&bitC);
-		/* fall through */
+		fallthrough;
 	case 2: HUF_encodeSymbol(&bitC, ip[n + 1], CTable); HUF_FLUSHBITS_1(&bitC);
-		/* fall through */
+		fallthrough;
 	case 1: HUF_encodeSymbol(&bitC, ip[n + 0], CTable); HUF_FLUSHBITS(&bitC);
 	case 0:
 	default:;
-- 
2.29.2.299.gdc1121823c-goog


^ permalink raw reply related

* [PATCH 0/3] PPC: Fix -Wimplicit-fallthrough for clang
From: Nick Desaulniers @ 2020-11-16  4:35 UTC (permalink / raw)
  To: Gustavo A . R . Silva, Nathan Chancellor, Miguel Ojeda,
	Michael Ellerman
  Cc: Nick Desaulniers, linux-kernel, clang-built-linux, Paul Mackerras,
	linuxppc-dev

While cleaning up the last few -Wimplicit-fallthrough warnings in tree
for Clang, I noticed
commit 6a9dc5fd6170d ("lib: Revert use of fallthrough pseudo-keyword in lib/")
which seemed to undo a bunch of fixes in lib/ due to breakage in
arch/powerpc/boot/ not including compiler_types.h.  We don't need
compiler_types.h for the definition of `fallthrough`, simply
compiler_attributes.h.  Include that, revert the revert to lib/, and fix
the last remaining cases I observed for powernv_defconfig.

Nick Desaulniers (3):
  powerpc: boot: include compiler_attributes.h
  Revert "lib: Revert use of fallthrough pseudo-keyword in lib/"
  powerpc: fix -Wimplicit-fallthrough

 arch/powerpc/boot/Makefile     |  1 +
 arch/powerpc/boot/decompress.c |  1 -
 arch/powerpc/kernel/uprobes.c  |  1 +
 arch/powerpc/perf/imc-pmu.c    |  1 +
 lib/asn1_decoder.c             |  4 ++--
 lib/assoc_array.c              |  2 +-
 lib/bootconfig.c               |  4 ++--
 lib/cmdline.c                  | 10 +++++-----
 lib/dim/net_dim.c              |  2 +-
 lib/dim/rdma_dim.c             |  4 ++--
 lib/glob.c                     |  2 +-
 lib/siphash.c                  | 36 +++++++++++++++++-----------------
 lib/ts_fsm.c                   |  2 +-
 lib/vsprintf.c                 | 14 ++++++-------
 lib/xz/xz_dec_lzma2.c          |  4 ++--
 lib/xz/xz_dec_stream.c         | 16 +++++++--------
 lib/zstd/bitstream.h           | 10 +++++-----
 lib/zstd/compress.c            |  2 +-
 lib/zstd/decompress.c          | 12 ++++++------
 lib/zstd/huf_compress.c        |  4 ++--
 20 files changed, 67 insertions(+), 65 deletions(-)

-- 
2.29.2.299.gdc1121823c-goog


^ permalink raw reply

* Re: [PATCH v2 03/19] powerpc: bad_page_fault, do_break get registers from regs
From: Nicholas Piggin @ 2020-11-16  1:52 UTC (permalink / raw)
  To: Christophe Leroy; +Cc: linuxppc-dev
In-Reply-To: <20201114200700.Horde.mC7ctDwjEwuYYJJgK2pO9A4@messagerie.c-s.fr>

Excerpts from Christophe Leroy's message of November 15, 2020 5:07 am:
> Hi,
> 
> Quoting Nicholas Piggin <npiggin@gmail.com>:
> 
>> This also moves the 32s DABR match to C.
> 
> I'm still not happy with that. What about the following instead ?

This seems quite clean. I'll pull it into the series and see how it looks.

Thanks,
Nick

> 
> diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S
> index 8cdc8bcde703..6253c4acb46d 100644
> --- a/arch/powerpc/kernel/entry_32.S
> +++ b/arch/powerpc/kernel/entry_32.S
> @@ -657,10 +657,6 @@ ppc_swapcontext:
>   	.globl	handle_page_fault
>   handle_page_fault:
>   	addi	r3,r1,STACK_FRAME_OVERHEAD
> -#ifdef CONFIG_PPC_BOOK3S_32
> -	andis.  r0,r5,DSISR_DABRMATCH@h
> -	bne-    handle_dabr_fault
> -#endif
>   	bl	do_page_fault
>   	cmpwi	r3,0
>   	beq+	ret_from_except
> @@ -674,17 +670,6 @@ handle_page_fault:
>   	bl	bad_page_fault
>   	b	ret_from_except_full
> 
> -#ifdef CONFIG_PPC_BOOK3S_32
> -	/* We have a data breakpoint exception - handle it */
> -handle_dabr_fault:
> -	SAVE_NVGPRS(r1)
> -	lwz	r0,_TRAP(r1)
> -	clrrwi	r0,r0,1
> -	stw	r0,_TRAP(r1)
> -	bl      do_break
> -	b	ret_from_except_full
> -#endif
> -
>   /*
>    * This routine switches between two different tasks.  The process
>    * state of one is saved on its kernel stack.  Then the state
> diff --git a/arch/powerpc/kernel/head_book3s_32.S  
> b/arch/powerpc/kernel/head_book3s_32.S
> index 9381aa867591..5cc71482b35f 100644
> --- a/arch/powerpc/kernel/head_book3s_32.S
> +++ b/arch/powerpc/kernel/head_book3s_32.S
> @@ -684,7 +684,10 @@ handle_page_fault_tramp_1:
>   	lwz	r5, _DSISR(r11)
>   	/* fall through */
>   handle_page_fault_tramp_2:
> +	andis.  r0, r5, DSISR_DABRMATCH@h
> +	bne-	1f
>   	EXC_XFER_LITE(0x300, handle_page_fault)
> +1:	EXC_XFER_STD(0x300, do_break)
> 
>   #ifdef CONFIG_VMAP_STACK
>   .macro save_regs_thread		thread
> ---
> Christophe
> 
>>
>> Similar to the previous patch this makes interrupt handler function
>> types more regular so they can be wrapped with the next patch.
>>
>> bad_page_fault and do_break are not performance critical.
>>
>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>> ---
>>  arch/powerpc/include/asm/bug.h             |  2 +-
>>  arch/powerpc/include/asm/debug.h           |  3 +--
>>  arch/powerpc/kernel/entry_32.S             | 14 ++++----------
>>  arch/powerpc/kernel/exceptions-64e.S       |  3 +--
>>  arch/powerpc/kernel/exceptions-64s.S       |  3 +--
>>  arch/powerpc/kernel/head_8xx.S             |  5 ++---
>>  arch/powerpc/kernel/process.c              |  7 +++----
>>  arch/powerpc/kernel/traps.c                |  2 +-
>>  arch/powerpc/mm/book3s64/hash_utils.c      |  4 ++--
>>  arch/powerpc/mm/book3s64/slb.c             |  2 +-
>>  arch/powerpc/mm/fault.c                    | 14 +++++++-------
>>  arch/powerpc/platforms/8xx/machine_check.c |  2 +-
>>  12 files changed, 25 insertions(+), 36 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/bug.h b/arch/powerpc/include/asm/bug.h
>> index 897bad6b6bbb..49162faba33f 100644
>> --- a/arch/powerpc/include/asm/bug.h
>> +++ b/arch/powerpc/include/asm/bug.h
>> @@ -113,7 +113,7 @@
>>  struct pt_regs;
>>  long do_page_fault(struct pt_regs *);
>>  long hash__do_page_fault(struct pt_regs *);
>> -extern void bad_page_fault(struct pt_regs *, unsigned long, int);
>> +void bad_page_fault(struct pt_regs *, int);
>>  extern void _exception(int, struct pt_regs *, int, unsigned long);
>>  extern void _exception_pkey(struct pt_regs *, unsigned long, int);
>>  extern void die(const char *, struct pt_regs *, long);
>> diff --git a/arch/powerpc/include/asm/debug.h  
>> b/arch/powerpc/include/asm/debug.h
>> index ec57daf87f40..0550eceab3ca 100644
>> --- a/arch/powerpc/include/asm/debug.h
>> +++ b/arch/powerpc/include/asm/debug.h
>> @@ -52,8 +52,7 @@ extern void do_send_trap(struct pt_regs *regs,  
>> unsigned long address,
>>  			 unsigned long error_code, int brkpt);
>>  #else
>>
>> -extern void do_break(struct pt_regs *regs, unsigned long address,
>> -		     unsigned long error_code);
>> +void do_break(struct pt_regs *regs);
>>  #endif
>>
>>  #endif /* _ASM_POWERPC_DEBUG_H */
>> diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S
>> index 8cdc8bcde703..eb97df234a0c 100644
>> --- a/arch/powerpc/kernel/entry_32.S
>> +++ b/arch/powerpc/kernel/entry_32.S
>> @@ -657,10 +657,6 @@ ppc_swapcontext:
>>  	.globl	handle_page_fault
>>  handle_page_fault:
>>  	addi	r3,r1,STACK_FRAME_OVERHEAD
>> -#ifdef CONFIG_PPC_BOOK3S_32
>> -	andis.  r0,r5,DSISR_DABRMATCH@h
>> -	bne-    handle_dabr_fault
>> -#endif
>>  	bl	do_page_fault
>>  	cmpwi	r3,0
>>  	beq+	ret_from_except
>> @@ -668,19 +664,17 @@ handle_page_fault:
>>  	lwz	r0,_TRAP(r1)
>>  	clrrwi	r0,r0,1
>>  	stw	r0,_TRAP(r1)
>> -	mr	r5,r3
>> +	mr	r4,r3		/* err arg for bad_page_fault */
>>  	addi	r3,r1,STACK_FRAME_OVERHEAD
>> -	lwz	r4,_DAR(r1)
>> +#ifdef CONFIG_PPC_BOOK3S_32
>> +	blt	handle_dabr_fault
>> +#endif
>>  	bl	bad_page_fault
>>  	b	ret_from_except_full
>>
>>  #ifdef CONFIG_PPC_BOOK3S_32
>>  	/* We have a data breakpoint exception - handle it */
>>  handle_dabr_fault:
>> -	SAVE_NVGPRS(r1)
>> -	lwz	r0,_TRAP(r1)
>> -	clrrwi	r0,r0,1
>> -	stw	r0,_TRAP(r1)
>>  	bl      do_break
>>  	b	ret_from_except_full
>>  #endif
>> diff --git a/arch/powerpc/kernel/exceptions-64e.S  
>> b/arch/powerpc/kernel/exceptions-64e.S
>> index 25fa7d5a643c..dc728bb1c89a 100644
>> --- a/arch/powerpc/kernel/exceptions-64e.S
>> +++ b/arch/powerpc/kernel/exceptions-64e.S
>> @@ -1018,9 +1018,8 @@ storage_fault_common:
>>  	bne-	1f
>>  	b	ret_from_except_lite
>>  1:	bl	save_nvgprs
>> -	mr	r5,r3
>> +	mr	r4,r3
>>  	addi	r3,r1,STACK_FRAME_OVERHEAD
>> -	ld	r4,_DAR(r1)
>>  	bl	bad_page_fault
>>  	b	ret_from_except
>>
>> diff --git a/arch/powerpc/kernel/exceptions-64s.S  
>> b/arch/powerpc/kernel/exceptions-64s.S
>> index 1f34cfd1887c..e6558c4d3f81 100644
>> --- a/arch/powerpc/kernel/exceptions-64s.S
>> +++ b/arch/powerpc/kernel/exceptions-64s.S
>> @@ -2135,8 +2135,7 @@ EXC_COMMON_BEGIN(h_data_storage_common)
>>  	GEN_COMMON h_data_storage
>>  	addi    r3,r1,STACK_FRAME_OVERHEAD
>>  BEGIN_MMU_FTR_SECTION
>> -	ld	r4,_DAR(r1)
>> -	li	r5,SIGSEGV
>> +	li	r4,SIGSEGV
>>  	bl      bad_page_fault
>>  MMU_FTR_SECTION_ELSE
>>  	bl      unknown_exception
>> diff --git a/arch/powerpc/kernel/head_8xx.S b/arch/powerpc/kernel/head_8xx.S
>> index 0cd95b633e2b..13eda7154695 100644
>> --- a/arch/powerpc/kernel/head_8xx.S
>> +++ b/arch/powerpc/kernel/head_8xx.S
>> @@ -408,10 +408,9 @@ do_databreakpoint:
>>  	addi	r3,r1,STACK_FRAME_OVERHEAD
>>  	mfspr	r4,SPRN_BAR
>>  	stw	r4,_DAR(r11)
>> -#ifdef CONFIG_VMAP_STACK
>> -	lwz	r5,_DSISR(r11)
>> -#else
>> +#ifndef CONFIG_VMAP_STACK
>>  	mfspr	r5,SPRN_DSISR
>> +	stw	r5,_DSISR(r11)
>>  #endif
>>  	EXC_XFER_STD(0x1c00, do_break)
>>
>> diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
>> index d421a2c7f822..0bdd3ed653df 100644
>> --- a/arch/powerpc/kernel/process.c
>> +++ b/arch/powerpc/kernel/process.c
>> @@ -660,11 +660,10 @@ static void do_break_handler(struct pt_regs *regs)
>>  	}
>>  }
>>
>> -void do_break (struct pt_regs *regs, unsigned long address,
>> -		    unsigned long error_code)
>> +void do_break(struct pt_regs *regs)
>>  {
>>  	current->thread.trap_nr = TRAP_HWBKPT;
>> -	if (notify_die(DIE_DABR_MATCH, "dabr_match", regs, error_code,
>> +	if (notify_die(DIE_DABR_MATCH, "dabr_match", regs, regs->dsisr,
>>  			11, SIGSEGV) == NOTIFY_STOP)
>>  		return;
>>
>> @@ -682,7 +681,7 @@ void do_break (struct pt_regs *regs, unsigned  
>> long address,
>>  		do_break_handler(regs);
>>
>>  	/* Deliver the signal to userspace */
>> -	force_sig_fault(SIGTRAP, TRAP_HWBKPT, (void __user *)address);
>> +	force_sig_fault(SIGTRAP, TRAP_HWBKPT, (void __user *)regs->dar);
>>  }
>>  #endif	/* CONFIG_PPC_ADV_DEBUG_REGS */
>>
>> diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
>> index 5006dcbe1d9f..902fcbd1a778 100644
>> --- a/arch/powerpc/kernel/traps.c
>> +++ b/arch/powerpc/kernel/traps.c
>> @@ -1641,7 +1641,7 @@ void alignment_exception(struct pt_regs *regs)
>>  	if (user_mode(regs))
>>  		_exception(sig, regs, code, regs->dar);
>>  	else
>> -		bad_page_fault(regs, regs->dar, sig);
>> +		bad_page_fault(regs, sig);
>>
>>  bail:
>>  	exception_exit(prev_state);
>> diff --git a/arch/powerpc/mm/book3s64/hash_utils.c  
>> b/arch/powerpc/mm/book3s64/hash_utils.c
>> index 0f0bd4af4b2d..731518e7d56f 100644
>> --- a/arch/powerpc/mm/book3s64/hash_utils.c
>> +++ b/arch/powerpc/mm/book3s64/hash_utils.c
>> @@ -1537,7 +1537,7 @@ long do_hash_fault(struct pt_regs *regs)
>>  	 * the access, or panic if there isn't a handler.
>>  	 */
>>  	if (unlikely(in_nmi())) {
>> -		bad_page_fault(regs, ea, SIGSEGV);
>> +		bad_page_fault(regs, SIGSEGV);
>>  		return 0;
>>  	}
>>
>> @@ -1576,7 +1576,7 @@ long do_hash_fault(struct pt_regs *regs)
>>  			else
>>  				_exception(SIGBUS, regs, BUS_ADRERR, ea);
>>  		} else {
>> -			bad_page_fault(regs, ea, SIGBUS);
>> +			bad_page_fault(regs, SIGBUS);
>>  		}
>>  		err = 0;
>>
>> diff --git a/arch/powerpc/mm/book3s64/slb.c b/arch/powerpc/mm/book3s64/slb.c
>> index cc34d50874c1..ae89ad516247 100644
>> --- a/arch/powerpc/mm/book3s64/slb.c
>> +++ b/arch/powerpc/mm/book3s64/slb.c
>> @@ -898,7 +898,7 @@ void do_bad_slb_fault(struct pt_regs *regs)
>>  		if (user_mode(regs))
>>  			_exception(SIGSEGV, regs, SEGV_BNDERR, regs->dar);
>>  		else
>> -			bad_page_fault(regs, regs->dar, SIGSEGV);
>> +			bad_page_fault(regs, SIGSEGV);
>>  	} else if (err == -EINVAL) {
>>  		unrecoverable_exception(regs);
>>  	} else {
>> diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c
>> index 390a296b16a3..db73b373e76c 100644
>> --- a/arch/powerpc/mm/fault.c
>> +++ b/arch/powerpc/mm/fault.c
>> @@ -375,7 +375,7 @@ static void sanity_check_fault(bool is_write,  
>> bool is_user,
>>  #elif defined(CONFIG_PPC_BOOK3E_64)
>>  #define page_fault_is_bad(__err)	((__err) & DSISR_BAD_FAULT_64S)
>>  #else
>> -#define page_fault_is_bad(__err)	((__err) & DSISR_BAD_FAULT_32S)
>> +#define page_fault_is_bad(__err)	((__err) & (DSISR_BAD_FAULT_32S |  
>> DSISR_DABRMATCH))
>>  #endif
>>  #endif
>>
>> @@ -408,7 +408,7 @@ static int __do_page_fault(struct pt_regs *regs,  
>> unsigned long address,
>>  		return 0;
>>
>>  	if (unlikely(page_fault_is_bad(error_code))) {
>> -		if (IS_ENABLED(CONFIG_PPC_BOOK3S_64) && (error_code & DSISR_DABRMATCH))
>> +		if (IS_ENABLED(CONFIG_PPC_BOOK3S) && (error_code & DSISR_DABRMATCH))
>>  			return -1;
>>
>>  		if (is_user) {
>> @@ -562,14 +562,14 @@ long do_page_fault(struct pt_regs *regs)
>>  	/* 32 and 64e handle errors in their asm code */
>>  	if (unlikely(err)) {
>>  		if (err > 0) {
>> -			bad_page_fault(regs, address, err);
>> +			bad_page_fault(regs, err);
>>  			err = 0;
>>  		} else {
>>  			/*
>>  			 * do_break() may change NV GPRS while handling the
>>  			 * breakpoint. Return -ve to caller to do that.
>>  			 */
>> -			do_break(regs, address, error_code);
>> +			do_break(regs);
>>  		}
>>  	}
>>  #endif
>> @@ -591,14 +591,14 @@ long hash__do_page_fault(struct pt_regs *regs)
>>  	err = __do_page_fault(regs, address, error_code);
>>  	if (unlikely(err)) {
>>  		if (err > 0) {
>> -			bad_page_fault(regs, address, err);
>> +			bad_page_fault(regs, err);
>>  			err = 0;
>>  		} else {
>>  			/*
>>  			 * do_break() may change NV GPRS while handling the
>>  			 * breakpoint. Return -ve to caller to do that.
>>  			 */
>> -			do_break(regs, address, error_code);
>> +			do_break(regs);
>>  		}
>>  	}
>>
>> @@ -612,7 +612,7 @@ NOKPROBE_SYMBOL(hash__do_page_fault);
>>   * It is called from the DSI and ISI handlers in head.S and from some
>>   * of the procedures in traps.c.
>>   */
>> -void bad_page_fault(struct pt_regs *regs, unsigned long address, int sig)
>> +void bad_page_fault(struct pt_regs *regs, int sig)
>>  {
>>  	const struct exception_table_entry *entry;
>>  	int is_write = page_fault_is_write(regs->dsisr);
>> diff --git a/arch/powerpc/platforms/8xx/machine_check.c  
>> b/arch/powerpc/platforms/8xx/machine_check.c
>> index 88dedf38eccd..656365975895 100644
>> --- a/arch/powerpc/platforms/8xx/machine_check.c
>> +++ b/arch/powerpc/platforms/8xx/machine_check.c
>> @@ -26,7 +26,7 @@ int machine_check_8xx(struct pt_regs *regs)
>>  	 * to deal with that than having a wart in the mcheck handler.
>>  	 * -- BenH
>>  	 */
>> -	bad_page_fault(regs, regs->dar, SIGBUS);
>> +	bad_page_fault(regs, SIGBUS);
>>  	return 1;
>>  #else
>>  	return 0;
>> --
>> 2.23.0
> 
> 
> 

^ permalink raw reply

* Re: [PATCH 1/3] asm-generic/atomic64: Add support for ARCH_ATOMIC
From: Nicholas Piggin @ 2020-11-16  1:48 UTC (permalink / raw)
  To: Christophe Leroy, Peter Zijlstra
  Cc: Christophe Leroy, linux-arch, Arnd Bergmann, Alexey Kardashevskiy,
	Boqun Feng, linux-kernel, Will Deacon, linuxppc-dev
In-Reply-To: <20201111134412.GT2611@hirez.programming.kicks-ass.net>

Excerpts from Peter Zijlstra's message of November 11, 2020 11:44 pm:
> On Wed, Nov 11, 2020 at 02:39:01PM +0100, Christophe Leroy wrote:
>> Hello,
>> 
>> Le 11/11/2020 à 12:07, Nicholas Piggin a écrit :
>> > This passes atomic64 selftest on ppc32 on qemu (uniprocessor only)
>> > both before and after powerpc is converted to use ARCH_ATOMIC.
>> 
>> Can you explain what this change does and why it is needed ?
> 
> That certainly should've been in the Changelog. This enables atomic
> instrumentation, see asm-generic/atomic-instrumented.h. IOW, it makes
> atomic ops visible to K*SAN.
> 

Right. This specific patch doesn't actually "do" anything except
allow generic atomic64 to be used with ARCH_ATOMIC. It does that
by re-naming some things to avoid name collisions and also providing
the arch_ prefix that ARCH_ATOMIC expects.

I don't know what should be in the changelog. I suppose the latter,
the former is discoverable by looking at ARCH_ATOMIC code and
patches but I guess it's polite and doesn't hurt to include the
former as well.

I'll send an update before long.

Thanks,
Nick

^ permalink raw reply

* Re: Duplicated ABI entries - Was: Re: [PATCH v2 20/39] docs: ABI: testing: make the files compatible with ReST output
From: Jonathan Cameron @ 2020-11-14 15:27 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Gautham R. Shenoy, Jason A. Donenfeld, Heikki Krogerus,
	Peter Meerwald-Stadler, Petr Mladek, Linux Doc Mailing List,
	Alexander Shishkin, Nayna Jain, Jonathan Cameron,
	Alexandre Belloni, Mimi Zohar, Sebastian Reichel, linux-mm,
	Bruno Meneguele, Vishal Verma, Pavel Machek, Hanjun Guo,
	Guenter Roeck, netdev, Oleh Kravchenko, Dan Williams,
	Andrew Donnellan, Javier González, Fabrice Gasnier,
	Mark Gross, linux-acpi, Jonathan Corbet, Chunyan Zhang,
	Mario Limonciello, linux-stm32, Lakshmi Ramasubramanian,
	Ludovic Desroches, Pawan Gupta, linux-arm-kernel, Tom Rix,
	Frederic Barrat, Niklas Cassel, Len Brown, Juergen Gross,
	linuxppc-dev, Mika Westerberg, Alexandre Torgue, linux-pm,
	linux-kernel, Richard Cochran, Oded Gabbay, Baolin Wang,
	Lars-Peter Clausen, Dan Murphy, Orson Zhai, Philippe Bergheaud,
	xen-devel, Boris Ostrovsky, Andy Shevchenko, Benson Leung,
	Konstantin Khlebnikov, Jens Axboe, Felipe Balbi, Kranthi Kuntala,
	Martin K. Petersen, Greg Kroah-Hartman, linux-usb,
	Rafael J. Wysocki, Nicolas Ferre, linux-iio, Thinh Nguyen,
	Sergey Senozhatsky, Stefano Stabellini, Thomas Gleixner,
	Leonid Maksymchuk, Maxime Coquelin, Johannes Thumshirn,
	Enric Balletbo i Serra, Vaibhav Jain, Vineela Tummalapalli,
	Peter Rosin, Mike Kravetz
In-Reply-To: <20201110082658.2edc1ab5@coco.lan>

On Tue, 10 Nov 2020 08:26:58 +0100
Mauro Carvalho Chehab <mchehab+huawei@kernel.org> wrote:

> Hi Jonathan,
> 
> Em Sun, 8 Nov 2020 16:56:21 +0000
> Jonathan Cameron <jic23@kernel.org> escreveu:
> 
> > > PS.: the IIO subsystem is the one that currently has more duplicated
> > > ABI entries:  
> > > $ ./scripts/get_abi.pl validate 2>&1|grep iio
> > > Warning: /sys/bus/iio/devices/iio:deviceX/in_accel_x_calibbias is defined 2 times:  Documentation/ABI/testing/sysfs-bus-iio-icm42600:0  Documentation/ABI/testing/sysfs-bus-iio:394
> > > Warning: /sys/bus/iio/devices/iio:deviceX/in_accel_y_calibbias is defined 2 times:  Documentation/ABI/testing/sysfs-bus-iio-icm42600:1  Documentation/ABI/testing/sysfs-bus-iio:395
> > > Warning: /sys/bus/iio/devices/iio:deviceX/in_accel_z_calibbias is defined 2 times:  Documentation/ABI/testing/sysfs-bus-iio-icm42600:2  Documentation/ABI/testing/sysfs-bus-iio:396
> > > Warning: /sys/bus/iio/devices/iio:deviceX/in_anglvel_x_calibbias is defined 2 times:  Documentation/ABI/testing/sysfs-bus-iio-icm42600:3  Documentation/ABI/testing/sysfs-bus-iio:397
> > > Warning: /sys/bus/iio/devices/iio:deviceX/in_anglvel_y_calibbias is defined 2 times:  Documentation/ABI/testing/sysfs-bus-iio-icm42600:4  Documentation/ABI/testing/sysfs-bus-iio:398
> > > Warning: /sys/bus/iio/devices/iio:deviceX/in_anglvel_z_calibbias is defined 2 times:  Documentation/ABI/testing/sysfs-bus-iio-icm42600:5  Documentation/ABI/testing/sysfs-bus-iio:399
> > > Warning: /sys/bus/iio/devices/iio:deviceX/in_count0_preset is defined 2 times:  Documentation/ABI/testing/sysfs-bus-iio-timer-stm32:100  Documentation/ABI/testing/sysfs-bus-iio-lptimer-stm32:0
> > > Warning: /sys/bus/iio/devices/iio:deviceX/in_count0_quadrature_mode is defined 2 times:  Documentation/ABI/testing/sysfs-bus-iio-timer-stm32:117  Documentation/ABI/testing/sysfs-bus-iio-lptimer-stm32:14
> > > Warning: /sys/bus/iio/devices/iio:deviceX/in_count_quadrature_mode_available is defined 3 times:  Documentation/ABI/testing/sysfs-bus-iio-counter-104-quad-8:2  Documentation/ABI/testing/sysfs-bus-iio-timer-stm32:111  Documentation/ABI/testing/sysfs-bus-iio-lptimer-stm32:8
> > > Warning: /sys/bus/iio/devices/iio:deviceX/out_altvoltageY_frequency is defined 2 times:  Documentation/ABI/testing/sysfs-bus-iio-frequency-adf4371:0  Documentation/ABI/testing/sysfs-bus-iio:599
> > > Warning: /sys/bus/iio/devices/iio:deviceX/out_altvoltageY_powerdown is defined 2 times:  Documentation/ABI/testing/sysfs-bus-iio-frequency-adf4371:36  Documentation/ABI/testing/sysfs-bus-iio:588
> > > Warning: /sys/bus/iio/devices/iio:deviceX/out_currentY_raw is defined 2 times:  Documentation/ABI/testing/sysfs-bus-iio-light-lm3533-als:43  Documentation/ABI/testing/sysfs-bus-iio-health-afe440x:38
> > > Warning: /sys/bus/iio/devices/iio:deviceX/out_current_heater_raw is defined 2 times:  Documentation/ABI/testing/sysfs-bus-iio-humidity-hdc2010:0  Documentation/ABI/testing/sysfs-bus-iio-humidity-hdc100x:0
> > > Warning: /sys/bus/iio/devices/iio:deviceX/out_current_heater_raw_available is defined 2 times:  Documentation/ABI/testing/sysfs-bus-iio-humidity-hdc2010:1  Documentation/ABI/testing/sysfs-bus-iio-humidity-hdc100x:1
> > > Warning: /sys/bus/iio/devices/iio:deviceX/sensor_sensitivity is defined 2 times:  Documentation/ABI/testing/sysfs-bus-iio-distance-srf08:0  Documentation/ABI/testing/sysfs-bus-iio-proximity-as3935:8
> > > Warning: /sys/bus/iio/devices/triggerX/sampling_frequency is defined 2 times:  Documentation/ABI/testing/sysfs-bus-iio-timer-stm32:92  Documentation/ABI/testing/sysfs-bus-iio:45    
> 
> > 
> > That was intentional.  Often these provide more information on the
> > ABI for a particular device than is present in the base ABI doc.  
> 
> FYI, right now, there are 20 duplicated entries, being 16 of them
> from IIO, on those files:
> 
> 	$ ./scripts/get_abi.pl validate 2>&1|perl -ne 'if (m,(Documentation/\S+)\:,g) { print "$1\n" }'|sort|uniq
> 	Documentation/ABI/stable/sysfs-driver-w1_ds28e04
> 	Documentation/ABI/testing/sysfs-bus-iio-counter-104-quad-8
> 	Documentation/ABI/testing/sysfs-bus-iio-distance-srf08
> 	Documentation/ABI/testing/sysfs-bus-iio-frequency-adf4371
> 	Documentation/ABI/testing/sysfs-bus-iio-humidity-hdc2010
> 	Documentation/ABI/testing/sysfs-bus-iio-icm42600
> 	Documentation/ABI/testing/sysfs-bus-iio-light-lm3533-als
> 	Documentation/ABI/testing/sysfs-bus-iio-timer-stm32
> 	Documentation/ABI/testing/sysfs-class-backlight-adp8860
> 	Documentation/ABI/testing/sysfs-class-led-trigger-pattern
> 	Documentation/ABI/testing/sysfs-kernel-iommu_groups
> 
> > 
> > A bit like when we have additional description for dt binding properties
> > for a particular device, even though they are standard properties.
> > 
> > Often a standard property allows for more values than the specific
> > one for a particular device.  There can also be obscuring coupling
> > between sysfs attributes due to hardware restrictions that we would
> > like to provide some explanatory info on.
> > 
> > I suppose we could add all this information to the parent doc but
> > that is pretty ugly and will make that doc very nasty to read.  
> 
> I understand what you meant to do, but right now, it is is actually
> a lot uglier than merging into a single entry ;-)
> 
> Let's view ABI from the PoV of a system admin that doesn't know
> yet about a certain ABI symbol.

I'd be surprised if a sys admin is looking at these at all. They
tend to be used only by userspace software writers.  But I guess the
point stands.

> 
> He'll try to seek for the symbol, more likely using the HTML 
> documentation. Only very senior system admins might try to take
> a look at the Kernel.

Sad truth here is that before these were in the html docs, they'd
have grepped and the right option would fairly obvious as it
would be the more specific file.  Ah well, sometimes progress bites :)

> 
> This is what happens when one would seek for a duplicated symbol
> via command line:
> 
> 	$ ./scripts/get_abi.pl search /sys/bus/iio/devices/iio:deviceX/out_altvoltageY_frequency$
> 	
> 	/sys/bus/iio/devices/iio:deviceX/out_altvoltageY_frequency
> 	----------------------------------------------------------
> 	
> 	Kernel version:		3.4.0
> 	Contact:		linux-iio@vger.kernel.org
> 	Defined on file(s):	Documentation/ABI/testing/sysfs-bus-iio-frequency-adf4371 Documentation/ABI/testing/sysfs-bus-iio
> 	
> 	Description:
> 	
> 	Stores the PLL frequency in Hz for channel Y.
> 	Reading returns the actual frequency in Hz.
> 	The ADF4371 has an integrated VCO with fundamendal output
> 	frequency ranging from 4000000000 Hz 8000000000 Hz.
> 	
> 	out_altvoltage0_frequency:
> 	        A divide by 1, 2, 4, 8, 16, 32 or circuit generates
> 	        frequencies from 62500000 Hz to 8000000000 Hz.
> 	out_altvoltage1_frequency:
> 	        This channel duplicates the channel 0 frequency
> 	out_altvoltage2_frequency:
> 	        A frequency doubler generates frequencies from
> 	        8000000000 Hz to 16000000000 Hz.
> 	out_altvoltage3_frequency:
> 	        A frequency quadrupler generates frequencies from
> 	        16000000000 Hz to 32000000000 Hz.
> 	
> 	Note: writes to one of the channels will affect the frequency of
> 	all the other channels, since it involves changing the VCO
> 	fundamental output frequency.
> 	
> 	Output frequency for channel Y in Hz. The number must always be
> 	specified and unique if the output corresponds to a single
> 	channel.
> 
> As the "What:" field is identical on both sysfs-bus-iio-frequency-adf4371
> and sysfs-bus-iio, those entries are merged, which produces an ABI
> documentation mixing both the generic one and the board specific one
> into a single output.
> 
> Worse than that, the "generic" content is at the end.
> 
> The same happens when generating the HTML output.
> 
> See, entries at the HTML output are ordered by the What: field,
> which is considered within the script as an unique key, as it is
> unique (except for IIO and a couple of other cases).
> 
> -
> 
> As I commented on an e-mail I sent to Greg, I see a few ways
> to solve it.
> 
> The most trivial one (which I used to solve a few conflicts on
> other places), is to place driver-specific details on a separate
> file under Documentation/driver-api, and mention it at the
> generic entries. The docs building system will generate cross
> references for Documentation/.../foo.rst files, so, everything
> should be OK.

Hmm. That might work out OK.  These devices tend to be weird enough
that they probably could do with some additional explanation anyway. 

> 
> The second alternative that I also used on a couple of places
> is to modify the generic entry for it to contain the generic
> definition first, followed by per-device details.

I'll do an audit of what we actually have here. Perhaps we end
up with a mixture of these two options.

Might take a little while though.

> 
> There is a third possible alternative: add a new optional field
> (something like Scope:) which would be part of the unique key,
> if present. Implementing support for it could be tricky, as the
> produced output would likely need to create cross-references
> between the generic field (if present) and the per-device details.
That would be lovely but probably not worth the effort for something
that occurs so rarely currently.

Jonathan

> 
> Thanks,
> Mauro
> 
> PS.: I'm taking a few days of PTO during this week. So, it
> could take a while for me to reply again to this thread.


^ permalink raw reply

* Re: [PATCH] arch: pgtable: define MAX_POSSIBLE_PHYSMEM_BITS where needed
From: Mike Rapoport @ 2020-11-15  6:44 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: linux-arch, Thomas Bogendoerfer, Arnd Bergmann, Minchan Kim,
	Vineet Gupta, Paul Walmsley, Russell King, Stefan Agner,
	linux-mips, linux-mm, Albert Ou, Paul Mackerras, Nitin Gupta,
	Palmer Dabbelt, linux-riscv, linux-snps-arc, linuxppc-dev,
	Kirill A . Shutemov, linux-arm-kernel
In-Reply-To: <20201113145932.10994-1-arnd@kernel.org>

On Fri, Nov 13, 2020 at 03:59:32PM +0100, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> Stefan Agner reported a bug when using zsram on 32-bit Arm machines
> with RAM above the 4GB address boundary:
> 
>   Unable to handle kernel NULL pointer dereference at virtual address 00000000
>   pgd = a27bd01c
>   [00000000] *pgd=236a0003, *pmd=1ffa64003
>   Internal error: Oops: 207 [#1] SMP ARM
>   Modules linked in: mdio_bcm_unimac(+) brcmfmac cfg80211 brcmutil raspberrypi_hwmon hci_uart crc32_arm_ce bcm2711_thermal phy_generic genet
>   CPU: 0 PID: 123 Comm: mkfs.ext4 Not tainted 5.9.6 #1
>   Hardware name: BCM2711
>   PC is at zs_map_object+0x94/0x338
>   LR is at zram_bvec_rw.constprop.0+0x330/0xa64
>   pc : [<c0602b38>]    lr : [<c0bda6a0>]    psr: 60000013
>   sp : e376bbe0  ip : 00000000  fp : c1e2921c
>   r10: 00000002  r9 : c1dda730  r8 : 00000000
>   r7 : e8ff7a00  r6 : 00000000  r5 : 02f9ffa0  r4 : e3710000
>   r3 : 000fdffe  r2 : c1e0ce80  r1 : ebf979a0  r0 : 00000000
>   Flags: nZCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment user
>   Control: 30c5383d  Table: 235c2a80  DAC: fffffffd
>   Process mkfs.ext4 (pid: 123, stack limit = 0x495a22e6)
>   Stack: (0xe376bbe0 to 0xe376c000)
> 
> As it turns out, zsram needs to know the maximum memory size, which
> is defined in MAX_PHYSMEM_BITS when CONFIG_SPARSEMEM is set, or in
> MAX_POSSIBLE_PHYSMEM_BITS on the x86 architecture.
> 
> The same problem will be hit on all 32-bit architectures that have a
> physical address space larger than 4GB and happen to not enable sparsemem
> and include asm/sparsemem.h from asm/pgtable.h.
> 
> After the initial discussion, I suggested just always defining
> MAX_POSSIBLE_PHYSMEM_BITS whenever CONFIG_PHYS_ADDR_T_64BIT is
> set, or provoking a build error otherwise. This addresses all
> configurations that can currently have this runtime bug, but
> leaves all other configurations unchanged.
> 
> I looked up the possible number of bits in source code and
> datasheets, here is what I found:
> 
>  - on ARC, CONFIG_ARC_HAS_PAE40 controls whether 32 or 40 bits are used
>  - on ARM, CONFIG_LPAE enables 40 bit addressing, without it we never
>    support more than 32 bits, even though supersections in theory allow
>    up to 40 bits as well.
>  - on MIPS, some MIPS32r1 or later chips support 36 bits, and MIPS32r5
>    XPA supports up to 60 bits in theory, but 40 bits are more than
>    anyone will ever ship
>  - On PowerPC, there are three different implementations of 36 bit
>    addressing, but 32-bit is used without CONFIG_PTE_64BIT
>  - On RISC-V, the normal page table format can support 34 bit
>    addressing. There is no highmem support on RISC-V, so anything
>    above 2GB is unused, but it might be useful to eventually support
>    CONFIG_ZRAM for high pages.
> 
> Fixes: 61989a80fb3a ("staging: zsmalloc: zsmalloc memory allocation library")
> Fixes: 02390b87a945 ("mm/zsmalloc: Prepare to variable MAX_PHYSMEM_BITS")
> Cc: Stefan Agner <stefan@agner.ch>
> Cc: Mike Rapoport <rppt@kernel.org>
> Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> Cc: Nitin Gupta <ngupta@vflare.org>
> Cc: Minchan Kim <minchan@kernel.org>
> Cc: Vineet Gupta <vgupta@synopsys.com>
> Cc: linux-snps-arc@lists.infradead.org
> Cc: Russell King <linux@armlinux.org.uk>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> Cc: linux-mips@vger.kernel.org
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: Paul Walmsley <paul.walmsley@sifive.com>
> Cc: Palmer Dabbelt <palmer@dabbelt.com>
> Cc: Albert Ou <aou@eecs.berkeley.edu>
> Cc: linux-riscv@lists.infradead.org
> Link: https://lore.kernel.org/linux-mm/bdfa44bf1c570b05d6c70898e2bbb0acf234ecdf.1604762181.git.stefan@agner.ch/
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Acked-by: Mike Rapoport <rppt@linux.ibm.com>

> ---
> If everyone is happy with this version, I would suggest merging this as
> a bugfix through my asm-generic tree for linux-5.10. I originally
> said I'd send individual patches for each architecture tree, but
> I now think this is easier and better documents what is going on.
> ---
>  arch/arc/include/asm/pgtable.h               |  2 ++
>  arch/arm/include/asm/pgtable-2level.h        |  2 ++
>  arch/arm/include/asm/pgtable-3level.h        |  2 ++
>  arch/mips/include/asm/pgtable-32.h           |  3 +++
>  arch/powerpc/include/asm/book3s/32/pgtable.h |  2 ++
>  arch/powerpc/include/asm/nohash/32/pgtable.h |  2 ++
>  arch/riscv/include/asm/pgtable-32.h          |  2 ++
>  include/linux/pgtable.h                      | 13 +++++++++++++
>  8 files changed, 28 insertions(+)
> 
> diff --git a/arch/arc/include/asm/pgtable.h b/arch/arc/include/asm/pgtable.h
> index f1ed17edb085..163641726a2b 100644
> --- a/arch/arc/include/asm/pgtable.h
> +++ b/arch/arc/include/asm/pgtable.h
> @@ -134,8 +134,10 @@
>  
>  #ifdef CONFIG_ARC_HAS_PAE40
>  #define PTE_BITS_NON_RWX_IN_PD1	(0xff00000000 | PAGE_MASK | _PAGE_CACHEABLE)
> +#define MAX_POSSIBLE_PHYSMEM_BITS 40
>  #else
>  #define PTE_BITS_NON_RWX_IN_PD1	(PAGE_MASK | _PAGE_CACHEABLE)
> +#define MAX_POSSIBLE_PHYSMEM_BITS 32
>  #endif
>  
>  /**************************************************************************
> diff --git a/arch/arm/include/asm/pgtable-2level.h b/arch/arm/include/asm/pgtable-2level.h
> index 3502c2f746ca..baf7d0204eb5 100644
> --- a/arch/arm/include/asm/pgtable-2level.h
> +++ b/arch/arm/include/asm/pgtable-2level.h
> @@ -75,6 +75,8 @@
>  #define PTE_HWTABLE_OFF		(PTE_HWTABLE_PTRS * sizeof(pte_t))
>  #define PTE_HWTABLE_SIZE	(PTRS_PER_PTE * sizeof(u32))
>  
> +#define MAX_POSSIBLE_PHYSMEM_BITS	32
> +
>  /*
>   * PMD_SHIFT determines the size of the area a second-level page table can map
>   * PGDIR_SHIFT determines what a third-level page table entry can map
> diff --git a/arch/arm/include/asm/pgtable-3level.h b/arch/arm/include/asm/pgtable-3level.h
> index fbb6693c3352..2b85d175e999 100644
> --- a/arch/arm/include/asm/pgtable-3level.h
> +++ b/arch/arm/include/asm/pgtable-3level.h
> @@ -25,6 +25,8 @@
>  #define PTE_HWTABLE_OFF		(0)
>  #define PTE_HWTABLE_SIZE	(PTRS_PER_PTE * sizeof(u64))
>  
> +#define MAX_POSSIBLE_PHYSMEM_BITS 40
> +
>  /*
>   * PGDIR_SHIFT determines the size a top-level page table entry can map.
>   */
> diff --git a/arch/mips/include/asm/pgtable-32.h b/arch/mips/include/asm/pgtable-32.h
> index a950fc1ddb4d..6c0532d7b211 100644
> --- a/arch/mips/include/asm/pgtable-32.h
> +++ b/arch/mips/include/asm/pgtable-32.h
> @@ -154,6 +154,7 @@ static inline void pmd_clear(pmd_t *pmdp)
>  
>  #if defined(CONFIG_XPA)
>  
> +#define MAX_POSSIBLE_PHYSMEM_BITS 40
>  #define pte_pfn(x)		(((unsigned long)((x).pte_high >> _PFN_SHIFT)) | (unsigned long)((x).pte_low << _PAGE_PRESENT_SHIFT))
>  static inline pte_t
>  pfn_pte(unsigned long pfn, pgprot_t prot)
> @@ -169,6 +170,7 @@ pfn_pte(unsigned long pfn, pgprot_t prot)
>  
>  #elif defined(CONFIG_PHYS_ADDR_T_64BIT) && defined(CONFIG_CPU_MIPS32)
>  
> +#define MAX_POSSIBLE_PHYSMEM_BITS 36
>  #define pte_pfn(x)		((unsigned long)((x).pte_high >> 6))
>  
>  static inline pte_t pfn_pte(unsigned long pfn, pgprot_t prot)
> @@ -183,6 +185,7 @@ static inline pte_t pfn_pte(unsigned long pfn, pgprot_t prot)
>  
>  #else
>  
> +#define MAX_POSSIBLE_PHYSMEM_BITS 32
>  #ifdef CONFIG_CPU_VR41XX
>  #define pte_pfn(x)		((unsigned long)((x).pte >> (PAGE_SHIFT + 2)))
>  #define pfn_pte(pfn, prot)	__pte(((pfn) << (PAGE_SHIFT + 2)) | pgprot_val(prot))
> diff --git a/arch/powerpc/include/asm/book3s/32/pgtable.h b/arch/powerpc/include/asm/book3s/32/pgtable.h
> index 36443cda8dcf..1376be95e975 100644
> --- a/arch/powerpc/include/asm/book3s/32/pgtable.h
> +++ b/arch/powerpc/include/asm/book3s/32/pgtable.h
> @@ -36,8 +36,10 @@ static inline bool pte_user(pte_t pte)
>   */
>  #ifdef CONFIG_PTE_64BIT
>  #define PTE_RPN_MASK	(~((1ULL << PTE_RPN_SHIFT) - 1))
> +#define MAX_POSSIBLE_PHYSMEM_BITS 36
>  #else
>  #define PTE_RPN_MASK	(~((1UL << PTE_RPN_SHIFT) - 1))
> +#define MAX_POSSIBLE_PHYSMEM_BITS 32
>  #endif
>  
>  /*
> diff --git a/arch/powerpc/include/asm/nohash/32/pgtable.h b/arch/powerpc/include/asm/nohash/32/pgtable.h
> index ee2243ba96cf..96522f7f0618 100644
> --- a/arch/powerpc/include/asm/nohash/32/pgtable.h
> +++ b/arch/powerpc/include/asm/nohash/32/pgtable.h
> @@ -153,8 +153,10 @@ int map_kernel_page(unsigned long va, phys_addr_t pa, pgprot_t prot);
>   */
>  #if defined(CONFIG_PPC32) && defined(CONFIG_PTE_64BIT)
>  #define PTE_RPN_MASK	(~((1ULL << PTE_RPN_SHIFT) - 1))
> +#define MAX_POSSIBLE_PHYSMEM_BITS 36
>  #else
>  #define PTE_RPN_MASK	(~((1UL << PTE_RPN_SHIFT) - 1))
> +#define MAX_POSSIBLE_PHYSMEM_BITS 32
>  #endif
>  
>  /*
> diff --git a/arch/riscv/include/asm/pgtable-32.h b/arch/riscv/include/asm/pgtable-32.h
> index b0ab66e5fdb1..5b2e79e5bfa5 100644
> --- a/arch/riscv/include/asm/pgtable-32.h
> +++ b/arch/riscv/include/asm/pgtable-32.h
> @@ -14,4 +14,6 @@
>  #define PGDIR_SIZE      (_AC(1, UL) << PGDIR_SHIFT)
>  #define PGDIR_MASK      (~(PGDIR_SIZE - 1))
>  
> +#define MAX_POSSIBLE_PHYSMEM_BITS 34
> +
>  #endif /* _ASM_RISCV_PGTABLE_32_H */
> diff --git a/include/linux/pgtable.h b/include/linux/pgtable.h
> index 71125a4676c4..e237004d498d 100644
> --- a/include/linux/pgtable.h
> +++ b/include/linux/pgtable.h
> @@ -1427,6 +1427,19 @@ typedef unsigned int pgtbl_mod_mask;
>  
>  #endif /* !__ASSEMBLY__ */
>  
> +#if !defined(MAX_POSSIBLE_PHYSMEM_BITS) && !defined(CONFIG_64BIT)
> +#ifdef CONFIG_PHYS_ADDR_T_64BIT
> +/*
> + * ZSMALLOC needs to know the highest PFN on 32-bit architectures
> + * with physical address space extension, but falls back to
> + * BITS_PER_LONG otherwise.
> + */
> +#error Missing MAX_POSSIBLE_PHYSMEM_BITS definition
> +#else
> +#define MAX_POSSIBLE_PHYSMEM_BITS 32
> +#endif
> +#endif
> +
>  #ifndef has_transparent_hugepage
>  #ifdef CONFIG_TRANSPARENT_HUGEPAGE
>  #define has_transparent_hugepage() 1
> -- 
> 2.27.0
> 

-- 
Sincerely yours,
Mike.

^ permalink raw reply

* Re: [PATCH v4] PCI: Unify ECAM constants in native PCI Express drivers
From: Krzysztof Wilczyński @ 2020-11-15  5:50 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: Heiko Stuebner, Shawn Lin, Paul Mackerras, Thomas Petazzoni,
	Jonathan Chocron, Toan Le, Will Deacon, Rob Herring,
	Lorenzo Pieralisi, Michal Simek, linux-rockchip,
	bcm-kernel-feedback-list, Jonathan Derrick, linux-pci, Ray Jui,
	linux-rpi-kernel, Jonathan Cameron, Bjorn Helgaas,
	linux-arm-kernel, Scott Branden, Zhou Wang, Robert Richter,
	linuxppc-dev, Nicolas Saenz Julienne
In-Reply-To: <429099a8-5186-40c3-f5c0-f219b3e79f01@gmail.com>

On 20-10-04 19:53:06, Florian Fainelli wrote:

Hi Florian,

Sorry for taking a long time to get back to you.

[...]
> This appears to be correct, so:
> 
> Acked-by: Florian Fainelli <f.fainelli@gmail.com>

Thank you!
 
> however, I would have defined a couple of additional helper macros and do:
> 
> 	idx = PCIE_ECAM_BUS(bus->number) | PCIE_ECAM_DEV(devfn) |
> PCIE_ECAM_FUN(devfn);
> 
> for clarity.
> 
[...]
> For instance, adding these two:
> 
> #define PCIE_ECAM_DEV(x)		(((x) & 0x1f) << PCIE_ECAM_DEV_SHIFT)
> #define PCIE_ECAM_FUN(x)		(((x) & 0x7) << PCIE_ECAM_FUN_SHIFT)
> 
> may be clearer for use in drivers like pcie-brcmstb.c that used to treat the
> device function in terms of device and function (though it was called slot
> there).

Regarding the suggestion above - it has been like that initially, albeit
Bjorn suggested that there is no need to reply on the macros that use
PCI_SLOT() and PCI_FUNC() macros, see:

https://lore.kernel.org/linux-pci/20200922232715.GA2238688@bjorn-Precision-5520/

I would be happy to put the macros back if there is a value in having
the extra macros added - perhaps for clarify, as you suggest.

Krzysztof

^ permalink raw reply

* Re: [PATCH net-next 04/12] ibmvnic: Introduce xmit_more support using batched subCRQ hcalls
From: Jakub Kicinski @ 2020-11-14 23:46 UTC (permalink / raw)
  To: Thomas Falcon
  Cc: cforno12, netdev, ljp, ricklind, dnbanerg, drt, brking, sukadev,
	linuxppc-dev
In-Reply-To: <1605208207-1896-5-git-send-email-tlfalcon@linux.ibm.com>

On Thu, 12 Nov 2020 13:09:59 -0600 Thomas Falcon wrote:
> Include support for the xmit_more feature utilizing the
> H_SEND_SUB_CRQ_INDIRECT hypervisor call which allows the sending
> of multiple subordinate Command Response Queue descriptors in one
> hypervisor call via a DMA-mapped buffer. This update reduces hypervisor
> calls and thus hypervisor call overhead per TX descriptor.
> 
> Signed-off-by: Thomas Falcon <tlfalcon@linux.ibm.com>

The common bug with xmit_more is not flushing the already queued
notifications when there is a drop. Any time you drop a skb you need 
to check it's not an skb that was the end of an xmit_more train and 
if so flush notifications (or just always flush on error).

Looking at the driver e.g. this starting goto:

        if (ibmvnic_xmit_workarounds(skb, netdev)) {                            
                tx_dropped++;                                                   
                tx_send_failed++;                                               
                ret = NETDEV_TX_OK;                                             
                goto out;                                                       
        }  

Does not seem to hit any flush on its way out AFAICS.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox