Re: [PATCH v3 2/2] iommu/vt-d:Add support for probing ACPI device in RMRR
Hi Felix,
On 8/27/20 6:02 PM, FelixCuioc wrote:
After acpi device in RMRR is detected,it is necessary
to establish a mapping for these devices.
In acpi_device_create_direct_mappings(),create a mapping
for the acpi device in RMRR.
Add a helper to achieve the acpi namespace device can
access the RMRR region.
Are those ACPI devices visible to kernel? If so, any device driver bound
for it?
Best regards,
baolu
Signed-off-by: FelixCuioc
---
drivers/iommu/intel/iommu.c | 29 +
drivers/iommu/iommu.c | 6 ++
include/linux/iommu.h | 3 +++
3 files changed, 38 insertions(+)
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 208a91605288..51d7a5b18f41 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -4799,6 +4799,21 @@ static int __init platform_optin_force_iommu(void)
return 1;
}
+static int acpi_device_create_direct_mappings(struct device *pn_dev, struct device *acpi_device)
+{
+ struct iommu_group *group;
+
+ acpi_device->bus->iommu_ops = &intel_iommu_ops;
+ group = iommu_group_get(pn_dev);
+ if (!group) {
+ pr_warn("ACPI name space devices create direct mappings
wrong!\n");
+ return -EINVAL;
+ }
+ __acpi_device_create_direct_mappings(group, acpi_device);
+
+ return 0;
+}
+
static int __init probe_acpi_namespace_devices(void)
{
struct dmar_drhd_unit *drhd;
@@ -4813,6 +4828,7 @@ static int __init probe_acpi_namespace_devices(void)
struct acpi_device_physical_node *pn;
struct iommu_group *group;
struct acpi_device *adev;
+ struct device *pn_dev = NULL;
if (dev->bus != &acpi_bus_type)
continue;
@@ -4823,6 +4839,7 @@ static int __init probe_acpi_namespace_devices(void)
&adev->physical_node_list, node) {
group = iommu_group_get(pn->dev);
if (group) {
+ pn_dev = pn->dev;
iommu_group_put(group);
continue;
}
@@ -4831,7 +4848,19 @@ static int __init probe_acpi_namespace_devices(void)
ret = iommu_probe_device(pn->dev);
if (ret)
break;
+ pn_dev = pn->dev;
+ }
+ if (!pn_dev) {
+ dev->bus->iommu_ops = &intel_iommu_ops;
+ ret = iommu_probe_device(dev);
+ if (ret) {
+ pr_err("acpi_device probe fail!
ret:%d\n", ret);
+ goto unlock;
+ }
+ goto unlock;
}
+ ret = acpi_device_create_direct_mappings(pn_dev, dev);
+unlock:
mutex_unlock(&adev->physical_node_lock);
if (ret)
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 609bd25bf154..4f714a2d5ef7 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -779,6 +779,12 @@ static bool iommu_is_attach_deferred(struct iommu_domain
*domain,
return false;
}
+void __acpi_device_create_direct_mappings(struct iommu_group *group, struct device *acpi_device)
+{
+ iommu_create_device_direct_mappings(group, acpi_device);
+}
+EXPORT_SYMBOL_GPL(__acpi_device_create_direct_mappings);
+
/**
* iommu_group_add_device - add a device to an iommu group
* @group: the group into which to add the device (reference should be held)
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index fee209efb756..9be134775886 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -514,6 +514,9 @@ extern void iommu_domain_window_disable(struct iommu_domain
*domain, u32 wnd_nr)
extern int report_iommu_fault(struct iommu_domain *domain, struct device *dev,
unsigned long iova, int flags);
+extern void __acpi_device_create_direct_mappings(struct iommu_group *group,
+ struct device *acpi_device);
+
static inline void iommu_flush_tlb_all(struct iommu_domain *domain)
{
if (domain->ops->flush_iotlb_all)
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 1/2] iommu/vt-d:Add support for detecting ACPI device in RMRR
Hi Felix,
On 8/27/20 6:02 PM, FelixCuioc wrote:
Some ACPI devices need to issue dma requests to access
the reserved memory area.BIOS uses the device scope type
ACPI_NAMESPACE_DEVICE in RMRR to report these ACPI devices.
This patch add support for detecting ACPI devices in RMRR.
Signed-off-by: FelixCuioc
---
drivers/iommu/intel/dmar.c | 76 +
drivers/iommu/intel/iommu.c | 23 ++-
include/linux/dmar.h| 12 +-
3 files changed, 76 insertions(+), 35 deletions(-)
diff --git a/drivers/iommu/intel/dmar.c b/drivers/iommu/intel/dmar.c
index 93e6345f3414..f6691c36bd3f 100644
--- a/drivers/iommu/intel/dmar.c
+++ b/drivers/iommu/intel/dmar.c
@@ -215,7 +215,7 @@ static bool dmar_match_pci_path(struct dmar_pci_notify_info
*info, int bus,
}
/* Return: > 0 if match found, 0 if no match found, < 0 if error happens */
-int dmar_insert_dev_scope(struct dmar_pci_notify_info *info,
+int dmar_pci_insert_dev_scope(struct dmar_pci_notify_info *info,
void *start, void*end, u16 segment,
struct dmar_dev_scope *devices,
int devices_cnt)
@@ -304,7 +304,7 @@ static int dmar_pci_bus_add_dev(struct dmar_pci_notify_info
*info)
drhd = container_of(dmaru->hdr,
struct acpi_dmar_hardware_unit, header);
- ret = dmar_insert_dev_scope(info, (void *)(drhd + 1),
+ ret = dmar_pci_insert_dev_scope(info, (void *)(drhd + 1),
((void *)drhd) + drhd->header.length,
dmaru->segment,
dmaru->devices, dmaru->devices_cnt);
@@ -697,47 +697,59 @@ dmar_find_matched_drhd_unit(struct pci_dev *dev)
return dmaru;
}
-static void __init dmar_acpi_insert_dev_scope(u8 device_number,
- struct acpi_device *adev)
+/* Return: > 0 if match found, 0 if no match found */
+bool dmar_acpi_insert_dev_scope(u8 device_number,
+ struct acpi_device *adev,
+ void *start, void *end,
+ struct dmar_dev_scope *devices,
+ int devices_cnt)
{
- struct dmar_drhd_unit *dmaru;
- struct acpi_dmar_hardware_unit *drhd;
struct acpi_dmar_device_scope *scope;
struct device *tmp;
int i;
struct acpi_dmar_pci_path *path;
+ for (; start < end; start += scope->length) {
+ scope = start;
+ if (scope->entry_type != ACPI_DMAR_SCOPE_TYPE_NAMESPACE)
+ continue;
+ if (scope->enumeration_id != device_number)
+ continue;
+ path = (void *)(scope + 1);
+ for_each_dev_scope(devices, devices_cnt, i, tmp)
+ if (tmp == NULL) {
+ devices[i].bus = scope->bus;
+ devices[i].devfn = PCI_DEVFN(path->device,
path->function);
+ rcu_assign_pointer(devices[i].dev,
+ get_device(&adev->dev));
+ return true;
+ }
+ WARN_ON(i >= devices_cnt);
+ }
+ return false;
+}
+
+static int dmar_acpi_bus_add_dev(u8 device_number, struct acpi_device *adev)
+{
+ struct dmar_drhd_unit *dmaru;
+ struct acpi_dmar_hardware_unit *drhd;
+ int ret;
+
for_each_drhd_unit(dmaru) {
drhd = container_of(dmaru->hdr,
struct acpi_dmar_hardware_unit,
header);
+ ret = dmar_acpi_insert_dev_scope(device_number, adev, (void
*)(drhd+1),
+ ((void
*)drhd)+drhd->header.length,
+ dmaru->devices,
dmaru->devices_cnt);
+ if (ret)
+ break;
+ }
+ if (ret > 0)
+ ret = dmar_rmrr_add_acpi_dev(device_number, adev);
- for (scope = (void *)(drhd + 1);
-(unsigned long)scope < ((unsigned long)drhd) +
drhd->header.length;
-scope = ((void *)scope) + scope->length) {
- if (scope->entry_type != ACPI_DMAR_SCOPE_TYPE_NAMESPACE)
- continue;
- if (scope->enumeration_id != device_number)
- continue;
+ return ret;
- path = (void *)(scope + 1);
- pr_info("ACPI device \"%s\" under DMAR at %llx as
%02x:%02x.%d\n",
- dev_name(&adev->dev), dmaru->reg_base_addr,
- scope->bus, path->device, path->function);
Please keep such information. People are used to use them as a method to
know the hardwa
Re: [PATCH 01/19] drm/msm: remove dangling submitqueue references
On Tue 01 Sep 03:42 UTC 2020, Rob Clark wrote:
> On Mon, Aug 31, 2020 at 7:35 PM Bjorn Andersson
> wrote:
> >
> > On Fri 14 Aug 02:40 UTC 2020, Rob Clark wrote:
> >
> > > From: Rob Clark
> > >
> > > Currently it doesn't matter, since we free the ctx immediately. But
> > > when we start refcnt'ing the ctx, we don't want old dangling list
> > > entries to hang around.
> > >
> > > Signed-off-by: Rob Clark
> > > ---
> > > drivers/gpu/drm/msm/msm_submitqueue.c | 4 +++-
> > > 1 file changed, 3 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/msm/msm_submitqueue.c
> > > b/drivers/gpu/drm/msm/msm_submitqueue.c
> > > index a1d94be7883a..90c9d84e6155 100644
> > > --- a/drivers/gpu/drm/msm/msm_submitqueue.c
> > > +++ b/drivers/gpu/drm/msm/msm_submitqueue.c
> > > @@ -49,8 +49,10 @@ void msm_submitqueue_close(struct msm_file_private
> > > *ctx)
> > >* No lock needed in close and there won't
> > >* be any more user ioctls coming our way
> > >*/
> > > - list_for_each_entry_safe(entry, tmp, &ctx->submitqueues, node)
> > > + list_for_each_entry_safe(entry, tmp, &ctx->submitqueues, node) {
> > > + list_del(&entry->node);
> >
> > If you refcount ctx, what does that do for the entries in the submit
> > queue?
> >
> > "entry" here is kref'ed, but you're popping it off the list regardless
> > of the put ends up freeing the object or not - which afaict would mean
> > leaking the object.
> >
>
> What ends up happening is the submit has reference to submit-queue,
> which has reference to the ctx.. the submitqueue could be alive still
> pending in-flight submits (in a later patch), but dead from the PoV of
> userspace interface.
>
> We aren't relying (or at least aren't in the end, and I *think* I
> didn't miss anything in the middle) relying on ctx->submitqueues list
> to clean anything up in the end, just track what is still a valid
> submitqueue from userspace PoV
>
Looks reasonable, thanks for the explanation.
> BR,
> -R
>
> >
> > On the other hand, with the current implementation an object with higher
> > refcount with adjacent objects of single refcount would end up with
> > dangling pointers after the put. So in itself this change seems like a
> > net gain, but I'm wondering about the plan described in the commit
> > message.
> >
> > Regards,
> > Bjorn
> >
> > > msm_submitqueue_put(entry);
> > > + }
> > > }
> > >
> > > int msm_submitqueue_create(struct drm_device *drm, struct
> > > msm_file_private *ctx,
> > > --
> > > 2.26.2
> > >
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 01/19] drm/msm: remove dangling submitqueue references
On Fri 14 Aug 02:40 UTC 2020, Rob Clark wrote:
> From: Rob Clark
>
> Currently it doesn't matter, since we free the ctx immediately. But
> when we start refcnt'ing the ctx, we don't want old dangling list
> entries to hang around.
>
> Signed-off-by: Rob Clark
Reviewed-by: Bjorn Andersson
> ---
> drivers/gpu/drm/msm/msm_submitqueue.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/msm/msm_submitqueue.c
> b/drivers/gpu/drm/msm/msm_submitqueue.c
> index a1d94be7883a..90c9d84e6155 100644
> --- a/drivers/gpu/drm/msm/msm_submitqueue.c
> +++ b/drivers/gpu/drm/msm/msm_submitqueue.c
> @@ -49,8 +49,10 @@ void msm_submitqueue_close(struct msm_file_private *ctx)
>* No lock needed in close and there won't
>* be any more user ioctls coming our way
>*/
> - list_for_each_entry_safe(entry, tmp, &ctx->submitqueues, node)
> + list_for_each_entry_safe(entry, tmp, &ctx->submitqueues, node) {
> + list_del(&entry->node);
> msm_submitqueue_put(entry);
> + }
> }
>
> int msm_submitqueue_create(struct drm_device *drm, struct msm_file_private
> *ctx,
> --
> 2.26.2
>
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 19/19] drm/msm: show process names in gem_describe
On Thu 13 Aug 21:41 CDT 2020, Rob Clark wrote:
> From: Rob Clark
>
> In $debugfs/gem we already show any vma(s) associated with an object.
> Also show process names if the vma's address space is a per-process
> address space.
>
Reviewed-by: Bjorn Andersson
> Signed-off-by: Rob Clark
> ---
> drivers/gpu/drm/msm/msm_drv.c | 2 +-
> drivers/gpu/drm/msm/msm_gem.c | 25 +
> drivers/gpu/drm/msm/msm_gem.h | 5 +
> drivers/gpu/drm/msm/msm_gem_vma.c | 1 +
> drivers/gpu/drm/msm/msm_gpu.c | 8 +---
> drivers/gpu/drm/msm/msm_gpu.h | 2 +-
> 6 files changed, 34 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
> index 8e70d220bba8..8d5c4f98c332 100644
> --- a/drivers/gpu/drm/msm/msm_drv.c
> +++ b/drivers/gpu/drm/msm/msm_drv.c
> @@ -597,7 +597,7 @@ static int context_init(struct drm_device *dev, struct
> drm_file *file)
> kref_init(&ctx->ref);
> msm_submitqueue_init(dev, ctx);
>
> - ctx->aspace = msm_gpu_create_private_address_space(priv->gpu);
> + ctx->aspace = msm_gpu_create_private_address_space(priv->gpu, current);
> file->driver_priv = ctx;
>
> return 0;
> diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c
> index 3cb7aeb93fd3..76a6c5271e57 100644
> --- a/drivers/gpu/drm/msm/msm_gem.c
> +++ b/drivers/gpu/drm/msm/msm_gem.c
> @@ -842,11 +842,28 @@ void msm_gem_describe(struct drm_gem_object *obj,
> struct seq_file *m)
>
> seq_puts(m, " vmas:");
>
> - list_for_each_entry(vma, &msm_obj->vmas, list)
> - seq_printf(m, " [%s: %08llx,%s,inuse=%d]",
> - vma->aspace != NULL ? vma->aspace->name : NULL,
> - vma->iova, vma->mapped ? "mapped" : "unmapped",
> + list_for_each_entry(vma, &msm_obj->vmas, list) {
> + const char *name, *comm;
> + if (vma->aspace) {
> + struct msm_gem_address_space *aspace =
> vma->aspace;
> + struct task_struct *task =
> + get_pid_task(aspace->pid, PIDTYPE_PID);
> + if (task) {
> + comm = kstrdup(task->comm, GFP_KERNEL);
> + } else {
> + comm = NULL;
> + }
> + name = aspace->name;
> + } else {
> + name = comm = NULL;
> + }
> + seq_printf(m, " [%s%s%s: aspace=%p,
> %08llx,%s,inuse=%d]",
> + name, comm ? ":" : "", comm ? comm : "",
> + vma->aspace, vma->iova,
> + vma->mapped ? "mapped" : "unmapped",
> vma->inuse);
> + kfree(comm);
> + }
>
> seq_puts(m, "\n");
> }
> diff --git a/drivers/gpu/drm/msm/msm_gem.h b/drivers/gpu/drm/msm/msm_gem.h
> index 9c573c4269cb..7b1c7a5f8eef 100644
> --- a/drivers/gpu/drm/msm/msm_gem.h
> +++ b/drivers/gpu/drm/msm/msm_gem.h
> @@ -24,6 +24,11 @@ struct msm_gem_address_space {
> spinlock_t lock; /* Protects drm_mm node allocation/removal */
> struct msm_mmu *mmu;
> struct kref kref;
> +
> + /* For address spaces associated with a specific process, this
> + * will be non-NULL:
> + */
> + struct pid *pid;
> };
>
> struct msm_gem_vma {
> diff --git a/drivers/gpu/drm/msm/msm_gem_vma.c
> b/drivers/gpu/drm/msm/msm_gem_vma.c
> index 29cc1305cf37..80a8a266d68f 100644
> --- a/drivers/gpu/drm/msm/msm_gem_vma.c
> +++ b/drivers/gpu/drm/msm/msm_gem_vma.c
> @@ -17,6 +17,7 @@ msm_gem_address_space_destroy(struct kref *kref)
> drm_mm_takedown(&aspace->mm);
> if (aspace->mmu)
> aspace->mmu->funcs->destroy(aspace->mmu);
> + put_pid(aspace->pid);
> kfree(aspace);
> }
>
> diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c
> index 951850804d77..ac8961187a73 100644
> --- a/drivers/gpu/drm/msm/msm_gpu.c
> +++ b/drivers/gpu/drm/msm/msm_gpu.c
> @@ -825,10 +825,9 @@ static int get_clocks(struct platform_device *pdev,
> struct msm_gpu *gpu)
>
> /* Return a new address space for a msm_drm_private instance */
> struct msm_gem_address_space *
> -msm_gpu_create_private_address_space(struct msm_gpu *gpu)
> +msm_gpu_create_private_address_space(struct msm_gpu *gpu, struct task_struct
> *task)
> {
> struct msm_gem_address_space *aspace = NULL;
> -
> if (!gpu)
> return NULL;
>
> @@ -836,8 +835,11 @@ msm_gpu_create_private_address_space(struct msm_gpu *gpu)
>* If the target doesn't support private address spaces then return
>* the global one
>*/
> - if (gpu->funcs->create_priva
Re: [PATCH 18/19] iommu/arm-smmu: add a way for implementations to influence SCTLR
On Thu 13 Aug 21:41 CDT 2020, Rob Clark wrote:
> From: Rob Clark
>
> For the Adreno GPU's SMMU, we want SCTLR.HUPCF set to ensure that
> pending translations are not terminated on iova fault. Otherwise
> a terminated CP read could hang the GPU by returning invalid
> command-stream data.
>
Reviewed-by: Bjorn Andersson
> Signed-off-by: Rob Clark
> ---
> drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 6 ++
> drivers/iommu/arm/arm-smmu/arm-smmu.c | 3 +++
> drivers/iommu/arm/arm-smmu/arm-smmu.h | 3 +++
> 3 files changed, 12 insertions(+)
>
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> index 5640d9960610..2aa6249050ff 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> @@ -127,6 +127,12 @@ static int qcom_adreno_smmu_init_context(struct
> arm_smmu_domain *smmu_domain,
> (smmu_domain->cfg.fmt == ARM_SMMU_CTX_FMT_AARCH64))
> pgtbl_cfg->quirks |= IO_PGTABLE_QUIRK_ARM_TTBR1;
>
> + /*
> + * On the GPU device we want to process subsequent transactions after a
> + * fault to keep the GPU from hanging
> + */
> + smmu_domain->cfg.sctlr_set |= ARM_SMMU_SCTLR_HUPCF;
> +
> /*
>* Initialize private interface with GPU:
>*/
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c
> b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> index e63a480d7f71..bbec5793faf8 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> @@ -617,6 +617,9 @@ void arm_smmu_write_context_bank(struct arm_smmu_device
> *smmu, int idx)
> if (IS_ENABLED(CONFIG_CPU_BIG_ENDIAN))
> reg |= ARM_SMMU_SCTLR_E;
>
> + reg |= cfg->sctlr_set;
> + reg &= ~cfg->sctlr_clr;
> +
> arm_smmu_cb_write(smmu, idx, ARM_SMMU_CB_SCTLR, reg);
> }
>
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.h
> b/drivers/iommu/arm/arm-smmu/arm-smmu.h
> index cd75a33967bb..2df3a70a8a41 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu.h
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.h
> @@ -144,6 +144,7 @@ enum arm_smmu_cbar_type {
> #define ARM_SMMU_CB_SCTLR0x0
> #define ARM_SMMU_SCTLR_S1_ASIDPNEBIT(12)
> #define ARM_SMMU_SCTLR_CFCFG BIT(7)
> +#define ARM_SMMU_SCTLR_HUPCF BIT(8)
> #define ARM_SMMU_SCTLR_CFIE BIT(6)
> #define ARM_SMMU_SCTLR_CFRE BIT(5)
> #define ARM_SMMU_SCTLR_E BIT(4)
> @@ -341,6 +342,8 @@ struct arm_smmu_cfg {
> u16 asid;
> u16 vmid;
> };
> + u32 sctlr_set;/* extra bits to set in
> SCTLR */
> + u32 sctlr_clr;/* bits to mask in SCTLR
> */
> enum arm_smmu_cbar_type cbar;
> enum arm_smmu_context_fmt fmt;
> };
> --
> 2.26.2
>
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 15/19] drm/msm: Add support for private address space instances
On Thu 13 Aug 21:41 CDT 2020, Rob Clark wrote:
> From: Jordan Crouse
>
> Add support for allocating private address space instances. Targets that
> support per-context pagetables should implement their own function to
> allocate private address spaces.
>
> The default will return a pointer to the global address space.
>
Reviewed-by: Bjorn Andersson
> Signed-off-by: Jordan Crouse
> Signed-off-by: Rob Clark
> ---
> drivers/gpu/drm/msm/msm_drv.c | 13 +++--
> drivers/gpu/drm/msm/msm_drv.h | 5 +
> drivers/gpu/drm/msm/msm_gem_vma.c | 9 +
> drivers/gpu/drm/msm/msm_gpu.c | 22 ++
> drivers/gpu/drm/msm/msm_gpu.h | 5 +
> 5 files changed, 48 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
> index 01845a3b8d52..8e70d220bba8 100644
> --- a/drivers/gpu/drm/msm/msm_drv.c
> +++ b/drivers/gpu/drm/msm/msm_drv.c
> @@ -597,7 +597,7 @@ static int context_init(struct drm_device *dev, struct
> drm_file *file)
> kref_init(&ctx->ref);
> msm_submitqueue_init(dev, ctx);
>
> - ctx->aspace = priv->gpu ? priv->gpu->aspace : NULL;
> + ctx->aspace = msm_gpu_create_private_address_space(priv->gpu);
> file->driver_priv = ctx;
>
> return 0;
> @@ -780,18 +780,19 @@ static int msm_ioctl_gem_cpu_fini(struct drm_device
> *dev, void *data,
> }
>
> static int msm_ioctl_gem_info_iova(struct drm_device *dev,
> - struct drm_gem_object *obj, uint64_t *iova)
> + struct drm_file *file, struct drm_gem_object *obj,
> + uint64_t *iova)
> {
> - struct msm_drm_private *priv = dev->dev_private;
> + struct msm_file_private *ctx = file->driver_priv;
>
> - if (!priv->gpu)
> + if (!ctx->aspace)
> return -EINVAL;
>
> /*
>* Don't pin the memory here - just get an address so that userspace can
>* be productive
>*/
> - return msm_gem_get_iova(obj, priv->gpu->aspace, iova);
> + return msm_gem_get_iova(obj, ctx->aspace, iova);
> }
>
> static int msm_ioctl_gem_info(struct drm_device *dev, void *data,
> @@ -830,7 +831,7 @@ static int msm_ioctl_gem_info(struct drm_device *dev,
> void *data,
> args->value = msm_gem_mmap_offset(obj);
> break;
> case MSM_INFO_GET_IOVA:
> - ret = msm_ioctl_gem_info_iova(dev, obj, &args->value);
> + ret = msm_ioctl_gem_info_iova(dev, file, obj, &args->value);
> break;
> case MSM_INFO_SET_NAME:
> /* length check should leave room for terminating null: */
> diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
> index 4561bfb5e745..2ca9c3c03845 100644
> --- a/drivers/gpu/drm/msm/msm_drv.h
> +++ b/drivers/gpu/drm/msm/msm_drv.h
> @@ -249,6 +249,10 @@ int msm_gem_map_vma(struct msm_gem_address_space *aspace,
> void msm_gem_close_vma(struct msm_gem_address_space *aspace,
> struct msm_gem_vma *vma);
>
> +
> +struct msm_gem_address_space *
> +msm_gem_address_space_get(struct msm_gem_address_space *aspace);
> +
> void msm_gem_address_space_put(struct msm_gem_address_space *aspace);
>
> struct msm_gem_address_space *
> @@ -434,6 +438,7 @@ static inline void __msm_file_private_destroy(struct kref
> *kref)
> struct msm_file_private *ctx = container_of(kref,
> struct msm_file_private, ref);
>
> + msm_gem_address_space_put(ctx->aspace);
> kfree(ctx);
> }
>
> diff --git a/drivers/gpu/drm/msm/msm_gem_vma.c
> b/drivers/gpu/drm/msm/msm_gem_vma.c
> index 5f6a11211b64..29cc1305cf37 100644
> --- a/drivers/gpu/drm/msm/msm_gem_vma.c
> +++ b/drivers/gpu/drm/msm/msm_gem_vma.c
> @@ -27,6 +27,15 @@ void msm_gem_address_space_put(struct
> msm_gem_address_space *aspace)
> kref_put(&aspace->kref, msm_gem_address_space_destroy);
> }
>
> +struct msm_gem_address_space *
> +msm_gem_address_space_get(struct msm_gem_address_space *aspace)
> +{
> + if (!IS_ERR_OR_NULL(aspace))
> + kref_get(&aspace->kref);
> +
> + return aspace;
> +}
> +
> /* Actually unmap memory for the vma */
> void msm_gem_purge_vma(struct msm_gem_address_space *aspace,
> struct msm_gem_vma *vma)
> diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c
> index e1a3cbe25a0c..951850804d77 100644
> --- a/drivers/gpu/drm/msm/msm_gpu.c
> +++ b/drivers/gpu/drm/msm/msm_gpu.c
> @@ -823,6 +823,28 @@ static int get_clocks(struct platform_device *pdev,
> struct msm_gpu *gpu)
> return 0;
> }
>
> +/* Return a new address space for a msm_drm_private instance */
> +struct msm_gem_address_space *
> +msm_gpu_create_private_address_space(struct msm_gpu *gpu)
> +{
> + struct msm_gem_address_space *aspace = NULL;
> +
> + if (!gpu)
> + return NULL;
> +
> + /*
> + * If the target doesn't support private address spaces then return
> + * the global one
> +
Re: [PATCH 14/19] drm/msm: Add support to create a local pagetable
On Thu 13 Aug 21:41 CDT 2020, Rob Clark wrote:
> From: Jordan Crouse
>
> Add support to create a io-pgtable for use by targets that support
> per-instance pagetables. In order to support per-instance pagetables the
> GPU SMMU device needs to have the qcom,adreno-smmu compatible string and
> split pagetables enabled.
>
Reviewed-by: Bjorn Andersson
> Signed-off-by: Jordan Crouse
> Signed-off-by: Rob Clark
> ---
> drivers/gpu/drm/msm/msm_gpummu.c | 2 +-
> drivers/gpu/drm/msm/msm_iommu.c | 199 ++-
> drivers/gpu/drm/msm/msm_mmu.h| 16 ++-
> 3 files changed, 214 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/msm_gpummu.c
> b/drivers/gpu/drm/msm/msm_gpummu.c
> index 310a31b05faa..aab121f4beb7 100644
> --- a/drivers/gpu/drm/msm/msm_gpummu.c
> +++ b/drivers/gpu/drm/msm/msm_gpummu.c
> @@ -102,7 +102,7 @@ struct msm_mmu *msm_gpummu_new(struct device *dev, struct
> msm_gpu *gpu)
> }
>
> gpummu->gpu = gpu;
> - msm_mmu_init(&gpummu->base, dev, &funcs);
> + msm_mmu_init(&gpummu->base, dev, &funcs, MSM_MMU_GPUMMU);
>
> return &gpummu->base;
> }
> diff --git a/drivers/gpu/drm/msm/msm_iommu.c b/drivers/gpu/drm/msm/msm_iommu.c
> index 1b6635504069..697cc0a059d6 100644
> --- a/drivers/gpu/drm/msm/msm_iommu.c
> +++ b/drivers/gpu/drm/msm/msm_iommu.c
> @@ -4,15 +4,210 @@
> * Author: Rob Clark
> */
>
> +#include
> +#include
> #include "msm_drv.h"
> #include "msm_mmu.h"
>
> struct msm_iommu {
> struct msm_mmu base;
> struct iommu_domain *domain;
> + atomic_t pagetables;
> };
> +
> #define to_msm_iommu(x) container_of(x, struct msm_iommu, base)
>
> +struct msm_iommu_pagetable {
> + struct msm_mmu base;
> + struct msm_mmu *parent;
> + struct io_pgtable_ops *pgtbl_ops;
> + phys_addr_t ttbr;
> + u32 asid;
> +};
> +static struct msm_iommu_pagetable *to_pagetable(struct msm_mmu *mmu)
> +{
> + return container_of(mmu, struct msm_iommu_pagetable, base);
> +}
> +
> +static int msm_iommu_pagetable_unmap(struct msm_mmu *mmu, u64 iova,
> + size_t size)
> +{
> + struct msm_iommu_pagetable *pagetable = to_pagetable(mmu);
> + struct io_pgtable_ops *ops = pagetable->pgtbl_ops;
> + size_t unmapped = 0;
> +
> + /* Unmap the block one page at a time */
> + while (size) {
> + unmapped += ops->unmap(ops, iova, 4096, NULL);
> + iova += 4096;
> + size -= 4096;
> + }
> +
> + iommu_flush_tlb_all(to_msm_iommu(pagetable->parent)->domain);
> +
> + return (unmapped == size) ? 0 : -EINVAL;
> +}
> +
> +static int msm_iommu_pagetable_map(struct msm_mmu *mmu, u64 iova,
> + struct sg_table *sgt, size_t len, int prot)
> +{
> + struct msm_iommu_pagetable *pagetable = to_pagetable(mmu);
> + struct io_pgtable_ops *ops = pagetable->pgtbl_ops;
> + struct scatterlist *sg;
> + size_t mapped = 0;
> + u64 addr = iova;
> + unsigned int i;
> +
> + for_each_sg(sgt->sgl, sg, sgt->nents, i) {
> + size_t size = sg->length;
> + phys_addr_t phys = sg_phys(sg);
> +
> + /* Map the block one page at a time */
> + while (size) {
> + if (ops->map(ops, addr, phys, 4096, prot, GFP_KERNEL)) {
> + msm_iommu_pagetable_unmap(mmu, iova, mapped);
> + return -EINVAL;
> + }
> +
> + phys += 4096;
> + addr += 4096;
> + size -= 4096;
> + mapped += 4096;
> + }
> + }
> +
> + return 0;
> +}
> +
> +static void msm_iommu_pagetable_destroy(struct msm_mmu *mmu)
> +{
> + struct msm_iommu_pagetable *pagetable = to_pagetable(mmu);
> + struct msm_iommu *iommu = to_msm_iommu(pagetable->parent);
> + struct adreno_smmu_priv *adreno_smmu =
> + dev_get_drvdata(pagetable->parent->dev);
> +
> + /*
> + * If this is the last attached pagetable for the parent,
> + * disable TTBR0 in the arm-smmu driver
> + */
> + if (atomic_dec_return(&iommu->pagetables) == 0)
> + adreno_smmu->set_ttbr0_cfg(adreno_smmu->cookie, NULL);
> +
> + free_io_pgtable_ops(pagetable->pgtbl_ops);
> + kfree(pagetable);
> +}
> +
> +int msm_iommu_pagetable_params(struct msm_mmu *mmu,
> + phys_addr_t *ttbr, int *asid)
> +{
> + struct msm_iommu_pagetable *pagetable;
> +
> + if (mmu->type != MSM_MMU_IOMMU_PAGETABLE)
> + return -EINVAL;
> +
> + pagetable = to_pagetable(mmu);
> +
> + if (ttbr)
> + *ttbr = pagetable->ttbr;
> +
> + if (asid)
> + *asid = pagetable->asid;
> +
> + return 0;
> +}
> +
> +static const struct msm_mmu_funcs pagetable_funcs = {
> + .map = msm_iommu_pagetable_map,
> + .unmap = msm_iommu_pagetable_unmap,
> + .destroy = msm_iommu_pagetable_destroy,
>
Re: [PATCH 13/19] drm/msm: Set the global virtual address range from the IOMMU domain
On Thu 13 Aug 21:41 CDT 2020, Rob Clark wrote:
> From: Jordan Crouse
>
> Use the aperture settings from the IOMMU domain to set up the virtual
> address range for the GPU. This allows us to transparently deal with
> IOMMU side features (like split pagetables).
>
Reviewed-by: Bjorn Andersson
> Signed-off-by: Jordan Crouse
> Signed-off-by: Rob Clark
> ---
> drivers/gpu/drm/msm/adreno/adreno_gpu.c | 13 +++--
> drivers/gpu/drm/msm/msm_iommu.c | 7 +++
> 2 files changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> index 533a34b4cce2..34e6242c1767 100644
> --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> @@ -192,9 +192,18 @@ adreno_iommu_create_address_space(struct msm_gpu *gpu,
> struct iommu_domain *iommu = iommu_domain_alloc(&platform_bus_type);
> struct msm_mmu *mmu = msm_iommu_new(&pdev->dev, iommu);
> struct msm_gem_address_space *aspace;
> + u64 start, size;
>
> - aspace = msm_gem_address_space_create(mmu, "gpu", SZ_16M,
> - 0x - SZ_16M);
> + /*
> + * Use the aperture start or SZ_16M, whichever is greater. This will
> + * ensure that we align with the allocated pagetable range while still
> + * allowing room in the lower 32 bits for GMEM and whatnot
> + */
> + start = max_t(u64, SZ_16M, iommu->geometry.aperture_start);
> + size = iommu->geometry.aperture_end - start + 1;
> +
> + aspace = msm_gem_address_space_create(mmu, "gpu",
> + start & GENMASK(48, 0), size);
>
> if (IS_ERR(aspace) && !IS_ERR(mmu))
> mmu->funcs->destroy(mmu);
> diff --git a/drivers/gpu/drm/msm/msm_iommu.c b/drivers/gpu/drm/msm/msm_iommu.c
> index 3a381a9674c9..1b6635504069 100644
> --- a/drivers/gpu/drm/msm/msm_iommu.c
> +++ b/drivers/gpu/drm/msm/msm_iommu.c
> @@ -36,6 +36,10 @@ static int msm_iommu_map(struct msm_mmu *mmu, uint64_t
> iova,
> struct msm_iommu *iommu = to_msm_iommu(mmu);
> size_t ret;
>
> + /* The arm-smmu driver expects the addresses to be sign extended */
> + if (iova & BIT_ULL(48))
> + iova |= GENMASK_ULL(63, 49);
> +
> ret = iommu_map_sg(iommu->domain, iova, sgt->sgl, sgt->nents, prot);
> WARN_ON(!ret);
>
> @@ -46,6 +50,9 @@ static int msm_iommu_unmap(struct msm_mmu *mmu, uint64_t
> iova, size_t len)
> {
> struct msm_iommu *iommu = to_msm_iommu(mmu);
>
> + if (iova & BIT_ULL(48))
> + iova |= GENMASK_ULL(63, 49);
> +
> iommu_unmap(iommu->domain, iova, len);
>
> return 0;
> --
> 2.26.2
>
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 12/19] drm/msm: Drop context arg to gpu->submit()
On Thu 13 Aug 21:41 CDT 2020, Rob Clark wrote:
> From: Jordan Crouse
>
> Now that we can get the ctx from the submitqueue, the extra arg is
> redundant.
>
Reviewed-by: Bjorn Andersson
> Signed-off-by: Jordan Crouse
> [split out of previous patch to reduce churny noise]
> Signed-off-by: Rob Clark
> ---
> drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 12 +---
> drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 5 ++---
> drivers/gpu/drm/msm/adreno/adreno_gpu.c | 5 ++---
> drivers/gpu/drm/msm/adreno/adreno_gpu.h | 3 +--
> drivers/gpu/drm/msm/msm_gem_submit.c| 2 +-
> drivers/gpu/drm/msm/msm_gpu.c | 9 -
> drivers/gpu/drm/msm/msm_gpu.h | 6 ++
> 7 files changed, 17 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
> b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
> index 9e63a190642c..eff2439ea57b 100644
> --- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
> @@ -43,8 +43,7 @@ static void a5xx_flush(struct msm_gpu *gpu, struct
> msm_ringbuffer *ring)
> gpu_write(gpu, REG_A5XX_CP_RB_WPTR, wptr);
> }
>
> -static void a5xx_submit_in_rb(struct msm_gpu *gpu, struct msm_gem_submit
> *submit,
> - struct msm_file_private *ctx)
> +static void a5xx_submit_in_rb(struct msm_gpu *gpu, struct msm_gem_submit
> *submit)
> {
> struct msm_drm_private *priv = gpu->dev->dev_private;
> struct msm_ringbuffer *ring = submit->ring;
> @@ -57,7 +56,7 @@ static void a5xx_submit_in_rb(struct msm_gpu *gpu, struct
> msm_gem_submit *submit
> case MSM_SUBMIT_CMD_IB_TARGET_BUF:
> break;
> case MSM_SUBMIT_CMD_CTX_RESTORE_BUF:
> - if (priv->lastctx == ctx)
> + if (priv->lastctx == submit->queue->ctx)
> break;
> /* fall-thru */
> case MSM_SUBMIT_CMD_BUF:
> @@ -103,8 +102,7 @@ static void a5xx_submit_in_rb(struct msm_gpu *gpu, struct
> msm_gem_submit *submit
> msm_gpu_retire(gpu);
> }
>
> -static void a5xx_submit(struct msm_gpu *gpu, struct msm_gem_submit *submit,
> - struct msm_file_private *ctx)
> +static void a5xx_submit(struct msm_gpu *gpu, struct msm_gem_submit *submit)
> {
> struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
> struct a5xx_gpu *a5xx_gpu = to_a5xx_gpu(adreno_gpu);
> @@ -114,7 +112,7 @@ static void a5xx_submit(struct msm_gpu *gpu, struct
> msm_gem_submit *submit,
>
> if (IS_ENABLED(CONFIG_DRM_MSM_GPU_SUDO) && submit->in_rb) {
> priv->lastctx = NULL;
> - a5xx_submit_in_rb(gpu, submit, ctx);
> + a5xx_submit_in_rb(gpu, submit);
> return;
> }
>
> @@ -148,7 +146,7 @@ static void a5xx_submit(struct msm_gpu *gpu, struct
> msm_gem_submit *submit,
> case MSM_SUBMIT_CMD_IB_TARGET_BUF:
> break;
> case MSM_SUBMIT_CMD_CTX_RESTORE_BUF:
> - if (priv->lastctx == ctx)
> + if (priv->lastctx == submit->queue->ctx)
> break;
> /* fall-thru */
> case MSM_SUBMIT_CMD_BUF:
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> index c5a3e4d4c007..5eabb0109577 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> @@ -81,8 +81,7 @@ static void get_stats_counter(struct msm_ringbuffer *ring,
> u32 counter,
> OUT_RING(ring, upper_32_bits(iova));
> }
>
> -static void a6xx_submit(struct msm_gpu *gpu, struct msm_gem_submit *submit,
> - struct msm_file_private *ctx)
> +static void a6xx_submit(struct msm_gpu *gpu, struct msm_gem_submit *submit)
> {
> unsigned int index = submit->seqno % MSM_GPU_SUBMIT_STATS_COUNT;
> struct msm_drm_private *priv = gpu->dev->dev_private;
> @@ -115,7 +114,7 @@ static void a6xx_submit(struct msm_gpu *gpu, struct
> msm_gem_submit *submit,
> case MSM_SUBMIT_CMD_IB_TARGET_BUF:
> break;
> case MSM_SUBMIT_CMD_CTX_RESTORE_BUF:
> - if (priv->lastctx == ctx)
> + if (priv->lastctx == submit->queue->ctx)
> break;
> /* fall-thru */
> case MSM_SUBMIT_CMD_BUF:
> diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> index d2dbb6968cba..533a34b4cce2 100644
> --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> @@ -457,8 +457,7 @@ void adreno_recover(struct msm_gpu *gpu)
> }
> }
>
> -void adreno_submit(struct msm_gpu *gpu, struct msm_gem_submit *submit,
> - struct msm_file_private *ctx)
> +void adreno_submit(struct msm_gpu *gpu, struct msm_gem_submit *submit)
> {
> struct adreno_gpu *adreno_gpu = to_adreno_gp
Re: [PATCH v1] iommu/vt-d: Move intel_iommu_gfx_mapped to Intel IOMMU header
Hi Andy,
On 8/29/20 12:12 AM, Andy Shevchenko wrote:
Static analyzer is not happy about intel_iommu_gfx_mapped declaration:
.../drivers/iommu/intel/iommu.c:364:5: warning: symbol 'intel_iommu_gfx_mapped'
was not declared. Should it be static?
Move its declaration to Intel IOMMU header file.
Signed-off-by: Andy Shevchenko
---
include/drm/intel-gtt.h | 5 +
include/linux/intel-iommu.h | 1 +
2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/include/drm/intel-gtt.h b/include/drm/intel-gtt.h
index 71d81923e6b0..abfefaaf897a 100644
--- a/include/drm/intel-gtt.h
+++ b/include/drm/intel-gtt.h
@@ -5,6 +5,7 @@
#define _DRM_INTEL_GTT_H
#include
+#include
#include
void intel_gtt_get(u64 *gtt_total,
@@ -33,8 +34,4 @@ void intel_gtt_clear_range(unsigned int first_entry, unsigned
int num_entries);
/* flag for GFDT type */
#define AGP_USER_CACHED_MEMORY_GFDT (1 << 3)
-#ifdef CONFIG_INTEL_IOMMU
-extern int intel_iommu_gfx_mapped;
-#endif
-
#endif
diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h
index 64fa5c56c825..fbd3bb64649b 100644
--- a/include/linux/intel-iommu.h
+++ b/include/linux/intel-iommu.h
@@ -794,6 +794,7 @@ extern int iommu_calculate_max_sagaw(struct intel_iommu
*iommu);
extern int dmar_disabled;
extern int intel_iommu_enabled;
extern int intel_iommu_tboot_noforce;
+extern int intel_iommu_gfx_mapped;
#else
static inline int iommu_calculate_agaw(struct intel_iommu *iommu)
{
Looks good to me.
Reviewed-by: Lu Baolu
Best regards,
baolu
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 11/19] drm/msm: Add a context pointer to the submitqueue
On Thu 13 Aug 21:41 CDT 2020, Rob Clark wrote:
> From: Jordan Crouse
>
> Each submitqueue is attached to a context. Add a pointer to the
> context to the submitqueue at create time and refcount it so
> that it stays around through the life of the queue.
>
Reviewed-by: Bjorn Andersson
> Co-developed-by: Rob Clark
> Signed-off-by: Jordan Crouse
> Signed-off-by: Rob Clark
> ---
> drivers/gpu/drm/msm/msm_drv.c | 3 ++-
> drivers/gpu/drm/msm/msm_drv.h | 20
> drivers/gpu/drm/msm/msm_gem.h | 1 +
> drivers/gpu/drm/msm/msm_gem_submit.c | 6 +++---
> drivers/gpu/drm/msm/msm_gpu.h | 1 +
> drivers/gpu/drm/msm/msm_submitqueue.c | 3 +++
> 6 files changed, 30 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
> index 7d641c7e3514..01845a3b8d52 100644
> --- a/drivers/gpu/drm/msm/msm_drv.c
> +++ b/drivers/gpu/drm/msm/msm_drv.c
> @@ -594,6 +594,7 @@ static int context_init(struct drm_device *dev, struct
> drm_file *file)
> if (!ctx)
> return -ENOMEM;
>
> + kref_init(&ctx->ref);
> msm_submitqueue_init(dev, ctx);
>
> ctx->aspace = priv->gpu ? priv->gpu->aspace : NULL;
> @@ -615,7 +616,7 @@ static int msm_open(struct drm_device *dev, struct
> drm_file *file)
> static void context_close(struct msm_file_private *ctx)
> {
> msm_submitqueue_close(ctx);
> - kfree(ctx);
> + msm_file_private_put(ctx);
> }
>
> static void msm_postclose(struct drm_device *dev, struct drm_file *file)
> diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
> index af259b0573ea..4561bfb5e745 100644
> --- a/drivers/gpu/drm/msm/msm_drv.h
> +++ b/drivers/gpu/drm/msm/msm_drv.h
> @@ -57,6 +57,7 @@ struct msm_file_private {
> struct list_head submitqueues;
> int queueid;
> struct msm_gem_address_space *aspace;
> + struct kref ref;
> };
>
> enum msm_mdp_plane_property {
> @@ -428,6 +429,25 @@ void msm_submitqueue_close(struct msm_file_private *ctx);
>
> void msm_submitqueue_destroy(struct kref *kref);
>
> +static inline void __msm_file_private_destroy(struct kref *kref)
> +{
> + struct msm_file_private *ctx = container_of(kref,
> + struct msm_file_private, ref);
> +
> + kfree(ctx);
> +}
> +
> +static inline void msm_file_private_put(struct msm_file_private *ctx)
> +{
> + kref_put(&ctx->ref, __msm_file_private_destroy);
> +}
> +
> +static inline struct msm_file_private *msm_file_private_get(
> + struct msm_file_private *ctx)
> +{
> + kref_get(&ctx->ref);
> + return ctx;
> +}
>
> #define DBG(fmt, ...) DRM_DEBUG_DRIVER(fmt"\n", ##__VA_ARGS__)
> #define VERB(fmt, ...) if (0) DRM_DEBUG_DRIVER(fmt"\n", ##__VA_ARGS__)
> diff --git a/drivers/gpu/drm/msm/msm_gem.h b/drivers/gpu/drm/msm/msm_gem.h
> index 972490b14ba5..9c573c4269cb 100644
> --- a/drivers/gpu/drm/msm/msm_gem.h
> +++ b/drivers/gpu/drm/msm/msm_gem.h
> @@ -142,6 +142,7 @@ struct msm_gem_submit {
> bool valid; /* true if no cmdstream patching needed */
> bool in_rb; /* "sudo" mode, copy cmds into RB */
> struct msm_ringbuffer *ring;
> + struct msm_file_private *ctx;
> unsigned int nr_cmds;
> unsigned int nr_bos;
> u32 ident; /* A "identifier" for the submit for logging */
> diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c
> b/drivers/gpu/drm/msm/msm_gem_submit.c
> index 8cb9aa15ff90..1464b04d25d3 100644
> --- a/drivers/gpu/drm/msm/msm_gem_submit.c
> +++ b/drivers/gpu/drm/msm/msm_gem_submit.c
> @@ -27,7 +27,7 @@
> #define BO_PINNED 0x2000
>
> static struct msm_gem_submit *submit_create(struct drm_device *dev,
> - struct msm_gpu *gpu, struct msm_gem_address_space *aspace,
> + struct msm_gpu *gpu,
> struct msm_gpu_submitqueue *queue, uint32_t nr_bos,
> uint32_t nr_cmds)
> {
> @@ -43,7 +43,7 @@ static struct msm_gem_submit *submit_create(struct
> drm_device *dev,
> return NULL;
>
> submit->dev = dev;
> - submit->aspace = aspace;
> + submit->aspace = queue->ctx->aspace;
> submit->gpu = gpu;
> submit->fence = NULL;
> submit->cmd = (void *)&submit->bos[nr_bos];
> @@ -677,7 +677,7 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void
> *data,
> }
> }
>
> - submit = submit_create(dev, gpu, ctx->aspace, queue, args->nr_bos,
> + submit = submit_create(dev, gpu, queue, args->nr_bos,
> args->nr_cmds);
> if (!submit) {
> ret = -ENOMEM;
> diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h
> index f91b141add75..97c527e98391 100644
> --- a/drivers/gpu/drm/msm/msm_gpu.h
> +++ b/drivers/gpu/drm/msm/msm_gpu.h
> @@ -190,6 +190,7 @@ struct msm_gpu_submitqueue {
> u32 flags;
> u32 prio;
> int faults;
> + struct msm_file_private *ctx;
> struct list_head node;
>
Re: [PATCH 09/19] iommu/arm-smmu-qcom: Add implementation for the adreno GPU SMMU
On Thu 13 Aug 21:41 CDT 2020, Rob Clark wrote:
> From: Jordan Crouse
>
> Add a special implementation for the SMMU attached to most Adreno GPU
> target triggered from the qcom,adreno-smmu compatible string.
>
> The new Adreno SMMU implementation will enable split pagetables
> (TTBR1) for the domain attached to the GPU device (SID 0) and
> hard code it context bank 0 so the GPU hardware can implement
> per-instance pagetables.
>
Reviewed-by: Bjorn Andersson
> Co-developed-by: Rob Clark
> Signed-off-by: Jordan Crouse
> Signed-off-by: Rob Clark
> ---
> drivers/iommu/arm/arm-smmu/arm-smmu-impl.c | 3 +
> drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 149 -
> drivers/iommu/arm/arm-smmu/arm-smmu.h | 1 +
> 3 files changed, 151 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c
> b/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c
> index 88f17cc33023..d199b4bff15d 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c
> @@ -223,6 +223,9 @@ struct arm_smmu_device *arm_smmu_impl_init(struct
> arm_smmu_device *smmu)
> of_device_is_compatible(np, "qcom,sm8250-smmu-500"))
> return qcom_smmu_impl_init(smmu);
>
> + if (of_device_is_compatible(smmu->dev->of_node, "qcom,adreno-smmu"))
> + return qcom_adreno_smmu_impl_init(smmu);
> +
> if (of_device_is_compatible(np, "marvell,ap806-smmu-500"))
> smmu->impl = &mrvl_mmu500_impl;
>
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> index be4318044f96..5640d9960610 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> @@ -3,6 +3,7 @@
> * Copyright (c) 2019, The Linux Foundation. All rights reserved.
> */
>
> +#include
> #include
> #include
>
> @@ -12,6 +13,132 @@ struct qcom_smmu {
> struct arm_smmu_device smmu;
> };
>
> +#define QCOM_ADRENO_SMMU_GPU_SID 0
> +
> +static bool qcom_adreno_smmu_is_gpu_device(struct device *dev)
> +{
> + struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
> + int i;
> +
> + /*
> + * The GPU will always use SID 0 so that is a handy way to uniquely
> + * identify it and configure it for per-instance pagetables
> + */
> + for (i = 0; i < fwspec->num_ids; i++) {
> + u16 sid = FIELD_GET(ARM_SMMU_SMR_ID, fwspec->ids[i]);
> +
> + if (sid == QCOM_ADRENO_SMMU_GPU_SID)
> + return true;
> + }
> +
> + return false;
> +}
> +
> +static const struct io_pgtable_cfg *qcom_adreno_smmu_get_ttbr1_cfg(
> + const void *cookie)
> +{
> + struct arm_smmu_domain *smmu_domain = (void *)cookie;
> + struct io_pgtable *pgtable =
> + io_pgtable_ops_to_pgtable(smmu_domain->pgtbl_ops);
> + return &pgtable->cfg;
> +}
> +
> +/*
> + * Local implementation to configure TTBR0 with the specified pagetable
> config.
> + * The GPU driver will call this to enable TTBR0 when per-instance pagetables
> + * are active
> + */
> +
> +static int qcom_adreno_smmu_set_ttbr0_cfg(const void *cookie,
> + const struct io_pgtable_cfg *pgtbl_cfg)
> +{
> + struct arm_smmu_domain *smmu_domain = (void *)cookie;
> + struct io_pgtable *pgtable =
> io_pgtable_ops_to_pgtable(smmu_domain->pgtbl_ops);
> + struct arm_smmu_cfg *cfg = &smmu_domain->cfg;
> + struct arm_smmu_cb *cb = &smmu_domain->smmu->cbs[cfg->cbndx];
> +
> + /* The domain must have split pagetables already enabled */
> + if (cb->tcr[0] & ARM_SMMU_TCR_EPD1)
> + return -EINVAL;
> +
> + /* If the pagetable config is NULL, disable TTBR0 */
> + if (!pgtbl_cfg) {
> + /* Do nothing if it is already disabled */
> + if ((cb->tcr[0] & ARM_SMMU_TCR_EPD0))
> + return -EINVAL;
> +
> + /* Set TCR to the original configuration */
> + cb->tcr[0] = arm_smmu_lpae_tcr(&pgtable->cfg);
> + cb->ttbr[0] = FIELD_PREP(ARM_SMMU_TTBRn_ASID, cb->cfg->asid);
> + } else {
> + u32 tcr = cb->tcr[0];
> +
> + /* Don't call this again if TTBR0 is already enabled */
> + if (!(cb->tcr[0] & ARM_SMMU_TCR_EPD0))
> + return -EINVAL;
> +
> + tcr |= arm_smmu_lpae_tcr(pgtbl_cfg);
> + tcr &= ~(ARM_SMMU_TCR_EPD0 | ARM_SMMU_TCR_EPD1);
> +
> + cb->tcr[0] = tcr;
> + cb->ttbr[0] = pgtbl_cfg->arm_lpae_s1_cfg.ttbr;
> + cb->ttbr[0] |= FIELD_PREP(ARM_SMMU_TTBRn_ASID, cb->cfg->asid);
> + }
> +
> + arm_smmu_write_context_bank(smmu_domain->smmu, cb->cfg->cbndx);
> +
> + return 0;
> +}
> +
> +static int qcom_adreno_smmu_alloc_context_bank(struct arm_smmu_domain
> *smmu_domain,
> + struct device *dev, int start, int count)
> +{
> + struct arm_smmu_device *smmu = smmu_domain->smm
Re: [PATCH 10/19] dt-bindings: arm-smmu: Add compatible string for Adreno GPU SMMU
On Thu 13 Aug 21:41 CDT 2020, Rob Clark wrote: > From: Jordan Crouse > > Every Qcom Adreno GPU has an embedded SMMU for its own use. These > devices depend on unique features such as split pagetables, > different stall/halt requirements and other settings. Identify them > with a compatible string so that they can be identified in the > arm-smmu implementation specific code. > Reviewed-by: Bjorn Andersson > Signed-off-by: Jordan Crouse > Reviewed-by: Rob Herring > Signed-off-by: Rob Clark > --- > Documentation/devicetree/bindings/iommu/arm,smmu.yaml | 4 > 1 file changed, 4 insertions(+) > > diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml > b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml > index 503160a7b9a0..5ec5d0d691f6 100644 > --- a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml > +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml > @@ -40,6 +40,10 @@ properties: >- qcom,sm8150-smmu-500 >- qcom,sm8250-smmu-500 >- const: arm,mmu-500 > + - description: Qcom Adreno GPUs implementing "arm,smmu-v2" > +items: > + - const: qcom,adreno-smmu > + - const: qcom,smmu-v2 >- description: Marvell SoCs implementing "arm,mmu-500" > items: >- const: marvell,ap806-smmu-500 > -- > 2.26.2 > ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 07/19] drm/msm: set adreno_smmu as gpu's drvdata
On Thu 13 Aug 21:41 CDT 2020, Rob Clark wrote:
> From: Rob Clark
>
> This will be populated by adreno-smmu, to provide a way for coordinating
> enabling/disabling TTBR0 translation.
>
Reviewed-by: Bjorn Andersson
> Signed-off-by: Rob Clark
> ---
> drivers/gpu/drm/msm/adreno/adreno_device.c | 2 --
> drivers/gpu/drm/msm/msm_gpu.c | 2 +-
> drivers/gpu/drm/msm/msm_gpu.h | 6 +-
> 3 files changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c
> b/drivers/gpu/drm/msm/adreno/adreno_device.c
> index 26664e1b30c0..58e03b20e1c7 100644
> --- a/drivers/gpu/drm/msm/adreno/adreno_device.c
> +++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
> @@ -417,8 +417,6 @@ static int adreno_bind(struct device *dev, struct device
> *master, void *data)
> return PTR_ERR(gpu);
> }
>
> - dev_set_drvdata(dev, gpu);
> -
> return 0;
> }
>
> diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c
> index 6aa9e04e52e7..806eb0957280 100644
> --- a/drivers/gpu/drm/msm/msm_gpu.c
> +++ b/drivers/gpu/drm/msm/msm_gpu.c
> @@ -892,7 +892,7 @@ int msm_gpu_init(struct drm_device *drm, struct
> platform_device *pdev,
> gpu->gpu_cx = NULL;
>
> gpu->pdev = pdev;
> - platform_set_drvdata(pdev, gpu);
> + platform_set_drvdata(pdev, &gpu->adreno_smmu);
>
> msm_devfreq_init(gpu);
>
> diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h
> index 8bda7beaed4b..f91b141add75 100644
> --- a/drivers/gpu/drm/msm/msm_gpu.h
> +++ b/drivers/gpu/drm/msm/msm_gpu.h
> @@ -7,6 +7,7 @@
> #ifndef __MSM_GPU_H__
> #define __MSM_GPU_H__
>
> +#include
> #include
> #include
> #include
> @@ -73,6 +74,8 @@ struct msm_gpu {
> struct platform_device *pdev;
> const struct msm_gpu_funcs *funcs;
>
> + struct adreno_smmu_priv adreno_smmu;
> +
> /* performance counters (hw & sw): */
> spinlock_t perf_lock;
> bool perfcntr_active;
> @@ -143,7 +146,8 @@ struct msm_gpu {
>
> static inline struct msm_gpu *dev_to_gpu(struct device *dev)
> {
> - return dev_get_drvdata(dev);
> + struct adreno_smmu_priv *adreno_smmu = dev_get_drvdata(dev);
> + return container_of(adreno_smmu, struct msm_gpu, adreno_smmu);
> }
>
> /* It turns out that all targets use the same ringbuffer size */
> --
> 2.26.2
>
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 08/19] iommu/arm-smmu: constify some helpers
On Thu 13 Aug 21:41 CDT 2020, Rob Clark wrote:
> From: Rob Clark
>
> Sprinkle a few `const`s where helpers don't need write access.
>
Reviewed-by: Bjorn Andersson
> Signed-off-by: Rob Clark
> ---
> drivers/iommu/arm/arm-smmu/arm-smmu.h | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.h
> b/drivers/iommu/arm/arm-smmu/arm-smmu.h
> index 59ff3fc5c6c8..27c8fc50 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu.h
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.h
> @@ -377,7 +377,7 @@ struct arm_smmu_master_cfg {
> s16 smendx[];
> };
>
> -static inline u32 arm_smmu_lpae_tcr(struct io_pgtable_cfg *cfg)
> +static inline u32 arm_smmu_lpae_tcr(const struct io_pgtable_cfg *cfg)
> {
> u32 tcr = FIELD_PREP(ARM_SMMU_TCR_TG0, cfg->arm_lpae_s1_cfg.tcr.tg) |
> FIELD_PREP(ARM_SMMU_TCR_SH0, cfg->arm_lpae_s1_cfg.tcr.sh) |
> @@ -398,13 +398,13 @@ static inline u32 arm_smmu_lpae_tcr(struct
> io_pgtable_cfg *cfg)
> return tcr;
> }
>
> -static inline u32 arm_smmu_lpae_tcr2(struct io_pgtable_cfg *cfg)
> +static inline u32 arm_smmu_lpae_tcr2(const struct io_pgtable_cfg *cfg)
> {
> return FIELD_PREP(ARM_SMMU_TCR2_PASIZE, cfg->arm_lpae_s1_cfg.tcr.ips) |
> FIELD_PREP(ARM_SMMU_TCR2_SEP, ARM_SMMU_TCR2_SEP_UPSTREAM);
> }
>
> -static inline u32 arm_smmu_lpae_vtcr(struct io_pgtable_cfg *cfg)
> +static inline u32 arm_smmu_lpae_vtcr(const struct io_pgtable_cfg *cfg)
> {
> return ARM_SMMU_VTCR_RES1 |
> FIELD_PREP(ARM_SMMU_VTCR_PS, cfg->arm_lpae_s2_cfg.vtcr.ps) |
> --
> 2.26.2
>
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 06/19] drm/msm/gpu: add dev_to_gpu() helper
On Thu 13 Aug 21:41 CDT 2020, Rob Clark wrote:
> From: Rob Clark
>
> In a later patch, the drvdata will not directly be 'struct msm_gpu *',
> so add a helper to reduce the churn.
>
> Signed-off-by: Rob Clark
> ---
> drivers/gpu/drm/msm/adreno/adreno_device.c | 10 --
> drivers/gpu/drm/msm/msm_gpu.c | 6 +++---
> drivers/gpu/drm/msm/msm_gpu.h | 5 +
> 3 files changed, 12 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c
> b/drivers/gpu/drm/msm/adreno/adreno_device.c
> index 9eeb46bf2a5d..26664e1b30c0 100644
> --- a/drivers/gpu/drm/msm/adreno/adreno_device.c
> +++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
> @@ -282,7 +282,7 @@ struct msm_gpu *adreno_load_gpu(struct drm_device *dev)
> int ret;
>
> if (pdev)
> - gpu = platform_get_drvdata(pdev);
> + gpu = dev_to_gpu(&pdev->dev);
>
> if (!gpu) {
> dev_err_once(dev->dev, "no GPU device was found\n");
> @@ -425,7 +425,7 @@ static int adreno_bind(struct device *dev, struct device
> *master, void *data)
> static void adreno_unbind(struct device *dev, struct device *master,
> void *data)
> {
> - struct msm_gpu *gpu = dev_get_drvdata(dev);
> + struct msm_gpu *gpu = dev_to_gpu(dev);
>
> pm_runtime_force_suspend(dev);
> gpu->funcs->destroy(gpu);
> @@ -490,16 +490,14 @@ static const struct of_device_id dt_match[] = {
> #ifdef CONFIG_PM
> static int adreno_resume(struct device *dev)
> {
> - struct platform_device *pdev = to_platform_device(dev);
> - struct msm_gpu *gpu = platform_get_drvdata(pdev);
> + struct msm_gpu *gpu = dev_to_gpu(dev);
>
> return gpu->funcs->pm_resume(gpu);
> }
>
> static int adreno_suspend(struct device *dev)
> {
> - struct platform_device *pdev = to_platform_device(dev);
> - struct msm_gpu *gpu = platform_get_drvdata(pdev);
> + struct msm_gpu *gpu = dev_to_gpu(dev);
>
> return gpu->funcs->pm_suspend(gpu);
> }
> diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c
> index d5645472b25d..6aa9e04e52e7 100644
> --- a/drivers/gpu/drm/msm/msm_gpu.c
> +++ b/drivers/gpu/drm/msm/msm_gpu.c
> @@ -24,7 +24,7 @@
> static int msm_devfreq_target(struct device *dev, unsigned long *freq,
> u32 flags)
> {
> - struct msm_gpu *gpu = platform_get_drvdata(to_platform_device(dev));
> + struct msm_gpu *gpu = dev_to_gpu(dev);
> struct dev_pm_opp *opp;
>
> opp = devfreq_recommended_opp(dev, freq, flags);
> @@ -45,7 +45,7 @@ static int msm_devfreq_target(struct device *dev, unsigned
> long *freq,
> static int msm_devfreq_get_dev_status(struct device *dev,
> struct devfreq_dev_status *status)
> {
> - struct msm_gpu *gpu = platform_get_drvdata(to_platform_device(dev));
> + struct msm_gpu *gpu = dev_to_gpu(dev);
> ktime_t time;
>
> if (gpu->funcs->gpu_get_freq)
> @@ -64,7 +64,7 @@ static int msm_devfreq_get_dev_status(struct device *dev,
>
> static int msm_devfreq_get_cur_freq(struct device *dev, unsigned long *freq)
> {
> - struct msm_gpu *gpu = platform_get_drvdata(to_platform_device(dev));
> + struct msm_gpu *gpu = dev_to_gpu(dev);
>
> if (gpu->funcs->gpu_get_freq)
> *freq = gpu->funcs->gpu_get_freq(gpu);
> diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h
> index 0db117a7339b..8bda7beaed4b 100644
> --- a/drivers/gpu/drm/msm/msm_gpu.h
> +++ b/drivers/gpu/drm/msm/msm_gpu.h
> @@ -141,6 +141,11 @@ struct msm_gpu {
> struct msm_gpu_state *crashstate;
> };
>
> +static inline struct msm_gpu *dev_to_gpu(struct device *dev)
That's a fairly generic name for a driver-global helper :)
Reviewed-by: Bjorn Andersson
Regards,
Bjorn
> +{
> + return dev_get_drvdata(dev);
> +}
> +
> /* It turns out that all targets use the same ringbuffer size */
> #define MSM_GPU_RINGBUFFER_SZ SZ_32K
> #define MSM_GPU_RINGBUFFER_BLKSIZE 32
> --
> 2.26.2
>
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 05/19] iommu: add private interface for adreno-smmu
On Thu 13 Aug 21:41 CDT 2020, Rob Clark wrote:
> From: Rob Clark
>
> This interface will be used for drm/msm to coordinate with the
> qcom_adreno_smmu_impl to enable/disable TTBR0 translation.
>
> Once TTBR0 translation is enabled, the GPU's CP (Command Processor)
> will directly switch TTBR0 pgtables (and do the necessary TLB inv)
> synchronized to the GPU's operation. But help from the SMMU driver
> is needed to initially bootstrap TTBR0 translation, which cannot be
> done from the GPU.
>
> Since this is a very special case, a private interface is used to
> avoid adding highly driver specific things to the public iommu
> interface.
>
> Signed-off-by: Rob Clark
Reviewed-by: Bjorn Andersson
> ---
> include/linux/adreno-smmu-priv.h | 36
> 1 file changed, 36 insertions(+)
> create mode 100644 include/linux/adreno-smmu-priv.h
>
> diff --git a/include/linux/adreno-smmu-priv.h
> b/include/linux/adreno-smmu-priv.h
> new file mode 100644
> index ..a889f28afb42
> --- /dev/null
> +++ b/include/linux/adreno-smmu-priv.h
> @@ -0,0 +1,36 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (C) 2020 Google, Inc
> + */
> +
> +#ifndef __ADRENO_SMMU_PRIV_H
> +#define __ADRENO_SMMU_PRIV_H
> +
> +#include
> +
> +/**
> + * struct adreno_smmu_priv - private interface between adreno-smmu and GPU
> + *
> + * @cookie:An opque token provided by adreno-smmu and passed
> + * back into the callbacks
> + * @get_ttbr1_cfg: Get the TTBR1 config for the GPUs context-bank
> + * @set_ttbr0_cfg: Set the TTBR0 config for the GPUs context bank. A
> + * NULL config disables TTBR0 translation, otherwise
> + * TTBR0 translation is enabled with the specified cfg
> + *
> + * The GPU driver (drm/msm) and adreno-smmu work together for controlling
> + * the GPU's SMMU instance. This is by necessity, as the GPU is directly
> + * updating the SMMU for context switches, while on the other hand we do
> + * not want to duplicate all of the initial setup logic from arm-smmu.
> + *
> + * This private interface is used for the two drivers to coordinate. The
> + * cookie and callback functions are populated when the GPU driver attaches
> + * it's domain.
> + */
> +struct adreno_smmu_priv {
> +const void *cookie;
> +const struct io_pgtable_cfg *(*get_ttbr1_cfg)(const void *cookie);
> +int (*set_ttbr0_cfg)(const void *cookie, const struct io_pgtable_cfg
> *cfg);
> +};
> +
> +#endif /* __ADRENO_SMMU_PRIV_H */
> \ No newline at end of file
> --
> 2.26.2
>
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 01/19] drm/msm: remove dangling submitqueue references
On Mon, Aug 31, 2020 at 7:35 PM Bjorn Andersson
wrote:
>
> On Fri 14 Aug 02:40 UTC 2020, Rob Clark wrote:
>
> > From: Rob Clark
> >
> > Currently it doesn't matter, since we free the ctx immediately. But
> > when we start refcnt'ing the ctx, we don't want old dangling list
> > entries to hang around.
> >
> > Signed-off-by: Rob Clark
> > ---
> > drivers/gpu/drm/msm/msm_submitqueue.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/msm/msm_submitqueue.c
> > b/drivers/gpu/drm/msm/msm_submitqueue.c
> > index a1d94be7883a..90c9d84e6155 100644
> > --- a/drivers/gpu/drm/msm/msm_submitqueue.c
> > +++ b/drivers/gpu/drm/msm/msm_submitqueue.c
> > @@ -49,8 +49,10 @@ void msm_submitqueue_close(struct msm_file_private *ctx)
> >* No lock needed in close and there won't
> >* be any more user ioctls coming our way
> >*/
> > - list_for_each_entry_safe(entry, tmp, &ctx->submitqueues, node)
> > + list_for_each_entry_safe(entry, tmp, &ctx->submitqueues, node) {
> > + list_del(&entry->node);
>
> If you refcount ctx, what does that do for the entries in the submit
> queue?
>
> "entry" here is kref'ed, but you're popping it off the list regardless
> of the put ends up freeing the object or not - which afaict would mean
> leaking the object.
>
What ends up happening is the submit has reference to submit-queue,
which has reference to the ctx.. the submitqueue could be alive still
pending in-flight submits (in a later patch), but dead from the PoV of
userspace interface.
We aren't relying (or at least aren't in the end, and I *think* I
didn't miss anything in the middle) relying on ctx->submitqueues list
to clean anything up in the end, just track what is still a valid
submitqueue from userspace PoV
BR,
-R
>
> On the other hand, with the current implementation an object with higher
> refcount with adjacent objects of single refcount would end up with
> dangling pointers after the put. So in itself this change seems like a
> net gain, but I'm wondering about the plan described in the commit
> message.
>
> Regards,
> Bjorn
>
> > msm_submitqueue_put(entry);
> > + }
> > }
> >
> > int msm_submitqueue_create(struct drm_device *drm, struct msm_file_private
> > *ctx,
> > --
> > 2.26.2
> >
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 03/19] iommu/arm-smmu: Add support for split pagetables
On Thu 13 Aug 21:40 CDT 2020, Rob Clark wrote:
> From: Jordan Crouse
>
> Enable TTBR1 for a context bank if IO_PGTABLE_QUIRK_ARM_TTBR1 is selected
> by the io-pgtable configuration.
>
> Signed-off-by: Jordan Crouse
> Signed-off-by: Rob Clark
> ---
> drivers/iommu/arm/arm-smmu/arm-smmu.c | 21 -
> drivers/iommu/arm/arm-smmu/arm-smmu.h | 25 +++--
> 2 files changed, 35 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c
> b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> index 37d8d49299b4..976d43a7f2ff 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> @@ -552,11 +552,15 @@ static void arm_smmu_init_context_bank(struct
> arm_smmu_domain *smmu_domain,
> cb->ttbr[0] = pgtbl_cfg->arm_v7s_cfg.ttbr;
> cb->ttbr[1] = 0;
> } else {
> - cb->ttbr[0] = pgtbl_cfg->arm_lpae_s1_cfg.ttbr;
> - cb->ttbr[0] |= FIELD_PREP(ARM_SMMU_TTBRn_ASID,
> - cfg->asid);
> + cb->ttbr[0] = FIELD_PREP(ARM_SMMU_TTBRn_ASID,
> + cfg->asid);
> cb->ttbr[1] = FIELD_PREP(ARM_SMMU_TTBRn_ASID,
> - cfg->asid);
> + cfg->asid);
The old indentation seems more appropriate.
Apart from that this looks sensible.
Reviewed-by: Bjorn Andersson
Regards,
Bjorn
> +
> + if (pgtbl_cfg->quirks & IO_PGTABLE_QUIRK_ARM_TTBR1)
> + cb->ttbr[1] |= pgtbl_cfg->arm_lpae_s1_cfg.ttbr;
> + else
> + cb->ttbr[0] |= pgtbl_cfg->arm_lpae_s1_cfg.ttbr;
> }
> } else {
> cb->ttbr[0] = pgtbl_cfg->arm_lpae_s2_cfg.vttbr;
> @@ -822,7 +826,14 @@ static int arm_smmu_init_domain_context(struct
> iommu_domain *domain,
>
> /* Update the domain's page sizes to reflect the page table format */
> domain->pgsize_bitmap = pgtbl_cfg.pgsize_bitmap;
> - domain->geometry.aperture_end = (1UL << ias) - 1;
> +
> + if (pgtbl_cfg.quirks & IO_PGTABLE_QUIRK_ARM_TTBR1) {
> + domain->geometry.aperture_start = ~0UL << ias;
> + domain->geometry.aperture_end = ~0UL;
> + } else {
> + domain->geometry.aperture_end = (1UL << ias) - 1;
> + }
> +
> domain->geometry.force_aperture = true;
>
> /* Initialise the context bank with our page table cfg */
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.h
> b/drivers/iommu/arm/arm-smmu/arm-smmu.h
> index 83294516ac08..f3e456893f28 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu.h
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.h
> @@ -169,10 +169,12 @@ enum arm_smmu_cbar_type {
> #define ARM_SMMU_CB_TCR 0x30
> #define ARM_SMMU_TCR_EAE BIT(31)
> #define ARM_SMMU_TCR_EPD1BIT(23)
> +#define ARM_SMMU_TCR_A1 BIT(22)
> #define ARM_SMMU_TCR_TG0 GENMASK(15, 14)
> #define ARM_SMMU_TCR_SH0 GENMASK(13, 12)
> #define ARM_SMMU_TCR_ORGN0 GENMASK(11, 10)
> #define ARM_SMMU_TCR_IRGN0 GENMASK(9, 8)
> +#define ARM_SMMU_TCR_EPD0BIT(7)
> #define ARM_SMMU_TCR_T0SZGENMASK(5, 0)
>
> #define ARM_SMMU_VTCR_RES1 BIT(31)
> @@ -350,12 +352,23 @@ struct arm_smmu_domain {
>
> static inline u32 arm_smmu_lpae_tcr(struct io_pgtable_cfg *cfg)
> {
> - return ARM_SMMU_TCR_EPD1 |
> -FIELD_PREP(ARM_SMMU_TCR_TG0, cfg->arm_lpae_s1_cfg.tcr.tg) |
> -FIELD_PREP(ARM_SMMU_TCR_SH0, cfg->arm_lpae_s1_cfg.tcr.sh) |
> -FIELD_PREP(ARM_SMMU_TCR_ORGN0, cfg->arm_lpae_s1_cfg.tcr.orgn) |
> -FIELD_PREP(ARM_SMMU_TCR_IRGN0, cfg->arm_lpae_s1_cfg.tcr.irgn) |
> -FIELD_PREP(ARM_SMMU_TCR_T0SZ, cfg->arm_lpae_s1_cfg.tcr.tsz);
> + u32 tcr = FIELD_PREP(ARM_SMMU_TCR_TG0, cfg->arm_lpae_s1_cfg.tcr.tg) |
> + FIELD_PREP(ARM_SMMU_TCR_SH0, cfg->arm_lpae_s1_cfg.tcr.sh) |
> + FIELD_PREP(ARM_SMMU_TCR_ORGN0, cfg->arm_lpae_s1_cfg.tcr.orgn) |
> + FIELD_PREP(ARM_SMMU_TCR_IRGN0, cfg->arm_lpae_s1_cfg.tcr.irgn) |
> + FIELD_PREP(ARM_SMMU_TCR_T0SZ, cfg->arm_lpae_s1_cfg.tcr.tsz);
> +
> + /*
> + * When TTBR1 is selected shift the TCR fields by 16 bits and disable
> + * translation in TTBR0
> + */
> + if (cfg->quirks & IO_PGTABLE_QUIRK_ARM_TTBR1) {
> + tcr = (tcr << 16) & ~ARM_SMMU_TCR_A1;
> + tcr |= ARM_SMMU_TCR_EPD0;
> + } else
> + tcr |= ARM_SMMU_TCR_EPD1;
> +
> + return tcr;
> }
>
> static inline u32 arm_smmu_lpae_tcr2(struct io_pgtable_cfg *cfg)
> --
> 2.26.2
>
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/
[PATCH v4 4/5] vfio/type1: Use iommu_aux_at(de)tach_group() APIs
Replace iommu_aux_at(de)tach_device() with iommu_at(de)tach_subdev_group().
Signed-off-by: Lu Baolu
---
drivers/vfio/vfio_iommu_type1.c | 43 +
1 file changed, 6 insertions(+), 37 deletions(-)
diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
index 5e556ac9102a..8d5eb7ce0986 100644
--- a/drivers/vfio/vfio_iommu_type1.c
+++ b/drivers/vfio/vfio_iommu_type1.c
@@ -1627,45 +1627,13 @@ static struct device *vfio_mdev_get_iommu_device(struct
device *dev)
return NULL;
}
-static int vfio_mdev_attach_domain(struct device *dev, void *data)
-{
- struct iommu_domain *domain = data;
- struct device *iommu_device;
-
- iommu_device = vfio_mdev_get_iommu_device(dev);
- if (iommu_device) {
- if (iommu_dev_feature_enabled(iommu_device, IOMMU_DEV_FEAT_AUX))
- return iommu_aux_attach_device(domain, iommu_device);
- else
- return iommu_attach_device(domain, iommu_device);
- }
-
- return -EINVAL;
-}
-
-static int vfio_mdev_detach_domain(struct device *dev, void *data)
-{
- struct iommu_domain *domain = data;
- struct device *iommu_device;
-
- iommu_device = vfio_mdev_get_iommu_device(dev);
- if (iommu_device) {
- if (iommu_dev_feature_enabled(iommu_device, IOMMU_DEV_FEAT_AUX))
- iommu_aux_detach_device(domain, iommu_device);
- else
- iommu_detach_device(domain, iommu_device);
- }
-
- return 0;
-}
-
static int vfio_iommu_attach_group(struct vfio_domain *domain,
struct vfio_group *group)
{
if (group->mdev_group)
- return iommu_group_for_each_dev(group->iommu_group,
- domain->domain,
- vfio_mdev_attach_domain);
+ return iommu_attach_subdev_group(domain->domain,
+group->iommu_group,
+vfio_mdev_get_iommu_device);
else
return iommu_attach_group(domain->domain, group->iommu_group);
}
@@ -1674,8 +1642,9 @@ static void vfio_iommu_detach_group(struct vfio_domain
*domain,
struct vfio_group *group)
{
if (group->mdev_group)
- iommu_group_for_each_dev(group->iommu_group, domain->domain,
-vfio_mdev_detach_domain);
+ iommu_detach_subdev_group(domain->domain,
+ group->iommu_group,
+ vfio_mdev_get_iommu_device);
else
iommu_detach_group(domain->domain, group->iommu_group);
}
--
2.17.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v4 1/5] iommu: Add optional subdev in aux_at(de)tach ops
In the vfio/mdev use case of aux-domain, the subdevices are created from
the physical devices with IOMMU_DEV_FEAT_AUX enabled and the aux-domains
are attached to the subdevices through the iommu_ops.aux_attach_dev()
interface.
Current iommu_ops.aux_at(de)tach_dev() design only takes the aux-domain
and the physical device as the parameters, this is insufficient if we
want the vendor iommu drivers to learn the knowledge about relationships
between the aux-domains and the subdevices. Add a @subdev parameter to
iommu_ops.aux_at(de)tach_dev() interfaces so that a subdevice could be
opt-in.
Signed-off-by: Lu Baolu
---
drivers/iommu/intel/iommu.c | 10 ++
drivers/iommu/iommu.c | 4 ++--
include/linux/iommu.h | 6 --
3 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index bce158468abf..3c12fd06856c 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -5338,8 +5338,9 @@ static int intel_iommu_attach_device(struct iommu_domain
*domain,
return domain_add_dev_info(to_dmar_domain(domain), dev);
}
-static int intel_iommu_aux_attach_device(struct iommu_domain *domain,
-struct device *dev)
+static int
+intel_iommu_aux_attach_device(struct iommu_domain *domain,
+ struct device *dev, struct device *subdev)
{
int ret;
@@ -5359,8 +5360,9 @@ static void intel_iommu_detach_device(struct iommu_domain
*domain,
dmar_remove_one_dev_info(dev);
}
-static void intel_iommu_aux_detach_device(struct iommu_domain *domain,
- struct device *dev)
+static void
+intel_iommu_aux_detach_device(struct iommu_domain *domain, struct device *dev,
+ struct device *subdev)
{
aux_domain_remove_dev(to_dmar_domain(domain), dev);
}
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 609bd25bf154..38cdfeb887e1 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -2728,7 +2728,7 @@ int iommu_aux_attach_device(struct iommu_domain *domain,
struct device *dev)
int ret = -ENODEV;
if (domain->ops->aux_attach_dev)
- ret = domain->ops->aux_attach_dev(domain, dev);
+ ret = domain->ops->aux_attach_dev(domain, dev, NULL);
if (!ret)
trace_attach_device_to_domain(dev);
@@ -2740,7 +2740,7 @@ EXPORT_SYMBOL_GPL(iommu_aux_attach_device);
void iommu_aux_detach_device(struct iommu_domain *domain, struct device *dev)
{
if (domain->ops->aux_detach_dev) {
- domain->ops->aux_detach_dev(domain, dev);
+ domain->ops->aux_detach_dev(domain, dev, NULL);
trace_detach_device_from_domain(dev);
}
}
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index fee209efb756..871267104915 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -279,8 +279,10 @@ struct iommu_ops {
int (*dev_disable_feat)(struct device *dev, enum iommu_dev_features f);
/* Aux-domain specific attach/detach entries */
- int (*aux_attach_dev)(struct iommu_domain *domain, struct device *dev);
- void (*aux_detach_dev)(struct iommu_domain *domain, struct device *dev);
+ int (*aux_attach_dev)(struct iommu_domain *domain, struct device *dev,
+ struct device *subdev);
+ void (*aux_detach_dev)(struct iommu_domain *domain, struct device *dev,
+ struct device *subdev);
int (*aux_get_pasid)(struct iommu_domain *domain, struct device *dev);
struct iommu_sva *(*sva_bind)(struct device *dev, struct mm_struct *mm,
--
2.17.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v4 0/5] iommu aux-domain APIs extensions
This series aims to extend the IOMMU aux-domain API set so that it
could be more friendly to vfio/mdev usage. The interactions between
vfio/mdev and iommu during mdev creation and passthr are:
1. Create a group for mdev with iommu_group_alloc();
2. Add the device to the group with
group = iommu_group_alloc();
if (IS_ERR(group))
return PTR_ERR(group);
ret = iommu_group_add_device(group, &mdev->dev);
if (!ret)
dev_info(&mdev->dev, "MDEV: group_id = %d\n",
iommu_group_id(group));
3. Allocate an aux-domain with iommu_domain_alloc();
4. Attach the aux-domain to the iommu_group.
iommu_group_for_each_dev {
if (iommu_dev_feature_enabled(iommu_device, IOMMU_DEV_FEAT_AUX))
return iommu_aux_attach_device(domain, iommu_device);
else
return iommu_attach_device(domain, iommu_device);
}
where, iommu_device is the aux-domain-capable device. The mdev's in
the group are all derived from it.
In the whole process, an iommu group was allocated for the mdev and an
iommu domain was attached to the group, but the group->domain leaves
NULL. As the result, iommu_get_domain_for_dev() (or other similar
interfaces) doesn't work anymore.
The iommu_get_domain_for_dev() is a necessary interface for device
drivers that want to support vfio/mdev based aux-domain. For example,
unsigned long pasid;
struct iommu_domain *domain;
struct device *dev = mdev_dev(mdev);
struct device *iommu_device = vfio_mdev_get_iommu_device(dev);
domain = iommu_aux_get_domain_for_dev(dev);
if (!domain)
return -ENODEV;
pasid = iommu_aux_get_pasid(domain, iommu_device);
if (pasid <= 0)
return -EINVAL;
/* Program the device context */
We tried to address this by extending iommu_aux_at(de)tach_device() so that
the users could pass in an optional device pointer (for example vfio/mdev).
(v2 of this series)
https://lore.kernel.org/linux-iommu/20200707013957.23672-1-baolu...@linux.intel.com/
But that will cause a lock issue as group->mutex has been applied in
iommu_group_for_each_dev(), but has to be reapplied again in the
iommu_aux_attach_device().
We also tried to implement an equivalent iommu_attch_group() for groups
which includes subdevices derived from a single physical device. (v3 of
this series)
https://lore.kernel.org/linux-iommu/20200714055703.5510-1-baolu...@linux.intel.com/
But that's too harsh (requires that all subdevices in an iommu_group
must be derived from a same physical device) and breaks some generic
concept of iommmu_group.
This version continues to address this by introducing some new APIs into
the aux-domain API set according to comments during v3 reviewing period.
/**
* iommu_attach_subdev_group - attach domain to an iommu_group which
* contains subdevices.
*
* @domain: domain
* @group: iommu_group which contains subdevices
* @fn: callback for each subdevice in the @iommu_group to retrieve the
* physical device where the subdevice was created from.
*
* Returns 0 on success, or an error value.
*/
int iommu_attach_subdev_group(struct iommu_domain *domain,
struct iommu_group *group,
iommu_device_lookup_t fn)
/**
* iommu_detach_subdev_group - detach domain from an iommu_group which
* contains subdevices
*
* @domain: domain
* @group: iommu_group which contains subdevices
* @fn: callback for each subdevice in the @iommu_group to retrieve the
* physical device where the subdevice was created from.
*
* The domain must have been attached to @group via iommu_attach_subdev_group().
*/
void iommu_detach_subdev_group(struct iommu_domain *domain,
struct iommu_group *group,
iommu_device_lookup_t fn)
struct iommu_domain *iommu_aux_get_domain_for_dev(struct device *subdev)
This version is evolved according to feedbacks from Robin, Alex and Kevin.
I'm very appreciated to their contributions.
Best regards,
baolu
---
Change log:
- v1->v2:
-
https://lore.kernel.org/linux-iommu/20200627031532.28046-1-baolu...@linux.intel.com/
- Suggested by Robin.
- v2->v3:
-
https://lore.kernel.org/linux-iommu/20200707013957.23672-1-baolu...@linux.intel.com/
- Suggested by Alex, Kevin.
- v3->v4:
-
https://lore.kernel.org/linux-iommu/20200714055703.5510-1-baolu...@linux.intel.com/
- Evolve the aux_attach_group APIs to take an iommu_device lookup
callback.
- Add interface to check whether a domain is aux-domain for a device.
- Return domain only if the domain is aux-domain in
iommu_aux_get_domain_for_dev().
Lu Baolu (5):
iommu: Add optional subdev in aux_at(de)tach ops
iommu: Add iommu_at(de)tach_subd
[PATCH v4 5/5] iommu/vt-d: Add is_aux_domain support
With subdevice information opt-in through iommu_ops.aux_at(de)tach_dev()
interfaces, the vendor iommu driver is able to learn the knowledge about
the relationships between the subdevices and the aux-domains. Implement
is_aux_domain() support based on the relationship knowledges.
Signed-off-by: Lu Baolu
---
drivers/iommu/intel/iommu.c | 125 ++--
include/linux/intel-iommu.h | 17 +++--
2 files changed, 103 insertions(+), 39 deletions(-)
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 3c12fd06856c..50431c7b2e71 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -334,6 +334,8 @@ static int intel_iommu_attach_device(struct iommu_domain
*domain,
struct device *dev);
static phys_addr_t intel_iommu_iova_to_phys(struct iommu_domain *domain,
dma_addr_t iova);
+static bool intel_iommu_dev_feat_enabled(struct device *dev,
+enum iommu_dev_features feat);
#ifdef CONFIG_INTEL_IOMMU_DEFAULT_ON
int dmar_disabled = 0;
@@ -1832,6 +1834,7 @@ static struct dmar_domain *alloc_domain(int flags)
domain->flags |= DOMAIN_FLAG_USE_FIRST_LEVEL;
domain->has_iotlb_device = false;
INIT_LIST_HEAD(&domain->devices);
+ INIT_LIST_HEAD(&domain->subdevices);
return domain;
}
@@ -2580,7 +2583,7 @@ static struct dmar_domain
*dmar_insert_one_dev_info(struct intel_iommu *iommu,
info->iommu = iommu;
info->pasid_table = NULL;
info->auxd_enabled = 0;
- INIT_LIST_HEAD(&info->auxiliary_domains);
+ INIT_LIST_HEAD(&info->subdevices);
if (dev && dev_is_pci(dev)) {
struct pci_dev *pdev = to_pci_dev(info->dev);
@@ -5137,21 +5140,28 @@ static void intel_iommu_domain_free(struct iommu_domain
*domain)
domain_exit(to_dmar_domain(domain));
}
-/*
- * Check whether a @domain could be attached to the @dev through the
- * aux-domain attach/detach APIs.
- */
-static inline bool
-is_aux_domain(struct device *dev, struct iommu_domain *domain)
+/* Lookup subdev_info in the domain's subdevice siblings. */
+static struct subdev_info *
+subdev_lookup_domain(struct dmar_domain *domain, struct device *dev,
+struct device *subdev)
{
- struct device_domain_info *info = get_domain_info(dev);
+ struct subdev_info *sinfo = NULL, *tmp;
- return info && info->auxd_enabled &&
- domain->type == IOMMU_DOMAIN_UNMANAGED;
+ assert_spin_locked(&device_domain_lock);
+
+ list_for_each_entry(tmp, &domain->subdevices, link_domain) {
+ if ((!dev || tmp->pdev == dev) && tmp->dev == subdev) {
+ sinfo = tmp;
+ break;
+ }
+ }
+
+ return sinfo;
}
-static void auxiliary_link_device(struct dmar_domain *domain,
- struct device *dev)
+static void
+subdev_link_device(struct dmar_domain *domain, struct device *dev,
+ struct subdev_info *sinfo)
{
struct device_domain_info *info = get_domain_info(dev);
@@ -5159,12 +5169,13 @@ static void auxiliary_link_device(struct dmar_domain
*domain,
if (WARN_ON(!info))
return;
- domain->auxd_refcnt++;
- list_add(&domain->auxd, &info->auxiliary_domains);
+ list_add(&info->subdevices, &sinfo->link_phys);
+ list_add(&domain->subdevices, &sinfo->link_domain);
}
-static void auxiliary_unlink_device(struct dmar_domain *domain,
- struct device *dev)
+static void
+subdev_unlink_device(struct dmar_domain *domain, struct device *dev,
+struct subdev_info *sinfo)
{
struct device_domain_info *info = get_domain_info(dev);
@@ -5172,24 +5183,30 @@ static void auxiliary_unlink_device(struct dmar_domain
*domain,
if (WARN_ON(!info))
return;
- list_del(&domain->auxd);
- domain->auxd_refcnt--;
+ list_del(&sinfo->link_phys);
+ list_del(&sinfo->link_domain);
+ kfree(sinfo);
- if (!domain->auxd_refcnt && domain->default_pasid > 0)
+ if (list_empty(&domain->subdevices) && domain->default_pasid > 0)
ioasid_free(domain->default_pasid);
}
-static int aux_domain_add_dev(struct dmar_domain *domain,
- struct device *dev)
+static int aux_domain_add_dev(struct dmar_domain *domain, struct device *dev,
+ struct device *subdev)
{
int ret;
unsigned long flags;
struct intel_iommu *iommu;
+ struct subdev_info *sinfo;
iommu = device_to_iommu(dev, NULL, NULL);
if (!iommu)
return -ENODEV;
+ sinfo = kzalloc(sizeof(*sinfo), GFP_KERNEL);
+ if (!sinfo)
+ return -ENOMEM;
+
if (domain->defaul
[PATCH v4 3/5] iommu: Add iommu_aux_get_domain_for_dev()
The device driver needs an API to get its aux-domain. A typical usage
scenario is:
unsigned long pasid;
struct iommu_domain *domain;
struct device *dev = mdev_dev(mdev);
struct device *iommu_device = vfio_mdev_get_iommu_device(dev);
domain = iommu_aux_get_domain_for_dev(dev);
if (!domain)
return -ENODEV;
pasid = iommu_aux_get_pasid(domain, iommu_device);
if (pasid <= 0)
return -EINVAL;
/* Program the device context */
This adds an API for such use case.
Suggested-by: Alex Williamson
Link: https://lore.kernel.org/linux-iommu/20200708130749.1b1e1...@x1.home/
Signed-off-by: Lu Baolu
---
drivers/iommu/iommu.c | 18 ++
include/linux/iommu.h | 20
2 files changed, 38 insertions(+)
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index fb21c2ff4861..8fd93a5b8764 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -2893,6 +2893,24 @@ void iommu_detach_subdev_group(struct iommu_domain
*domain,
}
EXPORT_SYMBOL_GPL(iommu_detach_subdev_group);
+struct iommu_domain *iommu_aux_get_domain_for_dev(struct device *subdev)
+{
+ struct iommu_domain *domain = NULL;
+ struct iommu_group *group;
+
+ group = iommu_group_get(subdev);
+ if (!group || !group->domain)
+ return NULL;
+
+ if (domain_is_aux(group->domain, subdev))
+ domain = group->domain;
+
+ iommu_group_put(group);
+
+ return domain;
+}
+EXPORT_SYMBOL_GPL(iommu_aux_get_domain_for_dev);
+
/**
* iommu_sva_bind_device() - Bind a process address space to a device
* @dev: the device
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index b9df8b510d4f..ea660a887dbf 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -120,6 +120,7 @@ enum iommu_attr {
DOMAIN_ATTR_NESTING,/* two stages of translation */
DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE,
DOMAIN_ATTR_MAX,
+ DOMAIN_ATTR_IS_AUX,
};
/* These are the possible reserved region types */
@@ -622,6 +623,12 @@ static inline void dev_iommu_priv_set(struct device *dev,
void *priv)
dev->iommu->priv = priv;
}
+static inline bool
+domain_is_aux(struct iommu_domain *domain, struct device *subdev)
+{
+ return iommu_domain_get_attr(domain, DOMAIN_ATTR_IS_AUX, subdev);
+}
+
int iommu_probe_device(struct device *dev);
void iommu_release_device(struct device *dev);
@@ -638,6 +645,7 @@ int iommu_attach_subdev_group(struct iommu_domain *domain,
void iommu_detach_subdev_group(struct iommu_domain *domain,
struct iommu_group *group,
iommu_device_lookup_t fn);
+struct iommu_domain *iommu_aux_get_domain_for_dev(struct device *subdev);
struct iommu_sva *iommu_sva_bind_device(struct device *dev,
struct mm_struct *mm,
@@ -1039,6 +1047,18 @@ iommu_detach_subdev_group(struct iommu_domain *domain,
struct iommu_group *group
{
}
+static inline bool
+domain_is_aux(struct iommu_domain *domain, struct device *subdev)
+{
+ return false;
+}
+
+static inline struct iommu_domain *
+iommu_aux_get_domain_for_dev(struct device *subdev)
+{
+ return NULL;
+}
+
static inline struct iommu_sva *
iommu_sva_bind_device(struct device *dev, struct mm_struct *mm, void *drvdata)
{
--
2.17.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v4 2/5] iommu: Add iommu_at(de)tach_subdev_group()
This adds two new APIs for the use cases like vfio/mdev where subdevices
derived from physical devices are created and put in an iommu_group. The
new IOMMU API interfaces mimic the vfio_mdev_at(de)tach_domain() directly,
testing whether the resulting device supports IOMMU_DEV_FEAT_AUX and using
an aux vs non-aux at(de)tach.
By doing this we could
- Set the iommu_group.domain. The iommu_group.domain is private to iommu
core (therefore vfio code cannot set it), but we need it set in order
for iommu_get_domain_for_dev() to work with a group attached to an aux
domain.
- Prefer to use the _attach_group() interfaces while the _attach_device()
interfaces are relegated to special cases.
Link: https://lore.kernel.org/linux-iommu/20200730134658.44c57...@x1.home/
Link: https://lore.kernel.org/linux-iommu/20200730151703.5daf8...@x1.home/
Signed-off-by: Lu Baolu
---
drivers/iommu/iommu.c | 136 ++
include/linux/iommu.h | 20 +++
2 files changed, 156 insertions(+)
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 38cdfeb887e1..fb21c2ff4861 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -2757,6 +2757,142 @@ int iommu_aux_get_pasid(struct iommu_domain *domain,
struct device *dev)
}
EXPORT_SYMBOL_GPL(iommu_aux_get_pasid);
+static int __iommu_aux_attach_device(struct iommu_domain *domain,
+struct device *phys_dev,
+struct device *sub_dev)
+{
+ int ret;
+
+ if (unlikely(!domain->ops->aux_attach_dev))
+ return -ENODEV;
+
+ ret = domain->ops->aux_attach_dev(domain, phys_dev, sub_dev);
+ if (!ret)
+ trace_attach_device_to_domain(sub_dev);
+
+ return ret;
+}
+
+static void __iommu_aux_detach_device(struct iommu_domain *domain,
+ struct device *phys_dev,
+ struct device *sub_dev)
+{
+ if (unlikely(!domain->ops->aux_detach_dev))
+ return;
+
+ domain->ops->aux_detach_dev(domain, phys_dev, sub_dev);
+ trace_detach_device_from_domain(sub_dev);
+}
+
+static int __iommu_attach_subdev_group(struct iommu_domain *domain,
+ struct iommu_group *group,
+ iommu_device_lookup_t fn)
+{
+ struct group_device *device;
+ struct device *phys_dev;
+ int ret = -ENODEV;
+
+ list_for_each_entry(device, &group->devices, list) {
+ phys_dev = fn(device->dev);
+ if (!phys_dev) {
+ ret = -ENODEV;
+ break;
+ }
+
+ if (iommu_dev_feature_enabled(phys_dev, IOMMU_DEV_FEAT_AUX))
+ ret = __iommu_aux_attach_device(domain, phys_dev,
+ device->dev);
+ else
+ ret = __iommu_attach_device(domain, phys_dev);
+
+ if (ret)
+ break;
+ }
+
+ return ret;
+}
+
+static void __iommu_detach_subdev_group(struct iommu_domain *domain,
+ struct iommu_group *group,
+ iommu_device_lookup_t fn)
+{
+ struct group_device *device;
+ struct device *phys_dev;
+
+ list_for_each_entry(device, &group->devices, list) {
+ phys_dev = fn(device->dev);
+ if (!phys_dev)
+ break;
+
+ if (iommu_dev_feature_enabled(phys_dev, IOMMU_DEV_FEAT_AUX))
+ __iommu_aux_detach_device(domain, phys_dev,
device->dev);
+ else
+ __iommu_detach_device(domain, phys_dev);
+ }
+}
+
+/**
+ * iommu_attach_subdev_group - attach domain to an iommu_group which
+ *contains subdevices.
+ *
+ * @domain: domain
+ * @group: iommu_group which contains subdevices
+ * @fn: callback for each subdevice in the @iommu_group to retrieve the
+ * physical device where the subdevice was created from.
+ *
+ * Returns 0 on success, or an error value.
+ */
+int iommu_attach_subdev_group(struct iommu_domain *domain,
+ struct iommu_group *group,
+ iommu_device_lookup_t fn)
+{
+ int ret = -ENODEV;
+
+ mutex_lock(&group->mutex);
+ if (group->domain) {
+ ret = -EBUSY;
+ goto unlock_out;
+ }
+
+ ret = __iommu_attach_subdev_group(domain, group, fn);
+ if (ret)
+ __iommu_detach_subdev_group(domain, group, fn);
+ else
+ group->domain = domain;
+
+unlock_out:
+ mutex_unlock(&group->mutex);
+ return ret;
+}
+EXPORT_SYMBOL_GPL(iommu_attach_subdev_group);
+
+/**
+ * iommu_detach_subdev_group - detach domain from an iommu_group which
+ *
Re: [PATCH 02/19] iommu/arm-smmu: Pass io-pgtable config to implementation specific function
On Thu 13 Aug 21:40 CDT 2020, Rob Clark wrote:
> From: Jordan Crouse
>
> Construct the io-pgtable config before calling the implementation specific
> init_context function and pass it so the implementation specific function
> can get a chance to change it before the io-pgtable is created.
>
> Signed-off-by: Jordan Crouse
> Signed-off-by: Rob Clark
Reviewed-by: Bjorn Andersson
Regards,
Bjorn
> ---
> drivers/iommu/arm/arm-smmu/arm-smmu-impl.c | 3 ++-
> drivers/iommu/arm/arm-smmu/arm-smmu.c | 11 ++-
> drivers/iommu/arm/arm-smmu/arm-smmu.h | 3 ++-
> 3 files changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c
> b/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c
> index f4ff124a1967..a9861dcd0884 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c
> @@ -68,7 +68,8 @@ static int cavium_cfg_probe(struct arm_smmu_device *smmu)
> return 0;
> }
>
> -static int cavium_init_context(struct arm_smmu_domain *smmu_domain)
> +static int cavium_init_context(struct arm_smmu_domain *smmu_domain,
> + struct io_pgtable_cfg *pgtbl_cfg)
> {
> struct cavium_smmu *cs = container_of(smmu_domain->smmu,
> struct cavium_smmu, smmu);
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c
> b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> index 09c42af9f31e..37d8d49299b4 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> @@ -795,11 +795,6 @@ static int arm_smmu_init_domain_context(struct
> iommu_domain *domain,
> cfg->asid = cfg->cbndx;
>
> smmu_domain->smmu = smmu;
> - if (smmu->impl && smmu->impl->init_context) {
> - ret = smmu->impl->init_context(smmu_domain);
> - if (ret)
> - goto out_unlock;
> - }
>
> pgtbl_cfg = (struct io_pgtable_cfg) {
> .pgsize_bitmap = smmu->pgsize_bitmap,
> @@ -810,6 +805,12 @@ static int arm_smmu_init_domain_context(struct
> iommu_domain *domain,
> .iommu_dev = smmu->dev,
> };
>
> + if (smmu->impl && smmu->impl->init_context) {
> + ret = smmu->impl->init_context(smmu_domain, &pgtbl_cfg);
> + if (ret)
> + goto out_clear_smmu;
> + }
> +
> if (smmu_domain->non_strict)
> pgtbl_cfg.quirks |= IO_PGTABLE_QUIRK_NON_STRICT;
>
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.h
> b/drivers/iommu/arm/arm-smmu/arm-smmu.h
> index d890a4a968e8..83294516ac08 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu.h
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.h
> @@ -386,7 +386,8 @@ struct arm_smmu_impl {
> u64 val);
> int (*cfg_probe)(struct arm_smmu_device *smmu);
> int (*reset)(struct arm_smmu_device *smmu);
> - int (*init_context)(struct arm_smmu_domain *smmu_domain);
> + int (*init_context)(struct arm_smmu_domain *smmu_domain,
> + struct io_pgtable_cfg *cfg);
> void (*tlb_sync)(struct arm_smmu_device *smmu, int page, int sync,
>int status);
> int (*def_domain_type)(struct device *dev);
> --
> 2.26.2
>
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v8 7/7] iommu/vt-d: Check UAPI data processed by IOMMU core
Hi Jacob,
On 9/1/20 2:25 AM, Jacob Pan wrote:
IOMMU generic layer already does sanity checks on UAPI data for version
match and argsz range based on generic information.
This patch adjusts the following data checking responsibilities:
- removes the redundant version check from VT-d driver
- removes the check for vendor specific data size
- adds check for the use of reserved/undefined flags
Signed-off-by: Jacob Pan
---
drivers/iommu/intel/iommu.c | 3 +--
drivers/iommu/intel/svm.c | 11 +--
include/uapi/linux/iommu.h | 1 +
3 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 43f16f0cebde..a3a0b5c8921d 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -5399,8 +5399,7 @@ intel_iommu_sva_invalidate(struct iommu_domain *domain,
struct device *dev,
int ret = 0;
u64 size = 0;
- if (!inv_info || !dmar_domain ||
- inv_info->version != IOMMU_CACHE_INVALIDATE_INFO_VERSION_1)
+ if (!inv_info || !dmar_domain)
return -EINVAL;
if (!dev || !dev_is_pci(dev))
diff --git a/drivers/iommu/intel/svm.c b/drivers/iommu/intel/svm.c
index 99353d6468fa..0cb9a15f1112 100644
--- a/drivers/iommu/intel/svm.c
+++ b/drivers/iommu/intel/svm.c
@@ -284,8 +284,15 @@ int intel_svm_bind_gpasid(struct iommu_domain *domain,
struct device *dev,
if (WARN_ON(!iommu) || !data)
return -EINVAL;
- if (data->version != IOMMU_GPASID_BIND_VERSION_1 ||
- data->format != IOMMU_PASID_FORMAT_INTEL_VTD)
+ if (data->format != IOMMU_PASID_FORMAT_INTEL_VTD)
+ return -EINVAL;
+
+ /* IOMMU core ensures argsz is more than the start of the union */
+ if (data->argsz < offsetofend(struct iommu_gpasid_bind_data,
vendor.vtd))
+ return -EINVAL;
+
+ /* Make sure no undefined flags are used in vendor data */
+ if (data->vendor.vtd.flags & ~(IOMMU_SVA_VTD_GPASID_LAST - 1))
return -EINVAL;
if (!dev_is_pci(dev))
diff --git a/include/uapi/linux/iommu.h b/include/uapi/linux/iommu.h
index c64bca5af419..1ebc23df4fbc 100644
--- a/include/uapi/linux/iommu.h
+++ b/include/uapi/linux/iommu.h
@@ -288,6 +288,7 @@ struct iommu_gpasid_bind_data_vtd {
#define IOMMU_SVA_VTD_GPASID_PWT (1 << 3) /* page-level write through */
#define IOMMU_SVA_VTD_GPASID_EMTE (1 << 4) /* extended mem type enable */
#define IOMMU_SVA_VTD_GPASID_CD (1 << 5) /* PASID-level cache
disable */
+#define IOMMU_SVA_VTD_GPASID_LAST (1 << 6)
__u64 flags;
__u32 pat;
__u32 emt;
Reviewed-by: Lu Baolu
Best regards,
baolu
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 01/19] drm/msm: remove dangling submitqueue references
On Fri 14 Aug 02:40 UTC 2020, Rob Clark wrote:
> From: Rob Clark
>
> Currently it doesn't matter, since we free the ctx immediately. But
> when we start refcnt'ing the ctx, we don't want old dangling list
> entries to hang around.
>
> Signed-off-by: Rob Clark
> ---
> drivers/gpu/drm/msm/msm_submitqueue.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/msm/msm_submitqueue.c
> b/drivers/gpu/drm/msm/msm_submitqueue.c
> index a1d94be7883a..90c9d84e6155 100644
> --- a/drivers/gpu/drm/msm/msm_submitqueue.c
> +++ b/drivers/gpu/drm/msm/msm_submitqueue.c
> @@ -49,8 +49,10 @@ void msm_submitqueue_close(struct msm_file_private *ctx)
>* No lock needed in close and there won't
>* be any more user ioctls coming our way
>*/
> - list_for_each_entry_safe(entry, tmp, &ctx->submitqueues, node)
> + list_for_each_entry_safe(entry, tmp, &ctx->submitqueues, node) {
> + list_del(&entry->node);
If you refcount ctx, what does that do for the entries in the submit
queue?
"entry" here is kref'ed, but you're popping it off the list regardless
of the put ends up freeing the object or not - which afaict would mean
leaking the object.
On the other hand, with the current implementation an object with higher
refcount with adjacent objects of single refcount would end up with
dangling pointers after the put. So in itself this change seems like a
net gain, but I'm wondering about the plan described in the commit
message.
Regards,
Bjorn
> msm_submitqueue_put(entry);
> + }
> }
>
> int msm_submitqueue_create(struct drm_device *drm, struct msm_file_private
> *ctx,
> --
> 2.26.2
>
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [Regression] [PATCH] iommu: Avoid crash in init_no_remapping_devices if iommu is NULL
Hi Torsten,
Thank you for reporting this.
On 8/31/20 7:03 PM, Torsten Hilbrich wrote:
I noticed a kernel crash when trying to boot a v5.9-rc2 based kernel.
The crash reads as:
<1>[7.410192] BUG: kernel NULL pointer dereference, address:
0038
<1>[7.410196] #PF: supervisor write access in kernel mode
<1>[7.410199] #PF: error_code(0x0002) - not-present page
<6>[7.410202] PGD 0 P4D 0
<4>[7.410207] Oops: 0002 [#1] SMP PTI
<4>[7.410212] CPU: 1 PID: 1 Comm: swapper/0 Not tainted 5.9.0-devel+ #1
<4>[7.410215] Hardware name: LENOVO 20HGS0TW00/20HGS0TW00, BIOS N1WET46S
(1.25s ) 03/30/2018
<4>[7.410224] RIP: 0010:intel_iommu_init+0xed0/0x1136
<4>[7.410229] Code: fe e9 61 02 00 00 bb f4 ff ff ff e9 57 02 00 00 48 63 d1 48
c1 e2 04 48 03 50 20 48 8b 12 48 85 d2 74 0b 48 8b 92 d0 02 00 00 <48> 89 7a 38 ff c1
e9 15 f5 ff ff 48 c7 c7 00 a5 ac a1 49 c7 c7 20
<4>[7.410236] RSP: :b14e40073dd0 EFLAGS: 00010282
<4>[7.410240] RAX: 8d0643731720 RBX: RCX:
<4>[7.410244] RDX: RSI: RDI:
<4>[7.410247] RBP: b14e40073e90 R08: 0001 R09:
8d0643803700
<4>[7.410250] R10: 8d064262 R11: 0016 R12:
000b
<4>[7.410254] R13: 8d064361e650 R14: a2455d80 R15:
<4>[7.410258] FS: () GS:8d064748()
knlGS:
<4>[7.410262] CS: 0010 DS: ES: CR0: 80050033
<4>[7.410266] CR2: 0038 CR3: 00056760a001 CR4:
003706e0
<4>[7.410269] Call Trace:
<4>[7.410280] ? call_rcu+0x10e/0x320
<4>[7.410286] ? trace_hardirqs_on+0x2c/0xd0
<4>[7.410291] ? rdinit_setup+0x2c/0x2c
<4>[7.410297] ? e820__memblock_setup+0x8b/0x8b
<4>[7.410302] pci_iommu_init+0x16/0x3f
<4>[7.410307] do_one_initcall+0x46/0x1e4
<4>[7.410313] kernel_init_freeable+0x169/0x1b2
<4>[7.410320] ? rest_init+0x9f/0x9f
<4>[7.410325] kernel_init+0xa/0x101
<4>[7.410329] ret_from_fork+0x22/0x30
<4>[7.410333] Modules linked in:
<4>[7.410338] CR2: 0038
<4>[7.410344] ---[ end trace 16bcdb1e11668fcd ]---
Full kernel message is attached in kernel.log.
I tracked the problem down to the dev_iommu_priv_set call in
init_no_remapping_devices. It seems that for one device the dev->iommu member
is NULL.
An dev_err outputs the device as "pci :00:02.0: DMAR" which is the intel HD
620 graphic adapter in a Lenovo T470s device.
The following patch (also attached as intel-iommu.patch) avoids this crash by
checking the pointer.
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index f8177c59d229..2d285a42db3f 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -4053,7 +4053,8 @@ static void __init init_no_remapping_devices(void)
drhd->ignored = 1;
for_each_active_dev_scope(drhd->devices,
drhd->devices_cnt, i, dev)
- dev_iommu_priv_set(dev,
DUMMY_DEVICE_DOMAIN_INFO);
+ if (dev->iommu)
+ dev_iommu_priv_set(dev,
DUMMY_DEVICE_DOMAIN_INFO);
This looks more like a generic issue, used-before-allocated, and should
be fixed in iommu.c instead of VT-d driver. How about
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 8fd93a5b8764..a599da87eb60 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -190,6 +190,28 @@ static void dev_iommu_free(struct device *dev)
dev->iommu = NULL;
}
+void *dev_iommu_priv_get(struct device *dev)
+{
+ struct dev_iommu *param = dev_iommu_get(dev);
+
+ if (WARN_ON(!param))
+ return ERR_PTR(-ENOMEM);
+
+return param->priv;
+}
+EXPORT_SYMBOL_GPL(dev_iommu_priv_get);
+
+void dev_iommu_priv_set(struct device *dev, void *priv)
+{
+ struct dev_iommu *param = dev_iommu_get(dev);
+
+ if (WARN_ON(!param))
+ return;
+
+param->priv = priv;
+}
+EXPORT_SYMBOL_GPL(dev_iommu_priv_set);
+
Best regards,
baolu
}
}
}
I assume the problem might be related to the following commit introduced in
v5.9-rc1:
commit 01b9d4e21148c89fdbab3d6b3705f9791314b631
Author: Joerg Roedel
Date: Thu Jun 25 15:08:25 2020 +0200
iommu/vt-d: Use dev_iommu_priv_get/set()
Remove the use of dev->archdata.iommu and use the private per-device
pointer provided by IOMMU core code instead.
With regards,
Torsten Hilbrich
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundati
[PATCH v8 3/7] iommu/uapi: Introduce enum type for PASID data format
There can be multiple vendor-specific PASID data formats used in UAPI
structures. This patch adds enum type with a last entry which makes
range checking much easier.
Suggested-by: Alex Williamson
Reviewed-by: Eric Auger
Signed-off-by: Jacob Pan
---
include/uapi/linux/iommu.h | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/include/uapi/linux/iommu.h b/include/uapi/linux/iommu.h
index b42acc8fe007..7cc6ee6c41f7 100644
--- a/include/uapi/linux/iommu.h
+++ b/include/uapi/linux/iommu.h
@@ -298,11 +298,16 @@ struct iommu_gpasid_bind_data_vtd {
IOMMU_SVA_VTD_GPASID_PCD | \
IOMMU_SVA_VTD_GPASID_PWT)
+enum iommu_pasid_data_format {
+ IOMMU_PASID_FORMAT_INTEL_VTD = 1,
+ IOMMU_PASID_FORMAT_LAST,
+};
+
/**
* struct iommu_gpasid_bind_data - Information about device and guest PASID
binding
* @argsz: User filled size of this data
* @version: Version of this data structure
- * @format:PASID table entry format
+ * @format:PASID table entry format of enum iommu_pasid_data_format type
* @flags: Additional information on guest bind request
* @gpgd: Guest page directory base of the guest mm to bind
* @hpasid:Process address space ID used for the guest mm in host IOMMU
@@ -321,7 +326,6 @@ struct iommu_gpasid_bind_data {
__u32 argsz;
#define IOMMU_GPASID_BIND_VERSION_11
__u32 version;
-#define IOMMU_PASID_FORMAT_INTEL_VTD 1
__u32 format;
__u32 addr_width;
#define IOMMU_SVA_GPASID_VAL (1 << 0) /* guest PASID valid */
--
2.7.4
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v8 2/7] iommu/uapi: Add argsz for user filled data
As IOMMU UAPI gets extended, user data size may increase. To support
backward compatibiliy, this patch introduces a size field to each UAPI
data structures. It is *always* the responsibility for the user to fill in
the correct size. Padding fields are adjusted to ensure 8 byte alignment.
Specific scenarios for user data handling are documented in:
Documentation/userspace-api/iommu.rst
As there is no current users of the API, struct version is not
incremented.
Reviewed-by: Eric Auger
Signed-off-by: Liu Yi L
Signed-off-by: Jacob Pan
---
include/uapi/linux/iommu.h | 12 +---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/include/uapi/linux/iommu.h b/include/uapi/linux/iommu.h
index c2b2caf9ed41..b42acc8fe007 100644
--- a/include/uapi/linux/iommu.h
+++ b/include/uapi/linux/iommu.h
@@ -139,6 +139,7 @@ enum iommu_page_response_code {
/**
* struct iommu_page_response - Generic page response information
+ * @argsz: User filled size of this data
* @version: API version of this structure
* @flags: encodes whether the corresponding fields are valid
* (IOMMU_FAULT_PAGE_RESPONSE_* values)
@@ -147,6 +148,7 @@ enum iommu_page_response_code {
* @code: response code from &enum iommu_page_response_code
*/
struct iommu_page_response {
+ __u32 argsz;
#define IOMMU_PAGE_RESP_VERSION_1 1
__u32 version;
#define IOMMU_PAGE_RESP_PASID_VALID(1 << 0)
@@ -222,6 +224,7 @@ struct iommu_inv_pasid_info {
/**
* struct iommu_cache_invalidate_info - First level/stage invalidation
* information
+ * @argsz: User filled size of this data
* @version: API version of this structure
* @cache: bitfield that allows to select which caches to invalidate
* @granularity: defines the lowest granularity used for the invalidation:
@@ -250,6 +253,7 @@ struct iommu_inv_pasid_info {
* must support the used granularity.
*/
struct iommu_cache_invalidate_info {
+ __u32 argsz;
#define IOMMU_CACHE_INVALIDATE_INFO_VERSION_1 1
__u32 version;
/* IOMMU paging structure cache */
@@ -259,7 +263,7 @@ struct iommu_cache_invalidate_info {
#define IOMMU_CACHE_INV_TYPE_NR(3)
__u8cache;
__u8granularity;
- __u8padding[2];
+ __u8padding[6];
union {
struct iommu_inv_pasid_info pasid_info;
struct iommu_inv_addr_info addr_info;
@@ -296,6 +300,7 @@ struct iommu_gpasid_bind_data_vtd {
/**
* struct iommu_gpasid_bind_data - Information about device and guest PASID
binding
+ * @argsz: User filled size of this data
* @version: Version of this data structure
* @format:PASID table entry format
* @flags: Additional information on guest bind request
@@ -313,17 +318,18 @@ struct iommu_gpasid_bind_data_vtd {
* PASID to host PASID based on this bind data.
*/
struct iommu_gpasid_bind_data {
+ __u32 argsz;
#define IOMMU_GPASID_BIND_VERSION_11
__u32 version;
#define IOMMU_PASID_FORMAT_INTEL_VTD 1
__u32 format;
+ __u32 addr_width;
#define IOMMU_SVA_GPASID_VAL (1 << 0) /* guest PASID valid */
__u64 flags;
__u64 gpgd;
__u64 hpasid;
__u64 gpasid;
- __u32 addr_width;
- __u8 padding[12];
+ __u8 padding[8];
/* Vendor specific data */
union {
struct iommu_gpasid_bind_data_vtd vtd;
--
2.7.4
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v8 6/7] iommu/uapi: Handle data and argsz filled by users
IOMMU user APIs are responsible for processing user data. This patch
changes the interface such that user pointers can be passed into IOMMU
code directly. Separate kernel APIs without user pointers are introduced
for in-kernel users of the UAPI functionality.
IOMMU UAPI data has a user filled argsz field which indicates the data
length of the structure. User data is not trusted, argsz must be
validated based on the current kernel data size, mandatory data size,
and feature flags.
User data may also be extended, resulting in possible argsz increase.
Backward compatibility is ensured based on size and flags (or
the functional equivalent fields) checking.
This patch adds sanity checks in the IOMMU layer. In addition to argsz,
reserved/unused fields in padding, flags, and version are also checked.
Details are documented in Documentation/userspace-api/iommu.rst
Signed-off-by: Liu Yi L
Signed-off-by: Jacob Pan
---
drivers/iommu/iommu.c | 201 --
include/linux/iommu.h | 28 ---
2 files changed, 212 insertions(+), 17 deletions(-)
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 4ae02291ccc2..3bc263ae31ed 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1961,33 +1961,218 @@ int iommu_attach_device(struct iommu_domain *domain,
struct device *dev)
}
EXPORT_SYMBOL_GPL(iommu_attach_device);
+/*
+ * Check flags and other user provided data for valid combinations. We also
+ * make sure no reserved fields or unused flags are set. This is to ensure
+ * not breaking userspace in the future when these fields or flags are used.
+ */
+static int iommu_check_cache_invl_data(struct iommu_cache_invalidate_info
*info)
+{
+ u32 mask;
+ int i;
+
+ if (info->version != IOMMU_CACHE_INVALIDATE_INFO_VERSION_1)
+ return -EINVAL;
+
+ mask = (1 << IOMMU_CACHE_INV_TYPE_NR) - 1;
+ if (info->cache & ~mask)
+ return -EINVAL;
+
+ if (info->granularity >= IOMMU_INV_GRANU_NR)
+ return -EINVAL;
+
+ switch (info->granularity) {
+ case IOMMU_INV_GRANU_ADDR:
+ if (info->cache & IOMMU_CACHE_INV_TYPE_PASID)
+ return -EINVAL;
+
+ mask = IOMMU_INV_ADDR_FLAGS_PASID |
+ IOMMU_INV_ADDR_FLAGS_ARCHID |
+ IOMMU_INV_ADDR_FLAGS_LEAF;
+
+ if (info->granu.addr_info.flags & ~mask)
+ return -EINVAL;
+ break;
+ case IOMMU_INV_GRANU_PASID:
+ mask = IOMMU_INV_PASID_FLAGS_PASID |
+ IOMMU_INV_PASID_FLAGS_ARCHID;
+ if (info->granu.pasid_info.flags & ~mask)
+ return -EINVAL;
+
+ break;
+ case IOMMU_INV_GRANU_DOMAIN:
+ if (info->cache & IOMMU_CACHE_INV_TYPE_DEV_IOTLB)
+ return -EINVAL;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ /* Check reserved padding fields */
+ for (i = 0; i < sizeof(info->padding); i++) {
+ if (info->padding[i])
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
int iommu_uapi_cache_invalidate(struct iommu_domain *domain, struct device
*dev,
- struct iommu_cache_invalidate_info *inv_info)
+ void __user *uinfo)
{
+ struct iommu_cache_invalidate_info inv_info = { 0 };
+ u32 minsz;
+ int ret = 0;
+
if (unlikely(!domain->ops->cache_invalidate))
return -ENODEV;
- return domain->ops->cache_invalidate(domain, dev, inv_info);
+ /*
+* No new spaces can be added before the variable sized union, the
+* minimum size is the offset to the union.
+*/
+ minsz = offsetof(struct iommu_cache_invalidate_info, granu);
+
+ /* Copy minsz from user to get flags and argsz */
+ if (copy_from_user(&inv_info, uinfo, minsz))
+ return -EFAULT;
+
+ /* Fields before variable size union is mandatory */
+ if (inv_info.argsz < minsz)
+ return -EINVAL;
+
+ /* PASID and address granu require additional info beyond minsz */
+ if (inv_info.argsz == minsz &&
+ ((inv_info.granularity == IOMMU_INV_GRANU_PASID) ||
+ (inv_info.granularity == IOMMU_INV_GRANU_ADDR)))
+ return -EINVAL;
+
+ if (inv_info.granularity == IOMMU_INV_GRANU_PASID &&
+ inv_info.argsz < offsetofend(struct iommu_cache_invalidate_info,
granu.pasid_info))
+ return -EINVAL;
+
+ if (inv_info.granularity == IOMMU_INV_GRANU_ADDR &&
+ inv_info.argsz < offsetofend(struct iommu_cache_invalidate_info,
granu.addr_info))
+ return -EINVAL;
+
+ /*
+* User might be using a newer UAPI header which has a larger data
+* size, we shall support the existing flag
[PATCH v8 4/7] iommu/uapi: Use named union for user data
IOMMU UAPI data size is filled by the user space which must be validated
by the kernel. To ensure backward compatibility, user data can only be
extended by either re-purpose padding bytes or extend the variable sized
union at the end. No size change is allowed before the union. Therefore,
the minimum size is the offset of the union.
To use offsetof() on the union, we must make it named.
Link: https://lore.kernel.org/linux-iommu/20200611145518.0c281...@x1.home/
Signed-off-by: Jacob Pan
Reviewed-by: Lu Baolu
Reviewed-by: Eric Auger
---
drivers/iommu/intel/iommu.c | 22 +++---
drivers/iommu/intel/svm.c | 2 +-
include/uapi/linux/iommu.h | 4 ++--
3 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index e9864e52b0e9..43f16f0cebde 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -5425,8 +5425,8 @@ intel_iommu_sva_invalidate(struct iommu_domain *domain,
struct device *dev,
/* Size is only valid in address selective invalidation */
if (inv_info->granularity == IOMMU_INV_GRANU_ADDR)
- size = to_vtd_size(inv_info->addr_info.granule_size,
- inv_info->addr_info.nb_granules);
+ size = to_vtd_size(inv_info->granu.addr_info.granule_size,
+ inv_info->granu.addr_info.nb_granules);
for_each_set_bit(cache_type,
(unsigned long *)&inv_info->cache,
@@ -5447,20 +5447,20 @@ intel_iommu_sva_invalidate(struct iommu_domain *domain,
struct device *dev,
* granularity.
*/
if (inv_info->granularity == IOMMU_INV_GRANU_PASID &&
- (inv_info->pasid_info.flags & IOMMU_INV_PASID_FLAGS_PASID))
- pasid = inv_info->pasid_info.pasid;
+ (inv_info->granu.pasid_info.flags &
IOMMU_INV_PASID_FLAGS_PASID))
+ pasid = inv_info->granu.pasid_info.pasid;
else if (inv_info->granularity == IOMMU_INV_GRANU_ADDR &&
-(inv_info->addr_info.flags &
IOMMU_INV_ADDR_FLAGS_PASID))
- pasid = inv_info->addr_info.pasid;
+(inv_info->granu.addr_info.flags &
IOMMU_INV_ADDR_FLAGS_PASID))
+ pasid = inv_info->granu.addr_info.pasid;
switch (BIT(cache_type)) {
case IOMMU_CACHE_INV_TYPE_IOTLB:
/* HW will ignore LSB bits based on address mask */
if (inv_info->granularity == IOMMU_INV_GRANU_ADDR &&
size &&
- (inv_info->addr_info.addr & ((BIT(VTD_PAGE_SHIFT +
size)) - 1))) {
+ (inv_info->granu.addr_info.addr &
((BIT(VTD_PAGE_SHIFT + size)) - 1))) {
pr_err_ratelimited("User address not aligned,
0x%llx, size order %llu\n",
- inv_info->addr_info.addr,
size);
+
inv_info->granu.addr_info.addr, size);
}
/*
@@ -5468,9 +5468,9 @@ intel_iommu_sva_invalidate(struct iommu_domain *domain,
struct device *dev,
* We use npages = -1 to indicate that.
*/
qi_flush_piotlb(iommu, did, pasid,
- mm_to_dma_pfn(inv_info->addr_info.addr),
+
mm_to_dma_pfn(inv_info->granu.addr_info.addr),
(granu == QI_GRAN_NONG_PASID) ? -1 : 1
<< size,
- inv_info->addr_info.flags &
IOMMU_INV_ADDR_FLAGS_LEAF);
+ inv_info->granu.addr_info.flags &
IOMMU_INV_ADDR_FLAGS_LEAF);
if (!info->ats_enabled)
break;
@@ -5493,7 +5493,7 @@ intel_iommu_sva_invalidate(struct iommu_domain *domain,
struct device *dev,
size = 64 - VTD_PAGE_SHIFT;
addr = 0;
} else if (inv_info->granularity ==
IOMMU_INV_GRANU_ADDR) {
- addr = inv_info->addr_info.addr;
+ addr = inv_info->granu.addr_info.addr;
}
if (info->ats_enabled)
diff --git a/drivers/iommu/intel/svm.c b/drivers/iommu/intel/svm.c
index 95c3164a2302..99353d6468fa 100644
--- a/drivers/iommu/intel/svm.c
+++ b/drivers/iommu/intel/svm.c
@@ -370,7 +370,7 @@ int intel_svm_bind_gpasid(struct iommu_domain *domain,
struct device *dev,
spin_lock(&iommu->lock);
ret = intel_pasid_setup_nested(iommu, dev,
(pgd_t *)(uintptr_t)data->gpgd,
- data->hpasid,
[PATCH v8 7/7] iommu/vt-d: Check UAPI data processed by IOMMU core
IOMMU generic layer already does sanity checks on UAPI data for version
match and argsz range based on generic information.
This patch adjusts the following data checking responsibilities:
- removes the redundant version check from VT-d driver
- removes the check for vendor specific data size
- adds check for the use of reserved/undefined flags
Signed-off-by: Jacob Pan
---
drivers/iommu/intel/iommu.c | 3 +--
drivers/iommu/intel/svm.c | 11 +--
include/uapi/linux/iommu.h | 1 +
3 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 43f16f0cebde..a3a0b5c8921d 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -5399,8 +5399,7 @@ intel_iommu_sva_invalidate(struct iommu_domain *domain,
struct device *dev,
int ret = 0;
u64 size = 0;
- if (!inv_info || !dmar_domain ||
- inv_info->version != IOMMU_CACHE_INVALIDATE_INFO_VERSION_1)
+ if (!inv_info || !dmar_domain)
return -EINVAL;
if (!dev || !dev_is_pci(dev))
diff --git a/drivers/iommu/intel/svm.c b/drivers/iommu/intel/svm.c
index 99353d6468fa..0cb9a15f1112 100644
--- a/drivers/iommu/intel/svm.c
+++ b/drivers/iommu/intel/svm.c
@@ -284,8 +284,15 @@ int intel_svm_bind_gpasid(struct iommu_domain *domain,
struct device *dev,
if (WARN_ON(!iommu) || !data)
return -EINVAL;
- if (data->version != IOMMU_GPASID_BIND_VERSION_1 ||
- data->format != IOMMU_PASID_FORMAT_INTEL_VTD)
+ if (data->format != IOMMU_PASID_FORMAT_INTEL_VTD)
+ return -EINVAL;
+
+ /* IOMMU core ensures argsz is more than the start of the union */
+ if (data->argsz < offsetofend(struct iommu_gpasid_bind_data,
vendor.vtd))
+ return -EINVAL;
+
+ /* Make sure no undefined flags are used in vendor data */
+ if (data->vendor.vtd.flags & ~(IOMMU_SVA_VTD_GPASID_LAST - 1))
return -EINVAL;
if (!dev_is_pci(dev))
diff --git a/include/uapi/linux/iommu.h b/include/uapi/linux/iommu.h
index c64bca5af419..1ebc23df4fbc 100644
--- a/include/uapi/linux/iommu.h
+++ b/include/uapi/linux/iommu.h
@@ -288,6 +288,7 @@ struct iommu_gpasid_bind_data_vtd {
#define IOMMU_SVA_VTD_GPASID_PWT (1 << 3) /* page-level write through */
#define IOMMU_SVA_VTD_GPASID_EMTE (1 << 4) /* extended mem type enable */
#define IOMMU_SVA_VTD_GPASID_CD(1 << 5) /* PASID-level cache
disable */
+#define IOMMU_SVA_VTD_GPASID_LAST (1 << 6)
__u64 flags;
__u32 pat;
__u32 emt;
--
2.7.4
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v8 5/7] iommu/uapi: Rename uapi functions
User APIs such as iommu_sva_unbind_gpasid() may also be used by the
kernel. Since we introduced user pointer to the UAPI functions,
in-kernel callers cannot share the same APIs. In-kernel callers are also
trusted, there is no need to validate the data.
We plan to have two flavors of the same API functions, one called
through ioctls, carrying a user pointer and one called directly with
valid IOMMU UAPI structs. To differentiate both, let's rename existing
functions with an iommu_uapi_ prefix.
Suggested-by: Alex Williamson
Reviewed-by: Eric Auger
Signed-off-by: Jacob Pan
---
drivers/iommu/iommu.c | 18 +-
include/linux/iommu.h | 31 ---
2 files changed, 25 insertions(+), 24 deletions(-)
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 609bd25bf154..4ae02291ccc2 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1961,35 +1961,35 @@ int iommu_attach_device(struct iommu_domain *domain,
struct device *dev)
}
EXPORT_SYMBOL_GPL(iommu_attach_device);
-int iommu_cache_invalidate(struct iommu_domain *domain, struct device *dev,
- struct iommu_cache_invalidate_info *inv_info)
+int iommu_uapi_cache_invalidate(struct iommu_domain *domain, struct device
*dev,
+ struct iommu_cache_invalidate_info *inv_info)
{
if (unlikely(!domain->ops->cache_invalidate))
return -ENODEV;
return domain->ops->cache_invalidate(domain, dev, inv_info);
}
-EXPORT_SYMBOL_GPL(iommu_cache_invalidate);
+EXPORT_SYMBOL_GPL(iommu_uapi_cache_invalidate);
-int iommu_sva_bind_gpasid(struct iommu_domain *domain,
- struct device *dev, struct iommu_gpasid_bind_data
*data)
+int iommu_uapi_sva_bind_gpasid(struct iommu_domain *domain,
+ struct device *dev, struct
iommu_gpasid_bind_data *data)
{
if (unlikely(!domain->ops->sva_bind_gpasid))
return -ENODEV;
return domain->ops->sva_bind_gpasid(domain, dev, data);
}
-EXPORT_SYMBOL_GPL(iommu_sva_bind_gpasid);
+EXPORT_SYMBOL_GPL(iommu_uapi_sva_bind_gpasid);
-int iommu_sva_unbind_gpasid(struct iommu_domain *domain, struct device *dev,
-ioasid_t pasid)
+int iommu_uapi_sva_unbind_gpasid(struct iommu_domain *domain, struct device
*dev,
+ioasid_t pasid)
{
if (unlikely(!domain->ops->sva_unbind_gpasid))
return -ENODEV;
return domain->ops->sva_unbind_gpasid(dev, pasid);
}
-EXPORT_SYMBOL_GPL(iommu_sva_unbind_gpasid);
+EXPORT_SYMBOL_GPL(iommu_uapi_sva_unbind_gpasid);
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 fee209efb756..710d5d2691eb 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -424,13 +424,13 @@ 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_cache_invalidate(struct iommu_domain *domain,
- struct device *dev,
- struct iommu_cache_invalidate_info *inv_info);
-extern int iommu_sva_bind_gpasid(struct iommu_domain *domain,
- struct device *dev, struct iommu_gpasid_bind_data *data);
-extern int iommu_sva_unbind_gpasid(struct iommu_domain *domain,
- struct device *dev, ioasid_t pasid);
+extern int iommu_uapi_cache_invalidate(struct iommu_domain *domain,
+ struct device *dev,
+ struct iommu_cache_invalidate_info
*inv_info);
+extern int iommu_uapi_sva_bind_gpasid(struct iommu_domain *domain,
+ struct device *dev, struct
iommu_gpasid_bind_data *data);
+extern int iommu_uapi_sva_unbind_gpasid(struct iommu_domain *domain,
+ struct device *dev, ioasid_t pasid);
extern struct iommu_domain *iommu_get_domain_for_dev(struct device *dev);
extern struct iommu_domain *iommu_get_dma_domain(struct device *dev);
extern int iommu_map(struct iommu_domain *domain, unsigned long iova,
@@ -1032,21 +1032,22 @@ static inline int iommu_sva_get_pasid(struct iommu_sva
*handle)
return IOMMU_PASID_INVALID;
}
-static inline int
-iommu_cache_invalidate(struct iommu_domain *domain,
- struct device *dev,
- struct iommu_cache_invalidate_info *inv_info)
+static inline int iommu_uapi_cache_invalidate(struct iommu_domain *domain,
+ struct device *dev,
+ struct
iommu_cache_invalidate_info *inv_info)
{
return -ENODEV;
}
-static inline int iom
[PATCH v8 0/7] IOMMU user API enhancement
IOMMU user API header was introduced to support nested DMA translation and related fault handling. The current UAPI data structures consist of three areas that cover the interactions between host kernel and guest: - fault handling - cache invalidation - bind guest page tables, i.e. guest PASID Future extensions are likely to support more architectures and vIOMMU features. In the previous discussion, using user-filled data size and feature flags is made a preferred approach over a unified version number. https://lkml.org/lkml/2020/1/29/45 In addition to introduce argsz field to data structures, this patchset is also trying to document the UAPI design, usage, and extension rules. VT-d driver changes to utilize the new argsz field is included, VFIO usage is to follow. This set is available at: https://github.com/jacobpan/linux.git vsva_v5.8_uapi_v8 Thanks, Jacob Changelog: v8 - Rebased to v5.9-rc2 - Addressed review comments from Eric Auger 1. added a check for the unused vendor flags 2. commit message improvements v7 - Added PASID data format enum for range checking - Tidy up based on reviews from Alex W. - Removed doc section for vIOMMU fault handling v6 - Renamed all UAPI functions with iommu_uapi_ prefix - Replaced argsz maxsz checking with flag specific size checks - Documentation improvements based on suggestions by Eric Auger Replaced example code with a pointer to the actual code - Added more checks for illegal flags combinations - Added doc file to MAINTAINERS v5 - Addjusted paddings in UAPI data to be 8 byte aligned - Do not clobber argsz in IOMMU core before passing on to vendor driver - Removed pr_warn_ for invalid UAPI data check, just return -EINVAL - Clarified VFIO responsibility in UAPI data handling - Use iommu_uapi prefix to differentiate APIs has in-kernel caller - Added comment for unchecked flags of invalidation granularity - Added example in doc to show vendor data checking v4 - Added checks of UAPI data for reserved fields, version, and flags. - Removed version check from vendor driver (vt-d) - Relaxed argsz check to match the UAPI struct size instead of variable union size - Updated documentation v3: - Rewrote backward compatibility rule to support existing code re-compiled with newer kernel UAPI header that runs on older kernel. Based on review comment from Alex W. https://lore.kernel.org/linux-iommu/20200611094741.6d118...@w520.home/ - Take user pointer directly in UAPI functions. Perform argsz check and copy_from_user() in IOMMU driver. Eliminate the need for VFIO or other upper layer to parse IOMMU data. - Create wrapper function for in-kernel users of UAPI functions v2: - Removed unified API version and helper - Introduced argsz for each UAPI data - Introduced UAPI doc Jacob Pan (7): docs: IOMMU user API iommu/uapi: Add argsz for user filled data iommu/uapi: Introduce enum type for PASID data format iommu/uapi: Use named union for user data iommu/uapi: Rename uapi functions iommu/uapi: Handle data and argsz filled by users iommu/vt-d: Check UAPI data processed by IOMMU core Documentation/userspace-api/iommu.rst | 211 ++ MAINTAINERS | 1 + drivers/iommu/intel/iommu.c | 25 ++-- drivers/iommu/intel/svm.c | 13 ++- drivers/iommu/iommu.c | 205 +++-- include/linux/iommu.h | 35 -- include/uapi/linux/iommu.h| 25 ++-- 7 files changed, 470 insertions(+), 45 deletions(-) create mode 100644 Documentation/userspace-api/iommu.rst -- 2.7.4 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v8 1/7] docs: IOMMU user API
IOMMU UAPI is newly introduced to support communications between guest virtual IOMMU and host IOMMU. There has been lots of discussions on how it should work with VFIO UAPI and userspace in general. This document is intended to clarify the UAPI design and usage. The mechanics of how future extensions should be achieved are also covered in this documentation. Reviewed-by: Eric Auger Signed-off-by: Liu Yi L Signed-off-by: Jacob Pan --- Documentation/userspace-api/iommu.rst | 211 ++ MAINTAINERS | 1 + 2 files changed, 212 insertions(+) create mode 100644 Documentation/userspace-api/iommu.rst diff --git a/Documentation/userspace-api/iommu.rst b/Documentation/userspace-api/iommu.rst new file mode 100644 index ..1e68e8f05bb3 --- /dev/null +++ b/Documentation/userspace-api/iommu.rst @@ -0,0 +1,211 @@ +.. SPDX-License-Identifier: GPL-2.0 +.. iommu: + += +IOMMU Userspace API += + +IOMMU UAPI is used for virtualization cases where communications are +needed between physical and virtual IOMMU drivers. For baremetal +usage, the IOMMU is a system device which does not need to communicate +with user space directly. + +The primary use cases are guest Shared Virtual Address (SVA) and +guest IO virtual address (IOVA), wherin the vIOMMU implementation +relies on the physical IOMMU and for this reason requires interactions +with the host driver. + +.. contents:: :local: + +Functionalities +=== +Communications of user and kernel involve both directions. The +supported user-kernel APIs are as follows: + +1. Alloc/Free PASID +2. Bind/Unbind guest PASID (e.g. Intel VT-d) +3. Bind/Unbind guest PASID table (e.g. ARM SMMU) +4. Invalidate IOMMU caches upon guest requests +5. Report errors to the guest and serve page requests + +Requirements + +The IOMMU UAPIs are generic and extensible to meet the following +requirements: + +1. Emulated and para-virtualised vIOMMUs +2. Multiple vendors (Intel VT-d, ARM SMMU, etc.) +3. Extensions to the UAPI shall not break existing user space + +Interfaces +== +Although the data structures defined in IOMMU UAPI are self-contained, +there is no user API functions introduced. Instead, IOMMU UAPI is +designed to work with existing user driver frameworks such as VFIO. + +Extension Rules & Precautions +- +When IOMMU UAPI gets extended, the data structures can *only* be +modified in two ways: + +1. Adding new fields by re-purposing the padding[] field. No size change. +2. Adding new union members at the end. May increase the structure sizes. + +No new fields can be added *after* the variable sized union in that it +will break backward compatibility when offset moves. A new flag must +be introduced whenever a change affects the structure using either +method. The IOMMU driver processes the data based on flags which +ensures backward compatibility. + +Version field is only reserved for the unlikely event of UAPI upgrade +at its entirety. + +It's *always* the caller's responsibility to indicate the size of the +structure passed by setting argsz appropriately. +Though at the same time, argsz is user provided data which is not +trusted. The argsz field allows the user app to indicate how much data +it is providing, it's still the kernel's responsibility to validate +whether it's correct and sufficient for the requested operation. + +Compatibility Checking +-- +When IOMMU UAPI extension results in some structure size increase, +IOMMU UAPI code shall handle the following cases: + +1. User and kernel has exact size match +2. An older user with older kernel header (smaller UAPI size) running on a + newer kernel (larger UAPI size) +3. A newer user with newer kernel header (larger UAPI size) running + on an older kernel. +4. A malicious/misbehaving user pass illegal/invalid size but within + range. The data may contain garbage. + +Feature Checking + +While launching a guest with vIOMMU, it is strongly advised to check +the compatibility upfront, as some subsequent errors happening during +vIOMMU operation, such as cache invalidation failures cannot be nicely +escaladated to the guest due to IOMMU specifications. This can lead to +catastrophic failures for the users. + +User applications such as QEMU are expected to import kernel UAPI +headers. Backward compatibility is supported per feature flags. +For example, an older QEMU (with older kernel header) can run on newer +kernel. Newer QEMU (with new kernel header) may refuse to initialize +on an older kernel if new feature flags are not supported by older +kernel. Simply recompiling existing code with newer kernel header should +not be an issue in that only existing flags are used. + +IOMMU vendor driver should report the below features to IOMMU UAPI +consumers (e.g. via VFIO). + +1. IOMMU_NESTING_FEAT_SYSWIDE_PASID
Re: [PATCH v7 7/7] iommu/vt-d: Check UAPI data processed by IOMMU core
On Thu, 13 Aug 2020 11:19:46 +0200 Auger Eric wrote: > Hi Jacob, > > On 7/30/20 2:21 AM, Jacob Pan wrote: > > IOMMU generic layer already does sanity checks UAPI data for version > > match and argsz range under generic information. > > Remove the redundant version check from VT-d driver and check for > > vendor specific data size. > > > > Signed-off-by: Jacob Pan > > > --- > > drivers/iommu/intel/iommu.c | 3 +-- > > drivers/iommu/intel/svm.c | 7 +-- > > 2 files changed, 6 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/iommu/intel/iommu.c > > b/drivers/iommu/intel/iommu.c index 021f62078f52..7e03cca31a0e > > 100644 --- a/drivers/iommu/intel/iommu.c > > +++ b/drivers/iommu/intel/iommu.c > > @@ -5391,8 +5391,7 @@ intel_iommu_sva_invalidate(struct > > iommu_domain *domain, struct device *dev, int ret = 0; > > u64 size = 0; > > > > - if (!inv_info || !dmar_domain || > > - inv_info->version != > > IOMMU_CACHE_INVALIDATE_INFO_VERSION_1) > > + if (!inv_info || !dmar_domain) > > return -EINVAL; > > > > if (!dev || !dev_is_pci(dev)) > > diff --git a/drivers/iommu/intel/svm.c b/drivers/iommu/intel/svm.c > > index 713b3a218483..55ea11e9c0f5 100644 > > --- a/drivers/iommu/intel/svm.c > > +++ b/drivers/iommu/intel/svm.c > > @@ -240,8 +240,11 @@ int intel_svm_bind_gpasid(struct iommu_domain > > *domain, struct device *dev, if (WARN_ON(!iommu) || !data) > > return -EINVAL; > > > > - if (data->version != IOMMU_GPASID_BIND_VERSION_1 || > > - data->format != IOMMU_PASID_FORMAT_INTEL_VTD) > > + if (data->format != IOMMU_PASID_FORMAT_INTEL_VTD) > > + return -EINVAL; > > + > > + /* IOMMU core ensures argsz is more than the start of the > > union */ > > + if (data->argsz < offsetofend(struct > > iommu_gpasid_bind_data, vendor.vtd)) return -EINVAL; > Shouldn't you test the vendor flags here? > intel_pasid_setup_bind_data() only checks valid ones but not ~mask. > Yes, vendor flags should be tested to make sure there is no undefined flags being used. Thanks! > Thanks > > Eric > > > > if (!dev_is_pci(dev)) > > > [Jacob Pan] ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [patch V2 46/46] irqchip: Add IMS (Interrupt Message Storm) driver - NOT FOR MERGING
On Wed, Aug 26, 2020 at 01:17:14PM +0200, Thomas Gleixner wrote: > + * ims_queue_info - Information to create an IMS queue domain > + * @queue_lock: Callback which informs the device driver that > + * an interrupt management operation starts. > + * @queue_sync_unlock: Callback which informs the device driver that an > + * interrupt management operation ends. > + > + * @queue_get_shadow: Callback to retrieve te shadow storage for a MSI > + * entry associated to a queue. The queue is > + * identified by the device struct which is used for > + * allocating interrupts and the msi entry index. > + * > + * @queue_lock() and @queue_sync_unlock() are only called for management > + * operations on a particular interrupt: request, free, enable, disable, > + * affinity setting. These functions are never called from atomic context, > + * like low level interrupt handling code. The purpose of these functions > + * is to signal the device driver the start and end of an operation which > + * affects the IMS queue shadow state. @queue_lock() allows the driver to > + * do preperatory work, e.g. locking. Note, that @queue_lock() has to > + * preserve the sleepable state on return. That means the driver cannot > + * disable preemption and (soft)interrupts in @queue_lock and then undo > + * that operation in @queue_sync_unlock() which restricts the lock types > + * for eventual serialization of these operations to sleepable locks. Of > + * course the driver can disable preemption and (soft)interrupts > + * temporarily for internal work. > + * > + * On @queue_sync_unlock() the driver has to check whether the shadow state > + * changed and issue a command to update the hardware state and wait for > + * the command to complete. If the command fails or times out then the > + * driver has to take care of the resulting mess as this is called from > + * functions which have no return value and none of the callers can deal > + * with the failure. The lock which is used by the driver to protect a > + * operation sequence must obviously not be released before the command > + * completes or fails. Otherwise new operations on the same interrupt line > + * could take place and change the shadow state before the driver was able > + * to compose the command. I haven't looked through everything in detail, but this does look like it is good for the mlx5 devices. Looked like it was only one small update to the set_affinity, so not very disruptive? Thanks, Jason ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [patch V2 24/46] PCI: vmd: Mark VMD irqdomain with DOMAIN_BUS_VMD_MSI
On Wed, Aug 26, 2020 at 01:16:52PM +0200, Thomas Gleixner wrote: > From: Thomas Gleixner > > Devices on the VMD bus use their own MSI irq domain, but it is not > distinguishable from regular PCI/MSI irq domains. This is required > to exclude VMD devices from getting the irq domain pointer set by > interrupt remapping. > > Override the default bus token. > > Signed-off-by: Thomas Gleixner > Acked-by: Bjorn Helgaas > drivers/pci/controller/vmd.c |6 ++ > 1 file changed, 6 insertions(+) > > +++ b/drivers/pci/controller/vmd.c > @@ -579,6 +579,12 @@ static int vmd_enable_domain(struct vmd_ > return -ENODEV; > } > > + /* > + * Override the irq domain bus token so the domain can be distinguished > + * from a regular PCI/MSI domain. > + */ > + irq_domain_update_bus_token(vmd->irq_domain, DOMAIN_BUS_VMD_MSI); > + Having the non-transparent-bridge hold a MSI table and multiplex/de-multiplex IRQs looks like another good use case for something close to pci_subdevice_msi_create_irq_domain()? If each device could have its own internal MSI-X table programmed properly things would work alot better. Disable capture/remap of the MSI range in the NTB. Then each device could have a proper non-multiplexed interrupt delivered to the CPU.. Affinity would work properly, no need to share IRQs (eg vmd_irq() goes away)/etc. Something for the VMD maintainers to think about at least. As I hear more about NTB these days a full MSI scheme for NTB seems interesting to have in the PCI-E core code.. Jason ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[Regression] [PATCH] iommu: Avoid crash in init_no_remapping_devices if iommu is NULL
I noticed a kernel crash when trying to boot a v5.9-rc2 based kernel. The crash reads as: <1>[7.410192] BUG: kernel NULL pointer dereference, address: 0038 <1>[7.410196] #PF: supervisor write access in kernel mode <1>[7.410199] #PF: error_code(0x0002) - not-present page <6>[7.410202] PGD 0 P4D 0 <4>[7.410207] Oops: 0002 [#1] SMP PTI <4>[7.410212] CPU: 1 PID: 1 Comm: swapper/0 Not tainted 5.9.0-devel+ #1 <4>[7.410215] Hardware name: LENOVO 20HGS0TW00/20HGS0TW00, BIOS N1WET46S (1.25s ) 03/30/2018 <4>[7.410224] RIP: 0010:intel_iommu_init+0xed0/0x1136 <4>[7.410229] Code: fe e9 61 02 00 00 bb f4 ff ff ff e9 57 02 00 00 48 63 d1 48 c1 e2 04 48 03 50 20 48 8b 12 48 85 d2 74 0b 48 8b 92 d0 02 00 00 <48> 89 7a 38 ff c1 e9 15 f5 ff ff 48 c7 c7 00 a5 ac a1 49 c7 c7 20 <4>[7.410236] RSP: :b14e40073dd0 EFLAGS: 00010282 <4>[7.410240] RAX: 8d0643731720 RBX: RCX: <4>[7.410244] RDX: RSI: RDI: <4>[7.410247] RBP: b14e40073e90 R08: 0001 R09: 8d0643803700 <4>[7.410250] R10: 8d064262 R11: 0016 R12: 000b <4>[7.410254] R13: 8d064361e650 R14: a2455d80 R15: <4>[7.410258] FS: () GS:8d064748() knlGS: <4>[7.410262] CS: 0010 DS: ES: CR0: 80050033 <4>[7.410266] CR2: 0038 CR3: 00056760a001 CR4: 003706e0 <4>[7.410269] Call Trace: <4>[7.410280] ? call_rcu+0x10e/0x320 <4>[7.410286] ? trace_hardirqs_on+0x2c/0xd0 <4>[7.410291] ? rdinit_setup+0x2c/0x2c <4>[7.410297] ? e820__memblock_setup+0x8b/0x8b <4>[7.410302] pci_iommu_init+0x16/0x3f <4>[7.410307] do_one_initcall+0x46/0x1e4 <4>[7.410313] kernel_init_freeable+0x169/0x1b2 <4>[7.410320] ? rest_init+0x9f/0x9f <4>[7.410325] kernel_init+0xa/0x101 <4>[7.410329] ret_from_fork+0x22/0x30 <4>[7.410333] Modules linked in: <4>[7.410338] CR2: 0038 <4>[7.410344] ---[ end trace 16bcdb1e11668fcd ]--- Full kernel message is attached in kernel.log. I tracked the problem down to the dev_iommu_priv_set call in init_no_remapping_devices. It seems that for one device the dev->iommu member is NULL. An dev_err outputs the device as "pci :00:02.0: DMAR" which is the intel HD 620 graphic adapter in a Lenovo T470s device. The following patch (also attached as intel-iommu.patch) avoids this crash by checking the pointer. diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c index f8177c59d229..2d285a42db3f 100644 --- a/drivers/iommu/intel/iommu.c +++ b/drivers/iommu/intel/iommu.c @@ -4053,7 +4053,8 @@ static void __init init_no_remapping_devices(void) drhd->ignored = 1; for_each_active_dev_scope(drhd->devices, drhd->devices_cnt, i, dev) - dev_iommu_priv_set(dev, DUMMY_DEVICE_DOMAIN_INFO); + if (dev->iommu) + dev_iommu_priv_set(dev, DUMMY_DEVICE_DOMAIN_INFO); } } } I assume the problem might be related to the following commit introduced in v5.9-rc1: commit 01b9d4e21148c89fdbab3d6b3705f9791314b631 Author: Joerg Roedel Date: Thu Jun 25 15:08:25 2020 +0200 iommu/vt-d: Use dev_iommu_priv_get/set() Remove the use of dev->archdata.iommu and use the private per-device pointer provided by IOMMU core code instead. With regards, Torsten Hilbrich -- Dipl.-Inform. Torsten Hilbrich Teamleiter Bereich Entwicklung Abteilung Network & Client Security Divison Public Authorities secunet Security Networks AG Konrad-Zuse-Platz 2-4 81829 München Tel: +49-201-5454-3522 Fax: +49-201-5454-1327 torsten.hilbr...@secunet.com www.secunet.com __ secunet Security Networks AG Sitz: Kurfürstenstraße 58, 45138 Essen, Deutschland Amtsgericht Essen HRB 13615 Vorstand: Axel Deininger (Vors.), Torsten Henn, Dr. Kai Martius, Thomas Pleines Aufsichtsratsvorsitzender: Ralf Wintergerst __ Oops#1 Part1 <6>[0.00] microcode: microcode updated early to revision 0xd6, date = 2020-04-27 <5>[0.00] Linux version 5.9.0-devel+ (thilbrich@ts-6) (gcc (Debian 8.3.0-6) 8.3.0, GNU ld (GNU Binutils for Debian) 2.31.1) #2 SMP Mon Aug 31 09:20:36 CEST 2020 <6>[0.00] Command line: BOOT_IMAGE=/isolinux/bzImage console=tty1 intel_iommu=on,igfx_off loglevel=7 initcall_debug <6>[0.00] x86/fpu: Supporting XSAVE feature 0x001: 'x87 floating point registers' <6>[0.00] x86/fpu: Supporting XSAVE feature 0x002: 'SSE registers' <6>[0.00] x86/fpu: Supporting XSAVE feature 0
[PATCH v5] iommu/mediatek: check 4GB mode by reading infracfg
In previous discussion [1] and [2], we found that it is risky to
use max_pfn or totalram_pages to tell if 4GB mode is enabled.
Check 4GB mode by reading infracfg register, remove the usage
of the un-exported symbol max_pfn.
This is a step towards building mtk_iommu as a kernel module.
[1] https://lore.kernel.org/lkml/20200603161132.2441-1-miles.c...@mediatek.com/
[2] https://lore.kernel.org/lkml/20200604080120.2628-1-miles.c...@mediatek.com/
[3] https://lore.kernel.org/lkml/20200715205120.GA778876@bogus/
Cc: Mike Rapoport
Cc: David Hildenbrand
Cc: Yong Wu
Cc: Yingjoe Chen
Cc: Christoph Hellwig
Cc: Rob Herring
Cc: Matthias Brugger
Signed-off-by: Miles Chen
---
Change since v4
- remove unnecessary data->enable_4GB = false, since it is kzalloc()ed.
Change since v3
- use lore.kernel.org links
- move "change since..." after "---"
Change since v2:
- determine compatible string by m4u_plat
- rebase to next-20200720
- add "---"
Change since v1:
- remove the phandle usage, search for infracfg instead [3]
- use infracfg instead of infracfg_regmap
- move infracfg definitaions to linux/soc/mediatek/infracfg.h
- update enable_4GB only when has_4gb_mode
---
drivers/iommu/mtk_iommu.c | 33 +++
include/linux/soc/mediatek/infracfg.h | 3 +++
2 files changed, 31 insertions(+), 5 deletions(-)
diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index 785b228d39a6..e7b8b2bb08a9 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -3,7 +3,6 @@
* Copyright (c) 2015-2016 MediaTek Inc.
* Author: Yong Wu
*/
-#include
#include
#include
#include
@@ -15,13 +14,16 @@
#include
#include
#include
+#include
#include
#include
#include
#include
#include
+#include
#include
#include
+#include
#include
#include
@@ -640,8 +642,11 @@ static int mtk_iommu_probe(struct platform_device *pdev)
struct resource *res;
resource_size_t ioaddr;
struct component_match *match = NULL;
+ struct regmap *infracfg;
void*protect;
int i, larb_nr, ret;
+ u32 val;
+ char*p;
data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
if (!data)
@@ -655,10 +660,28 @@ static int mtk_iommu_probe(struct platform_device *pdev)
return -ENOMEM;
data->protect_base = ALIGN(virt_to_phys(protect), MTK_PROTECT_PA_ALIGN);
- /* Whether the current dram is over 4GB */
- data->enable_4GB = !!(max_pfn > (BIT_ULL(32) >> PAGE_SHIFT));
- if (!MTK_IOMMU_HAS_FLAG(data->plat_data, HAS_4GB_MODE))
- data->enable_4GB = false;
+ if (MTK_IOMMU_HAS_FLAG(data->plat_data, HAS_4GB_MODE)) {
+ switch (data->plat_data->m4u_plat) {
+ case M4U_MT2712:
+ p = "mediatek,mt2712-infracfg";
+ break;
+ case M4U_MT8173:
+ p = "mediatek,mt8173-infracfg";
+ break;
+ default:
+ p = NULL;
+ }
+
+ infracfg = syscon_regmap_lookup_by_compatible(p);
+
+ if (IS_ERR(infracfg))
+ return PTR_ERR(infracfg);
+
+ ret = regmap_read(infracfg, REG_INFRA_MISC, &val);
+ if (ret)
+ return ret;
+ data->enable_4GB = !!(val & F_DDR_4GB_SUPPORT_EN);
+ }
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
data->base = devm_ioremap_resource(dev, res);
diff --git a/include/linux/soc/mediatek/infracfg.h
b/include/linux/soc/mediatek/infracfg.h
index fd25f0148566..233463d789c6 100644
--- a/include/linux/soc/mediatek/infracfg.h
+++ b/include/linux/soc/mediatek/infracfg.h
@@ -32,6 +32,9 @@
#define MT7622_TOP_AXI_PROT_EN_WB (BIT(2) | BIT(6) | \
BIT(7) | BIT(8))
+#define REG_INFRA_MISC 0xf00
+#define F_DDR_4GB_SUPPORT_EN BIT(13)
+
int mtk_infracfg_set_bus_protection(struct regmap *infracfg, u32 mask,
bool reg_update);
int mtk_infracfg_clear_bus_protection(struct regmap *infracfg, u32 mask,
--
2.18.0
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v6 0/2] make dma_alloc_coherent NUMA-aware by per-NUMA CMA
This is on my todo list to be applied this week. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH v6 0/2] make dma_alloc_coherent NUMA-aware by per-NUMA CMA
> -Original Message- > From: linux-kernel-ow...@vger.kernel.org > [mailto:linux-kernel-ow...@vger.kernel.org] On Behalf Of Christoph Hellwig > Sent: Friday, August 21, 2020 6:19 PM > To: Song Bao Hua (Barry Song) > Cc: h...@lst.de; m.szyprow...@samsung.com; robin.mur...@arm.com; > w...@kernel.org; ganapatrao.kulka...@cavium.com; > catalin.mari...@arm.com; iommu@lists.linux-foundation.org; Linuxarm > ; linux-arm-ker...@lists.infradead.org; > linux-ker...@vger.kernel.org; huangdaode > Subject: Re: [PATCH v6 0/2] make dma_alloc_coherent NUMA-aware by > per-NUMA CMA > > FYI, as of the last one I'm fine now, bit I really need an ACK from > the arm64 maintainers. Hi Christoph, For the changes in arch/arm64, Will gave his ack here: https://lore.kernel.org/linux-iommu/20200821090116.GB20255@willie-the-truck/ and the patchset has been refined to v8 https://lore.kernel.org/linux-iommu/20200823230309.28980-1-song.bao@hisilicon.com/ with one additional patch to remove magic number: [PATCH v8 3/3] mm: cma: use CMA_MAX_NAME to define the length of cma name array https://lore.kernel.org/linux-iommu/20200823230309.28980-4-song.bao@hisilicon.com/ Hopefully, you didn't miss it:-) Does the new one need an Ack from Linux-mm maintainer? Thanks Barry ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [patch V2 00/46] x86, PCI, XEN, genirq ...: Prepare for device MSI
Hi Thomas, On 2020/8/31 15:10, Thomas Gleixner wrote: On Mon, Aug 31 2020 at 08:51, Lu Baolu wrote: On 8/26/20 7:16 PM, Thomas Gleixner wrote: This is the second version of providing a base to support device MSI (non PCI based) and on top of that support for IMS (Interrupt Message Storm) based devices in a halfways architecture independent way. After applying this patch series, the dmar_alloc_hwirq() helper doesn't work anymore during boot. This causes the IOMMU driver to fail to register the DMA fault handler and abort the IOMMU probe processing. Is this a known issue? See replies to patch 15/46 or pull the git tree. It has the issue fixed. Ah! Yes. Sorry for the noise. Beset regards, baolu ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 5/5] powerpc: use the generic dma_ops_bypass mode
On 8/31/20 8:40 AM, Christoph Hellwig wrote:
> On Sun, Aug 30, 2020 at 11:04:21AM +0200, Cédric Le Goater wrote:
>> Hello,
>>
>> On 7/8/20 5:24 PM, Christoph Hellwig wrote:
>>> Use the DMA API bypass mechanism for direct window mappings. This uses
>>> common code and speed up the direct mapping case by avoiding indirect
>>> calls just when not using dma ops at all. It also fixes a problem where
>>> the sync_* methods were using the bypass check for DMA allocations, but
>>> those are part of the streaming ops.
>>>
>>> Note that this patch loses the DMA_ATTR_WEAK_ORDERING override, which
>>> has never been well defined, as is only used by a few drivers, which
>>> IIRC never showed up in the typical Cell blade setups that are affected
>>> by the ordering workaround.
>>>
>>> Fixes: efd176a04bef ("powerpc/pseries/dma: Allow SWIOTLB")
>>> Signed-off-by: Christoph Hellwig
>>> ---
>>> arch/powerpc/Kconfig | 1 +
>>> arch/powerpc/include/asm/device.h | 5 --
>>> arch/powerpc/kernel/dma-iommu.c | 90 ---
>>> 3 files changed, 10 insertions(+), 86 deletions(-)
>>
>> I am seeing corruptions on a couple of POWER9 systems (boston) when
>> stressed with IO. stress-ng gives some results but I have first seen
>> it when compiling the kernel in a guest and this is still the best way
>> to raise the issue.
>>
>> These systems have of a SAS Adaptec controller :
>>
>> 0003:01:00.0 Serial Attached SCSI controller: Adaptec Series 8 12G
>> SAS/PCIe 3 (rev 01)
>>
>> When the failure occurs, the POWERPC EEH interrupt fires and dumps
>> lowlevel PHB4 registers among which :
>>
>> [ 2179.251069490,3] PHB#0003[0:3]: phbErrorStatus =
>> 0280
>> [ 2179.251117476,3] PHB#0003[0:3]: phbFirstErrorStatus =
>> 0200
>>
>> The bits raised identify a PPC 'TCE' error, which means it is related
>> to DMAs. See below for more details.
>>
>>
>> Reverting this patch "fixes" the issue but it is probably else where,
>> in some other layers or in the aacraid driver. How should I proceed
>> to get more information ?
>
> The aacraid DMA masks look like a mess. Can you try the hack
> below and see it it helps?
No effect. The system crashes the same. But Alexey spotted some issue
with swiotlb.
C.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [patch V2 00/46] x86, PCI, XEN, genirq ...: Prepare for device MSI
On Mon, Aug 31 2020 at 08:51, Lu Baolu wrote: > On 8/26/20 7:16 PM, Thomas Gleixner wrote: >> This is the second version of providing a base to support device MSI (non >> PCI based) and on top of that support for IMS (Interrupt Message Storm) >> based devices in a halfways architecture independent way. > > After applying this patch series, the dmar_alloc_hwirq() helper doesn't > work anymore during boot. This causes the IOMMU driver to fail to > register the DMA fault handler and abort the IOMMU probe processing. > Is this a known issue? See replies to patch 15/46 or pull the git tree. It has the issue fixed. Thanks, tglx ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v1] iommu/vt-d: Move intel_iommu_ops to header file
Hi Andy, On 2020/8/31 14:30, Andy Shevchenko wrote: On Sat, Aug 29, 2020 at 07:58:46AM +0100, Christoph Hellwig wrote: On Fri, Aug 28, 2020 at 07:05:02PM +0300, Andy Shevchenko wrote: Compiler is not happy about hidden declaration of intel_iommu_ops. .../drivers/iommu/intel/iommu.c:414:24: warning: symbol 'intel_iommu_ops' was not declared. Should it be static? Move declaration to header file to make compiler happy. What about a factoring out a helper that does iommu_device_sysfs_add + iommu_device_set_ops + iommu_device_register and then mark intel_iommu_ops static? I am okay with this proposal, but I think the better if IOMMU folks can answer to this before I'm going to invest time into it. Thanks for your patch! I am okay with Christoph's proposal as well. Best regards, baolu ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
