* [PATCH 0/2] ARM SMMU fixes
@ 2014-02-28 15:37 Laurent Pinchart
2014-02-28 15:37 ` [PATCH 1/2] iommu/arm-smmu: Replace list walk with platform driver data Laurent Pinchart
` (2 more replies)
0 siblings, 3 replies; 13+ messages in thread
From: Laurent Pinchart @ 2014-02-28 15:37 UTC (permalink / raw)
To: linux-arm-kernel
Hello Will,
I've studied your arm-smmu driver as a base to write a Renesas IOMMU driver
and found two small issues. Here are patches to fix them. Please bear with me
if my understanding was incorrect and the patches wrong :-)
Laurent Pinchart (2):
iommu/arm-smmu: Replace list walk with platform driver data
iommu/arm-smmu: Return 0 on unmap failure
drivers/iommu/arm-smmu.c | 17 +++++------------
1 file changed, 5 insertions(+), 12 deletions(-)
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH 1/2] iommu/arm-smmu: Replace list walk with platform driver data 2014-02-28 15:37 [PATCH 0/2] ARM SMMU fixes Laurent Pinchart @ 2014-02-28 15:37 ` Laurent Pinchart 2014-02-28 16:38 ` Will Deacon 2014-02-28 15:37 ` [PATCH 2/2] iommu/arm-smmu: Return 0 on unmap failure Laurent Pinchart 2014-04-02 23:52 ` [PATCH 0/2] ARM SMMU fixes Laurent Pinchart 2 siblings, 1 reply; 13+ messages in thread From: Laurent Pinchart @ 2014-02-28 15:37 UTC (permalink / raw) To: linux-arm-kernel Instead of walking the list of registered SMMU devices at remove time to locate the device being removed, set platform driver data at probe time to point to the SMMU and retrieve the pointer at remove time. Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> --- drivers/iommu/arm-smmu.c | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c index 1d9ab39..c33a310 100644 --- a/drivers/iommu/arm-smmu.c +++ b/drivers/iommu/arm-smmu.c @@ -1912,6 +1912,8 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev) list_add(&smmu->list, &arm_smmu_devices); spin_unlock(&arm_smmu_devices_lock); + platform_set_drvdata(pdev, smmu); + arm_smmu_device_reset(smmu); return 0; @@ -1937,22 +1939,13 @@ static int arm_smmu_device_remove(struct platform_device *pdev) { int i; struct device *dev = &pdev->dev; - struct arm_smmu_device *curr, *smmu = NULL; + struct arm_smmu_device *smmu = platform_get_drvdata(pdev); struct rb_node *node; spin_lock(&arm_smmu_devices_lock); - list_for_each_entry(curr, &arm_smmu_devices, list) { - if (curr->dev == dev) { - smmu = curr; - list_del(&smmu->list); - break; - } - } + list_del(&smmu->list); spin_unlock(&arm_smmu_devices_lock); - if (!smmu) - return -ENODEV; - if (smmu->parent_of_node) of_node_put(smmu->parent_of_node); -- 1.8.3.2 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 1/2] iommu/arm-smmu: Replace list walk with platform driver data 2014-02-28 15:37 ` [PATCH 1/2] iommu/arm-smmu: Replace list walk with platform driver data Laurent Pinchart @ 2014-02-28 16:38 ` Will Deacon 2014-03-02 17:59 ` Laurent Pinchart 0 siblings, 1 reply; 13+ messages in thread From: Will Deacon @ 2014-02-28 16:38 UTC (permalink / raw) To: linux-arm-kernel Hi Laurent, On Fri, Feb 28, 2014 at 03:37:09PM +0000, Laurent Pinchart wrote: > Instead of walking the list of registered SMMU devices at remove time to > locate the device being removed, set platform driver data at probe time > to point to the SMMU and retrieve the pointer at remove time. What does this gain us other than a slight code shrinkage? If we could get rid of the arm_smmu_devices list, then I'd be more excited by this change! Will ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 1/2] iommu/arm-smmu: Replace list walk with platform driver data 2014-02-28 16:38 ` Will Deacon @ 2014-03-02 17:59 ` Laurent Pinchart 0 siblings, 0 replies; 13+ messages in thread From: Laurent Pinchart @ 2014-03-02 17:59 UTC (permalink / raw) To: linux-arm-kernel Hi Will, On Friday 28 February 2014 16:38:37 Will Deacon wrote: > Hi Laurent, > > On Fri, Feb 28, 2014 at 03:37:09PM +0000, Laurent Pinchart wrote: > > Instead of walking the list of registered SMMU devices at remove time to > > locate the device being removed, set platform driver data at probe time > > to point to the SMMU and retrieve the pointer at remove time. > > What does this gain us other than a slight code shrinkage? Just a slight code shrinkage. That's better than nothing, isn't it ? :-) > If we could get rid of the arm_smmu_devices list, then I'd be more excited > by this change! Associating devices with IOMMUs is something that should be performed by the IOMMU core. I might give this a try, but I'll be busy with other tasks for the next couple of weeks. -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/2] iommu/arm-smmu: Return 0 on unmap failure 2014-02-28 15:37 [PATCH 0/2] ARM SMMU fixes Laurent Pinchart 2014-02-28 15:37 ` [PATCH 1/2] iommu/arm-smmu: Replace list walk with platform driver data Laurent Pinchart @ 2014-02-28 15:37 ` Laurent Pinchart 2014-02-28 16:39 ` Will Deacon 2014-04-02 23:52 ` [PATCH 0/2] ARM SMMU fixes Laurent Pinchart 2 siblings, 1 reply; 13+ messages in thread From: Laurent Pinchart @ 2014-02-28 15:37 UTC (permalink / raw) To: linux-arm-kernel The IOMMU core expects the unmap operation to return the number of bytes that have been unmapped or 0 on failure, a negative return value being treated like a number of bytes. Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> --- drivers/iommu/arm-smmu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c index c33a310..e7528f0 100644 --- a/drivers/iommu/arm-smmu.c +++ b/drivers/iommu/arm-smmu.c @@ -1464,7 +1464,7 @@ static size_t arm_smmu_unmap(struct iommu_domain *domain, unsigned long iova, ret = arm_smmu_handle_mapping(smmu_domain, iova, 0, size, 0); arm_smmu_tlb_inv_context(&smmu_domain->root_cfg); - return ret ? ret : size; + return ret ? 0 : size; } static phys_addr_t arm_smmu_iova_to_phys(struct iommu_domain *domain, -- 1.8.3.2 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 2/2] iommu/arm-smmu: Return 0 on unmap failure 2014-02-28 15:37 ` [PATCH 2/2] iommu/arm-smmu: Return 0 on unmap failure Laurent Pinchart @ 2014-02-28 16:39 ` Will Deacon 0 siblings, 0 replies; 13+ messages in thread From: Will Deacon @ 2014-02-28 16:39 UTC (permalink / raw) To: linux-arm-kernel On Fri, Feb 28, 2014 at 03:37:10PM +0000, Laurent Pinchart wrote: > The IOMMU core expects the unmap operation to return the number of bytes > that have been unmapped or 0 on failure, a negative return value being > treated like a number of bytes. Makes sense, thanks. It's a pity we can't propagate the error code. Will > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> > --- > drivers/iommu/arm-smmu.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c > index c33a310..e7528f0 100644 > --- a/drivers/iommu/arm-smmu.c > +++ b/drivers/iommu/arm-smmu.c > @@ -1464,7 +1464,7 @@ static size_t arm_smmu_unmap(struct iommu_domain *domain, unsigned long iova, > > ret = arm_smmu_handle_mapping(smmu_domain, iova, 0, size, 0); > arm_smmu_tlb_inv_context(&smmu_domain->root_cfg); > - return ret ? ret : size; > + return ret ? 0 : size; > } > > static phys_addr_t arm_smmu_iova_to_phys(struct iommu_domain *domain, > -- > 1.8.3.2 > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel at lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 0/2] ARM SMMU fixes 2014-02-28 15:37 [PATCH 0/2] ARM SMMU fixes Laurent Pinchart 2014-02-28 15:37 ` [PATCH 1/2] iommu/arm-smmu: Replace list walk with platform driver data Laurent Pinchart 2014-02-28 15:37 ` [PATCH 2/2] iommu/arm-smmu: Return 0 on unmap failure Laurent Pinchart @ 2014-04-02 23:52 ` Laurent Pinchart 2014-04-08 13:41 ` Laurent Pinchart 2 siblings, 1 reply; 13+ messages in thread From: Laurent Pinchart @ 2014-04-02 23:52 UTC (permalink / raw) To: linux-arm-kernel Hi Will, On Friday 28 February 2014 16:37:08 Laurent Pinchart wrote: > Hello Will, > > I've studied your arm-smmu driver as a base to write a Renesas IOMMU driver > and found two small issues. Here are patches to fix them. Please bear with > me if my understanding was incorrect and the patches wrong :-) > > Laurent Pinchart (2): > iommu/arm-smmu: Replace list walk with platform driver data > iommu/arm-smmu: Return 0 on unmap failure > > drivers/iommu/arm-smmu.c | 17 +++++------------ > 1 file changed, 5 insertions(+), 12 deletions(-) Do you plan to take these patches (or at least patch 2/2) in your tree ? I can send a pull request to Joerg if you give me your acked-by. -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 0/2] ARM SMMU fixes 2014-04-02 23:52 ` [PATCH 0/2] ARM SMMU fixes Laurent Pinchart @ 2014-04-08 13:41 ` Laurent Pinchart 2014-04-08 13:57 ` Marc Zyngier 0 siblings, 1 reply; 13+ messages in thread From: Laurent Pinchart @ 2014-04-08 13:41 UTC (permalink / raw) To: linux-arm-kernel I've obviously forgotten that Will was away for a month. CC'ing Marc Zyngier. On Thursday 03 April 2014 01:52:55 Laurent Pinchart wrote: > On Friday 28 February 2014 16:37:08 Laurent Pinchart wrote: > > Hello Will, > > > > I've studied your arm-smmu driver as a base to write a Renesas IOMMU > > driver and found two small issues. Here are patches to fix them. Please > > bear with me if my understanding was incorrect and the patches wrong :-) > > > > Laurent Pinchart (2): > > iommu/arm-smmu: Replace list walk with platform driver data > > iommu/arm-smmu: Return 0 on unmap failure > > > > drivers/iommu/arm-smmu.c | 17 +++++------------ > > 1 file changed, 5 insertions(+), 12 deletions(-) > > Do you plan to take these patches (or at least patch 2/2) in your tree ? I > can send a pull request to Joerg if you give me your acked-by. Marc, would you like to handle this, or would you prefer to wait until Will comes back ? -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 0/2] ARM SMMU fixes 2014-04-08 13:41 ` Laurent Pinchart @ 2014-04-08 13:57 ` Marc Zyngier 2014-04-14 16:58 ` Will Deacon 0 siblings, 1 reply; 13+ messages in thread From: Marc Zyngier @ 2014-04-08 13:57 UTC (permalink / raw) To: linux-arm-kernel On 08/04/14 14:41, Laurent Pinchart wrote: > I've obviously forgotten that Will was away for a month. CC'ing Marc Zyngier. > > On Thursday 03 April 2014 01:52:55 Laurent Pinchart wrote: >> On Friday 28 February 2014 16:37:08 Laurent Pinchart wrote: >>> Hello Will, >>> >>> I've studied your arm-smmu driver as a base to write a Renesas IOMMU >>> driver and found two small issues. Here are patches to fix them. Please >>> bear with me if my understanding was incorrect and the patches wrong :-) >>> >>> Laurent Pinchart (2): >>> iommu/arm-smmu: Replace list walk with platform driver data >>> iommu/arm-smmu: Return 0 on unmap failure >>> >>> drivers/iommu/arm-smmu.c | 17 +++++------------ >>> 1 file changed, 5 insertions(+), 12 deletions(-) >> >> Do you plan to take these patches (or at least patch 2/2) in your tree ? I >> can send a pull request to Joerg if you give me your acked-by. > > Marc, would you like to handle this, or would you prefer to wait until Will > comes back ? Hi Laurent, Yup, I'll have a look and stash them in a temp tree. Given that Will will be back in about a week, he will have the final say. Thanks, M. -- Jazz is not dead. It just smells funny... ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 0/2] ARM SMMU fixes 2014-04-08 13:57 ` Marc Zyngier @ 2014-04-14 16:58 ` Will Deacon 2014-04-15 15:55 ` Laurent Pinchart 0 siblings, 1 reply; 13+ messages in thread From: Will Deacon @ 2014-04-14 16:58 UTC (permalink / raw) To: linux-arm-kernel On Tue, Apr 08, 2014 at 02:57:43PM +0100, Marc Zyngier wrote: > On 08/04/14 14:41, Laurent Pinchart wrote: > > I've obviously forgotten that Will was away for a month. CC'ing Marc Zyngier. > > > > On Thursday 03 April 2014 01:52:55 Laurent Pinchart wrote: > >> On Friday 28 February 2014 16:37:08 Laurent Pinchart wrote: > >>> Hello Will, > >>> > >>> I've studied your arm-smmu driver as a base to write a Renesas IOMMU > >>> driver and found two small issues. Here are patches to fix them. Please > >>> bear with me if my understanding was incorrect and the patches wrong :-) > >>> > >>> Laurent Pinchart (2): > >>> iommu/arm-smmu: Replace list walk with platform driver data > >>> iommu/arm-smmu: Return 0 on unmap failure > >>> > >>> drivers/iommu/arm-smmu.c | 17 +++++------------ > >>> 1 file changed, 5 insertions(+), 12 deletions(-) > >> > >> Do you plan to take these patches (or at least patch 2/2) in your tree ? I > >> can send a pull request to Joerg if you give me your acked-by. > > > > Marc, would you like to handle this, or would you prefer to wait until Will > > comes back ? > > Hi Laurent, > > Yup, I'll have a look and stash them in a temp tree. Given that Will > will be back in about a week, he will have the final say. I've already got the fix queued ("Return 0 on unmap failure") and plan to send it to Joerg this week. I think the other patch doesn't really add anything to the driver :) Will ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 0/2] ARM SMMU fixes 2014-04-14 16:58 ` Will Deacon @ 2014-04-15 15:55 ` Laurent Pinchart 2014-04-16 14:22 ` Will Deacon 0 siblings, 1 reply; 13+ messages in thread From: Laurent Pinchart @ 2014-04-15 15:55 UTC (permalink / raw) To: linux-arm-kernel Hi Will, On Monday 14 April 2014 17:58:58 Will Deacon wrote: > On Tue, Apr 08, 2014 at 02:57:43PM +0100, Marc Zyngier wrote: > > On 08/04/14 14:41, Laurent Pinchart wrote: > > > I've obviously forgotten that Will was away for a month. CC'ing Marc > > > Zyngier. > > > > > > On Thursday 03 April 2014 01:52:55 Laurent Pinchart wrote: > > >> On Friday 28 February 2014 16:37:08 Laurent Pinchart wrote: > > >>> Hello Will, > > >>> > > >>> I've studied your arm-smmu driver as a base to write a Renesas IOMMU > > >>> driver and found two small issues. Here are patches to fix them. > > >>> Please bear with me if my understanding was incorrect and the patches > > >>> wrong :-) > > >>> > > >>> Laurent Pinchart (2): > > >>> iommu/arm-smmu: Replace list walk with platform driver data > > >>> iommu/arm-smmu: Return 0 on unmap failure > > >>> > > >>> drivers/iommu/arm-smmu.c | 17 +++++------------ > > >>> 1 file changed, 5 insertions(+), 12 deletions(-) > > >> > > >> Do you plan to take these patches (or at least patch 2/2) in your tree > > >> ? I can send a pull request to Joerg if you give me your acked-by. > > > > > > Marc, would you like to handle this, or would you prefer to wait until > > > Will comes back ? > > > > Hi Laurent, > > > > Yup, I'll have a look and stash them in a temp tree. Given that Will > > will be back in about a week, he will have the final say. > > I've already got the fix queued ("Return 0 on unmap failure") and plan to > send it to Joerg this week. Thank you. > I think the other patch doesn't really add anything to the driver :) Fair enough, it's your driver, so the decision is yours :-) On a different but related topic, I've written an ipmmu-vmsa.c driver for a Renesas IOMMU. The IP core has custom registers but uses VMSA-compatible page tables. What would you think about sharing the page table management code between the two drivers ? The biggest difference between the two implementations is that I've hardcoded the long descriptor format, while you have reused more system MMU macros that make the arm-smmu driver use 2 or 3 levels of page tables depending on whether LPAE is disabled or enabled. -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 0/2] ARM SMMU fixes 2014-04-15 15:55 ` Laurent Pinchart @ 2014-04-16 14:22 ` Will Deacon 2014-04-16 14:25 ` Laurent Pinchart 0 siblings, 1 reply; 13+ messages in thread From: Will Deacon @ 2014-04-16 14:22 UTC (permalink / raw) To: linux-arm-kernel On Tue, Apr 15, 2014 at 04:55:07PM +0100, Laurent Pinchart wrote: > Hi Will, Hi Laurent, > On a different but related topic, I've written an ipmmu-vmsa.c driver for a > Renesas IOMMU. The IP core has custom registers but uses VMSA-compatible page > tables. What would you think about sharing the page table management code > between the two drivers ? The biggest difference between the two > implementations is that I've hardcoded the long descriptor format, while you > have reused more system MMU macros that make the arm-smmu driver use 2 or 3 > levels of page tables depending on whether LPAE is disabled or enabled. Actually, my driver also only supports the long-descriptor format, so the code should be shareable. Things I support that you might have issues with are: - Stage-1 and Stage-2 formats (although long-descriptor only) - 4k and 64k pages (the latter for arm64 only) - Contiguous pte hints Do you have support for section mappings in your driver? That is something I'd be keen to add for the ARM SMMU, but it really complicates the code. Will ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 0/2] ARM SMMU fixes 2014-04-16 14:22 ` Will Deacon @ 2014-04-16 14:25 ` Laurent Pinchart 0 siblings, 0 replies; 13+ messages in thread From: Laurent Pinchart @ 2014-04-16 14:25 UTC (permalink / raw) To: linux-arm-kernel Hi Will, On Wednesday 16 April 2014 15:22:06 Will Deacon wrote: > On Tue, Apr 15, 2014 at 04:55:07PM +0100, Laurent Pinchart wrote: > > Hi Will, > > Hi Laurent, > > > On a different but related topic, I've written an ipmmu-vmsa.c driver for > > a Renesas IOMMU. The IP core has custom registers but uses VMSA-compatible > > page tables. What would you think about sharing the page table management > > code between the two drivers ? The biggest difference between the two > > implementations is that I've hardcoded the long descriptor format, while > > you have reused more system MMU macros that make the arm-smmu driver use > > 2 or 3 levels of page tables depending on whether LPAE is disabled or > > enabled. > > Actually, my driver also only supports the long-descriptor format, so the > code should be shareable. Things I support that you might have issues with > are: > > - Stage-1 and Stage-2 formats (although long-descriptor only) > - 4k and 64k pages (the latter for arm64 only) > - Contiguous pte hints > > Do you have support for section mappings in your driver? That is something > I'd be keen to add for the ARM SMMU, but it really complicates the code. I have pending patches for that, I still need to clean them up. I'll try to post them in the next few days and I'll CC you. Discussing the implementation will be easier with the code, so I propose resuming this thread in response to the patches. -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2014-04-16 14:25 UTC | newest] Thread overview: 13+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-02-28 15:37 [PATCH 0/2] ARM SMMU fixes Laurent Pinchart 2014-02-28 15:37 ` [PATCH 1/2] iommu/arm-smmu: Replace list walk with platform driver data Laurent Pinchart 2014-02-28 16:38 ` Will Deacon 2014-03-02 17:59 ` Laurent Pinchart 2014-02-28 15:37 ` [PATCH 2/2] iommu/arm-smmu: Return 0 on unmap failure Laurent Pinchart 2014-02-28 16:39 ` Will Deacon 2014-04-02 23:52 ` [PATCH 0/2] ARM SMMU fixes Laurent Pinchart 2014-04-08 13:41 ` Laurent Pinchart 2014-04-08 13:57 ` Marc Zyngier 2014-04-14 16:58 ` Will Deacon 2014-04-15 15:55 ` Laurent Pinchart 2014-04-16 14:22 ` Will Deacon 2014-04-16 14:25 ` Laurent Pinchart
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).