From: "Wu, Hao A" <hao.a.wu@intel.com>
To: "Cohen, Eugene" <eugene@hp.com>,
Ashish Singhal <ashishsingha@nvidia.com>,
Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: "edk2-devel@lists.01.org" <edk2-devel@lists.01.org>,
"Kim, Sangwoo (??? SW1Lab.)" <sangwoo.kim@hp.com>
Subject: Re: [PATCH] MdeModulePkg/SdMmcPciHcDxe: Fix DMA on SDHC v3 64-bit systems
Date: Mon, 4 Mar 2019 02:39:11 +0000 [thread overview]
Message-ID: <B80AF82E9BFB8E4FBD8C89DA810C6A093C8A8E0C@SHSMSX104.ccr.corp.intel.com> (raw)
In-Reply-To: <DM6PR12MB3324C4AD552D361C80FBFEFEBA760@DM6PR12MB3324.namprd12.prod.outlook.com>
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:ashishsingha@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 <ashishsingha@nvidia.com>
>
> -----Original Message-----
> From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Sent: Friday, March 1, 2019 4:39 AM
> To: Cohen, Eugene <eugene@hp.com>
> Cc: Ashish Singhal <ashishsingha@nvidia.com>; Wu, Hao A
> <hao.a.wu@intel.com>; edk2-devel@lists.01.org; Kim, Sangwoo (김상우
> SW1Lab.) <sangwoo.kim@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 <eugene@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.biesheuvel@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.
> -----------------------------------------------------------------------------------
next prev parent reply other threads:[~2019-03-04 2:39 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-02-27 10:58 [PATCH] MdeModulePkg/SdMmcPciHcDxe: Fix DMA on SDHC v3 64-bit systems Cohen, Eugene
2019-02-28 3:24 ` Wu, Hao A
2019-02-28 11:23 ` Cohen, Eugene
2019-02-28 19:15 ` Ashish Singhal
2019-02-28 19:56 ` Cohen, Eugene
2019-02-28 21:27 ` Ashish Singhal
2019-02-28 21:58 ` Cohen, Eugene
2019-02-28 22:20 ` Ashish Singhal
2019-02-28 22:40 ` Cohen, Eugene
2019-02-28 23:58 ` Ashish Singhal
2019-03-01 0:10 ` Cohen, Eugene
2019-03-01 0:19 ` Ashish Singhal
2019-03-01 10:32 ` Ard Biesheuvel
2019-03-01 10:34 ` Ard Biesheuvel
2019-03-01 10:54 ` Cohen, Eugene
2019-03-01 11:38 ` Ard Biesheuvel
2019-03-01 12:31 ` Ashish Singhal
2019-03-01 18:31 ` Ashish Singhal
2019-03-01 15:25 ` Ashish Singhal
2019-03-04 2:39 ` Wu, Hao A [this message]
2019-03-04 4:00 ` Ashish Singhal
2019-03-04 4:26 ` Wu, Hao A
2019-03-05 11:58 ` Cohen, Eugene
2019-03-01 11:02 ` Cohen, Eugene
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=B80AF82E9BFB8E4FBD8C89DA810C6A093C8A8E0C@SHSMSX104.ccr.corp.intel.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox