Xen devel - Dec 2013 - [PATCH] xen/x86: Be more power-efficient when waiting forever

The effect is unchanged, but the processor will be spending most of its time
in the C1 or C1E power state rather than C0.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Keir Fraser <keir@xen.org>
CC: Jan Beulich <JBeulich@suse.com>
CC: George Dunlap <george.dunlap@eu.citrix.com>

---

This was discovered while I was playing about in the early_page_fault()
handler with the IBM box which was corruption the Xen code section.

There is one final for(;;); loop, but is in common code and immediately
following a call to panic(), so is actually dead code and will be cleaned up
in v2 of my "noreturn" series at the start of 4.5

George: I am requesting that this get a freeze exception, under the
qualification of #2 "An awesome release", given no function change
threatening
---
 xen/arch/x86/efi/boot.c |    3 ++-
 xen/arch/x86/traps.c    |    3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/efi/boot.c b/xen/arch/x86/efi/boot.c
index 0dd935c..602cc12 100644
--- a/xen/arch/x86/efi/boot.c
+++ b/xen/arch/x86/efi/boot.c
@@ -201,7 +201,8 @@ static void __init __attribute__((__noreturn__))
blexit(const CHAR16 *str)
         efi_bs->FreePages(xsm.addr, PFN_UP(xsm.size));
 
     efi_bs->Exit(efi_ih, EFI_SUCCESS, 0, NULL);
-    for( ; ; ); /* not reached */
+    for ( ; ; ) /* not reached */
+        halt();
 }
 
 /* generic routine for printing error messages */
diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index d8b3eac..8c355e5 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -1448,7 +1448,8 @@ void __init do_early_page_fault(struct cpu_user_regs
*regs)
         printk("Stack dump: ");
         while ( ((long)stk & ((PAGE_SIZE - 1) & ~(BYTES_PER_LONG - 1)))
!= 0 )
             printk("%p ", _p(*stk++));
-        for ( ; ; ) ;
+        for ( ; ; )
+            halt();
     }
 }
 
-- 
1.7.10.4

>>> On 02.12.13 at 15:38, Andrew Cooper
<andrew.cooper3@citrix.com> wrote:
> There is one final for(;;); loop, but is in common code and immediately
> following a call to panic(), so is actually dead code and will be cleaned
up
> in v2 of my "noreturn" series at the start of 4.5
In the context of this, this one ...
> --- a/xen/arch/x86/efi/boot.c
> +++ b/xen/arch/x86/efi/boot.c
> @@ -201,7 +201,8 @@ static void __init __attribute__((__noreturn__)) 
> blexit(const CHAR16 *str)
>          efi_bs->FreePages(xsm.addr, PFN_UP(xsm.size));
>  
>      efi_bs->Exit(efi_ih, EFI_SUCCESS, 0, NULL);
> -    for( ; ; ); /* not reached */
> +    for ( ; ; ) /* not reached */
> +        halt();
... seems pointless (but of course also not wrong).
> --- a/xen/arch/x86/traps.c
> +++ b/xen/arch/x86/traps.c
> @@ -1448,7 +1448,8 @@ void __init do_early_page_fault(struct cpu_user_regs
*regs)
>          printk("Stack dump: ");
>          while ( ((long)stk & ((PAGE_SIZE - 1) & ~(BYTES_PER_LONG -
1))) != 0 )
>              printk("%p ", _p(*stk++));
> -        for ( ; ; ) ;
> +        for ( ; ; )
> +            halt();
While clearly this one is desirable.

Jan

On 02/12/13 15:03, Jan Beulich wrote:
>>>> On 02.12.13 at 15:38, Andrew Cooper
<andrew.cooper3@citrix.com> wrote:
>> There is one final for(;;); loop, but is in common code and immediately
>> following a call to panic(), so is actually dead code and will be
cleaned up
>> in v2 of my "noreturn" series at the start of 4.5
> In the context of this, this one ...
>
>> --- a/xen/arch/x86/efi/boot.c
>> +++ b/xen/arch/x86/efi/boot.c
>> @@ -201,7 +201,8 @@ static void __init __attribute__((__noreturn__)) 
>> blexit(const CHAR16 *str)
>>          efi_bs->FreePages(xsm.addr, PFN_UP(xsm.size));
>>  
>>      efi_bs->Exit(efi_ih, EFI_SUCCESS, 0, NULL);
>> -    for( ; ; ); /* not reached */
>> +    for ( ; ; ) /* not reached */
>> +        halt();
> ... seems pointless (but of course also not wrong).
I suppose - I guess all that is needed here is for the compiler to find
no way to the end of this function.

I wonder whether an "unreachable()" would suffice?  In the optimal
case,
the compiler should not emit any instructions for it.

~Andrew
>
>> --- a/xen/arch/x86/traps.c
>> +++ b/xen/arch/x86/traps.c
>> @@ -1448,7 +1448,8 @@ void __init do_early_page_fault(struct
cpu_user_regs *regs)
>>          printk("Stack dump: ");
>>          while ( ((long)stk & ((PAGE_SIZE - 1) &
~(BYTES_PER_LONG - 1))) != 0 )
>>              printk("%p ", _p(*stk++));
>> -        for ( ; ; ) ;
>> +        for ( ; ; )
>> +            halt();
> While clearly this one is desirable.
>
> Jan
>

On 02/12/2013 14:38, "Andrew Cooper" <andrew.cooper3@citrix.com>
wrote:
> The effect is unchanged, but the processor will be spending most of its
time
> in the C1 or C1E power state rather than C0.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> CC: Keir Fraser <keir@xen.org>
> CC: Jan Beulich <JBeulich@suse.com>
> CC: George Dunlap <george.dunlap@eu.citrix.com>
Acked-by: Keir Fraser <keir@xen.org>
> ---
> 
> This was discovered while I was playing about in the early_page_fault()
> handler with the IBM box which was corruption the Xen code section.
> 
> There is one final for(;;); loop, but is in common code and immediately
> following a call to panic(), so is actually dead code and will be cleaned
up
> in v2 of my "noreturn" series at the start of 4.5
> 
> George: I am requesting that this get a freeze exception, under the
> qualification of #2 "An awesome release", given no function
change threatening
> ---
>  xen/arch/x86/efi/boot.c |    3 ++-
>  xen/arch/x86/traps.c    |    3 ++-
>  2 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/arch/x86/efi/boot.c b/xen/arch/x86/efi/boot.c
> index 0dd935c..602cc12 100644
> --- a/xen/arch/x86/efi/boot.c
> +++ b/xen/arch/x86/efi/boot.c
> @@ -201,7 +201,8 @@ static void __init __attribute__((__noreturn__))
> blexit(const CHAR16 *str)
>          efi_bs->FreePages(xsm.addr, PFN_UP(xsm.size));
>  
>      efi_bs->Exit(efi_ih, EFI_SUCCESS, 0, NULL);
> -    for( ; ; ); /* not reached */
> +    for ( ; ; ) /* not reached */
> +        halt();
>  }
>  
>  /* generic routine for printing error messages */
> diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
> index d8b3eac..8c355e5 100644
> --- a/xen/arch/x86/traps.c
> +++ b/xen/arch/x86/traps.c
> @@ -1448,7 +1448,8 @@ void __init do_early_page_fault(struct cpu_user_regs
> *regs)
>          printk("Stack dump: ");
>          while ( ((long)stk & ((PAGE_SIZE - 1) & ~(BYTES_PER_LONG -
1))) != 0
> )
>              printk("%p ", _p(*stk++));
> -        for ( ; ; ) ;
> +        for ( ; ; )
> +            halt();
>      }
>  }
>