RE: [PATCH v2 2/2] Intel-IOMMU, intr-remap: source-id checking

[Han, Weidong]

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 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/