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::143; helo=mail-it1-x143.google.com; envelope-from=ard.biesheuvel@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-it1-x143.google.com (mail-it1-x143.google.com [IPv6:2607:f8b0:4864:20::143]) (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 5E4AF211E2C0A for ; Thu, 21 Mar 2019 03:09:00 -0700 (PDT) Received: by mail-it1-x143.google.com with SMTP id m18so3271643ita.3 for ; Thu, 21 Mar 2019 03:09:00 -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=CLYuf8IMt4QI4aYo9HL9iY2AhJe6288JD5Kgp/2Ig7U=; b=HVeM65OvnS2tdMNy8NqkYFN/ZxLYgu5fGesrMlHTpxCUAfc7AmWOdAcJfABbNpGA0x c8TEFq5O0nMOqDdHhYmfUPSgJRP9xMxPE5CJ1KEJ6PML3D2j/4OCYvVm7So8t10IH8qz eidbg8mvfSriNHBpDxeUiA+gFQ4YHQL9AKS8hZaX7Hzrf8RzSz+d8Q6fJVBTaf9Le1vr svJoAUmjYUdhSNga6o/5ZWKzImR1oqkVaK+UQYIeJsQz+KNDr+NttSFYn1at8WnTTaEu 0HmtAO3+Q89AivEuMUw03ED+Yco/70ROcXgVaFCfwR7sx1sD3rUPWNX7DWKujXykS9DP HJDw== 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=CLYuf8IMt4QI4aYo9HL9iY2AhJe6288JD5Kgp/2Ig7U=; b=Y5HTc38/XNB/1gtfPvs3Vfi9cZVxYUp9ZuHtsqlH7JkxffQIMtGeGfWI+sMxgaDnnN K1zbeQiWYXxvZ4w8zlYhLUj55x7JQW05ZkagypA1YGOj3myvsJODNrp1ZmWe0beEP/vR 52qyv0SIOrWBDwi9IEaA1t6cxY9LVcZiirdPrhs+vPZwqMYPmYZzZnV24pwTSU7t826l l8NV1uyG+/lcV3Nm7N1nCiydHZZRsyLtN6F3b0IX6TvrkX+He4sfjg9UD65CvrZBZOic USeoMv6oWgFmJDr4yj8QRWZ7NKiwfoUr5xdbdk6rAiWEGvnDFygmX7VOqw1YI9VNWg3J kZdg== X-Gm-Message-State: APjAAAUgMo4SU3IVS8RqCPPZCEKkD9x+zqhyOLWUD52G1tm9l84v1NMb nhcMESwG1RDO4itbjHSSp0AkCHVs9Bv3jwuO0wG+iQ== X-Google-Smtp-Source: APXvYqzvdQmnPEhS4k19Fyaf3j5KUiWisWnFZ7U10bnsqEo1t2+3mLY4HiZPke7CsBaiBfc1s6INoqqYQSS8nMa2gOw= X-Received: by 2002:a24:11ce:: with SMTP id 197mr1805423itf.121.1553162939334; Thu, 21 Mar 2019 03:08:59 -0700 (PDT) MIME-Version: 1.0 References: <20190315071603.16936-1-hao.a.wu@intel.com> <2cef7754-a5ab-274e-44ab-14ba092f7d40@redhat.com> In-Reply-To: From: Ard Biesheuvel Date: Thu, 21 Mar 2019 11:08:48 +0100 Message-ID: To: "Wu, Hao A" Cc: Laszlo Ersek , "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: Thu, 21 Mar 2019 10:09:01 -0000 Content-Type: text/plain; charset="UTF-8" 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? > > > 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? > 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.