Re: [Xen-devel] [PATCH] AMD IOMMU: drop amd_iommu_setup_hwdom_device()

["Jan Beulich" <jbeulich@xxxxxxxx>]

>>> Roger Pau Monné <roger.pau@xxxxxxxxxx> 07/17/17 1:31 PM >>>
>On Fri, Jul 14, 2017 at 08:04:16AM -0600, Jan Beulich wrote:
>> By moving its bridge special casing to amd_iommu_add_device(), we can
>> pass the latter to setup_hwdom_pci_devices() and at once consistently
>> handle bridges discovered at boot time as well as such reported by Dom0
>> later on.
>> 
>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>
>Reviewed-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>

Thanks!

>With one nit:
>
>> @@ -490,15 +465,25 @@ static int amd_iommu_add_device(u8 devfn
>>  {
>>      struct amd_iommu *iommu;
>>      u16 bdf;
>> +
>>      if ( !pdev->domain )
>>          return -EINVAL;
>>  
>>      bdf = PCI_BDF2(pdev->bus, pdev->devfn);
>>      iommu = find_iommu_for_device(pdev->seg, bdf);
>> -    if ( !iommu )
>> +    if ( unlikely(!iommu) )
>>      {
>> -        AMD_IOMMU_DEBUG("Fail to find iommu."
>> -                        " %04x:%02x:%02x.%u cannot be assigned to dom%d\n",
>> +        /* Filter bridge devices. */
>> +        if ( pdev->type == DEV_TYPE_PCI_HOST_BRIDGE &&
>> +             is_hardware_domain(pdev->domain) )
>> +        {
>> +            AMD_IOMMU_DEBUG("Skipping host bridge %04x:%02x:%02x.%u\n",
>> +                            pdev->seg, PCI_BUS(bdf), PCI_SLOT(bdf),
>> +                            PCI_FUNC(bdf));
>
>Is there any reason to use bdf instead of pdev->bus and devfn? I'm
>asking because that's done below, so I would rather use that for
>coherency.

I guess I did simply copy another AMD_IOMMU_DEBUG() invocation. It's
questionable whether using one vs the other would be more efficient. If there
really is a difference, I'd suppose we should put together a cleanup patch
dealing with other such instances too.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel