Re: [PATCH edk2-platforms 5/8] Silicon/SynQuacer: disable PCI RC #0 DT node if disabled

[Ard Biesheuvel <ard.biesheuvel@linaro.org>]

On 12 December 2017 at 17:32, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> Suggested subject tweak:
> Silicon/SynQuacer: disable PCI RC #0 DT node if disabled ->
> Silicon/SynQuacer: disable PCI RC DT node if RC disabled.
>

Ack. It's out of date, as you probably suspected

> The code below will disable either #0 or #1 node if not enabled by
> Pcd.
>
> On Tue, Dec 12, 2017 at 10:38:04AM +0000, Ard Biesheuvel wrote:
>> If PCIe RC #0 is not enabled (due to the fact that the slot is not
>> populated), set its DT node 'status' property to 'disabled' so that
>> the OS will not attempt to attach to it.
>>
>> This means we will need to switch from the default DtPlatformDtbLoaderLib
>> to a special one for our platform.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>>  Platform/Socionext/DeveloperBox/DeveloperBox.dsc                                    |  8 +-
>>  Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc                        |  3 +-
>>  Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.c   | 94 ++++++++++++++++++++
>>  Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.inf | 42 +++++++++
>>  4 files changed, 141 insertions(+), 6 deletions(-)
>>
>> diff --git a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
>> index 5ec26f9cdd34..80728fedbc20 100644
>> --- a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
>> +++ b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
>> @@ -160,7 +160,8 @@ [LibraryClasses.common.DXE_CORE]
>>    PerformanceLib|MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.inf
>>
>>  [LibraryClasses.common.DXE_DRIVER]
>> -  DtPlatformDtbLoaderLib|EmbeddedPkg/Library/DxeDtPlatformDtbLoaderLibDefault/DxeDtPlatformDtbLoaderLibDefault.inf
>> +  DtPlatformDtbLoaderLib|Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.inf
>> +  FdtLib|EmbeddedPkg/Library/FdtLib/FdtLib.inf
>>    SecurityManagementLib|MdeModulePkg/Library/DxeSecurityManagementLib/DxeSecurityManagementLib.inf
>>    PerformanceLib|MdeModulePkg/Library/DxePerformanceLib/DxePerformanceLib.inf
>>
>> @@ -611,10 +612,7 @@ [Components.common]
>>    #
>>    # Console preference selection
>>    #
>> -  EmbeddedPkg/Drivers/ConsolePrefDxe/ConsolePrefDxe.inf {
>> -    <LibraryClasses>
>> -      FdtLib|EmbeddedPkg/Library/FdtLib/FdtLib.inf
>> -  }
>> +  EmbeddedPkg/Drivers/ConsolePrefDxe/ConsolePrefDxe.inf
>>
>>    #
>>    # DT support
>> diff --git a/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc b/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc
>> index bc8ddd452d4b..c71425664bdc 100644
>> --- a/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc
>> +++ b/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc
>> @@ -159,7 +159,8 @@ [LibraryClasses.common.DXE_CORE]
>>    PerformanceLib|MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.inf
>>
>>  [LibraryClasses.common.DXE_DRIVER]
>> -  DtPlatformDtbLoaderLib|EmbeddedPkg/Library/DxeDtPlatformDtbLoaderLibDefault/DxeDtPlatformDtbLoaderLibDefault.inf
>> +  DtPlatformDtbLoaderLib|Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.inf
>> +  FdtLib|EmbeddedPkg/Library/FdtLib/FdtLib.inf
>>    SecurityManagementLib|MdeModulePkg/Library/DxeSecurityManagementLib/DxeSecurityManagementLib.inf
>>    PerformanceLib|MdeModulePkg/Library/DxePerformanceLib/DxePerformanceLib.inf
>>
>> diff --git a/Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.c b/Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.c
>> new file mode 100644
>> index 000000000000..a93a6027e64d
>> --- /dev/null
>> +++ b/Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.c
>> @@ -0,0 +1,94 @@
>> +/** @file
>> +*
>> +*  Copyright (c) 2017, Linaro, Ltd. All rights reserved.
>> +*
>> +*  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.
>> +*
>> +**/
>> +
>> +#include <PiDxe.h>
>> +
>> +#include <libfdt.h>
>> +#include <Library/BaseLib.h>
>> +#include <Library/DebugLib.h>
>> +#include <Library/DxeServicesLib.h>
>> +#include <Library/MemoryAllocationLib.h>
>> +
>> +#define DTB_PADDING               64
>
> If there isn't a simplish way of determining this value
> arithmetically, can you add a comment explaining why 64 is sufficient?
>

Something like

// add enough space for two instances of 'status = "disabled"'

