public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Zeng, Star" <star.zeng@intel.com>
To: Laszlo Ersek <lersek@redhat.com>,
	edk2-devel-01 <edk2-devel@lists.01.org>
Cc: Aleksei Kovura <alex3kov@zoho.com>,
	Ard Biesheuvel <ard.biesheuvel@linaro.org>,
	Dann Frazier <dannf@ubuntu.com>,
	"Dong, Eric" <eric.dong@intel.com>,
	"Zeng, Star" <star.zeng@intel.com>
Subject: Re: [PATCH] MdeModulePkg/AtaAtapiPassThru: disable only BM-DMA at ExitBootServices()
Date: Fri, 27 Oct 2017 03:23:18 +0000	[thread overview]
Message-ID: <0C09AFA07DD0434D9E2A0C6AEB0483103B9AEBA9@shsmsx102.ccr.corp.intel.com> (raw)
In-Reply-To: <20171026154819.20865-1-lersek@redhat.com>

Reviewed-by: Star Zeng <star.zeng@intel.com> after the minor typo " isse " is fixed to " issue ".

Thanks,
Star
-----Original Message-----
From: Laszlo Ersek [mailto:lersek@redhat.com] 
Sent: Thursday, October 26, 2017 11:48 PM
To: edk2-devel-01 <edk2-devel@lists.01.org>
Cc: Aleksei Kovura <alex3kov@zoho.com>; Ard Biesheuvel <ard.biesheuvel@linaro.org>; Dann Frazier <dannf@ubuntu.com>; Dong, Eric <eric.dong@intel.com>; Zeng, Star <star.zeng@intel.com>
Subject: [PATCH] MdeModulePkg/AtaAtapiPassThru: disable only BM-DMA at ExitBootServices()

Clearing I/O port decoding in the PCI command register at
ExitBootServices() breaks IDE boot in Windows, on QEMU's "pc" (i440fx) machine type. (AHCI boot on "q35" is unaffected.) Windows seems repeatedly stuck, apparently waiting for a timeout of sorts.

This is arguably a Windows bug; a native OS driver should not expect the firmware to leave the PCI command register in any particular state.

Strictly speaking, we only need to disable BM-DMA at ExitBootServices(), in order to abort pending transfers to/from RAM, which is soon to be owned by the OS. BM-DMA is also the only bit that's explicitly named by the UEFI Driver Writers' Guide, for clearing at ExitBootServices().

I've verified that clearing only BM-DMA fixes the isse (boot time) on i440fx, and does not regress q35/AHCI.

Cc: Aleksei Kovura <alex3kov@zoho.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Dann Frazier <dannf@ubuntu.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Reported-by: Aleksei Kovura <alex3kov@zoho.com>
Reported-by: Dann Frazier <dannf@ubuntu.com>
Reported-by: https://launchpad.net/~cjkrupp
Bisected-by: Dann Frazier <dannf@ubuntu.com>
Bisected-by: https://launchpad.net/~cjkrupp
Suggested-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Suggested-by: Star Zeng <star.zeng@intel.com>
Ref: https://bugs.launchpad.net/ubuntu/+source/edk2/+bug/1725560
Fixes: 6fb8ddd36bde45614b0a069528cdc97077835a74
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---

Notes:
    Repo:   https://github.com/lersek/edk2.git
    Branch: ata_disable_only_bmdma

 MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h | 3 +--  MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c | 5 ++---
 2 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h
index 8d6eac706c0b..92c5bf2001cd 100644
--- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h
+++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h
@@ -123,8 +123,7 @@ typedef struct {
   LIST_ENTRY                        NonBlockingTaskList;
 
   //
-  // For disabling the device (especially Bus Master DMA) at
-  // ExitBootServices().
+  // For disabling Bus Master DMA on the device at ExitBootServices().
   //
   EFI_EVENT                         ExitBootEvent;
 } ATA_ATAPI_PASS_THRU_INSTANCE;
diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c
index 09064dda18b7..e10e0d4e65f6 100644
--- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c
+++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c
@@ -480,8 +480,7 @@ InitializeAtaAtapiPassThru (  }
 
 /**
-  Disable the device (especially Bus Master DMA) when exiting the boot
-  services.
+  Disable Bus Master DMA on the device when exiting the boot services.
 
   @param[in] Event    Event for which this notification function is being
                       called.
@@ -506,7 +505,7 @@ AtaPassThruExitBootServices (
   PciIo->Attributes (
            PciIo,
            EfiPciIoAttributeOperationDisable,
-           Instance->EnabledPciAttributes,
+           Instance->EnabledPciAttributes & 
+ EFI_PCI_IO_ATTRIBUTE_BUS_MASTER,
            NULL
            );
 }
--
2.14.1.3.gb7cf6e02401b



  parent reply	other threads:[~2017-10-27  3:19 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-10-26 15:48 [PATCH] MdeModulePkg/AtaAtapiPassThru: disable only BM-DMA at ExitBootServices() Laszlo Ersek
2017-10-26 19:56 ` dann frazier
2017-10-27  3:23 ` Zeng, Star [this message]
2017-10-27 12:25   ` Laszlo Ersek
2017-10-27 14:57 ` Aleksei
2017-10-27 16:09 ` Laszlo Ersek
2017-11-22 10:05   ` Ni, Ruiyu
2017-11-22 10:26     ` Zeng, Star
2017-11-23  2:20       ` Ni, Ruiyu
2017-11-23 13:08         ` Laszlo Ersek
2017-11-23 14:58           ` Ni, Ruiyu
2017-11-23 17:19             ` Laszlo Ersek
2017-11-23 23:06               ` Ni, Ruiyu
2017-11-24  0:01           ` Paolo Bonzini
2017-11-24  1:04             ` Ni, Ruiyu
2017-11-24  1:40               ` Yao, Jiewen
2017-11-27 12:29                 ` Laszlo Ersek
2017-11-27 12:58                   ` Zeng, Star
2017-11-27 13:41                   ` Yao, Jiewen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=0C09AFA07DD0434D9E2A0C6AEB0483103B9AEBA9@shsmsx102.ccr.corp.intel.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox