Kilian,
Thank you very much. Comments below.
From: Kilian Kegel <kilian_ke...@outlook.com>
Sent: Thursday, November 14, 2019 1:24 PM
To: Ni, Ray <ray...@intel.com>; devel@edk2.groups.io; Gao, Liming
<liming....@intel.com>
Cc: Richardson, Brian <brian.richard...@intel.com>; Kinney, Michael D
<michael.d.kin...@intel.com>
Subject: RE: [edk2-devel] [[PATCH 1/1]] EmulatorPkg using CpuBreakPoint
I have no preference at all here. We can also consider to use a stricmp() to be
case-in-sensitive...
[Ray] I am ok to either case sensitive or insensitive.
Thanks,
Kilian
diff --git a/EmulatorPkg/Win/Host/WinHost.c b/EmulatorPkg/Win/Host/WinHost.c
index 62a89f7617..0eeb5979ff 100644
--- a/EmulatorPkg/Win/Host/WinHost.c
+++ b/EmulatorPkg/Win/Host/WinHost.c
@@ -394,6 +394,7 @@ Returns:
UINTN ProcessAffinityMask;
UINTN SystemAffinityMask;
INT32 LowBit;
+ INT32 i;
[Ray] 1. Can you please rename it to "Index" to align the coding style?
//
// Enable the privilege so that RTC driver can successfully run SetTime()
@@ -410,6 +411,26 @@ Returns:
SecPrint ("\n\rEDK II WIN Host Emulation Environment from
http://www.tianocore.org/edk2/\n\r<http://www.tianocore.org/edk2/n/r>");
+ SecPrint
("######################################################################################################\n");
+ SecPrint ("add \"/debug\" command line switch, to connect to the debugger at
the very beginning of POST emulation\n");
+ SecPrint
("######################################################################################################\n");
+ if (Argc > 1) {
[Ray] 2. Can you remove the if check? It's unnecessary because of below
for-loop.
+ for (i = 1; i < Argc; i++) {
+ if (0 == strcmp("/debug", Argv[i])) {
+ SecPrint("\t1. start your debug engine\n");
+ SecPrint("\t2. attach to process WinHost.exe\n");
[Ray] 3. I don't think it's proper to hardcode line number in code because that
will cause this debug message is changed in future.
Can you remove the line number?
+ SecPrint("\t4. SET A SOFTWARE BREAKPOINT (F9) in line 431\n");
+ SecPrint("\t5. and press ENTER in the WinHost command box\n");
+ SecPrint("\t6. go back to the debug engine and RUN/SINGLE STEP the
application\n");
+ SecPrint("\t7. otherwise press enter to continue...\n");
+
+ getchar(); //wait for keyboard input
+
+ SecPrint("");//now you can single step the entire boot/emulation
process, good luck...
+ }
+ }
+ }
+
//
// Determine the first thread available to this process.
//
From: Ni, Ray<mailto:ray...@intel.com>
Sent: Wednesday, November 13, 2019 03:27 AM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Gao,
Liming<mailto:liming....@intel.com>;
kilian_ke...@outlook.com<mailto:kilian_ke...@outlook.com>
Cc: Richardson, Brian<mailto:brian.richard...@intel.com>; Kinney, Michael
D<mailto:michael.d.kin...@intel.com>
Subject: RE: [edk2-devel] [[PATCH 1/1]] EmulatorPkg using CpuBreakPoint
Kilian,
Thank you very much for enabling the better VS debugging experience.
Minor comments below.
From: devel@edk2.groups.io<mailto:devel@edk2.groups.io>
<devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Liming Gao
Sent: Wednesday, November 13, 2019 9:07 AM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>;
kilian_ke...@outlook.com<mailto:kilian_ke...@outlook.com>
Cc: Richardson, Brian
<brian.richard...@intel.com<mailto:brian.richard...@intel.com>>; Kinney,
Michael D <michael.d.kin...@intel.com<mailto:michael.d.kin...@intel.com>>; Gao,
Liming <liming....@intel.com<mailto:liming....@intel.com>>
Subject: Re: [edk2-devel] [[PATCH 1/1]] EmulatorPkg using CpuBreakPoint
Kilian:
Can you create the patch with the commit message?
For the patch, I add my comments.
From: devel@edk2.groups.io<mailto:devel@edk2.groups.io>
[mailto:devel@edk2.groups.io] On Behalf Of Kilian Kegel
Sent: Wednesday, November 06, 2019 1:36 AM
To: Gao, Liming <liming....@intel.com<mailto:liming....@intel.com>>
Cc: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Richardson, Brian
<brian.richard...@intel.com<mailto:brian.richard...@intel.com>>; Kinney,
Michael D <michael.d.kin...@intel.com<mailto:michael.d.kin...@intel.com>>
Subject: [edk2-devel] [[PATCH 1/1]] EmulatorPkg using CpuBreakPoint
Hi Liming,
>Kilian: can you send the patch to edk2 mail list for code review?
Regards,
Kilian
https://bugzilla.tianocore.org/show_bug.cgi?id=2319
diff --git a/EmulatorPkg/Win/Host/WinHost.c b/EmulatorPkg/Win/Host/WinHost.c
index 62a89f7617..53cda86f9a 100644
--- a/EmulatorPkg/Win/Host/WinHost.c
+++ b/EmulatorPkg/Win/Host/WinHost.c
@@ -409,7 +409,30 @@ Returns:
FirmwareVolumesStr = (CHAR16 *) PcdGetPtr (PcdEmuFirmwareVolume);
SecPrint ("\n\rEDK II WIN Host Emulation Environment from
http://www.tianocore.org/edk2/\n\r<http://www.tianocore.org/edk2/n/r>");
-
+
+ if (1) {
Please use TRUE instead of 1
[Ray] 1. Can you remove "if(1)"?
+ int i;
Please define it in the begin of the function, and use edk2 data type and
coding style, such as UINTN Index.
+ SecPrint
("######################################################################################################\n");
+ SecPrint ("add \"/debug\" command line switch, to connect to the debugger
at the very beginning of POST emulation\n");
+ SecPrint
("######################################################################################################\n");
+ if (Argc > 1) {
+ for (i = 1; i < Argc; i++) {
+ if (0 == strcmp("/debug", Argv[i])) {
How about use strncmp?
[Ray] 2. strncmp assumes user may input "/debugxxxxx". Is that a valid
assumption? I thought we only accept "/debug" the whole word as input parameter
to trigger debugging.
Thanks
Liming
+ //SecPrint("IF YOU WANT TO DEBUG from the very beginning of the
EMULATION:\n\t1. start the TASKMGR\n\t2. connect WinHost.exe to the
debugger\n\t3. and press ENTER in this command box\nOR\n");
[Ray] 3. I prefer to break the long string to multiple lines of SecPrint() call
as what the code does in below.
+ SecPrint("\t1. start your debug engine\n");
+ SecPrint("\t2. attache to process WinHost.exe\n");
+ SecPrint("\t4. SET A SOFTWARE BREAKPOINT (F9) in line 431\n");
+ SecPrint("\t5. and press ENTER in the WinHost command box\n");
+ SecPrint("\t6. go back to the debug engine and RUN/SINGLE STEP the
application\n");
+ SecPrint("\t7. otherwise press enter to continue...\n");
+
+ getchar(); //wait for keyboard input
+
+ SecPrint("");//now you can single step the entire boot/emulation
process, good luck...
+ }
+ }
+ }
+ }
//
// Determine the first thread available to this process.
//
From: Kilian Kegel <kilian_ke...@outlook.com<mailto:kilian_ke...@outlook.com>>
Sent: Monday, October 28, 2019 2:01:23 PM
To: liming....@intel.com<mailto:liming....@intel.com>
<liming....@intel.com<mailto:liming....@intel.com>>
Cc: devel@edk2.groups.io<mailto:devel@edk2.groups.io>
<devel@edk2.groups.io<mailto:devel@edk2.groups.io>>; Richardson, Brian
<brian.richard...@intel.com<mailto:brian.richard...@intel.com>>; Kinney,
Michael D <michael.d.kin...@intel.com<mailto:michael.d.kin...@intel.com>>
Subject: [edk2-devel] [edk2] [EmulatorPkg] using __debugbreak()
Hi Liming,
If have observed in newer Windows 10 versions, when using __debugbreak()in any
application that Windows just terminates the app, instead offering to debug it.
So in WinHost.exe too.
That's why I usually insert the code snippet below to run into getchar()
when the program was started with the /debug command line switch.
As long as the App waits for the next keystroke, I can start the debugger
(VS2019)
and connect to WinHost.exe process for debugging.
Do you have a better solution without modifying the source code?
Thanks,
Kilian
if(1){
int i;
SecPrint
("######################################################################################################\n");
SecPrint ("add \"/debug\" command line switch, to connect to the
debugger at the very beginning of POST emulation\n");
SecPrint
("######################################################################################################\n");
if(Argc > 1){
for(i = 1 ; i < Argc ; i++){
if(0 == strcmp("/debug", Argv[i])){
//SecPrint("IF YOU WANT TO DEBUG from the very
beginning of the EMULATION:\n\t1. start the TASKMGR\n\t2. connect WinHost.exe
to the debugger\n\t3. and press ENTER in this command box\nOR\n");
SecPrint("\t1. start Visual Studio\n");
SecPrint("\t2. DEBUG->ATTACH TO PROCESS (CTRL + ALT +
P) --> WinHost.exe\n");
SecPrint("\t3. Break All (CTRL + ALT + Break) -->
WinHost.exe\n");
SecPrint("\t4. SET A SOFTWARE BREAKPOINT (F9) in line
445\n");
SecPrint("\t5. and press ENTER in this command box\n");
SecPrint("\t6. go back to Visual Studio an RUN/SINGLE
STEP the application\n");
SecPrint("\t7. otherwise press enter to continue...\n");
getchar();
//
// 1. SET A SOFTWARE BREAKPOINT TO THE NEXT LINE of
code below -->> SecPrint("");
// 2. switch to the command box and press ENTER
// 3. start single stepping the entire boot/emulation
process, good luck...
//
SecPrint("");//now you can single step the entire
boot/emulation process, good luck...
}
}
}
}
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#50642): https://edk2.groups.io/g/devel/message/50642
Mute This Topic: https://groups.io/mt/42294764/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
