Re: [edk2] [patch 1/2] MdeModulePkg/Ide: return correct status when DRQ is not ready for ATAPI

[Laszlo Ersek]

On 01/19/16 21:34, Paolo Bonzini wrote:
> 
> 
> On 19/01/2016 00:29, Laszlo Ersek wrote:
>>> INQUIRY works slightly different from other commands. It has a maximum
>>> length instead of a deterministic length; once HCYL/LCYL are read back,
>>> should RequiredBytes not be adjusted in this case? (This way we don't
>>> short the loop, we just finish gracefully.)
> 
> I like John's suggestion, and it can be applied to _any_ command, not
> just INQUIRY.
> 
> However, ByteCount should be changed to a UINT32* and updated by
> AtaPacketReadWrite.  Then the ActualWordCount can be reflected into the
> InTransferLength or OutTransferLength of struct
> EFI_EXT_SCSI_PASS_THRU_SCSI_REQUEST_PACKET.

Thanks!

Can you contribute a patch? :)

Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [patch 1/2] MdeModulePkg/Ide: return correct status when DRQ is not ready for ATAPI

[Paolo Bonzini]

On 19/01/2016 00:29, Laszlo Ersek wrote:
>> INQUIRY works slightly different from other commands. It has a maximum
>> length instead of a deterministic length; once HCYL/LCYL are read back,
>> should RequiredBytes not be adjusted in this case? (This way we don't
>> short the loop, we just finish gracefully.)

I like John's suggestion, and it can be applied to _any_ command, not
just INQUIRY.

However, ByteCount should be changed to a UINT32* and updated by
AtaPacketReadWrite.  Then the ActualWordCount can be reflected into the
InTransferLength or OutTransferLength of struct
EFI_EXT_SCSI_PASS_THRU_SCSI_REQUEST_PACKET.

Paolo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [patch 1/2] MdeModulePkg/Ide: return correct status when DRQ is not ready for ATAPI

[Laszlo Ersek]

