* [PATCH] ArmPlatformPkg: MemoryInitPei: fix wrong check for system memory
@ 2017-01-12 15:46 Michael Zimmermann
2017-01-13 4:47 ` Michael Zimmermann
2017-01-16 18:09 ` Leif Lindholm
0 siblings, 2 replies; 5+ messages in thread
From: Michael Zimmermann @ 2017-01-12 15:46 UTC (permalink / raw)
To: edk2-devel@lists.01.org
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Michael Zimmermann <sigmaepsilon92@gmail.com>
---
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)))
+ (NextHob.ResourceDescriptor->PhysicalStart +
NextHob.ResourceDescriptor->ResourceLength >= PcdGet64
(PcdSystemMemoryBase) + PcdGet64 (PcdSystemMemorySize)))
{
Found = TRUE;
break;
--
2.11.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] ArmPlatformPkg: MemoryInitPei: fix wrong check for system memory
2017-01-12 15:46 [PATCH] ArmPlatformPkg: MemoryInitPei: fix wrong check for system memory Michael Zimmermann
@ 2017-01-13 4:47 ` Michael Zimmermann
2017-01-16 18:09 ` Leif Lindholm
1 sibling, 0 replies; 5+ messages in thread
From: Michael Zimmermann @ 2017-01-13 4:47 UTC (permalink / raw)
To: edk2-devel@lists.01.org, Leif Lindholm, Ard Biesheuvel
sry, but I forgot to CC the maintainers again so doing this with this
self-reply.
On Thu, Jan 12, 2017 at 4:46 PM, Michael Zimmermann <
sigmaepsilon92@gmail.com> wrote:
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Michael Zimmermann <sigmaepsilon92@gmail.com>
> ---
> 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)))
> + (NextHob.ResourceDescriptor->PhysicalStart +
> NextHob.ResourceDescriptor->ResourceLength >= PcdGet64
> (PcdSystemMemoryBase) + PcdGet64 (PcdSystemMemorySize)))
> {
> Found = TRUE;
> break;
> --
> 2.11.0
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ArmPlatformPkg: MemoryInitPei: fix wrong check for system memory
2017-01-12 15:46 [PATCH] ArmPlatformPkg: MemoryInitPei: fix wrong check for system memory Michael Zimmermann
2017-01-13 4:47 ` Michael Zimmermann
@ 2017-01-16 18:09 ` Leif Lindholm
2017-01-16 18:43 ` Michael Zimmermann
1 sibling, 1 reply; 5+ messages in thread
From: Leif Lindholm @ 2017-01-16 18:09 UTC (permalink / raw)
To: Michael Zimmermann; +Cc: edk2-devel
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 <sigmaepsilon92@gmail.com>
> ---
> 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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ArmPlatformPkg: MemoryInitPei: fix wrong check for system memory
2017-01-16 18:09 ` Leif Lindholm
@ 2017-01-16 18:43 ` Michael Zimmermann
2017-01-16 18:44 ` Michael Zimmermann
0 siblings, 1 reply; 5+ messages in thread
From: Michael Zimmermann @ 2017-01-16 18:43 UTC (permalink / raw)
To: Leif Lindholm; +Cc: edk2-devel@lists.01.org
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 <sigmaepsilon92@gmail.com>
---
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 <leif.lindholm@linaro.org> 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 <sigmaepsilon92@gmail.com>
>> ---
>> 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
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] ArmPlatformPkg: MemoryInitPei: fix wrong check for system memory
2017-01-16 18:43 ` Michael Zimmermann
@ 2017-01-16 18:44 ` Michael Zimmermann
0 siblings, 0 replies; 5+ messages in thread
From: Michael Zimmermann @ 2017-01-16 18:44 UTC (permalink / raw)
To: Leif Lindholm; +Cc: edk2-devel@lists.01.org
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
<sigmaepsilon92@gmail.com> 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 <sigmaepsilon92@gmail.com>
> ---
> 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 <leif.lindholm@linaro.org> 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 <sigmaepsilon92@gmail.com>
>>> ---
>>> 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
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-01-16 18:44 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-01-12 15:46 [PATCH] ArmPlatformPkg: MemoryInitPei: fix wrong check for system memory Michael Zimmermann
2017-01-13 4:47 ` Michael Zimmermann
2017-01-16 18:09 ` Leif Lindholm
2017-01-16 18:43 ` Michael Zimmermann
2017-01-16 18:44 ` Michael Zimmermann
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox