On 17 March 2017 at 10:47, Suzuki K Poulose [off-list ref] wrote:

On 17/03/17 16:28, Leo Yan wrote:

quoted

On Fri, Mar 17, 2017 at 09:50:07AM -0600, Mathieu Poirier wrote:

[...]

quoted

If we don't check for "nohlt" some platform may freeze, others may work.
If we
mandate that "nohlt" be present on the kernel cmd line it works in all
cases.
As such mandating that "nohlt" be present is a better way to go.

Sure, so I will add below checking code in the probe function, please
let me know if you have alter better way to implement this:

+       if (IS_ENABLED(CONFIG_CPU_IDLE) &&
+                       !strstr(boot_command_line, "nohlt")) {
+               dev_err(dev, "May not be accessible in CPU power
domain.\n");
+               return -EPERM;
+       }

There is an API which kind of achieves what "nohlt" does at runtime :

  cpu_idle_poll_ctrl(true)

So may be we could use that instead of depending on "nohlt". The other side
of the issues is "when do we decide to use the API". May be we could add
something
like : enable_debug, which could then trigger the panic notifier
registrations
and the above. That would still leave us with a case where the system
crashes
even before the user gets a terminal. May be the following is the best
option :

1) Dedicated kernel command line parameter for enabling the CPU debug at
boot/probe.

and

2) Runtime enable method via sysfs.

What do you think ?

In my opinion booting with "nohlt" on the cmd line is sufficient to
determine if we should use the driver or not.  That way we also avoid
declaring yet another sysfs flag, something I really want to avoid.

Thanks,
Mathieu

Suzuki