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 <[email protected]> --- 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 <[email protected]> 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 <[email protected]> >> --- >> 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 >> [email protected] >> https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
