Kilian, Thank you very much for enabling the better VS debugging experience. Minor comments below. From: devel@edk2.groups.io On Behalf Of Liming Gao Sent: Wednesday, November 13, 2019 9:07 AM To: devel@edk2.groups.io; KILIAN_KEGEL@OUTLOOK.COM Cc: Richardson, Brian ; Kinney, Michael D ; Gao, Liming 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] On Behalf Of Kilian Kegel Sent: Wednesday, November 06, 2019 1:36 AM To: Gao, Liming > Cc: devel@edk2.groups.io; Richardson, Brian >; Kinney, Michael D > 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"); - + + 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 > Sent: Monday, October 28, 2019 2:01:23 PM To: liming.gao@intel.com > Cc: devel@edk2.groups.io >; Richardson, Brian >; Kinney, Michael D > 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... } } } }