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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 18.12.2025 16:32, Grygorii Strashko wrote:
> On 30.09.25 22:05, 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.
>>
>> The Delivery Status (DS) is not emulated by Xen - there is no IRQ msg bus,
>> and the IRQ is:
>> - or accepted at destination and appears as pending
>>    (vLAPIC Interrupt Request Register (IRR))
>> - or get rejected immediately.
>>
>> The Remote IRR Flag (RIR) behavior emulation is not implemented for
>> LINT0/LINT1 in Xen for now.
>>
>> The current vLAPIC implementations allows guest to write to these RO bits.
>>
>> The vLAPIC LVTx registers write happens in vlapic_reg_write() which expect
>> to implement "Write ignore" access type for RO bits by applying masks from
>> vlapic_lvt_mask[], but vlapic_lvt_mask[] contains incorrect masks which
>> allows writing to RO fields.
>>
>> Hence it is definitely wrong to allow guest to write to LVTx regs RO bits,
>> fix it by fixing LVTx registers masks in vlapic_lvt_mask[].
>>
>> In case of WRMSR (guest_wrmsr_x2apic()) access to LVTx registers, the SDM
>> clearly defines access type for "Reserved" bits as RsvdZ (Non-zero writes
>> to reserved bits should cause #GP exception), but contains no statements
>> for RO bits except that they are not "Reserved". So, guest_wrmsr_x2apic()
>> now uses different masks (than vlapic_reg_write()) for checking LVTx
>> registers values for "Reserved" bit settings, which include RO bits and
>> do not cause #GP exception.
>>
>> Fixes: d1bd157fbc9b ("Big merge the HVM full-virtualisation abstractions.")
>> Signed-off-by: Grygorii Strashko <grygorii_strashko@xxxxxxxx>
>> ---
>> Changes in v2:
>> - masks fixed in vlapic_lvt_mask[]
>> - commit msg reworded
>>
>> v1: 
>> https://patchwork.kernel.org/project/xen-devel/patch/20250925195558.519568-1-grygorii_strashko@xxxxxxxx/>
>>    xen/arch/x86/hvm/vlapic.c | 14 ++++++++------
>>   1 file changed, 8 insertions(+), 6 deletions(-)
>>
>> diff --git a/xen/arch/x86/hvm/vlapic.c b/xen/arch/x86/hvm/vlapic.c
>> index 79697487ba90..2ecba8163f48 100644
>> --- a/xen/arch/x86/hvm/vlapic.c
>> +++ b/xen/arch/x86/hvm/vlapic.c
>> @@ -44,15 +44,17 @@
>>   static const unsigned int vlapic_lvt_mask[VLAPIC_LVT_NUM] =
>>   {
>>        /* LVTT */
>> -     LVT_MASK | APIC_TIMER_MODE_MASK,
>> +     (LVT_MASK | APIC_TIMER_MODE_MASK) & ~APIC_SEND_PENDING,
>>        /* LVTTHMR */
>> -     LVT_MASK | APIC_DM_MASK,
>> +     (LVT_MASK | APIC_DM_MASK) & ~APIC_SEND_PENDING,
>>        /* LVTPC */
>> -     LVT_MASK | APIC_DM_MASK,
>> -     /* LVT0-1 */
>> -     LINT_MASK, LINT_MASK,
>> +     (LVT_MASK | APIC_DM_MASK) & ~APIC_SEND_PENDING,
>> +     /* LVT0 */
>> +     LINT_MASK & ~(APIC_LVT_REMOTE_IRR | APIC_SEND_PENDING),
>> +     /* LVT1 */
>> +     LINT_MASK & ~(APIC_LVT_REMOTE_IRR | APIC_SEND_PENDING),
>>        /* LVTERR */
>> -     LVT_MASK
>> +     LVT_MASK & ~APIC_SEND_PENDING,
>>   };
>>   
>>   #define vlapic_lvtt_period(vlapic)                              \
> 
> I'd like to return back here and finally get this issue fixed (and make tests 
> green again).
> 
> So could this simple fix be merged?
> 
> There is also follow up patch [1] which can be still a "follow up" patch or 
> can be made prerequisite patch.
> 
> [XEN] x86: hvm: vlapic: rework WR/rsvdz masks for LVTx regs
> [1] 
> https://patchwork.kernel.org/project/xen-devel/patch/20251009114249.1964387-1-grygorii_strashko@xxxxxxxx/

I was really intending for "x86/vLAPIC: CMCI LVT handling" to go first. Which
obviously requires someone to review it ...

Jan