* [PATCH v4] EmbeddedPkg/MmcDxe: Align the ExtCSD buffer
@ 2017-06-30 2:43 Jun Nie
2017-07-03 16:45 ` Leif Lindholm
0 siblings, 1 reply; 2+ messages in thread
From: Jun Nie @ 2017-06-30 2:43 UTC (permalink / raw)
To: ard.biesheuvel, leif.lindholm, haojian.zhuang, edk2-devel
Cc: shawn.guo, jason.liu, Jun Nie
ExtCSD structure may be read via DMA. So align it to
page to avoid data corruption.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jun Nie <jun.nie@linaro.org>
---
EmbeddedPkg/Universal/MmcDxe/Mmc.h | 2 +-
EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c | 19 ++++++++++++++-----
2 files changed, 15 insertions(+), 6 deletions(-)
diff --git a/EmbeddedPkg/Universal/MmcDxe/Mmc.h b/EmbeddedPkg/Universal/MmcDxe/Mmc.h
index 8a7d5a3..f3e56ff 100644
--- a/EmbeddedPkg/Universal/MmcDxe/Mmc.h
+++ b/EmbeddedPkg/Universal/MmcDxe/Mmc.h
@@ -319,7 +319,7 @@ typedef struct {
OCR OCRData;
CID CIDData;
CSD CSDData;
- ECSD ECSDData; // MMC V4 extended card specific
+ ECSD *ECSDData; // MMC V4 extended card specific
} CARD_INFO;
typedef struct _MMC_HOST_INSTANCE {
diff --git a/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c b/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c
index c28207e..7f74c54 100644
--- a/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c
+++ b/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c
@@ -13,6 +13,7 @@
**/
#include <Library/BaseMemoryLib.h>
+#include <Library/MemoryAllocationLib.h>
#include <Library/TimerLib.h>
#include "Mmc.h"
@@ -210,15 +211,19 @@ EmmcIdentificationMode (
}
// Fetch ECSD
+ MmcHostInstance->CardInfo.ECSDData = AllocatePages (EFI_SIZE_TO_PAGES (sizeof (ECSD)));
+ if (MmcHostInstance->CardInfo.ECSDData == NULL) {
+ return EFI_OUT_OF_RESOURCES;
+ }
Status = Host->SendCommand (Host, MMC_CMD8, 0);
if (EFI_ERROR (Status)) {
DEBUG ((EFI_D_ERROR, "EmmcIdentificationMode(): ECSD fetch error, Status=%r.\n", Status));
}
- Status = Host->ReadBlockData (Host, 0, 512, (UINT32 *)&(MmcHostInstance->CardInfo.ECSDData));
+ Status = Host->ReadBlockData (Host, 0, 512, (UINT32 *)MmcHostInstance->CardInfo.ECSDData);
if (EFI_ERROR (Status)) {
DEBUG ((EFI_D_ERROR, "EmmcIdentificationMode(): ECSD read error, Status=%r.\n", Status));
- return Status;
+ goto FreePageExit;
}
// Make sure device exiting data mode
@@ -226,7 +231,7 @@ EmmcIdentificationMode (
Status = EmmcGetDeviceState (MmcHostInstance, &State);
if (EFI_ERROR (Status)) {
DEBUG ((EFI_D_ERROR, "EmmcIdentificationMode(): Failed to get device state, Status=%r.\n", Status));
- return Status;
+ goto FreePageExit;
}
} while (State == EMMC_DATA_STATE);
@@ -237,12 +242,16 @@ EmmcIdentificationMode (
Media->LogicalBlocksPerPhysicalBlock = 1;
Media->IoAlign = 4;
// Compute last block using bits [215:212] of the ECSD
- Media->LastBlock = MmcHostInstance->CardInfo.ECSDData.SECTOR_COUNT - 1; // eMMC isn't supposed to report this for
+ Media->LastBlock = MmcHostInstance->CardInfo.ECSDData->SECTOR_COUNT - 1; // eMMC isn't supposed to report this for
// Cards <2GB in size, but the model does.
// Setup card type
MmcHostInstance->CardInfo.CardType = EMMC_CARD;
return EFI_SUCCESS;
+
+FreePageExit:
+ FreePages (MmcHostInstance->CardInfo.ECSDData, EFI_SIZE_TO_PAGES (sizeof (ECSD)));
+ return Status;
}
STATIC
@@ -258,7 +267,7 @@ InitializeEmmcDevice (
UINT32 TimingMode[4] = {EMMCHS52DDR1V2, EMMCHS52DDR1V8, EMMCHS52, EMMCHS26};
Host = MmcHostInstance->MmcHost;
- ECSDData = &MmcHostInstance->CardInfo.ECSDData;
+ ECSDData = MmcHostInstance->CardInfo.ECSDData;
if (ECSDData->DEVICE_TYPE == EMMCBACKWARD)
return EFI_SUCCESS;
--
1.9.1
^ permalink raw reply related [flat|nested] 2+ messages in thread* Re: [PATCH v4] EmbeddedPkg/MmcDxe: Align the ExtCSD buffer
2017-06-30 2:43 [PATCH v4] EmbeddedPkg/MmcDxe: Align the ExtCSD buffer Jun Nie
@ 2017-07-03 16:45 ` Leif Lindholm
0 siblings, 0 replies; 2+ messages in thread
From: Leif Lindholm @ 2017-07-03 16:45 UTC (permalink / raw)
To: Jun Nie; +Cc: ard.biesheuvel, haojian.zhuang, edk2-devel, shawn.guo, jason.liu
On Fri, Jun 30, 2017 at 10:43:00AM +0800, Jun Nie wrote:
> ExtCSD structure may be read via DMA. So align it to
> page to avoid data corruption.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jun Nie <jun.nie@linaro.org>
> ---
> EmbeddedPkg/Universal/MmcDxe/Mmc.h | 2 +-
> EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c | 19 ++++++++++++++-----
> 2 files changed, 15 insertions(+), 6 deletions(-)
>
> diff --git a/EmbeddedPkg/Universal/MmcDxe/Mmc.h b/EmbeddedPkg/Universal/MmcDxe/Mmc.h
> index 8a7d5a3..f3e56ff 100644
> --- a/EmbeddedPkg/Universal/MmcDxe/Mmc.h
> +++ b/EmbeddedPkg/Universal/MmcDxe/Mmc.h
> @@ -319,7 +319,7 @@ typedef struct {
> OCR OCRData;
> CID CIDData;
> CSD CSDData;
> - ECSD ECSDData; // MMC V4 extended card specific
> + ECSD *ECSDData; // MMC V4 extended card specific
> } CARD_INFO;
>
> typedef struct _MMC_HOST_INSTANCE {
> diff --git a/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c b/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c
> index c28207e..7f74c54 100644
> --- a/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c
> +++ b/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c
> @@ -13,6 +13,7 @@
> **/
>
> #include <Library/BaseMemoryLib.h>
> +#include <Library/MemoryAllocationLib.h>
> #include <Library/TimerLib.h>
>
> #include "Mmc.h"
> @@ -210,15 +211,19 @@ EmmcIdentificationMode (
> }
>
> // Fetch ECSD
> + MmcHostInstance->CardInfo.ECSDData = AllocatePages (EFI_SIZE_TO_PAGES (sizeof (ECSD)));
> + if (MmcHostInstance->CardInfo.ECSDData == NULL) {
> + return EFI_OUT_OF_RESOURCES;
> + }
> Status = Host->SendCommand (Host, MMC_CMD8, 0);
> if (EFI_ERROR (Status)) {
> DEBUG ((EFI_D_ERROR, "EmmcIdentificationMode(): ECSD fetch error, Status=%r.\n", Status));
> }
>
> - Status = Host->ReadBlockData (Host, 0, 512, (UINT32 *)&(MmcHostInstance->CardInfo.ECSDData));
> + Status = Host->ReadBlockData (Host, 0, 512, (UINT32 *)MmcHostInstance->CardInfo.ECSDData);
> if (EFI_ERROR (Status)) {
> DEBUG ((EFI_D_ERROR, "EmmcIdentificationMode(): ECSD read error, Status=%r.\n", Status));
> - return Status;
> + goto FreePageExit;
> }
>
> // Make sure device exiting data mode
> @@ -226,7 +231,7 @@ EmmcIdentificationMode (
> Status = EmmcGetDeviceState (MmcHostInstance, &State);
> if (EFI_ERROR (Status)) {
> DEBUG ((EFI_D_ERROR, "EmmcIdentificationMode(): Failed to get device state, Status=%r.\n", Status));
> - return Status;
> + goto FreePageExit;
> }
> } while (State == EMMC_DATA_STATE);
>
> @@ -237,12 +242,16 @@ EmmcIdentificationMode (
> Media->LogicalBlocksPerPhysicalBlock = 1;
> Media->IoAlign = 4;
> // Compute last block using bits [215:212] of the ECSD
> - Media->LastBlock = MmcHostInstance->CardInfo.ECSDData.SECTOR_COUNT - 1; // eMMC isn't supposed to report this for
> + Media->LastBlock = MmcHostInstance->CardInfo.ECSDData->SECTOR_COUNT - 1; // eMMC isn't supposed to report this for
> // Cards <2GB in size, but the model does.
>
> // Setup card type
> MmcHostInstance->CardInfo.CardType = EMMC_CARD;
> return EFI_SUCCESS;
> +
> +FreePageExit:
> + FreePages (MmcHostInstance->CardInfo.ECSDData, EFI_SIZE_TO_PAGES (sizeof (ECSD)));
So, this memory is now being freed when the operation fails.
But it also needs to be cleaned up when the host is deleted.
I suggested adding the free operation to DestroyMmcHostInstance.
/
Leif
> + return Status;
> }
>
> STATIC
> @@ -258,7 +267,7 @@ InitializeEmmcDevice (
> UINT32 TimingMode[4] = {EMMCHS52DDR1V2, EMMCHS52DDR1V8, EMMCHS52, EMMCHS26};
>
> Host = MmcHostInstance->MmcHost;
> - ECSDData = &MmcHostInstance->CardInfo.ECSDData;
> + ECSDData = MmcHostInstance->CardInfo.ECSDData;
> if (ECSDData->DEVICE_TYPE == EMMCBACKWARD)
> return EFI_SUCCESS;
>
> --
> 1.9.1
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2017-07-03 16:44 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-06-30 2:43 [PATCH v4] EmbeddedPkg/MmcDxe: Align the ExtCSD buffer Jun Nie
2017-07-03 16:45 ` Leif Lindholm
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox