* Re: [RESEND 3/7] mm/gup: Change GUP fast to use flags rather than a write 'bool'
From: Ira Weiny @ 2019-03-25 8:26 UTC (permalink / raw)
To: Dan Williams
Cc: Michal Hocko, Linux-sh, Peter Zijlstra, James Hogan,
Heiko Carstens, linux-mips, linux-mm, Rich Felker, Paul Mackerras,
sparclinux, linux-s390, Yoshinori Sato, linux-rdma,
Jason Gunthorpe, Ingo Molnar, John Hubbard, Borislav Petkov,
Thomas Gleixner, netdev@vger.kernel.org,
Linux Kernel Mailing List, Ralf Baechle, Martin Schwidefsky,
Andrew Morton, linuxppc-dev, David S. Miller, Kirill A. Shutemov
In-Reply-To: <CAA9_cmd9gUWMbpkP_AuxZ08iqvZdxjbtDoR-FpSjAyhZJisRZA@mail.gmail.com>
On Fri, Mar 22, 2019 at 03:05:53PM -0700, Dan Williams wrote:
> On Sun, Mar 17, 2019 at 7:36 PM <ira.weiny@intel.com> wrote:
> >
> > From: Ira Weiny <ira.weiny@intel.com>
> >
> > To facilitate additional options to get_user_pages_fast() change the
> > singular write parameter to be gup_flags.
> >
> > This patch does not change any functionality. New functionality will
> > follow in subsequent patches.
> >
> > Some of the get_user_pages_fast() call sites were unchanged because they
> > already passed FOLL_WRITE or 0 for the write parameter.
> >
> > Signed-off-by: Ira Weiny <ira.weiny@intel.com>
> >
> > ---
> > Changes from V1:
> > Rebase to current merge tree
> > arch/powerpc/mm/mmu_context_iommu.c no longer calls gup_fast
> > The gup_longterm was converted in patch 1
> >
> > arch/mips/mm/gup.c | 11 ++++++-----
> > arch/powerpc/kvm/book3s_64_mmu_hv.c | 4 ++--
> > arch/powerpc/kvm/e500_mmu.c | 2 +-
> > arch/s390/kvm/interrupt.c | 2 +-
> > arch/s390/mm/gup.c | 12 ++++++------
> > arch/sh/mm/gup.c | 11 ++++++-----
> > arch/sparc/mm/gup.c | 9 +++++----
> > arch/x86/kvm/paging_tmpl.h | 2 +-
> > arch/x86/kvm/svm.c | 2 +-
> > drivers/fpga/dfl-afu-dma-region.c | 2 +-
> > drivers/gpu/drm/via/via_dmablit.c | 3 ++-
> > drivers/infiniband/hw/hfi1/user_pages.c | 3 ++-
> > drivers/misc/genwqe/card_utils.c | 2 +-
> > drivers/misc/vmw_vmci/vmci_host.c | 2 +-
> > drivers/misc/vmw_vmci/vmci_queue_pair.c | 6 ++++--
> > drivers/platform/goldfish/goldfish_pipe.c | 3 ++-
> > drivers/rapidio/devices/rio_mport_cdev.c | 4 +++-
> > drivers/sbus/char/oradax.c | 2 +-
> > drivers/scsi/st.c | 3 ++-
> > drivers/staging/gasket/gasket_page_table.c | 4 ++--
> > drivers/tee/tee_shm.c | 2 +-
> > drivers/vfio/vfio_iommu_spapr_tce.c | 3 ++-
> > drivers/vhost/vhost.c | 2 +-
> > drivers/video/fbdev/pvr2fb.c | 2 +-
> > drivers/virt/fsl_hypervisor.c | 2 +-
> > drivers/xen/gntdev.c | 2 +-
> > fs/orangefs/orangefs-bufmap.c | 2 +-
> > include/linux/mm.h | 4 ++--
> > kernel/futex.c | 2 +-
> > lib/iov_iter.c | 7 +++++--
> > mm/gup.c | 10 +++++-----
> > mm/util.c | 8 ++++----
> > net/ceph/pagevec.c | 2 +-
> > net/rds/info.c | 2 +-
> > net/rds/rdma.c | 3 ++-
> > 35 files changed, 79 insertions(+), 63 deletions(-)
>
>
> >
> > diff --git a/arch/mips/mm/gup.c b/arch/mips/mm/gup.c
> > index 0d14e0d8eacf..4c2b4483683c 100644
> > --- a/arch/mips/mm/gup.c
> > +++ b/arch/mips/mm/gup.c
> > @@ -235,7 +235,7 @@ int __get_user_pages_fast(unsigned long start, int nr_pages, int write,
> > * get_user_pages_fast() - pin user pages in memory
> > * @start: starting user address
> > * @nr_pages: number of pages from start to pin
> > - * @write: whether pages will be written to
> > + * @gup_flags: flags modifying pin behaviour
> > * @pages: array that receives pointers to the pages pinned.
> > * Should be at least nr_pages long.
> > *
> > @@ -247,8 +247,8 @@ int __get_user_pages_fast(unsigned long start, int nr_pages, int write,
> > * requested. If nr_pages is 0 or negative, returns 0. If no pages
> > * were pinned, returns -errno.
> > */
> > -int get_user_pages_fast(unsigned long start, int nr_pages, int write,
> > - struct page **pages)
> > +int get_user_pages_fast(unsigned long start, int nr_pages,
> > + unsigned int gup_flags, struct page **pages)
>
> This looks a tad scary given all related thrash especially when it's
> only 1 user that wants to do get_user_page_fast_longterm, right?
Agreed but the discussion back in Feb agreed that it would be better to make
get_user_pages_fast() use flags rather than add another *_longterm call, and I
agree.
> Maybe
> something like the following. Note I explicitly moved the flags to the
> end so that someone half paying attention that calls
> __get_user_pages_fast will get a compile error if they specify the
> args in the same order.
I did this to remain consistent with the other public calls. They put the
"return" pages parameter at the end. For example get_user_pages() is defined
this way with pages and vmas at the end.
long get_user_pages(unsigned long start, unsigned long nr_pages,
unsigned int gup_flags, struct page **pages,
struct vm_area_struct **vmas)
I'm pretty sure I got all the callers. Is this worth making the signature
"non-standard" WRT the other calls?
Ira
>
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 76ba638ceda8..c6c743bc2c68 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -1505,8 +1505,15 @@ static inline long
> get_user_pages_longterm(unsigned long start,
> }
> #endif /* CONFIG_FS_DAX */
>
> -int get_user_pages_fast(unsigned long start, int nr_pages, int write,
> - struct page **pages);
> +
> +int __get_user_pages_fast(unsigned long start, int nr_pages,
> + struct page **pages, unsigned int gup_flags);
> +
> +static inline int get_user_pages_fast(unsigned long start, int
> nr_pages, int write,
> + struct page **pages)
> +{
> + return __get_user_pages_fast(start, nr_pages, pages, write ?
> FOLL_WRITE);
> +}
>
> /* Container for pinned pfns / pages */
> struct frame_vector {
>
^ permalink raw reply
* Re: [PATCH] kbuild: strip whitespace in cmd_record_mcount findstring
From: Steven Rostedt @ 2019-03-25 16:21 UTC (permalink / raw)
To: Joe Lawrence
Cc: Michal Marek, linux-kbuild, linux-kernel, Nicholas Piggin,
Masahiro Yamada, linuxppc-dev
In-Reply-To: <20190325160438.8982-1-joe.lawrence@redhat.com>
On Mon, 25 Mar 2019 12:04:38 -0400
Joe Lawrence <joe.lawrence@redhat.com> wrote:
> CC_FLAGS_FTRACE may contain trailing whitespace that interferes with
> findstring.
>
> For example, commit 6977f95e63b9 ("powerpc: avoid -mno-sched-epilog on
> GCC 4.9 and newer") introduced a change such that on my ppc64le box,
> CC_FLAGS_FTRACE="-pg -mprofile-kernel ". (Note the trailing space.)
> When cmd_record_mcount is now invoked, findstring fails as the ftrace
> flags were found at very end of _c_flags, without the trailing space.
>
> _c_flags=" ... -pg -mprofile-kernel"
> CC_FLAGS_FTRACE="-pg -mprofile-kernel "
> ^
> findstring is looking for this extra space
Wow, what a bug.
>
> Remove the redundant whitespaces from CC_FLAGS_FTRACE in
> cmd_record_mcount to avoid this problem.
>
> Fixes: 6977f95e63b9 ("powerpc: avoid -mno-sched-epilog on GCC 4.9 and newer").
> Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
Acked-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
-- Steve
> ---
>
> Standard disclaimer: I'm not a kbuild expert, but this works around the
> problem I reported where ftrace and livepatch self-tests were failing as
> specified object files were not run through the recordmcount.pl script:
>
> ppc64le: ftrace self-tests and $(CC_FLAGS_FTRACE) broken?
> https://lists.ozlabs.org/pipermail/linuxppc-dev/2019-March/187298.html
>
> scripts/Makefile.build | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> index 2554a15ecf2b..74d402b5aa3c 100644
> --- a/scripts/Makefile.build
> +++ b/scripts/Makefile.build
> @@ -199,10 +199,10 @@ sub_cmd_record_mcount = perl $(srctree)/scripts/recordmcount.pl "$(ARCH)" \
> "$(if $(part-of-module),1,0)" "$(@)";
> recordmcount_source := $(srctree)/scripts/recordmcount.pl
> endif # BUILD_C_RECORDMCOUNT
> -cmd_record_mcount = \
> - if [ "$(findstring $(CC_FLAGS_FTRACE),$(_c_flags))" = \
> - "$(CC_FLAGS_FTRACE)" ]; then \
> - $(sub_cmd_record_mcount) \
> +cmd_record_mcount = \
> + if [ "$(findstring $(strip $(CC_FLAGS_FTRACE)),$(_c_flags))" = \
> + "$(strip $(CC_FLAGS_FTRACE))" ]; then \
> + $(sub_cmd_record_mcount) \
> fi
> endif # CC_USING_RECORD_MCOUNT
> endif # CONFIG_FTRACE_MCOUNT_RECORD
^ permalink raw reply
* Re: Regression in 5.1.0-rc2: PowerBook G4 Aluminum fails to boot - bisected to commit 0df977eafc792
From: Larry Finger @ 2019-03-25 16:19 UTC (permalink / raw)
To: Christophe Leroy, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras, ppc-dev
Cc: LKML
In-Reply-To: <1c1f7eae-0c49-dcb2-f27b-96a83d811df7@c-s.fr>
On 3/25/19 3:46 AM, Christophe Leroy wrote:
>
>
> Le 25/03/2019 à 01:49, Larry Finger a écrit :
>> A build of kernel 5.1.0-rc2 resulted in a failure to boot on my PowerBook G4
>> Aluminum. The bootstrap loads the initial kernel and issues the appropriate
>> start, but the machine hangs at that point.
>>
>> The problem does not depend on the choice of PPC32 processor type. This
>> machine has a 7447A according to /proc/cpuinfo.
>>
>> The problem was bisected to the following:
>>
>> commit 0df977eafc792a5365a7f81d8d5920132e03afad
>> Author: Christophe Leroy <christophe.leroy@c-s.fr>
>> Date: Thu Feb 21 10:37:54 2019 +0000
>>
>> powerpc/6xx: Don't use SPRN_SPRG2 for storing stack pointer while in RTAS
>>
>> When calling RTAS, the stack pointer is stored in SPRN_SPRG2
>> in order to be able to restore it in case of machine check in RTAS.
>>
>> As machine check is not a perfomance critical path, this patch
>> frees SPRN_SPRG2 by using a field in thread struct instead.
>>
>> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
>> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
>>
>> I reverted this patch and found that the system began execution, and then
>> failed, likely due to the reassignment of SPRN_SPRG2.
>>
>> I had found this problem with 5.1.0-rc1, but -rc2 was out by the time I
>> finished the bisection. Unfortunately, none of the changes in -rc2 fixed the
>> problem.
>
> I think I identified the issue, see https://patchwork.ozlabs.org/patch/1063962/
>
> It is now booting properly under QEMU with your .config
>
> Could you please test it on your target ?
Thanks. That patch fixes the issue.
Larry
^ permalink raw reply
* Re: [PATCH] powerpc/rtas: fix early boot failure.
From: Larry Finger @ 2019-03-25 16:18 UTC (permalink / raw)
To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras,
Michael Ellerman
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <aab41779cac995ebdc03e700a9dc7cfc30a28770.1553503163.git.christophe.leroy@c-s.fr>
On 3/25/19 3:43 AM, Christophe Leroy wrote:
> Commit 0df977eafc79 ("powerpc/6xx: Don't use SPRN_SPRG2 for storing
> stack pointer while in RTAS") changes the code to use a field in
> thread struct to store the stack pointer while in RTAS instead of
> using SPRN_SPRG2. It therefore converts all places which were
> manipulating SPRN_SPRG2 to use that field. During early startup,
> the zeroing of SPRN_SPRG2 has been replaced by a zeroing of that
> field in thread struct. But at least in start_here, that's done
> wrongly because it used the physical address of the fields while
> MMU is on at that time.
>
> So the virtual address of the field should be used instead, but in
> the meantime, thread struct has already been zeroised and initialised
> so we can just drop this initialisation.
>
> Reported-by: Larry Finger <Larry.Finger@lwfinger.net>
> Fixes: 0df977eafc79 ("powerpc/6xx: Don't use SPRN_SPRG2 for storing stack pointer while in RTAS")
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
Tested-by: Larry Finger <Larry.Finger@lwfinger.net>
My PPC box now boots OK.
Larry
^ permalink raw reply
* Re: ppc64le: ftrace self-tests and $(CC_FLAGS_FTRACE) broken?
From: Joe Lawrence @ 2019-03-25 16:07 UTC (permalink / raw)
To: Steven Rostedt; +Cc: linuxppc-dev, Nicholas Piggin
In-Reply-To: <16061c33-4b42-8b8c-008c-123b2991677c@redhat.com>
On 3/23/19 1:27 PM, Joe Lawrence wrote:
> On 03/23/2019 12:17 PM, Steven Rostedt wrote:
>> On Sat, 23 Mar 2019 09:19:50 -0400
>> Joe Lawrence <joe.lawrence@redhat.com> wrote:
>>
>>> Perhaps this is gcc version specific? I didn't see any other reports,
>>> so maybe its specific to my config. If I run make V=1, I can see that
>>> gcc is called with '-pg -mprofile-kernel', but then the record_mcount
>>> script is skipped. Any ideas?
>>
>> But you see it running the script if you remove that commit? Do you
>> happen to see -mrecord-mcount at all in a V=1 make?
>>
>
> See entire make output from the two runs here:
> http://people.redhat.com/~jolawren/ppc64le-mprofile/
Mystery solved! That other commit inadvertently added a trailing space
to CC_FLAGS_FTRACE, which then confused the findstring call in the
cmd_record_mcount Makefile function.
Workaround patch just posted ... kbuild experts can suggest better fixes
if that one is sub optimal.
Thanks,
-- Joe
^ permalink raw reply
* [PATCH] kbuild: strip whitespace in cmd_record_mcount findstring
From: Joe Lawrence @ 2019-03-25 16:04 UTC (permalink / raw)
To: linux-kbuild, linuxppc-dev, linux-kernel
Cc: Masahiro Yamada, Michal Marek, Joe Lawrence, Steven Rostedt,
Nicholas Piggin
CC_FLAGS_FTRACE may contain trailing whitespace that interferes with
findstring.
For example, commit 6977f95e63b9 ("powerpc: avoid -mno-sched-epilog on
GCC 4.9 and newer") introduced a change such that on my ppc64le box,
CC_FLAGS_FTRACE="-pg -mprofile-kernel ". (Note the trailing space.)
When cmd_record_mcount is now invoked, findstring fails as the ftrace
flags were found at very end of _c_flags, without the trailing space.
_c_flags=" ... -pg -mprofile-kernel"
CC_FLAGS_FTRACE="-pg -mprofile-kernel "
^
findstring is looking for this extra space
Remove the redundant whitespaces from CC_FLAGS_FTRACE in
cmd_record_mcount to avoid this problem.
Fixes: 6977f95e63b9 ("powerpc: avoid -mno-sched-epilog on GCC 4.9 and newer").
Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
---
Standard disclaimer: I'm not a kbuild expert, but this works around the
problem I reported where ftrace and livepatch self-tests were failing as
specified object files were not run through the recordmcount.pl script:
ppc64le: ftrace self-tests and $(CC_FLAGS_FTRACE) broken?
https://lists.ozlabs.org/pipermail/linuxppc-dev/2019-March/187298.html
scripts/Makefile.build | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 2554a15ecf2b..74d402b5aa3c 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -199,10 +199,10 @@ sub_cmd_record_mcount = perl $(srctree)/scripts/recordmcount.pl "$(ARCH)" \
"$(if $(part-of-module),1,0)" "$(@)";
recordmcount_source := $(srctree)/scripts/recordmcount.pl
endif # BUILD_C_RECORDMCOUNT
-cmd_record_mcount = \
- if [ "$(findstring $(CC_FLAGS_FTRACE),$(_c_flags))" = \
- "$(CC_FLAGS_FTRACE)" ]; then \
- $(sub_cmd_record_mcount) \
+cmd_record_mcount = \
+ if [ "$(findstring $(strip $(CC_FLAGS_FTRACE)),$(_c_flags))" = \
+ "$(strip $(CC_FLAGS_FTRACE))" ]; then \
+ $(sub_cmd_record_mcount) \
fi
endif # CC_USING_RECORD_MCOUNT
endif # CONFIG_FTRACE_MCOUNT_RECORD
--
2.20.1
^ permalink raw reply related
* Re: Regression in 5.1.0-rc2: PowerBook G4 Aluminum fails to boot - bisected to commit 0df977eafc792
From: Larry Finger @ 2019-03-25 15:54 UTC (permalink / raw)
To: Christophe Leroy, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras, ppc-dev
Cc: LKML
In-Reply-To: <ee247f37-8889-934c-571c-01c43fc31071@c-s.fr>
On 3/25/19 1:53 AM, Christophe Leroy wrote:
>
>
> Le 25/03/2019 à 01:49, Larry Finger a écrit :
>> A build of kernel 5.1.0-rc2 resulted in a failure to boot on my PowerBook G4
>> Aluminum. The bootstrap loads the initial kernel and issues the appropriate
>> start, but the machine hangs at that point.
>
> Can you please be more explicit ? What do you mean by "issues the appropriate
> start" ? What is "that point" ? Any messages on the console ?
Sorry to be unclear. The bootstrap code prints a line saying "Booting Linux via
__start() @ 0x00c00000" and then hangs.
Your idea of confusion between physical and virtual address sounds right. I am
building a kernel with that patch applied now. As it seems to be going quickly,
I should have the answer fairly quickly.
Thanks,
Larry
^ permalink raw reply
* Re: [PATCH v3 7/7] ocxl: Provide global MMIO accessors for external drivers
From: Frederic Barrat @ 2019-03-25 15:49 UTC (permalink / raw)
To: Alastair D'Silva, alastair
Cc: Arnd Bergmann, Greg Kroah-Hartman, Greg Kurz, linux-kernel,
Andrew Donnellan, linuxppc-dev
In-Reply-To: <20190325054438.15022-8-alastair@au1.ibm.com>
Le 25/03/2019 à 06:44, Alastair D'Silva a écrit :
> From: Alastair D'Silva <alastair@d-silva.org>
>
> External drivers that communicate via OpenCAPI will need to make
> MMIO calls to interact with the devices.
>
> Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> Reviewed-by: Greg Kurz <groug@kaod.org>
> ---
Acked-by: Frederic Barrat <fbarrat@linux.ibm.com>
> drivers/misc/ocxl/Makefile | 2 +-
> drivers/misc/ocxl/mmio.c | 234 +++++++++++++++++++++++++++++++++++++
> include/misc/ocxl.h | 110 +++++++++++++++++
> 3 files changed, 345 insertions(+), 1 deletion(-)
> create mode 100644 drivers/misc/ocxl/mmio.c
>
> diff --git a/drivers/misc/ocxl/Makefile b/drivers/misc/ocxl/Makefile
> index bc4e39bfda7b..d07d1bb8e8d4 100644
> --- a/drivers/misc/ocxl/Makefile
> +++ b/drivers/misc/ocxl/Makefile
> @@ -1,7 +1,7 @@
> # SPDX-License-Identifier: GPL-2.0+
> ccflags-$(CONFIG_PPC_WERROR) += -Werror
>
> -ocxl-y += main.o pci.o config.o file.o pasid.o
> +ocxl-y += main.o pci.o config.o file.o pasid.o mmio.o
> ocxl-y += link.o context.o afu_irq.o sysfs.o trace.o
> ocxl-y += core.o
> obj-$(CONFIG_OCXL) += ocxl.o
> diff --git a/drivers/misc/ocxl/mmio.c b/drivers/misc/ocxl/mmio.c
> new file mode 100644
> index 000000000000..aae713db4ebe
> --- /dev/null
> +++ b/drivers/misc/ocxl/mmio.c
> @@ -0,0 +1,234 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +// Copyright 2019 IBM Corp.
> +#include <linux/sched/mm.h>
> +#include "trace.h"
> +#include "ocxl_internal.h"
> +
> +int ocxl_global_mmio_read32(struct ocxl_afu *afu, size_t offset,
> + enum ocxl_endian endian, u32 *val)
> +{
> + if (offset > afu->config.global_mmio_size - 4)
> + return -EINVAL;
> +
> +#ifdef __BIG_ENDIAN__
> + if (endian == OCXL_HOST_ENDIAN)
> + endian = OCXL_BIG_ENDIAN;
> +#endif
> +
> + switch (endian) {
> + case OCXL_BIG_ENDIAN:
> + *val = readl_be((char *)afu->global_mmio_ptr + offset);
> + break;
> +
> + default:
> + *val = readl((char *)afu->global_mmio_ptr + offset);
> + break;
> + }
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(ocxl_global_mmio_read32);
> +
> +int ocxl_global_mmio_read64(struct ocxl_afu *afu, size_t offset,
> + enum ocxl_endian endian, u64 *val)
> +{
> + if (offset > afu->config.global_mmio_size - 8)
> + return -EINVAL;
> +
> +#ifdef __BIG_ENDIAN__
> + if (endian == OCXL_HOST_ENDIAN)
> + endian = OCXL_BIG_ENDIAN;
> +#endif
> +
> + switch (endian) {
> + case OCXL_BIG_ENDIAN:
> + *val = readq_be((char *)afu->global_mmio_ptr + offset);
> + break;
> +
> + default:
> + *val = readq((char *)afu->global_mmio_ptr + offset);
> + break;
> + }
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(ocxl_global_mmio_read64);
> +
> +int ocxl_global_mmio_write32(struct ocxl_afu *afu, size_t offset,
> + enum ocxl_endian endian, u32 val)
> +{
> + if (offset > afu->config.global_mmio_size - 4)
> + return -EINVAL;
> +
> +#ifdef __BIG_ENDIAN__
> + if (endian == OCXL_HOST_ENDIAN)
> + endian = OCXL_BIG_ENDIAN;
> +#endif
> +
> + switch (endian) {
> + case OCXL_BIG_ENDIAN:
> + writel_be(val, (char *)afu->global_mmio_ptr + offset);
> + break;
> +
> + default:
> + writel(val, (char *)afu->global_mmio_ptr + offset);
> + break;
> + }
> +
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(ocxl_global_mmio_write32);
> +
> +int ocxl_global_mmio_write64(struct ocxl_afu *afu, size_t offset,
> + enum ocxl_endian endian, u64 val)
> +{
> + if (offset > afu->config.global_mmio_size - 8)
> + return -EINVAL;
> +
> +#ifdef __BIG_ENDIAN__
> + if (endian == OCXL_HOST_ENDIAN)
> + endian = OCXL_BIG_ENDIAN;
> +#endif
> +
> + switch (endian) {
> + case OCXL_BIG_ENDIAN:
> + writeq_be(val, (char *)afu->global_mmio_ptr + offset);
> + break;
> +
> + default:
> + writeq(val, (char *)afu->global_mmio_ptr + offset);
> + break;
> + }
> +
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(ocxl_global_mmio_write64);
> +
> +int ocxl_global_mmio_set32(struct ocxl_afu *afu, size_t offset,
> + enum ocxl_endian endian, u32 mask)
> +{
> + u32 tmp;
> +
> + if (offset > afu->config.global_mmio_size - 4)
> + return -EINVAL;
> +
> +#ifdef __BIG_ENDIAN__
> + if (endian == OCXL_HOST_ENDIAN)
> + endian = OCXL_BIG_ENDIAN;
> +#endif
> +
> + switch (endian) {
> + case OCXL_BIG_ENDIAN:
> + tmp = readl_be((char *)afu->global_mmio_ptr + offset);
> + tmp |= mask;
> + writel_be(tmp, (char *)afu->global_mmio_ptr + offset);
> + break;
> +
> + default:
> + tmp = readl((char *)afu->global_mmio_ptr + offset);
> + tmp |= mask;
> + writel(tmp, (char *)afu->global_mmio_ptr + offset);
> + break;
> + }
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(ocxl_global_mmio_set32);
> +
> +int ocxl_global_mmio_set64(struct ocxl_afu *afu, size_t offset,
> + enum ocxl_endian endian, u64 mask)
> +{
> + u64 tmp;
> +
> + if (offset > afu->config.global_mmio_size - 8)
> + return -EINVAL;
> +
> +#ifdef __BIG_ENDIAN__
> + if (endian == OCXL_HOST_ENDIAN)
> + endian = OCXL_BIG_ENDIAN;
> +#endif
> +
> + switch (endian) {
> + case OCXL_BIG_ENDIAN:
> + tmp = readq_be((char *)afu->global_mmio_ptr + offset);
> + tmp |= mask;
> + writeq_be(tmp, (char *)afu->global_mmio_ptr + offset);
> + break;
> +
> + default:
> + tmp = readq((char *)afu->global_mmio_ptr + offset);
> + tmp |= mask;
> + writeq(tmp, (char *)afu->global_mmio_ptr + offset);
> + break;
> + }
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(ocxl_global_mmio_set64);
> +
> +int ocxl_global_mmio_clear32(struct ocxl_afu *afu, size_t offset,
> + enum ocxl_endian endian, u32 mask)
> +{
> + u32 tmp;
> +
> + if (offset > afu->config.global_mmio_size - 4)
> + return -EINVAL;
> +
> +#ifdef __BIG_ENDIAN__
> + if (endian == OCXL_HOST_ENDIAN)
> + endian = OCXL_BIG_ENDIAN;
> +#endif
> +
> + switch (endian) {
> + case OCXL_BIG_ENDIAN:
> + tmp = readl_be((char *)afu->global_mmio_ptr + offset);
> + tmp &= ~mask;
> + writel_be(tmp, (char *)afu->global_mmio_ptr + offset);
> + break;
> +
> + default:
> + tmp = readl((char *)afu->global_mmio_ptr + offset);
> + tmp &= ~mask;
> + writel(tmp, (char *)afu->global_mmio_ptr + offset);
> + break;
> + }
> +
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(ocxl_global_mmio_clear32);
> +
> +int ocxl_global_mmio_clear64(struct ocxl_afu *afu, size_t offset,
> + enum ocxl_endian endian, u64 mask)
> +{
> + u64 tmp;
> +
> + if (offset > afu->config.global_mmio_size - 8)
> + return -EINVAL;
> +
> +#ifdef __BIG_ENDIAN__
> + if (endian == OCXL_HOST_ENDIAN)
> + endian = OCXL_BIG_ENDIAN;
> +#endif
> +
> + switch (endian) {
> + case OCXL_BIG_ENDIAN:
> + tmp = readq_be((char *)afu->global_mmio_ptr + offset);
> + tmp &= ~mask;
> + writeq_be(tmp, (char *)afu->global_mmio_ptr + offset);
> + break;
> +
> + default:
> + tmp = readq((char *)afu->global_mmio_ptr + offset);
> + tmp &= ~mask;
> + writeq(tmp, (char *)afu->global_mmio_ptr + offset);
> + break;
> + }
> +
> + writeq(tmp, (char *)afu->global_mmio_ptr + offset);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(ocxl_global_mmio_clear64);
> diff --git a/include/misc/ocxl.h b/include/misc/ocxl.h
> index 1b48e9d63abb..e0742c00fb72 100644
> --- a/include/misc/ocxl.h
> +++ b/include/misc/ocxl.h
> @@ -45,6 +45,12 @@ struct ocxl_fn_config {
> s8 max_afu_index;
> };
>
> +enum ocxl_endian {
> + OCXL_BIG_ENDIAN = 0, /**< AFU data is big-endian */
> + OCXL_LITTLE_ENDIAN = 1, /**< AFU data is little-endian */
> + OCXL_HOST_ENDIAN = 2, /**< AFU data is the same endianness as the host */
> +};
> +
> // These are opaque outside the ocxl driver
> struct ocxl_afu;
> struct ocxl_fn;
> @@ -236,6 +242,110 @@ void ocxl_afu_set_private(struct ocxl_afu *afu, void *private);
> */
> void *ocxl_afu_get_private(struct ocxl_afu *dev);
>
> +// Global MMIO
> +/**
> + * Read a 32 bit value from global MMIO
> + *
> + * @afu: The AFU
> + * @offset: The Offset from the start of MMIO
> + * @endian: the endianness that the MMIO data is in
> + * @val: returns the value
> + *
> + * Returns 0 for success, negative on error
> + */
> +int ocxl_global_mmio_read32(struct ocxl_afu *afu, size_t offset,
> + enum ocxl_endian endian, u32 *val);
> +
> +/**
> + * Read a 64 bit value from global MMIO
> + *
> + * @afu: The AFU
> + * @offset: The Offset from the start of MMIO
> + * @endian: the endianness that the MMIO data is in
> + * @val: returns the value
> + *
> + * Returns 0 for success, negative on error
> + */
> +int ocxl_global_mmio_read64(struct ocxl_afu *afu, size_t offset,
> + enum ocxl_endian endian, u64 *val);
> +
> +/**
> + * Write a 32 bit value to global MMIO
> + *
> + * @afu: The AFU
> + * @offset: The Offset from the start of MMIO
> + * @endian: the endianness that the MMIO data is in
> + * @val: The value to write
> + *
> + * Returns 0 for success, negative on error
> + */
> +int ocxl_global_mmio_write32(struct ocxl_afu *afu, size_t offset,
> + enum ocxl_endian endian, u32 val);
> +
> +/**
> + * Write a 64 bit value to global MMIO
> + *
> + * @afu: The AFU
> + * @offset: The Offset from the start of MMIO
> + * @endian: the endianness that the MMIO data is in
> + * @val: The value to write
> + *
> + * Returns 0 for success, negative on error
> + */
> +int ocxl_global_mmio_write64(struct ocxl_afu *afu, size_t offset,
> + enum ocxl_endian endian, u64 val);
> +
> +/**
> + * Set bits in a 32 bit global MMIO register
> + *
> + * @afu: The AFU
> + * @offset: The Offset from the start of MMIO
> + * @endian: the endianness that the MMIO data is in
> + * @mask: a mask of the bits to set
> + *
> + * Returns 0 for success, negative on error
> + */
> +int ocxl_global_mmio_set32(struct ocxl_afu *afu, size_t offset,
> + enum ocxl_endian endian, u32 mask);
> +
> +/**
> + * Set bits in a 64 bit global MMIO register
> + *
> + * @afu: The AFU
> + * @offset: The Offset from the start of MMIO
> + * @endian: the endianness that the MMIO data is in
> + * @mask: a mask of the bits to set
> + *
> + * Returns 0 for success, negative on error
> + */
> +int ocxl_global_mmio_set64(struct ocxl_afu *afu, size_t offset,
> + enum ocxl_endian endian, u64 mask);
> +
> +/**
> + * Set bits in a 32 bit global MMIO register
> + *
> + * @afu: The AFU
> + * @offset: The Offset from the start of MMIO
> + * @endian: the endianness that the MMIO data is in
> + * @mask: a mask of the bits to set
> + *
> + * Returns 0 for success, negative on error
> + */
> +int ocxl_global_mmio_clear32(struct ocxl_afu *afu, size_t offset,
> + enum ocxl_endian endian, u32 mask);
> +
> +/**
> + * Set bits in a 64 bit global MMIO register
> + *
> + * @afu: The AFU
> + * @offset: The Offset from the start of MMIO
> + * @endian: the endianness that the MMIO data is in
> + * @mask: a mask of the bits to set
> + *
> + * Returns 0 for success, negative on error
> + */
> +int ocxl_global_mmio_clear64(struct ocxl_afu *afu, size_t offset,
> + enum ocxl_endian endian, u64 mask);
>
> // Functions left here are for compatibility with the cxlflash driver
>
>
^ permalink raw reply
* Re: [PATCH v3 6/7] ocxl: move event_fd handling to frontend
From: Frederic Barrat @ 2019-03-25 15:41 UTC (permalink / raw)
To: Alastair D'Silva, alastair
Cc: Greg Kroah-Hartman, linuxppc-dev, Arnd Bergmann, Andrew Donnellan,
linux-kernel
In-Reply-To: <20190325054438.15022-7-alastair@au1.ibm.com>
Le 25/03/2019 à 06:44, Alastair D'Silva a écrit :
> From: Alastair D'Silva <alastair@d-silva.org>
>
> Event_fd is only used in the driver frontend, so it does not
> need to exist in the backend code. Relocate it to the frontend
> and provide an opaque mechanism for consumers instead.
>
> Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> ---
> drivers/misc/ocxl/afu_irq.c | 76 ++++++++++++++++++-------------
> drivers/misc/ocxl/file.c | 22 ++++++++-
> drivers/misc/ocxl/ocxl_internal.h | 5 --
> include/misc/ocxl.h | 46 +++++++++++++++++++
> 4 files changed, 111 insertions(+), 38 deletions(-)
>
> diff --git a/drivers/misc/ocxl/afu_irq.c b/drivers/misc/ocxl/afu_irq.c
> index 2d410cd6f817..d71e62df7d31 100644
> --- a/drivers/misc/ocxl/afu_irq.c
> +++ b/drivers/misc/ocxl/afu_irq.c
> @@ -1,7 +1,7 @@
> // SPDX-License-Identifier: GPL-2.0+
> // Copyright 2017 IBM Corp.
> #include <linux/interrupt.h>
> -#include <linux/eventfd.h>
> +#include <asm/pnv-ocxl.h>
> #include "ocxl_internal.h"
> #include "trace.h"
>
> @@ -11,7 +11,9 @@ struct afu_irq {
> unsigned int virq;
> char *name;
> u64 trigger_page;
> - struct eventfd_ctx *ev_ctx;
> + irqreturn_t (*handler)(void *private);
> + void (*free_private)(void *private);
> + void *private;
> };
>
> int ocxl_irq_offset_to_id(struct ocxl_context *ctx, u64 offset)
> @@ -24,14 +26,43 @@ u64 ocxl_irq_id_to_offset(struct ocxl_context *ctx, int irq_id)
> return ctx->afu->irq_base_offset + (irq_id << PAGE_SHIFT);
> }
>
> +int ocxl_irq_set_handler(struct ocxl_context *ctx, int irq_id,
> + irqreturn_t (*handler)(void *private),
> + void (*free_private)(void *private),
> + void *private)
> +{
> + struct afu_irq *irq;
> + int rc;
> +
> + mutex_lock(&ctx->irq_lock);
> + irq = idr_find(&ctx->irq_idr, irq_id);
> + if (!irq) {
> + rc = -EINVAL;
> + goto unlock;
> + }
> +
> + irq->handler = handler;
> + irq->private = private;
> +
> + rc = 0;
> + goto unlock;
That goto is too much.
> +
> +unlock:
> + mutex_unlock(&ctx->irq_lock);
> + return rc;
> +}
> +EXPORT_SYMBOL_GPL(ocxl_irq_set_handler);
> +
> static irqreturn_t afu_irq_handler(int virq, void *data)
> {
> struct afu_irq *irq = (struct afu_irq *) data;
>
> trace_ocxl_afu_irq_receive(virq);
> - if (irq->ev_ctx)
> - eventfd_signal(irq->ev_ctx, 1);
> - return IRQ_HANDLED;
> +
> + if (irq->handler)
> + return irq->handler(irq->private);
> +
> + return IRQ_HANDLED; // Just drop it on the ground
> }
>
> static int setup_afu_irq(struct ocxl_context *ctx, struct afu_irq *irq)
> @@ -118,6 +149,8 @@ int ocxl_afu_irq_alloc(struct ocxl_context *ctx, int *irq_id)
> return rc;
> }
>
> +EXPORT_SYMBOL_GPL(ocxl_afu_irq_alloc);
nit: checkpatch doesn't like the empty line between the function and its
symbol export. Also true for a few other symbols in that file.
> diff --git a/include/misc/ocxl.h b/include/misc/ocxl.h
> index a8fe0ce4ea67..1b48e9d63abb 100644
> --- a/include/misc/ocxl.h
> +++ b/include/misc/ocxl.h
> @@ -161,6 +161,52 @@ int ocxl_context_attach(struct ocxl_context *ctx, u64 amr,
> */
> int ocxl_context_detach(struct ocxl_context *ctx);
>
> +// AFU IRQs
> +
> +/**
> + * Allocate an IRQ associated with an AFU context
> + * @ctx: the AFU context
> + * @irq_id: out, the IRQ ID
> + *
> + * Returns 0 on success, negative on failure
> + */
> +extern int ocxl_afu_irq_alloc(struct ocxl_context *ctx, int *irq_id);
> +
> +/**
> + * Frees an IRQ associated with an AFU context
> + * @ctx: the AFU context
> + * @irq_id: the IRQ ID
> + *
> + * Returns 0 on success, negative on failure
> + */
> +extern int ocxl_afu_irq_free(struct ocxl_context *ctx, int irq_id);
> +
> +/**
> + * Gets the address of the trigger page for an IRQ
> + * This can then be provided to an AFU which will write to that
> + * page to trigger the IRQ.
> + * @ctx: The AFU context that the IRQ is associated with
> + * @irq_id: The IRQ ID
> + *
> + * returns the trigger page address, or 0 if the IRQ is not valid
> + */
> +extern u64 ocxl_afu_irq_get_addr(struct ocxl_context *ctx, int irq_id);
> +
> +/**
> + * Provide a callback to be called when an IRQ is triggered
> + * @ctx: The AFU context that the IRQ is associated with
> + * @irq_id: The IRQ ID
> + * @handler: the callback to be called when the IRQ is triggered
> + * @free_private: the callback to be called when the IRQ is freed
We should mention free_private can be NULL, in which case it is ignored.
Fred
^ permalink raw reply
* Re: [PATCH v5 3/3] locking/rwsem: Optimize down_read_trylock()
From: Christophe Leroy @ 2019-03-25 15:25 UTC (permalink / raw)
To: Waiman Long, Peter Zijlstra, Ingo Molnar, Will Deacon,
Thomas Gleixner
Cc: linux-ia64, linux-sh, linux-kernel, H. Peter Anvin, sparclinux,
linux-riscv, linux-arch, linux-s390, Davidlohr Bueso,
linux-c6x-dev, linux-hexagon, x86, uclinux-h8-devel, linux-xtensa,
Arnd Bergmann, linux-um, linuxppc-dev, linux-m68k, openrisc,
Borislav Petkov, linux-arm-kernel, linux-parisc, Linus Torvalds,
linux-mips, linux-alpha, nios2-dev, Andrew Morton, Tim Chen
In-Reply-To: <20190322143008.21313-4-longman@redhat.com>
Hi,
Could you share the microbenchmark you are using ?
I'd like to test the series on powerpc.
Thanks
Christophe
Le 22/03/2019 à 15:30, Waiman Long a écrit :
> Modify __down_read_trylock() to optimize for an unlocked rwsem and make
> it generate slightly better code.
>
> Before this patch, down_read_trylock:
>
> 0x0000000000000000 <+0>: callq 0x5 <down_read_trylock+5>
> 0x0000000000000005 <+5>: jmp 0x18 <down_read_trylock+24>
> 0x0000000000000007 <+7>: lea 0x1(%rdx),%rcx
> 0x000000000000000b <+11>: mov %rdx,%rax
> 0x000000000000000e <+14>: lock cmpxchg %rcx,(%rdi)
> 0x0000000000000013 <+19>: cmp %rax,%rdx
> 0x0000000000000016 <+22>: je 0x23 <down_read_trylock+35>
> 0x0000000000000018 <+24>: mov (%rdi),%rdx
> 0x000000000000001b <+27>: test %rdx,%rdx
> 0x000000000000001e <+30>: jns 0x7 <down_read_trylock+7>
> 0x0000000000000020 <+32>: xor %eax,%eax
> 0x0000000000000022 <+34>: retq
> 0x0000000000000023 <+35>: mov %gs:0x0,%rax
> 0x000000000000002c <+44>: or $0x3,%rax
> 0x0000000000000030 <+48>: mov %rax,0x20(%rdi)
> 0x0000000000000034 <+52>: mov $0x1,%eax
> 0x0000000000000039 <+57>: retq
>
> After patch, down_read_trylock:
>
> 0x0000000000000000 <+0>: callq 0x5 <down_read_trylock+5>
> 0x0000000000000005 <+5>: xor %eax,%eax
> 0x0000000000000007 <+7>: lea 0x1(%rax),%rdx
> 0x000000000000000b <+11>: lock cmpxchg %rdx,(%rdi)
> 0x0000000000000010 <+16>: jne 0x29 <down_read_trylock+41>
> 0x0000000000000012 <+18>: mov %gs:0x0,%rax
> 0x000000000000001b <+27>: or $0x3,%rax
> 0x000000000000001f <+31>: mov %rax,0x20(%rdi)
> 0x0000000000000023 <+35>: mov $0x1,%eax
> 0x0000000000000028 <+40>: retq
> 0x0000000000000029 <+41>: test %rax,%rax
> 0x000000000000002c <+44>: jns 0x7 <down_read_trylock+7>
> 0x000000000000002e <+46>: xor %eax,%eax
> 0x0000000000000030 <+48>: retq
>
> By using a rwsem microbenchmark, the down_read_trylock() rate (with a
> load of 10 to lengthen the lock critical section) on a x86-64 system
> before and after the patch were:
>
> Before Patch After Patch
> # of Threads rlock rlock
> ------------ ----- -----
> 1 14,496 14,716
> 2 8,644 8,453
> 4 6,799 6,983
> 8 5,664 7,190
>
> On a ARM64 system, the performance results were:
>
> Before Patch After Patch
> # of Threads rlock rlock
> ------------ ----- -----
> 1 23,676 24,488
> 2 7,697 9,502
> 4 4,945 3,440
> 8 2,641 1,603
>
> For the uncontended case (1 thread), the new down_read_trylock() is a
> little bit faster. For the contended cases, the new down_read_trylock()
> perform pretty well in x86-64, but performance degrades at high
> contention level on ARM64.
>
> Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
> Signed-off-by: Waiman Long <longman@redhat.com>
> ---
> kernel/locking/rwsem.h | 13 ++++++++-----
> 1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/kernel/locking/rwsem.h b/kernel/locking/rwsem.h
> index 45ee00236e03..1f5775aa6a1d 100644
> --- a/kernel/locking/rwsem.h
> +++ b/kernel/locking/rwsem.h
> @@ -174,14 +174,17 @@ static inline int __down_read_killable(struct rw_semaphore *sem)
>
> static inline int __down_read_trylock(struct rw_semaphore *sem)
> {
> - long tmp;
> + /*
> + * Optimize for the case when the rwsem is not locked at all.
> + */
> + long tmp = RWSEM_UNLOCKED_VALUE;
>
> - while ((tmp = atomic_long_read(&sem->count)) >= 0) {
> - if (tmp == atomic_long_cmpxchg_acquire(&sem->count, tmp,
> - tmp + RWSEM_ACTIVE_READ_BIAS)) {
> + do {
> + if (atomic_long_try_cmpxchg_acquire(&sem->count, &tmp,
> + tmp + RWSEM_ACTIVE_READ_BIAS)) {
> return 1;
> }
> - }
> + } while (tmp >= 0);
> return 0;
> }
>
>
^ permalink raw reply
* Re: [PATCH v3 5/7] ocxl: afu_irq only deals with IRQ IDs, not offsets
From: Frederic Barrat @ 2019-03-25 15:24 UTC (permalink / raw)
To: Alastair D'Silva, alastair
Cc: Greg Kroah-Hartman, linuxppc-dev, Arnd Bergmann, Andrew Donnellan,
linux-kernel
In-Reply-To: <20190325054438.15022-6-alastair@au1.ibm.com>
Le 25/03/2019 à 06:44, Alastair D'Silva a écrit :
> From: Alastair D'Silva <alastair@d-silva.org>
>
> The use of offsets is required only in the frontend, so alter
> the IRQ API to only work with IRQ IDs in the backend.
>
> Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> ---
Acked-by: Frederic Barrat <fbarrat@linux.ibm.com>
> drivers/misc/ocxl/afu_irq.c | 34 +++++++++++++++----------------
> drivers/misc/ocxl/context.c | 7 +++++--
> drivers/misc/ocxl/file.c | 13 +++++++-----
> drivers/misc/ocxl/ocxl_internal.h | 10 +++++----
> drivers/misc/ocxl/trace.h | 12 ++++-------
> 5 files changed, 39 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/misc/ocxl/afu_irq.c b/drivers/misc/ocxl/afu_irq.c
> index 11ab996657a2..2d410cd6f817 100644
> --- a/drivers/misc/ocxl/afu_irq.c
> +++ b/drivers/misc/ocxl/afu_irq.c
> @@ -14,14 +14,14 @@ struct afu_irq {
> struct eventfd_ctx *ev_ctx;
> };
>
> -static int irq_offset_to_id(struct ocxl_context *ctx, u64 offset)
> +int ocxl_irq_offset_to_id(struct ocxl_context *ctx, u64 offset)
> {
> return (offset - ctx->afu->irq_base_offset) >> PAGE_SHIFT;
> }
>
> -static u64 irq_id_to_offset(struct ocxl_context *ctx, int id)
> +u64 ocxl_irq_id_to_offset(struct ocxl_context *ctx, int irq_id)
> {
> - return ctx->afu->irq_base_offset + (id << PAGE_SHIFT);
> + return ctx->afu->irq_base_offset + (irq_id << PAGE_SHIFT);
> }
>
> static irqreturn_t afu_irq_handler(int virq, void *data)
> @@ -69,7 +69,7 @@ static void release_afu_irq(struct afu_irq *irq)
> kfree(irq->name);
> }
>
> -int ocxl_afu_irq_alloc(struct ocxl_context *ctx, u64 *irq_offset)
> +int ocxl_afu_irq_alloc(struct ocxl_context *ctx, int *irq_id)
> {
> struct afu_irq *irq;
> int rc;
> @@ -101,11 +101,11 @@ int ocxl_afu_irq_alloc(struct ocxl_context *ctx, u64 *irq_offset)
> if (rc)
> goto err_alloc;
>
> - *irq_offset = irq_id_to_offset(ctx, irq->id);
> -
> - trace_ocxl_afu_irq_alloc(ctx->pasid, irq->id, irq->virq, irq->hw_irq,
> - *irq_offset);
> + trace_ocxl_afu_irq_alloc(ctx->pasid, irq->id, irq->virq, irq->hw_irq);
> mutex_unlock(&ctx->irq_lock);
> +
> + *irq_id = irq->id;
> +
> return 0;
>
> err_alloc:
> @@ -123,7 +123,7 @@ static void afu_irq_free(struct afu_irq *irq, struct ocxl_context *ctx)
> trace_ocxl_afu_irq_free(ctx->pasid, irq->id);
> if (ctx->mapping)
> unmap_mapping_range(ctx->mapping,
> - irq_id_to_offset(ctx, irq->id),
> + ocxl_irq_id_to_offset(ctx, irq->id),
> 1 << PAGE_SHIFT, 1);
> release_afu_irq(irq);
> if (irq->ev_ctx)
> @@ -132,14 +132,13 @@ static void afu_irq_free(struct afu_irq *irq, struct ocxl_context *ctx)
> kfree(irq);
> }
>
> -int ocxl_afu_irq_free(struct ocxl_context *ctx, u64 irq_offset)
> +int ocxl_afu_irq_free(struct ocxl_context *ctx, int irq_id)
> {
> struct afu_irq *irq;
> - int id = irq_offset_to_id(ctx, irq_offset);
>
> mutex_lock(&ctx->irq_lock);
>
> - irq = idr_find(&ctx->irq_idr, id);
> + irq = idr_find(&ctx->irq_idr, irq_id);
> if (!irq) {
> mutex_unlock(&ctx->irq_lock);
> return -EINVAL;
> @@ -161,14 +160,14 @@ void ocxl_afu_irq_free_all(struct ocxl_context *ctx)
> mutex_unlock(&ctx->irq_lock);
> }
>
> -int ocxl_afu_irq_set_fd(struct ocxl_context *ctx, u64 irq_offset, int eventfd)
> +int ocxl_afu_irq_set_fd(struct ocxl_context *ctx, int irq_id, int eventfd)
> {
> struct afu_irq *irq;
> struct eventfd_ctx *ev_ctx;
> - int rc = 0, id = irq_offset_to_id(ctx, irq_offset);
> + int rc = 0;
>
> mutex_lock(&ctx->irq_lock);
> - irq = idr_find(&ctx->irq_idr, id);
> + irq = idr_find(&ctx->irq_idr, irq_id);
> if (!irq) {
> rc = -EINVAL;
> goto unlock;
> @@ -186,14 +185,13 @@ int ocxl_afu_irq_set_fd(struct ocxl_context *ctx, u64 irq_offset, int eventfd)
> return rc;
> }
>
> -u64 ocxl_afu_irq_get_addr(struct ocxl_context *ctx, u64 irq_offset)
> +u64 ocxl_afu_irq_get_addr(struct ocxl_context *ctx, int irq_id)
> {
> struct afu_irq *irq;
> - int id = irq_offset_to_id(ctx, irq_offset);
> u64 addr = 0;
>
> mutex_lock(&ctx->irq_lock);
> - irq = idr_find(&ctx->irq_idr, id);
> + irq = idr_find(&ctx->irq_idr, irq_id);
> if (irq)
> addr = irq->trigger_page;
> mutex_unlock(&ctx->irq_lock);
> diff --git a/drivers/misc/ocxl/context.c b/drivers/misc/ocxl/context.c
> index 8b97b0f19db8..d6056883b85d 100644
> --- a/drivers/misc/ocxl/context.c
> +++ b/drivers/misc/ocxl/context.c
> @@ -93,8 +93,9 @@ static vm_fault_t map_afu_irq(struct vm_area_struct *vma, unsigned long address,
> u64 offset, struct ocxl_context *ctx)
> {
> u64 trigger_addr;
> + int irq_id = ocxl_irq_offset_to_id(ctx, offset);
>
> - trigger_addr = ocxl_afu_irq_get_addr(ctx, offset);
> + trigger_addr = ocxl_afu_irq_get_addr(ctx, irq_id);
> if (!trigger_addr)
> return VM_FAULT_SIGBUS;
>
> @@ -154,12 +155,14 @@ static const struct vm_operations_struct ocxl_vmops = {
> static int check_mmap_afu_irq(struct ocxl_context *ctx,
> struct vm_area_struct *vma)
> {
> + int irq_id = ocxl_irq_offset_to_id(ctx, vma->vm_pgoff << PAGE_SHIFT);
> +
> /* only one page */
> if (vma_pages(vma) != 1)
> return -EINVAL;
>
> /* check offset validty */
> - if (!ocxl_afu_irq_get_addr(ctx, vma->vm_pgoff << PAGE_SHIFT))
> + if (!ocxl_afu_irq_get_addr(ctx, irq_id))
> return -EINVAL;
>
> /*
> diff --git a/drivers/misc/ocxl/file.c b/drivers/misc/ocxl/file.c
> index e51578186fd4..2585fc991108 100644
> --- a/drivers/misc/ocxl/file.c
> +++ b/drivers/misc/ocxl/file.c
> @@ -196,6 +196,7 @@ static long afu_ioctl(struct file *file, unsigned int cmd,
> {
> struct ocxl_context *ctx = file->private_data;
> struct ocxl_ioctl_irq_fd irq_fd;
> + int irq_id;
> u64 irq_offset;
> long rc;
> bool closed;
> @@ -217,12 +218,13 @@ static long afu_ioctl(struct file *file, unsigned int cmd,
> break;
>
> case OCXL_IOCTL_IRQ_ALLOC:
> - rc = ocxl_afu_irq_alloc(ctx, &irq_offset);
> + rc = ocxl_afu_irq_alloc(ctx, &irq_id);
> if (!rc) {
> + irq_offset = ocxl_irq_id_to_offset(ctx, irq_id);
> rc = copy_to_user((u64 __user *) args, &irq_offset,
> sizeof(irq_offset));
> if (rc) {
> - ocxl_afu_irq_free(ctx, irq_offset);
> + ocxl_afu_irq_free(ctx, irq_id);
> return -EFAULT;
> }
> }
> @@ -233,7 +235,8 @@ static long afu_ioctl(struct file *file, unsigned int cmd,
> sizeof(irq_offset));
> if (rc)
> return -EFAULT;
> - rc = ocxl_afu_irq_free(ctx, irq_offset);
> + irq_id = ocxl_irq_offset_to_id(ctx, irq_offset);
> + rc = ocxl_afu_irq_free(ctx, irq_id);
> break;
>
> case OCXL_IOCTL_IRQ_SET_FD:
> @@ -243,8 +246,8 @@ static long afu_ioctl(struct file *file, unsigned int cmd,
> return -EFAULT;
> if (irq_fd.reserved)
> return -EINVAL;
> - rc = ocxl_afu_irq_set_fd(ctx, irq_fd.irq_offset,
> - irq_fd.eventfd);
> + irq_id = ocxl_irq_offset_to_id(ctx, irq_fd.irq_offset);
> + rc = ocxl_afu_irq_set_fd(ctx, irq_id, irq_fd.eventfd);
> break;
>
> case OCXL_IOCTL_GET_METADATA:
> diff --git a/drivers/misc/ocxl/ocxl_internal.h b/drivers/misc/ocxl/ocxl_internal.h
> index cda1e7667fc8..923e7f29c693 100644
> --- a/drivers/misc/ocxl/ocxl_internal.h
> +++ b/drivers/misc/ocxl/ocxl_internal.h
> @@ -137,12 +137,14 @@ void ocxl_context_detach_all(struct ocxl_afu *afu);
> int ocxl_sysfs_register_afu(struct ocxl_afu *afu);
> void ocxl_sysfs_unregister_afu(struct ocxl_afu *afu);
>
> -int ocxl_afu_irq_alloc(struct ocxl_context *ctx, u64 *irq_offset);
> -int ocxl_afu_irq_free(struct ocxl_context *ctx, u64 irq_offset);
> +int ocxl_irq_offset_to_id(struct ocxl_context *ctx, u64 offset);
> +u64 ocxl_irq_id_to_offset(struct ocxl_context *ctx, int irq_id);
> +int ocxl_afu_irq_alloc(struct ocxl_context *ctx, int *irq_id);
> +int ocxl_afu_irq_free(struct ocxl_context *ctx, int irq_id);
> void ocxl_afu_irq_free_all(struct ocxl_context *ctx);
> -int ocxl_afu_irq_set_fd(struct ocxl_context *ctx, u64 irq_offset,
> +int ocxl_afu_irq_set_fd(struct ocxl_context *ctx, int irq_id,
> int eventfd);
> -u64 ocxl_afu_irq_get_addr(struct ocxl_context *ctx, u64 irq_offset);
> +u64 ocxl_afu_irq_get_addr(struct ocxl_context *ctx, int irq_id);
>
> /**
> * Free an AFU
> diff --git a/drivers/misc/ocxl/trace.h b/drivers/misc/ocxl/trace.h
> index bcb7ff330c1e..024f417e7e01 100644
> --- a/drivers/misc/ocxl/trace.h
> +++ b/drivers/misc/ocxl/trace.h
> @@ -107,16 +107,14 @@ DEFINE_EVENT(ocxl_fault_handler, ocxl_fault_ack,
> );
>
> TRACE_EVENT(ocxl_afu_irq_alloc,
> - TP_PROTO(int pasid, int irq_id, unsigned int virq, int hw_irq,
> - u64 irq_offset),
> - TP_ARGS(pasid, irq_id, virq, hw_irq, irq_offset),
> + TP_PROTO(int pasid, int irq_id, unsigned int virq, int hw_irq),
> + TP_ARGS(pasid, irq_id, virq, hw_irq),
>
> TP_STRUCT__entry(
> __field(int, pasid)
> __field(int, irq_id)
> __field(unsigned int, virq)
> __field(int, hw_irq)
> - __field(u64, irq_offset)
> ),
>
> TP_fast_assign(
> @@ -124,15 +122,13 @@ TRACE_EVENT(ocxl_afu_irq_alloc,
> __entry->irq_id = irq_id;
> __entry->virq = virq;
> __entry->hw_irq = hw_irq;
> - __entry->irq_offset = irq_offset;
> ),
>
> - TP_printk("pasid=0x%x irq_id=%d virq=%u hw_irq=%d irq_offset=0x%llx",
> + TP_printk("pasid=0x%x irq_id=%d virq=%u hw_irq=%d",
> __entry->pasid,
> __entry->irq_id,
> __entry->virq,
> - __entry->hw_irq,
> - __entry->irq_offset
> + __entry->hw_irq
> )
> );
>
>
^ permalink raw reply
* Re: [PATCH v3 4/7] ocxl: Allow external drivers to use OpenCAPI contexts
From: Frederic Barrat @ 2019-03-25 15:13 UTC (permalink / raw)
To: Alastair D'Silva, alastair
Cc: Arnd Bergmann, Greg Kroah-Hartman, Greg Kurz, linux-kernel,
Andrew Donnellan, linuxppc-dev
In-Reply-To: <20190325054438.15022-5-alastair@au1.ibm.com>
Le 25/03/2019 à 06:44, Alastair D'Silva a écrit :
> From: Alastair D'Silva <alastair@d-silva.org>
>
> Most OpenCAPI operations require a valid context, so
> exposing these functions to external drivers is necessary.
>
> Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> Reviewed-by: Greg Kurz <groug@kaod.org>
> ---
See comment on previous patch regarding merging ocxl_context_alloc() and
ocxl_context_init(), it would impact the exported symbols.
Fred
> drivers/misc/ocxl/context.c | 9 +++++--
> drivers/misc/ocxl/file.c | 2 +-
> drivers/misc/ocxl/ocxl_internal.h | 6 -----
> include/misc/ocxl.h | 45 +++++++++++++++++++++++++++++++
> 4 files changed, 53 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/misc/ocxl/context.c b/drivers/misc/ocxl/context.c
> index c73a859d2224..8b97b0f19db8 100644
> --- a/drivers/misc/ocxl/context.c
> +++ b/drivers/misc/ocxl/context.c
> @@ -8,6 +8,7 @@ struct ocxl_context *ocxl_context_alloc(void)
> {
> return kzalloc(sizeof(struct ocxl_context), GFP_KERNEL);
> }
> +EXPORT_SYMBOL_GPL(ocxl_context_alloc);
>
> int ocxl_context_init(struct ocxl_context *ctx, struct ocxl_afu *afu,
> struct address_space *mapping)
> @@ -43,6 +44,7 @@ int ocxl_context_init(struct ocxl_context *ctx, struct ocxl_afu *afu,
> ocxl_afu_get(afu);
> return 0;
> }
> +EXPORT_SYMBOL_GPL(ocxl_context_init);
>
> /*
> * Callback for when a translation fault triggers an error
> @@ -63,7 +65,7 @@ static void xsl_fault_error(void *data, u64 addr, u64 dsisr)
> wake_up_all(&ctx->events_wq);
> }
>
> -int ocxl_context_attach(struct ocxl_context *ctx, u64 amr)
> +int ocxl_context_attach(struct ocxl_context *ctx, u64 amr, struct mm_struct *mm)
> {
> int rc;
>
> @@ -75,7 +77,7 @@ int ocxl_context_attach(struct ocxl_context *ctx, u64 amr)
> }
>
> rc = ocxl_link_add_pe(ctx->afu->fn->link, ctx->pasid,
> - current->mm->context.id, ctx->tidr, amr, current->mm,
> + mm->context.id, ctx->tidr, amr, mm,
> xsl_fault_error, ctx);
> if (rc)
> goto out;
> @@ -85,6 +87,7 @@ int ocxl_context_attach(struct ocxl_context *ctx, u64 amr)
> mutex_unlock(&ctx->status_mutex);
> return rc;
> }
> +EXPORT_SYMBOL_GPL(ocxl_context_attach);
>
> static vm_fault_t map_afu_irq(struct vm_area_struct *vma, unsigned long address,
> u64 offset, struct ocxl_context *ctx)
> @@ -243,6 +246,7 @@ int ocxl_context_detach(struct ocxl_context *ctx)
> }
> return 0;
> }
> +EXPORT_SYMBOL_GPL(ocxl_context_detach);
>
> void ocxl_context_detach_all(struct ocxl_afu *afu)
> {
> @@ -280,3 +284,4 @@ void ocxl_context_free(struct ocxl_context *ctx)
> ocxl_afu_put(ctx->afu);
> kfree(ctx);
> }
> +EXPORT_SYMBOL_GPL(ocxl_context_free);
> diff --git a/drivers/misc/ocxl/file.c b/drivers/misc/ocxl/file.c
> index e6e6121cd9a3..e51578186fd4 100644
> --- a/drivers/misc/ocxl/file.c
> +++ b/drivers/misc/ocxl/file.c
> @@ -94,7 +94,7 @@ static long afu_ioctl_attach(struct ocxl_context *ctx,
> return -EINVAL;
>
> amr = arg.amr & mfspr(SPRN_UAMOR);
> - rc = ocxl_context_attach(ctx, amr);
> + rc = ocxl_context_attach(ctx, amr, current->mm);
> return rc;
> }
>
> diff --git a/drivers/misc/ocxl/ocxl_internal.h b/drivers/misc/ocxl/ocxl_internal.h
> index e04e547df29e..cda1e7667fc8 100644
> --- a/drivers/misc/ocxl/ocxl_internal.h
> +++ b/drivers/misc/ocxl/ocxl_internal.h
> @@ -130,15 +130,9 @@ int ocxl_config_check_afu_index(struct pci_dev *dev,
> */
> int ocxl_link_update_pe(void *link_handle, int pasid, __u16 tid);
>
> -struct ocxl_context *ocxl_context_alloc(void);
> -int ocxl_context_init(struct ocxl_context *ctx, struct ocxl_afu *afu,
> - struct address_space *mapping);
> -int ocxl_context_attach(struct ocxl_context *ctx, u64 amr);
> int ocxl_context_mmap(struct ocxl_context *ctx,
> struct vm_area_struct *vma);
> -int ocxl_context_detach(struct ocxl_context *ctx);
> void ocxl_context_detach_all(struct ocxl_afu *afu);
> -void ocxl_context_free(struct ocxl_context *ctx);
>
> int ocxl_sysfs_register_afu(struct ocxl_afu *afu);
> void ocxl_sysfs_unregister_afu(struct ocxl_afu *afu);
> diff --git a/include/misc/ocxl.h b/include/misc/ocxl.h
> index 8bafd748e380..a8fe0ce4ea67 100644
> --- a/include/misc/ocxl.h
> +++ b/include/misc/ocxl.h
> @@ -116,6 +116,51 @@ const struct ocxl_fn_config *ocxl_function_config(struct ocxl_fn *fn);
> */
> void ocxl_function_close(struct ocxl_fn *fn);
>
> +// Context allocation
> +
> +/**
> + * Allocate space for a new OpenCAPI context
> + *
> + * Returns NULL on failure
> + */
> +struct ocxl_context *ocxl_context_alloc(void);
> +
> +/**
> + * Initialize an OpenCAPI context
> + *
> + * @ctx: The OpenCAPI context to initialize
> + * @afu: The AFU the context belongs to
> + * @mapping: The mapping to unmap when the context is closed (may be NULL)
> + */
> +int ocxl_context_init(struct ocxl_context *ctx, struct ocxl_afu *afu,
> + struct address_space *mapping);
> +
> +/**
> + * Free an OpenCAPI context
> + *
> + * @ctx: The OpenCAPI context to free
> + */
> +void ocxl_context_free(struct ocxl_context *ctx);
> +
> +/**
> + * Grant access to an MM to an OpenCAPI context
> + * @ctx: The OpenCAPI context to attach
> + * @amr: The value of the AMR register to restrict access
> + * @mm: The mm to attach to the context
> + *
> + * Returns 0 on success, negative on failure
> + */
> +int ocxl_context_attach(struct ocxl_context *ctx, u64 amr,
> + struct mm_struct *mm);
> +
> +/**
> + * Detach an MM from an OpenCAPI context
> + * @ctx: The OpenCAPI context to attach
> + *
> + * Returns 0 on success, negative on failure
> + */
> +int ocxl_context_detach(struct ocxl_context *ctx);
> +
> // AFU Metadata
>
> /**
>
^ permalink raw reply
* Re: [PATCH v3 3/7] ocxl: Create a clear delineation between ocxl backend & frontend
From: Frederic Barrat @ 2019-03-25 15:11 UTC (permalink / raw)
To: Alastair D'Silva, alastair
Cc: Greg Kroah-Hartman, linuxppc-dev, Arnd Bergmann, Andrew Donnellan,
linux-kernel
In-Reply-To: <20190325054438.15022-4-alastair@au1.ibm.com>
This is a huge patch, there are probably ways to split it in smaller
pieces to make the review easier. However, considering how much time
we've already invested discussing and reviewing it, it's with by me to
keep it as is.
The ref-counting and device management look good to me now. A few
details below.
> --- a/drivers/misc/ocxl/file.c
> +++ b/drivers/misc/ocxl/file.c
> @@ -17,12 +17,10 @@ static struct class *ocxl_class;
> static struct mutex minors_idr_lock;
> static struct idr minors_idr;
>
> -static struct ocxl_afu *find_and_get_afu(dev_t devno)
> +static struct ocxl_file_info *find_file_info(dev_t devno)
> {
> - struct ocxl_afu *afu;
> - int afu_minor;
> + struct ocxl_file_info *info;
>
> - afu_minor = MINOR(devno);
> /*
> * We don't declare an RCU critical section here, as our AFU
> * is protected by a reference counter on the device. By the time the
> @@ -30,56 +28,52 @@ static struct ocxl_afu *find_and_get_afu(dev_t devno)
> * the device is already at 0, so no user API will access that AFU and
> * this function can't return it.
> */
The comment is still true, but needs tuning. Something like:
"We don't declare an RCU critical section here, as our info structure is
protected by a reference counter on the device. By the time the info
reference is removed from the idr, the ref count of the device is
already at 0, so no user API will access the corresponding AFU and this
function can't return it."
> - afu = idr_find(&minors_idr, afu_minor);
> - if (afu)
> - ocxl_afu_get(afu);
> - return afu;
> + info = idr_find(&minors_idr, MINOR(devno));
> + return info;
> }
>
> -static int allocate_afu_minor(struct ocxl_afu *afu)
> +static int allocate_minor(struct ocxl_file_info *info)
> {
> int minor;
>
> mutex_lock(&minors_idr_lock);
> - minor = idr_alloc(&minors_idr, afu, 0, OCXL_NUM_MINORS, GFP_KERNEL);
> + minor = idr_alloc(&minors_idr, info, 0, OCXL_NUM_MINORS, GFP_KERNEL);
> mutex_unlock(&minors_idr_lock);
> return minor;
> }
>
> -static void free_afu_minor(struct ocxl_afu *afu)
> +static void free_minor(struct ocxl_file_info *info)
> {
> mutex_lock(&minors_idr_lock);
> - idr_remove(&minors_idr, MINOR(afu->dev.devt));
> + idr_remove(&minors_idr, MINOR(info->dev.devt));
> mutex_unlock(&minors_idr_lock);
> }
>
> static int afu_open(struct inode *inode, struct file *file)
> {
> - struct ocxl_afu *afu;
> + struct ocxl_file_info *info;
> struct ocxl_context *ctx;
> int rc;
>
> pr_debug("%s for device %x\n", __func__, inode->i_rdev);
>
> - afu = find_and_get_afu(inode->i_rdev);
> - if (!afu)
> + info = find_file_info(inode->i_rdev);
> + if (!info)
> return -ENODEV;
>
> ctx = ocxl_context_alloc();
> if (!ctx) {
> rc = -ENOMEM;
> - goto put_afu;
> + goto err;
> }
>
> - rc = ocxl_context_init(ctx, afu, inode->i_mapping);
> + rc = ocxl_context_init(ctx, info->afu, inode->i_mapping);
> if (rc)
> - goto put_afu;
> + goto err;
> file->private_data = ctx;
> - ocxl_afu_put(afu);
> return 0;
>
> -put_afu:
> - ocxl_afu_put(afu);
> +err:
> return rc;
The error path with goto is here useless. However, if
ocxl_context_init() fails, the memory for the context is never released.
You may consider either getting rid of ocxl_context_alloc(), which is
just a simple wrapper around kzalloc(), or merging the allocation in
ocxl_context_init(). It would impact the external API, but having 2
calls (alloc and init) feels like there's one too many.
> +static int ocxl_file_make_visible(struct ocxl_afu *afu)
> {
> int rc;
> + struct ocxl_file_info *info = ocxl_afu_get_private(afu);
>
> - cdev_init(&afu->cdev, &ocxl_afu_fops);
> - rc = cdev_add(&afu->cdev, afu->dev.devt, 1);
> + cdev_init(&info->cdev, &ocxl_afu_fops);
> + rc = cdev_add(&info->cdev, info->dev.devt, 1);
> if (rc) {
> - dev_err(&afu->dev, "Unable to add afu char device: %d\n", rc);
> + dev_err(&info->dev, "Unable to add afu char device: %d\n", rc);
> return rc;
> }
> +
> return 0;
> }
>
> -void ocxl_destroy_cdev(struct ocxl_afu *afu)
> +void ocxl_file_make_invisible(struct ocxl_afu *afu)
This function is not called anywhere?
> -void ocxl_unregister_afu(struct ocxl_afu *afu)
> +void ocxl_file_unregister_afu(struct ocxl_afu *afu)
> {
> - free_afu_minor(afu);
> + struct ocxl_file_info *info = ocxl_afu_get_private(afu);
> +
> + if (!info)
> + return;
> +
So that's likely where we miss the "make invisible" call. However, is it
enough to rely on the private data to be set on the AFU?
> diff --git a/drivers/misc/ocxl/ocxl_internal.h b/drivers/misc/ocxl/ocxl_internal.h
> index 81086534dab5..e04e547df29e 100644
> --- a/drivers/misc/ocxl/ocxl_internal.h
> +++ b/drivers/misc/ocxl/ocxl_internal.h
> +/**
> + * Free an AFU
> + *
> + * afu: The AFU to free
> + */
> +void ocxl_free_afu(struct ocxl_afu *afu);
This is obsolete and should go away.
Fred
^ permalink raw reply
* [PATCH 2/2] arch: add pidfd and io_uring syscalls everywhere
From: Arnd Bergmann @ 2019-03-25 14:47 UTC (permalink / raw)
To: Andrew Morton
Cc: Rich Felker, linux-ia64, linux-sh, Heiko Carstens, linux-mips,
James E . J . Bottomley, Max Filippov, Paul Mackerras, sparclinux,
linux-s390, Helge Deller, Russell King, Geert Uytterhoeven,
Catalin Marinas, James Hogan, Firoz Khan, Matt Turner, Fenghua Yu,
Arnd Bergmann, Will Deacon, linux-m68k, Ivan Kokshaysky,
linux-arm-kernel, Richard Henderson, Michal Simek, Tony Luck,
linux-parisc, linux-kernel, Ralf Baechle, Paul Burton,
linux-alpha, Martin Schwidefsky, linuxppc-dev, David S . Miller
In-Reply-To: <20190325143521.34928-1-arnd@arndb.de>
Add the io_uring and pidfd_send_signal system calls to all architectures.
These system calls are designed to handle both native and compat tasks,
so all entries are the same across architectures, only arm-compat and
the generic tale still use an old format.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
arch/alpha/kernel/syscalls/syscall.tbl | 4 ++++
arch/arm/tools/syscall.tbl | 4 ++++
arch/arm64/include/asm/unistd.h | 2 +-
arch/arm64/include/asm/unistd32.h | 8 ++++++++
arch/ia64/kernel/syscalls/syscall.tbl | 4 ++++
arch/m68k/kernel/syscalls/syscall.tbl | 4 ++++
arch/microblaze/kernel/syscalls/syscall.tbl | 4 ++++
arch/mips/kernel/syscalls/syscall_n32.tbl | 4 ++++
arch/mips/kernel/syscalls/syscall_n64.tbl | 4 ++++
arch/mips/kernel/syscalls/syscall_o32.tbl | 4 ++++
arch/parisc/kernel/syscalls/syscall.tbl | 4 ++++
arch/powerpc/kernel/syscalls/syscall.tbl | 4 ++++
arch/s390/kernel/syscalls/syscall.tbl | 4 ++++
arch/sh/kernel/syscalls/syscall.tbl | 4 ++++
arch/sparc/kernel/syscalls/syscall.tbl | 4 ++++
arch/xtensa/kernel/syscalls/syscall.tbl | 4 ++++
16 files changed, 65 insertions(+), 1 deletion(-)
diff --git a/arch/alpha/kernel/syscalls/syscall.tbl b/arch/alpha/kernel/syscalls/syscall.tbl
index 63ed39cbd3bd..165f268beafc 100644
--- a/arch/alpha/kernel/syscalls/syscall.tbl
+++ b/arch/alpha/kernel/syscalls/syscall.tbl
@@ -463,3 +463,7 @@
532 common getppid sys_getppid
# all other architectures have common numbers for new syscall, alpha
# is the exception.
+534 common pidfd_send_signal sys_pidfd_send_signal
+535 common io_uring_setup sys_io_uring_setup
+536 common io_uring_enter sys_io_uring_enter
+537 common io_uring_register sys_io_uring_register
diff --git a/arch/arm/tools/syscall.tbl b/arch/arm/tools/syscall.tbl
index 9016f4081bb9..0393917eaa57 100644
--- a/arch/arm/tools/syscall.tbl
+++ b/arch/arm/tools/syscall.tbl
@@ -437,3 +437,7 @@
421 common rt_sigtimedwait_time64 sys_rt_sigtimedwait
422 common futex_time64 sys_futex
423 common sched_rr_get_interval_time64 sys_sched_rr_get_interval
+424 common pidfd_send_signal sys_pidfd_send_signal
+425 common io_uring_setup sys_io_uring_setup
+426 common io_uring_enter sys_io_uring_enter
+427 common io_uring_register sys_io_uring_register
diff --git a/arch/arm64/include/asm/unistd.h b/arch/arm64/include/asm/unistd.h
index 310d8f1cae7a..c6946fe640e6 100644
--- a/arch/arm64/include/asm/unistd.h
+++ b/arch/arm64/include/asm/unistd.h
@@ -49,7 +49,7 @@
#define __ARM_NR_compat_set_tls (__ARM_NR_COMPAT_BASE + 5)
#define __ARM_NR_COMPAT_END (__ARM_NR_COMPAT_BASE + 0x800)
-#define __NR_compat_syscalls 424
+#define __NR_compat_syscalls 428
#endif
#define __ARCH_WANT_SYS_CLONE
diff --git a/arch/arm64/include/asm/unistd32.h b/arch/arm64/include/asm/unistd32.h
index 5590f2623690..23f1a44acada 100644
--- a/arch/arm64/include/asm/unistd32.h
+++ b/arch/arm64/include/asm/unistd32.h
@@ -866,6 +866,14 @@ __SYSCALL(__NR_rt_sigtimedwait_time64, compat_sys_rt_sigtimedwait_time64)
__SYSCALL(__NR_futex_time64, sys_futex)
#define __NR_sched_rr_get_interval_time64 423
__SYSCALL(__NR_sched_rr_get_interval_time64, sys_sched_rr_get_interval)
+#define __NR_pidfd_send_signal 424
+__SYSCALL(__NR_pidfd_send_signal, sys_pidfd_send_signal)
+#define __NR_io_uring_setup 425
+__SYSCALL(__NR_io_uring_setup, sys_io_uring_setup)
+#define __NR_io_uring_enter 426
+__SYSCALL(__NR_io_uring_enter, sys_io_uring_enter)
+#define __NR_io_uring_register 427
+__SYSCALL(__NR_io_uring_register, sys_io_uring_register)
/*
* Please add new compat syscalls above this comment and update
diff --git a/arch/ia64/kernel/syscalls/syscall.tbl b/arch/ia64/kernel/syscalls/syscall.tbl
index ab9cda5f6136..56e3d0b685e1 100644
--- a/arch/ia64/kernel/syscalls/syscall.tbl
+++ b/arch/ia64/kernel/syscalls/syscall.tbl
@@ -344,3 +344,7 @@
332 common pkey_free sys_pkey_free
333 common rseq sys_rseq
# 334 through 423 are reserved to sync up with other architectures
+424 common pidfd_send_signal sys_pidfd_send_signal
+425 common io_uring_setup sys_io_uring_setup
+426 common io_uring_enter sys_io_uring_enter
+427 common io_uring_register sys_io_uring_register
diff --git a/arch/m68k/kernel/syscalls/syscall.tbl b/arch/m68k/kernel/syscalls/syscall.tbl
index 125c14178979..df4ec3ec71d1 100644
--- a/arch/m68k/kernel/syscalls/syscall.tbl
+++ b/arch/m68k/kernel/syscalls/syscall.tbl
@@ -423,3 +423,7 @@
421 common rt_sigtimedwait_time64 sys_rt_sigtimedwait
422 common futex_time64 sys_futex
423 common sched_rr_get_interval_time64 sys_sched_rr_get_interval
+424 common pidfd_send_signal sys_pidfd_send_signal
+425 common io_uring_setup sys_io_uring_setup
+426 common io_uring_enter sys_io_uring_enter
+427 common io_uring_register sys_io_uring_register
diff --git a/arch/microblaze/kernel/syscalls/syscall.tbl b/arch/microblaze/kernel/syscalls/syscall.tbl
index 8ee3a8c18498..4964947732af 100644
--- a/arch/microblaze/kernel/syscalls/syscall.tbl
+++ b/arch/microblaze/kernel/syscalls/syscall.tbl
@@ -429,3 +429,7 @@
421 common rt_sigtimedwait_time64 sys_rt_sigtimedwait
422 common futex_time64 sys_futex
423 common sched_rr_get_interval_time64 sys_sched_rr_get_interval
+424 common pidfd_send_signal sys_pidfd_send_signal
+425 common io_uring_setup sys_io_uring_setup
+426 common io_uring_enter sys_io_uring_enter
+427 common io_uring_register sys_io_uring_register
diff --git a/arch/mips/kernel/syscalls/syscall_n32.tbl b/arch/mips/kernel/syscalls/syscall_n32.tbl
index 15f4117900ee..9392dfe33f97 100644
--- a/arch/mips/kernel/syscalls/syscall_n32.tbl
+++ b/arch/mips/kernel/syscalls/syscall_n32.tbl
@@ -362,3 +362,7 @@
421 n32 rt_sigtimedwait_time64 compat_sys_rt_sigtimedwait_time64
422 n32 futex_time64 sys_futex
423 n32 sched_rr_get_interval_time64 sys_sched_rr_get_interval
+424 n32 pidfd_send_signal sys_pidfd_send_signal
+425 n32 io_uring_setup sys_io_uring_setup
+426 n32 io_uring_enter sys_io_uring_enter
+427 n32 io_uring_register sys_io_uring_register
diff --git a/arch/mips/kernel/syscalls/syscall_n64.tbl b/arch/mips/kernel/syscalls/syscall_n64.tbl
index c85502e67b44..c4a49f7d57bb 100644
--- a/arch/mips/kernel/syscalls/syscall_n64.tbl
+++ b/arch/mips/kernel/syscalls/syscall_n64.tbl
@@ -338,3 +338,7 @@
327 n64 rseq sys_rseq
328 n64 io_pgetevents sys_io_pgetevents
# 329 through 423 are reserved to sync up with other architectures
+424 common pidfd_send_signal sys_pidfd_send_signal
+425 common io_uring_setup sys_io_uring_setup
+426 common io_uring_enter sys_io_uring_enter
+427 common io_uring_register sys_io_uring_register
diff --git a/arch/mips/kernel/syscalls/syscall_o32.tbl b/arch/mips/kernel/syscalls/syscall_o32.tbl
index 2e063d0f837e..e849e8ffe4a2 100644
--- a/arch/mips/kernel/syscalls/syscall_o32.tbl
+++ b/arch/mips/kernel/syscalls/syscall_o32.tbl
@@ -411,3 +411,7 @@
421 o32 rt_sigtimedwait_time64 sys_rt_sigtimedwait compat_sys_rt_sigtimedwait_time64
422 o32 futex_time64 sys_futex sys_futex
423 o32 sched_rr_get_interval_time64 sys_sched_rr_get_interval sys_sched_rr_get_interval
+424 o32 pidfd_send_signal sys_pidfd_send_signal
+425 o32 io_uring_setup sys_io_uring_setup
+426 o32 io_uring_enter sys_io_uring_enter
+427 o32 io_uring_register sys_io_uring_register
diff --git a/arch/parisc/kernel/syscalls/syscall.tbl b/arch/parisc/kernel/syscalls/syscall.tbl
index b26766c6647d..fe8ca623add8 100644
--- a/arch/parisc/kernel/syscalls/syscall.tbl
+++ b/arch/parisc/kernel/syscalls/syscall.tbl
@@ -420,3 +420,7 @@
421 32 rt_sigtimedwait_time64 sys_rt_sigtimedwait compat_sys_rt_sigtimedwait_time64
422 32 futex_time64 sys_futex sys_futex
423 32 sched_rr_get_interval_time64 sys_sched_rr_get_interval sys_sched_rr_get_interval
+424 common pidfd_send_signal sys_pidfd_send_signal
+425 common io_uring_setup sys_io_uring_setup
+426 common io_uring_enter sys_io_uring_enter
+427 common io_uring_register sys_io_uring_register
diff --git a/arch/powerpc/kernel/syscalls/syscall.tbl b/arch/powerpc/kernel/syscalls/syscall.tbl
index b18abb0c3dae..00f5a63c8d9a 100644
--- a/arch/powerpc/kernel/syscalls/syscall.tbl
+++ b/arch/powerpc/kernel/syscalls/syscall.tbl
@@ -505,3 +505,7 @@
421 32 rt_sigtimedwait_time64 sys_rt_sigtimedwait compat_sys_rt_sigtimedwait_time64
422 32 futex_time64 sys_futex sys_futex
423 32 sched_rr_get_interval_time64 sys_sched_rr_get_interval sys_sched_rr_get_interval
+424 common pidfd_send_signal sys_pidfd_send_signal
+425 common io_uring_setup sys_io_uring_setup
+426 common io_uring_enter sys_io_uring_enter
+427 common io_uring_register sys_io_uring_register
diff --git a/arch/s390/kernel/syscalls/syscall.tbl b/arch/s390/kernel/syscalls/syscall.tbl
index 02579f95f391..3eb56e639b96 100644
--- a/arch/s390/kernel/syscalls/syscall.tbl
+++ b/arch/s390/kernel/syscalls/syscall.tbl
@@ -426,3 +426,7 @@
421 32 rt_sigtimedwait_time64 - compat_sys_rt_sigtimedwait_time64
422 32 futex_time64 - sys_futex
423 32 sched_rr_get_interval_time64 - sys_sched_rr_get_interval
+424 common pidfd_send_signal sys_pidfd_send_signal
+425 common io_uring_setup sys_io_uring_setup
+426 common io_uring_enter sys_io_uring_enter
+427 common io_uring_register sys_io_uring_register
diff --git a/arch/sh/kernel/syscalls/syscall.tbl b/arch/sh/kernel/syscalls/syscall.tbl
index bfda678576e4..480b057556ee 100644
--- a/arch/sh/kernel/syscalls/syscall.tbl
+++ b/arch/sh/kernel/syscalls/syscall.tbl
@@ -426,3 +426,7 @@
421 common rt_sigtimedwait_time64 sys_rt_sigtimedwait
422 common futex_time64 sys_futex
423 common sched_rr_get_interval_time64 sys_sched_rr_get_interval
+424 common pidfd_send_signal sys_pidfd_send_signal
+425 common io_uring_setup sys_io_uring_setup
+426 common io_uring_enter sys_io_uring_enter
+427 common io_uring_register sys_io_uring_register
diff --git a/arch/sparc/kernel/syscalls/syscall.tbl b/arch/sparc/kernel/syscalls/syscall.tbl
index b9a5a04b2d2c..a1dd24307b00 100644
--- a/arch/sparc/kernel/syscalls/syscall.tbl
+++ b/arch/sparc/kernel/syscalls/syscall.tbl
@@ -469,3 +469,7 @@
421 32 rt_sigtimedwait_time64 sys_rt_sigtimedwait compat_sys_rt_sigtimedwait_time64
422 32 futex_time64 sys_futex sys_futex
423 32 sched_rr_get_interval_time64 sys_sched_rr_get_interval sys_sched_rr_get_interval
+424 common pidfd_send_signal sys_pidfd_send_signal
+425 common io_uring_setup sys_io_uring_setup
+426 common io_uring_enter sys_io_uring_enter
+427 common io_uring_register sys_io_uring_register
diff --git a/arch/xtensa/kernel/syscalls/syscall.tbl b/arch/xtensa/kernel/syscalls/syscall.tbl
index 6af49929de85..30084eaf8422 100644
--- a/arch/xtensa/kernel/syscalls/syscall.tbl
+++ b/arch/xtensa/kernel/syscalls/syscall.tbl
@@ -394,3 +394,7 @@
421 common rt_sigtimedwait_time64 sys_rt_sigtimedwait
422 common futex_time64 sys_futex
423 common sched_rr_get_interval_time64 sys_sched_rr_get_interval
+424 common pidfd_send_signal sys_pidfd_send_signal
+425 common io_uring_setup sys_io_uring_setup
+426 common io_uring_enter sys_io_uring_enter
+427 common io_uring_register sys_io_uring_register
--
2.20.0
^ permalink raw reply related
* Re: [RESEND 1/7] mm/gup: Replace get_user_pages_longterm() with FOLL_LONGTERM
From: Ira Weiny @ 2019-03-25 6:19 UTC (permalink / raw)
To: Dan Williams
Cc: Michal Hocko, Linux-sh, Peter Zijlstra, James Hogan,
Heiko Carstens, Michal Hocko, linux-mm, Rich Felker,
Paul Mackerras, sparclinux, linux-s390, Yoshinori Sato,
linux-rdma, Aneesh Kumar K . V, Jason Gunthorpe, Ingo Molnar,
linux-mips, John Hubbard, Borislav Petkov, Thomas Gleixner,
netdev@vger.kernel.org, Linux Kernel Mailing List, Ralf Baechle,
Martin Schwidefsky, Andrew Morton, linuxppc-dev, David S. Miller,
Kirill A. Shutemov
In-Reply-To: <CAA9_cmffz1VBOJ0ykBtcj+hiznn-kbbuotu1uUhPiJtXiFjJXg@mail.gmail.com>
On Fri, Mar 22, 2019 at 02:24:40PM -0700, Dan Williams wrote:
> On Sun, Mar 17, 2019 at 7:36 PM <ira.weiny@intel.com> wrote:
> >
> > From: Ira Weiny <ira.weiny@intel.com>
> >
> > Rather than have a separate get_user_pages_longterm() call,
> > introduce FOLL_LONGTERM and change the longterm callers to use
> > it.
> >
> > This patch does not change any functionality.
> >
> > FOLL_LONGTERM can only be supported with get_user_pages() as it
> > requires vmas to determine if DAX is in use.
> >
> > CC: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> > CC: Andrew Morton <akpm@linux-foundation.org>
> > CC: Michal Hocko <mhocko@kernel.org>
> > Signed-off-by: Ira Weiny <ira.weiny@intel.com>
> [..]
> > diff --git a/include/linux/mm.h b/include/linux/mm.h
> > index 2d483dbdffc0..6831077d126c 100644
> > --- a/include/linux/mm.h
> > +++ b/include/linux/mm.h
> [..]
> > @@ -2609,6 +2596,7 @@ struct page *follow_page(struct vm_area_struct *vma, unsigned long address,
> > #define FOLL_REMOTE 0x2000 /* we are working on non-current tsk/mm */
> > #define FOLL_COW 0x4000 /* internal GUP flag */
> > #define FOLL_ANON 0x8000 /* don't do file mappings */
> > +#define FOLL_LONGTERM 0x10000 /* mapping is intended for a long term pin */
>
> Let's change this comment to say something like /* mapping lifetime is
> indefinite / at the discretion of userspace */, since "longterm is not
> well defined.
>
> I think it should also include a /* FIXME: */ to say something about
> the havoc a long term pin might wreak on fs and mm code paths.
Will do.
>
> > static inline int vm_fault_to_errno(vm_fault_t vm_fault, int foll_flags)
> > {
> > diff --git a/mm/gup.c b/mm/gup.c
> > index f84e22685aaa..8cb4cff067bc 100644
> > --- a/mm/gup.c
> > +++ b/mm/gup.c
> > @@ -1112,26 +1112,7 @@ long get_user_pages_remote(struct task_struct *tsk, struct mm_struct *mm,
> > }
> > EXPORT_SYMBOL(get_user_pages_remote);
> >
> > -/*
> > - * This is the same as get_user_pages_remote(), just with a
> > - * less-flexible calling convention where we assume that the task
> > - * and mm being operated on are the current task's and don't allow
> > - * passing of a locked parameter. We also obviously don't pass
> > - * FOLL_REMOTE in here.
> > - */
> > -long get_user_pages(unsigned long start, unsigned long nr_pages,
> > - unsigned int gup_flags, struct page **pages,
> > - struct vm_area_struct **vmas)
> > -{
> > - return __get_user_pages_locked(current, current->mm, start, nr_pages,
> > - pages, vmas, NULL,
> > - gup_flags | FOLL_TOUCH);
> > -}
> > -EXPORT_SYMBOL(get_user_pages);
> > -
> > #if defined(CONFIG_FS_DAX) || defined (CONFIG_CMA)
> > -
> > -#ifdef CONFIG_FS_DAX
> > static bool check_dax_vmas(struct vm_area_struct **vmas, long nr_pages)
> > {
> > long i;
> > @@ -1150,12 +1131,6 @@ static bool check_dax_vmas(struct vm_area_struct **vmas, long nr_pages)
> > }
> > return false;
> > }
> > -#else
> > -static inline bool check_dax_vmas(struct vm_area_struct **vmas, long nr_pages)
> > -{
> > - return false;
> > -}
> > -#endif
> >
> > #ifdef CONFIG_CMA
> > static struct page *new_non_cma_page(struct page *page, unsigned long private)
> > @@ -1209,10 +1184,13 @@ static struct page *new_non_cma_page(struct page *page, unsigned long private)
> > return __alloc_pages_node(nid, gfp_mask, 0);
> > }
> >
> > -static long check_and_migrate_cma_pages(unsigned long start, long nr_pages,
> > - unsigned int gup_flags,
> > +static long check_and_migrate_cma_pages(struct task_struct *tsk,
> > + struct mm_struct *mm,
> > + unsigned long start,
> > + unsigned long nr_pages,
> > struct page **pages,
> > - struct vm_area_struct **vmas)
> > + struct vm_area_struct **vmas,
> > + unsigned int gup_flags)
> > {
> > long i;
> > bool drain_allow = true;
> > @@ -1268,10 +1246,14 @@ static long check_and_migrate_cma_pages(unsigned long start, long nr_pages,
> > putback_movable_pages(&cma_page_list);
> > }
> > /*
> > - * We did migrate all the pages, Try to get the page references again
> > - * migrating any new CMA pages which we failed to isolate earlier.
> > + * We did migrate all the pages, Try to get the page references
> > + * again migrating any new CMA pages which we failed to isolate
> > + * earlier.
> > */
> > - nr_pages = get_user_pages(start, nr_pages, gup_flags, pages, vmas);
> > + nr_pages = __get_user_pages_locked(tsk, mm, start, nr_pages,
> > + pages, vmas, NULL,
> > + gup_flags);
> > +
>
> Why did this need to change to __get_user_pages_locked?
__get_uer_pages_locked() is now the "internal call" for get_user_pages.
Technically it did not _have_ to change but there is no need to call
get_user_pages() again because the FOLL_TOUCH flags is already set. Also this
call then matches the __get_user_pages_locked() which was called on the pages
we migrated from. Mostly this keeps the code "symmetrical" in that we called
__get_user_pages_locked() on the pages we are migrating from and the same call
on the pages we migrated to.
While the change here looks funny I think the final code is better.
>
> > if ((nr_pages > 0) && migrate_allow) {
> > drain_allow = true;
> > goto check_again;
> > @@ -1281,66 +1263,115 @@ static long check_and_migrate_cma_pages(unsigned long start, long nr_pages,
> > return nr_pages;
> > }
> > #else
> > -static inline long check_and_migrate_cma_pages(unsigned long start, long nr_pages,
> > - unsigned int gup_flags,
> > - struct page **pages,
> > - struct vm_area_struct **vmas)
> > +static long check_and_migrate_cma_pages(struct task_struct *tsk,
> > + struct mm_struct *mm,
> > + unsigned long start,
> > + unsigned long nr_pages,
> > + struct page **pages,
> > + struct vm_area_struct **vmas,
> > + unsigned int gup_flags)
> > {
> > return nr_pages;
> > }
> > #endif
> >
> > /*
> > - * This is the same as get_user_pages() in that it assumes we are
> > - * operating on the current task's mm, but it goes further to validate
> > - * that the vmas associated with the address range are suitable for
> > - * longterm elevated page reference counts. For example, filesystem-dax
> > - * mappings are subject to the lifetime enforced by the filesystem and
> > - * we need guarantees that longterm users like RDMA and V4L2 only
> > - * establish mappings that have a kernel enforced revocation mechanism.
> > + * __gup_longterm_locked() is a wrapper for __get_uer_pages_locked which
>
> s/uer/user/
Check
>
> > + * allows us to process the FOLL_LONGTERM flag if present.
> > + *
> > + * FOLL_LONGTERM Checks for either DAX VMAs or PPC CMA regions and either fails
> > + * the pin or attempts to migrate the page as appropriate.
> > + *
> > + * In the filesystem-dax case mappings are subject to the lifetime enforced by
> > + * the filesystem and we need guarantees that longterm users like RDMA and V4L2
> > + * only establish mappings that have a kernel enforced revocation mechanism.
> > + *
> > + * In the CMA case pages can't be pinned in a CMA region as this would
> > + * unnecessarily fragment that region. So CMA attempts to migrate the page
> > + * before pinning.
> > *
> > * "longterm" == userspace controlled elevated page count lifetime.
> > * Contrast this to iov_iter_get_pages() usages which are transient.
>
> Ah, here's the longterm documentation, but if I was a developer
> considering whether to use FOLL_LONGTERM or not I would expect to find
> the documentation at the flag definition site.
>
> I think it has become more clear since get_user_pages_longterm() was
> initially merged that we need to warn people not to use it, or at
> least seriously reconsider whether they want an interface to support
> indefinite pins.
Good point will move
>
> > */
> > -long get_user_pages_longterm(unsigned long start, unsigned long nr_pages,
> > - unsigned int gup_flags, struct page **pages,
> > - struct vm_area_struct **vmas_arg)
> > +static __always_inline long __gup_longterm_locked(struct task_struct *tsk,
>
> ...why the __always_inline?
This was because it was only called from get_user_pages() in this patch. But
later on I use it elsewhere so __always_inline is wrong.
Ira
^ permalink raw reply
* Re: [PATCH v3] powerpc/64: Fix memcmp reading past the end of src/dest
From: Michael Ellerman @ 2019-03-25 12:33 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: linuxppc-dev, chandan
In-Reply-To: <20190322182943.GF3969@gate.crashing.org>
Segher Boessenkool <segher@kernel.crashing.org> writes:
> On Fri, Mar 22, 2019 at 11:37:24PM +1100, Michael Ellerman wrote:
>> .Lcmp_rest_lt8bytes:
>> - /* Here we have only less than 8 bytes to compare with. at least s1
>> - * Address is aligned with 8 bytes.
>> - * The next double words are load and shift right with appropriate
>> - * bits.
>> + /*
>> + * Here we have less than 8 bytes to compare. At least s1 is aligned to
>> + * 8 bytes, but s2 may not be. We must make sure s2 + 8 doesn't cross a
>
> "s2 + 7"? The code is fine though (bgt, not bge).
Duh, thanks for catching it.
>> + * page boundary, otherwise we might read past the end of the buffer and
>> + * trigger a page fault. We use 4K as the conservative minimum page
>> + * size. If we detect that case we go to the byte-by-byte loop.
>> + *
>> + * Otherwise the next double word is loaded from s1 and s2, and shifted
>> + * right to compare the appropriate bits.
>> */
>> + clrldi r6,r4,(64-12) // r6 = r4 & 0xfff
>
> You can just write
> rlwinm r6,r4,0,0x0fff
> if that is clearer? Or do you still want a comment with that :-)
I don't think it's clearer doing a rotate of zero bits :)
And yeah I'd probably still leave the comment, so I'm inclined to stick
with the clrldi?
Would be nice if the assembler could support:
andi r6, r4, 0x0fff
And turn it into the rlwinm, or rldicl :)
>> + cmpdi r6,0xff8
>> + bgt .Lshort
>
> Reviewed-by: Segher Boessenkool <segher@kernel.crashing.org>
I'll fixup the comment. Thanks.
cheers
^ permalink raw reply
* PowerPC fixes 5.1-3: CONFIG_SPARSEMEM doesn't exist in the kernel source code
From: Christian Zigotzky @ 2019-03-25 12:23 UTC (permalink / raw)
To: linuxppc-dev; +Cc: R.T.Dickinson, Darren Stevens
In-Reply-To: <675068dc-3874-4ff3-fdf3-8b96b8e43574@xenosoft.de>
I was able to activate CONFIG_SPARSEMEM in the kernel configuration. But
does the P.A. Semi Nemo board need this option?
-- Christian
On 25 March 2019 at 12:00PM, Christian Zigotzky wrote:
> Hi All,
>
> I wasn't able to compile the RC2 today because of the following error
> messages:
>
> CC arch/powerpc/mm/slb.o
> In file included from ./arch/powerpc/include/asm/book3s/64/mmu.h:39:0,
> from ./arch/powerpc/include/asm/mmu.h:360,
> from ./arch/powerpc/include/asm/lppaca.h:36,
> from ./arch/powerpc/include/asm/paca.h:21,
> from ./arch/powerpc/include/asm/current.h:16,
> from ./include/linux/thread_info.h:21,
> from ./include/asm-generic/preempt.h:5,
> from ./arch/powerpc/include/generated/asm/preempt.h:1,
> from ./include/linux/preempt.h:78,
> from ./include/linux/spinlock.h:51,
> from ./include/linux/mmzone.h:8,
> from ./include/linux/gfp.h:6,
> from ./include/linux/mm.h:10,
> from ./arch/powerpc/include/asm/cacheflush.h:12,
> from ./arch/powerpc/include/asm/asm-prototypes.h:16,
> from arch/powerpc/mm/slb.c:17:
> ./arch/powerpc/include/asm/book3s/64/mmu-hash.h:584:6: warning:
> "MAX_PHYSMEM_BITS" is not defined [-Wundef]
> #if (MAX_PHYSMEM_BITS > MAX_EA_BITS_PER_CONTEXT)
> ^
> arch/powerpc/mm/slb.c: In function 'slb_allocate_kernel':
> arch/powerpc/mm/slb.c:697:37: error: 'MAX_PHYSMEM_BITS' undeclared
> (first use in this function)
> if ((ea & ~REGION_MASK) > (1UL << MAX_PHYSMEM_BITS))
> ^
> arch/powerpc/mm/slb.c:697:37: note: each undeclared identifier is
> reported only once for each function it appears in
> scripts/Makefile.build:278: recipe for target 'arch/powerpc/mm/slb.o'
> failed
> make[3]: *** [arch/powerpc/mm/slb.o] Error 1
> scripts/Makefile.build:489: recipe for target 'arch/powerpc/mm' failed
> make[2]: *** [arch/powerpc/mm] Error 2
> /home/christian/Downloads/a/Makefile:1046: recipe for target
> 'arch/powerpc' failed
> make[1]: *** [arch/powerpc] Error 2
> Makefile:170: recipe for target 'sub-make' failed
> make: *** [sub-make] Error 2
>
> -----
>
> The variable MAX_PHYSMEM_BITS isn't defined. The problem is in the
> last PowerPC fixes 5.1-3:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?h=v5.1-rc2&id=a5ed1e96cafde5ba48638f486bfca0685dc6ddc9
>
> Commit log: powerpc/mm: Only define MAX_PHYSMEM_BITS in SPARSEMEM
> configurations
>
> Problematic fix:
>
> diff --git a/arch/powerpc/include/asm/mmu.h
> b/arch/powerpc/include/asm/mmu.h
> index d34ad1657d7b..598cdcdd1355 100644
> --- a/arch/powerpc/include/asm/mmu.h
> +++ b/arch/powerpc/include/asm/mmu.h
> @@ -352,7 +352,7 @@ static inline bool strict_kernel_rwx_enabled(void)
> #if defined(CONFIG_SPARSEMEM_VMEMMAP) &&
> defined(CONFIG_SPARSEMEM_EXTREME) && \
> defined (CONFIG_PPC_64K_PAGES)
> #define MAX_PHYSMEM_BITS 51
> -#else
> +#elif defined(CONFIG_SPARSEMEM)
> #define MAX_PHYSMEM_BITS 46
> #endif
>
> The first if statement isn't successfull because the variables
> CONFIG_SPARSEMEM_EXTREME and CONFIG_PPC_64K_PAGES aren't activated in
> our kernel configuration. Therefore we need MAX_PHYSMEM_BITS 46 for
> our Nemo board. Unfortunately CONFIG_SPARSEMEM doesn't exist in the
> kernel source code so we can't activate it in the kernel config.
>
> I replaced '#elif defined(CONFIG_SPARSEMEM)' with '#else' and after
> that the compiling of the RC2 works again.
>
> Please check if CONFIG_SPARSEMEM exists.
>
> Thanks,
> Christian
^ permalink raw reply
* Re: powerpc/mm: Only define MAX_PHYSMEM_BITS in SPARSEMEM configurations
From: Christophe Leroy @ 2019-03-25 12:03 UTC (permalink / raw)
To: Michael Ellerman, Ben Hutchings, Andreas Schwab
Cc: linuxppc-dev, Paul Mackerras
In-Reply-To: <87a7hjp4oi.fsf@concordia.ellerman.id.au>
Le 25/03/2019 à 12:35, Michael Ellerman a écrit :
> Ben Hutchings <ben@decadent.org.uk> writes:
>> On Mon, 2019-03-25 at 01:03 +0100, Andreas Schwab wrote:
>>> On Mär 24 2019, Ben Hutchings <ben@decadent.org.uk> wrote:
>>>
>>>> Presumably you have CONFIG_PPC_BOOK3S_64 enabled and
>>>> CONFIG_SPARSEMEM
>>>> disabled? Was this configuration actually usable?
>>>
>>> Why not?
>>
>> I assume that CONFIG_SPARSEMEM is the default for a good reason.
>> What I don't know is how strong that reason is (I am not a Power expert
>> at all). Looking a bit further, it seems to be related to CONFIG_NUMA
>> in that you can enable CONFIG_FLATMEM if and only if that's disabled.
>> So I suppose the configuration you used works for non-NUMA systems.
>
> Aneesh pointed out this fix would break FLATMEM after I'd merged it, but
> it didn't break any of our defconfigs so I wondered if anyone would
> notice.
>
> I checked today and a G5 will boot with FLATMEM, which I assume is what
> Andreas is using.
>
> I guess we should fix this build break for now.
>
> Even some G5's have discontiguous memory, so FLATMEM is not clearly a
> good choice even for all G5's, and actually a fresh g5_defconfig uses
> SPARSEMEM.
>
> So I'm inclined to just switch to always using SPARSEMEM on 64-bit
> Book3S, because that's what's well tested and we hardly need more code
> paths to test. Unless anyone has a strong objection, I haven't actually
> benchmarked FLATMEM vs SPARSEMEM on a G5.
>
Christian Zigotzky reported a build failure with this patch.
Christophe
^ permalink raw reply
* Re: powerpc/mm: Only define MAX_PHYSMEM_BITS in SPARSEMEM configurations
From: Michael Ellerman @ 2019-03-25 11:35 UTC (permalink / raw)
To: Ben Hutchings, Andreas Schwab; +Cc: Paul Mackerras, linuxppc-dev
In-Reply-To: <cf9154f1812e0e9a328a1e21ef13d80a46e5feb6.camel@decadent.org.uk>
Ben Hutchings <ben@decadent.org.uk> writes:
> On Mon, 2019-03-25 at 01:03 +0100, Andreas Schwab wrote:
>> On Mär 24 2019, Ben Hutchings <ben@decadent.org.uk> wrote:
>>
>> > Presumably you have CONFIG_PPC_BOOK3S_64 enabled and
>> > CONFIG_SPARSEMEM
>> > disabled? Was this configuration actually usable?
>>
>> Why not?
>
> I assume that CONFIG_SPARSEMEM is the default for a good reason.
> What I don't know is how strong that reason is (I am not a Power expert
> at all). Looking a bit further, it seems to be related to CONFIG_NUMA
> in that you can enable CONFIG_FLATMEM if and only if that's disabled.
> So I suppose the configuration you used works for non-NUMA systems.
Aneesh pointed out this fix would break FLATMEM after I'd merged it, but
it didn't break any of our defconfigs so I wondered if anyone would
notice.
I checked today and a G5 will boot with FLATMEM, which I assume is what
Andreas is using.
I guess we should fix this build break for now.
Even some G5's have discontiguous memory, so FLATMEM is not clearly a
good choice even for all G5's, and actually a fresh g5_defconfig uses
SPARSEMEM.
So I'm inclined to just switch to always using SPARSEMEM on 64-bit
Book3S, because that's what's well tested and we hardly need more code
paths to test. Unless anyone has a strong objection, I haven't actually
benchmarked FLATMEM vs SPARSEMEM on a G5.
cheers
^ permalink raw reply
* PowerPC fixes 5.1-3: CONFIG_SPARSEMEM doesn't exist in the kernel source code
From: Christian Zigotzky @ 2019-03-25 11:00 UTC (permalink / raw)
To: linuxppc-dev; +Cc: R.T.Dickinson, Darren Stevens
Hi All,
I wasn't able to compile the RC2 today because of the following error
messages:
CC arch/powerpc/mm/slb.o
In file included from ./arch/powerpc/include/asm/book3s/64/mmu.h:39:0,
from ./arch/powerpc/include/asm/mmu.h:360,
from ./arch/powerpc/include/asm/lppaca.h:36,
from ./arch/powerpc/include/asm/paca.h:21,
from ./arch/powerpc/include/asm/current.h:16,
from ./include/linux/thread_info.h:21,
from ./include/asm-generic/preempt.h:5,
from ./arch/powerpc/include/generated/asm/preempt.h:1,
from ./include/linux/preempt.h:78,
from ./include/linux/spinlock.h:51,
from ./include/linux/mmzone.h:8,
from ./include/linux/gfp.h:6,
from ./include/linux/mm.h:10,
from ./arch/powerpc/include/asm/cacheflush.h:12,
from ./arch/powerpc/include/asm/asm-prototypes.h:16,
from arch/powerpc/mm/slb.c:17:
./arch/powerpc/include/asm/book3s/64/mmu-hash.h:584:6: warning:
"MAX_PHYSMEM_BITS" is not defined [-Wundef]
#if (MAX_PHYSMEM_BITS > MAX_EA_BITS_PER_CONTEXT)
^
arch/powerpc/mm/slb.c: In function 'slb_allocate_kernel':
arch/powerpc/mm/slb.c:697:37: error: 'MAX_PHYSMEM_BITS' undeclared
(first use in this function)
if ((ea & ~REGION_MASK) > (1UL << MAX_PHYSMEM_BITS))
^
arch/powerpc/mm/slb.c:697:37: note: each undeclared identifier is
reported only once for each function it appears in
scripts/Makefile.build:278: recipe for target 'arch/powerpc/mm/slb.o' failed
make[3]: *** [arch/powerpc/mm/slb.o] Error 1
scripts/Makefile.build:489: recipe for target 'arch/powerpc/mm' failed
make[2]: *** [arch/powerpc/mm] Error 2
/home/christian/Downloads/a/Makefile:1046: recipe for target
'arch/powerpc' failed
make[1]: *** [arch/powerpc] Error 2
Makefile:170: recipe for target 'sub-make' failed
make: *** [sub-make] Error 2
-----
The variable MAX_PHYSMEM_BITS isn't defined. The problem is in the last
PowerPC fixes 5.1-3:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?h=v5.1-rc2&id=a5ed1e96cafde5ba48638f486bfca0685dc6ddc9
Commit log: powerpc/mm: Only define MAX_PHYSMEM_BITS in SPARSEMEM
configurations
Problematic fix:
diff --git a/arch/powerpc/include/asm/mmu.h b/arch/powerpc/include/asm/mmu.h
index d34ad1657d7b..598cdcdd1355 100644
--- a/arch/powerpc/include/asm/mmu.h
+++ b/arch/powerpc/include/asm/mmu.h
@@ -352,7 +352,7 @@ static inline bool strict_kernel_rwx_enabled(void)
#if defined(CONFIG_SPARSEMEM_VMEMMAP) &&
defined(CONFIG_SPARSEMEM_EXTREME) && \
defined (CONFIG_PPC_64K_PAGES)
#define MAX_PHYSMEM_BITS 51
-#else
+#elif defined(CONFIG_SPARSEMEM)
#define MAX_PHYSMEM_BITS 46
#endif
The first if statement isn't successfull because the variables
CONFIG_SPARSEMEM_EXTREME and CONFIG_PPC_64K_PAGES aren't activated in
our kernel configuration. Therefore we need MAX_PHYSMEM_BITS 46 for our
Nemo board. Unfortunately CONFIG_SPARSEMEM doesn't exist in the kernel
source code so we can't activate it in the kernel config.
I replaced '#elif defined(CONFIG_SPARSEMEM)' with '#else' and after that
the compiling of the RC2 works again.
Please check if CONFIG_SPARSEMEM exists.
Thanks,
Christian
^ permalink raw reply related
* Re: [PATCH v2] powerpc/8xx: fix possible object reference leak
From: Michael Ellerman @ 2019-03-25 10:47 UTC (permalink / raw)
To: Peng Hao, vitb, benh, paulus, christophe.leroy
Cc: linuxppc-dev, Wen Yang, linux-kernel
In-Reply-To: <1553533919-75527-1-git-send-email-peng.hao2@zte.com.cn>
Peng Hao <peng.hao2@zte.com.cn> writes:
> From: Wen Yang <wen.yang99@zte.com.cn>
>
> The call to of_find_compatible_node returns a node pointer with refcount
> incremented thus it must be explicitly decremented after the last
> usage.
> irq_domain_add_linear also calls of_node_get to increase refcount,
> so irq_domain will not be affected when it is released.
>
> Detected by coccinelle with the following warnings:
> ./arch/powerpc/platforms/8xx/pic.c:158:1-7: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 136, but without a corresponding object release within this function.
>
> Fixes: a8db8cf0d894 ("irq_domain: Replace irq_alloc_host() with
> revmap-specific initializers")
> Signed-off-by: Wen Yang <wen.yang99@zte.com.cn>
> Suggested-by: Christophe Leroy <christophe.leroy@c-s.fr>
> Reviewed-by: Peng Hao <peng.hao2@zte.com.cn>
> Cc: Vitaly Bordug <vitb@kernel.crashing.org>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: linux-kernel@vger.kernel.org
> ---
> arch/powerpc/platforms/8xx/pic.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/arch/powerpc/platforms/8xx/pic.c b/arch/powerpc/platforms/8xx/pic.c
> index 8d5a25d..4453df6 100644
> --- a/arch/powerpc/platforms/8xx/pic.c
> +++ b/arch/powerpc/platforms/8xx/pic.c
> @@ -153,9 +153,7 @@ int mpc8xx_pic_init(void)
> if (mpc8xx_pic_host == NULL) {
> printk(KERN_ERR "MPC8xx PIC: failed to allocate irq host!\n");
> ret = -ENOMEM;
> - goto out;
> }
> - return 0;
It's fragile to rely on ret being equal to zero here. If the code
further up is changed it's easy enough to miss that ret is expected to
be zero.
Better to set it to zero here explicitly, this is the success path after
all, eg:
ret = 0;
> out:
> of_node_put(np);
cheers
^ permalink raw reply
* Re: [PATCH] compiler: allow all arches to enable CONFIG_OPTIMIZE_INLINING
From: Arnd Bergmann @ 2019-03-25 10:27 UTC (permalink / raw)
To: Masahiro Yamada
Cc: linux-arch, linux-s390, Dave Hansen, the arch/x86 maintainers,
linux-mips, Linux Kernel Mailing List, Paul Burton, Ingo Molnar,
linux-mtd, Andrew Morton, linuxppc-dev, Linux ARM
In-Reply-To: <CAK8P3a0GEYTbw5XCwzVeZe_-pGF=7e=1kXhH3U+fidnMZeP0CA@mail.gmail.com>
On Wed, Mar 20, 2019 at 2:34 PM Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Wed, Mar 20, 2019 at 10:41 AM Arnd Bergmann <arnd@arndb.de> wrote:
> >
> > I've added your patch to my randconfig test setup and will let you
> > know if I see anything noticeable. I'm currently testing clang-arm32,
> > clang-arm64 and gcc-x86.
>
> This is the only additional bug that has come up so far:
>
> `.exit.text' referenced in section `.alt.smp.init' of
> drivers/char/ipmi/ipmi_msghandler.o: defined in discarded section
> `exit.text' of drivers/char/ipmi/ipmi_msghandler.o
FWIW, the message up there does not match the patch anyway,
that was an unrelated bug.
Arnd
^ permalink raw reply
* Re: [PATCH] compiler: allow all arches to enable CONFIG_OPTIMIZE_INLINING
From: Arnd Bergmann @ 2019-03-25 10:25 UTC (permalink / raw)
To: Masahiro Yamada
Cc: linux-arch, linux-s390, Dave Hansen, the arch/x86 maintainers,
linux-mips, Linux Kernel Mailing List, Paul Burton, Ingo Molnar,
linux-mtd, Andrew Morton, linuxppc-dev, Linux ARM
In-Reply-To: <CAK7LNARatcRQ2u0JhiZHx4qU+mzS=SUv-kj7F-7-Vx--RFX5tw@mail.gmail.com>
On Mon, Mar 25, 2019 at 8:55 AM Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
>
> On Mon, Mar 25, 2019 at 4:33 PM Arnd Bergmann <arnd@arndb.de> wrote:
> >
> > On Mon, Mar 25, 2019 at 7:11 AM Masahiro Yamada
> > <yamada.masahiro@socionext.com> wrote:
> > > I do not know why to reproduce it,
> > > but is "__init __noreturn" more sensible than
> > > "__always_inline" here?
> >
> > It's in a header file, so it has to be 'inline'. We could make it
> > static inline __init __noreturn,
>
> Yes, I like 'static inline __init __noreturn'
>
> > but I don't see an advantage over
> > __always_inline there.
>
> __always_inline takes away the compiler's freedom.
>
> I'd like to leave it up to the compiler where possible.
Ok, fair enough.
Arnd
^ permalink raw reply
* Re: [PATCH v3 2/7] ocxl: Don't pass pci_dev around
From: Frederic Barrat @ 2019-03-25 10:04 UTC (permalink / raw)
To: Alastair D'Silva, alastair
Cc: Greg Kroah-Hartman, linuxppc-dev, Arnd Bergmann, Andrew Donnellan,
linux-kernel
In-Reply-To: <20190325054438.15022-3-alastair@au1.ibm.com>
Le 25/03/2019 à 06:44, Alastair D'Silva a écrit :
> From: Alastair D'Silva <alastair@d-silva.org>
>
> This data is already available in a struct
>
> Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> ---
Acked-by: Frederic Barrat <fbarrat@linux.ibm.com>
> drivers/misc/ocxl/core.c | 38 +++++++++++++++++++++-----------------
> 1 file changed, 21 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/misc/ocxl/core.c b/drivers/misc/ocxl/core.c
> index 1a4411b72d35..2f2fe12eac1e 100644
> --- a/drivers/misc/ocxl/core.c
> +++ b/drivers/misc/ocxl/core.c
> @@ -66,10 +66,11 @@ static int set_afu_device(struct ocxl_afu *afu, const char *location)
> return rc;
> }
>
> -static int assign_afu_actag(struct ocxl_afu *afu, struct pci_dev *dev)
> +static int assign_afu_actag(struct ocxl_afu *afu)
> {
> struct ocxl_fn *fn = afu->fn;
> int actag_count, actag_offset;
> + struct pci_dev *pci_dev = to_pci_dev(fn->dev.parent);
>
> /*
> * if there were not enough actags for the function, each afu
> @@ -79,16 +80,16 @@ static int assign_afu_actag(struct ocxl_afu *afu, struct pci_dev *dev)
> fn->actag_enabled / fn->actag_supported;
> actag_offset = ocxl_actag_afu_alloc(fn, actag_count);
> if (actag_offset < 0) {
> - dev_err(&afu->dev, "Can't allocate %d actags for AFU: %d\n",
> + dev_err(&pci_dev->dev, "Can't allocate %d actags for AFU: %d\n",
> actag_count, actag_offset);
> return actag_offset;
> }
> afu->actag_base = fn->actag_base + actag_offset;
> afu->actag_enabled = actag_count;
>
> - ocxl_config_set_afu_actag(dev, afu->config.dvsec_afu_control_pos,
> + ocxl_config_set_afu_actag(pci_dev, afu->config.dvsec_afu_control_pos,
> afu->actag_base, afu->actag_enabled);
> - dev_dbg(&afu->dev, "actag base=%d enabled=%d\n",
> + dev_dbg(&pci_dev->dev, "actag base=%d enabled=%d\n",
> afu->actag_base, afu->actag_enabled);
> return 0;
> }
> @@ -103,10 +104,11 @@ static void reclaim_afu_actag(struct ocxl_afu *afu)
> ocxl_actag_afu_free(afu->fn, start_offset, size);
> }
>
> -static int assign_afu_pasid(struct ocxl_afu *afu, struct pci_dev *dev)
> +static int assign_afu_pasid(struct ocxl_afu *afu)
> {
> struct ocxl_fn *fn = afu->fn;
> int pasid_count, pasid_offset;
> + struct pci_dev *pci_dev = to_pci_dev(fn->dev.parent);
>
> /*
> * We only support the case where the function configuration
> @@ -115,7 +117,7 @@ static int assign_afu_pasid(struct ocxl_afu *afu, struct pci_dev *dev)
> pasid_count = 1 << afu->config.pasid_supported_log;
> pasid_offset = ocxl_pasid_afu_alloc(fn, pasid_count);
> if (pasid_offset < 0) {
> - dev_err(&afu->dev, "Can't allocate %d PASIDs for AFU: %d\n",
> + dev_err(&pci_dev->dev, "Can't allocate %d PASIDs for AFU: %d\n",
> pasid_count, pasid_offset);
> return pasid_offset;
> }
> @@ -123,10 +125,10 @@ static int assign_afu_pasid(struct ocxl_afu *afu, struct pci_dev *dev)
> afu->pasid_count = 0;
> afu->pasid_max = pasid_count;
>
> - ocxl_config_set_afu_pasid(dev, afu->config.dvsec_afu_control_pos,
> + ocxl_config_set_afu_pasid(pci_dev, afu->config.dvsec_afu_control_pos,
> afu->pasid_base,
> afu->config.pasid_supported_log);
> - dev_dbg(&afu->dev, "PASID base=%d, enabled=%d\n",
> + dev_dbg(&pci_dev->dev, "PASID base=%d, enabled=%d\n",
> afu->pasid_base, pasid_count);
> return 0;
> }
> @@ -172,9 +174,10 @@ static void release_fn_bar(struct ocxl_fn *fn, int bar)
> WARN_ON(fn->bar_used[idx] < 0);
> }
>
> -static int map_mmio_areas(struct ocxl_afu *afu, struct pci_dev *dev)
> +static int map_mmio_areas(struct ocxl_afu *afu)
> {
> int rc;
> + struct pci_dev *pci_dev = to_pci_dev(afu->fn->dev.parent);
>
> rc = reserve_fn_bar(afu->fn, afu->config.global_mmio_bar);
> if (rc)
> @@ -187,10 +190,10 @@ static int map_mmio_areas(struct ocxl_afu *afu, struct pci_dev *dev)
> }
>
> afu->global_mmio_start =
> - pci_resource_start(dev, afu->config.global_mmio_bar) +
> + pci_resource_start(pci_dev, afu->config.global_mmio_bar) +
> afu->config.global_mmio_offset;
> afu->pp_mmio_start =
> - pci_resource_start(dev, afu->config.pp_mmio_bar) +
> + pci_resource_start(pci_dev, afu->config.pp_mmio_bar) +
> afu->config.pp_mmio_offset;
>
> afu->global_mmio_ptr = ioremap(afu->global_mmio_start,
> @@ -198,7 +201,7 @@ static int map_mmio_areas(struct ocxl_afu *afu, struct pci_dev *dev)
> if (!afu->global_mmio_ptr) {
> release_fn_bar(afu->fn, afu->config.pp_mmio_bar);
> release_fn_bar(afu->fn, afu->config.global_mmio_bar);
> - dev_err(&dev->dev, "Error mapping global mmio area\n");
> + dev_err(&pci_dev->dev, "Error mapping global mmio area\n");
> return -ENOMEM;
> }
>
> @@ -234,17 +237,17 @@ static int configure_afu(struct ocxl_afu *afu, u8 afu_idx, struct pci_dev *dev)
> if (rc)
> return rc;
>
> - rc = assign_afu_actag(afu, dev);
> + rc = assign_afu_actag(afu);
> if (rc)
> return rc;
>
> - rc = assign_afu_pasid(afu, dev);
> + rc = assign_afu_pasid(afu);
> if (rc) {
> reclaim_afu_actag(afu);
> return rc;
> }
>
> - rc = map_mmio_areas(afu, dev);
> + rc = map_mmio_areas(afu);
> if (rc) {
> reclaim_afu_pasid(afu);
> reclaim_afu_actag(afu);
> @@ -331,7 +334,7 @@ void remove_afu(struct ocxl_afu *afu)
> device_unregister(&afu->dev);
> }
>
> -static struct ocxl_fn *alloc_function(struct pci_dev *dev)
> +static struct ocxl_fn *alloc_function(void)
> {
> struct ocxl_fn *fn;
>
> @@ -342,6 +345,7 @@ static struct ocxl_fn *alloc_function(struct pci_dev *dev)
> INIT_LIST_HEAD(&fn->afu_list);
> INIT_LIST_HEAD(&fn->pasid_list);
> INIT_LIST_HEAD(&fn->actag_list);
> +
> return fn;
> }
>
> @@ -491,7 +495,7 @@ struct ocxl_fn *init_function(struct pci_dev *dev)
> struct ocxl_fn *fn;
> int rc;
>
> - fn = alloc_function(dev);
> + fn = alloc_function();
> if (!fn)
> return ERR_PTR(-ENOMEM);
>
>
^ permalink raw reply
* Re: [PATCH v3 1/7] ocxl: Split pci.c
From: Frederic Barrat @ 2019-03-25 10:01 UTC (permalink / raw)
To: Alastair D'Silva, alastair
Cc: Greg Kroah-Hartman, linuxppc-dev, Arnd Bergmann, Andrew Donnellan,
linux-kernel
In-Reply-To: <20190325054438.15022-2-alastair@au1.ibm.com>
Le 25/03/2019 à 06:44, Alastair D'Silva a écrit :
> From: Alastair D'Silva <alastair@d-silva.org>
>
> In preparation for making core code available for external drivers,
> move the core code out of pci.c and into core.c
>
> Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> ---
Acked-by: Frederic Barrat <fbarrat@linux.ibm.com>
> drivers/misc/ocxl/Makefile | 1 +
> drivers/misc/ocxl/core.c | 517 +++++++++++++++++++++++++++++
> drivers/misc/ocxl/ocxl_internal.h | 5 +
> drivers/misc/ocxl/pci.c | 519 +-----------------------------
> 4 files changed, 524 insertions(+), 518 deletions(-)
> create mode 100644 drivers/misc/ocxl/core.c
>
> diff --git a/drivers/misc/ocxl/Makefile b/drivers/misc/ocxl/Makefile
> index 5229dcda8297..bc4e39bfda7b 100644
> --- a/drivers/misc/ocxl/Makefile
> +++ b/drivers/misc/ocxl/Makefile
> @@ -3,6 +3,7 @@ ccflags-$(CONFIG_PPC_WERROR) += -Werror
>
> ocxl-y += main.o pci.o config.o file.o pasid.o
> ocxl-y += link.o context.o afu_irq.o sysfs.o trace.o
> +ocxl-y += core.o
> obj-$(CONFIG_OCXL) += ocxl.o
>
> # For tracepoints to include our trace.h from tracepoint infrastructure:
> diff --git a/drivers/misc/ocxl/core.c b/drivers/misc/ocxl/core.c
> new file mode 100644
> index 000000000000..1a4411b72d35
> --- /dev/null
> +++ b/drivers/misc/ocxl/core.c
> @@ -0,0 +1,517 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +// Copyright 2019 IBM Corp.
> +#include <linux/idr.h>
> +#include "ocxl_internal.h"
> +
> +static struct ocxl_fn *ocxl_fn_get(struct ocxl_fn *fn)
> +{
> + return (get_device(&fn->dev) == NULL) ? NULL : fn;
> +}
> +
> +static void ocxl_fn_put(struct ocxl_fn *fn)
> +{
> + put_device(&fn->dev);
> +}
> +
> +struct ocxl_afu *ocxl_afu_get(struct ocxl_afu *afu)
> +{
> + return (get_device(&afu->dev) == NULL) ? NULL : afu;
> +}
> +
> +void ocxl_afu_put(struct ocxl_afu *afu)
> +{
> + put_device(&afu->dev);
> +}
> +
> +static struct ocxl_afu *alloc_afu(struct ocxl_fn *fn)
> +{
> + struct ocxl_afu *afu;
> +
> + afu = kzalloc(sizeof(struct ocxl_afu), GFP_KERNEL);
> + if (!afu)
> + return NULL;
> +
> + mutex_init(&afu->contexts_lock);
> + mutex_init(&afu->afu_control_lock);
> + idr_init(&afu->contexts_idr);
> + afu->fn = fn;
> + ocxl_fn_get(fn);
> + return afu;
> +}
> +
> +static void free_afu(struct ocxl_afu *afu)
> +{
> + idr_destroy(&afu->contexts_idr);
> + ocxl_fn_put(afu->fn);
> + kfree(afu);
> +}
> +
> +static void free_afu_dev(struct device *dev)
> +{
> + struct ocxl_afu *afu = to_ocxl_afu(dev);
> +
> + ocxl_unregister_afu(afu);
> + free_afu(afu);
> +}
> +
> +static int set_afu_device(struct ocxl_afu *afu, const char *location)
> +{
> + struct ocxl_fn *fn = afu->fn;
> + int rc;
> +
> + afu->dev.parent = &fn->dev;
> + afu->dev.release = free_afu_dev;
> + rc = dev_set_name(&afu->dev, "%s.%s.%hhu", afu->config.name, location,
> + afu->config.idx);
> + return rc;
> +}
> +
> +static int assign_afu_actag(struct ocxl_afu *afu, struct pci_dev *dev)
> +{
> + struct ocxl_fn *fn = afu->fn;
> + int actag_count, actag_offset;
> +
> + /*
> + * if there were not enough actags for the function, each afu
> + * reduces its count as well
> + */
> + actag_count = afu->config.actag_supported *
> + fn->actag_enabled / fn->actag_supported;
> + actag_offset = ocxl_actag_afu_alloc(fn, actag_count);
> + if (actag_offset < 0) {
> + dev_err(&afu->dev, "Can't allocate %d actags for AFU: %d\n",
> + actag_count, actag_offset);
> + return actag_offset;
> + }
> + afu->actag_base = fn->actag_base + actag_offset;
> + afu->actag_enabled = actag_count;
> +
> + ocxl_config_set_afu_actag(dev, afu->config.dvsec_afu_control_pos,
> + afu->actag_base, afu->actag_enabled);
> + dev_dbg(&afu->dev, "actag base=%d enabled=%d\n",
> + afu->actag_base, afu->actag_enabled);
> + return 0;
> +}
> +
> +static void reclaim_afu_actag(struct ocxl_afu *afu)
> +{
> + struct ocxl_fn *fn = afu->fn;
> + int start_offset, size;
> +
> + start_offset = afu->actag_base - fn->actag_base;
> + size = afu->actag_enabled;
> + ocxl_actag_afu_free(afu->fn, start_offset, size);
> +}
> +
> +static int assign_afu_pasid(struct ocxl_afu *afu, struct pci_dev *dev)
> +{
> + struct ocxl_fn *fn = afu->fn;
> + int pasid_count, pasid_offset;
> +
> + /*
> + * We only support the case where the function configuration
> + * requested enough PASIDs to cover all AFUs.
> + */
> + pasid_count = 1 << afu->config.pasid_supported_log;
> + pasid_offset = ocxl_pasid_afu_alloc(fn, pasid_count);
> + if (pasid_offset < 0) {
> + dev_err(&afu->dev, "Can't allocate %d PASIDs for AFU: %d\n",
> + pasid_count, pasid_offset);
> + return pasid_offset;
> + }
> + afu->pasid_base = fn->pasid_base + pasid_offset;
> + afu->pasid_count = 0;
> + afu->pasid_max = pasid_count;
> +
> + ocxl_config_set_afu_pasid(dev, afu->config.dvsec_afu_control_pos,
> + afu->pasid_base,
> + afu->config.pasid_supported_log);
> + dev_dbg(&afu->dev, "PASID base=%d, enabled=%d\n",
> + afu->pasid_base, pasid_count);
> + return 0;
> +}
> +
> +static void reclaim_afu_pasid(struct ocxl_afu *afu)
> +{
> + struct ocxl_fn *fn = afu->fn;
> + int start_offset, size;
> +
> + start_offset = afu->pasid_base - fn->pasid_base;
> + size = 1 << afu->config.pasid_supported_log;
> + ocxl_pasid_afu_free(afu->fn, start_offset, size);
> +}
> +
> +static int reserve_fn_bar(struct ocxl_fn *fn, int bar)
> +{
> + struct pci_dev *dev = to_pci_dev(fn->dev.parent);
> + int rc, idx;
> +
> + if (bar != 0 && bar != 2 && bar != 4)
> + return -EINVAL;
> +
> + idx = bar >> 1;
> + if (fn->bar_used[idx]++ == 0) {
> + rc = pci_request_region(dev, bar, "ocxl");
> + if (rc)
> + return rc;
> + }
> + return 0;
> +}
> +
> +static void release_fn_bar(struct ocxl_fn *fn, int bar)
> +{
> + struct pci_dev *dev = to_pci_dev(fn->dev.parent);
> + int idx;
> +
> + if (bar != 0 && bar != 2 && bar != 4)
> + return;
> +
> + idx = bar >> 1;
> + if (--fn->bar_used[idx] == 0)
> + pci_release_region(dev, bar);
> + WARN_ON(fn->bar_used[idx] < 0);
> +}
> +
> +static int map_mmio_areas(struct ocxl_afu *afu, struct pci_dev *dev)
> +{
> + int rc;
> +
> + rc = reserve_fn_bar(afu->fn, afu->config.global_mmio_bar);
> + if (rc)
> + return rc;
> +
> + rc = reserve_fn_bar(afu->fn, afu->config.pp_mmio_bar);
> + if (rc) {
> + release_fn_bar(afu->fn, afu->config.global_mmio_bar);
> + return rc;
> + }
> +
> + afu->global_mmio_start =
> + pci_resource_start(dev, afu->config.global_mmio_bar) +
> + afu->config.global_mmio_offset;
> + afu->pp_mmio_start =
> + pci_resource_start(dev, afu->config.pp_mmio_bar) +
> + afu->config.pp_mmio_offset;
> +
> + afu->global_mmio_ptr = ioremap(afu->global_mmio_start,
> + afu->config.global_mmio_size);
> + if (!afu->global_mmio_ptr) {
> + release_fn_bar(afu->fn, afu->config.pp_mmio_bar);
> + release_fn_bar(afu->fn, afu->config.global_mmio_bar);
> + dev_err(&dev->dev, "Error mapping global mmio area\n");
> + return -ENOMEM;
> + }
> +
> + /*
> + * Leave an empty page between the per-process mmio area and
> + * the AFU interrupt mappings
> + */
> + afu->irq_base_offset = afu->config.pp_mmio_stride + PAGE_SIZE;
> + return 0;
> +}
> +
> +static void unmap_mmio_areas(struct ocxl_afu *afu)
> +{
> + if (afu->global_mmio_ptr) {
> + iounmap(afu->global_mmio_ptr);
> + afu->global_mmio_ptr = NULL;
> + }
> + afu->global_mmio_start = 0;
> + afu->pp_mmio_start = 0;
> + release_fn_bar(afu->fn, afu->config.pp_mmio_bar);
> + release_fn_bar(afu->fn, afu->config.global_mmio_bar);
> +}
> +
> +static int configure_afu(struct ocxl_afu *afu, u8 afu_idx, struct pci_dev *dev)
> +{
> + int rc;
> +
> + rc = ocxl_config_read_afu(dev, &afu->fn->config, &afu->config, afu_idx);
> + if (rc)
> + return rc;
> +
> + rc = set_afu_device(afu, dev_name(&dev->dev));
> + if (rc)
> + return rc;
> +
> + rc = assign_afu_actag(afu, dev);
> + if (rc)
> + return rc;
> +
> + rc = assign_afu_pasid(afu, dev);
> + if (rc) {
> + reclaim_afu_actag(afu);
> + return rc;
> + }
> +
> + rc = map_mmio_areas(afu, dev);
> + if (rc) {
> + reclaim_afu_pasid(afu);
> + reclaim_afu_actag(afu);
> + return rc;
> + }
> + return 0;
> +}
> +
> +static void deconfigure_afu(struct ocxl_afu *afu)
> +{
> + unmap_mmio_areas(afu);
> + reclaim_afu_pasid(afu);
> + reclaim_afu_actag(afu);
> +}
> +
> +static int activate_afu(struct pci_dev *dev, struct ocxl_afu *afu)
> +{
> + int rc;
> +
> + ocxl_config_set_afu_state(dev, afu->config.dvsec_afu_control_pos, 1);
> + /*
> + * Char device creation is the last step, as processes can
> + * call our driver immediately, so all our inits must be finished.
> + */
> + rc = ocxl_create_cdev(afu);
> + if (rc)
> + return rc;
> + return 0;
> +}
> +
> +static void deactivate_afu(struct ocxl_afu *afu)
> +{
> + struct pci_dev *dev = to_pci_dev(afu->fn->dev.parent);
> +
> + ocxl_destroy_cdev(afu);
> + ocxl_config_set_afu_state(dev, afu->config.dvsec_afu_control_pos, 0);
> +}
> +
> +int init_afu(struct pci_dev *dev, struct ocxl_fn *fn, u8 afu_idx)
> +{
> + int rc;
> + struct ocxl_afu *afu;
> +
> + afu = alloc_afu(fn);
> + if (!afu)
> + return -ENOMEM;
> +
> + rc = configure_afu(afu, afu_idx, dev);
> + if (rc) {
> + free_afu(afu);
> + return rc;
> + }
> +
> + rc = ocxl_register_afu(afu);
> + if (rc)
> + goto err;
> +
> + rc = ocxl_sysfs_add_afu(afu);
> + if (rc)
> + goto err;
> +
> + rc = activate_afu(dev, afu);
> + if (rc)
> + goto err_sys;
> +
> + list_add_tail(&afu->list, &fn->afu_list);
> + return 0;
> +
> +err_sys:
> + ocxl_sysfs_remove_afu(afu);
> +err:
> + deconfigure_afu(afu);
> + device_unregister(&afu->dev);
> + return rc;
> +}
> +
> +void remove_afu(struct ocxl_afu *afu)
> +{
> + list_del(&afu->list);
> + ocxl_context_detach_all(afu);
> + deactivate_afu(afu);
> + ocxl_sysfs_remove_afu(afu);
> + deconfigure_afu(afu);
> + device_unregister(&afu->dev);
> +}
> +
> +static struct ocxl_fn *alloc_function(struct pci_dev *dev)
> +{
> + struct ocxl_fn *fn;
> +
> + fn = kzalloc(sizeof(struct ocxl_fn), GFP_KERNEL);
> + if (!fn)
> + return NULL;
> +
> + INIT_LIST_HEAD(&fn->afu_list);
> + INIT_LIST_HEAD(&fn->pasid_list);
> + INIT_LIST_HEAD(&fn->actag_list);
> + return fn;
> +}
> +
> +static void free_function(struct ocxl_fn *fn)
> +{
> + WARN_ON(!list_empty(&fn->afu_list));
> + WARN_ON(!list_empty(&fn->pasid_list));
> + kfree(fn);
> +}
> +
> +static void free_function_dev(struct device *dev)
> +{
> + struct ocxl_fn *fn = to_ocxl_function(dev);
> +
> + free_function(fn);
> +}
> +
> +static int set_function_device(struct ocxl_fn *fn, struct pci_dev *dev)
> +{
> + int rc;
> +
> + fn->dev.parent = &dev->dev;
> + fn->dev.release = free_function_dev;
> + rc = dev_set_name(&fn->dev, "ocxlfn.%s", dev_name(&dev->dev));
> + if (rc)
> + return rc;
> + pci_set_drvdata(dev, fn);
> + return 0;
> +}
> +
> +static int assign_function_actag(struct ocxl_fn *fn)
> +{
> + struct pci_dev *dev = to_pci_dev(fn->dev.parent);
> + u16 base, enabled, supported;
> + int rc;
> +
> + rc = ocxl_config_get_actag_info(dev, &base, &enabled, &supported);
> + if (rc)
> + return rc;
> +
> + fn->actag_base = base;
> + fn->actag_enabled = enabled;
> + fn->actag_supported = supported;
> +
> + ocxl_config_set_actag(dev, fn->config.dvsec_function_pos,
> + fn->actag_base, fn->actag_enabled);
> + dev_dbg(&fn->dev, "actag range starting at %d, enabled %d\n",
> + fn->actag_base, fn->actag_enabled);
> + return 0;
> +}
> +
> +static int set_function_pasid(struct ocxl_fn *fn)
> +{
> + struct pci_dev *dev = to_pci_dev(fn->dev.parent);
> + int rc, desired_count, max_count;
> +
> + /* A function may not require any PASID */
> + if (fn->config.max_pasid_log < 0)
> + return 0;
> +
> + rc = ocxl_config_get_pasid_info(dev, &max_count);
> + if (rc)
> + return rc;
> +
> + desired_count = 1 << fn->config.max_pasid_log;
> +
> + if (desired_count > max_count) {
> + dev_err(&fn->dev,
> + "Function requires more PASIDs than is available (%d vs. %d)\n",
> + desired_count, max_count);
> + return -ENOSPC;
> + }
> +
> + fn->pasid_base = 0;
> + return 0;
> +}
> +
> +static int configure_function(struct ocxl_fn *fn, struct pci_dev *dev)
> +{
> + int rc;
> +
> + rc = pci_enable_device(dev);
> + if (rc) {
> + dev_err(&dev->dev, "pci_enable_device failed: %d\n", rc);
> + return rc;
> + }
> +
> + /*
> + * Once it has been confirmed to work on our hardware, we
> + * should reset the function, to force the adapter to restart
> + * from scratch.
> + * A function reset would also reset all its AFUs.
> + *
> + * Some hints for implementation:
> + *
> + * - there's not status bit to know when the reset is done. We
> + * should try reading the config space to know when it's
> + * done.
> + * - probably something like:
> + * Reset
> + * wait 100ms
> + * issue config read
> + * allow device up to 1 sec to return success on config
> + * read before declaring it broken
> + *
> + * Some shared logic on the card (CFG, TLX) won't be reset, so
> + * there's no guarantee that it will be enough.
> + */
> + rc = ocxl_config_read_function(dev, &fn->config);
> + if (rc)
> + return rc;
> +
> + rc = set_function_device(fn, dev);
> + if (rc)
> + return rc;
> +
> + rc = assign_function_actag(fn);
> + if (rc)
> + return rc;
> +
> + rc = set_function_pasid(fn);
> + if (rc)
> + return rc;
> +
> + rc = ocxl_link_setup(dev, 0, &fn->link);
> + if (rc)
> + return rc;
> +
> + rc = ocxl_config_set_TL(dev, fn->config.dvsec_tl_pos);
> + if (rc) {
> + ocxl_link_release(dev, fn->link);
> + return rc;
> + }
> + return 0;
> +}
> +
> +static void deconfigure_function(struct ocxl_fn *fn)
> +{
> + struct pci_dev *dev = to_pci_dev(fn->dev.parent);
> +
> + ocxl_link_release(dev, fn->link);
> + pci_disable_device(dev);
> +}
> +
> +struct ocxl_fn *init_function(struct pci_dev *dev)
> +{
> + struct ocxl_fn *fn;
> + int rc;
> +
> + fn = alloc_function(dev);
> + if (!fn)
> + return ERR_PTR(-ENOMEM);
> +
> + rc = configure_function(fn, dev);
> + if (rc) {
> + free_function(fn);
> + return ERR_PTR(rc);
> + }
> +
> + rc = device_register(&fn->dev);
> + if (rc) {
> + deconfigure_function(fn);
> + put_device(&fn->dev);
> + return ERR_PTR(rc);
> + }
> + return fn;
> +}
> +
> +void remove_function(struct ocxl_fn *fn)
> +{
> + deconfigure_function(fn);
> + device_unregister(&fn->dev);
> +}
> diff --git a/drivers/misc/ocxl/ocxl_internal.h b/drivers/misc/ocxl/ocxl_internal.h
> index 06fd98c989c8..81086534dab5 100644
> --- a/drivers/misc/ocxl/ocxl_internal.h
> +++ b/drivers/misc/ocxl/ocxl_internal.h
> @@ -150,4 +150,9 @@ int ocxl_afu_irq_set_fd(struct ocxl_context *ctx, u64 irq_offset,
> int eventfd);
> u64 ocxl_afu_irq_get_addr(struct ocxl_context *ctx, u64 irq_offset);
>
> +struct ocxl_fn *init_function(struct pci_dev *dev);
> +void remove_function(struct ocxl_fn *fn);
> +int init_afu(struct pci_dev *dev, struct ocxl_fn *fn, u8 afu_idx);
> +void remove_afu(struct ocxl_afu *afu);
> +
> #endif /* _OCXL_INTERNAL_H_ */
> diff --git a/drivers/misc/ocxl/pci.c b/drivers/misc/ocxl/pci.c
> index 21f425472a82..4ed7cb1a667f 100644
> --- a/drivers/misc/ocxl/pci.c
> +++ b/drivers/misc/ocxl/pci.c
> @@ -1,9 +1,6 @@
> // SPDX-License-Identifier: GPL-2.0+
> -// Copyright 2017 IBM Corp.
> +// Copyright 2019 IBM Corp.
> #include <linux/module.h>
> -#include <linux/pci.h>
> -#include <linux/idr.h>
> -#include <asm/pnv-ocxl.h>
> #include "ocxl_internal.h"
>
> /*
> @@ -17,520 +14,6 @@ static const struct pci_device_id ocxl_pci_tbl[] = {
> };
> MODULE_DEVICE_TABLE(pci, ocxl_pci_tbl);
>
> -
> -static struct ocxl_fn *ocxl_fn_get(struct ocxl_fn *fn)
> -{
> - return (get_device(&fn->dev) == NULL) ? NULL : fn;
> -}
> -
> -static void ocxl_fn_put(struct ocxl_fn *fn)
> -{
> - put_device(&fn->dev);
> -}
> -
> -struct ocxl_afu *ocxl_afu_get(struct ocxl_afu *afu)
> -{
> - return (get_device(&afu->dev) == NULL) ? NULL : afu;
> -}
> -
> -void ocxl_afu_put(struct ocxl_afu *afu)
> -{
> - put_device(&afu->dev);
> -}
> -
> -static struct ocxl_afu *alloc_afu(struct ocxl_fn *fn)
> -{
> - struct ocxl_afu *afu;
> -
> - afu = kzalloc(sizeof(struct ocxl_afu), GFP_KERNEL);
> - if (!afu)
> - return NULL;
> -
> - mutex_init(&afu->contexts_lock);
> - mutex_init(&afu->afu_control_lock);
> - idr_init(&afu->contexts_idr);
> - afu->fn = fn;
> - ocxl_fn_get(fn);
> - return afu;
> -}
> -
> -static void free_afu(struct ocxl_afu *afu)
> -{
> - idr_destroy(&afu->contexts_idr);
> - ocxl_fn_put(afu->fn);
> - kfree(afu);
> -}
> -
> -static void free_afu_dev(struct device *dev)
> -{
> - struct ocxl_afu *afu = to_ocxl_afu(dev);
> -
> - ocxl_unregister_afu(afu);
> - free_afu(afu);
> -}
> -
> -static int set_afu_device(struct ocxl_afu *afu, const char *location)
> -{
> - struct ocxl_fn *fn = afu->fn;
> - int rc;
> -
> - afu->dev.parent = &fn->dev;
> - afu->dev.release = free_afu_dev;
> - rc = dev_set_name(&afu->dev, "%s.%s.%hhu", afu->config.name, location,
> - afu->config.idx);
> - return rc;
> -}
> -
> -static int assign_afu_actag(struct ocxl_afu *afu, struct pci_dev *dev)
> -{
> - struct ocxl_fn *fn = afu->fn;
> - int actag_count, actag_offset;
> -
> - /*
> - * if there were not enough actags for the function, each afu
> - * reduces its count as well
> - */
> - actag_count = afu->config.actag_supported *
> - fn->actag_enabled / fn->actag_supported;
> - actag_offset = ocxl_actag_afu_alloc(fn, actag_count);
> - if (actag_offset < 0) {
> - dev_err(&afu->dev, "Can't allocate %d actags for AFU: %d\n",
> - actag_count, actag_offset);
> - return actag_offset;
> - }
> - afu->actag_base = fn->actag_base + actag_offset;
> - afu->actag_enabled = actag_count;
> -
> - ocxl_config_set_afu_actag(dev, afu->config.dvsec_afu_control_pos,
> - afu->actag_base, afu->actag_enabled);
> - dev_dbg(&afu->dev, "actag base=%d enabled=%d\n",
> - afu->actag_base, afu->actag_enabled);
> - return 0;
> -}
> -
> -static void reclaim_afu_actag(struct ocxl_afu *afu)
> -{
> - struct ocxl_fn *fn = afu->fn;
> - int start_offset, size;
> -
> - start_offset = afu->actag_base - fn->actag_base;
> - size = afu->actag_enabled;
> - ocxl_actag_afu_free(afu->fn, start_offset, size);
> -}
> -
> -static int assign_afu_pasid(struct ocxl_afu *afu, struct pci_dev *dev)
> -{
> - struct ocxl_fn *fn = afu->fn;
> - int pasid_count, pasid_offset;
> -
> - /*
> - * We only support the case where the function configuration
> - * requested enough PASIDs to cover all AFUs.
> - */
> - pasid_count = 1 << afu->config.pasid_supported_log;
> - pasid_offset = ocxl_pasid_afu_alloc(fn, pasid_count);
> - if (pasid_offset < 0) {
> - dev_err(&afu->dev, "Can't allocate %d PASIDs for AFU: %d\n",
> - pasid_count, pasid_offset);
> - return pasid_offset;
> - }
> - afu->pasid_base = fn->pasid_base + pasid_offset;
> - afu->pasid_count = 0;
> - afu->pasid_max = pasid_count;
> -
> - ocxl_config_set_afu_pasid(dev, afu->config.dvsec_afu_control_pos,
> - afu->pasid_base,
> - afu->config.pasid_supported_log);
> - dev_dbg(&afu->dev, "PASID base=%d, enabled=%d\n",
> - afu->pasid_base, pasid_count);
> - return 0;
> -}
> -
> -static void reclaim_afu_pasid(struct ocxl_afu *afu)
> -{
> - struct ocxl_fn *fn = afu->fn;
> - int start_offset, size;
> -
> - start_offset = afu->pasid_base - fn->pasid_base;
> - size = 1 << afu->config.pasid_supported_log;
> - ocxl_pasid_afu_free(afu->fn, start_offset, size);
> -}
> -
> -static int reserve_fn_bar(struct ocxl_fn *fn, int bar)
> -{
> - struct pci_dev *dev = to_pci_dev(fn->dev.parent);
> - int rc, idx;
> -
> - if (bar != 0 && bar != 2 && bar != 4)
> - return -EINVAL;
> -
> - idx = bar >> 1;
> - if (fn->bar_used[idx]++ == 0) {
> - rc = pci_request_region(dev, bar, "ocxl");
> - if (rc)
> - return rc;
> - }
> - return 0;
> -}
> -
> -static void release_fn_bar(struct ocxl_fn *fn, int bar)
> -{
> - struct pci_dev *dev = to_pci_dev(fn->dev.parent);
> - int idx;
> -
> - if (bar != 0 && bar != 2 && bar != 4)
> - return;
> -
> - idx = bar >> 1;
> - if (--fn->bar_used[idx] == 0)
> - pci_release_region(dev, bar);
> - WARN_ON(fn->bar_used[idx] < 0);
> -}
> -
> -static int map_mmio_areas(struct ocxl_afu *afu, struct pci_dev *dev)
> -{
> - int rc;
> -
> - rc = reserve_fn_bar(afu->fn, afu->config.global_mmio_bar);
> - if (rc)
> - return rc;
> -
> - rc = reserve_fn_bar(afu->fn, afu->config.pp_mmio_bar);
> - if (rc) {
> - release_fn_bar(afu->fn, afu->config.global_mmio_bar);
> - return rc;
> - }
> -
> - afu->global_mmio_start =
> - pci_resource_start(dev, afu->config.global_mmio_bar) +
> - afu->config.global_mmio_offset;
> - afu->pp_mmio_start =
> - pci_resource_start(dev, afu->config.pp_mmio_bar) +
> - afu->config.pp_mmio_offset;
> -
> - afu->global_mmio_ptr = ioremap(afu->global_mmio_start,
> - afu->config.global_mmio_size);
> - if (!afu->global_mmio_ptr) {
> - release_fn_bar(afu->fn, afu->config.pp_mmio_bar);
> - release_fn_bar(afu->fn, afu->config.global_mmio_bar);
> - dev_err(&dev->dev, "Error mapping global mmio area\n");
> - return -ENOMEM;
> - }
> -
> - /*
> - * Leave an empty page between the per-process mmio area and
> - * the AFU interrupt mappings
> - */
> - afu->irq_base_offset = afu->config.pp_mmio_stride + PAGE_SIZE;
> - return 0;
> -}
> -
> -static void unmap_mmio_areas(struct ocxl_afu *afu)
> -{
> - if (afu->global_mmio_ptr) {
> - iounmap(afu->global_mmio_ptr);
> - afu->global_mmio_ptr = NULL;
> - }
> - afu->global_mmio_start = 0;
> - afu->pp_mmio_start = 0;
> - release_fn_bar(afu->fn, afu->config.pp_mmio_bar);
> - release_fn_bar(afu->fn, afu->config.global_mmio_bar);
> -}
> -
> -static int configure_afu(struct ocxl_afu *afu, u8 afu_idx, struct pci_dev *dev)
> -{
> - int rc;
> -
> - rc = ocxl_config_read_afu(dev, &afu->fn->config, &afu->config, afu_idx);
> - if (rc)
> - return rc;
> -
> - rc = set_afu_device(afu, dev_name(&dev->dev));
> - if (rc)
> - return rc;
> -
> - rc = assign_afu_actag(afu, dev);
> - if (rc)
> - return rc;
> -
> - rc = assign_afu_pasid(afu, dev);
> - if (rc) {
> - reclaim_afu_actag(afu);
> - return rc;
> - }
> -
> - rc = map_mmio_areas(afu, dev);
> - if (rc) {
> - reclaim_afu_pasid(afu);
> - reclaim_afu_actag(afu);
> - return rc;
> - }
> - return 0;
> -}
> -
> -static void deconfigure_afu(struct ocxl_afu *afu)
> -{
> - unmap_mmio_areas(afu);
> - reclaim_afu_pasid(afu);
> - reclaim_afu_actag(afu);
> -}
> -
> -static int activate_afu(struct pci_dev *dev, struct ocxl_afu *afu)
> -{
> - int rc;
> -
> - ocxl_config_set_afu_state(dev, afu->config.dvsec_afu_control_pos, 1);
> - /*
> - * Char device creation is the last step, as processes can
> - * call our driver immediately, so all our inits must be finished.
> - */
> - rc = ocxl_create_cdev(afu);
> - if (rc)
> - return rc;
> - return 0;
> -}
> -
> -static void deactivate_afu(struct ocxl_afu *afu)
> -{
> - struct pci_dev *dev = to_pci_dev(afu->fn->dev.parent);
> -
> - ocxl_destroy_cdev(afu);
> - ocxl_config_set_afu_state(dev, afu->config.dvsec_afu_control_pos, 0);
> -}
> -
> -static int init_afu(struct pci_dev *dev, struct ocxl_fn *fn, u8 afu_idx)
> -{
> - int rc;
> - struct ocxl_afu *afu;
> -
> - afu = alloc_afu(fn);
> - if (!afu)
> - return -ENOMEM;
> -
> - rc = configure_afu(afu, afu_idx, dev);
> - if (rc) {
> - free_afu(afu);
> - return rc;
> - }
> -
> - rc = ocxl_register_afu(afu);
> - if (rc)
> - goto err;
> -
> - rc = ocxl_sysfs_add_afu(afu);
> - if (rc)
> - goto err;
> -
> - rc = activate_afu(dev, afu);
> - if (rc)
> - goto err_sys;
> -
> - list_add_tail(&afu->list, &fn->afu_list);
> - return 0;
> -
> -err_sys:
> - ocxl_sysfs_remove_afu(afu);
> -err:
> - deconfigure_afu(afu);
> - device_unregister(&afu->dev);
> - return rc;
> -}
> -
> -static void remove_afu(struct ocxl_afu *afu)
> -{
> - list_del(&afu->list);
> - ocxl_context_detach_all(afu);
> - deactivate_afu(afu);
> - ocxl_sysfs_remove_afu(afu);
> - deconfigure_afu(afu);
> - device_unregister(&afu->dev);
> -}
> -
> -static struct ocxl_fn *alloc_function(struct pci_dev *dev)
> -{
> - struct ocxl_fn *fn;
> -
> - fn = kzalloc(sizeof(struct ocxl_fn), GFP_KERNEL);
> - if (!fn)
> - return NULL;
> -
> - INIT_LIST_HEAD(&fn->afu_list);
> - INIT_LIST_HEAD(&fn->pasid_list);
> - INIT_LIST_HEAD(&fn->actag_list);
> - return fn;
> -}
> -
> -static void free_function(struct ocxl_fn *fn)
> -{
> - WARN_ON(!list_empty(&fn->afu_list));
> - WARN_ON(!list_empty(&fn->pasid_list));
> - kfree(fn);
> -}
> -
> -static void free_function_dev(struct device *dev)
> -{
> - struct ocxl_fn *fn = to_ocxl_function(dev);
> -
> - free_function(fn);
> -}
> -
> -static int set_function_device(struct ocxl_fn *fn, struct pci_dev *dev)
> -{
> - int rc;
> -
> - fn->dev.parent = &dev->dev;
> - fn->dev.release = free_function_dev;
> - rc = dev_set_name(&fn->dev, "ocxlfn.%s", dev_name(&dev->dev));
> - if (rc)
> - return rc;
> - pci_set_drvdata(dev, fn);
> - return 0;
> -}
> -
> -static int assign_function_actag(struct ocxl_fn *fn)
> -{
> - struct pci_dev *dev = to_pci_dev(fn->dev.parent);
> - u16 base, enabled, supported;
> - int rc;
> -
> - rc = ocxl_config_get_actag_info(dev, &base, &enabled, &supported);
> - if (rc)
> - return rc;
> -
> - fn->actag_base = base;
> - fn->actag_enabled = enabled;
> - fn->actag_supported = supported;
> -
> - ocxl_config_set_actag(dev, fn->config.dvsec_function_pos,
> - fn->actag_base, fn->actag_enabled);
> - dev_dbg(&fn->dev, "actag range starting at %d, enabled %d\n",
> - fn->actag_base, fn->actag_enabled);
> - return 0;
> -}
> -
> -static int set_function_pasid(struct ocxl_fn *fn)
> -{
> - struct pci_dev *dev = to_pci_dev(fn->dev.parent);
> - int rc, desired_count, max_count;
> -
> - /* A function may not require any PASID */
> - if (fn->config.max_pasid_log < 0)
> - return 0;
> -
> - rc = ocxl_config_get_pasid_info(dev, &max_count);
> - if (rc)
> - return rc;
> -
> - desired_count = 1 << fn->config.max_pasid_log;
> -
> - if (desired_count > max_count) {
> - dev_err(&fn->dev,
> - "Function requires more PASIDs than is available (%d vs. %d)\n",
> - desired_count, max_count);
> - return -ENOSPC;
> - }
> -
> - fn->pasid_base = 0;
> - return 0;
> -}
> -
> -static int configure_function(struct ocxl_fn *fn, struct pci_dev *dev)
> -{
> - int rc;
> -
> - rc = pci_enable_device(dev);
> - if (rc) {
> - dev_err(&dev->dev, "pci_enable_device failed: %d\n", rc);
> - return rc;
> - }
> -
> - /*
> - * Once it has been confirmed to work on our hardware, we
> - * should reset the function, to force the adapter to restart
> - * from scratch.
> - * A function reset would also reset all its AFUs.
> - *
> - * Some hints for implementation:
> - *
> - * - there's not status bit to know when the reset is done. We
> - * should try reading the config space to know when it's
> - * done.
> - * - probably something like:
> - * Reset
> - * wait 100ms
> - * issue config read
> - * allow device up to 1 sec to return success on config
> - * read before declaring it broken
> - *
> - * Some shared logic on the card (CFG, TLX) won't be reset, so
> - * there's no guarantee that it will be enough.
> - */
> - rc = ocxl_config_read_function(dev, &fn->config);
> - if (rc)
> - return rc;
> -
> - rc = set_function_device(fn, dev);
> - if (rc)
> - return rc;
> -
> - rc = assign_function_actag(fn);
> - if (rc)
> - return rc;
> -
> - rc = set_function_pasid(fn);
> - if (rc)
> - return rc;
> -
> - rc = ocxl_link_setup(dev, 0, &fn->link);
> - if (rc)
> - return rc;
> -
> - rc = ocxl_config_set_TL(dev, fn->config.dvsec_tl_pos);
> - if (rc) {
> - ocxl_link_release(dev, fn->link);
> - return rc;
> - }
> - return 0;
> -}
> -
> -static void deconfigure_function(struct ocxl_fn *fn)
> -{
> - struct pci_dev *dev = to_pci_dev(fn->dev.parent);
> -
> - ocxl_link_release(dev, fn->link);
> - pci_disable_device(dev);
> -}
> -
> -static struct ocxl_fn *init_function(struct pci_dev *dev)
> -{
> - struct ocxl_fn *fn;
> - int rc;
> -
> - fn = alloc_function(dev);
> - if (!fn)
> - return ERR_PTR(-ENOMEM);
> -
> - rc = configure_function(fn, dev);
> - if (rc) {
> - free_function(fn);
> - return ERR_PTR(rc);
> - }
> -
> - rc = device_register(&fn->dev);
> - if (rc) {
> - deconfigure_function(fn);
> - put_device(&fn->dev);
> - return ERR_PTR(rc);
> - }
> - return fn;
> -}
> -
> -static void remove_function(struct ocxl_fn *fn)
> -{
> - deconfigure_function(fn);
> - device_unregister(&fn->dev);
> -}
> -
> static int ocxl_probe(struct pci_dev *dev, const struct pci_device_id *id)
> {
> int rc, afu_count = 0;
>
^ 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