From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2607:f8b0:4864:20::129; helo=mail-it1-x129.google.com; envelope-from=ard.biesheuvel@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-it1-x129.google.com (mail-it1-x129.google.com [IPv6:2607:f8b0:4864:20::129]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 015E9211E744D for ; Fri, 22 Mar 2019 02:41:17 -0700 (PDT) Received: by mail-it1-x129.google.com with SMTP id y63so2413332itb.5 for ; Fri, 22 Mar 2019 02:41:17 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=N+dLB/dxSyz+FWr1RprxR37Chw8T+0FfUKCiCr6cKbY=; b=ArJ1DexD60iPybDBq6NAQQ5EyI7+TwHrGCeNWwSYb6u0oGgdxhPYtgcZ5oAQ/zmGgZ IljkUZ2QJZAP8d/YA9e/I4FVcmBytkuy2jCgCTEy7FyVbJaFjcP2rZj25Dch7y9pa9On ueEK60oHLtZdj7ROpa2rXjYDHYLQ4Bj/R1orirbVSyzP5PRC2nl9CUds8LNR4z/LKcqg ZB+crIShbzUNDIOmifaSEW4C1QilFtOFed8/frv7aa8TysFJqbU2oEs4S40tY740TQ28 JvP/k0+AyVP2cODAhUhU9AVGRtq5AExSrwS71MP9Thn7PkcyFyboU4XKwwbNXfMQ0lJG oRNQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=N+dLB/dxSyz+FWr1RprxR37Chw8T+0FfUKCiCr6cKbY=; b=qzrFoU9u+rSyYghLkvA73yUsGVyZ3MDdUwmrCygKp8/jeMxr01SOwa7n0iU9/Bl1Qz ClEV3oZ1DwyooWeTP6QGX0svS6WF5MDc8bkecPLPmMPXobxNGpsaAVfNaeWq54p/fJ7I wTFadRynpETZG68DZfzsdNtQLBSfg1Nia9Kxqc1+mMa8fz6g9sjeT6JHhSlR/T22UKBb DBFUb+Y/rUcCo+S25GRI0ISqfyS8wpuRM6zDgIJJ9r4bTbCQvmcieX8q0BLFQTSRTzAd RiR0XqMoBk3CDqwOkwgq+JuU+L6tILHAM3EZwKahNZKaJkEVG6KdTKqZgVGJWxxtGdVf fLSw== X-Gm-Message-State: APjAAAXOO+pmuLXEIsh4RYbV1BvWQm9G+ZYSOvrfQ1jhrTClKA7mTqdp 3atL6//lRCj4TvGYahe4zKp5JqDqFh8Fre+1KnXlAg== X-Google-Smtp-Source: APXvYqxXbgfpsGjkah9aJaBaVIja4sEThZutl4SXwjB67Q5Yym7W4cVM4L+AvLRsQjt+RQiIjjoMBWHW+Li++OIgk3A= X-Received: by 2002:a24:11ce:: with SMTP id 197mr510625itf.121.1553247676728; Fri, 22 Mar 2019 02:41:16 -0700 (PDT) MIME-Version: 1.0 References: <20190315071603.16936-1-hao.a.wu@intel.com> <2cef7754-a5ab-274e-44ab-14ba092f7d40@redhat.com> <8aecf13a-97d9-357d-b343-094f95f36f86@redhat.com> <746e3c63-6db6-ac1b-7b46-32e1aba99768@redhat.com> In-Reply-To: <746e3c63-6db6-ac1b-7b46-32e1aba99768@redhat.com> From: Ard Biesheuvel Date: Fri, 22 Mar 2019 10:41:04 +0100 Message-ID: To: Laszlo Ersek Cc: "Wu, Hao A" , "edk2-devel@lists.01.org" , "Justen, Jordan L" , "Ni, Ray" Subject: Re: [PATCH v1 0/2] Ovmf: Stop using ISA drivers within IntelFrameworkModulePkg X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 22 Mar 2019 09:41:18 -0000 Content-Type: text/plain; charset="UTF-8" On Fri, 22 Mar 2019 at 10:25, Laszlo Ersek wrote: > > On 03/22/19 02:33, Wu, Hao A wrote: > >> -----Original Message----- > >> From: Laszlo Ersek [mailto:lersek@redhat.com] > >> Sent: Friday, March 22, 2019 3:04 AM > >> To: Ard Biesheuvel; Wu, Hao A > >> Cc: edk2-devel@lists.01.org; Justen, Jordan L; Ni, Ray > >> Subject: Re: [PATCH v1 0/2] Ovmf: Stop using ISA drivers within > >> IntelFrameworkModulePkg > >> > >> On 03/21/19 11:08, Ard Biesheuvel wrote: > >>> On Thu, 21 Mar 2019 at 07:44, Wu, Hao A 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. > > > > Hello Laszlo and Ard, > > > > How about the below approach: > > > > 1. Keep the current ISA stack in Ovmf: > > PcAtChipsetPkg/IsaAcpiDxe/IsaAcpi.inf > > IntelFrameworkModulePkg/Bus/Isa/IsaBusDxe/IsaBusDxe.inf > > IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/IsaSerialDxe.inf > > IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2keyboardDxe.inf > > > > 2. Add the proposed SioBusDxe driver in the OvmfPkg. > > > > 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) Origin ISA stack (the PcAtChipsetPkg & IntelFrameworkModulePkg one); > > c) Default behavior will be using the stack mentioned in a). > > > > 4a. If the removal of PcAtChipsetPkg/IsaAcpiDxe & IntelFrameworkModulePkg > > comes before the edk2-stable201905 tag, copy the drivers in 1. into > > OvmfPkg. > > > > 4b. If the removal of PcAtChipsetPkg/IsaAcpiDxe & IntelFrameworkModulePkg > > comes after the edk2-stable201905 tag, drop the > > PcAtChipsetPkg/IntelFrameworkModulePkg ISA stack in OVMF together with > > the removal of PcAtChipsetPkg/IsaAcpiDxe & IntelFrameworkModulePkg. > > > > What do you think of the above strategy? Thanks. > > It sounds exactly right to me. > > The important thing is that "edk2-stable201905" be released with the new > stuff (SioBusDxe + MdeModulePkg/Bus/Isa) as the default driver stack in > OVMF, but with an easy build option for users to revert to the old stack. > > (And if they feel forced to exercise that option, they should report it > to us, so we can make an informed decision about dropping the old stack > for good in the stable release that follows "edk2-stable201905".) > > Both (4a) and (4b) ensure this, so the plan looks good to me. > I think 4a is pointless: it permits IntelFrameworkModulePkg/ to be removed, but the exact same code lives on under OvmfPkg/. So if the intent is to remove IntelFrameworkModulePkg entirely, this only achieves it in a very bureaucratic sense, with extra churn in the OvmfPkg for no other reason than to make the dates on your bugzilla entries look good. So I strongly favour keeping IntelFrameworkModulePkg around (or at least the pieces OVMF depends on) until we are ready to stop using it altogether. Anything beyond that only satisfies artificial process needs rather than technical needs.