RE: [EXT] Re: [PATCH v2 3/3] iommu/virtio: Reject IOMMU page granule larger than PAGE_SIZE

2020-03-26 Thread Bharat Bhushan
Hi Jean,

> -Original Message-
> From: Auger Eric 
> Sent: Thursday, March 26, 2020 11:11 PM
> To: Jean-Philippe Brucker ; iommu@lists.linux-
> foundation.org
> Cc: virtualizat...@lists.linux-foundation.org; j...@8bytes.org; 
> m...@redhat.com;
> jasow...@redhat.com; Bharat Bhushan 
> Subject: [EXT] Re: [PATCH v2 3/3] iommu/virtio: Reject IOMMU page granule 
> larger
> than PAGE_SIZE
> 
> External Email
> 
> --
> Hi Jean,
> 
> On 3/26/20 10:35 AM, Jean-Philippe Brucker wrote:
> > We don't currently support IOMMUs with a page granule larger than the
> > system page size. The IOVA allocator has a BUG_ON() in this case, and
> > VFIO has a WARN_ON().
> >
> > Removing these obstacles ranges doesn't seem possible without major
> > changes to the DMA API and VFIO. Some callers of iommu_map(), for
> > example, want to map multiple page-aligned regions adjacent to each
> > others for scatter-gather purposes. Even in simple DMA API uses, a
> > call to dma_map_page() would let the endpoint access neighbouring
> > memory. And VFIO users cannot ensure that their virtual address buffer
> > is physically contiguous at the IOMMU granule.
> >
> > Rather than triggering the IOVA BUG_ON() on mismatched page sizes,
> > abort the vdomain finalise() with an error message. We could simply
> > abort the viommu probe(), but an upcoming extension to virtio-iommu
> > will allow setting different page masks for each endpoint.
> >
> > Reported-by: Bharat Bhushan 
> > Signed-off-by: Jean-Philippe Brucker 
> Reviewed-by: Eric Auger 

Reviewed-by: Eric Auger 

Thanks
-Bharat

> 
> Thanks
> 
> Eric
> > ---
> > v1->v2: Move to vdomain_finalise(), improve commit message
> > ---
> >  drivers/iommu/virtio-iommu.c | 14 --
> >  1 file changed, 12 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/iommu/virtio-iommu.c
> > b/drivers/iommu/virtio-iommu.c index 5eed75cd121f..750f69c49b95 100644
> > --- a/drivers/iommu/virtio-iommu.c
> > +++ b/drivers/iommu/virtio-iommu.c
> > @@ -607,12 +607,22 @@ static struct iommu_domain
> *viommu_domain_alloc(unsigned type)
> > return >domain;
> >  }
> >
> > -static int viommu_domain_finalise(struct viommu_dev *viommu,
> > +static int viommu_domain_finalise(struct viommu_endpoint *vdev,
> >   struct iommu_domain *domain)
> >  {
> > int ret;
> > +   unsigned long viommu_page_size;
> > +   struct viommu_dev *viommu = vdev->viommu;
> > struct viommu_domain *vdomain = to_viommu_domain(domain);
> >
> > +   viommu_page_size = 1UL << __ffs(viommu->pgsize_bitmap);
> > +   if (viommu_page_size > PAGE_SIZE) {
> > +   dev_err(vdev->dev,
> > +   "granule 0x%lx larger than system page size 0x%lx\n",
> > +   viommu_page_size, PAGE_SIZE);
> > +   return -EINVAL;
> > +   }
> > +
> > ret = ida_alloc_range(>domain_ids, viommu->first_domain,
> >   viommu->last_domain, GFP_KERNEL);
> > if (ret < 0)
> > @@ -659,7 +669,7 @@ static int viommu_attach_dev(struct iommu_domain
> *domain, struct device *dev)
> >  * Properly initialize the domain now that we know which viommu
> >  * owns it.
> >  */
> > -   ret = viommu_domain_finalise(vdev->viommu, domain);
> > +   ret = viommu_domain_finalise(vdev, domain);
> > } else if (vdomain->viommu != vdev->viommu) {
> > dev_err(dev, "cannot attach to foreign vIOMMU\n");
> > ret = -EXDEV;
> >

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


Re: [PATCH v4 02/16] ACPI/IORT: Remove direct access of dev->iommu_fwspec

2020-03-26 Thread Hanjun Guo
On 2020/3/26 23:08, Joerg Roedel wrote:
> From: Joerg Roedel 
> 
> Use the accessor functions instead of directly dereferencing
> dev->iommu_fwspec.
> 
> Tested-by: Hanjun Guo 
> Reviewed-by: Jean-Philippe Brucker 
> Signed-off-by: Joerg Roedel 
> ---
>  drivers/acpi/arm64/iort.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
> index ed3d2d1a7ae9..7d04424189df 100644
> --- a/drivers/acpi/arm64/iort.c
> +++ b/drivers/acpi/arm64/iort.c
> @@ -1015,6 +1015,7 @@ const struct iommu_ops *iort_iommu_configure(struct 
> device *dev)
>   return ops;
>  
>   if (dev_is_pci(dev)) {
> + struct iommu_fwspec *fwspec;
>   struct pci_bus *bus = to_pci_dev(dev)->bus;
>   struct iort_pci_alias_info info = { .dev = dev };
>  
> @@ -1027,8 +1028,9 @@ const struct iommu_ops *iort_iommu_configure(struct 
> device *dev)
>   err = pci_for_each_dma_alias(to_pci_dev(dev),
>iort_pci_iommu_init, );

...

>  
> - if (!err && iort_pci_rc_supports_ats(node))
> - dev->iommu_fwspec->flags |= IOMMU_FWSPEC_PCI_RC_ATS;
> + fwspec = dev_iommu_fwspec_get(dev);
> + if (fwspec && iort_pci_rc_supports_ats(node))

Should we check !err as well?

Thanks
Hanjun

> + fwspec->flags |= IOMMU_FWSPEC_PCI_RC_ATS;
>   } else {
>   int i = 0;
>  
> 

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


RE: [PATCH v2 1/3] iommu/uapi: Define uapi version and capabilities

2020-03-26 Thread Tian, Kevin
> From: Jacob Pan 
> Sent: Friday, March 27, 2020 12:45 AM
> 
> Hi Christoph,
> 
> Thanks for the review. Please see my comments inline.
> 
> On Thu, 26 Mar 2020 02:23:16 -0700
> Christoph Hellwig  wrote:
> 
> > On Wed, Mar 25, 2020 at 04:17:05PM -0700, Jacob Pan wrote:
> > > Having a single UAPI version to govern the user-kernel data
> > > structures makes compatibility check straightforward. On the
> > > contrary, supporting combinations of multiple versions of the data
> > > can be a nightmare to maintain.
> > >
> > > This patch defines a unified UAPI version to be used for
> > > compatibility checks between user and kernel.
> >
> > This is bullshit.  Version numbers don't scale and we've avoided them
> > everywhere.  You need need flags specifying specific behavior.
> >
> We have flags or the equivalent in each UAPI structures. The flags
> are used for checking validity of extensions as well. And you are right
> we can use them for checking specific behavior.
> 
> So we have two options here:
> 1. Have a overall version for a quick compatibility check while
> starting a VM. If not compatible, we will stop guest SVA entirely.
> 
> 2. Let each API calls check its own capabilities/flags at runtime. It
> may fail later on and lead to more complex error handling.
> For example, guest starts with SVA support, allocate a PASID, bind
> guest PASID works great. Then when IO page fault happens, it try to do
> page request service and found out certain flags are not compatible.
> This case is more complex to handle.

If those API calls are inter-dependent for composing a feature (e.g. SVA),
shouldn't we need a way to check them together before exposing the 
feature to the guest, e.g. through a iommu_get_uapi_capabilities interface?

> 
> I am guessing your proposal is #2. right?
> 
> Overall, we don;t expect much change to the UAPI other than adding some
> vendor specific part of the union. Is the scalability concern based on
> frequent changes?
> 
> > > +#define IOMMU_UAPI_VERSION   1
> > > +static inline int iommu_get_uapi_version(void)
> > > +{
> > > + return IOMMU_UAPI_VERSION;
> > > +}
> >
> > Also inline functions like this in UAPI headers that actually get
> > included by userspace programs simply don't work.
> 
> I will remove that, user can just use IOMMU_UAPI_VERSION directly.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH 03/10] iommu/ioasid: Introduce per set allocation APIs

2020-03-26 Thread Jacob Pan
Hi Baolu,

On Thu, 26 Mar 2020 10:12:36 +0800
Lu Baolu  wrote:

> On 2020/3/26 1:55, Jacob Pan wrote:
> > IOASID set defines a group of IDs that share the same token. The
> > ioasid_set concept helps to do permission checking among users as
> > in the current code.
> > 
> > With guest SVA usage, each VM has its own IOASID set. More
> > functionalities are needed:
> > 1. Enforce quota, each guest may be assigned limited quota such
> > that one guest cannot abuse all the system resource.
> > 2. Stores IOASID mapping between guest and host IOASIDs
> > 3. Per set operations, e.g. free the entire set
> > 
> > For each ioasid_set token, a unique set ID is assigned. This makes
> > reference of the set and data lookup much easier to implement.
> > 
> > Signed-off-by: Liu Yi L 
> > Signed-off-by: Jacob Pan 
> > ---
> >   drivers/iommu/ioasid.c | 147
> > +
> > include/linux/ioasid.h |  13 + 2 files changed, 160
> > insertions(+)
> > 
> > diff --git a/drivers/iommu/ioasid.c b/drivers/iommu/ioasid.c
> > index 4026e52855b9..27ee57f7079b 100644
> > --- a/drivers/iommu/ioasid.c
> > +++ b/drivers/iommu/ioasid.c
> > @@ -10,6 +10,25 @@
> >   #include 
> >   #include 
> >   
> > +static DEFINE_XARRAY_ALLOC(ioasid_sets);
> > +/**
> > + * struct ioasid_set_data - Meta data about ioasid_set
> > + *
> > + * @token: Unique to identify an IOASID set
> > + * @xa:XArray to store subset ID and IOASID mapping
> > + * @size:  Max number of IOASIDs can be allocated within the
> > set
> > + * @nr_ioasids Number of IOASIDs allocated in the set
> > + * @sidID of the set
> > + */
> > +struct ioasid_set_data {
> > +   struct ioasid_set *token;
> > +   struct xarray xa;
> > +   int size;
> > +   int nr_ioasids;
> > +   int sid;
> > +   struct rcu_head rcu;
> > +};
> > +
> >   struct ioasid_data {
> > ioasid_t id;
> > struct ioasid_set *set;
> > @@ -388,6 +407,111 @@ void ioasid_free(ioasid_t ioasid)
> >   EXPORT_SYMBOL_GPL(ioasid_free);
> >   
> >   /**
> > + * ioasid_alloc_set - Allocate a set of IOASIDs
> > + * @token: Unique token of the IOASID set
> > + * @quota: Quota allowed in this set
> > + * @sid:   IOASID set ID to be assigned
> > + *
> > + * Return 0 upon success. Token will be stored internally for
> > lookup,
> > + * IOASID allocation within the set and other per set operations
> > will use
> > + * the @sid assigned.
> > + *
> > + */
> > +int ioasid_alloc_set(struct ioasid_set *token, ioasid_t quota, int
> > *sid) +{
> > +   struct ioasid_set_data *sdata;
> > +   ioasid_t id;
> > +   int ret = 0;
> > +
> > +   if (quota > ioasid_capacity_avail) {
> > +   pr_warn("Out of IOASID capacity! ask %d, avail
> > %d\n",
> > +   quota, ioasid_capacity_avail);
> > +   return -ENOSPC;
> > +   }
> > +
> > +   sdata = kzalloc(sizeof(*sdata), GFP_KERNEL);
> > +   if (!sdata)
> > +   return -ENOMEM;
> > +
> > +   spin_lock(_allocator_lock);
> > +
> > +   ret = xa_alloc(_sets, , sdata,
> > +  XA_LIMIT(0, ioasid_capacity_avail - quota),
> > +  GFP_KERNEL);
> > +   if (ret) {
> > +   kfree(sdata);
> > +   goto error;
> > +   }
> > +
> > +   sdata->token = token;
> > +   sdata->size = quota;
> > +   sdata->sid = id;
> > +
> > +   /*
> > +* Set Xarray is used to store IDs within the set, get
> > ready for
> > +* sub-set ID and system-wide IOASID allocation results.
> > +*/
> > +   xa_init_flags(>xa, XA_FLAGS_ALLOC);
> > +
> > +   ioasid_capacity_avail -= quota;
> > +   *sid = id;
> > +
> > +error:
> > +   spin_unlock(_allocator_lock);
> > +
> > +   return ret;
> > +}
> > +EXPORT_SYMBOL_GPL(ioasid_alloc_set);
> > +
> > +/**
> > + * ioasid_free_set - Free all IOASIDs within the set
> > + *
> > + * @sid:   The IOASID set ID to be freed
> > + * @destroy_set:   Whether to keep the set for further
> > allocation.
> > + * If true, the set will be destroyed.
> > + *
> > + * All IOASIDs allocated within the set will be freed upon return.
> > + */
> > +void ioasid_free_set(int sid, bool destroy_set)
> > +{
> > +   struct ioasid_set_data *sdata;
> > +   struct ioasid_data *entry;
> > +   unsigned long index;
> > +
> > +   spin_lock(_allocator_lock);
> > +   sdata = xa_load(_sets, sid);
> > +   if (!sdata) {
> > +   pr_err("No IOASID set found to free %d\n", sid);
> > +   goto done_unlock;
> > +   }
> > +
> > +   if (xa_empty(>xa)) {
> > +   pr_warn("No IOASIDs in the set %d\n", sdata->sid);
> > +   goto done_destroy;
> > +   }
> > +
> > +   /* Just a place holder for now */
> > +   xa_for_each(>xa, index, entry) {
> > +   /* Free from per sub-set pool */
> > +   xa_erase(>xa, index);
> > +   }
> > +
> > +done_destroy:
> > +   if (destroy_set) {
> > +   xa_erase(_sets, sid);
> > +
> > +   /* Return the quota back to system pool */
> > +   ioasid_capacity_avail += sdata->size;
> > +  

Re: [PATCH v2 3/3] iommu/virtio: Reject IOMMU page granule larger than PAGE_SIZE

2020-03-26 Thread Auger Eric
Hi Jean,

On 3/26/20 10:35 AM, Jean-Philippe Brucker wrote:
> We don't currently support IOMMUs with a page granule larger than the
> system page size. The IOVA allocator has a BUG_ON() in this case, and
> VFIO has a WARN_ON().
> 
> Removing these obstacles ranges doesn't seem possible without major
> changes to the DMA API and VFIO. Some callers of iommu_map(), for
> example, want to map multiple page-aligned regions adjacent to each
> others for scatter-gather purposes. Even in simple DMA API uses, a call
> to dma_map_page() would let the endpoint access neighbouring memory. And
> VFIO users cannot ensure that their virtual address buffer is physically
> contiguous at the IOMMU granule.
> 
> Rather than triggering the IOVA BUG_ON() on mismatched page sizes, abort
> the vdomain finalise() with an error message. We could simply abort the
> viommu probe(), but an upcoming extension to virtio-iommu will allow
> setting different page masks for each endpoint.
> 
> Reported-by: Bharat Bhushan 
> Signed-off-by: Jean-Philippe Brucker 
Reviewed-by: Eric Auger 

Thanks

Eric
> ---
> v1->v2: Move to vdomain_finalise(), improve commit message
> ---
>  drivers/iommu/virtio-iommu.c | 14 --
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c
> index 5eed75cd121f..750f69c49b95 100644
> --- a/drivers/iommu/virtio-iommu.c
> +++ b/drivers/iommu/virtio-iommu.c
> @@ -607,12 +607,22 @@ static struct iommu_domain 
> *viommu_domain_alloc(unsigned type)
>   return >domain;
>  }
>  
> -static int viommu_domain_finalise(struct viommu_dev *viommu,
> +static int viommu_domain_finalise(struct viommu_endpoint *vdev,
> struct iommu_domain *domain)
>  {
>   int ret;
> + unsigned long viommu_page_size;
> + struct viommu_dev *viommu = vdev->viommu;
>   struct viommu_domain *vdomain = to_viommu_domain(domain);
>  
> + viommu_page_size = 1UL << __ffs(viommu->pgsize_bitmap);
> + if (viommu_page_size > PAGE_SIZE) {
> + dev_err(vdev->dev,
> + "granule 0x%lx larger than system page size 0x%lx\n",
> + viommu_page_size, PAGE_SIZE);
> + return -EINVAL;
> + }
> +
>   ret = ida_alloc_range(>domain_ids, viommu->first_domain,
> viommu->last_domain, GFP_KERNEL);
>   if (ret < 0)
> @@ -659,7 +669,7 @@ static int viommu_attach_dev(struct iommu_domain *domain, 
> struct device *dev)
>* Properly initialize the domain now that we know which viommu
>* owns it.
>*/
> - ret = viommu_domain_finalise(vdev->viommu, domain);
> + ret = viommu_domain_finalise(vdev, domain);
>   } else if (vdomain->viommu != vdev->viommu) {
>   dev_err(dev, "cannot attach to foreign vIOMMU\n");
>   ret = -EXDEV;
> 

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


Re: [PATCH] swiotlb: Allow swiotlb to live at pre-defined address

2020-03-26 Thread David Woodhouse
On Thu, 2020-03-26 at 18:11 +0100, Alexander Graf wrote:
> I'm with you on that sentiment, but in the environment I'm currently 
> looking at, we have neither DT nor ACPI: The kernel gets purely 
> configured via kernel command line. For other unenumerable artifacts on 
> the system, such as virtio-mmio platform devices, that works well enough 
> and also basically "hacks a kernel parameter" to specify the system layout.

Well... maybe it should also feed in a DT for those too?



smime.p7s
Description: S/MIME cryptographic signature
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH] swiotlb: Allow swiotlb to live at pre-defined address

2020-03-26 Thread Alexander Graf via iommu




On 26.03.20 18:05, Christoph Hellwig wrote:


On Thu, Mar 26, 2020 at 05:29:22PM +0100, Alexander Graf wrote:

The swiotlb is a very convenient fallback mechanism for bounce buffering of
DMAable data. It is usually used for the compatibility case where devices
can only DMA to a "low region".

However, in some scenarios this "low region" may be bound even more
heavily. For example, there are embedded system where only an SRAM region
is shared between device and CPU. There are also heterogeneous computing
scenarios where only a subset of RAM is cache coherent between the
components of the system. There are partitioning hypervisors, where
a "control VM" that implements device emulation has limited view into a
partition's memory for DMA capabilities due to safety concerns.

This patch adds a command line driven mechanism to move all DMA memory into
a predefined shared memory region which may or may not be part of the
physical address layout of the Operating System.

Ideally, the typical path to set this configuration would be through Device
Tree or ACPI, but neither of the two mechanisms is standardized yet. Also,
in the x86 MicroVM use case, we have neither ACPI nor Device Tree, but
instead configure the system purely through kernel command line options.

I'm sure other people will find the functionality useful going forward
though and extend it to be triggered by DT/ACPI in the future.


I'm totally against hacking in a kernel parameter for this.  We'll need
a proper documented DT or ACPI way.  


I'm with you on that sentiment, but in the environment I'm currently 
looking at, we have neither DT nor ACPI: The kernel gets purely 
configured via kernel command line. For other unenumerable artifacts on 
the system, such as virtio-mmio platform devices, that works well enough 
and also basically "hacks a kernel parameter" to specify the system layout.



We also need to feed this information
into the actual DMA bounce buffering decisions and not just the swiotlb
placement.


Care to elaborate a bit here? I was under the impression that 
"swiotlb=force" basically allows you to steer the DMA bounce buffering 
decisions already.



Thanks!

Alex



Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879



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


Re: [PATCH] swiotlb: Allow swiotlb to live at pre-defined address

2020-03-26 Thread Christoph Hellwig
On Thu, Mar 26, 2020 at 05:29:22PM +0100, Alexander Graf wrote:
> The swiotlb is a very convenient fallback mechanism for bounce buffering of
> DMAable data. It is usually used for the compatibility case where devices
> can only DMA to a "low region".
> 
> However, in some scenarios this "low region" may be bound even more
> heavily. For example, there are embedded system where only an SRAM region
> is shared between device and CPU. There are also heterogeneous computing
> scenarios where only a subset of RAM is cache coherent between the
> components of the system. There are partitioning hypervisors, where
> a "control VM" that implements device emulation has limited view into a
> partition's memory for DMA capabilities due to safety concerns.
> 
> This patch adds a command line driven mechanism to move all DMA memory into
> a predefined shared memory region which may or may not be part of the
> physical address layout of the Operating System.
> 
> Ideally, the typical path to set this configuration would be through Device
> Tree or ACPI, but neither of the two mechanisms is standardized yet. Also,
> in the x86 MicroVM use case, we have neither ACPI nor Device Tree, but
> instead configure the system purely through kernel command line options.
> 
> I'm sure other people will find the functionality useful going forward
> though and extend it to be triggered by DT/ACPI in the future.

I'm totally against hacking in a kernel parameter for this.  We'll need
a proper documented DT or ACPI way.  We also need to feed this information
into the actual DMA bounce buffering decisions and not just the swiotlb
placement.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


[PATCH] swiotlb: Allow swiotlb to live at pre-defined address

2020-03-26 Thread Alexander Graf via iommu
The swiotlb is a very convenient fallback mechanism for bounce buffering of
DMAable data. It is usually used for the compatibility case where devices
can only DMA to a "low region".

However, in some scenarios this "low region" may be bound even more
heavily. For example, there are embedded system where only an SRAM region
is shared between device and CPU. There are also heterogeneous computing
scenarios where only a subset of RAM is cache coherent between the
components of the system. There are partitioning hypervisors, where
a "control VM" that implements device emulation has limited view into a
partition's memory for DMA capabilities due to safety concerns.

This patch adds a command line driven mechanism to move all DMA memory into
a predefined shared memory region which may or may not be part of the
physical address layout of the Operating System.

Ideally, the typical path to set this configuration would be through Device
Tree or ACPI, but neither of the two mechanisms is standardized yet. Also,
in the x86 MicroVM use case, we have neither ACPI nor Device Tree, but
instead configure the system purely through kernel command line options.

I'm sure other people will find the functionality useful going forward
though and extend it to be triggered by DT/ACPI in the future.

Signed-off-by: Alexander Graf 
---
 Documentation/admin-guide/kernel-parameters.txt |  3 +-
 Documentation/x86/x86_64/boot-options.rst   |  4 ++-
 kernel/dma/swiotlb.c| 46 +++--
 3 files changed, 49 insertions(+), 4 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt 
b/Documentation/admin-guide/kernel-parameters.txt
index c07815d230bc..d085d55c3cbe 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -4785,11 +4785,12 @@
it if 0 is given (See 
Documentation/admin-guide/cgroup-v1/memory.rst)
 
swiotlb=[ARM,IA-64,PPC,MIPS,X86]
-   Format: {  | force | noforce }
+   Format: {  | force | noforce | addr= }
 -- Number of I/O TLB slabs
force -- force using of bounce buffers even if they
 wouldn't be automatically used by the kernel
noforce -- Never use bounce buffers (for debugging)
+   addr= -- Try to allocate SWIOTLB at defined 
address
 
switches=   [HW,M68k]
 
diff --git a/Documentation/x86/x86_64/boot-options.rst 
b/Documentation/x86/x86_64/boot-options.rst
index 2b98efb5ba7f..ca46c57b68c9 100644
--- a/Documentation/x86/x86_64/boot-options.rst
+++ b/Documentation/x86/x86_64/boot-options.rst
@@ -297,11 +297,13 @@ iommu options only relevant to the AMD GART hardware 
IOMMU:
 iommu options only relevant to the software bounce buffering (SWIOTLB) IOMMU
 implementation:
 
-swiotlb=[,force]
+swiotlb=[,force][,addr=]
   
 Prereserve that many 128K pages for the software IO bounce buffering.
   force
 Force all IO through the software TLB.
+  addr=
+Try to allocate SWIOTLB at defined address
 
 Settings for the IBM Calgary hardware IOMMU currently found in IBM
 pSeries and xSeries machines
diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index c19379fabd20..83da0caa2f93 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -46,6 +46,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #define CREATE_TRACE_POINTS
 #include 
@@ -102,6 +103,12 @@ unsigned int max_segment;
 #define INVALID_PHYS_ADDR (~(phys_addr_t)0)
 static phys_addr_t *io_tlb_orig_addr;
 
+/*
+ * The TLB phys addr may be defined on the command line. Store it here if it 
is.
+ */
+static phys_addr_t io_tlb_addr = INVALID_PHYS_ADDR;
+
+
 /*
  * Protect the above data structures in the map and unmap calls
  */
@@ -119,11 +126,23 @@ setup_io_tlb_npages(char *str)
}
if (*str == ',')
++str;
-   if (!strcmp(str, "force")) {
+   if (!strncmp(str, "force", 5)) {
swiotlb_force = SWIOTLB_FORCE;
-   } else if (!strcmp(str, "noforce")) {
+   str += 5;
+   } else if (!strncmp(str, "noforce", 7)) {
swiotlb_force = SWIOTLB_NO_FORCE;
io_tlb_nslabs = 1;
+   str += 7;
+   }
+
+   if (*str == ',')
+   ++str;
+   if (!strncmp(str, "addr=", 5)) {
+   char *addrstr = str + 5;
+
+   io_tlb_addr = kstrtoul(addrstr, 0, );
+   if (addrstr == str)
+   io_tlb_addr = INVALID_PHYS_ADDR;
}
 
return 0;
@@ -239,6 +258,25 @@ int __init swiotlb_init_with_tbl(char *tlb, unsigned long 
nslabs, int verbose)
return 0;
 }
 
+static int __init swiotlb_init_io(int verbose, unsigned long bytes)
+{
+   unsigned __iomem char *vstart;
+
+   if (io_tlb_addr == INVALID_PHYS_ADDR)
+ 

Re: [PATCH v2 1/3] iommu/uapi: Define uapi version and capabilities

2020-03-26 Thread Jacob Pan
Hi Christoph,

Thanks for the review. Please see my comments inline.

On Thu, 26 Mar 2020 02:23:16 -0700
Christoph Hellwig  wrote:

> On Wed, Mar 25, 2020 at 04:17:05PM -0700, Jacob Pan wrote:
> > Having a single UAPI version to govern the user-kernel data
> > structures makes compatibility check straightforward. On the
> > contrary, supporting combinations of multiple versions of the data
> > can be a nightmare to maintain.
> > 
> > This patch defines a unified UAPI version to be used for
> > compatibility checks between user and kernel.  
> 
> This is bullshit.  Version numbers don't scale and we've avoided them
> everywhere.  You need need flags specifying specific behavior.
> 
We have flags or the equivalent in each UAPI structures. The flags
are used for checking validity of extensions as well. And you are right
we can use them for checking specific behavior.

So we have two options here:
1. Have a overall version for a quick compatibility check while
starting a VM. If not compatible, we will stop guest SVA entirely.

2. Let each API calls check its own capabilities/flags at runtime. It
may fail later on and lead to more complex error handling.
For example, guest starts with SVA support, allocate a PASID, bind
guest PASID works great. Then when IO page fault happens, it try to do
page request service and found out certain flags are not compatible.
This case is more complex to handle.

I am guessing your proposal is #2. right?

Overall, we don;t expect much change to the UAPI other than adding some
vendor specific part of the union. Is the scalability concern based on
frequent changes?

> > +#define IOMMU_UAPI_VERSION 1
> > +static inline int iommu_get_uapi_version(void)
> > +{
> > +   return IOMMU_UAPI_VERSION;
> > +}  
> 
> Also inline functions like this in UAPI headers that actually get
> included by userspace programs simply don't work.

I will remove that, user can just use IOMMU_UAPI_VERSION directly.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v4 05/16] iommu: Rename struct iommu_param to dev_iommu

2020-03-26 Thread Greg Kroah-Hartman
On Thu, Mar 26, 2020 at 04:08:30PM +0100, Joerg Roedel wrote:
> From: Joerg Roedel 
> 
> The term dev_iommu aligns better with other existing structures and
> their accessor functions.
> 
> Cc: Greg Kroah-Hartman 
> Tested-by: Will Deacon  # arm-smmu
> Reviewed-by: Jean-Philippe Brucker 
> Signed-off-by: Joerg Roedel 
> ---
>  drivers/iommu/iommu.c  | 28 ++--
>  include/linux/device.h |  6 +++---
>  include/linux/iommu.h  |  4 ++--
>  3 files changed, 19 insertions(+), 19 deletions(-)

Reviewed-by: Greg Kroah-Hartman 
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v4 06/16] iommu: Move iommu_fwspec to struct dev_iommu

2020-03-26 Thread Greg Kroah-Hartman
On Thu, Mar 26, 2020 at 04:08:31PM +0100, Joerg Roedel wrote:
> From: Joerg Roedel 
> 
> Move the iommu_fwspec pointer in struct device into struct dev_iommu.
> This is a step in the effort to reduce the iommu related pointers in
> struct device to one.
> 
> Cc: Greg Kroah-Hartman 
> Tested-by: Will Deacon  # arm-smmu
> Reviewed-by: Jean-Philippe Brucker 
> Signed-off-by: Joerg Roedel 
> ---
>  drivers/iommu/iommu.c  |  3 +++
>  include/linux/device.h |  3 ---
>  include/linux/iommu.h  | 12 
>  3 files changed, 11 insertions(+), 7 deletions(-)

Reviewed-by: Greg Kroah-Hartman 

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


[PATCH v4 16/16] iommu: Move fwspec->iommu_priv to struct dev_iommu

2020-03-26 Thread Joerg Roedel
From: Joerg Roedel 

Move the pointer for iommu private data from struct iommu_fwspec to
struct dev_iommu.

Tested-by: Will Deacon  # arm-smmu
Reviewed-by: Jean-Philippe Brucker 
Signed-off-by: Joerg Roedel 
---
 include/linux/iommu.h | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 056900e75758..8c4d45fce042 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -369,6 +369,7 @@ struct iommu_fault_param {
  *
  * @fault_param: IOMMU detected device fault reporting data
  * @fwspec: IOMMU fwspec data
+ * @priv:   IOMMU Driver private data
  *
  * TODO: migrate other per device data pointers under iommu_dev_data, e.g.
  * struct iommu_group  *iommu_group;
@@ -377,6 +378,7 @@ struct dev_iommu {
struct mutex lock;
struct iommu_fault_param*fault_param;
struct iommu_fwspec *fwspec;
+   void*priv;
 };
 
 int  iommu_device_register(struct iommu_device *iommu);
@@ -589,7 +591,6 @@ struct iommu_group *fsl_mc_device_group(struct device *dev);
 struct iommu_fwspec {
const struct iommu_ops  *ops;
struct fwnode_handle*iommu_fwnode;
-   void*iommu_priv;
u32 flags;
u32 num_pasid_bits;
unsigned intnum_ids;
@@ -629,12 +630,12 @@ static inline void dev_iommu_fwspec_set(struct device 
*dev,
 
 static inline void *dev_iommu_priv_get(struct device *dev)
 {
-   return dev->iommu->fwspec->iommu_priv;
+   return dev->iommu->priv;
 }
 
 static inline void dev_iommu_priv_set(struct device *dev, void *priv)
 {
-   dev->iommu->fwspec->iommu_priv = priv;
+   dev->iommu->priv = priv;
 }
 
 int iommu_probe_device(struct device *dev);
-- 
2.17.1

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


[PATCH v4 09/16] iommu/arm-smmu-v3: Use accessor functions for iommu private data

2020-03-26 Thread Joerg Roedel
From: Joerg Roedel 

Make use of dev_iommu_priv_set/get() functions in the code.

Tested-by: Hanjun Guo 
Reviewed-by: Jean-Philippe Brucker 
Signed-off-by: Joerg Roedel 
---
 drivers/iommu/arm-smmu-v3.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index aa3ac2a03807..2b68498dfb66 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -2659,7 +2659,7 @@ static int arm_smmu_attach_dev(struct iommu_domain 
*domain, struct device *dev)
if (!fwspec)
return -ENOENT;
 
-   master = fwspec->iommu_priv;
+   master = dev_iommu_priv_get(dev);
smmu = master->smmu;
 
arm_smmu_detach_dev(master);
@@ -2795,7 +2795,7 @@ static int arm_smmu_add_device(struct device *dev)
if (!fwspec || fwspec->ops != _smmu_ops)
return -ENODEV;
 
-   if (WARN_ON_ONCE(fwspec->iommu_priv))
+   if (WARN_ON_ONCE(dev_iommu_priv_get(dev)))
return -EBUSY;
 
smmu = arm_smmu_get_by_fwnode(fwspec->iommu_fwnode);
@@ -2810,7 +2810,7 @@ static int arm_smmu_add_device(struct device *dev)
master->smmu = smmu;
master->sids = fwspec->ids;
master->num_sids = fwspec->num_ids;
-   fwspec->iommu_priv = master;
+   dev_iommu_priv_set(dev, master);
 
/* Check the SIDs are in range of the SMMU and our stream table */
for (i = 0; i < master->num_sids; i++) {
@@ -2852,7 +2852,7 @@ static int arm_smmu_add_device(struct device *dev)
iommu_device_unlink(>iommu, dev);
 err_free_master:
kfree(master);
-   fwspec->iommu_priv = NULL;
+   dev_iommu_priv_set(dev, NULL);
return ret;
 }
 
@@ -2865,7 +2865,7 @@ static void arm_smmu_remove_device(struct device *dev)
if (!fwspec || fwspec->ops != _smmu_ops)
return;
 
-   master = fwspec->iommu_priv;
+   master = dev_iommu_priv_get(dev);
smmu = master->smmu;
arm_smmu_detach_dev(master);
iommu_group_remove_device(dev);
-- 
2.17.1

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


[PATCH v4 13/16] iommu/mediatek: Use accessor functions for iommu private data

2020-03-26 Thread Joerg Roedel
From: Joerg Roedel 

Make use of dev_iommu_priv_set/get() functions.

Reviewed-by: Jean-Philippe Brucker 
Signed-off-by: Joerg Roedel 
---
 drivers/iommu/mtk_iommu.c| 13 ++---
 drivers/iommu/mtk_iommu_v1.c | 14 +++---
 2 files changed, 13 insertions(+), 14 deletions(-)

diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index 95945f467c03..5f4d6df59cf6 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -358,8 +358,8 @@ static void mtk_iommu_domain_free(struct iommu_domain 
*domain)
 static int mtk_iommu_attach_device(struct iommu_domain *domain,
   struct device *dev)
 {
+   struct mtk_iommu_data *data = dev_iommu_priv_get(dev);
struct mtk_iommu_domain *dom = to_mtk_domain(domain);
-   struct mtk_iommu_data *data = dev_iommu_fwspec_get(dev)->iommu_priv;
 
if (!data)
return -ENODEV;
@@ -378,7 +378,7 @@ static int mtk_iommu_attach_device(struct iommu_domain 
*domain,
 static void mtk_iommu_detach_device(struct iommu_domain *domain,
struct device *dev)
 {
-   struct mtk_iommu_data *data = dev_iommu_fwspec_get(dev)->iommu_priv;
+   struct mtk_iommu_data *data = dev_iommu_priv_get(dev);
 
if (!data)
return;
@@ -450,7 +450,7 @@ static int mtk_iommu_add_device(struct device *dev)
if (!fwspec || fwspec->ops != _iommu_ops)
return -ENODEV; /* Not a iommu client device */
 
-   data = fwspec->iommu_priv;
+   data = dev_iommu_priv_get(dev);
iommu_device_link(>iommu, dev);
 
group = iommu_group_get_for_dev(dev);
@@ -469,7 +469,7 @@ static void mtk_iommu_remove_device(struct device *dev)
if (!fwspec || fwspec->ops != _iommu_ops)
return;
 
-   data = fwspec->iommu_priv;
+   data = dev_iommu_priv_get(dev);
iommu_device_unlink(>iommu, dev);
 
iommu_group_remove_device(dev);
@@ -496,7 +496,6 @@ static struct iommu_group *mtk_iommu_device_group(struct 
device *dev)
 
 static int mtk_iommu_of_xlate(struct device *dev, struct of_phandle_args *args)
 {
-   struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
struct platform_device *m4updev;
 
if (args->args_count != 1) {
@@ -505,13 +504,13 @@ static int mtk_iommu_of_xlate(struct device *dev, struct 
of_phandle_args *args)
return -EINVAL;
}
 
-   if (!fwspec->iommu_priv) {
+   if (!dev_iommu_priv_get(dev)) {
/* Get the m4u device */
m4updev = of_find_device_by_node(args->np);
if (WARN_ON(!m4updev))
return -EINVAL;
 
-   fwspec->iommu_priv = platform_get_drvdata(m4updev);
+   dev_iommu_priv_set(dev, platform_get_drvdata(m4updev));
}
 
return iommu_fwspec_add_ids(dev, args->args, 1);
diff --git a/drivers/iommu/mtk_iommu_v1.c b/drivers/iommu/mtk_iommu_v1.c
index e93b94ecac45..a31be05601c9 100644
--- a/drivers/iommu/mtk_iommu_v1.c
+++ b/drivers/iommu/mtk_iommu_v1.c
@@ -263,8 +263,8 @@ static void mtk_iommu_domain_free(struct iommu_domain 
*domain)
 static int mtk_iommu_attach_device(struct iommu_domain *domain,
   struct device *dev)
 {
+   struct mtk_iommu_data *data = dev_iommu_priv_get(dev);
struct mtk_iommu_domain *dom = to_mtk_domain(domain);
-   struct mtk_iommu_data *data = dev_iommu_fwspec_get(dev)->iommu_priv;
int ret;
 
if (!data)
@@ -286,7 +286,7 @@ static int mtk_iommu_attach_device(struct iommu_domain 
*domain,
 static void mtk_iommu_detach_device(struct iommu_domain *domain,
struct device *dev)
 {
-   struct mtk_iommu_data *data = dev_iommu_fwspec_get(dev)->iommu_priv;
+   struct mtk_iommu_data *data = dev_iommu_priv_get(dev);
 
if (!data)
return;
@@ -387,20 +387,20 @@ static int mtk_iommu_create_mapping(struct device *dev,
return -EINVAL;
}
 
-   if (!fwspec->iommu_priv) {
+   if (!dev_iommu_priv_get(dev)) {
/* Get the m4u device */
m4updev = of_find_device_by_node(args->np);
if (WARN_ON(!m4updev))
return -EINVAL;
 
-   fwspec->iommu_priv = platform_get_drvdata(m4updev);
+   dev_iommu_priv_set(dev, platform_get_drvdata(m4updev));
}
 
ret = iommu_fwspec_add_ids(dev, args->args, 1);
if (ret)
return ret;
 
-   data = fwspec->iommu_priv;
+   data = dev_iommu_priv_get(dev);
m4udev = data->dev;
mtk_mapping = m4udev->archdata.iommu;
if (!mtk_mapping) {
@@ -459,7 +459,7 @@ static int mtk_iommu_add_device(struct device *dev)
if (err)
return err;
 
-   data = fwspec->iommu_priv;
+   data = dev_iommu_priv_get(dev);
mtk_mapping = data->dev->archdata.iommu;

[PATCH v4 08/16] iommu: Introduce accessors for iommu private data

2020-03-26 Thread Joerg Roedel
From: Joerg Roedel 

Add dev_iommu_priv_get/set() functions to access per-device iommu
private data. This makes it easier to move the pointer to a different
location.

Tested-by: Will Deacon  # arm-smmu
Reviewed-by: Jean-Philippe Brucker 
Signed-off-by: Joerg Roedel 
---
 include/linux/iommu.h | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index f5edc21a644d..056900e75758 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -627,6 +627,16 @@ static inline void dev_iommu_fwspec_set(struct device *dev,
dev->iommu->fwspec = fwspec;
 }
 
+static inline void *dev_iommu_priv_get(struct device *dev)
+{
+   return dev->iommu->fwspec->iommu_priv;
+}
+
+static inline void dev_iommu_priv_set(struct device *dev, void *priv)
+{
+   dev->iommu->fwspec->iommu_priv = priv;
+}
+
 int iommu_probe_device(struct device *dev);
 void iommu_release_device(struct device *dev);
 
-- 
2.17.1

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


[PATCH v4 02/16] ACPI/IORT: Remove direct access of dev->iommu_fwspec

2020-03-26 Thread Joerg Roedel
From: Joerg Roedel 

Use the accessor functions instead of directly dereferencing
dev->iommu_fwspec.

Tested-by: Hanjun Guo 
Reviewed-by: Jean-Philippe Brucker 
Signed-off-by: Joerg Roedel 
---
 drivers/acpi/arm64/iort.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index ed3d2d1a7ae9..7d04424189df 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -1015,6 +1015,7 @@ const struct iommu_ops *iort_iommu_configure(struct 
device *dev)
return ops;
 
if (dev_is_pci(dev)) {
+   struct iommu_fwspec *fwspec;
struct pci_bus *bus = to_pci_dev(dev)->bus;
struct iort_pci_alias_info info = { .dev = dev };
 
@@ -1027,8 +1028,9 @@ const struct iommu_ops *iort_iommu_configure(struct 
device *dev)
err = pci_for_each_dma_alias(to_pci_dev(dev),
 iort_pci_iommu_init, );
 
-   if (!err && iort_pci_rc_supports_ats(node))
-   dev->iommu_fwspec->flags |= IOMMU_FWSPEC_PCI_RC_ATS;
+   fwspec = dev_iommu_fwspec_get(dev);
+   if (fwspec && iort_pci_rc_supports_ats(node))
+   fwspec->flags |= IOMMU_FWSPEC_PCI_RC_ATS;
} else {
int i = 0;
 
-- 
2.17.1

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


[PATCH v4 12/16] iommu/renesas: Use accessor functions for iommu private data

2020-03-26 Thread Joerg Roedel
From: Joerg Roedel 

Make use of dev_iommu_priv_set/get() functions.

Reviewed-by: Jean-Philippe Brucker 
Signed-off-by: Joerg Roedel 
---
 drivers/iommu/ipmmu-vmsa.c | 7 ++-
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
index ecb3f9464dd5..310cf09feea3 100644
--- a/drivers/iommu/ipmmu-vmsa.c
+++ b/drivers/iommu/ipmmu-vmsa.c
@@ -89,9 +89,7 @@ static struct ipmmu_vmsa_domain *to_vmsa_domain(struct 
iommu_domain *dom)
 
 static struct ipmmu_vmsa_device *to_ipmmu(struct device *dev)
 {
-   struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
-
-   return fwspec ? fwspec->iommu_priv : NULL;
+   return dev_iommu_priv_get(dev);
 }
 
 #define TLB_LOOP_TIMEOUT   100 /* 100us */
@@ -727,14 +725,13 @@ static phys_addr_t ipmmu_iova_to_phys(struct iommu_domain 
*io_domain,
 static int ipmmu_init_platform_device(struct device *dev,
  struct of_phandle_args *args)
 {
-   struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
struct platform_device *ipmmu_pdev;
 
ipmmu_pdev = of_find_device_by_node(args->np);
if (!ipmmu_pdev)
return -ENODEV;
 
-   fwspec->iommu_priv = platform_get_drvdata(ipmmu_pdev);
+   dev_iommu_priv_set(dev, platform_get_drvdata(ipmmu_pdev));
 
return 0;
 }
-- 
2.17.1

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


[PATCH v4 10/16] iommu/arm-smmu: Refactor master_cfg/fwspec usage

2020-03-26 Thread Joerg Roedel
From: Robin Murphy 

In preparation for restructuring iommu_fwspec, refactor the way we
access the arm_smmu_master_cfg private data to be less dependent on
the current layout.

Signed-off-by: Robin Murphy 
Signed-off-by: Joerg Roedel 
---
 drivers/iommu/arm-smmu.c | 42 +---
 1 file changed, 22 insertions(+), 20 deletions(-)

diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 980aae73b45b..3cef2bfd6f3e 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -98,12 +98,10 @@ struct arm_smmu_master_cfg {
s16 smendx[];
 };
 #define INVALID_SMENDX -1
-#define __fwspec_cfg(fw) ((struct arm_smmu_master_cfg *)fw->iommu_priv)
-#define fwspec_smmu(fw)  (__fwspec_cfg(fw)->smmu)
-#define fwspec_smendx(fw, i) \
-   (i >= fw->num_ids ? INVALID_SMENDX : __fwspec_cfg(fw)->smendx[i])
-#define for_each_cfg_sme(fw, i, idx) \
-   for (i = 0; idx = fwspec_smendx(fw, i), i < fw->num_ids; ++i)
+#define cfg_smendx(cfg, fw, i) \
+   (i >= fw->num_ids ? INVALID_SMENDX : cfg->smendx[i])
+#define for_each_cfg_sme(cfg, fw, i, idx) \
+   for (i = 0; idx = cfg_smendx(cfg, fw, i), i < fw->num_ids; ++i)
 
 static bool using_legacy_binding, using_generic_binding;
 
@@ -1069,7 +1067,7 @@ static int arm_smmu_master_alloc_smes(struct device *dev)
 
mutex_lock(>stream_map_mutex);
/* Figure out a viable stream map entry allocation */
-   for_each_cfg_sme(fwspec, i, idx) {
+   for_each_cfg_sme(cfg, fwspec, i, idx) {
u16 sid = FIELD_GET(ARM_SMMU_SMR_ID, fwspec->ids[i]);
u16 mask = FIELD_GET(ARM_SMMU_SMR_MASK, fwspec->ids[i]);
 
@@ -1100,7 +1098,7 @@ static int arm_smmu_master_alloc_smes(struct device *dev)
iommu_group_put(group);
 
/* It worked! Now, poke the actual hardware */
-   for_each_cfg_sme(fwspec, i, idx) {
+   for_each_cfg_sme(cfg, fwspec, i, idx) {
arm_smmu_write_sme(smmu, idx);
smmu->s2crs[idx].group = group;
}
@@ -1117,14 +1115,14 @@ static int arm_smmu_master_alloc_smes(struct device 
*dev)
return ret;
 }
 
-static void arm_smmu_master_free_smes(struct iommu_fwspec *fwspec)
+static void arm_smmu_master_free_smes(struct arm_smmu_master_cfg *cfg,
+ struct iommu_fwspec *fwspec)
 {
-   struct arm_smmu_device *smmu = fwspec_smmu(fwspec);
-   struct arm_smmu_master_cfg *cfg = fwspec->iommu_priv;
+   struct arm_smmu_device *smmu = cfg->smmu;
int i, idx;
 
mutex_lock(>stream_map_mutex);
-   for_each_cfg_sme(fwspec, i, idx) {
+   for_each_cfg_sme(cfg, fwspec, i, idx) {
if (arm_smmu_free_sme(smmu, idx))
arm_smmu_write_sme(smmu, idx);
cfg->smendx[i] = INVALID_SMENDX;
@@ -1133,6 +1131,7 @@ static void arm_smmu_master_free_smes(struct iommu_fwspec 
*fwspec)
 }
 
 static int arm_smmu_domain_add_master(struct arm_smmu_domain *smmu_domain,
+ struct arm_smmu_master_cfg *cfg,
  struct iommu_fwspec *fwspec)
 {
struct arm_smmu_device *smmu = smmu_domain->smmu;
@@ -1146,7 +1145,7 @@ static int arm_smmu_domain_add_master(struct 
arm_smmu_domain *smmu_domain,
else
type = S2CR_TYPE_TRANS;
 
-   for_each_cfg_sme(fwspec, i, idx) {
+   for_each_cfg_sme(cfg, fwspec, i, idx) {
if (type == s2cr[idx].type && cbndx == s2cr[idx].cbndx)
continue;
 
@@ -1162,8 +1161,9 @@ static int arm_smmu_attach_dev(struct iommu_domain 
*domain, struct device *dev)
 {
int ret;
struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
-   struct arm_smmu_device *smmu;
struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
+   struct arm_smmu_master_cfg *cfg;
+   struct arm_smmu_device *smmu;
 
if (!fwspec || fwspec->ops != _smmu_ops) {
dev_err(dev, "cannot attach to SMMU, is it on the same bus?\n");
@@ -1177,10 +1177,11 @@ static int arm_smmu_attach_dev(struct iommu_domain 
*domain, struct device *dev)
 * domains, just say no (but more politely than by dereferencing NULL).
 * This should be at least a WARN_ON once that's sorted.
 */
-   if (!fwspec->iommu_priv)
+   cfg = fwspec->iommu_priv;
+   if (!cfg)
return -ENODEV;
 
-   smmu = fwspec_smmu(fwspec);
+   smmu = cfg->smmu;
 
ret = arm_smmu_rpm_get(smmu);
if (ret < 0)
@@ -1204,7 +1205,7 @@ static int arm_smmu_attach_dev(struct iommu_domain 
*domain, struct device *dev)
}
 
/* Looks ok, so add the device to the domain */
-   ret = arm_smmu_domain_add_master(smmu_domain, fwspec);
+   ret = arm_smmu_domain_add_master(smmu_domain, cfg, fwspec);
 
/*
 * Setup an autosuspend delay to avoid bouncing runpm state.
@@ 

[PATCH v4 01/16] iommu: Define dev_iommu_fwspec_get() for !CONFIG_IOMMU_API

2020-03-26 Thread Joerg Roedel
From: Joerg Roedel 

There are users outside of the IOMMU code that need to call that
function. Define it for !CONFIG_IOMMU_API too so that compilation does
not break.

Reported-by: kbuild test robot 
Reviewed-by: Jean-Philippe Brucker 
Signed-off-by: Joerg Roedel 
---
 include/linux/iommu.h | 4 
 1 file changed, 4 insertions(+)

diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index d1b5f4d98569..3c4ca041d7a2 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -1073,6 +1073,10 @@ static inline int iommu_sva_unbind_gpasid(struct 
iommu_domain *domain,
return -ENODEV;
 }
 
+static inline struct iommu_fwspec *dev_iommu_fwspec_get(struct device *dev)
+{
+   return NULL;
+}
 #endif /* CONFIG_IOMMU_API */
 
 #ifdef CONFIG_IOMMU_DEBUGFS
-- 
2.17.1

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


[PATCH v4 04/16] iommu/tegra-gart: Remove direct access of dev->iommu_fwspec

2020-03-26 Thread Joerg Roedel
From: Joerg Roedel 

Use the accessor functions instead of directly dereferencing
dev->iommu_fwspec.

Reviewed-by: Jean-Philippe Brucker 
Signed-off-by: Joerg Roedel 
---
 drivers/iommu/tegra-gart.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iommu/tegra-gart.c b/drivers/iommu/tegra-gart.c
index 3fb7ba72507d..db6559e8336f 100644
--- a/drivers/iommu/tegra-gart.c
+++ b/drivers/iommu/tegra-gart.c
@@ -247,7 +247,7 @@ static int gart_iommu_add_device(struct device *dev)
 {
struct iommu_group *group;
 
-   if (!dev->iommu_fwspec)
+   if (!dev_iommu_fwspec_get(dev))
return -ENODEV;
 
group = iommu_group_get_for_dev(dev);
-- 
2.17.1

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


[PATCH v4 06/16] iommu: Move iommu_fwspec to struct dev_iommu

2020-03-26 Thread Joerg Roedel
From: Joerg Roedel 

Move the iommu_fwspec pointer in struct device into struct dev_iommu.
This is a step in the effort to reduce the iommu related pointers in
struct device to one.

Cc: Greg Kroah-Hartman 
Tested-by: Will Deacon  # arm-smmu
Reviewed-by: Jean-Philippe Brucker 
Signed-off-by: Joerg Roedel 
---
 drivers/iommu/iommu.c  |  3 +++
 include/linux/device.h |  3 ---
 include/linux/iommu.h  | 12 
 3 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index beac2ef063dd..826a67ba247f 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -2405,6 +2405,9 @@ int iommu_fwspec_init(struct device *dev, struct 
fwnode_handle *iommu_fwnode,
if (fwspec)
return ops == fwspec->ops ? 0 : -EINVAL;
 
+   if (!dev_iommu_get(dev))
+   return -ENOMEM;
+
fwspec = kzalloc(sizeof(*fwspec), GFP_KERNEL);
if (!fwspec)
return -ENOMEM;
diff --git a/include/linux/device.h b/include/linux/device.h
index 405a8f11bec1..fc1427ab7e85 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -42,7 +42,6 @@ struct device_node;
 struct fwnode_handle;
 struct iommu_ops;
 struct iommu_group;
-struct iommu_fwspec;
 struct dev_pin_info;
 struct dev_iommu;
 
@@ -513,7 +512,6 @@ struct dev_links_info {
  * gone away. This should be set by the allocator of the
  * device (i.e. the bus driver that discovered the device).
  * @iommu_group: IOMMU group the device belongs to.
- * @iommu_fwspec: IOMMU-specific properties supplied by firmware.
  * @iommu: Per device generic IOMMU runtime data
  *
  * @offline_disabled: If set, the device is permanently online.
@@ -613,7 +611,6 @@ struct device {
 
void(*release)(struct device *dev);
struct iommu_group  *iommu_group;
-   struct iommu_fwspec *iommu_fwspec;
struct dev_iommu*iommu;
 
booloffline_disabled:1;
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 1c9fa5c1174b..f5edc21a644d 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -368,14 +368,15 @@ struct iommu_fault_param {
  * struct dev_iommu - Collection of per-device IOMMU data
  *
  * @fault_param: IOMMU detected device fault reporting data
+ * @fwspec: IOMMU fwspec data
  *
  * TODO: migrate other per device data pointers under iommu_dev_data, e.g.
  * struct iommu_group  *iommu_group;
- * struct iommu_fwspec *iommu_fwspec;
  */
 struct dev_iommu {
struct mutex lock;
-   struct iommu_fault_param *fault_param;
+   struct iommu_fault_param*fault_param;
+   struct iommu_fwspec *fwspec;
 };
 
 int  iommu_device_register(struct iommu_device *iommu);
@@ -614,13 +615,16 @@ const struct iommu_ops *iommu_ops_from_fwnode(struct 
fwnode_handle *fwnode);
 
 static inline struct iommu_fwspec *dev_iommu_fwspec_get(struct device *dev)
 {
-   return dev->iommu_fwspec;
+   if (dev->iommu)
+   return dev->iommu->fwspec;
+   else
+   return NULL;
 }
 
 static inline void dev_iommu_fwspec_set(struct device *dev,
struct iommu_fwspec *fwspec)
 {
-   dev->iommu_fwspec = fwspec;
+   dev->iommu->fwspec = fwspec;
 }
 
 int iommu_probe_device(struct device *dev);
-- 
2.17.1

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


[PATCH v4 14/16] iommu/qcom: Use accessor functions for iommu private data

2020-03-26 Thread Joerg Roedel
From: Joerg Roedel 

Make use of dev_iommu_priv_set/get() functions.

Signed-off-by: Joerg Roedel 
---
 drivers/iommu/qcom_iommu.c | 61 ++
 1 file changed, 36 insertions(+), 25 deletions(-)

diff --git a/drivers/iommu/qcom_iommu.c b/drivers/iommu/qcom_iommu.c
index 4328da0b0a9f..824a9e85fac6 100644
--- a/drivers/iommu/qcom_iommu.c
+++ b/drivers/iommu/qcom_iommu.c
@@ -74,16 +74,19 @@ static struct qcom_iommu_domain 
*to_qcom_iommu_domain(struct iommu_domain *dom)
 
 static const struct iommu_ops qcom_iommu_ops;
 
-static struct qcom_iommu_dev * to_iommu(struct iommu_fwspec *fwspec)
+static struct qcom_iommu_dev * to_iommu(struct device *dev)
 {
+   struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
+
if (!fwspec || fwspec->ops != _iommu_ops)
return NULL;
-   return fwspec->iommu_priv;
+
+   return dev_iommu_priv_get(dev);
 }
 
-static struct qcom_iommu_ctx * to_ctx(struct iommu_fwspec *fwspec, unsigned 
asid)
+static struct qcom_iommu_ctx * to_ctx(struct device *dev, unsigned asid)
 {
-   struct qcom_iommu_dev *qcom_iommu = to_iommu(fwspec);
+   struct qcom_iommu_dev *qcom_iommu = to_iommu(dev);
if (!qcom_iommu)
return NULL;
return qcom_iommu->ctxs[asid - 1];
@@ -115,11 +118,14 @@ iommu_readq(struct qcom_iommu_ctx *ctx, unsigned reg)
 
 static void qcom_iommu_tlb_sync(void *cookie)
 {
-   struct iommu_fwspec *fwspec = cookie;
+   struct iommu_fwspec *fwspec;
+   struct device *dev = cookie;
unsigned i;
 
+   fwspec = dev_iommu_fwspec_get(dev);
+
for (i = 0; i < fwspec->num_ids; i++) {
-   struct qcom_iommu_ctx *ctx = to_ctx(fwspec, fwspec->ids[i]);
+   struct qcom_iommu_ctx *ctx = to_ctx(dev, fwspec->ids[i]);
unsigned int val, ret;
 
iommu_writel(ctx, ARM_SMMU_CB_TLBSYNC, 0);
@@ -133,11 +139,14 @@ static void qcom_iommu_tlb_sync(void *cookie)
 
 static void qcom_iommu_tlb_inv_context(void *cookie)
 {
-   struct iommu_fwspec *fwspec = cookie;
+   struct device *dev = cookie;
+   struct iommu_fwspec *fwspec;
unsigned i;
 
+   fwspec = dev_iommu_fwspec_get(dev);
+
for (i = 0; i < fwspec->num_ids; i++) {
-   struct qcom_iommu_ctx *ctx = to_ctx(fwspec, fwspec->ids[i]);
+   struct qcom_iommu_ctx *ctx = to_ctx(dev, fwspec->ids[i]);
iommu_writel(ctx, ARM_SMMU_CB_S1_TLBIASID, ctx->asid);
}
 
@@ -147,13 +156,16 @@ static void qcom_iommu_tlb_inv_context(void *cookie)
 static void qcom_iommu_tlb_inv_range_nosync(unsigned long iova, size_t size,
size_t granule, bool leaf, void 
*cookie)
 {
-   struct iommu_fwspec *fwspec = cookie;
+   struct device *dev = cookie;
+   struct iommu_fwspec *fwspec;
unsigned i, reg;
 
reg = leaf ? ARM_SMMU_CB_S1_TLBIVAL : ARM_SMMU_CB_S1_TLBIVA;
 
+   fwspec = dev_iommu_fwspec_get(dev);
+
for (i = 0; i < fwspec->num_ids; i++) {
-   struct qcom_iommu_ctx *ctx = to_ctx(fwspec, fwspec->ids[i]);
+   struct qcom_iommu_ctx *ctx = to_ctx(dev, fwspec->ids[i]);
size_t s = size;
 
iova = (iova >> 12) << 12;
@@ -222,9 +234,10 @@ static irqreturn_t qcom_iommu_fault(int irq, void *dev)
 
 static int qcom_iommu_init_domain(struct iommu_domain *domain,
  struct qcom_iommu_dev *qcom_iommu,
- struct iommu_fwspec *fwspec)
+ struct device *dev)
 {
struct qcom_iommu_domain *qcom_domain = to_qcom_iommu_domain(domain);
+   struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
struct io_pgtable_ops *pgtbl_ops;
struct io_pgtable_cfg pgtbl_cfg;
int i, ret = 0;
@@ -243,7 +256,7 @@ static int qcom_iommu_init_domain(struct iommu_domain 
*domain,
};
 
qcom_domain->iommu = qcom_iommu;
-   pgtbl_ops = alloc_io_pgtable_ops(ARM_32_LPAE_S1, _cfg, fwspec);
+   pgtbl_ops = alloc_io_pgtable_ops(ARM_32_LPAE_S1, _cfg, dev);
if (!pgtbl_ops) {
dev_err(qcom_iommu->dev, "failed to allocate pagetable ops\n");
ret = -ENOMEM;
@@ -256,7 +269,7 @@ static int qcom_iommu_init_domain(struct iommu_domain 
*domain,
domain->geometry.force_aperture = true;
 
for (i = 0; i < fwspec->num_ids; i++) {
-   struct qcom_iommu_ctx *ctx = to_ctx(fwspec, fwspec->ids[i]);
+   struct qcom_iommu_ctx *ctx = to_ctx(dev, fwspec->ids[i]);
 
if (!ctx->secure_init) {
ret = qcom_scm_restore_sec_cfg(qcom_iommu->sec_id, 
ctx->asid);
@@ -363,8 +376,7 @@ static void qcom_iommu_domain_free(struct iommu_domain 
*domain)
 
 static int qcom_iommu_attach_dev(struct iommu_domain *domain, struct device 
*dev)
 {
-   struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
-   

[PATCH v4 11/16] iommu/arm-smmu: Use accessor functions for iommu private data

2020-03-26 Thread Joerg Roedel
From: Joerg Roedel 

Make use of dev_iommu_priv_set/get() functions and simplify the code
where possible with this change.

Tested-by: Will Deacon  # arm-smmu
Signed-off-by: Joerg Roedel 
---
 drivers/iommu/arm-smmu.c | 17 +
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 3cef2bfd6f3e..a6a5796e9c41 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -1059,7 +1059,7 @@ static bool arm_smmu_free_sme(struct arm_smmu_device 
*smmu, int idx)
 static int arm_smmu_master_alloc_smes(struct device *dev)
 {
struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
-   struct arm_smmu_master_cfg *cfg = fwspec->iommu_priv;
+   struct arm_smmu_master_cfg *cfg = dev_iommu_priv_get(dev);
struct arm_smmu_device *smmu = cfg->smmu;
struct arm_smmu_smr *smrs = smmu->smrs;
struct iommu_group *group;
@@ -1159,11 +1159,11 @@ static int arm_smmu_domain_add_master(struct 
arm_smmu_domain *smmu_domain,
 
 static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
 {
-   int ret;
-   struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
+   struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
struct arm_smmu_master_cfg *cfg;
struct arm_smmu_device *smmu;
+   int ret;
 
if (!fwspec || fwspec->ops != _smmu_ops) {
dev_err(dev, "cannot attach to SMMU, is it on the same bus?\n");
@@ -1177,7 +1177,7 @@ static int arm_smmu_attach_dev(struct iommu_domain 
*domain, struct device *dev)
 * domains, just say no (but more politely than by dereferencing NULL).
 * This should be at least a WARN_ON once that's sorted.
 */
-   cfg = fwspec->iommu_priv;
+   cfg = dev_iommu_priv_get(dev);
if (!cfg)
return -ENODEV;
 
@@ -1430,7 +1430,7 @@ static int arm_smmu_add_device(struct device *dev)
goto out_free;
 
cfg->smmu = smmu;
-   fwspec->iommu_priv = cfg;
+   dev_iommu_priv_set(dev, cfg);
while (i--)
cfg->smendx[i] = INVALID_SMENDX;
 
@@ -1468,7 +1468,7 @@ static void arm_smmu_remove_device(struct device *dev)
if (!fwspec || fwspec->ops != _smmu_ops)
return;
 
-   cfg  = fwspec->iommu_priv;
+   cfg  = dev_iommu_priv_get(dev);
smmu = cfg->smmu;
 
ret = arm_smmu_rpm_get(smmu);
@@ -1480,15 +1480,16 @@ static void arm_smmu_remove_device(struct device *dev)
 
arm_smmu_rpm_put(smmu);
 
+   dev_iommu_priv_set(dev, NULL);
iommu_group_remove_device(dev);
-   kfree(fwspec->iommu_priv);
+   kfree(cfg);
iommu_fwspec_free(dev);
 }
 
 static struct iommu_group *arm_smmu_device_group(struct device *dev)
 {
+   struct arm_smmu_master_cfg *cfg = dev_iommu_priv_get(dev);
struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
-   struct arm_smmu_master_cfg *cfg = fwspec->iommu_priv;
struct arm_smmu_device *smmu = cfg->smmu;
struct iommu_group *group = NULL;
int i, idx;
-- 
2.17.1

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


[PATCH v4 07/16] iommu/arm-smmu: Fix uninitilized variable warning

2020-03-26 Thread Joerg Roedel
From: Joerg Roedel 

Some unrelated changes in the iommu code caused a new warning to
appear in the arm-smmu driver:

  CC  drivers/iommu/arm-smmu.o
drivers/iommu/arm-smmu.c: In function 'arm_smmu_add_device':
drivers/iommu/arm-smmu.c:1441:2: warning: 'smmu' may be used uninitialized in 
this function [-Wmaybe-uninitialized]
  arm_smmu_rpm_put(smmu);
  ^~

The warning is a false positive, but initialize the variable to NULL
to get rid of it.

Tested-by: Will Deacon  # arm-smmu
Reviewed-by: Jean-Philippe Brucker 
Signed-off-by: Joerg Roedel 
---
 drivers/iommu/arm-smmu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 16c4b87af42b..980aae73b45b 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -1383,7 +1383,7 @@ struct arm_smmu_device *arm_smmu_get_by_fwnode(struct 
fwnode_handle *fwnode)
 
 static int arm_smmu_add_device(struct device *dev)
 {
-   struct arm_smmu_device *smmu;
+   struct arm_smmu_device *smmu = NULL;
struct arm_smmu_master_cfg *cfg;
struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
int i, ret;
-- 
2.17.1

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


[PATCH v4 15/16] iommu/virtio: Use accessor functions for iommu private data

2020-03-26 Thread Joerg Roedel
From: Joerg Roedel 

Make use of dev_iommu_priv_set/get() functions.

Reviewed-by: Jean-Philippe Brucker 
Signed-off-by: Joerg Roedel 
---
 drivers/iommu/virtio-iommu.c | 11 +--
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c
index cce329d71fba..8ead57f031f5 100644
--- a/drivers/iommu/virtio-iommu.c
+++ b/drivers/iommu/virtio-iommu.c
@@ -466,7 +466,7 @@ static int viommu_probe_endpoint(struct viommu_dev *viommu, 
struct device *dev)
struct virtio_iommu_req_probe *probe;
struct virtio_iommu_probe_property *prop;
struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
-   struct viommu_endpoint *vdev = fwspec->iommu_priv;
+   struct viommu_endpoint *vdev = dev_iommu_priv_get(dev);
 
if (!fwspec->num_ids)
return -EINVAL;
@@ -648,7 +648,7 @@ static int viommu_attach_dev(struct iommu_domain *domain, 
struct device *dev)
int ret = 0;
struct virtio_iommu_req_attach req;
struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
-   struct viommu_endpoint *vdev = fwspec->iommu_priv;
+   struct viommu_endpoint *vdev = dev_iommu_priv_get(dev);
struct viommu_domain *vdomain = to_viommu_domain(domain);
 
mutex_lock(>mutex);
@@ -807,8 +807,7 @@ static void viommu_iotlb_sync(struct iommu_domain *domain,
 static void viommu_get_resv_regions(struct device *dev, struct list_head *head)
 {
struct iommu_resv_region *entry, *new_entry, *msi = NULL;
-   struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
-   struct viommu_endpoint *vdev = fwspec->iommu_priv;
+   struct viommu_endpoint *vdev = dev_iommu_priv_get(dev);
int prot = IOMMU_WRITE | IOMMU_NOEXEC | IOMMU_MMIO;
 
list_for_each_entry(entry, >resv_regions, list) {
@@ -876,7 +875,7 @@ static int viommu_add_device(struct device *dev)
vdev->dev = dev;
vdev->viommu = viommu;
INIT_LIST_HEAD(>resv_regions);
-   fwspec->iommu_priv = vdev;
+   dev_iommu_priv_set(dev, vdev);
 
if (viommu->probe_size) {
/* Get additional information for this endpoint */
@@ -920,7 +919,7 @@ static void viommu_remove_device(struct device *dev)
if (!fwspec || fwspec->ops != _ops)
return;
 
-   vdev = fwspec->iommu_priv;
+   vdev = dev_iommu_priv_get(dev);
 
iommu_group_remove_device(dev);
iommu_device_unlink(>viommu->iommu, dev);
-- 
2.17.1

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


[PATCH v4 03/16] drm/msm/mdp5: Remove direct access of dev->iommu_fwspec

2020-03-26 Thread Joerg Roedel
From: Joerg Roedel 

Use the accessor functions instead of directly dereferencing
dev->iommu_fwspec.

Reviewed-by: Jean-Philippe Brucker 
Signed-off-by: Joerg Roedel 
---
 drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c 
b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
index e43ecd4be10a..1252e1d76340 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
@@ -725,7 +725,7 @@ struct msm_kms *mdp5_kms_init(struct drm_device *dev)
 
if (config->platform.iommu) {
iommu_dev = >dev;
-   if (!iommu_dev->iommu_fwspec)
+   if (!dev_iommu_fwspec_get(iommu_dev))
iommu_dev = iommu_dev->parent;
 
aspace = msm_gem_address_space_create(iommu_dev,
-- 
2.17.1

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


[PATCH v4 05/16] iommu: Rename struct iommu_param to dev_iommu

2020-03-26 Thread Joerg Roedel
From: Joerg Roedel 

The term dev_iommu aligns better with other existing structures and
their accessor functions.

Cc: Greg Kroah-Hartman 
Tested-by: Will Deacon  # arm-smmu
Reviewed-by: Jean-Philippe Brucker 
Signed-off-by: Joerg Roedel 
---
 drivers/iommu/iommu.c  | 28 ++--
 include/linux/device.h |  6 +++---
 include/linux/iommu.h  |  4 ++--
 3 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 3e3528436e0b..beac2ef063dd 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -152,9 +152,9 @@ void iommu_device_unregister(struct iommu_device *iommu)
 }
 EXPORT_SYMBOL_GPL(iommu_device_unregister);
 
-static struct iommu_param *iommu_get_dev_param(struct device *dev)
+static struct dev_iommu *dev_iommu_get(struct device *dev)
 {
-   struct iommu_param *param = dev->iommu_param;
+   struct dev_iommu *param = dev->iommu;
 
if (param)
return param;
@@ -164,14 +164,14 @@ static struct iommu_param *iommu_get_dev_param(struct 
device *dev)
return NULL;
 
mutex_init(>lock);
-   dev->iommu_param = param;
+   dev->iommu = param;
return param;
 }
 
-static void iommu_free_dev_param(struct device *dev)
+static void dev_iommu_free(struct device *dev)
 {
-   kfree(dev->iommu_param);
-   dev->iommu_param = NULL;
+   kfree(dev->iommu);
+   dev->iommu = NULL;
 }
 
 int iommu_probe_device(struct device *dev)
@@ -183,7 +183,7 @@ int iommu_probe_device(struct device *dev)
if (!ops)
return -EINVAL;
 
-   if (!iommu_get_dev_param(dev))
+   if (!dev_iommu_get(dev))
return -ENOMEM;
 
if (!try_module_get(ops->owner)) {
@@ -200,7 +200,7 @@ int iommu_probe_device(struct device *dev)
 err_module_put:
module_put(ops->owner);
 err_free_dev_param:
-   iommu_free_dev_param(dev);
+   dev_iommu_free(dev);
return ret;
 }
 
@@ -211,9 +211,9 @@ void iommu_release_device(struct device *dev)
if (dev->iommu_group)
ops->remove_device(dev);
 
-   if (dev->iommu_param) {
+   if (dev->iommu) {
module_put(ops->owner);
-   iommu_free_dev_param(dev);
+   dev_iommu_free(dev);
}
 }
 
@@ -972,7 +972,7 @@ int iommu_register_device_fault_handler(struct device *dev,
iommu_dev_fault_handler_t handler,
void *data)
 {
-   struct iommu_param *param = dev->iommu_param;
+   struct dev_iommu *param = dev->iommu;
int ret = 0;
 
if (!param)
@@ -1015,7 +1015,7 @@ EXPORT_SYMBOL_GPL(iommu_register_device_fault_handler);
  */
 int iommu_unregister_device_fault_handler(struct device *dev)
 {
-   struct iommu_param *param = dev->iommu_param;
+   struct dev_iommu *param = dev->iommu;
int ret = 0;
 
if (!param)
@@ -1055,7 +1055,7 @@ EXPORT_SYMBOL_GPL(iommu_unregister_device_fault_handler);
  */
 int iommu_report_device_fault(struct device *dev, struct iommu_fault_event 
*evt)
 {
-   struct iommu_param *param = dev->iommu_param;
+   struct dev_iommu *param = dev->iommu;
struct iommu_fault_event *evt_pending = NULL;
struct iommu_fault_param *fparam;
int ret = 0;
@@ -1104,7 +1104,7 @@ int iommu_page_response(struct device *dev,
int ret = -EINVAL;
struct iommu_fault_event *evt;
struct iommu_fault_page_request *prm;
-   struct iommu_param *param = dev->iommu_param;
+   struct dev_iommu *param = dev->iommu;
struct iommu_domain *domain = iommu_get_domain_for_dev(dev);
 
if (!domain || !domain->ops->page_response)
diff --git a/include/linux/device.h b/include/linux/device.h
index fa04dfd22bbc..405a8f11bec1 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -44,7 +44,7 @@ struct iommu_ops;
 struct iommu_group;
 struct iommu_fwspec;
 struct dev_pin_info;
-struct iommu_param;
+struct dev_iommu;
 
 /**
  * struct subsys_interface - interfaces to device functions
@@ -514,7 +514,7 @@ struct dev_links_info {
  * device (i.e. the bus driver that discovered the device).
  * @iommu_group: IOMMU group the device belongs to.
  * @iommu_fwspec: IOMMU-specific properties supplied by firmware.
- * @iommu_param: Per device generic IOMMU runtime data
+ * @iommu: Per device generic IOMMU runtime data
  *
  * @offline_disabled: If set, the device is permanently online.
  * @offline:   Set after successful invocation of bus type's .offline().
@@ -614,7 +614,7 @@ struct device {
void(*release)(struct device *dev);
struct iommu_group  *iommu_group;
struct iommu_fwspec *iommu_fwspec;
-   struct iommu_param  *iommu_param;
+   struct dev_iommu*iommu;
 
booloffline_disabled:1;
booloffline:1;
diff --git 

[PATCH v4 00/16] iommu: Move iommu_fwspec out of 'struct device'

2020-03-26 Thread Joerg Roedel
Hi,

here is the updated version of the changes to move iommu_fwspec out of
'struct device'. Previous versions of this patch-set can be found here:

v3: https://lore.kernel.org/lkml/20200320091414.3941-1-j...@8bytes.org/

v2: https://lore.kernel.org/lkml/20200310091229.29830-1-j...@8bytes.org/

v1: https://lore.kernel.org/lkml/20200228150820.15340-1-j...@8bytes.org/

Changes to v2:

- Addressed Robins review comments

- Added Robins patch to optimize arm-smmu changes

- Rebased to v5.6-rc7

Please review.

Thanks,

Joerg

Joerg Roedel (15):
  iommu: Define dev_iommu_fwspec_get() for !CONFIG_IOMMU_API
  ACPI/IORT: Remove direct access of dev->iommu_fwspec
  drm/msm/mdp5: Remove direct access of dev->iommu_fwspec
  iommu/tegra-gart: Remove direct access of dev->iommu_fwspec
  iommu: Rename struct iommu_param to dev_iommu
  iommu: Move iommu_fwspec to struct dev_iommu
  iommu/arm-smmu: Fix uninitilized variable warning
  iommu: Introduce accessors for iommu private data
  iommu/arm-smmu-v3: Use accessor functions for iommu private data
  iommu/arm-smmu: Use accessor functions for iommu private data
  iommu/renesas: Use accessor functions for iommu private data
  iommu/mediatek: Use accessor functions for iommu private data
  iommu/qcom: Use accessor functions for iommu private data
  iommu/virtio: Use accessor functions for iommu private data
  iommu: Move fwspec->iommu_priv to struct dev_iommu

Robin Murphy (1):
  iommu/arm-smmu: Refactor master_cfg/fwspec usage

 drivers/acpi/arm64/iort.c|  6 ++-
 drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c |  2 +-
 drivers/iommu/arm-smmu-v3.c  | 10 ++--
 drivers/iommu/arm-smmu.c | 55 +++--
 drivers/iommu/iommu.c| 31 ++--
 drivers/iommu/ipmmu-vmsa.c   |  7 +--
 drivers/iommu/mtk_iommu.c| 13 +++--
 drivers/iommu/mtk_iommu_v1.c | 14 +++---
 drivers/iommu/qcom_iommu.c   | 61 ++--
 drivers/iommu/tegra-gart.c   |  2 +-
 drivers/iommu/virtio-iommu.c | 11 ++---
 include/linux/device.h   |  9 ++--
 include/linux/iommu.h| 33 ++---
 13 files changed, 142 insertions(+), 112 deletions(-)

-- 
2.17.1

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


Re: [PATCH v3 10/15] iommu/arm-smmu: Use accessor functions for iommu private data

2020-03-26 Thread Joerg Roedel
Hi Robin,

On Wed, Mar 25, 2020 at 12:31:46PM +, Robin Murphy wrote:
> Oops, sorry - as you might imagine I'm not in my normal workflow :)

No problem, nobody is right now :)

> Let me rebase that onto something actually in your tree (rather than
> whatever detached HEAD this is checked out out on my laptop...) and try
> resending it properly.

Got it, thanks. Added to the next version of the patch-set which I will
send out shortly.

> Cool, let me know if you need a hand with all the *_iommu_configure() stuff
> - I still have plans for overhauling that lot anyway, but not imminently, so
> it probably is worthwhile to do the straightforward housekeeping first.

Okay, I'll get back to you if I need help with the conversion.

Thanks,

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


RE: [PATCH v1 0/8] vfio: expose virtual Shared Virtual Addressing to VMs

2020-03-26 Thread Liu, Yi L
> From: Liu, Yi L 
> Sent: Sunday, March 22, 2020 8:32 PM
> To: alex.william...@redhat.com; eric.au...@redhat.com
> Subject: [PATCH v1 0/8] vfio: expose virtual Shared Virtual Addressing to VMs
> 
> From: Liu Yi L 
> 
> Shared Virtual Addressing (SVA), a.k.a, Shared Virtual Memory (SVM) on
> Intel platforms allows address space sharing between device DMA and
> applications. SVA can reduce programming complexity and enhance security.
> 
> This VFIO series is intended to expose SVA usage to VMs. i.e. Sharing
> guest application address space with passthru devices. This is called
> vSVA in this series. The whole vSVA enabling requires QEMU/VFIO/IOMMU
> changes. For IOMMU and QEMU changes, they are in separate series (listed
> in the "Related series").
> 
> The high-level architecture for SVA virtualization is as below, the key
> design of vSVA support is to utilize the dual-stage IOMMU translation (
> also known as IOMMU nesting translation) capability in host IOMMU.
> 
> 
> .-.  .---.
> |   vIOMMU|  | Guest process CR3, FL only|
> | |  '---'
> ./
> | PASID Entry |--- PASID cache flush -
> '-'   |
> | |   V
> | |CR3 in GPA
> '-'
> Guest
> --| Shadow |--|
>   vv  v
> Host
> .-.  .--.
> |   pIOMMU|  | Bind FL for GVA-GPA  |
> | |  '--'
> ./  |
> | PASID Entry | V (Nested xlate)
> '\.--.
> | |   |SL for GPA-HPA, default domain|
> | |   '--'
> '-'
> Where:
>  - FL = First level/stage one page tables
>  - SL = Second level/stage two page tables
> 
> There are roughly four parts in this patchset which are
> corresponding to the basic vSVA support for PCI device
> assignment
>  1. vfio support for PASID allocation and free for VMs
>  2. vfio support for guest page table binding request from VMs
>  3. vfio support for IOMMU cache invalidation from VMs
>  4. vfio support for vSVA usage on IOMMU-backed mdevs
> 
> The complete vSVA kernel upstream patches are divided into three phases:
> 1. Common APIs and PCI device direct assignment
> 2. IOMMU-backed Mediated Device assignment
> 3. Page Request Services (PRS) support
> 
> This patchset is aiming for the phase 1 and phase 2, and based on Jacob's
> below series.
> [PATCH V10 00/11] Nested Shared Virtual Address (SVA) VT-d support:
> https://lkml.org/lkml/2020/3/20/1172
> 
> Complete set for current vSVA can be found in below branch.
> https://github.com/luxis1999/linux-vsva.git: vsva-linux-5.6-rc6
> 
> The corresponding QEMU patch series is as below, complete QEMU set can be
> found in below branch.
> [PATCH v1 00/22] intel_iommu: expose Shared Virtual Addressing to VMs
> complete QEMU set can be found in below link:
> https://github.com/luxis1999/qemu.git: sva_vtd_v10_v1

The ioasid extension is in the below link.

https://lkml.org/lkml/2020/3/25/874

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


Re: [PATCH v2 3/3] iommu/virtio: Reject IOMMU page granule larger than PAGE_SIZE

2020-03-26 Thread Robin Murphy

On 2020-03-26 9:35 am, Jean-Philippe Brucker wrote:

We don't currently support IOMMUs with a page granule larger than the
system page size. The IOVA allocator has a BUG_ON() in this case, and
VFIO has a WARN_ON().

Removing these obstacles ranges doesn't seem possible without major
changes to the DMA API and VFIO. Some callers of iommu_map(), for
example, want to map multiple page-aligned regions adjacent to each
others for scatter-gather purposes. Even in simple DMA API uses, a call
to dma_map_page() would let the endpoint access neighbouring memory. And
VFIO users cannot ensure that their virtual address buffer is physically
contiguous at the IOMMU granule.

Rather than triggering the IOVA BUG_ON() on mismatched page sizes, abort
the vdomain finalise() with an error message. We could simply abort the
viommu probe(), but an upcoming extension to virtio-iommu will allow
setting different page masks for each endpoint.


Reviewed-by: Robin Murphy 


Reported-by: Bharat Bhushan 
Signed-off-by: Jean-Philippe Brucker 
---
v1->v2: Move to vdomain_finalise(), improve commit message
---
  drivers/iommu/virtio-iommu.c | 14 --
  1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c
index 5eed75cd121f..750f69c49b95 100644
--- a/drivers/iommu/virtio-iommu.c
+++ b/drivers/iommu/virtio-iommu.c
@@ -607,12 +607,22 @@ static struct iommu_domain *viommu_domain_alloc(unsigned 
type)
return >domain;
  }
  
-static int viommu_domain_finalise(struct viommu_dev *viommu,

+static int viommu_domain_finalise(struct viommu_endpoint *vdev,
  struct iommu_domain *domain)
  {
int ret;
+   unsigned long viommu_page_size;
+   struct viommu_dev *viommu = vdev->viommu;
struct viommu_domain *vdomain = to_viommu_domain(domain);
  
+	viommu_page_size = 1UL << __ffs(viommu->pgsize_bitmap);

+   if (viommu_page_size > PAGE_SIZE) {
+   dev_err(vdev->dev,
+   "granule 0x%lx larger than system page size 0x%lx\n",
+   viommu_page_size, PAGE_SIZE);
+   return -EINVAL;
+   }
+
ret = ida_alloc_range(>domain_ids, viommu->first_domain,
  viommu->last_domain, GFP_KERNEL);
if (ret < 0)
@@ -659,7 +669,7 @@ static int viommu_attach_dev(struct iommu_domain *domain, 
struct device *dev)
 * Properly initialize the domain now that we know which viommu
 * owns it.
 */
-   ret = viommu_domain_finalise(vdev->viommu, domain);
+   ret = viommu_domain_finalise(vdev, domain);
} else if (vdomain->viommu != vdev->viommu) {
dev_err(dev, "cannot attach to foreign vIOMMU\n");
ret = -EXDEV;


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


Re: [PATCH v2 2/3] iommu/virtio: Fix freeing of incomplete domains

2020-03-26 Thread Robin Murphy

On 2020-03-26 9:35 am, Jean-Philippe Brucker wrote:

Calling viommu_domain_free() on a domain that hasn't been finalised (not
attached to any device, for example) can currently cause an Oops,
because we attempt to call ida_free() on ID 0, which may either be
unallocated or used by another domain.

Only initialise the vdomain->viommu pointer, which denotes a finalised
domain, at the end of a successful viommu_domain_finalise().


Reviewed-by: Robin Murphy 


Fixes: edcd69ab9a32 ("iommu: Add virtio-iommu driver")
Reported-by: Eric Auger 
Signed-off-by: Jean-Philippe Brucker 
---
  drivers/iommu/virtio-iommu.c | 16 +---
  1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c
index cce329d71fba..5eed75cd121f 100644
--- a/drivers/iommu/virtio-iommu.c
+++ b/drivers/iommu/virtio-iommu.c
@@ -613,18 +613,20 @@ static int viommu_domain_finalise(struct viommu_dev 
*viommu,
int ret;
struct viommu_domain *vdomain = to_viommu_domain(domain);
  
-	vdomain->viommu		= viommu;

-   vdomain->map_flags   = viommu->map_flags;
+   ret = ida_alloc_range(>domain_ids, viommu->first_domain,
+ viommu->last_domain, GFP_KERNEL);
+   if (ret < 0)
+   return ret;
+
+   vdomain->id  = (unsigned int)ret;
  
  	domain->pgsize_bitmap	= viommu->pgsize_bitmap;

domain->geometry = viommu->geometry;
  
-	ret = ida_alloc_range(>domain_ids, viommu->first_domain,

- viommu->last_domain, GFP_KERNEL);
-   if (ret >= 0)
-   vdomain->id = (unsigned int)ret;
+   vdomain->map_flags   = viommu->map_flags;
+   vdomain->viommu  = viommu;
  
-	return ret > 0 ? 0 : ret;

+   return 0;
  }
  
  static void viommu_domain_free(struct iommu_domain *domain)



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


[PATCH v2 0/3] iommu/virtio: Misc fixes

2020-03-26 Thread Jean-Philippe Brucker
A collection of fixes for the virtio-iommu driver. It might be too late
for v5.6 since they need more review. Patch 2 is new, the others were
posted separately. 

Jean-Philippe Brucker (3):
  iommu/virtio: Fix sparse warning
  iommu/virtio: Fix freeing of incomplete domains
  iommu/virtio: Reject IOMMU page granule larger than PAGE_SIZE

 include/uapi/linux/virtio_iommu.h | 12 ++--
 drivers/iommu/virtio-iommu.c  | 30 +-
 2 files changed, 27 insertions(+), 15 deletions(-)

-- 
2.25.1

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


[PATCH v2 1/3] iommu/virtio: Fix sparse warning

2020-03-26 Thread Jean-Philippe Brucker
We copied the virtio_iommu_config from the virtio-iommu specification,
which declares the fields using little-endian annotations (for example
le32). Unfortunately this causes sparse to warn about comparison between
little- and cpu-endian, because of the typecheck() in virtio_cread():

drivers/iommu/virtio-iommu.c:1024:9: sparse: sparse: incompatible types in 
comparison expression (different base types):
drivers/iommu/virtio-iommu.c:1024:9: sparse:restricted __le64 *
drivers/iommu/virtio-iommu.c:1024:9: sparse:unsigned long long *
drivers/iommu/virtio-iommu.c:1036:9: sparse: sparse: incompatible types in 
comparison expression (different base types):
drivers/iommu/virtio-iommu.c:1036:9: sparse:restricted __le64 *
drivers/iommu/virtio-iommu.c:1036:9: sparse:unsigned long long *
drivers/iommu/virtio-iommu.c:1040:9: sparse: sparse: incompatible types in 
comparison expression (different base types):
drivers/iommu/virtio-iommu.c:1040:9: sparse:restricted __le64 *
drivers/iommu/virtio-iommu.c:1040:9: sparse:unsigned long long *
drivers/iommu/virtio-iommu.c:1044:9: sparse: sparse: incompatible types in 
comparison expression (different base types):
drivers/iommu/virtio-iommu.c:1044:9: sparse:restricted __le32 *
drivers/iommu/virtio-iommu.c:1044:9: sparse:unsigned int *
drivers/iommu/virtio-iommu.c:1048:9: sparse: sparse: incompatible types in 
comparison expression (different base types):
drivers/iommu/virtio-iommu.c:1048:9: sparse:restricted __le32 *
drivers/iommu/virtio-iommu.c:1048:9: sparse:unsigned int *
drivers/iommu/virtio-iommu.c:1052:9: sparse: sparse: incompatible types in 
comparison expression (different base types):
drivers/iommu/virtio-iommu.c:1052:9: sparse:restricted __le32 *
drivers/iommu/virtio-iommu.c:1052:9: sparse:unsigned int *

Although virtio_cread() does convert virtio-endian (in our case
little-endian) to cpu-endian, the typecheck() needs the two arguments to
have the same endianness. Do as UAPI headers of other virtio devices do,
and remove the endian annotation from the device config.

Even though we change the UAPI this shouldn't cause any regression since
QEMU, the existing implementation of virtio-iommu that uses this header,
already removes the annotations when importing headers.

Reported-by: kbuild test robot 
Signed-off-by: Jean-Philippe Brucker 
---
 include/uapi/linux/virtio_iommu.h | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/include/uapi/linux/virtio_iommu.h 
b/include/uapi/linux/virtio_iommu.h
index 237e36a280cb..48e3c29223b5 100644
--- a/include/uapi/linux/virtio_iommu.h
+++ b/include/uapi/linux/virtio_iommu.h
@@ -18,24 +18,24 @@
 #define VIRTIO_IOMMU_F_MMIO5
 
 struct virtio_iommu_range_64 {
-   __le64  start;
-   __le64  end;
+   __u64   start;
+   __u64   end;
 };
 
 struct virtio_iommu_range_32 {
-   __le32  start;
-   __le32  end;
+   __u32   start;
+   __u32   end;
 };
 
 struct virtio_iommu_config {
/* Supported page sizes */
-   __le64  page_size_mask;
+   __u64   page_size_mask;
/* Supported IOVA range */
struct virtio_iommu_range_64input_range;
/* Max domain ID size */
struct virtio_iommu_range_32domain_range;
/* Probe buffer size */
-   __le32  probe_size;
+   __u32   probe_size;
 };
 
 /* Request types */
-- 
2.25.1

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


[PATCH v2 3/3] iommu/virtio: Reject IOMMU page granule larger than PAGE_SIZE

2020-03-26 Thread Jean-Philippe Brucker
We don't currently support IOMMUs with a page granule larger than the
system page size. The IOVA allocator has a BUG_ON() in this case, and
VFIO has a WARN_ON().

Removing these obstacles ranges doesn't seem possible without major
changes to the DMA API and VFIO. Some callers of iommu_map(), for
example, want to map multiple page-aligned regions adjacent to each
others for scatter-gather purposes. Even in simple DMA API uses, a call
to dma_map_page() would let the endpoint access neighbouring memory. And
VFIO users cannot ensure that their virtual address buffer is physically
contiguous at the IOMMU granule.

Rather than triggering the IOVA BUG_ON() on mismatched page sizes, abort
the vdomain finalise() with an error message. We could simply abort the
viommu probe(), but an upcoming extension to virtio-iommu will allow
setting different page masks for each endpoint.

Reported-by: Bharat Bhushan 
Signed-off-by: Jean-Philippe Brucker 
---
v1->v2: Move to vdomain_finalise(), improve commit message
---
 drivers/iommu/virtio-iommu.c | 14 --
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c
index 5eed75cd121f..750f69c49b95 100644
--- a/drivers/iommu/virtio-iommu.c
+++ b/drivers/iommu/virtio-iommu.c
@@ -607,12 +607,22 @@ static struct iommu_domain *viommu_domain_alloc(unsigned 
type)
return >domain;
 }
 
-static int viommu_domain_finalise(struct viommu_dev *viommu,
+static int viommu_domain_finalise(struct viommu_endpoint *vdev,
  struct iommu_domain *domain)
 {
int ret;
+   unsigned long viommu_page_size;
+   struct viommu_dev *viommu = vdev->viommu;
struct viommu_domain *vdomain = to_viommu_domain(domain);
 
+   viommu_page_size = 1UL << __ffs(viommu->pgsize_bitmap);
+   if (viommu_page_size > PAGE_SIZE) {
+   dev_err(vdev->dev,
+   "granule 0x%lx larger than system page size 0x%lx\n",
+   viommu_page_size, PAGE_SIZE);
+   return -EINVAL;
+   }
+
ret = ida_alloc_range(>domain_ids, viommu->first_domain,
  viommu->last_domain, GFP_KERNEL);
if (ret < 0)
@@ -659,7 +669,7 @@ static int viommu_attach_dev(struct iommu_domain *domain, 
struct device *dev)
 * Properly initialize the domain now that we know which viommu
 * owns it.
 */
-   ret = viommu_domain_finalise(vdev->viommu, domain);
+   ret = viommu_domain_finalise(vdev, domain);
} else if (vdomain->viommu != vdev->viommu) {
dev_err(dev, "cannot attach to foreign vIOMMU\n");
ret = -EXDEV;
-- 
2.25.1

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


[PATCH v2 2/3] iommu/virtio: Fix freeing of incomplete domains

2020-03-26 Thread Jean-Philippe Brucker
Calling viommu_domain_free() on a domain that hasn't been finalised (not
attached to any device, for example) can currently cause an Oops,
because we attempt to call ida_free() on ID 0, which may either be
unallocated or used by another domain.

Only initialise the vdomain->viommu pointer, which denotes a finalised
domain, at the end of a successful viommu_domain_finalise().

Fixes: edcd69ab9a32 ("iommu: Add virtio-iommu driver")
Reported-by: Eric Auger 
Signed-off-by: Jean-Philippe Brucker 
---
 drivers/iommu/virtio-iommu.c | 16 +---
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c
index cce329d71fba..5eed75cd121f 100644
--- a/drivers/iommu/virtio-iommu.c
+++ b/drivers/iommu/virtio-iommu.c
@@ -613,18 +613,20 @@ static int viommu_domain_finalise(struct viommu_dev 
*viommu,
int ret;
struct viommu_domain *vdomain = to_viommu_domain(domain);
 
-   vdomain->viommu = viommu;
-   vdomain->map_flags  = viommu->map_flags;
+   ret = ida_alloc_range(>domain_ids, viommu->first_domain,
+ viommu->last_domain, GFP_KERNEL);
+   if (ret < 0)
+   return ret;
+
+   vdomain->id = (unsigned int)ret;
 
domain->pgsize_bitmap   = viommu->pgsize_bitmap;
domain->geometry= viommu->geometry;
 
-   ret = ida_alloc_range(>domain_ids, viommu->first_domain,
- viommu->last_domain, GFP_KERNEL);
-   if (ret >= 0)
-   vdomain->id = (unsigned int)ret;
+   vdomain->map_flags  = viommu->map_flags;
+   vdomain->viommu = viommu;
 
-   return ret > 0 ? 0 : ret;
+   return 0;
 }
 
 static void viommu_domain_free(struct iommu_domain *domain)
-- 
2.25.1

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


Re: [PATCH v2 1/3] iommu/uapi: Define uapi version and capabilities

2020-03-26 Thread Christoph Hellwig
On Wed, Mar 25, 2020 at 04:17:05PM -0700, Jacob Pan wrote:
> Having a single UAPI version to govern the user-kernel data structures
> makes compatibility check straightforward. On the contrary, supporting
> combinations of multiple versions of the data can be a nightmare to
> maintain.
> 
> This patch defines a unified UAPI version to be used for compatibility
> checks between user and kernel.

This is bullshit.  Version numbers don't scale and we've avoided them
everywhere.  You need need flags specifying specific behavior.

> +#define IOMMU_UAPI_VERSION   1
> +static inline int iommu_get_uapi_version(void)
> +{
> + return IOMMU_UAPI_VERSION;
> +}

Also inline functions like this in UAPI headers that actually get
included by userspace programs simply don't work.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu