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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 14.10.2025 21:36, Andrew Cooper wrote:
> On 08/10/2025 1:09 pm, Jan Beulich wrote:
>> --- a/xen/arch/x86/hvm/vlapic.c
>> +++ b/xen/arch/x86/hvm/vlapic.c
>> @@ -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) )
>> +            return X86EMUL_EXCEPTION;
>> +        break;
>> +    }
>>  
>>      *val = high | vlapic_read_aligned(vlapic, offset);
>>  
>> @@ -868,6 +881,10 @@ void vlapic_reg_write(struct vcpu *v, un
>>          vlapic_set_reg(vlapic, APIC_ICR2, val & 0xff000000U);
>>          break;
>>  
>> +    case APIC_CMCI:         /* LVT CMCI */
>> +        if ( !(v->arch.vmce.mcg_cap & MCG_CMCI_P) )
>> +            break;
>> +        fallthrough;
>>      case APIC_LVTT:         /* LVT Timer Reg */
>>          if ( vlapic_lvtt_tdt(vlapic) !=
>>               ((val & APIC_TIMER_MODE_MASK) == APIC_TIMER_MODE_TSC_DEADLINE) 
>> )
>> @@ -1024,9 +1041,12 @@ int guest_wrmsr_x2apic(struct vcpu *v, u
>>              return X86EMUL_EXCEPTION;
>>          break;
>>  
>> +    case APIC_CMCI:
>> +        if ( !(v->arch.vmce.mcg_cap & MCG_CMCI_P) )
>> +            return X86EMUL_EXCEPTION;
>> +        fallthrough;
>>      case APIC_LVTTHMR:
>>      case APIC_LVTPC:
>> -    case APIC_CMCI:
>>          if ( val & ~(LVT_MASK | APIC_DM_MASK) )
>>              return X86EMUL_EXCEPTION;
>>          break;
> 
> This is almost certainly not how real hardware behaves.
> 
> The APIC is a discrete block of logic, whether it's integrated into the
> core or not.  A new LVT is "just" another interrupt source, and if
> nothing is wired into that pin, then it's just a register which never
> produces an interrupt.
> 
> Accessibility of LVT_CMCI will depend on MAXLVT and nothing else.  In
> silicon, I'm pretty sure it will be hardcoded as fully absent or
> present, because I can't see any reason to make this configurable.
> 
> At this point, things get more complicated.
> 
> On Intel, there's no such thing as x2APIC capable (irrespective of
> x2APIC enabled) without LVT_CMCI which is why there are no additional
> access constraints on the register.
> 
> On AMD, there's no LVT_CMCI even on systems which support x2APIC. 
> Instead, ELVTs are used and it is MCE-configuration based which ELVT the
> interrupt is delivered through.
> 
> Choosing a default MAXLVT based on MCG_CMCI_P is probably fine (although
> it certainly is ugly to tie APIC and vMCE together), but controls on the
> access to APIC_CMCI should be based on MAXLVT.

As you ask for it, I can certainly do so. I didn't because the way it's
done now the checks are cheaper overall.

Jan