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

[Grygorii Strashko <grygorii_strashko@xxxxxxxx>]

On 26.09.25 13:52, Jan Beulich wrote:
> 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).
Sorry, ignore ^ sentence.

> 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.
Above reflects current code - guest_wrmsr_x2apic() allows valid RO bits,
then vlapic_reg_write() ignores (W/I) them.
That's why masks are different between guest_wrmsr_x2apic and vlapic_reg_write.

> > > 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:
it will end up calling vlapic_reg_write() which doesn't have case statement for
APIC_CMCI - write ignored.

> > > 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.
Looking at:

 #define VLAPIC_VERSION                  0x00050014

which means "Max LVT Entries" = 6 (5+1)

Looking at linux kernel apic.c:
#ifdef CONFIG_X86_MCE_INTEL
        if (maxlvt >= 6) {
                v = apic_read(APIC_LVTCMCI);
                if (!(v & APIC_LVT_MASKED))
                        apic_write(APIC_LVTCMCI, v | APIC_LVT_MASKED);
        }
#endif

Which means Xen never really emulated APIC_CMCI, so wouldn't it be correct to 
just drop APIC_CMCI?
Also, taking into account that it's Intel specific.

--
Best regards,
-grygorii