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 <andrew.coop...@citrix.com> >> 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. Jan
