lkml.org 
[lkml]   [2004]   [May]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [patch] bug in cpuid & msr on nosmp machine
    Rusty Russell <rusty@rustcorp.com.au> wrote:
    >
    > On Thu, 2004-05-20 at 17:32, Andrew Morton wrote:
    > > I think what you want here is
    > >
    > > if (!cpu_possible(cpu) || !cpu_online(cpu))
    > > return -ENXIO;
    >
    > It works, but it's not really correct. cpu_possible() is correct, but
    > cpu_online() might no longer be true by the time do_cpu_read() calls
    > do_cpu_id().

    mutter. Are we likely to see any ia32 or x86_64 hotplug-cpu machines?

    > One way would be to do lock_cpu_hotplug() in cpuid_open() and introduce
    > a cpuid_close() which would do unlock_cpu_hotplug(). Another would be
    > to drop the check here, and fail the actual read. A final way is to do
    > no checks, in which case it becomes a noop if the cpu is offline.

    Too many options for me!

    Doing lock_cpu_hotplug() in open() would allow an app to take that lock
    permanently.

    How about this?

    25-akpm/arch/i386/kernel/cpuid.c | 38 +++++++++++++++-----
    25-akpm/arch/i386/kernel/msr.c | 74 ++++++++++++++++++++++++++++-----------
    2 files changed, 85 insertions(+), 27 deletions(-)

    diff -puN arch/i386/kernel/cpuid.c~cpuid-msr-range-checking-fix arch/i386/kernel/cpuid.c
    --- 25/arch/i386/kernel/cpuid.c~cpuid-msr-range-checking-fix 2004-05-20 01:51:19.395701600 -0700
    +++ 25-akpm/arch/i386/kernel/cpuid.c 2004-05-20 01:51:19.400700840 -0700
    @@ -115,9 +115,19 @@ static ssize_t cpuid_read(struct file *f
    size_t rv;
    u32 reg = *ppos;
    int cpu = iminor(file->f_dentry->d_inode);
    + ssize_t ret = 0;

    - if (count % 16)
    - return -EINVAL; /* Invalid chunk size */
    + lock_cpu_hotplug();
    +
    + if (!cpu_possible(cpu) || !cpu_online(cpu)) {
    + ret = -ENXIO; /* No such CPU */
    + goto out;
    + }
    +
    + if (count % 16) {
    + ret = -EINVAL; /* Invalid chunk size */
    + goto out;
    + }

    for (rv = 0; count; count -= 16) {
    do_cpuid(cpu, reg, data);
    @@ -127,20 +137,32 @@ static ssize_t cpuid_read(struct file *f
    *ppos = reg++;
    }

    - return ((char *)tmp) - buf;
    + ret = ((char *)tmp) - buf;
    +out:
    + lock_cpu_hotplug();
    + return ret;
    }

    static int cpuid_open(struct inode *inode, struct file *file)
    {
    int cpu = iminor(file->f_dentry->d_inode);
    - struct cpuinfo_x86 *c = &(cpu_data)[cpu];
    + struct cpuinfo_x86 *c;
    + int ret = 0;

    - if (!cpu_online(cpu))
    - return -ENXIO; /* No such CPU */
    + lock_cpu_hotplug();
    +
    + if (!cpu_possible(cpu) || !cpu_online(cpu)) {
    + ret = -ENXIO; /* No such CPU */
    + goot out;
    + }
    +
    + c = &(cpu_data)[cpu];
    if (c->cpuid_level < 0)
    - return -EIO; /* CPUID not supported */
    + ret = -EIO; /* CPUID not supported */

    - return 0;
    +out:
    + unlock_cpu_hotplug();
    + return ret;
    }

    /*
    diff -puN arch/i386/kernel/msr.c~cpuid-msr-range-checking-fix arch/i386/kernel/msr.c
    --- 25/arch/i386/kernel/msr.c~cpuid-msr-range-checking-fix 2004-05-20 01:51:19.396701448 -0700
    +++ 25-akpm/arch/i386/kernel/msr.c 2004-05-20 01:51:19.401700688 -0700
    @@ -195,20 +195,37 @@ static ssize_t msr_read(struct file * fi
    u32 reg = *ppos;
    int cpu = iminor(file->f_dentry->d_inode);
    int err;
    + ssize_t ret = 0;

    - if ( count % 8 )
    - return -EINVAL; /* Invalid chunk size */
    + lock_cpu_hotplug();
    +
    + if (!cpu_possible(cpu) || !cpu_online(cpu)) {
    + ret = -ENXIO; /* No such CPU */
    + goto out;
    + }
    +
    + if ( count % 8 ) {
    + ret = -EINVAL; /* Invalid chunk size */
    + goto out;
    + }

    for ( rv = 0 ; count ; count -= 8 ) {
    err = do_rdmsr(cpu, reg, &data[0], &data[1]);
    - if ( err )
    - return err;
    - if ( copy_to_user(tmp,&data,8) )
    - return -EFAULT;
    + if ( err ) {
    + ret = err;
    + goto out;
    + }
    + if ( copy_to_user(tmp,&data,8) ) {
    + ret = -EFAULT;
    + goto out;
    + }
    tmp += 2;
    }

    - return ((char *)tmp) - buf;
    + ret = ((char *)tmp) - buf;
    +out:
    + unlock_cpu_hotplug();
    + return ret;
    }

    static ssize_t msr_write(struct file * file, const char __user * buf,
    @@ -220,29 +237,48 @@ static ssize_t msr_write(struct file * f
    u32 reg = *ppos;
    int cpu = iminor(file->f_dentry->d_inode);
    int err;
    + int ret = 0;
    +
    + lock_cpu_hotplug();
    +
    + if (!cpu_possible(cpu) || !cpu_online(cpu)) {
    + ret = -ENXIO; /* No such CPU */
    + goto out;
    + }
    +
    + if ( count % 8 ) {
    + ret = -EINVAL; /* Invalid chunk size */
    + goto out;
    + }

    - if ( count % 8 )
    - return -EINVAL; /* Invalid chunk size */
    -
    for ( rv = 0 ; count ; count -= 8 ) {
    - if ( copy_from_user(&data,tmp,8) )
    - return -EFAULT;
    + if ( copy_from_user(&data,tmp,8) ) {
    + ret = -EFAULT;
    + goto out;
    + }
    err = do_wrmsr(cpu, reg, data[0], data[1]);
    - if ( err )
    - return err;
    + if ( err ) {
    + ret = err;
    + goto out;
    + }
    tmp += 2;
    }

    - return ((char *)tmp) - buf;
    + ret = ((char *)tmp) - buf;
    +out:
    + unlock_cpu_hotplug();
    + return ret;
    }

    static int msr_open(struct inode *inode, struct file *file)
    {
    int cpu = iminor(file->f_dentry->d_inode);
    - struct cpuinfo_x86 *c = &(cpu_data)[cpu];
    -
    - if (!cpu_online(cpu))
    - return -ENXIO; /* No such CPU */
    + struct cpuinfo_x86 *c;
    +
    + if (!cpu_possible(cpu) || !cpu_online(cpu))
    + return -ENXIO; /* No such CPU */
    +
    + c = &(cpu_data)[cpu];
    if ( !cpu_has(c, X86_FEATURE_MSR) )
    return -EIO; /* MSR not supported */

    _

    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2005-03-22 14:03    [W:0.032 / U:59.548 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site