From: Jean-Philippe Brucker <Jean-Philippe.Brucker-5wv7dgnIgG8@public.gmane.org>
To: Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org>
Cc: joro-zLv9SwRftAIdnm+yROfE0A@public.gmane.org,
will.deacon-5wv7dgnIgG8@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
Lorenzo.Pieralisi-5wv7dgnIgG8@public.gmane.org
Subject: Re: [PATCH v2 6/7] iommu/arm-smmu: Finish off SMMUv3 default domain support
Date: Mon, 6 Jun 2016 16:47:12 +0100 [thread overview]
Message-ID: <20160606154712.GA9864@e106794-lin.localdomain> (raw)
In-Reply-To: <aabcdd814b93b2b6a902b189f6911f929a29b64f.1464966939.git.robin.murphy-5wv7dgnIgG8@public.gmane.org>
Hi Robin,
I quite like this change, the result looks pretty clean. I rebased my
current work and didn't notice any major issue. Some nits below.
On Fri, Jun 03, 2016 at 06:15:41PM +0100, Robin Murphy wrote:
> The driver's current reliance on attaching/detaching an entire group
> for the first device it sees is at odds with the IOMMU core's initial
> construction of a group by adding each device and attaching it to the
> default domain in turn. As it turns out, we can happily do away with the
> whole palaver by simply letting each device be in charge of its own
> stream ID/stream table entry, and reducing the problem of tracking
> duplicate IDs and domains down to "Is the STE already associated with
> the appropriate context?", which is easily done by just looking at the
> stream table itself.
>
> With an of_xlate() callback in place, devices attached to default
> domains will now get appropriate DMA ops installed, so make sure we
> enable translation again to stop them getting horribly confused - this
> reverts the SMMUv3 portion of cbf8277ef456 ("iommu/arm-smmu: Treat
> IOMMU_DOMAIN_DMA as bypass for now")
>
> Signed-off-by: Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org>
> ---
...
>
> -static int arm_smmu_install_ste_for_group(struct arm_smmu_group *smmu_group)
> +static void arm_smmu_install_ste(struct arm_smmu_master_data *master,
> + struct arm_smmu_domain *smmu_domain)
(Second line is misaligned here)
> {
> - int i;
> - struct arm_smmu_domain *smmu_domain = smmu_group->domain;
> - struct arm_smmu_strtab_ent *ste = &smmu_group->ste;
> - struct arm_smmu_device *smmu = smmu_group->smmu;
> + struct arm_smmu_device *smmu = master->smmu;
> + struct arm_smmu_strtab_ent *ste = &master->ste;
> + __le64 *step = arm_smmu_get_step_for_sid(smmu, master->sid);
>
> if (smmu_domain->stage == ARM_SMMU_DOMAIN_S1) {
> ste->s1_cfg = &smmu_domain->s1_cfg;
> @@ -1634,42 +1628,16 @@ static int arm_smmu_install_ste_for_group(struct arm_smmu_group *smmu_group)
> ste->s2_cfg = &smmu_domain->s2_cfg;
> }
>
> - for (i = 0; i < smmu_group->num_sids; ++i) {
> - u32 sid = smmu_group->sids[i];
> - __le64 *step = arm_smmu_get_step_for_sid(smmu, sid);
> -
> - arm_smmu_write_strtab_ent(smmu, sid, step, ste);
> - }
> -
> - return 0;
> -}
> -
> -static void arm_smmu_detach_dev(struct device *dev)
> -{
> - struct arm_smmu_group *smmu_group = arm_smmu_group_get(dev);
> -
> - smmu_group->ste.bypass = true;
> - if (arm_smmu_install_ste_for_group(smmu_group) < 0)
> - dev_warn(dev, "failed to install bypass STE\n");
> -
> - smmu_group->domain = NULL;
> + arm_smmu_write_strtab_ent(smmu, master->sid, step, ste);
> }
>
> static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
> {
> int ret = 0;
> - struct arm_smmu_device *smmu;
> struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
> - struct arm_smmu_group *smmu_group = arm_smmu_group_get(dev);
> + struct arm_smmu_master_data *master = dev->archdata.iommu;
(calling this member 'master' or 'smmu_master' consistently, instead of
'data', would make the driver easier to grep)
> + struct arm_smmu_device *smmu = master->smmu;
>
> - if (!smmu_group)
> - return -ENOENT;
> -
> - /* Already attached to a different domain? */
> - if (smmu_group->domain && smmu_group->domain != smmu_domain)
> - arm_smmu_detach_dev(dev);
> -
> - smmu = smmu_group->smmu;
> mutex_lock(&smmu_domain->init_mutex);
>
> if (!smmu_domain->smmu) {
> @@ -1688,21 +1656,9 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
> goto out_unlock;
> }
>
> - /* Group already attached to this domain? */
> - if (smmu_group->domain)
> - goto out_unlock;
> + master->ste.bypass = false;
Should also set master->ste.valid = true. It worked out of the box
during my first test, because master is allocated with kmalloc and
initialised with garbage. Could we also use kzalloc, in the of_xlate
patch?
Thanks,
Jean-Philippe
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
next prev parent reply other threads:[~2016-06-06 15:47 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-06-03 17:15 [PATCH v2 0/7] Generic DT bindings for PCI IOMMUs and ARM SMMUv3 Robin Murphy
[not found] ` <cover.1464966939.git.robin.murphy-5wv7dgnIgG8@public.gmane.org>
2016-06-03 17:15 ` [PATCH v2 1/7] iommu/of: Respect disabled IOMMUs Robin Murphy
[not found] ` <aca0b44206c87f6cb75d156a53e08aa968981119.1464966939.git.robin.murphy-5wv7dgnIgG8@public.gmane.org>
2016-06-14 14:11 ` Will Deacon
[not found] ` <20160614141148.GH19407-5wv7dgnIgG8@public.gmane.org>
2016-06-14 15:04 ` Robin Murphy
2016-06-03 17:15 ` [PATCH v2 2/7] Docs: dt: add PCI IOMMU map bindings Robin Murphy
[not found] ` <3a7e47d7b8839ff079568137b5b1b438cfbb44ac.1464966939.git.robin.murphy-5wv7dgnIgG8@public.gmane.org>
2016-06-14 14:16 ` Will Deacon
2016-06-03 17:15 ` [PATCH v2 3/7] of/irq: Break out msi-map lookup (again) Robin Murphy
[not found] ` <2cbffa9a341edfd10114994f6486f6b08d0c390c.1464966939.git.robin.murphy-5wv7dgnIgG8@public.gmane.org>
2016-06-04 8:10 ` Marc Zyngier
2016-06-14 14:37 ` Will Deacon
[not found] ` <20160614143750.GJ19407-5wv7dgnIgG8@public.gmane.org>
2016-06-14 18:12 ` Robin Murphy
[not found] ` <5760491E.7060104-5wv7dgnIgG8@public.gmane.org>
2016-06-14 18:20 ` Will Deacon
2016-06-14 17:01 ` Rob Herring
[not found] ` <CAL_JsqJ4km0dH2PxHma9-cPwC20WR0ejn_14UeV0vBWJ+XLBBg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2016-06-15 10:16 ` Robin Murphy
2016-06-03 17:15 ` [PATCH v2 4/7] iommu/of: Handle iommu-map property for PCI Robin Murphy
[not found] ` <69952eda726c370ed6e5739bdd2e32cdc4466bfb.1464966939.git.robin.murphy-5wv7dgnIgG8@public.gmane.org>
2016-06-14 14:45 ` Will Deacon
[not found] ` <20160614144559.GK19407-5wv7dgnIgG8@public.gmane.org>
2016-06-15 11:21 ` Robin Murphy
2016-06-03 17:15 ` [PATCH v2 5/7] iommu/arm-smmu: Implement of_xlate() for SMMUv3 Robin Murphy
[not found] ` <55aa94e099f00fd586077c45d4c4fd1c010981d9.1464966939.git.robin.murphy-5wv7dgnIgG8@public.gmane.org>
2016-06-14 15:07 ` Will Deacon
[not found] ` <20160614150757.GB16531-5wv7dgnIgG8@public.gmane.org>
2016-06-14 16:11 ` Robin Murphy
2016-06-03 17:15 ` [PATCH v2 6/7] iommu/arm-smmu: Finish off SMMUv3 default domain support Robin Murphy
[not found] ` <aabcdd814b93b2b6a902b189f6911f929a29b64f.1464966939.git.robin.murphy-5wv7dgnIgG8@public.gmane.org>
2016-06-06 15:47 ` Jean-Philippe Brucker [this message]
[not found] ` <20160606154712.GA9864-lfHAr0XZR/FyySVAYrpuPyZi+YwRKgec@public.gmane.org>
2016-06-06 17:22 ` Robin Murphy
2016-06-14 15:59 ` Will Deacon
2016-06-03 17:15 ` [PATCH v2 7/7] iommu/arm-smmu: Support non-PCI devices with SMMUv3 Robin Murphy
[not found] ` <42a8a71932f766d70ea9dcae5d11d5f33dcc3652.1464966939.git.robin.murphy-5wv7dgnIgG8@public.gmane.org>
2016-06-14 15:16 ` Will Deacon
[not found] ` <20160614151642.GC16531-5wv7dgnIgG8@public.gmane.org>
2016-06-15 1:22 ` Leizhen (ThunderTown)
[not found] ` <5760ADC6.8000803-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
2016-06-17 1:54 ` Leizhen (ThunderTown)
[not found] ` <57635843.6000906-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
2016-06-17 9:14 ` Robin Murphy
[not found] ` <5763BF69.7010205-5wv7dgnIgG8@public.gmane.org>
2016-06-21 8:36 ` Leizhen (ThunderTown)
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160606154712.GA9864@e106794-lin.localdomain \
--to=jean-philippe.brucker-5wv7dgnigg8@public.gmane.org \
--cc=Lorenzo.Pieralisi-5wv7dgnIgG8@public.gmane.org \
--cc=devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
--cc=iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org \
--cc=joro-zLv9SwRftAIdnm+yROfE0A@public.gmane.org \
--cc=linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org \
--cc=robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
--cc=robin.murphy-5wv7dgnIgG8@public.gmane.org \
--cc=will.deacon-5wv7dgnIgG8@public.gmane.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).