?


>> +STATIC
>> +VOID
>> +DisableDtNode (
>> +  IN  VOID                        *Dtb,
>> +  IN  CONST CHAR8                 *NodePath
>> +  )
>> +{
>> +  INT32                           Node;
>> +  INT32                           Rc;
>> +
>> +  Node = fdt_path_offset (Dtb, NodePath);
>> +  if (Node < 0) {
>> +    DEBUG ((DEBUG_ERROR, "%a: failed to locate DT path '%a': %a\n",
>> +      __FUNCTION__, NodePath, fdt_strerror (Node)));
>> +    return;
>> +  }
>> +  Rc = fdt_setprop_string (Dtb, Node, "status", "disabled");
>> +  if (Rc < 0) {
>> +    DEBUG ((DEBUG_ERROR, "%a: failed to set status to 'disabled' on '%a': %a\n",
>> +      __FUNCTION__, NodePath, fdt_strerror (Rc)));
>> +  }
>> +}
>> +
>> +/**
>> +  Return a pool allocated copy of the DTB image that is appropriate for
>> +  booting the current platform via DT.
>> +
>> +  @param[out]   Dtb                   Pointer to the DTB copy
>> +  @param[out]   DtbSize               Size of the DTB copy
>> +
>> +  @retval       EFI_SUCCESS           Operation completed successfully
>> +  @retval       EFI_NOT_FOUND         No suitable DTB image could be located
>> +  @retval       EFI_OUT_OF_RESOURCES  No pool memory available
>> +
>> +**/
>> +EFI_STATUS
>> +EFIAPI
>> +DtPlatformLoadDtb (
>> +  OUT   VOID        **Dtb,
>> +  OUT   UINTN       *DtbSize
>> +  )
>> +{
>> +  EFI_STATUS      Status;
>> +  VOID            *OrigDtb;
>> +  VOID            *CopyDtb;
>> +  UINTN           OrigDtbSize;
>> +
>> +  Status = GetSectionFromAnyFv (&gDtPlatformDefaultDtbFileGuid,
>> +             EFI_SECTION_RAW, 0, &OrigDtb, &OrigDtbSize);
>> +  if (EFI_ERROR (Status)) {
>> +    return EFI_NOT_FOUND;
>> +  }
>> +
>> +  CopyDtb = AllocateCopyPool (OrigDtbSize + DTB_PADDING, OrigDtb);
>> +  if (CopyDtb == NULL) {
>> +    return EFI_OUT_OF_RESOURCES;
>> +  }
>> +
>> +  if (!(PcdGet8 (PcdPcieEnableMask) & BIT0)) {
>> +    DisableDtNode (CopyDtb, "/pcie@60000000");
>> +  }
>> +  if (!(PcdGet8 (PcdPcieEnableMask) & BIT1)) {
>> +    DisableDtNode (CopyDtb, "/pcie@70000000");
>> +  }
>> +
>> +  *Dtb = CopyDtb;
>> +  *DtbSize = OrigDtbSize + DTB_PADDING;
>> +
>> +  return EFI_SUCCESS;
>> +}
>> diff --git a/Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.inf b/Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.inf
>> new file mode 100644
>> index 000000000000..e1f564f73078
>> --- /dev/null
>> +++ b/Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.inf
>> @@ -0,0 +1,42 @@
>> +/** @file
>> +*
>> +*  Copyright (c) 2017, Linaro, Ltd. All rights reserved.
>> +*
>> +*  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                    = 0x0001001A
>> +  BASE_NAME                      = SynQuacerDtbLoaderLib
>> +  FILE_GUID                      = 59df69c4-8724-4e49-8974-d0691364338c
>> +  MODULE_TYPE                    = DXE_DRIVER
>> +  VERSION_STRING                 = 1.0
>> +  LIBRARY_CLASS                  = DtPlatformDtbLoaderLib|DXE_DRIVER
>> +
>> +[Sources]
>> +  SynQuacerDtbLoaderLib.c
>> +
>> +[Packages]
>> +  MdePkg/MdePkg.dec
>> +  EmbeddedPkg/EmbeddedPkg.dec
>> +  Silicon/Socionext/SynQuacer/SynQuacer.dec
>> +
>> +[LibraryClasses]
>> +  BaseLib
>> +  DebugLib
>> +  DxeServicesLib
>> +  FdtLib
>> +  MemoryAllocationLib
>> +
>> +[Pcd]
>> +  gSynQuacerTokenSpaceGuid.PcdPcieEnableMask
>> +
>> +[Guids]
>> +  gDtPlatformDefaultDtbFileGuid
>> --
>> 2.11.0
>>