Commit Graph

754 Commits

Author SHA1 Message Date
Hao Wu b92efc9fe5 MdeModulePkg/EmmcDxe: Make sure no extra data is erased by EraseBlocks
V3 changes:
Add debug messages for new return path when successfully erase the
specified blocks. Refine logic for calculating the size for writing
zeros to device.

V2 changes:

The Trim command is not supported on all eMMC devices. For those devices
that do not support such command, add codes to handle the scenario.

Commit message:

The current implementation of the Erase Block Protocol service
EraseBlocks() uses the erase command. According to spec eMMC Electrical
Standard 5.1, Section 6.6.9:

The erasable unit of the eMMC is the "Erase Group"; Erase group is
measured in write blocks that are the basic writable units of the Device.
...
When the Erase is executed it will apply to all write blocks within an
erase group.

However, code logic in function EmmcEraseBlocks() does not check whether
the blocks to be erased form complete erase groups. Missing such checks
will lead to erasing extra data on the device.

This commit will:
a. If the device support the Trim command, use the Trim command to
perform the erase operations for eMMC devices.

According to the spec:
Unlike the Erase command, the Trim function applies the erase operation to
write blocks instead of erase groups.

b. If the device does not support the Trim command, use the Erase command
to erase the data in the erase groups. And write zeros to those blocks
that cannot form a complete erase group.

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>
2017-08-14 08:45:10 +08:00
Ruiyu Ni 7111e46fde MdeModulePkg/NvmExpressDxe: Notify NVME HW when system reset happens
Per NVM Express Spec, software should notify NVME HW when shutdown
occurs.

The NVME controller requires notification for shutdown as part of
its management of internal structures. Even with FUA, failing to
notify the NVME controller to shutdown power off causes the NVME
controller to take quite some time to organize its tables on the
next power on. This time exceeds the normal timeout, so we would
fail to boot the NVME disk.

The host should set the Shutdown Notification (CC.SHN) field to 01b
to indicate a normal shutdown operation. The controller indicates
when shutdown processing is completed by updating the Shutdown Status
(CSTS.SHST) field to 10b.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Reviewed-by: Hao A Wu <hao.a.wu@intel.com>
2017-08-09 15:59:13 +08:00
Hao Wu c16eee92d4 MdeModulePkg/Ufs: Set 'Data Segment Length' field for Write Descriptor
According to the Universal Flash Storage (UFS) Version 2.1 (JESD220C) spec
Section 10.7.8.5, the DATA SEGMENT LENGTH field of the UPIU shall also be
set to number of descriptor bytes to write.

The origin codes miss the above operation.

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>
2017-08-02 20:56:53 +08:00
Hao Wu 32c9049de1 MdeModulePkg/UfsPassThruDxe: Add impl of UFS Device Config Protocol
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>
2017-08-02 20:56:45 +08:00
Ruiyu Ni fff2623cc2 MdeModulePkg/PciBus: Avoid hang when BUS pad resource is not in top
PciScanBus() assumes the GetResourcePadding() puts BUS descriptor
in the very beginning, if it's not, the Descriptors will be updated
to point to middle of the pool buffer, which can cause
FreePool(Descriptors) hang in DEBUG image.
No functionality impact to RELEASE image.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
2017-07-28 17:52:46 +08:00
Star Zeng 958a81813b MdeModulePkg Xhci: Also RecoverHaltedEndpoint for BABBLE_ERROR
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=644

According to XHCI spec:
4.10.2.1 Stall Error

4.10.2.4 Babble Detected Error
When a device transmits more data on the USB than the host controller
is expecting for a transaction, it is defined to be babbling.
In general, this is called a Babble Error. When a device sends more
data than the TD Transfer Size bytes (TD Babble), unexpected activity
that persists beyond a specified point in a (micro)frame (Frame Babble),
or a packet greater than Max Packet Size (Packet Babble), the host
controller shall set the Babble Detected Error in the Completion Code
field of the TRB, generate an Error Event, and halt the endpoint
(refer to Section 4.10.2.1).

