From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga18.intel.com (mga18.intel.com [134.134.136.126]) by mx.groups.io with SMTP id smtpd.web12.1806.1573612069889312818 for ; Tue, 12 Nov 2019 18:27:50 -0800 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 134.134.136.126, mailfrom: ray.ni@intel.com) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga106.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 12 Nov 2019 18:27:49 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.68,298,1569308400"; d="scan'208,217";a="216239165" Received: from fmsmsx108.amr.corp.intel.com ([10.18.124.206]) by orsmga002.jf.intel.com with ESMTP; 12 Nov 2019 18:27:48 -0800 Received: from fmsmsx125.amr.corp.intel.com (10.18.125.40) by FMSMSX108.amr.corp.intel.com (10.18.124.206) with Microsoft SMTP Server (TLS) id 14.3.439.0; Tue, 12 Nov 2019 18:27:48 -0800 Received: from shsmsx105.ccr.corp.intel.com (10.239.4.158) by FMSMSX125.amr.corp.intel.com (10.18.125.40) with Microsoft SMTP Server (TLS) id 14.3.439.0; Tue, 12 Nov 2019 18:27:48 -0800 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.127]) by SHSMSX105.ccr.corp.intel.com ([169.254.11.225]) with mapi id 14.03.0439.000; Wed, 13 Nov 2019 10:27:42 +0800 From: "Ni, Ray" To: "devel@edk2.groups.io" , "Gao, Liming" , "KILIAN_KEGEL@OUTLOOK.COM" CC: "Richardson, Brian" , "Kinney, Michael D" Subject: Re: [edk2-devel] [[PATCH 1/1]] EmulatorPkg using CpuBreakPoint Thread-Topic: [edk2-devel] [[PATCH 1/1]] EmulatorPkg using CpuBreakPoint Thread-Index: AQHVk/+REEaxIo9uCEeRSmOwJIoTnKeHzyCAgACbP3A= Date: Wed, 13 Nov 2019 02:27:42 +0000 Message-ID: <734D49CCEBEEF84792F5B80ED585239D5C35B652@SHSMSX104.ccr.corp.intel.com> References: <2d5ec4d308504c87b7b1390c1e2f1495@zhaoxin.com>,<4A89E2EF3DFEDB4C8BFDE51014F606A14E5325C1@SHSMSX104.ccr.corp.intel.com>, <4A89E2EF3DFEDB4C8BFDE51014F606A14E53EAA0@SHSMSX104.ccr.corp.intel.com> In-Reply-To: <4A89E2EF3DFEDB4C8BFDE51014F606A14E53EAA0@SHSMSX104.ccr.corp.intel.com> Accept-Language: en-US, zh-CN X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Return-Path: ray.ni@intel.com Content-Language: en-US Content-Type: multipart/alternative; boundary="_000_734D49CCEBEEF84792F5B80ED585239D5C35B652SHSMSX104ccrcor_" --_000_734D49CCEBEEF84792F5B80ED585239D5C35B652SHSMSX104ccrcor_ Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable 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 <= brian.richardson@intel.com>; Kinney, Mic= hael 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=3D2319 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 =3D (CHAR16 *) PcdGetPtr (PcdEmuFirmwareVolume); SecPrint ("\n\rEDK II WIN Host Emulation Environment from http://www.ti= anocore.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 debu= gger at the very beginning of POST emulation\n"); + SecPrint ("##########################################################= ############################################\n"); + if (Argc > 1) { + for (i =3D 1; i < Argc; i++) { + if (0 =3D=3D strcmp("/debug", Argv[i])) { How about use strncmp? [Ray] 2. strncmp assumes user may input "/debugxxxxx". Is that a valid ass= umption? I thought we only accept "/debug" the whole word as input paramete= r 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 debugg= er\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/emulatio= n 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 debu= g 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 d= ebugger at the very beginning of POST emulation\n"); SecPrint ("#######################################################= ###############################################\n"); if(Argc > 1){ for(i =3D 1 ; i < Argc ; i++){ if(0 =3D=3D strcmp("/debug", Argv[i])){ //SecPrint("IF YOU WANT TO DEBUG from the very beg= inning 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 + AL= T + P) --> WinHost.exe\n"); SecPrint("\t3. Break All (CTRL + ALT + Break) -->= WinHost.exe\n"); SecPrint("\t4. SET A SOFTWARE BREAKPOINT (F9) in l= ine 445\n"); SecPrint("\t5. and press ENTER in this command box= \n"); SecPrint("\t6. go back to Visual Studio an RUN/SIN= GLE STEP the application\n"); SecPrint("\t7. otherwise press enter to continue..= .\n"); getchar(); // // 1. SET A SOFTWARE BREAKPOINT TO THE NEXT LINE o= f code below -->> SecPrint(""); // 2. switch to the command box and press ENTER // 3. start single stepping the entire boot/emulat= ion process, good luck... // SecPrint("");//now you can single step the entire = boot/emulation process, good luck... } } } } --_000_734D49CCEBEEF84792F5B80ED585239D5C35B652SHSMSX104ccrcor_ Content-Type: text/html; charset="us-ascii" Content-Transfer-Encoding: quoted-printable

Kilian,

Thank you very much for enabling the better VS debu= gging experience.

 

Minor comments below.

 

From: devel@edk2.groups.io <devel@edk2.gr= oups.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 <brian.richardson@intel.com>; Kinney, M= ichael D <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@int= el.com>
Subject: Re: [edk2-devel] [[PATCH 1/1]] EmulatorPkg using CpuBreakP= oint

 

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 <liming.= gao@intel.com>
Cc: devel@edk2.groups.io; Richardson, Brian <bria= n.richardson@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
Subject: [edk2-devel] [[PATCH 1/1]] EmulatorPkg using CpuBreakPoint=

 

