* [PATCH] UefiCpuPkg/CpuDxe: Do not ASSERT on AllocateMemorySpace() error
@ 2017-02-24 6:12 Jeff Fan
2017-02-24 6:13 ` Tian, Feng
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Jeff Fan @ 2017-02-24 6:12 UTC (permalink / raw)
To: edk2-devel; +Cc: Laszlo Ersek, Star Zeng, Feng Tian, Michael D Kinney
Platform PEI may add LOCAL APIC memory mapped space into
EFI_HOB_MEMORY_ALLOCATION. Or platform may allocate this range before.
So, we skip AllocateMemorySpace()'s return status checking. Instead, we add one
DEBUG message for possible trace.
https://bugzilla.tianocore.org/show_bug.cgi?id=390
This updating is suggested by Ersek's comments at
https://www.mail-archive.com/edk2-devel@lists.01.org/msg22585.html
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Feng Tian <feng.tian@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jeff Fan <jeff.fan@intel.com>
---
UefiCpuPkg/CpuDxe/CpuDxe.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/UefiCpuPkg/CpuDxe/CpuDxe.c b/UefiCpuPkg/CpuDxe/CpuDxe.c
index 2fd2f31..4a5e282 100644
--- a/UefiCpuPkg/CpuDxe/CpuDxe.c
+++ b/UefiCpuPkg/CpuDxe/CpuDxe.c
@@ -1075,6 +1075,11 @@ AddLocalApicMemorySpace (
Status = AddMemoryMappedIoSpace (BaseAddress, SIZE_4KB, EFI_MEMORY_UC);
ASSERT_EFI_ERROR (Status);
+ //
+ // Try to allocate APIC memory mapped space, does not check return
+ // status because it may be allocated by other driver, or DXE Core if
+ // this range is built into Memory Allocation HOB.
+ //
Status = gDS->AllocateMemorySpace (
EfiGcdAllocateAddress,
EfiGcdMemoryTypeMemoryMappedIo,
@@ -1084,7 +1089,10 @@ AddLocalApicMemorySpace (
ImageHandle,
NULL
);
- ASSERT_EFI_ERROR (Status);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_INFO, "%a: %a: AllocateMemorySpace() Status - %r\n",
+ gEfiCallerBaseName, __FUNCTION__, Status));
+ }
}
/**
--
2.9.3.windows.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] UefiCpuPkg/CpuDxe: Do not ASSERT on AllocateMemorySpace() error
2017-02-24 6:12 [PATCH] UefiCpuPkg/CpuDxe: Do not ASSERT on AllocateMemorySpace() error Jeff Fan
@ 2017-02-24 6:13 ` Tian, Feng
2017-02-24 6:24 ` Zeng, Star
2017-02-24 9:43 ` Laszlo Ersek
2 siblings, 0 replies; 5+ messages in thread
From: Tian, Feng @ 2017-02-24 6:13 UTC (permalink / raw)
To: Fan, Jeff, edk2-devel@lists.01.org
Cc: Laszlo Ersek, Zeng, Star, Kinney, Michael D, Tian, Feng
Reviewed-by: Feng Tian <feng.tian@intel.com>
Thanks
Feng
-----Original Message-----
From: Fan, Jeff
Sent: Friday, February 24, 2017 2:12 PM
To: edk2-devel@lists.01.org
Cc: Laszlo Ersek <lersek@redhat.com>; Zeng, Star <star.zeng@intel.com>; Tian, Feng <feng.tian@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
Subject: [PATCH] UefiCpuPkg/CpuDxe: Do not ASSERT on AllocateMemorySpace() error
Platform PEI may add LOCAL APIC memory mapped space into EFI_HOB_MEMORY_ALLOCATION. Or platform may allocate this range before.
So, we skip AllocateMemorySpace()'s return status checking. Instead, we add one DEBUG message for possible trace.
https://bugzilla.tianocore.org/show_bug.cgi?id=390
This updating is suggested by Ersek's comments at https://www.mail-archive.com/edk2-devel@lists.01.org/msg22585.html
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Feng Tian <feng.tian@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jeff Fan <jeff.fan@intel.com>
---
UefiCpuPkg/CpuDxe/CpuDxe.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/UefiCpuPkg/CpuDxe/CpuDxe.c b/UefiCpuPkg/CpuDxe/CpuDxe.c index 2fd2f31..4a5e282 100644
--- a/UefiCpuPkg/CpuDxe/CpuDxe.c
+++ b/UefiCpuPkg/CpuDxe/CpuDxe.c
@@ -1075,6 +1075,11 @@ AddLocalApicMemorySpace (
Status = AddMemoryMappedIoSpace (BaseAddress, SIZE_4KB, EFI_MEMORY_UC);
ASSERT_EFI_ERROR (Status);
+ //
+ // Try to allocate APIC memory mapped space, does not check return
+ // status because it may be allocated by other driver, or DXE Core if
+ // this range is built into Memory Allocation HOB.
+ //
Status = gDS->AllocateMemorySpace (
EfiGcdAllocateAddress,
EfiGcdMemoryTypeMemoryMappedIo, @@ -1084,7 +1089,10 @@ AddLocalApicMemorySpace (
ImageHandle,
NULL
);
- ASSERT_EFI_ERROR (Status);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_INFO, "%a: %a: AllocateMemorySpace() Status - %r\n",
+ gEfiCallerBaseName, __FUNCTION__, Status)); }
}
/**
--
2.9.3.windows.2
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] UefiCpuPkg/CpuDxe: Do not ASSERT on AllocateMemorySpace() error
2017-02-24 6:12 [PATCH] UefiCpuPkg/CpuDxe: Do not ASSERT on AllocateMemorySpace() error Jeff Fan
2017-02-24 6:13 ` Tian, Feng
@ 2017-02-24 6:24 ` Zeng, Star
2017-02-24 9:43 ` Laszlo Ersek
2 siblings, 0 replies; 5+ messages in thread
From: Zeng, Star @ 2017-02-24 6:24 UTC (permalink / raw)
To: Fan, Jeff, edk2-devel@lists.01.org
Cc: Kinney, Michael D, Tian, Feng, Laszlo Ersek, Zeng, Star
Reviewed-by: Star Zeng <star.zeng@intel.com>
-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Jeff Fan
Sent: Friday, February 24, 2017 2:12 PM
To: edk2-devel@lists.01.org
Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Tian, Feng <feng.tian@intel.com>; Laszlo Ersek <lersek@redhat.com>; Zeng, Star <star.zeng@intel.com>
Subject: [edk2] [PATCH] UefiCpuPkg/CpuDxe: Do not ASSERT on AllocateMemorySpace() error
Platform PEI may add LOCAL APIC memory mapped space into EFI_HOB_MEMORY_ALLOCATION. Or platform may allocate this range before.
So, we skip AllocateMemorySpace()'s return status checking. Instead, we add one DEBUG message for possible trace.
https://bugzilla.tianocore.org/show_bug.cgi?id=390
This updating is suggested by Ersek's comments at https://www.mail-archive.com/edk2-devel@lists.01.org/msg22585.html
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Feng Tian <feng.tian@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jeff Fan <jeff.fan@intel.com>
---
UefiCpuPkg/CpuDxe/CpuDxe.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/UefiCpuPkg/CpuDxe/CpuDxe.c b/UefiCpuPkg/CpuDxe/CpuDxe.c index 2fd2f31..4a5e282 100644
--- a/UefiCpuPkg/CpuDxe/CpuDxe.c
+++ b/UefiCpuPkg/CpuDxe/CpuDxe.c
@@ -1075,6 +1075,11 @@ AddLocalApicMemorySpace (
Status = AddMemoryMappedIoSpace (BaseAddress, SIZE_4KB, EFI_MEMORY_UC);
ASSERT_EFI_ERROR (Status);
+ //
+ // Try to allocate APIC memory mapped space, does not check return
+ // status because it may be allocated by other driver, or DXE Core if
+ // this range is built into Memory Allocation HOB.
+ //
Status = gDS->AllocateMemorySpace (
EfiGcdAllocateAddress,
EfiGcdMemoryTypeMemoryMappedIo, @@ -1084,7 +1089,10 @@ AddLocalApicMemorySpace (
ImageHandle,
NULL
);
- ASSERT_EFI_ERROR (Status);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_INFO, "%a: %a: AllocateMemorySpace() Status - %r\n",
+ gEfiCallerBaseName, __FUNCTION__, Status)); }
}
/**
--
2.9.3.windows.2
_______________________________________________
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] UefiCpuPkg/CpuDxe: Do not ASSERT on AllocateMemorySpace() error
2017-02-24 6:12 [PATCH] UefiCpuPkg/CpuDxe: Do not ASSERT on AllocateMemorySpace() error Jeff Fan
2017-02-24 6:13 ` Tian, Feng
2017-02-24 6:24 ` Zeng, Star
@ 2017-02-24 9:43 ` Laszlo Ersek
2017-02-27 2:22 ` Fan, Jeff
2 siblings, 1 reply; 5+ messages in thread
From: Laszlo Ersek @ 2017-02-24 9:43 UTC (permalink / raw)
To: Jeff Fan, edk2-devel; +Cc: Star Zeng, Feng Tian, Michael D Kinney
On 02/24/17 07:12, Jeff Fan wrote:
> Platform PEI may add LOCAL APIC memory mapped space into
> EFI_HOB_MEMORY_ALLOCATION. Or platform may allocate this range before.
>
> So, we skip AllocateMemorySpace()'s return status checking. Instead, we add one
> DEBUG message for possible trace.
>
> https://bugzilla.tianocore.org/show_bug.cgi?id=390
>
> This updating is suggested by Ersek's comments at
> https://www.mail-archive.com/edk2-devel@lists.01.org/msg22585.html
>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Feng Tian <feng.tian@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jeff Fan <jeff.fan@intel.com>
> ---
> UefiCpuPkg/CpuDxe/CpuDxe.c | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/UefiCpuPkg/CpuDxe/CpuDxe.c b/UefiCpuPkg/CpuDxe/CpuDxe.c
> index 2fd2f31..4a5e282 100644
> --- a/UefiCpuPkg/CpuDxe/CpuDxe.c
> +++ b/UefiCpuPkg/CpuDxe/CpuDxe.c
> @@ -1075,6 +1075,11 @@ AddLocalApicMemorySpace (
> Status = AddMemoryMappedIoSpace (BaseAddress, SIZE_4KB, EFI_MEMORY_UC);
> ASSERT_EFI_ERROR (Status);
>
> + //
> + // Try to allocate APIC memory mapped space, does not check return
> + // status because it may be allocated by other driver, or DXE Core if
> + // this range is built into Memory Allocation HOB.
> + //
> Status = gDS->AllocateMemorySpace (
> EfiGcdAllocateAddress,
> EfiGcdMemoryTypeMemoryMappedIo,
> @@ -1084,7 +1089,10 @@ AddLocalApicMemorySpace (
> ImageHandle,
> NULL
> );
> - ASSERT_EFI_ERROR (Status);
> + if (EFI_ERROR (Status)) {
> + DEBUG ((DEBUG_INFO, "%a: %a: AllocateMemorySpace() Status - %r\n",
> + gEfiCallerBaseName, __FUNCTION__, Status));
> + }
> }
>
> /**
>
Did you actually hit the ASSERT on some platform soon after we discussed
it? :) Either way,
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Thanks!
Laszlo
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] UefiCpuPkg/CpuDxe: Do not ASSERT on AllocateMemorySpace() error
2017-02-24 9:43 ` Laszlo Ersek
@ 2017-02-27 2:22 ` Fan, Jeff
0 siblings, 0 replies; 5+ messages in thread
From: Fan, Jeff @ 2017-02-27 2:22 UTC (permalink / raw)
To: Laszlo Ersek, edk2-devel@ml01.01.org
Cc: Zeng, Star, Tian, Feng, Kinney, Michael D
Yes. Some platform sets memory allocation HOB. It makes sense. So, I remove ASSERT() from the code.
-----Original Message-----
From: Laszlo Ersek [mailto:lersek@redhat.com]
Sent: Friday, February 24, 2017 5:44 PM
To: Fan, Jeff; edk2-devel@ml01.01.org
Cc: Zeng, Star; Tian, Feng; Kinney, Michael D
Subject: Re: [PATCH] UefiCpuPkg/CpuDxe: Do not ASSERT on AllocateMemorySpace() error
On 02/24/17 07:12, Jeff Fan wrote:
> Platform PEI may add LOCAL APIC memory mapped space into
> EFI_HOB_MEMORY_ALLOCATION. Or platform may allocate this range before.
>
> So, we skip AllocateMemorySpace()'s return status checking. Instead,
> we add one DEBUG message for possible trace.
>
> https://bugzilla.tianocore.org/show_bug.cgi?id=390
>
> This updating is suggested by Ersek's comments at
> https://www.mail-archive.com/edk2-devel@lists.01.org/msg22585.html
>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Feng Tian <feng.tian@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jeff Fan <jeff.fan@intel.com>
> ---
> UefiCpuPkg/CpuDxe/CpuDxe.c | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/UefiCpuPkg/CpuDxe/CpuDxe.c b/UefiCpuPkg/CpuDxe/CpuDxe.c
> index 2fd2f31..4a5e282 100644
> --- a/UefiCpuPkg/CpuDxe/CpuDxe.c
> +++ b/UefiCpuPkg/CpuDxe/CpuDxe.c
> @@ -1075,6 +1075,11 @@ AddLocalApicMemorySpace (
> Status = AddMemoryMappedIoSpace (BaseAddress, SIZE_4KB, EFI_MEMORY_UC);
> ASSERT_EFI_ERROR (Status);
>
> + //
> + // Try to allocate APIC memory mapped space, does not check return
> + // status because it may be allocated by other driver, or DXE Core
> + if // this range is built into Memory Allocation HOB.
> + //
> Status = gDS->AllocateMemorySpace (
> EfiGcdAllocateAddress,
> EfiGcdMemoryTypeMemoryMappedIo, @@ -1084,7 +1089,10
> @@ AddLocalApicMemorySpace (
> ImageHandle,
> NULL
> );
> - ASSERT_EFI_ERROR (Status);
> + if (EFI_ERROR (Status)) {
> + DEBUG ((DEBUG_INFO, "%a: %a: AllocateMemorySpace() Status - %r\n",
> + gEfiCallerBaseName, __FUNCTION__, Status));
> + }
> }
>
> /**
>
Did you actually hit the ASSERT on some platform soon after we discussed it? :) Either way,
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Thanks!
Laszlo
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-02-27 2:22 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-02-24 6:12 [PATCH] UefiCpuPkg/CpuDxe: Do not ASSERT on AllocateMemorySpace() error Jeff Fan
2017-02-24 6:13 ` Tian, Feng
2017-02-24 6:24 ` Zeng, Star
2017-02-24 9:43 ` Laszlo Ersek
2017-02-27 2:22 ` Fan, Jeff
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox