From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail02.groups.io (mail02.groups.io [66.175.222.108]) by spool.mail.gandi.net (Postfix) with ESMTPS id 4AB50D8024B for ; Wed, 10 Jan 2024 16:41:30 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=qucutUl8PfOGnxZ4INN2EFI6CQQjwhHC48xGhMRCvEg=; c=relaxed/simple; d=groups.io; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From:In-Reply-To:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Language:Content-Type:Content-Transfer-Encoding; s=20140610; t=1704904888; v=1; b=bWdsyAUpM3DoiECOER+Q8KOm+xmsTGs3xj9LGWq8Hrpn+0ntiCqIPuhmHf8WtvstKZ5k93SX vQFrLPIQj7nN0zk1MSuTpptS4NwD0qJtPbWcnGrIaBhAES/cK3td9dzg0Z/+BmpZID71f62X87X W6DixuusHvgaN7ckmk9hhC2Y= X-Received: by 127.0.0.2 with SMTP id aEYvYY7687511x2Q80n7Y4zC; Wed, 10 Jan 2024 08:41:28 -0800 X-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.web11.16461.1704904888141252735 for ; Wed, 10 Jan 2024 08:41:28 -0800 X-Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-554--lEQb9B5MyawH0lUyjifNg-1; Wed, 10 Jan 2024 11:41:23 -0500 X-MC-Unique: -lEQb9B5MyawH0lUyjifNg-1 X-Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.rdu2.redhat.com [10.11.54.7]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id F34F084C65A; Wed, 10 Jan 2024 16:41:22 +0000 (UTC) X-Received: from [10.39.192.166] (unknown [10.39.192.166]) by smtp.corp.redhat.com (Postfix) with ESMTPS id C38391C060AF; Wed, 10 Jan 2024 16:41:21 +0000 (UTC) Message-ID: Date: Wed, 10 Jan 2024 17:41:20 +0100 MIME-Version: 1.0 Subject: Re: [edk2-devel] [PATCH v1 1/1] StandaloneMmPkg: Initialise serial port early in StandaloneMmEntryPoint To: "levi.yun" , Oliver Smith-Denny , devel@edk2.groups.io, Ard Biesheuvel Cc: sami.mujawar@arm.com, ray.ni@intel.com, pierre.gondois@arm.com, nd@arm.com References: <20240105114931.844886-1-yeoreum.yun@arm.com> <5a07db2b-ea25-495d-91f8-7b51ddd9ec75@arm.com> <51aea8c8-25bd-4630-b305-e4337284661e@linux.microsoft.com> <90dd9b46-b0a6-99f3-db30-4225c337e0a9@redhat.com> <5a9cd535-988e-4212-a0bb-f340d0b124f9@arm.com> From: "Laszlo Ersek" In-Reply-To: <5a9cd535-988e-4212-a0bb-f340d0b124f9@arm.com> X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.7 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Precedence: Bulk List-Subscribe: List-Help: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Reply-To: devel@edk2.groups.io,lersek@redhat.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: X-Gm-Message-State: 39Fj82lbewLkznKfGdpkaotBx7686176AA= Content-Language: en-US Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20140610 header.b=bWdsyAUp; spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce@groups.io; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=redhat.com (policy=none) On 1/10/24 17:13, levi.yun wrote: >> My personal conclusion in that thread was [1], and correspondingly, >> commit 5087a0773645 ("ArmVirtPkg/FdtPL011SerialPortLib: initialize >> implicitly", 2023-10-07). In the end, the only tractable solution was to >> initialize the serial port (hardware, and library instance) exactly >> once, in (a) the constructor, or (b) the explicit SerialPortInitialize() >> call, or (c) any SerialPortLib API, whichever occurred first. (And (a) >> and (b) can be coalesced, because SerialPortInitialize() can be marked >> as the constructor for the lib instance.) >> >> [1] >> http://mid.mail-archive.com/542db9e1-cd28-27a2-3a98-5b0c85cd7c79@redhat.com >>      https://edk2.groups.io/g/devel/message/109235 >> >> Laszlo >> > In my personal thinking, It's better to make new interface like > > RETURN_STATUS > EFIAPI > SerialPortInitializeEarly ( > VOID >   ); > > to solve this problem. > > Because, It makes a memory permission fault > when we call SerialPortInitialize like > ArmVirtPkg/Library/FdtPL011SerialPortLib > where try to modify global variable. > > At the _ModuleEntryPoint of StandAloneMmCore which is FIRST entry from TF-A > All of Image area is mapped as RO+X, so before load StandaloneMmCore, > We couldn't write global variable. > > the purposes of above interface are: >     - Initalize serial port to use in early environment only (like > StandAloneMmCore Entry Point) where couldn't write global variable by > static information (FixedPcd). >     - It presume that all setting configured by it will be overwritten > by SerialPortInitialize. This is not a scalable solution (assuming I understand your proposal right). It would require the SerialPortLib *class* (i.e., header) to grow a new API called SerialPortInitializeEarly(), and then all existent SerialPortLib *instances* -- and there are too many of them -- would have to implement that function, even if the new function were empty in several particular library instances. If you don't have writeable global variables at the time SerialPortInitialize() is called, then there are two options: (a) the common option is to just not use global variables for caching state, and to perform the serial port init on every API call. (b) A somewhat nasty / limited, but functional approach could be to check the page protection covering the global variable. Something like this: STATIC BOOLEAN mInitialized; ... UINTN AddressOfGlobal; BOOLEAN GlobalsWriteable; if (mInitialized) { // // we're done, nothing to be done // return RETURN_SUCCESS; } // // here, perform the serial port initialization // ... // // finally: // AddressOfGlobal = (UINTN)&mInitialized; GlobalsWriteable = CheckWritable (AddressOfGlobal); if (GlobalsWriteable) { mInitialized = TRUE; } This will keep initing the serial port upon every API call until the global variable becomes writeable, and then the next API call will init the serial port for one last time, and also prevent further page table checks. The CheckWritable() function is an implementation detail. In the DXE phase, it could be implemented (I think?) with the GetMemorySpaceDescriptor() DXE service, or perhaps even the EFI_MEMORY_ATTRIBUTE_PROTOCOL.GetMemoryAttributes() UEFI protocol member function. In the standalone MM core, CheckWritable() could walk the page tables explicitly. The idea is, either way, to *predict* whether writing to "mInitialized" would trap. Now I think that speculative / out of order execution could actually trigger the trap *before* GlobalsWriteable is calculated; however, I think such a trap should be architecturally hidden (i.e., invisible). I think at worst we could need a compiler barrier (maybe throw in some "volatile" for GlobalsWriteable and mInitialized), so that the *compiler* not try to reorder the accesses. But even that sounds like a stretch. Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#113535): https://edk2.groups.io/g/devel/message/113535 Mute This Topic: https://groups.io/mt/103540969/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=-