* Re: Buggy commit tracked to: "Re: [PATCH 2/9] iov_iter: move rw_copy_check_uvector() into lib/iov_iter.c"
From: Segher Boessenkool @ 2020-10-23 18:27 UTC (permalink / raw)
To: Al Viro
Cc: linux-aio@kvack.org, David Hildenbrand,
linux-mips@vger.kernel.org, David Howells, linux-mm@kvack.org,
keyrings@vger.kernel.org, sparclinux@vger.kernel.org,
Christoph Hellwig, linux-arch@vger.kernel.org,
linux-s390@vger.kernel.org, linux-scsi@vger.kernel.org,
kernel-team@android.com, Arnd Bergmann,
linux-block@vger.kernel.org, io-uring@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, Jens Axboe,
linux-parisc@vger.kernel.org, 'Greg KH', Nick Desaulniers,
linux-kernel@vger.kernel.org,
linux-security-module@vger.kernel.org, David Laight,
netdev@vger.kernel.org, linux-fsdevel@vger.kernel.org,
Andrew Morton, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <20201023175857.GA3576660@ZenIV.linux.org.uk>
On Fri, Oct 23, 2020 at 06:58:57PM +0100, Al Viro wrote:
> On Fri, Oct 23, 2020 at 03:09:30PM +0200, David Hildenbrand wrote:
>
> > Now, I am not a compiler expert, but as I already cited, at least on
> > x86-64 clang expects that the high bits were cleared by the caller - in
> > contrast to gcc. I suspect it's the same on arm64, but again, I am no
> > compiler expert.
> >
> > If what I said and cites for x86-64 is correct, if the function expects
> > an "unsigned int", it will happily use 64bit operations without further
> > checks where valid when assuming high bits are zero. That's why even
> > converting everything to "unsigned int" as proposed by me won't work on
> > clang - it assumes high bits are zero (as indicated by Nick).
> >
> > As I am neither a compiler experts (did I mention that already? ;) ) nor
> > an arm64 experts, I can't tell if this is a compiler BUG or not.
>
> On arm64 when callee expects a 32bit argument, the caller is *not* responsible
> for clearing the upper half of 64bit register used to pass the value - it only
> needs to store the actual value into the lower half. The callee must consider
> the contents of the upper half of that register as undefined. See AAPCS64 (e.g.
> https://github.com/ARM-software/abi-aa/blob/master/aapcs64/aapcs64.rst#parameter-passing-rules
> ); AFAICS, the relevant bit is
> "Unlike in the 32-bit AAPCS, named integral values must be narrowed by
> the callee rather than the caller."
Or the formal rule:
C.9 If the argument is an Integral or Pointer Type, the size of the
argument is less than or equal to 8 bytes and the NGRN is less
than 8, the argument is copied to the least significant bits in
x[NGRN]. The NGRN is incremented by one. The argument has now
been allocated.
Segher
^ permalink raw reply
* Re: Buggy commit tracked to: "Re: [PATCH 2/9] iov_iter: move rw_copy_check_uvector() into lib/iov_iter.c"
From: Al Viro @ 2020-10-23 17:58 UTC (permalink / raw)
To: David Hildenbrand
Cc: linux-aio@kvack.org, linux-mips@vger.kernel.org, David Howells,
linux-mm@kvack.org, keyrings@vger.kernel.org,
sparclinux@vger.kernel.org, Christoph Hellwig,
linux-arch@vger.kernel.org, linux-s390@vger.kernel.org,
linux-scsi@vger.kernel.org, kernel-team@android.com,
Arnd Bergmann, linux-block@vger.kernel.org,
io-uring@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
Jens Axboe, linux-parisc@vger.kernel.org, 'Greg KH',
Nick Desaulniers, linux-kernel@vger.kernel.org,
linux-security-module@vger.kernel.org, David Laight,
netdev@vger.kernel.org, linux-fsdevel@vger.kernel.org,
Andrew Morton, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <935f7168-c2f5-dd14-7124-412b284693a2@redhat.com>
On Fri, Oct 23, 2020 at 03:09:30PM +0200, David Hildenbrand wrote:
> Now, I am not a compiler expert, but as I already cited, at least on
> x86-64 clang expects that the high bits were cleared by the caller - in
> contrast to gcc. I suspect it's the same on arm64, but again, I am no
> compiler expert.
>
> If what I said and cites for x86-64 is correct, if the function expects
> an "unsigned int", it will happily use 64bit operations without further
> checks where valid when assuming high bits are zero. That's why even
> converting everything to "unsigned int" as proposed by me won't work on
> clang - it assumes high bits are zero (as indicated by Nick).
>
> As I am neither a compiler experts (did I mention that already? ;) ) nor
> an arm64 experts, I can't tell if this is a compiler BUG or not.
On arm64 when callee expects a 32bit argument, the caller is *not* responsible
for clearing the upper half of 64bit register used to pass the value - it only
needs to store the actual value into the lower half. The callee must consider
the contents of the upper half of that register as undefined. See AAPCS64 (e.g.
https://github.com/ARM-software/abi-aa/blob/master/aapcs64/aapcs64.rst#parameter-passing-rules
); AFAICS, the relevant bit is
"Unlike in the 32-bit AAPCS, named integral values must be narrowed by
the callee rather than the caller."
^ permalink raw reply
* Re: Buggy commit tracked to: "Re: [PATCH 2/9] iov_iter: move rw_copy_check_uvector() into lib/iov_iter.c"
From: David Hildenbrand @ 2020-10-23 16:33 UTC (permalink / raw)
To: 'Greg KH', David Laight
Cc: linux-aio@kvack.org, linux-mips@vger.kernel.org, David Howells,
linux-mm@kvack.org, keyrings@vger.kernel.org,
sparclinux@vger.kernel.org, Christoph Hellwig,
linux-arch@vger.kernel.org, linux-s390@vger.kernel.org,
linux-scsi@vger.kernel.org, kernel-team@android.com,
Arnd Bergmann, linux-block@vger.kernel.org, Al Viro,
io-uring@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
Jens Axboe, linux-parisc@vger.kernel.org, netdev@vger.kernel.org,
Nick Desaulniers, linux-kernel@vger.kernel.org,
linux-security-module@vger.kernel.org,
linux-fsdevel@vger.kernel.org, Andrew Morton,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <20201023144718.GA2525489@kroah.com>
On 23.10.20 16:47, 'Greg KH' wrote:
> On Fri, Oct 23, 2020 at 02:39:24PM +0000, David Laight wrote:
>> From: David Hildenbrand
>>> Sent: 23 October 2020 15:33
>> ...
>>> I just checked against upstream code generated by clang 10 and it
>>> properly discards the upper 32bit via a mov w23 w2.
>>>
>>> So at least clang 10 indeed properly assumes we could have garbage and
>>> masks it off.
>>>
>>> Maybe the issue is somewhere else, unrelated to nr_pages ... or clang 11
>>> behaves differently.
>>
>> We'll need the disassembly from a failing kernel image.
>> It isn't that big to hand annotate.
>
> I've worked around the merge at the moment in the android tree, but it
> is still quite reproducable, and will try to get a .o file to
> disassemble on Monday or so...
I just compiled pre and post fb041b598997d63c0f7d7305dfae70046bf66fe1 with
clang version 11.0.0 (Fedora 11.0.0-0.2.rc1.fc33)
for aarch64 with defconfig and extracted import_iovec and
rw_copy_check_uvector (skipping the compat things)
Pre fb041b598997d63c0f7d7305dfae70046bf66fe1 import_iovec
-> https://pastebin.com/LtnYMLJt
Post fb041b598997d63c0f7d7305dfae70046bf66fe1 import_iovec
-> https://pastebin.com/BWPmXrAf
Pre fb041b598997d63c0f7d7305dfae70046bf66fe1 rw_copy_check_uvector
-> https://pastebin.com/4nSBYRbf
Post fb041b598997d63c0f7d7305dfae70046bf66fe1 rw_copy_check_uvector
-> https://pastebin.com/hPtEgaEW
I'm only able to spot minor differences ... less gets inlined than I
would have expected. But there are some smaller differences.
Maybe someone wants to have a look before we have object files as used
by Greg ...
--
Thanks,
David / dhildenb
^ permalink raw reply
* Re: Buggy commit tracked to: "Re: [PATCH 2/9] iov_iter: move rw_copy_check_uvector() into lib/iov_iter.c"
From: 'Greg KH' @ 2020-10-23 14:47 UTC (permalink / raw)
To: David Laight
Cc: linux-aio@kvack.org, 'David Hildenbrand',
linux-mips@vger.kernel.org, David Howells, linux-mm@kvack.org,
keyrings@vger.kernel.org, sparclinux@vger.kernel.org,
Christoph Hellwig, linux-arch@vger.kernel.org,
linux-s390@vger.kernel.org, linux-scsi@vger.kernel.org,
kernel-team@android.com, Arnd Bergmann,
linux-block@vger.kernel.org, Al Viro, io-uring@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, Jens Axboe,
linux-parisc@vger.kernel.org, netdev@vger.kernel.org,
Nick Desaulniers, linux-kernel@vger.kernel.org,
linux-security-module@vger.kernel.org,
linux-fsdevel@vger.kernel.org, Andrew Morton,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <35d0ec90ef4f4a35a75b9df7d791f719@AcuMS.aculab.com>
On Fri, Oct 23, 2020 at 02:39:24PM +0000, David Laight wrote:
> From: David Hildenbrand
> > Sent: 23 October 2020 15:33
> ...
> > I just checked against upstream code generated by clang 10 and it
> > properly discards the upper 32bit via a mov w23 w2.
> >
> > So at least clang 10 indeed properly assumes we could have garbage and
> > masks it off.
> >
> > Maybe the issue is somewhere else, unrelated to nr_pages ... or clang 11
> > behaves differently.
>
> We'll need the disassembly from a failing kernel image.
> It isn't that big to hand annotate.
I've worked around the merge at the moment in the android tree, but it
is still quite reproducable, and will try to get a .o file to
disassemble on Monday or so...
thanks,
greg k-h
^ permalink raw reply
* RE: Buggy commit tracked to: "Re: [PATCH 2/9] iov_iter: move rw_copy_check_uvector() into lib/iov_iter.c"
From: David Laight @ 2020-10-23 14:39 UTC (permalink / raw)
To: 'David Hildenbrand', 'Greg KH'
Cc: linux-aio@kvack.org, linux-mips@vger.kernel.org, David Howells,
linux-mm@kvack.org, keyrings@vger.kernel.org,
sparclinux@vger.kernel.org, Christoph Hellwig,
linux-arch@vger.kernel.org, linux-s390@vger.kernel.org,
linux-scsi@vger.kernel.org, kernel-team@android.com,
Arnd Bergmann, linux-block@vger.kernel.org, Al Viro,
io-uring@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
Jens Axboe, linux-parisc@vger.kernel.org, netdev@vger.kernel.org,
Nick Desaulniers, linux-kernel@vger.kernel.org,
linux-security-module@vger.kernel.org,
linux-fsdevel@vger.kernel.org, Andrew Morton,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <999e2926-9a75-72fd-007a-1de0af341292@redhat.com>
From: David Hildenbrand
> Sent: 23 October 2020 15:33
...
> I just checked against upstream code generated by clang 10 and it
> properly discards the upper 32bit via a mov w23 w2.
>
> So at least clang 10 indeed properly assumes we could have garbage and
> masks it off.
>
> Maybe the issue is somewhere else, unrelated to nr_pages ... or clang 11
> behaves differently.
We'll need the disassembly from a failing kernel image.
It isn't that big to hand annotate.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
^ permalink raw reply
* Re: Buggy commit tracked to: "Re: [PATCH 2/9] iov_iter: move rw_copy_check_uvector() into lib/iov_iter.c"
From: David Hildenbrand @ 2020-10-23 14:33 UTC (permalink / raw)
To: David Laight, 'Greg KH'
Cc: linux-aio@kvack.org, linux-mips@vger.kernel.org, David Howells,
linux-mm@kvack.org, keyrings@vger.kernel.org,
sparclinux@vger.kernel.org, Christoph Hellwig,
linux-arch@vger.kernel.org, linux-s390@vger.kernel.org,
linux-scsi@vger.kernel.org, kernel-team@android.com,
Arnd Bergmann, linux-block@vger.kernel.org, Al Viro,
io-uring@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
Jens Axboe, linux-parisc@vger.kernel.org, netdev@vger.kernel.org,
Nick Desaulniers, linux-kernel@vger.kernel.org,
linux-security-module@vger.kernel.org,
linux-fsdevel@vger.kernel.org, Andrew Morton,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <935f7168-c2f5-dd14-7124-412b284693a2@redhat.com>
On 23.10.20 15:09, David Hildenbrand wrote:
> On 23.10.20 14:46, David Laight wrote:
>> From: Greg KH <gregkh@linuxfoundation.org>
>>> Sent: 22 October 2020 14:51
>>
>> I've rammed the code into godbolt.
>>
>> https://godbolt.org/z/9v5PPW
>>
>> Definitely a clang bug.
>>
>> Search for [wx]24 in the clang output.
>> nr_segs comes in as w2 and the initial bound checks are done on w2.
>> w24 is loaded from w2 - I don't believe this changes the high bits.
>> There are no references to w24, just x24.
>> So the kmalloc_array() is passed 'huge' and will fail.
>> The iov_iter_init also gets the 64bit value.
>>
>> Note that the gcc code has a sign-extend copy of w2.
>
> Do we have a result from using "unsigned long" in the base function and
> explicitly masking of the high bits? That should definitely work.
>
> Now, I am not a compiler expert, but as I already cited, at least on
> x86-64 clang expects that the high bits were cleared by the caller - in
> contrast to gcc. I suspect it's the same on arm64, but again, I am no
> compiler expert.
>
> If what I said and cites for x86-64 is correct, if the function expects
> an "unsigned int", it will happily use 64bit operations without further
> checks where valid when assuming high bits are zero. That's why even
> converting everything to "unsigned int" as proposed by me won't work on
> clang - it assumes high bits are zero (as indicated by Nick).
>
> As I am neither a compiler experts (did I mention that already? ;) ) nor
> an arm64 experts, I can't tell if this is a compiler BUG or not.
>
I just checked against upstream code generated by clang 10 and it
properly discards the upper 32bit via a mov w23 w2.
So at least clang 10 indeed properly assumes we could have garbage and
masks it off.
Maybe the issue is somewhere else, unrelated to nr_pages ... or clang 11
behaves differently.
--
Thanks,
David / dhildenb
^ permalink raw reply
* Re: [PATCH v8 2/8] powerpc/vdso: Remove __kernel_datapage_offset and simplify __get_datapage()
From: Dmitry Safonov @ 2020-10-23 13:29 UTC (permalink / raw)
To: Christophe Leroy, Will Deacon
Cc: nathanl, linux-arch, Arnd Bergmann, open list, Paul Mackerras,
Andy Lutomirski, Thomas Gleixner, Vincenzo Frascino, linuxppc-dev
In-Reply-To: <284cacf4-9811-4b67-385c-2783a7cd9b31@csgroup.eu>
Hi Christophe, Will,
On 10/23/20 12:57 PM, Christophe Leroy wrote:
>
>
> Le 23/10/2020 à 13:25, Will Deacon a écrit :
>> On Fri, Oct 23, 2020 at 01:22:04PM +0200, Christophe Leroy wrote:
>>> Hi Dmitry,
[..]
>>> I haven't seen the patches, did you sent them out finally ?
I was working on .close() hook, but while cooking it, I thought it may
be better to make tracking of user landing generic. Note that the vdso
base address is mostly needed by kernel as an address to land in
userspace after processing a signal.
I have some raw patches that add
+#ifdef CONFIG_ARCH_HAS_USER_LANDING
+ struct vm_area_struct *user_landing;
+#endif
inside mm_struct and I plan to finish them after rc1 gets released.
While working on that, I noticed that arm32 and some other architectures
track vdso position in mm.context with the only reason to add
AT_SYSINFO_EHDR in the elf header that's being loaded. That's quite
overkill to have a pointer in mm.context that rather can be a local
variable in elf binfmt loader. Also, I found some issues with mremap
code. The patches series mentioned are at the base of the branch with
generic user landing. I have sent only those patches not the full branch
as I remember there was a policy that during merge window one should
send only fixes, rather than refactoring/new code.
>> I think it's this series:
>>
>> https://lore.kernel.org/r/20201013013416.390574-1-dima@arista.com
>>
>> but they look really invasive to me, so I may cook a small hack for arm64
>> in the meantine / for stable.
I don't mind small hacks, but I'm concerned that the suggested fix which
sets `mm->context.vdso_base = 0` on munmap() may have it's issue: that
won't work if a user for whatever-broken-reason will mremap() vdso on 0
address. As the fix supposes to fix an issue that hasn't fired for
anyone yet, it probably shouldn't introduce another. That's why I've
used vm_area_struct to track vdso position in the patches set.
Probably, temporary, you could use something like:
#define BAD_VDSO_ADDRESS (-1)UL
Or non-page-aligned address.
But the signal code that checks if it can land on vdso/sigpage should be
also aligned with the new definition.
> Not sure we are talking about the same thing.
>
> I can't see any new .close function added to vm_special_mapping in order
> to replace arch_unmap() hook.
Thanks,
Dmitry
^ permalink raw reply
* RE: Buggy commit tracked to: "Re: [PATCH 2/9] iov_iter: move rw_copy_check_uvector() into lib/iov_iter.c"
From: David Laight @ 2020-10-23 13:28 UTC (permalink / raw)
To: 'Arnd Bergmann'
Cc: linux-aio@kvack.org, David Hildenbrand,
linux-mips@vger.kernel.org, David Howells, linux-mm@kvack.org,
keyrings@vger.kernel.org, sparclinux@vger.kernel.org,
Christoph Hellwig, linux-arch@vger.kernel.org,
linux-s390@vger.kernel.org, linux-scsi@vger.kernel.org,
kernel-team@android.com, linux-block@vger.kernel.org, Al Viro,
io-uring@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
Jens Axboe, linux-parisc@vger.kernel.org, Greg KH,
Nick Desaulniers, linux-kernel@vger.kernel.org,
linux-security-module@vger.kernel.org, netdev@vger.kernel.org,
linux-fsdevel@vger.kernel.org, Andrew Morton,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <CAK8P3a1n+b8hOMhNQSDzgic03dyXbmpccfTJ3C1bGKvzsgMXbg@mail.gmail.com>
From: Arnd Bergmann
> Sent: 23 October 2020 14:23
>
> On Fri, Oct 23, 2020 at 2:46 PM David Laight <David.Laight@aculab.com> wrote:
> >
> > From: Greg KH <gregkh@linuxfoundation.org>
> > > Sent: 22 October 2020 14:51
> >
> > I've rammed the code into godbolt.
> >
> > https://godbolt.org/z/9v5PPW
> >
> > Definitely a clang bug.
> >
> > Search for [wx]24 in the clang output.
> > nr_segs comes in as w2 and the initial bound checks are done on w2.
> > w24 is loaded from w2 - I don't believe this changes the high bits.
>
> You believe wrong, "mov w24, w2" is a zero-extending operation.
Ah ok, but gcc uses utxw for the same task.
I guess they could be the same opcode.
Last time I wrote ARM thumb didn't really exist - never mind 64bit
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
^ permalink raw reply
* Re: C vdso
From: Michael Ellerman @ 2020-10-23 13:24 UTC (permalink / raw)
To: Christophe Leroy; +Cc: linuxppc-dev@ozlabs.org
In-Reply-To: <be21c7c8-6828-b757-064d-20f74e5c1a31@csgroup.eu>
Christophe Leroy <christophe.leroy@csgroup.eu> writes:
> Le 24/09/2020 à 15:17, Christophe Leroy a écrit :
>> Le 17/09/2020 à 14:33, Michael Ellerman a écrit :
>>> Christophe Leroy <christophe.leroy@csgroup.eu> writes:
>>>>
>>>> What is the status with the generic C vdso merge ?
>>>> In some mail, you mentionned having difficulties getting it working on
>>>> ppc64, any progress ? What's the problem ? Can I help ?
>>>
>>> Yeah sorry I was hoping to get time to work on it but haven't been able
>>> to.
>>>
>>> It's causing crashes on ppc64 ie. big endian.
...
>>
>> Can you tell what defconfig you are using ? I have been able to setup a full glibc PPC64 cross
>> compilation chain and been able to test it under QEMU with success, using Nathan's vdsotest tool.
>
> What config are you using ?
ppc64_defconfig + guest.config
Or pseries_defconfig.
I'm using Ubuntu GCC 9.3.0 mostly, but it happens with other toolchains too.
At a minimum we're seeing relocations in the output, which is a problem:
$ readelf -r build\~/arch/powerpc/kernel/vdso64/vdso64.so
Relocation section '.rela.dyn' at offset 0x12a8 contains 8 entries:
Offset Info Type Sym. Value Sym. Name + Addend
000000001368 000000000016 R_PPC64_RELATIVE 7c0
000000001370 000000000016 R_PPC64_RELATIVE 9300
000000001380 000000000016 R_PPC64_RELATIVE 970
000000001388 000000000016 R_PPC64_RELATIVE 9300
000000001398 000000000016 R_PPC64_RELATIVE a90
0000000013a0 000000000016 R_PPC64_RELATIVE 9300
0000000013b0 000000000016 R_PPC64_RELATIVE b20
0000000013b8 000000000016 R_PPC64_RELATIVE 9300
cheers
^ permalink raw reply
* Re: Buggy commit tracked to: "Re: [PATCH 2/9] iov_iter: move rw_copy_check_uvector() into lib/iov_iter.c"
From: Arnd Bergmann @ 2020-10-23 13:23 UTC (permalink / raw)
To: David Laight
Cc: linux-aio@kvack.org, David Hildenbrand,
linux-mips@vger.kernel.org, David Howells, linux-mm@kvack.org,
keyrings@vger.kernel.org, sparclinux@vger.kernel.org,
Christoph Hellwig, linux-arch@vger.kernel.org,
linux-s390@vger.kernel.org, linux-scsi@vger.kernel.org,
kernel-team@android.com, linux-block@vger.kernel.org, Al Viro,
io-uring@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
Jens Axboe, linux-parisc@vger.kernel.org, Greg KH,
Nick Desaulniers, linux-kernel@vger.kernel.org,
linux-security-module@vger.kernel.org, netdev@vger.kernel.org,
linux-fsdevel@vger.kernel.org, Andrew Morton,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <134f162d711d466ebbd88906fae35b33@AcuMS.aculab.com>
On Fri, Oct 23, 2020 at 2:46 PM David Laight <David.Laight@aculab.com> wrote:
>
> From: Greg KH <gregkh@linuxfoundation.org>
> > Sent: 22 October 2020 14:51
>
> I've rammed the code into godbolt.
>
> https://godbolt.org/z/9v5PPW
>
> Definitely a clang bug.
>
> Search for [wx]24 in the clang output.
> nr_segs comes in as w2 and the initial bound checks are done on w2.
> w24 is loaded from w2 - I don't believe this changes the high bits.
You believe wrong, "mov w24, w2" is a zero-extending operation.
Arnd
^ permalink raw reply
* Re: Buggy commit tracked to: "Re: [PATCH 2/9] iov_iter: move rw_copy_check_uvector() into lib/iov_iter.c"
From: David Hildenbrand @ 2020-10-23 13:12 UTC (permalink / raw)
To: Al Viro, Nick Desaulniers
Cc: linux-aio@kvack.org, linux-mips@vger.kernel.org, David Howells,
linux-mm@kvack.org, keyrings@vger.kernel.org,
sparclinux@vger.kernel.org, Christoph Hellwig,
linux-arch@vger.kernel.org, linux-s390@vger.kernel.org,
linux-scsi@vger.kernel.org, kernel-team@android.com,
Arnd Bergmann, linux-block@vger.kernel.org,
io-uring@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
Jens Axboe, linux-parisc@vger.kernel.org, Greg KH,
linux-kernel@vger.kernel.org,
linux-security-module@vger.kernel.org, David Laight,
netdev@vger.kernel.org, linux-fsdevel@vger.kernel.org,
Andrew Morton, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <20201022192458.GV3576660@ZenIV.linux.org.uk>
On 22.10.20 21:24, Al Viro wrote:
> On Thu, Oct 22, 2020 at 12:04:52PM -0700, Nick Desaulniers wrote:
>
>> Passing an `unsigned long` as an `unsigned int` does no such
>> narrowing: https://godbolt.org/z/TvfMxe (same vice-versa, just tail
>> calls, no masking instructions).
>> So if rw_copy_check_uvector() is inlined into import_iovec() (looking
>> at the mainline@1028ae406999), then children calls of
>> `rw_copy_check_uvector()` will be interpreting the `nr_segs` register
>> unmodified, ie. garbage in the upper 32b.
>
> FWIW,
>
> void f(unsinged long v)
> {
> if (v != 1)
> printf("failed\n");
> }
>
> void g(unsigned int v)
> {
> f(v);
> }
>
> void h(unsigned long v)
> {
> g(v);
> }
>
> main()
> {
> h(0x100000001);
> }
>
> must not produce any output on a host with 32bit int and 64bit long, regardless of
> the inlining, having functions live in different compilation units, etc.
>
> Depending upon the calling conventions, compiler might do truncation in caller or
> in a callee, but it must be done _somewhere_.
The interesting case is having g() in a separate compilation unit and
force-calling g() with 0x100000001 via inline ASM. So forcing garbage
into high bits.
I'll paly with it.
--
Thanks,
David / dhildenb
^ permalink raw reply
* Re: Buggy commit tracked to: "Re: [PATCH 2/9] iov_iter: move rw_copy_check_uvector() into lib/iov_iter.c"
From: David Hildenbrand @ 2020-10-23 13:09 UTC (permalink / raw)
To: David Laight, 'Greg KH'
Cc: linux-aio@kvack.org, linux-mips@vger.kernel.org, David Howells,
linux-mm@kvack.org, keyrings@vger.kernel.org,
sparclinux@vger.kernel.org, Christoph Hellwig,
linux-arch@vger.kernel.org, linux-s390@vger.kernel.org,
linux-scsi@vger.kernel.org, kernel-team@android.com,
Arnd Bergmann, linux-block@vger.kernel.org, Al Viro,
io-uring@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
Jens Axboe, linux-parisc@vger.kernel.org, netdev@vger.kernel.org,
Nick Desaulniers, linux-kernel@vger.kernel.org,
linux-security-module@vger.kernel.org,
linux-fsdevel@vger.kernel.org, Andrew Morton,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <134f162d711d466ebbd88906fae35b33@AcuMS.aculab.com>
On 23.10.20 14:46, David Laight wrote:
> From: Greg KH <gregkh@linuxfoundation.org>
>> Sent: 22 October 2020 14:51
>
> I've rammed the code into godbolt.
>
> https://godbolt.org/z/9v5PPW
>
> Definitely a clang bug.
>
> Search for [wx]24 in the clang output.
> nr_segs comes in as w2 and the initial bound checks are done on w2.
> w24 is loaded from w2 - I don't believe this changes the high bits.
> There are no references to w24, just x24.
> So the kmalloc_array() is passed 'huge' and will fail.
> The iov_iter_init also gets the 64bit value.
>
> Note that the gcc code has a sign-extend copy of w2.
Do we have a result from using "unsigned long" in the base function and
explicitly masking of the high bits? That should definitely work.
Now, I am not a compiler expert, but as I already cited, at least on
x86-64 clang expects that the high bits were cleared by the caller - in
contrast to gcc. I suspect it's the same on arm64, but again, I am no
compiler expert.
If what I said and cites for x86-64 is correct, if the function expects
an "unsigned int", it will happily use 64bit operations without further
checks where valid when assuming high bits are zero. That's why even
converting everything to "unsigned int" as proposed by me won't work on
clang - it assumes high bits are zero (as indicated by Nick).
As I am neither a compiler experts (did I mention that already? ;) ) nor
an arm64 experts, I can't tell if this is a compiler BUG or not.
Main issue seems to be garbage in high bits as originally suggested by me.
--
Thanks,
David / dhildenb
^ permalink raw reply
* RE: Buggy commit tracked to: "Re: [PATCH 2/9] iov_iter: move rw_copy_check_uvector() into lib/iov_iter.c"
From: David Laight @ 2020-10-23 12:46 UTC (permalink / raw)
To: 'Greg KH', David Hildenbrand
Cc: linux-aio@kvack.org, linux-mips@vger.kernel.org, David Howells,
linux-mm@kvack.org, keyrings@vger.kernel.org,
sparclinux@vger.kernel.org, Christoph Hellwig,
linux-arch@vger.kernel.org, linux-s390@vger.kernel.org,
linux-scsi@vger.kernel.org, kernel-team@android.com,
Arnd Bergmann, linux-block@vger.kernel.org, Al Viro,
io-uring@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
Jens Axboe, linux-parisc@vger.kernel.org, netdev@vger.kernel.org,
Nick Desaulniers, linux-kernel@vger.kernel.org,
linux-security-module@vger.kernel.org,
linux-fsdevel@vger.kernel.org, Andrew Morton,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <20201022135036.GA1787470@kroah.com>
From: Greg KH <gregkh@linuxfoundation.org>
> Sent: 22 October 2020 14:51
I've rammed the code into godbolt.
https://godbolt.org/z/9v5PPW
Definitely a clang bug.
Search for [wx]24 in the clang output.
nr_segs comes in as w2 and the initial bound checks are done on w2.
w24 is loaded from w2 - I don't believe this changes the high bits.
There are no references to w24, just x24.
So the kmalloc_array() is passed 'huge' and will fail.
The iov_iter_init also gets the 64bit value.
Note that the gcc code has a sign-extend copy of w2.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
^ permalink raw reply
* Re: [PATCH] x86/mpx: fix recursive munmap() corruption
From: Christophe Leroy @ 2020-10-23 12:28 UTC (permalink / raw)
To: Laurent Dufour, Michael Ellerman
Cc: mhocko, rguenther, linux-mm, Dave Hansen, x86, stable, LKML,
Dave Hansen, Thomas Gleixner, luto, linuxppc-dev, Andrew Morton,
vbabka
In-Reply-To: <9c2b2826-4083-fc9c-5a4d-c101858dd560@linux.vnet.ibm.com>
Hi Laurent
Le 07/05/2019 à 18:35, Laurent Dufour a écrit :
> Le 01/05/2019 à 12:32, Michael Ellerman a écrit :
>> Laurent Dufour <ldufour@linux.vnet.ibm.com> writes:
>>> Le 23/04/2019 à 18:04, Dave Hansen a écrit :
>>>> On 4/23/19 4:16 AM, Laurent Dufour wrote:
>> ...
>>>>> There are 2 assumptions here:
>>>>> 1. 'start' and 'end' are page aligned (this is guaranteed by __do_munmap().
>>>>> 2. the VDSO is 1 page (this is guaranteed by the union vdso_data_store on powerpc)
>>>>
>>>> Are you sure about #2? The 'vdso64_pages' variable seems rather
>>>> unnecessary if the VDSO is only 1 page. ;)
>>>
>>> Hum, not so sure now ;)
>>> I got confused, only the header is one page.
>>> The test is working as a best effort, and don't cover the case where
>>> only few pages inside the VDSO are unmmapped (start >
>>> mm->context.vdso_base). This is not what CRIU is doing and so this was
>>> enough for CRIU support.
>>>
>>> Michael, do you think there is a need to manage all the possibility
>>> here, since the only user is CRIU and unmapping the VDSO is not a so
>>> good idea for other processes ?
>>
>> Couldn't we implement the semantic that if any part of the VDSO is
>> unmapped then vdso_base is set to zero? That should be fairly easy, eg:
>>
>> if (start < vdso_end && end >= mm->context.vdso_base)
>> mm->context.vdso_base = 0;
>>
>>
>> We might need to add vdso_end to the mm->context, but that should be OK.
>>
>> That seems like it would work for CRIU and make sense in general?
>
> Sorry for the late answer, yes this would make more sense.
>
> Here is a patch doing that.
>
In your patch, the test seems overkill:
+ if ((start <= vdso_base && vdso_end <= end) || /* 1 */
+ (vdso_base <= start && start < vdso_end) || /* 3,4 */
+ (vdso_base < end && end <= vdso_end)) /* 2,3 */
+ mm->context.vdso_base = mm->context.vdso_end = 0;
What about
if (start < vdso_end && vdso_start < end)
mm->context.vdso_base = mm->context.vdso_end = 0;
This should cover all cases, or am I missing something ?
And do we really need to store vdso_end in the context ?
I think it should be possible to re-calculate it: the size of the VDSO should be (&vdso32_end -
&vdso32_start) + PAGE_SIZE for 32 bits VDSO, and (&vdso64_end - &vdso64_start) + PAGE_SIZE for the
64 bits VDSO.
Christophe
^ permalink raw reply
* Re: [PATCH v8 2/8] powerpc/vdso: Remove __kernel_datapage_offset and simplify __get_datapage()
From: Christophe Leroy @ 2020-10-23 11:57 UTC (permalink / raw)
To: Will Deacon
Cc: nathanl, linux-arch, Arnd Bergmann, Dmitry Safonov, open list,
Paul Mackerras, Andy Lutomirski, Thomas Gleixner,
Vincenzo Frascino, linuxppc-dev
In-Reply-To: <20201023112514.GE20933@willie-the-truck>
Le 23/10/2020 à 13:25, Will Deacon a écrit :
> On Fri, Oct 23, 2020 at 01:22:04PM +0200, Christophe Leroy wrote:
>> Hi Dmitry,
>>
>> Le 28/09/2020 à 17:08, Dmitry Safonov a écrit :
>>> On 9/27/20 8:43 AM, Christophe Leroy wrote:
>>>>
>>>>
>>>> Le 21/09/2020 à 13:26, Will Deacon a écrit :
>>>>> On Fri, Aug 28, 2020 at 12:14:28PM +1000, Michael Ellerman wrote:
>>>>>> Dmitry Safonov <0x7f454c46@gmail.com> writes:
>>> [..]
>>>>>>> I'll cook a patch for vm_special_mapping if you don't mind :-)
>>>>>>
>>>>>> That would be great, thanks!
>>>>>
>>>>> I lost track of this one. Is there a patch kicking around to resolve
>>>>> this,
>>>>> or is the segfault expected behaviour?
>>>>>
>>>>
>>>> IIUC dmitry said he will cook a patch. I have not seen any patch yet.
>>>
>>> Yes, sorry about the delay - I was a bit busy with xfrm patches.
>>>
>>> I'll send patches for .close() this week, working on them now.
>>
>> I haven't seen the patches, did you sent them out finally ?
>
> I think it's this series:
>
> https://lore.kernel.org/r/20201013013416.390574-1-dima@arista.com
>
> but they look really invasive to me, so I may cook a small hack for arm64
> in the meantine / for stable.
>
Not sure we are talking about the same thing.
I can't see any new .close function added to vm_special_mapping in order to replace arch_unmap() hook.
Christophe
^ permalink raw reply
* Re: [PATCH v8 2/8] powerpc/vdso: Remove __kernel_datapage_offset and simplify __get_datapage()
From: Will Deacon @ 2020-10-23 11:25 UTC (permalink / raw)
To: Christophe Leroy
Cc: nathanl, linux-arch, Arnd Bergmann, Dmitry Safonov, open list,
Paul Mackerras, Andy Lutomirski, Thomas Gleixner,
Vincenzo Frascino, linuxppc-dev
In-Reply-To: <ba9861da-2f5b-a649-5626-af00af634546@csgroup.eu>
On Fri, Oct 23, 2020 at 01:22:04PM +0200, Christophe Leroy wrote:
> Hi Dmitry,
>
> Le 28/09/2020 à 17:08, Dmitry Safonov a écrit :
> > On 9/27/20 8:43 AM, Christophe Leroy wrote:
> > >
> > >
> > > Le 21/09/2020 à 13:26, Will Deacon a écrit :
> > > > On Fri, Aug 28, 2020 at 12:14:28PM +1000, Michael Ellerman wrote:
> > > > > Dmitry Safonov <0x7f454c46@gmail.com> writes:
> > [..]
> > > > > > I'll cook a patch for vm_special_mapping if you don't mind :-)
> > > > >
> > > > > That would be great, thanks!
> > > >
> > > > I lost track of this one. Is there a patch kicking around to resolve
> > > > this,
> > > > or is the segfault expected behaviour?
> > > >
> > >
> > > IIUC dmitry said he will cook a patch. I have not seen any patch yet.
> >
> > Yes, sorry about the delay - I was a bit busy with xfrm patches.
> >
> > I'll send patches for .close() this week, working on them now.
>
> I haven't seen the patches, did you sent them out finally ?
I think it's this series:
https://lore.kernel.org/r/20201013013416.390574-1-dima@arista.com
but they look really invasive to me, so I may cook a small hack for arm64
in the meantine / for stable.
Will
^ permalink raw reply
* Re: [PATCH v8 2/8] powerpc/vdso: Remove __kernel_datapage_offset and simplify __get_datapage()
From: Christophe Leroy @ 2020-10-23 11:22 UTC (permalink / raw)
To: Dmitry Safonov, Will Deacon, Michael Ellerman
Cc: nathanl, linux-arch, Arnd Bergmann, open list, Paul Mackerras,
Andy Lutomirski, Thomas Gleixner, Vincenzo Frascino, linuxppc-dev
In-Reply-To: <542145eb-7d90-0444-867e-c9cbb6bdd8e3@gmail.com>
Hi Dmitry,
Le 28/09/2020 à 17:08, Dmitry Safonov a écrit :
> On 9/27/20 8:43 AM, Christophe Leroy wrote:
>>
>>
>> Le 21/09/2020 à 13:26, Will Deacon a écrit :
>>> On Fri, Aug 28, 2020 at 12:14:28PM +1000, Michael Ellerman wrote:
>>>> Dmitry Safonov <0x7f454c46@gmail.com> writes:
> [..]
>>>>> I'll cook a patch for vm_special_mapping if you don't mind :-)
>>>>
>>>> That would be great, thanks!
>>>
>>> I lost track of this one. Is there a patch kicking around to resolve
>>> this,
>>> or is the segfault expected behaviour?
>>>
>>
>> IIUC dmitry said he will cook a patch. I have not seen any patch yet.
>
> Yes, sorry about the delay - I was a bit busy with xfrm patches.
>
> I'll send patches for .close() this week, working on them now.
I haven't seen the patches, did you sent them out finally ?
Christophe
^ permalink raw reply
* Re: [PATCH v2 1/2] powerpc: Introduce POWER10_DD1 feature
From: Ravi Bangoria @ 2020-10-23 10:14 UTC (permalink / raw)
To: Jordan Niethe
Cc: Christophe Leroy, Ravi Bangoria, Michael Neuling, Nicholas Piggin,
maddy, Paul Mackerras, naveen.n.rao, linuxppc-dev
In-Reply-To: <CACzsE9rqopXj37vrOcyM3sisomeYzxbeR6V4o5K3_Y8xqo5hHw@mail.gmail.com>
>>>> +static void __init fixup_cpu_features(void)
>>>> +{
>>>> + unsigned long version = mfspr(SPRN_PVR);
>>>> +
>>>> + if ((version & 0xffffffff) == 0x00800100)
>>>> + cur_cpu_spec->cpu_features |= CPU_FTR_POWER10_DD1;
>>>> +}
>>>> +
>>> I am just wondering why this is needed here, but the same thing is not
>>> done for, say, CPU_FTR_POWER9_DD2_1?
>>
>> When we don't use DT cpu_features (PowerVM / kvm geusts), we call
>> identify_cpu() twice. First with Real PVR which sets "raw" cpu_spec
>> as cur_cpu_spec and then 2nd time with Logical PVR (0x0f...) which
>> (mostly) overwrites the cur_cpu_spec with "architected" mode cpu_spec.
>> I don't see DD version specific entries for "architected" mode in
>> cpu_specs[] for any previous processors. So I've introduced this
>> function to tweak cpu_features.
>>
>> Though, I don't know why we don't have similar thing for
>> CPU_FTR_POWER9_DD2_1. I've to check that.
>>
>>> And should we get a /* Power10 DD 1 */ added to cpu_specs[]?
>>
>> IIUC, we don't need such entry. For PowerVM / kvm guests, we overwrite
>> cpu_spec, so /* Power10 */ "raw" entry is sufficient. And For baremetal,
>> we don't use cpu_specs[] at all.
> I think even for powernv, using dt features can be disabled by the
> cmdline with dt_cpu_ftrs=off, then cpu_specs[] will then be used.
Ok... with dt_cpu_ftrs=off, we seem to be using raw mode cpu_specs[] entry on
baremetal. So yeah, I'll add /* Power10 DD1 */ raw mode entry into cpu_specs[].
Thanks for pointing it out.
-Ravi
^ permalink raw reply
* Re: [PATCH] powerpc: Send SIGBUS from machine_check
From: Joakim Tjernlund @ 2020-10-23 9:23 UTC (permalink / raw)
To: linuxppc-dev@lists.ozlabs.org, mpe@ellerman.id.au
In-Reply-To: <87d019yd0w.fsf@mpe.ellerman.id.au>
On Fri, 2020-10-23 at 11:57 +1100, Michael Ellerman wrote:
>
>
> Joakim Tjernlund <joakim.tjernlund@infinera.com> writes:
> > Embedded PPC CPU should send SIGBUS to user space when applicable.
>
> Yeah, but it's not clear that it's applicable in all cases.
>
> At least I need some reasoning for why it's safe in all cases below to
> just send a SIGBUS and take no other action.
For me this came from an User SDK accessing a PCI device(also using PCI IRQs) and this
SDK did some strange stuff during shutdown which disabled the device before SW was done.
This caused PCI accesses, both from User Space and kernel PCI IRQs access) to the device
which caused an Machine Check(PCI transfer failed). Without this patch, the kernel
would just OOPS and hang/do strange things even for an access made by User space.
Now the User app just gets a SIGBUS and the kernel still works as it should.
Perhaps a SIGBUS and recover isn't right in all cases but without it there will be a
system break down.
> Is there a particular CPU you're working on? Can we start with that and
> look at all the machine check causes and which can be safely handled.
This was a T1042(e5500) but we have e500 and mpc832x as well.
>
> Some comments below ...
>
>
> > diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
> > index 0381242920d9..12715d24141c 100644
> > --- a/arch/powerpc/kernel/traps.c
> > +++ b/arch/powerpc/kernel/traps.c
> > @@ -621,6 +621,11 @@ int machine_check_e500mc(struct pt_regs *regs)
>
> At the beginning of the function we have:
>
> printk("Machine check in kernel mode.\n");
>
> Which should be updated.
Sure, just remove the "in kernel mode" perhaps?
>
> > reason & MCSR_MEA ? "Effective" : "Physical", addr);
> > }
> >
> > + if ((user_mode(regs))) {
> > + _exception(SIGBUS, regs, reason, regs->nip);
> > + recoverable = 1;
> > + }
>
> For most of the error causes we take no action and set recoverable = 0.
>
> Then you just declare that it is recoverable because it hit in
> userspace. Depending on the cause that might be OK, but it's not
> obviously correct in all cases.
Not so familiar with PPC that I can make out what is OK or not.
I do think you stand a better chance now that before though.
>
>
> > +
> > silent_out:
> > mtspr(SPRN_MCSR, mcsr);
> > return mfspr(SPRN_MCSR) == 0 && recoverable;
> > @@ -665,6 +670,10 @@ int machine_check_e500(struct pt_regs *regs)
>
> Same comment about the printk().
>
> > if (reason & MCSR_BUS_RPERR)
> > printk("Bus - Read Parity Error\n");
> >
> > + if ((user_mode(regs))) {
> > + _exception(SIGBUS, regs, reason, regs->nip);
> > + return 1;
> > + }
>
> And same comment more or less.
>
> Other than the MCSR_BUS_RBERR cases that are explicitly checked, the
> function does nothing to clear the cause of the machine check.
>
> > return 0;
> > }
> >
> > @@ -695,6 +704,10 @@ int machine_check_e200(struct pt_regs *regs)
> > if (reason & MCSR_BUS_WRERR)
> > printk("Bus - Write Bus Error on buffered store or cache line push\n");
> >
> > + if ((user_mode(regs))) {
> > + _exception(SIGBUS, regs, reason, regs->nip);
> > + return 1;
> > + }
>
> Same.
>
> > return 0;
> > }
> > #elif defined(CONFIG_PPC32)
> > @@ -731,6 +744,10 @@ int machine_check_generic(struct pt_regs *regs)
> > default:
> > printk("Unknown values in msr\n");
> > }
> > + if ((user_mode(regs))) {
> > + _exception(SIGBUS, regs, reason, regs->nip);
> > + return 1;
> > + }
>
> Same.
>
> > return 0;
> > }
> > #endif /* everything else */
> > --
> > 2.26.2
>
>
> cheers
^ permalink raw reply
* Re: [PATCH] treewide: Convert macro and uses of __section(foo) to __section("foo")
From: Joe Perches @ 2020-10-23 8:03 UTC (permalink / raw)
To: Miguel Ojeda
Cc: clang-built-linux, Nick Desaulniers, Linus Torvalds, linux-kernel,
linuxppc-dev
In-Reply-To: <CANiq72nfHjXkN65jy+unz0k66qvAALNhhhDZsxqPRLdtLKOW_Q@mail.gmail.com>
On Fri, 2020-10-23 at 08:08 +0200, Miguel Ojeda wrote:
> On Thu, Oct 22, 2020 at 4:36 AM Joe Perches <joe@perches.com> wrote:
> >
> > Use a more generic form for __section that requires quotes to avoid
> > complications with clang and gcc differences.
>
> I performed visual inspection (one by one...) and the only thing I saw
> is that sometimes the `__attribute__` has a whitespace afterwards and
> sometimes it doesn't, same for the commas inside, e.g.:
>
> - __used __attribute__((section(".modinfo"), unused, aligned(1))) \
> + __used __section(".modinfo") __attribute__((unused, aligned(1))) \
>
> and:
>
> - __attribute__ ((unused,__section__ ("__param"),aligned(sizeof(void *)))) \
> + __section("__param") __attribute__ ((unused, aligned(sizeof(void *)))) \
>
> I think the patch tries to follow the style of the replaced line, but
> for the commas in this last case it didn't. Anyway, it is not
> important.
Here the change follows the kernel style of space after comma.
> I can pick it up in my queue along with the __alias one and keep it
> for a few weeks in -next.
Thanks Miguel, but IMO it doesn't need time in next.
Applying it just before an rc1 minimizes conflicts.
^ permalink raw reply
* Re: C vdso
From: Christophe Leroy @ 2020-10-23 6:28 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev@ozlabs.org
In-Reply-To: <cc532aa8-a9e0-a105-b7b1-ee8d723b7ed6@csgroup.eu>
Hi Michael,
Le 24/09/2020 à 15:17, Christophe Leroy a écrit :
> Hi Michael
>
> Le 17/09/2020 à 14:33, Michael Ellerman a écrit :
>> Hi Christophe,
>>
>> Christophe Leroy <christophe.leroy@csgroup.eu> writes:
>>> Hi Michael,
>>>
>>> What is the status with the generic C vdso merge ?
>>> In some mail, you mentionned having difficulties getting it working on
>>> ppc64, any progress ? What's the problem ? Can I help ?
>>
>> Yeah sorry I was hoping to get time to work on it but haven't been able
>> to.
>>
>> It's causing crashes on ppc64 ie. big endian.
>>
>
>>
>> As you can see from the instruction dump we have jumped into the weeds somewhere.
>>
>> We also had the report from the kbuild robot about rela.opd being
>> discarded, which I think is indicative of a bigger problem. ie. we don't
>> process relocations for the VDSO, but opds require relocations (they
>> contain an absolute pointer).
>>
>> I thought we could get away with that, because the VDSO entry points
>> aren't proper functions (so they don't have opds), and I didn't think
>> we'd be calling via function pointers in the VDSO code (which would
>> require opds). But seems something is not working right.
>>
>> Sorry I haven't got back to you with those details. Things are a bit of
>> a mess inside IBM at the moment (always?), and I've been trying to get
>> everything done before I take a holiday next week.
>>
>
>
> Can you tell what defconfig you are using ? I have been able to setup a full glibc PPC64 cross
> compilation chain and been able to test it under QEMU with success, using Nathan's vdsotest tool.
What config are you using ?
Christophe
>
> I tested with both ppc64_defconfig and pseries_defconfig.
>
> The only problem I got is with getcpu, which segfaults but both before and after applying my series,
> so I guess this is unrelated.
>
> Not sure we can pay too much attention to the exact measurement as it is a ppc64 QEMU running on a
> x86 Linux which is running in a Virtual Box on a x86 windows Laptop, but at least it works:
>
> Without the series:
>
> clock-getres-monotonic: vdso: 389 nsec/call
> clock-gettime-monotonic: vdso: 781 nsec/call
> clock-getres-monotonic-coarse: vdso: 13715 nsec/call
> clock-gettime-monotonic-coarse: vdso: 312 nsec/call
> clock-getres-monotonic-raw: vdso: 13589 nsec/call
> clock-getres-tai: vdso: 13827 nsec/call
> clock-gettime-tai: vdso: 14846 nsec/call
> clock-getres-boottime: vdso: 13596 nsec/call
> clock-gettime-boottime: vdso: 14758 nsec/call
> clock-getres-realtime: vdso: 327 nsec/call
> clock-gettime-realtime: vdso: 717 nsec/call
> clock-getres-realtime-coarse: vdso: 14102 nsec/call
> clock-gettime-realtime-coarse: vdso: 299 nsec/call
> gettimeofday: vdso: 771 nsec/call
>
> With the series:
>
> clock-getres-monotonic: vdso: 350 nsec/call
> clock-gettime-monotonic: vdso: 726 nsec/call
> clock-getres-monotonic-coarse: vdso: 356 nsec/call
> clock-gettime-monotonic-coarse: vdso: 423 nsec/call
> clock-getres-monotonic-raw: vdso: 349 nsec/call
> clock-getres-tai: vdso: 419 nsec/call
> clock-gettime-tai: vdso: 724 nsec/call
> clock-getres-boottime: vdso: 352 nsec/call
> clock-gettime-boottime: vdso: 752 nsec/call
> clock-getres-realtime: vdso: 351 nsec/call
> clock-gettime-realtime: vdso: 733 nsec/call
> clock-getres-realtime-coarse: vdso: 356 nsec/call
> clock-gettime-realtime-coarse: vdso: 367 nsec/call
> gettimeofday: vdso: 796 nsec/call
>
>
> Thanks
> Christophe
^ permalink raw reply
* Re: [PATCH] treewide: Convert macro and uses of __section(foo) to __section("foo")
From: Miguel Ojeda @ 2020-10-23 6:08 UTC (permalink / raw)
To: Joe Perches
Cc: clang-built-linux, Nick Desaulniers, Linus Torvalds, linux-kernel,
linuxppc-dev
In-Reply-To: <fe8abcc88cff676ead8ee48db1e993e63b0611c7.1603327264.git.joe@perches.com>
On Thu, Oct 22, 2020 at 4:36 AM Joe Perches <joe@perches.com> wrote:
>
> Use a more generic form for __section that requires quotes to avoid
> complications with clang and gcc differences.
I performed visual inspection (one by one...) and the only thing I saw
is that sometimes the `__attribute__` has a whitespace afterwards and
sometimes it doesn't, same for the commas inside, e.g.:
- __used __attribute__((section(".modinfo"), unused, aligned(1))) \
+ __used __section(".modinfo") __attribute__((unused, aligned(1))) \
and:
- __attribute__ ((unused,__section__ ("__param"),aligned(sizeof(void *)))) \
+ __section("__param") __attribute__ ((unused, aligned(sizeof(void *)))) \
I think the patch tries to follow the style of the replaced line, but
for the commas in this last case it didn't. Anyway, it is not
important.
I can pick it up in my queue along with the __alias one and keep it
for a few weeks in -next.
Cheers,
Miguel
^ permalink raw reply
* [powerpc:fixes] BUILD SUCCESS 4ff753feab021242144818b9a3ba011238218145
From: kernel test robot @ 2020-10-23 4:14 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git fixes
branch HEAD: 4ff753feab021242144818b9a3ba011238218145 powerpc/pseries: Avoid using addr_to_pfn in real mode
elapsed time: 723m
configs tested: 120
configs skipped: 2
The following configs have been built successfully.
More configs may be tested in the coming days.
gcc tested configs:
arm defconfig
arm64 allyesconfig
arm64 defconfig
arm allyesconfig
arm allmodconfig
powerpc mpc837x_mds_defconfig
mips ath79_defconfig
um kunit_defconfig
powerpc ppa8548_defconfig
arm spear6xx_defconfig
riscv alldefconfig
arm xcep_defconfig
arm exynos_defconfig
arm multi_v5_defconfig
powerpc ksi8560_defconfig
arm s3c6400_defconfig
mips maltaup_defconfig
arm omap1_defconfig
arm lubbock_defconfig
arm spitz_defconfig
powerpc xes_mpc85xx_defconfig
mips vocore2_defconfig
arm jornada720_defconfig
arc axs103_defconfig
arm vt8500_v6_v7_defconfig
powerpc redwood_defconfig
sh sh03_defconfig
mips bmips_stb_defconfig
sparc64 alldefconfig
sh magicpanelr2_defconfig
arm axm55xx_defconfig
c6x evmc6678_defconfig
mips bigsur_defconfig
powerpc tqm8560_defconfig
mips cavium_octeon_defconfig
xtensa xip_kc705_defconfig
m68k m5407c3_defconfig
sh espt_defconfig
arm realview_defconfig
xtensa virt_defconfig
powerpc ppc6xx_defconfig
arm versatile_defconfig
h8300 defconfig
m68k stmark2_defconfig
powerpc icon_defconfig
ia64 allmodconfig
ia64 defconfig
ia64 allyesconfig
m68k allmodconfig
m68k defconfig
m68k allyesconfig
nios2 defconfig
arc allyesconfig
nds32 allnoconfig
c6x allyesconfig
nds32 defconfig
nios2 allyesconfig
csky defconfig
alpha defconfig
alpha allyesconfig
xtensa allyesconfig
h8300 allyesconfig
arc defconfig
sh allmodconfig
parisc defconfig
s390 allyesconfig
parisc allyesconfig
s390 defconfig
i386 allyesconfig
sparc allyesconfig
sparc defconfig
i386 defconfig
mips allyesconfig
mips allmodconfig
powerpc allyesconfig
powerpc allmodconfig
powerpc allnoconfig
i386 randconfig-a002-20201023
i386 randconfig-a005-20201023
i386 randconfig-a003-20201023
i386 randconfig-a001-20201023
i386 randconfig-a006-20201023
i386 randconfig-a004-20201023
i386 randconfig-a002-20201022
i386 randconfig-a005-20201022
i386 randconfig-a003-20201022
i386 randconfig-a001-20201022
i386 randconfig-a006-20201022
i386 randconfig-a004-20201022
x86_64 randconfig-a011-20201022
x86_64 randconfig-a013-20201022
x86_64 randconfig-a016-20201022
x86_64 randconfig-a015-20201022
x86_64 randconfig-a012-20201022
x86_64 randconfig-a014-20201022
i386 randconfig-a016-20201022
i386 randconfig-a014-20201022
i386 randconfig-a015-20201022
i386 randconfig-a012-20201022
i386 randconfig-a013-20201022
i386 randconfig-a011-20201022
riscv nommu_k210_defconfig
riscv allyesconfig
riscv nommu_virt_defconfig
riscv allnoconfig
riscv defconfig
riscv rv32_defconfig
riscv allmodconfig
x86_64 rhel
x86_64 allyesconfig
x86_64 rhel-7.6-kselftests
x86_64 defconfig
x86_64 rhel-8.3
x86_64 kexec
clang tested configs:
x86_64 randconfig-a001-20201022
x86_64 randconfig-a002-20201022
x86_64 randconfig-a003-20201022
x86_64 randconfig-a006-20201022
x86_64 randconfig-a004-20201022
x86_64 randconfig-a005-20201022
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply
* [PATCH] powerpc: Add config fragment for disabling -Werror
From: Michael Ellerman @ 2020-10-23 4:00 UTC (permalink / raw)
To: linuxppc-dev
This makes it easy to disable building with -Werror:
$ make defconfig
$ grep WERROR .config
# CONFIG_PPC_DISABLE_WERROR is not set
CONFIG_PPC_WERROR=y
$ make disable-werror.config
GEN Makefile
Using .config as base
Merging arch/powerpc/configs/disable-werror.config
Value of CONFIG_PPC_DISABLE_WERROR is redefined by fragment arch/powerpc/configs/disable-werror.config:
Previous value: # CONFIG_PPC_DISABLE_WERROR is not set
New value: CONFIG_PPC_DISABLE_WERROR=y
...
$ grep WERROR .config
CONFIG_PPC_DISABLE_WERROR=y
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/configs/disable-werror.config | 1 +
1 file changed, 1 insertion(+)
create mode 100644 arch/powerpc/configs/disable-werror.config
diff --git a/arch/powerpc/configs/disable-werror.config b/arch/powerpc/configs/disable-werror.config
new file mode 100644
index 000000000000..6ea12a12432c
--- /dev/null
+++ b/arch/powerpc/configs/disable-werror.config
@@ -0,0 +1 @@
+CONFIG_PPC_DISABLE_WERROR=y
--
2.25.1
^ permalink raw reply related
* [PATCH] net: ucc_geth: Drop extraneous parentheses in comparison
From: Michael Ellerman @ 2020-10-23 3:32 UTC (permalink / raw)
To: linuxppc-dev, netdev; +Cc: kuba, leoyang.li, davem, linux-kernel
Clang warns about the extra parentheses in this comparison:
drivers/net/ethernet/freescale/ucc_geth.c:1361:28:
warning: equality comparison with extraneous parentheses
if ((ugeth->phy_interface == PHY_INTERFACE_MODE_SGMII))
~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~
It seems clear the intent here is to do a comparison not an
assignment, so drop the extra parentheses to avoid any confusion.
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
drivers/net/ethernet/freescale/ucc_geth.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/freescale/ucc_geth.c b/drivers/net/ethernet/freescale/ucc_geth.c
index db791f60b884..d8ad478a0a13 100644
--- a/drivers/net/ethernet/freescale/ucc_geth.c
+++ b/drivers/net/ethernet/freescale/ucc_geth.c
@@ -1358,7 +1358,7 @@ static int adjust_enet_interface(struct ucc_geth_private *ugeth)
(ugeth->phy_interface == PHY_INTERFACE_MODE_RTBI)) {
upsmr |= UCC_GETH_UPSMR_TBIM;
}
- if ((ugeth->phy_interface == PHY_INTERFACE_MODE_SGMII))
+ if (ugeth->phy_interface == PHY_INTERFACE_MODE_SGMII)
upsmr |= UCC_GETH_UPSMR_SGMM;
out_be32(&uf_regs->upsmr, upsmr);
--
2.25.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox