lkml.org 
[lkml]   [2015]   [May]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH 3/6] edac, mce_amd_inj: Modify flags attrigute to use string arguments
    On Wed, May 27, 2015 at 02:03:35PM -0500, Aravind Gopalakrishnan wrote:
    > Use char values such as "hw" or "sw" to indicate the type of error
    > injection to be performed.
    >
    > Current flags attribute derives the meanings of values that can be
    > programmed into it from asm/mce.h. Moving to defined strings for the
    > atribute allows this module to be self sufficient and removes the
    > dependency. Also, we can introduce new flags as and when needed without
    > having to worry about conflicting with the flags already defined
    > in asm/mce.h
    >
    > Also, modify do_inject() to use the newly defined injection_type enum
    > to figure out the injection mechanism we need to use
    >
    > Signed-off-by: Aravind Gopalakrishnan <Aravind.Gopalakrishnan@amd.com>
    > ---
    > drivers/edac/mce_amd_inj.c | 85 +++++++++++++++++++++++++++++++++++++++++-----
    > 1 file changed, 76 insertions(+), 9 deletions(-)
    >
    > diff --git a/drivers/edac/mce_amd_inj.c b/drivers/edac/mce_amd_inj.c
    > index 15f6aa1..daec4af 100644
    > --- a/drivers/edac/mce_amd_inj.c
    > +++ b/drivers/edac/mce_amd_inj.c
    > @@ -15,6 +15,8 @@
    > #include <linux/device.h>
    > #include <linux/module.h>
    > #include <linux/cpu.h>
    > +#include <linux/string.h>
    > +#include <linux/uaccess.h>
    > #include <asm/mce.h>
    >
    > #include "mce_amd.h"
    > @@ -27,6 +29,23 @@ static struct dentry *dfs_inj;
    >
    > static u8 n_banks;
    >
    > +#define MAX_FLAG_OPT_SIZE 10

    Why 10?

    This should be 2 and increased when another, longer injection type
    string gets introduced.

    > +
    > +enum injection_type {
    > + SW_INJ = 0, /* SW injection, simply decode the error */
    > + HW_INJ, /* Trigger a #MC */
    > + N_INJ_TYPES,
    > +};
    > +
    > +static const char * const flags_options[] = {
    > + [SW_INJ] = "sw",
    > + [HW_INJ] = "hw",
    > + NULL
    > +};


    > +
    > +/* Set default injection to SW_INJ */
    > +enum injection_type inj_type = SW_INJ;
    > +
    > #define MCE_INJECT_SET(reg) \
    > static int inj_##reg##_set(void *data, u64 val) \
    > { \
    > @@ -81,24 +100,72 @@ static int toggle_hw_mce_inject(unsigned int cpu, bool enable)
    > return err;
    > }
    >
    > -static int flags_get(void *data, u64 *val)
    > +static int __set_inj(const char *buf, size_t cnt)
    > {
    > - struct mce *m = (struct mce *)data;
    > + int i;
    > + const char *inj_op = NULL;
    >
    > - *val = m->inject_flags;
    > + for (i = 0; i <= N_INJ_TYPES; i++) {
    > + inj_op = flags_options[i];
    > + if (inj_op && strncmp(inj_op, buf, cnt) == 0)
    > + break;
    > + }

    What's wrong with:

    for (i = 0; i < N_INJ_TYPES; i++) {
    if (!strncmp(flags_options[i], buf, strlen(flags_options[i]))) {
    inj_type = i;
    return 0;
    }
    }

    return -EINVAL;

    > +
    > + if (!inj_op)
    > + return -EINVAL;
    > +
    > + inj_type = i;
    >
    > return 0;
    > }
    >
    > -static int flags_set(void *data, u64 val)
    > +static ssize_t flags_read(struct file *filp, char __user *ubuf,
    > + size_t cnt, loff_t *ppos)
    > {
    > - struct mce *m = (struct mce *)data;
    > + char buf[MAX_FLAG_OPT_SIZE + 1];
    > + int n;
    >
    > - m->inject_flags = (u8)val;
    > - return 0;
    > + n = sprintf(buf, "%s\n", flags_options[inj_type]);
    > +
    > + return simple_read_from_buffer(ubuf, cnt, ppos, buf, n);
    > }
    >
    > -DEFINE_SIMPLE_ATTRIBUTE(flags_fops, flags_get, flags_set, "%llu\n");
    > +static ssize_t flags_write(struct file *filp, const char __user *ubuf,
    > + size_t cnt, loff_t *ppos)
    > +{
    > + char buf[MAX_FLAG_OPT_SIZE + 1];
    > + int err;
    > + size_t ret;
    > +
    > + ret = cnt;

    You're assigning cnt to ret here...

    > +
    > + if (cnt > MAX_FLAG_OPT_SIZE)
    > + cnt = MAX_FLAG_OPT_SIZE;

    ... but correcting cnt afterwards. The assignment should be *after* that
    correction.

    > +
    > + if (copy_from_user(&buf, ubuf, cnt))
    > + return -EFAULT;
    > +
    > + buf[cnt] = 0;
    > +
    > + /* strip whitespaces.. */
    > + strstrip(buf);
    > +
    > + err = __set_inj(buf, cnt - 1);
    > + if (err) {
    > + pr_err("%s: Invalid flags value: %s\n", __func__, buf);
    > + return err;
    > + }
    > +
    > + *ppos += ret;
    > +
    > + return ret;
    > +}
    > +
    > +static const struct file_operations flags_fops = {
    > + .read = flags_read,
    > + .write = flags_write,
    > + .llseek = generic_file_llseek,
    > +};
    >
    > /*
    > * On which CPU to inject?
    > @@ -130,7 +197,7 @@ static void do_inject(void)
    > unsigned int cpu = i_mce.extcpu;
    > u8 b = i_mce.bank;
    >
    > - if (!(i_mce.inject_flags & MCJ_EXCEPTION)) {
    > + if (inj_type == SW_INJ) {
    > amd_decode_mce(NULL, 0, &i_mce);
    > return;
    > }
    > --
    > 2.4.0
    >
    >

    --
    Regards/Gruss,
    Boris.

    ECO tip #101: Trim your mails when you reply.
    --

    \
     
     \ /
      Last update: 2015-05-29 16:01    [W:4.200 / U:0.324 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site