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(-)

-- 
2.15.0.gvfs.1.preview.4

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

Both patches are reviewed-by: Jiewen.yao@intel.com

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ruiyu
> Ni
> Sent: Monday, November 27, 2017 9:20 AM
> To: edk2-devel@lists.01.org
> Subject: [edk2] [PATCH 0/2] MdeModulePkg/AtaAtapiPassThru: Revert patch to
> disable PCI attributes
> 
> 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(-)
> 
> --
> 2.15.0.gvfs.1.preview.4
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Reviewed-by: Star Zeng <star.zeng@intel.com>

-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ruiyu Ni
Sent: Monday, November 27, 2017 9:20 AM
To: edk2-devel@lists.01.org
Subject: [edk2] [PATCH 0/2] MdeModulePkg/AtaAtapiPassThru: Revert patch to disable PCI attributes

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(-)

--
2.15.0.gvfs.1.preview.4

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

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
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

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
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

On 11/28/17 08:39, Ni, Ruiyu wrote:
> 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! ☹

Fair enough; thank you for the explanation.

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Laszlo

>> -----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

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