Xen project Mailing List

Re: [XEN PATCH v1 2/7] x86/intel: guard vmce_has_lmce() with INTEL option

From: Sergiy Kibrik <sergiy_kibrik@xxxxxxxx>

Date: Tue, 30 Apr 2024 12:42:23 +0300

Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=epam.com; dmarc=pass action=none header.from=epam.com; dkim=pass header.d=epam.com; arc=none

Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=ditHuX6z+IfdxOLMjl/YFMyXeFgvG7YgOVRNLPoIams=; b=kPAZGMuQPgBiHqtgKqde4aWRlmITCXH8CAI28J+0GcMl8GHzYH6MidMm5nytt3pGJRowDWwVlAKBI4bvc00LMMrWPaqXdJG4q9TPZ4TsdfH4pZn1+xLQLwwo9rUU5C4696iuE0QtoT0BBWRIyrxwjsuHH4ZWYNZhu3Tp3ytEL8ADHQcK4ESIT9nXDLgiPxZPIu5n/HahRLlzM312+7XfiCAFvTOEskKFj/0WXOaBJPxZlaFXcoKCJp8EcUFcNxjCe4dKVT/Ykb7YoHPUnfjKdgGt+X0g0owHGLHYzOWJtL6rrFVtPlF52ln3hyHqWl8etUBrV+zTVwu6OJONGB/O2w==

Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=J61Gjg1gZDUrbCWJUZ/trS8saNJYBaZGbP9/ix0HzKMrGUM6ygmaXZ4XVdw/EA+Z+WH9iKRYR+nvZ/dhSDwU90yh+M5ij6+qYHFP4K8ii3XHtkPqgvRsWgSiq1Acxk2A1e0sBhOFHMgBPO0CrrVsAEkRvtTJeeN30csw+BuhUX20Nf6ETKxVbuL1W3sNNvp/XRJIR75fUN33nJFiOxTOewA2dQCnUbXMlCnfgDcI1iCNEXxDge4XwHzOZPgPSOxUmiR6k+uLZysknvIJJoj5sU1dxwTmfusuS1mWTqNSTYlfkpqBnQ5GiGnkc0rH2Nv+mLsmfSX1aIHnYgWuyFOuVg==

Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxxx

Delivery-date: Tue, 30 Apr 2024 09:42:51 +0000

List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

29.04.24 18:34, Jan Beulich:

On 23.04.2024 10:50, Sergiy Kibrik wrote:

Since MCG_LMCE_P bit is specific to Intel CPUs


That's the case now. It could change going forward, and an underlying hypervisor
might also have (obscure?) reasons to surface it elsewhere.

the code to check it can
possibly be excluded from build if !CONFIG_INTEL. With these guards
calls to vmce_has_lmce() are eliminated and mce_intel.c can end up
not being built.

Also replace boilerplate code that checks for MCG_LMCE_P flag with
vmce_has_lmce(), which might contribute to readability a bit.


Alternatively, have you considered making that function an inline one in a
suitable header? Besides addressing your build issue (I think), ...

--- a/xen/arch/x86/cpu/mcheck/vmce.c
+++ b/xen/arch/x86/cpu/mcheck/vmce.c
@@ -199,7 +199,7 @@ int vmce_rdmsr(uint32_t msr, uint64_t *val)
           * bits are always set in guest MSR_IA32_FEATURE_CONTROL by Xen, so it
           * does not need to check them here.
           */
-        if ( cur->arch.vmce.mcg_cap & MCG_LMCE_P )
+        if ( IS_ENABLED(CONFIG_INTEL) && vmce_has_lmce(cur) )


... doing so would alternatively also permit integrating the IS_ENABLED()
into the function, rather than repeating the same ...

@@ -324,7 +324,7 @@ int vmce_wrmsr(uint32_t msr, uint64_t val)
          break;

case MSR_IA32_MCG_EXT_CTL:

-        if ( (cur->arch.vmce.mcg_cap & MCG_LMCE_P) &&
+        if ( IS_ENABLED(CONFIG_INTEL) && vmce_has_lmce(cur) &&
               !(val & ~MCG_EXT_CTL_LMCE_EN) )
              cur->arch.vmce.mcg_ext_ctl = val;
          else
--- a/xen/arch/x86/msr.c
+++ b/xen/arch/x86/msr.c
@@ -86,7 +86,7 @@ int guest_rdmsr(struct vcpu *v, uint32_t msr, uint64_t *val)
              goto gp_fault;

*val = IA32_FEATURE_CONTROL_LOCK;

-        if ( vmce_has_lmce(v) )
+        if ( IS_ENABLED(CONFIG_INTEL) && vmce_has_lmce(v) )
              *val |= IA32_FEATURE_CONTROL_LMCE_ON;
          if ( cp->basic.vmx )
              *val |= IA32_FEATURE_CONTROL_ENABLE_VMXON_OUTSIDE_SMX;


... three times.

I think I'll move vmce_has_lmce() to arch/x86/cpu/mcheck/mce.h then, if

no objections.

-Sergiy

©2013 Xen Project, A Linux Foundation Collaborative Project. All Rights Reserved.
Linux Foundation is a registered trademark of The Linux Foundation.
Xen Project is a trademark of The Linux Foundation.