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 not support it; > 2. If the driver does not implement the 64-bit DMA mode that the device does > support. Thus, for the current implementation of the SdMmcPciHcDxe driver (including the V4 ADMA descriptor support from Ashish): * The driver should set the DUAL_ADDRESS_CYCLE attribute only when 'SysBus64V4' bit set, because of the statement 2 above. And for the previous implementation (before the change from Ashish): * The driver should not set the DUAL_ADDRESS_CYCLE attribute at all, since the implementation was written to support only the 32b ADMA descriptor. If this is true, I am fine with your proposed fix. Eugene, Could you help to state the reason for the fix a bit more clear in the commit log? Also, I have filed a Bugzilla tracker for this one: https://bugzilla.tianocore.org/show_bug.cgi?id=1583 Could you help to add this information into the commit log as well? Thanks. Best Regards, Hao Wu > -----Original Message----- > From: Ashish Singhal [mailto:ashishsin...@nvidia.com] > Sent: Friday, March 01, 2019 11:25 PM > To: Ard Biesheuvel; Cohen, Eugene > Cc: Wu, Hao A; edk2-devel@lists.01.org; Kim, Sangwoo (김상우 SW1Lab.) > Subject: RE: [PATCH] MdeModulePkg/SdMmcPciHcDxe: Fix DMA on SDHC v3 > 64-bit systems > > Acked-by: Ashish Singhal <ashishsin...@nvidia.com> > > -----Original Message----- > From: Ard Biesheuvel <ard.biesheu...@linaro.org> > Sent: Friday, March 1, 2019 4:39 AM > To: Cohen, Eugene <eug...@hp.com> > Cc: Ashish Singhal <ashishsin...@nvidia.com>; Wu, Hao A > <hao.a...@intel.com>; edk2-devel@lists.01.org; Kim, Sangwoo (김상우 > SW1Lab.) <sangwoo....@hp.com> > Subject: Re: [PATCH] MdeModulePkg/SdMmcPciHcDxe: Fix DMA on SDHC v3 > 64-bit systems > > On Fri, 1 Mar 2019 at 11:54, Cohen, Eugene <eug...@hp.com> 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 32-bit addressable DRAM to begin with. > > > > Interesting - I did not realize that there were designs that were crazy > enough to have no addressable DRAM below 4G. > > > > You must be new here :-) > > But seriously, it does make sense for an implementation to, say, put all > peripherals, PCIe resource windows etc in the bottom half and all DRAM in > the top half of a 40-bit address space, which is how the AMD Seattle SoC > ended with its system memory at address 0x80_0000_0000. > Note that on this platform, we can still use 32-bit DMA if we want to with the > help of the SMMUs, but we haven't wired those up in UEFI (and the generic > host bridge driver did not have the IOMMU hooks at the > time) > > > > The obvious conclusion is that the driver should not set the > > > EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE attribute if the device > does > > > not support it, or, which seems to be our case, if the driver does > > > not implement the 64-bit DMA mode that the driver does support. > > > However, since there are platforms for which bounce buffering is not > > > an option (since there is no 32-bit addressable memory to bounce > > > to), this is not just a performance optimization, and so it would be > > > useful to fix the code so it can drive all 64-bit DMA capable hardware. > > > > Okay, that's a great reason - let's get V3 64b ADMA2 in! > > > > Any objection to committing the original patch in the short term? > > > > not at all > > Acked-by: Ard Biesheuvel <ard.biesheu...@linaro.org> > > ----------------------------------------------------------------------------------- > This email message is for the sole use of the intended recipient(s) and may > contain > confidential information. Any unauthorized review, use, disclosure or > distribution > is prohibited. If you are not the intended recipient, please contact the > sender by > reply email and destroy all copies of the original message. > ----------------------------------------------------------------------------------- _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
