Re: Removal of bus->msi assignment breaks MSI with stacked domains
From: Yijing Wang
Date: Fri Nov 21 2014 - 23:14:08 EST
å 2014/11/22 1:31, Bjorn Helgaas åé:
On Fri, Nov 21, 2014 at 09:54:40AM +0800, Yijing Wang wrote:
Thomas, let me know if you want to do that. I suppose we could add a new
patch to add it back, but that would leave bisection broken for the
interval between c167caf8d174 and the patch that adds it back.
Fortunately my irq/irqdomain branch is not immutable yet. So we have
no problem at that point. I can rebase on your branch until tomorrow
night. Or just rebase on mainline and we sort out the merge conflicts
later, i.e. delegate them to Linus so his job of pulling stuff gets
not completely boring.
Hi Thomas, sorry for my introducing the broken.
What I'm more worried about is whether this intended change is going
to inflict a problem on Jiangs intention to deduce the MSI irq domain
from the device, which we really need for making DMAR work w/o going
through loops and hoops.
I have limited knowledge about the actual scope of iommu (DMAR) units
versus device/bus/host-controllers, so I would appreciate a proper
explanation for that from you or Jiang or both.
In my personal opinion, if it's not necessary, we should not put stuff
into pci_dev or pci_bus. If we plan to save msi_controller in pci_bus or
pci_dev.
I have a proposal, I would be appreciated if you could give some comments.
First we refactor pci_host_bridge to make a generic
pci_host_bridge, then we could save pci domain in it to eliminate
arch specific functions. I aslo wanted to save msi_controller as
pci domain, but now Jiang refactor hierarchy irq domain, and
pci devices under the same pci host bridge may need to associate
to different msi_controllers.
I think this is getting ahead of ourselves. Let's make small steps.
We currently have the msi_controller pointer in struct pci_bus. That was
there even before your series. Your series added pci_msi_controller(),
and I reworked it so it looks like this:
static struct msi_controller *pci_msi_controller(struct pci_dev *dev)
{
struct msi_controller *msi_ctrl = dev->bus->msi;
if (msi_ctrl)
return msi_ctrl;
return pcibios_msi_controller(dev);
}
So now your series basically just removes the ARM add_bus() and
remove_bus() methods and gets the MSI controller info from the ARM
pci_sys_data struct instead of from pci_bus. Of course, that assumes that
on ARM, all devices under a host bridge have the same MSI controller. That
seems like an unwarranted assumption, but if you want to do it for ARM,
that's fine with me.
Agree, we could use pci_msi_controller() to find msi_controller for
pci_dev before a
better common way found.
So I want to associate a msi_controller finding ops with generic pci_host_bridge,
then every pci device could find its msi_controller/irq_domain by a
common function
E.g
struct msi_controller *pci_msi_controller(struct pci_dev *pdev)
{
struct msi_controller *ctrl;
struct pci_host_bridge *host = find_pci_host_bridge(pdev->bus);
if (host && host->pci_get_msi_controller)
ctrl = pci_host_bridge->pci_get_msi_controller(struct pci_dev *pdev);
return ctrl;
}
You can do this for ARM if you want (and your series already accomplishes
the same effect, though implemented differently). But I don't think this
is appropriate for the PCI core.
OK. We need a better solution, not only for arm, also need to consider
arm64 and
other platforms.
For anybody who is on this thread but not the original, I reworked the
series slightly, see [1].
Bjorn
[1] http://lkml.kernel.org/r/20141121172018.GA6578@xxxxxxxxxx
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/