From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by mx.groups.io with SMTP id smtpd.web11.32867.1630998462926121206 for ; Tue, 07 Sep 2021 00:07:43 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=AZLlkT8B; spf=pass (domain: redhat.com, ip: 216.205.24.124, mailfrom: kraxel@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1630998462; 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=Uvb9vzkklVaOL/AJdGHZiaDuf6EtNF4GeOYP6nVv5Iw=; b=AZLlkT8Bdovq5pKs057420kaGnhzKVH26BfnTr3go3eqynn2fTt8I0PHi7sBViFoH3CuGc RsdnxCo56JIJpafdso96fgBLsz3Tg5v+7tA2c0A1CQgN1CQJSbPEr1uHRIbFP7x+Jwcjyt e6SO23i46HJ3YJQj7W84UFxMveCst+E= 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-175-8nKEM17ROsOm0jHwa9ec4A-1; Tue, 07 Sep 2021 03:07:36 -0400 X-MC-Unique: 8nKEM17ROsOm0jHwa9ec4A-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 91037824FB3; Tue, 7 Sep 2021 07:07:34 +0000 (UTC) Received: from sirius.home.kraxel.org (unknown [10.39.192.91]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 17D055D6CF; Tue, 7 Sep 2021 07:07:34 +0000 (UTC) Received: by sirius.home.kraxel.org (Postfix, from userid 1000) id 71294180039F; Tue, 7 Sep 2021 09:07:32 +0200 (CEST) Date: Tue, 7 Sep 2021 09:07:32 +0200 From: "Gerd Hoffmann" To: "Xu, Min M" Cc: "devel@edk2.groups.io" , Brijesh Singh , James Bottomley , "Yao, Jiewen" , Tom Lendacky , "Justen, Jordan L" , Ard Biesheuvel , Erdem Aktas , Michael Roth Subject: Re: [edk2-devel] [PATCH v6 06/29] OvmfPkg/ResetVector: pre-validate the data pages used in SEC phase Message-ID: <20210907070732.xcokfdn5iw3wyqbu@sirius.home.kraxel.org> References: <20210901161646.24763-1-brijesh.singh@amd.com> <20210901161646.24763-7-brijesh.singh@amd.com> <20210902082029.tfdt4s5s76qknpiq@sirius.home.kraxel.org> <20210906121650.vwgt5y5hdwxfugvh@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 Hi, > > [ Looking at https://www.mail- > > archive.com/devel@edk2.groups.io/msg33605.html ] > > > > So, there isn't much tdx-specific in tdx-metadata. Most ranges are > > TDX_METADATA_SECTION_TYPE_TEMP_MEM which I think basically means > > these ranges should be accepted by the hypervisor, which is pretty much the > > same issue snp tries to solve with this pre-validation range. Then there are > > the ranges for code (aka bfv), for vars (aka cfv) and td_hob. > > > > td_hob is the only tdx-specific item there, and even that concept (pass > > memory ranges as hob list from hypervisor to guest) might be useful outside > > tdx. > Mailbox is tdx-specific too. But Stack/Heap/OvmfWorkarea/OvmfPageTable are > common. BFV/CFV are common too. Mailbox is tagged "TDX_METADATA_SECTION_TYPE_TEMP_MEM", so nothing special to do when loading the firmware, right? > > I'd suggest we generalize the tdx-metadata idea and define both generic and > > vmm-specific section types: > > > > enum { > > OVMF_SECTION_TYPE_UNDEFINED = 0; > > > > /* generic */ > > OVMF_SECTION_TYPE_CODE = 0x100, > > OVMF_SECTION_TYPE_VARS > > OVMF_SECTION_TYPE_SEC_MEM /* vmm should accept/validate this */ > > > > /* sev */ > > OVMF_SECTION_TYPE_SEV_SECRETS = 0x200, > > OVMF_SECTION_TYPE_SEV_CPUID /* or move to generic? */ > > > > /* tdx */ > > OVMV_SECTION_TYPE_TDX_TD_HOB = 0x300, > > }; > > > > Comments? > TDX has similar section type. Yes. Both TDX and SNP have simliar requirements, they want store memory ranges in the firmware binary in a way that allows qemu finding them and using them when initializing the guest. SNP stores the ranges directly in the GUID-chained block in the reset vector. The range types are implicit (first is pre-validate area, second is cpuid page, ...). TDX stores a pointer to tdx-metadata in the GUID-chained block, then the tdx-metadata has a list of ranges. The ranges are explicitly typed (section type field). The indirection used by TDX keeps the reset vector small. Also the explicit typing of the ranges makes it easier to extend later on if needed. IMHO SEV should at minimum add explicit types to the memory ranges in the boot block, but I'd very much prefer it if SEV and TDX can agree on a way to store the memory ranges. > But I am not sure if SEV can use this metadata mechanism. > Need SEV's comments. Brijesh? > > Looking at tdx-metadata I have a few questions: > > > > +_Bfv: > > + DD TDX_BFV_RAW_DATA_OFFSET > > + DD TDX_BFV_RAW_DATA_SIZE > > > > What is this and why is it needed? > Host VMM need to measure the code part (BFV) to MRTD register > (which is similar to TPM PCRs). Sure, but why you can't use TDX_BFV_MEMORY_BASE + TDX_BFV_MEMORY_SIZE for that? The tdvf design guide says it is the file offset. The firmware must be mapped right below 4G, which implies RAW_DATA_OFFSET + (4G - FIRMWARE_SIZE) == MEMORY_BASE. So having both RAW_DATA_OFFSET and MEMORY_BASE looks redundant to me. > > + DQ TDX_BFV_MEMORY_BASE > > + DQ TDX_BFV_MEMORY_SIZE > > > > Why "DQ"? TDX is defined to start in 32bit mode, so you can hardly have > > addresses here which do not fit into "DD", correct? > Those are the memory. TDX is running in long mode. So it is DQ. Hmm? TDX entry vector is defined to be 32bit. Which pretty much implies you can't map the firmware above 4G, even if one of the first actions of the reset vector code is the switch to long mode. take care, Gerd