On 28/11/2024 11:50 am, Jan Beulich wrote: > On 28.11.2024 12:10, Andrew Cooper wrote: >> On 28/11/2024 10:31 am, Jan Beulich wrote: >>> On 28.11.2024 01:47, Andrew Cooper wrote: >>>> Xen currently presents APIC_ESR to guests as a simple read/write register. >>>> >>>> This is incorrect. The SDM states: >>>> >>>> The ESR is a write/read register. Before attempt to read from the ESR, >>>> software should first write to it. (The value written does not affect the >>>> values read subsequently; only zero may be written in x2APIC mode.) This >>>> write clears any previously logged errors and updates the ESR with any >>>> errors detected since the last write to the ESR. This write also rearms >>>> the >>>> APIC error interrupt triggering mechanism. >>>> >>>> Introduce a new pending_esr field in hvm_hw_lapic. Update vlapic_error() >>>> to >>>> accumulate errors here, and extend vlapic_reg_write() to discard the >>>> written >>>> value, and instead transfer pending_esr into APIC_ESR. Reads are still as >>>> before. >>>> >>>> Importantly, this means that guests no longer destroys the ESR value it's >>>> looking for in the LVTERR handler when following the SDM instructions. >>>> >>>> Signed-off-by: Andrew Cooper <[email protected]> >>> No Fixes: tag presumably because the issue had been there forever? >> Oh, I forgot to note that. >> >> I can't decide between forever, or since the introduction of the ESR >> support (so Xen 4.5 like XSA-462, and still basically forever). >>>> --- >>>> Slightly RFC. This collides with Alejandro's patch which adds the apic_id >>>> field to hvm_hw_lapic too. However, this is a far more obvious backport >>>> candidate. >>>> >>>> lapic_check_hidden() might in principle want to audit this field, but it's >>>> not >>>> clear what to check. While prior Xen will never have produced it in the >>>> migration stream, Intel APIC-V will set APIC_ESR_ILLREGA above and beyond >>>> what >>>> Xen will currently emulate. >>> The ESR really is an 8-bit value (in a 32-bit register), so checking the >>> upper bits may be necessary. >> It is now, but it may not be in the future. >> >> My concern is that this value is generated by microcode, so we can't >> audit based on which reserved bits we think prior versions of Xen never set. >> >> I don't particularly care about a toolstack deciding to feed ~0 in >> here. But, if any bit beyond 7 gets allocated in the future, then >> auditing the bottom byte would lead to a migration failure of what is in >> practice a correct value. > If a bit beyond zero got allocated, then it being set in an incoming stream > will, for an unaware Xen version, still be illegal. Such a guest simply can't > be migrated to a Xen version unaware of the bit. Once Xen becomes aware, the > auditing would (of course) also need adjustment.
That's the whole point. It's not about Xen's awareness; it's what APIC-V/AVIC might do *in existing configurations* on future hardware without taking a VMExit. If there were no APIC-V support to begin with, this would be easy and auditing would be limited to SENDILL|RECVILL as those are the only two bits Xen knows about. ~Andrew
