Re: [PATCH for-4.21??? 3/3] x86/vLAPIC: properly support the CMCI LVT

[Jan Beulich <jbeulich@xxxxxxxx>]

On 14.10.2025 16:12, Grygorii Strashko wrote:
> 
> 
> On 09.10.25 18:37, Jan Beulich wrote:
>> On 09.10.2025 17:08, Grygorii Strashko wrote:
>>> On 08.10.25 15:09, Jan Beulich wrote:
>>>> --- a/xen/arch/x86/hvm/vlapic.c
>>>> +++ b/xen/arch/x86/hvm/vlapic.c
>>>> @@ -31,10 +31,13 @@
>>>>    #include <public/hvm/ioreq.h>
>>>>    #include <public/hvm/params.h>
>>>>    
>>>> -#define LVT_BIAS(reg)                   (((reg) - APIC_LVTT) >> 4)
>>>> +#include <../cpu/mcheck/x86_mca.h> /* MCG_CMCI_P */
>>>
>>> This include... You probably do not like it also
>>> It is dependency outside HVM code.
>>>
>>> I've been thinking about something like vlapic->caps which can be filed 
>>> before vlapic_init()
>>> or passed as parameter, but seems x86 toolstack is considered to be able 
>>> overwrite anything,
>>> including v->arch.vmce.
>>>
>>> Seems, no better options here.
>>
>> Same here, hence why I used it despite not liking it.
>>
>>>> @@ -697,8 +701,17 @@ int guest_rdmsr_x2apic(const struct vcpu
>>>>            return X86EMUL_EXCEPTION;
>>>>    
>>>>        offset = reg << 4;
>>>> -    if ( offset == APIC_ICR )
>>>> +    switch ( offset )
>>>> +    {
>>>> +    case APIC_ICR:
>>>>            high = (uint64_t)vlapic_read_aligned(vlapic, APIC_ICR2) << 32;
>>>> +        break;
>>>> +
>>>> +    case APIC_CMCI:
>>>> +        if ( !(v->arch.vmce.mcg_cap & MCG_CMCI_P) )
>>>
>>> Could it be done using wrapper, like vmce_has_cmci()?
>>> As this is Intel specific it's candidate to be opt-out eventually.
>>
>> Possible. I wanted to limit the churn, hence why I preferred not to introduce
>> a wrapper. Such an abstraction I wouldn't like to be a function taking a 
>> vCPU;
>> really this should be a domain property imo.
> 
> My intention was to limit spreading direct access to "vmce" data over vlapic 
> code:
> 
> static bool vlapic_has_cmci(const struct vcpu *v)
> {
>       return v->arch.vmce.mcg_cap & MCG_CMCI_P;
> }

"vlapic" in the name makes it seemingly better, but not really. As before: I
think such a predicate should be taking a const struct domain * as input.
Unless of course we expected that different vCPU-s in a guest may have
different settings of the MCG_CMCI_P bit.

Jan