On Wed, Nov 10, 2021 at 11:40 AM Kalle Valo <kvalo@xxxxxxxxxxxxxx> wrote: > > Emmanuel Grumbach <egrumbach@xxxxxxxxx> writes: > > >> > >> Emmanuel Grumbach <egrumbach@xxxxxxxxx> writes: > >> > >> > On Mon, Oct 18, 2021 at 1:58 PM Kalle Valo <kvalo@xxxxxxxxxxxxxx> wrote: > >> >> > >> >> Emmanuel Grumbach <emmanuel.grumbach@xxxxxxxxx> 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? > > > > Ok, I'll share more details. > > The coexistence between iwlwifi and CSME has never really been taken care > > of. CSME is a feature meant for enterprise and those systems typically run > > Windows which is why the Windows driver has had this feature for years > > but not Linux. > > iwlmei will not do anything if you don't have CSME configured to use the > > WLAN device. Someone needs to actively configure CSME to use WLAN for > > this to happen. Very few systems have this configuration. > > So in a case when CSME/AMT WLAN (or whatever the feature under > discussion is called) is disabled in the BIOS and CONFIG_IWLMEI is > enabled in the kernel build, iwlwifi will work as before? This is the > most important part for me, we must not break people's network access > when they upgrade the kernel. Yes. We need another fix that is not in v7 for a bug we found in our testing, but when the user doesn't use AMT, nothing will happen even if IWLMEI is compiled. > > > Now the question is what happens today (without iwlmei) when CSME is > > configured to use WLAN compared to what happens with iwlmei, or in other > > words, do we have a "regression"? > > > > Today: there is some mechanism that allows iwlwifi to tell CSME not to touch > > the device. This mechanism is not very well tested because only Linux uses it > > and the assumption in Linux has almost always been that CSME is not > > configured on systems that run Linux. > > > > with iwlmei: we will be using the same interface as windows towards CSME which > > is a well tested interface and we will be able to use the features > > that CSME provides > > > > Why the warning then? The integration with CSME has been done on 9260 > > only and not on newer devices. We do have plans to add support for > > more devices but the current code doesn't support them. > > > > What will happen if a user enables iwlmei on a system that has a > > post-9260 device and CSME configured to use WLAN? Most probably, it'll > > work, but this has not been tested yet > > I hope you are printing an understandable info message when CSME is > enabled so that the situation is easy to notice and debug. > > > This configuration is far from the default and considered illegal by > > Intel at this point I know that a user can always do something stupid, > > but he can likewise disable mac80211 and complain that his wifi > > doesn't work. > > A user cannot compile iwlwifi unless mac80211 is enabled and likewise > iwlwifi module won't load unless mac80211 is loaded first. So we do have > checks for a lot of stuff. > > > My suggestion is to add a WARNING (or an dev_err print) in iwlwifi if > > we detect this situation so that it'll be very easy to catch this > > problem in the configuration. > > Heh, exactly what I was thinking above. No problem. I'll do that. Our testing teams are right now working on testing more devices. > > > The only way to prevent this situation would be to know what device > > the user has from iwlmei in the mei bus enumeration but this is > > unfortunately not possible. > > > > I hope that made things clearer. I know all this is confusing, many > > pieces of details that I hoped I wouldn't have to bore you with :) > > It would have been a lot easier and faster to mention all this in the > commit logs from the beginning. Dumping me patches without commit logs > and no clear explanation is not really building confidence for me. > I understand. There are loads of details and I didn't really want to bother you with all of them, I guess I now know that you are the type of person who want the details and I'll be clearer the next time. Practically speaking, I owe you a v8 with the WARNING I mentioned plus the bugfix I talked about. Do you want it now or you want me to wait for more comments?