Eric W. Biederman wrote: > "Han, Weidong" <weidong.han@xxxxxxxxx> writes: > >> Ingo Molnar wrote: >>> * Eric W. Biederman <ebiederm@xxxxxxxxxxxx> wrote: >>> >>>> Not finding an upstream pcie_bridge and then concluding we are a >>>> pcie device seems bogus. >>>> >> >> If device is pcie, pci_find_upstream_pcie_bridge() will return >> NULL. For root complex integrated device, it won't find upstream >> bridge, and also return NULL. What's more, pcie device and root >> complex integrated device will be handled as the same way to set >> sid, so I mix them here. But it also returns NULL for busted >> hardware. I think no parent bus can be considered Root Complex >> integrated device, right? If so, I think can handle it as follows: > > I have problems with pci_find_upstream_pcie_bridge. The > name is actively misleading about what that function does. > Returning a pci_to_pci bridge is strongly counter intuitive > give that name. > > Can we change it to pci_find_upstream_pcie_to_pci_bridge. > Returning NULL in all cases when there is not an upstream > pcie_to_pci bridge. pci_find_upstream_pcie_bridge returns upstream pcie-to-pci/pcix bridge or legacy pci bridge for a pci device. pci_find_upstream_bridge may be more suitable. > > For the set_sid case that is ideal. For the other cases in > intel-iommu.c it may be a problem. Is it even possible to have a > genuine pci device not behind a pcie to pci bridge on an intel > chipset with this iommu? > I think it's little possible. But coincide to VT-d spec, we need to handle differently for devices behind pcie-to-pci/pcix bridge and behind legacy pci bridge. > >> ... >> if (dev->is_pcie || !dev->bus->parent) { >> set_irte_sid(...); >> return 0; >> } >> >> bridge = pci_find_upstream_pcie_bridge(dev); >> if (bridge) { >> if (bridge->is_pcie) /* PCIE-to-PCI/PCIx bridge */ >> set_irte_sid(...); else /* legacy PCI bridge */ >> set_irte_sid(..); >> } >> >> return 0; >> >>>> Why if we do have an upstream pcie bridge do we only want to do a >>>> bus range verification instead of checking just for the bus >>>>> devfn? >>>> >>>> The legacy PCI case seems even stranger. >> >> Why? If a PCI device isn't connected to a PCIe bridge, it should be >> a legacy bridge. > > I am not deep in the IOV specification at the moment. I am mostly > wondering why we pick the parts we pick to verify. I recall > bus and devfn being on the pcie packets so that makes sense. > > Why would we ever want to do something different? Does a pcie to pci > bridge do something different in it's translation? source-id is different between devices pcie-to-pci/pcix bridge and behind legacy pci bridge. Thus VT-d needs to handle it differently. The pcie-to-pci/pcix bridges may generate a different requester-id and tag combination in some instances for transactions forwarded to the bridge's PCI Express interface. The action of replacing the original transaction's requester-id with one assigned by the bridge is generally referred to as taking 'ownership' of the transaction. If the bridge generates a new requester-id for a transaction forwarded from the secondary interface to the primary interface, the bridge assigns the PCI Express requester-id using the secondary interface's bus number, and sets both the device number and function number fields to zero. Refer to the PCI Express-to-PCI/PCI-X bridge specifications for more details. For devices behind conventional PCI bridges, the source-id in the DMA requests is the requester-id of the bridge device. Regards, Weidong -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html