* Re: [PATCH 5/7] CXL: Disable AFU debug flag
From: Ian Munsie @ 2014-12-09 5:39 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <1418026681-14787-5-git-send-email-imunsie@au.ibm.com>
This one would be nice to go to stable, but I'm not sure it really meets
the rules. It could be a problem for userspace error paths checking the
result of MMIO reads, but only if the AFU has actually been unexpectedly
disabled somehow yet the PSL is still responding...
I don't think this is a high priority to go to stable unless it starts
causing problems for someone.
Cheers,
-Ian
^ permalink raw reply
* Re: [PATCH 7/7] CXL: Unmap MMIO regions when detaching a context
From: Ian Munsie @ 2014-12-09 5:41 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <1418026681-14787-7-git-send-email-imunsie@au.ibm.com>
This one should go to stable - this was the first bug uncovered after
fixing the sleep while atomic and force unbinding the driver.
Cheers,
-Ian
Excerpts from Ian Munsie's message of 2014-12-08 19:18:01 +1100:
> From: Ian Munsie <imunsie@au1.ibm.com>
>
> If we need to force detach a context (e.g. due to EEH or simply force
> unbinding the driver) we should prevent the userspace contexts from
> being able to access the Problem State Area MMIO region further, which
> they may have mapped with mmap().
>
> This patch unmaps any mapped MMIO regions when detaching a userspace
> context.
>
> Signed-off-by: Ian Munsie <imunsie@au1.ibm.com>
> ---
> drivers/misc/cxl/context.c | 11 ++++++++++-
> drivers/misc/cxl/cxl.h | 7 ++++++-
> drivers/misc/cxl/file.c | 6 +++++-
> 3 files changed, 21 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/misc/cxl/context.c b/drivers/misc/cxl/context.c
> index 4aa31a3..51fd6b5 100644
> --- a/drivers/misc/cxl/context.c
> +++ b/drivers/misc/cxl/context.c
> @@ -34,7 +34,8 @@ struct cxl_context *cxl_context_alloc(void)
> /*
> * Initialises a CXL context.
> */
> -int cxl_context_init(struct cxl_context *ctx, struct cxl_afu *afu, bool master)
> +int cxl_context_init(struct cxl_context *ctx, struct cxl_afu *afu, bool master,
> + struct address_space *mapping)
> {
> int i;
>
> @@ -42,6 +43,8 @@ int cxl_context_init(struct cxl_context *ctx, struct cxl_afu *afu, bool master)
> ctx->afu = afu;
> ctx->master = master;
> ctx->pid = NULL; /* Set in start work ioctl */
> + mutex_init(&ctx->mapping_lock);
> + ctx->mapping = mapping;
>
> /*
> * Allocate the segment table before we put it in the IDR so that we
> @@ -147,6 +150,12 @@ static void __detach_context(struct cxl_context *ctx)
> afu_release_irqs(ctx);
> flush_work(&ctx->fault_work); /* Only needed for dedicated process */
> wake_up_all(&ctx->wq);
> +
> + /* Release Problem State Area mapping */
> + mutex_lock(&ctx->mapping_lock);
> + if (ctx->mapping)
> + unmap_mapping_range(ctx->mapping, 0, 0, 1);
> + mutex_unlock(&ctx->mapping_lock);
> }
>
> /*
> diff --git a/drivers/misc/cxl/cxl.h b/drivers/misc/cxl/cxl.h
> index c1f8aa6..0df0438 100644
> --- a/drivers/misc/cxl/cxl.h
> +++ b/drivers/misc/cxl/cxl.h
> @@ -405,6 +405,10 @@ struct cxl_context {
> phys_addr_t psn_phys;
> u64 psn_size;
>
> + /* Used to unmap any mmaps when force detaching */
> + struct address_space *mapping;
> + struct mutex mapping_lock;
> +
> spinlock_t sste_lock; /* Protects segment table entries */
> struct cxl_sste *sstp;
> u64 sstp0, sstp1;
> @@ -606,7 +610,8 @@ int cxl_alloc_sst(struct cxl_context *ctx);
> void init_cxl_native(void);
>
> struct cxl_context *cxl_context_alloc(void);
> -int cxl_context_init(struct cxl_context *ctx, struct cxl_afu *afu, bool master);
> +int cxl_context_init(struct cxl_context *ctx, struct cxl_afu *afu, bool master,
> + struct address_space *mapping);
> void cxl_context_free(struct cxl_context *ctx);
> int cxl_context_iomap(struct cxl_context *ctx, struct vm_area_struct *vma);
>
> diff --git a/drivers/misc/cxl/file.c b/drivers/misc/cxl/file.c
> index 2e067a5..b09be44 100644
> --- a/drivers/misc/cxl/file.c
> +++ b/drivers/misc/cxl/file.c
> @@ -77,7 +77,7 @@ static int __afu_open(struct inode *inode, struct file *file, bool master)
> goto err_put_afu;
> }
>
> - if ((rc = cxl_context_init(ctx, afu, master)))
> + if ((rc = cxl_context_init(ctx, afu, master, inode->i_mapping)))
> goto err_put_afu;
>
> pr_devel("afu_open pe: %i\n", ctx->pe);
> @@ -113,6 +113,10 @@ static int afu_release(struct inode *inode, struct file *file)
> __func__, ctx->pe);
> cxl_context_detach(ctx);
>
> + mutex_lock(&ctx->mapping_lock);
> + ctx->mapping = NULL;
> + mutex_unlock(&ctx->mapping_lock);
> +
> put_device(&ctx->afu->dev);
>
> /*
^ permalink raw reply
* [PATCH] selftest/ppc: Add subpage protection self test.
From: Aneesh Kumar K.V @ 2014-12-09 6:14 UTC (permalink / raw)
To: benh, paulus, mpe; +Cc: linuxppc-dev, Aneesh Kumar K.V
From: Paul Mackerras <paulus@samba.org>
Signed-off-by: Paul Mackerras <paulus@samba.org>
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
tools/testing/selftests/powerpc/mm/Makefile | 2 +-
tools/testing/selftests/powerpc/mm/subpage_prot.c | 201 ++++++++++++++++++++++
2 files changed, 202 insertions(+), 1 deletion(-)
create mode 100644 tools/testing/selftests/powerpc/mm/subpage_prot.c
diff --git a/tools/testing/selftests/powerpc/mm/Makefile b/tools/testing/selftests/powerpc/mm/Makefile
index 357ccbd6bad9..fb00c6f7d675 100644
--- a/tools/testing/selftests/powerpc/mm/Makefile
+++ b/tools/testing/selftests/powerpc/mm/Makefile
@@ -1,7 +1,7 @@
noarg:
$(MAKE) -C ../
-PROGS := hugetlb_vs_thp_test
+PROGS := hugetlb_vs_thp_test subpage_prot
all: $(PROGS)
diff --git a/tools/testing/selftests/powerpc/mm/subpage_prot.c b/tools/testing/selftests/powerpc/mm/subpage_prot.c
new file mode 100644
index 000000000000..62f0db3e3fb2
--- /dev/null
+++ b/tools/testing/selftests/powerpc/mm/subpage_prot.c
@@ -0,0 +1,201 @@
+/*
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of version 2.1 of the GNU Lesser General Public License
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it would be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
+ *
+ */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <signal.h>
+#include <stdarg.h>
+#include <sys/ptrace.h>
+#include <sys/mman.h>
+#include <errno.h>
+#include <ucontext.h>
+#include <assert.h>
+#include <fcntl.h>
+
+#include "utils.h"
+
+void *mallocblock;
+unsigned long mallocsize;
+char *file_name;
+void *fileblock;
+off_t filesize;
+
+int in_test;
+volatile int faulted;
+volatile void *dar;
+int errors;
+
+static void segv(int signum, siginfo_t *info, void *ctxt_v)
+{
+ ucontext_t *ctxt = (ucontext_t *)ctxt_v;
+ struct pt_regs *regs = ctxt->uc_mcontext.regs;
+
+ if (!in_test) {
+ fprintf(stderr, "Segfault outside of test !\n");
+ exit(1);
+ }
+ faulted = 1;
+ dar = (void *)regs->dar;
+ regs->nip += 4;
+}
+
+static inline void do_read(const volatile void *addr)
+{
+ int ret;
+
+ asm volatile("lwz %0,0(%1); twi 0,%0,0; isync;\n"
+ : "=r" (ret) : "r" (addr) : "memory");
+}
+
+static inline void do_write(const volatile void *addr)
+{
+ int val = 0x1234567;
+
+ asm volatile("stw %0,0(%1); sync; \n"
+ : : "r" (val), "r" (addr) : "memory");
+}
+
+static inline void check_faulted(void *addr, long page, long subpage, int write)
+{
+ int want_fault = (subpage == ((page + 3) % 16));
+
+ if (write)
+ want_fault |= (subpage == ((page + 1) % 16));
+
+ if (faulted != want_fault) {
+ printf("Failed at 0x%p (p=%ld,sp=%ld,w=%d), want=%s, got=%s !\n",
+ addr, page, subpage, write,
+ want_fault ? "fault" : "pass",
+ faulted ? "fault" : "pass");
+ ++errors;
+ }
+ if (faulted) {
+ if (dar != addr) {
+ printf("Fault expected at 0x%p and happened at 0x%p !\n",
+ addr, dar);
+ }
+ faulted = 0;
+ asm volatile("sync" : : : "memory");
+ }
+}
+
+static int run_test(void *addr, unsigned long size)
+{
+ unsigned int *map;
+ long i, j, pages, err;
+
+ pages = size / 0x10000;
+ map = malloc(pages * 4);
+ assert(map);
+
+ /* for each page, mark subpage i % 16 read only and subpage
+ * (i + 3) % 16 inaccessible
+ */
+ for (i = 0; i < pages; i++)
+ map[i] = (0x40000000 >> (((i + 1) * 2) % 32)) |
+ (0xc0000000 >> (((i + 3) * 2) % 32));
+ err = syscall(310, addr, size, map);
+ if (err) {
+ perror("subpage_perm");
+ return 1;
+ }
+ free(map);
+
+ in_test = 1;
+ errors = 0;
+ for (i = 0; i < pages; i++)
+ for (j = 0; j < 16; j++, addr += 0x1000) {
+ do_read(addr);
+ check_faulted(addr, i, j, 0);
+ do_write(addr);
+ check_faulted(addr, i, j, 1);
+ }
+ in_test = 0;
+ if (errors) {
+ printf("%d errors detected\n", errors);
+ return 1;
+ }
+ return 0;
+}
+
+int test_anon(void)
+{
+ unsigned long align;
+
+ if (getpagesize() != 0x10000) {
+ fprintf(stderr, "Kernel page size must be 64K!\n");
+ return 1;
+ }
+
+ struct sigaction act = {
+ .sa_sigaction = segv,
+ .sa_flags = SA_SIGINFO
+ };
+ sigaction(SIGSEGV, &act, NULL);
+
+ mallocsize = 4*16*1024*1024;
+ posix_memalign(&mallocblock, 64*1024, mallocsize);
+ assert(mallocblock);
+ align = (unsigned long)mallocblock;
+ if (align & 0xffff)
+ align = (align | 0xffff) + 1;
+ mallocblock = (void *)align;
+
+ printf("allocated malloc block of 0x%lx bytes at 0x%p\n",
+ mallocsize, mallocblock);
+
+ printf("testing malloc block...\n");
+ return run_test(mallocblock, mallocsize);
+}
+
+int test_file(void)
+{
+ struct sigaction act = {
+ .sa_sigaction = segv,
+ .sa_flags = SA_SIGINFO
+ };
+ int fd = open(file_name, O_RDWR);
+
+ if (fd == -1) {
+ perror("failed to open file");
+ return 1;
+ }
+ sigaction(SIGSEGV, &act, NULL);
+
+
+ filesize = lseek(fd, 0, SEEK_END);
+ if (filesize & 0xffff)
+ filesize &= ~0xfffful;
+ fileblock = mmap(NULL, filesize, PROT_READ | PROT_WRITE,
+ MAP_SHARED, fd, 0);
+ if (fileblock == MAP_FAILED) {
+ perror("failed to map file");
+ return 1;
+ }
+ printf("allocated %s for 0x%lx bytes at 0x%p\n",
+ file_name, filesize, fileblock);
+
+ printf("testing file map...\n");
+ return run_test(fileblock, filesize);
+}
+
+int main(int argc, char *argv[])
+{
+ test_harness(test_anon, "subpage_prot_anon");
+ if (argc > 1) {
+ file_name = argv[1];
+ test_harness(test_file, "subpage_prot_file");
+ }
+ return 0;
+}
--
2.1.0
^ permalink raw reply related
* Re: [PATCH 2 1/4] powerpc: drop the ability to tweak SMT mode at boot time
From: Greg Kurz @ 2014-12-09 8:53 UTC (permalink / raw)
To: Michael Ellerman; +Cc: Scott Wood, linuxppc-dev
In-Reply-To: <1418098262.527.1.camel@concordia>
On Tue, 09 Dec 2014 15:11:02 +1100
Michael Ellerman <mpe@ellerman.id.au> wrote:
> On Fri, 2014-12-05 at 12:52 -0600, Scott Wood wrote:
> > On Fri, 2014-12-05 at 16:14 +0100, Greg Kurz wrote:
> > > The smt-enabled kernel parameter basically leaves unwanted cpus executing
> > > in firmware or wherever they happen to be. The very same applies to the
> > > ibm,smt-enabled DT property which is no more used by anything known. These
> > > are hacks that shoudn't be used in a production environment.
> > >
> > > Quoting mpe, "there are better ways for firmware to disable SMT".
> >
> > Those "better ways" don't apply to Freescale chips, where the OS enables
> > (or not) SMT without any interaction with firmware.
>
> But how does it know there even are SMT threads? From the device tree? So
> just don't present the threads in the device tree?
>
> cheers
>
>
Michael,
Maybe we can first kill the cpu_bootable hook in powernv only, for bug fix.
Then we can take time to do the thing right for all platforms. Thoughts ?
--
Greg
^ permalink raw reply
* Re: [PATCH v3] i2c: Driver to expose PowerNV platform i2c busses
From: Wolfram Sang @ 2014-12-09 8:54 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: Neelesh Gupta, linuxppc-dev, linux-i2c, devicetree
In-Reply-To: <1418075026.13358.4.camel@kernel.crashing.org>
[-- Attachment #1: Type: text/plain, Size: 1042 bytes --]
> > Oh, I thought we agreed that you take it via powerpc. I still think this
> > is the best solution.
>
> I threatened to do that :-) I don't remember you replying, did I miss
> it ?
It is here:
http://thread.gmane.org/gmane.linux.drivers.i2c/20762/focus=21099
> If you are ok with the driver and are happy for me to take it,
> please send an Ack.
"Happy" is not the correct word, but let's just go over with it. Maybe
like this:
Acked-by: Wolfram Sang <wsa@the-dreams.de> (I2C part, excluding the bindings)
> From a binding perspective, it's just a piece of additional info that
> the firmware provides for convenience.
I do understand the use case. I even agree it makes sense to have
something like this. It is just that I'd prefer a generic, widely
acknowledged solution, with consensus where it belongs and how it should
be named. Not a custom solution which, frankly, feels forced on me
by time pressure I have nothing to do with. So, not happy here, but also
not looking for drama. Let's move on...
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH v3] i2c: Driver to expose PowerNV platform i2c busses
From: Benjamin Herrenschmidt @ 2014-12-09 9:43 UTC (permalink / raw)
To: Wolfram Sang; +Cc: Neelesh Gupta, linuxppc-dev, linux-i2c, devicetree
In-Reply-To: <20141209085435.GA1154@katana>
On Tue, 2014-12-09 at 09:54 +0100, Wolfram Sang wrote:
> > > Oh, I thought we agreed that you take it via powerpc. I still think this
> > > is the best solution.
> >
> > I threatened to do that :-) I don't remember you replying, did I miss
> > it ?
> It is here:
> http://thread.gmane.org/gmane.linux.drivers.i2c/20762/focus=21099
Weird, it never made it to my mbox... anyway:
> I did not invent DT bindings.
No I did :) Or rather Mitch did with OF and I contributed heavily at
slapping it onto everbody's face :) Then I let Grant run with it and
deal with the carnage... but as you can imagine, I feel like whatever
rule I made for others don't apply to me :)
> I did not invent that DT is/should be a
>hardware description.
This is the basic idea but it's flexible. It is in essence a description
of the environment, which is essentially the HW but there is no taboo
about adding various ancilliary pieces of informations that one deems
useful, especially if they are prefixed by a vendor prefix to avoid
collision with "well defined" properties.
I think we ended up being fairly strict about the rules initially to try
to rein in the crowd of ARM embedded folks who really went all over the
place but like every rule, it's meant to be broken (hear the french guy
talking ...).
>For me, it is a burden that I (as a subsystem
> maintainer for mainly drivers) have to prevent people from using DT ?>
> for software configuration (some people use it as an 1:1 mapping for
> platform data even.)
Well, yes and no... for example, it's perfectly legit to have a node
representing a UART, have the firmware slap into it the default expected
baud rate as a property (or whatever it has configured it to be) which
makes it then a reasonable default for the kernel to use.
Generally speaking there is nothing fundamentally wrong about having
configuration information in the device-tree, but it has to be clearly
identifiable as such.
The description of the HW in my world at least also implies how that HW
is meant to be configured for a given platform.
> Since there are no guidelines (probably there can't
> be), I developed a set of rules out of experience and when those don't
> match I ask for help. Having a different set of rules for
> powerpc/arm/... (or server/embedded for that matter) will increase
> this burden a lot. People will come and say "But they did it as
> well..."
The basic rule is "does it make sense ?". Really. Apply your jugement
based on your experience as to whether something is a reasonable
comprimise or not and whether it will turn into a big mess in the long
run or, on the contrary, is a perfectly fine ad-hoc solution for a given
setup.
> It's getting quite tempting to just throw that driver into powerpc.git
Maybe this is the easiest. Just make sure that MAINTAINERS also point
this driver to you or PowerNV maintainers. And no Ack from me, please.
Then, I can always say "I dunno" if people start asking questions.
:)
Technically I need your ack if we are to follow the process for Linux
upstreaming. I doubt Linux will holler if I just put it in the tree but
I'd rather follow the process if possible.
>> And I don't give a flying crap about what random ARM SOC vendor
>> thinks of my powerpc FW interface for a powerpc unique FW interface.
>
> But you are not alone here. If you open the box for giving busses a
> configurable name, I can see other people (without FW) wanting this,
> too. So, this discussion will come anyhow IMO.
Right and I personally don't see a problem with that ... what's
fundamentally wrong with letting the platform description (ie,. the DT)
specify reasonable names for i2c busses ? It has pretty much no impact
on drivers nowadays but means things are easier to figure out/locate for
users/admin/developers and eases diagnostics.
> > If you are ok with the driver and are happy for me to take it,
> > please send an Ack.
>
> "Happy" is not the correct word, but let's just go over with it. Maybe
> like this:
>
> Acked-by: Wolfram Sang <wsa@the-dreams.de> (I2C part, excluding the bindings)
Forget about the binding mess, Olof reminded me that the result of one
of the recent KS was that the bindings no longer needed "approval", and
are to be sent to the list purely for informational purposes, otherwise
the process is a mess. We have to provide at least some trust here, and
we can reject the driver if we think the binding is really way too
gross.
> > From a binding perspective, it's just a piece of additional info that
> > the firmware provides for convenience.
>
> I do understand the use case. I even agree it makes sense to have
> something like this. It is just that I'd prefer a generic, widely
> acknowledged solution, with consensus where it belongs and how it should
> be named. Not a custom solution which, frankly, feels forced on me
> by time pressure I have nothing to do with. So, not happy here, but also
> not looking for drama. Let's move on...
Adding a generic binding for i2c controllers to name their respective
busses sounds like a laudable idea, and if that happens I'll be happy to
update the driver to take that into account so that a future FW version
can add it (in addition to the old property for backward compat).
Ben.
^ permalink raw reply
* Re: [v3] i2c: Driver to expose PowerNV platform i2c busses
From: Michael Ellerman @ 2014-12-09 9:45 UTC (permalink / raw)
To: Neelesh Gupta, linuxppc-dev, devicetree, linux-i2c, wsa
In-Reply-To: <20141208063039.14356.34770.stgit@localhost.localdomain>
On Mon, 2014-08-12 at 06:36:16 UTC, Neelesh Gupta wrote:
> The patch exposes the available i2c busses on the PowerNV platform
> to the kernel and implements the bus driver to support i2c and
> smbus commands.
> The driver uses the platform device infrastructure to probe the busses
> on the platform and registers them with the i2c driver framework.
> arch/powerpc/include/asm/opal.h | 29 ++
> arch/powerpc/platforms/powernv/opal-wrappers.S | 1
> arch/powerpc/platforms/powernv/opal.c | 11 +
This had major conflicts in the above files.
I've fixed it up but please check I did it correctly:
https://github.com/mpe/powerpc-merge/commits/pw/neelesh-i2c
It also doesn't build as a module:
$ grep CONFIG_I2C_OPAL .config
CONFIG_I2C_OPAL=m
$ make ...
...
ERROR: "opal_i2c_request" [drivers/i2c/busses/i2c-opal.ko] undefined!
cheers
^ permalink raw reply
* Re: [4/5] powerpc, dscr: Added some in-code documentation
From: Michael Ellerman @ 2014-12-09 10:03 UTC (permalink / raw)
To: Anshuman Khandual, linuxppc-dev; +Cc: mikey, anton
In-Reply-To: <1418020212-4303-4-git-send-email-khandual@linux.vnet.ibm.com>
On Mon, 2014-08-12 at 06:30:11 UTC, Anshuman Khandual wrote:
> This patch adds some in-code documentation to the DSCR related
> code to make it more readable without having any functional
> change to it.
Adding documentation is always good, but ...
> diff --git a/arch/powerpc/include/asm/processor.h b/arch/powerpc/include/asm/processor.h
> index dda7ac4..81c1aeb 100644
> --- a/arch/powerpc/include/asm/processor.h
> +++ b/arch/powerpc/include/asm/processor.h
> @@ -295,6 +295,14 @@ struct thread_struct {
> #endif
> #ifdef CONFIG_PPC64
> unsigned long dscr;
> + /*
> + * XXX: dscr_inherit indicates that the process has explicitly
Please don't use XXX as a matter of practice.
It should be saved for *really* tricky/complicated code, and this isn't that.
> diff --git a/arch/powerpc/kernel/sysfs.c b/arch/powerpc/kernel/sysfs.c
> index 67fd2fd..edde3f0 100644
> --- a/arch/powerpc/kernel/sysfs.c
> +++ b/arch/powerpc/kernel/sysfs.c
> @@ -496,8 +496,21 @@ static DEVICE_ATTR(spurr, 0400, show_spurr, NULL);
> static DEVICE_ATTR(purr, 0400, show_purr, store_purr);
> static DEVICE_ATTR(pir, 0400, show_pir, NULL);
>
> +/*
> + * XXX: This is the system wide DSCR register default value.
> + * Any change to this value through the sysfs interface will
> + * update all per-cpu DSCR default values across the system
> + * stored in their respective PACA structures.
> + */
> static unsigned long dscr_default;
Yeah it seems you're right, writing updates the values in all pacas, reading
returns the value in the current cpu's paca. So why do we need this copy of the
value?
> +/*
> + * XXX: read_dscr and write_dscr are the functions for the
> + * per-cpu DSCR default sysfs files present for each cpu.
> + * Though updates to per-cpu DSCR value also gets called
> + * for all the CPUs on the system when the system wide
> + * global dscr_default gets changed.
> + */
> static void read_dscr(void *val)
> {
Please make these proper kernel-doc comments. I've definitely asked you to do
that at least once before on a different patch, to check you can do:
$ ./scripts/kernel-doc -text arch/powerpc/kernel/sysfs.c
The comments for write_dscr() should be attached to that function.
cheers
^ permalink raw reply
* Re: powerpc32: missing accessors to pgprot_t objects
From: Michael Ellerman @ 2014-12-09 10:06 UTC (permalink / raw)
To: LEROY Christophe, Benjamin Herrenschmidt, Paul Mackerras,
scottwood
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <20141208141629.912861A5D5B@localhost.localdomain>
On Mon, 2014-08-12 at 14:16:29 UTC, LEROY Christophe wrote:
> Compilation with #define STRICT_MM_TYPECHECKS in arch/powerpc/include/asm/page.h
> fails due to missing use of pgprot_val() when using pgprot_t objects.
Any idea when this broke? Recently, or has it gone unnoticed for a long time?
cheers
^ permalink raw reply
* Re: [1/5] powerpc: Fix handling of DSCR related facility unavailable exception
From: Michael Ellerman @ 2014-12-09 10:11 UTC (permalink / raw)
To: Anshuman Khandual, linuxppc-dev; +Cc: mikey, anton
In-Reply-To: <1418020212-4303-1-git-send-email-khandual@linux.vnet.ibm.com>
On Mon, 2014-08-12 at 06:30:08 UTC, Anshuman Khandual wrote:
> Currently DSCR (Data Stream Control Register) can be accessed with
> mfspr or mtspr instructions inside a thread via two different SPR
> numbers. One being the user accessible problem state SPR number 0x03
> and the other being the privilege state SPR number 0x11. All access
> through the privilege state SPR number get emulated through illegal
> instruction exception. Any access through the problem state SPR number
> raises one facility unavailable exception which sets the thread based
> dscr_inherit bit and enables DSCR facility through FSCR register thus
> allowing direct access to DSCR without going through this exception in
> the future. We set the thread.dscr_inherit bit whether the access was
> with mfspr or mtspr instruction which is neither correct nor does it
> match the behaviour through the instruction emulation code path driven
> from privilege state SPR number. User currently observes two different
> kind of behaviour when accessing the DSCR through these two SPR numbers.
> This problem can be observed through these two test cases by replacing
> the privilege state SPR number with the problem state SPR number.
>
> (1) http://ozlabs.org/~anton/junkcode/dscr_default_test.c
> (2) http://ozlabs.org/~anton/junkcode/dscr_explicit_test.c
Can you convert those into a selftest please?
cheers
^ permalink raw reply
* Re: powerpc32: missing accessors to pgprot_t objects
From: Benjamin Herrenschmidt @ 2014-12-09 10:13 UTC (permalink / raw)
To: Michael Ellerman; +Cc: scottwood, Paul Mackerras, linuxppc-dev, linux-kernel
In-Reply-To: <20141209100617.630111400EA@ozlabs.org>
On Tue, 2014-12-09 at 21:06 +1100, Michael Ellerman wrote:
> On Mon, 2014-08-12 at 14:16:29 UTC, LEROY Christophe wrote:
> > Compilation with #define STRICT_MM_TYPECHECKS in arch/powerpc/include/asm/page.h
> > fails due to missing use of pgprot_val() when using pgprot_t objects.
>
> Any idea when this broke? Recently, or has it gone unnoticed for a long time?
Probably a very long time...
Now the reason we didn't leave STRICT_MM_TYPECHECKS enable back in the
day is that gcc was doing a terrible job at compiling it resulting in
bloated inefficient code.
I wouldn't be surprised if that is all fixed...
Cheers,
Ben.
^ permalink raw reply
* Re: powerpc32: missing accessors to pgprot_t objects
From: Michael Ellerman @ 2014-12-09 10:18 UTC (permalink / raw)
To: LEROY Christophe, Benjamin Herrenschmidt, Paul Mackerras,
scottwood, aneesh.kumar
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <20141208141629.912861A5D5B@localhost.localdomain>
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 1371 bytes --]
On Mon, 2014-08-12 at 14:16:29 UTC, LEROY Christophe wrote:
> Compilation with #define STRICT_MM_TYPECHECKS in arch/powerpc/include/asm/page.h
> fails due to missing use of pgprot_val() when using pgprot_t objects.
Hmm, looks like 64 bit doesn't build either.
Aneesh I think you added this, mind fixing it up?
In file included from ../arch/powerpc/include/asm/thread_info.h:34:0,
from ../include/linux/thread_info.h:54,
from ../include/asm-generic/preempt.h:4,
from arch/powerpc/include/generated/asm/preempt.h:1,
from ../include/linux/preempt.h:18,
from ../include/linux/spinlock.h:50,
from ../include/linux/mmzone.h:7,
from ../include/linux/gfp.h:5,
from ../include/linux/mm.h:9,
from ../arch/powerpc/mm/tlb_hash64.c:25:
../arch/powerpc/mm/tlb_hash64.c: In function ‘__flush_hash_table_range’:
../arch/powerpc/include/asm/page.h:286:24: error: request for member ‘pte’ in something not a structure or union
#define pte_val(x) ((x).pte)
^
../arch/powerpc/mm/tlb_hash64.c:219:37: note: in expansion of macro ‘pte_val’
trace_hugepage_invalidate(start, pte_val(pte));
^
make[2]: *** [arch/powerpc/mm/tlb_hash64.o] Error 1
cheers
^ permalink raw reply
* Re: [v3] i2c: Driver to expose PowerNV platform i2c busses
From: Benjamin Herrenschmidt @ 2014-12-09 10:21 UTC (permalink / raw)
To: Michael Ellerman
Cc: devicetree, wsa, linuxppc-dev, linux-i2c, Neelesh Gupta,
Jeremy Kerr
In-Reply-To: <20141209094503.100F81400DE@ozlabs.org>
On Tue, 2014-12-09 at 20:45 +1100, Michael Ellerman wrote:
> On Mon, 2014-08-12 at 06:36:16 UTC, Neelesh Gupta wrote:
> > The patch exposes the available i2c busses on the PowerNV platform
> > to the kernel and implements the bus driver to support i2c and
> > smbus commands.
> > The driver uses the platform device infrastructure to probe the busses
> > on the platform and registers them with the i2c driver framework.
>
> > arch/powerpc/include/asm/opal.h | 29 ++
> > arch/powerpc/platforms/powernv/opal-wrappers.S | 1
> > arch/powerpc/platforms/powernv/opal.c | 11 +
>
> This had major conflicts in the above files.
>
> I've fixed it up but please check I did it correctly:
>
> https://github.com/mpe/powerpc-merge/commits/pw/neelesh-i2c
>
> It also doesn't build as a module:
>
> $ grep CONFIG_I2C_OPAL .config
> CONFIG_I2C_OPAL=m
> $ make ...
> ...
> ERROR: "opal_i2c_request" [drivers/i2c/busses/i2c-opal.ko] undefined!
Right that needs to be exported, however for those OPAL wrappers to work
when exported as modules on an LE kernel with ABI v2 (pfiew !) we need
another patch from jk to sort out the external entry to the asm...
Jeremy, is that already in or not yet ?
Cheers,
Ben.
> cheers
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [4/5] powerpc, dscr: Added some in-code documentation
From: Anshuman Khandual @ 2014-12-09 13:03 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev; +Cc: mikey, anton
In-Reply-To: <20141209100307.ACBE41400DE@ozlabs.org>
On 12/09/2014 03:33 PM, Michael Ellerman wrote:
> On Mon, 2014-08-12 at 06:30:11 UTC, Anshuman Khandual wrote:
>> This patch adds some in-code documentation to the DSCR related
>> code to make it more readable without having any functional
>> change to it.
>
> Adding documentation is always good, but ...
>
>> diff --git a/arch/powerpc/include/asm/processor.h b/arch/powerpc/include/asm/processor.h
>> index dda7ac4..81c1aeb 100644
>> --- a/arch/powerpc/include/asm/processor.h
>> +++ b/arch/powerpc/include/asm/processor.h
>> @@ -295,6 +295,14 @@ struct thread_struct {
>> #endif
>> #ifdef CONFIG_PPC64
>> unsigned long dscr;
>> + /*
>> + * XXX: dscr_inherit indicates that the process has explicitly
>
> Please don't use XXX as a matter of practice.
>
> It should be saved for *really* tricky/complicated code, and this isn't that.
Sure, got it. Will remove them.
>
>> diff --git a/arch/powerpc/kernel/sysfs.c b/arch/powerpc/kernel/sysfs.c
>> index 67fd2fd..edde3f0 100644
>> --- a/arch/powerpc/kernel/sysfs.c
>> +++ b/arch/powerpc/kernel/sysfs.c
>> @@ -496,8 +496,21 @@ static DEVICE_ATTR(spurr, 0400, show_spurr, NULL);
>> static DEVICE_ATTR(purr, 0400, show_purr, store_purr);
>> static DEVICE_ATTR(pir, 0400, show_pir, NULL);
>>
>> +/*
>> + * XXX: This is the system wide DSCR register default value.
>> + * Any change to this value through the sysfs interface will
>> + * update all per-cpu DSCR default values across the system
>> + * stored in their respective PACA structures.
>> + */
>> static unsigned long dscr_default;
>
> Yeah it seems you're right, writing updates the values in all pacas, reading
> returns the value in the current cpu's paca. So why do we need this copy of the
> value?
My comment here might be little confusing. The read_dscr/write_dscr functions
are used for per-CPU PACA DSCR values which can read/update the per-CPU PACA
variable directly. The functions show_dscr_default/store_dscr_default are used
to read/update the system wide DSCR default which is the above 'dscr_default'
variable. Function store_dscr_default also calls write_dscr to update PACA on
every CPU present on the system. I will re-write the code comment to make more
sense.
>
>> +/*
>> + * XXX: read_dscr and write_dscr are the functions for the
>> + * per-cpu DSCR default sysfs files present for each cpu.
>> + * Though updates to per-cpu DSCR value also gets called
>> + * for all the CPUs on the system when the system wide
>> + * global dscr_default gets changed.
>> + */
>> static void read_dscr(void *val)
>> {
>
> Please make these proper kernel-doc comments. I've definitely asked you to do
> that at least once before on a different patch, to check you can do:
Yes you had. Thought that this function is too small but as you said if we
are writing documentation for it we should write kernel-doc format only.
Will make sure about this now onward.
>
> $ ./scripts/kernel-doc -text arch/powerpc/kernel/sysfs.c
>
> The comments for write_dscr() should be attached to that function.
Sure.
^ permalink raw reply
* Re: [1/5] powerpc: Fix handling of DSCR related facility unavailable exception
From: Anshuman Khandual @ 2014-12-09 13:15 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev; +Cc: mikey, anton
In-Reply-To: <20141209101117.1CEC9140100@ozlabs.org>
On 12/09/2014 03:41 PM, Michael Ellerman wrote:
> On Mon, 2014-08-12 at 06:30:08 UTC, Anshuman Khandual wrote:
>> Currently DSCR (Data Stream Control Register) can be accessed with
>> mfspr or mtspr instructions inside a thread via two different SPR
>> numbers. One being the user accessible problem state SPR number 0x03
>> and the other being the privilege state SPR number 0x11. All access
>> through the privilege state SPR number get emulated through illegal
>> instruction exception. Any access through the problem state SPR number
>> raises one facility unavailable exception which sets the thread based
>> dscr_inherit bit and enables DSCR facility through FSCR register thus
>> allowing direct access to DSCR without going through this exception in
>> the future. We set the thread.dscr_inherit bit whether the access was
>> with mfspr or mtspr instruction which is neither correct nor does it
>> match the behaviour through the instruction emulation code path driven
>> from privilege state SPR number. User currently observes two different
>> kind of behaviour when accessing the DSCR through these two SPR numbers.
>> This problem can be observed through these two test cases by replacing
>> the privilege state SPR number with the problem state SPR number.
>>
>> (1) http://ozlabs.org/~anton/junkcode/dscr_default_test.c
>> (2) http://ozlabs.org/~anton/junkcode/dscr_explicit_test.c
>
> Can you convert those into a selftest please?
Hey Michael,
Yeah I wanted to convert all these tests which are related to DSCR into
individual self tests for powerpc. All these test cases have Anton Blanchard
and IBM's copyright on it but they are licensed with GPL V2. Not sure whether
Anton needs to okay this before I can modify them for self tests, put his
(Anton's) signed-off-by and then post it in the mailing list.
(1) http://ozlabs.org/~anton/junkcode/dscr_default_test.c
(2) http://ozlabs.org/~anton/junkcode/dscr_explicit_test.c
(3) http://ozlabs.org/~anton/junkcode/dscr_inherit_exec_test.c
(4) http://ozlabs.org/~anton/junkcode/dscr_inherit_test.c
(5) http://ozlabs.org/~anton/junkcode/user_dscr_test.c
^ permalink raw reply
* Re: [v3] i2c: Driver to expose PowerNV platform i2c busses
From: Neelesh Gupta @ 2014-12-09 13:18 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev, devicetree, linux-i2c, wsa
In-Reply-To: <20141209094503.100F81400DE@ozlabs.org>
On 12/09/2014 03:15 PM, Michael Ellerman wrote:
> On Mon, 2014-08-12 at 06:36:16 UTC, Neelesh Gupta wrote:
>> The patch exposes the available i2c busses on the PowerNV platform
>> to the kernel and implements the bus driver to support i2c and
>> smbus commands.
>> The driver uses the platform device infrastructure to probe the busses
>> on the platform and registers them with the i2c driver framework.
>> arch/powerpc/include/asm/opal.h | 29 ++
>> arch/powerpc/platforms/powernv/opal-wrappers.S | 1
>> arch/powerpc/platforms/powernv/opal.c | 11 +
> This had major conflicts in the above files.
>
> I've fixed it up but please check I did it correctly:
>
> https://github.com/mpe/powerpc-merge/commits/pw/neelesh-i2c
>
> It also doesn't build as a module:
>
> $ grep CONFIG_I2C_OPAL .config
> CONFIG_I2C_OPAL=m
> $ make ...
> ...
> ERROR: "opal_i2c_request" [drivers/i2c/busses/i2c-opal.ko] undefined!
Yeah, this needs to be exported.
I think for the same reason, "opal-rtc" is still waiting, plus awaiting
"ack" from
maintainers.
- Neelesh
>
> cheers
>
^ permalink raw reply
* Re: powerpc32: missing accessors to pgprot_t objects
From: Aneesh Kumar K.V @ 2014-12-09 15:06 UTC (permalink / raw)
To: Michael Ellerman, LEROY Christophe, Benjamin Herrenschmidt,
Paul Mackerras, scottwood
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <20141209101841.7D9241400D2@ozlabs.org>
Michael Ellerman <mpe@ellerman.id.au> writes:
> On Mon, 2014-08-12 at 14:16:29 UTC, LEROY Christophe wrote:
>> Compilation with #define STRICT_MM_TYPECHECKS in arch/powerpc/include/as=
m/page.h
>> fails due to missing use of pgprot_val() when using pgprot_t objects.
>
> Hmm, looks like 64 bit doesn't build either.
>
> Aneesh I think you added this, mind fixing it up?
>
> In file included from ../arch/powerpc/include/asm/thread_info.h:34:0,
> from ../include/linux/thread_info.h:54,
> from ../include/asm-generic/preempt.h:4,
> from arch/powerpc/include/generated/asm/preempt.h:1,
> from ../include/linux/preempt.h:18,
> from ../include/linux/spinlock.h:50,
> from ../include/linux/mmzone.h:7,
> from ../include/linux/gfp.h:5,
> from ../include/linux/mm.h:9,
> from ../arch/powerpc/mm/tlb_hash64.c:25:
> ../arch/powerpc/mm/tlb_hash64.c: In function =E2=80=98__flush_hash_table_=
range=E2=80=99:
> ../arch/powerpc/include/asm/page.h:286:24: error: request for member =E2=
=80=98pte=E2=80=99 in something not a structure or union
> #define pte_val(x) ((x).pte)
> ^=20=20=20=20
> ../arch/powerpc/mm/tlb_hash64.c:219:37: note: in expansion of macro =E2=
=80=98pte_val=E2=80=99
> trace_hugepage_invalidate(start, pte_val(pte));
> ^=20=20=20=20
> make[2]: *** [arch/powerpc/mm/tlb_hash64.o] Error 1
>
Will send a proper patch after compile testing with other configs. The
kvm hunk is really ugly, will try to rework.=20
diff --git a/arch/powerpc/include/asm/kvm_book3s_64.h b/arch/powerpc/includ=
e/asm/kvm_book3s_64.h
index 0aa8179..cd0ff37 100644
--- a/arch/powerpc/include/asm/kvm_book3s_64.h
+++ b/arch/powerpc/include/asm/kvm_book3s_64.h
@@ -291,11 +291,11 @@ static inline pte_t kvmppc_read_update_linux_pte(pte_=
t *ptep, int writing,
pte_t old_pte, new_pte =3D __pte(0);
=20
while (1) {
- old_pte =3D pte_val(*ptep);
+ old_pte =3D *ptep;
/*
* wait until _PAGE_BUSY is clear then set it atomically
*/
- if (unlikely(old_pte & _PAGE_BUSY)) {
+ if (unlikely(pte_val(old_pte) & _PAGE_BUSY)) {
cpu_relax();
continue;
}
@@ -306,16 +306,18 @@ static inline pte_t kvmppc_read_update_linux_pte(pte_=
t *ptep, int writing,
return __pte(0);
#endif
/* If pte is not present return None */
- if (unlikely(!(old_pte & _PAGE_PRESENT)))
+ if (unlikely(!(pte_val(old_pte) & _PAGE_PRESENT)))
return __pte(0);
=20
new_pte =3D pte_mkyoung(old_pte);
if (writing && pte_write(old_pte))
new_pte =3D pte_mkdirty(new_pte);
=20
- if (old_pte =3D=3D __cmpxchg_u64((unsigned long *)ptep, old_pte,
- new_pte))
+ if (pte_val(old_pte) =3D=3D __cmpxchg_u64((unsigned long *)ptep,
+ pte_val(old_pte),
+ pte_val(new_pte))) {
break;
+ }
}
return new_pte;
}
diff --git a/arch/powerpc/include/asm/page.h b/arch/powerpc/include/asm/pag=
e.h
index 26fe1ae..cc62ab9 100644
--- a/arch/powerpc/include/asm/page.h
+++ b/arch/powerpc/include/asm/page.h
@@ -278,7 +278,7 @@ extern long long virt_phys_offset;
=20
#ifndef __ASSEMBLY__
=20
-#undef STRICT_MM_TYPECHECKS
+#define STRICT_MM_TYPECHECKS 1
=20
#ifdef STRICT_MM_TYPECHECKS
/* These are used to make use of C type-checking. */
diff --git a/arch/powerpc/include/asm/pgtable.h b/arch/powerpc/include/asm/=
pgtable.h
index 316f9a5..3e29088 100644
--- a/arch/powerpc/include/asm/pgtable.h
+++ b/arch/powerpc/include/asm/pgtable.h
@@ -77,8 +77,8 @@ static inline void pmdp_set_numa(struct mm_struct *mm, un=
signed long addr,
* which was inherited from x86. For the purposes of powerpc pte_basic_t a=
nd
* pmd_t are equivalent
*/
-#define pteval_t pte_basic_t
-#define pmdval_t pmd_t
+typedef unsigned long pteval_t;
+typedef unsigned long pmdval_t;
static inline pteval_t ptenuma_flags(pte_t pte)
{
return pte_val(pte) & _PAGE_NUMA_MASK;
diff --git a/arch/powerpc/mm/pgtable_64.c b/arch/powerpc/mm/pgtable_64.c
index c8d709a..5162936 100644
--- a/arch/powerpc/mm/pgtable_64.c
+++ b/arch/powerpc/mm/pgtable_64.c
@@ -714,7 +714,7 @@ void set_pmd_at(struct mm_struct *mm, unsigned long add=
r,
assert_spin_locked(&mm->page_table_lock);
WARN_ON(!pmd_trans_huge(pmd));
#endif
- trace_hugepage_set_pmd(addr, pmd);
+ trace_hugepage_set_pmd(addr, pmd_val(pmd));
return set_pte_at(mm, addr, pmdp_ptep(pmdp), pmd_pte(pmd));
}
=20
diff --git a/arch/powerpc/mm/tlb_hash64.c b/arch/powerpc/mm/tlb_hash64.c
index d2a94b8..c522969 100644
--- a/arch/powerpc/mm/tlb_hash64.c
+++ b/arch/powerpc/mm/tlb_hash64.c
@@ -216,7 +216,7 @@ void __flush_hash_table_range(struct mm_struct *mm, uns=
igned long start,
continue;
pte =3D pte_val(*ptep);
if (hugepage_shift)
- trace_hugepage_invalidate(start, pte_val(pte));
+ trace_hugepage_invalidate(start, pte);
if (!(pte & _PAGE_HASHPTE))
continue;
if (unlikely(hugepage_shift && pmd_trans_huge(*(pmd_t *)pte)))
^ permalink raw reply related
* [PATCH 1/1] uio: uio_fsl_elbc_gpcm: new driver
From: John Ogness @ 2014-12-09 16:43 UTC (permalink / raw)
To: linux-kernel
Cc: devicetree, Shaohui.Xie, dongsheng.wang, robh+dt, scottwood, hjk,
paulus, gregkh, grant.likely, linuxppc-dev
This driver provides UIO access to memory of a peripheral connected
to the Freescale enhanced local bus controller (eLBC) interface
using the general purpose chip-select mode (GPCM).
Signed-off-by: John Ogness <john.ogness@linutronix.de>
---
There are currently drivers that use FCM and UPM modes. But there
are no drivers using the very simple GPCM mode. If other drivers
from other subsystems should start using this mode (for example,
mtd, eeprom, char) then it may make sense to try to implement
generic GPCM support in arch/powerpc/sysdev/fsl_lbc.c. There
would not be much overlap, but there is no point in having
multiple drivers implement bank and OR/BR validation.
As for the device-specific netX support, I considered if a common
header file with the PCI variant drivers/uio/uio_netx.c should
exist. Merging the two drivers didn't seem to make sense since
this is actually a generic eLBC-GPCM driver (that optionally
supports netX) and the other is a PCI netX driver.
.../devicetree/bindings/powerpc/fsl/lbc.txt | 18 +-
arch/powerpc/include/asm/fsl_lbc.h | 3 +
drivers/uio/Kconfig | 20 +
drivers/uio/Makefile | 1 +
drivers/uio/uio_fsl_elbc_gpcm.c | 499 ++++++++++++++++++++
5 files changed, 536 insertions(+), 5 deletions(-)
create mode 100644 drivers/uio/uio_fsl_elbc_gpcm.c
diff --git a/Documentation/devicetree/bindings/powerpc/fsl/lbc.txt b/Documentation/devicetree/bindings/powerpc/fsl/lbc.txt
index 3300fec..1c80fce 100644
--- a/Documentation/devicetree/bindings/powerpc/fsl/lbc.txt
+++ b/Documentation/devicetree/bindings/powerpc/fsl/lbc.txt
@@ -16,20 +16,28 @@ Example:
"fsl,pq2-localbus";
#address-cells = <2>;
#size-cells = <1>;
- reg = <f0010100 40>;
+ reg = <0xf0010100 0x40>;
- ranges = <0 0 fe000000 02000000
- 1 0 f4500000 00008000>;
+ ranges = <0x0 0x0 0xfe000000 0x02000000
+ 0x1 0x0 0xf4500000 0x00008000
+ 0x2 0x0 0xfd810000 0x00010000>;
flash@0,0 {
compatible = "jedec-flash";
- reg = <0 0 2000000>;
+ reg = <0x0 0x0 0x2000000>;
bank-width = <4>;
device-width = <1>;
};
board-control@1,0 {
- reg = <1 0 20>;
+ reg = <0x1 0x0 0x20>;
compatible = "fsl,mpc8272ads-bcsr";
};
+
+ simple-periph@2,0 {
+ compatible = "fsl,elbc-gpcm-uio";
+ reg = <0x2 0x0 0x10000>;
+ elbc-gpcm-br = <0xfd810800>;
+ elbc-gpcm-or = <0xffff09f7>;
+ };
};
diff --git a/arch/powerpc/include/asm/fsl_lbc.h b/arch/powerpc/include/asm/fsl_lbc.h
index 067fb0d..c7240a0 100644
--- a/arch/powerpc/include/asm/fsl_lbc.h
+++ b/arch/powerpc/include/asm/fsl_lbc.h
@@ -95,6 +95,9 @@ struct fsl_lbc_bank {
#define OR_FCM_TRLX_SHIFT 2
#define OR_FCM_EHTR 0x00000002
#define OR_FCM_EHTR_SHIFT 1
+
+#define OR_GPCM_AM 0xFFFF8000
+#define OR_GPCM_AM_SHIFT 15
};
struct fsl_lbc_regs {
diff --git a/drivers/uio/Kconfig b/drivers/uio/Kconfig
index 5a90914..8a15c32 100644
--- a/drivers/uio/Kconfig
+++ b/drivers/uio/Kconfig
@@ -104,6 +104,26 @@ config UIO_NETX
To compile this driver as a module, choose M here; the module
will be called uio_netx.
+config UIO_FSL_ELBC_GPCM
+ tristate "eLBC/GPCM driver"
+ depends on FSL_LBC
+ help
+ Generic driver for accessing a peripheral connected to an eLBC port
+ that is running in GPCM mode. GPCM is an interface for simple lower
+ performance memories and memory-mapped devices. For devices using
+ FCM or UPM eLBC modes, other device-specific drivers are available.
+
+config UIO_FSL_ELBC_GPCM_NETX5152
+ bool "eLBC/GPCM netX 51/52 support"
+ depends on UIO_FSL_ELBC_GPCM
+ help
+ This will add support for netX 51/52 devices connected via eLBC/GPCM.
+ In particular, it implements interrupt handling. This can be used
+ together with the userspace netX stack from Hilscher.
+
+ Information about this hardware can be found at:
+ http://www.hilscher.com/netx
+
config UIO_PRUSS
tristate "Texas Instruments PRUSS driver"
depends on ARCH_DAVINCI_DA850
diff --git a/drivers/uio/Makefile b/drivers/uio/Makefile
index d3218bd..8560dad 100644
--- a/drivers/uio/Makefile
+++ b/drivers/uio/Makefile
@@ -8,3 +8,4 @@ obj-$(CONFIG_UIO_PCI_GENERIC) += uio_pci_generic.o
obj-$(CONFIG_UIO_NETX) += uio_netx.o
obj-$(CONFIG_UIO_PRUSS) += uio_pruss.o
obj-$(CONFIG_UIO_MF624) += uio_mf624.o
+obj-$(CONFIG_UIO_FSL_ELBC_GPCM) += uio_fsl_elbc_gpcm.o
diff --git a/drivers/uio/uio_fsl_elbc_gpcm.c b/drivers/uio/uio_fsl_elbc_gpcm.c
new file mode 100644
index 0000000..b6cac91
--- /dev/null
+++ b/drivers/uio/uio_fsl_elbc_gpcm.c
@@ -0,0 +1,499 @@
+/* uio_fsl_elbc_gpcm: UIO driver for eLBC/GPCM peripherals
+
+ Copyright (C) 2014 Linutronix GmbH
+ Author: John Ogness <john.ogness@linutronix.de>
+
+ This driver provides UIO access to memory of a peripheral connected
+ to the Freescale enhanced local bus controller (eLBC) interface
+ using the general purpose chip-select mode (GPCM).
+
+ Here is an example of the device tree entries:
+
+ localbus@ffe05000 {
+ ranges = <0x2 0x0 0x0 0xff810000 0x10000>;
+
+ dpm@2,0 {
+ compatible = "fsl,elbc-gpcm-uio";
+ reg = <0x2 0x0 0x10000>;
+ elbc-gpcm-br = <0xff810800>;
+ elbc-gpcm-or = <0xffff09f7>;
+ interrupt-parent = <&mpic>;
+ interrupts = <4 1>;
+ device_type = "netx5152";
+ uio_name = "netx_custom";
+ netx5152,init-win0-offset = <0x0>;
+ };
+ };
+
+ Only the entries reg (to identify bank) and elbc-gpcm-* (initial BR/OR
+ values) are required. The entries interrupt*, device_type, and uio_name
+ are optional (as well as any type-specific options such as
+ netx5152,init-win0-offset). As long as no interrupt handler is needed,
+ this driver can be used without any type-specific implementation.
+
+ The netx5152 type has been tested to work with the netX 51/52 hardware
+ from Hilscher using the Hilscher userspace netX stack.
+
+ The netx5152 type should serve as a model to add new type-specific
+ devices as needed.
+*/
+
+#include <linux/module.h>
+#include <linux/device.h>
+#include <linux/string.h>
+#include <linux/slab.h>
+#include <linux/platform_device.h>
+#include <linux/uio_driver.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+
+#include <asm/fsl_lbc.h>
+
+#define MAX_BANKS 8
+
+struct fsl_elbc_gpcm {
+ struct device *dev;
+ struct fsl_lbc_regs __iomem *lbc;
+ u32 bank;
+ const char *name;
+
+ void (*init)(struct uio_info *info);
+ void (*shutdown)(struct uio_info *info, bool init_err);
+ irqreturn_t (*irq_handler)(int irq, struct uio_info *info);
+};
+
+static ssize_t reg_show(struct device *dev, struct device_attribute *attr,
+ char *buf);
+static ssize_t reg_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count);
+
+DEVICE_ATTR(reg_br, S_IRUGO|S_IWUSR|S_IWGRP, reg_show, reg_store);
+DEVICE_ATTR(reg_or, S_IRUGO|S_IWUSR|S_IWGRP, reg_show, reg_store);
+
+static ssize_t reg_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct platform_device *pdev = to_platform_device(dev);
+ struct uio_info *info = platform_get_drvdata(pdev);
+ struct fsl_elbc_gpcm *priv = info->priv;
+ struct fsl_lbc_bank *bank = &priv->lbc->bank[priv->bank];
+
+ if (attr == &dev_attr_reg_br) {
+ return scnprintf(buf, PAGE_SIZE, "0x%08x\n",
+ in_be32(&bank->br));
+
+ } else if (attr == &dev_attr_reg_or) {
+ return scnprintf(buf, PAGE_SIZE, "0x%08x\n",
+ in_be32(&bank->or));
+ }
+
+ return 0;
+}
+
+static ssize_t reg_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct platform_device *pdev = to_platform_device(dev);
+ struct uio_info *info = platform_get_drvdata(pdev);
+ struct fsl_elbc_gpcm *priv = info->priv;
+ struct fsl_lbc_bank *bank = &priv->lbc->bank[priv->bank];
+ unsigned long val;
+ u32 reg_br_cur;
+ u32 reg_or_cur;
+ u32 reg_new;
+
+ /* parse use input */
+ if (kstrtoul(buf, 0, &val) != 0)
+ return -EINVAL;
+ reg_new = (u32)val;
+
+ /* read current values */
+ reg_br_cur = in_be32(&bank->br);
+ reg_or_cur = in_be32(&bank->or);
+
+ if (attr == &dev_attr_reg_br) {
+ /* not allowed to change effective base address */
+ if ((reg_br_cur & reg_or_cur & BR_BA) !=
+ (reg_new & reg_or_cur & BR_BA)) {
+ return -EINVAL;
+ }
+
+ /* not allowed to change mode */
+ if ((reg_new & BR_MSEL) != BR_MS_GPCM)
+ return -EINVAL;
+
+ /* write new value (force valid) */
+ out_be32(&bank->br, reg_new | BR_V);
+
+ } else if (attr == &dev_attr_reg_or) {
+ /* not allowed to change access mask */
+ if ((reg_or_cur & OR_GPCM_AM) != (reg_new & OR_GPCM_AM))
+ return -EINVAL;
+
+ /* write new value */
+ out_be32(&bank->or, reg_new);
+
+ } else {
+ return -EINVAL;
+ }
+
+ return count;
+}
+
+#ifdef CONFIG_UIO_FSL_ELBC_GPCM_NETX5152
+#define DPM_HOST_WIN0_OFFSET 0xff00
+#define DPM_HOST_INT_STAT0 0xe0
+#define DPM_HOST_INT_EN0 0xf0
+#define DPM_HOST_INT_MASK 0xe600ffff
+#define DPM_HOST_INT_GLOBAL_EN 0x80000000
+
+static irqreturn_t netx5152_irq_handler(int irq, struct uio_info *info)
+{
+ void __iomem *reg_int_en = info->mem[0].internal_addr +
+ DPM_HOST_WIN0_OFFSET +
+ DPM_HOST_INT_EN0;
+ void __iomem *reg_int_stat = info->mem[0].internal_addr +
+ DPM_HOST_WIN0_OFFSET +
+ DPM_HOST_INT_STAT0;
+
+ /* check if an interrupt is enabled and active */
+ if ((ioread32(reg_int_en) & ioread32(reg_int_stat) &
+ DPM_HOST_INT_MASK) == 0) {
+ return IRQ_NONE;
+ }
+
+ /* disable interrupts */
+ iowrite32(ioread32(reg_int_en) & ~DPM_HOST_INT_GLOBAL_EN, reg_int_en);
+
+ return IRQ_HANDLED;
+}
+
+static void netx5152_init(struct uio_info *info)
+{
+ unsigned long win0_offset = DPM_HOST_WIN0_OFFSET;
+ struct fsl_elbc_gpcm *priv = info->priv;
+ const void *prop;
+
+ /* get an optional initial win0 offset */
+ prop = of_get_property(priv->dev->of_node,
+ "netx5152,init-win0-offset", NULL);
+ if (prop)
+ win0_offset = of_read_ulong(prop, 1);
+
+ /* disable interrupts */
+ iowrite32(0, info->mem[0].internal_addr + win0_offset +
+ DPM_HOST_INT_EN0);
+}
+
+static void netx5152_shutdown(struct uio_info *info, bool init_err)
+{
+ if (init_err)
+ return;
+
+ /* disable interrupts */
+ iowrite32(0, info->mem[0].internal_addr + DPM_HOST_WIN0_OFFSET +
+ DPM_HOST_INT_EN0);
+}
+#endif
+
+static void setup_periph(struct fsl_elbc_gpcm *priv,
+ const char *type)
+{
+#ifdef CONFIG_UIO_FSL_ELBC_GPCM_NETX5152
+ if (strcmp(type, "netx5152") == 0) {
+ priv->irq_handler = netx5152_irq_handler;
+ priv->init = netx5152_init;
+ priv->shutdown = netx5152_shutdown;
+ priv->name = "netX 51/52";
+ return;
+ }
+#endif
+}
+
+static int check_of_data(struct fsl_elbc_gpcm *priv,
+ struct resource *res,
+ u32 reg_br, u32 reg_or)
+{
+ /* check specified bank */
+ if (priv->bank >= MAX_BANKS) {
+ dev_err(priv->dev, "invalid bank\n");
+ return -ENODEV;
+ }
+
+ /* check specified mode (BR_MS_GPCM is 0) */
+ if ((reg_br & BR_MSEL) != BR_MS_GPCM) {
+ dev_err(priv->dev, "unsupported mode\n");
+ return -ENODEV;
+ }
+
+ /* check specified mask vs. resource size */
+ if ((~(reg_or & OR_GPCM_AM) + 1) != resource_size(res)) {
+ dev_err(priv->dev, "address mask / size mismatch\n");
+ return -ENODEV;
+ }
+
+ /* check specified address */
+ if ((reg_br & reg_or & BR_BA) != fsl_lbc_addr(res->start)) {
+ dev_err(priv->dev, "base address mismatch\n");
+ return -ENODEV;
+ }
+
+ return 0;
+}
+
+static int get_of_data(struct fsl_elbc_gpcm *priv, struct device_node *node,
+ struct resource *res, u32 *reg_br,
+ u32 *reg_or, unsigned int *irq, char **name)
+{
+ const char *dt_name;
+ const char *type;
+ int ret;
+
+ /* get the memory resource */
+ ret = of_address_to_resource(node, 0, res);
+ if (ret) {
+ dev_err(priv->dev, "failed to get resource\n");
+ return ret;
+ }
+
+ /* get the bank number */
+ ret = of_property_read_u32(node, "reg", &priv->bank);
+ if (ret) {
+ dev_err(priv->dev, "failed to get bank number\n");
+ return ret;
+ }
+
+ /* get BR value to set */
+ ret = of_property_read_u32(node, "elbc-gpcm-br", reg_br);
+ if (ret) {
+ dev_err(priv->dev, "missing elbc-gpcm-br value\n");
+ return ret;
+ }
+
+ /* get OR value to set */
+ ret = of_property_read_u32(node, "elbc-gpcm-or", reg_or);
+ if (ret) {
+ dev_err(priv->dev, "missing elbc-gpcm-or value\n");
+ return ret;
+ }
+
+ /* get optional peripheral type */
+ priv->name = "generic";
+ if (of_property_read_string(node, "device_type", &type) == 0)
+ setup_periph(priv, type);
+
+ /* get optional irq value */
+ *irq = irq_of_parse_and_map(node, 0);
+
+ /* sanity check device tree data */
+ ret = check_of_data(priv, res, *reg_br, *reg_or);
+ if (ret)
+ return ret;
+
+ /* get optional uio name */
+ if (of_property_read_string(node, "uio_name", &dt_name) != 0)
+ dt_name = "eLBC_GPCM";
+ *name = kstrdup(dt_name, GFP_KERNEL);
+ if (!*name)
+ return -ENOMEM;
+
+ return 0;
+}
+
+static int uio_fsl_elbc_gpcm_probe(struct platform_device *pdev)
+{
+ struct device_node *node = pdev->dev.of_node;
+ struct fsl_elbc_gpcm *priv;
+ struct uio_info *info;
+ char *uio_name = NULL;
+ struct resource res;
+ unsigned int irq;
+ u32 reg_br_cur;
+ u32 reg_or_cur;
+ u32 reg_br_new;
+ u32 reg_or_new;
+ int ret;
+
+ if (!fsl_lbc_ctrl_dev || !fsl_lbc_ctrl_dev->regs)
+ return -ENODEV;
+
+ /* allocate private data */
+ priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+ priv->dev = &pdev->dev;
+ priv->lbc = fsl_lbc_ctrl_dev->regs;
+
+ /* get device tree data */
+ ret = get_of_data(priv, node, &res, ®_br_new, ®_or_new,
+ &irq, &uio_name);
+ if (ret)
+ goto out_err0;
+
+ /* allocate UIO structure */
+ info = kzalloc(sizeof(*info), GFP_KERNEL);
+ if (!info) {
+ ret = -ENOMEM;
+ goto out_err0;
+ }
+
+ /* get current BR/OR values */
+ reg_br_cur = in_be32(&priv->lbc->bank[priv->bank].br);
+ reg_or_cur = in_be32(&priv->lbc->bank[priv->bank].or);
+
+ /* if bank already configured, make sure it matches */
+ if ((reg_br_cur & BR_V)) {
+ if ((reg_br_cur & BR_MSEL) != BR_MS_GPCM ||
+ (reg_br_cur & reg_or_cur & BR_BA)
+ != fsl_lbc_addr(res.start)) {
+ dev_err(priv->dev,
+ "bank in use by another peripheral\n");
+ ret = -ENODEV;
+ goto out_err1;
+ }
+
+ /* warn if behavior settings changing */
+ if ((reg_br_cur & ~(BR_BA | BR_V)) !=
+ (reg_br_new & ~(BR_BA | BR_V))) {
+ dev_warn(priv->dev,
+ "modifying BR settings: 0x%08x -> 0x%08x",
+ reg_br_cur, reg_br_new);
+ }
+ if ((reg_or_cur & ~OR_GPCM_AM) != (reg_or_new & ~OR_GPCM_AM)) {
+ dev_warn(priv->dev,
+ "modifying OR settings: 0x%08x -> 0x%08x",
+ reg_or_cur, reg_or_new);
+ }
+ }
+
+ /* configure the bank (force base address and GPCM) */
+ reg_br_new &= ~(BR_BA | BR_MSEL);
+ reg_br_new |= fsl_lbc_addr(res.start) | BR_MS_GPCM | BR_V;
+ out_be32(&priv->lbc->bank[priv->bank].or, reg_or_new);
+ out_be32(&priv->lbc->bank[priv->bank].br, reg_br_new);
+
+ /* map the memory resource */
+ info->mem[0].internal_addr = ioremap(res.start, resource_size(&res));
+ if (!info->mem[0].internal_addr) {
+ dev_err(priv->dev, "failed to map chip region\n");
+ ret = -ENODEV;
+ goto out_err1;
+ }
+
+ /* set all UIO data */
+ if (node->name)
+ info->mem[0].name = kstrdup(node->name, GFP_KERNEL);
+ info->mem[0].addr = res.start;
+ info->mem[0].size = resource_size(&res);
+ info->mem[0].memtype = UIO_MEM_PHYS;
+ info->priv = priv;
+ info->name = uio_name;
+ info->version = "0.0.1";
+ if (irq != NO_IRQ) {
+ if (priv->irq_handler) {
+ info->irq = irq;
+ info->irq_flags = IRQF_SHARED;
+ info->handler = priv->irq_handler;
+ } else {
+ irq = NO_IRQ;
+ dev_warn(priv->dev, "ignoring irq, no handler\n");
+ }
+ }
+
+ if (priv->init)
+ priv->init(info);
+
+ /* register UIO device */
+ if (uio_register_device(priv->dev, info) != 0) {
+ dev_err(priv->dev, "UIO registration failed\n");
+ ret = -ENODEV;
+ goto out_err2;
+ }
+
+ /* store private data */
+ platform_set_drvdata(pdev, info);
+
+ /* create sysfs files */
+ ret = device_create_file(priv->dev, &dev_attr_reg_br);
+ if (ret)
+ goto out_err3;
+ ret = device_create_file(priv->dev, &dev_attr_reg_or);
+ if (ret)
+ goto out_err4;
+
+ dev_info(priv->dev,
+ "eLBC/GPCM device (%s) at 0x%llx, bank %d, irq=%d\n",
+ priv->name, (unsigned long long)res.start, priv->bank,
+ irq != NO_IRQ ? irq : -1);
+
+ return 0;
+out_err4:
+ device_remove_file(priv->dev, &dev_attr_reg_br);
+out_err3:
+ platform_set_drvdata(pdev, NULL);
+ uio_unregister_device(info);
+out_err2:
+ if (priv->shutdown)
+ priv->shutdown(info, true);
+ iounmap(info->mem[0].internal_addr);
+out_err1:
+ kfree(info->mem[0].name);
+ kfree(info);
+out_err0:
+ kfree(uio_name);
+ kfree(priv);
+ return ret;
+}
+
+static int uio_fsl_elbc_gpcm_remove(struct platform_device *pdev)
+{
+ struct uio_info *info = platform_get_drvdata(pdev);
+ struct fsl_elbc_gpcm *priv = info->priv;
+
+ device_remove_file(priv->dev, &dev_attr_reg_or);
+ device_remove_file(priv->dev, &dev_attr_reg_br);
+ platform_set_drvdata(pdev, NULL);
+ uio_unregister_device(info);
+ if (priv->shutdown)
+ priv->shutdown(info, false);
+ iounmap(info->mem[0].internal_addr);
+ kfree(info->mem[0].name);
+ kfree(info->name);
+ kfree(info);
+ kfree(priv);
+
+ return 0;
+
+}
+
+static const struct of_device_id uio_fsl_elbc_gpcm_match[] = {
+ { .compatible = "fsl,elbc-gpcm-uio", },
+ {}
+};
+
+static struct platform_driver uio_fsl_elbc_gpcm_driver = {
+ .driver = {
+ .name = "fsl,elbc-gpcm-uio",
+ .owner = THIS_MODULE,
+ .of_match_table = uio_fsl_elbc_gpcm_match,
+ },
+ .probe = uio_fsl_elbc_gpcm_probe,
+ .remove = uio_fsl_elbc_gpcm_remove,
+};
+
+static int __init uio_fsl_elbc_gpcm_init(void)
+{
+ return platform_driver_register(&uio_fsl_elbc_gpcm_driver);
+}
+
+static void __exit uio_fsl_elbc_gpcm_exit(void)
+{
+ platform_driver_unregister(&uio_fsl_elbc_gpcm_driver);
+}
+
+module_init(uio_fsl_elbc_gpcm_init);
+module_exit(uio_fsl_elbc_gpcm_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("John Ogness <john.ogness@linutronix.de>");
+MODULE_DESCRIPTION("Freescale Enhanced Local Bus Controller GPCM driver");
--
1.7.10.4
^ permalink raw reply related
* [RFC PATCH 1/8] kprobes: Fix kallsyms lookup across powerpc ABIv1 and ABIv2
From: Naveen N. Rao @ 2014-12-09 17:33 UTC (permalink / raw)
To: linuxppc-dev, linux-kernel, acme, mpe
In-Reply-To: <cover.1418146300.git.naveen.n.rao@linux.vnet.ibm.com>
Currently, all non-dot symbols are being treated as function descriptors
in ABIv1. This is incorrect and is resulting in perf probe not working:
# perf probe do_fork
Added new event:
Failed to write event: Invalid argument
Error: Failed to add events.
# dmesg | tail -1
[192268.073063] Could not insert probe at _text+768432: -22
_text is being resolved incorrectly and is resulting in the above error.
Fix this by changing how we lookup symbol addresses on ppc64. We first
check for the dot variant of a symbol and look at the non-dot variant
only if that fails. In this manner, we avoid having to look at the
function descriptor.
Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
arch/powerpc/include/asm/code-patching.h | 26 +++++++++-----
arch/powerpc/include/asm/kprobes.h | 58 ++++++++++++++++++++++----------
2 files changed, 58 insertions(+), 26 deletions(-)
diff --git a/arch/powerpc/include/asm/code-patching.h b/arch/powerpc/include/asm/code-patching.h
index 840a550..19c5bab 100644
--- a/arch/powerpc/include/asm/code-patching.h
+++ b/arch/powerpc/include/asm/code-patching.h
@@ -47,18 +47,12 @@ void __patch_exception(int exc, unsigned long addr);
#define ADDIS_R2_R12 0x3c4c0000UL
#define ADDI_R2_R2 0x38420000UL
-static inline unsigned long ppc_function_entry(void *func)
+static inline unsigned long ppc_local_function_entry(void *func)
{
-#if defined(CONFIG_PPC64)
-#if defined(_CALL_ELF) && _CALL_ELF == 2
+#if defined(CONFIG_PPC64) && defined(_CALL_ELF) && _CALL_ELF == 2
u32 *insn = func;
/*
- * A PPC64 ABIv2 function may have a local and a global entry
- * point. We need to use the local entry point when patching
- * functions, so identify and step over the global entry point
- * sequence.
- *
* The global entry point sequence is always of the form:
*
* addis r2,r12,XXXX
@@ -76,6 +70,22 @@ static inline unsigned long ppc_function_entry(void *func)
else
return (unsigned long)func;
#else
+ return (unsigned long)func;
+#endif
+}
+
+static inline unsigned long ppc_function_entry(void *func)
+{
+#if defined(CONFIG_PPC64)
+#if defined(_CALL_ELF) && _CALL_ELF == 2
+ /*
+ * A PPC64 ABIv2 function may have a local and a global entry
+ * point. We need to use the local entry point when patching
+ * functions, so identify and step over the global entry point
+ * sequence.
+ */
+ return ppc_local_function_entry(func);
+#else
/*
* On PPC64 ABIv1 the function pointer actually points to the
* function's descriptor. The first entry in the descriptor is the
diff --git a/arch/powerpc/include/asm/kprobes.h b/arch/powerpc/include/asm/kprobes.h
index af15d4d..060bdea 100644
--- a/arch/powerpc/include/asm/kprobes.h
+++ b/arch/powerpc/include/asm/kprobes.h
@@ -42,30 +42,52 @@ typedef ppc_opcode_t kprobe_opcode_t;
#ifdef CONFIG_PPC64
/*
- * 64bit powerpc uses function descriptors.
- * Handle cases where:
- * - User passes a <.symbol> or <module:.symbol>
- * - User passes a <symbol> or <module:symbol>
- * - User passes a non-existent symbol, kallsyms_lookup_name
- * returns 0. Don't deref the NULL pointer in that case
+ * ppc64[le] uses function descriptors with ABIv1 and global/local
+ * entry points for ABIv2:
+ * - Check for the dot variant of the symbol first. If that exists, then
+ * we know this is ABIv1 and we have the symbol and not the descriptor.
+ * - If that fails, try looking up the symbol provided. If that works,
+ * then we either have ABIv1 symbol (not the descriptor) or ABIv2
+ * global entry point.
+ *
+ * Also handle <module:symbol> format.
*/
#define kprobe_lookup_name(name, addr) \
{ \
- addr = (kprobe_opcode_t *)kallsyms_lookup_name(name); \
- if (addr) { \
- char *colon; \
- if ((colon = strchr(name, ':')) != NULL) { \
- colon++; \
- if (*colon != '\0' && *colon != '.') \
- addr = (kprobe_opcode_t *)ppc_function_entry(addr); \
- } else if (name[0] != '.') \
- addr = (kprobe_opcode_t *)ppc_function_entry(addr); \
- } else { \
- char dot_name[KSYM_NAME_LEN]; \
+ char dot_name[MODULE_NAME_LEN + 1 + KSYM_NAME_LEN]; \
+ char *modsym; \
+ bool dot_appended = false; \
+ if ((modsym = strchr(name, ':')) != NULL) { \
+ modsym++; \
+ if (*modsym != '\0' && *modsym != '.') { \
+ /* Convert to <module:.symbol> */ \
+ strncpy(dot_name, name, modsym - name); \
+ dot_name[modsym - name] = '.'; \
+ dot_name[modsym - name + 1] = '\0'; \
+ strncat(dot_name, modsym, sizeof(dot_name) - (modsym - name) - 2); \
+ dot_appended = true; \
+ } else { \
+ dot_name[0] = '\0'; \
+ strncat(dot_name, name, sizeof(dot_name) - 1); \
+ } \
+ } else if (name[0] != '.') { \
dot_name[0] = '.'; \
dot_name[1] = '\0'; \
strncat(dot_name, name, KSYM_NAME_LEN - 2); \
- addr = (kprobe_opcode_t *)kallsyms_lookup_name(dot_name); \
+ dot_appended = true; \
+ } else { \
+ dot_name[0] = '\0'; \
+ strncat(dot_name, name, KSYM_NAME_LEN - 1); \
+ } \
+ addr = (kprobe_opcode_t *)kallsyms_lookup_name(dot_name); \
+ if (!addr && dot_appended) { \
+ /* Let's try the original symbol lookup */ \
+ addr = (kprobe_opcode_t *)kallsyms_lookup_name(name); \
+ if (addr) { \
+ /* We know this isn't a function descriptor */ \
+ /* But, this could be the global entry point */ \
+ addr = (kprobe_opcode_t *)ppc_local_function_entry(addr); \
+ } \
} \
}
#endif
--
2.1.3
^ permalink raw reply related
* [RFC PATCH 0/8] Fix perf probe issues on powerpc
From: Naveen N. Rao @ 2014-12-09 17:33 UTC (permalink / raw)
To: linuxppc-dev, linux-kernel, acme, mpe
This patchset fixes various issues with perf probe on powerpc
across ABIv1 and ABIv2:
- in the presence of DWARF debug-info,
- in the absence of DWARF, but with the symbol table, and
- in the absence of debug-info, but with kallsyms.
Applies cleanly on v3.18 and on -tip with minor changes to patch 6.
Tested on ppc64 BE and LE.
- Naveen
Naveen N. Rao (8):
kprobes: Fix kallsyms lookup across powerpc ABIv1 and ABIv2
perf probe powerpc: Fix symbol fixup issues due to ELF type
perf probe: Improve detection of file/function name in the probe
pattern
perf probe powerpc: Handle powerpc dot symbols
perf probe powerpc: Allow matching against dot symbols
perf tools powerpc: Fix PPC64 ELF ABIv2 symbol decoding
perf probe powerpc: Use DWARF info only if necessary
perf probe powerpc: Fixup function entry if using kallsyms lookup
arch/powerpc/include/asm/code-patching.h | 26 ++++++++----
arch/powerpc/include/asm/kprobes.h | 58 ++++++++++++++++++---------
tools/perf/arch/powerpc/Makefile | 1 +
tools/perf/arch/powerpc/util/elf-sym-decode.c | 27 +++++++++++++
tools/perf/config/Makefile | 1 +
tools/perf/util/elf_sym.h | 13 ++++++
tools/perf/util/probe-event.c | 57 ++++++++++++++++++++++++--
tools/perf/util/symbol-elf.c | 11 ++++-
tools/perf/util/symbol.c | 6 +++
9 files changed, 170 insertions(+), 30 deletions(-)
create mode 100644 tools/perf/arch/powerpc/util/elf-sym-decode.c
create mode 100644 tools/perf/util/elf_sym.h
--
2.1.3
^ permalink raw reply
* [RFC PATCH 2/8] perf probe powerpc: Fix symbol fixup issues due to ELF type
From: Naveen N. Rao @ 2014-12-09 17:34 UTC (permalink / raw)
To: linuxppc-dev, linux-kernel, acme, mpe
In-Reply-To: <cover.1418146300.git.naveen.n.rao@linux.vnet.ibm.com>
If using the symbol table, symbol addresses are not being fixed up
properly, resulting in probes being placed at wrong addresses:
# perf probe do_fork
Added new event:
probe:do_fork (on do_fork)
You can now use it in all perf tools, such as:
perf record -e probe:do_fork -aR sleep 1
# cat /sys/kernel/debug/tracing/kprobe_events
p:probe/do_fork _text+635952
# printf "%x" 635952
9b430
# grep do_fork /boot/System.map
c0000000000ab430 T .do_fork
Fix by checking for ELF type ET_DYN used by ppc64 kernels.
Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
tools/perf/util/symbol-elf.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index 1e23a5b..67e4392 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -629,7 +629,8 @@ int symsrc__init(struct symsrc *ss, struct dso *dso, const char *name,
NULL) != NULL);
} else {
ss->adjust_symbols = ehdr.e_type == ET_EXEC ||
- ehdr.e_type == ET_REL;
+ ehdr.e_type == ET_REL ||
+ ehdr.e_type == ET_DYN;
}
ss->name = strdup(name);
--
2.1.3
^ permalink raw reply related
* [RFC PATCH 3/8] perf probe: Improve detection of file/function name in the probe pattern
From: Naveen N. Rao @ 2014-12-09 17:34 UTC (permalink / raw)
To: linuxppc-dev, linux-kernel, acme, mpe
In-Reply-To: <cover.1418146300.git.naveen.n.rao@linux.vnet.ibm.com>
Currently, perf probe considers patterns including a '.' to be a file.
However, this causes problems on powerpc ABIv1 where all functions have
a leading '.':
$ perf probe -F | grep schedule_timeout_interruptible
.schedule_timeout_interruptible
$ perf probe .schedule_timeout_interruptible
Semantic error :File always requires line number or lazy pattern.
Error: Command Parse Error.
Fix this by checking the probe pattern in more detail.
Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
tools/perf/util/probe-event.c | 23 ++++++++++++++++++++---
1 file changed, 20 insertions(+), 3 deletions(-)
diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index c150ca4..c7e01ef 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -999,6 +999,24 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
arg = tmp;
}
+ /*
+ * Check arg is function or file name and copy it.
+ *
+ * We consider arg to be a file spec if and only if it satisfies
+ * all of the below criteria::
+ * - it does not include any of "+@%",
+ * - it includes one of ":;", and
+ * - it has a period '.' in the name.
+ *
+ * Otherwise, we consider arg to be a function specification.
+ */
+ c = 0;
+ if (!strpbrk(arg, "+@%") && (ptr = strpbrk(arg, ";:")) != NULL) {
+ /* This is a file spec if it includes a '.' before ; or : */
+ if (memchr(arg, '.', ptr-arg))
+ c = 1;
+ }
+
ptr = strpbrk(arg, ";:+@%");
if (ptr) {
nc = *ptr;
@@ -1009,10 +1027,9 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
if (tmp == NULL)
return -ENOMEM;
- /* Check arg is function or file and copy it */
- if (strchr(tmp, '.')) /* File */
+ if (c == 1)
pp->file = tmp;
- else /* Function */
+ else
pp->function = tmp;
/* Parse other options */
--
2.1.3
^ permalink raw reply related
* [RFC PATCH 4/8] perf probe powerpc: Handle powerpc dot symbols
From: Naveen N. Rao @ 2014-12-09 17:34 UTC (permalink / raw)
To: linuxppc-dev, linux-kernel, acme, mpe
In-Reply-To: <cover.1418146300.git.naveen.n.rao@linux.vnet.ibm.com>
Fix up various perf aspects related to ppc64's usage of dot functions:
- ignore leading '.' when generating event names and when looking for
existing events.
- use the proper prefix when ignoring SyS symbol lookups.
Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
tools/perf/util/probe-event.c | 8 ++++++++
tools/perf/util/symbol.c | 6 ++++++
2 files changed, 14 insertions(+)
diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index c7e01ef..d465f7c 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -2080,6 +2080,10 @@ static int get_new_event_name(char *buf, size_t len, const char *base,
{
int i, ret;
+ /* Skip the leading dot on powerpc */
+ if (*base == '.')
+ base++;
+
/* Try no suffix */
ret = e_snprintf(buf, len, "%s", base);
if (ret < 0) {
@@ -2538,6 +2542,10 @@ int del_perf_probe_events(struct strlist *dellist)
event = str;
}
+ /* Skip the leading dot on powerpc */
+ if (event && *event == '.')
+ event++;
+
ret = e_snprintf(buf, 128, "%s:%s", group, event);
if (ret < 0) {
pr_err("Failed to copy event.");
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 0783311..cc04475 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -137,6 +137,12 @@ static int choose_best_symbol(struct symbol *syma, struct symbol *symb)
if (na >= 10 && !strncmp(syma->name, "compat_SyS", 10))
return SYMBOL_B;
+ /* On powerpc, ignore the dot variants */
+ if (na >= 4 && !strncmp(syma->name, ".SyS", 4))
+ return SYMBOL_B;
+ if (na >= 11 && !strncmp(syma->name, ".compat_SyS", 11))
+ return SYMBOL_B;
+
return SYMBOL_A;
}
--
2.1.3
^ permalink raw reply related
* [RFC PATCH 5/8] perf probe powerpc: Allow matching against dot symbols
From: Naveen N. Rao @ 2014-12-09 17:34 UTC (permalink / raw)
To: linuxppc-dev, linux-kernel, acme, mpe
In-Reply-To: <cover.1418146300.git.naveen.n.rao@linux.vnet.ibm.com>
Allow perf probe to work on powerpc ABIv1 without the need to specify the
leading dot '.' for functions. 'perf probe do_fork' works with this patch.
Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
tools/perf/util/probe-event.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index d465f7c..174c22e 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -2221,6 +2221,15 @@ static int probe_function_filter(struct map *map __maybe_unused,
num_matched_functions++;
return 0;
}
+#ifdef __powerpc64__
+ /* Allow matching against the dot variant */
+ if (sym->name[0] == '.' && looking_function_name[0] != '.' &&
+ (sym->binding == STB_GLOBAL || sym->binding == STB_LOCAL) &&
+ strcmp(looking_function_name, sym->name+1) == 0) {
+ num_matched_functions++;
+ return 0;
+ }
+#endif
return 1;
}
--
2.1.3
^ permalink raw reply related
* [RFC PATCH 6/8] perf tools powerpc: Fix PPC64 ELF ABIv2 symbol decoding
From: Naveen N. Rao @ 2014-12-09 17:34 UTC (permalink / raw)
To: linuxppc-dev, linux-kernel, acme, mpe
In-Reply-To: <cover.1418146300.git.naveen.n.rao@linux.vnet.ibm.com>
PPC64 ELF ABIv2 has a Global Entry Point (GEP) and a Local Entry Point
(LEP). For purposes of probing, we need the LEP. Offset to the LEP is
encoded in st_other.
Signed-off-by: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
tools/perf/arch/powerpc/Makefile | 1 +
tools/perf/arch/powerpc/util/elf-sym-decode.c | 27 +++++++++++++++++++++++++++
tools/perf/config/Makefile | 1 +
tools/perf/util/elf_sym.h | 13 +++++++++++++
tools/perf/util/symbol-elf.c | 8 ++++++++
5 files changed, 50 insertions(+)
create mode 100644 tools/perf/arch/powerpc/util/elf-sym-decode.c
create mode 100644 tools/perf/util/elf_sym.h
diff --git a/tools/perf/arch/powerpc/Makefile b/tools/perf/arch/powerpc/Makefile
index 6f7782b..8621439 100644
--- a/tools/perf/arch/powerpc/Makefile
+++ b/tools/perf/arch/powerpc/Makefile
@@ -3,4 +3,5 @@ PERF_HAVE_DWARF_REGS := 1
LIB_OBJS += $(OUTPUT)arch/$(ARCH)/util/dwarf-regs.o
LIB_OBJS += $(OUTPUT)arch/$(ARCH)/util/skip-callchain-idx.o
endif
+LIB_OBJS += $(OUTPUT)arch/$(ARCH)/util/elf-sym-decode.o
LIB_OBJS += $(OUTPUT)arch/$(ARCH)/util/header.o
diff --git a/tools/perf/arch/powerpc/util/elf-sym-decode.c b/tools/perf/arch/powerpc/util/elf-sym-decode.c
new file mode 100644
index 0000000..7434656
--- /dev/null
+++ b/tools/perf/arch/powerpc/util/elf-sym-decode.c
@@ -0,0 +1,27 @@
+/*
+ * Decode offset from Global Entry Point to Local Entry Point on PPC64
+ * ELF ABIv2.
+ *
+ * Derived from definitions in arch/powerpc/kernel/module_64.c
+ *
+ * Copyright (C) 2014 Ananth N Mavinakayanahalli, IBM Corporation.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ */
+
+#include <gelf.h>
+#include "elf_sym.h"
+
+/* PowerPC64 ABIv2 specific values for the ELF64_Sym st_other field. */
+#define STO_PPC64_LOCAL_BIT 5
+#define STO_PPC64_LOCAL_MASK (7 << STO_PPC64_LOCAL_BIT)
+#define PPC64_LOCAL_ENTRY_OFFSET(other) \
+ (((1 << (((other) & STO_PPC64_LOCAL_MASK) >> STO_PPC64_LOCAL_BIT)) >> 2) << 2)
+
+unsigned int arch_elf_sym_decode_offset(GElf_Sym *sym)
+{
+ return PPC64_LOCAL_ENTRY_OFFSET(sym->st_other);
+}
diff --git a/tools/perf/config/Makefile b/tools/perf/config/Makefile
index 58f6091..8f64557 100644
--- a/tools/perf/config/Makefile
+++ b/tools/perf/config/Makefile
@@ -378,6 +378,7 @@ ifeq ($(ARCH),powerpc)
ifndef NO_DWARF
CFLAGS += -DHAVE_SKIP_CALLCHAIN_IDX
endif
+ CFLAGS += -DHAVE_ELF_SYM_DECODE
endif
ifndef NO_LIBUNWIND
diff --git a/tools/perf/util/elf_sym.h b/tools/perf/util/elf_sym.h
new file mode 100644
index 0000000..0176f21
--- /dev/null
+++ b/tools/perf/util/elf_sym.h
@@ -0,0 +1,13 @@
+#ifndef __PERF_ELF_SYM_H
+#define __PERF_ELF_SYM_H
+
+#ifdef HAVE_ELF_SYM_DECODE
+extern unsigned int arch_elf_sym_decode_offset(GElf_Sym *sym);
+#else
+static inline unsigned int arch_elf_sym_decode_offset(GElf_Sym *sym __maybe_unused)
+{
+ return 0;
+}
+#endif
+
+#endif /* __PERF_ELF_SYM_H */
diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index 67e4392..92a424e 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -10,6 +10,7 @@
#include "vdso.h"
#include <symbol/kallsyms.h>
#include "debug.h"
+#include "elf_sym.h"
#ifndef HAVE_ELF_GETPHDRNUM_SUPPORT
static int elf_getphdrnum(Elf *elf, size_t *dst)
@@ -848,6 +849,13 @@ int dso__load_sym(struct dso *dso, struct map *map,
(sym.st_value & 1))
--sym.st_value;
+ /*
+ * PPC64 ELF ABIv2 encodes Local Entry Point offset in
+ * the st_other field
+ */
+ if ((map->type == MAP__FUNCTION) && sym.st_other)
+ sym.st_value += arch_elf_sym_decode_offset(&sym);
+
if (dso->kernel || kmodule) {
char dso_name[PATH_MAX];
--
2.1.3
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).