On 06/25/15 10:44, Ni, Ruiyu wrote:
> Laszlo/Jordan,

> It seems to generalize the code is not very much possible, or at
> least not an easy task. How about we do the things in multiple steps?
> So it won't block your current work.

> 1. Duplicate the PciHostBridgeDxe in OvmfPkg and add these special
> OVMF logic.

Works for me.

> 2. Review the different versions of PciHostBridgeDxe (OVMF, Coreboot,
> DUET) and try to combine them.

I can promise it right now that I won't have capacity for this. Jordan,
will you? :)

> 3. Keep now and remove the PciHostBridgeDxe in future. Maybe some
> close-source platforms are using this driver.

Works for me.

Considering (1) and (3), I structured this v2 patch series exactly so
that I could accommodate such a proposal. In order to comply with (1)
and (3), patch v2 02/24 should be simply dropped.

Thanks
Laszlo


> 
> Thanks,
> Ray
> 
>> -----Original Message-----
>> From: Laszlo Ersek [mailto:ler...@redhat.com]
>> Sent: Thursday, June 25, 2015 4:24 PM
>> To: Ni, Ruiyu
>> Cc: Justen, Jordan L; edk2-devel@lists.sourceforge.net; Kinney, Michael D
>> Subject: Re: [edk2] [PATCH v2 02/24] PcAtChipsetPkg: remove
>> PciHostBridgeDxe
>>
>> On 06/25/15 04:10, 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?
>>
>> The specific behaviors of OVMF can be seen in this patch series
>> precisely. Please read:
>>
>> - the v1 blurb:
>>
>> http://thread.gmane.org/gmane.comp.emulators.qemu/342206/focus=153
>> 28
>>
>> - the v2 blurb:
>>
>> http://thread.gmane.org/gmane.comp.bios.tianocore.devel/15643/focus=1
>> 5646
>>
>> - and the commit messages of all the patches in v2.
>>
>> I'm getting pretty tired of having to wait for weeks to get just the
>> most basic feedback, "what are you trying to do", when that's obvious
>> from the patch series itself.
>>
>> In any case, very roughly:
>>
>> - The first half of the series only reformats and cleans up
>>   PciHostBridgeDxe. This part could be applied to the driver under
>>   PcAtChipsetPkg immediately. If you could find the time to review those
>>   patches, of course. As I pointed out earlier, these patches (on the
>>   list) are immediately reviewable for PcAtChipsetPkg too.
>>
>> - Other patches eliminate some *speculative* generality from the driver.
>>   The driver appears as if it could support multiple root bridges, but
>>   (a) that code is never actually put to use, (b) the code is static,
>>   not dynamic, (c) there is code that would be actually buggy if used
>>   with multiple root bridges.
>>
>> - The OVMF specific code implements a brute-force scan for extra root
>>   buses, and then uses a QEMU-specific optimization for shortening that
>>   brute force scan.
>>
>>> 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.
>>
>> The PCD that Jordan suggested was only for the very last step; ie.
>> shortening the brute force scan. The PCD would live in the
>> PcAtChipsetPkg token space, and would be set by platform specific code;
>> in OVMF's case, it would be set by separate QEMU-specific code.
>>
>> ... For what it's worth, based on qemu-devel discussions that occurred
>> since I had posted the OVMF v1 patch series, it seems that even the
>> brute force scan for the extra root buses is QEMU specific. So even that
>> patch might not be suitable for PcAtChipsetPkg.
>>
>> 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

Reply via email to