BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3228
This change is to fix the false compiler error on GCC49 release build.
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Signed-off-by: Liming Gao <gaoliming@byosoft.com.cn>
Reviewed-by: Hao A Wu <hao.a.wu@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
This simplify ATA driver debugging all ATA packets will be printed to
debug port on DEBUG_VERBOSE level along with the packet execution
status. Additionally failed packets and the failed packet execution
status will be printed on DEBUG_ERROR level.
Signed-off-by: Mateusz Albecki <mateusz.albecki@intel.com>
Reviewed-by: Hao A Wu <hao.a.wu@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3026
This commit adds code to restart the ATA packets that failed due to the
CRC error or other link condition. For sync transfers the code will try
to get the command working for up to 5 times. For async transfers, the
command will be retried until the timeout value timeout specified by the
requester is reached. For sync case the count of 5 retries has been
chosen arbitrarily and if needed can be increased or decreased.
Signed-off-by: Mateusz Albecki <mateusz.albecki@intel.com>
Reviewed-by: Hao A Wu <hao.a.wu@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3025
This commit adds error recovery flow on SATA port when the error
condition is reported. Commit only implements SATA port reset flow which
is executed when PxTFD indicates BSY or DRQ. Commit does not implement
HBA level reset.
Signed-off-by: Mateusz Albecki <mateusz.albecki@intel.com>
Reviewed-by: Hao A Wu <hao.a.wu@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3024
AHCI driver used to poll D2H register type to determine whether the FIS
has been received. This caused a problem of long timeouts when the link
got a CRC error and the FIS never arrives. To fix this this change
switches AHCI driver to poll the IS register which will signal both the
reception of FIS and the occurrence of error.
Signed-off-by: Mateusz Albecki <mateusz.albecki@intel.com>
Reviewed-by: Hao A Wu <hao.a.wu@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Fix various typos in documentation, comments and debug strings.
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Signed-off-by: Antoine Coeur <coeur@gmx.fr>
Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
Acked-by: Liming Gao <liming.gao@intel.com>
Signed-off-by: Philippe Mathieu-Daude <philmd@redhat.com>
Message-Id: <20200207010831.9046-12-philmd@redhat.com>
[lersek@redhat.com: replace EFI_D_xxx w/ DEBUG_xxx to shut up PatchCheck]
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1862
Current implementation of function AhciModeInitialization() has an
incorrect assumption that the value in the CAP (offset 00h) register will
always be greater than the highest bit set for the value in the PI (offset
0Ch) register.
This will lead to an issue that hard disk devices may not be found in the
non-S3 boot path for some AHCI controller capabilities.
More specifically, variable 'PortInitializeBitMap' will have the value
from 'Private->PortBitMap', which will be 0xFFFFFFFF in non-S3 boot path.
When the CAP register is of value 0x1 and PI register with value 0x4
(meaning port 2 is available), the current logic will only enumerate port
0. And the device attached behind port 2 will not be enumerated.
To address this issue, variable 'PortInitializeBitMap' will now take the
bitwise and result between 'Private->PortBitMap' and the value read from
the PI register.
Please note that there will be no function impact for S3 path, since in
this case, the bits being set in 'Private->PortBitMap' will be a subset
of the bits being set in the PI register. Their bitwise and operation will
still be the value of 'Private->PortBitMap'.
Cc: Maggie Chu <maggie.chu@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Signed-off-by: Hao A Wu <hao.a.wu@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1483
This commit will add the PEI BlockIO (2) PPIs support for AHCI mode ATA
devices.
More specifically, the driver will consume the ATA AHCI host controller
PPI for ATA controllers working under AHCI code within the system. And
then produces the below additional PPIs for each controller:
EFI PEI Recovery Block IO PPI
EFI PEI Recovery Block IO2 PPI
Cc: Eric Dong <eric.dong@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1483
Due to the limited resource on the VTd DMA buffer size in the PEI phase,
the driver will limit the maximum transfer block number for 48-bit
addressing.
According to PCDs:
gIntelSiliconPkgTokenSpaceGuid.PcdVTdPeiDmaBufferSize|0x00400000
gIntelSiliconPkgTokenSpaceGuid.PcdVTdPeiDmaBufferSizeS3|0x00200000
The default buffer size allocated for IOMMU mapping is:
* 4M bytes for non-S3 cases;
* 2M bytes for S3
For ATA devices in 48-bit address mode, the maximum block number is
currently set to 0xFFFF. For a device with block size equal to 512 bytes,
the maximum buffer allowed for mapping within AhciPei driver will be close
to 32M bytes. Thus, this commit will limit the 48-bit mode maximum block
number to 0x800, which means 1M-byte maximum buffer for mapping when the
block size of a device is 512 bytes. By doing so, potential failure on
calls to the IOMMU 'Map' service can be avoided.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1409
This commit will add the AHCI mode ATA device support in the PEI phase.
More specifically, the newly add AhciPei driver will consume the ATA AHCI
host controller PPI for ATA controllers working under AHCI code within the
system. And then produces the below PPIs for each controller:
EDKII PEI ATA PassThru PPI
Storage Security Command PPI
Also, the driver will consume the S3StorageDeviceInitList LockBox in S3
phase. The purpose is to perform an on-demand (partial) ATA device
enumeration/initialization on each controller to benefit the S3 resume
performance.
The implementation of this driver is currently based on the below
specifications:
Serial ATA Revision 2.6
Serial ATA Advanced Host Controller Interface (AHCI) 1.3.1
AT Attachment with Packet Interface - 6 (ATA/ATAPI-6)
Cc: Jian J Wang <jian.j.wang@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Removing rules for Ipf sources file:
* Remove the source file which path with "ipf" and also listed in
[Sources.IPF] section of INF file.
* Remove the source file which listed in [Components.IPF] section
of DSC file and not listed in any other [Components] section.
* Remove the embedded Ipf code for MDE_CPU_IPF.
Removing rules for Inf file:
* Remove IPF from VALID_ARCHITECTURES comments.
* Remove DXE_SAL_DRIVER from LIBRARY_CLASS in [Defines] section.
* Remove the INF which only listed in [Components.IPF] section in DSC.
* Remove statements from [BuildOptions] that provide IPF specific flags.
* Remove any IPF sepcific sections.
Removing rules for Dec file:
* Remove [Includes.IPF] section from Dec.
Removing rules for Dsc file:
* Remove IPF from SUPPORTED_ARCHITECTURES in [Defines] section of DSC.
* Remove any IPF specific sections.
* Remove statements from [BuildOptions] that provide IPF specific flags.
Cc: Star Zeng <star.zeng@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Chen A Chen <chen.a.chen@intel.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
The functions that are never called have been removed.
They are AhciCheckDeviceStatus,AhciPortReset,DRDYReady,
DRDYReady2,WaitForBSYClear2 and AtaSoftReset.
https://bugzilla.tianocore.org/show_bug.cgi?id=1062
v2: DRDYReady, DRDYReady2, WaitForBSYClear2 and AtaSoftReset
are added to the commit message.
Cc: Star Zeng <star.zeng@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: shenglei <shenglei.zhang@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=916
Within function AtaAtapiPassThruStart():
Add missing NULL pointer check for variable 'Instance' under the
'ErrorExit' code logic.
Cc: Steven Shi <steven.shi@intel.com>
Cc: Eric Dong <eric.dong@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>
1. Do not use tab characters
2. No trailing white space in one line
3. All files must end with CRLF
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Liming Gao <liming.gao@intel.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
The patch doesn't have functionality impact. It is just to make
VS2010/VS2012 happy.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Reviewed-by: Dandan Bi <dandan.bi@intel.com>
The patch adds support to certain devices that support PUIS (Power
up in Standby).
For those devices that supports SET_FEATURE spin up, SW needs to
send SET_FEATURE subcommand to spin up the devices.
For those devices that doesn't support SET_FEATURE spin up, SW needs
to send read sectors command to spin up the devices.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Reviewed-by: Chasel Chiu <chasel.chiu@intel.com>
Reviewed-by: Hao A Wu <hao.a.wu@intel.com>
This patch caused Windows 10 S4 resume failure.
Considering the similar changes are reverted from PciBus driver,
revert the patch from AtaAtapiPassThru as well.
Revert "MdeModulePkg/AtaAtapiPassThru: disable the device
at ExitBootServices()"
This reverts commit 6fb8ddd36b.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
This patch caused Windows 10 S4 resume failure.
Considering the similar changes are reverted from PciBus driver,
revert the patch from AtaAtapiPassThru as well.
Revert "MdeModulePkg/AtaAtapiPassThru: disable only BM-DMA at
ExitBootServices()"
This reverts commit 76fd5a660d.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
Clearing I/O port decoding in the PCI command register at
ExitBootServices() breaks IDE boot in Windows, on QEMU's "pc" (i440fx)
machine type. (AHCI boot on "q35" is unaffected.) Windows seems repeatedly
stuck, apparently waiting for a timeout of sorts.
This is arguably a Windows bug; a native OS driver should not expect the
firmware to leave the PCI command register in any particular state.
Strictly speaking, we only need to disable BM-DMA at ExitBootServices(),
in order to abort pending transfers to/from RAM, which is soon to be owned
by the OS. BM-DMA is also the only bit that's explicitly named by the UEFI
Driver Writers' Guide, for clearing at ExitBootServices().
I've verified that clearing only BM-DMA fixes the issue (boot time) on
i440fx, and does not regress q35/AHCI.
Cc: Aleksei Kovura <alex3kov@zoho.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Dann Frazier <dannf@ubuntu.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Reported-by: Aleksei Kovura <alex3kov@zoho.com>
Reported-by: Dann Frazier <dannf@ubuntu.com>
Reported-by: https://launchpad.net/~cjkrupp
Bisected-by: Dann Frazier <dannf@ubuntu.com>
Bisected-by: https://launchpad.net/~cjkrupp
Suggested-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Suggested-by: Star Zeng <star.zeng@intel.com>
Ref: https://bugs.launchpad.net/ubuntu/+source/edk2/+bug/1725560
Fixes: 6fb8ddd36b
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Tested-by: dann frazier <dann.frazier@canonical.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
Tested-by: Aleksei Kovura <alex3kov@zoho.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=699
Within function AhciModeInitialization(), left shift operations of 'BIT0'
in the following statements:
"if ((PortImplementBitMap & (BIT0 << Port)) != 0) {"
will incur possible out of range left shift when Port is 31, since
"1 << 31" is possible to exceed the range of type 'int' (signed).
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.
This commit explicitly cast 'BIT0' with UINT32 to resolve this issue.
Cc: Steven Shi <steven.shi@intel.com>
Cc: Eric Dong <eric.dong@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>
The AtaAtapiPassThru driver maps three system memory regions for Bus
Master Common Buffer operation on the following call path, if the
controller has PCI_CLASS_MASS_STORAGE_SATADPA class code:
AtaAtapiPassThruStart()
EnumerateAttachedDevice()
AhciModeInitialization()
AhciCreateTransferDescriptor()
The device is disabled (including Bus Master DMA) when the controller is
unbound, in AtaAtapiPassThruStop(). Then the regions are unmapped.
The former step should also be done when we exit the boot services, and
the OS gains ownership of system memory.
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
In AtaAtapiPassThruStop(), if the device has been operating in AHCI mode,
we unmap the DMA buffers and then disable the device (including bus master
DMA). The order of these actions is wrong; we shouldn't unmap DMA buffers
until bus master DMA is turned off. Reverse the steps.
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Both AtaAtapiPassThruStart() and AtaAtapiPassThruStop() fetch the
supported attributes of the device, just so they can toggle the
IO+MMIO+BusMaster subset.
After we compute this bitmask in AtaAtapiPassThruStart(), we can cache it
for later, and save the fetch in AtaAtapiPassThruStop().
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
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>
According to AHCI Spec 1.3 GHC.AE bit description:
"The implementation of this bit is dependent upon the value of the
CAP.SAM bit. If CAP.SAM is '0', then GHC.AE shall be read-write and shall
have a reset value of '0'. If CAP.SAM is '1', then AE shall be read-only
and shall have a reset value of '1'."
Being in AhciMode, for proper operation it is required, that GHC.AE bit
is always set, before any other AHCI registers are written to. Current
AhciMode implementation, both in AhciReset() and AhciModeInitialization()
functions, set GHC.AE bit only depending on 'CAP.SAM == 0' condition,
assuming (according to the AHCI spec), that otherwise it has to be set
anyway. It may however happen, that even if 'CAP.SAM == 1', GHC.AE
requires updating by software.
This patch enables in AhciMode setting GHC.AE in case its initial value
is '0'. It fixes AHCI support for Marvell Armada 70x0 and 80x0 SoC
families. The change is transparent to all other platforms.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Marcin Wojtas <mw@semihalf.com>
Signed-off-by: Jan Dabros <jsd@semihalf.com>
Reviewed-by: Feng Tian <feng.tian@intel.com>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
AhciDumpPortStatus doesn't fully populate all the fields of
AtaStatusBlock after completing command execution, which may bring
issue if someone depends on the return status.
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: Star Zeng <star.zeng@intel.com>
If there is no port multiplier (PortMultiplierPort = 0xFFFF), current code
in functions TransferAtaDevice() and TrustTransferAtaDevice() will always
set the DEV bit of the ATA device register. It causes that ATA commands
cannot be sent to some ATA hard drives.
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>
It is enough to set PxCMD.FRE bit, which cause HBA to post received FISes
into the FIS receive area. According to AHCI Specification, only polling on
PxCMD.FRE to be cleared is necessary, when it is needeed to stop FIS engine
(eg. in order to change PxCMD.FB address).
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>
If there is no multiplier, the DEV bit of the ATA device register would
always be set. It causes that some ATA hard drives don't response the
ATA identity command sent to them.
Below is the description about DEV bit in ATA spec:
A device is selected when the DEV bit of the Device register is equal to
the device number assigned to the device by means of a Device 0/Device 1
jumper or switch, or use of the CSEL signal.
Below is the description about DEV bit in SATA spec:
When the DEV bit in the Device register is set to one, selecting the
non-existent Device 1, the host adapter shall respond to register reads
and writes as specified for a Device 0 with no Device 1 present, as
defined in the ATA/ATAPI-5 standard.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Cinnamon Shia <cinnamon.shia@hpe.com>
Reviewed-by: Feng Tian <feng.tian@intel.com>
Reviewed-by: Hao Wu <hao.a.wu@intel.com>
The Mantis ticket 1353 <https://mantis.uefi.org/mantis/view.php?id=1353>
and Mantis ticket 1472 <https://mantis.uefi.org/mantis/view.php?id=1472>
updated the description of the port multiplier port number parameter in
SATA Device Path Node and ATA Pass-Through Protocol.
Now, this parameter should be set to 0xFFFF instead of 0 to indicate that
an ATA device is directly attached on the controller port.
Please note that this is an incompatible change. The consumer of SATA
device path or ATA_PASS_THRU needs to re-examine its usage to follow UEFI
2.5 mantis 1353 and 1472.
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 some SCSI commands, notably INQUIRY, it's relatively common for
the device to provide less data than we intended to read, and for
this reason EFI_EXT_SCSI_PASS_THRU_SCSI_REQUEST_PACKET makes
InTransferLength and OutTransferLength read-write. Make ATAPI
aware of this.
This makes it possible to handle EFI_NOT_READY always, not just
for read as done in r19685.
I've chosen to use a break statement instead of calling
CheckStatusRegister directly; the break statement reaches a
pre-existing call the CheckStatusRegister function. This
ensures that the assignment to *ByteCount is not missed, and
adds a further sanity check to DRQClear.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Tested-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Feng Tian <feng.tian@intel.com>
git-svn-id: https://svn.code.sf.net/p/edk2/code/trunk/edk2@19737 6f19259b-4bc3-4df7-8a09-765794883524
SVN r19611 (git commit 7cac240163), "MdeModulePkg/Ide: return correct
status when DRQ is not ready for ATAPI", changed the behavior of
AtaPacketReadWrite(), when DRQReady2() reported an error. The previous
logic had been to:
(a) terminate the transfer loop,
(b) check the status register with CheckStatusRegister(), and determine
AtaPacketReadWrite()'s return code directly from that.
Action (a) had been correct, but action (b) had masked genuine errors.
For example, when DRQReady2() reported EFI_TIMEOUT -- because the BSY bit
had not been cleared within the allotted time --, CheckStatusRegister()
would report EFI_SUCCESS, simply *because* BSY was still set, and the rest
of the status bits could not be evaluated.
SVN r19611 (git commit 7cac240163) intended to fix action (b) by directly
propagating the error code of DRQReady2() from AtaPacketReadWrite(),
eliminating the CheckStatusRegister() call. This was the right thing for
most of the errors reported by DRQReady2() -- timeout, command abort,
other device error --, but there was one exception: the "read" sub-case of
EFI_NOT_READY, which stands for "'read' complete, with less data available
than the requested amount".
Regarding the "write" sub-case of EFI_NOT_READY: the
AtaPacketCommandExecute() function programs the full transfer length into
the IDE device before it calls AtaPacketReadWrite(), and
AtaPacketReadWrite() only uses CylinderLsb and CylinderMsb for "chunking"
(as requested by the device). Therefore the device cannot justifiedly
clear DRQ earlier than seeing the entire data, when writing.
However, when reading from the device, a "short read" is a successful
operation. (The actual read length will be decoded by the higher level
protocols.) And "short reads" had been handled correctly by the logic
before git 7cac240163. Namely, when DRQReady2() returns EFI_NOT_READY, the
BSY bit is already clear, and we can call CheckStatusRegister() to
investigate all the other bits it cares about.
Therefore restore the logic from before git 7cac240163, but only for the
"read" sub-case of EFI_NOT_READY.
This problem was encountered with OVMF running on QEMU's i440fx IDE
emulation. Many thanks to John Snow for analyzing QEMU's behavior, and
pointing out that it adhered to the relevant specs.
Cc: Feng Tian <feng.tian@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: John Snow <jsnow@redhat.com>
Reference: https://github.com/tianocore/edk2/issues/43
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Feng Tian <feng.tian@intel.com>
git-svn-id: https://svn.code.sf.net/p/edk2/code/trunk/edk2@19685 6f19259b-4bc3-4df7-8a09-765794883524
The DRQReady() and DRQReady2() functions only differ in that they poll
different status registers for BSY, ERR, and DRQ: the former looks at the
Status Register (clearing interrupt status), while the latter looks at the
Alternate Status Register (not clearing interrupt status).
They both correctly return a unique status code, EFI_NOT_READY, for the
BSY==0 && ERR==0 && DRQ==0
case; that is, when the device reports "command complete".
However, the functions' leading comments don't explain this case, so it's
easy to miss in callers. Update the comments.
Cc: Feng Tian <feng.tian@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: John Snow <jsnow@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Feng Tian <feng.tian@intel.com>
git-svn-id: https://svn.code.sf.net/p/edk2/code/trunk/edk2@19684 6f19259b-4bc3-4df7-8a09-765794883524
When executing ATAPI cmd at IDE mode, EFI_SUCCESS may be returned wrongly
with old logic but in fact DRQ is not ready and the transaction doesn't
get executed correctly at this time.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Feng Tian <feng.tian@intel.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
git-svn-id: https://svn.code.sf.net/p/edk2/code/trunk/edk2@19611 6f19259b-4bc3-4df7-8a09-765794883524
Some IDE controllers only update ID fields such as sector information on
specific commands such as the DIAG command.
The master/slave device is therefore selected both before and after
sending the DIAG command; otherwise reading the IDE registers yields the
master's ID fields.
(Reza's patch originally *moved* the master/slave selection from after the
DIAG command before it, however Feng asked for the original master/slave
select to be preserved; effectively making the master/slave select
*bracket* the DIAG command. See this subthread:
<http://thread.gmane.org/gmane.comp.bios.tianocore.devel/10545/focus=10550>.)
Cc: Alexander Graf <agraf@suse.de>
Cc: Reza Jelveh <reza.jelveh@tuhh.de>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Hannes Reinecke <hare@suse.de>
Cc: Gabriel L. Somlo <somlo@cmu.edu>
Cc: Feng Tian <feng.tian@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Reza Jelveh <reza.jelveh@tuhh.de>
[feng.tian@intel.com: see above]
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Feng Tian <feng.tian@intel.com>
[lersek@redhat.com: updated commit message]
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Feng Tian <feng.tian@intel.com>
Tested-by: Gabriel Somlo <somlo@cmu.edu>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
git-svn-id: https://svn.code.sf.net/p/edk2/code/trunk/edk2@18529 6f19259b-4bc3-4df7-8a09-765794883524
The TPer Reset operation would be moved into SecurityPkg/Tcg/
MemoryOverwriteControl/TcgMor.inf module and be triggered at EndOfDxe.
By this way, all encrypted drives which produce EFI_STORAGE_SECURITY_
RPOTOCOL interface would be force reset when MOR is set.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Tian Feng <feng.tian@intel.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
git-svn-id: https://svn.code.sf.net/p/edk2/code/trunk/edk2@17717 6f19259b-4bc3-4df7-8a09-765794883524
Follow IDE BUS Master spec to ensure the PRDT table not cross 64k
boundary in memory.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Feng Tian <feng.tian@intel.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
git-svn-id: https://svn.code.sf.net/p/edk2/code/trunk/edk2@17552 6f19259b-4bc3-4df7-8a09-765794883524
The original code hard-codes block size to 512. But after ATA 7 spec,
the non-512 block size is also supported. The code is updated to
dynamically calculate the block size according to IDENTIFY data.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Feng Tian <feng.tian@intel.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
git-svn-id: https://svn.code.sf.net/p/edk2/code/trunk/edk2@17495 6f19259b-4bc3-4df7-8a09-765794883524
Setting a bit in the PxSACT register for a command in the command list indicates
to the controller that the command list slot contains a native queued command (NCQ).
This can cause problems with some controllers (one such controller is the Marvell 9128).
Since NCQ commands are not used, don't set the PxACT register for commands issued.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Chris Ruffin <chris.ruffin@intel.com>
Reviewed-by: Feng Tian <feng.tian@intel.com>
git-svn-id: https://svn.code.sf.net/p/edk2/code/trunk/edk2@16536 6f19259b-4bc3-4df7-8a09-765794883524
Per AHCI 1.1 spec, AE bit of GHC register is read-only if CAP.SAM is 1
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Feng Tian <feng.tian@intel.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
git-svn-id: https://svn.code.sf.net/p/edk2/code/trunk/edk2@16321 6f19259b-4bc3-4df7-8a09-765794883524
Liming Gao
Albecki, Mateusz
Antoine Coeur
Hao A Wu
Michael D Kinney
Chen A Chen
shenglei
Liming Gao
Ruiyu Ni
Laszlo Ersek
Ard Biesheuvel
Marcin Wojtas
Feng Tian
Gary Lin
Jan D?bro?
Cinnamon Shia
Paolo Bonzini
Samer El-Haj-Mahmoud
Jordan Justen
Reza Jelveh
Chris Ruffin