On 20.05.2024 11:32, Sergiy Kibrik wrote:
> 16.05.24 12:39, Jan Beulich:
>> On 14.05.2024 10:20, Sergiy Kibrik wrote:
>>> Moving this function out of mce_intel.c would make it possible to disable
>>> build of Intel MCE code later on, because the function gets called from
>>> common x86 code.
>>
>> Why "would"? "Will" or "is going to" would seem more to the point to me.
>
> yes, sure
>
>> But anyway.
>>
>>> --- a/xen/arch/x86/cpu/mcheck/mce.h
>>> +++ b/xen/arch/x86/cpu/mcheck/mce.h
>>> @@ -170,6 +170,11 @@ static inline int mce_bank_msr(const struct vcpu *v,
>>> uint32_t msr)
>>> return 0;
>>> }
>>>
>>> +static inline bool vmce_has_lmce(const struct vcpu *v)
>>> +{
>>> + return v->arch.vmce.mcg_cap & MCG_LMCE_P;
>>> +}
>>
>> Is there a particular reason this is placed here, rather than ...
>>
>>> --- a/xen/arch/x86/include/asm/mce.h
>>> +++ b/xen/arch/x86/include/asm/mce.h
>>> @@ -41,7 +41,6 @@ extern void vmce_init_vcpu(struct vcpu *v);
>>> extern int vmce_restore_vcpu(struct vcpu *v, const struct hvm_vmce_vcpu
>>> *ctxt);
>>> extern int vmce_wrmsr(uint32_t msr, uint64_t val);
>>> extern int vmce_rdmsr(uint32_t msr, uint64_t *val);
>>> -extern bool vmce_has_lmce(const struct vcpu *v);
>>> extern int vmce_enable_mca_cap(struct domain *d, uint64_t cap);
>>
>> ... in the file the declaration was in, thus avoiding ...
>>
>>> --- a/xen/arch/x86/msr.c
>>> +++ b/xen/arch/x86/msr.c
>>> @@ -24,6 +24,8 @@
>>>
>>> #include <public/hvm/params.h>
>>>
>>> +#include "cpu/mcheck/mce.h"
>>
>> ... the need for such a non-standard, cross-directory #include?
>>
>
>
> This is because MCG_LMCE_P is defined in arch/x86/cpu/mcheck/x86_mca.h
> -- so either MCG_LMCE_P (+ a bunch of MCG_* declarations) has to be
> moved to common header to be accessible, or local x86_mca.h got to be
> included from common arch/x86/include/asm/mce.h.
>
> As for the MCG_* declarations movement I didn't think there's a good
> enough reason to do it; as for the inclusion of x86_mca.h it didn't look
> nice at all.
I'm afraid I don't follow the latter: Why's including x86_mca.h any worse
than what you do right now?
Jan
