[PATCH] x86/acpi/cstate delete some unuseful operations

[PATCH] x86/acpi/cstate delete some unuseful operations

[TimGuo]

Unuseful cache flush operations which will be executed by ucode when entering C3 will
cause larger C3 enter latency. And the bus master disable operation is not need for
centaur platforms.

Signed-off-by: TimGuo <timguo@zhaoxin.com>
---
 arch/x86/kernel/acpi/cstate.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/x86/kernel/acpi/cstate.c b/arch/x86/kernel/acpi/cstate.c
index dde437f..3eee490 100644
--- a/arch/x86/kernel/acpi/cstate.c
+++ b/arch/x86/kernel/acpi/cstate.c
@@ -51,6 +51,18 @@ void acpi_processor_power_init_bm_check(struct acpi_processor_flags *flags,
        if (c->x86_vendor == X86_VENDOR_INTEL &&
            (c->x86 > 0xf || (c->x86 == 6 && c->x86_model >= 0x0f)))
                        flags->bm_control = 0;
+
+       if (c->x86_vendor == X86_VENDOR_CENTAUR) {
+               /*
+                * on all centaur CPUs, sw need not execute cache flush operation
+                * when entering C3 type state.
+                *
+                * On all Centaur platforms, ARB_DISABLE is not required while
+                * entering C3 type state.
+                */
+               flags->bm_check = 1;
+               flags->bm_control = 0;
+        }
 }
 EXPORT_SYMBOL(acpi_processor_power_init_bm_check);

--
1.9.1



保密声明：
本邮件含有保密或专有信息，仅供指定收件人使用。严禁对本邮件或其内容做任何未经授权的查阅、使用、复制或转发。
CONFIDENTIAL NOTE:
This email contains confidential or legally privileged information and is for the sole use of its intended recipient. Any unauthorized review, use, copying or forwarding of this email or the content of this email is strictly prohibited.

Re: [PATCH] x86/acpi/cstate delete some unuseful operations

[Rafael J. Wysocki]

On Friday, December 22, 2017 12:55:04 PM CET TimGuo wrote:
> Unuseful cache flush operations which will be executed by ucode when entering C3 will
> cause larger C3 enter latency. And the bus master disable operation is not need for
> centaur platforms.

My attempts to make some sense of the above hoplessly failed. :-/

I understood that it was not necessary to disable bus master arbitration on C3
entry for Centaur CPUs, which is why you clear bm_control, right?

And the goal is to reduce the C3 latency, but I'm not sure about the cache
flushing part.  Do you want to say that disabling bus master arbitration causes
the CPU caches to be flushed which is time-consuming and should better be avoided
if not necessary?

> Signed-off-by: TimGuo <timguo@zhaoxin.com>
> ---
>  arch/x86/kernel/acpi/cstate.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/arch/x86/kernel/acpi/cstate.c b/arch/x86/kernel/acpi/cstate.c
> index dde437f..3eee490 100644
> --- a/arch/x86/kernel/acpi/cstate.c
> +++ b/arch/x86/kernel/acpi/cstate.c
> @@ -51,6 +51,18 @@ void acpi_processor_power_init_bm_check(struct acpi_processor_flags *flags,
>         if (c->x86_vendor == X86_VENDOR_INTEL &&
>             (c->x86 > 0xf || (c->x86 == 6 && c->x86_model >= 0x0f)))
>                         flags->bm_control = 0;
> +
> +       if (c->x86_vendor == X86_VENDOR_CENTAUR) {
> +               /*
> +                * on all centaur CPUs, sw need not execute cache flush operation
> +                * when entering C3 type state.
> +                *
> +                * On all Centaur platforms, ARB_DISABLE is not required while
> +                * entering C3 type state.
> +                */
> +               flags->bm_check = 1;
> +               flags->bm_control = 0;
> +        }
>  }
>  EXPORT_SYMBOL(acpi_processor_power_init_bm_check);
> 
> --
> 1.9.1
> 
> 
> 
> 保密声明：
> 本邮件含有保密或专有信息，仅供指定收件人使用。严禁对本邮件或其内容做任何未经授权的查阅、使用、复制或转发。
> CONFIDENTIAL NOTE:
> This email contains confidential or legally privileged information and is for the sole use of its intended recipient. Any unauthorized review, use, copying or forwarding of this email or the content of this email is strictly prohibited.
> 

