public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH v5] EmbeddedPkg/MmcDxe: Align the ExtCSD buffer
@ 2017-07-04 15:43 Jun Nie
  2017-07-05 15:18 ` Leif Lindholm
  0 siblings, 1 reply; 2+ messages in thread
From: Jun Nie @ 2017-07-04 15: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.c               |  3 +++
 EmbeddedPkg/Universal/MmcDxe/Mmc.h               |  2 +-
 EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c | 19 ++++++++++++++-----
 3 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/EmbeddedPkg/Universal/MmcDxe/Mmc.c b/EmbeddedPkg/Universal/MmcDxe/Mmc.c
index 30268e3..ce27150 100644
--- a/EmbeddedPkg/Universal/MmcDxe/Mmc.c
+++ b/EmbeddedPkg/Universal/MmcDxe/Mmc.c
@@ -177,6 +177,9 @@ EFI_STATUS DestroyMmcHostInstance (
   if (MmcHostInstance->BlockIo.Media) {
     FreePool(MmcHostInstance->BlockIo.Media);
   }
+  if (MmcHostInstance->CardInfo.ECSDData) {
+    FreePages (MmcHostInstance->CardInfo.ECSDData, EFI_SIZE_TO_PAGES (sizeof (ECSD)));
+  }
   FreePool (MmcHostInstance);
 
   return Status;
diff --git a/EmbeddedPkg/Universal/MmcDxe/Mmc.h b/EmbeddedPkg/Universal/MmcDxe/Mmc.h
index b7b9454..eb4652b 100644
--- a/EmbeddedPkg/Universal/MmcDxe/Mmc.h
+++ b/EmbeddedPkg/Universal/MmcDxe/Mmc.h
@@ -320,7 +320,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 v5] EmbeddedPkg/MmcDxe: Align the ExtCSD buffer
  2017-07-04 15:43 [PATCH v5] EmbeddedPkg/MmcDxe: Align the ExtCSD buffer Jun Nie
@ 2017-07-05 15:18 ` Leif Lindholm
  0 siblings, 0 replies; 2+ messages in thread
From: Leif Lindholm @ 2017-07-05 15:18 UTC (permalink / raw)
  To: Jun Nie; +Cc: ard.biesheuvel, haojian.zhuang, edk2-devel, shawn.guo, jason.liu

On Tue, Jul 04, 2017 at 11:43:16PM +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>

Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
Pushed as 7bb5fad566.

> ---
>  EmbeddedPkg/Universal/MmcDxe/Mmc.c               |  3 +++
>  EmbeddedPkg/Universal/MmcDxe/Mmc.h               |  2 +-
>  EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c | 19 ++++++++++++++-----
>  3 files changed, 18 insertions(+), 6 deletions(-)
> 
> diff --git a/EmbeddedPkg/Universal/MmcDxe/Mmc.c b/EmbeddedPkg/Universal/MmcDxe/Mmc.c
> index 30268e3..ce27150 100644
> --- a/EmbeddedPkg/Universal/MmcDxe/Mmc.c
> +++ b/EmbeddedPkg/Universal/MmcDxe/Mmc.c
> @@ -177,6 +177,9 @@ EFI_STATUS DestroyMmcHostInstance (
>    if (MmcHostInstance->BlockIo.Media) {
>      FreePool(MmcHostInstance->BlockIo.Media);
>    }
> +  if (MmcHostInstance->CardInfo.ECSDData) {
> +    FreePages (MmcHostInstance->CardInfo.ECSDData, EFI_SIZE_TO_PAGES (sizeof (ECSD)));
> +  }
>    FreePool (MmcHostInstance);
>  
>    return Status;
> diff --git a/EmbeddedPkg/Universal/MmcDxe/Mmc.h b/EmbeddedPkg/Universal/MmcDxe/Mmc.h
> index b7b9454..eb4652b 100644
> --- a/EmbeddedPkg/Universal/MmcDxe/Mmc.h
> +++ b/EmbeddedPkg/Universal/MmcDxe/Mmc.h
> @@ -320,7 +320,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	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2017-07-05 15:16 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-07-04 15:43 [PATCH v5] EmbeddedPkg/MmcDxe: Align the ExtCSD buffer Jun Nie
2017-07-05 15:18 ` Leif Lindholm

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox