From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id E760681ECB for ; Tue, 15 Nov 2016 18:08:45 -0800 (PST) Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by fmsmga103.fm.intel.com with ESMTP; 15 Nov 2016 18:08:50 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.31,497,1473145200"; d="scan'208,217";a="31798816" Received: from fmsmsx107.amr.corp.intel.com ([10.18.124.205]) by fmsmga005.fm.intel.com with ESMTP; 15 Nov 2016 18:08:50 -0800 Received: from shsmsx101.ccr.corp.intel.com (10.239.4.153) by fmsmsx107.amr.corp.intel.com (10.18.124.205) with Microsoft SMTP Server (TLS) id 14.3.248.2; Tue, 15 Nov 2016 18:08:50 -0800 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.239]) by SHSMSX101.ccr.corp.intel.com ([169.254.1.239]) with mapi id 14.03.0248.002; Wed, 16 Nov 2016 10:08:48 +0800 From: "Yao, Jiewen" To: "Yao, Jiewen" , Pete Batard , "Laszlo Ersek" CC: "Kinney, Michael D" , "edk2-devel@lists.01.org" Thread-Topic: [edk2] [PATCH v2 0/3] MdeModulePkg/EbcDxe: add EBC Debugger Thread-Index: AQHSPmd+0iW6zOo0OkKEKt/4TzVJdqDX19eAgAAAroCAABe8AIAAjwuA//9/rACAAIp1EIACVBvg Date: Wed, 16 Nov 2016 02:08:47 +0000 Message-ID: <74D8A39837DF1E4DA445A8C0B3885C50386D1B7C@shsmsx102.ccr.corp.intel.com> References: <637b6291-5132-a825-3207-dbae9b586228@redhat.com> <3fa49493-5d95-6fce-a92e-7c1c73f65a5f@redhat.com> <9f4d4fac-a603-ee66-aad3-e4fcd0a57c8d@akeo.ie> <74D8A39837DF1E4DA445A8C0B3885C50386D0543@shsmsx102.ccr.corp.intel.com> <70ceee92-2fc0-ef63-bbfd-50f4f71cab75@akeo.ie> <74D8A39837DF1E4DA445A8C0B3885C50386D05A3@shsmsx102.ccr.corp.intel.com> In-Reply-To: <74D8A39837DF1E4DA445A8C0B3885C50386D05A3@shsmsx102.ccr.corp.intel.com> Accept-Language: zh-CN, en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 X-Content-Filtered-By: Mailman/MimeDel 2.1.21 Subject: Re: [PATCH v2 0/3] MdeModulePkg/EbcDxe: add EBC Debugger X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 16 Nov 2016 02:08:46 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Hi Pete Maybe you can consider preparing V3, since there is no much other feedback. I forget to mention that: One process we need to do is to run BaseTools\Scr= ipts\PatchCheck.py to check if a patch meets check in criteria. You can find info at https://github.com/tianocore/tianocore.github.io/wiki/= Commit-Message-Format and https://github.com/tianocore/tianocore.github.io/wiki/Code-Style-C I found PatchCheck.py complain a lot for these 3 patches, especially [edk2]= -[PATCH-(RESEND)-v2-2-3]-MdeModulePkg-EbcDxe-add-EBC-Debugger.patch. :) Please run the tool and make sure zero issue reported. If you have any question, feel free to ask please. Thank you Yao Jiewen From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Yao,= Jiewen Sent: Monday, November 14, 2016 10:39 PM To: Pete Batard ; Laszlo Ersek Cc: Kinney, Michael D ; edk2-devel@lists.01.org= Subject: Re: [edk2] [PATCH v2 0/3] MdeModulePkg/EbcDxe: add EBC Debugger 1) Maybe we can wait for a while to see if there is commend from other peop= le. :) 2) I do not we need move all to EbcVmTest.h. As long as the OPCODE/OPRANG are defined in UEFI spec. It is OK to keep it = in MdePkg. We have similar examples, such as ACPI related OPCODE in MdePkg\Include\Ind= ustryStandard\AcpiAml.h And all device path related definition in MdePkg\Include\Protocol\DevicePat= h.h Thank you Yao Jiewen From: Pete Batard [mailto:pete@akeo.ie] Sent: Monday, November 14, 2016 10:11 PM To: Yao, Jiewen >; Laszlo= Ersek > Cc: Kinney, Michael D >; edk2-devel@lists.01.org > Subject: Re: [edk2] [PATCH v2 0/3] MdeModulePkg/EbcDxe: add EBC Debugger On 2016.11.14 13:58, Yao, Jiewen wrote: > One minor concern I have is that: I found you include below in > Protocol/Ebc.h > > #define GETOPERANDS(pVM) (UINT8) (*(UINT8 *) (pVM->Ip + 1)) > #define GETOPCODE(pVM) (UINT8) (*(UINT8 *) pVM->Ip) > #define OPERAND1_REGDATA(pvm, op) pvm->Gpr[OPERAND1_REGNUM (op)] > #define OPERAND2_REGDATA(pvm, op) pvm->Gpr[OPERAND2_REGNUM (op)] > > However, pvm->Gpr and pVM->Ip are defined in Protocol/EbcVmTest.h. > > I recommend that we had better move PVM related definition there. I agree. Do you want me to re-send a patch for that? On that subject, I've also been wondering on whether we might just move all the operand related definitions into EbcVmTest.h and leave Ebc.h as it was. The more I think about it, the more I wonder if a header that's just meant to define the EBC interpreter interface is the best place to have EBC internal definitions. Maybe the header that is intended for use for the actual implementation of the EBC VM (EbcVmTest.h) is where we should move everything into? Regards, /Pete _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel