LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH v2 2/3] powerpc/memcpy: Add memcpy_mcsafe for pmem
From: Oliver @ 2018-04-05 11:26 UTC (permalink / raw)
  To: Balbir Singh
  Cc: linuxppc-dev, linux-nvdimm@lists.01.org, Nicholas Piggin,
	Michael Ellerman
In-Reply-To: <20180405071500.22320-3-bsingharora@gmail.com>

On Thu, Apr 5, 2018 at 5:14 PM, Balbir Singh <bsingharora@gmail.com> wrote:
> The pmem infrastructure uses memcpy_mcsafe in the pmem
> layer so as to convert machine check excpetions into
> a return value on failure in case a machine check
> exception is encoutered during the memcpy.
>
> This patch largely borrows from the copyuser_power7
> logic and does not add the VMX optimizations, largely
> to keep the patch simple. If needed those optimizations
> can be folded in.
>
> Signed-off-by: Balbir Singh <bsingharora@gmail.com>
> Acked-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>  arch/powerpc/include/asm/string.h   |   2 +
>  arch/powerpc/lib/Makefile           |   2 +-
>  arch/powerpc/lib/memcpy_mcsafe_64.S | 212 ++++++++++++++++++++++++++++++++++++
>  3 files changed, 215 insertions(+), 1 deletion(-)
>  create mode 100644 arch/powerpc/lib/memcpy_mcsafe_64.S
>
> diff --git a/arch/powerpc/include/asm/string.h b/arch/powerpc/include/asm/string.h
> index 9b8cedf618f4..b7e872a64726 100644
> --- a/arch/powerpc/include/asm/string.h
> +++ b/arch/powerpc/include/asm/string.h
> @@ -30,7 +30,9 @@ extern void * memcpy_flushcache(void *,const void *,__kernel_size_t);
>  #ifdef CONFIG_PPC64
>  #define __HAVE_ARCH_MEMSET32
>  #define __HAVE_ARCH_MEMSET64
> +#define __HAVE_ARCH_MEMCPY_MCSAFE
>
> +extern int memcpy_mcsafe(void *dst, const void *src, __kernel_size_t sz);
>  extern void *__memset16(uint16_t *, uint16_t v, __kernel_size_t);
>  extern void *__memset32(uint32_t *, uint32_t v, __kernel_size_t);
>  extern void *__memset64(uint64_t *, uint64_t v, __kernel_size_t);
> diff --git a/arch/powerpc/lib/Makefile b/arch/powerpc/lib/Makefile
> index 3c29c9009bbf..048afee9f518 100644
> --- a/arch/powerpc/lib/Makefile
> +++ b/arch/powerpc/lib/Makefile
> @@ -24,7 +24,7 @@ endif
>
>  obj64-y        += copypage_64.o copyuser_64.o mem_64.o hweight_64.o \
>            copyuser_power7.o string_64.o copypage_power7.o memcpy_power7.o \
> -          memcpy_64.o memcmp_64.o pmem.o
> +          memcpy_64.o memcmp_64.o pmem.o memcpy_mcsafe_64.o
>
>  obj64-$(CONFIG_SMP)    += locks.o
>  obj64-$(CONFIG_ALTIVEC)        += vmx-helper.o
> diff --git a/arch/powerpc/lib/memcpy_mcsafe_64.S b/arch/powerpc/lib/memcpy_mcsafe_64.S
> new file mode 100644
> index 000000000000..e7eaa9b6cded
> --- /dev/null
> +++ b/arch/powerpc/lib/memcpy_mcsafe_64.S
> @@ -0,0 +1,212 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (C) IBM Corporation, 2011
> + * Derived from copyuser_power7.s by Anton Blanchard <anton@au.ibm.com>
> + * Author - Balbir Singh <bsingharora@gmail.com>
> + */
> +#include <asm/ppc_asm.h>
> +#include <asm/errno.h>
> +
> +       .macro err1
> +100:
> +       EX_TABLE(100b,.Ldo_err1)
> +       .endm
> +
> +       .macro err2
> +200:
> +       EX_TABLE(200b,.Ldo_err2)
> +       .endm

Would it be possible to move the bulk of the copyuser code into a
seperate file which can be #included once the these err macros are
defined? Anton's memcpy is pretty hairy and I don't think anyone wants
to have multiple copies of it in the tree, even in a cut down form.

> +
> +.Ldo_err2:
> +       ld      r22,STK_REG(R22)(r1)
> +       ld      r21,STK_REG(R21)(r1)
> +       ld      r20,STK_REG(R20)(r1)
> +       ld      r19,STK_REG(R19)(r1)
> +       ld      r18,STK_REG(R18)(r1)
> +       ld      r17,STK_REG(R17)(r1)
> +       ld      r16,STK_REG(R16)(r1)
> +       ld      r15,STK_REG(R15)(r1)
> +       ld      r14,STK_REG(R14)(r1)
> +       addi    r1,r1,STACKFRAMESIZE
> +.Ldo_err1:
> +       li      r3,-EFAULT
> +       blr
> +
> +
> +_GLOBAL(memcpy_mcsafe)
> +       cmpldi  r5,16
> +       blt     .Lshort_copy
> +
> +.Lcopy:
> +       /* Get the source 8B aligned */
> +       neg     r6,r4
> +       mtocrf  0x01,r6
> +       clrldi  r6,r6,(64-3)
> +
> +       bf      cr7*4+3,1f
> +err1;  lbz     r0,0(r4)
> +       addi    r4,r4,1
> +err1;  stb     r0,0(r3)
> +       addi    r3,r3,1
> +
> +1:     bf      cr7*4+2,2f
> +err1;  lhz     r0,0(r4)
> +       addi    r4,r4,2
> +err1;  sth     r0,0(r3)
> +       addi    r3,r3,2
> +
> +2:     bf      cr7*4+1,3f
> +err1;  lwz     r0,0(r4)
> +       addi    r4,r4,4
> +err1;  stw     r0,0(r3)
> +       addi    r3,r3,4
> +
> +3:     sub     r5,r5,r6
> +       cmpldi  r5,128
> +       blt     5f
> +
> +       mflr    r0
> +       stdu    r1,-STACKFRAMESIZE(r1)
> +       std     r14,STK_REG(R14)(r1)
> +       std     r15,STK_REG(R15)(r1)
> +       std     r16,STK_REG(R16)(r1)
> +       std     r17,STK_REG(R17)(r1)
> +       std     r18,STK_REG(R18)(r1)
> +       std     r19,STK_REG(R19)(r1)
> +       std     r20,STK_REG(R20)(r1)
> +       std     r21,STK_REG(R21)(r1)
> +       std     r22,STK_REG(R22)(r1)
> +       std     r0,STACKFRAMESIZE+16(r1)
> +
> +       srdi    r6,r5,7
> +       mtctr   r6
> +
> +       /* Now do cacheline (128B) sized loads and stores. */
> +       .align  5
> +4:
> +err2;  ld      r0,0(r4)
> +err2;  ld      r6,8(r4)
> +err2;  ld      r7,16(r4)
> +err2;  ld      r8,24(r4)
> +err2;  ld      r9,32(r4)
> +err2;  ld      r10,40(r4)
> +err2;  ld      r11,48(r4)
> +err2;  ld      r12,56(r4)
> +err2;  ld      r14,64(r4)
> +err2;  ld      r15,72(r4)
> +err2;  ld      r16,80(r4)
> +err2;  ld      r17,88(r4)
> +err2;  ld      r18,96(r4)
> +err2;  ld      r19,104(r4)
> +err2;  ld      r20,112(r4)
> +err2;  ld      r21,120(r4)
> +       addi    r4,r4,128
> +err2;  std     r0,0(r3)
> +err2;  std     r6,8(r3)
> +err2;  std     r7,16(r3)
> +err2;  std     r8,24(r3)
> +err2;  std     r9,32(r3)
> +err2;  std     r10,40(r3)
> +err2;  std     r11,48(r3)
> +err2;  std     r12,56(r3)
> +err2;  std     r14,64(r3)
> +err2;  std     r15,72(r3)
> +err2;  std     r16,80(r3)
> +err2;  std     r17,88(r3)
> +err2;  std     r18,96(r3)
> +err2;  std     r19,104(r3)
> +err2;  std     r20,112(r3)
> +err2;  std     r21,120(r3)
> +       addi    r3,r3,128
> +       bdnz    4b
> +
> +       clrldi  r5,r5,(64-7)
> +
> +       ld      r14,STK_REG(R14)(r1)
> +       ld      r15,STK_REG(R15)(r1)
> +       ld      r16,STK_REG(R16)(r1)
> +       ld      r17,STK_REG(R17)(r1)
> +       ld      r18,STK_REG(R18)(r1)
> +       ld      r19,STK_REG(R19)(r1)
> +       ld      r20,STK_REG(R20)(r1)
> +       ld      r21,STK_REG(R21)(r1)
> +       ld      r22,STK_REG(R22)(r1)
> +       addi    r1,r1,STACKFRAMESIZE
> +
> +       /* Up to 127B to go */
> +5:     srdi    r6,r5,4
> +       mtocrf  0x01,r6
> +
> +6:     bf      cr7*4+1,7f
> +err1;  ld      r0,0(r4)
> +err1;  ld      r6,8(r4)
> +err1;  ld      r7,16(r4)
> +err1;  ld      r8,24(r4)
> +err1;  ld      r9,32(r4)
> +err1;  ld      r10,40(r4)
> +err1;  ld      r11,48(r4)
> +err1;  ld      r12,56(r4)
> +       addi    r4,r4,64
> +err1;  std     r0,0(r3)
> +err1;  std     r6,8(r3)
> +err1;  std     r7,16(r3)
> +err1;  std     r8,24(r3)
> +err1;  std     r9,32(r3)
> +err1;  std     r10,40(r3)
> +err1;  std     r11,48(r3)
> +err1;  std     r12,56(r3)
> +       addi    r3,r3,64
> +
> +       /* Up to 63B to go */
> +7:     bf      cr7*4+2,8f
> +err1;  ld      r0,0(r4)
> +err1;  ld      r6,8(r4)
> +err1;  ld      r7,16(r4)
> +err1;  ld      r8,24(r4)
> +       addi    r4,r4,32
> +err1;  std     r0,0(r3)
> +err1;  std     r6,8(r3)
> +err1;  std     r7,16(r3)
> +err1;  std     r8,24(r3)
> +       addi    r3,r3,32
> +
> +       /* Up to 31B to go */
> +8:     bf      cr7*4+3,9f
> +err1;  ld      r0,0(r4)
> +err1;  ld      r6,8(r4)
> +       addi    r4,r4,16
> +err1;  std     r0,0(r3)
> +err1;  std     r6,8(r3)
> +       addi    r3,r3,16
> +
> +9:     clrldi  r5,r5,(64-4)
> +
> +       /* Up to 15B to go */
> +.Lshort_copy:
> +       mtocrf  0x01,r5
> +       bf      cr7*4+0,12f
> +err1;  lwz     r0,0(r4)        /* Less chance of a reject with word ops */
> +err1;  lwz     r6,4(r4)
> +       addi    r4,r4,8
> +err1;  stw     r0,0(r3)
> +err1;  stw     r6,4(r3)
> +       addi    r3,r3,8
> +
> +12:    bf      cr7*4+1,13f
> +err1;  lwz     r0,0(r4)
> +       addi    r4,r4,4
> +err1;  stw     r0,0(r3)
> +       addi    r3,r3,4
> +
> +13:    bf      cr7*4+2,14f
> +err1;  lhz     r0,0(r4)
> +       addi    r4,r4,2
> +err1;  sth     r0,0(r3)
> +       addi    r3,r3,2
> +
> +14:    bf      cr7*4+3,15f
> +err1;  lbz     r0,0(r4)
> +err1;  stb     r0,0(r3)
> +
> +15:    li      r3,0
> +       blr
> --
> 2.13.6
>

^ permalink raw reply

* Re: [RFC] virtio: Use DMA MAP API for devices without an IOMMU
From: Balbir Singh @ 2018-04-05 11:14 UTC (permalink / raw)
  To: Anshuman Khandual
  Cc: virtualization, linux-kernel@vger.kernel.org, robh,
	Benjamin Herrenschmidt, Michael Ellerman, Joe Perches,
	open list:LINUX FOR POWERPC (32-BIT AND 64-BIT), Markus Elfring,
	David Gibson
In-Reply-To: <20180405105631.9514-1-khandual@linux.vnet.ibm.com>

