On January 28, 2023 3:24:56 AM PST, Alexey Kardashevskiy <aik@xxxxxxx> wrote: > > >On 28/1/23 04:25, Joerg Roedel wrote: >> On Fri, Jan 27, 2023 at 10:56:26PM +1100, Alexey Kardashevskiy wrote: >>> https://github.com/aik/linux/commit/d0d6bbb58fcd927ddd1f8e9d42ab121920c7eafc >> >> Okay, I reproduced the problem here and the root cause turned out to be >> that the compiler moved the DR7 read instruction before the 5-byte NOP >> which becomes the call to sev_es_ist_enter() in SEV-ES guests. This is >> guaranteed to cause #VC exception stack recursion if the NMI was >> triggered on the #VC stack, and that leads to all kinds of undefined >> behavior. > >Cool! > >(out of curiosity) where do you see these NOPs? "objdump -D vmlinux" does not show any, is this after lifepatching? > >Meanwhile, this seems to be doing the right thing: > > >diff --git a/arch/x86/include/asm/debugreg.h b/arch/x86/include/asm/debugreg.h >index b049d950612f..687b15297057 100644 >--- a/arch/x86/include/asm/debugreg.h >+++ b/arch/x86/include/asm/debugreg.h >@@ -39,7 +39,7 @@ static __always_inline unsigned long native_get_debugreg(int regno) > asm("mov %%db6, %0" :"=r" (val)); > break; > case 7: >- asm("mov %%db7, %0" :"=r" (val)); >+ asm volatile ("mov %%db7, %0" :"=r" (val)); > > > It's somewhat odd to me that reading %dr7 is volatile, but %dr6 is not... %dr6 is the status register! I believe they should all be volatile (the compiler semantics is that volatile operations are always executed exactly once, in strict program order with respect to any other volatile operations); the real question is if there should also be memory clobbers on %dr6 reads and any %dr write.
