From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [63.128.21.124]) by mx.groups.io with SMTP id smtpd.web11.6199.1605772242194815212 for ; Wed, 18 Nov 2020 23:50:42 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=JyFf19wV; spf=pass (domain: redhat.com, ip: 63.128.21.124, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1605772241; 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=T2kfGy0cbxBDyb0Q0dai50d6Rqk44xTdM716ZPqTzlw=; b=JyFf19wVxW/uL5WALmFHMRPud0tozYbJlnqLRm7Hw9Ou1oRDHp269PJJr91YtmDnNpfQ/R N0AcLkOobkUq2UWUgjf5lzNX942VuNbyKCHrQ8Anx1dZ+BNzW/oDqE3JfbsaOQ9kjmuoJG G7cg3OGnnVz7gxByQ008fF7FsWUdz5M= 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-268-jAFcAZ3YNnCMS3Zzv4gNJQ-1; Thu, 19 Nov 2020 02:50:37 -0500 X-MC-Unique: jAFcAZ3YNnCMS3Zzv4gNJQ-1 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 94DE58144E1; Thu, 19 Nov 2020 07:50:35 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-112-236.ams2.redhat.com [10.36.112.236]) by smtp.corp.redhat.com (Postfix) with ESMTP id C605310013D7; Thu, 19 Nov 2020 07:50:32 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH 3/4] OvmfPkg: create a SEV secret area in the AmdSev memfd To: jejb@linux.ibm.com, devel@edk2.groups.io Cc: dovmurik@linux.vnet.ibm.com, Dov.Murik1@il.ibm.com, ashish.kalra@amd.com, brijesh.singh@amd.com, tobin@ibm.com, david.kaplan@amd.com, jon.grimm@amd.com, thomas.lendacky@amd.com, frankeh@us.ibm.com, "Dr . David Alan Gilbert" References: <20201112001316.11341-1-jejb@linux.ibm.com> <20201112001316.11341-4-jejb@linux.ibm.com> <6db69ccd-340f-2df2-718b-5f7db09da0b8@redhat.com> <111856145fea09b5ed88a1dfa7b7d7ff6eece639.camel@linux.ibm.com> From: "Laszlo Ersek" Message-ID: <0576e9ad-e844-c50a-a329-ba19138b587d@redhat.com> Date: Thu, 19 Nov 2020 08:50:31 +0100 MIME-Version: 1.0 In-Reply-To: <111856145fea09b5ed88a1dfa7b7d7ff6eece639.camel@linux.ibm.com> X-Scanned-By: MIMEDefang 2.84 on 10.5.11.22 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=lersek@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 11/18/20 21:23, James Bottomley wrote: > On Mon, 2020-11-16 at 23:46 +0100, Laszlo Ersek wrote: >> On 11/12/20 01:13, James Bottomley wrote: > [... I made all the changes above this] >>> diff --git a/OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm >>> b/OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm >>> index 980e0138e7..7d3214e55d 100644 >>> --- a/OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm >>> +++ b/OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm >>> @@ -35,6 +35,8 @@ ALIGN 16 >>> ; the build time RIP value. The GUID must always be 48 bytes >>> from the >>> ; end of the firmware. >>> ; >>> +; 0xffffffc2 (-0x3e) - Base Location of the SEV Launch Secret >>> +; 0xffffffc6 (-0x3a) - Size of SEV Launch Secret >>> ; 0xffffffca (-0x36) - IP value >>> ; 0xffffffcc (-0x34) - CS segment base [31:16] >>> ; 0xffffffce (-0x32) - Size of the SEV-ES reset block >>> @@ -51,6 +53,8 @@ ALIGN 16 >>> TIMES (32 - (sevEsResetBlockEnd - sevEsResetBlockStart)) DB 0 >>> >>> sevEsResetBlockStart: >>> + DD SEV_LAUNCH_SECRET_BASE >>> + DD SEV_LAUNCH_SECRET_SIZE >>> DD SEV_ES_AP_RESET_IP >>> DW sevEsResetBlockEnd - sevEsResetBlockStart >>> DB 0xDE, 0x71, 0xF7, 0x00, 0x7E, 0x1A, 0xCB, 0x4F >> >> (5) I'd prefer if we could introduce a new GUID-ed structure for >> these new fields. The logic in QEMU should be extended to start >> scanning at 4GB-48 for GUIDS. If the GUID is not recognized, then >> terminate scanning. Otherwise, act upon the GUID-ed structure found >> there as necessary, and then determine the next GUID *candidate* >> location by subtracting the last recognized GUID-ed structure's >> "size" field. > > So for this one, we can do it either way. However, the current design > of the sevEsRestBlock is (according to AMD) to allow the addition of > SEV specific information. Each piece of information is a specific > offset from the GUID and the length of the structure can only grow, so > the ordering is fixed once the info is added and you can tell if the > section contains what you're looking for is present if the length > covers it. > > We can certainly move this to a fully GUID based system, which would > allow us to have an unordered list rather than the strict definition > the never decreasing length scheme allows, but if we do that, the > length word above becomes redundant. Well, GUIDed structs in UEFI/PI are sometimes permitted to grow compatibily, and for that, either a revision field or a size field is necessary / used. I kind of desire both here -- it makes sense to extend (for example) the SEV-ES reset block with relevant information, and to add other blocks of information (identified with different GUIDs). Basically I wouldn't want to finalize the SEV-ES AP reset block just yet, *but* I also think this new information does not beloing in the SEV-ES *AP reset block*. The new info is related to SEV-ES alright, but not to the AP reset block, in my opinion. If you read the larger context (the docs) in the assembly source around "sevEsResetBlockStart", the launch secret just doesn't seem to fit that. > I don't have a huge preference for either mechanism ... they seem to > work equally well, but everyone should agree before I replace the > length based scheme. I agree we should all agree about it first. And, to reiterate, I'd like to keep both the length fields and the GUID-ed identification. In other words, a GUID should not imply an exact struct size, just a minimum struct size. Thanks! Laszlo