答复: [PATCH] x86/acpi/cstate delete some unuseful operations

[Tim Guo(BJ-RD)]

Dear Rafael,
Sorry to miss this mail.
I understood that it was not necessary to disable bus master arbitration on C3 entry for Centaur CPUs, which is why you clear bm_control, right?
[Comment by Tim] Yes.

And the goal is to reduce the C3 latency, but I'm not sure about the cache flushing part.  Do you want to say that disabling bus master arbitration causes the CPU caches to be flushed which is time-consuming and should better be avoided if not necessary?
[Comment by Tim] Let's see acpi_idle_enter() function. When entering C3 type state, and when no bm_status has been detected, will execute acpi_idle_enter_bm() function when setting flags.bm_check with 1. In acpi_idle_enter_bm() function, will not execute ACPI_FLUSH_CPU_CACHE(). ACPI_FLUSH_CPU_CACHE need long time to complete. Actually, this software operation for centaur CPU is not necessary.Because the cache flush operation will be executed by CPU microcode automatically.

static int acpi_idle_enter(struct cpuidle_device *dev,
   struct cpuidle_driver *drv, int index)
{
struct acpi_processor_cx *cx = per_cpu(acpi_cstate[index], dev->cpu);
struct acpi_processor *pr;

pr = __this_cpu_read(processors);
if (unlikely(!pr))
return -EINVAL;

if (cx->type != ACPI_STATE_C1) {
if (acpi_idle_fallback_to_c1(pr) && num_online_cpus() > 1) {
index = ACPI_IDLE_STATE_START;
cx = per_cpu(acpi_cstate[index], dev->cpu);
} else if (cx->type == ACPI_STATE_C3 && pr->flags.bm_check) {
if (cx->bm_sts_skip || !acpi_idle_bm_check()) {
acpi_idle_enter_bm(pr, cx, true);
return index;
} else if (drv->safe_state_index >= 0) {
index = drv->safe_state_index;
cx = per_cpu(acpi_cstate[index], dev->cpu);
} else {
acpi_safe_halt();
return -EBUSY;
}
}
}

lapic_timer_state_broadcast(pr, cx, 1);

if (cx->type == ACPI_STATE_C3)
ACPI_FLUSH_CPU_CACHE();

acpi_idle_do_entry(cx);

lapic_timer_state_broadcast(pr, cx, 0);

return index;
}

Thank you.

Tim

-----邮件原件-----
发件人: Rafael J. Wysocki [mailto:rjw@rjwysocki.net]
发送时间: 2018年1月5日 21:29
收件人: Tim Guo(BJ-RD) <TimGuo@zhaoxin.com>
抄送: tglx@linutronix.de; mingo@redhat.com; hpa@zytor.com; mingo@kernel.org; x86@kernel.org; linux-pm@vger.kernel.org; linux-kernel@vger.kernel.org; Bruce Chang (VAS) <BruceChang@via-alliance.com>; Cooper Yan(BJ-RD) <CooperYan@zhaoxin.com>; Qiyuan Wang(BJ-RD) <QiyuanWang@zhaoxin.com>; Benjamin Pan <BenjaminPan@viatech.com>
主题: Re: [PATCH] x86/acpi/cstate delete some unuseful operations

On Friday, December 22, 2017 12:55:04 PM CET TimGuo wrote:
> Unuseful cache flush operations which will be executed by ucode when
> entering C3 will cause larger C3 enter latency. And the bus master
> disable operation is not need for centaur platforms.

My attempts to make some sense of the above hoplessly failed. :-/

I understood that it was not necessary to disable bus master arbitration on C3 entry for Centaur CPUs, which is why you clear bm_control, right?

And the goal is to reduce the C3 latency, but I'm not sure about the cache flushing part.  Do you want to say that disabling bus master arbitration causes the CPU caches to be flushed which is time-consuming and should better be avoided if not necessary?

> Signed-off-by: TimGuo <timguo@zhaoxin.com>
> ---
>  arch/x86/kernel/acpi/cstate.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/arch/x86/kernel/acpi/cstate.c
> b/arch/x86/kernel/acpi/cstate.c index dde437f..3eee490 100644
> --- a/arch/x86/kernel/acpi/cstate.c
> +++ b/arch/x86/kernel/acpi/cstate.c
> @@ -51,6 +51,18 @@ void acpi_processor_power_init_bm_check(struct acpi_processor_flags *flags,
>         if (c->x86_vendor == X86_VENDOR_INTEL &&
>             (c->x86 > 0xf || (c->x86 == 6 && c->x86_model >= 0x0f)))
>                         flags->bm_control = 0;
> +
> +       if (c->x86_vendor == X86_VENDOR_CENTAUR) {
> +               /*
> +                * on all centaur CPUs, sw need not execute cache flush operation
> +                * when entering C3 type state.
> +                *
> +                * On all Centaur platforms, ARB_DISABLE is not required while
> +                * entering C3 type state.
> +                */
> +               flags->bm_check = 1;
> +               flags->bm_control = 0;
> +        }
>  }
>  EXPORT_SYMBOL(acpi_processor_power_init_bm_check);
>
> --
> 1.9.1
>
>
>
> 保密声明：
> 本邮件含有保密或专有信息，仅供指定收件人使用。严禁对本邮件或其内容做任何未经授权的查阅、使用、复制或转发。
> CONFIDENTIAL NOTE:
> This email contains confidential or legally privileged information and is for the sole use of its intended recipient. Any unauthorized review, use, copying or forwarding of this email or the content of this email is strictly prohibited.
>




保密声明：
本邮件含有保密或专有信息，仅供指定收件人使用。严禁对本邮件或其内容做任何未经授权的查阅、使用、复制或转发。
CONFIDENTIAL NOTE:
This email contains confidential or legally privileged information and is for the sole use of its intended recipient. Any unauthorized review, use, copying or forwarding of this email or the content of this email is strictly prohibited.

答复: [PATCH] x86/acpi/cstate delete some unuseful operations

[Tim Guo(BJ-RD)]

Add more.

Tim

-----邮件原件-----
发件人: Tim Guo(BJ-RD)
发送时间: 2018年1月10日 11:47
收件人: 'Rafael J. Wysocki' <rjw@rjwysocki.net>
抄送: tglx@linutronix.de; mingo@redhat.com; hpa@zytor.com; mingo@kernel.org; x86@kernel.org; linux-pm@vger.kernel.org; linux-kernel@vger.kernel.org; Bruce Chang (VAS) <BruceChang@via-alliance.com>; Cooper Yan(BJ-RD) <CooperYan@zhaoxin.com>; Qiyuan Wang(BJ-RD) <QiyuanWang@zhaoxin.com>; Benjamin Pan <BenjaminPan@viatech.com>
主题: 答复: [PATCH] x86/acpi/cstate delete some unuseful operations

Dear Rafael,
Sorry to miss this mail.
I understood that it was not necessary to disable bus master arbitration on C3 entry for Centaur CPUs, which is why you clear bm_control, right?
[Comment by Tim] Yes.

And the goal is to reduce the C3 latency, but I'm not sure about the cache flushing part.  Do you want to say that disabling bus master arbitration causes the CPU caches to be flushed which is time-consuming and should better be avoided if not necessary?
[Comment by Tim] Let's see acpi_idle_enter() function. When entering C3 type state, and when no bm_status has been detected, will execute acpi_idle_enter_bm() function when setting flags.bm_check with 1. In acpi_idle_enter_bm() function, will not execute ACPI_FLUSH_CPU_CACHE(). ACPI_FLUSH_CPU_CACHE need long time to complete. Actually, this software operation for centaur CPU is not necessary.Because the cache flush operation will be executed by CPU microcode automatically.

