On 8/7/2013 10:42 AM, Jan Beulich wrote:
>>>> On 07.08.13 at 17:31, Suravee Suthikulanit <suravee.suthikulpanit@amd.com> wrote:
>> On 8/7/2013 2:33 AM, Jan Beulich wrote:
>>>>>> On 07.08.13 at 04:40, <suravee.suthikulpanit@amd.com> wrote:
>>>> +        /* Filter the bridge devices */
>>>> +        if ( (pdev->type == DEV_TYPE_PCIe_BRIDGE)
>>>> +        ||   (pdev->type == DEV_TYPE_PCIe2PCI_BRIDGE)
>>>> +        ||   (pdev->type == DEV_TYPE_LEGACY_PCI_BRIDGE)
>>>> +        ||   (pdev->type == DEV_TYPE_PCI_HOST_BRIDGE) )
>>>> +        {
>>>> +            AMD_IOMMU_DEBUG("Skipping device %04x:%02x:%02x.%u (type %x)\n",
>>>> +                        pdev->seg, PCI_BUS(bdf), PCI_SLOT(bdf), PCI_FUNC(bdf),
>>>> +                        pdev->type);
>>> I can see why host bridges can be skipped, but is this really also true
>>> for other bridge types, most importantly legacy ones?
>> I am using the same logic here as in Intel's version in the
>> driver/passthrough/vtd/iommu/domain_context_map.
> No, not really: That code establishes a mapping for the upstream
> bridge of a legacy PCI device when handling that device. Your
> proposed code doesn't afaics. (This I understand is because
> bridges aren't expected to get assigned to guests, and hence
> otherwise the mapping for the bridge would never get established
> for a device behind it for other than Dom0.)
>
> Jan
Jan,

AFAICT from the domain_context_mapping() below, which get called when 
the devices are assigned to domains

static int domain_context_mapping(
     struct domain *domain, u8 devfn, const struct pci_dev *pdev)
{
     struct acpi_drhd_unit *drhd;
     int ret = 0;
     u8 seg = pdev->seg, bus = pdev->bus, secbus;

     drhd = acpi_find_matched_drhd_unit(pdev);
     if ( !drhd )
         return -ENODEV;

     ASSERT(spin_is_locked(&pcidevs_lock));

     switch ( pdev->type )
     {
     case DEV_TYPE_PCIe_BRIDGE:
     case DEV_TYPE_PCIe2PCI_BRIDGE:
     case DEV_TYPE_LEGACY_PCI_BRIDGE:
         break;

These bridges are actually not mapped(i.e. skipped).  That is why I 
think the logic above is supposed to be doing the same thing.

Suravee