Re: [v6 1/3] ACPI/IORT: Fixup SMMUv3 resource size for Cavium ThunderX2 SMMUv3 model

2017-05-12 Thread Geetha Akula 
On Sat, May 13, 2017 at 6:03 AM, kbuild test robot <l...@intel.com> wrote:
> Hi Linu,
>
> [auto build test ERROR on arm64/for-next/core]
> [also build test ERROR on v4.11 next-20170512]
> [if your patch is applied to the wrong git tree, please drop us a note to 
> help improve the system]
>
> url:
> https://github.com/0day-ci/linux/commits/Geetha-sowjanya/Cavium-ThunderX2-SMMUv3-errata-workarounds/20170513-065956
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git 
> for-next/core
> config: arm64-defconfig (attached as .config)
> compiler: aarch64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
> reproduce:
> wget 
> https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O 
> ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # save the attached .config to linux build tree
> make.cross ARCH=arm64
>
> All errors (new ones prefixed by >>):
>
>drivers/acpi/arm64/iort.c: In function 'arm_smmu_v3_init_resources':
>>> drivers/acpi/arm64/iort.c:777:21: error: 'ACPI_IORT_SMMU_CAVIUM_CN99XX' 
>>> undeclared (first use in this function)
>  if (smmu->model == ACPI_IORT_SMMU_CAVIUM_CN99XX)
> ^~~~
>drivers/acpi/arm64/iort.c:777:21: note: each undeclared identifier is 
> reported only once for each function it appears in
>
> vim +/ACPI_IORT_SMMU_CAVIUM_CN99XX +777 drivers/acpi/arm64/iort.c
>
>771  smmu = (struct acpi_iort_smmu_v3 *)node->node_data;
>772
>773  /*
>774   * Override the size, for Cavium ThunderX2 implementation
>775   * which doesn't support the page 1 SMMU register space.
>776   */
>  > 777  if (smmu->model == ACPI_IORT_SMMU_CAVIUM_CN99XX)
>778  size = SZ_64K;
>779
>780  res[num_res].start = smmu->base_address;
>
> ---
> 0-DAY kernel test infrastructureOpen Source Technology Center
> https://lists.01.org/pipermail/kbuild-all   Intel Corporation

As menctioned in cover letter this patchset is based on Robin' s latest
patch "Update SMMU models for IORT rev. C" .
 https://www.spinics.net/lists/arm-kernel/msg580728.html



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

Re: [v6 1/3] ACPI/IORT: Fixup SMMUv3 resource size for Cavium ThunderX2 SMMUv3 model

2017-05-12 Thread kbuild test robot 
Hi Linu,

[auto build test ERROR on arm64/for-next/core]
[also build test ERROR on v4.11 next-20170512]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Geetha-sowjanya/Cavium-ThunderX2-SMMUv3-errata-workarounds/20170513-065956
base:   https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git 
for-next/core
config: arm64-defconfig (attached as .config)
compiler: aarch64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget 
https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=arm64 

All errors (new ones prefixed by >>):

   drivers/acpi/arm64/iort.c: In function 'arm_smmu_v3_init_resources':
>> drivers/acpi/arm64/iort.c:777:21: error: 'ACPI_IORT_SMMU_CAVIUM_CN99XX' 
>> undeclared (first use in this function)
 if (smmu->model == ACPI_IORT_SMMU_CAVIUM_CN99XX)
^~~~
   drivers/acpi/arm64/iort.c:777:21: note: each undeclared identifier is 
reported only once for each function it appears in

vim +/ACPI_IORT_SMMU_CAVIUM_CN99XX +777 drivers/acpi/arm64/iort.c

   771  smmu = (struct acpi_iort_smmu_v3 *)node->node_data;
   772  
   773  /*
   774   * Override the size, for Cavium ThunderX2 implementation
   775   * which doesn't support the page 1 SMMU register space.
   776   */
 > 777  if (smmu->model == ACPI_IORT_SMMU_CAVIUM_CN99XX)
   778  size = SZ_64K;
   779  
   780  res[num_res].start = smmu->base_address;

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [RFC PATCH 2/8] iommu/vt-d: add bind_pasid_table function

2017-05-12 Thread Alex Williamson 
On Wed, 26 Apr 2017 18:11:59 +0800
"Liu, Yi L"  wrote:

