[edk2] Build SCT with edk2
Hello, It seems building SCT is supported with edk2 UDK2017 and not something recent. When I tried building it with tip, it fails. Is someone already working on making sure SCT can be built with edk2 tip? Thanks Ashish --- 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
Re: [edk2] [PATCH] MdeModulePkg/SdMmcPciHcDxe: Add V3 64b DMA Support
Hi Eugene,
Thanks for confirming. Can you please validate the v2 patch I sent as well for
completeness?
Thanks
Ashish
From: Cohen, Eugene
Sent: Wednesday, March 6, 2019 4:05 PM
To: Wu, Hao A ; Ashish Singhal ;
edk2-devel@lists.01.org
Subject: RE: [PATCH] MdeModulePkg/SdMmcPciHcDxe: Add V3 64b DMA Support
* I verified the patch on SDHC version 3.00 with 64-bit System Address
* Support. Hope more configurations are available for testing on Eugene's
* side.
This patch works for us. Tested that the V3 64-bit DMA works and verified that
addresses above 4GB DMA correctly.
Thanks for putting this together.
Feel free to add my Tested-By.
Eugene
From: Wu, Hao A mailto:hao.a...@intel.com>>
Sent: Tuesday, March 5, 2019 8:01 PM
To: Ashish Singhal mailto:ashishsin...@nvidia.com>>;
edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
Cc: Cohen, Eugene mailto:eug...@hp.com>>
Subject: RE: [PATCH] MdeModulePkg/SdMmcPciHcDxe: Add V3 64b DMA Support
Hi Ashish,
One thing to confirm, for the updated checks within
SdMmcPciHcDriverBindingStart():
> if ((Private->ControllerVersion[Slot] == SD_MMC_HC_CTRL_VER_300 &&
> Private->Capability[Slot].SysBus64V3 == 0) ||
> (Private->ControllerVersion[Slot] == SD_MMC_HC_CTRL_VER_400 &&
> Private->Capability[Slot].SysBus64V3 == 0) ||
> (Private->ControllerVersion[Slot] >= SD_MMC_HC_CTRL_VER_410 &&
> Private->Capability[Slot].SysBus64V4 == 0)) {
> Support64BitDma = FALSE;
> }
When the SDHC with version greater than 4.10, the check is only performed
against the 'SysBus64V4' bit. My understanding of the purpose is that:
1. For SDHC with version 4.00, the support of V3 mode and V4 mode of
64-bit System Address are reflect by bit 'SysBus64V3'. Thus, I can infer
that the possible support case is both or neither.
2. The spec states that SDHC with version greater than 4.10 divides the V3
mode and V4 mode support into 2 bits (SysBus64V3, SysBus64V4) so that the
V3 mode support can be optional.
So based on 1 & 2, we do not even bother to check the 'SysBus64V3' bit
when HC version >= 4.10. Is that right?
I verified the patch on SDHC version 3.00 with 64-bit System Address
Support. Hope more configurations are available for testing on Eugene's
side.
Besides, some minor comments below:
> -Original Message-
> From: Ashish Singhal [mailto:ashishsin...@nvidia.com]
> Sent: Saturday, March 02, 2019 2:30 AM
> To: edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
> Cc: Wu, Hao A; eug...@hp.com<mailto:eug...@hp.com>; Ashish Singhal
> Subject: [PATCH] MdeModulePkg/SdMmcPciHcDxe: Add V3 64b DMA
> Support
Please help to add the below Bugzilla tracker for reference:
https://bugzilla.tianocore.org/show_bug.cgi?id=1583
I have updated the above tracker to match the purpose of the proposed
patch.
>
> Driver was supporting only 32b DMA support for V3 controllers. Add
> support for 64b DMA as well for completeness.
>
> For V4.0 64b support, driver was looking at incorrect capability
> register bit. Fix for that is present as well.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ashish Singhal
> mailto:ashishsin...@nvidia.com>>
> ---
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c | 10 +-
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h | 6 +-
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c | 199
> ++---
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h | 29 ++-
> 4 files changed, 170 insertions(+), 74 deletions(-)
>
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> index b474f8d..9b7b88c 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> @@ -6,7 +6,7 @@
>
> It would expose EFI_SD_MMC_PASS_THRU_PROTOCOL for upper layer use.
>
> - Copyright (c) 2018, NVIDIA CORPORATION. All rights reserved.
> + Copyright (c) 2018-2019, NVIDIA CORPORATION. All rights reserved.
> Copyright (c) 2015 - 2019, Intel Corporation. All rights reserved.
> This program and the accompanying materials
> are licensed and made available under the terms and conditions of the BSD
> License
> @@ -666,8 +666,12 @@ 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 == 0 &&
> - Private->Capability[Slot].SysBus64V4 == 0) {
> + if ((Private->ControllerVersion[Slot] == SD_MMC_HC_CTRL_VER_300 &&
> + Private->Capability[Slot].SysBus64V3 == 0) ||
> + (Private->ControllerVersion[Slot] == SD_MMC_HC_CTRL_VER_400 &&
> + Private->Capability[Slot].SysBus64V3 =
[edk2] [PATCH v2] MdeModulePkg/SdMmcPciHcDxe: Add V3 64b DMA Support
Driver was supporting only 32b DMA support for V3 controllers. Add
support for 64b DMA as well for completeness.
For V4.0 64b support, driver was looking at incorrect capability
register bit. Fix for that is present as well.
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1583
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ashish Singhal
---
MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c | 10 +-
MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h | 6 +-
MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c | 189 ++---
MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h | 29 +++-
4 files changed, 161 insertions(+), 73 deletions(-)
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
index b474f8d..9b7b88c 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
@@ -6,7 +6,7 @@
It would expose EFI_SD_MMC_PASS_THRU_PROTOCOL for upper layer use.
- Copyright (c) 2018, NVIDIA CORPORATION. All rights reserved.
+ Copyright (c) 2018-2019, NVIDIA CORPORATION. All rights reserved.
Copyright (c) 2015 - 2019, Intel Corporation. All rights reserved.
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD
License
@@ -666,8 +666,12 @@ 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 == 0 &&
-Private->Capability[Slot].SysBus64V4 == 0) {
+if ((Private->ControllerVersion[Slot] == SD_MMC_HC_CTRL_VER_300 &&
+ Private->Capability[Slot].SysBus64V3 == 0) ||
+(Private->ControllerVersion[Slot] == SD_MMC_HC_CTRL_VER_400 &&
+ Private->Capability[Slot].SysBus64V3 == 0) ||
+(Private->ControllerVersion[Slot] >= SD_MMC_HC_CTRL_VER_410 &&
+ Private->Capability[Slot].SysBus64V4 == 0)) {
Support64BitDma = FALSE;
}
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
index 1bb701a..8846fde 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
@@ -2,7 +2,7 @@
Provides some data structure definitions used by the SD/MMC host controller
driver.
-Copyright (c) 2018, NVIDIA CORPORATION. All rights reserved.
+Copyright (c) 2018-2019, NVIDIA CORPORATION. All rights reserved.
Copyright (c) 2015, Intel Corporation. All rights reserved.
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD
License
@@ -145,13 +145,15 @@ typedef struct {
EFI_PHYSICAL_ADDRESSDataPhy;
VOID*DataMap;
SD_MMC_HC_TRANSFER_MODE Mode;
+ SD_MMC_HC_ADMA_LENGTH_MODE AdmaLengthMode;
EFI_EVENT Event;
BOOLEAN Started;
UINT64 Timeout;
SD_MMC_HC_ADMA_32_DESC_LINE *Adma32Desc;
- SD_MMC_HC_ADMA_64_DESC_LINE *Adma64Desc;
+ SD_MMC_HC_ADMA_64_V3_DESC_LINE *Adma64V3Desc;
+ SD_MMC_HC_ADMA_64_V4_DESC_LINE *Adma64V4Desc;
EFI_PHYSICAL_ADDRESSAdmaDescPhy;
VOID*AdmaMap;
UINT32 AdmaPages;
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
index d73fa10..6fefed1 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
@@ -6,7 +6,7 @@
It would expose EFI_SD_MMC_PASS_THRU_PROTOCOL for upper layer use.
- Copyright (c) 2018, NVIDIA CORPORATION. All rights reserved.
+ Copyright (c) 2018-2019, NVIDIA CORPORATION. All rights reserved.
Copyright (c) 2015 - 2019, Intel Corporation. All rights reserved.
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD
License
@@ -1010,16 +1010,28 @@ SdMmcHcInitV4Enhancements (
if (ControllerVer >= SD_MMC_HC_CTRL_VER_400) {
HostCtrl2 = SD_MMC_HC_V4_EN;
//
-// Check if V4 64bit support is available
+// Check if controller version V4.0
//
-if (Capability.SysBus64V4 != 0) {
- HostCtrl2 |= SD_MMC_HC_64_ADDR_EN;
- DEBUG ((DEBUG_INFO, "Enabled V4 64 bit system bus support\n"));
+if (ControllerVer == SD_MMC_HC_CTRL_VER_400) {
+ //
+ // Check if 64bit support is available
+ //
+ if (Capability.SysBus64V3 != 0) {
+HostCtrl2 |= SD_MMC_HC_64_ADDR_EN;
+DEBUG ((DEBUG_INFO, "Enabled V4 64 bit system bus support\n"));
+ }
}
//
// Check if controlle
Re: [edk2] [PATCH] MdeModulePkg/SdMmcPciHcDxe: Add V3 64b DMA Support
Hi Hao,
That is right. Also, I have addressed your comments and have submitted v2 patch.
Thanks
Ashish
-Original Message-
From: Wu, Hao A
Sent: Tuesday, March 5, 2019 8:01 PM
To: Ashish Singhal ; edk2-devel@lists.01.org
Cc: eug...@hp.com
Subject: RE: [PATCH] MdeModulePkg/SdMmcPciHcDxe: Add V3 64b DMA Support
Hi Ashish,
One thing to confirm, for the updated checks within
SdMmcPciHcDriverBindingStart():
> if ((Private->ControllerVersion[Slot] == SD_MMC_HC_CTRL_VER_300 &&
> Private->Capability[Slot].SysBus64V3 == 0) ||
> (Private->ControllerVersion[Slot] == SD_MMC_HC_CTRL_VER_400 &&
> Private->Capability[Slot].SysBus64V3 == 0) ||
> (Private->ControllerVersion[Slot] >= SD_MMC_HC_CTRL_VER_410 &&
> Private->Capability[Slot].SysBus64V4 == 0)) {
>Support64BitDma = FALSE;
> }
When the SDHC with version greater than 4.10, the check is only performed
against the 'SysBus64V4' bit. My understanding of the purpose is that:
1. For SDHC with version 4.00, the support of V3 mode and V4 mode of 64-bit
System Address are reflect by bit 'SysBus64V3'. Thus, I can infer that the
possible support case is both or neither.
2. The spec states that SDHC with version greater than 4.10 divides the V3 mode
and V4 mode support into 2 bits (SysBus64V3, SysBus64V4) so that the
V3 mode support can be optional.
So based on 1 & 2, we do not even bother to check the 'SysBus64V3' bit when HC
version >= 4.10. Is that right?
I verified the patch on SDHC version 3.00 with 64-bit System Address Support.
Hope more configurations are available for testing on Eugene's side.
Besides, some minor comments below:
> -Original Message-
> From: Ashish Singhal [mailto:ashishsin...@nvidia.com]
> Sent: Saturday, March 02, 2019 2:30 AM
> To: edk2-devel@lists.01.org
> Cc: Wu, Hao A; eug...@hp.com; Ashish Singhal
> Subject: [PATCH] MdeModulePkg/SdMmcPciHcDxe: Add V3 64b DMA Support
Please help to add the below Bugzilla tracker for reference:
https://bugzilla.tianocore.org/show_bug.cgi?id=1583
I have updated the above tracker to match the purpose of the proposed patch.
>
> Driver was supporting only 32b DMA support for V3 controllers. Add
> support for 64b DMA as well for completeness.
>
> For V4.0 64b support, driver was looking at incorrect capability
> register bit. Fix for that is present as well.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ashish Singhal
> ---
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c | 10 +-
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h | 6 +-
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c | 199
> ++---
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h | 29 ++-
> 4 files changed, 170 insertions(+), 74 deletions(-)
>
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> index b474f8d..9b7b88c 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> @@ -6,7 +6,7 @@
>
>It would expose EFI_SD_MMC_PASS_THRU_PROTOCOL for upper layer use.
>
> - Copyright (c) 2018, NVIDIA CORPORATION. All rights reserved.
> + Copyright (c) 2018-2019, NVIDIA CORPORATION. All rights reserved.
>Copyright (c) 2015 - 2019, Intel Corporation. All rights reserved.
>This program and the accompanying materials
>are licensed and made available under the terms and conditions of
> the BSD License @@ -666,8 +666,12 @@ 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 == 0 &&
> -Private->Capability[Slot].SysBus64V4 == 0) {
> +if ((Private->ControllerVersion[Slot] == SD_MMC_HC_CTRL_VER_300 &&
> + Private->Capability[Slot].SysBus64V3 == 0) ||
> +(Private->ControllerVersion[Slot] == SD_MMC_HC_CTRL_VER_400 &&
> + Private->Capability[Slot].SysBus64V3 == 0) ||
> +(Private->ControllerVersion[Slot] >= SD_MMC_HC_CTRL_VER_410 &&
> + Private->Capability[Slot].SysBus64V4 == 0)) {
>Support64BitDma = FALSE;
> }
>
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> index 1bb701a..68d8a5c 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> @@ -2,7 +2,7 @@
>
>Provides some data structure definitions used by the SD/MMC host
> controller driver.
>
> -Copyright (c) 2018, NVIDIA CORPORATION. Al
Re: [edk2] [PATCH v2 0/1] MdeModulePkg/SdMmcPciHcDxe: Fix DMA on SDHC v3 64-bit systems
Ok with me. Get Outlook for iOS<https://aka.ms/o0ukef> From: Ard Biesheuvel Sent: Tuesday, March 5, 2019 6:39 AM To: Wu, Hao A Cc: edk2-devel@lists.01.org; Eugene Cohen; Ashish Singhal Subject: Re: [PATCH v2 0/1] MdeModulePkg/SdMmcPciHcDxe: Fix DMA on SDHC v3 64-bit systems On Tue, 5 Mar 2019 at 03:18, Wu, Hao A wrote: > > > -Original Message- > > From: Wu, Hao A > > Sent: Tuesday, March 05, 2019 9:14 AM > > To: edk2-devel@lists.01.org > > Cc: Wu, Hao A; Eugene Cohen; Ard Biesheuvel; Ashish Singhal > > Subject: [PATCH v2 0/1] MdeModulePkg/SdMmcPciHcDxe: Fix DMA on SDHC > > v3 64-bit systems > > Since Ashish already posted a patch to add the 64-bit System Address > support for V3 mode SDHC: > https://www.mail-archive.com/edk2-devel@lists.01.org/msg52057.html > > I think this patch can be dropped. > > But since Ashish's patch above is considered as a new feature addition, it > will be pushed (if passes the review process) after the 19`Q1 release tag. > > So Eugene, Ard and Ashish, do you have concern on this? > That works for me. --- 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
Re: [edk2] [PATCH] MdeModulePkg/SdMmcPciHcDxe: Add V3 64b DMA Support
Liming,
I am OK waiting till the freeze is over.
Thanks
Ashish
-Original Message-
From: Gao, Liming
Sent: Saturday, March 2, 2019 9:01 AM
To: Ashish Singhal ; edk2-devel@lists.01.org
Cc: Wu, Hao A
Subject: RE: [edk2] [PATCH] MdeModulePkg/SdMmcPciHcDxe: Add V3 64b DMA Support
Ashish:
This seems a feature. Now, we are in Hard Feature Freeze. So, it will not be
added until edk2-stable201903 tag is created at 2019-03-08.
Thanks
Liming
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Ashish Singhal
> Sent: Saturday, March 2, 2019 2:30 AM
> To: edk2-devel@lists.01.org
> Cc: Wu, Hao A ; Ashish Singhal
>
> Subject: [edk2] [PATCH] MdeModulePkg/SdMmcPciHcDxe: Add V3 64b DMA
> Support
>
> Driver was supporting only 32b DMA support for V3 controllers. Add
> support for 64b DMA as well for completeness.
>
> For V4.0 64b support, driver was looking at incorrect capability
> register bit. Fix for that is present as well.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ashish Singhal
> ---
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c | 10 +-
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h | 6 +-
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c | 199
> ++---
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h | 29 ++-
> 4 files changed, 170 insertions(+), 74 deletions(-)
>
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> index b474f8d..9b7b88c 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> @@ -6,7 +6,7 @@
>
>It would expose EFI_SD_MMC_PASS_THRU_PROTOCOL for upper layer use.
>
> - Copyright (c) 2018, NVIDIA CORPORATION. All rights reserved.
> + Copyright (c) 2018-2019, NVIDIA CORPORATION. All rights reserved.
>Copyright (c) 2015 - 2019, Intel Corporation. All rights reserved.
>This program and the accompanying materials
>are licensed and made available under the terms and conditions of
> the BSD License @@ -666,8 +666,12 @@ 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 == 0 &&
> -Private->Capability[Slot].SysBus64V4 == 0) {
> +if ((Private->ControllerVersion[Slot] == SD_MMC_HC_CTRL_VER_300 &&
> + Private->Capability[Slot].SysBus64V3 == 0) ||
> +(Private->ControllerVersion[Slot] == SD_MMC_HC_CTRL_VER_400 &&
> + Private->Capability[Slot].SysBus64V3 == 0) ||
> +(Private->ControllerVersion[Slot] >= SD_MMC_HC_CTRL_VER_410 &&
> + Private->Capability[Slot].SysBus64V4 == 0)) {
>Support64BitDma = FALSE;
> }
>
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> index 1bb701a..68d8a5c 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> @@ -2,7 +2,7 @@
>
>Provides some data structure definitions used by the SD/MMC host
> controller driver.
>
> -Copyright (c) 2018, NVIDIA CORPORATION. All rights reserved.
> +Copyright (c) 2018-2019, NVIDIA CORPORATION. All rights reserved.
> Copyright (c) 2015, Intel Corporation. All rights reserved. This
> program and the accompanying materials are licensed and made
> available under the terms and conditions of the BSD License @@ -145,13
> +145,15 @@ typedef struct {
>EFI_PHYSICAL_ADDRESSDataPhy;
>VOID*DataMap;
>SD_MMC_HC_TRANSFER_MODE Mode;
> + SD_MMC_HC_ADMA_LEGTHLength;
>
>EFI_EVENT Event;
>BOOLEAN Started;
>UINT64 Timeout;
>
>SD_MMC_HC_ADMA_32_DESC_LINE *Adma32Desc;
> - SD_MMC_HC_ADMA_64_DESC_LINE *Adma64Desc;
> + SD_MMC_HC_ADMA_64_V3_DESC_LINE *Adma64V3Desc;
> + SD_MMC_HC_ADMA_64_V4_DESC_LINE *Adma64V4Desc;
>EFI_PHYSICAL_ADDRESSAdmaDescPhy;
>VOID*AdmaMap;
>UINT32 AdmaPages;
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> index d73fa10..a6d2395 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> @@ -6,7 +6,7
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 sense. Eugene, Please let me know once you have tried my patch on your board. Thanks Ashish -Original Message- From: Wu, Hao A Sent: Sunday, March 3, 2019 7:39 PM To: Cohen, Eugene ; Ashish Singhal ; Ard Biesheuvel Cc: edk2-devel@lists.01.org; Kim, Sangwoo (??? SW1Lab.) Subject: RE: [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 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 > > -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 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 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__. > 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 > > -- > - 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 yo
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 (김상우 SW1Lab.) Subject: Re: [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<https://aka.ms/o0ukef> From: Ard Biesheuvel mailto:ard.biesheu...@linaro.org>> Sent: Friday, March 1, 2019 4:40 AM To: Cohen, Eugene Cc: Ashish Singhal; Wu, Hao A; edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>; Kim, Sangwoo (김상우 SW1Lab.) Subject: Re: [PATCH] MdeModulePkg/SdMmcPciHcDxe: Fix DMA on SDHC v3 64-bit systems On Fri, 1 Mar 2019 at 11:54, Cohen, Eugene mailto: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__. 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 mailto: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
[edk2] [PATCH] MdeModulePkg/SdMmcPciHcDxe: Add V3 64b DMA Support
Driver was supporting only 32b DMA support for V3 controllers. Add
support for 64b DMA as well for completeness.
For V4.0 64b support, driver was looking at incorrect capability
register bit. Fix for that is present as well.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ashish Singhal
---
MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c | 10 +-
MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h | 6 +-
MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c | 199 ++---
MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h | 29 ++-
4 files changed, 170 insertions(+), 74 deletions(-)
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
index b474f8d..9b7b88c 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
@@ -6,7 +6,7 @@
It would expose EFI_SD_MMC_PASS_THRU_PROTOCOL for upper layer use.
- Copyright (c) 2018, NVIDIA CORPORATION. All rights reserved.
+ Copyright (c) 2018-2019, NVIDIA CORPORATION. All rights reserved.
Copyright (c) 2015 - 2019, Intel Corporation. All rights reserved.
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD
License
@@ -666,8 +666,12 @@ 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 == 0 &&
-Private->Capability[Slot].SysBus64V4 == 0) {
+if ((Private->ControllerVersion[Slot] == SD_MMC_HC_CTRL_VER_300 &&
+ Private->Capability[Slot].SysBus64V3 == 0) ||
+(Private->ControllerVersion[Slot] == SD_MMC_HC_CTRL_VER_400 &&
+ Private->Capability[Slot].SysBus64V3 == 0) ||
+(Private->ControllerVersion[Slot] >= SD_MMC_HC_CTRL_VER_410 &&
+ Private->Capability[Slot].SysBus64V4 == 0)) {
Support64BitDma = FALSE;
}
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
index 1bb701a..68d8a5c 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
@@ -2,7 +2,7 @@
Provides some data structure definitions used by the SD/MMC host controller
driver.
-Copyright (c) 2018, NVIDIA CORPORATION. All rights reserved.
+Copyright (c) 2018-2019, NVIDIA CORPORATION. All rights reserved.
Copyright (c) 2015, Intel Corporation. All rights reserved.
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD
License
@@ -145,13 +145,15 @@ typedef struct {
EFI_PHYSICAL_ADDRESSDataPhy;
VOID*DataMap;
SD_MMC_HC_TRANSFER_MODE Mode;
+ SD_MMC_HC_ADMA_LEGTHLength;
EFI_EVENT Event;
BOOLEAN Started;
UINT64 Timeout;
SD_MMC_HC_ADMA_32_DESC_LINE *Adma32Desc;
- SD_MMC_HC_ADMA_64_DESC_LINE *Adma64Desc;
+ SD_MMC_HC_ADMA_64_V3_DESC_LINE *Adma64V3Desc;
+ SD_MMC_HC_ADMA_64_V4_DESC_LINE *Adma64V4Desc;
EFI_PHYSICAL_ADDRESSAdmaDescPhy;
VOID*AdmaMap;
UINT32 AdmaPages;
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
index d73fa10..a6d2395 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
@@ -6,7 +6,7 @@
It would expose EFI_SD_MMC_PASS_THRU_PROTOCOL for upper layer use.
- Copyright (c) 2018, NVIDIA CORPORATION. All rights reserved.
+ Copyright (c) 2018-2019, NVIDIA CORPORATION. All rights reserved.
Copyright (c) 2015 - 2019, Intel Corporation. All rights reserved.
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD
License
@@ -1010,18 +1010,32 @@ SdMmcHcInitV4Enhancements (
if (ControllerVer >= SD_MMC_HC_CTRL_VER_400) {
HostCtrl2 = SD_MMC_HC_V4_EN;
//
-// Check if V4 64bit support is available
+// Check if controller version V4.0
//
-if (Capability.SysBus64V4 != 0) {
- HostCtrl2 |= SD_MMC_HC_64_ADDR_EN;
- DEBUG ((DEBUG_INFO, "Enabled V4 64 bit system bus support\n"));
+if (ControllerVer == SD_MMC_HC_CTRL_VER_400) {
+ //
+ // Check if 64bit support is available
+ //
+ if (Capability.SysBus64V3 != 0) {
+HostCtrl2 |= SD_MMC_HC_64_ADDR_EN;
+DEBUG ((DEBUG_INFO, "Enabled V4 64 bit system bus support\n"));
+ }
}
//
// Check if controller version V4.10 or higher
//
-if (ControllerVer >= SD_M
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 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 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__. 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 --- 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
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<https://aka.ms/o0ukef> From: Ard Biesheuvel Sent: Friday, March 1, 2019 4:40 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 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 > > 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__. 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 --- 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
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<https://aka.ms/o0ukef>
From: Cohen, Eugene
Sent: Thursday, February 28, 2019 5:11 PM
To: Ashish Singhal; Wu, Hao A; edk2-devel@lists.01.org; Ard Biesheuvel
Subject: RE: [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: Thursday, February 28, 2019 4:58 PM
To: Cohen, Eugene ; Wu, Hao A ;
edk2-devel@lists.01.org; Ard Biesheuvel
Subject: RE: [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. This will take
maybe a day or two and can be done.
Thanks
Ashish
From: Cohen, Eugene mailto:eug...@hp.com>>
Sent: Thursday, February 28, 2019 3:40 PM
To: Ashish Singhal mailto:ashishsin...@nvidia.com>>;
Wu, Hao A mailto:hao.a...@intel.com>>;
edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>; Ard Biesheuvel
mailto:ard.biesheu...@linaro.org>>
Subject: RE: [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 Support64BitDma
will still be TRUE resulting in DUAL_ADDRESS_CYCLE being set which disables
bounce buffering.
Since no code is in place to do V3 64b DMA we will still hit the same problem,
specifically namely that buffers that are not DMAable will be allocated and we
will still fail the check
here<https://github.com/tianocore/edk2/blob/ece4c1de3e7b2340d351c2054c79ea689a954ed6/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c#L1426>.
Until such time that V3 64b DMA support is in place I believe only the V4 bit
should be evaluated.
Eugene
From: Ashish Singhal mailto:ashishsin...@nvidia.com>>
Sent: Thursday, February 28, 2019 3:21 PM
To: Cohen, Eugene mailto:eug...@hp.com>>; Wu, Hao A
mailto:hao.a...@intel.com>>;
edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>; Ard Biesheuvel
mailto:ard.biesheu...@linaro.org>>
Subject: RE: [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:
if ((Private->ControllerVersion[Slot] == SD_MMC_HC_CTRL_VER_300 &&
Private->Capability[Slot].SysBus64V3 == 0) ||
(Private->ControllerVersion[Slot] >= SD_MMC_HC_CTRL_VER_400 &&
Private->Capability[Slot].SysBus64V4 == 0)) {
Support64BitDma = FALSE;
}
With this, we would be checking 64b capability based on the version we are
using and not for something we may not be using despite of being advertised in
the controller.
Thanks
Ashish
From: Cohen, Eugene mailto:eug...@hp.com>>
Sent: Thursday, February 28, 2019 2:59 PM
To: Ashish Singhal mailto:ashishsin...@nvidia.com>>;
Wu, Hao A mailto:hao.a...@intel.com>>;
edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>; Ard Biesheuvel
mailto:ard.biesheu...@linaro.org>>
Subject: RE: [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 support it in V3 as
well as V4. If all controllers support it in V3 or V4 we keep 64b DMA support
enabled.
That is precisely the problem. An SDHC v3 controller might support 64b DMA in
V3 but not in V4 mode. The current code will leave 64b DMA support enabled
resulting in the issuing of the PCI DUAL_ADDRESS_CYCLE attribute ( see
https://github.com/tianocore/edk2/blob/ece4c1de3e7b2340d351c2054c79ea689a954ed6/Md
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. This will take
maybe a day or two and can be done.
Thanks
Ashish
From: Cohen, Eugene
Sent: Thursday, February 28, 2019 3:40 PM
To: Ashish Singhal ; Wu, Hao A ;
edk2-devel@lists.01.org; Ard Biesheuvel
Subject: RE: [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 Support64BitDma
will still be TRUE resulting in DUAL_ADDRESS_CYCLE being set which disables
bounce buffering.
Since no code is in place to do V3 64b DMA we will still hit the same problem,
specifically namely that buffers that are not DMAable will be allocated and we
will still fail the check
here<https://github.com/tianocore/edk2/blob/ece4c1de3e7b2340d351c2054c79ea689a954ed6/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c#L1426>.
Until such time that V3 64b DMA support is in place I believe only the V4 bit
should be evaluated.
Eugene
From: Ashish Singhal mailto:ashishsin...@nvidia.com>>
Sent: Thursday, February 28, 2019 3:21 PM
To: Cohen, Eugene mailto:eug...@hp.com>>; Wu, Hao A
mailto:hao.a...@intel.com>>;
edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>; Ard Biesheuvel
mailto:ard.biesheu...@linaro.org>>
Subject: RE: [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:
if ((Private->ControllerVersion[Slot] == SD_MMC_HC_CTRL_VER_300 &&
Private->Capability[Slot].SysBus64V3 == 0) ||
(Private->ControllerVersion[Slot] >= SD_MMC_HC_CTRL_VER_400 &&
Private->Capability[Slot].SysBus64V4 == 0)) {
Support64BitDma = FALSE;
}
With this, we would be checking 64b capability based on the version we are
using and not for something we may not be using despite of being advertised in
the controller.
Thanks
Ashish
From: Cohen, Eugene mailto:eug...@hp.com>>
Sent: Thursday, February 28, 2019 2:59 PM
To: Ashish Singhal mailto:ashishsin...@nvidia.com>>;
Wu, Hao A mailto:hao.a...@intel.com>>;
edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>; Ard Biesheuvel
mailto:ard.biesheu...@linaro.org>>
Subject: RE: [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 support it in V3 as
well as V4. If all controllers support it in V3 or V4 we keep 64b DMA support
enabled.
That is precisely the problem. An SDHC v3 controller might support 64b DMA in
V3 but not in V4 mode. The current code will leave 64b DMA support enabled
resulting in the issuing of the PCI DUAL_ADDRESS_CYCLE attribute ( see
https://github.com/tianocore/edk2/blob/ece4c1de3e7b2340d351c2054c79ea689a954ed6/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c#L738
) which then causes buffers to be allocated that cannot be DMAed.
For reference look at this snippet of the NonDiscoverablePciDeviceIo driver:
https://github.com/tianocore/edk2/blob/ece4c1de3e7b2340d351c2054c79ea689a954ed6/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c#L622
and you can see that bounce buffering will only occur if DUAL_ADDRESS_CYCLE is
clear.
So since we do not have V3 64b DMA (96-bit descriptor) support in place we must
not allow the DUAL_ADDRESS_CYCLE attribute to be set or we will fail with this
check:
https://github.com/tianocore/edk2/blob/ece4c1de3e7b2340d351c2054c79ea689a954ed6/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c#L1426
I've added Ard who updated the driver with DUAL_ADDRESS_CYCLE support.
Eugene
From: Ashish Singhal mailto:ashishsin...@nvidia.com>>
Sent: Thursday, February 28, 2019 2:28 PM
To: Cohen, Eugene mailto:eug...@hp.com>>; Wu, Hao A
mailto:hao.a...@intel.com>>;
edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
Subject: RE: [PATCH] MdeModulePkg/SdMmcPciHcDxe: Fix DMA on SDHC v3 64-bit
systems
Eugene,
We do not
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:
if ((Private->ControllerVersion[Slot] == SD_MMC_HC_CTRL_VER_300 &&
Private->Capability[Slot].SysBus64V3 == 0) ||
(Private->ControllerVersion[Slot] >= SD_MMC_HC_CTRL_VER_400 &&
Private->Capability[Slot].SysBus64V4 == 0)) {
Support64BitDma = FALSE;
}
With this, we would be checking 64b capability based on the version we are
using and not for something we may not be using despite of being advertised in
the controller.
Thanks
Ashish
From: Cohen, Eugene
Sent: Thursday, February 28, 2019 2:59 PM
To: Ashish Singhal ; Wu, Hao A ;
edk2-devel@lists.01.org; Ard Biesheuvel
Subject: RE: [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 support it in V3 as
well as V4. If all controllers support it in V3 or V4 we keep 64b DMA support
enabled.
That is precisely the problem. An SDHC v3 controller might support 64b DMA in
V3 but not in V4 mode. The current code will leave 64b DMA support enabled
resulting in the issuing of the PCI DUAL_ADDRESS_CYCLE attribute ( see
https://github.com/tianocore/edk2/blob/ece4c1de3e7b2340d351c2054c79ea689a954ed6/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c#L738
) which then causes buffers to be allocated that cannot be DMAed.
For reference look at this snippet of the NonDiscoverablePciDeviceIo driver:
https://github.com/tianocore/edk2/blob/ece4c1de3e7b2340d351c2054c79ea689a954ed6/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c#L622
and you can see that bounce buffering will only occur if DUAL_ADDRESS_CYCLE is
clear.
So since we do not have V3 64b DMA (96-bit descriptor) support in place we must
not allow the DUAL_ADDRESS_CYCLE attribute to be set or we will fail with this
check:
https://github.com/tianocore/edk2/blob/ece4c1de3e7b2340d351c2054c79ea689a954ed6/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c#L1426
I've added Ard who updated the driver with DUAL_ADDRESS_CYCLE support.
Eugene
From: Ashish Singhal mailto:ashishsin...@nvidia.com>>
Sent: Thursday, February 28, 2019 2:28 PM
To: Cohen, Eugene mailto:eug...@hp.com>>; Wu, Hao A
mailto:hao.a...@intel.com>>;
edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
Subject: RE: [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 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 support it in V3 as
well as V4. If all controllers support it in V3 or V4 we keep 64b DMA support
enabled.
//
// Enable 64-bit DMA support in the PCI layer if this controller
// supports it.
//
if (Support64BitDma) {
Status = PciIo->Attributes (
PciIo,
EfiPciIoAttributeOperationEnable,
EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE,
NULL
);
if (EFI_ERROR (Status)) {
DEBUG ((DEBUG_WARN, "SdMmcPciHcDriverBindingStart: failed to enable
64-bit DMA (%r)\n", Status));
}
}
Thanks
Ashish
From: Cohen, Eugene mailto:eug...@hp.com>>
Sent: Thursday, February 28, 2019 12:56 PM
To: Ashish Singhal mailto:ashishsin...@nvidia.com>>;
Wu, Hao A mailto:hao.a...@intel.com>>;
edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
Subject: RE: [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 = FALSE;
}
which means that for a SDHC v3 controller you have SysBus64V3=1 and
SysBus64V4=0 the FALSE assignment is never done - this is not correct. Perhaps
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
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 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 support it in V3 as
well as V4. If all controllers support it in V3 or V4 we keep 64b DMA support
enabled.
//
// Enable 64-bit DMA support in the PCI layer if this controller
// supports it.
//
if (Support64BitDma) {
Status = PciIo->Attributes (
PciIo,
EfiPciIoAttributeOperationEnable,
EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE,
NULL
);
if (EFI_ERROR (Status)) {
DEBUG ((DEBUG_WARN, "SdMmcPciHcDriverBindingStart: failed to enable
64-bit DMA (%r)\n", Status));
}
}
Thanks
Ashish
From: Cohen, Eugene
Sent: Thursday, February 28, 2019 12:56 PM
To: Ashish Singhal ; Wu, Hao A ;
edk2-devel@lists.01.org
Subject: RE: [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 = FALSE;
}
which means that for a SDHC v3 controller you have SysBus64V3=1 and
SysBus64V4=0 the FALSE assignment is never done - this is not correct. Perhaps
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 use
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 since the
driver was unprepared to do V3 64-bit DMA operations anyways.
Eugene
From: Ashish Singhal mailto:ashishsin...@nvidia.com>>
Sent: Thursday, February 28, 2019 12:15 PM
To: Cohen, Eugene mailto:eug...@hp.com>>; Wu, Hao A
mailto:hao.a...@intel.com>>;
edk2-devel@lists.01.org<mailto:edk2-devel@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 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 64b DMA, we were not enabling it in
PCI layer. 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.
This check in my opinion is better because we only disable 64b DMA PCI support
when both V3 and V4 have it disabled.
Thanks
Ashish
-Original Message-
From: Cohen, Eugene mailto:eug...@hp.com>>
Sent: Thursday, February 28, 2019 4:24 AM
To: Wu, Hao A mailto:hao.a...@intel.com>>;
edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
Cc: Ashish Singhal mailto:ashishsin...@nvidia.com>>
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 driver
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 descriptor
(SD_MMC_HC_ADMA_32_DESC_LINE which is 64-bits in size) and the V4 64-bit 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-bit
DMA and used it to set the PCI DUAL_ADDRESS_CYCLE attributre which then does
not the 32-bit compatible bounce buffer mechanism. Later, when we attempt 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 patch I
submitted simply removes the unnecessary check of the V3 64-bit DMA capability
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
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 64b DMA, we were not enabling it in PCI layer. 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. This check in my opinion is better because we only disable 64b DMA PCI support 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@lists.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 driver 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 descriptor (SD_MMC_HC_ADMA_32_DESC_LINE which is 64-bits in size) and the V4 64-bit 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-bit DMA and used it to set the PCI DUAL_ADDRESS_CYCLE attributre which then does not the 32-bit compatible bounce buffer mechanism. Later, when we attempt 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 patch I submitted simply removes the unnecessary check of the V3 64-bit DMA capability 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 enhancement for V3 64-bit ADMA would improve performance on controllers that support that mode by eliminating the bounce buffer and associated memory copies. 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 our host controller. Thanks, Eugene --- From: Wu, Hao A Sent: Wednesday, February 27, 2019 8:25 PM To: Cohen, Eugene ; edk2-devel@lists.01.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-boun...@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 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. Maybe there is something I miss here. Could you help to provide some more detail on the issue you met? Thanks. Best Regards, Hao Wu > > Cc: Hao Wu <mailto:hao.a...@intel.com> > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Eugene Cohen <mailto:eug...@hp.com> > --- > 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 &
Re: [edk2] [PATCH] MdePkg/UefiLib: Simplify protocol un/installation abstraction
Mike,
Do you have any update on this change yet?
Thanks
Ashish
-Original Message-
From: Kinney, Michael D
Sent: Tuesday, February 19, 2019 12:56 PM
To: Ashish Singhal ; edk2-devel@lists.01.org; Kinney,
Michael D
Cc: Gao, Liming
Subject: RE: [PATCH] MdePkg/UefiLib: Simplify protocol un/installation
abstraction
Ashish,
Thanks for looking at simplifying this logic again.
I have not had a chance to run the size analysis yet.
I will get back to you in a couple of days.
Thanks,
Mike
> -Original Message-
> From: Ashish Singhal [mailto:ashishsin...@nvidia.com]
> Sent: Tuesday, February 19, 2019 8:19 AM
> To: edk2-devel@lists.01.org
> Cc: Kinney, Michael D ;
> Gao, Liming
> Subject: RE: [PATCH] MdePkg/UefiLib: Simplify protocol
> un/installation abstraction
>
> Hello Mike/Lao,
>
> Were you able to have a look at this?
>
> Thanks
> Ashish
>
> -Original Message-
> From: Ashish Singhal
> Sent: Monday, February 4, 2019 1:16 PM
> To: edk2-devel@lists.01.org
> Cc: michael.d.kin...@intel.com; liming@intel.com;
> Ashish Singhal
> Subject: [PATCH] MdePkg/UefiLib: Simplify protocol
> un/installation abstraction
>
> Add helper functions to operate upon protocol
> installation and
> uninstallation instead of every function doing it by
> itself.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ashish Singhal
> ---
> MdePkg/Library/UefiLib/UefiDriverModel.c | 2040 +-
>
> 1 file changed, 342 insertions(+), 1698 deletions(-)
>
> diff --git a/MdePkg/Library/UefiLib/UefiDriverModel.c
> b/MdePkg/Library/UefiLib/UefiDriverModel.c
> index 262d8bc..268edf7 100644
> --- a/MdePkg/Library/UefiLib/UefiDriverModel.c
> +++ b/MdePkg/Library/UefiLib/UefiDriverModel.c
> @@ -17,6 +17,290 @@
>
> #include "UefiLibInternal.h"
>
> +
> +#define MAX_SUPPORTED_PROTOCOLS 7
> +typedef struct {
> + EFI_GUID *Guid;
> + VOID *Interface;
> +} EFI_PROCESS_PROTOCOL;
> +
> +
> +static
> +EFI_STATUS
> +EFIAPI
> +EfiLibProcessProtocols (
> + IN CONST EFI_HANDLE
> ImageHandle,
> + IN EFI_DRIVER_BINDING_PROTOCOL
> *DriverBinding,
> + IN EFI_HANDLE
> DriverBindingHandle,
> + IN BOOLEAN Install,
> + IN CONST EFI_PROCESS_PROTOCOL
> *ProtocolArray
> + )
> +{
> + ASSERT (DriverBinding != NULL);
> + ASSERT (ProtocolArray != NULL);
> +
> + if (Install) {
> +//
> +// Update the ImageHandle and DriverBindingHandle
> fields of the Driver Binding Protocol
> +//
> +DriverBinding->ImageHandle = ImageHandle;
> +DriverBinding->DriverBindingHandle =
> DriverBindingHandle;
> +
> +return gBS->InstallMultipleProtocolInterfaces (
> + >DriverBindingHandle,
> + ProtocolArray[0].Guid,
> ProtocolArray[0].Interface,
> + ProtocolArray[1].Guid,
> ProtocolArray[1].Interface,
> + ProtocolArray[2].Guid,
> ProtocolArray[2].Interface,
> + ProtocolArray[3].Guid,
> ProtocolArray[3].Interface,
> + ProtocolArray[4].Guid,
> ProtocolArray[4].Interface,
> + ProtocolArray[5].Guid,
> ProtocolArray[5].Interface,
> + ProtocolArray[6].Guid,
> ProtocolArray[6].Interface,
> + NULL
> + );
> + } else {
> +return gBS->UninstallMultipleProtocolInterfaces (
> + DriverBinding->DriverBindingHandle,
> + ProtocolArray[0].Guid,
> ProtocolArray[0].Interface,
> + ProtocolArray[1].Guid,
> ProtocolArray[1].Interface,
> + ProtocolArray[2].Guid,
> ProtocolArray[2].Interface,
> + ProtocolArray[3].Guid,
> ProtocolArray[3].Interface,
> + ProtocolArray[4].Guid,
> ProtocolArray[4].Interface,
> + ProtocolArray[5].Guid,
> ProtocolArray[5].Interface,
> + ProtocolArray[6].Guid,
> ProtocolArray[6].Interface,
> + NULL
> + );
> + }
> +}
> +
> +
> +
> +static
> +EFI_STATUS
> +EFIAPI
> +EfiLibProcessAllDriverProtocols (
> + IN CONST EFI_HANDLE
> ImageHandle,
> + IN EFI_DRIVER_BINDING_PROTOCOL
> *DriverBinding,
> + IN EFI_HANDLE
> DriverBindingHandle,
> + IN BOOLEAN Install,
> + IN CONST EFI_COMPONENT_NAME_PROTOCOL
> *ComponentName,
> + IN CONST EFI_DRIVER_CONFIGURATION_PROTOCOL
> *DriverConfiguration,
> + IN CONST EFI_DRIVER_DIAGNOSTICS_PROTOCOL
> *DriverDiagnostics
> + )
> +{
> + EFI_STATUS Status;
>
[edk2] [PATCH 2/2] DynamicTablesPkg/AcpiSpcrLibArm: Support 16550 UART.
Add support for 16550 UART to ACPI SPCR table as it is a
supported UART type by HLOS.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ashish Singhal
---
DynamicTablesPkg/Library/Acpi/Arm/AcpiSpcrLibArm/SpcrGenerator.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSpcrLibArm/SpcrGenerator.c
b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSpcrLibArm/SpcrGenerator.c
index 23d3a50..a4654ac 100644
--- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSpcrLibArm/SpcrGenerator.c
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSpcrLibArm/SpcrGenerator.c
@@ -189,6 +189,8 @@ BuildSpcrTable (
(SerialPortInfo->PortSubtype !=
EFI_ACPI_DBG2_PORT_SUBTYPE_SERIAL_ARM_SBSA_GENERIC_UART) &&
(SerialPortInfo->PortSubtype !=
+ EFI_ACPI_DBG2_PORT_SUBTYPE_SERIAL_FULL_16550) &&
+ (SerialPortInfo->PortSubtype !=
EFI_ACPI_DBG2_PORT_SUBTYPE_SERIAL_DCC)) {
Status = EFI_INVALID_PARAMETER;
DEBUG ((
--
2.7.4
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH 1/2] DynamicTablesPkg/DynamicTableManagerDxe: Update DEPEX
DynamicTableManagerDxe initialization fails if gEdkiiDynamicTableFactoryProtocolGuid, gEdkiiConfigurationManagerProtocolGuid and gEfiAcpiTableProtocolGuid are not already available. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Ashish Singhal --- .../Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.inf | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.inf b/DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.inf index 2aeaf15..fef8b20 100644 --- a/DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.inf +++ b/DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.inf @@ -46,5 +46,7 @@ gEdkiiDynamicTableFactoryProtocolGuid [Depex] - gEdkiiConfigurationManagerProtocolGuid + gEfiAcpiTableProtocolGuid AND + gEdkiiConfigurationManagerProtocolGuid AND + gEdkiiDynamicTableFactoryProtocolGuid -- 2.7.4 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH 0/2] DynamicTablesPkg Updates
DynamicTablesPkg/DynamicTableManagerDxe: Update DEPEX This patch adds appropriate dependencies to DynamicTableManagerDxe. The initialization function fails if gEdkiiDynamicTableFactoryProtocolGuid and gEdkiiConfigurationManagerProtocolGuid are not present already. Since we are not relying on a callback but locating these in initialization, we should add these dependencies. Towards the end of initialization function where we build and install ACPI tables, we locate gEfiAcpiTableProtocolGuid and return a failure is not present. We need to add approriate dependency for this as well. Adding these proper dependencies would make the code not rely on drivers forcefully dispatched in a particular order DynamicTablesPkg/AcpiSpcrLibArm: Support 16550 UART. This patch adds support for 16550 UART in ACPI SPCR table. HLOS support for this type of UART is already present. Both the patches have been verified to work on hardware. Ashish Singhal (2): DynamicTablesPkg/DynamicTableManagerDxe: Update DEPEX DynamicTablesPkg/AcpiSpcrLibArm: Support 16550 UART. .../Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.inf | 4 +++- DynamicTablesPkg/Library/Acpi/Arm/AcpiSpcrLibArm/SpcrGenerator.c | 2 ++ 2 files changed, 5 insertions(+), 1 deletion(-) -- 2.7.4 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH] MdePkg/UefiLib: Simplify protocol un/installation abstraction
Hello Mike/Lao,
Were you able to have a look at this?
Thanks
Ashish
-Original Message-
From: Ashish Singhal
Sent: Monday, February 4, 2019 1:16 PM
To: edk2-devel@lists.01.org
Cc: michael.d.kin...@intel.com; liming@intel.com; Ashish Singhal
Subject: [PATCH] MdePkg/UefiLib: Simplify protocol un/installation abstraction
Add helper functions to operate upon protocol installation and
uninstallation instead of every function doing it by itself.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ashish Singhal
---
MdePkg/Library/UefiLib/UefiDriverModel.c | 2040 +-
1 file changed, 342 insertions(+), 1698 deletions(-)
diff --git a/MdePkg/Library/UefiLib/UefiDriverModel.c
b/MdePkg/Library/UefiLib/UefiDriverModel.c
index 262d8bc..268edf7 100644
--- a/MdePkg/Library/UefiLib/UefiDriverModel.c
+++ b/MdePkg/Library/UefiLib/UefiDriverModel.c
@@ -17,6 +17,290 @@
#include "UefiLibInternal.h"
+
+#define MAX_SUPPORTED_PROTOCOLS 7
+typedef struct {
+ EFI_GUID *Guid;
+ VOID *Interface;
+} EFI_PROCESS_PROTOCOL;
+
+
+static
+EFI_STATUS
+EFIAPI
+EfiLibProcessProtocols (
+ IN CONST EFI_HANDLE ImageHandle,
+ IN EFI_DRIVER_BINDING_PROTOCOL *DriverBinding,
+ IN EFI_HANDLE DriverBindingHandle,
+ IN BOOLEAN Install,
+ IN CONST EFI_PROCESS_PROTOCOL *ProtocolArray
+ )
+{
+ ASSERT (DriverBinding != NULL);
+ ASSERT (ProtocolArray != NULL);
+
+ if (Install) {
+//
+// Update the ImageHandle and DriverBindingHandle fields of the Driver
Binding Protocol
+//
+DriverBinding->ImageHandle = ImageHandle;
+DriverBinding->DriverBindingHandle = DriverBindingHandle;
+
+return gBS->InstallMultipleProtocolInterfaces (
+ >DriverBindingHandle,
+ ProtocolArray[0].Guid, ProtocolArray[0].Interface,
+ ProtocolArray[1].Guid, ProtocolArray[1].Interface,
+ ProtocolArray[2].Guid, ProtocolArray[2].Interface,
+ ProtocolArray[3].Guid, ProtocolArray[3].Interface,
+ ProtocolArray[4].Guid, ProtocolArray[4].Interface,
+ ProtocolArray[5].Guid, ProtocolArray[5].Interface,
+ ProtocolArray[6].Guid, ProtocolArray[6].Interface,
+ NULL
+ );
+ } else {
+return gBS->UninstallMultipleProtocolInterfaces (
+ DriverBinding->DriverBindingHandle,
+ ProtocolArray[0].Guid, ProtocolArray[0].Interface,
+ ProtocolArray[1].Guid, ProtocolArray[1].Interface,
+ ProtocolArray[2].Guid, ProtocolArray[2].Interface,
+ ProtocolArray[3].Guid, ProtocolArray[3].Interface,
+ ProtocolArray[4].Guid, ProtocolArray[4].Interface,
+ ProtocolArray[5].Guid, ProtocolArray[5].Interface,
+ ProtocolArray[6].Guid, ProtocolArray[6].Interface,
+ NULL
+ );
+ }
+}
+
+
+
+static
+EFI_STATUS
+EFIAPI
+EfiLibProcessAllDriverProtocols (
+ IN CONST EFI_HANDLE ImageHandle,
+ IN EFI_DRIVER_BINDING_PROTOCOL *DriverBinding,
+ IN EFI_HANDLE DriverBindingHandle,
+ IN BOOLEAN Install,
+ IN CONST EFI_COMPONENT_NAME_PROTOCOL*ComponentName,
+ IN CONST EFI_DRIVER_CONFIGURATION_PROTOCOL *DriverConfiguration,
+ IN CONST EFI_DRIVER_DIAGNOSTICS_PROTOCOL*DriverDiagnostics
+ )
+{
+ EFI_STATUS Status;
+ EFI_PROCESS_PROTOCOL ProtocolArray[MAX_SUPPORTED_PROTOCOLS];
+ UINT8ProtocolCount;
+
+ ASSERT (DriverBinding != NULL);
+
+ //
+ // ZI the ProtocolArray structure. Both InstallMultipleProtocolInterfaces
+ // and UninstallMultipleProtocolInterfaces would stop processing
ProtocolArray
+ // elements as soon as they encounter a NULL.
+ //
+ ZeroMem(ProtocolArray, sizeof(ProtocolArray));
+ ProtocolCount = 0;
+
+ //
+ // Populate ProtocolArray with valid protocol interfaces.
+ //
+ ProtocolArray[ProtocolCount].Guid =
+ ProtocolArray[ProtocolCount].Interface = DriverBinding;
+ ProtocolCount++;
+
+ if (ComponentName != NULL && !FeaturePcdGet(PcdComponentNameDisable)) {
+ProtocolArray[ProtocolCount].Guid =
+ProtocolArray[ProtocolCount].Interface = (VOID *)ComponentName;
+ProtocolCount++;
+ }
+
+ if (DriverConfiguration != NULL) {
+ProtocolArray[ProtocolCount].Guid =
+ProtocolArray[ProtocolCount].Interface = (VOID *)DriverConfiguration;
+ProtocolCount++;
+ }
+
+ if (DriverDiagnostics != NULL &&
!FeaturePcdGet(PcdDriverDiagnosticsDisable)) {
+ProtocolArray[ProtocolCount].Guid =
+ProtocolArray[ProtocolCount].Interface = (VOID *)DriverDiagnostics;
+ProtocolCount++;
+ }
+
+ Status = EfiLibProcessProtocols (
+
Re: [edk2] [PATCH v3 0/2] Provide UEFILib functions for protocol uninstallation.
Mike, I have refactored my change and with this I am not seeing any size bloating in DEBUG or RELEASE images I am building for my platform. The new change has been submitted for consideration. Thanks Ashish -Original Message- From: Ashish Singhal Sent: Tuesday, January 8, 2019 8:02 PM To: 'Kinney, Michael D' ; edk2-devel@lists.01.org; Gao, Liming ; Fu, Siyuan ; Wu, Jiaxin Subject: RE: [PATCH v3 0/2] Provide UEFILib functions for protocol uninstallation. Hi Mike, Thanks for the analysis. I will investigate at my end what could be causing this. Meanwhile, to fix the issue do you want to submit PATCH v2 1/4 and 2/4 patches? These 2 patches are just what you suggested. Thanks Ashish -Original Message- From: Kinney, Michael D Sent: Tuesday, January 8, 2019 7:58 PM To: Ashish Singhal ; edk2-devel@lists.01.org; Gao, Liming ; Fu, Siyuan ; Wu, Jiaxin ; Kinney, Michael D Subject: RE: [PATCH v3 0/2] Provide UEFILib functions for protocol uninstallation. Ashish, When I do full platform builds and individual driver builds, the result is always a little bigger with the V3 version of the patch. DEBUG DEBUG+Patch Delta RELEASE RELEASE+Patch Delta * *** * *** * * Quark IA32 FVMAIN2337360 2339408 2048 1181168 1182992 1824 Quark IA32 FVMAIN_COMPACT 814664 815272608 516208 516904696 DiskIoDxe.efi 1875218880128 5824 5952128 DiskIoDxe.ROM 1024010240 0 3419 3569150 OVMF X64 DXEFV 4447368 4456968 9600 2956808 2966312 9504 OVMF X64 FVMAIN_COMPACT 1164264 1164784520 912528 913048520 I recommend the install APIs remain unchanged, and the uninstall APIs use the same logic as the install APIs. Best regards, Mike > -Original Message- > From: Ashish Singhal [mailto:ashishsin...@nvidia.com] > Sent: Tuesday, January 8, 2019 9:24 AM > To: Kinney, Michael D ; > edk2-devel@lists.01.org; Gao, Liming ; Fu, > Siyuan ; Wu, Jiaxin > Subject: RE: [PATCH v3 0/2] Provide UEFILib functions for protocol > uninstallation. > > Thanks Mike. Please let me know if you have any more questions and/or > comments. > > Thanks > Ashish > > -Original Message- > From: Kinney, Michael D > Sent: Tuesday, January 8, 2019 9:26 AM > To: Ashish Singhal ; edk2- > de...@lists.01.org; Gao, Liming ; Fu, Siyuan > ; Wu, Jiaxin ; Kinney, > Michael D > Subject: RE: [PATCH v3 0/2] Provide UEFILib functions for protocol > uninstallation. > > Ashish, > > Good point. I was looking at the code size of a single uncompressed > UEFI Driver (DiskIoDxe). I suspect that the patch provides a more > consistent pattern across all drivers that use these UefiLib APIs, and > then provides better compression on an FV that contains many UEFI > Drivers. > > I also suspect there may be a small size increase for the single > compressed UEFI Driver use case for PCI Option ROMs. > > I will run a few more experiments this morning. > > Thanks, > > Mike > > > -Original Message- > > From: Ashish Singhal [mailto:ashishsin...@nvidia.com] > > Sent: Monday, January 7, 2019 2:51 PM > > To: Kinney, Michael D ; > > edk2-devel@lists.01.org; Gao, Liming > ; Fu, > > Siyuan ; Wu, Jiaxin > > > Subject: RE: [PATCH v3 0/2] Provide UEFILib functions > for protocol > > uninstallation. > > > > Hi Mike, > > > > I build both DEBUG and RELEASE variant of the library > and they both > > built a few KB less in size compared to what is in > tip right now. Can > > you please help me with the optimization settings you > have enabled so > > that I can try the same at my end? Also, if you want, > we can look at > > the optimization part going forward and fix the issue > first by pushing > > in PATCH v2 1/4 and PATCH v2 2/4 which just adds > uninstallation APIs > > keeping the same code structure as for install. > > > > Thanks > > Ashish > > > > -Original Message- > > From: Kinney, Michael D > > Sent: Monday, January 7, 2019 3:23 PM > > To: Ashish Singhal ; edk2- > > de...@lists.01.org; Gao, Liming > ; Fu, Siyuan > > ; Wu, Jiaxin > ; Kinney, > > Michael D > > Subject: RE: [PATCH v3 0/2] Provide UEFILib functions > for protocol > > uninstallation. > > > > Hi Ashish, > > > > My main concern with this patch is that the generated > code for > > optimized RELEASE builds is not as small. > > > > From a
[edk2] [PATCH] MdePkg/UefiLib: Simplify protocol un/installation abstraction
Add helper functions to operate upon protocol installation and
uninstallation instead of every function doing it by itself.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ashish Singhal
---
MdePkg/Library/UefiLib/UefiDriverModel.c | 2040 +-
1 file changed, 342 insertions(+), 1698 deletions(-)
diff --git a/MdePkg/Library/UefiLib/UefiDriverModel.c
b/MdePkg/Library/UefiLib/UefiDriverModel.c
index 262d8bc..268edf7 100644
--- a/MdePkg/Library/UefiLib/UefiDriverModel.c
+++ b/MdePkg/Library/UefiLib/UefiDriverModel.c
@@ -17,6 +17,290 @@
#include "UefiLibInternal.h"
+
+#define MAX_SUPPORTED_PROTOCOLS 7
+typedef struct {
+ EFI_GUID *Guid;
+ VOID *Interface;
+} EFI_PROCESS_PROTOCOL;
+
+
+static
+EFI_STATUS
+EFIAPI
+EfiLibProcessProtocols (
+ IN CONST EFI_HANDLE ImageHandle,
+ IN EFI_DRIVER_BINDING_PROTOCOL *DriverBinding,
+ IN EFI_HANDLE DriverBindingHandle,
+ IN BOOLEAN Install,
+ IN CONST EFI_PROCESS_PROTOCOL *ProtocolArray
+ )
+{
+ ASSERT (DriverBinding != NULL);
+ ASSERT (ProtocolArray != NULL);
+
+ if (Install) {
+//
+// Update the ImageHandle and DriverBindingHandle fields of the Driver
Binding Protocol
+//
+DriverBinding->ImageHandle = ImageHandle;
+DriverBinding->DriverBindingHandle = DriverBindingHandle;
+
+return gBS->InstallMultipleProtocolInterfaces (
+ >DriverBindingHandle,
+ ProtocolArray[0].Guid, ProtocolArray[0].Interface,
+ ProtocolArray[1].Guid, ProtocolArray[1].Interface,
+ ProtocolArray[2].Guid, ProtocolArray[2].Interface,
+ ProtocolArray[3].Guid, ProtocolArray[3].Interface,
+ ProtocolArray[4].Guid, ProtocolArray[4].Interface,
+ ProtocolArray[5].Guid, ProtocolArray[5].Interface,
+ ProtocolArray[6].Guid, ProtocolArray[6].Interface,
+ NULL
+ );
+ } else {
+return gBS->UninstallMultipleProtocolInterfaces (
+ DriverBinding->DriverBindingHandle,
+ ProtocolArray[0].Guid, ProtocolArray[0].Interface,
+ ProtocolArray[1].Guid, ProtocolArray[1].Interface,
+ ProtocolArray[2].Guid, ProtocolArray[2].Interface,
+ ProtocolArray[3].Guid, ProtocolArray[3].Interface,
+ ProtocolArray[4].Guid, ProtocolArray[4].Interface,
+ ProtocolArray[5].Guid, ProtocolArray[5].Interface,
+ ProtocolArray[6].Guid, ProtocolArray[6].Interface,
+ NULL
+ );
+ }
+}
+
+
+
+static
+EFI_STATUS
+EFIAPI
+EfiLibProcessAllDriverProtocols (
+ IN CONST EFI_HANDLE ImageHandle,
+ IN EFI_DRIVER_BINDING_PROTOCOL *DriverBinding,
+ IN EFI_HANDLE DriverBindingHandle,
+ IN BOOLEAN Install,
+ IN CONST EFI_COMPONENT_NAME_PROTOCOL*ComponentName,
+ IN CONST EFI_DRIVER_CONFIGURATION_PROTOCOL *DriverConfiguration,
+ IN CONST EFI_DRIVER_DIAGNOSTICS_PROTOCOL*DriverDiagnostics
+ )
+{
+ EFI_STATUS Status;
+ EFI_PROCESS_PROTOCOL ProtocolArray[MAX_SUPPORTED_PROTOCOLS];
+ UINT8ProtocolCount;
+
+ ASSERT (DriverBinding != NULL);
+
+ //
+ // ZI the ProtocolArray structure. Both InstallMultipleProtocolInterfaces
+ // and UninstallMultipleProtocolInterfaces would stop processing
ProtocolArray
+ // elements as soon as they encounter a NULL.
+ //
+ ZeroMem(ProtocolArray, sizeof(ProtocolArray));
+ ProtocolCount = 0;
+
+ //
+ // Populate ProtocolArray with valid protocol interfaces.
+ //
+ ProtocolArray[ProtocolCount].Guid =
+ ProtocolArray[ProtocolCount].Interface = DriverBinding;
+ ProtocolCount++;
+
+ if (ComponentName != NULL && !FeaturePcdGet(PcdComponentNameDisable)) {
+ProtocolArray[ProtocolCount].Guid =
+ProtocolArray[ProtocolCount].Interface = (VOID *)ComponentName;
+ProtocolCount++;
+ }
+
+ if (DriverConfiguration != NULL) {
+ProtocolArray[ProtocolCount].Guid =
+ProtocolArray[ProtocolCount].Interface = (VOID *)DriverConfiguration;
+ProtocolCount++;
+ }
+
+ if (DriverDiagnostics != NULL &&
!FeaturePcdGet(PcdDriverDiagnosticsDisable)) {
+ProtocolArray[ProtocolCount].Guid =
+ProtocolArray[ProtocolCount].Interface = (VOID *)DriverDiagnostics;
+ProtocolCount++;
+ }
+
+ Status = EfiLibProcessProtocols (
+ ImageHandle,
+ DriverBinding,
+ DriverBindingHandle,
+ Install,
+ ProtocolArray
+ );
+
+ //
+ // ASSERT if the call to InstallMultipleProtocolInterfaces() or
+ // UninstallMultipleProtocolInterfaces() failed
+ //
+ ASSERT_EFI_ERROR (Status);
+
+ return
[edk2] Fmp Payload Header Usage
Hello Michael/Liming, I am trying to use FmpDevicePkg for FMP based capsule update and am failing in FmpPayloadHeaderLib while verifying FMP payload header. I am building capsule and payload using FDF file itself and not calling Capsule tools explicitly from basetools. Is there a special build flag I need to provide to add FMP payload header to my payload? Thanks Ashish --- 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
Re: [edk2] [PATCH] NetworkPkg: Protocol Uninstallation Cleanup
Thanks Jiaxin. Please let me know if anything else is needed to mainline it.
-Original Message-
From: Wu, Jiaxin
Sent: Thursday, January 10, 2019 6:01 PM
To: Ashish Singhal ; edk2-devel@lists.01.org
Cc: Fu, Siyuan
Subject: RE: [PATCH] NetworkPkg: Protocol Uninstallation Cleanup
Looks good to me.
Reviewed-by: Wu Jiaxin
Thanks,
Jiaxin
> -Original Message-
> From: Ashish Singhal [mailto:ashishsin...@nvidia.com]
> Sent: Friday, January 11, 2019 3:27 AM
> To: edk2-devel@lists.01.org
> Cc: Fu, Siyuan ; Wu, Jiaxin
> ; Ashish Singhal
> Subject: [PATCH] NetworkPkg: Protocol Uninstallation Cleanup
>
> Use UEFILib provided protocol uninstallation abstraction instead of
> direct API for a proper cleanup.
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1444
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ashish Singhal
> ---
> NetworkPkg/DnsDxe/DnsDriver.c | 30 ++
> NetworkPkg/HttpBootDxe/HttpBootDxe.c | 15 +--
> NetworkPkg/HttpDxe/HttpDriver.c | 15 +--
> NetworkPkg/IpSecDxe/IpSecDriver.c | 15 +--
> NetworkPkg/TcpDxe/TcpDriver.c | 15 +--
> NetworkPkg/UefiPxeBcDxe/PxeBcDriver.c | 15 +--
> 6 files changed, 35 insertions(+), 70 deletions(-)
>
> diff --git a/NetworkPkg/DnsDxe/DnsDriver.c
> b/NetworkPkg/DnsDxe/DnsDriver.c index 1f9b924..b74f5ba 100644
> --- a/NetworkPkg/DnsDxe/DnsDriver.c
> +++ b/NetworkPkg/DnsDxe/DnsDriver.c
> @@ -510,28 +510,18 @@ DnsDriverEntryPoint (
> FreePool (mDriverData);
>
>Error2:
> - gBS->UninstallMultipleProtocolInterfaces (
> - gDns6DriverBinding.DriverBindingHandle,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - NULL
> - );
> + EfiLibUninstallDriverBindingComponentName2 (
> + ,
> + ,
> +
> + );
>
>Error1:
> -gBS->UninstallMultipleProtocolInterfaces (
> - ImageHandle,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - NULL
> - );
> +EfiLibUninstallDriverBindingComponentName2 (
> + ,
> + ,
> +
> + );
>
>return Status;
> }
> diff --git a/NetworkPkg/HttpBootDxe/HttpBootDxe.c
> b/NetworkPkg/HttpBootDxe/HttpBootDxe.c
> index 7ec06f960..0b16f95 100644
> --- a/NetworkPkg/HttpBootDxe/HttpBootDxe.c
> +++ b/NetworkPkg/HttpBootDxe/HttpBootDxe.c
> @@ -1327,16 +1327,11 @@ HttpBootDxeDriverEntryPoint (
>
> );
>if (EFI_ERROR (Status)) {
> -gBS->UninstallMultipleProtocolInterfaces(
> - ImageHandle,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - NULL
> - );
> +EfiLibUninstallDriverBindingComponentName2(
> + ,
> + ,
> +
> + );
>}
>return Status;
> }
> diff --git a/NetworkPkg/HttpDxe/HttpDriver.c
> b/NetworkPkg/HttpDxe/HttpDriver.c index 8df984d..979d76d 100644
> --- a/NetworkPkg/HttpDxe/HttpDriver.c
> +++ b/NetworkPkg/HttpDxe/HttpDriver.c
> @@ -230,16 +230,11 @@ HttpDxeDriverEntryPoint (
>
> );
>if (EFI_ERROR (Status)) {
> -gBS->UninstallMultipleProtocolInterfaces (
> - ImageHandle,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - NULL
> - );
> +EfiLibUninstallDriverBindingComponentName2 (
> + ,
> + ,
> +
> + );
>}
>return Status;
> }
> diff --git a/NetworkPkg/IpSecDxe/IpSecDriver.c
> b/NetworkPkg/IpSecDxe/IpSecDriver.c
> index f66f89a..3082d99 100644
> --- a/NetworkPkg/IpSecDxe/IpSecDriver.c
> +++ b/NetworkPkg/IpSecDxe/IpSecDriver.c
> @@ -631,16 +631,11 @@ IpSecDriverEntryPoint (
>return Status;
>
> ON_UNINSTALL_IPSEC4_DB:
> - gBS->UninstallMultipleProtocolInterfaces (
> - ImageHandle,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - NULL
> - );
> + EfiLibUninstallDriverBindingComponentName2 (
> +,
> +,
> +
> +);
>
> ON_UNINSTALL_IPSEC:
>gBS->UninstallProtocolInterface (
> diff --git a/NetworkPkg/TcpDxe/TcpDriver.c
> b/NetworkPkg/TcpDxe/TcpDriver.c index 2d4b16c..00d172b 100644
> --- a/NetworkPkg/TcpDxe/TcpDriver.c
> +++ b/NetworkPkg/TcpDxe/TcpDriver.c
> @@ -202,16 +202,11 @@ TcpDriverEntryPoint (
>
>
[edk2] [PATCH] NetworkPkg: Protocol Uninstallation Cleanup
Use UEFILib provided protocol uninstallation abstraction
instead of direct API for a proper cleanup.
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1444
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ashish Singhal
---
NetworkPkg/DnsDxe/DnsDriver.c | 30 ++
NetworkPkg/HttpBootDxe/HttpBootDxe.c | 15 +--
NetworkPkg/HttpDxe/HttpDriver.c | 15 +--
NetworkPkg/IpSecDxe/IpSecDriver.c | 15 +--
NetworkPkg/TcpDxe/TcpDriver.c | 15 +--
NetworkPkg/UefiPxeBcDxe/PxeBcDriver.c | 15 +--
6 files changed, 35 insertions(+), 70 deletions(-)
diff --git a/NetworkPkg/DnsDxe/DnsDriver.c b/NetworkPkg/DnsDxe/DnsDriver.c
index 1f9b924..b74f5ba 100644
--- a/NetworkPkg/DnsDxe/DnsDriver.c
+++ b/NetworkPkg/DnsDxe/DnsDriver.c
@@ -510,28 +510,18 @@ DnsDriverEntryPoint (
FreePool (mDriverData);
Error2:
- gBS->UninstallMultipleProtocolInterfaces (
- gDns6DriverBinding.DriverBindingHandle,
- ,
- ,
- ,
- ,
- ,
- ,
- NULL
- );
+ EfiLibUninstallDriverBindingComponentName2 (
+ ,
+ ,
+
+ );
Error1:
-gBS->UninstallMultipleProtocolInterfaces (
- ImageHandle,
- ,
- ,
- ,
- ,
- ,
- ,
- NULL
- );
+EfiLibUninstallDriverBindingComponentName2 (
+ ,
+ ,
+
+ );
return Status;
}
diff --git a/NetworkPkg/HttpBootDxe/HttpBootDxe.c
b/NetworkPkg/HttpBootDxe/HttpBootDxe.c
index 7ec06f960..0b16f95 100644
--- a/NetworkPkg/HttpBootDxe/HttpBootDxe.c
+++ b/NetworkPkg/HttpBootDxe/HttpBootDxe.c
@@ -1327,16 +1327,11 @@ HttpBootDxeDriverEntryPoint (
);
if (EFI_ERROR (Status)) {
-gBS->UninstallMultipleProtocolInterfaces(
- ImageHandle,
- ,
- ,
- ,
- ,
- ,
- ,
- NULL
- );
+EfiLibUninstallDriverBindingComponentName2(
+ ,
+ ,
+
+ );
}
return Status;
}
diff --git a/NetworkPkg/HttpDxe/HttpDriver.c b/NetworkPkg/HttpDxe/HttpDriver.c
index 8df984d..979d76d 100644
--- a/NetworkPkg/HttpDxe/HttpDriver.c
+++ b/NetworkPkg/HttpDxe/HttpDriver.c
@@ -230,16 +230,11 @@ HttpDxeDriverEntryPoint (
);
if (EFI_ERROR (Status)) {
-gBS->UninstallMultipleProtocolInterfaces (
- ImageHandle,
- ,
- ,
- ,
- ,
- ,
- ,
- NULL
- );
+EfiLibUninstallDriverBindingComponentName2 (
+ ,
+ ,
+
+ );
}
return Status;
}
diff --git a/NetworkPkg/IpSecDxe/IpSecDriver.c
b/NetworkPkg/IpSecDxe/IpSecDriver.c
index f66f89a..3082d99 100644
--- a/NetworkPkg/IpSecDxe/IpSecDriver.c
+++ b/NetworkPkg/IpSecDxe/IpSecDriver.c
@@ -631,16 +631,11 @@ IpSecDriverEntryPoint (
return Status;
ON_UNINSTALL_IPSEC4_DB:
- gBS->UninstallMultipleProtocolInterfaces (
- ImageHandle,
- ,
- ,
- ,
- ,
- ,
- ,
- NULL
- );
+ EfiLibUninstallDriverBindingComponentName2 (
+,
+,
+
+);
ON_UNINSTALL_IPSEC:
gBS->UninstallProtocolInterface (
diff --git a/NetworkPkg/TcpDxe/TcpDriver.c b/NetworkPkg/TcpDxe/TcpDriver.c
index 2d4b16c..00d172b 100644
--- a/NetworkPkg/TcpDxe/TcpDriver.c
+++ b/NetworkPkg/TcpDxe/TcpDriver.c
@@ -202,16 +202,11 @@ TcpDriverEntryPoint (
);
if (EFI_ERROR (Status)) {
-gBS->UninstallMultipleProtocolInterfaces (
- ImageHandle,
- ,
- ,
- ,
- ,
- ,
- ,
- NULL
- );
+EfiLibUninstallDriverBindingComponentName2 (
+ ,
+ ,
+
+ );
return Status;
}
diff --git a/NetworkPkg/UefiPxeBcDxe/PxeBcDriver.c
b/NetworkPkg/UefiPxeBcDxe/PxeBcDriver.c
index 0ab640b..f25c27a 100644
--- a/NetworkPkg/UefiPxeBcDxe/PxeBcDriver.c
+++ b/NetworkPkg/UefiPxeBcDxe/PxeBcDriver.c
@@ -1269,16 +1269,11 @@ PxeBcDriverEntryPoint (
);
if (EFI_ERROR (Status)) {
-gBS->UninstallMultipleProtocolInterfaces (
- ImageHandle,
- ,
- ,
- ,
- ,
- ,
- ,
- NULL
- );
+EfiLibUninstallDriverBindingComponentName2 (
+ ,
+ ,
+
+ );
}
return Status;
--
2.7.4
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v4 0/2] Provide UEFILib functions for protocol uninstallation
Thanks everyone. -Original Message- From: Gao, Liming Sent: Thursday, January 10, 2019 8:39 AM To: Ashish Singhal ; Kinney, Michael D ; edk2-devel@lists.01.org Cc: Fu, Siyuan ; Wu, Jiaxin Subject: RE: [PATCH v4 0/2] Provide UEFILib functions for protocol uninstallation Pushed them at 0290fca..15666b > -Original Message- > From: Ashish Singhal [mailto:ashishsin...@nvidia.com] > Sent: Thursday, January 10, 2019 11:33 PM > To: Gao, Liming ; Kinney, Michael D > ; edk2-devel@lists.01.org > Cc: Fu, Siyuan ; Wu, Jiaxin > Subject: RE: [PATCH v4 0/2] Provide UEFILib functions for protocol > uninstallation > > Thanks Liming. I have files BZ: > https://bugzilla.tianocore.org/show_bug.cgi?id=1444 to update UEFI drivers to > use new APIs. I have not > assigned it to anyone as there are many drivers across packages that need to > be looked at. I would try to fix the ones I hit an issue with. > > Over the weekend Siyuan approved the patch from PATCH v2 which is exactly > same as in PATCH v4. > > Thanks > Ashish > > -Original Message- > From: Gao, Liming > Sent: Thursday, January 10, 2019 8:23 AM > To: Ashish Singhal ; Kinney, Michael D > ; edk2-devel@lists.01.org > Cc: Fu, Siyuan ; Wu, Jiaxin > Subject: RE: [PATCH v4 0/2] Provide UEFILib functions for protocol > uninstallation > > Ashish: > The MdePkg change is good to me. Reviewed-by: Liming Gao > > Please help submit another BZ to update UefiDriver to uninstall protocol > when failure with new APIs. > > If Siyuan/Jiaxin has no other comments, I will help push this patch set. > > Thanks > Liming > > -Original Message- > > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of > > Ashish Singhal > > Sent: Thursday, January 10, 2019 9:19 AM > > To: Kinney, Michael D ; > > edk2-devel@lists.01.org > > Cc: Fu, Siyuan ; Wu, Jiaxin > > ; Gao, Liming > > Subject: Re: [edk2] [PATCH v4 0/2] Provide UEFILib functions for > > protocol uninstallation > > > > Thanks Mike. Hope to see the patches merged soon. Please let me know if you > > want me to file the BZ. > > > > Hi Liming, > > > > Please let me know if you need me to take care of anything in the patch > > before you push it. > > > > Hi Siyuan/Jiaxin, > > > > I think you reviewed the changes in PATCH v2 which is same as in PATCH v4. > > Please let me know if you have any issues with this going > in. > > > > Thanks > > Ashish > > > > -Original Message- > > From: Kinney, Michael D > > Sent: Wednesday, January 9, 2019 5:56 PM > > To: Ashish Singhal ; edk2-devel@lists.01.org; > > Kinney, Michael D > > Cc: Gao, Liming ; Fu, Siyuan > > ; Wu, Jiaxin > > Subject: RE: [PATCH v4 0/2] Provide UEFILib functions for protocol > > uninstallation > > > > Hi Ashish, > > > > This V4 version of the patch produces the expected size results for > > platform and driver builds. > > > > There are some very minor issues with some extra carriage returns, but > > those can be handled by Liming when the patch series is committed. > > > > I may be good to have an additional BZ to use these new APIs from all > > UEFI Driver Model drivers that have failure paths in their entry point or > > support the unload feature. > > Those updates can be done later. > > > > Thanks, > > > > Mike > > > > > -Original Message- > > > From: Ashish Singhal [mailto:ashishsin...@nvidia.com] > > > Sent: Wednesday, January 9, 2019 12:59 PM > > > To: edk2-devel@lists.01.org > > > Cc: Kinney, Michael D ; Gao, Liming > > > ; Fu, Siyuan ; Wu, Jiaxin > > > ; Ashish Singhal > > > Subject: [PATCH v4 0/2] Provide UEFILib functions for protocol > > > uninstallation > > > > > > An issue was seen in IScsiDxe in NetworkPkg where driver cleanup > > > after initialization failure was not done right. Bug 1428 was filed > > > in this regard. > > > As per discussions with Mike, it was also discussed that having > > > UEFILib provide protocol uninstallation abstraction would help to > > > avoid these issues in the future. Bug 1429 was found to track this. > > > These 2 patches > > > take care of this. > > > > > > > > > Ashish Singhal (2): > > > MdePkg/UefiLib: Abstract driver model protocol uninstallation > > > NetworkPkg/IScsiDxe: Use UEFILib APIs to uninstall protocols. > > > > > > MdePkg/Include/Library/UefiLib.h
Re: [edk2] [PATCH v4 0/2] Provide UEFILib functions for protocol uninstallation
Thanks Liming. I have files BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1444 to update UEFI drivers to use new APIs. I have not assigned it to anyone as there are many drivers across packages that need to be looked at. I would try to fix the ones I hit an issue with. Over the weekend Siyuan approved the patch from PATCH v2 which is exactly same as in PATCH v4. Thanks Ashish -Original Message- From: Gao, Liming Sent: Thursday, January 10, 2019 8:23 AM To: Ashish Singhal ; Kinney, Michael D ; edk2-devel@lists.01.org Cc: Fu, Siyuan ; Wu, Jiaxin Subject: RE: [PATCH v4 0/2] Provide UEFILib functions for protocol uninstallation Ashish: The MdePkg change is good to me. Reviewed-by: Liming Gao Please help submit another BZ to update UefiDriver to uninstall protocol when failure with new APIs. If Siyuan/Jiaxin has no other comments, I will help push this patch set. Thanks Liming > -Original Message- > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of > Ashish Singhal > Sent: Thursday, January 10, 2019 9:19 AM > To: Kinney, Michael D ; > edk2-devel@lists.01.org > Cc: Fu, Siyuan ; Wu, Jiaxin > ; Gao, Liming > Subject: Re: [edk2] [PATCH v4 0/2] Provide UEFILib functions for > protocol uninstallation > > Thanks Mike. Hope to see the patches merged soon. Please let me know if you > want me to file the BZ. > > Hi Liming, > > Please let me know if you need me to take care of anything in the patch > before you push it. > > Hi Siyuan/Jiaxin, > > I think you reviewed the changes in PATCH v2 which is same as in PATCH v4. > Please let me know if you have any issues with this going in. > > Thanks > Ashish > > -Original Message- > From: Kinney, Michael D > Sent: Wednesday, January 9, 2019 5:56 PM > To: Ashish Singhal ; edk2-devel@lists.01.org; > Kinney, Michael D > Cc: Gao, Liming ; Fu, Siyuan > ; Wu, Jiaxin > Subject: RE: [PATCH v4 0/2] Provide UEFILib functions for protocol > uninstallation > > Hi Ashish, > > This V4 version of the patch produces the expected size results for platform > and driver builds. > > There are some very minor issues with some extra carriage returns, but > those can be handled by Liming when the patch series is committed. > > I may be good to have an additional BZ to use these new APIs from all > UEFI Driver Model drivers that have failure paths in their entry point or > support the unload feature. > Those updates can be done later. > > Thanks, > > Mike > > > -Original Message- > > From: Ashish Singhal [mailto:ashishsin...@nvidia.com] > > Sent: Wednesday, January 9, 2019 12:59 PM > > To: edk2-devel@lists.01.org > > Cc: Kinney, Michael D ; Gao, Liming > > ; Fu, Siyuan ; Wu, Jiaxin > > ; Ashish Singhal > > Subject: [PATCH v4 0/2] Provide UEFILib functions for protocol > > uninstallation > > > > An issue was seen in IScsiDxe in NetworkPkg where driver cleanup > > after initialization failure was not done right. Bug 1428 was filed > > in this regard. > > As per discussions with Mike, it was also discussed that having > > UEFILib provide protocol uninstallation abstraction would help to > > avoid these issues in the future. Bug 1429 was found to track this. > > These 2 patches > > take care of this. > > > > > > Ashish Singhal (2): > > MdePkg/UefiLib: Abstract driver model protocol uninstallation > > NetworkPkg/IScsiDxe: Use UEFILib APIs to uninstall protocols. > > > > MdePkg/Include/Library/UefiLib.h | 103 > > MdePkg/Library/UefiLib/UefiDriverModel.c | 972 > > ++- > > NetworkPkg/IScsiDxe/IScsiDriver.c| 31 +- > > 3 files changed, 1085 insertions(+), 21 deletions(-) > > > > -- > > 2.7.4 > > -- > - 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 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v4 0/2] Provide UEFILib functions for protocol uninstallation
Thanks Mike. Hope to see the patches merged soon. Please let me know if you want me to file the BZ. Hi Liming, Please let me know if you need me to take care of anything in the patch before you push it. Hi Siyuan/Jiaxin, I think you reviewed the changes in PATCH v2 which is same as in PATCH v4. Please let me know if you have any issues with this going in. Thanks Ashish -Original Message- From: Kinney, Michael D Sent: Wednesday, January 9, 2019 5:56 PM To: Ashish Singhal ; edk2-devel@lists.01.org; Kinney, Michael D Cc: Gao, Liming ; Fu, Siyuan ; Wu, Jiaxin Subject: RE: [PATCH v4 0/2] Provide UEFILib functions for protocol uninstallation Hi Ashish, This V4 version of the patch produces the expected size results for platform and driver builds. There are some very minor issues with some extra carriage returns, but those can be handled by Liming when the patch series is committed. I may be good to have an additional BZ to use these new APIs from all UEFI Driver Model drivers that have failure paths in their entry point or support the unload feature. Those updates can be done later. Thanks, Mike > -Original Message- > From: Ashish Singhal [mailto:ashishsin...@nvidia.com] > Sent: Wednesday, January 9, 2019 12:59 PM > To: edk2-devel@lists.01.org > Cc: Kinney, Michael D ; Gao, Liming > ; Fu, Siyuan ; Wu, Jiaxin > ; Ashish Singhal > Subject: [PATCH v4 0/2] Provide UEFILib functions for protocol > uninstallation > > An issue was seen in IScsiDxe in NetworkPkg where driver cleanup after > initialization failure was not done right. Bug 1428 was filed in this > regard. > As per discussions with Mike, it was also discussed that having > UEFILib provide protocol uninstallation abstraction would help to > avoid these issues in the future. Bug 1429 was found to track this. > These 2 patches > take care of this. > > > Ashish Singhal (2): > MdePkg/UefiLib: Abstract driver model protocol uninstallation > NetworkPkg/IScsiDxe: Use UEFILib APIs to uninstall protocols. > > MdePkg/Include/Library/UefiLib.h | 103 > MdePkg/Library/UefiLib/UefiDriverModel.c | 972 > ++- > NetworkPkg/IScsiDxe/IScsiDriver.c| 31 +- > 3 files changed, 1085 insertions(+), 21 deletions(-) > > -- > 2.7.4 --- 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
[edk2] [PATCH v4 1/2] MdePkg/UefiLib: Abstract driver model protocol uninstallation
Provided functions in UEFILib that abstract driver model protocol uninstallation. This helps drivers to install and uninstall protocols using a library to keep things seemless. REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1429 Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Ashish Singhal --- MdePkg/Include/Library/UefiLib.h | 103 MdePkg/Library/UefiLib/UefiDriverModel.c | 972 ++- 2 files changed, 1074 insertions(+), 1 deletion(-) diff --git a/MdePkg/Include/Library/UefiLib.h b/MdePkg/Include/Library/UefiLib.h index 468bffc..08222d4 100644 --- a/MdePkg/Include/Library/UefiLib.h +++ b/MdePkg/Include/Library/UefiLib.h @@ -12,6 +12,7 @@ of size reduction when compiler optimization is disabled. If MDEPKG_NDEBUG is defined, then debug and assert related macros wrapped by it are the NULL implementations. +Copyright (c) 2019, NVIDIA Corporation. All rights reserved. Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved. This program and the accompanying materials are licensed and made available under the terms and conditions of the BSD License that accompanies this distribution. @@ -1283,6 +1284,7 @@ AsciiPrintXY ( ... ); + /** Installs and completes the initialization of a Driver Binding Protocol instance. @@ -1316,6 +1318,25 @@ EfiLibInstallDriverBinding ( /** + Uninstalls a Driver Binding Protocol instance. + + If DriverBinding is NULL, then ASSERT(). + If DriverBinding can not be uninstalled, then ASSERT(). + + @param DriverBindingA Driver Binding Protocol instance that this driver produced. + + @retval EFI_SUCCESS The protocol uninstallation successfully completed. + @retval OthersStatus from gBS->UninstallMultipleProtocolInterfaces(). + +**/ +EFI_STATUS +EFIAPI +EfiLibUninstallDriverBinding ( + IN EFI_DRIVER_BINDING_PROTOCOL *DriverBinding + ); + + +/** Installs and completes the initialization of a Driver Binding Protocol instance and optionally installs the Component Name, Driver Configuration and Driver Diagnostics Protocols. @@ -1354,6 +1375,31 @@ EfiLibInstallAllDriverProtocols ( ); +/** + Uninstalls a Driver Binding Protocol instance and optionally uninstalls the + Component Name, Driver Configuration and Driver Diagnostics Protocols. + + If DriverBinding is NULL, then ASSERT(). + If the uninstallation fails, then ASSERT(). + + @param DriverBindingA Driver Binding Protocol instance that this driver produced. + @param ComponentNameA Component Name Protocol instance that this driver produced. + @param DriverConfiguration A Driver Configuration Protocol instance that this driver produced. + @param DriverDiagnosticsA Driver Diagnostics Protocol instance that this driver produced. + + @retval EFI_SUCCESS The protocol uninstallation successfully completed. + @retval OthersStatus from gBS->UninstallMultipleProtocolInterfaces(). + +**/ +EFI_STATUS +EFIAPI +EfiLibUninstallAllDriverProtocols ( + IN EFI_DRIVER_BINDING_PROTOCOL *DriverBinding, + IN CONST EFI_COMPONENT_NAME_PROTOCOL*ComponentName, OPTIONAL + IN CONST EFI_DRIVER_CONFIGURATION_PROTOCOL *DriverConfiguration, OPTIONAL + IN CONST EFI_DRIVER_DIAGNOSTICS_PROTOCOL*DriverDiagnosticsOPTIONAL + ); + /** Installs Driver Binding Protocol with optional Component Name and Component Name 2 Protocols. @@ -1391,6 +1437,29 @@ EfiLibInstallDriverBindingComponentName2 ( /** + Uninstalls Driver Binding Protocol with optional Component Name and Component Name 2 Protocols. + + If DriverBinding is NULL, then ASSERT(). + If the uninstallation fails, then ASSERT(). + + @param DriverBindingA Driver Binding Protocol instance that this driver produced. + @param ComponentNameA Component Name Protocol instance that this driver produced. + @param ComponentName2 A Component Name 2 Protocol instance that this driver produced. + + @retval EFI_SUCCESS The protocol installation successfully completed. + @retval OthersStatus from gBS->UninstallMultipleProtocolInterfaces(). + +**/ +EFI_STATUS +EFIAPI +EfiLibUninstallDriverBindingComponentName2 ( + IN EFI_DRIVER_BINDING_PROTOCOL *DriverBinding, + IN CONST EFI_COMPONENT_NAME_PROTOCOL*ComponentName, OPTIONAL + IN CONST EFI_COMPONENT_NAME2_PROTOCOL *ComponentName2 OPTIONAL + ); + + +/** Installs Driver Binding Protocol with optional Component Name, Component Name 2, Driver Configuration, Driver Configuration 2, Driver Diagnostics, and Driver Diagnostics 2 Protocols. @@ -1434,6 +1503,40 @@ EfiLibInstallAllDriverProtocols2 ( IN CONST EFI_DRIVER_DIAGNOSTICS2_PROTOCOL *DriverDiagnostics2OPTIONAL ); + +/** + Uninstalls Driver Binding Protocol with optional Component Name, Component Name
[edk2] [PATCH v4 2/2] NetworkPkg/IScsiDxe: Use UEFILib APIs to uninstall protocols.
During cleanup in case of initialization failure, some driver bindings are not installed. Using abstractions in UEFILib takes care of it. REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1428 Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Ashish Singhal --- NetworkPkg/IScsiDxe/IScsiDriver.c | 31 +++ 1 file changed, 11 insertions(+), 20 deletions(-) diff --git a/NetworkPkg/IScsiDxe/IScsiDriver.c b/NetworkPkg/IScsiDxe/IScsiDriver.c index 91176e6..8747de7 100644 --- a/NetworkPkg/IScsiDxe/IScsiDriver.c +++ b/NetworkPkg/IScsiDxe/IScsiDriver.c @@ -1,6 +1,7 @@ /** @file The entry point of IScsi driver. +Copyright (c) 2019, NVIDIA Corporation. All rights reserved. Copyright (c) 2004 - 2018, Intel Corporation. All rights reserved. (C) Copyright 2017 Hewlett Packard Enterprise Development LP @@ -1861,28 +1862,18 @@ Error3: ); Error2: - gBS->UninstallMultipleProtocolInterfaces ( - gIScsiIp6DriverBinding.DriverBindingHandle, - , - , - , - , - , - , - NULL - ); + EfiLibUninstallDriverBindingComponentName2 ( +, +, + +); Error1: - gBS->UninstallMultipleProtocolInterfaces ( - ImageHandle, - , - , - , - , - , - , - NULL - ); + EfiLibUninstallDriverBindingComponentName2 ( +, +, + +); return Status; } -- 2.7.4 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH v4 0/2] Provide UEFILib functions for protocol uninstallation
An issue was seen in IScsiDxe in NetworkPkg where driver cleanup after initialization failure was not done right. Bug 1428 was filed in this regard. As per discussions with Mike, it was also discussed that having UEFILib provide protocol uninstallation abstraction would help to avoid these issues in the future. Bug 1429 was found to track this. These 2 patches take care of this. Ashish Singhal (2): MdePkg/UefiLib: Abstract driver model protocol uninstallation NetworkPkg/IScsiDxe: Use UEFILib APIs to uninstall protocols. MdePkg/Include/Library/UefiLib.h | 103 MdePkg/Library/UefiLib/UefiDriverModel.c | 972 ++- NetworkPkg/IScsiDxe/IScsiDriver.c| 31 +- 3 files changed, 1085 insertions(+), 21 deletions(-) -- 2.7.4 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v3 0/2] Provide UEFILib functions for protocol uninstallation.
Hi Mike, Thanks for the analysis. I will investigate at my end what could be causing this. Meanwhile, to fix the issue do you want to submit PATCH v2 1/4 and 2/4 patches? These 2 patches are just what you suggested. Thanks Ashish -Original Message- From: Kinney, Michael D Sent: Tuesday, January 8, 2019 7:58 PM To: Ashish Singhal ; edk2-devel@lists.01.org; Gao, Liming ; Fu, Siyuan ; Wu, Jiaxin ; Kinney, Michael D Subject: RE: [PATCH v3 0/2] Provide UEFILib functions for protocol uninstallation. Ashish, When I do full platform builds and individual driver builds, the result is always a little bigger with the V3 version of the patch. DEBUG DEBUG+Patch Delta RELEASE RELEASE+Patch Delta * *** * *** * * Quark IA32 FVMAIN2337360 2339408 2048 1181168 1182992 1824 Quark IA32 FVMAIN_COMPACT 814664 815272608 516208 516904696 DiskIoDxe.efi 1875218880128 5824 5952128 DiskIoDxe.ROM 1024010240 0 3419 3569150 OVMF X64 DXEFV 4447368 4456968 9600 2956808 2966312 9504 OVMF X64 FVMAIN_COMPACT 1164264 1164784520 912528 913048520 I recommend the install APIs remain unchanged, and the uninstall APIs use the same logic as the install APIs. Best regards, Mike > -Original Message- > From: Ashish Singhal [mailto:ashishsin...@nvidia.com] > Sent: Tuesday, January 8, 2019 9:24 AM > To: Kinney, Michael D ; > edk2-devel@lists.01.org; Gao, Liming ; Fu, > Siyuan ; Wu, Jiaxin > Subject: RE: [PATCH v3 0/2] Provide UEFILib functions for protocol > uninstallation. > > Thanks Mike. Please let me know if you have any more questions and/or > comments. > > Thanks > Ashish > > -Original Message- > From: Kinney, Michael D > Sent: Tuesday, January 8, 2019 9:26 AM > To: Ashish Singhal ; edk2- > de...@lists.01.org; Gao, Liming ; Fu, Siyuan > ; Wu, Jiaxin ; Kinney, > Michael D > Subject: RE: [PATCH v3 0/2] Provide UEFILib functions for protocol > uninstallation. > > Ashish, > > Good point. I was looking at the code size of a single uncompressed > UEFI Driver (DiskIoDxe). I suspect that the patch provides a more > consistent pattern across all drivers that use these UefiLib APIs, and > then provides better compression on an FV that contains many UEFI > Drivers. > > I also suspect there may be a small size increase for the single > compressed UEFI Driver use case for PCI Option ROMs. > > I will run a few more experiments this morning. > > Thanks, > > Mike > > > -Original Message- > > From: Ashish Singhal [mailto:ashishsin...@nvidia.com] > > Sent: Monday, January 7, 2019 2:51 PM > > To: Kinney, Michael D ; > > edk2-devel@lists.01.org; Gao, Liming > ; Fu, > > Siyuan ; Wu, Jiaxin > > > Subject: RE: [PATCH v3 0/2] Provide UEFILib functions > for protocol > > uninstallation. > > > > Hi Mike, > > > > I build both DEBUG and RELEASE variant of the library > and they both > > built a few KB less in size compared to what is in > tip right now. Can > > you please help me with the optimization settings you > have enabled so > > that I can try the same at my end? Also, if you want, > we can look at > > the optimization part going forward and fix the issue > first by pushing > > in PATCH v2 1/4 and PATCH v2 2/4 which just adds > uninstallation APIs > > keeping the same code structure as for install. > > > > Thanks > > Ashish > > > > -Original Message- > > From: Kinney, Michael D > > Sent: Monday, January 7, 2019 3:23 PM > > To: Ashish Singhal ; edk2- > > de...@lists.01.org; Gao, Liming > ; Fu, Siyuan > > ; Wu, Jiaxin > ; Kinney, > > Michael D > > Subject: RE: [PATCH v3 0/2] Provide UEFILib functions > for protocol > > uninstallation. > > > > Hi Ashish, > > > > My main concern with this patch is that the generated > code for > > optimized RELEASE builds is not as small. > > > > From a source maintenance perspective, the patch you > have provided is > > easier to maintain. However, the implementation of > the APIs that > > install protocols was done to make sure the optimizer > produces the > > smallest number of instructions to install the > protocols. > > > > I would prefer the APIs that install protocols remain > unchanged, and > > that only the new APIs to uninstall the protocols be > added. The same >
Re: [edk2] [PATCH v3 0/2] Provide UEFILib functions for protocol uninstallation.
Thanks Mike. Please let me know if you have any more questions and/or comments. Thanks Ashish -Original Message- From: Kinney, Michael D Sent: Tuesday, January 8, 2019 9:26 AM To: Ashish Singhal ; edk2-devel@lists.01.org; Gao, Liming ; Fu, Siyuan ; Wu, Jiaxin ; Kinney, Michael D Subject: RE: [PATCH v3 0/2] Provide UEFILib functions for protocol uninstallation. Ashish, Good point. I was looking at the code size of a single uncompressed UEFI Driver (DiskIoDxe). I suspect that the patch provides a more consistent pattern across all drivers that use these UefiLib APIs, and then provides better compression on an FV that contains many UEFI Drivers. I also suspect there may be a small size increase for the single compressed UEFI Driver use case for PCI Option ROMs. I will run a few more experiments this morning. Thanks, Mike > -Original Message- > From: Ashish Singhal [mailto:ashishsin...@nvidia.com] > Sent: Monday, January 7, 2019 2:51 PM > To: Kinney, Michael D ; > edk2-devel@lists.01.org; Gao, Liming ; Fu, > Siyuan ; Wu, Jiaxin > Subject: RE: [PATCH v3 0/2] Provide UEFILib functions for protocol > uninstallation. > > Hi Mike, > > I build both DEBUG and RELEASE variant of the library and they both > built a few KB less in size compared to what is in tip right now. Can > you please help me with the optimization settings you have enabled so > that I can try the same at my end? Also, if you want, we can look at > the optimization part going forward and fix the issue first by pushing > in PATCH v2 1/4 and PATCH v2 2/4 which just adds uninstallation APIs > keeping the same code structure as for install. > > Thanks > Ashish > > -Original Message- > From: Kinney, Michael D > Sent: Monday, January 7, 2019 3:23 PM > To: Ashish Singhal ; edk2- > de...@lists.01.org; Gao, Liming ; Fu, Siyuan > ; Wu, Jiaxin ; Kinney, > Michael D > Subject: RE: [PATCH v3 0/2] Provide UEFILib functions for protocol > uninstallation. > > Hi Ashish, > > My main concern with this patch is that the generated code for > optimized RELEASE builds is not as small. > > From a source maintenance perspective, the patch you have provided is > easier to maintain. However, the implementation of the APIs that > install protocols was done to make sure the optimizer produces the > smallest number of instructions to install the protocols. > > I would prefer the APIs that install protocols remain unchanged, and > that only the new APIs to uninstall the protocols be added. The same > approach could be taken in the implementation to produce the exact > right form of the uninstall action that is guaranteed to succeed if > the uninstall API matches the API that was used to install. > > Thanks, > > Mike > > > -Original Message- > > From: Ashish Singhal [mailto:ashishsin...@nvidia.com] > > Sent: Monday, January 7, 2019 6:02 AM > > To: edk2-devel@lists.01.org; Kinney, Michael D > > ; Gao, Liming > ; Fu, > > Siyuan ; Wu, Jiaxin > > > Subject: RE: [PATCH v3 0/2] Provide UEFILib functions > for protocol > > uninstallation. > > > > + Maintainers > > > > -Original Message- > > From: Ashish Singhal > > Sent: Sunday, January 6, 2019 9:38 PM > > To: edk2-devel@lists.01.org > > Cc: Ashish Singhal > > Subject: [PATCH v3 0/2] Provide UEFILib functions for > protocol > > uninstallation. > > > > An issue was seen in IScsiDxe in NetworkPkg where > driver cleanup after > > initialization failure was not done right. Bug 1428 > was filed in this > > regard. > > As per discussions with Mike, it was also discussed > that having > > UEFILib provide protocol uninstallation abstraction > would help to > > avoid these issues in the future. Bug 1429 was found > to track this. > > The first 2 patches take care of this. > > > > Patch number 1 also simplifies the UEFILib protocol > installation and > > uninstallation abstraction by adding a helper > function doing > > operations instead of every public function. > > > > Ashish Singhal (2): > > MdePkg/UefiLib: Abstract driver model protocol > uninstallation > > NetworkPkg/IScsiDxe: Use UEFILib APIs to uninstall > protocols. > > > > MdePkg/Include/Library/UefiLib.h | 103 +++ > > MdePkg/Library/UefiLib/UefiDriverModel.c | 1186 > > -- > > NetworkPkg/IScsiDxe/IScsiDriver.c| 31 +- > > 3 files changed, 435 insertions(+), 885 deletions(-) > > > > -- >
Re: [edk2] [PATCH v3 0/2] Provide UEFILib functions for protocol uninstallation.
Hi Mike, I build both DEBUG and RELEASE variant of the library and they both built a few KB less in size compared to what is in tip right now. Can you please help me with the optimization settings you have enabled so that I can try the same at my end? Also, if you want, we can look at the optimization part going forward and fix the issue first by pushing in PATCH v2 1/4 and PATCH v2 2/4 which just adds uninstallation APIs keeping the same code structure as for install. Thanks Ashish -Original Message- From: Kinney, Michael D Sent: Monday, January 7, 2019 3:23 PM To: Ashish Singhal ; edk2-devel@lists.01.org; Gao, Liming ; Fu, Siyuan ; Wu, Jiaxin ; Kinney, Michael D Subject: RE: [PATCH v3 0/2] Provide UEFILib functions for protocol uninstallation. Hi Ashish, My main concern with this patch is that the generated code for optimized RELEASE builds is not as small. >From a source maintenance perspective, the patch you have provided is easier >to maintain. However, the implementation of the APIs that install protocols >was done to make sure the optimizer produces the smallest number of >instructions to install the protocols. I would prefer the APIs that install protocols remain unchanged, and that only the new APIs to uninstall the protocols be added. The same approach could be taken in the implementation to produce the exact right form of the uninstall action that is guaranteed to succeed if the uninstall API matches the API that was used to install. Thanks, Mike > -Original Message- > From: Ashish Singhal [mailto:ashishsin...@nvidia.com] > Sent: Monday, January 7, 2019 6:02 AM > To: edk2-devel@lists.01.org; Kinney, Michael D > ; Gao, Liming ; Fu, > Siyuan ; Wu, Jiaxin > Subject: RE: [PATCH v3 0/2] Provide UEFILib functions for protocol > uninstallation. > > + Maintainers > > -Original Message- > From: Ashish Singhal > Sent: Sunday, January 6, 2019 9:38 PM > To: edk2-devel@lists.01.org > Cc: Ashish Singhal > Subject: [PATCH v3 0/2] Provide UEFILib functions for protocol > uninstallation. > > An issue was seen in IScsiDxe in NetworkPkg where driver cleanup after > initialization failure was not done right. Bug 1428 was filed in this > regard. > As per discussions with Mike, it was also discussed that having > UEFILib provide protocol uninstallation abstraction would help to > avoid these issues in the future. Bug 1429 was found to track this. > The first 2 patches take care of this. > > Patch number 1 also simplifies the UEFILib protocol installation and > uninstallation abstraction by adding a helper function doing > operations instead of every public function. > > Ashish Singhal (2): > MdePkg/UefiLib: Abstract driver model protocol uninstallation > NetworkPkg/IScsiDxe: Use UEFILib APIs to uninstall protocols. > > MdePkg/Include/Library/UefiLib.h | 103 +++ > MdePkg/Library/UefiLib/UefiDriverModel.c | 1186 > -- > NetworkPkg/IScsiDxe/IScsiDriver.c| 31 +- > 3 files changed, 435 insertions(+), 885 deletions(-) > > -- > 2.7.4 > > --- > > 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
Re: [edk2] [PATCH v3 0/2] Provide UEFILib functions for protocol uninstallation.
+ Maintainers -Original Message- From: Ashish Singhal Sent: Sunday, January 6, 2019 9:38 PM To: edk2-devel@lists.01.org Cc: Ashish Singhal Subject: [PATCH v3 0/2] Provide UEFILib functions for protocol uninstallation. An issue was seen in IScsiDxe in NetworkPkg where driver cleanup after initialization failure was not done right. Bug 1428 was filed in this regard. As per discussions with Mike, it was also discussed that having UEFILib provide protocol uninstallation abstraction would help to avoid these issues in the future. Bug 1429 was found to track this. The first 2 patches take care of this. Patch number 1 also simplifies the UEFILib protocol installation and uninstallation abstraction by adding a helper function doing operations instead of every public function. Ashish Singhal (2): MdePkg/UefiLib: Abstract driver model protocol uninstallation NetworkPkg/IScsiDxe: Use UEFILib APIs to uninstall protocols. MdePkg/Include/Library/UefiLib.h | 103 +++ MdePkg/Library/UefiLib/UefiDriverModel.c | 1186 -- NetworkPkg/IScsiDxe/IScsiDriver.c| 31 +- 3 files changed, 435 insertions(+), 885 deletions(-) -- 2.7.4 --- 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
[edk2] [PATCH v3 2/2] NetworkPkg/IScsiDxe: Use UEFILib APIs to uninstall protocols.
During cleanup in case of initialization failure, some driver bindings are not installed. Using abstractions in UEFILib takes care of it. REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1428 Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Ashish Singhal --- NetworkPkg/IScsiDxe/IScsiDriver.c | 31 +++ 1 file changed, 11 insertions(+), 20 deletions(-) diff --git a/NetworkPkg/IScsiDxe/IScsiDriver.c b/NetworkPkg/IScsiDxe/IScsiDriver.c index 91176e6..8747de7 100644 --- a/NetworkPkg/IScsiDxe/IScsiDriver.c +++ b/NetworkPkg/IScsiDxe/IScsiDriver.c @@ -1,6 +1,7 @@ /** @file The entry point of IScsi driver. +Copyright (c) 2019, NVIDIA Corporation. All rights reserved. Copyright (c) 2004 - 2018, Intel Corporation. All rights reserved. (C) Copyright 2017 Hewlett Packard Enterprise Development LP @@ -1861,28 +1862,18 @@ Error3: ); Error2: - gBS->UninstallMultipleProtocolInterfaces ( - gIScsiIp6DriverBinding.DriverBindingHandle, - , - , - , - , - , - , - NULL - ); + EfiLibUninstallDriverBindingComponentName2 ( +, +, + +); Error1: - gBS->UninstallMultipleProtocolInterfaces ( - ImageHandle, - , - , - , - , - , - , - NULL - ); + EfiLibUninstallDriverBindingComponentName2 ( +, +, + +); return Status; } -- 2.7.4 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH v3 0/2] Provide UEFILib functions for protocol uninstallation.
An issue was seen in IScsiDxe in NetworkPkg where driver cleanup after initialization failure was not done right. Bug 1428 was filed in this regard. As per discussions with Mike, it was also discussed that having UEFILib provide protocol uninstallation abstraction would help to avoid these issues in the future. Bug 1429 was found to track this. The first 2 patches take care of this. Patch number 1 also simplifies the UEFILib protocol installation and uninstallation abstraction by adding a helper function doing operations instead of every public function. Ashish Singhal (2): MdePkg/UefiLib: Abstract driver model protocol uninstallation NetworkPkg/IScsiDxe: Use UEFILib APIs to uninstall protocols. MdePkg/Include/Library/UefiLib.h | 103 +++ MdePkg/Library/UefiLib/UefiDriverModel.c | 1186 -- NetworkPkg/IScsiDxe/IScsiDriver.c| 31 +- 3 files changed, 435 insertions(+), 885 deletions(-) -- 2.7.4 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH v3 1/2] MdePkg/UefiLib: Abstract driver model protocol uninstallation
Provided functions in UEFILib that abstract driver model protocol uninstallation. This helps drivers to install and uninstall protocols using a library to keep things seemless. Also, add a helper function to operate upon protocol installation and uninstallation instead of every function doing it by itself. REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1429 Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Ashish Singhal --- MdePkg/Include/Library/UefiLib.h | 103 +++ MdePkg/Library/UefiLib/UefiDriverModel.c | 1186 -- 2 files changed, 424 insertions(+), 865 deletions(-) diff --git a/MdePkg/Include/Library/UefiLib.h b/MdePkg/Include/Library/UefiLib.h index 468bffc..08222d4 100644 --- a/MdePkg/Include/Library/UefiLib.h +++ b/MdePkg/Include/Library/UefiLib.h @@ -12,6 +12,7 @@ of size reduction when compiler optimization is disabled. If MDEPKG_NDEBUG is defined, then debug and assert related macros wrapped by it are the NULL implementations. +Copyright (c) 2019, NVIDIA Corporation. All rights reserved. Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved. This program and the accompanying materials are licensed and made available under the terms and conditions of the BSD License that accompanies this distribution. @@ -1283,6 +1284,7 @@ AsciiPrintXY ( ... ); + /** Installs and completes the initialization of a Driver Binding Protocol instance. @@ -1316,6 +1318,25 @@ EfiLibInstallDriverBinding ( /** + Uninstalls a Driver Binding Protocol instance. + + If DriverBinding is NULL, then ASSERT(). + If DriverBinding can not be uninstalled, then ASSERT(). + + @param DriverBindingA Driver Binding Protocol instance that this driver produced. + + @retval EFI_SUCCESS The protocol uninstallation successfully completed. + @retval OthersStatus from gBS->UninstallMultipleProtocolInterfaces(). + +**/ +EFI_STATUS +EFIAPI +EfiLibUninstallDriverBinding ( + IN EFI_DRIVER_BINDING_PROTOCOL *DriverBinding + ); + + +/** Installs and completes the initialization of a Driver Binding Protocol instance and optionally installs the Component Name, Driver Configuration and Driver Diagnostics Protocols. @@ -1354,6 +1375,31 @@ EfiLibInstallAllDriverProtocols ( ); +/** + Uninstalls a Driver Binding Protocol instance and optionally uninstalls the + Component Name, Driver Configuration and Driver Diagnostics Protocols. + + If DriverBinding is NULL, then ASSERT(). + If the uninstallation fails, then ASSERT(). + + @param DriverBindingA Driver Binding Protocol instance that this driver produced. + @param ComponentNameA Component Name Protocol instance that this driver produced. + @param DriverConfiguration A Driver Configuration Protocol instance that this driver produced. + @param DriverDiagnosticsA Driver Diagnostics Protocol instance that this driver produced. + + @retval EFI_SUCCESS The protocol uninstallation successfully completed. + @retval OthersStatus from gBS->UninstallMultipleProtocolInterfaces(). + +**/ +EFI_STATUS +EFIAPI +EfiLibUninstallAllDriverProtocols ( + IN EFI_DRIVER_BINDING_PROTOCOL *DriverBinding, + IN CONST EFI_COMPONENT_NAME_PROTOCOL*ComponentName, OPTIONAL + IN CONST EFI_DRIVER_CONFIGURATION_PROTOCOL *DriverConfiguration, OPTIONAL + IN CONST EFI_DRIVER_DIAGNOSTICS_PROTOCOL*DriverDiagnosticsOPTIONAL + ); + /** Installs Driver Binding Protocol with optional Component Name and Component Name 2 Protocols. @@ -1391,6 +1437,29 @@ EfiLibInstallDriverBindingComponentName2 ( /** + Uninstalls Driver Binding Protocol with optional Component Name and Component Name 2 Protocols. + + If DriverBinding is NULL, then ASSERT(). + If the uninstallation fails, then ASSERT(). + + @param DriverBindingA Driver Binding Protocol instance that this driver produced. + @param ComponentNameA Component Name Protocol instance that this driver produced. + @param ComponentName2 A Component Name 2 Protocol instance that this driver produced. + + @retval EFI_SUCCESS The protocol installation successfully completed. + @retval OthersStatus from gBS->UninstallMultipleProtocolInterfaces(). + +**/ +EFI_STATUS +EFIAPI +EfiLibUninstallDriverBindingComponentName2 ( + IN EFI_DRIVER_BINDING_PROTOCOL *DriverBinding, + IN CONST EFI_COMPONENT_NAME_PROTOCOL*ComponentName, OPTIONAL + IN CONST EFI_COMPONENT_NAME2_PROTOCOL *ComponentName2 OPTIONAL + ); + + +/** Installs Driver Binding Protocol with optional Component Name, Component Name 2, Driver Configuration, Driver Configuration 2, Driver Diagnostics, and Driver Diagnostics 2 Protocols. @@ -1434,6 +1503,40 @@ EfiLibInstallAllDriverProtocols2 ( IN CONST EFI_DRIVER_DIAGNOSTICS2_PROTOCOL *DriverDia
Re: [edk2] [PATCH v2 3/4] MdePkg/UefiLib: Simplify protocol un/installation abstraction
Hello Liming, I am not touching APIs for Install and am OK keeping Uninstall API same as what I had in patch 1/4. I thought it would be easier for the developer to keep the interface similar to install but I do not have a strong preference either way. If you are OK with the Uninstall API as in patch 1/4, I am OK submitting a new patch where I can squash 1/4 and 3/4 together into a single commit and keep API as in 1/4. Thanks Ashish -Original Message- From: Gao, Liming Sent: Sunday, January 6, 2019 5:33 PM To: Ashish Singhal ; edk2-devel@lists.01.org Subject: RE: [edk2] [PATCH v2 3/4] MdePkg/UefiLib: Simplify protocol un/installation abstraction Ashish: UefiLib implementation simplification doesn't require to change library APIs. UninstallApi() interfaces are not required to be updated. Below Install API can still be kept. I don't think we need to keep the same interfaces for Install and Uninstall APIs. EfiLibUninstallAllDriverProtocols2 ( IN EFI_DRIVER_BINDING_PROTOCOL *DriverBinding, IN CONST EFI_COMPONENT_NAME_PROTOCOL*ComponentName,OPTIONAL IN CONST EFI_COMPONENT_NAME2_PROTOCOL *ComponentName2, OPTIONAL IN CONST EFI_DRIVER_CONFIGURATION_PROTOCOL *DriverConfiguration, OPTIONAL IN CONST EFI_DRIVER_CONFIGURATION2_PROTOCOL *DriverConfiguration2, OPTIONAL IN CONST EFI_DRIVER_DIAGNOSTICS_PROTOCOL*DriverDiagnostics,OPTIONAL IN CONST EFI_DRIVER_DIAGNOSTICS2_PROTOCOL *DriverDiagnostics2OPTIONAL ); Thanks Liming >-Original Message- >From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of >Ashish Singhal >Sent: Saturday, January 05, 2019 7:07 AM >To: edk2-devel@lists.01.org >Cc: Ashish Singhal >Subject: [edk2] [PATCH v2 3/4] MdePkg/UefiLib: Simplify protocol >un/installation abstraction > >Add a helper function to operate upon protocol installation and >uninstallation instead of every function doing it by itself. > >Contributed-under: TianoCore Contribution Agreement 1.1 >Signed-off-by: Ashish Singhal >--- > MdePkg/Include/Library/UefiLib.h | 26 +- > MdePkg/Library/UefiLib/UefiDriverModel.c | 1980 -- > 2 files changed, 270 insertions(+), 1736 deletions(-) > >diff --git a/MdePkg/Include/Library/UefiLib.h >b/MdePkg/Include/Library/UefiLib.h >index 08222d4..fbc9739 100644 >--- a/MdePkg/Include/Library/UefiLib.h >+++ b/MdePkg/Include/Library/UefiLib.h >@@ -1323,7 +1323,10 @@ EfiLibInstallDriverBinding ( > If DriverBinding is NULL, then ASSERT(). > If DriverBinding can not be uninstalled, then ASSERT(). > >+ @param ImageHandle The image handle of the driver. >+ @param SystemTable The EFI System Table that was passed to the >driver's entry point. > @param DriverBindingA Driver Binding Protocol instance that this > driver >produced. >+ @param DriverBindingHandle The handle that DriverBinding is to be >installed onto. > > @retval EFI_SUCCESS The protocol uninstallation successfully >completed. > @retval OthersStatus from gBS- >>UninstallMultipleProtocolInterfaces(). >@@ -1332,7 +1335,10 @@ EfiLibInstallDriverBinding ( > EFI_STATUS > EFIAPI > EfiLibUninstallDriverBinding ( >- IN EFI_DRIVER_BINDING_PROTOCOL *DriverBinding >+ IN CONST EFI_HANDLE ImageHandle, >+ IN CONST EFI_SYSTEM_TABLE *SystemTable, >+ IN EFI_DRIVER_BINDING_PROTOCOL *DriverBinding, >+ IN EFI_HANDLE DriverBindingHandle > ); > > >@@ -1382,7 +1388,10 @@ EfiLibInstallAllDriverProtocols ( > If DriverBinding is NULL, then ASSERT(). > If the uninstallation fails, then ASSERT(). > >+ @param ImageHandle The image handle of the driver. >+ @param SystemTable The EFI System Table that was passed to the >driver's entry point. > @param DriverBindingA Driver Binding Protocol instance that this > driver >produced. >+ @param DriverBindingHandle The handle that DriverBinding is to be >installed onto. > @param ComponentNameA Component Name Protocol instance that >this driver produced. > @param DriverConfiguration A Driver Configuration Protocol instance that >this driver produced. > @param DriverDiagnosticsA Driver Diagnostics Protocol instance that > this >driver produced. >@@ -1394,7 +1403,10 @@ EfiLibInstallAllDriverProtocols ( > EFI_STATUS > EFIAPI > EfiLibUninstallAllDriverProtocols ( >+ IN CONST EFI_HANDLE ImageHandle, >+ IN CONST EFI_SYSTEM_TABLE *SystemTable, > IN EFI_DRIVER_BINDING_PROTOCOL *DriverBinding, >+ IN EFI_HANDLE DriverBindingHandle, > IN CONST EFI_COMPONENT_NAME_PROTOCOL*
[edk2] [PATCH v2 4/4] NetworkPkg/IScsiDxe: Update UEFILib Usage
Update interfaces as exposed by UEFILib for protocol installation and uninstallation abstraction. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Ashish Singhal --- NetworkPkg/IScsiDxe/IScsiDriver.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/NetworkPkg/IScsiDxe/IScsiDriver.c b/NetworkPkg/IScsiDxe/IScsiDriver.c index 8747de7..fa0ea00 100644 --- a/NetworkPkg/IScsiDxe/IScsiDriver.c +++ b/NetworkPkg/IScsiDxe/IScsiDriver.c @@ -1863,14 +1863,20 @@ Error3: Error2: EfiLibUninstallDriverBindingComponentName2 ( +ImageHandle, +SystemTable, , +NULL, , ); Error1: EfiLibUninstallDriverBindingComponentName2 ( +ImageHandle, +SystemTable, , +NULL, , ); -- 2.7.4 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH v2 3/4] MdePkg/UefiLib: Simplify protocol un/installation abstraction
Add a helper function to operate upon protocol installation and uninstallation instead of every function doing it by itself. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Ashish Singhal --- MdePkg/Include/Library/UefiLib.h | 26 +- MdePkg/Library/UefiLib/UefiDriverModel.c | 1980 -- 2 files changed, 270 insertions(+), 1736 deletions(-) diff --git a/MdePkg/Include/Library/UefiLib.h b/MdePkg/Include/Library/UefiLib.h index 08222d4..fbc9739 100644 --- a/MdePkg/Include/Library/UefiLib.h +++ b/MdePkg/Include/Library/UefiLib.h @@ -1323,7 +1323,10 @@ EfiLibInstallDriverBinding ( If DriverBinding is NULL, then ASSERT(). If DriverBinding can not be uninstalled, then ASSERT(). + @param ImageHandle The image handle of the driver. + @param SystemTable The EFI System Table that was passed to the driver's entry point. @param DriverBindingA Driver Binding Protocol instance that this driver produced. + @param DriverBindingHandle The handle that DriverBinding is to be installed onto. @retval EFI_SUCCESS The protocol uninstallation successfully completed. @retval OthersStatus from gBS->UninstallMultipleProtocolInterfaces(). @@ -1332,7 +1335,10 @@ EfiLibInstallDriverBinding ( EFI_STATUS EFIAPI EfiLibUninstallDriverBinding ( - IN EFI_DRIVER_BINDING_PROTOCOL *DriverBinding + IN CONST EFI_HANDLE ImageHandle, + IN CONST EFI_SYSTEM_TABLE *SystemTable, + IN EFI_DRIVER_BINDING_PROTOCOL *DriverBinding, + IN EFI_HANDLE DriverBindingHandle ); @@ -1382,7 +1388,10 @@ EfiLibInstallAllDriverProtocols ( If DriverBinding is NULL, then ASSERT(). If the uninstallation fails, then ASSERT(). + @param ImageHandle The image handle of the driver. + @param SystemTable The EFI System Table that was passed to the driver's entry point. @param DriverBindingA Driver Binding Protocol instance that this driver produced. + @param DriverBindingHandle The handle that DriverBinding is to be installed onto. @param ComponentNameA Component Name Protocol instance that this driver produced. @param DriverConfiguration A Driver Configuration Protocol instance that this driver produced. @param DriverDiagnosticsA Driver Diagnostics Protocol instance that this driver produced. @@ -1394,7 +1403,10 @@ EfiLibInstallAllDriverProtocols ( EFI_STATUS EFIAPI EfiLibUninstallAllDriverProtocols ( + IN CONST EFI_HANDLE ImageHandle, + IN CONST EFI_SYSTEM_TABLE *SystemTable, IN EFI_DRIVER_BINDING_PROTOCOL *DriverBinding, + IN EFI_HANDLE DriverBindingHandle, IN CONST EFI_COMPONENT_NAME_PROTOCOL*ComponentName, OPTIONAL IN CONST EFI_DRIVER_CONFIGURATION_PROTOCOL *DriverConfiguration, OPTIONAL IN CONST EFI_DRIVER_DIAGNOSTICS_PROTOCOL*DriverDiagnosticsOPTIONAL @@ -1442,7 +1454,10 @@ EfiLibInstallDriverBindingComponentName2 ( If DriverBinding is NULL, then ASSERT(). If the uninstallation fails, then ASSERT(). + @param ImageHandle The image handle of the driver. + @param SystemTable The EFI System Table that was passed to the driver's entry point. @param DriverBindingA Driver Binding Protocol instance that this driver produced. + @param DriverBindingHandle The handle that DriverBinding is to be installed onto. @param ComponentNameA Component Name Protocol instance that this driver produced. @param ComponentName2 A Component Name 2 Protocol instance that this driver produced. @@ -1453,7 +1468,10 @@ EfiLibInstallDriverBindingComponentName2 ( EFI_STATUS EFIAPI EfiLibUninstallDriverBindingComponentName2 ( + IN CONST EFI_HANDLE ImageHandle, + IN CONST EFI_SYSTEM_TABLE *SystemTable, IN EFI_DRIVER_BINDING_PROTOCOL *DriverBinding, + IN EFI_HANDLE DriverBindingHandle, IN CONST EFI_COMPONENT_NAME_PROTOCOL*ComponentName, OPTIONAL IN CONST EFI_COMPONENT_NAME2_PROTOCOL *ComponentName2 OPTIONAL ); @@ -1512,7 +1530,10 @@ EfiLibInstallAllDriverProtocols2 ( If the installation fails, then ASSERT(). + @param ImageHandle The image handle of the driver. + @param SystemTable The EFI System Table that was passed to the driver's entry point. @param DriverBinding A Driver Binding Protocol instance that this driver produced. + @param DriverBindingHandle The handle that DriverBinding is to be installed onto. @param ComponentName A Component Name Protocol instance that this driver produced. @param ComponentName2A Component Name 2 Protocol instance that this driver produced. @param DriverConfiguration A Driver Configuration Proto
[edk2] [PATCH v2 2/4] NetworkPkg/IScsiDxe: Use UEFILib APIs to uninstall protocols.
During cleanup in case of initialization failure, some driver bindings are not installed. Using abstractions in UEFILib takes care of it. REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1428 Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Ashish Singhal --- NetworkPkg/IScsiDxe/IScsiDriver.c | 31 +++ 1 file changed, 11 insertions(+), 20 deletions(-) diff --git a/NetworkPkg/IScsiDxe/IScsiDriver.c b/NetworkPkg/IScsiDxe/IScsiDriver.c index 91176e6..8747de7 100644 --- a/NetworkPkg/IScsiDxe/IScsiDriver.c +++ b/NetworkPkg/IScsiDxe/IScsiDriver.c @@ -1,6 +1,7 @@ /** @file The entry point of IScsi driver. +Copyright (c) 2019, NVIDIA Corporation. All rights reserved. Copyright (c) 2004 - 2018, Intel Corporation. All rights reserved. (C) Copyright 2017 Hewlett Packard Enterprise Development LP @@ -1861,28 +1862,18 @@ Error3: ); Error2: - gBS->UninstallMultipleProtocolInterfaces ( - gIScsiIp6DriverBinding.DriverBindingHandle, - , - , - , - , - , - , - NULL - ); + EfiLibUninstallDriverBindingComponentName2 ( +, +, + +); Error1: - gBS->UninstallMultipleProtocolInterfaces ( - ImageHandle, - , - , - , - , - , - , - NULL - ); + EfiLibUninstallDriverBindingComponentName2 ( +, +, + +); return Status; } -- 2.7.4 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH v2 0/4] Provide UEFILib functions for protocol uninstallation.
An issue was seen in IScsiDxe in NetworkPkg where driver cleanup after initialization failure was not done right. Bug 1428 was filed in this regard. As per discussions with Mike, it was also discussed that having UEFILib provide protocol uninstallation abstraction would help to avoid these issues in the future. Bug 1429 was found to track this. The first 2 patches take care of this. Patch number 3 simplifies the UEFILib protocol installation and uninstallation abstraction by adding a helper function doing operations instead of every public function. Patch set 4 uses the updated uninstallation interfaces as a result of patch 3. Ashish Singhal (4): MdePkg/UefiLib: Abstract driver model protocol uninstallation NetworkPkg/IScsiDxe: Use UEFILib APIs to uninstall protocols. MdePkg/UefiLib: Simplify protocol un/installation abstraction NetworkPkg/IScsiDxe: Update UEFILib Usage MdePkg/Include/Library/UefiLib.h | 127 MdePkg/Library/UefiLib/UefiDriverModel.c | 1210 +- NetworkPkg/IScsiDxe/IScsiDriver.c| 37 +- 3 files changed, 489 insertions(+), 885 deletions(-) -- 2.7.4 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH v2 1/4] MdePkg/UefiLib: Abstract driver model protocol uninstallation
Provided functions in UEFILib that abstract driver model protocol uninstallation. This helps drivers to install and uninstall protocols using a library to keep things seemless. REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1429 Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Ashish Singhal --- MdePkg/Include/Library/UefiLib.h | 103 MdePkg/Library/UefiLib/UefiDriverModel.c | 972 ++- 2 files changed, 1074 insertions(+), 1 deletion(-) diff --git a/MdePkg/Include/Library/UefiLib.h b/MdePkg/Include/Library/UefiLib.h index 468bffc..08222d4 100644 --- a/MdePkg/Include/Library/UefiLib.h +++ b/MdePkg/Include/Library/UefiLib.h @@ -12,6 +12,7 @@ of size reduction when compiler optimization is disabled. If MDEPKG_NDEBUG is defined, then debug and assert related macros wrapped by it are the NULL implementations. +Copyright (c) 2019, NVIDIA Corporation. All rights reserved. Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved. This program and the accompanying materials are licensed and made available under the terms and conditions of the BSD License that accompanies this distribution. @@ -1283,6 +1284,7 @@ AsciiPrintXY ( ... ); + /** Installs and completes the initialization of a Driver Binding Protocol instance. @@ -1316,6 +1318,25 @@ EfiLibInstallDriverBinding ( /** + Uninstalls a Driver Binding Protocol instance. + + If DriverBinding is NULL, then ASSERT(). + If DriverBinding can not be uninstalled, then ASSERT(). + + @param DriverBindingA Driver Binding Protocol instance that this driver produced. + + @retval EFI_SUCCESS The protocol uninstallation successfully completed. + @retval OthersStatus from gBS->UninstallMultipleProtocolInterfaces(). + +**/ +EFI_STATUS +EFIAPI +EfiLibUninstallDriverBinding ( + IN EFI_DRIVER_BINDING_PROTOCOL *DriverBinding + ); + + +/** Installs and completes the initialization of a Driver Binding Protocol instance and optionally installs the Component Name, Driver Configuration and Driver Diagnostics Protocols. @@ -1354,6 +1375,31 @@ EfiLibInstallAllDriverProtocols ( ); +/** + Uninstalls a Driver Binding Protocol instance and optionally uninstalls the + Component Name, Driver Configuration and Driver Diagnostics Protocols. + + If DriverBinding is NULL, then ASSERT(). + If the uninstallation fails, then ASSERT(). + + @param DriverBindingA Driver Binding Protocol instance that this driver produced. + @param ComponentNameA Component Name Protocol instance that this driver produced. + @param DriverConfiguration A Driver Configuration Protocol instance that this driver produced. + @param DriverDiagnosticsA Driver Diagnostics Protocol instance that this driver produced. + + @retval EFI_SUCCESS The protocol uninstallation successfully completed. + @retval OthersStatus from gBS->UninstallMultipleProtocolInterfaces(). + +**/ +EFI_STATUS +EFIAPI +EfiLibUninstallAllDriverProtocols ( + IN EFI_DRIVER_BINDING_PROTOCOL *DriverBinding, + IN CONST EFI_COMPONENT_NAME_PROTOCOL*ComponentName, OPTIONAL + IN CONST EFI_DRIVER_CONFIGURATION_PROTOCOL *DriverConfiguration, OPTIONAL + IN CONST EFI_DRIVER_DIAGNOSTICS_PROTOCOL*DriverDiagnosticsOPTIONAL + ); + /** Installs Driver Binding Protocol with optional Component Name and Component Name 2 Protocols. @@ -1391,6 +1437,29 @@ EfiLibInstallDriverBindingComponentName2 ( /** + Uninstalls Driver Binding Protocol with optional Component Name and Component Name 2 Protocols. + + If DriverBinding is NULL, then ASSERT(). + If the uninstallation fails, then ASSERT(). + + @param DriverBindingA Driver Binding Protocol instance that this driver produced. + @param ComponentNameA Component Name Protocol instance that this driver produced. + @param ComponentName2 A Component Name 2 Protocol instance that this driver produced. + + @retval EFI_SUCCESS The protocol installation successfully completed. + @retval OthersStatus from gBS->UninstallMultipleProtocolInterfaces(). + +**/ +EFI_STATUS +EFIAPI +EfiLibUninstallDriverBindingComponentName2 ( + IN EFI_DRIVER_BINDING_PROTOCOL *DriverBinding, + IN CONST EFI_COMPONENT_NAME_PROTOCOL*ComponentName, OPTIONAL + IN CONST EFI_COMPONENT_NAME2_PROTOCOL *ComponentName2 OPTIONAL + ); + + +/** Installs Driver Binding Protocol with optional Component Name, Component Name 2, Driver Configuration, Driver Configuration 2, Driver Diagnostics, and Driver Diagnostics 2 Protocols. @@ -1434,6 +1503,40 @@ EfiLibInstallAllDriverProtocols2 ( IN CONST EFI_DRIVER_DIAGNOSTICS2_PROTOCOL *DriverDiagnostics2OPTIONAL ); + +/** + Uninstalls Driver Binding Protocol with optional Component Name, Component Name
[edk2] [PATCH 3/4] MdePkg/UefiLib: Simplify protocol un/installation abstraction
Add a helper function to operate upon protocol installation and uninstallation instead of every function doing it by itself. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Ashish Singhal --- MdePkg/Include/Library/UefiLib.h | 26 +- MdePkg/Library/UefiLib/UefiDriverModel.c | 1975 -- 2 files changed, 265 insertions(+), 1736 deletions(-) diff --git a/MdePkg/Include/Library/UefiLib.h b/MdePkg/Include/Library/UefiLib.h index 08222d4..fbc9739 100644 --- a/MdePkg/Include/Library/UefiLib.h +++ b/MdePkg/Include/Library/UefiLib.h @@ -1323,7 +1323,10 @@ EfiLibInstallDriverBinding ( If DriverBinding is NULL, then ASSERT(). If DriverBinding can not be uninstalled, then ASSERT(). + @param ImageHandle The image handle of the driver. + @param SystemTable The EFI System Table that was passed to the driver's entry point. @param DriverBindingA Driver Binding Protocol instance that this driver produced. + @param DriverBindingHandle The handle that DriverBinding is to be installed onto. @retval EFI_SUCCESS The protocol uninstallation successfully completed. @retval OthersStatus from gBS->UninstallMultipleProtocolInterfaces(). @@ -1332,7 +1335,10 @@ EfiLibInstallDriverBinding ( EFI_STATUS EFIAPI EfiLibUninstallDriverBinding ( - IN EFI_DRIVER_BINDING_PROTOCOL *DriverBinding + IN CONST EFI_HANDLE ImageHandle, + IN CONST EFI_SYSTEM_TABLE *SystemTable, + IN EFI_DRIVER_BINDING_PROTOCOL *DriverBinding, + IN EFI_HANDLE DriverBindingHandle ); @@ -1382,7 +1388,10 @@ EfiLibInstallAllDriverProtocols ( If DriverBinding is NULL, then ASSERT(). If the uninstallation fails, then ASSERT(). + @param ImageHandle The image handle of the driver. + @param SystemTable The EFI System Table that was passed to the driver's entry point. @param DriverBindingA Driver Binding Protocol instance that this driver produced. + @param DriverBindingHandle The handle that DriverBinding is to be installed onto. @param ComponentNameA Component Name Protocol instance that this driver produced. @param DriverConfiguration A Driver Configuration Protocol instance that this driver produced. @param DriverDiagnosticsA Driver Diagnostics Protocol instance that this driver produced. @@ -1394,7 +1403,10 @@ EfiLibInstallAllDriverProtocols ( EFI_STATUS EFIAPI EfiLibUninstallAllDriverProtocols ( + IN CONST EFI_HANDLE ImageHandle, + IN CONST EFI_SYSTEM_TABLE *SystemTable, IN EFI_DRIVER_BINDING_PROTOCOL *DriverBinding, + IN EFI_HANDLE DriverBindingHandle, IN CONST EFI_COMPONENT_NAME_PROTOCOL*ComponentName, OPTIONAL IN CONST EFI_DRIVER_CONFIGURATION_PROTOCOL *DriverConfiguration, OPTIONAL IN CONST EFI_DRIVER_DIAGNOSTICS_PROTOCOL*DriverDiagnosticsOPTIONAL @@ -1442,7 +1454,10 @@ EfiLibInstallDriverBindingComponentName2 ( If DriverBinding is NULL, then ASSERT(). If the uninstallation fails, then ASSERT(). + @param ImageHandle The image handle of the driver. + @param SystemTable The EFI System Table that was passed to the driver's entry point. @param DriverBindingA Driver Binding Protocol instance that this driver produced. + @param DriverBindingHandle The handle that DriverBinding is to be installed onto. @param ComponentNameA Component Name Protocol instance that this driver produced. @param ComponentName2 A Component Name 2 Protocol instance that this driver produced. @@ -1453,7 +1468,10 @@ EfiLibInstallDriverBindingComponentName2 ( EFI_STATUS EFIAPI EfiLibUninstallDriverBindingComponentName2 ( + IN CONST EFI_HANDLE ImageHandle, + IN CONST EFI_SYSTEM_TABLE *SystemTable, IN EFI_DRIVER_BINDING_PROTOCOL *DriverBinding, + IN EFI_HANDLE DriverBindingHandle, IN CONST EFI_COMPONENT_NAME_PROTOCOL*ComponentName, OPTIONAL IN CONST EFI_COMPONENT_NAME2_PROTOCOL *ComponentName2 OPTIONAL ); @@ -1512,7 +1530,10 @@ EfiLibInstallAllDriverProtocols2 ( If the installation fails, then ASSERT(). + @param ImageHandle The image handle of the driver. + @param SystemTable The EFI System Table that was passed to the driver's entry point. @param DriverBinding A Driver Binding Protocol instance that this driver produced. + @param DriverBindingHandle The handle that DriverBinding is to be installed onto. @param ComponentName A Component Name Protocol instance that this driver produced. @param ComponentName2A Component Name 2 Protocol instance that this driver produced. @param DriverConfiguration A Driver Configuration Proto
Re: [edk2] Uninstalling Invalid Protocol Interfaces
Mike, I have addressed the issue along with some optimizations and have submitted patches for review. Thanks Ashish From: Ashish Singhal Sent: Friday, January 4, 2019 10:33 AM To: 'Kinney, Michael D' ; edk2-devel@lists.01.org Cc: Gao, Liming ; Fu, Siyuan ; Wu, Jiaxin Subject: RE: Uninstalling Invalid Protocol Interfaces Mike, I have filed https://bugzilla.tianocore.org/show_bug.cgi?id=1428 and https://bugzilla.tianocore.org/show_bug.cgi?id=1429 to address this. I have assigned these to me as I already have fix for these. Also, how do we ensure all components which may have this issue (may not have exposed it yet) also absorb the change? Thanks Ashish From: Kinney, Michael D mailto:michael.d.kin...@intel.com>> Sent: Friday, January 4, 2019 10:15 AM To: Ashish Singhal mailto:ashishsin...@nvidia.com>>; edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>; Kinney, Michael D mailto:michael.d.kin...@intel.com>> Cc: Gao, Liming mailto:liming@intel.com>>; Fu, Siyuan mailto:siyuan...@intel.com>>; Wu, Jiaxin mailto:jiaxin...@intel.com>> Subject: RE: Uninstalling Invalid Protocol Interfaces Ashish, Thanks for the pointer. I agree there is an issue here. Please enter a Bugzilla against the IScsiDxe module for this issue so we can fix this failure. You are also welcome to enter a Bugzilla for a feature request to add UefiLib APIs that can be used to safely uninstall all the driver model related protocol that can be used in error paths in the entry point and in unload handlers. Thanks, Mike From: Ashish Singhal [mailto:ashishsin...@nvidia.com] Sent: Thursday, January 3, 2019 4:16 PM To: Kinney, Michael D mailto:michael.d.kin...@intel.com>>; edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org> Cc: Gao, Liming mailto:liming@intel.com>>; Fu, Siyuan mailto:siyuan...@intel.com>>; Wu, Jiaxin mailto:jiaxin...@intel.com>> Subject: RE: Uninstalling Invalid Protocol Interfaces Hi Mike, Call to UninstallMultipleProtocolInterfaces at https://github.com/tianocore/edk2/blob/master/NetworkPkg/IScsiDxe/IScsiDriver.c#L1864 fails because the component name interface may not be installed depending on the state of PCD. Thanks Ashish From: Kinney, Michael D mailto:michael.d.kin...@intel.com>> Sent: Thursday, January 3, 2019 5:09 PM To: Ashish Singhal mailto:ashishsin...@nvidia.com>>; edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>; Kinney, Michael D mailto:michael.d.kin...@intel.com>> Cc: Gao, Liming mailto:liming@intel.com>>; Fu, Siyuan mailto:siyuan...@intel.com>>; Wu, Jiaxin mailto:jiaxin...@intel.com>> Subject: RE: Uninstalling Invalid Protocol Interfaces Hi Ashish, Can you provide a pointer to the UninstallMultipleProtocolInterfaces() call that is causing this failure? I agree that the UefiLib could provide additional services to simplify the implementation of the unload handlers. Matching the UefiLib APIs that install the Uefi Driver Model protocol would be useful, so the driver entry point and the driver unload functions can use matching APIs. Mike From: Ashish Singhal [mailto:ashishsin...@nvidia.com] Sent: Thursday, January 3, 2019 3:39 PM To: edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org> Cc: Kinney, Michael D mailto:michael.d.kin...@intel.com>>; Gao, Liming mailto:liming@intel.com>>; Fu, Siyuan mailto:siyuan...@intel.com>>; Wu, Jiaxin mailto:jiaxin...@intel.com>> Subject: Uninstalling Invalid Protocol Interfaces Hello, As part of moving from MdeModulePkg implementation of IScsiDxe to the implementation in NetworkPkg, I started hitting exception in the driver loaded after IScsiDxe if IScsiDxe's installation fails for some reason. Upon debugging, I found out that calls to UninstallMultipleProtocolInterfaces as part of Error 2 as well Error 1 fail while trying to uninstall component name protocol-interface pair and return error code EFI_INVALID_PARAMETER. As per UninstallMultipleProtocolInterfaces's documentation, if uninstallation of any of the input protocol-interface pair fails, it will reinstall any just uninstalled protocol and return EFI_INVALID_PARAMETER which causes the cleanup of this driver corrupt leading to failure in next driver getting loaded. The reason of failure in UninstallMultipleProtocolInterfaces is because the driver uses EfiLibInstallDriverBindingComponentName2 to install interfaces which may not install component name interfaces depending on the value of PCDs PcdComponentNameDi sable and PcdComponentName2Disable. I have the following proposals to get around this issue. 1. Instead of calling UninstallMultipleProtocolInterfaces once and list all protocol-interface pairs, do it sequentially for every pair so that the once which was installed correctly, gets uninstalled instead of getting reinstalled because of a failure uninstalling another pa
[edk2] [PATCH 1/4] MdePkg/UefiLib: Abstract driver model protocol uninstallation
Provided functions in UEFILib that abstract driver model protocol uninstallation. This helps drivers to install and uninstall protocols using a library to keep things seemless. REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1429 Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Ashish Singhal --- MdePkg/Include/Library/UefiLib.h | 103 MdePkg/Library/UefiLib/UefiDriverModel.c | 972 ++- 2 files changed, 1074 insertions(+), 1 deletion(-) diff --git a/MdePkg/Include/Library/UefiLib.h b/MdePkg/Include/Library/UefiLib.h index 468bffc..08222d4 100644 --- a/MdePkg/Include/Library/UefiLib.h +++ b/MdePkg/Include/Library/UefiLib.h @@ -12,6 +12,7 @@ of size reduction when compiler optimization is disabled. If MDEPKG_NDEBUG is defined, then debug and assert related macros wrapped by it are the NULL implementations. +Copyright (c) 2019, NVIDIA Corporation. All rights reserved. Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved. This program and the accompanying materials are licensed and made available under the terms and conditions of the BSD License that accompanies this distribution. @@ -1283,6 +1284,7 @@ AsciiPrintXY ( ... ); + /** Installs and completes the initialization of a Driver Binding Protocol instance. @@ -1316,6 +1318,25 @@ EfiLibInstallDriverBinding ( /** + Uninstalls a Driver Binding Protocol instance. + + If DriverBinding is NULL, then ASSERT(). + If DriverBinding can not be uninstalled, then ASSERT(). + + @param DriverBindingA Driver Binding Protocol instance that this driver produced. + + @retval EFI_SUCCESS The protocol uninstallation successfully completed. + @retval OthersStatus from gBS->UninstallMultipleProtocolInterfaces(). + +**/ +EFI_STATUS +EFIAPI +EfiLibUninstallDriverBinding ( + IN EFI_DRIVER_BINDING_PROTOCOL *DriverBinding + ); + + +/** Installs and completes the initialization of a Driver Binding Protocol instance and optionally installs the Component Name, Driver Configuration and Driver Diagnostics Protocols. @@ -1354,6 +1375,31 @@ EfiLibInstallAllDriverProtocols ( ); +/** + Uninstalls a Driver Binding Protocol instance and optionally uninstalls the + Component Name, Driver Configuration and Driver Diagnostics Protocols. + + If DriverBinding is NULL, then ASSERT(). + If the uninstallation fails, then ASSERT(). + + @param DriverBindingA Driver Binding Protocol instance that this driver produced. + @param ComponentNameA Component Name Protocol instance that this driver produced. + @param DriverConfiguration A Driver Configuration Protocol instance that this driver produced. + @param DriverDiagnosticsA Driver Diagnostics Protocol instance that this driver produced. + + @retval EFI_SUCCESS The protocol uninstallation successfully completed. + @retval OthersStatus from gBS->UninstallMultipleProtocolInterfaces(). + +**/ +EFI_STATUS +EFIAPI +EfiLibUninstallAllDriverProtocols ( + IN EFI_DRIVER_BINDING_PROTOCOL *DriverBinding, + IN CONST EFI_COMPONENT_NAME_PROTOCOL*ComponentName, OPTIONAL + IN CONST EFI_DRIVER_CONFIGURATION_PROTOCOL *DriverConfiguration, OPTIONAL + IN CONST EFI_DRIVER_DIAGNOSTICS_PROTOCOL*DriverDiagnosticsOPTIONAL + ); + /** Installs Driver Binding Protocol with optional Component Name and Component Name 2 Protocols. @@ -1391,6 +1437,29 @@ EfiLibInstallDriverBindingComponentName2 ( /** + Uninstalls Driver Binding Protocol with optional Component Name and Component Name 2 Protocols. + + If DriverBinding is NULL, then ASSERT(). + If the uninstallation fails, then ASSERT(). + + @param DriverBindingA Driver Binding Protocol instance that this driver produced. + @param ComponentNameA Component Name Protocol instance that this driver produced. + @param ComponentName2 A Component Name 2 Protocol instance that this driver produced. + + @retval EFI_SUCCESS The protocol installation successfully completed. + @retval OthersStatus from gBS->UninstallMultipleProtocolInterfaces(). + +**/ +EFI_STATUS +EFIAPI +EfiLibUninstallDriverBindingComponentName2 ( + IN EFI_DRIVER_BINDING_PROTOCOL *DriverBinding, + IN CONST EFI_COMPONENT_NAME_PROTOCOL*ComponentName, OPTIONAL + IN CONST EFI_COMPONENT_NAME2_PROTOCOL *ComponentName2 OPTIONAL + ); + + +/** Installs Driver Binding Protocol with optional Component Name, Component Name 2, Driver Configuration, Driver Configuration 2, Driver Diagnostics, and Driver Diagnostics 2 Protocols. @@ -1434,6 +1503,40 @@ EfiLibInstallAllDriverProtocols2 ( IN CONST EFI_DRIVER_DIAGNOSTICS2_PROTOCOL *DriverDiagnostics2OPTIONAL ); + +/** + Uninstalls Driver Binding Protocol with optional Component Name, Component Name
[edk2] [PATCH 4/4] NetworkPkg/IScsiDxe: Update UEFILib Usage
Update interfaces as exposed by UEFILib for protocol installation and uninstallation abstraction. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Ashish Singhal --- NetworkPkg/IScsiDxe/IScsiDriver.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/NetworkPkg/IScsiDxe/IScsiDriver.c b/NetworkPkg/IScsiDxe/IScsiDriver.c index 8747de7..fa0ea00 100644 --- a/NetworkPkg/IScsiDxe/IScsiDriver.c +++ b/NetworkPkg/IScsiDxe/IScsiDriver.c @@ -1863,14 +1863,20 @@ Error3: Error2: EfiLibUninstallDriverBindingComponentName2 ( +ImageHandle, +SystemTable, , +NULL, , ); Error1: EfiLibUninstallDriverBindingComponentName2 ( +ImageHandle, +SystemTable, , +NULL, , ); -- 2.7.4 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH 2/4] NetworkPkg/IScsiDxe: Use UEFILib APIs to uninstall protocols.
During cleanup in case of initialization failure, some driver bindings are not installed. Using abstractions in UEFILib takes care of it. REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1428 Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Ashish Singhal --- NetworkPkg/IScsiDxe/IScsiDriver.c | 31 +++ 1 file changed, 11 insertions(+), 20 deletions(-) diff --git a/NetworkPkg/IScsiDxe/IScsiDriver.c b/NetworkPkg/IScsiDxe/IScsiDriver.c index 91176e6..8747de7 100644 --- a/NetworkPkg/IScsiDxe/IScsiDriver.c +++ b/NetworkPkg/IScsiDxe/IScsiDriver.c @@ -1,6 +1,7 @@ /** @file The entry point of IScsi driver. +Copyright (c) 2019, NVIDIA Corporation. All rights reserved. Copyright (c) 2004 - 2018, Intel Corporation. All rights reserved. (C) Copyright 2017 Hewlett Packard Enterprise Development LP @@ -1861,28 +1862,18 @@ Error3: ); Error2: - gBS->UninstallMultipleProtocolInterfaces ( - gIScsiIp6DriverBinding.DriverBindingHandle, - , - , - , - , - , - , - NULL - ); + EfiLibUninstallDriverBindingComponentName2 ( +, +, + +); Error1: - gBS->UninstallMultipleProtocolInterfaces ( - ImageHandle, - , - , - , - , - , - , - NULL - ); + EfiLibUninstallDriverBindingComponentName2 ( +, +, + +); return Status; } -- 2.7.4 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH 0/4] Provide UEFILib functions for protocol uninstallation.
An issue was seen in IScsiDxe in NetworkPkg where driver cleanup after initialization failure was not done right. Bug 1428 was filed in this regard. As per discussions with Mike, it was also discussed that having UEFILib provide protocol uninstallation abstraction would help to avoid these issues in the future. Bug 1429 was found to track this. The first 2 patches take care of this. Patch number 3 simplifies the UEFILib protocol installation and uninstallation abstraction by adding a helper function doing operations instead of every public function. Patch set 4 uses the updated uninstallation interfaces as a result of patch 3. Ashish Singhal (4): MdePkg/UefiLib: Abstract driver model protocol uninstallation NetworkPkg/IScsiDxe: Use UEFILib APIs to uninstall protocols. MdePkg/UefiLib: Simplify protocol un/installation abstraction NetworkPkg/IScsiDxe: Update UEFILib Usage MdePkg/Include/Library/UefiLib.h | 127 MdePkg/Library/UefiLib/UefiDriverModel.c | 1205 +- NetworkPkg/IScsiDxe/IScsiDriver.c| 37 +- 3 files changed, 484 insertions(+), 885 deletions(-) -- 2.7.4 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] Uninstalling Invalid Protocol Interfaces
Mike, I have filed https://bugzilla.tianocore.org/show_bug.cgi?id=1428 and https://bugzilla.tianocore.org/show_bug.cgi?id=1429 to address this. I have assigned these to me as I already have fix for these. Also, how do we ensure all components which may have this issue (may not have exposed it yet) also absorb the change? Thanks Ashish From: Kinney, Michael D Sent: Friday, January 4, 2019 10:15 AM To: Ashish Singhal ; edk2-devel@lists.01.org; Kinney, Michael D Cc: Gao, Liming ; Fu, Siyuan ; Wu, Jiaxin Subject: RE: Uninstalling Invalid Protocol Interfaces Ashish, Thanks for the pointer. I agree there is an issue here. Please enter a Bugzilla against the IScsiDxe module for this issue so we can fix this failure. You are also welcome to enter a Bugzilla for a feature request to add UefiLib APIs that can be used to safely uninstall all the driver model related protocol that can be used in error paths in the entry point and in unload handlers. Thanks, Mike From: Ashish Singhal [mailto:ashishsin...@nvidia.com] Sent: Thursday, January 3, 2019 4:16 PM To: Kinney, Michael D mailto:michael.d.kin...@intel.com>>; edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org> Cc: Gao, Liming mailto:liming@intel.com>>; Fu, Siyuan mailto:siyuan...@intel.com>>; Wu, Jiaxin mailto:jiaxin...@intel.com>> Subject: RE: Uninstalling Invalid Protocol Interfaces Hi Mike, Call to UninstallMultipleProtocolInterfaces at https://github.com/tianocore/edk2/blob/master/NetworkPkg/IScsiDxe/IScsiDriver.c#L1864 fails because the component name interface may not be installed depending on the state of PCD. Thanks Ashish From: Kinney, Michael D mailto:michael.d.kin...@intel.com>> Sent: Thursday, January 3, 2019 5:09 PM To: Ashish Singhal mailto:ashishsin...@nvidia.com>>; edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>; Kinney, Michael D mailto:michael.d.kin...@intel.com>> Cc: Gao, Liming mailto:liming@intel.com>>; Fu, Siyuan mailto:siyuan...@intel.com>>; Wu, Jiaxin mailto:jiaxin...@intel.com>> Subject: RE: Uninstalling Invalid Protocol Interfaces Hi Ashish, Can you provide a pointer to the UninstallMultipleProtocolInterfaces() call that is causing this failure? I agree that the UefiLib could provide additional services to simplify the implementation of the unload handlers. Matching the UefiLib APIs that install the Uefi Driver Model protocol would be useful, so the driver entry point and the driver unload functions can use matching APIs. Mike From: Ashish Singhal [mailto:ashishsin...@nvidia.com] Sent: Thursday, January 3, 2019 3:39 PM To: edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org> Cc: Kinney, Michael D mailto:michael.d.kin...@intel.com>>; Gao, Liming mailto:liming@intel.com>>; Fu, Siyuan mailto:siyuan...@intel.com>>; Wu, Jiaxin mailto:jiaxin...@intel.com>> Subject: Uninstalling Invalid Protocol Interfaces Hello, As part of moving from MdeModulePkg implementation of IScsiDxe to the implementation in NetworkPkg, I started hitting exception in the driver loaded after IScsiDxe if IScsiDxe's installation fails for some reason. Upon debugging, I found out that calls to UninstallMultipleProtocolInterfaces as part of Error 2 as well Error 1 fail while trying to uninstall component name protocol-interface pair and return error code EFI_INVALID_PARAMETER. As per UninstallMultipleProtocolInterfaces's documentation, if uninstallation of any of the input protocol-interface pair fails, it will reinstall any just uninstalled protocol and return EFI_INVALID_PARAMETER which causes the cleanup of this driver corrupt leading to failure in next driver getting loaded. The reason of failure in UninstallMultipleProtocolInterfaces is because the driver uses EfiLibInstallDriverBindingComponentName2 to install interfaces which may not install component name interfaces depending on the value of PCDs PcdComponentNameDi sable and PcdComponentName2Disable. I have the following proposals to get around this issue. 1. Instead of calling UninstallMultipleProtocolInterfaces once and list all protocol-interface pairs, do it sequentially for every pair so that the once which was installed correctly, gets uninstalled instead of getting reinstalled because of a failure uninstalling another pair. This would however make us not really use use UninstallMultipleProtocolInterfaces API to its full caliber. 2. In UEFILib, add a function EfiLibUninstallDriverBindingComponentName2 for uninstalling protocol interfaces taking into consideration the state of PCDs PcdComponentNameDisable and PcdComponentName2Disable. 3. In UEFILib, add uninstall functions for all corresponding install APIs to get coverage for all scenarios. I would certainly prefer option 2 or 3 as they seem to be more correct and would provide a for all drivers which may hit the
Re: [edk2] Uninstalling Invalid Protocol Interfaces
Hi Mike, Call to UninstallMultipleProtocolInterfaces at https://github.com/tianocore/edk2/blob/master/NetworkPkg/IScsiDxe/IScsiDriver.c#L1864 fails because the component name interface may not be installed depending on the state of PCD. Thanks Ashish From: Kinney, Michael D Sent: Thursday, January 3, 2019 5:09 PM To: Ashish Singhal ; edk2-devel@lists.01.org; Kinney, Michael D Cc: Gao, Liming ; Fu, Siyuan ; Wu, Jiaxin Subject: RE: Uninstalling Invalid Protocol Interfaces Hi Ashish, Can you provide a pointer to the UninstallMultipleProtocolInterfaces() call that is causing this failure? I agree that the UefiLib could provide additional services to simplify the implementation of the unload handlers. Matching the UefiLib APIs that install the Uefi Driver Model protocol would be useful, so the driver entry point and the driver unload functions can use matching APIs. Mike From: Ashish Singhal [mailto:ashishsin...@nvidia.com] Sent: Thursday, January 3, 2019 3:39 PM To: edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org> Cc: Kinney, Michael D mailto:michael.d.kin...@intel.com>>; Gao, Liming mailto:liming@intel.com>>; Fu, Siyuan mailto:siyuan...@intel.com>>; Wu, Jiaxin mailto:jiaxin...@intel.com>> Subject: Uninstalling Invalid Protocol Interfaces Hello, As part of moving from MdeModulePkg implementation of IScsiDxe to the implementation in NetworkPkg, I started hitting exception in the driver loaded after IScsiDxe if IScsiDxe's installation fails for some reason. Upon debugging, I found out that calls to UninstallMultipleProtocolInterfaces as part of Error 2 as well Error 1 fail while trying to uninstall component name protocol-interface pair and return error code EFI_INVALID_PARAMETER. As per UninstallMultipleProtocolInterfaces's documentation, if uninstallation of any of the input protocol-interface pair fails, it will reinstall any just uninstalled protocol and return EFI_INVALID_PARAMETER which causes the cleanup of this driver corrupt leading to failure in next driver getting loaded. The reason of failure in UninstallMultipleProtocolInterfaces is because the driver uses EfiLibInstallDriverBindingComponentName2 to install interfaces which may not install component name interfaces depending on the value of PCDs PcdComponentNameDi sable and PcdComponentName2Disable. I have the following proposals to get around this issue. 1. Instead of calling UninstallMultipleProtocolInterfaces once and list all protocol-interface pairs, do it sequentially for every pair so that the once which was installed correctly, gets uninstalled instead of getting reinstalled because of a failure uninstalling another pair. This would however make us not really use use UninstallMultipleProtocolInterfaces API to its full caliber. 2. In UEFILib, add a function EfiLibUninstallDriverBindingComponentName2 for uninstalling protocol interfaces taking into consideration the state of PCDs PcdComponentNameDisable and PcdComponentName2Disable. 3. In UEFILib, add uninstall functions for all corresponding install APIs to get coverage for all scenarios. I would certainly prefer option 2 or 3 as they seem to be more correct and would provide a for all drivers which may hit the issue. I am happy to make the code changes as needed and suggested by the maintainers. Thanks Ashish 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
[edk2] Uninstalling Invalid Protocol Interfaces
Hello, As part of moving from MdeModulePkg implementation of IScsiDxe to the implementation in NetworkPkg, I started hitting exception in the driver loaded after IScsiDxe if IScsiDxe's installation fails for some reason. Upon debugging, I found out that calls to UninstallMultipleProtocolInterfaces as part of Error 2 as well Error 1 fail while trying to uninstall component name protocol-interface pair and return error code EFI_INVALID_PARAMETER. As per UninstallMultipleProtocolInterfaces's documentation, if uninstallation of any of the input protocol-interface pair fails, it will reinstall any just uninstalled protocol and return EFI_INVALID_PARAMETER which causes the cleanup of this driver corrupt leading to failure in next driver getting loaded. The reason of failure in UninstallMultipleProtocolInterfaces is because the driver uses EfiLibInstallDriverBindingComponentName2 to install interfaces which may not install component name interfaces depending on the value of PCDs PcdComponentNameDi sable and PcdComponentName2Disable. I have the following proposals to get around this issue. 1. Instead of calling UninstallMultipleProtocolInterfaces once and list all protocol-interface pairs, do it sequentially for every pair so that the once which was installed correctly, gets uninstalled instead of getting reinstalled because of a failure uninstalling another pair. This would however make us not really use use UninstallMultipleProtocolInterfaces API to its full caliber. 2. In UEFILib, add a function EfiLibUninstallDriverBindingComponentName2 for uninstalling protocol interfaces taking into consideration the state of PCDs PcdComponentNameDisable and PcdComponentName2Disable. 3. In UEFILib, add uninstall functions for all corresponding install APIs to get coverage for all scenarios. I would certainly prefer option 2 or 3 as they seem to be more correct and would provide a for all drivers which may hit the issue. I am happy to make the code changes as needed and suggested by the maintainers. Thanks Ashish --- 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
Re: [edk2] [PATCH v7] MdeModulePkg/SdMmcPciHcDxe: Add SDMMC HC v4 and above Support.
Hello Hao,
I have submitted patch v8 which incorporates all the changes.
Thanks
Ashish
-Original Message-
From: Wu, Hao A
Sent: Tuesday, January 1, 2019 11:37 PM
To: Wu, Hao A ; Ashish Singhal ;
edk2-devel@lists.01.org
Subject: RE: [edk2] [PATCH v7] MdeModulePkg/SdMmcPciHcDxe: Add SDMMC HC v4 and
above Support.
Missed one comment for commit message:
"If V4 64 bit address mode is enabled in compatibility register"
->
"If V4 64 bit address mode is supported in capabilities register"
Best Regards,
Hao Wu
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Wu, Hao A
> Sent: Wednesday, January 02, 2019 2:29 PM
> To: Ashish Singhal; edk2-devel@lists.01.org
> Subject: Re: [edk2] [PATCH v7] MdeModulePkg/SdMmcPciHcDxe: Add SDMMC
> HC v4 and above Support.
>
> Hello,
>
> I have 2 minor comments, please refer to those inline comments.
> Apart from that, the patch is good to me:
> Reviewed-by: Hao Wu
>
> If you agree those inline comments, I will directly modify the patch
> on my side and then push it into the repository.
>
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf
> > Of Ashish Singhal
> > Sent: Wednesday, December 19, 2018 5:29 AM
> > To: edk2-devel@lists.01.org
> > Cc: Ashish Singhal
> > Subject: [edk2] [PATCH v7] MdeModulePkg/SdMmcPciHcDxe: Add SDMMC HC
> > v4 and above Support.
>
> Add the Bugzilla tracker information at the start of the commit log message:
>
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1359
>
> >
> > Add SDMA, ADMA2 and 26b data length support.
> >
> > If V4 64 bit address mode is enabled in compatibility register,
> > program controller to enable V4 host mode and use appropriate SDMA
> > registers supporting 64 bit addresses.
> >
> > If V4 64 bit address mode is enabled in compatibility register,
> > program controller to enable V4 host mode and use appropriate ADMA
> > descriptors supporting 64 bit addresses.
> >
> > If host controller version is above V4.0, enable ADMA2 with 26b data
> > length support for better performance. HC 2 register is configured
> > to use 26 bit data lengths and ADMA2 descriptors are configured
> appropriately.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Ashish Singhal
> > ---
> > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c| 2 +-
> > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c | 4 +-
> > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c | 21 +-
> > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h | 7 +-
> > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c | 328
> > +
> > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h | 84 --
> > 6 files changed, 363 insertions(+), 83 deletions(-) mode change
> > 100755 => 100644 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
> >
> > diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
> > b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
> > old mode 100755
> > new mode 100644
> > index 2d3fb68..0c5646f
> > --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
> > +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
> > @@ -707,7 +707,7 @@ EmmcSwitchClockFreq (
> >//
> >// Convert the clock freq unit from MHz to KHz.
> >//
> > - Status = SdMmcHcClockSupply (PciIo, Slot, ClockFreq * 1000,
> > Private-
> > >BaseClkFreq[Slot]);
> > + Status = SdMmcHcClockSupply (PciIo, Slot, ClockFreq * 1000,
> > + Private-
> > >BaseClkFreq[Slot], Private->ControllerVersion[Slot]);
> >if (EFI_ERROR (Status)) {
> > return Status;
> >}
> > diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> > b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> > index 68485c8..cdcdfa3 100644
> > --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> > +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> > @@ -864,7 +864,7 @@ SdCardSetBusMode (
> > return Status;
> >}
> >
> > - Status = SdMmcHcClockSupply (PciIo, Slot, ClockFreq * 1000,
> > Private-
> > >BaseClkFreq[Slot]);
> > + Status = SdMmcHcClockSupply (PciIo, Slot, ClockFreq * 1000,
> > + Private-
> > >BaseClkFreq[Slot], Private->ControllerVersion[Slot]);
> >if (EFI_ERROR (Status)) {
> > return Status;
> >}
> > @@ -1064,7 +1064,7 @@ SdCardIdentification (
> > goto Error;
> >}
> >
[edk2] [PATCH v8] MdeModulePkg/SdMmcPciHcDxe: Add SDMMC HC v4 and above Support.
Add SDMA, ADMA2 and 26b data length support.
If V4 64 bit address mode is supported in capabilities register,
program controller to enable V4 host mode and use appropriate
SDMA registers supporting 64 bit addresses.
If V4 64 bit address mode is supported in capabilities register,
program controller to enable V4 host mode and use appropriate
ADMA descriptors supporting 64 bit addresses.
If host controller version is above V4.0, enable ADMA2 with 26b data
length support for better performance. HC 2 register is configured to
use 26 bit data lengths and ADMA2 descriptors are configured appropriately.
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1359
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ashish Singhal
Reviewed-by: Hao Wu
---
MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c| 3 +-
MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c | 5 +-
MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c | 22 +-
MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h | 7 +-
MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c | 328 +
MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h | 84 --
6 files changed, 366 insertions(+), 83 deletions(-)
mode change 100755 => 100644 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
old mode 100755
new mode 100644
index 2d3fb68..4ef849f
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
@@ -1,6 +1,7 @@
/** @file
This file provides some helper functions which are specific for EMMC device.
+ Copyright (c) 2018, NVIDIA CORPORATION. All rights reserved.
Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved.
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD
License
@@ -707,7 +708,7 @@ EmmcSwitchClockFreq (
//
// Convert the clock freq unit from MHz to KHz.
//
- Status = SdMmcHcClockSupply (PciIo, Slot, ClockFreq * 1000,
Private->BaseClkFreq[Slot]);
+ Status = SdMmcHcClockSupply (PciIo, Slot, ClockFreq * 1000,
Private->BaseClkFreq[Slot], Private->ControllerVersion[Slot]);
if (EFI_ERROR (Status)) {
return Status;
}
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
index 68485c8..83e6bf0 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
@@ -1,6 +1,7 @@
/** @file
This file provides some helper functions which are specific for SD card
device.
+ Copyright (c) 2018, NVIDIA CORPORATION. All rights reserved.
Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved.
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD
License
@@ -864,7 +865,7 @@ SdCardSetBusMode (
return Status;
}
- Status = SdMmcHcClockSupply (PciIo, Slot, ClockFreq * 1000,
Private->BaseClkFreq[Slot]);
+ Status = SdMmcHcClockSupply (PciIo, Slot, ClockFreq * 1000,
Private->BaseClkFreq[Slot], Private->ControllerVersion[Slot]);
if (EFI_ERROR (Status)) {
return Status;
}
@@ -1064,7 +1065,7 @@ SdCardIdentification (
goto Error;
}
- SdMmcHcInitClockFreq (PciIo, Slot, Private->BaseClkFreq[Slot]);
+ SdMmcHcInitClockFreq (PciIo, Slot, Private->BaseClkFreq[Slot],
Private->ControllerVersion[Slot]);
gBS->Stall (1000);
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
index a87f8de..76c32a4 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
@@ -4,6 +4,7 @@
It would expose EFI_SD_MMC_PASS_THRU_PROTOCOL for upper layer use.
+ Copyright (c) 2018, NVIDIA CORPORATION. All rights reserved.
Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved.
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD
License
@@ -62,7 +63,9 @@ SD_MMC_HC_PRIVATE_DATA gSdMmcPciHcTemplate = {
{ // MaxCurrent
0,
},
- 0 // ControllerVersion
+ {
+0 // ControllerVersion
+ }
};
SD_DEVICE_PATHmSdDpTemplate = {
@@ -621,6 +624,14 @@ SdMmcPciHcDriverBindingStart (
for (Slot = FirstBar; Slot < (FirstBar + SlotNum); Slot++) {
Private->Slot[Slot].Enable = TRUE;
+//
+// Get SD/MMC Pci Host Controller Version
+//
+Status = SdMmcHcGetControllerVersion (PciIo, Slot,
>ControllerVersion[Slot]);
+if (EFI_ERROR (Status)) {
+ continue;
+}
+
Status = SdMmcHcGetCapability (PciIo, Slot, >Capability[Slot]);
if (EFI_ERROR (
Re: [edk2] [PATCH v7] MdeModulePkg/SdMmcPciHcDxe: Add SDMMC HC v4 and above Support.
Hello Hao,
Do you happen to have any update on this?
Thanks
Ashish
-Original Message-
From: Wu, Hao A
Sent: Tuesday, December 18, 2018 6:47 PM
To: Ashish Singhal ; edk2-devel@lists.01.org
Subject: RE: [edk2] [PATCH v7] MdeModulePkg/SdMmcPciHcDxe: Add SDMMC HC v4 and
above Support.
Hi,
Could you grant me some time for reviewing this patch?
I will try to give you the feedbacks before the end of WW01 2019. Sorry for the
potential delay.
Best Regards,
Hao Wu
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Ashish Singhal
> Sent: Wednesday, December 19, 2018 5:29 AM
> To: edk2-devel@lists.01.org
> Cc: Ashish Singhal
> Subject: [edk2] [PATCH v7] MdeModulePkg/SdMmcPciHcDxe: Add SDMMC HC v4
> and above Support.
>
> Add SDMA, ADMA2 and 26b data length support.
>
> If V4 64 bit address mode is enabled in compatibility register,
> program controller to enable V4 host mode and use appropriate SDMA
> registers supporting 64 bit addresses.
>
> If V4 64 bit address mode is enabled in compatibility register,
> program controller to enable V4 host mode and use appropriate ADMA
> descriptors supporting 64 bit addresses.
>
> If host controller version is above V4.0, enable ADMA2 with 26b data
> length support for better performance. HC 2 register is configured to
> use 26 bit data lengths and ADMA2 descriptors are configured appropriately.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ashish Singhal
> ---
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c| 2 +-
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c | 4 +-
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c | 21 +-
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h | 7 +-
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c | 328
> +
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h | 84 --
> 6 files changed, 363 insertions(+), 83 deletions(-) mode change
> 100755 => 100644 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
>
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
> old mode 100755
> new mode 100644
> index 2d3fb68..0c5646f
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
> @@ -707,7 +707,7 @@ EmmcSwitchClockFreq (
>//
>// Convert the clock freq unit from MHz to KHz.
>//
> - Status = SdMmcHcClockSupply (PciIo, Slot, ClockFreq * 1000,
> Private-
> >BaseClkFreq[Slot]);
> + Status = SdMmcHcClockSupply (PciIo, Slot, ClockFreq * 1000,
> + Private-
> >BaseClkFreq[Slot], Private->ControllerVersion[Slot]);
>if (EFI_ERROR (Status)) {
> return Status;
>}
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> index 68485c8..cdcdfa3 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> @@ -864,7 +864,7 @@ SdCardSetBusMode (
> return Status;
>}
>
> - Status = SdMmcHcClockSupply (PciIo, Slot, ClockFreq * 1000,
> Private-
> >BaseClkFreq[Slot]);
> + Status = SdMmcHcClockSupply (PciIo, Slot, ClockFreq * 1000,
> + Private-
> >BaseClkFreq[Slot], Private->ControllerVersion[Slot]);
>if (EFI_ERROR (Status)) {
> return Status;
>}
> @@ -1064,7 +1064,7 @@ SdCardIdentification (
> goto Error;
>}
>
> - SdMmcHcInitClockFreq (PciIo, Slot, Private->BaseClkFreq[Slot]);
> + SdMmcHcInitClockFreq (PciIo, Slot, Private->BaseClkFreq[Slot],
> + Private-
> >ControllerVersion[Slot]);
>
>gBS->Stall (1000);
>
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> index a87f8de..b5bc260 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> @@ -62,7 +62,9 @@ SD_MMC_HC_PRIVATE_DATA gSdMmcPciHcTemplate = {
>{ // MaxCurrent
> 0,
>},
> - 0 // ControllerVersion
> + {
> +0 // ControllerVersion
> + }
> };
>
> SD_DEVICE_PATHmSdDpTemplate = {
> @@ -621,6 +623,14 @@ SdMmcPciHcDriverBindingStart (
>for (Slot = FirstBar; Slot < (FirstBar + SlotNum); Slot++) {
> Private->Slot[Slot].Enable = TRUE;
>
> +//
> +// Get SD/MMC Pci Host Controller Version
> +//
> +Status = SdMmcHcGetControllerVersion (PciIo, Slot,
> >ControllerVersion[Slot]);
> +i
Re: [edk2] [PATCH v7] MdeModulePkg/SdMmcPciHcDxe: Add SDMMC HC v4 and above Support.
No problem. Please let me know when you have reviewed it and I will send the
final patch with your name as reviewer.
Thanks
Ashish
From: Wu, Hao A
Sent: Tuesday, December 18, 2018 6:46:57 PM
To: Ashish Singhal; edk2-devel@lists.01.org
Subject: RE: [edk2] [PATCH v7] MdeModulePkg/SdMmcPciHcDxe: Add SDMMC HC v4 and
above Support.
Hi,
Could you grant me some time for reviewing this patch?
I will try to give you the feedbacks before the end of WW01 2019. Sorry for the
potential delay.
Best Regards,
Hao Wu
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Ashish Singhal
> Sent: Wednesday, December 19, 2018 5:29 AM
> To: edk2-devel@lists.01.org
> Cc: Ashish Singhal
> Subject: [edk2] [PATCH v7] MdeModulePkg/SdMmcPciHcDxe: Add SDMMC
> HC v4 and above Support.
>
> Add SDMA, ADMA2 and 26b data length support.
>
> If V4 64 bit address mode is enabled in compatibility register,
> program controller to enable V4 host mode and use appropriate
> SDMA registers supporting 64 bit addresses.
>
> If V4 64 bit address mode is enabled in compatibility register,
> program controller to enable V4 host mode and use appropriate
> ADMA descriptors supporting 64 bit addresses.
>
> If host controller version is above V4.0, enable ADMA2 with 26b data
> length support for better performance. HC 2 register is configured to
> use 26 bit data lengths and ADMA2 descriptors are configured appropriately.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ashish Singhal
> ---
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c| 2 +-
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c | 4 +-
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c | 21 +-
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h | 7 +-
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c | 328
> +
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h | 84 --
> 6 files changed, 363 insertions(+), 83 deletions(-)
> mode change 100755 => 100644
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
>
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
> old mode 100755
> new mode 100644
> index 2d3fb68..0c5646f
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
> @@ -707,7 +707,7 @@ EmmcSwitchClockFreq (
>//
>// Convert the clock freq unit from MHz to KHz.
>//
> - Status = SdMmcHcClockSupply (PciIo, Slot, ClockFreq * 1000, Private-
> >BaseClkFreq[Slot]);
> + Status = SdMmcHcClockSupply (PciIo, Slot, ClockFreq * 1000, Private-
> >BaseClkFreq[Slot], Private->ControllerVersion[Slot]);
>if (EFI_ERROR (Status)) {
> return Status;
>}
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> index 68485c8..cdcdfa3 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> @@ -864,7 +864,7 @@ SdCardSetBusMode (
> return Status;
>}
>
> - Status = SdMmcHcClockSupply (PciIo, Slot, ClockFreq * 1000, Private-
> >BaseClkFreq[Slot]);
> + Status = SdMmcHcClockSupply (PciIo, Slot, ClockFreq * 1000, Private-
> >BaseClkFreq[Slot], Private->ControllerVersion[Slot]);
>if (EFI_ERROR (Status)) {
> return Status;
>}
> @@ -1064,7 +1064,7 @@ SdCardIdentification (
> goto Error;
>}
>
> - SdMmcHcInitClockFreq (PciIo, Slot, Private->BaseClkFreq[Slot]);
> + SdMmcHcInitClockFreq (PciIo, Slot, Private->BaseClkFreq[Slot], Private-
> >ControllerVersion[Slot]);
>
>gBS->Stall (1000);
>
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> index a87f8de..b5bc260 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> @@ -62,7 +62,9 @@ SD_MMC_HC_PRIVATE_DATA gSdMmcPciHcTemplate =
> {
>{ // MaxCurrent
> 0,
>},
> - 0 // ControllerVersion
> + {
> +0 // ControllerVersion
> + }
> };
>
> SD_DEVICE_PATHmSdDpTemplate = {
> @@ -621,6 +623,14 @@ SdMmcPciHcDriverBindingStart (
>for (Slot = FirstBar; Slot < (FirstBar + SlotNum); Slot++) {
> Private->Slot[Slot].Enable = TRUE;
>
> +//
> +// Get SD/MMC Pci Host Controller Version
> +//
> +Status = SdMmcHcGetControllerVersion (PciIo, Slot,
> >ControllerVersion
[edk2] [PATCH v7] MdeModulePkg/SdMmcPciHcDxe: Add SDMMC HC v4 and above Support.
Add SDMA, ADMA2 and 26b data length support.
If V4 64 bit address mode is enabled in compatibility register,
program controller to enable V4 host mode and use appropriate
SDMA registers supporting 64 bit addresses.
If V4 64 bit address mode is enabled in compatibility register,
program controller to enable V4 host mode and use appropriate
ADMA descriptors supporting 64 bit addresses.
If host controller version is above V4.0, enable ADMA2 with 26b data
length support for better performance. HC 2 register is configured to
use 26 bit data lengths and ADMA2 descriptors are configured appropriately.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ashish Singhal
---
MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c| 2 +-
MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c | 4 +-
MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c | 21 +-
MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h | 7 +-
MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c | 328 +
MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h | 84 --
6 files changed, 363 insertions(+), 83 deletions(-)
mode change 100755 => 100644 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
old mode 100755
new mode 100644
index 2d3fb68..0c5646f
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
@@ -707,7 +707,7 @@ EmmcSwitchClockFreq (
//
// Convert the clock freq unit from MHz to KHz.
//
- Status = SdMmcHcClockSupply (PciIo, Slot, ClockFreq * 1000,
Private->BaseClkFreq[Slot]);
+ Status = SdMmcHcClockSupply (PciIo, Slot, ClockFreq * 1000,
Private->BaseClkFreq[Slot], Private->ControllerVersion[Slot]);
if (EFI_ERROR (Status)) {
return Status;
}
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
index 68485c8..cdcdfa3 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
@@ -864,7 +864,7 @@ SdCardSetBusMode (
return Status;
}
- Status = SdMmcHcClockSupply (PciIo, Slot, ClockFreq * 1000,
Private->BaseClkFreq[Slot]);
+ Status = SdMmcHcClockSupply (PciIo, Slot, ClockFreq * 1000,
Private->BaseClkFreq[Slot], Private->ControllerVersion[Slot]);
if (EFI_ERROR (Status)) {
return Status;
}
@@ -1064,7 +1064,7 @@ SdCardIdentification (
goto Error;
}
- SdMmcHcInitClockFreq (PciIo, Slot, Private->BaseClkFreq[Slot]);
+ SdMmcHcInitClockFreq (PciIo, Slot, Private->BaseClkFreq[Slot],
Private->ControllerVersion[Slot]);
gBS->Stall (1000);
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
index a87f8de..b5bc260 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
@@ -62,7 +62,9 @@ SD_MMC_HC_PRIVATE_DATA gSdMmcPciHcTemplate = {
{ // MaxCurrent
0,
},
- 0 // ControllerVersion
+ {
+0 // ControllerVersion
+ }
};
SD_DEVICE_PATHmSdDpTemplate = {
@@ -621,6 +623,14 @@ SdMmcPciHcDriverBindingStart (
for (Slot = FirstBar; Slot < (FirstBar + SlotNum); Slot++) {
Private->Slot[Slot].Enable = TRUE;
+//
+// Get SD/MMC Pci Host Controller Version
+//
+Status = SdMmcHcGetControllerVersion (PciIo, Slot,
>ControllerVersion[Slot]);
+if (EFI_ERROR (Status)) {
+ goto Done;
+}
+
Status = SdMmcHcGetCapability (PciIo, Slot, >Capability[Slot]);
if (EFI_ERROR (Status)) {
continue;
@@ -649,7 +659,14 @@ SdMmcPciHcDriverBindingStart (
Private->BaseClkFreq[Slot]
));
-Support64BitDma &= Private->Capability[Slot].SysBus64;
+//
+// 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 == 0 &&
+Private->Capability[Slot].SysBus64V4 == 0) {
+ Support64BitDma = FALSE;
+}
Status = SdMmcHcGetMaxCurrent (PciIo, Slot, >MaxCurrent[Slot]);
if (EFI_ERROR (Status)) {
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
index 8c1a589..1bb701a 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
@@ -2,6 +2,7 @@
Provides some data structure definitions used by the SD/MMC host controller
driver.
+Copyright (c) 2018, NVIDIA CORPORATION. All rights reserved.
Copyright (c) 2015, Intel Corporation. All rights reserved.
This program and the accompanying materials
are licensed and made available under the terms and conditions of
Re: [edk2] [PATCH v6 2/2] MdeModulePkg/SdMmcPciHcDxe: Add SDMMC HC v4 and above Support.
Hello Hao,
I have made all the changes as recommended and have submitted patch v7. I have
completed all verification as suggested by you and it all seems to be working.
I have also made changes for passing controller version through private
structure.
Thanks
Ashish
From: Wu, Hao A
Sent: Friday, November 30, 2018 1:03:15 AM
To: Ashish Singhal; edk2-devel@lists.01.org
Subject: RE: [edk2] [PATCH v6 2/2] MdeModulePkg/SdMmcPciHcDxe: Add SDMMC HC v4
and above Support.
Hello,
Please refer to the inline comments below:
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Ashish Singhal
> Sent: Friday, November 30, 2018 3:15 AM
> To: edk2-devel@lists.01.org
> Cc: Ashish Singhal
> Subject: [edk2] [PATCH v6 2/2] MdeModulePkg/SdMmcPciHcDxe: Add
> SDMMC HC v4 and above Support.
>
> Add SDMA, ADMA2 and 26b data length support.
>
> If V4 64 bit address mode is enabled in compatibility register,
> program controller to enable V4 host mode and use appropriate
> SDMA registers supporting 64 bit addresses.
>
> If V4 64 bit address mode is enabled in compatibility register,
> program controller to enable V4 host mode and use appropriate
> ADMA descriptors supporting 64 bit addresses.
>
> If host controller version is above V4.0, enable ADMA2 with 26b data
> length support for better performance. HC 2 register is configured to
> use 26 bit data lengths and ADMA2 descriptors are configured appropriately.
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1359
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ashish Singhal
> ---
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h | 4 +-
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c | 328
> ++---
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h | 48 ++-
> 3 files changed, 322 insertions(+), 58 deletions(-)
>
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> index 8c1a589..3af7c95 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> @@ -2,6 +2,7 @@
>
>Provides some data structure definitions used by the SD/MMC host
> controller driver.
>
> +Copyright (c) 2018, NVIDIA CORPORATION. All rights reserved.
> Copyright (c) 2015, Intel Corporation. All rights reserved.
> This program and the accompanying materials
> are licensed and made available under the terms and conditions of the BSD
> License
> @@ -150,7 +151,8 @@ typedef struct {
>BOOLEAN Started;
>UINT64 Timeout;
>
> - SD_MMC_HC_ADMA_DESC_LINE*AdmaDesc;
> + SD_MMC_HC_ADMA_32_DESC_LINE *Adma32Desc;
> + SD_MMC_HC_ADMA_64_DESC_LINE *Adma64Desc;
>EFI_PHYSICAL_ADDRESSAdmaDescPhy;
>VOID*AdmaMap;
>UINT32 AdmaPages;
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> index 598b6a3..debc3be 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> @@ -4,6 +4,7 @@
>
>It would expose EFI_SD_MMC_PASS_THRU_PROTOCOL for upper layer use.
>
> + Copyright (c) 2018, NVIDIA CORPORATION. All rights reserved.
>Copyright (c) 2015 - 2017, Intel Corporation. All rights reserved.
>This program and the accompanying materials
>are licensed and made available under the terms and conditions of the
> BSD License
> @@ -418,6 +419,36 @@ SdMmcHcWaitMmioSet (
> }
>
> /**
> + Get the controller version information from the specified slot.
> +
> + @param[in] PciIo The PCI IO protocol instance.
> + @param[in] SlotThe slot number of the SD card to send the
> command to.
> + @param[out] Version The buffer to store the version information.
> +
> + @retval EFI_SUCCESS The operation executes successfully.
> + @retval Others The operation fails.
> +
> +**/
> +EFI_STATUS
> +SdMmcHcGetControllerVersion (
> + IN EFI_PCI_IO_PROTOCOL *PciIo,
> + IN UINT8Slot,
> + OUT UINT16 *Version
> + )
> +{
> + EFI_STATUSStatus;
> +
> + Status = SdMmcHcRwMmio (PciIo, Slot, SD_MMC_HC_CTRL_VER, TRUE,
> sizeof (UINT16), Version);
> + if (EFI_ERROR (Status)) {
> +return Status;
> + }
> +
> + *Version &= 0xFF;
> +
> + return EFI_SUCCESS;
> +}
> +
> +/**
>Software reset the specified SD/M
Re: [edk2] [PATCH v3 2/2] MdeModulePkg/SdMmcPciHcDxe: Add V4 64bit SDMA and ADMA2 support.
Missed one of your suggested compiler related change in Patch 5. Have posted
patch 6.
Thanks
Ashish Singhal
-Original Message-
From: Ashish Singhal
Sent: Thursday, November 29, 2018 11:48 AM
To: 'Wu, Hao A' ; edk2-devel@lists.01.org
Subject: RE: [edk2] [PATCH v3 2/2] MdeModulePkg/SdMmcPciHcDxe: Add V4 64bit
SDMA and ADMA2 support.
Hello Hao,
Answers to your comments:
1. Patch 5 fixes the issue with accessing SD as well as eMMC files.
2. Using Private variable for controller version would mean I can only access
it in functions having an instance of the installed protocol which is not
possible in some functions for example where we set clock dividers. We either
need to use what I have or pass Private or controller version as an argument to
functions using it.
3. I think there is a bigger issue here. During host controller initialization
we need to setup 64b addressing which happens before we initialize 64b DMA in
PCI layer. So what you are suggesting may not be that helpful. Also, what we
are doing right now is that we go through all slots and if controller on any
slot does not support 64b, we do not enable 64b DMA in PCI layer. What is the
likelihood of all controllers on an SOC not supporting similar address width?
Also, shouldn't we be enabling 64b DMA in PCI layer if any of the controller
supports 64b addressing? Wouldn't 64b DMA enabled mean 32b is enabled by
default?
4. Patch 5 has been rebased on tip of edk2.
Thanks
Ashish
-Original Message-
From: Wu, Hao A
Sent: Wednesday, November 28, 2018 12:25 AM
To: Ashish Singhal ; edk2-devel@lists.01.org
Subject: RE: [edk2] [PATCH v3 2/2] MdeModulePkg/SdMmcPciHcDxe: Add V4 64bit
SDMA and ADMA2 support.
Hello,
Sorry for the delayed response.
Apart from inserting the Bugzilla tracker information, several more general
level
comments:
1. Cannot access the files (content) on SD & eMMC devices
After applying (rebasing onto the latest codes) your patches, I found that
though the SD & eMMC devices can be detected, yet the content cannot be
accessed.
There are 1 SD card and 1 embedded eMMC device within the system. Under Shell,
I can see 2 file systems on SD & eMMC devices being detected, but when I try to
access the files on those file systems by using the 'ls' command, it fails,
saying "ls: File Not Found - 'FS0:\'". I can confirm that files can be listed
successfully without applying this patch.
I also tried the 'dblk' command for display the data via BlockIO protocols
created on those devices. I found that for SD, the command works fine. But for
eMMC, the command fails with message "dblk: Read file error - 'BlockIo'".
For the hardware I own, the controllers are all version 3.0. I tested on both
IA32 and X64 arch image. The results were the same as described above.
Unfortunately, I do not have access to boards with SD controller with version
newer than 3.0. So I am not able to perform those tests on my side for Version
4.0 or greater SD controllers.
Do you have any hardware with SD controller of version 3.0? Is it working on
your side?
Please let me know if additional information is needed.
2. On SdMmcHcGetControllerVersion()
I found that there is a 'ControllerVersion' version in the data structure
SD_MMC_HC_PRIVATE_DATA. But currently, this field seems never used.
I think we can get the controller version information only once and use this
field to store it. Hence, this new function can be eliminated and also can
avoid calling it multiple times.
3. Setting the SD_MMC_HC_64_ADDR_EN bit in SdMmcHcV4Init64BitSupport()
The setting of the SD_MMC_HC_64_ADDR_EN bit is decided by the 'SysBus64V4'
field in the CAP register. For me, additional dependency on the 64-bit DMA
support in the PCI layer is needed as well.
In function SdMmcPciHcDriverBindingStart():
//
// Enable 64-bit DMA support in the PCI layer if this controller
// supports it.
//
if (Support64BitDma) {
Status = PciIo->Attributes (
PciIo,
EfiPciIoAttributeOperationEnable,
EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE,
NULL
);
if (EFI_ERROR (Status)) {
DEBUG ((DEBUG_WARN, "SdMmcPciHcDriverBindingStart: failed to enable
64-bit DMA (%r)\n", Status));
}
}
If the above PCI attribute setting fails, the PCI layer will not support the
64-bit DMA.
Hence, I am thinking to introduce a new BOOLEAN field in the
"SD_MMC_HC_PRIVATE_DATA" data structure (maybe putting the 'Support64BitDma'
into the data structure). If the above PCI operation succeeds, the BOOLEAN
field will be set to TRUE, otherwise, FALSE.
And the setting of the SD_MMC_HC_64_ADDR_EN bit should depend on the BOOLEAN
field as well.
4. Please help to rebase the patch upon the latest master branch.
Some changes within the SdMmcPciHcDxe have been pushed recently. Could you help
to rebase this p
Re: [edk2] [PATCH v3 2/2] MdeModulePkg/SdMmcPciHcDxe: Add V4 64bit SDMA and ADMA2 support.
Hello Hao,
Answers to your comments:
1. Patch 5 fixes the issue with accessing SD as well as eMMC files.
2. Using Private variable for controller version would mean I can only access
it in functions having an instance of the installed protocol which is not
possible in some functions for example where we set clock dividers. We either
need to use what I have or pass Private or controller version as an argument to
functions using it.
3. I think there is a bigger issue here. During host controller initialization
we need to setup 64b addressing which happens before we initialize 64b DMA in
PCI layer. So what you are suggesting may not be that helpful. Also, what we
are doing right now is that we go through all slots and if controller on any
slot does not support 64b, we do not enable 64b DMA in PCI layer. What is the
likelihood of all controllers on an SOC not supporting similar address width?
Also, shouldn't we be enabling 64b DMA in PCI layer if any of the controller
supports 64b addressing? Wouldn't 64b DMA enabled mean 32b is enabled by
default?
4. Patch 5 has been rebased on tip of edk2.
Thanks
Ashish
-Original Message-
From: Wu, Hao A
Sent: Wednesday, November 28, 2018 12:25 AM
To: Ashish Singhal ; edk2-devel@lists.01.org
Subject: RE: [edk2] [PATCH v3 2/2] MdeModulePkg/SdMmcPciHcDxe: Add V4 64bit
SDMA and ADMA2 support.
Hello,
Sorry for the delayed response.
Apart from inserting the Bugzilla tracker information, several more general
level
comments:
1. Cannot access the files (content) on SD & eMMC devices
After applying (rebasing onto the latest codes) your patches, I found that
though the SD & eMMC devices can be detected, yet the content cannot be
accessed.
There are 1 SD card and 1 embedded eMMC device within the system. Under Shell,
I can see 2 file systems on SD & eMMC devices being detected, but when I try to
access the files on those file systems by using the 'ls' command, it fails,
saying "ls: File Not Found - 'FS0:\'". I can confirm that files can be listed
successfully without applying this patch.
I also tried the 'dblk' command for display the data via BlockIO protocols
created on those devices. I found that for SD, the command works fine. But for
eMMC, the command fails with message "dblk: Read file error - 'BlockIo'".
For the hardware I own, the controllers are all version 3.0. I tested on both
IA32 and X64 arch image. The results were the same as described above.
Unfortunately, I do not have access to boards with SD controller with version
newer than 3.0. So I am not able to perform those tests on my side for Version
4.0 or greater SD controllers.
Do you have any hardware with SD controller of version 3.0? Is it working on
your side?
Please let me know if additional information is needed.
2. On SdMmcHcGetControllerVersion()
I found that there is a 'ControllerVersion' version in the data structure
SD_MMC_HC_PRIVATE_DATA. But currently, this field seems never used.
I think we can get the controller version information only once and use this
field to store it. Hence, this new function can be eliminated and also can
avoid calling it multiple times.
3. Setting the SD_MMC_HC_64_ADDR_EN bit in SdMmcHcV4Init64BitSupport()
The setting of the SD_MMC_HC_64_ADDR_EN bit is decided by the 'SysBus64V4'
field in the CAP register. For me, additional dependency on the 64-bit DMA
support in the PCI layer is needed as well.
In function SdMmcPciHcDriverBindingStart():
//
// Enable 64-bit DMA support in the PCI layer if this controller
// supports it.
//
if (Support64BitDma) {
Status = PciIo->Attributes (
PciIo,
EfiPciIoAttributeOperationEnable,
EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE,
NULL
);
if (EFI_ERROR (Status)) {
DEBUG ((DEBUG_WARN, "SdMmcPciHcDriverBindingStart: failed to enable
64-bit DMA (%r)\n", Status));
}
}
If the above PCI attribute setting fails, the PCI layer will not support the
64-bit DMA.
Hence, I am thinking to introduce a new BOOLEAN field in the
"SD_MMC_HC_PRIVATE_DATA" data structure (maybe putting the 'Support64BitDma'
into the data structure). If the above PCI operation succeeds, the BOOLEAN
field will be set to TRUE, otherwise, FALSE.
And the setting of the SD_MMC_HC_64_ADDR_EN bit should depend on the BOOLEAN
field as well.
4. Please help to rebase the patch upon the latest master branch.
Some changes within the SdMmcPciHcDxe have been pushed recently. Could you help
to rebase this patch upon the latest changes. Thanks in advance.
Also, some inline comments below.
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Ashish Singhal
> Sent: Tuesday, November 20, 2018 4:59 AM
> To: edk2-devel@lists.01.org
> Cc: Ashish Singhal
> Subject: [edk2] [PATC
[edk2] [PATCH v5 2/2] MdeModulePkg/SdMmcPciHcDxe: Add SDMMC HC v4 and above Support.
Add SDMA, ADMA2 and 26b data length support.
If V4 64 bit address mode is enabled in compatibility register,
program controller to enable V4 host mode and use appropriate
SDMA registers supporting 64 bit addresses.
If V4 64 bit address mode is enabled in compatibility register,
program controller to enable V4 host mode and use appropriate
ADMA descriptors supporting 64 bit addresses.
If host controller version is above V4.0, enable ADMA2 with 26b data
length support for better performance. HC 2 register is configured to
use 26 bit data lengths and ADMA2 descriptors are configured appropriately.
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1359
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ashish Singhal
---
MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h | 4 +-
MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c | 328 ++---
MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h | 48 ++-
3 files changed, 322 insertions(+), 58 deletions(-)
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
index 8c1a589..3af7c95 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
@@ -2,6 +2,7 @@
Provides some data structure definitions used by the SD/MMC host controller
driver.
+Copyright (c) 2018, NVIDIA CORPORATION. All rights reserved.
Copyright (c) 2015, Intel Corporation. All rights reserved.
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD
License
@@ -150,7 +151,8 @@ typedef struct {
BOOLEAN Started;
UINT64 Timeout;
- SD_MMC_HC_ADMA_DESC_LINE*AdmaDesc;
+ SD_MMC_HC_ADMA_32_DESC_LINE *Adma32Desc;
+ SD_MMC_HC_ADMA_64_DESC_LINE *Adma64Desc;
EFI_PHYSICAL_ADDRESSAdmaDescPhy;
VOID*AdmaMap;
UINT32 AdmaPages;
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
index 598b6a3..debc3be 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
@@ -4,6 +4,7 @@
It would expose EFI_SD_MMC_PASS_THRU_PROTOCOL for upper layer use.
+ Copyright (c) 2018, NVIDIA CORPORATION. All rights reserved.
Copyright (c) 2015 - 2017, Intel Corporation. All rights reserved.
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD
License
@@ -418,6 +419,36 @@ SdMmcHcWaitMmioSet (
}
/**
+ Get the controller version information from the specified slot.
+
+ @param[in] PciIo The PCI IO protocol instance.
+ @param[in] SlotThe slot number of the SD card to send the
command to.
+ @param[out] Version The buffer to store the version information.
+
+ @retval EFI_SUCCESS The operation executes successfully.
+ @retval Others The operation fails.
+
+**/
+EFI_STATUS
+SdMmcHcGetControllerVersion (
+ IN EFI_PCI_IO_PROTOCOL *PciIo,
+ IN UINT8Slot,
+ OUT UINT16 *Version
+ )
+{
+ EFI_STATUSStatus;
+
+ Status = SdMmcHcRwMmio (PciIo, Slot, SD_MMC_HC_CTRL_VER, TRUE, sizeof
(UINT16), Version);
+ if (EFI_ERROR (Status)) {
+return Status;
+ }
+
+ *Version &= 0xFF;
+
+ return EFI_SUCCESS;
+}
+
+/**
Software reset the specified SD/MMC host controller and enable all
interrupts.
@param[in] PrivateA pointer to the SD_MMC_HC_PRIVATE_DATA instance.
@@ -776,18 +807,19 @@ SdMmcHcClockSupply (
DEBUG ((DEBUG_INFO, "BaseClkFreq %dMHz Divisor %d ClockFreq %dKhz\n",
BaseClkFreq, Divisor, ClockFreq));
- Status = SdMmcHcRwMmio (PciIo, Slot, SD_MMC_HC_CTRL_VER, TRUE, sizeof
(ControllerVer), );
+ Status = SdMmcHcGetControllerVersion (PciIo, Slot, );
if (EFI_ERROR (Status)) {
return Status;
}
//
// Set SDCLK Frequency Select and Internal Clock Enable fields in Clock
Control register.
//
- if (((ControllerVer & 0xFF) >= SD_MMC_HC_CTRL_VER_300) &&
- ((ControllerVer & 0xFF) <= SD_MMC_HC_CTRL_VER_420)) {
+ if ((ControllerVer >= SD_MMC_HC_CTRL_VER_300) &&
+ (ControllerVer <= SD_MMC_HC_CTRL_VER_420)) {
ASSERT (Divisor <= 0x3FF);
ClockCtrl = ((Divisor & 0xFF) << 8) | ((Divisor & 0x300) >> 2);
- } else if (((ControllerVer & 0xFF) == 0) || ((ControllerVer & 0xFF) == 1)) {
+ } else if ((ControllerVer == SD_MMC_HC_CTRL_VER_100) ||
+ (ControllerVer == SD_MMC_HC_CTRL_VER_200)) {
//
// Only the most significant bit can be used as divisor.
//
@@ -935,6 +967,60 @@ SdMmcHcSetBusWidth (
}
/**
+ Configure V4 controller enhancements at initialization
[edk2] [PATCH v5 1/2] MdeModulePkg/SdMmcPciHcDxe: Declare V4 64 bit address capability
Add capability declaration for V4.x 64 bit system address support.
This would be used for host controllers working in version 4. Enable
64 bit DMA support in PCI layer if V3 or V4 64 bit support is
enabled in host capability register.
The usage of this new field does not need a guard for version check as
spec for previous SDMMC versions defines this field as reserved with
default value of 0.
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1359
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ashish Singhal
---
MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c | 9 -
MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c | 3 ++-
MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h | 10 +-
3 files changed, 15 insertions(+), 7 deletions(-)
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
index a87f8de..2bfd758 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
@@ -649,7 +649,14 @@ SdMmcPciHcDriverBindingStart (
Private->BaseClkFreq[Slot]
));
-Support64BitDma &= Private->Capability[Slot].SysBus64;
+//
+// 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 == 0 &&^M
+Private->Capability[Slot].SysBus64V4 == 0) {^M
+ Support64BitDma &= FALSE;^M
+}
Status = SdMmcHcGetMaxCurrent (PciIo, Slot, >MaxCurrent[Slot]);
if (EFI_ERROR (Status)) {
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
index ddf6dcf..598b6a3 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
@@ -45,7 +45,8 @@ DumpCapabilityReg (
DEBUG ((DEBUG_INFO, " Voltage 3.3 %a\n", Capability->Voltage33 ?
"TRUE" : "FALSE"));
DEBUG ((DEBUG_INFO, " Voltage 3.0 %a\n", Capability->Voltage30 ?
"TRUE" : "FALSE"));
DEBUG ((DEBUG_INFO, " Voltage 1.8 %a\n", Capability->Voltage18 ?
"TRUE" : "FALSE"));
- DEBUG ((DEBUG_INFO, " 64-bit Sys Bus%a\n", Capability->SysBus64 ?
"TRUE" : "FALSE"));
+ DEBUG ((DEBUG_INFO, " V4 64-bit Sys Bus %a\n", Capability->SysBus64V4 ?
"TRUE" : "FALSE"));
+ DEBUG ((DEBUG_INFO, " V3 64-bit Sys Bus %a\n", Capability->SysBus64V3 ?
"TRUE" : "FALSE"));
DEBUG ((DEBUG_INFO, " Async Interrupt %a\n", Capability->AsyncInt ?
"TRUE" : "FALSE"));
DEBUG ((DEBUG_INFO, " SlotType "));
if (Capability->SlotType == 0x00) {
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
index dd45cbd..ad9ce64 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
@@ -129,24 +129,24 @@ typedef struct {
UINT32 Voltage33:1; // bit 24
UINT32 Voltage30:1; // bit 25
UINT32 Voltage18:1; // bit 26
- UINT32 Reserved3:1; // bit 27
- UINT32 SysBus64:1;// bit 28
+ UINT32 SysBus64V4:1; // bit 27
+ UINT32 SysBus64V3:1; // bit 28
UINT32 AsyncInt:1;// bit 29
UINT32 SlotType:2;// bit 30:31
UINT32 Sdr50:1; // bit 32
UINT32 Sdr104:1; // bit 33
UINT32 Ddr50:1; // bit 34
- UINT32 Reserved4:1; // bit 35
+ UINT32 Reserved3:1; // bit 35
UINT32 DriverTypeA:1; // bit 36
UINT32 DriverTypeC:1; // bit 37
UINT32 DriverTypeD:1; // bit 38
UINT32 DriverType4:1; // bit 39
UINT32 TimerCount:4; // bit 40:43
- UINT32 Reserved5:1; // bit 44
+ UINT32 Reserved4:1; // bit 44
UINT32 TuningSDR50:1; // bit 45
UINT32 RetuningMod:2; // bit 46:47
UINT32 ClkMultiplier:8; // bit 48:55
- UINT32 Reserved6:7; // bit 56:62
+ UINT32 Reserved5:7; // bit 56:62
UINT32 Hs400:1; // bit 63
} SD_MMC_HC_SLOT_CAP;
--
2.7.4
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v3 1/2] MdeModulePkg/SdMmcPciHcDxe: Declare V4 64 bit address capability
Separate V4 1/2 patch has been submitted with changes made as per feedback.
Thanks
Ashish
-Original Message-
From: Wu, Hao A
Sent: Wednesday, November 28, 2018 12:25 AM
To: Ashish Singhal ; edk2-devel@lists.01.org
Subject: RE: [edk2] [PATCH v3 1/2] MdeModulePkg/SdMmcPciHcDxe: Declare V4 64
bit address capability
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Ashish Singhal
> Sent: Tuesday, November 20, 2018 4:59 AM
> To: edk2-devel@lists.01.org
> Cc: Ashish Singhal
> Subject: [edk2] [PATCH v3 1/2] MdeModulePkg/SdMmcPciHcDxe: Declare V4
> 64 bit address capability
>
> Add capability declaration for V4.x 64 bit system address support.
> This would be used for host controllers working in version 4. Enable
> 64 bit DMA support in PCI layer if V3 or V4 64 bit support is enabled
> in host capability register.
>
> The usage of this new field does not need a guard for version check as
> spec for previous SDMMC versions defines this field as reserved with
> default value of 0.
Hello,
Sorry for the delayed response.
I have filed a Tianocore Feature Requests Bugzilla tracker for the 64-bit
SDMA/ADMA support for Sd/MMC host controller driver:
https://bugzilla.tianocore.org/show_bug.cgi?id=1359
Could you help to include this Bugzilla tracker message in your 2 proposed
patches?
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ashish Singhal
> ---
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c | 4 ++--
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c | 3 ++-
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h | 10 +-
> 3 files changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> index bf9869d..1c18ea4 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> @@ -617,7 +617,6 @@ SdMmcPciHcDriverBindingStart (
> }
>}
>
> - Support64BitDma = TRUE;
Please keep the above line, otherwise GCC compiler (I am testing with GCC4.9)
seems not happy with it.
>for (Slot = FirstBar; Slot < (FirstBar + SlotNum); Slot++) {
> Private->Slot[Slot].Enable = TRUE;
>
> @@ -638,7 +637,8 @@ SdMmcPciHcDriverBindingStart (
> }
> DumpCapabilityReg (Slot, >Capability[Slot]);
>
> -Support64BitDma &= Private->Capability[Slot].SysBus64;
> +Support64BitDma = (Private->Capability[Slot].SysBus64V3 |
> + Private->Capability[Slot].SysBus64V4);
For the above statement, how about:
Support64BitDma &= (Private->Capability[Slot].SysBus64V3 |
Private->Capability[Slot].SysBus64V4);
The Visual Studio 2015 complier build fails for your current proposed change.
Best Regards,
Hao Wu
>
> Status = SdMmcHcGetMaxCurrent (PciIo, Slot,
> >MaxCurrent[Slot]);
> if (EFI_ERROR (Status)) {
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> index bedc968..e506875 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> @@ -45,7 +45,8 @@ DumpCapabilityReg (
>DEBUG ((DEBUG_INFO, " Voltage 3.3 %a\n", Capability->Voltage33 ?
> "TRUE" : "FALSE"));
>DEBUG ((DEBUG_INFO, " Voltage 3.0 %a\n", Capability->Voltage30 ?
> "TRUE" : "FALSE"));
>DEBUG ((DEBUG_INFO, " Voltage 1.8 %a\n", Capability->Voltage18 ?
> "TRUE" : "FALSE"));
> - DEBUG ((DEBUG_INFO, " 64-bit Sys Bus%a\n", Capability->SysBus64 ?
> "TRUE" : "FALSE"));
> + DEBUG ((DEBUG_INFO, " V4 64-bit Sys Bus %a\n", Capability-
> >SysBus64V4 ? "TRUE" : "FALSE"));
> + DEBUG ((DEBUG_INFO, " V3 64-bit Sys Bus %a\n", Capability-
> >SysBus64V3 ? "TRUE" : "FALSE"));
>DEBUG ((DEBUG_INFO, " Async Interrupt %a\n", Capability->AsyncInt ?
> "TRUE" : "FALSE"));
>DEBUG ((DEBUG_INFO, " SlotType "));
>if (Capability->SlotType == 0x00) { diff --git
> a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> index 7e3f588..cc138fc 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> @@ -114,24 +114,24 @@ typedef struct {
>UINT32 Voltage33:1; // bit 24
>UINT32 Voltage30:1
Re: [edk2] [PATCH v3 2/2] MdeModulePkg/SdMmcPciHcDxe: Add V4 64bit SDMA and ADMA2 support.
Hello,
Any feedback on this patch yet?
Thanks
Ashish
-Original Message-
From: Ashish Singhal
Sent: Monday, November 19, 2018 1:59 PM
To: edk2-devel@lists.01.org
Cc: Ashish Singhal
Subject: [PATCH v3 2/2] MdeModulePkg/SdMmcPciHcDxe: Add V4 64bit SDMA and ADMA2
support.
If V4 64 bit address mode is enabled in compatibility register, program
controller to enable V4 host mode.
Use appropriate ADMA2 descriptors supporting 64 bit addresses.
Use appropriate registers for SDMA mode operation.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ashish Singhal
---
MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h | 4 +-
MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c | 273 +
MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h | 28 ++-
3 files changed, 260 insertions(+), 45 deletions(-)
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
index c683600..22795df 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
@@ -2,6 +2,7 @@
Provides some data structure definitions used by the SD/MMC host controller
driver.
+Copyright (c) 2018, NVIDIA CORPORATION. All rights reserved.
Copyright (c) 2015, Intel Corporation. All rights reserved. This program
and the accompanying materials are licensed and made available under the terms
and conditions of the BSD License @@ -144,7 +145,8 @@ typedef struct {
BOOLEAN Started;
UINT64 Timeout;
- SD_MMC_HC_ADMA_DESC_LINE*AdmaDesc;
+ SD_MMC_HC_ADMA_32_DESC_LINE *Adma32Desc;
+ SD_MMC_HC_ADMA_64_DESC_LINE *Adma64Desc;
EFI_PHYSICAL_ADDRESSAdmaDescPhy;
VOID*AdmaMap;
UINT32 AdmaPages;
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
index e506875..9fef3fb 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
@@ -4,6 +4,7 @@
It would expose EFI_SD_MMC_PASS_THRU_PROTOCOL for upper layer use.
+ Copyright (c) 2018, NVIDIA CORPORATION. All rights reserved.
Copyright (c) 2015 - 2017, Intel Corporation. All rights reserved.
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD
License @@ -418,6 +419,36 @@ SdMmcHcWaitMmioSet ( }
/**
+ Get the controller version information from the specified slot.
+
+ @param[in] PciIo The PCI IO protocol instance.
+ @param[in] SlotThe slot number of the SD card to send the
command to.
+ @param[out] Version The buffer to store the version information.
+
+ @retval EFI_SUCCESS The operation executes successfully.
+ @retval Others The operation fails.
+
+**/
+EFI_STATUS
+SdMmcHcGetControllerVersion (
+ IN EFI_PCI_IO_PROTOCOL *PciIo,
+ IN UINT8Slot,
+ OUT UINT16 *Version
+ )
+{
+ EFI_STATUSStatus;
+
+ Status = SdMmcHcRwMmio (PciIo, Slot, SD_MMC_HC_CTRL_VER, TRUE, sizeof
+ (UINT16), Version); if (EFI_ERROR (Status)) {
+return Status;
+ }
+
+ *Version &= 0xFF;
+
+ return EFI_SUCCESS;
+}
+
+/**
Software reset the specified SD/MMC host controller and enable all
interrupts.
@param[in] PrivateA pointer to the SD_MMC_HC_PRIVATE_DATA instance.
@@ -776,18 +807,18 @@ SdMmcHcClockSupply (
DEBUG ((DEBUG_INFO, "BaseClkFreq %dMHz Divisor %d ClockFreq %dKhz\n",
BaseClkFreq, Divisor, ClockFreq));
- Status = SdMmcHcRwMmio (PciIo, Slot, SD_MMC_HC_CTRL_VER, TRUE, sizeof
(ControllerVer), );
+ Status = SdMmcHcGetControllerVersion (PciIo, Slot, );
if (EFI_ERROR (Status)) {
return Status;
}
//
// Set SDCLK Frequency Select and Internal Clock Enable fields in Clock
Control register.
//
- if (((ControllerVer & 0xFF) >= SD_MMC_HC_CTRL_VER_300) &&
- ((ControllerVer & 0xFF) <= SD_MMC_HC_CTRL_VER_420)) {
+ if ((ControllerVer >= SD_MMC_HC_CTRL_VER_300) &&
+ (ControllerVer <= SD_MMC_HC_CTRL_VER_420)) {
ASSERT (Divisor <= 0x3FF);
ClockCtrl = ((Divisor & 0xFF) << 8) | ((Divisor & 0x300) >> 2);
- } else if (((ControllerVer & 0xFF) == 0) || ((ControllerVer & 0xFF) == 1)) {
+ } else if ((ControllerVer == 0) || (ControllerVer == 1)) {
//
// Only the most significant bit can be used as divisor.
//
@@ -935,6 +966,54 @@ SdMmcHcSetBusWidth ( }
/**
+ Configure V4 64 bit system address support at initialization.
+
+ @param[in] PciIo The PCI IO protocol instance.
+ @param[in] Slot The slot number of the SD card to send the command
to.
+ @param[in] Capability The capabili
Re: [edk2] [PATCH 2/2] MdeModulePkg/SdMmcPciHcDxe: Add V4 64bit ADMA2 support.
Hi,
I was planning on submitting SDMA changes as a separate patch but that would
have broken SDMA if only ADMA2 patch would have been picked. I have sent V2
patches which combine SDMA as well as ADMA2 changes. I have tested the changes
to be working on V3 as well V4 mode usage both with and without enabling 64b
addressing support.
Thanks
Ashish
-Original Message-
From: Wu, Hao A
Sent: Friday, November 16, 2018 1:27 AM
To: Ashish Singhal ; edk2-devel@lists.01.org
Subject: RE: [edk2] [PATCH 2/2] MdeModulePkg/SdMmcPciHcDxe: Add V4 64bit ADMA2
support.
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Ashish Singhal
> Sent: Friday, November 09, 2018 2:58 AM
> To: edk2-devel@lists.01.org
> Cc: Ashish Singhal
> Subject: [edk2] [PATCH 2/2] MdeModulePkg/SdMmcPciHcDxe: Add V4 64bit
> ADMA2 support.
>
> If V4 64 bit address mode is enabled in compatibility register,
> program controller to enable V4 host mode and use appropriate ADMA
> descriptors supporting 64 bit addresses.
Hi,
I have a quick check on the SD Host Controller Simplified Specification Version
4.20. When the 'Host Version 4 Enable' bit in the HC2 Register is set, for host
controller with version newer than 3.0:
SDMA mode transfer no longer use SDMA System Address Register for system
address, and this SDMA System Address Register will be used as 32-bit Block
Count instead under certain case.
So I think the way to start an SDMA mode transfer is not the same between host
controllers with different versions. However, I do not see this patch handle
this potential behavior change. Please correct me if my understanding is
incorrect here.
Also, could you help to provide information on what tests have been performed
on this series?
Thanks in advance.
Best Regards,
Hao Wu
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ashish Singhal
> ---
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h | 4 +-
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c | 183
> +
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h | 28 +++-
> 3 files changed, 183 insertions(+), 32 deletions(-)
>
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> index c683600..22795df 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> @@ -2,6 +2,7 @@
>
>Provides some data structure definitions used by the SD/MMC host
> controller driver.
>
> +Copyright (c) 2018, NVIDIA CORPORATION. All rights reserved.
> Copyright (c) 2015, Intel Corporation. All rights reserved. This
> program and the accompanying materials are licensed and made
> available under the terms and conditions of the BSD License @@ -144,7
> +145,8 @@ typedef struct {
>BOOLEAN Started;
>UINT64 Timeout;
>
> - SD_MMC_HC_ADMA_DESC_LINE*AdmaDesc;
> + SD_MMC_HC_ADMA_32_DESC_LINE *Adma32Desc;
> + SD_MMC_HC_ADMA_64_DESC_LINE *Adma64Desc;
>EFI_PHYSICAL_ADDRESSAdmaDescPhy;
>VOID*AdmaMap;
>UINT32 AdmaPages;
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> index e506875..bcd2707 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> @@ -4,6 +4,7 @@
>
>It would expose EFI_SD_MMC_PASS_THRU_PROTOCOL for upper layer use.
>
> + Copyright (c) 2018, NVIDIA CORPORATION. All rights reserved.
>Copyright (c) 2015 - 2017, Intel Corporation. All rights reserved.
>This program and the accompanying materials
>are licensed and made available under the terms and conditions of
> the BSD License @@ -418,6 +419,36 @@ SdMmcHcWaitMmioSet ( }
>
> /**
> + Get the controller version information from the specified slot.
> +
> + @param[in] PciIo The PCI IO protocol instance.
> + @param[in] SlotThe slot number of the SD card to send the
> command to.
> + @param[out] Version The buffer to store the version information.
> +
> + @retval EFI_SUCCESS The operation executes successfully.
> + @retval Others The operation fails.
> +
> +**/
> +EFI_STATUS
> +SdMmcHcGetControllerVersion (
> + IN EFI_PCI_IO_PROTOCOL *PciIo,
> + IN UINT8Slot,
> + OUT UINT16 *Version
> + )
> +{
> + EFI_STATUSStatus;
> +
> + Status = SdMmcHcRwMmio (PciIo, Slot, SD_MMC_HC_CTRL_VER, TRU
[edk2] [PATCH v2 2/2] MdeModulePkg/SdMmcPciHcDxe: Add V4 64bit SDMA and ADMA2 support.
If V4 64 bit address mode is enabled in compatibility register,
program controller to enable V4 host mode.
Use appropriate ADMA2 descriptors supporting 64 bit addresses.
Use appropriate registers for SDMA mode operation.
Change-Id: I1f6c984368988e51999eb289aa29677f9b0cdf49
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ashish Singhal
---
MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h | 4 +-
MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c | 273 +
MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h | 28 ++-
3 files changed, 260 insertions(+), 45 deletions(-)
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
index c683600..22795df 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
@@ -2,6 +2,7 @@
Provides some data structure definitions used by the SD/MMC host controller
driver.
+Copyright (c) 2018, NVIDIA CORPORATION. All rights reserved.
Copyright (c) 2015, Intel Corporation. All rights reserved.
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD
License
@@ -144,7 +145,8 @@ typedef struct {
BOOLEAN Started;
UINT64 Timeout;
- SD_MMC_HC_ADMA_DESC_LINE*AdmaDesc;
+ SD_MMC_HC_ADMA_32_DESC_LINE *Adma32Desc;
+ SD_MMC_HC_ADMA_64_DESC_LINE *Adma64Desc;
EFI_PHYSICAL_ADDRESSAdmaDescPhy;
VOID*AdmaMap;
UINT32 AdmaPages;
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
index e506875..9fef3fb 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
@@ -4,6 +4,7 @@
It would expose EFI_SD_MMC_PASS_THRU_PROTOCOL for upper layer use.
+ Copyright (c) 2018, NVIDIA CORPORATION. All rights reserved.
Copyright (c) 2015 - 2017, Intel Corporation. All rights reserved.
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD
License
@@ -418,6 +419,36 @@ SdMmcHcWaitMmioSet (
}
/**
+ Get the controller version information from the specified slot.
+
+ @param[in] PciIo The PCI IO protocol instance.
+ @param[in] SlotThe slot number of the SD card to send the
command to.
+ @param[out] Version The buffer to store the version information.
+
+ @retval EFI_SUCCESS The operation executes successfully.
+ @retval Others The operation fails.
+
+**/
+EFI_STATUS
+SdMmcHcGetControllerVersion (
+ IN EFI_PCI_IO_PROTOCOL *PciIo,
+ IN UINT8Slot,
+ OUT UINT16 *Version
+ )
+{
+ EFI_STATUSStatus;
+
+ Status = SdMmcHcRwMmio (PciIo, Slot, SD_MMC_HC_CTRL_VER, TRUE, sizeof
(UINT16), Version);
+ if (EFI_ERROR (Status)) {
+return Status;
+ }
+
+ *Version &= 0xFF;
+
+ return EFI_SUCCESS;
+}
+
+/**
Software reset the specified SD/MMC host controller and enable all
interrupts.
@param[in] PrivateA pointer to the SD_MMC_HC_PRIVATE_DATA instance.
@@ -776,18 +807,18 @@ SdMmcHcClockSupply (
DEBUG ((DEBUG_INFO, "BaseClkFreq %dMHz Divisor %d ClockFreq %dKhz\n",
BaseClkFreq, Divisor, ClockFreq));
- Status = SdMmcHcRwMmio (PciIo, Slot, SD_MMC_HC_CTRL_VER, TRUE, sizeof
(ControllerVer), );
+ Status = SdMmcHcGetControllerVersion (PciIo, Slot, );
if (EFI_ERROR (Status)) {
return Status;
}
//
// Set SDCLK Frequency Select and Internal Clock Enable fields in Clock
Control register.
//
- if (((ControllerVer & 0xFF) >= SD_MMC_HC_CTRL_VER_300) &&
- ((ControllerVer & 0xFF) <= SD_MMC_HC_CTRL_VER_420)) {
+ if ((ControllerVer >= SD_MMC_HC_CTRL_VER_300) &&
+ (ControllerVer <= SD_MMC_HC_CTRL_VER_420)) {
ASSERT (Divisor <= 0x3FF);
ClockCtrl = ((Divisor & 0xFF) << 8) | ((Divisor & 0x300) >> 2);
- } else if (((ControllerVer & 0xFF) == 0) || ((ControllerVer & 0xFF) == 1)) {
+ } else if ((ControllerVer == 0) || (ControllerVer == 1)) {
//
// Only the most significant bit can be used as divisor.
//
@@ -935,6 +966,54 @@ SdMmcHcSetBusWidth (
}
/**
+ Configure V4 64 bit system address support at initialization.
+
+ @param[in] PciIo The PCI IO protocol instance.
+ @param[in] Slot The slot number of the SD card to send the command
to.
+ @param[in] Capability The capability of the slot.
+
+ @retval EFI_SUCCESS The clock is supplied successfully.
+
+**/
+EFI_STATUS
+SdMmcHcV4Init64BitSupport (
+ IN EFI_PCI_IO_PROTOCOL*PciIo,
+ IN UINT8 Slot,
+ IN SD_MMC_HC_SLOT_CAP Capability
+ )
+
[edk2] [PATCH v2 1/2] MdeModulePkg/SdMmcPciHcDxe: Declare V4 64 bit address capability
Add capability declaration for V4.x 64 bit system address support.
This would be used for host controllers working in version 4. Enable
64 bit DMA support in PCI layer if V3 or V4 64 bit support is
enabled in host capability register.
The usage of this new field does not need a guard for version check as
spec for previous SDMMC versions defines this field as reserved with
default value of 0.
Change-Id: I64fcb674ec566c46a37ea6597ae06cb194625cae
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ashish Singhal
---
MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c | 4 ++--
MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c | 3 ++-
MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h | 10 +-
3 files changed, 9 insertions(+), 8 deletions(-)
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
index bf9869d..1c18ea4 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
@@ -617,7 +617,6 @@ SdMmcPciHcDriverBindingStart (
}
}
- Support64BitDma = TRUE;
for (Slot = FirstBar; Slot < (FirstBar + SlotNum); Slot++) {
Private->Slot[Slot].Enable = TRUE;
@@ -638,7 +637,8 @@ SdMmcPciHcDriverBindingStart (
}
DumpCapabilityReg (Slot, >Capability[Slot]);
-Support64BitDma &= Private->Capability[Slot].SysBus64;
+Support64BitDma = (Private->Capability[Slot].SysBus64V3 |
+ Private->Capability[Slot].SysBus64V4);
Status = SdMmcHcGetMaxCurrent (PciIo, Slot, >MaxCurrent[Slot]);
if (EFI_ERROR (Status)) {
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
index bedc968..e506875 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
@@ -45,7 +45,8 @@ DumpCapabilityReg (
DEBUG ((DEBUG_INFO, " Voltage 3.3 %a\n", Capability->Voltage33 ?
"TRUE" : "FALSE"));
DEBUG ((DEBUG_INFO, " Voltage 3.0 %a\n", Capability->Voltage30 ?
"TRUE" : "FALSE"));
DEBUG ((DEBUG_INFO, " Voltage 1.8 %a\n", Capability->Voltage18 ?
"TRUE" : "FALSE"));
- DEBUG ((DEBUG_INFO, " 64-bit Sys Bus%a\n", Capability->SysBus64 ?
"TRUE" : "FALSE"));
+ DEBUG ((DEBUG_INFO, " V4 64-bit Sys Bus %a\n", Capability->SysBus64V4 ?
"TRUE" : "FALSE"));
+ DEBUG ((DEBUG_INFO, " V3 64-bit Sys Bus %a\n", Capability->SysBus64V3 ?
"TRUE" : "FALSE"));
DEBUG ((DEBUG_INFO, " Async Interrupt %a\n", Capability->AsyncInt ?
"TRUE" : "FALSE"));
DEBUG ((DEBUG_INFO, " SlotType "));
if (Capability->SlotType == 0x00) {
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
index 7e3f588..cc138fc 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
@@ -114,24 +114,24 @@ typedef struct {
UINT32 Voltage33:1; // bit 24
UINT32 Voltage30:1; // bit 25
UINT32 Voltage18:1; // bit 26
- UINT32 Reserved3:1; // bit 27
- UINT32 SysBus64:1;// bit 28
+ UINT32 SysBus64V4:1; // bit 27
+ UINT32 SysBus64V3:1; // bit 28
UINT32 AsyncInt:1;// bit 29
UINT32 SlotType:2;// bit 30:31
UINT32 Sdr50:1; // bit 32
UINT32 Sdr104:1; // bit 33
UINT32 Ddr50:1; // bit 34
- UINT32 Reserved4:1; // bit 35
+ UINT32 Reserved3:1; // bit 35
UINT32 DriverTypeA:1; // bit 36
UINT32 DriverTypeC:1; // bit 37
UINT32 DriverTypeD:1; // bit 38
UINT32 DriverType4:1; // bit 39
UINT32 TimerCount:4; // bit 40:43
- UINT32 Reserved5:1; // bit 44
+ UINT32 Reserved4:1; // bit 44
UINT32 TuningSDR50:1; // bit 45
UINT32 RetuningMod:2; // bit 46:47
UINT32 ClkMultiplier:8; // bit 48:55
- UINT32 Reserved6:7; // bit 56:62
+ UINT32 Reserved5:7; // bit 56:62
UINT32 Hs400:1; // bit 63
} SD_MMC_HC_SLOT_CAP;
--
2.7.4
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH 2/2] MdeModulePkg/SdMmcPciHcDxe: Add V4 64bit ADMA2 support.
If V4 64 bit address mode is enabled in compatibility register,
program controller to enable V4 host mode and use appropriate
ADMA descriptors supporting 64 bit addresses.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ashish Singhal
---
MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h | 4 +-
MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c | 183 +
MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h | 28 +++-
3 files changed, 183 insertions(+), 32 deletions(-)
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
index c683600..22795df 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
@@ -2,6 +2,7 @@
Provides some data structure definitions used by the SD/MMC host controller
driver.
+Copyright (c) 2018, NVIDIA CORPORATION. All rights reserved.
Copyright (c) 2015, Intel Corporation. All rights reserved.
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD
License
@@ -144,7 +145,8 @@ typedef struct {
BOOLEAN Started;
UINT64 Timeout;
- SD_MMC_HC_ADMA_DESC_LINE*AdmaDesc;
+ SD_MMC_HC_ADMA_32_DESC_LINE *Adma32Desc;
+ SD_MMC_HC_ADMA_64_DESC_LINE *Adma64Desc;
EFI_PHYSICAL_ADDRESSAdmaDescPhy;
VOID*AdmaMap;
UINT32 AdmaPages;
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
index e506875..bcd2707 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
@@ -4,6 +4,7 @@
It would expose EFI_SD_MMC_PASS_THRU_PROTOCOL for upper layer use.
+ Copyright (c) 2018, NVIDIA CORPORATION. All rights reserved.
Copyright (c) 2015 - 2017, Intel Corporation. All rights reserved.
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD
License
@@ -418,6 +419,36 @@ SdMmcHcWaitMmioSet (
}
/**
+ Get the controller version information from the specified slot.
+
+ @param[in] PciIo The PCI IO protocol instance.
+ @param[in] SlotThe slot number of the SD card to send the
command to.
+ @param[out] Version The buffer to store the version information.
+
+ @retval EFI_SUCCESS The operation executes successfully.
+ @retval Others The operation fails.
+
+**/
+EFI_STATUS
+SdMmcHcGetControllerVersion (
+ IN EFI_PCI_IO_PROTOCOL *PciIo,
+ IN UINT8Slot,
+ OUT UINT16 *Version
+ )
+{
+ EFI_STATUSStatus;
+
+ Status = SdMmcHcRwMmio (PciIo, Slot, SD_MMC_HC_CTRL_VER, TRUE, sizeof
(UINT16), Version);
+ if (EFI_ERROR (Status)) {
+return Status;
+ }
+
+ *Version &= 0xFF;
+
+ return EFI_SUCCESS;
+}
+
+/**
Software reset the specified SD/MMC host controller and enable all
interrupts.
@param[in] PrivateA pointer to the SD_MMC_HC_PRIVATE_DATA instance.
@@ -776,18 +807,18 @@ SdMmcHcClockSupply (
DEBUG ((DEBUG_INFO, "BaseClkFreq %dMHz Divisor %d ClockFreq %dKhz\n",
BaseClkFreq, Divisor, ClockFreq));
- Status = SdMmcHcRwMmio (PciIo, Slot, SD_MMC_HC_CTRL_VER, TRUE, sizeof
(ControllerVer), );
+ Status = SdMmcHcGetControllerVersion (PciIo, Slot, );
if (EFI_ERROR (Status)) {
return Status;
}
//
// Set SDCLK Frequency Select and Internal Clock Enable fields in Clock
Control register.
//
- if (((ControllerVer & 0xFF) >= SD_MMC_HC_CTRL_VER_300) &&
- ((ControllerVer & 0xFF) <= SD_MMC_HC_CTRL_VER_420)) {
+ if ((ControllerVer >= SD_MMC_HC_CTRL_VER_300) &&
+ (ControllerVer <= SD_MMC_HC_CTRL_VER_420)) {
ASSERT (Divisor <= 0x3FF);
ClockCtrl = ((Divisor & 0xFF) << 8) | ((Divisor & 0x300) >> 2);
- } else if (((ControllerVer & 0xFF) == 0) || ((ControllerVer & 0xFF) == 1)) {
+ } else if ((ControllerVer == 0) || (ControllerVer == 1)) {
//
// Only the most significant bit can be used as divisor.
//
@@ -935,6 +966,41 @@ SdMmcHcSetBusWidth (
}
/**
+ Configure V4 64 bit system address support at initialization.
+
+ @param[in] PciIo The PCI IO protocol instance.
+ @param[in] Slot The slot number of the SD card to send the command
to.
+ @param[in] Capability The capability of the slot.
+
+ @retval EFI_SUCCESS The clock is supplied successfully.
+
+**/
+EFI_STATUS
+SdMmcHcV4Init64BitSupport (
+ IN EFI_PCI_IO_PROTOCOL*PciIo,
+ IN UINT8 Slot,
+ IN SD_MMC_HC_SLOT_CAP Capability
+ )
+{
+ EFI_STATUSStatus;
+ UINT16HostCtrl2;
+
+ //
+ //
[edk2] [PATCH 1/2] MdeModulePkg/SdMmcPciHcDxe: Declare V4 64 bit address capability
Add capability declaration for V4.x 64 bit system address support.
This would be used for host controllers working in version 4. Enable
64 bit DMA support in PCI layer if V3 or V4 64 bit support is
enabled in host capability register.
The usage of this new field does not need a guard for version check as
spec for previous SDMMC versions defines this field as reserved with
default value of 0.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ashish Singhal
---
MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c | 4 ++--
MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c | 3 ++-
MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h | 10 +-
3 files changed, 9 insertions(+), 8 deletions(-)
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
index bf9869d..1c18ea4 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
@@ -617,7 +617,6 @@ SdMmcPciHcDriverBindingStart (
}
}
- Support64BitDma = TRUE;
for (Slot = FirstBar; Slot < (FirstBar + SlotNum); Slot++) {
Private->Slot[Slot].Enable = TRUE;
@@ -638,7 +637,8 @@ SdMmcPciHcDriverBindingStart (
}
DumpCapabilityReg (Slot, >Capability[Slot]);
-Support64BitDma &= Private->Capability[Slot].SysBus64;
+Support64BitDma = (Private->Capability[Slot].SysBus64V3 |
+ Private->Capability[Slot].SysBus64V4);
Status = SdMmcHcGetMaxCurrent (PciIo, Slot, >MaxCurrent[Slot]);
if (EFI_ERROR (Status)) {
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
index bedc968..e506875 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
@@ -45,7 +45,8 @@ DumpCapabilityReg (
DEBUG ((DEBUG_INFO, " Voltage 3.3 %a\n", Capability->Voltage33 ?
"TRUE" : "FALSE"));
DEBUG ((DEBUG_INFO, " Voltage 3.0 %a\n", Capability->Voltage30 ?
"TRUE" : "FALSE"));
DEBUG ((DEBUG_INFO, " Voltage 1.8 %a\n", Capability->Voltage18 ?
"TRUE" : "FALSE"));
- DEBUG ((DEBUG_INFO, " 64-bit Sys Bus%a\n", Capability->SysBus64 ?
"TRUE" : "FALSE"));
+ DEBUG ((DEBUG_INFO, " V4 64-bit Sys Bus %a\n", Capability->SysBus64V4 ?
"TRUE" : "FALSE"));
+ DEBUG ((DEBUG_INFO, " V3 64-bit Sys Bus %a\n", Capability->SysBus64V3 ?
"TRUE" : "FALSE"));
DEBUG ((DEBUG_INFO, " Async Interrupt %a\n", Capability->AsyncInt ?
"TRUE" : "FALSE"));
DEBUG ((DEBUG_INFO, " SlotType "));
if (Capability->SlotType == 0x00) {
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
index 7e3f588..cc138fc 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
@@ -114,24 +114,24 @@ typedef struct {
UINT32 Voltage33:1; // bit 24
UINT32 Voltage30:1; // bit 25
UINT32 Voltage18:1; // bit 26
- UINT32 Reserved3:1; // bit 27
- UINT32 SysBus64:1;// bit 28
+ UINT32 SysBus64V4:1; // bit 27
+ UINT32 SysBus64V3:1; // bit 28
UINT32 AsyncInt:1;// bit 29
UINT32 SlotType:2;// bit 30:31
UINT32 Sdr50:1; // bit 32
UINT32 Sdr104:1; // bit 33
UINT32 Ddr50:1; // bit 34
- UINT32 Reserved4:1; // bit 35
+ UINT32 Reserved3:1; // bit 35
UINT32 DriverTypeA:1; // bit 36
UINT32 DriverTypeC:1; // bit 37
UINT32 DriverTypeD:1; // bit 38
UINT32 DriverType4:1; // bit 39
UINT32 TimerCount:4; // bit 40:43
- UINT32 Reserved5:1; // bit 44
+ UINT32 Reserved4:1; // bit 44
UINT32 TuningSDR50:1; // bit 45
UINT32 RetuningMod:2; // bit 46:47
UINT32 ClkMultiplier:8; // bit 48:55
- UINT32 Reserved6:7; // bit 56:62
+ UINT32 Reserved5:7; // bit 56:62
UINT32 Hs400:1; // bit 63
} SD_MMC_HC_SLOT_CAP;
--
2.7.4
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
