Re: [RFC PATCH v4 04/21] x86/hpet: Add hpet_set_comparator() for periodic and one-shot modes
On Thu, 23 May 2019, Ricardo Neri wrote:
> +/**
> + * hpet_set_comparator() - Helper function for setting comparator register
> + * @num: The timer ID
> + * @cmp: The value to be written to the comparator/accumulator
> + * @period: The value to be written to the period (0 = oneshot mode)
> + *
> + * Helper function for updating comparator, accumulator and period values.
> + *
> + * In periodic mode, HPET needs HPET_TN_SETVAL to be set before writing
> + * to the Tn_CMP to update the accumulator. Then, HPET needs a second
> + * write (with HPET_TN_SETVAL cleared) to Tn_CMP to set the period.
> + * The HPET_TN_SETVAL bit is automatically cleared after the first write.
> + *
> + * For one-shot mode, HPET_TN_SETVAL does not need to be set.
> + *
> + * See the following documents:
> + * - Intel IA-PC HPET (High Precision Event Timers) Specification
> + * - AMD-8111 HyperTransport I/O Hub Data Sheet, Publication # 24674
> + */
> +void hpet_set_comparator(int num, unsigned int cmp, unsigned int period)
> +{
> + if (period) {
> + unsigned int v = hpet_readl(HPET_Tn_CFG(num));
> +
> + hpet_writel(v | HPET_TN_SETVAL, HPET_Tn_CFG(num));
> + }
> +
> + hpet_writel(cmp, HPET_Tn_CMP(num));
> +
> + if (!period)
> + return;
TBH, I hate this conditional handling. What's wrong with two functions?
> +
> + /*
> + * This delay is seldom used: never in one-shot mode and in periodic
> + * only when reprogramming the timer.
> + */
> + udelay(1);
> + hpet_writel(period, HPET_Tn_CMP(num));
> +}
> +EXPORT_SYMBOL_GPL(hpet_set_comparator);
Why is this exported? Which module user needs this?
Thanks,
tglx
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 7/7] arc: use the generic remapping allocator for coherent DMA allocations
Hi Christoph,
Regular question - do you have any public git repository with all this dma
changes?
I want to test it for ARC.
Pretty sure the
[PATCH 2/7] arc: remove the partial DMA_ATTR_NON_CONSISTENT support
is fine.
Not so sure about
[PATCH 7/7] arc: use the generic remapping allocator for coherent DMA
allocations
:)
On Fri, 2019-06-14 at 16:44 +0200, Christoph Hellwig wrote:
> Replace the code that sets up uncached PTEs with the generic vmap based
> remapping code. It also provides an atomic pool for allocations from
> non-blocking context, which we not properly supported by the existing
> arc code.
>
> Signed-off-by: Christoph Hellwig
> ---
> arch/arc/Kconfig | 2 ++
> arch/arc/mm/dma.c | 62 ---
> 2 files changed, 12 insertions(+), 52 deletions(-)
>
> diff --git a/arch/arc/Kconfig b/arch/arc/Kconfig
> index 23e063df5d2c..cdad7d30ff1d 100644
> --- a/arch/arc/Kconfig
> +++ b/arch/arc/Kconfig
> @@ -10,6 +10,7 @@ config ARC
> def_bool y
> select ARC_TIMERS
> select ARCH_HAS_DMA_COHERENT_TO_PFN
> + select ARCH_HAS_DMA_PREP_COHERENT
> select ARCH_HAS_PTE_SPECIAL
> select ARCH_HAS_SETUP_DMA_OPS
> select ARCH_HAS_SYNC_DMA_FOR_CPU
> @@ -19,6 +20,7 @@ config ARC
> select BUILDTIME_EXTABLE_SORT
> select CLONE_BACKWARDS
> select COMMON_CLK
> + select DMA_DIRECT_REMAP
> select GENERIC_ATOMIC64 if !ISA_ARCV2 || !(ARC_HAS_LL64 && ARC_HAS_LLSC)
> select GENERIC_CLOCKEVENTS
> select GENERIC_FIND_FIRST_BIT
> diff --git a/arch/arc/mm/dma.c b/arch/arc/mm/dma.c
> index 9832928f896d..0fa850709fac 100644
> --- a/arch/arc/mm/dma.c
> +++ b/arch/arc/mm/dma.c
> @@ -11,46 +11,15 @@
> #include
>
> /*
> - * ARCH specific callbacks for generic noncoherent DMA ops
> (dma/noncoherent.c)
> + * ARCH specific callbacks for generic noncoherent DMA ops
> * - hardware IOC not available (or "dma-coherent" not set for device in DT)
> * - But still handle both coherent and non-coherent requests from caller
> *
> * For DMA coherent hardware (IOC) generic code suffices
> */
> -void *arch_dma_alloc(struct device *dev, size_t size, dma_addr_t *dma_handle,
> - gfp_t gfp, unsigned long attrs)
> -{
> - unsigned long order = get_order(size);
> - struct page *page;
> - phys_addr_t paddr;
> - void *kvaddr;
> -
> - /*
> - * __GFP_HIGHMEM flag is cleared by upper layer functions
> - * (in include/linux/dma-mapping.h) so we should never get a
> - * __GFP_HIGHMEM here.
> - */
> - BUG_ON(gfp & __GFP_HIGHMEM);
> -
> - page = alloc_pages(gfp | __GFP_ZERO, order);
> - if (!page)
> - return NULL;
> -
> - /* This is linear addr (0x8000_ based) */
> - paddr = page_to_phys(page);
> -
> - *dma_handle = paddr;
> -
> - /*
> - * A coherent buffer needs MMU mapping to enforce non-cachability.
> - * kvaddr is kernel Virtual address (0x7000_ based).
> - */
> - kvaddr = ioremap_nocache(paddr, size);
> - if (kvaddr == NULL) {
> - __free_pages(page, order);
> - return NULL;
> - }
>
> +void arch_dma_prep_coherent(struct page *page, size_t size)
> +{
> /*
>* Evict any existing L1 and/or L2 lines for the backing page
>* in case it was used earlier as a normal "cached" page.
> @@ -61,24 +30,7 @@ void *arch_dma_alloc(struct device *dev, size_t size,
> dma_addr_t *dma_handle,
>* Currently flush_cache_vmap nukes the L1 cache completely which
>* will be optimized as a separate commit
>*/
> - dma_cache_wback_inv(paddr, size);
> - return kvaddr;
> -}
> -
> -void arch_dma_free(struct device *dev, size_t size, void *vaddr,
> - dma_addr_t dma_handle, unsigned long attrs)
> -{
> - phys_addr_t paddr = dma_handle;
> - struct page *page = virt_to_page(paddr);
> -
> - iounmap((void __force __iomem *)vaddr);
> - __free_pages(page, get_order(size));
> -}
> -
> -long arch_dma_coherent_to_pfn(struct device *dev, void *cpu_addr,
> - dma_addr_t dma_addr)
> -{
> - return __phys_to_pfn(dma_addr);
> + dma_cache_wback_inv(page_to_phys(page), size);
> }
>
> /*
> @@ -155,3 +107,9 @@ void arch_setup_dma_ops(struct device *dev, u64 dma_base,
> u64 size,
> dev_info(dev, "use %sncoherent DMA ops\n",
>dev->dma_coherent ? "" : "non");
> }
> +
> +static int __init atomic_pool_init(void)
> +{
> + return dma_atomic_pool_init(GFP_KERNEL, pgprot_noncached(PAGE_KERNEL));
> +}
> +postcore_initcall(atomic_pool_init);
--
Eugeniy Paltsev
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 01/10] iommu/exynos: convert to SPDX license tags
Need me to make them a patch? MBR, Yangtao ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [Intel-gfx] [PATCH 03/16] drm/i915: stop using drm_pci_alloc
On Fri, Jun 14, 2019 at 03:47:13PM +0200, Christoph Hellwig wrote:
> Remove usage of the legacy drm PCI DMA wrappers, and with that the
> incorrect usage cocktail of __GFP_COMP, virt_to_page on DMA allocation
> and SetPageReserved.
>
> Signed-off-by: Christoph Hellwig
> ---
> drivers/gpu/drm/i915/i915_gem.c| 30 +-
> drivers/gpu/drm/i915/i915_gem_object.h | 3 ++-
> drivers/gpu/drm/i915/intel_display.c | 2 +-
> 3 files changed, 18 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index ad01c92aaf74..8f2053c91aff 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -228,7 +228,6 @@ i915_gem_get_aperture_ioctl(struct drm_device *dev, void
> *data,
> static int i915_gem_object_get_pages_phys(struct drm_i915_gem_object *obj)
> {
> struct address_space *mapping = obj->base.filp->f_mapping;
> - drm_dma_handle_t *phys;
> struct sg_table *st;
> struct scatterlist *sg;
> char *vaddr;
> @@ -242,13 +241,13 @@ static int i915_gem_object_get_pages_phys(struct
> drm_i915_gem_object *obj)
>* to handle all possible callers, and given typical object sizes,
>* the alignment of the buddy allocation will naturally match.
>*/
> - phys = drm_pci_alloc(obj->base.dev,
> - roundup_pow_of_two(obj->base.size),
> - roundup_pow_of_two(obj->base.size));
> - if (!phys)
> + obj->phys_vaddr = dma_alloc_coherent(>base.dev->pdev->dev,
> + roundup_pow_of_two(obj->base.size),
> + >phys_handle, GFP_KERNEL);
> + if (!obj->phys_vaddr)
> return -ENOMEM;
>
> - vaddr = phys->vaddr;
> + vaddr = obj->phys_vaddr;
> for (i = 0; i < obj->base.size / PAGE_SIZE; i++) {
> struct page *page;
> char *src;
> @@ -286,18 +285,17 @@ static int i915_gem_object_get_pages_phys(struct
> drm_i915_gem_object *obj)
> sg->offset = 0;
> sg->length = obj->base.size;
>
> - sg_dma_address(sg) = phys->busaddr;
> + sg_dma_address(sg) = obj->phys_handle;
> sg_dma_len(sg) = obj->base.size;
>
> - obj->phys_handle = phys;
> -
> __i915_gem_object_set_pages(obj, st, sg->length);
>
> return 0;
>
> err_phys:
> - drm_pci_free(obj->base.dev, phys);
> -
> + dma_free_coherent(>base.dev->pdev->dev,
> + roundup_pow_of_two(obj->base.size), obj->phys_vaddr,
> + obj->phys_handle);
Need to undo the damage to obj->phys_vaddr here since
i915_gem_pwrite_ioctl() will now use that to determine if it's
dealing with a phys obj.
> return err;
> }
>
> @@ -335,7 +333,7 @@ i915_gem_object_put_pages_phys(struct drm_i915_gem_object
> *obj,
>
> if (obj->mm.dirty) {
> struct address_space *mapping = obj->base.filp->f_mapping;
> - char *vaddr = obj->phys_handle->vaddr;
> + char *vaddr = obj->phys_vaddr;
> int i;
>
> for (i = 0; i < obj->base.size / PAGE_SIZE; i++) {
> @@ -363,7 +361,9 @@ i915_gem_object_put_pages_phys(struct drm_i915_gem_object
> *obj,
> sg_free_table(pages);
> kfree(pages);
>
> - drm_pci_free(obj->base.dev, obj->phys_handle);
> + dma_free_coherent(>base.dev->pdev->dev,
> + roundup_pow_of_two(obj->base.size), obj->phys_vaddr,
> + obj->phys_handle);
This one is fine I think since the object remains a phys obj once
turned into one. At least the current code isn't clearing
phys_handle here. But my memory is a bit hazy on the details. Chris?
Also maybe s/phys_handle/phys_busaddr/ all over?
> }
>
> static void
> @@ -603,7 +603,7 @@ i915_gem_phys_pwrite(struct drm_i915_gem_object *obj,
>struct drm_i915_gem_pwrite *args,
>struct drm_file *file)
> {
> - void *vaddr = obj->phys_handle->vaddr + args->offset;
> + void *vaddr = obj->phys_vaddr + args->offset;
> char __user *user_data = u64_to_user_ptr(args->data_ptr);
>
> /* We manually control the domain here and pretend that it
> @@ -1431,7 +1431,7 @@ i915_gem_pwrite_ioctl(struct drm_device *dev, void
> *data,
> ret = i915_gem_gtt_pwrite_fast(obj, args);
>
> if (ret == -EFAULT || ret == -ENOSPC) {
> - if (obj->phys_handle)
> + if (obj->phys_vaddr)
> ret = i915_gem_phys_pwrite(obj, args, file);
> else
> ret = i915_gem_shmem_pwrite(obj, args);
> diff --git a/drivers/gpu/drm/i915/i915_gem_object.h
> b/drivers/gpu/drm/i915/i915_gem_object.h
> index ca93a40c0c87..14bd2d61d0f6 100644
> --- a/drivers/gpu/drm/i915/i915_gem_object.h
> +++ b/drivers/gpu/drm/i915/i915_gem_object.h
> @@ -290,7 +290,8 @@ struct drm_i915_gem_object {
> };
>
> /** for phys allocated objects
Re: [git pull] IOMMU Fixes for Linux v5.2-rc4
The pull request you sent on Fri, 14 Jun 2019 11:39:15 +0200: > git://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git > tags/iommu-fixes-v5.2-rc4 has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/c78ad1be4b4d65eb214421b90a788abf3c85c3ea Thank you! -- Deet-doot-dot, I am a bot. https://korg.wiki.kernel.org/userdoc/prtracker ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH v4 05/21] x86/hpet: Reserve timer for the HPET hardlockup detector
On Thu, 13 Jun 2019, Ricardo Neri wrote: > On Tue, Jun 11, 2019 at 09:54:25PM +0200, Thomas Gleixner wrote: > > On Thu, 23 May 2019, Ricardo Neri wrote: > > > > > HPET timer 2 will be used to drive the HPET-based hardlockup detector. > > > Reserve such timer to ensure it cannot be used by user space programs or > > > for clock events. > > > > > > When looking for MSI-capable timers for clock events, skip timer 2 if > > > the HPET hardlockup detector is selected. > > > > Why? Both the changelog and the code change lack an explanation why this > > timer is actually touched after it got reserved for the platform. The > > reservation should make it inaccessible for other things. > > hpet_reserve_platform_timers() will give the HPET char driver a data > structure which specifies which drivers are reserved. In this manner, > they cannot be used by applications via file opens. The timer used by > the hardlockup detector should be marked as reserved. > > Also, hpet_msi_capability_lookup() populates another data structure > which is used when obtaining an unused timer for a HPET clock event. > The timer used by the hardlockup detector should not be included in such > data structure. > > Is this the explanation you would like to see? If yes, I will include it > in the changelog. Yes, the explanation makes sense. The code still sucks. Not really your fault, but this is not making it any better. What bothers me most is the fact that CONFIG_X86_HARDLOCKUP_DETECTOR_HPET removes one HPET timer unconditionally. It neither checks whether the hpet watchdog is actually enabled on the command line, nor does it validate upfront whether the HPET supports FSB delivery. That wastes an HPET timer unconditionally for no value. Not that I personally care much about /dev/hpet, but some older laptops depend on HPET per cpu timers as the local APIC timer stops in C2/3. So this unconditional reservation will cause regressions for no reason. The proper approach here is to: 1) Evaluate the command line _before_ hpet_enable() is invoked 2) Check the availability of FSB delivery in hpet_enable() Reserve an HPET channel for the watchdog only when #1 and #2 are true. Thanks, tglx
Re: [RFC PATCH v4 03/21] x86/hpet: Calculate ticks-per-second in a separate function
On Fri, 14 Jun 2019, Thomas Gleixner wrote:
> On Thu, 23 May 2019, Ricardo Neri wrote:
> > int hpet_alloc(struct hpet_data *hdp)
> > {
> > u64 cap, mcfg;
> > @@ -844,7 +867,6 @@ int hpet_alloc(struct hpet_data *hdp)
> > struct hpets *hpetp;
> > struct hpet __iomem *hpet;
> > static struct hpets *last;
> > - unsigned long period;
> > unsigned long long temp;
> > u32 remainder;
> >
> > @@ -894,12 +916,7 @@ int hpet_alloc(struct hpet_data *hdp)
> >
> > last = hpetp;
> >
> > - period = (cap & HPET_COUNTER_CLK_PERIOD_MASK) >>
> > - HPET_COUNTER_CLK_PERIOD_SHIFT; /* fs, 10^-15 */
> > - temp = 1000uLL; /* 10^15 femtoseconds per second */
> > - temp += period >> 1; /* round */
> > - do_div(temp, period);
> > - hpetp->hp_tick_freq = temp; /* ticks per second */
> > + hpetp->hp_tick_freq = hpet_get_ticks_per_sec(cap);
>
> Why are we actually computing this over and over?
>
> In hpet_enable() which is the first function invoked we have:
>
> /*
> * The period is a femto seconds value. Convert it to a
> * frequency.
> */
> freq = FSEC_PER_SEC;
> do_div(freq, hpet_period);
> hpet_freq = freq;
>
> So we already have ticks per second, aka frequency, right? So why do we
> need yet another function instead of using the value which is computed
> once? The frequency of the HPET channels has to be identical no matter
> what. If it's not HPET is broken beyond repair.
Aside of that this change breaks the IA64 support for /dev/hpet.
Thanks,
tglx
Re: [RFC PATCH v4 03/21] x86/hpet: Calculate ticks-per-second in a separate function
On Thu, 23 May 2019, Ricardo Neri wrote:
>
> +u64 hpet_get_ticks_per_sec(u64 hpet_caps)
> +{
> + u64 ticks_per_sec, period;
> +
> + period = (hpet_caps & HPET_COUNTER_CLK_PERIOD_MASK) >>
> + HPET_COUNTER_CLK_PERIOD_SHIFT; /* fs, 10^-15 */
> +
> + /*
> + * The frequency is the reciprocal of the period. The period is given
> + * in femtoseconds per second. Thus, prepare a dividend to obtain the
> + * frequency in ticks per second.
> + */
> +
> + /* 10^15 femtoseconds per second */
> + ticks_per_sec = 1000ULL;
> + ticks_per_sec += period >> 1; /* round */
> +
> + /* The quotient is put in the dividend. We drop the remainder. */
> + do_div(ticks_per_sec, period);
> +
> + return ticks_per_sec;
> +}
> +
> int hpet_alloc(struct hpet_data *hdp)
> {
> u64 cap, mcfg;
> @@ -844,7 +867,6 @@ int hpet_alloc(struct hpet_data *hdp)
> struct hpets *hpetp;
> struct hpet __iomem *hpet;
> static struct hpets *last;
> - unsigned long period;
> unsigned long long temp;
> u32 remainder;
>
> @@ -894,12 +916,7 @@ int hpet_alloc(struct hpet_data *hdp)
>
> last = hpetp;
>
> - period = (cap & HPET_COUNTER_CLK_PERIOD_MASK) >>
> - HPET_COUNTER_CLK_PERIOD_SHIFT; /* fs, 10^-15 */
> - temp = 1000uLL; /* 10^15 femtoseconds per second */
> - temp += period >> 1; /* round */
> - do_div(temp, period);
> - hpetp->hp_tick_freq = temp; /* ticks per second */
> + hpetp->hp_tick_freq = hpet_get_ticks_per_sec(cap);
Why are we actually computing this over and over?
In hpet_enable() which is the first function invoked we have:
/*
* The period is a femto seconds value. Convert it to a
* frequency.
*/
freq = FSEC_PER_SEC;
do_div(freq, hpet_period);
hpet_freq = freq;
So we already have ticks per second, aka frequency, right? So why do we
need yet another function instead of using the value which is computed
once? The frequency of the HPET channels has to be identical no matter
what. If it's not HPET is broken beyond repair.
Thanks,
tglx
Re: [PATCH 12/16] staging/comedi: mark as broken
On Fri, Jun 14, 2019 at 05:30:32PM +0200, Greg KH wrote: > On Fri, Jun 14, 2019 at 04:48:57PM +0200, Christoph Hellwig wrote: > > On Fri, Jun 14, 2019 at 04:02:39PM +0200, Greg KH wrote: > > > Perhaps a hint as to how we can fix this up? This is the first time > > > I've heard of the comedi code not handling dma properly. > > > > It can be fixed by: > > > > a) never calling virt_to_page (or vmalloc_to_page for that matter) > > on dma allocation > > b) never remapping dma allocation with conflicting cache modes > > (no remapping should be doable after a) anyway). > > Ok, fair enough, have any pointers of drivers/core code that does this > correctly? I can put it on my todo list, but might take a week or so... Just about everyone else. They just need to remove the vmap and either do one large allocation, or live with the fact that they need helpers to access multiple array elements instead of one net vmap, which most of the users already seem to do anyway, with just a few using the vmap (which might explain why we didn't see blowups yet). ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 12/16] staging/comedi: mark as broken
On Fri, Jun 14, 2019 at 04:48:57PM +0200, Christoph Hellwig wrote: > On Fri, Jun 14, 2019 at 04:02:39PM +0200, Greg KH wrote: > > Perhaps a hint as to how we can fix this up? This is the first time > > I've heard of the comedi code not handling dma properly. > > It can be fixed by: > > a) never calling virt_to_page (or vmalloc_to_page for that matter) > on dma allocation > b) never remapping dma allocation with conflicting cache modes > (no remapping should be doable after a) anyway). Ok, fair enough, have any pointers of drivers/core code that does this correctly? I can put it on my todo list, but might take a week or so... Ian, want to look into doing this sooner? thanks, greg k-h ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH 16/16] dma-mapping: use exact allocation in dma_alloc_contiguous
From: Robin Murphy > Sent: 14 June 2019 16:06 ... > Well, apart from the bit in DMA-API-HOWTO which has said this since > forever (well, before Git history, at least): > > "The CPU virtual address and the DMA address are both > guaranteed to be aligned to the smallest PAGE_SIZE order which > is greater than or equal to the requested size. This invariant > exists (for example) to guarantee that if you allocate a chunk > which is smaller than or equal to 64 kilobytes, the extent of the > buffer you receive will not cross a 64K boundary." I knew it was somewhere :-) Interestingly that also implies that the address returned for a size of (say) 128 will also be page aligned. In that case 128 byte alignment should probably be ok - but it is still an API change that could have horrid consequences. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales) ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 16/16] dma-mapping: use exact allocation in dma_alloc_contiguous
On Fri, Jun 14, 2019 at 04:05:33PM +0100, Robin Murphy wrote: > That said, I don't believe this particular patch should make any > appreciable difference - alloc_pages_exact() is still going to give back > the same base address as the rounded up over-allocation would, and > PAGE_ALIGN()ing the size passed to get_order() already seemed to be > pointless. True, we actually do get the right alignment just about anywhere. Not 100% sure about the various static pool implementations, but we can make sure if any didn't we'll do that right thing once those get consolidated. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 16/16] dma-mapping: use exact allocation in dma_alloc_contiguous
On Fri, Jun 14, 2019 at 03:01:22PM +, David Laight wrote: > I'm pretty sure there is a lot of code out there that makes that assumption. > Without it many drivers will have to allocate almost double the > amount of memory they actually need in order to get the required alignment. > So instead of saving memory you'll actually make more be used. That code would already be broken on a lot of Linux platforms. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 16/16] dma-mapping: use exact allocation in dma_alloc_contiguous
On 14/06/2019 15:50, 'Christoph Hellwig' wrote: On Fri, Jun 14, 2019 at 02:15:44PM +, David Laight wrote: Does this still guarantee that requests for 16k will not cross a 16k boundary? It looks like you are losing the alignment parameter. The DMA API never gave you alignment guarantees to start with, and you can get not naturally aligned memory from many of our current implementations. Well, apart from the bit in DMA-API-HOWTO which has said this since forever (well, before Git history, at least): "The CPU virtual address and the DMA address are both guaranteed to be aligned to the smallest PAGE_SIZE order which is greater than or equal to the requested size. This invariant exists (for example) to guarantee that if you allocate a chunk which is smaller than or equal to 64 kilobytes, the extent of the buffer you receive will not cross a 64K boundary." That said, I don't believe this particular patch should make any appreciable difference - alloc_pages_exact() is still going to give back the same base address as the rounded up over-allocation would, and PAGE_ALIGN()ing the size passed to get_order() already seemed to be pointless. Robin. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH 16/16] dma-mapping: use exact allocation in dma_alloc_contiguous
From: 'Christoph Hellwig' > Sent: 14 June 2019 15:50 > To: David Laight > On Fri, Jun 14, 2019 at 02:15:44PM +, David Laight wrote: > > Does this still guarantee that requests for 16k will not cross a 16k > > boundary? > > It looks like you are losing the alignment parameter. > > The DMA API never gave you alignment guarantees to start with, > and you can get not naturally aligned memory from many of our > current implementations. Hmmm... I thought that was even documented. I'm pretty sure there is a lot of code out there that makes that assumption. Without it many drivers will have to allocate almost double the amount of memory they actually need in order to get the required alignment. So instead of saving memory you'll actually make more be used. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales) ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 16/16] dma-mapping: use exact allocation in dma_alloc_contiguous
On Fri, Jun 14, 2019 at 02:15:44PM +, David Laight wrote: > Does this still guarantee that requests for 16k will not cross a 16k boundary? > It looks like you are losing the alignment parameter. The DMA API never gave you alignment guarantees to start with, and you can get not naturally aligned memory from many of our current implementations. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 12/16] staging/comedi: mark as broken
On Fri, Jun 14, 2019 at 04:02:39PM +0200, Greg KH wrote: > Perhaps a hint as to how we can fix this up? This is the first time > I've heard of the comedi code not handling dma properly. It can be fixed by: a) never calling virt_to_page (or vmalloc_to_page for that matter) on dma allocation b) never remapping dma allocation with conflicting cache modes (no remapping should be doable after a) anyway). ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 7/7] arc: use the generic remapping allocator for coherent DMA allocations
Replace the code that sets up uncached PTEs with the generic vmap based
remapping code. It also provides an atomic pool for allocations from
non-blocking context, which we not properly supported by the existing
arc code.
Signed-off-by: Christoph Hellwig
---
arch/arc/Kconfig | 2 ++
arch/arc/mm/dma.c | 62 ---
2 files changed, 12 insertions(+), 52 deletions(-)
diff --git a/arch/arc/Kconfig b/arch/arc/Kconfig
index 23e063df5d2c..cdad7d30ff1d 100644
--- a/arch/arc/Kconfig
+++ b/arch/arc/Kconfig
@@ -10,6 +10,7 @@ config ARC
def_bool y
select ARC_TIMERS
select ARCH_HAS_DMA_COHERENT_TO_PFN
+ select ARCH_HAS_DMA_PREP_COHERENT
select ARCH_HAS_PTE_SPECIAL
select ARCH_HAS_SETUP_DMA_OPS
select ARCH_HAS_SYNC_DMA_FOR_CPU
@@ -19,6 +20,7 @@ config ARC
select BUILDTIME_EXTABLE_SORT
select CLONE_BACKWARDS
select COMMON_CLK
+ select DMA_DIRECT_REMAP
select GENERIC_ATOMIC64 if !ISA_ARCV2 || !(ARC_HAS_LL64 && ARC_HAS_LLSC)
select GENERIC_CLOCKEVENTS
select GENERIC_FIND_FIRST_BIT
diff --git a/arch/arc/mm/dma.c b/arch/arc/mm/dma.c
index 9832928f896d..0fa850709fac 100644
--- a/arch/arc/mm/dma.c
+++ b/arch/arc/mm/dma.c
@@ -11,46 +11,15 @@
#include
/*
- * ARCH specific callbacks for generic noncoherent DMA ops (dma/noncoherent.c)
+ * ARCH specific callbacks for generic noncoherent DMA ops
* - hardware IOC not available (or "dma-coherent" not set for device in DT)
* - But still handle both coherent and non-coherent requests from caller
*
* For DMA coherent hardware (IOC) generic code suffices
*/
-void *arch_dma_alloc(struct device *dev, size_t size, dma_addr_t *dma_handle,
- gfp_t gfp, unsigned long attrs)
-{
- unsigned long order = get_order(size);
- struct page *page;
- phys_addr_t paddr;
- void *kvaddr;
-
- /*
-* __GFP_HIGHMEM flag is cleared by upper layer functions
-* (in include/linux/dma-mapping.h) so we should never get a
-* __GFP_HIGHMEM here.
-*/
- BUG_ON(gfp & __GFP_HIGHMEM);
-
- page = alloc_pages(gfp | __GFP_ZERO, order);
- if (!page)
- return NULL;
-
- /* This is linear addr (0x8000_ based) */
- paddr = page_to_phys(page);
-
- *dma_handle = paddr;
-
- /*
-* A coherent buffer needs MMU mapping to enforce non-cachability.
-* kvaddr is kernel Virtual address (0x7000_ based).
-*/
- kvaddr = ioremap_nocache(paddr, size);
- if (kvaddr == NULL) {
- __free_pages(page, order);
- return NULL;
- }
+void arch_dma_prep_coherent(struct page *page, size_t size)
+{
/*
* Evict any existing L1 and/or L2 lines for the backing page
* in case it was used earlier as a normal "cached" page.
@@ -61,24 +30,7 @@ void *arch_dma_alloc(struct device *dev, size_t size,
dma_addr_t *dma_handle,
* Currently flush_cache_vmap nukes the L1 cache completely which
* will be optimized as a separate commit
*/
- dma_cache_wback_inv(paddr, size);
- return kvaddr;
-}
-
-void arch_dma_free(struct device *dev, size_t size, void *vaddr,
- dma_addr_t dma_handle, unsigned long attrs)
-{
- phys_addr_t paddr = dma_handle;
- struct page *page = virt_to_page(paddr);
-
- iounmap((void __force __iomem *)vaddr);
- __free_pages(page, get_order(size));
-}
-
-long arch_dma_coherent_to_pfn(struct device *dev, void *cpu_addr,
- dma_addr_t dma_addr)
-{
- return __phys_to_pfn(dma_addr);
+ dma_cache_wback_inv(page_to_phys(page), size);
}
/*
@@ -155,3 +107,9 @@ void arch_setup_dma_ops(struct device *dev, u64 dma_base,
u64 size,
dev_info(dev, "use %sncoherent DMA ops\n",
dev->dma_coherent ? "" : "non");
}
+
+static int __init atomic_pool_init(void)
+{
+ return dma_atomic_pool_init(GFP_KERNEL, pgprot_noncached(PAGE_KERNEL));
+}
+postcore_initcall(atomic_pool_init);
--
2.20.1
[PATCH 5/7] dma-direct: handle DMA_ATTR_NON_CONSISTENT in common code
Only call into arch_dma_alloc if we require an uncached mapping,
and remove the parisc code manually doing normal cached
DMA_ATTR_NON_CONSISTENT allocations.
Signed-off-by: Christoph Hellwig
---
arch/parisc/kernel/pci-dma.c | 48 ++--
kernel/dma/direct.c | 4 +--
2 files changed, 15 insertions(+), 37 deletions(-)
diff --git a/arch/parisc/kernel/pci-dma.c b/arch/parisc/kernel/pci-dma.c
index 239162355b58..ca35d9a76e50 100644
--- a/arch/parisc/kernel/pci-dma.c
+++ b/arch/parisc/kernel/pci-dma.c
@@ -394,17 +394,20 @@ pcxl_dma_init(void)
__initcall(pcxl_dma_init);
-static void *pcxl_dma_alloc(struct device *dev, size_t size,
- dma_addr_t *dma_handle, gfp_t flag, unsigned long attrs)
+void *arch_dma_alloc(struct device *dev, size_t size,
+ dma_addr_t *dma_handle, gfp_t gfp, unsigned long attrs)
{
unsigned long vaddr;
unsigned long paddr;
int order;
+ if (boot_cpu_data.cpu_type != pcxl2 && boot_cpu_data.cpu_type != pcxl)
+ return NULL;
+
order = get_order(size);
size = 1 << (order + PAGE_SHIFT);
vaddr = pcxl_alloc_range(size);
- paddr = __get_free_pages(flag | __GFP_ZERO, order);
+ paddr = __get_free_pages(gfp | __GFP_ZERO, order);
flush_kernel_dcache_range(paddr, size);
paddr = __pa(paddr);
map_uncached_pages(vaddr, size, paddr);
@@ -421,44 +424,19 @@ static void *pcxl_dma_alloc(struct device *dev, size_t
size,
return (void *)vaddr;
}
-static void *pcx_dma_alloc(struct device *dev, size_t size,
- dma_addr_t *dma_handle, gfp_t flag, unsigned long attrs)
-{
- void *addr;
-
- if ((attrs & DMA_ATTR_NON_CONSISTENT) == 0)
- return NULL;
-
- addr = (void *)__get_free_pages(flag | __GFP_ZERO, get_order(size));
- if (addr)
- *dma_handle = (dma_addr_t)virt_to_phys(addr);
-
- return addr;
-}
-
-void *arch_dma_alloc(struct device *dev, size_t size,
- dma_addr_t *dma_handle, gfp_t gfp, unsigned long attrs)
-{
-
- if (boot_cpu_data.cpu_type == pcxl2 || boot_cpu_data.cpu_type == pcxl)
- return pcxl_dma_alloc(dev, size, dma_handle, gfp, attrs);
- else
- return pcx_dma_alloc(dev, size, dma_handle, gfp, attrs);
-}
-
void arch_dma_free(struct device *dev, size_t size, void *vaddr,
dma_addr_t dma_handle, unsigned long attrs)
{
int order = get_order(size);
- if (boot_cpu_data.cpu_type == pcxl2 || boot_cpu_data.cpu_type == pcxl) {
- size = 1 << (order + PAGE_SHIFT);
- unmap_uncached_pages((unsigned long)vaddr, size);
- pcxl_free_range((unsigned long)vaddr, size);
+ WARN_ON_ONCE(boot_cpu_data.cpu_type != pcxl2 &&
+boot_cpu_data.cpu_type != pcxl);
- vaddr = __va(dma_handle);
- }
- free_pages((unsigned long)vaddr, get_order(size));
+ size = 1 << (order + PAGE_SHIFT);
+ unmap_uncached_pages((unsigned long)vaddr, size);
+ pcxl_free_range((unsigned long)vaddr, size);
+
+ free_pages((unsigned long)__va(dma_handle), order);
}
void arch_sync_dma_for_device(struct device *dev, phys_addr_t paddr,
diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index c2893713bf80..fc354f4f490b 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -191,7 +191,7 @@ void *dma_direct_alloc(struct device *dev, size_t size,
dma_addr_t *dma_handle, gfp_t gfp, unsigned long attrs)
{
if (!IS_ENABLED(CONFIG_ARCH_HAS_UNCACHED_SEGMENT) &&
- !dev_is_dma_coherent(dev))
+ dma_alloc_need_uncached(dev, attrs))
return arch_dma_alloc(dev, size, dma_handle, gfp, attrs);
return dma_direct_alloc_pages(dev, size, dma_handle, gfp, attrs);
}
@@ -200,7 +200,7 @@ void dma_direct_free(struct device *dev, size_t size,
void *cpu_addr, dma_addr_t dma_addr, unsigned long attrs)
{
if (!IS_ENABLED(CONFIG_ARCH_HAS_UNCACHED_SEGMENT) &&
- !dev_is_dma_coherent(dev))
+ dma_alloc_need_uncached(dev, attrs))
arch_dma_free(dev, size, cpu_addr, dma_addr, attrs);
else
dma_direct_free_pages(dev, size, cpu_addr, dma_addr, attrs);
--
2.20.1
[PATCH 6/7] dma-direct: handle DMA_ATTR_NO_KERNEL_MAPPING in common code
DMA_ATTR_NO_KERNEL_MAPPING is generally implemented by allocating
normal cacheable pages or CMA memory, and then returning the page
pointer as the opaque handle. Lift that code from the xtensa and
generic dma remapping implementations into the generic dma-direct
code so that we don't even call arch_dma_alloc for these allocations.
Signed-off-by: Christoph Hellwig
---
arch/xtensa/kernel/pci-dma.c| 8 +---
include/linux/dma-noncoherent.h | 2 ++
kernel/dma/direct.c | 14 ++
kernel/dma/remap.c | 13 ++---
4 files changed, 19 insertions(+), 18 deletions(-)
diff --git a/arch/xtensa/kernel/pci-dma.c b/arch/xtensa/kernel/pci-dma.c
index 9171bff76fc4..206771277dff 100644
--- a/arch/xtensa/kernel/pci-dma.c
+++ b/arch/xtensa/kernel/pci-dma.c
@@ -167,10 +167,6 @@ void *arch_dma_alloc(struct device *dev, size_t size,
dma_addr_t *handle,
*handle = phys_to_dma(dev, page_to_phys(page));
- if (attrs & DMA_ATTR_NO_KERNEL_MAPPING) {
- return page;
- }
-
#ifdef CONFIG_MMU
if (PageHighMem(page)) {
void *p;
@@ -196,9 +192,7 @@ void arch_dma_free(struct device *dev, size_t size, void
*vaddr,
unsigned long count = PAGE_ALIGN(size) >> PAGE_SHIFT;
struct page *page;
- if (attrs & DMA_ATTR_NO_KERNEL_MAPPING) {
- page = vaddr;
- } else if (platform_vaddr_uncached(vaddr)) {
+ if (platform_vaddr_uncached(vaddr)) {
page = virt_to_page(platform_vaddr_to_cached(vaddr));
} else {
#ifdef CONFIG_MMU
diff --git a/include/linux/dma-noncoherent.h b/include/linux/dma-noncoherent.h
index 732919ac5c11..53ee36ecdf37 100644
--- a/include/linux/dma-noncoherent.h
+++ b/include/linux/dma-noncoherent.h
@@ -28,6 +28,8 @@ static inline bool dma_alloc_need_uncached(struct device *dev,
{
if (dev_is_dma_coherent(dev))
return false;
+ if (attrs & DMA_ATTR_NO_KERNEL_MAPPING)
+ return false;
if (IS_ENABLED(CONFIG_DMA_NONCOHERENT_CACHE_SYNC) &&
(attrs & DMA_ATTR_NON_CONSISTENT))
return false;
diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index fc354f4f490b..b90e1aede743 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -138,6 +138,14 @@ void *dma_direct_alloc_pages(struct device *dev, size_t
size,
if (!page)
return NULL;
+ if (attrs & DMA_ATTR_NO_KERNEL_MAPPING) {
+ /* remove any dirty cache lines on the kernel alias */
+ if (!PageHighMem(page))
+ arch_dma_prep_coherent(page, size);
+ /* return the page pointer as the opaque cookie */
+ return page;
+ }
+
if (PageHighMem(page)) {
/*
* Depending on the cma= arguments and per-arch setup
@@ -178,6 +186,12 @@ void dma_direct_free_pages(struct device *dev, size_t
size, void *cpu_addr,
{
unsigned int page_order = get_order(size);
+ if (attrs & DMA_ATTR_NO_KERNEL_MAPPING) {
+ /* cpu_addr is a struct page cookie, not a kernel address */
+ __dma_direct_free_pages(dev, size, cpu_addr);
+ return;
+ }
+
if (force_dma_unencrypted())
set_memory_encrypted((unsigned long)cpu_addr, 1 << page_order);
diff --git a/kernel/dma/remap.c b/kernel/dma/remap.c
index 0207e3764d52..a594aec07882 100644
--- a/kernel/dma/remap.c
+++ b/kernel/dma/remap.c
@@ -202,8 +202,7 @@ void *arch_dma_alloc(struct device *dev, size_t size,
dma_addr_t *dma_handle,
size = PAGE_ALIGN(size);
- if (!gfpflags_allow_blocking(flags) &&
- !(attrs & DMA_ATTR_NO_KERNEL_MAPPING)) {
+ if (!gfpflags_allow_blocking(flags)) {
ret = dma_alloc_from_pool(size, , flags);
if (!ret)
return NULL;
@@ -217,11 +216,6 @@ void *arch_dma_alloc(struct device *dev, size_t size,
dma_addr_t *dma_handle,
/* remove any dirty cache lines on the kernel alias */
arch_dma_prep_coherent(page, size);
- if (attrs & DMA_ATTR_NO_KERNEL_MAPPING) {
- ret = page; /* opaque cookie */
- goto done;
- }
-
/* create a coherent mapping */
ret = dma_common_contiguous_remap(page, size, VM_USERMAP,
arch_dma_mmap_pgprot(dev, PAGE_KERNEL, attrs),
@@ -240,10 +234,7 @@ void *arch_dma_alloc(struct device *dev, size_t size,
dma_addr_t *dma_handle,
void arch_dma_free(struct device *dev, size_t size, void *vaddr,
dma_addr_t dma_handle, unsigned long attrs)
{
- if (attrs & DMA_ATTR_NO_KERNEL_MAPPING) {
- /* vaddr is a struct page cookie, not a kernel address */
- __dma_direct_free_pages(dev, size, vaddr);
- } else if (!dma_free_from_pool(vaddr, PAGE_ALIGN(size))) {
+ if (!dma_free_from_pool(vaddr, PAGE_ALIGN(size))) {
[PATCH 4/7] dma-mapping: add a dma_alloc_need_uncached helper
Check if we need to allocate uncached memory for a device given the
allocation flags. Switch over the uncached segment check to this helper
to deal with architectures that do not support the dma_cache_sync
operation and thus should not returned cacheable memory for
DMA_ATTR_NON_CONSISTENT allocations.
Signed-off-by: Christoph Hellwig
---
include/linux/dma-noncoherent.h | 14 ++
kernel/dma/direct.c | 4 ++--
2 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/include/linux/dma-noncoherent.h b/include/linux/dma-noncoherent.h
index 7e0126a04e02..732919ac5c11 100644
--- a/include/linux/dma-noncoherent.h
+++ b/include/linux/dma-noncoherent.h
@@ -20,6 +20,20 @@ static inline bool dev_is_dma_coherent(struct device *dev)
}
#endif /* CONFIG_ARCH_HAS_DMA_COHERENCE_H */
+/*
+ * Check if an allocation needs to be marked uncached to be coherent.
+ */
+static inline bool dma_alloc_need_uncached(struct device *dev,
+ unsigned long attrs)
+{
+ if (dev_is_dma_coherent(dev))
+ return false;
+ if (IS_ENABLED(CONFIG_DMA_NONCOHERENT_CACHE_SYNC) &&
+ (attrs & DMA_ATTR_NON_CONSISTENT))
+ return false;
+ return true;
+}
+
void *arch_dma_alloc(struct device *dev, size_t size, dma_addr_t *dma_handle,
gfp_t gfp, unsigned long attrs);
void arch_dma_free(struct device *dev, size_t size, void *cpu_addr,
diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index b67f0aa08aa3..c2893713bf80 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -160,7 +160,7 @@ void *dma_direct_alloc_pages(struct device *dev, size_t
size,
memset(ret, 0, size);
if (IS_ENABLED(CONFIG_ARCH_HAS_UNCACHED_SEGMENT) &&
- !dev_is_dma_coherent(dev) && !(attrs & DMA_ATTR_NON_CONSISTENT)) {
+ dma_alloc_need_uncached(dev, attrs)) {
arch_dma_prep_coherent(page, size);
ret = uncached_kernel_address(ret);
}
@@ -182,7 +182,7 @@ void dma_direct_free_pages(struct device *dev, size_t size,
void *cpu_addr,
set_memory_encrypted((unsigned long)cpu_addr, 1 << page_order);
if (IS_ENABLED(CONFIG_ARCH_HAS_UNCACHED_SEGMENT) &&
- !dev_is_dma_coherent(dev) && !(attrs & DMA_ATTR_NON_CONSISTENT))
+ dma_alloc_need_uncached(dev, attrs))
cpu_addr = cached_kernel_address(cpu_addr);
__dma_direct_free_pages(dev, size, virt_to_page(cpu_addr));
}
--
2.20.1
[PATCH 3/7] openrisc: remove the partial DMA_ATTR_NON_CONSISTENT support
The openrisc DMA code supports DMA_ATTR_NON_CONSISTENT allocations, but
does not provide a cache_sync operation. This means any user of it
will never be able to actually transfer cache ownership and thus cause
coherency bugs.
Signed-off-by: Christoph Hellwig
---
arch/openrisc/kernel/dma.c | 22 +-
1 file changed, 9 insertions(+), 13 deletions(-)
diff --git a/arch/openrisc/kernel/dma.c b/arch/openrisc/kernel/dma.c
index f79457cb3741..9f25fd0fbb5d 100644
--- a/arch/openrisc/kernel/dma.c
+++ b/arch/openrisc/kernel/dma.c
@@ -98,15 +98,13 @@ arch_dma_alloc(struct device *dev, size_t size, dma_addr_t
*dma_handle,
va = (unsigned long)page;
- if ((attrs & DMA_ATTR_NON_CONSISTENT) == 0) {
- /*
-* We need to iterate through the pages, clearing the dcache for
-* them and setting the cache-inhibit bit.
-*/
- if (walk_page_range(va, va + size, )) {
- free_pages_exact(page, size);
- return NULL;
- }
+ /*
+* We need to iterate through the pages, clearing the dcache for
+* them and setting the cache-inhibit bit.
+*/
+ if (walk_page_range(va, va + size, )) {
+ free_pages_exact(page, size);
+ return NULL;
}
return (void *)va;
@@ -122,10 +120,8 @@ arch_dma_free(struct device *dev, size_t size, void *vaddr,
.mm = _mm
};
- if ((attrs & DMA_ATTR_NON_CONSISTENT) == 0) {
- /* walk_page_range shouldn't be able to fail here */
- WARN_ON(walk_page_range(va, va + size, ));
- }
+ /* walk_page_range shouldn't be able to fail here */
+ WARN_ON(walk_page_range(va, va + size, ));
free_pages_exact(vaddr, size);
}
--
2.20.1
[PATCH 1/7] arm-nommu: remove the partial DMA_ATTR_NON_CONSISTENT support
The arm-nommu DMA code supports DMA_ATTR_NON_CONSISTENT allocations, but
does not provide a cache_sync operation. This means any user of it
will never be able to actually transfer cache ownership and thus cause
coherency bugs.
Signed-off-by: Christoph Hellwig
---
arch/arm/mm/dma-mapping-nommu.c | 24 +++-
1 file changed, 3 insertions(+), 21 deletions(-)
diff --git a/arch/arm/mm/dma-mapping-nommu.c b/arch/arm/mm/dma-mapping-nommu.c
index f304b10e23a4..bc003df45546 100644
--- a/arch/arm/mm/dma-mapping-nommu.c
+++ b/arch/arm/mm/dma-mapping-nommu.c
@@ -39,18 +39,7 @@ static void *arm_nommu_dma_alloc(struct device *dev, size_t
size,
unsigned long attrs)
{
- void *ret;
-
- /*
-* Try generic allocator first if we are advertised that
-* consistency is not required.
-*/
-
- if (attrs & DMA_ATTR_NON_CONSISTENT)
- return dma_direct_alloc_pages(dev, size, dma_handle, gfp,
- attrs);
-
- ret = dma_alloc_from_global_coherent(size, dma_handle);
+ void *ret = dma_alloc_from_global_coherent(size, dma_handle);
/*
* dma_alloc_from_global_coherent() may fail because:
@@ -70,16 +59,9 @@ static void arm_nommu_dma_free(struct device *dev, size_t
size,
void *cpu_addr, dma_addr_t dma_addr,
unsigned long attrs)
{
- if (attrs & DMA_ATTR_NON_CONSISTENT) {
- dma_direct_free_pages(dev, size, cpu_addr, dma_addr, attrs);
- } else {
- int ret = dma_release_from_global_coherent(get_order(size),
- cpu_addr);
-
- WARN_ON_ONCE(ret == 0);
- }
+ int ret = dma_release_from_global_coherent(get_order(size), cpu_addr);
- return;
+ WARN_ON_ONCE(ret == 0);
}
static int arm_nommu_dma_mmap(struct device *dev, struct vm_area_struct *vma,
--
2.20.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 2/7] arc: remove the partial DMA_ATTR_NON_CONSISTENT support
The arc DMA code supports DMA_ATTR_NON_CONSISTENT allocations, but does
not provide a cache_sync operation. This means any user of it will
never be able to actually transfer cache ownership and thus cause
coherency bugs.
Signed-off-by: Christoph Hellwig
---
arch/arc/mm/dma.c | 21 ++---
1 file changed, 6 insertions(+), 15 deletions(-)
diff --git a/arch/arc/mm/dma.c b/arch/arc/mm/dma.c
index 1525ac00fd02..9832928f896d 100644
--- a/arch/arc/mm/dma.c
+++ b/arch/arc/mm/dma.c
@@ -24,7 +24,6 @@ void *arch_dma_alloc(struct device *dev, size_t size,
dma_addr_t *dma_handle,
struct page *page;
phys_addr_t paddr;
void *kvaddr;
- bool need_coh = !(attrs & DMA_ATTR_NON_CONSISTENT);
/*
* __GFP_HIGHMEM flag is cleared by upper layer functions
@@ -46,14 +45,10 @@ void *arch_dma_alloc(struct device *dev, size_t size,
dma_addr_t *dma_handle,
* A coherent buffer needs MMU mapping to enforce non-cachability.
* kvaddr is kernel Virtual address (0x7000_ based).
*/
- if (need_coh) {
- kvaddr = ioremap_nocache(paddr, size);
- if (kvaddr == NULL) {
- __free_pages(page, order);
- return NULL;
- }
- } else {
- kvaddr = (void *)(u32)paddr;
+ kvaddr = ioremap_nocache(paddr, size);
+ if (kvaddr == NULL) {
+ __free_pages(page, order);
+ return NULL;
}
/*
@@ -66,9 +61,7 @@ void *arch_dma_alloc(struct device *dev, size_t size,
dma_addr_t *dma_handle,
* Currently flush_cache_vmap nukes the L1 cache completely which
* will be optimized as a separate commit
*/
- if (need_coh)
- dma_cache_wback_inv(paddr, size);
-
+ dma_cache_wback_inv(paddr, size);
return kvaddr;
}
@@ -78,9 +71,7 @@ void arch_dma_free(struct device *dev, size_t size, void
*vaddr,
phys_addr_t paddr = dma_handle;
struct page *page = virt_to_page(paddr);
- if (!(attrs & DMA_ATTR_NON_CONSISTENT))
- iounmap((void __force __iomem *)vaddr);
-
+ iounmap((void __force __iomem *)vaddr);
__free_pages(page, get_order(size));
}
--
2.20.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: How to resolve an issue in swiotlb environment?
On Thu, 13 Jun 2019, shuah wrote: > > Great! So all we have to do is fix vhci-hcd. Then we can remove all > > the virt_boundary_mask stuff from usb-storage and uas entirely. > > > > (I'm assuming wireless USB isn't a genuine issue. As far as I know, it > > is pretty much abandoned at this point.) > > > > Valentina and Shua: Adding SG support to vhci-hcd shouldn't be too > > hard. It ought to be possible even without changing the network > > protocol. > > > > I will start taking a look at this. Is there a target release in plan > to drop virt_boundary_mask stuff? Not yet. But since it doesn't do what we want anyway, this should be fixed quickly. Alan Stern
handle "special" dma allocation in common code
Hi all,, this series ensures that the common dma-direct code handles the somewhat special allocation types requested by the DMA_ATTR_NON_CONSISTENT and DMA_ATTR_NO_KERNEL_MAPPING flags directly. To do so it also removes three partial and thus broken implementations of DMA_ATTR_NON_CONSISTENT. Last but not least it switches arc to use the generic dma remapping code now that arc doesn't implement any special behavior. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2] driver: core: Allow subsystems to continue deferring probe
On Fri, Jun 14, 2019 at 12:10:10PM +0200, Rafael J. Wysocki wrote:
> On Fri, Jun 14, 2019 at 11:39 AM Thierry Reding
> wrote:
> >
> > On Fri, Jun 14, 2019 at 11:10:58AM +0200, Greg Kroah-Hartman wrote:
> > > On Thu, Jun 13, 2019 at 07:00:11PM +0200, Thierry Reding wrote:
> > > > From: Thierry Reding
> > > >
>
> [cut]
>
> >
> > To avoid further back and forth, what exactly is it that you would have
> > me do? That is, what do you consider to be the correct way to do this?
> >
> > Would you prefer me to add another function with a different name that
> > reimplements the functionality only with the exception? Something along
> > the lines of:
> >
> > int driver_deferred_probe_check_state_continue(struct device *dev)
> > {
> > int ret;
> >
> > ret = driver_deferred_probe_check_state(dev);
> > if (ret == -ENODEV)
> > return -EPROBE_DEFER;
> >
> > return ret;
> > }
> >
> > ? I'd need to split that up some more to avoid the warning that the
> > inner function prints before returning -ENODEV, but that's a minor
> > detail. Would that API be more to your liking?
>
> Well, why don't you do
>
> static int deferred_probe_check_state_internal(struct device *dev)
> {
> if (!initcalls_done)
> return -EPROBE_DEFER;
>
> if (!deferred_probe_timeout) {
> dev_WARN(dev, "deferred probe timeout, ignoring dependency");
> return -ETIMEDOUT;
> }
>
> return 0;
> }
>
> int driver_deferred_probe_check_state(struct device *dev)
> {
> int ret = deferred_probe_check_state_internal(dev);
>
> if (ret)
> return ret;
>
> dev_warn(dev, "ignoring dependency for device, assuming no driver");
> return -ENODEV;
> }
>
> int driver_deferred_probe_check_state_continue(struct device *dev)
> {
> int ret = deferred_probe_check_state_internal(dev);
>
> if (ret)
> return ret;
>
> return -EPROBE_DEFER;
> }
Yes, that's much more sane. Self-decribing apis are the key here, I did
not want a boolean flag, or any other flag, as part of the public api as
they do not describe what the call does at all.
thanks,
greg k-h
RE: [PATCH 16/16] dma-mapping: use exact allocation in dma_alloc_contiguous
From: Christoph Hellwig
> Sent: 14 June 2019 14:47
>
> Many architectures (e.g. arm, m68 and sh) have always used exact
> allocation in their dma coherent allocator, which avoids a lot of
> memory waste especially for larger allocations. Lift this behavior
> into the generic allocator so that dma-direct and the generic IOMMU
> code benefit from this behavior as well.
>
> Signed-off-by: Christoph Hellwig
> ---
> include/linux/dma-contiguous.h | 8 +---
> kernel/dma/contiguous.c| 17 +++--
> 2 files changed, 16 insertions(+), 9 deletions(-)
>
> diff --git a/include/linux/dma-contiguous.h b/include/linux/dma-contiguous.h
> index c05d4e661489..2e542e314acf 100644
> --- a/include/linux/dma-contiguous.h
> +++ b/include/linux/dma-contiguous.h
> @@ -161,15 +161,17 @@ static inline struct page *dma_alloc_contiguous(struct
> device *dev, size_t size,
> gfp_t gfp)
> {
> int node = dev ? dev_to_node(dev) : NUMA_NO_NODE;
> - size_t align = get_order(PAGE_ALIGN(size));
> + void *cpu_addr = alloc_pages_exact_node(node, size, gfp);
>
> - return alloc_pages_node(node, gfp, align);
> + if (!cpu_addr)
> + return NULL;
> + return virt_to_page(p);
> }
Does this still guarantee that requests for 16k will not cross a 16k boundary?
It looks like you are losing the alignment parameter.
There may be drivers and hardware that also require 12k allocates
to not cross 16k boundaries (etc).
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT,
UK
Registration No: 1397386 (Wales)
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 12/16] staging/comedi: mark as broken
On Fri, Jun 14, 2019 at 03:47:22PM +0200, Christoph Hellwig wrote: > comedi_buf.c abuse the DMA API in gravely broken ways, as it assumes it > can call virt_to_page on the result, and the just remap it as uncached > using vmap. Disable the driver until this API abuse has been fixed. > > Signed-off-by: Christoph Hellwig > --- > drivers/staging/comedi/Kconfig | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/staging/comedi/Kconfig b/drivers/staging/comedi/Kconfig > index 049b659fa6ad..e7c021d76cfa 100644 > --- a/drivers/staging/comedi/Kconfig > +++ b/drivers/staging/comedi/Kconfig > @@ -1,6 +1,7 @@ > # SPDX-License-Identifier: GPL-2.0 > config COMEDI > tristate "Data acquisition support (comedi)" > + depends on BROKEN Um, that's a huge sledgehammer. Perhaps a hint as to how we can fix this up? This is the first time I've heard of the comedi code not handling dma properly. thanks, greg k-h ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 13/16] mm: rename alloc_pages_exact_nid to alloc_pages_exact_node
This fits in with the naming scheme used by alloc_pages_node.
Signed-off-by: Christoph Hellwig
---
include/linux/gfp.h | 2 +-
mm/page_alloc.c | 4 ++--
mm/page_ext.c | 2 +-
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/include/linux/gfp.h b/include/linux/gfp.h
index fb07b503dc45..4274ea6bc72b 100644
--- a/include/linux/gfp.h
+++ b/include/linux/gfp.h
@@ -532,7 +532,7 @@ extern unsigned long get_zeroed_page(gfp_t gfp_mask);
void *alloc_pages_exact(size_t size, gfp_t gfp_mask);
void free_pages_exact(void *virt, size_t size);
-void * __meminit alloc_pages_exact_nid(int nid, size_t size, gfp_t gfp_mask);
+void * __meminit alloc_pages_exact_node(int nid, size_t size, gfp_t gfp_mask);
#define __get_free_page(gfp_mask) \
__get_free_pages((gfp_mask), 0)
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index d66bc8abe0af..dd2fed66b656 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -4888,7 +4888,7 @@ void *alloc_pages_exact(size_t size, gfp_t gfp_mask)
EXPORT_SYMBOL(alloc_pages_exact);
/**
- * alloc_pages_exact_nid - allocate an exact number of physically-contiguous
+ * alloc_pages_exact_node - allocate an exact number of physically-contiguous
*pages on a node.
* @nid: the preferred node ID where memory should be allocated
* @size: the number of bytes to allocate
@@ -4899,7 +4899,7 @@ EXPORT_SYMBOL(alloc_pages_exact);
*
* Return: pointer to the allocated area or %NULL in case of error.
*/
-void * __meminit alloc_pages_exact_nid(int nid, size_t size, gfp_t gfp_mask)
+void * __meminit alloc_pages_exact_node(int nid, size_t size, gfp_t gfp_mask)
{
unsigned int order = get_order(size);
struct page *p;
diff --git a/mm/page_ext.c b/mm/page_ext.c
index d8f1aca4ad43..bca6bb316714 100644
--- a/mm/page_ext.c
+++ b/mm/page_ext.c
@@ -215,7 +215,7 @@ static void *__meminit alloc_page_ext(size_t size, int nid)
gfp_t flags = GFP_KERNEL | __GFP_ZERO | __GFP_NOWARN;
void *addr = NULL;
- addr = alloc_pages_exact_nid(nid, size, flags);
+ addr = alloc_pages_exact_node(nid, size, flags);
if (addr) {
kmemleak_alloc(addr, size, 1, flags);
return addr;
--
2.20.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 16/16] dma-mapping: use exact allocation in dma_alloc_contiguous
Many architectures (e.g. arm, m68 and sh) have always used exact
allocation in their dma coherent allocator, which avoids a lot of
memory waste especially for larger allocations. Lift this behavior
into the generic allocator so that dma-direct and the generic IOMMU
code benefit from this behavior as well.
Signed-off-by: Christoph Hellwig
---
include/linux/dma-contiguous.h | 8 +---
kernel/dma/contiguous.c| 17 +++--
2 files changed, 16 insertions(+), 9 deletions(-)
diff --git a/include/linux/dma-contiguous.h b/include/linux/dma-contiguous.h
index c05d4e661489..2e542e314acf 100644
--- a/include/linux/dma-contiguous.h
+++ b/include/linux/dma-contiguous.h
@@ -161,15 +161,17 @@ static inline struct page *dma_alloc_contiguous(struct
device *dev, size_t size,
gfp_t gfp)
{
int node = dev ? dev_to_node(dev) : NUMA_NO_NODE;
- size_t align = get_order(PAGE_ALIGN(size));
+ void *cpu_addr = alloc_pages_exact_node(node, size, gfp);
- return alloc_pages_node(node, gfp, align);
+ if (!cpu_addr)
+ return NULL;
+ return virt_to_page(p);
}
static inline void dma_free_contiguous(struct device *dev, struct page *page,
size_t size)
{
- __free_pages(page, get_order(size));
+ free_pages_exact(page_address(page), get_order(size));
}
#endif
diff --git a/kernel/dma/contiguous.c b/kernel/dma/contiguous.c
index bfc0c17f2a3d..84f41eea2741 100644
--- a/kernel/dma/contiguous.c
+++ b/kernel/dma/contiguous.c
@@ -232,9 +232,8 @@ struct page *dma_alloc_contiguous(struct device *dev,
size_t size, gfp_t gfp)
{
int node = dev ? dev_to_node(dev) : NUMA_NO_NODE;
size_t count = PAGE_ALIGN(size) >> PAGE_SHIFT;
- size_t align = get_order(PAGE_ALIGN(size));
- struct page *page = NULL;
struct cma *cma = NULL;
+ void *cpu_addr;
if (dev && dev->cma_area)
cma = dev->cma_area;
@@ -243,14 +242,20 @@ struct page *dma_alloc_contiguous(struct device *dev,
size_t size, gfp_t gfp)
/* CMA can be used only in the context which permits sleeping */
if (cma && gfpflags_allow_blocking(gfp)) {
+ size_t align = get_order(PAGE_ALIGN(size));
+ struct page *page;
+
align = min_t(size_t, align, CONFIG_CMA_ALIGNMENT);
page = cma_alloc(cma, count, align, gfp & __GFP_NOWARN);
+ if (page)
+ return page;
}
/* Fallback allocation of normal pages */
- if (!page)
- page = alloc_pages_node(node, gfp, align);
- return page;
+ cpu_addr = alloc_pages_exact_node(node, size, gfp);
+ if (!cpu_addr)
+ return NULL;
+ return virt_to_page(cpu_addr);
}
/**
@@ -267,7 +272,7 @@ struct page *dma_alloc_contiguous(struct device *dev,
size_t size, gfp_t gfp)
void dma_free_contiguous(struct device *dev, struct page *page, size_t size)
{
if (!cma_release(dev_get_cma_area(dev), page, size >> PAGE_SHIFT))
- __free_pages(page, get_order(size));
+ free_pages_exact(page_address(page), get_order(size));
}
/*
--
2.20.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 14/16] mm: use alloc_pages_exact_node to implement alloc_pages_exact
No need to duplicate the logic over two functions that are almost the
same.
Signed-off-by: Christoph Hellwig
---
include/linux/gfp.h | 5 +++--
mm/page_alloc.c | 39 +++
2 files changed, 10 insertions(+), 34 deletions(-)
diff --git a/include/linux/gfp.h b/include/linux/gfp.h
index 4274ea6bc72b..c616a23a3f81 100644
--- a/include/linux/gfp.h
+++ b/include/linux/gfp.h
@@ -530,9 +530,10 @@ extern struct page *alloc_pages_vma(gfp_t gfp_mask, int
order,
extern unsigned long __get_free_pages(gfp_t gfp_mask, unsigned int order);
extern unsigned long get_zeroed_page(gfp_t gfp_mask);
-void *alloc_pages_exact(size_t size, gfp_t gfp_mask);
void free_pages_exact(void *virt, size_t size);
-void * __meminit alloc_pages_exact_node(int nid, size_t size, gfp_t gfp_mask);
+void *alloc_pages_exact_node(int nid, size_t size, gfp_t gfp_mask);
+#define alloc_pages_exact(size, gfp_mask) \
+ alloc_pages_exact_node(NUMA_NO_NODE, size, gfp_mask)
#define __get_free_page(gfp_mask) \
__get_free_pages((gfp_mask), 0)
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index dd2fed66b656..dec68bd21a71 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -4859,34 +4859,6 @@ static void *make_alloc_exact(unsigned long addr,
unsigned int order,
return (void *)addr;
}
-/**
- * alloc_pages_exact - allocate an exact number physically-contiguous pages.
- * @size: the number of bytes to allocate
- * @gfp_mask: GFP flags for the allocation, must not contain __GFP_COMP
- *
- * This function is similar to alloc_pages(), except that it allocates the
- * minimum number of pages to satisfy the request. alloc_pages() can only
- * allocate memory in power-of-two pages.
- *
- * This function is also limited by MAX_ORDER.
- *
- * Memory allocated by this function must be released by free_pages_exact().
- *
- * Return: pointer to the allocated area or %NULL in case of error.
- */
-void *alloc_pages_exact(size_t size, gfp_t gfp_mask)
-{
- unsigned int order = get_order(size);
- unsigned long addr;
-
- if (WARN_ON_ONCE(gfp_mask & __GFP_COMP))
- gfp_mask &= ~__GFP_COMP;
-
- addr = __get_free_pages(gfp_mask, order);
- return make_alloc_exact(addr, order, size);
-}
-EXPORT_SYMBOL(alloc_pages_exact);
-
/**
* alloc_pages_exact_node - allocate an exact number of physically-contiguous
*pages on a node.
@@ -4894,12 +4866,15 @@ EXPORT_SYMBOL(alloc_pages_exact);
* @size: the number of bytes to allocate
* @gfp_mask: GFP flags for the allocation, must not contain __GFP_COMP
*
- * Like alloc_pages_exact(), but try to allocate on node nid first before
falling
- * back.
+ * This function is similar to alloc_pages_node(), except that it allocates the
+ * minimum number of pages to satisfy the request while alloc_pages() can only
+ * allocate memory in power-of-two pages. This function is also limited by
+ * MAX_ORDER.
*
- * Return: pointer to the allocated area or %NULL in case of error.
+ * Returns a pointer to the allocated area or %NULL in case of error, memory
+ * allocated by this function must be released by free_pages_exact().
*/
-void * __meminit alloc_pages_exact_node(int nid, size_t size, gfp_t gfp_mask)
+void *alloc_pages_exact_node(int nid, size_t size, gfp_t gfp_mask)
{
unsigned int order = get_order(size);
struct page *p;
--
2.20.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 15/16] dma-mapping: clear __GFP_COMP in dma_alloc_attrs
Lift the code to clear __GFP_COMP from arm into the common DMA allocator path. For one this fixes the various other patches that call alloc_pages_exact or split_page in case a bogus driver passes the argument, and it also prepares for doing exact allocation in the generic dma-direct allocator. Signed-off-by: Christoph Hellwig --- arch/arm/mm/dma-mapping.c | 17 - kernel/dma/mapping.c | 9 + 2 files changed, 9 insertions(+), 17 deletions(-) diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c index 0a75058c11f3..86135feb2c05 100644 --- a/arch/arm/mm/dma-mapping.c +++ b/arch/arm/mm/dma-mapping.c @@ -759,14 +759,6 @@ static void *__dma_alloc(struct device *dev, size_t size, dma_addr_t *handle, if (mask < 0xULL) gfp |= GFP_DMA; - /* -* Following is a work-around (a.k.a. hack) to prevent pages -* with __GFP_COMP being passed to split_page() which cannot -* handle them. The real problem is that this flag probably -* should be 0 on ARM as it is not supported on this -* platform; see CONFIG_HUGETLBFS. -*/ - gfp &= ~(__GFP_COMP); args.gfp = gfp; *handle = DMA_MAPPING_ERROR; @@ -1527,15 +1519,6 @@ static void *__arm_iommu_alloc_attrs(struct device *dev, size_t size, return __iommu_alloc_simple(dev, size, gfp, handle, coherent_flag, attrs); - /* -* Following is a work-around (a.k.a. hack) to prevent pages -* with __GFP_COMP being passed to split_page() which cannot -* handle them. The real problem is that this flag probably -* should be 0 on ARM as it is not supported on this -* platform; see CONFIG_HUGETLBFS. -*/ - gfp &= ~(__GFP_COMP); - pages = __iommu_alloc_buffer(dev, size, gfp, attrs, coherent_flag); if (!pages) return NULL; diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c index f7afdadb6770..4b618e1abbc1 100644 --- a/kernel/dma/mapping.c +++ b/kernel/dma/mapping.c @@ -252,6 +252,15 @@ void *dma_alloc_attrs(struct device *dev, size_t size, dma_addr_t *dma_handle, /* let the implementation decide on the zone to allocate from: */ flag &= ~(__GFP_DMA | __GFP_DMA32 | __GFP_HIGHMEM); + /* +* __GFP_COMP interacts badly with splitting up a larger order +* allocation. But as our allocations might not even come from the +* page allocator, the callers can't rely on the fact that they +* even get pages, never mind which kind. +*/ + if (WARN_ON_ONCE(flag & __GFP_COMP)) + flag &= ~__GFP_COMP; + if (dma_is_direct(ops)) cpu_addr = dma_direct_alloc(dev, size, dma_handle, flag, attrs); else if (ops->alloc) -- 2.20.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 12/16] staging/comedi: mark as broken
comedi_buf.c abuse the DMA API in gravely broken ways, as it assumes it can call virt_to_page on the result, and the just remap it as uncached using vmap. Disable the driver until this API abuse has been fixed. Signed-off-by: Christoph Hellwig --- drivers/staging/comedi/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/staging/comedi/Kconfig b/drivers/staging/comedi/Kconfig index 049b659fa6ad..e7c021d76cfa 100644 --- a/drivers/staging/comedi/Kconfig +++ b/drivers/staging/comedi/Kconfig @@ -1,6 +1,7 @@ # SPDX-License-Identifier: GPL-2.0 config COMEDI tristate "Data acquisition support (comedi)" + depends on BROKEN help Enable support for a wide range of data acquisition devices for Linux. -- 2.20.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 11/16] s390/ism: stop passing bogus gfp flags arguments to dma_alloc_coherent
dma_alloc_coherent is not just the page allocator. The only valid arguments to pass are either GFP_ATOMIC or GFP_ATOMIC with possible modifiers of __GFP_NORETRY or __GFP_NOWARN. Signed-off-by: Christoph Hellwig --- drivers/s390/net/ism_drv.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/s390/net/ism_drv.c b/drivers/s390/net/ism_drv.c index 4fc2056bd227..4ff5506fa4c6 100644 --- a/drivers/s390/net/ism_drv.c +++ b/drivers/s390/net/ism_drv.c @@ -241,7 +241,8 @@ static int ism_alloc_dmb(struct ism_dev *ism, struct smcd_dmb *dmb) dmb->cpu_addr = dma_alloc_coherent(>pdev->dev, dmb->dmb_len, >dma_addr, - GFP_KERNEL | __GFP_NOWARN | __GFP_NOMEMALLOC | __GFP_COMP | __GFP_NORETRY); + GFP_KERNEL | __GFP_NOWARN | + __GFP_NORETRY); if (!dmb->cpu_addr) clear_bit(dmb->sba_idx, ism->sba_bitmap); -- 2.20.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 10/16] iwlwifi: stop passing bogus gfp flags arguments to dma_alloc_coherent
dma_alloc_coherent is not just the page allocator. The only valid
arguments to pass are either GFP_ATOMIC or GFP_ATOMIC with possible
modifiers of __GFP_NORETRY or __GFP_NOWARN.
Signed-off-by: Christoph Hellwig
---
drivers/net/wireless/intel/iwlwifi/fw/dbg.c | 3 +--
drivers/net/wireless/intel/iwlwifi/pcie/trans.c | 3 +--
2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/net/wireless/intel/iwlwifi/fw/dbg.c
b/drivers/net/wireless/intel/iwlwifi/fw/dbg.c
index 5f52e40a2903..323dc5d5ee88 100644
--- a/drivers/net/wireless/intel/iwlwifi/fw/dbg.c
+++ b/drivers/net/wireless/intel/iwlwifi/fw/dbg.c
@@ -2361,8 +2361,7 @@ iwl_fw_dbg_buffer_allocation(struct iwl_fw_runtime *fwrt,
u32 size)
virtual_addr =
dma_alloc_coherent(fwrt->trans->dev, size, _addr,
- GFP_KERNEL | __GFP_NOWARN | __GFP_ZERO |
- __GFP_COMP);
+ GFP_KERNEL | __GFP_NOWARN);
/* TODO: alloc fragments if needed */
if (!virtual_addr)
diff --git a/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
b/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
index 803fcbac4152..22a47f928dc8 100644
--- a/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
+++ b/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
@@ -210,8 +210,7 @@ static void iwl_pcie_alloc_fw_monitor_block(struct
iwl_trans *trans,
for (power = max_power; power >= min_power; power--) {
size = BIT(power);
cpu_addr = dma_alloc_coherent(trans->dev, size, ,
- GFP_KERNEL | __GFP_NOWARN |
- __GFP_ZERO | __GFP_COMP);
+ GFP_KERNEL | __GFP_NOWARN);
if (!cpu_addr)
continue;
--
2.20.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 09/16] cnic: stop passing bogus gfp flags arguments to dma_alloc_coherent
dma_alloc_coherent is not just the page allocator. The only valid
arguments to pass are either GFP_ATOMIC or GFP_ATOMIC with possible
modifiers of __GFP_NORETRY or __GFP_NOWARN.
Signed-off-by: Christoph Hellwig
---
drivers/net/ethernet/broadcom/cnic.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/cnic.c
b/drivers/net/ethernet/broadcom/cnic.c
index 57dc3cbff36e..bd1c993680e5 100644
--- a/drivers/net/ethernet/broadcom/cnic.c
+++ b/drivers/net/ethernet/broadcom/cnic.c
@@ -1028,7 +1028,7 @@ static int __cnic_alloc_uio_rings(struct cnic_uio_dev
*udev, int pages)
udev->l2_ring_size = pages * CNIC_PAGE_SIZE;
udev->l2_ring = dma_alloc_coherent(>pdev->dev, udev->l2_ring_size,
>l2_ring_map,
- GFP_KERNEL | __GFP_COMP);
+ GFP_KERNEL);
if (!udev->l2_ring)
return -ENOMEM;
@@ -1036,7 +1036,7 @@ static int __cnic_alloc_uio_rings(struct cnic_uio_dev
*udev, int pages)
udev->l2_buf_size = CNIC_PAGE_ALIGN(udev->l2_buf_size);
udev->l2_buf = dma_alloc_coherent(>pdev->dev, udev->l2_buf_size,
>l2_buf_map,
- GFP_KERNEL | __GFP_COMP);
+ GFP_KERNEL);
if (!udev->l2_buf) {
__cnic_free_uio_rings(udev);
return -ENOMEM;
--
2.20.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 07/16] IB/hfi1: stop passing bogus gfp flags arguments to dma_alloc_coherent
dma_alloc_coherent is not just the page allocator. The only valid
arguments to pass are either GFP_ATOMIC or GFP_ATOMIC with possible
modifiers of __GFP_NORETRY or __GFP_NOWARN.
Signed-off-by: Christoph Hellwig
---
drivers/infiniband/hw/hfi1/init.c | 22 +++---
1 file changed, 3 insertions(+), 19 deletions(-)
diff --git a/drivers/infiniband/hw/hfi1/init.c
b/drivers/infiniband/hw/hfi1/init.c
index 71cb9525c074..ff9d106ee784 100644
--- a/drivers/infiniband/hw/hfi1/init.c
+++ b/drivers/infiniband/hw/hfi1/init.c
@@ -1846,17 +1846,10 @@ int hfi1_create_rcvhdrq(struct hfi1_devdata *dd, struct
hfi1_ctxtdata *rcd)
u64 reg;
if (!rcd->rcvhdrq) {
- gfp_t gfp_flags;
-
amt = rcvhdrq_size(rcd);
- if (rcd->ctxt < dd->first_dyn_alloc_ctxt || rcd->is_vnic)
- gfp_flags = GFP_KERNEL;
- else
- gfp_flags = GFP_USER;
rcd->rcvhdrq = dma_alloc_coherent(>pcidev->dev, amt,
- >rcvhdrq_dma,
- gfp_flags | __GFP_COMP);
+ >rcvhdrq_dma,
GFP_KERNEL);
if (!rcd->rcvhdrq) {
dd_dev_err(dd,
@@ -1870,7 +1863,7 @@ int hfi1_create_rcvhdrq(struct hfi1_devdata *dd, struct
hfi1_ctxtdata *rcd)
rcd->rcvhdrtail_kvaddr =
dma_alloc_coherent(>pcidev->dev,
PAGE_SIZE,
>rcvhdrqtailaddr_dma,
- gfp_flags);
+ GFP_KERNEL);
if (!rcd->rcvhdrtail_kvaddr)
goto bail_free;
}
@@ -1926,19 +1919,10 @@ int hfi1_setup_eagerbufs(struct hfi1_ctxtdata *rcd)
{
struct hfi1_devdata *dd = rcd->dd;
u32 max_entries, egrtop, alloced_bytes = 0;
- gfp_t gfp_flags;
u16 order, idx = 0;
int ret = 0;
u16 round_mtu = roundup_pow_of_two(hfi1_max_mtu);
- /*
-* GFP_USER, but without GFP_FS, so buffer cache can be
-* coalesced (we hope); otherwise, even at order 4,
-* heavy filesystem activity makes these fail, and we can
-* use compound pages.
-*/
- gfp_flags = __GFP_RECLAIM | __GFP_IO | __GFP_COMP;
-
/*
* The minimum size of the eager buffers is a groups of MTU-sized
* buffers.
@@ -1969,7 +1953,7 @@ int hfi1_setup_eagerbufs(struct hfi1_ctxtdata *rcd)
dma_alloc_coherent(>pcidev->dev,
rcd->egrbufs.rcvtid_size,
>egrbufs.buffers[idx].dma,
- gfp_flags);
+ GFP_KERNEL);
if (rcd->egrbufs.buffers[idx].addr) {
rcd->egrbufs.buffers[idx].len =
rcd->egrbufs.rcvtid_size;
--
2.20.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 04/16] drm: move drm_pci_{alloc,free} to drm_legacy
These functions are rather broken in that they try to pass __GFP_COMP
to dma_alloc_coherent, call virt_to_page on the return value and
mess with PageReserved. And not actually used by any modern driver.
Signed-off-by: Christoph Hellwig
---
drivers/gpu/drm/drm_bufs.c | 85
drivers/gpu/drm/drm_pci.c | 89 --
2 files changed, 85 insertions(+), 89 deletions(-)
diff --git a/drivers/gpu/drm/drm_bufs.c b/drivers/gpu/drm/drm_bufs.c
index bfc419ed9d6c..7418872d87c6 100644
--- a/drivers/gpu/drm/drm_bufs.c
+++ b/drivers/gpu/drm/drm_bufs.c
@@ -38,6 +38,91 @@
#include
+/**
+ * drm_pci_alloc - Allocate a PCI consistent memory block, for DMA.
+ * @dev: DRM device
+ * @size: size of block to allocate
+ * @align: alignment of block
+ *
+ * FIXME: This is a needless abstraction of the Linux dma-api and should be
+ * removed.
+ *
+ * Return: A handle to the allocated memory block on success or NULL on
+ * failure.
+ */
+drm_dma_handle_t *drm_pci_alloc(struct drm_device * dev, size_t size, size_t
align)
+{
+ drm_dma_handle_t *dmah;
+ unsigned long addr;
+ size_t sz;
+
+ /* pci_alloc_consistent only guarantees alignment to the smallest
+* PAGE_SIZE order which is greater than or equal to the requested size.
+* Return NULL here for now to make sure nobody tries for larger
alignment
+*/
+ if (align > size)
+ return NULL;
+
+ dmah = kmalloc(sizeof(drm_dma_handle_t), GFP_KERNEL);
+ if (!dmah)
+ return NULL;
+
+ dmah->size = size;
+ dmah->vaddr = dma_alloc_coherent(>pdev->dev, size,
+>busaddr,
+GFP_KERNEL | __GFP_COMP);
+
+ if (dmah->vaddr == NULL) {
+ kfree(dmah);
+ return NULL;
+ }
+
+ /* XXX - Is virt_to_page() legal for consistent mem? */
+ /* Reserve */
+ for (addr = (unsigned long)dmah->vaddr, sz = size;
+sz > 0; addr += PAGE_SIZE, sz -= PAGE_SIZE) {
+ SetPageReserved(virt_to_page((void *)addr));
+ }
+
+ return dmah;
+}
+
+/*
+ * Free a PCI consistent memory block without freeing its descriptor.
+ *
+ * This function is for internal use in the Linux-specific DRM core code.
+ */
+void __drm_legacy_pci_free(struct drm_device * dev, drm_dma_handle_t * dmah)
+{
+ unsigned long addr;
+ size_t sz;
+
+ if (dmah->vaddr) {
+ /* XXX - Is virt_to_page() legal for consistent mem? */
+ /* Unreserve */
+ for (addr = (unsigned long)dmah->vaddr, sz = dmah->size;
+sz > 0; addr += PAGE_SIZE, sz -= PAGE_SIZE) {
+ ClearPageReserved(virt_to_page((void *)addr));
+ }
+ dma_free_coherent(>pdev->dev, dmah->size, dmah->vaddr,
+ dmah->busaddr);
+ }
+}
+
+/**
+ * drm_pci_free - Free a PCI consistent memory block
+ * @dev: DRM device
+ * @dmah: handle to memory block
+ *
+ * FIXME: This is a needless abstraction of the Linux dma-api and should be
+ * removed.
+ */
+void drm_pci_free(struct drm_device * dev, drm_dma_handle_t * dmah)
+{
+ __drm_legacy_pci_free(dev, dmah);
+ kfree(dmah);
+}
+
static struct drm_map_list *drm_find_matching_map(struct drm_device *dev,
struct drm_local_map *map)
{
diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c
index 693748ad8b88..77a215f2a8e4 100644
--- a/drivers/gpu/drm/drm_pci.c
+++ b/drivers/gpu/drm/drm_pci.c
@@ -31,95 +31,6 @@
#include "drm_internal.h"
#include "drm_legacy.h"
-/**
- * drm_pci_alloc - Allocate a PCI consistent memory block, for DMA.
- * @dev: DRM device
- * @size: size of block to allocate
- * @align: alignment of block
- *
- * FIXME: This is a needless abstraction of the Linux dma-api and should be
- * removed.
- *
- * Return: A handle to the allocated memory block on success or NULL on
- * failure.
- */
-drm_dma_handle_t *drm_pci_alloc(struct drm_device * dev, size_t size, size_t
align)
-{
- drm_dma_handle_t *dmah;
- unsigned long addr;
- size_t sz;
-
- /* pci_alloc_consistent only guarantees alignment to the smallest
-* PAGE_SIZE order which is greater than or equal to the requested size.
-* Return NULL here for now to make sure nobody tries for larger
alignment
-*/
- if (align > size)
- return NULL;
-
- dmah = kmalloc(sizeof(drm_dma_handle_t), GFP_KERNEL);
- if (!dmah)
- return NULL;
-
- dmah->size = size;
- dmah->vaddr = dma_alloc_coherent(>pdev->dev, size,
->busaddr,
-GFP_KERNEL | __GFP_COMP);
-
- if (dmah->vaddr == NULL) {
- kfree(dmah);
- return NULL;
- }
-
-
[PATCH 08/16] IB/qib: stop passing bogus gfp flags arguments to dma_alloc_coherent
dma_alloc_coherent is not just the page allocator. The only valid
arguments to pass are either GFP_ATOMIC or GFP_ATOMIC with possible
modifiers of __GFP_NORETRY or __GFP_NOWARN.
Signed-off-by: Christoph Hellwig
---
drivers/infiniband/hw/qib/qib_iba6120.c | 2 +-
drivers/infiniband/hw/qib/qib_init.c| 20 +++-
2 files changed, 4 insertions(+), 18 deletions(-)
diff --git a/drivers/infiniband/hw/qib/qib_iba6120.c
b/drivers/infiniband/hw/qib/qib_iba6120.c
index 531d8a1db2c3..d8a0b8993d22 100644
--- a/drivers/infiniband/hw/qib/qib_iba6120.c
+++ b/drivers/infiniband/hw/qib/qib_iba6120.c
@@ -2076,7 +2076,7 @@ static void alloc_dummy_hdrq(struct qib_devdata *dd)
dd->cspec->dummy_hdrq = dma_alloc_coherent(>pcidev->dev,
dd->rcd[0]->rcvhdrq_size,
>cspec->dummy_hdrq_phys,
- GFP_ATOMIC | __GFP_COMP);
+ GFP_ATOMIC);
if (!dd->cspec->dummy_hdrq) {
qib_devinfo(dd->pcidev, "Couldn't allocate dummy hdrq\n");
/* fallback to just 0'ing */
diff --git a/drivers/infiniband/hw/qib/qib_init.c
b/drivers/infiniband/hw/qib/qib_init.c
index d4fd8a6cff7b..072885a6684d 100644
--- a/drivers/infiniband/hw/qib/qib_init.c
+++ b/drivers/infiniband/hw/qib/qib_init.c
@@ -1547,18 +1547,13 @@ int qib_create_rcvhdrq(struct qib_devdata *dd, struct
qib_ctxtdata *rcd)
if (!rcd->rcvhdrq) {
dma_addr_t phys_hdrqtail;
- gfp_t gfp_flags;
-
amt = ALIGN(dd->rcvhdrcnt * dd->rcvhdrentsize *
sizeof(u32), PAGE_SIZE);
- gfp_flags = (rcd->ctxt >= dd->first_user_ctxt) ?
- GFP_USER : GFP_KERNEL;
old_node_id = dev_to_node(>pcidev->dev);
set_dev_node(>pcidev->dev, rcd->node_id);
rcd->rcvhdrq = dma_alloc_coherent(
- >pcidev->dev, amt, >rcvhdrq_phys,
- gfp_flags | __GFP_COMP);
+ >pcidev->dev, amt, >rcvhdrq_phys, GFP_KERNEL);
set_dev_node(>pcidev->dev, old_node_id);
if (!rcd->rcvhdrq) {
@@ -1578,7 +1573,7 @@ int qib_create_rcvhdrq(struct qib_devdata *dd, struct
qib_ctxtdata *rcd)
set_dev_node(>pcidev->dev, rcd->node_id);
rcd->rcvhdrtail_kvaddr = dma_alloc_coherent(
>pcidev->dev, PAGE_SIZE, _hdrqtail,
- gfp_flags);
+ GFP_KERNEL);
set_dev_node(>pcidev->dev, old_node_id);
if (!rcd->rcvhdrtail_kvaddr)
goto bail_free;
@@ -1622,17 +1617,8 @@ int qib_setup_eagerbufs(struct qib_ctxtdata *rcd)
struct qib_devdata *dd = rcd->dd;
unsigned e, egrcnt, egrperchunk, chunk, egrsize, egroff;
size_t size;
- gfp_t gfp_flags;
int old_node_id;
- /*
-* GFP_USER, but without GFP_FS, so buffer cache can be
-* coalesced (we hope); otherwise, even at order 4,
-* heavy filesystem activity makes these fail, and we can
-* use compound pages.
-*/
- gfp_flags = __GFP_RECLAIM | __GFP_IO | __GFP_COMP;
-
egrcnt = rcd->rcvegrcnt;
egroff = rcd->rcvegr_tid_base;
egrsize = dd->rcvegrbufsize;
@@ -1664,7 +1650,7 @@ int qib_setup_eagerbufs(struct qib_ctxtdata *rcd)
rcd->rcvegrbuf[e] =
dma_alloc_coherent(>pcidev->dev, size,
>rcvegrbuf_phys[e],
- gfp_flags);
+ GFP_KERNEL);
set_dev_node(>pcidev->dev, old_node_id);
if (!rcd->rcvegrbuf[e])
goto bail_rcvegrbuf_phys;
--
2.20.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 05/16] drm: don't mark pages returned from drm_pci_alloc reserved
We are not allowed to call virt_to_page on pages returned from
dma_alloc_coherent, as in many cases the virtual address returned
is aactually a kernel direct mapping. Also there generally is no
need to mark dma memory as reserved.
Signed-off-by: Christoph Hellwig
---
drivers/gpu/drm/drm_bufs.c | 16 +---
1 file changed, 1 insertion(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/drm_bufs.c b/drivers/gpu/drm/drm_bufs.c
index 7418872d87c6..b640437ce90f 100644
--- a/drivers/gpu/drm/drm_bufs.c
+++ b/drivers/gpu/drm/drm_bufs.c
@@ -77,13 +77,6 @@ drm_dma_handle_t *drm_pci_alloc(struct drm_device * dev,
size_t size, size_t ali
return NULL;
}
- /* XXX - Is virt_to_page() legal for consistent mem? */
- /* Reserve */
- for (addr = (unsigned long)dmah->vaddr, sz = size;
-sz > 0; addr += PAGE_SIZE, sz -= PAGE_SIZE) {
- SetPageReserved(virt_to_page((void *)addr));
- }
-
return dmah;
}
@@ -97,16 +90,9 @@ void __drm_legacy_pci_free(struct drm_device * dev,
drm_dma_handle_t * dmah)
unsigned long addr;
size_t sz;
- if (dmah->vaddr) {
- /* XXX - Is virt_to_page() legal for consistent mem? */
- /* Unreserve */
- for (addr = (unsigned long)dmah->vaddr, sz = dmah->size;
-sz > 0; addr += PAGE_SIZE, sz -= PAGE_SIZE) {
- ClearPageReserved(virt_to_page((void *)addr));
- }
+ if (dmah->vaddr)
dma_free_coherent(>pdev->dev, dmah->size, dmah->vaddr,
dmah->busaddr);
- }
}
/**
--
2.20.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 06/16] drm: don't pass __GFP_COMP to dma_alloc_coherent in drm_pci_alloc
The memory returned from dma_alloc_coherent is opaqueue to the user,
thus the exact way of page refcounting shall not matter either.
Signed-off-by: Christoph Hellwig
---
drivers/gpu/drm/drm_bufs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_bufs.c b/drivers/gpu/drm/drm_bufs.c
index b640437ce90f..7a79a16a055c 100644
--- a/drivers/gpu/drm/drm_bufs.c
+++ b/drivers/gpu/drm/drm_bufs.c
@@ -70,7 +70,7 @@ drm_dma_handle_t *drm_pci_alloc(struct drm_device * dev,
size_t size, size_t ali
dmah->size = size;
dmah->vaddr = dma_alloc_coherent(>pdev->dev, size,
>busaddr,
-GFP_KERNEL | __GFP_COMP);
+GFP_KERNEL);
if (dmah->vaddr == NULL) {
kfree(dmah);
--
2.20.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 03/16] drm/i915: stop using drm_pci_alloc
Remove usage of the legacy drm PCI DMA wrappers, and with that the
incorrect usage cocktail of __GFP_COMP, virt_to_page on DMA allocation
and SetPageReserved.
Signed-off-by: Christoph Hellwig
---
drivers/gpu/drm/i915/i915_gem.c| 30 +-
drivers/gpu/drm/i915/i915_gem_object.h | 3 ++-
drivers/gpu/drm/i915/intel_display.c | 2 +-
3 files changed, 18 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index ad01c92aaf74..8f2053c91aff 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -228,7 +228,6 @@ i915_gem_get_aperture_ioctl(struct drm_device *dev, void
*data,
static int i915_gem_object_get_pages_phys(struct drm_i915_gem_object *obj)
{
struct address_space *mapping = obj->base.filp->f_mapping;
- drm_dma_handle_t *phys;
struct sg_table *st;
struct scatterlist *sg;
char *vaddr;
@@ -242,13 +241,13 @@ static int i915_gem_object_get_pages_phys(struct
drm_i915_gem_object *obj)
* to handle all possible callers, and given typical object sizes,
* the alignment of the buddy allocation will naturally match.
*/
- phys = drm_pci_alloc(obj->base.dev,
-roundup_pow_of_two(obj->base.size),
-roundup_pow_of_two(obj->base.size));
- if (!phys)
+ obj->phys_vaddr = dma_alloc_coherent(>base.dev->pdev->dev,
+ roundup_pow_of_two(obj->base.size),
+ >phys_handle, GFP_KERNEL);
+ if (!obj->phys_vaddr)
return -ENOMEM;
- vaddr = phys->vaddr;
+ vaddr = obj->phys_vaddr;
for (i = 0; i < obj->base.size / PAGE_SIZE; i++) {
struct page *page;
char *src;
@@ -286,18 +285,17 @@ static int i915_gem_object_get_pages_phys(struct
drm_i915_gem_object *obj)
sg->offset = 0;
sg->length = obj->base.size;
- sg_dma_address(sg) = phys->busaddr;
+ sg_dma_address(sg) = obj->phys_handle;
sg_dma_len(sg) = obj->base.size;
- obj->phys_handle = phys;
-
__i915_gem_object_set_pages(obj, st, sg->length);
return 0;
err_phys:
- drm_pci_free(obj->base.dev, phys);
-
+ dma_free_coherent(>base.dev->pdev->dev,
+ roundup_pow_of_two(obj->base.size), obj->phys_vaddr,
+ obj->phys_handle);
return err;
}
@@ -335,7 +333,7 @@ i915_gem_object_put_pages_phys(struct drm_i915_gem_object
*obj,
if (obj->mm.dirty) {
struct address_space *mapping = obj->base.filp->f_mapping;
- char *vaddr = obj->phys_handle->vaddr;
+ char *vaddr = obj->phys_vaddr;
int i;
for (i = 0; i < obj->base.size / PAGE_SIZE; i++) {
@@ -363,7 +361,9 @@ i915_gem_object_put_pages_phys(struct drm_i915_gem_object
*obj,
sg_free_table(pages);
kfree(pages);
- drm_pci_free(obj->base.dev, obj->phys_handle);
+ dma_free_coherent(>base.dev->pdev->dev,
+ roundup_pow_of_two(obj->base.size), obj->phys_vaddr,
+ obj->phys_handle);
}
static void
@@ -603,7 +603,7 @@ i915_gem_phys_pwrite(struct drm_i915_gem_object *obj,
struct drm_i915_gem_pwrite *args,
struct drm_file *file)
{
- void *vaddr = obj->phys_handle->vaddr + args->offset;
+ void *vaddr = obj->phys_vaddr + args->offset;
char __user *user_data = u64_to_user_ptr(args->data_ptr);
/* We manually control the domain here and pretend that it
@@ -1431,7 +1431,7 @@ i915_gem_pwrite_ioctl(struct drm_device *dev, void *data,
ret = i915_gem_gtt_pwrite_fast(obj, args);
if (ret == -EFAULT || ret == -ENOSPC) {
- if (obj->phys_handle)
+ if (obj->phys_vaddr)
ret = i915_gem_phys_pwrite(obj, args, file);
else
ret = i915_gem_shmem_pwrite(obj, args);
diff --git a/drivers/gpu/drm/i915/i915_gem_object.h
b/drivers/gpu/drm/i915/i915_gem_object.h
index ca93a40c0c87..14bd2d61d0f6 100644
--- a/drivers/gpu/drm/i915/i915_gem_object.h
+++ b/drivers/gpu/drm/i915/i915_gem_object.h
@@ -290,7 +290,8 @@ struct drm_i915_gem_object {
};
/** for phys allocated objects */
- struct drm_dma_handle *phys_handle;
+ dma_addr_t phys_handle;
+ void *phys_vaddr;
struct reservation_object __builtin_resv;
};
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 5098228f1302..4f8b368ac4e2 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -10066,7 +10066,7 @@ static u32 intel_cursor_base(const struct
intel_plane_state *plane_state)
u32 base;
if (INTEL_INFO(dev_priv)->display.cursor_needs_physical)
-
[PATCH 02/16] drm/ati_pcigart: stop using drm_pci_alloc
Remove usage of the legacy drm PCI DMA wrappers, and with that the
incorrect usage cocktail of __GFP_COMP, virt_to_page on DMA allocation
and SetPageReserved.
Signed-off-by: Christoph Hellwig
---
drivers/gpu/drm/ati_pcigart.c | 27 +++
include/drm/ati_pcigart.h | 5 -
2 files changed, 15 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/ati_pcigart.c b/drivers/gpu/drm/ati_pcigart.c
index 2362f07fe1fc..f66d4fccd812 100644
--- a/drivers/gpu/drm/ati_pcigart.c
+++ b/drivers/gpu/drm/ati_pcigart.c
@@ -41,21 +41,14 @@
static int drm_ati_alloc_pcigart_table(struct drm_device *dev,
struct drm_ati_pcigart_info *gart_info)
{
- gart_info->table_handle = drm_pci_alloc(dev, gart_info->table_size,
- PAGE_SIZE);
- if (gart_info->table_handle == NULL)
+ gart_info->table_vaddr = dma_alloc_coherent(>pdev->dev,
+ gart_info->table_size, _info->table_handle,
+ GFP_KERNEL);
+ if (!gart_info->table_vaddr)
return -ENOMEM;
-
return 0;
}
-static void drm_ati_free_pcigart_table(struct drm_device *dev,
- struct drm_ati_pcigart_info *gart_info)
-{
- drm_pci_free(dev, gart_info->table_handle);
- gart_info->table_handle = NULL;
-}
-
int drm_ati_pcigart_cleanup(struct drm_device *dev, struct
drm_ati_pcigart_info *gart_info)
{
struct drm_sg_mem *entry = dev->sg;
@@ -87,8 +80,10 @@ int drm_ati_pcigart_cleanup(struct drm_device *dev, struct
drm_ati_pcigart_info
}
if (gart_info->gart_table_location == DRM_ATI_GART_MAIN &&
- gart_info->table_handle) {
- drm_ati_free_pcigart_table(dev, gart_info);
+ gart_info->table_vaddr) {
+ dma_free_coherent(>pdev->dev, gart_info->table_size,
+ gart_info->table_vaddr,
gart_info->table_handle);
+ gart_info->table_vaddr = NULL;
}
return 1;
@@ -127,9 +122,9 @@ int drm_ati_pcigart_init(struct drm_device *dev, struct
drm_ati_pcigart_info *ga
goto done;
}
- pci_gart = gart_info->table_handle->vaddr;
- address = gart_info->table_handle->vaddr;
- bus_address = gart_info->table_handle->busaddr;
+ pci_gart = gart_info->table_vaddr;
+ address = gart_info->table_vaddr;
+ bus_address = gart_info->table_handle;
} else {
address = gart_info->addr;
bus_address = gart_info->bus_addr;
diff --git a/include/drm/ati_pcigart.h b/include/drm/ati_pcigart.h
index a728a1364e66..2ffe278ba3b3 100644
--- a/include/drm/ati_pcigart.h
+++ b/include/drm/ati_pcigart.h
@@ -18,7 +18,10 @@ struct drm_ati_pcigart_info {
void *addr;
dma_addr_t bus_addr;
dma_addr_t table_mask;
- struct drm_dma_handle *table_handle;
+
+ dma_addr_t table_handle;
+ void *table_vaddr;
+
struct drm_local_map mapping;
int table_size;
};
--
2.20.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 01/16] media: videobuf-dma-contig: use dma_mmap_coherent
dma_alloc_coherent does not return a physical address, but a DMA
address, which might be remapped or have an offset. Passing this
DMA address to vm_iomap_memory is completely bogus. Use the proper
dma_mmap_coherent helper instead, and stop passing __GFP_COMP
to dma_alloc_coherent, as the memory management inside the DMA
allocator is hidden from the callers.
Fixes: a8f3c203e19b ("[media] videobuf-dma-contig: add cache support")
Signed-off-by: Christoph Hellwig
---
drivers/media/v4l2-core/videobuf-dma-contig.c | 23 +++
1 file changed, 8 insertions(+), 15 deletions(-)
diff --git a/drivers/media/v4l2-core/videobuf-dma-contig.c
b/drivers/media/v4l2-core/videobuf-dma-contig.c
index e1bf50df4c70..a5942ea38f1f 100644
--- a/drivers/media/v4l2-core/videobuf-dma-contig.c
+++ b/drivers/media/v4l2-core/videobuf-dma-contig.c
@@ -39,11 +39,11 @@ struct videobuf_dma_contig_memory {
static int __videobuf_dc_alloc(struct device *dev,
struct videobuf_dma_contig_memory *mem,
- unsigned long size, gfp_t flags)
+ unsigned long size)
{
mem->size = size;
- mem->vaddr = dma_alloc_coherent(dev, mem->size,
- >dma_handle, flags);
+ mem->vaddr = dma_alloc_coherent(dev, mem->size, >dma_handle,
+ GFP_KERNEL);
if (!mem->vaddr) {
dev_err(dev, "memory alloc size %ld failed\n", mem->size);
@@ -260,8 +260,7 @@ static int __videobuf_iolock(struct videobuf_queue *q,
return videobuf_dma_contig_user_get(mem, vb);
/* allocate memory for the read() method */
- if (__videobuf_dc_alloc(q->dev, mem, PAGE_ALIGN(vb->size),
- GFP_KERNEL))
+ if (__videobuf_dc_alloc(q->dev, mem, PAGE_ALIGN(vb->size)))
return -ENOMEM;
break;
case V4L2_MEMORY_OVERLAY:
@@ -280,7 +279,6 @@ static int __videobuf_mmap_mapper(struct videobuf_queue *q,
struct videobuf_dma_contig_memory *mem;
struct videobuf_mapping *map;
int retval;
- unsigned long size;
dev_dbg(q->dev, "%s\n", __func__);
@@ -298,23 +296,18 @@ static int __videobuf_mmap_mapper(struct videobuf_queue
*q,
BUG_ON(!mem);
MAGIC_CHECK(mem->magic, MAGIC_DC_MEM);
- if (__videobuf_dc_alloc(q->dev, mem, PAGE_ALIGN(buf->bsize),
- GFP_KERNEL | __GFP_COMP))
+ if (__videobuf_dc_alloc(q->dev, mem, PAGE_ALIGN(buf->bsize)))
goto error;
- /* Try to remap memory */
- size = vma->vm_end - vma->vm_start;
- vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
-
/* the "vm_pgoff" is just used in v4l2 to find the
* corresponding buffer data structure which is allocated
* earlier and it does not mean the offset from the physical
* buffer start address as usual. So set it to 0 to pass
-* the sanity check in vm_iomap_memory().
+* the sanity check in dma_mmap_coherent().
*/
vma->vm_pgoff = 0;
-
- retval = vm_iomap_memory(vma, mem->dma_handle, size);
+ retval = dma_mmap_coherent(q->dev, vma, mem->vaddr, mem->dma_handle,
+ vma->vm_end - vma->vm_start);
if (retval) {
dev_err(q->dev, "mmap: remap failed with error %d. ",
retval);
--
2.20.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
use exact allocation for dma coherent memory
Hi all, various architectures have used exact memory allocations for dma allocations for a long time, but x86 and thus the common code based on it kept using our normal power of two allocator, which tends to waste a lot of memory for certain allocations. Switching to a slightly cleaned up alloc_pages_exact is pretty easy, but it turns out that because we didn't filter valid gfp_t flags on the DMA allocator, a bunch of drivers were passing __GFP_COMP to it, which is rather bogus in too many ways to explain. Arm has been filtering it for a while, but this series instead tries to fix the drivers and warn when __GFP_COMP is passed, which makes it much larger than just adding the functionality. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v8 23/29] vfio: VFIO_IOMMU_CACHE_INVALIDATE
Hi Liu,
On 6/14/19 2:38 PM, Liu, Yi L wrote:
> Hi Eric,
>
>> From: Eric Auger [mailto:eric.au...@redhat.com]
>> Sent: Monday, May 27, 2019 12:10 AM
>> Subject: [PATCH v8 23/29] vfio: VFIO_IOMMU_CACHE_INVALIDATE
>>
>> From: "Liu, Yi L"
>>
>> When the guest "owns" the stage 1 translation structures, the host IOMMU
>> driver
>> has no knowledge of caching structure updates unless the guest invalidation
>> requests are trapped and passed down to the host.
>>
>> This patch adds the VFIO_IOMMU_CACHE_INVALIDATE ioctl with aims at
>> propagating guest stage1 IOMMU cache invalidations to the host.
>>
>> Signed-off-by: Liu, Yi L
>> Signed-off-by: Eric Auger
>>
>> ---
>> v6 -> v7:
>> - Use iommu_capsule struct
>> - renamed vfio_iommu_for_each_dev into vfio_iommu_lookup_dev
>> due to checkpatch error related to for_each_dev suffix
>>
>> v2 -> v3:
>> - introduce vfio_iommu_for_each_dev back in this patch
>>
>> v1 -> v2:
>> - s/TLB/CACHE
>> - remove vfio_iommu_task usage
>> - commit message rewording
>> ---
>> drivers/vfio/vfio_iommu_type1.c | 55 +
>> include/uapi/linux/vfio.h | 13
>> 2 files changed, 68 insertions(+)
>>
>> diff --git a/drivers/vfio/vfio_iommu_type1.c
>> b/drivers/vfio/vfio_iommu_type1.c
>> index b2d609d6fe83..6fda4fbc9bfa 100644
>> --- a/drivers/vfio/vfio_iommu_type1.c
>> +++ b/drivers/vfio/vfio_iommu_type1.c
>> @@ -120,6 +120,34 @@ struct vfio_regions {
>> #define IS_IOMMU_CAP_DOMAIN_IN_CONTAINER(iommu) \
>> (!list_empty(>domain_list))
>>
>> +struct domain_capsule {
>> +struct iommu_domain *domain;
>> +void *data;
>> +};
>> +
>> +/* iommu->lock must be held */
>> +static int
>> +vfio_iommu_lookup_dev(struct vfio_iommu *iommu,
>> + int (*fn)(struct device *dev, void *data),
>> + void *data)
>> +{
>> +struct domain_capsule dc = {.data = data};
>> +struct vfio_domain *d;
>> +struct vfio_group *g;
>> +int ret = 0;
>> +
>> +list_for_each_entry(d, >domain_list, next) {
>> +dc.domain = d->domain;
>> +list_for_each_entry(g, >group_list, next) {
>> +ret = iommu_group_for_each_dev(g->iommu_group,
>> + , fn);
>> +if (ret)
>> +break;
>> +}
>> +}
>> +return ret;
>> +}
>> +
>> static int put_pfn(unsigned long pfn, int prot);
>>
>> /*
>> @@ -1795,6 +1823,15 @@ vfio_attach_pasid_table(struct vfio_iommu *iommu,
>> return ret;
>> }
>>
>> +static int vfio_cache_inv_fn(struct device *dev, void *data) {
>> +struct domain_capsule *dc = (struct domain_capsule *)data;
>> +struct vfio_iommu_type1_cache_invalidate *ustruct =
>> +(struct vfio_iommu_type1_cache_invalidate *)dc->data;
>> +
>> +return iommu_cache_invalidate(dc->domain, dev, >info); }
>> +
>> static long vfio_iommu_type1_ioctl(void *iommu_data,
>> unsigned int cmd, unsigned long arg) { @@ -
>> 1881,6 +1918,24 @@ static long vfio_iommu_type1_ioctl(void *iommu_data,
>> } else if (cmd == VFIO_IOMMU_DETACH_PASID_TABLE) {
>> vfio_detach_pasid_table(iommu);
>> return 0;
>> +} else if (cmd == VFIO_IOMMU_CACHE_INVALIDATE) {
>> +struct vfio_iommu_type1_cache_invalidate ustruct;
>> +int ret;
>> +
>> +minsz = offsetofend(struct vfio_iommu_type1_cache_invalidate,
>> +info);
>> +
>> +if (copy_from_user(, (void __user *)arg, minsz))
>> +return -EFAULT;
>> +
>> +if (ustruct.argsz < minsz || ustruct.flags)
>
> May remove the flags field?
>
>> +return -EINVAL;
>> +
>> +mutex_lock(>lock);
>> +ret = vfio_iommu_lookup_dev(iommu, vfio_cache_inv_fn,
>> +);
>> +mutex_unlock(>lock);
>> +return ret;
>> }
>>
>> return -ENOTTY;
>> diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h index
>> 4316dd8cb5b5..055aa9b9745a 100644
>> --- a/include/uapi/linux/vfio.h
>> +++ b/include/uapi/linux/vfio.h
>> @@ -785,6 +785,19 @@ struct vfio_iommu_type1_attach_pasid_table {
>> */
>> #define VFIO_IOMMU_DETACH_PASID_TABLE _IO(VFIO_TYPE, VFIO_BASE + 23)
>>
>> +/**
>> + * VFIO_IOMMU_CACHE_INVALIDATE - _IOWR(VFIO_TYPE, VFIO_BASE + 24,
>> + * struct vfio_iommu_type1_cache_invalidate)
>> + *
>> + * Propagate guest IOMMU cache invalidation to the host.
>> + */
>> +struct vfio_iommu_type1_cache_invalidate {
>> +__u32 argsz;
>> +__u32 flags;
>
> Looks like there is no more usage on "flags". is it?
Agreed. However all the other vfio structs embed it.
vfio_iommu_type1_dma_unmap (VFIO_IOMMU_UNMAP_DMA) or
vfio_iommu_type1_info (VFIO_IOMMU_GET_INFO) also feature it and do not
use it either if I am not wrong.
So I
Re: status of the calgary iommu driver
On Wed, May 08, 2019 at 06:56:46PM +0100, Jon Mason wrote: > I do have a system. So, it could be tested. However given the age of > the HW, I would say it is not worth the effort to update and it would > be best to remove it from the kernel. > > I can send a patch to do this, unless you would prefer to do it (or > already have something handy). Do you still plan to send that patch? ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH v8 23/29] vfio: VFIO_IOMMU_CACHE_INVALIDATE
Hi Eric,
> From: Eric Auger [mailto:eric.au...@redhat.com]
> Sent: Monday, May 27, 2019 12:10 AM
> Subject: [PATCH v8 23/29] vfio: VFIO_IOMMU_CACHE_INVALIDATE
>
> From: "Liu, Yi L"
>
> When the guest "owns" the stage 1 translation structures, the host IOMMU
> driver
> has no knowledge of caching structure updates unless the guest invalidation
> requests are trapped and passed down to the host.
>
> This patch adds the VFIO_IOMMU_CACHE_INVALIDATE ioctl with aims at
> propagating guest stage1 IOMMU cache invalidations to the host.
>
> Signed-off-by: Liu, Yi L
> Signed-off-by: Eric Auger
>
> ---
> v6 -> v7:
> - Use iommu_capsule struct
> - renamed vfio_iommu_for_each_dev into vfio_iommu_lookup_dev
> due to checkpatch error related to for_each_dev suffix
>
> v2 -> v3:
> - introduce vfio_iommu_for_each_dev back in this patch
>
> v1 -> v2:
> - s/TLB/CACHE
> - remove vfio_iommu_task usage
> - commit message rewording
> ---
> drivers/vfio/vfio_iommu_type1.c | 55 +
> include/uapi/linux/vfio.h | 13
> 2 files changed, 68 insertions(+)
>
> diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
> index b2d609d6fe83..6fda4fbc9bfa 100644
> --- a/drivers/vfio/vfio_iommu_type1.c
> +++ b/drivers/vfio/vfio_iommu_type1.c
> @@ -120,6 +120,34 @@ struct vfio_regions {
> #define IS_IOMMU_CAP_DOMAIN_IN_CONTAINER(iommu) \
> (!list_empty(>domain_list))
>
> +struct domain_capsule {
> + struct iommu_domain *domain;
> + void *data;
> +};
> +
> +/* iommu->lock must be held */
> +static int
> +vfio_iommu_lookup_dev(struct vfio_iommu *iommu,
> + int (*fn)(struct device *dev, void *data),
> + void *data)
> +{
> + struct domain_capsule dc = {.data = data};
> + struct vfio_domain *d;
> + struct vfio_group *g;
> + int ret = 0;
> +
> + list_for_each_entry(d, >domain_list, next) {
> + dc.domain = d->domain;
> + list_for_each_entry(g, >group_list, next) {
> + ret = iommu_group_for_each_dev(g->iommu_group,
> +, fn);
> + if (ret)
> + break;
> + }
> + }
> + return ret;
> +}
> +
> static int put_pfn(unsigned long pfn, int prot);
>
> /*
> @@ -1795,6 +1823,15 @@ vfio_attach_pasid_table(struct vfio_iommu *iommu,
> return ret;
> }
>
> +static int vfio_cache_inv_fn(struct device *dev, void *data) {
> + struct domain_capsule *dc = (struct domain_capsule *)data;
> + struct vfio_iommu_type1_cache_invalidate *ustruct =
> + (struct vfio_iommu_type1_cache_invalidate *)dc->data;
> +
> + return iommu_cache_invalidate(dc->domain, dev, >info); }
> +
> static long vfio_iommu_type1_ioctl(void *iommu_data,
> unsigned int cmd, unsigned long arg) { @@ -
> 1881,6 +1918,24 @@ static long vfio_iommu_type1_ioctl(void *iommu_data,
> } else if (cmd == VFIO_IOMMU_DETACH_PASID_TABLE) {
> vfio_detach_pasid_table(iommu);
> return 0;
> + } else if (cmd == VFIO_IOMMU_CACHE_INVALIDATE) {
> + struct vfio_iommu_type1_cache_invalidate ustruct;
> + int ret;
> +
> + minsz = offsetofend(struct vfio_iommu_type1_cache_invalidate,
> + info);
> +
> + if (copy_from_user(, (void __user *)arg, minsz))
> + return -EFAULT;
> +
> + if (ustruct.argsz < minsz || ustruct.flags)
May remove the flags field?
> + return -EINVAL;
> +
> + mutex_lock(>lock);
> + ret = vfio_iommu_lookup_dev(iommu, vfio_cache_inv_fn,
> + );
> + mutex_unlock(>lock);
> + return ret;
> }
>
> return -ENOTTY;
> diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h index
> 4316dd8cb5b5..055aa9b9745a 100644
> --- a/include/uapi/linux/vfio.h
> +++ b/include/uapi/linux/vfio.h
> @@ -785,6 +785,19 @@ struct vfio_iommu_type1_attach_pasid_table {
> */
> #define VFIO_IOMMU_DETACH_PASID_TABLE_IO(VFIO_TYPE, VFIO_BASE + 23)
>
> +/**
> + * VFIO_IOMMU_CACHE_INVALIDATE - _IOWR(VFIO_TYPE, VFIO_BASE + 24,
> + * struct vfio_iommu_type1_cache_invalidate)
> + *
> + * Propagate guest IOMMU cache invalidation to the host.
> + */
> +struct vfio_iommu_type1_cache_invalidate {
> + __u32 argsz;
> + __u32 flags;
Looks like there is no more usage on "flags". is it?
Regards,
Yi Liu
> + struct iommu_cache_invalidate_info info; };
> +#define VFIO_IOMMU_CACHE_INVALIDATE _IO(VFIO_TYPE, VFIO_BASE + 24)
> +
> /* Additional API for SPAPR TCE (Server POWERPC) IOMMU */
>
> /*
> --
> 2.20.1
___
iommu mailing list
iommu@lists.linux-foundation.org
Re: [PATCH] dma-remap: Avoid de-referencing NULL atomic_pool
Thanks, applied to the dma-mapping for-next branch. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/dma: Apply dma_{alloc,free}_contiguous functions
On Fri, Jun 14, 2019 at 11:42:54AM +0100, Robin Murphy wrote: > On 06/06/2019 07:28, Christoph Hellwig wrote: >> Looks fine to me. Robin, any comments? > > AFAICS this looks like the obvious conversion, so... no? :) Yep. Just want to make sure I don't apply dma-iommu patches without your review. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/dma: Apply dma_{alloc,free}_contiguous functions
Thanks, applied to the dma-mapping for-next branch.
Re: [RFC] switch nds32 to use the generic remapping DMA allocator
On Fri, Jun 14, 2019 at 07:35:29PM +0800, Greentime Hu wrote: > It looks good to me. I just verified in nds32 platform and it works fine. > Should I put it in my next-tree or you will pick it up in your tree? :) Either way works for me, let me know what you prefer. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC] switch nds32 to use the generic remapping DMA allocator
Christoph Hellwig 於 2019年6月14日 週五 下午6:09寫道： > > Hi Greentime and Vicent, > > can you take a look at the (untested) patch below? It converts nds32 > to use the generic remapping DMA allocator, which is also used by > arm64 and csky. Hi Christoph, It looks good to me. I just verified in nds32 platform and it works fine. Should I put it in my next-tree or you will pick it up in your tree? :) Tested-by: Greentime Hu Reviewed-by: Greentime Hu ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/dma: Apply dma_{alloc,free}_contiguous functions
On 06/06/2019 07:28, Christoph Hellwig wrote:
Looks fine to me. Robin, any comments?
AFAICS this looks like the obvious conversion, so... no? :)
On Mon, Jun 03, 2019 at 03:52:59PM -0700, Nicolin Chen wrote:
This patch replaces dma_{alloc,release}_from_contiguous() with
dma_{alloc,free}_contiguous() to simplify those function calls.
Acked-by: Robin Murphy
Signed-off-by: Nicolin Chen
---
drivers/iommu/dma-iommu.c | 14 --
1 file changed, 4 insertions(+), 10 deletions(-)
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 0cd49c2d3770..cc3d39dbbe1a 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -951,8 +951,8 @@ static void __iommu_dma_free(struct device *dev, size_t
size, void *cpu_addr)
if (pages)
__iommu_dma_free_pages(pages, count);
- if (page && !dma_release_from_contiguous(dev, page, count))
- __free_pages(page, get_order(alloc_size));
+ if (page)
+ dma_free_contiguous(dev, page, alloc_size);
}
static void iommu_dma_free(struct device *dev, size_t size, void *cpu_addr,
@@ -970,12 +970,7 @@ static void *iommu_dma_alloc_pages(struct device *dev,
size_t size,
struct page *page = NULL;
void *cpu_addr;
- if (gfpflags_allow_blocking(gfp))
- page = dma_alloc_from_contiguous(dev, alloc_size >> PAGE_SHIFT,
-get_order(alloc_size),
-gfp & __GFP_NOWARN);
- if (!page)
- page = alloc_pages(gfp, get_order(alloc_size));
+ page = dma_alloc_contiguous(dev, alloc_size, gfp);
if (!page)
return NULL;
@@ -997,8 +992,7 @@ static void *iommu_dma_alloc_pages(struct device *dev, size_t size,
memset(cpu_addr, 0, alloc_size);
return cpu_addr;
out_free_pages:
- if (!dma_release_from_contiguous(dev, page, alloc_size >> PAGE_SHIFT))
- __free_pages(page, get_order(alloc_size));
+ dma_free_contiguous(dev, page, alloc_size);
return NULL;
}
--
2.17.1
---end quoted text---
Re: [RFC PATCH v6 5/5] mmc: queue: Use bigger segments if IOMMU can merge the segments
> > + host->can_merge = 1; > > + else > > + host->can_merge = 0; > > + > > can_merge seems a little too generic a name to me. Maybe can_iommu_merge? Ack. signature.asc Description: PGP signature
[PATCH 2/2] m68k: implement arch_dma_prep_coherent
When we remap memory as non-cached to be used as a DMA coherent buffer
we should writeback all cache and invalidate the cache lines so that
we make sure we have a clean slate. Implement this using the
cache_push() helper.
Signed-off-by: Christoph Hellwig
---
arch/m68k/Kconfig | 1 +
arch/m68k/kernel/dma.c | 5 +
2 files changed, 6 insertions(+)
diff --git a/arch/m68k/Kconfig b/arch/m68k/Kconfig
index 2571a8fba4b0..64b122595896 100644
--- a/arch/m68k/Kconfig
+++ b/arch/m68k/Kconfig
@@ -4,6 +4,7 @@ config M68K
default y
select ARCH_32BIT_OFF_T
select ARCH_HAS_DMA_MMAP_PGPROT if MMU && !COLDFIRE
+ select ARCH_HAS_DMA_PREP_COHERENT
select ARCH_HAS_SYNC_DMA_FOR_DEVICE if HAS_DMA
select ARCH_MIGHT_HAVE_PC_PARPORT if ISA
select ARCH_NO_COHERENT_DMA_MMAP if !MMU
diff --git a/arch/m68k/kernel/dma.c b/arch/m68k/kernel/dma.c
index 9c6a350a16d8..e720e6eed838 100644
--- a/arch/m68k/kernel/dma.c
+++ b/arch/m68k/kernel/dma.c
@@ -18,6 +18,11 @@
#include
#if defined(CONFIG_MMU) && !defined(CONFIG_COLDFIRE)
+void arch_dma_prep_coherent(struct page *page, size_t size)
+{
+ cache_push(page_to_phys(page), size);
+}
+
pgprot_t arch_dma_mmap_pgprot(struct device *dev, pgprot_t prot,
unsigned long attrs)
{
--
2.20.1
[PATCH 1/2] m68k: use the generic dma coherent remap allocator
This switche to using common code for the DMA allocations, including
potential use of the CMA allocator if configure. Also add a
comment where the existing behavior seems to be lacking.
Signed-off-by: Christoph Hellwig
---
arch/m68k/Kconfig | 2 ++
arch/m68k/kernel/dma.c | 59 --
2 files changed, 13 insertions(+), 48 deletions(-)
diff --git a/arch/m68k/Kconfig b/arch/m68k/Kconfig
index 218e037ef901..2571a8fba4b0 100644
--- a/arch/m68k/Kconfig
+++ b/arch/m68k/Kconfig
@@ -3,10 +3,12 @@ config M68K
bool
default y
select ARCH_32BIT_OFF_T
+ select ARCH_HAS_DMA_MMAP_PGPROT if MMU && !COLDFIRE
select ARCH_HAS_SYNC_DMA_FOR_DEVICE if HAS_DMA
select ARCH_MIGHT_HAVE_PC_PARPORT if ISA
select ARCH_NO_COHERENT_DMA_MMAP if !MMU
select ARCH_NO_PREEMPT if !COLDFIRE
+ select DMA_DIRECT_REMAP if MMU && !COLDFIRE
select HAVE_IDE
select HAVE_AOUT if MMU
select HAVE_DEBUG_BUGVERBOSE
diff --git a/arch/m68k/kernel/dma.c b/arch/m68k/kernel/dma.c
index b4aa853051bd..9c6a350a16d8 100644
--- a/arch/m68k/kernel/dma.c
+++ b/arch/m68k/kernel/dma.c
@@ -18,57 +18,20 @@
#include
#if defined(CONFIG_MMU) && !defined(CONFIG_COLDFIRE)
-
-void *arch_dma_alloc(struct device *dev, size_t size, dma_addr_t *handle,
- gfp_t flag, unsigned long attrs)
+pgprot_t arch_dma_mmap_pgprot(struct device *dev, pgprot_t prot,
+ unsigned long attrs)
{
- struct page *page, **map;
- pgprot_t pgprot;
- void *addr;
- int i, order;
-
- pr_debug("dma_alloc_coherent: %d,%x\n", size, flag);
-
- size = PAGE_ALIGN(size);
- order = get_order(size);
-
- page = alloc_pages(flag | __GFP_ZERO, order);
- if (!page)
- return NULL;
-
- *handle = page_to_phys(page);
- map = kmalloc(sizeof(struct page *) << order, flag & ~__GFP_DMA);
- if (!map) {
- __free_pages(page, order);
- return NULL;
+ /*
+* XXX: this doesn't seem to handle the sun3 MMU at all.
+*/
+ if (CPU_IS_040_OR_060) {
+ pgprot_val(prot) &= ~_PAGE_CACHE040;
+ pgprot_val(prot) |= _PAGE_GLOBAL040 | _PAGE_NOCACHE_S;
+ } else {
+ pgprot_val(prot) |= _PAGE_NOCACHE030;
}
- split_page(page, order);
-
- order = 1 << order;
- size >>= PAGE_SHIFT;
- map[0] = page;
- for (i = 1; i < size; i++)
- map[i] = page + i;
- for (; i < order; i++)
- __free_page(page + i);
- pgprot = __pgprot(_PAGE_PRESENT | _PAGE_ACCESSED | _PAGE_DIRTY);
- if (CPU_IS_040_OR_060)
- pgprot_val(pgprot) |= _PAGE_GLOBAL040 | _PAGE_NOCACHE_S;
- else
- pgprot_val(pgprot) |= _PAGE_NOCACHE030;
- addr = vmap(map, size, VM_MAP, pgprot);
- kfree(map);
-
- return addr;
+ return prot;
}
-
-void arch_dma_free(struct device *dev, size_t size, void *addr,
- dma_addr_t handle, unsigned long attrs)
-{
- pr_debug("dma_free_coherent: %p, %x\n", addr, handle);
- vfree(addr);
-}
-
#else
#include
--
2.20.1
[RFC] switch m68k to use the generic remapping DMA allocator
Hi Geert and Greg, can you take a look at the (untested) patches below? They convert m68k to use the generic remapping DMA allocator, which is also used by arm64 and csky. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2] driver: core: Allow subsystems to continue deferring probe
On Fri, Jun 14, 2019 at 11:39 AM Thierry Reding
wrote:
>
> On Fri, Jun 14, 2019 at 11:10:58AM +0200, Greg Kroah-Hartman wrote:
> > On Thu, Jun 13, 2019 at 07:00:11PM +0200, Thierry Reding wrote:
> > > From: Thierry Reding
> > >
[cut]
>
> To avoid further back and forth, what exactly is it that you would have
> me do? That is, what do you consider to be the correct way to do this?
>
> Would you prefer me to add another function with a different name that
> reimplements the functionality only with the exception? Something along
> the lines of:
>
> int driver_deferred_probe_check_state_continue(struct device *dev)
> {
> int ret;
>
> ret = driver_deferred_probe_check_state(dev);
> if (ret == -ENODEV)
> return -EPROBE_DEFER;
>
> return ret;
> }
>
> ? I'd need to split that up some more to avoid the warning that the
> inner function prints before returning -ENODEV, but that's a minor
> detail. Would that API be more to your liking?
Well, why don't you do
static int deferred_probe_check_state_internal(struct device *dev)
{
if (!initcalls_done)
return -EPROBE_DEFER;
if (!deferred_probe_timeout) {
dev_WARN(dev, "deferred probe timeout, ignoring dependency");
return -ETIMEDOUT;
}
return 0;
}
int driver_deferred_probe_check_state(struct device *dev)
{
int ret = deferred_probe_check_state_internal(dev);
if (ret)
return ret;
dev_warn(dev, "ignoring dependency for device, assuming no driver");
return -ENODEV;
}
int driver_deferred_probe_check_state_continue(struct device *dev)
{
int ret = deferred_probe_check_state_internal(dev);
if (ret)
return ret;
return -EPROBE_DEFER;
}
[PATCH] nds32: use the generic remapping allocator for coherent DMA allocations
Replace the code that sets up uncached PTEs with the generic vmap based
remapping code. It also provides an atomic pool for allocations from
non-blocking context, which we not properly supported by the existing
nds32 code.
Signed-off-by: Christoph Hellwig
---
arch/nds32/Kconfig | 2 +
arch/nds32/kernel/dma.c | 325 ++--
2 files changed, 13 insertions(+), 314 deletions(-)
diff --git a/arch/nds32/Kconfig b/arch/nds32/Kconfig
index 3299e287a477..643ea6b4bfa2 100644
--- a/arch/nds32/Kconfig
+++ b/arch/nds32/Kconfig
@@ -7,12 +7,14 @@
config NDS32
def_bool y
select ARCH_32BIT_OFF_T
+ select ARCH_HAS_DMA_PREP_COHERENT
select ARCH_HAS_SYNC_DMA_FOR_CPU
select ARCH_HAS_SYNC_DMA_FOR_DEVICE
select ARCH_WANT_FRAME_POINTERS if FTRACE
select CLKSRC_MMIO
select CLONE_BACKWARDS
select COMMON_CLK
+ select DMA_DIRECT_REMAP
select GENERIC_ATOMIC64
select GENERIC_CPU_DEVICES
select GENERIC_CLOCKEVENTS
diff --git a/arch/nds32/kernel/dma.c b/arch/nds32/kernel/dma.c
index d0dbd4fe9645..490e3720d694 100644
--- a/arch/nds32/kernel/dma.c
+++ b/arch/nds32/kernel/dma.c
@@ -3,327 +3,13 @@
#include
#include
-#include
#include
-#include
#include
#include
-#include
#include
#include
#include
-/*
- * This is the page table (2MB) covering uncached, DMA consistent allocations
- */
-static pte_t *consistent_pte;
-static DEFINE_RAW_SPINLOCK(consistent_lock);
-
-/*
- * VM region handling support.
- *
- * This should become something generic, handling VM region allocations for
- * vmalloc and similar (ioremap, module space, etc).
- *
- * I envisage vmalloc()'s supporting vm_struct becoming:
- *
- * struct vm_struct {
- *struct vm_region region;
- *unsigned longflags;
- *struct page **pages;
- *unsigned int nr_pages;
- *unsigned longphys_addr;
- * };
- *
- * get_vm_area() would then call vm_region_alloc with an appropriate
- * struct vm_region head (eg):
- *
- * struct vm_region vmalloc_head = {
- * .vm_list= LIST_HEAD_INIT(vmalloc_head.vm_list),
- * .vm_start = VMALLOC_START,
- * .vm_end = VMALLOC_END,
- * };
- *
- * However, vmalloc_head.vm_start is variable (typically, it is dependent on
- * the amount of RAM found at boot time.) I would imagine that get_vm_area()
- * would have to initialise this each time prior to calling vm_region_alloc().
- */
-struct arch_vm_region {
- struct list_head vm_list;
- unsigned long vm_start;
- unsigned long vm_end;
- struct page *vm_pages;
-};
-
-static struct arch_vm_region consistent_head = {
- .vm_list = LIST_HEAD_INIT(consistent_head.vm_list),
- .vm_start = CONSISTENT_BASE,
- .vm_end = CONSISTENT_END,
-};
-
-static struct arch_vm_region *vm_region_alloc(struct arch_vm_region *head,
- size_t size, int gfp)
-{
- unsigned long addr = head->vm_start, end = head->vm_end - size;
- unsigned long flags;
- struct arch_vm_region *c, *new;
-
- new = kmalloc(sizeof(struct arch_vm_region), gfp);
- if (!new)
- goto out;
-
- raw_spin_lock_irqsave(_lock, flags);
-
- list_for_each_entry(c, >vm_list, vm_list) {
- if ((addr + size) < addr)
- goto nospc;
- if ((addr + size) <= c->vm_start)
- goto found;
- addr = c->vm_end;
- if (addr > end)
- goto nospc;
- }
-
-found:
- /*
-* Insert this entry _before_ the one we found.
-*/
- list_add_tail(>vm_list, >vm_list);
- new->vm_start = addr;
- new->vm_end = addr + size;
-
- raw_spin_unlock_irqrestore(_lock, flags);
- return new;
-
-nospc:
- raw_spin_unlock_irqrestore(_lock, flags);
- kfree(new);
-out:
- return NULL;
-}
-
-static struct arch_vm_region *vm_region_find(struct arch_vm_region *head,
-unsigned long addr)
-{
- struct arch_vm_region *c;
-
- list_for_each_entry(c, >vm_list, vm_list) {
- if (c->vm_start == addr)
- goto out;
- }
- c = NULL;
-out:
- return c;
-}
-
-void *arch_dma_alloc(struct device *dev, size_t size, dma_addr_t *handle,
- gfp_t gfp, unsigned long attrs)
-{
- struct page *page;
- struct arch_vm_region *c;
- unsigned long order;
- u64 mask = ~0ULL, limit;
- pgprot_t prot = pgprot_noncached(PAGE_KERNEL);
-
- if (!consistent_pte) {
- pr_err("%s: not initialized\n", __func__);
- dump_stack();
- return NULL;
- }
-
- if (dev) {
- mask = dev->coherent_dma_mask;
-
- /*
-* Sanity check the DMA mask - it must be non-zero, and
-
[RFC] switch nds32 to use the generic remapping DMA allocator
Hi Greentime and Vicent, can you take a look at the (untested) patch below? It converts nds32 to use the generic remapping DMA allocator, which is also used by arm64 and csky.
Re: [RFC PATCH v6 3/5] block: add a helper function to merge the segments by an IOMMU
On 13/06/2019 11:20, Yoshihiro Shimoda wrote:
This patch adds a helper function whether a queue can merge
the segments by an IOMMU.
Signed-off-by: Yoshihiro Shimoda
---
block/blk-settings.c | 28
include/linux/blkdev.h | 2 ++
2 files changed, 30 insertions(+)
diff --git a/block/blk-settings.c b/block/blk-settings.c
index 45f2c52..4e4e13e 100644
--- a/block/blk-settings.c
+++ b/block/blk-settings.c
@@ -4,9 +4,11 @@
*/
#include
#include
+#include
#include
#include
#include
+#include
#include
#include
#include
@@ -831,6 +833,32 @@ void blk_queue_write_cache(struct request_queue *q, bool
wc, bool fua)
}
EXPORT_SYMBOL_GPL(blk_queue_write_cache);
+/**
+ * blk_queue_can_use_iommu_merging - configure queue for merging segments.
+ * @q: the request queue for the device
+ * @dev: the device pointer for dma
+ *
+ * Tell the block layer about the iommu merging of @q.
+ */
+bool blk_queue_can_use_iommu_merging(struct request_queue *q,
+struct device *dev)
+{
+ struct iommu_domain *domain;
+
+ /*
+* If the device DMA is translated by an IOMMU, we can assume
+* the device can merge the segments.
+*/
+ if (!device_iommu_mapped(dev))
Careful here - I think this validates the comment I made when this
function was introduced, in that that name doesn't necesarily mean what
it sounds like it might mean - "iommu_mapped" was as close as we managed
to get to a convenient shorthand for "performs DMA through an
IOMMU-API-enabled IOMMU". Specifically, it does not imply that
translation is *currently* active; if you boot with "iommu=pt" or
equivalent this will still return true even though the device will be
using direct/SWIOTLB DMA ops without any IOMMU translation.
Robin.
+ return false;
+
+ domain = iommu_get_domain_for_dev(dev);
+ /* No need to update max_segment_size. see blk_queue_virt_boundary() */
+ blk_queue_virt_boundary(q, iommu_get_minimum_page_size(domain) - 1);
+
+ return true;
+}
+
static int __init blk_settings_init(void)
{
blk_max_low_pfn = max_low_pfn - 1;
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 592669b..4d1f7dc 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -1091,6 +1091,8 @@ extern void blk_queue_dma_alignment(struct request_queue
*, int);
extern void blk_queue_update_dma_alignment(struct request_queue *, int);
extern void blk_queue_rq_timeout(struct request_queue *, unsigned int);
extern void blk_queue_write_cache(struct request_queue *q, bool enabled, bool
fua);
+extern bool blk_queue_can_use_iommu_merging(struct request_queue *q,
+ struct device *dev);
/*
* Number of physical segments as sent to the device.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 2/2] swiotlb: Return consistent SWIOTLB segments/nr_tbl
On Tue, Jun 11, 2019 at 10:58:25AM -0700, Florian Fainelli wrote:
> With a specifically contrived memory layout where there is no physical
> memory available to the kernel below the 4GB boundary, we will fail to
> perform the initial swiotlb_init() call and set no_iotlb_memory to true.
>
> There are drivers out there that call into swiotlb_nr_tbl() to determine
> whether they can use the SWIOTLB. With the right DMA_BIT_MASK() value
> for these drivers (say 64-bit), they won't ever need to hit
> swiotlb_tbl_map_single() so this can go unoticed and we would be
> possibly lying about those drivers.
>
> Signed-off-by: Florian Fainelli
> ---
> kernel/dma/swiotlb.c | 8
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
> index b2b5c5df273c..e906ef2e6315 100644
> --- a/kernel/dma/swiotlb.c
> +++ b/kernel/dma/swiotlb.c
> @@ -129,15 +129,17 @@ setup_io_tlb_npages(char *str)
> }
> early_param("swiotlb", setup_io_tlb_npages);
>
> +static bool no_iotlb_memory;
> +
> unsigned long swiotlb_nr_tbl(void)
> {
> - return io_tlb_nslabs;
> + return unlikely(no_iotlb_memory) ? 0 : io_tlb_nslabs;
> }
> EXPORT_SYMBOL_GPL(swiotlb_nr_tbl);
>
> unsigned int swiotlb_max_segment(void)
> {
> - return max_segment;
> + return unlikely(no_iotlb_memory) ? 0 : max_segment;
I wouldn't bother with the unlikely here as anythign querying
swiotlb details should pretty much be a slow path already.
Otherwise looks good:
Reviewed-by: Christoph Hellwig
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/2] swiotlb: Group identical cleanup in swiotlb_cleanup()
On Tue, Jun 11, 2019 at 10:58:24AM -0700, Florian Fainelli wrote: > Avoid repeating the zeroing of global swiotlb variables in two locations > and introduce swiotlb_cleanup() to do that. > > Signed-off-by: Florian Fainelli Looks good, Reviewed-by: Christoph Hellwig ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH v6 1/5] iommu: add an exported function to get minimum page size for a domain
On 13/06/2019 11:20, Yoshihiro Shimoda wrote:
This patch adds an exported function to get minimum page size for
a domain. This patch also modifies similar codes on the iommu.c.
Heh, seeing this gave me a genuine déjà vu moment...
...but it turns out I actually *have* reviewed this patch before :)
https://lore.kernel.org/lkml/05eca601-0264-8141-ceeb-7ef7ad5d5...@arm.com/
Robin.
Signed-off-by: Yoshihiro Shimoda
---
drivers/iommu/iommu.c | 18 +++---
include/linux/iommu.h | 1 +
2 files changed, 16 insertions(+), 3 deletions(-)
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 2a90638..7ed16af 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -280,6 +280,18 @@ iommu_insert_device_resv_regions(struct list_head
*dev_resv_regions,
return ret;
}
+/**
+ * iommu_get_minimum_page_size - get minimum page size for a domain
+ * @domain: the domain
+ *
+ * Allow iommu driver to get a minimum page size for a domain.
+ */
+unsigned long iommu_get_minimum_page_size(struct iommu_domain *domain)
+{
+ return 1UL << __ffs(domain->pgsize_bitmap);
+}
+EXPORT_SYMBOL_GPL(iommu_get_minimum_page_size);
+
int iommu_get_group_resv_regions(struct iommu_group *group,
struct list_head *head)
{
@@ -558,7 +570,7 @@ static int iommu_group_create_direct_mappings(struct
iommu_group *group,
BUG_ON(!domain->pgsize_bitmap);
- pg_size = 1UL << __ffs(domain->pgsize_bitmap);
+ pg_size = iommu_get_minimum_page_size(domain);
INIT_LIST_HEAD();
iommu_get_resv_regions(dev, );
@@ -1595,7 +1607,7 @@ int iommu_map(struct iommu_domain *domain, unsigned long
iova,
return -EINVAL;
/* find out the minimum page size supported */
- min_pagesz = 1 << __ffs(domain->pgsize_bitmap);
+ min_pagesz = iommu_get_minimum_page_size(domain);
/*
* both the virtual address and the physical one, as well as
@@ -1655,7 +1667,7 @@ static size_t __iommu_unmap(struct iommu_domain *domain,
return 0;
/* find out the minimum page size supported */
- min_pagesz = 1 << __ffs(domain->pgsize_bitmap);
+ min_pagesz = iommu_get_minimum_page_size(domain);
/*
* The virtual address, as well as the size of the mapping, must be
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 91af22a..7e53b43 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -366,6 +366,7 @@ extern int iommu_request_dma_domain_for_dev(struct device
*dev);
extern struct iommu_resv_region *
iommu_alloc_resv_region(phys_addr_t start, size_t length, int prot,
enum iommu_resv_type type);
+extern unsigned long iommu_get_minimum_page_size(struct iommu_domain *domain);
extern int iommu_get_group_resv_regions(struct iommu_group *group,
struct list_head *head);
Re: [PATCH v2] driver: core: Allow subsystems to continue deferring probe
On Fri, Jun 14, 2019 at 11:10:58AM +0200, Greg Kroah-Hartman wrote:
> On Thu, Jun 13, 2019 at 07:00:11PM +0200, Thierry Reding wrote:
> > From: Thierry Reding
> >
> > Some subsystems, such as pinctrl, allow continuing to defer probe
> > indefinitely. This is useful for devices that depend on resources
> > provided by devices that are only probed after the init stage.
> >
> > One example of this can be seen on Tegra, where the DPAUX hardware
> > contains pinmuxing controls for pins that it shares with an I2C
> > controller. The I2C controller is typically used for communication
> > with a monitor over HDMI (DDC). However, other instances of the I2C
> > controller are used to access system critical components, such as a
> > PMIC. The I2C controller driver will therefore usually be a builtin
> > driver, whereas the DPAUX driver is part of the display driver that
> > is loaded from a module to avoid bloating the kernel image with all
> > of the DRM/KMS subsystem.
> >
> > In this particular case the pins used by this I2C/DDC controller
> > become accessible very late in the boot process. However, since the
> > controller is only used in conjunction with display, that's not an
> > issue.
> >
> > Unfortunately the driver core currently outputs a warning message
> > when a device fails to get the pinctrl before the end of the init
> > stage. That can be confusing for the user because it may sound like
> > an unwanted error occurred, whereas it's really an expected and
> > harmless situation.
> >
> > In order to eliminate this warning, this patch allows callers of the
> > driver_deferred_probe_check_state() helper to specify that they want
> > to continue deferring probe, regardless of whether we're past the
> > init stage or not. All of the callers of that function are updated
> > for the new signature, but only the pinctrl subsystem passes a true
> > value in the new persist parameter if appropriate.
> >
> > Signed-off-by: Thierry Reding
> > ---
> > Changes in v2:
> > - pass persist flag via flags parameter to make the function call easier
> > to understand
> >
> > drivers/base/dd.c| 19 ++-
> > drivers/base/power/domain.c | 2 +-
> > drivers/iommu/of_iommu.c | 2 +-
> > drivers/pinctrl/devicetree.c | 9 +
> > include/linux/device.h | 18 +-
> > 5 files changed, 38 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> > index 0df9b4461766..0399a6f6c479 100644
> > --- a/drivers/base/dd.c
> > +++ b/drivers/base/dd.c
> > @@ -238,23 +238,32 @@ __setup("deferred_probe_timeout=",
> > deferred_probe_timeout_setup);
> > /**
> > * driver_deferred_probe_check_state() - Check deferred probe state
> > * @dev: device to check
> > + * @flags: Flags used to control the behavior of this function. Drivers can
> > + * set the DRIVER_DEFER_PROBE_PERSIST flag to indicate that they want to
> > + * keep trying to probe after built-in drivers have had a chance to
> > probe.
> > + * This is useful for built-in drivers that rely on resources provided by
> > + * modular drivers.
> > *
> > * Returns -ENODEV if init is done and all built-in drivers have had a
> > chance
> > - * to probe (i.e. initcalls are done), -ETIMEDOUT if deferred probe debug
> > - * timeout has expired, or -EPROBE_DEFER if none of those conditions are
> > met.
> > + * to probe (i.e. initcalls are done) and unless the
> > DRIVER_DEFER_PROBE_PERSIST
> > + * flag is set, -ETIMEDOUT if deferred probe debug timeout has expired, or
> > + * -EPROBE_DEFER if none of those conditions are met.
> > *
> > * Drivers or subsystems can opt-in to calling this function instead of
> > directly
> > * returning -EPROBE_DEFER.
> > */
> > -int driver_deferred_probe_check_state(struct device *dev)
> > +int driver_deferred_probe_check_state(struct device *dev, unsigned long
> > flags)
> > {
> > if (initcalls_done) {
> > if (!deferred_probe_timeout) {
> > dev_WARN(dev, "deferred probe timeout, ignoring
> > dependency");
> > return -ETIMEDOUT;
> > }
> > - dev_warn(dev, "ignoring dependency for device, assuming no
> > driver");
> > - return -ENODEV;
> > +
> > + if ((flags & DRIVER_DEFER_PROBE_PERSIST) == 0) {
> > + dev_warn(dev, "ignoring dependency for device, assuming
> > no driver");
> > + return -ENODEV;
> > + }
> > }
> > return -EPROBE_DEFER;
> > }
> > diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> > index 33c30c1e6a30..6198c6a30fe2 100644
> > --- a/drivers/base/power/domain.c
> > +++ b/drivers/base/power/domain.c
> > @@ -2423,7 +2423,7 @@ static int __genpd_dev_pm_attach(struct device *dev,
> > struct device *base_dev,
> > mutex_unlock(_list_lock);
> > dev_dbg(dev, "%s() failed to find PM domain: %ld\n",
> > __func__,
[git pull] IOMMU Fixes for Linux v5.2-rc4
Hi Linus, The following changes since commit cd6c84d8f0cdc911df435bb075ba22ce3c605b07: Linux 5.2-rc2 (2019-05-26 16:49:19 -0700) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git tags/iommu-fixes-v5.2-rc4 for you to fetch changes up to 4e4abae311e4b44aaf61f18a826fd7136037f199: iommu/arm-smmu: Avoid constant zero in TLBI writes (2019-06-12 10:08:56 +0200) IOMMU Fixes for Linux v5.2-rc4 Including: - Three Fixes for Intel VT-d to fix a potential dead-lock, a formatting fix and a bit setting fix. - One fix for the ARM-SMMU to make it work on some platforms with sub-optimal SMMU emulation. Dave Jiang (1): iommu/vt-d: Fix lock inversion between iommu->lock and device_domain_lock Lu Baolu (2): iommu: Add missing new line for dma type iommu/vt-d: Set the right field for Page Walk Snoop Robin Murphy (1): iommu/arm-smmu: Avoid constant zero in TLBI writes drivers/iommu/arm-smmu.c| 15 --- drivers/iommu/intel-iommu.c | 7 --- drivers/iommu/intel-pasid.c | 2 +- drivers/iommu/iommu.c | 2 +- 4 files changed, 18 insertions(+), 8 deletions(-) Please pull. Thanks, Joerg signature.asc Description: Digital signature ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 3/4] iommu/arm-smmu: Add support to handle Qcom's wait-for-safe logic
On 6/14/2019 9:35 AM, Bjorn Andersson wrote:
On Wed 12 Jun 00:15 PDT 2019, Vivek Gautam wrote:
Qcom's implementation of arm,mmu-500 adds a WAIT-FOR-SAFE logic
to address under-performance issues in real-time clients, such as
Display, and Camera.
On receiving an invalidation requests, the SMMU forwards SAFE request
to these clients and waits for SAFE ack signal from real-time clients.
The SAFE signal from such clients is used to qualify the start of
invalidation.
This logic is controlled by chicken bits, one for each - MDP (display),
IFE0, and IFE1 (camera), that can be accessed only from secure software
on sdm845.
This configuration, however, degrades the performance of non-real time
clients, such as USB, and UFS etc. This happens because, with wait-for-safe
logic enabled the hardware tries to throttle non-real time clients while
waiting for SAFE ack signals from real-time clients.
On MTP sdm845 devices, with wait-for-safe logic enabled at the boot time
by the bootloaders we see degraded performance of USB and UFS when kernel
enables the smmu stage-1 translations for these clients.
Turn off this wait-for-safe logic from the kernel gets us back the perf
of USB and UFS devices until we re-visit this when we start seeing perf
issues on display/camera on upstream supported SDM845 platforms.
Now, different bootloaders with their access control policies allow this
register access differently through secure monitor calls -
1) With one we can issue io-read/write secure monitor call (qcom-scm)
to update the register, while,
2) With other, such as one on MTP sdm845 we should use the specific
qcom-scm command to send request to do the complete register
configuration.
Adding a separate device tree flag for arm-smmu to identify which
firmware configuration of the two mentioned above we use.
Not adding code change to allow type-(1) bootloaders to toggle the
safe using io-read/write qcom-scm call.
This change is inspired by the downstream change from Patrick Daly
to address performance issues with display and camera by handling
this wait-for-safe within separte io-pagetable ops to do TLB
maintenance. So a big thanks to him for the change.
Without this change the UFS reads are pretty slow:
$ time dd if=/dev/sda of=/dev/zero bs=1048576 count=10 conv=sync
10+0 records in
10+0 records out
10485760 bytes (10.0MB) copied, 22.394903 seconds, 457.2KB/s
real0m 22.39s
user0m 0.00s
sys 0m 0.01s
With this change they are back to rock!
$ time dd if=/dev/sda of=/dev/zero bs=1048576 count=300 conv=sync
300+0 records in
300+0 records out
314572800 bytes (300.0MB) copied, 1.030541 seconds, 291.1MB/s
real0m 1.03s
user0m 0.00s
sys 0m 0.54s
Signed-off-by: Vivek Gautam
---
drivers/iommu/arm-smmu.c | 16
1 file changed, 16 insertions(+)
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 0ad086da399c..3c3ad43eda97 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -39,6 +39,7 @@
#include
#include
#include
+#include
#include
#include
@@ -177,6 +178,7 @@ struct arm_smmu_device {
u32 features;
#define ARM_SMMU_OPT_SECURE_CFG_ACCESS (1 << 0)
+#define ARM_SMMU_OPT_QCOM_FW_IMPL_SAFE_ERRATA (1 << 1)
u32 options;
enum arm_smmu_arch_version version;
enum arm_smmu_implementationmodel;
@@ -262,6 +264,7 @@ static bool using_legacy_binding, using_generic_binding;
static struct arm_smmu_option_prop arm_smmu_options[] = {
{ ARM_SMMU_OPT_SECURE_CFG_ACCESS, "calxeda,smmu-secure-config-access" },
+ { ARM_SMMU_OPT_QCOM_FW_IMPL_SAFE_ERRATA,
"qcom,smmu-500-fw-impl-safe-errata" },
This should be added to the DT binding as well.
Ah right. I missed that. Will add this and respin unless Robin and Will
have concerns with this change.
{ 0, NULL},
};
@@ -2292,6 +2295,19 @@ static int arm_smmu_device_probe(struct platform_device *pdev)
arm_smmu_device_reset(smmu);
arm_smmu_test_smr_masks(smmu);
+ /*
+* To address performance degradation in non-real time clients,
+* such as USB and UFS, turn off wait-for-safe on sdm845 platforms,
+* such as MTP, whose firmwares implement corresponding secure monitor
+* call handlers.
+*/
+ if (of_device_is_compatible(smmu->dev->of_node, "qcom,sdm845-smmu-500")
&&
+ smmu->options & ARM_SMMU_OPT_QCOM_FW_IMPL_SAFE_ERRATA) {
+ err = qcom_scm_qsmmu500_wait_safe_toggle(0);
+ if (err)
+ dev_warn(dev, "Failed to turn off SAFE logic\n");
+ }
+
This looks good, I presume at some point we can profile things and
review if it's worth toggling this on the fly, but given that this is
conditioned on smmu->options that should be an implementation detail..
Reviewed-by: Bjorn Andersson
Thanks Bjorn.
Best regards
Vivek
Regards,
Bjorn
Re: [PATCH v2] driver: core: Allow subsystems to continue deferring probe
On Thu, Jun 13, 2019 at 07:00:11PM +0200, Thierry Reding wrote:
> From: Thierry Reding
>
> Some subsystems, such as pinctrl, allow continuing to defer probe
> indefinitely. This is useful for devices that depend on resources
> provided by devices that are only probed after the init stage.
>
> One example of this can be seen on Tegra, where the DPAUX hardware
> contains pinmuxing controls for pins that it shares with an I2C
> controller. The I2C controller is typically used for communication
> with a monitor over HDMI (DDC). However, other instances of the I2C
> controller are used to access system critical components, such as a
> PMIC. The I2C controller driver will therefore usually be a builtin
> driver, whereas the DPAUX driver is part of the display driver that
> is loaded from a module to avoid bloating the kernel image with all
> of the DRM/KMS subsystem.
>
> In this particular case the pins used by this I2C/DDC controller
> become accessible very late in the boot process. However, since the
> controller is only used in conjunction with display, that's not an
> issue.
>
> Unfortunately the driver core currently outputs a warning message
> when a device fails to get the pinctrl before the end of the init
> stage. That can be confusing for the user because it may sound like
> an unwanted error occurred, whereas it's really an expected and
> harmless situation.
>
> In order to eliminate this warning, this patch allows callers of the
> driver_deferred_probe_check_state() helper to specify that they want
> to continue deferring probe, regardless of whether we're past the
> init stage or not. All of the callers of that function are updated
> for the new signature, but only the pinctrl subsystem passes a true
> value in the new persist parameter if appropriate.
>
> Signed-off-by: Thierry Reding
> ---
> Changes in v2:
> - pass persist flag via flags parameter to make the function call easier
> to understand
>
> drivers/base/dd.c| 19 ++-
> drivers/base/power/domain.c | 2 +-
> drivers/iommu/of_iommu.c | 2 +-
> drivers/pinctrl/devicetree.c | 9 +
> include/linux/device.h | 18 +-
> 5 files changed, 38 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> index 0df9b4461766..0399a6f6c479 100644
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -238,23 +238,32 @@ __setup("deferred_probe_timeout=",
> deferred_probe_timeout_setup);
> /**
> * driver_deferred_probe_check_state() - Check deferred probe state
> * @dev: device to check
> + * @flags: Flags used to control the behavior of this function. Drivers can
> + * set the DRIVER_DEFER_PROBE_PERSIST flag to indicate that they want to
> + * keep trying to probe after built-in drivers have had a chance to probe.
> + * This is useful for built-in drivers that rely on resources provided by
> + * modular drivers.
> *
> * Returns -ENODEV if init is done and all built-in drivers have had a chance
> - * to probe (i.e. initcalls are done), -ETIMEDOUT if deferred probe debug
> - * timeout has expired, or -EPROBE_DEFER if none of those conditions are met.
> + * to probe (i.e. initcalls are done) and unless the
> DRIVER_DEFER_PROBE_PERSIST
> + * flag is set, -ETIMEDOUT if deferred probe debug timeout has expired, or
> + * -EPROBE_DEFER if none of those conditions are met.
> *
> * Drivers or subsystems can opt-in to calling this function instead of
> directly
> * returning -EPROBE_DEFER.
> */
> -int driver_deferred_probe_check_state(struct device *dev)
> +int driver_deferred_probe_check_state(struct device *dev, unsigned long
> flags)
> {
> if (initcalls_done) {
> if (!deferred_probe_timeout) {
> dev_WARN(dev, "deferred probe timeout, ignoring
> dependency");
> return -ETIMEDOUT;
> }
> - dev_warn(dev, "ignoring dependency for device, assuming no
> driver");
> - return -ENODEV;
> +
> + if ((flags & DRIVER_DEFER_PROBE_PERSIST) == 0) {
> + dev_warn(dev, "ignoring dependency for device, assuming
> no driver");
> + return -ENODEV;
> + }
> }
> return -EPROBE_DEFER;
> }
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index 33c30c1e6a30..6198c6a30fe2 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -2423,7 +2423,7 @@ static int __genpd_dev_pm_attach(struct device *dev,
> struct device *base_dev,
> mutex_unlock(_list_lock);
> dev_dbg(dev, "%s() failed to find PM domain: %ld\n",
> __func__, PTR_ERR(pd));
> - return driver_deferred_probe_check_state(base_dev);
> + return driver_deferred_probe_check_state(base_dev, 0);
Again, I said no odd flags for functions, how is anyone supposed to know
what "0" means here?
You just
Re: [PATCH v2] driver: core: Allow subsystems to continue deferring probe
On Thu, Jun 13, 2019 at 7:00 PM Thierry Reding wrote:
>
> From: Thierry Reding
>
> Some subsystems, such as pinctrl, allow continuing to defer probe
> indefinitely. This is useful for devices that depend on resources
> provided by devices that are only probed after the init stage.
>
> One example of this can be seen on Tegra, where the DPAUX hardware
> contains pinmuxing controls for pins that it shares with an I2C
> controller. The I2C controller is typically used for communication
> with a monitor over HDMI (DDC). However, other instances of the I2C
> controller are used to access system critical components, such as a
> PMIC. The I2C controller driver will therefore usually be a builtin
> driver, whereas the DPAUX driver is part of the display driver that
> is loaded from a module to avoid bloating the kernel image with all
> of the DRM/KMS subsystem.
>
> In this particular case the pins used by this I2C/DDC controller
> become accessible very late in the boot process. However, since the
> controller is only used in conjunction with display, that's not an
> issue.
>
> Unfortunately the driver core currently outputs a warning message
> when a device fails to get the pinctrl before the end of the init
> stage. That can be confusing for the user because it may sound like
> an unwanted error occurred, whereas it's really an expected and
> harmless situation.
>
> In order to eliminate this warning, this patch allows callers of the
> driver_deferred_probe_check_state() helper to specify that they want
> to continue deferring probe, regardless of whether we're past the
> init stage or not. All of the callers of that function are updated
> for the new signature, but only the pinctrl subsystem passes a true
> value in the new persist parameter if appropriate.
>
> Signed-off-by: Thierry Reding
> ---
> Changes in v2:
> - pass persist flag via flags parameter to make the function call easier
> to understand
>
> drivers/base/dd.c| 19 ++-
> drivers/base/power/domain.c | 2 +-
> drivers/iommu/of_iommu.c | 2 +-
> drivers/pinctrl/devicetree.c | 9 +
> include/linux/device.h | 18 +-
> 5 files changed, 38 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> index 0df9b4461766..0399a6f6c479 100644
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -238,23 +238,32 @@ __setup("deferred_probe_timeout=",
> deferred_probe_timeout_setup);
> /**
> * driver_deferred_probe_check_state() - Check deferred probe state
> * @dev: device to check
> + * @flags: Flags used to control the behavior of this function. Drivers can
> + * set the DRIVER_DEFER_PROBE_PERSIST flag to indicate that they want to
What about calling this flag DRIVER_DEFER_PROBE_CONTINUE ?
Also, I would just say
@flags: Flags used to control the behavior of this function.
here and added the description of the flag below.
> + * keep trying to probe after built-in drivers have had a chance to probe.
> + * This is useful for built-in drivers that rely on resources provided by
> + * modular drivers.
> *
> * Returns -ENODEV if init is done and all built-in drivers have had a chance
> - * to probe (i.e. initcalls are done), -ETIMEDOUT if deferred probe debug
> - * timeout has expired, or -EPROBE_DEFER if none of those conditions are met.
> + * to probe (i.e. initcalls are done) and unless the
> DRIVER_DEFER_PROBE_PERSIST
"unless DRIVER_DEFER_PROBE_CONTINUE is set in @flags"
> + * flag is set, -ETIMEDOUT if deferred probe debug timeout has expired, or
> + * -EPROBE_DEFER if none of those conditions are met.
> *
> * Drivers or subsystems can opt-in to calling this function instead of
> directly
> * returning -EPROBE_DEFER.
And here:
"In that case, passing DRIVER_DEFER_PROBE_CONTINUE in @flags indicates
that the caller wants to keep trying to probe after built-in drivers
have had a chance to probe, which is useful for built-in drivers that
rely on resources provided by modular drivers."
> */
> -int driver_deferred_probe_check_state(struct device *dev)
> +int driver_deferred_probe_check_state(struct device *dev, unsigned long
> flags)
> {
> if (initcalls_done) {
> if (!deferred_probe_timeout) {
> dev_WARN(dev, "deferred probe timeout, ignoring
> dependency");
> return -ETIMEDOUT;
> }
> - dev_warn(dev, "ignoring dependency for device, assuming no
> driver");
> - return -ENODEV;
> +
> + if ((flags & DRIVER_DEFER_PROBE_PERSIST) == 0) {
What about
if (!(flags & DRIVER_DEFER_PROBE_PERSIST)) {
> + dev_warn(dev, "ignoring dependency for device,
> assuming no driver");
> + return -ENODEV;
> + }
> }
> return -EPROBE_DEFER;
> }
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index
Re: [PATCH v3 4/4] arm64: dts/sdm845: Enable FW implemented safe sequence handler on MTP
On 6/14/2019 9:36 AM, Bjorn Andersson wrote: On Wed 12 Jun 00:15 PDT 2019, Vivek Gautam wrote: Indicate on MTP SDM845 that firmware implements handler to TLB invalidate erratum SCM call where SAFE sequence is toggled to achieve optimum performance on real-time clients, such as display and camera. Signed-off-by: Vivek Gautam Reviewed-by: Bjorn Andersson Thanks Bjorn for reviewing this. Best regards Vivek [snip] ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/2] dma-mapping: truncate dma masks to what dma_addr_t can hold
If I don't hear anything back in the next days I will just merge
these patches, please comment.
On Wed, May 29, 2019 at 02:22:19PM +0200, Christoph Hellwig wrote:
> Russell,
>
> any additional comments on this series?
>
> On Tue, May 21, 2019 at 03:15:03PM +0200, Christoph Hellwig wrote:
> > On Tue, May 21, 2019 at 02:04:37PM +0100, Russell King - ARM Linux admin
> > wrote:
> > > So how does the driver negotiation for >32bit addresses work if we don't
> > > fail for large masks?
> > >
> > > I'm thinking about all those PCI drivers that need DAC cycles for >32bit
> > > addresses, such as e1000, which negotiate via (eg):
> > >
> > > /* there is a workaround being applied below that limits
> > > * 64-bit DMA addresses to 64-bit hardware. There are some
> > > * 32-bit adapters that Tx hang when given 64-bit DMA addresses
> > > */
> > > pci_using_dac = 0;
> > > if ((hw->bus_type == e1000_bus_type_pcix) &&
> > > !dma_set_mask_and_coherent(>dev, DMA_BIT_MASK(64))) {
> > > pci_using_dac = 1;
> > > } else {
> > > err = dma_set_mask_and_coherent(>dev,
> > > DMA_BIT_MASK(32));
> > > if (err) {
> > > pr_err("No usable DMA config, aborting\n");
> > > goto err_dma;
> > > }
> > > }
> > >
> > > and similar. If we blindly trunate the 64-bit to 32-bit, aren't we
> > > going to end up with PCI cards using DAC cycles to a host bridge that
> > > do not support DAC cycles?
> >
> > In general PCI devices just use DAC cycles when they need it. I only
> > know of about a handful of devices that need to negotiate their
> > addressing mode, and those already use the proper API for that, which
> > is dma_get_required_mask.
> >
> > The e1000 example is a good case of how the old API confused people.
> > First it only sets the 64-bit mask for devices which can support it,
> > which is good, but then it sets the NETIF_F_HIGHDMA flag only if we
> > set a 64-bit mask, which is completely unrelated to the DMA mask,
> > it just means the driver can handle sk_buff fragments that do not
> > have a kernel mapping, which really is a driver and not a hardware
> > issue.
> >
> > So what this driver really should do is something like:
> >
> >
> > diff --git a/drivers/net/ethernet/intel/e1000/e1000_main.c
> > b/drivers/net/ethernet/intel/e1000/e1000_main.c
> > index 551de8c2fef2..d9236083da94 100644
> > --- a/drivers/net/ethernet/intel/e1000/e1000_main.c
> > +++ b/drivers/net/ethernet/intel/e1000/e1000_main.c
> > @@ -925,7 +925,7 @@ static int e1000_probe(struct pci_dev *pdev, const
> > struct pci_device_id *ent)
> >
> > static int cards_found;
> > static int global_quad_port_a; /* global ksp3 port a indication */
> > - int i, err, pci_using_dac;
> > + int i, err;
> > u16 eeprom_data = 0;
> > u16 tmp = 0;
> > u16 eeprom_apme_mask = E1000_EEPROM_APME;
> > @@ -996,16 +996,11 @@ static int e1000_probe(struct pci_dev *pdev, const
> > struct pci_device_id *ent)
> > * 64-bit DMA addresses to 64-bit hardware. There are some
> > * 32-bit adapters that Tx hang when given 64-bit DMA addresses
> > */
> > - pci_using_dac = 0;
> > - if ((hw->bus_type == e1000_bus_type_pcix) &&
> > - !dma_set_mask_and_coherent(>dev, DMA_BIT_MASK(64))) {
> > - pci_using_dac = 1;
> > - } else {
> > - err = dma_set_mask_and_coherent(>dev, DMA_BIT_MASK(32));
> > - if (err) {
> > - pr_err("No usable DMA config, aborting\n");
> > - goto err_dma;
> > - }
> > + err = dma_set_mask_and_coherent(>dev,
> > + DMA_BIT_MASK(hw->bus_type == e1000_bus_type_pcix ? 64 : 32));
> > + if (err) {
> > + pr_err("No usable DMA config, aborting\n");
> > + goto err_dma;
> > }
> >
> > netdev->netdev_ops = _netdev_ops;
> > @@ -1047,19 +1042,15 @@ static int e1000_probe(struct pci_dev *pdev, const
> > struct pci_device_id *ent)
> >
> > netdev->priv_flags |= IFF_SUPP_NOFCS;
> >
> > - netdev->features |= netdev->hw_features;
> > + netdev->features |= netdev->hw_features | NETIF_F_HIGHDMA;
> > netdev->hw_features |= (NETIF_F_RXCSUM |
> > NETIF_F_RXALL |
> > NETIF_F_RXFCS);
> >
> > - if (pci_using_dac) {
> > - netdev->features |= NETIF_F_HIGHDMA;
> > - netdev->vlan_features |= NETIF_F_HIGHDMA;
> > - }
> > -
> > netdev->vlan_features |= (NETIF_F_TSO |
> > NETIF_F_HW_CSUM |
> > - NETIF_F_SG);
> > + NETIF_F_SG |
> > + NETIF_F_HIGHDMA);
> >
> > /* Do not set IFF_UNICAST_FLT for VMWare's 82545EM */
> > if (hw->device_id != E1000_DEV_ID_82545EM_COPPER ||
> >
> ---end quoted text---
---end quoted text---
Re: [PATCH v8 5/7] iommu: Add virtio-iommu driver
Hi jean,
On 5/30/19 7:09 PM, Jean-Philippe Brucker wrote:
> The virtio IOMMU is a para-virtualized device, allowing to send IOMMU
> requests such as map/unmap over virtio transport without emulating page
> tables. This implementation handles ATTACH, DETACH, MAP and UNMAP
> requests.
>
> The bulk of the code transforms calls coming from the IOMMU API into
> corresponding virtio requests. Mappings are kept in an interval tree
> instead of page tables. A little more work is required for modular and x86
> support, so for the moment the driver depends on CONFIG_VIRTIO=y and
> CONFIG_ARM64.
>
> Acked-by: Joerg Roedel
> Signed-off-by: Jean-Philippe Brucker
Reviewed-by: Eric Auger
Thanks
Eric
> ---
> MAINTAINERS | 7 +
> drivers/iommu/Kconfig | 11 +
> drivers/iommu/Makefile| 1 +
> drivers/iommu/virtio-iommu.c | 934 ++
> include/uapi/linux/virtio_ids.h | 1 +
> include/uapi/linux/virtio_iommu.h | 110
> 6 files changed, 1064 insertions(+)
> create mode 100644 drivers/iommu/virtio-iommu.c
> create mode 100644 include/uapi/linux/virtio_iommu.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 429c6c624861..62bd1834d95a 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -16807,6 +16807,13 @@ S: Maintained
> F: drivers/virtio/virtio_input.c
> F: include/uapi/linux/virtio_input.h
>
> +VIRTIO IOMMU DRIVER
> +M: Jean-Philippe Brucker
> +L: virtualizat...@lists.linux-foundation.org
> +S: Maintained
> +F: drivers/iommu/virtio-iommu.c
> +F: include/uapi/linux/virtio_iommu.h
> +
> VIRTUAL BOX GUEST DEVICE DRIVER
> M: Hans de Goede
> M: Arnd Bergmann
> diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
> index 83664db5221d..e15cdcd8cb3c 100644
> --- a/drivers/iommu/Kconfig
> +++ b/drivers/iommu/Kconfig
> @@ -473,4 +473,15 @@ config HYPERV_IOMMU
> Stub IOMMU driver to handle IRQs as to allow Hyper-V Linux
> guests to run with x2APIC mode enabled.
>
> +config VIRTIO_IOMMU
> + bool "Virtio IOMMU driver"
> + depends on VIRTIO=y
> + depends on ARM64
> + select IOMMU_API
> + select INTERVAL_TREE
> + help
> + Para-virtualised IOMMU driver with virtio.
> +
> + Say Y here if you intend to run this kernel as a guest.
> +
> endif # IOMMU_SUPPORT
> diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile
> index 8c71a15e986b..f13f36ae1af6 100644
> --- a/drivers/iommu/Makefile
> +++ b/drivers/iommu/Makefile
> @@ -33,3 +33,4 @@ obj-$(CONFIG_FSL_PAMU) += fsl_pamu.o fsl_pamu_domain.o
> obj-$(CONFIG_S390_IOMMU) += s390-iommu.o
> obj-$(CONFIG_QCOM_IOMMU) += qcom_iommu.o
> obj-$(CONFIG_HYPERV_IOMMU) += hyperv-iommu.o
> +obj-$(CONFIG_VIRTIO_IOMMU) += virtio-iommu.o
> diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c
> new file mode 100644
> index ..b2719a87c3c5
> --- /dev/null
> +++ b/drivers/iommu/virtio-iommu.c
> @@ -0,0 +1,934 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Virtio driver for the paravirtualized IOMMU
> + *
> + * Copyright (C) 2019 Arm Limited
> + */
> +
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include
> +
> +#define MSI_IOVA_BASE0x800
> +#define MSI_IOVA_LENGTH 0x10
> +
> +#define VIOMMU_REQUEST_VQ0
> +#define VIOMMU_NR_VQS1
> +
> +struct viommu_dev {
> + struct iommu_device iommu;
> + struct device *dev;
> + struct virtio_device*vdev;
> +
> + struct ida domain_ids;
> +
> + struct virtqueue*vqs[VIOMMU_NR_VQS];
> + spinlock_t request_lock;
> + struct list_headrequests;
> +
> + /* Device configuration */
> + struct iommu_domain_geometrygeometry;
> + u64 pgsize_bitmap;
> + u32 first_domain;
> + u32 last_domain;
> + /* Supported MAP flags */
> + u32 map_flags;
> +};
> +
> +struct viommu_mapping {
> + phys_addr_t paddr;
> + struct interval_tree_node iova;
> + u32 flags;
> +};
> +
> +struct viommu_domain {
> + struct iommu_domain domain;
> + struct viommu_dev *viommu;
> + struct mutexmutex; /* protects viommu pointer */
> + unsigned intid;
> + u32 map_flags;
> +
> + spinlock_t mappings_lock;
> + struct rb_root_cached mappings;
> +
> + unsigned long nr_endpoints;
> +};
Re: [RFC PATCH v6 4/5] mmc: tmio: Use dma_max_mapping_size() instead of a workaround
Hi Christoph,
On Fri, Jun 14, 2019 at 9:18 AM Christoph Hellwig wrote:
> On Thu, Jun 13, 2019 at 10:35:44PM +0200, Geert Uytterhoeven wrote:
> > I'm always triggered by the use of min_t() and other casts:
> > mmc->max_blk_size and mmc->max_blk_count are both unsigned int.
> > dma_max_mapping_size() returns size_t, which can be 64-bit.
> >
> > 1) Can the multiplication overflow?
> > Probably not, as per commit 2a55c1eac7882232 ("mmc: renesas_sdhi:
> > prevent overflow for max_req_size"), but I thought I'd better ask.
> > 2) In theory, dma_max_mapping_size() can return a number that doesn't
> > fit in 32-bit, and will be truncated (to e.g. 0), leading to
> > max_req_size
> > is zero?
>
> This really should use a min_t on size_t. Otherwise the patch looks
> fine:
Followed by another min() to make it fit in mmc->max_req_size, which is
unsigned int.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH v6 5/5] mmc: queue: Use bigger segments if IOMMU can merge the segments
On Thu, Jun 13, 2019 at 07:20:15PM +0900, Yoshihiro Shimoda wrote:
> +static unsigned int mmc_get_max_segments(struct mmc_host *host)
> +{
> + return host->can_merge ? BLK_MAX_SEGMENTS : host->max_segs;
> +}
Note that BLK_MAX_SEGMENTS is really a little misnamed, it just
is a BLK_DEFAULT_SEGMENTS. I think you are better of picking your
own value here (even if 128 ends up ok) than reusing this somewhat
confusing constant.
> + /*
> + * Since blk_mq_alloc_tag_set() calls .init_request() of mmc_mq_ops,
> + * the host->can_merge should be set before to get max_segs from
> + * mmc_get_max_segments().
> + */
> + if (host->max_segs < BLK_MAX_SEGMENTS &&
> + device_iommu_mapped(mmc_dev(host)))
> + host->can_merge = 1;
> + else
> + host->can_merge = 0;
> +
can_merge seems a little too generic a name to me. Maybe can_iommu_merge?
Re: [RFC PATCH v6 3/5] block: add a helper function to merge the segments by an IOMMU
I'm a little worried about this directly calling into the iommu API instead of going through the DMA mapping code. We still have plenty of iommus not using the iommu layer for DMA mapping. But at least this code is in the block layer and not the driver, so maybe we can live with it.
Re: [RFC PATCH v6 4/5] mmc: tmio: Use dma_max_mapping_size() instead of a workaround
On Thu, Jun 13, 2019 at 10:35:44PM +0200, Geert Uytterhoeven wrote:
> I'm always triggered by the use of min_t() and other casts:
> mmc->max_blk_size and mmc->max_blk_count are both unsigned int.
> dma_max_mapping_size() returns size_t, which can be 64-bit.
>
> 1) Can the multiplication overflow?
> Probably not, as per commit 2a55c1eac7882232 ("mmc: renesas_sdhi:
> prevent overflow for max_req_size"), but I thought I'd better ask.
> 2) In theory, dma_max_mapping_size() can return a number that doesn't
> fit in 32-bit, and will be truncated (to e.g. 0), leading to max_req_size
> is zero?
This really should use a min_t on size_t. Otherwise the patch looks
fine:
Reviewed-by: Christoph Hellwig
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH v6 1/5] iommu: add an exported function to get minimum page size for a domain
On Thu, Jun 13, 2019 at 09:37:59PM +0200, Wolfram Sang wrote: > What about making this a 'static inline' in the iommu header file? I'd > think it is simple enough and would save us the EXPORT symbol. Agreed, this seems simple enought for an inline.
Re: [PATCH 01/10] iommu/exynos: convert to SPDX license tags
Thomas Gleixner is doing automated SPDX conversion that directly got to Linux at the moment. I'd avoid doing more manual ones for now as it will just create conflicts.
Re: switch nios2 and microblaze to use the generic uncached segement support
On Fri, Jun 14, 2019 at 06:11:00AM +, Tan, Ley Foon wrote: > On Fri, 2019-06-14 at 07:44 +0200, Christoph Hellwig wrote: > > On Fri, Jun 14, 2019 at 09:40:34AM +0800, Ley Foon Tan wrote: > > > > > > Hi Christoph > > > > > > Can this patch in http://git.infradead.org/users/hch/dma-mapping.gi > > > t/sh > > > ortlog/refs/heads/for-next > > > > > > [PATCH 1/2] nios2: use the generic uncached segment support in dma- > > > direct > > Hi Ley Foon, > > > > I don't understand the above sentence. Does it imply a Reviewed-by? > Sorry, typo in my previous email. > Can't find this patch in the git link you provided (for-next branch). > Did you push the patch? No, I did not push the microblaze and nios2 patches there. The for-next patch just has the baseline, you'll need to apply the nios2 on top of that branch to test it. If it tests good and you are fine with it I'd like to apply it to that branch. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 01/10] iommu/exynos: convert to SPDX license tags
On Thu, 13 Jun 2019 at 18:27, Yangtao Li wrote: > > Updates license to use SPDX-License-Identifier. > > Signed-off-by: Yangtao Li > --- > drivers/iommu/exynos-iommu.c | 5 + > 1 file changed, 1 insertion(+), 4 deletions(-) Splitting this per driver is too much... it is not necessary. Such changes should be done in one patch per subsystem. Best regards, Krzysztof
Re: switch nios2 and microblaze to use the generic uncached segement support
On Fri, 2019-06-14 at 07:44 +0200, Christoph Hellwig wrote: > On Fri, Jun 14, 2019 at 09:40:34AM +0800, Ley Foon Tan wrote: > > > > Hi Christoph > > > > Can this patch in http://git.infradead.org/users/hch/dma-mapping.gi > > t/sh > > ortlog/refs/heads/for-next > > > > [PATCH 1/2] nios2: use the generic uncached segment support in dma- > > direct > Hi Ley Foon, > > I don't understand the above sentence. Does it imply a Reviewed-by? Sorry, typo in my previous email. Can't find this patch in the git link you provided (for-next branch). Did you push the patch? Regards Ley Foon ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
