From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pl1-f201.google.com (mail-pl1-f201.google.com [209.85.214.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 14AFA3CD8B0 for ; Fri, 3 Apr 2026 16:30:48 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.214.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1775233852; cv=none; b=m9szIQ/k+IfxU8Ap1z8Ph6xHKj2Vo6twXJnDNXybiSADE+DtAUTCZkkJ0KQRE8eLxSJfDhloPXE3Rd+rYD4kyie4ICBTqoioBCt6lXvVXcOvTzNMWmNik7iCgTajsoDe+UN0Ms8QMCncS2x0MzKHxT/PvYe4a/rpkYvHLm28ipc= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1775233852; c=relaxed/simple; bh=rGZgWtwCCI3+eFvVKc+X88h43lrdjuX32gE4iywL6A4=; h=Date:In-Reply-To:Mime-Version:References:Message-ID:Subject:From: To:Cc:Content-Type; b=fcJcqTVFr6GjI10YvRBxGLGFLB82pAzGxb8BxKcrKbRp4vUm1PEf8QjlPT5kwuPcJPZqsEL5QFkfboIMTkATf3lNZBaG0zz+nNiQK1n1wGACxOQwZ0fwKt4z7D7lqGvmBnvp3IrcrsA13JCj7eEGdMfUJp2ey+ntRhlJBPqwTHM= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com; spf=pass smtp.mailfrom=flex--seanjc.bounces.google.com; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b=syfRd8WS; arc=none smtp.client-ip=209.85.214.201 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=flex--seanjc.bounces.google.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="syfRd8WS" Received: by mail-pl1-f201.google.com with SMTP id d9443c01a7336-2b24cd2e2b3so20367665ad.0 for ; Fri, 03 Apr 2026 09:30:48 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20251104; t=1775233847; x=1775838647; darn=vger.kernel.org; h=content-transfer-encoding:cc:to:from:subject:message-id:references :mime-version:in-reply-to:date:from:to:cc:subject:date:message-id :reply-to; bh=2+Zt8Ye6QZDycK/H8I2xKI9t6Wy2CW5OkF+BQs3682Q=; b=syfRd8WSLrPJ6apEQ604WeEvOF3fivRBdp2ojXHjExtESYQt1WMCLa4T9b30Y4I9py XgjanW6SYEz+XHJD68+Y8bUOmx+q1SPlCvCKpxQXcrtisBeEJWJbP6B40Oy68kR1+Uar 7Y3Si/V2T87rPei7TT/cLo53tzCbWM4rFtyBWwSgRoWvDaCreO3eIjJrJtN8AmAUSOth F9xj1r5UvYwxZN6RieSdw+cmjQLzY/LU7LXeI/vMAa2Hdtk6cBm202BUDPuy1fXMgFBI Ia25KfciaS6UwE3Wf7UK7azc8LeVpJ7zVn8G7XTTxKVqzROZSj5jDFLdN1uAsBas6gyR e8aA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1775233847; x=1775838647; h=content-transfer-encoding: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=2+Zt8Ye6QZDycK/H8I2xKI9t6Wy2CW5OkF+BQs3682Q=; b=KgTL5tcxIChvO9wQGK5Ykazsm61XjP+YVRdKzptiKaT3lG9LpO1HmLZLvWrU3YPH7v yW4yU/+WD4ph2FSxYRT5W6DpuVz5jzKqR+Zipp/dUL+fMO96omzgdkCPGyTqgq7bzn7N JmqutS1YvHwv/gF+VvtT/C5PgyHFP1CGH9uwmvKLZTRl/9HE1oHy5Zk1eMlusDhFiEld RTRRkHOeWnOxHaVTtAM4m1q72VfLETmPkpREeAJ9bd7n/3Fb0FngmYQSTUSlKYs/2xua tlYzeFZpmRkF1127MLcUB5dLLwcdouOUsZJT06pz4ICU++Jj8Pe7gpc9DOF59u6bzIdu kjSg== X-Forwarded-Encrypted: i=1; AJvYcCV/St93IwPrFnsjOnkdCAvDQP2qL2DgT5D9Aw4klWHEU4hIDGMJKVxb3TeTnv3mKhpxGl4cJcQUB3eLYOQ=@vger.kernel.org X-Gm-Message-State: AOJu0YyNPImb8GDzWZwfuAqx8OwTYXmvZXTzUTVN6VCvKsj8/Xts6fQ7 tmNmLp030nQv8cvwQNnomA4THPnmAbsMFO7b+vmlsxIWTfRpx1kRVeKYLMLhy8utkuifa7ZWTo0 CcaFCmw== X-Received: from plrx13.prod.google.com ([2002:a17:902:b40d:b0:2ae:d0cb:5159]) (user=seanjc job=prod-delivery.src-stubby-dispatcher) by 2002:a17:902:f54f:b0:2ae:5eab:132e with SMTP id d9443c01a7336-2b28173fa9dmr36278905ad.12.1775233847240; Fri, 03 Apr 2026 09:30:47 -0700 (PDT) Date: Fri, 3 Apr 2026 09:30:45 -0700 In-Reply-To: Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 References: <20260318190111.1041924-1-dmaluka@chromium.org> <94b06319-2be8-4f01-87d1-8989ae1ca85d@intel.com> <93358559-5ed1-4574-8951-24d7ea9354e4@intel.com> Message-ID: Subject: Re: [PATCH] KVM: TDX: Fix APIC MSR ranges in tdx_has_emulated_msr() From: Sean Christopherson To: Rick P Edgecombe Cc: "dmaluka@chromium.org" , "kvm@vger.kernel.org" , "pbonzini@redhat.com" , Dave Hansen , "binbin.wu@linux.intel.com" , Isaku Yamahata , "bp@alien8.de" , "x86@kernel.org" , "kas@kernel.org" , "hpa@zytor.com" , "linux-kernel@vger.kernel.org" , "mingo@redhat.com" , "dave.hansen@linux.intel.com" , "tglx@kernel.org" , "linux-coco@lists.linux.dev" Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable On Thu, Mar 19, 2026, Rick P Edgecombe wrote: > On Thu, 2026-03-19 at 15:40 +0800, Binbin Wu wrote: > > tdx_has_emulated_msr() is used by KVM to decide whether to emulate a MS= R access from the > > TDVMCALL or just return the error code. > >=20 > > During an off-list discussion, Rick noted that #VE reduction could chan= ge the behavior of > > accessing an MSR (e.g., from #VE to #GP or to be virtualized by the TDX= module) without > > KVM knowing.Because KVM lacks the full context to perfectly decide if a= n MSR should be > > emulated, the question was raised: Can we just delete tdx_has_emulated_= msr() entirely? > >=20 > > However, these native type x2apic MSRs are a special case. Since the TD= X module owns the > > APICv page, KVM cannot emulate these MSRs. If we remove tdx_has_emulate= d_msr(), a guest > > directly issuing TDVMCALLs for these native type x2apic MSRs will trigg= er a silent failure, > > even though this is the guest's fault. > >=20 > > It comes down to a tradeoff. Should we prioritize code simplicity by dr= opping the function, > > or keep it to explicitly catch this misbehaving guest corner case? >=20 > I think from KVM's perspective it doesn't want to help the guest behave > correctly. Uh, yes KVM does does. KVM is responsible for emulating the APIC timer, is= n't it? > So we can ignore that I think. But it does really care to not define > any specific guest ABI that it has to maintain. So tdx_has_emulated_msr()= has > some value there.=C2=A0And even more, it wants to not allow the guest to = hurt the > host. >=20 > On the latter point, another problem with deleting tdx_has_emulated_msr()= is the > current code path skips the checks done in the other MSR paths. So we wou= ld need > to call some appropriate higher up MSR helper to protect the host? And th= at > wades into the CPUID bit consistency issues. >=20 > So maybe... could we do a more limited version of the deletion where we a= llow > all the APIC MSRs through? We'd have to check that it won't cause problem= s. What? No. KVM can't get actually read/write most (all?) MSRs, allowing ac= cess is far worse than returning an error, as for all intents and purposes KVM w= ill silently drop writes, and return garbage on reads. > Failing that, we should maybe just explicitly list the ones TDX supports = rather > than the current way we define the APIC ones. As you mention below, it's = not > correct in other ways too so it could be more robust. No? Don't we just want to allow access to MSRs that aren't accelerated? W= hat the TDX-Module supports is largely irrelevant, I think. diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c index 1e47c194af53..28e87630870b 100644 --- a/arch/x86/kvm/vmx/tdx.c +++ b/arch/x86/kvm/vmx/tdx.c @@ -2116,23 +2116,26 @@ bool tdx_has_emulated_msr(u32 index) case MSR_IA32_MC0_CTL2 ... MSR_IA32_MCx_CTL2(KVM_MAX_MCE_BANKS) - 1= : /* MSR_IA32_MCx_{CTL, STATUS, ADDR, MISC, CTL2} */ case MSR_KVM_POLL_CONTROL: + /* + * x2APIC registers that are virtualized by the CPU can't be + * emulated, KVM doesn't have access to the virtual APIC page. + */ + case X2APIC_MSR(APIC_ID): + case X2APIC_MSR(APIC_LVR): + case X2APIC_MSR(APIC_LDR): + case X2APIC_MSR(APIC_SPIV): + case X2APIC_MSR(APIC_ESR): + case X2APIC_MSR(APIC_ICR): + case X2APIC_MSR(APIC_LVTT): + case X2APIC_MSR(APIC_LVTTHMR): + case X2APIC_MSR(APIC_LVTPC): + case X2APIC_MSR(APIC_LVT0): + case X2APIC_MSR(APIC_LVT1): + case X2APIC_MSR(APIC_LVTERR): + case X2APIC_MSR(APIC_TMICT): + case X2APIC_MSR(APIC_TMCCT): + case X2APIC_MSR(APIC_TDCR): return true; - case APIC_BASE_MSR ... APIC_BASE_MSR + 0xff: - /* - * x2APIC registers that are virtualized by the CPU can't b= e - * emulated, KVM doesn't have access to the virtual APIC pa= ge. - */ - switch (index) { - case X2APIC_MSR(APIC_TASKPRI): - case X2APIC_MSR(APIC_PROCPRI): - case X2APIC_MSR(APIC_EOI): - case X2APIC_MSR(APIC_ISR) ... X2APIC_MSR(APIC_ISR + APIC_IS= R_NR): - case X2APIC_MSR(APIC_TMR) ... X2APIC_MSR(APIC_TMR + APIC_IS= R_NR): - case X2APIC_MSR(APIC_IRR) ... X2APIC_MSR(APIC_IRR + APIC_IS= R_NR): - return false; - default: - return true; - } default: return false; }