> From: Jacob Pan 
> 
> Add Intel VT-d ops to the generic iommu_bind_pasid_table API
> functions.
> 
> The primary use case is for direct assignment of SVM capable
> device. Originated from emulated IOMMU in the guest, the request goes
> through many layers (e.g. VFIO). Upon calling host IOMMU driver, caller
> passes guest PASID table pointer (GPA) and size.
> 
> Device context table entry is modified by Intel IOMMU specific
> bind_pasid_table function. This will turn on nesting mode and matching
> translation type.
> 
> The unbind operation restores default context mapping.
> 
> Signed-off-by: Jacob Pan 
> Signed-off-by: Liu, Yi L 
> ---
>  drivers/iommu/intel-iommu.c   | 103 
> ++
>  include/linux/dma_remapping.h |   1 +
>  2 files changed, 104 insertions(+)
> 
> diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
> index 646756c..6d5b939 100644
> --- a/drivers/iommu/intel-iommu.c
> +++ b/drivers/iommu/intel-iommu.c
> @@ -5306,6 +5306,105 @@ struct intel_iommu *intel_svm_device_to_iommu(struct 
> device *dev)
>  
>   return iommu;
>  }
> +
> +static int intel_iommu_bind_pasid_table(struct iommu_domain *domain,
> + struct device *dev, struct pasid_table_info *pasidt_binfo)
> +{
> + struct intel_iommu *iommu;
> + struct context_entry *context;
> + struct dmar_domain *dmar_domain = to_dmar_domain(domain);
> + struct device_domain_info *info;
> + u8 bus, devfn;
> + u16 did, *sid;
> + int ret = 0;
> + unsigned long flags;
> + u64 ctx_lo;
> +
> + if (pasidt_binfo == NULL || pasidt_binfo->model != INTEL_IOMMU) {
> + pr_warn("%s: Invalid bind request!\n", __func__);
> + return -EINVAL;
> + }
> +
> + iommu = device_to_iommu(dev, , );
> + if (!iommu)
> + return -ENODEV;
> +
> + sid = (u16 *)_binfo->opaque;

struct pasid_table_info is expected to be provided by a user, the
opaque data structure for model == INTEL_IOMMU therefore needs to be
documented in uapi.

> + /* check SID, if it is not correct, return */
> + if (PCI_DEVID(bus, devfn) != *sid)
> + return 0;

This is a bit weird, it took me until later in the series to understand
why this is a success case.  Perhaps the device matching needs to be
standardized in pasid_table_info rather than the opaque data.
Minimally, more comments.

> +
> + info = dev->archdata.iommu;
> + if (!info || !info->pasid_supported) {
> + pr_err("Device %d:%d.%d has no pasid support\n", bus,
> + PCI_SLOT(devfn), PCI_FUNC(devfn));

PCI addresses should be printed in hex and include the segment.  This
also looks like it might be user reachable, so a user could DoS the
host by continuously calling this where pasid is not supported and fill
logs with pr_err.  Maybe dropping the pr_err is the better choice.


> + ret = -EINVAL;
> + goto out;
> + }
> +
> + if (pasidt_binfo->size >= intel_iommu_get_pts(iommu)) {
> + pr_err("Invalid gPASID table size %llu, host size %lu\n",
> + pasidt_binfo->size,
> + intel_iommu_get_pts(iommu));
> + ret = -EINVAL;
> + goto out;

equal is not valid?

> + }
> + spin_lock_irqsave(>lock, flags);
> + context = iommu_context_addr(iommu, bus, devfn, 0);
> + if (!context || !context_present(context)) {
> + pr_warn("%s: ctx not present for bus devfn %x:%x\n",
> + __func__, bus, devfn);

Use standard PCI address format, including segment.

> + spin_unlock_irqrestore(>lock, flags);
> + goto out;
> + }
> + /* Anticipate guest to use SVM and owns the first level */
> + ctx_lo = context[0].lo;
> + ctx_lo |= CONTEXT_NESTE;
> + ctx_lo |= CONTEXT_PRS;
> + ctx_lo |= CONTEXT_PASIDE;
> + ctx_lo &= ~CONTEXT_TT_MASK;
> + ctx_lo |= CONTEXT_TT_DEV_IOTLB << 2;
> + context[0].lo = ctx_lo;
> +
> + /* Assign guest PASID table pointer and size */
> + ctx_lo = (pasidt_binfo->ptr & VTD_PAGE_MASK) | pasidt_binfo->size;
> + context[1].lo = ctx_lo;
> + /* make sure context entry is updated before flushing */
> + wmb();
> + did = dmar_domain->iommu_did[iommu->seq_id];
> + iommu->flush.flush_context(iommu, did,
> + (((u16)bus) << 8) | devfn,
> + DMA_CCMD_MASK_NOBIT,
> + DMA_CCMD_DEVICE_INVL);
> + iommu->flush.flush_iotlb(iommu, did, 0, 0, DMA_TLB_DSI_FLUSH);
> + spin_unlock_irqrestore(>lock, flags);

Mildly concerned what sort of Pandora's box this opens, but I guess
we're relying on the 2nd level translation to validate and make sure
the user can only hurt themselves.

> +
> +

Re: [RFC PATCH 3/8] iommu: Introduce iommu do invalidate API function

2017-05-12 Thread Alex Williamson 
On Wed, 26 Apr 2017 18:12:00 +0800
"Liu, Yi L"  wrote:

> From: "Liu, Yi L" 
> 
> When a SVM capable device is assigned to a guest, the first level page
> tables are owned by the guest and the guest PASID table pointer is
> linked to the device context entry of the physical IOMMU.
> 
> Host IOMMU driver has no knowledge of caching structure updates unless
> the guest invalidation activities are passed down to the host. The
> primary usage is derived from emulated IOMMU in the guest, where QEMU
> can trap invalidation activities before pass them down the
> host/physical IOMMU. There are IOMMU architectural specific actions
> need to be taken which requires the generic APIs introduced in this
> patch to have opaque data in the tlb_invalidate_info argument.
> 
> Signed-off-by: Liu, Yi L 
> Signed-off-by: Jacob Pan 
> ---
>  drivers/iommu/iommu.c | 13 +
>  include/linux/iommu.h | 16 
>  2 files changed, 29 insertions(+)
> 
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index f2da636..ca7cff2 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -1153,6 +1153,19 @@ int iommu_unbind_pasid_table(struct iommu_domain 
> *domain, struct device *dev)
>  }
>  EXPORT_SYMBOL_GPL(iommu_unbind_pasid_table);
>  
> +int iommu_do_invalidate(struct iommu_domain *domain,
> + struct device *dev, struct tlb_invalidate_info *inv_info)
> +{
> + int ret = 0;
> +
> + if (unlikely(domain->ops->do_invalidate == NULL))
> + return -ENODEV;
> +
> + ret = domain->ops->do_invalidate(domain, dev, inv_info);
> + return ret;

nit, ret is unnecessary.

> +}
> +EXPORT_SYMBOL_GPL(iommu_do_invalidate);
> +
>  static void __iommu_detach_device(struct iommu_domain *domain,
> struct device *dev)
>  {
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index 491a011..a48e3b75 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -140,6 +140,11 @@ struct pasid_table_info {
>   __u8opaque[];/* IOMMU-specific details */
>  };
>  
> +struct tlb_invalidate_info {
> + __u32   model;
> + __u8opaque[];
> +};

I'm wondering if 'model' is really necessary here, shouldn't this
function only be called if a bind_pasid_table() succeeded, and then the
model would be set at that time?

This also needs to be uapi since you're expecting a user to provide it
to vfio.  The opaque data needs to be fully specified (relative to
uapi) per model.

> +
>  #ifdef CONFIG_IOMMU_API
>  
>  /**
> @@ -215,6 +220,8 @@ struct iommu_ops {
>   struct pasid_table_info *pasidt_binfo);
>   int (*unbind_pasid_table)(struct iommu_domain *domain,
>   struct device *dev);
> + int (*do_invalidate)(struct iommu_domain *domain,
> + struct device *dev, struct tlb_invalidate_info *inv_info);
>  
>   unsigned long pgsize_bitmap;
>  };
> @@ -240,6 +247,9 @@ extern int iommu_bind_pasid_table(struct iommu_domain 
> *domain,
>   struct device *dev, struct pasid_table_info *pasidt_binfo);
>  extern int iommu_unbind_pasid_table(struct iommu_domain *domain,
>   struct device *dev);
> +extern int iommu_do_invalidate(struct iommu_domain *domain,
> + struct device *dev, struct tlb_invalidate_info *inv_info);
> +
>  extern struct iommu_domain *iommu_get_domain_for_dev(struct device *dev);
>  extern int iommu_map(struct iommu_domain *domain, unsigned long iova,
>phys_addr_t paddr, size_t size, int prot);
> @@ -626,6 +636,12 @@ int iommu_unbind_pasid_table(struct iommu_domain 
> *domain, struct device *dev)
>   return -EINVAL;
>  }
>  
> +static inline int iommu_do_invalidate(struct iommu_domain *domain,
> + struct device *dev, struct tlb_invalidate_info *inv_info)
> +{
> + return -EINVAL;
> +}
> +
>  #endif /* CONFIG_IOMMU_API */
>  
>  #endif /* __LINUX_IOMMU_H */

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

Re: [RFC PATCH 4/8] iommu/vt-d: Add iommu do invalidate function

2017-05-12 Thread Alex Williamson 
On Wed, 26 Apr 2017 18:12:01 +0800
"Liu, Yi L"  wrote:

> From: Jacob Pan 
> 
> This patch adds Intel VT-d specific function to implement
> iommu_do_invalidate API.
> 
> The use case is for supporting caching structure invalidation
> of assigned SVM capable devices. Emulated IOMMU exposes queue
> invalidation capability and passes down all descriptors from the guest
> to the physical IOMMU.
> 
> The assumption is that guest to host device ID mapping should be
> resolved prior to calling IOMMU driver. Based on the device handle,
> host IOMMU driver can replace certain fields before submit to the
> invalidation queue.
> 
> Signed-off-by: Liu, Yi L 
> Signed-off-by: Jacob Pan 
> ---
>  drivers/iommu/intel-iommu.c | 43 +++
>  include/linux/intel-iommu.h | 11 +++
>  2 files changed, 54 insertions(+)
> 
> diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
> index 6d5b939..0b098ad 100644
> --- a/drivers/iommu/intel-iommu.c
> +++ b/drivers/iommu/intel-iommu.c
> @@ -5042,6 +5042,48 @@ static void intel_iommu_detach_device(struct 
> iommu_domain *domain,
>   dmar_remove_one_dev_info(to_dmar_domain(domain), dev);
>  }
>  
> +static int intel_iommu_do_invalidate(struct iommu_domain *domain,
> + struct device *dev, struct tlb_invalidate_info *inv_info)
> +{
> + int ret = 0;
> + struct intel_iommu *iommu;
> + struct dmar_domain *dmar_domain = to_dmar_domain(domain);
> + struct intel_invalidate_data *inv_data;
> + struct qi_desc *qi;
> + u16 did;
> + u8 bus, devfn;
> +
> + if (!inv_info || !dmar_domain || (inv_info->model != INTEL_IOMMU))
> + return -EINVAL;
> +
> + iommu = device_to_iommu(dev, , );
> + if (!iommu)
> + return -ENODEV;
> +
> + inv_data = (struct intel_invalidate_data *)_info->opaque;
> +
> + /* check SID */
> + if (PCI_DEVID(bus, devfn) != inv_data->sid)
> + return 0;
> +
> + qi = _data->inv_desc;
> +
> + switch (qi->low & QI_TYPE_MASK) {
> + case QI_DIOTLB_TYPE:
> + case QI_DEIOTLB_TYPE:
> + /* for device IOTLB, we just let it pass through */
> + break;
> + default:
> + did = dmar_domain->iommu_did[iommu->seq_id];
> + set_mask_bits(>low, QI_DID_MASK, QI_DID(did));
> + break;
> + }
> +
> + ret = qi_submit_sync(qi, iommu);
> +
> + return ret;

nit, ret variable is unnecessary.

> +}
> +
>  static int intel_iommu_map(struct iommu_domain *domain,
>  unsigned long iova, phys_addr_t hpa,
>  size_t size, int iommu_prot)
> @@ -5416,6 +5458,7 @@ static int intel_iommu_unbind_pasid_table(struct 
> iommu_domain *domain,
>  #ifdef CONFIG_INTEL_IOMMU_SVM
>   .bind_pasid_table   = intel_iommu_bind_pasid_table,
>   .unbind_pasid_table = intel_iommu_unbind_pasid_table,
> + .do_invalidate  = intel_iommu_do_invalidate,
>  #endif
>   .map= intel_iommu_map,
>   .unmap  = intel_iommu_unmap,
> diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h
> index ac04f28..9d6562c 100644
> --- a/include/linux/intel-iommu.h
> +++ b/include/linux/intel-iommu.h
> @@ -29,6 +29,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  
> @@ -271,6 +272,10 @@ enum {
>  #define QI_PGRP_RESP_TYPE0x9
>  #define QI_PSTRM_RESP_TYPE   0xa
>  
> +#define QI_DID(did)  (((u64)did & 0x) << 16)
> +#define QI_DID_MASK  GENMASK(31, 16)
> +#define QI_TYPE_MASK GENMASK(3, 0)
> +
>  #define QI_IEC_SELECTIVE (((u64)1) << 4)
>  #define QI_IEC_IIDEX(idx)(((u64)(idx & 0x) << 32))
>  #define QI_IEC_IM(m) (((u64)(m & 0x1f) << 27))
> @@ -529,6 +534,12 @@ struct intel_svm {
>  extern struct intel_iommu *intel_svm_device_to_iommu(struct device *dev);
>  #endif
>  
> +struct intel_invalidate_data {
> + u16 sid;
> + u32 pasid;
> + struct qi_desc inv_desc;
> +};

This needs to be uapi since the vfio user is expected to create it, so
we need a uapi version of qi_desc too.

> +
>  extern const struct attribute_group *intel_iommu_groups[];
>  extern void intel_iommu_debugfs_init(void);
>  extern struct context_entry *iommu_context_addr(struct intel_iommu *iommu,

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

Re: [RFC PATCH 1/8] iommu: Introduce bind_pasid_table API function

2017-05-12 Thread Alex Williamson 
On Wed, 26 Apr 2017 18:11:58 +0800
"Liu, Yi L"  wrote:

> From: Jacob Pan 
> 
> Virtual IOMMU was proposed to support Shared Virtual Memory (SVM) use
> case in the guest:
> https://lists.gnu.org/archive/html/qemu-devel/2016-11/msg05311.html
> 
> As part of the proposed architecture, when a SVM capable PCI
> device is assigned to a guest, nested mode is turned on. Guest owns the
> first level page tables (request with PASID) and performs GVA->GPA
> translation. Second level page tables are owned by the host for GPA->HPA
> translation for both request with and without PASID.
> 
> A new IOMMU driver interface is therefore needed to perform tasks as
> follows:
> * Enable nested translation and appropriate translation type
> * Assign guest PASID table pointer (in GPA) and size to host IOMMU
> 
> This patch introduces new functions called iommu_(un)bind_pasid_table()
> to IOMMU APIs. Architecture specific IOMMU function can be added later
> to perform the specific steps for binding pasid table of assigned devices.
> 
> This patch also adds model definition in iommu.h. It would be used to
> check if the bind request is from a compatible entity. e.g. a bind
> request from an intel_iommu emulator may not be supported by an ARM SMMU
> driver.
> 
> Signed-off-by: Jacob Pan 
> Signed-off-by: Liu, Yi L 
> ---
>  drivers/iommu/iommu.c | 19 +++
>  include/linux/iommu.h | 31 +++
>  2 files changed, 50 insertions(+)
> 
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index dbe7f65..f2da636 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -1134,6 +1134,25 @@ int iommu_attach_device(struct iommu_domain *domain, 
> struct device *dev)
>  }
>  EXPORT_SYMBOL_GPL(iommu_attach_device);
>  
> +int iommu_bind_pasid_table(struct iommu_domain *domain, struct device *dev,
> + struct pasid_table_info *pasidt_binfo)
> +{
> + if (unlikely(!domain->ops->bind_pasid_table))
> + return -EINVAL;
> +
> + return domain->ops->bind_pasid_table(domain, dev, pasidt_binfo);
> +}
> +EXPORT_SYMBOL_GPL(iommu_bind_pasid_table);
> +
> +int iommu_unbind_pasid_table(struct iommu_domain *domain, struct device *dev)
> +{
> + if (unlikely(!domain->ops->unbind_pasid_table))
> + return -EINVAL;
> +
> + return domain->ops->unbind_pasid_table(domain, dev);
> +}
> +EXPORT_SYMBOL_GPL(iommu_unbind_pasid_table);
> +
>  static void __iommu_detach_device(struct iommu_domain *domain,
> struct device *dev)
>  {
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index 0ff5111..491a011 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -131,6 +131,15 @@ struct iommu_dm_region {
>   int prot;
>  };
>  
> +struct pasid_table_info {
> + __u64   ptr;/* PASID table ptr */
> + __u64   size;   /* PASID table size*/
> + __u32   model;  /* magic number */
> +#define INTEL_IOMMU  (1 << 0)
> +#define ARM_SMMU (1 << 1)
> + __u8opaque[];/* IOMMU-specific details */
> +};

This needs to be in uapi since you're expecting a user to pass it 

> +
>  #ifdef CONFIG_IOMMU_API
>  
>  /**
> @@ -159,6 +168,8 @@ struct iommu_dm_region {
>   * @domain_get_windows: Return the number of windows for a domain
>   * @of_xlate: add OF master IDs to iommu grouping
>   * @pgsize_bitmap: bitmap of all possible supported page sizes
> + * @bind_pasid_table: bind pasid table pointer for guest SVM
> + * @unbind_pasid_table: unbind pasid table pointer and restore defaults
>   */
>  struct iommu_ops {
>   bool (*capable)(enum iommu_cap);
> @@ -200,6 +211,10 @@ struct iommu_ops {
>   u32 (*domain_get_windows)(struct iommu_domain *domain);
>  
>   int (*of_xlate)(struct device *dev, struct of_phandle_args *args);
> + int (*bind_pasid_table)(struct iommu_domain *domain, struct device *dev,
> + struct pasid_table_info *pasidt_binfo);
> + int (*unbind_pasid_table)(struct iommu_domain *domain,
> + struct device *dev);
>  
>   unsigned long pgsize_bitmap;
>  };
> @@ -221,6 +236,10 @@ extern int iommu_attach_device(struct iommu_domain 
> *domain,
>  struct device *dev);
>  extern void iommu_detach_device(struct iommu_domain *domain,
>   struct device *dev);
> +extern int iommu_bind_pasid_table(struct iommu_domain *domain,
> + struct device *dev, struct pasid_table_info *pasidt_binfo);
> +extern int iommu_unbind_pasid_table(struct iommu_domain *domain,
> + struct device *dev);
>  extern struct iommu_domain *iommu_get_domain_for_dev(struct device *dev);
>  extern int iommu_map(struct iommu_domain *domain, unsigned long iova,
>phys_addr_t paddr, size_t size, int prot);
>

Re: [RFC PATCH 6/8] VFIO: do pasid table binding

2017-05-12 Thread Alex Williamson 
On Wed, 26 Apr 2017 18:12:03 +0800
"Liu, Yi L"  wrote:

> From: "Liu, Yi L" 
> 
> This patch adds IOCTL processing in vfio_iommu_type1 for
> VFIO_IOMMU_SVM_BIND_TASK. Binds the PASID table bind by
> calling iommu_ops->bind_pasid_table to link the whole
> PASID table to pIOMMU.
> 
> For VT-d, it is linking the guest PASID table to host pIOMMU.
> This is key point to support SVM virtualization on VT-d.
> 
> Signed-off-by: Liu, Yi L 
> ---
>  drivers/vfio/vfio_iommu_type1.c | 72 
> +
>  1 file changed, 72 insertions(+)
> 
> diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
> index b3cc33f..30b6d48 100644
> --- a/drivers/vfio/vfio_iommu_type1.c
> +++ b/drivers/vfio/vfio_iommu_type1.c
> @@ -1512,6 +1512,50 @@ static int vfio_domains_have_iommu_cache(struct 
> vfio_iommu *iommu)
>   return ret;
>  }
>  
> +struct vfio_svm_task {
> + struct iommu_domain *domain;
> + void *payload;
> +};
> +
> +static int bind_pasid_tbl_fn(struct device *dev, void *data)
> +{
> + int ret = 0;
> + struct vfio_svm_task *task = data;

Maybe avoid "task" or use svm_task to differentiate from task_struct
task used elsewhere in this file.

> + struct pasid_table_info *pasidt_binfo;
> +
> + pasidt_binfo = task->payload;
> + ret = iommu_bind_pasid_table(task->domain, dev, pasidt_binfo);
> + return ret;
> +}
> +
> +static int vfio_do_svm_task(struct vfio_iommu *iommu, void *data,
> + int (*fn)(struct device *, void *))
> +{
> + int ret = 0;
> + struct vfio_domain *d;
> + struct vfio_group *g;
> + struct vfio_svm_task task;
> +
> + task.payload = data;
> +
> + mutex_lock(>lock);
> +
> + list_for_each_entry(d, >domain_list, next) {
> + list_for_each_entry(g, >group_list, next) {
> + if (g->iommu_group != NULL) {

Can it ever be NULL?

> + task.domain = d->domain;
> + ret = iommu_group_for_each_dev(
> + g->iommu_group, , fn);
> + if (ret != 0)
> + break;
> + }
> + }
> + }
> +
> + mutex_unlock(>lock);
> + return ret;
> +}
> +
>  static long vfio_iommu_type1_ioctl(void *iommu_data,
>  unsigned int cmd, unsigned long arg)
>  {
> @@ -1582,6 +1626,34 @@ static long vfio_iommu_type1_ioctl(void *iommu_data,
>  
>   return copy_to_user((void __user *)arg, , minsz) ?
>   -EFAULT : 0;
> + } else if (cmd == VFIO_IOMMU_SVM_BIND_TASK) {
> + struct vfio_device_svm hdr;
> + u8 *data = NULL;

But it really should be a struct pasid_table_info.

> + int ret = 0;
> +
> + minsz = offsetofend(struct vfio_device_svm, length);
> + if (copy_from_user(, (void __user *)arg, minsz))
> + return -EFAULT;
> +
> + if (hdr.length == 0)
> + return -EINVAL;
> +
> + data = memdup_user((void __user *)(arg + minsz),
> + hdr.length);
> + if (IS_ERR(data))
> + return PTR_ERR(data);
> +
> + switch (hdr.flags & VFIO_SVM_TYPE_MASK) {
> + case VFIO_SVM_BIND_PASIDTBL:
> + ret = vfio_do_svm_task(iommu, data,
> + bind_pasid_tbl_fn);
> + break;
> + default:
> + ret = -EINVAL;
> + break;
> + }
> + kfree(data);
> + return ret;
>   }
>  
>   return -ENOTTY;

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

Re: [RFC PATCH 5/8] VFIO: Add new IOTCL for PASID Table bind propagation

2017-05-12 Thread Alex Williamson 
On Wed, 26 Apr 2017 18:12:02 +0800
"Liu, Yi L"  wrote:

> From: "Liu, Yi L" 
> 
> This patch adds VFIO_IOMMU_SVM_BIND_TASK for potential PASID table
> binding requests.
> 
> On VT-d, this IOCTL cmd would be used to link the guest PASID page table
> to host. While for other vendors, it may also be used to support other
> kind of SVM bind request. Previously, there is a discussion on it with
> ARM engineer. It can be found by the link below. This IOCTL cmd may
> support SVM PASID bind request from userspace driver, or page table(cr3)
> bind request from guest. These SVM bind requests would be supported by
> adding different flags. e.g. VFIO_SVM_BIND_PASID is added to support
> PASID bind from userspace driver, VFIO_SVM_BIND_PGTABLE is added to
> support page table bind from guest.
> 
> https://patchwork.kernel.org/patch/9594231/
> 
> Signed-off-by: Liu, Yi L 
> ---
>  include/uapi/linux/vfio.h | 17 +
>  1 file changed, 17 insertions(+)
> 
> diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
> index 519eff3..6b97987 100644
> --- a/include/uapi/linux/vfio.h
> +++ b/include/uapi/linux/vfio.h
> @@ -547,6 +547,23 @@ struct vfio_iommu_type1_dma_unmap {
>  #define VFIO_IOMMU_ENABLE_IO(VFIO_TYPE, VFIO_BASE + 15)
>  #define VFIO_IOMMU_DISABLE   _IO(VFIO_TYPE, VFIO_BASE + 16)
>  
> +/* IOCTL for Shared Virtual Memory Bind */
> +struct vfio_device_svm {
> + __u32   argsz;
> +#define VFIO_SVM_BIND_PASIDTBL   (1 << 0) /* Bind PASID Table */
> +#define VFIO_SVM_BIND_PASID  (1 << 1) /* Bind PASID from userspace driver */
> +#define VFIO_SVM_BIND_PGTABLE(1 << 2) /* Bind guest mmu page table */
> + __u32   flags;
> + __u32   length;
> + __u8data[];

In the case of VFIO_SVM_BIND_PASIDTBL this is clearly struct
pasid_table_info?  So at a minimum this is a union including struct
pasid_table_info.  Furthermore how does a user learn what the opaque
data in struct pasid_table_info is without looking at the code?  A user
API needs to be clear and documented, not opaque and variable.  We
should also have references to the hardware spec for an Intel or ARM
PASID table in uapi.  flags should be defined as they're used, let's
not reserve them with the expectation of future use.

> +};
> +
> +#define VFIO_SVM_TYPE_MASK   (VFIO_SVM_BIND_PASIDTBL | \
> + VFIO_SVM_BIND_PASID | \
> + VFIO_SVM_BIND_PGTABLE)
> +
> +#define VFIO_IOMMU_SVM_BIND_TASK _IO(VFIO_TYPE, VFIO_BASE + 22)
> +
>  /*  Additional API for SPAPR TCE (Server POWERPC) IOMMU  */
>  
>  /*

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

Re: [RFC PATCH 7/8] VFIO: Add new IOCTL for IOMMU TLB invalidate propagation

2017-05-12 Thread Alex Williamson 
On Wed, 26 Apr 2017 18:12:04 +0800
"Liu, Yi L"  wrote:

> From: "Liu, Yi L" 
> 
> This patch adds VFIO_IOMMU_TLB_INVALIDATE to propagate IOMMU TLB
> invalidate request from guest to host.
> 
> In the case of SVM virtualization on VT-d, host IOMMU driver has
> no knowledge of caching structure updates unless the guest
> invalidation activities are passed down to the host. So a new
> IOCTL is needed to propagate the guest cache invalidation through
> VFIO.
> 
> Signed-off-by: Liu, Yi L 
> ---
>  include/uapi/linux/vfio.h | 9 +
>  1 file changed, 9 insertions(+)
> 
> diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
> index 6b97987..50c51f8 100644
> --- a/include/uapi/linux/vfio.h
> +++ b/include/uapi/linux/vfio.h
> @@ -564,6 +564,15 @@ struct vfio_device_svm {
>  
>  #define VFIO_IOMMU_SVM_BIND_TASK _IO(VFIO_TYPE, VFIO_BASE + 22)
>  
> +/* For IOMMU TLB Invalidation Propagation */
> +struct vfio_iommu_tlb_invalidate {
> + __u32   argsz;
> + __u32   length;
> + __u8data[];
> +};
> +
> +#define VFIO_IOMMU_TLB_INVALIDATE_IO(VFIO_TYPE, VFIO_BASE + 23)

I'm kind of wondering why this isn't just a new flag bit on
vfio_device_svm, the data structure is so similar.  Of course data
needs to be fully specified in uapi.

> +
>  /*  Additional API for SPAPR TCE (Server POWERPC) IOMMU  */
>  
>  /*

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

[v6 3/3] iommu/arm-smmu-v3: Add workaround for Cavium ThunderX2 erratum #126

2017-05-12 Thread Geetha sowjanya 
From: Geetha Sowjanya 

Cavium ThunderX2 SMMU doesn't support MSI and also doesn't have unique irq
lines for gerror, eventq and cmdq-sync.

This patch addresses the issue by checking if any interrupt sources are
using same irq number, then they are registered as shared irqs.

Signed-off-by: Geetha Sowjanya 
---
 Documentation/arm64/silicon-errata.txt |  1 +
 drivers/iommu/arm-smmu-v3.c| 29 +
 2 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/Documentation/arm64/silicon-errata.txt 
b/Documentation/arm64/silicon-errata.txt
index 4693a32..42422f6 100644
--- a/Documentation/arm64/silicon-errata.txt
+++ b/Documentation/arm64/silicon-errata.txt
@@ -63,6 +63,7 @@ stable kernels.
 | Cavium | ThunderX Core   | #27456  | CAVIUM_ERRATUM_27456
|
 | Cavium | ThunderX SMMUv2 | #27704  | N/A 
|
 | Cavium | ThunderX2 SMMUv3| #74 | N/A 
|
+| Cavium | ThunderX2 SMMUv3| #126| N/A 
|
 || | | 
|
 | Freescale/NXP  | LS2080A/LS1043A | A-008585| FSL_ERRATUM_A008585 
|
 || | | 
|
diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index c519927c..69d8506 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -2232,6 +2232,25 @@ static void arm_smmu_setup_msis(struct arm_smmu_device 
*smmu)
devm_add_action(dev, arm_smmu_free_msis, dev);
 }
 
+static int get_irq_flags(struct arm_smmu_device *smmu, int irq)
+{
+   int match_count = 0;
+
+   if (irq == smmu->evtq.q.irq)
+   match_count++;
+   if (irq == smmu->cmdq.q.irq)
+   match_count++;
+   if (irq == smmu->gerr_irq)
+   match_count++;
+   if (irq == smmu->priq.q.irq)
+   match_count++;
+
+   if (match_count > 1)
+   return IRQF_SHARED | IRQF_ONESHOT;
+
+   return IRQF_ONESHOT;
+}
+
 static int arm_smmu_setup_irqs(struct arm_smmu_device *smmu)
 {
int ret, irq;
@@ -2252,7 +2271,7 @@ static int arm_smmu_setup_irqs(struct arm_smmu_device 
*smmu)
if (irq) {
ret = devm_request_threaded_irq(smmu->dev, irq, NULL,
arm_smmu_evtq_thread,
-   IRQF_ONESHOT,
+   get_irq_flags(smmu, irq),
"arm-smmu-v3-evtq", smmu);
if (ret < 0)
dev_warn(smmu->dev, "failed to enable evtq irq\n");
@@ -2261,7 +2280,8 @@ static int arm_smmu_setup_irqs(struct arm_smmu_device 
*smmu)
irq = smmu->cmdq.q.irq;
if (irq) {
ret = devm_request_irq(smmu->dev, irq,
-  arm_smmu_cmdq_sync_handler, 0,
+  arm_smmu_cmdq_sync_handler,
+  get_irq_flags(smmu, irq),
   "arm-smmu-v3-cmdq-sync", smmu);
if (ret < 0)
dev_warn(smmu->dev, "failed to enable cmdq-sync irq\n");
@@ -2270,7 +2290,8 @@ static int arm_smmu_setup_irqs(struct arm_smmu_device 
*smmu)
irq = smmu->gerr_irq;
if (irq) {
ret = devm_request_irq(smmu->dev, irq, arm_smmu_gerror_handler,
-  0, "arm-smmu-v3-gerror", smmu);
+   get_irq_flags(smmu, irq),
+   "arm-smmu-v3-gerror", smmu);
if (ret < 0)
dev_warn(smmu->dev, "failed to enable gerror irq\n");
}
@@ -2280,7 +2301,7 @@ static int arm_smmu_setup_irqs(struct arm_smmu_device 
*smmu)
if (irq) {
ret = devm_request_threaded_irq(smmu->dev, irq, NULL,
arm_smmu_priq_thread,
-   IRQF_ONESHOT,
+   get_irq_flags(smmu, 
irq),
"arm-smmu-v3-priq",
smmu);
if (ret < 0)
-- 
1.8.3.1

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

[v6 2/3] iommu/arm-smmu-v3: Add workaround for Cavium ThunderX2 erratum #74

2017-05-12 Thread Geetha sowjanya 
From: Linu Cherian 

Cavium ThunderX2 SMMU implementation doesn't support page 1 register space
and PAGE0_REGS_ONLY option is enabled as an errata workaround.
This option when turned on, replaces all page 1 offsets used for
EVTQ_PROD/CONS, PRIQ_PROD/CONS register access with page 0 offsets.

SMMU resource size checks are now based on SMMU option PAGE0_REGS_ONLY,
since resource size can be either 64k/128k.
For this, arm_smmu_device_dt_probe/acpi_probe has been moved before
platform_get_resource call, so that SMMU options are set beforehand.

Signed-off-by: Linu Cherian 
Signed-off-by: Geetha Sowjanya 
---
 Documentation/arm64/silicon-errata.txt |  1 +
 .../devicetree/bindings/iommu/arm,smmu-v3.txt  |  6 ++
 drivers/iommu/arm-smmu-v3.c| 64 +-
 3 files changed, 56 insertions(+), 15 deletions(-)

diff --git a/Documentation/arm64/silicon-errata.txt 
b/Documentation/arm64/silicon-errata.txt
index 10f2ddd..4693a32 100644
--- a/Documentation/arm64/silicon-errata.txt
+++ b/Documentation/arm64/silicon-errata.txt
@@ -62,6 +62,7 @@ stable kernels.
 | Cavium | ThunderX GICv3  | #23154  | CAVIUM_ERRATUM_23154
|
 | Cavium | ThunderX Core   | #27456  | CAVIUM_ERRATUM_27456
|
 | Cavium | ThunderX SMMUv2 | #27704  | N/A 
|
+| Cavium | ThunderX2 SMMUv3| #74 | N/A 
|
 || | | 
|
 | Freescale/NXP  | LS2080A/LS1043A | A-008585| FSL_ERRATUM_A008585 
|
 || | | 
|
diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt 
b/Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt
index be57550..e6da62b 100644
--- a/Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt
+++ b/Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt
@@ -49,6 +49,12 @@ the PCIe specification.
 - hisilicon,broken-prefetch-cmd
 : Avoid sending CMD_PREFETCH_* commands to the SMMU.
 
+- cavium-cn99xx,broken-page1-regspace
+: Replaces all page 1 offsets used for EVTQ_PROD/CONS,
+   PRIQ_PROD/CONS register access 
with page 0 offsets.
+   Set for Caviun ThunderX2 
silicon that doesn't support
+   SMMU page1 register space.
+
 ** Example
 
 smmu@2b40 {
diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 380969a..c519927c 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -412,6 +412,9 @@
 #define MSI_IOVA_BASE  0x800
 #define MSI_IOVA_LENGTH0x10
 
+#define ARM_SMMU_PAGE0_REGS_ONLY(smmu) \
+   ((smmu)->options & ARM_SMMU_OPT_PAGE0_REGS_ONLY)
+
 static bool disable_bypass;
 module_param_named(disable_bypass, disable_bypass, bool, S_IRUGO);
 MODULE_PARM_DESC(disable_bypass,
@@ -597,6 +600,7 @@ struct arm_smmu_device {
u32 features;
 
 #define ARM_SMMU_OPT_SKIP_PREFETCH (1 << 0)
+#define ARM_SMMU_OPT_PAGE0_REGS_ONLY(1 << 1)
u32 options;
 
struct arm_smmu_cmdqcmdq;
@@ -663,9 +667,19 @@ struct arm_smmu_option_prop {
 
 static struct arm_smmu_option_prop arm_smmu_options[] = {
{ ARM_SMMU_OPT_SKIP_PREFETCH, "hisilicon,broken-prefetch-cmd" },
+   { ARM_SMMU_OPT_PAGE0_REGS_ONLY, "cavium-cn99xx,broken-page1-regspace"},
{ 0, NULL},
 };
 
+static inline void __iomem *arm_smmu_page1_fixup(unsigned long offset,
+struct arm_smmu_device *smmu)
+{
+   if (offset > SZ_64K && ARM_SMMU_PAGE0_REGS_ONLY(smmu))
+   offset -= SZ_64K;
+
+   return smmu->base + offset;
+}
+
 static struct arm_smmu_domain *to_smmu_domain(struct iommu_domain *dom)
 {
return container_of(dom, struct arm_smmu_domain, domain);
@@ -1961,8 +1975,8 @@ static int arm_smmu_init_one_queue(struct arm_smmu_device 
*smmu,
return -ENOMEM;
}
 
-   q->prod_reg = smmu->base + prod_off;
-   q->cons_reg = smmu->base + cons_off;
+   q->prod_reg = arm_smmu_page1_fixup(prod_off, smmu);
+   q->cons_reg = arm_smmu_page1_fixup(cons_off, smmu);
q->ent_dwords   = dwords;
 
q->q_base  = Q_BASE_RWA;
@@ -2363,8 +2377,10 @@ static int arm_smmu_device_reset(struct arm_smmu_device 
*smmu, bool bypass)
 
/* Event queue */
writeq_relaxed(smmu->evtq.q.q_base, smmu->base + ARM_SMMU_EVTQ_BASE);
-   writel_relaxed(smmu->evtq.q.prod, smmu->base + ARM_SMMU_EVTQ_PROD);
-   writel_relaxed(smmu->evtq.q.cons, smmu->base +

[v6 0/3] Cavium ThunderX2 SMMUv3 errata workarounds

2017-05-12 Thread Geetha sowjanya 
From: Linu Cherian 

Cavium ThunderX2 SMMUv3 implementation has two Silicon Erratas.
1. Errata ID #74
   SMMU register alias Page 1 is not implemented
2. Errata ID #126
   SMMU doesnt support unique IRQ lines and also MSI for gerror,
   eventq and cmdq-sync

The following patchset does software workaround for these two erratas.

This series is based on patchset.
https://www.spinics.net/lists/arm-kernel/msg578443.html

Changes since v5:
  - Rebased on Robin's "Update SMMU models for IORT rev. C" patch.
 https://www.spinics.net/lists/arm-kernel/msg580728.html
  - Replaced ACPI_IORT_SMMU_V3_CAVIUM_CN99XX macro with 
ACPI_IORT_SMMU_CAVIUM_CN99XX

Changes since v4:
 - Replaced all page1 offset macros ARM_SMMU_EVTQ/PRIQ_PROD/CONS with
arm_smmu_page1_fixup(ARM_SMMU_EVTQ/PRIQ_PROD/CONS, smmu)

Changes since v3:
 - Merged patches 1, 2 and 4 of Version 3.
 - Modified the page1_offset_adjust() and get_irq_flags() implementation as
   suggested by Robin.

Changes since v2:
 - Updated "Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt" document 
with
   new SMMU option used to enable errata workaround.

Changes since v1:
 - Since the use of MIDR register is rejected and SMMU_IIDR is broken on this
   silicon, as suggested by Will Deacon modified the patches to use ThunderX2
   SMMUv3 IORT model number to enable errata workaround. 


Geetha Sowjanya (1):
  iommu/arm-smmu-v3: Add workaround for Cavium ThunderX2 erratum #126

Linu Cherian (2):
  ACPI/IORT: Fixup SMMUv3 resource size for Cavium ThunderX2 SMMUv3
model
  iommu/arm-smmu-v3: Add workaround for Cavium ThunderX2 erratum #74

 Documentation/arm64/silicon-errata.txt |  2 +
 .../devicetree/bindings/iommu/arm,smmu-v3.txt  |  6 ++
 drivers/acpi/arm64/iort.c  | 10 ++-
 drivers/iommu/arm-smmu-v3.c| 93 +-
 4 files changed, 91 insertions(+), 20 deletions(-)

-- 
1.8.3.1

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

Re: [RFC PATCH 7/8] VFIO: Add new IOCTL for IOMMU TLB invalidate propagation

2017-05-12 Thread Jean-Philippe Brucker 
Hi Yi,

On 26/04/17 11:12, Liu, Yi L wrote:
> From: "Liu, Yi L" 
> 
> This patch adds VFIO_IOMMU_TLB_INVALIDATE to propagate IOMMU TLB
> invalidate request from guest to host.
> 
> In the case of SVM virtualization on VT-d, host IOMMU driver has
> no knowledge of caching structure updates unless the guest
> invalidation activities are passed down to the host. So a new
> IOCTL is needed to propagate the guest cache invalidation through
> VFIO.
> 
> Signed-off-by: Liu, Yi L 
> ---
>  include/uapi/linux/vfio.h | 9 +
>  1 file changed, 9 insertions(+)
> 
> diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
> index 6b97987..50c51f8 100644
> --- a/include/uapi/linux/vfio.h
> +++ b/include/uapi/linux/vfio.h
> @@ -564,6 +564,15 @@ struct vfio_device_svm {
>  
>  #define VFIO_IOMMU_SVM_BIND_TASK _IO(VFIO_TYPE, VFIO_BASE + 22)
>  
> +/* For IOMMU TLB Invalidation Propagation */
> +struct vfio_iommu_tlb_invalidate {
> + __u32   argsz;
> + __u32   length;
> + __u8data[];
> +};

We initially discussed something a little more generic than this, with
most info explicitly described and only pIOMMU-specific quirks and hints
in an opaque structure. Out of curiosity, why the change? I'm not against
a fully opaque structure, but there seem to be a large overlap between TLB
invalidations across architectures.


For what it's worth, when prototyping the paravirtualized IOMMU I came up
with the following.

(From the paravirtualized POV, the SMMU also has to swizzle endianess
after unpacking an opaque structure, since userspace doesn't know what's
in it and guest might use a different endianess. So we need to force all
opaque data to be e.g. little-endian.)

struct vfio_iommu_tlb_invalidate {
__u32   argsz;
__u32   scope;
__u32   flags;
__u32   pasid;
__u64   vaddr;
__u64   size;
__u8data[];
};

Scope is a bitfields restricting the invalidation scope. By default
invalidate the whole container (all PASIDs and all VAs). @pasid, @vaddr
and @size are unused.

Adding VFIO_IOMMU_INVALIDATE_PASID (1 << 0) restricts the invalidation
scope to the pasid described by @pasid.
Adding VFIO_IOMMU_INVALIDATE_VADDR (1 << 1) restricts the invalidation
scope to the address range described by (@vaddr, @size).

So setting scope = VFIO_IOMMU_INVALIDATE_VADDR would invalidate the VA
range for *all* pasids (as well as no_pasid). Setting scope =
(VFIO_IOMMU_INVALIDATE_VADDR|VFIO_IOMMU_INVALIDATE_PASID) would invalidate
the VA range only for @pasid.

Flags depend on the selected scope:

VFIO_IOMMU_INVALIDATE_NO_PASID, indicating that invalidation (either
without scope or with INVALIDATE_VADDR) targets non-pasid mappings
exclusively (some architectures, e.g. SMMU, allow this)

VFIO_IOMMU_INVALIDATE_VADDR_LEAF, indicating that the pIOMMU doesn't need
to invalidate all intermediate tables cached as part of the PTW for vaddr,
only the last-level entry (pte). This is a hint.

I guess what's missing for Intel IOMMU and would go in @data is the
"global" hint (which we don't have in SMMU invalidations). Do you see
anything else, that the pIOMMU cannot deduce from this structure?

Thanks,
Jean


> +#define VFIO_IOMMU_TLB_INVALIDATE_IO(VFIO_TYPE, VFIO_BASE + 23)
> +
>  /*  Additional API for SPAPR TCE (Server POWERPC) IOMMU  */
>  
>  /*
> 

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

Re: [v5 1/4] ACPICA: IORT: Add Cavium ThunderX2 SMMUv3 model definition.

2017-05-12 Thread Geetha Akula 
On Fri, May 12, 2017 at 3:54 PM, Will Deacon  wrote:
> On Thu, May 11, 2017 at 04:40:51PM +0200, Rafael J. Wysocki wrote:
>> On Thursday, May 11, 2017 09:45:25 AM Will Deacon wrote:
>> > On Thu, May 11, 2017 at 02:26:02AM +0200, Rafael J. Wysocki wrote:
>> > > On Wednesday, May 10, 2017 05:01:55 PM Geetha sowjanya wrote:
>> > > > From: Linu Cherian 
>> > > >
>> > > > Add SMMUv3 model definition for ThunderX2.
>> > > >
>> > > > Signed-off-by: Linu Cherian 
>> > > > Signed-off-by: Geetha Sowjanya 
>> > >
>> > > This is an ACPICA change, but you have not included the ACPICA 
>> > > maintainers
>> > > into your original CC list (added now).
>> > >
>> > > Bob, Lv, how should this be routed?
>> > >
>> > > Do you want to apply this patch upstream first or can we make this 
>> > > change in
>> > > Linux and upstream in parallel?  That shouldn't be a big deal, right?
>> >
>> > I think we're still waiting for the updated IORT document to be published 
>> > (I
>> > think this should be in the next week or so), so I don't think we should
>> > commit the new ID before that happens.
>>
>> OK, thanks for the heads-up.
>>
>> I think it's better to submit the actbl2.h update directly to ACPICA
>> upstream when the doc is published and then work on top of that.
>
> The doc is now published:
>
> http://infocenter.arm.com/help/topic/com.arm.doc.den0049c/DEN0049C_IO_Remapping_Table.pdf
>
> so the new model numbers are confirmed.
>
> Cheers,
>
> Will

Thanks Will.

Will resubmit the patches based on Robin's latest patch "acpica: iort:
Update SMMU models for IORT rev. C",
as there is a miss match in the macro name used to define ThunderX2
iort model number.
https://lkml.org/lkml/2017/5/12/211



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

Re: [Qemu-devel] [RFC PATCH 6/8] VFIO: do pasid table binding

2017-05-12 Thread Liu, Yi L 
On Tue, May 09, 2017 at 03:55:20PM +0800, Xiao Guangrong wrote:
> 
> 
> On 04/26/2017 06:12 PM, Liu, Yi L wrote:
> >From: "Liu, Yi L" 
> >
> >This patch adds IOCTL processing in vfio_iommu_type1 for
> >VFIO_IOMMU_SVM_BIND_TASK. Binds the PASID table bind by
> >calling iommu_ops->bind_pasid_table to link the whole
> >PASID table to pIOMMU.
> >
> >For VT-d, it is linking the guest PASID table to host pIOMMU.
> >This is key point to support SVM virtualization on VT-d.
> >
> >Signed-off-by: Liu, Yi L 
> >---
> >  drivers/vfio/vfio_iommu_type1.c | 72 
> > +
> >  1 file changed, 72 insertions(+)
> >
> >diff --git a/drivers/vfio/vfio_iommu_type1.c 
> >b/drivers/vfio/vfio_iommu_type1.c
> >index b3cc33f..30b6d48 100644
> >--- a/drivers/vfio/vfio_iommu_type1.c
> >+++ b/drivers/vfio/vfio_iommu_type1.c
> >@@ -1512,6 +1512,50 @@ static int vfio_domains_have_iommu_cache(struct 
> >vfio_iommu *iommu)
> > return ret;
> >  }
> >+struct vfio_svm_task {
> >+struct iommu_domain *domain;
> >+void *payload;
> >+};
> >+
> >+static int bind_pasid_tbl_fn(struct device *dev, void *data)
> >+{
> >+int ret = 0;
> >+struct vfio_svm_task *task = data;
> >+struct pasid_table_info *pasidt_binfo;
> >+
> >+pasidt_binfo = task->payload;
> >+ret = iommu_bind_pasid_table(task->domain, dev, pasidt_binfo);
> >+return ret;
> >+}
> >+
> >+static int vfio_do_svm_task(struct vfio_iommu *iommu, void *data,
> >+int (*fn)(struct device *, void *))
> >+{
> >+int ret = 0;
> >+struct vfio_domain *d;
> >+struct vfio_group *g;
> >+struct vfio_svm_task task;
> >+
> >+task.payload = data;
> >+
> >+mutex_lock(>lock);
> >+
> >+list_for_each_entry(d, >domain_list, next) {
> >+list_for_each_entry(g, >group_list, next) {
> >+if (g->iommu_group != NULL) {
> >+task.domain = d->domain;
> >+ret = iommu_group_for_each_dev(
> >+g->iommu_group, , fn);
> >+if (ret != 0)
> >+break;
> >+}
> >+}
> >+}
> >+
> >+mutex_unlock(>lock);
> >+return ret;
> >+}
> >+
> >  static long vfio_iommu_type1_ioctl(void *iommu_data,
> >unsigned int cmd, unsigned long arg)
> >  {
> >@@ -1582,6 +1626,34 @@ static long vfio_iommu_type1_ioctl(void *iommu_data,
> > return copy_to_user((void __user *)arg, , minsz) ?
> > -EFAULT : 0;
> >+} else if (cmd == VFIO_IOMMU_SVM_BIND_TASK) {
> >+struct vfio_device_svm hdr;
> >+u8 *data = NULL;
> >+int ret = 0;
> >+
> >+minsz = offsetofend(struct vfio_device_svm, length);
> >+if (copy_from_user(, (void __user *)arg, minsz))
> >+return -EFAULT;
> >+
> >+if (hdr.length == 0)
> >+return -EINVAL;
> >+
> >+data = memdup_user((void __user *)(arg + minsz),
> >+hdr.length);
> 
> You should check the @length is at least sizeof(struct pasid_table_info) as
> kernel uses it as pasid_table_info, a evil application can crash kernel.

Yes, thx for the remind.

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

Re: [Qemu-devel] [RFC PATCH 5/8] VFIO: Add new IOTCL for PASID Table bind propagation

2017-05-12 Thread Liu, Yi L 
On Wed, Apr 26, 2017 at 06:12:02PM +0800, Liu, Yi L wrote:
> From: "Liu, Yi L" 

Hi Alex,

In this patchset, I'm trying to add two new IOCTL cmd for Shared
Virtual Memory virtualization. One for binding guest PASID Table
and one for iommu tlb invalidation from guest. ARM has similar
requirement on SVM supporting. Since it touched VFIO, I'd like
to know your comments on changes in VFIO.

Thanks,
Yi L

> This patch adds VFIO_IOMMU_SVM_BIND_TASK for potential PASID table
> binding requests.
> 
> On VT-d, this IOCTL cmd would be used to link the guest PASID page table
> to host. While for other vendors, it may also be used to support other
> kind of SVM bind request. Previously, there is a discussion on it with
> ARM engineer. It can be found by the link below. This IOCTL cmd may
> support SVM PASID bind request from userspace driver, or page table(cr3)
> bind request from guest. These SVM bind requests would be supported by
> adding different flags. e.g. VFIO_SVM_BIND_PASID is added to support
> PASID bind from userspace driver, VFIO_SVM_BIND_PGTABLE is added to
> support page table bind from guest.
> 
> https://patchwork.kernel.org/patch/9594231/
> 
> Signed-off-by: Liu, Yi L 
> ---
>  include/uapi/linux/vfio.h | 17 +
>  1 file changed, 17 insertions(+)
> 
> diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
> index 519eff3..6b97987 100644
> --- a/include/uapi/linux/vfio.h
> +++ b/include/uapi/linux/vfio.h
> @@ -547,6 +547,23 @@ struct vfio_iommu_type1_dma_unmap {
>  #define VFIO_IOMMU_ENABLE_IO(VFIO_TYPE, VFIO_BASE + 15)
>  #define VFIO_IOMMU_DISABLE   _IO(VFIO_TYPE, VFIO_BASE + 16)
>  
> +/* IOCTL for Shared Virtual Memory Bind */
> +struct vfio_device_svm {
> + __u32   argsz;
> +#define VFIO_SVM_BIND_PASIDTBL   (1 << 0) /* Bind PASID Table */
> +#define VFIO_SVM_BIND_PASID  (1 << 1) /* Bind PASID from userspace driver */
> +#define VFIO_SVM_BIND_PGTABLE(1 << 2) /* Bind guest mmu page table */
> + __u32   flags;
> + __u32   length;
> + __u8data[];
> +};
> +
> +#define VFIO_SVM_TYPE_MASK   (VFIO_SVM_BIND_PASIDTBL | \
> + VFIO_SVM_BIND_PASID | \
> + VFIO_SVM_BIND_PGTABLE)
> +
> +#define VFIO_IOMMU_SVM_BIND_TASK _IO(VFIO_TYPE, VFIO_BASE + 22)
> +
>  /*  Additional API for SPAPR TCE (Server POWERPC) IOMMU  */
>  
>  /*
> -- 
> 1.9.1
> 
> 
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH 2/2] iommu/arm-smmu: Plumb in new ACPI identifiers

2017-05-12 Thread Robin Murphy 
Revision C of IORT now allows us to identify ARM MMU-401 and the Cavium
ThunderX implementation; wire them up so that the appropriate quirks get
enabled when booting with ACPI.

Signed-off-by: Robin Murphy 
---
 drivers/iommu/arm-smmu.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 6dadd51d486c..d9ec840defc9 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -2018,6 +2018,10 @@ static int acpi_smmu_get_data(u32 model, struct 
arm_smmu_device *smmu)
smmu->version = ARM_SMMU_V1;
smmu->model = GENERIC_SMMU;
break;
+   case ACPI_IORT_SMMU_CORELINK_MMU401:
+   smmu->version = ARM_SMMU_V1_64K;
+   smmu->model = GENERIC_SMMU;
+   break;
case ACPI_IORT_SMMU_V2:
smmu->version = ARM_SMMU_V2;
smmu->model = GENERIC_SMMU;
@@ -2026,6 +2030,10 @@ static int acpi_smmu_get_data(u32 model, struct 
arm_smmu_device *smmu)
smmu->version = ARM_SMMU_V2;
smmu->model = ARM_MMU500;
break;
+   case ACPI_IORT_SMMU_CAVIUM_SMMUV2:
+   smmu->version = ARM_SMMU_V2;
+   smmu->model = CAVIUM_SMMUV2;
+   break;
default:
ret = -ENODEV;
}
-- 
2.12.2.dirty

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

[PATCH 1/2] acpica: iort: Update SMMU models for IORT rev. C

2017-05-12 Thread Robin Murphy 
IORT revision C has been published with a number of new SMMU
implementation identifiers; define them.

CC: Rafael J. Wysocki 
CC: Robert Moore 
CC: Lv Zheng 
Signed-off-by: Robin Murphy 
---
 include/acpi/actbl2.h | 8 
 1 file changed, 8 insertions(+)

diff --git a/include/acpi/actbl2.h b/include/acpi/actbl2.h
index 7aee9fb3bd1f..0242be07f292 100644
--- a/include/acpi/actbl2.h
+++ b/include/acpi/actbl2.h
@@ -777,6 +777,8 @@ struct acpi_iort_smmu {
 #define ACPI_IORT_SMMU_V2   0x0001 /* Generic SMMUv2 */
 #define ACPI_IORT_SMMU_CORELINK_MMU400  0x0002 /* ARM Corelink MMU-400 
*/
 #define ACPI_IORT_SMMU_CORELINK_MMU500  0x0003 /* ARM Corelink MMU-500 
*/
+#define ACPI_IORT_SMMU_CORELINK_MMU401  0x0004 /* ARM Corelink MMU-401 
*/
+#define ACPI_IORT_SMMU_CAVIUM_SMMUV20x0005 /* Cavium ThunderX 
SMMUv2 */
 
 /* Masks for Flags field above */
 
@@ -795,6 +797,12 @@ struct acpi_iort_smmu_v3 {
u32 sync_gsiv;
 };
 
+/* Values for Model field above */
+
+#define ACPI_IORT_SMMU_V3   0x /* Generic SMMUv3 */
+#define ACPI_IORT_SMMU_HISILICON_HI161X 0x0001 /* HiSilicon Hi161x 
SMMUv3 */
+#define ACPI_IORT_SMMU_CAVIUM_CN99XX0x0002 /* Cavium CN99xx SMMUv3 
*/
+
 /* Masks for Flags field above */
 
 #define ACPI_IORT_SMMU_V3_COHACC_OVERRIDE   (1)
-- 
2.12.2.dirty

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

Re: [v5 1/4] ACPICA: IORT: Add Cavium ThunderX2 SMMUv3 model definition.

2017-05-12 Thread Will Deacon 
On Thu, May 11, 2017 at 04:40:51PM +0200, Rafael J. Wysocki wrote:
> On Thursday, May 11, 2017 09:45:25 AM Will Deacon wrote:
> > On Thu, May 11, 2017 at 02:26:02AM +0200, Rafael J. Wysocki wrote:
> > > On Wednesday, May 10, 2017 05:01:55 PM Geetha sowjanya wrote:
> > > > From: Linu Cherian 
> > > > 
> > > > Add SMMUv3 model definition for ThunderX2.
> > > > 
> > > > Signed-off-by: Linu Cherian 
> > > > Signed-off-by: Geetha Sowjanya 
> > > 
> > > This is an ACPICA change, but you have not included the ACPICA maintainers
> > > into your original CC list (added now).
> > > 
> > > Bob, Lv, how should this be routed?
> > > 
> > > Do you want to apply this patch upstream first or can we make this change 
> > > in
> > > Linux and upstream in parallel?  That shouldn't be a big deal, right?
> > 
> > I think we're still waiting for the updated IORT document to be published (I
> > think this should be in the next week or so), so I don't think we should
> > commit the new ID before that happens.
> 
> OK, thanks for the heads-up.
> 
> I think it's better to submit the actbl2.h update directly to ACPICA
> upstream when the doc is published and then work on top of that.

The doc is now published:

http://infocenter.arm.com/help/topic/com.arm.doc.den0049c/DEN0049C_IO_Remapping_Table.pdf

so the new model numbers are confirmed.

Cheers,

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