Hi Mark, On 7/19/21 2:46 PM, Mark Pearson wrote: > Thanks for the review Hans > > On 2021-07-17 9:59 a.m., Hans de Goede wrote: >> Hi Mark, >> >> On 7/16/21 1:08 AM, Mark Pearson wrote: >>> Many Lenovo BIOS's support the ability to send a debug command which >>> is useful for debugging and testing unreleased or early features. >>> Adding support for this feature. >>> >>> Also moved the pending_reboot node under attributes dir where it should >>> correctly live. Got that wrong in my last commit and realised as I was >>> updating the documentation for debug_cmd >>> >>> Signed-off-by: Mark Pearson <markpearson@xxxxxxxxxx> >> >> Thank you for the patch, I'm not entirely sure if we want this in its >> current form though, isn't this new debug_cmd file not something which >> would be better under debugfs ? Or maybe have it only show up if >> a module parameter is passed to enable it ? >> >> Note that both have implications wrt secureboot. debugfs is only >> writeable when secureboot is disabled; and ATM module options are >> not protected by secureboot, but at least in Fedora we would actually >> like to change that in the future. >> >> Anyways, lets discuss this a bit further and then merge something >> for this later. >> > I wasn't sure about debugfs but did consider it. It seemed to be adding a whole extra layer. I'm happy to do that if that's what is recommended but not having it available with secureboot could be annoying for users. I agree that adding debugfs support just for a single file seems a bit overkill. > This feature is mostly used internally (which is fine) but we have had occasions where we've used it with customers; and with the WMI interface it's usually the enterprise customers who do have secure boot enabled. I'd like to avoid that limitation if possible. So given that using debugfs seems a bit overkill and it has issues with selinux I think that it might be best to hide this file behind a module option (and mention that in the docs, or maybe not document it at all?). At least for now kernel commandline options an be set without breaking secureboot and even if secureboot ever starts verifying the cmdline then we can always use a /etc/modprobe.d/*.conf file to specify the option instead of passing it through the kernel cmdline. Would using a module option to enable the debug_cmd file work for you ? Regards, Hans
