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

[Jan Beulich <jbeulich@xxxxxxxx>]

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.

> and the current vLAPIC implementations allows guest to write to these RO bits.
> 
> 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.
> 
> Hence it is definitely wrong to allow guest to write to LVTx regs RO bits,
> fix it by unconditionally cleaning up those bits in vlapic_reg_write().
> 
> Signed-off-by: Grygorii Strashko <grygorii_strashko@xxxxxxxx>

Please also add a Fixes: tag when correcting code.

> --- 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[]. (Orthogonal to this I wonder whether
guest_wrmsr_x2apic() wouldn't better use that array, too.)

While looking at this, don't we have an issue with CMCI as well?
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 ...

Jan