[edk2] [PATCH 4/4] EmbeddedPkg/DwEmmc: Check DMA completion in SendCommand
From: "Loh, Tien Hock"
DwEmmcReadBlockData and DwEmmcWriteBlockData needs to check for the
transfer completion before returning. This also adds error checking
to the DMA transfer.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Loh Tien Hock
---
EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c | 30 --
1 file changed, 28 insertions(+), 2 deletions(-)
diff --git a/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c
b/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c
index c232309..baf299d 100644
--- a/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c
+++ b/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c
@@ -563,8 +563,9 @@ DwEmmcReadBlockData (
)
{
EFI_STATUS Status;
- UINT32 DescPages, CountPerPage, Count;
+ UINT32 DescPages, CountPerPage, Count, ErrMask;
EFI_TPL Tpl;
+ UINTN Rintsts;
Tpl = gBS->RaiseTPL (TPL_NOTIFY);
@@ -587,6 +588,18 @@ DwEmmcReadBlockData (
DEBUG ((DEBUG_ERROR, "Failed to read data, mDwEmmcCommand:%x,
mDwEmmcArgument:%x, Status:%r\n", mDwEmmcCommand, mDwEmmcArgument, Status));
goto out;
}
+
+ while(!((MmioRead32(DWEMMC_RINTSTS) & (DWEMMC_INT_DTO {
+Rintsts = MmioRead32 (DWEMMC_RINTSTS);
+ }
+ ErrMask = DWEMMC_INT_EBE | DWEMMC_INT_HLE | DWEMMC_INT_RTO |
+DWEMMC_INT_RCRC | DWEMMC_INT_RE | DWEMMC_INT_DCRC |
+DWEMMC_INT_DRT | DWEMMC_INT_SBE;
+
+ if (Rintsts & ErrMask) {
+Status = EFI_DEVICE_ERROR;
+goto out;
+ }
out:
// Restore Tpl
gBS->RestoreTPL (Tpl);
@@ -602,8 +615,9 @@ DwEmmcWriteBlockData (
)
{
EFI_STATUS Status;
- UINT32 DescPages, CountPerPage, Count;
+ UINT32 DescPages, CountPerPage, Count, ErrMask;
EFI_TPL Tpl;
+ UINTN Rintsts;
Tpl = gBS->RaiseTPL (TPL_NOTIFY);
@@ -626,6 +640,18 @@ DwEmmcWriteBlockData (
DEBUG ((DEBUG_ERROR, "Failed to write data, mDwEmmcCommand:%x,
mDwEmmcArgument:%x, Status:%r\n", mDwEmmcCommand, mDwEmmcArgument, Status));
goto out;
}
+
+ while(!((MmioRead32(DWEMMC_RINTSTS) & (DWEMMC_INT_DTO {
+Rintsts = MmioRead32 (DWEMMC_RINTSTS);
+ }
+ ErrMask = DWEMMC_INT_EBE | DWEMMC_INT_HLE | DWEMMC_INT_RTO |
+DWEMMC_INT_RCRC | DWEMMC_INT_RE | DWEMMC_INT_DCRC |
+DWEMMC_INT_DRT | DWEMMC_INT_SBE;
+
+ if (Rintsts & ErrMask) {
+Status = EFI_DEVICE_ERROR;
+goto out;
+ }
out:
// Restore Tpl
gBS->RestoreTPL (Tpl);
--
2.2.2
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH 2/4] EmbeddedPkg/DwEmmc: Fix SendCommand parameters
From: "Loh, Tien Hock"
Only send BIT_CMD_CHECK_RESPONSE_CRC if MMC commands needs it.
Fixes parameters to ACMD6 where if CMD is application command, ie. CMD55 is
sent before ACMD6, to do response instead of data transfer.
Added CMD51 handling as CMD51 is a data transfer, and needs BIT_CMD_READ
and BIT_CMD_DATA_EXPECTED to be set.
Updates DwEmmcReceiveResponse to SendCommand only if IsPendingReadCommand
or IsPendingWriteCommand is true.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Loh Tien Hock
---
EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c | 59 +++
1 file changed, 45 insertions(+), 14 deletions(-)
diff --git a/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c
b/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c
index 6d0f472..600ab01 100644
--- a/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c
+++ b/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c
@@ -45,6 +45,7 @@ DWEMMC_IDMAC_DESCRIPTOR *gpIdmacDesc;
EFI_GUID mDwEmmcDevicePathGuid = EFI_CALLER_ID_GUID;
STATIC UINT32 mDwEmmcCommand;
STATIC UINT32 mDwEmmcArgument;
+STATIC BOOLEAN mIsACmd = FALSE;
EFI_STATUS
DwEmmcReadBlockData (
@@ -321,68 +322,93 @@ DwEmmcSendCommand (
break;
case MMC_INDX(2):
Cmd = BIT_CMD_RESPONSE_EXPECT | BIT_CMD_LONG_RESPONSE |
- BIT_CMD_CHECK_RESPONSE_CRC | BIT_CMD_SEND_INIT;
+ BIT_CMD_SEND_INIT;
break;
case MMC_INDX(3):
-Cmd = BIT_CMD_RESPONSE_EXPECT | BIT_CMD_CHECK_RESPONSE_CRC |
+Cmd = BIT_CMD_RESPONSE_EXPECT |
BIT_CMD_SEND_INIT;
break;
+ case MMC_INDX(6):
+if(mIsACmd) {
+ Cmd = BIT_CMD_RESPONSE_EXPECT ;
+}
+else {
+ Cmd = BIT_CMD_RESPONSE_EXPECT | BIT_CMD_DATA_EXPECTED |
+BIT_CMD_READ;
+}
+break;
case MMC_INDX(7):
if (Argument)
-Cmd = BIT_CMD_RESPONSE_EXPECT | BIT_CMD_CHECK_RESPONSE_CRC;
+Cmd = BIT_CMD_RESPONSE_EXPECT;
else
Cmd = 0;
break;
case MMC_INDX(8):
-Cmd = BIT_CMD_RESPONSE_EXPECT | BIT_CMD_CHECK_RESPONSE_CRC |
- BIT_CMD_DATA_EXPECTED | BIT_CMD_READ |
+Cmd = BIT_CMD_RESPONSE_EXPECT |
BIT_CMD_WAIT_PRVDATA_COMPLETE;
break;
case MMC_INDX(9):
-Cmd = BIT_CMD_RESPONSE_EXPECT | BIT_CMD_CHECK_RESPONSE_CRC |
+Cmd = BIT_CMD_RESPONSE_EXPECT |
BIT_CMD_LONG_RESPONSE;
break;
case MMC_INDX(12):
-Cmd = BIT_CMD_RESPONSE_EXPECT | BIT_CMD_CHECK_RESPONSE_CRC |
+Cmd = BIT_CMD_RESPONSE_EXPECT |
BIT_CMD_STOP_ABORT_CMD;
break;
case MMC_INDX(13):
-Cmd = BIT_CMD_RESPONSE_EXPECT | BIT_CMD_CHECK_RESPONSE_CRC |
+Cmd = BIT_CMD_RESPONSE_EXPECT |
BIT_CMD_WAIT_PRVDATA_COMPLETE;
break;
case MMC_INDX(16):
-Cmd = BIT_CMD_RESPONSE_EXPECT | BIT_CMD_CHECK_RESPONSE_CRC |
+Cmd = BIT_CMD_RESPONSE_EXPECT |
BIT_CMD_DATA_EXPECTED | BIT_CMD_READ |
BIT_CMD_WAIT_PRVDATA_COMPLETE;
break;
case MMC_INDX(17):
case MMC_INDX(18):
-Cmd = BIT_CMD_RESPONSE_EXPECT | BIT_CMD_CHECK_RESPONSE_CRC |
+Cmd = BIT_CMD_RESPONSE_EXPECT |
BIT_CMD_DATA_EXPECTED | BIT_CMD_READ |
BIT_CMD_WAIT_PRVDATA_COMPLETE;
break;
case MMC_INDX(24):
case MMC_INDX(25):
-Cmd = BIT_CMD_RESPONSE_EXPECT | BIT_CMD_CHECK_RESPONSE_CRC |
+Cmd = BIT_CMD_RESPONSE_EXPECT |
BIT_CMD_DATA_EXPECTED | BIT_CMD_WRITE |
BIT_CMD_WAIT_PRVDATA_COMPLETE;
break;
case MMC_INDX(30):
-Cmd = BIT_CMD_RESPONSE_EXPECT | BIT_CMD_CHECK_RESPONSE_CRC |
+Cmd = BIT_CMD_RESPONSE_EXPECT |
BIT_CMD_DATA_EXPECTED;
break;
+ case MMC_INDX(51):
+Cmd = BIT_CMD_RESPONSE_EXPECT | BIT_CMD_DATA_EXPECTED |
+ BIT_CMD_READ | BIT_CMD_WAIT_PRVDATA_COMPLETE;
+break;
default:
-Cmd = BIT_CMD_RESPONSE_EXPECT | BIT_CMD_CHECK_RESPONSE_CRC;
+Cmd = BIT_CMD_RESPONSE_EXPECT ;
break;
}
Cmd |= MMC_GET_INDX(MmcCmd) | BIT_CMD_USE_HOLD_REG | BIT_CMD_START;
+
+ if(MMC_INDX(55) == MMC_GET_INDX(MmcCmd))
+mIsACmd = TRUE;
+ else
+mIsACmd = FALSE;
+
+ if (!(MmcCmd & MMC_CMD_NO_CRC_RESPONSE)) {
+Cmd |= BIT_CMD_CHECK_RESPONSE_CRC;
+ }
+
if (IsPendingReadCommand (Cmd) || IsPendingWriteCommand (Cmd)) {
mDwEmmcCommand = Cmd;
mDwEmmcArgument = Argument;
} else {
+mDwEmmcCommand = Cmd;
+mDwEmmcArgument = Argument;
Status = SendCommand (Cmd, Argument);
}
+
return Status;
}
@@ -393,6 +419,11 @@ DwEmmcReceiveResponse (
IN UINT32*Buffer
)
{
+ EFI_STATUS Status = EFI_SUCCESS;
+
+ if(IsPendingReadCommand (mDwEmmcCommand) ||
IsPendingWriteCommand(mDwEmmcCommand))
+Status = SendCommand (mDwEmmcCommand, mDwEmmcArgument);
+
if (Buffer == NULL) {
return EFI_INVALID_PARAMETER;
}
@@ -410,7 +441,7 @@ DwEmmcReceiveResponse (
Buffer[2] = MmioRead32 (DWEMMC_RESP2);
Buffer[3] = MmioRead32 (DWEMMC_RESP3);
}
- return EFI_SUCCESS;
+ return Status;
}
VOID
--
2.2.2
[edk2] [PATCH 3/4] EmbeddedPkg/DwEmmc: Fix DMA transfer length
From: "Loh, Tien Hock"
DMA should not transfer more than requested length otherwise FIFO might run
into buffer underrun and causes errors in future transfers.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Loh Tien Hock
---
EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c | 19 ---
1 file changed, 16 insertions(+), 3 deletions(-)
diff --git a/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c
b/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c
index 600ab01..c232309 100644
--- a/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c
+++ b/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c
@@ -496,7 +496,13 @@ PrepareDmaData (
Cnt = (Length + DWEMMC_DMA_BUF_SIZE - 1) / DWEMMC_DMA_BUF_SIZE;
Blks = (Length + DWEMMC_BLOCK_SIZE - 1) / DWEMMC_BLOCK_SIZE;
- Length = DWEMMC_BLOCK_SIZE * Blks;
+
+ if(Length < DWEMMC_BLOCK_SIZE) {
+Length = Length;
+ }
+ else {
+Length = DWEMMC_BLOCK_SIZE * Blks;
+ }
for (Idx = 0; Idx < Cnt; Idx++) {
(IdmacDesc + Idx)->Des0 = DWEMMC_IDMAC_DES0_OWN | DWEMMC_IDMAC_DES0_CH |
@@ -534,11 +540,18 @@ StartDma (
Data |= DWEMMC_CTRL_INT_EN | DWEMMC_CTRL_DMA_EN | DWEMMC_CTRL_IDMAC_EN;
MmioWrite32 (DWEMMC_CTRL, Data);
Data = MmioRead32 (DWEMMC_BMOD);
+
Data |= DWEMMC_IDMAC_ENABLE | DWEMMC_IDMAC_FB;
MmioWrite32 (DWEMMC_BMOD, Data);
- MmioWrite32 (DWEMMC_BLKSIZ, DWEMMC_BLOCK_SIZE);
- MmioWrite32 (DWEMMC_BYTCNT, Length);
+ if(Length < DWEMMC_BLOCK_SIZE) {
+MmioWrite32 (DWEMMC_BLKSIZ, Length);
+MmioWrite32 (DWEMMC_BYTCNT, Length);
+ }
+ else {
+MmioWrite32 (DWEMMC_BLKSIZ, DWEMMC_BLOCK_SIZE);
+MmioWrite32 (DWEMMC_BYTCNT, Length);
+ }
}
EFI_STATUS
--
2.2.2
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH 0/4] EmbeddedPkg/DwEmmc: Fix bugs causing DwEmmc to fail to initialize
From: Loh Tien Hock This patch series fixes bugs with DwEmmc driver, namely: * Added CMD6 handling * Fixed workaround querying SendCommand using delays * Fix DMA transfer length causing buffer underrun in FIFO * Check DMA completion before returning from SendCommand Loh, Tien Hock (4): EmbeddedPkg/DwEmmc: Remove unnecessary MicroSecondDelay EmbeddedPkg/DwEmmc: Fix SendCommand parameters EmbeddedPkg/DwEmmc: Fix DMA transfer length EmbeddedPkg/DwEmmc: Check DMA completion in SendCommand EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c | 122 +++--- 1 file changed, 95 insertions(+), 27 deletions(-) -- 2.2.2 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH 1/4] EmbeddedPkg/DwEmmc: Remove unnecessary MicroSecondDelay
From: "Loh, Tien Hock"
Existing implementation checks for error regardless of if
DWEMMC_INT_CMD_DONE is set, causing the loop check to errors out
even when it shouldn't if the MicroSecondDelay doesn't do long
enough delays. This removes MicroSecondDelay and updates the
function to check for CMD_DONE before doing any error checking.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Loh Tien Hock
---
EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c | 14 ++
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c
b/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c
index 0437e30..6d0f472 100644
--- a/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c
+++ b/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c
@@ -290,17 +290,15 @@ SendCommand (
ErrMask = DWEMMC_INT_EBE | DWEMMC_INT_HLE | DWEMMC_INT_RTO |
DWEMMC_INT_RCRC | DWEMMC_INT_RE;
ErrMask |= DWEMMC_INT_DCRC | DWEMMC_INT_DRT | DWEMMC_INT_SBE;
+
do {
-MicroSecondDelay(500);
Data = MmioRead32 (DWEMMC_RINTSTS);
-
-if (Data & ErrMask) {
- return EFI_DEVICE_ERROR;
-}
-if (Data & DWEMMC_INT_DTO) { // Transfer Done
- break;
-}
} while (!(Data & DWEMMC_INT_CMD_DONE));
+
+ if (Data & ErrMask) {
+return EFI_DEVICE_ERROR;
+ }
+
return EFI_SUCCESS;
}
--
2.2.2
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH 1/1] BaseTools: Fix BPDG tool print traceback info issue
Fix BPDG tool print traceback info issue
and remove abundant code
Cc: Liming Gao
Cc: Yonghong Zhu
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Yunhua Feng
---
BaseTools/Source/Python/BPDG/BPDG.py | 5 -
BaseTools/Source/Python/Common/VpdInfoFile.py | 5 ++---
2 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/BaseTools/Source/Python/BPDG/BPDG.py
b/BaseTools/Source/Python/BPDG/BPDG.py
index 2ec1516c0a..c30e062a69 100644
--- a/BaseTools/Source/Python/BPDG/BPDG.py
+++ b/BaseTools/Source/Python/BPDG/BPDG.py
@@ -151,11 +151,14 @@ def StartBpdg(InputFileName, MapFileName, VpdFileName,
Force):
GenVPD.GenerateVpdFile(MapFileName, VpdFileName)
EdkLogger.info("- Vpd pcd fixed done! -")
if __name__ == '__main__':
-r = main()
+try:
+r = main()
+except FatalError as e:
+r = e
## 0-127 is a safe return range, and 1 is a standard default error
if r < 0 or r > 127: r = 1
sys.exit(r)
diff --git a/BaseTools/Source/Python/Common/VpdInfoFile.py
b/BaseTools/Source/Python/Common/VpdInfoFile.py
index 0485bf482e..2fb8e66fe9 100644
--- a/BaseTools/Source/Python/Common/VpdInfoFile.py
+++ b/BaseTools/Source/Python/Common/VpdInfoFile.py
@@ -252,11 +252,10 @@ def CallExtenalBPDGTool(ToolPath, VpdFileName):
print(out)
while PopenObject.returncode is None :
PopenObject.wait()
if PopenObject.returncode != 0:
-if PopenObject.returncode != 0:
-EdkLogger.debug(EdkLogger.DEBUG_1, "Fail to call BPDG tool",
str(error))
-EdkLogger.error("BPDG", BuildToolError.COMMAND_FAILURE, "Fail to
execute BPDG tool with exit code: %d, the error message is: \n %s" % \
+EdkLogger.debug(EdkLogger.DEBUG_1, "Fail to call BPDG tool",
str(error))
+EdkLogger.error("BPDG", BuildToolError.COMMAND_FAILURE, "Fail to
execute BPDG tool with exit code: %d, the error message is: \n %s" % \
(PopenObject.returncode, str(error)))
return PopenObject.returncode
--
2.12.2.windows.2
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [Patch 0/6] Fix coding style issues.
On 10/25/2018 10:25 AM, Eric Dong wrote: Fixed ECC issues caused by change serial from d5aa2078 to d28daadd. Eric Dong (6): UefiCpuPkg/RegisterCpuFeaturesLib: Fix build failure for VS2012 and GCC49. UefiCpuPkg/CpuCommonFeaturesLib: Remove white space at line end. UefiCpuPkg/RegisterCpuFeaturesLib: Fix ECC issues. UefiCpuPkg/PiSmmCpuDxeSmm: Remove white space at line end. UefiCpuPkg/PiSmmCpuDxeSmm: Clean up useless code. UefiCpuPkg/PiSmmCpuDxeSmm: Fix build failure for VS2012 and GCC49. .../Library/CpuCommonFeaturesLib/MachineCheck.c| 2 +- .../RegisterCpuFeaturesLib/CpuFeaturesInitialize.c | 1 + .../PeiRegisterCpuFeaturesLib.c| 2 ++ .../RegisterCpuFeaturesLib.c | 24 +++--- UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c | 5 +++-- UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c | 10 + UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h | 16 --- 7 files changed, 20 insertions(+), 40 deletions(-) Reviewed-by: Ruiyu Ni -- Thanks, Ray ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [Patch 5/6] UefiCpuPkg/PiSmmCpuDxeSmm: Clean up useless code.
On 10/25/2018 10:26 AM, Eric Dong wrote:
Remove useless code after change 93324390.
Cc: Ruiyu Ni
Cc: Laszlo Ersek
Cc: Dandan Bi
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong
---
UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c | 10 +-
UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h | 16
2 files changed, 1 insertion(+), 25 deletions(-)
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
index 42b040531e..abcc3eea05 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
@@ -1272,7 +1272,6 @@ InitializeSmmCpuSemaphores (
UINTN TotalSize;
UINTN GlobalSemaphoresSize;
UINTN CpuSemaphoresSize;
- UINTN MsrSemahporeSize;
UINTN SemaphoreSize;
UINTN Pages;
UINTN *SemaphoreBlock;
@@ -1282,8 +1281,7 @@ InitializeSmmCpuSemaphores (
ProcessorCount = gSmmCpuPrivate->SmmCoreEntryContext.NumberOfCpus;
GlobalSemaphoresSize = (sizeof (SMM_CPU_SEMAPHORE_GLOBAL) / sizeof (VOID
*)) * SemaphoreSize;
CpuSemaphoresSize= (sizeof (SMM_CPU_SEMAPHORE_CPU) / sizeof (VOID *)) *
ProcessorCount * SemaphoreSize;
- MsrSemahporeSize = MSR_SPIN_LOCK_INIT_NUM * SemaphoreSize;
- TotalSize = GlobalSemaphoresSize + CpuSemaphoresSize + MsrSemahporeSize;
+ TotalSize = GlobalSemaphoresSize + CpuSemaphoresSize;
DEBUG((EFI_D_INFO, "One Semaphore Size= 0x%x\n", SemaphoreSize));
DEBUG((EFI_D_INFO, "Total Semaphores Size = 0x%x\n", TotalSize));
Pages = EFI_SIZE_TO_PAGES (TotalSize);
@@ -1311,12 +1309,6 @@ InitializeSmmCpuSemaphores (
SemaphoreAddr += ProcessorCount * SemaphoreSize;
mSmmCpuSemaphores.SemaphoreCpu.Present = (BOOLEAN *)SemaphoreAddr;
- SemaphoreAddr = (UINTN)SemaphoreBlock + GlobalSemaphoresSize + CpuSemaphoresSize;
- mSmmCpuSemaphores.SemaphoreMsr.Msr = (SPIN_LOCK *)SemaphoreAddr;
- mSmmCpuSemaphores.SemaphoreMsr.AvailableCounter =
-((UINTN)SemaphoreBlock + Pages * SIZE_4KB - SemaphoreAddr) /
SemaphoreSize;
- ASSERT (mSmmCpuSemaphores.SemaphoreMsr.AvailableCounter >=
MSR_SPIN_LOCK_INIT_NUM);
-
mPFLock = mSmmCpuSemaphores.SemaphoreGlobal.PFLock;
mConfigSmmCodeAccessCheckLock =
mSmmCpuSemaphores.SemaphoreGlobal.CodeAccessCheckLock;
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
index e2970308fe..61d4bd3085 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
@@ -347,13 +347,6 @@ typedef struct {
volatile BOOLEAN *CandidateBsp;
} SMM_DISPATCHER_MP_SYNC_DATA;
-#define MSR_SPIN_LOCK_INIT_NUM 15
-
-typedef struct {
- SPIN_LOCK*SpinLock;
- UINT32 MsrIndex;
-} MP_MSR_LOCK;
-
#define SMM_PSD_OFFSET 0xfb00
///
@@ -376,21 +369,12 @@ typedef struct {
volatile BOOLEAN *Present;
} SMM_CPU_SEMAPHORE_CPU;
-///
-/// All MSRs semaphores' pointer and counter
-///
-typedef struct {
- SPIN_LOCK*Msr;
- UINTNAvailableCounter;
-} SMM_CPU_SEMAPHORE_MSR;
-
///
/// All semaphores' information
///
typedef struct {
SMM_CPU_SEMAPHORE_GLOBAL SemaphoreGlobal;
SMM_CPU_SEMAPHORE_CPU SemaphoreCpu;
- SMM_CPU_SEMAPHORE_MSR SemaphoreMsr;
} SMM_CPU_SEMAPHORES;
extern IA32_DESCRIPTOR gcSmiGdtr;
Reviewed-by: Ruiyu Ni
--
Thanks,
Ray
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH 2/2] ShellPkg-Shell App: Provide fully-qualified path to shell scripts
On 10/25/2018 2:03 AM, Carsey, Jaben wrote:
Looks good to me.
Ray?
Reviewed-by: Jaben Carsey
Reviewed-by: Ruiyu Ni
p.s. Ray if you agree you can RB and I will handle the push.
-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
jim.dai...@dell.com
Sent: Wednesday, October 24, 2018 9:36 AM
To: edk2-devel@lists.01.org
Cc: Ni, Ruiyu ; Carsey, Jaben
Subject: [edk2] [PATCH 2/2] ShellPkg-Shell App: Provide fully-qualified path
to shell scripts
Importance: High
Section 3.6.2 of version 2.2 of the shell specification requires that
the first positional argument (i.e. arg 0) of a shell script resolves
to "the full path name of the script itself."
Ensure that the startup script and any scripts launched by the shell
meet this requirement.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jim Dailey
---
ShellPkg/Application/Shell/Shell.c | 12 ++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/ShellPkg/Application/Shell/Shell.c
b/ShellPkg/Application/Shell/Shell.c
index 6185b6ac80..fe88177d57 100644
--- a/ShellPkg/Application/Shell/Shell.c
+++ b/ShellPkg/Application/Shell/Shell.c
@@ -3,6 +3,7 @@
Copyright (c) 2009 - 2018, Intel Corporation. All rights reserved.
(C) Copyright 2013-2014 Hewlett-Packard Development Company, L.P.
+ Copyright 2018 Dell Technologies.
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
@@ -1275,7 +1276,8 @@ DoStartupScript(
FileStringPath = LocateStartupScript (ImagePath, FilePath);
if (FileStringPath != NULL) {
-Status = RunScriptFile (FileStringPath, NULL, L"",
ShellInfoObject.NewShellParametersProtocol);
+FileStringPath = FullyQualifyPath();
+Status = RunScriptFile (FileStringPath, NULL, FileStringPath,
ShellInfoObject.NewShellParametersProtocol);
FreePool (FileStringPath);
} else {
//
@@ -2474,6 +2476,7 @@ RunCommandOrFile(
}
switch (Type) {
case Script_File_Name:
+ CommandWithPath = FullyQualifyPath();
Status = RunScriptFile (CommandWithPath, NULL, CmdLine,
ParamProtocol);
break;
case Efi_Application:
@@ -2812,7 +2815,12 @@ RunScriptFileHandle (
DeleteScriptFileStruct(NewScriptFile);
return (EFI_OUT_OF_RESOURCES);
}
-for (LoopVar = 0 ; LoopVar < 10 && LoopVar < NewScriptFile->Argc;
LoopVar++) {
+//
+// Put the full path of the script file into Argv[0] as required by section
+// 3.6.2 of version 2.2 of the shell specification.
+//
+NewScriptFile->Argv[0] = StrnCatGrow(>Argv[0], NULL,
NewScriptFile->ScriptName, 0);
+for (LoopVar = 1 ; LoopVar < 10 && LoopVar < NewScriptFile->Argc;
LoopVar++) {
ASSERT(NewScriptFile->Argv[LoopVar] == NULL);
NewScriptFile->Argv[LoopVar] = StrnCatGrow(
Argv[LoopVar], NULL, ShellInfoObject.NewShellParametersProtocol-
Argv[LoopVar], 0);
if (NewScriptFile->Argv[LoopVar] == NULL) {
--
2.17.0.windows.1
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
--
Thanks,
Ray
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH 1/2] ShellPkg-Shell App: Provide fully-qualified path to shell scripts
On 10/25/2018 12:35 AM, jim.dai...@dell.com wrote: Add a function to return the fully-qualified version of some path. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jim Dailey --- ShellPkg/Include/Library/ShellLib.h | 40 + ShellPkg/Library/UefiShellLib/UefiShellLib.c | 93 +++- 2 files changed, 132 insertions(+), 1 deletion(-) diff --git a/ShellPkg/Include/Library/ShellLib.h b/ShellPkg/Include/Library/ShellLib.h index 92fddc50f5..cd7e9c47c3 100644 --- a/ShellPkg/Include/Library/ShellLib.h +++ b/ShellPkg/Include/Library/ShellLib.h @@ -2,6 +2,7 @@ Provides interface to shell functionality for shell commands and applications. Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved. + Copyright 2018 Dell Technologies. 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 @@ -35,6 +36,45 @@ extern EFI_SHELL_PARAMETERS_PROTOCOL *gEfiShellParametersProtocol; extern EFI_SHELL_PROTOCOL*gEfiShellProtocol; +/** + Return the fully-qualified version of a relative path or an absolute path that + does not include a file system reference. + + If ASSERTs are disabled, and if the input parameter is NULL or points to NULL, + then NULL is returned. + + If the input path contains a ":", this function assumes that it is part of a + reference to a file system (e.g. "FS0:"). In such a case, Path is cleaned + and returned. + + If there is no working directory or there is not enough memory available to + create the fully-qualified path, Path is cleaned and returned. + + Otherwise, the current file system or working directory (as appropriate) is + prepended to Path. The input Path is freed and the resulting path is cleaned, + assigned to Path, and returned. + + NOTE: If the input path is an empty string, then the current working directory + (if it exists) is returned. In other words, an empty input path is treated + exactly the same as ".". + + @param[in, out] Path On input, a pointer to some file or directory path. On +output, a pointer to the clean and possibly fully- +qualified version of the input path. The input pointer +may be freed and reassigned on output. + + @retval NULL The input pointer or the path itself was NULL. + + @return A pointer to the clean, fully-qualified version of Path. If memory + allocation fails, or if there is no working directory, then a pointer + to the clean, but not necessarily fully-qualified version of Path. +**/ +CHAR16* +EFIAPI +FullyQualifyPath( + IN OUT CHAR16 **Path + ); + /** This function will retrieve the information about the file for the handle specified and store it in allocated pool memory. diff --git a/ShellPkg/Library/UefiShellLib/UefiShellLib.c b/ShellPkg/Library/UefiShellLib/UefiShellLib.c index f04adbb63f..52ca3ce1b1 100644 --- a/ShellPkg/Library/UefiShellLib/UefiShellLib.c +++ b/ShellPkg/Library/UefiShellLib/UefiShellLib.c @@ -2,7 +2,7 @@ Provides interface to shell functionality for shell commands and applications. (C) Copyright 2016 Hewlett Packard Enterprise Development LP - Copyright 2016 Dell Inc. + Copyright 2016-2018 Dell Technologies. Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved. This program and the accompanying materials are licensed and made available under the terms and conditions of the BSD License @@ -36,6 +36,97 @@ EFI_HANDLEmEfiShellEnvironment2Handle; FILE_HANDLE_FUNCTION_MAP FileFunctionMap; EFI_UNICODE_COLLATION_PROTOCOL *mUnicodeCollationProtocol; +/** + Return the fully-qualified version of a relative path or an absolute path that + does not include a file system reference. + + If asserts are disabled, and if the input parameter is NULL or points to NULL, + then NULL is returned. + + If the input path contains a ":", this function assumes that it is part of a + reference to a file system (e.g. "FS0:"). In such a case, Path is cleaned + and returned. + + If there is no working directory or there is not enough memory available to + create the fully-qualified path, Path is cleaned and returned. + + Otherwise, the current file system or working directory (as appropriate) is + prepended to Path. The input Path is freed and the resulting path is cleaned, + assigned to Path, and returned. + + NOTE: If the input path is an empty string, then the current working directory + (if it exists) is returned. In other words, an empty input path is treated + exactly the same as ".". + + @param[in, out] Path On input, a pointer to some file or directory path. On +output, a pointer to the clean and possibly fully- +
Re: [edk2] [PATCH v3 6/6] MdeModulePkg/Core: add freed-memory guard feature
Star,
Regards,
Jian
> -Original Message-
> From: Zeng, Star
> Sent: Thursday, October 25, 2018 11:37 AM
> To: Wang, Jian J ; edk2-devel@lists.01.org
> Cc: Kinney, Michael D ; Ni, Ruiyu
> ; Yao, Jiewen ; Laszlo Ersek
> ; Zeng, Star
> Subject: Re: [edk2] [PATCH v3 6/6] MdeModulePkg/Core: add freed-memory
> guard feature
>
> On 2018/10/24 13:26, Jian J Wang wrote:
> >> v3 changes:
> >> a. Merge from #4 and #5 of v2 patch
> >> b. Coding style cleanup
> >
> > Freed-memory guard is used to detect UAF (Use-After-Free) memory issue
> > which is illegal access to memory which has been freed. The principle
> > behind is similar to heap guard feature, that is we'll turn all pool
>
> Since we also regard UAF part of heap guard feature, better to use
> "pool/page heap guard feature" instead of "heap guard feature" here.
>
You're right. I'll change it.
> I quoted a piece of code at below and have question that why it uses
> hard code Attribute parameter?
>
> + CoreAddRange (
> +EfiConventionalMemory,
> +StartAddress,
> +EndAddress,
> +EFI_MEMORY_UC | EFI_MEMORY_WC | EFI_MEMORY_WT |
> EFI_MEMORY_WB
> +);
>
Because I don't know the actual attributes at that time so I think it'd be
safer to add all supported ones.
> Thanks,
> Star
>
> > memory allocation to page allocation and mark them to be not-present
> > once they are freed. The freed memory block will not be added back into
> > memory pool.
> >
> > This means that, once a page is allocated and freed, it cannot be
> > re-allocated. This will bring an issue, which is that there's
> > risk that memory space will be used out. To address it, the memory
> > service add logic to put part (at most 64 pages a time) of freed pages
> > back into page pool, so that the memory service can still have memory
> > to allocate, when all memory space have been allocated once. This is
> > called memory promotion. The promoted pages are always from the eldest
> > pages which haven been freed.
> >
> > This feature brings another problem is that memory map descriptors will
> > be increased enormously (200+ -> 2000+). One of change in this patch
> > is to update MergeMemoryMap() in file PropertiesTable.c to allow merge
> > freed pages back into the memory map. Now the number can stay at around
> > 510.
> >
> > Cc: Star Zeng
> > Cc: Michael D Kinney
> > Cc: Jiewen Yao
> > Cc: Ruiyu Ni
> > Cc: Laszlo Ersek
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Jian J Wang
> > ---
> > MdeModulePkg/Core/Dxe/Mem/HeapGuard.c | 409
> +-
> > MdeModulePkg/Core/Dxe/Mem/HeapGuard.h | 65 +++-
> > MdeModulePkg/Core/Dxe/Mem/Page.c | 41 ++-
> > MdeModulePkg/Core/Dxe/Mem/Pool.c | 23 +-
> > MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 2 +-
> > MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c | 18 +-
> > 6 files changed, 524 insertions(+), 34 deletions(-)
> >
> > diff --git a/MdeModulePkg/Core/Dxe/Mem/HeapGuard.c
> b/MdeModulePkg/Core/Dxe/Mem/HeapGuard.c
> > index 663f969c0d..449a022658 100644
> > --- a/MdeModulePkg/Core/Dxe/Mem/HeapGuard.c
> > +++ b/MdeModulePkg/Core/Dxe/Mem/HeapGuard.c
> > @@ -44,6 +44,11 @@ GLOBAL_REMOVE_IF_UNREFERENCED UINTN
> mLevelShift[GUARDED_HEAP_MAP_TABLE_DEPTH]
> > GLOBAL_REMOVE_IF_UNREFERENCED UINTN
> mLevelMask[GUARDED_HEAP_MAP_TABLE_DEPTH]
> > = GUARDED_HEAP_MAP_TABLE_DEPTH_MASKS;
> >
> > +//
> > +// Used for promoting freed but not used pages.
> > +//
> > +GLOBAL_REMOVE_IF_UNREFERENCED EFI_PHYSICAL_ADDRESS
> mLastPromotedPage = BASE_4GB;
> > +
> > /**
> > Set corresponding bits in bitmap table to 1 according to the address.
> >
> > @@ -379,7 +384,7 @@ ClearGuardedMemoryBits (
> >
> > @return An integer containing the guarded memory bitmap.
> > **/
> > -UINTN
> > +UINT64
> > GetGuardedMemoryBits (
> > IN EFI_PHYSICAL_ADDRESSAddress,
> > IN UINTN NumberOfPages
> > @@ -387,7 +392,7 @@ GetGuardedMemoryBits (
> > {
> > UINT64*BitMap;
> > UINTN Bits;
> > - UINTN Result;
> > + UINT64Result;
> > UINTN Shift;
> > UINTN BitsToUnitEnd;
> >
> > @@ -660,15 +665,16 @@ IsPageTypeToGuard (
> > /**
> > Check to see if the heap guard is enabled for page and/or pool
> > allocation.
> >
> > + @param[in] GuardType Specify the sub-type(s) of Heap Guard.
> > +
> > @return TRUE/FALSE.
> > **/
> > BOOLEAN
> > IsHeapGuardEnabled (
> > - VOID
> > + UINT8 GuardType
> > )
> > {
> > - return IsMemoryTypeToGuard (EfiMaxMemoryType, AllocateAnyPages,
> > - GUARD_HEAP_TYPE_POOL|GUARD_HEAP_TYPE_PAGE);
> > + return IsMemoryTypeToGuard (EfiMaxMemoryType, AllocateAnyPages,
> GuardType);
> > }
> >
> > /**
> > @@ -1203,6 +1209,380 @@ SetAllGuardPages (
> > }
> > }
> >
> > +/**
> > +
Re: [edk2] [PATCH v3 5/6] MdeModulePkg/Core: prevent re-acquire GCD memory lock
Sure. I'll change them. Thanks.
Regards,
Jian
> -Original Message-
> From: Zeng, Star
> Sent: Thursday, October 25, 2018 11:23 AM
> To: Wang, Jian J ; edk2-devel@lists.01.org
> Cc: Kinney, Michael D ; Ni, Ruiyu
> ; Yao, Jiewen ; Laszlo Ersek
> ; Zeng, Star
> Subject: Re: [edk2] [PATCH v3 5/6] MdeModulePkg/Core: prevent re-acquire
> GCD memory lock
>
> Some minor comments are added below.
> With them addressed, Reviewed-by: Star Zeng .
>
> On 2018/10/24 13:26, Jian J Wang wrote:
> >> v3 changes:
> >> a. drop the changes to CoreGetIoSpaceMap() because it won't cause
> >> problem
> >> b. refine the logic in changes of CoreGetMemorySpaceMap()
> >> and add more comments
> >
> > This issue is hidden in current code but exposed by introduction
> > of freed-memory guard feature due to the fact that the feature
> > will turn all pool allocation to page allocation.
> >
> > The solution is move the memory allocation in CoreGetMemorySpaceMap()
>
> Use 'moving' instead of 'move'?
>
> > and CoreGetIoSpaceMap() to be out of the GCD memory map lock.
>
> Please remove CoreGetIoSpaceMap() as the code does not touch it at all.
>
> >
> > Although it's rare, a while-loop is added to make sure that the count
> > of descriptor of memory map is the same after memory allocation, because
> > it's executed outside the GCD memory lock.
> >
> > CoreDumpGcdMemorySpaceMap()
> > => CoreGetMemorySpaceMap()
> > => CoreAcquireGcdMemoryLock () *
> > AllocatePool()
> > => InternalAllocatePool()
> > => CoreAllocatePool()
> > => CoreAllocatePoolI()
> > => CoreAllocatePoolPagesI()
> > => CoreAllocatePoolPages()
> > => FindFreePages()
> > => PromoteMemoryResource()
> > => CoreAcquireGcdMemoryLock() **
> >
> > Cc: Star Zeng
> > Cc: Michael D Kinney
> > Cc: Jiewen Yao
> > Cc: Ruiyu Ni
> > Cc: Laszlo Ersek
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Jian J Wang
> > ---
> > MdeModulePkg/Core/Dxe/Gcd/Gcd.c | 80 +++-
> -
> > 1 file changed, 54 insertions(+), 26 deletions(-)
> >
> > diff --git a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c
> b/MdeModulePkg/Core/Dxe/Gcd/Gcd.c
> > index d9c65a8045..bc02945721 100644
> > --- a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c
> > +++ b/MdeModulePkg/Core/Dxe/Gcd/Gcd.c
> > @@ -1691,10 +1691,10 @@ CoreGetMemorySpaceMap (
> > OUT EFI_GCD_MEMORY_SPACE_DESCRIPTOR **MemorySpaceMap
> > )
> > {
> > - EFI_STATUS Status;
> > LIST_ENTRY *Link;
> > EFI_GCD_MAP_ENTRY*Entry;
> > EFI_GCD_MEMORY_SPACE_DESCRIPTOR *Descriptor;
> > + UINTNDescriptorCount;
> >
> > //
> > // Make sure parameters are valid
> > @@ -1706,38 +1706,66 @@ CoreGetMemorySpaceMap (
> > return EFI_INVALID_PARAMETER;
> > }
> >
> > + *NumberOfDescriptors = 0;
> > + *MemorySpaceMap = NULL;
> > +
> > CoreAcquireGcdMemoryLock ();
>
> Better to add comment for it like below quoted from the example at
> https://lists.01.org/pipermail/edk2-devel/2018-October/031279.html given
> by Laszlo.
>
> //
> // Take the lock, for entering the loop with the lock held.
> //
>
> >
> > - //
> > - // Count the number of descriptors
> > - //
> > - *NumberOfDescriptors = CoreCountGcdMapEntry
> ();
> > + while (TRUE) {
> > +//
> > +// Count the number of descriptors. It might be done more than once
> because
>
> Use 'count' instead of 'number' to match the variable name?
>
> > +// there's code which has to be running outside the GCD lock.
>
> Use "AllocatePool() calling code below" instead of "there's code"?
>
> > +//
> > +DescriptorCount = CoreCountGcdMapEntry ();
> > +if (DescriptorCount == *NumberOfDescriptors) {
> > + //
> > + // Fill in the MemorySpaceMap if no memory space map change.
> > + //
> > + Descriptor = *MemorySpaceMap;
> > + Link = mGcdMemorySpaceMap.ForwardLink;
> > + while (Link != ) {
> > +Entry = CR (Link, EFI_GCD_MAP_ENTRY, Link,
> EFI_GCD_MAP_SIGNATURE);
> > +BuildMemoryDescriptor (Descriptor, Entry);
> > +Descriptor++;
> > +Link = Link->ForwardLink;
> > + }
> >
> > - //
> > - // Allocate the MemorySpaceMap
> > - //
> > - *MemorySpaceMap = AllocatePool (*NumberOfDescriptors * sizeof
> (EFI_GCD_MEMORY_SPACE_DESCRIPTOR));
> > - if (*MemorySpaceMap == NULL) {
> > -Status = EFI_OUT_OF_RESOURCES;
> > -goto Done;
> > - }
> > + break;
> > +}
> >
> > - //
> > - // Fill in the MemorySpaceMap
> > - //
> > - Descriptor = *MemorySpaceMap;
> > - Link = mGcdMemorySpaceMap.ForwardLink;
> > - while (Link != ) {
> > -Entry = CR (Link, EFI_GCD_MAP_ENTRY, Link, EFI_GCD_MAP_SIGNATURE);
> > -BuildMemoryDescriptor (Descriptor, Entry);
> > -Descriptor++;
> > -Link = Link->ForwardLink;
> > +//
> > +// Release the lock before memory allocation, because it might cause
> > +// GCD lock conflict in one
Re: [edk2] [PATCH v3 2/6] MdeModulePkg: introduce UEFI freed-memory guard bit in HeapGuard PCD
Star, Regards, Jian > -Original Message- > From: Zeng, Star > Sent: Thursday, October 25, 2018 11:02 AM > To: Wang, Jian J ; edk2-devel@lists.01.org > Cc: Kinney, Michael D ; Ni, Ruiyu > ; Yao, Jiewen ; Laszlo Ersek > ; Zeng, Star > Subject: Re: [edk2] [PATCH v3 2/6] MdeModulePkg: introduce UEFI freed- > memory guard bit in HeapGuard PCD > > On 2018/10/24 13:26, Jian J Wang wrote: > >> v3 changes: > >> a. split from v2 #1 patch file. > >> b. refine the commit message and title. > > > > UAF (Use-After-Free) memory issue is kind of illegal access to memory > > which has been freed. It can be detected by a new freed-memory guard > > enforced onto freed memory. > > > > BIT4 of following PCD is used to enable the freed-memory guard feature. > > > >gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPropertyMask > > > > Please note this feature is for debug purpose and should not be enabled > > Suggest also adding this information into the PCD description. > Pool/page heap guard also has same condition, right? > If yes, we can have a generic sentence for whole PCD. > > With this addressed, Reviewed-by: Star Zeng . > Sure. I'll update the dec/uni file with it. Thanks. > > Thanks, > Star > > > in product BIOS, and cannot be enabled with pool/page heap guard at the > > same time. It's disabled by default. > > > > Cc: Star Zeng > > Cc: Michael D Kinney > > Cc: Jiewen Yao > > Cc: Ruiyu Ni > > Cc: Laszlo Ersek > > Contributed-under: TianoCore Contribution Agreement 1.1 > > Signed-off-by: Jian J Wang > > --- > > MdeModulePkg/MdeModulePkg.dec | 6 ++ > > MdeModulePkg/MdeModulePkg.uni | 4 +++- > > 2 files changed, 9 insertions(+), 1 deletion(-) > > > > diff --git a/MdeModulePkg/MdeModulePkg.dec > b/MdeModulePkg/MdeModulePkg.dec > > index 2009dbc5fd..255b92ea67 100644 > > --- a/MdeModulePkg/MdeModulePkg.dec > > +++ b/MdeModulePkg/MdeModulePkg.dec > > @@ -1011,14 +1011,20 @@ > > > gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPoolType|0x0|UINT64|0x30 > 001053 > > > > ## This mask is to control Heap Guard behavior. > > + # > > # Note that due to the limit of pool memory implementation and the > alignment > > # requirement of UEFI spec, BIT7 is a try-best setting which cannot > guarantee > > # that the returned pool is exactly adjacent to head guard page or tail > > guard > > # page. > > + # > > + # Note that UEFI freed-memory guard and pool/page guard cannot be > enabled > > + # at the same time. > > + # > > # BIT0 - Enable UEFI page guard. > > # BIT1 - Enable UEFI pool guard. > > # BIT2 - Enable SMM page guard. > > # BIT3 - Enable SMM pool guard. > > + # BIT4 - Enable UEFI freed-memory guard (Use-After-Free memory > detection). > > # BIT6 - Enable non-stop mode. > > # BIT7 - The direction of Guard Page for Pool Guard. > > # 0 - The returned pool is near the tail guard page. > > diff --git a/MdeModulePkg/MdeModulePkg.uni > b/MdeModulePkg/MdeModulePkg.uni > > index 9d2e473fa9..e72b893509 100644 > > --- a/MdeModulePkg/MdeModulePkg.uni > > +++ b/MdeModulePkg/MdeModulePkg.uni > > @@ -1227,11 +1227,13 @@ > > > > "Note that due to the limit > of pool memory implementation and the alignment\n" > > > > "requirement of UEFI spec, > BIT7 is a try-best setting which cannot guarantee\n" > > > > "that the returned pool is > exactly adjacent to head guard page or tail guard\n" > > - > > "page.\n" > > + > > "page.\n\n" > > + > > "Note that UEFI freed- > memory guard and pool/page guard cannot be enabled at the same time.\n\n" > > > > " BIT0 - Enable UEFI page > guard.\n" > > > > " BIT1 - Enable UEFI pool > guard.\n" > > > > " BIT2 - Enable SMM page > guard.\n" > > > > " BIT3 - Enable SMM pool > guard.\n" > > + > > " BIT4 - Enable UEFI > freed-memory guard (Use-After-Free memory detection).\n" > > > > " BIT7 - The
Re: [edk2] [PATCH v3 1/6] MdeModulePkg: cleanup Heap Guard pool/page type PCD documentation
Got it. Thanks. Regards, Jian > -Original Message- > From: Zeng, Star > Sent: Thursday, October 25, 2018 10:56 AM > To: Wang, Jian J ; edk2-devel@lists.01.org > Cc: Kinney, Michael D ; Ni, Ruiyu > ; Yao, Jiewen ; Laszlo Ersek > ; Zeng, Star > Subject: Re: [edk2] [PATCH v3 1/6] MdeModulePkg: cleanup Heap Guard > pool/page type PCD documentation > > On 2018/10/24 13:26, Jian J Wang wrote: > >> v3 changes: > >> a. split from #1 patch of v2 > >> b. update title > > > > This cleanup is meant for avoiding misuse of newly introduced BIT4 > > (UAF detection) of PCD PcdHeapGuardPropertyMask, because it applies > > to all types of physical memory. In another words, > > PcdHeapGuardPoolType and PcdHeapGuardPageType don't have effect to > > the BIT4 of PcdHeapGuardPropertyMask. > > > > Cc: Star Zeng > > Cc: Michael D Kinney > > Cc: Jiewen Yao > > Cc: Ruiyu Ni > > Cc: Laszlo Ersek > > Contributed-under: TianoCore Contribution Agreement 1.1 > > Signed-off-by: Jian J Wang > > Reviewed-by: Star Zeng > > You may can add Laszlo's RB and even Suggested-by according to Laszlo's > feedback to V2 patch series. > > > Thanks, > Star > > > --- > > MdeModulePkg/MdeModulePkg.dec | 4 > > MdeModulePkg/MdeModulePkg.uni | 2 ++ > > 2 files changed, 6 insertions(+) > > > > diff --git a/MdeModulePkg/MdeModulePkg.dec > b/MdeModulePkg/MdeModulePkg.dec > > index 6037504fa7..2009dbc5fd 100644 > > --- a/MdeModulePkg/MdeModulePkg.dec > > +++ b/MdeModulePkg/MdeModulePkg.dec > > @@ -955,6 +955,8 @@ > > # free pages for all of them. The page allocation for the type related > > to > > # cleared bits keeps the same as ususal. > > # > > + # This PCD is only valid if BIT0 and/or BIT2 are set in > PcdHeapGuardPropertyMask. > > + # > > # Below is bit mask for this PCD: (Order is same as UEFI spec) > > # EfiReservedMemoryType 0x0001 > > # EfiLoaderCode 0x0002 > > @@ -984,6 +986,8 @@ > > # if there's enough free memory for all of them. The pool allocation > > for the > > # type related to cleared bits keeps the same as ususal. > > # > > + # This PCD is only valid if BIT1 and/or BIT3 are set in > PcdHeapGuardPropertyMask. > > + # > > # Below is bit mask for this PCD: (Order is same as UEFI spec) > > # EfiReservedMemoryType 0x0001 > > # EfiLoaderCode 0x0002 > > diff --git a/MdeModulePkg/MdeModulePkg.uni > b/MdeModulePkg/MdeModulePkg.uni > > index a6bcb627cf..9d2e473fa9 100644 > > --- a/MdeModulePkg/MdeModulePkg.uni > > +++ b/MdeModulePkg/MdeModulePkg.uni > > @@ -1171,6 +1171,7 @@ > > > > " before and after > corresponding type of pages allocated if there's enough\n" > > > > " free pages for all of them. > The page allocation for the type related to\n" > > > > " cleared bits keeps the same > as ususal.\n\n" > > + > > " This PCD is only valid if BIT0 > and/or BIT2 are set in PcdHeapGuardPropertyMask.\n\n" > > > > " Below is bit mask for this > PCD: (Order is same as UEFI spec)\n" > > > > " EfiReservedMemoryType > 0x0001\n" > > > > " EfiLoaderCode > 0x0002\n" > > @@ -1198,6 +1199,7 @@ > > > > " before and after > corresponding type of pages which the allocated pool occupies,\n" > > > > " if there's enough free > memory for all of them. The pool allocation for the\n" > > > > " type related to cleared bits > keeps the same as ususal.\n\n" > > + > > " This PCD is only valid if BIT1 > and/or BIT3 are set in PcdHeapGuardPropertyMask.\n\n" > > > > " Below is bit mask for this > PCD: (Order is same as UEFI spec)\n" > > > > " EfiReservedMemoryType > 0x0001\n" > > > > " EfiLoaderCode
Re: [edk2] [PATCH v3 6/6] MdeModulePkg/Core: add freed-memory guard feature
On 2018/10/24 13:26, Jian J Wang wrote:
v3 changes:
a. Merge from #4 and #5 of v2 patch
b. Coding style cleanup
Freed-memory guard is used to detect UAF (Use-After-Free) memory issue
which is illegal access to memory which has been freed. The principle
behind is similar to heap guard feature, that is we'll turn all pool
Since we also regard UAF part of heap guard feature, better to use
"pool/page heap guard feature" instead of "heap guard feature" here.
I quoted a piece of code at below and have question that why it uses
hard code Attribute parameter?
+ CoreAddRange (
+EfiConventionalMemory,
+StartAddress,
+EndAddress,
+EFI_MEMORY_UC | EFI_MEMORY_WC | EFI_MEMORY_WT | EFI_MEMORY_WB
+);
Thanks,
Star
memory allocation to page allocation and mark them to be not-present
once they are freed. The freed memory block will not be added back into
memory pool.
This means that, once a page is allocated and freed, it cannot be
re-allocated. This will bring an issue, which is that there's
risk that memory space will be used out. To address it, the memory
service add logic to put part (at most 64 pages a time) of freed pages
back into page pool, so that the memory service can still have memory
to allocate, when all memory space have been allocated once. This is
called memory promotion. The promoted pages are always from the eldest
pages which haven been freed.
This feature brings another problem is that memory map descriptors will
be increased enormously (200+ -> 2000+). One of change in this patch
is to update MergeMemoryMap() in file PropertiesTable.c to allow merge
freed pages back into the memory map. Now the number can stay at around
510.
Cc: Star Zeng
Cc: Michael D Kinney
Cc: Jiewen Yao
Cc: Ruiyu Ni
Cc: Laszlo Ersek
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jian J Wang
---
MdeModulePkg/Core/Dxe/Mem/HeapGuard.c | 409 +-
MdeModulePkg/Core/Dxe/Mem/HeapGuard.h | 65 +++-
MdeModulePkg/Core/Dxe/Mem/Page.c | 41 ++-
MdeModulePkg/Core/Dxe/Mem/Pool.c | 23 +-
MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 2 +-
MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c | 18 +-
6 files changed, 524 insertions(+), 34 deletions(-)
diff --git a/MdeModulePkg/Core/Dxe/Mem/HeapGuard.c
b/MdeModulePkg/Core/Dxe/Mem/HeapGuard.c
index 663f969c0d..449a022658 100644
--- a/MdeModulePkg/Core/Dxe/Mem/HeapGuard.c
+++ b/MdeModulePkg/Core/Dxe/Mem/HeapGuard.c
@@ -44,6 +44,11 @@ GLOBAL_REMOVE_IF_UNREFERENCED UINTN
mLevelShift[GUARDED_HEAP_MAP_TABLE_DEPTH]
GLOBAL_REMOVE_IF_UNREFERENCED UINTN mLevelMask[GUARDED_HEAP_MAP_TABLE_DEPTH]
= GUARDED_HEAP_MAP_TABLE_DEPTH_MASKS;
+//
+// Used for promoting freed but not used pages.
+//
+GLOBAL_REMOVE_IF_UNREFERENCED EFI_PHYSICAL_ADDRESS mLastPromotedPage =
BASE_4GB;
+
/**
Set corresponding bits in bitmap table to 1 according to the address.
@@ -379,7 +384,7 @@ ClearGuardedMemoryBits (
@return An integer containing the guarded memory bitmap.
**/
-UINTN
+UINT64
GetGuardedMemoryBits (
IN EFI_PHYSICAL_ADDRESSAddress,
IN UINTN NumberOfPages
@@ -387,7 +392,7 @@ GetGuardedMemoryBits (
{
UINT64*BitMap;
UINTN Bits;
- UINTN Result;
+ UINT64Result;
UINTN Shift;
UINTN BitsToUnitEnd;
@@ -660,15 +665,16 @@ IsPageTypeToGuard (
/**
Check to see if the heap guard is enabled for page and/or pool allocation.
+ @param[in] GuardType Specify the sub-type(s) of Heap Guard.
+
@return TRUE/FALSE.
**/
BOOLEAN
IsHeapGuardEnabled (
- VOID
+ UINT8 GuardType
)
{
- return IsMemoryTypeToGuard (EfiMaxMemoryType, AllocateAnyPages,
- GUARD_HEAP_TYPE_POOL|GUARD_HEAP_TYPE_PAGE);
+ return IsMemoryTypeToGuard (EfiMaxMemoryType, AllocateAnyPages, GuardType);
}
/**
@@ -1203,6 +1209,380 @@ SetAllGuardPages (
}
}
+/**
+ Find the address of top-most guarded free page.
+
+ @param[out] AddressStart address of top-most guarded free page.
+
+ @return VOID.
+**/
+VOID
+GetLastGuardedFreePageAddress (
+ OUT EFI_PHYSICAL_ADDRESS *Address
+ )
+{
+ EFI_PHYSICAL_ADDRESSAddressGranularity;
+ EFI_PHYSICAL_ADDRESSBaseAddress;
+ UINTN Level;
+ UINT64 Map;
+ INTNIndex;
+
+ ASSERT (mMapLevel >= 1);
+
+ BaseAddress = 0;
+ Map = mGuardedMemoryMap;
+ for (Level = GUARDED_HEAP_MAP_TABLE_DEPTH - mMapLevel;
+ Level < GUARDED_HEAP_MAP_TABLE_DEPTH;
+ ++Level) {
+AddressGranularity = LShiftU64 (1, mLevelShift[Level]);
+
+//
+// Find the non-NULL entry at largest index.
+//
+for (Index = (INTN)mLevelMask[Level]; Index >= 0 ; --Index) {
+ if (((UINT64 *)(UINTN)Map)[Index] != 0) {
+BaseAddress +=
Re: [edk2] [PATCH v3 5/6] MdeModulePkg/Core: prevent re-acquire GCD memory lock
Some minor comments are added below.
With them addressed, Reviewed-by: Star Zeng .
On 2018/10/24 13:26, Jian J Wang wrote:
v3 changes:
a. drop the changes to CoreGetIoSpaceMap() because it won't cause
problem
b. refine the logic in changes of CoreGetMemorySpaceMap()
and add more comments
This issue is hidden in current code but exposed by introduction
of freed-memory guard feature due to the fact that the feature
will turn all pool allocation to page allocation.
The solution is move the memory allocation in CoreGetMemorySpaceMap()
Use 'moving' instead of 'move'?
and CoreGetIoSpaceMap() to be out of the GCD memory map lock.
Please remove CoreGetIoSpaceMap() as the code does not touch it at all.
Although it's rare, a while-loop is added to make sure that the count
of descriptor of memory map is the same after memory allocation, because
it's executed outside the GCD memory lock.
CoreDumpGcdMemorySpaceMap()
=> CoreGetMemorySpaceMap()
=> CoreAcquireGcdMemoryLock () *
AllocatePool()
=> InternalAllocatePool()
=> CoreAllocatePool()
=> CoreAllocatePoolI()
=> CoreAllocatePoolPagesI()
=> CoreAllocatePoolPages()
=> FindFreePages()
=> PromoteMemoryResource()
=> CoreAcquireGcdMemoryLock() **
Cc: Star Zeng
Cc: Michael D Kinney
Cc: Jiewen Yao
Cc: Ruiyu Ni
Cc: Laszlo Ersek
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jian J Wang
---
MdeModulePkg/Core/Dxe/Gcd/Gcd.c | 80 +++--
1 file changed, 54 insertions(+), 26 deletions(-)
diff --git a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c b/MdeModulePkg/Core/Dxe/Gcd/Gcd.c
index d9c65a8045..bc02945721 100644
--- a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c
+++ b/MdeModulePkg/Core/Dxe/Gcd/Gcd.c
@@ -1691,10 +1691,10 @@ CoreGetMemorySpaceMap (
OUT EFI_GCD_MEMORY_SPACE_DESCRIPTOR **MemorySpaceMap
)
{
- EFI_STATUS Status;
LIST_ENTRY *Link;
EFI_GCD_MAP_ENTRY*Entry;
EFI_GCD_MEMORY_SPACE_DESCRIPTOR *Descriptor;
+ UINTNDescriptorCount;
//
// Make sure parameters are valid
@@ -1706,38 +1706,66 @@ CoreGetMemorySpaceMap (
return EFI_INVALID_PARAMETER;
}
+ *NumberOfDescriptors = 0;
+ *MemorySpaceMap = NULL;
+
CoreAcquireGcdMemoryLock ();
Better to add comment for it like below quoted from the example at
https://lists.01.org/pipermail/edk2-devel/2018-October/031279.html given
by Laszlo.
//
// Take the lock, for entering the loop with the lock held.
//
- //
- // Count the number of descriptors
- //
- *NumberOfDescriptors = CoreCountGcdMapEntry ();
+ while (TRUE) {
+//
+// Count the number of descriptors. It might be done more than once because
Use 'count' instead of 'number' to match the variable name?
+// there's code which has to be running outside the GCD lock.
Use "AllocatePool() calling code below" instead of "there's code"?
+//
+DescriptorCount = CoreCountGcdMapEntry ();
+if (DescriptorCount == *NumberOfDescriptors) {
+ //
+ // Fill in the MemorySpaceMap if no memory space map change.
+ //
+ Descriptor = *MemorySpaceMap;
+ Link = mGcdMemorySpaceMap.ForwardLink;
+ while (Link != ) {
+Entry = CR (Link, EFI_GCD_MAP_ENTRY, Link, EFI_GCD_MAP_SIGNATURE);
+BuildMemoryDescriptor (Descriptor, Entry);
+Descriptor++;
+Link = Link->ForwardLink;
+ }
- //
- // Allocate the MemorySpaceMap
- //
- *MemorySpaceMap = AllocatePool (*NumberOfDescriptors * sizeof
(EFI_GCD_MEMORY_SPACE_DESCRIPTOR));
- if (*MemorySpaceMap == NULL) {
-Status = EFI_OUT_OF_RESOURCES;
-goto Done;
- }
+ break;
+}
- //
- // Fill in the MemorySpaceMap
- //
- Descriptor = *MemorySpaceMap;
- Link = mGcdMemorySpaceMap.ForwardLink;
- while (Link != ) {
-Entry = CR (Link, EFI_GCD_MAP_ENTRY, Link, EFI_GCD_MAP_SIGNATURE);
-BuildMemoryDescriptor (Descriptor, Entry);
-Descriptor++;
-Link = Link->ForwardLink;
+//
+// Release the lock before memory allocation, because it might cause
+// GCD lock conflict in one of calling path in AllocatPool().
+//
+CoreReleaseGcdMemoryLock ();
+
+//
+// Allocate memory to store the MemorySpaceMap. Note it might be already
+// allocated if there's map descriptor change during memory allocation at
+// last time.
+//
+if (*MemorySpaceMap != NULL) {
+ FreePool (*MemorySpaceMap);
+}
+
+*MemorySpaceMap = AllocatePool (DescriptorCount *
+sizeof (EFI_GCD_MEMORY_SPACE_DESCRIPTOR));
+if (*MemorySpaceMap == NULL) {
+ *NumberOfDescriptors = 0;
+ return EFI_OUT_OF_RESOURCES;
+}
+
+//
+// Save the descriptor number got before for another round of check to make
Use 'count' instead of 'number' to match the variable name?
+// sure we won't miss any, since we have code running outside the GCD lock.
+//
+
Re: [edk2] [PATCH v3 2/6] MdeModulePkg: introduce UEFI freed-memory guard bit in HeapGuard PCD
On 2018/10/24 13:26, Jian J Wang wrote: v3 changes: a. split from v2 #1 patch file. b. refine the commit message and title. UAF (Use-After-Free) memory issue is kind of illegal access to memory which has been freed. It can be detected by a new freed-memory guard enforced onto freed memory. BIT4 of following PCD is used to enable the freed-memory guard feature. gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPropertyMask Please note this feature is for debug purpose and should not be enabled Suggest also adding this information into the PCD description. Pool/page heap guard also has same condition, right? If yes, we can have a generic sentence for whole PCD. With this addressed, Reviewed-by: Star Zeng . Thanks, Star in product BIOS, and cannot be enabled with pool/page heap guard at the same time. It's disabled by default. Cc: Star Zeng Cc: Michael D Kinney Cc: Jiewen Yao Cc: Ruiyu Ni Cc: Laszlo Ersek Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang --- MdeModulePkg/MdeModulePkg.dec | 6 ++ MdeModulePkg/MdeModulePkg.uni | 4 +++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec index 2009dbc5fd..255b92ea67 100644 --- a/MdeModulePkg/MdeModulePkg.dec +++ b/MdeModulePkg/MdeModulePkg.dec @@ -1011,14 +1011,20 @@ gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPoolType|0x0|UINT64|0x30001053 ## This mask is to control Heap Guard behavior. + # # Note that due to the limit of pool memory implementation and the alignment # requirement of UEFI spec, BIT7 is a try-best setting which cannot guarantee # that the returned pool is exactly adjacent to head guard page or tail guard # page. + # + # Note that UEFI freed-memory guard and pool/page guard cannot be enabled + # at the same time. + # # BIT0 - Enable UEFI page guard. # BIT1 - Enable UEFI pool guard. # BIT2 - Enable SMM page guard. # BIT3 - Enable SMM pool guard. + # BIT4 - Enable UEFI freed-memory guard (Use-After-Free memory detection). # BIT6 - Enable non-stop mode. # BIT7 - The direction of Guard Page for Pool Guard. # 0 - The returned pool is near the tail guard page. diff --git a/MdeModulePkg/MdeModulePkg.uni b/MdeModulePkg/MdeModulePkg.uni index 9d2e473fa9..e72b893509 100644 --- a/MdeModulePkg/MdeModulePkg.uni +++ b/MdeModulePkg/MdeModulePkg.uni @@ -1227,11 +1227,13 @@ "Note that due to the limit of pool memory implementation and the alignment\n" "requirement of UEFI spec, BIT7 is a try-best setting which cannot guarantee\n" "that the returned pool is exactly adjacent to head guard page or tail guard\n" - "page.\n" + "page.\n\n" + "Note that UEFI freed-memory guard and pool/page guard cannot be enabled at the same time.\n\n" " BIT0 - Enable UEFI page guard.\n" " BIT1 - Enable UEFI pool guard.\n" " BIT2 - Enable SMM page guard.\n" " BIT3 - Enable SMM pool guard.\n" + " BIT4 - Enable UEFI freed-memory guard (Use-After-Free memory detection).\n" " BIT7 - The direction of Guard Page for Pool Guard.\n" " 0 - The returned pool is near the tail guard page.\n" " 1 - The returned pool is near the head guard page." ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v3 1/6] MdeModulePkg: cleanup Heap Guard pool/page type PCD documentation
On 2018/10/24 13:26, Jian J Wang wrote: v3 changes: a. split from #1 patch of v2 b. update title This cleanup is meant for avoiding misuse of newly introduced BIT4 (UAF detection) of PCD PcdHeapGuardPropertyMask, because it applies to all types of physical memory. In another words, PcdHeapGuardPoolType and PcdHeapGuardPageType don't have effect to the BIT4 of PcdHeapGuardPropertyMask. Cc: Star Zeng Cc: Michael D Kinney Cc: Jiewen Yao Cc: Ruiyu Ni Cc: Laszlo Ersek Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang Reviewed-by: Star Zeng You may can add Laszlo's RB and even Suggested-by according to Laszlo's feedback to V2 patch series. Thanks, Star --- MdeModulePkg/MdeModulePkg.dec | 4 MdeModulePkg/MdeModulePkg.uni | 2 ++ 2 files changed, 6 insertions(+) diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec index 6037504fa7..2009dbc5fd 100644 --- a/MdeModulePkg/MdeModulePkg.dec +++ b/MdeModulePkg/MdeModulePkg.dec @@ -955,6 +955,8 @@ # free pages for all of them. The page allocation for the type related to # cleared bits keeps the same as ususal. # + # This PCD is only valid if BIT0 and/or BIT2 are set in PcdHeapGuardPropertyMask. + # # Below is bit mask for this PCD: (Order is same as UEFI spec) # EfiReservedMemoryType 0x0001 # EfiLoaderCode 0x0002 @@ -984,6 +986,8 @@ # if there's enough free memory for all of them. The pool allocation for the # type related to cleared bits keeps the same as ususal. # + # This PCD is only valid if BIT1 and/or BIT3 are set in PcdHeapGuardPropertyMask. + # # Below is bit mask for this PCD: (Order is same as UEFI spec) # EfiReservedMemoryType 0x0001 # EfiLoaderCode 0x0002 diff --git a/MdeModulePkg/MdeModulePkg.uni b/MdeModulePkg/MdeModulePkg.uni index a6bcb627cf..9d2e473fa9 100644 --- a/MdeModulePkg/MdeModulePkg.uni +++ b/MdeModulePkg/MdeModulePkg.uni @@ -1171,6 +1171,7 @@ " before and after corresponding type of pages allocated if there's enough\n" " free pages for all of them. The page allocation for the type related to\n" " cleared bits keeps the same as ususal.\n\n" + " This PCD is only valid if BIT0 and/or BIT2 are set in PcdHeapGuardPropertyMask.\n\n" " Below is bit mask for this PCD: (Order is same as UEFI spec)\n" " EfiReservedMemoryType 0x0001\n" " EfiLoaderCode 0x0002\n" @@ -1198,6 +1199,7 @@ " before and after corresponding type of pages which the allocated pool occupies,\n" " if there's enough free memory for all of them. The pool allocation for the\n" " type related to cleared bits keeps the same as ususal.\n\n" + " This PCD is only valid if BIT1 and/or BIT3 are set in PcdHeapGuardPropertyMask.\n\n" " Below is bit mask for this PCD: (Order is same as UEFI spec)\n" " EfiReservedMemoryType 0x0001\n" " EfiLoaderCode 0x0002\n" ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [Patch 2/6] UefiCpuPkg/CpuCommonFeaturesLib: Remove white space at line end.
Remove extra white space at the end of line. Cc: Ruiyu Ni Cc: Laszlo Ersek Cc: Dandan Bi Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Eric Dong --- UefiCpuPkg/Library/CpuCommonFeaturesLib/MachineCheck.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/UefiCpuPkg/Library/CpuCommonFeaturesLib/MachineCheck.c b/UefiCpuPkg/Library/CpuCommonFeaturesLib/MachineCheck.c index f8bee53819..57648352ec 100644 --- a/UefiCpuPkg/Library/CpuCommonFeaturesLib/MachineCheck.c +++ b/UefiCpuPkg/Library/CpuCommonFeaturesLib/MachineCheck.c @@ -328,7 +328,7 @@ LmceInitialize ( MSR_IA32_FEATURE_CONTROL_REGISTER*MsrRegister; // - // The scope of FastStrings bit in the MSR_IA32_MISC_ENABLE is core for below processor type, only program + // The scope of FastStrings bit in the MSR_IA32_MISC_ENABLE is core for below processor type, only program // MSR_IA32_MISC_ENABLE for thread 0 in each core. // if (IS_SILVERMONT_PROCESSOR (CpuInfo->DisplayFamily, CpuInfo->DisplayModel) || -- 2.15.0.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [Patch 1/6] UefiCpuPkg/RegisterCpuFeaturesLib: Fix build failure for VS2012 and GCC49.
Code initialized in function can't be correctly detected by build tool.
Add code to clearly initialize the local variable before use it.
Cc: Ruiyu Ni
Cc: Laszlo Ersek
Cc: Dandan Bi
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong
---
UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
index 7a5939c966..173f2edbea 100644
--- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
+++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
@@ -1029,6 +1029,7 @@ SetProcessorRegister (
InitApicId = GetInitialApicId ();
RegisterTable = NULL;
+ ProcIndex = (UINTN)-1;
for (Index = 0; Index < AcpiCpuData->NumberOfCpus; Index++) {
if (RegisterTables[Index].InitialApicId == InitApicId) {
RegisterTable = [Index];
--
2.15.0.windows.1
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [Patch 4/6] UefiCpuPkg/PiSmmCpuDxeSmm: Remove white space at line end.
Remove extra white space at the end of line. Cc: Ruiyu Ni Cc: Laszlo Ersek Cc: Dandan Bi Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Eric Dong --- UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c index fec53c522f..5193fea2b3 100644 --- a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c @@ -346,7 +346,7 @@ ProgramProcessorRegister ( // n * P(0) n * P(1) ... n * P(n) // ASSERT ( -(ApLocation != NULL) && +(ApLocation != NULL) && (CpuStatus->ValidCoreCountPerPackage != 0) && (CpuFlags->SemaphoreCount) != NULL ); @@ -428,7 +428,7 @@ ProgramProcessorRegister ( /** Set Processor register for one AP. - + @param PreSmmRegisterTable Use pre Smm register table or register table. **/ -- 2.15.0.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [Patch 6/6] UefiCpuPkg/PiSmmCpuDxeSmm: Fix build failure for VS2012 and GCC49.
Code initialized in function can't be correctly detected by build tool.
Add code to clearly initialize the local variable before use it.
Cc: Ruiyu Ni
Cc: Laszlo Ersek
Cc: Dandan Bi
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong
---
UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
index 5193fea2b3..a45e2dd3d7 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
@@ -451,6 +451,7 @@ SetRegister (
InitApicId = GetInitialApicId ();
RegisterTable = NULL;
+ ProcIndex = (UINTN)-1;
for (Index = 0; Index < mAcpiCpuData.NumberOfCpus; Index++) {
if (RegisterTables[Index].InitialApicId == InitApicId) {
RegisterTable = [Index];
--
2.15.0.windows.1
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [Patch 5/6] UefiCpuPkg/PiSmmCpuDxeSmm: Clean up useless code.
Remove useless code after change 93324390.
Cc: Ruiyu Ni
Cc: Laszlo Ersek
Cc: Dandan Bi
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong
---
UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c | 10 +-
UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h | 16
2 files changed, 1 insertion(+), 25 deletions(-)
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
index 42b040531e..abcc3eea05 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
@@ -1272,7 +1272,6 @@ InitializeSmmCpuSemaphores (
UINTN TotalSize;
UINTN GlobalSemaphoresSize;
UINTN CpuSemaphoresSize;
- UINTN MsrSemahporeSize;
UINTN SemaphoreSize;
UINTN Pages;
UINTN *SemaphoreBlock;
@@ -1282,8 +1281,7 @@ InitializeSmmCpuSemaphores (
ProcessorCount = gSmmCpuPrivate->SmmCoreEntryContext.NumberOfCpus;
GlobalSemaphoresSize = (sizeof (SMM_CPU_SEMAPHORE_GLOBAL) / sizeof (VOID *))
* SemaphoreSize;
CpuSemaphoresSize= (sizeof (SMM_CPU_SEMAPHORE_CPU) / sizeof (VOID *)) *
ProcessorCount * SemaphoreSize;
- MsrSemahporeSize = MSR_SPIN_LOCK_INIT_NUM * SemaphoreSize;
- TotalSize = GlobalSemaphoresSize + CpuSemaphoresSize + MsrSemahporeSize;
+ TotalSize = GlobalSemaphoresSize + CpuSemaphoresSize;
DEBUG((EFI_D_INFO, "One Semaphore Size= 0x%x\n", SemaphoreSize));
DEBUG((EFI_D_INFO, "Total Semaphores Size = 0x%x\n", TotalSize));
Pages = EFI_SIZE_TO_PAGES (TotalSize);
@@ -1311,12 +1309,6 @@ InitializeSmmCpuSemaphores (
SemaphoreAddr += ProcessorCount * SemaphoreSize;
mSmmCpuSemaphores.SemaphoreCpu.Present = (BOOLEAN *)SemaphoreAddr;
- SemaphoreAddr = (UINTN)SemaphoreBlock + GlobalSemaphoresSize +
CpuSemaphoresSize;
- mSmmCpuSemaphores.SemaphoreMsr.Msr = (SPIN_LOCK *)SemaphoreAddr;
- mSmmCpuSemaphores.SemaphoreMsr.AvailableCounter =
-((UINTN)SemaphoreBlock + Pages * SIZE_4KB - SemaphoreAddr) /
SemaphoreSize;
- ASSERT (mSmmCpuSemaphores.SemaphoreMsr.AvailableCounter >=
MSR_SPIN_LOCK_INIT_NUM);
-
mPFLock = mSmmCpuSemaphores.SemaphoreGlobal.PFLock;
mConfigSmmCodeAccessCheckLock =
mSmmCpuSemaphores.SemaphoreGlobal.CodeAccessCheckLock;
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
index e2970308fe..61d4bd3085 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
@@ -347,13 +347,6 @@ typedef struct {
volatile BOOLEAN *CandidateBsp;
} SMM_DISPATCHER_MP_SYNC_DATA;
-#define MSR_SPIN_LOCK_INIT_NUM 15
-
-typedef struct {
- SPIN_LOCK*SpinLock;
- UINT32 MsrIndex;
-} MP_MSR_LOCK;
-
#define SMM_PSD_OFFSET 0xfb00
///
@@ -376,21 +369,12 @@ typedef struct {
volatile BOOLEAN *Present;
} SMM_CPU_SEMAPHORE_CPU;
-///
-/// All MSRs semaphores' pointer and counter
-///
-typedef struct {
- SPIN_LOCK*Msr;
- UINTNAvailableCounter;
-} SMM_CPU_SEMAPHORE_MSR;
-
///
/// All semaphores' information
///
typedef struct {
SMM_CPU_SEMAPHORE_GLOBAL SemaphoreGlobal;
SMM_CPU_SEMAPHORE_CPU SemaphoreCpu;
- SMM_CPU_SEMAPHORE_MSR SemaphoreMsr;
} SMM_CPU_SEMAPHORES;
extern IA32_DESCRIPTOR gcSmiGdtr;
--
2.15.0.windows.1
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [Patch 0/6] Fix coding style issues.
Fixed ECC issues caused by change serial from d5aa2078 to d28daadd. Eric Dong (6): UefiCpuPkg/RegisterCpuFeaturesLib: Fix build failure for VS2012 and GCC49. UefiCpuPkg/CpuCommonFeaturesLib: Remove white space at line end. UefiCpuPkg/RegisterCpuFeaturesLib: Fix ECC issues. UefiCpuPkg/PiSmmCpuDxeSmm: Remove white space at line end. UefiCpuPkg/PiSmmCpuDxeSmm: Clean up useless code. UefiCpuPkg/PiSmmCpuDxeSmm: Fix build failure for VS2012 and GCC49. .../Library/CpuCommonFeaturesLib/MachineCheck.c| 2 +- .../RegisterCpuFeaturesLib/CpuFeaturesInitialize.c | 1 + .../PeiRegisterCpuFeaturesLib.c| 2 ++ .../RegisterCpuFeaturesLib.c | 24 +++--- UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c | 5 +++-- UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c | 10 + UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h | 16 --- 7 files changed, 20 insertions(+), 40 deletions(-) -- 2.15.0.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [Patch V2] BaseTools: Fix the bug for Pcd used in command line's override
Reviewed-by: Liming Gao
>-Original Message-
>From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
>Yonghong Zhu
>Sent: Wednesday, October 24, 2018 10:06 PM
>To: edk2-devel@lists.01.org
>Subject: [edk2] [Patch V2] BaseTools: Fix the bug for Pcd used in command
>line's override
>
>V2: remove the not used parameter i
>
>Fix the bug for Pcd used in command line not override the Pcd used
>in the [component] driver's sub-section.
>
>Case:
>DSC file:
>[PcdsFixedAtBuild]
>TokenSpaceGuid.PcdTest
>
>[Components]
> TestPkg/TestDriver.inf {
>
> TokenSpaceGuid.PcdTest|"b"
> }
>
>build command with --pcd TokenSpaceGuid.PcdTest="BB"
>
>Then we found the Pcd value in the AutoGen.c file is incorrect,
>because of the incorrect logic that use the pcd in the [component]
>section to re-override it.
>
>Contributed-under: TianoCore Contribution Agreement 1.1
>Signed-off-by: Yonghong Zhu
>---
> BaseTools/Source/Python/AutoGen/AutoGen.py| 7 +++
> BaseTools/Source/Python/Workspace/DscBuildData.py | 5 +
> 2 files changed, 12 insertions(+)
>
>diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py
>b/BaseTools/Source/Python/AutoGen/AutoGen.py
>index 804f579..84645e3 100644
>--- a/BaseTools/Source/Python/AutoGen/AutoGen.py
>+++ b/BaseTools/Source/Python/AutoGen/AutoGen.py
>@@ -2118,10 +2118,17 @@ class PlatformAutoGen(AutoGen):
>
> # override PCD settings with module specific setting
> if Module in self.Platform.Modules:
> PlatformModule = self.Platform.Modules[str(Module)]
> for Key in PlatformModule.Pcds:
>+if GlobalData.BuildOptionPcd:
>+for pcd in GlobalData.BuildOptionPcd:
>+(TokenSpaceGuidCName, TokenCName, FieldName,
>pcdvalue, _)
>= pcd
>+if (TokenCName, TokenSpaceGuidCName) == Key and
>FieldName =="":
>+PlatformModule.Pcds[Key].DefaultValue = pcdvalue
>+PlatformModule.Pcds[Key].PcdValueFromComm =
>pcdvalue
>+break
> Flag = False
> if Key in Pcds:
> ToPcd = Pcds[Key]
> Flag = True
> elif Key in GlobalData.MixedPcd:
>diff --git a/BaseTools/Source/Python/Workspace/DscBuildData.py
>b/BaseTools/Source/Python/Workspace/DscBuildData.py
>index b0e88a9..162360c 100644
>--- a/BaseTools/Source/Python/Workspace/DscBuildData.py
>+++ b/BaseTools/Source/Python/Workspace/DscBuildData.py
>@@ -1058,11 +1058,16 @@ class DscBuildData(PlatformBuildClassObject):
> IsValid, Cause = CheckPcdDatum(PcdDatumType, pcdvalue)
> if not IsValid:
> EdkLogger.error("build", FORMAT_INVALID, Cause,
>ExtraData="%s.%s" % (TokenSpaceGuidCName, TokenCName))
> GlobalData.BuildOptionPcd[i] = (TokenSpaceGuidCName,
>TokenCName, FieldName, pcdvalue, ("build command options", 1))
>
>+if GlobalData.BuildOptionPcd:
>+for pcd in GlobalData.BuildOptionPcd:
>+(TokenSpaceGuidCName, TokenCName, FieldName, pcdvalue, _) =
>pcd
> for BuildData in self._Bdb._CACHE_.values():
>+if BuildData.Arch != self.Arch:
>+continue
> if BuildData.MetaFile.Ext == '.dec' or
> BuildData.MetaFile.Ext ==
>'.dsc':
> continue
> for key in BuildData.Pcds:
> PcdItem = BuildData.Pcds[key]
> if (TokenSpaceGuidCName, TokenCName) ==
>(PcdItem.TokenSpaceGuidCName, PcdItem.TokenCName) and FieldName
>=="":
>--
>2.6.1.windows.1
>
>___
>edk2-devel mailing list
>edk2-devel@lists.01.org
>https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH 1/2] ShellPkg-Shell App: Provide fully-qualified path to shell scripts
Reviewed-by: Jaben Carsey Ray? I can push this one also if you don't see an issue. > -Original Message- > From: jim.dai...@dell.com [mailto:jim.dai...@dell.com] > Sent: Wednesday, October 24, 2018 9:35 AM > To: edk2-devel@lists.01.org > Cc: Ni, Ruiyu ; Carsey, Jaben > Subject: [edk2][PATCH 1/2] ShellPkg-Shell App: Provide fully-qualified path > to shell scripts > Importance: High > > Add a function to return the fully-qualified version of some path. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Jim Dailey > --- > ShellPkg/Include/Library/ShellLib.h | 40 + > ShellPkg/Library/UefiShellLib/UefiShellLib.c | 93 +++- > 2 files changed, 132 insertions(+), 1 deletion(-) > > diff --git a/ShellPkg/Include/Library/ShellLib.h > b/ShellPkg/Include/Library/ShellLib.h > index 92fddc50f5..cd7e9c47c3 100644 > --- a/ShellPkg/Include/Library/ShellLib.h > +++ b/ShellPkg/Include/Library/ShellLib.h > @@ -2,6 +2,7 @@ >Provides interface to shell functionality for shell commands and > applications. > >Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved. > + Copyright 2018 Dell Technologies. >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 > @@ -35,6 +36,45 @@ > extern EFI_SHELL_PARAMETERS_PROTOCOL *gEfiShellParametersProtocol; > extern EFI_SHELL_PROTOCOL*gEfiShellProtocol; > > +/** > + Return the fully-qualified version of a relative path or an absolute path > that > + does not include a file system reference. > + > + If ASSERTs are disabled, and if the input parameter is NULL or points to > NULL, > + then NULL is returned. > + > + If the input path contains a ":", this function assumes that it is part of > a > + reference to a file system (e.g. "FS0:"). In such a case, Path is cleaned > + and returned. > + > + If there is no working directory or there is not enough memory available to > + create the fully-qualified path, Path is cleaned and returned. > + > + Otherwise, the current file system or working directory (as appropriate) is > + prepended to Path. The input Path is freed and the resulting path is > cleaned, > + assigned to Path, and returned. > + > + NOTE: If the input path is an empty string, then the current working > directory > + (if it exists) is returned. In other words, an empty input path is treated > + exactly the same as ".". > + > + @param[in, out] Path On input, a pointer to some file or directory path. > On > +output, a pointer to the clean and possibly fully- > +qualified version of the input path. The input > pointer > +may be freed and reassigned on output. > + > + @retval NULL The input pointer or the path itself was NULL. > + > + @return A pointer to the clean, fully-qualified version of Path. If memory > + allocation fails, or if there is no working directory, then a > pointer > + to the clean, but not necessarily fully-qualified version of Path. > +**/ > +CHAR16* > +EFIAPI > +FullyQualifyPath( > + IN OUT CHAR16 **Path > + ); > + > /** >This function will retrieve the information about the file for the handle >specified and store it in allocated pool memory. > diff --git a/ShellPkg/Library/UefiShellLib/UefiShellLib.c > b/ShellPkg/Library/UefiShellLib/UefiShellLib.c > index f04adbb63f..52ca3ce1b1 100644 > --- a/ShellPkg/Library/UefiShellLib/UefiShellLib.c > +++ b/ShellPkg/Library/UefiShellLib/UefiShellLib.c > @@ -2,7 +2,7 @@ >Provides interface to shell functionality for shell commands and > applications. > >(C) Copyright 2016 Hewlett Packard Enterprise Development LP > - Copyright 2016 Dell Inc. > + Copyright 2016-2018 Dell Technologies. >Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved. >This program and the accompanying materials >are licensed and made available under the terms and conditions of the BSD > License > @@ -36,6 +36,97 @@ EFI_HANDLEmEfiShellEnvironment2Handle; > FILE_HANDLE_FUNCTION_MAP FileFunctionMap; > EFI_UNICODE_COLLATION_PROTOCOL *mUnicodeCollationProtocol; > > +/** > + Return the fully-qualified version of a relative path or an absolute path > that > + does not include a file system reference. > + > + If asserts are disabled, and if the input parameter is NULL or points to > NULL, > + then NULL is returned. > + > + If the input path contains a ":", this function assumes that it is part of > a > + reference to a file system (e.g. "FS0:"). In such a case, Path is cleaned > + and returned. > + > + If there is no working directory or there is not enough memory available to > + create the fully-qualified path, Path is cleaned and returned. > + > + Otherwise, the current
Re: [edk2] [PATCH 2/2] ShellPkg-Shell App: Provide fully-qualified path to shell scripts
Looks good to me.
Ray?
Reviewed-by: Jaben Carsey
p.s. Ray if you agree you can RB and I will handle the push.
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> jim.dai...@dell.com
> Sent: Wednesday, October 24, 2018 9:36 AM
> To: edk2-devel@lists.01.org
> Cc: Ni, Ruiyu ; Carsey, Jaben
> Subject: [edk2] [PATCH 2/2] ShellPkg-Shell App: Provide fully-qualified path
> to shell scripts
> Importance: High
>
> Section 3.6.2 of version 2.2 of the shell specification requires that
> the first positional argument (i.e. arg 0) of a shell script resolves
> to "the full path name of the script itself."
>
> Ensure that the startup script and any scripts launched by the shell
> meet this requirement.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Jim Dailey
> ---
> ShellPkg/Application/Shell/Shell.c | 12 ++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/ShellPkg/Application/Shell/Shell.c
> b/ShellPkg/Application/Shell/Shell.c
> index 6185b6ac80..fe88177d57 100644
> --- a/ShellPkg/Application/Shell/Shell.c
> +++ b/ShellPkg/Application/Shell/Shell.c
> @@ -3,6 +3,7 @@
>
>Copyright (c) 2009 - 2018, Intel Corporation. All rights reserved.
>(C) Copyright 2013-2014 Hewlett-Packard Development Company, L.P.
> + Copyright 2018 Dell Technologies.
>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
> @@ -1275,7 +1276,8 @@ DoStartupScript(
>
>FileStringPath = LocateStartupScript (ImagePath, FilePath);
>if (FileStringPath != NULL) {
> -Status = RunScriptFile (FileStringPath, NULL, L"",
> ShellInfoObject.NewShellParametersProtocol);
> +FileStringPath = FullyQualifyPath();
> +Status = RunScriptFile (FileStringPath, NULL, FileStringPath,
> ShellInfoObject.NewShellParametersProtocol);
> FreePool (FileStringPath);
>} else {
> //
> @@ -2474,6 +2476,7 @@ RunCommandOrFile(
>}
>switch (Type) {
> case Script_File_Name:
> + CommandWithPath = FullyQualifyPath();
>Status = RunScriptFile (CommandWithPath, NULL, CmdLine,
> ParamProtocol);
>break;
> case Efi_Application:
> @@ -2812,7 +2815,12 @@ RunScriptFileHandle (
>DeleteScriptFileStruct(NewScriptFile);
>return (EFI_OUT_OF_RESOURCES);
> }
> -for (LoopVar = 0 ; LoopVar < 10 && LoopVar < NewScriptFile->Argc;
> LoopVar++) {
> +//
> +// Put the full path of the script file into Argv[0] as required by
> section
> +// 3.6.2 of version 2.2 of the shell specification.
> +//
> +NewScriptFile->Argv[0] = StrnCatGrow(>Argv[0], NULL,
> NewScriptFile->ScriptName, 0);
> +for (LoopVar = 1 ; LoopVar < 10 && LoopVar < NewScriptFile->Argc;
> LoopVar++) {
>ASSERT(NewScriptFile->Argv[LoopVar] == NULL);
>NewScriptFile->Argv[LoopVar] = StrnCatGrow(
> >Argv[LoopVar], NULL, ShellInfoObject.NewShellParametersProtocol-
> >Argv[LoopVar], 0);
>if (NewScriptFile->Argv[LoopVar] == NULL) {
> --
> 2.17.0.windows.1
>
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH] CryptoPkg/BaseCryptLib: Fix potential integer overflow issue.
On 10/24/18 15:22, Long Qin wrote:
> The LookupFreeMemRegion() in RuntimeMemAllocate.c is used to look-up
> free memory region for runtime resource allocation, which was designed
> to support runtime authenticated variable service.
> The direct offset subtractions in this function may bring possible
> integer overflow issue.
>
> This patch is to add the extra parameter checks to remove this possible
> overflow risk.
>
> Cc: Ye Ting
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Long Qin
> ---
> .../Library/BaseCryptLib/SysCall/RuntimeMemAllocation.c| 14
> +-
> 1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/CryptoPkg/Library/BaseCryptLib/SysCall/RuntimeMemAllocation.c
> b/CryptoPkg/Library/BaseCryptLib/SysCall/RuntimeMemAllocation.c
> index 463f2bf855..92bb9ddccd 100644
> --- a/CryptoPkg/Library/BaseCryptLib/SysCall/RuntimeMemAllocation.c
> +++ b/CryptoPkg/Library/BaseCryptLib/SysCall/RuntimeMemAllocation.c
> @@ -2,7 +2,7 @@
>Light-weight Memory Management Routines for OpenSSL-based Crypto
>Library at Runtime Phase.
>
> -Copyright (c) 2009 - 2017, Intel Corporation. All rights reserved.
> +Copyright (c) 2009 - 2018, Intel Corporation. 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
> @@ -141,6 +141,12 @@ LookupFreeMemRegion (
>
>StartPageIndex = RT_SIZE_TO_PAGES (mRTPageTable->LastEmptyPageOffset);
>ReqPages = RT_SIZE_TO_PAGES (AllocationSize);
> + if (ReqPages > mRTPageTable->PageCount) {
> +//
> +// No enough region for object allocation.
> +//
> +return (UINTN)(-1);
> + }
>
>//
>// Look up the free memory region with in current memory map table.
> @@ -176,6 +182,12 @@ LookupFreeMemRegion (
>// Look up the free memory region from the beginning of the memory table
>// until the StartCursorOffset
>//
> + if (ReqPages > StartPageIndex) {
> +//
> +// No enough region for object allocation.
> +//
> +return (UINTN)(-1);
> + }
>for (Index = 0; Index < (StartPageIndex - ReqPages); ) {
> //
> // Check Consecutive ReqPages Pages.
>
As far as I can see, "RuntimeCryptLib.inf" (where this file is used) is
only linked into runtime DXE modules -- not SMM modules. That means this
issue is not a security bug, because runtime DXE modules can be
overwritten by the OS anyway. (They reside in normal RAM.) Can you
please confirm?
Nonetheless, it would be nice to explain in the commit message, what
exactly "ReqPages" depends on.
If needed, please file a BZ as well. (I'm not saying it's required, but
you might want to consider it, and reference it in the commit message.)
Thanks
Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH 2/2] ShellPkg-Shell App: Provide fully-qualified path to shell scripts
Section 3.6.2 of version 2.2 of the shell specification requires that
the first positional argument (i.e. arg 0) of a shell script resolves
to "the full path name of the script itself."
Ensure that the startup script and any scripts launched by the shell
meet this requirement.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jim Dailey
---
ShellPkg/Application/Shell/Shell.c | 12 ++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/ShellPkg/Application/Shell/Shell.c
b/ShellPkg/Application/Shell/Shell.c
index 6185b6ac80..fe88177d57 100644
--- a/ShellPkg/Application/Shell/Shell.c
+++ b/ShellPkg/Application/Shell/Shell.c
@@ -3,6 +3,7 @@
Copyright (c) 2009 - 2018, Intel Corporation. All rights reserved.
(C) Copyright 2013-2014 Hewlett-Packard Development Company, L.P.
+ Copyright 2018 Dell Technologies.
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
@@ -1275,7 +1276,8 @@ DoStartupScript(
FileStringPath = LocateStartupScript (ImagePath, FilePath);
if (FileStringPath != NULL) {
-Status = RunScriptFile (FileStringPath, NULL, L"",
ShellInfoObject.NewShellParametersProtocol);
+FileStringPath = FullyQualifyPath();
+Status = RunScriptFile (FileStringPath, NULL, FileStringPath,
ShellInfoObject.NewShellParametersProtocol);
FreePool (FileStringPath);
} else {
//
@@ -2474,6 +2476,7 @@ RunCommandOrFile(
}
switch (Type) {
case Script_File_Name:
+ CommandWithPath = FullyQualifyPath();
Status = RunScriptFile (CommandWithPath, NULL, CmdLine,
ParamProtocol);
break;
case Efi_Application:
@@ -2812,7 +2815,12 @@ RunScriptFileHandle (
DeleteScriptFileStruct(NewScriptFile);
return (EFI_OUT_OF_RESOURCES);
}
-for (LoopVar = 0 ; LoopVar < 10 && LoopVar < NewScriptFile->Argc;
LoopVar++) {
+//
+// Put the full path of the script file into Argv[0] as required by section
+// 3.6.2 of version 2.2 of the shell specification.
+//
+NewScriptFile->Argv[0] = StrnCatGrow(>Argv[0], NULL,
NewScriptFile->ScriptName, 0);
+for (LoopVar = 1 ; LoopVar < 10 && LoopVar < NewScriptFile->Argc;
LoopVar++) {
ASSERT(NewScriptFile->Argv[LoopVar] == NULL);
NewScriptFile->Argv[LoopVar] =
StrnCatGrow(>Argv[LoopVar], NULL,
ShellInfoObject.NewShellParametersProtocol->Argv[LoopVar], 0);
if (NewScriptFile->Argv[LoopVar] == NULL) {
--
2.17.0.windows.1
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH 1/2] ShellPkg-Shell App: Provide fully-qualified path to shell scripts
Add a function to return the fully-qualified version of some path. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jim Dailey --- ShellPkg/Include/Library/ShellLib.h | 40 + ShellPkg/Library/UefiShellLib/UefiShellLib.c | 93 +++- 2 files changed, 132 insertions(+), 1 deletion(-) diff --git a/ShellPkg/Include/Library/ShellLib.h b/ShellPkg/Include/Library/ShellLib.h index 92fddc50f5..cd7e9c47c3 100644 --- a/ShellPkg/Include/Library/ShellLib.h +++ b/ShellPkg/Include/Library/ShellLib.h @@ -2,6 +2,7 @@ Provides interface to shell functionality for shell commands and applications. Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved. + Copyright 2018 Dell Technologies. 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 @@ -35,6 +36,45 @@ extern EFI_SHELL_PARAMETERS_PROTOCOL *gEfiShellParametersProtocol; extern EFI_SHELL_PROTOCOL*gEfiShellProtocol; +/** + Return the fully-qualified version of a relative path or an absolute path that + does not include a file system reference. + + If ASSERTs are disabled, and if the input parameter is NULL or points to NULL, + then NULL is returned. + + If the input path contains a ":", this function assumes that it is part of a + reference to a file system (e.g. "FS0:"). In such a case, Path is cleaned + and returned. + + If there is no working directory or there is not enough memory available to + create the fully-qualified path, Path is cleaned and returned. + + Otherwise, the current file system or working directory (as appropriate) is + prepended to Path. The input Path is freed and the resulting path is cleaned, + assigned to Path, and returned. + + NOTE: If the input path is an empty string, then the current working directory + (if it exists) is returned. In other words, an empty input path is treated + exactly the same as ".". + + @param[in, out] Path On input, a pointer to some file or directory path. On +output, a pointer to the clean and possibly fully- +qualified version of the input path. The input pointer +may be freed and reassigned on output. + + @retval NULL The input pointer or the path itself was NULL. + + @return A pointer to the clean, fully-qualified version of Path. If memory + allocation fails, or if there is no working directory, then a pointer + to the clean, but not necessarily fully-qualified version of Path. +**/ +CHAR16* +EFIAPI +FullyQualifyPath( + IN OUT CHAR16 **Path + ); + /** This function will retrieve the information about the file for the handle specified and store it in allocated pool memory. diff --git a/ShellPkg/Library/UefiShellLib/UefiShellLib.c b/ShellPkg/Library/UefiShellLib/UefiShellLib.c index f04adbb63f..52ca3ce1b1 100644 --- a/ShellPkg/Library/UefiShellLib/UefiShellLib.c +++ b/ShellPkg/Library/UefiShellLib/UefiShellLib.c @@ -2,7 +2,7 @@ Provides interface to shell functionality for shell commands and applications. (C) Copyright 2016 Hewlett Packard Enterprise Development LP - Copyright 2016 Dell Inc. + Copyright 2016-2018 Dell Technologies. Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved. This program and the accompanying materials are licensed and made available under the terms and conditions of the BSD License @@ -36,6 +36,97 @@ EFI_HANDLEmEfiShellEnvironment2Handle; FILE_HANDLE_FUNCTION_MAP FileFunctionMap; EFI_UNICODE_COLLATION_PROTOCOL *mUnicodeCollationProtocol; +/** + Return the fully-qualified version of a relative path or an absolute path that + does not include a file system reference. + + If asserts are disabled, and if the input parameter is NULL or points to NULL, + then NULL is returned. + + If the input path contains a ":", this function assumes that it is part of a + reference to a file system (e.g. "FS0:"). In such a case, Path is cleaned + and returned. + + If there is no working directory or there is not enough memory available to + create the fully-qualified path, Path is cleaned and returned. + + Otherwise, the current file system or working directory (as appropriate) is + prepended to Path. The input Path is freed and the resulting path is cleaned, + assigned to Path, and returned. + + NOTE: If the input path is an empty string, then the current working directory + (if it exists) is returned. In other words, an empty input path is treated + exactly the same as ".". + + @param[in, out] Path On input, a pointer to some file or directory path. On +output, a pointer to the clean and possibly fully- +qualified version of the input path. The input pointer +
Re: [edk2] [Patch] BaseTools: Move PcdValueInit to platform build folder
Reviewed-by: Liming Gao
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of BobCF
> Sent: Monday, October 22, 2018 11:24 AM
> To: edk2-devel@lists.01.org
> Cc: Gao, Liming
> Subject: [edk2] [Patch] BaseTools: Move PcdValueInit to platform build folder
>
> PcdValueInit tool is platform scope.
> It should be generated into Platform output directory.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Bob C Feng
> Cc: Liming Gao
> ---
> .../Source/Python/Workspace/DscBuildData.py | 25 +--
> 1 file changed, 17 insertions(+), 8 deletions(-)
>
> diff --git a/BaseTools/Source/Python/Workspace/DscBuildData.py
> b/BaseTools/Source/Python/Workspace/DscBuildData.py
> index 17e6f62cac..6fe03ff91a 100644
> --- a/BaseTools/Source/Python/Workspace/DscBuildData.py
> +++ b/BaseTools/Source/Python/Workspace/DscBuildData.py
> @@ -84,10 +84,16 @@ PcdMakefileEnd = '''
> LIBS = $(LIB_PATH)\Common.lib
>
> !INCLUDE $(BASE_TOOLS_PATH)\Source\C\Makefiles\ms.app
> '''
>
> +AppTarget = '''
> +all: $(APPFILE)
> +$(APPFILE): $(OBJECTS)
> +%s
> +'''
> +
> PcdGccMakefile = '''
> MAKEROOT ?= $(EDK_TOOLS_PATH)/Source/C
> LIBS = -lCommon
> '''
>
> @@ -2253,14 +2259,14 @@ class DscBuildData(PlatformBuildClassObject):
> CAppBaseFileName = os.path.join(self.OutputPath, PcdValueInitName)
> SaveFileOnChange(CAppBaseFileName + '.c', CApp, False)
>
> MakeApp = PcdMakefileHeader
> if sys.platform == "win32":
> -MakeApp = MakeApp + 'APPNAME = %s\n' % (PcdValueInitName) +
> 'OBJECTS = %s\%s.obj\n' % (self.OutputPath,
> PcdValueInitName) + 'INC = '
> +MakeApp = MakeApp + 'APPFILE = %s\%s.exe\n' % (self.OutputPath,
> PcdValueInitName) + 'APPNAME = %s\n' %
> (PcdValueInitName) + 'OBJECTS = %s\%s.obj\n' % (self.OutputPath,
> PcdValueInitName) + 'INC = '
> else:
> MakeApp = MakeApp + PcdGccMakefile
> -MakeApp = MakeApp + 'APPNAME = %s\n' % (PcdValueInitName) +
> 'OBJECTS = %s/%s.o\n' % (self.OutputPath,
> PcdValueInitName) + \
> +MakeApp = MakeApp + 'APPFILE = %s/%s\n' % (self.OutputPath,
> PcdValueInitName) + 'APPNAME = %s\n' %
> (PcdValueInitName) + 'OBJECTS = %s/%s.o\n' % (self.OutputPath,
> PcdValueInitName) + \
>'include $(MAKEROOT)/Makefiles/app.makefile\n' +
> 'INCLUDE +='
>
> IncSearchList = []
> PlatformInc = OrderedDict()
> for Cache in self._Bdb._CACHE_.values():
> @@ -2332,10 +2338,13 @@ class DscBuildData(PlatformBuildClassObject):
> CC_FLAGS += ' ' + Item
> MakeApp += CC_FLAGS
>
> if sys.platform == "win32":
> MakeApp = MakeApp + PcdMakefileEnd
> +MakeApp = MakeApp + AppTarget % ("""\tcopy $(APPLICATION)
> $(APPFILE) /y """)
> +else:
> +MakeApp = MakeApp + AppTarget % ("""\tcp $(APPLICATION)
> $(APPFILE) """)
> MakeApp = MakeApp + '\n'
> IncludeFileFullPaths = []
> for includefile in IncludeFiles:
> for includepath in IncSearchList:
> includefullpath = os.path.join(str(includepath), includefile)
> @@ -2355,25 +2364,25 @@ class DscBuildData(PlatformBuildClassObject):
>
> InputValueFile = os.path.join(self.OutputPath, 'Input.txt')
> OutputValueFile = os.path.join(self.OutputPath, 'Output.txt')
> SaveFileOnChange(InputValueFile, InitByteValue, False)
>
> -PcdValueInitExe = PcdValueInitName
> +Dest_PcdValueInitExe = PcdValueInitName
> if not sys.platform == "win32":
> -PcdValueInitExe = os.path.join(os.getenv("EDK_TOOLS_PATH"),
> 'Source', 'C', 'bin', PcdValueInitName)
> +Dest_PcdValueInitExe = os.path.join(self.OutputPath,
> PcdValueInitName)
> else:
> -PcdValueInitExe = os.path.join(os.getenv("EDK_TOOLS_PATH"),
> 'Bin', 'Win32', PcdValueInitName) +".exe"
> -
> +Dest_PcdValueInitExe = os.path.join(self.OutputPath,
> PcdValueInitName) +".exe"
> Messages = ''
> if sys.platform == "win32":
> MakeCommand = 'nmake -f %s' % (MakeFileName)
> returncode, StdOut, StdErr = DscBuildData.ExecuteCommand
> (MakeCommand)
> Messages = StdOut
> else:
> MakeCommand = 'make -f %s' % (MakeFileName)
> returncode, StdOut, StdErr = DscBuildData.ExecuteCommand
> (MakeCommand)
> Messages = StdErr
> +
> Messages = Messages.split('\n')
> MessageGroup = []
> if returncode != 0:
> CAppBaseFileName = os.path.join(self.OutputPath,
> PcdValueInitName)
> File = open (CAppBaseFileName + '.c', 'r')
> @@ -2414,12 +2423,12 @@ class DscBuildData(PlatformBuildClassObject):
> if MessageGroup:
> EdkLogger.error("build", PCD_STRUCTURE_PCD_ERROR,
>
Re: [edk2] [Patch] BaseTool: Filter out unused structure pcds
Reviewed-by: Liming Gao
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of BobCF
> Sent: Friday, October 19, 2018 8:00 PM
> To: edk2-devel@lists.01.org
> Cc: Gao, Liming
> Subject: [edk2] [Patch] BaseTool: Filter out unused structure pcds
>
> The current code handle all the structure pcds
> even if there is no module or library use them.
> This patch is going to filter out the unused structure pcds.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Bob Feng
> Cc: Liming Gao
> ---
> .../Source/Python/Workspace/DscBuildData.py | 19 +++
> 1 file changed, 19 insertions(+)
>
> diff --git a/BaseTools/Source/Python/Workspace/DscBuildData.py
> b/BaseTools/Source/Python/Workspace/DscBuildData.py
> index e481ea4f64..31bce16d15 100644
> --- a/BaseTools/Source/Python/Workspace/DscBuildData.py
> +++ b/BaseTools/Source/Python/Workspace/DscBuildData.py
> @@ -274,10 +274,11 @@ class DscBuildData(PlatformBuildClassObject):
> self._RFCLanguages = None
> self._ISOLanguages = None
> self._VpdToolGuid = None
> self._MacroDict = None
> self.DefaultStores = None
> +self.UsedStructurePcd = None
>
> ## handle Override Path of Module
> def _HandleOverridePath(self):
> RecordList = self._RawData[MODEL_META_DATA_COMPONENT, self._Arch]
> for Record in RecordList:
> @@ -1476,10 +1477,11 @@ class DscBuildData(PlatformBuildClassObject):
> if str_pcd_obj.Type in
> [self._PCD_TYPE_STRING_[MODEL_PCD_DYNAMIC_HII],
> self._PCD_TYPE_STRING_[MODEL_PCD_DYNAMIC_EX_HII]]:
> str_pcd_obj_str.DefaultFromDSC =
> {skuname:{defaultstore:
> str_pcd_obj.SkuInfoList[skuname].DefaultStoreDict.get(defaultstore,
> str_pcd_obj.SkuInfoList[skuname].HiiDefaultValue) for defaultstore
> in DefaultStores} for skuname in str_pcd_obj.SkuInfoList}
> else:
> str_pcd_obj_str.DefaultFromDSC =
> {skuname:{defaultstore:
> str_pcd_obj.SkuInfoList[skuname].DefaultStoreDict.get(defaultstore,
> str_pcd_obj.SkuInfoList[skuname].DefaultValue) for defaultstore in
> DefaultStores} for skuname in str_pcd_obj.SkuInfoList}
> S_pcd_set[Pcd] = str_pcd_obj_str
> +self.FilterStrcturePcd(S_pcd_set)
> if S_pcd_set:
> GlobalData.gStructurePcd[self.Arch] = S_pcd_set
> for stru_pcd in S_pcd_set.values():
> for skuid in SkuIds:
> if skuid in stru_pcd.SkuOverrideValues:
> @@ -1571,10 +1573,27 @@ class DscBuildData(PlatformBuildClassObject):
> elif TAB_DEFAULT in pcd.SkuInfoList and TAB_COMMON in
> pcd.SkuInfoList:
> del pcd.SkuInfoList[TAB_COMMON]
>
> map(self.FilterSkuSettings, [Pcds[pcdkey] for pcdkey in Pcds if
> Pcds[pcdkey].Type in DynamicPcdType])
> return Pcds
> +#Filter the StrucutrePcd that is not used by any module in dsc file and
> fdf file.
> +def FilterStrcturePcd(self, S_pcd_set):
> +if not self.UsedStructurePcd:
> +FdfInfList = []
> +if GlobalData.gFdfParser:
> +FdfInfList = GlobalData.gFdfParser.Profile.InfList
> +FdfModuleList = [PathClass(NormPath(Inf), GlobalData.gWorkspace,
> Arch=self._Arch) for Inf in FdfInfList]
> +AllModulePcds = set()
> +ModuleSet = set(self._Modules.keys() + self.LibraryInstances +
> FdfModuleList)
> +for ModuleFile in ModuleSet:
> +ModuleData = self._Bdb[ModuleFile, self._Arch, self._Target,
> self._Toolchain]
> +AllModulePcds = AllModulePcds | set(ModuleData.Pcds.keys())
> +
> +self.UsedStructurePcd = AllModulePcds
> +UnusedStruPcds = set(S_pcd_set.keys()) - self.UsedStructurePcd
> +for (Token, TokenSpaceGuid) in UnusedStruPcds:
> +del S_pcd_set[(Token, TokenSpaceGuid)]
>
> ## Retrieve non-dynamic PCD settings
> #
> # @param TypePCD type
> #
> --
> 2.18.0.windows.1
>
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [Patch V2] BaseTools: Fix the bug for Pcd used in command line's override
V2: remove the not used parameter i
Fix the bug for Pcd used in command line not override the Pcd used
in the [component] driver's sub-section.
Case:
DSC file:
[PcdsFixedAtBuild]
TokenSpaceGuid.PcdTest
[Components]
TestPkg/TestDriver.inf {
TokenSpaceGuid.PcdTest|"b"
}
build command with --pcd TokenSpaceGuid.PcdTest="BB"
Then we found the Pcd value in the AutoGen.c file is incorrect,
because of the incorrect logic that use the pcd in the [component]
section to re-override it.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Yonghong Zhu
---
BaseTools/Source/Python/AutoGen/AutoGen.py| 7 +++
BaseTools/Source/Python/Workspace/DscBuildData.py | 5 +
2 files changed, 12 insertions(+)
diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py
b/BaseTools/Source/Python/AutoGen/AutoGen.py
index 804f579..84645e3 100644
--- a/BaseTools/Source/Python/AutoGen/AutoGen.py
+++ b/BaseTools/Source/Python/AutoGen/AutoGen.py
@@ -2118,10 +2118,17 @@ class PlatformAutoGen(AutoGen):
# override PCD settings with module specific setting
if Module in self.Platform.Modules:
PlatformModule = self.Platform.Modules[str(Module)]
for Key in PlatformModule.Pcds:
+if GlobalData.BuildOptionPcd:
+for pcd in GlobalData.BuildOptionPcd:
+(TokenSpaceGuidCName, TokenCName, FieldName, pcdvalue,
_) = pcd
+if (TokenCName, TokenSpaceGuidCName) == Key and
FieldName =="":
+PlatformModule.Pcds[Key].DefaultValue = pcdvalue
+PlatformModule.Pcds[Key].PcdValueFromComm =
pcdvalue
+break
Flag = False
if Key in Pcds:
ToPcd = Pcds[Key]
Flag = True
elif Key in GlobalData.MixedPcd:
diff --git a/BaseTools/Source/Python/Workspace/DscBuildData.py
b/BaseTools/Source/Python/Workspace/DscBuildData.py
index b0e88a9..162360c 100644
--- a/BaseTools/Source/Python/Workspace/DscBuildData.py
+++ b/BaseTools/Source/Python/Workspace/DscBuildData.py
@@ -1058,11 +1058,16 @@ class DscBuildData(PlatformBuildClassObject):
IsValid, Cause = CheckPcdDatum(PcdDatumType, pcdvalue)
if not IsValid:
EdkLogger.error("build", FORMAT_INVALID, Cause,
ExtraData="%s.%s" % (TokenSpaceGuidCName, TokenCName))
GlobalData.BuildOptionPcd[i] = (TokenSpaceGuidCName,
TokenCName, FieldName, pcdvalue, ("build command options", 1))
+if GlobalData.BuildOptionPcd:
+for pcd in GlobalData.BuildOptionPcd:
+(TokenSpaceGuidCName, TokenCName, FieldName, pcdvalue, _) = pcd
for BuildData in self._Bdb._CACHE_.values():
+if BuildData.Arch != self.Arch:
+continue
if BuildData.MetaFile.Ext == '.dec' or
BuildData.MetaFile.Ext == '.dsc':
continue
for key in BuildData.Pcds:
PcdItem = BuildData.Pcds[key]
if (TokenSpaceGuidCName, TokenCName) ==
(PcdItem.TokenSpaceGuidCName, PcdItem.TokenCName) and FieldName =="":
--
2.6.1.windows.1
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH] CryptoPkg/BaseCryptLib: Fix potential integer overflow issue.
The LookupFreeMemRegion() in RuntimeMemAllocate.c is used to look-up
free memory region for runtime resource allocation, which was designed
to support runtime authenticated variable service.
The direct offset subtractions in this function may bring possible
integer overflow issue.
This patch is to add the extra parameter checks to remove this possible
overflow risk.
Cc: Ye Ting
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Long Qin
---
.../Library/BaseCryptLib/SysCall/RuntimeMemAllocation.c| 14 +-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/CryptoPkg/Library/BaseCryptLib/SysCall/RuntimeMemAllocation.c
b/CryptoPkg/Library/BaseCryptLib/SysCall/RuntimeMemAllocation.c
index 463f2bf855..92bb9ddccd 100644
--- a/CryptoPkg/Library/BaseCryptLib/SysCall/RuntimeMemAllocation.c
+++ b/CryptoPkg/Library/BaseCryptLib/SysCall/RuntimeMemAllocation.c
@@ -2,7 +2,7 @@
Light-weight Memory Management Routines for OpenSSL-based Crypto
Library at Runtime Phase.
-Copyright (c) 2009 - 2017, Intel Corporation. All rights reserved.
+Copyright (c) 2009 - 2018, Intel Corporation. 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
@@ -141,6 +141,12 @@ LookupFreeMemRegion (
StartPageIndex = RT_SIZE_TO_PAGES (mRTPageTable->LastEmptyPageOffset);
ReqPages = RT_SIZE_TO_PAGES (AllocationSize);
+ if (ReqPages > mRTPageTable->PageCount) {
+//
+// No enough region for object allocation.
+//
+return (UINTN)(-1);
+ }
//
// Look up the free memory region with in current memory map table.
@@ -176,6 +182,12 @@ LookupFreeMemRegion (
// Look up the free memory region from the beginning of the memory table
// until the StartCursorOffset
//
+ if (ReqPages > StartPageIndex) {
+//
+// No enough region for object allocation.
+//
+return (UINTN)(-1);
+ }
for (Index = 0; Index < (StartPageIndex - ReqPages); ) {
//
// Check Consecutive ReqPages Pages.
--
2.16.1.windows.1
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH] BaseTools:Not miss the full assign value of FixedAtBuild structure PCD
For structure PCD, if it is a FixedAtBuild PCD, the full assign value in
dsc file should not be missed when updating the structure PCD value.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: ZhiqiangX Zhao
Cc: Liming Gao
Cc: Yonghong Zhu
Cc: Bob Feng
---
BaseTools/Source/Python/Workspace/DscBuildData.py | 28 +--
1 file changed, 16 insertions(+), 12 deletions(-)
diff --git a/BaseTools/Source/Python/Workspace/DscBuildData.py
b/BaseTools/Source/Python/Workspace/DscBuildData.py
index b0e88a93ce..e24daa63b6 100644
--- a/BaseTools/Source/Python/Workspace/DscBuildData.py
+++ b/BaseTools/Source/Python/Workspace/DscBuildData.py
@@ -1420,6 +1420,7 @@ class DscBuildData(PlatformBuildClassObject):
SkuIds = self.SkuIds
self.SkuIdMgr.AvailableSkuIdSet.update({TAB_DEFAULT:0})
DefaultStores = {storename for pcdobj in AllPcds.values() for skuobj
in pcdobj.SkuInfoList.values() for storename in skuobj.DefaultStoreDict}
+DefaultStores.add(TAB_DEFAULT_STORES_DEFAULT)
S_PcdSet = []
# Find out all possible PCD candidates for self._Arch
@@ -1589,7 +1590,7 @@ class DscBuildData(PlatformBuildClassObject):
#
AvailableSkuIdSet = copy.copy(self.SkuIds)
-PcdDict = tdict(True, 3)
+PcdDict = tdict(True, 4)
PcdSet = set()
# Find out all possible PCD candidates for self._Arch
RecordList = self._RawData[Type, self._Arch]
@@ -1600,10 +1601,9 @@ class DscBuildData(PlatformBuildClassObject):
if SkuName not in AvailableSkuIdSet:
EdkLogger.error('build ', PARAMETER_INVALID, 'Sku %s is not
defined in [SkuIds] section' % SkuName,
File=self.MetaFile, Line=Dummy5)
-if SkuName in (self.SkuIdMgr.SystemSkuId, TAB_DEFAULT, TAB_COMMON):
-if "." not in TokenSpaceGuid:
-PcdSet.add((PcdCName, TokenSpaceGuid, SkuName, Dummy5))
-PcdDict[Arch, PcdCName, TokenSpaceGuid, SkuName] = Setting
+if "." not in TokenSpaceGuid:
+PcdSet.add((PcdCName, TokenSpaceGuid, SkuName, Dummy5))
+PcdDict[Arch, PcdCName, TokenSpaceGuid, SkuName] = Setting
for PcdCName, TokenSpaceGuid, SkuName, Dummy4 in PcdSet:
Setting = PcdDict[self._Arch, PcdCName, TokenSpaceGuid, SkuName]
@@ -1646,10 +1646,11 @@ class DscBuildData(PlatformBuildClassObject):
False,
None,
IsDsc=True)
-
-if self.SkuIdMgr.SystemSkuId not in Pcds[PcdCName,
TokenSpaceGuid].DscRawValue:
-Pcds[PcdCName,
TokenSpaceGuid].DscRawValue[self.SkuIdMgr.SystemSkuId] = {}
-Pcds[PcdCName,
TokenSpaceGuid].DscRawValue[self.SkuIdMgr.SystemSkuId][TAB_DEFAULT_STORES_DEFAULT]
= PcdValue
+for SkuName in PcdValueDict[PcdCName, TokenSpaceGuid]:
+Settings = PcdValueDict[PcdCName, TokenSpaceGuid][SkuName]
+if SkuName not in Pcds[PcdCName, TokenSpaceGuid].DscRawValue:
+Pcds[PcdCName, TokenSpaceGuid].DscRawValue[SkuName] = {}
+Pcds[PcdCName,
TokenSpaceGuid].DscRawValue[SkuName][TAB_DEFAULT_STORES_DEFAULT] = Settings[0]
return Pcds
def GetStructurePcdMaxSize(self, str_pcd):
@@ -1884,10 +1885,13 @@ class DscBuildData(PlatformBuildClassObject):
CApp = CApp + "// SkuName: %s, DefaultStoreName: %s \n" %
(TAB_DEFAULT, TAB_DEFAULT_STORES_DEFAULT)
inherit_OverrideValues = Pcd.SkuOverrideValues[SkuName]
-if (SkuName, DefaultStoreName) == (TAB_DEFAULT,
TAB_DEFAULT_STORES_DEFAULT):
-pcddefaultvalue = Pcd.DefaultFromDSC.get(TAB_DEFAULT,
{}).get(TAB_DEFAULT_STORES_DEFAULT) if Pcd.DefaultFromDSC else None
+if Pcd.Type in PCD_DYNAMIC_TYPE_SET or Pcd.Type in
PCD_DYNAMIC_EX_TYPE_SET:
+if (SkuName, DefaultStoreName) == (TAB_DEFAULT,
TAB_DEFAULT_STORES_DEFAULT):
+pcddefaultvalue = Pcd.DefaultFromDSC.get(TAB_DEFAULT,
{}).get(TAB_DEFAULT_STORES_DEFAULT) if Pcd.DefaultFromDSC else None
+else:
+pcddefaultvalue = Pcd.DscRawValue.get(SkuName,
{}).get(DefaultStoreName)
else:
-pcddefaultvalue = Pcd.DscRawValue.get(SkuName,
{}).get(DefaultStoreName)
+pcddefaultvalue = Pcd.DscRawValue.get(SkuName,
{}).get(TAB_DEFAULT_STORES_DEFAULT)
for FieldList in [pcddefaultvalue,
inherit_OverrideValues.get(DefaultStoreName)]:
if not FieldList:
continue
--
2.14.1.windows.1
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [edk2-InfSpecification PATCH] INF Spec: Amend the OptionROM spec to allow multiple PCI_DEVICE_IDs
The BaseTools have been updated to allow multiple PCI_DEVICE_IDs following the Device List introduced in the PCI Spec rev 3.0. This change documents the syntax. Signed-off-by: Tomas Pilar Contributed-under: TianoCore Contribution Agreement 1.1 --- 2_inf_overview/24_[defines]_section.md | 2 +- 3_edk_ii_inf_file_format/34_[defines]_section.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/2_inf_overview/24_[defines]_section.md b/2_inf_overview/24_[defines]_section.md index 0afdfed..8e5706c 100644 --- a/2_inf_overview/24_[defines]_section.md +++ b/2_inf_overview/24_[defines]_section.md @@ -128,7 +128,7 @@ dispatch instance. |`CONSTRUCTOR` |Not required - Library Only |CName | This only applies to components that are libraries. It is required for EDK II libraries if the module's INF contains a Constructor element. This value is used to call the specified function before calling into the library itself. | |`DESTRUCTOR`|Not required - Library Only |CName | This only applies to components that are libraries. This value is used to call the specified function before calling into the library itself. | |`SHADOW`|Not required - SEC, PEIM and PEI_CORE Driver modules only|TRUE FALSE | This boolean operator is used by `SEC`, `PEI_CORE` and `PEIM` modules to indicate if the module was coded to use `REGISTER_FOR_SHADOW`. If the value is TRUE, the .reloc section of the PE32 image is not removed, otherwise, the .reloc section is stripped to conserve space in the final binary images. The default value is FALSE. | -|`PCI_DEVICE_ID` |Not required - Required for UEFI PCI Option ROMs |UINT16 Value| The PCI Device Id for this device | +|`PCI_DEVICE_ID` |Not required - Required for UEFI PCI Option ROMs |List of UINT16 Values | The list of PCI Device Ids for this device | |`PCI_VENDOR_ID` |Not required - Required for UEFI PCI Option ROMs |UINT16 Value| The PCI Vendor Id for this device | |`PCI_CLASS_CODE`|Not required - Required for UEFI PCI Option ROMs |UINT8 Value | The PCI Class Code for this device | |`PCI_REVISION` |Not required - Required for UEFI PCI Option ROMs |UINT8 Value | The PCI revision for this device | diff --git a/3_edk_ii_inf_file_format/34_[defines]_section.md b/3_edk_ii_inf_file_format/34_[defines]_section.md index f512ff9..4ee6313 100644 --- a/3_edk_ii_inf_file_format/34_[defines]_section.md +++
[edk2] [edk2-FdfSpecification PATCH] Amend the OptionROM spec to allow multiple PCI_DEVICE_IDs
The BaseTools are updated to allow multiple PCI_DEVICE_ID fields following
the Device List introduced in the PCI Spec rev 3.0. This commit documents the
amended syntax.
Signed-off-by: Tomas Pilar
Contributed-under: TianoCore Contribution Agreement 1.1
---
3_edk_ii_fdf_file_format/311_pci_optionrom_section.md | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/3_edk_ii_fdf_file_format/311_pci_optionrom_section.md
b/3_edk_ii_fdf_file_format/311_pci_optionrom_section.md
index 08f50e7..046eacf 100644
--- a/3_edk_ii_fdf_file_format/311_pci_optionrom_section.md
+++ b/3_edk_ii_fdf_file_format/311_pci_optionrom_section.md
@@ -58,7 +58,7 @@ Conditional statements may be used anywhere within this
section.
::= "{"
[ "PCI_VENDOR_ID" ]
[ "PCI_CLASS_CODE" ]
- [ "PCI_DEVICE_ID" ]
+ [ "PCI_DEVICE_ID" + ]
[ "PCI_REVISION" ]
[ "PCI_COMPRESS" ]
"}"
@@ -108,6 +108,7 @@ for the .efi extension in the ENBF above.
[OptionRom.AtapiPassThru]
INF USE = IA32 OptionRomPkg/AtapiPassThruDxe/AtapiPassThruDxe.inf {
PCI_REVISION = 0x0020
+PCI_DEVICE_ID = 0x0A03 0x0B03
}
INF USE = EBC OptionRomPkg/AtapiPassThruDxe/AtapiPassThruDxe.inf
```
--
2.17.2
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH v2] BaseTools: Allow multiple PciDeviceId in Fdf OptionROM override
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Tomas Pilar
---
BaseTools/Source/Python/GenFds/FdfParser.py | 12 +---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/BaseTools/Source/Python/GenFds/FdfParser.py
b/BaseTools/Source/Python/GenFds/FdfParser.py
index 63687e98bb..8f53fbeb55 100644
--- a/BaseTools/Source/Python/GenFds/FdfParser.py
+++ b/BaseTools/Source/Python/GenFds/FdfParser.py
@@ -1,3 +1,4 @@
+
## @file
# parse FDF file
#
@@ -4469,10 +4470,15 @@ class FdfParser:
if self.__IsKeyword( "PCI_DEVICE_ID"):
if not self.__IsToken( "="):
raise Warning("expected '='", self.FileName,
self.CurrentLineNumber)
-if not self.__GetNextHexNumber():
-raise Warning("expected Hex device id", self.FileName,
self.CurrentLineNumber)
-Overrides.PciDeviceId = self.__Token
+# Get a list of PCI IDs
+Overrides.PciDeviceId = ""
+
+while (self.__GetNextHexNumber()):
+Overrides.PciDeviceId = "{}
{}".format(Overrides.PciDeviceId, self.__Token)
+
+if not Overrides.PciDeviceId:
+raise Warning("expected one or more Hex device ids",
self.FileName, self.CurrentLineNumber)
continue
if self.__IsKeyword( "PCI_REVISION"):
--
2.17.2
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH] IntelSiliconPkg VTdDxe: Option to force no early access attr request
Reviewed-by: jiewen@intel.com
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Star Zeng
> Sent: Wednesday, October 24, 2018 11:32 AM
> To: edk2-devel@lists.01.org
> Cc: Yao, Jiewen ; Zeng, Star
> Subject: [edk2] [PATCH] IntelSiliconPkg VTdDxe: Option to force no early
> access attr request
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1272
>
> To have high confidence in usage for platform, add option (BIT2 of
> PcdVTdPolicyPropertyMask) to force no IOMMU access attribute request
> recording before DMAR table is installed.
>
> Check PcdVTdPolicyPropertyMask BIT2 before RequestAccessAttribute()
> and ProcessRequestedAccessAttribute(), then RequestAccessAttribute(),
> ProcessRequestedAccessAttribute() and mAccessRequestXXX variables
> could be optimized by compiler when PcdVTdPolicyPropertyMask BIT2 = 1.
>
> Test done:
> 1: Created case that has IOMMU access attribute request before DMAR
>table is installed, ASSERT was triggered after setting
>PcdVTdPolicyPropertyMask BIT2 to 1.
>
> 2. Confirmed RequestAccessAttribute(), ProcessRequestedAccessAttribute()
>and mAccessRequestXXX variables were optimized by compiler after
>setting PcdVTdPolicyPropertyMask BIT2 to 1.
>
> Cc: Jiewen Yao
> Cc: Rangasai V Chaganty
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Star Zeng
> ---
> IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmaProtection.c | 8 +++-
> IntelSiliconPkg/Feature/VTd/IntelVTdDxe/IntelVTdDxe.c | 7 +++
> IntelSiliconPkg/IntelSiliconPkg.dec | 1 +
> 3 files changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmaProtection.c
> b/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmaProtection.c
> index 86d50eb6f288..7784545631b3 100644
> --- a/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmaProtection.c
> +++ b/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmaProtection.c
> @@ -515,7 +515,13 @@ SetupVtd (
>
>ParseDmarAcpiTableRmrr ();
>
> - ProcessRequestedAccessAttribute ();
> + if ((PcdGet8 (PcdVTdPolicyPropertyMask) & BIT2) == 0) {
> +//
> +// Support IOMMU access attribute request recording before DMAR
> table is installed.
> +// Here is to process the requests.
> +//
> +ProcessRequestedAccessAttribute ();
> + }
>
>for (Index = 0; Index < mVtdUnitNumber; Index++) {
> DEBUG ((DEBUG_INFO,"VTD Unit %d (Segment: %04x)\n", Index,
> mVtdUnitInformation[Index].Segment));
> diff --git a/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/IntelVTdDxe.c
> b/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/IntelVTdDxe.c
> index 25d7c80af1d4..09948ce50e94 100644
> --- a/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/IntelVTdDxe.c
> +++ b/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/IntelVTdDxe.c
> @@ -254,6 +254,13 @@ VTdSetAttribute (
> // Record the entry to driver global variable.
> // As such once VTd is activated, the setting can be adopted.
> //
> +if ((PcdGet8 (PcdVTdPolicyPropertyMask) & BIT2) != 0) {
> + //
> + // Force no IOMMU access attribute request recording before
> DMAR table is installed.
> + //
> + ASSERT_EFI_ERROR (EFI_NOT_READY);
> + return EFI_NOT_READY;
> +}
> Status = RequestAccessAttribute (Segment, SourceId, DeviceAddress,
> Length, IoMmuAccess);
>} else {
> PERF_CODE (
> diff --git a/IntelSiliconPkg/IntelSiliconPkg.dec
> b/IntelSiliconPkg/IntelSiliconPkg.dec
> index b9646d773b95..900e8f63c64d 100644
> --- a/IntelSiliconPkg/IntelSiliconPkg.dec
> +++ b/IntelSiliconPkg/IntelSiliconPkg.dec
> @@ -64,6 +64,7 @@ [PcdsFixedAtBuild, PcdsPatchableInModule,
> PcdsDynamic, PcdsDynamicEx]
>## The mask is used to control VTd behavior.
># BIT0: Enable IOMMU during boot (If DMAR table is installed in DXE. If
> VTD_INFO_PPI is installed in PEI.)
># BIT1: Enable IOMMU when transfer control to OS (ExitBootService in
> normal boot. EndOfPEI in S3)
> + # BIT2: Force no IOMMU access attribute request recording before
> DMAR table is installed.
># @Prompt The policy for VTd driver behavior.
>
> gIntelSiliconPkgTokenSpaceGuid.PcdVTdPolicyPropertyMask|1|UINT8|0x000
> 2
>
> --
> 2.7.0.windows.1
>
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH] IntelSiliconPkg VTdDxe: Report status code for VTd error
Reviewed-by: jiewen@intel.com
> -Original Message-
> From: Zeng, Star
> Sent: Wednesday, October 24, 2018 11:36 AM
> To: edk2-devel@lists.01.org
> Cc: Zeng, Star ; Yao, Jiewen ;
> Chaganty, Rangasai V
> Subject: [PATCH] IntelSiliconPkg VTdDxe: Report status code for VTd error
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1273
>
> Current code only uses DEBUG() for VTd error.
> This patch updates to also report status code for VTd error.
>
> Test done:
> Created case that has VTd error and confirmed the error
> status code could be reported as expected.
>
> Cc: Jiewen Yao
> Cc: Rangasai V Chaganty
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Star Zeng
> ---
> IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmaProtection.h | 1 +
> IntelSiliconPkg/Feature/VTd/IntelVTdDxe/IntelVTdDxe.inf | 2 ++
> IntelSiliconPkg/Feature/VTd/IntelVTdDxe/VtdReg.c| 1 +
> IntelSiliconPkg/IntelSiliconPkg.dec | 6 ++
> 4 files changed, 10 insertions(+)
>
> diff --git a/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmaProtection.h
> b/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmaProtection.h
> index 2ec92fe523c3..baa092f3ac0c 100644
> --- a/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmaProtection.h
> +++ b/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmaProtection.h
> @@ -28,6 +28,7 @@
> #include
> #include
> #include
> +#include
>
> #include
> #include
> diff --git a/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/IntelVTdDxe.inf
> b/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/IntelVTdDxe.inf
> index 60bb335da946..ca1f2d709215 100644
> --- a/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/IntelVTdDxe.inf
> +++ b/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/IntelVTdDxe.inf
> @@ -60,6 +60,7 @@ [LibraryClasses]
>CacheMaintenanceLib
>PerformanceLib
>PrintLib
> + ReportStatusCodeLib
>
> [Guids]
>gEfiEventExitBootServicesGuid ## CONSUMES ## Event
> @@ -78,6 +79,7 @@ [Protocols]
>
> [Pcd]
>gIntelSiliconPkgTokenSpaceGuid.PcdVTdPolicyPropertyMask ##
> CONSUMES
> + gIntelSiliconPkgTokenSpaceGuid.PcdErrorCodeVTdError ##
> CONSUMES
>
> [Depex]
>gEfiPciRootBridgeIoProtocolGuid
> diff --git a/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/VtdReg.c
> b/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/VtdReg.c
> index e564d373c756..45285510a500 100644
> --- a/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/VtdReg.c
> +++ b/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/VtdReg.c
> @@ -545,6 +545,7 @@ DumpVtdIfError (
> }
>
> if (HasError) {
> + REPORT_STATUS_CODE (EFI_ERROR_CODE, PcdGet32
> (PcdErrorCodeVTdError));
>DEBUG((DEBUG_INFO, "\n ERROR \n"));
>DumpVtdRegs (Num);
>DEBUG((DEBUG_INFO, " ERROR \n\n"));
> diff --git a/IntelSiliconPkg/IntelSiliconPkg.dec
> b/IntelSiliconPkg/IntelSiliconPkg.dec
> index 2f5bef6089f9..900e8f63c64d 100644
> --- a/IntelSiliconPkg/IntelSiliconPkg.dec
> +++ b/IntelSiliconPkg/IntelSiliconPkg.dec
> @@ -47,6 +47,12 @@ [Ppis]
> [Protocols]
>gEdkiiPlatformVTdPolicyProtocolGuid = { 0x3d17e448, 0x466, 0x4e20,
> { 0x99, 0x9f, 0xb2, 0xe1, 0x34, 0x88, 0xee, 0x22 }}
>
> +[PcdsFixedAtBuild, PcdsPatchableInModule]
> + ## Error code for VTd error.
> + # EDKII_ERROR_CODE_VTD_ERROR = (EFI_IO_BUS_UNSPECIFIED |
> (EFI_OEM_SPECIFIC | 0x)) = 0x02008000
> + # @Prompt Error code for VTd error.
> +
> gIntelSiliconPkgTokenSpaceGuid.PcdErrorCodeVTdError|0x02008000|UINT3
> 2|0x0005
> +
> [PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx]
>## This is the GUID of the FFS which contains the Graphics Video BIOS
> Table (VBT)
># The VBT content is stored as a RAW section which is consumed by GOP
> PEI/UEFI driver.
> --
> 2.7.0.windows.1
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v2 7/7] ArmPkg: Extra action to update permissions for S-ELO MM Image
On 24 October 2018 at 05:22, Achin Gupta wrote: > Hi Ard, > > Please see CIL.. > FYI I will be on leave until 5th of November, so I will get to this once I get back. -- Ard. > On Fri, Aug 24, 2018 at 03:55:29PM +0100, Ard Biesheuvel wrote: >> On 21 August 2018 at 07:50, Sughosh Ganu wrote: >> > hi Ard, >> > >> > On Tue July 23, 2018 at 11:03PM +0530, Supreeth Venkatesh wrote: >> >> >> >> On Sat, 2018-07-21 at 20:06 +0900, Ard Biesheuvel wrote: >> >> > On 20 July 2018 at 21:38, Sughosh Ganu wrote: >> >> > > >> >> > > From: Achin Gupta >> >> > > >> >> > > The Standalone MM drivers runs in S-EL0 in AArch64 on ARM Standard >> >> > > Platforms and is deployed during SEC phase. The memory allocated to >> >> > > the Standalone MM drivers should be marked as RO+X. >> >> > > >> >> > > During PE/COFF Image section parsing, this patch implements extra >> >> > > action "UpdatePeCoffPermissions" to request the privileged firmware >> >> > > in >> >> > > EL3 to update the permissions. >> >> > > >> >> > > Contributed-under: TianoCore Contribution Agreement 1.1 >> >> > > Signed-off-by: Sughosh Ganu >> >> > Apologies for bringing this up only now, but I don't think I was ever >> >> > cc'ed on these patches. >> >> > >> >> Apologies if you have missed it. But I am pretty sure it was part of >> >> earlier large patch-set on which you and leif were copied, as it was >> >> part of ArmPkg. >> >> > >> >> > We are relying on a debug hook in the PE/COFF loader to ensure that >> >> > we >> >> > don't end up with memory that is both writable and executable in the >> >> > secure world. Do we really think that is a good idea? >> >> > >> >> > (I know this code was derived from a proof of concept that I did >> >> > years >> >> > ago, but that was just a PoC) >> >> I think we need a little bit more details on what is your suggestion? >> >> >> >> A little bit background here: This code runs in S-EL0 and Request gets >> >> sent to secure world SPM to ensure that the region permissions are >> >> updated correctly via the "ArmMmuStandaloneMmCoreLib" SVC - >> >> ARM_SVC_ID_SP_SET_MEM_ATTRIBUTES_AARCH64. >> >> >> >> DebugPeCoffExtraActionLib is just used to extract image region >> >> information, but the region permission >> >> update request is sent to secure world for validation. >> >> >> >> With the above explanation, can you provide an insight into what was >> >> your thinking? >> >> Do you want us to create a separate library and call it >> >> as PeCoffExtraActionLib to avoid the "Debug" word though it is a hook >> >> to PeCoffExtraActionLib in MdePkg or do we want to create this library >> >> in a separate package (may be in MdePkg?) or something totally >> >> different. >> > >> > Supreeth had replied to your comments on the patch. Can you please >> > check this. If you feel that this needs to be implemented differently, >> > can you please suggest it to us. Thanks. >> > >> >> My point is that such a fundamental action that needs to occur while >> loading the PE/COFF image should not be hooked into the loader this >> way. > > Based upon our discussion at the Linaro Connect, we investigated leveraging > the > DXE Image Protection support [1] in Standalone MM (StMM). Amongst other > challenges, there is a chicken and egg problem. I will try and explain. > > DXE Memory protection has dependencies that cannot be fulfilled in StMM. A > non-exhaustive list is: > > 1. Dependency on CPU_ARCH protocol > 2. Dependency on Loaded Image patch protocol > 3. Dependency on Boot services > > There is an inherent assumption that this support will never be used in > SMM. Furthermore, in StMM, permissions are changed when the StMM drivers are > first dispatched. A dependency on a driver to change the permissions is the > chicken and egg. So we need a library. > > One option is to introduce a memory protection library in StMM i.e. a library > interface like StandaloneMmImageProtect(). This function will be called from > generic code after the PE-COFF loader has loaded and relocated the StMM driver > image. However, this support is not required on x86. They will have to > include a > NULL library implementation. This would be in addition to the NULL > PeCoffExtraActionLib they already include through MdePkg.dsc. > > I am hesitant to take this approach in the absence of a requirement on x86. At > the same time, the current approach of leveraging the > DebugPeCoffExtraActionLib > in ArmPkg does not make sense either. > > IMO, the better approach would be to add a AArch64 specific > StandaloneMmPeCoffExtraActionLib in the StandaloneMmPkg. Memory protection > will > be implemented in the relocation hook. There will be no impact on x86 or the > ArmPkg. If in future there is a requirement to support this feature on x86 as > well, then a separate library could be implemented. > > Please let us know if this sounds reasonable to you. Sughosh will be posting > the > patches with this approach in a bit to aid the discussion. > > Cheers, > Achin > > [1]
Re: [edk2] [PATCH] BaseTools: Allow multiple PciDeviceId in Fdf OptionROM override
Sorry, was travelling to plugfest. Respinning patches now.
Cheers,
Tom
On 12/10/2018 06:53, Zhu, Yonghong wrote:
> Hi Pilar,
>
> Will you update a V2 to cover Jaben's comment ?
>
> Best Regards,
> Zhu Yonghong
>
>
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Carsey, Jaben
> Sent: Monday, October 8, 2018 11:00 PM
> To: Gao, Liming ; Tomas Pilar (tpilar)
> ; edk2-devel@lists.01.org
> Subject: Re: [edk2] [PATCH] BaseTools: Allow multiple PciDeviceId in Fdf
> OptionROM override
>
>
>
>> -Original Message-
>> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
>> Gao, Liming
>> Sent: Sunday, October 07, 2018 6:42 PM
>> To: Tomas Pilar (tpilar) ;
>> edk2-devel@lists.01.org
>> Subject: Re: [edk2] [PATCH] BaseTools: Allow multiple PciDeviceId in
>> Fdf OptionROM override
>>
>> Pilar:
>> The change is good. Could you also update INF and FDF spec for this usage?
>> If you don't know how to update INF and FDF spec, please submit BZ. I
>> will provide the spec patch.
>>
>> Reviewed-by: Liming Gao
>>
>> Thanks
>> Liming
>>> -Original Message-
>>> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf
>>> Of Tomas Pilar (tpilar)
>>> Sent: Tuesday, October 02, 2018 10:46 PM
>>> To: edk2-devel@lists.01.org
>>> Subject: [edk2] [PATCH] BaseTools: Allow multiple PciDeviceId in Fdf
>>> OptionROM override
>>>
>>> Contributed-under: TianoCore Contribution Agreement 1.1
>>> Signed-off-by: Tomas Pilar
>>> ---
>>> BaseTools/Source/Python/GenFds/FdfParser.py | 11 ---
>>> 1 file changed, 8 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/BaseTools/Source/Python/GenFds/FdfParser.py
>>> b/BaseTools/Source/Python/GenFds/FdfParser.py
>>> index 63687e98bb..a65f2cfd2d 100644
>>> --- a/BaseTools/Source/Python/GenFds/FdfParser.py
>>> +++ b/BaseTools/Source/Python/GenFds/FdfParser.py
>>> @@ -4469,10 +4469,15 @@ class FdfParser:
>>> if self.__IsKeyword( "PCI_DEVICE_ID"):
>>> if not self.__IsToken( "="):
>>> raise Warning("expected '='", self.FileName,
>>> self.CurrentLineNumber)
>>> -if not self.__GetNextHexNumber():
>>> -raise Warning("expected Hex device id",
>>> self.FileName,
>>> self.CurrentLineNumber)
>>>
>>> -Overrides.PciDeviceId = self.__Token
>>> +# Get a list of PCI IDs
>>> +Overrides.PciDeviceId = ""
>>> +
>>> +while self.__GetNextHexNumber():
>>> +Overrides.PciDeviceId += " " + self.__Token
> Can we change to minimize looping string concatenation here? This in a loop
> will cause lots of memory allocation/deallocation and slow things down.
>
> Maybe :
> Overrides.PciDeviceId = "{} {}".format(Overrides.PciDeviceId, self.__Token)
>
>
>>> +
>>> +if not Overrides.PciDeviceId:
>>> +raise Warning("expected one or more Hex
>>> + device ids",
>>> self.FileName, self.CurrentLineNumber)
>>> continue
>>>
>>> if self.__IsKeyword( "PCI_REVISION"):
>>> --
>>> 2.17.1
>>>
>>> ___
>>> edk2-devel mailing list
>>> edk2-devel@lists.01.org
>>> https://lists.01.org/mailman/listinfo/edk2-devel
>> ___
>> edk2-devel mailing list
>> edk2-devel@lists.01.org
>> https://lists.01.org/mailman/listinfo/edk2-devel
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH 4/4] OvmfPkg: simply use the Bochs interface for vmsvga
On Wed, Oct 24, 2018 at 02:40:08PM +0800, yuchen...@synology.com wrote: > From: yuchenlin > > BAR |std vga | vmsvga > - > 0| Framebuffer | I/O space > 1| Reserved | Framebuffer > 2| MMIO | FIFO > > Because of the PCI BARs difference between std vga and > vmsvga, we can not simply recognize the "QEMU VMWare SVGA" > as the QEMU_VIDEO_BOCHS_MMIO variant. > > Instead, we remain variant QEMU_VIDEO_VMWARE_SVGA, and use > it for: > > (1) Get framebuffer from correct PCI BAR > (2) Prevent using BAR2 for MMIO looks good to me. seavgabios uses the same logic. cheers, Gerd ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v2 7/7] ArmPkg: Extra action to update permissions for S-ELO MM Image
Hi Ard, Please see CIL.. On Fri, Aug 24, 2018 at 03:55:29PM +0100, Ard Biesheuvel wrote: > On 21 August 2018 at 07:50, Sughosh Ganu wrote: > > hi Ard, > > > > On Tue July 23, 2018 at 11:03PM +0530, Supreeth Venkatesh wrote: > >> > >> On Sat, 2018-07-21 at 20:06 +0900, Ard Biesheuvel wrote: > >> > On 20 July 2018 at 21:38, Sughosh Ganu wrote: > >> > > > >> > > From: Achin Gupta > >> > > > >> > > The Standalone MM drivers runs in S-EL0 in AArch64 on ARM Standard > >> > > Platforms and is deployed during SEC phase. The memory allocated to > >> > > the Standalone MM drivers should be marked as RO+X. > >> > > > >> > > During PE/COFF Image section parsing, this patch implements extra > >> > > action "UpdatePeCoffPermissions" to request the privileged firmware > >> > > in > >> > > EL3 to update the permissions. > >> > > > >> > > Contributed-under: TianoCore Contribution Agreement 1.1 > >> > > Signed-off-by: Sughosh Ganu > >> > Apologies for bringing this up only now, but I don't think I was ever > >> > cc'ed on these patches. > >> > > >> Apologies if you have missed it. But I am pretty sure it was part of > >> earlier large patch-set on which you and leif were copied, as it was > >> part of ArmPkg. > >> > > >> > We are relying on a debug hook in the PE/COFF loader to ensure that > >> > we > >> > don't end up with memory that is both writable and executable in the > >> > secure world. Do we really think that is a good idea? > >> > > >> > (I know this code was derived from a proof of concept that I did > >> > years > >> > ago, but that was just a PoC) > >> I think we need a little bit more details on what is your suggestion? > >> > >> A little bit background here: This code runs in S-EL0 and Request gets > >> sent to secure world SPM to ensure that the region permissions are > >> updated correctly via the "ArmMmuStandaloneMmCoreLib" SVC - > >> ARM_SVC_ID_SP_SET_MEM_ATTRIBUTES_AARCH64. > >> > >> DebugPeCoffExtraActionLib is just used to extract image region > >> information, but the region permission > >> update request is sent to secure world for validation. > >> > >> With the above explanation, can you provide an insight into what was > >> your thinking? > >> Do you want us to create a separate library and call it > >> as PeCoffExtraActionLib to avoid the "Debug" word though it is a hook > >> to PeCoffExtraActionLib in MdePkg or do we want to create this library > >> in a separate package (may be in MdePkg?) or something totally > >> different. > > > > Supreeth had replied to your comments on the patch. Can you please > > check this. If you feel that this needs to be implemented differently, > > can you please suggest it to us. Thanks. > > > > My point is that such a fundamental action that needs to occur while > loading the PE/COFF image should not be hooked into the loader this > way. Based upon our discussion at the Linaro Connect, we investigated leveraging the DXE Image Protection support [1] in Standalone MM (StMM). Amongst other challenges, there is a chicken and egg problem. I will try and explain. DXE Memory protection has dependencies that cannot be fulfilled in StMM. A non-exhaustive list is: 1. Dependency on CPU_ARCH protocol 2. Dependency on Loaded Image patch protocol 3. Dependency on Boot services There is an inherent assumption that this support will never be used in SMM. Furthermore, in StMM, permissions are changed when the StMM drivers are first dispatched. A dependency on a driver to change the permissions is the chicken and egg. So we need a library. One option is to introduce a memory protection library in StMM i.e. a library interface like StandaloneMmImageProtect(). This function will be called from generic code after the PE-COFF loader has loaded and relocated the StMM driver image. However, this support is not required on x86. They will have to include a NULL library implementation. This would be in addition to the NULL PeCoffExtraActionLib they already include through MdePkg.dsc. I am hesitant to take this approach in the absence of a requirement on x86. At the same time, the current approach of leveraging the DebugPeCoffExtraActionLib in ArmPkg does not make sense either. IMO, the better approach would be to add a AArch64 specific StandaloneMmPeCoffExtraActionLib in the StandaloneMmPkg. Memory protection will be implemented in the relocation hook. There will be no impact on x86 or the ArmPkg. If in future there is a requirement to support this feature on x86 as well, then a separate library could be implemented. Please let us know if this sounds reasonable to you. Sughosh will be posting the patches with this approach in a bit to aid the discussion. Cheers, Achin [1] MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c > ___ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel ___ edk2-devel mailing list edk2-devel@lists.01.org
[edk2] [Patch] BaseTools: list .nasm include inc files as its dependency
From: zhijufan
.nasm source file may include some header files.
header file should be listed in Makefile as .nasm source file
dependency.
But now, BaseTools doesn't find them and list them in Makefile.
This is a missing, because original ASM file supports it.
Cc: Liming Gao
Cc: Yonghong Zhu
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Zhiju.Fan
Signed-off-by: Yonghong Zhu
---
BaseTools/Source/Python/AutoGen/GenMake.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/BaseTools/Source/Python/AutoGen/GenMake.py
b/BaseTools/Source/Python/AutoGen/GenMake.py
index b4377ee..d94d8f9 100644
--- a/BaseTools/Source/Python/AutoGen/GenMake.py
+++ b/BaseTools/Source/Python/AutoGen/GenMake.py
@@ -28,11 +28,11 @@ from .BuildEngine import *
import Common.GlobalData as GlobalData
from collections import OrderedDict
from Common.DataType import TAB_COMPILER_MSFT
## Regular expression for finding header file inclusions
-gIncludePattern = re.compile(r"^[ \t]*#?[ \t]*include(?:[
\t]*(?:\\(?:\r\n|\r|\n))*[ \t]*)*(?:\(?[\"<]?[ \t]*)([-\w.\\/() \t]+)(?:[
\t]*[\">]?\)?)", re.MULTILINE | re.UNICODE | re.IGNORECASE)
+gIncludePattern = re.compile(r"^[ \t]*[#%]?[ \t]*include(?:[
\t]*(?:\\(?:\r\n|\r|\n))*[ \t]*)*(?:\(?[\"<]?[ \t]*)([-\w.\\/() \t]+)(?:[
\t]*[\">]?\)?)", re.MULTILINE | re.UNICODE | re.IGNORECASE)
## Regular expression for matching macro used in header file inclusion
gMacroPattern = re.compile("([_A-Z][_A-Z0-9]*)[ \t]*\((.+)\)", re.UNICODE)
gIsFileMap = {}
--
2.6.1.windows.1
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH] BaseTools: Not convert the void* pcd string in command line to array.
Reviewed-by: Bob Feng
-Original Message-
From: Zhao, ZhiqiangX
Sent: Monday, October 22, 2018 6:04 PM
To: edk2-devel@lists.01.org
Cc: Zhao, ZhiqiangX ; Gao, Liming
; Zhu, Yonghong ; Feng, Bob C
Subject: [PATCH] BaseTools: Not convert the void* pcd string in command line to
array.
For void* type pcd in command line, if its value is string, code should not
convert the void* pcd string in command line to array, otherwise it will make
the pcd value in report not match its real raw value.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: ZhiqiangX Zhao
Cc: Liming Gao
Cc: Yonghong Zhu
Cc: Bob Feng
---
BaseTools/Source/Python/AutoGen/AutoGen.py| 2 +-
BaseTools/Source/Python/Workspace/DscBuildData.py | 38 +--
2 files changed, 9 insertions(+), 31 deletions(-)
diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py
b/BaseTools/Source/Python/AutoGen/AutoGen.py
index 804f579f5c..15d9706e35 100644
--- a/BaseTools/Source/Python/AutoGen/AutoGen.py
+++ b/BaseTools/Source/Python/AutoGen/AutoGen.py
@@ -1168,7 +1168,7 @@ class PlatformAutoGen(AutoGen):
VariableGuidStructure = Sku.VariableGuidValue
VariableGuid =
GuidStructureStringToGuidString(VariableGuidStructure)
for StorageName in Sku.DefaultStoreDict:
-VariableInfo.append_variable(var_info(Index, pcdname,
StorageName, SkuName, StringToArray(Sku.VariableName), VariableGuid,
Sku.VariableOffset, Sku.VariableAttribute, Sku.HiiDefaultValue,
Sku.DefaultStoreDict[StorageName], Pcd.DatumType,
Pcd.CustomAttribute['DscPosition'], Pcd.CustomAttribute.get('IsStru',False)))
+VariableInfo.append_variable(var_info(Index,
+ pcdname, StorageName, SkuName, StringToArray(Sku.VariableName),
+ VariableGuid, Sku.VariableOffset, Sku.VariableAttribute,
+ Sku.HiiDefaultValue, Sku.DefaultStoreDict[StorageName] if
+ Pcd.DatumType in TAB_PCD_NUMERIC_TYPES else
+ StringToArray(Sku.DefaultStoreDict[StorageName]), Pcd.DatumType,
+ Pcd.CustomAttribute['DscPosition'],
+ Pcd.CustomAttribute.get('IsStru',False)))
Index += 1
return VariableInfo
diff --git a/BaseTools/Source/Python/Workspace/DscBuildData.py
b/BaseTools/Source/Python/Workspace/DscBuildData.py
index b0e88a93ce..7ae2ff0683 100644
--- a/BaseTools/Source/Python/Workspace/DscBuildData.py
+++ b/BaseTools/Source/Python/Workspace/DscBuildData.py
@@ -1307,29 +1307,16 @@ class DscBuildData(PlatformBuildClassObject):
if isinstance(self._DecPcds.get((Pcd.TokenCName,
Pcd.TokenSpaceGuidCName), None), StructurePcd):
self._DecPcds.get((Pcd.TokenCName,
Pcd.TokenSpaceGuidCName)).PcdValueFromComm =
NoFiledValues[(Pcd.TokenSpaceGuidCName, Pcd.TokenCName)][0]
else:
-if Pcd.DatumType in TAB_PCD_NUMERIC_TYPES:
-Pcd.PcdValueFromComm =
NoFiledValues[(Pcd.TokenSpaceGuidCName, Pcd.TokenCName)][0]
-Pcd.DefaultValue =
NoFiledValues[(Pcd.TokenSpaceGuidCName, Pcd.TokenCName)][0]
-else:
-Pcd.PcdValueFromComm =
StringToArray(NoFiledValues[(Pcd.TokenSpaceGuidCName, Pcd.TokenCName)][0])
-Pcd.DefaultValue =
StringToArray(NoFiledValues[(Pcd.TokenSpaceGuidCName, Pcd.TokenCName)][0])
+Pcd.PcdValueFromComm =
NoFiledValues[(Pcd.TokenSpaceGuidCName, Pcd.TokenCName)][0]
+Pcd.DefaultValue =
+ NoFiledValues[(Pcd.TokenSpaceGuidCName, Pcd.TokenCName)][0]
for sku in Pcd.SkuInfoList:
SkuInfo = Pcd.SkuInfoList[sku]
if SkuInfo.DefaultValue:
-if Pcd.DatumType in TAB_PCD_NUMERIC_TYPES:
-SkuInfo.DefaultValue =
NoFiledValues[(Pcd.TokenSpaceGuidCName, Pcd.TokenCName)][0]
-else:
-SkuInfo.DefaultValue =
StringToArray(NoFiledValues[(Pcd.TokenSpaceGuidCName, Pcd.TokenCName)][0])
+SkuInfo.DefaultValue =
+ NoFiledValues[(Pcd.TokenSpaceGuidCName, Pcd.TokenCName)][0]
else:
-if Pcd.DatumType in TAB_PCD_NUMERIC_TYPES:
-SkuInfo.HiiDefaultValue =
NoFiledValues[(Pcd.TokenSpaceGuidCName, Pcd.TokenCName)][0]
-else:
-SkuInfo.HiiDefaultValue =
StringToArray(NoFiledValues[(Pcd.TokenSpaceGuidCName, Pcd.TokenCName)][0])
+SkuInfo.HiiDefaultValue =
+ NoFiledValues[(Pcd.TokenSpaceGuidCName, Pcd.TokenCName)][0]
for defaultstore in SkuInfo.DefaultStoreDict:
-if Pcd.DatumType in TAB_PCD_NUMERIC_TYPES:
-SkuInfo.DefaultStoreDict[defaultstore] =
Re: [edk2] [PATCH V2] BaseTools: Convert "Unicode string" to "byte array" if value type diff
Reviewed-by: Bob Feng
-Original Message-
From: Zhao, ZhiqiangX
Sent: Thursday, October 18, 2018 1:09 PM
To: edk2-devel@lists.01.org
Cc: Zhao, ZhiqiangX ; Gao, Liming
; Zhu, Yonghong ; Feng, Bob C
Subject: [PATCH V2] BaseTools: Convert "Unicode string" to "byte array" if
value type diff
V2:
Fixed 3 typo.
Use startswith(('L"',"L'")) to check if a string is Unicode string.
Use a set PcdValueTypeSet instead of a list PcdValueTypeList to save memory.
V1:
For the same one VOID* pcd, if the default value type of one SKU is "Unicode
string", the other SKUs are "OtherVOID*"(ASCII string or byte array),Then
convert "Unicode string" to "byte array".
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: ZhiqiangX Zhao
Cc: Liming Gao
Cc: Yonghong Zhu
Cc: Bob Feng
---
BaseTools/Source/Python/Workspace/DscBuildData.py | 9 +
1 file changed, 9 insertions(+)
diff --git a/BaseTools/Source/Python/Workspace/DscBuildData.py
b/BaseTools/Source/Python/Workspace/DscBuildData.py
index 7854e71db6..9b9ace9b56 100644
--- a/BaseTools/Source/Python/Workspace/DscBuildData.py
+++ b/BaseTools/Source/Python/Workspace/DscBuildData.py
@@ -2877,6 +2877,15 @@ class DscBuildData(PlatformBuildClassObject):
elif TAB_DEFAULT in pcd.SkuInfoList and TAB_COMMON in
pcd.SkuInfoList:
del pcd.SkuInfoList[TAB_COMMON]
+#For the same one VOID* pcd, if the default value type of one SKU is
"Unicode string",
+#the other SKUs are "OtherVOID*"(ASCII string or byte array),Then
convert "Unicode string" to "byte array".
+for pcd in Pcds.values():
+PcdValueTypeSet = set()
+for sku in pcd.SkuInfoList.values():
+PcdValueTypeSet.add("UnicodeString" if
sku.DefaultValue.startswith(('L"',"L'")) else "OtherVOID*")
+if len(PcdValueTypeSet) > 1:
+for sku in pcd.SkuInfoList.values():
+sku.DefaultValue = StringToArray(sku.DefaultValue)
+ if sku.DefaultValue.startswith(('L"',"L'")) else sku.DefaultValue
map(self.FilterSkuSettings, Pcds.values())
return Pcds
--
2.14.1.windows.1
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH V2] BaseTool: Support different PCDs that refers to the same EFI variable.
Reviewed-by: Bob Feng
-Original Message-
From: Zhao, ZhiqiangX
Sent: Thursday, October 18, 2018 3:12 PM
To: edk2-devel@lists.01.org
Cc: Zhao, ZhiqiangX ; Gao, Liming
; Zhu, Yonghong ; Feng, Bob C
Subject: [PATCH V2] BaseTool: Support different PCDs that refers to the same
EFI variable.
V2:
Make the code of patch both compatible for Python2 and Python3.
V1:
If different PCDs refer to the same EFI variable, then do EFI variable
combination, according to the VariableOffset of different PCDS.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: ZhiqiangX Zhao
Cc: Liming Gao
Cc: Yonghong Zhu
Cc: Bob Feng
---
BaseTools/Source/Python/AutoGen/GenVar.py | 97 ++-
1 file changed, 30 insertions(+), 67 deletions(-)
diff --git a/BaseTools/Source/Python/AutoGen/GenVar.py
b/BaseTools/Source/Python/AutoGen/GenVar.py
index 036f00e2bb..98f88e2497 100644
--- a/BaseTools/Source/Python/AutoGen/GenVar.py
+++ b/BaseTools/Source/Python/AutoGen/GenVar.py
@@ -56,51 +56,7 @@ class VariableMgr(object):
value_str += ",".join(default_var_bin_strip)
value_str += "}"
return value_str
-def Do_combine(self,sku_var_info_offset_list):
-newvalue = {}
-for item in sku_var_info_offset_list:
-data_type = item.data_type
-value_list = item.default_value.strip("{").strip("}").split(",")
-if data_type in DataType.TAB_PCD_NUMERIC_TYPES:
-data_flag =
DataType.PACK_CODE_BY_SIZE[MAX_SIZE_TYPE[data_type]]
-data = value_list[0]
-value_list = []
-for data_byte in pack(data_flag, int(data, 16) if
data.upper().startswith('0X') else int(data)):
-value_list.append(hex(unpack("B", data_byte)[0]))
-newvalue[int(item.var_offset, 16) if
item.var_offset.upper().startswith("0X") else int(item.var_offset)] = value_list
-try:
-newvaluestr = "{" +
",".join(VariableMgr.assemble_variable(newvalue)) +"}"
-except:
-EdkLogger.error("build", AUTOGEN_ERROR, "Variable offset conflict
in PCDs: %s \n" % (" and ".join(item.pcdname for item in
sku_var_info_offset_list)))
-return newvaluestr
-def Do_Merge(self,sku_var_info_offset_list):
-StructrurePcds = sorted([item for item in sku_var_info_offset_list if
item.StructurePcd], key = lambda x: x.PcdDscLine, reverse =True )
-Base = StructrurePcds[0]
-BaseValue = Base.default_value.strip("{").strip("}").split(",")
-Override = [item for item in sku_var_info_offset_list if not
item.StructurePcd and item.PcdDscLine > Base.PcdDscLine]
-newvalue = {}
-for item in Override:
-data_type = item.data_type
-value_list = item.default_value.strip("{").strip("}").split(",")
-if data_type in DataType.TAB_PCD_NUMERIC_TYPES:
-data_flag =
DataType.PACK_CODE_BY_SIZE[MAX_SIZE_TYPE[data_type]]
-data = value_list[0]
-value_list = []
-for data_byte in pack(data_flag, int(data, 16) if
data.upper().startswith('0X') else int(data)):
-value_list.append(hex(unpack("B", data_byte)[0]))
-newvalue[int(item.var_offset, 16) if
item.var_offset.upper().startswith("0X") else int(item.var_offset)] =
(value_list,item.pcdname,item.PcdDscLine)
-for offset in newvalue:
-value_list,itemPcdname,itemPcdDscLine = newvalue[offset]
-if offset > len(BaseValue) or (offset + len(value_list) >
len(BaseValue)):
-EdkLogger.error("build", AUTOGEN_ERROR, "The EFI Variable
referred by PCD %s in line %s exceeds variable size: %s\n" %
(itemPcdname,itemPcdDscLine,hex(len(BaseValue
-for i in xrange(len(value_list)):
-BaseValue[offset + i] = value_list[i]
-newvaluestr = "{" + ",".join(BaseValue) +"}"
-return newvaluestr
-def NeedMerge(self,sku_var_info_offset_list):
-if [item for item in sku_var_info_offset_list if item.StructurePcd]:
-return True
-return False
+
def combine_variable(self):
indexedvarinfo = collections.OrderedDict()
for item in self.VarInfo:
@@ -109,30 +65,37 @@ class VariableMgr(object):
indexedvarinfo[(item.skuname, item.defaultstoragename,
item.var_name, item.var_guid)].append(item)
for key in indexedvarinfo:
sku_var_info_offset_list = indexedvarinfo[key]
-if len(sku_var_info_offset_list) == 1:
-continue
-
+sku_var_info_offset_list.sort(key=lambda x:x.PcdDscLine)
+FirstOffset = int(sku_var_info_offset_list[0].var_offset, 16) if
sku_var_info_offset_list[0].var_offset.upper().startswith("0X") else
int(sku_var_info_offset_list[0].var_offset)
+fisrtvalue_list =
sku_var_info_offset_list[0].default_value.strip("{").strip("}").split(",")
+
Re: [edk2] [PATCH] BaseTools:Translate the StructurePCD value in field to correct format.
Reviewed-by: Bob Feng
-Original Message-
From: Zhao, ZhiqiangX
Sent: Monday, October 22, 2018 3:06 PM
To: edk2-devel@lists.01.org
Cc: Zhao, ZhiqiangX ; Gao, Liming
; Zhu, Yonghong ; Feng, Bob C
Subject: [PATCH] BaseTools:Translate the StructurePCD value in field to correct
format.
For StructurePCD value got from DSC file, translate its field value in to
correct format in report.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: ZhiqiangX Zhao
Cc: Liming Gao
Cc: Yonghong Zhu
Cc: Bob Feng
---
BaseTools/Source/Python/Workspace/DscBuildData.py | 7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/BaseTools/Source/Python/Workspace/DscBuildData.py
b/BaseTools/Source/Python/Workspace/DscBuildData.py
index 17e6f62cac..9b1be3ec59 100644
--- a/BaseTools/Source/Python/Workspace/DscBuildData.py
+++ b/BaseTools/Source/Python/Workspace/DscBuildData.py
@@ -1925,15 +1925,12 @@ class DscBuildData(PlatformBuildClassObject):
IsArray = IsFieldValueAnArray(FieldList[FieldName][0])
if IsArray:
try:
-FieldValue =
ValueExpressionEx(FieldList[FieldName][0], TAB_VOID, self._GuidDict)(True)
+FieldList[FieldName][0] =
+ ValueExpressionEx(FieldList[FieldName][0], TAB_VOID,
+ self._GuidDict)(True)
except BadExpression:
EdkLogger.error('Build', FORMAT_INVALID, "Invalid
value format for %s. From %s Line %d " %
(".".join((Pcd.TokenSpaceGuidCName, Pcd.TokenCName, FieldName)),
FieldList[FieldName][1], FieldList[FieldName][2]))
try:
-if IsArray:
-Value, ValueSize = ParseFieldValue (FieldValue)
-else:
-Value, ValueSize = ParseFieldValue
(FieldList[FieldName][0])
+Value, ValueSize = ParseFieldValue
+ (FieldList[FieldName][0])
except Exception:
EdkLogger.error('Build', FORMAT_INVALID, "Invalid
value format for %s. From %s Line %d " % (".".join((Pcd.TokenSpaceGuidCName,
Pcd.TokenCName, FieldName)), FieldList[FieldName][1], FieldList[FieldName][2]))
if isinstance(Value, str):
--
2.14.1.windows.1
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH] BaseTools: add ASSERT checker for array buffer in fdf and command line
Reviewed-by: Bob Feng
-Original Message-
From: Zhao, ZhiqiangX
Sent: Monday, October 22, 2018 4:38 PM
To: edk2-devel@lists.01.org
Cc: Zhao, ZhiqiangX ; Gao, Liming
; Zhu, Yonghong ; Feng, Bob C
Subject: [PATCH] BaseTools: add ASSERT checker for array buffer in fdf and
command line
For structure PCD in fdf file and command line, 1. use compiler time assert to
check the array index, report error if array index exceeds the array number.
2. use compiler time assert to check the array size, report error if the user
declared size in header file is smaller than the user used in fdf file and
command line.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: ZhiqiangX Zhao
Cc: Liming Gao
Cc: Yonghong Zhu
Cc: Bob Feng
---
BaseTools/Source/Python/Workspace/DscBuildData.py | 8
1 file changed, 8 insertions(+)
diff --git a/BaseTools/Source/Python/Workspace/DscBuildData.py
b/BaseTools/Source/Python/Workspace/DscBuildData.py
index b0e88a93ce..01a565aa08 100644
--- a/BaseTools/Source/Python/Workspace/DscBuildData.py
+++ b/BaseTools/Source/Python/Workspace/DscBuildData.py
@@ -2013,8 +2013,12 @@ class DscBuildData(PlatformBuildClassObject):
#
CApp = CApp + ' FieldSize = __FIELD_SIZE(%s, %s);\n' %
(Pcd.DatumType, FieldName)
CApp = CApp + ' Value = %s; // From %s Line %d Value
%s\n' % (DscBuildData.IntToCString(Value, ValueSize), FieldList[FieldName][1],
FieldList[FieldName][2], FieldList[FieldName][0])
+CApp = CApp + ' __STATIC_ASSERT((__FIELD_SIZE(%s,
+ %s) >= %d) || (__FIELD_SIZE(%s, %s) == 0), "Input buffer exceeds the
+ buffer array"); // From %s Line %d Value %s\n' % (Pcd.DatumType,
+ FieldName, ValueSize, Pcd.DatumType, FieldName,
+ FieldList[FieldName][1], FieldList[FieldName][2],
+ FieldList[FieldName][0])
CApp = CApp + ' memcpy (>%s, Value, (FieldSize > 0
&& FieldSize < %d) ? FieldSize : %d);\n' % (FieldName, ValueSize, ValueSize)
else:
+if '[' in FieldName and ']' in FieldName:
+Index = int(FieldName.split('[')[1].split(']')[0])
+CApp = CApp + ' __STATIC_ASSERT((%d <
+ __ARRAY_SIZE(Pcd->%s)) || (__ARRAY_SIZE(Pcd->%s) == 0), "array index
+ exceeds the array number"); // From %s Line %d Index of %s\n' %
+ (Index, FieldName.split('[')[0], FieldName.split('[')[0],
+ FieldList[FieldName][1], FieldList[FieldName][2], FieldName)
if ValueSize > 4:
CApp = CApp + ' Pcd->%s = %dULL; // From %s Line %d
Value %s\n' % (FieldName, Value, FieldList[FieldName][1],
FieldList[FieldName][2], FieldList[FieldName][0])
else:
@@ -2077,8 +2081,12 @@ class DscBuildData(PlatformBuildClassObject):
#
CApp = CApp + ' FieldSize = __FIELD_SIZE(%s, %s);\n' %
(Pcd.DatumType, FieldName)
CApp = CApp + ' Value = %s; // From %s Line %d Value
%s\n' % (DscBuildData.IntToCString(Value, ValueSize), FieldList[FieldName][1],
FieldList[FieldName][2], FieldList[FieldName][0])
+CApp = CApp + ' __STATIC_ASSERT((__FIELD_SIZE(%s,
+ %s) >= %d) || (__FIELD_SIZE(%s, %s) == 0), "Input buffer exceeds the
+ buffer array"); // From %s Line %d Value %s\n' % (Pcd.DatumType,
+ FieldName, ValueSize, Pcd.DatumType, FieldName,
+ FieldList[FieldName][1], FieldList[FieldName][2],
+ FieldList[FieldName][0])
CApp = CApp + ' memcpy (>%s, Value, (FieldSize > 0
&& FieldSize < %d) ? FieldSize : %d);\n' % (FieldName, ValueSize, ValueSize)
else:
+if '[' in FieldName and ']' in FieldName:
+Index = int(FieldName.split('[')[1].split(']')[0])
+CApp = CApp + ' __STATIC_ASSERT((%d <
+ __ARRAY_SIZE(Pcd->%s)) || (__ARRAY_SIZE(Pcd->%s) == 0), "array index
+ exceeds the array number"); // From %s Line %d Index of %s\n' %
+ (Index, FieldName.split('[')[0], FieldName.split('[')[0],
+ FieldList[FieldName][1], FieldList[FieldName][2], FieldName)
if ValueSize > 4:
CApp = CApp + ' Pcd->%s = %dULL; // From %s Line %d
Value %s\n' % (FieldName, Value, FieldList[FieldName][1],
FieldList[FieldName][2], FieldList[FieldName][0])
else:
--
2.14.1.windows.1
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [Patch] BaseTools: Fix the bug for Pcd used in command line's override
Fix the bug for Pcd used in command line not override the Pcd used
in the [component] driver's sub-section.
Case:
DSC file:
[PcdsFixedAtBuild]
TokenSpaceGuid.PcdTest
[Components]
TestPkg/TestDriver.inf {
TokenSpaceGuid.PcdTest|"b"
}
build command with --pcd TokenSpaceGuid.PcdTest="BB"
Then we found the Pcd value in the AutoGen.c file is incorrect,
because of the incorrect logic that use the pcd in the [component]
section to re-override it.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Yonghong Zhu
---
BaseTools/Source/Python/AutoGen/AutoGen.py| 7 +++
BaseTools/Source/Python/Workspace/DscBuildData.py | 5 +
2 files changed, 12 insertions(+)
diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py
b/BaseTools/Source/Python/AutoGen/AutoGen.py
index 804f579..84645e3 100644
--- a/BaseTools/Source/Python/AutoGen/AutoGen.py
+++ b/BaseTools/Source/Python/AutoGen/AutoGen.py
@@ -2118,10 +2118,17 @@ class PlatformAutoGen(AutoGen):
# override PCD settings with module specific setting
if Module in self.Platform.Modules:
PlatformModule = self.Platform.Modules[str(Module)]
for Key in PlatformModule.Pcds:
+if GlobalData.BuildOptionPcd:
+for i, pcd in enumerate(GlobalData.BuildOptionPcd):
+(TokenSpaceGuidCName, TokenCName, FieldName, pcdvalue,
_) = pcd
+if (TokenCName, TokenSpaceGuidCName) == Key and
FieldName =="":
+PlatformModule.Pcds[Key].DefaultValue = pcdvalue
+PlatformModule.Pcds[Key].PcdValueFromComm =
pcdvalue
+break
Flag = False
if Key in Pcds:
ToPcd = Pcds[Key]
Flag = True
elif Key in GlobalData.MixedPcd:
diff --git a/BaseTools/Source/Python/Workspace/DscBuildData.py
b/BaseTools/Source/Python/Workspace/DscBuildData.py
index b0e88a9..162360c 100644
--- a/BaseTools/Source/Python/Workspace/DscBuildData.py
+++ b/BaseTools/Source/Python/Workspace/DscBuildData.py
@@ -1058,11 +1058,16 @@ class DscBuildData(PlatformBuildClassObject):
IsValid, Cause = CheckPcdDatum(PcdDatumType, pcdvalue)
if not IsValid:
EdkLogger.error("build", FORMAT_INVALID, Cause,
ExtraData="%s.%s" % (TokenSpaceGuidCName, TokenCName))
GlobalData.BuildOptionPcd[i] = (TokenSpaceGuidCName,
TokenCName, FieldName, pcdvalue, ("build command options", 1))
+if GlobalData.BuildOptionPcd:
+for i, pcd in enumerate(GlobalData.BuildOptionPcd):
+(TokenSpaceGuidCName, TokenCName, FieldName, pcdvalue, _) = pcd
for BuildData in self._Bdb._CACHE_.values():
+if BuildData.Arch != self.Arch:
+continue
if BuildData.MetaFile.Ext == '.dec' or
BuildData.MetaFile.Ext == '.dsc':
continue
for key in BuildData.Pcds:
PcdItem = BuildData.Pcds[key]
if (TokenSpaceGuidCName, TokenCName) ==
(PcdItem.TokenSpaceGuidCName, PcdItem.TokenCName) and FieldName =="":
--
2.6.1.windows.1
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH 4/4] OvmfPkg: simply use the Bochs interface for vmsvga
From: yuchenlin
BAR |std vga | vmsvga
-
0| Framebuffer | I/O space
1| Reserved | Framebuffer
2| MMIO | FIFO
Because of the PCI BARs difference between std vga and
vmsvga, we can not simply recognize the "QEMU VMWare SVGA"
as the QEMU_VIDEO_BOCHS_MMIO variant.
Instead, we remain variant QEMU_VIDEO_VMWARE_SVGA, and use
it for:
(1) Get framebuffer from correct PCI BAR
(2) Prevent using BAR2 for MMIO
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: yuchenlin
---
OvmfPkg/QemuVideoDxe/Driver.c | 16 +++-
OvmfPkg/QemuVideoDxe/Gop.c| 3 ++-
OvmfPkg/QemuVideoDxe/Qemu.h | 2 ++
3 files changed, 19 insertions(+), 2 deletions(-)
diff --git a/OvmfPkg/QemuVideoDxe/Driver.c b/OvmfPkg/QemuVideoDxe/Driver.c
index 2304afd1e6..a1785c2285 100644
--- a/OvmfPkg/QemuVideoDxe/Driver.c
+++ b/OvmfPkg/QemuVideoDxe/Driver.c
@@ -16,6 +16,7 @@
#include "Qemu.h"
#include
+#include
EFI_DRIVER_BINDING_PROTOCOL gQemuVideoDriverBinding = {
QemuVideoControllerDriverSupported,
@@ -69,6 +70,12 @@ QEMU_VIDEO_CARD gQemuVideoCardList[] = {
0x1050,
QEMU_VIDEO_BOCHS_MMIO,
L"QEMU VirtIO VGA"
+},{
+PCI_CLASS_DISPLAY_VGA,
+VMWARE_PCI_VENDOR_ID_VMWARE,
+VMWARE_PCI_DEVICE_ID_VMWARE_SVGA2,
+QEMU_VIDEO_VMWARE_SVGA,
+L"QEMU VMWare SVGA"
},{
0 /* end of list */
}
@@ -256,6 +263,11 @@ QemuVideoControllerDriverStart (
goto ClosePciIo;
}
Private->Variant = Card->Variant;
+ Private->FrameBufferVramBarIndex = PCI_BAR_IDX0;
+ if (Private->Variant == QEMU_VIDEO_VMWARE_SVGA) {
+Private->FrameBufferVramBarIndex = PCI_BAR_IDX1;
+ }
+
//
// IsQxl is based on the detected Card->Variant, which at a later point might
@@ -320,7 +332,8 @@ QemuVideoControllerDriverStart (
// Check if accessing the bochs interface works.
//
if (Private->Variant == QEMU_VIDEO_BOCHS_MMIO ||
- Private->Variant == QEMU_VIDEO_BOCHS) {
+ Private->Variant == QEMU_VIDEO_BOCHS ||
+ Private->Variant == QEMU_VIDEO_VMWARE_SVGA) {
UINT16 BochsId;
BochsId = BochsRead(Private, VBE_DISPI_INDEX_ID);
if ((BochsId & 0xFFF0) != VBE_DISPI_ID0) {
@@ -383,6 +396,7 @@ QemuVideoControllerDriverStart (
break;
case QEMU_VIDEO_BOCHS_MMIO:
case QEMU_VIDEO_BOCHS:
+ case QEMU_VIDEO_VMWARE_SVGA:
Status = QemuVideoBochsModeSetup (Private, IsQxl);
break;
default:
diff --git a/OvmfPkg/QemuVideoDxe/Gop.c b/OvmfPkg/QemuVideoDxe/Gop.c
index d490fa7a2e..3abc5eeb36 100644
--- a/OvmfPkg/QemuVideoDxe/Gop.c
+++ b/OvmfPkg/QemuVideoDxe/Gop.c
@@ -60,7 +60,7 @@ QemuVideoCompleteModeData (
Private->PciIo->GetBarAttributes (
Private->PciIo,
-0,
+Private->FrameBufferVramBarIndex,
NULL,
(VOID**)
);
@@ -177,6 +177,7 @@ Routine Description:
break;
case QEMU_VIDEO_BOCHS_MMIO:
case QEMU_VIDEO_BOCHS:
+ case QEMU_VIDEO_VMWARE_SVGA:
InitializeBochsGraphicsMode (Private,
[ModeData->InternalModeIndex]);
break;
default:
diff --git a/OvmfPkg/QemuVideoDxe/Qemu.h b/OvmfPkg/QemuVideoDxe/Qemu.h
index d7da761705..3aac9eeca6 100644
--- a/OvmfPkg/QemuVideoDxe/Qemu.h
+++ b/OvmfPkg/QemuVideoDxe/Qemu.h
@@ -92,6 +92,7 @@ typedef enum {
QEMU_VIDEO_CIRRUS_5446,
QEMU_VIDEO_BOCHS,
QEMU_VIDEO_BOCHS_MMIO,
+ QEMU_VIDEO_VMWARE_SVGA,
} QEMU_VIDEO_VARIANT;
typedef struct {
@@ -120,6 +121,7 @@ typedef struct {
QEMU_VIDEO_VARIANTVariant;
FRAME_BUFFER_CONFIGURE*FrameBufferBltConfigure;
UINTN FrameBufferBltConfigureSize;
+ UINT8 FrameBufferVramBarIndex;
} QEMU_VIDEO_PRIVATE_DATA;
///
--
2.18.0
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH 2/4] Revert "OvmfPkg/QemuVideoDxe: Helper functions for unaligned port I/O."
From: yuchenlin
This reverts commit 05a5379458725234de8a05780fcb5da2c12680e4.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: yuchenlin
---
OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf | 6 --
OvmfPkg/QemuVideoDxe/UnalignedIoGcc.c | 70
OvmfPkg/QemuVideoDxe/UnalignedIoIcc.c | 80 ---
OvmfPkg/QemuVideoDxe/UnalignedIoInternal.h| 59 --
OvmfPkg/QemuVideoDxe/UnalignedIoMsc.c | 78 --
OvmfPkg/QemuVideoDxe/UnalignedIoUnsupported.c | 66 ---
6 files changed, 359 deletions(-)
delete mode 100644 OvmfPkg/QemuVideoDxe/UnalignedIoGcc.c
delete mode 100644 OvmfPkg/QemuVideoDxe/UnalignedIoIcc.c
delete mode 100644 OvmfPkg/QemuVideoDxe/UnalignedIoInternal.h
delete mode 100644 OvmfPkg/QemuVideoDxe/UnalignedIoMsc.c
delete mode 100644 OvmfPkg/QemuVideoDxe/UnalignedIoUnsupported.c
diff --git a/OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf
b/OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf
index a04ed537dd..c071fafa4e 100644
--- a/OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf
+++ b/OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf
@@ -42,15 +42,9 @@
UnalignedIoInternal.h
[Sources.Ia32, Sources.X64]
- UnalignedIoGcc.c| GCC
- UnalignedIoIcc.c| INTEL
- UnalignedIoMsc.c| MSFT
VbeShim.c
VbeShim.h
-[Sources.EBC]
- UnalignedIoUnsupported.c
-
[Packages]
MdePkg/MdePkg.dec
MdeModulePkg/MdeModulePkg.dec
diff --git a/OvmfPkg/QemuVideoDxe/UnalignedIoGcc.c
b/OvmfPkg/QemuVideoDxe/UnalignedIoGcc.c
deleted file mode 100644
index 105d55d3b9..00
--- a/OvmfPkg/QemuVideoDxe/UnalignedIoGcc.c
+++ /dev/null
@@ -1,70 +0,0 @@
-/** @file
- Unaligned Port I/O. This file has compiler specifics for GCC as there is no
- ANSI C standard for doing IO.
-
- Based on IoLibGcc.c.
-
- Copyright (c) 2006 - 2010, Intel Corporation. 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 "UnalignedIoInternal.h"
-
-/**
- Performs a 32-bit write to the specified, possibly unaligned I/O-type
- address.
-
- Writes the 32-bit I/O port specified by Port with the value specified by
- Value and returns Value. This function must guarantee that all I/O read and
- write operations are serialized.
-
- If 32-bit unaligned I/O port operations are not supported, then ASSERT().
-
- @param[in] Port I/O port address
- @param[in] Value 32-bit word to write
-
- @return The value written to the I/O port.
-
-**/
-UINT32
-UnalignedIoWrite32 (
- IN UINTN Port,
- IN UINT32Value
- )
-{
- __asm__ __volatile__ ( "outl %0, %1" : : "a" (Value), "d" ((UINT16)Port) );
- return Value;
-}
-
-/**
- Reads a 32-bit word from the specified, possibly unaligned I/O-type address.
-
- Reads the 32-bit I/O port specified by Port. The 32-bit read value is
- returned. This function must guarantee that all I/O read and write operations
- are serialized.
-
- If 32-bit unaligned I/O port operations are not supported, then ASSERT().
-
- @param[in] Port The I/O port to read.
-
- @return The value read.
-
-**/
-UINT32
-UnalignedIoRead32 (
- IN UINTN Port
- )
-{
- UINT32 Data;
- __asm__ __volatile__ ( "inl %1, %0" : "=a" (Data) : "d" ((UINT16)Port) );
- return Data;
-}
-
diff --git a/OvmfPkg/QemuVideoDxe/UnalignedIoIcc.c
b/OvmfPkg/QemuVideoDxe/UnalignedIoIcc.c
deleted file mode 100644
index 79f3e446dd..00
--- a/OvmfPkg/QemuVideoDxe/UnalignedIoIcc.c
+++ /dev/null
@@ -1,80 +0,0 @@
-/** @file
- Unaligned port I/O. This file has compiler specifics for ICC as there
- is no ANSI C standard for doing IO.
-
- Based on IoLibIcc.c.
-
- Copyright (c) 2006 - 2008, Intel Corporation. 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 "UnalignedIoInternal.h"
-
-/**
- Performs a 32-bit write to the specified, possibly unaligned I/O-type
- address.
-
- Writes the 32-bit I/O port specified by Port with the value specified by
- Value and returns Value. This function must guarantee that all I/O read and
- write operations are serialized.
-
- If 32-bit unaligned I/O port operations are not supported, then ASSERT().
-
- @param[in] Port I/O port address
[edk2] [PATCH 0/4] OvmfPkg: simply use the Bochs interface for vmsvga
From: yuchenlin In this series, replace the original vmsvga driver to Bochs interface. Simply revert vmsvga driver implementation. After it, use Bochs interface for initializing vmsvga. Because of the PCI BARs difference between std vga and vmsvga. We can not simply recognize the "QEMU VMWare SVGA" as the QEMU_VIDEO_BOCHS_MMIO variant. BAR |std vga | vmsvga - 0| Framebuffer | I/O space 1| Reserved | Framebuffer 2| MMIO | FIFO To overcome this problem, we remain variant QEMU_VIDEO_VMWARE_SVGA, and use it for: (1) Get framebuffer from correct PCI BAR (2) Prevent using BAR2 for MMIO We have tested on qemu before and after commit 104bd1dc70 and all worked. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: yuchenlin yuchenlin (4): Revert "OvmfPkg/QemuVideoDxe: VMWare SVGA device support" Revert "OvmfPkg/QemuVideoDxe: Helper functions for unaligned port I/O." Revert "OvmfPkg/QemuVideoDxe: list "UnalignedIoInternal.h" in the INF file" OvmfPkg: simply use the Bochs interface for vmsvga OvmfPkg/QemuVideoDxe/Driver.c | 137 ++- OvmfPkg/QemuVideoDxe/Gop.c| 68 +--- OvmfPkg/QemuVideoDxe/Initialize.c | 157 -- OvmfPkg/QemuVideoDxe/Qemu.h | 27 --- OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf | 7 - OvmfPkg/QemuVideoDxe/UnalignedIoGcc.c | 70 OvmfPkg/QemuVideoDxe/UnalignedIoIcc.c | 80 - OvmfPkg/QemuVideoDxe/UnalignedIoInternal.h| 59 --- OvmfPkg/QemuVideoDxe/UnalignedIoMsc.c | 78 - OvmfPkg/QemuVideoDxe/UnalignedIoUnsupported.c | 66 10 files changed, 17 insertions(+), 732 deletions(-) delete mode 100644 OvmfPkg/QemuVideoDxe/UnalignedIoGcc.c delete mode 100644 OvmfPkg/QemuVideoDxe/UnalignedIoIcc.c delete mode 100644 OvmfPkg/QemuVideoDxe/UnalignedIoInternal.h delete mode 100644 OvmfPkg/QemuVideoDxe/UnalignedIoMsc.c delete mode 100644 OvmfPkg/QemuVideoDxe/UnalignedIoUnsupported.c -- 2.18.0 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH 1/4] Revert "OvmfPkg/QemuVideoDxe: VMWare SVGA device support"
From: yuchenlin
This reverts commit c137d95081690d4877fbeb5f1856972e84ac32f2.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: yuchenlin
---
OvmfPkg/QemuVideoDxe/Driver.c | 135 +
OvmfPkg/QemuVideoDxe/Gop.c| 65 +
OvmfPkg/QemuVideoDxe/Initialize.c | 157 --
OvmfPkg/QemuVideoDxe/Qemu.h | 29 --
4 files changed, 7 insertions(+), 379 deletions(-)
diff --git a/OvmfPkg/QemuVideoDxe/Driver.c b/OvmfPkg/QemuVideoDxe/Driver.c
index 73eb2cad05..2304afd1e6 100644
--- a/OvmfPkg/QemuVideoDxe/Driver.c
+++ b/OvmfPkg/QemuVideoDxe/Driver.c
@@ -14,10 +14,8 @@
**/
-#include
-#include
#include "Qemu.h"
-#include "UnalignedIoInternal.h"
+#include
EFI_DRIVER_BINDING_PROTOCOL gQemuVideoDriverBinding = {
QemuVideoControllerDriverSupported,
@@ -71,12 +69,6 @@ QEMU_VIDEO_CARD gQemuVideoCardList[] = {
0x1050,
QEMU_VIDEO_BOCHS_MMIO,
L"QEMU VirtIO VGA"
-},{
-PCI_CLASS_DISPLAY_VGA,
-VMWARE_PCI_VENDOR_ID_VMWARE,
-VMWARE_PCI_DEVICE_ID_VMWARE_SVGA2,
-QEMU_VIDEO_VMWARE_SVGA,
-L"QEMU VMWare SVGA"
},{
0 /* end of list */
}
@@ -264,7 +256,6 @@ QemuVideoControllerDriverStart (
goto ClosePciIo;
}
Private->Variant = Card->Variant;
- Private->FrameBufferVramBarIndex = PCI_BAR_IDX0;
//
// IsQxl is based on the detected Card->Variant, which at a later point might
@@ -339,58 +330,6 @@ QemuVideoControllerDriverStart (
}
}
- //
- // Check if accessing Vmware SVGA interface works
- //
- if (Private->Variant == QEMU_VIDEO_VMWARE_SVGA) {
-EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *IoDesc;
-UINT32TargetId;
-UINT32SvgaIdRead;
-
-IoDesc = NULL;
-Status = Private->PciIo->GetBarAttributes (
- Private->PciIo,
- PCI_BAR_IDX0,
- NULL,
- (VOID**)
- );
-if (EFI_ERROR (Status) ||
-IoDesc->ResType != ACPI_ADDRESS_SPACE_TYPE_IO ||
-IoDesc->AddrRangeMin > MAX_UINT16 + 1 - (VMWARE_SVGA_VALUE_PORT + 4)) {
- if (IoDesc != NULL) {
-FreePool (IoDesc);
- }
- Status = EFI_DEVICE_ERROR;
- goto RestoreAttributes;
-}
-Private->VmwareSvgaBasePort = (UINT16) IoDesc->AddrRangeMin;
-FreePool (IoDesc);
-
-TargetId = VMWARE_SVGA_ID_2;
-while (TRUE) {
- VmwareSvgaWrite (Private, VmwareSvgaRegId, TargetId);
- SvgaIdRead = VmwareSvgaRead (Private, VmwareSvgaRegId);
- if ((SvgaIdRead == TargetId) || (TargetId <= VMWARE_SVGA_ID_0)) {
-break;
- }
- TargetId--;
-}
-
-if (SvgaIdRead != TargetId) {
- DEBUG ((
-DEBUG_ERROR,
-"QemuVideo: QEMU_VIDEO_VMWARE_SVGA ID mismatch "
-"(got 0x%x, base address 0x%x)\n",
-SvgaIdRead,
-Private->VmwareSvgaBasePort
-));
- Status = EFI_DEVICE_ERROR;
- goto RestoreAttributes;
-}
-
-Private->FrameBufferVramBarIndex = PCI_BAR_IDX1;
- }
-
//
// Get ParentDevicePath
//
@@ -446,9 +385,6 @@ QemuVideoControllerDriverStart (
case QEMU_VIDEO_BOCHS:
Status = QemuVideoBochsModeSetup (Private, IsQxl);
break;
- case QEMU_VIDEO_VMWARE_SVGA:
-Status = QemuVideoVmwareSvgaModeSetup (Private);
-break;
default:
ASSERT (FALSE);
Status = EFI_DEVICE_ERROR;
@@ -510,9 +446,6 @@ DestructQemuVideoGraphics:
FreeModeData:
FreePool (Private->ModeData);
- if (Private->VmwareSvgaModeInfo != NULL) {
-FreePool (Private->VmwareSvgaModeInfo);
- }
UninstallGopDevicePath:
gBS->UninstallProtocolInterface (Private->Handle,
@@ -634,9 +567,6 @@ QemuVideoControllerDriverStop (
);
FreePool (Private->ModeData);
- if (Private->VmwareSvgaModeInfo != NULL) {
-FreePool (Private->VmwareSvgaModeInfo);
- }
gBS->UninstallProtocolInterface (Private->Handle,
, Private->GopDevicePath);
FreePool (Private->GopDevicePath);
@@ -834,7 +764,7 @@ ClearScreen (
Private->PciIo->Mem.Write (
Private->PciIo,
EfiPciIoWidthFillUint32,
-Private->FrameBufferVramBarIndex,
+0,
0,
0x40 >> 2,
@@ -971,38 +901,6 @@ BochsRead (
return Data;
}
-VOID
-VmwareSvgaWrite (
- QEMU_VIDEO_PRIVATE_DATA *Private,
- UINT16Register,
- UINT32Value
- )
-{
- UnalignedIoWrite32 (
-Private->VmwareSvgaBasePort + VMWARE_SVGA_INDEX_PORT,
-Register
-);
- UnalignedIoWrite32 (
-Private->VmwareSvgaBasePort + VMWARE_SVGA_VALUE_PORT,
-Value
-);
-}
-
-UINT32
-VmwareSvgaRead (
- QEMU_VIDEO_PRIVATE_DATA *Private,
- UINT16Register
- )
-{
-
[edk2] [PATCH 3/4] Revert "OvmfPkg/QemuVideoDxe: list "UnalignedIoInternal.h" in the INF file"
From: yuchenlin This reverts commit b2959e9f1a57279506ca46d56bc424fd7fa6b62a. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: yuchenlin --- OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf | 1 - 1 file changed, 1 deletion(-) diff --git a/OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf b/OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf index c071fafa4e..e5575622dc 100644 --- a/OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf +++ b/OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf @@ -39,7 +39,6 @@ Gop.c Initialize.c Qemu.h - UnalignedIoInternal.h [Sources.Ia32, Sources.X64] VbeShim.c -- 2.18.0 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
