Re: [PATCH 0/4] Generic IOMMU page table framework

2014-12-01 Thread Will Deacon
On Sun, Nov 30, 2014 at 10:03:08PM +, Laurent Pinchart wrote:
> Hi Will,

Hi Laurent,

> On Thursday 27 November 2014 11:51:14 Will Deacon wrote:
> > Hi all,
> > 
> > This series introduces a generic IOMMU page table allocation framework,
> > implements support for ARM long-descriptors and then ports the arm-smmu
> > driver over to the new code.
> > 
> > There are a few reasons for doing this:
> > 
> >   - Page table code is hard, and I don't enjoy shopping
> > 
> >   - A number of IOMMUs actually use the same table format, but currently
> > duplicate the code
> > 
> >   - It provides a CPU (and architecture) independent allocator, which
> > may be useful for some systems where the CPU is using a different
> > table format for its own mappings
> > 
> > As illustrated in the final patch, an IOMMU driver interacts with the
> > allocator by passing in a configuration structure describing the
> > input and output address ranges, the supported pages sizes and a set of
> > ops for performing various TLB invalidation and PTE flushing routines.
> > 
> > The LPAE code implements support for 4k/2M/1G, 16k/32M and 64k/512M
> > mappings, but I decided not to implement the contiguous bit in the
> > interest of trying to keep the code semi-readable. This could always be
> > added later, if needed.
> 
> Do you have any idea how much the contiguous bit can improve performances in 
> real use cases ?

It depends on the TLB, really. Given that the contiguous sized map directly
onto block sizes using different granules, I didn't see that the complexity
was worth it.

For example:

   4k granule : 16 contiguous entries => {64k, 32M, 16G}
  16k granule : 128 contiguous lvl3 entries => 2M
32 contiguous lvl2 entries => 1G
  64k granule : 32 contiguous entries => {2M, 16G}

If we use block mappings, then we get:

   4k granule : 2M @ lvl2, 1G @ lvl1
  16k granule : 32M @ lvl2
  64k granule : 512M @ lvl2

so really, we only miss the ability to create 16G mappings. I doubt
that hardware even implements that size in the TLB (the contiguous bit
is only a hint).

On top of that, the contiguous bit leads to additional expense on unmap,
since you have extra TLB invalidation splitting the thing into
non-contiguous pages before you can do anything.

Will
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH 1/4] iommu: introduce generic page table allocation framework

2014-12-01 Thread Will Deacon
On Sun, Nov 30, 2014 at 10:00:21PM +, Laurent Pinchart wrote:
> Hi Will,

Hi Laurent,

> Thank you for the patch.

Cheers for the review!

> On Thursday 27 November 2014 11:51:15 Will Deacon wrote:
> > diff --git a/drivers/iommu/io-pgtable.c b/drivers/iommu/io-pgtable.c
> > new file mode 100644
> > index ..82e39a0db94b
> > --- /dev/null
> > +++ b/drivers/iommu/io-pgtable.c
> > @@ -0,0 +1,71 @@
> > +/*
> > + * Generic page table allocator for IOMMUs.
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > + * GNU General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU General Public License
> > + * along with this program; if not, write to the Free Software
> > + * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307,
> > USA. + *
> > + * Copyright (C) 2014 ARM Limited
> > + *
> > + * Author: Will Deacon 
> > + */
> > +
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#include "io-pgtable.h"
> > +
> > +static struct io_pgtable_init_fns
> 
> Any reason not to make the table const ?

No reason, I'll give it a go.

> > diff --git a/drivers/iommu/io-pgtable.h b/drivers/iommu/io-pgtable.h
> > new file mode 100644
> > index ..5ae75d9cae50
> > --- /dev/null
> > +++ b/drivers/iommu/io-pgtable.h
> > @@ -0,0 +1,65 @@
> > +#ifndef __IO_PGTABLE_H
> > +#define __IO_PGTABLE_H
> > +
> > +struct io_pgtable_ops {
> > +   int (*map)(struct io_pgtable_ops *ops, unsigned long iova,
> 
> How about passing a struct io_pgtable * instead of the ops pointer ? This 
> would require returning a struct io_pgtable from the alloc function, which I 
> suppose you didn't want to do to ensure the caller will not touch the struct 
> io_pgtable fields directly. Do we really need to go that far, or can we 
> simply 
> document struct io_pgtable as being private to the pg alloc framework core 
> and 
> allocators ? Someone who really wants to get hold of the io_pgtable instance 
> could use container_of on the ops anyway, like the allocators do.

Hmm, currently the struct io_pgtable is private to the page table allocator,
so I don't like the IOMMU driver having an explicit handle to that. I also
like having the value returned from alloc_io_pgtable_ops being used as the
handle to pass around -- it keeps things simple for the caller because
there's one structure that you get back and that's the thing you use as a
reference.

What do we gain by returning the struct io_pgtable pointer instead?

> > +  phys_addr_t paddr, size_t size, int prot);
> > +   int (*unmap)(struct io_pgtable_ops *ops, unsigned long iova,
> > +size_t size);
> > +   phys_addr_t (*iova_to_phys)(struct io_pgtable_ops *ops,
> > +   unsigned long iova);
> > +};
> > +
> > +struct iommu_gather_ops {
> > +   /* Synchronously invalidate the entire TLB context */
> > +   void (*tlb_flush_all)(void *cookie);
> > +
> > +   /* Queue up a TLB invalidation for a virtual address range */
> > +   void (*tlb_add_flush)(unsigned long iova, size_t size, bool leaf,
> > + void *cookie);
> 
> Is there a limit to the number of entries that can be queued, or any other 
> kind of restriction ? Implementing a completely generic TLB flush queue can 
> become complex for IOMMU drivers.

I think it's only as complicated as you decide to make it. For example, 
you could just issue the TLBI directly in the add_flush callback (like I
do for the arm-smmu driver), but then don't bother polling the hardware
for completion until the sync callback.

> I would also document in which context(s) this callback will be called, as 
> IOMMU drivers might be tempted to allocate memory in order to implement a TLB 
> flush queue.

Good idea.

> > +   /* Ensure any queued TLB invalidation has taken effect */
> > +   void (*tlb_sync)(void *cookie);
> > +
> > +   /* Ensure page tables updates are visible to the IOMMU */
> > +   void (*flush_pgtable)(void *ptr, size_t size, void *cookie);
> > +};
> 
> I suppose kerneldoc will come in the next version ;-)

Bah, ok then, if you insist!

