* Re: [PATCH 1/2] pseries/vio: Remove stray #ifdef CONFIG_PPC_PSERIES
From: Michael Ellerman @ 2020-01-31 10:38 UTC (permalink / raw)
To: Oliver O'Halloran, linuxppc-dev; +Cc: Oliver O'Halloran
In-Reply-To: <20200130063153.19915-1-oohall@gmail.com>
Subject: Re: [PATCH 1/2] pseries/vio: Remove stray #ifdef CONFIG_PPC_PSERIES
^
powerpc/
Please.
I'll fix it up.
Oliver O'Halloran <oohall@gmail.com> writes:
> vio.c requires CONFIG_IBMVIO which in turn depends on PPC_PSERIES.
> In other words, this ifdef is pointless.
And all of platforms/pseries is only built if PPC_PSERIES=y.
> At a guess it's a carry-over from pre-history.
It's not pre-history. Probably should have been cleaned up in:
commit b0787660260604ba63621881851de0032279819b
Author: Stephen Rothwell <sfr@canb.auug.org.au>
AuthorDate: Wed Mar 7 18:43:10 2012 +0000
Commit: Benjamin Herrenschmidt <benh@kernel.crashing.org>
CommitDate: Fri Mar 9 10:35:23 2012 +1100
powerpc: clean up vio.c
This cleans up vio.c after the removal of the legacy iSeries platform.
It also removes some no longer referenced include files.
Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
cheers
> diff --git a/arch/powerpc/platforms/pseries/vio.c b/arch/powerpc/platforms/pseries/vio.c
> index f682b7b..37f1f25 100644
> --- a/arch/powerpc/platforms/pseries/vio.c
> +++ b/arch/powerpc/platforms/pseries/vio.c
> @@ -1628,7 +1628,6 @@ const void *vio_get_attribute(struct vio_dev *vdev, char *which, int *length)
> }
> EXPORT_SYMBOL(vio_get_attribute);
>
> -#ifdef CONFIG_PPC_PSERIES
> /* vio_find_name() - internal because only vio.c knows how we formatted the
> * kobject name
> */
> @@ -1698,7 +1697,6 @@ int vio_disable_interrupts(struct vio_dev *dev)
> return rc;
> }
> EXPORT_SYMBOL(vio_disable_interrupts);
> -#endif /* CONFIG_PPC_PSERIES */
>
> static int __init vio_init(void)
> {
> --
> 2.9.5
^ permalink raw reply
* Re: [PATCH v6 00/10] mm/memory_hotplug: Shrink zones before removing memory
From: David Hildenbrand @ 2020-01-31 10:36 UTC (permalink / raw)
To: Michal Hocko
Cc: Mark Rutland, Pankaj Gupta, Rich Felker, linux-ia64, linux-sh,
Peter Zijlstra, Catalin Marinas, Dave Hansen, Heiko Carstens,
Wei Yang, linux-mm, Pavel Tatashin, Alexander Potapenko,
H. Peter Anvin, Alexander Duyck, Ira Weiny, Thomas Gleixner,
Qian Cai, linux-s390, Yu Zhao, Yoshinori Sato, Jason Gunthorpe,
Aneesh Kumar K . V, x86, Matthew Wilcox (Oracle), Mike Rapoport,
Halil Pasic, Christian Borntraeger, Ingo Molnar, Gerald Schaefer,
Wei Yang, Fenghua Yu, Pavel Tatashin, Vasily Gorbik,
Anshuman Khandual, Vlastimil Babka, Will Deacon, Robin Murphy,
Jun Yao, Borislav Petkov, Andy Lutomirski, Dan Williams,
linux-arm-kernel, Oscar Salvador, Tony Luck, Mel Gorman,
Masahiro Yamada, Greg Kroah-Hartman, Steve Capper, linux-kernel,
Logan Gunthorpe, Tom Lendacky, Paul Mackerras, Andrew Morton,
linuxppc-dev
In-Reply-To: <20200131100341.GB24244@dhcp22.suse.cz>
>>> The first patch has reviews, the remainder are unloved.
>>
>> Trying hard not to rant about the review mentality on this list, but I'm
>> afraid I can't totally bite my tongue ... :)
>
> I am afraid this is less about mentality than the lack of man power.
> This is not a new problem. We have much more code producers than
> reviewers.
It's part of the "take-and-not-give-back review mentality" and I hope
you "smelled" that the comment was not targeted at you.
[...]
>> Although it will make upstreaming stuff *even harder* and *even slower*,
>> maybe we should start to only queue patches that have an ACK/RB, so they
>> won't get blocked by this later on? At least that makes your life easier
>> and people won't have to eventually follow up on patches that have been
>> in linux-next for months.
>
> I wouldn't mind if patched got merged to mmotm less pro-actively at all.
> People tend to care less to follow up on patches that are in the queue
> already from my past experience. And also it encourages to generate more
> code than review.
The current process will at least encourage me in the long term to
generate less code (changes) as well. I consider patches that have not
been merged upstream as on my TODO list - and it seems to keep on
growing. (yes, there are people that fire-and-forget)
Then, I much rather prefer to just get no reply on my patches, ping two
times, and eventually dump them into the trash. As explained, will WHP
result in me generating less code changes overall. Problem partially
solved (at least one producer less) :)
>
> This is certainly not a black or white of course. Some areas have barely
> anybody for a review except for the person actively writing code in that
> area so this really needs the case by case approach.
Yes.
>
> Anyway this is not a new discussion or a new problem we are facing. I
> believe that part of the problem is that the MM subsystem doesn't really
> have official maintainers so there is nobody really responsible for
> particular parts of the subsystem. Sure Andrew is merging patches based
> on the review feedback or his gut feeling but I am afraid this is not
> enough.
If we would have "official maintainers" would it really help (besides
Andrew having less stuff to do of course)? E.g., if you would pick up
the memory hotplug patches, you would still have to have a look at them
- it's still the producer-consumer imbalance (and you would have an even
higher workload).
But yeah, we should most probably finally have official maintainers. For
people sending patches, it's often not obvious whom to cc (and whom to
ping).
Smells like a "LSF/MM/BPF TOPIC", but as you said, it's not a new
problem/discussion. (I won't be around, so I can't bring that topic up)
Anyhow, my two cents.
--
Thanks,
David / dhildenb
^ permalink raw reply
* Re: [PATCH v6 00/10] mm/memory_hotplug: Shrink zones before removing memory
From: Michal Hocko @ 2020-01-31 10:03 UTC (permalink / raw)
To: David Hildenbrand
Cc: Mark Rutland, Pankaj Gupta, Rich Felker, linux-ia64, linux-sh,
Peter Zijlstra, Catalin Marinas, Dave Hansen, Heiko Carstens,
Wei Yang, linux-mm, Pavel Tatashin, Alexander Potapenko,
H. Peter Anvin, Alexander Duyck, Ira Weiny, Thomas Gleixner,
Qian Cai, linux-s390, Yu Zhao, Yoshinori Sato, Jason Gunthorpe,
Aneesh Kumar K . V, x86, Matthew Wilcox (Oracle), Mike Rapoport,
Halil Pasic, Christian Borntraeger, Ingo Molnar, Gerald Schaefer,
Wei Yang, Fenghua Yu, Pavel Tatashin, Vasily Gorbik,
Anshuman Khandual, Vlastimil Babka, Will Deacon, Robin Murphy,
Jun Yao, Borislav Petkov, Andy Lutomirski, Dan Williams,
linux-arm-kernel, Oscar Salvador, Tony Luck, Mel Gorman,
Masahiro Yamada, Greg Kroah-Hartman, Steve Capper, linux-kernel,
Logan Gunthorpe, Tom Lendacky, Paul Mackerras, Andrew Morton,
linuxppc-dev
In-Reply-To: <f7ed4448-8f41-599d-4689-914eeaf84d6d@redhat.com>
On Fri 31-01-20 10:18:34, David Hildenbrand wrote:
> On 31.01.20 05:40, Andrew Morton wrote:
> > On Tue, 3 Dec 2019 14:36:38 +0100 Oscar Salvador <osalvador@suse.de> wrote:
> >
> >> On Mon, Dec 02, 2019 at 10:09:51AM +0100, David Hildenbrand wrote:
> >>> @Michal, @Oscar, can some of you at least have a patch #5 now so we can
> >>> proceed with that? (the other patches can stay in -next some time longer)
> >>
> >> Hi,
> >>
> >> I will be having a look at patch#5 shortly.
> >>
> >> Thanks for the reminder
> >
> > Things haven't improved a lot :(
> >
> > mm-memmap_init-update-variable-name-in-memmap_init_zone.patch
> > mm-memory_hotplug-poison-memmap-in-remove_pfn_range_from_zone.patch
> > mm-memory_hotplug-we-always-have-a-zone-in-find_smallestbiggest_section_pfn.patch
> > mm-memory_hotplug-dont-check-for-all-holes-in-shrink_zone_span.patch
> > mm-memory_hotplug-drop-local-variables-in-shrink_zone_span.patch
> > mm-memory_hotplug-cleanup-__remove_pages.patch
> >
> > The first patch has reviews, the remainder are unloved.
>
> Trying hard not to rant about the review mentality on this list, but I'm
> afraid I can't totally bite my tongue ... :)
I am afraid this is less about mentality than the lack of man power.
This is not a new problem. We have much more code producers than
reviewers.
In this particular case the review is expected from me and I am
sorry that my bandwith doesn't scale with the email traffic in my inbox.
I do very much appreciate the amount of work you are doing in the
hotplug area but we need more reviewers here.
> Now, this is an uncomfortable situation for you and me. You have to ping
> people about review and patches are stuck in your tree. I have a growing
> list of patches that are somewhat considered "done", but well,
> not-upstream-at-all. I have patches that are long in RHEL and were
> properly tested, but could get dropped any time because -ENOREVIEW.
>
> Our process nowadays seems to be, to only upstream what has an ACK/RB
> (fixes/features/cleanups). I can understand this is desirable (yet, I am
> not sure if this makes sense with the current take-and-not-give-back
> review mentality on this list).
>
> Although it will make upstreaming stuff *even harder* and *even slower*,
> maybe we should start to only queue patches that have an ACK/RB, so they
> won't get blocked by this later on? At least that makes your life easier
> and people won't have to eventually follow up on patches that have been
> in linux-next for months.
I wouldn't mind if patched got merged to mmotm less pro-actively at all.
People tend to care less to follow up on patches that are in the queue
already from my past experience. And also it encourages to generate more
code than review.
This is certainly not a black or white of course. Some areas have barely
anybody for a review except for the person actively writing code in that
area so this really needs the case by case approach.
Anyway this is not a new discussion or a new problem we are facing. I
believe that part of the problem is that the MM subsystem doesn't really
have official maintainers so there is nobody really responsible for
particular parts of the subsystem. Sure Andrew is merging patches based
on the review feedback or his gut feeling but I am afraid this is not
enough.
--
Michal Hocko
SUSE Labs
^ permalink raw reply
* Re: [PATCH v6 00/10] mm/memory_hotplug: Shrink zones before removing memory
From: David Hildenbrand @ 2020-01-31 9:18 UTC (permalink / raw)
To: Andrew Morton, Oscar Salvador
Cc: Mark Rutland, Pankaj Gupta, Michal Hocko, linux-ia64, linux-sh,
Peter Zijlstra, Catalin Marinas, Dave Hansen, Heiko Carstens,
Wei Yang, linux-mm, Pavel Tatashin, Rich Felker,
Alexander Potapenko, H. Peter Anvin, Alexander Duyck, Ira Weiny,
Thomas Gleixner, Qian Cai, linux-s390, Yu Zhao, Yoshinori Sato,
Jason Gunthorpe, Aneesh Kumar K . V, x86, Matthew Wilcox (Oracle),
Mike Rapoport, Halil Pasic, Christian Borntraeger, Ingo Molnar,
Gerald Schaefer, Fenghua Yu, Pavel Tatashin, Vasily Gorbik,
Anshuman Khandual, Vlastimil Babka, Will Deacon, Robin Murphy,
Jun Yao, Borislav Petkov, Andy Lutomirski, Dan Williams,
linux-arm-kernel, Tony Luck, Mel Gorman, Masahiro Yamada,
Greg Kroah-Hartman, Steve Capper, linux-kernel, Logan Gunthorpe,
Wei Yang, Paul Mackerras, Tom Lendacky, linuxppc-dev
In-Reply-To: <20200130204043.29e21049775e3a637db733e0@linux-foundation.org>
On 31.01.20 05:40, Andrew Morton wrote:
> On Tue, 3 Dec 2019 14:36:38 +0100 Oscar Salvador <osalvador@suse.de> wrote:
>
>> On Mon, Dec 02, 2019 at 10:09:51AM +0100, David Hildenbrand wrote:
>>> @Michal, @Oscar, can some of you at least have a patch #5 now so we can
>>> proceed with that? (the other patches can stay in -next some time longer)
>>
>> Hi,
>>
>> I will be having a look at patch#5 shortly.
>>
>> Thanks for the reminder
>
> Things haven't improved a lot :(
>
> mm-memmap_init-update-variable-name-in-memmap_init_zone.patch
> mm-memory_hotplug-poison-memmap-in-remove_pfn_range_from_zone.patch
> mm-memory_hotplug-we-always-have-a-zone-in-find_smallestbiggest_section_pfn.patch
> mm-memory_hotplug-dont-check-for-all-holes-in-shrink_zone_span.patch
> mm-memory_hotplug-drop-local-variables-in-shrink_zone_span.patch
> mm-memory_hotplug-cleanup-__remove_pages.patch
>
> The first patch has reviews, the remainder are unloved.
Trying hard not to rant about the review mentality on this list, but I'm
afraid I can't totally bite my tongue ... :)
Now, this is an uncomfortable situation for you and me. You have to ping
people about review and patches are stuck in your tree. I have a growing
list of patches that are somewhat considered "done", but well,
not-upstream-at-all. I have patches that are long in RHEL and were
properly tested, but could get dropped any time because -ENOREVIEW.
Our process nowadays seems to be, to only upstream what has an ACK/RB
(fixes/features/cleanups). I can understand this is desirable (yet, I am
not sure if this makes sense with the current take-and-not-give-back
review mentality on this list).
Although it will make upstreaming stuff *even harder* and *even slower*,
maybe we should start to only queue patches that have an ACK/RB, so they
won't get blocked by this later on? At least that makes your life easier
and people won't have to eventually follow up on patches that have been
in linux-next for months.
Note: the result will be that many of my patches will still not get
reviewed, won't get queued/upstreamed, I will continuously ping and
resend, I will lose interest because I have better things to do, I will
lose interest in our code quality, I will lose interest to review.
(side note: some people might actually enjoy me sending less cleanup
patches, so this approach might be desirable for some ;) )
One alternative is to send patches upstream once they have been lying
around in linux-next for $RANDOM number of months, because they
obviously saw some testing and nobody started to yell at them once
stumbling over them on linux-mm.
--
Thanks,
David / dhildenb
^ permalink raw reply
* Re: [PATCH v2] powerpc: Do not consider weak unresolved symbol relocations as bad
From: Michael Ellerman @ 2020-01-31 9:18 UTC (permalink / raw)
To: Alex Ghiti, Benjamin Herrenschmidt, Paul Mackerras, linuxppc-dev,
linux-kernel, Stephen Rothwell, Alexei Starovoitov, linux-next,
Zong Li, Palmer Dabbelt
In-Reply-To: <8a8d45c6-4ad2-c682-abfb-3d97188d0d45@ghiti.fr>
Alex Ghiti <alex@ghiti.fr> writes:
> On 1/18/20 12:03 PM, Alexandre Ghiti wrote:
>> Commit 8580ac9404f6 ("bpf: Process in-kernel BTF") introduced two weak
>> symbols that may be unresolved at link time which result in an absolute
>> relocation to 0. relocs_check.sh emits the following warning:
>>
>> "WARNING: 2 bad relocations
>> c000000001a41478 R_PPC64_ADDR64 _binary__btf_vmlinux_bin_start
>> c000000001a41480 R_PPC64_ADDR64 _binary__btf_vmlinux_bin_end"
>>
>> whereas those relocations are legitimate even for a relocatable kernel
>> compiled with -pie option.
>>
>> relocs_check.sh already excluded some weak unresolved symbols explicitly:
>> remove those hardcoded symbols and add some logic that parses the symbols
>> using nm, retrieves all the weak unresolved symbols and excludes those from
>> the list of the potential bad relocations.
>>
>> Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
>> Signed-off-by: Alexandre Ghiti <alex@ghiti.fr>
>> ---
>>
>> Changes in v2:
>> - Follow Stephen advice of using grep -F instead of looping over weak symbols
>> using read, patch is way smaller and cleaner.
>> - Add missing nm in comment
>>
>> arch/powerpc/Makefile.postlink | 4 ++--
>> arch/powerpc/tools/relocs_check.sh | 20 ++++++++++++--------
>> 2 files changed, 14 insertions(+), 10 deletions(-)
>>
...
>
> Hi guys,
>
>
> Any thought about that ?
>
> I do think this patch makes the whole check about absolute relocations
> clearer.
> And in the future, it will avoid anyone to spend some time on those
> "bad" relocations
> which actually aren't.
Sorry I missed the v2. Will pick it up.
cheers
^ permalink raw reply
* Re: [PATCH] lkdtm: Test KUAP directional user access unlocks on powerpc
From: Russell Currey @ 2020-01-31 7:01 UTC (permalink / raw)
To: Christophe Leroy, keescook, mpe
Cc: kernel-hardening, linuxppc-dev, linux-kernel, dja
In-Reply-To: <c05a4327-0c81-0e3e-d93a-9d62183b146c@c-s.fr>
On Fri, 2020-01-31 at 07:58 +0100, Christophe Leroy wrote:
>
> Le 31/01/2020 à 07:53, Russell Currey a écrit :
> > On Fri, 2020-01-31 at 07:44 +0100, Christophe Leroy wrote:
> > > Le 31/01/2020 à 06:31, Russell Currey a écrit :
> > > > + pr_info("attempting bad read at %px with write
> > > > allowed\n",
> > > > ptr);
> > > > + tmp = *ptr;
> > > > + tmp += 0xc0dec0de;
> > > > + prevent_write_to_user(ptr, sizeof(unsigned long));
> > >
> > > Does it work ? I would have thought that if the read fails the
> > > process
> > > will die and the following test won't be performed.
> >
> > Correct, the ACCESS_USERSPACE test does the same thing. Splitting
> > this
> > into separate R and W tests makes sense, even if it is unlikely
> > that
> > one would be broken without the other.
> >
>
> Or once we are using user_access_begin() stuff, we can use
> unsafe_put_user() and unsafe_get_user() which should return an error
> instead of killing the caller.
Even better, and thanks for your work on all this stuff.
- Russell
>
> Christophe
^ permalink raw reply
* Re: [PATCH] lkdtm: Test KUAP directional user access unlocks on powerpc
From: Christophe Leroy @ 2020-01-31 6:58 UTC (permalink / raw)
To: Russell Currey, keescook, mpe
Cc: kernel-hardening, linuxppc-dev, linux-kernel, dja
In-Reply-To: <0b016861756cbe27e66651b5c21229a06558cb57.camel@russell.cc>
Le 31/01/2020 à 07:53, Russell Currey a écrit :
> On Fri, 2020-01-31 at 07:44 +0100, Christophe Leroy wrote:
>>
>> Le 31/01/2020 à 06:31, Russell Currey a écrit :
>>> + pr_info("attempting bad read at %px with write allowed\n",
>>> ptr);
>>> + tmp = *ptr;
>>> + tmp += 0xc0dec0de;
>>> + prevent_write_to_user(ptr, sizeof(unsigned long));
>>
>> Does it work ? I would have thought that if the read fails the
>> process
>> will die and the following test won't be performed.
>
> Correct, the ACCESS_USERSPACE test does the same thing. Splitting this
> into separate R and W tests makes sense, even if it is unlikely that
> one would be broken without the other.
>
Or once we are using user_access_begin() stuff, we can use
unsafe_put_user() and unsafe_get_user() which should return an error
instead of killing the caller.
Christophe
^ permalink raw reply
* Re: [PATCH] lkdtm: Test KUAP directional user access unlocks on powerpc
From: Russell Currey @ 2020-01-31 6:53 UTC (permalink / raw)
To: Christophe Leroy, keescook, mpe
Cc: kernel-hardening, linuxppc-dev, linux-kernel, dja
In-Reply-To: <1b40cea6-0675-731a-58b1-bdc65f1e495e@c-s.fr>
On Fri, 2020-01-31 at 07:44 +0100, Christophe Leroy wrote:
>
> Le 31/01/2020 à 06:31, Russell Currey a écrit :
> > Kernel Userspace Access Prevention (KUAP) on powerpc supports
> > allowing only one access direction (Read or Write) when allowing
> > access
> > to or from user memory.
> >
> > A bug was recently found that showed that these one-way unlocks
> > never
> > worked, and allowing Read *or* Write would actually unlock Read
> > *and*
> > Write. We should have a test case for this so we can make sure
> > this
> > doesn't happen again.
> >
> > Like ACCESS_USERSPACE, the correct result is for the test to fault.
> >
> > At the time of writing this, the upstream kernel still has this bug
> > present, so the test will allow both accesses whereas
> > ACCESS_USERSPACE
> > will correctly fault.
> >
> > Signed-off-by: Russell Currey <ruscur@russell.cc>
> > ---
> > drivers/misc/lkdtm/core.c | 3 +++
> > drivers/misc/lkdtm/lkdtm.h | 3 +++
> > drivers/misc/lkdtm/perms.c | 43
> > ++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 49 insertions(+)
> >
> > diff --git a/drivers/misc/lkdtm/core.c b/drivers/misc/lkdtm/core.c
> > index ee0d6e721441..baef3c6f48d6 100644
> > --- a/drivers/misc/lkdtm/core.c
> > +++ b/drivers/misc/lkdtm/core.c
> > @@ -137,6 +137,9 @@ static const struct crashtype crashtypes[] = {
> > CRASHTYPE(EXEC_USERSPACE),
> > CRASHTYPE(EXEC_NULL),
> > CRASHTYPE(ACCESS_USERSPACE),
> > +#ifdef CONFIG_PPC_KUAP
> > + CRASHTYPE(ACCESS_USERSPACE_KUAP),
> > +#endif
>
> I'm not sure it is a good idea to build this test as a specific test
> for
> powerpc, more comments below.
>
> > CRASHTYPE(ACCESS_NULL),
> > CRASHTYPE(WRITE_RO),
> > CRASHTYPE(WRITE_RO_AFTER_INIT),
> > diff --git a/drivers/misc/lkdtm/lkdtm.h
> > b/drivers/misc/lkdtm/lkdtm.h
> > index c56d23e37643..406a3fb32e6f 100644
> > --- a/drivers/misc/lkdtm/lkdtm.h
> > +++ b/drivers/misc/lkdtm/lkdtm.h
> > @@ -57,6 +57,9 @@ void lkdtm_EXEC_RODATA(void);
> > void lkdtm_EXEC_USERSPACE(void);
> > void lkdtm_EXEC_NULL(void);
> > void lkdtm_ACCESS_USERSPACE(void);
> > +#ifdef CONFIG_PPC_KUAP
> > +void lkdtm_ACCESS_USERSPACE_KUAP(void);
> > +#endif
> > void lkdtm_ACCESS_NULL(void);
> >
> > /* lkdtm_refcount.c */
> > diff --git a/drivers/misc/lkdtm/perms.c
> > b/drivers/misc/lkdtm/perms.c
> > index 62f76d506f04..2c9aa0114333 100644
> > --- a/drivers/misc/lkdtm/perms.c
> > +++ b/drivers/misc/lkdtm/perms.c
> > @@ -10,6 +10,9 @@
> > #include <linux/mman.h>
> > #include <linux/uaccess.h>
> > #include <asm/cacheflush.h>
> > +#ifdef CONFIG_PPC_KUAP
> > +#include <asm/uaccess.h>
> > +#endif
>
> asm/uaccess.h is already included by linux/uaccess.h
I should have actually read the other includes rather than assuming I
needed this, pretty silly
>
> >
> > /* Whether or not to fill the target memory area with
> > do_nothing(). */
> > #define CODE_WRITE true
> > @@ -200,6 +203,46 @@ void lkdtm_ACCESS_USERSPACE(void)
> > vm_munmap(user_addr, PAGE_SIZE);
> > }
> >
> > +/* Test that KUAP's directional user access unlocks work as
> > intended */
> > +#ifdef CONFIG_PPC_KUAP
> > +void lkdtm_ACCESS_USERSPACE_KUAP(void)
> > +{
> > + unsigned long user_addr, tmp = 0;
> > + unsigned long *ptr;
>
> Should be a __user ptr because allow_write_to_user() and friends
> takes
> __user pointers.
>
> > +
> > + user_addr = vm_mmap(NULL, 0, PAGE_SIZE,
> > + PROT_READ | PROT_WRITE | PROT_EXEC,
> > + MAP_ANONYMOUS | MAP_PRIVATE, 0);
> > + if (user_addr >= TASK_SIZE) {
>
> Should use IS_ERR_VALUE() here.
>
> > + pr_warn("Failed to allocate user memory\n");
> > + return;
> > + }
> > +
> > + if (copy_to_user((void __user *)user_addr, &tmp, sizeof(tmp)))
> > {
>
> Should use ptr instead of casted user_addr.
>
> Why using copy_to_user() for writing an unsigned long ? put_user()
> should be enough.
>
> > + pr_warn("copy_to_user failed\n");
> > + vm_munmap(user_addr, PAGE_SIZE);
> > + return;
> > + }
> > +
> > + ptr = (unsigned long *)user_addr;
>
> move before copy_to_user() and use there.
All of the above is from the original ACCESS_USERSPACE test, not to
imply that it's perfect, but I'm not sure it's worth changing in one
place only
>
> > +
> > + /* Allowing "write to" should not allow "read from" */
> > + allow_write_to_user(ptr, sizeof(unsigned long));
>
> This is powerpc specific. I think we should build this around the
> user_access_begin()/user_access_end() generic fonctions.
>
> I'm about to propose an enhancement to this in order to allow
> unlocking
> only read or write. See discussion at
> https://patchwork.ozlabs.org/patch/1227926/.
>
> My plan is to propose my enhancement once powerpc implementation of
> user_access_begin stuff is merged. I don't know if Michael is still
> planning to merge the series for 5.6
> (https://patchwork.ozlabs.org/patch/1228801/ - patch 1 of the series
> has
> already been merged by Linus in 5.5)
You're correct, making generic user_access_begin() calls aware of
direction solves the arch-specific problem, so unless your series
somehow ends up being unviable (or taking a very long time to get
merged) we can drop this idea and have a generic implementation
instead.
>
>
> > + pr_info("attempting bad read at %px with write allowed\n",
> > ptr);
> > + tmp = *ptr;
> > + tmp += 0xc0dec0de;
> > + prevent_write_to_user(ptr, sizeof(unsigned long));
>
> Does it work ? I would have thought that if the read fails the
> process
> will die and the following test won't be performed.
Correct, the ACCESS_USERSPACE test does the same thing. Splitting this
into separate R and W tests makes sense, even if it is unlikely that
one would be broken without the other.
- Russell
>
> > +
> > + /* Allowing "read from" should not allow "write to" */
> > + allow_read_from_user(ptr, sizeof(unsigned long));
> > + pr_info("attempting bad write at %px with read allowed\n",
> > ptr);
> > + *ptr = tmp;
> > + prevent_read_from_user(ptr, sizeof(unsigned long));
> > +
> > + vm_munmap(user_addr, PAGE_SIZE);
> > +}
> > +#endif
> > +
> > void lkdtm_ACCESS_NULL(void)
> > {
> > unsigned long tmp;
> >
>
> Christophe
^ permalink raw reply
* Re: [PATCH] lkdtm: Test KUAP directional user access unlocks on powerpc
From: Christophe Leroy @ 2020-01-31 6:44 UTC (permalink / raw)
To: Russell Currey, keescook, mpe
Cc: kernel-hardening, linuxppc-dev, linux-kernel, dja
In-Reply-To: <20200131053157.22463-1-ruscur@russell.cc>
Le 31/01/2020 à 06:31, Russell Currey a écrit :
> Kernel Userspace Access Prevention (KUAP) on powerpc supports
> allowing only one access direction (Read or Write) when allowing access
> to or from user memory.
>
> A bug was recently found that showed that these one-way unlocks never
> worked, and allowing Read *or* Write would actually unlock Read *and*
> Write. We should have a test case for this so we can make sure this
> doesn't happen again.
>
> Like ACCESS_USERSPACE, the correct result is for the test to fault.
>
> At the time of writing this, the upstream kernel still has this bug
> present, so the test will allow both accesses whereas ACCESS_USERSPACE
> will correctly fault.
>
> Signed-off-by: Russell Currey <ruscur@russell.cc>
> ---
> drivers/misc/lkdtm/core.c | 3 +++
> drivers/misc/lkdtm/lkdtm.h | 3 +++
> drivers/misc/lkdtm/perms.c | 43 ++++++++++++++++++++++++++++++++++++++
> 3 files changed, 49 insertions(+)
>
> diff --git a/drivers/misc/lkdtm/core.c b/drivers/misc/lkdtm/core.c
> index ee0d6e721441..baef3c6f48d6 100644
> --- a/drivers/misc/lkdtm/core.c
> +++ b/drivers/misc/lkdtm/core.c
> @@ -137,6 +137,9 @@ static const struct crashtype crashtypes[] = {
> CRASHTYPE(EXEC_USERSPACE),
> CRASHTYPE(EXEC_NULL),
> CRASHTYPE(ACCESS_USERSPACE),
> +#ifdef CONFIG_PPC_KUAP
> + CRASHTYPE(ACCESS_USERSPACE_KUAP),
> +#endif
I'm not sure it is a good idea to build this test as a specific test for
powerpc, more comments below.
> CRASHTYPE(ACCESS_NULL),
> CRASHTYPE(WRITE_RO),
> CRASHTYPE(WRITE_RO_AFTER_INIT),
> diff --git a/drivers/misc/lkdtm/lkdtm.h b/drivers/misc/lkdtm/lkdtm.h
> index c56d23e37643..406a3fb32e6f 100644
> --- a/drivers/misc/lkdtm/lkdtm.h
> +++ b/drivers/misc/lkdtm/lkdtm.h
> @@ -57,6 +57,9 @@ void lkdtm_EXEC_RODATA(void);
> void lkdtm_EXEC_USERSPACE(void);
> void lkdtm_EXEC_NULL(void);
> void lkdtm_ACCESS_USERSPACE(void);
> +#ifdef CONFIG_PPC_KUAP
> +void lkdtm_ACCESS_USERSPACE_KUAP(void);
> +#endif
> void lkdtm_ACCESS_NULL(void);
>
> /* lkdtm_refcount.c */
> diff --git a/drivers/misc/lkdtm/perms.c b/drivers/misc/lkdtm/perms.c
> index 62f76d506f04..2c9aa0114333 100644
> --- a/drivers/misc/lkdtm/perms.c
> +++ b/drivers/misc/lkdtm/perms.c
> @@ -10,6 +10,9 @@
> #include <linux/mman.h>
> #include <linux/uaccess.h>
> #include <asm/cacheflush.h>
> +#ifdef CONFIG_PPC_KUAP
> +#include <asm/uaccess.h>
> +#endif
asm/uaccess.h is already included by linux/uaccess.h
>
> /* Whether or not to fill the target memory area with do_nothing(). */
> #define CODE_WRITE true
> @@ -200,6 +203,46 @@ void lkdtm_ACCESS_USERSPACE(void)
> vm_munmap(user_addr, PAGE_SIZE);
> }
>
> +/* Test that KUAP's directional user access unlocks work as intended */
> +#ifdef CONFIG_PPC_KUAP
> +void lkdtm_ACCESS_USERSPACE_KUAP(void)
> +{
> + unsigned long user_addr, tmp = 0;
> + unsigned long *ptr;
Should be a __user ptr because allow_write_to_user() and friends takes
__user pointers.
> +
> + user_addr = vm_mmap(NULL, 0, PAGE_SIZE,
> + PROT_READ | PROT_WRITE | PROT_EXEC,
> + MAP_ANONYMOUS | MAP_PRIVATE, 0);
> + if (user_addr >= TASK_SIZE) {
Should use IS_ERR_VALUE() here.
> + pr_warn("Failed to allocate user memory\n");
> + return;
> + }
> +
> + if (copy_to_user((void __user *)user_addr, &tmp, sizeof(tmp))) {
Should use ptr instead of casted user_addr.
Why using copy_to_user() for writing an unsigned long ? put_user()
should be enough.
> + pr_warn("copy_to_user failed\n");
> + vm_munmap(user_addr, PAGE_SIZE);
> + return;
> + }
> +
> + ptr = (unsigned long *)user_addr;
move before copy_to_user() and use there.
> +
> + /* Allowing "write to" should not allow "read from" */
> + allow_write_to_user(ptr, sizeof(unsigned long));
This is powerpc specific. I think we should build this around the
user_access_begin()/user_access_end() generic fonctions.
I'm about to propose an enhancement to this in order to allow unlocking
only read or write. See discussion at
https://patchwork.ozlabs.org/patch/1227926/.
My plan is to propose my enhancement once powerpc implementation of
user_access_begin stuff is merged. I don't know if Michael is still
planning to merge the series for 5.6
(https://patchwork.ozlabs.org/patch/1228801/ - patch 1 of the series has
already been merged by Linus in 5.5)
> + pr_info("attempting bad read at %px with write allowed\n", ptr);
> + tmp = *ptr;
> + tmp += 0xc0dec0de;
> + prevent_write_to_user(ptr, sizeof(unsigned long));
Does it work ? I would have thought that if the read fails the process
will die and the following test won't be performed.
> +
> + /* Allowing "read from" should not allow "write to" */
> + allow_read_from_user(ptr, sizeof(unsigned long));
> + pr_info("attempting bad write at %px with read allowed\n", ptr);
> + *ptr = tmp;
> + prevent_read_from_user(ptr, sizeof(unsigned long));
> +
> + vm_munmap(user_addr, PAGE_SIZE);
> +}
> +#endif
> +
> void lkdtm_ACCESS_NULL(void)
> {
> unsigned long tmp;
>
Christophe
^ permalink raw reply
* [PATCH] lkdtm: Test KUAP directional user access unlocks on powerpc
From: Russell Currey @ 2020-01-31 5:31 UTC (permalink / raw)
To: keescook
Cc: kernel-hardening, linux-kernel, Russell Currey, linuxppc-dev, dja
Kernel Userspace Access Prevention (KUAP) on powerpc supports
allowing only one access direction (Read or Write) when allowing access
to or from user memory.
A bug was recently found that showed that these one-way unlocks never
worked, and allowing Read *or* Write would actually unlock Read *and*
Write. We should have a test case for this so we can make sure this
doesn't happen again.
Like ACCESS_USERSPACE, the correct result is for the test to fault.
At the time of writing this, the upstream kernel still has this bug
present, so the test will allow both accesses whereas ACCESS_USERSPACE
will correctly fault.
Signed-off-by: Russell Currey <ruscur@russell.cc>
---
drivers/misc/lkdtm/core.c | 3 +++
drivers/misc/lkdtm/lkdtm.h | 3 +++
drivers/misc/lkdtm/perms.c | 43 ++++++++++++++++++++++++++++++++++++++
3 files changed, 49 insertions(+)
diff --git a/drivers/misc/lkdtm/core.c b/drivers/misc/lkdtm/core.c
index ee0d6e721441..baef3c6f48d6 100644
--- a/drivers/misc/lkdtm/core.c
+++ b/drivers/misc/lkdtm/core.c
@@ -137,6 +137,9 @@ static const struct crashtype crashtypes[] = {
CRASHTYPE(EXEC_USERSPACE),
CRASHTYPE(EXEC_NULL),
CRASHTYPE(ACCESS_USERSPACE),
+#ifdef CONFIG_PPC_KUAP
+ CRASHTYPE(ACCESS_USERSPACE_KUAP),
+#endif
CRASHTYPE(ACCESS_NULL),
CRASHTYPE(WRITE_RO),
CRASHTYPE(WRITE_RO_AFTER_INIT),
diff --git a/drivers/misc/lkdtm/lkdtm.h b/drivers/misc/lkdtm/lkdtm.h
index c56d23e37643..406a3fb32e6f 100644
--- a/drivers/misc/lkdtm/lkdtm.h
+++ b/drivers/misc/lkdtm/lkdtm.h
@@ -57,6 +57,9 @@ void lkdtm_EXEC_RODATA(void);
void lkdtm_EXEC_USERSPACE(void);
void lkdtm_EXEC_NULL(void);
void lkdtm_ACCESS_USERSPACE(void);
+#ifdef CONFIG_PPC_KUAP
+void lkdtm_ACCESS_USERSPACE_KUAP(void);
+#endif
void lkdtm_ACCESS_NULL(void);
/* lkdtm_refcount.c */
diff --git a/drivers/misc/lkdtm/perms.c b/drivers/misc/lkdtm/perms.c
index 62f76d506f04..2c9aa0114333 100644
--- a/drivers/misc/lkdtm/perms.c
+++ b/drivers/misc/lkdtm/perms.c
@@ -10,6 +10,9 @@
#include <linux/mman.h>
#include <linux/uaccess.h>
#include <asm/cacheflush.h>
+#ifdef CONFIG_PPC_KUAP
+#include <asm/uaccess.h>
+#endif
/* Whether or not to fill the target memory area with do_nothing(). */
#define CODE_WRITE true
@@ -200,6 +203,46 @@ void lkdtm_ACCESS_USERSPACE(void)
vm_munmap(user_addr, PAGE_SIZE);
}
+/* Test that KUAP's directional user access unlocks work as intended */
+#ifdef CONFIG_PPC_KUAP
+void lkdtm_ACCESS_USERSPACE_KUAP(void)
+{
+ unsigned long user_addr, tmp = 0;
+ unsigned long *ptr;
+
+ user_addr = vm_mmap(NULL, 0, PAGE_SIZE,
+ PROT_READ | PROT_WRITE | PROT_EXEC,
+ MAP_ANONYMOUS | MAP_PRIVATE, 0);
+ if (user_addr >= TASK_SIZE) {
+ pr_warn("Failed to allocate user memory\n");
+ return;
+ }
+
+ if (copy_to_user((void __user *)user_addr, &tmp, sizeof(tmp))) {
+ pr_warn("copy_to_user failed\n");
+ vm_munmap(user_addr, PAGE_SIZE);
+ return;
+ }
+
+ ptr = (unsigned long *)user_addr;
+
+ /* Allowing "write to" should not allow "read from" */
+ allow_write_to_user(ptr, sizeof(unsigned long));
+ pr_info("attempting bad read at %px with write allowed\n", ptr);
+ tmp = *ptr;
+ tmp += 0xc0dec0de;
+ prevent_write_to_user(ptr, sizeof(unsigned long));
+
+ /* Allowing "read from" should not allow "write to" */
+ allow_read_from_user(ptr, sizeof(unsigned long));
+ pr_info("attempting bad write at %px with read allowed\n", ptr);
+ *ptr = tmp;
+ prevent_read_from_user(ptr, sizeof(unsigned long));
+
+ vm_munmap(user_addr, PAGE_SIZE);
+}
+#endif
+
void lkdtm_ACCESS_NULL(void)
{
unsigned long tmp;
--
2.25.0
^ permalink raw reply related
* Re: [PATCH 1/5] mm/memremap_pages: Kill unused __devm_memremap_pages()
From: Christoph Hellwig @ 2020-01-31 5:32 UTC (permalink / raw)
To: Dan Williams
Cc: vishal.l.verma, linuxppc-dev, Christoph Hellwig, linux-kernel,
linux-nvdimm
In-Reply-To: <158041476158.3889308.4221100673554151124.stgit@dwillia2-desk3.amr.corp.intel.com>
On Thu, Jan 30, 2020 at 12:06:01PM -0800, Dan Williams wrote:
> Kill this definition that was introduced in commit 41e94a851304 ("add
> devm_memremap_pages") add never used.
>
> Cc: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
Looks good,
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply
* Re: [PATCH 09/10] powerpc: Enable OpenCAPI Storage Class Memory driver on bare metal
From: Dan Williams @ 2020-01-31 5:14 UTC (permalink / raw)
To: Alastair D'Silva
Cc: Oscar Salvador, Madhavan Srinivasan, Geert Uytterhoeven,
David Hildenbrand, Mahesh Salgaonkar, Wei Yang, Keith Busch,
Linux MM, Michal Hocko, Paul Mackerras, Ira Weiny,
Andrew Donnellan, Dave Jiang, linux-nvdimm, Vishal Verma,
Krzysztof Kozlowski, Anju T Sudhakar, Pavel Tatashin,
Arnd Bergmann, Greg Kurz, Qian Cai, Cédric Le Goater,
Thomas Gleixner, Hari Bathini, Greg Kroah-Hartman,
Linux Kernel Mailing List, Vasant Hegde, Frederic Barrat,
Andrew Morton, linuxppc-dev
In-Reply-To: <df24e47c2bd9472c7be06c6c266b2a250c30068f.camel@au1.ibm.com>
On Thu, Jan 30, 2020 at 8:57 PM Alastair D'Silva <alastair@au1.ibm.com> wrote:
>
> On Fri, 2019-11-08 at 08:10 +0100, Frederic Barrat wrote:
> >
> > Le 25/10/2019 à 06:47, Alastair D'Silva a écrit :
> > > From: Alastair D'Silva <alastair@d-silva.org>
> > >
> > > Enable OpenCAPI Storage Class Memory driver on bare metal
> > >
> > > Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> > > ---
> > > arch/powerpc/configs/powernv_defconfig | 4 ++++
> > > 1 file changed, 4 insertions(+)
> > >
> > > diff --git a/arch/powerpc/configs/powernv_defconfig
> > > b/arch/powerpc/configs/powernv_defconfig
> > > index 6658cceb928c..45c0eff94964 100644
> > > --- a/arch/powerpc/configs/powernv_defconfig
> > > +++ b/arch/powerpc/configs/powernv_defconfig
> > > @@ -352,3 +352,7 @@ CONFIG_KVM_BOOK3S_64=m
> > > CONFIG_KVM_BOOK3S_64_HV=m
> > > CONFIG_VHOST_NET=m
> > > CONFIG_PRINTK_TIME=y
> > > +CONFIG_OCXL_SCM=m
> > > +CONFIG_DEV_DAX=y
> > > +CONFIG_DEV_DAX_PMEM=y
This specific line is not needed since DEV_DAX_PMEM already defaults to DEV_DAX.
> > > +CONFIG_FS_DAX=y
> >
> > If this really the intent or do we want to activate DAX only if
> > CONFIG_OCXL_SCM is enabled?
> >
> > Fred
>
> We had a bit of a play around with reworking this the other day.
>
> Putting them in as depends didn't make sense, as they are "soft"
> dependancies - the driver works and you can do some things without DAX.
>
> Adding them as selects was rejected as selecting symbols that can also
> be manually select is discouraged.
>
> We ended up going full circle and adding them back to the defconfig.
This dovetails with a suggestion Dave made a while back [1]. Given all
the pieces that need to be turned on to have a "feature complete"
persistent memory enabled build it would be nice to have general
config symbols that go and select all the necessary dependencies for
DAX, and let the rest happen by default.
[1]: https://lore.kernel.org/lkml/20161129021052.GF28177@dastard/
^ permalink raw reply
* Re: [PATCH 09/10] powerpc: Enable OpenCAPI Storage Class Memory driver on bare metal
From: Alastair D'Silva @ 2020-01-31 4:56 UTC (permalink / raw)
To: Frederic Barrat
Cc: Oscar Salvador, Madhavan Srinivasan, Geert Uytterhoeven,
David Hildenbrand, Mahesh Salgaonkar, Wei Yang, Keith Busch,
linux-mm, Michal Hocko, Paul Mackerras, Ira Weiny, Dan Williams,
Andrew Donnellan, Dave Jiang, linux-nvdimm, Vishal Verma,
Krzysztof Kozlowski, Anju T Sudhakar, Pavel Tatashin,
Arnd Bergmann, Greg Kurz, Qian Cai, Cédric Le Goater,
Thomas Gleixner, Hari Bathini, Greg Kroah-Hartman, linux-kernel,
Vasant Hegde, Andrew Morton, linuxppc-dev
In-Reply-To: <3ba57ce6-9135-0d83-b99d-1c5b0c744855@linux.ibm.com>
On Fri, 2019-11-08 at 08:10 +0100, Frederic Barrat wrote:
>
> Le 25/10/2019 à 06:47, Alastair D'Silva a écrit :
> > From: Alastair D'Silva <alastair@d-silva.org>
> >
> > Enable OpenCAPI Storage Class Memory driver on bare metal
> >
> > Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> > ---
> > arch/powerpc/configs/powernv_defconfig | 4 ++++
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/arch/powerpc/configs/powernv_defconfig
> > b/arch/powerpc/configs/powernv_defconfig
> > index 6658cceb928c..45c0eff94964 100644
> > --- a/arch/powerpc/configs/powernv_defconfig
> > +++ b/arch/powerpc/configs/powernv_defconfig
> > @@ -352,3 +352,7 @@ CONFIG_KVM_BOOK3S_64=m
> > CONFIG_KVM_BOOK3S_64_HV=m
> > CONFIG_VHOST_NET=m
> > CONFIG_PRINTK_TIME=y
> > +CONFIG_OCXL_SCM=m
> > +CONFIG_DEV_DAX=y
> > +CONFIG_DEV_DAX_PMEM=y
> > +CONFIG_FS_DAX=y
>
> If this really the intent or do we want to activate DAX only if
> CONFIG_OCXL_SCM is enabled?
>
> Fred
We had a bit of a play around with reworking this the other day.
Putting them in as depends didn't make sense, as they are "soft"
dependancies - the driver works and you can do some things without DAX.
Adding them as selects was rejected as selecting symbols that can also
be manually select is discouraged.
We ended up going full circle and adding them back to the defconfig.
--
Alastair D'Silva
Open Source Developer
Linux Technology Centre, IBM Australia
mob: 0423 762 819
^ permalink raw reply
* Re: [PATCH v6 00/10] mm/memory_hotplug: Shrink zones before removing memory
From: Andrew Morton @ 2020-01-31 4:40 UTC (permalink / raw)
To: Oscar Salvador
Cc: Mark Rutland, Pankaj Gupta, Michal Hocko, linux-ia64,
David Hildenbrand, Peter Zijlstra, Catalin Marinas, Dave Hansen,
Heiko Carstens, x86, Wei Yang, linux-mm, Pavel Tatashin,
Rich Felker, Alexander Potapenko, H. Peter Anvin, Alexander Duyck,
Ira Weiny, Thomas Gleixner, Qian Cai, linux-s390, Yu Zhao,
Yoshinori Sato, Jason Gunthorpe, Aneesh Kumar K . V, linux-sh,
Matthew Wilcox (Oracle), Mike Rapoport, Halil Pasic,
Christian Borntraeger, Ingo Molnar, Gerald Schaefer, Fenghua Yu,
Pavel Tatashin, Vasily Gorbik, Anshuman Khandual, Vlastimil Babka,
Will Deacon, Robin Murphy, Jun Yao, Borislav Petkov,
Andy Lutomirski, Dan Williams, linux-arm-kernel, Tony Luck,
Mel Gorman, Masahiro Yamada, Greg Kroah-Hartman, Steve Capper,
linux-kernel, Logan Gunthorpe, Wei Yang, Paul Mackerras,
Tom Lendacky, linuxppc-dev
In-Reply-To: <20191203133633.GA2600@linux>
On Tue, 3 Dec 2019 14:36:38 +0100 Oscar Salvador <osalvador@suse.de> wrote:
> On Mon, Dec 02, 2019 at 10:09:51AM +0100, David Hildenbrand wrote:
> > @Michal, @Oscar, can some of you at least have a patch #5 now so we can
> > proceed with that? (the other patches can stay in -next some time longer)
>
> Hi,
>
> I will be having a look at patch#5 shortly.
>
> Thanks for the reminder
Things haven't improved a lot :(
mm-memmap_init-update-variable-name-in-memmap_init_zone.patch
mm-memory_hotplug-poison-memmap-in-remove_pfn_range_from_zone.patch
mm-memory_hotplug-we-always-have-a-zone-in-find_smallestbiggest_section_pfn.patch
mm-memory_hotplug-dont-check-for-all-holes-in-shrink_zone_span.patch
mm-memory_hotplug-drop-local-variables-in-shrink_zone_span.patch
mm-memory_hotplug-cleanup-__remove_pages.patch
The first patch has reviews, the remainder are unloved.
^ permalink raw reply
* Re: [PATCH 1/2] pseries/vio: Remove stray #ifdef CONFIG_PPC_PSERIES
From: Tyrel Datwyler @ 2020-01-31 0:37 UTC (permalink / raw)
To: Oliver O'Halloran, linuxppc-dev
In-Reply-To: <20200130063153.19915-1-oohall@gmail.com>
On 1/29/20 10:31 PM, Oliver O'Halloran wrote:
> vio.c requires CONFIG_IBMVIO which in turn depends on PPC_PSERIES.
> In other words, this ifdef is pointless. At a guess it's a carry-over
> from pre-history.
>
> Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
Reviewed-by: Tyrel Datwyler <tyreld@linux.ibm.com>
^ permalink raw reply
* Re: [PATCH 2/2] pseries/makefile: Remove CONFIG_PPC_PSERIES check
From: Tyrel Datwyler @ 2020-01-31 0:39 UTC (permalink / raw)
To: Oliver O'Halloran, linuxppc-dev
In-Reply-To: <20200130063153.19915-2-oohall@gmail.com>
On 1/29/20 10:31 PM, Oliver O'Halloran wrote:
> The platform makefile (arch/powerpc/platforms/pseries/Makefile) is only
> included by the platform makefile (arch/powerpc/platform/Makefile) when
> CONFIG_PPC_PSERIES is selected, so checking for CONFIG_PPC_PSERIES in the
> pseries makefile is pointless.
>
> Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
Reviewed-by: Tyrel Datwyler <tyreld@linux.ibm.com>
^ permalink raw reply
* Re: [PATCH v18 00/24] selftests, powerpc, x86: Memory Protection Keys
From: Dave Hansen @ 2020-01-30 21:51 UTC (permalink / raw)
To: Sandipan Das, shuah, skhan, linux-kselftest
Cc: linux-arch, fweimer, aneesh.kumar, x86, linuxram, mhocko,
linux-mm, mingo, msuchanek, linuxppc-dev, bauerman
In-Reply-To: <cover.1580365432.git.sandipan@linux.ibm.com>
On 1/29/20 10:36 PM, Sandipan Das wrote:
> v18:
> (1) Fixed issues with x86 multilib builds based on
> feedback from Dave.
> (2) Moved patch 2 to the end of the series.
These (finally) build and run successfully for me on an x86 system with
protection keys. Feel free to add my Tested-by, and Acked-by.
FWIW, I don't think look perfect, but my standards are lower for
selftests/ than normal kernel code. :)
^ permalink raw reply
* Re: powerpc Linux scv support and scv system call ABI proposal
From: Segher Boessenkool @ 2020-01-30 21:41 UTC (permalink / raw)
To: Adhemerval Zanella
Cc: Florian Weimer, libc-alpha, Tulio Magno Quites Machado Filho,
linuxppc-dev, Nicholas Piggin
In-Reply-To: <f46bafbd-c553-565a-38a4-73d81cc5a8d2@linaro.org>
Hi!
On Thu, Jan 30, 2020 at 02:04:51PM -0300, Adhemerval Zanella wrote:
> On 30/01/2020 10:50, Segher Boessenkool wrote:
> > On Thu, Jan 30, 2020 at 01:03:53PM +0100, Florian Weimer wrote:
> >>> This is why that *is* the only supported use. The documentation could
> >>> use a touch-up, I think. Unless we still have problems here?
> >>
> >> I really don't know. GCC still has *some* support for the old behavior,
> >> though.
> >
> > No. No support. It still does some of the same things, but that can
> > change (and probably should). But this hasn't been supported since the
> > dark ages, and the documentation has become gradually more explicit
> > about it.
> >
>
> I think this might be related to an odd sparc32 issue I am seeing with
> newer clock_nanosleep. The expanded code is:
>
> --
> register long err __asm__("g1"); // INTERNAL_SYSCALL_DECL (err)
> r = ({ // r = INTERNAL_SYSCALL_CANCEL (...)
> long int sc_ret;
> if (SINGLE_THREAD_P)
> sc_ret = INTERNAL_SYSCALL_CALL (__VA_ARGS__);
> else
> {
> int sc_cancel_oldtype = __libc_enable_asynccancel ();
> sc_ret = INTERNAL_SYSCALL_CALL (__VA_ARGS__); // It issues the syscall with the asm (...)
> __librt_disable_asynccancel (sc_cancel_oldtype);
> }
> sc_ret;
> });
> if ((void) (val), __builtin_expect((err) != 0, 0)) // if (! INTERNAL_SYSCALL_ERROR_P (r, err))
> return 0;
> if ((-(val)) != ENOSYS) // if (INTERNAL_SYSCALL_ERRNO (r, err) != ENOSYS)
> return ((-(val))); // return INTERNAL_SYSCALL_ERRNO (r, err);
>
> [...]
>
> r = ({ // r = INTERNAL_SYSCALL_CANCEL (...)
> [...]
> )}
> if ((void) (val), __builtin_expect((err) != 0, 0)) // if (! INTERNAL_SYSCALL_ERROR_P (r, err))
> {
> [...]
> }
> return ((void) (val), __builtin_expect((err) != 0, 0)) // return (INTERNAL_SYSCALL_ERROR_P (r, err)
> ? ((-(val))) : 0; // ? INTERNAL_SYSCALL_ERRNO (r, err) : 0);
> --
>
> It requires that 'err' (assigned to 'g1')
What do you mean by "assigned to g1"?
> be value propagated over
> functions calls and over different scopes, which I take from your
> explanation is not supported and fragile.
You probably misundertand that, but let me ask: where is err assigned to
at all in the code you quoted? I don't see it. Maybe it's hidden in some
macro?
Or, maybe some asm writes to g1? This is explicitly not okay (quote
from the GCC manual):
Defining a register variable does not reserve the register. Other than
when invoking the Extended 'asm', the contents of the specified register
are not guaranteed. For this reason, the following uses are explicitly
_not_ supported. If they appear to work, it is only happenstance, and
may stop working as intended due to (seemingly) unrelated changes in
surrounding code, or even minor changes in the optimization of a future
version of gcc:
* Passing parameters to or from Basic 'asm'
* Passing parameters to or from Extended 'asm' without using input or
output operands.
* Passing parameters to or from routines written in assembler (or
other languages) using non-standard calling conventions.
> It also seems that if I
> move the __libc_enable_* calls before 'err' initialization and after
> its usage the code seems to works, but again it seems this usage
> is not really supported on gcc.
>
> So it seems that the current usage of 'INTERNAL_SYSCALL_DECL' and
> 'INTERNAL_SYSCALL_ERROR_P' are fragile if the architecture *does*
> use the 'err' variable and it is defined a register alias (which
> its the case only for sparc currently).
>
> Although a straightforward for sparc would be redefine
> INTERNAL_SYSCALL_DECL to not use a register alias, I still think
> we should just follow Linux kernel ABI convention where value in
> the range between -4095 and -1 indicates an error and handle any
> specific symbols that might not strictly follow it with an
> arch-specific implementation (as we do for lseek on x32 and
> mips64n32). It would allow cleanup a lot of code and avoid such
> pitfalls.
I don't really understand what you call a "register alias", either.
(And i don't know the Sparc ABI well enough to help you with that).
Segher
^ permalink raw reply
* [PATCH 5/5] libnvdimm/region: Introduce an 'align' attribute
From: Dan Williams @ 2020-01-30 20:06 UTC (permalink / raw)
To: linux-nvdimm
Cc: Aneesh Kumar K.V, linux-kernel, Jeff Moyer, vishal.l.verma,
linuxppc-dev, hch
In-Reply-To: <158041475480.3889308.655103391935006598.stgit@dwillia2-desk3.amr.corp.intel.com>
The align attribute applies an alignment constraint for namespace
creation in a region. Whereas the 'align' attribute of a namespace
applied alignment padding via an info block, the 'align' attribute
applies alignment constraints to the free space allocation.
The default for 'align' is the maximum known memremap_compat_align()
across all archs (16MiB from PowerPC at time of writing) multiplied by
the number of interleave ways if there is blk-aliasing. The minimum is
PAGE_SIZE and allows for the creation of cross-arch incompatible
namespaces, just as previous kernels allowed, but the expectation is
cross-arch and mode-independent compatibility by default.
The regression risk with this change is limited to cases that were
dependent on the ability to create unaligned namespaces, *and* for some
reason are unable to opt-out of aligned namespaces by writing to
'regionX/align'. If such a scenario arises the default can be flipped
from opt-out to opt-in of compat-aligned namespace creation, but that is
a last resort. The kernel will otherwise continue to support existing
defined misaligned namespaces.
Unfortunately this change needs to touch several parts of the
implementation at once:
- region/available_size: expand busy extents to current align
- region/max_available_extent: expand busy extents to current align
- namespace/size: trim free space to current align
...to keep the free space accounting conforming to the dynamic align
setting.
Reported-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
Reported-by: Jeff Moyer <jmoyer@redhat.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
drivers/nvdimm/dimm_devs.c | 86 +++++++++++++++++++++++----
drivers/nvdimm/namespace_devs.c | 9 ++-
drivers/nvdimm/nd.h | 1
drivers/nvdimm/region_devs.c | 122 ++++++++++++++++++++++++++++++++++++---
4 files changed, 192 insertions(+), 26 deletions(-)
diff --git a/drivers/nvdimm/dimm_devs.c b/drivers/nvdimm/dimm_devs.c
index 64159d4d4b8f..b4994abb655f 100644
--- a/drivers/nvdimm/dimm_devs.c
+++ b/drivers/nvdimm/dimm_devs.c
@@ -563,6 +563,21 @@ int nvdimm_security_freeze(struct nvdimm *nvdimm)
return rc;
}
+static unsigned long dpa_align(struct nd_region *nd_region)
+{
+ struct device *dev = &nd_region->dev;
+
+ if (dev_WARN_ONCE(dev, !is_nvdimm_bus_locked(dev),
+ "bus lock required for capacity provision\n"))
+ return 0;
+ if (dev_WARN_ONCE(dev, !nd_region->ndr_mappings || nd_region->align
+ % nd_region->ndr_mappings,
+ "invalid region align %#lx mappings: %d\n",
+ nd_region->align, nd_region->ndr_mappings))
+ return 0;
+ return nd_region->align / nd_region->ndr_mappings;
+}
+
int alias_dpa_busy(struct device *dev, void *data)
{
resource_size_t map_end, blk_start, new;
@@ -571,6 +586,7 @@ int alias_dpa_busy(struct device *dev, void *data)
struct nd_region *nd_region;
struct nvdimm_drvdata *ndd;
struct resource *res;
+ unsigned long align;
int i;
if (!is_memory(dev))
@@ -608,13 +624,21 @@ int alias_dpa_busy(struct device *dev, void *data)
* Find the free dpa from the end of the last pmem allocation to
* the end of the interleave-set mapping.
*/
+ align = dpa_align(nd_region);
+ if (!align)
+ return 0;
+
for_each_dpa_resource(ndd, res) {
+ resource_size_t start, end;
+
if (strncmp(res->name, "pmem", 4) != 0)
continue;
- if ((res->start >= blk_start && res->start < map_end)
- || (res->end >= blk_start
- && res->end <= map_end)) {
- new = max(blk_start, min(map_end + 1, res->end + 1));
+
+ start = ALIGN_DOWN(res->start, align);
+ end = ALIGN(res->end + 1, align) - 1;
+ if ((start >= blk_start && start < map_end)
+ || (end >= blk_start && end <= map_end)) {
+ new = max(blk_start, min(map_end, end) + 1);
if (new != blk_start) {
blk_start = new;
goto retry;
@@ -654,6 +678,7 @@ resource_size_t nd_blk_available_dpa(struct nd_region *nd_region)
.res = NULL,
};
struct resource *res;
+ unsigned long align;
if (!ndd)
return 0;
@@ -661,10 +686,20 @@ resource_size_t nd_blk_available_dpa(struct nd_region *nd_region)
device_for_each_child(&nvdimm_bus->dev, &info, alias_dpa_busy);
/* now account for busy blk allocations in unaliased dpa */
+ align = dpa_align(nd_region);
+ if (!align)
+ return 0;
for_each_dpa_resource(ndd, res) {
+ resource_size_t start, end, size;
+
if (strncmp(res->name, "blk", 3) != 0)
continue;
- info.available -= resource_size(res);
+ start = ALIGN_DOWN(res->start, align);
+ end = ALIGN(res->end + 1, align) - 1;
+ size = end - start + 1;
+ if (size >= info.available)
+ return 0;
+ info.available -= size;
}
return info.available;
@@ -683,19 +718,31 @@ resource_size_t nd_pmem_max_contiguous_dpa(struct nd_region *nd_region,
struct nvdimm_bus *nvdimm_bus;
resource_size_t max = 0;
struct resource *res;
+ unsigned long align;
/* if a dimm is disabled the available capacity is zero */
if (!ndd)
return 0;
+ align = dpa_align(nd_region);
+ if (!align)
+ return 0;
+
nvdimm_bus = walk_to_nvdimm_bus(ndd->dev);
if (__reserve_free_pmem(&nd_region->dev, nd_mapping->nvdimm))
return 0;
for_each_dpa_resource(ndd, res) {
+ resource_size_t start, end;
+
if (strcmp(res->name, "pmem-reserve") != 0)
continue;
- if (resource_size(res) > max)
- max = resource_size(res);
+ /* trim free space relative to current alignment setting */
+ start = ALIGN(res->start, align);
+ end = ALIGN_DOWN(res->end + 1, align) - 1;
+ if (end < start)
+ continue;
+ if (end - start + 1 > max)
+ max = end - start + 1;
}
release_free_pmem(nvdimm_bus, nd_mapping);
return max;
@@ -723,24 +770,33 @@ resource_size_t nd_pmem_available_dpa(struct nd_region *nd_region,
struct nvdimm_drvdata *ndd = to_ndd(nd_mapping);
struct resource *res;
const char *reason;
+ unsigned long align;
if (!ndd)
return 0;
+ align = dpa_align(nd_region);
+ if (!align)
+ return 0;
+
map_start = nd_mapping->start;
map_end = map_start + nd_mapping->size - 1;
blk_start = max(map_start, map_end + 1 - *overlap);
for_each_dpa_resource(ndd, res) {
- if (res->start >= map_start && res->start < map_end) {
+ resource_size_t start, end;
+
+ start = ALIGN_DOWN(res->start, align);
+ end = ALIGN(res->end + 1, align) - 1;
+ if (start >= map_start && start < map_end) {
if (strncmp(res->name, "blk", 3) == 0)
blk_start = min(blk_start,
- max(map_start, res->start));
- else if (res->end > map_end) {
+ max(map_start, start));
+ else if (end > map_end) {
reason = "misaligned to iset";
goto err;
} else
- busy += resource_size(res);
- } else if (res->end >= map_start && res->end <= map_end) {
+ busy += end - start + 1;
+ } else if (end >= map_start && end <= map_end) {
if (strncmp(res->name, "blk", 3) == 0) {
/*
* If a BLK allocation overlaps the start of
@@ -749,8 +805,8 @@ resource_size_t nd_pmem_available_dpa(struct nd_region *nd_region,
*/
blk_start = map_start;
} else
- busy += resource_size(res);
- } else if (map_start > res->start && map_start < res->end) {
+ busy += end - start + 1;
+ } else if (map_start > start && map_start < end) {
/* total eclipse of the mapping */
busy += nd_mapping->size;
blk_start = map_start;
@@ -760,7 +816,7 @@ resource_size_t nd_pmem_available_dpa(struct nd_region *nd_region,
*overlap = map_end + 1 - blk_start;
available = blk_start - map_start;
if (busy < available)
- return available - busy;
+ return ALIGN_DOWN(available - busy, align);
return 0;
err:
diff --git a/drivers/nvdimm/namespace_devs.c b/drivers/nvdimm/namespace_devs.c
index 30cda9f235de..4720ad69e1c5 100644
--- a/drivers/nvdimm/namespace_devs.c
+++ b/drivers/nvdimm/namespace_devs.c
@@ -541,6 +541,11 @@ static void space_valid(struct nd_region *nd_region, struct nvdimm_drvdata *ndd,
{
bool is_reserve = strcmp(label_id->id, "pmem-reserve") == 0;
bool is_pmem = strncmp(label_id->id, "pmem", 4) == 0;
+ unsigned long align;
+
+ align = nd_region->align / nd_region->ndr_mappings;
+ valid->start = ALIGN(valid->start, align);
+ valid->end = ALIGN_DOWN(valid->end + 1, align) - 1;
if (valid->start >= valid->end)
goto invalid;
@@ -980,10 +985,10 @@ static ssize_t __size_store(struct device *dev, unsigned long long val)
return -ENXIO;
}
- div_u64_rem(val, PAGE_SIZE * nd_region->ndr_mappings, &remainder);
+ div_u64_rem(val, nd_region->align, &remainder);
if (remainder) {
dev_dbg(dev, "%llu is not %ldK aligned\n", val,
- (PAGE_SIZE * nd_region->ndr_mappings) / SZ_1K);
+ nd_region->align / SZ_1K);
return -EINVAL;
}
diff --git a/drivers/nvdimm/nd.h b/drivers/nvdimm/nd.h
index ca39abe29c7c..c4d69c1cce55 100644
--- a/drivers/nvdimm/nd.h
+++ b/drivers/nvdimm/nd.h
@@ -146,6 +146,7 @@ struct nd_region {
struct device *btt_seed;
struct device *pfn_seed;
struct device *dax_seed;
+ unsigned long align;
u16 ndr_mappings;
u64 ndr_size;
u64 ndr_start;
diff --git a/drivers/nvdimm/region_devs.c b/drivers/nvdimm/region_devs.c
index a5fc6e4c56ff..bf239e783940 100644
--- a/drivers/nvdimm/region_devs.c
+++ b/drivers/nvdimm/region_devs.c
@@ -216,21 +216,25 @@ int nd_region_to_nstype(struct nd_region *nd_region)
}
EXPORT_SYMBOL(nd_region_to_nstype);
-static ssize_t size_show(struct device *dev,
- struct device_attribute *attr, char *buf)
+static unsigned long long region_size(struct nd_region *nd_region)
{
- struct nd_region *nd_region = to_nd_region(dev);
- unsigned long long size = 0;
-
- if (is_memory(dev)) {
- size = nd_region->ndr_size;
+ if (is_memory(&nd_region->dev)) {
+ return nd_region->ndr_size;
} else if (nd_region->ndr_mappings == 1) {
struct nd_mapping *nd_mapping = &nd_region->mapping[0];
- size = nd_mapping->size;
+ return nd_mapping->size;
}
- return sprintf(buf, "%llu\n", size);
+ return 0;
+}
+
+static ssize_t size_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct nd_region *nd_region = to_nd_region(dev);
+
+ return sprintf(buf, "%llu\n", region_size(nd_region));
}
static DEVICE_ATTR_RO(size);
@@ -529,6 +533,55 @@ static ssize_t read_only_store(struct device *dev,
}
static DEVICE_ATTR_RW(read_only);
+static ssize_t align_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct nd_region *nd_region = to_nd_region(dev);
+
+ return sprintf(buf, "%#lx\n", nd_region->align);
+}
+
+static ssize_t align_store(struct device *dev,
+ struct device_attribute *attr, const char *buf, size_t len)
+{
+ struct nd_region *nd_region = to_nd_region(dev);
+ unsigned long val, dpa;
+ u32 remainder;
+ int rc;
+
+ rc = kstrtoul(buf, 0, &val);
+ if (rc)
+ return rc;
+
+ if (!nd_region->ndr_mappings)
+ return -ENXIO;
+
+ /*
+ * Ensure space-align is evenly divisible by the region
+ * interleave-width because the kernel typically has no facility
+ * to determine which DIMM(s), dimm-physical-addresses, would
+ * contribute to the tail capacity in system-physical-address
+ * space for the namespace.
+ */
+ dpa = val;
+ remainder = do_div(dpa, nd_region->ndr_mappings);
+ if (!is_power_of_2(dpa) || dpa < PAGE_SIZE
+ || val > region_size(nd_region) || remainder)
+ return -EINVAL;
+
+ /*
+ * Given that space allocation consults this value multiple
+ * times ensure it does not change for the duration of the
+ * allocation.
+ */
+ nvdimm_bus_lock(dev);
+ nd_region->align = val;
+ nvdimm_bus_unlock(dev);
+
+ return len;
+}
+static DEVICE_ATTR_RW(align);
+
static ssize_t region_badblocks_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
@@ -571,6 +624,7 @@ static DEVICE_ATTR_RO(persistence_domain);
static struct attribute *nd_region_attributes[] = {
&dev_attr_size.attr,
+ &dev_attr_align.attr,
&dev_attr_nstype.attr,
&dev_attr_mappings.attr,
&dev_attr_btt_seed.attr,
@@ -626,6 +680,19 @@ static umode_t region_visible(struct kobject *kobj, struct attribute *a, int n)
return a->mode;
}
+ if (a == &dev_attr_align.attr) {
+ int i;
+
+ for (i = 0; i < nd_region->ndr_mappings; i++) {
+ struct nd_mapping *nd_mapping = &nd_region->mapping[i];
+ struct nvdimm *nvdimm = nd_mapping->nvdimm;
+
+ if (test_bit(NDD_LABELING, &nvdimm->flags))
+ return a->mode;
+ }
+ return 0;
+ }
+
if (a != &dev_attr_set_cookie.attr
&& a != &dev_attr_available_size.attr)
return a->mode;
@@ -935,6 +1002,42 @@ void nd_region_release_lane(struct nd_region *nd_region, unsigned int lane)
}
EXPORT_SYMBOL(nd_region_release_lane);
+/*
+ * PowerPC requires this alignment for memremap_pages(). All other archs
+ * should be ok with SUBSECTION_SIZE (see memremap_compat_align()).
+ */
+#define MEMREMAP_COMPAT_ALIGN_MAX SZ_16M
+
+static unsigned long default_align(struct nd_region *nd_region)
+{
+ unsigned long align, per_mapping;
+ int i, mappings;
+ u32 remainder;
+
+ if (is_nd_blk(&nd_region->dev))
+ align = PAGE_SIZE;
+ else
+ align = MEMREMAP_COMPAT_ALIGN_MAX;
+
+ for (i = 0; i < nd_region->ndr_mappings; i++) {
+ struct nd_mapping *nd_mapping = &nd_region->mapping[i];
+ struct nvdimm *nvdimm = nd_mapping->nvdimm;
+
+ if (test_bit(NDD_ALIASING, &nvdimm->flags)) {
+ align = MEMREMAP_COMPAT_ALIGN_MAX;
+ break;
+ }
+ }
+
+ mappings = max_t(u16, 1, nd_region->ndr_mappings);
+ per_mapping = align;
+ remainder = do_div(per_mapping, mappings);
+ if (remainder)
+ align *= mappings;
+
+ return align;
+}
+
static struct nd_region *nd_region_create(struct nvdimm_bus *nvdimm_bus,
struct nd_region_desc *ndr_desc,
const struct device_type *dev_type, const char *caller)
@@ -1039,6 +1142,7 @@ static struct nd_region *nd_region_create(struct nvdimm_bus *nvdimm_bus,
dev->of_node = ndr_desc->of_node;
nd_region->ndr_size = resource_size(ndr_desc->res);
nd_region->ndr_start = ndr_desc->res->start;
+ nd_region->align = default_align(nd_region);
if (ndr_desc->flush)
nd_region->flush = ndr_desc->flush;
else
^ permalink raw reply related
* [PATCH 4/5] libnvdimm/region: Introduce NDD_LABELING
From: Dan Williams @ 2020-01-30 20:06 UTC (permalink / raw)
To: linux-nvdimm
Cc: Vishal Verma, linux-kernel, Oliver O'Halloran,
Aneesh Kumar K.V, linuxppc-dev, hch
In-Reply-To: <158041475480.3889308.655103391935006598.stgit@dwillia2-desk3.amr.corp.intel.com>
The NDD_ALIASING flag is used to indicate where pmem capacity might
alias with blk capacity and require labeling. It is also used to
indicate whether the DIMM supports labeling. Separate this latter
capability into its own flag so that the NDD_ALIASING flag is scoped to
true aliased configurations.
To my knowledge aliased configurations only exist in the ACPI spec,
there are no known platforms that ship this support in production.
This clarity allows namespace-capacity alignment constraints around
interleave-ways to be relaxed.
Cc: Vishal Verma <vishal.l.verma@intel.com>
Cc: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
Cc: Oliver O'Halloran <oohall@gmail.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
arch/powerpc/platforms/pseries/papr_scm.c | 2 +-
drivers/acpi/nfit/core.c | 4 +++-
drivers/nvdimm/dimm.c | 2 +-
drivers/nvdimm/dimm_devs.c | 9 +++++----
drivers/nvdimm/namespace_devs.c | 2 +-
drivers/nvdimm/nd.h | 2 +-
drivers/nvdimm/region_devs.c | 10 +++++-----
include/linux/libnvdimm.h | 2 ++
8 files changed, 19 insertions(+), 14 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c
index c2ef320ba1bf..aae60cfd4e38 100644
--- a/arch/powerpc/platforms/pseries/papr_scm.c
+++ b/arch/powerpc/platforms/pseries/papr_scm.c
@@ -326,7 +326,7 @@ static int papr_scm_nvdimm_init(struct papr_scm_priv *p)
}
dimm_flags = 0;
- set_bit(NDD_ALIASING, &dimm_flags);
+ set_bit(NDD_LABELING, &dimm_flags);
p->nvdimm = nvdimm_create(p->bus, p, NULL, dimm_flags,
PAPR_SCM_DIMM_CMD_MASK, 0, NULL);
diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
index a3320f93616d..71d7f2aa1b12 100644
--- a/drivers/acpi/nfit/core.c
+++ b/drivers/acpi/nfit/core.c
@@ -2026,8 +2026,10 @@ static int acpi_nfit_register_dimms(struct acpi_nfit_desc *acpi_desc)
continue;
}
- if (nfit_mem->bdw && nfit_mem->memdev_pmem)
+ if (nfit_mem->bdw && nfit_mem->memdev_pmem) {
set_bit(NDD_ALIASING, &flags);
+ set_bit(NDD_LABELING, &flags);
+ }
/* collate flags across all memdevs for this dimm */
list_for_each_entry(nfit_memdev, &acpi_desc->memdevs, list) {
diff --git a/drivers/nvdimm/dimm.c b/drivers/nvdimm/dimm.c
index 64776ed15bb3..7d4ddc4d9322 100644
--- a/drivers/nvdimm/dimm.c
+++ b/drivers/nvdimm/dimm.c
@@ -99,7 +99,7 @@ static int nvdimm_probe(struct device *dev)
if (ndd->ns_current >= 0) {
rc = nd_label_reserve_dpa(ndd);
if (rc == 0)
- nvdimm_set_aliasing(dev);
+ nvdimm_set_labeling(dev);
}
nvdimm_bus_unlock(dev);
diff --git a/drivers/nvdimm/dimm_devs.c b/drivers/nvdimm/dimm_devs.c
index 94ea6dba6b4f..64159d4d4b8f 100644
--- a/drivers/nvdimm/dimm_devs.c
+++ b/drivers/nvdimm/dimm_devs.c
@@ -32,7 +32,7 @@ int nvdimm_check_config_data(struct device *dev)
if (!nvdimm->cmd_mask ||
!test_bit(ND_CMD_GET_CONFIG_DATA, &nvdimm->cmd_mask)) {
- if (test_bit(NDD_ALIASING, &nvdimm->flags))
+ if (test_bit(NDD_LABELING, &nvdimm->flags))
return -ENXIO;
else
return -ENOTTY;
@@ -173,11 +173,11 @@ int nvdimm_set_config_data(struct nvdimm_drvdata *ndd, size_t offset,
return rc;
}
-void nvdimm_set_aliasing(struct device *dev)
+void nvdimm_set_labeling(struct device *dev)
{
struct nvdimm *nvdimm = to_nvdimm(dev);
- set_bit(NDD_ALIASING, &nvdimm->flags);
+ set_bit(NDD_LABELING, &nvdimm->flags);
}
void nvdimm_set_locked(struct device *dev)
@@ -312,8 +312,9 @@ static ssize_t flags_show(struct device *dev,
{
struct nvdimm *nvdimm = to_nvdimm(dev);
- return sprintf(buf, "%s%s\n",
+ return sprintf(buf, "%s%s%s\n",
test_bit(NDD_ALIASING, &nvdimm->flags) ? "alias " : "",
+ test_bit(NDD_LABELING, &nvdimm->flags) ? "label" : "",
test_bit(NDD_LOCKED, &nvdimm->flags) ? "lock " : "");
}
static DEVICE_ATTR_RO(flags);
diff --git a/drivers/nvdimm/namespace_devs.c b/drivers/nvdimm/namespace_devs.c
index aff1f32fdb4f..30cda9f235de 100644
--- a/drivers/nvdimm/namespace_devs.c
+++ b/drivers/nvdimm/namespace_devs.c
@@ -2531,7 +2531,7 @@ static int init_active_labels(struct nd_region *nd_region)
if (!ndd) {
if (test_bit(NDD_LOCKED, &nvdimm->flags))
/* fail, label data may be unreadable */;
- else if (test_bit(NDD_ALIASING, &nvdimm->flags))
+ else if (test_bit(NDD_LABELING, &nvdimm->flags))
/* fail, labels needed to disambiguate dpa */;
else
return 0;
diff --git a/drivers/nvdimm/nd.h b/drivers/nvdimm/nd.h
index c9f6a5b5253a..ca39abe29c7c 100644
--- a/drivers/nvdimm/nd.h
+++ b/drivers/nvdimm/nd.h
@@ -252,7 +252,7 @@ int nvdimm_set_config_data(struct nvdimm_drvdata *ndd, size_t offset,
void *buf, size_t len);
long nvdimm_clear_poison(struct device *dev, phys_addr_t phys,
unsigned int len);
-void nvdimm_set_aliasing(struct device *dev);
+void nvdimm_set_labeling(struct device *dev);
void nvdimm_set_locked(struct device *dev);
void nvdimm_clear_locked(struct device *dev);
int nvdimm_security_setup_events(struct device *dev);
diff --git a/drivers/nvdimm/region_devs.c b/drivers/nvdimm/region_devs.c
index a19e535830d9..a5fc6e4c56ff 100644
--- a/drivers/nvdimm/region_devs.c
+++ b/drivers/nvdimm/region_devs.c
@@ -195,16 +195,16 @@ EXPORT_SYMBOL_GPL(nd_blk_region_set_provider_data);
int nd_region_to_nstype(struct nd_region *nd_region)
{
if (is_memory(&nd_region->dev)) {
- u16 i, alias;
+ u16 i, label;
- for (i = 0, alias = 0; i < nd_region->ndr_mappings; i++) {
+ for (i = 0, label = 0; i < nd_region->ndr_mappings; i++) {
struct nd_mapping *nd_mapping = &nd_region->mapping[i];
struct nvdimm *nvdimm = nd_mapping->nvdimm;
- if (test_bit(NDD_ALIASING, &nvdimm->flags))
- alias++;
+ if (test_bit(NDD_LABELING, &nvdimm->flags))
+ label++;
}
- if (alias)
+ if (label)
return ND_DEVICE_NAMESPACE_PMEM;
else
return ND_DEVICE_NAMESPACE_IO;
diff --git a/include/linux/libnvdimm.h b/include/linux/libnvdimm.h
index 9df091bd30ba..18da4059be09 100644
--- a/include/linux/libnvdimm.h
+++ b/include/linux/libnvdimm.h
@@ -37,6 +37,8 @@ enum {
NDD_WORK_PENDING = 4,
/* ignore / filter NSLABEL_FLAG_LOCAL for this DIMM, i.e. no aliasing */
NDD_NOBLK = 5,
+ /* dimm supports namespace labels */
+ NDD_LABELING = 6,
/* need to set a limit somewhere, but yes, this is likely overkill */
ND_IOCTL_MAX_BUFLEN = SZ_4M,
^ permalink raw reply related
* [PATCH 3/5] libnvdimm/namespace: Enforce memremap_compat_align()
From: Dan Williams @ 2020-01-30 20:06 UTC (permalink / raw)
To: linux-nvdimm
Cc: Aneesh Kumar K.V, linux-kernel, Jeff Moyer, vishal.l.verma,
linuxppc-dev, hch
In-Reply-To: <158041475480.3889308.655103391935006598.stgit@dwillia2-desk3.amr.corp.intel.com>
The pmem driver on PowerPC crashes with the following signature when
instantiating misaligned namespaces that map their capacity via
memremap_pages().
BUG: Unable to handle kernel data access at 0xc001000406000000
Faulting instruction address: 0xc000000000090790
NIP [c000000000090790] arch_add_memory+0xc0/0x130
LR [c000000000090744] arch_add_memory+0x74/0x130
Call Trace:
arch_add_memory+0x74/0x130 (unreliable)
memremap_pages+0x74c/0xa30
devm_memremap_pages+0x3c/0xa0
pmem_attach_disk+0x188/0x770
nvdimm_bus_probe+0xd8/0x470
With the assumption that only memremap_pages() has alignment
constraints, enforce memremap_compat_align() for
pmem_should_map_pages(), nd_pfn, or nd_dax cases.
Reported-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
Cc: Jeff Moyer <jmoyer@redhat.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
drivers/nvdimm/namespace_devs.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/nvdimm/namespace_devs.c b/drivers/nvdimm/namespace_devs.c
index 032dc61725ff..aff1f32fdb4f 100644
--- a/drivers/nvdimm/namespace_devs.c
+++ b/drivers/nvdimm/namespace_devs.c
@@ -1739,6 +1739,16 @@ struct nd_namespace_common *nvdimm_namespace_common_probe(struct device *dev)
return ERR_PTR(-ENODEV);
}
+ if (pmem_should_map_pages(dev) || nd_pfn || nd_dax) {
+ struct nd_namespace_io *nsio = to_nd_namespace_io(&ndns->dev);
+ resource_size_t start = nsio->res.start;
+
+ if (!IS_ALIGNED(start | size, memremap_compat_align())) {
+ dev_dbg(&ndns->dev, "misaligned, unable to map\n");
+ return ERR_PTR(-EOPNOTSUPP);
+ }
+ }
+
if (is_namespace_pmem(&ndns->dev)) {
struct nd_namespace_pmem *nspm;
^ permalink raw reply related
* [PATCH 2/5] mm/memremap_pages: Introduce memremap_compat_align()
From: Dan Williams @ 2020-01-30 20:06 UTC (permalink / raw)
To: linux-nvdimm
Cc: Aneesh Kumar K.V, linux-kernel, Jeff Moyer, Paul Mackerras,
vishal.l.verma, linuxppc-dev, hch
In-Reply-To: <158041475480.3889308.655103391935006598.stgit@dwillia2-desk3.amr.corp.intel.com>
The "sub-section memory hotplug" facility allows memremap_pages() users
like libnvdimm to compensate for hardware platforms like x86 that have a
section size larger than their hardware memory mapping granularity. The
compensation that sub-section support affords is being tolerant of
physical memory resources shifting by units smaller (64MiB on x86) than
the memory-hotplug section size (128 MiB). Where the platform
physical-memory mapping granularity is limited by the number and
capability of address-decode-registers in the memory controller.
While the sub-section support allows memremap_pages() to operate on
sub-section (2MiB) granularity, the Power architecture may still
require 16MiB alignment on "!radix_enabled()" platforms.
In order for libnvdimm to be able to detect and manage this per-arch
limitation, introduce memremap_compat_align() as a common minimum
alignment across all driver-facing memory-mapping interfaces, and let
Power override it to 16MiB in the "!radix_enabled()" case.
The assumption / requirement for 16MiB to be a viable
memremap_compat_align() value is that Power does not have platforms
where its equivalent of address-decode-registers never hardware remaps a
persistent memory resource on smaller than 16MiB boundaries.
Based on an initial patch by Aneesh.
Link: http://lore.kernel.org/r/CAPcyv4gBGNP95APYaBcsocEa50tQj9b5h__83vgngjq3ouGX_Q@mail.gmail.com
Reported-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
Reported-by: Jeff Moyer <jmoyer@redhat.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
arch/powerpc/include/asm/io.h | 10 ++++++++++
drivers/nvdimm/pfn_devs.c | 2 +-
include/linux/io.h | 23 +++++++++++++++++++++++
include/linux/mmzone.h | 1 +
4 files changed, 35 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/include/asm/io.h b/arch/powerpc/include/asm/io.h
index a63ec938636d..0fa2dc483008 100644
--- a/arch/powerpc/include/asm/io.h
+++ b/arch/powerpc/include/asm/io.h
@@ -734,6 +734,16 @@ extern void __iomem * __ioremap_at(phys_addr_t pa, void *ea,
unsigned long size, pgprot_t prot);
extern void __iounmap_at(void *ea, unsigned long size);
+#ifdef CONFIG_SPARSEMEM
+static inline unsigned long memremap_compat_align(void)
+{
+ if (radix_enabled())
+ return SUBSECTION_SIZE;
+ return (1UL << mmu_psize_defs[mmu_linear_psize].shift);
+}
+#define memremap_compat_align memremap_compat_align
+#endif
+
/*
* When CONFIG_PPC_INDIRECT_PIO is set, we use the generic iomap implementation
* which needs some additional definitions here. They basically allow PIO
diff --git a/drivers/nvdimm/pfn_devs.c b/drivers/nvdimm/pfn_devs.c
index b94f7a7e94b8..a5c25cb87116 100644
--- a/drivers/nvdimm/pfn_devs.c
+++ b/drivers/nvdimm/pfn_devs.c
@@ -750,7 +750,7 @@ static int nd_pfn_init(struct nd_pfn *nd_pfn)
start = nsio->res.start;
size = resource_size(&nsio->res);
npfns = PHYS_PFN(size - SZ_8K);
- align = max(nd_pfn->align, (1UL << SUBSECTION_SHIFT));
+ align = max(nd_pfn->align, SUBSECTION_SIZE);
end_trunc = start + size - ALIGN_DOWN(start + size, align);
if (nd_pfn->mode == PFN_MODE_PMEM) {
/*
diff --git a/include/linux/io.h b/include/linux/io.h
index 35e8d84935e0..ccd34519fad3 100644
--- a/include/linux/io.h
+++ b/include/linux/io.h
@@ -6,6 +6,7 @@
#ifndef _LINUX_IO_H
#define _LINUX_IO_H
+#include <linux/mmzone.h>
#include <linux/types.h>
#include <linux/init.h>
#include <linux/bug.h>
@@ -79,6 +80,28 @@ void *devm_memremap(struct device *dev, resource_size_t offset,
size_t size, unsigned long flags);
void devm_memunmap(struct device *dev, void *addr);
+#ifndef memremap_compat_align
+#ifdef CONFIG_SPARSEMEM
+/*
+ * Minimum compatible alignment of the resource (start, end) across
+ * memremap interfaces (i.e. memremap + memremap_pages)
+ */
+static inline unsigned long memremap_compat_align(void)
+{
+ return SUBSECTION_SIZE;
+}
+#else /* CONFIG_SPARSEMEM */
+/*
+ * No ZONE_DEVICE / memremap_pages() support so the minimum mapping
+ * granularity is a single page.
+ */
+static inline unsigned long memremap_compat_align(void)
+{
+ return PAGE_SIZE;
+}
+#endif /* CONFIG_SPARSEMEM */
+#endif /* memremap_compat_align */
+
#ifdef CONFIG_PCI
/*
* The PCI specifications (Rev 3.0, 3.2.5 "Transaction Ordering and
diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
index 89d8ff06c9ce..b0de83620cd7 100644
--- a/include/linux/mmzone.h
+++ b/include/linux/mmzone.h
@@ -1171,6 +1171,7 @@ static inline unsigned long section_nr_to_pfn(unsigned long sec)
#define SECTION_ALIGN_DOWN(pfn) ((pfn) & PAGE_SECTION_MASK)
#define SUBSECTION_SHIFT 21
+#define SUBSECTION_SIZE (1UL << SUBSECTION_SHIFT)
#define PFN_SUBSECTION_SHIFT (SUBSECTION_SHIFT - PAGE_SHIFT)
#define PAGES_PER_SUBSECTION (1UL << PFN_SUBSECTION_SHIFT)
^ permalink raw reply related
* [PATCH 1/5] mm/memremap_pages: Kill unused __devm_memremap_pages()
From: Dan Williams @ 2020-01-30 20:06 UTC (permalink / raw)
To: linux-nvdimm
Cc: vishal.l.verma, linuxppc-dev, Christoph Hellwig, linux-kernel
In-Reply-To: <158041475480.3889308.655103391935006598.stgit@dwillia2-desk3.amr.corp.intel.com>
Kill this definition that was introduced in commit 41e94a851304 ("add
devm_memremap_pages") add never used.
Cc: Christoph Hellwig <hch@lst.de>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
include/linux/io.h | 2 --
1 file changed, 2 deletions(-)
diff --git a/include/linux/io.h b/include/linux/io.h
index a59834bc0a11..35e8d84935e0 100644
--- a/include/linux/io.h
+++ b/include/linux/io.h
@@ -79,8 +79,6 @@ void *devm_memremap(struct device *dev, resource_size_t offset,
size_t size, unsigned long flags);
void devm_memunmap(struct device *dev, void *addr);
-void *__devm_memremap_pages(struct device *dev, struct resource *res);
-
#ifdef CONFIG_PCI
/*
* The PCI specifications (Rev 3.0, 3.2.5 "Transaction Ordering and
^ permalink raw reply related
* [PATCH 0/5] libnvdimm: Cross-arch compatible namespace alignment
From: Dan Williams @ 2020-01-30 20:05 UTC (permalink / raw)
To: linux-nvdimm
Cc: linux-kernel, Vishal Verma, Jeff Moyer, Oliver O'Halloran,
Aneesh Kumar K.V, Paul Mackerras, linuxppc-dev, Christoph Hellwig
Aneesh reports that PowerPC requires 16MiB alignment for the address
range passed to devm_memremap_pages(), and Jeff reports that it is
possible to create a misaligned namespace which blocks future namespace
creation in that region. Both of these issues require namespace
alignment to be managed at the region level rather than padding at the
namespace level which has been a broken approach to date.
Introduce memremap_compat_align() to indicate the hard requirements of
an arch's memremap_pages() implementation. Use the maximum known
memremap_compat_align() to set the default namespace alignment for
libnvdimm. Consult that alignment when allocating free space. Finally,
allow the default region alignment to be overridden to maintain the same
namespace creation capability as previous kernels.
The ndctl unit tests, which have some misaligned namespace assumptions,
are updated to use the alignment override where necessary.
Thanks to Aneesh for early feedback and testing on this improved
alignment handling.
---
Dan Williams (5):
mm/memremap_pages: Kill unused __devm_memremap_pages()
mm/memremap_pages: Introduce memremap_compat_align()
libnvdimm/namespace: Enforce memremap_compat_align()
libnvdimm/region: Introduce NDD_LABELING
libnvdimm/region: Introduce an 'align' attribute
arch/powerpc/include/asm/io.h | 10 ++
arch/powerpc/platforms/pseries/papr_scm.c | 2
drivers/acpi/nfit/core.c | 4 +
drivers/nvdimm/dimm.c | 2
drivers/nvdimm/dimm_devs.c | 95 +++++++++++++++++----
drivers/nvdimm/namespace_devs.c | 21 ++++-
drivers/nvdimm/nd.h | 3 -
drivers/nvdimm/pfn_devs.c | 2
drivers/nvdimm/region_devs.c | 132 ++++++++++++++++++++++++++---
include/linux/io.h | 23 +++++
include/linux/libnvdimm.h | 2
include/linux/mmzone.h | 1
12 files changed, 255 insertions(+), 42 deletions(-)
^ permalink raw reply
* Re: [PATCH v2] powerpc: Do not consider weak unresolved symbol relocations as bad
From: Alex Ghiti @ 2020-01-30 20:07 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
linuxppc-dev, linux-kernel, Stephen Rothwell, Alexei Starovoitov,
linux-next, Zong Li, Palmer Dabbelt
In-Reply-To: <20200118170335.21440-1-alex@ghiti.fr>
On 1/18/20 12:03 PM, Alexandre Ghiti wrote:
> Commit 8580ac9404f6 ("bpf: Process in-kernel BTF") introduced two weak
> symbols that may be unresolved at link time which result in an absolute
> relocation to 0. relocs_check.sh emits the following warning:
>
> "WARNING: 2 bad relocations
> c000000001a41478 R_PPC64_ADDR64 _binary__btf_vmlinux_bin_start
> c000000001a41480 R_PPC64_ADDR64 _binary__btf_vmlinux_bin_end"
>
> whereas those relocations are legitimate even for a relocatable kernel
> compiled with -pie option.
>
> relocs_check.sh already excluded some weak unresolved symbols explicitly:
> remove those hardcoded symbols and add some logic that parses the symbols
> using nm, retrieves all the weak unresolved symbols and excludes those from
> the list of the potential bad relocations.
>
> Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
> Signed-off-by: Alexandre Ghiti <alex@ghiti.fr>
> ---
>
> Changes in v2:
> - Follow Stephen advice of using grep -F instead of looping over weak symbols
> using read, patch is way smaller and cleaner.
> - Add missing nm in comment
>
> arch/powerpc/Makefile.postlink | 4 ++--
> arch/powerpc/tools/relocs_check.sh | 20 ++++++++++++--------
> 2 files changed, 14 insertions(+), 10 deletions(-)
>
> diff --git a/arch/powerpc/Makefile.postlink b/arch/powerpc/Makefile.postlink
> index 134f12f89b92..2268396ff4bb 100644
> --- a/arch/powerpc/Makefile.postlink
> +++ b/arch/powerpc/Makefile.postlink
> @@ -17,11 +17,11 @@ quiet_cmd_head_check = CHKHEAD $@
> quiet_cmd_relocs_check = CHKREL $@
> ifdef CONFIG_PPC_BOOK3S_64
> cmd_relocs_check = \
> - $(CONFIG_SHELL) $(srctree)/arch/powerpc/tools/relocs_check.sh "$(OBJDUMP)" "$@" ; \
> + $(CONFIG_SHELL) $(srctree)/arch/powerpc/tools/relocs_check.sh "$(OBJDUMP)" "$(NM)" "$@" ; \
> $(BASH) $(srctree)/arch/powerpc/tools/unrel_branch_check.sh "$(OBJDUMP)" "$@"
> else
> cmd_relocs_check = \
> - $(CONFIG_SHELL) $(srctree)/arch/powerpc/tools/relocs_check.sh "$(OBJDUMP)" "$@"
> + $(CONFIG_SHELL) $(srctree)/arch/powerpc/tools/relocs_check.sh "$(OBJDUMP)" "$(NM)" "$@"
> endif
>
> # `@true` prevents complaint when there is nothing to be done
> diff --git a/arch/powerpc/tools/relocs_check.sh b/arch/powerpc/tools/relocs_check.sh
> index 7b9fe0a567cf..014e00e74d2b 100755
> --- a/arch/powerpc/tools/relocs_check.sh
> +++ b/arch/powerpc/tools/relocs_check.sh
> @@ -10,14 +10,21 @@
> # based on relocs_check.pl
> # Copyright © 2009 IBM Corporation
>
> -if [ $# -lt 2 ]; then
> - echo "$0 [path to objdump] [path to vmlinux]" 1>&2
> +if [ $# -lt 3 ]; then
> + echo "$0 [path to objdump] [path to nm] [path to vmlinux]" 1>&2
> exit 1
> fi
>
> -# Have Kbuild supply the path to objdump so we handle cross compilation.
> +# Have Kbuild supply the path to objdump and nm so we handle cross compilation.
> objdump="$1"
> -vmlinux="$2"
> +nm="$2"
> +vmlinux="$3"
> +
> +# Remove from the bad relocations those that match an undefined weak symbol
> +# which will result in an absolute relocation to 0.
> +# Weak unresolved symbols are of that form in nm output:
> +# " w _binary__btf_vmlinux_bin_end"
> +undef_weak_symbols=$($nm "$vmlinux" | awk '$1 ~ /w/ { print $2 }')
>
> bad_relocs=$(
> $objdump -R "$vmlinux" |
> @@ -26,8 +33,6 @@ $objdump -R "$vmlinux" |
> # These relocations are okay
> # On PPC64:
> # R_PPC64_RELATIVE, R_PPC64_NONE
> - # R_PPC64_ADDR64 mach_<name>
> - # R_PPC64_ADDR64 __crc_<name>
> # On PPC:
> # R_PPC_RELATIVE, R_PPC_ADDR16_HI,
> # R_PPC_ADDR16_HA,R_PPC_ADDR16_LO,
> @@ -39,8 +44,7 @@ R_PPC_ADDR16_HI
> R_PPC_ADDR16_HA
> R_PPC_RELATIVE
> R_PPC_NONE' |
> - grep -E -v '\<R_PPC64_ADDR64[[:space:]]+mach_' |
> - grep -E -v '\<R_PPC64_ADDR64[[:space:]]+__crc_'
> + ([ "$undef_weak_symbols" ] && grep -F -w -v "$undef_weak_symbols" || cat)
> )
>
> if [ -z "$bad_relocs" ]; then
Hi guys,
Any thought about that ?
I do think this patch makes the whole check about absolute relocations
clearer.
And in the future, it will avoid anyone to spend some time on those
"bad" relocations
which actually aren't.
Thanks,
Alex
^ permalink raw reply
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