From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH] VT-d/ATS: correct and clean up dev_invalidate_iotlb() Date: Wed, 25 Jun 2014 13:50:56 +0100 Message-ID: <53AAC5B0.9070008@citrix.com> References: <53A9A68C020000780001CDD4@mail.emea.novell.com> <53A9A2C7.6010708@citrix.com> <53AAC54C020000780001D2A5@mail.emea.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta4.messagelabs.com ([85.158.143.247]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1WzmfF-0003fi-AL for xen-devel@lists.xenproject.org; Wed, 25 Jun 2014 12:51:02 +0000 In-Reply-To: <53AAC54C020000780001D2A5@mail.emea.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: Yang Z Zhang , xen-devel List-Id: xen-devel@lists.xenproject.org On 25/06/14 11:49, Jan Beulich wrote: >>>> On 24.06.14 at 18:09, wrote: >> On 24/06/14 15:25, Jan Beulich wrote: >>> @@ -133,32 +135,37 @@ int dev_invalidate_iotlb(struct iommu *i >>> /* invalidate all translations: sbit=1,bit_63=0,bit[62:12]=1 */ >>> sbit = 1; >>> addr = (~0 << PAGE_SHIFT_4K) & 0x7FFFFFFFFFFFFFFF; >>> - ret |= qinval_device_iotlb(iommu, pdev->ats_queue_depth, >>> - sid, sbit, addr); >>> + rc = qinval_device_iotlb(iommu, pdev->ats_queue_depth, >>> + sid, sbit, addr); >>> break; >>> case DMA_TLB_PSI_FLUSH: >>> if ( !device_in_domain(iommu, pdev, did) ) >>> break; >>> >>> - addr &= ~0 << (PAGE_SHIFT + size_order); >>> - >>> /* if size <= 4K, set sbit = 0, else set sbit = 1 */ >>> sbit = size_order ? 1 : 0; >>> >>> /* clear lower bits */ >>> - addr &= (~0 << (PAGE_SHIFT + size_order)); >>> + addr &= ~0 << PAGE_SHIFT_4K; >> Doesn't this need to be ~0ULL as addr is u64? > No (or else further up in the code shown as context the same > would need to be done): Conversion here goes via sign-extension > (effectively int -> int64_t -> uint64_t). > > Jan > Ok, in which case Reviewed-by: Andrew Cooper