Re: [PATCH] media: dvb: get rid of VIDEO_SET_SPU_PALETTE

[Christoph Hellwig]

On Mon, May 28, 2018 at 11:32:41AM -0300, Mauro Carvalho Chehab wrote:
> No upstream drivers use it. It doesn't make any sense to have
> a compat32 code for something that nobody uses upstream.
> 
> Reported-by: Alexander Viro 
> Signed-off-by: Mauro Carvalho Chehab 

Looks good,

Reviewed-by: Christoph Hellwig 

Re: [Linaro-mm-sig] noveau vs arm dma ops

[Christoph Hellwig]

On Thu, Apr 26, 2018 at 11:20:44AM +0200, Daniel Vetter wrote:
> The above is already what we're implementing in i915, at least
> conceptually (it all boils down to clflush instructions because those
> both invalidate and flush).

The clwb instruction that just writes back dirty cache lines might
be very interesting for the x86 non-coherent dma case.  A lot of
architectures use their equivalent to prepare to to device transfers.

> One architectural guarantee we're exploiting is that prefetched (and
> hence non-dirty) cachelines will never get written back, but dropped
> instead.

And to make this work you'll need exactly this guarantee.

Re: noveau vs arm dma ops

[Christoph Hellwig]

On Wed, Apr 25, 2018 at 11:54:43PM +0100, Russell King - ARM Linux wrote:
> 
> if the memory was previously dirty (iow, CPU has written), you need to
> flush the dirty cache lines _before_ the GPU writes happen, but you
> don't know whether the CPU has speculatively prefetched, so you need
> to flush any prefetched cache lines before reading from the cacheable
> memory _after_ the GPU has finished writing.
> 
> Also note that "flush" there can be "clean the cache", "clean and
> invalidate the cache" or "invalidate the cache" as appropriate - some
> CPUs are able to perform those three operations, and the appropriate
> one depends on not only where in the above sequence it's being used,
> but also on what the operations are.

Agreed on all these counts.

> If we can agree a set of interfaces that allows _proper_ use of these
> facilities, one which can be used appropriately, then there shouldn't
> be a problem.  The DMA API does that via it's ideas about who owns a
> particular buffer (because of the above problem) and that's something
> which would need to be carried over to such a cache flushing API (it
> should be pretty obvious that having a GPU read or write memory while
> the cache for that memory is being cleaned will lead to unexpected
> results.)

I've been trying to come up with such an interface, for now only for
internal use in a generic set of noncoherent ops.  The API is basically
a variant of the existing dma_sync_single_to_device/cpu calls:

http://git.infradead.org/users/hch/misc.git/commitdiff/044dae5f94509288f4655de2f22cb0bea85778af

Review welcome!

> The next issue, which I've brought up before, is that exposing cache
> flushing to userspace on architectures where it isn't already exposed
> comes.  As has been shown by Google Project Zero, this risks exposing
> those architectures to Spectre and Meltdown exploits where they weren't
> at such a risk before.  (I've pretty much shown here that you _do_
> need to control which cache lines get flushed to make these exploits
> work, and flushing the cache by reading lots of data in liu of having
> the ability to explicitly flush bits of cache makes it very difficult
> to impossible for them to work.)

Extending dma coherence to userspace is going to be the next major
nightmare indeed.  I'm not sure how much of that actually still is
going on in the graphics world, but we'll need a coherent (pun intended)
plan how to deal with it.

Re: noveau vs arm dma ops

[Christoph Hellwig]

On Wed, Apr 25, 2018 at 11:35:13PM +0200, Daniel Vetter wrote:
> > get_required_mask() is supposed to tell you if you are safe.  However
> > we are missing lots of implementations of it for iommus so you might get
> > some false negatives, improvements welcome.  It's been on my list of
> > things to fix in the DMA API, but it is nowhere near the top.
> 
> I hasn't come up in a while in some fireworks, so I honestly don't
> remember exactly what the issues have been. But
> 
> commit d766ef53006c2c38a7fe2bef0904105a793383f2
> Author: Chris Wilson 
> Date:   Mon Dec 19 12:43:45 2016 +
> 
> drm/i915: Fallback to single PAGE_SIZE segments for DMA remapping
> 
> and the various bits of code that a
> 
> $ git grep SWIOTLB -- drivers/gpu
> 
> turns up is what we're doing to hack around that stuff. And in general
> (there's some exceptions) gpus should be able to address everything,
> so I never fully understood where that's even coming from.

I'm pretty sure I've seen some oddly low dma masks in GPU drivers.  E.g.
duplicated in various AMD files:

adev->need_dma32 = false;
dma_bits = adev->need_dma32 ? 32 : 40;
r = pci_set_dma_mask(adev->pdev, DMA_BIT_MASK(dma_bits));
if (r) {
adev->need_dma32 = true;
dma_bits = 32;
dev_warn(adev->dev, "amdgpu: No suitable DMA available.\n");
}

synopsis:

drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c:pdevinfo.dma_mask   
= DMA_BIT_MASK(32);
drivers/gpu/drm/bridge/synopsys/dw-hdmi.c:  pdevinfo.dma_mask = 
DMA_BIT_MASK(32);
drivers/gpu/drm/bridge/synopsys/dw-hdmi.c:  pdevinfo.dma_mask = 
DMA_BIT_MASK(32);

etnaviv gets it right:

drivers/gpu/drm/etnaviv/etnaviv_gpu.c:  u32 dma_mask = 
(u32)dma_get_required_mask(gpu->dev);


But yes, the swiotlb hackery really irks me.  I just have some more
important and bigger fires to fight first, but I plan to get back to the
root cause of that eventually.

> 
> >> - dma api hides the cache flushing requirements from us. GPUs love
> >>   non-snooped access, and worse give userspace control over that. We want
> >>   a strict separation between mapping stuff and flushing stuff. With the
> >>   IOMMU api we mostly have the former, but for the later arch maintainers
> >>   regularly tells they won't allow that. So we have drm_clflush.c.
> >
> > The problem is that a cache flushing API entirely separate is hard. That
> > being said if you look at my generic dma-noncoherent API series it tries
> > to move that way.  So far it is in early stages and apparently rather
> > buggy unfortunately.
> 
> I'm assuming this stuff here?
> 
> https://lkml.org/lkml/2018/4/20/146
> 
> Anyway got lost in all that work a bit, looks really nice.

That url doesn't seem to work currently.  But I am talking about the
thread titled '[RFC] common non-cache coherent direct dma mapping ops'

> Yeah the above is pretty much what we do on x86. dma-api believes
> everything is coherent, so dma_map_sg does the mapping we want and
> nothing else (minus swiotlb fun). Cache flushing, allocations, all
> done by the driver.

Which sounds like the right thing to do to me.

> On arm that doesn't work. The iommu api seems like a good fit, except
> the dma-api tends to get in the way a bit (drm/msm apparently has
> similar problems like tegra), and if you need contiguous memory
> dma_alloc_coherent is the only way to get at contiguous memory. There
> was a huge discussion years ago about that, and direct cma access was
> shot down because it would have exposed too much of the caching
> attribute mangling required (most arm platforms need wc-pages to not
> be in the kernel's linear map apparently).

Simple cma_alloc() doesn't do anything about cache handling, it
just is a very dumb allocator for large contiguous regions inside
a big pool.

I'm not the CMA maintainer, but in general I'd love to see an
EXPORT_SYMBOL_GPL slapped onto cma_alloc/release and drivers use
that were needed.  Using that plus dma_map*/dma_unmap* sounds like
a much saner interface than dma_alloc_attrs + DMA_ATTR_NON_CONSISTENT
or DMA_ATTR_NO_KERNEL_MAPPING.

You don't happen to have a pointer to that previous discussion?

> Anything that separate these 3 things more (allocation pools, mapping
> through IOMMUs and flushing cpu caches) sounds like the right
> direction to me. Even if that throws some portability across platforms
> away - drivers who want to control things in this much detail aren't
> really portable (without some serious work) anyway.

As long as we stay away from the dma coherent allocations separating
them is fine, and I'm working towards it.  dma coherent allocations on
the other hand are very hairy beasts, and provide by very different
implementations depending on the architecture, or often even depending
on the specifics of individual implementations inside the architecture.

So for your GPU/media case it seems like you'd better 

Re: noveau vs arm dma ops

[Christoph Hellwig]

On Wed, Apr 25, 2018 at 12:04:29PM +0200, Daniel Vetter wrote:
> > Coordinating the backport of a trivial helper in the arm tree is not
> > the end of the world.  Really, this cowboy attitude is a good reason
> > why graphics folks have such a bad rep.  You keep poking into random
> > kernel internals, don't talk to anoyone and then complain if people
> > are upset.  This shouldn't be surprising.
> 
> Not really agreeing on the cowboy thing. The fundamental problem is that
> the dma api provides abstraction that seriously gets in the way of writing
> a gpu driver. Some examples:

So talk to other people.  Maybe people share your frustation.  Or maybe
other people have a way to help.

> - We never want bounce buffers, ever. dma_map_sg gives us that, so there's
>   hacks to fall back to a cache of pages allocated using
>   dma_alloc_coherent if you build a kernel with bounce buffers.

get_required_mask() is supposed to tell you if you are safe.  However
we are missing lots of implementations of it for iommus so you might get
some false negatives, improvements welcome.  It's been on my list of
things to fix in the DMA API, but it is nowhere near the top.

> - dma api hides the cache flushing requirements from us. GPUs love
>   non-snooped access, and worse give userspace control over that. We want
>   a strict separation between mapping stuff and flushing stuff. With the
>   IOMMU api we mostly have the former, but for the later arch maintainers
>   regularly tells they won't allow that. So we have drm_clflush.c.

The problem is that a cache flushing API entirely separate is hard. That
being said if you look at my generic dma-noncoherent API series it tries
to move that way.  So far it is in early stages and apparently rather
buggy unfortunately.

> - dma api hides how/where memory is allocated. Kinda similar problem,
>   except now for CMA or address limits. So either we roll our own
>   allocators and then dma_map_sg (and pray it doesn't bounce buffer), or
>   we use dma_alloc_coherent and then grab the sgt to get at the CMA
>   allocations because that's the only way. Which sucks, because we can't
>   directly tell CMA how to back off if there's some way to make CMA memory
>   available through other means (gpus love to hog all of memory, so we
>   have shrinkers and everything).

If you really care about doing explicitly cache flushing anyway (see
above) allocating your own memory and mapping it where needed is by
far the superior solution.  On cache coherent architectures
dma_alloc_coherent is nothing but allocate memory + dma_map_single.
On non coherent allocations the memory might come through a special
pool or must be used through a special virtual address mapping that
is set up either statically or dynamically.  For that case splitting
allocation and mapping is a good idea in many ways, and I plan to move
towards that once the number of dma mapping implementations is down
to a reasonable number so that it can actually be done.

Re: [PATCH] media: zoran: move to dma-mapping interface

[Christoph Hellwig]

On Wed, Apr 25, 2018 at 01:15:18PM +0200, Arnd Bergmann wrote:
> That thought had occurred to me as well. I removed the oldest ISDN
> drivers already some years ago, and the OSS sound drivers
> got removed as well, and comedi got converted to the dma-mapping
> interfaces, so there isn't much left at all now. This is what we
> have as of v4.17-rc1:

Yes, I've been looking at various grotty old bits to purge.  Usually
I've been looking for some non-tree wide patches and CCed the last
active people to see if they care.  In a few cases people do, but
most often no one does.

> My feeling is that we want to keep most of the arch specific
> ones, in particular removing the m68k drivers would break
> a whole class of machines.

For the arch specific ones it would good to just ping the relevant
maintainers.  Especially m68k and parisc folks seems to be very
responsive.

Re: [Linaro-mm-sig] [PATCH 4/8] dma-buf: add peer2peer flag

[Christoph Hellwig]

On Wed, Apr 25, 2018 at 09:56:43AM +0200, Thierry Reding wrote:
> And to add to the confusion, none of this seems to be an issue on 64-bit
> ARM where the generic DMA/IOMMU code from drivers/iommu/dma-iommu.c is
> used.

In the long term I want everyone to use that code.  Help welcome!

noveau vs arm dma ops

[Christoph Hellwig]

[discussion about this patch, which should have been cced to the iommu
 and linux-arm-kernel lists, but wasn't:
 https://www.spinics.net/lists/dri-devel/msg173630.html]

On Wed, Apr 25, 2018 at 09:41:51AM +0200, Thierry Reding wrote:
> > API from the iommu/dma-mapping code.  Drivers have no business poking
> > into these details.
> 
> The interfaces that the above patch uses are all EXPORT_SYMBOL_GPL,
> which is rather misleading if they are not meant to be used by drivers
> directly.

The only reason the DMA ops are exported is because get_arch_dma_ops
references (or in case of the coherent ones used to reference).  We
don't let drivers assign random dma ops.

> 
> > Thierry, please resend this with at least the iommu list and
> > linux-arm-kernel in Cc to have a proper discussion on the right API.
> 
> I'm certainly open to help with finding a correct solution, but the
> patch above was purposefully terse because this is something that I
> hope we can get backported to v4.16 to unbreak Nouveau. Coordinating
> such a backport between ARM and DRM trees does not sound like something
> that would help getting this fixed in v4.16.

Coordinating the backport of a trivial helper in the arm tree is not
the end of the world.  Really, this cowboy attitude is a good reason
why graphics folks have such a bad rep.  You keep poking into random
kernel internals, don't talk to anoyone and then complain if people
are upset.  This shouldn't be surprising.

> Granted, this issue could've been caught with a little more testing, but
> in retrospect I think it would've been a lot better if ARM_DMA_USE_IOMMU
> was just enabled unconditionally if it has side-effects that platforms
> don't opt in to but have to explicitly opt out of.

Agreed on that count.  Please send a patch.

Re: [PATCH] media: zoran: move to dma-mapping interface

[Christoph Hellwig]

On Wed, Apr 25, 2018 at 09:08:13AM +0200, Arnd Bergmann wrote:
> > That probably also means it can use dma_mmap_coherent instead of the
> > handcrafted remap_pfn_range loop and the PageReserved abuse.
> 
> I'd rather not touch that code. How about adding a comment about
> the fact that it should use dma_mmap_coherent()?

Maybe the real question is if there is anyone that actually cares
for this driver, or if we are better off just removing it?

Same is true for various other virt_to_bus using drivers, e.g. the
grotty atm drivers.

Re: [Linaro-mm-sig] [PATCH 4/8] dma-buf: add peer2peer flag

[Christoph Hellwig]

On Wed, Apr 25, 2018 at 09:02:17AM +0200, Daniel Vetter wrote:
> Can we please not nack everything right away? Doesn't really motivate
> me to show you all the various things we're doing in gpu to make the
> dma layer work for us. That kind of noodling around in lower levels to
> get them to do what we want is absolutely par-for-course for gpu
> drivers. If you just nack everything I point you at for illustrative
> purposes, then I can't show you stuff anymore.

No, it's not.  No driver (and that includes the magic GPUs) has
any business messing with dma ops directly.

A GPU driver imght have a very valid reason to disable the IOMMU,
but the code to do so needs to be at least in the arch code, maybe
in the dma-mapping/iommu code, not in the driver.

As a first step to get the discussion started we'll simply need
to move the code Thierry wrote into a helper in arch/arm and that
alone would be a massive improvement.  I'm not even talking about
minor details like actually using arm_get_dma_map_ops instead
of duplicating it.

And doing this basic trivial work really helps to get this whole
mess under control.

Re: [Linaro-mm-sig] [PATCH 4/8] dma-buf: add peer2peer flag

[Christoph Hellwig]

On Wed, Apr 25, 2018 at 08:23:15AM +0200, Daniel Vetter wrote:
> For more fun:
> 
> https://www.spinics.net/lists/dri-devel/msg173630.html
> 
> Yeah, sometimes we want to disable the iommu because the on-gpu
> pagetables are faster ...

I am not on this list, but remote NAK from here.  This needs an
API from the iommu/dma-mapping code.  Drivers have no business poking
into these details.

Thierry, please resend this with at least the iommu list and
linux-arm-kernel in Cc to have a proper discussion on the right API.

Re: [Linaro-mm-sig] [PATCH 4/8] dma-buf: add peer2peer flag

[Christoph Hellwig]

On Wed, Apr 25, 2018 at 02:24:36AM -0400, Alex Deucher wrote:
> > It has a non-coherent transaction mode (which the chipset can opt to
> > not implement and still flush), to make sure the AGP horror show
> > doesn't happen again and GPU folks are happy with PCIe. That's at
> > least my understanding from digging around in amd the last time we had
> > coherency issues between intel and amd gpus. GPUs have some bits
> > somewhere (in the pagetables, or in the buffer object description
> > table created by userspace) to control that stuff.
> 
> Right.  We have a bit in the GPU page table entries that determines
> whether we snoop the CPU's cache or not.

I can see how that works with the GPU on the same SOC or SOC set as the
CPU.  But how is that going to work for a GPU that is a plain old PCIe
card?  The cache snooping in that case is happening in the PCIe root
complex.

Re: [PATCH] media: zoran: move to dma-mapping interface

[Christoph Hellwig]

On Tue, Apr 24, 2018 at 10:40:45PM +0200, Arnd Bergmann wrote:
> No drivers should use virt_to_bus() any more. This converts
> one of the few remaining ones to the DMA mapping interface.
> 
> Signed-off-by: Arnd Bergmann 
> ---
>  drivers/media/pci/zoran/Kconfig|  2 +-
>  drivers/media/pci/zoran/zoran.h| 10 +--
>  drivers/media/pci/zoran/zoran_card.c   | 10 +--
>  drivers/media/pci/zoran/zoran_device.c | 16 +-
>  drivers/media/pci/zoran/zoran_driver.c | 54 
> +-
>  5 files changed, 63 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/media/pci/zoran/Kconfig b/drivers/media/pci/zoran/Kconfig
> index 39ec35bd21a5..26f40e124a32 100644
> --- a/drivers/media/pci/zoran/Kconfig
> +++ b/drivers/media/pci/zoran/Kconfig
> @@ -1,6 +1,6 @@
>  config VIDEO_ZORAN
>   tristate "Zoran ZR36057/36067 Video For Linux"
> - depends on PCI && I2C_ALGOBIT && VIDEO_V4L2 && VIRT_TO_BUS
> + depends on PCI && I2C_ALGOBIT && VIDEO_V4L2
>   depends on !ALPHA
>   help
> Say Y for support for MJPEG capture cards based on the Zoran
> diff --git a/drivers/media/pci/zoran/zoran.h b/drivers/media/pci/zoran/zoran.h
> index 9bb3c21aa275..9ff3a9acb60a 100644
> --- a/drivers/media/pci/zoran/zoran.h
> +++ b/drivers/media/pci/zoran/zoran.h
> @@ -183,13 +183,14 @@ struct zoran_buffer {
>   struct zoran_sync bs;   /* DONE: info to return to application 
> */
>   union {
>   struct {
> - __le32 *frag_tab;   /* addresses of frag table */
> - u32 frag_tab_bus;   /* same value cached to save 
> time in ISR */
> + __le32 *frag_tab;   /* DMA addresses of frag table 
> */
> + void **frag_virt_tab;   /* virtual addresses of frag 
> table */
> + dma_addr_t frag_tab_dma;/* same value cached to save 
> time in ISR */
>   } jpg;
>   struct {
>   char *fbuffer;  /* virtual address of frame 
> buffer */
>   unsigned long fbuffer_phys;/* physical address of frame 
> buffer */
> - unsigned long fbuffer_bus;/* bus address of frame 
> buffer */
> + dma_addr_t fbuffer_dma;/* bus address of frame buffer */
>   } v4l;
>   };
>  };
> @@ -221,6 +222,7 @@ struct zoran_fh {
>  
>   struct zoran_overlay_settings overlay_settings;
>   u32 *overlay_mask;  /* overlay mask */
> + dma_addr_t overlay_mask_dma;
>   enum zoran_lock_activity overlay_active;/* feature currently in use? */
>  
>   struct zoran_buffer_col buffers;/* buffers' info */
> @@ -307,6 +309,7 @@ struct zoran {
>  
>   struct zoran_overlay_settings overlay_settings;
>   u32 *overlay_mask;  /* overlay mask */
> + dma_addr_t overlay_mask_dma;
>   enum zoran_lock_activity overlay_active;/* feature currently in 
> use? */
>  
>   wait_queue_head_t v4l_capq;
> @@ -346,6 +349,7 @@ struct zoran {
>  
>   /* zr36057's code buffer table */
>   __le32 *stat_com;   /* stat_com[i] is indexed by 
> dma_head/tail & BUZ_MASK_STAT_COM */
> + dma_addr_t stat_com_dma;
>  
>   /* (value & BUZ_MASK_FRAME) corresponds to index in pend[] queue */
>   int jpg_pend[BUZ_MAX_FRAME];
> diff --git a/drivers/media/pci/zoran/zoran_card.c 
> b/drivers/media/pci/zoran/zoran_card.c
> index a6b9ebd20263..dabd8bf77472 100644
> --- a/drivers/media/pci/zoran/zoran_card.c
> +++ b/drivers/media/pci/zoran/zoran_card.c
> @@ -890,6 +890,7 @@ zoran_open_init_params (struct zoran *zr)
>   /* User must explicitly set a window */
>   zr->overlay_settings.is_set = 0;
>   zr->overlay_mask = NULL;
> + zr->overlay_mask_dma = 0;

I don't think this zeroing is required, and given that 0 is a valid
dma address on some platforms is also is rather confusing.:w

>  
>   mask_line_size = (BUZ_MAX_WIDTH + 31) / 32;
> - reg = virt_to_bus(zr->overlay_mask);
> + reg = zr->overlay_mask_dma;
>   btwrite(reg, ZR36057_MMTR);
> - reg = virt_to_bus(zr->overlay_mask + mask_line_size);
> + reg = zr->overlay_mask_dma + mask_line_size;

I think this would be easier to read if the reg assignments got
removed, e.g.

btwrite(zr->overlay_mask_dma, ZR36057_MMTR);
btwrite(zr->overlay_mask_dma + mask_line_size, ZR36057_MMBR);

Same in a few other places.

> + virt_tab = (void *)get_zeroed_page(GFP_KERNEL);
> + if (!mem || !virt_tab) {
>   dprintk(1,
>   KERN_ERR
>   "%s: %s - get_zeroed_page (frag_tab) failed for 
> buffer %d\n",
>   ZR_DEVNAME(zr), __func__, i);
> + kfree(mem);
> + kfree(virt_tab);
>   

Re: [Linaro-mm-sig] [PATCH 4/8] dma-buf: add peer2peer flag

[Christoph Hellwig]

On Tue, Apr 24, 2018 at 09:32:20PM +0200, Daniel Vetter wrote:
> Out of curiosity, how much virtual flushing stuff is there still out
> there? At least in drm we've pretty much ignore this, and seem to be
> getting away without a huge uproar (at least from driver developers
> and users, core folks are less amused about that).

As I've just been wading through the code, the following architectures
have non-coherent dma that flushes by virtual address for at least some
platforms:

 - arm [1], arm64, hexagon, nds32, nios2, parisc, sh, xtensa, mips,
   powerpc

These have non-coherent dma ops that flush by physical address:

 - arc, arm [1], c6x, m68k, microblaze, openrisc, sparc

And these do not have non-coherent dma ops at all:

 - alpha, h8300, riscv, unicore32, x86

[1] arm ѕeems to do both virtually and physically based ops, further
audit needed.

Note that using virtual addresses in the cache flushing interface
doesn't mean that the cache actually is virtually indexed, but it at
least allows for the possibility.

> > I think the most important thing about such a buffer object is that
> > it can distinguish the underlying mapping types.  While
> > dma_alloc_coherent, dma_alloc_attrs with DMA_ATTR_NON_CONSISTENT,
> > dma_map_page/dma_map_single/dma_map_sg and dma_map_resource all give
> > back a dma_addr_t they are in now way interchangable.  And trying to
> > stuff them all into a structure like struct scatterlist that has
> > no indication what kind of mapping you are dealing with is just
> > asking for trouble.
> 
> Well the idea was to have 1 interface to allow all drivers to share
> buffers with anything else, no matter how exactly they're allocated.

Isn't that interface supposed to be dmabuf?  Currently dma_map leaks
a scatterlist through the sg_table in dma_buf_map_attachment /
->map_dma_buf, but looking at a few of the callers it seems like they
really do not even want a scatterlist to start with, but check that
is contains a physically contiguous range first.  So kicking the
scatterlist our there will probably improve the interface in general.

> dma-buf has all the functions for flushing, so you can have coherent
> mappings, non-coherent mappings and pretty much anything else. Or well
> could, because in practice people hack up layering violations until it
> works for the 2-3 drivers they care about. On top of that there's the
> small issue that x86 insists that dma is coherent (and that's true for
> most devices, including v4l drivers you might want to share stuff
> with), and gpus really, really really do want to make almost
> everything incoherent.

How do discrete GPUs manage to be incoherent when attached over PCIe?

Re: [Linaro-mm-sig] [PATCH 4/8] dma-buf: add peer2peer flag

[Christoph Hellwig]

On Fri, Apr 20, 2018 at 05:21:11PM +0200, Daniel Vetter wrote:
> > At the very lowest level they will need to be handled differently for
> > many architectures, the questions is at what point we'll do the
> > branching out.
> 
> Having at least struct page also in that list with (dma_addr_t, lenght)
> pairs has a bunch of benefits for drivers in unifying buffer handling
> code. You just pass that one single list around, use the dma_addr_t side
> for gpu access (generally bashing it into gpu ptes). And the struct page
> (if present) for cpu access, using kmap or vm_insert_*. We generally
> ignore virt, if we do need a full mapping then we construct a vmap for
> that buffer of our own.

Well, for mapping a resource (which gets back to the start of the
discussion) you will need an explicit virt pointer.  You also need
an explicit virt pointer and not just page_address/kmap for users of
dma_get_sgtable, because for many architectures you will need to flush
the virtual address used to access the data, which might be a
vmap/ioremap style mapping retourned from dma_alloc_address, and not
the directly mapped kernel address.

Here is another idea at the low-level dma API level:

 - dma_get_sgtable goes away.  The replacement is a new
   dma_alloc_remap helper that takes the virtual address returned
   from dma_alloc_attrs/coherent and creates a dma_addr_t for the
   given new device.  If the original allocation was a coherent
   one no cache flushing is required either (because the arch
   made sure it is coherent), if the original allocation used
   DMA_ATTR_NON_CONSISTENT the new allocation will need
   dma_cache_sync calls as well.
 - you never even try to share a mapping retourned from
   dma_map_resource - instead each device using it creates a new
   mapping, which makes sense as no virtual addresses are involved
   at all.

> So maybe a list of (struct page *, dma_addr_t, num_pages) would suit best,
> with struct page * being optional (if it's a resource, or something else
> that the kernel core mm isn't aware of). But that only has benefits if we
> really roll it out everywhere, in all the subsystems and drivers, since if
> we don't we've made the struct pages ** <-> sgt conversion fun only worse
> by adding a 3 representation of gpu buffer object backing storage.

I think the most important thing about such a buffer object is that
it can distinguish the underlying mapping types.  While
dma_alloc_coherent, dma_alloc_attrs with DMA_ATTR_NON_CONSISTENT,
dma_map_page/dma_map_single/dma_map_sg and dma_map_resource all give
back a dma_addr_t they are in now way interchangable.  And trying to
stuff them all into a structure like struct scatterlist that has
no indication what kind of mapping you are dealing with is just
asking for trouble.

Re: [PATCH 4/8] dma-buf: add peer2peer flag

[Christoph Hellwig]

On Fri, Apr 20, 2018 at 12:44:01PM +0200, Christian König wrote:
> > > What we need is an sg_alloc_table_from_resources(dev, resources,
> > > num_resources) which does the handling common to all drivers.
> > A structure that contains
> > 
> > {page,offset,len} + {dma_addr+dma_len}
> > 
> > is not a good container for storing
> > 
> > {virt addr, dma_addr, len}
> > 
> > no matter what interface you build arond it.
> 
> Why not? I mean at least for my use case we actually don't need the virtual
> address.

If you don't need the virtual address you need scatterlist even list.

> What we need is {dma_addr+dma_len} in a consistent interface which can come
> from both {page,offset,len} as well as {resource, len}.

Ok.

> What I actually don't need is separate handling for system memory and
> resources, but that would we get exactly when we don't use sg_table.

At the very lowest level they will need to be handled differently for
many architectures, the questions is at what point we'll do the
branching out.

Re: [PATCH 4/8] dma-buf: add peer2peer flag

[Christoph Hellwig]

On Fri, Apr 20, 2018 at 10:58:50AM +0200, Christian König wrote:
> > Yes there's a bit a layering violation insofar that drivers really
> > shouldn't each have their own copy of "how do I convert a piece of dma
> > memory into  dma-buf", but that doesn't render the interface a bad idea.
> 
> Completely agree on that.
> 
> What we need is an sg_alloc_table_from_resources(dev, resources,
> num_resources) which does the handling common to all drivers.

A structure that contains

{page,offset,len} + {dma_addr+dma_len}

is not a good container for storing

{virt addr, dma_addr, len}

no matter what interface you build arond it.  And that is discounting
all the problems around mapping coherent allocations for other devices,
or the iommu merging problem we are having another thread on.

So let's come up with a better high level interface first, and then
worrty about how to implement it in the low-level dma-mapping interface
second.  Especially given that my consolidation of the dma_map_ops
implementation is in full stream and there shoudn't be all that many
to bother with.

So first question:  Do you actually care about having multiple
pairs of the above, or instead of all chunks just deal with a single
of the above?  In that case we really should not need that many
new interfaces as dma_map_resource will be all you need anyway.

> 
> Christian.
> 
> > -Daniel
> 
---end quoted text---

Re: [PATCH 4/8] dma-buf: add peer2peer flag

[Christoph Hellwig]

On Mon, Apr 16, 2018 at 03:38:56PM +0200, Daniel Vetter wrote:
> We've broken that assumption in i915 years ago. Not struct page backed
> gpu memory is very real.
> 
> Of course we'll never feed such a strange sg table to a driver which
> doesn't understand it, but allowing sg_page == NULL works perfectly
> fine. At least for gpu drivers.

For GPU drivers on x86 with no dma coherency problems, sure.  But not
all the world is x86.  We already have problems due to dmabugs use
of the awkward get_sgtable interface (see the common on
arm_dma_get_sgtable that I fully agree with), and doing this for memory
that doesn't have a struct page at all will make things even worse.

> If that's not acceptable then I guess we could go over the entire tree
> and frob all the gpu related code to switch over to a new struct
> sg_table_might_not_be_struct_page_backed, including all the other
> functions we added over the past few years to iterate over sg tables.
> But seems slightly silly, given that sg tables seem to do exactly what
> we need.

It isn't silly.  We will have to do some surgery like that anyway
because the current APIs don't work.  So relax, sit back and come up
with an API that solves the existing issues and serves us well in
the future.

Re: [PATCH 4/8] dma-buf: add peer2peer flag

[Christoph Hellwig]

On Tue, Apr 03, 2018 at 08:08:32PM +0200, Daniel Vetter wrote:
> I did not mean you should dma_map_sg/page. I just meant that using
> dma_map_resource to fill only the dma address part of the sg table seems
> perfectly sufficient.

But that is not how the interface work, especially facing sg_dma_len.

> Assuming you get an sg table that's been mapping by calling dma_map_sg was
> always a bit a case of bending the abstraction to avoid typing code. The
> only thing an importer ever should have done is look at the dma addresses
> in that sg table, nothing else.

The scatterlist is not a very good abstraction unfortunately, but it
it is spread all over the kernel.  And we do expect that anyone who
gets passed a scatterlist can use sg_page() or sg_virt() (which calls
sg_page()) on it.  Your changes would break that, and will cause major
trouble because of that.

If you want to expose p2p memory returned from dma_map_resource in
dmabuf do not use scatterlists for this please, but with a new interface
that explicitly passes a virtual address, a dma address and a length
and make it very clear that virt_to_page will not work on the virtual
address.

Re: [PATCH 2/8] PCI: Add pci_find_common_upstream_dev()

[Christoph Hellwig]

On Thu, Mar 29, 2018 at 09:58:54PM -0400, Jerome Glisse wrote:
> dma_map_resource() is the right API (thought its current implementation
> is fill with x86 assumptions). So i would argue that arch can decide to
> implement it or simply return dma error address which trigger fallback
> path into the caller (at least for GPU drivers). SG variant can be added
> on top.

It isn't in general.  It doesn't integrate with scatterlists (see my
comment to page one), and it doesn't integrate with all the subsystems
that also need a kernel virtual address.

Re: [PATCH 2/8] PCI: Add pci_find_common_upstream_dev()

[Christoph Hellwig]

On Sun, Mar 25, 2018 at 12:59:54PM +0200, Christian König wrote:
> From: "wda...@nvidia.com" 
> 
> Add an interface to find the first device which is upstream of both
> devices.

Please work with Logan and base this on top of the outstanding peer
to peer patchset.

Re: [PATCH 1/8] lib/scatterlist: add sg_set_dma_addr() helper

[Christoph Hellwig]

On Sun, Mar 25, 2018 at 12:59:53PM +0200, Christian König wrote:
> Use this function to set an sg entry to point to device resources mapped
> using dma_map_resource(). The page pointer is set to NULL and only the DMA
> address, length and offset values are valid.

NAK.  Please provide a higher level primitive.

Btw, you series seems to miss a cover letter.

Re: [PATCH 3/4] tsi108_eth: use dma API properly

[Christoph Hellwig]

On Wed, Jan 10, 2018 at 10:09:20PM +0200, Andy Shevchenko wrote:
> > +   struct platform_device *pdev;
> 
> Do you really need platform_defice reference?
> 
> Perhaps
> 
> struct device *hdev; // hardware device
> 
> 
> data->hdev = >dev;
> 
> Another idea
> 
> dev->dev.parent = >dev;
> 
> No new member needed.

Maybe.  But what I've done is the simplest change in a long obsolete
driver that I don't understand at all.  I'd rather keep it simple.

remove pci_dma_* abuses and workarounds V2

[Christoph Hellwig]

Back before the dawn of time pci_dma_* with a NULL pci_dev argument
was used for all kinds of things, e.g. dma mapping for non-PCI
devices.  All this has been long removed, but it turns out we
still care for a NULL pci_dev in the wrappers, and we still have
two odd USB drivers that use pci_dma_alloc_consistent for allocating
memory while ignoring the dma_addr_t entirely, and a network driver
mixing the already wrong usage of dma_* with a NULL device with a
single call to pci_free_consistent.

This series switches the two usb drivers to use plain kzalloc, the
net driver to properly use the dma API and then removes the handling
of the NULL pci_dev in the pci_dma_* wrappers.

Changes since V1:
 - remove allocation failure printks
 - use kcalloc
 - fix tsi108_eth
 - improve changelogs

[PATCH 1/4] media/ttusb-budget: remove pci_zalloc_coherent abuse

[Christoph Hellwig]

Switch to a plain kzalloc instead of pci_zalloc_coherent to allocate
memory for the USB DMA.

Signed-off-by: Christoph Hellwig <h...@lst.de>
---
 drivers/media/usb/ttusb-budget/dvb-ttusb-budget.c | 18 --
 1 file changed, 4 insertions(+), 14 deletions(-)

diff --git a/drivers/media/usb/ttusb-budget/dvb-ttusb-budget.c 
b/drivers/media/usb/ttusb-budget/dvb-ttusb-budget.c
index a142b9dc0feb..ea40a24947ba 100644
--- a/drivers/media/usb/ttusb-budget/dvb-ttusb-budget.c
+++ b/drivers/media/usb/ttusb-budget/dvb-ttusb-budget.c
@@ -102,7 +102,6 @@ struct ttusb {
unsigned int isoc_in_pipe;
 
void *iso_buffer;
-   dma_addr_t iso_dma_handle;
 
struct urb *iso_urb[ISO_BUF_COUNT];
 
@@ -792,26 +791,17 @@ static void ttusb_free_iso_urbs(struct ttusb *ttusb)
 
for (i = 0; i < ISO_BUF_COUNT; i++)
usb_free_urb(ttusb->iso_urb[i]);
-
-   pci_free_consistent(NULL,
-   ISO_FRAME_SIZE * FRAMES_PER_ISO_BUF *
-   ISO_BUF_COUNT, ttusb->iso_buffer,
-   ttusb->iso_dma_handle);
+   kfree(ttusb->iso_buffer);
 }
 
 static int ttusb_alloc_iso_urbs(struct ttusb *ttusb)
 {
int i;
 
-   ttusb->iso_buffer = pci_zalloc_consistent(NULL,
- ISO_FRAME_SIZE * 
FRAMES_PER_ISO_BUF * ISO_BUF_COUNT,
- >iso_dma_handle);
-
-   if (!ttusb->iso_buffer) {
-   dprintk("%s: pci_alloc_consistent - not enough memory\n",
-   __func__);
+   ttusb->iso_buffer = kcalloc(FRAMES_PER_ISO_BUF * ISO_BUF_COUNT,
+   ISO_FRAME_SIZE, GFP_KERNEL);
+   if (!ttusb->iso_buffer)
return -ENOMEM;
-   }
 
for (i = 0; i < ISO_BUF_COUNT; i++) {
struct urb *urb;
-- 
2.14.2

[PATCH 2/4] media/ttusb-dev: remove pci_zalloc_coherent abuse

[Christoph Hellwig]

Switch to a plain kzalloc instead of pci_zalloc_coherent to allocate
memory for the USB DMA.

Signed-off-by: Christoph Hellwig <h...@lst.de>
---
 drivers/media/usb/ttusb-dec/ttusb_dec.c | 18 --
 1 file changed, 4 insertions(+), 14 deletions(-)

diff --git a/drivers/media/usb/ttusb-dec/ttusb_dec.c 
b/drivers/media/usb/ttusb-dec/ttusb_dec.c
index cdefb5dfbbdc..4d5acdf578a6 100644
--- a/drivers/media/usb/ttusb-dec/ttusb_dec.c
+++ b/drivers/media/usb/ttusb-dec/ttusb_dec.c
@@ -127,7 +127,6 @@ struct ttusb_dec {
struct urb  *irq_urb;
dma_addr_t  irq_dma_handle;
void*iso_buffer;
-   dma_addr_t  iso_dma_handle;
struct urb  *iso_urb[ISO_BUF_COUNT];
int iso_stream_count;
struct mutexiso_mutex;
@@ -1185,11 +1184,7 @@ static void ttusb_dec_free_iso_urbs(struct ttusb_dec 
*dec)
 
for (i = 0; i < ISO_BUF_COUNT; i++)
usb_free_urb(dec->iso_urb[i]);
-
-   pci_free_consistent(NULL,
-   ISO_FRAME_SIZE * (FRAMES_PER_ISO_BUF *
- ISO_BUF_COUNT),
-   dec->iso_buffer, dec->iso_dma_handle);
+   kfree(dec->iso_buffer);
 }
 
 static int ttusb_dec_alloc_iso_urbs(struct ttusb_dec *dec)
@@ -1198,15 +1193,10 @@ static int ttusb_dec_alloc_iso_urbs(struct ttusb_dec 
*dec)
 
dprintk("%s\n", __func__);
 
-   dec->iso_buffer = pci_zalloc_consistent(NULL,
-   ISO_FRAME_SIZE * 
(FRAMES_PER_ISO_BUF * ISO_BUF_COUNT),
-   >iso_dma_handle);
-
-   if (!dec->iso_buffer) {
-   dprintk("%s: pci_alloc_consistent - not enough memory\n",
-   __func__);
+   dec->iso_buffer = kcalloc(FRAMES_PER_ISO_BUF * ISO_BUF_COUNT,
+   ISO_FRAME_SIZE, GFP_KERNEL);
+   if (!dec->iso_buffer)
return -ENOMEM;
-   }
 
for (i = 0; i < ISO_BUF_COUNT; i++) {
struct urb *urb;
-- 
2.14.2

[PATCH 3/4] tsi108_eth: use dma API properly

[Christoph Hellwig]

We need to pass a struct device to the dma API, even if some
architectures still support that for legacy reasons, and should not mix
it with the old PCI dma API.

Note that the driver also seems to never actually unmap its dma mappings,
but to fix that we'll need someone more familar with the driver.

Signed-off-by: Christoph Hellwig <h...@lst.de>
---
 drivers/net/ethernet/tundra/tsi108_eth.c | 36 ++--
 1 file changed, 20 insertions(+), 16 deletions(-)

diff --git a/drivers/net/ethernet/tundra/tsi108_eth.c 
b/drivers/net/ethernet/tundra/tsi108_eth.c
index 0624b71ab5d4..edcd1e60b30d 100644
--- a/drivers/net/ethernet/tundra/tsi108_eth.c
+++ b/drivers/net/ethernet/tundra/tsi108_eth.c
@@ -152,6 +152,8 @@ struct tsi108_prv_data {
u32 msg_enable; /* debug message level */
struct mii_if_info mii_if;
unsigned int init_media;
+
+   struct platform_device *pdev;
 };
 
 /* Structure for a device driver */
@@ -703,17 +705,18 @@ static int tsi108_send_packet(struct sk_buff * skb, 
struct net_device *dev)
data->txskbs[tx] = skb;
 
if (i == 0) {
-   data->txring[tx].buf0 = dma_map_single(NULL, skb->data,
-   skb_headlen(skb), DMA_TO_DEVICE);
+   data->txring[tx].buf0 = dma_map_single(>pdev->dev,
+   skb->data, skb_headlen(skb),
+   DMA_TO_DEVICE);
data->txring[tx].len = skb_headlen(skb);
misc |= TSI108_TX_SOF;
} else {
const skb_frag_t *frag = _shinfo(skb)->frags[i - 1];
 
-   data->txring[tx].buf0 = skb_frag_dma_map(NULL, frag,
-0,
-
skb_frag_size(frag),
-DMA_TO_DEVICE);
+   data->txring[tx].buf0 =
+   skb_frag_dma_map(>pdev->dev, frag,
+   0, skb_frag_size(frag),
+   DMA_TO_DEVICE);
data->txring[tx].len = skb_frag_size(frag);
}
 
@@ -808,9 +811,9 @@ static int tsi108_refill_rx(struct net_device *dev, int 
budget)
if (!skb)
break;
 
-   data->rxring[rx].buf0 = dma_map_single(NULL, skb->data,
-   TSI108_RX_SKB_SIZE,
-   DMA_FROM_DEVICE);
+   data->rxring[rx].buf0 = dma_map_single(>pdev->dev,
+   skb->data, TSI108_RX_SKB_SIZE,
+   DMA_FROM_DEVICE);
 
/* Sometimes the hardware sets blen to zero after packet
 * reception, even though the manual says that it's only ever
@@ -1308,15 +1311,15 @@ static int tsi108_open(struct net_device *dev)
   data->id, dev->irq, dev->name);
}
 
-   data->rxring = dma_zalloc_coherent(NULL, rxring_size, >rxdma,
-  GFP_KERNEL);
+   data->rxring = dma_zalloc_coherent(>pdev->dev, rxring_size,
+   >rxdma, GFP_KERNEL);
if (!data->rxring)
return -ENOMEM;
 
-   data->txring = dma_zalloc_coherent(NULL, txring_size, >txdma,
-  GFP_KERNEL);
+   data->txring = dma_zalloc_coherent(>pdev->dev, txring_size,
+   >txdma, GFP_KERNEL);
if (!data->txring) {
-   pci_free_consistent(NULL, rxring_size, data->rxring,
+   dma_free_coherent(>pdev->dev, rxring_size, data->rxring,
data->rxdma);
return -ENOMEM;
}
@@ -1428,10 +1431,10 @@ static int tsi108_close(struct net_device *dev)
dev_kfree_skb(skb);
}
 
-   dma_free_coherent(0,
+   dma_free_coherent(>pdev->dev,
TSI108_RXRING_LEN * sizeof(rx_desc),
data->rxring, data->rxdma);
-   dma_free_coherent(0,
+   dma_free_coherent(>pdev->dev,
TSI108_TXRING_LEN * sizeof(tx_desc),
data->txring, data->txdma);
 
@@ -1576,6 +1579,7 @@ tsi108_init_one(struct platform_device *pdev)
printk("tsi108_eth%d: probe...\n", pdev->id);
data = netdev_priv(dev);
data->dev = dev;
+   data->pdev = pdev;
 
pr_debug("tsi108_eth%d:regs:phyresgs:phy:irq_num=0x%x:0x%x:0x%x:0x%x\n",
pdev->id, einfo->regs, einfo->phyregs,
-- 
2.14.2

[PATCH 4/4] PCI: Remove NULL device handling from PCI DMA API

[Christoph Hellwig]

Historically some ISA drivers used the old PCI DMA API with a NULL pdev
argument, but these days this isn't used and not too useful due to the
per-device DMA ops, so remove it.

Signed-off-by: Christoph Hellwig <h...@lst.de>
---
 include/linux/pci-dma-compat.h | 27 +--
 1 file changed, 13 insertions(+), 14 deletions(-)

diff --git a/include/linux/pci-dma-compat.h b/include/linux/pci-dma-compat.h
index d1f9fdade1e0..0dd1a3f7b309 100644
--- a/include/linux/pci-dma-compat.h
+++ b/include/linux/pci-dma-compat.h
@@ -17,91 +17,90 @@ static inline void *
 pci_alloc_consistent(struct pci_dev *hwdev, size_t size,
 dma_addr_t *dma_handle)
 {
-   return dma_alloc_coherent(hwdev == NULL ? NULL : >dev, size, 
dma_handle, GFP_ATOMIC);
+   return dma_alloc_coherent(>dev, size, dma_handle, GFP_ATOMIC);
 }
 
 static inline void *
 pci_zalloc_consistent(struct pci_dev *hwdev, size_t size,
  dma_addr_t *dma_handle)
 {
-   return dma_zalloc_coherent(hwdev == NULL ? NULL : >dev,
-  size, dma_handle, GFP_ATOMIC);
+   return dma_zalloc_coherent(>dev, size, dma_handle, GFP_ATOMIC);
 }
 
 static inline void
 pci_free_consistent(struct pci_dev *hwdev, size_t size,
void *vaddr, dma_addr_t dma_handle)
 {
-   dma_free_coherent(hwdev == NULL ? NULL : >dev, size, vaddr, 
dma_handle);
+   dma_free_coherent(>dev, size, vaddr, dma_handle);
 }
 
 static inline dma_addr_t
 pci_map_single(struct pci_dev *hwdev, void *ptr, size_t size, int direction)
 {
-   return dma_map_single(hwdev == NULL ? NULL : >dev, ptr, size, 
(enum dma_data_direction)direction);
+   return dma_map_single(>dev, ptr, size, (enum 
dma_data_direction)direction);
 }
 
 static inline void
 pci_unmap_single(struct pci_dev *hwdev, dma_addr_t dma_addr,
 size_t size, int direction)
 {
-   dma_unmap_single(hwdev == NULL ? NULL : >dev, dma_addr, size, 
(enum dma_data_direction)direction);
+   dma_unmap_single(>dev, dma_addr, size, (enum 
dma_data_direction)direction);
 }
 
 static inline dma_addr_t
 pci_map_page(struct pci_dev *hwdev, struct page *page,
 unsigned long offset, size_t size, int direction)
 {
-   return dma_map_page(hwdev == NULL ? NULL : >dev, page, offset, 
size, (enum dma_data_direction)direction);
+   return dma_map_page(>dev, page, offset, size, (enum 
dma_data_direction)direction);
 }
 
 static inline void
 pci_unmap_page(struct pci_dev *hwdev, dma_addr_t dma_address,
   size_t size, int direction)
 {
-   dma_unmap_page(hwdev == NULL ? NULL : >dev, dma_address, size, 
(enum dma_data_direction)direction);
+   dma_unmap_page(>dev, dma_address, size, (enum 
dma_data_direction)direction);
 }
 
 static inline int
 pci_map_sg(struct pci_dev *hwdev, struct scatterlist *sg,
   int nents, int direction)
 {
-   return dma_map_sg(hwdev == NULL ? NULL : >dev, sg, nents, (enum 
dma_data_direction)direction);
+   return dma_map_sg(>dev, sg, nents, (enum 
dma_data_direction)direction);
 }
 
 static inline void
 pci_unmap_sg(struct pci_dev *hwdev, struct scatterlist *sg,
 int nents, int direction)
 {
-   dma_unmap_sg(hwdev == NULL ? NULL : >dev, sg, nents, (enum 
dma_data_direction)direction);
+   dma_unmap_sg(>dev, sg, nents, (enum 
dma_data_direction)direction);
 }
 
 static inline void
 pci_dma_sync_single_for_cpu(struct pci_dev *hwdev, dma_addr_t dma_handle,
size_t size, int direction)
 {
-   dma_sync_single_for_cpu(hwdev == NULL ? NULL : >dev, dma_handle, 
size, (enum dma_data_direction)direction);
+   dma_sync_single_for_cpu(>dev, dma_handle, size, (enum 
dma_data_direction)direction);
 }
 
 static inline void
 pci_dma_sync_single_for_device(struct pci_dev *hwdev, dma_addr_t dma_handle,
size_t size, int direction)
 {
-   dma_sync_single_for_device(hwdev == NULL ? NULL : >dev, 
dma_handle, size, (enum dma_data_direction)direction);
+   dma_sync_single_for_device(>dev, dma_handle, size, (enum 
dma_data_direction)direction);
 }
 
 static inline void
 pci_dma_sync_sg_for_cpu(struct pci_dev *hwdev, struct scatterlist *sg,
int nelems, int direction)
 {
-   dma_sync_sg_for_cpu(hwdev == NULL ? NULL : >dev, sg, nelems, 
(enum dma_data_direction)direction);
+   dma_sync_sg_for_cpu(>dev, sg, nelems, (enum 
dma_data_direction)direction);
 }
 
 static inline void
 pci_dma_sync_sg_for_device(struct pci_dev *hwdev, struct scatterlist *sg,
int nelems, int direction)
 {
-   dma_sync_sg_for_device(hwdev == NULL ? NULL : >dev, sg, nelems, 
(enum dma_data_direction)direction);
+   dma_sync_sg_for_device(>dev, sg, nelems, (enum 
dma_data_direction)direction);
 }
 
 static inline int
-- 
2.14.2

Re: [PATCH 1/3] media/ttusb-budget: remove pci_zalloc_coherent abuse

[Christoph Hellwig]

On Tue, Jan 09, 2018 at 12:49:26PM -0800, Joe Perches wrote:
> This message doesn't make sense anymore and it might as well
> be deleted.
> 
> And it might be better to use kcalloc
> 
>   ttusb->iso_buffer = kcalloc(FRAMES_PER_ISO_BUF * ISO_BUF_COUNT,
>   ISO_FRAME_SIZE, GFP_KERNEL);

Sure.

Re: [PATCH 3/3] pci-dma-compat: remove handling of NULL pdev arguments

[Christoph Hellwig]

On Tue, Jan 09, 2018 at 06:25:44PM -0600, Bjorn Helgaas wrote:
> It looks like "pci_free_consistent(NULL" is still used in
> drivers/net/ethernet/tundra/tsi108_eth.c.

Yikes.  That one needs to pass the device is the platform dev to
the dma_map_* routines to start with, and mixing that with PCI
is pretty horrible.  I'll add a conversion of that driver to the
next resend.

> > Signed-off-by: Christoph Hellwig <h...@lst.de>
> 
> With Mauro's ack on the media/ttusb-dev patches, I could merge the
> whole series via the PCI tree?

Fine with me.

[PATCH 2/3] media/ttusb-dev: remove pci_zalloc_coherent abuse

[Christoph Hellwig]

Switch to a plain kzalloc instea of pci_zalloc_coherent to allocate
memory for the USB DMA.

Signed-off-by: Christoph Hellwig <h...@lst.de>
---
 drivers/media/usb/ttusb-dec/ttusb_dec.c | 13 +++--
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/drivers/media/usb/ttusb-dec/ttusb_dec.c 
b/drivers/media/usb/ttusb-dec/ttusb_dec.c
index cdefb5dfbbdc..794ea8a78181 100644
--- a/drivers/media/usb/ttusb-dec/ttusb_dec.c
+++ b/drivers/media/usb/ttusb-dec/ttusb_dec.c
@@ -127,7 +127,6 @@ struct ttusb_dec {
struct urb  *irq_urb;
dma_addr_t  irq_dma_handle;
void*iso_buffer;
-   dma_addr_t  iso_dma_handle;
struct urb  *iso_urb[ISO_BUF_COUNT];
int iso_stream_count;
struct mutexiso_mutex;
@@ -1185,11 +1184,7 @@ static void ttusb_dec_free_iso_urbs(struct ttusb_dec 
*dec)
 
for (i = 0; i < ISO_BUF_COUNT; i++)
usb_free_urb(dec->iso_urb[i]);
-
-   pci_free_consistent(NULL,
-   ISO_FRAME_SIZE * (FRAMES_PER_ISO_BUF *
- ISO_BUF_COUNT),
-   dec->iso_buffer, dec->iso_dma_handle);
+   kfree(dec->iso_buffer);
 }
 
 static int ttusb_dec_alloc_iso_urbs(struct ttusb_dec *dec)
@@ -1198,10 +1193,8 @@ static int ttusb_dec_alloc_iso_urbs(struct ttusb_dec 
*dec)
 
dprintk("%s\n", __func__);
 
-   dec->iso_buffer = pci_zalloc_consistent(NULL,
-   ISO_FRAME_SIZE * 
(FRAMES_PER_ISO_BUF * ISO_BUF_COUNT),
-   >iso_dma_handle);
-
+   dec->iso_buffer = kzalloc(ISO_FRAME_SIZE *
+   (FRAMES_PER_ISO_BUF * ISO_BUF_COUNT), GFP_KERNEL);
if (!dec->iso_buffer) {
dprintk("%s: pci_alloc_consistent - not enough memory\n",
__func__);
-- 
2.14.2

[PATCH 1/3] media/ttusb-budget: remove pci_zalloc_coherent abuse

[Christoph Hellwig]

Switch to a plain kzalloc instea of pci_zalloc_coherent to allocate
memory for the USB DMA.

Signed-off-by: Christoph Hellwig <h...@lst.de>
---
 drivers/media/usb/ttusb-budget/dvb-ttusb-budget.c | 13 +++--
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/drivers/media/usb/ttusb-budget/dvb-ttusb-budget.c 
b/drivers/media/usb/ttusb-budget/dvb-ttusb-budget.c
index a142b9dc0feb..b8619fb23351 100644
--- a/drivers/media/usb/ttusb-budget/dvb-ttusb-budget.c
+++ b/drivers/media/usb/ttusb-budget/dvb-ttusb-budget.c
@@ -102,7 +102,6 @@ struct ttusb {
unsigned int isoc_in_pipe;
 
void *iso_buffer;
-   dma_addr_t iso_dma_handle;
 
struct urb *iso_urb[ISO_BUF_COUNT];
 
@@ -792,21 +791,15 @@ static void ttusb_free_iso_urbs(struct ttusb *ttusb)
 
for (i = 0; i < ISO_BUF_COUNT; i++)
usb_free_urb(ttusb->iso_urb[i]);
-
-   pci_free_consistent(NULL,
-   ISO_FRAME_SIZE * FRAMES_PER_ISO_BUF *
-   ISO_BUF_COUNT, ttusb->iso_buffer,
-   ttusb->iso_dma_handle);
+   kfree(ttusb->iso_buffer);
 }
 
 static int ttusb_alloc_iso_urbs(struct ttusb *ttusb)
 {
int i;
 
-   ttusb->iso_buffer = pci_zalloc_consistent(NULL,
- ISO_FRAME_SIZE * 
FRAMES_PER_ISO_BUF * ISO_BUF_COUNT,
- >iso_dma_handle);
-
+   ttusb->iso_buffer = kzalloc(ISO_FRAME_SIZE * FRAMES_PER_ISO_BUF *
+   ISO_BUF_COUNT, GFP_KERNEL);
if (!ttusb->iso_buffer) {
dprintk("%s: pci_alloc_consistent - not enough memory\n",
__func__);
-- 
2.14.2

[PATCH 3/3] pci-dma-compat: remove handling of NULL pdev arguments

[Christoph Hellwig]

Historically some ISA drivers used the old pci DMA API with a NULL pdev
argument, but these days this isn't used and not too useful due to the
per-device DMA ops, so remove it.

Signed-off-by: Christoph Hellwig <h...@lst.de>
---
 include/linux/pci-dma-compat.h | 27 +--
 1 file changed, 13 insertions(+), 14 deletions(-)

diff --git a/include/linux/pci-dma-compat.h b/include/linux/pci-dma-compat.h
index d1f9fdade1e0..0dd1a3f7b309 100644
--- a/include/linux/pci-dma-compat.h
+++ b/include/linux/pci-dma-compat.h
@@ -17,91 +17,90 @@ static inline void *
 pci_alloc_consistent(struct pci_dev *hwdev, size_t size,
 dma_addr_t *dma_handle)
 {
-   return dma_alloc_coherent(hwdev == NULL ? NULL : >dev, size, 
dma_handle, GFP_ATOMIC);
+   return dma_alloc_coherent(>dev, size, dma_handle, GFP_ATOMIC);
 }
 
 static inline void *
 pci_zalloc_consistent(struct pci_dev *hwdev, size_t size,
  dma_addr_t *dma_handle)
 {
-   return dma_zalloc_coherent(hwdev == NULL ? NULL : >dev,
-  size, dma_handle, GFP_ATOMIC);
+   return dma_zalloc_coherent(>dev, size, dma_handle, GFP_ATOMIC);
 }
 
 static inline void
 pci_free_consistent(struct pci_dev *hwdev, size_t size,
void *vaddr, dma_addr_t dma_handle)
 {
-   dma_free_coherent(hwdev == NULL ? NULL : >dev, size, vaddr, 
dma_handle);
+   dma_free_coherent(>dev, size, vaddr, dma_handle);
 }
 
 static inline dma_addr_t
 pci_map_single(struct pci_dev *hwdev, void *ptr, size_t size, int direction)
 {
-   return dma_map_single(hwdev == NULL ? NULL : >dev, ptr, size, 
(enum dma_data_direction)direction);
+   return dma_map_single(>dev, ptr, size, (enum 
dma_data_direction)direction);
 }
 
 static inline void
 pci_unmap_single(struct pci_dev *hwdev, dma_addr_t dma_addr,
 size_t size, int direction)
 {
-   dma_unmap_single(hwdev == NULL ? NULL : >dev, dma_addr, size, 
(enum dma_data_direction)direction);
+   dma_unmap_single(>dev, dma_addr, size, (enum 
dma_data_direction)direction);
 }
 
 static inline dma_addr_t
 pci_map_page(struct pci_dev *hwdev, struct page *page,
 unsigned long offset, size_t size, int direction)
 {
-   return dma_map_page(hwdev == NULL ? NULL : >dev, page, offset, 
size, (enum dma_data_direction)direction);
+   return dma_map_page(>dev, page, offset, size, (enum 
dma_data_direction)direction);
 }
 
 static inline void
 pci_unmap_page(struct pci_dev *hwdev, dma_addr_t dma_address,
   size_t size, int direction)
 {
-   dma_unmap_page(hwdev == NULL ? NULL : >dev, dma_address, size, 
(enum dma_data_direction)direction);
+   dma_unmap_page(>dev, dma_address, size, (enum 
dma_data_direction)direction);
 }
 
 static inline int
 pci_map_sg(struct pci_dev *hwdev, struct scatterlist *sg,
   int nents, int direction)
 {
-   return dma_map_sg(hwdev == NULL ? NULL : >dev, sg, nents, (enum 
dma_data_direction)direction);
+   return dma_map_sg(>dev, sg, nents, (enum 
dma_data_direction)direction);
 }
 
 static inline void
 pci_unmap_sg(struct pci_dev *hwdev, struct scatterlist *sg,
 int nents, int direction)
 {
-   dma_unmap_sg(hwdev == NULL ? NULL : >dev, sg, nents, (enum 
dma_data_direction)direction);
+   dma_unmap_sg(>dev, sg, nents, (enum 
dma_data_direction)direction);
 }
 
 static inline void
 pci_dma_sync_single_for_cpu(struct pci_dev *hwdev, dma_addr_t dma_handle,
size_t size, int direction)
 {
-   dma_sync_single_for_cpu(hwdev == NULL ? NULL : >dev, dma_handle, 
size, (enum dma_data_direction)direction);
+   dma_sync_single_for_cpu(>dev, dma_handle, size, (enum 
dma_data_direction)direction);
 }
 
 static inline void
 pci_dma_sync_single_for_device(struct pci_dev *hwdev, dma_addr_t dma_handle,
size_t size, int direction)
 {
-   dma_sync_single_for_device(hwdev == NULL ? NULL : >dev, 
dma_handle, size, (enum dma_data_direction)direction);
+   dma_sync_single_for_device(>dev, dma_handle, size, (enum 
dma_data_direction)direction);
 }
 
 static inline void
 pci_dma_sync_sg_for_cpu(struct pci_dev *hwdev, struct scatterlist *sg,
int nelems, int direction)
 {
-   dma_sync_sg_for_cpu(hwdev == NULL ? NULL : >dev, sg, nelems, 
(enum dma_data_direction)direction);
+   dma_sync_sg_for_cpu(>dev, sg, nelems, (enum 
dma_data_direction)direction);
 }
 
 static inline void
 pci_dma_sync_sg_for_device(struct pci_dev *hwdev, struct scatterlist *sg,
int nelems, int direction)
 {
-   dma_sync_sg_for_device(hwdev == NULL ? NULL : >dev, sg, nelems, 
(enum dma_data_direction)direction);
+   dma_sync_sg_for_device(>dev, sg, nelems, (enum 
dma_data_direction)direction);
 }
 
 static inline int
-- 
2.14.2

remove pci_dma_* abuses and workarounds

[Christoph Hellwig]

Back before the dawn of time pci_dma_* with a NULL pci_dev argument
was used for all kinds of things, e.g. dma mapping for non-PCI
devices.  All this has been long removed, but it turns out we
still care for a NULL pci_dev in the wrappers, and we still have
two odd USB drivers that use pci_dma_alloc_consistent for allocating
memory while ignoring the dma_addr_t entirely.

This series switches the two drivers to use plain kzalloc and then
removes the handling of the NULL pci_dev in the pci_dma_* wrappers.

Re: [PATCH v4 00/12] Intel IPU3 ImgU patchset

[Christoph Hellwig]

Please keep everyone on CC for all the patches, othervise they are
complete unreviable and will be ignored.

Re: [PATCH v3 0/2] [media] videobuf2-dc: Add support for cacheable MMAP

[Christoph Hellwig]

On Thu, Jul 13, 2017 at 02:21:53PM +0100, Russell King - ARM Linux wrote:
> My conclusion of the dma_alloc_noncoherent() and dma_cache_sync() API
> when it was introduced is that it's basically a completely broken
> interface, and I've never seen any point to it.  Maybe some of that is
> because it's badly documented - which in turn makes it badly designed
> (because there's no specification detailing what it's supposed to be
> doing.)
> 
> I'd like to see that thing die...

It's not exactly the best interface ever, so any improvement is welcome.

I've posted a series to kill dma_alloc_noncoherent in favor of
dma_alloc_attrs a short while ago, and a big chunk of it should have
made it into 4.12.  I plan to kill it off entirely for 4.13.

That leaves dma_cache_sync() - it's used by 6 drivers:

drivers/net/ethernet/i825xx/lasi_82596.c
drivers/net/ethernet/seeq/sgiseeq.c
drivers/scsi/53c700.c
drivers/scsi/sgiwd93.c
drivers/sh/maple/maple.c
drivers/tty/serial/mpsc.c

Those are used on parisc, mips for a few old SGI systems, the SH
dreamcast and powerpc marvell mv64x60 devices.

So it shouldn't be too hard to figure out if they could be moved
to the normal dma_sync_* calls.

On parisc dma_cache_sync is equivalent to dma_sync_single_for_cpu,
so that should be fine.

On mips the implementation of dma_sync_single_for_cpu is a little
more complicated, but both dma_sync_single_for_cpu and dma_cache_sync
end up calling __dma_sync_virtual, so they look like the same in
the end as well.

On SH sync_single_for_device is implemented using dma_cache_sync,
and there is no dma_sync_single_for_cpu.

On powerpc both dma_sync_single_for_cpu and dma_sync_single_for_device
are implemented using the same primitive as dma_cache_sync.

In short: killing off dma_cache_sync and using the existing and
better defined syncing primitives looks entirely feasible.

I'll add it to my TODO list for 4.13.

Re: [PATCH v3 0/2] [media] videobuf2-dc: Add support for cacheable MMAP

[Christoph Hellwig]

On Mon, Jul 03, 2017 at 11:27:32AM +0200, Marek Szyprowski wrote:
> The main question here if we want to merge incomplete solution or not. As
> for now, there is no support in ARM/ARM64 for NON_CONSISTENT attribute.
> Also none of the v4l2 drivers use it. Sadly support for NON_CONSISTENT
> attribute is not fully implemented nor even defined in mainline.
>

DMA_ATTR_NON_CONSISTENT is the way to get the dma_alloc_noncoherent
semantics through the dma_alloc_attr API, and as such I think it is
pretty well defined, although the documentation in
Documentation/DMA-attributes.txt is really bad and we need to improve
it, by merging it with the dma_alloc_noncoherent description in
Documentation/DMA-API.txt. My series to remove dma_alloc_noncoherent
updates the latter to mention DMA_ATTR_NON_CONSISTENT, but
we should probably merge Documentation/DMA-API.txt,
Documentation/DMA-attributes.txt and Documentation/DMA-API-HOWTO.txt
into a single coherent document.


> I know that it works fine for some vendor kernel trees, but supporting it in
> mainline was a bit controversial. There is no proper way to sync cache for 
> such
> buffers. Calling dma_sync_sg worked so far, but it has to be first agreed as
> a proper DMA API.

As documented in Documentation/DMA-API.txt the proper way to sync
noncoherent/nonconsistent regions is to call dma_cache_sync.  It seems
like it generally is the same as dma_sync_range/sg so if we could
eventually merge these APIs that should reduce the confusion further.

Re: [PATCH v2 01/21] scatterlist: Introduce sg_map helper functions

[Christoph Hellwig]

On Wed, Apr 26, 2017 at 12:11:33PM -0600, Logan Gunthorpe wrote:
> Ok, well for starters I think you are mistaken about kmap being able to
> fail. I'm having a hard time finding many users of that function that
> bother to check for an error when calling it.

A quick audit of the arch code shows you're right - kmap can't fail
anywhere anymore.

> The main difficulty we
> have now is that neither of those functions are expected to fail and we
> need them to be able to in cases where the page doesn't map to system
> RAM. This patch series is trying to address it for users of scatterlist.
> I'm certainly open to other suggestions.

I think you'll need to follow the existing kmap semantics and never
fail the iomem version either.  Otherwise you'll have a special case
that's almost never used that has a different error path.

> There are a fair number of cases in the kernel that do something like:
> 
> if (something)
> x = kmap(page);
> else
> x = kmap_atomic(page);
> ...
> if (something)
> kunmap(page)
> else
> kunmap_atomic(x)
> 
> Which just seems cumbersome to me.

Passing a different flag based on something isn't really much better.

> In any case, if you can accept an sg_kmap and sg_kmap_atomic api just
> say so and I'll make the change. But I'll still need a flags variable
> for SG_MAP_MUST_NOT_FAIL to support legacy cases that have no fail path
> and both of those functions will need to be pretty nearly replicas of
> each other.

Again, wrong way.  Suddenly making things fail for your special case
that normally don't fail is a receipe for bugs.

Re: [PATCH v2 02/21] libiscsi: Add an internal error code

[Christoph Hellwig]

On Tue, Apr 25, 2017 at 12:20:49PM -0600, Logan Gunthorpe wrote:
> This is a prep patch to add a new error code to libiscsi. We want to
> rework some kmap calls to be able to fail. When we do, we'd like to
> use this error code.

The kmap case in iscsi_tcp_segment_map can already fail.  Please add
handling of that failure to this patch, and we should get it merged
ASAP.

Re: [PATCH v2 01/21] scatterlist: Introduce sg_map helper functions

[Christoph Hellwig]

On Tue, Apr 25, 2017 at 12:20:48PM -0600, Logan Gunthorpe wrote:
> This patch introduces functions which kmap the pages inside an sgl.
> These functions replace a common pattern of kmap(sg_page(sg)) that is
> used in more than 50 places within the kernel.
> 
> The motivation for this work is to eventually safely support sgls that
> contain io memory. In order for that to work, any access to the contents
> of an iomem SGL will need to be done with iomemcpy or hit some warning.
> (The exact details of how this will work have yet to be worked out.)

I think we'll at least need a draft of those to make sense of these
patches.  Otherwise they just look very clumsy.

> + *   Use this function to map a page in the scatterlist at the specified
> + *   offset. sg->offset is already added for you. Note: the semantics of
> + *   this function are that it may fail. Thus, its output should be checked
> + *   with IS_ERR and PTR_ERR. Otherwise, a pointer to the specified offset
> + *   in the mapped page is returned.
> + *
> + *   Flags can be any of:
> + *   * SG_KMAP   - Use kmap to create the mapping
> + *   * SG_KMAP_ATOMIC- Use kmap_atomic to map the page atommically.
> + * Thus, the rules of that function apply: the
> + * cpu may not sleep until it is unmaped.
> + *   * SG_MAP_MUST_NOT_FAIL  - Indicate that sg_map must not fail.
> + * If it does, it will issue a BUG_ON instead.
> + * This is intended for legacy code only, it
> + * is not to be used in new code.

I'm sorry but this API is just a trainwreck.  Right now we have the
nice little kmap_atomic API, which never fails and has a very nice
calling convention where we just pass back the return address, but does
not support sleeping inside the critical section.

And kmap, whіch may fail and requires the original page to be passed
back.  Anything that mixes these two concepts up is simply a non-starter.

Re: [PATCH 09/22] dm-crypt: Make use of the new sg_map helper in 4 call sites

[Christoph Hellwig]

On Thu, Apr 13, 2017 at 04:05:22PM -0600, Logan Gunthorpe wrote:
> Very straightforward conversion to the new function in all four spots.

I think the right fix here is to switch dm-crypt to the ahash API
that takes a scatterlist.

Re: [PATCH 03/22] libiscsi: Make use of new the sg_map helper function

[Christoph Hellwig]

On Thu, Apr 13, 2017 at 04:05:16PM -0600, Logan Gunthorpe wrote:
> Convert the kmap and kmap_atomic uses to the sg_map function. We now
> store the flags for the kmap instead of a boolean to indicate
> atomicitiy. We also propogate a possible kmap error down and create
> a new ISCSI_TCP_INTERNAL_ERR error type for this.

Can you split out the new error handling into a separate prep patch
which should go to the iscsi maintainers ASAP?

Re: [PATCH 01/22] scatterlist: Introduce sg_map helper functions

[Christoph Hellwig]

> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index 0007b79..b95934b 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -37,6 +37,9 @@
>  
>  #include 
>  
> +/* Prevent the highmem.h macro from aliasing ops->kunmap_atomic */
> +#undef kunmap_atomic
> +
>  static inline int is_dma_buf_file(struct file *);
>  
>  struct dma_buf_list {

I think the right fix here is to rename the operation to unmap_atomic
and send out a little patch for that ASAP.

> + *   Flags can be any of:
> + *   * SG_KMAP- Use kmap to create the mapping
> + *   * SG_KMAP_ATOMIC - Use kmap_atomic to map the page atommically.
> + *  Thus, the rules of that function apply: the cpu
> + *  may not sleep until it is unmaped.
> + *
> + *   Also, consider carefully whether this function is appropriate. It is
> + *   largely not recommended for new code and if the sgl came from another
> + *   subsystem and you don't know what kind of memory might be in the list
> + *   then you definitely should not call it. Non-mappable memory may be in
> + *   the sgl and thus this function may fail unexpectedly.
> + **/
> +static inline void *sg_map_offset(struct scatterlist *sg, size_t offset,
> +int flags)

I'd rather have separate functions for kmap vs kmap_atomic instead of
the flags parameter.  And while you're at it just always pass the 0
offset parameter instead of adding a wrapper..

Otherwise this looks good to me.

Re: [PATCH 02/22] nvmet: Make use of the new sg_map helper function

[Christoph Hellwig]

On Thu, Apr 13, 2017 at 11:06:16PM -0600, Logan Gunthorpe wrote:
> Or maybe I'll just send a patch for that
> separately seeing it doesn't depend on anything and is pretty simple. I
> can do that next week.

Yes, please just send that patch linux-nvme, we should be able to get
it into 4.12.

Re: [PATCH 02/22] nvmet: Make use of the new sg_map helper function

[Christoph Hellwig]

On Thu, Apr 13, 2017 at 04:05:15PM -0600, Logan Gunthorpe wrote:
> This is a straight forward conversion in two places. Should kmap fail,
> the code will return an INVALD_DATA error in the completion.

It really should be using nvmet_copy_from_sgl to make things safer,
as we don't want to rely on any particular SG list layout.  In fact
I'm pretty sure I did the conversion at some point, but it must never
have made it upstream.

Re: kill off pci_enable_msi_{exact,range}

[Christoph Hellwig]

On Fri, Jan 13, 2017 at 11:13:21AM -0600, Bjorn Helgaas wrote:
> I dropped the empty commit and replaced the xgbe patch with the one below.
> Can you take a look at [1] and make sure it's what you expected?

This looks great, thanks!
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: kill off pci_enable_msi_{exact,range}

[Christoph Hellwig]

On Fri, Jan 13, 2017 at 08:55:03AM +0100, Christoph Hellwig wrote:
> On Thu, Jan 12, 2017 at 03:29:00PM -0600, Bjorn Helgaas wrote:
> > Applied all three (with Tom's ack on the amd-xgbe patch) to pci/msi for
> > v4.11, thanks!
> 
> Tom had just send me an event better version of the xgbe patch.  Tom,
> maybe you can resend that relative to the PCI tree [1], so that we don't
> lose it for next merge window?

Actually - Bjorn, your msi branch contains an empty commit from this
thread:


https://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/commit/?h=pci/msi=7a8191de43faa9869b421a1b06075d8126ce7c0b

Maybe we should rebase it after all to avoid that?  In that case please
pick up the xgbe patch from Tom below:

---
From: Tom Lendacky <thomas.lenda...@amd.com>
Subject: [PATCH] amd-xgbe: Update PCI support to use new IRQ functions

Some of the PCI MSI/MSI-X functions have been deprecated and it is
recommended to use the new pci_alloc_irq_vectors() function. Convert
the code over to use the new function. Also, modify the way in which
the IRQs are requested - try for multiple MSI-X/MSI first, then a
single MSI/legacy interrupt.

Signed-off-by: Tom Lendacky <thomas.lenda...@amd.com>
Signed-off-by: Christoph Hellwig <h...@lst.de>
---
 drivers/net/ethernet/amd/xgbe/xgbe-pci.c |  128 +-
 drivers/net/ethernet/amd/xgbe/xgbe.h |8 +-
 2 files changed, 41 insertions(+), 95 deletions(-)

diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-pci.c 
b/drivers/net/ethernet/amd/xgbe/xgbe-pci.c
index e76b7f6..e436902 100644
--- a/drivers/net/ethernet/amd/xgbe/xgbe-pci.c
+++ b/drivers/net/ethernet/amd/xgbe/xgbe-pci.c
@@ -122,104 +122,40 @@
 #include "xgbe.h"
 #include "xgbe-common.h"
 
-static int xgbe_config_msi(struct xgbe_prv_data *pdata)
+static int xgbe_config_multi_msi(struct xgbe_prv_data *pdata)
 {
-   unsigned int msi_count;
+   unsigned int vector_count;
unsigned int i, j;
int ret;
 
-   msi_count = XGBE_MSIX_BASE_COUNT;
-   msi_count += max(pdata->rx_ring_count,
-pdata->tx_ring_count);
-   msi_count = roundup_pow_of_two(msi_count);
+   vector_count = XGBE_MSI_BASE_COUNT;
+   vector_count += max(pdata->rx_ring_count,
+   pdata->tx_ring_count);
 
-   ret = pci_enable_msi_exact(pdata->pcidev, msi_count);
+   ret = pci_alloc_irq_vectors(pdata->pcidev, XGBE_MSI_MIN_COUNT,
+   vector_count, PCI_IRQ_MSI | PCI_IRQ_MSIX);
if (ret < 0) {
-   dev_info(pdata->dev, "MSI request for %u interrupts failed\n",
-msi_count);
-
-   ret = pci_enable_msi(pdata->pcidev);
-   if (ret < 0) {
-   dev_info(pdata->dev, "MSI enablement failed\n");
-   return ret;
-   }
-
-   msi_count = 1;
-   }
-
-   pdata->irq_count = msi_count;
-
-   pdata->dev_irq = pdata->pcidev->irq;
-
-   if (msi_count > 1) {
-   pdata->ecc_irq = pdata->pcidev->irq + 1;
-   pdata->i2c_irq = pdata->pcidev->irq + 2;
-   pdata->an_irq = pdata->pcidev->irq + 3;
-
-   for (i = XGBE_MSIX_BASE_COUNT, j = 0;
-(i < msi_count) && (j < XGBE_MAX_DMA_CHANNELS);
-i++, j++)
-   pdata->channel_irq[j] = pdata->pcidev->irq + i;
-   pdata->channel_irq_count = j;
-
-   pdata->per_channel_irq = 1;
-   pdata->channel_irq_mode = XGBE_IRQ_MODE_LEVEL;
-   } else {
-   pdata->ecc_irq = pdata->pcidev->irq;
-   pdata->i2c_irq = pdata->pcidev->irq;
-   pdata->an_irq = pdata->pcidev->irq;
-   }
-
-   if (netif_msg_probe(pdata))
-   dev_dbg(pdata->dev, "MSI interrupts enabled\n");
-
-   return 0;
-}
-
-static int xgbe_config_msix(struct xgbe_prv_data *pdata)
-{
-   unsigned int msix_count;
-   unsigned int i, j;
-   int ret;
-
-   msix_count = XGBE_MSIX_BASE_COUNT;
-   msix_count += max(pdata->rx_ring_count,
- pdata->tx_ring_count);
-
-   pdata->msix_entries = devm_kcalloc(pdata->dev, msix_count,
-  sizeof(struct msix_entry),
-  GFP_KERNEL);
-   if (!pdata->msix_entries)
-   return -ENOMEM;
-
-   for (i = 0; i < msix_count; i++)
-   pdata->msix_entries[i].entry = i;
-
-   ret = pci_enable_msix_range(pdata->pcidev, pdata->msix_entries,
-   XGBE_MSIX_MIN_COUNT, msix_count);
-   if (ret < 0) {
-

Re: kill off pci_enable_msi_{exact,range}

[Christoph Hellwig]

On Thu, Jan 12, 2017 at 03:29:00PM -0600, Bjorn Helgaas wrote:
> Applied all three (with Tom's ack on the amd-xgbe patch) to pci/msi for
> v4.11, thanks!

Tom had just send me an event better version of the xgbe patch.  Tom,
maybe you can resend that relative to the PCI tree [1], so that we don't
lose it for next merge window?

[1] https://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git)
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] xgbe: switch to pci_irq_alloc_vectors

[Christoph Hellwig]

On Tue, Jan 10, 2017 at 12:40:10PM -0600, Tom Lendacky wrote:
> On 1/9/2017 2:37 PM, Christoph Hellwig wrote:
> > The newly added xgbe drivers uses the deprecated pci_enable_msi_exact
> > and pci_enable_msix_range interfaces.  Switch it to use
> > pci_irq_alloc_vectors instead.
> 
> I was just working on switching over to this API with some additional
> changes / simplification.  I'm ok with using this patch so that you get
> the API removal accomplished.  Going through the PCI tree just means
> it will probably be easier for me to hold off on the additional changes
> I wanted to make until later.

Hi Tom,

if you have a better patch I'd be more than happy to use that one instead,
this one was intended as a stupid search and replace.  The important
part for me is to get the two conversions and the interface removal
in together.

E.g. I've alreayd wondered why the driver requires the exact vector
number for MSI and a variable one for MSI-X, and there certainly is
all kinds of opportunity for cosmetic cleanup.
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/3] PCI/msi: remove pci_enable_msi_{exact,range}

[Christoph Hellwig]

All multi-MSI allocations are now done through pci_irq_alloc_vectors,
so remove the old interface.

Signed-off-by: Christoph Hellwig <h...@lst.de>
---
 Documentation/PCI/MSI-HOWTO.txt |  6 ++
 drivers/pci/msi.c   | 26 +-
 include/linux/pci.h | 16 ++--
 3 files changed, 13 insertions(+), 35 deletions(-)

diff --git a/Documentation/PCI/MSI-HOWTO.txt b/Documentation/PCI/MSI-HOWTO.txt
index cd9c9f6..b570a92 100644
--- a/Documentation/PCI/MSI-HOWTO.txt
+++ b/Documentation/PCI/MSI-HOWTO.txt
@@ -162,8 +162,6 @@ The following old APIs to enable and disable MSI or MSI-X 
interrupts should
 not be used in new code:
 
   pci_enable_msi() /* deprecated */
-  pci_enable_msi_range()   /* deprecated */
-  pci_enable_msi_exact()   /* deprecated */
   pci_disable_msi()/* deprecated */
   pci_enable_msix_range()  /* deprecated */
   pci_enable_msix_exact()  /* deprecated */
@@ -268,5 +266,5 @@ or disabled (0).  If 0 is found in any of the msi_bus files 
belonging
 to bridges between the PCI root and the device, MSIs are disabled.
 
 It is also worth checking the device driver to see whether it supports MSIs.
-For example, it may contain calls to pci_enable_msi_range() or
-pci_enable_msix_range().
+For example, it may contain calls to pci_irq_alloc_vectors with the
+PCI_IRQ_MSI or PCI_IRQ_MSIX flags.
diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index 50c5003..16dda43 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -1109,23 +1109,15 @@ static int __pci_enable_msi_range(struct pci_dev *dev, 
int minvec, int maxvec,
}
 }
 
-/**
- * pci_enable_msi_range - configure device's MSI capability structure
- * @dev: device to configure
- * @minvec: minimal number of interrupts to configure
- * @maxvec: maximum number of interrupts to configure
- *
- * This function tries to allocate a maximum possible number of interrupts in a
- * range between @minvec and @maxvec. It returns a negative errno if an error
- * occurs. If it succeeds, it returns the actual number of interrupts allocated
- * and updates the @dev's irq member to the lowest new interrupt number;
- * the other interrupt numbers allocated to this device are consecutive.
- **/
-int pci_enable_msi_range(struct pci_dev *dev, int minvec, int maxvec)
+/* deprecated, don't use */
+int pci_enable_msi(struct pci_dev *dev)
 {
-   return __pci_enable_msi_range(dev, minvec, maxvec, NULL);
+   int rc = __pci_enable_msi_range(dev, 1, 1, NULL);
+   if (rc < 0)
+   return rc;
+   return 0;
 }
-EXPORT_SYMBOL(pci_enable_msi_range);
+EXPORT_SYMBOL(pci_enable_msi);
 
 static int __pci_enable_msix_range(struct pci_dev *dev,
   struct msix_entry *entries, int minvec,
@@ -1381,7 +1373,7 @@ int pci_msi_domain_check_cap(struct irq_domain *domain,
 {
struct msi_desc *desc = first_pci_msi_entry(to_pci_dev(dev));
 
-   /* Special handling to support pci_enable_msi_range() */
+   /* Special handling to support __pci_enable_msi_range() */
if (pci_msi_desc_is_multi_msi(desc) &&
!(info->flags & MSI_FLAG_MULTI_PCI_MSI))
return 1;
@@ -1394,7 +1386,7 @@ int pci_msi_domain_check_cap(struct irq_domain *domain,
 static int pci_msi_domain_handle_error(struct irq_domain *domain,
   struct msi_desc *desc, int error)
 {
-   /* Special handling to support pci_enable_msi_range() */
+   /* Special handling to support __pci_enable_msi_range() */
if (pci_msi_desc_is_multi_msi(desc) && error == -ENOSPC)
return 1;
 
diff --git a/include/linux/pci.h b/include/linux/pci.h
index e2d1a12..2159376 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1309,14 +1309,7 @@ void pci_msix_shutdown(struct pci_dev *dev);
 void pci_disable_msix(struct pci_dev *dev);
 void pci_restore_msi_state(struct pci_dev *dev);
 int pci_msi_enabled(void);
-int pci_enable_msi_range(struct pci_dev *dev, int minvec, int maxvec);
-static inline int pci_enable_msi_exact(struct pci_dev *dev, int nvec)
-{
-   int rc = pci_enable_msi_range(dev, nvec, nvec);
-   if (rc < 0)
-   return rc;
-   return 0;
-}
+int pci_enable_msi(struct pci_dev *dev);
 int pci_enable_msix_range(struct pci_dev *dev, struct msix_entry *entries,
  int minvec, int maxvec);
 static inline int pci_enable_msix_exact(struct pci_dev *dev,
@@ -1347,10 +1340,7 @@ static inline void pci_msix_shutdown(struct pci_dev 
*dev) { }
 static inline void pci_disable_msix(struct pci_dev *dev) { }
 static inline void pci_restore_msi_state(struct pci_dev *dev) { }
 static inline int pci_msi_enabled(void) { return 0; }
-static inline int pci_enable_msi_range(struct pci_dev *dev, int minvec,
-  int maxvec)
-{ return -ENOSYS; }
-static inline int pci_enable_msi_exact(struct pci

[PATCH 2/3] xgbe: switch to pci_irq_alloc_vectors

[Christoph Hellwig]

The newly added xgbe drivers uses the deprecated pci_enable_msi_exact
and pci_enable_msix_range interfaces.  Switch it to use
pci_irq_alloc_vectors instead.

Signed-off-by: Christoph Hellwig <h...@lst.de>
---
 drivers/net/ethernet/amd/xgbe/xgbe-pci.c | 47 +---
 drivers/net/ethernet/amd/xgbe/xgbe.h |  1 -
 2 files changed, 19 insertions(+), 29 deletions(-)

diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-pci.c 
b/drivers/net/ethernet/amd/xgbe/xgbe-pci.c
index e76b7f6..be2690e 100644
--- a/drivers/net/ethernet/amd/xgbe/xgbe-pci.c
+++ b/drivers/net/ethernet/amd/xgbe/xgbe-pci.c
@@ -133,12 +133,13 @@ static int xgbe_config_msi(struct xgbe_prv_data *pdata)
 pdata->tx_ring_count);
msi_count = roundup_pow_of_two(msi_count);
 
-   ret = pci_enable_msi_exact(pdata->pcidev, msi_count);
+   ret = pci_alloc_irq_vectors(pdata->pcidev, msi_count, msi_count,
+   PCI_IRQ_MSI);
if (ret < 0) {
dev_info(pdata->dev, "MSI request for %u interrupts failed\n",
 msi_count);
 
-   ret = pci_enable_msi(pdata->pcidev);
+   ret = pci_alloc_irq_vectors(pdata->pcidev, 1, 1, PCI_IRQ_MSI);
if (ret < 0) {
dev_info(pdata->dev, "MSI enablement failed\n");
return ret;
@@ -149,25 +150,26 @@ static int xgbe_config_msi(struct xgbe_prv_data *pdata)
 
pdata->irq_count = msi_count;
 
-   pdata->dev_irq = pdata->pcidev->irq;
+   pdata->dev_irq = pci_irq_vector(pdata->pcidev, 0);
 
if (msi_count > 1) {
-   pdata->ecc_irq = pdata->pcidev->irq + 1;
-   pdata->i2c_irq = pdata->pcidev->irq + 2;
-   pdata->an_irq = pdata->pcidev->irq + 3;
+   pdata->ecc_irq = pci_irq_vector(pdata->pcidev, 1);
+   pdata->i2c_irq = pci_irq_vector(pdata->pcidev, 2);
+   pdata->an_irq = pci_irq_vector(pdata->pcidev, 3);
 
for (i = XGBE_MSIX_BASE_COUNT, j = 0;
 (i < msi_count) && (j < XGBE_MAX_DMA_CHANNELS);
 i++, j++)
-   pdata->channel_irq[j] = pdata->pcidev->irq + i;
+   pdata->channel_irq[j] =
+   pci_irq_vector(pdata->pcidev, i);
pdata->channel_irq_count = j;
 
pdata->per_channel_irq = 1;
pdata->channel_irq_mode = XGBE_IRQ_MODE_LEVEL;
} else {
-   pdata->ecc_irq = pdata->pcidev->irq;
-   pdata->i2c_irq = pdata->pcidev->irq;
-   pdata->an_irq = pdata->pcidev->irq;
+   pdata->ecc_irq = pci_irq_vector(pdata->pcidev, 0);
+   pdata->i2c_irq = pci_irq_vector(pdata->pcidev, 0);
+   pdata->an_irq = pci_irq_vector(pdata->pcidev, 0);
}
 
if (netif_msg_probe(pdata))
@@ -186,33 +188,22 @@ static int xgbe_config_msix(struct xgbe_prv_data *pdata)
msix_count += max(pdata->rx_ring_count,
  pdata->tx_ring_count);
 
-   pdata->msix_entries = devm_kcalloc(pdata->dev, msix_count,
-  sizeof(struct msix_entry),
-  GFP_KERNEL);
-   if (!pdata->msix_entries)
-   return -ENOMEM;
-
-   for (i = 0; i < msix_count; i++)
-   pdata->msix_entries[i].entry = i;
-
-   ret = pci_enable_msix_range(pdata->pcidev, pdata->msix_entries,
-   XGBE_MSIX_MIN_COUNT, msix_count);
+   ret = pci_alloc_irq_vectors(pdata->pcidev, XGBE_MSIX_MIN_COUNT,
+   msix_count, PCI_IRQ_MSIX);
if (ret < 0) {
dev_info(pdata->dev, "MSI-X enablement failed\n");
-   devm_kfree(pdata->dev, pdata->msix_entries);
-   pdata->msix_entries = NULL;
return ret;
}
 
pdata->irq_count = ret;
 
-   pdata->dev_irq = pdata->msix_entries[0].vector;
-   pdata->ecc_irq = pdata->msix_entries[1].vector;
-   pdata->i2c_irq = pdata->msix_entries[2].vector;
-   pdata->an_irq = pdata->msix_entries[3].vector;
+   pdata->dev_irq = pci_irq_vector(pdata->pcidev, 0);
+   pdata->ecc_irq = pci_irq_vector(pdata->pcidev, 1);
+   pdata->i2c_irq = pci_irq_vector(pdata->pcidev, 2);
+   pdata->an_irq = pci_irq_vector(pdata->pcidev, 3);
 
for (i = XGBE_MSIX_BASE_COUNT, j = 0; i < ret; i++, j++)
-   pdata->channel_irq[j] = pdata->msix_entries[i].vector;
+   pdata->channel_irq[j] = pci_irq_vector(pdata->pcidev, i);

[PATCH 1/3] media/cobalt: use pci_irq_allocate_vectors

[Christoph Hellwig]

Simply the interrupt setup by using the new PCI layer helpers.

Despite using pci_enable_msi_range, this driver was only requesting a
single MSI vector anyway.

Signed-off-by: Christoph Hellwig <h...@lst.de>
---
 drivers/media/pci/cobalt/cobalt-driver.c | 8 ++--
 drivers/media/pci/cobalt/cobalt-driver.h | 2 --
 2 files changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/media/pci/cobalt/cobalt-driver.c 
b/drivers/media/pci/cobalt/cobalt-driver.c
index 9796340..d5c911c 100644
--- a/drivers/media/pci/cobalt/cobalt-driver.c
+++ b/drivers/media/pci/cobalt/cobalt-driver.c
@@ -308,9 +308,7 @@ static void cobalt_pci_iounmap(struct cobalt *cobalt, 
struct pci_dev *pci_dev)
 static void cobalt_free_msi(struct cobalt *cobalt, struct pci_dev *pci_dev)
 {
free_irq(pci_dev->irq, (void *)cobalt);
-
-   if (cobalt->msi_enabled)
-   pci_disable_msi(pci_dev);
+   pci_free_irq_vectors(pci_dev);
 }
 
 static int cobalt_setup_pci(struct cobalt *cobalt, struct pci_dev *pci_dev,
@@ -387,14 +385,12 @@ static int cobalt_setup_pci(struct cobalt *cobalt, struct 
pci_dev *pci_dev,
   from being generated. */
cobalt_set_interrupt(cobalt, false);
 
-   if (pci_enable_msi_range(pci_dev, 1, 1) < 1) {
+   if (pci_alloc_irq_vectors(pci_dev, 1, 1, PCI_IRQ_MSI) < 1) {
cobalt_err("Could not enable MSI\n");
-   cobalt->msi_enabled = false;
ret = -EIO;
goto err_release;
}
msi_config_show(cobalt, pci_dev);
-   cobalt->msi_enabled = true;
 
/* Register IRQ */
if (request_irq(pci_dev->irq, cobalt_irq_handler, IRQF_SHARED,
diff --git a/drivers/media/pci/cobalt/cobalt-driver.h 
b/drivers/media/pci/cobalt/cobalt-driver.h
index ed00dc9..00f773e 100644
--- a/drivers/media/pci/cobalt/cobalt-driver.h
+++ b/drivers/media/pci/cobalt/cobalt-driver.h
@@ -287,8 +287,6 @@ struct cobalt {
u32 irq_none;
u32 irq_full_fifo;
 
-   bool msi_enabled;
-
/* omnitek dma */
int dma_channels;
int first_fifo_channel;
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

kill off pci_enable_msi_{exact,range}

[Christoph Hellwig]

I had hope that we could kill these old interfaces of for 4.10-rc,
but as of today Linus tree still has two users:

 (1) the cobalt media driver, for which I sent a patch long time ago,
 it got missed in the merge window.
 (2) the new xgbe driver was merged in 4.10-rc but used the old interfaces
 anyway

This series resend the patch for (1) and adds a new one for (2), as well
as having the final removal patch behind it.  Maybe we should just queue
up all three together in the PCI tree for 4.11?
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 7/7] uapi: export all headers under uapi directories

[Christoph Hellwig]

On Fri, Jan 06, 2017 at 10:43:59AM +0100, Nicolas Dichtel wrote:
> Regularly, when a new header is created in include/uapi/, the developer
> forgets to add it in the corresponding Kbuild file. This error is usually
> detected after the release is out.
> 
> In fact, all headers under uapi directories should be exported, thus it's
> useless to have an exhaustive list.
> 
> After this patch, the following files, which were not exported, are now
> exported (with make headers_install_all):

... snip ...

> linux/genwqe/.install
> linux/genwqe/..install.cmd
> linux/cifs/.install
> linux/cifs/..install.cmd

I'm pretty sure these should not be exported!
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 6/6] media/cobalt: use pci_irq_allocate_vectors

[Christoph Hellwig]

On Wed, Dec 14, 2016 at 11:37:17AM +0100, Hans Verkuil wrote:
> Completely forgot this. Is it OK to queue it for 4.11? Or is it blocking
> other follow-up work you want to do for 4.10?

My plan was to see if Bjorn would take the patch to do the trivial removal
of pci_enable_msix_exact and pci_enable_msix_range even as a late 4.10 patch
given it's so harmless, but either way there is follow work pending ASAP
so getting it in for 4.10 would be very helpful.
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 6/6] media/cobalt: use pci_irq_allocate_vectors

[Christoph Hellwig]

Hi Hans,

just checked the current Linux tree and cobalt still uses the old
pci_enable_msi_range call.  Did you queue this patch up for 4.10?
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Enabling peer to peer device transactions for PCIe devices

[Christoph Hellwig]

On Tue, Dec 06, 2016 at 09:38:50AM -0700, Jason Gunthorpe wrote:
> > > I'm not opposed to mapping /dev/nvmeX.  However, the lookup is trivial
> > > to accomplish in sysfs through /sys/dev/char to find the sysfs path of the
> > > device-dax instance under the nvme device, or if you already have the nvme
> > > sysfs path the dax instance(s) will appear under the "dax" sub-directory.
> > 
> > Personally I think mapping the dax resource in the sysfs tree is a nice
> > way to do this and a bit more intuitive than mapping a /dev/nvmeX.
> 
> It is still not at all clear to me what userpsace is supposed to do
> with this on nvme.. How is the CMB usable from userspace?

I don't think trying to expose it to userspace makes any sense.
Exposing it to in-kernel storage targets on the other hand makes a lot
of sense.
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Enabling peer to peer device transactions for PCIe devices

[Christoph Hellwig]

On Mon, Dec 05, 2016 at 12:46:14PM -0700, Jason Gunthorpe wrote:
> In any event the allocator still needs to track which regions are in
> use and be able to hook 'free' from userspace. That does suggest it
> should be integrated into the nvme driver and not a bolt on driver..

Two totally different use cases:

 - a card that exposes directly byte addressable storage as a PCI-e
   bar.  Thin of it as a nvdimm on a PCI-e card.  That's the iopmem
   case.
 - the NVMe CMB which exposes a byte addressable indirection buffer for
   I/O, but does not actually provide byte addressable persistent
   storage.  This is something that needs to be added to the NVMe driver
   (and the block layer for the abstraction probably).
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Enabling peer to peer device transactions for PCIe devices

[Christoph Hellwig]

On Thu, Nov 24, 2016 at 11:11:34AM -0700, Logan Gunthorpe wrote:
> * Regular DAX in the FS doesn't work at this time because the FS can
> move the file you think your transfer to out from under you. Though I
> understand there's been some work with XFS to solve that issue.

The file system will never move anything under locked down pages,
locking down pages is used exactly to protect against that.  So as long
as we page structures available RDMA to/from device memory _from kernel
space_ is trivial, although for file systems to work properly you
really want a notification to the consumer if the file systems wants
to remove the mapping.  We have implemented that using FL_LAYOUTS locks
for NFSD, but only XFS supports it so far.  Without that a long term
locked down region of memory (e.g. a kernel MR) would prevent various
file operations that would simply hang.
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 00/28] Reenable maybe-uninitialized warnings

[Christoph Hellwig]

On Tue, Oct 18, 2016 at 12:03:28AM +0200, Arnd Bergmann wrote:
> This is a set of patches that I hope to get into v4.9 in some form
> in order to turn on the -Wmaybe-uninitialized warnings again.

Hi Arnd,

I jsut complained to Geert that I was introducing way to many
bugs or pointless warnings for some compilers lately, but gcc didn't
warn me about them.  From a little research the lack of
-Wmaybe-uninitialized seems to be the reason for it, so I'm all
for re-enabling it.
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/6] vfio_pci: use pci_irq_allocate_vectors

[Christoph Hellwig]

On Thu, Sep 29, 2016 at 01:21:01PM -0600, Alex Williamson wrote:
> Sorry for the delay, slipped by me.  Overall a really nice cleanup.
> One tiny nit, the commit log mis-names the function as
> pci_irq_allocate_vectors instead of pci_alloc_irq_vectors.  With that,
> 
> Acked-by: Alex Williamson 
> 
> Let me know if you're wanting me to pull this through my tree, I'm
> assuming not.  Thanks,

Please pull in through your tree.  If you can also just fix up that
type that'd be even better.

Thanks a lot!
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/6] genwqe: use pci_irq_allocate_vectors

[Christoph Hellwig]

On Thu, Sep 29, 2016 at 03:45:29PM -0300, Gabriel Krisman Bertazi wrote:
> I'm stepping up to assist with the genwqe_card driver just now, since we
> (ibm) missed some of the last patches that went in.  I'll add myself to
> maintainers file.

Can your forward it to Greg together with whatever other changes are
pending for the driver?
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/6] genwqe: use pci_irq_allocate_vectors

[Christoph Hellwig]

On Thu, Sep 29, 2016 at 03:28:02PM -0300, Gabriel Krisman Bertazi wrote:
> Christoph Hellwig <h...@lst.de> writes:
> 
> > Simply the interrupt setup by using the new PCI layer helpers.
> 
> Good clean up.  Tested and:
> 
> Acked-by: Gabriel Krisman Bertazi <kris...@linux.vnet.ibm.com>

Which tree should this go in through?
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/6] ipr: use pci_irq_allocate_vectors

[Christoph Hellwig]

On Thu, Sep 29, 2016 at 09:01:44AM -0500, Brian King wrote:
> Thanks Christoph. Very nice. As I was reviewing the patch, I noticed
> the additional PCI_IRQ_AFFINITY flag, which is currently not being set
> in this patch. Is the intention to set that globally by default, or
> should I follow up with a one liner to add that to the ipr driver
> in the next patch set I send out?

Hi Brian,

PCI_IRQ_AFFINITY seems useful for ipr, especially if you also increase
the number of vectors above the current default 2.  And yes, please
make it a separate patch.
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 6/6] media/cobalt: use pci_irq_allocate_vectors

[Christoph Hellwig]

On Fri, Sep 16, 2016 at 10:01:42AM +0200, Hans Verkuil wrote:
> PCI_IRQ_MSI is unknown, I assume that this will appear in 4.9?

The flag is in 4.8-rc.
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/6] ipr: use pci_irq_allocate_vectors

[Christoph Hellwig]

Switch the ipr driver to use pci_alloc_irq_vectors.  We need to two calls to
pci_alloc_irq_vectors as ipr only supports multiple MSI-X vectors, but not
multiple MSI vectors.

Otherwise this cleans up a lot of cruft and allows to use a common
request_irq loop for irq types, which happens to only iterate over a
single line in the non MSI-X case.

Signed-off-by: Christoph Hellwig <h...@lst.de>
---
 drivers/scsi/ipr.c | 173 -
 drivers/scsi/ipr.h |   7 +--
 2 files changed, 52 insertions(+), 128 deletions(-)

diff --git a/drivers/scsi/ipr.c b/drivers/scsi/ipr.c
index 17d04c7..cadf56c 100644
--- a/drivers/scsi/ipr.c
+++ b/drivers/scsi/ipr.c
@@ -186,16 +186,16 @@ static const struct ipr_chip_cfg_t ipr_chip_cfg[] = {
 };
 
 static const struct ipr_chip_t ipr_chip[] = {
-   { PCI_VENDOR_ID_MYLEX, PCI_DEVICE_ID_IBM_GEMSTONE, IPR_USE_LSI, 
IPR_SIS32, IPR_PCI_CFG, _chip_cfg[0] },
-   { PCI_VENDOR_ID_IBM, PCI_DEVICE_ID_IBM_CITRINE, IPR_USE_LSI, IPR_SIS32, 
IPR_PCI_CFG, _chip_cfg[0] },
-   { PCI_VENDOR_ID_ADAPTEC2, PCI_DEVICE_ID_ADAPTEC2_OBSIDIAN, IPR_USE_LSI, 
IPR_SIS32, IPR_PCI_CFG, _chip_cfg[0] },
-   { PCI_VENDOR_ID_IBM, PCI_DEVICE_ID_IBM_OBSIDIAN, IPR_USE_LSI, 
IPR_SIS32, IPR_PCI_CFG, _chip_cfg[0] },
-   { PCI_VENDOR_ID_IBM, PCI_DEVICE_ID_IBM_OBSIDIAN_E, IPR_USE_MSI, 
IPR_SIS32, IPR_PCI_CFG, _chip_cfg[0] },
-   { PCI_VENDOR_ID_IBM, PCI_DEVICE_ID_IBM_SNIPE, IPR_USE_LSI, IPR_SIS32, 
IPR_PCI_CFG, _chip_cfg[1] },
-   { PCI_VENDOR_ID_ADAPTEC2, PCI_DEVICE_ID_ADAPTEC2_SCAMP, IPR_USE_LSI, 
IPR_SIS32, IPR_PCI_CFG, _chip_cfg[1] },
-   { PCI_VENDOR_ID_IBM, PCI_DEVICE_ID_IBM_CROC_FPGA_E2, IPR_USE_MSI, 
IPR_SIS64, IPR_MMIO, _chip_cfg[2] },
-   { PCI_VENDOR_ID_IBM, PCI_DEVICE_ID_IBM_CROCODILE, IPR_USE_MSI, 
IPR_SIS64, IPR_MMIO, _chip_cfg[2] },
-   { PCI_VENDOR_ID_IBM, PCI_DEVICE_ID_IBM_RATTLESNAKE, IPR_USE_MSI, 
IPR_SIS64, IPR_MMIO, _chip_cfg[2] }
+   { PCI_VENDOR_ID_MYLEX, PCI_DEVICE_ID_IBM_GEMSTONE, false, IPR_SIS32, 
IPR_PCI_CFG, _chip_cfg[0] },
+   { PCI_VENDOR_ID_IBM, PCI_DEVICE_ID_IBM_CITRINE, false, IPR_SIS32, 
IPR_PCI_CFG, _chip_cfg[0] },
+   { PCI_VENDOR_ID_ADAPTEC2, PCI_DEVICE_ID_ADAPTEC2_OBSIDIAN, false, 
IPR_SIS32, IPR_PCI_CFG, _chip_cfg[0] },
+   { PCI_VENDOR_ID_IBM, PCI_DEVICE_ID_IBM_OBSIDIAN, false, IPR_SIS32, 
IPR_PCI_CFG, _chip_cfg[0] },
+   { PCI_VENDOR_ID_IBM, PCI_DEVICE_ID_IBM_OBSIDIAN_E, true, IPR_SIS32, 
IPR_PCI_CFG, _chip_cfg[0] },
+   { PCI_VENDOR_ID_IBM, PCI_DEVICE_ID_IBM_SNIPE, false, IPR_SIS32, 
IPR_PCI_CFG, _chip_cfg[1] },
+   { PCI_VENDOR_ID_ADAPTEC2, PCI_DEVICE_ID_ADAPTEC2_SCAMP, false, 
IPR_SIS32, IPR_PCI_CFG, _chip_cfg[1] },
+   { PCI_VENDOR_ID_IBM, PCI_DEVICE_ID_IBM_CROC_FPGA_E2, true, IPR_SIS64, 
IPR_MMIO, _chip_cfg[2] },
+   { PCI_VENDOR_ID_IBM, PCI_DEVICE_ID_IBM_CROCODILE, true, IPR_SIS64, 
IPR_MMIO, _chip_cfg[2] },
+   { PCI_VENDOR_ID_IBM, PCI_DEVICE_ID_IBM_RATTLESNAKE, true, IPR_SIS64, 
IPR_MMIO, _chip_cfg[2] }
 };
 
 static int ipr_max_bus_speeds[] = {
@@ -9356,23 +9356,11 @@ static void ipr_free_mem(struct ipr_ioa_cfg *ioa_cfg)
 static void ipr_free_irqs(struct ipr_ioa_cfg *ioa_cfg)
 {
struct pci_dev *pdev = ioa_cfg->pdev;
+   int i;
 
-   if (ioa_cfg->intr_flag == IPR_USE_MSI ||
-   ioa_cfg->intr_flag == IPR_USE_MSIX) {
-   int i;
-   for (i = 0; i < ioa_cfg->nvectors; i++)
-   free_irq(ioa_cfg->vectors_info[i].vec,
-_cfg->hrrq[i]);
-   } else
-   free_irq(pdev->irq, _cfg->hrrq[0]);
-
-   if (ioa_cfg->intr_flag == IPR_USE_MSI) {
-   pci_disable_msi(pdev);
-   ioa_cfg->intr_flag &= ~IPR_USE_MSI;
-   } else if (ioa_cfg->intr_flag == IPR_USE_MSIX) {
-   pci_disable_msix(pdev);
-   ioa_cfg->intr_flag &= ~IPR_USE_MSIX;
-   }
+   for (i = 0; i < ioa_cfg->nvectors; i++)
+   free_irq(pci_irq_vector(pdev, i), _cfg->hrrq[i]);
+   pci_free_irq_vectors(pdev);
 }
 
 /**
@@ -9799,45 +9787,6 @@ static void ipr_wait_for_pci_err_recovery(struct 
ipr_ioa_cfg *ioa_cfg)
}
 }
 
-static int ipr_enable_msix(struct ipr_ioa_cfg *ioa_cfg)
-{
-   struct msix_entry entries[IPR_MAX_MSIX_VECTORS];
-   int i, vectors;
-
-   for (i = 0; i < ARRAY_SIZE(entries); ++i)
-   entries[i].entry = i;
-
-   vectors = pci_enable_msix_range(ioa_cfg->pdev,
-   entries, 1, ipr_number_of_msix);
-   if (vectors < 0) {
-   ipr_wait_for_pci_err_recovery(ioa_cfg);
-   return vectors;
-   }
-
-   for (i = 0; i < vectors; i++)
-   ioa_cfg->vectors_info[i].vec = entries[i].vector;
-   ioa_cfg->nvectors = vectors;
-
-   return 0;
-}
-
-static int ipr_enable_msi(struct ipr_ioa_cfg *ioa_cfg)
-{
-   int i, vector

[PATCH 5/6] genwqe: use pci_irq_allocate_vectors

[Christoph Hellwig]

Simply the interrupt setup by using the new PCI layer helpers.

One odd thing about this driver is that it looks like it could request
multiple MSI vectors, but it will then only ever use a single one.

Signed-off-by: Christoph Hellwig <h...@lst.de>
---
 drivers/misc/genwqe/card_base.h  |  1 -
 drivers/misc/genwqe/card_utils.c | 12 ++--
 2 files changed, 2 insertions(+), 11 deletions(-)

diff --git a/drivers/misc/genwqe/card_base.h b/drivers/misc/genwqe/card_base.h
index cb851c1..5813b5f 100644
--- a/drivers/misc/genwqe/card_base.h
+++ b/drivers/misc/genwqe/card_base.h
@@ -41,7 +41,6 @@
 #include "genwqe_driver.h"
 
 #define GENWQE_MSI_IRQS4  /* Just one supported, no 
MSIx */
-#define GENWQE_FLAG_MSI_ENABLED(1 << 0)
 
 #define GENWQE_MAX_VFS 15 /* maximum 15 VFs are possible */
 #define GENWQE_MAX_FUNCS   16 /* 1 PF and 15 VFs */
diff --git a/drivers/misc/genwqe/card_utils.c b/drivers/misc/genwqe/card_utils.c
index 222367c..da424c2 100644
--- a/drivers/misc/genwqe/card_utils.c
+++ b/drivers/misc/genwqe/card_utils.c
@@ -730,13 +730,10 @@ int genwqe_read_softreset(struct genwqe_dev *cd)
 int genwqe_set_interrupt_capability(struct genwqe_dev *cd, int count)
 {
int rc;
-   struct pci_dev *pci_dev = cd->pci_dev;
 
-   rc = pci_enable_msi_range(pci_dev, 1, count);
+   rc = pci_alloc_irq_vectors(cd->pci_dev, 1, count, PCI_IRQ_MSI);
if (rc < 0)
return rc;
-
-   cd->flags |= GENWQE_FLAG_MSI_ENABLED;
return 0;
 }
 
@@ -746,12 +743,7 @@ int genwqe_set_interrupt_capability(struct genwqe_dev *cd, 
int count)
  */
 void genwqe_reset_interrupt_capability(struct genwqe_dev *cd)
 {
-   struct pci_dev *pci_dev = cd->pci_dev;
-
-   if (cd->flags & GENWQE_FLAG_MSI_ENABLED) {
-   pci_disable_msi(pci_dev);
-   cd->flags &= ~GENWQE_FLAG_MSI_ENABLED;
-   }
+   pci_free_irq_vectors(cd->pci_dev);
 }
 
 /**
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 6/6] media/cobalt: use pci_irq_allocate_vectors

[Christoph Hellwig]

Simply the interrupt setup by using the new PCI layer helpers.

Despite using pci_enable_msi_range, this driver was only requesting a
single MSI vector anyway.

Signed-off-by: Christoph Hellwig <h...@lst.de>
---
 drivers/media/pci/cobalt/cobalt-driver.c | 8 ++--
 drivers/media/pci/cobalt/cobalt-driver.h | 2 --
 2 files changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/media/pci/cobalt/cobalt-driver.c 
b/drivers/media/pci/cobalt/cobalt-driver.c
index 476f7f0..5a0a9e4 100644
--- a/drivers/media/pci/cobalt/cobalt-driver.c
+++ b/drivers/media/pci/cobalt/cobalt-driver.c
@@ -307,9 +307,7 @@ static void cobalt_pci_iounmap(struct cobalt *cobalt, 
struct pci_dev *pci_dev)
 static void cobalt_free_msi(struct cobalt *cobalt, struct pci_dev *pci_dev)
 {
free_irq(pci_dev->irq, (void *)cobalt);
-
-   if (cobalt->msi_enabled)
-   pci_disable_msi(pci_dev);
+   pci_free_irq_vectors(pci_dev);
 }
 
 static int cobalt_setup_pci(struct cobalt *cobalt, struct pci_dev *pci_dev,
@@ -386,14 +384,12 @@ static int cobalt_setup_pci(struct cobalt *cobalt, struct 
pci_dev *pci_dev,
   from being generated. */
cobalt_set_interrupt(cobalt, false);
 
-   if (pci_enable_msi_range(pci_dev, 1, 1) < 1) {
+   if (pci_alloc_irq_vectors(pci_dev, 1, 1, PCI_IRQ_MSI) < 1) {
cobalt_err("Could not enable MSI\n");
-   cobalt->msi_enabled = false;
ret = -EIO;
goto err_release;
}
msi_config_show(cobalt, pci_dev);
-   cobalt->msi_enabled = true;
 
/* Register IRQ */
if (request_irq(pci_dev->irq, cobalt_irq_handler, IRQF_SHARED,
diff --git a/drivers/media/pci/cobalt/cobalt-driver.h 
b/drivers/media/pci/cobalt/cobalt-driver.h
index ed00dc9..00f773e 100644
--- a/drivers/media/pci/cobalt/cobalt-driver.h
+++ b/drivers/media/pci/cobalt/cobalt-driver.h
@@ -287,8 +287,6 @@ struct cobalt {
u32 irq_none;
u32 irq_full_fifo;
 
-   bool msi_enabled;
-
/* omnitek dma */
int dma_channels;
int first_fifo_channel;
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/6] arcmsr: use pci_alloc_irq_vectors

[Christoph Hellwig]

Switch the arcmsr driver to use pci_alloc_irq_vectors.  We need to two
calls to pci_alloc_irq_vectors as arcmsr only supports multiple MSI-X
vectors, but not multiple MSI vectors.

Otherwise this cleans up a lot of cruft and allows to use a common
request_irq loop for irq types, which happens to only iterate over a
single line in the non MSI-X case.

Signed-off-by: Christoph Hellwig <h...@lst.de>
---
 drivers/scsi/arcmsr/arcmsr.h |  5 +--
 drivers/scsi/arcmsr/arcmsr_hba.c | 82 
 2 files changed, 33 insertions(+), 54 deletions(-)

diff --git a/drivers/scsi/arcmsr/arcmsr.h b/drivers/scsi/arcmsr/arcmsr.h
index cf99f8c..a254b32 100644
--- a/drivers/scsi/arcmsr/arcmsr.h
+++ b/drivers/scsi/arcmsr/arcmsr.h
@@ -629,7 +629,6 @@ struct AdapterControlBlock
struct pci_dev *pdev;
struct Scsi_Host *  host;
unsigned long   vir2phy_offset;
-   struct msix_entry   entries[ARCMST_NUM_MSIX_VECTORS];
/* Offset is used in making arc cdb physical to virtual calculations */
uint32_toutbound_int_enable;
uint32_tcdb_phyaddr_hi32;
@@ -671,8 +670,6 @@ struct AdapterControlBlock
/* iop init */
#define ACB_F_ABORT 0x0200
#define ACB_F_FIRMWARE_TRAP 0x0400
-   #define ACB_F_MSI_ENABLED   0x1000
-   #define ACB_F_MSIX_ENABLED  0x2000
struct CommandControlBlock *
pccb_pool[ARCMSR_MAX_FREECCB_NUM];
/* used for memory free */
struct list_headccb_free_list;
@@ -725,7 +722,7 @@ struct AdapterControlBlock
atomic_trq_map_token;
atomic_tante_token_value;
uint32_tmaxOutstanding;
-   int msix_vector_count;
+   int vector_count;
 };/* HW_DEVICE_EXTENSION */
 /*
 ***
diff --git a/drivers/scsi/arcmsr/arcmsr_hba.c b/drivers/scsi/arcmsr/arcmsr_hba.c
index 7640498..a267327 100644
--- a/drivers/scsi/arcmsr/arcmsr_hba.c
+++ b/drivers/scsi/arcmsr/arcmsr_hba.c
@@ -720,51 +720,39 @@ static void arcmsr_message_isr_bh_fn(struct work_struct 
*work)
 static int
 arcmsr_request_irq(struct pci_dev *pdev, struct AdapterControlBlock *acb)
 {
-   int i, j, r;
-   struct msix_entry entries[ARCMST_NUM_MSIX_VECTORS];
-
-   for (i = 0; i < ARCMST_NUM_MSIX_VECTORS; i++)
-   entries[i].entry = i;
-   r = pci_enable_msix_range(pdev, entries, 1, ARCMST_NUM_MSIX_VECTORS);
-   if (r < 0)
-   goto msi_int;
-   acb->msix_vector_count = r;
-   for (i = 0; i < r; i++) {
-   if (request_irq(entries[i].vector,
-   arcmsr_do_interrupt, 0, "arcmsr", acb)) {
+   unsigned long flags;
+   int nvec, i;
+
+   nvec = pci_alloc_irq_vectors(pdev, 1, ARCMST_NUM_MSIX_VECTORS,
+   PCI_IRQ_MSIX);
+   if (nvec > 0) {
+   pr_info("arcmsr%d: msi-x enabled\n", acb->host->host_no);
+   flags = 0;
+   } else {
+   nvec = pci_alloc_irq_vectors(pdev, 1, 1,
+   PCI_IRQ_MSI | PCI_IRQ_LEGACY);
+   if (nvec < 1)
+   return FAILED;
+
+   flags = IRQF_SHARED;
+   }
+
+   acb->vector_count = nvec;
+   for (i = 0; i < nvec; i++) {
+   if (request_irq(pci_irq_vector(pdev, i), arcmsr_do_interrupt,
+   flags, "arcmsr", acb)) {
pr_warn("arcmsr%d: request_irq =%d failed!\n",
-   acb->host->host_no, entries[i].vector);
-   for (j = 0 ; j < i ; j++)
-   free_irq(entries[j].vector, acb);
-   pci_disable_msix(pdev);
-   goto msi_int;
+   acb->host->host_no, pci_irq_vector(pdev, i));
+   goto out_free_irq;
}
-   acb->entries[i] = entries[i];
-   }
-   acb->acb_flags |= ACB_F_MSIX_ENABLED;
-   pr_info("arcmsr%d: msi-x enabled\n", acb->host->host_no);
-   return SUCCESS;
-msi_int:
-   if (pci_enable_msi_exact(pdev, 1) < 0)
-   goto legacy_int;
-   if (request_irq(pdev->irq, arcmsr_do_interrupt,
-   IRQF_SHARED, "arcmsr", acb)) {
-   pr_warn("arcmsr%d: request_irq =%d failed!\n",
-   acb->host->host_no, pdev->irq);
-   pci_disable_msi(pdev);
-   goto legacy_int;
-   }
-   acb->acb_flags |= ACB_F_MSI_ENABLED;
-   

[PATCH 4/6] vfio_pci: use pci_irq_allocate_vectors

[Christoph Hellwig]

Simply the interrupt setup by using the new PCI layer helpers.

Signed-off-by: Christoph Hellwig <h...@lst.de>
---
 drivers/vfio/pci/vfio_pci_intrs.c   | 45 +
 drivers/vfio/pci/vfio_pci_private.h |  1 -
 2 files changed, 10 insertions(+), 36 deletions(-)

diff --git a/drivers/vfio/pci/vfio_pci_intrs.c 
b/drivers/vfio/pci/vfio_pci_intrs.c
index 152b438..a1d283e 100644
--- a/drivers/vfio/pci/vfio_pci_intrs.c
+++ b/drivers/vfio/pci/vfio_pci_intrs.c
@@ -250,6 +250,7 @@ static irqreturn_t vfio_msihandler(int irq, void *arg)
 static int vfio_msi_enable(struct vfio_pci_device *vdev, int nvec, bool msix)
 {
struct pci_dev *pdev = vdev->pdev;
+   unsigned int flag = msix ? PCI_IRQ_MSIX : PCI_IRQ_MSI;
int ret;
 
if (!is_irq_none(vdev))
@@ -259,35 +260,13 @@ static int vfio_msi_enable(struct vfio_pci_device *vdev, 
int nvec, bool msix)
if (!vdev->ctx)
return -ENOMEM;
 
-   if (msix) {
-   int i;
-
-   vdev->msix = kzalloc(nvec * sizeof(struct msix_entry),
-GFP_KERNEL);
-   if (!vdev->msix) {
-   kfree(vdev->ctx);
-   return -ENOMEM;
-   }
-
-   for (i = 0; i < nvec; i++)
-   vdev->msix[i].entry = i;
-
-   ret = pci_enable_msix_range(pdev, vdev->msix, 1, nvec);
-   if (ret < nvec) {
-   if (ret > 0)
-   pci_disable_msix(pdev);
-   kfree(vdev->msix);
-   kfree(vdev->ctx);
-   return ret;
-   }
-   } else {
-   ret = pci_enable_msi_range(pdev, 1, nvec);
-   if (ret < nvec) {
-   if (ret > 0)
-   pci_disable_msi(pdev);
-   kfree(vdev->ctx);
-   return ret;
-   }
+   /* return the number of supported vectors if we can't get all: */
+   ret = pci_alloc_irq_vectors(pdev, 1, nvec, flag);
+   if (ret < nvec) {
+   if (ret > 0)
+   pci_free_irq_vectors(pdev);
+   kfree(vdev->ctx);
+   return ret;
}
 
vdev->num_ctx = nvec;
@@ -315,7 +294,7 @@ static int vfio_msi_set_vector_signal(struct 
vfio_pci_device *vdev,
if (vector < 0 || vector >= vdev->num_ctx)
return -EINVAL;
 
-   irq = msix ? vdev->msix[vector].vector : pdev->irq + vector;
+   irq = pci_irq_vector(pdev, vector);
 
if (vdev->ctx[vector].trigger) {
free_irq(irq, vdev->ctx[vector].trigger);
@@ -408,11 +387,7 @@ static void vfio_msi_disable(struct vfio_pci_device *vdev, 
bool msix)
 
vfio_msi_set_block(vdev, 0, vdev->num_ctx, NULL, msix);
 
-   if (msix) {
-   pci_disable_msix(vdev->pdev);
-   kfree(vdev->msix);
-   } else
-   pci_disable_msi(pdev);
+   pci_free_irq_vectors(pdev);
 
vdev->irq_type = VFIO_PCI_NUM_IRQS;
vdev->num_ctx = 0;
diff --git a/drivers/vfio/pci/vfio_pci_private.h 
b/drivers/vfio/pci/vfio_pci_private.h
index 2128de8..f561ac1 100644
--- a/drivers/vfio/pci/vfio_pci_private.h
+++ b/drivers/vfio/pci/vfio_pci_private.h
@@ -72,7 +72,6 @@ struct vfio_pci_device {
struct perm_bits*msi_perm;
spinlock_t  irqlock;
struct mutexigate;
-   struct msix_entry   *msix;
struct vfio_pci_irq_ctx *ctx;
int num_ctx;
int irq_type;
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

replace pci_enable_msi_{exact_range} with pci_alloc_irq_vectors

[Christoph Hellwig]

Hi all,

this series switch the remaining users of pci_enable_msi_{exact_range}
(accounting for ahci and nvme being done through other channels) to
use the pci_alloc_irq_vectors helper instead and thus simplify the
interrupt code in those drivers a lot.  I decided to post it as a
series to everyone involved and linux-pci so that we can get a bit of
cross-review given that I have none of the involved hardware myself.

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/6] skd: use pci_alloc_irq_vectors

[Christoph Hellwig]

Switch the skd driver to use pci_alloc_irq_vectors.  We need to two calls to
pci_alloc_irq_vectors as skd only supports multiple MSI-X vectors, but not
multiple MSI vectors.

Otherwise this cleans up a lot of cruft and allows to a lot more common code.

Signed-off-by: Christoph Hellwig <h...@lst.de>
---
 drivers/block/skd_main.c | 212 +++
 1 file changed, 66 insertions(+), 146 deletions(-)

diff --git a/drivers/block/skd_main.c b/drivers/block/skd_main.c
index 3822eae..702f543 100644
--- a/drivers/block/skd_main.c
+++ b/drivers/block/skd_main.c
@@ -270,8 +270,6 @@ struct skd_device {
resource_size_t mem_phys[SKD_MAX_BARS];
u32 mem_size[SKD_MAX_BARS];
 
-   skd_irq_type_t irq_type;
-   u32 msix_count;
struct skd_msix_entry *msix_entries;
 
struct pci_dev *pdev;
@@ -3821,10 +3819,6 @@ static irqreturn_t skd_qfull_isr(int irq, void 
*skd_host_data)
  */
 
 struct skd_msix_entry {
-   int have_irq;
-   u32 vector;
-   u32 entry;
-   struct skd_device *rsp;
char isr_name[30];
 };
 
@@ -3853,56 +3847,21 @@ static struct skd_init_msix_entry 
msix_entries[SKD_MAX_MSIX_COUNT] = {
{ "(Queue Full 3)", skd_qfull_isr},
 };
 
-static void skd_release_msix(struct skd_device *skdev)
-{
-   struct skd_msix_entry *qentry;
-   int i;
-
-   if (skdev->msix_entries) {
-   for (i = 0; i < skdev->msix_count; i++) {
-   qentry = >msix_entries[i];
-   skdev = qentry->rsp;
-
-   if (qentry->have_irq)
-   devm_free_irq(>pdev->dev,
- qentry->vector, qentry->rsp);
-   }
-
-   kfree(skdev->msix_entries);
-   }
-
-   if (skdev->msix_count)
-   pci_disable_msix(skdev->pdev);
-
-   skdev->msix_count = 0;
-   skdev->msix_entries = NULL;
-}
-
 static int skd_acquire_msix(struct skd_device *skdev)
 {
int i, rc;
struct pci_dev *pdev = skdev->pdev;
-   struct msix_entry *entries;
-   struct skd_msix_entry *qentry;
-
-   entries = kzalloc(sizeof(struct msix_entry) * SKD_MAX_MSIX_COUNT,
- GFP_KERNEL);
-   if (!entries)
-   return -ENOMEM;
 
-   for (i = 0; i < SKD_MAX_MSIX_COUNT; i++)
-   entries[i].entry = i;
-
-   rc = pci_enable_msix_exact(pdev, entries, SKD_MAX_MSIX_COUNT);
-   if (rc) {
+   rc = pci_alloc_irq_vectors(pdev, SKD_MAX_MSIX_COUNT, SKD_MAX_MSIX_COUNT,
+   PCI_IRQ_MSIX);
+   if (rc < 0) {
pr_err("(%s): failed to enable MSI-X %d\n",
   skd_name(skdev), rc);
goto msix_out;
}
 
-   skdev->msix_count = SKD_MAX_MSIX_COUNT;
-   skdev->msix_entries = kzalloc(sizeof(struct skd_msix_entry) *
- skdev->msix_count, GFP_KERNEL);
+   skdev->msix_entries = kcalloc(SKD_MAX_MSIX_COUNT,
+   sizeof(struct skd_msix_entry), GFP_KERNEL);
if (!skdev->msix_entries) {
rc = -ENOMEM;
pr_err("(%s): msix table allocation error\n",
@@ -3910,136 +3869,98 @@ static int skd_acquire_msix(struct skd_device *skdev)
goto msix_out;
}
 
-   for (i = 0; i < skdev->msix_count; i++) {
-   qentry = >msix_entries[i];
-   qentry->vector = entries[i].vector;
-   qentry->entry = entries[i].entry;
-   qentry->rsp = NULL;
-   qentry->have_irq = 0;
-   pr_debug("%s:%s:%d %s: <%s> msix (%d) vec %d, entry %x\n",
-skdev->name, __func__, __LINE__,
-pci_name(pdev), skdev->name,
-i, qentry->vector, qentry->entry);
-   }
-
/* Enable MSI-X vectors for the base queue */
-   for (i = 0; i < skdev->msix_count; i++) {
-   qentry = >msix_entries[i];
+   for (i = 0; i < SKD_MAX_MSIX_COUNT; i++) {
+   struct skd_msix_entry *qentry = >msix_entries[i];
+
snprintf(qentry->isr_name, sizeof(qentry->isr_name),
 "%s%d-msix %s", DRV_NAME, skdev->devno,
 msix_entries[i].name);
-   rc = devm_request_irq(>pdev->dev, qentry->vector,
- msix_entries[i].handler, 0,
- qentry->isr_name, skdev);
+
+   rc = devm_request_irq(>pdev->dev,
+   pci_irq_vector(skdev->pdev, i),
+   msix_entries[i].handler, 0,
+   qentry->isr_name, skdev);
 

Re: cx23885: use pci_set_dma_mask insted of pci_dma_supported

[Christoph Hellwig]

Hi Tycho,

please try the patch below - Andrew should be sending it on to Linux soon.

---
>From 4c03a9f77104b04af45833e0424954191ca94a12 Mon Sep 17 00:00:00 2001
From: Christoph Hellwig <h...@lst.de>
Date: Wed, 11 Nov 2015 18:13:09 +0100
Subject: various: fix pci_set_dma_mask return value checking

pci_set_dma_mask returns a negative errno value, not a bool
like pci_dma_supported.  This of course was just a giant test
for attention :)

Reported-by: Jongman Heo <jongman@samsung.com>
Tested-by: Jongman Heo <jongman@samsung.com>[pcnet32]
Signed-off-by: Christoph Hellwig <h...@lst.de>
---
 drivers/media/pci/cx23885/cx23885-core.c   | 4 ++--
 drivers/media/pci/cx25821/cx25821-core.c   | 3 ++-
 drivers/media/pci/cx88/cx88-alsa.c | 4 ++--
 drivers/media/pci/cx88/cx88-mpeg.c | 3 ++-
 drivers/media/pci/cx88/cx88-video.c| 4 ++--
 drivers/media/pci/netup_unidvb/netup_unidvb_core.c | 2 +-
 drivers/media/pci/saa7134/saa7134-core.c   | 4 ++--
 drivers/media/pci/saa7164/saa7164-core.c   | 4 ++--
 drivers/media/pci/tw68/tw68-core.c | 4 ++--
 drivers/net/ethernet/amd/pcnet32.c | 5 +++--
 10 files changed, 20 insertions(+), 17 deletions(-)

diff --git a/drivers/media/pci/cx23885/cx23885-core.c 
b/drivers/media/pci/cx23885/cx23885-core.c
index 35759a9..e8f8472 100644
--- a/drivers/media/pci/cx23885/cx23885-core.c
+++ b/drivers/media/pci/cx23885/cx23885-core.c
@@ -1992,9 +1992,9 @@ static int cx23885_initdev(struct pci_dev *pci_dev,
(unsigned long long)pci_resource_start(pci_dev, 0));
 
pci_set_master(pci_dev);
-   if (!pci_set_dma_mask(pci_dev, 0x)) {
+   err = pci_set_dma_mask(pci_dev, 0x);
+   if (err) {
printk("%s/0: Oops: no 32bit PCI DMA ???\n", dev->name);
-   err = -EIO;
goto fail_context;
}
 
diff --git a/drivers/media/pci/cx25821/cx25821-core.c 
b/drivers/media/pci/cx25821/cx25821-core.c
index dbc695f..0042803 100644
--- a/drivers/media/pci/cx25821/cx25821-core.c
+++ b/drivers/media/pci/cx25821/cx25821-core.c
@@ -1319,7 +1319,8 @@ static int cx25821_initdev(struct pci_dev *pci_dev,
dev->pci_lat, (unsigned long long)dev->base_io_addr);
 
pci_set_master(pci_dev);
-   if (!pci_set_dma_mask(pci_dev, 0x)) {
+   err = pci_set_dma_mask(pci_dev, 0x);
+   if (err) {
pr_err("%s/0: Oops: no 32bit PCI DMA ???\n", dev->name);
err = -EIO;
goto fail_irq;
diff --git a/drivers/media/pci/cx88/cx88-alsa.c 
b/drivers/media/pci/cx88/cx88-alsa.c
index 0ed1b65..1b5268f 100644
--- a/drivers/media/pci/cx88/cx88-alsa.c
+++ b/drivers/media/pci/cx88/cx88-alsa.c
@@ -890,9 +890,9 @@ static int snd_cx88_create(struct snd_card *card, struct 
pci_dev *pci,
return err;
}
 
-   if (!pci_set_dma_mask(pci,DMA_BIT_MASK(32))) {
+   err = pci_set_dma_mask(pci,DMA_BIT_MASK(32));
+   if (err) {
dprintk(0, "%s/1: Oops: no 32bit PCI DMA ???\n",core->name);
-   err = -EIO;
cx88_core_put(core, pci);
return err;
}
diff --git a/drivers/media/pci/cx88/cx88-mpeg.c 
b/drivers/media/pci/cx88/cx88-mpeg.c
index 9db7767..f34c229 100644
--- a/drivers/media/pci/cx88/cx88-mpeg.c
+++ b/drivers/media/pci/cx88/cx88-mpeg.c
@@ -393,7 +393,8 @@ static int cx8802_init_common(struct cx8802_dev *dev)
if (pci_enable_device(dev->pci))
return -EIO;
pci_set_master(dev->pci);
-   if (!pci_set_dma_mask(dev->pci,DMA_BIT_MASK(32))) {
+   err = pci_set_dma_mask(dev->pci,DMA_BIT_MASK(32));
+   if (err) {
printk("%s/2: Oops: no 32bit PCI DMA ???\n",dev->core->name);
return -EIO;
}
diff --git a/drivers/media/pci/cx88/cx88-video.c 
b/drivers/media/pci/cx88/cx88-video.c
index 0de1ad5..aef9acf 100644
--- a/drivers/media/pci/cx88/cx88-video.c
+++ b/drivers/media/pci/cx88/cx88-video.c
@@ -1314,9 +1314,9 @@ static int cx8800_initdev(struct pci_dev *pci_dev,
   dev->pci_lat,(unsigned long long)pci_resource_start(pci_dev,0));
 
pci_set_master(pci_dev);
-   if (!pci_set_dma_mask(pci_dev,DMA_BIT_MASK(32))) {
+   err = pci_set_dma_mask(pci_dev,DMA_BIT_MASK(32));
+   if (err) {
printk("%s/0: Oops: no 32bit PCI DMA ???\n",core->name);
-   err = -EIO;
goto fail_core;
}
dev->alloc_ctx = vb2_dma_sg_init_ctx(_dev->dev);
diff --git a/drivers/media/pci/netup_unidvb/netup_unidvb_core.c 
b/drivers/media/pci/netup_unidvb/netup_unidvb_core.c
index 60b2d46..3fdbd81 100644
--- a/drivers/media/pci/netup_unidvb/netup_unidvb_core.c
+++ b/drivers/media/pci/netup_unidvb/netup_unidvb_core.c
@@ -810,7 

[PATCH 05/15] cx88: use pci_set_dma_mask insted of pci_dma_supported

[Christoph Hellwig]

This ensures the dma mask that is supported by the driver is recorded
in the device structure.

Signed-off-by: Christoph Hellwig <h...@lst.de>
---
 drivers/media/pci/cx88/cx88-alsa.c  | 2 +-
 drivers/media/pci/cx88/cx88-mpeg.c  | 2 +-
 drivers/media/pci/cx88/cx88-video.c | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/media/pci/cx88/cx88-alsa.c 
b/drivers/media/pci/cx88/cx88-alsa.c
index 7f8dc60..0703a81 100644
--- a/drivers/media/pci/cx88/cx88-alsa.c
+++ b/drivers/media/pci/cx88/cx88-alsa.c
@@ -890,7 +890,7 @@ static int snd_cx88_create(struct snd_card *card, struct 
pci_dev *pci,
return err;
}
 
-   if (!pci_dma_supported(pci,DMA_BIT_MASK(32))) {
+   if (!pci_set_dma_mask(pci,DMA_BIT_MASK(32))) {
dprintk(0, "%s/1: Oops: no 32bit PCI DMA ???\n",core->name);
err = -EIO;
cx88_core_put(core, pci);
diff --git a/drivers/media/pci/cx88/cx88-mpeg.c 
b/drivers/media/pci/cx88/cx88-mpeg.c
index 34f5057..9b3b565 100644
--- a/drivers/media/pci/cx88/cx88-mpeg.c
+++ b/drivers/media/pci/cx88/cx88-mpeg.c
@@ -393,7 +393,7 @@ static int cx8802_init_common(struct cx8802_dev *dev)
if (pci_enable_device(dev->pci))
return -EIO;
pci_set_master(dev->pci);
-   if (!pci_dma_supported(dev->pci,DMA_BIT_MASK(32))) {
+   if (!pci_set_dma_mask(dev->pci,DMA_BIT_MASK(32))) {
printk("%s/2: Oops: no 32bit PCI DMA ???\n",dev->core->name);
return -EIO;
}
diff --git a/drivers/media/pci/cx88/cx88-video.c 
b/drivers/media/pci/cx88/cx88-video.c
index 400e5ca..f12af31 100644
--- a/drivers/media/pci/cx88/cx88-video.c
+++ b/drivers/media/pci/cx88/cx88-video.c
@@ -1311,7 +1311,7 @@ static int cx8800_initdev(struct pci_dev *pci_dev,
   dev->pci_lat,(unsigned long long)pci_resource_start(pci_dev,0));
 
pci_set_master(pci_dev);
-   if (!pci_dma_supported(pci_dev,DMA_BIT_MASK(32))) {
+   if (!pci_set_dma_mask(pci_dev,DMA_BIT_MASK(32))) {
printk("%s/0: Oops: no 32bit PCI DMA ???\n",core->name);
err = -EIO;
goto fail_core;
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 02/15] tw68-core: use pci_set_dma_mask insted of pci_dma_supported

[Christoph Hellwig]

This ensures the dma mask that is supported by the driver is recorded
in the device structure.

Signed-off-by: Christoph Hellwig <h...@lst.de>
---
 drivers/media/pci/tw68/tw68-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/pci/tw68/tw68-core.c 
b/drivers/media/pci/tw68/tw68-core.c
index 04706cc..8c5655d 100644
--- a/drivers/media/pci/tw68/tw68-core.c
+++ b/drivers/media/pci/tw68/tw68-core.c
@@ -257,7 +257,7 @@ static int tw68_initdev(struct pci_dev *pci_dev,
dev->name, pci_name(pci_dev), dev->pci_rev, pci_dev->irq,
dev->pci_lat, (u64)pci_resource_start(pci_dev, 0));
pci_set_master(pci_dev);
-   if (!pci_dma_supported(pci_dev, DMA_BIT_MASK(32))) {
+   if (!pci_set_dma_mask(pci_dev, DMA_BIT_MASK(32))) {
pr_info("%s: Oops: no 32bit PCI DMA ???\n", dev->name);
err = -EIO;
goto fail1;
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

remove dma_supported and pci_dma_supported as public APIs

[Christoph Hellwig]

All driver should be using dma_set_mask / pci_set_dma_mask to try
to set the dma mask instead of just querying it.  Without that some
iommu implementations may not work.

pci_dma_supported is removed entirely, but dma_supported stays for
dma_ops implementations for now.

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 06/15] cx25821: use pci_set_dma_mask insted of pci_dma_supported

[Christoph Hellwig]

This ensures the dma mask that is supported by the driver is recorded
in the device structure.

Signed-off-by: Christoph Hellwig <h...@lst.de>
---
 drivers/media/pci/cx25821/cx25821-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/pci/cx25821/cx25821-core.c 
b/drivers/media/pci/cx25821/cx25821-core.c
index 559f829..dbc695f 100644
--- a/drivers/media/pci/cx25821/cx25821-core.c
+++ b/drivers/media/pci/cx25821/cx25821-core.c
@@ -1319,7 +1319,7 @@ static int cx25821_initdev(struct pci_dev *pci_dev,
dev->pci_lat, (unsigned long long)dev->base_io_addr);
 
pci_set_master(pci_dev);
-   if (!pci_dma_supported(pci_dev, 0x)) {
+   if (!pci_set_dma_mask(pci_dev, 0x)) {
pr_err("%s/0: Oops: no 32bit PCI DMA ???\n", dev->name);
err = -EIO;
goto fail_irq;
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 03/15] saa7164: use pci_set_dma_mask insted of pci_dma_supported

[Christoph Hellwig]

This ensures the dma mask that is supported by the driver is recorded
in the device structure.

Signed-off-by: Christoph Hellwig <h...@lst.de>
---
 drivers/media/pci/saa7164/saa7164-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/pci/saa7164/saa7164-core.c 
b/drivers/media/pci/saa7164/saa7164-core.c
index 3206a82..8f36b48 100644
--- a/drivers/media/pci/saa7164/saa7164-core.c
+++ b/drivers/media/pci/saa7164/saa7164-core.c
@@ -1264,7 +1264,7 @@ static int saa7164_initdev(struct pci_dev *pci_dev,
 
pci_set_master(pci_dev);
/* TODO */
-   if (!pci_dma_supported(pci_dev, 0x)) {
+   if (!pci_set_dma_mask(pci_dev, 0x)) {
printk("%s/0: Oops: no 32bit PCI DMA ???\n", dev->name);
err = -EIO;
goto fail_irq;
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 04/15] saa7134: use pci_set_dma_mask insted of pci_dma_supported

[Christoph Hellwig]

This ensures the dma mask that is supported by the driver is recorded
in the device structure.

Signed-off-by: Christoph Hellwig <h...@lst.de>
---
 drivers/media/pci/saa7134/saa7134-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/pci/saa7134/saa7134-core.c 
b/drivers/media/pci/saa7134/saa7134-core.c
index 72d7f99..6ba4086 100644
--- a/drivers/media/pci/saa7134/saa7134-core.c
+++ b/drivers/media/pci/saa7134/saa7134-core.c
@@ -949,7 +949,7 @@ static int saa7134_initdev(struct pci_dev *pci_dev,
   pci_name(pci_dev), dev->pci_rev, pci_dev->irq,
   dev->pci_lat,(unsigned long long)pci_resource_start(pci_dev,0));
pci_set_master(pci_dev);
-   if (!pci_dma_supported(pci_dev, DMA_BIT_MASK(32))) {
+   if (!pci_set_dma_mask(pci_dev, DMA_BIT_MASK(32))) {
pr_warn("%s: Oops: no 32bit PCI DMA ???\n", dev->name);
err = -EIO;
goto fail1;
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 01/15] pcnet32: use pci_set_dma_mask insted of pci_dma_supported

[Christoph Hellwig]

This ensures the dma mask that is supported by the driver is recorded
in the device structure.

Signed-off-by: Christoph Hellwig <h...@lst.de>
---
 drivers/net/ethernet/amd/pcnet32.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/amd/pcnet32.c 
b/drivers/net/ethernet/amd/pcnet32.c
index bc8b04f..e2afabf 100644
--- a/drivers/net/ethernet/amd/pcnet32.c
+++ b/drivers/net/ethernet/amd/pcnet32.c
@@ -1500,7 +1500,7 @@ pcnet32_probe_pci(struct pci_dev *pdev, const struct 
pci_device_id *ent)
return -ENODEV;
}
 
-   if (!pci_dma_supported(pdev, PCNET32_DMA_MASK)) {
+   if (!pci_set_dma_mask(pdev, PCNET32_DMA_MASK)) {
if (pcnet32_debug & NETIF_MSG_PROBE)
pr_err("architecture does not support 32bit PCI 
busmaster DMA\n");
return -ENODEV;
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 14/15] pci: remove pci_dma_supported

[Christoph Hellwig]

Signed-off-by: Christoph Hellwig <h...@lst.de>
---
 drivers/parisc/ccio-dma.c| 2 --
 include/asm-generic/pci-dma-compat.h | 6 --
 2 files changed, 8 deletions(-)

diff --git a/drivers/parisc/ccio-dma.c b/drivers/parisc/ccio-dma.c
index 957b421..8e11fb2 100644
--- a/drivers/parisc/ccio-dma.c
+++ b/drivers/parisc/ccio-dma.c
@@ -704,8 +704,6 @@ ccio_mark_invalid(struct ioc *ioc, dma_addr_t iova, size_t 
byte_cnt)
  * ccio_dma_supported - Verify the IOMMU supports the DMA address range.
  * @dev: The PCI device.
  * @mask: A bit mask describing the DMA address range of the device.
- *
- * This function implements the pci_dma_supported function.
  */
 static int 
 ccio_dma_supported(struct device *dev, u64 mask)
diff --git a/include/asm-generic/pci-dma-compat.h 
b/include/asm-generic/pci-dma-compat.h
index c110843..eafce7b 100644
--- a/include/asm-generic/pci-dma-compat.h
+++ b/include/asm-generic/pci-dma-compat.h
@@ -6,12 +6,6 @@
 
 #include 
 
-static inline int
-pci_dma_supported(struct pci_dev *hwdev, u64 mask)
-{
-   return dma_supported(hwdev == NULL ? NULL : >dev, mask);
-}
-
 static inline void *
 pci_alloc_consistent(struct pci_dev *hwdev, size_t size,
 dma_addr_t *dma_handle)
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 15/15] dma: remove external references to dma_supported

[Christoph Hellwig]

Signed-off-by: Christoph Hellwig <h...@lst.de>
---
 Documentation/DMA-API.txt   | 13 -
 drivers/usb/host/ehci-hcd.c |  2 +-
 drivers/usb/host/fotg210-hcd.c  |  2 +-
 drivers/usb/host/fusbh200-hcd.c |  2 +-
 drivers/usb/host/oxu210hp-hcd.c |  2 +-
 5 files changed, 4 insertions(+), 17 deletions(-)

diff --git a/Documentation/DMA-API.txt b/Documentation/DMA-API.txt
index edccacd..55f48684 100644
--- a/Documentation/DMA-API.txt
+++ b/Documentation/DMA-API.txt
@@ -142,19 +142,6 @@ Part Ic - DMA addressing limitations
 
 
 int
-dma_supported(struct device *dev, u64 mask)
-
-Checks to see if the device can support DMA to the memory described by
-mask.
-
-Returns: 1 if it can and 0 if it can't.
-
-Notes: This routine merely tests to see if the mask is possible.  It
-won't change the current mask settings.  It is more intended as an
-internal API for use by the platform than an external API for use by
-driver writers.
-
-int
 dma_set_mask_and_coherent(struct device *dev, u64 mask)
 
 Checks to see if the mask is possible and updates the device
diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index c63d82c..48c92bf 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -589,7 +589,7 @@ static int ehci_run (struct usb_hcd *hcd)
 * streaming mappings for I/O buffers, like pci_map_single(),
 * can return segments above 4GB, if the device allows.
 *
-* NOTE:  the dma mask is visible through dma_supported(), so
+* NOTE:  the dma mask is visible through dev->dma_mask, so
 * drivers can pass this info along ... like NETIF_F_HIGHDMA,
 * Scsi_Host.highmem_io, and so forth.  It's readonly to all
 * host side drivers though.
diff --git a/drivers/usb/host/fotg210-hcd.c b/drivers/usb/host/fotg210-hcd.c
index 000ed80..c5fc3ef 100644
--- a/drivers/usb/host/fotg210-hcd.c
+++ b/drivers/usb/host/fotg210-hcd.c
@@ -5258,7 +5258,7 @@ static int fotg210_run(struct usb_hcd *hcd)
 * streaming mappings for I/O buffers, like pci_map_single(),
 * can return segments above 4GB, if the device allows.
 *
-* NOTE:  the dma mask is visible through dma_supported(), so
+* NOTE:  the dma mask is visible through dev->dma_mask, so
 * drivers can pass this info along ... like NETIF_F_HIGHDMA,
 * Scsi_Host.highmem_io, and so forth.  It's readonly to all
 * host side drivers though.
diff --git a/drivers/usb/host/fusbh200-hcd.c b/drivers/usb/host/fusbh200-hcd.c
index 1fd8718..4a1243a 100644
--- a/drivers/usb/host/fusbh200-hcd.c
+++ b/drivers/usb/host/fusbh200-hcd.c
@@ -5181,7 +5181,7 @@ static int fusbh200_run (struct usb_hcd *hcd)
 * streaming mappings for I/O buffers, like pci_map_single(),
 * can return segments above 4GB, if the device allows.
 *
-* NOTE:  the dma mask is visible through dma_supported(), so
+* NOTE:  the dma mask is visible through dev->dma_mask, so
 * drivers can pass this info along ... like NETIF_F_HIGHDMA,
 * Scsi_Host.highmem_io, and so forth.  It's readonly to all
 * host side drivers though.
diff --git a/drivers/usb/host/oxu210hp-hcd.c b/drivers/usb/host/oxu210hp-hcd.c
index fe3bd1c..1f139d8 100644
--- a/drivers/usb/host/oxu210hp-hcd.c
+++ b/drivers/usb/host/oxu210hp-hcd.c
@@ -2721,7 +2721,7 @@ static int oxu_run(struct usb_hcd *hcd)
 * streaming mappings for I/O buffers, like pci_map_single(),
 * can return segments above 4GB, if the device allows.
 *
-* NOTE:  the dma mask is visible through dma_supported(), so
+* NOTE:  the dma mask is visible through dev->dma_mask, so
 * drivers can pass this info along ... like NETIF_F_HIGHDMA,
 * Scsi_Host.highmem_io, and so forth.  It's readonly to all
 * host side drivers though.
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 13/15] usbnet: remove ifdefed out call to dma_supported

[Christoph Hellwig]

Signed-off-by: Christoph Hellwig <h...@lst.de>
---
 drivers/net/usb/usbnet.c | 6 --
 1 file changed, 6 deletions(-)

diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index b4cf107..9497d51 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -1661,12 +1661,6 @@ usbnet_probe (struct usb_interface *udev, const struct 
usb_device_id *prod)
 * bind() should set rx_urb_size in that case.
 */
dev->hard_mtu = net->mtu + net->hard_header_len;
-#if 0
-// dma_supported() is deeply broken on almost all architectures
-   // possible with some EHCI controllers
-   if (dma_supported (>dev, DMA_BIT_MASK(64)))
-   net->features |= NETIF_F_HIGHDMA;
-#endif
 
net->netdev_ops = _netdev_ops;
net->watchdog_timeo = TX_TIMEOUT_JIFFIES;
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 08/15] netup_unidvb: use pci_set_dma_mask insted of pci_dma_supported

[Christoph Hellwig]

This ensures the dma mask that is supported by the driver is recorded
in the device structure.

Signed-off-by: Christoph Hellwig <h...@lst.de>
---
 drivers/media/pci/netup_unidvb/netup_unidvb_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/pci/netup_unidvb/netup_unidvb_core.c 
b/drivers/media/pci/netup_unidvb/netup_unidvb_core.c
index 6d8bf627..511144f 100644
--- a/drivers/media/pci/netup_unidvb/netup_unidvb_core.c
+++ b/drivers/media/pci/netup_unidvb/netup_unidvb_core.c
@@ -809,7 +809,7 @@ static int netup_unidvb_initdev(struct pci_dev *pci_dev,
"%s(): board vendor 0x%x, revision 0x%x\n",
__func__, board_vendor, board_revision);
pci_set_master(pci_dev);
-   if (!pci_dma_supported(pci_dev, 0x)) {
+   if (!pci_set_dma_mask(pci_dev, 0x)) {
dev_err(_dev->dev,
"%s(): 32bit PCI DMA is not supported\n", __func__);
goto pci_detect_err;
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 12/15] kaweth: remove ifdefed out call to dma_supported

[Christoph Hellwig]

Signed-off-by: Christoph Hellwig <h...@lst.de>
---
 drivers/net/usb/kaweth.c | 6 --
 1 file changed, 6 deletions(-)

diff --git a/drivers/net/usb/kaweth.c b/drivers/net/usb/kaweth.c
index 1e9cdca..f64b25c 100644
--- a/drivers/net/usb/kaweth.c
+++ b/drivers/net/usb/kaweth.c
@@ -1177,12 +1177,6 @@ err_fw:
INIT_DELAYED_WORK(>lowmem_work, kaweth_resubmit_tl);
usb_set_intfdata(intf, kaweth);
 
-#if 0
-// dma_supported() is deeply broken on almost all architectures
-   if (dma_supported (dev, 0xULL))
-   kaweth->net->features |= NETIF_F_HIGHDMA;
-#endif
-
SET_NETDEV_DEV(netdev, dev);
if (register_netdev(netdev) != 0) {
dev_err(dev, "Error registering netdev.\n");
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 07/15] cx23885: use pci_set_dma_mask insted of pci_dma_supported

[Christoph Hellwig]

This ensures the dma mask that is supported by the driver is recorded
in the device structure.

Signed-off-by: Christoph Hellwig <h...@lst.de>
---
 drivers/media/pci/cx23885/cx23885-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/pci/cx23885/cx23885-core.c 
b/drivers/media/pci/cx23885/cx23885-core.c
index 7aee76a..8194052 100644
--- a/drivers/media/pci/cx23885/cx23885-core.c
+++ b/drivers/media/pci/cx23885/cx23885-core.c
@@ -1990,7 +1990,7 @@ static int cx23885_initdev(struct pci_dev *pci_dev,
(unsigned long long)pci_resource_start(pci_dev, 0));
 
pci_set_master(pci_dev);
-   if (!pci_dma_supported(pci_dev, 0x)) {
+   if (!pci_set_dma_mask(pci_dev, 0x)) {
printk("%s/0: Oops: no 32bit PCI DMA ???\n", dev->name);
err = -EIO;
goto fail_context;
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 10/15] nouveau: don't call pci_dma_supported

[Christoph Hellwig]

Just try to set a 64-bit DMA mask first and retry with the smaller dma_mask
if dma_set_mask failed.

Signed-off-by: Christoph Hellwig <h...@lst.de>
---
 drivers/gpu/drm/nouveau/nouveau_ttm.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_ttm.c 
b/drivers/gpu/drm/nouveau/nouveau_ttm.c
index 3f0fb55..bb030e6 100644
--- a/drivers/gpu/drm/nouveau/nouveau_ttm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_ttm.c
@@ -350,11 +350,14 @@ nouveau_ttm_init(struct nouveau_drm *drm)
 
bits = nvxx_mmu(>device)->dma_bits;
if (nvxx_device(>device)->func->pci) {
-   if (drm->agp.bridge ||
-!pci_dma_supported(dev->pdev, DMA_BIT_MASK(bits)))
+   if (drm->agp.bridge)
bits = 32;
 
ret = pci_set_dma_mask(dev->pdev, DMA_BIT_MASK(bits));
+   if (ret && bits != 32) {
+   bits = 32;
+   ret = pci_set_dma_mask(dev->pdev, DMA_BIT_MASK(bits));
+   }
if (ret)
return ret;
 
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 11/15] sfc: don't call dma_supported

[Christoph Hellwig]

dma_set_mask already checks for a supported DMA mask before updating it,
the call to dma_supported is redundant.

Signed-off-by: Christoph Hellwig <h...@lst.de>
---
 drivers/net/ethernet/sfc/efx.c | 8 +++-
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/sfc/efx.c b/drivers/net/ethernet/sfc/efx.c
index 974637d..4abe886 100644
--- a/drivers/net/ethernet/sfc/efx.c
+++ b/drivers/net/ethernet/sfc/efx.c
@@ -1247,11 +1247,9 @@ static int efx_init_io(struct efx_nic *efx)
 * masks event though they reject 46 bit masks.
 */
while (dma_mask > 0x7fffUL) {
-   if (dma_supported(_dev->dev, dma_mask)) {
-   rc = dma_set_mask_and_coherent(_dev->dev, dma_mask);
-   if (rc == 0)
-   break;
-   }
+   rc = dma_set_mask_and_coherent(_dev->dev, dma_mask);
+   if (rc == 0)
+   break;
dma_mask >>= 1;
}
if (rc) {
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 09/15] mpsc: use dma_set_mask insted of dma_supported

[Christoph Hellwig]

This ensures the dma mask that is supported by the driver is recorded
in the device structure.

Signed-off-by: Christoph Hellwig <h...@lst.de>
---
 drivers/tty/serial/mpsc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/serial/mpsc.c b/drivers/tty/serial/mpsc.c
index 82bb6d1..11e084e 100644
--- a/drivers/tty/serial/mpsc.c
+++ b/drivers/tty/serial/mpsc.c
@@ -755,7 +755,7 @@ static int mpsc_alloc_ring_mem(struct mpsc_port_info *pi)
pi->port.line);
 
if (!pi->dma_region) {
-   if (!dma_supported(pi->port.dev, 0x)) {
+   if (!dma_set_mask(pi->port.dev, 0x)) {
printk(KERN_ERR "MPSC: Inadequate DMA support\n");
rc = -ENXIO;
} else if ((pi->dma_region = dma_alloc_noncoherent(pi->port.dev,
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 29/31] parisc: handle page-less SG entries

[Christoph Hellwig]

On Wed, Aug 12, 2015 at 09:01:02AM -0700, Linus Torvalds wrote:
 I'm assuming that anybody who wants to use the page-less
 scatter-gather lists always does so on memory that isn't actually
 virtually mapped at all, or only does so on sane architectures that
 are cache coherent at a physical level, but I'd like that assumption
 *documented* somewhere.

It's temporarily mapped by kmap-like helpers.  That code isn't in
this series. The most recent version of it is here:

https://git.kernel.org/cgit/linux/kernel/git/djbw/nvdimm.git/commit/?h=pfnid=de8237c99fdb4352be2193f3a7610e902b9bb2f0

note that it's not doing the cache flushing it would have to do yet, but
it's also only enabled for x86 at the moment.
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 31/31] dma-mapping-common: skip kmemleak checks for page-less SG entries

[Christoph Hellwig]

On Wed, Aug 12, 2015 at 09:05:15AM -0700, Linus Torvalds wrote:
 [ Again, I'm responding to one random patch - this pattern was in
 other patches too.  ]
 
 A question: do we actually expect to mix page-less and pageful SG
 entries in the same SG list?
 
 How does that happen?

Both for DAX and the video buffer case people could do direct I/O
spanning the boundary between such a VMA and a normal one unless
we add special code to prevent that.  Right now I don't think it's
all that useful, but then again it doesn't seem harmful either
and adding those checks might add up.
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: RFC: prepare for struct scatterlist entries without page backing

[Christoph Hellwig]

On Thu, Aug 13, 2015 at 09:37:37AM +1000, Julian Calaby wrote:
 I.e. ~90% of this patch set seems to be just mechanically dropping
 BUG_ON()s and converting open coded stuff to use accessor functions
 (which should be macros or get inlined, right?) - and the remaining
 bit is not flushing if we don't have a physical page somewhere.

Which is was 90%.  By lines changed most actually is the diffs for
the cache flushing.

 Would it make sense to split this patch set into a few bits: one to
 drop all the useless BUG_ON()s, one to convert all the open coded
 stuff to accessor functions, then another to do the actual page-less
 sg stuff?

Without the ifs the BUG_ON() actually are useful to assert we
never feed the sort of physical addresses we can't otherwise support,
so I don't think that part is doable.

A simple series to make more use of sg_phys and add sg_pfn might
still be useful, though.
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: RFC: prepare for struct scatterlist entries without page backing

[Christoph Hellwig]

On Wed, Aug 12, 2015 at 03:42:47PM +0300, Boaz Harrosh wrote:
 The support I have suggested and submitted for zone-less sections.
 (In my add_persistent_memory() patchset)

 Would work perfectly well and transparent for all such multimedia cases.
 (All hacks removed). In fact I have loaded pmem (with-pages) on a VRAM
 a few times and it is great easy fun. (I wanted to experiment with cached
 memory over a pcie)

And everyone agree that it was both buggy and incomplete.

Dan has done a respin of the page backed nvdimm work with most of
these comments addressed.

I have to say I hate both pfn-based I/O [1] and page backed nvdimms with
passion, so we're looking into the lesser evil with an open mind.

[1] not the SGL part posted here, which I think is quite sane.  The bio
side is much worse, though.
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 16/31] s390: handle page-less SG entries

[Christoph Hellwig]

Use sg_phys() instead of page_to_phys(sg_page(sg)) so that we don't
require a page structure for all DMA memory.

Signed-off-by: Christoph Hellwig h...@lst.de
---
 arch/s390/pci/pci_dma.c | 20 ++--
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/arch/s390/pci/pci_dma.c b/arch/s390/pci/pci_dma.c
index 6fd8d58..aae5a47 100644
--- a/arch/s390/pci/pci_dma.c
+++ b/arch/s390/pci/pci_dma.c
@@ -272,14 +272,13 @@ int dma_set_mask(struct device *dev, u64 mask)
 }
 EXPORT_SYMBOL_GPL(dma_set_mask);
 
-static dma_addr_t s390_dma_map_pages(struct device *dev, struct page *page,
-unsigned long offset, size_t size,
+static dma_addr_t s390_dma_map_phys(struct device *dev, unsigned long pa,
+size_t size,
 enum dma_data_direction direction,
 struct dma_attrs *attrs)
 {
struct zpci_dev *zdev = get_zdev(to_pci_dev(dev));
unsigned long nr_pages, iommu_page_index;
-   unsigned long pa = page_to_phys(page) + offset;
int flags = ZPCI_PTE_VALID;
dma_addr_t dma_addr;
 
@@ -301,7 +300,7 @@ static dma_addr_t s390_dma_map_pages(struct device *dev, 
struct page *page,
 
if (!dma_update_trans(zdev, pa, dma_addr, size, flags)) {
atomic64_add(nr_pages, zdev-mapped_pages);
-   return dma_addr + (offset  ~PAGE_MASK);
+   return dma_addr + (pa  ~PAGE_MASK);
}
 
 out_free:
@@ -312,6 +311,16 @@ out_err:
return DMA_ERROR_CODE;
 }
 
+static dma_addr_t s390_dma_map_pages(struct device *dev, struct page *page,
+unsigned long offset, size_t size,
+enum dma_data_direction direction,
+struct dma_attrs *attrs)
+{
+   unsigned long pa = page_to_phys(page) + offset;
+
+   return s390_dma_map_phys(dev, pa, size, direction, attrs);
+}
+
 static void s390_dma_unmap_pages(struct device *dev, dma_addr_t dma_addr,
 size_t size, enum dma_data_direction direction,
 struct dma_attrs *attrs)
@@ -384,8 +393,7 @@ static int s390_dma_map_sg(struct device *dev, struct 
scatterlist *sg,
int i;
 
for_each_sg(sg, s, nr_elements, i) {
-   struct page *page = sg_page(s);
-   s-dma_address = s390_dma_map_pages(dev, page, s-offset,
+   s-dma_address = s390_dma_map_phys(dev, sg_phys(s),
s-length, dir, NULL);
if (!dma_mapping_error(dev, s-dma_address)) {
s-dma_length = s-length;
-- 
1.9.1

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 17/31] ia64/sba_iommu: remove sba_sg_address

[Christoph Hellwig]

Signed-off-by: Christoph Hellwig h...@lst.de
---
 arch/ia64/hp/common/sba_iommu.c | 22 ++
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/arch/ia64/hp/common/sba_iommu.c b/arch/ia64/hp/common/sba_iommu.c
index 344387a..9e5aa8e 100644
--- a/arch/ia64/hp/common/sba_iommu.c
+++ b/arch/ia64/hp/common/sba_iommu.c
@@ -248,8 +248,6 @@ static int reserve_sba_gart = 1;
 static SBA_INLINE void sba_mark_invalid(struct ioc *, dma_addr_t, size_t);
 static SBA_INLINE void sba_free_range(struct ioc *, dma_addr_t, size_t);
 
-#define sba_sg_address(sg) sg_virt((sg))
-
 #ifdef FULL_VALID_PDIR
 static u64 prefetch_spill_page;
 #endif
@@ -397,7 +395,7 @@ sba_dump_sg( struct ioc *ioc, struct scatterlist *startsg, 
int nents)
while (nents--  0) {
printk(KERN_DEBUG  %d : DMA %08lx/%05x CPU %p\n, nents,
   startsg-dma_address, startsg-dma_length,
-  sba_sg_address(startsg));
+  sg_virt(startsg));
startsg = sg_next(startsg);
}
 }
@@ -409,7 +407,7 @@ sba_check_sg( struct ioc *ioc, struct scatterlist *startsg, 
int nents)
int the_nents = nents;
 
while (the_nents--  0) {
-   if (sba_sg_address(the_sg) == 0x0UL)
+   if (sg_virt(the_sg) == 0x0UL)
sba_dump_sg(NULL, startsg, nents);
the_sg = sg_next(the_sg);
}
@@ -1243,11 +1241,11 @@ sba_fill_pdir(
if (dump_run_sg)
printk( %2d : %08lx/%05x %p\n,
nents, startsg-dma_address, cnt,
-   sba_sg_address(startsg));
+   sg_virt(startsg));
 #else
DBG_RUN_SG( %d : %08lx/%05x %p\n,
nents, startsg-dma_address, cnt,
-   sba_sg_address(startsg));
+   sg_virt(startsg));
 #endif
/*
** Look for the start of a new DMA stream
@@ -1267,7 +1265,7 @@ sba_fill_pdir(
** Look for a VCONTIG chunk
*/
if (cnt) {
-   unsigned long vaddr = (unsigned long) 
sba_sg_address(startsg);
+   unsigned long vaddr = (unsigned long) sg_virt(startsg);
ASSERT(pdirp);
 
/* Since multiple Vcontig blocks could make up
@@ -1335,7 +1333,7 @@ sba_coalesce_chunks(struct ioc *ioc, struct device *dev,
int idx;
 
while (nents  0) {
-   unsigned long vaddr = (unsigned long) sba_sg_address(startsg);
+   unsigned long vaddr = (unsigned long) sg_virt(startsg);
 
/*
** Prepare for first/next DMA stream
@@ -1380,7 +1378,7 @@ sba_coalesce_chunks(struct ioc *ioc, struct device *dev,
**
** append the next transaction?
*/
-   vaddr = (unsigned long) sba_sg_address(startsg);
+   vaddr = (unsigned long) sg_virt(startsg);
if  (vcontig_end == vaddr)
{
vcontig_len += startsg-length;
@@ -1479,7 +1477,7 @@ static int sba_map_sg_attrs(struct device *dev, struct 
scatterlist *sglist,
if (likely((ioc-dma_mask  ~to_pci_dev(dev)-dma_mask) == 0)) {
for_each_sg(sglist, sg, nents, filled) {
sg-dma_length = sg-length;
-   sg-dma_address = virt_to_phys(sba_sg_address(sg));
+   sg-dma_address = virt_to_phys(sg_virt(sg));
}
return filled;
}
@@ -1487,7 +1485,7 @@ static int sba_map_sg_attrs(struct device *dev, struct 
scatterlist *sglist,
/* Fast path single entry scatterlists. */
if (nents == 1) {
sglist-dma_length = sglist-length;
-   sglist-dma_address = sba_map_single_attrs(dev, 
sba_sg_address(sglist), sglist-length, dir, attrs);
+   sglist-dma_address = sba_map_single_attrs(dev, 
sg_virt(sglist), sglist-length, dir, attrs);
return 1;
}
 
@@ -1563,7 +1561,7 @@ static void sba_unmap_sg_attrs(struct device *dev, struct 
scatterlist *sglist,
 #endif
 
DBG_RUN_SG(%s() START %d entries,  %p,%x\n,
-  __func__, nents, sba_sg_address(sglist), sglist-length);
+  __func__, nents, sg_virt(sglist), sglist-length);
 
 #ifdef ASSERT_PDIR_SANITY
ioc = GET_IOC(dev);
-- 
1.9.1

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 18/31] nios2: handle page-less SG entries

[Christoph Hellwig]

Make all cache invalidation conditional on sg_has_page() and use
sg_phys to get the physical address directly.

Signed-off-by: Christoph Hellwig h...@lst.de
---
 arch/nios2/mm/dma-mapping.c | 29 +++--
 1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/arch/nios2/mm/dma-mapping.c b/arch/nios2/mm/dma-mapping.c
index ac5da75..1a0a68d 100644
--- a/arch/nios2/mm/dma-mapping.c
+++ b/arch/nios2/mm/dma-mapping.c
@@ -64,13 +64,11 @@ int dma_map_sg(struct device *dev, struct scatterlist *sg, 
int nents,
BUG_ON(!valid_dma_direction(direction));
 
for_each_sg(sg, sg, nents, i) {
-   void *addr;
-
-   addr = sg_virt(sg);
-   if (addr) {
-   __dma_sync_for_device(addr, sg-length, direction);
-   sg-dma_address = sg_phys(sg);
+   if (sg_has_page(sg)) {
+   __dma_sync_for_device(sg_virt(sg), sg-length,
+   direction);
}
+   sg-dma_address = sg_phys(sg);
}
 
return nents;
@@ -113,9 +111,8 @@ void dma_unmap_sg(struct device *dev, struct scatterlist 
*sg, int nhwentries,
return;
 
for_each_sg(sg, sg, nhwentries, i) {
-   addr = sg_virt(sg);
-   if (addr)
-   __dma_sync_for_cpu(addr, sg-length, direction);
+   if (sg_has_page(sg))
+   __dma_sync_for_cpu(sg_virt(sg), sg-length, direction);
}
 }
 EXPORT_SYMBOL(dma_unmap_sg);
@@ -166,8 +163,10 @@ void dma_sync_sg_for_cpu(struct device *dev, struct 
scatterlist *sg, int nelems,
BUG_ON(!valid_dma_direction(direction));
 
/* Make sure that gcc doesn't leave the empty loop body.  */
-   for_each_sg(sg, sg, nelems, i)
-   __dma_sync_for_cpu(sg_virt(sg), sg-length, direction);
+   for_each_sg(sg, sg, nelems, i) {
+   if (sg_has_page(sg))
+   __dma_sync_for_cpu(sg_virt(sg), sg-length, direction);
+   }
 }
 EXPORT_SYMBOL(dma_sync_sg_for_cpu);
 
@@ -179,8 +178,10 @@ void dma_sync_sg_for_device(struct device *dev, struct 
scatterlist *sg,
BUG_ON(!valid_dma_direction(direction));
 
/* Make sure that gcc doesn't leave the empty loop body.  */
-   for_each_sg(sg, sg, nelems, i)
-   __dma_sync_for_device(sg_virt(sg), sg-length, direction);
-
+   for_each_sg(sg, sg, nelems, i) {
+   if (sg_has_page(sg))
+   __dma_sync_for_device(sg_virt(sg), sg-length,
+   direction);
+   }
 }
 EXPORT_SYMBOL(dma_sync_sg_for_device);
-- 
1.9.1

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RFC: prepare for struct scatterlist entries without page backing

[Christoph Hellwig]

Dan Williams started to look into addressing I/O to and from
Persistent Memory in his series from June:

http://thread.gmane.org/gmane.linux.kernel.cross-arch/27944

I've started looking into DMA mapping of these SGLs specifically instead
of the map_pfn method in there.  In addition to supporting NVDIMM backed
I/O I also suspect this would be highly useful for media drivers that
go through nasty hoops to be able to DMA from/to their ioremapped regions,
with vb2_dc_get_userptr in drivers/media/v4l2-core/videobuf2-dma-contig.c
being a prime example for the unsafe hacks currently used.

It turns out most DMA mapping implementation can handle SGLs without
page structures with some fairly simple mechanical work.  Most of it
is just about consistently using sg_phys.  For implementations that
need to flush caches we need a new helper that skips these cache
flushes if a entry doesn't have a kernel virtual address.

However the ccio (parisc) and sba_iommu (parisc  ia64) IOMMUs seem
to be operate mostly on virtual addresses.  It's a fairly odd concept
that I don't fully grasp, so I'll need some help with those if we want
to bring this forward.

Additional this series skips ARM entirely for now.  The reason is
that most arm implementations of the .map_sg operation just iterate
over all entries and call -map_page for it, which means we'd need
to convert those to a -map_pfn similar to Dan's previous approach.

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 03/31] dma-debug: handle page-less SG entries

[Christoph Hellwig]

Use sg_pfn to get a the PFN and skip checks that require a kernel
virtual address.

Signed-off-by: Christoph Hellwig h...@lst.de
---
 lib/dma-debug.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/lib/dma-debug.c b/lib/dma-debug.c
index dace71f..a215a80 100644
--- a/lib/dma-debug.c
+++ b/lib/dma-debug.c
@@ -1368,7 +1368,7 @@ void debug_dma_map_sg(struct device *dev, struct 
scatterlist *sg,
 
entry-type   = dma_debug_sg;
entry-dev= dev;
-   entry-pfn= page_to_pfn(sg_page(s));
+   entry-pfn= sg_pfn(s);
entry-offset = s-offset,
entry-size   = sg_dma_len(s);
entry-dev_addr   = sg_dma_address(s);
@@ -1376,7 +1376,7 @@ void debug_dma_map_sg(struct device *dev, struct 
scatterlist *sg,
entry-sg_call_ents   = nents;
entry-sg_mapped_ents = mapped_ents;
 
-   if (!PageHighMem(sg_page(s))) {
+   if (sg_has_page(s)  !PageHighMem(sg_page(s))) {
check_for_stack(dev, sg_virt(s));
check_for_illegal_area(dev, sg_virt(s), sg_dma_len(s));
}
@@ -1419,7 +1419,7 @@ void debug_dma_unmap_sg(struct device *dev, struct 
scatterlist *sglist,
struct dma_debug_entry ref = {
.type   = dma_debug_sg,
.dev= dev,
-   .pfn= page_to_pfn(sg_page(s)),
+   .pfn= sg_pfn(s),
.offset = s-offset,
.dev_addr   = sg_dma_address(s),
.size   = sg_dma_len(s),
@@ -1580,7 +1580,7 @@ void debug_dma_sync_sg_for_cpu(struct device *dev, struct 
scatterlist *sg,
struct dma_debug_entry ref = {
.type   = dma_debug_sg,
.dev= dev,
-   .pfn= page_to_pfn(sg_page(s)),
+   .pfn= sg_pfn(s),
.offset = s-offset,
.dev_addr   = sg_dma_address(s),
.size   = sg_dma_len(s),
@@ -1613,7 +1613,7 @@ void debug_dma_sync_sg_for_device(struct device *dev, 
struct scatterlist *sg,
struct dma_debug_entry ref = {
.type   = dma_debug_sg,
.dev= dev,
-   .pfn= page_to_pfn(sg_page(s)),
+   .pfn= sg_pfn(s),
.offset = s-offset,
.dev_addr   = sg_dma_address(s),
.size   = sg_dma_len(s),
-- 
1.9.1

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 01/31] scatterlist: add sg_pfn and sg_has_page helpers

[Christoph Hellwig]

Signed-off-by: Christoph Hellwig h...@lst.de
---
 include/linux/scatterlist.h | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h
index 9b1ef0c..b1056bf 100644
--- a/include/linux/scatterlist.h
+++ b/include/linux/scatterlist.h
@@ -230,6 +230,16 @@ static inline dma_addr_t sg_phys(struct scatterlist *sg)
return page_to_phys(sg_page(sg)) + sg-offset;
 }
 
+static inline unsigned long sg_pfn(struct scatterlist *sg)
+{
+   return page_to_pfn(sg_page(sg));
+}
+
+static inline bool sg_has_page(struct scatterlist *sg)
+{
+   return true;
+}
+
 /**
  * sg_virt - Return virtual address of an sg entry
  * @sg:  SG entry
-- 
1.9.1

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 02/31] scatterlist: use sg_phys()

[Christoph Hellwig]

From: Dan Williams dan.j.willi...@intel.com

Coccinelle cleanup to replace open coded sg to physical address
translations.  This is in preparation for introducing scatterlists that
reference __pfn_t.

// sg_phys.cocci: convert usage page_to_phys(sg_page(sg)) to sg_phys(sg)
// usage: make coccicheck COCCI=sg_phys.cocci MODE=patch

virtual patch

@@
struct scatterlist *sg;
@@

- page_to_phys(sg_page(sg)) + sg-offset
+ sg_phys(sg)

@@
struct scatterlist *sg;
@@

- page_to_phys(sg_page(sg))
+ sg_phys(sg)  PAGE_MASK

Signed-off-by: Dan Williams dan.j.willi...@intel.com
---
 arch/arm/mm/dma-mapping.c| 2 +-
 arch/microblaze/kernel/dma.c | 3 +--
 drivers/iommu/intel-iommu.c  | 4 ++--
 drivers/iommu/iommu.c| 2 +-
 drivers/staging/android/ion/ion_chunk_heap.c | 4 ++--
 5 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index cba12f3..3d3d6aa 100644
--- a/arch/arm/mm/dma-mapping.c
+++ b/arch/arm/mm/dma-mapping.c
@@ -1520,7 +1520,7 @@ static int __map_sg_chunk(struct device *dev, struct 
scatterlist *sg,
return -ENOMEM;
 
for (count = 0, s = sg; count  (size  PAGE_SHIFT); s = sg_next(s)) {
-   phys_addr_t phys = page_to_phys(sg_page(s));
+   phys_addr_t phys = sg_phys(s)  PAGE_MASK;
unsigned int len = PAGE_ALIGN(s-offset + s-length);
 
if (!is_coherent 
diff --git a/arch/microblaze/kernel/dma.c b/arch/microblaze/kernel/dma.c
index bf4dec2..c89da63 100644
--- a/arch/microblaze/kernel/dma.c
+++ b/arch/microblaze/kernel/dma.c
@@ -61,8 +61,7 @@ static int dma_direct_map_sg(struct device *dev, struct 
scatterlist *sgl,
/* FIXME this part of code is untested */
for_each_sg(sgl, sg, nents, i) {
sg-dma_address = sg_phys(sg);
-   __dma_sync(page_to_phys(sg_page(sg)) + sg-offset,
-   sg-length, direction);
+   __dma_sync(sg_phys(sg), sg-length, direction);
}
 
return nents;
diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 0649b94..3541d65 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -2097,7 +2097,7 @@ static int __domain_mapping(struct dmar_domain *domain, 
unsigned long iov_pfn,
sg_res = aligned_nrpages(sg-offset, sg-length);
sg-dma_address = ((dma_addr_t)iov_pfn  
VTD_PAGE_SHIFT) + sg-offset;
sg-dma_length = sg-length;
-   pteval = page_to_phys(sg_page(sg)) | prot;
+   pteval = (sg_phys(sg)  PAGE_MASK) | prot;
phys_pfn = pteval  VTD_PAGE_SHIFT;
}
 
@@ -3623,7 +3623,7 @@ static int intel_nontranslate_map_sg(struct device *hddev,
 
for_each_sg(sglist, sg, nelems, i) {
BUG_ON(!sg_page(sg));
-   sg-dma_address = page_to_phys(sg_page(sg)) + sg-offset;
+   sg-dma_address = sg_phys(sg);
sg-dma_length = sg-length;
}
return nelems;
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index f286090..049df49 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1408,7 +1408,7 @@ size_t default_iommu_map_sg(struct iommu_domain *domain, 
unsigned long iova,
min_pagesz = 1  __ffs(domain-ops-pgsize_bitmap);
 
for_each_sg(sg, s, nents, i) {
-   phys_addr_t phys = page_to_phys(sg_page(s)) + s-offset;
+   phys_addr_t phys = sg_phys(s);
 
/*
 * We are mapping on IOMMU page boundaries, so offset within
diff --git a/drivers/staging/android/ion/ion_chunk_heap.c 
b/drivers/staging/android/ion/ion_chunk_heap.c
index 5474615..f7b6ef9 100644
--- a/drivers/staging/android/ion/ion_chunk_heap.c
+++ b/drivers/staging/android/ion/ion_chunk_heap.c
@@ -81,7 +81,7 @@ static int ion_chunk_heap_allocate(struct ion_heap *heap,
 err:
sg = table-sgl;
for (i -= 1; i = 0; i--) {
-   gen_pool_free(chunk_heap-pool, page_to_phys(sg_page(sg)),
+   gen_pool_free(chunk_heap-pool, sg_phys(sg)  PAGE_MASK,
  sg-length);
sg = sg_next(sg);
}
@@ -109,7 +109,7 @@ static void ion_chunk_heap_free(struct ion_buffer *buffer)
DMA_BIDIRECTIONAL);
 
for_each_sg(table-sgl, sg, table-nents, i) {
-   gen_pool_free(chunk_heap-pool, page_to_phys(sg_page(sg)),
+   gen_pool_free(chunk_heap-pool, sg_phys(sg)  PAGE_MASK,
  sg-length);
}
chunk_heap-allocated -= allocated_size;
-- 
1.9.1

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at