Received-SPF: pass (google.com: domain of linux-kernel+bounces-130041-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Date: Wed, 3 Apr 2024 08:07:23 -0700
In-Reply-To: <Zgz8YFgD1CysKaDl@chao-email>
Precedence: bulk
Mime-Version: 1.0
References: <cover.1708933498.git.isaku.yamahata@intel.com>
 <62f8890cb90e49a3e0b0d5946318c0267b80c540.1708933498.git.isaku.yamahata@intel.com>
 <Zgz8YFgD1CysKaDl@chao-email>
Message-ID: <Zg1wqxSHF-RyLihQ@google.com>
Subject: Re: [PATCH v19 111/130] KVM: TDX: Implement callbacks for MSR
 operations for TDX
From: Sean Christopherson <seanjc@google.com>
To: Chao Gao <chao.gao@intel.com>
Cc: isaku.yamahata@intel.com, kvm@vger.kernel.org, 
	linux-kernel@vger.kernel.org, isaku.yamahata@gmail.com, 
	Paolo Bonzini <pbonzini@redhat.com>, erdemaktas@google.com, Sagi Shahar <sagis@google.com>, 
	Kai Huang <kai.huang@intel.com>, chen.bo@intel.com, hang.yuan@intel.com, 
	tina.zhang@intel.com
Content-Type: text/plain; charset="us-ascii"

On Wed, Apr 03, 2024, Chao Gao wrote:
> On Mon, Feb 26, 2024 at 12:26:53AM -0800, isaku.yamahata@intel.com wrote:
> >+bool tdx_has_emulated_msr(u32 index, bool write)
> >+{
> >+	switch (index) {
> >+	case MSR_IA32_UCODE_REV:
> >+	case MSR_IA32_ARCH_CAPABILITIES:
> >+	case MSR_IA32_POWER_CTL:
> >+	case MSR_IA32_CR_PAT:
> >+	case MSR_IA32_TSC_DEADLINE:
> >+	case MSR_IA32_MISC_ENABLE:
> >+	case MSR_PLATFORM_INFO:
> >+	case MSR_MISC_FEATURES_ENABLES:
> >+	case MSR_IA32_MCG_CAP:
> >+	case MSR_IA32_MCG_STATUS:
> >+	case MSR_IA32_MCG_CTL:
> >+	case MSR_IA32_MCG_EXT_CTL:
> >+	case MSR_IA32_MC0_CTL ... MSR_IA32_MCx_CTL(KVM_MAX_MCE_BANKS) - 1:
> >+	case MSR_IA32_MC0_CTL2 ... MSR_IA32_MCx_CTL2(KVM_MAX_MCE_BANKS) - 1:
> >+		/* MSR_IA32_MCx_{CTL, STATUS, ADDR, MISC, CTL2} */
> >+		return true;
> >+	case APIC_BASE_MSR ... APIC_BASE_MSR + 0xff:
> >+		/*
> >+		 * x2APIC registers that are virtualized by the CPU can't be
> >+		 * emulated, KVM doesn't have access to the virtual APIC page.
> >+		 */
> >+		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_ISR_NR):
> >+		case X2APIC_MSR(APIC_TMR) ... X2APIC_MSR(APIC_TMR + APIC_ISR_NR):
> >+		case X2APIC_MSR(APIC_IRR) ... X2APIC_MSR(APIC_IRR + APIC_ISR_NR):
> >+			return false;
> >+		default:
> >+			return true;
> >+		}
> >+	case MSR_IA32_APICBASE:
> >+	case MSR_EFER:
> >+		return !write;
> >+	case 0x4b564d00 ... 0x4b564dff:
> >+		/* KVM custom MSRs */
> >+		return tdx_is_emulated_kvm_msr(index, write);
> >+	default:
> >+		return false;
> >+	}
> 
> The only call site with a non-Null KVM parameter is:
> 
> 	r = static_call(kvm_x86_has_emulated_msr)(kvm, MSR_IA32_SMBASE);
> 
> Only MSR_IA32_SMBASE needs to be handled. So, this function is much more
> complicated than it should be.

No, because it's also used by tdx_{g,s}et_msr().