On Thu, Apr 5, 2018 at 8:56 PM, Anshuman Khandual
<khandual@linux.vnet.ibm.com> wrote:
> There are certian platforms which would like to use SWIOTLB based DMA API
> for bouncing purpose without actually requiring an IOMMU back end. But the
> virtio core does not allow such mechanism. Right now DMA MAP API is only
> selected for devices which have an IOMMU and then the QEMU/host back end
> will process all incoming SG buffer addresses as IOVA instead of simple
> GPA which is the case for simple bounce buffers after being processed with
> SWIOTLB API. To enable this usage, it introduces an architecture specific
> function which will just make virtio core front end select DMA operations
> structure.
>
> Signed-off-by: Anshuman Khandual <khandual@linux.vnet.ibm.com>
> ---
> This RFC is just to get some feedback. Please ignore the function call
> back into the architecture. It can be worked out properly later on. But
> the question is can we have virtio devices in the guest which would like
> to use SWIOTLB based (or any custom DMA API based) bounce buffering with
> out actually being an IOMMU devices emulated by QEMU/host as been with
> the current VIRTIO_F_IOMMU_PLATFORM virtio flag ?
>
>  arch/powerpc/platforms/pseries/iommu.c | 6 ++++++
>  drivers/virtio/virtio_ring.c           | 4 ++++
>  include/linux/virtio.h                 | 2 ++
>  3 files changed, 12 insertions(+)
>
> diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c
> index 06f02960b439..dd15fbddbe89 100644
> --- a/arch/powerpc/platforms/pseries/iommu.c
> +++ b/arch/powerpc/platforms/pseries/iommu.c
> @@ -1396,3 +1396,9 @@ static int __init disable_multitce(char *str)
>  __setup("multitce=", disable_multitce);
>
>  machine_subsys_initcall_sync(pseries, tce_iommu_bus_notifier_init);
> +
> +bool is_virtio_dma_platform(void)
> +{
> +       return true;
> +}
> +EXPORT_SYMBOL(is_virtio_dma_platform);
> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> index 71458f493cf8..9f205a79d378 100644
> --- a/drivers/virtio/virtio_ring.c
> +++ b/drivers/virtio/virtio_ring.c
> @@ -144,6 +144,10 @@ struct vring_virtqueue {
>
>  static bool vring_use_dma_api(struct virtio_device *vdev)
>  {
> +       /* Use DMA API even for virtio devices without an IOMMU */
> +       if (is_virtio_dma_platform())
> +               return true;
> +
>         if (!virtio_has_iommu_quirk(vdev))
>                 return true;
>
> diff --git a/include/linux/virtio.h b/include/linux/virtio.h
> index 988c7355bc22..d8bb83d753ea 100644
> --- a/include/linux/virtio.h
> +++ b/include/linux/virtio.h
> @@ -200,6 +200,8 @@ static inline struct virtio_driver *drv_to_virtio(struct device_driver *drv)
>  int register_virtio_driver(struct virtio_driver *drv);
>  void unregister_virtio_driver(struct virtio_driver *drv);
>
> +extern bool is_virtio_dma_platform(void);
> +

Where is the default implementation for non-pseries platforms? Will they compile
after these changes?

Balbir

^ permalink raw reply

* [RFC 2/2] powerpc/mm/memtrace: Let the arch hotunplug code flush cache
From: Balbir Singh @ 2018-04-05 11:11 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: npiggin, mpe, benh, Balbir Singh
In-Reply-To: <20180405111153.907-1-bsingharora@gmail.com>

Don't do this via custom code, instead now that we have support
in the arch hotplug/hotunplug code, rely on those routines
to do the right thing.

Signed-off-by: Balbir Singh <bsingharora@gmail.com>
---
 arch/powerpc/platforms/powernv/memtrace.c | 17 -----------------
 1 file changed, 17 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/memtrace.c b/arch/powerpc/platforms/powernv/memtrace.c
index de470caf0784..fc222a0c2ac4 100644
--- a/arch/powerpc/platforms/powernv/memtrace.c
+++ b/arch/powerpc/platforms/powernv/memtrace.c
@@ -82,19 +82,6 @@ static const struct file_operations memtrace_fops = {
 	.open	= simple_open,
 };
 
-static void flush_memory_region(u64 base, u64 size)
-{
-	unsigned long line_size = ppc64_caches.l1d.size;
-	u64 end = base + size;
-	u64 addr;
-
-	base = round_down(base, line_size);
-	end = round_up(end, line_size);
-
-	for (addr = base; addr < end; addr += line_size)
-		asm volatile("dcbf 0,%0" : "=r" (addr) :: "memory");
-}
-
 static int check_memblock_online(struct memory_block *mem, void *arg)
 {
 	if (mem->state != MEM_ONLINE)
@@ -132,10 +119,6 @@ static bool memtrace_offline_pages(u32 nid, u64 start_pfn, u64 nr_pages)
 	walk_memory_range(start_pfn, end_pfn, (void *)MEM_OFFLINE,
 			  change_memblock_state);
 
-	/* RCU grace period? */
-	flush_memory_region((u64)__va(start_pfn << PAGE_SHIFT),
-			    nr_pages << PAGE_SHIFT);
-
 	lock_device_hotplug();
 	remove_memory(nid, start_pfn << PAGE_SHIFT, nr_pages << PAGE_SHIFT);
 	unlock_device_hotplug();
-- 
2.13.6

^ permalink raw reply related

* [RFC 1/2] powerpc/mm: Flush cache on memory hot(un)plug
From: Balbir Singh @ 2018-04-05 11:11 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: npiggin, mpe, benh, Balbir Singh

This patch adds support for flushing potentially dirty cache lines when
memory is hot-plugged/hot-un-plugged.  The support is currently limited
to 64 bit systems.

The bug was exposed when mappings for a coherent memory device were
actually hot-unplugged and plugged in back later.  A similar issue was
observed during the development of memtrace, but memtrace does it's own
flushing of region via a custom routine.

These patches do a flush both on hotplug/unplug to clear any stale data
in the cache w.r.t mappings, there is a small race window where a clean
cache line may be created again just prior to tearing down the mapping.
Flushing them on establishment of new mappings helps start with a clean
state.

The patches were tested by disabling the flush routines in memtrace and
doing I/O on the trace file. The system immediately checkstops (quite
reliably, if prior to the hot-unplug of the memtrace region, we memset
the regions we are about to hot unplug). After these patches no custom
flushing is needed in the memtrace code.

Signed-off-by: Balbir Singh <bsingharora@gmail.com>
---
 arch/powerpc/mm/mem.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
index 85245ef97e72..0a8959b15b39 100644
--- a/arch/powerpc/mm/mem.c
+++ b/arch/powerpc/mm/mem.c
@@ -143,6 +143,7 @@ int __meminit arch_add_memory(int nid, u64 start, u64 size, struct vmem_altmap *
 			start, start + size, rc);
 		return -EFAULT;
 	}
+	flush_inval_dcache_range(start, start + size);
 
 	return __add_pages(nid, start_pfn, nr_pages, altmap, want_memblock);
 }
@@ -169,6 +170,7 @@ int __meminit arch_remove_memory(u64 start, u64 size, struct vmem_altmap *altmap
 
 	/* Remove htab bolted mappings for this section of memory */
 	start = (unsigned long)__va(start);
+	flush_inval_dcache_range(start, start + size);
 	ret = remove_section_mapping(start, start + size);
 
 	/* Ensure all vmalloc mappings are flushed in case they also
-- 
2.13.6

^ permalink raw reply related

* [RFC] virtio: Use DMA MAP API for devices without an IOMMU
From: Anshuman Khandual @ 2018-04-05 10:56 UTC (permalink / raw)
  To: virtualization, linux-kernel
  Cc: linuxppc-dev, aik, robh, joe, elfring, david, jasowang, benh, mpe

There are certian platforms which would like to use SWIOTLB based DMA API
for bouncing purpose without actually requiring an IOMMU back end. But the
virtio core does not allow such mechanism. Right now DMA MAP API is only
selected for devices which have an IOMMU and then the QEMU/host back end
will process all incoming SG buffer addresses as IOVA instead of simple
GPA which is the case for simple bounce buffers after being processed with
SWIOTLB API. To enable this usage, it introduces an architecture specific
function which will just make virtio core front end select DMA operations
structure.

Signed-off-by: Anshuman Khandual <khandual@linux.vnet.ibm.com>
---
This RFC is just to get some feedback. Please ignore the function call
back into the architecture. It can be worked out properly later on. But
the question is can we have virtio devices in the guest which would like
to use SWIOTLB based (or any custom DMA API based) bounce buffering with
out actually being an IOMMU devices emulated by QEMU/host as been with
the current VIRTIO_F_IOMMU_PLATFORM virtio flag ?

 arch/powerpc/platforms/pseries/iommu.c | 6 ++++++
 drivers/virtio/virtio_ring.c           | 4 ++++
 include/linux/virtio.h                 | 2 ++
 3 files changed, 12 insertions(+)

diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c
index 06f02960b439..dd15fbddbe89 100644
--- a/arch/powerpc/platforms/pseries/iommu.c
+++ b/arch/powerpc/platforms/pseries/iommu.c
@@ -1396,3 +1396,9 @@ static int __init disable_multitce(char *str)
 __setup("multitce=", disable_multitce);
 
 machine_subsys_initcall_sync(pseries, tce_iommu_bus_notifier_init);
+
+bool is_virtio_dma_platform(void)
+{
+	return true;
+}
+EXPORT_SYMBOL(is_virtio_dma_platform);
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 71458f493cf8..9f205a79d378 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -144,6 +144,10 @@ struct vring_virtqueue {
 
 static bool vring_use_dma_api(struct virtio_device *vdev)
 {
+	/* Use DMA API even for virtio devices without an IOMMU */
+	if (is_virtio_dma_platform())
+		return true;
+
 	if (!virtio_has_iommu_quirk(vdev))
 		return true;
 
diff --git a/include/linux/virtio.h b/include/linux/virtio.h
index 988c7355bc22..d8bb83d753ea 100644
--- a/include/linux/virtio.h
+++ b/include/linux/virtio.h
@@ -200,6 +200,8 @@ static inline struct virtio_driver *drv_to_virtio(struct device_driver *drv)
 int register_virtio_driver(struct virtio_driver *drv);
 void unregister_virtio_driver(struct virtio_driver *drv);
 
+extern bool is_virtio_dma_platform(void);
+
 /* module_virtio_driver() - Helper macro for drivers that don't do
  * anything special in module init/exit.  This eliminates a lot of
  * boilerplate.  Each module may only use this macro once, and
-- 
2.14.1

^ permalink raw reply related

* [PATCH 6/6] powerpc/xive: standardise OPAL_BUSY delays
From: Nicholas Piggin @ 2018-04-05  8:15 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Nicholas Piggin, Benjamin Herrenschmidt, Stewart Smith
In-Reply-To: <20180405081547.13266-1-npiggin@gmail.com>

Convert to using the standard delay poll/delay form.

The XIVE driver:

- Did not previously loop on the OPAL_BUSY_EVENT case.
- Used a 1ms sleep.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/sysdev/xive/native.c | 193 ++++++++++++++++++++++----------------
 1 file changed, 111 insertions(+), 82 deletions(-)

diff --git a/arch/powerpc/sysdev/xive/native.c b/arch/powerpc/sysdev/xive/native.c
index d22aeb0b69e1..682f79dabb4a 100644
--- a/arch/powerpc/sysdev/xive/native.c
+++ b/arch/powerpc/sysdev/xive/native.c
@@ -103,14 +103,18 @@ EXPORT_SYMBOL_GPL(xive_native_populate_irq_data);
 
 int xive_native_configure_irq(u32 hw_irq, u32 target, u8 prio, u32 sw_irq)
 {
-	s64 rc;
+	s64 rc = OPAL_BUSY;
 
-	for (;;) {
+	while (rc == OPAL_BUSY || rc == OPAL_BUSY_EVENT) {
 		rc = opal_xive_set_irq_config(hw_irq, target, prio, sw_irq);
-		if (rc != OPAL_BUSY)
-			break;
-		msleep(1);
+		if (rc == OPAL_BUSY_EVENT) {
+			msleep(OPAL_BUSY_DELAY_MS);
+			opal_poll_events(NULL);
+		} else if (rc == OPAL_BUSY) {
+			msleep(OPAL_BUSY_DELAY_MS);
+		}
 	}
+
 	return rc == 0 ? 0 : -ENXIO;
 }
 EXPORT_SYMBOL_GPL(xive_native_configure_irq);
@@ -159,12 +163,17 @@ int xive_native_configure_queue(u32 vp_id, struct xive_q *q, u8 prio,
 	}
 
 	/* Configure and enable the queue in HW */
-	for (;;) {
+	rc = OPAL_BUSY;
+	while (rc == OPAL_BUSY || rc == OPAL_BUSY_EVENT) {
 		rc = opal_xive_set_queue_info(vp_id, prio, qpage_phys, order, flags);
-		if (rc != OPAL_BUSY)
-			break;
-		msleep(1);
+		if (rc == OPAL_BUSY_EVENT) {
+			msleep(OPAL_BUSY_DELAY_MS);
+			opal_poll_events(NULL);
+		} else if (rc == OPAL_BUSY) {
+			msleep(OPAL_BUSY_DELAY_MS);
+		}
 	}
+
 	if (rc) {
 		pr_err("Error %lld setting queue for prio %d\n", rc, prio);
 		rc = -EIO;
@@ -183,14 +192,17 @@ EXPORT_SYMBOL_GPL(xive_native_configure_queue);
 
 static void __xive_native_disable_queue(u32 vp_id, struct xive_q *q, u8 prio)
 {
-	s64 rc;
+	s64 rc = OPAL_BUSY;
 
 	/* Disable the queue in HW */
-	for (;;) {
+	while (rc == OPAL_BUSY || rc == OPAL_BUSY_EVENT) {
 		rc = opal_xive_set_queue_info(vp_id, prio, 0, 0, 0);
-		if (rc != OPAL_BUSY)
-			break;
-		msleep(1);
+		if (rc == OPAL_BUSY_EVENT) {
+			msleep(OPAL_BUSY_DELAY_MS);
+			opal_poll_events(NULL);
+		} else if (rc == OPAL_BUSY) {
+			msleep(OPAL_BUSY_DELAY_MS);
+		}
 	}
 	if (rc)
 		pr_err("Error %lld disabling queue for prio %d\n", rc, prio);
@@ -240,7 +252,7 @@ static int xive_native_get_ipi(unsigned int cpu, struct xive_cpu *xc)
 {
 	struct device_node *np;
 	unsigned int chip_id;
-	s64 irq;
+	s64 rc = OPAL_BUSY;
 
 	/* Find the chip ID */
 	np = of_get_cpu_node(cpu, NULL);
@@ -250,33 +262,39 @@ static int xive_native_get_ipi(unsigned int cpu, struct xive_cpu *xc)
 	}
 
 	/* Allocate an IPI and populate info about it */
-	for (;;) {
-		irq = opal_xive_allocate_irq(chip_id);
-		if (irq == OPAL_BUSY) {
-			msleep(1);
-			continue;
-		}
-		if (irq < 0) {
-			pr_err("Failed to allocate IPI on CPU %d\n", cpu);
-			return -ENXIO;
+	while (rc == OPAL_BUSY || rc == OPAL_BUSY_EVENT) {
+		rc = opal_xive_allocate_irq(chip_id);
+		if (rc == OPAL_BUSY_EVENT) {
+			msleep(OPAL_BUSY_DELAY_MS);
+			opal_poll_events(NULL);
+		} else if (rc == OPAL_BUSY) {
+			msleep(OPAL_BUSY_DELAY_MS);
 		}
-		xc->hw_ipi = irq;
-		break;
 	}
+	if (rc < 0) {
+		pr_err("Failed to allocate IPI on CPU %d\n", cpu);
+		return -ENXIO;
+	}
+	xc->hw_ipi = rc;
+
 	return 0;
 }
 #endif /* CONFIG_SMP */
 
 u32 xive_native_alloc_irq(void)
 {
-	s64 rc;
+	s64 rc = OPAL_BUSY;
 
-	for (;;) {
+	while (rc == OPAL_BUSY || rc == OPAL_BUSY_EVENT) {
 		rc = opal_xive_allocate_irq(OPAL_XIVE_ANY_CHIP);
-		if (rc != OPAL_BUSY)
-			break;
-		msleep(1);
+		if (rc == OPAL_BUSY_EVENT) {
+			msleep(OPAL_BUSY_DELAY_MS);
+			opal_poll_events(NULL);
+		} else if (rc == OPAL_BUSY) {
+			msleep(OPAL_BUSY_DELAY_MS);
+		}
 	}
+
 	if (rc < 0)
 		return 0;
 	return rc;
@@ -285,11 +303,16 @@ EXPORT_SYMBOL_GPL(xive_native_alloc_irq);
 
 void xive_native_free_irq(u32 irq)
 {
-	for (;;) {
-		s64 rc = opal_xive_free_irq(irq);
-		if (rc != OPAL_BUSY)
-			break;
-		msleep(1);
+	s64 rc = OPAL_BUSY;
+
+	while (rc == OPAL_BUSY || rc == OPAL_BUSY_EVENT) {
+		rc = opal_xive_free_irq(irq);
+		if (rc == OPAL_BUSY_EVENT) {
+			msleep(OPAL_BUSY_DELAY_MS);
+			opal_poll_events(NULL);
+		} else if (rc == OPAL_BUSY) {
+			msleep(OPAL_BUSY_DELAY_MS);
+		}
 	}
 }
 EXPORT_SYMBOL_GPL(xive_native_free_irq);
@@ -297,20 +320,11 @@ EXPORT_SYMBOL_GPL(xive_native_free_irq);
 #ifdef CONFIG_SMP
 static void xive_native_put_ipi(unsigned int cpu, struct xive_cpu *xc)
 {
-	s64 rc;
-
 	/* Free the IPI */
 	if (!xc->hw_ipi)
 		return;
-	for (;;) {
-		rc = opal_xive_free_irq(xc->hw_ipi);
-		if (rc == OPAL_BUSY) {
-			msleep(1);
-			continue;
-		}
-		xc->hw_ipi = 0;
-		break;
-	}
+	xive_native_free_irq(xc->hw_ipi);
+	xc->hw_ipi = 0;
 }
 #endif /* CONFIG_SMP */
 
@@ -381,7 +395,7 @@ static void xive_native_eoi(u32 hw_irq)
 
 static void xive_native_setup_cpu(unsigned int cpu, struct xive_cpu *xc)
 {
-	s64 rc;
+	s64 rc = OPAL_BUSY;
 	u32 vp;
 	__be64 vp_cam_be;
 	u64 vp_cam;
@@ -392,12 +406,16 @@ static void xive_native_setup_cpu(unsigned int cpu, struct xive_cpu *xc)
 	/* Enable the pool VP */
 	vp = xive_pool_vps + cpu;
 	pr_debug("CPU %d setting up pool VP 0x%x\n", cpu, vp);
-	for (;;) {
+	while (rc == OPAL_BUSY || rc == OPAL_BUSY_EVENT) {
 		rc = opal_xive_set_vp_info(vp, OPAL_XIVE_VP_ENABLED, 0);
-		if (rc != OPAL_BUSY)
-			break;
-		msleep(1);
+		if (rc == OPAL_BUSY_EVENT) {
+			msleep(OPAL_BUSY_DELAY_MS);
+			opal_poll_events(NULL);
+		} else if (rc == OPAL_BUSY) {
+			msleep(OPAL_BUSY_DELAY_MS);
+		}
 	}
+
 	if (rc) {
 		pr_err("Failed to enable pool VP on CPU %d\n", cpu);
 		return;
@@ -425,7 +443,7 @@ static void xive_native_setup_cpu(unsigned int cpu, struct xive_cpu *xc)
 
 static void xive_native_teardown_cpu(unsigned int cpu, struct xive_cpu *xc)
 {
-	s64 rc;
+	s64 rc = OPAL_BUSY;
 	u32 vp;
 
 	if (xive_pool_vps == XIVE_INVALID_VP)
@@ -436,11 +454,14 @@ static void xive_native_teardown_cpu(unsigned int cpu, struct xive_cpu *xc)
 
 	/* Disable it */
 	vp = xive_pool_vps + cpu;
-	for (;;) {
+	while (rc == OPAL_BUSY || rc == OPAL_BUSY_EVENT) {
 		rc = opal_xive_set_vp_info(vp, 0, 0);
-		if (rc != OPAL_BUSY)
-			break;
-		msleep(1);
+		if (rc == OPAL_BUSY_EVENT) {
+			msleep(OPAL_BUSY_DELAY_MS);
+			opal_poll_events(NULL);
+		} else if (rc == OPAL_BUSY) {
+			msleep(OPAL_BUSY_DELAY_MS);
+		}
 	}
 }
 
@@ -627,7 +648,7 @@ static bool xive_native_provision_pages(void)
 
 u32 xive_native_alloc_vp_block(u32 max_vcpus)
 {
-	s64 rc;
+	s64 rc = OPAL_BUSY;
 	u32 order;
 
 	order = fls(max_vcpus) - 1;
@@ -637,25 +658,25 @@ u32 xive_native_alloc_vp_block(u32 max_vcpus)
 	pr_debug("VP block alloc, for max VCPUs %d use order %d\n",
 		 max_vcpus, order);
 
-	for (;;) {
+	while (rc == OPAL_BUSY || rc == OPAL_BUSY_EVENT) {
 		rc = opal_xive_alloc_vp_block(order);
-		switch (rc) {
-		case OPAL_BUSY:
-			msleep(1);
-			break;
-		case OPAL_XIVE_PROVISIONING:
+		if (rc == OPAL_BUSY_EVENT) {
+			msleep(OPAL_BUSY_DELAY_MS);
+			opal_poll_events(NULL);
+		} else if (rc == OPAL_BUSY) {
+			msleep(OPAL_BUSY_DELAY_MS);
+		} else if (rc == OPAL_XIVE_PROVISIONING) {
 			if (!xive_native_provision_pages())
 				return XIVE_INVALID_VP;
-			break;
-		default:
-			if (rc < 0) {
-				pr_err("OPAL failed to allocate VCPUs order %d, err %lld\n",
-				       order, rc);
-				return XIVE_INVALID_VP;
-			}
-			return rc;
+			rc = OPAL_BUSY; /* go around again */
 		}
 	}
+	if (rc < 0) {
+		pr_err("OPAL failed to allocate VCPUs order %d, err %lld\n",
+		       order, rc);
+		return XIVE_INVALID_VP;
+	}
+	return rc;
 }
 EXPORT_SYMBOL_GPL(xive_native_alloc_vp_block);
 
@@ -674,30 +695,38 @@ EXPORT_SYMBOL_GPL(xive_native_free_vp_block);
 
 int xive_native_enable_vp(u32 vp_id, bool single_escalation)
 {
-	s64 rc;
+	s64 rc = OPAL_BUSY;
 	u64 flags = OPAL_XIVE_VP_ENABLED;
 
 	if (single_escalation)
 		flags |= OPAL_XIVE_VP_SINGLE_ESCALATION;
-	for (;;) {
+
+	while (rc == OPAL_BUSY || rc == OPAL_BUSY_EVENT) {
 		rc = opal_xive_set_vp_info(vp_id, flags, 0);
-		if (rc != OPAL_BUSY)
-			break;
-		msleep(1);
+		if (rc == OPAL_BUSY_EVENT) {
+			msleep(OPAL_BUSY_DELAY_MS);
+			opal_poll_events(NULL);
+		} else if (rc == OPAL_BUSY) {
+			msleep(OPAL_BUSY_DELAY_MS);
+		}
 	}
+
 	return rc ? -EIO : 0;
 }
 EXPORT_SYMBOL_GPL(xive_native_enable_vp);
 
 int xive_native_disable_vp(u32 vp_id)
 {
-	s64 rc;
+	s64 rc = OPAL_BUSY;
 
-	for (;;) {
+	while (rc == OPAL_BUSY || rc == OPAL_BUSY_EVENT) {
 		rc = opal_xive_set_vp_info(vp_id, 0, 0);
-		if (rc != OPAL_BUSY)
-			break;
-		msleep(1);
+		if (rc == OPAL_BUSY_EVENT) {
+			msleep(OPAL_BUSY_DELAY_MS);
+			opal_poll_events(NULL);
+		} else if (rc == OPAL_BUSY) {
+			msleep(OPAL_BUSY_DELAY_MS);
+		}
 	}
 	return rc ? -EIO : 0;
 }
-- 
2.16.3

^ permalink raw reply related

* [PATCH 5/6] powerpc/powernv: OPAL dump support standardise OPAL_BUSY delays
From: Nicholas Piggin @ 2018-04-05  8:15 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Nicholas Piggin, Benjamin Herrenschmidt, Stewart Smith
In-Reply-To: <20180405081547.13266-1-npiggin@gmail.com>

Convert to using the standard delay poll/delay form.

The dump code:

- Did not previously delay or sleep in the OPAL_BUSY case.
- Used a 20ms sleep.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/platforms/powernv/opal-dump.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/platforms/powernv/opal-dump.c b/arch/powerpc/platforms/powernv/opal-dump.c
index 0dc8fa4e0af2..603c4ffdb45c 100644
--- a/arch/powerpc/platforms/powernv/opal-dump.c
+++ b/arch/powerpc/platforms/powernv/opal-dump.c
@@ -264,8 +264,10 @@ static int64_t dump_read_data(struct dump_obj *dump)
 	while (rc == OPAL_BUSY || rc == OPAL_BUSY_EVENT) {
 		rc = opal_dump_read(dump->id, addr);
 		if (rc == OPAL_BUSY_EVENT) {
+			msleep(OPAL_BUSY_DELAY_MS);
 			opal_poll_events(NULL);
-			msleep(20);
+		} else if (rc == OPAL_BUSY) {
+			msleep(OPAL_BUSY_DELAY_MS);
 		}
 	}
 
-- 
2.16.3

^ permalink raw reply related

* [PATCH 4/6] powerpc/powernv: OPAL NVRAM driver standardise OPAL_BUSY delays
From: Nicholas Piggin @ 2018-04-05  8:15 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Nicholas Piggin, Benjamin Herrenschmidt, Stewart Smith
In-Reply-To: <20180405081547.13266-1-npiggin@gmail.com>

Convert to using the standard delay poll/delay form.

The NVRAM driver:

- Did not previously delay or sleep in its OPAL_BUSY loop.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/platforms/powernv/opal-nvram.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/platforms/powernv/opal-nvram.c b/arch/powerpc/platforms/powernv/opal-nvram.c
index 9db4398ded5d..732732bddc28 100644
--- a/arch/powerpc/platforms/powernv/opal-nvram.c
+++ b/arch/powerpc/platforms/powernv/opal-nvram.c
@@ -11,6 +11,7 @@
 
 #define DEBUG
 
+#include <linux/delay.h>
 #include <linux/kernel.h>
 #include <linux/init.h>
 #include <linux/of.h>
@@ -56,8 +57,12 @@ static ssize_t opal_nvram_write(char *buf, size_t count, loff_t *index)
 
 	while (rc == OPAL_BUSY || rc == OPAL_BUSY_EVENT) {
 		rc = opal_write_nvram(__pa(buf), count, off);
-		if (rc == OPAL_BUSY_EVENT)
+		if (rc == OPAL_BUSY_EVENT) {
+			msleep(OPAL_BUSY_DELAY_MS);
 			opal_poll_events(NULL);
+		} else if (rc == OPAL_BUSY) {
+			msleep(OPAL_BUSY_DELAY_MS);
+		}
 	}
 	*index += count;
 	return count;
-- 
2.16.3

^ permalink raw reply related

* [PATCH 3/6] powerpc/powernv: OPAL platform standardise OPAL_BUSY loops
From: Nicholas Piggin @ 2018-04-05  8:15 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Nicholas Piggin, Benjamin Herrenschmidt, Stewart Smith
In-Reply-To: <20180405081547.13266-1-npiggin@gmail.com>

Convert to using the standard delay poll/delay form.

The platform code:

- Used delay when called from a schedule()able context.
- Did not previously delay or sleep in the OPAL_BUSY_EVENT case.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/platforms/powernv/opal.c  |  8 +++++---
 arch/powerpc/platforms/powernv/setup.c | 16 ++++++++++------
 2 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/opal.c b/arch/powerpc/platforms/powernv/opal.c
index c15182765ff5..fb13bcabe609 100644
--- a/arch/powerpc/platforms/powernv/opal.c
+++ b/arch/powerpc/platforms/powernv/opal.c
@@ -896,10 +896,12 @@ void opal_shutdown(void)
 	 */
 	while (rc == OPAL_BUSY || rc == OPAL_BUSY_EVENT) {
 		rc = opal_sync_host_reboot();
-		if (rc == OPAL_BUSY)
+		if (rc == OPAL_BUSY_EVENT) {
+			msleep(OPAL_BUSY_DELAY_MS);
 			opal_poll_events(NULL);
-		else
-			mdelay(10);
+		} else {
+			msleep(OPAL_BUSY_DELAY_MS);
+		}
 	}
 
 	/* Unregister memory dump region */
diff --git a/arch/powerpc/platforms/powernv/setup.c b/arch/powerpc/platforms/powernv/setup.c
index 092715b9674b..6ea79d906784 100644
--- a/arch/powerpc/platforms/powernv/setup.c
+++ b/arch/powerpc/platforms/powernv/setup.c
@@ -187,10 +187,12 @@ static void  __noreturn pnv_restart(char *cmd)
 
 	while (rc == OPAL_BUSY || rc == OPAL_BUSY_EVENT) {
 		rc = opal_cec_reboot();
-		if (rc == OPAL_BUSY_EVENT)
+		if (rc == OPAL_BUSY_EVENT) {
+			msleep(OPAL_BUSY_DELAY_MS);
 			opal_poll_events(NULL);
-		else
-			mdelay(10);
+		} else if (rc == OPAL_BUSY) {
+			msleep(OPAL_BUSY_DELAY_MS);
+		}
 	}
 	for (;;)
 		opal_poll_events(NULL);
@@ -204,10 +206,12 @@ static void __noreturn pnv_power_off(void)
 
 	while (rc == OPAL_BUSY || rc == OPAL_BUSY_EVENT) {
 		rc = opal_cec_power_down(0);
-		if (rc == OPAL_BUSY_EVENT)
+		if (rc == OPAL_BUSY_EVENT) {
+			msleep(OPAL_BUSY_DELAY_MS);
 			opal_poll_events(NULL);
-		else
-			mdelay(10);
+		} else if (rc == OPAL_BUSY) {
+			msleep(OPAL_BUSY_DELAY_MS);
+		}
 	}
 	for (;;)
 		opal_poll_events(NULL);
-- 
2.16.3

^ permalink raw reply related

* [PATCH 2/6] powerpc/powernv: OPAL RTC driver standardise OPAL_BUSY loops
From: Nicholas Piggin @ 2018-04-05  8:15 UTC (permalink / raw)
  To: linuxppc-dev
  Cc: Nicholas Piggin, Benjamin Herrenschmidt, Stewart Smith, linux-rtc
In-Reply-To: <20180405081547.13266-1-npiggin@gmail.com>

Convert to using the standard delay poll/delay form.

The OPAL RTC driver:

- Did not previously delay or sleep in the OPAL_BUSY_EVENT case.
  There have been scheduling delays of up to 50 seconds observed here
  (BMC reboot can do it), which this should fix.

Cc: linux-rtc@vger.kernel.org
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/platforms/powernv/opal-rtc.c |  6 ++++--
 drivers/rtc/rtc-opal.c                    | 33 ++++++++++++++++++++-----------
 2 files changed, 25 insertions(+), 14 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/opal-rtc.c b/arch/powerpc/platforms/powernv/opal-rtc.c
index f8868864f373..f530cf62594d 100644
--- a/arch/powerpc/platforms/powernv/opal-rtc.c
+++ b/arch/powerpc/platforms/powernv/opal-rtc.c
@@ -48,10 +48,12 @@ unsigned long __init opal_get_boot_time(void)
 
 	while (rc == OPAL_BUSY || rc == OPAL_BUSY_EVENT) {
 		rc = opal_rtc_read(&__y_m_d, &__h_m_s_ms);
-		if (rc == OPAL_BUSY_EVENT)
+		if (rc == OPAL_BUSY_EVENT) {
+			mdelay(OPAL_BUSY_DELAY_MS);
 			opal_poll_events(NULL);
-		else if (rc == OPAL_BUSY)
+		} else if (rc == OPAL_BUSY) {
 			mdelay(10);
+		}
 	}
 	if (rc != OPAL_SUCCESS)
 		return 0;
diff --git a/drivers/rtc/rtc-opal.c b/drivers/rtc/rtc-opal.c
index 304e891e35fc..cddcc4749d39 100644
--- a/drivers/rtc/rtc-opal.c
+++ b/drivers/rtc/rtc-opal.c
@@ -57,7 +57,7 @@ static void tm_to_opal(struct rtc_time *tm, u32 *y_m_d, u64 *h_m_s_ms)
 
 static int opal_get_rtc_time(struct device *dev, struct rtc_time *tm)
 {
-	long rc = OPAL_BUSY;
+	s64 rc = OPAL_BUSY;
 	int retries = 10;
 	u32 y_m_d;
 	u64 h_m_s_ms;
@@ -66,13 +66,17 @@ static int opal_get_rtc_time(struct device *dev, struct rtc_time *tm)
 
 	while (rc == OPAL_BUSY || rc == OPAL_BUSY_EVENT) {
 		rc = opal_rtc_read(&__y_m_d, &__h_m_s_ms);
-		if (rc == OPAL_BUSY_EVENT)
+		if (rc == OPAL_BUSY_EVENT) {
+			msleep(OPAL_BUSY_DELAY_MS);
 			opal_poll_events(NULL);
-		else if (retries-- && (rc == OPAL_HARDWARE
-				       || rc == OPAL_INTERNAL_ERROR))
+		} else if (rc == OPAL_BUSY) {
 			msleep(10);
-		else if (rc != OPAL_BUSY && rc != OPAL_BUSY_EVENT)
-			break;
+		} else if (rc == OPAL_HARDWARE || rc == OPAL_INTERNAL_ERROR) {
+			if (retries--) {
+				msleep(10); /* Wait 10ms before retry */
+				rc = OPAL_BUSY; /* go around again */
+			}
+		}
 	}
 
 	if (rc != OPAL_SUCCESS)
@@ -87,21 +91,26 @@ static int opal_get_rtc_time(struct device *dev, struct rtc_time *tm)
 
 static int opal_set_rtc_time(struct device *dev, struct rtc_time *tm)
 {
-	long rc = OPAL_BUSY;
+	s64 rc = OPAL_BUSY;
 	int retries = 10;
 	u32 y_m_d = 0;
 	u64 h_m_s_ms = 0;
 
 	tm_to_opal(tm, &y_m_d, &h_m_s_ms);
+
 	while (rc == OPAL_BUSY || rc == OPAL_BUSY_EVENT) {
 		rc = opal_rtc_write(y_m_d, h_m_s_ms);
-		if (rc == OPAL_BUSY_EVENT)
+		if (rc == OPAL_BUSY_EVENT) {
+			msleep(OPAL_BUSY_DELAY_MS);
 			opal_poll_events(NULL);
-		else if (retries-- && (rc == OPAL_HARDWARE
-				       || rc == OPAL_INTERNAL_ERROR))
+		} else if (rc == OPAL_BUSY) {
 			msleep(10);
-		else if (rc != OPAL_BUSY && rc != OPAL_BUSY_EVENT)
-			break;
+		} else if (rc == OPAL_HARDWARE || rc == OPAL_INTERNAL_ERROR) {
+			if (retries--) {
+				msleep(10); /* Wait 10ms before retry */
+				rc = OPAL_BUSY; /* go around again */
+			}
+		}
 	}
 
 	return rc == OPAL_SUCCESS ? 0 : -EIO;
-- 
2.16.3

^ permalink raw reply related

* [PATCH 1/6] powerpc/powernv: define a standard delay for OPAL_BUSY type retry loops
From: Nicholas Piggin @ 2018-04-05  8:15 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Nicholas Piggin, Benjamin Herrenschmidt, Stewart Smith
In-Reply-To: <20180405081547.13266-1-npiggin@gmail.com>

This is the start of an effort to tidy up and standardise all the
delays. Existing loops have a range of delay/sleep periods from 1ms
to 20ms, and some have no delay. They all loop forever except rtc,
which times out after 10 retries, and that uses 10ms delays. So use
10ms as our standard delay. The OPAL maintainer agrees 10ms is a
reasonable starting point.

The idea is to use the same recipe everywhere, once this is proven to
work then it will be documented as an OPAL API standard. Then both
firmware and OS can agree, and if a particular call needs something
else, then that can be documented with reasoning.

This is not the end-all of this effort, it's just a relatively easy
change that fixes some existing high latency delays. There should be
provision for standardising timeouts and/or interruptible loops where
possible, so non-fatal firmware errors don't cause hangs.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/include/asm/opal.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/powerpc/include/asm/opal.h b/arch/powerpc/include/asm/opal.h
index 12e70fb58700..fcf3ed5b8b18 100644
--- a/arch/powerpc/include/asm/opal.h
+++ b/arch/powerpc/include/asm/opal.h
@@ -21,6 +21,9 @@
 /* We calculate number of sg entries based on PAGE_SIZE */
 #define SG_ENTRIES_PER_NODE ((PAGE_SIZE - 16) / sizeof(struct opal_sg_entry))
 
+/* Default time to sleep or delay between OPAL_BUSY/OPAL_BUSY_EVENT loops */
+#define OPAL_BUSY_DELAY_MS	10
+
 /* /sys/firmware/opal */
 extern struct kobject *opal_kobj;
 
-- 
2.16.3

^ permalink raw reply related

* [PATCH 0/6] first step of standardising OPAL_BUSY handling
From: Nicholas Piggin @ 2018-04-05  8:15 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Nicholas Piggin, Benjamin Herrenschmidt, Stewart Smith

Patch 1 explains most of the reasoning.
Patch 1+2 and possibly 4 (just that we've seen a bug caused by the
RTC driver but not yet one caused by NVRAM) could be backported as
bugfixes, in most other cases the changes are inconsequential or
unlikely to be a problem.

Thanks,
Nick

Nicholas Piggin (6):
  powerpc/powernv: define a standard delay for OPAL_BUSY type retry
    loops
  powerpc/powernv: OPAL RTC driver standardise OPAL_BUSY loops
  powerpc/powernv: OPAL platform standardise OPAL_BUSY loops
  powerpc/powernv: OPAL NVRAM driver standardise OPAL_BUSY delays
  powerpc/powernv: OPAL dump support standardise OPAL_BUSY delays
  powerpc/xive: standardise OPAL_BUSY delays

 arch/powerpc/include/asm/opal.h             |   3 +
 arch/powerpc/platforms/powernv/opal-dump.c  |   4 +-
 arch/powerpc/platforms/powernv/opal-nvram.c |   7 +-
 arch/powerpc/platforms/powernv/opal-rtc.c   |   6 +-
 arch/powerpc/platforms/powernv/opal.c       |   8 +-
 arch/powerpc/platforms/powernv/setup.c      |  16 ++-
 arch/powerpc/sysdev/xive/native.c           | 193 ++++++++++++++++------------
 drivers/rtc/rtc-opal.c                      |  33 +++--
 8 files changed, 163 insertions(+), 107 deletions(-)

-- 
2.16.3

^ permalink raw reply

* [PATCH v2 3/3] powerpc/mce: Handle memcpy_mcsafe
From: Balbir Singh @ 2018-04-05  7:15 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: linux-nvdimm, npiggin, mpe, oohall, Balbir Singh
In-Reply-To: <20180405071500.22320-1-bsingharora@gmail.com>

Add a blocking notifier callback to be called in real-mode
on machine check exceptions for UE (ld/st) errors only.
The patch registers a callback on boot to be notified
of machine check exceptions and returns a NOTIFY_STOP when
a page of interest is seen as the source of the machine
check exception. This page of interest is a ZONE_DEVICE
page and hence for now, for memcpy_mcsafe to work, the page
needs to belong to ZONE_DEVICE and memcpy_mcsafe should be
used to access the memory.

The patch also modifies the NIP of the exception context
to go back to the fixup handler (in memcpy_mcsafe) and does
not print any error message as the error is treated as
returned via a return value and handled.

Signed-off-by: Balbir Singh <bsingharora@gmail.com>
---
 arch/powerpc/include/asm/mce.h |  3 +-
 arch/powerpc/kernel/mce.c      | 77 ++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 77 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/include/asm/mce.h b/arch/powerpc/include/asm/mce.h
index 3a1226e9b465..a76638e3e47e 100644
--- a/arch/powerpc/include/asm/mce.h
+++ b/arch/powerpc/include/asm/mce.h
@@ -125,7 +125,8 @@ struct machine_check_event {
 			enum MCE_UeErrorType ue_error_type:8;
 			uint8_t		effective_address_provided;
 			uint8_t		physical_address_provided;
-			uint8_t		reserved_1[5];
+			uint8_t		error_return;
+			uint8_t		reserved_1[4];
 			uint64_t	effective_address;
 			uint64_t	physical_address;
 			uint8_t		reserved_2[8];
diff --git a/arch/powerpc/kernel/mce.c b/arch/powerpc/kernel/mce.c
index efdd16a79075..b9e4881fa8c5 100644
--- a/arch/powerpc/kernel/mce.c
+++ b/arch/powerpc/kernel/mce.c
@@ -28,7 +28,9 @@
 #include <linux/percpu.h>
 #include <linux/export.h>
 #include <linux/irq_work.h>
+#include <linux/extable.h>
 
+#include <asm/extable.h>
 #include <asm/machdep.h>
 #include <asm/mce.h>
 
@@ -54,6 +56,52 @@ static struct irq_work mce_event_process_work = {
 
 DECLARE_WORK(mce_ue_event_work, machine_process_ue_event);
 
+static BLOCKING_NOTIFIER_HEAD(mce_notifier_list);
+
+int register_mce_notifier(struct notifier_block *nb)
+{
+	return blocking_notifier_chain_register(&mce_notifier_list, nb);
+}
+EXPORT_SYMBOL_GPL(register_mce_notifier);
+
+int unregister_mce_notifier(struct notifier_block *nb)
+{
+	return blocking_notifier_chain_unregister(&mce_notifier_list, nb);
+}
+EXPORT_SYMBOL_GPL(unregister_mce_notifier);
+
+
+static int check_memcpy_mcsafe(struct notifier_block *nb,
+			unsigned long val, void *data)
+{
+	/*
+	 * val contains the physical_address of the bad address
+	 */
+	unsigned long pfn = val >> PAGE_SHIFT;
+	struct page *page = realmode_pfn_to_page(pfn);
+	int rc = NOTIFY_DONE;
+
+	if (!page)
+		goto out;
+
+	if (is_zone_device_page(page))	/* for HMM and PMEM */
+		rc = NOTIFY_STOP;
+out:
+	return rc;
+}
+
+struct notifier_block memcpy_mcsafe_nb = {
+	.priority = 0,
+	.notifier_call = check_memcpy_mcsafe,
+};
+
+int  mce_mcsafe_register(void)
+{
+	register_mce_notifier(&memcpy_mcsafe_nb);
+	return 0;
+}
+arch_initcall(mce_mcsafe_register);
+
 static void mce_set_error_info(struct machine_check_event *mce,
 			       struct mce_error_info *mce_err)
 {
@@ -151,9 +199,31 @@ void save_mce_event(struct pt_regs *regs, long handled,
 		mce->u.ue_error.effective_address_provided = true;
 		mce->u.ue_error.effective_address = addr;
 		if (phys_addr != ULONG_MAX) {
+			int rc;
+			const struct exception_table_entry *entry;
+
+			/*
+			 * Once we have the physical address, we check to
+			 * see if the current nip has a fixup entry.
+			 * Having a fixup entry plus the notifier stating
+			 * that it can handle the exception is an indication
+			 * that we should return to the fixup entry and
+			 * return an error from there
+			 */
 			mce->u.ue_error.physical_address_provided = true;
 			mce->u.ue_error.physical_address = phys_addr;
-			machine_check_ue_event(mce);
+
+			rc = blocking_notifier_call_chain(&mce_notifier_list,
+							phys_addr, NULL);
+			if (rc & NOTIFY_STOP_MASK) {
+				entry = search_exception_tables(regs->nip);
+				if (entry != NULL) {
+					mce->u.ue_error.error_return = 1;
+					regs->nip = extable_fixup(entry);
+				} else
+					machine_check_ue_event(mce);
+			} else
+				machine_check_ue_event(mce);
 		}
 	}
 	return;
@@ -208,7 +278,6 @@ void release_mce_event(void)
 	get_mce_event(NULL, true);
 }
 
-
 /*
  * Queue up the MCE event which then can be handled later.
  */
@@ -239,6 +308,10 @@ void machine_check_queue_event(void)
 	if (!get_mce_event(&evt, MCE_EVENT_RELEASE))
 		return;
 
+	if (evt.error_type == MCE_ERROR_TYPE_UE &&
+			evt.u.ue_error.error_return == 1)
+		return;
+
 	index = __this_cpu_inc_return(mce_queue_count) - 1;
 	/* If queue is full, just return for now. */
 	if (index >= MAX_MC_EVT) {
-- 
2.13.6

^ permalink raw reply related

* [PATCH v2 2/3] powerpc/memcpy: Add memcpy_mcsafe for pmem
From: Balbir Singh @ 2018-04-05  7:14 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: linux-nvdimm, npiggin, mpe, oohall, Balbir Singh
In-Reply-To: <20180405071500.22320-1-bsingharora@gmail.com>

The pmem infrastructure uses memcpy_mcsafe in the pmem
layer so as to convert machine check excpetions into
a return value on failure in case a machine check
exception is encoutered during the memcpy.

This patch largely borrows from the copyuser_power7
logic and does not add the VMX optimizations, largely
to keep the patch simple. If needed those optimizations
can be folded in.

Signed-off-by: Balbir Singh <bsingharora@gmail.com>
Acked-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/include/asm/string.h   |   2 +
 arch/powerpc/lib/Makefile           |   2 +-
 arch/powerpc/lib/memcpy_mcsafe_64.S | 212 ++++++++++++++++++++++++++++++++++++
 3 files changed, 215 insertions(+), 1 deletion(-)
 create mode 100644 arch/powerpc/lib/memcpy_mcsafe_64.S

diff --git a/arch/powerpc/include/asm/string.h b/arch/powerpc/include/asm/string.h
index 9b8cedf618f4..b7e872a64726 100644
--- a/arch/powerpc/include/asm/string.h
+++ b/arch/powerpc/include/asm/string.h
@@ -30,7 +30,9 @@ extern void * memcpy_flushcache(void *,const void *,__kernel_size_t);
 #ifdef CONFIG_PPC64
 #define __HAVE_ARCH_MEMSET32
 #define __HAVE_ARCH_MEMSET64
+#define __HAVE_ARCH_MEMCPY_MCSAFE
 
+extern int memcpy_mcsafe(void *dst, const void *src, __kernel_size_t sz);
 extern void *__memset16(uint16_t *, uint16_t v, __kernel_size_t);
 extern void *__memset32(uint32_t *, uint32_t v, __kernel_size_t);
 extern void *__memset64(uint64_t *, uint64_t v, __kernel_size_t);
diff --git a/arch/powerpc/lib/Makefile b/arch/powerpc/lib/Makefile
index 3c29c9009bbf..048afee9f518 100644
--- a/arch/powerpc/lib/Makefile
+++ b/arch/powerpc/lib/Makefile
@@ -24,7 +24,7 @@ endif
 
 obj64-y	+= copypage_64.o copyuser_64.o mem_64.o hweight_64.o \
 	   copyuser_power7.o string_64.o copypage_power7.o memcpy_power7.o \
-	   memcpy_64.o memcmp_64.o pmem.o
+	   memcpy_64.o memcmp_64.o pmem.o memcpy_mcsafe_64.o
 
 obj64-$(CONFIG_SMP)	+= locks.o
 obj64-$(CONFIG_ALTIVEC)	+= vmx-helper.o
diff --git a/arch/powerpc/lib/memcpy_mcsafe_64.S b/arch/powerpc/lib/memcpy_mcsafe_64.S
new file mode 100644
index 000000000000..e7eaa9b6cded
--- /dev/null
+++ b/arch/powerpc/lib/memcpy_mcsafe_64.S
@@ -0,0 +1,212 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) IBM Corporation, 2011
+ * Derived from copyuser_power7.s by Anton Blanchard <anton@au.ibm.com>
+ * Author - Balbir Singh <bsingharora@gmail.com>
+ */
+#include <asm/ppc_asm.h>
+#include <asm/errno.h>
+
+	.macro err1
+100:
+	EX_TABLE(100b,.Ldo_err1)
+	.endm
+
+	.macro err2
+200:
+	EX_TABLE(200b,.Ldo_err2)
+	.endm
+
+.Ldo_err2:
+	ld	r22,STK_REG(R22)(r1)
+	ld	r21,STK_REG(R21)(r1)
+	ld	r20,STK_REG(R20)(r1)
+	ld	r19,STK_REG(R19)(r1)
+	ld	r18,STK_REG(R18)(r1)
+	ld	r17,STK_REG(R17)(r1)
+	ld	r16,STK_REG(R16)(r1)
+	ld	r15,STK_REG(R15)(r1)
+	ld	r14,STK_REG(R14)(r1)
+	addi	r1,r1,STACKFRAMESIZE
+.Ldo_err1:
+	li	r3,-EFAULT
+	blr
+
+
+_GLOBAL(memcpy_mcsafe)
+	cmpldi	r5,16
+	blt	.Lshort_copy
+
+.Lcopy:
+	/* Get the source 8B aligned */
+	neg	r6,r4
+	mtocrf	0x01,r6
+	clrldi	r6,r6,(64-3)
+
+	bf	cr7*4+3,1f
+err1;	lbz	r0,0(r4)
+	addi	r4,r4,1
+err1;	stb	r0,0(r3)
+	addi	r3,r3,1
+
+1:	bf	cr7*4+2,2f
+err1;	lhz	r0,0(r4)
+	addi	r4,r4,2
+err1;	sth	r0,0(r3)
+	addi	r3,r3,2
+
+2:	bf	cr7*4+1,3f
+err1;	lwz	r0,0(r4)
+	addi	r4,r4,4
+err1;	stw	r0,0(r3)
+	addi	r3,r3,4
+
+3:	sub	r5,r5,r6
+	cmpldi	r5,128
+	blt	5f
+
+	mflr	r0
+	stdu	r1,-STACKFRAMESIZE(r1)
+	std	r14,STK_REG(R14)(r1)
+	std	r15,STK_REG(R15)(r1)
+	std	r16,STK_REG(R16)(r1)
+	std	r17,STK_REG(R17)(r1)
+	std	r18,STK_REG(R18)(r1)
+	std	r19,STK_REG(R19)(r1)
+	std	r20,STK_REG(R20)(r1)
+	std	r21,STK_REG(R21)(r1)
+	std	r22,STK_REG(R22)(r1)
+	std	r0,STACKFRAMESIZE+16(r1)
+
+	srdi	r6,r5,7
+	mtctr	r6
+
+	/* Now do cacheline (128B) sized loads and stores. */
+	.align	5
+4:
+err2;	ld	r0,0(r4)
+err2;	ld	r6,8(r4)
+err2;	ld	r7,16(r4)
+err2;	ld	r8,24(r4)
+err2;	ld	r9,32(r4)
+err2;	ld	r10,40(r4)
+err2;	ld	r11,48(r4)
+err2;	ld	r12,56(r4)
+err2;	ld	r14,64(r4)
+err2;	ld	r15,72(r4)
+err2;	ld	r16,80(r4)
+err2;	ld	r17,88(r4)
+err2;	ld	r18,96(r4)
+err2;	ld	r19,104(r4)
+err2;	ld	r20,112(r4)
+err2;	ld	r21,120(r4)
+	addi	r4,r4,128
+err2;	std	r0,0(r3)
+err2;	std	r6,8(r3)
+err2;	std	r7,16(r3)
+err2;	std	r8,24(r3)
+err2;	std	r9,32(r3)
+err2;	std	r10,40(r3)
+err2;	std	r11,48(r3)
+err2;	std	r12,56(r3)
+err2;	std	r14,64(r3)
+err2;	std	r15,72(r3)
+err2;	std	r16,80(r3)
+err2;	std	r17,88(r3)
+err2;	std	r18,96(r3)
+err2;	std	r19,104(r3)
+err2;	std	r20,112(r3)
+err2;	std	r21,120(r3)
+	addi	r3,r3,128
+	bdnz	4b
+
+	clrldi	r5,r5,(64-7)
+
+	ld	r14,STK_REG(R14)(r1)
+	ld	r15,STK_REG(R15)(r1)
+	ld	r16,STK_REG(R16)(r1)
+	ld	r17,STK_REG(R17)(r1)
+	ld	r18,STK_REG(R18)(r1)
+	ld	r19,STK_REG(R19)(r1)
+	ld	r20,STK_REG(R20)(r1)
+	ld	r21,STK_REG(R21)(r1)
+	ld	r22,STK_REG(R22)(r1)
+	addi	r1,r1,STACKFRAMESIZE
+
+	/* Up to 127B to go */
+5:	srdi	r6,r5,4
+	mtocrf	0x01,r6
+
+6:	bf	cr7*4+1,7f
+err1;	ld	r0,0(r4)
+err1;	ld	r6,8(r4)
+err1;	ld	r7,16(r4)
+err1;	ld	r8,24(r4)
+err1;	ld	r9,32(r4)
+err1;	ld	r10,40(r4)
+err1;	ld	r11,48(r4)
+err1;	ld	r12,56(r4)
+	addi	r4,r4,64
+err1;	std	r0,0(r3)
+err1;	std	r6,8(r3)
+err1;	std	r7,16(r3)
+err1;	std	r8,24(r3)
+err1;	std	r9,32(r3)
+err1;	std	r10,40(r3)
+err1;	std	r11,48(r3)
+err1;	std	r12,56(r3)
+	addi	r3,r3,64
+
+	/* Up to 63B to go */
+7:	bf	cr7*4+2,8f
+err1;	ld	r0,0(r4)
+err1;	ld	r6,8(r4)
+err1;	ld	r7,16(r4)
+err1;	ld	r8,24(r4)
+	addi	r4,r4,32
+err1;	std	r0,0(r3)
+err1;	std	r6,8(r3)
+err1;	std	r7,16(r3)
+err1;	std	r8,24(r3)
+	addi	r3,r3,32
+
+	/* Up to 31B to go */
+8:	bf	cr7*4+3,9f
+err1;	ld	r0,0(r4)
+err1;	ld	r6,8(r4)
+	addi	r4,r4,16
+err1;	std	r0,0(r3)
+err1;	std	r6,8(r3)
+	addi	r3,r3,16
+
+9:	clrldi	r5,r5,(64-4)
+
+	/* Up to 15B to go */
+.Lshort_copy:
+	mtocrf	0x01,r5
+	bf	cr7*4+0,12f
+err1;	lwz	r0,0(r4)	/* Less chance of a reject with word ops */
+err1;	lwz	r6,4(r4)
+	addi	r4,r4,8
+err1;	stw	r0,0(r3)
+err1;	stw	r6,4(r3)
+	addi	r3,r3,8
+
+12:	bf	cr7*4+1,13f
+err1;	lwz	r0,0(r4)
+	addi	r4,r4,4
+err1;	stw	r0,0(r3)
+	addi	r3,r3,4
+
+13:	bf	cr7*4+2,14f
+err1;	lhz	r0,0(r4)
+	addi	r4,r4,2
+err1;	sth	r0,0(r3)
+	addi	r3,r3,2
+
+14:	bf	cr7*4+3,15f
+err1;	lbz	r0,0(r4)
+err1;	stb	r0,0(r3)
+
+15:	li	r3,0
+	blr
-- 
2.13.6

^ permalink raw reply related

* [PATCH v2 1/3] powerpc/mce: Bug fixes for MCE handling in kernel space
From: Balbir Singh @ 2018-04-05  7:14 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: linux-nvdimm, npiggin, mpe, oohall, Balbir Singh
In-Reply-To: <20180405071500.22320-1-bsingharora@gmail.com>

The code currently assumes PAGE_SHIFT as the shift value of
the pfn, this works correctly (mostly) for user space pages,
but the correct thing to do is

1. Extract the shift value returned via the pte-walk API's
2. Use the shift value to access the instruction address.

Note, the final physical address still use PAGE_SHIFT for
computation. handle_ierror() is not modified and handle_derror()
is modified just for extracting the correct instruction
address.

This is largely due to __find_linux_pte() returning pfn's
shifted by pdshift. The code is much more generic and can
handle shift values returned.

Fixes: ba41e1e1ccb9 ("powerpc/mce: Hookup derror (load/store) UE errors")

Signed-off-by: Balbir Singh <bsingharora@gmail.com>
---
 arch/powerpc/kernel/mce_power.c | 26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/arch/powerpc/kernel/mce_power.c b/arch/powerpc/kernel/mce_power.c
index fe6fc63251fe..bd9754def479 100644
--- a/arch/powerpc/kernel/mce_power.c
+++ b/arch/powerpc/kernel/mce_power.c
@@ -36,7 +36,8 @@
  * Convert an address related to an mm to a PFN. NOTE: we are in real
  * mode, we could potentially race with page table updates.
  */
-static unsigned long addr_to_pfn(struct pt_regs *regs, unsigned long addr)
+static unsigned long addr_to_pfn(struct pt_regs *regs, unsigned long addr,
+		unsigned int *shift)
 {
 	pte_t *ptep;
 	unsigned long flags;
@@ -49,13 +50,15 @@ static unsigned long addr_to_pfn(struct pt_regs *regs, unsigned long addr)
 
 	local_irq_save(flags);
 	if (mm == current->mm)
-		ptep = find_current_mm_pte(mm->pgd, addr, NULL, NULL);
+		ptep = find_current_mm_pte(mm->pgd, addr, NULL, shift);
 	else
-		ptep = find_init_mm_pte(addr, NULL);
+		ptep = find_init_mm_pte(addr, shift);
 	local_irq_restore(flags);
 	if (!ptep || pte_special(*ptep))
 		return ULONG_MAX;
-	return pte_pfn(*ptep);
+	if (!*shift)
+		*shift = PAGE_SHIFT;
+	return (pte_val(*ptep) & PTE_RPN_MASK) >> *shift;
 }
 
 /* flush SLBs and reload */
@@ -353,15 +356,16 @@ static int mce_find_instr_ea_and_pfn(struct pt_regs *regs, uint64_t *addr,
 	unsigned long pfn, instr_addr;
 	struct instruction_op op;
 	struct pt_regs tmp = *regs;
+	unsigned int shift;
 
-	pfn = addr_to_pfn(regs, regs->nip);
+	pfn = addr_to_pfn(regs, regs->nip, &shift);
 	if (pfn != ULONG_MAX) {
-		instr_addr = (pfn << PAGE_SHIFT) + (regs->nip & ~PAGE_MASK);
+		instr_addr = (pfn << shift) + (regs->nip & ((1 << shift) - 1));
 		instr = *(unsigned int *)(instr_addr);
 		if (!analyse_instr(&op, &tmp, instr)) {
-			pfn = addr_to_pfn(regs, op.ea);
+			pfn = addr_to_pfn(regs, op.ea, &shift);
 			*addr = op.ea;
-			*phys_addr = (pfn << PAGE_SHIFT);
+			*phys_addr = (pfn << shift);
 			return 0;
 		}
 		/*
@@ -435,12 +439,14 @@ static int mce_handle_ierror(struct pt_regs *regs,
 			if (mce_err->severity == MCE_SEV_ERROR_SYNC &&
 				table[i].error_type == MCE_ERROR_TYPE_UE) {
 				unsigned long pfn;
+				unsigned int shift;
 
 				if (get_paca()->in_mce < MAX_MCE_DEPTH) {
-					pfn = addr_to_pfn(regs, regs->nip);
+					pfn = addr_to_pfn(regs, regs->nip,
+							&shift);
 					if (pfn != ULONG_MAX) {
 						*phys_addr =
-							(pfn << PAGE_SHIFT);
+							(pfn << shift);
 						handled = 1;
 					}
 				}
-- 
2.13.6

^ permalink raw reply related

* [PATCH v2 0/3]  Add support for memcpy_mcsafe
From: Balbir Singh @ 2018-04-05  7:14 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: linux-nvdimm, npiggin, mpe, oohall, Balbir Singh

memcpy_mcsafe() is an API currently used by the pmem subsystem to convert
errors while doing a memcpy (machine check exception errors) to a return
value. This patchset consists of three patches

1. The first patch is a bug fix to handle machine check errors correctly
while walking the page tables in kernel mode, due to huge pmd/pud sizes
2. The second patch adds memcpy_mcsafe() support, this is largely derived
from existing code
3. The third patch registers for callbacks on machine check exceptions and
in them uses specialized knowledge of the type of page to decide whether
to handle the MCE as is or to return to a fixup address present in
memcpy_mcsafe(). If a fixup address is used, then we return an error
value of -EFAULT to the caller.

Testing

A large part of the testing was done under a simulator by selectively
inserting machine check exceptions in a test driver doing memcpy_mcsafe
via ioctls.

Changelog v2
 - Fix the logic of shifting in addr_to_pfn
 - Use shift consistently instead of PAGE_SHIFT
 - Fix a typo in patch1

Balbir Singh (3):
  powerpc/mce: Bug fixes for MCE handling in kernel space
  powerpc/memcpy: Add memcpy_mcsafe for pmem
  powerpc/mce: Handle memcpy_mcsafe

 arch/powerpc/include/asm/mce.h      |   3 +-
 arch/powerpc/include/asm/string.h   |   2 +
 arch/powerpc/kernel/mce.c           |  77 ++++++++++++-
 arch/powerpc/kernel/mce_power.c     |  26 +++--
 arch/powerpc/lib/Makefile           |   2 +-
 arch/powerpc/lib/memcpy_mcsafe_64.S | 212 ++++++++++++++++++++++++++++++++++++
 6 files changed, 308 insertions(+), 14 deletions(-)
 create mode 100644 arch/powerpc/lib/memcpy_mcsafe_64.S

-- 
2.13.6

^ permalink raw reply

* Re: [PATCH v2 03/19] powerpc: Mark variables as unused
From: LEROY Christophe @ 2018-04-05  7:01 UTC (permalink / raw)
  To: Michael Ellerman
  Cc: Benjamin Herrenschmidt, Paul Mackerras, linuxppc-dev,
	linux-kernel, Mathieu Malaterre
In-Reply-To: <87fu4atct4.fsf@concordia.ellerman.id.au>

Michael Ellerman <mpe@ellerman.id.au> a =C3=A9crit=C2=A0:

> LEROY Christophe <christophe.leroy@c-s.fr> writes:
>
>> Mathieu Malaterre <malat@debian.org> a =C3=A9crit=C2=A0:
>>
>>> Add gcc attribute unused for two variables. Fix warnings treated as err=
ors
>>> with W=3D1:
>>>
>>>   arch/powerpc/kernel/prom_init.c:1388:8: error: variable =E2=80=98path=
=E2=80=99 set
>>> but not used [-Werror=3Dunused-but-set-variable]
>>>
>>> Suggested-by: Christophe Leroy <christophe.leroy@c-s.fr>
>>> Signed-off-by: Mathieu Malaterre <malat@debian.org>
>>> ---
>>> v2: move path within ifdef DEBUG_PROM
>>>
>>>  arch/powerpc/kernel/prom_init.c | 6 +++---
>>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/arch/powerpc/kernel/prom_init.c
>>> b/arch/powerpc/kernel/prom_init.c
>>> index acf4b2e0530c..4163b11abb6c 100644
>>> --- a/arch/powerpc/kernel/prom_init.c
>>> +++ b/arch/powerpc/kernel/prom_init.c
>>> @@ -603,7 +603,7 @@ static void __init early_cmdline_parse(void)
>>>  	const char *opt;
>>>
>>>  	char *p;
>>> -	int l =3D 0;
>>> +	int l __maybe_unused =3D 0;
>>>
>>>  	prom_cmd_line[0] =3D 0;
>>>  	p =3D prom_cmd_line;
>>> @@ -1385,7 +1385,7 @@ static void __init reserve_mem(u64 base, u64 size=
)
>>>  static void __init prom_init_mem(void)
>>>  {
>>>  	phandle node;
>>> -	char *path, type[64];
>>> +	char *path __maybe_unused, type[64];
>>
>> You should enclose that in an ifdef DEBUG_PROM instead of hiding the war=
ning
>
> I disagree, the result is horrible:
>
>  static void __init prom_init_mem(void)
>  {
> 	phandle node;
> -	char *path, type[64];
> +#ifdef DEBUG_PROM
> +	char *path;
> +#endif
> +	char type[64];
> 	unsigned int plen;
> 	cell_t *p, *endp;
> 	__be32 val;
>
>
> The right fix is to move the debug logic into a helper, and put the path
> in there, eg. something like (not tested):
>
> diff --git a/arch/powerpc/kernel/prom_init.c=20=20
>=20b/arch/powerpc/kernel/prom_init.c
> index f9d6befb55a6..b02fa2ccc70b 100644
> --- a/arch/powerpc/kernel/prom_init.c
> +++ b/arch/powerpc/kernel/prom_init.c
> @@ -1389,6 +1389,18 @@ static void __init reserve_mem(u64 base, u64 size)
>  	mem_reserve_cnt =3D cnt + 1;
>  }
>
> +#ifdef DEBUG_PROM
> +static void prom_debug_path(phandle node)
> +{
> +	char *path;
> +	path =3D prom_scratch;
> +	memset(path, 0, PROM_SCRATCH_SIZE);
> +	call_prom("package-to-path", 3, 1, node, path, PROM_SCRATCH_SIZE-1);
> +	prom_debug("  node %s :\n", path);
> +}
> +#else
> +static void prom_debug_path(phandle node) { }

Or put the ifdef inside the function to avoid double definition ?

> +#endif /* DEBUG_PROM */
>  /*
>   * Initialize memory allocation mechanism, parse "memory" nodes and
>   * obtain that way the top of memory and RMO to setup out local allocato=
r
> @@ -1441,11 +1453,7 @@ static void __init prom_init_mem(void)
>  		p =3D regbuf;
>  		endp =3D p + (plen / sizeof(cell_t));
>
> -#ifdef DEBUG_PROM
> -		memset(path, 0, PROM_SCRATCH_SIZE);
> -		call_prom("package-to-path", 3, 1, node, path, PROM_SCRATCH_SIZE-1);
> -		prom_debug("  node %s :\n", path);
> -#endif /* DEBUG_PROM */
> +		prom_debug_path(node);
>
>  		while ((endp - p) >=3D (rac + rsc)) {
>  			unsigned long base, size;
>
>
> Although that also begs the question of why the hell do we need path at
> all, and not just use prom_scratch directly?

Wondering the same, why not use prom_scratch directly

Christophe

>
> cheers

^ permalink raw reply

* Re: [RESEND 2/3] powerpc/memcpy: Add memcpy_mcsafe for pmem
From: Nicholas Piggin @ 2018-04-05  6:45 UTC (permalink / raw)
  To: Balbir Singh
  Cc: Dan Williams, Michael Ellerman, linuxppc-dev, linux-nvdimm,
	Christoph Hellwig, Matthew Wilcox, Luck, Tony
In-Reply-To: <20180405155307.49f748f3@gmail.com>

On Thu, 5 Apr 2018 15:53:07 +1000
Balbir Singh <bsingharora@gmail.com> wrote:

> On Thu, 5 Apr 2018 15:04:05 +1000
> Nicholas Piggin <npiggin@gmail.com> wrote:
> 
> > On Wed, 4 Apr 2018 20:00:52 -0700
> > Dan Williams <dan.j.williams@intel.com> wrote:
> >   
> > > [ adding Matthew, Christoph, and Tony  ]
> > > 
> > > On Wed, Apr 4, 2018 at 4:57 PM, Nicholas Piggin <npiggin@gmail.com> wrote:    
> > > > On Thu,  5 Apr 2018 09:19:42 +1000
> > > > Balbir Singh <bsingharora@gmail.com> wrote:
> > > >      
> > > >> The pmem infrastructure uses memcpy_mcsafe in the pmem
> > > >> layer so as to convert machine check excpetions into
> > > >> a return value on failure in case a machine check
> > > >> exception is encoutered during the memcpy.
> > > >>
> > > >> This patch largely borrows from the copyuser_power7
> > > >> logic and does not add the VMX optimizations, largely
> > > >> to keep the patch simple. If needed those optimizations
> > > >> can be folded in.      
> > > >
> > > > So memcpy_mcsafe doesn't return number of bytes copied?
> > > > Huh, well that makes it simple.      
> > > 
> > > Well, not in current kernels, but we need to add that support or
> > > remove the direct call to copy_to_iter() in fs/dax.c. I'm looking
> > > right now to add "bytes remaining" support to the x86 memcpy_mcsafe(),
> > > but for copy_to_user we also need to handle bytes remaining for write
> > > faults. That fix is hopefully something that can land in an early
> > > 4.17-rc, but it won't be ready for -rc1.    
> > 
> > I wonder if the powerpc implementation should just go straight to
> > counting bytes. Backporting to this interface would be trivial, but
> > it would just mean there's only one variant of the code to support.
> > That's up to Balbir though.
> >   
> 
> I'm thinking about it, I wonder what "bytes remaining" mean in pmem context
> in the context of a machine check exception. Also, do we want to be byte
> accurate or cache-line accurate for the bytes remaining? The former is much
> easier than the latter :)

The ideal would be a linear measure of how much of your copy reached
(or can reach) non-volatile storage with nothing further copied. You
may have to allow for some relaxing of the semantics depending on
what the architecture can support.

What's the problem with just counting bytes copied like usercopy --
why is that harder than cacheline accuracy?

> I'd rather implement the existing interface and port/support the new interface
> as it becomes available

Fair enough.

Thanks,
Nick

^ permalink raw reply

* [PATCH] powerpc/64s/idle: POWER9 restore AMOR after deep sleep
From: Nicholas Piggin @ 2018-04-05  6:10 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Nicholas Piggin, Vaidyanathan Srinivasan

POWER8 restores AMOR when waking from deep sleep, but POWER9 does not,
because it does not go through the subcore restore.

Have POWER9 restore it in core restore.

Cc: Vaidyanathan Srinivasan <svaidy@linux.vnet.ibm.com>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---

Do we need this guy after waking from deep sleep?

This code is a little messy at the moment, it can be a bit tricky to
see exactly what we've restored. I'm doing a bit of work to tidy it
up and make it clearer, but that's not going to make 4.17 or backports.

 arch/powerpc/kernel/idle_book3s.S | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/powerpc/kernel/idle_book3s.S b/arch/powerpc/kernel/idle_book3s.S
index bc4e391d031e..e72e385a4973 100644
--- a/arch/powerpc/kernel/idle_book3s.S
+++ b/arch/powerpc/kernel/idle_book3s.S
@@ -857,6 +857,8 @@ BEGIN_FTR_SECTION
 	mtspr	SPRN_PTCR,r4
 	ld	r4,_RPR(r1)
 	mtspr	SPRN_RPR,r4
+	ld	r4,_AMOR(r1)
+	mtspr	SPRN_AMOR,r4
 END_FTR_SECTION_IFSET(CPU_FTR_ARCH_300)
 
 	ld	r4,_TSCR(r1)
-- 
2.16.3

^ permalink raw reply related

* [PATCH 2/2] powerpc/64s: Fix POWER9 DD2.2 and above in cputable features
From: Nicholas Piggin @ 2018-04-05  5:57 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Nicholas Piggin
In-Reply-To: <20180405055755.30090-1-npiggin@gmail.com>

The CPU_FTR_POWER9_DD2_1 flag is intended to be set for DD2.1 and
above (which is what the dt_cpu_ftrs setup does). Fix cputable for
DD2.2 to match.

This came about due to patches b5af4f279323 ("powerpc: Add CPU feature
bits for TM bug workarounds on POWER9 v2.2"), and 9e9626ed3a4a
("powerpc/64s: Fix POWER9 DD2.2 and above in DT CPU features") being
in-flight at once. The latter patch fixed dt_cpu_ftrs like this one
does. The former changed cputable to match dt_cpu_ftrs.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/include/asm/cputable.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/include/asm/cputable.h b/arch/powerpc/include/asm/cputable.h
index 4e332f3531c5..931dda8be87c 100644
--- a/arch/powerpc/include/asm/cputable.h
+++ b/arch/powerpc/include/asm/cputable.h
@@ -467,7 +467,8 @@ static inline void cpu_feature_keys_init(void) { }
 			     (~CPU_FTR_SAO))
 #define CPU_FTRS_POWER9_DD2_0 CPU_FTRS_POWER9
 #define CPU_FTRS_POWER9_DD2_1 (CPU_FTRS_POWER9 | CPU_FTR_POWER9_DD2_1)
-#define CPU_FTRS_POWER9_DD2_2 (CPU_FTRS_POWER9 | CPU_FTR_P9_TM_HV_ASSIST | \
+#define CPU_FTRS_POWER9_DD2_2 (CPU_FTRS_POWER9 | CPU_FTR_POWER9_DD2_1 | \
+			       CPU_FTR_P9_TM_HV_ASSIST | \
 			       CPU_FTR_P9_TM_XER_SO_BUG)
 #define CPU_FTRS_CELL	(CPU_FTR_LWSYNC | \
 	    CPU_FTR_PPCAS_ARCH_V2 | CPU_FTR_CTRL | \
-- 
2.16.3

^ permalink raw reply related

* [PATCH 1/2] powerpc/64s: Fix pkey support in dt_cpu_ftrs, add CPU_FTR_PKEY bit
From: Nicholas Piggin @ 2018-04-05  5:57 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Nicholas Piggin, Ram Pai
In-Reply-To: <20180405055755.30090-1-npiggin@gmail.com>

The pkey code added a CPU_FTR_PKEY bit, but did not add it to the
dt_cpu_ftrs feature set. Although capability is supported by all
processors in the base dt_cpu_ftrs set for 64s, it's a significant
and sufficiently well defined feature to make it optional. So add
it as a quirk for now, which can be versioned out then controlled
by the firmware (once dt_cpu_ftrs gains versioning support).

Fixes: cf43d3b264 ("powerpc: Enable pkey subsystem ")
Cc: Ram Pai <linuxram@us.ibm.com>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/kernel/dt_cpu_ftrs.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/powerpc/kernel/dt_cpu_ftrs.c b/arch/powerpc/kernel/dt_cpu_ftrs.c
index ed7605d8fd2d..e88fbb1fdb8f 100644
--- a/arch/powerpc/kernel/dt_cpu_ftrs.c
+++ b/arch/powerpc/kernel/dt_cpu_ftrs.c
@@ -729,6 +729,13 @@ static __init void cpufeatures_cpu_quirks(void)
 		cur_cpu_spec->cpu_features &= ~(CPU_FTR_DAWR);
 		cur_cpu_spec->cpu_features |= CPU_FTR_P9_TLBIE_BUG;
 	}
+
+	/*
+	 * PKEY was not in the initial base or feature node
+	 * specification, but it should become optional in the next
+	 * cpu feature version sequence.
+	 */
+	cur_cpu_spec->cpu_features |= CPU_FTR_PKEY;
 }
 
 static void __init cpufeatures_setup_finished(void)
-- 
2.16.3

^ permalink raw reply related

* [PATCH 0/2] a couple of cpu ftrs fixes
From: Nicholas Piggin @ 2018-04-05  5:57 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Nicholas Piggin

These are a couple of differences between cputable and dt_cpu_ftrs
I noticed with CPU_FTR bits. We have to be a bit careful now to keep
them in sync when we change one or the other.

Nicholas Piggin (2):
  powerpc/64s: Fix pkey support in dt_cpu_ftrs, add CPU_FTR_PKEY bit
  powerpc/64s: Fix POWER9 DD2.2 and above in cputable features

 arch/powerpc/include/asm/cputable.h | 3 ++-
 arch/powerpc/kernel/dt_cpu_ftrs.c   | 7 +++++++
 2 files changed, 9 insertions(+), 1 deletion(-)

-- 
2.16.3

^ permalink raw reply

* Re: [PATCH v2 03/19] powerpc: Mark variables as unused
From: Michael Ellerman @ 2018-04-05  5:57 UTC (permalink / raw)
  To: LEROY Christophe, Mathieu Malaterre
  Cc: linux-kernel, linuxppc-dev, Paul Mackerras,
	Benjamin Herrenschmidt
In-Reply-To: <20180329181422.Horde.4gvIQbpSZkuyC4Yzg06PKw2@messagerie.si.c-s.fr>

LEROY Christophe <christophe.leroy@c-s.fr> writes:

> Mathieu Malaterre <malat@debian.org> a =C3=A9crit=C2=A0:
>
>> Add gcc attribute unused for two variables. Fix warnings treated as erro=
rs
>> with W=3D1:
>>
>>   arch/powerpc/kernel/prom_init.c:1388:8: error: variable =E2=80=98path=
=E2=80=99 set=20=20
>> but not used [-Werror=3Dunused-but-set-variable]
>>
>> Suggested-by: Christophe Leroy <christophe.leroy@c-s.fr>
>> Signed-off-by: Mathieu Malaterre <malat@debian.org>
>> ---
>> v2: move path within ifdef DEBUG_PROM
>>
>>  arch/powerpc/kernel/prom_init.c | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/powerpc/kernel/prom_init.c=20=20
>> b/arch/powerpc/kernel/prom_init.c
>> index acf4b2e0530c..4163b11abb6c 100644
>> --- a/arch/powerpc/kernel/prom_init.c
>> +++ b/arch/powerpc/kernel/prom_init.c
>> @@ -603,7 +603,7 @@ static void __init early_cmdline_parse(void)
>>  	const char *opt;
>>
>>  	char *p;
>> -	int l =3D 0;
>> +	int l __maybe_unused =3D 0;
>>
>>  	prom_cmd_line[0] =3D 0;
>>  	p =3D prom_cmd_line;
>> @@ -1385,7 +1385,7 @@ static void __init reserve_mem(u64 base, u64 size)
>>  static void __init prom_init_mem(void)
>>  {
>>  	phandle node;
>> -	char *path, type[64];
>> +	char *path __maybe_unused, type[64];
>
> You should enclose that in an ifdef DEBUG_PROM instead of hiding the warn=
ing

I disagree, the result is horrible:

 static void __init prom_init_mem(void)
 {
	phandle node;
-	char *path, type[64];
+#ifdef DEBUG_PROM
+	char *path;
+#endif
+	char type[64];
	unsigned int plen;
	cell_t *p, *endp;
	__be32 val;


The right fix is to move the debug logic into a helper, and put the path
in there, eg. something like (not tested):

diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_ini=
t.c
index f9d6befb55a6..b02fa2ccc70b 100644
--- a/arch/powerpc/kernel/prom_init.c
+++ b/arch/powerpc/kernel/prom_init.c
@@ -1389,6 +1389,18 @@ static void __init reserve_mem(u64 base, u64 size)
 	mem_reserve_cnt =3D cnt + 1;
 }
=20
+#ifdef DEBUG_PROM
+static void prom_debug_path(phandle node)
+{
+	char *path;
+	path =3D prom_scratch;
+	memset(path, 0, PROM_SCRATCH_SIZE);
+	call_prom("package-to-path", 3, 1, node, path, PROM_SCRATCH_SIZE-1);
+	prom_debug("  node %s :\n", path);
+}
+#else
+static void prom_debug_path(phandle node) { }
+#endif /* DEBUG_PROM */
 /*
  * Initialize memory allocation mechanism, parse "memory" nodes and
  * obtain that way the top of memory and RMO to setup out local allocator
@@ -1441,11 +1453,7 @@ static void __init prom_init_mem(void)
 		p =3D regbuf;
 		endp =3D p + (plen / sizeof(cell_t));
=20
-#ifdef DEBUG_PROM
-		memset(path, 0, PROM_SCRATCH_SIZE);
-		call_prom("package-to-path", 3, 1, node, path, PROM_SCRATCH_SIZE-1);
-		prom_debug("  node %s :\n", path);
-#endif /* DEBUG_PROM */
+		prom_debug_path(node);
=20
 		while ((endp - p) >=3D (rac + rsc)) {
 			unsigned long base, size;


Although that also begs the question of why the hell do we need path at
all, and not just use prom_scratch directly?

cheers

^ permalink raw reply related

* Re: [RESEND 2/3] powerpc/memcpy: Add memcpy_mcsafe for pmem
From: Balbir Singh @ 2018-04-05  5:53 UTC (permalink / raw)
  To: Nicholas Piggin
  Cc: Dan Williams, Michael Ellerman, linuxppc-dev, linux-nvdimm,
	Christoph Hellwig, Matthew Wilcox, Luck, Tony
In-Reply-To: <20180405150405.5b902b41@roar.ozlabs.ibm.com>

On Thu, 5 Apr 2018 15:04:05 +1000
Nicholas Piggin <npiggin@gmail.com> wrote:

> On Wed, 4 Apr 2018 20:00:52 -0700
> Dan Williams <dan.j.williams@intel.com> wrote:
> 
> > [ adding Matthew, Christoph, and Tony  ]
> > 
> > On Wed, Apr 4, 2018 at 4:57 PM, Nicholas Piggin <npiggin@gmail.com> wrote:  
> > > On Thu,  5 Apr 2018 09:19:42 +1000
> > > Balbir Singh <bsingharora@gmail.com> wrote:
> > >    
> > >> The pmem infrastructure uses memcpy_mcsafe in the pmem
> > >> layer so as to convert machine check excpetions into
> > >> a return value on failure in case a machine check
> > >> exception is encoutered during the memcpy.
> > >>
> > >> This patch largely borrows from the copyuser_power7
> > >> logic and does not add the VMX optimizations, largely
> > >> to keep the patch simple. If needed those optimizations
> > >> can be folded in.    
> > >
> > > So memcpy_mcsafe doesn't return number of bytes copied?
> > > Huh, well that makes it simple.    
> > 
> > Well, not in current kernels, but we need to add that support or
> > remove the direct call to copy_to_iter() in fs/dax.c. I'm looking
> > right now to add "bytes remaining" support to the x86 memcpy_mcsafe(),
> > but for copy_to_user we also need to handle bytes remaining for write
> > faults. That fix is hopefully something that can land in an early
> > 4.17-rc, but it won't be ready for -rc1.  
> 
> I wonder if the powerpc implementation should just go straight to
> counting bytes. Backporting to this interface would be trivial, but
> it would just mean there's only one variant of the code to support.
> That's up to Balbir though.
> 

I'm thinking about it, I wonder what "bytes remaining" mean in pmem context
in the context of a machine check exception. Also, do we want to be byte
accurate or cache-line accurate for the bytes remaining? The former is much
easier than the latter :)


I'd rather implement the existing interface and port/support the new interface
as it becomes available

Balbir Singh.

^ permalink raw reply

* [PATCH] powerpc/64s: Fix dt_cpu_ftrs to have restore_cpu clear unwanted LPCR bits
From: Nicholas Piggin @ 2018-04-05  5:50 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Nicholas Piggin, stable

Presently the dt_cpu_ftrs restore_cpu will only add bits to the LPCR
for secondaries, but some bits must be removed (e.g., UPRT for HPT).
Not clearing these bits on secondaries causes checkstops when booting
with disable_radix.

restore_cpu can not just set LPCR, because it is also called by the
idle wakeup code which relies on opal_slw_set_reg to restore the value
of LPCR, at least on P8 which does not save LPCR to stack in the idle
code.

Fix this by including a mask of bits to clear from LPCR as well, which
is used by restore_cpu.

This is a little messy now, but it's a minimal fix that can be
backported.  Longer term, the idle SPR save/restore code can be
reworked to completely avoid calls to restore_cpu, then restore_cpu
would be able to unconditionally set LPCR to match boot processor
environment.

Fixes: 5a61ef74f269f ("powerpc/64s: Support new device tree binding for discovering CPU features")
Cc: stable@vger.kernel.org # v4.12+
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---

I tested this fix and it boots a POWER9 with disable_radix, where
previously it checkstopped. Deeper idle states seem to work too,
after they're enabled with a firmware override.


 arch/powerpc/kernel/dt_cpu_ftrs.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/kernel/dt_cpu_ftrs.c b/arch/powerpc/kernel/dt_cpu_ftrs.c
index 11a3a4fed3fb..ed7605d8fd2d 100644
--- a/arch/powerpc/kernel/dt_cpu_ftrs.c
+++ b/arch/powerpc/kernel/dt_cpu_ftrs.c
@@ -83,6 +83,7 @@ static int hv_mode;
 
 static struct {
 	u64	lpcr;
+	u64	lpcr_clear;
 	u64	hfscr;
 	u64	fscr;
 } system_registers;
@@ -91,6 +92,8 @@ static void (*init_pmu_registers)(void);
 
 static void __restore_cpu_cpufeatures(void)
 {
+	u64 lpcr;
+
 	/*
 	 * LPCR is restored by the power on engine already. It can be changed
 	 * after early init e.g., by radix enable, and we have no unified API
@@ -103,8 +106,10 @@ static void __restore_cpu_cpufeatures(void)
 	 * The best we can do to accommodate secondary boot and idle restore
 	 * for now is "or" LPCR with existing.
 	 */
-
-	mtspr(SPRN_LPCR, system_registers.lpcr | mfspr(SPRN_LPCR));
+	lpcr = mfspr(SPRN_LPCR);
+	lpcr |= system_registers.lpcr;
+	lpcr &= ~system_registers.lpcr_clear;
+	mtspr(SPRN_LPCR, lpcr);
 	if (hv_mode) {
 		mtspr(SPRN_LPID, 0);
 		mtspr(SPRN_HFSCR, system_registers.hfscr);
@@ -324,8 +329,9 @@ static int __init feat_enable_mmu_hash_v3(struct dt_cpu_feature *f)
 {
 	u64 lpcr;
 
+	system_registers.lpcr_clear |= (LPCR_ISL | LPCR_UPRT | LPCR_HR);
 	lpcr = mfspr(SPRN_LPCR);
-	lpcr &= ~LPCR_ISL;
+	lpcr &= ~(LPCR_ISL | LPCR_UPRT | LPCR_HR);
 	mtspr(SPRN_LPCR, lpcr);
 
 	cur_cpu_spec->mmu_features |= MMU_FTRS_HASH_BASE;
-- 
2.16.3

^ permalink raw reply related


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