Re: [PATCH v4 4/4] hwmon: (occ) Provide the SBEFIFO FFDC in binary sysfs

[Guenter Roeck <linux@xxxxxxxxxxxx>]

On 10/21/21 4:09 PM, Joel Stanley wrote:
> On Tue, 19 Oct 2021 at 20:53, Eddie James <eajames@xxxxxxxxxxxxx> wrote:
> > Save any FFDC provided by the OCC driver, and provide it to userspace
> > through a binary sysfs entry. Notify userspace pollers when there is an
> > error too.
> >
> > Signed-off-by: Eddie James <eajames@xxxxxxxxxxxxx>
> > Reviewed-by: Guenter Roeck <linux@xxxxxxxxxxxx>
>
> Guenter, I will merge this through the fsi tree unless you have objections.

Go ahead.

Guenter

> Cheers,
>
> Joel
>
> > ---
> >   drivers/hwmon/occ/p9_sbe.c | 86 +++++++++++++++++++++++++++++++++++++-
> >   1 file changed, 85 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/hwmon/occ/p9_sbe.c b/drivers/hwmon/occ/p9_sbe.c
> > index 9709f2b9c052..e50243580269 100644
> > --- a/drivers/hwmon/occ/p9_sbe.c
> > +++ b/drivers/hwmon/occ/p9_sbe.c
> > @@ -4,18 +4,79 @@
> >   #include <linux/device.h>
> >   #include <linux/errno.h>
> >   #include <linux/fsi-occ.h>
> > +#include <linux/mm.h>
> >   #include <linux/module.h>
> > +#include <linux/mutex.h>
> >   #include <linux/platform_device.h>
> > +#include <linux/string.h>
> > +#include <linux/sysfs.h>
> >
> >   #include "common.h"
> >
> >   struct p9_sbe_occ {
> >          struct occ occ;
> > +       bool sbe_error;
> > +       void *ffdc;
> > +       size_t ffdc_len;
> > +       size_t ffdc_size;
> > +       struct mutex sbe_error_lock;    /* lock access to ffdc data */
> >          struct device *sbe;
> >   };
> >
> >   #define to_p9_sbe_occ(x)       container_of((x), struct p9_sbe_occ, occ)
> >
> > +static ssize_t ffdc_read(struct file *filp, struct kobject *kobj,
> > +                        struct bin_attribute *battr, char *buf, loff_t pos,
> > +                        size_t count)
> > +{
> > +       ssize_t rc = 0;
> > +       struct occ *occ = dev_get_drvdata(kobj_to_dev(kobj));
> > +       struct p9_sbe_occ *ctx = to_p9_sbe_occ(occ);
> > +
> > +       mutex_lock(&ctx->sbe_error_lock);
> > +       if (ctx->sbe_error) {
> > +               rc = memory_read_from_buffer(buf, count, &pos, ctx->ffdc,
> > +                                            ctx->ffdc_len);
> > +               if (pos >= ctx->ffdc_len)
> > +                       ctx->sbe_error = false;
> > +       }
> > +       mutex_unlock(&ctx->sbe_error_lock);
> > +
> > +       return rc;
> > +}
> > +static BIN_ATTR_RO(ffdc, OCC_MAX_RESP_WORDS * 4);
> > +
> > +static bool p9_sbe_occ_save_ffdc(struct p9_sbe_occ *ctx, const void *resp,
> > +                                size_t resp_len)
> > +{
> > +       bool notify = false;
> > +
> > +       mutex_lock(&ctx->sbe_error_lock);
> > +       if (!ctx->sbe_error) {
> > +               if (resp_len > ctx->ffdc_size) {
> > +                       if (ctx->ffdc)
> > +                               kvfree(ctx->ffdc);
> > +                       ctx->ffdc = kvmalloc(resp_len, GFP_KERNEL);
> > +                       if (!ctx->ffdc) {
> > +                               ctx->ffdc_len = 0;
> > +                               ctx->ffdc_size = 0;
> > +                               goto done;
> > +                       }
> > +
> > +                       ctx->ffdc_size = resp_len;
> > +               }
> > +
> > +               notify = true;
> > +               ctx->sbe_error = true;
> > +               ctx->ffdc_len = resp_len;
> > +               memcpy(ctx->ffdc, resp, resp_len);
> > +       }
> > +
> > +done:
> > +       mutex_unlock(&ctx->sbe_error_lock);
> > +       return notify;
> > +}
> > +
> >   static int p9_sbe_occ_send_cmd(struct occ *occ, u8 *cmd, size_t len)
> >   {
> >          struct occ_response *resp = &occ->resp;
> > @@ -24,8 +85,15 @@ static int p9_sbe_occ_send_cmd(struct occ *occ, u8 *cmd, size_t len)
> >          int rc;
> >
> >          rc = fsi_occ_submit(ctx->sbe, cmd, len, resp, &resp_len);
> > -       if (rc < 0)
> > +       if (rc < 0) {
> > +               if (resp_len) {
> > +                       if (p9_sbe_occ_save_ffdc(ctx, resp, resp_len))
> > +                               sysfs_notify(&occ->bus_dev->kobj, NULL,
> > +                                            bin_attr_ffdc.attr.name);
> > +               }
> > +
> >                  return rc;
> > +       }
> >
> >          switch (resp->return_status) {
> >          case OCC_RESP_CMD_IN_PRG:
> > @@ -65,6 +133,8 @@ static int p9_sbe_occ_probe(struct platform_device *pdev)
> >          if (!ctx)
> >                  return -ENOMEM;
> >
> > +       mutex_init(&ctx->sbe_error_lock);
> > +
> >          ctx->sbe = pdev->dev.parent;
> >          occ = &ctx->occ;
> >          occ->bus_dev = &pdev->dev;
> > @@ -78,6 +148,15 @@ static int p9_sbe_occ_probe(struct platform_device *pdev)
> >          if (rc == -ESHUTDOWN)
> >                  rc = -ENODEV;   /* Host is shutdown, don't spew errors */
> >
> > +       if (!rc) {
> > +               rc = device_create_bin_file(occ->bus_dev, &bin_attr_ffdc);
> > +               if (rc) {
> > +                       dev_warn(occ->bus_dev,
> > +                                "failed to create SBE error ffdc file\n");
> > +                       rc = 0;
> > +               }
> > +       }
> > +
> >          return rc;
> >   }
> >
> > @@ -86,9 +165,14 @@ static int p9_sbe_occ_remove(struct platform_device *pdev)
> >          struct occ *occ = platform_get_drvdata(pdev);
> >          struct p9_sbe_occ *ctx = to_p9_sbe_occ(occ);
> >
> > +       device_remove_bin_file(occ->bus_dev, &bin_attr_ffdc);
> > +
> >          ctx->sbe = NULL;
> >          occ_shutdown(occ);
> >
> > +       if (ctx->ffdc)
> > +               kvfree(ctx->ffdc);
> > +
> >          return 0;
> >   }
> >
> > --
> > 2.27.0