Re: [PATCH 01/14] iommu/amd: Move allocation of the top table into v1_alloc_pgtable

[Jason Gunthorpe <jgg@nvidia.com>]

On Fri, Aug 23, 2024 at 11:26:17AM +0200, Joerg Roedel wrote:
> On Wed, Aug 21, 2024 at 02:37:07PM -0300, Jason Gunthorpe wrote:
> > All the page table memory should be allocated/free within the io_pgtable
> > struct. The v2 path is already doing this, make it consistent.
> > 
> > It is hard to see but the free of the root in protection_domain_free() is
> > a NOP on the success path because v1_free_pgtable() does
> > amd_iommu_domain_clr_pt_root().
> > 
> > The root memory is already freed because free_sub_pt() put it on the
> > freelist. The free path in protection_domain_free() is only used during
> > error unwind of protection_domain_alloc().
> > 
> > Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> > ---
> >  drivers/iommu/amd/io_pgtable.c |  8 ++++++--
> >  drivers/iommu/amd/iommu.c      | 21 ++-------------------
> >  2 files changed, 8 insertions(+), 21 deletions(-)
> > 
> > diff --git a/drivers/iommu/amd/io_pgtable.c b/drivers/iommu/amd/io_pgtable.c
> > index 1074ee25064d06..05aed3cb46f1bf 100644
> > --- a/drivers/iommu/amd/io_pgtable.c
> > +++ b/drivers/iommu/amd/io_pgtable.c
> > @@ -574,20 +574,24 @@ static void v1_free_pgtable(struct io_pgtable *iop)
> >  	       pgtable->mode > PAGE_MODE_6_LEVEL);
> >  
> >  	free_sub_pt(pgtable->root, pgtable->mode, &freelist);
> > +	iommu_put_pages_list(&freelist);
> 
> You are freeing pages here before the IOMMU and device TLBs are flushed,
> no?

No see

 [PATCH 04/14] iommu/amd: Remove amd_iommu_domain_update() from page table freeing

This is on the domain free path. It is required that the domain not be
attached to anything. All the TLBs were flushed during the detatch
operations already.

Jason