On 03/21/19 11:08, Ard Biesheuvel wrote: > On Thu, 21 Mar 2019 at 07:44, Wu, Hao A <hao.a...@intel.com> wrote: >> >>>>> >>>>> Just a couple of notes from my side - I'm sure Laszlo will have a much >>>>> longer list :-) >>>>> >>>>> - Dropping the floppy driver is fine with me. >>>>> - What is OVMF specific about this driver? Is it only the hardcoded >>>>> list of COM1/COM2/PS2 keyboard? If so, should we split this into a >>>>> driver and a library class, where the driver lives in MdeModulePkg, >>>>> and the library is implemented in the context of OVMF? >>>> >>>> Hello Ard, >>>> >>>> I think the special thing for this one is that: >>>> For QEMU, it does not have a Super I/O (SIO) chip. While, as far as I >>>> know, the SIO chip exists on other platforms. The driver proposed here >>>> simulates the behavior of an SIO chip. IMO, if we find more platforms that >>>> do not have a SIO chip, we can convert the driver into a general one. >>>> >>>> Also, for the implementation of the services in the Super I/O protocol, >>>> the proposed driver just does the minimal effort in order to support the >>>> serial/PS2 keyboard. >>> >>> Here's why I'd like the majority of this driver to live under >>> MdeModulePkg (for example through a lib class separation like Ard suggests): >>> >>> Because then its maintenance would not be the responsibility of OvmfPkg >>> maintainers. >>> >>> Consider, this driver is absolutely huge (1.5-2 kLOC), for doing "the >>> minimal effort in order to support the serial/PS2 keyboard". >>> >>> The risk of regressions is extreme (the PS/2 keyboard is the default >>> one, and if it breaks *subtly*, almost all users will be inconvenienced, >>> but not necessarily soon enough for us to get reports about it *early* >>> in the current development cycle). >>> >>> I realize that IntelFrameworkModulePkg/Bus/Isa/* drivers are frowned >>> upon nowadays, they may be ugly / platform specific / etc etc etc, but >>> they have also proved themselves to *work*, and (as far as I remember) >>> they have required practically zero fixes in order to function well on QEMU. >>> >>> It is very unwelcome by me to take on the maintenance burden for a >>> driver that is all of: >>> - not widely tested, >>> - replacing a proven set of drivers that is critical to users, >>> - large. >>> >>> I understand that Intel wants to stop maintaining >>> IntelFrameworkModulePkg/Bus/Isa/*, but the above price is too high for me. >>> >>> Compare the case if we simply moved the >>> IntelFrameworkModulePkg/Bus/Isa/* drivers under OvmfPkg: >>> - still large, >>> - but widely tested (with minimal churn in the past), >>> - and no risk of regressions. >>> >>> So in this form, I'm generally opposed to the switch. The two sets of >>> drivers need to coexist for a while, and we must expose the new drivers >>> to users while providing them with some sort of easy fallback. (I'd >>> prefer that fallback to be dynamically configurable, but, again, if your >>> keyboard breaks, how do you interact with e.g. the UEFI shell? So I >>> guess a static build flag would do as well.) I think the old drivers >> >> Hello Laszlo, >> >> I agree with your point. So your suggestion is to: >> >> 1. Duplicate the below drivers into OvmfPkg: >> PcAtChipsetPkg/IsaAcpiDxe/IsaAcpi.inf >> IntelFrameworkModulePkg/Bus/Isa/IsaBusDxe/IsaBusDxe.inf >> IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/IsaSerialDxe.inf >> IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2keyboardDxe.inf >> >> 2. Meanwhile, add the proposed SioBusDxe driver in the OvmfPkg as well >> >> 3. Add a static build flag within OvmfPkg to let users choose between: >> a) New OVMF SioBusDxe driver + ISA device drivers under >> MdeModulePkg/Bus/Isa; >> b) Legacy ISA stack copied from PcAtChipsetPkg & IntelFrameworkModulePkg >> >> Is my understanding correct?
Yes (but see below, at the end). >>> should be removed only in the edk2 stable tag that comes *after* the >>> next one, once we've given the drivers enough time to "prove themselves". >> >> Do you mean we should keep the copy of the legacy ISA stack from >> PcAtChipsetPkg & IntelFrameworkModulePkg until the announcement of >> edk2-stable201905 tag? Yes, exactly. People that adopt "edk2-stable201905" should be able to switch back to the old driver stack. NB: I certainly agree that the new code should be made the *default*. >> > > I think we should just keep the IntelFrameworkModulePkg components in > place until we are ready to stop using them in OVMF. Cloning them into > OvmfPkg now just so we can remove IntelFrameworkModulePkg in its > entirety has little added value IMO. I fully agree with this modification (it minimizes the churn), but I'm unsure how quickly Intel would like to rid themselves of IntelFrameworkModulePkg. If their deadline is edk2-stable201905, then that conflicts with my request above, and we might have no choice in moving the code to OvmfPkg, for the sake of one more stable tag. Thanks Laszlo _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
