From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=216.205.24.162; helo=us-smtp-delivery-162.mimecast.com; envelope-from=eugene@hp.com; receiver=edk2-devel@lists.01.org Received: from us-smtp-delivery-162.mimecast.com (us-smtp-delivery-162.mimecast.com [216.205.24.162]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id ADDA2211CFFDB for ; Thu, 28 Feb 2019 11:56:34 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=hp.com; s=mimecast20180716; t=1551383793; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=XXU5ThvlcttHWFzrrJ3tmRnMIIn6PGiBLm07WzDD9HY=; b=YXLEw3RISADxI/zRfyTtI4LhsJSKjX97FNX221z3oPu8MV3+8xdo4BJYKFlTxue8bJSRbnhkc8HeRZo1sCUWWwxSlCP09dOxc1t80DRdbuFJNi15pPzsXJUGNnfiiSVvXyqJaynz5cZhU/MrDqALtKvpZ/ZDQrfuOmNFQCT0uao= Received: from NAM03-CO1-obe.outbound.protection.outlook.com (mail-co1nam03lp2057.outbound.protection.outlook.com [104.47.40.57]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-77-TMaEay_KP9uVpZNvTimW_g-1; Thu, 28 Feb 2019 14:56:31 -0500 Received: from CS1PR8401MB1189.NAMPRD84.PROD.OUTLOOK.COM (10.169.97.20) by CS1PR8401MB0600.NAMPRD84.PROD.OUTLOOK.COM (10.169.13.142) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.1665.15; Thu, 28 Feb 2019 19:56:29 +0000 Received: from CS1PR8401MB1189.NAMPRD84.PROD.OUTLOOK.COM ([fe80::ec9d:c9c3:8a92:4378]) by CS1PR8401MB1189.NAMPRD84.PROD.OUTLOOK.COM ([fe80::ec9d:c9c3:8a92:4378%4]) with mapi id 15.20.1643.019; Thu, 28 Feb 2019 19:56:29 +0000 From: "Cohen, Eugene" To: Ashish Singhal , "Wu, Hao A" , "edk2-devel@lists.01.org" Thread-Topic: [PATCH] MdeModulePkg/SdMmcPciHcDxe: Fix DMA on SDHC v3 64-bit systems Thread-Index: AdTOimUh6bq74L7bQyCZsF0hnADHfgAhuWlQABE+2EAAEKP68AABVU+w Date: Thu, 28 Feb 2019 19:56:28 +0000 Message-ID: References: In-Reply-To: Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [15.65.252.14] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: 8293ab8f-3ed0-4e27-9455-08d69db6d441 x-microsoft-antispam: BCL:0; PCL:0; RULEID:(2390118)(7020095)(4652040)(8989299)(4534185)(4627221)(201703031133081)(201702281549075)(8990200)(5600127)(711020)(4605104)(2017052603328)(7153060)(7193020); SRVR:CS1PR8401MB0600; x-ms-traffictypediagnostic: CS1PR8401MB0600: x-ms-exchange-purlcount: 1 x-microsoft-exchange-diagnostics: =?iso-8859-1?Q?1; CS1PR8401MB0600; 23:9D/anYhXA9VOesfTGYru/3JfoFTEM4a1esLHw?= =?iso-8859-1?Q?nwkvd1n6d8xIxnWovmA54JrMfjWVJ4Pvw3/fglYxNuyNzdh+bIUhta939i?= =?iso-8859-1?Q?lf2Bp4SY3vgro8NHiaziFJG1aATL0CYsrk96gpvEOU+61ML65RDl/HIfWa?= =?iso-8859-1?Q?0QiYYuDLRXNzSc/J0OCVx8ucPzmx74unlOsWsg27oH0jzCqobKWnyaP0+L?= =?iso-8859-1?Q?Azwb0H5VYah8J/61KCGk5vb9GVXLB8jHbs0eT9NZQIRehhTIIdI3ntwjVs?= =?iso-8859-1?Q?JGf/SUdYCiIULMTepfheUh+hin2JoFzUhA+erVevxe9yJu4LR20+Mgjdkh?= =?iso-8859-1?Q?qpy2Umzc8dHGCWTVlLHRjHMbGk5GUhXgQl9pZyA5AaopUVxHPyepHsY0Tn?= =?iso-8859-1?Q?Tr4p4f8Bbe9537fITpLYSYoA4AKFUD0Pt/edASOAE75CuO0PrMF5Nmasd0?= =?iso-8859-1?Q?H187FUsIdIK9dmeWBMDEos7GMRkO8a4c/r9PLA/61mxIJGTeJ3r3xX8v5F?= =?iso-8859-1?Q?cfCHfoxznIl3NptCvWPctvINgqqLFHxXi8JiR5jMO/SJYbaDXsa21YLHJe?= =?iso-8859-1?Q?basVJcblUjAIOKpXfXE4SS5TQZaNRK7m60OAJrcNqJm/Wdksa3b6SlXtLF?= =?iso-8859-1?Q?3qUKPS5maffPpFTHwIbhgFar+kYIowKSF7aTjuyieO38uT8G15FEOMLTZs?= =?iso-8859-1?Q?twNpJ8DIXLUE7fB9VbRSC9EZFk1vrMKRNY9opwmntNhzsrTobxBE7pP1A1?= =?iso-8859-1?Q?l4igieH8ZvwRQUoVWJZiKwyKB+dL3SNv4QvHv/oFcpwO45AARO/o+gTXLg?= =?iso-8859-1?Q?JYsYYnBts0cY8cKjKIRg/EYPsUQQmCRHn6dhDu70VZTmB/elgKwJ5hG+pb?= =?iso-8859-1?Q?Y5luemjsuYrTgldDpn2sRCTvd8KzcyH+uWgQYM91k6NHKHXPJVrZd47p1n?= =?iso-8859-1?Q?RPOLQDD6Iz+hVbEscT7TxTw7G+nW3ZYruL4rMFmC63JhSlbyR42YH1mOxi?= =?iso-8859-1?Q?LZu/uXNxKKZA1773iDjXToqPw581s7C5SHzFnFf9nOBW0SbXvMSO6Frgry?= =?iso-8859-1?Q?zQJquwFRC/zTuUzQ8i47Dc6IRQPpttDA5KrwY47Nvfw8UejnaSDb6OKVhw?= =?iso-8859-1?Q?oY71Ihb122h2TMBV13K28ka7ydOoBzAH8s+PUAyl7EPhMajAkmf0I6or1g?= =?iso-8859-1?Q?0v9HuiumHUAnQvXfiJtDAlz1mBgBY+5G0CdbBokvsacPuUcd/WuFysQgLB?= =?iso-8859-1?Q?wlyovGklxdgEPk8SAxekTQVo9JD3kUZ0XkmdJr5U+wm/FTNe9ABPJ+QCkD?= =?iso-8859-1?Q?775jLod+XhV0r5CLjg3bTzxMSYXl7aFM/p8WHXgqrCFHKTbXuZjVp0wpAw?= =?iso-8859-1?Q?kd5BBTYPv2SF3WZfcHBUTmvbnia4W5dDx4OlVX9n0jd/BUFytRwc2d31Tn?= =?iso-8859-1?Q?hwo3RNBS40OhtOqQ=3D?= x-microsoft-antispam-prvs: x-forefront-prvs: 0962D394D2 x-forefront-antispam-report: SFV:NSPM; SFS:(10019020)(6029001)(396003)(346002)(376002)(366004)(136003)(39860400002)(13464003)(6602003)(189003)(199004)(33656002)(86362001)(25786009)(9686003)(236005)(7696005)(53936002)(229853002)(8936002)(102836004)(26005)(93886005)(14454004)(71190400001)(790700001)(71200400001)(966005)(2906002)(6116002)(97736004)(6246003)(110136005)(76176011)(316002)(99286004)(68736007)(3846002)(5660300002)(478600001)(66066001)(606006)(446003)(105586002)(106356001)(2501003)(11346002)(486006)(476003)(54896002)(6306002)(6506007)(8676002)(55016002)(53546011)(14444005)(256004)(74316002)(7736002)(81156014)(186003)(6436002)(81166006); DIR:OUT; SFP:1102; SCL:1; SRVR:CS1PR8401MB0600; H:CS1PR8401MB1189.NAMPRD84.PROD.OUTLOOK.COM; FPR:; SPF:None; LANG:en; PTR:InfoNoRecords; MX:1; A:1; x-ms-exchange-senderadcheck: 1 x-microsoft-antispam-message-info: ktk8+TmQZfZpuGOv8glCqKo2clWTSCeOMhPFC2icQge8ItmMRcAaa/cGK7Adj43lO02mshOPX85Nye05BWT+kKDKe9Urx6Ka4zHMkMWkoMQO4S1rOSmrhjDrv8F4rvaqrrCxeqF8HX98IK9AIKAnTpq2c9T2poypSfsJ+u6fJ6ycBSgnpno/NX9Q4pzzKrhXM0O6GNiojn8YXrKKNCI9lzlHP3CrgdXZNjAjxG9E/zpQB9lLXXJzMdZavaRQuVfm49hsCiPxQ8dnNIdG+aubPKvtMCHMhDFYkbCa8OVVad5QCyF0OJSUvggnAaPynYTcKoFZyAwB26diw7o5Ha1ZP5tYeDwchmQZQagzdA9j1RoAdXt/WhdcYc6g/sY3BUj0vXsUk9AUcrxNv/7ur2+4SwPQQHwGh/mkV/OMwJBWvpg= MIME-Version: 1.0 X-OriginatorOrg: hp.com X-MS-Exchange-CrossTenant-Network-Message-Id: 8293ab8f-3ed0-4e27-9455-08d69db6d441 X-MS-Exchange-CrossTenant-originalarrivaltime: 28 Feb 2019 19:56:28.9489 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: ca7981a2-785a-463d-b82a-3db87dfc3ce6 X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-Transport-CrossTenantHeadersStamped: CS1PR8401MB0600 X-MC-Unique: TMaEay_KP9uVpZNvTimW_g-1 X-Mimecast-Spam-Score: 0 X-Content-Filtered-By: Mailman/MimeDel 2.1.29 Subject: Re: [PATCH] MdeModulePkg/SdMmcPciHcDxe: Fix DMA on SDHC v3 64-bit systems X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 28 Feb 2019 19:56:36 -0000 Content-Language: en-US Content-Type: text/plain; charset=WINDOWS-1252 Content-Transfer-Encoding: quoted-printable Ashish, =D8 With my change, if any of the controller did not support 64b DMA in V3= as well as V4 capability, we are not enabling it in PCI layer. The logic is: if (Private->Capability[Slot].SysBus64V3 =3D=3D 0 && Private->Capability[Slot].SysBus64V4 =3D=3D 0) { Support64BitDma =3D FALSE; } which means that for a SDHC v3 controller you have SysBus64V3=3D1 and SysBu= s64V4=3D0 the FALSE assignment is never done - this is not correct. Perhap= s you intended an OR instead of an AND? Either way changing this to an || = or using my patch is the same logical result because a V3 controller will u= se 32-bit DMA and V4 controller will use 64-bit DMA (a V4 controller should= have the V3 bit set). I really saw no reason to be checking the V3 bit si= nce the driver was unprepared to do V3 64-bit DMA operations anyways. Eugene From: Ashish Singhal Sent: Thursday, February 28, 2019 12:15 PM To: Cohen, Eugene ; Wu, Hao A ; edk2-dev= el@lists.01.org Subject: RE: [PATCH] MdeModulePkg/SdMmcPciHcDxe: Fix DMA on SDHC v3 64-bit = systems Hello Eugene, My patch enabled support for SDHC 4.0 and above in general including suppor= t for 64b ADMA descriptor. The check for V3 capability for 64b DMA was alre= ady there and similar check was implemented for V4 capability for 64b DMA. = Earlier, if any of the V3 controller did not support 64b DMA, we were not e= nabling it in PCI layer. With my change, if any of the controller did not s= upport 64b DMA in V3 as well as V4 capability, we are not enabling it in PC= I layer. This check in my opinion is better because we only disable 64b DMA PCI supp= ort when both V3 and V4 have it disabled. Thanks Ashish -----Original Message----- From: Cohen, Eugene > Sent: Thursday, February 28, 2019 4:24 AM To: Wu, Hao A >; edk2-devel@l= ists.01.org Cc: Ashish Singhal = > Subject: RE: [PATCH] MdeModulePkg/SdMmcPciHcDxe: Fix DMA on SDHC v3 64-bit = systems Hao, > I remember the commit b5547b9ce97e80c3127682a2a5d4b9bd14af353e from > Ashish only handles the controllers with version greater or equal to 4.00= . Right - that commit added support for SDHC 4.0 and above. The original driv= er supported SDHC 3.0 albeit only with SDMA and 32-bit ADMA support. With that commit two descriptor types are supported the 32-bit ADMA descrip= tor (SD_MMC_HC_ADMA_32_DESC_LINE which is 64-bits in size) and the V4 64-bi= t ADMA descriptor (SD_MMC_HC_ADMA_64_DESC_LINE which is 128-bits in size). However the commit mistakenly added a check for the V3 capability for 64-bi= t DMA and used it to set the PCI DUAL_ADDRESS_CYCLE attributre which then d= oes not the 32-bit compatible bounce buffer mechanism. Later, when we attem= pt an ADMA data transfer we hit an ASSERT because the PCI DMA subsystem is = not using bounce buffers to provide 32-bit DMA compatible memory. So the pa= tch I submitted simply removes the unnecessary check of the V3 64-bit DMA c= apability check so the PCI DUAL_ADDRESS_CYCLE attribute is not set allowing= 32-bit DMA to succeed on these platforms. > And the ADMA2 (96-bit Descriptor) mode for V3 controllers is selected > by setting the 'DMA Select' filed in the Host Control 1 Register to > 11b. But the currently behavior of the driver is setting the field to > 10b, which I think will not switch to the ADMA2 (96-bit Descriptor) mode = for V3. Correct, right now for a V3 controller only 32-bit DMA is supported. An enh= ancement for V3 64-bit ADMA would improve performance on controllers that s= upport that mode by eliminating the bounce buffer and associated memory cop= ies. I think we should file a BZ for SD HCI V3 64-bit ADMA support - if you= agree I would be happy to do that. I should point out that we have done extensive testing of this change on ou= r host controller. Thanks, Eugene --- From: Wu, Hao A > Sent: Wednesday, February 27, 2019 8:25 PM To: Cohen, Eugene >; edk2-devel@lists.0= 1.org Cc: Ashish Singhal = > Subject: RE: [PATCH] MdeModulePkg/SdMmcPciHcDxe: Fix DMA on SDHC v3 64-bit = systems Loop Ashish in. Some comments below. > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of > Cohen, Eugene > Sent: Wednesday, February 27, 2019 6:59 PM > To: mailto:edk2-devel@lists.01.org; Wu, Hao A > Subject: [edk2] [PATCH] MdeModulePkg/SdMmcPciHcDxe: Fix DMA on SDHC > v3 64-bit systems > > The SdMmcPciHcDriverBindingStart function was checking two different > capability bits in determining whether 64-bit DMA modes were > supported, one mode is defined in the SDHC version > 3 specification (using 96-bit descriptors) and another is defined in > the SDHC version 4 specification (using 128-bit descriptors). Since > the currently implementation of 64-bit > ADMA2 only supports the SDHC version 4 implementation it is incorrect > to check the V3 64-bit capability bit since this will activate V4 > ADMA2 on V3 controllers. I remember the commit b5547b9ce97e80c3127682a2a5d4b9bd14af353e from Ashish = only handles the controllers with version greater or equal to 4.00. And the ADMA2 (96-bit Descriptor) mode for V3 controllers is selected by se= tting the 'DMA Select' filed in the Host Control 1 Register to 11b. But the= currently behavior of the driver is setting the field to 10b, which I thin= k will not switch to the ADMA2 (96-bit Descriptor) mode for V3. Maybe there is something I miss here. Could you help to provide some more d= etail on the issue you met? Thanks. Best Regards, Hao Wu > > Cc: Hao Wu > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Eugene Cohen > --- > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c > b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c > index b474f8d..5bc91c5 100644 > --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c > +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c > @@ -666,8 +666,7 @@ SdMmcPciHcDriverBindingStart ( // If any of the > slots does not support 64b system bus // do not enable 64b DMA in the > PCI layer. > // > - if (Private->Capability[Slot].SysBus64V3 =3D=3D 0 && > - Private->Capability[Slot].SysBus64V4 =3D=3D 0) { > + if (Private->Capability[Slot].SysBus64V4 =3D=3D 0) { > Support64BitDma =3D FALSE; > } > > -- > 2.7.4 > _______________________________________________ > edk2-devel mailing list > mailto:edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel ---------------------------------------------------------------------------= -------- This email message is for the sole use of the intended recipient(s) and may= contain confidential information. Any unauthorized review, use, disclosure or distr= ibution is prohibited. If you are not the intended recipient, please contact the se= nder by reply email and destroy all copies of the original message. ---------------------------------------------------------------------------= --------