Hi Liming,

>Kilian: can you send the patch to edk2 mail lis= t for code review?

Regards,

Kilian

 

https://bugzilla.tianocore.org/show_bug.cgi?id=3D2319

 

diff --git a/EmulatorPkg/Win/Host/WinHost.c b/Emula= torPkg/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 =3D (CHAR16 *) PcdG= etPtr (PcdEmuFirmwareVolume);

   SecPrint ("\n\rEDK II WIN Host Em= ulation Environment from http://www.tianocore.org/edk= 2/\n\r");

-

+ 

+  if (1) {

Please use TRUE inste= ad of 1

[Ray] 1. Can you remove “if(1)”?

 

+    int i;

Please define it in t= he 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 b= eginning of POST emulation\n");

+    SecPrint ("###########= ###########################################################################= ################\n");

+    if (Argc > 1) {

+      for (i =3D 1; i= < Argc; i++) {

+        if = (0 =3D=3D strcmp("/debug", Argv[i])) {

How about use strncmp= ?

[Ray] 2. strncmp assumes user may input “/deb= ugxxxxx”. Is that a valid assumption? I thought we only accept “= ;/debug” the whole word as input parameter to trigger debugging.=

 

Thanks

Liming

+       &nbs= p;  //SecPrint("IF YOU WANT TO DEBUG from the very beginning of t= he EMULATION:\n\t1. start the TASKMGR\n\t2. connect WinHost.exe to the debu= gger\n\t3. and press ENTER in this command box\nOR\n");

[Ray] 3. I prefer to break the long string to multi= ple lines of SecPrint() call as what the code does in below.

 

+       &nbs= p;  SecPrint("\t1. start your debug engine\n");

+       &nbs= p;  SecPrint("\t2. attache to process WinHost.exe\n");<= /o:p>

+       &nbs= p;  SecPrint("\t4. SET A SOFTWARE BREAKPOINT (F9) in line 431\n&q= uot;);

+       &nbs= p;  SecPrint("\t5. and press ENTER in the WinHost command box\n&q= uot;);

+       &nbs= p;  SecPrint("\t6. go back to the debug engine and RUN/SINGLE STE= P the application\n");

+       &nbs= p;  SecPrint("\t7. otherwise press enter to continue...\n");=

+

+       &nbs= p;  getchar();    //wait for keyboard input<= /p>

+

+       &nbs= p;  SecPrint("");//now you can single step the entire boot/e= mulation process, good luck...

+        }

+      }

+    }

+  }

   //

   // Determine the first thread availabl= e to this process.

   //

 

From: Kilian Kegel <kilian_kegel@outlook.com>
Sent: Monday, October 28, 2019 2:01:23 PM
To: liming.gao@intel.com <liming.gao@intel.com><= br> Cc: devel@edk2.groups.io <devel@edk2.groups.io>;= Richardson, Brian <brian.= richardson@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
Subject: [edk2-devel] [edk2] [EmulatorPkg] using __debugbreak()

 

Hi Liming,

 

If have observed in newer Windows 10 versions, whe= n 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 comma= nd 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.<= o:p>

 

Do you have a better solution without modifying th= e source code?

 

Thanks,

Kilian

    if(1){

        int = i;

 

        SecP= rint ("###############################################################= #######################################\n");

        SecP= rint ("add \"/debug\" command line switch, to connect to the= debugger at the very beginning of POST emulation\n");<= /p>

        SecP= rint ("###############################################################= #######################################\n");

 

        if(A= rgc > 1){

        = ;    for(i =3D 1 ; i < Argc ; i++){

        = ;        if(0 =3D=3D strcmp("/debug= ", Argv[i])){

        = ;            &n= bsp;   //SecPrint("IF YOU WANT TO DEBUG from the very beginn= ing 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");

        = ;            &n= bsp;   SecPrint("\t1. start Visual Studio\n");=

        = ;            &n= bsp;   SecPrint("\t2. DEBUG->ATTACH TO PROCESS (CTRL += ; ALT + P) --> WinHost.exe\n");

        = ;            &n= bsp;   SecPrint("\t3. Break All  (CTRL + ALT + = Break) --> WinHost.exe\n");

        = ;             &= nbsp;  SecPrint("\t4. SET A SOFTWARE BREAKPOINT (F9) in line 445\= n");

        = ;            &n= bsp;   SecPrint("\t5. and press ENTER in this command box\n&= quot;);

        = ;            &n= bsp;   SecPrint("\t6. go back to Visual Studio an RUN/SINGLE= STEP the application\n");

        = ;            &n= bsp;   SecPrint("\t7. otherwise press enter to continue...\n= ");

        = ;            &n= bsp;   getchar();

        = ;            &n= bsp;   //

        = ;            &n= bsp;   // 1. SET A SOFTWARE BREAKPOINT TO THE NEXT LINE of code b= elow -->> SecPrint("");

        = ;            &n= bsp;   // 2. switch to the command box and press ENTER

        = ;            &n= bsp;   // 3. start single stepping the entire boot/emulation proc= ess, good luck...

        = ;            &n= bsp;   //

        = ;            &n= bsp;   SecPrint("");//now you can single step the entir= e boot/emulation process, good luck...

        = ;           }

        = ;    }

        }

    }

--_000_734D49CCEBEEF84792F5B80ED585239D5C35B652SHSMSX104ccrcor_--