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.web09.8533.1637162399122188017 for ; Wed, 17 Nov 2021 07:19:59 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=HOOhORvn; spf=pass (domain: redhat.com, ip: 170.10.133.124, mailfrom: kraxel@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1637162398; 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: in-reply-to:in-reply-to:references:references; bh=7qsb2RC7NGvJRW+cMJxhrGOUvtl+D4X3IjMPsSMcn/o=; b=HOOhORvnt+Eq0AY4/x9RjWu/PeiCCXrNl4Exzl3XZdiYcpUser5/KSnQHZQ0ulnnwu5a9G Zb0qa35TVN9PEhIdlUpWV0oLlIWNG2vYqrehMbUl0g20fF5c4QDAdMJHs5vmCpqRVfhWA2 awV9Yclf/D6Tq4OwMpnGcFFZr+K8gVk= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-216-h7dOogDFNNmtRSUJXy1UQA-1; Wed, 17 Nov 2021 10:19:51 -0500 X-MC-Unique: h7dOogDFNNmtRSUJXy1UQA-1 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 44629100C66A; Wed, 17 Nov 2021 15:19:50 +0000 (UTC) Received: from sirius.home.kraxel.org (unknown [10.39.193.245]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 7F5E156A88; Wed, 17 Nov 2021 15:19:49 +0000 (UTC) Received: by sirius.home.kraxel.org (Postfix, from userid 1000) id 4F35C18000B2; Wed, 17 Nov 2021 16:19:42 +0100 (CET) Date: Wed, 17 Nov 2021 16:19:42 +0100 From: "Gerd Hoffmann" To: "Xu, Min M" Cc: "devel@edk2.groups.io" , Ard Biesheuvel , "Justen, Jordan L" , Brijesh Singh , Erdem Aktas , James Bottomley , "Yao, Jiewen" , Tom Lendacky Subject: Re: [PATCH V3 15/29] OvmfPkg: Update SecEntry.nasm to support Tdx Message-ID: <20211117151942.iqow75zq2lrn5xlc@sirius.home.kraxel.org> References: <867e8a2aaf28c308b20a659057217453c6e38e00.1635769996.git.min.m.xu@intel.com> <20211103063045.kmttoxyluifwo2bq@sirius.home.kraxel.org> MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=kraxel@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Tue, Nov 16, 2021 at 12:11:33PM +0000, Xu, Min M wrote: > On November 3, 2021 2:31 PM, Gerd Hoffmann wrote: > > Hi, > > > > > - AcceptPages: > > > To mitigate the performance impact of accepting pages in SEC phase on > > > BSP, BSP will parse memory resources and assign each AP the task of > > > accepting a subset of pages. This command may be called several times > > > until all memory resources are processed. In accepting pages, PageLevel > > > may fall back to smaller one if SIZE_MISMATCH error is returned. > > > > Why add an assembler version of this? There already is a C version (in TdxLib, > > patch #2). When adding lazy accept at some point in the future we will stop > > accepting all pages in the SEC phase anyway. There is Mp support (patch #14) > > so you can distribute the load to all CPUs in PEI / DXE phase if you want > > (although the benefits of parallel accept will be much smaller once lazy > > accept is there). > There are below considerations about accept pages in SEC phase. > > 1. There is a minimal memory requirement in DxeCore [1]. In legacy > Ovmf the memory is initialized in PEI phase. Yes. > But TDVF has 2 configurations (Config-A and Config-B) [2]. PEI phase > is skipped in Config-B. So we have to accept memories in SEC phase. I'm sure I've asked this before: Why skip the PEI phase? So far I have not seen any convincing argument for it. Jiewen argued this is a simplification. Which is not completely wrong, but it's also only half the truth. Switching all OVMF builds over to PEI-less boot doesn't work because some features supported by OVMF depend on PEI Modules. Therefore TDX Config-B skipping the PEI phase means we would have to maintain two boot work flows (with and without PEI phase) for OVMF. Which in turn would imply more work for maintenance, testing and so on. Also note that accepting all memory in SEC phase would be temporary only. Once we have support for lazy accept in OVMF we will accept most memory in DXE phase (or leave it to the linux kernel), whereas SEC/PEI will accept only a small fraction of the memory. Just enough to allow DXE phase initialize memory management & lazy accept support. > This is to make the code consistent in Config-A and Config-B. I want TDVF be consistent with the rest of OVMF. Makes long-term maintenance easier. Building a single binary for both SEV and TDX with full confidential computing support (including config-b features) will be easier too. > 2. TDVF's MP support is via Mailbox [3] . BSP wakes up APs by putting > command/parameters in Mailbox. So we have this patch [4]. While > EDK2's MP support (CpuMpPei/CpuDxe) is via INIT-SIPI-SIPI. They're > different. We cannot distribute the load to all CPUs with EDK2's MP > service. > Patch #14 [5] is the commit to enable TDX in MpInitLib. Actually this > is to make sure the CpuMpPei/CpuDxe will not break in Td guest in > run-time. Patch #14 is rather simple, for example, ApWorker is not > supported. Well, MpInitLib seems to have full support (including ApWorker) for SEV. I'd expect you can add TDX support too, and schedule the jobs you want run on the APs via TDX mailbox instead of using IPIs. And I think to support parallel lazy accept in the DXE phase (for lazy accept) you will need proper MpInitLib support anyway. > 3. In current TDVF implementation, Stack is not set for APs. So C > functions cannot be called for APs. If stack is set for APs, then more > memories should be reserved in MEMFD. For example, if 1 AP needs 4k > size stack, then 15 AP need 60k memory. (We assume only 1+15 CPUs > accept memory). This makes things complicated. I think skipping PEI phase and moving stuff to SEC phase makes things complicated. Reserving stacks in MEMFD would only be needed because you can't allocate memory in the SEC phase. When you initialize the APs in PEI instead you can just allocate memory and the MEMFD dependency goes away. > Based on above considerations, we use the current design that BSP-APs > accept memory in SEC phase (in assembly code). I think the design doesn't make much sense for the reasons outlined above. I'd suggest to put aside parallel accept for now and just let the BSP accept the memory for initial TDX support. Focus on adding lazy accept support next. Finally re-evaluate parallel accept support, *after* merging lazy accept. With a major change in the memory acceptance workflow being planned it doesn't look like a good idea to me to tune memory accept performance *now*. My naive expectation would be that parallel accept in SEC/PEI simply isn't worth the effort due to the small amount of memory being needed. Parallel accept in DXE probably is useful, but how much it speeds up boot depends on how much accepted memory we must hand over to the linux kernel so it has enough room to successfully initialize memory management & lazy accept. take care, Gerd