From mboxrd@z Thu Jan 1 00:00:00 1970 From: Lu Baolu Subject: Re: [PATCH v4 04/12] iommu/vt-d: Add 256-bit invalidation descriptor support Date: Thu, 8 Nov 2018 14:15:26 +0800 Message-ID: References: <20181105053151.7173-1-baolu.lu@linux.intel.com> <20181105053151.7173-5-baolu.lu@linux.intel.com> <915876cd-6a1f-097b-b9be-9cb3df18a1df@linux.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Return-path: In-Reply-To: Content-Language: en-US Sender: linux-kernel-owner@vger.kernel.org To: "Liu, Yi L" , Joerg Roedel , David Woodhouse Cc: baolu.lu@linux.intel.com, "Raj, Ashok" , "Kumar, Sanjay K" , "Pan, Jacob jun" , "Tian, Kevin" , "Sun, Yi Y" , "peterx@redhat.com" , Jean-Philippe Brucker , "iommu@lists.linux-foundation.org" , "linux-kernel@vger.kernel.org" , Jacob Pan List-Id: iommu@lists.linux-foundation.org On 11/8/18 1:48 PM, Liu, Yi L wrote: >> From: Liu, Yi L >> Sent: Thursday, November 8, 2018 1:45 PM >>>>>>> + memcpy(desc, qi->desc + (wait_index << shift), >>>>>> >>>>>> Would "memcpy(desc, (unsigned long long) (qi->desc + (wait_index >>>>>> << shift)," be more safe? >>>>> >>>>> Can that be compiled? memcpy() requires a "const void *" for the >>>>> second >>> parameter. >>>>> By the way, why it's safer with this casting? >>>> >>>> This is just an example. My point is the possibility that "qi->desc >>>> + (wait_index << >>> shift)" >>>> would be treated as "qi->desc plus (wait_index << >>>> shift)*sizeof(*qi->desc)". Is it possible for kernel build? >>> >>> qi->desc is of type of "void *". >> >> no, I don’t think so... Refer to the code below. Even it has no correctness issue her, >> It's not due to qi->desc is "void *" type... >> >> struct qi_desc { >> - u64 low, high; >> + u64 qw0; >> + u64 qw1; >> + u64 qw2; >> + u64 qw3; >> }; > > Oops, just see you modified it to be "void *" in this patch. Ok, then this is fair enough. Yes. :-) Best regards, Lu Baolu