* 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 16/20] powerpc/dma: use dma_direct_{alloc,free}
From: Benjamin Herrenschmidt @ 2018-08-09 0:52 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, Scott Wood
In-Reply-To: <20180730163824.10064-17-hch@lst.de>
On Mon, 2018-07-30 at 18:38 +0200, Christoph Hellwig wrote:
> These do the same functionality as the existing helpers, but do it
> simpler, and also allow the (optional) use of CMA.
>
> Note that the swiotlb code now calls into the dma_direct code directly,
> given that it doesn't work with noncoherent caches at all, and isn't called
> when we have an iommu either, so the iommu special case in
> dma_nommu_alloc_coherent isn't required for swiotlb.
I am not convinced that this will produce the same results due to
the way the zone picking works.
As for the interaction with swiotlb, we'll need the FSL guys to have
a look. Scott, do you remember what this is about ?
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> arch/powerpc/include/asm/pgtable.h | 1 -
> arch/powerpc/kernel/dma-swiotlb.c | 4 +-
> arch/powerpc/kernel/dma.c | 78 ++++--------------------------
> arch/powerpc/mm/mem.c | 19 --------
> 4 files changed, 11 insertions(+), 91 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/pgtable.h b/arch/powerpc/include/asm/pgtable.h
> index 14c79a7dc855..123de4958d2e 100644
> --- a/arch/powerpc/include/asm/pgtable.h
> +++ b/arch/powerpc/include/asm/pgtable.h
> @@ -38,7 +38,6 @@ extern unsigned long empty_zero_page[];
> extern pgd_t swapper_pg_dir[];
>
> void limit_zone_pfn(enum zone_type zone, unsigned long max_pfn);
> -int dma_pfn_limit_to_zone(u64 pfn_limit);
> extern void paging_init(void);
>
> /*
> diff --git a/arch/powerpc/kernel/dma-swiotlb.c b/arch/powerpc/kernel/dma-swiotlb.c
> index f6e0701c5303..25986fcd1e5e 100644
> --- a/arch/powerpc/kernel/dma-swiotlb.c
> +++ b/arch/powerpc/kernel/dma-swiotlb.c
> @@ -46,8 +46,8 @@ static u64 swiotlb_powerpc_get_required(struct device *dev)
> * for everything else.
> */
> const struct dma_map_ops powerpc_swiotlb_dma_ops = {
> - .alloc = __dma_nommu_alloc_coherent,
> - .free = __dma_nommu_free_coherent,
> + .alloc = dma_direct_alloc,
> + .free = dma_direct_free,
> .mmap = dma_nommu_mmap_coherent,
> .map_sg = swiotlb_map_sg_attrs,
> .unmap_sg = swiotlb_unmap_sg_attrs,
> diff --git a/arch/powerpc/kernel/dma.c b/arch/powerpc/kernel/dma.c
> index 2cfc45acbb52..2b90a403cdac 100644
> --- a/arch/powerpc/kernel/dma.c
> +++ b/arch/powerpc/kernel/dma.c
> @@ -26,75 +26,6 @@
> * can set archdata.dma_data to an unsigned long holding the offset. By
> * default the offset is PCI_DRAM_OFFSET.
> */
> -
> -static u64 __maybe_unused get_pfn_limit(struct device *dev)
> -{
> - u64 pfn = (dev->coherent_dma_mask >> PAGE_SHIFT) + 1;
> - struct dev_archdata __maybe_unused *sd = &dev->archdata;
> -
> -#ifdef CONFIG_SWIOTLB
> - if (sd->max_direct_dma_addr && dev->dma_ops == &powerpc_swiotlb_dma_ops)
> - pfn = min_t(u64, pfn, sd->max_direct_dma_addr >> PAGE_SHIFT);
> -#endif
> -
> - return pfn;
> -}
> -
> -#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;
> - struct page *page;
> - int node = dev_to_node(dev);
> -#ifdef CONFIG_FSL_SOC
> - u64 pfn = get_pfn_limit(dev);
> - int zone;
> -
> - /*
> - * This code should be OK on other platforms, but we have drivers that
> - * don't set coherent_dma_mask. As a workaround we just ifdef it. This
> - * whole routine needs some serious cleanup.
> - */
> -
> - zone = dma_pfn_limit_to_zone(pfn);
> - if (zone < 0) {
> - dev_err(dev, "%s: No suitable zone for pfn %#llx\n",
> - __func__, pfn);
> - return NULL;
> - }
> -
> - switch (zone) {
> - case ZONE_DMA:
> - flag |= GFP_DMA;
> - break;
> -#ifdef CONFIG_ZONE_DMA32
> - case ZONE_DMA32:
> - flag |= GFP_DMA32;
> - break;
> -#endif
> - };
> -#endif /* CONFIG_FSL_SOC */
> -
> - page = alloc_pages_node(node, flag, get_order(size));
> - if (page == NULL)
> - return NULL;
> - ret = page_address(page);
> - memset(ret, 0, size);
> - *dma_handle = phys_to_dma(dev,__pa(ret));
> -
> - return ret;
> -}
> -
> -void __dma_nommu_free_coherent(struct device *dev, size_t size,
> - void *vaddr, dma_addr_t dma_handle,
> - unsigned long attrs)
> -{
> - free_pages((unsigned long)vaddr, get_order(size));
> -}
> -#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,
> unsigned long attrs)
> @@ -105,8 +36,12 @@ static void *dma_nommu_alloc_coherent(struct device *dev, size_t size,
> * we can really use the direct ops
> */
> if (dma_direct_supported(dev, dev->coherent_dma_mask))
> +#ifdef CONFIG_NOT_COHERENT_CACHE
> return __dma_nommu_alloc_coherent(dev, size, dma_handle,
> flag, attrs);
> +#else
> + return dma_direct_alloc(dev, size, dma_handle, flag, attrs);
> +#endif
>
> /* Ok we can't ... do we have an iommu ? If not, fail */
> iommu = get_iommu_table_base(dev);
> @@ -127,8 +62,13 @@ static void dma_nommu_free_coherent(struct device *dev, size_t size,
>
> /* See comments in dma_nommu_alloc_coherent() */
> if (dma_direct_supported(dev, dev->coherent_dma_mask))
> +#ifdef CONFIG_NOT_COHERENT_CACHE
> return __dma_nommu_free_coherent(dev, size, vaddr, dma_handle,
> attrs);
> +#else
> + return dma_direct_free(dev, size, vaddr, dma_handle, attrs);
> +#endif
> +
> /* Maybe we used an iommu ... */
> iommu = get_iommu_table_base(dev);
>
> diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
> index 5c8530d0c611..ec8ed9d7abef 100644
> --- a/arch/powerpc/mm/mem.c
> +++ b/arch/powerpc/mm/mem.c
> @@ -276,25 +276,6 @@ void __init limit_zone_pfn(enum zone_type zone, unsigned long pfn_limit)
> }
> }
>
> -/*
> - * Find the least restrictive zone that is entirely below the
> - * specified pfn limit. Returns < 0 if no suitable zone is found.
> - *
> - * pfn_limit must be u64 because it can exceed 32 bits even on 32-bit
> - * systems -- the DMA limit can be higher than any possible real pfn.
> - */
> -int dma_pfn_limit_to_zone(u64 pfn_limit)
> -{
> - int i;
> -
> - for (i = TOP_ZONE; i >= 0; i--) {
> - if (max_zone_pfns[i] <= pfn_limit)
> - return i;
> - }
> -
> - return -EPERM;
> -}
> -
> /*
> * paging_init() sets up the page tables - in fact we've already done this.
> */
^ permalink raw reply
* Re: [PATCH 17/20] powerpc/dma-swiotlb: use generic swiotlb_dma_ops
From: Benjamin Herrenschmidt @ 2018-08-09 0: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-18-hch@lst.de>
On Mon, 2018-07-30 at 18:38 +0200, Christoph Hellwig wrote:
> These are identical to the arch specific ones, so remove them.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> ---
> arch/powerpc/include/asm/dma-direct.h | 4 ++++
> arch/powerpc/include/asm/swiotlb.h | 2 --
> arch/powerpc/kernel/dma-swiotlb.c | 28 ++-------------------------
> arch/powerpc/sysdev/fsl_pci.c | 2 +-
> 4 files changed, 7 insertions(+), 29 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/dma-direct.h b/arch/powerpc/include/asm/dma-direct.h
> index 0fba19445ae8..657f84ddb20d 100644
> --- a/arch/powerpc/include/asm/dma-direct.h
> +++ b/arch/powerpc/include/asm/dma-direct.h
> @@ -30,4 +30,8 @@ static inline phys_addr_t __dma_to_phys(struct device *dev, dma_addr_t daddr)
> return daddr - PCI_DRAM_OFFSET;
> return daddr - dev->archdata.dma_offset;
> }
> +
> +u64 swiotlb_powerpc_get_required(struct device *dev);
> +#define swiotlb_get_required_mask swiotlb_powerpc_get_required
> +
> #endif /* ASM_POWERPC_DMA_DIRECT_H */
> diff --git a/arch/powerpc/include/asm/swiotlb.h b/arch/powerpc/include/asm/swiotlb.h
> index f65ecf57b66c..1d8c1da26ab3 100644
> --- a/arch/powerpc/include/asm/swiotlb.h
> +++ b/arch/powerpc/include/asm/swiotlb.h
> @@ -13,8 +13,6 @@
>
> #include <linux/swiotlb.h>
>
> -extern const struct dma_map_ops powerpc_swiotlb_dma_ops;
> -
> extern unsigned int ppc_swiotlb_enable;
> int __init swiotlb_setup_bus_notifier(void);
>
> diff --git a/arch/powerpc/kernel/dma-swiotlb.c b/arch/powerpc/kernel/dma-swiotlb.c
> index 25986fcd1e5e..0c269de61f39 100644
> --- a/arch/powerpc/kernel/dma-swiotlb.c
> +++ b/arch/powerpc/kernel/dma-swiotlb.c
> @@ -24,7 +24,7 @@
>
> unsigned int ppc_swiotlb_enable;
>
> -static u64 swiotlb_powerpc_get_required(struct device *dev)
> +u64 swiotlb_powerpc_get_required(struct device *dev)
> {
> u64 end, mask, max_direct_dma_addr = dev->archdata.max_direct_dma_addr;
>
> @@ -38,30 +38,6 @@ static u64 swiotlb_powerpc_get_required(struct device *dev)
> return mask;
> }
>
> -/*
> - * At the moment, all platforms that use this code only require
> - * swiotlb to be used if we're operating on HIGHMEM. Since
> - * we don't ever call anything other than map_sg, unmap_sg,
> - * map_page, and unmap_page on highmem, use normal dma_ops
> - * for everything else.
> - */
> -const struct dma_map_ops powerpc_swiotlb_dma_ops = {
> - .alloc = dma_direct_alloc,
> - .free = dma_direct_free,
> - .mmap = dma_nommu_mmap_coherent,
> - .map_sg = swiotlb_map_sg_attrs,
> - .unmap_sg = swiotlb_unmap_sg_attrs,
> - .dma_supported = swiotlb_dma_supported,
> - .map_page = swiotlb_map_page,
> - .unmap_page = swiotlb_unmap_page,
> - .sync_single_for_cpu = swiotlb_sync_single_for_cpu,
> - .sync_single_for_device = swiotlb_sync_single_for_device,
> - .sync_sg_for_cpu = swiotlb_sync_sg_for_cpu,
> - .sync_sg_for_device = swiotlb_sync_sg_for_device,
> - .mapping_error = swiotlb_dma_mapping_error,
> - .get_required_mask = swiotlb_powerpc_get_required,
> -};
> -
> void pci_dma_dev_setup_swiotlb(struct pci_dev *pdev)
> {
> struct pci_controller *hose;
> @@ -88,7 +64,7 @@ static int ppc_swiotlb_bus_notify(struct notifier_block *nb,
>
> /* May need to bounce if the device can't address all of DRAM */
> if ((dma_get_mask(dev) + 1) < memblock_end_of_DRAM())
> - set_dma_ops(dev, &powerpc_swiotlb_dma_ops);
> + set_dma_ops(dev, &swiotlb_dma_ops);
>
> return NOTIFY_DONE;
> }
> diff --git a/arch/powerpc/sysdev/fsl_pci.c b/arch/powerpc/sysdev/fsl_pci.c
> index 918be816b097..daf44bc0108d 100644
> --- a/arch/powerpc/sysdev/fsl_pci.c
> +++ b/arch/powerpc/sysdev/fsl_pci.c
> @@ -118,7 +118,7 @@ static void setup_swiotlb_ops(struct pci_controller *hose)
> {
> if (ppc_swiotlb_enable) {
> hose->controller_ops.dma_dev_setup = pci_dma_dev_setup_swiotlb;
> - set_pci_dma_ops(&powerpc_swiotlb_dma_ops);
> + set_pci_dma_ops(&swiotlb_dma_ops);
> }
> }
> #else
^ permalink raw reply
* Re: [PATCH 18/20] powerpc/dma-noncoherent: use generic dma_noncoherent_ops
From: Benjamin Herrenschmidt @ 2018-08-09 1:00 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-19-hch@lst.de>
On Mon, 2018-07-30 at 18:38 +0200, Christoph Hellwig wrote:
> The generic dma-noncoherent code provides all that is needed by powerpc.
>
> Note that the cache maintainance in the existing code is a bit odd
> as it implements both the sync_to_device and sync_to_cpu callouts,
> but never flushes caches when unmapping. This patch keeps both
> directions arounds, which will lead to more flushing than the previous
> implementation. Someone more familar with the required CPUs should
> eventually take a look and optimize the cache flush handling if needed.
The original code looks bogus indeed.
I think we got away with it because those older CPUs wouldn't speculate
or prefetch aggressively enough (or at all) so the flush on map was
sufficient, the stuff wouldn't come back into the cache.
But safe is better than sorry, so ... tentative Ack, I do need to try
to dig one of these things to test, which might take a while.
Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> arch/powerpc/Kconfig | 2 +-
> arch/powerpc/include/asm/dma-mapping.h | 29 -------------
> arch/powerpc/kernel/dma.c | 59 +++-----------------------
> arch/powerpc/kernel/pci-common.c | 5 ++-
> arch/powerpc/kernel/setup-common.c | 4 ++
> arch/powerpc/mm/dma-noncoherent.c | 52 +++++++++++++++++------
> arch/powerpc/platforms/44x/warp.c | 2 +-
> arch/powerpc/platforms/Kconfig.cputype | 6 ++-
> 8 files changed, 60 insertions(+), 99 deletions(-)
>
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index bbfa6a8df4da..33c6017ffce6 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -129,7 +129,7 @@ config PPC
> # Please keep this list sorted alphabetically.
> #
> select ARCH_HAS_DEVMEM_IS_ALLOWED
> - select ARCH_HAS_DMA_SET_COHERENT_MASK
> + select ARCH_HAS_DMA_SET_COHERENT_MASK if !NOT_COHERENT_CACHE
> select ARCH_HAS_ELF_RANDOMIZE
> select ARCH_HAS_FORTIFY_SOURCE
> select ARCH_HAS_GCOV_PROFILE_ALL
> diff --git a/arch/powerpc/include/asm/dma-mapping.h b/arch/powerpc/include/asm/dma-mapping.h
> index f0bf7ac2686c..879c4efba785 100644
> --- a/arch/powerpc/include/asm/dma-mapping.h
> +++ b/arch/powerpc/include/asm/dma-mapping.h
> @@ -19,40 +19,11 @@
> #include <asm/swiotlb.h>
>
> /* Some dma direct funcs must be visible for use in other dma_ops */
> -extern void *__dma_nommu_alloc_coherent(struct device *dev, size_t size,
> - dma_addr_t *dma_handle, gfp_t flag,
> - unsigned long attrs);
> -extern void __dma_nommu_free_coherent(struct device *dev, size_t size,
> - void *vaddr, dma_addr_t dma_handle,
> - unsigned long attrs);
> extern int dma_nommu_mmap_coherent(struct device *dev,
> struct vm_area_struct *vma,
> void *cpu_addr, dma_addr_t handle,
> size_t size, unsigned long attrs);
>
> -#ifdef CONFIG_NOT_COHERENT_CACHE
> -/*
> - * DMA-consistent mapping functions for PowerPCs that don't support
> - * cache snooping. These allocate/free a region of uncached mapped
> - * memory space for use with DMA devices. Alternatively, you could
> - * allocate the space "normally" and use the cache management functions
> - * to ensure it is consistent.
> - */
> -struct device;
> -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);
> -extern unsigned long __dma_get_coherent_pfn(unsigned long cpu_addr);
> -
> -#else /* ! CONFIG_NOT_COHERENT_CACHE */
> -/*
> - * Cache coherent cores.
> - */
> -
> -#define __dma_sync(addr, size, rw) ((void)0)
> -#define __dma_sync_page(pg, off, sz, rw) ((void)0)
> -
> -#endif /* ! CONFIG_NOT_COHERENT_CACHE */
>
> static inline unsigned long device_to_mask(struct device *dev)
> {
> diff --git a/arch/powerpc/kernel/dma.c b/arch/powerpc/kernel/dma.c
> index 2b90a403cdac..b2e88075b2ea 100644
> --- a/arch/powerpc/kernel/dma.c
> +++ b/arch/powerpc/kernel/dma.c
> @@ -36,12 +36,7 @@ static void *dma_nommu_alloc_coherent(struct device *dev, size_t size,
> * we can really use the direct ops
> */
> if (dma_direct_supported(dev, dev->coherent_dma_mask))
> -#ifdef CONFIG_NOT_COHERENT_CACHE
> - return __dma_nommu_alloc_coherent(dev, size, dma_handle,
> - flag, attrs);
> -#else
> return dma_direct_alloc(dev, size, dma_handle, flag, attrs);
> -#endif
>
> /* Ok we can't ... do we have an iommu ? If not, fail */
> iommu = get_iommu_table_base(dev);
> @@ -62,12 +57,7 @@ static void dma_nommu_free_coherent(struct device *dev, size_t size,
>
> /* See comments in dma_nommu_alloc_coherent() */
> if (dma_direct_supported(dev, dev->coherent_dma_mask))
> -#ifdef CONFIG_NOT_COHERENT_CACHE
> - return __dma_nommu_free_coherent(dev, size, vaddr, dma_handle,
> - attrs);
> -#else
> return dma_direct_free(dev, size, vaddr, dma_handle, attrs);
> -#endif
>
> /* Maybe we used an iommu ... */
> iommu = get_iommu_table_base(dev);
> @@ -84,14 +74,8 @@ int dma_nommu_mmap_coherent(struct device *dev, struct vm_area_struct *vma,
> void *cpu_addr, dma_addr_t handle, size_t size,
> unsigned long attrs)
> {
> - unsigned long pfn;
> + unsigned long pfn = page_to_pfn(virt_to_page(cpu_addr));
>
> -#ifdef CONFIG_NOT_COHERENT_CACHE
> - vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
> - pfn = __dma_get_coherent_pfn((unsigned long)cpu_addr);
> -#else
> - pfn = page_to_pfn(virt_to_page(cpu_addr));
> -#endif
> return remap_pfn_range(vma, vma->vm_start,
> pfn + vma->vm_pgoff,
> vma->vm_end - vma->vm_start,
> @@ -108,17 +92,13 @@ static int dma_nommu_map_sg(struct device *dev, struct scatterlist *sgl,
> for_each_sg(sgl, sg, nents, i) {
> sg->dma_address = phys_to_dma(dev, sg_phys(sg));
> sg->dma_length = sg->length;
> -
> - if (attrs & DMA_ATTR_SKIP_CPU_SYNC)
> - continue;
> -
> - __dma_sync_page(sg_page(sg), sg->offset, sg->length, direction);
> }
>
> return nents;
> }
>
> -static u64 dma_nommu_get_required_mask(struct device *dev)
> +/* note: needs to be called arch_get_required_mask for dma-noncoherent.c */
> +u64 arch_get_required_mask(struct device *dev)
> {
> u64 end, mask;
>
> @@ -137,32 +117,9 @@ static inline dma_addr_t dma_nommu_map_page(struct device *dev,
> enum dma_data_direction dir,
> unsigned long attrs)
> {
> - if (!(attrs & DMA_ATTR_SKIP_CPU_SYNC))
> - __dma_sync_page(page, offset, size, dir);
> -
> return phys_to_dma(dev, page_to_phys(page)) + offset;
> }
>
> -#ifdef CONFIG_NOT_COHERENT_CACHE
> -static inline void dma_nommu_sync_sg(struct device *dev,
> - struct scatterlist *sgl, int nents,
> - enum dma_data_direction direction)
> -{
> - struct scatterlist *sg;
> - int i;
> -
> - for_each_sg(sgl, sg, nents, i)
> - __dma_sync_page(sg_page(sg), sg->offset, sg->length, direction);
> -}
> -
> -static inline void dma_nommu_sync_single(struct device *dev,
> - dma_addr_t dma_handle, size_t size,
> - enum dma_data_direction direction)
> -{
> - __dma_sync(bus_to_virt(dma_handle), size, direction);
> -}
> -#endif
> -
> const struct dma_map_ops dma_nommu_ops = {
> .alloc = dma_nommu_alloc_coherent,
> .free = dma_nommu_free_coherent,
> @@ -170,15 +127,10 @@ const struct dma_map_ops dma_nommu_ops = {
> .map_sg = dma_nommu_map_sg,
> .dma_supported = dma_direct_supported,
> .map_page = dma_nommu_map_page,
> - .get_required_mask = dma_nommu_get_required_mask,
> -#ifdef CONFIG_NOT_COHERENT_CACHE
> - .sync_single_for_cpu = dma_nommu_sync_single,
> - .sync_single_for_device = dma_nommu_sync_single,
> - .sync_sg_for_cpu = dma_nommu_sync_sg,
> - .sync_sg_for_device = dma_nommu_sync_sg,
> -#endif
> + .get_required_mask = arch_get_required_mask,
> };
>
> +#ifndef CONFIG_NOT_COHERENT_CACHE
> int dma_set_coherent_mask(struct device *dev, u64 mask)
> {
> if (!dma_supported(dev, mask)) {
> @@ -197,6 +149,7 @@ int dma_set_coherent_mask(struct device *dev, u64 mask)
> return 0;
> }
> EXPORT_SYMBOL(dma_set_coherent_mask);
> +#endif
>
> int dma_set_mask(struct device *dev, u64 dma_mask)
> {
> diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
> index fe9733ffffaa..898ffb636b75 100644
> --- a/arch/powerpc/kernel/pci-common.c
> +++ b/arch/powerpc/kernel/pci-common.c
> @@ -59,8 +59,11 @@ static DECLARE_BITMAP(phb_bitmap, MAX_PHBS);
> resource_size_t isa_mem_base;
> EXPORT_SYMBOL(isa_mem_base);
>
> -
> +#ifdef CONFIG_NOT_COHERENT_CACHE
> +static const struct dma_map_ops *pci_dma_ops = &dma_noncoherent_ops;
> +#else
> static const struct dma_map_ops *pci_dma_ops = &dma_nommu_ops;
> +#endif
>
> void set_pci_dma_ops(const struct dma_map_ops *dma_ops)
> {
> diff --git a/arch/powerpc/kernel/setup-common.c b/arch/powerpc/kernel/setup-common.c
> index 40b44bb53a4e..2488826fa543 100644
> --- a/arch/powerpc/kernel/setup-common.c
> +++ b/arch/powerpc/kernel/setup-common.c
> @@ -792,7 +792,11 @@ void arch_setup_pdev_archdata(struct platform_device *pdev)
> {
> pdev->archdata.dma_mask = DMA_BIT_MASK(32);
> pdev->dev.dma_mask = &pdev->archdata.dma_mask;
> +#ifdef CONFIG_NOT_COHERENT_CACHE
> + set_dma_ops(&pdev->dev, &dma_noncoherent_ops);
> +#else
> set_dma_ops(&pdev->dev, &dma_nommu_ops);
> +#endif
> }
>
> static __init void print_system_info(void)
> diff --git a/arch/powerpc/mm/dma-noncoherent.c b/arch/powerpc/mm/dma-noncoherent.c
> index cfc48a253707..1ceea32c0112 100644
> --- a/arch/powerpc/mm/dma-noncoherent.c
> +++ b/arch/powerpc/mm/dma-noncoherent.c
> @@ -30,6 +30,7 @@
> #include <linux/types.h>
> #include <linux/highmem.h>
> #include <linux/dma-direct.h>
> +#include <linux/dma-noncoherent.h>
> #include <linux/export.h>
>
> #include <asm/tlbflush.h>
> @@ -151,8 +152,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_nommu_alloc_coherent(struct device *dev, size_t size,
> - dma_addr_t *dma_handle, gfp_t gfp, unsigned long attrs)
> +void *arch_dma_alloc(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;
> @@ -253,7 +254,7 @@ void *__dma_nommu_alloc_coherent(struct device *dev, size_t size,
> /*
> * free a page as defined by the above mapping.
> */
> -void __dma_nommu_free_coherent(struct device *dev, size_t size, void *vaddr,
> +void arch_dma_free(struct device *dev, size_t size, void *vaddr,
> dma_addr_t dma_handle, unsigned long attrs)
> {
> struct ppc_vm_region *c;
> @@ -313,7 +314,7 @@ void __dma_nommu_free_coherent(struct device *dev, size_t size, void *vaddr,
> /*
> * make an area consistent.
> */
> -void __dma_sync(void *vaddr, size_t size, int direction)
> +static void __dma_sync(void *vaddr, size_t size, int direction)
> {
> unsigned long start = (unsigned long)vaddr;
> unsigned long end = start + size;
> @@ -339,7 +340,6 @@ void __dma_sync(void *vaddr, size_t size, int direction)
> break;
> }
> }
> -EXPORT_SYMBOL(__dma_sync);
>
> #ifdef CONFIG_HIGHMEM
> /*
> @@ -382,23 +382,36 @@ static inline void __dma_sync_page_highmem(struct page *page,
> * __dma_sync_page makes memory consistent. identical to __dma_sync, but
> * takes a struct page instead of a virtual address
> */
> -void __dma_sync_page(struct page *page, unsigned long offset,
> - size_t size, int direction)
> +static void __dma_sync_page(phys_addr_t paddr, size_t size, int dir)
> {
> + struct page *page = pfn_to_page(paddr >> PAGE_SHIFT);
> + unsigned offset = paddr & ~PAGE_MASK;
> +
> #ifdef CONFIG_HIGHMEM
> - __dma_sync_page_highmem(page, offset, size, direction);
> + __dma_sync_page_highmem(page, offset, size, dir);
> #else
> unsigned long start = (unsigned long)page_address(page) + offset;
> - __dma_sync((void *)start, size, direction);
> + __dma_sync((void *)start, size, dir);
> #endif
> }
> -EXPORT_SYMBOL(__dma_sync_page);
> +
> +void arch_sync_dma_for_device(struct device *dev, phys_addr_t paddr,
> + size_t size, enum dma_data_direction dir)
> +{
> + __dma_sync_page(paddr, size, dir);
> +}
> +
> +void arch_sync_dma_for_cpu(struct device *dev, phys_addr_t paddr,
> + size_t size, enum dma_data_direction dir)
> +{
> + __dma_sync_page(paddr, size, dir);
> +}
>
> /*
> - * Return the PFN for a given cpu virtual address returned by
> - * __dma_nommu_alloc_coherent. This is used by dma_mmap_coherent()
> + * Return the PFN for a given cpu virtual address returned by __arch_dma_alloc.
> + * This is used by dma_mmap_coherent()
> */
> -unsigned long __dma_get_coherent_pfn(unsigned long cpu_addr)
> +static unsigned long __dma_get_coherent_pfn(unsigned long cpu_addr)
> {
> /* This should always be populated, so we don't test every
> * level. If that fails, we'll have a nice crash which
> @@ -413,3 +426,16 @@ unsigned long __dma_get_coherent_pfn(unsigned long cpu_addr)
> return 0;
> return pte_pfn(*ptep);
> }
> +
> +int arch_dma_mmap(struct device *dev, struct vm_area_struct *vma,
> + void *cpu_addr, dma_addr_t handle, size_t size,
> + unsigned long attrs)
> +{
> + unsigned long pfn = __dma_get_coherent_pfn((unsigned long)cpu_addr);
> +
> + vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
> + return remap_pfn_range(vma, vma->vm_start,
> + pfn + vma->vm_pgoff,
> + vma->vm_end - vma->vm_start,
> + vma->vm_page_prot);
> +}
> diff --git a/arch/powerpc/platforms/44x/warp.c b/arch/powerpc/platforms/44x/warp.c
> index 7e4f8ca19ce8..c0e6fb270d59 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_nommu_alloc_coherent */
> + /* For arch_dma_alloc */
> ISA_DMA_THRESHOLD = ~0L;
>
> return 1;
> diff --git a/arch/powerpc/platforms/Kconfig.cputype b/arch/powerpc/platforms/Kconfig.cputype
> index a2578bf8d560..9d83f54ccf11 100644
> --- a/arch/powerpc/platforms/Kconfig.cputype
> +++ b/arch/powerpc/platforms/Kconfig.cputype
> @@ -387,7 +387,11 @@ config NOT_COHERENT_CACHE
> depends on 4xx || PPC_8xx || E200 || PPC_MPC512x || GAMECUBE_COMMON
> default n if PPC_47x
> default y
> - select NEED_DMA_MAP_STATE
> + select ARCH_HAS_SYNC_DMA_FOR_DEVICE
> + select ARCH_HAS_SYNC_DMA_FOR_CPU
> + select DMA_NONCOHERENT_GET_REQUIRED
> + select DMA_NONCOHERENT_MMAP
> + select DMA_NONCOHERENT_OPS
>
> config CHECK_CACHE_COHERENCY
> bool
^ permalink raw reply
* Re: [PATCH 20/20] powerpc/dma: remove dma_nommu_mmap_coherent
From: Benjamin Herrenschmidt @ 2018-08-09 1:05 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-21-hch@lst.de>
On Mon, 2018-07-30 at 18:38 +0200, Christoph Hellwig wrote:
> The remaining implementation for coherent caches is functionally
> identical to the default provided in common code.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> ---
> arch/powerpc/include/asm/dma-mapping.h | 7 -------
> arch/powerpc/kernel/dma-iommu.c | 1 -
> arch/powerpc/kernel/dma.c | 13 -------------
> arch/powerpc/platforms/pseries/vio.c | 1 -
> 4 files changed, 22 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/dma-mapping.h b/arch/powerpc/include/asm/dma-mapping.h
> index 879c4efba785..e62e23aa3714 100644
> --- a/arch/powerpc/include/asm/dma-mapping.h
> +++ b/arch/powerpc/include/asm/dma-mapping.h
> @@ -18,13 +18,6 @@
> #include <asm/io.h>
> #include <asm/swiotlb.h>
>
> -/* Some dma direct funcs must be visible for use in other dma_ops */
> -extern int dma_nommu_mmap_coherent(struct device *dev,
> - struct vm_area_struct *vma,
> - void *cpu_addr, dma_addr_t handle,
> - size_t size, unsigned long attrs);
> -
> -
> static inline unsigned long device_to_mask(struct device *dev)
> {
> if (dev->dma_mask && *dev->dma_mask)
> diff --git a/arch/powerpc/kernel/dma-iommu.c b/arch/powerpc/kernel/dma-iommu.c
> index f9fe2080ceb9..bf5234e1f71b 100644
> --- a/arch/powerpc/kernel/dma-iommu.c
> +++ b/arch/powerpc/kernel/dma-iommu.c
> @@ -114,7 +114,6 @@ int dma_iommu_mapping_error(struct device *dev, dma_addr_t dma_addr)
> struct dma_map_ops dma_iommu_ops = {
> .alloc = dma_iommu_alloc_coherent,
> .free = dma_iommu_free_coherent,
> - .mmap = dma_nommu_mmap_coherent,
> .map_sg = dma_iommu_map_sg,
> .unmap_sg = dma_iommu_unmap_sg,
> .dma_supported = dma_iommu_dma_supported,
> diff --git a/arch/powerpc/kernel/dma.c b/arch/powerpc/kernel/dma.c
> index 08b12cbd7abf..5b71c9d1b8cc 100644
> --- a/arch/powerpc/kernel/dma.c
> +++ b/arch/powerpc/kernel/dma.c
> @@ -70,18 +70,6 @@ static void dma_nommu_free_coherent(struct device *dev, size_t size,
> iommu_free_coherent(iommu, size, vaddr, dma_handle);
> }
>
> -int dma_nommu_mmap_coherent(struct device *dev, struct vm_area_struct *vma,
> - void *cpu_addr, dma_addr_t handle, size_t size,
> - unsigned long attrs)
> -{
> - unsigned long pfn = page_to_pfn(virt_to_page(cpu_addr));
> -
> - return remap_pfn_range(vma, vma->vm_start,
> - pfn + vma->vm_pgoff,
> - vma->vm_end - vma->vm_start,
> - vma->vm_page_prot);
> -}
> -
> /* note: needs to be called arch_get_required_mask for dma-noncoherent.c */
> u64 arch_get_required_mask(struct device *dev)
> {
> @@ -98,7 +86,6 @@ u64 arch_get_required_mask(struct device *dev)
> const struct dma_map_ops dma_nommu_ops = {
> .alloc = dma_nommu_alloc_coherent,
> .free = dma_nommu_free_coherent,
> - .mmap = dma_nommu_mmap_coherent,
> .map_sg = dma_direct_map_sg,
> .map_page = dma_direct_map_page,
> .get_required_mask = arch_get_required_mask,
> diff --git a/arch/powerpc/platforms/pseries/vio.c b/arch/powerpc/platforms/pseries/vio.c
> index 49e04ec19238..51d564313bd0 100644
> --- a/arch/powerpc/platforms/pseries/vio.c
> +++ b/arch/powerpc/platforms/pseries/vio.c
> @@ -618,7 +618,6 @@ static u64 vio_dma_get_required_mask(struct device *dev)
> static const struct dma_map_ops vio_dma_mapping_ops = {
> .alloc = vio_dma_iommu_alloc_coherent,
> .free = vio_dma_iommu_free_coherent,
> - .mmap = dma_nommu_mmap_coherent,
> .map_sg = vio_dma_iommu_map_sg,
> .unmap_sg = vio_dma_iommu_unmap_sg,
> .map_page = vio_dma_iommu_map_page,
^ permalink raw reply
* Re: [PATCH v7 7/9] powerpc/pseries: Dump the SLB contents on SLB MCE errors.
From: Michael Ellerman @ 2018-08-09 1:05 UTC (permalink / raw)
To: Mahesh J Salgaonkar, linuxppc-dev
Cc: Aneesh Kumar K.V, Aneesh Kumar K.V, Michal Suchanek,
Ananth Narayan, Nicholas Piggin, Laurent Dufour
In-Reply-To: <153365145460.14256.11932687379471923123.stgit@jupiter.in.ibm.com>
Mahesh J Salgaonkar <mahesh@linux.vnet.ibm.com> writes:
> diff --git a/arch/powerpc/include/asm/paca.h b/arch/powerpc/include/asm/paca.h
> index 7f22929ce915..233d25ff6f64 100644
> --- a/arch/powerpc/include/asm/paca.h
> +++ b/arch/powerpc/include/asm/paca.h
> @@ -254,6 +254,10 @@ struct paca_struct {
> #endif
> #ifdef CONFIG_PPC_PSERIES
> u8 *mce_data_buf; /* buffer to hold per cpu rtas errlog */
> +
> + /* Capture SLB related old contents in MCE handler. */
> + struct slb_entry *mce_faulty_slbs;
> + u16 slb_save_cache_ptr;
> #endif /* CONFIG_PPC_PSERIES */
^^^^^^^^^^^^^^^^^
> diff --git a/arch/powerpc/mm/slb.c b/arch/powerpc/mm/slb.c
> index e89f675f1b5e..16a53689ffd4 100644
> --- a/arch/powerpc/mm/slb.c
> +++ b/arch/powerpc/mm/slb.c
> @@ -151,6 +151,79 @@ void slb_flush_and_rebolt_realmode(void)
> get_paca()->slb_cache_ptr = 0;
> }
>
> +void slb_save_contents(struct slb_entry *slb_ptr)
> +{
> + int i;
> + unsigned long e, v;
> +
> + /* Save slb_cache_ptr value. */
> + get_paca()->slb_save_cache_ptr = get_paca()->slb_cache_ptr;
This isn't inside CONFIG_PPC_PSERIES which breaks lots of configs, eg
powernv.
arch/powerpc/mm/slb.c:160:12: error: 'struct paca_struct' has no member named 'slb_save_cache_ptr'
arch/powerpc/mm/slb.c:218:27: error: 'struct paca_struct' has no member named 'slb_save_cache_ptr'
arch/powerpc/mm/slb.c:216:49: error: 'struct paca_struct' has no member named 'slb_save_cache_ptr'
http://kisskb.ozlabs.ibm.com/kisskb/head/219f20e490add009194d94fdeb480da2e385f1c6/
cheers
^ permalink raw reply
* Re: [PATCH v7 8/9] powerpc/mce: Add sysctl control for recovery action on MCE.
From: Nicholas Piggin @ 2018-08-09 1:43 UTC (permalink / raw)
To: Michael Ellerman
Cc: Mahesh J Salgaonkar, linuxppc-dev, Aneesh Kumar K.V,
Michal Suchanek, Ananth Narayan, Laurent Dufour
In-Reply-To: <87o9ecaovz.fsf@concordia.ellerman.id.au>
On Thu, 09 Aug 2018 00:56:00 +1000
Michael Ellerman <mpe@ellerman.id.au> 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.
Yeah it's a lot of knobs that administrators can hardly be expected to
tune. Hypervisor or firmware should really eventually make the MCE
unrecoverable if we aren't making progress.
That said, x86 has a bunch of options, and for debugging a rare crash
or specialised installations it might be useful. But we should follow
the normal format, /proc/sys/kernel/panic_on_mce.
Thanks,
Nick
^ permalink raw reply
* Re: [PATCH 17/20] powerpc/dma-swiotlb: use generic swiotlb_dma_ops
From: Benjamin Herrenschmidt @ 2018-08-09 1:57 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: <556a4302d13b1caafb223112c4de43d959146215.camel@kernel.crashing.org>
On Thu, 2018-08-09 at 10:54 +1000, Benjamin Herrenschmidt wrote:
> On Mon, 2018-07-30 at 18:38 +0200, Christoph Hellwig wrote:
> > These are identical to the arch specific ones, so remove them.
> >
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
>
> Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Note: We will still need to implement some custom variant of this
for our secure VMs ...
Basically we'll need to use the existing bounce bufferring as-is but
the condition will be different, it won't be whether the address is
below a certain limit, it will be *always*.
Cheers,
Ben.
> > ---
> > arch/powerpc/include/asm/dma-direct.h | 4 ++++
> > arch/powerpc/include/asm/swiotlb.h | 2 --
> > arch/powerpc/kernel/dma-swiotlb.c | 28 ++-------------------------
> > arch/powerpc/sysdev/fsl_pci.c | 2 +-
> > 4 files changed, 7 insertions(+), 29 deletions(-)
> >
> > diff --git a/arch/powerpc/include/asm/dma-direct.h b/arch/powerpc/include/asm/dma-direct.h
> > index 0fba19445ae8..657f84ddb20d 100644
> > --- a/arch/powerpc/include/asm/dma-direct.h
> > +++ b/arch/powerpc/include/asm/dma-direct.h
> > @@ -30,4 +30,8 @@ static inline phys_addr_t __dma_to_phys(struct device *dev, dma_addr_t daddr)
> > return daddr - PCI_DRAM_OFFSET;
> > return daddr - dev->archdata.dma_offset;
> > }
> > +
> > +u64 swiotlb_powerpc_get_required(struct device *dev);
> > +#define swiotlb_get_required_mask swiotlb_powerpc_get_required
> > +
> > #endif /* ASM_POWERPC_DMA_DIRECT_H */
> > diff --git a/arch/powerpc/include/asm/swiotlb.h b/arch/powerpc/include/asm/swiotlb.h
> > index f65ecf57b66c..1d8c1da26ab3 100644
> > --- a/arch/powerpc/include/asm/swiotlb.h
> > +++ b/arch/powerpc/include/asm/swiotlb.h
> > @@ -13,8 +13,6 @@
> >
> > #include <linux/swiotlb.h>
> >
> > -extern const struct dma_map_ops powerpc_swiotlb_dma_ops;
> > -
> > extern unsigned int ppc_swiotlb_enable;
> > int __init swiotlb_setup_bus_notifier(void);
> >
> > diff --git a/arch/powerpc/kernel/dma-swiotlb.c b/arch/powerpc/kernel/dma-swiotlb.c
> > index 25986fcd1e5e..0c269de61f39 100644
> > --- a/arch/powerpc/kernel/dma-swiotlb.c
> > +++ b/arch/powerpc/kernel/dma-swiotlb.c
> > @@ -24,7 +24,7 @@
> >
> > unsigned int ppc_swiotlb_enable;
> >
> > -static u64 swiotlb_powerpc_get_required(struct device *dev)
> > +u64 swiotlb_powerpc_get_required(struct device *dev)
> > {
> > u64 end, mask, max_direct_dma_addr = dev->archdata.max_direct_dma_addr;
> >
> > @@ -38,30 +38,6 @@ static u64 swiotlb_powerpc_get_required(struct device *dev)
> > return mask;
> > }
> >
> > -/*
> > - * At the moment, all platforms that use this code only require
> > - * swiotlb to be used if we're operating on HIGHMEM. Since
> > - * we don't ever call anything other than map_sg, unmap_sg,
> > - * map_page, and unmap_page on highmem, use normal dma_ops
> > - * for everything else.
> > - */
> > -const struct dma_map_ops powerpc_swiotlb_dma_ops = {
> > - .alloc = dma_direct_alloc,
> > - .free = dma_direct_free,
> > - .mmap = dma_nommu_mmap_coherent,
> > - .map_sg = swiotlb_map_sg_attrs,
> > - .unmap_sg = swiotlb_unmap_sg_attrs,
> > - .dma_supported = swiotlb_dma_supported,
> > - .map_page = swiotlb_map_page,
> > - .unmap_page = swiotlb_unmap_page,
> > - .sync_single_for_cpu = swiotlb_sync_single_for_cpu,
> > - .sync_single_for_device = swiotlb_sync_single_for_device,
> > - .sync_sg_for_cpu = swiotlb_sync_sg_for_cpu,
> > - .sync_sg_for_device = swiotlb_sync_sg_for_device,
> > - .mapping_error = swiotlb_dma_mapping_error,
> > - .get_required_mask = swiotlb_powerpc_get_required,
> > -};
> > -
> > void pci_dma_dev_setup_swiotlb(struct pci_dev *pdev)
> > {
> > struct pci_controller *hose;
> > @@ -88,7 +64,7 @@ static int ppc_swiotlb_bus_notify(struct notifier_block *nb,
> >
> > /* May need to bounce if the device can't address all of DRAM */
> > if ((dma_get_mask(dev) + 1) < memblock_end_of_DRAM())
> > - set_dma_ops(dev, &powerpc_swiotlb_dma_ops);
> > + set_dma_ops(dev, &swiotlb_dma_ops);
> >
> > return NOTIFY_DONE;
> > }
> > diff --git a/arch/powerpc/sysdev/fsl_pci.c b/arch/powerpc/sysdev/fsl_pci.c
> > index 918be816b097..daf44bc0108d 100644
> > --- a/arch/powerpc/sysdev/fsl_pci.c
> > +++ b/arch/powerpc/sysdev/fsl_pci.c
> > @@ -118,7 +118,7 @@ static void setup_swiotlb_ops(struct pci_controller *hose)
> > {
> > if (ppc_swiotlb_enable) {
> > hose->controller_ops.dma_dev_setup = pci_dma_dev_setup_swiotlb;
> > - set_pci_dma_ops(&powerpc_swiotlb_dma_ops);
> > + set_pci_dma_ops(&swiotlb_dma_ops);
> > }
> > }
> > #else
^ permalink raw reply
* Re: [RFC 0/4] Virtio uses DMA API for all devices
From: Benjamin Herrenschmidt @ 2018-08-09 2:00 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: <98eb367ce322ad84baa31e3c7beffc4a42be8458.camel@kernel.crashing.org>
On Thu, 2018-08-09 at 08:13 +1000, Benjamin Herrenschmidt wrote:
> > 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...
To elaborate a bit ...
For our secure VMs, we will need bounce buffering for everything
anyway. virtio, emulated PCI, or vfio.
By ensuring that we create an identity mapping in the IOMMU for
the bounce buffering pool, we enable virtio "legacy/direct" to
use the same mapping ops as things using the iommu.
That said, we still need somewhere in arch/powerpc a set of dma
ops which we'll attach to all PCI devices of a secure VM to force
bouncing always, rather than just based on address (which is what
the standard swiotlb ones do)... Unless we can tweak the swiotlb
"threshold" for example by using an empty mask.
We'll need the same set of DMA ops for VIO devices too, not just PCI.
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH] lib/test_hexdump: fix failure on big endian cpu
From: rashmica @ 2018-08-09 2:04 UTC (permalink / raw)
To: Michael Ellerman, Christophe Leroy, Andy Shevchenko,
Andrew Morton, Linus Torvalds
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <87mutxb9qy.fsf@concordia.ellerman.id.au>
On 08/08/18 17:25, Michael Ellerman wrote:
> Christophe Leroy <christophe.leroy@c-s.fr> writes:
>> diff --git a/lib/test_hexdump.c b/lib/test_hexdump.c
>> index 3f415d8101f3..626f580b4ff7 100644
>> --- a/lib/test_hexdump.c
>> +++ b/lib/test_hexdump.c
>> @@ -32,16 +32,33 @@ static const char * const test_data_2_le[] __initconst = {
>> "d14c", "9919", "b143", "0caf",
>> };
>>
>> +static const char * const test_data_2_be[] __initconst = {
>> + "be32", "db7b", "0a18", "93b2",
>> + "70ba", "c424", "7d83", "349b",
>> + "a69c", "31ad", "9c0f", "ace9",
>> + "4cd1", "1999", "43b1", "af0c",
>> +};
>> +
>> static const char * const test_data_4_le[] __initconst = {
>> "7bdb32be", "b293180a", "24c4ba70", "9b34837d",
>> "ad319ca6", "e9ac0f9c", "9919d14c", "0cafb143",
>> };
>>
>> +static const char * const test_data_4_be[] __initconst = {
>> + "be32db7b", "0a1893b2", "70bac424", "7d83349b",
>> + "a69c31ad", "9c0face9", "4cd11999", "43b1af0c",
>> +};
>> +
> Is there a reason we can't do it all at compile time?
mpe I sent a patch doing that awhile ago and you obviously didn't like
it because you never merged it :P
http://patchwork.ozlabs.org/patch/620405/ I prefer this version because
of the IS_ENABLED
> eg:
>
> static const char * const test_data_4[] __initconst = {
> #ifdef CONFIG_CPU_LITTLE_ENDIAN
> "7bdb32be", "b293180a", "24c4ba70", "9b34837d",
> "ad319ca6", "e9ac0f9c", "9919d14c", "0cafb143",
> #else
> "be32db7b", "0a1893b2", "70bac424", "7d83349b",
> "a69c31ad", "9c0face9", "4cd11999", "43b1af0c",
> #endif
> };
>
>
> cheers
^ permalink raw reply
* RE: [RFC 5/5] powerpc/fsl: Add supported-irq-ranges for P2020
From: Bharat Bhushan @ 2018-08-09 3:28 UTC (permalink / raw)
To: Scott Wood, 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: <bef421feea16da2fdab333e63a6828741bd59055.camel@buserror.net>
DQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogU2NvdHQgV29vZCBbbWFp
bHRvOm9zc0BidXNlcnJvci5uZXRdDQo+IFNlbnQ6IFdlZG5lc2RheSwgQXVndXN0IDgsIDIwMTgg
MTE6MjcgUE0NCj4gVG86IEJoYXJhdCBCaHVzaGFuIDxiaGFyYXQuYmh1c2hhbkBueHAuY29tPjsN
Cj4gYmVuaEBrZXJuZWwuY3Jhc2hpbmcub3JnOyBwYXVsdXNAc2FtYmEub3JnOyBtcGVAZWxsZXJt
YW4uaWQuYXU7DQo+IGdhbGFrQGtlcm5lbC5jcmFzaGluZy5vcmc7IG1hcmsucnV0bGFuZEBhcm0u
Y29tOw0KPiBrc3Rld2FydEBsaW51eGZvdW5kYXRpb24ub3JnOyBncmVna2hAbGludXhmb3VuZGF0
aW9uLm9yZzsNCj4gZGV2aWNldHJlZUB2Z2VyLmtlcm5lbC5vcmc7IGxpbnV4cHBjLWRldkBsaXN0
cy5vemxhYnMub3JnOyBsaW51eC0NCj4ga2VybmVsQHZnZXIua2VybmVsLm9yZw0KPiBDYzogcm9i
aEBrZXJuZWwub3JnOyBrZWVzY29va0BjaHJvbWl1bS5vcmc7IHR5cmVsZEBsaW51eC52bmV0Lmli
bS5jb207DQo+IGpvZUBwZXJjaGVzLmNvbQ0KPiBTdWJqZWN0OiBSZTogW1JGQyA1LzVdIHBvd2Vy
cGMvZnNsOiBBZGQgc3VwcG9ydGVkLWlycS1yYW5nZXMgZm9yIFAyMDIwDQo+IA0KPiBPbiBXZWQs
IDIwMTgtMDgtMDggYXQgMDY6MjggKzAwMDAsIEJoYXJhdCBCaHVzaGFuIHdyb3RlOg0KPiA+ID4g
LS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gPiA+IEZyb206IFNjb3R0IFdvb2QgW21haWx0
bzpvc3NAYnVzZXJyb3IubmV0XQ0KPiA+ID4gU2VudDogV2VkbmVzZGF5LCBBdWd1c3QgOCwgMjAx
OCAxMToyNiBBTQ0KPiA+ID4gVG86IEJoYXJhdCBCaHVzaGFuIDxiaGFyYXQuYmh1c2hhbkBueHAu
Y29tPjsNCj4gPiA+IGJlbmhAa2VybmVsLmNyYXNoaW5nLm9yZzsgcGF1bHVzQHNhbWJhLm9yZzsg
bXBlQGVsbGVybWFuLmlkLmF1Ow0KPiA+ID4gZ2FsYWtAa2VybmVsLmNyYXNoaW5nLm9yZzsgbWFy
ay5ydXRsYW5kQGFybS5jb207DQo+ID4gPiBrc3Rld2FydEBsaW51eGZvdW5kYXRpb24ub3JnOyBn
cmVna2hAbGludXhmb3VuZGF0aW9uLm9yZzsNCj4gPiA+IGRldmljZXRyZWVAdmdlci5rZXJuZWwu
b3JnOyBsaW51eHBwYy1kZXZAbGlzdHMub3psYWJzLm9yZzsgbGludXgtDQo+ID4gPiBrZXJuZWxA
dmdlci5rZXJuZWwub3JnDQo+ID4gPiBDYzogcm9iaEBrZXJuZWwub3JnOyBrZWVzY29va0BjaHJv
bWl1bS5vcmc7DQo+ID4gPiB0eXJlbGRAbGludXgudm5ldC5pYm0uY29tOyBqb2VAcGVyY2hlcy5j
b20NCj4gPiA+IFN1YmplY3Q6IFJlOiBbUkZDIDUvNV0gcG93ZXJwYy9mc2w6IEFkZCBzdXBwb3J0
ZWQtaXJxLXJhbmdlcyBmb3INCj4gPiA+IFAyMDIwDQo+ID4gPg0KPiA+ID4gT24gV2VkLCAyMDE4
LTA4LTA4IGF0IDAzOjQ0ICswMDAwLCBCaGFyYXQgQmh1c2hhbiB3cm90ZToNCj4gPiA+ID4gPiAt
LS0tLU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQ0KPiA+ID4gPiA+IEZyb206IFNjb3R0IFdvb2QgW21h
aWx0bzpvc3NAYnVzZXJyb3IubmV0XQ0KPiA+ID4gPiA+IFNlbnQ6IFdlZG5lc2RheSwgQXVndXN0
IDgsIDIwMTggMjo0NCBBTQ0KPiA+ID4gPiA+IFRvOiBCaGFyYXQgQmh1c2hhbiA8YmhhcmF0LmJo
dXNoYW5AbnhwLmNvbT47DQo+ID4gPiA+ID4gYmVuaEBrZXJuZWwuY3Jhc2hpbmcub3JnOyBwYXVs
dXNAc2FtYmEub3JnOyBtcGVAZWxsZXJtYW4uaWQuYXU7DQo+ID4gPiA+ID4gZ2FsYWtAa2VybmVs
LmNyYXNoaW5nLm9yZzsgbWFyay5ydXRsYW5kQGFybS5jb207DQo+ID4gPiA+ID4ga3N0ZXdhcnRA
bGludXhmb3VuZGF0aW9uLm9yZzsgZ3JlZ2toQGxpbnV4Zm91bmRhdGlvbi5vcmc7DQo+ID4gPiA+
ID4gZGV2aWNldHJlZUB2Z2VyLmtlcm5lbC5vcmc7IGxpbnV4cHBjLWRldkBsaXN0cy5vemxhYnMu
b3JnOw0KPiA+ID4gPiA+IGxpbnV4LSBrZXJuZWxAdmdlci5rZXJuZWwub3JnDQo+ID4gPiA+ID4g
Q2M6IHJvYmhAa2VybmVsLm9yZzsga2Vlc2Nvb2tAY2hyb21pdW0ub3JnOw0KPiA+ID4gPiA+IHR5
cmVsZEBsaW51eC52bmV0LmlibS5jb207IGpvZUBwZXJjaGVzLmNvbQ0KPiA+ID4gPiA+IFN1Ympl
Y3Q6IFJlOiBbUkZDIDUvNV0gcG93ZXJwYy9mc2w6IEFkZCBzdXBwb3J0ZWQtaXJxLXJhbmdlcyBm
b3INCj4gPiA+ID4gPiBQMjAyMA0KPiA+ID4gPiA+DQo+ID4gPiA+ID4gT24gRnJpLCAyMDE4LTA3
LTI3IGF0IDE1OjE4ICswNTMwLCBCaGFyYXQgQmh1c2hhbiB3cm90ZToNCj4gPiA+ID4gPiA+IE1Q
SUMgb24gTlhQIChGcmVlc2NhbGUpIFAyMDIwIHN1cHBvcnRzIGZvbGxvd2luZyBpcnENCj4gPiA+
ID4gPiA+IHJhbmdlczoNCj4gPiA+ID4gPiA+ICAgPiAwIC0gMTEgICAgICAoRXh0ZXJuYWwgaW50
ZXJydXB0KQ0KPiA+ID4gPiA+ID4gICA+IDE2IC0gNzkgICAgIChJbnRlcm5hbCBpbnRlcnJ1cHQp
DQo+ID4gPiA+ID4gPiAgID4gMTc2IC0gMTgzICAgKE1lc3NhZ2luZyBpbnRlcnJ1cHQpDQo+ID4g
PiA+ID4gPiAgID4gMjI0IC0gMjMxICAgKFNoYXJlZCBtZXNzYWdlIHNpZ25hbGVkIGludGVycnVw
dCkNCj4gPiA+ID4gPg0KPiA+ID4gPiA+IFdoeSBkb24ndCB5b3UgY29udmVydCB0byB0aGUgNC1j
ZWxsIGludGVycnVwdCBzcGVjaWZpZXJzIHRoYXQNCj4gPiA+ID4gPiBtYWtlIGRlYWxpbmcgd2l0
aCB0aGVzZSByYW5nZXMgbGVzcyBlcnJvci1wcm9uZT8NCj4gPiA+ID4NCj4gPiA+ID4gT2sgLCB3
aWxsIGRvIGlmIHdlIGFncmVlIHRvIGhhdmUgdGhpcyBzZXJpZXMgYXMgcGVyIGNvbW1lbnQgb24N
Cj4gPiA+ID4gb3RoZXIgcGF0Y2guDQo+ID4gPg0KPiA+ID4gSWYgeW91J3JlIGNvbmNlcm5lZCB3
aXRoIGVycm9ycywgdGhpcyB3b3VsZCBiZSBhIGdvb2QgdGhpbmdzIHRvIGRvDQo+ID4gPiByZWdh
cmRsZXNzLg0KPiA+ID4gIEFjdHVhbGx5LCBpdCBzZWVtcyB0aGF0IHAyMDIwc2ktcG9zdC5kdHNp
IGFscmVhZHkgdXNlcyA0LWNlbGwgaW50ZXJydXB0cy4NCj4gPiA+DQo+ID4gPiBXaGF0IGlzIG1v
dGl2YXRpbmcgdGhpcyBwYXRjaHNldD8gIElzIHRoZXJlIHNvbWV0aGluZyB3cm9uZyBpbiB0aGUN
Cj4gPiA+IGV4aXN0aW5nIGR0cyBmaWxlcz8NCj4gPg0KPiA+IFRoZXJlIGlzIG5vIGVycm9yIGlu
IGRldmljZSB0cmVlLiBNYWluIG1vdGl2YXRpb24gaXMgdG8gaW1wcm92ZSBjb2RlDQo+ID4gZm9y
IGZvbGxvd2luZyByZWFzb25zOg0KPiA+ICAgLSBXaGlsZSBjb2RlIHN0dWR5IGl0IHdhcyBmb3Vu
ZCB0aGF0IGlmIGEgcmVzZXJ2ZWQgaXJxLW51bWJlciB1c2VkDQo+ID4gdGhlbiB0aGVyZSBhcmUg
bm8gY2hlY2sgaW4gZHJpdmVyLiBpcnEgd2lsbCBiZSBjb25maWd1cmVkIGFzIGNvcnJlY3QNCj4g
PiBhbmQgaW50ZXJydXB0IHdpbGwgbmV2ZXIgZmlyZS4NCj4gDQo+IEFnYWluLCBhIHdyb25nIGlu
dGVycnVwdCBudW1iZXIgd29uJ3QgZmlyZSwgd2hldGhlciBhbiBpbnRlcnJ1cHQgYnkgdGhhdA0K
PiBudW1iZXIgZXhpc3RzIG9yIG5vdC4gIEkgd291bGRuJ3QgbWluZCBhIHNhbml0eSBjaGVjayBp
biB0aGUgZHJpdmVyIGlmIHRoZQ0KPiBwcm9ncmFtbWluZyBtb2RlbCBtYWRlIGl0IHByb3Blcmx5
IGRpc2NvdmVyYWJsZSwgYnV0IEkgZG9uJ3QgdGhpbmsgaXQncw0KPiB3b3J0aCBtZXNzaW5nIHdp
dGggZGV2aWNlIHRyZWVzIGp1c3QgZm9yIHRoaXMgKGFuZCBldmVuIGxlc3Mgc28gZ2l2ZW4gdGhh
dA0KPiB0aGVyZSBkb24ndCBzZWVtIHRvIGJlIG5ldyBjaGlwcyBjb21pbmcgb3V0IHRoYXQgdGhp
cyB3b3VsZCBiZSByZWxldmFudA0KPiBmb3IpLg0KDQpGYWlyIGVub3VnaCwgd2UgY2FuIHVzZSBN
UElDIHZlcnNpb24gdG8gZGVmaW5lIHN1cHBvcnRlZCBpbnRlcnJ1cHRzIHJhbmdlcy4gV2lsbCB0
aGF0IGJlIGFjY2VwdGFibGUuDQoNClRoYW5rcw0KLUJoYXJhdA0KDQo+IA0KPiA+ID4gPiBPbmUg
b3RoZXIgY29uZnVzaW5nIG9ic2VydmF0aW9uIEkgaGF2ZSBpcyB0aGF0ICJpcnFfY291bnQiIGZy
b20NCj4gPiA+ID4gcGxhdGZvcm0gY29kZSBpcyBnaXZlbiBwcmVjZWRlbmNlIG92ZXIgImxhc3Qt
aW50ZXJydXB0LXNvdXJjZSIgaW4NCj4gPiA+ID4gZGV2aWNlLQ0KPiA+ID4NCj4gPiA+IHRyZWUu
DQo+ID4gPiA+IFNob3VsZCBub3QgZGV2aWNlLXRyZWUgc2hvdWxkIGhhdmUgcHJlY2VkZW5jZSBv
dGhlcndpc2UgdGhlcmUgaXMNCj4gPiA+ID4gbm8gcG9pbnQgdXNpbmcgIiBsYXN0LWludGVycnVw
dC1zb3VyY2UiIGlmIHBsYXRmb3JtIGNvZGUgcGFzc2VzDQo+ID4gPiA+ICJpcnFfY291bnQiIGlu
IG1waWNfYWxsb2MoKS4NCj4gPiA+DQo+ID4gPiBNYXliZSwgdGhvdWdoIEkgZG9uJ3QgdGhpbmsg
aXQgbWF0dGVycyBtdWNoIGdpdmVuIHRoYXQNCj4gPiA+IGxhc3QtaW50ZXJydXB0LSBzb3VyY2Ug
d2FzIG9ubHkgYWRkZWQgdG8gYXZvaWQgaGF2aW5nIHRvIHBhc3MNCj4gPiA+IGlycV9jb3VudCBp
biBwbGF0Zm9ybSBjb2RlLg0KPiA+DQo+ID4gVGhhbmtzIGZvciBjbGFyaWZ5aW5nOw0KPiA+DQo+
ID4gTXkgdW5kZXJzdGFuZGluZyB3YXMgdGhhdCAibGFzdC1pbnRlcnJ1cHQtc291cmNlIiBhZGRl
ZCB0byBlbnN1cmUgdGhhdA0KPiA+IHdlIGNhbiBvdmVyLXJpZGUgdmFsdWUgcGFzc2VkIGZyb20g
cGxhdGZvcm0gY29kZS4gSW4gdGhhdCBjYXNlIHdlIGRvDQo+ID4gbm90IG5lZWQgdG8gY2hhbmdl
IGNvZGUgYW5kIGNhbiBjb250cm9sIGZyb20gZGV2aWNlIHRyZWUuDQo+IA0KPiBUaGUgY2hhbmdl
bG9nIHNheXMsICJUbyBhdm9pZCBuZWVkaW5nIHRvIHdyaXRlIGN1c3RvbSBib2FyZC1zcGVjaWZp
YyBjb2RlDQo+IHRvIGRldGVjdCB0aGF0IHNjZW5hcmlvLCBhbGxvdyBpdCB0byBiZSBlYXNpbHkg
b3ZlcnJpZGRlbiBpbiB0aGUgZGV2aWNlLXRyZWUsIg0KPiB3aGVyZSAiaXQiIG1lYW5zIHRoZSB2
YWx1ZSBwcm92aWRlZCBieSBoYXJkd2FyZS4gIFRoZSBnb2FsIHdhcyB0byBwYXNzIGluDQo+IDI1
NiB3aXRob3V0IGJvYXJkIGNvZGUgaW4gdGhlIGtlcm5lbCwgbm90IHRvIG92ZXJyaWRlIHRoZSAy
NTYuDQo+IA0KPiAtU2NvdHQNCg0K
^ permalink raw reply
* Re: [RFC PATCH kernel 0/5] powerpc/P9/vfio: Pass through NVIDIA Tesla V100
From: Alexey Kardashevskiy @ 2018-08-09 4:21 UTC (permalink / raw)
To: Alex Williamson
Cc: Benjamin Herrenschmidt, linuxppc-dev, David Gibson, kvm-ppc,
Ram Pai, kvm, Alistair Popple
In-Reply-To: <dc0e5c0b-c6de-df1c-176f-7f64cbd6a7ba@ozlabs.ru>
On 08/08/2018 18:39, Alexey Kardashevskiy wrote:
>
>
> On 02/08/2018 02:16, Alex Williamson wrote:
>> On Wed, 1 Aug 2018 18:37:35 +1000
>> Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>>
>>> On 01/08/2018 00:29, Alex Williamson wrote:
>>>> On Tue, 31 Jul 2018 14:03:35 +1000
>>>> Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>>>>
>>>>> On 31/07/2018 02:29, Alex Williamson wrote:
>>>>>> On Mon, 30 Jul 2018 18:58:49 +1000
>>>>>> Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>>>>>>> After some local discussions, it was pointed out that force disabling
>>>>>>> nvlinks won't bring us much as for an nvlink to work, both sides need to
>>>>>>> enable it so malicious guests cannot penetrate good ones (or a host)
>>>>>>> unless a good guest enabled the link but won't happen with a well
>>>>>>> behaving guest. And if two guests became malicious, then can still only
>>>>>>> harm each other, and so can they via other ways such network. This is
>>>>>>> different from PCIe as once PCIe link is unavoidably enabled, a well
>>>>>>> behaving device cannot firewall itself from peers as it is up to the
>>>>>>> upstream bridge(s) now to decide the routing; with nvlink2, a GPU still
>>>>>>> has means to protect itself, just like a guest can run "firewalld" for
>>>>>>> network.
>>>>>>>
>>>>>>> Although it would be a nice feature to have an extra barrier between
>>>>>>> GPUs, is inability to block the links in hypervisor still a blocker for
>>>>>>> V100 pass through?
>>>>>>
>>>>>> How is the NVLink configured by the guest, is it 'on'/'off' or are
>>>>>> specific routes configured?
>>>>>
>>>>> The GPU-GPU links need not to be blocked and need to be enabled
>>>>> (==trained) by a driver in the guest. There are no routes between GPUs
>>>>> in NVLink fabric, these are direct links, it is just a switch on each
>>>>> side, both switches need to be on for a link to work.
>>>>
>>>> Ok, but there is at least the possibility of multiple direct links per
>>>> GPU, the very first diagram I find of NVlink shows 8 interconnected
>>>> GPUs:
>>>>
>>>> https://www.nvidia.com/en-us/data-center/nvlink/
>>>
>>> Out design is like the left part of the picture but it is just a detail.
>>
>> Unless we can specifically identify a direct link vs a mesh link, we
>> shouldn't be making assumptions about the degree of interconnect.
>>
>>>> So if each switch enables one direct, point to point link, how does the
>>>> guest know which links to open for which peer device?
>>>
>>> It uses PCI config space on GPUs to discover the topology.
>>
>> So do we need to virtualize this config space if we're going to
>> virtualize the topology?
>>
>>>> And of course
>>>> since we can't see the spec, a security audit is at best hearsay :-\
>>>
>>> Yup, the exact discovery protocol is hidden.
>>
>> It could be reverse engineered...
>>
>>>>> The GPU-CPU links - the GPU bit is the same switch, the CPU NVlink state
>>>>> is controlled via the emulated PCI bridges which I pass through together
>>>>> with the GPU.
>>>>
>>>> So there's a special emulated switch, is that how the guest knows which
>>>> GPUs it can enable NVLinks to?
>>>
>>> Since it only has PCI config space (there is nothing relevant in the
>>> device tree at all), I assume (double checking with the NVIDIA folks
>>> now) the guest driver enables them all, tests which pair works and
>>> disables the ones which do not. This gives a malicious guest a tiny
>>> window of opportunity to break into a good guest. Hm :-/
>>
>> Let's not minimize that window, that seems like a prime candidate for
>> an exploit.
>>
>>>>>> If the former, then isn't a non-malicious
>>>>>> guest still susceptible to a malicious guest?
>>>>>
>>>>> A non-malicious guest needs to turn its switch on for a link to a GPU
>>>>> which belongs to a malicious guest.
>>>>
>>>> Actual security, or obfuscation, will we ever know...
>>>>>>> If the latter, how is
>>>>>> routing configured by the guest given that the guest view of the
>>>>>> topology doesn't match physical hardware? Are these routes
>>>>>> deconfigured by device reset? Are they part of the save/restore
>>>>>> state? Thanks,
>>>>
>>>> Still curious what happens to these routes on reset. Can a later user
>>>> of a GPU inherit a device where the links are already enabled? Thanks,
>>>
>>> I am told that the GPU reset disables links. As a side effect, we get an
>>> HMI (a hardware fault which reset the host machine) when trying
>>> accessing the GPU RAM which indicates that the link is down as the
>>> memory is only accessible via the nvlink. We have special fencing code
>>> in our host firmware (skiboot) to fence this memory on PCI reset so
>>> reading from it returns zeroes instead of HMIs.
>>
>> What sort of reset is required for this? Typically we rely on
>> secondary bus reset for GPUs, but it would be a problem if GPUs were to
>> start implementing FLR and nobody had a spec to learn that FLR maybe
>> didn't disable the link. The better approach to me still seems to be
>> virtualizing these NVLink config registers to an extent that the user
>> can only enabling links where they have ownership of both ends of the
>> connection. Thanks,
>
>
> I re-read what I wrote and I owe some explanation.
>
> The link state can be:
> - disabled (or masked),
> - enabled (or not-disabled? unmasked?),
> - trained (configured).
>
> At the moment no reset disables links, on sec bus reset they are
> unconfigured and go to the initial enabled-and-not-trained state which
> is the default config. The NVIDIA driver in the guest trains links to do
> the topology discovery. We can disable links and this disabled status
> remains until sec bus reset and there is no way to re-enable links other
> than sec bus reset. This is what I get from NVIDIA. FLR should not be
> able to change a thing here.
btw using this masking mechanism does not involve any virtualizing -
these are MMIO registers which a powernv platform reset hook will write
to in order to stay in sync with already configured IOMMU groups and
that's all, the guest will still be able to access them with no
filtering on the way, it just won't do anything. Or this is still called
virtualizing?
--
Alexey
^ permalink raw reply
* Re: [PATCH kernel RFC 0/3] powerpc/pseries/iommu: GPU coherent memory pass through
From: Alexey Kardashevskiy @ 2018-08-09 4:41 UTC (permalink / raw)
To: linuxppc-dev
Cc: David Gibson, kvm-ppc, Benjamin Herrenschmidt, Michael Ellerman,
Paul Mackerras, Russell Currey
In-Reply-To: <20180725095032.2196-1-aik@ozlabs.ru>
On 25/07/2018 19:50, Alexey Kardashevskiy wrote:
> I am trying to pass through a 3D controller:
> [0302]: NVIDIA Corporation GV100GL [Tesla V100 SXM2] [10de:1db1] (rev a1)
>
> which has a quite unique feature as coherent memory directly accessible
> from a POWER9 CPU via an NVLink2 transport.
>
> So in addition to passing a PCI device + accompanying NPU devices,
> we will also be passing the host physical address range as it is done
> on the bare metal system.
>
> The memory on the host is presented as:
>
> ===
> [aik@yc02goos ~]$ lsprop /proc/device-tree/memory@42000000000
> ibm,chip-id 000000fe (254)
> device_type "memory"
> compatible "ibm,coherent-device-memory"
> reg 00000420 00000000 00000020 00000000
> linux,usable-memory
> 00000420 00000000 00000000 00000000
> phandle 00000726 (1830)
> name "memory"
> ibm,associativity
> 00000004 000000fe 000000fe 000000fe 000000fe
> ===
>
> and the host does not touch it as the second 64bit value of
> "linux,usable-memory" - the size - is null. Later on the NVIDIA driver
> trains the NVLink2 and probes this memory and this is how it becomes
> onlined.
>
> In the virtual environment I am planning on doing the same thing,
> however there is a difference in 64bit DMA handling. The powernv
> platform uses a PHB3 bypass mode and that just works but
> the pseries platform uses DDW RTAS API to achieve the same
> result and the problem with this is that we need a huge DMA
> window to start from zero (because this GPU supports less than
> 50bits for DMA address space) and cover not just present memory
> but also this new coherent memory.
>
>
> This is based on sha1
> d72e90f3 Linus Torvalds "Linux 4.18-rc6".
>
> Please comment. Thanks.
Ping?
>
>
>
> Alexey Kardashevskiy (3):
> powerpc/pseries/iommu: Allow dynamic window to start from zero
> powerpc/pseries/iommu: Force default DMA window removal
> powerpc/pseries/iommu: Use memory@ nodes in max RAM address
> calculation
>
> arch/powerpc/platforms/pseries/iommu.c | 77 ++++++++++++++++++++++++++++++----
> 1 file changed, 70 insertions(+), 7 deletions(-)
>
--
Alexey
^ permalink raw reply
* [PATCH v6 0/2] powerpc: Detection and scheduler optimization for POWER9 bigcore
From: Gautham R. Shenoy @ 2018-08-09 5:32 UTC (permalink / raw)
To: Srikar Dronamraju, Michael Ellerman, Benjamin Herrenschmidt,
Michael Neuling, Vaidyanathan Srinivasan, Akshay Adiga,
Shilpasri G Bhat, Oliver O'Halloran, Nicholas Piggin,
Murilo Opsfelder Araujo, Anton Blanchard
Cc: linuxppc-dev, linux-kernel, Gautham R. Shenoy
From: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com>
Hi,
This is the fifth iteration of the patchset to add support for
big-core on POWER9. This patch also optimizes the task placement on
such big-core systems.
The previous versions can be found here:
v5: https://lkml.org/lkml/2018/8/6/587
v4: https://lkml.org/lkml/2018/7/24/79
v3: https://lkml.org/lkml/2018/7/6/255
v2: https://lkml.org/lkml/2018/7/3/401
v1: https://lkml.org/lkml/2018/5/11/245
Changes :
v5 --> v6:
- Fixed the code to build without warnings for !CONFIG_SCHED_SMT.
- While checking for shared caches on big-core system, use the
smallcore_sibling_mask to compare with compare with
l2_cache_mask, which will ensure that the CACHE level
sched-domain is created.
- Added benchmark results with hackbench to demonstrate the
benefits of having the CACHE level sched-domain.
v4 --> v5:
- Patch 2 is entirely different: Instead of using CPU_FTR_ASYM_SMT
feature, use the small core siblings at the SMT level
sched-domain. This was suggested by Nicholas Piggin and Michael
Ellerman.
- A more detailed description follows below.
v3 --> v4:
- Build fix for powerpc-g5 : Enable CPU_FTR_ASYM_SMT only on
CONFIG_PPC_POWERNV and CONFIG_PPC_PSERIES.
- Fixed a minor error in the ABI description.
v2 --> v3
- Set sane values in the tg->property, tg->nr_groups inside
parse_thread_groups before returning due to an error.
- Define a helper function to determine whether a CPU device node
is a big-core or not.
- Updated the comments around the functions to describe the
arguments passed to them.
v1 --> v2
- Added comments explaining the "ibm,thread-groups" device tree property.
- Uses cleaner device-tree parsing functions to parse the u32 arrays.
- Adds a sysfs file listing the small-core siblings for every CPU.
- Enables the scheduler optimization by setting the CPU_FTR_ASYM_SMT bit
in the cur_cpu_spec->cpu_features on detecting the presence
of interleaved big-core.
- Handles the corner case where there is only a single thread-group
or when there is a single thread in a thread-group.
Description:
~~~~~~~~~~~~~~~~~~~~
A pair of IBM POWER9 SMT4 cores can be fused together to form a
big-core with 8 SMT threads. This can be discovered via the
"ibm,thread-groups" CPU property in the device tree which will
indicate which group of threads that share the L1 cache, translation
cache and instruction data flow. If there are multiple such group of
threads, then the core is a big-core. Furthermore, on POWER9 the thread-ids of
such a big-core is obtained by interleaving the thread-ids of the
component SMT4 cores.
Eg: Threads in the pair of component SMT4 cores of an interleaved
big-core are numbered {0,2,4,6} and {1,3,5,7} respectively.
-------------------------
| L1 Cache |
----------------------------------
|L2| | | | |
| | 0 | 2 | 4 | 6 |Small Core0
|C | | | | |
Big |a --------------------------
Core |c | | | | |
|h | 1 | 3 | 5 | 7 | Small Core1
|e | | | | |
-----------------------------
| L1 Cache |
--------------------------
On such a big-core system, when multiple tasks are scheduled to run on
the big-core, we get the best performance when the tasks are spread
across the pair of SMT4 cores.
Eg: Suppose there 4 tasks {p1, p2, p3, p4} are run on a big core, then
An Example of Optimal Task placement:
--------------------------
| | | | |
| 0 | 2 | 4 | 6 | Small Core0
| (p1)| (p2)| | |
Big Core --------------------------
| | | | |
| 1 | 3 | 5 | 7 | Small Core1
| | (p3)| | (p4) |
--------------------------
An example of Suboptimal Task placement:
--------------------------
| | | | |
| 0 | 2 | 4 | 6 | Small Core0
| (p1)| (p2)| | (p4)|
Big Core --------------------------
| | | | |
| 1 | 3 | 5 | 7 | Small Core1
| | (p3)| | |
--------------------------
In order to achieve optimal task placement, on big-core systems, we
define the SMT level sched-domain to consist of the threads belonging
to the small cores. The CACHE level sched domain will consist of all
the threads belonging to the big-core. With this, the Linux Kernel
load-balancer will ensure that the tasks are spread across all the
component small cores in the system, thereby yielding optimum
performance.
Furthermore, this solution works correctly across all SMT modes
(8,4,2), as the interleaved thread-ids ensures that when we go to
lower SMT modes (4,2) the threads are offlined in a descending order,
thereby leaving equal number of threads from the component small cores
online as illustrated below.
With Patches: (ppc64_cpu --smt=on) : SMT domain
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
CPU0 attaching sched-domain(s):
domain-0: span=0,2,4,6 level=SMT
groups: 0:{ span=0 cap=294 }, 2:{ span=2 cap=294 },
4:{ span=4 cap=294 }, 6:{ span=6 cap=294 }
CPU1 attaching sched-domain(s):
domain-0: span=1,3,5,7 level=SMT
groups: 1:{ span=1 cap=294 }, 3:{ span=3 cap=294 },
5:{ span=5 cap=294 }, 7:{ span=7 cap=294 }
Optimal Task placement (SMT 8)
--------------------------
| | | | |
| 0 | 2 | 4 | 6 | Small Core0
| (p1)| (p2)| | |
Big Core --------------------------
| | | | |
| 1 | 3 | 5 | 7 | Small Core1
| | (p3)| | (p4) |
--------------------------
With Patches : (ppc64_cpu --smt=4) : SMT domain
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
CPU0 attaching sched-domain(s):
domain-0: span=0,2 level=SMT
groups: 0:{ span=0 cap=589 }, 2:{ span=2 cap=589 }
CPU1 attaching sched-domain(s):
domain-0: span=1,3 level=SMT
groups: 1:{ span=1 cap=589 }, 3:{ span=3 cap=589 }
Optimal Task placement (SMT 4)
--------------------------
| | | | |
| 0 | 2 | 4 | 6 | Small Core0
| (p1)| (p2)| Off | Off |
Big Core --------------------------
| | | | |
| 1 | 3 | 5 | 7 | Small Core1
| (p4)| (p3)| Off | Off |
--------------------------
With Patches : (ppc64_cpu --smt=2) : SMT domain ceases to exist.
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Optimal Task placement (SMT 2)
--------------------------
| (p2)| | | |
| 0 | 2 | 4 | 6 | Small Core0
| (p1)| Off | Off | Off |
Big Core --------------------------
| (p3)| | | |
| 1 | 3 | 5 | 7 | Small Core1
| (p4)| Off | Off | Off |
--------------------------
Thus, as an added advantage in SMT=2 mode, we will only have 3 levels
in the sched-domain topology (CACHE, DIE and NUMA).
The SMT levels, without the patches are as follows.
Without Patches: (ppc64_cpu --smt=on) : SMT domain
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
CPU0 attaching sched-domain(s):
domain-0: span=0-7 level=SMT
groups: 0:{ span=0 cap=147 }, 1:{ span=1 cap=147 },
2:{ span=2 cap=147 }, 3:{ span=3 cap=147 },
4:{ span=4 cap=147 }, 5:{ span=5 cap=147 },
6:{ span=6 cap=147 }, 7:{ span=7 cap=147 }
CPU1 attaching sched-domain(s):
domain-0: span=0-7 level=SMT
groups: 1:{ span=1 cap=147 }, 2:{ span=2 cap=147 },
3:{ span=3 cap=147 }, 4:{ span=4 cap=147 },
5:{ span=5 cap=147 }, 6:{ span=6 cap=147 },
7:{ span=7 cap=147 }, 0:{ span=0 cap=147 }
Without Patches: (ppc64_cpu --smt=4) : SMT domain
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
CPU0 attaching sched-domain(s):
domain-0: span=0-3 level=SMT
groups: 0:{ span=0 cap=294 }, 1:{ span=1 cap=294 },
2:{ span=2 cap=294 }, 3:{ span=3 cap=294 },
CPU1 attaching sched-domain(s):
domain-0: span=0-3 level=SMT
groups: 1:{ span=1 cap=294 }, 2:{ span=2 cap=294 },
3:{ span=3 cap=294 }, 0:{ span=0 cap=294 }
Without Patches: (ppc64_cpu --smt=2) : SMT domain
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
CPU0 attaching sched-domain(s):
domain-0: span=0-1 level=SMT
groups: 0:{ span=0 cap=589 }, 1:{ span=1 cap=589 },
CPU1 attaching sched-domain(s):
domain-0: span=0-1 level=SMT
groups: 1:{ span=1 cap=589 }, 0:{ span=0 cap=589 },
This patchset contains two patches which on detecting the presence of
big-cores, defines the SMT level sched domain to correspond to the
threads of the small cores.
Patch 1: adds support to detect the presence of
big-cores and reports the small-core siblings of each CPU X
via the sysfs file "/sys/devices/system/cpu/cpuX/small_core_siblings".
Patch 2: Defines the SMT level sched domain to correspond to the
threads of the small cores.
Results:
~~~~~~~~~~~~~~~~~
1) 2 thread ebizzy
~~~~~~~~~~~~~~~~~~~~~~
Experimental results for ebizzy with 2 threads, bound to a single big-core
show a marked improvement with this patchset over the 4.18-rc5 vanilla
kernel.
The result of 100 such runs for 4.18-rc7 kernel and the 4.18-rc7 +
big-core-smt-patches are as follows
4.18.0-rc7 vanilla
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
records/s : # samples : Histogram
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[ 0 - 1000000] : 0 : #
[1000000 - 2000000] : 3 : #
[2000000 - 3000000] : 7 : ##
[3000000 - 4000000] : 26 : ######
[4000000 - 5000000] : 4 : #
[5000000 - 6000000] : 60 : #############
4.18.0-rc7 + big-core-smt-patches
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
records/s : # samples : Histogram
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[ 0 - 1000000] : 0 : #
[1000000 - 2000000] : 0 : #
[2000000 - 3000000] : 11 : ###
[3000000 - 4000000] : 0 : #
[4000000 - 5000000] : 0 : #
[5000000 - 6000000] : 89 : ##################
2) Hackbench (perf bench sched pipe)
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
100 iterations of the hackbench run both on 4.18-rc7 vanilla kernel
and v.18-rc7 + big-core-smt-patches. All the values are time in
seconds (Lower the better)
4.18.0-rc7 vanilla
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
N Min Max Median Avg Stddev
x 100 4.225 9.754 6.174 6.00402 0.88311027
4.18.0-rc7 + big-core-smt-patches (v6 : the present version)
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
N Min Max Median Avg Stddev
x 100 4.069 6.745 6.08 5.72414 0.73853727
The presence of the CACHE level sched-domain in v6, which was absent
in v5 of the patches seems to be making a difference, as the median
and the average times taken by hackbench both drop.
4.18.0-rc7 + big-core-smt-patches (v5 : the previous version)
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
N Min Max Median Avg Stddev
x 100 4.972 10.123 6.177 6.323 0.68728617
Gautham R. Shenoy (2):
powerpc: Detect the presence of big-cores via "ibm,thread-groups"
powerpc: Use cpu_smallcore_sibling_mask at SMT level on bigcores
Documentation/ABI/testing/sysfs-devices-system-cpu | 8 ++
arch/powerpc/include/asm/cputhreads.h | 22 +++
arch/powerpc/include/asm/smp.h | 6 +
arch/powerpc/kernel/setup-common.c | 154 +++++++++++++++++++++
arch/powerpc/kernel/smp.c | 62 ++++++++-
arch/powerpc/kernel/sysfs.c | 35 +++++
6 files changed, 282 insertions(+), 5 deletions(-)
--
1.9.4
^ permalink raw reply
* [PATCH v6 1/2] powerpc: Detect the presence of big-cores via "ibm, thread-groups"
From: Gautham R. Shenoy @ 2018-08-09 5:32 UTC (permalink / raw)
To: Srikar Dronamraju, Michael Ellerman, Benjamin Herrenschmidt,
Michael Neuling, Vaidyanathan Srinivasan, Akshay Adiga,
Shilpasri G Bhat, Oliver O'Halloran, Nicholas Piggin,
Murilo Opsfelder Araujo, Anton Blanchard
Cc: linuxppc-dev, linux-kernel, Gautham R. Shenoy
In-Reply-To: <1533792728-6304-1-git-send-email-ego@linux.vnet.ibm.com>
From: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com>
On IBM POWER9, the device tree exposes a property array identifed by
"ibm,thread-groups" which will indicate which groups of threads share a
particular set of resources.
As of today we only have one form of grouping identifying the group of
threads in the core that share the L1 cache, translation cache and
instruction data flow.
This patch defines the helper function to parse the contents of
"ibm,thread-groups" and a new structure to contain the parsed output.
The patch also creates the sysfs file named "small_core_siblings" that
returns the physical ids of the threads in the core that share the L1
cache, translation cache and instruction data flow.
Signed-off-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
---
Documentation/ABI/testing/sysfs-devices-system-cpu | 8 ++
arch/powerpc/include/asm/cputhreads.h | 22 +++
arch/powerpc/kernel/setup-common.c | 154 +++++++++++++++++++++
arch/powerpc/kernel/sysfs.c | 35 +++++
4 files changed, 219 insertions(+)
diff --git a/Documentation/ABI/testing/sysfs-devices-system-cpu b/Documentation/ABI/testing/sysfs-devices-system-cpu
index 9c5e7732..52c9b50 100644
--- a/Documentation/ABI/testing/sysfs-devices-system-cpu
+++ b/Documentation/ABI/testing/sysfs-devices-system-cpu
@@ -487,3 +487,11 @@ Description: Information about CPU vulnerabilities
"Not affected" CPU is not affected by the vulnerability
"Vulnerable" CPU is affected and no mitigation in effect
"Mitigation: $M" CPU is affected and mitigation $M is in effect
+
+What: /sys/devices/system/cpu/cpu[0-9]+/small_core_siblings
+Date: 06-Aug-2018
+KernelVersion: v4.19.0
+Contact: Linux for PowerPC mailing list <linuxppc-dev@ozlabs.org>
+Description: List of Physical ids of CPUs which share the L1 cache,
+ translation cache and instruction data-flow with this CPU.
+Values: Comma separated list of decimal integers.
diff --git a/arch/powerpc/include/asm/cputhreads.h b/arch/powerpc/include/asm/cputhreads.h
index d71a909..33226d7 100644
--- a/arch/powerpc/include/asm/cputhreads.h
+++ b/arch/powerpc/include/asm/cputhreads.h
@@ -23,11 +23,13 @@
extern int threads_per_core;
extern int threads_per_subcore;
extern int threads_shift;
+extern bool has_big_cores;
extern cpumask_t threads_core_mask;
#else
#define threads_per_core 1
#define threads_per_subcore 1
#define threads_shift 0
+#define has_big_cores 0
#define threads_core_mask (*get_cpu_mask(0))
#endif
@@ -69,12 +71,32 @@ static inline cpumask_t cpu_online_cores_map(void)
return cpu_thread_mask_to_cores(cpu_online_mask);
}
+#define MAX_THREAD_LIST_SIZE 8
+struct thread_groups {
+ unsigned int property;
+ unsigned int nr_groups;
+ unsigned int threads_per_group;
+ unsigned int thread_list[MAX_THREAD_LIST_SIZE];
+};
+
#ifdef CONFIG_SMP
int cpu_core_index_of_thread(int cpu);
int cpu_first_thread_of_core(int core);
+int parse_thread_groups(struct device_node *dn, struct thread_groups *tg);
+int get_cpu_thread_group_start(int cpu, struct thread_groups *tg);
#else
static inline int cpu_core_index_of_thread(int cpu) { return cpu; }
static inline int cpu_first_thread_of_core(int core) { return core; }
+static inline int parse_thread_groups(struct device_node *dn,
+ struct thread_groups *tg)
+{
+ return -ENODATA;
+}
+
+static inline int get_cpu_thread_group_start(int cpu, struct thread_groups *tg)
+{
+ return -1;
+}
#endif
static inline int cpu_thread_in_core(int cpu)
diff --git a/arch/powerpc/kernel/setup-common.c b/arch/powerpc/kernel/setup-common.c
index 40b44bb..989edc1 100644
--- a/arch/powerpc/kernel/setup-common.c
+++ b/arch/powerpc/kernel/setup-common.c
@@ -402,10 +402,12 @@ void __init check_for_initrd(void)
#ifdef CONFIG_SMP
int threads_per_core, threads_per_subcore, threads_shift;
+bool has_big_cores;
cpumask_t threads_core_mask;
EXPORT_SYMBOL_GPL(threads_per_core);
EXPORT_SYMBOL_GPL(threads_per_subcore);
EXPORT_SYMBOL_GPL(threads_shift);
+EXPORT_SYMBOL_GPL(has_big_cores);
EXPORT_SYMBOL_GPL(threads_core_mask);
static void __init cpu_init_thread_core_maps(int tpc)
@@ -433,6 +435,152 @@ static void __init cpu_init_thread_core_maps(int tpc)
u32 *cpu_to_phys_id = NULL;
+/*
+ * parse_thread_groups: Parses the "ibm,thread-groups" device tree
+ * property for the CPU device node @dn and stores
+ * the parsed output in the thread_groups
+ * structure @tg.
+ *
+ * @dn: The device node of the CPU device.
+ * @tg: Pointer to a thread group structure into which the parsed
+ * output of "ibm,thread-groups" is stored.
+ *
+ * ibm,thread-groups[0..N-1] array defines which group of threads in
+ * the CPU-device node can be grouped together based on the property.
+ *
+ * ibm,thread-groups[0] tells us the property based on which the
+ * threads are being grouped together. If this value is 1, it implies
+ * that the threads in the same group share L1, translation cache.
+ *
+ * ibm,thread-groups[1] tells us how many such thread groups exist.
+ *
+ * ibm,thread-groups[2] tells us the number of threads in each such
+ * group.
+ *
+ * ibm,thread-groups[3..N-1] is the list of threads identified by
+ * "ibm,ppc-interrupt-server#s" arranged as per their membership in
+ * the grouping.
+ *
+ * Example: If ibm,thread-groups = [1,2,4,5,6,7,8,9,10,11,12] it
+ * implies that there are 2 groups of 4 threads each, where each group
+ * of threads share L1, translation cache.
+ *
+ * The "ibm,ppc-interrupt-server#s" of the first group is {5,6,7,8}
+ * and the "ibm,ppc-interrupt-server#s" of the second group is {9, 10,
+ * 11, 12} structure
+ *
+ * Returns 0 on success, -EINVAL if the property does not exist,
+ * -ENODATA if property does not have a value, and -EOVERFLOW if the
+ * property data isn't large enough.
+ */
+int parse_thread_groups(struct device_node *dn,
+ struct thread_groups *tg)
+{
+ unsigned int nr_groups, threads_per_group, property;
+ int i;
+ u32 thread_group_array[3 + MAX_THREAD_LIST_SIZE];
+ u32 *thread_list;
+ size_t total_threads;
+ int ret;
+
+ ret = of_property_read_u32_array(dn, "ibm,thread-groups",
+ thread_group_array, 3);
+
+ if (ret)
+ goto out_err;
+
+ property = thread_group_array[0];
+ nr_groups = thread_group_array[1];
+ threads_per_group = thread_group_array[2];
+ total_threads = nr_groups * threads_per_group;
+
+ ret = of_property_read_u32_array(dn, "ibm,thread-groups",
+ thread_group_array,
+ 3 + total_threads);
+ if (ret)
+ goto out_err;
+
+ thread_list = &thread_group_array[3];
+
+ for (i = 0 ; i < total_threads; i++)
+ tg->thread_list[i] = thread_list[i];
+
+ tg->property = property;
+ tg->nr_groups = nr_groups;
+ tg->threads_per_group = threads_per_group;
+
+ return 0;
+out_err:
+ tg->property = 0;
+ tg->nr_groups = 0;
+ tg->threads_per_group = 0;
+ return ret;
+}
+
+/*
+ * dt_has_big_core : Parses the device tree property
+ * "ibm,thread-groups" for device node pointed by @dn
+ * and stores the parsed output in the structure
+ * pointed to by @tg. Then checks if the output in
+ * @tg corresponds to a big-core.
+ *
+ * @dn: Device node pointer of the CPU node being checked for a
+ * big-core.
+ * @tg: Pointer to thread_groups struct in which parsed output of
+ * "ibm,thread-groups" is recorded.
+ *
+ * Returns true if the @dn points to a big-core.
+ * Returns false if there is an error in parsing "ibm,thread-groups"
+ * or the parsed output doesn't correspond to a big-core.
+ */
+static inline bool dt_has_big_core(struct device_node *dn,
+ struct thread_groups *tg)
+{
+ if (parse_thread_groups(dn, tg))
+ return false;
+
+ if (tg->property != 1)
+ return false;
+
+ if (tg->nr_groups < 1)
+ return false;
+
+ return true;
+}
+
+/*
+ * get_cpu_thread_group_start : Searches the thread group in tg->thread_list
+ * that @cpu belongs to.
+ *
+ * @cpu : The logical CPU whose thread group is being searched.
+ * @tg : The thread-group structure of the CPU node which @cpu belongs
+ * to.
+ *
+ * Returns the index to tg->thread_list that points to the the start
+ * of the thread_group that @cpu belongs to.
+ *
+ * Returns -1 if cpu doesn't belong to any of the groups pointed to by
+ * tg->thread_list.
+ */
+int get_cpu_thread_group_start(int cpu, struct thread_groups *tg)
+{
+ int hw_cpu_id = get_hard_smp_processor_id(cpu);
+ int i, j;
+
+ for (i = 0; i < tg->nr_groups; i++) {
+ int group_start = i * tg->threads_per_group;
+
+ for (j = 0; j < tg->threads_per_group; j++) {
+ int idx = group_start + j;
+
+ if (tg->thread_list[idx] == hw_cpu_id)
+ return group_start;
+ }
+ }
+
+ return -1;
+}
+
/**
* setup_cpu_maps - initialize the following cpu maps:
* cpu_possible_mask
@@ -457,6 +605,7 @@ void __init smp_setup_cpu_maps(void)
int cpu = 0;
int nthreads = 1;
+ has_big_cores = true;
DBG("smp_setup_cpu_maps()\n");
cpu_to_phys_id = __va(memblock_alloc(nr_cpu_ids * sizeof(u32),
@@ -467,6 +616,7 @@ void __init smp_setup_cpu_maps(void)
const __be32 *intserv;
__be32 cpu_be;
int j, len;
+ struct thread_groups tg;
DBG(" * %pOF...\n", dn);
@@ -505,6 +655,10 @@ void __init smp_setup_cpu_maps(void)
cpu++;
}
+ if (has_big_cores && !dt_has_big_core(dn, &tg)) {
+ has_big_cores = false;
+ }
+
if (cpu >= nr_cpu_ids) {
of_node_put(dn);
break;
diff --git a/arch/powerpc/kernel/sysfs.c b/arch/powerpc/kernel/sysfs.c
index 755dc98..f5717de 100644
--- a/arch/powerpc/kernel/sysfs.c
+++ b/arch/powerpc/kernel/sysfs.c
@@ -18,6 +18,7 @@
#include <asm/smp.h>
#include <asm/pmc.h>
#include <asm/firmware.h>
+#include <asm/cputhreads.h>
#include "cacheinfo.h"
#include "setup.h"
@@ -1025,6 +1026,33 @@ static ssize_t show_physical_id(struct device *dev,
}
static DEVICE_ATTR(physical_id, 0444, show_physical_id, NULL);
+static ssize_t show_small_core_siblings(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct cpu *cpu = container_of(dev, struct cpu, dev);
+ struct device_node *dn = of_get_cpu_node(cpu->dev.id, NULL);
+ struct thread_groups tg;
+ int i, j;
+ ssize_t ret = 0;
+
+ if (parse_thread_groups(dn, &tg))
+ return -ENODATA;
+
+ i = get_cpu_thread_group_start(cpu->dev.id, &tg);
+
+ if (i == -1)
+ return -ENODATA;
+
+ for (j = 0; j < tg.threads_per_group - 1; j++)
+ ret += sprintf(buf + ret, "%d,", tg.thread_list[i + j]);
+
+ ret += sprintf(buf + ret, "%d\n", tg.thread_list[i + j]);
+
+ return ret;
+}
+static DEVICE_ATTR(small_core_siblings, 0444, show_small_core_siblings, NULL);
+
static int __init topology_init(void)
{
int cpu, r;
@@ -1048,6 +1076,13 @@ static int __init topology_init(void)
register_cpu(c, cpu);
device_create_file(&c->dev, &dev_attr_physical_id);
+
+ if (has_big_cores) {
+ const struct device_attribute *attr =
+ &dev_attr_small_core_siblings;
+
+ device_create_file(&c->dev, attr);
+ }
}
}
r = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "powerpc/topology:online",
--
1.9.4
^ permalink raw reply related
* [PATCH v6 2/2] powerpc: Use cpu_smallcore_sibling_mask at SMT level on bigcores
From: Gautham R. Shenoy @ 2018-08-09 5:32 UTC (permalink / raw)
To: Srikar Dronamraju, Michael Ellerman, Benjamin Herrenschmidt,
Michael Neuling, Vaidyanathan Srinivasan, Akshay Adiga,
Shilpasri G Bhat, Oliver O'Halloran, Nicholas Piggin,
Murilo Opsfelder Araujo, Anton Blanchard
Cc: linuxppc-dev, linux-kernel, Gautham R. Shenoy
In-Reply-To: <1533792728-6304-1-git-send-email-ego@linux.vnet.ibm.com>
From: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com>
Each of the SMT4 cores forming a big-core are more or less independent
units. Thus when multiple tasks are scheduled to run on the fused
core, we get the best performance when the tasks are spread across the
pair of SMT4 cores.
This patch achieves this by setting the SMT level mask to correspond
to the smallcore sibling mask on big-core systems. This patch also
ensures that while checked for shared-caches on big-core system, we
use the smallcore_sibling_mask to compare with the l2_cache_mask.
This ensure that the CACHE level sched-domain is created, whose groups
correspond to the threads of the big-core.
With this patch, the SMT sched-domain with SMT=8,4,2 on big-core
systems are as follows:
1) ppc64_cpu --smt=8
CPU0 attaching sched-domain(s):
domain-0: span=0,2,4,6 level=SMT
groups: 0:{ span=0 cap=294 }, 2:{ span=2 cap=294 },
4:{ span=4 cap=294 }, 6:{ span=6 cap=294 }
CPU1 attaching sched-domain(s):
domain-0: span=1,3,5,7 level=SMT
groups: 1:{ span=1 cap=294 }, 3:{ span=3 cap=294 },
5:{ span=5 cap=294 }, 7:{ span=7 cap=294 }
2) ppc64_cpu --smt=4
CPU0 attaching sched-domain(s):
domain-0: span=0,2 level=SMT
groups: 0:{ span=0 cap=589 }, 2:{ span=2 cap=589 }
CPU1 attaching sched-domain(s):
domain-0: span=1,3 level=SMT
groups: 1:{ span=1 cap=589 }, 3:{ span=3 cap=589 }
3) ppc64_cpu --smt=2
SMT domain ceases to exist as each domain consists of just one
group.
Signed-off-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
---
arch/powerpc/include/asm/smp.h | 6 ++++
arch/powerpc/kernel/smp.c | 62 ++++++++++++++++++++++++++++++++++++++----
2 files changed, 63 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/include/asm/smp.h b/arch/powerpc/include/asm/smp.h
index 29ffaab..30798c7 100644
--- a/arch/powerpc/include/asm/smp.h
+++ b/arch/powerpc/include/asm/smp.h
@@ -99,6 +99,7 @@ static inline void set_hard_smp_processor_id(int cpu, int phys)
#endif
DECLARE_PER_CPU(cpumask_var_t, cpu_sibling_map);
+DECLARE_PER_CPU(cpumask_var_t, cpu_smallcore_sibling_map);
DECLARE_PER_CPU(cpumask_var_t, cpu_l2_cache_map);
DECLARE_PER_CPU(cpumask_var_t, cpu_core_map);
@@ -107,6 +108,11 @@ static inline struct cpumask *cpu_sibling_mask(int cpu)
return per_cpu(cpu_sibling_map, cpu);
}
+static inline struct cpumask *cpu_smallcore_sibling_mask(int cpu)
+{
+ return per_cpu(cpu_smallcore_sibling_map, cpu);
+}
+
static inline struct cpumask *cpu_core_mask(int cpu)
{
return per_cpu(cpu_core_map, cpu);
diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index 4794d6b..a515780 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -76,10 +76,12 @@
struct thread_info *secondary_ti;
DEFINE_PER_CPU(cpumask_var_t, cpu_sibling_map);
+DEFINE_PER_CPU(cpumask_var_t, cpu_smallcore_sibling_map);
DEFINE_PER_CPU(cpumask_var_t, cpu_l2_cache_map);
DEFINE_PER_CPU(cpumask_var_t, cpu_core_map);
EXPORT_PER_CPU_SYMBOL(cpu_sibling_map);
+EXPORT_PER_CPU_SYMBOL(cpu_smallcore_sibling_map);
EXPORT_PER_CPU_SYMBOL(cpu_l2_cache_map);
EXPORT_PER_CPU_SYMBOL(cpu_core_map);
@@ -689,6 +691,9 @@ void __init smp_prepare_cpus(unsigned int max_cpus)
for_each_possible_cpu(cpu) {
zalloc_cpumask_var_node(&per_cpu(cpu_sibling_map, cpu),
GFP_KERNEL, cpu_to_node(cpu));
+ zalloc_cpumask_var_node(&per_cpu(cpu_smallcore_sibling_map,
+ cpu),
+ GFP_KERNEL, cpu_to_node(cpu));
zalloc_cpumask_var_node(&per_cpu(cpu_l2_cache_map, cpu),
GFP_KERNEL, cpu_to_node(cpu));
zalloc_cpumask_var_node(&per_cpu(cpu_core_map, cpu),
@@ -707,6 +712,10 @@ void __init smp_prepare_cpus(unsigned int max_cpus)
cpumask_set_cpu(boot_cpuid, cpu_sibling_mask(boot_cpuid));
cpumask_set_cpu(boot_cpuid, cpu_l2_cache_mask(boot_cpuid));
cpumask_set_cpu(boot_cpuid, cpu_core_mask(boot_cpuid));
+ if (has_big_cores) {
+ cpumask_set_cpu(boot_cpuid,
+ cpu_smallcore_sibling_mask(boot_cpuid));
+ }
if (smp_ops && smp_ops->probe)
smp_ops->probe();
@@ -991,6 +1000,10 @@ static void remove_cpu_from_masks(int cpu)
set_cpus_unrelated(cpu, i, cpu_core_mask);
set_cpus_unrelated(cpu, i, cpu_l2_cache_mask);
set_cpus_unrelated(cpu, i, cpu_sibling_mask);
+ if (has_big_cores) {
+ set_cpus_unrelated(cpu, i,
+ cpu_smallcore_sibling_mask);
+ }
}
}
#endif
@@ -999,7 +1012,17 @@ static void add_cpu_to_masks(int cpu)
{
int first_thread = cpu_first_thread_sibling(cpu);
int chipid = cpu_to_chip_id(cpu);
- int i;
+
+ struct thread_groups tg;
+ int i, cpu_group_start = -1;
+
+ if (has_big_cores) {
+ struct device_node *dn = of_get_cpu_node(cpu, NULL);
+
+ parse_thread_groups(dn, &tg);
+ cpu_group_start = get_cpu_thread_group_start(cpu, &tg);
+ cpumask_set_cpu(cpu, cpu_smallcore_sibling_mask(cpu));
+ }
/*
* This CPU will not be in the online mask yet so we need to manually
@@ -1007,9 +1030,21 @@ static void add_cpu_to_masks(int cpu)
*/
cpumask_set_cpu(cpu, cpu_sibling_mask(cpu));
- for (i = first_thread; i < first_thread + threads_per_core; i++)
- if (cpu_online(i))
- set_cpus_related(i, cpu, cpu_sibling_mask);
+ for (i = first_thread; i < first_thread + threads_per_core; i++) {
+ int i_group_start;
+
+ if (!cpu_online(i))
+ continue;
+
+ set_cpus_related(i, cpu, cpu_sibling_mask);
+
+ if (!has_big_cores)
+ continue;
+
+ i_group_start = get_cpu_thread_group_start(i, &tg);
+ if (i_group_start == cpu_group_start)
+ set_cpus_related(i, cpu, cpu_smallcore_sibling_mask);
+ }
/*
* Copy the thread sibling mask into the cache sibling mask
@@ -1040,6 +1075,7 @@ static void add_cpu_to_masks(int cpu)
void start_secondary(void *unused)
{
unsigned int cpu = smp_processor_id();
+ struct cpumask *(*sibling_mask)(int) = cpu_sibling_mask;
mmgrab(&init_mm);
current->active_mm = &init_mm;
@@ -1065,11 +1101,13 @@ void start_secondary(void *unused)
/* Update topology CPU masks */
add_cpu_to_masks(cpu);
+ if (has_big_cores)
+ sibling_mask = cpu_smallcore_sibling_mask;
/*
* Check for any shared caches. Note that this must be done on a
* per-core basis because one core in the pair might be disabled.
*/
- if (!cpumask_equal(cpu_l2_cache_mask(cpu), cpu_sibling_mask(cpu)))
+ if (!cpumask_equal(cpu_l2_cache_mask(cpu), sibling_mask(cpu)))
shared_caches = true;
set_numa_node(numa_cpu_lookup_table[cpu]);
@@ -1136,6 +1174,13 @@ static const struct cpumask *shared_cache_mask(int cpu)
return cpu_l2_cache_mask(cpu);
}
+#ifdef CONFIG_SCHED_SMT
+static const struct cpumask *smallcore_smt_mask(int cpu)
+{
+ return cpu_smallcore_sibling_mask(cpu);
+}
+#endif
+
static struct sched_domain_topology_level power9_topology[] = {
#ifdef CONFIG_SCHED_SMT
{ cpu_smt_mask, powerpc_smt_flags, SD_INIT_NAME(SMT) },
@@ -1158,6 +1203,13 @@ void __init smp_cpus_done(unsigned int max_cpus)
dump_numa_cpu_topology();
+#ifdef CONFIG_SCHED_SMT
+ if (has_big_cores) {
+ pr_info("Using small cores at SMT level\n");
+ power9_topology[0].mask = smallcore_smt_mask;
+ powerpc_topology[0].mask = smallcore_smt_mask;
+ }
+#endif
/*
* If any CPU detects that it's sharing a cache with another CPU then
* use the deeper topology that is aware of this sharing.
--
1.9.4
^ permalink raw reply related
* Re: [RFC PATCH 3/3] cpuidle/powernv: Conditionally save-restore sprs using opal
From: Gautham R Shenoy @ 2018-08-08 15:41 UTC (permalink / raw)
To: Nicholas Piggin
Cc: Akshay Adiga, linux-kernel, linuxppc-dev, mpe, benh, ego, huntbag
In-Reply-To: <20180803000547.08a37175@roar.ozlabs.ibm.com>
Hello Nicholas,
On Fri, Aug 03, 2018 at 12:05:47AM +1000, Nicholas Piggin wrote:
> On Thu, 2 Aug 2018 10:21:32 +0530
> Akshay Adiga <akshay.adiga@linux.vnet.ibm.com> wrote:
>
> > From: Abhishek Goel <huntbag@linux.vnet.ibm.com>
> >
> > If a state has "opal-supported" compat flag in device-tree, an opal call
> > needs to be made during the entry and exit of the stop state. This patch
> > passes a hint to the power9_idle_stop and power9_offline_stop.
> >
> > This patch moves the saving and restoring of sprs for P9 cpuidle
> > from kernel to opal. This patch still uses existing code to detect
> > first thread in core.
> > In an attempt to make the powernv idle code backward compatible,
> > and to some extent forward compatible, add support for pre-stop entry
> > and post-stop exit actions in OPAL. If a kernel knows about this
> > opal call, then just a firmware supporting newer hardware is required,
> > instead of waiting for kernel updates.
>
> Still think we should make these do-everything calls. Including
> executing nap/stop instructions, restoring timebase, possibly even
> saving and restoring SLB (although a return code could be used to
> tell the kernel to do that maybe if performance advantage is
enough).
So, if we execute the stop instruction in opal, the wakeup from stop
still happens at the hypervisor 0x100. On wake up, we need to check
SRR1 to see if we have lost state, in which case, the stop exit also
needs to be handled inside opal. On return from this opal call, we
need to unwind the extra stack frame that would have been created when
kernel entered opal to execute the stop from which there was no
return. In the case where a lossy stop state was requested, but wakeup
happened from a lossless stop state, this adds additional overhead.
Furthermore, the measurements show that the additional time taken to
perform the restore of the resources in OPAL vs doing so in Kernel on
wakeup from stop takes additional 5-10us. For the current stop states
that lose hypervisor state, since the latency is relatively high (100s
of us), this is a relatively small penalty (~1%) .
However, in future if we do have states that lose only a part of
hypervisor state to provide a wakeup latency in the order of few tens
of microseconds the additional latency caused by OPAL call would
become noticable, no ?
>
> I haven't had a lot of time to go through it, I'm working on moving
> ~all of idle_book3s.S to C code, I'd like to do that before this
> OPAL idle driver if possible.
>
> A minor thing I just noticed, you don't have to allocate the opal
> spr save space in Linux, just do it all in OPAL.
The idea was to not leave any state in OPAL, as OPAL is supposed to be
state-less. However, I agree, that if OPAL is not going to interpret
the contents of the save/area, it should be harmless to move that bit
into OPAL.
That said, if we are going to add the logic of determining the first
thread in the core waking up, etc, then we have no choice but to
maintain that state in OPAL.
>
> Thanks,
> Nick
>
--
Thanks and Regards
gautham.
^ permalink raw reply
* Re: [RFC 0/4] Virtio uses DMA API for all devices
From: Christoph Hellwig @ 2018-08-09 5:40 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: Michael S. Tsirkin, 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: <98eb367ce322ad84baa31e3c7beffc4a42be8458.camel@kernel.crashing.org>
On Thu, Aug 09, 2018 at 08:13:32AM +1000, Benjamin Herrenschmidt wrote:
> > > - 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.
Yes. I also generally want to avoid too much arch specific magic.
FYI, I'm off to a week-long vacation today, don't expect quick replies.
^ permalink raw reply
* Re: [PATCH] lib/test_hexdump: fix failure on big endian cpu
From: Michael Ellerman @ 2018-08-09 6:18 UTC (permalink / raw)
To: rashmica, Christophe Leroy, Andy Shevchenko, Andrew Morton,
Linus Torvalds
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <7857c69a-a209-541b-b2e1-9fe2e288ca65@gmail.com>
rashmica <rashmicy@gmail.com> writes:
> On 08/08/18 17:25, Michael Ellerman wrote:
>> Christophe Leroy <christophe.leroy@c-s.fr> writes:
>>> diff --git a/lib/test_hexdump.c b/lib/test_hexdump.c
>>> index 3f415d8101f3..626f580b4ff7 100644
>>> --- a/lib/test_hexdump.c
>>> +++ b/lib/test_hexdump.c
>>> @@ -32,16 +32,33 @@ static const char * const test_data_2_le[] __initconst = {
>>> "d14c", "9919", "b143", "0caf",
>>> };
>>>
>>> +static const char * const test_data_2_be[] __initconst = {
>>> + "be32", "db7b", "0a18", "93b2",
>>> + "70ba", "c424", "7d83", "349b",
>>> + "a69c", "31ad", "9c0f", "ace9",
>>> + "4cd1", "1999", "43b1", "af0c",
>>> +};
>>> +
>>> static const char * const test_data_4_le[] __initconst = {
>>> "7bdb32be", "b293180a", "24c4ba70", "9b34837d",
>>> "ad319ca6", "e9ac0f9c", "9919d14c", "0cafb143",
>>> };
>>>
>>> +static const char * const test_data_4_be[] __initconst = {
>>> + "be32db7b", "0a1893b2", "70bac424", "7d83349b",
>>> + "a69c31ad", "9c0face9", "4cd11999", "43b1af0c",
>>> +};
>>> +
>> Is there a reason we can't do it all at compile time?
>
> mpe I sent a patch doing that awhile ago and you obviously didn't like
> it because you never merged it :P
Sorry, I wasn't sure who should merge it, and never followed up.
cheers
^ permalink raw reply
* Re: [RFC 5/5] powerpc/fsl: Add supported-irq-ranges for P2020
From: Scott Wood @ 2018-08-09 6:11 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: <AM5PR0401MB25458654D6CEFBF2F05FC7709A250@AM5PR0401MB2545.eurprd04.prod.outlook.com>
On Thu, 2018-08-09 at 03:28 +0000, Bharat Bhushan wrote:
> > -----Original Message-----
> > From: Scott Wood [mailto:oss@buserror.net]
> > Sent: Wednesday, August 8, 2018 11:27 PM
> > 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 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).
>
> Fair enough, we can use MPIC version to define supported interrupts ranges.
> Will that be acceptable.
It's better than device tree changes but I'm not convinced it's worthwhile
just to suppress some simulator warnings. If the warnings really bother you,
you can use pic-no-reset in the device tree (assuming this isn't some new chip
that you want to make sure doesn't fall over when the usual mpic init happens)
and/or convince the hardware people to make the interface properly
discoverable including discontiguous regions (if there *is* some new chip I
haven't heard about).
-Scott
^ permalink raw reply
* Re: [PATCH v7 8/9] powerpc/mce: Add sysctl control for recovery action on MCE.
From: Michael Ellerman @ 2018-08-09 6:34 UTC (permalink / raw)
To: Aneesh Kumar K.V, Mahesh J Salgaonkar, linuxppc-dev
Cc: Aneesh Kumar K.V, Michal Suchanek, Ananth Narayan,
Nicholas Piggin, Laurent Dufour
In-Reply-To: <d08a7794-e5c2-2b05-c21a-ae5a7baa7d88@linux.ibm.com>
"Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes:
> 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.
If you're debugging something complex that you can't determine from the
SLB dump then you should be running a debug kernel anyway. And if
anything you want to drop into xmon and sit there, preserving the most
state, rather than taking a dump.
The last SLB multi-hit I debugged was this:
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=db7130d63fd8
Which took quite a while to track down, including a bunch of tracing and
so on. A dump would not have helped in the slightest.
cheers
^ permalink raw reply
* Re: [PATCH 1/5] powerpc/asm: Add a patch_site macro & helpers for patching instructions
From: Christophe LEROY @ 2018-08-09 6:56 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev
In-Reply-To: <4572374a-d131-b1ad-0d2d-9564925b52cf@c-s.fr>
Le 08/08/2018 à 18:30, Christophe LEROY a écrit :
>
>
> 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 ?
Forget my stupid question, I didn't see it was a relative address and
not an absolute one.
Christophe
>
> 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: [RFC 5/5] powerpc/fsl: Add supported-irq-ranges for P2020
From: Bharat Bhushan @ 2018-08-09 7:04 UTC (permalink / raw)
To: Scott Wood, 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: <5c1bcbd2c753f4224764c1932e5a9a99b4a604f5.camel@buserror.net>
DQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogU2NvdHQgV29vZCBbbWFp
bHRvOm9zc0BidXNlcnJvci5uZXRdDQo+IFNlbnQ6IFRodXJzZGF5LCBBdWd1c3QgOSwgMjAxOCAx
MTo0MiBBTQ0KPiBUbzogQmhhcmF0IEJodXNoYW4gPGJoYXJhdC5iaHVzaGFuQG54cC5jb20+Ow0K
PiBiZW5oQGtlcm5lbC5jcmFzaGluZy5vcmc7IHBhdWx1c0BzYW1iYS5vcmc7IG1wZUBlbGxlcm1h
bi5pZC5hdTsNCj4gZ2FsYWtAa2VybmVsLmNyYXNoaW5nLm9yZzsgbWFyay5ydXRsYW5kQGFybS5j
b207DQo+IGtzdGV3YXJ0QGxpbnV4Zm91bmRhdGlvbi5vcmc7IGdyZWdraEBsaW51eGZvdW5kYXRp
b24ub3JnOw0KPiBkZXZpY2V0cmVlQHZnZXIua2VybmVsLm9yZzsgbGludXhwcGMtZGV2QGxpc3Rz
Lm96bGFicy5vcmc7IGxpbnV4LQ0KPiBrZXJuZWxAdmdlci5rZXJuZWwub3JnDQo+IENjOiByb2Jo
QGtlcm5lbC5vcmc7IGtlZXNjb29rQGNocm9taXVtLm9yZzsgdHlyZWxkQGxpbnV4LnZuZXQuaWJt
LmNvbTsNCj4gam9lQHBlcmNoZXMuY29tDQo+IFN1YmplY3Q6IFJlOiBbUkZDIDUvNV0gcG93ZXJw
Yy9mc2w6IEFkZCBzdXBwb3J0ZWQtaXJxLXJhbmdlcyBmb3IgUDIwMjANCj4gDQo+IE9uIFRodSwg
MjAxOC0wOC0wOSBhdCAwMzoyOCArMDAwMCwgQmhhcmF0IEJodXNoYW4gd3JvdGU6DQo+ID4gPiAt
LS0tLU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQ0KPiA+ID4gRnJvbTogU2NvdHQgV29vZCBbbWFpbHRv
Om9zc0BidXNlcnJvci5uZXRdDQo+ID4gPiBTZW50OiBXZWRuZXNkYXksIEF1Z3VzdCA4LCAyMDE4
IDExOjI3IFBNDQo+ID4gPiBUbzogQmhhcmF0IEJodXNoYW4gPGJoYXJhdC5iaHVzaGFuQG54cC5j
b20+Ow0KPiA+ID4gYmVuaEBrZXJuZWwuY3Jhc2hpbmcub3JnOyBwYXVsdXNAc2FtYmEub3JnOyBt
cGVAZWxsZXJtYW4uaWQuYXU7DQo+ID4gPiBnYWxha0BrZXJuZWwuY3Jhc2hpbmcub3JnOyBtYXJr
LnJ1dGxhbmRAYXJtLmNvbTsNCj4gPiA+IGtzdGV3YXJ0QGxpbnV4Zm91bmRhdGlvbi5vcmc7IGdy
ZWdraEBsaW51eGZvdW5kYXRpb24ub3JnOw0KPiA+ID4gZGV2aWNldHJlZUB2Z2VyLmtlcm5lbC5v
cmc7IGxpbnV4cHBjLWRldkBsaXN0cy5vemxhYnMub3JnOyBsaW51eC0NCj4gPiA+IGtlcm5lbEB2
Z2VyLmtlcm5lbC5vcmcNCj4gPiA+IENjOiByb2JoQGtlcm5lbC5vcmc7IGtlZXNjb29rQGNocm9t
aXVtLm9yZzsNCj4gPiA+IHR5cmVsZEBsaW51eC52bmV0LmlibS5jb207IGpvZUBwZXJjaGVzLmNv
bQ0KPiA+ID4gU3ViamVjdDogUmU6IFtSRkMgNS81XSBwb3dlcnBjL2ZzbDogQWRkIHN1cHBvcnRl
ZC1pcnEtcmFuZ2VzIGZvcg0KPiA+ID4gUDIwMjANCj4gPiA+DQo+ID4gPiBPbiBXZWQsIDIwMTgt
MDgtMDggYXQgMDY6MjggKzAwMDAsIEJoYXJhdCBCaHVzaGFuIHdyb3RlOg0KPiA+ID4gPiA+IC0t
LS0tT3JpZ2luYWwgTWVzc2FnZS0tLS0tDQo+ID4gPiA+ID4gRnJvbTogU2NvdHQgV29vZCBbbWFp
bHRvOm9zc0BidXNlcnJvci5uZXRdDQo+ID4gPiA+ID4gU2VudDogV2VkbmVzZGF5LCBBdWd1c3Qg
OCwgMjAxOCAxMToyNiBBTQ0KPiA+ID4gPiA+IFRvOiBCaGFyYXQgQmh1c2hhbiA8YmhhcmF0LmJo
dXNoYW5AbnhwLmNvbT47DQo+ID4gPiA+ID4gYmVuaEBrZXJuZWwuY3Jhc2hpbmcub3JnOyBwYXVs
dXNAc2FtYmEub3JnOyBtcGVAZWxsZXJtYW4uaWQuYXU7DQo+ID4gPiA+ID4gZ2FsYWtAa2VybmVs
LmNyYXNoaW5nLm9yZzsgbWFyay5ydXRsYW5kQGFybS5jb207DQo+ID4gPiA+ID4ga3N0ZXdhcnRA
bGludXhmb3VuZGF0aW9uLm9yZzsgZ3JlZ2toQGxpbnV4Zm91bmRhdGlvbi5vcmc7DQo+ID4gPiA+
ID4gZGV2aWNldHJlZUB2Z2VyLmtlcm5lbC5vcmc7IGxpbnV4cHBjLWRldkBsaXN0cy5vemxhYnMu
b3JnOw0KPiA+ID4gPiA+IGxpbnV4LSBrZXJuZWxAdmdlci5rZXJuZWwub3JnDQo+ID4gPiA+ID4g
Q2M6IHJvYmhAa2VybmVsLm9yZzsga2Vlc2Nvb2tAY2hyb21pdW0ub3JnOw0KPiA+ID4gPiA+IHR5
cmVsZEBsaW51eC52bmV0LmlibS5jb207IGpvZUBwZXJjaGVzLmNvbQ0KPiA+ID4gPiA+IFN1Ympl
Y3Q6IFJlOiBbUkZDIDUvNV0gcG93ZXJwYy9mc2w6IEFkZCBzdXBwb3J0ZWQtaXJxLXJhbmdlcyBm
b3INCj4gPiA+ID4gPiBQMjAyMA0KPiA+ID4gPiA+DQo+ID4gPiA+ID4gT24gV2VkLCAyMDE4LTA4
LTA4IGF0IDAzOjQ0ICswMDAwLCBCaGFyYXQgQmh1c2hhbiB3cm90ZToNCj4gPiA+ID4gPiA+ID4g
LS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gPiA+ID4gPiA+ID4gRnJvbTogU2NvdHQgV29v
ZCBbbWFpbHRvOm9zc0BidXNlcnJvci5uZXRdDQo+ID4gPiA+ID4gPiA+IFNlbnQ6IFdlZG5lc2Rh
eSwgQXVndXN0IDgsIDIwMTggMjo0NCBBTQ0KPiA+ID4gPiA+ID4gPiBUbzogQmhhcmF0IEJodXNo
YW4gPGJoYXJhdC5iaHVzaGFuQG54cC5jb20+Ow0KPiA+ID4gPiA+ID4gPiBiZW5oQGtlcm5lbC5j
cmFzaGluZy5vcmc7IHBhdWx1c0BzYW1iYS5vcmc7DQo+ID4gPiA+ID4gPiA+IG1wZUBlbGxlcm1h
bi5pZC5hdTsgZ2FsYWtAa2VybmVsLmNyYXNoaW5nLm9yZzsNCj4gPiA+ID4gPiA+ID4gbWFyay5y
dXRsYW5kQGFybS5jb207IGtzdGV3YXJ0QGxpbnV4Zm91bmRhdGlvbi5vcmc7DQo+ID4gPiA+ID4g
PiA+IGdyZWdraEBsaW51eGZvdW5kYXRpb24ub3JnOyBkZXZpY2V0cmVlQHZnZXIua2VybmVsLm9y
ZzsNCj4gPiA+ID4gPiA+ID4gbGludXhwcGMtZGV2QGxpc3RzLm96bGFicy5vcmc7DQo+ID4gPiA+
ID4gPiA+IGxpbnV4LSBrZXJuZWxAdmdlci5rZXJuZWwub3JnDQo+ID4gPiA+ID4gPiA+IENjOiBy
b2JoQGtlcm5lbC5vcmc7IGtlZXNjb29rQGNocm9taXVtLm9yZzsNCj4gPiA+ID4gPiA+ID4gdHly
ZWxkQGxpbnV4LnZuZXQuaWJtLmNvbTsgam9lQHBlcmNoZXMuY29tDQo+ID4gPiA+ID4gPiA+IFN1
YmplY3Q6IFJlOiBbUkZDIDUvNV0gcG93ZXJwYy9mc2w6IEFkZCBzdXBwb3J0ZWQtaXJxLXJhbmdl
cw0KPiA+ID4gPiA+ID4gPiBmb3INCj4gPiA+ID4gPiA+ID4gUDIwMjANCj4gPiA+ID4gPiA+ID4N
Cj4gPiA+ID4gPiA+ID4gT24gRnJpLCAyMDE4LTA3LTI3IGF0IDE1OjE4ICswNTMwLCBCaGFyYXQg
Qmh1c2hhbiB3cm90ZToNCj4gPiA+ID4gPiA+ID4gPiBNUElDIG9uIE5YUCAoRnJlZXNjYWxlKSBQ
MjAyMCBzdXBwb3J0cyBmb2xsb3dpbmcgaXJxDQo+ID4gPiA+ID4gPiA+ID4gcmFuZ2VzOg0KPiA+
ID4gPiA+ID4gPiA+ICAgPiAwIC0gMTEgICAgICAoRXh0ZXJuYWwgaW50ZXJydXB0KQ0KPiA+ID4g
PiA+ID4gPiA+ICAgPiAxNiAtIDc5ICAgICAoSW50ZXJuYWwgaW50ZXJydXB0KQ0KPiA+ID4gPiA+
ID4gPiA+ICAgPiAxNzYgLSAxODMgICAoTWVzc2FnaW5nIGludGVycnVwdCkNCj4gPiA+ID4gPiA+
ID4gPiAgID4gMjI0IC0gMjMxICAgKFNoYXJlZCBtZXNzYWdlIHNpZ25hbGVkIGludGVycnVwdCkN
Cj4gPiA+ID4gPiA+ID4NCj4gPiA+ID4gPiA+ID4gV2h5IGRvbid0IHlvdSBjb252ZXJ0IHRvIHRo
ZSA0LWNlbGwgaW50ZXJydXB0IHNwZWNpZmllcnMNCj4gPiA+ID4gPiA+ID4gdGhhdCBtYWtlIGRl
YWxpbmcgd2l0aCB0aGVzZSByYW5nZXMgbGVzcyBlcnJvci1wcm9uZT8NCj4gPiA+ID4gPiA+DQo+
ID4gPiA+ID4gPiBPayAsIHdpbGwgZG8gaWYgd2UgYWdyZWUgdG8gaGF2ZSB0aGlzIHNlcmllcyBh
cyBwZXIgY29tbWVudCBvbg0KPiA+ID4gPiA+ID4gb3RoZXIgcGF0Y2guDQo+ID4gPiA+ID4NCj4g
PiA+ID4gPiBJZiB5b3UncmUgY29uY2VybmVkIHdpdGggZXJyb3JzLCB0aGlzIHdvdWxkIGJlIGEg
Z29vZCB0aGluZ3MgdG8NCj4gPiA+ID4gPiBkbyByZWdhcmRsZXNzLg0KPiA+ID4gPiA+ICBBY3R1
YWxseSwgaXQgc2VlbXMgdGhhdCBwMjAyMHNpLXBvc3QuZHRzaSBhbHJlYWR5IHVzZXMgNC1jZWxs
DQo+ID4gPiA+ID4gaW50ZXJydXB0cy4NCj4gPiA+ID4gPg0KPiA+ID4gPiA+IFdoYXQgaXMgbW90
aXZhdGluZyB0aGlzIHBhdGNoc2V0PyAgSXMgdGhlcmUgc29tZXRoaW5nIHdyb25nIGluDQo+ID4g
PiA+ID4gdGhlIGV4aXN0aW5nIGR0cyBmaWxlcz8NCj4gPiA+ID4NCj4gPiA+ID4gVGhlcmUgaXMg
bm8gZXJyb3IgaW4gZGV2aWNlIHRyZWUuIE1haW4gbW90aXZhdGlvbiBpcyB0byBpbXByb3ZlDQo+
ID4gPiA+IGNvZGUgZm9yIGZvbGxvd2luZyByZWFzb25zOg0KPiA+ID4gPiAgIC0gV2hpbGUgY29k
ZSBzdHVkeSBpdCB3YXMgZm91bmQgdGhhdCBpZiBhIHJlc2VydmVkIGlycS1udW1iZXINCj4gPiA+
ID4gdXNlZCB0aGVuIHRoZXJlIGFyZSBubyBjaGVjayBpbiBkcml2ZXIuIGlycSB3aWxsIGJlIGNv
bmZpZ3VyZWQgYXMNCj4gPiA+ID4gY29ycmVjdCBhbmQgaW50ZXJydXB0IHdpbGwgbmV2ZXIgZmly
ZS4NCj4gPiA+DQo+ID4gPiBBZ2FpbiwgYSB3cm9uZyBpbnRlcnJ1cHQgbnVtYmVyIHdvbid0IGZp
cmUsIHdoZXRoZXIgYW4gaW50ZXJydXB0IGJ5DQo+ID4gPiB0aGF0IG51bWJlciBleGlzdHMgb3Ig
bm90LiAgSSB3b3VsZG4ndCBtaW5kIGEgc2FuaXR5IGNoZWNrIGluIHRoZQ0KPiA+ID4gZHJpdmVy
IGlmIHRoZSBwcm9ncmFtbWluZyBtb2RlbCBtYWRlIGl0IHByb3Blcmx5IGRpc2NvdmVyYWJsZSwg
YnV0IEkNCj4gPiA+IGRvbid0IHRoaW5rIGl0J3Mgd29ydGggbWVzc2luZyB3aXRoIGRldmljZSB0
cmVlcyBqdXN0IGZvciB0aGlzIChhbmQNCj4gPiA+IGV2ZW4gbGVzcyBzbyBnaXZlbiB0aGF0IHRo
ZXJlIGRvbid0IHNlZW0gdG8gYmUgbmV3IGNoaXBzIGNvbWluZyBvdXQNCj4gPiA+IHRoYXQgdGhp
cyB3b3VsZCBiZSByZWxldmFudCBmb3IpLg0KPiA+DQo+ID4gRmFpciBlbm91Z2gsIHdlIGNhbiB1
c2UgTVBJQyB2ZXJzaW9uIHRvIGRlZmluZSBzdXBwb3J0ZWQgaW50ZXJydXB0cw0KPiByYW5nZXMu
DQo+ID4gV2lsbCB0aGF0IGJlIGFjY2VwdGFibGUuDQo+IA0KPiBJdCdzIGJldHRlciB0aGFuIGRl
dmljZSB0cmVlIGNoYW5nZXMgYnV0IEknbSBub3QgY29udmluY2VkIGl0J3Mgd29ydGh3aGlsZSBq
dXN0DQo+IHRvIHN1cHByZXNzIHNvbWUgc2ltdWxhdG9yIHdhcm5pbmdzLg0KPiAgSWYgdGhlIHdh
cm5pbmdzIHJlYWxseSBib3RoZXIgeW91LCB5b3UNCj4gY2FuIHVzZSBwaWMtbm8tcmVzZXQgaW4g
dGhlIGRldmljZSB0cmVlIChhc3N1bWluZyB0aGlzIGlzbid0IHNvbWUgbmV3IGNoaXANCj4gdGhh
dCB5b3Ugd2FudCB0byBtYWtlIHN1cmUgZG9lc24ndCBmYWxsIG92ZXIgd2hlbiB0aGUgdXN1YWwg
bXBpYyBpbml0DQo+IGhhcHBlbnMpIGFuZC9vciBjb252aW5jZSB0aGUgaGFyZHdhcmUgcGVvcGxl
IHRvIG1ha2UgdGhlIGludGVyZmFjZQ0KPiBwcm9wZXJseSBkaXNjb3ZlcmFibGUgaW5jbHVkaW5n
IGRpc2NvbnRpZ3VvdXMgcmVnaW9ucyAoaWYgdGhlcmUgKmlzKiBzb21lDQo+IG5ldyBjaGlwIEkg
aGF2ZW4ndCBoZWFyZCBhYm91dCkuDQoNClRoZXJlIGlzIG5ldyBjaGlwIHVuZGVyIGRldmVsb3Bt
ZW50IGJhc2VkIG9uIGUyMDAsIHdoaWNoIHVzZXMgc2FtZSBtcGljIGFzIGluIG9sZGVyIFBvd2Vy
UEMgdmVyc2lvbnMuDQpUaGlzIHBhdGNoIHdhcyBub3QganVzdCBhYm91dCBzdXBwcmVzc2luZyB0
aGUgd2FybmluZyBidXQgd2FybmluZyB3YXMgdGhlIG9ic2VydmF0aW9uIHRoYXQgcmVzZXJ2ZWQg
cmVnaW9uIGlzIGdldHRpbmcgYWNjZXNzZWQgZHVyaW5nIGluaXQvdW5pbml0L3NhdmUvcmVzdG9y
ZS4gVGhpcyBwYXRjaCB3YXMganVzdCBhbiBtaW5vciBpbXByb3ZlbWVudCB0byBhdm9pZCB0aGVz
ZSBhY2Nlc3Nlcy4gV2Ugd2lsbCBkcm9wIHRoaXMgc2VyaWVzIGlmIHRoaXMgaW1wcm92ZW1lbnQg
aXMgbm90IGNvbnZpbmNpbmcuDQoNClRoYW5rcw0KLUJoYXJhdA0KDQo+IA0KPiAtU2NvdHQNCg0K
^ permalink raw reply
* Re: [PATCH v7 8/9] powerpc/mce: Add sysctl control for recovery action on MCE.
From: Nicholas Piggin @ 2018-08-09 8:02 UTC (permalink / raw)
To: Michael Ellerman
Cc: Aneesh Kumar K.V, Mahesh J Salgaonkar, linuxppc-dev,
Aneesh Kumar K.V, Michal Suchanek, Ananth Narayan, Laurent Dufour
In-Reply-To: <87d0us9hgg.fsf@concordia.ellerman.id.au>
On Thu, 09 Aug 2018 16:34:07 +1000
Michael Ellerman <mpe@ellerman.id.au> wrote:
> "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes:
> > 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.
>
> If you're debugging something complex that you can't determine from the
> SLB dump then you should be running a debug kernel anyway. And if
> anything you want to drop into xmon and sit there, preserving the most
> state, rather than taking a dump.
I'm not saying for a dump specifically, just some form of crash. And we
really should have an option to xmon on panic, but that's another story.
I think HA/failover kind of environments use options like this too. If
anything starts going bad they don't want to try limping along but stop
ASAP.
Thanks,
Nick
^ permalink raw reply
* Re: [PATCH v7 8/9] powerpc/mce: Add sysctl control for recovery action on MCE.
From: Ananth N Mavinakayanahalli @ 2018-08-09 8:09 UTC (permalink / raw)
To: Nicholas Piggin
Cc: Michael Ellerman, Aneesh Kumar K.V, Mahesh J Salgaonkar,
linuxppc-dev, Aneesh Kumar K.V, Michal Suchanek, Laurent Dufour
In-Reply-To: <20180809180253.5665ddf5@roar.ozlabs.ibm.com>
On Thu, Aug 09, 2018 at 06:02:53PM +1000, Nicholas Piggin wrote:
> On Thu, 09 Aug 2018 16:34:07 +1000
> Michael Ellerman <mpe@ellerman.id.au> wrote:
>
> > "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes:
> > > 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.
> >
> > If you're debugging something complex that you can't determine from the
> > SLB dump then you should be running a debug kernel anyway. And if
> > anything you want to drop into xmon and sit there, preserving the most
> > state, rather than taking a dump.
>
> I'm not saying for a dump specifically, just some form of crash. And we
> really should have an option to xmon on panic, but that's another story.
That's fine during development or in a lab, not something we could
enforce in a customer environment, could we?
> I think HA/failover kind of environments use options like this too. If
> anything starts going bad they don't want to try limping along but stop
> ASAP.
Right. And in this particular case, can we guarantee no corruption
(leading to or post the multihit recovery) when running a customer workload,
is the question...
Ananth
^ 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