[gPXE-devel] [PATCH] [comboot] Propagate carry flag from COMBOOT API

[H. Peter Anvin]

On 04/27/2010 11:03 PM, Stefan Hajnoczi wrote:
> COMBOOT API calls set the carry flag on failure.  This was not being
> propagated because the COMBOOT interrupt handler used iret to return
> with EFLAGS restored from the stack.  This patch uses a far return that
> discards the stale EFLAGS value from the stack instead.
> 
> Reported-by: Geoff Lywood <glywood at vmware.com>
> Signed-off-by: Stefan Hajnoczi <stefanha at gmail.com>
> ---
> This patch has been tested and reviewed in the issue tracker:
> http://support.etherboot.org/index.php?do=details&task_id=77
> 
>  src/arch/i386/interface/syslinux/comboot_call.c |    8 ++++----
>  1 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/src/arch/i386/interface/syslinux/comboot_call.c b/src/arch/i386/interface/syslinux/comboot_call.c
> index 51a1b6d..2ce39aa 100644
> --- a/src/arch/i386/interface/syslinux/comboot_call.c
> +++ b/src/arch/i386/interface/syslinux/comboot_call.c
> @@ -669,7 +669,7 @@ void hook_comboot_interrupts ( ) {
>  		              "pushw %%cs\n\t"
>  		              "call prot_call\n\t"
>  		              "addw $4, %%sp\n\t"
> -		              "iret\n\t" )
> +		              "lret $2\n\t" )
>  		          : : "i" ( int20 ) );
>  
>  	hook_bios_interrupt ( 0x20, ( unsigned int ) int20_wrapper,
> @@ -681,7 +681,7 @@ void hook_comboot_interrupts ( ) {
>  		              "pushw %%cs\n\t"
>  		              "call prot_call\n\t"
>  		              "addw $4, %%sp\n\t"
> -		              "iret\n\t" )
> +		              "lret $2\n\t" )
>  		          : : "i" ( int21 ) );
>  
>  	hook_bios_interrupt ( 0x21, ( unsigned int ) int21_wrapper,
> @@ -693,8 +693,8 @@ void hook_comboot_interrupts ( ) {
>  		              "pushw %%cs\n\t"
>  		              "call prot_call\n\t"
>  		              "addw $4, %%sp\n\t"
> -		              "iret\n\t" )
> -		          : : "i" ( int22) );
> +		              "lret $2\n\t" )
> +		          : : "i" ( int22 ) );
>  
>  	hook_bios_interrupt ( 0x22, ( unsigned int ) int22_wrapper,
>  	                      &int22_vector );

NEVER EVER[*] use "lret $2" to return from an interrupt handler!!!!  It
means all flags are dropped, **INCLUDING IF**!

I fixed this bug in gPXE once before already (in another spot).  It
doesn't really amuse me to see more places do this.

Instead, use setc to modify the bottom byte of the flag value on the
stack and return with iret.

	-hpa