From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 1292F21AE30C5 for ; Thu, 1 Jun 2017 00:39:59 -0700 (PDT) Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by orsmga101.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 01 Jun 2017 00:40:50 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.39,278,1493708400"; d="scan'208";a="268849879" Received: from dbanursh-mobl2.amr.corp.intel.com (HELO localhost) ([10.254.181.134]) by fmsmga004.fm.intel.com with ESMTP; 01 Jun 2017 00:40:49 -0700 MIME-Version: 1.0 To: Brijesh Singh , Laszlo Ersek , edk2-devel@lists.01.org Message-ID: <149630284935.10663.16670660897918560882@jljusten-skl> From: Jordan Justen In-Reply-To: <14301d64-9fa3-8231-42c1-52c2dcd9f96f@amd.com> Cc: Thomas.Lendacky@amd.com, leo.duran@amd.com, Jeff Fan , Liming Gao , Jiewen Yao References: <1495809845-32472-1-git-send-email-brijesh.singh@amd.com> <149583274037.25973.13062338567511386932@jljusten-skl> <6ecd0138-454e-6a6e-d034-beaf63466120@redhat.com> <149609029319.5770.13917390389219314003@jljusten-skl> <14301d64-9fa3-8231-42c1-52c2dcd9f96f@amd.com> User-Agent: alot/0.5.1 Date: Thu, 01 Jun 2017 00:40:49 -0700 Subject: Re: [PATCH v6 00/17] x86: Secure Encrypted Virtualization (AMD) X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 01 Jun 2017 07:39:59 -0000 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable On 2017-05-29 14:59:46, Brijesh Singh wrote: > = > = > On 5/29/17 3:38 PM, Jordan Justen wrote: > > On 2017-05-29 04:16:15, Laszlo Ersek wrote: > >> (looks like I was the one to comment as second reviewer after all :) ) > >> > >> On 05/26/17 23:05, Jordan Justen wrote: > >>> On 2017-05-26 07:43:48, Brijesh Singh wrote: > >>>> Changes since v4: > >>>> - decouple IoMmu protocol implementation from AmdSevDxe into a sepe= rate > >>>> IoMmuDxe driver. And introduce a placeholder protocol to provide = the > >>>> dependency support for the dependent modules. > >>> I think you split IoMmuDxe out from AmdSevDxe based on my feedback > >>> regarding APRIORI, but I don't think this helped. > >>> > >>> Ideally I would like to see one driver named IoMmuDxe that is *not* in > >>> APRIORI. > >> There are two separate goals here: > >> > >> (1) Make sure that any driver that adds MMIO ranges will automatically > >> add those ranges with the C bit cleared in the PTEs, without actually > >> knowing about SEV. > > Ok, this sounds reasonable. > > > > The APRIORI method looks like a hack. Why is this not being handled at > > the time the page tables are being built, in DxeIpl? Couldn't we > > define a platform Page Tables library to allow a platform to somehow > > modify the page tables as they are built? Or, maybe just after? This > > would also make sure it happens before DXE runs. > = > Before introducing AmdSevDxe driver, we did proposed patches to clear > the C-bit during the page table creation time. In the first patch [1], > Leo tried to teach gcd.c to clear the C-bit from MMIO. IIRC, the main > concern was -- typically Dxecore does not do any CPU specific thing > hence we should try to find some alternative approach. DxeCore doesn't build the page tables. DxeIpl builds them. I agree that DxeCore is not the right place to handle this. In https://lists.01.org/pipermail/edk2-devel/2017-March/008987.html Jiewen suggested that DxeIpl could be updated during page table creation time. In https://lists.01.org/pipermail/edk2-devel/2017-April/009883.html Leo said that DxeIpl won't work because new I/O ranges might be added. I don't understand this, because isn't DxeIpl and an early APRIORI entry are roughly equivalent in the boot sequence? -Jordan > In second patch > [2], Leo tried to introduce a new notify protocol to get MMIO add/remove > events. During discussion Jiewen suggested to look into adding a new > platform driver into APRIORI to avoid the need for any modifications > inside the Gcdcore - this seems workable solution which did not require > adding any CPU specific code inside the Gcd. > = > [1] https://lists.01.org/pipermail/edk2-devel/2017-March/008974.html > [2] https://lists.01.org/pipermail/edk2-devel/2017-April/009852.html >=20