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 bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 41187D1812A for ; Mon, 14 Oct 2024 16:08:49 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:Content-Type:Cc:To:From: Subject:Message-ID:References:Mime-Version:In-Reply-To:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=pwSWZjQ27GxDWRHISycf7nMSxcYhNRFFhPOH/5On2AI=; b=GDf1YwfvyyBmcLbVKsFeJE9rqG rixCHaYlYo+ZSoL6ZDC6hTMuuTSKrT06YXGL9VSmnH9QCAuA6fLTQ96qupaZjOK8HrX3EwKjX004W iSosC2m+FCY8t+sIjRQDvScnCOEOHNCY9cHaNSlQzM6GVNhykYDBz88fQDEUAsXLMbvCM33QKgO9J T3rx0lF/C0Wc8AqiHdTDd7F6E+Vv9TIGhpphTaGxgFkjq6IcdG0ToJsDW2ETEbTGYkRjroimAKpQS m88MvzWHHrcYTj0C5hrh37viW3gFRYQhZgGeAvH/eKOYYU5YPndyjIo3omr5IvNjBdkupSXOpovtD sSbutgPQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98 #2 (Red Hat Linux)) id 1t0Ncc-00000005o9B-3JhF; Mon, 14 Oct 2024 16:08:38 +0000 Received: from mail-pl1-x64a.google.com ([2607:f8b0:4864:20::64a]) by bombadil.infradead.org with esmtps (Exim 4.98 #2 (Red Hat Linux)) id 1t0Nau-00000005npn-3Rxm for linux-arm-kernel@lists.infradead.org; Mon, 14 Oct 2024 16:06:54 +0000 Received: by mail-pl1-x64a.google.com with SMTP id d9443c01a7336-20c8a637b77so59611115ad.2 for ; Mon, 14 Oct 2024 09:06:51 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1728922010; x=1729526810; darn=lists.infradead.org; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=pwSWZjQ27GxDWRHISycf7nMSxcYhNRFFhPOH/5On2AI=; b=DFwLkvuq69I9DnMi896d41ypNS5UBjB8RvQUH4IkGPcNXNT63U+VWIEV2zWwn1Ad0M qVj75n7nT7UBXahWVkXuQQMg9JB5DpMqe8CslrXhJIj4PyObO267m4PiCiasAkV26+fq RoM99vK9iS3bfohThe/Fch4PPSwcJb6d/6XiocTo3qqFbExnPeD0B+YjDLPHdUn2+wYV G1C8H6YIIbbncCIQpTm9elEwexEdTy74UswMeyz+oA8XTwAJxlcHuso2XbnJCqJwrn+1 3f2zzw6OHg+Q1mPpwbvLcwU8eFAVDk2eF3GJGJGhQnfPGcwYmK5h34NGUAons48DtuLH +RHw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1728922010; x=1729526810; 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=pwSWZjQ27GxDWRHISycf7nMSxcYhNRFFhPOH/5On2AI=; b=vGGLefDXlYsGr4LIePua8zwZii9Mg2zu2vqN5XVHwa2eaqN8AqkH7oGFZatSiXq6X4 H12BvAqrZMJF7wFvlCvi9GQCKIDXcqC9Zfz9bJsTrP04Cyz4GAOWDS0uOa6pgqWkX0Vs faBSvUva/lbYObOSmEonjGrURS06LPAMzdzPqIVYrnLLeeQNiznium8sU0D6uWEiGrLo wS+RRaqQtJNuZ08pDwl7sCDmtvWjkS9TS/bouCcwi9udLFMD7ab+QrLqVYoR5KQJ4rAg +PnlBBd5COxXSQ5NzCZaM1p7Gd1vdCxXACKZ5DQIn/Y3r0QJMeHvWoHF7HmRI+Pvm5N5 W5oQ== X-Forwarded-Encrypted: i=1; AJvYcCWLr4yejtyqmsSKQ8YqFgzV9F5q9usBMrBH7zM+xngEBO0rc+mgT1b2apMZQGQ0G7GBPanhpsiwM4DDIo2N3C0b@lists.infradead.org X-Gm-Message-State: AOJu0YwkJNoQxKbB2IkRio9AwMJMI/DeMFWNELWfL/UPHrnAoWFNA4Kf UAD8JON+zpA5OWyUbfM9LfLUqd1dce0RP5MCoJ/y4AHwc9TaAJRFbIVXSaH/qzq3o2+ZysLbhxa E0w== X-Google-Smtp-Source: AGHT+IEwjwfWJSyr0D7btX97mpAtj9F5UTed1EZBw/lMmbFEbnas4u3vHagJykzf8XQHofrX4lrNygFFlNk= X-Received: from zagreus.c.googlers.com ([fda3:e722:ac3:cc00:9d:3983:ac13:c240]) (user=seanjc job=sendgmr) by 2002:a17:902:e5d0:b0:20b:861a:25d4 with SMTP id d9443c01a7336-20ca1460029mr661295ad.5.1728922010162; Mon, 14 Oct 2024 09:06:50 -0700 (PDT) Date: Mon, 14 Oct 2024 09:06:48 -0700 In-Reply-To: <20241014105124.24473-3-adrian.hunter@intel.com> Mime-Version: 1.0 References: <20241014105124.24473-1-adrian.hunter@intel.com> <20241014105124.24473-3-adrian.hunter@intel.com> Message-ID: Subject: Re: [PATCH V13 02/14] KVM: x86: Fix Intel PT IA32_RTIT_CTL MSR validation From: Sean Christopherson To: Adrian Hunter Cc: Peter Zijlstra , Paolo Bonzini , Ingo Molnar , Mark Rutland , Alexander Shishkin , Heiko Carstens , Thomas Richter , Hendrik Brueckner , Suzuki K Poulose , Mike Leach , James Clark , coresight@lists.linaro.org, linux-arm-kernel@lists.infradead.org, Yicong Yang , Jonathan Cameron , Will Deacon , Arnaldo Carvalho de Melo , Jiri Olsa , Namhyung Kim , Ian Rogers , Andi Kleen , Thomas Gleixner , Borislav Petkov , Dave Hansen , x86@kernel.org, H Peter Anvin , Kan Liang , Zhenyu Wang , mizhang@google.com, kvm@vger.kernel.org, Shuah Khan , linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org Content-Type: text/plain; charset="us-ascii" X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20241014_090652_892479_02B6CF8A X-CRM114-Status: GOOD ( 19.61 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org "KVM: VMX:" for the scope. And I would much prefer to actually state what is changing. "Fix XYZ" isn't helpful in understanding what's actually broken, fallout from the bug, etc. It's never easy to describe bugs where the logic is flat out busted, but I think we can at least capture the basic gist, and allude to the badness being a wrongly disallowed write. On Mon, Oct 14, 2024, Adrian Hunter wrote: > Fix KVM IA32_RTIT_CTL MSR validation logic so that if RTIT_CTL_TRACEEN > bit is cleared, then other bits are allowed to change also. For example, > writing 0 to IA32_RTIT_CTL in order to stop tracing, is valid. There's a fair amount of extraneous and disctracting information in both the shortlog and changelog. E.g. "Intel PT IA32_RTIT_CTL MSR" can simply be MSR_IA32_RTIT_CTL. And the I'll fix up to the below when applying; AFAICT, this fix is completely independent of the rest of the series. KVM: VMX: Allow toggling bits in MSR_IA32_RTIT_CTL when enable bit is cleared Allow toggling other bits in MSR_IA32_RTIT_CTL if the enable bit is being cleared, the existing logic simply ignores the enable bit. E.g. KVM will incorrectly reject a write of '0' to stop tracing. > Fixes: bf8c55d8dc09 ("KVM: x86: Implement Intel PT MSRs read/write emulation") > Cc: stable@vger.kernel.org > Signed-off-by: Adrian Hunter > --- > arch/x86/kvm/vmx/vmx.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c > index 1a4438358c5e..eaf4965ac6df 100644 > --- a/arch/x86/kvm/vmx/vmx.c > +++ b/arch/x86/kvm/vmx/vmx.c > @@ -1635,7 +1635,8 @@ static int vmx_rtit_ctl_check(struct kvm_vcpu *vcpu, u64 data) > * result in a #GP unless the same write also clears TraceEn. > */ > if ((vmx->pt_desc.guest.ctl & RTIT_CTL_TRACEEN) && > - ((vmx->pt_desc.guest.ctl ^ data) & ~RTIT_CTL_TRACEEN)) > + (data & RTIT_CTL_TRACEEN) && > + data != vmx->pt_desc.guest.ctl) > return 1; > > /* > -- > 2.43.0 >