From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mx.groups.io with SMTP id smtpd.web10.12644.1675338445227004307 for ; Thu, 02 Feb 2023 03:47:25 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=epNhXt3O; spf=pass (domain: redhat.com, ip: 170.10.133.124, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1675338444; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=5esz1bwExAqUqK9PRMNegjPZ0LZzF5xcvE1iJrFh8oI=; b=epNhXt3OjfJYdayLrPvlIqp8yhhgl8BmSmXR494GWOuGFYXWzVWHhGHg1teL6LYD0uASnS fnjjF6wurF2QZhonx9MQ8fN7IMaUOZi10EP/kqvlw9HE2/znRDEHj8iH8SMsp+pg+bctBa j9wBs2Y+eJuL/iBzAp9fP0w/M01tMmw= Received: from mimecast-mx02.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-626-tyI6qJtrNJaNV6JwmzCquw-1; Thu, 02 Feb 2023 06:47:21 -0500 X-MC-Unique: tyI6qJtrNJaNV6JwmzCquw-1 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.rdu2.redhat.com [10.11.54.3]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id D5D4D3C0219C; Thu, 2 Feb 2023 11:47:20 +0000 (UTC) Received: from [10.39.192.122] (unknown [10.39.192.122]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 7D8F5112132C; Thu, 2 Feb 2023 11:47:19 +0000 (UTC) Message-ID: <00b01cd3-7ed2-b0f1-e2ef-1d48930a0083@redhat.com> Date: Thu, 2 Feb 2023 12:47:18 +0100 MIME-Version: 1.0 Subject: Re: [PATCH v3 5/5] OvmfPkg/SmmCpuFeaturesLib: Skip SMBASE configuration To: Gerd Hoffmann , "Wu, Jiaxin" Cc: "Ni, Ray" , "devel@edk2.groups.io" , "Dong, Eric" , "Zeng, Star" , "Kumar, Rahul R" References: <20230118095620.9860-1-jiaxin.wu@intel.com> <20230118095620.9860-6-jiaxin.wu@intel.com> <20230118121958.cxbfh3fljedvebis@sirius.home.kraxel.org> <20230119075303.nkyno36h25xscwkn@sirius.home.kraxel.org> <20230201134051.7jlc7a74cogcskw5@sirius.home.kraxel.org> <20230202090003.5vmmeyhsv4zn7wn4@sirius.home.kraxel.org> From: "Laszlo Ersek" In-Reply-To: <20230202090003.5vmmeyhsv4zn7wn4@sirius.home.kraxel.org> X-Scanned-By: MIMEDefang 3.1 on 10.11.54.3 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit I'm going to comment on this one email up-stream, because it showcases the community problem, as far as I'm concerned, and because Jiaxin made a reference to my initial request. On 2/2/23 10:00, Gerd Hoffmann wrote: > Hi, > >>> But the serialized SMBASE programming still happens, now in the PEI >>> module, and I don't see a good reason why the runtime the new PEI module >>> and the runtime of PiSmmCpuDxeSmm combined is faster than before. >> >> As I said, PEI module can also programs SMBASE in parallel, for >> example program the some register directly instead of depending the >> existing RSM instruction to reload the SMBASE register with the new >> allocated SMBASE each time when it exits SMM. > > Ok. So new Intel processors apparently got new MSR(s) to set SMBASE > directly. Any specific reason why you don't add support for that to > PiSmmCpuDxeSmm? That would avoid needing the new HOB (and the related > problems with the 8190 cpu limit) in the first place. See this is *exactly* my problem. The *whole work* on this should have started like this, with a new Feature Request Bugzilla: "Intel are introducing a new processor register (MSR or other method) with their XY product line where firmware can program the SMBASE independently of the RSM instruction. The PEI code performing this logic will not be open sourced, similarly to other things that are kept binary-only in the FSP (firmware support packages), and perhaps similarly to how memory chips are initialized in the PEI phase too, by "MRC" (memory reference code). Because there is no intent to open source the initialization logic, possibly due to the new MSR not even being slated for documentation in the Intel SDM, we need a new *binary-only* interface." *That* would have been honest, straight talk. Not this smoke-screen with "another vendor might have a different method". That's entirely speculative generality. Speculative generality has been an anti-pattern in software development for decades, even merely for technical means, but here the justification for it is not even technical: the language around the generality is just to hide the one actual purpose of the feature. Don't do that please. Describe your *specific* use case, list your arguments, and then explain your approach for making it regression-free for the existent cases. PI and UEFI are all about binary interfaces between proprietary vendors. As much as I disagree with the entire concept [*], I accept it as a fact of life. I just ask that, whenever that pattern (= introducing ABIs, rather than APIs) is exercised, at least the *actual goal* be documented. ([*] I disagree with the concept for two reasons. One, ideological (no further explanation needed). Two, practical. If you "standardize" an interface when you have *one* application of it, that's always trouble. The second application, if there's ever going to be a second one, will almost surely not fit within the framework of the "standard". So it's best to either open source all the implementations, or at least openly document their *actual* interface needs. Clearly state that the initial interface implementation is "work in progress". If there are multiple (divergent) applications, *then* try to extract something common, either from the open source code bases, or the clearly documented data dependencies, and then codify that. UEFI is *hugely* harmed by the proliferation of protocols, where every new feature needs to be standardized, as soon as one implementation exists. These protocols then get ossified and linger around for absolutely forever. I feel that it's totally self-inflicted damage; it is the consequence of the proprietary software development model -- effectively the unwillingness to share.) >>> Do you intent submitting code for OVMF producing such a HOB? >>> There isn't any in this series. >> >> No, we won't do that. > > Then there is no point in changing the OVMF code, > other than maybe adding an ASSERT that there is no such HOB. I agree. My initial request was for *considering* OVMF's library instance. "Considering" means evaluating, and modifying *if* necessary, and *as* necessary. I could not perform this evaluation myself: initially, the purpose of the new interface was obscure, so I couldn't tell if OVMF was affected or not. Now that we *know* that OVMF is unaffected, we should just use library instances for what library instances exist for: customization (platform or otherwise). Because OVMF will not contain a PEI module initializing the SMBASE regs as described, there is no need for adding (dead) code to OVMF's lib instance. Adding the ASSERT() *definitely* makes sense however: it's a statement that we have *considered* the applicability of the feature. That is *precisely* what I missed from the initial announcement. Look, when you see a summary diffstat for a patch set that modifies N instances of a library class, but not the one in OVMF -- or in any one particular subsystem --, you ask yourself: "is this an oversight, or is this intentional? did they just miss or ignore that platform or subsystem, or did they evaluate it as unaffected"? *That* is exactly what should have been in the original BZ or cover letter. "I've grepped the edk2 and edk2-platforms tree for all instances of this library class; I need to modify instances X, Y, Z; I *know* I *need not* modify P, Q; and I'm *unsure* about U, V and W. Please advise". The ASSERT() will give us a good opportunity to write a comment and/or commit message around this. This whole ordeal is just another piece of evidence for why I cannot remain a member of this community. Hiding information, as a *basic modus operandi*, is incompatible with open source development. It's *fine* if a project or a contributor adopts a "my way or the high way" attitude. Throwing open source code over the wall is still open source. It's not open development any more, but still open source -- it is still valuable (though less so). There may be many *valid* reasons for doing open source, but not open development. What pains me is the dishonest or at least mixed / sloppy messaging about *what edk2 is*. Is it open source, or is it open development? https://github.com/tianocore/tianocore.github.io/wiki/TianoCore-Who-we-are Laszlo