Re: [PATCH 01/10] ARC: set level of log per CPU during boot to be debug level
Hi Noam, On Thu, 2017-05-25 at 05:34 +0300, Noam Camus wrote: > From: Noam Camus > > The reasons are: > 1) speeding up boot time, becomes critical for many CPUs machine, > e.g. NPS400 with 4K CPUs > 2) shorten kernel log at boot time, again easy to scan for large > scale machines such NPS400 > > Signed-off-by: Noam Camus I do understand your concern on getting 4k instances of pretty much the same boiler plates for each core in NPS. But even in real life those headers provide quite important information on what is user's HW and SW configuration. That said for more generic use-cases especially when we have just a few cores I'd say those messages really make sense. What we may also do for example add "ccflags += -DDEBUG" in arch/arc/Makefile so all pr_debug() instances again start to print messages in console. But do it say only if !EzChip's platform is selected. So it keeps everything as it is for non-EzChip cases and makes you guys happy as you no longer see those 4096 headers. Let's see what Vineet says. -Alexey ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH 06/10] ARC: [plat-eznps] Fix TLB Errata
Hi Noam, On Thu, 2017-05-25 at 05:34 +0300, Noam Camus wrote: > From: Noam Camus > > Due to a HW bug in NPS400 we get from time to time false TLB miss. > Workaround this by validating each miss. > > Signed-off-by: Noam Camus > --- > arch/arc/mm/tlbex.S | 10 ++ > 1 files changed, 10 insertions(+), 0 deletions(-) > > diff --git a/arch/arc/mm/tlbex.S b/arch/arc/mm/tlbex.S > index b30e4e3..1d48723 100644 > --- a/arch/arc/mm/tlbex.S > +++ b/arch/arc/mm/tlbex.S > @@ -274,6 +274,13 @@ ex_saved_reg1: > .macro COMMIT_ENTRY_TO_MMU > #if (CONFIG_ARC_MMU_VER < 4) > > +#ifdef CONFIG_EZNPS_MTM_EXT > + /* verify if entry for this vaddr+ASID already exists */ > + srTLBProbe, [ARC_REG_TLBCOMMAND] > + lrr0, [ARC_REG_TLBINDEX] > + bbit0 r0, 31, 88f > +#endif That's funny. I think we used to have something like that in the past. > /* Get free TLB slot: Set = computed from vaddr, way = random */ > sr TLBGetIndex, [ARC_REG_TLBCOMMAND] > > @@ -287,6 +294,9 @@ ex_saved_reg1: > #else > sr TLBInsertEntry, [ARC_REG_TLBCOMMAND] > #endif > +#ifdef CONFIG_EZNPS_MTM_EXT > +88: > +#endif Not sure if label itself required wrapping in ifdefs. It just makes code bulkier and harder to read. -Alexey ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH 03/10] ARC: typo fix in mm/fault.c
Hi Noam, On Thu, 2017-05-25 at 05:34 +0300, Noam Camus wrote: > From: Liav Rehana > > Signed-off-by: Liav Rehana > Signed-off-by: Noam Camus > --- > arch/arc/mm/fault.c |2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/arch/arc/mm/fault.c b/arch/arc/mm/fault.c > index 162c975..a0b7bd6 100644 > --- a/arch/arc/mm/fault.c > +++ b/arch/arc/mm/fault.c > @@ -207,7 +207,7 @@ void do_page_fault(unsigned long address, struct pt_regs > *regs) > /* Are we prepared to handle this kernel fault? > * > * (The kernel has valid exception-points in the source > - * when it acesses user-memory. When it fails in one > + * when it accesses user-memory. When it fails in one > * of those points, we find it in a table and do a jump > * to some fixup code that loads an appropriate error > * code) Reviewed-by: Alexey Brodkin ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH 04/10] ARC: typos fix in kernel/entry-compact.S
Hi Noam, On Thu, 2017-05-25 at 05:34 +0300, Noam Camus wrote: > From: Liav Rehana > > Signed-off-by: Liav Rehana > Signed-off-by: Noam Camus > --- > arch/arc/kernel/entry-compact.S | 22 +++--- > 1 files changed, 11 insertions(+), 11 deletions(-) Reviewed-by: Alexey Brodkin ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH 05/10] ARC: [plat-eznps] typo fix at Kconfig
Hi Noam, On Thu, 2017-05-25 at 05:34 +0300, Noam Camus wrote: > From: Noam Camus > > Signed-off-by: Noam Camus > --- > arch/arc/plat-eznps/Kconfig |4 ++-- > 1 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/arc/plat-eznps/Kconfig b/arch/arc/plat-eznps/Kconfig > index 1595a38..feaa471 100644 > --- a/arch/arc/plat-eznps/Kconfig > +++ b/arch/arc/plat-eznps/Kconfig > @@ -12,8 +12,8 @@ menuconfig ARC_PLAT_EZNPS > help > Support for EZchip development platforms, > based on ARC700 cores. > - We handle few flavours: > - - Hardware Emulator AKA HE which is FPGA based chasis > + We handle few flavors: Not really sure that change worth a trouble. The point is both options could be used: either "flavour" (Brit-ish) or "flavor" (American-ish). Otherwise... Reviewed-by: Alexey Brodkin ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH 07/10] ARC: [plat-eznps] disabled stall counter due to a HW bug
Hi Noam, On Thu, 2017-05-25 at 05:34 +0300, Noam Camus wrote: > From: Noam Camus > > This counter represents threshold for consecutive stall that which > trigger HW threads scheduling. > Low values of this counter cause downgrade in performance > and in the worst case even a livelock. > > Signed-off-by: Noam Camus > --- > arch/arc/plat-eznps/mtm.c |2 -- > 1 files changed, 0 insertions(+), 2 deletions(-) > > diff --git a/arch/arc/plat-eznps/mtm.c b/arch/arc/plat-eznps/mtm.c > index ffd..e0cb36b 100644 > --- a/arch/arc/plat-eznps/mtm.c > +++ b/arch/arc/plat-eznps/mtm.c > @@ -119,8 +119,6 @@ void mtm_enable_core(unsigned int cpu) > mt_ctrl.value = 0; > mt_ctrl.hsen = 1; > mt_ctrl.hs_cnt = MT_CTRL_HS_CNT; > - mt_ctrl.sten = 1; > - mt_ctrl.st_cnt = MT_CTRL_ST_CNT; Even though I don't know your architecture this change doesn't make enough sense to me in absence of better explanation of what is really done here. I.e. how removal of those 2 lines above improve your situation. -Alexey ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
RE: [PATCH 06/10] ARC: [plat-eznps] Fix TLB Errata
>From: Alexey Brodkin [mailto:alexey.brod...@synopsys.com] >Sent: Thursday, May 25, 2017 14:01 PM ... >> /* Get free TLB slot: Set = computed from vaddr, way = random */ >> sr TLBGetIndex, [ARC_REG_TLBCOMMAND] >> >> @@ -287,6 +294,9 @@ ex_saved_reg1: >> #else >> sr TLBInsertEntry, [ARC_REG_TLBCOMMAND] >> #endif >> +#ifdef CONFIG_EZNPS_MTM_EXT >> +88: >> +#endif >Not sure if label itself required wrapping in ifdefs. It just makes code >bulkier and harder to read. I will remove the wrapping. -Noam ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH 10/10] ARC: [plat-eznps] Handle memory error as an exception
Hi Noam, On Thu, 2017-05-25 at 05:34 +0300, Noam Camus wrote: > From: Noam Camus > > This commit adds the configuration CONFIG_EZNPS_MEM_ERROR. > If set, it will cause the kernel to handle user memory error > as a machine check exception. > It is required in order to align the NPS simulator memory > error handling to the one of the NPS400 real chip behavior. > > Signed-off-by: Elad Kanfi > Signed-off-by: Noam Camus > --- > arch/arc/kernel/entry-compact.S | 11 +++ > arch/arc/plat-eznps/Kconfig | 11 +++ > 2 files changed, 22 insertions(+), 0 deletions(-) > > diff --git a/arch/arc/kernel/entry-compact.S b/arch/arc/kernel/entry-compact.S > index f285dbb..d152d36 100644 > --- a/arch/arc/kernel/entry-compact.S > +++ b/arch/arc/kernel/entry-compact.S > @@ -203,6 +203,17 @@ END(handle_interrupt_level2) > ; - > ENTRY(mem_service) > > +#if defined(CONFIG_EZNPS_MEM_ERROR) > +; SW workaround to cover up on a difference between > +; NPS real chip and simulator behaviors. > +; NPS real chip will activate a machine check exception > +; in case of memory error, while the simulator will > +; trigger a level 2 interrupt. Therefor this code section > +; should be reached only in simulation mode. > +; DEAD END: display Regs and HALT I'm not really buying that. Why don't you just make simulator behaving exactly as your real chip? Adding those stubs for some corner-cases here and there complicate code, affect maintainability etc. -Alexey ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
RE: [PATCH 07/10] ARC: [plat-eznps] disabled stall counter due to a HW bug
>From: Alexey Brodkin [mailto:alexey.brod...@synopsys.com] >Sent: Thursday, May 25, 2017 14:10 PM ... >> diff --git a/arch/arc/plat-eznps/mtm.c b/arch/arc/plat-eznps/mtm.c >> index ffd..e0cb36b 100644 >> --- a/arch/arc/plat-eznps/mtm.c >> +++ b/arch/arc/plat-eznps/mtm.c >> @@ -119,8 +119,6 @@ void mtm_enable_core(unsigned int cpu) >> mt_ctrl.value = 0; >> mt_ctrl.hsen = 1; >> mt_ctrl.hs_cnt = MT_CTRL_HS_CNT; >> -mt_ctrl.sten = 1; >> -mt_ctrl.st_cnt = MT_CTRL_ST_CNT; >Even though I don't know your architecture this change doesn't make enough >sense to me in absence of better explanation of what is really done here. >I.e. how removal of those 2 lines above improve your situation. By removing 2 lines I am resorting to HW reset value where sten=0 i.e. feature is disabled. I will rewrite the explanation -Noam ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
RE: [PATCH 10/10] ARC: [plat-eznps] Handle memory error as an exception
> From: Alexey Brodkin [mailto:alexey.brod...@synopsys.com] > Sent: Thursday, May 25, 2017 14:15 PM >> >> diff --git a/arch/arc/kernel/entry-compact.S >> b/arch/arc/kernel/entry-compact.S index f285dbb..d152d36 100644 >> --- a/arch/arc/kernel/entry-compact.S >> +++ b/arch/arc/kernel/entry-compact.S >> @@ -203,6 +203,17 @@ END(handle_interrupt_level2) >> ; - >> ENTRY(mem_service) >> >> +#if defined(CONFIG_EZNPS_MEM_ERROR) >> +; SW workaround to cover up on a difference between >> +; NPS real chip and simulator behaviors. >> +; NPS real chip will activate a machine check exception >> +; in case of memory error, while the simulator will >> +; trigger a level 2 interrupt. Therefor this code section >> +; should be reached only in simulation mode. >> +; DEAD END: display Regs and HALT >I'm not really buying that. >Why don't you just make simulator behaving exactly as your real chip? I can't change simulator core behavior. nSIM is a Synopsys proprietary code. >Adding those stubs for some corner-cases here and there complicate code, >affect maintainability etc. I agree, any suggestions to still have this but with reduced cost? -Noam ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH 10/10] ARC: [plat-eznps] Handle memory error as an exception
Hi Noam, On Thu, 2017-05-25 at 11:26 +, Noam Camus wrote: > > > > From: Alexey Brodkin [mailto:alexey.brod...@synopsys.com] > > Sent: Thursday, May 25, 2017 14:15 PM > > > > > > > > > > > > diff --git a/arch/arc/kernel/entry-compact.S > > > b/arch/arc/kernel/entry-compact.S index f285dbb..d152d36 100644 > > > --- a/arch/arc/kernel/entry-compact.S > > > +++ b/arch/arc/kernel/entry-compact.S > > > @@ -203,6 +203,17 @@ END(handle_interrupt_level2) > > > ; - > > > ENTRY(mem_service) > > > > > > +#if defined(CONFIG_EZNPS_MEM_ERROR) > > > +; SW workaround to cover up on a difference between > > > +; NPS real chip and simulator behaviors. > > > +; NPS real chip will activate a machine check exception > > > +; in case of memory error, while the simulator will > > > +; trigger a level 2 interrupt. Therefor this code section > > > +; should be reached only in simulation mode. > > > +; DEAD END: display Regs and HALT > > > > > I'm not really buying that. > > > > > Why don't you just make simulator behaving exactly as your real chip? > I can't change simulator core behavior. nSIM is a Synopsys proprietary code. Well probably it worth discussing with nSIM team if they may have any suggestions on how to align nSIM behavior with your real HW? -Alexey ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH 02/10] ARC: send ipi to all cpus sharing task mm in case of page fault
Hi Noam, On Thu, 2017-05-25 at 05:34 +0300, Noam Camus wrote: > From: Noam Camus > > This patch is derived due to performance issue. > The use case is a page fault that resides on more than the local cpu. > Trying to broadcast all CPUs results on performance degradation. > So we try to avoid this by sending only to the relevant CPUs. > > Signed-off-by: Noam Camus Really nice catch! Reviewed-by: Alexey Brodkin ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
RE: [PATCH 10/10] ARC: [plat-eznps] Handle memory error as an exception
>From: Alexey Brodkin [mailto:alexey.brod...@synopsys.com] >Sent: Thursday, May 25, 2017 14:31 PM ... >> > Why don't you just make simulator behaving exactly as your real chip? >> I can't change simulator core behavior. nSIM is a Synopsys proprietary code. >Well probably it worth discussing with nSIM team if they may have any >suggestions on how to align nSIM behavior with your real HW? We already talked with them, nothing we can do at this point. What about turning mem_service into weak symbol and have my own platform copy (just like we do with res_service)? -Noam ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH 10/10] ARC: [plat-eznps] Handle memory error as an exception
Hi Noam, On Thu, 2017-05-25 at 12:03 +, Noam Camus wrote: > > > > From: Alexey Brodkin [mailto:alexey.brod...@synopsys.com] > > Sent: Thursday, May 25, 2017 14:31 PM > ... > > > > > > > > > > > > > Why don't you just make simulator behaving exactly as your real chip? > > > I can't change simulator core behavior. nSIM is a Synopsys proprietary > > > code. > > > > > Well probably it worth discussing with nSIM team if they may have any > > suggestions on how to align nSIM behavior with your real HW? > We already talked with them, nothing we can do at this point. > What about turning mem_service into weak symbol and have my own platform copy > (just like we do with res_service)? That looks nicer to me. Let's wait for Vineet's opinion on that. -Alexey ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH 1/1] futex: remove duplicated code
On Mon, May 22, 2017 at 11:11:33PM +0200, Thomas Gleixner wrote:
> On Mon, 15 May 2017, Will Deacon wrote:
> > On Mon, May 15, 2017 at 03:07:42PM +0200, Jiri Slaby wrote:
> > > There is code duplicated over all architecture's headers for
> > > futex_atomic_op_inuser. Namely op decoding, access_ok check for uaddr,
> > > and comparison of the result.
> > >
> > > Remove this duplication and leave up to the arches only the needed
> > > assembly which is now in arch_futex_atomic_op_inuser.
> > >
> > > Note that s390 removed access_ok check in d12a29703 ("s390/uaccess:
> > > remove pointless access_ok() checks") as access_ok there returns true.
> > > We introduce it back to the helper for the sake of simplicity (it gets
> > > optimized away anyway).
> >
> > Whilst I think this is a good idea, the code in question actually results
> > in undefined behaviour per the C spec and is reported by UBSAN. See my
> > patch fixing arm64 here (which I'd forgotten about):
> >
> > https://www.spinics.net/lists/linux-arch/msg38564.html
> >
> > But, as stated in the thread above, I think we should go a step further
> > and remove FUTEX_OP_{OR,ANDN,XOR,OPARG_SHIFT} altogether. They don't
> > appear to be used by userspace, and this whole thing is a total mess.
>
> You wish. The constants are not used, but FUTEX_WAKE_OP _IS_ used by
> glibc. They only have one argument it seems:
>
>#define FUTEX_OP_CLEAR_WAKE_IF_GT_ONE ((4 << 24) | 1)
>
> but I'm pretty sure that there is enough (probably horrible) code (think
> java) out there using FUTEX_WAKE_OP for whatever (non)sensical reasons in
> any available combination.
Indeed, and I'm not proposing to get rid of that. It's the grossly
over-engineered array of operations and the FUTEX_OP_OPARG_SHIFT modifier
that I think we should kill. The latter likely behaves differently across
different architectures and potentially depending on the toolchain you used
to build the kernel.
Does anybody know the history behind the interface design?
Will
___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
[PATCH] arc: Remove sometimes misleading toolchain check
Thinking of a toolchains for ARCompact and ARCv2 ISAs we implicitly think about libgcc.a build for one of those ISAs which we're linking with. And given there's no multiarch uClibc toolchain for ARC (as probably for any other architecture) the assumption is the only way to get libgcc.a for desired ISA is from a toolchain built right for that same ISA. So what we do we check what's GCC's default architecture ARC700 or not. But generally speaking default arch makes not a lot of sense if explicit command line option exist like "-mcpu=archs". In other words exactly the same GCC might build executables for both ARC700 and ARC HS38. But in real life libgcc could be easily built on a separate step independently of the compiler and friends. And that really happens. For example OpenEmbedded prefers to reuse the same toolchain for both arches having libgcc built separately. Anyways given we have plans to get rid of libgcc dependency that change is sort of future proof. Signed-off-by: Alexey Brodkin --- arch/arc/Makefile | 14 -- 1 file changed, 14 deletions(-) diff --git a/arch/arc/Makefile b/arch/arc/Makefile index 44ef35d33956..d8c99fefaebb 100644 --- a/arch/arc/Makefile +++ b/arch/arc/Makefile @@ -22,20 +22,6 @@ cflags-y += -fno-common -pipe -fno-builtin -D__linux__ cflags-$(CONFIG_ISA_ARCOMPACT) += -mA7 cflags-$(CONFIG_ISA_ARCV2) += -mcpu=archs -is_700 = $(shell $(CC) -dM -E - < /dev/null | grep -q "ARC700" && echo 1 || echo 0) - -ifdef CONFIG_ISA_ARCOMPACT -ifeq ($(is_700), 0) -$(error Toolchain not configured for ARCompact builds) -endif -endif - -ifdef CONFIG_ISA_ARCV2 -ifeq ($(is_700), 1) -$(error Toolchain not configured for ARCv2 builds) -endif -endif - ifdef CONFIG_ARC_CURR_IN_REG # For a global register defintion, make sure it gets passed to every file # We had a customer reported bug where some code built in kernel was NOT using -- 2.7.4 ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH 1/1] futex: remove duplicated code
On Thu, 25 May 2017, Will Deacon wrote:
> On Mon, May 22, 2017 at 11:11:33PM +0200, Thomas Gleixner wrote:
> > On Mon, 15 May 2017, Will Deacon wrote:
> > > On Mon, May 15, 2017 at 03:07:42PM +0200, Jiri Slaby wrote:
> > > > There is code duplicated over all architecture's headers for
> > > > futex_atomic_op_inuser. Namely op decoding, access_ok check for uaddr,
> > > > and comparison of the result.
> > > >
> > > > Remove this duplication and leave up to the arches only the needed
> > > > assembly which is now in arch_futex_atomic_op_inuser.
> > > >
> > > > Note that s390 removed access_ok check in d12a29703 ("s390/uaccess:
> > > > remove pointless access_ok() checks") as access_ok there returns true.
> > > > We introduce it back to the helper for the sake of simplicity (it gets
> > > > optimized away anyway).
> > >
> > > Whilst I think this is a good idea, the code in question actually results
> > > in undefined behaviour per the C spec and is reported by UBSAN. See my
> > > patch fixing arm64 here (which I'd forgotten about):
> > >
> > > https://www.spinics.net/lists/linux-arch/msg38564.html
> > >
> > > But, as stated in the thread above, I think we should go a step further
> > > and remove FUTEX_OP_{OR,ANDN,XOR,OPARG_SHIFT} altogether. They don't
> > > appear to be used by userspace, and this whole thing is a total mess.
> >
> > You wish. The constants are not used, but FUTEX_WAKE_OP _IS_ used by
> > glibc. They only have one argument it seems:
> >
> >#define FUTEX_OP_CLEAR_WAKE_IF_GT_ONE ((4 << 24) | 1)
> >
> > but I'm pretty sure that there is enough (probably horrible) code (think
> > java) out there using FUTEX_WAKE_OP for whatever (non)sensical reasons in
> > any available combination.
>
> Indeed, and I'm not proposing to get rid of that. It's the grossly
> over-engineered array of operations and the FUTEX_OP_OPARG_SHIFT modifier
> that I think we should kill. The latter likely behaves differently across
> different architectures and potentially depending on the toolchain you used
> to build the kernel.
>
> Does anybody know the history behind the interface design?
Which design?
4732efbeb997 ("[PATCH] FUTEX_WAKE_OP: pthread_cond_signal() speedup")
Thanks,
tglx
___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
