linux-kernel - Re: [PATCH v3 4/5] x86/umwait: Add sysfs interface to control umwait maximum time

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 30 May 2019 14:11:48 -0700
From:   Andy Lutomirski <luto@...nel.org>
To:     Fenghua Yu <fenghua.yu@...el.com>
Cc:     Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        H Peter Anvin <hpa@...or.com>,
        Andrew Cooper <andrew.cooper3@...rix.com>,
        Ashok Raj <ashok.raj@...el.com>,
        Tony Luck <tony.luck@...el.com>,
        Ravi V Shankar <ravi.v.shankar@...el.com>,
        linux-kernel <linux-kernel@...r.kernel.org>, x86 <x86@...nel.org>
Subject: Re: [PATCH v3 4/5] x86/umwait: Add sysfs interface to control umwait
 maximum time

On Fri, May 24, 2019 at 5:05 PM Fenghua Yu <fenghua.yu@...el.com> wrote:
>
> IA32_UMWAIT_CONTROL[31:2] determines the maximum time in TSC-quanta
> that processor can stay in C0.1 or C0.2. A zero value means no maximum
> time.
>
> Each instruction sets its own deadline in the instruction's implicit
> input EDX:EAX value. The instruction wakes up if the time-stamp counter
> reaches or exceeds the specified deadline, or the umwait maximum time
> expires, or a store happens in the monitored address range in umwait.
>
> Users can write an unsigned 32-bit number to
> /sys/devices/system/cpu/umwait_control/max_time to change the default
> value. Note that a value of zero means there is no limit. Low order
> two bits are ignored.
>
> Signed-off-by: Fenghua Yu <fenghua.yu@...el.com>
> Reviewed-by: Ashok Raj <ashok.raj@...el.com>
> Reviewed-by: Tony Luck <tony.luck@...el.com>
> ---
>  arch/x86/power/umwait.c | 37 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 37 insertions(+)
>
> diff --git a/arch/x86/power/umwait.c b/arch/x86/power/umwait.c
> index cf5de7e1cc24..61076aad7138 100644
> --- a/arch/x86/power/umwait.c
> +++ b/arch/x86/power/umwait.c
> @@ -103,8 +103,45 @@ static ssize_t enable_c0_2_store(struct device *dev,
>  }
>  static DEVICE_ATTR_RW(enable_c0_2);
>
> +static ssize_t
> +max_time_show(struct device *kobj, struct device_attribute *attr, char *buf)
> +{
> +       return sprintf(buf, "%u\n", umwait_max_time);
> +}
> +
> +static ssize_t max_time_store(struct device *kobj,
> +                             struct device_attribute *attr,
> +                             const char *buf, size_t count)
> +{
> +       u32 max_time;
> +       int ret;
> +
> +       ret = kstrtou32(buf, 0, &max_time);
> +       if (ret)
> +               return ret;
> +
> +       mutex_lock(&umwait_lock);
> +
> +       /* Only get max time value from bits[31:2] */
> +       max_time &= MSR_IA32_UMWAIT_CONTROL_MAX_TIME;

I think you should error out if high bits are set.  I'm okay with
masking off low bits, except that an input of 1 should not turn into
0, since 0 is special IIRC.