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 A280C21A16ED6 for ; Tue, 16 May 2017 10:56:38 -0700 (PDT) Received: from fmsmga006.fm.intel.com ([10.253.24.20]) by orsmga101.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 16 May 2017 10:56:38 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.38,350,1491289200"; d="scan'208";a="102828925" Received: from casander-mobl.amr.corp.intel.com (HELO localhost) ([10.254.34.31]) by fmsmga006.fm.intel.com with ESMTP; 16 May 2017 10:56:37 -0700 MIME-Version: 1.0 To: Brijesh Singh , Laszlo Ersek , edk2-devel@lists.01.org Message-ID: <149495739728.2794.9440373089056382638@jljusten-skl> From: Jordan Justen In-Reply-To: <89da82c5-1e23-190b-0725-76f4da595987@amd.com> Cc: Thomas.Lendacky@amd.com, leo.duran@amd.com, Jiewen Yao References: <1494454162-9940-1-git-send-email-brijesh.singh@amd.com> <1494454162-9940-7-git-send-email-brijesh.singh@amd.com> <1ebdde6a-bad7-ed9a-b2af-7334477c8ae3@redhat.com> <149452462682.9607.8151737434916832513@jljusten-skl.jf.intel.com> <149487045771.31444.19976106484440238@jljusten-skl> <89da82c5-1e23-190b-0725-76f4da595987@amd.com> User-Agent: alot/0.5.1 Date: Tue, 16 May 2017 10:56:37 -0700 Subject: Re: [RFC v4 06/13] OvmfPkg:AmdSevDxe: add AmdSevDxe driver 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: Tue, 16 May 2017 17:56:38 -0000 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable On 2017-05-16 05:04:58, Brijesh Singh wrote: > Hi Jordan, > = > On 5/15/17 12:47 PM, Jordan Justen wrote: > > On 2017-05-11 11:01:57, Brijesh Singh wrote: > >> > >> We basically need some kind of guarantee that this driver is run befor= e any other > >> drivers or libs access MMIO register/buffers. In additional to clearin= g encryption > >> bit from MMIO spaces, the driver also installs IOMMU protocol. So far,= IOMMU protocol > >> is directly consumed by PciHostBridgeDxe driver and QemuFwCfgDxeLib. > > What about adding a NULL protocol named > > gOvmfIoMmuDetectionProtocolGuid? (Better name suggestions welcomed. :) > > Then we can use this protocol in a depex where needed. > It should be doable, If I find better name then I will use that :) > > Maybe we should consider naming the driver IoMmuDxe instead? > > > > I think the generic PciRoot bridge driver shouldn't need this in the > > depex because it will not start until the BDS phase, and the IoMmuDxe > > driver would have been dispatched by then. > Are you suggesting that we introduce a new IoMmuDxe driver and install > IOMMU protocol unconditionally ? No. I'm suggesting we have a new protocol that only exists to allow dependency expressions to know that we've attempted to detect an IoMmu implementation. The driver would "install" the protocol with a NULL pointer regardless of whether the IoMmu protocol was installed. Maybe gOvmfIoMmuDetectionAttemptedProtocolGuid would be a better name? The DXE fw-cfg library should then list this under depex. I think the PCI Host bridge driver doesn't require the depex for the reason I mentioned abobe. The gEdkiiIoMmuProtocolGuid protocol would only be installed be installed when detected like your patches currently do. This method should allow the driver runtime order dependency to be explicitly indicated. Regarding the 'IoMmuDxe' name, I was suggesting that AmdSevDxe be renamed to IoMmuDxe. Since we would be installing the 'we tried to detect iommu' protocol, it probably makes sense to put all the iommu implementation support into a single driver and only install the 'detection attempted' protocol it after trying to detect all supported iommu implementations. There could be an issue with this. FvbServicesRuntimeDxe.inf is in the apriori currently if SMM_REQUIRE is set, so if it needs the iommu treatment, then this wouldn't work. This driver does use MM I/O just below 4GB. I don't think your current patches would change how this driver runs since it doesn't use the PCI host bridge protocol, so I guess it is ok? (It would be nice to get FvbServicesRuntimeDxe.inf out of the apriori too, but that is a separate issue.) -Jordan > I was hoping that we install IOMMU > protocol only when SEV is enabled. A non-SEV guest will still use the > old approach. I was minimizing changes into non-SEV code flow. Please > note that since AmdSevDxe driver does *two* things; a) clear C-bit from > MMIO b) installs IOMMU protocol hence I will not able to remove = > AmdSevDxe completely. But I can remove IOMMU protocol installation part > from AmdSevDxe and move it into new IoMmuDxe driver. Please let me know > if this is what you are asking. thanks > = > -Brijesh