On 01/18/16 19:44, John Snow wrote:
> 
> 
> On 01/18/2016 05:57 AM, Laszlo Ersek wrote:
>> Hello Feng, John,
>>
>> On 11/03/15 02:48, Tian Feng wrote:
>>> 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 
>>> Cc: Star Zeng 
>>> ---
>>>  MdeModulePkg/Bus/Ata/AtaAtapiPassThru/IdeMode.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/IdeMode.c 
>>> b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/IdeMode.c
>>> index 4928ed5..f74e5ca 100644
>>> --- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/IdeMode.c
>>> +++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/IdeMode.c
>>> @@ -1949,7 +1949,7 @@ AtaPacketReadWrite (
>>>  //
>>>  Status = DRQReady2 (PciIo, IdeRegisters, Timeout);
>>>  if (EFI_ERROR (Status)) {
>>> -  return CheckStatusRegister (PciIo, IdeRegisters);
>>> +  return Status;
>>>  }
>>>  
>>>  //
>>>
>>
>> Unfortunately, this patch breaks OVMF on QEMU's ide-cd device. The
>> symptoms were reported in ;
> 
> To confirm, does this affect both the AHCI ATAPI device and the BMDMA
> ATAPI device? (Q35 and PC default HBA, respectively) Or is it just AHCI?

I only used i440fx for testing. This is my command line:

qemu-system-x86_64 \
  -debugcon file:ovmf.log \
  -global isa-debugcon.iobase=0x402 \
  -net none \
  -enable-kvm \
  -pflash OVMF.fd \
  -drive id=cd0,if=none,readonly,format=raw,file=f22.iso \
  -device ide-cd,drive=cd0,bootindex=0

If I add "-M q35" (--> AHCI), then things seem to work fine; I can reach
grub on the F22 workstation installer CD.

>> I reproduced the issue and bisected it to this patch (git commit
>> 7cac2401 / SVN 19611). I also confirmed it by reverting the patch on top
>> of current master, and the CD-ROM started to work.
>>
>> I'm adding John Snow, who maintains QEMU's IDE emulation.
>>
>> Can you guys please investigate this patch, and discuss why it breaks on
>> QEMU's ide-cd device?
>>
>> John, if you'd like to browse the code, the following link highlights
>> the line that the above patch changes:
>>
>> https://github.com/tianocore/edk2/blob/7cac2401635317360226a235d1f95f8347081cbb/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/IdeMode.c#L1952
>>
>> As far as I can see, this problem could be timing-sensitive. The
>> DRQReady2() function polls for the DRQ bit to become set, in a certain
>> amount of time. If that doesn't happen (or a definitive error emerges),
>> the function fails.
>>
>> Pre-patch, such failure would not be decisive; the status register would
>> be read and evaluated. Post-patch, the failure is decisive, even if it
>> is just a timeout, and the CheckStatusRegister() call would otherwise
>> return success.
>>
>> ... Hm, I added a debug log right after DRQReady2(), on the error branch
>> (i.e., when it fails). The status code is *not* EFI_TIMEOUT, it is
>> EFI_NOT_READY.
>>
>> Looking at DRQReady2(), this means:
>> - BSY is clear
>> - ERR is clear
>> - DRQ is clear too
>>
>> Apparently, the DRQReady2() function expects that as soon as BSY is
>> clear (and there is no error), DRQ must be immediately set (more or
>> less: not busy, no error, so ready to transfer data).
>>
> 
> Thank you for your research!
> 
>> Is that a valid assumption to make?
>>
> 
> Whenever BSY, DRQ and ERR are all clear that should be confirmation of a
> completed command.

That's interesting, because the edk2 code seems to treat this condition
as "failure to transition to data transfer", or some such.

>Under AHCI, this should *not* be true before the AHCI
> device has signalled completion (Either via the status shadow register,
> an interrupt, or an MSI interrupt, depending on device configuration.)
> 
> (In fact, hw/ide/core.c ide_exec_cmd uses the presence of DRQ|BSY to
> know whether or not it is in a state fit to accept a new command. If the
> guest is not blocked when BSY|DRQ are unset, this means a new command
> can be issued.)
> 
> == Thought #1: ==
> 
> However, when a command completes, it updates the shadow registers in
> the AHCI HBA right before signalling completion. It is, I think,
> possible that if you are polling the AHCI shadow registers instead of
> polling the AHCI status registers that you could find DRQ/BSY/ERR unset
> before receiving a command completed notice (by a time window of two
> passing electrons.)
> 
> Take a look at ahci_cmd_done, which calls ahci_write_fis_d2h. We
> populate a guest memory FIS buffer with the values of the registers,
> then update the status shadow register, then signal completion (via IRQ,
> MSI, or neither.)
> 
> You are probably not hitting that reliably, so can I ask what command is
> erroring out, here? 0xA0 PACKET for IDE, and 

Re: [edk2] [patch 1/2] MdeModulePkg/Ide: return correct status when DRQ is not ready for ATAPI

[Laszlo Ersek]

On 01/19/16 00:08, John Snow wrote:
> 
> 
> On 01/18/2016 05:28 PM, Laszlo Ersek wrote:
>> On 01/18/16 22:05, John Snow wrote:
>>>
>>>
>>> On 01/18/2016 02:33 PM, Laszlo Ersek wrote:
 On 01/18/16 19:44, John Snow wrote:
>
>
> On 01/18/2016 05:57 AM, Laszlo Ersek wrote:
>> Hello Feng, John,
>>
>> On 11/03/15 02:48, Tian Feng wrote:
>>> 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 
>>> Cc: Star Zeng 
>>> ---
>>>  MdeModulePkg/Bus/Ata/AtaAtapiPassThru/IdeMode.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/IdeMode.c 
>>> b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/IdeMode.c
>>> index 4928ed5..f74e5ca 100644
>>> --- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/IdeMode.c
>>> +++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/IdeMode.c
>>> @@ -1949,7 +1949,7 @@ AtaPacketReadWrite (
>>>  //
>>>  Status = DRQReady2 (PciIo, IdeRegisters, Timeout);
>>>  if (EFI_ERROR (Status)) {
>>> -  return CheckStatusRegister (PciIo, IdeRegisters);
>>> +  return Status;
>>>  }
>>>  
>>>  //
>>>
>>
>> Unfortunately, this patch breaks OVMF on QEMU's ide-cd device. The
>> symptoms were reported in ;
>
> To confirm, does this affect both the AHCI ATAPI device and the BMDMA
> ATAPI device? (Q35 and PC default HBA, respectively) Or is it just AHCI?

 I only used i440fx for testing. This is my command line:

 qemu-system-x86_64 \
   -debugcon file:ovmf.log \
   -global isa-debugcon.iobase=0x402 \
   -net none \
   -enable-kvm \
   -pflash OVMF.fd \
   -drive id=cd0,if=none,readonly,format=raw,file=f22.iso \
   -device ide-cd,drive=cd0,bootindex=0

 If I add "-M q35" (--> AHCI), then things seem to work fine; I can reach
 grub on the F22 workstation installer CD.

>> I reproduced the issue and bisected it to this patch (git commit
>> 7cac2401 / SVN 19611). I also confirmed it by reverting the patch on top
>> of current master, and the CD-ROM started to work.
>>
>> I'm adding John Snow, who maintains QEMU's IDE emulation.
>>
>> Can you guys please investigate this patch, and discuss why it breaks on
>> QEMU's ide-cd device?
>>
>> John, if you'd like to browse the code, the following link highlights
>> the line that the above patch changes:
>>
>> https://github.com/tianocore/edk2/blob/7cac2401635317360226a235d1f95f8347081cbb/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/IdeMode.c#L1952
>>
>> As far as I can see, this problem could be timing-sensitive. The
>> DRQReady2() function polls for the DRQ bit to become set, in a certain
>> amount of time. If that doesn't happen (or a definitive error emerges),
>> the function fails.
>>
>> Pre-patch, such failure would not be decisive; the status register would
>> be read and evaluated. Post-patch, the failure is decisive, even if it
>> is just a timeout, and the CheckStatusRegister() call would otherwise
>> return success.
>>
>> ... Hm, I added a debug log right after DRQReady2(), on the error branch
>> (i.e., when it fails). The status code is *not* EFI_TIMEOUT, it is
>> EFI_NOT_READY.
>>
>> Looking at DRQReady2(), this means:
>> - BSY is clear
>> - ERR is clear
>> - DRQ is clear too
>>
>> Apparently, the DRQReady2() function expects that as soon as BSY is
>> clear (and there is no error), DRQ must be immediately set (more or
>> less: not busy, no error, so ready to transfer data).
>>
>
> Thank you for your research!
>
>> Is that a valid assumption to make?
>>
>
> Whenever BSY, DRQ and ERR are all clear that should be confirmation of a
> completed command.

 That's interesting, because the edk2 code seems to treat this condition
 as "failure to transition to data transfer", or some such.

>>>
>>> If it's in the middle of a transfer loop, it's a weird condition to
>>> encounter. We expect to see BSY and/or DRQ for the duration of the
>>> command. At the end of the command, though, READY|SEEK is the normal
>>> idle status.
>>>
>Under AHCI, this should *not* be true before the AHCI
> device has signalled completion (Either via the status shadow register,
> an interrupt, or an MSI interrupt, depending on device configuration.)
>
> (In fact, hw/ide/core.c ide_exec_cmd uses the presence of DRQ|BSY to
> know whether or not it is in a state fit to accept 

Re: [edk2] [patch 1/2] MdeModulePkg/Ide: return correct status when DRQ is not ready for ATAPI

[Laszlo Ersek]

On 01/18/16 22:05, John Snow wrote:
> 
> 
> On 01/18/2016 02:33 PM, Laszlo Ersek wrote:
>> On 01/18/16 19:44, John Snow wrote:
>>>
>>>
>>> On 01/18/2016 05:57 AM, Laszlo Ersek wrote:
 Hello Feng, John,

 On 11/03/15 02:48, Tian Feng wrote:
> 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 
> Cc: Star Zeng 
> ---
>  MdeModulePkg/Bus/Ata/AtaAtapiPassThru/IdeMode.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/IdeMode.c 
> b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/IdeMode.c
> index 4928ed5..f74e5ca 100644
> --- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/IdeMode.c
> +++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/IdeMode.c
> @@ -1949,7 +1949,7 @@ AtaPacketReadWrite (
>  //
>  Status = DRQReady2 (PciIo, IdeRegisters, Timeout);
>  if (EFI_ERROR (Status)) {
> -  return CheckStatusRegister (PciIo, IdeRegisters);
> +  return Status;
>  }
>  
>  //
>

 Unfortunately, this patch breaks OVMF on QEMU's ide-cd device. The
 symptoms were reported in ;
>>>
>>> To confirm, does this affect both the AHCI ATAPI device and the BMDMA
>>> ATAPI device? (Q35 and PC default HBA, respectively) Or is it just AHCI?
>>
>> I only used i440fx for testing. This is my command line:
>>
>> qemu-system-x86_64 \
>>   -debugcon file:ovmf.log \
>>   -global isa-debugcon.iobase=0x402 \
>>   -net none \
>>   -enable-kvm \
>>   -pflash OVMF.fd \
>>   -drive id=cd0,if=none,readonly,format=raw,file=f22.iso \
>>   -device ide-cd,drive=cd0,bootindex=0
>>
>> If I add "-M q35" (--> AHCI), then things seem to work fine; I can reach
>> grub on the F22 workstation installer CD.
>>
 I reproduced the issue and bisected it to this patch (git commit
 7cac2401 / SVN 19611). I also confirmed it by reverting the patch on top
 of current master, and the CD-ROM started to work.

 I'm adding John Snow, who maintains QEMU's IDE emulation.

 Can you guys please investigate this patch, and discuss why it breaks on
 QEMU's ide-cd device?

 John, if you'd like to browse the code, the following link highlights
 the line that the above patch changes:

 https://github.com/tianocore/edk2/blob/7cac2401635317360226a235d1f95f8347081cbb/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/IdeMode.c#L1952

 As far as I can see, this problem could be timing-sensitive. The
 DRQReady2() function polls for the DRQ bit to become set, in a certain
 amount of time. If that doesn't happen (or a definitive error emerges),
 the function fails.

 Pre-patch, such failure would not be decisive; the status register would
 be read and evaluated. Post-patch, the failure is decisive, even if it
 is just a timeout, and the CheckStatusRegister() call would otherwise
 return success.

 ... Hm, I added a debug log right after DRQReady2(), on the error branch
 (i.e., when it fails). The status code is *not* EFI_TIMEOUT, it is
 EFI_NOT_READY.

 Looking at DRQReady2(), this means:
 - BSY is clear
 - ERR is clear
 - DRQ is clear too

 Apparently, the DRQReady2() function expects that as soon as BSY is
 clear (and there is no error), DRQ must be immediately set (more or
 less: not busy, no error, so ready to transfer data).

>>>
>>> Thank you for your research!
>>>
 Is that a valid assumption to make?

>>>
>>> Whenever BSY, DRQ and ERR are all clear that should be confirmation of a
>>> completed command.
>>
>> That's interesting, because the edk2 code seems to treat this condition
>> as "failure to transition to data transfer", or some such.
>>
> 
> If it's in the middle of a transfer loop, it's a weird condition to
> encounter. We expect to see BSY and/or DRQ for the duration of the
> command. At the end of the command, though, READY|SEEK is the normal
> idle status.
> 
>>>Under AHCI, this should *not* be true before the AHCI
>>> device has signalled completion (Either via the status shadow register,
>>> an interrupt, or an MSI interrupt, depending on device configuration.)
>>>
>>> (In fact, hw/ide/core.c ide_exec_cmd uses the presence of DRQ|BSY to
>>> know whether or not it is in a state fit to accept a new command. If the
>>> guest is not blocked when BSY|DRQ are unset, this means a new command
>>> can be issued.)
>>>
>>> == Thought #1: ==
>>>
>>> However, when a command completes, it updates the shadow registers in
>>> the AHCI HBA right before signalling completion. It is, I think,
>>> possible that if you are polling the 

Re: [edk2] [patch 1/2] MdeModulePkg/Ide: return correct status when DRQ is not ready for ATAPI

[Laszlo Ersek]

On 01/18/16 20:33, Laszlo Ersek wrote:
> On 01/18/16 19:44, John Snow wrote:
>>
>>
>> On 01/18/2016 05:57 AM, Laszlo Ersek wrote:
>>> Hello Feng, John,
>>>
>>> On 11/03/15 02:48, Tian Feng wrote:
 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 
 Cc: Star Zeng 
 ---
  MdeModulePkg/Bus/Ata/AtaAtapiPassThru/IdeMode.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/IdeMode.c 
 b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/IdeMode.c
 index 4928ed5..f74e5ca 100644
 --- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/IdeMode.c
 +++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/IdeMode.c
 @@ -1949,7 +1949,7 @@ AtaPacketReadWrite (
  //
  Status = DRQReady2 (PciIo, IdeRegisters, Timeout);
  if (EFI_ERROR (Status)) {
 -  return CheckStatusRegister (PciIo, IdeRegisters);
 +  return Status;
  }
  
  //

>>>
>>> Unfortunately, this patch breaks OVMF on QEMU's ide-cd device. The
>>> symptoms were reported in ;
>>
>> To confirm, does this affect both the AHCI ATAPI device and the BMDMA
>> ATAPI device? (Q35 and PC default HBA, respectively) Or is it just AHCI?
> 
> I only used i440fx for testing. This is my command line:
> 
> qemu-system-x86_64 \
>   -debugcon file:ovmf.log \
>   -global isa-debugcon.iobase=0x402 \
>   -net none \
>   -enable-kvm \
>   -pflash OVMF.fd \
>   -drive id=cd0,if=none,readonly,format=raw,file=f22.iso \
>   -device ide-cd,drive=cd0,bootindex=0
> 
> If I add "-M q35" (--> AHCI), then things seem to work fine; I can reach
> grub on the F22 workstation installer CD.
> 
>>> I reproduced the issue and bisected it to this patch (git commit
>>> 7cac2401 / SVN 19611). I also confirmed it by reverting the patch on top
>>> of current master, and the CD-ROM started to work.
>>>
>>> I'm adding John Snow, who maintains QEMU's IDE emulation.
>>>
>>> Can you guys please investigate this patch, and discuss why it breaks on
>>> QEMU's ide-cd device?
>>>
>>> John, if you'd like to browse the code, the following link highlights
>>> the line that the above patch changes:
>>>
>>> https://github.com/tianocore/edk2/blob/7cac2401635317360226a235d1f95f8347081cbb/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/IdeMode.c#L1952
>>>
>>> As far as I can see, this problem could be timing-sensitive. The
>>> DRQReady2() function polls for the DRQ bit to become set, in a certain
>>> amount of time. If that doesn't happen (or a definitive error emerges),
>>> the function fails.
>>>
>>> Pre-patch, such failure would not be decisive; the status register would
>>> be read and evaluated. Post-patch, the failure is decisive, even if it
>>> is just a timeout, and the CheckStatusRegister() call would otherwise
>>> return success.
>>>
>>> ... Hm, I added a debug log right after DRQReady2(), on the error branch
>>> (i.e., when it fails). The status code is *not* EFI_TIMEOUT, it is
>>> EFI_NOT_READY.
>>>
>>> Looking at DRQReady2(), this means:
>>> - BSY is clear
>>> - ERR is clear
>>> - DRQ is clear too
>>>
>>> Apparently, the DRQReady2() function expects that as soon as BSY is
>>> clear (and there is no error), DRQ must be immediately set (more or
>>> less: not busy, no error, so ready to transfer data).
>>>
>>
>> Thank you for your research!
>>
>>> Is that a valid assumption to make?
>>>
>>
>> Whenever BSY, DRQ and ERR are all clear that should be confirmation of a
>> completed command.
> 
> That's interesting, because the edk2 code seems to treat this condition
> as "failure to transition to data transfer", or some such.
> 
>>Under AHCI, this should *not* be true before the AHCI
>> device has signalled completion (Either via the status shadow register,
>> an interrupt, or an MSI interrupt, depending on device configuration.)
>>
>> (In fact, hw/ide/core.c ide_exec_cmd uses the presence of DRQ|BSY to
>> know whether or not it is in a state fit to accept a new command. If the
>> guest is not blocked when BSY|DRQ are unset, this means a new command
>> can be issued.)
>>
>> == Thought #1: ==
>>
>> However, when a command completes, it updates the shadow registers in
>> the AHCI HBA right before signalling completion. It is, I think,
>> possible that if you are polling the AHCI shadow registers instead of
>> polling the AHCI status registers that you could find DRQ/BSY/ERR unset
>> before receiving a command completed notice (by a time window of two
>> passing electrons.)
>>
>> Take a look at ahci_cmd_done, which calls ahci_write_fis_d2h. We
>> populate a guest memory FIS buffer with the values of the registers,
>> then update the status shadow register, then 

[edk2] [patch 1/2] MdeModulePkg/Ide: return correct status when DRQ is not ready for ATAPI

[Tian Feng]

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 
Cc: Star Zeng 
---
 MdeModulePkg/Bus/Ata/AtaAtapiPassThru/IdeMode.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/IdeMode.c 
b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/IdeMode.c
index 4928ed5..f74e5ca 100644
--- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/IdeMode.c
+++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/IdeMode.c
@@ -1949,7 +1949,7 @@ AtaPacketReadWrite (
 //
 Status = DRQReady2 (PciIo, IdeRegisters, Timeout);
 if (EFI_ERROR (Status)) {
-  return CheckStatusRegister (PciIo, IdeRegisters);
+  return Status;
 }
 
 //
-- 
1.9.5.msysgit.0

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel