Re: [PATCH] x86/ACPI: Fix build error when tboot is disabled

[Costin Lupu <costin.lupu@xxxxxxxxx>]

On 4/29/21 3:40 PM, Jan Beulich wrote:
> On 29.04.2021 14:11, Costin Lupu wrote:
>> When tboot is disabled via menuconfig we get undefined reference error for
>> g_tboot_shared. This patch fixes that by disabling the causing source code
>> when tboot is disabled.
> 
> There must be more to this: Our shim config also builds with tboot
> disabled, without running into any build issue. Furthermore ...
> 
>> --- a/xen/arch/x86/acpi/power.c
>> +++ b/xen/arch/x86/acpi/power.c
>> @@ -407,6 +407,7 @@ static int acpi_get_wake_status(void)
>>      return val;
>>  }
>>  
>> +#ifdef CONFIG_TBOOT
>>  static void tboot_sleep(u8 sleep_state)
>>  {
>>      uint32_t shutdown_type;
>> @@ -451,18 +452,21 @@ static void tboot_sleep(u8 sleep_state)
>>  
>>      tboot_shutdown(shutdown_type);
>>  }
>> +#endif
>>  
>>  /* System is really put into sleep state by this stub */
>>  acpi_status acpi_enter_sleep_state(u8 sleep_state)
>>  {
>>      acpi_status status;
>>  
>> +#ifdef CONFIG_TBOOT
>>      if ( tboot_in_measured_env() )
>>      {
>>          tboot_sleep(sleep_state);
>>          printk(XENLOG_ERR "TBOOT failed entering s3 state\n");
>>          return_ACPI_STATUS(AE_ERROR);
>>      }
>> +#endif
> 
> ... tboot_in_measured_env() already has a stub returning 0 when
> !TBOOT (which is what I would have recommended instead of the
> #ifdef-ary).
> 
> If there is a specific case where the compiler fails to DCE the
> offending code, then you need to describe this in sufficient
> detail.

Yes, indeed. My bad, it is for a debug build with -O0, so without DCE.

Cheers,
Costin