Re: [PATCH] MdeModulePkg/PciBusDxe: Enable Bus Master on P2P bridges on demand

["Yao, Jiewen" <jiewen.yao@intel.com>]

Reviewed-by: Jiewen.yao@intel.com

> -----Original Message-----
> From: Ni, Ruiyu
> Sent: Friday, August 25, 2017 3:43 PM
> To: edk2-devel@lists.01.org
> Cc: Ruiyu Ni <ruiyu.ni@microsoft.com>; Sean Brogan
> <sean.brogan@microsoft.com>; Yao, Jiewen <jiewen.yao@intel.com>; Kinney,
> Michael D <michael.d.kinney@intel.com>
> Subject: [PATCH] MdeModulePkg/PciBusDxe: Enable Bus Master on P2P bridges
> on demand
> 
> From: Ruiyu Ni <ruiyu.ni@microsoft.com>
> 
> The patch dynamically enables Bus Master on P2P bridges only
> when requested by a device driver through PciIo.Attribute() to enable
> the Bus Master.
> 
> Signed-off-by: Sean Brogan <sean.brogan@microsoft.com>
> Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> ---
>  MdeModulePkg/Bus/Pci/PciBusDxe/PciDeviceSupport.c     | 16
> +++++++++++++---
>  MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c | 18
> +++++++++++++++---
>  MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c                |  8 ++++----
>  3 files changed, 32 insertions(+), 10 deletions(-)
> 
> diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciDeviceSupport.c
> b/MdeModulePkg/Bus/Pci/PciBusDxe/PciDeviceSupport.c
> index c0227fa2b6..359b9ded6d 100644
> --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciDeviceSupport.c
> +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciDeviceSupport.c
> @@ -1,7 +1,7 @@
>  /** @file
>    Supporting functions implementaion for PCI devices management.
> 
> -Copyright (c) 2006 - 2015, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
>  This program and the accompanying materials
>  are licensed and made available under the terms and conditions of the BSD
> License
>  which accompanies this distribution.  The full text of the license may be found
> at
> @@ -711,7 +711,12 @@ StartPciDevicesOnBridge (
>                               0,
>                               &Supports
>                               );
> -        Supports &= (UINT64)EFI_PCI_DEVICE_ENABLE;
> +        //
> +        // By default every bridge's IO and MMIO spaces are enabled.
> +        // Bridge's Bus Master will be enabled when any device behind it
> requests
> +        // to enable Bus Master.
> +        //
> +        Supports &= (UINT64) (EFI_PCI_IO_ATTRIBUTE_IO |
> EFI_PCI_IO_ATTRIBUTE_MEMORY);
>          PciIoDevice->PciIo.Attributes (
>                               &(PciIoDevice->PciIo),
>                               EfiPciIoAttributeOperationEnable,
> @@ -763,7 +768,12 @@ StartPciDevicesOnBridge (
>                               0,
>                               &Supports
>                               );
> -        Supports &= (UINT64)EFI_PCI_DEVICE_ENABLE;
> +        //
> +        // By default every bridge's IO and MMIO spaces are enabled.
> +        // Bridge's Bus Master will be enabled when any device behind it
> requests
> +        // to enable Bus Master.
> +        //
> +        Supports &= (UINT64) (EFI_PCI_IO_ATTRIBUTE_IO |
> EFI_PCI_IO_ATTRIBUTE_MEMORY);
>          PciIoDevice->PciIo.Attributes (
>                               &(PciIoDevice->PciIo),
>                               EfiPciIoAttributeOperationEnable,
> diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c
> b/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c
> index 81171c82d9..f73756a31e 100644
> --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c
> +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c
> @@ -1218,11 +1218,12 @@ DetermineDeviceAttribute (
>        return Status;
>      }
>      //
> -    // Assume the PCI Root Bridge supports DAC
> +    // Assume the PCI Root Bridge supports DAC and Bus Master.
>      //
>      PciIoDevice->Supports |=
> (UINT64)(EFI_PCI_IO_ATTRIBUTE_EMBEDDED_DEVICE |
>                                EFI_PCI_IO_ATTRIBUTE_EMBEDDED_ROM |
> -
> EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE);
> +
> EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE |
> +                              EFI_PCI_IO_ATTRIBUTE_BUS_MASTER);
> 
>    } else {
> 
> @@ -1233,9 +1234,16 @@ DetermineDeviceAttribute (
>      //
>      Command = EFI_PCI_COMMAND_IO_SPACE     |
>                EFI_PCI_COMMAND_MEMORY_SPACE |
> -              EFI_PCI_COMMAND_BUS_MASTER   |
>                EFI_PCI_COMMAND_VGA_PALETTE_SNOOP;
> 
> +    //
> +    // Per PCI-to-PCI Bridge Architecture all PCI-to-PCI bridges are Bus Master
> capable.
> +    // So only test the Bus Master capability for PCI devices.
> +    //
> +    if (!IS_PCI_BRIDGE(&PciIoDevice->Pci)) {
> +      Command |= EFI_PCI_COMMAND_BUS_MASTER;
> +    }
> +
>      BridgeControl = EFI_PCI_BRIDGE_CONTROL_ISA |
> EFI_PCI_BRIDGE_CONTROL_VGA | EFI_PCI_BRIDGE_CONTROL_VGA_16;
> 
>      //
> @@ -1245,7 +1253,11 @@ DetermineDeviceAttribute (
> 
>      //
>      // Set the supported attributes for specified PCI device
> +    // Per PCI-to-PCI Bridge Architecture all PCI-to-PCI bridges are Bus Master
> capable.
>      //
> +    if (IS_PCI_BRIDGE(&PciIoDevice->Pci)) {
> +      Command |= EFI_PCI_COMMAND_BUS_MASTER;
> +    }
>      PciSetDeviceAttribute (PciIoDevice, Command, BridgeControl,
> EFI_SET_SUPPORTS);
> 
>      //
> diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c
> b/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c
> index cc7125e4fc..659f480d71 100644
> --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c
> +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c
> @@ -1348,7 +1348,8 @@ ModifyRootBridgeAttributes (
>    //
>    Attributes &= ~(UINT64)(EFI_PCI_IO_ATTRIBUTE_EMBEDDED_DEVICE |
>                            EFI_PCI_IO_ATTRIBUTE_EMBEDDED_ROM |
> -                          EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE);
> +                          EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE |
> +                          EFI_PCI_IO_ATTRIBUTE_BUS_MASTER);
> 
>    //
>    // Record the new attribute of the Root Bridge
> @@ -1726,12 +1727,11 @@ PciIoAttributes (
>    }
>    //
>    // The upstream bridge should be also set to revelant attribute
> -  // expect for IO, Mem and BusMaster
> +  // expect for IO and Mem
>    //
>    UpStreamAttributes = Attributes &
>                         (~(EFI_PCI_IO_ATTRIBUTE_IO     |
> -                          EFI_PCI_IO_ATTRIBUTE_MEMORY |
> -                          EFI_PCI_IO_ATTRIBUTE_BUS_MASTER
> +                          EFI_PCI_IO_ATTRIBUTE_MEMORY
>                            )
>                          );
>    UpStreamBridge = PciIoDevice->Parent;
> --
> 2.12.2.windows.2