From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail02.groups.io (mail02.groups.io [66.175.222.108]) by spool.mail.gandi.net (Postfix) with ESMTPS id DB87E740035 for ; Thu, 24 Aug 2023 10:31:49 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=bGcRXxCaezQ5vVt9SI6P5w5mkKcI731EiKq7W07k2rQ=; c=relaxed/simple; d=groups.io; h=MIME-Version:References:In-Reply-To:From:Date:Message-ID:Subject:To:Cc:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Type:Content-Transfer-Encoding; s=20140610; t=1692873107; v=1; b=h1ZvZ71DSg2bwbMPAGEdBejwdX9lqPy1aBxyUsLstrGNwoUyKl53gq+scJt0U8iKqdcBH6us B5pPoc5QOvEQclPzPpAWdzfXY4dVHSqwvbr3ZGr+CVfoduIZbFw3mYYn1VYjnc9S4VRis/F1sNl P9grD1DPvTrLleeiBmnoVEEc= X-Received: by 127.0.0.2 with SMTP id razuYY7687511xl8sJS4AZeC; Thu, 24 Aug 2023 03:31:47 -0700 X-Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by mx.groups.io with SMTP id smtpd.web10.7877.1692873107223198161 for ; Thu, 24 Aug 2023 03:31:47 -0700 X-Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 7BCD8665C9 for ; Thu, 24 Aug 2023 10:31:46 +0000 (UTC) X-Received: by smtp.kernel.org (Postfix) with ESMTPSA id E41DBC433CD for ; Thu, 24 Aug 2023 10:31:45 +0000 (UTC) X-Received: by mail-lf1-f45.google.com with SMTP id 2adb3069b0e04-4ff88239785so10164758e87.0 for ; Thu, 24 Aug 2023 03:31:45 -0700 (PDT) X-Gm-Message-State: NdGAvWJjYZSZXGo2btuBqKV3x7686176AA= X-Google-Smtp-Source: AGHT+IERczhJh72rp5yR3h7XvnKfQuEUlFWYzi4VFKlN2T8CcuRqCL/j1z5PmKVsRIugfPm0CrrwuLJtLKionn2JGC4= X-Received: by 2002:ac2:4f15:0:b0:4fe:49d:6ae2 with SMTP id k21-20020ac24f15000000b004fe049d6ae2mr12944224lfr.0.1692873103853; Thu, 24 Aug 2023 03:31:43 -0700 (PDT) MIME-Version: 1.0 References: <20230720134557.3903923-1-ardb@kernel.org> In-Reply-To: From: "Ard Biesheuvel" Date: Thu, 24 Aug 2023 12:31:32 +0200 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [edk2-devel] [RFC/RFT PATCH] OvmfPkg/IoMmuDxe: don't rely on TPLs for re-entrancy To: Gerd Hoffmann Cc: Pedro Falcato , devel@edk2.groups.io, Jiewen Yao , Michael Brown , Tom Lendacky , Michael Roth Precedence: Bulk List-Subscribe: List-Help: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Reply-To: devel@edk2.groups.io,ardb@kernel.org List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20140610 header.b=h1ZvZ71D; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=kernel.org (policy=none); spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce@groups.io On Thu, 24 Aug 2023 at 10:06, Gerd Hoffmann wrote: > > On Wed, Aug 23, 2023 at 07:10:52PM +0100, Pedro Falcato wrote: > > On Wed, Aug 23, 2023 at 4:12=E2=80=AFPM Ard Biesheuvel wrote: > > > > > > On Wed, 23 Aug 2023 at 13:08, Gerd Hoffmann wrote= : > > > > > > > > Hmm, QE reports back it slows down the boot alot. No boot hangs ye= t > > > > with 12 test runs so far, which isn't that much for a reproduce rat= e > > > > below 20% ... > > > > > > > > https://bugzilla.redhat.com//show_bug.cgi?id=3D2211060#c28 > > > > > > > > So I guess we go with the TPL version for the coming stable tag and > > > > leave any improvements for later ... > > > > > > Yeah, this was not going to make the stable tag in any case. > > > > > > The boot speed regression seems odd, though - this is effectively UP > > > code so there shouldn't be any contention, the only thing this patch > > > does is ensure that the critical section is restarted if it was > > > interrupted > > QE reported back boot times without this patch are ~20-30 seconds, > with this patch it can be more than 3 minutes. > > > FWIW: Given completely correct logic, straightforward logic, a lock > > cmpxchg is much slower(3-4x) than an a non-lock cmpxchg, which itself > > is around 2x as slow as a regular relaxed load + store. > > See https://gist.github.com/heatd/49c9be23ccb1f4ad8dfeac231da2647a for > > a nice fun test benchmark. > > Also note that IoMmuDxe is only used in case memory encryption is > enabled (I/O uses unencrypted bounce buffers so the host can virtio > emulation can read/write stuff there). Maybe that affects performance > too. > > Cc'ing the AMD people for comments on that. > > > HOWEVER, given this is likely in IO paths, I would *really* not expect > > this to make a difference, right? Even virtio drivers will themselves > > trap with a VMEXIT whenever you touch a hardware register... > > It's only a single VMEXIT per I/O request (ring the doorbell after > adding a request to the ring), but still, this is heavy enough that > the cmpxchg difference should be in the noise. > > > Gerd, could you folks get a perf kvm (perf-kvm(1)) recording out of > > that OVMF build? Assuming you can get that thing to work, that is, > > personally it mysteriously stopped working 6 years ago for me :) > > I can try hack IoMmuDxe so it is used unconditionally and try reproduce > locally (without sev-capable hardware), but most likely not this week. > I have tried the patch below, and I don't see any slowdowns with or without the patch, running both DEBUG and RELEASE builds under ordinary KVM/nested-virt. Note that the change in the first hunk will cause the ASSERT()s removed in the other hunks to trigger so the code is definitely being exercised. diff --git a/OvmfPkg/IoMmuDxe/IoMmuDxe.c b/OvmfPkg/IoMmuDxe/IoMmuDxe.c index aab6d8b90687..c1082b733d3a 100644 --- a/OvmfPkg/IoMmuDxe/IoMmuDxe.c +++ b/OvmfPkg/IoMmuDxe/IoMmuDxe.c @@ -25,7 +25,7 @@ IoMmuDxeEntryPoint ( // When SEV or TDX is enabled, install IoMmu protocol otherwise install = the // placeholder protocol so that other dependent module can run. // - if (MemEncryptSevIsEnabled () || MemEncryptTdxIsEnabled ()) { + if (TRUE || MemEncryptSevIsEnabled () || MemEncryptTdxIsEnabled ()) { Status =3D InstallIoMmuProtocol (); } else { Handle =3D NULL; diff --git a/OvmfPkg/IoMmuDxe/IoMmuBuffer.c b/OvmfPkg/IoMmuDxe/IoMmuBuffer.= c index 2764c35044ac..27fe068362ff 100644 --- a/OvmfPkg/IoMmuDxe/IoMmuBuffer.c +++ b/OvmfPkg/IoMmuDxe/IoMmuBuffer.c @@ -184,7 +184,7 @@ IoMmuInitReservedSharedMem ( ); ASSERT (!EFI_ERROR (Status)); } else { - ASSERT (FALSE); + //ASSERT (FALSE); } } @@ -233,7 +233,7 @@ IoMmuReleaseReservedSharedMem ( ); ASSERT (!EFI_ERROR (Status)); } else { - ASSERT (FALSE); + //ASSERT (FALSE); } } } -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#108002): https://edk2.groups.io/g/devel/message/108002 Mute This Topic: https://groups.io/mt/100256049/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-