>>> On 16.03.18 at 15:29, <andrew.coop...@citrix.com> wrote: > On 16/03/18 14:13, Jan Beulich wrote: >> Commit 9d1d31ad94 ("x86: slightly reduce Meltdown band-aid overhead") >> moved the STI past the PUSHF. While this isn't an active problem (as we >> force EFLAGS.IF to 1 before exiting to guest context), let's not risk >> internal confusion by finding a PV guest frame with interrupts >> apparently off. >> >> Signed-off-by: Jan Beulich <jbeul...@suse.com> >> >> --- a/xen/arch/x86/x86_64/entry.S >> +++ b/xen/arch/x86/x86_64/entry.S >> @@ -281,6 +281,8 @@ GLOBAL(sysenter_eflags_saved) >> /* WARNING! `ret`, `call *`, `jmp *` not safe before this point. */ >> >> GET_STACK_END(bx) >> + /* PUSHF above has saved EFLAGS.IF clear (the caller had it set). */ >> + orl $X86_EFLAGS_IF, UREGS_eflags(%rsp) > > For the sake of a single or (which would be beside a line of adjacent > stack accesses anyway), I think it would be better to have this > immediately after sysenter_eflags_saved. It doesn't have an impact on > speculation safety, and can't plausibly be impacted by SMAP.
Well, I had considered that, but that'll be yet one more separate place to NOP out later on. > It is perhaps not very important, but is it worth encoding this as: > > orb $(X86_EFLAGS_IF >> 8), UREGS_eflags+1(%rsp) > > We have a similar pattern when testing the interrupt flag. Aren't back to back different size writes to the same location recommended against? Then again, the push is a qword write already anyway, followed by (currently) a dword write. I can certainly do that. But let's first agree on the placement. > Somewhat independently of this patch, I think we should assert that > flags are in the expected state in the return-to-guest path, so we > notice accidental breakage like this more easily. Not sure - nothing was broken here afaict, we just want to play safe. And as said the exit paths already force EFLAGS.IF to 1. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel