On 23/09/2024 2:03 pm, Jan Beulich wrote: > On 23.09.2024 12:14, Alejandro Vallejo wrote: >> On Fri Sep 20, 2024 at 3:12 PM BST, Roger Pau Monné wrote: >>> On Wed, Sep 18, 2024 at 02:05:54PM +0100, Alejandro Vallejo wrote: >>>> Moves sti directly after the cr2 read and immediately after the #PF >>>> handler. >>> I think you need to add some context about why this is needed, iow: >>> avoid corrupting %cr2 if a nested 3PF happens. >> I can send a v3 with: >> >> ``` >> Hitting a page fault clobbers %cr2, so if a page fault is handled while >> handling a previous page fault then %cr2 will hold the address of the >> latter >> fault rather than the former. This patch makes the page fault path delay >> re-enabling IRQs until %cr2 has been read in order to ensure it stays >> consistent. > And under what conditions would we experience #PF while already processing > an earlier #PF? If an interrupt kicks in, that's not supposed to by raising > any #PF itself. Which isn't to say that the change isn't worthwhile to make, > but it would be nice if it was explicit whether there are active issues, or > whether this is merely to be on the safe side going forward.
My understanding is that this came from code inspection, not an active issue. The same is true for %dr6 and #DB, and MSR_XFD_ERR and #NM. I think we can safely agree to veto the use of AMX in the #NM handler, and IST exceptions don't re-enable interrupts[1], so #PF is the only problem case. Debug keys happen off the back of plain IRQs, and we can get #PF when interrogating guest stacks. Also, I'm far from certain we're safe to spurious #PF's from updating Xen mappings, so I think there are a bunch of risky corner cases that we might be exposed to. And I really need to find some time to get FRED working... ~Andrew [1] We do re-enable interrupts in order to IPI cpu0 for "clean" shutdown, and this explodes in our faces if kexec isn't active and we crashed in the middle of context switch. We really need to not need irqs-on in order to shut down.
