ARM SMMU fixes
cover ARM SMMU fixes
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(-)
Signature · 3 lines
Regards, Laurent Pinchart
PATCH 1/2 iommu/arm-smmu: Replace list walk with platform driver data
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.
--- drivers/iommu/arm-smmu.c | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-)
drivers/iommu/arm-smmu.c +4 -11
@@ -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);
Signature · 1 line
1.8.3.2
PATCH 1/2 iommu/arm-smmu: Replace list walk with platform driver data
Hi Laurent,
Quoted 3 lines · Laurent Pinchart
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
PATCH 1/2 iommu/arm-smmu: Replace list walk with platform driver data
Hi Will,
Quoted 8 lines · On Friday 28 February 2014 16:38:37 Will Deacon wrote:
Hi Laurent,
Quoted 3 lines · Laurent Pinchart
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 ? :-)
Quoted 2 lines
If we could get rid of the
arm_smmu_deviceslist, 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.
Signature · 3 lines
Regards, Laurent Pinchart
PATCH 2/2 iommu/arm-smmu: Return 0 on unmap failure
In reply to Laurent PinchartThe 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.
--- drivers/iommu/arm-smmu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/iommu/arm-smmu.c +1 -1
@@ -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,
Signature · 1 line
1.8.3.2
PATCH 2/2 iommu/arm-smmu: Return 0 on unmap failure
Quoted 3 lines · Laurent Pinchart
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
Quoted 27 lines
Signed-off-by Laurent Pinchart <[email protected]>--- drivers/iommu/arm-smmu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/iommu/arm-smmu.c +1
-1@@ -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,Signature · 7 lines
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
cover ARM SMMU fixes
In reply to Laurent PinchartHi Will,
Quoted 6 lines · 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 :-)
Quoted 6 lines
Laurent Pinchart (2):
iommu/arm-smmu: Replace list walk with platform driver data iommu/arm-smmu: Return 0 on unmap failuredrivers/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.
Signature · 3 lines
Regards, Laurent Pinchart
cover ARM SMMU fixes
I've obviously forgotten that Will was away for a month. CC'ing Marc Zyngier.
Quoted 11 lines · On Thursday 03 April 2014 01:52:55 Laurent Pinchart wrote:
Quoted 10 lines · 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
Quoted 5 lines
Quoted 2 lines
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 ?
Signature · 3 lines
Regards, Laurent Pinchart
cover ARM SMMU fixes
Quoted 14 lines · Laurent Pinchart
I've obviously forgotten that Will was away for a month. CC'ing Marc Zyngier.
Quoted 11 lines · On Thursday 03 April 2014 01:52:55 Laurent Pinchart wrote:
Quoted 10 lines · 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
Quoted 8 lines
Quoted 5 lines
Quoted 2 lines
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.
Signature · 1 line
Jazz is not dead. It just smells funny...
cover ARM SMMU fixes
Quoted 21 lines · Marc Zyngier
Quoted 20 lines · Laurent Pinchart
I've obviously forgotten that Will was away for a month. CC'ing Marc Zyngier.
Quoted 16 lines · On Thursday 03 April 2014 01:52:55 Laurent Pinchart wrote:
Quoted 6 lines · 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 :-)
Quoted 6 lines
Laurent Pinchart (2):
iommu/arm-smmu: Replace list walk with platform driver data iommu/arm-smmu: Return 0 on unmap failuredrivers/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.
Quoted 7 lines
Quoted 2 lines
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
cover ARM SMMU fixes
Hi Will,
Quoted 27 lines · On Monday 14 April 2014 17:58:58 Will Deacon wrote:
Quoted 26 lines · Marc Zyngier
Quoted 24 lines · Laurent Pinchart
I've obviously forgotten that Will was away for a month. CC'ing Marc Zyngier.
Quoted 12 lines · On Thursday 03 April 2014 01:52:55 Laurent Pinchart wrote:
Quoted 11 lines · 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 failureQuoted 5 lines
Quoted 2 lines
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 ?
Quoted 7 lines
Quoted 4 lines
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.
Quoted 1 line
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.
Signature · 3 lines
Regards, Laurent Pinchart
cover ARM SMMU fixes
Quoted 1 line · Laurent Pinchart
Hi Will,
Hi Laurent,
Quoted 7 lines
On a different but related topic, I've written an
ipmmu-vmsa.cdriver 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
cover ARM SMMU fixes
Hi Will,
Quoted 18 lines · On Wednesday 16 April 2014 15:22:06 Will Deacon wrote:
Quoted 1 line · Laurent Pinchart
Hi Will,
Hi Laurent,
Quoted 8 lines
On a different but related topic, I've written an
ipmmu-vmsa.cdriver 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:
Quoted 6 lines
- Stage-1 and Stage-2 formats (although long-descriptor only)
- 4k and 64k pages (the latter for arm64 only)
- Contiguous pte hintsDo 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.
Signature · 3 lines
Regards, Laurent Pinchart