From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) (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 3237A81CB8 for ; Mon, 14 Nov 2016 06:39:07 -0800 (PST) Received: from orsmga003.jf.intel.com ([10.7.209.27]) by fmsmga101.fm.intel.com with ESMTP; 14 Nov 2016 06:39:11 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.31,638,1473145200"; d="scan'208,217";a="901192384" Received: from fmsmsx106.amr.corp.intel.com ([10.18.124.204]) by orsmga003.jf.intel.com with ESMTP; 14 Nov 2016 06:39:11 -0800 Received: from fmsmsx153.amr.corp.intel.com (10.18.125.6) by FMSMSX106.amr.corp.intel.com (10.18.124.204) with Microsoft SMTP Server (TLS) id 14.3.248.2; Mon, 14 Nov 2016 06:39:10 -0800 Received: from shsmsx151.ccr.corp.intel.com (10.239.6.50) by FMSMSX153.amr.corp.intel.com (10.18.125.6) with Microsoft SMTP Server (TLS) id 14.3.248.2; Mon, 14 Nov 2016 06:39:10 -0800 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.239]) by SHSMSX151.ccr.corp.intel.com ([169.254.3.96]) with mapi id 14.03.0248.002; Mon, 14 Nov 2016 22:39:06 +0800 From: "Yao, Jiewen" To: 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/rACAAIp1EA== Date: Mon, 14 Nov 2016 14:39:06 +0000 Message-ID: <74D8A39837DF1E4DA445A8C0B3885C50386D05A3@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> In-Reply-To: <70ceee92-2fc0-ef63-bbfd-50f4f71cab75@akeo.ie> 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: Mon, 14 Nov 2016 14:39:07 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable 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