From: "maobibo" <maobibo@loongson.cn>
To: xianglai li <lixianglai@loongson.cn>, devel@edk2.groups.io
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>,
Chao Li <lichao@loongson.cn>,
Leif Lindholm <quic_llindhol@quicinc.com>,
Liming Gao <gaoliming@byosoft.com.cn>,
Michael D Kinney <michael.d.kinney@intel.com>
Subject: Re: [edk2-platforms][PATCH V1 3/4] Platform/Loongson: Support pflash for loongarch.
Date: Wed, 4 Jan 2023 11:57:03 +0800 [thread overview]
Message-ID: <35a7b1ae-caff-f62a-33f2-eedc55d12be1@loongson.cn> (raw)
In-Reply-To: <d307db200fde257eabf24596ad5b0611119383e1.1672801654.git.lixianglai@loongson.cn>
在 2023/1/4 11:10, xianglai li 写道:
> Add pflash driver for loongarch.
>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Bibo Mao <maobibo@loongson.cn>
> Cc: Chao Li <lichao@loongson.cn>
> Cc: Leif Lindholm <quic_llindhol@quicinc.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Signed-off-by: xianglai li <lixianglai@loongson.cn>
> ---
> .../Library/NorFlashQemuLib/NorFlashQemuLib.c | 138 ++++++++++++++++++
> .../NorFlashQemuLib/NorFlashQemuLib.inf | 43 ++++++
> .../Loongson/LoongArchQemuPkg/Loongson.dsc | 19 ++-
> .../Loongson/LoongArchQemuPkg/Loongson.fdf | 4 +-
> .../LoongArchQemuPkg/VarStore.fdf.inc | 67 +++++++++
> 5 files changed, 260 insertions(+), 11 deletions(-)
> create mode 100644 Platform/Loongson/LoongArchQemuPkg/Library/NorFlashQemuLib/NorFlashQemuLib.c
> create mode 100644 Platform/Loongson/LoongArchQemuPkg/Library/NorFlashQemuLib/NorFlashQemuLib.inf
> create mode 100644 Platform/Loongson/LoongArchQemuPkg/VarStore.fdf.inc
>
> diff --git a/Platform/Loongson/LoongArchQemuPkg/Library/NorFlashQemuLib/NorFlashQemuLib.c b/Platform/Loongson/LoongArchQemuPkg/Library/NorFlashQemuLib/NorFlashQemuLib.c
> new file mode 100644
> index 0000000000..2565e5ae70
> --- /dev/null
> +++ b/Platform/Loongson/LoongArchQemuPkg/Library/NorFlashQemuLib/NorFlashQemuLib.c
> @@ -0,0 +1,138 @@
> +/** @file
> +
> + Copyright (c) 2023 Loongson Technology Corporation Limited. All rights reserved.<BR>
> +
> + SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +
> +#include <Library/BaseLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/UefiBootServicesTableLib.h>
> +#include <Library/VirtNorFlashPlatformLib.h>
> +
> +#include <Protocol/FdtClient.h>
> +
> +#define QEMU_NOR_BLOCK_SIZE SIZE_128KB
> +
> +#define MAX_FLASH_BANKS 1
> +
> +EFI_STATUS
> +VirtNorFlashPlatformInitialization (
> + VOID
> + )
> +{
> + return EFI_SUCCESS;
> +}
> +
> +STATIC VIRT_NOR_FLASH_DESCRIPTION mNorFlashDevices[MAX_FLASH_BANKS];
> +
> +EFI_STATUS
> +VirtNorFlashPlatformGetDevices (
> + OUT VIRT_NOR_FLASH_DESCRIPTION **NorFlashDescriptions,
> + OUT UINT32 *Count
> + )
> +{
> + FDT_CLIENT_PROTOCOL *FdtClient;
> + INT32 Node;
> + EFI_STATUS Status;
> + EFI_STATUS FindNodeStatus;
> + CONST UINT32 *Reg;
> + UINT32 PropSize;
> + UINT32 Num;
> + UINT64 Base;
> + UINT64 Size;
> +
> + Status = gBS->LocateProtocol (
> + &gFdtClientProtocolGuid,
> + NULL,
> + (VOID **)&FdtClient
> + );
> + ASSERT_EFI_ERROR (Status);
> +
> + Num = 0;
> + for (FindNodeStatus = FdtClient->FindCompatibleNode (
> + FdtClient,
> + "cfi-flash",
> + &Node
> + );
> + !EFI_ERROR (FindNodeStatus) && Num < MAX_FLASH_BANKS;
> + FindNodeStatus = FdtClient->FindNextCompatibleNode (
> + FdtClient,
> + "cfi-flash",
> + Node,
> + &Node
> + ))
> + {
> + Status = FdtClient->GetNodeProperty (
> + FdtClient,
> + Node,
> + "reg",
> + (CONST VOID **)&Reg,
> + &PropSize
> + );
> + if (EFI_ERROR (Status)) {
> + DEBUG ((
> + DEBUG_ERROR,
> + "%a: GetNodeProperty () failed (Status == %r)\n",
> + __FUNCTION__,
> + Status
> + ));
> + continue;
> + }
> +
> + ASSERT ((PropSize % (4 * sizeof (UINT32))) == 0);
> +
> + while (PropSize >= (4 * sizeof (UINT32)) && Num < MAX_FLASH_BANKS) {
> + Base = SwapBytes64 (ReadUnaligned64 ((VOID *)&Reg[0]));
> + Size = SwapBytes64 (ReadUnaligned64 ((VOID *)&Reg[2]));
> + Reg += 4;
> +
> + PropSize -= 4 * sizeof (UINT32);
> +
> + mNorFlashDevices[Num].DeviceBaseAddress = (UINTN)Base;
> + mNorFlashDevices[Num].RegionBaseAddress = (UINTN)Base;
> + mNorFlashDevices[Num].Size = (UINTN)Size;
> + mNorFlashDevices[Num].BlockSize = QEMU_NOR_BLOCK_SIZE;
> + Num++;
> + }
Why is there while-loop? There is only one emulated plash device instead,
and variable Base will be overwritten during the loop.
> +
> + Status = PcdSet32S (PcdFlashNvStorageVariableBase, Base);
> + ASSERT_EFI_ERROR (Status);
> +
Can you add some notation as bellowing?
/*
* Base is the value of PcdFlashNvStorageVariableBase,
* PcdFlashNvStorageFtwWorkingBase can be got by
* PcdFlashNvStorageVariableBase + PcdFlashNvStorageVariableSize
*/
> + Base += PcdGet32 (PcdFlashNvStorageVariableSize);
> + Status = PcdSet32S (PcdFlashNvStorageFtwWorkingBase, Base);
> + ASSERT_EFI_ERROR (Status);
> +
ditto
> + Base += PcdGet32 (PcdFlashNvStorageFtwWorkingSize);
> + Status = PcdSet32S (PcdFlashNvStorageFtwSpareBase, Base);
> + ASSERT_EFI_ERROR (Status);
> +
> + //
> + // UEFI takes ownership of the NOR flash, and exposes its functionality
> + // through the UEFI Runtime Services GetVariable, SetVariable, etc. This
> + // means we need to disable it in the device tree to prevent the OS from
> + // attaching its device driver as well.
> + // Note that this also hides other flash banks, but the only other flash
> + // bank we expect to encounter is the one that carries the UEFI executable
> + // code, which is not intended to be guest updatable, and is usually backed
> + // in a readonly manner by QEMU anyway.
> + //
> + Status = FdtClient->SetNodeProperty (
> + FdtClient,
> + Node,
> + "status",
> + "disabled",
> + sizeof ("disabled")
> + );
> + if (EFI_ERROR (Status)) {
> + DEBUG ((DEBUG_WARN, "Failed to set NOR flash status to 'disabled'\n"));
> + }
> + }
> +
> + *NorFlashDescriptions = mNorFlashDevices;
> + *Count = Num;
> +
> + return EFI_SUCCESS;
> +}
> diff --git a/Platform/Loongson/LoongArchQemuPkg/Library/NorFlashQemuLib/NorFlashQemuLib.inf b/Platform/Loongson/LoongArchQemuPkg/Library/NorFlashQemuLib/NorFlashQemuLib.inf
> new file mode 100644
> index 0000000000..da05ca0898
> --- /dev/null
> +++ b/Platform/Loongson/LoongArchQemuPkg/Library/NorFlashQemuLib/NorFlashQemuLib.inf
> @@ -0,0 +1,43 @@
> +## @file
> +#
> +# Copyright (c) 2023 Loongson Technology Corporation Limited. All rights reserved.<BR>
> +#
> +# SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +##
> +
> +[Defines]
> + INF_VERSION = 0x00010005
> + BASE_NAME = NorFlashQemuLib
> + FILE_GUID = 339B7829-4C5F-4EFC-B2DD-5050E530DECE
> + MODULE_TYPE = DXE_DRIVER
> + VERSION_STRING = 1.0
> + LIBRARY_CLASS = VirtNorFlashPlatformLib
> +
> +[Sources.common]
> + NorFlashQemuLib.c
> +
> +[Packages]
> + MdePkg/MdePkg.dec
> + EmbeddedPkg/EmbeddedPkg.dec
> + OvmfPkg/OvmfPkg.dec
> + MdeModulePkg/MdeModulePkg.dec
> +
> +[LibraryClasses]
> + BaseLib
> + DebugLib
> + UefiBootServicesTableLib
> +
> +[Protocols]
> + gFdtClientProtocolGuid ## CONSUMES
> +
> +[Depex]
> + gFdtClientProtocolGuid
> +
> +[Pcd]
> +gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize
> +gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize
> +gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize
> +gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase
> +gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase
> +gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase
> diff --git a/Platform/Loongson/LoongArchQemuPkg/Loongson.dsc b/Platform/Loongson/LoongArchQemuPkg/Loongson.dsc
> index a9b5c8c514..650042662d 100644
> --- a/Platform/Loongson/LoongArchQemuPkg/Loongson.dsc
> +++ b/Platform/Loongson/LoongArchQemuPkg/Loongson.dsc
> @@ -175,6 +175,7 @@
> DebugLib | MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
> PeiServicesLib | MdePkg/Library/PeiServicesLib/PeiServicesLib.inf
> VariableFlashInfoLib | MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.inf
> + VirtNorFlashPlatformLib | Platform/Loongson/LoongArchQemuPkg/Library/NorFlashQemuLib/NorFlashQemuLib.inf
>
> [LibraryClasses.common.SEC]
> PcdLib | MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
> @@ -360,9 +361,9 @@
> #
> !include NetworkPkg/NetworkPcds.dsc.inc
>
> - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize | 0x10000
> - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize | 0x20000
> - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize | 0x10000
> + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize | 0x40000
> + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize | 0x40000
> + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize | 0x40000
>
> ################################################################################
> #
> @@ -370,10 +371,11 @@
> #
> ################################################################################
> [PcdsDynamicDefault]
> - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase | 0
> + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase | 0x1d080000
Since PcdFlashNvStorageFtwSpareBase can be parsed from fdt, why there is hardcoding value?
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64 | 0
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64 | 0
> - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase | 0
> + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase | 0x1d000000
> + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase | 0x1d040000
Can we remove these hardcoding value?
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64 | 0
> gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvStoreReserved | 0
> gEfiMdeModulePkgTokenSpaceGuid.PcdPciDisableBusEnumeration | FALSE
> @@ -471,15 +473,12 @@
> #
> # Variable
> #
> -
> - OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.inf {
> - <LibraryClasses>
> - PlatformFvbLib|OvmfPkg/Library/EmuVariableFvbLib/EmuVariableFvbLib.inf
> - }
> + OvmfPkg/VirtNorFlashDxe/VirtNorFlashDxe.inf
> MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf
> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf {
> <LibraryClasses>
> NULL|MdeModulePkg/Library/VarCheckUefiLib/VarCheckUefiLib.inf
> + NULL|EmbeddedPkg/Library/NvVarStoreFormattedLib/NvVarStoreFormattedLib.inf
> BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf
> }
>
> diff --git a/Platform/Loongson/LoongArchQemuPkg/Loongson.fdf b/Platform/Loongson/LoongArchQemuPkg/Loongson.fdf
> index ee89097344..ec47d96325 100644
> --- a/Platform/Loongson/LoongArchQemuPkg/Loongson.fdf
> +++ b/Platform/Loongson/LoongArchQemuPkg/Loongson.fdf
> @@ -30,6 +30,8 @@ $(DXEFV_OFFSET)|$(DXEFV_SIZE)
> gLoongArchQemuPkgTokenSpaceGuid.PcdFlashDxeFvBase|gLoongArchQemuPkgTokenSpaceGuid.PcdFlashDxeFvSize
> FV = FVMAIN_COMPACT
>
> +
> +!include VarStore.fdf.inc
> #####################################################################################################
> [FV.SECFV]
> FvNameGuid = 587d4265-5e71-41da-9c35-4258551f1e22
> @@ -135,7 +137,7 @@ INF MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntimeDxe.inf
> #
> # Variable
> #
> -INF OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.inf
> +INF OvmfPkg/VirtNorFlashDxe/VirtNorFlashDxe.inf
> INF MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf
> INF MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
> #
> diff --git a/Platform/Loongson/LoongArchQemuPkg/VarStore.fdf.inc b/Platform/Loongson/LoongArchQemuPkg/VarStore.fdf.inc
> new file mode 100644
> index 0000000000..c86255f3ca
> --- /dev/null
> +++ b/Platform/Loongson/LoongArchQemuPkg/VarStore.fdf.inc
> @@ -0,0 +1,67 @@
> +## @file
> +#
> +# Copyright (c) 2023 Loongson Technology Corporation Limited. All rights reserved.<BR>
> +#
> +# SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +##
> +
> +[FD.QEMU_VARS]
> +BaseAddress = 0x1d000000
Can we remove these hardcoding value here?
regards
bibo,mao
> +Size = 0x1000000
> +ErasePolarity = 1
> +BlockSize = 0x20000
> +NumBlocks = 128
> +
> +0x00000000|0x00040000
> +#NV_VARIABLE_STORE
> +DATA = {
> + ## This is the EFI_FIRMWARE_VOLUME_HEADER
> + # ZeroVector []
> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> + # FileSystemGuid: gEfiSystemNvDataFvGuid =
> + # { 0xFFF12B8D, 0x7696, 0x4C8B,
> + # { 0xA9, 0x85, 0x27, 0x47, 0x07, 0x5B, 0x4F, 0x50 }}
> + 0x8D, 0x2B, 0xF1, 0xFF, 0x96, 0x76, 0x8B, 0x4C,
> + 0xA9, 0x85, 0x27, 0x47, 0x07, 0x5B, 0x4F, 0x50,
> + # FvLength: 0xC0000
> + 0x00, 0x00, 0x0C, 0x00, 0x00, 0x00, 0x00, 0x00,
> + # Signature "_FVH" # Attributes
> + 0x5f, 0x46, 0x56, 0x48, 0xff, 0xfe, 0x04, 0x00,
> + # HeaderLength # CheckSum # ExtHeaderOffset #Reserved #Revision
> + 0x48, 0x00, 0x28, 0x09, 0x00, 0x00, 0x00, 0x02,
> + # Blockmap[0]: 0x3 Blocks * 0x40000 Bytes / Block
> + 0x3, 0x00, 0x00, 0x00, 0x00, 0x00, 0x04, 0x00,
> + # Blockmap[1]: End
> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> + ## This is the VARIABLE_STORE_HEADER
> + # It is compatible with SECURE_BOOT_ENABLE == FALSE as well.
> + # Signature: gEfiAuthenticatedVariableGuid =
> + # { 0xaaf32c78, 0x947b, 0x439a,
> + # { 0xa1, 0x80, 0x2e, 0x14, 0x4e, 0xc3, 0x77, 0x92 }}
> + 0x78, 0x2c, 0xf3, 0xaa, 0x7b, 0x94, 0x9a, 0x43,
> + 0xa1, 0x80, 0x2e, 0x14, 0x4e, 0xc3, 0x77, 0x92,
> + # Size: 0x40000 (gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize) -
> + # 0x48 (size of EFI_FIRMWARE_VOLUME_HEADER) = 0x3ffb8
> + # This can speed up the Variable Dispatch a bit.
> + 0xB8, 0xFF, 0x03, 0x00,
> + # FORMATTED: 0x5A #HEALTHY: 0xFE #Reserved: UINT16 #Reserved1: UINT32
> + 0x5A, 0xFE, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
> +}
> +
> +0x00040000|0x00040000
> +#NV_FTW_WORKING
> +DATA = {
> + # EFI_FAULT_TOLERANT_WORKING_BLOCK_HEADER->Signature = gEdkiiWorkingBlockSignatureGuid =
> + # { 0x9e58292b, 0x7c68, 0x497d, { 0xa0, 0xce, 0x65, 0x0, 0xfd, 0x9f, 0x1b, 0x95 }}
> + 0x2b, 0x29, 0x58, 0x9e, 0x68, 0x7c, 0x7d, 0x49,
> + 0xa0, 0xce, 0x65, 0x0, 0xfd, 0x9f, 0x1b, 0x95,
> + # Crc:UINT32 #WorkingBlockValid:1, WorkingBlockInvalid:1, Reserved
> + 0x5b, 0xe7, 0xc6, 0x86, 0xFE, 0xFF, 0xFF, 0xFF,
> + # WriteQueueSize: UINT64
> + 0xE0, 0xFF, 0x03, 0x00, 0x00, 0x00, 0x00, 0x00
> +}
> +
> +0x00080000|0x00040000
> +#NV_FTW_SPARE
next prev parent reply other threads:[~2023-01-04 3:57 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <cover.1672801654.git.lixianglai@loongson.cn>
2023-01-04 3:10 ` [edk2-platforms][PATCH V1 1/4] Platform/Loongson: add bootmode support xianglai
2023-01-04 3:10 ` [edk2-platforms][PATCH V1 2/4] Platform/Loongson:add nvme device driver for loongarch xianglai
2023-01-04 3:10 ` [edk2-platforms][PATCH V1 3/4] Platform/Loongson: Support pflash " xianglai
2023-01-04 3:57 ` maobibo [this message]
2023-01-04 3:10 ` [edk2-platforms][PATCH V1 4/4] Platform/Loongson: Fixed the bug during page table creation xianglai
2023-01-04 4:14 ` maobibo
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=35a7b1ae-caff-f62a-33f2-eedc55d12be1@loongson.cn \
--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