Re: [PATCH 2/3] x86/entry: Use POP_GPRS and remove RESTORE_ALL

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 07/07/2026 8:03 am, Jan Beulich wrote:
> On 06.07.2026 17:34, Andrew Cooper wrote:
>> POP_GPRS is shorter than RESTORE_ALL in terms of emitted code.
>>
>> By separating the popping of entry_vector/error_code off the stack,
>> SPEC_CTRL_COND_VERW doesn't need custom displacements.
>>
>> Get rid of the compat=1 special case for PV32.  It's not obviously a win, and
>> PV32 is getting increasingly rare these days.
>>
>> No functional change.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

Thanks.

> as-is, however ...
>
>> --- a/xen/arch/x86/x86_64/compat/entry.S
>> +++ b/xen/arch/x86/x86_64/compat/entry.S
>> @@ -160,12 +160,11 @@ FUNC(compat_restore_all_guest)
>>          /* WARNING! `ret`, `call *`, `jmp *` not safe beyond this point. */
>>          SPEC_CTRL_EXIT_TO_PV    /* Req: a=spec_ctrl %rsp=regs/cpuinfo, 
>> Clob: cd */
>>  
>> -        RESTORE_ALL adj=8, compat=1
>> +        POP_GPRS
>>  
>> -        /* Account for ev/ec having already been popped off the stack. */
>> -        SPEC_CTRL_COND_VERW \
>> -            scf=STK_REL(CPUINFO_scf,      CPUINFO_rip), \
>> -            sel=STK_REL(CPUINFO_verw_sel, CPUINFO_rip)
>> +        SPEC_CTRL_COND_VERW     /* Req: %rsp=eframe                    
>> Clob: efl */
>> +
>> +        add     $8, %rsp        /* Pop ev/ec off the stack */
>>  
>>          jmp     iret_to_guest
>>  END(compat_restore_all_guest)
>> --- a/xen/arch/x86/x86_64/entry.S
>> +++ b/xen/arch/x86/x86_64/entry.S
>> @@ -226,7 +226,8 @@ FUNC_LOCAL(restore_all_guest)
>>          /* WARNING! `ret`, `call *`, `jmp *` not safe beyond this point. */
>>          SPEC_CTRL_EXIT_TO_PV    /* Req: a=spec_ctrl %rsp=regs/cpuinfo, 
>> Clob: cd */
>>  
>> -        RESTORE_ALL
>> +        POP_GPRS
>> +
>>          BUILD_BUG_ON(TRAP_syscall & 0xff)
>>          testb $TRAP_syscall >> 8, EFRAME_entry_vector + 1(%rsp)
>>          jz    iret_exit_to_guest
>> @@ -753,20 +754,17 @@ UNLIKELY_END(exit_cr3)
>>          /* WARNING! `ret`, `call *`, `jmp *` not safe beyond this point. */
>>          SPEC_CTRL_EXIT_TO_XEN /* Req: %r12=ist_exit %r14=end %rsp=regs, 
>> Clob: abcd */
>>  
>> -        RESTORE_ALL adj=8
>> +        POP_GPRS
>>  
>>          /*
>>           * When the CPU pushed this exception frame, it zero-extended 
>> eflags.
>>           * For an IST exit, SPEC_CTRL_EXIT_TO_XEN stashed shadow copies of
>>           * scf and ver_sel above eflags, as we can't use any GPRs,
>>           * and we're at a random place on the stack, not in a CPUFINFO 
>> block.
>> -         *
>> -         * Account for ev/ec having already been popped off the stack.
>>           */
>> -        SPEC_CTRL_COND_VERW \
>> -            scf=STK_REL(EFRAME_shadow_scf, EFRAME_rip), \
>> -            sel=STK_REL(EFRAME_shadow_sel, EFRAME_rip)
>> +        SPEC_CTRL_COND_VERW     /* Req: %rsp=eframe                    
>> Clob: efl */
>>  
>> +        add     $8, %rsp        /* Pop ev/ec off the stack */
>>          iretq
>>  END(restore_all_xen)
> ... both uses of SPEC_CTRL_COND_VERW's with arguments go away. Shouldn't
> the macro thus also be pruned of its parameters?

Not in this patch.

~Andrew