On 04/09/20 07:26, Rebecca Cran wrote: > On 3/27/2020 1:01 PM, Laszlo Ersek wrote: >> >> I'm quite happy about this patch, but perhaps for an unexpected reason: >> namely, because it showcases how non-intuitive and unpredictable it can >> be to customize existent code for a new platform. > > Thanks! I was wondering if I should try and add new code into OvmfPkg, > or keep it separate. Also, good point about the commit message: I get > frustrated when people don't write proper/full messages, so I'm happy > you called me out on it. > > > The existing bhyve port removes everything related to QemuFwCfgLib, such > as calls to QemuFwCfgFindFile in PciHostBridgeLib.c. I'm not sure how I > should proceed given that there's so much commonality between the ovmf > and bhyve versions of the file: currently calls to QemuFwCfgFindFile > don't resolve since references are absent from the .dsc file, so I'm > wondering if I should re-add it, add a "#ifndef BHYVE" or similar to > avoid attempting to compile that code, or duplicate the file with that > code removed?
I'd recommend the following approaches for your consideration: (1) Yubo Miao is in the process of factoring out logic that is shared between ArmVirtPkg's and OvmfPkg's PciHostBridgeLib instances: 57fd0043-d63a-55b0-9c55-4ca079331885@redhat.com">http://mid.mail-archive.com/57fd0043-d63a-55b0-9c55-4ca079331885@redhat.com https://edk2.groups.io/g/devel/message/57062 I proposed that the new lib class be called PciHostBridgeUtilityLib. You could follow that development, with the intent of building BhyvePkg's PciHostBridgeLib instance in terms of the new PciHostBridgeUtilityLib. That would decrease code duplication in BhyvePkg. (2) If OvmfPkg's PciHostBridgeLib already does the right thing for BhyvePkg, assuming the QemuFwCfgFindFile ("etc/extra-pci-roots", &FwCfgItem, &FwCfgSize) call returns an error code, then you could introduce a Null instance of the QemuFwCfgLib class, under OvmfPkg/Library/QemuFwCfgLib. The new INF file could be called "QemuFwCfgLibNull.inf". There would be a new source file called "QemuFwCfgLibNull.c", with the following function definitions: - QemuFwCfgIsAvailable: return constant FALSE - QemuFwCfgSelectItem, QemuFwCfgReadBytes, QemuFwCfgWriteBytes, QemuFwCfgSkipBytes: do nothing - QemuFwCfgRead8, QemuFwCfgRead16, QemuFwCfgRead32, QemuFwCfgRead64: return 0 - QemuFwCfgFindFile: return RETURN_UNSUPPORTED Then you could resolve QemuFwCfgLib to this Null instance in the bhyve DSC file. (This approach might help you reuse further OvmfPkg modules in BhyvePkg. A new QemuFwCfgLibNull instance could simplify the existent "OvmfPkg/OvmfXen.dsc" platform too.) Thanks Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#57100): https://edk2.groups.io/g/devel/message/57100 Mute This Topic: https://groups.io/mt/72550105/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
