Yes, Laszlo, you are right. Back to that time, we did not realize there will be S4 issue.
Now, I agree with you that AhciReset is a better way. And I think we also need test different UEFI OS (normal boot/S4) to see if there is other issue. Hope AhciReset() is good enough, and won't bring compatibility issue. Thank you Yao Jiewen > -----Original Message----- > From: edk2-devel [mailto:[email protected]] On Behalf Of Laszlo > Ersek > Sent: Monday, November 27, 2017 8:30 PM > To: Yao, Jiewen <[email protected]>; Ni, Ruiyu <[email protected]>; Paolo > Bonzini <[email protected]> > Cc: edk2-devel-01 <[email protected]>; Dann Frazier > <[email protected]>; Dong, Eric <[email protected]>; Zeng, Star > <[email protected]>; Ard Biesheuvel <[email protected]> > Subject: Re: [edk2] [PATCH] MdeModulePkg/AtaAtapiPassThru: disable only > BM-DMA at ExitBootServices() > > Hi Jiewen, > > On 11/24/17 02:40, Yao, Jiewen wrote: > > Maybe, can we revisit the original requirement on why we need disable BME at > ExitBootService for OVMF? > > > > I recall we have lots of discussion at September. It is good to refresh. > > > > ============================== > > [edk2] [PATCH 0/4] MdeModulePkg: some PCI HC drivers: unmap common > buffers at ExitBootServices() > > https://lists.01.org/pipermail/edk2-devel/2017-September/014099.html > > > > At ExitBootServices(), PCI and VirtIo drivers should only care about > > aborting pending DMA on the devices. Cleaning up PciIo mappings (which > > ultimately boil down to IOMMU mappings) for those aborted DMA operations > > should be the job of the IOMMU driver. > > ============================== > > > > I think the Driver Writer's Guide recommend to stop the transaction. > > But it does not say you must turn off BME. > > Clear BME is just one way to meet the recommendation. > > Maybe we can figure out other way to halt the controller, or stop DMA > transaction? > > Such as stop timer event, set device reset/halt register, etc. > > I think USB has already done that. > > > > > > On the other hand, I do not think "OVMF does not support S4" is a good > justification to add PCD. > > Yes, it does not support at this moment. But who knows the status after 3 > > or 5 > years? > > I also heard some VMM do support S4 resume Guest. > > > > > > I also recommend to rollback all BME operation at EBS as first step, then go > back to see what is best way to > > I agree that, if the device and the driver offer another way to abort > pending DMA, we can use that too. > > In fact, my very first patch for AtaAtapiPassThru on this topic used > AhciReset(): > > [1] > http://mid.mail-archive.com/[email protected] > > But then you recommended clearing BusMasterEnable: > > [2] > http://mid.mail-archive.com/74D8A39837DF1E4DA445A8C0B3885C503A9A79 > [email protected] > > > (The old patch [1] I referenced above also called PciIo->Unmap(). We've > since agreed that *that* part of the idea was wrong, so I'm not > suggesting to return to PciIo->Unmap().) > > So, perhaps AhciReset() would be good enough after all, for aborting > pending DMA. > > Thanks, > Laszlo > > > > > >> -----Original Message----- > >> From: edk2-devel [mailto:[email protected]] On Behalf Of Ni, > >> Ruiyu > >> Sent: Friday, November 24, 2017 9:04 AM > >> To: Paolo Bonzini <[email protected]> > >> Cc: Dong, Eric <[email protected]>; Ard Biesheuvel > >> <[email protected]>; edk2-devel-01 <[email protected]>; > Dann > >> Frazier <[email protected]>; Laszlo Ersek <[email protected]>; Zeng, > Star > >> <[email protected]> > >> Subject: Re: [edk2] [PATCH] MdeModulePkg/AtaAtapiPassThru: disable only > >> BM-DMA at ExitBootServices() > >> > >> Maybe win10 does some optimization in S4 path. > >> > >> Sent from a small-screen device > >> > >> 在 2017年11月24日，上午8:01，Paolo Bonzini > >> <[email protected]<mailto:[email protected]>> 写道： > >> > >> On 23/11/2017 14:08, Laszlo Ersek wrote: > >> On 11/23/17 03:20, Ni, Ruiyu wrote: > >> I cannot explain precisely why the S4 resume fails. > >> I can just guess: Windows might have some assumptions on the BM bit. > >> Can we make this configurable on the platform level somehow? > >> > >> On one hand, I certainly don't want to break Windows 10, even in case > >> this issue ultimately turns out to be a Windows 10 bug. > >> > >> On the other hand, OVMF does not support S4, and disabling BMDMA at > >> ExitBootServices() in PCI drivers is specifically what the Driver > >> Writers' Guide recommends. Otherwise pending DMA could corrupt OS > memory. > >> > >> S4 can be done by the OS even if firmware says it doesn't support it. > >> > >> Once hibernation is done, it is merely a "courtesy" for the OSPM to turn > >> off the computer using the _S4 ACPI object rather than _S5. > >> > >> Paolo > >> _______________________________________________ > >> edk2-devel mailing list > >> [email protected] > >> https://lists.01.org/mailman/listinfo/edk2-devel > > _______________________________________________ > edk2-devel mailing list > [email protected] > https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
