Messages in this thread | | | Subject | Re: [PATCH v5 02/11] powerpc/kernel/iommu: Add new iommu_table_in_use() helper | From | Leonardo Brás <> | Date | Tue, 20 Jul 2021 02:38:16 -0300 |
| |
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
| |