From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: redhat.com, ip: 209.132.183.28, mailfrom: lersek@redhat.com) Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by groups.io with SMTP; Thu, 22 Aug 2019 06:46:12 -0700 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 mx1.redhat.com (Postfix) with ESMTPS id E04A5307CDEA; Thu, 22 Aug 2019 13:46:11 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (unknown [10.36.118.90]) by smtp.corp.redhat.com (Postfix) with ESMTP id 51F01100EBDD; Thu, 22 Aug 2019 13:46:09 +0000 (UTC) Subject: Re: [edk2-devel] [RFC PATCH 01/28] OvmfPkg/Sec: Enable cache early to speed up booting To: Jordan Justen , devel@edk2.groups.io, thomas.lendacky@amd.com Cc: Ard Biesheuvel , Michael D Kinney , Liming Gao , Eric Dong , Ray Ni , "Singh, Brijesh" , "Fang, Peter" , D Scott Phillips References: <6a37c84f4989304b21205d6263c6491f81da3233.1566250534.git.thomas.lendacky@amd.com> <6d3442d5-46ab-2b99-6100-0e5c56477735@redhat.com> <156642425970.26211.8321620974236559246@jljusten-skl> From: "Laszlo Ersek" Message-ID: <9659350e-fb81-fa3a-d86a-5a76d73c3ce1@redhat.com> Date: Thu, 22 Aug 2019 15:46:07 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <156642425970.26211.8321620974236559246@jljusten-skl> X-Scanned-By: MIMEDefang 2.84 on 10.5.11.22 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.49]); Thu, 22 Aug 2019 13:46:12 +0000 (UTC) Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 08/21/19 23:51, Jordan Justen wrote: > On 2019-08-21 07:21:25, Laszlo Ersek wrote: >> On 08/19/19 23:35, Lendacky, Thomas wrote: >>> From: Tom Lendacky >>> >>> Currently, the OVMF code relies on the hypervisor to enable the cache >>> support on the processor in order to improve the boot speed. However, >>> with SEV-ES, the hypervisor is not allowed to change the CR0 register >>> to enable caching. >>> >>> Update the OVMF Sec support to enable caching in order to improve the >>> boot speed. >>> >>> Signed-off-by: Tom Lendacky >>> --- >>> OvmfPkg/Sec/SecMain.c | 5 +++++ >>> 1 file changed, 5 insertions(+) >>> >>> diff --git a/OvmfPkg/Sec/SecMain.c b/OvmfPkg/Sec/SecMain.c >>> index 3914355cd17b..2448be0cd408 100644 >>> --- a/OvmfPkg/Sec/SecMain.c >>> +++ b/OvmfPkg/Sec/SecMain.c >>> @@ -739,6 +739,11 @@ SecCoreStartupWithStack ( >>> >>> ProcessLibraryConstructorList (NULL, NULL); >>> >>> + // >>> + // Enable caching >>> + // >>> + AsmEnableCache (); >>> + >>> DEBUG ((EFI_D_INFO, >>> "SecCoreStartupWithStack(0x%x, 0x%x)\n", >>> (UINT32)(UINTN)BootFv, >>> >> >> This makes me uncomfortable. There used to be problems related to >> caching when VFIO device assignment were used. My concern is admittedly >> vague, but this is a very brittle area of OVMF-on-KVM. If you asked me >> "well what could break here", I'd answer "you never know, and the burden >> of proof is not on me". :) Can we make this change conditional on SEV-ES? > > This was also raised as an issue by Peter for the ACRN hypervisor and > Scott for the bhyve hypervisor. > > I think it is rare for a platform to enable cache at this early of a > stage, but it is also rare to decompress a firmware volume at this > point. > > It appears that it could be helpful to figure out how to safely enable > cache by default here, since it does seem to be impacting several > hypervisors. I can't think of anything better than "trial and error". The issues that used to pop up in the past, due to host kernel (KVM) changes, particularly in connection with VFIO device assignment, have been completely obscure and unpenetrable to me. Even though I've contributed at least one KVM patch to mitigate those problems, they remain a mistery to me, and I remain unable to *reason* about the problems or the fixes. So I think we could only flip the behavior (enable cache by default) and collect bug reports. But that's extremely annoying for end-users, and I see "no regressions" as one of my top responsibilities. Even if we provided an fw_cfg knob to disable the change, using QemuFwCfgSecLib, similarly to commit ab081a50e565 ("OvmfPkg: PlatformPei: take no-exec DXE settings from the QEMU command line", 2015-09-15), such fw_cfg knobs are difficult to use through layered products, such as libvirt, proxmox, etc. And of course fw_cfg is only available on QEMU. Laszlo