Subject: Re: [v2] pseries/iommu: remove iommu device references via bus notifier

From: Nishanth Aravamudan <[email-masked]>
Date: Mon, 23 Feb 2015 10:54:29 -0800

On 23.02.2015 [13:27:24 +1100], Michael Ellerman wrote:
> On Sat, 2015-21-02 at 19:00:50 UTC, Nishanth Aravamudan wrote:
> > On 20.02.2015 [15:31:29 +1100], Michael Ellerman wrote:
> > > On Thu, 2015-02-19 at 10:41 -0800, Nishanth Aravamudan wrote:
> > > > After d905c5df9aef ("PPC: POWERNV: move iommu_add_device earlier"), the
> > > > refcnt on the kobject backing the IOMMU group for a PCI device is
> > > > elevated by each call to pci_dma_dev_setup_pSeriesLP() (via
> > > > set_iommu_table_base_and_group). When we go to dlpar a multi-function
> > > > PCI device out:
> > > > 
> > > > 	iommu_reconfig_notifier ->
> > > > 		iommu_free_table ->
> > > > 			iommu_group_put
> > > > 			BUG_ON(tbl->it_group)
> > > > 
> > > > We trip this BUG_ON, because there are still references on the table, so
> > > > it is not freed. Fix this by also adding a bus notifier identical to
> > > > PowerNV for pSeries.
> > > 
> > > Please put it somewhere common, arch/powerpc/kernel/iommu.c perhaps, and just
> > > add a second machine_init_call() for pseries.
> > 
> > How does this look? Only compile-tested with CONFIG_IOMMU_API on/off so
> > far, waiting for access to the test LPAR (should have it on Monday).
> 
> Yeah that looks better, thanks.
> 
> It probably doesn't build with CONFIG_PCI=n though, but I don't think
> CONFIG_PCI=n builds anyway.

Indeed it doesn't. Started looking at CONFIG_PCI=n and immediately hit
the following:

PCI_MSI depends on PCI

PCI can be manually turned off

PSERIES (and a bunch of other platforms) select PCI_MSI

So you end up with PCI_MSI on and PCI off and the build breaks.

Should the platforms depend on PCI_MSI instead?

Per the Documentation:
"        select should be used with care. select will force
        a symbol to a value without visiting the dependencies.
        By abusing select you are able to select a symbol FOO even
        if FOO depends on BAR that is not set."

Thanks,
Nish