From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-1.mimecast.com (us-smtp-delivery-1.mimecast.com [205.139.110.61]) by mx.groups.io with SMTP id smtpd.web10.676.1575291415789880978 for ; Mon, 02 Dec 2019 04:56:56 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=SwnTRqjk; spf=pass (domain: redhat.com, ip: 205.139.110.61, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1575291414; 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=spkuoPlevz5fmMiPXboaUlgVEOp4LzMYvTbCXj/c4bE=; b=SwnTRqjkzR8YLiiGyiKAehAcR1vHz8fqjzLk9xATLE+sTDwePgrQ1UWsvaUdrpDKn+S53f SKknulb28yTu0nEJyfCRQQPTJBZLIBqvpTOOA5+2hGlyMYXLnWj01ZK7UjkCAWfenJFIcn PyJ9vt5EXJJNrpLpWmQtg6SjydAx7sU= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-72-OH8MYl46NKqH1zkScOYASw-1; Mon, 02 Dec 2019 07:56:51 -0500 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id ECB518017DF; Mon, 2 Dec 2019 12:56:49 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-207.ams2.redhat.com [10.36.116.207]) by smtp.corp.redhat.com (Postfix) with ESMTP id 7F21B10016DA; Mon, 2 Dec 2019 12:56:48 +0000 (UTC) Subject: Re: [PATCH v2] UefiCpuPkg/PiSmmCpuDxeSmm: Avoid allocate Token every time. To: "Ni, Ray" , "Dong, Eric" , "devel@edk2.groups.io" Cc: "Gao, Liming" References: <20191128061716.196-1-eric.dong@intel.com> <7ac1f712-5da3-e6b2-f24e-936feb81daa3@redhat.com> <734D49CCEBEEF84792F5B80ED585239D5C372368@SHSMSX104.ccr.corp.intel.com> <1f1dd740-420d-2074-dd7e-198b3b5386ad@redhat.com> <734D49CCEBEEF84792F5B80ED585239D5C38341C@SHSMSX104.ccr.corp.intel.com> From: "Laszlo Ersek" Message-ID: <6fbd4469-26a3-c0eb-3c3b-200c2eec3b50@redhat.com> Date: Mon, 2 Dec 2019 13:56:47 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <734D49CCEBEEF84792F5B80ED585239D5C38341C@SHSMSX104.ccr.corp.intel.com> X-Scanned-By: MIMEDefang 2.84 on 10.5.11.22 X-MC-Unique: OH8MYl46NKqH1zkScOYASw-1 X-Mimecast-Spam-Score: 0 Content-Language: en-US Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit On 12/02/19 06:14, Ni, Ray wrote: >>> Laszlo, >>> I agree with you to use to PCD for "token count per chunk". >>> But I suggest the default value of that PCD can be at least 64. >>> Reasons: >>> 1. MM_MP is the new MP protocol in SMM environment and we expect >>> all SMM code start to use the new protocol instead of the original >> StartupThisAp >>> service exposed through SMM System table. >> >> If we consider the older SmmStartupThisAp() / MmStartupThisAp() member >> functions, the situation is almost the same -- (almost) no callers. In >> edk2, only SmmPeriodicSmiLib seems to call this SMST (MMST) API. In >> edk2-platforms, MinPlatformPkg uses the lib class, and also calls >> SmmStartupThisAp(). >> >> I'm just saying that "all SMM code" using "the original StartupThisAp >> service" in fact means a single open source platform. > > The benefit of MmMp is it introduces StartupAllAps service which > is very useful in many core system. > There are needs to call the new services in close source code. > > PI spec contains a more detailed description on the differences: > * It is possible to invoke a procedure on multiple processors. > * Supports blocking and non-blocking modes of operation. > >> >>> 2. A value of 1 is totally to disable the pre-allocating. >> >> I disagree; with value 1, the InitializeDataForMmMp() function allocates >> a new chunk, with room for 1 token. That occurs ahead of actually >> needing the token, therefore it is a preallocation. >> >> What you mean is that the chunking will not make a difference relative >> to the current behavior, because every token creation/use will require a >> new chunk allocation. Indeed. >> >> But that doesn't change the fact that the first token will be allocated >> prior to actually needing it, and if a platform never needs a token, >> then the first chunk will just be there doing nothing. > > I agree 1 is different from disabling the pre-allocating. > But 1 is not enough in a system that needs to use MmMp multiple times > in a single SMI. > >> >>> I prefer this >>> pre-allocating is activated by default given the reason #1. >>> >>> 64 * 32/64 (SPIN LOCK size) = 2KB/4KB is not a big size. >> >> Would it be possible to *not* produce the MM MP protocol at all, if the >> PCD were 0? Because in that case, the DEC default for the PCD could be >> 64 (or any other value), and in OVMF we could set the PCD in the DSC >> file to zero. > > Back to your suggestion, I am a bit curious why you prefer to have a > PCD to turn on/off MmMp instead of pre-allocating 2KB/4KB memory. I tend to perceive SMRAM as a "premium" (scarce) resource. I could be wrong. > Any PCD introduces a cost that future developers needs to understand > the meaning of the PCD and know how to set the PCD. Yes, I agree about that. But, at least, a PCD is a well-documented and explicit artifact. An allocated, but never used memory (SMRAM) block may similarly raise questions, *if* someone still remembers it later (or stumbles upon it). Documenting the SMRAM block (in the right place) can be difficult too. If we don't document it, then there seems to be no maintenance burden, but that's actually the worst solution (obscure behavior). Anyway I've said enough in this thread; please go ahead with the solution you deem best. Thanks Laszlo