mh after thinking about it, wouldn't it be better to swap the '<=' operands instead of changing the operator? this way the PCD memory would be on the left side for both comparisons which seems more clear.
On Mon, Jan 16, 2017 at 7:43 PM, Michael Zimmermann <[email protected]> wrote: > 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
