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 DD90D7803D0 for ; Wed, 31 Jan 2024 14:55:34 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=Ouqnt1cyI7VAfWBFHh5QT5uo2YqjEHlrP3IwLvaCqCI=; c=relaxed/simple; d=groups.io; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version:In-Reply-To:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Type:Content-Disposition; s=20140610; t=1706712933; v=1; b=ZPLwR7izrjso+v9veB6JM3Ik7NHNt3CbKVOWOF+Suq+j8hwsgaE2Mst+uj3aTReYO4C2YG2f ZI0CCMKjxmmUio3sdOXRKmLZU2ly7Kp2p6VnHU9YnDi/U3VVctISVYIWon/cAdrnCAdQoquGulu hd165lIjSKpnwnScU/ZFv91g= X-Received: by 127.0.0.2 with SMTP id f5nRYY7687511xZgYcJg02H7; Wed, 31 Jan 2024 06:55:33 -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.15880.1706712932593388424 for ; Wed, 31 Jan 2024 06:55:32 -0800 X-Received: from mimecast-mx02.redhat.com (mx-ext.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-408-BkxNVUMBNou50lRCk_TYYw-1; Wed, 31 Jan 2024 09:55:28 -0500 X-MC-Unique: BkxNVUMBNou50lRCk_TYYw-1 X-Received: from smtp.corp.redhat.com (int-mx10.intmail.prod.int.rdu2.redhat.com [10.11.54.10]) (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 CFDCE3C025C1; Wed, 31 Jan 2024 14:55:27 +0000 (UTC) X-Received: from sirius.home.kraxel.org (unknown [10.39.192.243]) by smtp.corp.redhat.com (Postfix) with ESMTPS id AAC0A492BE2; Wed, 31 Jan 2024 14:55:27 +0000 (UTC) X-Received: by sirius.home.kraxel.org (Postfix, from userid 1000) id 658521800DDC; Wed, 31 Jan 2024 15:55:26 +0100 (CET) Date: Wed, 31 Jan 2024 15:55:26 +0100 From: "Gerd Hoffmann" To: Laszlo Ersek Cc: devel@edk2.groups.io, Oliver Steffen , Jiewen Yao , Ard Biesheuvel Subject: Re: [edk2-devel] [PATCH 1/3] OvmfPkg/PlatformPei: consider AP stacks for pei memory cap Message-ID: References: <20240131120000.358090-1-kraxel@redhat.com> <20240131120000.358090-2-kraxel@redhat.com> <65a072d8-9f2c-3487-dc26-9a72e9eccc3d@redhat.com> MIME-Version: 1.0 In-Reply-To: <65a072d8-9f2c-3487-dc26-9a72e9eccc3d@redhat.com> X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.10 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,kraxel@redhat.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: X-Gm-Message-State: t9KauhMTnMdsazPQ9qpWueCXx7686176AA= Content-Type: text/plain; charset=us-ascii Content-Disposition: inline X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20140610 header.b=ZPLwR7iz; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=redhat.com (policy=none); spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce@groups.io Hi, > > + if (MemoryCap > MAX_UINT32) { > > + MemoryCap = MAX_UINT32; > > + } > > + > > (2) This doesn't look good, for two reasons. > > First, if UINTN is UINT32, then we're too late to check. (If it's intentional that the code be dead on IA32, then that should be documented.) IA32 has an early exit in this function. Looking again I see this applies to 32-bit DXE only, so with mixed 32-bit PEI / 64-bit DXE we can still land here. Hmm, so yes, you have a point here. In practice we don't come even close to MAX_UINT32. With 4096 vcpus the amount of memory needed for the AP stacks sums up to 128 MB. The page table memory wouldn't grow that big too because OVMF will use at most 1 TB of address space if gigabyte pages are not available (to limit page table memory and boot time needed to set them all up). > Second, while returning MAX_UINT32 from this function is safe, it's also obscure. After the call site, we've got > > MemoryBase = PlatformInfoHob->S3Supported && PlatformInfoHob->SmmSmramRequire ? > PcdGet32 (PcdOvmfDecompressionScratchEnd) : > PcdGet32 (PcdOvmfDxeMemFvBase) + PcdGet32 (PcdOvmfDxeMemFvSize); > MemorySize = LowerMemorySize - MemoryBase; > if (MemorySize > PeiMemoryCap) { > MemoryBase = LowerMemorySize - PeiMemoryCap; > MemorySize = PeiMemoryCap; > } MemorySize is UINT64. So I guess the easiest would be to just switch MemoryCap variable and GetPeiMemoryCap() return value to UINT64 too. Avoids all the thinking and arguing whenever UINT32 is safe or not. And maybe add an else branch to log a warning in case MemorySize < PeiMemoryCap. take care, Gerd -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#114903): https://edk2.groups.io/g/devel/message/114903 Mute This Topic: https://groups.io/mt/104073297/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=-