Re: [PATCH v8 33/33] KVM: x86/vmx: refactor VMX_DO_EVENT_IRQOFF to generate FRED stack frames

[Sean Christopherson]

On Wed, Apr 12, 2023, Xin3 Li wrote:
> 
> > And then this is equally gross.  Rather than funnel FRED+legacy into a single
> > function only to split them back out, just route FRED into its own asm subroutine.
> > The common bits are basically the creation/destruction of the stack frame and
> > the CALL itself, i.e. the truly interesting bits are what's different.
> 
> I try to catch up with you but am still confused.
> 
> Because a FRED stack frame always contains an error code pushed after RIP,
> the FRED entry code doesn't push any error code.
> 
> Thus I introduced a trampoline code, which is called to have the return
> instruction address pushed first. Then the trampoline code pushes an error
> code (0 for both IRQ and NMI) and jumps to fred_entrypoint_kernel() for NMI
> handling or calls external_interrupt() for IRQ handling.
> 
> The return RIP is used to return from fred_entrypoint_kernel(), but not
> external_interrupt().

...

> > +	/*
> > +	* A FRED stack frame has extra 16 bytes of information pushed at the
> > +	* regular stack top compared to an IDT stack frame.
> > +	*/
> > +	push $0         /* Reserved by FRED, must be 0 */
> > +	push $0         /* FRED event data, 0 for NMI and external interrupts */
> > +	shl $32, %rax
> > +	orq $__KERNEL_DS | $FRED_64_BIT_MODE, %ax
> > +	push %rax	/* Vector (from the "caller") and DS */
> > +
> > +	push %rbp
> > +	pushf
> > +	push \cs_val
> 
> We need to push the RIP of the next instruction here. Or are you suggesting
> we don't need to care about it because it may not be used to return from the
> callee?

...

> > +	push $0 /* FRED error code, 0 for NMI and external interrupts */
> > +	PUSH_REGS
> > +
> > +	/* Load @pt_regs */
> > +	movq    %rsp, %_ASM_ARG1
> > +
> > +	call \call_target

The CALL here would push RIP, I missed/forgot the detail that the error code needs
to be pushed _after_ RIP, not before.

Unless CET complains, there's no need for a trampoline, just LEA+PUSH the return
RIP, PUSH the error code, and JMP to the handler.  IMO, that isn't any weirder than
a trampoline, and it's a bit more obviously weird, e.g. the LEA+PUSH can have a nice
big comment.