Re: [PATCH] hwmon: (dell-smm-hwmon) Disable BIOS fan control on SET_FAN

[Pali RohÃr]

Hi!

On Friday 15 November 2019 10:51:48 Giovanni Mascellani wrote:
> Hi,
> 
> Il 14/11/19 22:51, Pali RohÃr ha scritto:
> > This is model or BIOS specific. For example on E6440 are used 0x34a3 /
> > 0x35a3 SMM calls. Because of these platform specific problems we have
> > never incorporated this patch into mainline kernel.
> 
> Would it be sensible to use a dmi_system_id table to discriminate
> between the known models and choose the right commands? Of course we
> wouldn't know the complete table at the beginning, but it can be filled
> as unknown models are reported.

Yes, we can create a whitelist table with known models and correct SMM
commands.

> As a matter of facts, testing your patch I discovered that 0x34a3 /
> 0x35a3 work on my system as well (Dell Precision 5530). Do you know
> systems on which other codes only are known to work?

No. I have not tested that my patch on other models. You can look at
that my patch link, some people already reported that on some models it
does not work...

> > Also note that userspace can issue those SMM commands on its own (via
> > sys_iopl or sys_ioperm), fully bypassing such "protection" proposed in
> > this new patch.
> 
> Yes, I know, but this is incompatible with Secure Boot

What is incompatible with Secure Boot? sys_iopl nor sys_ioperm has
nothing to do with UEFI Secure Boot. They are just ordinary syscalls
like any other and are executed on kernel side. And IIRC it is up to the
kernel how it set privileges for I/O ports. Maybe bootloaders under
Secure Boot can set some other default values, but kernel can overwrite
them. I really do not see reason why it could be incompatible with UEFI
Secure Boot nor why it should not work. It just looks like improper
setup from userspace.

> so I believe
> that this feature should go in the kernel module, and userspace should
> eventually stop doing direct requests and rely on the module. Isn't
> userspace sidestepping the kernel in this way already assumed to take
> their own responsibilities, much like userspace writing random things to
> /dev/mem?

It makes sense to have implemented in kernel switching between automatic
and manual mode as kernel has API for it. But it depends on the fact
that we know which SMM API to call. And currently it is just some random
call which we somehow observed that is working on 2 machines and on some
more other does not. Until we have fully working implementation we
cannot put it into kernel.

What does not make sense for me is to have that "protection" in kernel.

-- 
Pali RohÃr
pali.rohar@xxxxxxxxx