Re: [PATCH v2] IOMMU: Prevent VT-d device IOTLB operations on wrong IOMMU

[Malcolm Crossley <malcolm.crossley@citrix.com>]

On 17/06/14 14:49, Jan Beulich wrote:
>>>> On 17.06.14 at 14:28, <malcolm.crossley@citrix.com> wrote:
>> Optimised AMD's IOMMU code by leveraging the new IOMMU field in the ATS
>> structure.
> 
> This optimization should presumably be dropped after having done
> the constification:

An ATS device will always be physically connected to the same IOMMU so
the optimisation should still be valid.

> 
>> --- a/xen/drivers/passthrough/amd/iommu_cmd.c
>> +++ b/xen/drivers/passthrough/amd/iommu_cmd.c
>> @@ -303,8 +303,7 @@ void amd_iommu_flush_iotlb(u8 devfn, con
>>      if ( !pci_ats_enabled(ats_pdev->seg, ats_pdev->bus, ats_pdev->devfn) )
>>          return;
>>  
>> -    iommu = find_iommu_for_device(ats_pdev->seg,
>> -                                  PCI_BDF2(ats_pdev->bus, ats_pdev->devfn));
>> +    iommu = (struct amd_iommu *) ats_pdev->iommu;
> 
> Casts like this are ugly and error prone (leaving aside that it violates
> the promises "const" makes).

It was either this or I have to patch several later functions to expect
const struct amd_iommu, this may unravel to needing to patch many locations.
> 
>> @@ -34,7 +35,7 @@ struct pci_ats_dev {
>>  extern struct list_head ats_devices;
>>  extern bool_t ats_enabled;
>>  
>> -int enable_ats_device(int seg, int bus, int devfn);
>> +int enable_ats_device(void *iommu, int seg, int bus, int devfn);
> 
> The first parameter should now be "const" too.

Ok, I missed that.
> 
> Jan
> 

So do I drop the optimisation to avoid the problems with using the const?

Malcolm