Re: [Xen-devel] [edk2] [PATCH RFC v2 1/7] MdeModulePkg: introduce PcdPciAllowFullEnumeration

["Kinney, Michael D" <michael.d.kinney@xxxxxxxxx>]

Jordan,

I agree that removing code duplication is a good idea.

I believe we can make the one in the MdeModulePkg functional everywhere.  Size 
will be the only remaining difference.  This can be addressed longer term.  If 
the proposed PCD is configured as FixedAtBuild in a DSC file, we should be able 
to get the optimized code generation for the PciBusDxe to remove all the 
code/data that is only used for PCI Enumeration.  Then the PciBusNoEnumeration 
could be retired.

I have no issues with adding a PCD to make the PciBusDxe in the MdeModulePkg 
skip enumeration all together.

Mike

-----Original Message-----
From: Jordan Justen [mailto:jljusten@xxxxxxxxx] 
Sent: Sunday, November 24, 2013 6:27 PM
To: edk2-devel@xxxxxxxxxxxxxxxxxxxxx
Cc: Tian, Feng; Wei Liu; Kinney, Michael D; xen-devel
Subject: Re: [edk2] [PATCH RFC v2 1/7] MdeModulePkg: introduce 
PcdPciAllowFullEnumeration

On Sun, Nov 24, 2013 at 5:47 PM, Kinney, Michael D
<michael.d.kinney@xxxxxxxxx> wrote:
> Jordan,
>
> There is an alternate module already available.
>
>         DuetPkg/PciBusNoEnumeration
>
> The concept was to have a different implementation that was smaller and just 
> produced PCI I/O handles for PCI devices that are already enumerated.

This was already discussed (and tried). (See Xen emails from Wei.)

I'd really prefer to continue to use the MdeModulePkg version for
OVMF, and avoid extra Xen specific build flags which we haven't needed
previously.

The PCD seems very unobtrusive for the driver. (That surprised me.) It
should not even cause different code to be generated when a fixed-PCD
is used.

Would a setting of gFullEnumeration = FALSE be enough to allow DuetPkg
to use the MdeModulePkg module as well? Wei's work seems to give some
signs that the answer would be yes. Wouldn't we like to avoid code
duplication for such a complex module?

-Jordan

> -----Original Message-----
> From: Jordan Justen [mailto:jljusten@xxxxxxxxx]
> Sent: Sunday, November 24, 2013 2:05 PM
> To: edk2-devel@xxxxxxxxxxxxxxxxxxxxx; Tian, Feng; Wei Liu
> Cc: xen-devel
> Subject: Re: [edk2] [PATCH RFC v2 1/7] MdeModulePkg: introduce 
> PcdPciAllowFullEnumeration
>
> Feng, what do you think of this change to MdeModulePkg?
>
> Wei, How about PcdPciDisableBusEnumeration instead?
>
> -Jordan
>
> On Tue, Nov 19, 2013 at 12:38 PM, Wei Liu <wei.liu2@xxxxxxxxxx> wrote:
>> Platforms such as Xen already enumerates PCI bridges and devices. Use
>> this PCD to control EDK2 behavior.
>>
>> The default behavior is to allow full PCI enumeration. This is the same
>> behavior as before this change.
>>
>> Signed-off-by: Wei Liu <wei.liu2@xxxxxxxxxx>
>> Cc: Gerd Hoffmann <kraxel@xxxxxxxxxx>
>> Cc: Laszlo Ersek <lersek@xxxxxxxxxx>
>> ---
>>  MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c      |    5 ++++-
>>  MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf |    1 +
>>  MdeModulePkg/MdeModulePkg.dec                |    3 +++
>>  3 files changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c 
>> b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c
>> index 5afbb82..49c204c 100644
>> --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c
>> +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c
>> @@ -284,7 +284,10 @@ PciBusDriverBindingStart (
>>            );
>>    }
>>
>> -  gFullEnumeration = (BOOLEAN) ((SearchHostBridgeHandle (Controller) ? 
>> FALSE : TRUE));
>> +  if (PcdGetBool (PcdPciAllowFullEnumeration))
>> +    gFullEnumeration = (BOOLEAN) ((SearchHostBridgeHandle (Controller) ? 
>> FALSE : TRUE));
>> +  else
>> +    gFullEnumeration = FALSE;
>>
>>    //
>>    // Open Device Path Protocol for PCI root bridge
>> diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf 
>> b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
>> index 34eb672..626ae99 100644
>> --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
>> +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
>> @@ -108,6 +108,7 @@
>>    gEfiMdeModulePkgTokenSpaceGuid.PcdSrIovSupport
>>    gEfiMdeModulePkgTokenSpaceGuid.PcdAriSupport
>>    gEfiMdeModulePkgTokenSpaceGuid.PcdMrIovSupport
>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdPciAllowFullEnumeration
>>
>>  # [Event]
>>  #   ##
>> diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
>> index b627eb1..274d2e5 100644
>> --- a/MdeModulePkg/MdeModulePkg.dec
>> +++ b/MdeModulePkg/MdeModulePkg.dec
>> @@ -737,6 +737,9 @@
>>    ## This PCD specifies whether the Multi Root I/O virtualization support.
>>    gEfiMdeModulePkgTokenSpaceGuid.PcdMrIovSupport|FALSE|BOOLEAN|0x10000046
>>
>> +  ## This PCD specifies whether full PCI enumeration is allowed.
>> +  
>> gEfiMdeModulePkgTokenSpaceGuid.PcdPciAllowFullEnumeration|TRUE|BOOLEAN|0x10000048
>> +
>>  [PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx]
>>    ## Single root I/O virtualization virtual function memory BAR alignment
>>    #  BITN set indicates 2 of n+12 power
>> --
>> 1.7.10.4
>>
>>
>> ------------------------------------------------------------------------------
>> Shape the Mobile Experience: Free Subscription
>> Software experts and developers: Be at the forefront of tech innovation.
>> Intel(R) Software Adrenaline delivers strategic insight and game-changing
>> conversations that shape the rapidly evolving mobile landscape. Sign up now.
>> http://pubads.g.doubleclick.net/gampad/clk?id=63431311&iu=/4140/ostg.clktrk
>> _______________________________________________
>> edk2-devel mailing list
>> edk2-devel@xxxxxxxxxxxxxxxxxxxxx
>> https://lists.sourceforge.net/lists/listinfo/edk2-devel
>
> ------------------------------------------------------------------------------
> Shape the Mobile Experience: Free Subscription
> Software experts and developers: Be at the forefront of tech innovation.
> Intel(R) Software Adrenaline delivers strategic insight and game-changing
> conversations that shape the rapidly evolving mobile landscape. Sign up now.
> http://pubads.g.doubleclick.net/gampad/clk?id=63431311&iu=/4140/ostg.clktrk
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.sourceforge.net/lists/listinfo/edk2-devel
>
> ------------------------------------------------------------------------------
> Shape the Mobile Experience: Free Subscription
> Software experts and developers: Be at the forefront of tech innovation.
> Intel(R) Software Adrenaline delivers strategic insight and game-changing
> conversations that shape the rapidly evolving mobile landscape. Sign up now.
> http://pubads.g.doubleclick.net/gampad/clk?id=63431311&iu=/4140/ostg.clktrk
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.sourceforge.net/lists/listinfo/edk2-devel

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel