On 03/07/16 04:51, Jordan Justen wrote: > On 2016-03-06 06:28:26, Laszlo Ersek wrote:
[snip] >> So here's what I would like, in decreasing order of preference: >> >> * Keep the patch, as is. (Most preferred.) >> >> * Keep the library instance under OvmfPkg, but set MODULE_TYPE to >> DXE_DRIVER, and/or replace the "I440FxQ35" suffix with "PcieCf8". >> >> * Turn the library instance into a core module. Don't restrict it to >> DXE and later. Consume a new core PCD, introducing obscure ordering >> requirements that are not enforced at build time. >> >> Set this new PCD explicitly in OVMF's PlatformPei, near the current >> >> PcdSet16 (PcdOvmfHostBridgePciDevId, mHostBridgeDevId) >> >> call (where we capture the platform type). >> > > I don't see this as much different than depending on the > PcdOvmfHostBridgePciDevId. > > I prefer this, or the next option, but it is probably better to push > out the SKU thing until later. So, I guess I'd prefer this option for > now. If Mike doesn't think the library makes sense for MdePkg, then > I'd still like to define it generically enough that it *could* live in > MdePkg, even if it ends up in OVMF. (I seem to waste a lot of time on > such pointless things. For example, > OvmfPkg/Include/Library/PlatformFvbLib.h) > > I guess I'm fine with the library as currently defined. So, if you > would prefer that I make these changes, then I guess we can move > forward with what you currently have. I am not theoretically opposed to option #3 (I *am* opposed to option #4 below -- the SKU thing). So, if you were willing to accept this patch as-is (option #1), and were willing to refactor it for option #3 later, I would *greatly* appreciate it. (If the library already existed as described in option #3, I would simply use it without a second thought.) It's just my experience that I usually need two or three turns at this kind of refactoring until I can satisfy your taste with it. Functionally this library instance is trivial, so I'd prefer to move forward, and gladly leave the "upstreaming into core" to you, if that works for you. Thank you Laszlo > -Jordan > >> * Turn the library instance into a core module. Don't restrict it to >> DXE and later. Consume a new core PCD, introducing obscure ordering >> requirements that are not enforced at build time. >> >> Set SKU-dependent DynamicDefaults for this PCD (and maybe other OVMF >> PCDs as well) in OVMF's DSC files. Call LibPcdSetSku() in >> PlatformPei, instead of the current call to >> >> PcdSet16 (PcdOvmfHostBridgePciDevId, mHostBridgeDevId). >> >> (Least preferred.) [snip] _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
