From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by mx.groups.io with SMTP id smtpd.web12.2646.1589277180401001616 for ; Tue, 12 May 2020 02:53:00 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: arm.com, ip: 217.140.110.172, mailfrom: ard.biesheuvel@arm.com) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id E20FA1FB; Tue, 12 May 2020 02:52:58 -0700 (PDT) Received: from [192.168.1.81] (unknown [10.37.8.255]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id DE0C53F71E; Tue, 12 May 2020 02:52:56 -0700 (PDT) Subject: Re: [edk2-devel] Where to put the bhyve code in the edk2 repo: BhyvePkg, or under OvmfPkg? To: Laszlo Ersek , Rebecca Cran , devel@edk2.groups.io, michael.d.kinney@intel.com Cc: Andrew Fish , Leif Lindholm , "Justen, Jordan L" References: <20CE2FEE-3844-422E-8DB2-2784C9B56CE9@bsdio.com> <2de7aa2e-a024-3c2b-14c0-161e68c31121@redhat.com> From: "Ard Biesheuvel" Message-ID: Date: Tue, 12 May 2020 11:52:53 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.7.0 MIME-Version: 1.0 In-Reply-To: <2de7aa2e-a024-3c2b-14c0-161e68c31121@redhat.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit On 5/12/20 11:28 AM, Laszlo Ersek wrote: > On 05/11/20 23:58, Rebecca Cran wrote: >> On May 11, 2020, at 3:22 PM, Michael D Kinney >> wrote: >>> >>> Hi Laszlo, >>> >>> Is there an option for Bhyve to live in an edk2-platforms branch >>> or in edk2-staging branch until it meets the quality criteria for >>> OvmfPkg in the edk2 repo? > > I'd like to have Bhyve in edk2 in order for it to share the git history > with the other edk2 modules (OvmfPkg, core, etc). > > edk2-platforms being separate is already causing massive amounts of > pain. It's now common that we can't merge an otherwise self-contained > edk2 patch series, implemented, posted, and reviewed as a unit of work > with well defined boundaries, because it would break edk2-platforms, and > there is no way to cover two distinct repositories in a single patch > set. > > There have been three separate instances of that concern just in the > last ~5 days: > > - https://edk2.groups.io/g/devel/message/58872 > (May 8th) > > - https://edk2.groups.io/g/devel/message/58874 > (May 8th) > > - https://edk2.groups.io/g/devel/message/59204 > (May 11th) > >> Also, what is the quality criteria? I'd be interested in working to >> improve any problems it currently has. I'm also planning to work on >> the Azure agent for FreeBSD and get a set of CI tests running for it. > > Hm. Maybe I made a mistake. I segued from Ard's words to the Linux > "staging" definition; that may have been wrong. Yes. I never suggested that we should merge any code that doesn't meet our quality *at all*, nor did I suggest that the Bhyve code fails to meet them (and I wouldn't even know since I haven't looked at the code that carefully) > Ard's words were: > >> However, I don't think every platforms in core EDK2 can be a first >> class citizen. There is simply no way we can expect contributors to >> make sure that their changes don't break under Bhyve, and the same >> will be true once (if) we merge kvmtool guest support, which is under >> development as well (given that it supports virtualization only, and >> so unlike QEMU, which supports emulation as well, it requires a native >> host) >> >> So I agree that it makes sense to incorporate Bhyve into core EDK2, >> but we have to decide on some rules regarding 'second class' >> platforms: how/when to test them, and how urgently we treat >> regressions found during such testing. We can treat ArmVirtXen the >> same way, imo, as well as KvmTool when it lands. > > So maybe the question is how large the user base of a given > virtualization platform is, how wide-spread and easy-to-use the > virtualization platform is. How much time and presence can be dedicated > to maintaining its firmware. > > If it's a niche virt platform, then keeping its firmware-side > counterparts regression-free is more difficult for the community (due to > the disproportionate amount of resources that its testing would > require), and so the quality of *that* firmware code is expected to be > lower. > It is not about code quality. It is about rules and guidelines we set for contributors. I think it would be excellent if the azure CI would be extended to cover boot of a VM under bhyve. But what if that fails without any usable output? Is the burden going to be on me as a contributor to go and debug that platform if my code changes break it but nothing else? > Importantly, I do think "manpower dedicated" outweighs "platform is > niche". There was a time when UefiCpuPkg changes would break OVMF every > two weeks. We didn't solve that by making QEMU/KVM "less of a niche" for > the UefiCpuPkg owners -- we solved it by me asking (first informally) to > be CC'd on all UefiCpuPkg patches, and then formally to be marked as a > designated reviewer for UefiCpuPkg. And I'd review and regression-test a > whole bunch of UefiCpuPkg patches, just so I could catch regressions > before they made it into the tree. > > If the bhyve community can *permanently* provide reviews / > regression-testing for such OVMF contributors that never use bhyve, that > would significantly increase the stability of bhyve firmware code, and > it would outweigh bhyve's user base (likely) being smaller. Xen > regressions were also reduced when the Xen community finally delegated > designated reviewers to edk2. > > Reviewing and testing patches you don't really care for, but see as > possibly regressive for the platform you do care about, is a *lot* of > work. So I guess it could boil down to how much work your platform's > user base can contribute to the edk2 project. > That is an important aspect, but not really what I am concerned about here. If nobody is invested enough in a platform to notice that is gets broken, I don't think we should put the burden on other maintainers to take responsibility for that.