On 07/07/16 19:36, Jordan Justen wrote:
> On 2016-06-30 18:10:53, Laszlo Ersek wrote:
>> +EFI_STATUS
>> +EFIAPI
>> +DriverInitialize (
>> + IN EFI_HANDLE ImageHandle,
>> + IN EFI_SYSTEM_TABLE *SystemTable
>> + )
>> +{
>> + EFI_STATUS Status;
>> +
>> + if (!FeaturePcdGet (PcdPciBusHotplugDeviceSupport)) {
>> + DEBUG ((EFI_D_WARN,
>> + "%a: this driver should not have been built into the firmware\n",
>> + gEfiCallerBaseName));
>> + return EFI_UNSUPPORTED;
>> + }
>
> It looks like we'll always have this PCD as true, and always include
> the driver. I don't have a concern about that. (Do you know of a
> reason that we might need to make it conditional?)
No, I don't expect any changes to PcdPciBusHotplugDeviceSupport. It's
just a warning for the case if we happen to change the PCD nonetheless.
In that case though, the warning is not high priority enough to prevent
the firmware from proceeding. Hence no ASSERT() / CpuDeadLoop(). It's
just a message for the maintainers to clean up their DSC.
> What if we instead add this to PlatformDxe, and either drop, or turn
> the PcdPciBusHotplugDeviceSupport check into an ASSERT?
I'm perfectly fine with dropping the PcdPciBusHotplugDeviceSupport check
-- I was a bit torn about adding it in the first place, but I wanted to
solicit your feedback about it, and including the check seemed the best
way to do that :)
About including the functionality in PlatformDxe: I disagree with that
idea. I realize this is a (standardized) PCI Bus platform tweak, but we
already have IncompatiblePciDeviceSupportDxe separately, which is the
exact same kind of (standardized) PCI Bus platform tweak. (And that
driver in turn exists separately because the example driver
"MdeModulePkg/Bus/Pci/IncompatiblePciDeviceSupportDxe" exists separately
as well.)
More importantly, to me PlatformDxe is primarily a HII-centered driver.
I would not like to dilute that "HII focus" in it. I have had to look at
PlatformDxe several times in the past few months -- you surely recall
those iPXE HII debugging sessions on the list -- for refreshing my
(sadly not extensive enough) HII knowlege. Turning PlatformDxe into a
grab-bag of platform tweaks would harm the driver, in my opinion.
(I think about PlatformPei a bit differently -- it *is* a grab-bag of
platform tweaks, but its scope is much more focused. It does low-level
chipset, CPU, and memory setup.)
If you would like to avoid yet another (standardized) PCI Bus driver
tweak, I guess I could fuse this driver with
IncompatiblePciDeviceSupportDxe. If you agree with that: do you have a
good name for the unified driver?
Thanks
Laszlo
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel
