[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [XEN][PATCH] x86/hvm: vlapic: fix RO bits emulation in LVTx regs
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). 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. This is veeery old code. Probably f7c8af3a6476e ("[XEN] HVM: Clean up and simplify vlapic device-model code") or 50b3cef2eecb0 ("[HVM] Place all APIC registers into one page in native format.") --- 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. While looking at this, don't we have an issue with CMCI as well? I see no APIC_CMCI write emulation. only read. 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? (have no idea what's CMCI :) -- Best regards, -grygorii
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |