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 <[email protected]>
> ---
>  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.
--
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to