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

["Woods, Brian" <Brian.Woods@xxxxxxx>]

On Tue, Jun 11, 2019 at 06:42:33AM -0600, Jan Beulich wrote:
> >>> On 10.06.19 at 18:28, <andrew.cooper3@xxxxxxxxxx> wrote:
> > On 23/05/2019 13:18, 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>
> >> ---
> >> TBD: Can we set local_apic_timer_c2_ok to true? I can't seem to find any
> >>      statement in the BKDG / PPR as to whether the LAPIC timer continues
> >>      running in CC6.
> > 
> > This ought to be easy to determine.  Given the description of CC6
> > flushing the cache and power gating the core, I'd say there is a
> > reasonable chance that the LAPIC timer stops in CC6.
> 
> But "reasonable chance" isn't enough for my taste here. And from
> what you deduce, the answer to the question would be "no", and
> hence simply no change to be made anywhere. (I do think though
> that it's more complicated than this, because iirc much also depends
> on what the firmware actually does.)

The LAPIC timer never stops on the currently platforms (Naples and
Rome).  This is a knowledgable HW engineer so.

> >> TBD: We may want to verify that HLT indeed is configured to enter CC6.
> > 
> > I can't actually spot anything which talks about HLT directly.  The
> > closest I can post is CFOH (cache flush on halt) which is an
> > auto-transition from CC1 to CC6 after a specific timeout, but the
> > wording suggests that mwait would also take this path.
> 
> Well, I had come across a section describing how HLT can be
> configured to be the same action as the I/O port read from one
> of the three ports involved in C-state management
> (CStateBaseAddr+0...2). But I can't seem to find this again.
> 
> As to MWAIT behaving the same, I don't think I can spot proof
> of your interpretation or proof of Brian's.

It's not really documented clearly.  I got my information from the HW
engineers.  I've already posted what information I know so I won't
repeat it.

> >> --- a/xen/arch/x86/acpi/cpu_idle.c
> >> +++ b/xen/arch/x86/acpi/cpu_idle.c
> >> @@ -1283,6 +1288,98 @@ 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,
> >> +            .latency = 1,
> >> +        },
> >> +        {
> >> +            .type = ACPI_STATE_C2,
> >> +            .entry_method = ACPI_CSTATE_EM_HALT,
> >> +            .latency = 400,
> >> +        },
> >> +    };
> >> +
> >> +    if ( pm_idle_save && pm_idle != acpi_processor_idle )
> >> +        return;
> >> +
> >> +    if ( vendor_override < 0 )
> >> +        return;
> >> +
> >> +    switch ( c->x86 )
> >> +    {
> >> +    case 0x17:
> > 
> > With Hygon in the mix, this should be expanded to Fam18h.
> 
> But only once we get a guarantee from AMD that they won't use
> family 18h. Otherwise we'd have to use vendor checks here.
> Anyway this series predates the merging of the Hygon one. But
> yes, I can easily do this for v2.
> 
> Jan
> 
> 

-- 
Brian Woods

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