From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-vk0-x241.google.com (mail-vk0-x241.google.com [IPv6:2607:f8b0:400c:c05::241]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 3D34681AF2 for ; Mon, 16 Jan 2017 10:43:06 -0800 (PST) Received: by mail-vk0-x241.google.com with SMTP id t8so3189329vke.0 for ; Mon, 16 Jan 2017 10:43:06 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=4c2whgOw/VB6TDELx8QrLAmvjAX5NlnKRQCBwZTHKQo=; b=c9a7W8P/dne0SlK3UKHzkvf6uXBeYq1PO+UPiE4G0ik7Ow9CjWEYr/C0tcSl8xMYZV bT/2U6jTrhvlzguTBkJFVj9sQMzte/ww/6ZBl/ITe4Yo85H0T444LD229EEa4ixXcDj4 hOooSzxXNaSFtHUh72jqK6D7x2ZhN8CJtwsslus0J+Sy+xTrIE87UiqG5i0eT/qsIk2S giFOatzkH4CmM/EOGVJ4dEWCITeub/jHdnpsbDm16Pt2jjOkCPXP87dCi4TkVLOzK+TD jxqb2SRg4YFiwdQsDuQwc0AczYfI8AUQKD0af+EP+13gWFheEimEnkDqVsS1eK4tj52l rVRw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=4c2whgOw/VB6TDELx8QrLAmvjAX5NlnKRQCBwZTHKQo=; b=AZLbTxTPMwVSg6QuM0jPvvFcgWSNwxntt62H7zuuZ3s+ISwy5ndeDh20DxuobfQ7cH zbKAMAHWLN6lVu0XrIBE2d6bTK3MEN9T7yuB6V+m/mk5jzgEFrLUVeVEdxkjh84VzNdD RsF2rGcllZc1QJ227AJE82orfc5DoUgAmKV6Lg2I6/c2uO9oGhBbVuCFsLljXDewARXH 4ZF6UYbd0n7cN6O2pPM+ZvbTTP2EGHzfYBMOmrcmPnWIASZVcBYQFify1XZzrtGA6gKu 2YV8G25RmANtNTERaysam1MU6wRZukZFhlg9sfePOCTC3VTMmAWTqC0v9r57vLcsocWw 5Lyg== X-Gm-Message-State: AIkVDXKSDW4oXWh0mId6h13hhoxUP19RkTxBsj2iPfVAoHNp0A065sDvgzhZeFvf9sjXe83P8hosq+o+CYEp5g== X-Received: by 10.31.132.129 with SMTP id g123mr2771346vkd.94.1484592185667; Mon, 16 Jan 2017 10:43:05 -0800 (PST) MIME-Version: 1.0 Received: by 10.31.151.68 with HTTP; Mon, 16 Jan 2017 10:43:05 -0800 (PST) In-Reply-To: <20170116180940.GO25883@bivouac.eciton.net> References: <20170116180940.GO25883@bivouac.eciton.net> From: Michael Zimmermann Date: Mon, 16 Jan 2017 19:43:05 +0100 Message-ID: To: Leif Lindholm Cc: "edk2-devel@lists.01.org" Subject: Re: [PATCH] ArmPlatformPkg: MemoryInitPei: fix wrong check for system memory X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 16 Jan 2017 18:43:06 -0000 Content-Type: text/plain; charset=UTF-8 The code in question is supposed to check if the memory range defined in the PCD is fully included in the memory resource descriptor of the current iteration. The condition for that is wrong. example: PcdSystemMemoryBase = 0x80000000 PcdSystemMemorySize = 0x10000000 ResourceDescriptor->PhysicalStart = 0x40000000 NextHob.ResourceDescriptor->ResourceLength = 0x10000000 obviously, the resource descriptor doesn't contain the system memory. but the old logic would have selected this Hob because: 0x80000000>=0x40000000 && 0x50000000 <= 0x90000000 The second comparison probably was supposed to have the ResourceDescriptor values on the right side, I fixed it by just inverting the direction of the comparison which then becomes: 0x80000000>=0x40000000 && 0x50000000 >= 0x90000000 And this comparison now successfully fails but will still be successful for the case we're checking for. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Michael Zimmermann --- ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c index 2feb11f..8f67812 100644 --- a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c +++ b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c @@ -103,7 +103,7 @@ MemoryPeim ( while ((NextHob.Raw = GetNextHob (EFI_HOB_TYPE_RESOURCE_DESCRIPTOR, NextHob.Raw)) != NULL) { if ((NextHob.ResourceDescriptor->ResourceType == EFI_RESOURCE_SYSTEM_MEMORY) && (PcdGet64 (PcdSystemMemoryBase) >= NextHob.ResourceDescriptor->PhysicalStart) && - (NextHob.ResourceDescriptor->PhysicalStart + NextHob.ResourceDescriptor->ResourceLength <= PcdGet64 (PcdSystemMemoryBase) + PcdGet64 (PcdSystemMemorySize))) + (NextHob.ResourceDescriptor->PhysicalStart + NextHob.ResourceDescriptor->ResourceLength >= PcdGet64 (PcdSystemMemoryBase) + PcdGet64 (PcdSystemMemorySize))) { Found = TRUE; break; On Mon, Jan 16, 2017 at 7:09 PM, Leif Lindholm wrote: > Hi Michael, > > Could you provide a message body explaining the error that this patch > fixes so that I don't sprain my brain trying to figure it out? > > On Thu, Jan 12, 2017 at 04:46:57PM +0100, Michael Zimmermann wrote: >> Contributed-under: TianoCore Contribution Agreement 1.0 >> Signed-off-by: Michael Zimmermann >> --- >> ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c >> b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c >> index 2feb11f21d..8f6781212e 100644 >> --- a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c >> +++ b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c >> @@ -103,7 +103,7 @@ MemoryPeim ( >> while ((NextHob.Raw = GetNextHob (EFI_HOB_TYPE_RESOURCE_DESCRIPTOR, >> NextHob.Raw)) != NULL) { >> if ((NextHob.ResourceDescriptor->ResourceType == >> EFI_RESOURCE_SYSTEM_MEMORY) && >> (PcdGet64 (PcdSystemMemoryBase) >= >> NextHob.ResourceDescriptor->PhysicalStart) && >> - (NextHob.ResourceDescriptor->PhysicalStart + >> NextHob.ResourceDescriptor->ResourceLength <= PcdGet64 >> (PcdSystemMemoryBase) + PcdGet64 (PcdSystemMemorySize))) > > It looks like the patch got corrupted on submission (lines wrapped). > Could you resend it please? > >> + (NextHob.ResourceDescriptor->PhysicalStart + >> NextHob.ResourceDescriptor->ResourceLength >= PcdGet64 >> (PcdSystemMemoryBase) + PcdGet64 (PcdSystemMemorySize))) >> { >> Found = TRUE; >> break; >> -- >> 2.11.0 >> _______________________________________________ >> edk2-devel mailing list >> edk2-devel@lists.01.org >> https://lists.01.org/mailman/listinfo/edk2-devel