Re: [PATCH 0/2] MdeModulePkg/AtaAtapiPassThru: Revert patch to disable PCI attributes

["Ni, Ruiyu" <ruiyu.ni@intel.com>]

Laszlo,
The Windows resume issue is urgent to fix.
I'd like to check-in the patches first.
If I didn't know the change may cause combability issue, I'd be very fine
to change it to AhciReset() then submit the patch, then after got a R-b,
I'd be very happy to check in that patch.
But since now I know there might be some combability issue, I don't
want to switch to AhciReset() solution without thoroughly testing.
And frankly I don't know what a thoroughly testing would be like.
There are many OSes and many PCI storage cards! ☹

Thanks/Ray

> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Monday, November 27, 2017 10:10 PM
> To: Ni, Ruiyu <ruiyu.ni@intel.com>
> Cc: edk2-devel@lists.01.org; Paolo Bonzini <pbonzini@redhat.com>; Yao,
> Jiewen <jiewen.yao@intel.com>; Zeng, Star <star.zeng@intel.com>
> Subject: Re: [edk2] [PATCH 0/2] MdeModulePkg/AtaAtapiPassThru: Revert
> patch to disable PCI attributes
> 
> Hello Ray,
> 
> On 11/27/17 02:19, Ruiyu Ni wrote:
> > The patches caused Windows 10 S4 resume failure.
> > Considering the similar changes are reverted from PciBus driver,
> > revert the patches from AtaAtapiPassThru as well.
> >
> > Ruiyu Ni (2):
> >   MdeModulePkg/AtaAtapiPassThru: Revert patch to disable Bus Master
> >   MdeModulePkg/AtaAtapiPassThru: Revert patch to disable PCI
> > attributes
> >
> >  .../Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c    | 58 +--------------------
> -
> >  .../Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h    |  5 --
> >  2 files changed, 1 insertion(+), 62 deletions(-)
> >
> 
> it looks like these patches have not been committed yet, which is a good
> thing, because apparently there's a better solution than a full revert.
> Namely, in the other sub-thread at
> <http://mid.mail-archive.com/0236afa2-e365-af7a-9374-
> 7fd1ad742c36@redhat.com>
> (alternative link:
> <https://lists.01.org/pipermail/edk2-devel/2017-November/018046.html>),
> Jiewen and Star seem to accept AhciReset() as a better way to abort pending
> DMA.
> 
> This means that we need not revert the EBS-handler altogether, only change
> what it does. Could you give that a try please?
> 
> (If the Windows regression is very urgent to fix, then I don't mind if the Bus
> Master disabling is removed separately, before AhciReset() is added; but in
> that case, a full revert looks unjustified, since the EBS handler will have to be
> re-added for AhciReset() anyway.)
> 
> Thanks,
> Laszlo