From: "Kubacki, Michael A" <michael.a.kubacki@intel.com>
To: "Desimone, Nathaniel L" <nathaniel.l.desimone@intel.com>,
"edk2-devel@lists.01.org" <edk2-devel@lists.01.org>
Cc: "Chiu, Chasel" <chasel.chiu@intel.com>,
"Gao, Liming" <liming.gao@intel.com>,
"Kinney, Michael D" <michael.d.kinney@intel.com>
Subject: Re: [edk2-platforms][PATCH v1 1/3] KabylakeSiliconPkg: Add SPI write support in PEI
Date: Mon, 1 Apr 2019 16:33:13 +0000 [thread overview]
Message-ID: <49AB4ACB9627B8468F29D589A27B745584531E54@ORSMSX121.amr.corp.intel.com> (raw)
In-Reply-To: <02A34F284D1DA44BB705E61F7180EF0AAE9B2D38@ORSMSX114.amr.corp.intel.com>
1. I missed in a local merge, will be fixed in v2.
2. You're correct that SpiProtocolConstructor ( ) is used in PEI+DXE+SMM. PeiSpiLib initializes the SPI BAR before calling SpiProtocolConstructor ( ).
> -----Original Message-----
> From: Desimone, Nathaniel L
> Sent: Sunday, March 31, 2019 10:37 PM
> To: Kubacki, Michael A <michael.a.kubacki@intel.com>; edk2-
> devel@lists.01.org
> Cc: Chiu, Chasel <chasel.chiu@intel.com>; Gao, Liming
> <liming.gao@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
> Subject: RE: [edk2-platforms][PATCH v1 1/3] KabylakeSiliconPkg: Add SPI write
> support in PEI
>
> 1. You are adding a redundant second copy of ASSERT (SpiInstance-
> >PchAcpiBase != 0); 2. Is SpiProtocolConstructor() really the best place to
> initialize the SPI BAR? I believe that function gets used in PEI + DXE + SMM. I
> suspect the new PeiSpiLib you are adding would be a better spot.
>
> Thanks,
> Nate
>
> -----Original Message-----
> From: Kubacki, Michael A
> Sent: Sunday, March 31, 2019 4:28 PM
> To: edk2-devel@lists.01.org
> Cc: Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; Chiu, Chasel
> <chasel.chiu@intel.com>; Gao, Liming <liming.gao@intel.com>; Kinney,
> Michael D <michael.d.kinney@intel.com>
> Subject: [edk2-platforms][PATCH v1 1/3] KabylakeSiliconPkg: Add SPI write
> support in PEI
>
> Adds a new library PeiSpiLib to perform the initialization necessary to perform
> SPI write cycles in PEI. After initialization, it installs an instance of the
> PCH_SPI_PPI.
>
> Cc: Nate DeSimone <mailto:nathaniel.l.desimone@intel.com>
> Cc: Chasel Chiu <mailto:chasel.chiu@intel.com>
> Cc: Liming Gao <mailto:liming.gao@intel.com>
> Cc: Michael D Kinney <mailto:michael.d.kinney@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Michael Kubacki <mailto:michael.a.kubacki@intel.com>
> ---
> Silicon/Intel/KabylakeSiliconPkg/SiPkgPeiLib.dsc | 3 +-
> .../Pch/Library/PeiSpiLib/PeiSpiLib.inf | 45 +++++
> .../BasePchSpiCommonLib/BasePchSpiCommonLib.inf | 6 +-
> .../Pch/Include/Library/SpiLib.h | 32 ++++
> .../Pch/Library/PeiSpiLib/PeiSpiLib.c | 211 +++++++++++++++++++++
> .../LibraryPrivate/BasePchSpiCommonLib/SpiCommon.c | 10 +-
> 6 files changed, 302 insertions(+), 5 deletions(-) create mode 100644
> Silicon/Intel/KabylakeSiliconPkg/Pch/Library/PeiSpiLib/PeiSpiLib.inf
> create mode 100644
> Silicon/Intel/KabylakeSiliconPkg/Pch/Include/Library/SpiLib.h
> create mode 100644
> Silicon/Intel/KabylakeSiliconPkg/Pch/Library/PeiSpiLib/PeiSpiLib.c
>
> diff --git a/Silicon/Intel/KabylakeSiliconPkg/SiPkgPeiLib.dsc
> b/Silicon/Intel/KabylakeSiliconPkg/SiPkgPeiLib.dsc
> index b81a736486..bb95ce3888 100644
> --- a/Silicon/Intel/KabylakeSiliconPkg/SiPkgPeiLib.dsc
> +++ b/Silicon/Intel/KabylakeSiliconPkg/SiPkgPeiLib.dsc
> @@ -1,7 +1,7 @@
> ## @file
> # Component description file for the SkyLake SiPkg PEI libraries.
> #
> -# Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
> +# Copyright (c) 2017 - 2019, Intel Corporation. All rights
> +reserved.<BR>
> #
> # This program and the accompanying materials are licensed and made
> available under # the terms and conditions of the BSD License which
> accompanies this distribution.
> @@ -30,6 +30,7 @@
> !endif
>
> ResetSystemLib|$(PLATFORM_SI_PACKAGE)/Pch/Library/PeiResetSystemLib/Pe
> iResetSystemLib.inf
>
> PchResetLib|$(PLATFORM_SI_PACKAGE)/Pch/Library/PeiPchResetLib/PeiPchRe
> setLib.inf
> + SpiLib|$(PLATFORM_SI_PACKAGE)/Pch/Library/PeiSpiLib/PeiSpiLib.inf
>
> #
> # Cpu
> diff --git a/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/PeiSpiLib/PeiSpiLib.inf
> b/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/PeiSpiLib/PeiSpiLib.inf
> new file mode 100644
> index 0000000000..6d2e70f012
> --- /dev/null
> +++ b/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/PeiSpiLib/PeiSpiLib.i
> +++ nf
> @@ -0,0 +1,45 @@
> +## @file
> +# Component description file for PEI PCH SPI Initialization # #
> +Copyright (c) 2019, Intel Corporation. All rights reserved.<BR> # #
> +This program and the accompanying materials are licensed and made
> +available under # the terms and conditions of the BSD License which
> accompanies this distribution.
> +# The full text of the license may be found at #
> +http://opensource.org/licenses/bsd-license.php
> +#
> +# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"
> BASIS,
> +# WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
> EXPRESS OR IMPLIED.
> +#
> +##
> +
> +[Defines]
> + INF_VERSION = 0x00010017
> + BASE_NAME = PeiSpiLib
> + FILE_GUID = 4998447D-7948-448F-AB75-96E24E18FF23
> + VERSION_STRING = 1.0
> + MODULE_TYPE = PEIM
> + LIBRARY_CLASS = SpiLib|PEIM PEI_CORE
> + #
> + # The following information is for reference only and not required by the
> build tools.
> + #
> + # VALID_ARCHITECTURES = IA32 X64 IPF
> + #
> +
> +[LibraryClasses]
> + DebugLib
> + MemoryAllocationLib
> + PchSpiCommonLib
> + PciSegmentLib
> + PeiServicesLib
> + PeiServicesTablePointerLib
> +
> +[Packages]
> + MdePkg/MdePkg.dec
> + KabylakeSiliconPkg/SiPkg.dec
> +
> +[Sources]
> + PeiSpiLib.c
> +
> +[Ppis]
> + gPchSpiPpiGuid ## PRODUCES
> diff --git
> a/Silicon/Intel/KabylakeSiliconPkg/Pch/LibraryPrivate/BasePchSpiCommonLib/
> BasePchSpiCommonLib.inf
> b/Silicon/Intel/KabylakeSiliconPkg/Pch/LibraryPrivate/BasePchSpiCommonLib/
> BasePchSpiCommonLib.inf
> index 128f7adcea..2c531e7816 100644
> ---
> a/Silicon/Intel/KabylakeSiliconPkg/Pch/LibraryPrivate/BasePchSpiCommonLib/
> BasePchSpiCommonLib.inf
> +++ b/Silicon/Intel/KabylakeSiliconPkg/Pch/LibraryPrivate/BasePchSpiComm
> +++ onLib/BasePchSpiCommonLib.inf
> @@ -1,7 +1,7 @@
> ## @file
> # Component description file for the PchSpiCommonLib # -# Copyright (c)
> 2017, Intel Corporation. All rights reserved.<BR>
> +# Copyright (c) 2017 - 2019, Intel Corporation. All rights
> +reserved.<BR>
> #
> # This program and the accompanying materials are licensed and made
> available under # the terms and conditions of the BSD License which
> accompanies this distribution.
> @@ -32,4 +32,8 @@
> [LibraryClasses]
> IoLib
> DebugLib
> + PcdLib
> PchCycleDecodingLib
> +
> +[Pcd]
> + gSiPkgTokenSpaceGuid.PcdAcpiBaseAddress ## CONSUMES
> diff --git a/Silicon/Intel/KabylakeSiliconPkg/Pch/Include/Library/SpiLib.h
> b/Silicon/Intel/KabylakeSiliconPkg/Pch/Include/Library/SpiLib.h
> new file mode 100644
> index 0000000000..6af66f8869
> --- /dev/null
> +++ b/Silicon/Intel/KabylakeSiliconPkg/Pch/Include/Library/SpiLib.h
> @@ -0,0 +1,32 @@
> +/** @file
> + Library to initialize SPI services for future SPI accesses.
> +
> +Copyright (c) 2019, Intel Corporation. All rights reserved.<BR> This
> +program and the accompanying materials are licensed and made available
> +under the terms and conditions of the BSD License that accompanies this
> distribution.
> +The full text of the license may be found at
> +http://opensource.org/licenses/bsd-license.php.
> +
> +THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS
> OR IMPLIED.
> +
> +**/
> +
> +#ifndef _SPI_LIB_H_
> +#define _SPI_LIB_H_
> +
> +/**
> + Initializes SPI for access from future services.
> +
> + @retval EFI_SUCCESS The SPI service was initialized successfully.
> + @retval EFI_OUT_OF_RESOUCES Insufficient memory available to allocate
> structures required for initialization.
> + @retval Others An error occurred initializing SPI services.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +SpiServiceInit (
> + VOID
> + );
> +
> +#endif
> diff --git a/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/PeiSpiLib/PeiSpiLib.c
> b/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/PeiSpiLib/PeiSpiLib.c
> new file mode 100644
> index 0000000000..712354af07
> --- /dev/null
> +++ b/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/PeiSpiLib/PeiSpiLib.c
> @@ -0,0 +1,211 @@
> +/** @file
> + PCH SPI PEI Library implements the SPI Host Controller Interface.
> +
> +Copyright (c) 2019, Intel Corporation. All rights reserved.<BR> This
> +program and the accompanying materials are licensed and made available
> +under the terms and conditions of the BSD License that accompanies this
> distribution.
> +The full text of the license may be found at
> +http://opensource.org/licenses/bsd-license.php.
> +
> +THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS
> OR IMPLIED.
> +
> +**/
> +
> +#include <PchReservedResources.h>
> +#include <IndustryStandard/Pci30.h>
> +#include <Ppi/Spi.h>
> +#include <Register/PchRegsLpc.h>
> +#include <Register/PchRegsSpi.h>
> +#include <Library/DebugLib.h>
> +#include <Library/IoLib.h>
> +#include <Library/MemoryAllocationLib.h> #include
> +<Library/PchSpiCommonLib.h> #include <Library/PciSegmentLib.h> #include
> +<Library/PeiServicesLib.h>
> +
> +typedef struct {
> + EFI_PEI_PPI_DESCRIPTOR PpiDescriptor;
> + SPI_INSTANCE SpiInstance;
> +} PEI_SPI_INSTANCE;
> +
> +/**
> + Initializes the SPI BAR0 value to a default value and enables memory space
> decoding.
> +
> + The SPI BAR0 will be assigned later in PCI enumeration.
> +
> +**/
> +VOID
> +InitSpiBar0 (
> + VOID
> + )
> +{
> + UINT64 PchSpiBase;
> + PchSpiBase = PCI_SEGMENT_LIB_ADDRESS (
> + 0,
> + 0,
> + PCI_DEVICE_NUMBER_PCH_SPI,
> + PCI_FUNCTION_NUMBER_PCH_SPI,
> + 0
> + );
> + PciSegmentWrite32 (PchSpiBase + R_PCH_SPI_BAR0,
> +PCH_SPI_BASE_ADDRESS);
> + PciSegmentOr32 (PchSpiBase + PCI_COMMAND_OFFSET,
> +EFI_PCI_COMMAND_MEMORY_SPACE); }
> +
> +/**
> + Initializes SPI for access from future services.
> +
> + @retval EFI_SUCCESS The SPI service was initialized successfully.
> + @retval EFI_OUT_OF_RESOUCES Insufficient memory available to allocate
> structures required for initialization.
> + @retval Others An error occurred initializing SPI services.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +SpiServiceInit (
> + VOID
> + )
> +{
> + EFI_STATUS Status;
> + PEI_SPI_INSTANCE *PeiSpiInstance;
> + SPI_INSTANCE *SpiInstance;
> + PCH_SPI_PPI *SpiPpi;
> +
> + Status = PeiServicesLocatePpi (
> + &gPchSpiPpiGuid,
> + 0,
> + NULL,
> + (VOID **) &SpiPpi
> + );
> +
> + if (Status != EFI_SUCCESS) {
> + //
> + // Prior to PCI enumeration, initialize SPI BAR0 to a default value
> + // and also enable memory space decoding for SPI
> + //
> + InitSpiBar0 ();
> +
> + PeiSpiInstance = (PEI_SPI_INSTANCE *) AllocateZeroPool (sizeof
> (PEI_SPI_INSTANCE));
> + if (NULL == PeiSpiInstance) {
> + return EFI_OUT_OF_RESOURCES;
> + }
> +
> + SpiInstance = &(PeiSpiInstance->SpiInstance);
> + SpiProtocolConstructor (SpiInstance);
> +
> + PeiSpiInstance->PpiDescriptor.Flags = EFI_PEI_PPI_DESCRIPTOR_PPI |
> EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST;
> + PeiSpiInstance->PpiDescriptor.Guid = &gPchSpiPpiGuid;
> + PeiSpiInstance->PpiDescriptor.Ppi = &(SpiInstance->SpiProtocol);
> +
> + Status = PeiServicesInstallPpi (&PeiSpiInstance->PpiDescriptor);
> + }
> + return Status;
> +}
> +
> +/**
> + Acquires the PCH SPI BAR0 MMIO address.
> +
> + @param[in] SpiInstance Pointer to SpiInstance to initialize
> +
> + @retval UINTN The SPIO BAR0 MMIO address
> +
> +**/
> +UINTN
> +AcquireSpiBar0 (
> + IN SPI_INSTANCE *SpiInstance
> + )
> +{
> + return MmioRead32 (SpiInstance->PchSpiBase + R_PCH_SPI_BAR0) &
> +~(B_PCH_SPI_BAR0_MASK); }
> +
> +/**
> + Release the PCH SPI BAR0 MMIO address.
> +
> + @param[in] SpiInstance Pointer to SpiInstance to initialize
> +
> + @retval None
> +**/
> +VOID
> +ReleaseSpiBar0 (
> + IN SPI_INSTANCE *SpiInstance
> + )
> +{
> + return;
> +}
> +
> +/**
> + Disables BIOS Write Protect
> +
> + @retval EFI_SUCCESS BIOS Write Protect was disabled successfully
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +DisableBiosWriteProtect (
> + VOID
> + )
> +{
> + UINT64 SpiBaseAddress;
> +
> + SpiBaseAddress = PCI_SEGMENT_LIB_ADDRESS (
> + 0,
> + 0,
> + PCI_DEVICE_NUMBER_PCH_SPI,
> + PCI_FUNCTION_NUMBER_PCH_SPI,
> + 0
> + );
> + //
> + // Clear EISS bit to allow for SPI use //
> + PciSegmentAnd8 (SpiBaseAddress + R_PCH_SPI_BC, (UINT8)
> + ~B_PCH_SPI_BC_EISS);
> +
> + //
> + // Write clear BC_SYNC_SS prior to change WPD from 0 to 1.
> + //
> + PciSegmentOr8 (
> + SpiBaseAddress + R_PCH_SPI_BC + 1,
> + (B_PCH_SPI_BC_SYNC_SS >> 8)
> + );
> +
> + //
> + // Set BIOSWE bit (SPI PCI Offset DCh [0]) = 1b // Enable the access
> + to the BIOS space for both read and write cycles //
> + PciSegmentOr8 (
> + SpiBaseAddress + R_PCH_SPI_BC,
> + B_PCH_SPI_BC_WPD
> + );
> +
> + ASSERT ((PciSegmentRead8 (SpiBaseAddress + R_PCH_SPI_BC) &
> + B_PCH_SPI_BC_EISS) != 0);
> +
> + return EFI_SUCCESS;
> +}
> +
> +/**
> + Enables BIOS Write Protect
> +
> +**/
> +VOID
> +EFIAPI
> +EnableBiosWriteProtect (
> + VOID
> + )
> +{
> + UINT64 SpiBaseAddress;
> +
> + SpiBaseAddress = PCI_SEGMENT_LIB_ADDRESS (
> + 0,
> + 0,
> + PCI_DEVICE_NUMBER_PCH_SPI,
> + PCI_FUNCTION_NUMBER_PCH_SPI,
> + 0
> + );
> +
> + //
> + // Disable the access to the BIOS space for write cycles
> + //
> + PciSegmentAnd8 (
> + SpiBaseAddress + R_PCH_SPI_BC,
> + (UINT8) (~B_PCH_SPI_BC_WPD)
> + );
> +}
> diff --git
> a/Silicon/Intel/KabylakeSiliconPkg/Pch/LibraryPrivate/BasePchSpiCommonLib/
> SpiCommon.c
> b/Silicon/Intel/KabylakeSiliconPkg/Pch/LibraryPrivate/BasePchSpiCommonLib/
> SpiCommon.c
> index 46184d4994..1e9a4e91f8 100644
> ---
> a/Silicon/Intel/KabylakeSiliconPkg/Pch/LibraryPrivate/BasePchSpiCommonLib/
> SpiCommon.c
> +++ b/Silicon/Intel/KabylakeSiliconPkg/Pch/LibraryPrivate/BasePchSpiComm
> +++ onLib/SpiCommon.c
> @@ -1,7 +1,7 @@
> /** @file
> PCH SPI Common Driver implements the SPI Host Controller Compatibility
> Interface.
>
> -Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2017 - 2019, Intel Corporation. All rights reserved.<BR>
> This program and the accompanying materials are licensed and made
> available under the terms and conditions of the BSD License that
> accompanies this distribution.
> The full text of the license may be found at @@ -14,6 +14,7 @@ WITHOUT
> WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR
> IMPLIED.
> #include <Uefi/UefiBaseType.h>
> #include <Library/IoLib.h>
> #include <Library/DebugLib.h>
> +#include <Library/PcdLib.h>
> #include <Library/BaseMemoryLib.h>
> #include <IndustryStandard/Pci30.h>
> #include <PchAccess.h>
> @@ -62,6 +63,10 @@ SpiProtocolConstructor (
> );
>
> PchAcpiBaseGet (&(SpiInstance->PchAcpiBase));
> + if (SpiInstance->PchAcpiBase == 0) {
> + PchAcpiBaseSet (PcdGet16 (PcdAcpiBaseAddress)); } ASSERT
> + (SpiInstance->PchAcpiBase != 0);
> ASSERT (SpiInstance->PchAcpiBase != 0);
>
> PchSpiBar0 = MmioRead32 (SpiInstance->PchSpiBase + R_PCH_SPI_BAR0) &
> ~(B_PCH_SPI_BAR0_MASK); @@ -760,9 +765,8 @@ SendSpiCmd (
> Status = EFI_SUCCESS;
> SpiInstance = SPI_INSTANCE_FROM_SPIPROTOCOL (This);
> SpiBaseAddress = SpiInstance->PchSpiBase;
> - PchSpiBar0 = AcquireSpiBar0 (SpiInstance);
> - SpiBaseAddress = SpiInstance->PchSpiBase;
> ABase = SpiInstance->PchAcpiBase;
> + PchSpiBar0 = AcquireSpiBar0 (SpiInstance);
>
> //
> // Disable SMIs to make sure normal mode flash access is not interrupted by
> an SMI
> --
> 2.16.2.windows.1
next prev parent reply other threads:[~2019-04-01 16:33 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-03-31 23:27 [edk2-platforms][PATCH v1 0/3] Enable SPI flash debug messages Michael Kubacki
2019-03-31 23:27 ` [edk2-platforms][PATCH v1 1/3] KabylakeSiliconPkg: Add SPI write support in PEI Michael Kubacki
2019-04-01 5:37 ` Desimone, Nathaniel L
2019-04-01 16:33 ` Kubacki, Michael A [this message]
2019-03-31 23:27 ` [edk2-platforms][PATCH v1 2/3] ClevoOpenBoardPkg/N1xxWU: Flash map update Michael Kubacki
2019-04-01 5:42 ` Desimone, Nathaniel L
2019-04-02 1:10 ` Chiu, Chasel
2019-03-31 23:27 ` [edk2-platforms][PATCH v1 3/3] ClevoOpenBoardPkg/N1xxWU: Write PEI debug messages to SPI flash Michael Kubacki
2019-04-01 5:53 ` Desimone, Nathaniel L
2019-04-01 16:54 ` Kubacki, Michael A
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=49AB4ACB9627B8468F29D589A27B745584531E54@ORSMSX121.amr.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