public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Ni, Ray" <ray.ni@intel.com>
To: "Wu, Hao A" <hao.a.wu@intel.com>,
	"edk2-devel@lists.01.org" <edk2-devel@lists.01.org>
Subject: Re: [PATCH v2 05/12] MdeModulePkg/NvmExpressPei: Avoid updating the module-level variable
Date: Thu, 31 Jan 2019 03:28:17 +0000	[thread overview]
Message-ID: <734D49CCEBEEF84792F5B80ED585239D5BFFFF6B@SHSMSX104.ccr.corp.intel.com> (raw)
In-Reply-To: <20190131024854.4880-6-hao.a.wu@intel.com>

Reviewed-by: Ray Ni <ray.ni@intel.com>

> -----Original Message-----
> From: Wu, Hao A <hao.a.wu@intel.com>
> Sent: Thursday, January 31, 2019 10:49 AM
> To: edk2-devel@lists.01.org
> Cc: Wu, Hao A <hao.a.wu@intel.com>; Wang, Jian J <jian.j.wang@intel.com>;
> Ni, Ray <ray.ni@intel.com>
> Subject: [PATCH v2 05/12] MdeModulePkg/NvmExpressPei: Avoid updating
> the module-level variable
> 
> This commit is out of the scope for BZ-1409. The commit will remove the call
> of RegisterForShadow() at the entry point of the driver. By doing so, the
> driver is now possible to be executed without being re-loaded into
> permanent memory.
> 
> Thus, this commit will update the NvmExpressPei driver to avoid updating
> the content of a global variable.
> 
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Hao Wu <hao.a.wu@intel.com>
> ---
>  MdeModulePkg/Bus/Pci/NvmExpressPei/NvmExpressPei.h |  12 +-
>  MdeModulePkg/Bus/Pci/NvmExpressPei/DmaMem.c        | 153
> +++++++++++---------
>  MdeModulePkg/Bus/Pci/NvmExpressPei/NvmExpressPei.c |  11 +-
>  3 files changed, 92 insertions(+), 84 deletions(-)
> 
> diff --git a/MdeModulePkg/Bus/Pci/NvmExpressPei/NvmExpressPei.h
> b/MdeModulePkg/Bus/Pci/NvmExpressPei/NvmExpressPei.h
> index 0bd62c2459..0135eca6f0 100644
> --- a/MdeModulePkg/Bus/Pci/NvmExpressPei/NvmExpressPei.h
> +++ b/MdeModulePkg/Bus/Pci/NvmExpressPei/NvmExpressPei.h
> @@ -2,7 +2,7 @@
>    The NvmExpressPei driver is used to manage non-volatile memory
> subsystem
>    which follows NVM Express specification at PEI phase.
> 
> -  Copyright (c) 2018, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2018 - 2019, Intel Corporation. All rights
> + reserved.<BR>
> 
>    This program and the accompanying materials
>    are licensed and made available under the terms and conditions @@ -
> 147,13 +147,9 @@ struct _PEI_NVME_CONTROLLER_PRIVATE_DATA {
>    CR (a, PEI_NVME_CONTROLLER_PRIVATE_DATA, EndOfPeiNotifyList,
> NVME_PEI_CONTROLLER_PRIVATE_DATA_SIGNATURE)
> 
> 
> -/**
> -  Initialize IOMMU.
> -**/
> -VOID
> -IoMmuInit (
> -  VOID
> -  );
> +//
> +// Internal functions
> +//
> 
>  /**
>    Allocates pages that are suitable for an OperationBusMasterCommonBuffer
> or diff --git a/MdeModulePkg/Bus/Pci/NvmExpressPei/DmaMem.c
> b/MdeModulePkg/Bus/Pci/NvmExpressPei/DmaMem.c
> index 51b48d38dd..cb629c16b0 100644
> --- a/MdeModulePkg/Bus/Pci/NvmExpressPei/DmaMem.c
> +++ b/MdeModulePkg/Bus/Pci/NvmExpressPei/DmaMem.c
> @@ -1,7 +1,7 @@
>  /** @file
>    The DMA memory help function.
> 
> -  Copyright (c) 2018, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2018 - 2019, Intel Corporation. All rights
> + reserved.<BR>
> 
>    This program and the accompanying materials
>    are licensed and made available under the terms and conditions @@ -16,7
> +16,33 @@
> 
>  #include "NvmExpressPei.h"
> 
> -EDKII_IOMMU_PPI  *mIoMmu;
> +/**
> +  Get IOMMU PPI.
> +
> +  @return Pointer to IOMMU PPI.
> +
> +**/
> +EDKII_IOMMU_PPI *
> +GetIoMmu (
> +  VOID
> +  )
> +{
> +  EFI_STATUS         Status;
> +  EDKII_IOMMU_PPI    *IoMmu;
> +
> +  IoMmu  = NULL;
> +  Status = PeiServicesLocatePpi (
> +             &gEdkiiIoMmuPpiGuid,
> +             0,
> +             NULL,
> +             (VOID **) &IoMmu
> +             );
> +  if (!EFI_ERROR (Status) && (IoMmu != NULL)) {
> +    return IoMmu;
> +  }
> +
> +  return NULL;
> +}
> 
>  /**
>    Provides the controller-specific addresses required to access system
> memory from a @@ -46,18 +72,21 @@ IoMmuMap (
>    OUT VOID                  **Mapping
>    )
>  {
> -  EFI_STATUS  Status;
> -  UINT64      Attribute;
> -
> -  if (mIoMmu != NULL) {
> -    Status = mIoMmu->Map (
> -                       mIoMmu,
> -                       Operation,
> -                       HostAddress,
> -                       NumberOfBytes,
> -                       DeviceAddress,
> -                       Mapping
> -                       );
> +  EFI_STATUS         Status;
> +  UINT64             Attribute;
> +  EDKII_IOMMU_PPI    *IoMmu;
> +
> +  IoMmu = GetIoMmu ();
> +
> +  if (IoMmu != NULL) {
> +    Status = IoMmu->Map (
> +                     IoMmu,
> +                     Operation,
> +                     HostAddress,
> +                     NumberOfBytes,
> +                     DeviceAddress,
> +                     Mapping
> +                     );
>      if (EFI_ERROR (Status)) {
>        return EFI_OUT_OF_RESOURCES;
>      }
> @@ -78,11 +107,11 @@ IoMmuMap (
>        ASSERT(FALSE);
>        return EFI_INVALID_PARAMETER;
>      }
> -    Status = mIoMmu->SetAttribute (
> -                       mIoMmu,
> -                       *Mapping,
> -                       Attribute
> -                       );
> +    Status = IoMmu->SetAttribute (
> +                      IoMmu,
> +                      *Mapping,
> +                      Attribute
> +                      );
>      if (EFI_ERROR (Status)) {
>        return Status;
>      }
> @@ -108,11 +137,14 @@ IoMmuUnmap (
>    IN VOID                  *Mapping
>    )
>  {
> -  EFI_STATUS  Status;
> +  EFI_STATUS         Status;
> +  EDKII_IOMMU_PPI    *IoMmu;
> +
> +  IoMmu = GetIoMmu ();
> 
> -  if (mIoMmu != NULL) {
> -    Status = mIoMmu->SetAttribute (mIoMmu, Mapping, 0);
> -    Status = mIoMmu->Unmap (mIoMmu, Mapping);
> +  if (IoMmu != NULL) {
> +    Status = IoMmu->SetAttribute (IoMmu, Mapping, 0);
> +    Status = IoMmu->Unmap (IoMmu, Mapping);
>    } else {
>      Status = EFI_SUCCESS;
>    }
> @@ -148,39 +180,42 @@ IoMmuAllocateBuffer (
>    EFI_STATUS            Status;
>    UINTN                 NumberOfBytes;
>    EFI_PHYSICAL_ADDRESS  HostPhyAddress;
> +  EDKII_IOMMU_PPI       *IoMmu;
> 
>    *HostAddress = NULL;
>    *DeviceAddress = 0;
> 
> -  if (mIoMmu != NULL) {
> -    Status = mIoMmu->AllocateBuffer (
> -                       mIoMmu,
> -                       EfiBootServicesData,
> -                       Pages,
> -                       HostAddress,
> -                       0
> -                       );
> +  IoMmu = GetIoMmu ();
> +
> +  if (IoMmu != NULL) {
> +    Status = IoMmu->AllocateBuffer (
> +                      IoMmu,
> +                      EfiBootServicesData,
> +                      Pages,
> +                      HostAddress,
> +                      0
> +                      );
>      if (EFI_ERROR (Status)) {
>        return EFI_OUT_OF_RESOURCES;
>      }
> 
>      NumberOfBytes = EFI_PAGES_TO_SIZE(Pages);
> -    Status = mIoMmu->Map (
> -                       mIoMmu,
> -                       EdkiiIoMmuOperationBusMasterCommonBuffer,
> -                       *HostAddress,
> -                       &NumberOfBytes,
> -                       DeviceAddress,
> -                       Mapping
> -                       );
> +    Status = IoMmu->Map (
> +                      IoMmu,
> +                      EdkiiIoMmuOperationBusMasterCommonBuffer,
> +                      *HostAddress,
> +                      &NumberOfBytes,
> +                      DeviceAddress,
> +                      Mapping
> +                      );
>      if (EFI_ERROR (Status)) {
>        return EFI_OUT_OF_RESOURCES;
>      }
> -    Status = mIoMmu->SetAttribute (
> -                       mIoMmu,
> -                       *Mapping,
> -                       EDKII_IOMMU_ACCESS_READ | EDKII_IOMMU_ACCESS_WRITE
> -                       );
> +    Status = IoMmu->SetAttribute (
> +                      IoMmu,
> +                      *Mapping,
> +                      EDKII_IOMMU_ACCESS_READ | EDKII_IOMMU_ACCESS_WRITE
> +                      );
>      if (EFI_ERROR (Status)) {
>        return Status;
>      }
> @@ -219,31 +254,17 @@ IoMmuFreeBuffer (
>    IN VOID                   *Mapping
>    )
>  {
> -  EFI_STATUS  Status;
> +  EFI_STATUS         Status;
> +  EDKII_IOMMU_PPI    *IoMmu;
> 
> -  if (mIoMmu != NULL) {
> -    Status = mIoMmu->SetAttribute (mIoMmu, Mapping, 0);
> -    Status = mIoMmu->Unmap (mIoMmu, Mapping);
> -    Status = mIoMmu->FreeBuffer (mIoMmu, Pages, HostAddress);
> +  IoMmu = GetIoMmu ();
> +
> +  if (IoMmu != NULL) {
> +    Status = IoMmu->SetAttribute (IoMmu, Mapping, 0);
> +    Status = IoMmu->Unmap (IoMmu, Mapping);
> +    Status = IoMmu->FreeBuffer (IoMmu, Pages, HostAddress);
>    } else {
>      Status = EFI_SUCCESS;
>    }
>    return Status;
>  }
> -
> -/**
> -  Initialize IOMMU.
> -**/
> -VOID
> -IoMmuInit (
> -  VOID
> -  )
> -{
> -  PeiServicesLocatePpi (
> -    &gEdkiiIoMmuPpiGuid,
> -    0,
> -    NULL,
> -    (VOID **)&mIoMmu
> -    );
> -}
> -
> diff --git a/MdeModulePkg/Bus/Pci/NvmExpressPei/NvmExpressPei.c
> b/MdeModulePkg/Bus/Pci/NvmExpressPei/NvmExpressPei.c
> index fabec37e36..2fe73e942c 100644
> --- a/MdeModulePkg/Bus/Pci/NvmExpressPei/NvmExpressPei.c
> +++ b/MdeModulePkg/Bus/Pci/NvmExpressPei/NvmExpressPei.c
> @@ -2,7 +2,7 @@
>    The NvmExpressPei driver is used to manage non-volatile memory
> subsystem
>    which follows NVM Express specification at PEI phase.
> 
> -  Copyright (c) 2018, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2018 - 2019, Intel Corporation. All rights
> + reserved.<BR>
> 
>    This program and the accompanying materials
>    are licensed and made available under the terms and conditions @@ -
> 215,13 +215,6 @@ NvmExpressPeimEntry (
>    EFI_PHYSICAL_ADDRESS                     DeviceAddress;
> 
>    //
> -  // Shadow this PEIM to run from memory
> -  //
> -  if (!EFI_ERROR (PeiServicesRegisterForShadow (FileHandle))) {
> -    return EFI_SUCCESS;
> -  }
> -
> -  //
>    // Locate the NVME host controller PPI
>    //
>    Status = PeiServicesLocatePpi (
> @@ -235,8 +228,6 @@ NvmExpressPeimEntry (
>      return EFI_UNSUPPORTED;
>    }
> 
> -  IoMmuInit ();
> -
>    Controller = 0;
>    MmioBase   = 0;
>    while (TRUE) {
> --
> 2.12.0.windows.1



  reply	other threads:[~2019-01-31  3:31 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-31  2:48 [PATCH v2 00/12] Split the S3 PEI phase HW init codes from Opal driver Hao Wu
2019-01-31  2:48 ` [PATCH v2 01/12] MdeModulePkg: Add definitions for ATA AHCI host controller PPI Hao Wu
2019-01-31  3:25   ` Ni, Ray
2019-01-31  2:48 ` [PATCH v2 02/12] MdeModulePkg: Add definitions for EDKII PEI ATA PassThru PPI Hao Wu
2019-01-31  3:22   ` Ni, Ray
2019-01-31  5:28     ` Wu, Hao A
2019-01-31  2:48 ` [PATCH v2 03/12] MdeModulePkg: Add definitions for Storage Security Command PPI Hao Wu
2019-01-31  3:26   ` Ni, Ray
2019-01-31  2:48 ` [PATCH v2 04/12] MdeModulePkg: Add GUID for LockBox to save storage dev to init in S3 Hao Wu
2019-01-31  3:27   ` Ni, Ray
2019-01-31  5:30     ` Wu, Hao A
2019-01-31  2:48 ` [PATCH v2 05/12] MdeModulePkg/NvmExpressPei: Avoid updating the module-level variable Hao Wu
2019-01-31  3:28   ` Ni, Ray [this message]
2019-01-31  2:48 ` [PATCH v2 06/12] MdeModulePkg/NvmExpressPei: Add logic to produce SSC PPI Hao Wu
2019-01-31  3:35   ` Ni, Ray
2019-01-31  5:40     ` Wu, Hao A
2019-01-31  2:48 ` [PATCH v2 07/12] MdeModulePkg/NvmExpressPei: Consume S3StorageDeviceInitList LockBox Hao Wu
2019-01-31  3:45   ` Ni, Ray
2019-01-31  5:45     ` Wu, Hao A
2019-01-31  2:48 ` [PATCH v2 08/12] MdeModulePkg/AhciPei: Add AHCI mode ATA device support in PEI Hao Wu
2019-01-31  5:49   ` Ni, Ruiyu
2019-01-31  2:48 ` [PATCH v2 09/12] MdeModulePkg/SmmLockBoxLib: Use 'DEBUG_' prefix instead of 'EFI_D_' Hao Wu
2019-01-31  5:49   ` Ni, Ruiyu
2019-01-31  2:48 ` [PATCH v2 10/12] MdeModulePkg/SmmLockBox(PEI): Remove an ASSERT in RestoreLockBox() Hao Wu
2019-01-31  5:50   ` Ni, Ruiyu
2019-01-31  5:53     ` Wu, Hao A
2019-01-31  2:48 ` [PATCH v2 11/12] MdeModulePkg/SmmLockBoxLib: Support LockBox enlarge in UpdateLockBox() Hao Wu
2019-01-31  6:00   ` Ni, Ruiyu
2019-01-31  2:48 ` [PATCH v2 12/12] SecurityPkg/OpalPassword: Remove HW init codes and consume SSC PPI Hao Wu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=734D49CCEBEEF84792F5B80ED585239D5BFFFF6B@SHSMSX104.ccr.corp.intel.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox