On 06/28/15 01:36, Jordan Justen wrote:
> On 2015-06-25 21:26:20, Ni, Ruiyu wrote:
>> Jordan,
>> Simply using PCD to split the code is very straightforward. Another
>> approach is to introduce a new library class with carefully defined
>> library APIs so that different platforms can use one common driver
>> plus different instances of libraries.
>> After all, abstracting is never a easy work.
>> So let's firstly make OVMF work by duplicating a PciHostBridgeDxe.
>
> Is this the preferred method of making progress then? Duplicate code
> first, then figure out how to make it more general?
Duplicate and customize the code first, yes, then identify and extract
the common parts as a library. Otherwise, if you try to define the
interface in advance, that's just going to be speculative generality,
and if a new client doesn't find it a good match, then the interface
will have to be modified again.
This library would not be a utility library, with a generic interface
and several possible implementations. Its sole purpose would be code
de-duplication on the source code level.
> I don't see
> drivers duplicated all that often,
Probably because all that happens in forks that are proprietary.
Actual platform trees duplicate entire *packags*, not just libraries and
drivers. I have seen that in at least three examples:
(1) APM's XGene platform,
(2) in edk2 under "Vlv2TbltDevicePkg/Override",
(3 "Quark_EDKII_v1.1.0.tar.gz" in
"Board_Support_Package_Sources_for_Intel_Quark_v1.1.0.7z",
duplicates all of the following (not an exhaustive list):
BaseLib
DxeImageVerificationLib
MtrrLib
TpmCommLib
UefiBootManagerLib
S3Resume2Pei
BdsDxe
BootScriptExecutorDxe
EhciDxe
FaultTolerantWriteDxe
PcatRealTimeClockRuntimeDxe
PciHostBridge <----- !!!
SmmLockBox
TcgDxe
UsbBusDxe
VariableAuthenticated/RuntimeDxe
> so it seems a little strange.
Well, to me it seems unfair that my open source and *upstream first*
contributions are blocked on hoops that (a) proprietary or independent
forks do not have to jump through, (b) Intel's own (open source or
otherwise) code bases avoid oh so conveniently.
> I haven't heard you say the PCD idea can't work, so can you spend a
> little more time considering it to see if it seems reasonable to you?
> If you are still concerned about the PCDs, then fine we'll just
> duplicate the code.
Again, I'm fine with whatever resolution the two of you agree upon.
> And, Laszlo, is this really that urgent that you'd rather duplicate a
> driver under the platform rather than allowing for some discussion on
> a possible common solution?
When I posted the series first, on June 6th, it had not been "that
urgent". Until next Friday, it will have been almost a month. I think
that deciding how to avoid code duplication ("some discussion") should
be more than feasible over the course of a month. So yes, *now* it is
that urgent.
There's one real cost that you are not considering, and that's the
number of things (patch series) that I have to juggle in parallel.
Just considering these:
- the SMM patchset,
- the >=64GB guest support patchset,
- the multiple extra root buses support patchset,
- and the EndOfDxe patchset
have various inter-dependencies. The longer I have to work on them (and
on more of them) in parallel, the more work I waste, because ultimately
I will have to rebase those that go in later, and those rebases can be
quite intrusive actually. For example, the SMM and >=64GB guest patch
series are going to have conflicts, and the extra root buses series and
the EndOfDxe set may have conflicts too (in PlatformBdsLib).
I cannot just stop and put aside my OVMF work until you guys decide, in
a month, how to avoid code duplication. I will unavoidably work on other
OVMF bugs or features meanwhile, and all this delay creates inter-series
conflicts and wasted work for me.
Thanks
Laszlo
>
> -Jordan
>
>> What do you think?
>>
>> Laszlo,
>> I don't want to block you. And I do not see a big conflict between
>> you and Jordan. Thank you very much for your continuously code
>> contribution.
>>
>> Thanks,
>> Ray
>>
>>> -----Original Message-----
>>> From: Laszlo Ersek [mailto:ler...@redhat.com]
>>> Sent: Friday, June 26, 2015 3:07 AM
>>> To: Justen, Jordan L; Ni, Ruiyu
>>> Cc: edk2-devel@lists.sourceforge.net; Kinney, Michael D
>>> Subject: Re: [edk2] [PATCH v2 02/24] PcAtChipsetPkg: remove
>>> PciHostBridgeDxe
>>>
>>> Jordan, Ray,
>>>
>>> On 06/25/15 19:14, Jordan Justen wrote:
>>>> On 2015-06-24 19:10:01, Ni, Ruiyu wrote:
>>>>> Jordan,
>>>>> I prefer to share the code across multiple platforms as well, if
>>>>> that's possible.
>>>>>
>>>>> In real world, DUET's PciHostBridgeDxe driver does something
>>>>> additionally: it gathers all option roms from PCI devices and
>>>>> transfers them to its own special PciBus driver to dispatch. I am
>>>>> not familiar to OVMF and CorebootPayloadPkg's PciHostBridgeDxe
>>>>> drivers. What specific behaviors do they do?
>>>>>
>>>>> Can we generalize all the special behaviors to a common driver? I do
>>>>> NOT like to introduce a bunch of PCDs like PcdDuet, PcdOvmf and
>>>>> PcdCoreboot.
>>>>
>>>> I think the names would not need to include the platform names.
>>>>
>>>> For this case, it seems like 1 or 2 PCDs would be sufficient:
>>>> * PcdScanForAdditionalPciRootBuses (boolean / feature PCD)
>>>> * PcdAdditionalRootBusesMaxBusNumber
>>>>
>>>> We could also only have 1 PCD (PcdAdditionalRootBusesMaxBusNumber)
>>> and
>>>> set it to 0 to disable the feature.
>>>
>>> here's my respectful request for the two of you:
>>>
>>> Please work out an agreement between the two of you, by the end of next
>>> week, Friday, July 3rd, 2015, End-of-Business, in Jordan's timezone.
>>>
>>> (Reminder: the first version of the series (with practically identical
>>> PciHostBridgeDxe impact) has been posted on June 6th, 19 days ago.)
>>>
>>> I have already agreed to both of your designs, but I can't satisfy both
>>> at once, because your requirements conflict with each other.
>>>
>>> I'm (obviously) willing to implement what Ray suggests.
>>>
>>> I'm also willing to implement Jordan's suggestion, but then I will need
>>> some form of *commitment* from Ray in advance. Namely, I said earlier,
>>> and I'm saying again, that the *overwhelming majority* of the series
>>> applies immediately to the driver in PcAtChipsetPkg, therefore Ray can
>>> review those patches right now, on a higher level, and express if he
>>> agrees with those patches ending up under PcAtChipsetPkg.
>>>
>>> I will be on PTO next week. If you can reach an agreement until next
>>> Friday, I will do my best after, to implement that shared design of yours.
>>>
>>> If the two of you can't reach an agreement until next Friday, I will
>>> abandon this series publicly, and we will carry it downstream only.
>>>
>>> Thanks
>>> Laszlo
------------------------------------------------------------------------------
Monitor 25 network devices or servers for free with OpManager!
OpManager is web-based network management software that monitors
network devices and physical & virtual servers, alerts via email & sms
for fault. Monitor 25 devices for free with no restriction. Download now
http://ad.doubleclick.net/ddm/clk/292181274;119417398;o
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel
