Re: [edk2] [PATCH] MdeModulePkg/SdMmcPciHcDxe: Fix DMA on SDHC v3 64-bit systems

Ashish, Thanks - I haven't forgotten. We are still doing tests with the 32-bit ADMA driver and running into issues on our newer platform - once we have those resolved we will test the patch. (We are seeing a strange issue where Read Multiple Block works but Write Multiple Block does not.)

Re: [edk2] [PATCH] MdeModulePkg/SdMmcPciHcDxe: Fix DMA on SDHC v3 64-bit systems

> -Original Message- > From: Ashish Singhal [mailto:ashishsin...@nvidia.com] > Sent: Monday, March 04, 2019 12:00 PM > To: Wu, Hao A; Cohen, Eugene; Ard Biesheuvel > Cc: edk2-devel@lists.01.org; Kim, Sangwoo (??? SW1Lab.) > Subject: RE: [PATCH] MdeModulePkg/SdMmcPciHcDxe: Fix DMA on SDHC

Re: [edk2] [PATCH] MdeModulePkg/SdMmcPciHcDxe: Fix DMA on SDHC v3 64-bit systems

Hi Hao, I agree that there has been a bug all along which got exposed just now. We should submit the patch as proposed by Eugene. Also, I have submitted the patch for enabling 64b DMA for V3. Please take that into consideration once the freeze is over so that we can fix the issue in real

Re: [edk2] [PATCH] MdeModulePkg/SdMmcPciHcDxe: Fix DMA on SDHC v3 64-bit systems

Hi Eugene, Ashish and Ard Sorry for the delayed response, I was out of office in the previous several days. According to the discussion, my understanding is that (quote the comments from Ard): > Driver should not set the EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE attribute > 1. If the device does

Re: [edk2] [PATCH] MdeModulePkg/SdMmcPciHcDxe: Fix DMA on SDHC v3 64-bit systems

Eugene, I have submitted a patch for supporting 64b DMA on V3 controllers. Can you please validate it at your end as well? Thanks Ashish From: Ashish Singhal Sent: Friday, March 1, 2019 5:31 AM To: Ard Biesheuvel ; Cohen, Eugene Cc: Wu, Hao A ; edk2-devel@lists.01.org; Kim, Sangwoo (김상우

Re: [edk2] [PATCH] MdeModulePkg/SdMmcPciHcDxe: Fix DMA on SDHC v3 64-bit systems

Acked-by: Ashish Singhal -Original Message- From: Ard Biesheuvel Sent: Friday, March 1, 2019 4:39 AM To: Cohen, Eugene Cc: Ashish Singhal ; Wu, Hao A ; edk2-devel@lists.01.org; Kim, Sangwoo (김상우 SW1Lab.) Subject: Re: [PATCH] MdeModulePkg/SdMmcPciHcDxe: Fix DMA on SDHC v3 64-bit

Re: [edk2] [PATCH] MdeModulePkg/SdMmcPciHcDxe: Fix DMA on SDHC v3 64-bit systems

I've already started refactoring the driver to support V3 64b ADMA2. Meanwhile, I'm OK with the proposed patch. Thanks Ashish Get Outlook for iOS From: Ard Biesheuvel Sent: Friday, March 1, 2019 4:40 AM To: Cohen, Eugene Cc: Ashish

Re: [edk2] [PATCH] MdeModulePkg/SdMmcPciHcDxe: Fix DMA on SDHC v3 64-bit systems

On Fri, 1 Mar 2019 at 11:54, Cohen, Eugene wrote: > > Ard, > > > So before these changes, we were in the exact same situation, but since PC > > platforms never enable DMA above 4 GB in the first place, nobody ever > > noticed until we started running this code on arm64 platforms that have no > >

Re: [edk2] [PATCH] MdeModulePkg/SdMmcPciHcDxe: Fix DMA on SDHC v3 64-bit systems

Ashish, Yes this issue existed before V4 support came in. The previous code would test the (V3) SysBus64 bit: https://github.com/tianocore/edk2/blob/7f3b0bad4bbb3cb24014d2e6216615896ea09dbf/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c#L652 And then return an error building the ADMA

Re: [edk2] [PATCH] MdeModulePkg/SdMmcPciHcDxe: Fix DMA on SDHC v3 64-bit systems

Ard, > So before these changes, we were in the exact same situation, but since PC > platforms never enable DMA above 4 GB in the first place, nobody ever > noticed until we started running this code on arm64 platforms that have no > 32-bit addressable DRAM to begin with. Interesting - I did not

Re: [edk2] [PATCH] MdeModulePkg/SdMmcPciHcDxe: Fix DMA on SDHC v3 64-bit systems

On Fri, 1 Mar 2019 at 11:32, Ard Biesheuvel wrote: > > On Fri, 1 Mar 2019 at 01:19, Ashish Singhal wrote: > > > > Eugene, > > > > Small question. Did the issue appear after the V4 patch went in? Looking at > > the code before that patch, we were enabling 64b dma in pci based on > > capability

Re: [edk2] [PATCH] MdeModulePkg/SdMmcPciHcDxe: Fix DMA on SDHC v3 64-bit systems

On Fri, 1 Mar 2019 at 01:19, Ashish Singhal wrote: > > Eugene, > > Small question. Did the issue appear after the V4 patch went in? Looking at > the code before that patch, we were enabling 64b dma in pci based on > capability register already despite of driver supporting only 32b dma. > I

Re: [edk2] [PATCH] MdeModulePkg/SdMmcPciHcDxe: Fix DMA on SDHC v3 64-bit systems

Eugene, Small question. Did the issue appear after the V4 patch went in? Looking at the code before that patch, we were enabling 64b dma in pci based on capability register already despite of driver supporting only 32b dma. Thanks Ashish Get Outlook for iOS

Re: [edk2] [PATCH] MdeModulePkg/SdMmcPciHcDxe: Fix DMA on SDHC v3 64-bit systems

Ashish, Agreed - #2 would be better in the long run since this will have better performance by eliminating the bounce buffering. My original intent in submitting the patch was to fix the logic the current implementation with minimal impact. Thanks, Eugene From: Ashish Singhal Sent:

Re: [edk2] [PATCH] MdeModulePkg/SdMmcPciHcDxe: Fix DMA on SDHC v3 64-bit systems

Eugene, Thanks for pointing that out. This is a use case we are not covering as of now. I see two options to this: 1. Do not enable 64b DMA support in PCI based on V3 as driver does not support V3 64b ADMA. This is a quick fix. 2. Enable V3 64b ADMA support to add the missing feature.

Re: [edk2] [PATCH] MdeModulePkg/SdMmcPciHcDxe: Fix DMA on SDHC v3 64-bit systems

Ashish, I think that code will still fail for our use case. We are version 3 with 64-bit support so Private->Capability[Slot].SysBus64V3 == 0 will evaluate to FALSE. Since we are V3 Private->ControllerVersion[Slot] >= SD_MMC_HC_CTRL_VER_400 will also evaluate to FALSE. Therefore

Re: [edk2] [PATCH] MdeModulePkg/SdMmcPciHcDxe: Fix DMA on SDHC v3 64-bit systems

Eugene, Thanks for the explanation. The problem is valid and is more clear to me now. How about we do this: Instead of: if (Private->Capability[Slot].SysBus64V3 == 0 && Private->Capability[Slot].SysBus64V4 == 0) { Support64BitDma = FALSE; } What do you think about:

Re: [edk2] [PATCH] MdeModulePkg/SdMmcPciHcDxe: Fix DMA on SDHC v3 64-bit systems

Ashish, Ø Right now, we disable 64b DMA Support in PCI if the controller cannot support 64b DMA in V3 as well as V4. If either of these support 64b DMA, we do not disable it. In the code, we set Support64BitDma to TRUE by default and change it to FALSE only if any of the controller does not

Re: [edk2] [PATCH] MdeModulePkg/SdMmcPciHcDxe: Fix DMA on SDHC v3 64-bit systems

Eugene, We do not have support for V4 64b DMA right now but it can be added later if needed. I am trying to understand the reason behind changing the check from AND to OR. Right now, we disable 64b DMA Support in PCI if the controller cannot support 64b DMA in V3 as well as V4. If either of

Re: [edk2] [PATCH] MdeModulePkg/SdMmcPciHcDxe: Fix DMA on SDHC v3 64-bit systems

Ashish, Ø 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 == 0 && Private->Capability[Slot].SysBus64V4 == 0) { Support64BitDma =

Re: [edk2] [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 support for 64b ADMA descriptor. The check for V3 capability for 64b DMA was already there and similar check was implemented for V4 capability for 64b DMA. Earlier, if any of the V3 controller did not support

Re: [edk2] [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 driver supported SDHC 3.0 albeit only with SDMA and 32-bit ADMA

Re: [edk2] [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-boun...@lists.01.org] On Behalf Of > Cohen, Eugene > Sent: Wednesday, February 27, 2019 6:59 PM > To: edk2-devel@lists.01.org; Wu, Hao A > Subject: [edk2] [PATCH] MdeModulePkg/SdMmcPciHcDxe: