On 26.11.2024 21:07, Andrew Cooper wrote: > On 26/11/2024 5:06 pm, Javi Merino wrote: >> The logic to read the APIC_ESR was copied from linux in a commit from >> 2002: 4676bbf96dc8 (bitkeeper revision >> 1.2 (3ddb79c9KusG02eh7i-uXkgY0IksKA), 2002-11-20). In linux 3.14, >> this logic was fixed to follow the Intel SDM (see commit >> 60283df7ac26 (x86/apic: Read Error Status Register correctly, >> 2014-01-14) in the linux kernel). The Intel(r) 64 and IA-32 >> Architectures Software Develover's Manual currently says >> in Volume 3, Section 12.5.3: >> >> 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. >> >> Update error_interrupt() to remove the first read and follow the Intel >> manual. >> >> Signed-off-by: Javi Merino <[email protected]> > > In Linux, this bugfix was further corrected with > https://lore.kernel.org/lkml/[email protected]/ > > However, Xen being 64-bit only doesn't care about the Pentium 3AP errata > with writing to ESR. > > I'm tempted to take this patch as-is, then do a followup on top to > remove the remnants of the Pentium errata from Xen. I don't think it's > interesting to take bugfixes to bugfixes simply to delete them right after.
Hmm. I think the adjustment done here is actually removing part of the erratum workaround, and hence ought to be removed in one go by your patch. The double read there is - aiui - to be on the safe side wrt that erratum, i.e. to cover the (at that time) potential case that the erratum would also be present on CPUs with more than 3 LVTs. In any event the comment ahead of the code being touched ought to be removed as well, imo. Jan
