problems with iommu_free_table()

[Olof Johansson]

Benjamin Herrenschmidt wrote:

> Ouch, I should have commented earlier... now it went in and has
> problems:
> 
>  - It breaks build without CONFIG_PPC_PSERIES (try a pmac-only build).
> There is, more generally, a tendency at calling things in
> pSeries_iommu.c with the prefix "iommu_" without any mention of
> "pSeries" in the name. Hey guys ! pSeries isn't alone anymore ! So
> please call those pSeries-specific things pSeries_* or tce_* or
> whatever, but don't add back confusion where I had such a hard time
> splitting things.

Actually, you're wrong. :) It's not pSeries-specific, see below.

>  - It seems that any call to of_remove_node() will call
> iommu_free_table() on np->iommu_table. That sounds bad. The iommu_table
> pointer is copied at init time from the parent to all child nodes. So if
> we add a phb, and then remove a device from that bus, we end up
> disposing of the phb's iommu table ...

Yep, you're right. There's two ways to fix this: Add reference counting 
to the iommu tables and do automatic deallocation, or only delete the 
tables for PHB deallocation. The second option would be preferred, since 
it should be the right way to solve the layering violation.

> I'll send a patch fixing G5 build by renaming iommu_free_table to
> tce_free_table() and putting the call in #ifdef CONFIG_PPC_PSERIES for
> now,

This is the wrong solution. iommu_free_table is a companion to 
iommu_init_table, and it _is_ generic code, it just ended up in the 
wrong file (I didn't catch that myself, sorry about that).


-Olof