On 2025-11-18 15:04, Andrew Cooper wrote:
On 18/11/2025 3:09 pm, Jan Beulich wrote:
--- a/xen/arch/x86/acpi/cpufreq/hwp.c
+++ b/xen/arch/x86/acpi/cpufreq/hwp.c
@@ -183,29 +178,25 @@ static bool __init hwp_available(void)
return false;
- if ( !(eax & CPUID6_EAX_HWP_ENERGY_PERFORMANCE_PREFERENCE) )
+ if ( !host_cpu_policy.basic.pm.hwp_epp )
{
hwp_verbose("disabled: No energy/performance preference available");
return false;
}
- feature_hwp_notification = eax & CPUID6_EAX_HWP_NOTIFICATION;
- feature_hwp_activity_window = eax & CPUID6_EAX_HWP_ACTIVITY_WINDOW;
- feature_hdc = eax & CPUID6_EAX_HDC;
+ feature_hdc = host_cpu_policy.basic.pm.hdc;
Looking at how feature_hdc is used, I think it should be the bit within
the host policy, rather than a separate boolean.
The host policy "is" what Xen is using, so if the HWP code decides it
doesn't like HDC, then that does want reflecting.
Unrelated to this patch, but as I've been looking at the code. What
happens when hwp_init_msrs() fails on an AP? I can't see anything which
unwinds the initialised HDC state on the prior CPUs...
Assuming symmetry, it'd fail on the BSP and never get to an AP. Yes, I
didn't consider unwinding.
@@ -365,7 +357,7 @@ static void cf_check hwp_init_msrs(void
}
/* Ensure we don't generate interrupts */
- if ( feature_hwp_notification )
+ if ( host_cpu_policy.basic.pm.hwp_notification )
wrmsr_safe(MSR_HWP_INTERRUPT, 0);
Again, unrelated to the patch, but why is this a wrmsr_safe() ?
It seemed safer ;)
If the feature is enumerated, the MSR had better work.
Yes, but I wasn't sure how much to trust it. I only tested with 2
processor models, so I wanted to be safer. IIRC, I saw a #GP or two
during development. Those might have been from writing a reserved bit
and fixed during development.
Given my small testing size, I wanted to keep it safe and not take down Xen.
Regards,
Jason