This patch is to also check for EFI_USB_ERR_BABBLE error returned as
a TransferResult and then proceed to XhcRecoverhaltedEndPoint.

Cc: Vladimir Olovyannikov <vladimir.olovyannikov@broadcom.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Star Zeng <star.zeng@intel.com>
Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>
2017-07-26 16:06:19 +08:00
Bi, Dandan 9750503a11 MdeModulePkg/XhciDxe: Make comments align with function
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Hao Wu <hao.a.wu@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Dandan Bi <dandan.bi@intel.com>
Reviewed-by: Hao Wu <hao.a.wu@intel.com>
2017-07-11 08:51:13 +08:00
Hao Wu 19b2cb5c11 MdeModulePkg/EmmcDxe: Implementation of Disk Information Protocol
Adds the implementation of Disk Information Protocol for EMMC devices per
PI 1.6 spec.

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>
2017-07-06 12:55:43 +08:00
Hao Wu af6a6bf41b MdeModulePkg/SdDxe: Implementation of Disk Information Protocol
Adds the implementation of Disk Information Protocol for SD devices per
PI 1.6 spec.

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>
2017-07-06 12:55:38 +08:00
Hao Wu 5f5bba14b6 MdeModulePkg/NvmExpressDxe: Handle timeout for blocking PassThru req
https://bugzilla.tianocore.org/show_bug.cgi?id=433

When a blocking NVMe PassThru request experiences timeout, the current
codes in function NvmExpressPassThru() do not abort the timeout request
while advancing synchronous Submission Queue tail. Therefore, it is
possible to submit a new blocking PassThru request when the synchronous
Submission Queue is full.

The commit adds logic to abort the timeout request by resetting the NVMe
controller when a timeout occurs for a blocking PassThru request.

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>
2017-07-06 12:53:36 +08:00
Ruiyu Ni 49be9c3c20 MdeModulePkg/XhciDxe: Check timeout URB again after stopping endpoint
This fixes BULK data loss when transfer is detected as timeout but
finished just before stopping endpoint.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Reviewed-by: Hao Wu <hao.a.wu@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Feng Tian <feng.tian@intel.com>
2017-07-05 11:31:06 +08:00
Ruiyu Ni 41fb8ce939 MdeModulePkg/XhciDxe: Separate common logic to XhcTransfer
The patch separates the common logic in XhcControlTransfer,
XhcBulkTransfer and XhcSyncIntTransfer to a sub-routine
XhcTransfer. It doesn't have functionality impact.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Reviewed-by: Hao Wu <hao.a.wu@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Feng Tian <feng.tian@intel.com>
2017-07-05 11:31:06 +08:00
Ruiyu Ni 396ae94d46 MdeModulePkg/XhciDxe: Dump the CMD/EVENT/INT/BULK ring information
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Reviewed-by: Hao Wu <hao.a.wu@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Feng Tian <feng.tian@intel.com>
2017-07-05 11:31:05 +08:00
Ruiyu Ni 5a4b3388aa MdeModulePkg/XhciDxe: Refine IsTransferRingTrb and IsAsyncIntTrb
Current implementation of IsTransferRingTrb only checks whether
the TRB is in the RING of the URB.
The patch enhanced the logic to check that whether the TRB belongs
to the transaction of URB.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Reviewed-by: Hao Wu <hao.a.wu@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Feng Tian <feng.tian@intel.com>
2017-07-05 11:31:05 +08:00
Ard Biesheuvel 1fb805b1eb MdeModulePkg/AtaAtapiPassThru: relax PHY detect timeout
The SATA spec mandates that link detection by the PHY completes within
10 ms after receiving a reset signal. However, there is no obligation
to uphold this requirement at the driver end as strictly as we do, and
as it turns out, some combinations of host and device (e.g., Samsung
850 EVO connected to a LeMaker Cello) are only borderline compliant,
which means the device is not detected reliably.

So let's allow for a bit of margin, and increase the PHY detect timeout
value to 15 ms.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Star Zeng <star.zeng@intel.com>
2017-06-28 12:47:13 +00:00
Dandan Bi 6fbaed1f00 MdeModulePkg: Clean ACPI 2.0 characters in UEFI spec
v2: Add some missing changes.

Cc: Liming Gao <liming.gao@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Dandan Bi <dandan.bi@intel.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
2017-06-20 09:29:43 +08:00
Dandan Bi 3652f9902f MdeModulePkg: Update comments in SimpleTextInEx according to UEFI 2.7
v2: Add some missing changes Ps2KeyboardDxe.

Cc: Star Zeng <star.zeng@intel.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Dandan Bi <dandan.bi@intel.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>
2017-06-20 09:29:42 +08:00
Hao Wu 8df95dd04f MdeModulePkg/PciHostBridgeDxe: Make bitwise operands of the same size
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
2017-06-09 09:11:17 +08:00
Ruiyu Ni b0b626ea2f MdeModulePkg/Xhci: Correct the indention of comments
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
2017-06-02 15:55:23 +08:00
Ruiyu Ni b33b1055b0 MdeModulePkg/Xhci: Remove TRB when canceling Async Int Transfer
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>
2017-06-02 14:23:29 +08:00
Ruiyu Ni 76d1c752cb MdeModulePkg/UsbBus: Correct debug message
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
2017-06-02 12:59:28 +08:00
Ruiyu Ni b659b503fa MdeModulePkg/UsbBus: Fix system hang when failed to uninstall UsbIo
When "reconnect -r" is typed in shell, UsbFreeInterface() is called
to uninstall the UsbIo and DevicePath. But When a UsbIo is opened
by a driver and that driver rejects to close the UsbIo in Stop(),
the uninstall doesn't succeed.
But UsbFreeInterface () frees the DevicePath memory without check
whether the uninstall succeeds.
It leads to the DXE core database contain a DevicePath instance but
that instance's memory is freed.
Assertion happens when someone calls InstallProtocol(DevicePath)
because the InstallProtocol() checks all DevicePath instance to
find whether the same one exits in database.

We haven't seen any USB device driver which rejects to close UsbIo
in Stop(), but it's very likely.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Reviewed-by: Feng Tian <feng.tian@intel.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
2017-06-02 12:59:27 +08:00
Hao Wu 3719c2aa87 MdeModulePkg/Xhci: Fill the 'interval' field for ISO endpoint context
https://bugzilla.tianocore.org/show_bug.cgi?id=510

The commit fills the 'Interval' field of the Endpoint Context data for
isochronous endpoints. It will resolve the error when a Configure
Endpoint Command is sent to an isochronous endpoint.

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>
2017-06-01 08:54:03 +08:00
Dandan Bi 7a85e84741 MdeModulePkg/PciHostBridgeDxe: Fix EBC build failure
Cc: Jiewen Yao <jiewen.yao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Dandan Bi <dandan.bi@intel.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
2017-05-18 16:44:26 +08:00
Jiewen Yao 11a6cc5bda MdeModulePkg/PciBus: Add IOMMU support.
If IOMMU protocol is installed, PciBus need call IOMMU
to set access attribute for the PCI device in Map/Ummap.

Only after the access attribute is set, the PCI device can
access the DMA memory.

Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Leo Duran <leo.duran@amd.com>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Previous patch Tested-by: Brijesh Singh <brijesh.singh@amd.com>
Previous patch Tested-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>
Reviewed-by: Leo Duran <leo.duran@amd.com>
2017-05-17 16:05:17 +08:00
Jiewen Yao c15da8eb35 MdeModulePkg/PciHostBridge: Add IOMMU support.
If IOMMU protocol is installed, PciHostBridge just calls
IOMMU AllocateBuffer/FreeBuffer/Map/Unmap.

