From mboxrd@z Thu Jan  1 00:00:00 1970
From: Malcolm Crossley <malcolm.crossley@citrix.com>
Subject: Re: [PATCH v2] IOMMU: Prevent VT-d device IOTLB
 operations on wrong IOMMU
Date: Tue, 17 Jun 2014 14:56:25 +0100
Message-ID: <53A04909.6020800@citrix.com>
References: <b27eda6dc2e208b7481b.1403008092@malcolmc.uk.xensource.com>
	<53A0638E020000780001B17D@mail.emea.novell.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <53A0638E020000780001B17D@mail.emea.novell.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Jan Beulich <JBeulich@suse.com>
Cc: andrew.cooper3@citrix.com, kevin.tian@intel.com, Aravind.Gopalakrishnan@amd.com, suravee.suthikulpanit@amd.com, xen-devel@lists.xen.org
List-Id: xen-devel@lists.xenproject.org

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