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