static int acpi_idle_enter(struct cpuidle_device *dev,
   struct cpuidle_driver *drv, int index) {
struct acpi_processor_cx *cx = per_cpu(acpi_cstate[index], dev->cpu);
struct acpi_processor *pr;

pr = __this_cpu_read(processors);
if (unlikely(!pr))
return -EINVAL;

if (cx->type != ACPI_STATE_C1) {
if (acpi_idle_fallback_to_c1(pr) && num_online_cpus() > 1) {
index = ACPI_IDLE_STATE_START;
cx = per_cpu(acpi_cstate[index], dev->cpu);
} else if (cx->type == ACPI_STATE_C3 && pr->flags.bm_check) {
if (cx->bm_sts_skip || !acpi_idle_bm_check()) {
acpi_idle_enter_bm(pr, cx, true);
return index;
} else if (drv->safe_state_index >= 0) {
index = drv->safe_state_index;
cx = per_cpu(acpi_cstate[index], dev->cpu);
} else {
acpi_safe_halt();
return -EBUSY;
}
}
}

lapic_timer_state_broadcast(pr, cx, 1);

if (cx->type == ACPI_STATE_C3)
ACPI_FLUSH_CPU_CACHE();

acpi_idle_do_entry(cx);

lapic_timer_state_broadcast(pr, cx, 0);

return index;
}

Thank you.

Tim

-----邮件原件-----
发件人: Rafael J. Wysocki [mailto:rjw@rjwysocki.net]
发送时间: 2018年1月5日 21:29
收件人: Tim Guo(BJ-RD) <TimGuo@zhaoxin.com>
抄送: tglx@linutronix.de; mingo@redhat.com; hpa@zytor.com; mingo@kernel.org; x86@kernel.org; linux-pm@vger.kernel.org; linux-kernel@vger.kernel.org; Bruce Chang (VAS) <BruceChang@via-alliance.com>; Cooper Yan(BJ-RD) <CooperYan@zhaoxin.com>; Qiyuan Wang(BJ-RD) <QiyuanWang@zhaoxin.com>; Benjamin Pan <BenjaminPan@viatech.com>
主题: Re: [PATCH] x86/acpi/cstate delete some unuseful operations

On Friday, December 22, 2017 12:55:04 PM CET TimGuo wrote:
> Unuseful cache flush operations which will be executed by ucode when
> entering C3 will cause larger C3 enter latency. And the bus master
> disable operation is not need for centaur platforms.

My attempts to make some sense of the above hoplessly failed. :-/

I understood that it was not necessary to disable bus master arbitration on C3 entry for Centaur CPUs, which is why you clear bm_control, right?

And the goal is to reduce the C3 latency, but I'm not sure about the cache flushing part.  Do you want to say that disabling bus master arbitration causes the CPU caches to be flushed which is time-consuming and should better be avoided if not necessary?

> Signed-off-by: TimGuo <timguo@zhaoxin.com>
> ---
>  arch/x86/kernel/acpi/cstate.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/arch/x86/kernel/acpi/cstate.c
> b/arch/x86/kernel/acpi/cstate.c index dde437f..3eee490 100644
> --- a/arch/x86/kernel/acpi/cstate.c
> +++ b/arch/x86/kernel/acpi/cstate.c
> @@ -51,6 +51,18 @@ void acpi_processor_power_init_bm_check(struct acpi_processor_flags *flags,
>         if (c->x86_vendor == X86_VENDOR_INTEL &&
>             (c->x86 > 0xf || (c->x86 == 6 && c->x86_model >= 0x0f)))
>                         flags->bm_control = 0;
> +
> +       if (c->x86_vendor == X86_VENDOR_CENTAUR) {
> +               /*
> +                * on all centaur CPUs, sw need not execute cache flush operation
> +                * when entering C3 type state.
> +                *
> +                * On all Centaur platforms, ARB_DISABLE is not required while
> +                * entering C3 type state.
> +                */
> +               flags->bm_check = 1;
> +               flags->bm_control = 0;
> +        }
>  }
>  EXPORT_SYMBOL(acpi_processor_power_init_bm_check);
>
> --
> 1.9.1
>
>
>
> 保密声明：
> 本邮件含有保密或专有信息，仅供指定收件人使用。严禁对本邮件或其内容做任何未经授权的查阅、使用、复制或转发。
> CONFIDENTIAL NOTE:
> This email contains confidential or legally privileged information and is for the sole use of its intended recipient. Any unauthorized review, use, copying or forwarding of this email or the content of this email is strictly prohibited.
>




保密声明：
本邮件含有保密或专有信息，仅供指定收件人使用。严禁对本邮件或其内容做任何未经授权的查阅、使用、复制或转发。
CONFIDENTIAL NOTE:
This email contains confidential or legally privileged information and is for the sole use of its intended recipient. Any unauthorized review, use, copying or forwarding of this email or the content of this email is strictly prohibited.