[PATCH v5 02/11] powerpc/kernel/iommu: Add new iommu_table_in_use() helper

[Leonardo Brás]

Hello Fred, thanks for this feedback!

Sorry if I miss anything, this snippet was written for v1 over an year
ago, and I have not taken a look at it ever since.

On Mon, 2021-07-19 at 15:53 +0200, Frederic Barrat wrote:
> 
> 
> On 16/07/2021 10:27, Leonardo Bras wrote:
> > @@ -1099,18 +1105,13 @@ int iommu_take_ownership(struct iommu_table
> > *tbl)
> >         for (i = 0; i < tbl->nr_pools; i++)
> >                 spin_lock_nest_lock(&tbl->pools[i].lock, &tbl-
> > >large_pool.lock);
> >   
> > -       iommu_table_release_pages(tbl);
> > -
> > -       if (!bitmap_empty(tbl->it_map, tbl->it_size)) {
> > +       if (iommu_table_in_use(tbl)) {
> >                 pr_err("iommu_tce: it_map is not empty");
> >                 ret = -EBUSY;
> > -               /* Undo iommu_table_release_pages, i.e. restore
> > bit#0, etc */
> > -               iommu_table_reserve_pages(tbl, tbl-
> > >it_reserved_start,
> > -                               tbl->it_reserved_end);
> > -       } else {
> > -               memset(tbl->it_map, 0xff, sz);
> >         }
> >   
> > +       memset(tbl->it_map, 0xff, sz);
> > +
> 
> 
> So if the table is not empty, we fail (EBUSY) but we now also
> completely 
> overwrite the bitmap. It was in an unexpected state, but we're making
> it 
> worse. Or am I missing something?

IIRC there was a reason to do that at the time, but TBH I don't really
remember it, and by looking at the code right now you seem to be
correct about this causing trouble.

I will send a v6 fixing it soon.
Please review the remaining patches for some issue I may be missing.

Alexey, any comments on that?

> 
>    Fred
> 

Again, thank you for reviewing Fred! 
Best regards,
Leonardo Bras