Re: [PATCH 03/10] MdeModulePkg/AtaAtapiPassThru: disable the device at ExitBootServices()

["Zeng, Star" <star.zeng@intel.com>]

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


Thanks,
Star
-----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?