| Message ID | 20230420174623.2794634-1-robbarnes@google.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | platform/chrome: cros_ec: Report EC panic as uevent | expand |
On Thu, Apr 20, 2023 at 10:46 AM Rob Barnes <robbarnes@google.com> wrote: > > Create a uevent when an EC panic is detected. This will allow udev rules > to trigger when a panic occurs. E.g. a udev rule could be added to > create an EC coredump. This approach allows more code to be moved out > of the kernel drivers. Can you elaborate? What code do you plan on moving out of the kernel drivers? Is there a follow up patch which does this? Also, do you have a link to the userspace code that generates the coredump? > > Change-Id: I4fe7e4e603dcbf0ae6d4c4506f8978b0486abbbf Change-Id's are not accepted in upstream submissions. > Signed-off-by: Rob Barnes <robbarnes@google.com> > --- > drivers/platform/chrome/cros_ec_lpc.c | 20 ++++++++++++++++++++ > 1 file changed, 20 insertions(+) > > diff --git a/drivers/platform/chrome/cros_ec_lpc.c b/drivers/platform/chrome/cros_ec_lpc.c > index b399f7cbf17be..f2d5b00e6c3a0 100644 > --- a/drivers/platform/chrome/cros_ec_lpc.c > +++ b/drivers/platform/chrome/cros_ec_lpc.c > @@ -16,6 +16,7 @@ > #include <linux/delay.h> > #include <linux/io.h> > #include <linux/interrupt.h> > +#include <linux/kobject.h> > #include <linux/module.h> > #include <linux/platform_data/cros_ec_commands.h> > #include <linux/platform_data/cros_ec_proto.h> > @@ -313,6 +314,23 @@ static int cros_ec_lpc_readmem(struct cros_ec_device *ec, unsigned int offset, > return cnt; > } > > +/** > + * cros_ec_lpc_panic_uevent() - Publish EC panic uevent > + * @ec_dev: Device that panic'd > + * > + * This function will generate a uevent to notify userspace. > + * Typically the system will be reset shortly after an EC panic occurs. > + * > + * Return: 0 on success or <0 on error. > + */ > +static int cros_ec_lpc_panic_uevent(struct cros_ec_device *ec_dev) > +{ > + struct device *dev = ec_dev->dev; > + static char *env[] = { "ERROR=PANIC", NULL }; > + > + return kobject_uevent_env(&dev->kobj, KOBJ_CHANGE, env); > +} I think it's better to just inline this. I don't see any other user for this, and the return value is being ignored anyway. > + > static void cros_ec_lpc_acpi_notify(acpi_handle device, u32 value, void *data) > { > struct cros_ec_device *ec_dev = data; > @@ -324,6 +342,8 @@ static void cros_ec_lpc_acpi_notify(acpi_handle device, u32 value, void *data) > if (value == ACPI_NOTIFY_CROS_EC_PANIC) { > dev_emerg(ec_dev->dev, "CrOS EC Panic Reported. Shutdown is imminent!"); > blocking_notifier_call_chain(&ec_dev->panic_notifier, 0, ec_dev); > + cros_ec_lpc_panic_uevent(ec_dev); > + > /* Begin orderly shutdown. Force shutdown after 1 second. */ > hw_protection_shutdown("CrOS EC Panic", 1000); > /* Do not query for other events after a panic is reported */ > -- > 2.39.2 > >
On Thu, Apr 20, 2023 at 3:08 PM Prashant Malani <pmalani@chromium.org> wrote: > > On Thu, Apr 20, 2023 at 10:46 AM Rob Barnes <robbarnes@google.com> wrote: > > > > Create a uevent when an EC panic is detected. This will allow udev rules > > to trigger when a panic occurs. E.g. a udev rule could be added to > > create an EC coredump. This approach allows more code to be moved out > > of the kernel drivers. > > Can you elaborate? What code do you plan on moving out of the kernel drivers? > Is there a follow up patch which does this? > More accurately, it's preventing the need for more code to be added to the kernel drivers to support new features, like EC coredump. > Also, do you have a link to the userspace code that generates the coredump? > This CL adss a udev rule to trigger the crash collector on EC panic: https://chromium-review.googlesource.com/c/chromiumos/platform2/+/4450501 This WIP CL adds the EC coredump utility: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/4421417 This WIP CL adds a udev rule to trigger EC coredump utility on ec panic: https://chromium-review.googlesource.com/c/chromiumos/overlays/chromiumos-overlay/+/4424209 The WIP CLs should be ready for review later this week. > > > > Change-Id: I4fe7e4e603dcbf0ae6d4c4506f8978b0486abbbf > Change-Id's are not accepted in upstream submissions. > > > Signed-off-by: Rob Barnes <robbarnes@google.com> > > --- > > drivers/platform/chrome/cros_ec_lpc.c | 20 ++++++++++++++++++++ > > 1 file changed, 20 insertions(+) > > > > diff --git a/drivers/platform/chrome/cros_ec_lpc.c b/drivers/platform/chrome/cros_ec_lpc.c > > index b399f7cbf17be..f2d5b00e6c3a0 100644 > > --- a/drivers/platform/chrome/cros_ec_lpc.c > > +++ b/drivers/platform/chrome/cros_ec_lpc.c > > @@ -16,6 +16,7 @@ > > #include <linux/delay.h> > > #include <linux/io.h> > > #include <linux/interrupt.h> > > +#include <linux/kobject.h> > > #include <linux/module.h> > > #include <linux/platform_data/cros_ec_commands.h> > > #include <linux/platform_data/cros_ec_proto.h> > > @@ -313,6 +314,23 @@ static int cros_ec_lpc_readmem(struct cros_ec_device *ec, unsigned int offset, > > return cnt; > > } > > > > +/** > > + * cros_ec_lpc_panic_uevent() - Publish EC panic uevent > > + * @ec_dev: Device that panic'd > > + * > > + * This function will generate a uevent to notify userspace. > > + * Typically the system will be reset shortly after an EC panic occurs. > > + * > > + * Return: 0 on success or <0 on error. > > + */ > > +static int cros_ec_lpc_panic_uevent(struct cros_ec_device *ec_dev) > > +{ > > + struct device *dev = ec_dev->dev; > > + static char *env[] = { "ERROR=PANIC", NULL }; > > + > > + return kobject_uevent_env(&dev->kobj, KOBJ_CHANGE, env); > > +} > > I think it's better to just inline this. I don't see any other user for this, > and the return value is being ignored anyway. > > > + > > static void cros_ec_lpc_acpi_notify(acpi_handle device, u32 value, void *data) > > { > > struct cros_ec_device *ec_dev = data; > > @@ -324,6 +342,8 @@ static void cros_ec_lpc_acpi_notify(acpi_handle device, u32 value, void *data) > > if (value == ACPI_NOTIFY_CROS_EC_PANIC) { > > dev_emerg(ec_dev->dev, "CrOS EC Panic Reported. Shutdown is imminent!"); > > blocking_notifier_call_chain(&ec_dev->panic_notifier, 0, ec_dev); > > + cros_ec_lpc_panic_uevent(ec_dev); > > + > > /* Begin orderly shutdown. Force shutdown after 1 second. */ > > hw_protection_shutdown("CrOS EC Panic", 1000); > > /* Do not query for other events after a panic is reported */ > > -- > > 2.39.2 > > > >
diff --git a/drivers/platform/chrome/cros_ec_lpc.c b/drivers/platform/chrome/cros_ec_lpc.c index b399f7cbf17be..f2d5b00e6c3a0 100644 --- a/drivers/platform/chrome/cros_ec_lpc.c +++ b/drivers/platform/chrome/cros_ec_lpc.c @@ -16,6 +16,7 @@ #include <linux/delay.h> #include <linux/io.h> #include <linux/interrupt.h> +#include <linux/kobject.h> #include <linux/module.h> #include <linux/platform_data/cros_ec_commands.h> #include <linux/platform_data/cros_ec_proto.h> @@ -313,6 +314,23 @@ static int cros_ec_lpc_readmem(struct cros_ec_device *ec, unsigned int offset, return cnt; } +/** + * cros_ec_lpc_panic_uevent() - Publish EC panic uevent + * @ec_dev: Device that panic'd + * + * This function will generate a uevent to notify userspace. + * Typically the system will be reset shortly after an EC panic occurs. + * + * Return: 0 on success or <0 on error. + */ +static int cros_ec_lpc_panic_uevent(struct cros_ec_device *ec_dev) +{ + struct device *dev = ec_dev->dev; + static char *env[] = { "ERROR=PANIC", NULL }; + + return kobject_uevent_env(&dev->kobj, KOBJ_CHANGE, env); +} + static void cros_ec_lpc_acpi_notify(acpi_handle device, u32 value, void *data) { struct cros_ec_device *ec_dev = data; @@ -324,6 +342,8 @@ static void cros_ec_lpc_acpi_notify(acpi_handle device, u32 value, void *data) if (value == ACPI_NOTIFY_CROS_EC_PANIC) { dev_emerg(ec_dev->dev, "CrOS EC Panic Reported. Shutdown is imminent!"); blocking_notifier_call_chain(&ec_dev->panic_notifier, 0, ec_dev); + cros_ec_lpc_panic_uevent(ec_dev); + /* Begin orderly shutdown. Force shutdown after 1 second. */ hw_protection_shutdown("CrOS EC Panic", 1000); /* Do not query for other events after a panic is reported */
Create a uevent when an EC panic is detected. This will allow udev rules to trigger when a panic occurs. E.g. a udev rule could be added to create an EC coredump. This approach allows more code to be moved out of the kernel drivers. Change-Id: I4fe7e4e603dcbf0ae6d4c4506f8978b0486abbbf Signed-off-by: Rob Barnes <robbarnes@google.com> --- drivers/platform/chrome/cros_ec_lpc.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)