Re: [XEN PATCH v1 1/3] x86/hvm: introduce config option for ACPI PM timer

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Fri, 4 Oct 2024, Jan Beulich wrote:
> On 04.10.2024 15:09, Roger Pau Monné wrote:
> > On Fri, Oct 04, 2024 at 12:31:50PM +0300, Sergiy Kibrik wrote:
> >> --- a/xen/arch/x86/include/asm/domain.h
> >> +++ b/xen/arch/x86/include/asm/domain.h
> >> @@ -496,7 +496,8 @@ struct arch_domain
> >>  
> >>  #define has_vlapic(d)      (!!((d)->arch.emulation_flags & X86_EMU_LAPIC))
> >>  #define has_vhpet(d)       (!!((d)->arch.emulation_flags & X86_EMU_HPET))
> >> -#define has_vpm(d)         (!!((d)->arch.emulation_flags & X86_EMU_PM))
> >> +#define has_vpm(d)         (IS_ENABLED(CONFIG_X86_PMTIMER) && \
> >> +                            !!((d)->arch.emulation_flags & X86_EMU_PM))
> > 
> > Do you really need the IS_ENABLED() here?  If you modify
> > emulation_flags_ok() to reject the flag if not available it won't be
> > possible for any domain to have it set.
> 
> With the IS_ENABLED() the only other approach to have the compiler DCE any
> code left unreachable would be to #define X86_EMU_PM to 0 in that case. I
> guess I'd slightly prefer that alternative, but otherwise the approach used
> here would still be wanted imo.

The compiler DCE is important, either the approach in this patch or
Jan's suggestion would work fine as far as I can tell.