On 18.12.2025 16:32, Grygorii Strashko wrote: > On 30.09.25 22:05, Grygorii Strashko wrote: >> From: Grygorii Strashko <[email protected]> >> >> 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 <[email protected]> >> --- >> Changes in v2: >> - masks fixed in vlapic_lvt_mask[] >> - commit msg reworded >> >> v1: >> https://patchwork.kernel.org/project/xen-devel/patch/[email protected]/> >> 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/[email protected]/
I was really intending for "x86/vLAPIC: CMCI LVT handling" to go first. Which obviously requires someone to review it ... Jan
