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.web12.28212.1632729924622251594 for ; Mon, 27 Sep 2021 01:05:24 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=ZQUdqhW4; 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=1632729923; 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=8hVdXO5fQWkJWkiD5AirxHlk6MueOVdP1QBb7BHuiac=; b=ZQUdqhW4yaqI1I47ekoKZVKnT7s/oN/lJtLpVE93WESYEyI8B2QnLEnWqb79kOx9pnEISz kpYoL7IW8Lsoo9IOXE4UH3TV8HeKguXHzzzwwf3H/+/1DTN02f/4lpcF7wK+WIHGMf7Y/A l0+AYsI+MD2K/s4kwGRhqKJi3Do5Qxg= 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-256-oNFtnJyAMHm7rX9gMOOyKw-1; Mon, 27 Sep 2021 04:05:19 -0400 X-MC-Unique: oNFtnJyAMHm7rX9gMOOyKw-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 2F66718D6A2E; Mon, 27 Sep 2021 08:05:18 +0000 (UTC) Received: from sirius.home.kraxel.org (unknown [10.39.193.134]) by smtp.corp.redhat.com (Postfix) with ESMTPS id C92565D9D5; Mon, 27 Sep 2021 08:05:17 +0000 (UTC) Received: by sirius.home.kraxel.org (Postfix, from userid 1000) id 48F9B1800938; Mon, 27 Sep 2021 10:05:16 +0200 (CEST) Date: Mon, 27 Sep 2021 10:05:16 +0200 From: "Gerd Hoffmann" To: "Yao, Jiewen" Cc: "devel@edk2.groups.io" , "Xu, Min M" , Brijesh Singh , Ard Biesheuvel , "Justen, Jordan L" , Erdem Aktas , James Bottomley , Tom Lendacky Subject: Re: [edk2-devel] [PATCH V7 1/1] OvmfPkg: Enable TDX in ResetVector Message-ID: <20210927080516.a64levdyyg6b4hne@sirius.home.kraxel.org> References: <20210924052825.2qljhtvweonbov5q@sirius.home.kraxel.org> <20210924100726.m33otbjod4fo3vur@sirius.home.kraxel.org> <20210924140221.6b3nk32eofwbwpgb@sirius.home.kraxel.org> MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 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, > > Possible alternative approach: Define an extension > > (EFI_FIRMWARE_VOLUME_EXT_HEADER) for the load address and use that > > instead of defining something new. > > [Jiewen] I would say it is terrible idea to use EFI_FIRMWARE_VOLUME_HEADER. > > [ ... details snipped ... ] Ok, lets scratch the idea then. > > Still not clear why the size is in there twice. I think you could > > instead use a flag telling whenever a section must be loaded from > > the image or not. This is what COFF and ELF are doing too. > > > > Also not clear why you want stick to 64bit base address. Loading the > > firmware above 4G isn't going to work without also changing a bunch of > > other things and it will break backward compatibility anyway. > > [Jiewen] That is our previously experience, when we define a physical address, we always use 64bit to leave room for extension. > Like UEFI specification defines PHYSICAL_ADDRESS to be UINT64 even in IA32 platform. Sure, physical address space on IA32 is 64G which simply doesn't fit into UINT32 ... > Defining UINT64 gives us enough flexibility for the future, including test in above 4G environment. > I am wondering that, do you really care to save 4 bytes from UINT64 to UINT32 ? Well, MEMFD isn't that big, so we have to care about the size there ... > For type, maybe 2^16 is enough. But for flags, I prefer 32bit. Making one 16bit and the other 32bit doesn't make much sense due to struct padding and alignment. So with 64-bit load address and fields reordered so we don't have any padding the struct could look like this: struct { uint64_t load_address; uint32_t file_offset; uint32_t section_size; uint32_t section_type; uint32_t section_flags; }; > > But, again, I don't want have two completely different initialization > > code paths in OVMF. We can certainly investigate and discuss dropping > > PEI, but that clearly shouldn't be a TDX-only thing. When we eliminate > > PEI, we should do it for all OVMF builds. > > [Jiewen] I think this is out of scope of TDVF config-B patch series. > I don't think it is fair to enable OVMF to remove PEI, just because > TDVF does not need PEI. Hmm? TDVF is based on OVMF. My expectation would be that TDVF is basically OVMF with TDX support added and most code being identical. So with TDVF being able to boot without PEI most of the work should already be done ... > Each platform owner can have their own choice. Why do you consider TDVF a separate platform? I see it as OVMF variant. Or did you just fork OVMF for config-b? Doing so is certainly easier for initial development and prototyping, but it's bad in the long run. Having similar code twice in the code base means more long-term maintenance work, which isn't fair either. > If you have intertest to remove PEI, I am happy to discuss with you on > detail. However, I would treat "removing PEI from OVMF" and "enable > TDVF config-B" be two different tasks. Given TDVF wants boot without PEI the later depends on the former though. Or TDVF config-B continues to use the PEI-based boot workflow for now like config-A does, and we look into removing PEI from OVMF later. take care, Gerd