From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Jan Beulich" Subject: Re: [PATCH v2 13/19] hvmloader: construct SRAT Date: Wed, 10 Dec 2014 11:06:20 +0000 Message-ID: <5488373C020000780004E836@mail.emea.novell.com> References: <1417448023-27311-1-git-send-email-wei.liu2@citrix.com> <1417448023-27311-14-git-send-email-wei.liu2@citrix.com> <54873724020000780004E37B@mail.emea.novell.com> <20141209180657.GG25749@zion.uk.xensource.com> <54881066020000780004E6D6@mail.emea.novell.com> <20141210105449.GA15588@zion.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20141210105449.GA15588@zion.uk.xensource.com> Content-Disposition: inline List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Wei Liu Cc: ian.campbell@citrix.com, dario.faggioli@citrix.com, ian.jackson@eu.citrix.com, xen-devel@lists.xen.org, ufimtseva@gmail.com, boris.ostrovsky@oracle.com List-Id: xen-devel@lists.xenproject.org >>> On 10.12.14 at 11:54, wrote: > On Wed, Dec 10, 2014 at 08:20:38AM +0000, Jan Beulich wrote: >> >>> On 09.12.14 at 19:06, wrote: >> > On Tue, Dec 09, 2014 at 04:53:40PM +0000, Jan Beulich wrote: >> >> >>> On 01.12.14 at 16:33, wrote: >> >> > + memset(memory, 0, sizeof(*memory)); >> >> > + memory->type = ACPI_MEMORY_AFFINITY; >> >> > + memory->length = sizeof(*memory); >> >> > + memory->domain = vmemrange[i].nid; >> >> > + memory->flags = ACPI_MEM_AFFIN_ENABLED; >> >> > + memory->base_address = vmemrange[i].start; >> >> > + memory->mem_length = mem; >> >> > + memory++; >> >> > + } >> >> > + >> >> > + srat->header.length = size; >> >> >> >> Mind checking size == memory - p here? >> >> >> > >> > Why? There doesn't seem to be anything that would cause memory -p != >> > size in between during runtime. >> >> Except for that original calculation being wrong - that's what I would >> mean such a check to verify. >> > > Do you mean the code is wrong? I don't think so. No, the code looks right. It's just that two disconnected calculations easily get out of sync when someone later changes them. > Even if I have > > + if ( ((unsigned long)memory) - ((unsigned long)p) != size ) > + return NULL; > + > > Hvmloader doesn't complain, i.e. I don't see error message printed out > in the construct_secondary_tables. > > Anyway, if the above snippet is what you want, I can add it. I was actually more after a BUG_ON() / ASSERT() kind of thing. Jan