diff mbox series

[2/3] wifi: iwlwifi: mvm: unify and fix interface combinations

Message ID 20240618200104.3213638262ef.I2a0031b37623d7763fd0c5405477ea7206a3e923@changeid (mailing list archive)
State Accepted
Delegated to: Johannes Berg
Headers show
Series wifi: iwlwifi: bugfixes - 18-06-24 | expand

Commit Message

Korenblit, Miriam Rachel June 18, 2024, 5:03 p.m. UTC 
From: Johannes Berg <johannes.berg@intel.com>

AP interfaces fundamentally cannot leave the channel, so multi-
channel operation with them isn't really possible. We shouldn't
advertise support for such, at least not as long as we don't
have full multi-radio support. Thus, remove the AP bit from the
interface combinations for two channels and add another set for
just one channel that has it.

Also, to avoid duplicating everything even more, unify the NAN
and non-NAN cases.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Reviewed-by: Ilan Peer <ilan.peer@intel.com>
Signed-off-by: Miri Korenblit <miriam.rachel.korenblit@intel.com>
---
 .../net/wireless/intel/iwlwifi/mvm/mac80211.c | 43 ++++++++++++-------
 1 file changed, 28 insertions(+), 15 deletions(-)

Comments

Alexander Wetzel July 2, 2024, 10:41 a.m. UTC | #1 
The commit 5c38bedac16a ("wifi: iwlwifi: mvm: unify and fix interface
combinations") breaks mvm hard. wlan interface can't be created and even
rmmod fails.

On driver load I get:

WARNING: CPU: 5 PID: 1358 at net/wireless/core.c:689 wiphy_register+0x8ee/0x920 [cfg80211]
Modules linked in: snd_ctl_led snd_soc_skl_hda_dsp(+) snd_soc_hdac_hdmi snd_soc_intel_hda_dsp_common snd_sof_probes snd_hda_codec_realtek snd_hda_codec_generic snd_hda_scodec_compon ent snd_soc_dmic snd_sof_pci_intel_tgl snd_sof_pci_intel_cnl snd_sof_intel_hda_generic soundwire_intel soundwire_cadence snd_sof_intel_hda_common snd_sof_intel_hda_mlink snd_sof_intel_hda snd_sof_pci snd_sof_x tensa_dsp snd_sof snd_sof_utils snd_soc_hdac_hda snd_soc_acpi_intel_match soundwire_generic_allocation snd_soc_acpi soundwire_bus snd_soc_avs snd_soc_hda_codec snd_hda_ext_core intel_uncore_frequency intel_unc ore_frequency_common intel_tcc_cooling snd_soc_core x86_pkg_temp_thermal intel_powerclamp coretemp snd_compress snd_hda_codec_hdmi ac97_bus iwlmvm(+) snd_pcm_dmaengine kvm_intel snd_hda_intel vfat kvm mac80211 snd_intel_dspcfg fat snd_intel_sdw_acpi snd_hda_codec iTCO_wdt nouveau(+) intel_pmc_bxt libarc4 rapl processor_thermal_device_pci intel_rapl_msr mei_wdt iTCO_vendor_support snd_hda_core processor_thermal_devi ce Jul 01 16:15:41 Ohm kernel:  snd_hwdep processor_thermal_wt_hint iwlwifi intel_cstate processor_thermal_rfim snd_pcm drm_ttm_helper processor_thermal_rapl gpu_sched intel_rapl_common nxp_nci_i2c snd_timer thin k_lmi(+) intel_uncore pcspkr firmware_attributes_class wmi_bmof nxp_nci snd mei_me processor_thermal_wt_req nci drm_gpuvm cfg80211 i2c_i801 soundcore mei drm_exec processor_thermal_power_floor idma64 i2c_smbus mxm_wmi processor_thermal_mbox nfc intel_pmc_core rfkill int3403_thermal intel_vsec int340x_thermal_zone int3400_thermal pmt_telemetry intel_hid acpi_thermal_rel pmt_class acpi_tad acpi_pad sparse_keymap joyd ev loop fuse nfnetlink mmc_block nvme nvme_core crct10dif_pclmul crc32_pclmul crc32c_intel rtsx_pci_sdmmc polyval_clmulni polyval_generic ghash_clmulni_intel sha512_ssse3 mmc_core sha256_ssse3 hid_multitouch u csi_acpi sha1_ssse3 typec_ucsi thunderbolt rtsx_pci vmd typec i2c_hid_acpi i2c_hid pinctrl_alderlake serio_raw
CPU: 5 PID: 1358 Comm: modprobe Not tainted 6.10.0-rc5-wt+ #17
Hardware name: LENOVO 21D6CTO1WW/21D6CTO1WW, BIOS N3FET34W (1.19 ) 03/10/2023
RIP: 0010:wiphy_register+0x8ee/0x920 [cfg80211]
Code: ef f8 ff ff 0f 0b e9 e8 f8 ff ff 0f 0b e9 e1 f8 ff ff 0f 0b e9 da f8 ff ff bf 01 00 00 00 e9 a4 fc ff ff 0f 0b e9 c9 f8 ff ff <0f> 0b e9 c2 f8 ff ff 0f 0b e9 f1 fe ff ff f7 d1 e9 d4 fe ff ff 0f
RSP: 0018:ffffc9000167f7d0 EFLAGS: 00010297
skl_hda_dsp_generic skl_hda_dsp_generic: hda_dsp_hdmi_build_controls: no PCM in topology for HDMI converter 3
RAX: 0000000000000300 RBX: ffff888121b983c0 RCX: 0000000000000300
RDX: ffffffffc11f2b28 RSI: ffffffffc11f2ae0 RDI: 0000000000000000
RBP: ffffc9000167f840 R08: 0000000000000002 R09: 0000000000000000
R10: 0000000000000003 R11: ffffffffc11f2b10 R12: ffffffffc11f2b28
R13: 0000000000000001 R14: 0000000000000005 R15: 000000000000000c
FS:  00007f9b91560080(0000) GS:ffff88902ee80000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007fbbe4015608 CR3: 00000001454da000 CR4: 0000000000f50ef0
PKRU: 55555554
Call Trace:
 <TASK>
 ? wiphy_register+0x8ee/0x920 [cfg80211]
 ? __warn.cold+0x8e/0xe8
 ? wiphy_register+0x8ee/0x920 [cfg80211]
 ? report_bug+0xfb/0x140
 ? handle_bug+0x3c/0x80
 ? exc_invalid_op+0x13/0x60
 ? asm_exc_invalid_op+0x16/0x20
 ? wiphy_register+0x8ee/0x920 [cfg80211]
 ? netdev_run_todo+0x60/0x4f0
 ? kvmalloc_node_noprof+0x36/0xc0
 ieee80211_register_hw+0x832/0xd60 [mac80211]
 iwl_mvm_mac_setup_register+0xb62/0xd50 [iwlmvm]
 iwl_mvm_start_post_nvm+0x5f/0xe0 [iwlmvm]
 iwl_op_mode_mvm_start+0x86a/0x990 [iwlmvm]
 _iwl_op_mode_start.isra.0+0x3f/0x80 [iwlwifi]
 iwl_opmode_register+0x6d/0xe0 [iwlwifi]
 ? __pfx_iwl_mvm_init+0x10/0x10 [iwlmvm]
 iwl_mvm_init+0x34/0xff0 [iwlmvm]
 ? __pfx_iwl_mvm_init+0x10/0x10 [iwlmvm]
 do_one_initcall+0x54/0x2e0
 do_init_module+0x82/0x230
 init_module_from_file+0x86/0xc0
 idempotent_init_module+0x121/0x2b0
 __x64_sys_finit_module+0x5a/0xb0
 do_syscall_64+0x7e/0x190
 ? vfs_read+0x152/0x370
 ? __rseq_handle_notify_resume+0xa2/0x480
 ? sysvec_call_function+0xa/0x90
 ? asm_sysvec_call_function+0x16/0x20
 ? __pfx_generic_file_llseek+0x10/0x10
 ? syscall_exit_to_user_mode+0x76/0x210
 ? do_syscall_64+0x8a/0x190
 ? __pfx_page_put_link+0x10/0x10
 ? do_sys_openat2+0x9c/0xe0
 ? syscall_exit_to_user_mode+0x76/0x210
 ? do_syscall_64+0x8a/0x190
 ? do_syscall_64+0x8a/0x190
 ? exc_page_fault+0x7a/0x180
 entry_SYSCALL_64_after_hwframe+0x76/0x7e
RIP: 0033:0x7f9b9167275d
Code: ff c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 9b 16 0d 00 f7 d8 64 89 01 48
RSP: 002b:00007ffff874b378 EFLAGS: 00000246 ORIG_RAX: 0000000000000139
RAX: ffffffffffffffda RBX: 000055b8e84a7ee0 RCX: 00007f9b9167275d
RDX: 0000000000000000 RSI: 000055b8d1da19ca RDI: 0000000000000002
RBP: 0000000000000000 R08: 0000000000000070 R09: 000055b8e84aa7e0
R10: 00007f9b91744b20 R11: 0000000000000246 R12: 000055b8d1da19ca
R13: 0000000000040000 R14: 000055b8e84a7ff0 R15: 0000000000000000
 </TASK>
---[ end trace 0000000000000000 ]---

After bisecting the issue and reverting the identified commit service is restored.
Christian Lamparter July 3, 2024, 6:16 p.m. UTC | #2 
On 7/2/24 12:41 PM, Alexander Wetzel wrote:
> The commit 5c38bedac16a ("wifi: iwlwifi: mvm: unify and fix interface
> combinations") breaks mvm hard. wlan interface can't be created and even
> rmmod fails.
> 
> On driver load I get:
> 
> WARNING: CPU: 5 PID: 1358 at net/wireless/core.c:689 wiphy_register+0x8ee/0x920 [cfg80211]
ran into the same issue. Debugging revealed that the " - 1" is the problem
because wiphy_verify_combinations checks if the specified limits in n_limits.max
adds up to at least max_interfaces (=3), which isn't true anymore with the
" - 1" as the ieee80211_iface_limit with NL80211_IFTYPE_P2P_DEVICE is missing.

Not sure, if this needs fixing in cfg80211 or iwlwifi. But I can confirm that this
patch "works" (not sure if it's correct/intended though).
---
diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c b/drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c
index 60bfe42d5386..e40f993b17fd 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c
@@ -59,13 +59,13 @@ static const struct ieee80211_iface_combination iwl_mvm_iface_combinations[] = {
  		.num_different_channels = 2,
  		.max_interfaces = 3,
  		.limits = iwl_mvm_limits,
-		.n_limits = ARRAY_SIZE(iwl_mvm_limits) - 1,
+		.n_limits = ARRAY_SIZE(iwl_mvm_limits),
  	},
  	{
  		.num_different_channels = 1,
  		.max_interfaces = 3,
  		.limits = iwl_mvm_limits_ap,
-		.n_limits = ARRAY_SIZE(iwl_mvm_limits_ap) - 1,
+		.n_limits = ARRAY_SIZE(iwl_mvm_limits_ap),
  	},
  };
---

relevant iw phy dump

         valid interface combinations:
                  * #{ managed } <= 1, #{ P2P-client, P2P-GO } <= 1, #{ P2P-device } <= 1,
                    total <= 3, #channels <= 2
                  * #{ managed } <= 1, #{ AP, P2P-client, P2P-GO } <= 1, #{ P2P-device } <= 1,
                    total <= 3, #channels <= 1


Cheers,
Christian
Johannes Berg July 3, 2024, 6:56 p.m. UTC | #3 
On Wed, 2024-07-03 at 20:16 +0200, Christian Lamparter wrote:
> On 7/2/24 12:41 PM, Alexander Wetzel wrote:
> > The commit 5c38bedac16a ("wifi: iwlwifi: mvm: unify and fix interface
> > combinations") breaks mvm hard. wlan interface can't be created and even
> > rmmod fails.
> > 
> > On driver load I get:
> > 
> > WARNING: CPU: 5 PID: 1358 at net/wireless/core.c:689 wiphy_register+0x8ee/0x920 [cfg80211]
> ran into the same issue. Debugging revealed that the " - 1" is the problem
> because wiphy_verify_combinations checks if the specified limits in n_limits.max
> adds up to at least max_interfaces (=3), which isn't true anymore with the
> " - 1" as the ieee80211_iface_limit with NL80211_IFTYPE_P2P_DEVICE is missing.
> 
> Not sure, if this needs fixing in cfg80211 or iwlwifi. But I can confirm that this
> patch "works" (not sure if it's correct/intended though).
> 

I sent the exact same patch yesterday :)

https://lore.kernel.org/20240702130001.8c871a3f0b5a.I08a6542f52f63c5bd66bf3feb09e1998ce7c60e5@changeid/

We have a version that has another array entry for NAN support and then
subtracts 1 here to add it conditionally again later etc., and this got
mixed up when sending out the patch.

johannes
diff mbox series

Patch

diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c b/drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c
index 18ce060df9b5..eeec425dc4d5 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c
@@ -30,21 +30,28 @@ 
 #include "iwl-nvm-parse.h"
 #include "time-sync.h"
 
+#define IWL_MVM_LIMITS(ap)					\
+	{							\
+		.max = 1,					\
+		.types = BIT(NL80211_IFTYPE_STATION),		\
+	},							\
+	{							\
+		.max = 1,					\
+		.types = ap |					\
+			 BIT(NL80211_IFTYPE_P2P_CLIENT) |	\
+			 BIT(NL80211_IFTYPE_P2P_GO),		\
+	},							\
+	{							\
+		.max = 1,					\
+		.types = BIT(NL80211_IFTYPE_P2P_DEVICE),	\
+	}
+
 static const struct ieee80211_iface_limit iwl_mvm_limits[] = {
-	{
-		.max = 1,
-		.types = BIT(NL80211_IFTYPE_STATION),
-	},
-	{
-		.max = 1,
-		.types = BIT(NL80211_IFTYPE_AP) |
-			BIT(NL80211_IFTYPE_P2P_CLIENT) |
-			BIT(NL80211_IFTYPE_P2P_GO),
-	},
-	{
-		.max = 1,
-		.types = BIT(NL80211_IFTYPE_P2P_DEVICE),
-	},
+	IWL_MVM_LIMITS(0)
+};
+
+static const struct ieee80211_iface_limit iwl_mvm_limits_ap[] = {
+	IWL_MVM_LIMITS(BIT(NL80211_IFTYPE_AP))
 };
 
 static const struct ieee80211_iface_combination iwl_mvm_iface_combinations[] = {
@@ -52,7 +59,13 @@  static const struct ieee80211_iface_combination iwl_mvm_iface_combinations[] = {
 		.num_different_channels = 2,
 		.max_interfaces = 3,
 		.limits = iwl_mvm_limits,
-		.n_limits = ARRAY_SIZE(iwl_mvm_limits),
+		.n_limits = ARRAY_SIZE(iwl_mvm_limits) - 1,
+	},
+	{
+		.num_different_channels = 1,
+		.max_interfaces = 3,
+		.limits = iwl_mvm_limits_ap,
+		.n_limits = ARRAY_SIZE(iwl_mvm_limits_ap) - 1,
 	},
 };