Re: [PATCH] fsldma: fsl_ioread64*() do not need lower_32_bits()
On Sat, Aug 29, 2020 at 10:29:55AM -0700, Linus Torvalds wrote: > On Sat, Aug 29, 2020 at 5:46 AM Luc Van Oostenryck > wrote: > > > > But the pointer is already 32-bit, so simply cast the pointer to u32. > > Yeah, that code was completely pointless. If the pointer had actually > been 64-bit, the old code would have warned too. > > The odd thing is that the fsl_iowrite64() functions make sense. It's > only the fsl_ioread64() functions that seem to be written by somebody > who is really confused. > > That said, this patch only humors the confusion. The cast to 'u32' is > completely pointless. In fact, it seems to be actively wrong, because > it means that the later "fsl_addr + 1" is done entirely incorrectly - > it now literally adds "1" to an integer value, while the iowrite() > functions will add one to a "u32 __iomem *" pointer (so will do > pointer arithmetic, and add 4). > My bad. I had noticed the '+ 1' and so automatically assumed 'OK, pointer arithmetic now' without noticing that the cast was done only after the addition. Grrr. FWIW, the version you committed looks much better to me. -- Luc
[PATCH] fsldma: fsl_ioread64*() do not need lower_32_bits()
For ppc32, the functions fsl_ioread64() & fsl_ioread64be()
use lower_32_bits() as a fancy way to cast the pointer to u32
in order to do non-atomic 64-bit IO.
But the pointer is already 32-bit, so simply cast the pointer to u32.
This fixes a compile error introduced by
ef91bb196b0d ("kernel.h: Silence sparse warning in lower_32_bits")
Fixes: ef91bb196b0db1013ef8705367bc2d7944ef696b
Reported-by: Guenter Roeck
Cc: Li Yang
Cc: Zhang Wei
Cc: Dan Williams
Cc: Vinod Koul
Cc: Herbert Xu
Cc: linuxppc-dev@lists.ozlabs.org
Cc: dmaeng...@vger.kernel.org
Cc: linux-ker...@vger.kernel.org
Signed-off-by: Luc Van Oostenryck
---
drivers/dma/fsldma.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/dma/fsldma.h b/drivers/dma/fsldma.h
index 56f18ae99233..6f6fa7641fa2 100644
--- a/drivers/dma/fsldma.h
+++ b/drivers/dma/fsldma.h
@@ -205,7 +205,7 @@ struct fsldma_chan {
#else
static u64 fsl_ioread64(const u64 __iomem *addr)
{
- u32 fsl_addr = lower_32_bits(addr);
+ u32 fsl_addr = (u32) addr;
u64 fsl_addr_hi = (u64)in_le32((u32 *)(fsl_addr + 1)) << 32;
return fsl_addr_hi | in_le32((u32 *)fsl_addr);
@@ -219,7 +219,7 @@ static void fsl_iowrite64(u64 val, u64 __iomem *addr)
static u64 fsl_ioread64be(const u64 __iomem *addr)
{
- u32 fsl_addr = lower_32_bits(addr);
+ u32 fsl_addr = (u32) addr;
u64 fsl_addr_hi = (u64)in_be32((u32 *)fsl_addr) << 32;
return fsl_addr_hi | in_be32((u32 *)(fsl_addr + 1));
--
2.28.0
Re: [PATCH v1 16/46] powerpc/mm: Allocate static page tables for fixmap
On Tue, Mar 17, 2020 at 03:38:46PM +0100, Christophe Leroy wrote: > > diff --git a/arch/powerpc/mm/pgtable_32.c b/arch/powerpc/mm/pgtable_32.c > > index f62de06e3d07..9934659cb871 100644 > > --- a/arch/powerpc/mm/pgtable_32.c > > +++ b/arch/powerpc/mm/pgtable_32.c > > @@ -29,11 +29,27 @@ > > #include > > #include > > #include > > +#include > > #include > > extern char etext[], _stext[], _sinittext[], _einittext[]; > > +static u8 early_fixmap_pagetable[FIXMAP_PTE_SIZE] __page_aligned_data; > > Sparse reports this as a variable size array. This is definitely not. Gcc > properly sees it is an 8k table (2 pages). Yes, thing is that FIXMAP_PTE_SIZE is not that constant since it uses __builtin_ffs() (via PTE_SHIFT / PTE_T_LOG2). Nevertheless, since Sparse v0.6.1 (released in October) accepts these in constant expressions, like GCC does. -- Luc
Re: READ_ONCE() + STACKPROTECTOR_STRONG == :/ (was Re: [GIT PULL] Please pull powerpc/linux.git powerpc-5.5-2 tag (topic/kasan-bitops))
On Fri, Dec 13, 2019 at 01:56:18PM +0100, Peter Zijlstra wrote:
>
> Excellent! I had to change it to something like:
>
> #define unqual_typeof(x)typeof(({_Atomic typeof(x) ___x __maybe_unused;
> ___x; }))
>
> but that does indeed work!
>
> Now I suppose we should wrap that in a symbol that indicates our
> compiler does indeed support _Atomic, otherwise things will come apart.
>
> That is, my gcc-4.6 doesn't seem to have it, while gcc-4.8 does, which
> is exactly the range that needs the daft READ_ONCE() construct, how
> convenient :/
>
> Something a little like this perhaps?
Yes, this looks good to me.
Just a small nit here below.
> ---
>
> diff --git a/arch/arm64/include/asm/barrier.h
> b/arch/arm64/include/asm/barrier.h
> index 7d9cc5ec4971..c389af602da8 100644
> --- a/arch/arm64/include/asm/barrier.h
> +++ b/arch/arm64/include/asm/barrier.h
> @@ -75,9 +75,9 @@ static inline unsigned long
> array_index_mask_nospec(unsigned long idx,
>
> #define __smp_store_release(p, v)\
> do { \
> - typeof(p) __p = (p);\
> - union { typeof(*p) __val; char __c[1]; } __u = \
> - { .__val = (__force typeof(*p)) (v) }; \
> + unqual_typeof(p) __p = (p); \
> + union { unqual_typeof(*p) __val; char __c[1]; } __u = \
> + { .__val = (__force unqual_typeof(*p)) (v) }; \
The 2 two trailing backslashes are now off by one tab.
-- Luc
Re: READ_ONCE() + STACKPROTECTOR_STRONG == :/ (was Re: [GIT PULL] Please pull powerpc/linux.git powerpc-5.5-2 tag (topic/kasan-bitops))
On Thu, Dec 12, 2019 at 09:53:38PM +0100, Peter Zijlstra wrote:
> Now, looking at the current GCC source:
>
>
> https://github.com/gcc-mirror/gcc/blob/97d7270f894395e513667a031a0c309d1819d05e/gcc/c/c-parser.c#L3707
>
> it seems that __typeof__() is supposed to strip all qualifiers from
> _Atomic types. That lead me to try:
>
> typeof(_Atomic typeof(p)) __p = (p);
>
> But alas, I still get the same junk you got for ool_store_release() :/
I was checking this to see if Sparse was ready to support this.
I was a bit surprised because at first sigth GCC was doing as
it claims (typeof striping const & volatile on _Atomic types)
but your exampe wasn't working. But it's working if an
intermediate var is used:
_Atomic typeof(p) tmp;
typeof(tmp) __p = (p);
or, uglier but probably more practical:
typeof(({_Atomic typeof(p) tmp; })) __p = (p);
Go figure!
OTOH, at least on GCC 8.3, it seems to always do the same with
volatiles than it does with consts.
-- Luc Van Oostenryck
Re: [PATCH v2 26/35] powerpc/64: system call: Fix sparse warning about missing declaration
On Tue, Nov 26, 2019 at 09:13:40PM +0100, Michal Suchanek wrote: > Sparse warns about missing declarations for these functions: > > +arch/powerpc/kernel/syscall_64.c:108:23: warning: symbol > 'syscall_exit_prepare' was not declared. Should it be static? > +arch/powerpc/kernel/syscall_64.c:18:6: warning: symbol > 'system_call_exception' was not declared. Should it be static? > +arch/powerpc/kernel/syscall_64.c:200:23: warning: symbol > 'interrupt_exit_user_prepare' was not declared. Should it be static? > +arch/powerpc/kernel/syscall_64.c:288:23: warning: symbol > 'interrupt_exit_kernel_prepare' was not declared. Should it be static? > > Add declaration for them. I'm fine with this patch but, just FYI, lately people seems to prefer to add '__visible' to the function definition instead of creating such header files. Best regards, -- Luc Van Oostenryck
Re: Conflict between sparse and commit cafa0010cd51f ("Raise the minimum required gcc version to 4.6")
On Mon, Sep 10, 2018 at 04:05:34PM +0200, Christophe LEROY wrote: > > This time it works, thanks for your help. You're welcome. > Should we find a may to automate that in the Makefile when > CROSS_COMPILE is defined ? The situation here with an old gcc is really an oddity. I was instead thinking to update sparse so that it repports a GCC version of at least 4.6, maybe something even more recent. But maybe, yes, kbuild could pass GCC_VERSION to sparse so that both the compiler used and sparse will repport the same. I'll see. The problem is not tied to cross-compilation, though, just that sparse may be compiled with an older compiler. -- Luc
Re: Conflict between sparse and commit cafa0010cd51f ("Raise the minimum required gcc version to 4.6")
On Mon, Sep 10, 2018 at 01:19:07PM +, Christophe Leroy wrote: > > > On 09/10/2018 11:34 AM, Luc Van Oostenryck wrote: > > On Mon, Sep 10, 2018 at 09:56:33AM +, Christophe Leroy wrote: > > > > > > # export REAL_CC=ppc-linux-gcc > > > # make CHECK="cgcc -target=ppc -D_CALL_ELF=2 -D__GCC__=5 > > > -D__GCC_MINOR__=4" C=2 arch/powerpc/kernel/process.o > > > scripts/kconfig/conf --syncconfig Kconfig > > > # > > > # configuration written to .config > > > # > > >UPD include/config/kernel.release > > >UPD include/generated/utsrelease.h > > >CC kernel/bounds.s > > >CC arch/powerpc/kernel/asm-offsets.s > > >CALLscripts/checksyscalls.sh > > >CHECK scripts/mod/empty.c > > > Can't exec "/bin/sh": Argument list too long at /usr/local/bin/cgcc line > > > 86. > > > make[2]: *** [scripts/mod/empty.o] Error 1 > > > make[1]: *** [scripts/mod] Error 2 > > > make: *** [scripts] Error 2 > > > > OK. Clearly nobody has ever used it so :( > > There is an infinite loop because cgcc use the env var CHECK > > to call sparse while kbuild use CHECK to call cgcc here. > > > > The following seems to work here. > > $ export REAL_CC=ppc-linux-gcc > > $ make CHECK="CHECK=sparse cgcc -target=ppc ... > > Not yet ... > > [root@pc16082vm linux-powerpc]# export REAL_CC=ppc-linux-gcc > [root@pc16082vm linux-powerpc]# make CHECK="CHECK=sparse cgcc > -target=ppc -D_CALL_ELF=2 -D__GNUC__=5 -D__GNUC_MINOR__=4" C=2 > arch/powerpc/kernel/process.o > CALLscripts/checksyscalls.sh > CHECK scripts/mod/empty.c > :0:0: warning: "__STDC__" redefined > : note: this is the location of the previous definition > /opt/cldk-1.4.0/lib/gcc/ppc-linux/5.4.0/../../../../ppc-linux/lib/crt1.o:(.rodata+0x4): > undefined reference to `main' > collect2: error: ld returned 1 exit status > make[2]: *** [scripts/mod/empty.o] Error 1 > make[1]: *** [scripts/mod] Error 2 > make: *** [scripts] Error 2 OK. Using cgcc creates more problems that it solves and this file scripts/mod/empty.c is weird. Dropping cgcc and simply giving the GCC version to sparse works for me here (the needed defines are given by arch/powerpc/Makefile) but for sure I don't have the same environment as you have: $ make CHECK="sparse -D__GNUC__=5 -D__GNUC_MINOR__=4" ... Bonne chance, -- Luc
Re: Conflict between sparse and commit cafa0010cd51f ("Raise the minimum required gcc version to 4.6")
On Mon, Sep 10, 2018 at 09:56:33AM +, Christophe Leroy wrote: > > # export REAL_CC=ppc-linux-gcc > # make CHECK="cgcc -target=ppc -D_CALL_ELF=2 -D__GCC__=5 > -D__GCC_MINOR__=4" C=2 arch/powerpc/kernel/process.o > scripts/kconfig/conf --syncconfig Kconfig > # > # configuration written to .config > # > UPD include/config/kernel.release > UPD include/generated/utsrelease.h > CC kernel/bounds.s > CC arch/powerpc/kernel/asm-offsets.s > CALLscripts/checksyscalls.sh > CHECK scripts/mod/empty.c > Can't exec "/bin/sh": Argument list too long at /usr/local/bin/cgcc line 86. > make[2]: *** [scripts/mod/empty.o] Error 1 > make[1]: *** [scripts/mod] Error 2 > make: *** [scripts] Error 2 OK. Clearly nobody has ever used it so :( There is an infinite loop because cgcc use the env var CHECK to call sparse while kbuild use CHECK to call cgcc here. The following seems to work here. $ export REAL_CC=ppc-linux-gcc $ make CHECK="CHECK=sparse cgcc -target=ppc ... It's a bit kludgy, I admit. -- Luc
Re: [PATCH] perf: enum overflow in uapi/linux/perf_event.h
On Fri, Sep 07, 2018 at 08:43:59PM +0200, Luc Van Oostenryck wrote: > On Fri, Sep 07, 2018 at 04:15:33PM +0200, Christophe LEROY wrote: > > Le 07/09/2018 à 15:58, Peter Zijlstra a écrit : > > > On Fri, Sep 07, 2018 at 01:50:18PM +, Christophe Leroy wrote: > > > > > > > > > > > > On 09/07/2018 01:42 PM, Peter Zijlstra wrote: > > > > > On Fri, Sep 07, 2018 at 01:27:19PM +, Christophe Leroy wrote: > > > > > > On PPC32, enums are 32 bits, so __PERF_SAMPLE_CALLCHAIN_EARLY is > > > > > > out of scope. The following sparse warning is encountered: > > > > > > > > > > > > CHECK arch/powerpc/kernel/process.c > > > > > > ./include/uapi/linux/perf_event.h:147:56: warning: cast truncates > > > > > > bits from constant value (8000 becomes 0) > > > > > > > > > > Urgh... what compiler is that? I've not seen anything like that from > > > > > the > > > > > build bots. > > > > > > > > > > > > > [root@pc16082vm linux-powerpc]# sparse --version > > > > 0.5.2 > > > > > > > > [root@pc16082vm linux-powerpc]# ppc-linux-gcc --version > > > > ppc-linux-gcc (GCC) 5.4.0 > > > > > > Ah, that's a sparse warning. But does your GCC agree? The thing is, > > > sparse uses the C enum spec, but I suspect GCC uses the C++ enum spec > > > and it all works fine. > > Sparse is a bit weird about the exact underlying type used for enums. > > > Ah yes, it seems that GCC is happy. So sparse should be fixed instead ? > > I'll investigate (I suppose the same is given on x86-32). It's definitively a bug in sparse. A relatively nasty one and which open a can of worms. Fortunately, I had already looked at these problems in May, I just didn't had the time to push the patches. -- Luc
Re: [PATCH] perf: enum overflow in uapi/linux/perf_event.h
On Fri, Sep 07, 2018 at 04:15:33PM +0200, Christophe LEROY wrote: > Le 07/09/2018 à 15:58, Peter Zijlstra a écrit : > > On Fri, Sep 07, 2018 at 01:50:18PM +, Christophe Leroy wrote: > > > > > > > > > On 09/07/2018 01:42 PM, Peter Zijlstra wrote: > > > > On Fri, Sep 07, 2018 at 01:27:19PM +, Christophe Leroy wrote: > > > > > On PPC32, enums are 32 bits, so __PERF_SAMPLE_CALLCHAIN_EARLY is > > > > > out of scope. The following sparse warning is encountered: > > > > > > > > > > CHECK arch/powerpc/kernel/process.c > > > > > ./include/uapi/linux/perf_event.h:147:56: warning: cast truncates > > > > > bits from constant value (8000 becomes 0) > > > > > > > > Urgh... what compiler is that? I've not seen anything like that from the > > > > build bots. > > > > > > > > > > [root@pc16082vm linux-powerpc]# sparse --version > > > 0.5.2 > > > > > > [root@pc16082vm linux-powerpc]# ppc-linux-gcc --version > > > ppc-linux-gcc (GCC) 5.4.0 > > > > Ah, that's a sparse warning. But does your GCC agree? The thing is, > > sparse uses the C enum spec, but I suspect GCC uses the C++ enum spec > > and it all works fine. Sparse is a bit weird about the exact underlying type used for enums. > Ah yes, it seems that GCC is happy. So sparse should be fixed instead ? I'll investigate (I suppose the same is given on x86-32). -- Luc
Re: [Y2038] [PATCH 04/11] posix timers:Introduce the 64bit methods with timespec64 type for k_clock structure
On Wed, Apr 22, 2015 at 03:50:45PM +0200, Arnd Bergmann wrote: > On Wednesday 22 April 2015 13:07:44 Arnd Bergmann wrote: ... > select, old_selct, pselect6: deprecated Small, copy&paste error here for pselect6. Luc ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