PciHostBridge does not set IOMMU access attribute,
because it does not know which device request the DMA.
This work is done by PciBus driver.

Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Leo Duran <leo.duran@amd.com>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Previous patch Tested-by: Brijesh Singh <brijesh.singh@amd.com>
Previous patch Tested-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>
Reviewed-by: Leo Duran <leo.duran@amd.com>
2017-05-17 16:05:15 +08:00
Hao Wu a8321feebb MdeModulePkg/UfsPassThruDxe: Fix typo in UfsPassThruGetTargetLun()
For function UfsPassThruGetTargetLun(), the length of the input device
node specified by 'DevicePath' should be compared with the size of
'UFS_DEVICE_PATH' rather than the size of 'SCSI_DEVICE_PATH'.

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>
2017-05-17 15:26:24 +08:00
Hao Wu 1a5ae66175 MdeModulePkg/UfsPciHc: Avoid overriding return value in BindingStart
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>
2017-04-25 09:08:33 +08:00
Hao Wu c36ea72ebd MdeModulePkg/UfsPciHc: Remove unused field in UfsHc private struct
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>
2017-04-25 09:08:28 +08:00
Suman Prakash f6b139bde7 MdeModulePkg/NvmExpressDxe: Handling return of write to sq and cq db
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>
2017-04-24 09:06:44 +08:00
Hao Wu 95ad8f7f6a MdeModulePkg/Ufs: Wait fDeviceInit be cleared by devices during init
In the origin codes, the host sets the fDeviceInit flag to initiate device
initialization, but does not check whether the device resets this flag
to indicate the device initialization is completed.

Details can be referred at UFS 2.0 Spec Section 14.2 - Flags.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Mateusz Albecki <mateusz.albecki@intel.com>
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
Reviewed-by: Feng Tian <feng.tian@intel.com>
2017-04-20 13:09:51 +08:00
Hao Wu edd94e74c8 MdeModulePkg/UfsPassThruDxe: Replace 'EFI_D_XXX' with 'DEBUG_XXX'
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>
2017-04-20 13:09:40 +08:00
Hao Wu a2617ed627 MdeModulePkg/UsbBotPei: Fix undefined behavior in signed left shift
In function PeiUsbReadCapacity(), the following expression:
LastBlock = (Data.LastLba3 << 24) |
  (Data.LastLba2 << 16) |
  (Data.LastLba1 << 8) |
  Data.LastLba0;

(There is also a similar case in function PeiUsbReadFormattedCapacity().)

will involve undefined behavior in signed left shift operations.

Since Data.LastLbaX is of type UINT8, they 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
might incur potential issues.

This commit will add an explicit UINT32 type cast for Data.LastLba3 to
refine the codes.

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>
2017-04-13 08:59:07 +08:00
Hao Wu da117dda23 MdeModulePkg/UfsBlkIoPei: Fix undefined behavior in signed left shift
In function UfsBlockIoPeimGetMediaInfo(), the following expression:
Private->Media[DeviceIndex].LastBlock  = (Capacity16.LastLba3 << 24) |
  (Capacity16.LastLba2 << 16) |
  (Capacity16.LastLba1 << 8) |
  Capacity16.LastLba0;

(There is also a similar case in this function.)

will involve undefined behavior in signed left shift operations.

