* [PATCH kernel v3 0/2] DMA, powerpc/dma: Fallback to dma_ops when persistent memory present @ 2020-10-28 7:00 Alexey Kardashevskiy 2020-10-28 7:00 ` [PATCH kernel v3 1/2] dma: Allow mixing bypass and mapped DMA operation Alexey Kardashevskiy 2020-10-28 7:00 ` [PATCH kernel v3 2/2] powerpc/dma: Fallback to dma_ops when persistent memory present Alexey Kardashevskiy 0 siblings, 2 replies; 8+ messages in thread From: Alexey Kardashevskiy @ 2020-10-28 7:00 UTC (permalink / raw) To: linuxppc-dev Cc: Christoph Hellwig, Michael Ellerman, iommu, linux-kernel, Alexey Kardashevskiy This allows mixing direct DMA (to/from RAM) and IOMMU (to/from apersistent memory) on the PPC64/pseries platform. This replaces https://lkml.org/lkml/2020/10/27/418 which replaces https://lkml.org/lkml/2020/10/20/1085 This is based on sha1 4525c8781ec0 Linus Torvalds "scsi: qla2xxx: remove incorrect sparse #ifdef". Please comment. Thanks. Alexey Kardashevskiy (2): dma: Allow mixing bypass and mapped DMA operation powerpc/dma: Fallback to dma_ops when persistent memory present arch/powerpc/kernel/dma-iommu.c | 70 +++++++++++++++++++++++++- arch/powerpc/platforms/pseries/iommu.c | 44 ++++++++++++---- kernel/dma/mapping.c | 24 +++++++-- arch/powerpc/Kconfig | 1 + kernel/dma/Kconfig | 4 ++ 5 files changed, 127 insertions(+), 16 deletions(-) -- 2.17.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH kernel v3 1/2] dma: Allow mixing bypass and mapped DMA operation 2020-10-28 7:00 [PATCH kernel v3 0/2] DMA, powerpc/dma: Fallback to dma_ops when persistent memory present Alexey Kardashevskiy @ 2020-10-28 7:00 ` Alexey Kardashevskiy 2020-10-28 17:22 ` Christoph Hellwig 2020-10-28 7:00 ` [PATCH kernel v3 2/2] powerpc/dma: Fallback to dma_ops when persistent memory present Alexey Kardashevskiy 1 sibling, 1 reply; 8+ messages in thread From: Alexey Kardashevskiy @ 2020-10-28 7:00 UTC (permalink / raw) To: linuxppc-dev Cc: Christoph Hellwig, Michael Ellerman, iommu, linux-kernel, Alexey Kardashevskiy At the moment we allow bypassing DMA ops only when we can do this for the entire RAM. However there are configs with mixed type memory where we could still allow bypassing IOMMU in most cases; POWERPC with persistent memory is one example. This adds an arch hook to determine where bypass can still work and we invoke direct DMA API. The following patch checks the bus limit on POWERPC to allow or disallow direct mapping. This adds a CONFIG_ARCH_HAS_DMA_SET_MASK config option to make arch_xxxx hooks no-op by default. Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> --- kernel/dma/mapping.c | 24 ++++++++++++++++++++---- kernel/dma/Kconfig | 4 ++++ 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c index 51bb8fa8eb89..a0bc9eb876ed 100644 --- a/kernel/dma/mapping.c +++ b/kernel/dma/mapping.c @@ -137,6 +137,18 @@ static inline bool dma_map_direct(struct device *dev, return dma_go_direct(dev, *dev->dma_mask, ops); } +#ifdef CONFIG_ARCH_HAS_DMA_MAP_DIRECT +bool arch_dma_map_page_direct(struct device *dev, phys_addr_t addr); +bool arch_dma_unmap_page_direct(struct device *dev, dma_addr_t dma_handle); +bool arch_dma_map_sg_direct(struct device *dev, struct scatterlist *sg, int nents); +bool arch_dma_unmap_sg_direct(struct device *dev, struct scatterlist *sg, int nents); +#else +#define arch_dma_map_page_direct(d, a) (0) +#define arch_dma_unmap_page_direct(d, a) (0) +#define arch_dma_map_sg_direct(d, s, n) (0) +#define arch_dma_unmap_sg_direct(d, s, n) (0) +#endif + dma_addr_t dma_map_page_attrs(struct device *dev, struct page *page, size_t offset, size_t size, enum dma_data_direction dir, unsigned long attrs) @@ -149,7 +161,8 @@ dma_addr_t dma_map_page_attrs(struct device *dev, struct page *page, if (WARN_ON_ONCE(!dev->dma_mask)) return DMA_MAPPING_ERROR; - if (dma_map_direct(dev, ops)) + if (dma_map_direct(dev, ops) || + arch_dma_map_page_direct(dev, page_to_phys(page) + offset + size)) addr = dma_direct_map_page(dev, page, offset, size, dir, attrs); else addr = ops->map_page(dev, page, offset, size, dir, attrs); @@ -165,7 +178,8 @@ void dma_unmap_page_attrs(struct device *dev, dma_addr_t addr, size_t size, const struct dma_map_ops *ops = get_dma_ops(dev); BUG_ON(!valid_dma_direction(dir)); - if (dma_map_direct(dev, ops)) + if (dma_map_direct(dev, ops) || + arch_dma_unmap_page_direct(dev, addr + size)) dma_direct_unmap_page(dev, addr, size, dir, attrs); else if (ops->unmap_page) ops->unmap_page(dev, addr, size, dir, attrs); @@ -188,7 +202,8 @@ int dma_map_sg_attrs(struct device *dev, struct scatterlist *sg, int nents, if (WARN_ON_ONCE(!dev->dma_mask)) return 0; - if (dma_map_direct(dev, ops)) + if (dma_map_direct(dev, ops) || + arch_dma_map_sg_direct(dev, sg, nents)) ents = dma_direct_map_sg(dev, sg, nents, dir, attrs); else ents = ops->map_sg(dev, sg, nents, dir, attrs); @@ -207,7 +222,8 @@ void dma_unmap_sg_attrs(struct device *dev, struct scatterlist *sg, BUG_ON(!valid_dma_direction(dir)); debug_dma_unmap_sg(dev, sg, nents, dir); - if (dma_map_direct(dev, ops)) + if (dma_map_direct(dev, ops) || + arch_dma_unmap_sg_direct(dev, sg, nents)) dma_direct_unmap_sg(dev, sg, nents, dir, attrs); else if (ops->unmap_sg) ops->unmap_sg(dev, sg, nents, dir, attrs); diff --git a/kernel/dma/Kconfig b/kernel/dma/Kconfig index c99de4a21458..43d106598e82 100644 --- a/kernel/dma/Kconfig +++ b/kernel/dma/Kconfig @@ -20,6 +20,10 @@ config DMA_OPS config DMA_OPS_BYPASS bool +# Lets platform IOMMU driver choose between bypass and IOMMU +config ARCH_HAS_DMA_MAP_DIRECT + bool + config NEED_SG_DMA_LENGTH bool -- 2.17.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH kernel v3 1/2] dma: Allow mixing bypass and mapped DMA operation 2020-10-28 7:00 ` [PATCH kernel v3 1/2] dma: Allow mixing bypass and mapped DMA operation Alexey Kardashevskiy @ 2020-10-28 17:22 ` Christoph Hellwig 2020-10-28 23:15 ` Alexey Kardashevskiy 0 siblings, 1 reply; 8+ messages in thread From: Christoph Hellwig @ 2020-10-28 17:22 UTC (permalink / raw) To: Alexey Kardashevskiy Cc: linuxppc-dev, Christoph Hellwig, Michael Ellerman, iommu, linux-kernel On Wed, Oct 28, 2020 at 06:00:29PM +1100, Alexey Kardashevskiy wrote: > At the moment we allow bypassing DMA ops only when we can do this for > the entire RAM. However there are configs with mixed type memory > where we could still allow bypassing IOMMU in most cases; > POWERPC with persistent memory is one example. > > This adds an arch hook to determine where bypass can still work and > we invoke direct DMA API. The following patch checks the bus limit > on POWERPC to allow or disallow direct mapping. > > This adds a CONFIG_ARCH_HAS_DMA_SET_MASK config option to make arch_xxxx > hooks no-op by default. > > Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> > --- > kernel/dma/mapping.c | 24 ++++++++++++++++++++---- > kernel/dma/Kconfig | 4 ++++ > 2 files changed, 24 insertions(+), 4 deletions(-) > > diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c > index 51bb8fa8eb89..a0bc9eb876ed 100644 > --- a/kernel/dma/mapping.c > +++ b/kernel/dma/mapping.c > @@ -137,6 +137,18 @@ static inline bool dma_map_direct(struct device *dev, > return dma_go_direct(dev, *dev->dma_mask, ops); > } > > +#ifdef CONFIG_ARCH_HAS_DMA_MAP_DIRECT > +bool arch_dma_map_page_direct(struct device *dev, phys_addr_t addr); > +bool arch_dma_unmap_page_direct(struct device *dev, dma_addr_t dma_handle); > +bool arch_dma_map_sg_direct(struct device *dev, struct scatterlist *sg, int nents); > +bool arch_dma_unmap_sg_direct(struct device *dev, struct scatterlist *sg, int nents); > +#else > +#define arch_dma_map_page_direct(d, a) (0) > +#define arch_dma_unmap_page_direct(d, a) (0) > +#define arch_dma_map_sg_direct(d, s, n) (0) > +#define arch_dma_unmap_sg_direct(d, s, n) (0) > +#endif A bunch of overly long lines here. Except for that this looks ok to me. If you want me to queue up the series I can just fix it up. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH kernel v3 1/2] dma: Allow mixing bypass and mapped DMA operation 2020-10-28 17:22 ` Christoph Hellwig @ 2020-10-28 23:15 ` Alexey Kardashevskiy 0 siblings, 0 replies; 8+ messages in thread From: Alexey Kardashevskiy @ 2020-10-28 23:15 UTC (permalink / raw) To: Christoph Hellwig; +Cc: linuxppc-dev, Michael Ellerman, iommu, linux-kernel On 29/10/2020 04:22, Christoph Hellwig wrote: > On Wed, Oct 28, 2020 at 06:00:29PM +1100, Alexey Kardashevskiy wrote: >> At the moment we allow bypassing DMA ops only when we can do this for >> the entire RAM. However there are configs with mixed type memory >> where we could still allow bypassing IOMMU in most cases; >> POWERPC with persistent memory is one example. >> >> This adds an arch hook to determine where bypass can still work and >> we invoke direct DMA API. The following patch checks the bus limit >> on POWERPC to allow or disallow direct mapping. >> >> This adds a CONFIG_ARCH_HAS_DMA_SET_MASK config option to make arch_xxxx >> hooks no-op by default. >> >> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> >> --- >> kernel/dma/mapping.c | 24 ++++++++++++++++++++---- >> kernel/dma/Kconfig | 4 ++++ >> 2 files changed, 24 insertions(+), 4 deletions(-) >> >> diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c >> index 51bb8fa8eb89..a0bc9eb876ed 100644 >> --- a/kernel/dma/mapping.c >> +++ b/kernel/dma/mapping.c >> @@ -137,6 +137,18 @@ static inline bool dma_map_direct(struct device *dev, >> return dma_go_direct(dev, *dev->dma_mask, ops); >> } >> >> +#ifdef CONFIG_ARCH_HAS_DMA_MAP_DIRECT >> +bool arch_dma_map_page_direct(struct device *dev, phys_addr_t addr); >> +bool arch_dma_unmap_page_direct(struct device *dev, dma_addr_t dma_handle); >> +bool arch_dma_map_sg_direct(struct device *dev, struct scatterlist *sg, int nents); >> +bool arch_dma_unmap_sg_direct(struct device *dev, struct scatterlist *sg, int nents); >> +#else >> +#define arch_dma_map_page_direct(d, a) (0) >> +#define arch_dma_unmap_page_direct(d, a) (0) >> +#define arch_dma_map_sg_direct(d, s, n) (0) >> +#define arch_dma_unmap_sg_direct(d, s, n) (0) >> +#endif > > A bunch of overly long lines here. Except for that this looks ok to me. > If you want me to queue up the series I can just fix it up. I thought 100 is the new limit since https://lkml.org/lkml/2020/5/29/1038 (yeah that mentioned some Christoph :) ) and having these multiline does not make a huge difference but feel free fixing them up. Are you going to take both patches? Do you need mpe's ack? Thanks, -- Alexey ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH kernel v3 2/2] powerpc/dma: Fallback to dma_ops when persistent memory present 2020-10-28 7:00 [PATCH kernel v3 0/2] DMA, powerpc/dma: Fallback to dma_ops when persistent memory present Alexey Kardashevskiy 2020-10-28 7:00 ` [PATCH kernel v3 1/2] dma: Allow mixing bypass and mapped DMA operation Alexey Kardashevskiy @ 2020-10-28 7:00 ` Alexey Kardashevskiy 2020-10-29 0:40 ` Michael Ellerman 1 sibling, 1 reply; 8+ messages in thread From: Alexey Kardashevskiy @ 2020-10-28 7:00 UTC (permalink / raw) To: linuxppc-dev Cc: Christoph Hellwig, Michael Ellerman, iommu, linux-kernel, Alexey Kardashevskiy So far we have been using huge DMA windows to map all the RAM available. The RAM is normally mapped to the VM address space contiguously, and there is always a reasonable upper limit for possible future hot plugged RAM which makes it easy to map all RAM via IOMMU. Now there is persistent memory ("ibm,pmemory" in the FDT) which (unlike normal RAM) can map anywhere in the VM space beyond the maximum RAM size and since it can be used for DMA, it requires extending the huge window up to MAX_PHYSMEM_BITS which requires hypervisor support for: 1. huge TCE tables; 2. multilevel TCE tables; 3. huge IOMMU pages. Certain hypervisors cannot do either so the only option left is restricting the huge DMA window to include only RAM and fallback to the default DMA window for persistent memory. This defines arch_dma_map_direct/etc to allow generic DMA code perform additional checks on whether direct DMA is still possible. This checks if the system has persistent memory. If it does not, the DMA bypass mode is selected, i.e. * dev->bus_dma_limit = 0 * dev->dma_ops_bypass = true <- this avoid calling dma_ops for mapping. If there is such memory, this creates identity mapping only for RAM and sets the dev->bus_dma_limit to let the generic code decide whether to call into the direct DMA or the indirect DMA ops. This should not change the existing behaviour when no persistent memory as dev->dma_ops_bypass is expected to be set. Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> --- arch/powerpc/kernel/dma-iommu.c | 70 +++++++++++++++++++++++++- arch/powerpc/platforms/pseries/iommu.c | 44 ++++++++++++---- arch/powerpc/Kconfig | 1 + 3 files changed, 103 insertions(+), 12 deletions(-) diff --git a/arch/powerpc/kernel/dma-iommu.c b/arch/powerpc/kernel/dma-iommu.c index a1c744194018..21e2d9f059a9 100644 --- a/arch/powerpc/kernel/dma-iommu.c +++ b/arch/powerpc/kernel/dma-iommu.c @@ -10,6 +10,63 @@ #include <linux/pci.h> #include <asm/iommu.h> +#define can_map_direct(dev, addr) \ + ((dev)->bus_dma_limit >= phys_to_dma((dev), (addr))) + +bool arch_dma_map_page_direct(struct device *dev, phys_addr_t addr) +{ + if (likely(!dev->bus_dma_limit)) + return false; + + return can_map_direct(dev, addr); +} +EXPORT_SYMBOL_GPL(arch_dma_map_page_direct); + +#define is_direct_handle(dev, h) ((h) >= (dev)->archdata.dma_offset) + +bool arch_dma_unmap_page_direct(struct device *dev, dma_addr_t dma_handle) +{ + if (likely(!dev->bus_dma_limit)) + return false; + + return is_direct_handle(dev, dma_handle); +} +EXPORT_SYMBOL_GPL(arch_dma_unmap_page_direct); + +bool arch_dma_map_sg_direct(struct device *dev, struct scatterlist *sg, int nents) +{ + struct scatterlist *s; + int i; + + if (likely(!dev->bus_dma_limit)) + return false; + + for_each_sg(sg, s, nents, i) { + if (!can_map_direct(dev, sg_phys(s) + s->offset + s->length)) + return false; + } + + return true; +} +EXPORT_SYMBOL(arch_dma_map_sg_direct); + +bool arch_dma_unmap_sg_direct(struct device *dev, struct scatterlist *sg, int nents) +{ + struct scatterlist *s; + int i; + + if (likely(!dev->bus_dma_limit)) + return false; + + for_each_sg(sg, s, nents, i) { + if (!is_direct_handle(dev, s->dma_address + s->length)) + return false; + } + + return true; +} +EXPORT_SYMBOL(arch_dma_unmap_sg_direct); + /* * Generic iommu implementation */ @@ -90,8 +147,17 @@ int dma_iommu_dma_supported(struct device *dev, u64 mask) struct iommu_table *tbl = get_iommu_table_base(dev); if (dev_is_pci(dev) && dma_iommu_bypass_supported(dev, mask)) { - dev->dma_ops_bypass = true; - dev_dbg(dev, "iommu: 64-bit OK, using fixed ops\n"); + /* + * dma_iommu_bypass_supported() sets dma_max when there is + * 1:1 mapping but it is somehow limited. + * ibm,pmemory is one example. + */ + dev->dma_ops_bypass = dev->bus_dma_limit == 0; + if (!dev->dma_ops_bypass) + dev_warn(dev, "iommu: 64-bit OK but direct DMA is limited by %llx\n", + dev->bus_dma_limit); + else + dev_dbg(dev, "iommu: 64-bit OK, using fixed ops\n"); return 1; } diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c index e4198700ed1a..91112e748491 100644 --- a/arch/powerpc/platforms/pseries/iommu.c +++ b/arch/powerpc/platforms/pseries/iommu.c @@ -839,7 +839,7 @@ static void remove_ddw(struct device_node *np, bool remove_prop) np, ret); } -static u64 find_existing_ddw(struct device_node *pdn) +static u64 find_existing_ddw(struct device_node *pdn, int *window_shift) { struct direct_window *window; const struct dynamic_dma_window_prop *direct64; @@ -851,6 +851,7 @@ static u64 find_existing_ddw(struct device_node *pdn) if (window->device == pdn) { direct64 = window->prop; dma_addr = be64_to_cpu(direct64->dma_base); + *window_shift = be32_to_cpu(direct64->window_shift); break; } } @@ -1111,11 +1112,13 @@ static void reset_dma_window(struct pci_dev *dev, struct device_node *par_dn) */ static u64 enable_ddw(struct pci_dev *dev, struct device_node *pdn) { - int len, ret; + int len = 0, ret; + bool pmem_present = of_find_node_by_type(NULL, "ibm,pmemory") != NULL; + int max_ram_len = order_base_2(ddw_memory_hotplug_max()); struct ddw_query_response query; struct ddw_create_response create; int page_shift; - u64 dma_addr, max_addr; + u64 dma_addr; struct device_node *dn; u32 ddw_avail[DDW_APPLICABLE_SIZE]; struct direct_window *window; @@ -1126,7 +1129,7 @@ static u64 enable_ddw(struct pci_dev *dev, struct device_node *pdn) mutex_lock(&direct_window_init_mutex); - dma_addr = find_existing_ddw(pdn); + dma_addr = find_existing_ddw(pdn, &len); if (dma_addr != 0) goto out_unlock; @@ -1212,14 +1215,26 @@ static u64 enable_ddw(struct pci_dev *dev, struct device_node *pdn) } /* verify the window * number of ptes will map the partition */ /* check largest block * page size > max memory hotplug addr */ - max_addr = ddw_memory_hotplug_max(); - if (query.largest_available_block < (max_addr >> page_shift)) { - dev_dbg(&dev->dev, "can't map partition max 0x%llx with %llu " - "%llu-sized pages\n", max_addr, query.largest_available_block, - 1ULL << page_shift); + /* + * The "ibm,pmemory" can appear anywhere in the address space. + * Assuming it is still backed by page structs, try MAX_PHYSMEM_BITS + * for the upper limit and fallback to max RAM otherwise but this + * disables device::dma_ops_bypass. + */ + len = max_ram_len; + if (pmem_present) { + if (query.largest_available_block >= + (1ULL << (MAX_PHYSMEM_BITS - page_shift))) + len = MAX_PHYSMEM_BITS - page_shift; + else + dev_info(&dev->dev, "Skipping ibm,pmemory"); + } + + if (query.largest_available_block < (1ULL << (len - page_shift))) { + dev_dbg(&dev->dev, "can't map partition max 0x%llx with %llu %llu-sized pages\n", + 1ULL << len, query.largest_available_block, 1ULL << page_shift); goto out_failed; } - len = order_base_2(max_addr); win64 = kzalloc(sizeof(struct property), GFP_KERNEL); if (!win64) { dev_info(&dev->dev, @@ -1299,6 +1314,15 @@ static u64 enable_ddw(struct pci_dev *dev, struct device_node *pdn) out_unlock: mutex_unlock(&direct_window_init_mutex); + + /* + * If we have persistent memory and the window size is only as big + * as RAM, then we failed to create a window to cover persistent + * memory and need to set the DMA limit. + */ + if (pmem_present && dma_addr && (len == max_ram_len)) + dev->dev.bus_dma_limit = dma_addr + (1ULL << len); + return dma_addr; } diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig index e9f13fe08492..b2d4580acf79 100644 --- a/arch/powerpc/Kconfig +++ b/arch/powerpc/Kconfig @@ -159,6 +159,7 @@ config PPC select DCACHE_WORD_ACCESS if PPC64 && CPU_LITTLE_ENDIAN select DMA_OPS if PPC64 select DMA_OPS_BYPASS if PPC64 + select ARCH_HAS_DMA_MAP_DIRECT if PPC64 && PPC_PSERIES select DYNAMIC_FTRACE if FUNCTION_TRACER select EDAC_ATOMIC_SCRUB select EDAC_SUPPORT -- 2.17.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH kernel v3 2/2] powerpc/dma: Fallback to dma_ops when persistent memory present 2020-10-28 7:00 ` [PATCH kernel v3 2/2] powerpc/dma: Fallback to dma_ops when persistent memory present Alexey Kardashevskiy @ 2020-10-29 0:40 ` Michael Ellerman 2020-10-29 0:46 ` Alexey Kardashevskiy 0 siblings, 1 reply; 8+ messages in thread From: Michael Ellerman @ 2020-10-29 0:40 UTC (permalink / raw) To: Alexey Kardashevskiy, linuxppc-dev Cc: Christoph Hellwig, iommu, linux-kernel, Alexey Kardashevskiy Alexey Kardashevskiy <aik@ozlabs.ru> writes: > diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c > index e4198700ed1a..91112e748491 100644 > --- a/arch/powerpc/platforms/pseries/iommu.c > +++ b/arch/powerpc/platforms/pseries/iommu.c > @@ -1111,11 +1112,13 @@ static void reset_dma_window(struct pci_dev *dev, struct device_node *par_dn) > */ > static u64 enable_ddw(struct pci_dev *dev, struct device_node *pdn) > { > - int len, ret; > + int len = 0, ret; > + bool pmem_present = of_find_node_by_type(NULL, "ibm,pmemory") != NULL; That leaks a reference on the returned node. dn = of_find_node_by_type(NULL, "ibm,pmemory"); pmem_present = dn != NULL; of_node_put(dn); > @@ -1126,7 +1129,7 @@ static u64 enable_ddw(struct pci_dev *dev, struct device_node *pdn) > > mutex_lock(&direct_window_init_mutex); > > - dma_addr = find_existing_ddw(pdn); > + dma_addr = find_existing_ddw(pdn, &len); I don't see len used anywhere? > if (dma_addr != 0) > goto out_unlock; > > @@ -1212,14 +1215,26 @@ static u64 enable_ddw(struct pci_dev *dev, struct device_node *pdn) > } > /* verify the window * number of ptes will map the partition */ > /* check largest block * page size > max memory hotplug addr */ > - max_addr = ddw_memory_hotplug_max(); > - if (query.largest_available_block < (max_addr >> page_shift)) { > - dev_dbg(&dev->dev, "can't map partition max 0x%llx with %llu " > - "%llu-sized pages\n", max_addr, query.largest_available_block, > - 1ULL << page_shift); > + /* > + * The "ibm,pmemory" can appear anywhere in the address space. > + * Assuming it is still backed by page structs, try MAX_PHYSMEM_BITS > + * for the upper limit and fallback to max RAM otherwise but this > + * disables device::dma_ops_bypass. > + */ > + len = max_ram_len; Here you override whatever find_existing_ddw() wrote to len? > + if (pmem_present) { > + if (query.largest_available_block >= > + (1ULL << (MAX_PHYSMEM_BITS - page_shift))) > + len = MAX_PHYSMEM_BITS - page_shift; > + else > + dev_info(&dev->dev, "Skipping ibm,pmemory"); > + } > + > + if (query.largest_available_block < (1ULL << (len - page_shift))) { > + dev_dbg(&dev->dev, "can't map partition max 0x%llx with %llu %llu-sized pages\n", > + 1ULL << len, query.largest_available_block, 1ULL << page_shift); > goto out_failed; > } > - len = order_base_2(max_addr); > win64 = kzalloc(sizeof(struct property), GFP_KERNEL); > if (!win64) { > dev_info(&dev->dev, cheers ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH kernel v3 2/2] powerpc/dma: Fallback to dma_ops when persistent memory present 2020-10-29 0:40 ` Michael Ellerman @ 2020-10-29 0:46 ` Alexey Kardashevskiy 2020-10-29 9:55 ` Michael Ellerman 0 siblings, 1 reply; 8+ messages in thread From: Alexey Kardashevskiy @ 2020-10-29 0:46 UTC (permalink / raw) To: Michael Ellerman, linuxppc-dev; +Cc: Christoph Hellwig, iommu, linux-kernel On 29/10/2020 11:40, Michael Ellerman wrote: > Alexey Kardashevskiy <aik@ozlabs.ru> writes: >> diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c >> index e4198700ed1a..91112e748491 100644 >> --- a/arch/powerpc/platforms/pseries/iommu.c >> +++ b/arch/powerpc/platforms/pseries/iommu.c >> @@ -1111,11 +1112,13 @@ static void reset_dma_window(struct pci_dev *dev, struct device_node *par_dn) >> */ >> static u64 enable_ddw(struct pci_dev *dev, struct device_node *pdn) >> { >> - int len, ret; >> + int len = 0, ret; >> + bool pmem_present = of_find_node_by_type(NULL, "ibm,pmemory") != NULL; > > That leaks a reference on the returned node. > > dn = of_find_node_by_type(NULL, "ibm,pmemory"); > pmem_present = dn != NULL; > of_node_put(dn); ah, true. v4 then. > > >> @@ -1126,7 +1129,7 @@ static u64 enable_ddw(struct pci_dev *dev, struct device_node *pdn) >> >> mutex_lock(&direct_window_init_mutex); >> >> - dma_addr = find_existing_ddw(pdn); >> + dma_addr = find_existing_ddw(pdn, &len); > > I don't see len used anywhere? > >> if (dma_addr != 0) >> goto out_unlock; >> >> @@ -1212,14 +1215,26 @@ static u64 enable_ddw(struct pci_dev *dev, struct device_node *pdn) >> } >> /* verify the window * number of ptes will map the partition */ >> /* check largest block * page size > max memory hotplug addr */ >> - max_addr = ddw_memory_hotplug_max(); >> - if (query.largest_available_block < (max_addr >> page_shift)) { >> - dev_dbg(&dev->dev, "can't map partition max 0x%llx with %llu " >> - "%llu-sized pages\n", max_addr, query.largest_available_block, >> - 1ULL << page_shift); >> + /* >> + * The "ibm,pmemory" can appear anywhere in the address space. >> + * Assuming it is still backed by page structs, try MAX_PHYSMEM_BITS >> + * for the upper limit and fallback to max RAM otherwise but this >> + * disables device::dma_ops_bypass. >> + */ >> + len = max_ram_len; > > Here you override whatever find_existing_ddw() wrote to len? Not always, there is a bunch of gotos before this line to the end of the function and one (which returns the existing window) is legit. Thanks, > >> + if (pmem_present) { >> + if (query.largest_available_block >= >> + (1ULL << (MAX_PHYSMEM_BITS - page_shift))) >> + len = MAX_PHYSMEM_BITS - page_shift; >> + else >> + dev_info(&dev->dev, "Skipping ibm,pmemory"); >> + } >> + >> + if (query.largest_available_block < (1ULL << (len - page_shift))) { >> + dev_dbg(&dev->dev, "can't map partition max 0x%llx with %llu %llu-sized pages\n", >> + 1ULL << len, query.largest_available_block, 1ULL << page_shift); >> goto out_failed; >> } >> - len = order_base_2(max_addr); >> win64 = kzalloc(sizeof(struct property), GFP_KERNEL); >> if (!win64) { >> dev_info(&dev->dev, > > > cheers > -- Alexey ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH kernel v3 2/2] powerpc/dma: Fallback to dma_ops when persistent memory present 2020-10-29 0:46 ` Alexey Kardashevskiy @ 2020-10-29 9:55 ` Michael Ellerman 0 siblings, 0 replies; 8+ messages in thread From: Michael Ellerman @ 2020-10-29 9:55 UTC (permalink / raw) To: Alexey Kardashevskiy, linuxppc-dev; +Cc: Christoph Hellwig, iommu, linux-kernel Alexey Kardashevskiy <aik@ozlabs.ru> writes: > On 29/10/2020 11:40, Michael Ellerman wrote: >> Alexey Kardashevskiy <aik@ozlabs.ru> writes: >>> @@ -1126,7 +1129,7 @@ static u64 enable_ddw(struct pci_dev *dev, struct device_node *pdn) >>> >>> mutex_lock(&direct_window_init_mutex); >>> >>> - dma_addr = find_existing_ddw(pdn); >>> + dma_addr = find_existing_ddw(pdn, &len); >> >> I don't see len used anywhere? >> >>> if (dma_addr != 0) >>> goto out_unlock; >>> >>> @@ -1212,14 +1215,26 @@ static u64 enable_ddw(struct pci_dev *dev, struct device_node *pdn) >>> } >>> /* verify the window * number of ptes will map the partition */ >>> /* check largest block * page size > max memory hotplug addr */ >>> - max_addr = ddw_memory_hotplug_max(); >>> - if (query.largest_available_block < (max_addr >> page_shift)) { >>> - dev_dbg(&dev->dev, "can't map partition max 0x%llx with %llu " >>> - "%llu-sized pages\n", max_addr, query.largest_available_block, >>> - 1ULL << page_shift); >>> + /* >>> + * The "ibm,pmemory" can appear anywhere in the address space. >>> + * Assuming it is still backed by page structs, try MAX_PHYSMEM_BITS >>> + * for the upper limit and fallback to max RAM otherwise but this >>> + * disables device::dma_ops_bypass. >>> + */ >>> + len = max_ram_len; >> >> Here you override whatever find_existing_ddw() wrote to len? > > Not always, there is a bunch of gotos before this line to the end of the > function and one (which returns the existing window) is legit. Thanks, Ah yep I see it. Gotos considered confusing ;) cheers ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-11-04 3:55 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-10-28 7:00 [PATCH kernel v3 0/2] DMA, powerpc/dma: Fallback to dma_ops when persistent memory present Alexey Kardashevskiy 2020-10-28 7:00 ` [PATCH kernel v3 1/2] dma: Allow mixing bypass and mapped DMA operation Alexey Kardashevskiy 2020-10-28 17:22 ` Christoph Hellwig 2020-10-28 23:15 ` Alexey Kardashevskiy 2020-10-28 7:00 ` [PATCH kernel v3 2/2] powerpc/dma: Fallback to dma_ops when persistent memory present Alexey Kardashevskiy 2020-10-29 0:40 ` Michael Ellerman 2020-10-29 0:46 ` Alexey Kardashevskiy 2020-10-29 9:55 ` Michael Ellerman
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).