Re: [XEN][PATCH] x86/hvm: vlapic: fix RO bits emulation in LVTx regs

[Jan Beulich <jbeulich@xxxxxxxx>]

On 26.09.2025 12:38, Grygorii Strashko wrote:
> On 26.09.25 11:17, Jan Beulich wrote:
>> On 25.09.2025 21:55, Grygorii Strashko wrote:
>>> From: Grygorii Strashko <grygorii_strashko@xxxxxxxx>
>>>
>>> The LAPIC LVTx registers have two RO bits:
>>> - all: Delivery Status (DS) bit 12
>>> - LINT0/LINT1: Remote IRR Flag (RIR) bit 14.
>>>    This bit is reserved for other LVTx regs with RAZ/WI access type (MMIO), 
>>> while
>>>    WRMSR (guest_wrmsr_x2apic()) has appropiate checks for reserved bits
>>>    (MBZ access type).
>>
>> Question is what the behavior is for writing the r/o (but not reserved) bits.
>> I wasn't able to find any statement in the SDM.
> 
> Me too. Usually RO/WI on most HW.
> For example, LAPIC MMIO "Write" will be ignored (WRMSR will trigger 
> exception).

My remark was specifically about WRMSR, and what you say here contradicts ...

>>> --- a/xen/arch/x86/hvm/vlapic.c
>>> +++ b/xen/arch/x86/hvm/vlapic.c
>>> @@ -880,6 +880,7 @@ void vlapic_reg_write(struct vcpu *v, unsigned int reg, 
>>> uint32_t val)
>>>           if ( vlapic_sw_disabled(vlapic) )
>>>               val |= APIC_LVT_MASKED;
>>>           val &= array_access_nospec(vlapic_lvt_mask, (reg - APIC_LVTT) >> 
>>> 4);
>>> +        val &= ~(APIC_LVT_REMOTE_IRR | APIC_SEND_PENDING);
>>
>> There shouldn't be a 2nd &= here; what needs adding should imo be added to
>> (really: removed from) vlapic_lvt_mask[].
> 
> I'll try it.
> 
>   (Orthogonal to this I wonder whether
>> guest_wrmsr_x2apic() wouldn't better use that array, too.)
> 
> WRMSR checks for MBZ. RO bits are not MBZ, so masks are different.

... what you say here.

>> While looking at this, don't we have an issue with CMCI as well?
> 
> I see no APIC_CMCI write emulation. only read.

guest_wrmsr_x2apic() has

    case APIC_CMCI:

>> guest_{rd,wr}msr_x2apic() handle it, but vlapic_reg_write() doesn't. I.e. on
>> AMD we would fail to deliver #GP when the guest accesses it, while on Intel
>> we would lose the value written. And we also don't set its mask bit in
>> vlapic_do_init(). I guess I need to make a patch ...
> 
> Is'n it depends on CMCI capability exposing to guest?

Yes, that's part of what I was (effectively) saying.

> (have no idea what's CMCI :)

Corrected Machine Check Interrupt.

Jan