Re: [PATCH v1 12/14] x86/boot: tweak a20.c for better code generation

[David Laight <david.laight.linux@gmail.com>]

On Tue, 20 Jan 2026 11:54:04 -0800
"H. Peter Anvin" <hpa@zytor.com> wrote:

> Do some minor tweaks to arch/x86/boot/a20.c for better code
> generation, made possible by the __seg_fs/__seg_gs changes.
> 
> Move the die() call to a20.c itself; there is no reason to push an
> error code upwards just to die() there.

Can't this code just do:
	sv = *0000:addr;
	val = *ffff:addr+10;
	if (sv != val)
		return 1;
	*0000:addr = ~val;
	io_delay();
	val ^= *ffff:addr+10;
	*0000:addr = sv;
	return val != ~0;

That inverts all bits, inverting one is enough.
No loops needed.

	David

> 
> Signed-off-by: H. Peter Anvin (Intel) <hpa@zytor.com>
> ---
>  arch/x86/boot/a20.c  | 24 +++++++++++-------------
>  arch/x86/boot/boot.h |  2 +-
>  arch/x86/boot/pm.c   |  3 +--
>  3 files changed, 13 insertions(+), 16 deletions(-)
> 
> diff --git a/arch/x86/boot/a20.c b/arch/x86/boot/a20.c
> index 52c3fccdcb70..38a1cad8a553 100644
> --- a/arch/x86/boot/a20.c
> +++ b/arch/x86/boot/a20.c
> @@ -53,24 +53,22 @@ static int empty_8042(void)
>  
>  static int a20_test(int loops)
>  {
> -	int ok = 0;
>  	int saved, ctr;
>  
>  	set_fs(0xffff);
>  
>  	saved = ctr = rdgs32(A20_TEST_ADDR);
>  
> -	while (loops--) {
> +	do {
>  		wrgs32(++ctr, A20_TEST_ADDR);
>  		io_delay();	/* Serialize and make delay constant */
>  		barrier();	/* Compiler won't know about fs/gs overlap */
> -		ok = rdfs32(A20_TEST_ADDR+0x10) ^ ctr;
> -		if (ok)
> +		if (rdfs32(A20_TEST_ADDR+0x10) != ctr)
>  			break;
> -	}
> +	} while (--loops);
>  
>  	wrgs32(saved, A20_TEST_ADDR);
> -	return ok;
> +	return loops;
>  }
>  
>  /* Quick test to see if A20 is already enabled */
> @@ -125,7 +123,7 @@ static void enable_a20_fast(void)
>  
>  #define A20_ENABLE_LOOPS 255	/* Number of times to try */
>  
> -int enable_a20(void)
> +void enable_a20(void)
>  {
>         int loops = A20_ENABLE_LOOPS;
>         int kbc_err;
> @@ -134,30 +132,30 @@ int enable_a20(void)
>  	       /* First, check to see if A20 is already enabled
>  		  (legacy free, etc.) */
>  	       if (a20_test_short())
> -		       return 0;
> +		       return;
>  
>  	       /* Next, try the BIOS (INT 0x15, AX=0x2401) */
>  	       enable_a20_bios();
>  	       if (a20_test_short())
> -		       return 0;
> +		       return;
>  
>  	       /* Try enabling A20 through the keyboard controller */
>  	       kbc_err = empty_8042();
>  
>  	       if (a20_test_short())
> -		       return 0; /* BIOS worked, but with delayed reaction */
> +		       return; /* BIOS worked, but with delayed reaction */
>  
>  	       if (!kbc_err) {
>  		       enable_a20_kbc();
>  		       if (a20_test_long())
> -			       return 0;
> +			      return;
>  	       }
>  
>  	       /* Finally, try enabling the "fast A20 gate" */
>  	       enable_a20_fast();
>  	       if (a20_test_long())
> -		       return 0;
> +		       return;
>         }
>  
> -       return -1;
> +       die("A20 gate not responding, unable to boot...\n");
>  }
> diff --git a/arch/x86/boot/boot.h b/arch/x86/boot/boot.h
> index 4d3549ed7987..584c89d0738b 100644
> --- a/arch/x86/boot/boot.h
> +++ b/arch/x86/boot/boot.h
> @@ -167,7 +167,7 @@ void copy_to_fs(addr_t dst, void *src, size_t len);
>  void *copy_from_fs(void *dst, addr_t src, size_t len);
>  
>  /* a20.c */
> -int enable_a20(void);
> +void enable_a20(void);
>  
>  /* apm.c */
>  int query_apm_bios(void);
> diff --git a/arch/x86/boot/pm.c b/arch/x86/boot/pm.c
> index 3be89ba4b1b3..e39689ed65ea 100644
> --- a/arch/x86/boot/pm.c
> +++ b/arch/x86/boot/pm.c
> @@ -106,8 +106,7 @@ void go_to_protected_mode(void)
>  	realmode_switch_hook();
>  
>  	/* Enable the A20 gate */
> -	if (enable_a20())
> -		die("A20 gate not responding, unable to boot...\n");
> +	enable_a20();
>  
>  	/* Reset coprocessor (IGNNE#) */
>  	reset_coprocessor();