LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH V2] powerpc/mm/radix: Update pte fragment count from 16 to 256 on radix
From: Aneesh Kumar K.V @ 2018-03-22  8:43 UTC (permalink / raw)
  To: benh, paulus, mpe; +Cc: linuxppc-dev, Aneesh Kumar K.V

Right now we use only 4K out of the 64k page allocated for the level 4 page
table. W.r.t the performance impact due to lock contention, with ebizzy

256 threads:
without patch (10 runs of ./ebizzy  -m -n 1000 -s 131072 -S 100)
median = 15678.5
stdev = 42.1209

with patch:
median = 15354
stdev = 194.743

This is with THP disabled. With THP enabled the impact of the patch will be less.

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
 arch/powerpc/include/asm/book3s/64/hash-64k.h  | 10 ++++------
 arch/powerpc/include/asm/book3s/64/radix-4k.h  |  5 +++++
 arch/powerpc/include/asm/book3s/64/radix-64k.h |  6 ++++++
 arch/powerpc/mm/pgtable-radix.c                |  8 ++------
 4 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/arch/powerpc/include/asm/book3s/64/hash-64k.h b/arch/powerpc/include/asm/book3s/64/hash-64k.h
index 3bcf269f8f55..0aa4f755b3f6 100644
--- a/arch/powerpc/include/asm/book3s/64/hash-64k.h
+++ b/arch/powerpc/include/asm/book3s/64/hash-64k.h
@@ -24,16 +24,14 @@
 
 /* PTE flags to conserve for HPTE identification */
 #define _PAGE_HPTEFLAGS (H_PAGE_BUSY | H_PAGE_HASHPTE | H_PAGE_COMBO)
-/*
- * we support 16 fragments per PTE page of 64K size.
- */
-#define H_PTE_FRAG_NR	16
 /*
  * We use a 2K PTE page fragment and another 2K for storing
  * real_pte_t hash index
+ * 8 bytes per each pte entry and another 8 bytes for storing
+ * slot details.
  */
-#define H_PTE_FRAG_SIZE_SHIFT  12
-#define PTE_FRAG_SIZE (1UL << PTE_FRAG_SIZE_SHIFT)
+#define H_PTE_FRAG_SIZE_SHIFT  (H_PTE_INDEX_SIZE + 3 + 1)
+#define H_PTE_FRAG_NR	(PAGE_SIZE >> H_PTE_FRAG_SIZE_SHIFT)
 
 #ifndef __ASSEMBLY__
 #include <asm/errno.h>
diff --git a/arch/powerpc/include/asm/book3s/64/radix-4k.h b/arch/powerpc/include/asm/book3s/64/radix-4k.h
index a61aa9cd63ec..ca366ec86310 100644
--- a/arch/powerpc/include/asm/book3s/64/radix-4k.h
+++ b/arch/powerpc/include/asm/book3s/64/radix-4k.h
@@ -9,5 +9,10 @@
 #define RADIX_PMD_INDEX_SIZE  9  /* 1G huge page */
 #define RADIX_PUD_INDEX_SIZE	 9
 #define RADIX_PGD_INDEX_SIZE  13
+/*
+ * One fragment per per page
+ */
+#define RADIX_PTE_FRAG_SIZE_SHIFT  (RADIX_PTE_INDEX_SIZE + 3)
+#define RADIX_PTE_FRAG_NR	(PAGE_SIZE >> RADIX_PTE_FRAG_SIZE_SHIFT)
 
 #endif /* _ASM_POWERPC_PGTABLE_RADIX_4K_H */
diff --git a/arch/powerpc/include/asm/book3s/64/radix-64k.h b/arch/powerpc/include/asm/book3s/64/radix-64k.h
index c7e71ba29555..830082496876 100644
--- a/arch/powerpc/include/asm/book3s/64/radix-64k.h
+++ b/arch/powerpc/include/asm/book3s/64/radix-64k.h
@@ -10,4 +10,10 @@
 #define RADIX_PUD_INDEX_SIZE	 9
 #define RADIX_PGD_INDEX_SIZE  13
 
+/*
+ * We use a 256 byte PTE page fragment in radix
+ * 8 bytes per each PTE entry.
+ */
+#define RADIX_PTE_FRAG_SIZE_SHIFT  (RADIX_PTE_INDEX_SIZE + 3)
+#define RADIX_PTE_FRAG_NR	(PAGE_SIZE >> RADIX_PTE_FRAG_SIZE_SHIFT)
 #endif /* _ASM_POWERPC_PGTABLE_RADIX_64K_H */
diff --git a/arch/powerpc/mm/pgtable-radix.c b/arch/powerpc/mm/pgtable-radix.c
index 2e10a964e290..7fd51549b9e5 100644
--- a/arch/powerpc/mm/pgtable-radix.c
+++ b/arch/powerpc/mm/pgtable-radix.c
@@ -575,12 +575,8 @@ void __init radix__early_init_mmu(void)
 #ifdef CONFIG_PCI
 	pci_io_base = ISA_IO_BASE;
 #endif
-
-	/*
-	 * For now radix also use the same frag size
-	 */
-	__pte_frag_nr = H_PTE_FRAG_NR;
-	__pte_frag_size_shift = H_PTE_FRAG_SIZE_SHIFT;
+	__pte_frag_nr = RADIX_PTE_FRAG_NR;
+	__pte_frag_size_shift = RADIX_PTE_FRAG_SIZE_SHIFT;
 
 	if (!firmware_has_feature(FW_FEATURE_LPAR)) {
 		radix_init_native();
-- 
2.14.3

^ permalink raw reply related

* Re: RFC on writel and writel_relaxed
From: Oliver @ 2018-03-22  9:25 UTC (permalink / raw)
  To: Gabriel Paubert
  Cc: David Laight, Sinan Kaya, linux-rdma@vger.kernel.org,
	open list:LINUX FOR POWERPC (32-BIT AND 64-BIT)
In-Reply-To: <20180322082041.hvaneyfpzdwelely@lt-gp.iram.es>

On Thu, Mar 22, 2018 at 7:20 PM, Gabriel Paubert <paubert@iram.es> wrote:
> On Thu, Mar 22, 2018 at 04:24:24PM +1100, Oliver wrote:
>> On Thu, Mar 22, 2018 at 1:35 AM, David Laight <David.Laight@aculab.com> wrote:
>> >> x86 has compiler barrier inside the relaxed() API so that code does not
>> >> get reordered. ARM64 architecturally guarantees device writes to be observed
>> >> in order.
>> >
>> > There are places where you don't even need a compile barrier between
>> > every write.
>> >
>> > I had horrid problems getting some ppc code (for a specific embedded SoC)
>> > optimised to have no extra barriers.
>> > I ended up just writing through 'pointer to volatile' and adding an
>> > explicit 'eieio' between the block of writes and status read.
>>
>> This is what you are supposed to do. For accesses to MMIO (cache
>> inhibited + guarded) storage the Power ISA guarantees that load-load
>> and store-store pairs of accesses will always occur in program order,
>> but there's no implicit ordering between load-store or store-load
>
> And even for load store, eieio is not always necessary, in the important
> case of reading and writing to the same address, when modifying bits in
> a control register for example.
>
> Typically also loads will be moved ahead of stores, but not the other
> way around, so in practice you won't notice a missed eieio in this case.
> This does not mean you should not insert it.

Yep, but it doesn't really help us here. The generic accessors need to cope
with the general case.

>> pairs. In those cases you need an explicit eieio barrier between the
>> two accesses. At the HW level you can think of the CPU as having
>> separate queues for MMIO loads and stores. Accesses will be added to
>> the respective queue in program order, but there's no synchronisation
>> between the two queues. If the CPU is doing write combining it's easy
>> to imagine the whole store queue being emptied in one big gulp before
>> the load queue is even touched.
>
> Is write combining allowed on guarded storage?
>
> <Looking at docs>
> From PowerISA_V3.0.pdf, Book2, section 1.6.2 "Caching inhibited":
>
> "No combining occurs if the storage is also Guarded"

Yeah it's not allowed. That's what I get for handwaving examples ;)

^ permalink raw reply

* [PATCH v2] crypto: talitos - fix IPsec cipher in length
From: Christophe Leroy @ 2018-03-22  9:57 UTC (permalink / raw)
  To: Herbert Xu, David S. Miller, horia.geanta
  Cc: linux-crypto, linux-kernel, linuxppc-dev

For SEC 2.x+, cipher in length must contain only the ciphertext length.
In case of using hardware ICV checking, the ICV length is provided via
the "extent" field of the descriptor pointer.

Cc: <stable@vger.kernel.org> # 4.8+
Fixes: 549bd8bc5987 ("crypto: talitos - Implement AEAD for SEC1 using HMAC_SNOOP_NO_AFEU")
Reported-by: Horia Geantă <horia.geanta@nxp.com>
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
 drivers/crypto/talitos.c | 36 ++++++++++++++++++++----------------
 1 file changed, 20 insertions(+), 16 deletions(-)

diff --git a/drivers/crypto/talitos.c b/drivers/crypto/talitos.c
index 6882fa2f8bad..016ff8c4c305 100644
--- a/drivers/crypto/talitos.c
+++ b/drivers/crypto/talitos.c
@@ -1130,10 +1130,10 @@ static int sg_to_link_tbl_offset(struct scatterlist *sg, int sg_count,
 	return count;
 }
 
-static int talitos_sg_map(struct device *dev, struct scatterlist *src,
-		   unsigned int len, struct talitos_edesc *edesc,
-		   struct talitos_ptr *ptr,
-		   int sg_count, unsigned int offset, int tbl_off)
+static int talitos_sg_map_ext(struct device *dev, struct scatterlist *src,
+			      unsigned int len, struct talitos_edesc *edesc,
+			      struct talitos_ptr *ptr, int sg_count,
+			      unsigned int offset, int tbl_off, int elen)
 {
 	struct talitos_private *priv = dev_get_drvdata(dev);
 	bool is_sec1 = has_ftr_sec1(priv);
@@ -1142,6 +1142,7 @@ static int talitos_sg_map(struct device *dev, struct scatterlist *src,
 		to_talitos_ptr(ptr, 0, 0, is_sec1);
 		return 1;
 	}
+	to_talitos_ptr_ext_set(ptr, elen, is_sec1);
 	if (sg_count == 1) {
 		to_talitos_ptr(ptr, sg_dma_address(src) + offset, len, is_sec1);
 		return sg_count;
@@ -1150,7 +1151,7 @@ static int talitos_sg_map(struct device *dev, struct scatterlist *src,
 		to_talitos_ptr(ptr, edesc->dma_link_tbl + offset, len, is_sec1);
 		return sg_count;
 	}
-	sg_count = sg_to_link_tbl_offset(src, sg_count, offset, len,
+	sg_count = sg_to_link_tbl_offset(src, sg_count, offset, len + elen,
 					 &edesc->link_tbl[tbl_off]);
 	if (sg_count == 1) {
 		/* Only one segment now, so no link tbl needed*/
@@ -1164,6 +1165,15 @@ static int talitos_sg_map(struct device *dev, struct scatterlist *src,
 	return sg_count;
 }
 
+static int talitos_sg_map(struct device *dev, struct scatterlist *src,
+			  unsigned int len, struct talitos_edesc *edesc,
+			  struct talitos_ptr *ptr, int sg_count,
+			  unsigned int offset, int tbl_off)
+{
+	return talitos_sg_map_ext(dev, src, len, edesc, ptr, sg_count, offset,
+				  tbl_off, 0);
+}
+
 /*
  * fill in and submit ipsec_esp descriptor
  */
@@ -1181,7 +1191,7 @@ static int ipsec_esp(struct talitos_edesc *edesc, struct aead_request *areq,
 	unsigned int ivsize = crypto_aead_ivsize(aead);
 	int tbl_off = 0;
 	int sg_count, ret;
-	int sg_link_tbl_len;
+	int elen = 0;
 	bool sync_needed = false;
 	struct talitos_private *priv = dev_get_drvdata(dev);
 	bool is_sec1 = has_ftr_sec1(priv);
@@ -1223,17 +1233,11 @@ static int ipsec_esp(struct talitos_edesc *edesc, struct aead_request *areq,
 	 * extent is bytes of HMAC postpended to ciphertext,
 	 * typically 12 for ipsec
 	 */
-	sg_link_tbl_len = cryptlen;
-
-	if (is_ipsec_esp) {
-		to_talitos_ptr_ext_set(&desc->ptr[4], authsize, is_sec1);
-
-		if (desc->hdr & DESC_HDR_MODE1_MDEU_CICV)
-			sg_link_tbl_len += authsize;
-	}
+	if (is_ipsec_esp && (desc->hdr & DESC_HDR_MODE1_MDEU_CICV))
+		elen = authsize;
 
-	ret = talitos_sg_map(dev, areq->src, sg_link_tbl_len, edesc,
-			     &desc->ptr[4], sg_count, areq->assoclen, tbl_off);
+	ret = talitos_sg_map_ext(dev, areq->src, cryptlen, edesc, &desc->ptr[4],
+				 sg_count, areq->assoclen, tbl_off, elen);
 
 	if (ret > 1) {
 		tbl_off += ret;
-- 
2.13.3

^ permalink raw reply related

* Re: RFC on writel and writel_relaxed
From: Oliver @ 2018-03-22 10:15 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: Sinan Kaya, open list:LINUX FOR POWERPC (32-BIT AND 64-BIT),
	linux-rdma@vger.kernel.org
In-Reply-To: <1521692689.16434.293.camel@kernel.crashing.org>

On Thu, Mar 22, 2018 at 3:24 PM, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> On Wed, 2018-03-21 at 08:53 -0500, Sinan Kaya wrote:
>> writel_relaxed() needs to have ordering guarantees with respect to the order
>> device observes writes.
>
> Correct.
>
>> x86 has compiler barrier inside the relaxed() API so that code does not
>> get reordered. ARM64 architecturally guarantees device writes to be observed
>> in order.
>>
>> I was hoping that PPC could follow x86 and inject compiler barrier into the
>> relaxed functions.
>>
>> BTW, I have no idea what compiler barrier does on PPC and if
>>
>> wrltel() == compiler barrier() + wrltel_relaxed()
>>
>> can be said.
>
> No, it's not sufficient.
>
> Replacing wmb() + writel() with wmb() + writel_relaxed() will work on
> PPC, it will just not give you a benefit today.
>
> The main problem is that the semantics of writel/writel_relaxed (and
> read versions) aren't very well defined in Linux esp. when it comes
> to different memory types (NC, WC, ...).
>
> I've been wanting to implement the relaxed accessors for a while but
> was battling with this to try to also better support WC, and due to
> other commitments, this somewhat fell down the cracks.
>
> Two options I can think of:
>
>  - Just make the _relaxed variants use an eieio instead of a sync, this
> will effectively lift the ordering guarantee vs. cachable storage (and
> thus unlock) and might give a (small) performance improvement.

Wouldn't we still have the unlock ordering due to the io_sync hack or
are you thinking we should remove that too for the relaxed version?

> However,
> we still have the problem that on WC mappings, neither writel nor
> writel_relaxed will effectively allow combining to happen (only raw
> accesses will because on powerpc *all* barriers will break combining).

Hmm, eieio is only architected to affect CI+G (and WT) so it shouldn't
affect combining
on non-guarded memory. Do most implementations apply it to all CI
accesses anyway?

>  - Make writel_relaxed() be a simple store without barriers, and
> readl_relaxed() be "eieio, read, eieio", thus allowing write combining
> to happen between successive writel_relaxed on WC space (no change on
> normal NC space) while maintaining the ordering between relaxed reads
> and writes. The flip side is a (slight) increased overhead of
> readl_relaxed.

Are there many drivers that actually do writeX() on WC space?
memory-barriers.txt
pretty much says that all bets are off and no ordering guarantees can be assumed
when using readX/writeX on prefetchable IO memory. It seems sketchy enough to
give me some pause, but maybe it works fine elsewhere.

Oliver

^ permalink raw reply

* [PATCH v3 0/6] powerpc64/ftrace: Add support for ftrace_modify_call() and a few other fixes
From: Naveen N. Rao @ 2018-03-22 10:25 UTC (permalink / raw)
  To: Michael Ellerman, Steven Rostedt, Paul Mackerras
  Cc: linuxppc-dev, Anton Blanchard, Nicholas Piggin, sathnaga

This is v3 of the patches posted at:
https://www.mail-archive.com/linuxppc-dev@lists.ozlabs.org/msg130652.html

This series has been tested using mambo for p8 (hash) and p9 (radix).


The first two patches fix a kernel oops when function tracing is enabled 
while using KVM.

Patch 3 is new and changes how ftrace is disabled before kexec.

Patch 4 tightens how we detect _mcount() call sites for -mprofile-kernel 
during module loading.

The last two patches implement support for ftrace_caller() to 
conditionally save the register state. This speeds up the function 
tracer a bit. The existing ftrace_caller() is renamed to 
ftrace_regs_caller() since we save the entire pt_regs today. A new 
implementation of ftrace_caller() that saves the minimum register state 
is provided. We switch between the two variants through 
ftrace_modify_call(). The necessary support to call into the two 
different variants from modules is also added.


- Naveen


Naveen N. Rao (6):
  powerpc64/ftrace: Add a field in paca to disable ftrace in unsafe code
    paths
  powerpc64/ftrace: Disable ftrace during kvm guest entry/exit
  powerpc/kexec: Disable ftrace before switching to the new kernel
  powerpc64/module: Tighten detection of mcount call sites with
    -mprofile-kernel
  powerpc64/ftrace: Use the generic version of ftrace_replace_code()
  powerpc64/ftrace: Implement support for ftrace_regs_caller()

 arch/powerpc/include/asm/ftrace.h              |   2 -
 arch/powerpc/include/asm/module.h              |   3 +
 arch/powerpc/include/asm/paca.h                |   1 +
 arch/powerpc/kernel/asm-offsets.c              |   1 +
 arch/powerpc/kernel/machine_kexec.c            |   6 +-
 arch/powerpc/kernel/module_64.c                |  43 +++--
 arch/powerpc/kernel/trace/ftrace.c             | 210 ++++++++++++++++++++-----
 arch/powerpc/kernel/trace/ftrace_64_mprofile.S |  85 +++++++++-
 arch/powerpc/kernel/trace/ftrace_64_pg.S       |   4 +
 arch/powerpc/kvm/book3s_hv_rmhandlers.S        |   8 +
 10 files changed, 296 insertions(+), 67 deletions(-)

-- 
2.16.2

^ permalink raw reply

* [PATCH v3 1/6] powerpc64/ftrace: Add a field in paca to disable ftrace in unsafe code paths
From: Naveen N. Rao @ 2018-03-22 10:25 UTC (permalink / raw)
  To: Michael Ellerman, Steven Rostedt, Paul Mackerras
  Cc: linuxppc-dev, Anton Blanchard, Nicholas Piggin, sathnaga
In-Reply-To: <cover.1521713762.git.naveen.n.rao@linux.vnet.ibm.com>

We have some C code that we call into from real mode where we cannot
take any exceptions. Though the C functions themselves are mostly safe,
if these functions are traced, there is a possibility that we may take
an exception. For instance, in certain conditions, the ftrace code uses
WARN(), which uses a 'trap' to do its job.

For such scenarios, introduce a new field in paca 'ftrace_disabled',
which is checked on ftrace entry before continuing. This field can then
be set to a non-zero value to disable/pause ftrace, and reset to zero to
resume ftrace.

Acked-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
Changes since v2:
- Move paca->ftrace_disabled out of CONFIG_BOOK3S_64.
- Disable tracing when asked for, not the other way around.


 arch/powerpc/include/asm/paca.h                |  1 +
 arch/powerpc/kernel/asm-offsets.c              |  1 +
 arch/powerpc/kernel/trace/ftrace_64_mprofile.S | 14 ++++++++++++++
 arch/powerpc/kernel/trace/ftrace_64_pg.S       |  4 ++++
 4 files changed, 20 insertions(+)

diff --git a/arch/powerpc/include/asm/paca.h b/arch/powerpc/include/asm/paca.h
index d2bf71dddbef..5be57c20c8c5 100644
--- a/arch/powerpc/include/asm/paca.h
+++ b/arch/powerpc/include/asm/paca.h
@@ -164,6 +164,7 @@ struct paca_struct {
 	u8 io_sync;			/* writel() needs spin_unlock sync */
 	u8 irq_work_pending;		/* IRQ_WORK interrupt while soft-disable */
 	u8 nap_state_lost;		/* NV GPR values lost in power7_idle */
+	u8 ftrace_disabled;		/* Hard disable ftrace */
 	u64 sprg_vdso;			/* Saved user-visible sprg */
 #ifdef CONFIG_PPC_TRANSACTIONAL_MEM
	u64 tm_scratch;                 /* TM scratch area for reclaim 
*/
diff --git a/arch/powerpc/kernel/asm-offsets.c b/arch/powerpc/kernel/asm-offsets.c
index ea5eb91b836e..775c847f0e33 100644
--- a/arch/powerpc/kernel/asm-offsets.c
+++ b/arch/powerpc/kernel/asm-offsets.c
@@ -180,6 +180,7 @@ int main(void)
 	OFFSET(PACAKMSR, paca_struct, kernel_msr);
 	OFFSET(PACAIRQSOFTMASK, paca_struct, irq_soft_mask);
 	OFFSET(PACAIRQHAPPENED, paca_struct, irq_happened);
+	OFFSET(PACA_FTRACE_DISABLED, paca_struct, ftrace_disabled);
 #ifdef CONFIG_PPC_BOOK3S
 	OFFSET(PACACONTEXTID, paca_struct, mm_ctx_id);
 #ifdef CONFIG_PPC_MM_SLICES
diff --git a/arch/powerpc/kernel/trace/ftrace_64_mprofile.S b/arch/powerpc/kernel/trace/ftrace_64_mprofile.S
index 3f3e81852422..9d234835e1e6 100644
--- a/arch/powerpc/kernel/trace/ftrace_64_mprofile.S
+++ b/arch/powerpc/kernel/trace/ftrace_64_mprofile.S
@@ -47,6 +47,12 @@ _GLOBAL(ftrace_caller)
 	/* Save all gprs to pt_regs */
 	SAVE_GPR(0, r1)
 	SAVE_10GPRS(2, r1)
+
+	/* Ok to continue? */
+	lbz	r3, PACA_FTRACE_DISABLED(r13)
+	cmpdi	r3, 0
+	bne	ftrace_no_trace
+
 	SAVE_10GPRS(12, r1)
 	SAVE_10GPRS(22, r1)
 
@@ -168,6 +174,14 @@ _GLOBAL(ftrace_graph_stub)
 _GLOBAL(ftrace_stub)
 	blr
 
+ftrace_no_trace:
+	mflr	r3
+	mtctr	r3
+	REST_GPR(3, r1)
+	addi	r1, r1, SWITCH_FRAME_SIZE
+	mtlr	r0
+	bctr
+
 #ifdef CONFIG_LIVEPATCH
 	/*
 	 * This function runs in the mcount context, between two functions. As
diff --git a/arch/powerpc/kernel/trace/ftrace_64_pg.S b/arch/powerpc/kernel/trace/ftrace_64_pg.S
index f095358da96e..3d8186832a34 100644
--- a/arch/powerpc/kernel/trace/ftrace_64_pg.S
+++ b/arch/powerpc/kernel/trace/ftrace_64_pg.S
@@ -16,6 +16,10 @@
 
 #ifdef CONFIG_DYNAMIC_FTRACE
 _GLOBAL_TOC(ftrace_caller)
+	lbz	r3, PACA_FTRACE_DISABLED(r13)
+	cmpdi	r3, 0
+	bnelr
+
 	/* Taken from output of objdump from lib64/glibc */
 	mflr	r3
 	ld	r11, 0(r1)
-- 
2.16.2

^ permalink raw reply related

* [PATCH v3 2/6] powerpc64/ftrace: Disable ftrace during kvm guest entry/exit
From: Naveen N. Rao @ 2018-03-22 10:25 UTC (permalink / raw)
  To: Michael Ellerman, Steven Rostedt, Paul Mackerras
  Cc: linuxppc-dev, Anton Blanchard, Nicholas Piggin, sathnaga
In-Reply-To: <cover.1521713762.git.naveen.n.rao@linux.vnet.ibm.com>

During guest entry/exit, we switch over to/from the guest MMU context.
While doing so, we set our state to KVM_GUEST_MODE_HOST_HV to note down
the fact that we cannot take any exceptions in the hypervisor code.

Since ftrace may be enabled and since it can result in us taking a trap,
disable ftrace by setting paca->ftrace_disabled. Once we exit the guest
and restore host MMU context, we re-enable ftrace.

Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
 arch/powerpc/kvm/book3s_hv_rmhandlers.S | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
index f31f357b8c5a..9292087adb68 100644
--- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
+++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
@@ -600,6 +600,10 @@ kvmppc_hv_entry:
 	/* Save R1 in the PACA */
 	std	r1, HSTATE_HOST_R1(r13)
 
+	/* Disable ftrace since we can't take a trap any more */
+	li	r6, 1
+	stb	r6, PACA_FTRACE_DISABLED(r13)
+
 	li	r6, KVM_GUEST_MODE_HOST_HV
 	stb	r6, HSTATE_IN_GUEST(r13)
 
@@ -2048,6 +2052,8 @@ END_MMU_FTR_SECTION_IFSET(MMU_FTR_TYPE_RADIX)
 	/* Unset guest mode */
 	li	r0, KVM_GUEST_MODE_NONE
 	stb	r0, HSTATE_IN_GUEST(r13)
+	li	r0, 0
+	stb	r0, PACA_FTRACE_DISABLED(r13)
 
 	ld	r0, SFS+PPC_LR_STKOFF(r1)
 	addi	r1, r1, SFS
@@ -3379,6 +3385,8 @@ END_MMU_FTR_SECTION_IFSET(MMU_FTR_TYPE_RADIX)
 	ld	r8, KVM_HOST_LPCR(r10)
 	mtspr	SPRN_LPCR, r8
 	isync
+	li	r0, 0
+	stb	r0, PACA_FTRACE_DISABLED(r13)
 	li	r0, KVM_GUEST_MODE_NONE
 	stb	r0, HSTATE_IN_GUEST(r13)
 
-- 
2.16.2

^ permalink raw reply related

* [PATCH v3 3/6] powerpc/kexec: Disable ftrace before switching to the new kernel
From: Naveen N. Rao @ 2018-03-22 10:25 UTC (permalink / raw)
  To: Michael Ellerman, Steven Rostedt, Paul Mackerras
  Cc: linuxppc-dev, Anton Blanchard, Nicholas Piggin, sathnaga
In-Reply-To: <cover.1521713762.git.naveen.n.rao@linux.vnet.ibm.com>

If function_graph tracer is enabled during kexec, we see the below
exception in the simulator:
	root@(none):/# kexec -e
	kvm: exiting hardware virtualization
	kexec_core: Starting new kernel
	[   19.262020070,5] OPAL: Switch to big-endian OS
	kexec: Starting switchover sequence.
	Interrupt to 0xC000000000004380 from 0xC000000000004380
	** Execution stopped: Continuous Interrupt, Instruction caused exception,  **

Now that we have a more effective way to disable ftrace, let's use that
before switching to a new kernel during kexec.

Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
 arch/powerpc/kernel/machine_kexec.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/kernel/machine_kexec.c b/arch/powerpc/kernel/machine_kexec.c
index 2694d078741d..4a1b24a9dd61 100644
--- a/arch/powerpc/kernel/machine_kexec.c
+++ b/arch/powerpc/kernel/machine_kexec.c
@@ -95,16 +95,14 @@ void arch_crash_save_vmcoreinfo(void)
  */
 void machine_kexec(struct kimage *image)
 {
-	int save_ftrace_enabled;
-
-	save_ftrace_enabled = __ftrace_enabled_save();
+	get_paca()->ftrace_disabled = 1;
 
 	if (ppc_md.machine_kexec)
 		ppc_md.machine_kexec(image);
 	else
 		default_machine_kexec(image);
 
-	__ftrace_enabled_restore(save_ftrace_enabled);
+	get_paca()->ftrace_disabled = 0;
 
 	/* Fall back to normal restart if we're still alive. */
 	machine_restart(NULL);
-- 
2.16.2

^ permalink raw reply related

* [PATCH v3 4/6] powerpc64/module: Tighten detection of mcount call sites with -mprofile-kernel
From: Naveen N. Rao @ 2018-03-22 10:25 UTC (permalink / raw)
  To: Michael Ellerman, Steven Rostedt, Paul Mackerras
  Cc: linuxppc-dev, Anton Blanchard, Nicholas Piggin, sathnaga
In-Reply-To: <cover.1521713762.git.naveen.n.rao@linux.vnet.ibm.com>

For R_PPC64_REL24 relocations, we suppress emitting instructions for TOC
load/restore in the relocation stub if the relocation is for _mcount()
call when using -mprofile-kernel ABI.

To detect this, we check if the preceding instructions are per the
standard set of instructions emitted by gcc: either the two instruction
sequence of 'mflr r0; std r0,16(r1)', or the more optimized variant of a
single 'mflr r0'. This is not sufficient since nothing prevents users
from hand coding sequences involving a 'mflr r0' followed by a 'bl'.

For removing the toc save instruction from the stub, we additionally
check if the symbol is "_mcount". Add the same check here as well.

Also rename is_early_mcount_callsite() to is_mprofile_mcount_callsite()
since that is what is being checked. The use of "early" is misleading
since there is nothing involving this function that qualifies as early.

Fixes: 153086644fd1f ("powerpc/ftrace: Add support for -mprofile-kernel ftrace ABI")
Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
 arch/powerpc/kernel/module_64.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/arch/powerpc/kernel/module_64.c b/arch/powerpc/kernel/module_64.c
index a2636c250b7b..8413be31d6a4 100644
--- a/arch/powerpc/kernel/module_64.c
+++ b/arch/powerpc/kernel/module_64.c
@@ -463,8 +463,11 @@ static unsigned long stub_for_addr(const Elf64_Shdr *sechdrs,
 }
 
 #ifdef CC_USING_MPROFILE_KERNEL
-static bool is_early_mcount_callsite(u32 *instruction)
+static bool is_mprofile_mcount_callsite(const char *name, u32 *instruction)
 {
+	if (strcmp("_mcount", name))
+		return false;
+
 	/*
 	 * Check if this is one of the -mprofile-kernel sequences.
 	 */
@@ -496,8 +499,7 @@ static void squash_toc_save_inst(const char *name, unsigned long addr)
 #else
 static void squash_toc_save_inst(const char *name, unsigned long addr) { }
 
-/* without -mprofile-kernel, mcount calls are never early */
-static bool is_early_mcount_callsite(u32 *instruction)
+static bool is_mprofile_mcount_callsite(const char *name, u32 *instruction)
 {
 	return false;
 }
@@ -505,11 +507,11 @@ static bool is_early_mcount_callsite(u32 *instruction)
 
 /* We expect a noop next: if it is, replace it with instruction to
    restore r2. */
-static int restore_r2(u32 *instruction, struct module *me)
+static int restore_r2(const char *name, u32 *instruction, struct module *me)
 {
 	u32 *prev_insn = instruction - 1;
 
-	if (is_early_mcount_callsite(prev_insn))
+	if (is_mprofile_mcount_callsite(name, prev_insn))
 		return 1;
 
 	/*
@@ -650,7 +652,8 @@ int apply_relocate_add(Elf64_Shdr *sechdrs,
 				value = stub_for_addr(sechdrs, value, me);
 				if (!value)
 					return -ENOENT;
-				if (!restore_r2((u32 *)location + 1, me))
+				if (!restore_r2(strtab + sym->st_name,
+							(u32 *)location + 1, me))
 					return -ENOEXEC;
 
 				squash_toc_save_inst(strtab + sym->st_name, value);
-- 
2.16.2

^ permalink raw reply related

* [PATCH v3 6/6] powerpc64/ftrace: Implement support for ftrace_regs_caller()
From: Naveen N. Rao @ 2018-03-22 10:25 UTC (permalink / raw)
  To: Michael Ellerman, Steven Rostedt, Paul Mackerras
  Cc: linuxppc-dev, Anton Blanchard, Nicholas Piggin, sathnaga
In-Reply-To: <cover.1521713762.git.naveen.n.rao@linux.vnet.ibm.com>

With -mprofile-kernel, we always save the full register state in
ftrace_caller(). While this works, this is inefficient if we're not
interested in the register state, such as when we're using the function
tracer.

Rename the existing ftrace_caller() as ftrace_regs_caller() and provide
a simpler implementation for ftrace_caller() that is used when registers
are not required to be saved.

Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
Changes since v2:
- Disable ftrace when asked for, in ftrace_caller().


 arch/powerpc/include/asm/ftrace.h              |   2 -
 arch/powerpc/include/asm/module.h              |   3 +
 arch/powerpc/kernel/module_64.c                |  28 +++-
 arch/powerpc/kernel/trace/ftrace.c             | 184 +++++++++++++++++++++++--
 arch/powerpc/kernel/trace/ftrace_64_mprofile.S |  71 +++++++++-
 5 files changed, 262 insertions(+), 26 deletions(-)

diff --git a/arch/powerpc/include/asm/ftrace.h b/arch/powerpc/include/asm/ftrace.h
index 9abddde372ab..f7a23c2dce74 100644
--- a/arch/powerpc/include/asm/ftrace.h
+++ b/arch/powerpc/include/asm/ftrace.h
@@ -49,8 +49,6 @@
 extern void _mcount(void);
 
 #ifdef CONFIG_DYNAMIC_FTRACE
-# define FTRACE_ADDR ((unsigned long)ftrace_caller)
-# define FTRACE_REGS_ADDR FTRACE_ADDR
 static inline unsigned long ftrace_call_adjust(unsigned long addr)
 {
        /* reloction of mcount call site is the same as the address */
diff --git a/arch/powerpc/include/asm/module.h b/arch/powerpc/include/asm/module.h
index 7e28442827f1..2d16b6d9147d 100644
--- a/arch/powerpc/include/asm/module.h
+++ b/arch/powerpc/include/asm/module.h
@@ -43,6 +43,9 @@ struct mod_arch_specific {
 #ifdef CONFIG_DYNAMIC_FTRACE
 	unsigned long toc;
 	unsigned long tramp;
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
+	unsigned long tramp_regs;
+#endif
 #endif
 
 	/* For module function descriptor dereference */
diff --git a/arch/powerpc/kernel/module_64.c b/arch/powerpc/kernel/module_64.c
index 8413be31d6a4..f7667e2ebfcb 100644
--- a/arch/powerpc/kernel/module_64.c
+++ b/arch/powerpc/kernel/module_64.c
@@ -280,6 +280,10 @@ static unsigned long get_stubs_size(const Elf64_Ehdr *hdr,
 #ifdef CONFIG_DYNAMIC_FTRACE
 	/* make the trampoline to the ftrace_caller */
 	relocs++;
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
+	/* an additional one for ftrace_regs_caller */
+	relocs++;
+#endif
 #endif
 
 	pr_debug("Looks like a total of %lu stubs, max\n", relocs);
@@ -765,7 +769,8 @@ int apply_relocate_add(Elf64_Shdr *sechdrs,
  * via the paca (in r13). The target (ftrace_caller()) is responsible for
  * saving and restoring the toc before returning.
  */
-static unsigned long create_ftrace_stub(const Elf64_Shdr *sechdrs, struct module *me)
+static unsigned long create_ftrace_stub(const Elf64_Shdr *sechdrs,
+				struct module *me, unsigned long addr)
 {
 	struct ppc64_stub_entry *entry;
 	unsigned int i, num_stubs;
@@ -792,9 +797,10 @@ static unsigned long create_ftrace_stub(const Elf64_Shdr *sechdrs, struct module
 	memcpy(entry->jump, stub_insns, sizeof(stub_insns));
 
 	/* Stub uses address relative to kernel toc (from the paca) */
-	reladdr = (unsigned long)ftrace_caller - kernel_toc_addr();
+	reladdr = addr - kernel_toc_addr();
 	if (reladdr > 0x7FFFFFFF || reladdr < -(0x80000000L)) {
-		pr_err("%s: Address of ftrace_caller out of range of kernel_toc.\n", me->name);
+		pr_err("%s: Address of %ps out of range of kernel_toc.\n",
+							me->name, (void *)addr);
 		return 0;
 	}
 
@@ -802,22 +808,30 @@ static unsigned long create_ftrace_stub(const Elf64_Shdr *sechdrs, struct module
 	entry->jump[2] |= PPC_LO(reladdr);
 
 	/* Eventhough we don't use funcdata in the stub, it's needed elsewhere. */
-	entry->funcdata = func_desc((unsigned long)ftrace_caller);
+	entry->funcdata = func_desc(addr);
 	entry->magic = STUB_MAGIC;
 
 	return (unsigned long)entry;
 }
 #else
-static unsigned long create_ftrace_stub(const Elf64_Shdr *sechdrs, struct module *me)
+static unsigned long create_ftrace_stub(const Elf64_Shdr *sechdrs,
+				struct module *me, unsigned long addr)
 {
-	return stub_for_addr(sechdrs, (unsigned long)ftrace_caller, me);
+	return stub_for_addr(sechdrs, addr, me);
 }
 #endif
 
 int module_finalize_ftrace(struct module *mod, const Elf_Shdr *sechdrs)
 {
 	mod->arch.toc = my_r2(sechdrs, mod);
-	mod->arch.tramp = create_ftrace_stub(sechdrs, mod);
+	mod->arch.tramp = create_ftrace_stub(sechdrs, mod,
+					(unsigned long)ftrace_caller);
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
+	mod->arch.tramp_regs = create_ftrace_stub(sechdrs, mod,
+					(unsigned long)ftrace_regs_caller);
+	if (!mod->arch.tramp_regs)
+		return -ENOENT;
+#endif
 
 	if (!mod->arch.tramp)
 		return -ENOENT;
diff --git a/arch/powerpc/kernel/trace/ftrace.c b/arch/powerpc/kernel/trace/ftrace.c
index 80667128db3d..79d2924e75d5 100644
--- a/arch/powerpc/kernel/trace/ftrace.c
+++ b/arch/powerpc/kernel/trace/ftrace.c
@@ -357,6 +357,8 @@ __ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
 {
 	unsigned int op[2];
 	void *ip = (void *)rec->ip;
+	unsigned long entry, ptr, tramp;
+	struct module *mod = rec->arch.mod;
 
 	/* read where this goes */
 	if (probe_kernel_read(op, ip, sizeof(op)))
@@ -368,19 +370,44 @@ __ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
 		return -EINVAL;
 	}
 
-	/* If we never set up a trampoline to ftrace_caller, then bail */
-	if (!rec->arch.mod->arch.tramp) {
+	/* If we never set up ftrace trampoline(s), then bail */
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
+	if (!mod->arch.tramp || !mod->arch.tramp_regs) {
+#else
+	if (!mod->arch.tramp) {
+#endif
 		pr_err("No ftrace trampoline\n");
 		return -EINVAL;
 	}
 
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
+	if (rec->flags & FTRACE_FL_REGS)
+		tramp = mod->arch.tramp_regs;
+	else
+#endif
+		tramp = mod->arch.tramp;
+
+	if (module_trampoline_target(mod, tramp, &ptr)) {
+		pr_err("Failed to get trampoline target\n");
+		return -EFAULT;
+	}
+
+	pr_devel("trampoline target %lx", ptr);
+
+	entry = ppc_global_function_entry((void *)addr);
+	/* This should match what was called */
+	if (ptr != entry) {
+		pr_err("addr %lx does not match expected %lx\n", ptr, entry);
+		return -EINVAL;
+	}
+
 	/* Ensure branch is within 24 bits */
-	if (!create_branch(ip, rec->arch.mod->arch.tramp, BRANCH_SET_LINK)) {
+	if (!create_branch(ip, tramp, BRANCH_SET_LINK)) {
 		pr_err("Branch out of range\n");
 		return -EINVAL;
 	}
 
-	if (patch_branch(ip, rec->arch.mod->arch.tramp, BRANCH_SET_LINK)) {
+	if (patch_branch(ip, tramp, BRANCH_SET_LINK)) {
 		pr_err("REL24 out of range!\n");
 		return -EINVAL;
 	}
@@ -388,14 +415,6 @@ __ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
 	return 0;
 }
 
-#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
-int ftrace_modify_call(struct dyn_ftrace *rec, unsigned long old_addr,
-			unsigned long addr)
-{
-	return ftrace_make_call(rec, addr);
-}
-#endif
-
 #else  /* !CONFIG_PPC64: */
 static int
 __ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
@@ -472,6 +491,137 @@ int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
 #endif /* CONFIG_MODULES */
 }
 
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
+#ifdef CONFIG_MODULES
+static int
+__ftrace_modify_call(struct dyn_ftrace *rec, unsigned long old_addr,
+					unsigned long addr)
+{
+	unsigned int op;
+	unsigned long ip = rec->ip;
+	unsigned long entry, ptr, tramp;
+	struct module *mod = rec->arch.mod;
+
+	/* If we never set up ftrace trampolines, then bail */
+	if (!mod->arch.tramp || !mod->arch.tramp_regs) {
+		pr_err("No ftrace trampoline\n");
+		return -EINVAL;
+	}
+
+	/* read where this goes */
+	if (probe_kernel_read(&op, (void *)ip, sizeof(int))) {
+		pr_err("Fetching opcode failed.\n");
+		return -EFAULT;
+	}
+
+	/* Make sure that that this is still a 24bit jump */
+	if (!is_bl_op(op)) {
+		pr_err("Not expected bl: opcode is %x\n", op);
+		return -EINVAL;
+	}
+
+	/* lets find where the pointer goes */
+	tramp = find_bl_target(ip, op);
+	entry = ppc_global_function_entry((void *)old_addr);
+
+	pr_devel("ip:%lx jumps to %lx", ip, tramp);
+
+	if (tramp != entry) {
+		/* old_addr is not within range, so we must have used a trampoline */
+		if (module_trampoline_target(mod, tramp, &ptr)) {
+			pr_err("Failed to get trampoline target\n");
+			return -EFAULT;
+		}
+
+		pr_devel("trampoline target %lx", ptr);
+
+		/* This should match what was called */
+		if (ptr != entry) {
+			pr_err("addr %lx does not match expected %lx\n", ptr, entry);
+			return -EINVAL;
+		}
+	}
+
+	/* The new target may be within range */
+	if (test_24bit_addr(ip, addr)) {
+		/* within range */
+		if (patch_branch((unsigned int *)ip, addr, BRANCH_SET_LINK)) {
+			pr_err("REL24 out of range!\n");
+			return -EINVAL;
+		}
+
+		return 0;
+	}
+
+	if (rec->flags & FTRACE_FL_REGS)
+		tramp = mod->arch.tramp_regs;
+	else
+		tramp = mod->arch.tramp;
+
+	if (module_trampoline_target(mod, tramp, &ptr)) {
+		pr_err("Failed to get trampoline target\n");
+		return -EFAULT;
+	}
+
+	pr_devel("trampoline target %lx", ptr);
+
+	entry = ppc_global_function_entry((void *)addr);
+	/* This should match what was called */
+	if (ptr != entry) {
+		pr_err("addr %lx does not match expected %lx\n", ptr, entry);
+		return -EINVAL;
+	}
+
+	/* Ensure branch is within 24 bits */
+	if (!create_branch((unsigned int *)ip, tramp, BRANCH_SET_LINK)) {
+		pr_err("Branch out of range\n");
+		return -EINVAL;
+	}
+
+	if (patch_branch((unsigned int *)ip, tramp, BRANCH_SET_LINK)) {
+		pr_err("REL24 out of range!\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+#endif
+
+int ftrace_modify_call(struct dyn_ftrace *rec, unsigned long old_addr,
+			unsigned long addr)
+{
+	unsigned long ip = rec->ip;
+	unsigned int old, new;
+
+	/*
+	 * If the calling address is more that 24 bits away,
+	 * then we had to use a trampoline to make the call.
+	 * Otherwise just update the call site.
+	 */
+	if (test_24bit_addr(ip, addr) && test_24bit_addr(ip, old_addr)) {
+		/* within range */
+		old = ftrace_call_replace(ip, old_addr, 1);
+		new = ftrace_call_replace(ip, addr, 1);
+		return ftrace_modify_code(ip, old, new);
+	}
+
+#ifdef CONFIG_MODULES
+	/*
+	 * Out of range jumps are called from modules.
+	 */
+	if (!rec->arch.mod) {
+		pr_err("No module loaded\n");
+		return -EINVAL;
+	}
+
+	return __ftrace_modify_call(rec, old_addr, addr);
+#else
+	/* We should not get here without modules */
+	return -EINVAL;
+#endif /* CONFIG_MODULES */
+}
+#endif
+
 int ftrace_update_ftrace_func(ftrace_func_t func)
 {
 	unsigned long ip = (unsigned long)(&ftrace_call);
@@ -482,6 +632,16 @@ int ftrace_update_ftrace_func(ftrace_func_t func)
 	new = ftrace_call_replace(ip, (unsigned long)func, 1);
 	ret = ftrace_modify_code(ip, old, new);
 
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
+	/* Also update the regs callback function */
+	if (!ret) {
+		ip = (unsigned long)(&ftrace_regs_call);
+		old = *(unsigned int *)&ftrace_regs_call;
+		new = ftrace_call_replace(ip, (unsigned long)func, 1);
+		ret = ftrace_modify_code(ip, old, new);
+	}
+#endif
+
 	return ret;
 }
 
diff --git a/arch/powerpc/kernel/trace/ftrace_64_mprofile.S b/arch/powerpc/kernel/trace/ftrace_64_mprofile.S
index 9d234835e1e6..f57822aa2fe8 100644
--- a/arch/powerpc/kernel/trace/ftrace_64_mprofile.S
+++ b/arch/powerpc/kernel/trace/ftrace_64_mprofile.S
@@ -20,8 +20,8 @@
 #ifdef CONFIG_DYNAMIC_FTRACE
 /*
  *
- * ftrace_caller() is the function that replaces _mcount() when ftrace is
- * active.
+ * ftrace_caller()/ftrace_regs_caller() is the function that replaces _mcount()
+ * when ftrace is active.
  *
  * We arrive here after a function A calls function B, and we are the trace
  * function for B. When we enter r1 points to A's stack frame, B has not yet
@@ -37,7 +37,7 @@
  * Our job is to save the register state into a struct pt_regs (on the stack)
  * and then arrange for the ftrace function to be called.
  */
-_GLOBAL(ftrace_caller)
+_GLOBAL(ftrace_regs_caller)
 	/* Save the original return address in A's stack frame */
 	std	r0,LRSAVE(r1)
 
@@ -100,8 +100,8 @@ _GLOBAL(ftrace_caller)
 	addi    r6, r1 ,STACK_FRAME_OVERHEAD
 
 	/* ftrace_call(r3, r4, r5, r6) */
-.globl ftrace_call
-ftrace_call:
+.globl ftrace_regs_call
+ftrace_regs_call:
 	bl	ftrace_stub
 	nop
 
@@ -162,6 +162,7 @@ ftrace_call:
 	bne-	livepatch_handler
 #endif
 
+ftrace_caller_common:
 #ifdef CONFIG_FUNCTION_GRAPH_TRACER
 .globl ftrace_graph_call
 ftrace_graph_call:
@@ -182,6 +183,66 @@ ftrace_no_trace:
 	mtlr	r0
 	bctr
 
+_GLOBAL(ftrace_caller)
+	/* Save the original return address in A's stack frame */
+	std	r0, LRSAVE(r1)
+
+	/* Create our stack frame + pt_regs */
+	stdu	r1, -SWITCH_FRAME_SIZE(r1)
+
+	/* Save all gprs to pt_regs */
+	SAVE_8GPRS(3, r1)
+
+	lbz	r3, PACA_FTRACE_DISABLED(r13)
+	cmpdi	r3, 0
+	bne	ftrace_no_trace
+
+	/* Get the _mcount() call site out of LR */
+	mflr	r7
+	std     r7, _NIP(r1)
+
+	/* Save callee's TOC in the ABI compliant location */
+	std	r2, 24(r1)
+	ld	r2, PACATOC(r13)	/* get kernel TOC in r2 */
+
+	addis	r3, r2, function_trace_op@toc@ha
+	addi	r3, r3, function_trace_op@toc@l
+	ld	r5, 0(r3)
+
+	/* Calculate ip from nip-4 into r3 for call below */
+	subi    r3, r7, MCOUNT_INSN_SIZE
+
+	/* Put the original return address in r4 as parent_ip */
+	mr	r4, r0
+
+	/* Set pt_regs to NULL */
+	li	r6, 0
+
+	/* ftrace_call(r3, r4, r5, r6) */
+.globl ftrace_call
+ftrace_call:
+	bl	ftrace_stub
+	nop
+
+	ld	r3, _NIP(r1)
+	mtctr	r3
+
+	/* Restore gprs */
+	REST_8GPRS(3,r1)
+
+	/* Restore callee's TOC */
+	ld	r2, 24(r1)
+
+	/* Pop our stack frame */
+	addi	r1, r1, SWITCH_FRAME_SIZE
+
+	/* Reload original LR */
+	ld	r0, LRSAVE(r1)
+	mtlr	r0
+
+	/* Handle function_graph or go back */
+	b	ftrace_caller_common
+
 #ifdef CONFIG_LIVEPATCH
 	/*
 	 * This function runs in the mcount context, between two functions. As
-- 
2.16.2

^ permalink raw reply related

* [PATCH v3 5/6] powerpc64/ftrace: Use the generic version of ftrace_replace_code()
From: Naveen N. Rao @ 2018-03-22 10:25 UTC (permalink / raw)
  To: Michael Ellerman, Steven Rostedt, Paul Mackerras
  Cc: linuxppc-dev, Anton Blanchard, Nicholas Piggin, sathnaga
In-Reply-To: <cover.1521713762.git.naveen.n.rao@linux.vnet.ibm.com>

Our implementation matches that of the generic version, which also
handles FTRACE_UPDATE_MODIFY_CALL. So, remove our implementation in
favor of the generic version.

Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
 arch/powerpc/kernel/trace/ftrace.c | 36 ------------------------------------
 1 file changed, 36 deletions(-)

diff --git a/arch/powerpc/kernel/trace/ftrace.c b/arch/powerpc/kernel/trace/ftrace.c
index 4741fe112f05..80667128db3d 100644
--- a/arch/powerpc/kernel/trace/ftrace.c
+++ b/arch/powerpc/kernel/trace/ftrace.c
@@ -485,42 +485,6 @@ int ftrace_update_ftrace_func(ftrace_func_t func)
 	return ret;
 }
 
-static int __ftrace_replace_code(struct dyn_ftrace *rec, int enable)
-{
-	unsigned long ftrace_addr = (unsigned long)FTRACE_ADDR;
-	int ret;
-
-	ret = ftrace_update_record(rec, enable);
-
-	switch (ret) {
-	case FTRACE_UPDATE_IGNORE:
-		return 0;
-	case FTRACE_UPDATE_MAKE_CALL:
-		return ftrace_make_call(rec, ftrace_addr);
-	case FTRACE_UPDATE_MAKE_NOP:
-		return ftrace_make_nop(NULL, rec, ftrace_addr);
-	}
-
-	return 0;
-}
-
-void ftrace_replace_code(int enable)
-{
-	struct ftrace_rec_iter *iter;
-	struct dyn_ftrace *rec;
-	int ret;
-
-	for (iter = ftrace_rec_iter_start(); iter;
-	     iter = ftrace_rec_iter_next(iter)) {
-		rec = ftrace_rec_iter_record(iter);
-		ret = __ftrace_replace_code(rec, enable);
-		if (ret) {
-			ftrace_bug(ret, rec);
-			return;
-		}
-	}
-}
-
 /*
  * Use the default ftrace_modify_all_code, but without
  * stop_machine().
-- 
2.16.2

^ permalink raw reply related

* RE: RFC on writel and writel_relaxed
From: David Laight @ 2018-03-22 10:37 UTC (permalink / raw)
  To: 'Oliver'
  Cc: Sinan Kaya, linux-rdma@vger.kernel.org,
	open list:LINUX FOR POWERPC (32-BIT AND 64-BIT)
In-Reply-To: <CAOSf1CHTouXQ+sPxu7ySCtw--maef+knFC9bCQ7rpuk-hf-puQ@mail.gmail.com>

RnJvbTogT2xpdmVyDQo+IFNlbnQ6IDIyIE1hcmNoIDIwMTggMDU6MjQNCi4uLg0KPiA+IE5vIGxl
c3MgcGFpbmZ1bCB3YXMgZG9pbmcgYSBieXRlc3dhcHBpbmcgd3JpdGUgdG8gbm9ybWFsIG1lbW9y
eS4NCj4gDQo+IFdoYXQgd2FzIHRoZSBwcm9ibGVtPyBUaGUgcmV2ZXJzZSBpbmRleGVkIGxvYWQv
c3RvcmUgaW5zdHJ1Y3Rpb25zIGFyZQ0KPiBhIGxpdHRsZSBhd2t3YXJkIHRvIHVzZSwgYnV0IHRo
ZXkgd29yay4uLg0KDQpGaW5kaW5nIHNvbWV0aGluZyB0aGF0IHdvdWxkIGdlbmVyYXRlIHRoZSBy
aWdodCBpbnN0cnVjdGlvbiB3aXRob3V0IGFueQ0KYmFycmllcnMuDQpJU1RSIHdyaXRpbmcgbXkg
b3duIGFzbSBwYXR0ZXJuLg0KDQoJRGF2aWQNCg0K

^ permalink raw reply

* [PATCH] powerpc: Fix smp_wmb barrier definition use use lwsync consistently
From: Nicholas Piggin @ 2018-03-22 10:41 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Nicholas Piggin, Anton Blanchard

asm/barrier.h is not always included after asm/synch.h, which meant
it was missing __SUBARCH_HAS_LWSYNC, so in some files smp_wmb() would
be eieio when it should be lwsync. kernel/time/hrtimer.c is one case.

__SUBARCH_HAS_LWSYNC is only used in one place, so just fold it in
to where it's used. Previously with my small simulator config, 377
instances of eieio in the tree. After this patch there are 55.

Cc: Anton Blanchard <anton@samba.org>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/include/asm/barrier.h | 3 ++-
 arch/powerpc/include/asm/synch.h   | 4 ----
 2 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/include/asm/barrier.h b/arch/powerpc/include/asm/barrier.h
index 10daa1d56e0a..c7c63959ba91 100644
--- a/arch/powerpc/include/asm/barrier.h
+++ b/arch/powerpc/include/asm/barrier.h
@@ -35,7 +35,8 @@
 #define rmb()  __asm__ __volatile__ ("sync" : : : "memory")
 #define wmb()  __asm__ __volatile__ ("sync" : : : "memory")
 
-#ifdef __SUBARCH_HAS_LWSYNC
+/* The sub-arch has lwsync */
+#if defined(__powerpc64__) || defined(CONFIG_PPC_E500MC)
 #    define SMPWMB      LWSYNC
 #else
 #    define SMPWMB      eieio
diff --git a/arch/powerpc/include/asm/synch.h b/arch/powerpc/include/asm/synch.h
index 63e7f5a1f105..6ec546090ba1 100644
--- a/arch/powerpc/include/asm/synch.h
+++ b/arch/powerpc/include/asm/synch.h
@@ -6,10 +6,6 @@
 #include <linux/stringify.h>
 #include <asm/feature-fixups.h>
 
-#if defined(__powerpc64__) || defined(CONFIG_PPC_E500MC)
-#define __SUBARCH_HAS_LWSYNC
-#endif
-
 #ifndef __ASSEMBLY__
 extern unsigned int __start___lwsync_fixup, __stop___lwsync_fixup;
 extern void do_lwsync_fixups(unsigned long value, void *fixup_start,
-- 
2.16.1

^ permalink raw reply related

* [PATCH 0/3] Add support to disable sensor groups in P9
From: Shilpasri G Bhat @ 2018-03-22 10:54 UTC (permalink / raw)
  To: mpe, linux
  Cc: linuxppc-dev, linux-hwmon, linux-kernel, stewart,
	Shilpasri G Bhat

This patch series adds support to enable/disable OCC based
inband-sensor groups at runtime. The environmental sensor groups are
managed in HWMON and the remaining platform specific sensor groups are
managed in /sys/firmware/opal.

The firmware changes required for this patch is posted below:
https://lists.ozlabs.org/pipermail/skiboot/2018-March/010812.html

Shilpasri G Bhat (3):
  powernv:opal-sensor-groups: Add support to enable sensor groups
  hwmon: ibmpowernv: Add attributes to enable/disable sensor groups
  powernv: opal-sensor-groups: Add attributes to disable/enable sensors

 .../ABI/testing/sysfs-firmware-opal-sensor-groups  |  34 ++++++
 Documentation/hwmon/ibmpowernv                     |  31 ++++-
 arch/powerpc/include/asm/opal-api.h                |   4 +-
 arch/powerpc/include/asm/opal.h                    |   2 +
 .../powerpc/platforms/powernv/opal-sensor-groups.c | 104 ++++++++++++-----
 arch/powerpc/platforms/powernv/opal-wrappers.S     |   1 +
 drivers/hwmon/ibmpowernv.c                         | 127 +++++++++++++++++++--
 7 files changed, 265 insertions(+), 38 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-firmware-opal-sensor-groups

-- 
1.8.3.1

^ permalink raw reply

* [PATCH 1/3] powernv:opal-sensor-groups: Add support to enable sensor groups
From: Shilpasri G Bhat @ 2018-03-22 10:54 UTC (permalink / raw)
  To: mpe, linux
  Cc: linuxppc-dev, linux-hwmon, linux-kernel, stewart,
	Shilpasri G Bhat
In-Reply-To: <1521716075-807-1-git-send-email-shilpa.bhat@linux.vnet.ibm.com>

Adds support to enable/disable a sensor group at runtime. This
can be used to select the sensor groups that needs to be copied to
main memory by OCC. Sensor groups like power, temperature, current,
voltage, frequency, utilization can be enabled/disabled at runtime.

Signed-off-by: Shilpasri G Bhat <shilpa.bhat@linux.vnet.ibm.com>
---
 arch/powerpc/include/asm/opal-api.h                |  4 +++-
 arch/powerpc/include/asm/opal.h                    |  2 ++
 .../powerpc/platforms/powernv/opal-sensor-groups.c | 28 ++++++++++++++++++++++
 arch/powerpc/platforms/powernv/opal-wrappers.S     |  1 +
 4 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/include/asm/opal-api.h b/arch/powerpc/include/asm/opal-api.h
index 94bd1bf..b6bbbd8 100644
--- a/arch/powerpc/include/asm/opal-api.h
+++ b/arch/powerpc/include/asm/opal-api.h
@@ -204,7 +204,9 @@
 #define OPAL_NPU_SPA_SETUP			159
 #define OPAL_NPU_SPA_CLEAR_CACHE		160
 #define OPAL_NPU_TL_SET				161
-#define OPAL_LAST				161
+#define OPAL_SENSOR_READ_U64			162
+#define OPAL_SENSOR_GROUP_ENABLE		163
+#define OPAL_LAST				163
 
 /* Device tree flags */
 
diff --git a/arch/powerpc/include/asm/opal.h b/arch/powerpc/include/asm/opal.h
index 12e70fb..0d37bf2 100644
--- a/arch/powerpc/include/asm/opal.h
+++ b/arch/powerpc/include/asm/opal.h
@@ -286,6 +286,7 @@ int64_t opal_imc_counters_init(uint32_t type, uint64_t address,
 int opal_get_power_shift_ratio(u32 handle, int token, u32 *psr);
 int opal_set_power_shift_ratio(u32 handle, int token, u32 psr);
 int opal_sensor_group_clear(u32 group_hndl, int token);
+int opal_sensor_group_enable(u32 group_hndl, int token, bool enable);
 
 s64 opal_signal_system_reset(s32 cpu);
 
@@ -318,6 +319,7 @@ extern int opal_message_notifier_unregister(enum opal_msg_type msg_type,
 extern int opal_async_wait_response_interruptible(uint64_t token,
 		struct opal_msg *msg);
 extern int opal_get_sensor_data(u32 sensor_hndl, u32 *sensor_data);
+extern int sensor_group_enable(u32 grp_hndl, bool enable);
 
 struct rtc_time;
 extern unsigned long opal_get_boot_time(void);
diff --git a/arch/powerpc/platforms/powernv/opal-sensor-groups.c b/arch/powerpc/platforms/powernv/opal-sensor-groups.c
index 7e5a235..5b53f58 100644
--- a/arch/powerpc/platforms/powernv/opal-sensor-groups.c
+++ b/arch/powerpc/platforms/powernv/opal-sensor-groups.c
@@ -32,6 +32,34 @@ struct sg_attr {
 	struct sg_attr *sgattrs;
 } *sgs;
 
+int sensor_group_enable(u32 handle, bool enable)
+{
+	struct opal_msg msg;
+	int token, ret;
+
+	token = opal_async_get_token_interruptible();
+	if (token < 0)
+		return token;
+
+	ret = opal_sensor_group_enable(handle, token, enable);
+	if (ret == OPAL_ASYNC_COMPLETION) {
+		ret = opal_async_wait_response(token, &msg);
+		if (ret) {
+			pr_devel("Failed to wait for the async response\n");
+			ret = -EIO;
+			goto out;
+		}
+		ret = opal_error_code(opal_get_async_rc(msg));
+	} else {
+		ret = opal_error_code(ret);
+	}
+
+out:
+	opal_async_release_token(token);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(sensor_group_enable);
+
 static ssize_t sg_store(struct kobject *kobj, struct kobj_attribute *attr,
 			const char *buf, size_t count)
 {
diff --git a/arch/powerpc/platforms/powernv/opal-wrappers.S b/arch/powerpc/platforms/powernv/opal-wrappers.S
index 1b2936b..90c2b40 100644
--- a/arch/powerpc/platforms/powernv/opal-wrappers.S
+++ b/arch/powerpc/platforms/powernv/opal-wrappers.S
@@ -323,3 +323,4 @@ OPAL_CALL(opal_sensor_group_clear,		OPAL_SENSOR_GROUP_CLEAR);
 OPAL_CALL(opal_npu_spa_setup,			OPAL_NPU_SPA_SETUP);
 OPAL_CALL(opal_npu_spa_clear_cache,		OPAL_NPU_SPA_CLEAR_CACHE);
 OPAL_CALL(opal_npu_tl_set,			OPAL_NPU_TL_SET);
+OPAL_CALL(opal_sensor_group_enable,		OPAL_SENSOR_GROUP_ENABLE);
-- 
1.8.3.1

^ permalink raw reply related

* [PATCH 3/3] powernv: opal-sensor-groups: Add attributes to disable/enable sensors
From: Shilpasri G Bhat @ 2018-03-22 10:54 UTC (permalink / raw)
  To: mpe, linux
  Cc: linuxppc-dev, linux-hwmon, linux-kernel, stewart,
	Shilpasri G Bhat
In-Reply-To: <1521716075-807-1-git-send-email-shilpa.bhat@linux.vnet.ibm.com>

This patch provides support to disable and enable plaform specific
sensor groups like performance, utilization and frequency.

Signed-off-by: Shilpasri G Bhat <shilpa.bhat@linux.vnet.ibm.com>
---
 .../ABI/testing/sysfs-firmware-opal-sensor-groups  | 34 +++++++++
 .../powerpc/platforms/powernv/opal-sensor-groups.c | 80 +++++++++++++---------
 2 files changed, 83 insertions(+), 31 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-firmware-opal-sensor-groups

diff --git a/Documentation/ABI/testing/sysfs-firmware-opal-sensor-groups b/Documentation/ABI/testing/sysfs-firmware-opal-sensor-groups
new file mode 100644
index 0000000..484ff1c
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-firmware-opal-sensor-groups
@@ -0,0 +1,34 @@
+What:		/sys/firmware/opal/sensor_groups
+Date:		March 2018
+Contact:	Linux for PowerPC mailing list <linuxppc-dev@ozlabs.org>
+Description:	Sensor groups directory for POWER9 powernv servers
+
+		Each folder in this directory contains a sensor group
+		which are classified based on type of the sensor
+		like power, temperature, frequency, current, etc. They
+		can also indicate the group of sensors belonging to
+		different owners like CSM, Profiler, Job-Scheduler
+
+What:		/sys/firmware/opal/sensor_groups/<sensor_group_name>/clear
+Date:		March 2018
+Contact:	Linux for PowerPC mailing list <linuxppc-dev@ozlabs.org>
+Description:	Sysfs file to clear the min-max of all the sensors
+		belonging to the group.
+
+		Writing 1 to this file will clear the minimum and
+		maximum values of all the sensors in the group. The
+		min-max of a sensor is the historical minimum and
+		maximum value of the sensor cached by OCC.
+
+What:		/sys/firmware/opal/sensor_groups/<sensor_group_name>/enable
+Date:		March 2018
+Contact:	Linux for PowerPC mailing list <linuxppc-dev@ozlabs.org>
+Description:	Sysfs file to enable/disable the sensor-group
+
+		Writing 0 value to this file will disable the copying
+		of the sensor-group to main memory by OCC. And writing
+		1 to this file will enable the sensor-group copying.
+		By default all the sensor-groups are enabled and will
+		be copied to main memory. This file can be used to
+		increase the update frequency of selective
+		sensor-groups.
diff --git a/arch/powerpc/platforms/powernv/opal-sensor-groups.c b/arch/powerpc/platforms/powernv/opal-sensor-groups.c
index 5b53f58..67f28b2 100644
--- a/arch/powerpc/platforms/powernv/opal-sensor-groups.c
+++ b/arch/powerpc/platforms/powernv/opal-sensor-groups.c
@@ -24,6 +24,7 @@
 struct sg_attr {
 	u32 handle;
 	struct kobj_attribute attr;
+	u32 opal_no;
 };
 
 static struct sensor_group {
@@ -60,34 +61,17 @@ int sensor_group_enable(u32 handle, bool enable)
 }
 EXPORT_SYMBOL_GPL(sensor_group_enable);
 
-static ssize_t sg_store(struct kobject *kobj, struct kobj_attribute *attr,
-			const char *buf, size_t count)
+static int sensor_group_clear(u32 handle)
 {
-	struct sg_attr *sattr = container_of(attr, struct sg_attr, attr);
 	struct opal_msg msg;
-	u32 data;
-	int ret, token;
-
-	ret = kstrtoint(buf, 0, &data);
-	if (ret)
-		return ret;
-
-	if (data != 1)
-		return -EINVAL;
+	int token, ret;
 
 	token = opal_async_get_token_interruptible();
-	if (token < 0) {
-		pr_devel("Failed to get token\n");
+	if (token < 0)
 		return token;
-	}
 
-	ret = mutex_lock_interruptible(&sg_mutex);
-	if (ret)
-		goto out_token;
-
-	ret = opal_sensor_group_clear(sattr->handle, token);
-	switch (ret) {
-	case OPAL_ASYNC_COMPLETION:
+	ret = opal_sensor_group_clear(handle, token);
+	if (ret == OPAL_ASYNC_COMPLETION) {
 		ret = opal_async_wait_response(token, &msg);
 		if (ret) {
 			pr_devel("Failed to wait for the async response\n");
@@ -95,20 +79,48 @@ static ssize_t sg_store(struct kobject *kobj, struct kobj_attribute *attr,
 			goto out;
 		}
 		ret = opal_error_code(opal_get_async_rc(msg));
-		if (!ret)
-			ret = count;
+	} else {
+		ret = opal_error_code(ret);
+	}
+
+out:
+	opal_async_release_token(token);
+	return ret;
+}
+
+static ssize_t sg_store(struct kobject *kobj, struct kobj_attribute *attr,
+			const char *buf, size_t count)
+{
+	struct sg_attr *sattr = container_of(attr, struct sg_attr, attr);
+	u32 data;
+	int ret;
+
+	ret = kstrtoint(buf, 0, &data);
+	if (ret)
+		return ret;
+
+	ret = mutex_lock_interruptible(&sg_mutex);
+	if (ret)
+		return ret;
+
+	ret = -EINVAL;
+	switch (sattr->opal_no) {
+	case OPAL_SENSOR_GROUP_CLEAR:
+		if (data == 1)
+			ret = sensor_group_clear(sattr->handle);
 		break;
-	case OPAL_SUCCESS:
-		ret = count;
+	case OPAL_SENSOR_GROUP_ENABLE:
+		if (data == 0 || data == 1)
+			ret = sensor_group_enable(sattr->handle, data);
 		break;
 	default:
-		ret = opal_error_code(ret);
+		break;
 	}
 
-out:
+	if (!ret)
+		ret = count;
+
 	mutex_unlock(&sg_mutex);
-out_token:
-	opal_async_release_token(token);
 	return ret;
 }
 
@@ -118,12 +130,14 @@ static ssize_t sg_store(struct kobject *kobj, struct kobj_attribute *attr,
 	ssize_t (*store)(struct kobject *kobj, struct kobj_attribute *attr,
 			const char *buf, size_t count);
 } ops_info[] = {
-	{ OPAL_SENSOR_GROUP_CLEAR, "clear", sg_store },
+	{ OPAL_SENSOR_GROUP_CLEAR, "clear", sg_store},
+	{ OPAL_SENSOR_GROUP_ENABLE, "enable", sg_store},
 };
 
 static void add_attr(int handle, struct sg_attr *attr, int index)
 {
 	attr->handle = handle;
+	attr->opal_no = ops_info[index].opal_no;
 	sysfs_attr_init(&attr->attr.attr);
 	attr->attr.attr.name = ops_info[index].attr_name;
 	attr->attr.attr.mode = 0220;
@@ -186,6 +200,10 @@ void __init opal_sensor_groups_init(void)
 		const __be32 *ops;
 		u32 sgid, len, nr_attrs, chipid;
 
+		/* Skip sensor groups that are handled in HWMON */
+		if (of_device_is_compatible(node, "ibm,opal-sensor"))
+			continue;
+
 		ops = of_get_property(node, "ops", &len);
 		if (!ops)
 			continue;
-- 
1.8.3.1

^ permalink raw reply related

* [PATCH 2/3] hwmon: ibmpowernv: Add attributes to enable/disable sensor groups
From: Shilpasri G Bhat @ 2018-03-22 10:54 UTC (permalink / raw)
  To: mpe, linux
  Cc: linuxppc-dev, linux-hwmon, linux-kernel, stewart,
	Shilpasri G Bhat
In-Reply-To: <1521716075-807-1-git-send-email-shilpa.bhat@linux.vnet.ibm.com>

On-Chip-Controller(OCC) is an embedded micro-processor in POWER9 chip
which measures various system and chip level sensors. These sensors
comprises of environmental sensors (like power, temperature, current
and voltage) and performance sensors (like utilization, frequency).
All these sensors are copied to main memory at a regular interval of
100ms. OCC provides a way to select a group of sensors that is copied
to the main memory to increase the update frequency of selected sensor
groups. When a sensor-group is disabled, OCC will not copy it to main
memory and those sensors read 0 values.

This patch provides support for enabling/disabling the sensor groups
like power, temperature, current and voltage. This patch adds new
sysfs attributes to disable and enable them.

Signed-off-by: Shilpasri G Bhat <shilpa.bhat@linux.vnet.ibm.com>
---

- Should the new attributes be added to hwmon_chip_attrs[]?

 Documentation/hwmon/ibmpowernv |  31 +++++++++-
 drivers/hwmon/ibmpowernv.c     | 127 ++++++++++++++++++++++++++++++++++++++---
 2 files changed, 150 insertions(+), 8 deletions(-)

diff --git a/Documentation/hwmon/ibmpowernv b/Documentation/hwmon/ibmpowernv
index 8826ba2..5e510fd 100644
--- a/Documentation/hwmon/ibmpowernv
+++ b/Documentation/hwmon/ibmpowernv
@@ -38,4 +38,33 @@ tempX_max		Threshold ambient temperature for alert generation.
 inX_input		Measured power supply voltage
 inX_fault		0: No fail condition.
 			1: Failing power supply.
-power1_input		System power consumption (microWatt)
+powerX_input		Power consumption (microWatt)
+currX_input		Measured current
+
+Sysfs attributes in POWER9
+---------------------------
+
+On-Chip-Controller(OCC) copies the sensors to main memory. The
+environmental sensor groups can be dynamically configured by writing
+to the below sysfs files. Writing to this file configures the sensor
+group update for the all the OCC chips in the system.
+
+power_enable		Disable/enable copying of power sensors
+			0: Disable
+			1: Enable
+			RO
+
+in_enable		Disable/enable copying of voltage sensors
+			0: Disable
+			1: Enable
+			RO
+
+curr_enable		Disable/enable copying of current sensors
+			0: Disable
+			1: Enable
+			RO
+
+temp_enable		Disable/enable copying of temperature sensors
+			0: Disable
+			1: Enable
+			RO
diff --git a/drivers/hwmon/ibmpowernv.c b/drivers/hwmon/ibmpowernv.c
index 5ccdd0b..696ccbc 100644
--- a/drivers/hwmon/ibmpowernv.c
+++ b/drivers/hwmon/ibmpowernv.c
@@ -90,6 +90,16 @@ struct sensor_data {
 	struct device_attribute dev_attr;
 };
 
+struct sensor_group_data {
+	u32 nr_ids;
+	u32 *id;
+	char name[MAX_ATTR_LEN];
+	struct device_attribute dev_attr;
+};
+
+DEFINE_MUTEX(sensor_groups_mutex);
+static int nr_sg_attr_count;
+
 struct platform_data {
 	const struct attribute_group *attr_groups[MAX_SENSOR_TYPE + 1];
 	u32 sensors_count; /* Total count of sensors from each group */
@@ -117,6 +127,42 @@ static ssize_t show_sensor(struct device *dev, struct device_attribute *devattr,
 	return sprintf(buf, "%u\n", x);
 }
 
+static ssize_t store_enable(struct device *dev,
+			    struct device_attribute *devattr,
+			    const char *buf, size_t count)
+{
+	struct sensor_group_data *sdata = container_of(devattr,
+						       struct sensor_group_data,
+						       dev_attr);
+	int ret;
+	u32 data;
+	int i;
+
+	ret = kstrtoint(buf, 0, &data);
+	if (ret)
+		return ret;
+
+	if (data != 0 && data != 1)
+		return -EIO;
+
+	ret = mutex_lock_interruptible(&sensor_groups_mutex);
+	if (ret)
+		return ret;
+
+	for (i = 0; i < sdata->nr_ids; i++) {
+		ret =  sensor_group_enable(sdata->id[i], data);
+		if (ret) {
+			ret = -EIO;
+			goto out;
+		}
+	}
+
+	ret = count;
+out:
+	mutex_unlock(&sensor_groups_mutex);
+	return ret;
+}
+
 static ssize_t show_label(struct device *dev, struct device_attribute *devattr,
 			  char *buf)
 {
@@ -322,6 +368,21 @@ static int populate_attr_groups(struct platform_device *pdev)
 
 	of_node_put(opal);
 
+	opal = of_find_node_by_path("/ibm,opal/sensor-groups");
+	for (type = 0; type < MAX_SENSOR_TYPE; type++)
+		for_each_child_of_node(opal, np) {
+			enum sensors ctype;
+
+			ctype = get_sensor_type(np);
+			if (ctype == type) {
+				sensor_groups[ctype].attr_count++;
+				nr_sg_attr_count++;
+				break;
+			}
+		}
+
+	of_node_put(opal);
+
 	for (type = 0; type < MAX_SENSOR_TYPE; type++) {
 		sensor_groups[type].group.attrs = devm_kzalloc(&pdev->dev,
 					sizeof(struct attribute *) *
@@ -400,19 +461,25 @@ static int create_device_attrs(struct platform_device *pdev)
 	const struct attribute_group **pgroups = pdata->attr_groups;
 	struct device_node *opal, *np;
 	struct sensor_data *sdata;
+	struct sensor_group_data *sgdata;
 	u32 sensor_id;
 	enum sensors type;
 	u32 count = 0;
 	int err = 0;
+	int nr_id[MAX_SENSOR_TYPE] = {0};
 
-	opal = of_find_node_by_path("/ibm,opal/sensors");
-	sdata = devm_kzalloc(&pdev->dev, pdata->sensors_count * sizeof(*sdata),
+	sdata = devm_kzalloc(&pdev->dev, (pdata->sensors_count -
+			     nr_sg_attr_count) * sizeof(*sdata),
 			     GFP_KERNEL);
-	if (!sdata) {
-		err = -ENOMEM;
-		goto exit_put_node;
-	}
+	if (!sdata)
+		return -ENOMEM;
+
+	sgdata = devm_kzalloc(&pdev->dev, nr_sg_attr_count * sizeof(*sgdata),
+			      GFP_KERNEL);
+	if (!sgdata)
+		return -ENOMEM;
 
+	opal = of_find_node_by_path("/ibm,opal/sensors");
 	for_each_child_of_node(opal, np) {
 		const char *attr_name;
 		u32 opal_index;
@@ -496,7 +563,53 @@ static int create_device_attrs(struct platform_device *pdev)
 		}
 	}
 
-exit_put_node:
+	of_node_put(opal);
+
+	opal = of_find_node_by_path("/ibm,opal/sensor-groups");
+	for_each_child_of_node(opal, np) {
+		type = get_sensor_type(np);
+		if (type == MAX_SENSOR_TYPE)
+			continue;
+		nr_id[type]++;
+	}
+
+	count = 0;
+	for (type = 0; type < MAX_SENSOR_TYPE; type++) {
+		int c = 0;
+
+		if (!nr_id[type])
+			continue;
+
+		sgdata[count].nr_ids = nr_id[type];
+		sgdata[count].id = devm_kzalloc(&pdev->dev, nr_id[type] *
+					    sizeof(u32), GFP_KERNEL);
+		if (!sgdata[count].id)
+			return -ENOMEM;
+
+		for_each_child_of_node(opal, np) {
+			enum sensors ctype;
+
+			ctype = get_sensor_type(np);
+			if (type != ctype)
+				continue;
+
+			if (of_property_read_u32(np, "sensor-group-id",
+						&sensor_id))
+				continue;
+
+			sgdata[count].id[c++] = sensor_id;
+		}
+		snprintf(sgdata[count].name, MAX_ATTR_LEN, "%s_enable",
+			 sensor_groups[type].name);
+		sysfs_attr_init(&sgdata[count].dev_attr.attr);
+		sgdata[count].dev_attr.attr.name = sgdata[count].name;
+		sgdata[count].dev_attr.attr.mode = 00220;
+		sgdata[count].dev_attr.store = store_enable;
+		pgroups[type]->attrs[sensor_groups[type].attr_count++] =
+			&sgdata[count].dev_attr.attr;
+		count++;
+	}
+
 	of_node_put(opal);
 	return err;
 }
-- 
1.8.3.1

^ permalink raw reply related

* Re: RFC on writel and writel_relaxed
From: Gabriel Paubert @ 2018-03-22 11:25 UTC (permalink / raw)
  To: Oliver
  Cc: David Laight, Sinan Kaya, linux-rdma@vger.kernel.org,
	open list:LINUX FOR POWERPC (32-BIT AND 64-BIT)
In-Reply-To: <CAOSf1CE+gsoZB-YNYdx=oNmrdCje4yxiz76hSb4UQ4F5cL8SqQ@mail.gmail.com>

On Thu, Mar 22, 2018 at 08:25:43PM +1100, Oliver wrote:
> On Thu, Mar 22, 2018 at 7:20 PM, Gabriel Paubert <paubert@iram.es> wrote:
> > On Thu, Mar 22, 2018 at 04:24:24PM +1100, Oliver wrote:
> >> On Thu, Mar 22, 2018 at 1:35 AM, David Laight <David.Laight@aculab.com> wrote:
> >> >> x86 has compiler barrier inside the relaxed() API so that code does not
> >> >> get reordered. ARM64 architecturally guarantees device writes to be observed
> >> >> in order.
> >> >
> >> > There are places where you don't even need a compile barrier between
> >> > every write.
> >> >
> >> > I had horrid problems getting some ppc code (for a specific embedded SoC)
> >> > optimised to have no extra barriers.
> >> > I ended up just writing through 'pointer to volatile' and adding an
> >> > explicit 'eieio' between the block of writes and status read.
> >>
> >> This is what you are supposed to do. For accesses to MMIO (cache
> >> inhibited + guarded) storage the Power ISA guarantees that load-load
> >> and store-store pairs of accesses will always occur in program order,
> >> but there's no implicit ordering between load-store or store-load
> >
> > And even for load store, eieio is not always necessary, in the important
> > case of reading and writing to the same address, when modifying bits in
> > a control register for example.
> >
> > Typically also loads will be moved ahead of stores, but not the other
> > way around, so in practice you won't notice a missed eieio in this case.
> > This does not mean you should not insert it.
> 
> Yep, but it doesn't really help us here. The generic accessors need to cope
> with the general case.

A generic accessor for modifying fields in a device register might be an 
useful addition to the current set. This is a fairly frequent operation.

Actually I did add macros to do exactly this in drivers for our own 
hardware here almost 20 years ago. I was fed up with writing
writel(readl(reg) & mask | value, reg), especially when reg was not
that simple (one device had over 100 registers). The macros obviously 
guaranteed that both accesses would be to the same register, something 
easy to get wrong with cut and paste.

> 
> >> pairs. In those cases you need an explicit eieio barrier between the
> >> two accesses. At the HW level you can think of the CPU as having
> >> separate queues for MMIO loads and stores. Accesses will be added to
> >> the respective queue in program order, but there's no synchronisation
> >> between the two queues. If the CPU is doing write combining it's easy
> >> to imagine the whole store queue being emptied in one big gulp before
> >> the load queue is even touched.
> >
> > Is write combining allowed on guarded storage?
> >
> > <Looking at docs>
> > From PowerISA_V3.0.pdf, Book2, section 1.6.2 "Caching inhibited":
> >
> > "No combining occurs if the storage is also Guarded"
> 
> Yeah it's not allowed. That's what I get for handwaving examples ;)

At least it means that, for cache-inhibited guarded storage, there is a 
one to one correspondance between instructions and bus cycles. The only 
issue left is ordering ;)

	Gabriel

^ permalink raw reply

* Re: [PATCH V5 4/4] powerpc/mm/hash: Don't memset pgd table if not needed
From: Michael Ellerman @ 2018-03-22 11:54 UTC (permalink / raw)
  To: Aneesh Kumar K.V, benh, paulus; +Cc: linuxppc-dev, Aneesh Kumar K.V
In-Reply-To: <20180318110558.30493-5-aneesh.kumar@linux.vnet.ibm.com>

"Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> writes:

> We need to zero-out pgd table only if we share the slab cache with pud/pmd
> level caches. With the support of 4PB, we don't share the slab cache anymore.
> Instead of removing the code completely hide it within an #ifdef. We don't need
> to do this with any other page table level, because they all allocate table
> of double the size and we take of initializing the first half corrrectly during
> page table zap.
>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> ---
>  arch/powerpc/include/asm/book3s/64/pgalloc.h | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/include/asm/book3s/64/pgalloc.h b/arch/powerpc/include/asm/book3s/64/pgalloc.h
> index 4746bc68d446..07f0dbac479f 100644
> --- a/arch/powerpc/include/asm/book3s/64/pgalloc.h
> +++ b/arch/powerpc/include/asm/book3s/64/pgalloc.h
> @@ -80,8 +80,19 @@ static inline pgd_t *pgd_alloc(struct mm_struct *mm)
>  
>  	pgd = kmem_cache_alloc(PGT_CACHE(PGD_INDEX_SIZE),
>  			       pgtable_gfp_flags(mm, GFP_KERNEL));
> +	/*
> +	 * With hugetlb, we don't clear the second half of the page table.
> +	 * If we share the same slab cache with the pmd or pud level table,
> +	 * we need to make sure we zero out the full table on alloc.
> +	 * With 4K we don't store slot in the second half. Hence we don't
> +	 * need to do this for 4k.
> +	 */
> +#if (H_PGD_INDEX_SIZE == H_PUD_CACHE_INDEX) || \
> +		(H_PGD_INDEX_SIZE == H_PMD_CACHE_INDEX)
> +#if defined(CONFIG_HUGETLB_PAGE) && defined(CONFIG_PPC_64K_PAGES)
>  	memset(pgd, 0, PGD_TABLE_SIZE);
> -
> +#endif
> +#endif

As discussed I changed this to:

#if defined(CONFIG_HUGETLB_PAGE) && defined(CONFIG_PPC_64K_PAGES) && \
	((H_PGD_INDEX_SIZE == H_PUD_CACHE_INDEX) ||		     \
	 (H_PGD_INDEX_SIZE == H_PMD_CACHE_INDEX))
	memset(pgd, 0, PGD_TABLE_SIZE);
#endif

cheers

^ permalink raw reply

* Re: [PATCH V5 3/4] powerpc/mm/hash64: Increase the VA range
From: Michael Ellerman @ 2018-03-22 13:00 UTC (permalink / raw)
  To: Aneesh Kumar K.V, benh, paulus; +Cc: linuxppc-dev, Aneesh Kumar K.V
In-Reply-To: <20180318110558.30493-4-aneesh.kumar@linux.vnet.ibm.com>

"Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> writes:
> diff --git a/arch/powerpc/mm/init_64.c b/arch/powerpc/mm/init_64.c
> index fdb424a29f03..63470b06c502 100644
> --- a/arch/powerpc/mm/init_64.c
> +++ b/arch/powerpc/mm/init_64.c
> @@ -68,12 +68,6 @@
>  
>  #include "mmu_decl.h"
>  
> -#ifdef CONFIG_PPC_BOOK3S_64
> -#if H_PGTABLE_RANGE > USER_VSID_RANGE
> -#warning Limited user VSID range means pagetable space is wasted
> -#endif
> -#endif /* CONFIG_PPC_BOOK3S_64 */

As discussed I updated this instead of dropping it, and moved it to
pgtable-hash64.c, where it doesn't need the ifdef:

#if H_PGTABLE_RANGE > (USER_VSID_RANGE * (TASK_SIZE_USER64 / TASK_CONTEXT_SIZE))
#warning Limited user VSID range means pagetable space is wasted
#endif

cheers

^ permalink raw reply

* [RFC][PATCH] powerpc/64s: rewriting interrupt entry code
From: Nicholas Piggin @ 2018-03-22 13:05 UTC (permalink / raw)
  To: linuxppc-dev

Long long post ahead...

I've been playing with rewriting interrupt entry code, this is really
rough patch so far, but it boots mambo. I'll just post it now to get
opinions on the approach.

This implements a new set of exception macros, converts the decrementer
to use them (it's maskable so it covers more cases).

Overall two main points to this work. First is to make the code easier
to understand and hack on, second is to improve performance of the end
result.

For the former case, gas macros are used rather than cpp macros as the
main building block. IMO this really turns out a lot nicer for a few
reasons -- we can conditionally include code by testing args rather
than passing in other macros that define our conditional bits, and we
can use cpp conditional compilation easily inside the gas macros. These
two properties means we don't have bits of asm code scattered through
various macros which call each other and are passed into other macros
etc. The everything is pretty linear and flat. Not having to use big
split line makes things nicer to rejig too.

I tried to make the syntax to do conditional asm a bit nicer, but
couldn't find a great way. It's not *horrible*:

#ifdef CONFIG_KVM_BOOK3S_64_HANDLER
    .ifgt \kvm
    lbz    r25,HSTATE_IN_GUEST(r13)
    cmpwi  r25,0
    bne    1f
    .endif
#endif

We could improve it a bit maybe. You could put a cpp wrapper over it:
#ifdef CONFIG_KVM_BOOK3S_64_HANDLER
    IF(kvm)
    lbz    r25,HSTATE_IN_GUEST(r13)
    cmpwi  r25,0
    bne    1f
    ENDIF
#endif

Also if anyone actually read the code, the macro invocations are
bare:
	INT_ENTRY	decrementer,0x80,0,1,PACA_EXGEN,1,0,1,1,1

Again this could be wrapped:
        INT_ENTRY(decrementer, 0x80, INT_SRR, INT_REAL, INT_KVMTEST,
                  INT_NO_CFAR, INT_PPR, INT_TB)

I think this approach will allow the amount of open coded and randomly
used macros to be reduced too. I'd like to really standardize entry a
lot even if it means e.g., some less performance critical interrupts
like MCE and HMI and up saving slightly more regs than they needed to.

Second thing is performance. The biggest concern in entry code is SPR
accesses, then probably loads and stores (assuming we've minimised
branches already). SPR reads should all be done first before any SPR
writes, to avoid scoreboard stalls. SPR writes should be minimised and
so should serialising reads (CFAR, PPR, TB).

So my thinking is:

- Avoid some of these SPR reads if possible. We can avoid saving and
  setting PPR if we don't go to general C code (e.g. SLB miss). We
  could avoid CFAR for some async interrupts, if we could rely on 0x100
  for debug IPIs then important external and doorbell interrupts could
  avoid CFAR.
  
- Start with a bunch of stores to free up GPRs, then do the serializing
  SPR reads as soon as possible before the pipeline fills (these reads
  have to wait for all previous OPs to complete before they can begin).

- Don't store these SPR reads immediately into the PACA, but keep them
  in the GPRs we've just freed. This should make it simpler to keep all
  stores close in cache, and importantly it avoids involveing the LSU in
  this dependency. Stores interact with barriers, and store queue
  resources can be allocated while the store waits for this dependency.

- In some cases (e.g., SLB miss) the CFAR may never be used. If we avoid
  storing the value anywhere, the data doesn't end up in a critical
  execution path (though it still pushes completion out).

- SPRs can be passed via GPRs through to C interrupt handlers. In this
  case we read TB right up front and pass it into timer_interrupt to
  avoid a mftb there.

- A number of HSRR interrupts do not clear MSR[RI], so setting it
  should be avoided for those. But might as well go one further and
  avoid setting MSR[RI]=1 until we're ready to set MSR[EE]=1 so they
  can be done at once. It does increase RI=0 window a bit, but we
  don't take SLB misses on the kernel stack, and we already deal with
  IR=DR=1 && RI=0 case for virt interrupts so we're already exposed
  to machine check in translation there.

- Use non-volatile GPRs for scratch registers. This means we can save
  non-volatiles before calling a C function just by storing them
  immediately to the stack (rather than loading from paca first). It
  allows us to call C functions without blowing our scratch registers.

- Load the stack early from the paca so register saving stores to stack
  get their dependency as soon as possible.

- Not in this patch and not entirely depending on it, but I would like
  to convert kvm interrupt entry over to using this same convention of
  PACA_EX save areas and register layout. Existing KVM calls are slower
  than they could be because they switch to using HSTATE_SCRATCH etc
  and this gets even worse now with more registers saved before the
  KVM test. Other benefit is that KVM entry at the moment is not
  reentrant-safe (e.g., machine check interrupting a hypervisor doorbell
  while KVM is in guest will corrupt scratch space despite MSR[RI]=1).
  Using the different paca save areas would solve that.

That's about all I can think of at the moment.

Thanks,
Nick

diff --git a/arch/powerpc/include/asm/exception-64s-new.h b/arch/powerpc/include/asm/exception-64s-new.h
new file mode 100644
index 000000000000..f5fdc49d14c5
--- /dev/null
+++ b/arch/powerpc/include/asm/exception-64s-new.h
@@ -0,0 +1,291 @@
+#ifndef _ASM_POWERPC_EXCEPTION_NEW_H
+#define _ASM_POWERPC_EXCEPTION_NEW_H
+/*
+ * The following macros define the code that appears as
+ * the prologue to each of the exception handlers.  They
+ * are split into two parts to allow a single kernel binary
+ * to be used for pSeries and iSeries.
+ *
+ * We make as much of the exception code common between native
+ * exception handlers (including pSeries LPAR) and iSeries LPAR
+ * implementations as possible.
+ */
+#include <asm/head-64.h>
+#include <asm/exception-64s.h>
+
+#define EX_R16		0x00
+#define EX_R17		0x08
+#define EX_R18		0x10
+#define EX_R19		0x18
+#define EX_R20		0x20
+#define EX_R21		0x28
+#define EX_R22		0x30
+#define EX_R23		0x38
+#define EX_R24		0x40
+#define EX_R25		0x48
+#define EX_R26		0x50
+#define EX_R1		0x58
+
+.macro	INT_ENTRY name size hsrr virt area kvm cfar ppr tb stack
+	SET_SCRATCH0(r13)		/* save r13 */
+	GET_PACA(r13)
+	.ifgt \cfar
+	std	r16,\area+EX_R16(r13)
+	.endif
+	.ifgt \ppr
+	std	r17,\area+EX_R17(r13)
+	.endif
+	.ifgt \tb
+	std	r18,\area+EX_R18(r13)
+	.endif
+	.ifgt \stack
+	std	r19,\area+EX_R19(r13)
+	.endif
+	.ifgt \cfar
+	OPT_GET_SPR(r16, SPRN_CFAR, CPU_FTR_CFAR)
+	.endif
+	.if (\size == 0x20)
+	b	\name\()_tramp
+	.ifgt \virt
+		.pushsection "virt_trampolines"
+	.else
+		.pushsection "real_trampolines"
+	.endif
+\name\()_tramp:
+	.endif
+
+	.ifgt \ppr
+	OPT_GET_SPR(r17, SPRN_PPR, CPU_FTR_HAS_PPR)
+	.endif
+	.ifgt \tb
+	mftb	r18
+	.endif
+	.ifgt \stack
+	ld	r19,PACAKSAVE(r13)	/* kernel stack to use		*/
+	.endif
+	std	r20,\area+EX_R20(r13)
+	std	r21,\area+EX_R21(r13)
+	std	r22,\area+EX_R22(r13)
+	std	r23,\area+EX_R23(r13)
+	.ifgt \hsrr
+	mfspr	r20,SPRN_HSRR0
+	mfspr	r21,SPRN_HSRR1
+	.else
+	mfspr	r20,SPRN_SRR0
+	mfspr	r21,SPRN_SRR1
+	.endif
+	mfcr	r22
+	mfctr	r23
+	std	r24,\area+EX_R24(r13)
+	std	r25,\area+EX_R25(r13)
+	.ifgt \stack
+	mr	r24,r1
+	.endif
+#ifdef CONFIG_KVM_BOOK3S_64_HANDLER
+	.ifgt \kvm
+	lbz	r25,HSTATE_IN_GUEST(r13)
+	cmpwi	r25,0
+	bne	1f
+	.endif
+#endif
+#ifdef CONFIG_RELOCATABLE
+	.ifgt \virt
+	LOAD_HANDLER(r25,\name\()_virt)
+	.else
+	LOAD_HANDLER(r25,\name\()_real)
+	.endif
+	mtctr	r25
+	bctr
+#ifdef CONFIG_KVM_BOOK3S_64_HANDLER
+	.ifgt \kvm
+1:	LOAD_HANDLER(r25,\name\()_kvm)
+	mtctr	r25
+	bctr
+	.endif
+#endif
+#else /* CONFIG_RELOCATABLE */
+	.ifgt \virt
+	b	\name\()_virt
+	.else
+	b	\name\()_real
+	.endif
+#ifdef CONFIG_KVM_BOOK3S_64_HANDLER
+	.ifgt \kvm
+1:	b	\name\()_kvm
+	.endif
+#endif
+#endif /* CONFIG_RELOCATABLE */
+	.if (\size == 0x20)
+	.popsection
+	.endif
+.endm
+
+.macro INT_ENTRY_RESTORE area cfar ppr tb
+	mtcr	r22
+	mtctr	r23
+	mr	r1,r24
+	.ifgt \cfar
+	ld	r16,\area+EX_R16(r13)
+	.endif
+	.ifgt \ppr
+	ld	r17,\area+EX_R17(r13)
+	.endif
+	.ifgt \tb
+	ld	r18,\area+EX_R18(r13)
+	.endif
+	ld	r19,\area+EX_R19(r13)
+	ld	r20,\area+EX_R20(r13)
+	ld	r21,\area+EX_R21(r13)
+	ld	r22,\area+EX_R22(r13)
+	ld	r23,\area+EX_R23(r13)
+	ld	r24,\area+EX_R24(r13)
+	ld	r25,\area+EX_R25(r13)
+.endm
+
+/*
+ * After INT_ENTRY, with r1 set to a valid stack pointer, this macro sets up
+ * the stack frame, saves state into it, restores the NVGPR registers, and
+ * loads the TOC into r2.
+ */
+.macro INT_SETUP_C_CALL area cfar ppr tb
+	std	r24,0(r1)		/* make stack chain pointer	*/
+	std	r0,GPR0(r1)		/* save r0 in stackframe	*/
+	std	r24,GPR1(r1)		/* save r1 in stackframe	*/
+	std	r2,GPR2(r1)		/* save r2 in stackframe	*/
+	ld	r2,PACATOC(r13)		/* get kernel TOC into r2	*/
+	GET_SCRATCH0(r0)
+	SAVE_4GPRS(3, r1)		/* save r3 - r6 in stackframe  */
+	mflr	r3
+	mfspr	r4,SPRN_XER
+	ld	r5,PACACURRENT(r13)
+	ld	r6,exception_marker@toc(r2)
+	SAVE_4GPRS(7, r1)		/* save r7 - r10 in stackframe  */
+	SAVE_2GPRS(11, r1)		/* save r11 - r12 in stackframe  */
+	std	r0,GPR13(r1)
+	std	r20,_NIP(r1)		/* save SRR0 in stackframe	*/
+	std	r21,_MSR(r1)		/* save SRR1 in stackframe	*/
+	std	r22,_CCR(r1)		/* save CR in stackframe	*/
+	std	r23,_CTR(r1)		/* save CTR in stackframe	*/
+	std	r3,_LINK(r1)
+	std	r4,_XER(r1)
+	std	r25,_TRAP(r1)		/* set trap number		*/
+	li	r3,0
+	std	r3,RESULT(r1)		/* clear regs->result		*/
+	std	r19,SOFTE(r1)
+	std	r6,STACK_FRAME_OVERHEAD-16(r1) /* mark the frame	*/
+
+	HMT_MEDIUM /* XXX: where to put this? It is NTC SPR write, should go after all SPR reads, late but before NTC SPR read stores?? (cfar, tb, ppr) */
+
+#ifdef CONFIG_TRACE_IRQFLAGS
+	andi.	r0,r19,IRQS_DISABLED
+	bne	1f
+	TRACE_DISABLE_INTS /* clobbers volatile registers */
+1:
+#endif
+
+	/* XXX: async calls */
+	FINISH_NAP
+	RUNLATCH_ON
+
+	addi	r3,r1,STACK_FRAME_OVERHEAD
+	.ifgt \cfar
+	std	r16,ORIG_GPR3(r1)
+	ld	r16,\area+EX_R16(r13)
+	.endif
+	.ifgt \ppr
+	std	r17,TASKTHREADPPR(r5)
+	ld	r17,\area+EX_R17(r13)
+	.endif
+	.ifgt \tb
+	mr	r4,r18
+	ld	r18,\area+EX_R18(r13)
+	.endif
+	ld	r19,\area+EX_R19(r13)
+	ld	r20,\area+EX_R20(r13)
+	ld	r21,\area+EX_R21(r13)
+	ld	r22,\area+EX_R22(r13)
+	ld	r23,\area+EX_R23(r13)
+	ld	r24,\area+EX_R24(r13)
+	ld	r25,\area+EX_R25(r13)
+.endm
+
+.macro INT_COMMON name vec area mask cfar ppr tb
+\name\()_real:
+	ld	r25,PACAKMSR(r13)	/* MSR value for kernel */
+	xori	r25,r25,MSR_RI		/* clear MSR_RI */
+	mtmsrd	r25,0
+	nop				/* Quadword align the virt entry */
+\name\()_virt:
+	andi.	r25,r21,MSR_PR
+	mr	r1,r19
+	li	r19,IRQS_ENABLED
+	li	r25,PACA_IRQ_HARD_DIS
+	bne	1f
+	subi	r1,r24,INT_FRAME_SIZE
+	.ifgt \mask
+	lbz	r19,PACAIRQSOFTMASK(r13)
+	andi.	r25,r19,\mask
+	lbz	r25,PACAIRQHAPPENED(r13)
+	bne-	\name\()_masked_interrupt
+	.else
+	lbz	r25,PACAIRQHAPPENED(r13)
+	.endif
+	ori	r25,r25,PACA_IRQ_HARD_DIS
+1:
+	stb	r25,PACAIRQHAPPENED(r13)
+	li	r25,IRQS_ALL_DISABLED
+	stb	r25,PACAIRQSOFTMASK(r13)
+	li	r25,\vec + 1
+	cmpdi	r1,-INT_FRAME_SIZE	/* check if r1 is in userspace	*/
+	bge-	bad_stack_common	/* abort if it is		*/
+	INT_SETUP_C_CALL \area \cfar \ppr \tb
+.endm
+
+.macro INT_KVM name hsrr vec area skip cfar ppr tb
+#ifdef CONFIG_KVM_BOOK3S_64_HANDLER
+	.ifgt \skip
+	cmpwi	r25,KVM_GUEST_MODE_SKIP
+	beq	1f
+	HMT_MEDIUM /* XXX: where to put this? (see above) */
+	.endif
+	.ifgt \cfar
+	mr	r25,r16			/* No CFAR, set it to 0 */
+	.else
+	li	r25,0
+	.endif
+	std	r25,HSTATE_CFAR(r13)
+	.ifgt \ppr			/* No PPR */
+	mr	r25,r17
+	.else
+	li	r25,0
+	.endif
+	std	r25,HSTATE_PPR(r13)
+	INT_ENTRY_RESTORE \area \cfar \ppr \tb
+	std	r12,HSTATE_SCRATCH0(r13)
+	mfcr	r12
+	sldi	r12,r12,32
+	.ifgt \hsrr
+	ori	r12,r12,\vec + 0x2
+	.else
+	ori	r12,r12,\vec
+	.endif
+	b	kvmppc_interrupt
+
+	.ifgt \skip
+1:	addi	r20,r20,4
+	.ifgt \hsrr
+	mtspr	SPRN_HSRR0,r20
+	INT_ENTRY_RESTORE \area \cfar \ppr \tb
+	GET_SCRATCH0(r13)
+	HRFI_TO_KERNEL
+	.else
+	mtspr	SPRN_SRR0,r20
+	INT_ENTRY_RESTORE \area \cfar \ppr \tb
+	GET_SCRATCH0(r13)
+	RFI_TO_KERNEL
+	.endif
+	.endif
+#endif
+.endm
+
+#endif	/* _ASM_POWERPC_EXCEPTION_NEW_H */
diff --git a/arch/powerpc/include/asm/exception-64s.h b/arch/powerpc/include/asm/exception-64s.h
index 471b2274fbeb..a4d501947097 100644
--- a/arch/powerpc/include/asm/exception-64s.h
+++ b/arch/powerpc/include/asm/exception-64s.h
@@ -49,11 +49,12 @@
 #define EX_PPR		64
 #if defined(CONFIG_RELOCATABLE)
 #define EX_CTR		72
-#define EX_SIZE		10	/* size in u64 units */
 #else
-#define EX_SIZE		9	/* size in u64 units */
 #endif
 
+/* exception-64s-new.h uses 10 */
+#define EX_SIZE		10	/* size in u64 units */
+
 /*
  * maximum recursive depth of MCE exceptions
  */
diff --git a/arch/powerpc/include/asm/hw_irq.h b/arch/powerpc/include/asm/hw_irq.h
index 855e17d158b1..49fb156aa93a 100644
--- a/arch/powerpc/include/asm/hw_irq.h
+++ b/arch/powerpc/include/asm/hw_irq.h
@@ -54,7 +54,8 @@
 extern void replay_system_reset(void);
 extern void __replay_interrupt(unsigned int vector);
 
-extern void timer_interrupt(struct pt_regs *);
+extern void timer_interrupt(struct pt_regs *regs);
+extern void timer_interrupt_new(struct pt_regs *regs, u64 tb);
 extern void performance_monitor_exception(struct pt_regs *regs);
 extern void WatchdogException(struct pt_regs *regs);
 extern void unknown_exception(struct pt_regs *regs);
diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
index 2cb5109a7ea3..db934d29069c 100644
--- a/arch/powerpc/kernel/entry_64.S
+++ b/arch/powerpc/kernel/entry_64.S
@@ -995,7 +995,8 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
 1:	cmpwi	cr0,r3,0x900
 	bne	1f
 	addi	r3,r1,STACK_FRAME_OVERHEAD;
-	bl	timer_interrupt
+	mftb	r4
+	bl	timer_interrupt_new
 	b	ret_from_except
 #ifdef CONFIG_PPC_DOORBELL
 1:
diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
index b6d1baecfbff..c700a9d7e17a 100644
--- a/arch/powerpc/kernel/exceptions-64s.S
+++ b/arch/powerpc/kernel/exceptions-64s.S
@@ -820,11 +820,42 @@ END_FTR_SECTION_IFSET(CPU_FTR_TM)
 #endif
 
 
-EXC_REAL_MASKABLE(decrementer, 0x900, 0x80, IRQS_DISABLED)
-EXC_VIRT_MASKABLE(decrementer, 0x4900, 0x80, 0x900, IRQS_DISABLED)
-TRAMP_KVM(PACA_EXGEN, 0x900)
-EXC_COMMON_ASYNC(decrementer_common, 0x900, timer_interrupt)
+#include <asm/exception-64s-new.h>
+
+EXC_REAL_BEGIN(decrementer, 0x900, 0x80)
+	/*
+	 * decrementer handler:
+	 * SRR[01], real, exgen, kvm, !cfar, ppr, tb, stack
+	 */
+	INT_ENTRY	decrementer,0x80,0,1,PACA_EXGEN,1,0,1,1,1
+EXC_REAL_END(decrementer, 0x900, 0x80)
+
+EXC_VIRT_BEGIN(decrementer, 0x4900, 0x80)
+	/*
+	 * decrementer handler:
+	 * SRR[01], virt, exgen, kvm, !cfar, ppr, tb, stack
+	 */
+	INT_ENTRY	decrementer,0x80,0,1,PACA_EXGEN,1,0,1,1,1
+EXC_VIRT_END(decrementer, 0x4900, 0x80)
+
+EXC_COMMON_BEGIN(decrementer_kvm)
+	INT_KVM		decrementer,0,0x900,PACA_EXGEN,0,0,1,1
+
+EXC_COMMON_BEGIN(decrementer)
+	INT_COMMON	decrementer,0x900,PACA_EXGEN,IRQS_DISABLED,0,1,1
+	bl	timer_interrupt_new
+	b	ret_from_except_lite
+
+decrementer_masked_interrupt:
+	ori	r25,r25,SOFTEN_VALUE_0x900
+	stb	r25,PACAIRQHAPPENED(r13)
+	lis	r25,0x7fff
+	ori	r25,r25,0xffff
+	mtspr	SPRN_DEC,r25
+	INT_ENTRY_RESTORE PACA_EXGEN,0,1,1
+	RFI_TO_KERNEL
 
+EXC_COMMON_ASYNC(decrementer_common, 0x900, timer_interrupt)
 
 EXC_REAL_HV(hdecrementer, 0x980, 0x80)
 EXC_VIRT_HV(hdecrementer, 0x4980, 0x80, 0x980)
@@ -842,6 +873,7 @@ EXC_COMMON_ASYNC(doorbell_super_common, 0xa00, unknown_exception)
 #endif
 
 
+
 EXC_REAL(trap_0b, 0xb00, 0x100)
 EXC_VIRT(trap_0b, 0x4b00, 0x100, 0xb00)
 TRAMP_KVM(PACA_EXGEN, 0xb00)
@@ -1767,6 +1799,26 @@ END_FTR_SECTION_IFSET(CPU_FTR_CFAR)
 	b	1b
 _ASM_NOKPROBE_SYMBOL(bad_stack);
 
+/*
+ * Here we have detected that the kernel stack pointer is bad.
+ * R9 contains the saved CR, r13 points to the paca,
+ * r10 contains the (bad) kernel stack pointer,
+ * r11 and r12 contain the saved SRR0 and SRR1.
+ * We switch to using an emergency stack, save the registers there,
+ * and call kernel_bad_stack(), which panics.
+ */
+bad_stack_common:
+	ld	r1,PACAEMERGSP(r13)
+	subi	r1,r1,64+INT_FRAME_SIZE
+	/*
+	 * This clobbers r16-r18 for interrupts that use them, but we
+	 * never return to userspace.
+	 */
+	INT_SETUP_C_CALL PACA_EXGEN,0,0,0
+	bl	kernel_bad_stack
+	b	.
+_ASM_NOKPROBE_SYMBOL(bad_stack_common);
+
 /*
  * When doorbell is triggered from system reset wakeup, the message is
  * not cleared, so it would fire again when EE is enabled.
@@ -1786,6 +1838,29 @@ doorbell_super_common_msgclr:
 	PPC_MSGCLRP(3)
 	b 	doorbell_super_common
 
+replay_decrementer:
+	/* XXX: crashes */
+	subi	r1,r1,INT_FRAME_SIZE
+	std	r1,INT_FRAME_SIZE(r1)
+	std	r1,GPR1(r1)
+	std	r2,GPR2(r1)
+	ld	r5,PACACURRENT(r13)
+	ld	r6,exception_marker@toc(r2)
+	std	r11,_NIP(r1)
+	std	r12,_MSR(r1)
+	std	r9,_CCR(r1)
+	std	r3,_TRAP(r1)
+	li	r3,0
+	std	r3,RESULT(r1)
+	lbz	r3,PACAIRQSOFTMASK(r13)
+	std	r3,SOFTE(r1)
+	std	r6,STACK_FRAME_OVERHEAD-16(r1)
+	/* XXX: ppr? */
+	addi	r3,r1,STACK_FRAME_OVERHEAD
+	mftb	r4
+	bl	timer_interrupt_new
+	b	ret_from_except_lite
+
 /*
  * Called from arch_local_irq_enable when an interrupt needs
  * to be resent. r3 contains 0x500, 0x900, 0xa00 or 0xe80 to indicate
@@ -1811,6 +1886,7 @@ _GLOBAL(__replay_interrupt)
 	ori	r12,r12,MSR_EE
 	cmpwi	r3,0x900
 	beq	decrementer_common
+//	beq	replay_decrementer
 	cmpwi	r3,0x500
 BEGIN_FTR_SECTION
 	beq	h_virt_irq_common
diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
index a32823dcd9a4..72b38917fd77 100644
--- a/arch/powerpc/kernel/time.c
+++ b/arch/powerpc/kernel/time.c
@@ -100,7 +100,7 @@ static struct clocksource clocksource_timebase = {
 };
 
 #define DECREMENTER_DEFAULT_MAX 0x7FFFFFFF
-u64 decrementer_max = DECREMENTER_DEFAULT_MAX;
+u64 decrementer_max __read_mostly = DECREMENTER_DEFAULT_MAX;
 
 static int decrementer_set_next_event(unsigned long evt,
 				      struct clock_event_device *dev);
@@ -535,12 +535,11 @@ void arch_irq_work_raise(void)
 
 #endif /* CONFIG_IRQ_WORK */
 
-static void __timer_interrupt(void)
+static void __timer_interrupt(u64 now)
 {
 	struct pt_regs *regs = get_irq_regs();
 	u64 *next_tb = this_cpu_ptr(&decrementers_next_tb);
 	struct clock_event_device *evt = this_cpu_ptr(&decrementers);
-	u64 now;
 
 	trace_timer_interrupt_entry(regs);
 
@@ -549,7 +548,10 @@ static void __timer_interrupt(void)
 		irq_work_run();
 	}
 
+#ifndef CONFIG_PPC_BOOK3S_64
 	now = get_tb_or_rtc();
+#endif
+
 	if (now >= *next_tb) {
 		*next_tb = ~(u64)0;
 		if (evt->event_handler)
@@ -557,8 +559,9 @@ static void __timer_interrupt(void)
 		__this_cpu_inc(irq_stat.timer_irqs_event);
 	} else {
 		now = *next_tb - now;
-		if (now <= decrementer_max)
-			set_dec(now);
+		if (now > decrementer_max)
+			now = decrementer_max;
+		set_dec(now);
 		/* We may have raced with new irq work */
 		if (test_irq_work_pending())
 			set_dec(1);
@@ -576,19 +579,18 @@ static void __timer_interrupt(void)
 	trace_timer_interrupt_exit(regs);
 }
 
+void timer_interrupt(struct pt_regs * regs)
+{
+	timer_interrupt_new(regs, get_tb_or_rtc());
+}
+
 /*
  * timer_interrupt - gets called when the decrementer overflows,
  * with interrupts disabled.
  */
-void timer_interrupt(struct pt_regs * regs)
+void timer_interrupt_new(struct pt_regs * regs, u64 tb)
 {
 	struct pt_regs *old_regs;
-	u64 *next_tb = this_cpu_ptr(&decrementers_next_tb);
-
-	/* Ensure a positive value is written to the decrementer, or else
-	 * some CPUs will continue to take decrementer exceptions.
-	 */
-	set_dec(decrementer_max);
 
 	/* Some implementations of hotplug will get timer interrupts while
 	 * offline, just ignore these and we also need to set
@@ -596,15 +598,21 @@ void timer_interrupt(struct pt_regs * regs)
 	 * don't replay timer interrupt when return, otherwise we'll trap
 	 * here infinitely :(
 	 */
-	if (!cpu_online(smp_processor_id())) {
+	if (unlikely(!cpu_online(smp_processor_id()))) {
+		u64 *next_tb = this_cpu_ptr(&decrementers_next_tb);
 		*next_tb = ~(u64)0;
+		set_dec(decrementer_max);
 		return;
 	}
 
 	/* Conditionally hard-enable interrupts now that the DEC has been
 	 * bumped to its maximum value
 	 */
-	may_hard_irq_enable();
+	if (may_hard_irq_enable()) {
+		set_dec(decrementer_max);
+		get_paca()->irq_happened &= ~PACA_IRQ_HARD_DIS;
+		__hard_irq_enable();
+	}
 
 
 #if defined(CONFIG_PPC32) && defined(CONFIG_PPC_PMAC)
@@ -615,7 +623,7 @@ void timer_interrupt(struct pt_regs * regs)
 	old_regs = set_irq_regs(regs);
 	irq_enter();
 
-	__timer_interrupt();
+	__timer_interrupt(tb);
 	irq_exit();
 	set_irq_regs(old_regs);
 }
@@ -971,10 +979,11 @@ static int decrementer_shutdown(struct clock_event_device *dev)
 /* Interrupt handler for the timer broadcast IPI */
 void tick_broadcast_ipi_handler(void)
 {
+	u64 now = get_tb_or_rtc();
 	u64 *next_tb = this_cpu_ptr(&decrementers_next_tb);
 
-	*next_tb = get_tb_or_rtc();
-	__timer_interrupt();
+	*next_tb = now;
+	__timer_interrupt(now);
 }
 
 static void register_decrementer_clockevent(int cpu)

^ permalink raw reply related

* Re: [trivial PATCH V2] treewide: Align function definition open/close braces
From: Steven Rostedt @ 2018-03-22 13:37 UTC (permalink / raw)
  To: Joe Perches
  Cc: Andrew Morton, Will Deacon, Peter Zijlstra, Boqun Feng,
	Rafael J. Wysocki, Len Brown, Zhang Rui, Sathya Prakash,
	Chaitra P B, Suganath Prabu Subramani, Manish Chopra, Rahul Verma,
	Dept-GELinuxNICDev, QCA ath9k Development, Corentin Chary,
	Darren Hart, Andy Shevchenko, Alessandro Zummo, Alexandre Belloni,
	Adaptec OEM Raid Solutions, James E.J. Bottomley,
	Martin K. Petersen, Matthew Wilcox, Jeff Layton, J. Bruce Fields,
	Alexander Viro, Mark Fasheh, Joel Becker, Darrick J. Wong,
	linux-xfs, Paul Moore, Eric Paris, Timur Tabi, Nicolin Chen,
	Xiubo Li, Fabio Estevam, Jiri Kosina, Thomas Gleixner,
	H. Peter Anvin, x86, Alex Deucher, Christian König,
	David (ChunMing) Zhou, David Airlie, Mauro Carvalho Chehab,
	Kalle Valo, Liam Girdwood, Mark Brown, Jaroslav Kysela,
	Takashi Iwai, linux-kernel, linux-acpi, amd-gfx, dri-devel,
	linux-media, MPT-FusionLinux.pdl, linux-scsi, netdev,
	linux-wireless, acpi4asus-user, platform-driver-x86, linux-rtc,
	linux-fsdevel, ocfs2-devel, linux-audit, alsa-devel, linuxppc-dev
In-Reply-To: <5ccbbf083e26bddfb4ea4f819ed62347ce266f39.1521669820.git.joe@perches.com>

On Wed, 21 Mar 2018 15:09:32 -0700
Joe Perches <joe@perches.com> wrote:

> diff --git a/kernel/trace/trace_printk.c b/kernel/trace/trace_printk.c
> index ad1d6164e946..50f44b7b2b32 100644
> --- a/kernel/trace/trace_printk.c
> +++ b/kernel/trace/trace_printk.c
> @@ -196,7 +196,7 @@ struct notifier_block module_trace_bprintk_format_nb = {
>  };
>  
>  int __trace_bprintk(unsigned long ip, const char *fmt, ...)
> - {
> +{
>  	int ret;
>  	va_list ap;
>  
> @@ -214,7 +214,7 @@ int __trace_bprintk(unsigned long ip, const char *fmt, ...)
>  EXPORT_SYMBOL_GPL(__trace_bprintk);
>  
>  int __ftrace_vbprintk(unsigned long ip, const char *fmt, va_list ap)
> - {
> +{
>  	if (unlikely(!fmt))
>  		return 0;
>  

Acked-by: Steven Rostedt (VMware) <rostedt@goodmis.org>

-- Steve

^ permalink raw reply

* Re: RFC on writel and writel_relaxed
From: Benjamin Herrenschmidt @ 2018-03-22 13:52 UTC (permalink / raw)
  To: Oliver
  Cc: Sinan Kaya, open list:LINUX FOR POWERPC (32-BIT AND 64-BIT),
	linux-rdma@vger.kernel.org
In-Reply-To: <CAOSf1CFv1HHCL3YOVhRn2U=grdNjaQ=A4m3xwxN2Rek1-_TySg@mail.gmail.com>

On Thu, 2018-03-22 at 21:15 +1100, Oliver wrote:
> On Thu, Mar 22, 2018 at 3:24 PM, Benjamin Herrenschmidt
> <benh@kernel.crashing.org> wrote:
> > On Wed, 2018-03-21 at 08:53 -0500, Sinan Kaya wrote:
> > > writel_relaxed() needs to have ordering guarantees with respect to the order
> > > device observes writes.
> > 
> > Correct.
> > 
> > > x86 has compiler barrier inside the relaxed() API so that code does not
> > > get reordered. ARM64 architecturally guarantees device writes to be observed
> > > in order.
> > > 
> > > I was hoping that PPC could follow x86 and inject compiler barrier into the
> > > relaxed functions.
> > > 
> > > BTW, I have no idea what compiler barrier does on PPC and if
> > > 
> > > wrltel() == compiler barrier() + wrltel_relaxed()
> > > 
> > > can be said.
> > 
> > No, it's not sufficient.

Just to clarify ... barrier() is just a compiler barrier, it means the
compiler will generate things in the order they are written. This isn't
sufficient on archs with an OO memory model, where an actual memory
barrier instruction needs to be emited.

As for Oliver comments...

> > Replacing wmb() + writel() with wmb() + writel_relaxed() will work on
> > PPC, it will just not give you a benefit today.
> > 
> > The main problem is that the semantics of writel/writel_relaxed (and
> > read versions) aren't very well defined in Linux esp. when it comes
> > to different memory types (NC, WC, ...).
> > 
> > I've been wanting to implement the relaxed accessors for a while but
> > was battling with this to try to also better support WC, and due to
> > other commitments, this somewhat fell down the cracks.
> > 
> > Two options I can think of:
> > 
> >  - Just make the _relaxed variants use an eieio instead of a sync, this
> > will effectively lift the ordering guarantee vs. cachable storage (and
> > thus unlock) and might give a (small) performance improvement.
> 
> Wouldn't we still have the unlock ordering due to the io_sync hack or
> are you thinking we should remove that too for the relaxed version?

Well, the documentation says we don't care about synchronization vs.
locks so we should probably remove it (then we need to make sure mmiowb
works, thus sets the flag).

> > However,
> > we still have the problem that on WC mappings, neither writel nor
> > writel_relaxed will effectively allow combining to happen (only raw
> > accesses will because on powerpc *all* barriers will break combining).
> 
> Hmm, eieio is only architected to affect CI+G (and WT) so it shouldn't
> affect combining
> on non-guarded memory. Do most implementations apply it to all CI
> accesses anyway?

Yes, as far as I know all implementations will stop combining on *any*
barrier instruction.

> >  - Make writel_relaxed() be a simple store without barriers, and
> > readl_relaxed() be "eieio, read, eieio", thus allowing write combining
> > to happen between successive writel_relaxed on WC space (no change on
> > normal NC space) while maintaining the ordering between relaxed reads
> > and writes. The flip side is a (slight) increased overhead of
> > readl_relaxed.
> 
> Are there many drivers that actually do writeX() on WC space?
> memory-barriers.txt
> pretty much says that all bets are off and no ordering guarantees can be assumed
> when using readX/writeX on prefetchable IO memory. It seems sketchy enough to
> give me some pause, but maybe it works fine elsewhere.

I don't know whether any does it, but I want to provide a way for a
driver to somewhat reliably obtain write combine semantics without
having to hand code endian swap and other horrors involved with using
__raw_* accessors.

So my thinking is to define that the combination of WC + writel_relaxed
gives you that, which it does at least on x86 and ARM afaik.

Cheers,
Ben.

^ permalink raw reply

* Re: DMA Mapping Error in ppc64
From: Jared Bents @ 2018-03-22 14:37 UTC (permalink / raw)
  To: Oliver; +Cc: linuxppc-dev
In-Reply-To: <CAOSf1CGqc9=QnrChOY_+YL0v8TALQ+pH8u1e26Q--0PS4CGm2g@mail.gmail.com>

Thank you for the response but unfortunately, it looks like I already
have that and it is being used.  To verify, I commented that out and
got the failure "dma_direct_alloc_coherent: No suitable zone for pfn
0xe0000".  Below is the code flow for function
ath10k_pci_hif_exchange_bmi_msg which is showing the first dma mapping
error.

ath10k_pci_hif_exchange_bmi_msg -> dma_map_single ->
dma_map_single_attrs -> swiotlb_map_page -> dma_capable (returns
false)


dma_capable is what reports the failure in that flow.

static inline bool dma_capable(struct device *dev, dma_addr_t addr, size_t size)
{
#ifdef CONFIG_SWIOTLB
    struct dev_archdata *sd = &dev->archdata;

   if (sd->max_direct_dma_addr && addr + size > sd->max_direct_dma_addr)
        return false;
#endif

    if (!dev->dma_mask)
        return false;

    return addr + size - 1 <= *dev->dma_mask;
}
Getting the below values:
addr = 1ee376218
size = 4
sd->max_direct_dma_addr = e0000000 which is I believe DMA window size (e0000000)

when executed sd->max_direct_dma_addr(e0000000) && addr(1ee376218) +
size(4) becomes e0000004 which is > sd->max_direct_dma_addr (e0000000)


So even though limit_zone_pfn(ZONE_DMA32, 1UL << (31 - PAGE_SHIFT)) is
being used in arch/powerpc/platforms/85xx/corenet_generic.c,
kmemdup(req, req_len, GFP_KERNEL) is returning an address that when
sent to dma_map_single(), results in a bad map.

- Jared

On Wed, Mar 21, 2018 at 11:54 PM, Oliver <oohall@gmail.com> wrote:
> On Thu, Mar 22, 2018 at 8:00 AM, Jared Bents
> <jared.bents@rockwellcollins.com> wrote:
>> Hi all,
>>
>> Apologies for the amount of information but we've been debugging this
>> for a while and I wanted to get what we are seeing captured as much as
>> possible.  We are a T1042 processor and have a total 8GB DDR and our
>> kernel version is fsl-sdk-v2.0-1703 (linux v4.1.35) as that is the
>> latest version supplied by NXP.
>>
>> A while ago we ported from 32 bit to 64 bit.  Everything continued to
>> work except the ath10k module we have.  So as a first step, we checked
>> to see if an ath9k module also failed to work and it was also no
>> longer working.  The ath10k is working fine on a 32 bit system but
>> it's not working on 64 bit system as we are getting dma mapping errors
>> when trying to initialize the wifi modules.
>>
>> pci_bus 0002:01: bus scan returning with max=01
>> pci_bus 0002:01: busn_res: [bus 01] end is updated to 01
>> pci_bus 0002:00: bus scan returning with max=01
>> ath10k_pci 0000:01:00.0: unable to get target info from device
>> ath10k_pci 0000:01:00.0: could not get target info (-5)
>> ath10k_pci 0000:01:00.0: could not probe fw (-5)
>> ath10k_pci 0001:01:00.0: Direct firmware load for
>> ath10k/cal-pci-0001:01:00.0.bin failed with error -2
>>
>>
>> First, we have tried the mainline kernel (v4.15)  to see if that would
>> fix the issue, it did not.  So I made a patch for the ath10k driver to
>> restrict to just GFP_DMA areas when allocating memory or creating
>> sk_buffs and have attached it.  The ath10k wifi modules now initialize
>> correctly but when I try to connect them and send traffic, they get a
>> DMA mapping error from the sk_buff that it receives from elsewhere in
>> the kernel.  So while the driver appears to be fixable with the patch,
>> the modules are still unusable due to data being sent to the driver
>> when ath10k_tx is called and it tries to dma map with the provided
>> skb.  Also, according to the ath10k mailing list, GFP_DMA is not
>> supposed to be used in general.  The error below is the same sort of
>> dma mapping error that is seen when initializing the modules without
>> the patch to OR with GFP_DMA.
>>
>> ath10k_pci 0001:01:00.0: failed to transmit packet, dropping: -5
>>
>>
>> We asked on the ath10k mailing list if anyone else is having this
>> problem and no one else seems to have the issue but they are using
>> different architectures (ARM or X86). As a result, it does not seem to
>> be a driver issue to us but something within the PowerPC arch.  So we
>> dug a little deeper to try to find what addresses being mapped are
>> working and what address being mapped are not working.
>>
>> We found that when the virtual address of data pointer (a member of
>> sk_buff) is above ~3.7 GB RAM address range then return address from
>> dma_map_single API is failed to validate in dma_mapping_error
>> function.
>>
>> We also noticed that in a 64bit machine sometimes ping is working and
>> because of the virtual address is under ~3.7GAM RAM address range.  So
>> if we set mem=2048M in the bootargs, the ath10k module works
>> perfectly, however this isn't a real solution since it cuts our
>> available RAM from 8GB to 2GB.
>
> I think there's a known issue with the freescale PCIe root complex
> where it can't DMA beyond the 4GB mark. There's a workaround in
> the form of limit_zone_pfn() which you can use to put the lower 4GB into
> ZONE_DMA32 and allocate from there rather than ZONE_NORMAL.
> For details of how to use it have a look at corenet_gen_setup_arch() in
> arch/powerpc/platforms/85xx/corenet_generic.c
>
> Hope that helps,
> Oliver

^ permalink raw reply

* Re: [PATCH v2 06/38] cxlflash: Read host function configuration
From: Frederic Barrat @ 2018-03-22 15:43 UTC (permalink / raw)
  To: Uma Krishnan, linux-scsi, James Bottomley, Martin K. Petersen,
	Matthew R. Ochs, Manoj N. Kumar
  Cc: linuxppc-dev, Andrew Donnellan, Christophe Lombard
In-Reply-To: <1519683641-16967-1-git-send-email-ukrishn@linux.vnet.ibm.com>



Le 26/02/2018 à 23:20, Uma Krishnan a écrit :
> Per the OCXL specification, the underlying host can have multiple AFUs
> per function with each function supporting its own configuration. The host
> function configuration is read on the initialization path to evaluate the
> number of functions present and identify the features and configuration of
> the functions present. This data is cached for use in later configuration
> steps. Note that for the OCXL hardware supported by the cxlflash driver,
> only one AFU per function is expected.
> 
> Signed-off-by: Uma Krishnan <ukrishn@linux.vnet.ibm.com>
> Acked-by: Matthew R. Ochs <mrochs@linux.vnet.ibm.com>
> ---

Reviewed-by: Frederic Barrat <fbarrat@linux.vnet.ibm.com>


>   drivers/scsi/cxlflash/ocxl_hw.c | 41 +++++++++++++++++++++++++++++++++++++++++
>   drivers/scsi/cxlflash/ocxl_hw.h |  2 ++
>   2 files changed, 43 insertions(+)
> 
> diff --git a/drivers/scsi/cxlflash/ocxl_hw.c b/drivers/scsi/cxlflash/ocxl_hw.c
> index e3a0a9b..dc32a73 100644
> --- a/drivers/scsi/cxlflash/ocxl_hw.c
> +++ b/drivers/scsi/cxlflash/ocxl_hw.c
> @@ -32,6 +32,35 @@ static void ocxlflash_destroy_afu(void *afu_cookie)
>   }
> 
>   /**
> + * ocxlflash_config_fn() - configure the host function
> + * @pdev:	PCI device associated with the host.
> + * @afu:	AFU associated with the host.
> + *
> + * Return: 0 on success, -errno on failure
> + */
> +static int ocxlflash_config_fn(struct pci_dev *pdev, struct ocxl_hw_afu *afu)
> +{
> +	struct ocxl_fn_config *fcfg = &afu->fcfg;
> +	struct device *dev = &pdev->dev;
> +	int rc = 0;
> +
> +	/* Read DVSEC config of the function */
> +	rc = ocxl_config_read_function(pdev, fcfg);
> +	if (unlikely(rc)) {
> +		dev_err(dev, "%s: ocxl_config_read_function failed rc=%d\n",
> +			__func__, rc);
> +		goto out;
> +	}
> +
> +	/* Only one AFU per function is supported by ocxlflash */
> +	if (fcfg->max_afu_index != 0)
> +		dev_warn(dev, "%s: Unexpected AFU index value %d\n",
> +			 __func__, fcfg->max_afu_index);
> +out:
> +	return rc;
> +}
> +
> +/**
>    * ocxlflash_create_afu() - create the AFU for OCXL
>    * @pdev:	PCI device associated with the host.
>    *
> @@ -41,6 +70,7 @@ static void *ocxlflash_create_afu(struct pci_dev *pdev)
>   {
>   	struct device *dev = &pdev->dev;
>   	struct ocxl_hw_afu *afu;
> +	int rc;
> 
>   	afu = kzalloc(sizeof(*afu), GFP_KERNEL);
>   	if (unlikely(!afu)) {
> @@ -50,8 +80,19 @@ static void *ocxlflash_create_afu(struct pci_dev *pdev)
> 
>   	afu->pdev = pdev;
>   	afu->dev = dev;
> +
> +	rc = ocxlflash_config_fn(pdev, afu);
> +	if (unlikely(rc)) {
> +		dev_err(dev, "%s: Function configuration failed rc=%d\n",
> +			__func__, rc);
> +		goto err1;
> +	}
>   out:
>   	return afu;
> +err1:
> +	kfree(afu);
> +	afu = NULL;
> +	goto out;
>   }
> 
>   /* Backend ops to ocxlflash services */
> diff --git a/drivers/scsi/cxlflash/ocxl_hw.h b/drivers/scsi/cxlflash/ocxl_hw.h
> index c7e5c4d..658f420 100644
> --- a/drivers/scsi/cxlflash/ocxl_hw.h
> +++ b/drivers/scsi/cxlflash/ocxl_hw.h
> @@ -16,4 +16,6 @@
>   struct ocxl_hw_afu {
>   	struct pci_dev *pdev;		/* PCI device */
>   	struct device *dev;		/* Generic device */
> +
> +	struct ocxl_fn_config fcfg;	/* DVSEC config of the function */
>   };
> 

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox