public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Laszlo Ersek <lersek@redhat.com>
To: "Zeng, Star" <star.zeng@intel.com>,
	Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Brijesh Singh <brijesh.singh@amd.com>,
	edk2-devel-01 <edk2-devel@lists.01.org>,
	"Yao, Jiewen" <jiewen.yao@intel.com>,
	"Dong, Eric" <eric.dong@intel.com>,
	Igor Mammedov <imammedo@redhat.com>
Subject: Re: [PATCH 03/10] MdeModulePkg/AtaAtapiPassThru: disable the device at ExitBootServices()
Date: Thu, 26 Oct 2017 16:09:57 +0200	[thread overview]
Message-ID: <bb1adc9c-c724-74f5-2aec-0dcadf70e5f4@redhat.com> (raw)
In-Reply-To: <0C09AFA07DD0434D9E2A0C6AEB0483103B9AE2CE@shsmsx102.ccr.corp.intel.com>

Ard, Star,

(CC Igor)

On 10/26/17 07:08, Zeng, Star wrote:
> Good point.
> 
> Could we find out what change causes the performance regression? Bus Master disable / Memory Space disable / IO Space disable?
> How about to only disable Bus Master in the exit boot service event notification? It seems the key point suggested by UEFI Driver_Writer_Guide_V1.0.1_120308.pdf.
> 
> 7.7
> Examples from the EDK II that use this feature are the PCI device drivers for USB Host
> Controllers. Some USB Host Controllers are PCI Bus Masters that continuously access a
> memory buffer to poll for operation requests. Access to this memory buffer by a USB
> Host Controller may be required to boot an operation system, but this activity must be
> terminated when the OS calls ExitBootServices(). *The typical action in the Exit Boot
> Services Event for these types of drivers is to disable the PCI bus master* and place the
> USB Host Controller into a halted state

thank you for the ideas.

* Disabling only EFI_PCI_IO_ATTRIBUTE_BUS_MASTER at EBS mitigates the
  symptom.

* Disabling (EFI_PCI_IO_ATTRIBUTE_BUS_MASTER | EFI_PCI_IO_ATTRIBUTE_IO)
  at EBS preserves the symptom.

* Disabling
  (EFI_PCI_IO_ATTRIBUTE_BUS_MASTER | EFI_PCI_IO_ATTRIBUTE_MEMORY) at EBS
  also mitigates the symptom.

So it is as Ard suspected, disabling IO port decoding is what tickles
the bug in Windows.

(Now I'm vaguely recalling an earlier discussion from qemu-devel that
Windows has a bug in that, if any given PCI device is disabled at boot,
then Windows will not load drivers for it, or some such. I'm struggling
to recall the context; maybe it was related to ACPI generation in QEMU.
I'm CC'ing Igor; maybe he remembers better.)

I will post a patch, for disabling EFI_PCI_IO_ATTRIBUTE_BUS_MASTER only.
First, that's going to follow the driver writers' guide verbatim;
second, disabling BMDMA and MMIO, but not IO, would look weird in the
code. :/

Thank you both for the help!
Laszlo


> -----Original Message-----
> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] 
> Sent: Thursday, October 26, 2017 4:12 AM
> To: Laszlo Ersek <lersek@redhat.com>
> Cc: Dong, Eric <eric.dong@intel.com>; Zeng, Star <star.zeng@intel.com>; edk2-devel-01 <edk2-devel@lists.01.org>; Yao, Jiewen <jiewen.yao@intel.com>; Brijesh Singh <brijesh.singh@amd.com>
> Subject: Re: [edk2] [PATCH 03/10] MdeModulePkg/AtaAtapiPassThru: disable the device at ExitBootServices()
> 
> On 25 October 2017 at 16:26, Laszlo Ersek <lersek@redhat.com> wrote:
>> Hi Star, Eric,
>>
>> On 09/08/17 00:41, Laszlo Ersek wrote:
>>> The AtaAtapiPassThru driver maps three system memory regions for Bus 
>>> Master Common Buffer operation on the following call path, if the 
>>> controller has PCI_CLASS_MASS_STORAGE_SATADPA class code:
>>>
>>>   AtaAtapiPassThruStart()
>>>     EnumerateAttachedDevice()
>>>       AhciModeInitialization()
>>>         AhciCreateTransferDescriptor()
>>>
>>> The device is disabled (including Bus Master DMA) when the controller 
>>> is unbound, in AtaAtapiPassThruStop(). Then the regions are unmapped.
>>>
>>> The former step should also be done when we exit the boot services, 
>>> and the OS gains ownership of system memory.
>>>
>>> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>> Cc: Brijesh Singh <brijesh.singh@amd.com>
>>> Cc: Eric Dong <eric.dong@intel.com>
>>> Cc: Jiewen Yao <jiewen.yao@intel.com>
>>> Cc: Star Zeng <star.zeng@intel.com>
>>> Contributed-under: TianoCore Contribution Agreement 1.1
>>> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
>>> ---
>>>  MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h |  6 ++  
>>> MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c | 59 
>>> +++++++++++++++++++-
>>>  2 files changed, 64 insertions(+), 1 deletion(-)
>>
>> this patch -- that is, commit 6fb8ddd36bde
>> ("MdeModulePkg/AtaAtapiPassThru: disable the device at 
>> ExitBootServices()", 2017-09-03) -- has caused a performance 
>> regression in OVMF, in booting Windows installer ISOs from emulated IDE CD-ROMs.
>>
>> Interestingly, the performance regression only affects the "traditional"
>> IDE controller of the "pc" (i440fx) machine type of QEMU; it does not 
>> affect the AHCI/SATA controller of the "q35" machine type.
> 
> Does it make any difference if you only disable memory decoding and bus mastering, but leave I/O port decoding enabled?
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
> 



  reply	other threads:[~2017-10-26 14:06 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-09-07 22:41 [PATCH 00/10] MdeModulePkg, OvmfPkg: unmap DMA buffers at ExitBootServices Laszlo Ersek
2017-09-07 22:41 ` [PATCH 01/10] MdeModulePkg/AtaAtapiPassThru: cache EnabledPciAttributes Laszlo Ersek
2017-09-07 22:41 ` [PATCH 02/10] MdeModulePkg/AtaAtapiPassThru: unmap DMA buffers after disabling BM DMA Laszlo Ersek
2017-09-07 22:41 ` [PATCH 03/10] MdeModulePkg/AtaAtapiPassThru: disable the device at ExitBootServices() Laszlo Ersek
2017-10-25 15:26   ` Laszlo Ersek
2017-10-25 20:11     ` Ard Biesheuvel
2017-10-26  5:08       ` Zeng, Star
2017-10-26 14:09         ` Laszlo Ersek [this message]
2017-10-26 14:12           ` Ard Biesheuvel
2017-09-07 22:41 ` [PATCH 04/10] OvmfPkg/VirtioBlkDxe: don't unmap VRING " Laszlo Ersek
2017-09-07 22:41 ` [PATCH 05/10] OvmfPkg/VirtioGpuDxe: don't unmap VRING & BackingStore at ExitBootServices Laszlo Ersek
2017-09-07 22:41 ` [PATCH 06/10] OvmfPkg/VirtioRngDxe: don't unmap VRING at ExitBootServices() Laszlo Ersek
2017-09-07 22:41 ` [PATCH 07/10] OvmfPkg/VirtioScsiDxe: " Laszlo Ersek
2017-09-07 22:41 ` [PATCH 08/10] OvmfPkg/IoMmuDxe: track all mappings Laszlo Ersek
2017-09-07 22:41 ` [PATCH 09/10] OvmfPkg/IoMmuDxe: generalize IoMmuUnmap() to IoMmuUnmapWorker() Laszlo Ersek
2017-09-07 22:41 ` [PATCH 10/10] OvmfPkg/IoMmuDxe: unmap all IOMMU mappings at ExitBootServices() Laszlo Ersek
2017-09-08  7:28 ` [PATCH 00/10] MdeModulePkg, OvmfPkg: unmap DMA buffers at ExitBootServices Yao, Jiewen
2017-09-08  8:28   ` Yao, Jiewen
2017-09-08  7:29 ` Zeng, Star
2017-09-08  8:53 ` Ard Biesheuvel
2017-09-08  9:48   ` Laszlo Ersek
2017-09-08 11:25     ` Ard Biesheuvel
2017-09-08 15:50 ` Brijesh Singh
2017-09-08 16:00   ` Laszlo Ersek
2017-09-08 18:28 ` Laszlo Ersek

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=bb1adc9c-c724-74f5-2aec-0dcadf70e5f4@redhat.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