From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 9CCAFC77B7A for ; Fri, 26 May 2023 14:40:28 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S244048AbjEZOk0 (ORCPT ); Fri, 26 May 2023 10:40:26 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:34982 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S244085AbjEZOkC (ORCPT ); Fri, 26 May 2023 10:40:02 -0400 Received: from mail-yb1-xb49.google.com (mail-yb1-xb49.google.com [IPv6:2607:f8b0:4864:20::b49]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 635A5E54 for ; Fri, 26 May 2023 07:39:25 -0700 (PDT) Received: by mail-yb1-xb49.google.com with SMTP id 3f1490d57ef6-bacfa4eefcbso2164615276.1 for ; Fri, 26 May 2023 07:39:25 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20221208; t=1685111956; x=1687703956; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=HNAPtYMKE2DwuE0/Om5XERH8unHdbvGM9xOlT/rQSiM=; b=qsUncX2EbLctr4fP8gajSri5U1pxUkivvKJRV9QQZmG0n0CaNsLFJVwgtgSWDD+SMJ 2SdsjklIMX7MTPqTAJd6xEdCOhoRT1rQhi3AonOw5G76o0mFvMlyx2d+x+u3Lv3sR5XU SpIntlAP1o1CdhXFFuU5KFnjUOPei3x70ZeBUQzhnzfj1yKSBUU7AwTHLeMWgZpAsbgh zuODJ2RLL4JnK45BOaGIm4QWeVWqU00jBeMbuLaRDcz1u9yNd4/1VUeVhTAWMR5UXyCT aY3ep4oSH6a2l8RdFYcyXK7LfRXeh/9jcPQiNaj/YT6UTvBvSzXnYlMJLGCqHwQVu6Ai D9jQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1685111956; x=1687703956; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=HNAPtYMKE2DwuE0/Om5XERH8unHdbvGM9xOlT/rQSiM=; b=Tu93wzsx6kwQLur6FzSbyg/BwnZsSAzutop3CuxTGvdFMd8RIxRInI+ljnaZE1MTU3 /U+g0N+eGB0YwKdFnTBt7SsDqy+lKpEnVl+NT7AAl7PPvR/ygaCgYRdKH7MHUn0owanE glT87j3EvlSV+0oGY6chzpH+lFZEObXNUi7YuHhWjTkRutysoo8LrwblAL/+x/fz15FQ /tdNM//OUbixhK7JsbQwA3u+m2IkMhQTv4mrWENzOvsgkSQYxp2SVhEiylTUqYu7Nuk4 oOpHmBY6KfMLTEbc7KTQ3jRbAAn6flUpwkchkgkgnLsLA/2v98hI9mjgO3Iqk8fZQGA+ fm7Q== X-Gm-Message-State: AC+VfDxqIELxQKNDkVJpkBrmEjCMqtC1l0KMyBWCRXnsKwU1y9a3GX7e XyKDLzBmlzFQwbrv5Xm5oM3txnCNkM8= X-Google-Smtp-Source: ACHHUZ47oCO6HwXqYzWIxcyvOS4fpxEXPkDQACfUzuqbBxss74rfmln5dAtL/xTF8sqWXxmiF1DIIzBlvQQ= X-Received: from zagreus.c.googlers.com ([fda3:e722:ac3:cc00:7f:e700:c0a8:5c37]) (user=seanjc job=sendgmr) by 2002:a25:8206:0:b0:b9e:7fbc:15e1 with SMTP id q6-20020a258206000000b00b9e7fbc15e1mr4332600ybk.0.1685111955796; Fri, 26 May 2023 07:39:15 -0700 (PDT) Date: Fri, 26 May 2023 07:39:14 -0700 In-Reply-To: Mime-Version: 1.0 References: <20230411125718.2297768-1-aik@amd.com> <20230411125718.2297768-6-aik@amd.com> <719a6b42-fd91-8eb4-f773-9ed98d2fdb07@amd.com> Message-ID: Subject: Re: [PATCH kernel v5 5/6] KVM: SEV: Enable data breakpoints in SEV-ES From: Sean Christopherson To: Alexey Kardashevskiy Cc: kvm@vger.kernel.org, x86@kernel.org, linux-kernel@vger.kernel.org, Tom Lendacky , Pankaj Gupta , Nikunj A Dadhania , Santosh Shukla , Carlos Bilbao Content-Type: text/plain; charset="us-ascii" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, May 26, 2023, Alexey Kardashevskiy wrote: > > On 24/5/23 01:44, Sean Christopherson wrote: > > On Tue, May 23, 2023, Alexey Kardashevskiy wrote: > > > > Actually, can't disabling #DB interception for DebugSwap SEV-ES guests be a > > > > separate patch? KVM can still inject #DBs for SEV-ES guests, no? > > > > > > Sorry for my ignorance but what is the point of injecting #DB if there is no > > > way of changing the guest's DR7? > > > > Well, _injecting_ the #DB is necessary for correctness from the guest's perspective. > > "What's the point of _intercepting_ #DB" is the real question. And for SEV-ES guests > > with DebugSwap, there is no point, which is why I agree that KVM should disable > > interception in that case. What I'm calling out is that disabling #Db interception > > isn't _necessary_ for correctness (unless I'm missing something), which means that > > it can and should go in a separate patch. > > > About this. Instead of sev_es_init_vmcb(), I can toggle the #DB intercept > when toggling guest_debug, see below. This > kvm_x86_ops::update_exception_bitmap hook is called on vcpu reset and > kvm_arch_vcpu_ioctl_set_guest_debug (which skips this call if > guest_state_protected = true). KVM also intercepts #DB when single-stepping over IRET to find an NMI window, so you'd also have to factor in nmi_singlestep, and update svm_enable_nmi_window() and disable_nmi_singlestep() to call svm_update_exception_bitmap(). > Is there any downside? Complexity is the main one. The complexity is quite low, but the benefit to the guest is likely even lower. A #DB in the guest isn't likely to be performance sensitive. And on the flip side, opening an NMI window would be a tiny bit more expensive, though I doubt that would be meaningful either. All in all, I think it makes sense to just keep intercepting #DB for non-SEV-ES guests. Side topic, isn't there an existing bug regarding SEV-ES NMI windows? KVM can't actually single-step an SEV-ES guest, but tries to set RFLAGS.TF anyways. Blech, and suppressing EFER.SVME in efer_trap() is a bit gross, but I suppose since the GHCB doesn't allow for CLGI or STGI it's "fine". E.g. shouldn't KVM do this? diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c index ca32389f3c36..4e4a49031efe 100644 --- a/arch/x86/kvm/svm/svm.c +++ b/arch/x86/kvm/svm/svm.c @@ -3784,6 +3784,16 @@ static void svm_enable_nmi_window(struct kvm_vcpu *vcpu) if (svm_get_nmi_mask(vcpu) && !svm->awaiting_iret_completion) return; /* IRET will cause a vm exit */ + /* + * KVM can't single-step SEV-ES guests and instead assumes that IRET + * in the guest will always succeed, i.e. clears NMI masking on the + * next VM-Exit. Note, GIF is guaranteed to be '1' for SEV-ES guests + * as the GHCB doesn't allow for CLGI or STGI (and KVM suppresses + * EFER.SVME for good measure, see efer_trap()). + */ + if (sev_es_guest(vcpu->kvm)) + return; + if (!gif_set(svm)) { if (vgif) svm_set_intercept(svm, INTERCEPT_STGI);