Hi Ray, > -----Original Message----- > From: Ni, Ray > Sent: Thursday, December 5, 2019 1:25 PM > To: Dong, Eric <eric.d...@intel.com>; devel@edk2.groups.io > Cc: Laszlo Ersek <ler...@redhat.com> > Subject: RE: [PATCH v4] UefiCpuPkg/PiSmmCpuDxeSmm: Avoid allocate > Token every time > > Some comments. > > + // > > > > + // Not free the buffer, just clear the UsedTokenNum. In order to > > > > + // avoid triggering allocate action when we need to use the token, > > > > + // do not free the buffer. > 1. Can you reword to below? > "Only free the token buffer recorded in the OldTOkenBufList upon exiting > SMI. > Current token buffer stays allocated so next SMI doesn't need to re- > allocate." [[Eric]] will change to this in next version.
> > > + ASSERT_EFI_ERROR (TokenCountPerChunk != 0); > > 2. This is not right. Should use ASSERT(). [[Eric]] Will update in next version. > > > + DEBUG ((DEBUG_INFO, "CpuSmm: SpinLock Size = 0x%x, > > PreAllocateTokenNum = 0x%x\n", SpinLockSize, TokenCountPerChunk)); > > 3. "PreAllocateTokenNum" -> "PcdTokenCountPerChunk". Please directly > use the PCD name in debug log. [[Eric]] will update it in next version. > > > > > + UINT8 *CurrentTokenBuf; > > > > + UINTN UsedTokenNum; // Only record tokens > > used in > > CurrentTokenBuf. > > 4. I see a mismatch between types of UsedTokenNum and the PCD. One is > UINTN, the other is UINT32. > Better to use UINT32 here. [[Eric]] will use UINT32 in next version. > > > > gUefiCpuPkgTokenSpaceGuid.PcdTokenCountPerChunk|64|UINT32|0x3000 > 2002 > > 5. The PCD name is too generic. It's hard to tell when/where the PCD is used > by reading the name only. > Maybe "PcdCpuSmmMpTokenCountPerChunk"? [[Eric]] will use this name in next version. > > > +#string > > STR_gUefiCpuPkgTokenSpaceGuid_PcdTokenCountPerChunk_PROMPT > > #language en-US "Specify the size of pre allocated token count per > chunk.\n" > > 6. "Size of pre allocated token count per chunk" is a bit confusing. > How about "Count of pre allocated SMM MP tokens"? [[Eric]] will update to "Count of pre allocated SMM MP tokens per chunk " > Then maybe the PCD name can be "PcdCpuSmmMpPreAllocateTokenCount"? [[Eric]] I think " PcdCpuSmmMpTokenCountPerChunk " is better and will use it. Thanks, Eric > > Thanks, > Ray -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#51767): https://edk2.groups.io/g/devel/message/51767 Mute This Topic: https://groups.io/mt/66683028/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
