On Thu, 2013-06-20 at 21:31 +0530, Varun Sethi wrote: > +#define REQ_ACS_FLAGS (PCI_ACS_SV | PCI_ACS_RR | PCI_ACS_CR | > PCI_ACS_UF) > + > +static struct iommu_group *get_device_iommu_group(struct device *dev) > +{ > + struct iommu_group *group; > + > + group = iommu_group_get(dev); > + if (!group) > + group = iommu_group_alloc(); > + > + return group; > +} > + [snip] > +
This really gets parent or peer, right? > +static struct iommu_group *get_peer_pci_device_group(struct pci_dev *pdev) > +{ > + struct iommu_group *group = NULL; > + > + /* check if this is the first device on the bus*/ > + if (pdev->bus_list.next == pdev->bus_list.prev) { It's a list_head, use list functions. The list implementation should be treated as opaque. if (list_is_singular(&pdev->bus_list)) > + struct pci_bus *bus = pdev->bus->parent; > + /* Traverese the parent bus list to get > + * pdev & dev for the sibling device. > + */ > + while (bus) { > + if (!list_empty(&bus->devices)) { > + pdev = container_of(bus->devices.next, > + struct pci_dev, bus_list); pdev = list_first_entry(&bus->devices, struct pci_dev, bus_list); > + group = iommu_group_get(&pdev->dev); > + break; > + } else > + bus = bus->parent; Is this ever reached? Don't you always have bus->self? > + } > + } else { > + /* > + * Get the pdev & dev for the sibling device > + */ > + pdev = container_of(pdev->bus_list.prev, > + struct pci_dev, bus_list); How do you know if you're at the head or tail of the list? struct pci_dev *tmp; list_for_each_entry(tmp, &pdev->bus_list, bus_list) { if (tmp == pdev) continue; group = iommu_group_get(&tmp->dev); break; } > + group = iommu_group_get(&pdev->dev); > + } > + > + return group; > +} > + > +static struct iommu_group *get_pci_device_group(struct pci_dev *pdev) > +{ > + struct iommu_group *group = NULL; > + struct pci_dev *bridge, *dma_pdev = NULL; > + struct pci_controller *pci_ctl; > + bool pci_endpt_partioning; > + > + pci_ctl = pci_bus_to_host(pdev->bus); > + pci_endpt_partioning = check_pci_ctl_endpt_part(pci_ctl); > + /* We can partition PCIe devices so assign device group to the device */ > + if (pci_endpt_partioning) { > + bridge = pci_find_upstream_pcie_bridge(pdev); > + if (bridge) { > + if (pci_is_pcie(bridge)) > + dma_pdev = pci_get_domain_bus_and_slot( > + pci_domain_nr(pdev->bus), > + bridge->subordinate->number, 0); > + if (!dma_pdev) > + dma_pdev = pci_dev_get(bridge); > + } else > + dma_pdev = pci_dev_get(pdev); > + > + /* Account for quirked devices */ > + swap_pci_ref(&dma_pdev, pci_get_dma_source(dma_pdev)); > + > + /* > + * If it's a multifunction device that does not support our > + * required ACS flags, add to the same group as function 0. > + */ See c14d2690 in Joerg's next tree, using function 0 was a poor assumption. > + if (dma_pdev->multifunction && > + !pci_acs_enabled(dma_pdev, REQ_ACS_FLAGS)) > + swap_pci_ref(&dma_pdev, > + pci_get_slot(dma_pdev->bus, > + > PCI_DEVFN(PCI_SLOT(dma_pdev->devfn), > + 0))); > + > + group = get_device_iommu_group(&pdev->dev); > + pci_dev_put(pdev); What was the point of all the above if we use pdev here instead of dma_pdev? Wrong device and broken reference counting. This also isn't testing ACS all the way up to the root complex or controller. > + /* > + * PCIe controller is not a paritionable entity > + * free the controller device iommu_group. > + */ > + if (pci_ctl->parent->iommu_group) > + iommu_group_remove_device(pci_ctl->parent); > + } else { > + /* > + * All devices connected to the controller will share the > + * PCI controllers device group. If this is the first > + * device to be probed for the pci controller, copy the > + * device group information from the PCI controller device > + * node and remove the PCI controller iommu group. > + * For subsequent devices, the iommu group information can > + * be obtained from sibling devices (i.e. from the bus_devices > + * link list). > + */ > + if (pci_ctl->parent->iommu_group) { > + group = get_device_iommu_group(pci_ctl->parent); > + iommu_group_remove_device(pci_ctl->parent); > + } else > + group = get_peer_pci_device_group(pdev); > + } > + > + return group; > +} > + > +static int fsl_pamu_add_device(struct device *dev) > +{ > + struct iommu_group *group = NULL; > + struct pci_dev *pdev; > + const u32 *prop; > + int ret, len; > + > + /* > + * For platform devices we allocate a separate group for > + * each of the devices. > + */ > + if (dev->bus == &pci_bus_type) { > + pdev = to_pci_dev(dev); > + /* Don't create device groups for virtual PCI bridges */ > + if (pdev->subordinate) > + return 0; > + > + group = get_pci_device_group(pdev); > + > + } else { > + prop = of_get_property(dev->of_node, "fsl,liodn", &len); > + if (prop) > + group = get_device_iommu_group(dev); > + } > + > + if (!group || IS_ERR(group)) > + return PTR_ERR(group); > + > + ret = iommu_group_add_device(group, dev); > + > + iommu_group_put(group); > + return ret; > +} > + > +static void fsl_pamu_remove_device(struct device *dev) > +{ > + iommu_group_remove_device(dev); > +} -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/