Since Capacity16.LastLbaX is of type UINT8, and
Private->Media[DeviceIndex].LastBlock is of type UINT64. Therefore,
Capacity16.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 Capacity16.LastLba3 is 1, (Capacity16.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
Private->Media[DeviceIndex].LastBlock to 1 during the cast from type int
to type UINT64.

This commit will add an explicit UINT32 type cast for Capacity16.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>
2017-04-13 08:58:56 +08:00
Hao Wu 3778a4dfcd MdeModulePkg/IdeBusPei: Fix undefined behavior in signed left shift
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>
2017-04-13 08:58:51 +08:00
Hao Wu 7c115e775b MdeModulePkg/ScsiDiskDxe: Fix undefined behavior in signed left shift
In function GetMediaInfo(), the following expression:
ScsiDiskDevice->BlkIo.Media->LastBlock =  (Capacity10->LastLba3 << 24) |
                                          (Capacity10->LastLba2 << 16) |
                                          (Capacity10->LastLba1 << 8)  |
                                           Capacity10->LastLba0;
will involve undefined behavior in signed left shift operations.

Since Capacity10->LastLbaX is of type UINT8, and
ScsiDiskDevice->BlkIo.Media->LastBlock is of type UINT64. Therefore,
Capacity10->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 Capacity10->LastLba3 is 1, (Capacity10->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 ScsiDiskDevice->BlkIo.Media->LastBlock to 1 during the cast from type
int to type UINT64.

This commit will add an explicit UINT32 type cast for
Capacity10->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>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
2017-04-13 08:58:45 +08:00
Suman Prakash f2333c707d MdeModulePkg/NvmExpressDxe: Memory leak fix in async code flow
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>
2017-03-21 10:01:44 +08:00
Feng Tian ec86d28558 MdeModulePkg/SdMmc: Add break to avoid dead loop when polling OCR Reg
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>
2017-03-13 21:41:46 +08:00
Hao Wu 16f6922709 MdeModulePkg: Refine casting expression result to bigger size
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>
2017-03-06 14:33:20 +08:00
Hao Wu 809e2bbf41 MdeModulePkg: Refine type cast for pointer subtraction
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>
2017-03-06 14:14:09 +08:00
Ruiyu Ni 063bcff758 MdeModulePkg/PciBusDxe: Refine code to make it more readable
The patch doesn't impact functionality.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Reviewed-by: Hao A Wu <hao.a.wu@intel.com>
2017-02-21 11:16:01 +08:00
Hao Wu 5422804653 MdeModulePkg/SdMmc: Avoid assigning 0 to the BlockSize of Trb
Refine code to avoid potential divide by zero when calculating block
number.

Cc: Feng Tian <feng.tian@intel.com>
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>
2017-02-21 09:26:08 +08:00
Ruiyu Ni eb470e05a3 MdeModulePkg/PciBusDxe: Fix IA32 build failure
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>
2017-02-17 16:59:50 +08:00
Ruiyu Ni 29dbdcabf9 MdeModulePkg/IncompatiblePci: Use MAX_UINTN to match any IDs
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>
2017-02-10 16:52:00 +08:00
Ruiyu Ni c1a247b8a7 MdeModulePkg/IncompatiblePciDevice: Do not use deprecated macros
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>
2017-02-10 16:52:00 +08:00
Ruiyu Ni 8a93fa1efb MdeModulePkg/PciBus: Accept Spec values as BarIndex and Alignment
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>
2017-02-10 16:52:00 +08:00
Ruiyu Ni 9b64832379 MdeModulePkg/PciSioSerialDxe: Use MAX_UINT8 instead of PCI_BAR_ALL
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>
2017-02-10 16:52:00 +08:00
Thomas Huth cd5b2ae437 MdeModulePkg: Remove superfluous return statements
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>
2017-01-20 15:29:18 +08:00
Haojian Zhuang 4d580428a3 MdeModulePkg/UfsBlockIoPei: fix initialize OCS value to 0x0F
The OCS value should be initiliazed as 0x0F according to UFS spec.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
Reviewed-by: Feng Tian <feng.tian@intel.com>
2017-01-20 09:11:26 +08:00
Haojian Zhuang 86bd34ca3c MdeModulePkg/UfsPassThruDxe: fix initialize OCS value to 0x0F
The OCS value should be initiliazed as 0x0F according to UFS spec.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
Reviewed-by: Feng Tian <feng.tian@intel.com>
2017-01-20 09:11:25 +08:00