From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from EUR02-AM5-obe.outbound.protection.outlook.com (mail-am5eur02on0603.outbound.protection.outlook.com [IPv6:2a01:111:f400:fe07::603]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id A5E2A82192 for ; Tue, 20 Dec 2016 05:20:57 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=armh.onmicrosoft.com; s=selector1-arm-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version; bh=ENQdAbFdvkqtke7HugQ/IAXzNDa1FsXWN+5hcY9xr7o=; b=ngY8Ku28e74iTuGq3Rws89QOz+ERy4pU7GxAn7J311S7wBi76+rpSTiZ+k7S2JsIzzwrEWnJEnvI9femrD8n60uW9h/8pfu9zD/XL+zihssDZ6tQ6WEPgynY82PHrO/9j4cmqf4vUtQJSUzCEgymLWQCURT5GxH1DVsLLDf/DKU= Received: from AM5PR0801MB1762.eurprd08.prod.outlook.com (10.169.247.16) by DB6PR0802MB2504.eurprd08.prod.outlook.com (10.172.251.138) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P384) id 15.1.789.14; Tue, 20 Dec 2016 13:20:54 +0000 Received: from AM5PR0801MB1762.eurprd08.prod.outlook.com ([10.169.247.16]) by AM5PR0801MB1762.eurprd08.prod.outlook.com ([10.169.247.16]) with mapi id 15.01.0771.020; Tue, 20 Dec 2016 13:20:54 +0000 From: Evan Lloyd To: "Yao, Jiewen" , "Ni, Ruiyu" , "Carsey, Jaben" , Sami Mujawar CC: "Carsey, Jaben" , "edk2-devel@ml01.01.org" , Leif Lindholm , "ard.biesheuvel@linaro.org" Thread-Topic: [edk2] [PATCH] ShellPkg: Add acpiview tool to dump ACPI tables Thread-Index: AQHSV8nfzRASeiOrb0O70tD1DSK8y6EPCpYAgABWn6CAADMGgIAAllIAgAAP3gCAAA7pAIAAB/cAgAAKI4CAAG6acA== Date: Tue, 20 Dec 2016 13:20:54 +0000 Message-ID: References: <20161216182547.616-1-evan.lloyd@arm.com> <734D49CCEBEEF84792F5B80ED585239D5B836C04@SHSMSX103.ccr.corp.intel.com> <734D49CCEBEEF84792F5B80ED585239D5B8371E1@SHSMSX103.ccr.corp.intel.com> <74D8A39837DF1E4DA445A8C0B3885C503A8C6C52@shsmsx102.ccr.corp.intel.com> <734D49CCEBEEF84792F5B80ED585239D5B839479@SHSMSX103.ccr.corp.intel.com> <74D8A39837DF1E4DA445A8C0B3885C503A8C6CBD@shsmsx102.ccr.corp.intel.com> In-Reply-To: <74D8A39837DF1E4DA445A8C0B3885C503A8C6CBD@shsmsx102.ccr.corp.intel.com> Accept-Language: en-GB, en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: spf=none (sender IP is ) smtp.mailfrom=Evan.Lloyd@arm.com; x-originating-ip: [217.140.96.140] x-ld-processed: f34e5979-57d9-4aaa-ad4d-b122a662184d,ExtAddr x-ms-office365-filtering-correlation-id: 6fccbf01-6d5c-4cb1-fc41-08d428db06ea x-microsoft-antispam: UriScan:; BCL:0; PCL:0; RULEID:(22001); SRVR:DB6PR0802MB2504; x-microsoft-exchange-diagnostics: 1; DB6PR0802MB2504; 7:cNOu5jm1v69qstMJ+4TpcnPumx/qKhkn+Ycmg7bxjvKebZaJE8mzkHz5H0LCtKymsSgs7VFyOvDmNho9aCOXCo2B3/J4sKcAkSH2r8t4yVV4PLo3Ciy5KHP9y44IpLJXy7MZJ/NYCEVsmPy5kOy09Rk2J9pEo0qaDidLSt/VS9Lx0/OuLC248cX9b9FiK02Y1SxXNA5TwJz0Qqt5pTkLsGw26ZksXkiXy3GCHIHVQ7H95BRAc57IJALmyAel05L18h6qVXaE4OFHtL/46i0COP8qvzQy9qnjVAoEKj54ZTYmqyzBATqq4IByuHXv+G6zpxIhkDp8Rey95UvFFQJVgTIQUh79RF3jQHa9Jm/l9pzpzCYP302YeuXjby1cn5TOp0EqhKhYmMM8Jsr5YUIKrhuLYsnGL71D6Qr8Zrxv5ejzHL+zFd979BourPrIldLO5TIdjXiIol6kQujQY+XVOA== x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:(228905959029699); x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(6040375)(601004)(2401047)(8121501046)(5005006)(3002001)(10201501046)(6055026)(6041248)(20161123560025)(20161123564025)(20161123562025)(20161123555025)(6072148); SRVR:DB6PR0802MB2504; BCL:0; PCL:0; RULEID:; SRVR:DB6PR0802MB2504; x-forefront-prvs: 0162ACCC24 x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(6009001)(7916002)(39840400002)(39850400002)(39860400002)(39450400003)(39410400002)(199003)(40434004)(189002)(93886004)(92566002)(2906002)(4326007)(54356999)(50986999)(9686002)(76576001)(2900100001)(101416001)(3280700002)(76176999)(3660700001)(68736007)(33656002)(7736002)(305945005)(74316002)(105586002)(38730400001)(106116001)(86362001)(102836003)(106356001)(3846002)(6436002)(6506006)(77096006)(6116002)(8676002)(81166006)(25786008)(189998001)(229853002)(5660300001)(122556002)(6636002)(66066001)(7696004)(8936002)(2950100002)(5890100001)(97736004)(81156014)(5001770100001); DIR:OUT; SFP:1101; SCL:1; SRVR:DB6PR0802MB2504; H:AM5PR0801MB1762.eurprd08.prod.outlook.com; FPR:; SPF:None; PTR:InfoNoRecords; A:1; MX:1; LANG:en; received-spf: None (protection.outlook.com: arm.com does not designate permitted sender hosts) spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM MIME-Version: 1.0 X-OriginatorOrg: arm.com X-MS-Exchange-CrossTenant-originalarrivaltime: 20 Dec 2016 13:20:54.4193 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: f34e5979-57d9-4aaa-ad4d-b122a662184d X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB6PR0802MB2504 Subject: Re: [PATCH] ShellPkg: Add acpiview tool to dump ACPI tables 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: Tue, 20 Dec 2016 13:20:58 -0000 Content-Language: en-US Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Hi Jiewen. Some responses inline below: > From: Yao, Jiewen [mailto:jiewen.yao@intel.com] > Sent: 20 December 2016 05:54 > To: Ni, Ruiyu; Carsey, Jaben; Evan Lloyd > Cc: Carsey, Jaben; edk2-devel@ml01.01.org; Leif Lindholm; ard.biesheuvel@= linaro.org > Subject: RE: [edk2] [PATCH] ShellPkg: Add acpiview tool to dump ACPI tabl= es > > I like this idea - Shell.efi should only support commands defined by Shel= l spec. > Then we should not port code to be a shell library. > > I have concern on ShellPkg\Library\UefiDpLib, because DP depends on Timer= Lib and TimerLib is a platform specific library. > I do not object the idea to move DP from performancepkg to shellPkg. > I would suggest DP had better be a standard shell application, rather tha= n a shell lib and included in a shell bin. > > > Now back to acpiview: > First, it is great to have such tool in UEFI shell. Thanks. We hoped people would see the benefit. > > Second, I found the function is incomplete. > 1) Some ACPI defined tables are not complete. Take MADT as example,= it only supports EFI_ACPI_6_1_GIC/EFI_ACPI_6_1_GICD/EFI_ACPI_6_1_GIC_MSI_F= RAME/EFI_ACPI_6_1_GICR/EFI_ACPI_6_1_GIC_ITS. > 2) Some ACPI defined tables are missing. Such as BGRT, FPDT. At this stage we have only added tables for which we had an immediate use, = as available effort is limited. In addition, we did not want to submit code we were not able to test. Clea= rly, our platforms use GIC, not APIC. Also we are not well placed to judge the best representation for some fiel= ds, e.g. are APIC interrupt numbers normally decimal or hex? > 3) I found some ACPI reserved tables are added here. Such as DBG2, = IORT. Is there any problem with dumping the reserved tables? > 4) But not all ACPI reserved tables are added. Such as HPET, TPM2. We only wrote code for tables we could test. We have no example hardware/f= irmware with those tables. > May I know what criteria we are using to select which feature is supporte= d or not supported? As stated above, we only provided the tables/sections that we needed to dum= p and could test. >>From the commit message: """ Many tables are not explicitly handled, in part because no examples are ava= ilable for our testing. The program is designed to be extended to new tables with minimal effort, a= nd contributions are invited. """ > Or it is just something from start and still need some work to make it co= mplete? Yes, this is just a starting point and very far from complete. We found it detected a number of problems for us, so we felt it worth contr= ibuting. If we can get people to use it, that may mean fewer problems for us to debu= g. ;-) > > Third, I found we need a long ? ?switch (*AcpiTableSignature) case? to du= mp/parse ACPI table specific function. > Do you think it will be better, that we provide a register function to le= t each ACPI table register a parser? > For example: RegisterDumpAcpiTable (EFI_ACPI_1_0_APIC_SIGNATURE, DumpAcpi= MADT); > Then the core logic just need use a table-driven way to find out the corr= esponding parser function by a SIGNATURE. You are right, that would be a much better abstract design, and would make = adding a table simpler. We discussed this in our pre-release review. The trade-off is that you need to maintain a dynamic registry of handlers, = and write your own lookup code. Using a switch eliminates the registry, and relies on the compiler for effi= cient look up. We would be very happy with this change, should someone have the time avail= able to contribute it. Our review also suggested changing the switch to just set a function pointe= r, with a single call at the end. That would make each switch case much simpler. Would that be an acceptable= "half-way house"? > > Finally, I agree with Ruiyu that a standalone tool might be more proper. Agreed in other response. > > Thank you > Yao Jiewen >> > > ... > > > >> Guid("CE165669-3EF3-493F-B85D-6190EE5B9759") IMPORTANT NOTICE: The contents of this email and any attachments are confid= ential and may also be privileged. If you are not the intended recipient, p= lease notify the sender immediately and do not disclose the contents to any= other person, use it for any purpose, or store or copy the information in = any medium. Thank you.