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