Re: [Xen-devel] [PATCH v2 3/5] x86/AMD: make C-state handling independent of Dom0

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Wed, Jul 03, 2019 at 01:01:48PM +0000, Jan Beulich wrote:
> At least for more recent CPUs, following what BKDG / PPR suggest for the
> BIOS to surface via ACPI we can make ourselves independent of Dom0
> uploading respective data.
> 
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> ---
> v2: Handle Hygon Fam18. Set local_apic_timer_c2_ok (for Fam17 and Hygon
>      Fam18 only for now).
> ---
> TBD: We may want to verify that HLT indeed is configured to enter CC6.
> TBD: Brian's series specifies .target_residency as 1000 for CC6; may
>       want to do so here as well. Question then is whether this value is
>       also suitable for the older families.
> TBD: I guess we could extend this to families older then Fam15 as well.
> 
> --- a/xen/arch/x86/acpi/cpu_idle.c
> +++ b/xen/arch/x86/acpi/cpu_idle.c
> @@ -110,6 +110,8 @@ boolean_param("lapic_timer_c2_ok", local
>   
>   struct acpi_processor_power *__read_mostly processor_powers[NR_CPUS];
>   
> +static int8_t __read_mostly vendor_override;

AFAICT from the code below this is a tri-state (-1, 0, 1). Could it
maybe be turned into an enum with definitions of the different
states?

I think it would make the usage clearer.

> +
>   struct hw_residencies
>   {
>       uint64_t mc0;
> @@ -1214,6 +1216,9 @@ long set_cx_pminfo(uint32_t acpi_id, str
>       if ( pm_idle_save && pm_idle != acpi_processor_idle )
>           return 0;
>   
> +    if ( vendor_override > 0 )
> +        return 0;
> +
>       print_cx_pminfo(acpi_id, power);
>   
>       cpu_id = get_cpu_id(acpi_id);
> @@ -1286,6 +1291,103 @@ long set_cx_pminfo(uint32_t acpi_id, str
>       return 0;
>   }
>   
> +static void amd_cpuidle_init(struct acpi_processor_power *power)
> +{
> +    unsigned int i, nr = 0;
> +    const struct cpuinfo_x86 *c = &current_cpu_data;
> +    const unsigned int ecx_req = CPUID5_ECX_EXTENSIONS_SUPPORTED |
> +                                 CPUID5_ECX_INTERRUPT_BREAK;
> +    const struct acpi_processor_cx *cx = NULL;
> +    static const struct acpi_processor_cx fam17[] = {
> +        {
> +            .type = ACPI_STATE_C1,
> +            .entry_method = ACPI_CSTATE_EM_FFH,
> +            .address = 0,

address field will already get set to 0 by default.

> +            .latency = 1,
> +        },
> +        {
> +            .type = ACPI_STATE_C2,
> +            .entry_method = ACPI_CSTATE_EM_HALT,
> +            .latency = 400,

Maybe the latency values could be added to cpuidle.h as defines?

> +        },
> +    };
> +
> +    if ( pm_idle_save && pm_idle != acpi_processor_idle )
> +        return;
> +
> +    if ( vendor_override < 0 )
> +        return;
> +
> +    switch ( c->x86 )
> +    {
> +    case 0x18:
> +        if ( boot_cpu_data.x86_vendor != X86_VENDOR_HYGON )
> +        {
> +    default:
> +            vendor_override = -1;
> +            return;
> +        }
> +        /* fall through */
> +    case 0x17:
> +        if ( cpu_has_monitor && c->cpuid_level >= CPUID_MWAIT_LEAF &&
> +             (cpuid_ecx(CPUID_MWAIT_LEAF) & ecx_req) == ecx_req )
> +        {
> +            cx = fam17;
> +            nr = ARRAY_SIZE(fam17);
> +            local_apic_timer_c2_ok = true;
> +            break;
> +        }
> +        /* fall through */
> +    case 0x15:
> +    case 0x16:
> +        cx = &fam17[1];
> +        nr = ARRAY_SIZE(fam17) - 1;
> +        break;
> +    }
> +
> +    power->flags.has_cst = true;
> +
> +    for ( i = 0; i < nr; ++i )
> +    {
> +        if ( cx[i].type > max_cstate )
> +            break;
> +        power->states[i + 1] = cx[i];
> +        power->states[i + 1].idx = i + 1;
> +        power->states[i + 1].target_residency = cx[i].latency * 
> latency_factor;

You could set target_residency as part of the initialization, but I
guess latency_factor being non-const that would move fam17 outside of
.rodata?

Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel