[kbuild-all] Re: [PATCH v3] ath10k: high latency fixes for beacon buffer

Hi Fabio, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on ath6kl/ath-next] [also build test WARNING on wireless-drivers-next/master wireless-drivers/master v5.14-rc6 next-20210819] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Fabio-Estevam/ath10k-high-latenc... base: https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git ath-next config: arc-randconfig-p001-20210818 (attached as .config) compiler: arceb-elf-gcc (GCC) 11.2.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/0day-ci/linux/commit/eb431ba513a87e043e8e8ba40f6f9509e... git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Fabio-Estevam/ath10k-high-latency-fixes-for-beacon-buffer/20210819-072934 git checkout eb431ba513a87e043e8e8ba40f6f9509e62111bd # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross ARCH=arc If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp(a)intel.com&gt; All warnings (new ones prefixed by >>): drivers/net/wireless/ath/ath10k/mac.c: In function 'ath10k_add_interface': 5586 | arvif->beacon_paddr = (dma_addr_t)arvif->beacon_buf; | ^ vim +5586 drivers/net/wireless/ath/ath10k/mac.c 5449 5450 /* 5451 * TODO: 5452 * Figure out how to handle WMI_VDEV_SUBTYPE_P2P_DEVICE, 5453 * because we will send mgmt frames without CCK. This requirement 5454 * for P2P_FIND/GO_NEG should be handled by checking CCK flag 5455 * in the TX packet. 5456 */ 5457 static int ath10k_add_interface(struct ieee80211_hw *hw, 5458 struct ieee80211_vif *vif) 5459 { 5460 struct ath10k *ar = hw->priv; 5461 struct ath10k_vif *arvif = (void *)vif->drv_priv; 5462 struct ath10k_peer *peer; 5463 enum wmi_sta_powersave_param param; 5464 int ret = 0; 5465 u32 value; 5466 int bit; 5467 int i; 5468 u32 vdev_param; 5469 5470 vif->driver_flags |= IEEE80211_VIF_SUPPORTS_UAPSD; 5471 5472 mutex_lock(&ar->conf_mutex); 5473 5474 memset(arvif, 0, sizeof(*arvif)); 5475 ath10k_mac_txq_init(vif->txq); 5476 5477 arvif->ar = ar; 5478 arvif->vif = vif; 5479 5480 INIT_LIST_HEAD(&arvif->list); 5481 INIT_WORK(&arvif->ap_csa_work, ath10k_mac_vif_ap_csa_work); 5482 INIT_DELAYED_WORK(&arvif->connection_loss_work, 5483 ath10k_mac_vif_sta_connection_loss_work); 5484 5485 for (i = 0; i < ARRAY_SIZE(arvif->bitrate_mask.control); i++) { 5486 arvif->bitrate_mask.control[i].legacy = 0xffffffff; 5487 memset(arvif->bitrate_mask.control[i].ht_mcs, 0xff, 5488 sizeof(arvif->bitrate_mask.control[i].ht_mcs)); 5489 memset(arvif->bitrate_mask.control[i].vht_mcs, 0xff, 5490 sizeof(arvif->bitrate_mask.control[i].vht_mcs)); 5491 } 5492 5493 if (ar->num_peers >= ar->max_num_peers) { 5494 ath10k_warn(ar, "refusing vdev creation due to insufficient peer entry resources in firmware\n"); 5495 ret = -ENOBUFS; 5496 goto err; 5497 } 5498 5499 if (ar->free_vdev_map == 0) { 5500 ath10k_warn(ar, "Free vdev map is empty, no more interfaces allowed.\n"); 5501 ret = -EBUSY; 5502 goto err; 5503 } 5504 bit = __ffs64(ar->free_vdev_map); 5505 5506 ath10k_dbg(ar, ATH10K_DBG_MAC, "mac create vdev %i map %llx\n", 5507 bit, ar->free_vdev_map); 5508 5509 arvif->vdev_id = bit; 5510 arvif->vdev_subtype = 5511 ath10k_wmi_get_vdev_subtype(ar, WMI_VDEV_SUBTYPE_NONE); 5512 5513 switch (vif->type) { 5514 case NL80211_IFTYPE_P2P_DEVICE: 5515 arvif->vdev_type = WMI_VDEV_TYPE_STA; 5516 arvif->vdev_subtype = ath10k_wmi_get_vdev_subtype 5517 (ar, WMI_VDEV_SUBTYPE_P2P_DEVICE); 5518 break; 5519 case NL80211_IFTYPE_UNSPECIFIED: 5520 case NL80211_IFTYPE_STATION: 5521 arvif->vdev_type = WMI_VDEV_TYPE_STA; 5522 if (vif->p2p) 5523 arvif->vdev_subtype = ath10k_wmi_get_vdev_subtype 5524 (ar, WMI_VDEV_SUBTYPE_P2P_CLIENT); 5525 break; 5526 case NL80211_IFTYPE_ADHOC: 5527 arvif->vdev_type = WMI_VDEV_TYPE_IBSS; 5528 break; 5529 case NL80211_IFTYPE_MESH_POINT: 5530 if (test_bit(WMI_SERVICE_MESH_11S, ar->wmi.svc_map)) { 5531 arvif->vdev_subtype = ath10k_wmi_get_vdev_subtype 5532 (ar, WMI_VDEV_SUBTYPE_MESH_11S); 5533 } else if (!test_bit(ATH10K_FLAG_RAW_MODE, &ar->dev_flags)) { 5534 ret = -EINVAL; 5535 ath10k_warn(ar, "must load driver with rawmode=1 to add mesh interfaces\n"); 5536 goto err; 5537 } 5538 arvif->vdev_type = WMI_VDEV_TYPE_AP; 5539 break; 5540 case NL80211_IFTYPE_AP: 5541 arvif->vdev_type = WMI_VDEV_TYPE_AP; 5542 5543 if (vif->p2p) 5544 arvif->vdev_subtype = ath10k_wmi_get_vdev_subtype 5545 (ar, WMI_VDEV_SUBTYPE_P2P_GO); 5546 break; 5547 case NL80211_IFTYPE_MONITOR: 5548 arvif->vdev_type = WMI_VDEV_TYPE_MONITOR; 5549 break; 5550 default: 5551 WARN_ON(1); 5552 break; 5553 } 5554 5555 /* Using vdev_id as queue number will make it very easy to do per-vif 5556 * tx queue locking. This shouldn't wrap due to interface combinations 5557 * but do a modulo for correctness sake and prevent using offchannel tx 5558 * queues for regular vif tx. 5559 */ 5560 vif->cab_queue = arvif->vdev_id % (IEEE80211_MAX_QUEUES - 1); 5561 for (i = 0; i < ARRAY_SIZE(vif->hw_queue); i++) 5562 vif->hw_queue[i] = arvif->vdev_id % (IEEE80211_MAX_QUEUES - 1); 5563 5564 /* Some firmware revisions don't wait for beacon tx completion before 5565 * sending another SWBA event. This could lead to hardware using old 5566 * (freed) beacon data in some cases, e.g. tx credit starvation 5567 * combined with missed TBTT. This is very rare. 5568 * 5569 * On non-IOMMU-enabled hosts this could be a possible security issue 5570 * because hw could beacon some random data on the air. On 5571 * IOMMU-enabled hosts DMAR faults would occur in most cases and target 5572 * device would crash. 5573 * 5574 * Since there are no beacon tx completions (implicit nor explicit) 5575 * propagated to host the only workaround for this is to allocate a 5576 * DMA-coherent buffer for a lifetime of a vif and use it for all 5577 * beacon tx commands. Worst case for this approach is some beacons may 5578 * become corrupted, e.g. have garbled IEs or out-of-date TIM bitmap. 5579 */ 5580 if (vif->type == NL80211_IFTYPE_ADHOC || 5581 vif->type == NL80211_IFTYPE_MESH_POINT || 5582 vif->type == NL80211_IFTYPE_AP) { 5583 if (ar->bus_param.dev_type == ATH10K_DEV_TYPE_HL) { 5584 arvif->beacon_buf = kmalloc(IEEE80211_MAX_FRAME_LEN, 5585 GFP_KERNEL); 5587 } else { 5588 arvif->beacon_buf = 5589 dma_alloc_coherent(ar->dev, 5590 IEEE80211_MAX_FRAME_LEN, 5591 &arvif->beacon_paddr, 5592 GFP_ATOMIC); 5593 } 5594 if (!arvif->beacon_buf) { 5595 ret = -ENOMEM; 5596 ath10k_warn(ar, "failed to allocate beacon buffer: %d\n", 5597 ret); 5598 goto err; 5599 } 5600 } 5601 if (test_bit(ATH10K_FLAG_HW_CRYPTO_DISABLED, &ar->dev_flags)) 5602 arvif->nohwcrypt = true; 5603 5604 if (arvif->nohwcrypt && 5605 !test_bit(ATH10K_FLAG_RAW_MODE, &ar->dev_flags)) { 5606 ret = -EINVAL; 5607 ath10k_warn(ar, "cryptmode module param needed for sw crypto\n"); 5608 goto err; 5609 } 5610 5611 ath10k_dbg(ar, ATH10K_DBG_MAC, "mac vdev create %d (add interface) type %d subtype %d bcnmode %s\n", 5612 arvif->vdev_id, arvif->vdev_type, arvif->vdev_subtype, 5613 arvif->beacon_buf ? "single-buf" : "per-skb"); 5614 5615 ret = ath10k_wmi_vdev_create(ar, arvif->vdev_id, arvif->vdev_type, 5616 arvif->vdev_subtype, vif->addr); 5617 if (ret) { 5618 ath10k_warn(ar, "failed to create WMI vdev %i: %d\n", 5619 arvif->vdev_id, ret); 5620 goto err; 5621 } 5622 5623 if (test_bit(WMI_SERVICE_VDEV_DISABLE_4_ADDR_SRC_LRN_SUPPORT, 5624 ar->wmi.svc_map)) { 5625 vdev_param = ar->wmi.vdev_param->disable_4addr_src_lrn; 5626 ret = ath10k_wmi_vdev_set_param(ar, arvif->vdev_id, vdev_param, 5627 WMI_VDEV_DISABLE_4_ADDR_SRC_LRN); 5628 if (ret && ret != -EOPNOTSUPP) { 5629 ath10k_warn(ar, "failed to disable 4addr src lrn vdev %i: %d\n", 5630 arvif->vdev_id, ret); 5631 } 5632 } 5633 5634 ar->free_vdev_map &= ~(1LL << arvif->vdev_id); 5635 spin_lock_bh(&ar->data_lock); 5636 list_add(&arvif->list, &ar->arvifs); 5637 spin_unlock_bh(&ar->data_lock); 5638 5639 /* It makes no sense to have firmware do keepalives. mac80211 already 5640 * takes care of this with idle connection polling. 5641 */ 5642 ret = ath10k_mac_vif_disable_keepalive(arvif); 5643 if (ret) { 5644 ath10k_warn(ar, "failed to disable keepalive on vdev %i: %d\n", 5645 arvif->vdev_id, ret); 5646 goto err_vdev_delete; 5647 } 5648 5649 arvif->def_wep_key_idx = -1; 5650 5651 vdev_param = ar->wmi.vdev_param->tx_encap_type; 5652 ret = ath10k_wmi_vdev_set_param(ar, arvif->vdev_id, vdev_param, 5653 ATH10K_HW_TXRX_NATIVE_WIFI); 5654 /* 10.X firmware does not support this VDEV parameter. Do not warn */ 5655 if (ret && ret != -EOPNOTSUPP) { 5656 ath10k_warn(ar, "failed to set vdev %i TX encapsulation: %d\n", 5657 arvif->vdev_id, ret); 5658 goto err_vdev_delete; 5659 } 5660 5661 /* Configuring number of spatial stream for monitor interface is causing 5662 * target assert in qca9888 and qca6174. 5663 */ 5664 if (ar->cfg_tx_chainmask && (vif->type != NL80211_IFTYPE_MONITOR)) { 5665 u16 nss = get_nss_from_chainmask(ar->cfg_tx_chainmask); 5666 5667 vdev_param = ar->wmi.vdev_param->nss; 5668 ret = ath10k_wmi_vdev_set_param(ar, arvif->vdev_id, vdev_param, 5669 nss); 5670 if (ret) { 5671 ath10k_warn(ar, "failed to set vdev %i chainmask 0x%x, nss %i: %d\n", 5672 arvif->vdev_id, ar->cfg_tx_chainmask, nss, 5673 ret); 5674 goto err_vdev_delete; 5675 } 5676 } 5677 5678 if (arvif->vdev_type == WMI_VDEV_TYPE_AP || 5679 arvif->vdev_type == WMI_VDEV_TYPE_IBSS) { 5680 ret = ath10k_peer_create(ar, vif, NULL, arvif->vdev_id, 5681 vif->addr, WMI_PEER_TYPE_DEFAULT); 5682 if (ret) { 5683 ath10k_warn(ar, "failed to create vdev %i peer for AP/IBSS: %d\n", 5684 arvif->vdev_id, ret); 5685 goto err_vdev_delete; 5686 } 5687 5688 spin_lock_bh(&ar->data_lock); 5689 5690 peer = ath10k_peer_find(ar, arvif->vdev_id, vif->addr); 5691 if (!peer) { 5692 ath10k_warn(ar, "failed to lookup peer %pM on vdev %i\n", 5693 vif->addr, arvif->vdev_id); 5694 spin_unlock_bh(&ar->data_lock); 5695 ret = -ENOENT; 5696 goto err_peer_delete; 5697 } 5698 5699 arvif->peer_id = find_first_bit(peer->peer_ids, 5700 ATH10K_MAX_NUM_PEER_IDS); 5701 5702 spin_unlock_bh(&ar->data_lock); 5703 } else { 5704 arvif->peer_id = HTT_INVALID_PEERID; 5705 } 5706 5707 if (arvif->vdev_type == WMI_VDEV_TYPE_AP) { 5708 ret = ath10k_mac_set_kickout(arvif); 5709 if (ret) { 5710 ath10k_warn(ar, "failed to set vdev %i kickout parameters: %d\n", 5711 arvif->vdev_id, ret); 5712 goto err_peer_delete; 5713 } 5714 } 5715 5716 if (arvif->vdev_type == WMI_VDEV_TYPE_STA) { 5717 param = WMI_STA_PS_PARAM_RX_WAKE_POLICY; 5718 value = WMI_STA_PS_RX_WAKE_POLICY_WAKE; 5719 ret = ath10k_wmi_set_sta_ps_param(ar, arvif->vdev_id, 5720 param, value); 5721 if (ret) { 5722 ath10k_warn(ar, "failed to set vdev %i RX wake policy: %d\n", 5723 arvif->vdev_id, ret); 5724 goto err_peer_delete; 5725 } 5726 5727 ret = ath10k_mac_vif_recalc_ps_wake_threshold(arvif); 5728 if (ret) { 5729 ath10k_warn(ar, "failed to recalc ps wake threshold on vdev %i: %d\n", 5730 arvif->vdev_id, ret); 5731 goto err_peer_delete; 5732 } 5733 5734 ret = ath10k_mac_vif_recalc_ps_poll_count(arvif); 5735 if (ret) { 5736 ath10k_warn(ar, "failed to recalc ps poll count on vdev %i: %d\n", 5737 arvif->vdev_id, ret); 5738 goto err_peer_delete; 5739 } 5740 } 5741 5742 ret = ath10k_mac_set_txbf_conf(arvif); 5743 if (ret) { 5744 ath10k_warn(ar, "failed to set txbf for vdev %d: %d\n", 5745 arvif->vdev_id, ret); 5746 goto err_peer_delete; 5747 } 5748 5749 ret = ath10k_mac_set_rts(arvif, ar->hw->wiphy->rts_threshold); 5750 if (ret) { 5751 ath10k_warn(ar, "failed to set rts threshold for vdev %d: %d\n", 5752 arvif->vdev_id, ret); 5753 goto err_peer_delete; 5754 } 5755 5756 arvif->txpower = vif->bss_conf.txpower; 5757 ret = ath10k_mac_txpower_recalc(ar); 5758 if (ret) { 5759 ath10k_warn(ar, "failed to recalc tx power: %d\n", ret); 5760 goto err_peer_delete; 5761 } 5762 5763 if (test_bit(WMI_SERVICE_RTT_RESPONDER_ROLE, ar->wmi.svc_map)) { 5764 vdev_param = ar->wmi.vdev_param->rtt_responder_role; 5765 ret = ath10k_wmi_vdev_set_param(ar, arvif->vdev_id, vdev_param, 5766 arvif->ftm_responder); 5767 5768 /* It is harmless to not set FTM role. Do not warn */ 5769 if (ret && ret != -EOPNOTSUPP) 5770 ath10k_warn(ar, "failed to set vdev %i FTM Responder: %d\n", 5771 arvif->vdev_id, ret); 5772 } 5773 5774 if (vif->type == NL80211_IFTYPE_MONITOR) { 5775 ar->monitor_arvif = arvif; 5776 ret = ath10k_monitor_recalc(ar); 5777 if (ret) { 5778 ath10k_warn(ar, "failed to recalc monitor: %d\n", ret); 5779 goto err_peer_delete; 5780 } 5781 } 5782 5783 spin_lock_bh(&ar->htt.tx_lock); 5784 if (!ar->tx_paused) 5785 ieee80211_wake_queue(ar->hw, arvif->vdev_id); 5786 spin_unlock_bh(&ar->htt.tx_lock); 5787 5788 mutex_unlock(&ar->conf_mutex); 5789 return 0; 5790 5791 err_peer_delete: 5792 if (arvif->vdev_type == WMI_VDEV_TYPE_AP || 5793 arvif->vdev_type == WMI_VDEV_TYPE_IBSS) { 5794 ath10k_wmi_peer_delete(ar, arvif->vdev_id, vif->addr); 5795 ath10k_wait_for_peer_delete_done(ar, arvif->vdev_id, 5796 vif->addr); 5797 } 5798 5799 err_vdev_delete: 5800 ath10k_wmi_vdev_delete(ar, arvif->vdev_id); 5801 ar->free_vdev_map |= 1LL << arvif->vdev_id; 5802 spin_lock_bh(&ar->data_lock); 5803 list_del(&arvif->list); 5804 spin_unlock_bh(&ar->data_lock); 5805 5806 err: 5807 if (arvif->beacon_buf) { 5808 if (ar->bus_param.dev_type == ATH10K_DEV_TYPE_HL) 5809 kfree(arvif->beacon_buf); 5810 else 5811 dma_free_coherent(ar->dev, IEEE80211_MAX_FRAME_LEN, 5812 arvif->beacon_buf, 5813 arvif->beacon_paddr); 5814 arvif->beacon_buf = NULL; 5815 } 5816 5817 mutex_unlock(&ar->conf_mutex); 5818 5819 return ret; 5820 } 5821 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org