* Re: [PATCH 15/20] powerpc/dma: remove the unused unmap_page and unmap_sg methods
From: Benjamin Herrenschmidt @ 2018-08-09 0:49 UTC (permalink / raw)
To: Christoph Hellwig, Paul Mackerras, Michael Ellerman, Tony Luck,
Fenghua Yu
Cc: Konrad Rzeszutek Wilk, Robin Murphy, linuxppc-dev, iommu,
linux-ia64
In-Reply-To: <20180730163824.10064-16-hch@lst.de>
On Mon, 2018-07-30 at 18:38 +0200, Christoph Hellwig wrote:
> These methods are optional to start with, no need to implement no-op
> versions.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> ---
> arch/powerpc/kernel/dma.c | 16 ----------------
> 1 file changed, 16 deletions(-)
>
> diff --git a/arch/powerpc/kernel/dma.c b/arch/powerpc/kernel/dma.c
> index 511a4972560d..2cfc45acbb52 100644
> --- a/arch/powerpc/kernel/dma.c
> +++ b/arch/powerpc/kernel/dma.c
> @@ -178,12 +178,6 @@ static int dma_nommu_map_sg(struct device *dev, struct scatterlist *sgl,
> return nents;
> }
>
> -static void dma_nommu_unmap_sg(struct device *dev, struct scatterlist *sg,
> - int nents, enum dma_data_direction direction,
> - unsigned long attrs)
> -{
> -}
> -
> static u64 dma_nommu_get_required_mask(struct device *dev)
> {
> u64 end, mask;
> @@ -209,14 +203,6 @@ static inline dma_addr_t dma_nommu_map_page(struct device *dev,
> return phys_to_dma(dev, page_to_phys(page)) + offset;
> }
>
> -static inline void dma_nommu_unmap_page(struct device *dev,
> - dma_addr_t dma_address,
> - size_t size,
> - enum dma_data_direction direction,
> - unsigned long attrs)
> -{
> -}
> -
> #ifdef CONFIG_NOT_COHERENT_CACHE
> static inline void dma_nommu_sync_sg(struct device *dev,
> struct scatterlist *sgl, int nents,
> @@ -242,10 +228,8 @@ const struct dma_map_ops dma_nommu_ops = {
> .free = dma_nommu_free_coherent,
> .mmap = dma_nommu_mmap_coherent,
> .map_sg = dma_nommu_map_sg,
> - .unmap_sg = dma_nommu_unmap_sg,
> .dma_supported = dma_direct_supported,
> .map_page = dma_nommu_map_page,
> - .unmap_page = dma_nommu_unmap_page,
> .get_required_mask = dma_nommu_get_required_mask,
> #ifdef CONFIG_NOT_COHERENT_CACHE
> .sync_single_for_cpu = dma_nommu_sync_single,
^ permalink raw reply
* Re: [PATCH 14/20] powerpc/dma: replace dma_nommu_dma_supported with dma_direct_supported
From: Benjamin Herrenschmidt @ 2018-08-09 0:49 UTC (permalink / raw)
To: Christoph Hellwig, Paul Mackerras, Michael Ellerman, Tony Luck,
Fenghua Yu
Cc: Konrad Rzeszutek Wilk, Robin Murphy, linuxppc-dev, iommu,
linux-ia64
In-Reply-To: <20180730163824.10064-15-hch@lst.de>
On Mon, 2018-07-30 at 18:38 +0200, Christoph Hellwig wrote:
> The ppc32 case of dma_nommu_dma_supported already was a no-op, and the
> 64-bit case came to the same conclusion as dma_direct_supported, so
> replace it with the generic version.
It's not at all equivalent (see my review on your earlier patch) or
am I missing something ?
- ppc32 always return 1, but dma_direct_supported() will not for
devices with a <32-bit mask (and yes ppc32 isn't quite right to do
so, it should check against memory size, but in practice it worked
as the only limited devices we deal with on systems we still support
have a 31-bit limitation)
- ppc64 needs to check against the end of DRAM as some devices will
fail the check, dma_direct_supported() doesn't seem to be doing that.
Also as I mentioned, I'm not sure about the business with ZONE_DMA,
and that arbitrary 24-bit limit since our entire memory is in ZONE_DMA
but that's a different can of worms I suppose.
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> arch/powerpc/Kconfig | 1 +
> arch/powerpc/kernel/dma.c | 28 +++-------------------------
> 2 files changed, 4 insertions(+), 25 deletions(-)
>
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index f9cae7edd735..bbfa6a8df4da 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -158,6 +158,7 @@ config PPC
> select CLONE_BACKWARDS
> select DCACHE_WORD_ACCESS if PPC64 && CPU_LITTLE_ENDIAN
> select DYNAMIC_FTRACE if FUNCTION_TRACER
> + select DMA_DIRECT_OPS
> select EDAC_ATOMIC_SCRUB
> select EDAC_SUPPORT
> select GENERIC_ATOMIC64 if PPC32
> diff --git a/arch/powerpc/kernel/dma.c b/arch/powerpc/kernel/dma.c
> index 3487de83bb37..511a4972560d 100644
> --- a/arch/powerpc/kernel/dma.c
> +++ b/arch/powerpc/kernel/dma.c
> @@ -40,28 +40,6 @@ static u64 __maybe_unused get_pfn_limit(struct device *dev)
> return pfn;
> }
>
> -static int dma_nommu_dma_supported(struct device *dev, u64 mask)
> -{
> -#ifdef CONFIG_PPC64
> - u64 limit = phys_to_dma(dev, (memblock_end_of_DRAM() - 1));
> -
> - /* Limit fits in the mask, we are good */
> - if (mask >= limit)
> - return 1;
> -
> -#ifdef CONFIG_FSL_SOC
> - /* Freescale gets another chance via ZONE_DMA/ZONE_DMA32, however
> - * that will have to be refined if/when they support iommus
> - */
> - return 1;
> -#endif
> - /* Sorry ... */
> - return 0;
> -#else
> - return 1;
> -#endif
> -}
> -
> #ifndef CONFIG_NOT_COHERENT_CACHE
> void *__dma_nommu_alloc_coherent(struct device *dev, size_t size,
> dma_addr_t *dma_handle, gfp_t flag,
> @@ -126,7 +104,7 @@ static void *dma_nommu_alloc_coherent(struct device *dev, size_t size,
> /* The coherent mask may be smaller than the real mask, check if
> * we can really use the direct ops
> */
> - if (dma_nommu_dma_supported(dev, dev->coherent_dma_mask))
> + if (dma_direct_supported(dev, dev->coherent_dma_mask))
> return __dma_nommu_alloc_coherent(dev, size, dma_handle,
> flag, attrs);
>
> @@ -148,7 +126,7 @@ static void dma_nommu_free_coherent(struct device *dev, size_t size,
> struct iommu_table *iommu;
>
> /* See comments in dma_nommu_alloc_coherent() */
> - if (dma_nommu_dma_supported(dev, dev->coherent_dma_mask))
> + if (dma_direct_supported(dev, dev->coherent_dma_mask))
> return __dma_nommu_free_coherent(dev, size, vaddr, dma_handle,
> attrs);
> /* Maybe we used an iommu ... */
> @@ -265,7 +243,7 @@ const struct dma_map_ops dma_nommu_ops = {
> .mmap = dma_nommu_mmap_coherent,
> .map_sg = dma_nommu_map_sg,
> .unmap_sg = dma_nommu_unmap_sg,
> - .dma_supported = dma_nommu_dma_supported,
> + .dma_supported = dma_direct_supported,
> .map_page = dma_nommu_map_page,
> .unmap_page = dma_nommu_unmap_page,
> .get_required_mask = dma_nommu_get_required_mask,
^ permalink raw reply
* Re: [PATCH 13/20] powerpc/dma: remove get_dma_offset
From: Benjamin Herrenschmidt @ 2018-08-09 0:45 UTC (permalink / raw)
To: Christoph Hellwig, Paul Mackerras, Michael Ellerman, Tony Luck,
Fenghua Yu
Cc: Konrad Rzeszutek Wilk, Robin Murphy, linuxppc-dev, iommu,
linux-ia64
In-Reply-To: <20180730163824.10064-14-hch@lst.de>
On Mon, 2018-07-30 at 18:38 +0200, Christoph Hellwig wrote:
> Just fold the calculation into __phys_to_dma/__dma_to_phys as those are
> the only places that should know about it.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> ---
> arch/powerpc/include/asm/dma-direct.h | 8 ++++++--
> arch/powerpc/include/asm/dma-mapping.h | 16 ----------------
> 2 files changed, 6 insertions(+), 18 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/dma-direct.h b/arch/powerpc/include/asm/dma-direct.h
> index 7702875aabb7..0fba19445ae8 100644
> --- a/arch/powerpc/include/asm/dma-direct.h
> +++ b/arch/powerpc/include/asm/dma-direct.h
> @@ -19,11 +19,15 @@ static inline bool dma_capable(struct device *dev, dma_addr_t addr, size_t size)
>
> static inline dma_addr_t __phys_to_dma(struct device *dev, phys_addr_t paddr)
> {
> - return paddr + get_dma_offset(dev);
> + if (!dev)
> + return paddr + PCI_DRAM_OFFSET;
> + return paddr + dev->archdata.dma_offset;
> }
>
> static inline phys_addr_t __dma_to_phys(struct device *dev, dma_addr_t daddr)
> {
> - return daddr - get_dma_offset(dev);
> + if (!dev)
> + return daddr - PCI_DRAM_OFFSET;
> + return daddr - dev->archdata.dma_offset;
> }
> #endif /* ASM_POWERPC_DMA_DIRECT_H */
> diff --git a/arch/powerpc/include/asm/dma-mapping.h b/arch/powerpc/include/asm/dma-mapping.h
> index dacd0f93f2b2..f0bf7ac2686c 100644
> --- a/arch/powerpc/include/asm/dma-mapping.h
> +++ b/arch/powerpc/include/asm/dma-mapping.h
> @@ -80,22 +80,6 @@ static inline const struct dma_map_ops *get_arch_dma_ops(struct bus_type *bus)
> return NULL;
> }
>
> -/*
> - * get_dma_offset()
> - *
> - * Get the dma offset on configurations where the dma address can be determined
> - * from the physical address by looking at a simple offset. Direct dma and
> - * swiotlb use this function, but it is typically not used by implementations
> - * with an iommu.
> - */
> -static inline dma_addr_t get_dma_offset(struct device *dev)
> -{
> - if (dev)
> - return dev->archdata.dma_offset;
> -
> - return PCI_DRAM_OFFSET;
> -}
> -
> static inline void set_dma_offset(struct device *dev, dma_addr_t off)
> {
> if (dev)
^ permalink raw reply
* Re: [PATCH 12/20] powerpc/dma: use phys_to_dma instead of get_dma_offset
From: Benjamin Herrenschmidt @ 2018-08-09 0:43 UTC (permalink / raw)
To: Christoph Hellwig, Paul Mackerras, Michael Ellerman, Tony Luck,
Fenghua Yu
Cc: Konrad Rzeszutek Wilk, Robin Murphy, linuxppc-dev, iommu,
linux-ia64
In-Reply-To: <20180730163824.10064-13-hch@lst.de>
On Mon, 2018-07-30 at 18:38 +0200, Christoph Hellwig wrote:
> Use the standard portable helper instead of the powerpc specific one,
> which is about to go away.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> ---
> arch/powerpc/kernel/dma-swiotlb.c | 5 ++---
> arch/powerpc/kernel/dma.c | 12 ++++++------
> 2 files changed, 8 insertions(+), 9 deletions(-)
>
> diff --git a/arch/powerpc/kernel/dma-swiotlb.c b/arch/powerpc/kernel/dma-swiotlb.c
> index 88f3963ca30f..f6e0701c5303 100644
> --- a/arch/powerpc/kernel/dma-swiotlb.c
> +++ b/arch/powerpc/kernel/dma-swiotlb.c
> @@ -11,7 +11,7 @@
> *
> */
>
> -#include <linux/dma-mapping.h>
> +#include <linux/dma-direct.h>
> #include <linux/memblock.h>
> #include <linux/pfn.h>
> #include <linux/of_platform.h>
> @@ -31,9 +31,8 @@ static u64 swiotlb_powerpc_get_required(struct device *dev)
> end = memblock_end_of_DRAM();
> if (max_direct_dma_addr && end > max_direct_dma_addr)
> end = max_direct_dma_addr;
> - end += get_dma_offset(dev);
>
> - mask = 1ULL << (fls64(end) - 1);
> + mask = 1ULL << (fls64(phys_to_dma(dev, end)) - 1);
> mask += mask - 1;
>
> return mask;
> diff --git a/arch/powerpc/kernel/dma.c b/arch/powerpc/kernel/dma.c
> index eceaa92e6986..3487de83bb37 100644
> --- a/arch/powerpc/kernel/dma.c
> +++ b/arch/powerpc/kernel/dma.c
> @@ -6,7 +6,7 @@
> */
>
> #include <linux/device.h>
> -#include <linux/dma-mapping.h>
> +#include <linux/dma-direct.h>
> #include <linux/dma-debug.h>
> #include <linux/gfp.h>
> #include <linux/memblock.h>
> @@ -43,7 +43,7 @@ static u64 __maybe_unused get_pfn_limit(struct device *dev)
> static int dma_nommu_dma_supported(struct device *dev, u64 mask)
> {
> #ifdef CONFIG_PPC64
> - u64 limit = get_dma_offset(dev) + (memblock_end_of_DRAM() - 1);
> + u64 limit = phys_to_dma(dev, (memblock_end_of_DRAM() - 1));
>
> /* Limit fits in the mask, we are good */
> if (mask >= limit)
> @@ -104,7 +104,7 @@ void *__dma_nommu_alloc_coherent(struct device *dev, size_t size,
> return NULL;
> ret = page_address(page);
> memset(ret, 0, size);
> - *dma_handle = __pa(ret) + get_dma_offset(dev);
> + *dma_handle = phys_to_dma(dev,__pa(ret));
>
> return ret;
> }
> @@ -188,7 +188,7 @@ static int dma_nommu_map_sg(struct device *dev, struct scatterlist *sgl,
> int i;
>
> for_each_sg(sgl, sg, nents, i) {
> - sg->dma_address = sg_phys(sg) + get_dma_offset(dev);
> + sg->dma_address = phys_to_dma(dev, sg_phys(sg));
> sg->dma_length = sg->length;
>
> if (attrs & DMA_ATTR_SKIP_CPU_SYNC)
> @@ -210,7 +210,7 @@ static u64 dma_nommu_get_required_mask(struct device *dev)
> {
> u64 end, mask;
>
> - end = memblock_end_of_DRAM() + get_dma_offset(dev);
> + end = phys_to_dma(dev, memblock_end_of_DRAM());
>
> mask = 1ULL << (fls64(end) - 1);
> mask += mask - 1;
> @@ -228,7 +228,7 @@ static inline dma_addr_t dma_nommu_map_page(struct device *dev,
> if (!(attrs & DMA_ATTR_SKIP_CPU_SYNC))
> __dma_sync_page(page, offset, size, dir);
>
> - return page_to_phys(page) + offset + get_dma_offset(dev);
> + return phys_to_dma(dev, page_to_phys(page)) + offset;
> }
>
> static inline void dma_nommu_unmap_page(struct device *dev,
^ permalink raw reply
* Re: [PATCH 11/20] powerpc/dma: split the two __dma_alloc_coherent implementations
From: Benjamin Herrenschmidt @ 2018-08-09 0:40 UTC (permalink / raw)
To: Christoph Hellwig, Paul Mackerras, Michael Ellerman, Tony Luck,
Fenghua Yu
Cc: Konrad Rzeszutek Wilk, Robin Murphy, linuxppc-dev, iommu,
linux-ia64
In-Reply-To: <20180730163824.10064-12-hch@lst.de>
On Mon, 2018-07-30 at 18:38 +0200, Christoph Hellwig wrote:
> The implemementation for the CONFIG_NOT_COHERENT_CACHE case doesn't share
> any code with the one for systems with coherent caches. Split it off
> and merge it with the helpers in dma-noncoherent.c that have no other
> callers.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> ---
> arch/powerpc/include/asm/dma-mapping.h | 5 -----
> arch/powerpc/kernel/dma.c | 14 ++------------
> arch/powerpc/mm/dma-noncoherent.c | 15 +++++++--------
> arch/powerpc/platforms/44x/warp.c | 2 +-
> 4 files changed, 10 insertions(+), 26 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/dma-mapping.h b/arch/powerpc/include/asm/dma-mapping.h
> index f2a4a7142b1e..dacd0f93f2b2 100644
> --- a/arch/powerpc/include/asm/dma-mapping.h
> +++ b/arch/powerpc/include/asm/dma-mapping.h
> @@ -39,9 +39,6 @@ extern int dma_nommu_mmap_coherent(struct device *dev,
> * to ensure it is consistent.
> */
> struct device;
> -extern void *__dma_alloc_coherent(struct device *dev, size_t size,
> - dma_addr_t *handle, gfp_t gfp);
> -extern void __dma_free_coherent(size_t size, void *vaddr);
> extern void __dma_sync(void *vaddr, size_t size, int direction);
> extern void __dma_sync_page(struct page *page, unsigned long offset,
> size_t size, int direction);
> @@ -52,8 +49,6 @@ extern unsigned long __dma_get_coherent_pfn(unsigned long cpu_addr);
> * Cache coherent cores.
> */
>
> -#define __dma_alloc_coherent(dev, gfp, size, handle) NULL
> -#define __dma_free_coherent(size, addr) ((void)0)
> #define __dma_sync(addr, size, rw) ((void)0)
> #define __dma_sync_page(pg, off, sz, rw) ((void)0)
>
> diff --git a/arch/powerpc/kernel/dma.c b/arch/powerpc/kernel/dma.c
> index 3939589aab04..eceaa92e6986 100644
> --- a/arch/powerpc/kernel/dma.c
> +++ b/arch/powerpc/kernel/dma.c
> @@ -62,18 +62,12 @@ static int dma_nommu_dma_supported(struct device *dev, u64 mask)
> #endif
> }
>
> +#ifndef CONFIG_NOT_COHERENT_CACHE
> void *__dma_nommu_alloc_coherent(struct device *dev, size_t size,
> dma_addr_t *dma_handle, gfp_t flag,
> unsigned long attrs)
> {
> void *ret;
> -#ifdef CONFIG_NOT_COHERENT_CACHE
> - ret = __dma_alloc_coherent(dev, size, dma_handle, flag);
> - if (ret == NULL)
> - return NULL;
> - *dma_handle += get_dma_offset(dev);
> - return ret;
> -#else
> struct page *page;
> int node = dev_to_node(dev);
> #ifdef CONFIG_FSL_SOC
> @@ -113,19 +107,15 @@ void *__dma_nommu_alloc_coherent(struct device *dev, size_t size,
> *dma_handle = __pa(ret) + get_dma_offset(dev);
>
> return ret;
> -#endif
> }
>
> void __dma_nommu_free_coherent(struct device *dev, size_t size,
> void *vaddr, dma_addr_t dma_handle,
> unsigned long attrs)
> {
> -#ifdef CONFIG_NOT_COHERENT_CACHE
> - __dma_free_coherent(size, vaddr);
> -#else
> free_pages((unsigned long)vaddr, get_order(size));
> -#endif
> }
> +#endif /* !CONFIG_NOT_COHERENT_CACHE */
>
> static void *dma_nommu_alloc_coherent(struct device *dev, size_t size,
> dma_addr_t *dma_handle, gfp_t flag,
> diff --git a/arch/powerpc/mm/dma-noncoherent.c b/arch/powerpc/mm/dma-noncoherent.c
> index d1c16456abac..cfc48a253707 100644
> --- a/arch/powerpc/mm/dma-noncoherent.c
> +++ b/arch/powerpc/mm/dma-noncoherent.c
> @@ -29,7 +29,7 @@
> #include <linux/string.h>
> #include <linux/types.h>
> #include <linux/highmem.h>
> -#include <linux/dma-mapping.h>
> +#include <linux/dma-direct.h>
> #include <linux/export.h>
>
> #include <asm/tlbflush.h>
> @@ -151,8 +151,8 @@ static struct ppc_vm_region *ppc_vm_region_find(struct ppc_vm_region *head, unsi
> * Allocate DMA-coherent memory space and return both the kernel remapped
> * virtual and bus address for that space.
> */
> -void *
> -__dma_alloc_coherent(struct device *dev, size_t size, dma_addr_t *handle, gfp_t gfp)
> +void *__dma_nommu_alloc_coherent(struct device *dev, size_t size,
> + dma_addr_t *dma_handle, gfp_t gfp, unsigned long attrs)
> {
> struct page *page;
> struct ppc_vm_region *c;
> @@ -223,7 +223,7 @@ __dma_alloc_coherent(struct device *dev, size_t size, dma_addr_t *handle, gfp_t
> /*
> * Set the "dma handle"
> */
> - *handle = page_to_phys(page);
> + *dma_handle = phys_to_dma(dev, page_to_phys(page));
>
> do {
> SetPageReserved(page);
> @@ -249,12 +249,12 @@ __dma_alloc_coherent(struct device *dev, size_t size, dma_addr_t *handle, gfp_t
> no_page:
> return NULL;
> }
> -EXPORT_SYMBOL(__dma_alloc_coherent);
>
> /*
> * free a page as defined by the above mapping.
> */
> -void __dma_free_coherent(size_t size, void *vaddr)
> +void __dma_nommu_free_coherent(struct device *dev, size_t size, void *vaddr,
> + dma_addr_t dma_handle, unsigned long attrs)
> {
> struct ppc_vm_region *c;
> unsigned long flags, addr;
> @@ -309,7 +309,6 @@ void __dma_free_coherent(size_t size, void *vaddr)
> __func__, vaddr);
> dump_stack();
> }
> -EXPORT_SYMBOL(__dma_free_coherent);
>
> /*
> * make an area consistent.
> @@ -397,7 +396,7 @@ EXPORT_SYMBOL(__dma_sync_page);
>
> /*
> * Return the PFN for a given cpu virtual address returned by
> - * __dma_alloc_coherent. This is used by dma_mmap_coherent()
> + * __dma_nommu_alloc_coherent. This is used by dma_mmap_coherent()
> */
> unsigned long __dma_get_coherent_pfn(unsigned long cpu_addr)
> {
> diff --git a/arch/powerpc/platforms/44x/warp.c b/arch/powerpc/platforms/44x/warp.c
> index a886c2c22097..7e4f8ca19ce8 100644
> --- a/arch/powerpc/platforms/44x/warp.c
> +++ b/arch/powerpc/platforms/44x/warp.c
> @@ -47,7 +47,7 @@ static int __init warp_probe(void)
> if (!of_machine_is_compatible("pika,warp"))
> return 0;
>
> - /* For __dma_alloc_coherent */
> + /* For __dma_nommu_alloc_coherent */
> ISA_DMA_THRESHOLD = ~0L;
>
> return 1;
^ permalink raw reply
* Re: [PATCH 10/20] powerpc/dma-noncoherent: don't disable irqs over kmap_atomic
From: Benjamin Herrenschmidt @ 2018-08-09 0:27 UTC (permalink / raw)
To: Christoph Hellwig, Paul Mackerras, Michael Ellerman, Tony Luck,
Fenghua Yu
Cc: Konrad Rzeszutek Wilk, Robin Murphy, linuxppc-dev, iommu,
linux-ia64
In-Reply-To: <20180730163824.10064-11-hch@lst.de>
On Mon, 2018-07-30 at 18:38 +0200, Christoph Hellwig wrote:
> The requirement to disable local irqs over kmap_atomic is long gone,
> so remove those calls.
Really ? I'm trying to verify that and getting lost in a mess of macros
from hell in the per-cpu stuff but if you look at our implementation
of kmap_atomic_prot(), all it does is a preempt_disable(), and then
it uses kmap_atomic_idx_push():
int idx = __this_cpu_inc_return(__kmap_atomic_idx) - 1;
Note the use of __this_cpu_inc_return(), not this_cpu_inc_return(),
ie this is the non-interrupt safe version...
Ben.
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> arch/powerpc/mm/dma-noncoherent.c | 6 +-----
> 1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/arch/powerpc/mm/dma-noncoherent.c b/arch/powerpc/mm/dma-noncoherent.c
> index 382528475433..d1c16456abac 100644
> --- a/arch/powerpc/mm/dma-noncoherent.c
> +++ b/arch/powerpc/mm/dma-noncoherent.c
> @@ -357,12 +357,10 @@ static inline void __dma_sync_page_highmem(struct page *page,
> {
> size_t seg_size = min((size_t)(PAGE_SIZE - offset), size);
> size_t cur_size = seg_size;
> - unsigned long flags, start, seg_offset = offset;
> + unsigned long start, seg_offset = offset;
> int nr_segs = 1 + ((size - seg_size) + PAGE_SIZE - 1)/PAGE_SIZE;
> int seg_nr = 0;
>
> - local_irq_save(flags);
> -
> do {
> start = (unsigned long)kmap_atomic(page + seg_nr) + seg_offset;
>
> @@ -378,8 +376,6 @@ static inline void __dma_sync_page_highmem(struct page *page,
> cur_size += seg_size;
> seg_offset = 0;
> } while (seg_nr < nr_segs);
> -
> - local_irq_restore(flags);
> }
> #endif /* CONFIG_HIGHMEM */
>
^ permalink raw reply
* Re: [PATCH 09/20] powerpc/dma: remove the unused ISA_DMA_THRESHOLD export
From: Benjamin Herrenschmidt @ 2018-08-09 0:14 UTC (permalink / raw)
To: Christoph Hellwig, Paul Mackerras, Michael Ellerman, Tony Luck,
Fenghua Yu
Cc: Konrad Rzeszutek Wilk, Robin Murphy, linuxppc-dev, iommu,
linux-ia64
In-Reply-To: <20180730163824.10064-10-hch@lst.de>
On Mon, 2018-07-30 at 18:38 +0200, Christoph Hellwig wrote:
> Signed-off-by: Christoph Hellwig <hch@lst.de>
Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> ---
> arch/powerpc/kernel/setup_32.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/arch/powerpc/kernel/setup_32.c b/arch/powerpc/kernel/setup_32.c
> index 74457485574b..3c2d093f74c7 100644
> --- a/arch/powerpc/kernel/setup_32.c
> +++ b/arch/powerpc/kernel/setup_32.c
> @@ -55,7 +55,6 @@ unsigned long ISA_DMA_THRESHOLD;
> unsigned int DMA_MODE_READ;
> unsigned int DMA_MODE_WRITE;
>
> -EXPORT_SYMBOL(ISA_DMA_THRESHOLD);
> EXPORT_SYMBOL(DMA_MODE_READ);
> EXPORT_SYMBOL(DMA_MODE_WRITE);
> fooBenjam
^ permalink raw reply
* Re: [PATCH 08/20] powerpc/dma: remove the unused dma_nommu_ops export
From: Benjamin Herrenschmidt @ 2018-08-09 0:01 UTC (permalink / raw)
To: Christoph Hellwig, Paul Mackerras, Michael Ellerman, Tony Luck,
Fenghua Yu
Cc: linuxppc-dev, iommu, linux-ia64, Robin Murphy,
Konrad Rzeszutek Wilk
In-Reply-To: <20180731121625.GA29793@lst.de>
On Tue, 2018-07-31 at 14:16 +0200, Christoph Hellwig wrote:
> It turns out cxl actually uses it. So for now skip this patch,
> although random code in drivers messing with dma ops will need to
> be sorted out sooner or later.
CXL devices are "special", they bypass the classic iommu in favor of
allowing the device to operate using the main processor page tables
using an MMU context (so basically the device can use userspace
addresses directly), akin to ATS.
I think the code currently uses the nommu ops as a way to do a simple
kernel mapping for kernel drivers using CXL (not userspace stuff)
though.
Ben.
^ permalink raw reply
* Re: [PATCH 07/20] powerpc/dma: remove the unused ARCH_HAS_DMA_MMAP_COHERENT define
From: Benjamin Herrenschmidt @ 2018-08-08 23:56 UTC (permalink / raw)
To: Christoph Hellwig, Paul Mackerras, Michael Ellerman, Tony Luck,
Fenghua Yu
Cc: Konrad Rzeszutek Wilk, Robin Murphy, linuxppc-dev, iommu,
linux-ia64
In-Reply-To: <20180730163824.10064-8-hch@lst.de>
On Mon, 2018-07-30 at 18:38 +0200, Christoph Hellwig wrote:
> Signed-off-by: Christoph Hellwig <hch@lst.de>
Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> ---
> arch/powerpc/include/asm/dma-mapping.h | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/dma-mapping.h b/arch/powerpc/include/asm/dma-mapping.h
> index 8fa394520af6..f2a4a7142b1e 100644
> --- a/arch/powerpc/include/asm/dma-mapping.h
> +++ b/arch/powerpc/include/asm/dma-mapping.h
> @@ -112,7 +112,5 @@ extern int dma_set_mask(struct device *dev, u64 dma_mask);
>
> extern u64 __dma_get_required_mask(struct device *dev);
>
> -#define ARCH_HAS_DMA_MMAP_COHERENT
> -
> #endif /* __KERNEL__ */
> #endif /* _ASM_DMA_MAPPING_H */
^ permalink raw reply
* Re: [PATCH 02/20] kernel/dma/direct: refine dma_direct_alloc zone selection
From: Benjamin Herrenschmidt @ 2018-08-08 23:54 UTC (permalink / raw)
To: Christoph Hellwig, Paul Mackerras, Michael Ellerman, Tony Luck,
Fenghua Yu
Cc: Konrad Rzeszutek Wilk, Robin Murphy, linuxppc-dev, iommu,
linux-ia64
In-Reply-To: <20180730163824.10064-3-hch@lst.de>
On Mon, 2018-07-30 at 18:38 +0200, Christoph Hellwig wrote:
> We need to take the DMA offset and encryption bit into account when selecting
> a zone. Add a helper that takes those into account and use it.
That whole "encryption" stuff seems to be completely specific to the
way x86 does memory encryption, or am I mistaken ? It's not clear to me
what that does in practice and how it relates to DMA mappings.
I'm also not sure about that whole business with ZONE_DMA and
ARCH_ZONE_DMA_BITS...
On ppc64, unless you enable swiotlb (which we only do currently on
some embedded platforms), you have all of memory in ZONE_DMA.
[ 0.000000] Zone ranges:
[ 0.000000] DMA [mem 0x0000000000000000-0x0000001fffffffff]
[ 0.000000] DMA32 empty
[ 0.000000] Normal empty
[ 0.000000] Device empty
I'm not sure how this will work with that dma direct code.
I also see a number of tests against a 64-bit mask rather than the
top of memory...
Ben.
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> kernel/dma/direct.c | 16 ++++++++++++----
> 1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
> index d32d4f0d2c0c..c2c1df8827f2 100644
> --- a/kernel/dma/direct.c
> +++ b/kernel/dma/direct.c
> @@ -58,6 +58,14 @@ static bool dma_coherent_ok(struct device *dev, phys_addr_t phys, size_t size)
> return addr + size - 1 <= dev->coherent_dma_mask;
> }
>
> +static bool dma_coherent_below(struct device *dev, u64 mask)
> +{
> + dma_addr_t addr = force_dma_unencrypted() ?
> + __phys_to_dma(dev, mask) : phys_to_dma(dev, mask);
> +
> + return dev->coherent_dma_mask <= addr;
> +}
> +
> void *dma_direct_alloc(struct device *dev, size_t size, dma_addr_t *dma_handle,
> gfp_t gfp, unsigned long attrs)
> {
> @@ -70,9 +78,9 @@ void *dma_direct_alloc(struct device *dev, size_t size, dma_addr_t *dma_handle,
> gfp &= ~__GFP_ZERO;
>
> /* GFP_DMA32 and GFP_DMA are no ops without the corresponding zones: */
> - if (dev->coherent_dma_mask <= DMA_BIT_MASK(ARCH_ZONE_DMA_BITS))
> + if (dma_coherent_below(dev, DMA_BIT_MASK(ARCH_ZONE_DMA_BITS)))
> gfp |= GFP_DMA;
> - if (dev->coherent_dma_mask <= DMA_BIT_MASK(32) && !(gfp & GFP_DMA))
> + if (dma_coherent_below(dev, DMA_BIT_MASK(32) && !(gfp & GFP_DMA)))
> gfp |= GFP_DMA32;
>
> again:
> @@ -92,14 +100,14 @@ void *dma_direct_alloc(struct device *dev, size_t size, dma_addr_t *dma_handle,
> page = NULL;
>
> if (IS_ENABLED(CONFIG_ZONE_DMA32) &&
> - dev->coherent_dma_mask < DMA_BIT_MASK(64) &&
> + dma_coherent_below(dev, DMA_BIT_MASK(64)) &&
> !(gfp & (GFP_DMA32 | GFP_DMA))) {
> gfp |= GFP_DMA32;
> goto again;
> }
>
> if (IS_ENABLED(CONFIG_ZONE_DMA) &&
> - dev->coherent_dma_mask < DMA_BIT_MASK(32) &&
> + dma_coherent_below(dev, DMA_BIT_MASK(32)) &&
> !(gfp & GFP_DMA)) {
> gfp = (gfp & ~GFP_DMA32) | GFP_DMA;
> goto again;
^ permalink raw reply
* Re: [PATCH 01/20] kernel/dma/direct: take DMA offset into account in dma_direct_supported
From: Benjamin Herrenschmidt @ 2018-08-08 23:44 UTC (permalink / raw)
To: Christoph Hellwig, Paul Mackerras, Michael Ellerman, Tony Luck,
Fenghua Yu
Cc: Konrad Rzeszutek Wilk, Robin Murphy, linuxppc-dev, iommu,
linux-ia64
In-Reply-To: <20180730163824.10064-2-hch@lst.de>
On Mon, 2018-07-30 at 18:38 +0200, Christoph Hellwig wrote:
> When a device has a DMA offset the dma capable result will change due
> to the difference between the physical and DMA address. Take that into
> account.
The patch in itself makes sense.
However, there are a number of things in that dma_direct.c file that I
don't quite get:
- looking more generally at what that function does, I worry about the
switch of ppc32 to this later on:
We do have the occasional device with things like 31-bit DMA
limitation. We know they happens to work because those systems
can't have enough memory to be a problem. This is why our current
DMA direct ops in powerpc just unconditionally return true on ppc32.
The test against a full 32-bit mask here will break them I think.
Thing is, I'm not sure I still have access to one of these things
to test, I'll have to dig (from memory things like b43 wifi).
Also those platforms don't have an iommu.
- What is this trying to achieve ?
/*
* Various PCI/PCIe bridges have broken support for > 32bit DMA even
* if the device itself might support it.
*/
if (dev->dma_32bit_limit && mask > phys_to_dma(dev, DMA_BIT_MASK(32)))
return 0;
IE, if the device has a 32-bit limit, we fail an attempt at checking
if a >32-bit mask works ? That doesn't quite seem to be the right thing
to do... Shouldn't this be in dma_set_mask() and just clamp the mask down ?
IE, dma_set_mask() is what a driver uses to establish the device capability,
so it makes sense tot have dma_32bit_limit just reduce that capability, not
fail because the device can do more than what the bridge can....
Sorry if I'm a bit confused here.
- How is that file supposed to work on 64-bit platforms ? From what I can
tell, dma_supported() will unconditionally return true if the mask is
32-bit or larger (appart from the above issue). This doesn't look right,
the mask needs to be compared to the max memory address. There are a bunch
of devices out there with masks anywhere bettween 40 and 64 bits, and
some of these will not work "out of the box" if the offseted top
of memory is beyond the mask limit. Or am I missing something ?
Cheers,
Ben.
> Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> ---
> kernel/dma/direct.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
> index 8be8106270c2..d32d4f0d2c0c 100644
> --- a/kernel/dma/direct.c
> +++ b/kernel/dma/direct.c
> @@ -167,7 +167,7 @@ int dma_direct_map_sg(struct device *dev, struct scatterlist *sgl, int nents,
> int dma_direct_supported(struct device *dev, u64 mask)
> {
> #ifdef CONFIG_ZONE_DMA
> - if (mask < DMA_BIT_MASK(ARCH_ZONE_DMA_BITS))
> + if (mask < phys_to_dma(dev, DMA_BIT_MASK(ARCH_ZONE_DMA_BITS)))
> return 0;
> #else
> /*
> @@ -176,14 +176,14 @@ int dma_direct_supported(struct device *dev, u64 mask)
> * memory, or by providing a ZONE_DMA32. If neither is the case, the
> * architecture needs to use an IOMMU instead of the direct mapping.
> */
> - if (mask < DMA_BIT_MASK(32))
> + if (mask < phys_to_dma(dev, DMA_BIT_MASK(32)))
> return 0;
> #endif
> /*
> * Various PCI/PCIe bridges have broken support for > 32bit DMA even
> * if the device itself might support it.
> */
> - if (dev->dma_32bit_limit && mask > DMA_BIT_MASK(32))
> + if (dev->dma_32bit_limit && mask > phys_to_dma(dev, DMA_BIT_MASK(32)))
> return 0;
> return 1;
> }
^ permalink raw reply
* Re: [v2, 1/6] powerpc/pm: Fix suspend=n in menuconfig for e500mc platforms.
From: Scott Wood @ 2018-08-08 23:01 UTC (permalink / raw)
To: Ran Wang
Cc: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
Rob Herring, Mark Rutland, Kumar Gala, Li Yang, devicetree,
Zhao Chenhui, linux-kernel, linuxppc-dev, linux-arm-kernel
In-Reply-To: <20180411063551.30368-1-ran.wang_1@nxp.com>
On Wed, Apr 11, 2018 at 02:35:46PM +0800, Ran Wang wrote:
> Also, unselect FSL_PMC which is for older platfroms instead.
>
> Signed-off-by: Ran Wang <ran.wang_1@nxp.com>
> ---
> Changes in v2:
> - no change
>
> arch/powerpc/Kconfig | 4 +---
> 1 files changed, 1 insertions(+), 3 deletions(-)
It's not "fixing" anything, but rather enabling something that was
previously unsupported -- and that should happen after the code that
adds support.
-Scott
^ permalink raw reply
* Re: [2/3] powerpc/dts/fsl: t4240rdb: use the Cortina PHY driver compatible
From: Scott Wood @ 2018-08-08 22:47 UTC (permalink / raw)
To: Camelia Groza
Cc: robh+dt, mark.rutland, benh, devicetree, linux-kernel, paulus,
linuxppc-dev
In-Reply-To: <6fa5aa04d257525c5703b7c8ca833ec0ae0047c7.1531903211.git.camelia.groza@nxp.com>
On Wed, Jul 18, 2018 at 02:46:50PM +0300, Camelia Groza wrote:
> The Cortina PHY requires the use of the dedicated Cortina PHY driver
> instead of the generic one.
>
> Signed-off-by: Camelia Groza <camelia.groza@nxp.com>
> ---
> arch/powerpc/boot/dts/fsl/t4240rdb.dts | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
Applied with the changelog updated to talk about hardware compatibility
("The Cortina PHY is not compatible with IEEE 802.3 clause 45") rather
than driver choice.
-Scott
^ permalink raw reply
* Re: [PATCH 1/2] powerpc/64s: move machine check SLB flushing to mm/slb.c
From: Nicholas Piggin @ 2018-08-08 22:45 UTC (permalink / raw)
To: Michal Suchánek
Cc: linuxppc-dev, Gautham R . Shenoy, Mahesh Jagannath Salgaonkar,
kvm-ppc, Aneesh Kumar K.V, Akshay Adiga
In-Reply-To: <20180808222252.5be0feac@kitsune.suse.cz>
On Wed, 8 Aug 2018 22:22:52 +0200
Michal Such=C3=A1nek <msuchanek@suse.de> wrote:
> On Fri, 3 Aug 2018 14:13:49 +1000
> Nicholas Piggin <npiggin@gmail.com> wrote:
>=20
> > The machine check code that flushes and restores bolted segments in
> > real mode belongs in mm/slb.c. This will be used by pseries machine
> > check and idle code.
> >=20
> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> > ---
> > arch/powerpc/include/asm/book3s/64/mmu-hash.h | 3 ++
> > arch/powerpc/kernel/mce_power.c | 21 ++--------
> > arch/powerpc/mm/slb.c | 38
> > +++++++++++++++++++ 3 files changed, 44 insertions(+), 18 deletions(-)
> >=20
> > diff --git a/arch/powerpc/include/asm/book3s/64/mmu-hash.h
> > b/arch/powerpc/include/asm/book3s/64/mmu-hash.h index
> > 2f74bdc805e0..d4e398185b3a 100644 ---
> > a/arch/powerpc/include/asm/book3s/64/mmu-hash.h +++
> > b/arch/powerpc/include/asm/book3s/64/mmu-hash.h @@ -497,6 +497,9 @@
> > extern void hpte_init_native(void);=20
> > extern void slb_initialize(void);
> > extern void slb_flush_and_rebolt(void);
> > +extern void slb_flush_all_realmode(void);
> > +extern void __slb_restore_bolted_realmode(void);
> > +extern void slb_restore_bolted_realmode(void);
> > =20
> > extern void slb_vmalloc_update(void);
> > extern void slb_set_size(u16 size);
> > diff --git a/arch/powerpc/kernel/mce_power.c
> > b/arch/powerpc/kernel/mce_power.c index d6756af6ec78..50f7b9817246
> > 100644 --- a/arch/powerpc/kernel/mce_power.c
> > +++ b/arch/powerpc/kernel/mce_power.c
> > @@ -62,11 +62,8 @@ static unsigned long addr_to_pfn(struct pt_regs
> > *regs, unsigned long addr) #ifdef CONFIG_PPC_BOOK3S_64
> > static void flush_and_reload_slb(void)
> > {
> > - struct slb_shadow *slb;
> > - unsigned long i, n;
> > -
> > /* Invalidate all SLBs */
> > - asm volatile("slbmte %0,%0; slbia" : : "r" (0));
> > + slb_flush_all_realmode();
> > =20
> > #ifdef CONFIG_KVM_BOOK3S_HANDLER
> > /*
> > @@ -76,22 +73,10 @@ static void flush_and_reload_slb(void)
> > if (get_paca()->kvm_hstate.in_guest)
> > return;
> > #endif
> > -
> > - /* For host kernel, reload the SLBs from shadow SLB buffer.
> > */
> > - slb =3D get_slb_shadow();
> > - if (!slb)
> > + if (early_radix_enabled())
> > return; =20
>=20
> And we lose the check that the shadow slb exists. Is !slb equivalent to
> early_radix_enabled()=20
Yeah pretty close to.
>=20
> > =20
> > - n =3D min_t(u32, be32_to_cpu(slb->persistent), SLB_MIN_SIZE);
> > -
> > - /* Load up the SLB entries from shadow SLB */
> > - for (i =3D 0; i < n; i++) {
> > - unsigned long rb =3D
> > be64_to_cpu(slb->save_area[i].esid);
> > - unsigned long rs =3D
> > be64_to_cpu(slb->save_area[i].vsid); -
> > - rb =3D (rb & ~0xFFFul) | i;
> > - asm volatile("slbmte %0,%1" : : "r" (rs), "r" (rb));
> > - }
> > + slb_restore_bolted_realmode();
> > }
> > #endif
> > =20
> > diff --git a/arch/powerpc/mm/slb.c b/arch/powerpc/mm/slb.c
> > index cb796724a6fc..136db8652577 100644
> > --- a/arch/powerpc/mm/slb.c
> > +++ b/arch/powerpc/mm/slb.c
> > @@ -90,6 +90,44 @@ static inline void create_shadowed_slbe(unsigned
> > long ea, int ssize, : "memory" );
> > }
> > =20
> > +/*
> > + * Insert bolted entries into SLB (which may not be empty).
> > + */
> > +void __slb_restore_bolted_realmode(void)
> > +{
> > + struct slb_shadow *p =3D get_slb_shadow();
> > + enum slb_index index; =20
>=20
> or can we get here at some point when shadow slb is not populated?
We shouldn't because we won't turn the MMU on so we shouldn't get SLB
MCEs... But I don't think that's guaranteed anywhere, so yeah wouldn't
hurt to add that check back in.
I'll send out another revision.
Thanks,
Nick
^ permalink raw reply
* Re: [RFC 0/4] Virtio uses DMA API for all devices
From: Benjamin Herrenschmidt @ 2018-08-08 22:13 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Christoph Hellwig, Will Deacon, Anshuman Khandual, virtualization,
linux-kernel, linuxppc-dev, aik, robh, joe, elfring, david,
jasowang, mpe, linuxram, haren, paulus, srikar, robin.murphy,
jean-philippe.brucker, marc.zyngier
In-Reply-To: <20180808232210-mutt-send-email-mst@kernel.org>
On Wed, 2018-08-08 at 23:31 +0300, Michael S. Tsirkin wrote:
> On Wed, Aug 08, 2018 at 11:18:13PM +1000, Benjamin Herrenschmidt wrote:
> > Sure, but all of this is just the configuration of the iommu. But I
> > think we agree here, and your point remains valid, indeed my proposed
> > hack:
> >
> > > if ((flags & VIRTIO_F_IOMMU_PLATFORM) || arch_virtio_wants_dma_ops())
> >
> > Will only work if the IOMMU and non-IOMMU path are completely equivalent.
> >
> > We can provide that guarantee for our secure VM case, but not generally so if
> > we were to go down the route of a quirk in virtio, it might be better to
> > make it painfully obvious that it's specific to that one case with a different
> > kind of turd:
> >
> > - if (xen_domain())
> > + if (xen_domain() || pseries_secure_vm())
> > return true;
>
> I don't think it's pseries specific actually. E.g. I suspect AMD SEV
> might benefit from the same kind of hack.
As long as they can provide the same guarantee that the DMA ops are
completely equivalent between virtio and other PCI devices, at least on
the same bus, ie, we don't have to go hack special DMA ops.
I think the latter is really what Christoph wants to avoid for good
reasons.
> > So to summarize, and make sure I'm not missing something, the two approaches
> > at hand are either:
> >
> > 1- The above, which is a one liner and contained in the guest, so that's nice, but
> > also means another turd in virtio which isn't ...
> >
> > 2- We force pseries to always set VIRTIO_F_IOMMU_PLATFORM, but with the current
> > architecture on our side that will force virtio to always go through an emulated
> > iommu, as pseries doesn't have the concept of a real bypass window, and thus will
> > impact performance for both secure and non-secure VMs.
> >
> > 3- Invent a property that can be put in selected PCI device tree nodes that
> > indicates that for that device specifically, the iommu can be bypassed, along with
> > a hypercall to turn that bypass on/off. Virtio would then use VIRTIO_F_IOMMU_PLATFORM
> > but its DT nodes would also have that property and Linux would notice it and turn
> > bypass on.
>
> For completeness, virtio could also have its own bounce buffer
> outside of DMA API one. I don't see lots of benefits to this
> though.
Not fan of that either...
> > The resulting properties of those options are:
> >
> > 1- Is what I want because it's the simplest, provides the best performance now,
> > and works without code changes to qemu or non-secure Linux. However it does
> > add a tiny turd to virtio which is annoying.
> >
> > 2- This works but it puts the iommu in the way always, thus reducing virtio performance
> > accross the board for pseries unless we only do that for secure VMs but that is
> > difficult (as discussed earlier).
> >
> > 3- This would recover the performance lost in -2-, however it requires qemu *and*
> > guest changes. Specifically, existing guests (RHEL 7 etc...) would get the
> > performance hit of -2- unless modified to call that 'enable bypass' call, which
> > isn't great.
> >
> > So imho we have to chose one of 3 not-great solutions here... Unless I missed
> > something in your ideas of course.
> >
^ permalink raw reply
* Re: [PATCH] powerpc: fix size calculation using resource_size()
From: Tyrel Datwyler @ 2018-08-08 21:43 UTC (permalink / raw)
To: Dan Carpenter, Benjamin Herrenschmidt, Jia Hongtao
Cc: Paul Mackerras, Michael Ellerman, Rob Herring, linuxppc-dev,
kernel-janitors
In-Reply-To: <20180808115724.gh5mbbi6twzqbqao@kili.mountain>
On 08/08/2018 04:57 AM, Dan Carpenter wrote:
> The problem is the the calculation should be "end - start + 1" but the
> plus one is missing in this calculation.
>
> Fixes: 8626816e905e ("powerpc: add support for MPIC message register API")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> Static analysis. Not tested.
Looks sane to me.
Reviewed-by: Tyrel Datwyler <tyreld@linux.vnet.ibm.com>
>
> diff --git a/arch/powerpc/sysdev/mpic_msgr.c b/arch/powerpc/sysdev/mpic_msgr.c
> index eb69a5186243..280e964e1aa8 100644
> --- a/arch/powerpc/sysdev/mpic_msgr.c
> +++ b/arch/powerpc/sysdev/mpic_msgr.c
> @@ -196,7 +196,7 @@ static int mpic_msgr_probe(struct platform_device *dev)
>
> /* IO map the message register block. */
> of_address_to_resource(np, 0, &rsrc);
> - msgr_block_addr = ioremap(rsrc.start, rsrc.end - rsrc.start);
> + msgr_block_addr = ioremap(rsrc.start, resource_size(&rsrc));
> if (!msgr_block_addr) {
> dev_err(&dev->dev, "Failed to iomap MPIC message registers");
> return -EFAULT;
>
^ permalink raw reply
* Re: [RFC 0/4] Virtio uses DMA API for all devices
From: Michael S. Tsirkin @ 2018-08-08 20:31 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: Christoph Hellwig, Will Deacon, Anshuman Khandual, virtualization,
linux-kernel, linuxppc-dev, aik, robh, joe, elfring, david,
jasowang, mpe, linuxram, haren, paulus, srikar, robin.murphy,
jean-philippe.brucker, marc.zyngier
In-Reply-To: <b8b9150a747453c070ad3b0e4c92d2b1b052ad06.camel@kernel.crashing.org>
On Wed, Aug 08, 2018 at 11:18:13PM +1000, Benjamin Herrenschmidt wrote:
> Sure, but all of this is just the configuration of the iommu. But I
> think we agree here, and your point remains valid, indeed my proposed
> hack:
>
> > if ((flags & VIRTIO_F_IOMMU_PLATFORM) || arch_virtio_wants_dma_ops())
>
> Will only work if the IOMMU and non-IOMMU path are completely equivalent.
>
> We can provide that guarantee for our secure VM case, but not generally so if
> we were to go down the route of a quirk in virtio, it might be better to
> make it painfully obvious that it's specific to that one case with a different
> kind of turd:
>
> - if (xen_domain())
> + if (xen_domain() || pseries_secure_vm())
> return true;
I don't think it's pseries specific actually. E.g. I suspect AMD SEV
might benefit from the same kind of hack.
> So to summarize, and make sure I'm not missing something, the two approaches
> at hand are either:
>
> 1- The above, which is a one liner and contained in the guest, so that's nice, but
> also means another turd in virtio which isn't ...
>
> 2- We force pseries to always set VIRTIO_F_IOMMU_PLATFORM, but with the current
> architecture on our side that will force virtio to always go through an emulated
> iommu, as pseries doesn't have the concept of a real bypass window, and thus will
> impact performance for both secure and non-secure VMs.
>
> 3- Invent a property that can be put in selected PCI device tree nodes that
> indicates that for that device specifically, the iommu can be bypassed, along with
> a hypercall to turn that bypass on/off. Virtio would then use VIRTIO_F_IOMMU_PLATFORM
> but its DT nodes would also have that property and Linux would notice it and turn
> bypass on.
For completeness, virtio could also have its own bounce buffer
outside of DMA API one. I don't see lots of benefits to this
though.
> The resulting properties of those options are:
>
> 1- Is what I want because it's the simplest, provides the best performance now,
> and works without code changes to qemu or non-secure Linux. However it does
> add a tiny turd to virtio which is annoying.
>
> 2- This works but it puts the iommu in the way always, thus reducing virtio performance
> accross the board for pseries unless we only do that for secure VMs but that is
> difficult (as discussed earlier).
>
> 3- This would recover the performance lost in -2-, however it requires qemu *and*
> guest changes. Specifically, existing guests (RHEL 7 etc...) would get the
> performance hit of -2- unless modified to call that 'enable bypass' call, which
> isn't great.
>
> So imho we have to chose one of 3 not-great solutions here... Unless I missed
> something in your ideas of course.
>
> Cheers,
> Ben.
>
>
^ permalink raw reply
* Re: [PATCH 1/2] powerpc/64s: move machine check SLB flushing to mm/slb.c
From: Michal Suchánek @ 2018-08-08 20:22 UTC (permalink / raw)
To: Nicholas Piggin
Cc: linuxppc-dev, Gautham R . Shenoy, Mahesh Jagannath Salgaonkar,
kvm-ppc, Aneesh Kumar K.V, Akshay Adiga
In-Reply-To: <20180803041350.25493-1-npiggin@gmail.com>
On Fri, 3 Aug 2018 14:13:49 +1000
Nicholas Piggin <npiggin@gmail.com> wrote:
> The machine check code that flushes and restores bolted segments in
> real mode belongs in mm/slb.c. This will be used by pseries machine
> check and idle code.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> arch/powerpc/include/asm/book3s/64/mmu-hash.h | 3 ++
> arch/powerpc/kernel/mce_power.c | 21 ++--------
> arch/powerpc/mm/slb.c | 38
> +++++++++++++++++++ 3 files changed, 44 insertions(+), 18 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/book3s/64/mmu-hash.h
> b/arch/powerpc/include/asm/book3s/64/mmu-hash.h index
> 2f74bdc805e0..d4e398185b3a 100644 ---
> a/arch/powerpc/include/asm/book3s/64/mmu-hash.h +++
> b/arch/powerpc/include/asm/book3s/64/mmu-hash.h @@ -497,6 +497,9 @@
> extern void hpte_init_native(void);
> extern void slb_initialize(void);
> extern void slb_flush_and_rebolt(void);
> +extern void slb_flush_all_realmode(void);
> +extern void __slb_restore_bolted_realmode(void);
> +extern void slb_restore_bolted_realmode(void);
>
> extern void slb_vmalloc_update(void);
> extern void slb_set_size(u16 size);
> diff --git a/arch/powerpc/kernel/mce_power.c
> b/arch/powerpc/kernel/mce_power.c index d6756af6ec78..50f7b9817246
> 100644 --- a/arch/powerpc/kernel/mce_power.c
> +++ b/arch/powerpc/kernel/mce_power.c
> @@ -62,11 +62,8 @@ static unsigned long addr_to_pfn(struct pt_regs
> *regs, unsigned long addr) #ifdef CONFIG_PPC_BOOK3S_64
> static void flush_and_reload_slb(void)
> {
> - struct slb_shadow *slb;
> - unsigned long i, n;
> -
> /* Invalidate all SLBs */
> - asm volatile("slbmte %0,%0; slbia" : : "r" (0));
> + slb_flush_all_realmode();
>
> #ifdef CONFIG_KVM_BOOK3S_HANDLER
> /*
> @@ -76,22 +73,10 @@ static void flush_and_reload_slb(void)
> if (get_paca()->kvm_hstate.in_guest)
> return;
> #endif
> -
> - /* For host kernel, reload the SLBs from shadow SLB buffer.
> */
> - slb = get_slb_shadow();
> - if (!slb)
> + if (early_radix_enabled())
> return;
And we lose the check that the shadow slb exists. Is !slb equivalent to
early_radix_enabled()
>
> - n = min_t(u32, be32_to_cpu(slb->persistent), SLB_MIN_SIZE);
> -
> - /* Load up the SLB entries from shadow SLB */
> - for (i = 0; i < n; i++) {
> - unsigned long rb =
> be64_to_cpu(slb->save_area[i].esid);
> - unsigned long rs =
> be64_to_cpu(slb->save_area[i].vsid); -
> - rb = (rb & ~0xFFFul) | i;
> - asm volatile("slbmte %0,%1" : : "r" (rs), "r" (rb));
> - }
> + slb_restore_bolted_realmode();
> }
> #endif
>
> diff --git a/arch/powerpc/mm/slb.c b/arch/powerpc/mm/slb.c
> index cb796724a6fc..136db8652577 100644
> --- a/arch/powerpc/mm/slb.c
> +++ b/arch/powerpc/mm/slb.c
> @@ -90,6 +90,44 @@ static inline void create_shadowed_slbe(unsigned
> long ea, int ssize, : "memory" );
> }
>
> +/*
> + * Insert bolted entries into SLB (which may not be empty).
> + */
> +void __slb_restore_bolted_realmode(void)
> +{
> + struct slb_shadow *p = get_slb_shadow();
> + enum slb_index index;
or can we get here at some point when shadow slb is not populated?
Thanks
Michal
> +
> + /* No isync needed because realmode. */
> + for (index = 0; index < SLB_NUM_BOLTED; index++) {
> + asm volatile("slbmte %0,%1" :
> + : "r" (be64_to_cpu(p->save_area[index].vsid)),
> + "r" (be64_to_cpu(p->save_area[index].esid)));
> + }
> +}
> +
> +/*
> + * Insert bolted entries into an empty SLB.
> + * This is not the same as rebolt because the bolted segments
> + * (e.g., kstack) are not changed (rebolted).
> + */
> +void slb_restore_bolted_realmode(void)
> +{
> + __slb_restore_bolted_realmode();
> + get_paca()->slb_cache_ptr = 0;
> +}
> +
> +/*
> + * This flushes all SLB entries including 0, so it must be realmode.
> + */
> +void slb_flush_all_realmode(void)
> +{
> + /*
> + * This flushes all SLB entries including 0, so it must be
> realmode.
> + */
> + asm volatile("slbmte %0,%0; slbia" : : "r" (0));
> +}
> +
> static void __slb_flush_and_rebolt(void)
> {
> /* If you change this make sure you change SLB_NUM_BOLTED
^ permalink raw reply
* Re: [RFC 5/5] powerpc/fsl: Add supported-irq-ranges for P2020
From: Scott Wood @ 2018-08-08 17:57 UTC (permalink / raw)
To: Bharat Bhushan, benh@kernel.crashing.org, paulus@samba.org,
mpe@ellerman.id.au, galak@kernel.crashing.org,
mark.rutland@arm.com, kstewart@linuxfoundation.org,
gregkh@linuxfoundation.org, devicetree@vger.kernel.org,
linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org
Cc: robh@kernel.org, keescook@chromium.org, tyreld@linux.vnet.ibm.com,
joe@perches.com
In-Reply-To: <AM5PR0401MB2545041D99EEDC3136B201719A260@AM5PR0401MB2545.eurprd04.prod.outlook.com>
On Wed, 2018-08-08 at 06:28 +0000, Bharat Bhushan wrote:
> > -----Original Message-----
> > From: Scott Wood [mailto:oss@buserror.net]
> > Sent: Wednesday, August 8, 2018 11:26 AM
> > To: Bharat Bhushan <bharat.bhushan@nxp.com>;
> > benh@kernel.crashing.org; paulus@samba.org; mpe@ellerman.id.au;
> > galak@kernel.crashing.org; mark.rutland@arm.com;
> > kstewart@linuxfoundation.org; gregkh@linuxfoundation.org;
> > devicetree@vger.kernel.org; linuxppc-dev@lists.ozlabs.org; linux-
> > kernel@vger.kernel.org
> > Cc: robh@kernel.org; keescook@chromium.org; tyreld@linux.vnet.ibm.com;
> > joe@perches.com
> > Subject: Re: [RFC 5/5] powerpc/fsl: Add supported-irq-ranges for P2020
> >
> > On Wed, 2018-08-08 at 03:44 +0000, Bharat Bhushan wrote:
> > > > -----Original Message-----
> > > > From: Scott Wood [mailto:oss@buserror.net]
> > > > Sent: Wednesday, August 8, 2018 2:44 AM
> > > > To: Bharat Bhushan <bharat.bhushan@nxp.com>;
> > > > benh@kernel.crashing.org; paulus@samba.org; mpe@ellerman.id.au;
> > > > galak@kernel.crashing.org; mark.rutland@arm.com;
> > > > kstewart@linuxfoundation.org; gregkh@linuxfoundation.org;
> > > > devicetree@vger.kernel.org; linuxppc-dev@lists.ozlabs.org; linux-
> > > > kernel@vger.kernel.org
> > > > Cc: robh@kernel.org; keescook@chromium.org;
> > > > tyreld@linux.vnet.ibm.com; joe@perches.com
> > > > Subject: Re: [RFC 5/5] powerpc/fsl: Add supported-irq-ranges for
> > > > P2020
> > > >
> > > > On Fri, 2018-07-27 at 15:18 +0530, Bharat Bhushan wrote:
> > > > > MPIC on NXP (Freescale) P2020 supports following irq
> > > > > ranges:
> > > > > > 0 - 11 (External interrupt)
> > > > > > 16 - 79 (Internal interrupt)
> > > > > > 176 - 183 (Messaging interrupt)
> > > > > > 224 - 231 (Shared message signaled interrupt)
> > > >
> > > > Why don't you convert to the 4-cell interrupt specifiers that make
> > > > dealing with these ranges less error-prone?
> > >
> > > Ok , will do if we agree to have this series as per comment on other
> > > patch.
> >
> > If you're concerned with errors, this would be a good things to do
> > regardless.
> > Actually, it seems that p2020si-post.dtsi already uses 4-cell interrupts.
> >
> > What is motivating this patchset? Is there something wrong in the
> > existing
> > dts files?
>
> There is no error in device tree. Main motivation is to improve code for
> following reasons:
> - While code study it was found that if a reserved irq-number used then
> there are no check in driver. irq will be configured as correct and
> interrupt will never fire.
Again, a wrong interrupt number won't fire, whether an interrupt by that
number exists or not. I wouldn't mind a sanity check in the driver if the
programming model made it properly discoverable, but I don't think it's worth
messing with device trees just for this (and even less so given that there
don't seem to be new chips coming out that this would be relevant for).
> > > One other confusing observation I have is that "irq_count" from
> > > platform code is given precedence over "last-interrupt-source" in
> > > device-
> >
> > tree.
> > > Should not device-tree should have precedence otherwise there is no
> > > point using " last-interrupt-source" if platform code passes
> > > "irq_count" in mpic_alloc().
> >
> > Maybe, though I don't think it matters much given that last-interrupt-
> > source
> > was only added to avoid having to pass irq_count in platform code.
>
> Thanks for clarifying;
>
> My understanding was that "last-interrupt-source" added to ensure that we
> can over-ride value passed from platform code. In that case we do not need
> to change code and can control from device tree.
The changelog says, "To avoid needing to write custom board-specific code to
detect that scenario, allow it to be easily overridden in the device-tree,"
where "it" means the value provided by hardware. The goal was to pass in 256
without board code in the kernel, not to override the 256.
-Scott
^ permalink raw reply
* Re: [PATCH 1/5] powerpc/asm: Add a patch_site macro & helpers for patching instructions
From: Christophe LEROY @ 2018-08-08 16:30 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev
In-Reply-To: <20180723150756.11108-1-mpe@ellerman.id.au>
Le 23/07/2018 à 17:07, Michael Ellerman a écrit :
> Add a macro and some helper C functions for patching single asm
> instructions.
>
> The gas macro means we can do something like:
>
> 1: nop
> patch_site 1b, patch__foo
>
> Which is less visually distracting than defining a GLOBAL symbol at 1,
> and also doesn't pollute the symbol table which can confuse eg. perf.
>
> These are obviously similar to our existing feature sections, but are
> not automatically patched based on CPU/MMU features, rather they are
> designed to be manually patched by C code at some arbitrary point.
>
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> ---
> arch/powerpc/include/asm/code-patching-asm.h | 18 ++++++++++++++++++
> arch/powerpc/include/asm/code-patching.h | 2 ++
> arch/powerpc/lib/code-patching.c | 16 ++++++++++++++++
> 3 files changed, 36 insertions(+)
> create mode 100644 arch/powerpc/include/asm/code-patching-asm.h
>
> diff --git a/arch/powerpc/include/asm/code-patching-asm.h b/arch/powerpc/include/asm/code-patching-asm.h
> new file mode 100644
> index 000000000000..ed7b1448493a
> --- /dev/null
> +++ b/arch/powerpc/include/asm/code-patching-asm.h
> @@ -0,0 +1,18 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * Copyright 2018, Michael Ellerman, IBM Corporation.
> + */
> +#ifndef _ASM_POWERPC_CODE_PATCHING_ASM_H
> +#define _ASM_POWERPC_CODE_PATCHING_ASM_H
> +
> +/* Define a "site" that can be patched */
> +.macro patch_site label name
> + .pushsection ".rodata"
> + .balign 4
> + .global \name
> +\name:
> + .4byte \label - .
> + .popsection
> +.endm
> +
> +#endif /* _ASM_POWERPC_CODE_PATCHING_ASM_H */
> diff --git a/arch/powerpc/include/asm/code-patching.h b/arch/powerpc/include/asm/code-patching.h
> index 812535f40124..b2051234ada8 100644
> --- a/arch/powerpc/include/asm/code-patching.h
> +++ b/arch/powerpc/include/asm/code-patching.h
> @@ -32,6 +32,8 @@ unsigned int create_cond_branch(const unsigned int *addr,
> int patch_branch(unsigned int *addr, unsigned long target, int flags);
> int patch_instruction(unsigned int *addr, unsigned int instr);
> int raw_patch_instruction(unsigned int *addr, unsigned int instr);
> +int patch_instruction_site(s32 *addr, unsigned int instr);
> +int patch_branch_site(s32 *site, unsigned long target, int flags);
Why use s32* instead of unsigned int* as usual for pointer to code ?
Christophe
>
> int instr_is_relative_branch(unsigned int instr);
> int instr_is_relative_link_branch(unsigned int instr);
> diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
> index e0d881ab304e..850f3b8f4da5 100644
> --- a/arch/powerpc/lib/code-patching.c
> +++ b/arch/powerpc/lib/code-patching.c
> @@ -195,6 +195,22 @@ int patch_branch(unsigned int *addr, unsigned long target, int flags)
> return patch_instruction(addr, create_branch(addr, target, flags));
> }
>
> +int patch_branch_site(s32 *site, unsigned long target, int flags)
> +{
> + unsigned int *addr;
> +
> + addr = (unsigned int *)((unsigned long)site + *site);
> + return patch_instruction(addr, create_branch(addr, target, flags));
> +}
> +
> +int patch_instruction_site(s32 *site, unsigned int instr)
> +{
> + unsigned int *addr;
> +
> + addr = (unsigned int *)((unsigned long)site + *site);
> + return patch_instruction(addr, instr);
> +}
> +
> bool is_offset_in_branch_range(long offset)
> {
> /*
>
^ permalink raw reply
* Re: [PATCH v7 8/9] powerpc/mce: Add sysctl control for recovery action on MCE.
From: Michal Suchánek @ 2018-08-08 16:09 UTC (permalink / raw)
To: Aneesh Kumar K.V
Cc: Michael Ellerman, Mahesh J Salgaonkar, linuxppc-dev,
Laurent Dufour, Michal Suchanek, Aneesh Kumar K.V,
Nicholas Piggin, Ananth Narayan
In-Reply-To: <d08a7794-e5c2-2b05-c21a-ae5a7baa7d88@linux.ibm.com>
On Wed, 8 Aug 2018 21:07:11 +0530
"Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> wrote:
> On 08/08/2018 08:26 PM, Michael Ellerman wrote:
> > Mahesh J Salgaonkar <mahesh@linux.vnet.ibm.com> writes:
> >> From: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
> >>
> >> Introduce recovery action for recovered memory errors (MCEs).
> >> There are soft memory errors like SLB Multihit, which can be a
> >> result of a bad hardware OR software BUG. Kernel can easily
> >> recover from these soft errors by flushing SLB contents. After the
> >> recovery kernel can still continue to function without any issue.
> >> But in some scenario's we may keep getting these soft errors until
> >> the root cause is fixed. To be able to analyze and find the root
> >> cause, best way is to gather enough data and system state at the
> >> time of MCE. Hence this patch introduces a sysctl knob where user
> >> can decide either to continue after recovery or panic the kernel
> >> to capture the dump.
> >
> > I'm not convinced we want this.
> >
> > As we've discovered it's often not possible to reconstruct what
> > happened based on a dump anyway.
> >
> > The key thing you need is the content of the SLB and that's not
> > included in a dump.
> >
> > So I think we should dump the SLB content when we get the MCE (which
> > this series does) and any other useful info, and then if we can
> > recover we should.
> >
>
> The reasoning there is what if we got multi-hit due to some
> corruption in slb_cache_ptr. ie. some part of kernel is wrongly
> updating the paca data structure due to wrong pointer. Now that is
> far fetched, but then possible right?. Hence the idea that, if we
> don't have much insight into why a slb multi-hit occur from the dmesg
> which include slb content, slb_cache contents etc, there should be an
> easy way to force a dump that might assist in further debug.
Nonetheless this turns all MCEs into crashes. Are there any MCEs that
could happen during normal operation and should be handled by default?
Thanks
Michal
^ permalink raw reply
* Re: [PATCH v2] powerpc/tm: Print 64-bits MSR
From: Breno Leitao @ 2018-08-08 15:50 UTC (permalink / raw)
To: LEROY Christophe; +Cc: mikey, linuxppc-dev
In-Reply-To: <20180807205718.Horde.hQCYOtyzfY7xFtxCwRj2Iw1@messagerie.si.c-s.fr>
Hi Leroy,
On 08/07/2018 03:57 PM, LEROY Christophe wrote:
> Breno Leitao <leitao@debian.org> a écrit :
>> On 08/07/2018 02:15 PM, Christophe LEROY wrote:
>>> Le 07/08/2018 à 15:35, Breno Leitao a écrit :
>>> I think it would be better to change 'reason' to 'unsigned long' instead of
>>> replacing it by regs->msr for the printk.
>>
>> That was my initial approach, but this code seems to run on 32 bits system,
>> and I do not want to change the whole 'reason' bit width without having a 32
>> bits to test, at least.
>
> But 'unsigned long' is still 32 bits on ppc32, so it makes no difference with
> 'unsigned int'
> And I will test it for you if needed
Cool, I really appreciate it, and I would definitely need it once I have a
more intrusive HTM patchset I am working on.
Regarding this one, I think the change is so simple as-is that I would
prefer to continue with the v2 patch, if you do not mind.
Thank you!
^ permalink raw reply
* Re: [PATCH v02] powerpc/mobility: Fix node detach/rename problem
From: Michael Bringmann @ 2018-08-08 15:39 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <19f9e274-26ae-2ae5-de9f-c0fcd6c00570@linux.vnet.ibm.com>
I will update the header files 'of_private.h' and 'of.h' and repost.
Michael
On 08/08/2018 10:37 AM, Michael Bringmann wrote:
> On 08/08/2018 09:02 AM, Michael Ellerman wrote:
>> Michael Bringmann <mwb@linux.vnet.ibm.com> writes:
>>> diff --git a/arch/powerpc/platforms/pseries/mobility.c b/arch/powerpc/platforms/pseries/mobility.c
>>> index e245a88..efc9442 100644
>>> --- a/arch/powerpc/platforms/pseries/mobility.c
>>> +++ b/arch/powerpc/platforms/pseries/mobility.c
>>> @@ -22,6 +22,9 @@
>>> #include <asm/rtas.h>
>>> #include "pseries.h"
>>>
>>> +extern int of_free_phandle_cache(void);
>>> +extern void of_populate_phandle_cache(void);
>>
>> We don't do that, they should be in a header.
>>
>> But that's a minor problem given that the patch doesn't compile, because
>> both those functions are static.
>
> I am building against the latest 'linux-ppc' kernel. It includes patch
>
> Commit b9952b5218added5577e4a3443969bc20884cea9 Mon Sep 17 00:00:00 2001
> From: Frank Rowand <frank.rowand@sony.com>
> Date: Thu, 12 Jul 2018 14:00:07 -0700
> Subject: of: overlay: update phandle cache on overlay apply and remove
>
> which makes the functions static. I will rebuild and test with an
> earlier version if you will specify which one.
>
>>
>> Presumably you have a hack in your tree to make them non-static?
>> Please try and compile your patches in a clean tree before sending.
>>
>> cheers
>
> Regards,
> Michael
>
--
Michael W. Bringmann
Linux Technology Center
IBM Corporation
Tie-Line 363-5196
External: (512) 286-5196
Cell: (512) 466-0650
mwb@linux.vnet.ibm.com
^ permalink raw reply
* Re: [PATCH v02] powerpc/mobility: Fix node detach/rename problem
From: Michael Bringmann @ 2018-08-08 15:37 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <87zhxx9cs7.fsf@concordia.ellerman.id.au>
On 08/08/2018 09:02 AM, Michael Ellerman wrote:
> Michael Bringmann <mwb@linux.vnet.ibm.com> writes:
>> diff --git a/arch/powerpc/platforms/pseries/mobility.c b/arch/powerpc/platforms/pseries/mobility.c
>> index e245a88..efc9442 100644
>> --- a/arch/powerpc/platforms/pseries/mobility.c
>> +++ b/arch/powerpc/platforms/pseries/mobility.c
>> @@ -22,6 +22,9 @@
>> #include <asm/rtas.h>
>> #include "pseries.h"
>>
>> +extern int of_free_phandle_cache(void);
>> +extern void of_populate_phandle_cache(void);
>
> We don't do that, they should be in a header.
>
> But that's a minor problem given that the patch doesn't compile, because
> both those functions are static.
I am building against the latest 'linux-ppc' kernel. It includes patch
Commit b9952b5218added5577e4a3443969bc20884cea9 Mon Sep 17 00:00:00 2001
From: Frank Rowand <frank.rowand@sony.com>
Date: Thu, 12 Jul 2018 14:00:07 -0700
Subject: of: overlay: update phandle cache on overlay apply and remove
which makes the functions static. I will rebuild and test with an
earlier version if you will specify which one.
>
> Presumably you have a hack in your tree to make them non-static?
> Please try and compile your patches in a clean tree before sending.
>
> cheers
Regards,
Michael
--
Michael W. Bringmann
Linux Technology Center
IBM Corporation
Tie-Line 363-5196
External: (512) 286-5196
Cell: (512) 466-0650
mwb@linux.vnet.ibm.com
^ permalink raw reply
* Re: [PATCH v7 8/9] powerpc/mce: Add sysctl control for recovery action on MCE.
From: Aneesh Kumar K.V @ 2018-08-08 15:37 UTC (permalink / raw)
To: Michael Ellerman, Mahesh J Salgaonkar, linuxppc-dev
Cc: Aneesh Kumar K.V, Michal Suchanek, Ananth Narayan,
Nicholas Piggin, Laurent Dufour
In-Reply-To: <87o9ecaovz.fsf@concordia.ellerman.id.au>
On 08/08/2018 08:26 PM, Michael Ellerman wrote:
> Mahesh J Salgaonkar <mahesh@linux.vnet.ibm.com> writes:
>> From: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
>>
>> Introduce recovery action for recovered memory errors (MCEs). There are
>> soft memory errors like SLB Multihit, which can be a result of a bad
>> hardware OR software BUG. Kernel can easily recover from these soft errors
>> by flushing SLB contents. After the recovery kernel can still continue to
>> function without any issue. But in some scenario's we may keep getting
>> these soft errors until the root cause is fixed. To be able to analyze and
>> find the root cause, best way is to gather enough data and system state at
>> the time of MCE. Hence this patch introduces a sysctl knob where user can
>> decide either to continue after recovery or panic the kernel to capture the
>> dump.
>
> I'm not convinced we want this.
>
> As we've discovered it's often not possible to reconstruct what happened
> based on a dump anyway.
>
> The key thing you need is the content of the SLB and that's not included
> in a dump.
>
> So I think we should dump the SLB content when we get the MCE (which
> this series does) and any other useful info, and then if we can recover
> we should.
>
The reasoning there is what if we got multi-hit due to some corruption
in slb_cache_ptr. ie. some part of kernel is wrongly updating the paca
data structure due to wrong pointer. Now that is far fetched, but then
possible right?. Hence the idea that, if we don't have much insight into
why a slb multi-hit occur from the dmesg which include slb content,
slb_cache contents etc, there should be an easy way to force a dump that
might assist in further debug.
-aneesh
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox