Initialize local variable to suppress warning C4703:
potentially uninitialized local pointer variable.
Both reads (dereferences) of "PciRootBridgeIo" in
PciBusDriverBindingStart() are only reached if
"gFullEnumeration" is TRUE on entry *and* we successfully
open the EfiPciRootBridgeIoProtocol interface.
Cc: Star Zeng <star.zeng@intel.com>
Cc: Ruiyu Ni <ruiyu.ni@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>
Reviewed-by: Star Zeng <star.zeng@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
(cherry picked from commit a012bf6e3e)
Within function NvmExpressPassThru():
The data buffer for the below 2 Admin command:
Create I/O Completion Queue command (Opcode 01h)
Create I/O Submission Queue command (Opcode 05h)
are not mapped to the PCI controller specific addresses.
But the current code logic also prevents the below NVM command:
Write (Opcode 01h)
from mapping its data buffer.
Hence, this commit refine the logic to resolve this issue.
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Feng Tian <feng.tian@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
(cherry picked from commit 748cd9a680)
Per PI spec, the PciEnumerationComplete protocol installation
should be after PciIo installation.
Today's implementation installs the PciEnumerationComplete
after hardware enumeration is completed, but before PciIo
installation.
The change corrects the spec/implementation gap.
The change also benefits certain implementation that depends on
the PciIo handle in PciEnumerationComplete callback.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
(cherry picked from commit 42e8bc7d16)
The patch is just a code cleanup with no functionality impact.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
(cherry picked from commit 2632981783)
It didn't cause big issues when VT-d was disabled.
But in VT-d enabled platform, lack of EhcFreeUrb call caused
the DMA data was not moved back to user's buffer.
It caused the correct data cannot be got through sync interrupt
transfer.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
(cherry picked from commit c74805f1e7)
Update the UhciPei driver to consume IOMMU_PPI to allocate DMA buffer.
If no IOMMU_PPI exists, this driver still calls PEI service to allocate
DMA buffer, with assumption that DRAM==DMA.
Cc: Jiewen Yao <jiewen.yao@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>
(cherry picked from commit 8284b1791e)
This patch is following 2c656af04d.
1. Fix typo "XHC" to "EHC".
2. Reinitialize Request(Phy/Map) and Data(Phy/Map)
in Urb, otherwise the last time value of them may
be used in error handling when error happens.
Cc: Jiewen Yao <jiewen.yao@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>
(cherry picked from commit c34a5aab53)
1. Call IoMmuInit() after locating gPeiUsbControllerPpiGuid.
2. Call XhcPeiFreeSched() to do cleanup in XhcEndOfPei.
Cc: Jiewen Yao <jiewen.yao@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>
(cherry picked from commit 0aa1794118)
V2: Halt HC at EndOfPei.
Update the EhciPei driver to consume IOMMU_PPI to allocate DMA buffer.
If no IOMMU_PPI exists, this driver still calls PEI service to allocate
DMA buffer, with assumption that DRAM==DMA.
Cc: Jiewen Yao <jiewen.yao@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>
(cherry picked from commit 2c656af04d)
Update XHCI driver to consume IOMMU_PPI to allocate DMA buffer.
If no IOMMU_PPI exists, this driver still calls PEI service
to allocate DMA buffer, with assumption that DRAM==DMA.
This is a compatible change.
Cc: Star Zeng <star.zeng@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
(cherry picked from commit b575ca32c8)
We found there are loops of *2* Maps and only *1* Unmap and
the DMA buffer address is decreasing.
It is caused by the below code flow.
XhcAsyncInterruptTransfer ->
XhcCreateUrb ->
XhcCreateTransferTrb ->
Map Urb->DataMap (1)
Timer: loops of *2* Maps and only *1* Unmap
XhcMonitorAsyncRequests ->
XhcFlushAsyncIntMap ->
Unmap and Map Urb->DataMap (2)
XhcUpdateAsyncRequest ->
XhcCreateTransferTrb ->
Map Urb->DataMap (3)
This patch is to eliminate (3).
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Jiewen Yao <jiewen.yao@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>
(cherry picked from commit 0b9c0c6540)
Some USB devices don't report data periodically through Int
Transfer. They report data only when be asked. If the TRB
is not removed from the XHCI HW, when next time HOST asks
data again, the data is reported but consumed by the previous
TRB, which results the HOST thinks data never comes.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao A Wu <hao.a.wu@intel.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
(cherry picked from commit b33b1055b0)
In function UfsHcDriverBindingStart(), the return value 'Status' may be
overridden during the original PCI attributes restore process.
This commit refines the logic to avoid such override.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
(cherry picked from commit 1a5ae66175)
The commit removes the unused field 'EFI_HANDLE Handle' in Ufs host
controller private data structure 'UFS_HOST_CONTROLLER_PRIVATE_DATA'.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
(cherry picked from commit c36ea72ebd)
In case of an async command if updating the submission queue tail
doorbell fails then the command will not be picked up by device and
no completion response will be created. This scenario has to be handled.
Also if we create an AsyncRequest element and insert in the async queue,
it will never receive a completion so in the timer routine this element
won't be freed, resulting in memory leak. Also in case of blocking calls
we should capture the status of updating completion queue head doorbell
register and return it to caller of PassThru.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Suman Prakash <suman.p@samsung.com>
Reviewed-by: Hao Wu <hao.a.wu@intel.com>
(cherry picked from commit f6b139bde7)
In function ReadCapacity(), the following expression:
MediaInfo->LastBlock = (Data.LastLba3 << 24) |
(Data.LastLba2 << 16) |
(Data.LastLba1 << 8) |
Data.LastLba0;
(There is also a similar case in this function.)
will involve undefined behavior in signed left shift operations.
Since Data.LastLbaX is of type UINT8, and MediaInfo->LastBlock is of type
UINTN. Therefore, Data.LastLbaX will be promoted to int (32 bits, signed)
first, and then perform the left shift operation.
According to the C11 spec, Section 6.5.7:
4 The result of E1 << E2 is E1 left-shifted E2 bit positions; vacated
bits are filled with zeros. If E1 has an unsigned type, the value
of the result is E1 * 2^E2 , reduced modulo one more than the
maximum value representable in the result type. If E1 has a signed
type and nonnegative value, and E1 * 2^E2 is representable in the
result type, then that is the resulting value; otherwise, the
behavior is undefined.
So if bit 7 of Data.LastLba3 is 1, (Data.LastLba3 << 24) will be out of
the range within int type. The undefined behavior of the signed left shift
will lead to a potential of setting the high 32 bits of
MediaInfo->LastBlock to 1 during the cast from type int to type UINT64
for X64 builds.
This commit will add an explicit UINT32 type cast for Data.LastLba3 to
resolve this issue.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
Reviewed-by: Feng Tian <feng.tian@intel.com>
(cherry picked from commit 3778a4dfcd)
For async commands, the buffer allocated for Prp list is
not getting freed, which will cause memory leak for async
read write command. For example testing async command flow
with custom application to send multiple read write commands
were resulting in decrease of available memory page in memmap,
which eventually resulted in system hang. Hence freeing
AsyncRequest->MapData, AsyncRequest->MapMeta, AsyncRequest->MapPrpList and
AsyncRequest->PrpListHost when async command is completed.
Cc: Feng Tian <feng.tian@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Suman Prakash <suman.p@samsung.com.com>
Reviewed-by: Hao Wu <hao.a.wu@intel.com>
At worst case, OCR register may always not set BIT31. It will cause
original code enter to dead loop. Adding a break for such case.
Cc: Hao Wu <hao.a.wu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Feng Tian <feng.tian@intel.com>
Reviewed-by: Hao Wu <hao.a.wu@intel.com>
There are cases that the operands of an expression are all with rank less
than UINT64/INT64 and the result of the expression is explicitly cast to
UINT64/INT64 to fit the target size.
An example will be:
UINT32 a,b;
// a and b can be any unsigned int type with rank less than UINT64, like
// UINT8, UINT16, etc.
UINT64 c;
c = (UINT64) (a + b);
Some static code checkers may warn that the expression result might
overflow within the rank of "int" (integer promotions) and the result is
then cast to a bigger size.
The commit refines codes by the following rules:
1). When the expression is possible to overflow the range of unsigned int/
int:
c = (UINT64)a + b;
2). When the expression will not overflow within the rank of "int", remove
the explicit type casts:
c = a + b;
3). When the expression will be cast to pointer of possible greater size:
UINT32 a,b;
VOID *c;
c = (VOID *)(UINTN)(a + b); --> c = (VOID *)((UINTN)a + b);
4). When one side of a comparison expression contains only operands with
rank less than UINT32:
UINT8 a;
UINT16 b;
UINTN c;
if ((UINTN)(a + b) > c) {...} --> if (((UINT32)a + b) > c) {...}
For rule 4), if we remove the 'UINTN' type cast like:
if (a + b > c) {...}
The VS compiler will complain with warning C4018 (signed/unsigned
mismatch, level 3 warning) due to promoting 'a + b' to type 'int'.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
Reviewed-by: Feng Tian <feng.tian@intel.com>
For pointer subtraction, the result is of type "ptrdiff_t". According to
the C11 standard (Committee Draft - April 12, 2011):
"When two pointers are subtracted, both shall point to elements of the
same array object, or one past the last element of the array object; the
result is the difference of the subscripts of the two array elements. The
size of the result is implementation-defined, and its type (a signed
integer type) is ptrdiff_t defined in the <stddef.h> header. If the result
is not representable in an object of that type, the behavior is
undefined."
In our codes, there are cases that the pointer subtraction is not
performed by pointers to elements of the same array object. This might
lead to potential issues, since the behavior is undefined according to C11
standard.
Also, since the size of type "ptrdiff_t" is implementation-defined. Some
static code checkers may warn that the pointer subtraction might underflow
first and then being cast to a bigger size. For example:
UINT8 *Ptr1, *Ptr2;
UINTN PtrDiff;
...
PtrDiff = (UINTN) (Ptr1 - Ptr2);
The commit will refine the pointer subtraction expressions by casting each
pointer to UINTN first and then perform the subtraction:
PtrDiff = (UINTN) Ptr1 - (UINTN) Ptr2;
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
Acked-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Feng Tian <feng.tian@intel.com>
Compiler calculates the PciBar[BarIndex] using
sizeof (PciBar[0]) * BarIndex, when BarIndex is type of UINT64,
the above calculation generates assembly code using _allmul.
Change BarIndex to UINTN to avoid the build failure.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Reviewed-by: Wu Jiaxin <jiaxin.wu@intel.com>
When the VendorId/DeviceId/RevisionId/SubsystemVendorId
/SubsystemDeviceId is MAX_UINTN, IncompatiblePciDeviceSupport
driver doesn't use it to match any IDs.
The patch fixes this bug.
Since PciBus driver always calls IncompatiblePciDeviceSupport
using IDs read from HW, MAX_UINTN is never passed to this
driver.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Jeff Fan <jeff.fan@intel.com>
Reviewed-by: Feng Tian <feng.tian@intel.com>
The patch replaces the following macros:
DEVICE_ID_NOCARE (0xFF) --> MAX_UINT64
PCI_ACPI_UNUSED (0) --> 0
PCI_BAR_ALL (0xFF) --> MAX_UINT64
PCI_BAR_NOCHANGE (0) --> 0
PCI_BAR_EVEN_ALIGN --> EVEN_ALIGN (local definition)
Since the PciBus driver was updated to accept Spec defined values
in previous commit, the above replacements don't impact
functionality.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Jeff Fan <jeff.fan@intel.com>
Reviewed-by: Feng Tian <feng.tian@intel.com>
PI spec IncompatiblePciSupport part defines (UINT64)-1 as all BARs
and 0 to use existing alignment. PciBus driver didn't accept these
values. It treated 0xFF as all BARs and 0xFFFFFFFFFFFFFFFFULL to use
existing alignment.
The patch changes the code to still accept old values while also
accept values defined in PI spec. So that the driver can provide
backward compatibility and follow spec.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Jeff Fan <jeff.fan@intel.com>
Reviewed-by: Feng Tian <feng.tian@intel.com>
When BarIndex equals to 0xFF, default value 0 is used as the BAR
index. Though PCI_BAR_ALL and MAX_UINT8 shares the same value,
using PCI_BAR_ALL is like to match any BAR not BAR 0, it's more
proper to use MAX_UINT8 here.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Reviewed-by: Feng Tian <feng.tian@intel.com>
If the code eventually returns "Status" anyway, it does not make
sense to explicitly return "Status" in case of an error, too.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
Refine the codes to compare the definition 'SIZE_4GB' with type
EFI_PHYSICAL_ADDRESS.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>
Add support for non-coherent DMA, either by performing explicit cache
maintenance when DMA mappings are aligned to the CPU's DMA buffer alignment,
or by bounce buffering via uncached mappings otherwise.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Tested-by: Marcin Wojtas <mw@semihalf.com>
Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>
Fix switch/case statement type mismatch in functions PciIoMemRead &
PciIoMemWrite.
Parameter 'Width' is of enum type EFI_PCI_IO_PROTOCOL_WIDTH, but the enum
type provided in 'switch (Width)' block is of type
EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_WIDTH.
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>
Add missing EFIAPI modifiers to the functions that are exposed via the
PCI I/O protocol.
At the same time, add a missing UINT8 cast which breaks the build on
Visual Studio.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Dandan Bi <dandan.bi@intel.com>
This implements support for non-discoverable PCI compatible devices, i.e,
devices that are not on a PCI bus but that can be controlled by generic PCI
drivers in EDK2.
This is implemented as a UEFI driver, which means we take full advantage
of the UEFI driver model, and only instantiate those devices that are
necessary for booting.
Care is taken to deal with DMA addressing limitations: DMA mappings and
allocations are moved below 4 GB if the PCI driver has not informed us
that the device being driven is 64-bit DMA capable. DMA is implemented as
coherent, support for non-coherent DMA is implemented by a subsequent patch.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>
Tested-by: Marcin Wojtas <mw@semihalf.com>
Port status bits are clear in original code, so no enumeration
takes place.
Changing this to prevent the status bits from being cleared
allows enumeration to proceed normally.
Cc: Star Zeng <star.zeng@intel.com>
Cc: Mike Turner <Michael.Turner@microsoft.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Mike Turner <Michael.Turner@microsoft.com>
Signed-off-by: Feng Tian <feng.tian@intel.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
The commit e27cca has a typo on DEBUG level macro. And this debug
message should be DEBUG_INFO rather than DEBUG_ERROR.
Cc: Jan Dabros <jsd@semihalf.com>
Cc: Marcin Wojtas <mw@semihalf.com>
Cc: Star Zeng <star.zeng@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Feng Tian <feng.tian@intel.com>
reviewed-by: Marcin Wojtas <mw@semihalf.com>
EFI_D_INFO, EFI_D_VERBOSE, EFI_D_WARN and EFI_D_ERROR are replaced
with currently recommended values.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jan Dabros <jsd@semihalf.com>
Signed-off-by: Marcin Wojtas <mw@semihalf.com>
Reviewed-by: Feng Tian <feng.tian@intel.com>
We send ADDRESS DEVICE CMD in XhcInitializeDeviceSlot(), which will
cause XHC issue a USB SET_ADDRESS request to the USB Device.
According to USB spec, there should have a 10ms delay before this
operation after resetting a given port.
But in original code, there is a possible path which may have no such
10ms delay:
UsbHubResetPort()->UsbHubSetPortFeature()->Stall(20)->UsbHubGetPortSt
atus()->XhcPollPortStatusChange()->(if RESET_C bit is set)->
XhcInitializeDeviceSlot()->(if RESET_C bit is set)->Stall(10)
So this patch is used to fix above issue.
Cc: Star Zeng <star.zeng@intel.com>
Cc: Baraneedharan Anbazhagan <anbazhagan@hp.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Feng Tian <feng.tian@intel.com>
Tested-by: Baraneedharan Anbazhagan <anbazhagan@hp.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
When PciSioSerial is firstly started with a non-NULL remaining
device path, the UART instance is created using the parameters
specified in the remaining device path. Later when the driver
is started again on the same UART controller with NULL remaining
device path, the correct logic is to directly return SUCCESS
instead of current buggy implementation which wrongly produces
another UART using the default parameters.
The bug causes two UARTs are created when the UART is configured
in 57600 baud rate.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
