* [PATCH 1/2] MdeModulePkg/AtaAtapiPassThru: Revert patch to disable Bus Master
2017-11-27 1:19 [PATCH 0/2] MdeModulePkg/AtaAtapiPassThru: Revert patch to disable PCI attributes Ruiyu Ni
@ 2017-11-27 1:19 ` Ruiyu Ni
2017-11-27 1:19 ` [PATCH 2/2] MdeModulePkg/AtaAtapiPassThru: Revert patch to disable PCI attributes Ruiyu Ni
` (3 subsequent siblings)
4 siblings, 0 replies; 8+ messages in thread
From: Ruiyu Ni @ 2017-11-27 1:19 UTC (permalink / raw)
To: edk2-devel; +Cc: Laszlo Ersek, Jiewen Yao, Star Zeng
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 76fd5a660d704538a1b14a58d03a4eef9682b01c.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
---
MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c | 5 +++--
MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h | 3 ++-
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c
index e10e0d4e65..09064dda18 100644
--- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c
+++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c
@@ -480,7 +480,8 @@ InitializeAtaAtapiPassThru (
}
/**
- Disable Bus Master DMA on the device when exiting the boot services.
+ Disable the device (especially Bus Master DMA) when exiting the boot
+ services.
@param[in] Event Event for which this notification function is being
called.
@@ -505,7 +506,7 @@ AtaPassThruExitBootServices (
PciIo->Attributes (
PciIo,
EfiPciIoAttributeOperationDisable,
- Instance->EnabledPciAttributes & EFI_PCI_IO_ATTRIBUTE_BUS_MASTER,
+ Instance->EnabledPciAttributes,
NULL
);
}
diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h
index 92c5bf2001..8d6eac706c 100644
--- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h
+++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h
@@ -123,7 +123,8 @@ typedef struct {
LIST_ENTRY NonBlockingTaskList;
//
- // For disabling Bus Master DMA on the device at ExitBootServices().
+ // For disabling the device (especially Bus Master DMA) at
+ // ExitBootServices().
//
EFI_EVENT ExitBootEvent;
} ATA_ATAPI_PASS_THRU_INSTANCE;
--
2.15.0.gvfs.1.preview.4
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/2] MdeModulePkg/AtaAtapiPassThru: Revert patch to disable PCI attributes
2017-11-27 1:19 [PATCH 0/2] MdeModulePkg/AtaAtapiPassThru: Revert patch to disable PCI attributes Ruiyu Ni
2017-11-27 1:19 ` [PATCH 1/2] MdeModulePkg/AtaAtapiPassThru: Revert patch to disable Bus Master Ruiyu Ni
@ 2017-11-27 1:19 ` Ruiyu Ni
2017-11-27 4:20 ` [PATCH 0/2] " Yao, Jiewen
` (2 subsequent siblings)
4 siblings, 0 replies; 8+ messages in thread
From: Ruiyu Ni @ 2017-11-27 1:19 UTC (permalink / raw)
To: edk2-devel; +Cc: Laszlo Ersek, Jiewen Yao, Star Zeng
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 6fb8ddd36bde45614b0a069528cdc97077835a74.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
---
.../Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c | 59 +---------------------
.../Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h | 6 ---
2 files changed, 1 insertion(+), 64 deletions(-)
diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c
index 09064dda18..a48b295d26 100644
--- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c
+++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c
@@ -104,8 +104,7 @@ ATA_ATAPI_PASS_THRU_INSTANCE gAtaAtapiPassThruInstanceTemplate = {
{ // NonBlocking TaskList
NULL,
NULL
- },
- NULL, // ExitBootEvent
+ }
};
ATAPI_DEVICE_PATH mAtapiDevicePathTemplate = {
@@ -479,38 +478,6 @@ InitializeAtaAtapiPassThru (
return Status;
}
-/**
- Disable the device (especially Bus Master DMA) when exiting the boot
- services.
-
- @param[in] Event Event for which this notification function is being
- called.
- @param[in] Context Pointer to the ATA_ATAPI_PASS_THRU_INSTANCE that
- represents the HBA.
-**/
-VOID
-EFIAPI
-AtaPassThruExitBootServices (
- IN EFI_EVENT Event,
- IN VOID *Context
- )
-{
- ATA_ATAPI_PASS_THRU_INSTANCE *Instance;
- EFI_PCI_IO_PROTOCOL *PciIo;
-
- DEBUG ((DEBUG_VERBOSE, "%a: Context=0x%p\n", __FUNCTION__, Context));
-
- Instance = Context;
- PciIo = Instance->PciIo;
-
- PciIo->Attributes (
- PciIo,
- EfiPciIoAttributeOperationDisable,
- Instance->EnabledPciAttributes,
- NULL
- );
-}
-
/**
Tests to see if this driver supports a given controller. If a child device is provided,
it further tests to see if this driver supports creating a handle for the specified child device.
@@ -790,17 +757,6 @@ AtaAtapiPassThruStart (
InitializeListHead(&Instance->DeviceList);
InitializeListHead(&Instance->NonBlockingTaskList);
- Status = gBS->CreateEvent (
- EVT_SIGNAL_EXIT_BOOT_SERVICES,
- TPL_CALLBACK,
- AtaPassThruExitBootServices,
- Instance,
- &Instance->ExitBootEvent
- );
- if (EFI_ERROR (Status)) {
- goto ErrorExit;
- }
-
Instance->TimerEvent = NULL;
Status = gBS->CreateEvent (
@@ -854,10 +810,6 @@ ErrorExit:
gBS->CloseEvent (Instance->TimerEvent);
}
- if ((Instance != NULL) && (Instance->ExitBootEvent != NULL)) {
- gBS->CloseEvent (Instance->ExitBootEvent);
- }
-
//
// Remove all inserted ATA devices.
//
@@ -956,15 +908,6 @@ AtaAtapiPassThruStop (
Instance->TimerEvent = NULL;
}
DestroyAsynTaskList (Instance, FALSE);
-
- //
- // Close event signaled at gBS->ExitBootServices().
- //
- if (Instance->ExitBootEvent != NULL) {
- gBS->CloseEvent (Instance->ExitBootEvent);
- Instance->ExitBootEvent = NULL;
- }
-
//
// Free allocated resource
//
diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h
index 8d6eac706c..85e5a55539 100644
--- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h
+++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h
@@ -121,12 +121,6 @@ typedef struct {
//
EFI_EVENT TimerEvent;
LIST_ENTRY NonBlockingTaskList;
-
- //
- // For disabling the device (especially Bus Master DMA) at
- // ExitBootServices().
- //
- EFI_EVENT ExitBootEvent;
} ATA_ATAPI_PASS_THRU_INSTANCE;
//
--
2.15.0.gvfs.1.preview.4
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 0/2] MdeModulePkg/AtaAtapiPassThru: Revert patch to disable PCI attributes
2017-11-27 1:19 [PATCH 0/2] MdeModulePkg/AtaAtapiPassThru: Revert patch to disable PCI attributes Ruiyu Ni
2017-11-27 1:19 ` [PATCH 1/2] MdeModulePkg/AtaAtapiPassThru: Revert patch to disable Bus Master Ruiyu Ni
2017-11-27 1:19 ` [PATCH 2/2] MdeModulePkg/AtaAtapiPassThru: Revert patch to disable PCI attributes Ruiyu Ni
@ 2017-11-27 4:20 ` Yao, Jiewen
2017-11-27 5:18 ` Zeng, Star
2017-11-27 14:09 ` Laszlo Ersek
4 siblings, 0 replies; 8+ messages in thread
From: Yao, Jiewen @ 2017-11-27 4:20 UTC (permalink / raw)
To: Ni, Ruiyu, edk2-devel@lists.01.org
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 0/2] MdeModulePkg/AtaAtapiPassThru: Revert patch to disable PCI attributes
2017-11-27 1:19 [PATCH 0/2] MdeModulePkg/AtaAtapiPassThru: Revert patch to disable PCI attributes Ruiyu Ni
` (2 preceding siblings ...)
2017-11-27 4:20 ` [PATCH 0/2] " Yao, Jiewen
@ 2017-11-27 5:18 ` Zeng, Star
2017-11-27 14:09 ` Laszlo Ersek
4 siblings, 0 replies; 8+ messages in thread
From: Zeng, Star @ 2017-11-27 5:18 UTC (permalink / raw)
To: Ni, Ruiyu, edk2-devel@lists.01.org; +Cc: Zeng, Star
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 0/2] MdeModulePkg/AtaAtapiPassThru: Revert patch to disable PCI attributes
2017-11-27 1:19 [PATCH 0/2] MdeModulePkg/AtaAtapiPassThru: Revert patch to disable PCI attributes Ruiyu Ni
` (3 preceding siblings ...)
2017-11-27 5:18 ` Zeng, Star
@ 2017-11-27 14:09 ` Laszlo Ersek
2017-11-28 7:39 ` Ni, Ruiyu
4 siblings, 1 reply; 8+ messages in thread
From: Laszlo Ersek @ 2017-11-27 14:09 UTC (permalink / raw)
To: Ruiyu Ni; +Cc: edk2-devel, Paolo Bonzini, Jiewen Yao, Star Zeng
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 0/2] MdeModulePkg/AtaAtapiPassThru: Revert patch to disable PCI attributes
2017-11-27 14:09 ` Laszlo Ersek
@ 2017-11-28 7:39 ` Ni, Ruiyu
2017-11-28 12:55 ` Laszlo Ersek
0 siblings, 1 reply; 8+ messages in thread
From: Ni, Ruiyu @ 2017-11-28 7:39 UTC (permalink / raw)
To: Laszlo Ersek
Cc: edk2-devel@lists.01.org, Paolo Bonzini, Yao, Jiewen, Zeng, Star
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 0/2] MdeModulePkg/AtaAtapiPassThru: Revert patch to disable PCI attributes
2017-11-28 7:39 ` Ni, Ruiyu
@ 2017-11-28 12:55 ` Laszlo Ersek
0 siblings, 0 replies; 8+ messages in thread
From: Laszlo Ersek @ 2017-11-28 12:55 UTC (permalink / raw)
To: Ni, Ruiyu; +Cc: edk2-devel@lists.01.org, Paolo Bonzini, Yao, Jiewen, Zeng, Star
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
^ permalink raw reply [flat|nested] 8+ messages in thread