> > +struct io_pgtable_cfg {
> > +   int quirks; /* IO_PGTABLE_QUIRK_* */
> > +   unsigned long   pgsize_bitmap;
> > +   unsigned intias;
> > +   unsigned intoas;
> > +   struct iommu_gather_ops *tlb;
> > +
> > +   /* Low-level data specific to the table format */
> > +   union {
> > +   };
> > +};
> > +
> > +enum io_pgtable_fmt {
> > +   IO_PGTABLE_NUM_FMTS,
> > +};
> > +
> > +struct io_pgtable {
> > +   enum io_pgtable_fmt fmt;
> > +   void 

Re: [PATCH 0/5] iommu/vt-d: Fix crash dump failure caused by legacy DMA/IO

2014-12-01 Thread Joerg Roedel
On Mon, Dec 01, 2014 at 02:31:38PM +0800, Li, ZhenHua wrote:
> After I implement these two steps, there comes a new fault:
> 
> [1.594890] dmar: DRHD: handling fault status reg 2
> [1.594894] dmar: INTR-REMAP: Request device [[41:00.0] fault index 4d
> [1.594894] INTR-REMAP:[fault reason 34] Present field in the IRTE entry
> is clear
> 
> It is caused by similar reason, so I will fix it like fixing the DMAR
> faults: Do NOT disable and re-enable the interrupt remapping, try to
> use data from old kernel.

Yes, that sounds right, thanks. Looking forward to your patches.


Joerg

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v10 00/20] VFIO support for platform and ARM AMBA devices

2014-12-01 Thread Antonios Motakis
Hello Eric,

On Fri, Nov 28, 2014 at 7:05 PM, Eric Auger  wrote:
> Hi Antonios,
>
> you can add Tested-by: Eric Auger 
>

Thanks!

> I added some patches related to IOMMU_CAP_NOEXEC cap to compile your 2
> series with v3.18-rc5, maybe due to the fact I did not use some
> iommu/vt-d stuff? See my 3.18-rc6-v10 on my kernel git branch.
>

The IOMMU_NOEXEC stuff no lives in Joerg Roedel's iommu/next branch;
see https://git.kernel.org/cgit/linux/kernel/git/joro/iommu.git/log/?h=next

As mentioned in the cover of the first patch series, you will have to
apply the rest of the patches on that, until eventually they are
pulled in mainline. There are also some other dependencies in there as
well for the new type1 for ARM patch series.

Best regards
Antonios

> Best Regards
>
> Eric
>
> On 11/27/2014 06:32 PM, Antonios Motakis wrote:
>> This patch series aims to implement VFIO support for platform devices that
>> reside behind an IOMMU. Examples of such devices are devices behind an ARM
>> SMMU, or behind a Samsung Exynos System MMU.
>>
>> The API used is based on the existing VFIO API that is also used with PCI
>> devices. Only devices that include a basic set of IRQs and memory regions are
>> targeted; devices with complex relationships with other devices on a device
>> tree are not taken into account at this stage.
>>
>> This patch series may be applied on the following series/patches:
>>  - [PATCH] driver core: amba: add device binding path 'driver_override'
>>  - [PATCH v3 0/6] vfio: type1: support for ARM SMMUS with VFIO_IOMMU_TYPE1
>>
>> A copy can be cloned from the branch vfio-platform-v10 at:
>> g...@github.com:virtualopensystems/linux-kvm-arm.git
>>
>> Changes since v9:
>>  - Reworked the splitting of the patches that decouple virqfd from PCI
>>  - Some styling issues and typos
>>  - Removed superfluous includes
>>  - AMBA devices are now named vfio-amba- suffixed by the AMBA device id
>>  - Several other cleanups and fixes
>> Changes since v8:
>>  - Separate irq handler for edge and level triggered interrupts
>>  - Mutex based lock for VFIO fd open/release
>>  - Fixed bug where the first region of a platform device wasn't exposed
>>  - Read only regions can be MMAPed only read only
>>  - Code cleanups
>> Changes since v7:
>>  - Some initial placeholder functionality for PIO resources
>>  - Cleaned up code for IRQ triggering, masking and unmasking
>>  - Some functionality has been removed from this series and posted 
>> separately:
>>- VFIO_IOMMU_TYPE1 support for ARM SMMUs
>>- IOMMU NOEXEC patches
>>- driver_override functionality for AMBA devices
>>  - Several fixes
>> Changes since v6:
>>  - Integrated support for AMBA devices
>>  - Numerous cleanups and fixes
>> Changes since v5:
>>  - Full eventfd support for IRQ masking and unmasking.
>>  - Changed IOMMU_EXEC to IOMMU_NOEXEC, along with related flags in VFIO.
>>  - Other fixes based on reviewer comments.
>> Changes since v4:
>>  - Use static offsets for each region in the VFIO device fd
>>  - Include patch in the series for the ARM SMMU to expose IOMMU_EXEC
>>availability via IOMMU_CAP_DMA_EXEC
>>  - Rebased on VFIO multi domain support:
>>- IOMMU_EXEC is now available if at least one IOMMU in the container
>>  supports it
>>- Expose IOMMU_EXEC if available via the capability VFIO_IOMMU_PROT_EXEC
>>  - Some bug fixes
>> Changes since v3:
>>  - Use Kim Phillips' driver_probe_device()
>> Changes since v2:
>>  - Fixed Read/Write and MMAP on device regions
>>  - Removed dependency on Device Tree
>>  - Interrupts support
>>  - Interrupt masking/unmasking
>>  - Automask level sensitive interrupts
>>  - Introduced VFIO_DMA_MAP_FLAG_EXEC
>>  - Code clean ups
>>
>> Antonios Motakis (20):
>>   vfio/platform: initial skeleton of VFIO support for platform devices
>>   vfio: platform: probe to devices on the platform bus
>>   vfio: platform: add the VFIO PLATFORM module to Kconfig
>>   vfio: amba: VFIO support for AMBA devices
>>   vfio: amba: add the VFIO for AMBA devices module to Kconfig
>>   vfio/platform: return info for bound device
>>   vfio/platform: return info for device memory mapped IO regions
>>   vfio/platform: read and write support for the device fd
>>   vfio/platform: support MMAP of MMIO regions
>>   vfio/platform: return IRQ info
>>   vfio/platform: initial interrupts support code
>>   vfio/platform: trigger an interrupt via eventfd
>>   vfio/platform: support for level sensitive interrupts
>>   vfio: add a vfio_ prefix to virqfd_enable and virqfd_disable and
>> export
>>   vfio: virqfd: rename vfio_pci_virqfd_init and vfio_pci_virqfd_exit
>>   vfio: add local lock for virqfd instead of depending on VFIO PCI
>>   vfio: pass an opaque pointer on virqfd initialization
>>   vfio: move eventfd support code for VFIO_PCI to a separate file
>>   vfio: initialize the virqfd workqueue in VFIO generic code
>>   vfio/platform: implement IRQ masking/unmasking via an eventfd
>>
>>  drivers/vfio/Kconfig

Re: [PATCH 1/4] iommu: introduce generic page table allocation framework

2014-12-01 Thread Laurent Pinchart
Hi Will,

On Monday 01 December 2014 12:13:38 Will Deacon wrote:
> On Sun, Nov 30, 2014 at 10:00:21PM +, Laurent Pinchart wrote:
> > On Thursday 27 November 2014 11:51:15 Will Deacon wrote:
> > > diff --git a/drivers/iommu/io-pgtable.c b/drivers/iommu/io-pgtable.c
> > > new file mode 100644
> > > index ..82e39a0db94b
> > > --- /dev/null
> > > +++ b/drivers/iommu/io-pgtable.c
> > > @@ -0,0 +1,71 @@
> > > +/*
> > > + * Generic page table allocator for IOMMUs.
> > > + *
> > > + * This program is free software; you can redistribute it and/or modify
> > > + * it under the terms of the GNU General Public License version 2 as
> > > + * published by the Free Software Foundation.
> > > + *
> > > + * This program is distributed in the hope that it will be useful,
> > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > > + * GNU General Public License for more details.
> > > + *
> > > + * You should have received a copy of the GNU General Public License
> > > + * along with this program; if not, write to the Free Software
> > > + * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA
> > > 02111-1307, USA.

By the way you can remove this paragraph, we don't want to update all source 
files the day the FSF decides to move to a new address.

> > > + *
> > > + * Copyright (C) 2014 ARM Limited
> > > + *
> > > + * Author: Will Deacon 
> > > + */
> > > +
> > > +#include 
> > > +#include 
> > > +#include 
> > > +
> > > +#include "io-pgtable.h"
> > > +
> > > +static struct io_pgtable_init_fns
> > 
> > Any reason not to make the table const ?
> 
> No reason, I'll give it a go.
> 
> > > diff --git a/drivers/iommu/io-pgtable.h b/drivers/iommu/io-pgtable.h
> > > new file mode 100644
> > > index ..5ae75d9cae50
> > > --- /dev/null
> > > +++ b/drivers/iommu/io-pgtable.h
> > > @@ -0,0 +1,65 @@
> > > +#ifndef __IO_PGTABLE_H
> > > +#define __IO_PGTABLE_H
> > > +
> > > +struct io_pgtable_ops {
> > > + int (*map)(struct io_pgtable_ops *ops, unsigned long iova,
> > 
> > How about passing a struct io_pgtable * instead of the ops pointer ? This
> > would require returning a struct io_pgtable from the alloc function, which
> > I suppose you didn't want to do to ensure the caller will not touch the
> > struct io_pgtable fields directly. Do we really need to go that far, or
> > can we simply document struct io_pgtable as being private to the pg alloc
> > framework core and allocators ? Someone who really wants to get hold of
> > the io_pgtable instance could use container_of on the ops anyway, like
> > the allocators do.
> 
> Hmm, currently the struct io_pgtable is private to the page table allocator,
> so I don't like the IOMMU driver having an explicit handle to that.

I agree with this, but given that struct io_pgtable is defined in a header 
used by the IOMMU driver, and given that it directly embeds struct 
io_pgtable_ops, there's no big difference between the two structures.

> I also like having the value returned from alloc_io_pgtable_ops being used
> as the handle to pass around -- it keeps things simple for the caller
> because there's one structure that you get back and that's the thing you use
> as a reference.

I agree with that as well, my proposal was to return a struct io_pgtable from 
alloc_io_pgtable_ops.

> What do we gain by returning the struct io_pgtable pointer instead?

The ops structure could be made a const pointer. That's a pretty small 
optimization, granted.

> > > +phys_addr_t paddr, size_t size, int prot);
> > > + int (*unmap)(struct io_pgtable_ops *ops, unsigned long iova,
> > > +  size_t size);
> > > + phys_addr_t (*iova_to_phys)(struct io_pgtable_ops *ops,
> > > + unsigned long iova);
> > > +};
> > > +
> > > +struct iommu_gather_ops {
> > > + /* Synchronously invalidate the entire TLB context */
> > > + void (*tlb_flush_all)(void *cookie);
> > > +
> > > + /* Queue up a TLB invalidation for a virtual address range */
> > > + void (*tlb_add_flush)(unsigned long iova, size_t size, bool leaf,
> > > +   void *cookie);
> > 
> > Is there a limit to the number of entries that can be queued, or any other
> > kind of restriction ? Implementing a completely generic TLB flush queue
> > can become complex for IOMMU drivers.
> 
> I think it's only as complicated as you decide to make it. For example,
> you could just issue the TLBI directly in the add_flush callback (like I
> do for the arm-smmu driver), but then don't bother polling the hardware
> for completion until the sync callback.
> 
> > I would also document in which context(s) this callback will be called, as
> > IOMMU drivers might be tempted to allocate memory in order to implement a
> > TLB flush queue.
> 
> Good idea.
> 
> > > + /* Ensure any queued TLB invalidation has taken effect */
> > > + void (*tlb_sync)(void *cookie);
> > > +
> > > + /* Ensure page tables upd

Re: [PATCH 1/4] iommu: introduce generic page table allocation framework

2014-12-01 Thread Will Deacon
On Mon, Dec 01, 2014 at 01:33:09PM +, Laurent Pinchart wrote:
> On Monday 01 December 2014 12:13:38 Will Deacon wrote:
> > On Sun, Nov 30, 2014 at 10:00:21PM +, Laurent Pinchart wrote:
> > > On Thursday 27 November 2014 11:51:15 Will Deacon wrote:
> > > > diff --git a/drivers/iommu/io-pgtable.c b/drivers/iommu/io-pgtable.c
> > > > new file mode 100644
> > > > index ..82e39a0db94b
> > > > --- /dev/null
> > > > +++ b/drivers/iommu/io-pgtable.c
> > > > @@ -0,0 +1,71 @@
> > > > +/*
> > > > + * Generic page table allocator for IOMMUs.
> > > > + *
> > > > + * This program is free software; you can redistribute it and/or modify
> > > > + * it under the terms of the GNU General Public License version 2 as
> > > > + * published by the Free Software Foundation.
> > > > + *
> > > > + * This program is distributed in the hope that it will be useful,
> > > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > > > + * GNU General Public License for more details.
> > > > + *
> > > > + * You should have received a copy of the GNU General Public License
> > > > + * along with this program; if not, write to the Free Software
> > > > + * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA
> > > > 02111-1307, USA.
> 
> By the way you can remove this paragraph, we don't want to update all source 
> files the day the FSF decides to move to a new address.

Yeah, I missed that one (I fixed the lpae file already).

> > > > diff --git a/drivers/iommu/io-pgtable.h b/drivers/iommu/io-pgtable.h
> > > > new file mode 100644
> > > > index ..5ae75d9cae50
> > > > --- /dev/null
> > > > +++ b/drivers/iommu/io-pgtable.h
> > > > @@ -0,0 +1,65 @@
> > > > +#ifndef __IO_PGTABLE_H
> > > > +#define __IO_PGTABLE_H
> > > > +
> > > > +struct io_pgtable_ops {
> > > > +   int (*map)(struct io_pgtable_ops *ops, unsigned long iova,
> > > 
> > > How about passing a struct io_pgtable * instead of the ops pointer ? This
> > > would require returning a struct io_pgtable from the alloc function, which
> > > I suppose you didn't want to do to ensure the caller will not touch the
> > > struct io_pgtable fields directly. Do we really need to go that far, or
> > > can we simply document struct io_pgtable as being private to the pg alloc
> > > framework core and allocators ? Someone who really wants to get hold of
> > > the io_pgtable instance could use container_of on the ops anyway, like
> > > the allocators do.
> > 
> > Hmm, currently the struct io_pgtable is private to the page table allocator,
> > so I don't like the IOMMU driver having an explicit handle to that.
> 
> I agree with this, but given that struct io_pgtable is defined in a header 
> used by the IOMMU driver, and given that it directly embeds struct 
> io_pgtable_ops, there's no big difference between the two structures.

Right, but you have to do an explicit container_of and, with the kerneldoc
added, it should be clear that it's not a good idea to mess with things
like the cookie or the cfg after you've allocated the page tables.

> > I also like having the value returned from alloc_io_pgtable_ops being used
> > as the handle to pass around -- it keeps things simple for the caller
> > because there's one structure that you get back and that's the thing you use
> > as a reference.
> 
> I agree with that as well, my proposal was to return a struct io_pgtable from 
> alloc_io_pgtable_ops.
> 
> > What do we gain by returning the struct io_pgtable pointer instead?
> 
> The ops structure could be made a const pointer. That's a pretty small 
> optimization, granted.

I still think I'd rather keep things like they are. Let's see how it looks
in v2, when I've reordered the structures and documented them.

Will
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v5 0/8] Introduce automatic DMA configuration for IOMMU masters

2014-12-01 Thread Arnd Bergmann
On Friday 28 November 2014 13:29:38 Will Deacon wrote:
> Hi all,
> 
> Here is v5 of the patches I've previously sent here:
> 
>   RFCv1: 
> http://lists.infradead.org/pipermail/linux-arm-kernel/2014-August/283023.html
>   RFCv2: 
> http://lists.infradead.org/pipermail/linux-arm-kernel/2014-September/283752.html
>   RFCv3: 
> http://lists.infradead.org/pipermail/linux-arm-kernel/2014-September/287031.html
>   RFCv4: 
> http://lists.infradead.org/pipermail/linux-arm-kernel/2014-November/302711.html
> 
> Changes since RFCv4 include:
> 
>   - Dropped the RFC tag, since has been used by a couple of people now
> 
>   - Dropped the DMA segment configuration from of_dma_configure, as there
> appear to be assumptions about 64k segments elsewhere in the kernel
> 
>   - Added acks/tested-bys (thanks to everybody who reviewed the series)
> 
>   - A few small fixes for issues found by Marek
> 
> Arnd: Is this too late for 3.19? We could merge the first 6 patches
> with no issues, since there aren't any callers of of_iommu_init
> without patch 7 anyway.
> 
> Up to you.

I think this looks great overall. My only feedback is the exact same
comment that Joerg already made:

On Friday 28 November 2014 14:03:36 jroe...@suse.de wrote:
> Hmm, I don't like the idea of storing private data in iommu_ops. But
> given that this is already an improvement we can build on later, here is
> my
> 
> Acked-by: Joerg Roedel 
> 
> To further improve this we should probably introduce a seperate
> iommu-descriptor data-structure later which then describes a single
> hardware iommu device.

so I second that and add my

Acked-by: Arnd Bergmann 

Now, who should merge this series? I think someone should put all eight
patches into linux-next now, and if something goes wrong with the last
two, then we skip them for 3.19.

Arnd
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v10 00/20] VFIO support for platform and ARM AMBA devices

2014-12-01 Thread Eric Auger
On 12/01/2014 01:45 PM, Antonios Motakis wrote:
> Hello Eric,
> 
> On Fri, Nov 28, 2014 at 7:05 PM, Eric Auger  wrote:
>> Hi Antonios,
>>
>> you can add Tested-by: Eric Auger 
>>
> 
> Thanks!
> 
>> I added some patches related to IOMMU_CAP_NOEXEC cap to compile your 2
>> series with v3.18-rc5, maybe due to the fact I did not use some
>> iommu/vt-d stuff? See my 3.18-rc6-v10 on my kernel git branch.
>>
> 
> The IOMMU_NOEXEC stuff no lives in Joerg Roedel's iommu/next branch;
> see https://git.kernel.org/cgit/linux/kernel/git/joro/iommu.git/log/?h=next
> 
> As mentioned in the cover of the first patch series, you will have to
> apply the rest of the patches on that, until eventually they are
> pulled in mainline. There are also some other dependencies in there as
> well for the new type1 for ARM patch series.

ok thanks,

Eric
> 
> Best regards
> Antonios
> 
>> Best Regards
>>
>> Eric
>>
>> On 11/27/2014 06:32 PM, Antonios Motakis wrote:
>>> This patch series aims to implement VFIO support for platform devices that
>>> reside behind an IOMMU. Examples of such devices are devices behind an ARM
>>> SMMU, or behind a Samsung Exynos System MMU.
>>>
>>> The API used is based on the existing VFIO API that is also used with PCI
>>> devices. Only devices that include a basic set of IRQs and memory regions 
>>> are
>>> targeted; devices with complex relationships with other devices on a device
>>> tree are not taken into account at this stage.
>>>
>>> This patch series may be applied on the following series/patches:
>>>  - [PATCH] driver core: amba: add device binding path 'driver_override'
>>>  - [PATCH v3 0/6] vfio: type1: support for ARM SMMUS with VFIO_IOMMU_TYPE1
>>>
>>> A copy can be cloned from the branch vfio-platform-v10 at:
>>> g...@github.com:virtualopensystems/linux-kvm-arm.git
>>>
>>> Changes since v9:
>>>  - Reworked the splitting of the patches that decouple virqfd from PCI
>>>  - Some styling issues and typos
>>>  - Removed superfluous includes
>>>  - AMBA devices are now named vfio-amba- suffixed by the AMBA device id
>>>  - Several other cleanups and fixes
>>> Changes since v8:
>>>  - Separate irq handler for edge and level triggered interrupts
>>>  - Mutex based lock for VFIO fd open/release
>>>  - Fixed bug where the first region of a platform device wasn't exposed
>>>  - Read only regions can be MMAPed only read only
>>>  - Code cleanups
>>> Changes since v7:
>>>  - Some initial placeholder functionality for PIO resources
>>>  - Cleaned up code for IRQ triggering, masking and unmasking
>>>  - Some functionality has been removed from this series and posted 
>>> separately:
>>>- VFIO_IOMMU_TYPE1 support for ARM SMMUs
>>>- IOMMU NOEXEC patches
>>>- driver_override functionality for AMBA devices
>>>  - Several fixes
>>> Changes since v6:
>>>  - Integrated support for AMBA devices
>>>  - Numerous cleanups and fixes
>>> Changes since v5:
>>>  - Full eventfd support for IRQ masking and unmasking.
>>>  - Changed IOMMU_EXEC to IOMMU_NOEXEC, along with related flags in VFIO.
>>>  - Other fixes based on reviewer comments.
>>> Changes since v4:
>>>  - Use static offsets for each region in the VFIO device fd
>>>  - Include patch in the series for the ARM SMMU to expose IOMMU_EXEC
>>>availability via IOMMU_CAP_DMA_EXEC
>>>  - Rebased on VFIO multi domain support:
>>>- IOMMU_EXEC is now available if at least one IOMMU in the container
>>>  supports it
>>>- Expose IOMMU_EXEC if available via the capability VFIO_IOMMU_PROT_EXEC
>>>  - Some bug fixes
>>> Changes since v3:
>>>  - Use Kim Phillips' driver_probe_device()
>>> Changes since v2:
>>>  - Fixed Read/Write and MMAP on device regions
>>>  - Removed dependency on Device Tree
>>>  - Interrupts support
>>>  - Interrupt masking/unmasking
>>>  - Automask level sensitive interrupts
>>>  - Introduced VFIO_DMA_MAP_FLAG_EXEC
>>>  - Code clean ups
>>>
>>> Antonios Motakis (20):
>>>   vfio/platform: initial skeleton of VFIO support for platform devices
>>>   vfio: platform: probe to devices on the platform bus
>>>   vfio: platform: add the VFIO PLATFORM module to Kconfig
>>>   vfio: amba: VFIO support for AMBA devices
>>>   vfio: amba: add the VFIO for AMBA devices module to Kconfig
>>>   vfio/platform: return info for bound device
>>>   vfio/platform: return info for device memory mapped IO regions
>>>   vfio/platform: read and write support for the device fd
>>>   vfio/platform: support MMAP of MMIO regions
>>>   vfio/platform: return IRQ info
>>>   vfio/platform: initial interrupts support code
>>>   vfio/platform: trigger an interrupt via eventfd
>>>   vfio/platform: support for level sensitive interrupts
>>>   vfio: add a vfio_ prefix to virqfd_enable and virqfd_disable and
>>> export
>>>   vfio: virqfd: rename vfio_pci_virqfd_init and vfio_pci_virqfd_exit
>>>   vfio: add local lock for virqfd instead of depending on VFIO PCI
>>>   vfio: pass an opaque pointer on virqfd initialization
>>>   vfio: move eventfd support code for VFIO_P

Re: [PATCH v5 0/8] Introduce automatic DMA configuration for IOMMU masters

2014-12-01 Thread Will Deacon
On Mon, Dec 01, 2014 at 01:52:57PM +, Arnd Bergmann wrote:
> On Friday 28 November 2014 13:29:38 Will Deacon wrote:
> > Here is v5 of the patches I've previously sent here:
> > 
> >   RFCv1: 
> > http://lists.infradead.org/pipermail/linux-arm-kernel/2014-August/283023.html
> >   RFCv2: 
> > http://lists.infradead.org/pipermail/linux-arm-kernel/2014-September/283752.html
> >   RFCv3: 
> > http://lists.infradead.org/pipermail/linux-arm-kernel/2014-September/287031.html
> >   RFCv4: 
> > http://lists.infradead.org/pipermail/linux-arm-kernel/2014-November/302711.html
> > 
> > Changes since RFCv4 include:
> > 
> >   - Dropped the RFC tag, since has been used by a couple of people now
> > 
> >   - Dropped the DMA segment configuration from of_dma_configure, as there
> > appear to be assumptions about 64k segments elsewhere in the kernel
> > 
> >   - Added acks/tested-bys (thanks to everybody who reviewed the series)
> > 
> >   - A few small fixes for issues found by Marek
> > 
> > Arnd: Is this too late for 3.19? We could merge the first 6 patches
> > with no issues, since there aren't any callers of of_iommu_init
> > without patch 7 anyway.
> > 
> > Up to you.
> 
> I think this looks great overall. My only feedback is the exact same
> comment that Joerg already made:
> 
> On Friday 28 November 2014 14:03:36 jroe...@suse.de wrote:
> > Hmm, I don't like the idea of storing private data in iommu_ops. But
> > given that this is already an improvement we can build on later, here is
> > my
> > 
> > Acked-by: Joerg Roedel 
> > 
> > To further improve this we should probably introduce a seperate
> > iommu-descriptor data-structure later which then describes a single
> > hardware iommu device.
> 
> so I second that and add my
> 
> Acked-by: Arnd Bergmann 

Yup, I'll take a look at that as part of a separate series, since I really
want to have different pgsize_bitmaps anyway.

> Now, who should merge this series? I think someone should put all eight
> patches into linux-next now, and if something goes wrong with the last
> two, then we skip them for 3.19.

I think it makes most sense to go via arm-soc, but we'd need rmk's ack
on the last two patches.

Russell, are you ok with that plan?

Will
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH] iommu/vt-d: Fix an off-by-one bug in __domain_mapping()

2014-12-01 Thread Joerg Roedel
On Wed, Nov 26, 2014 at 09:42:10AM +0800, Jiang Liu wrote:
> There's an off-by-one bug in function __domain_mapping(), which may
> trigger the BUG_ON(nr_pages < lvl_pages) when
>   (nr_pages + 1) & superpage_mask == 0

What is the superpage_mask?

> The issue was introduced by commit 9051aa0268dc "intel-iommu: Combine
> domain_pfn_mapping() and domain_sg_mapping()", which sets sg_res to
> "nr_pages + 1" to avoid some of the 'sg_res==0' code paths.
> 
> It's safe to remove extra "+1" because sg_res is only used to calculate
> page size now.

>From your description and the (hard to read) code in __domain_mapping I
don't really understand the issue yet. Can you please elaborate on this
issue can be triggered?

Is the BUG_ON the only issue and, if yes, can that be fixed by just
changing the BUG_ON condition?

>   This issue was introduced in v2.6.31, but intel-iommu.c has
> been moved into drivers/iommu in v3.1. So what's the preferred way
> to deal with stable kernels between v2.6.31 and v3.1?

Just remove the kernel version marker from the stable tag. The stable
kernel maintainers for kernels >3.1 will ask you to backport the patch
or just backport it by themselfes.


Joerg

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


[PATCH v6 0/8] Introduce automatic DMA configuration for IOMMU masters

2014-12-01 Thread Will Deacon
Hello again,

This is version 6 of the patches previously posted here:

  RFCv1: 
http://lists.infradead.org/pipermail/linux-arm-kernel/2014-August/283023.html
  RFCv2: 
http://lists.infradead.org/pipermail/linux-arm-kernel/2014-September/283752.html
  RFCv3: 
http://lists.infradead.org/pipermail/linux-arm-kernel/2014-September/287031.html
  RFCv4: 
http://lists.infradead.org/pipermail/linux-arm-kernel/2014-November/302711.html
 v5: 
http://lists.infradead.org/pipermail/linux-arm-kernel/2014-November/307213.html

The only change since v5 is the addition of acks from various maintainers.

Now that the ARM bits have rmk's ack and the IOMMU bits have joro's ack,
I think this is good for merging via the arm-soc tree.

Please let me know if a pull request would be preferable.

Cheers,

Will

--->8

Marek Szyprowski (1):
  iommu: fix initialization without 'add_device' callback

Will Deacon (7):
  iommu: provide early initialisation hook for IOMMU drivers
  dma-mapping: replace set_arch_dma_coherent_ops with arch_setup_dma_ops
  iommu: add new iommu_ops callback for adding an OF device
  iommu: provide helper function to configure an IOMMU for an of master
  dma-mapping: detect and configure IOMMU in of_dma_configure
  arm: call iommu_init before of_platform_populate
  arm: dma-mapping: plumb our iommu mapping ops into arch_setup_dma_ops

 arch/arm/include/asm/dma-mapping.h | 12 +++---
 arch/arm/kernel/setup.c|  2 +
 arch/arm/mm/dma-mapping.c  | 83 ++
 drivers/iommu/Kconfig  |  2 +-
 drivers/iommu/iommu.c  |  2 +-
 drivers/iommu/of_iommu.c   | 50 +++
 drivers/of/platform.c  | 50 ---
 include/asm-generic/vmlinux.lds.h  |  2 +
 include/linux/dma-mapping.h| 13 +++---
 include/linux/iommu.h  |  8 
 include/linux/of_iommu.h   | 31 ++
 11 files changed, 211 insertions(+), 44 deletions(-)

-- 
2.1.1

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


[PATCH v6 5/8] iommu: fix initialization without 'add_device' callback

2014-12-01 Thread Will Deacon
From: Marek Szyprowski 

IOMMU drivers can be initialized from of_iommu helpers. Such drivers don't
need to provide device_add callbacks to operate properly, so there is no
need to fail initialization if the callback is missing.

Acked-by: Arnd Bergmann 
Signed-off-by: Marek Szyprowski 
Signed-off-by: Will Deacon 
---
 drivers/iommu/iommu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index ed8b04867b1f..02f798b7e295 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -737,7 +737,7 @@ static int add_iommu_group(struct device *dev, void *data)
const struct iommu_ops *ops = cb->ops;
 
if (!ops->add_device)
-   return -ENODEV;
+   return 0;
 
WARN_ON(dev->iommu_group);
 
-- 
2.1.1

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


[PATCH v6 4/8] iommu: provide helper function to configure an IOMMU for an of master

2014-12-01 Thread Will Deacon
The generic IOMMU device-tree bindings can be used to add arbitrary OF
masters to an IOMMU with a compliant binding.

This patch introduces of_iommu_configure, which does exactly that.

Acked-by: Arnd Bergmann 
Acked-by: Joerg Roedel 
Acked-by: Marek Szyprowski 
Tested-by: Robin Murphy 
Signed-off-by: Will Deacon 
---
 drivers/iommu/Kconfig|  2 +-
 drivers/iommu/of_iommu.c | 33 +
 include/linux/of_iommu.h |  6 ++
 3 files changed, 40 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index dd5112265cc9..6d13f962f156 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -15,7 +15,7 @@ if IOMMU_SUPPORT
 
 config OF_IOMMU
def_bool y
-   depends on OF
+   depends on OF && IOMMU_API
 
 config FSL_PAMU
bool "Freescale IOMMU support"
diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
index 89b903406968..73236d3cc955 100644
--- a/drivers/iommu/of_iommu.c
+++ b/drivers/iommu/of_iommu.c
@@ -18,6 +18,7 @@
  */
 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -93,6 +94,38 @@ int of_get_dma_window(struct device_node *dn, const char 
*prefix, int index,
 }
 EXPORT_SYMBOL_GPL(of_get_dma_window);
 
+struct iommu_ops *of_iommu_configure(struct device *dev)
+{
+   struct of_phandle_args iommu_spec;
+   struct device_node *np;
+   struct iommu_ops *ops = NULL;
+   int idx = 0;
+
+   /*
+* We don't currently walk up the tree looking for a parent IOMMU.
+* See the `Notes:' section of
+* Documentation/devicetree/bindings/iommu/iommu.txt
+*/
+   while (!of_parse_phandle_with_args(dev->of_node, "iommus",
+  "#iommu-cells", idx,
+  &iommu_spec)) {
+   np = iommu_spec.np;
+   ops = of_iommu_get_ops(np);
+
+   if (!ops || !ops->of_xlate || ops->of_xlate(dev, &iommu_spec))
+   goto err_put_node;
+
+   of_node_put(np);
+   idx++;
+   }
+
+   return ops;
+
+err_put_node:
+   of_node_put(np);
+   return NULL;
+}
+
 void __init of_iommu_init(void)
 {
struct device_node *np;
diff --git a/include/linux/of_iommu.h b/include/linux/of_iommu.h
index 5762cdc8effe..d03abbb11c34 100644
--- a/include/linux/of_iommu.h
+++ b/include/linux/of_iommu.h
@@ -1,6 +1,7 @@
 #ifndef __OF_IOMMU_H
 #define __OF_IOMMU_H
 
+#include 
 #include 
 #include 
 
@@ -11,6 +12,7 @@ extern int of_get_dma_window(struct device_node *dn, const 
char *prefix,
 size_t *size);
 
 extern void of_iommu_init(void);
+extern struct iommu_ops *of_iommu_configure(struct device *dev);
 
 #else
 
@@ -22,6 +24,10 @@ static inline int of_get_dma_window(struct device_node *dn, 
const char *prefix,
 }
 
 static inline void of_iommu_init(void) { }
+static inline struct iommu_ops *of_iommu_configure(struct device *dev)
+{
+   return NULL;
+}
 
 #endif /* CONFIG_OF_IOMMU */
 
-- 
2.1.1

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


[PATCH v6 7/8] arm: call iommu_init before of_platform_populate

2014-12-01 Thread Will Deacon
We need to ensure that the IOMMUs in the system have a chance to perform
some basic initialisation before we start adding masters to them.

This patch adds a call to of_iommu_init before of_platform_populate.

Acked-by: Russell King 
Acked-by: Arnd Bergmann 
Acked-by: Marek Szyprowski 
Signed-off-by: Will Deacon 
---
 arch/arm/kernel/setup.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
index c03106378b49..1cfc94aa7fa2 100644
--- a/arch/arm/kernel/setup.c
+++ b/arch/arm/kernel/setup.c
@@ -18,6 +18,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -806,6 +807,7 @@ static int __init customize_machine(void)
 * machine from the device tree, if no callback is provided,
 * otherwise we would always need an init_machine callback.
 */
+   of_iommu_init();
if (machine_desc->init_machine)
machine_desc->init_machine();
 #ifdef CONFIG_OF
-- 
2.1.1

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


[PATCH v6 8/8] arm: dma-mapping: plumb our iommu mapping ops into arch_setup_dma_ops

2014-12-01 Thread Will Deacon
This patch plumbs the existing ARM IOMMU DMA infrastructure (which isn't
actually called outside of a few drivers) into arch_setup_dma_ops, so
that we can use IOMMUs for DMA transfers in a more generic fashion.

Since this significantly complicates the arch_setup_dma_ops function,
it is moved out of line into dma-mapping.c. If CONFIG_ARM_DMA_USE_IOMMU
is not set, the iommu parameter is ignored and the normal ops are used
instead.

Acked-by: Russell King 
Acked-by: Arnd Bergmann 
Acked-by: Marek Szyprowski 
Signed-off-by: Will Deacon 
---
 arch/arm/include/asm/dma-mapping.h | 12 +++---
 arch/arm/mm/dma-mapping.c  | 83 ++
 2 files changed, 81 insertions(+), 14 deletions(-)

diff --git a/arch/arm/include/asm/dma-mapping.h 
b/arch/arm/include/asm/dma-mapping.h
index f3c0d953f6a2..9410b7e548fc 100644
--- a/arch/arm/include/asm/dma-mapping.h
+++ b/arch/arm/include/asm/dma-mapping.h
@@ -121,14 +121,12 @@ static inline unsigned long dma_max_pfn(struct device 
*dev)
 }
 #define dma_max_pfn(dev) dma_max_pfn(dev)
 
-static inline void arch_setup_dma_ops(struct device *dev, u64 dma_base,
- u64 size, struct iommu_ops *iommu,
- bool coherent)
-{
-   if (coherent)
-   set_dma_ops(dev, &arm_coherent_dma_ops);
-}
 #define arch_setup_dma_ops arch_setup_dma_ops
+extern void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size,
+  struct iommu_ops *iommu, bool coherent);
+
+#define arch_teardown_dma_ops arch_teardown_dma_ops
+extern void arch_teardown_dma_ops(struct device *dev);
 
 static inline dma_addr_t phys_to_dma(struct device *dev, phys_addr_t paddr)
 {
diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index e8907117861e..09645f00bd17 100644
--- a/arch/arm/mm/dma-mapping.c
+++ b/arch/arm/mm/dma-mapping.c
@@ -1947,9 +1947,8 @@ EXPORT_SYMBOL_GPL(arm_iommu_release_mapping);
  * arm_iommu_create_mapping)
  *
  * Attaches specified io address space mapping to the provided device,
- * this replaces the dma operations (dma_map_ops pointer) with the
- * IOMMU aware version. More than one client might be attached to
- * the same io address space mapping.
+ * More than one client might be attached to the same io address space
+ * mapping.
  */
 int arm_iommu_attach_device(struct device *dev,
struct dma_iommu_mapping *mapping)
@@ -1962,7 +1961,6 @@ int arm_iommu_attach_device(struct device *dev,
 
kref_get(&mapping->kref);
dev->archdata.mapping = mapping;
-   set_dma_ops(dev, &iommu_ops);
 
pr_debug("Attached IOMMU controller to %s device.\n", dev_name(dev));
return 0;
@@ -1974,7 +1972,6 @@ EXPORT_SYMBOL_GPL(arm_iommu_attach_device);
  * @dev: valid struct device pointer
  *
  * Detaches the provided device from a previously attached map.
- * This voids the dma operations (dma_map_ops pointer)
  */
 void arm_iommu_detach_device(struct device *dev)
 {
@@ -1989,10 +1986,82 @@ void arm_iommu_detach_device(struct device *dev)
iommu_detach_device(mapping->domain, dev);
kref_put(&mapping->kref, release_iommu_mapping);
dev->archdata.mapping = NULL;
-   set_dma_ops(dev, NULL);
 
pr_debug("Detached IOMMU controller from %s device.\n", dev_name(dev));
 }
 EXPORT_SYMBOL_GPL(arm_iommu_detach_device);
 
-#endif
+static struct dma_map_ops *arm_get_iommu_dma_map_ops(bool coherent)
+{
+   return coherent ? &iommu_coherent_ops : &iommu_ops;
+}
+
+static bool arm_setup_iommu_dma_ops(struct device *dev, u64 dma_base, u64 size,
+   struct iommu_ops *iommu)
+{
+   struct dma_iommu_mapping *mapping;
+
+   if (!iommu)
+   return false;
+
+   mapping = arm_iommu_create_mapping(dev->bus, dma_base, size);
+   if (IS_ERR(mapping)) {
+   pr_warn("Failed to create %llu-byte IOMMU mapping for device 
%s\n",
+   size, dev_name(dev));
+   return false;
+   }
+
+   if (arm_iommu_attach_device(dev, mapping)) {
+   pr_warn("Failed to attached device %s to IOMMU_mapping\n",
+   dev_name(dev));
+   arm_iommu_release_mapping(mapping);
+   return false;
+   }
+
+   return true;
+}
+
+static void arm_teardown_iommu_dma_ops(struct device *dev)
+{
+   struct dma_iommu_mapping *mapping = dev->archdata.mapping;
+
+   arm_iommu_detach_device(dev);
+   arm_iommu_release_mapping(mapping);
+}
+
+#else
+
+static bool arm_setup_iommu_dma_ops(struct device *dev, u64 dma_base, u64 size,
+   struct iommu_ops *iommu)
+{
+   return false;
+}
+
+static void arm_teardown_iommu_dma_ops(struct device *dev) { }
+
+#define arm_get_iommu_dma_map_ops arm_get_dma_map_ops
+
+#endif /* CONFIG_ARM_DMA_USE_IOMMU */
+
+static struct dma_map_ops *arm_get_dma_map_ops(bool coh

[PATCH v6 2/8] dma-mapping: replace set_arch_dma_coherent_ops with arch_setup_dma_ops

2014-12-01 Thread Will Deacon
set_arch_dma_coherent_ops is called from of_dma_configure in order to
swizzle the architectural dma-mapping functions over to a cache-coherent
implementation. This is currently implemented only for ARM.

In anticipation of re-using this mechanism for IOMMU-backed dma-mapping
ops too, this patch replaces the function with a broader
arch_setup_dma_ops callback which will be extended in future.

Acked-by: Arnd Bergmann 
Acked-by: Marek Szyprowski 
Tested-by: Robin Murphy 
Signed-off-by: Will Deacon 
---
 arch/arm/include/asm/dma-mapping.h |  8 
 drivers/of/platform.c  | 31 +--
 include/linux/dma-mapping.h|  7 ++-
 3 files changed, 19 insertions(+), 27 deletions(-)

diff --git a/arch/arm/include/asm/dma-mapping.h 
b/arch/arm/include/asm/dma-mapping.h
index 85738b200023..dc3420e77758 100644
--- a/arch/arm/include/asm/dma-mapping.h
+++ b/arch/arm/include/asm/dma-mapping.h
@@ -121,12 +121,12 @@ static inline unsigned long dma_max_pfn(struct device 
*dev)
 }
 #define dma_max_pfn(dev) dma_max_pfn(dev)
 
-static inline int set_arch_dma_coherent_ops(struct device *dev)
+static inline void arch_setup_dma_ops(struct device *dev, bool coherent)
 {
-   set_dma_ops(dev, &arm_coherent_dma_ops);
-   return 0;
+   if (coherent)
+   set_dma_ops(dev, &arm_coherent_dma_ops);
 }
-#define set_arch_dma_coherent_ops(dev) set_arch_dma_coherent_ops(dev)
+#define arch_setup_dma_ops arch_setup_dma_ops
 
 static inline dma_addr_t phys_to_dma(struct device *dev, phys_addr_t paddr)
 {
diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index 3b64d0bf5bba..ff1f4e9afccb 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -164,6 +164,8 @@ static void of_dma_configure(struct device *dev)
 {
u64 dma_addr, paddr, size;
int ret;
+   bool coherent;
+   unsigned long offset;
 
/*
 * Set default dma-mask to 32 bit. Drivers are expected to setup
@@ -178,28 +180,21 @@ static void of_dma_configure(struct device *dev)
if (!dev->dma_mask)
dev->dma_mask = &dev->coherent_dma_mask;
 
-   /*
-* if dma-coherent property exist, call arch hook to setup
-* dma coherent operations.
-*/
-   if (of_dma_is_coherent(dev->of_node)) {
-   set_arch_dma_coherent_ops(dev);
-   dev_dbg(dev, "device is dma coherent\n");
-   }
-
-   /*
-* if dma-ranges property doesn't exist - just return else
-* setup the dma offset
-*/
ret = of_dma_get_range(dev->of_node, &dma_addr, &paddr, &size);
if (ret < 0) {
-   dev_dbg(dev, "no dma range information to setup\n");
-   return;
+   dma_addr = offset = 0;
+   size = dev->coherent_dma_mask;
+   } else {
+   offset = PFN_DOWN(paddr - dma_addr);
+   dev_dbg(dev, "dma_pfn_offset(%#08lx)\n", dev->dma_pfn_offset);
}
+   dev->dma_pfn_offset = offset;
+
+   coherent = of_dma_is_coherent(dev->of_node);
+   dev_dbg(dev, "device is%sdma coherent\n",
+   coherent ? " " : " not ");
 
-   /* DMA ranges found. Calculate and set dma_pfn_offset */
-   dev->dma_pfn_offset = PFN_DOWN(paddr - dma_addr);
-   dev_dbg(dev, "dma_pfn_offset(%#08lx)\n", dev->dma_pfn_offset);
+   arch_setup_dma_ops(dev, coherent);
 }
 
 /**
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index d5d388160f42..8a1560f95d4a 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -129,11 +129,8 @@ static inline int dma_coerce_mask_and_coherent(struct 
device *dev, u64 mask)
 
 extern u64 dma_get_required_mask(struct device *dev);
 
-#ifndef set_arch_dma_coherent_ops
-static inline int set_arch_dma_coherent_ops(struct device *dev)
-{
-   return 0;
-}
+#ifndef arch_setup_dma_ops
+static inline void arch_setup_dma_ops(struct device *dev, bool coherent) { }
 #endif
 
 static inline unsigned int dma_get_max_seg_size(struct device *dev)
-- 
2.1.1

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


[PATCH v6 6/8] dma-mapping: detect and configure IOMMU in of_dma_configure

2014-12-01 Thread Will Deacon
This patch extends of_dma_configure so that it sets up the IOMMU for a
device, as well as the coherent/non-coherent DMA mapping ops.

Acked-by: Arnd Bergmann 
Acked-by: Marek Szyprowski 
Tested-by: Robin Murphy 
Signed-off-by: Will Deacon 
---
 arch/arm/include/asm/dma-mapping.h |  4 +++-
 drivers/of/platform.c  | 21 ++---
 include/linux/dma-mapping.h|  8 +++-
 3 files changed, 24 insertions(+), 9 deletions(-)

diff --git a/arch/arm/include/asm/dma-mapping.h 
b/arch/arm/include/asm/dma-mapping.h
index dc3420e77758..f3c0d953f6a2 100644
--- a/arch/arm/include/asm/dma-mapping.h
+++ b/arch/arm/include/asm/dma-mapping.h
@@ -121,7 +121,9 @@ static inline unsigned long dma_max_pfn(struct device *dev)
 }
 #define dma_max_pfn(dev) dma_max_pfn(dev)
 
-static inline void arch_setup_dma_ops(struct device *dev, bool coherent)
+static inline void arch_setup_dma_ops(struct device *dev, u64 dma_base,
+ u64 size, struct iommu_ops *iommu,
+ bool coherent)
 {
if (coherent)
set_dma_ops(dev, &arm_coherent_dma_ops);
diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index ff1f4e9afccb..b89caf8c7586 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -19,6 +19,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -166,6 +167,7 @@ static void of_dma_configure(struct device *dev)
int ret;
bool coherent;
unsigned long offset;
+   struct iommu_ops *iommu;
 
/*
 * Set default dma-mask to 32 bit. Drivers are expected to setup
@@ -194,7 +196,16 @@ static void of_dma_configure(struct device *dev)
dev_dbg(dev, "device is%sdma coherent\n",
coherent ? " " : " not ");
 
-   arch_setup_dma_ops(dev, coherent);
+   iommu = of_iommu_configure(dev);
+   dev_dbg(dev, "device is%sbehind an iommu\n",
+   iommu ? " " : " not ");
+
+   arch_setup_dma_ops(dev, dma_addr, size, iommu, coherent);
+}
+
+static void of_dma_deconfigure(struct device *dev)
+{
+   arch_teardown_dma_ops(dev);
 }
 
 /**
@@ -223,16 +234,12 @@ static struct platform_device 
*of_platform_device_create_pdata(
if (!dev)
goto err_clear_flag;
 
-   of_dma_configure(&dev->dev);
dev->dev.bus = &platform_bus_type;
dev->dev.platform_data = platform_data;
-
-   /* We do not fill the DMA ops for platform devices by default.
-* This is currently the responsibility of the platform code
-* to do such, possibly using a device notifier
-*/
+   of_dma_configure(&dev->dev);
 
if (of_device_add(dev) != 0) {
+   of_dma_deconfigure(&dev->dev);
platform_device_put(dev);
goto err_clear_flag;
}
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index 8a1560f95d4a..c3007cb4bfa6 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -130,7 +130,13 @@ static inline int dma_coerce_mask_and_coherent(struct 
device *dev, u64 mask)
 extern u64 dma_get_required_mask(struct device *dev);
 
 #ifndef arch_setup_dma_ops
-static inline void arch_setup_dma_ops(struct device *dev, bool coherent) { }
+static inline void arch_setup_dma_ops(struct device *dev, u64 dma_base,
+ u64 size, struct iommu_ops *iommu,
+ bool coherent) { }
+#endif
+
+#ifndef arch_teardown_dma_ops
+static inline void arch_teardown_dma_ops(struct device *dev) { }
 #endif
 
 static inline unsigned int dma_get_max_seg_size(struct device *dev)
-- 
2.1.1

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


[PATCH v6 1/8] iommu: provide early initialisation hook for IOMMU drivers

2014-12-01 Thread Will Deacon
IOMMU drivers must be initialised before any of their upstream devices,
otherwise the relevant iommu_ops won't be configured for the bus in
question. To solve this, a number of IOMMU drivers use initcalls to
initialise the driver before anything has a chance to be probed.

Whilst this solves the immediate problem, it leaves the job of probing
the IOMMU completely separate from the iommu_ops to configure the IOMMU,
which are called on a per-bus basis and require the driver to figure out
exactly which instance of the IOMMU is being requested. In particular,
the add_device callback simply passes a struct device to the driver,
which then has to parse firmware tables or probe buses to identify the
relevant IOMMU instance.

This patch takes the first step in addressing this problem by adding an
early initialisation pass for IOMMU drivers, giving them the ability to
store some per-instance data in their iommu_ops structure and store that
in their of_node. This can later be used when parsing OF masters to
identify the IOMMU instance in question.

Acked-by: Arnd Bergmann 
Acked-by: Joerg Roedel 
Acked-by: Marek Szyprowski 
Tested-by: Robin Murphy 
Signed-off-by: Will Deacon 
---
 drivers/iommu/of_iommu.c  | 17 +
 include/asm-generic/vmlinux.lds.h |  2 ++
 include/linux/iommu.h |  2 ++
 include/linux/of_iommu.h  | 25 +
 4 files changed, 46 insertions(+)

diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
index e550ccb7634e..89b903406968 100644
--- a/drivers/iommu/of_iommu.c
+++ b/drivers/iommu/of_iommu.c
@@ -22,6 +22,9 @@
 #include 
 #include 
 
+static const struct of_device_id __iommu_of_table_sentinel
+   __used __section(__iommu_of_table_end);
+
 /**
  * of_get_dma_window - Parse *dma-window property and returns 0 if found.
  *
@@ -89,3 +92,17 @@ int of_get_dma_window(struct device_node *dn, const char 
*prefix, int index,
return 0;
 }
 EXPORT_SYMBOL_GPL(of_get_dma_window);
+
+void __init of_iommu_init(void)
+{
+   struct device_node *np;
+   const struct of_device_id *match, *matches = &__iommu_of_table;
+
+   for_each_matching_node_and_match(np, matches, &match) {
+   const of_iommu_init_fn init_fn = match->data;
+
+   if (init_fn(np))
+   pr_err("Failed to initialise IOMMU %s\n",
+   of_node_full_name(np));
+   }
+}
diff --git a/include/asm-generic/vmlinux.lds.h 
b/include/asm-generic/vmlinux.lds.h
index aa70cbda327c..bee5d683074d 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -164,6 +164,7 @@
 #define CLKSRC_OF_TABLES() OF_TABLE(CONFIG_CLKSRC_OF, clksrc)
 #define IRQCHIP_OF_MATCH_TABLE() OF_TABLE(CONFIG_IRQCHIP, irqchip)
 #define CLK_OF_TABLES()OF_TABLE(CONFIG_COMMON_CLK, clk)
+#define IOMMU_OF_TABLES()  OF_TABLE(CONFIG_OF_IOMMU, iommu)
 #define RESERVEDMEM_OF_TABLES()OF_TABLE(CONFIG_OF_RESERVED_MEM, 
reservedmem)
 #define CPU_METHOD_OF_TABLES() OF_TABLE(CONFIG_SMP, cpu_method)
 #define EARLYCON_OF_TABLES()   OF_TABLE(CONFIG_SERIAL_EARLYCON, earlycon)
@@ -497,6 +498,7 @@
CLK_OF_TABLES() \
RESERVEDMEM_OF_TABLES() \
CLKSRC_OF_TABLES()  \
+   IOMMU_OF_TABLES()   \
CPU_METHOD_OF_TABLES()  \
KERNEL_DTB()\
IRQCHIP_OF_MATCH_TABLE()\
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index e6a7c9ff72f2..7b83f9f8e11d 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -103,6 +103,7 @@ enum iommu_attr {
  * @domain_get_attr: Query domain attributes
  * @domain_set_attr: Change domain attributes
  * @pgsize_bitmap: bitmap of supported page sizes
+ * @priv: per-instance data private to the iommu driver
  */
 struct iommu_ops {
bool (*capable)(enum iommu_cap);
@@ -133,6 +134,7 @@ struct iommu_ops {
u32 (*domain_get_windows)(struct iommu_domain *domain);
 
unsigned long pgsize_bitmap;
+   void *priv;
 };
 
 #define IOMMU_GROUP_NOTIFY_ADD_DEVICE  1 /* Device added */
diff --git a/include/linux/of_iommu.h b/include/linux/of_iommu.h
index 51a560f34bca..5762cdc8effe 100644
--- a/include/linux/of_iommu.h
+++ b/include/linux/of_iommu.h
@@ -1,12 +1,17 @@
 #ifndef __OF_IOMMU_H
 #define __OF_IOMMU_H
 
+#include 
+#include 
+
 #ifdef CONFIG_OF_IOMMU
 
 extern int of_get_dma_window(struct device_node *dn, const char *prefix,
 int index, unsigned long *busno, dma_addr_t *addr,
 size_t *size);
 
+extern void of_iommu_init(void);
+
 #else
 
 static inline int of_get_dma_window(struct device_node *dn, const char *prefix

[PATCH v6 3/8] iommu: add new iommu_ops callback for adding an OF device

2014-12-01 Thread Will Deacon
This patch adds a new function to the iommu_ops structure to allow an
OF device to be added to a specific IOMMU instance using the recently
merged generic devicetree binding for IOMMUs. The callback (of_xlate)
takes a struct device representing the master and an of_phandle_args
representing the IOMMU and the correspondong IDs for the new master.

Acked-by: Arnd Bergmann 
Acked-by: Joerg Roedel 
Acked-by: Marek Szyprowski 
Tested-by: Robin Murphy 
Signed-off-by: Will Deacon 
---
 include/linux/iommu.h | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 7b83f9f8e11d..415c7613d02c 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -21,6 +21,7 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -102,6 +103,7 @@ enum iommu_attr {
  * @remove_device: remove device from iommu grouping
  * @domain_get_attr: Query domain attributes
  * @domain_set_attr: Change domain attributes
+ * @of_xlate: add OF master IDs to iommu grouping
  * @pgsize_bitmap: bitmap of supported page sizes
  * @priv: per-instance data private to the iommu driver
  */
@@ -133,6 +135,10 @@ struct iommu_ops {
/* Get the numer of window per domain */
u32 (*domain_get_windows)(struct iommu_domain *domain);
 
+#ifdef CONFIG_OF_IOMMU
+   int (*of_xlate)(struct device *dev, struct of_phandle_args *args);
+#endif
+
unsigned long pgsize_bitmap;
void *priv;
 };
-- 
2.1.1

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH 2/4] iommu: add ARM LPAE page table allocator

2014-12-01 Thread Will Deacon
On Sun, Nov 30, 2014 at 11:29:46PM +, Laurent Pinchart wrote:
> Hi Will,

Hello again,

> On Thursday 27 November 2014 11:51:16 Will Deacon wrote:
> > +static arm_lpae_iopte arm_lpae_prot_to_pte(struct arm_lpae_io_pgtable
> > *data, + int prot)
> > +{
> > + arm_lpae_iopte pte;
> > +
> > + if (data->iop.fmt == ARM_LPAE_S1) {
> > + pte = ARM_LPAE_PTE_AP_UNPRIV | ARM_LPAE_PTE_nG;
> > +
> > + if (!(prot & IOMMU_WRITE) && (prot & IOMMU_READ))
> > + pte |= ARM_LPAE_PTE_AP_RDONLY;
> > +
> > + if (prot & IOMMU_CACHE)
> > + pte |= (ARM_LPAE_MAIR_ATTR_IDX_CACHE
> > + << ARM_LPAE_PTE_ATTRINDX_SHIFT);
> 
> In my case I'll need to manage the NS bit (here and when allocating tables in
> __arm_lpae_map). The exact requirements are not exactly clear at the moment
> I'm afraid, the datasheet doesn't clearly document secure behaviour, but tests
> showed that setting the NS was necessary.

Hurrah! You can add a quick to the currently unused quirks field that I have
in io_pgtable_cfg :)

> Given that arm_lpae_init_pte() will unconditionally set the AF and SH_IS bits
> you could set them here too, but that shouldn't make a big difference.

I prefer to keep only the protection bits handled here (i.e. those bits that
map directly to the IOMMU_* prot flags).

> > + } else {
> > + pte = ARM_LPAE_PTE_HAP_FAULT;
> > + if (prot & IOMMU_READ)
> > + pte |= ARM_LPAE_PTE_HAP_READ;
> > + if (prot & IOMMU_WRITE)
> > + pte |= ARM_LPAE_PTE_HAP_WRITE;
> > + if (prot & IOMMU_CACHE)
> > + pte |= ARM_LPAE_PTE_MEMATTR_OIWB;
> > + else
> > + pte |= ARM_LPAE_PTE_MEMATTR_NC;
> > + }
> > +
> > + if (prot & IOMMU_NOEXEC)
> > + pte |= ARM_LPAE_PTE_XN;
> > +
> > + return pte;
> > +}
> > +
> > +static int arm_lpae_map(struct io_pgtable_ops *ops, unsigned long iova,
> > + phys_addr_t paddr, size_t size, int iommu_prot)
> > +{
> > + struct arm_lpae_io_pgtable *data = io_pgtable_ops_to_data(ops);
> > + arm_lpae_iopte *ptep = data->pgd;
> > + int lvl = ARM_LPAE_START_LVL(data);
> > + arm_lpae_iopte prot;
> > +
> > + /* If no access, then nothing to do */
> > + if (!(iommu_prot & (IOMMU_READ | IOMMU_WRITE)))
> > + return 0;
> 
> Shouldn't this create a faulting entry instead ?

That's effectively what it does. Calling iommu_map on something that's
already mapped is a programming error, so therefore we know that the entry
is already faulting by virtue of it being unmapped.

> > +static struct io_pgtable *arm_lpae_alloc_pgtable_s1(struct io_pgtable_cfg
> > *cfg, +   void *cookie)
> > +{
> > + u64 reg;
> > + struct arm_lpae_io_pgtable *data = arm_lpae_alloc_pgtable(cfg);
> > +
> > + if (!data)
> > + return NULL;
> > +
> > + /* TCR */
> > + reg = ARM_LPAE_TCR_EAE |
> > +  (ARM_LPAE_TCR_SH_IS << ARM_LPAE_TCR_SH0_SHIFT) |
> > +  (ARM_LPAE_TCR_RGN_WBWA << ARM_LPAE_TCR_IRGN0_SHIFT) |
> > +  (ARM_LPAE_TCR_RGN_WBWA << ARM_LPAE_TCR_ORGN0_SHIFT);
> > +
> > + switch (1 << data->pg_shift) {
> > + case SZ_4K:
> > + reg |= ARM_LPAE_TCR_TG0_4K;
> > + break;
> > + case SZ_16K:
> > + reg |= ARM_LPAE_TCR_TG0_16K;
> > + break;
> > + case SZ_64K:
> > + reg |= ARM_LPAE_TCR_TG0_64K;
> > + break;
> > + }
> > +
> > + switch (cfg->oas) {
> > + case 32:
> > + reg |= (ARM_LPAE_TCR_PS_32_BIT << ARM_LPAE_TCR_IPS_SHIFT);
> > + break;
> > + case 36:
> > + reg |= (ARM_LPAE_TCR_PS_36_BIT << ARM_LPAE_TCR_IPS_SHIFT);
> > + break;
> > + case 40:
> > + reg |= (ARM_LPAE_TCR_PS_40_BIT << ARM_LPAE_TCR_IPS_SHIFT);
> > + break;
> > + case 42:
> > + reg |= (ARM_LPAE_TCR_PS_42_BIT << ARM_LPAE_TCR_IPS_SHIFT);
> > + break;
> > + case 44:
> > + reg |= (ARM_LPAE_TCR_PS_44_BIT << ARM_LPAE_TCR_IPS_SHIFT);
> > + break;
> > + case 48:
> > + reg |= (ARM_LPAE_TCR_PS_48_BIT << ARM_LPAE_TCR_IPS_SHIFT);
> > + break;
> > + default:
> > + goto out_free_data;
> > + }
> > +
> > + reg |= (64ULL - cfg->ias) << ARM_LPAE_TCR_T0SZ_SHIFT;
> > + cfg->arm_lpae_s1_cfg.tcr = reg;
> > +
> > + /* MAIRs */
> > + reg = (ARM_LPAE_MAIR_ATTR_NC
> > +<< ARM_LPAE_MAIR_ATTR_SHIFT(ARM_LPAE_MAIR_ATTR_IDX_NC)) |
> > +   (ARM_LPAE_MAIR_ATTR_WBRWA
> > +<< ARM_LPAE_MAIR_ATTR_SHIFT(ARM_LPAE_MAIR_ATTR_IDX_CACHE)) |
> > +   (ARM_LPAE_MAIR_ATTR_DEVICE
> > +<< ARM_LPAE_MAIR_ATTR_SHIFT(ARM_LPAE_MAIR_ATTR_IDX_DEV));
> > +
> > + c

Re: [PATCH 2/4] iommu: add ARM LPAE page table allocator

2014-12-01 Thread Laurent Pinchart
Hi Will,

On Monday 01 December 2014 17:23:15 Will Deacon wrote:
> On Sun, Nov 30, 2014 at 11:29:46PM +, Laurent Pinchart wrote:
> > Hi Will,
> 
> Hello again,
> 
> > On Thursday 27 November 2014 11:51:16 Will Deacon wrote:
> > > +static arm_lpae_iopte arm_lpae_prot_to_pte(struct arm_lpae_io_pgtable
> > > *data, + int prot)
> > > +{
> > > + arm_lpae_iopte pte;
> > > +
> > > + if (data->iop.fmt == ARM_LPAE_S1) {
> > > + pte = ARM_LPAE_PTE_AP_UNPRIV | ARM_LPAE_PTE_nG;
> > > +
> > > + if (!(prot & IOMMU_WRITE) && (prot & IOMMU_READ))
> > > + pte |= ARM_LPAE_PTE_AP_RDONLY;
> > > +
> > > + if (prot & IOMMU_CACHE)
> > > + pte |= (ARM_LPAE_MAIR_ATTR_IDX_CACHE
> > > + << ARM_LPAE_PTE_ATTRINDX_SHIFT);
> > 
> > In my case I'll need to manage the NS bit (here and when allocating tables
> > in __arm_lpae_map). The exact requirements are not exactly clear at the
> > moment I'm afraid, the datasheet doesn't clearly document secure
> > behaviour, but tests showed that setting the NS was necessary.
> 
> Hurrah! You can add a quick to the currently unused quirks field that I have
> in io_pgtable_cfg :)

:-)

> > Given that arm_lpae_init_pte() will unconditionally set the AF and SH_IS
> > bits you could set them here too, but that shouldn't make a big
> > difference.
>
> I prefer to keep only the protection bits handled here (i.e. those bits that
> map directly to the IOMMU_* prot flags).

I thought so. That's fine with me.

> > > + } else {
> > > + pte = ARM_LPAE_PTE_HAP_FAULT;
> > > + if (prot & IOMMU_READ)
> > > + pte |= ARM_LPAE_PTE_HAP_READ;
> > > + if (prot & IOMMU_WRITE)
> > > + pte |= ARM_LPAE_PTE_HAP_WRITE;
> > > + if (prot & IOMMU_CACHE)
> > > + pte |= ARM_LPAE_PTE_MEMATTR_OIWB;
> > > + else
> > > + pte |= ARM_LPAE_PTE_MEMATTR_NC;
> > > + }
> > > +
> > > + if (prot & IOMMU_NOEXEC)
> > > + pte |= ARM_LPAE_PTE_XN;
> > > +
> > > + return pte;
> > > +}
> > > +
> > > +static int arm_lpae_map(struct io_pgtable_ops *ops, unsigned long iova,
> > > + phys_addr_t paddr, size_t size, int iommu_prot)
> > > +{
> > > + struct arm_lpae_io_pgtable *data = io_pgtable_ops_to_data(ops);
> > > + arm_lpae_iopte *ptep = data->pgd;
> > > + int lvl = ARM_LPAE_START_LVL(data);
> > > + arm_lpae_iopte prot;
> > > +
> > > + /* If no access, then nothing to do */
> > > + if (!(iommu_prot & (IOMMU_READ | IOMMU_WRITE)))
> > > + return 0;
> > 
> > Shouldn't this create a faulting entry instead ?
> 
> That's effectively what it does. Calling iommu_map on something that's
> already mapped is a programming error, so therefore we know that the entry
> is already faulting by virtue of it being unmapped.

Indeed.

> > > +static struct io_pgtable *arm_lpae_alloc_pgtable_s1(struct
> > > io_pgtable_cfg
> > > *cfg, +   void *cookie)
> > > +{
> > > + u64 reg;
> > > + struct arm_lpae_io_pgtable *data = arm_lpae_alloc_pgtable(cfg);
> > > +
> > > + if (!data)
> > > + return NULL;
> > > +
> > > + /* TCR */
> > > + reg = ARM_LPAE_TCR_EAE |
> > > +  (ARM_LPAE_TCR_SH_IS << ARM_LPAE_TCR_SH0_SHIFT) |
> > > +  (ARM_LPAE_TCR_RGN_WBWA << ARM_LPAE_TCR_IRGN0_SHIFT) |
> > > +  (ARM_LPAE_TCR_RGN_WBWA << ARM_LPAE_TCR_ORGN0_SHIFT);
> > > +
> > > + switch (1 << data->pg_shift) {
> > > + case SZ_4K:
> > > + reg |= ARM_LPAE_TCR_TG0_4K;
> > > + break;
> > > + case SZ_16K:
> > > + reg |= ARM_LPAE_TCR_TG0_16K;
> > > + break;
> > > + case SZ_64K:
> > > + reg |= ARM_LPAE_TCR_TG0_64K;
> > > + break;
> > > + }
> > > +
> > > + switch (cfg->oas) {
> > > + case 32:
> > > + reg |= (ARM_LPAE_TCR_PS_32_BIT << ARM_LPAE_TCR_IPS_SHIFT);
> > > + break;
> > > + case 36:
> > > + reg |= (ARM_LPAE_TCR_PS_36_BIT << ARM_LPAE_TCR_IPS_SHIFT);
> > > + break;
> > > + case 40:
> > > + reg |= (ARM_LPAE_TCR_PS_40_BIT << ARM_LPAE_TCR_IPS_SHIFT);
> > > + break;
> > > + case 42:
> > > + reg |= (ARM_LPAE_TCR_PS_42_BIT << ARM_LPAE_TCR_IPS_SHIFT);
> > > + break;
> > > + case 44:
> > > + reg |= (ARM_LPAE_TCR_PS_44_BIT << ARM_LPAE_TCR_IPS_SHIFT);
> > > + break;
> > > + case 48:
> > > + reg |= (ARM_LPAE_TCR_PS_48_BIT << ARM_LPAE_TCR_IPS_SHIFT);
> > > + break;
> > > + default:
> > > + goto out_free_data;
> > > + }
> > > +
> > > + reg |= (64ULL - cfg->ias) << ARM_LPAE_TCR_T0SZ_SHIFT;
> > > + cfg->arm_lpae_s1_cfg.tcr = reg;
> > > +
> > > + /* MAIRs */
>

Re: [PATCH 5/7] iommu/arm-smmu: don't bother truncating the s1 output size to VA_BITS

2014-12-01 Thread Andreas Schwab
Will Deacon  writes:

> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> index ecad700cd4f4..2a7e3331b93a 100644
> --- a/drivers/iommu/arm-smmu.c
> +++ b/drivers/iommu/arm-smmu.c
> @@ -24,7 +24,7 @@
>   *   - v7/v8 long-descriptor format
>   *   - Non-secure access to the SMMU
>   *   - 4k and 64k pages, with contiguous pte hints.
> - *   - Up to 42-bit addressing (dependent on VA_BITS)
> + *   - Up to 48-bit addressing (dependent on VA_BITS)

Does that mean that the dependency on !ARM_SMMU for ARM64_VA_BITS_48 is
obsolete?

Andreas.

-- 
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v6 2/8] dma-mapping: replace set_arch_dma_coherent_ops with arch_setup_dma_ops

2014-12-01 Thread Rob Herring
On Mon, Dec 1, 2014 at 10:57 AM, Will Deacon  wrote:
> set_arch_dma_coherent_ops is called from of_dma_configure in order to
> swizzle the architectural dma-mapping functions over to a cache-coherent
> implementation. This is currently implemented only for ARM.
>
> In anticipation of re-using this mechanism for IOMMU-backed dma-mapping
> ops too, this patch replaces the function with a broader
> arch_setup_dma_ops callback which will be extended in future.
>
> Acked-by: Arnd Bergmann 
> Acked-by: Marek Szyprowski 
> Tested-by: Robin Murphy 
> Signed-off-by: Will Deacon 

One comment below, but for the DT parts:

Acked-by: Rob Herring 

> ---
>  arch/arm/include/asm/dma-mapping.h |  8 
>  drivers/of/platform.c  | 31 +--
>  include/linux/dma-mapping.h|  7 ++-
>  3 files changed, 19 insertions(+), 27 deletions(-)
>
> diff --git a/arch/arm/include/asm/dma-mapping.h 
> b/arch/arm/include/asm/dma-mapping.h
> index 85738b200023..dc3420e77758 100644
> --- a/arch/arm/include/asm/dma-mapping.h
> +++ b/arch/arm/include/asm/dma-mapping.h
> @@ -121,12 +121,12 @@ static inline unsigned long dma_max_pfn(struct device 
> *dev)
>  }
>  #define dma_max_pfn(dev) dma_max_pfn(dev)
>
> -static inline int set_arch_dma_coherent_ops(struct device *dev)
> +static inline void arch_setup_dma_ops(struct device *dev, bool coherent)
>  {
> -   set_dma_ops(dev, &arm_coherent_dma_ops);
> -   return 0;
> +   if (coherent)
> +   set_dma_ops(dev, &arm_coherent_dma_ops);
>  }
> -#define set_arch_dma_coherent_ops(dev) set_arch_dma_coherent_ops(dev)
> +#define arch_setup_dma_ops arch_setup_dma_ops
>
>  static inline dma_addr_t phys_to_dma(struct device *dev, phys_addr_t paddr)
>  {
> diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> index 3b64d0bf5bba..ff1f4e9afccb 100644
> --- a/drivers/of/platform.c
> +++ b/drivers/of/platform.c
> @@ -164,6 +164,8 @@ static void of_dma_configure(struct device *dev)
>  {
> u64 dma_addr, paddr, size;
> int ret;
> +   bool coherent;
> +   unsigned long offset;
>
> /*
>  * Set default dma-mask to 32 bit. Drivers are expected to setup
> @@ -178,28 +180,21 @@ static void of_dma_configure(struct device *dev)
> if (!dev->dma_mask)
> dev->dma_mask = &dev->coherent_dma_mask;
>
> -   /*
> -* if dma-coherent property exist, call arch hook to setup
> -* dma coherent operations.
> -*/
> -   if (of_dma_is_coherent(dev->of_node)) {
> -   set_arch_dma_coherent_ops(dev);
> -   dev_dbg(dev, "device is dma coherent\n");
> -   }
> -
> -   /*
> -* if dma-ranges property doesn't exist - just return else
> -* setup the dma offset
> -*/
> ret = of_dma_get_range(dev->of_node, &dma_addr, &paddr, &size);
> if (ret < 0) {
> -   dev_dbg(dev, "no dma range information to setup\n");
> -   return;
> +   dma_addr = offset = 0;
> +   size = dev->coherent_dma_mask;

It looks like size is not used.

> +   } else {
> +   offset = PFN_DOWN(paddr - dma_addr);
> +   dev_dbg(dev, "dma_pfn_offset(%#08lx)\n", dev->dma_pfn_offset);
> }
> +   dev->dma_pfn_offset = offset;
> +
> +   coherent = of_dma_is_coherent(dev->of_node);
> +   dev_dbg(dev, "device is%sdma coherent\n",
> +   coherent ? " " : " not ");
>
> -   /* DMA ranges found. Calculate and set dma_pfn_offset */
> -   dev->dma_pfn_offset = PFN_DOWN(paddr - dma_addr);
> -   dev_dbg(dev, "dma_pfn_offset(%#08lx)\n", dev->dma_pfn_offset);
> +   arch_setup_dma_ops(dev, coherent);
>  }
>
>  /**
> diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
> index d5d388160f42..8a1560f95d4a 100644
> --- a/include/linux/dma-mapping.h
> +++ b/include/linux/dma-mapping.h
> @@ -129,11 +129,8 @@ static inline int dma_coerce_mask_and_coherent(struct 
> device *dev, u64 mask)
>
>  extern u64 dma_get_required_mask(struct device *dev);
>
> -#ifndef set_arch_dma_coherent_ops
> -static inline int set_arch_dma_coherent_ops(struct device *dev)
> -{
> -   return 0;
> -}
> +#ifndef arch_setup_dma_ops
> +static inline void arch_setup_dma_ops(struct device *dev, bool coherent) { }
>  #endif
>
>  static inline unsigned int dma_get_max_seg_size(struct device *dev)
> --
> 2.1.1
>
>
> ___
> linux-arm-kernel mailing list
> linux-arm-ker...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v6 6/8] dma-mapping: detect and configure IOMMU in of_dma_configure

2014-12-01 Thread Rob Herring
On Mon, Dec 1, 2014 at 10:57 AM, Will Deacon  wrote:
> This patch extends of_dma_configure so that it sets up the IOMMU for a
> device, as well as the coherent/non-coherent DMA mapping ops.
>
> Acked-by: Arnd Bergmann 
> Acked-by: Marek Szyprowski 
> Tested-by: Robin Murphy 
> Signed-off-by: Will Deacon 

Acked-by: Rob Herring 

> ---
>  arch/arm/include/asm/dma-mapping.h |  4 +++-
>  drivers/of/platform.c  | 21 ++---
>  include/linux/dma-mapping.h|  8 +++-
>  3 files changed, 24 insertions(+), 9 deletions(-)
>
> diff --git a/arch/arm/include/asm/dma-mapping.h 
> b/arch/arm/include/asm/dma-mapping.h
> index dc3420e77758..f3c0d953f6a2 100644
> --- a/arch/arm/include/asm/dma-mapping.h
> +++ b/arch/arm/include/asm/dma-mapping.h
> @@ -121,7 +121,9 @@ static inline unsigned long dma_max_pfn(struct device 
> *dev)
>  }
>  #define dma_max_pfn(dev) dma_max_pfn(dev)
>
> -static inline void arch_setup_dma_ops(struct device *dev, bool coherent)
> +static inline void arch_setup_dma_ops(struct device *dev, u64 dma_base,
> + u64 size, struct iommu_ops *iommu,
> + bool coherent)
>  {
> if (coherent)
> set_dma_ops(dev, &arm_coherent_dma_ops);
> diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> index ff1f4e9afccb..b89caf8c7586 100644
> --- a/drivers/of/platform.c
> +++ b/drivers/of/platform.c
> @@ -19,6 +19,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -166,6 +167,7 @@ static void of_dma_configure(struct device *dev)
> int ret;
> bool coherent;
> unsigned long offset;
> +   struct iommu_ops *iommu;
>
> /*
>  * Set default dma-mask to 32 bit. Drivers are expected to setup
> @@ -194,7 +196,16 @@ static void of_dma_configure(struct device *dev)
> dev_dbg(dev, "device is%sdma coherent\n",
> coherent ? " " : " not ");
>
> -   arch_setup_dma_ops(dev, coherent);
> +   iommu = of_iommu_configure(dev);
> +   dev_dbg(dev, "device is%sbehind an iommu\n",
> +   iommu ? " " : " not ");
> +
> +   arch_setup_dma_ops(dev, dma_addr, size, iommu, coherent);
> +}
> +
> +static void of_dma_deconfigure(struct device *dev)
> +{
> +   arch_teardown_dma_ops(dev);
>  }
>
>  /**
> @@ -223,16 +234,12 @@ static struct platform_device 
> *of_platform_device_create_pdata(
> if (!dev)
> goto err_clear_flag;
>
> -   of_dma_configure(&dev->dev);
> dev->dev.bus = &platform_bus_type;
> dev->dev.platform_data = platform_data;
> -
> -   /* We do not fill the DMA ops for platform devices by default.
> -* This is currently the responsibility of the platform code
> -* to do such, possibly using a device notifier
> -*/
> +   of_dma_configure(&dev->dev);
>
> if (of_device_add(dev) != 0) {
> +   of_dma_deconfigure(&dev->dev);
> platform_device_put(dev);
> goto err_clear_flag;
> }
> diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
> index 8a1560f95d4a..c3007cb4bfa6 100644
> --- a/include/linux/dma-mapping.h
> +++ b/include/linux/dma-mapping.h
> @@ -130,7 +130,13 @@ static inline int dma_coerce_mask_and_coherent(struct 
> device *dev, u64 mask)
>  extern u64 dma_get_required_mask(struct device *dev);
>
>  #ifndef arch_setup_dma_ops
> -static inline void arch_setup_dma_ops(struct device *dev, bool coherent) { }
> +static inline void arch_setup_dma_ops(struct device *dev, u64 dma_base,
> + u64 size, struct iommu_ops *iommu,
> + bool coherent) { }
> +#endif
> +
> +#ifndef arch_teardown_dma_ops
> +static inline void arch_teardown_dma_ops(struct device *dev) { }
>  #endif
>
>  static inline unsigned int dma_get_max_seg_size(struct device *dev)
> --
> 2.1.1
>
>
> ___
> linux-arm-kernel mailing list
> linux-arm-ker...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v6 1/8] iommu: provide early initialisation hook for IOMMU drivers

2014-12-01 Thread Rob Herring
Adding Grant and Pantelis...

On Mon, Dec 1, 2014 at 10:57 AM, Will Deacon  wrote:
> IOMMU drivers must be initialised before any of their upstream devices,
> otherwise the relevant iommu_ops won't be configured for the bus in
> question. To solve this, a number of IOMMU drivers use initcalls to
> initialise the driver before anything has a chance to be probed.
>
> Whilst this solves the immediate problem, it leaves the job of probing
> the IOMMU completely separate from the iommu_ops to configure the IOMMU,
> which are called on a per-bus basis and require the driver to figure out
> exactly which instance of the IOMMU is being requested. In particular,
> the add_device callback simply passes a struct device to the driver,
> which then has to parse firmware tables or probe buses to identify the
> relevant IOMMU instance.
>
> This patch takes the first step in addressing this problem by adding an
> early initialisation pass for IOMMU drivers, giving them the ability to
> store some per-instance data in their iommu_ops structure and store that
> in their of_node. This can later be used when parsing OF masters to
> identify the IOMMU instance in question.
>
> Acked-by: Arnd Bergmann 
> Acked-by: Joerg Roedel 
> Acked-by: Marek Szyprowski 
> Tested-by: Robin Murphy 
> Signed-off-by: Will Deacon 
> ---
>  drivers/iommu/of_iommu.c  | 17 +
>  include/asm-generic/vmlinux.lds.h |  2 ++
>  include/linux/iommu.h |  2 ++
>  include/linux/of_iommu.h  | 25 +
>  4 files changed, 46 insertions(+)
>
> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
> index e550ccb7634e..89b903406968 100644
> --- a/drivers/iommu/of_iommu.c
> +++ b/drivers/iommu/of_iommu.c
> @@ -22,6 +22,9 @@
>  #include 
>  #include 
>
> +static const struct of_device_id __iommu_of_table_sentinel
> +   __used __section(__iommu_of_table_end);
> +
>  /**
>   * of_get_dma_window - Parse *dma-window property and returns 0 if found.
>   *
> @@ -89,3 +92,17 @@ int of_get_dma_window(struct device_node *dn, const char 
> *prefix, int index,
> return 0;
>  }
>  EXPORT_SYMBOL_GPL(of_get_dma_window);
> +
> +void __init of_iommu_init(void)
> +{
> +   struct device_node *np;
> +   const struct of_device_id *match, *matches = &__iommu_of_table;
> +
> +   for_each_matching_node_and_match(np, matches, &match) {
> +   const of_iommu_init_fn init_fn = match->data;
> +
> +   if (init_fn(np))
> +   pr_err("Failed to initialise IOMMU %s\n",
> +   of_node_full_name(np));
> +   }
> +}
> diff --git a/include/asm-generic/vmlinux.lds.h 
> b/include/asm-generic/vmlinux.lds.h
> index aa70cbda327c..bee5d683074d 100644
> --- a/include/asm-generic/vmlinux.lds.h
> +++ b/include/asm-generic/vmlinux.lds.h
> @@ -164,6 +164,7 @@
>  #define CLKSRC_OF_TABLES() OF_TABLE(CONFIG_CLKSRC_OF, clksrc)
>  #define IRQCHIP_OF_MATCH_TABLE() OF_TABLE(CONFIG_IRQCHIP, irqchip)
>  #define CLK_OF_TABLES()OF_TABLE(CONFIG_COMMON_CLK, clk)
> +#define IOMMU_OF_TABLES()  OF_TABLE(CONFIG_OF_IOMMU, iommu)
>  #define RESERVEDMEM_OF_TABLES()OF_TABLE(CONFIG_OF_RESERVED_MEM, 
> reservedmem)
>  #define CPU_METHOD_OF_TABLES() OF_TABLE(CONFIG_SMP, cpu_method)
>  #define EARLYCON_OF_TABLES()   OF_TABLE(CONFIG_SERIAL_EARLYCON, earlycon)
> @@ -497,6 +498,7 @@
> CLK_OF_TABLES() \
> RESERVEDMEM_OF_TABLES() \
> CLKSRC_OF_TABLES()  \
> +   IOMMU_OF_TABLES()   \
> CPU_METHOD_OF_TABLES()  \
> KERNEL_DTB()\
> IRQCHIP_OF_MATCH_TABLE()\
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index e6a7c9ff72f2..7b83f9f8e11d 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -103,6 +103,7 @@ enum iommu_attr {
>   * @domain_get_attr: Query domain attributes
>   * @domain_set_attr: Change domain attributes
>   * @pgsize_bitmap: bitmap of supported page sizes
> + * @priv: per-instance data private to the iommu driver
>   */
>  struct iommu_ops {
> bool (*capable)(enum iommu_cap);
> @@ -133,6 +134,7 @@ struct iommu_ops {
> u32 (*domain_get_windows)(struct iommu_domain *domain);
>
> unsigned long pgsize_bitmap;
> +   void *priv;
>  };
>
>  #define IOMMU_GROUP_NOTIFY_ADD_DEVICE  1 /* Device added */
> diff --git a/include/linux/of_iommu.h b/include/linux/of_iommu.h
> index 51a560f34bca..5762cdc8effe 100644
> --- a/include/linux/of_iommu.h
> +++ b/include/linux/of_iommu.h
> @@ -1,12 +1,17 @@
>  #ifndef __OF_IOMMU_H
>  #define __OF_IOMMU_H
>
> +#include 
> +#include 
> +
>  #ifdef CONFIG_OF_IOMMU
>
>  extern int of_

Re: [PATCH] iommu/vt-d: Fix an off-by-one bug in __domain_mapping()

2014-12-01 Thread Jiang Liu


On 2014/12/2 0:27, Joerg Roedel wrote:
> On Wed, Nov 26, 2014 at 09:42:10AM +0800, Jiang Liu wrote:
>> There's an off-by-one bug in function __domain_mapping(), which may
>> trigger the BUG_ON(nr_pages < lvl_pages) when
>>  (nr_pages + 1) & superpage_mask == 0
> 
> What is the superpage_mask?
Hi Joerg,
Sorry for the confusion. The really story is:
1) sg_res is set to nr_pages + 1 at the beginning of __domain_mapping()
2) then sg_res is used to choose super page by function
   hardware_largepage_caps(domain, iov_pfn, phys_pfn, sg_res).
The condition to trigger the issue is:
__domain_mapping is called by domain_pfn_mapping() with nr_pages
of 511, so sg_res is 512 and hardware_largepage_caps() will
choose a wrong super page size of 2M, which then trigger
BUG_ON(sg_res < lvl_pages).

So it's not only a BUG_ON() issue, but also causes incorrect super page
selection.
Thanks!
Gerry

> 
>> The issue was introduced by commit 9051aa0268dc "intel-iommu: Combine
>> domain_pfn_mapping() and domain_sg_mapping()", which sets sg_res to
>> "nr_pages + 1" to avoid some of the 'sg_res==0' code paths.
>>
>> It's safe to remove extra "+1" because sg_res is only used to calculate
>> page size now.
> 
> From your description and the (hard to read) code in __domain_mapping I
> don't really understand the issue yet. Can you please elaborate on this
> issue can be triggered?
> 
> Is the BUG_ON the only issue and, if yes, can that be fixed by just
> changing the BUG_ON condition?
> 
>>  This issue was introduced in v2.6.31, but intel-iommu.c has
>> been moved into drivers/iommu in v3.1. So what's the preferred way
>> to deal with stable kernels between v2.6.31 and v3.1?
> 
> Just remove the kernel version marker from the stable tag. The stable
> kernel maintainers for kernels >3.1 will ask you to backport the patch
> or just backport it by themselfes.
> 
> 
>   Joerg
> 
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [Patch Part3 v4 18/38] x86: irq_remapping: Clean up unsued code

2014-12-01 Thread Bjorn Helgaas
s/unsued/unused/ in subject (here and other patches).

"Clean up" is slightly ambiguous; it could mean either "make better" or
"remove."  "Remove unused code" would be less ambiguous.

On Tue, Nov 25, 2014 at 03:49:42PM +0800, Jiang Liu wrote:
> Now we have converted to hierarchy irqdomain, so clean up unused code.
> 
> Signed-off-by: Jiang Liu 
> Tested-by: Joerg Roedel 
> ---
>  arch/x86/include/asm/irq_remapping.h |   23 --
>  arch/x86/kernel/apic/vector.c|1 -
>  drivers/iommu/irq_remapping.c|   36 
> --
>  3 files changed, 60 deletions(-)
> 
> diff --git a/arch/x86/include/asm/irq_remapping.h 
> b/arch/x86/include/asm/irq_remapping.h
> index de8470b6f36a..6ba243126b3b 100644
> --- a/arch/x86/include/asm/irq_remapping.h
> +++ b/arch/x86/include/asm/irq_remapping.h
> @@ -26,12 +26,7 @@
>  #include 
>  #include 
>  
> -struct IO_APIC_route_entry;
> -struct io_apic_irq_attr;
> -struct irq_chip;
>  struct msi_msg;
> -struct pci_dev;
> -struct irq_cfg;
>  struct irq_alloc_info;
>  
>  #ifdef CONFIG_IRQ_REMAP
> @@ -44,13 +39,7 @@ extern int irq_remapping_enable(void);
>  extern void irq_remapping_disable(void);
>  extern int irq_remapping_reenable(int);
>  extern int irq_remap_enable_fault_handling(void);
> -extern void free_remapped_irq(int irq);
>  extern void panic_if_irq_remap(const char *msg);
> -extern bool setup_remapped_irq(int irq,
> -struct irq_cfg *cfg,
> -struct irq_chip *chip);
> -
> -void irq_remap_modify_chip_defaults(struct irq_chip *chip);
>  
>  extern struct irq_domain *
>  irq_remapping_get_ir_irq_domain( struct irq_alloc_info *info);
> @@ -77,23 +66,11 @@ static inline int irq_remapping_enable(void) { return 
> -ENODEV; }
>  static inline void irq_remapping_disable(void) { }
>  static inline int irq_remapping_reenable(int eim) { return -ENODEV; }
>  static inline int irq_remap_enable_fault_handling(void) { return -ENODEV; }
> -static inline void free_remapped_irq(int irq) { }
>  
>  static inline void panic_if_irq_remap(const char *msg)
>  {
>  }
>  
> -static inline void irq_remap_modify_chip_defaults(struct irq_chip *chip)
> -{
> -}
> -
> -static inline bool setup_remapped_irq(int irq,
> -   struct irq_cfg *cfg,
> -   struct irq_chip *chip)
> -{
> - return false;
> -}
> -
>  static inline struct irq_domain *
>  irq_remapping_get_ir_irq_domain(struct irq_alloc_info *info)
>  {
> diff --git a/arch/x86/kernel/apic/vector.c b/arch/x86/kernel/apic/vector.c
> index 31dc4a871673..ca30365507a0 100644
> --- a/arch/x86/kernel/apic/vector.c
> +++ b/arch/x86/kernel/apic/vector.c
> @@ -283,7 +283,6 @@ static void x86_vector_free_irqs(struct irq_domain 
> *domain,
>   for (i = 0; i < nr_irqs; i++) {
>   irq_data = irq_domain_get_irq_data(x86_vector_domain, virq + i);
>   if (irq_data && irq_data->chip_data) {
> - free_remapped_irq(virq);
>   clear_irq_vector(virq + i, irq_data->chip_data);
>   free_irq_cfg(irq_data->chip_data);
>  #ifdef   CONFIG_X86_IO_APIC
> diff --git a/drivers/iommu/irq_remapping.c b/drivers/iommu/irq_remapping.c
> index d175d6911821..3c3da04da8c5 100644
> --- a/drivers/iommu/irq_remapping.c
> +++ b/drivers/iommu/irq_remapping.c
> @@ -26,11 +26,6 @@ int no_x2apic_optout;
>  
>  static struct irq_remap_ops *remap_ops;
>  
> -static bool irq_remapped(struct irq_cfg *cfg)
> -{
> - return (cfg->remapped == 1);
> -}
> -
>  static void irq_remapping_disable_io_apic(void)
>  {
>   /*
> @@ -160,17 +155,6 @@ int __init irq_remap_enable_fault_handling(void)
>   return remap_ops->enable_faulting();
>  }
>  
> -void free_remapped_irq(int irq)
> -{
> - struct irq_cfg *cfg = irq_cfg(irq);
> -
> - if (!remap_ops || !remap_ops->free_irq)
> - return;
> -
> - if (irq_remapped(cfg))
> - remap_ops->free_irq(irq);
> -}
> -
>  void panic_if_irq_remap(const char *msg)
>  {
>   if (irq_remapping_enabled)
> @@ -195,26 +179,6 @@ void irq_remapping_print_chip(struct irq_data *data, 
> struct seq_file *p)
>   seq_printf(p, " %s", data->chip->name);
>  }
>  
> -static void ir_print_prefix(struct irq_data *data, struct seq_file *p)
> -{
> - seq_printf(p, " IR-%s", data->chip->name);
> -}
> -
> -void irq_remap_modify_chip_defaults(struct irq_chip *chip)
> -{
> - chip->irq_print_chip = ir_print_prefix;
> - chip->irq_ack = ir_ack_apic_edge;
> -}
> -
> -bool setup_remapped_irq(int irq, struct irq_cfg *cfg, struct irq_chip *chip)
> -{
> - if (!irq_remapped(cfg))
> - return false;
> - irq_set_status_flags(irq, IRQ_MOVE_PCNTXT);
> - irq_remap_modify_chip_defaults(chip);
> - return true;
> -}
> -
>  /**
>   * irq_remapping_get_ir_irq_domain - Get the irqdomain associated with the 
> IOMMU
>   *device