From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH for-4.6] xen/mm: populate_physmap: validate correctly the gfn for direct mapped domain Date: Wed, 12 Aug 2015 10:25:32 +0100 Message-ID: <55CB110C.1090502@citrix.com> References: <1439314888-24813-1-git-send-email-julien.grall@citrix.com> <55CB2B0B020000780009A035@prv-mh.provo.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta14.messagelabs.com ([193.109.254.103]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1ZPSHx-0006Fm-SJ for xen-devel@lists.xenproject.org; Wed, 12 Aug 2015 09:25:37 +0000 In-Reply-To: <55CB2B0B020000780009A035@prv-mh.provo.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: Wei.Liu2@citrix.com, ian.campbell@citrix.com, Tim Deegan , Ian Jackson , stefano.stabellini@citrix.com, xen-devel@lists.xenproject.org, Keir Fraser List-Id: xen-devel@lists.xenproject.org Hi Jan, On 12/08/2015 10:16, Jan Beulich wrote: >>>> On 11.08.15 at 19:41, wrote: >> --- a/xen/common/memory.c >> +++ b/xen/common/memory.c >> @@ -126,22 +126,28 @@ static void populate_physmap(struct memop_args *a) >> if ( is_domain_direct_mapped(d) ) >> { >> mfn = gpfn; >> - if ( !mfn_valid(mfn) ) >> + >> + for ( j = 0; j < (1 << a->extent_order); j++, mfn++ ) > > While benign I think we shouldn't repeat mistakes like this made > elsewhere in the code: At the very least this should be 1U, but > with j (needlessly, just like i) being unsigned long it would be > more consistent for this to be 1UL unless we clean up the variable > types (which I think I'll do). I admit that I copied the loop at then end of the function without thinking if it there was some issue or not. > > Since the change is so minor, I'd be fine with making it while > committing, unless you object. I'm fine with that. Regards, -- Julien Grall