From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from NAM01-BY2-obe.outbound.protection.outlook.com (mail-by2nam01on0608.outbound.protection.outlook.com [IPv6:2a01:111:f400:fe42::608]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id DEC3F817A8 for ; Thu, 5 Jan 2017 12:48:00 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=amdcloud.onmicrosoft.com; s=selector1-amd-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version; bh=rq9kJCPczpjv66+9RJW3mgHpkHKq6iaLNQe3HCowfxM=; b=OKyuc7kSZu/34XQNq0GsNPpQmE0b4DPWvIv1N6+/iU5f21PLeaGCW9xi+CIeElhBYTg9s/9/G6Rxm4qLc5TDRCI16QWMSDy7ek/0MGshmytWnBr3J0HQ+yk0p+5cQfWj2T/nXjO9qpdonvhahw/O4kzesmUkCFHqLbzu8kY9q3s= Received: from DM5PR12MB1243.namprd12.prod.outlook.com (10.168.237.22) by BLUPR12MB0660.namprd12.prod.outlook.com (10.163.217.30) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P384) id 15.1.817.10; Thu, 5 Jan 2017 20:47:59 +0000 Received: from DM5PR12MB1243.namprd12.prod.outlook.com ([10.168.237.22]) by DM5PR12MB1243.namprd12.prod.outlook.com ([10.168.237.22]) with mapi id 15.01.0817.009; Thu, 5 Jan 2017 20:47:58 +0000 From: "Duran, Leo" To: 'Laszlo Ersek' , "edk2-devel@ml01.01.org" CC: "Singh, Brijesh" , "liming.gao@intel.com" , "michael.d.kinney@intel.com" , "jeff.fan@intel.com" Thread-Topic: [edk2] [PATCH 0/4] *** BaseIoFifoLib *** Thread-Index: AQHSZt9oXVxwiTxBwEyVD0suZXU+DKEqI3IAgAA4hKA= Date: Thu, 5 Jan 2017 20:47:57 +0000 Message-ID: References: <1483571273-11187-1-git-send-email-leo.duran@amd.com> <6f0fe479-96d1-9c86-2a93-27f4f9dc2cef@redhat.com> In-Reply-To: <6f0fe479-96d1-9c86-2a93-27f4f9dc2cef@redhat.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: spf=none (sender IP is ) smtp.mailfrom=leo.duran@amd.com; x-originating-ip: [165.204.77.1] x-microsoft-exchange-diagnostics: 1; BLUPR12MB0660; 7:LbI4B7mdZKUth1xAZ1uGCHYKsupfmGpLA+kdZd5bYit4dw3JKv1posVUenmWBcH/MPtbKbTcjXSrj3kyElz81fwmljCa1KPyM0DrDqDtEVwbugrcfS9wQynzeXReemSN6uHYoumcfMTygVMd5nnD5f1+0OwExi4Lvc2UoQZCaraWVfJaU7S7yUWyH9zPW0XIt0vbhfDt64uYaF/xboSEtLLlUpemTfLM2bxak5YDAYYyWF2NykQM8MEbdpomnkKLgeYvjByjB/m3GWS6wXGQAIhXx83rqU2aBxhgMH46TVamLvxZI8oS8sQGEZ0qs2DHzUpI6ZkpBjVEQ7qCrr5NeXKNSBhjD7S52C9ZU3dYqueOz3kzu88dQfjty0keB6U+YlsHLTh5qaHYm8Z8rX0JRNPDGETrXne4JN76toy4movXO+8PkZEbT/61GdB7q1d0TCQRoxOhUpTHnETXpAbmmQ==; 20:uXHMJJBjd3xHWZlXe6r35gkKFoi1ccsrzbDzSNFx8j7sV0qx65JQR+RIIGfyRnlmc5cBJkxbgV0I9fGbmugnrBO7yI5Ib5Pn8kGK0PDf9JhalJnz1rYwrAeIMhhNde2vCddlFA8g8HedCTeoG6JXZJg+HYH8fnZY0FsfHboQCFDqRT+bgRvEWzHNdEPvYn9C4S/mO58QGMOzR20fQLuoWkOMLiormpfTSt4LX991ps8oGzVtucnNESZDXj7C9uMp x-forefront-antispam-report: SFV:SKI; SCL:-1SFV:NSPM; SFS:(10009020)(6029001)(6009001)(7916002)(39450400003)(39850400002)(39840400002)(39410400002)(39860400002)(199003)(377454003)(51914003)(13464003)(189002)(24454002)(81156014)(7736002)(81166006)(7696004)(2950100002)(33656002)(9686002)(8936002)(101416001)(55016002)(5660300001)(106356001)(76176999)(74316002)(106116001)(105586002)(305945005)(6116002)(66066001)(77096006)(6436002)(25786008)(2906002)(4326007)(229853002)(38730400001)(6506006)(2900100001)(3280700002)(3660700001)(92566002)(68736007)(122556002)(3846002)(102836003)(8676002)(86362001)(2501003)(99286003)(50986999)(54356999)(5001770100001)(189998001)(97736004)(54906002)(19627235001); DIR:OUT; SFP:1101; SCL:1; SRVR:BLUPR12MB0660; H:DM5PR12MB1243.namprd12.prod.outlook.com; FPR:; SPF:None; PTR:InfoNoRecords; A:1; MX:1; LANG:en; x-ms-office365-filtering-correlation-id: b68d1561-3d62-4649-1c19-08d435ac219e x-microsoft-antispam: UriScan:;BCL:0;PCL:0;RULEID:(22001);SRVR:BLUPR12MB0660; x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:(767451399110)(228905959029699); x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(6040375)(601004)(2401047)(8121501046)(5005006)(10201501046)(3002001)(6055026)(6041248)(20161123564025)(20161123555025)(20161123558021)(20161123562025)(20161123560025)(6072148); SRVR:BLUPR12MB0660; BCL:0; PCL:0; RULEID:; SRVR:BLUPR12MB0660; x-forefront-prvs: 0178184651 received-spf: None (protection.outlook.com: amd.com does not designate permitted sender hosts) spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM MIME-Version: 1.0 X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-originalarrivaltime: 05 Jan 2017 20:47:57.8387 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 3dd8961f-e488-4e60-8e11-a82d994e183d X-MS-Exchange-Transport-CrossTenantHeadersStamped: BLUPR12MB0660 Subject: Re: [PATCH 0/4] *** BaseIoFifoLib *** 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: Thu, 05 Jan 2017 20:48:01 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Lazlo, Thanks for the thorough feedback... I just sent out v2. Leo. > -----Original Message----- > From: Laszlo Ersek [mailto:lersek@redhat.com] > Sent: Thursday, January 05, 2017 11:25 AM > To: Duran, Leo ; edk2-devel@ml01.01.org > Cc: Singh, Brijesh ; liming.gao@intel.com; > michael.d.kinney@intel.com; jeff.fan@intel.com > Subject: Re: [edk2] [PATCH 0/4] *** BaseIoFifoLib *** >=20 > On 01/05/17 00:07, Leo Duran wrote: > > The UefiCpuPkg/CpuIo2Dxe driver and the QemuCfgLib library have > > duplicate implementations of I/O Fifo routines. The patch series moves > > the I/O Fifo routines into a common BaseIofifoLib library supporting > > IA32 and X64 architectures under MdePkg. > > > > The intent of this patch series is twofold: > > 1) Consolidate I/O Fifo routines into a common BaseIofifoLib library. > > 2) Allow override of BaseIofifoLib for specific platform implementation= s. > > For example, the OVMF package can provide its own version of > > BaseIoFifoLib to support Secure Encrypted Virtualization (SEV) guests, > > since SEV does not support string I/O instructions (rep ins/outs), and > > requires unrolled loops of single in/out instructions. >=20 > General comments: >=20 > (1) Please rebase the series, picking "edit" for each patch, and re-commi= t > each patch (without any code changes) with: >=20 > git commit --amend --author=3D'Brijesh Singh ' >=20 > This will cause the formatted patch emails to start with >=20 > From: Brijesh Singh >=20 > and Brijesh's authorship will be then correctly reflected in git metadata= after > applying the patches with "git am". >=20 > (I'm basing this on the Signed-off-by tags.) >=20 > (2) I think your S-o-b should also be present, at the bottom. >=20 > (3) If you updated the patches after getting them from Brijesh (i.e., you > aren't merely forwarding them), then I believe you should drop your > Reviewed-by. >=20 > (4) The new files shouldn't carry just Intel's copyright notice -- they a= re new > files, so they should get AMD's too. (Some of the new files do this, some > don't.) >=20 > Comments related to virt: >=20 > (5) OVMF now supports (and prefers) the DMA-like interface of fw_cfg > (available in newer QEMU releases), which doesn't use port IO at all; so = in > practice the REPs should cause problems under SEV, when used in > QemuFwCfgLib. >=20 > Nonetheless, I do agree that code deduplication is a good thing! >=20 > (6) Single IO port instructions in CpuIo2Dxe will slow down emulated IDE > transfers considerably. (I'm not holding this against the lib class extra= ction or > SEV in general. We should simply acknowledge this as another reason to > default to virtio-blk or virtio-scsi in VM configs.) >=20 > Thanks! > Laszlo >=20 >=20 > > > > Leo Duran (4): > > MdePkg: Add BaseIoFifoLib library > > Modify .DSC files that include UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf > > Modify UefiCpuPkg/CpuIo2Dxe to use new BaseIoFifoLib library. > > Modify QemuFwCfgLib to use new BaseIoFifoLib library. > > > > CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc | 1 + > > CorebootPayloadPkg/CorebootPayloadPkgIa32X64.dsc | 1 + > > DuetPkg/DuetPkgIa32.dsc | 1 + > > DuetPkg/DuetPkgX64.dsc | 1 + > > MdePkg/Include/Library/IoFifoLib.h | 175 > ++++++++++++++++++++++ > > MdePkg/Library/BaseIoFifoLib/BaseIoFifoLib.inf | 45 ++++++ > > MdePkg/Library/BaseIoFifoLib/Ia32/IoFifo.asm | 139 > +++++++++++++++++ > > MdePkg/Library/BaseIoFifoLib/Ia32/IoFifo.nasm | 135 > +++++++++++++++++ > > MdePkg/Library/BaseIoFifoLib/X64/IoFifo.asm | 125 > ++++++++++++++++ > > MdePkg/Library/BaseIoFifoLib/X64/IoFifo.nasm | 124 > +++++++++++++++ > > MdePkg/MdePkg.dec | 3 + > > MdePkg/MdePkg.dsc | 1 + > > OvmfPkg/Library/QemuFwCfgLib/Ia32/IoLibExAsm.nasm | 55 ------- > > OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c | 1 + > > OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.inf | 7 +- > > OvmfPkg/Library/QemuFwCfgLib/X64/IoLibExAsm.nasm | 52 ------- > > OvmfPkg/OvmfPkgIa32.dsc | 1 + > > OvmfPkg/OvmfPkgIa32X64.dsc | 1 + > > OvmfPkg/OvmfPkgX64.dsc | 1 + > > QuarkPlatformPkg/Quark.dsc | 1 + > > QuarkPlatformPkg/QuarkMin.dsc | 1 + > > UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.c | 1 - > > UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.h | 1 + > > UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf | 10 +- > > UefiCpuPkg/CpuIo2Dxe/Ia32/IoFifo.asm | 140 --------------= --- > > UefiCpuPkg/CpuIo2Dxe/Ia32/IoFifo.nasm | 136 --------------= --- > > UefiCpuPkg/CpuIo2Dxe/X64/IoFifo.asm | 126 --------------= -- > > UefiCpuPkg/CpuIo2Dxe/X64/IoFifo.nasm | 125 --------------= -- > > UefiCpuPkg/UefiCpuPkg.dsc | 1 + > > Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc | 1 + > > Vlv2TbltDevicePkg/PlatformPkgIA32.dsc | 1 + > > Vlv2TbltDevicePkg/PlatformPkgX64.dsc | 1 + > > 32 files changed, 764 insertions(+), 650 deletions(-) create mode > > 100644 MdePkg/Include/Library/IoFifoLib.h > > create mode 100644 MdePkg/Library/BaseIoFifoLib/BaseIoFifoLib.inf > > create mode 100644 MdePkg/Library/BaseIoFifoLib/Ia32/IoFifo.asm > > create mode 100644 MdePkg/Library/BaseIoFifoLib/Ia32/IoFifo.nasm > > create mode 100644 MdePkg/Library/BaseIoFifoLib/X64/IoFifo.asm > > create mode 100644 MdePkg/Library/BaseIoFifoLib/X64/IoFifo.nasm > > delete mode 100644 > OvmfPkg/Library/QemuFwCfgLib/Ia32/IoLibExAsm.nasm > > delete mode 100644 > OvmfPkg/Library/QemuFwCfgLib/X64/IoLibExAsm.nasm > > delete mode 100644 UefiCpuPkg/CpuIo2Dxe/Ia32/IoFifo.asm > > delete mode 100644 UefiCpuPkg/CpuIo2Dxe/Ia32/IoFifo.nasm > > delete mode 100644 UefiCpuPkg/CpuIo2Dxe/X64/IoFifo.asm > > delete mode 100644 UefiCpuPkg/CpuIo2Dxe/X64/IoFifo.nasm > >