On 21/08/2019 13:05, Will Deacon wrote:
Hi Robin,
Thanks for looking at this.
On Wed, Aug 21, 2019 at 12:42:11PM +0100, Robin Murphy wrote:
On 14/08/2019 18:56, Will Deacon wrote:
The ->tlb_add_flush() callback in the io-pgtable API now looks a bit
silly:
- It takes a size and a granule, which are always the same
- It takes a 'bool leaf', which is always true
- It only ever flushes a single page
With that in mind, replace it with an optional ->tlb_add_page() callback
that drops the useless parameters.
Signed-off-by: Will Deacon <w...@kernel.org>
[...]
-static const struct iommu_flush_ops arm_smmu_s2_tlb_ops_v2 = {
- .tlb_flush_all = arm_smmu_tlb_inv_context_s2,
- .tlb_flush_walk = arm_smmu_tlb_inv_walk,
- .tlb_flush_leaf = arm_smmu_tlb_inv_leaf,
- .tlb_add_flush = arm_smmu_tlb_inv_range_nosync,
- .tlb_sync = arm_smmu_tlb_sync_context,
+static const struct arm_smmu_flush_ops arm_smmu_s2_tlb_ops_v2 = {
+ .tlb = {
+ .tlb_flush_all = arm_smmu_tlb_inv_context_s2,
+ .tlb_flush_walk = arm_smmu_tlb_inv_walk,
+ .tlb_flush_leaf = arm_smmu_tlb_inv_leaf,
+ .tlb_add_page = arm_smmu_tlb_add_page,
+ .tlb_sync = arm_smmu_tlb_sync_context,
+ },
+ .tlb_inv_range = arm_smmu_tlb_inv_range_nosync,
};
-static const struct iommu_flush_ops arm_smmu_s2_tlb_ops_v1 = {
- .tlb_flush_all = arm_smmu_tlb_inv_context_s2,
- .tlb_flush_walk = arm_smmu_tlb_inv_walk,
- .tlb_flush_leaf = arm_smmu_tlb_inv_leaf,
- .tlb_add_flush = arm_smmu_tlb_inv_vmid_nosync,
- .tlb_sync = arm_smmu_tlb_sync_vmid,
+static const struct arm_smmu_flush_ops arm_smmu_s2_tlb_ops_v1 = {
+ .tlb = {
+ .tlb_flush_all = arm_smmu_tlb_inv_context_s2,
+ .tlb_flush_walk = arm_smmu_tlb_inv_walk,
+ .tlb_flush_leaf = arm_smmu_tlb_inv_leaf,
Urgh, that ain't right... :(
Sorry I've only spotted it now while trying to rebase onto Joerg's queue,
but we can't use either of those callbacks for v1 stage 2 since the
registers they access don't exist. I'll spin a fixup patch first, then come
back to the question of whether it's more practical to attempt merging my v2
or concede to rebasing a v3.
Although the code is quite difficult to follow, I think it's alright because
the tlb_flush_{walk,leaf} functions just indirect back through the
tlb_inv_range callback. This patch is supposed to be a big NOP moving
drivers over to the new API, but not actually exploiting its benefits.
Ah, sorry, I did indeed fail the reading comprehension test - too many
levels of indirection...
On second reading I agree that this probably should work out OK (other
than perhaps a performance hit from chaining more indirect branches).
I've noted on my to-do list to come back and clean up arm_smmu_flush_ops
for next cycle, but for now I'll get back to the more pressing matters.
Thanks,
Robin.
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
