original we get the VPD PCD items from the VPDGuid.map file in the FV
output folder. but this logic doesn't work when 1) there only have
single non Default SKU, 2) there have multiple SKU with same value.
Now we change it to get the really VPD Pcd items that already display
in the PCD section of the report.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Yonghong Zhu <yonghong.zhu@intel.com>
Reviewed-by: Liming Gao <liming.gao@intel.com>
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 {
<PcdsFixedAtBuild>
TokenSpaceGuid.PcdTest|"b"
}
build command with --pcd TokenSpaceGuid.PcdTest="AAAABB"
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 <yonghong.zhu@intel.com>
Reviewed-by: Liming Gao <liming.gao@intel.com>
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1260
For the PassThru() service of NVM Express Pass Through Protocol, the
current implementation (function NvmExpressPassThru()) will only use the
IO Completion/Submission queues created internally by this driver during
the controller initialization process. Any other IO queues created will
not be consumed.
So the value is little to accept external IO Completion/Submission queue
creation request. This commit will refine the behavior of function
NvmExpressPassThru(), it will only accept driver internal IO queue
creation commands and will return "EFI_UNSUPPORTED" for external ones.
Cc: Jiewen Yao <Jiewen.yao@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1259
According to the the NVM Express spec Revision 1.1, for some commands,
command-related information will be stored in the Dword 0 of the
completion queue entry.
One case is for the Get Features Command (Section 5.9.2 of the spec),
Dword 0 of the completion queue entry may contain feature information.
Hence, this commit will always copy the content of completion queue entry
to the PassThru packet regardless of the execution result of the command.
Cc: Liangcheng Tang <liangcheng.tang@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1142
According to the the NVM Express spec Revision 1.1, for some commands
(like Get/Set Feature Command, Figure 89 & 90 of the spec), the Memory
Buffer maybe optional although the command opcode indicates there is a
data transfer between host & controller (Get/Set Feature Command, Figure
38 of the spec).
Hence, this commit refine the checks for the 'TransferLength' and
'TransferBuffer' field of the EFI_NVM_EXPRESS_PASS_THRU_COMMAND_PACKET
structure to address this issue.
Cc: Liangcheng Tang <liangcheng.tang@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>
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 <jiewen.yao@intel.com>
Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng <star.zeng@intel.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
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 <jiewen.yao@intel.com>
Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng <star.zeng@intel.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
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 <bob.c.feng@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Reviewed-by: Liming Gao <liming.gao@intel.com>
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 <bob.c.feng@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Reviewed-by: Liming Gao <liming.gao@intel.com>
PcdValueInit tool is Arch independent, the Arch specific
build options should be removed from PcdValueInit makefile.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Bob C Feng <bob.c.feng@intel.com>
Reviewed-by: Liming Gao <liming.gao@intel.com>
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 <zhiqiangx.zhao@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Cc: Bob Feng <bob.c.feng@intel.com>
Reviewed-by: Bob Feng <bob.c.feng@intel.com>
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 <zhiqiangx.zhao@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Cc: Bob Feng <bob.c.feng@intel.com>
Reviewed-by: Bob Feng <bob.c.feng@intel.com>
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 <zhiqiangx.zhao@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Cc: Bob Feng <bob.c.feng@intel.com>
Reviewed-by: Bob Feng <bob.c.feng@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1243
AcpiExp text device path: AcpiExp(HID,CID,UIDSTR)
And according to UEFI spec, the CID parameter is optional
and has a default value of 0. But current implementation
miss to check following cases for the AcpiExp.
FromText:when text device is AcpiExp(HID,,UIDSTR)/AcpiExp(HID,0,UIDSTR)
ToText: when the CID is 0 in the node structure
This commit is to do the enhancement.
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Dandan Bi <dandan.bi@intel.com>
Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1243
According to UEFI spec,
for the Messaging Device Path with USB Class SubType, some paras
are optional in the text device path.
Take UsbClass(VID,PID,Class,SubClass,Protocol) for example,
The VID is an integer between 0 and 65535 and is optional. The
default value is 0xFFFF.
The PID is an integer between 0 and 65535 and is optional. The
default value is 0xFFFF.
The Class is an integer between 0 and 255 and is optional. The
default value is 0xFF.
The SubClass is an integer between 0 and 255 and is optional. The
default value is 0xFF.
The Protocol is an integer between 0 and 255 and is optional. The
default value is 0xFF.
So if any the optional para is not specified in the text device,
we should set related para in the node structure to default value.
This commit is to do the enhancement for USB Class device path
when optional para is not specified
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Dandan Bi <dandan.bi@intel.com>
Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1243
Sata device path format:Sata(HPN, PMPN, LUN)
According to UEFI Spec, the PMPN is an integer between
0 and 65535 and is optional. If not provided, the default is 0xFFFF.
This commit is to do the enhancement for Sata device path
when optional para is not specified.
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Dandan Bi <dandan.bi@intel.com>
Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>
Replace multiple, consecutive "\" characters prior to other processing
involving "\" characters. This fixes an issue where "\\..\\..",
"//..//..", and similar input paths are not cleaned properly.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jim Dailey <jim_dailey@dell.com>
Reviewed-by: Ruiyu Ni <Ruiyu.ni@Intel.com>
Reviewed-by: Liming Gao <liming.gao@intel.com>
This is a fix for a double cluster allocation when the disk is full.
The double allocation happens because FatGrowEof calls
FatAllocateCluster without immediately marking the each returned
cluster as allocated. The fix is to move the FatSetFatEntry call
inside the loop.
I've also include some improvements to the sanity checks that I added
while tracking this down. They are optional.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Herbie Robinson <Herbie.Robinson@stratus.com>
Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1236
When "dmem" runs without additional arguments, it dumps the memory
content of EFI_SYSTEM_TABLE. But today's implementation dumps 512
bytes. It's not correct because sizeof (EFI_SYSTEM_TABLE) is less
than 512, the 512-read causes page fault exception in a heap-guard
enabled environment.
The patch changes the implementation to only dump
sizeof (EFI_SYSTEM_TABLE) bytes for gST.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Jaben Carsey <jaben.carsey@intel.com>
Reviewed-by: Jim Dailey <jim_dailey@.com>
Commit e59db6a732
* MdeModulePkg/UsbMass: Merge UsbBoot(Read|Write)Blocks(16)
introduces a bug that causes writing to USB key always fails.
When that patch is verified, only reading was verified.
The root cause is when the writing operation is performed,
the data direction EfiUsbDataIn is wrongly used. Instead, it
should be EfiUsbDataOut.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
The tool now can detect the error that the content between double
quotes contains another double quotes or enter key.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hess Chen <hesheng.chen@intel.com>
Reviewed-by: Yonghong Zhu <yonghong.zhu@intel.com>
Change the criteria of the checkpoint of "#ifndef" to remove the requirement of prefix '_'.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hess Chen <hesheng.chen@intel.com>
Reviewed-by: Yonghong Zhu <yonghong.zhu@intel.com>
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1255
For function ReadFile():
If the line
Status = GetAedAdsData (
...
);
is reached multiple times during the 'for' loop, freeing the data pointed
by variable 'Data' may potentially lead to variable 'Ad' referencing the
already-freed data.
After calling function GetAllocationDescriptor(), 'Data' and 'Ad' may
point to the same memory (with some possible offset). Hence, this commit
will move the FreePool() call backwards to ensure the data will no longer
be used.
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
Reviewed-by: Paulo Alcantara <palcantara@suse.de>
Acked-by: Star Zeng <star.zeng@intel.com>
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1254
This commit will add an additional check within function GetPdFromLongAd()
when getting a Partition Descriptor from given a Long Allocation
Descriptor.
According to UDF 2.60 Spec, Section 2.2.13:
> The partition reference numbers used are determined by the order of the
> Partition Maps in the LVD.
(Also the picture comes before the above contents)
And a more detailed explanation of the partition reference numbers is at
https://sites.google.com/site/udfintro/ (seems not a formal documentation
though), Section 5.3.6.
Based on the above findings, the 'PartitionReferenceNumber' field in a
Long Allocation Descriptor is used as an index to access the Partition
Maps data within a Logical Volume Descriptor.
Hence, the new check focuses on the validity of this
'PartitionReferenceNumber' field in a Long Allocation Descriptor. Since
the current implementation of UdfDxe driver supports only one partition on
a Logical Volume, so the value of 'PartitionReferenceNumber' should be 0.
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
Reviewed-by: Paulo Alcantara <palcantara@suse.de>
Acked-by: Star Zeng <star.zeng@intel.com>
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1175
This commit will update the UdfGetInfo() function with the support of
EFI_FILE_SYSTEM_VOLUME_LABEL data information request.
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
Reviewed-by: Paulo Alcantara <palcantara@suse.de>
Acked-by: Star Zeng <star.zeng@intel.com>
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1253
Within function SetFileInfo():
This commit will fix a typo where 'Minute' should be used instead of
'Second'.
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
Reviewed-by: Paulo Alcantara <palcantara@suse.de>
Acked-by: Star Zeng <star.zeng@intel.com>
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1252
Calling the 'SetPosition' service of the EFI_FILE_PROTOCOL with 'Position'
equals to 0xFFFFFFFFFFFFFFFF for a file is to set the current position to
the end of the file. But the current implementation of function
UdfSetPosition() is to set it to the last byte (not EOF).
This commit will resolve this issue.
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
Reviewed-by: Paulo Alcantara <palcantara@suse.de>
Acked-by: Star Zeng <star.zeng@intel.com>
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=828
Within GetVolumeSize():
The boundary check will validate the 'NumberOfPartitions' field of a
Logical Volume Integrity Descriptor matches the data within the relating
Logical Volume Descriptor.
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
Reviewed-by: Paulo Alcantara <palcantara@suse.de>
Acked-by: Star Zeng <star.zeng@intel.com>
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=828
Within ResolveSymlink():
The boundary check will validate the 'LengthofComponentIdentifier' field
of a Path Component matches the data within the relating (Extended) File
Entry.
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
Reviewed-by: Paulo Alcantara <palcantara@suse.de>
Acked-by: Star Zeng <star.zeng@intel.com>
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=828
Within ReadFile():
Add checks to ensure that when getting the raw data or the Allocation
Descriptors' data from a FE/EFE, it will not consume data beyond the
size of a FE/EFE.
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
Reviewed-by: Paulo Alcantara <palcantara@suse.de>
Acked-by: Star Zeng <star.zeng@intel.com>
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=828
The commit refines the boundary checks for file/path name string to
prevent possible buffer overrun.
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
Reviewed-by: Paulo Alcantara <palcantara@suse.de>
Acked-by: Star Zeng <star.zeng@intel.com>
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=828
Within FindAnchorVolumeDescriptorPointer():
Add a check for the underlying device block size to ensure it is greater
than the size of an Anchor Volume Descriptor Pointer.
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
Reviewed-by: Paulo Alcantara <palcantara@suse.de>
Acked-by: Star Zeng <star.zeng@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1267
The value 5 Port Speed field of PORTSC is new defined in
XHCI 1.1 spec November 2017.
This patch updates XhciDxe and XhciPei to handle it, otherwise
the USB 3.1 device may not be recognized with the XHCI controller
following XHCI 1.1 spec November 2017.
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Hao Wu <hao.a.wu@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng <star.zeng@intel.com>
Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>
Current hard code Usb2Hc.XXXRevision may be not accurate.
This patch updates code to assign Usb2Hc.XXXRevision based on
SBRN (Serial Bus Release Number, PCI configuration space offset
0x60) although there is no code consuming them.
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Hao Wu <hao.a.wu@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng <star.zeng@intel.com>
Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>
Because of we newly add the PcdFieldValueFromComm and
PcdFieldValueFromFdf in early parser phase, so in the report we use
the saved value in this two variables to print it.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Yonghong Zhu <yonghong.zhu@intel.com>
Reviewed-by: Liming Gao <liming.gao@intel.com>
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1262
Current implementation of BaseLib APIs:
AsciiStrToUnicodeStr()
AsciiStrToUnicodeStrS()
AsciiStrnToUnicodeStrS()
do not handle EASCII properly.
More specifically, if the value of ASCII character is larger than 0x7F,
then the converted Unicode character will have all '1's in the higher 8
bits.
An example:
0xC9 => 0xFFC9 (current implementations)
and it should be:
0xC9 => 0x00C9
This commit will address this issue.
Cc: Bin.Lain <bin_601@mail2000.com.tw>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
Reviewed-by: Liming Gao <liming.gao@intel.com>
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 <zhiqiangx.zhao@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Cc: Bob Feng <bob.c.feng@intel.com>
Reviewed-by: Bob Feng <bob.c.feng@intel.com>
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 <zhiqiangx.zhao@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Cc: Bob Feng <bob.c.feng@intel.com>
Reviewed-by: Bob Feng <bob.c.feng@intel.com>
Because MSR has scope attribute, driver has no needs to set
MSR for all APs if MSR scope is core or package type. This patch
updates code to base on the MSR scope value to add MSR to the register
table.
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong <eric.dong@intel.com>
Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>
V4 changes:
1. Serial console log for different threads when program register table.
2. Check the AcpiCpuData before use it to avoid potential ASSERT.
V3 changes:
1. Use global variable instead of internal function to return string for register type
and dependence type.
2. Add comments for some complicated logic.
V1 changes:
Because this driver needs to set MSRs saved in normal boot phase, sync
semaphore logic from RegisterCpuFeaturesLib code which used for normal boot phase.
Detail see below change for RegisterCpuFeaturesLib:
UefiCpuPkg/RegisterCpuFeaturesLib: Add logic to support semaphore type.
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong <eric.dong@intel.com>
Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>
Acked-by: Laszlo Ersek <lersek@redhat.com>
Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
V4 changes include:
1. Serial debug message for different threads when program the register table.
V3 changes include:
1. Use global variable instead of internal function to return string for register type
and dependence type.
2. Add comments for some complicated logic.
V2 changes include:
1. Add more description for the code part which need easy to understand.
2. Refine some code base on feedback for V1 changes.
V1 changes include:
In a system which has multiple cores, current set register value task costs huge times.
After investigation, current set MSR task costs most of the times. Current logic uses
SpinLock to let set MSR task as an single thread task for all cores. Because MSR has
scope attribute which may cause GP fault if multiple APs set MSR at the same time,
current logic use an easiest solution (use SpinLock) to avoid this issue, but it will
cost huge times.
In order to fix this performance issue, new solution will set MSRs base on their scope
attribute. After this, the SpinLock will not needed. Without SpinLock, new issue raised
which is caused by MSR dependence. For example, MSR A depends on MSR B which means MSR A
must been set after MSR B has been set. Also MSR B is package scope level and MSR A is
thread scope level. If system has multiple threads, Thread 1 needs to set the thread level
MSRs and thread 2 needs to set thread and package level MSRs. Set MSRs task for thread 1
and thread 2 like below:
Thread 1 Thread 2
MSR B N Y
MSR A Y Y
If driver don't control execute MSR order, for thread 1, it will execute MSR A first, but
at this time, MSR B not been executed yet by thread 2. system may trig exception at this
time.
In order to fix the above issue, driver introduces semaphore logic to control the MSR
execute sequence. For the above case, a semaphore will be add between MSR A and B for
all threads. Semaphore has scope info for it. The possible scope value is core or package.
For each thread, when it meets a semaphore during it set registers, it will 1) release
semaphore (+1) for each threads in this core or package(based on the scope info for this
semaphore) 2) acquire semaphore (-1) for all the threads in this core or package(based
on the scope info for this semaphore). With these two steps, driver can control MSR
sequence. Sample code logic like below:
//
// First increase semaphore count by 1 for processors in this package.
//
for (ProcessorIndex = 0; ProcessorIndex < PackageThreadsCount ; ProcessorIndex ++) {
LibReleaseSemaphore ((UINT32 *) &SemaphorePtr[PackageOffset + ProcessorIndex]);
}
//
// Second, check whether the count has reach the check number.
//
for (ProcessorIndex = 0; ProcessorIndex < ValidApCount; ProcessorIndex ++) {
LibWaitForSemaphore (&SemaphorePtr[ApOffset]);
}
Platform Requirement:
1. This change requires register MSR setting base on MSR scope info. If still register MSR
for all threads, exception may raised.
Known limitation:
1. Current CpuFeatures driver supports DXE instance and PEI instance. But semaphore logic
requires Aps execute in async mode which is not supported by PEI driver. So CpuFeature
PEI instance not works after this change. We plan to support async mode for PEI in phase
2 for this task.
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong <eric.dong@intel.com>
Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>
Yonghong Zhu
Hao Wu
Star Zeng
Ruiyu Ni
BobCF
Zhaozh1x
Dandan Bi
Jim.Dailey@dell.com
Robinson, Herbie
shenglei
Hess Chen
Liming Gao
Eric Dong