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.

Will
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Reply via email to