[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



On Fri, Oct 04, 2024 at 12:31:50PM +0300, Sergiy Kibrik wrote:
> Introduce config option X86_PMTIMER so that pmtimer emulation driver can later
> be made configurable and be disabled on systems that don't need it.
> 
> As a first step the option is hidden from user, thus not making any functional
> changes here.
> 
> Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@xxxxxxxx>
> CC: Jan Beulich <jbeulich@xxxxxxxx>
> ---
>  xen/arch/x86/Kconfig               |  3 +++
>  xen/arch/x86/hvm/Makefile          |  2 +-
>  xen/arch/x86/include/asm/acpi.h    |  5 +++++
>  xen/arch/x86/include/asm/domain.h  |  3 ++-
>  xen/arch/x86/include/asm/hvm/vpt.h | 10 ++++++++++
>  5 files changed, 21 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig
> index 9cdd04721a..95275dc17e 100644
> --- a/xen/arch/x86/Kconfig
> +++ b/xen/arch/x86/Kconfig
> @@ -144,6 +144,9 @@ config INTEL_VMX
>         If your system includes a processor with Intel VT-x support, say Y.
>         If in doubt, say Y.
>  
> +config X86_PMTIMER
> +     def_bool HVM

The chunk in patch 3 that fill this option needs to be moved here,
together with the updated checks in emulation_flags_ok().

>  config XEN_SHSTK
>       bool "Supervisor Shadow Stacks"
>       depends on HAS_AS_CET_SS
> diff --git a/xen/arch/x86/hvm/Makefile b/xen/arch/x86/hvm/Makefile
> index 4c1fa5c6c2..321241f0bf 100644
> --- a/xen/arch/x86/hvm/Makefile
> +++ b/xen/arch/x86/hvm/Makefile
> @@ -18,7 +18,7 @@ obj-y += irq.o
>  obj-y += monitor.o
>  obj-y += mtrr.o
>  obj-y += nestedhvm.o
> -obj-y += pmtimer.o
> +obj-$(CONFIG_X86_PMTIMER) += pmtimer.o

I think you can also make the hvm_hw_acpi field in struct hvm_domain
presence dependent on CONFIG_X86_PMTIMER being enabled.

>  obj-y += quirks.o
>  obj-y += rtc.o
>  obj-y += save.o
> diff --git a/xen/arch/x86/include/asm/acpi.h b/xen/arch/x86/include/asm/acpi.h
> index 217819dd61..8d92014ae9 100644
> --- a/xen/arch/x86/include/asm/acpi.h
> +++ b/xen/arch/x86/include/asm/acpi.h
> @@ -150,8 +150,13 @@ void acpi_mmcfg_init(void);
>  /* Incremented whenever we transition through S3. Value is 1 during boot. */
>  extern uint32_t system_reset_counter;
>  
> +#ifdef CONFIG_X86_PMTIMER
>  void hvm_acpi_power_button(struct domain *d);
>  void hvm_acpi_sleep_button(struct domain *d);
> +#else
> +static inline void hvm_acpi_power_button(struct domain *d) {}
> +static inline void hvm_acpi_sleep_button(struct domain *d) {}
> +#endif

It would be best if those functions returned -ENODEV when the
interface is not available, but that's an existing issue, so won't
insist in you fixing it here.

>  /* suspend/resume */
>  void save_rest_processor_state(void);
> diff --git a/xen/arch/x86/include/asm/domain.h 
> b/xen/arch/x86/include/asm/domain.h
> index bdcdb8de09..3f65bfd190 100644
> --- 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.

Thanks, Roger.



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.