Re: [PATCH v6 2/7] iwlwifi: mei: add the driver to allow cooperation with CSME

[Kalle Valo <kvalo@codeaurora.org>]

Emmanuel Grumbach <egrumbach@gmail.com> writes:

> On Mon, Oct 18, 2021 at 1:58 PM Kalle Valo <kvalo@codeaurora.org> wrote:
>>
>> Emmanuel Grumbach <emmanuel.grumbach@intel.com> writes:
>>
>> > CSME in two words
>> > -----------------
>> > CSME stands for Converged Security and Management Engine. It is
>> > a CPU on the chipset and runs a dedicated firmware.
>> > AMT (Active Management Technology) is one of the applications
>> > that run on that CPU. AMT allows to control the platform remotely.
>> > Here is a partial list of the use cases:
>> > * View the screen of the plaform, with keyboard and mouse (KVM)
>> > * Attach a remote IDE device
>> > * Have a serial console to the device
>> > * Query the state of the platform
>> > * Reset / shut down / boot the platform
>>
>> [...]
>>
>> > --- a/drivers/net/wireless/intel/iwlwifi/Kconfig
>> > +++ b/drivers/net/wireless/intel/iwlwifi/Kconfig
>> > @@ -92,6 +92,23 @@ config IWLWIFI_BCAST_FILTERING
>> >         If unsure, don't enable this option, as some programs might
>> >         expect incoming broadcasts for their normal operations.
>> >
>> > +config IWLMEI
>> > +     tristate "Intel Management Engine communication over WLAN"
>> > +     depends on INTEL_MEI
>> > +     depends on PM
>> > +     depends on IWLMVM
>> > +     help
>> > +       Enables the iwlmei kernel module. This allows to communicate with
>> > +       the Intel Management Engine over Wifi. This is supported starting
>> > +       from Tiger Lake platforms and has been tested on 9260 devices only.
>> > +       Enabling this option on a platform that has a different device and
>> > +       has Wireless enabled on AMT can prevent WiFi from working correctly.
>>
>> I don't understand the last sentence, please elaborate. Are you saying
>> that enabling CONFIG_IWLMEI will break wi-fi in some cases?
>
> Since we don't support all the devices (yet), and we haven't integrated it
> with all our devices, then yes, it shouldn't be enabled unless you know
> exactly what platform this kernel runs on. 

So if I'm understanding correctly, enabling CONFIG_IWLMEI will break
existing iwlwifi setups? Or am I missing something?

If that's the case when we have a major problem as that would be a
regression. A kconfig option is not a free pass to do all sort of crazy
stuff, and especially it cannot break already working hardware. That
would be considered as a regression and we have a strong rule for
regressions.

> This is basically a warning for distros not to enable this kernel
> option (just as many others that are specific to a certain use case).
> The plan is to add support for more and more devices so that we will
> be able to remove this comment and be confident that it can be enabled
> on generic platforms as well.

I barely missed the warning and I consider myself as an extra careful
person, I can only imagine how many regular users will enable this
option without thinking twice. And people can use allmodconfig etc as
well.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches