[XEN PATCH v1] x86/mwait-idle: add dependency on general Intel CPU support

Sergiy Kibrik posted 1 patch 2 months, 2 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/20240905160058.493057-1-Sergiy._5FKibrik@epam.com
There is a newer version of this series
xen/arch/x86/cpu/Makefile          | 2 +-
xen/arch/x86/include/asm/cpuidle.h | 7 +++++++
xen/arch/x86/include/asm/mwait.h   | 7 +++++++
3 files changed, 15 insertions(+), 1 deletion(-)
[XEN PATCH v1] x86/mwait-idle: add dependency on general Intel CPU support
Posted by Sergiy Kibrik 2 months, 2 weeks ago
Currently mwait_idle driver in Xen only implements support for Intel CPUs.
Thus in order to reduce dead code in non-Intel build configurations it can
be made explicitly dependant on CONFIG_INTEL option.

Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
---
 xen/arch/x86/cpu/Makefile          | 2 +-
 xen/arch/x86/include/asm/cpuidle.h | 7 +++++++
 xen/arch/x86/include/asm/mwait.h   | 7 +++++++
 3 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/cpu/Makefile b/xen/arch/x86/cpu/Makefile
index eafce5f204..7cfe28b7ec 100644
--- a/xen/arch/x86/cpu/Makefile
+++ b/xen/arch/x86/cpu/Makefile
@@ -8,7 +8,7 @@ obj-y += common.o
 obj-y += hygon.o
 obj-y += intel.o
 obj-y += intel_cacheinfo.o
-obj-y += mwait-idle.o
+obj-$(CONFIG_INTEL) += mwait-idle.o
 obj-y += shanghai.o
 obj-y += vpmu.o
 obj-$(CONFIG_AMD) += vpmu_amd.o
diff --git a/xen/arch/x86/include/asm/cpuidle.h b/xen/arch/x86/include/asm/cpuidle.h
index 707b3e948d..fde2fa7b08 100644
--- a/xen/arch/x86/include/asm/cpuidle.h
+++ b/xen/arch/x86/include/asm/cpuidle.h
@@ -15,7 +15,14 @@ extern void (*lapic_timer_on)(void);
 
 extern uint64_t (*cpuidle_get_tick)(void);
 
+#ifdef CONFIG_INTEL
 int mwait_idle_init(struct notifier_block *nfb);
+#else
+static inline int mwait_idle_init(struct notifier_block *nfb)
+{
+    return -ENOSYS;
+}
+#endif
 int cpuidle_init_cpu(unsigned int cpu);
 void cf_check default_dead_idle(void);
 void cf_check acpi_dead_idle(void);
diff --git a/xen/arch/x86/include/asm/mwait.h b/xen/arch/x86/include/asm/mwait.h
index 9298f987c4..000a692f6d 100644
--- a/xen/arch/x86/include/asm/mwait.h
+++ b/xen/arch/x86/include/asm/mwait.h
@@ -14,6 +14,13 @@
 #define MWAIT_ECX_INTERRUPT_BREAK	0x1
 
 void mwait_idle_with_hints(unsigned int eax, unsigned int ecx);
+#ifdef CONFIG_INTEL
 bool mwait_pc10_supported(void);
+#else
+static inline bool mwait_pc10_supported(void)
+{
+    return false;
+}
+#endif
 
 #endif /* __ASM_X86_MWAIT_H__ */
-- 
2.25.1
Re: [XEN PATCH v1] x86/mwait-idle: add dependency on general Intel CPU support
Posted by Jan Beulich 2 months, 1 week ago
On 05.09.2024 18:00, Sergiy Kibrik wrote:
> --- a/xen/arch/x86/cpu/Makefile
> +++ b/xen/arch/x86/cpu/Makefile
> @@ -8,7 +8,7 @@ obj-y += common.o
>  obj-y += hygon.o
>  obj-y += intel.o
>  obj-y += intel_cacheinfo.o
> -obj-y += mwait-idle.o
> +obj-$(CONFIG_INTEL) += mwait-idle.o

I'm okay this way, but I wonder whether Andrew - just like for PSR - would
like this not directly keyed to INTEL.

> --- a/xen/arch/x86/include/asm/cpuidle.h
> +++ b/xen/arch/x86/include/asm/cpuidle.h
> @@ -15,7 +15,14 @@ extern void (*lapic_timer_on)(void);
>  
>  extern uint64_t (*cpuidle_get_tick)(void);
>  
> +#ifdef CONFIG_INTEL
>  int mwait_idle_init(struct notifier_block *nfb);
> +#else
> +static inline int mwait_idle_init(struct notifier_block *nfb)
> +{
> +    return -ENOSYS;
> +}

As mentioned elsewhere before - please don't abuse ENOSYS. Seeing how the
function is used I even wonder why it has return type "int".

Jan
Re: [XEN PATCH v1] x86/mwait-idle: add dependency on general Intel CPU support
Posted by Sergiy Kibrik 2 months, 1 week ago
09.09.24 17:28, Jan Beulich:
>> --- a/xen/arch/x86/include/asm/cpuidle.h
>> +++ b/xen/arch/x86/include/asm/cpuidle.h
>> @@ -15,7 +15,14 @@ extern void (*lapic_timer_on)(void);
>>   
>>   extern uint64_t (*cpuidle_get_tick)(void);
>>   
>> +#ifdef CONFIG_INTEL
>>   int mwait_idle_init(struct notifier_block *nfb);
>> +#else
>> +static inline int mwait_idle_init(struct notifier_block *nfb)
>> +{
>> +    return -ENOSYS;
>> +}
> As mentioned elsewhere before - please don't abuse ENOSYS. Seeing how the
> function is used I even wonder why it has return type "int".


I guess it probably should be -ENODEV, i.e. what mwait_idle_probe() 
returns for unknown CPU id.

   -Sergiy