* Re: [PATCH v2 0/4] Extended Key ID support
From: Johannes Berg @ 2019-04-08 19:57 UTC (permalink / raw)
To: Alexander Wetzel; +Cc: linux-wireless
In-Reply-To: <20190319203410.25145-1-alexander@wetzel-home.de>
On Tue, 2019-03-19 at 21:34 +0100, Alexander Wetzel wrote:
> This patch series adds support for IEEE 802.11-2016 Extended Key ID
> support. Compared to the last RFC there are again quite some API
> changes, but also some bug fixes. (The bug fixes I remember are outlined
> in the different patches.)
FWIW, I've applied the first two patches here.
I'd really like you to continue with only that for now, and (try to) get
hostapd/wpa_supplicant changes upstream, perhaps with corresponding
hwsim tests. That way, we can see this working live.
We briefly discussed this at the wireless workshop, and the general
feeling seems to have been that the compat mode is probably not really
worthwhile, but I haven't quite made up my mind on that.
Still, having tests and being able to check it out would help a lot,
also as part of perhaps building a case for compat mode.
Thanks,
johannes
^ permalink raw reply
* Re: Flag for detecting 802.11r Fast BSS Transition support
From: Johannes Berg @ 2019-04-08 19:52 UTC (permalink / raw)
To: Brian Norris, Matthew Wang; +Cc: linux-wireless, Kirtika Ruchandani
In-Reply-To: <20190403210200.GA93453@google.com>
On Wed, 2019-04-03 at 14:02 -0700, Brian Norris wrote:
> + Johannes
>
> On Thu, Oct 04, 2018 at 12:06:50PM -0700, Matthew Wang wrote:
> > Hi,
> >
> > I'm wondering if there is a flag for detecting firmware/driver support
> > for FT. It seems like checking for SME support is a pretty good proxy
> > for this, but there is a non-mac80211 driver that can do FT as well
> > (ath/wil6210). It would be great if anyone knows of a feature flag
> > specifically for FT.
>
> I chatted with Johannes, and he agreed that there was no such flag
> today. It also sounded like he was open to adding one, even if it's
> several years too late. I don't think there's any useful way people
> could (generically) use FT support today.
I guess I'm not really sure if the flag is needed, but I guess like the
original question says, it'd be more about non-mac80211 drivers.
So I guess you'd have to figure out what operations the drivers need to
support then? I'm not even sure how wpa_s would handle this for SME
offload devices.
johannes
^ permalink raw reply
* Re: [RFC V3 1/2] mac80211: add hw 80211 encapsulation offloading support
From: Johannes Berg @ 2019-04-08 19:28 UTC (permalink / raw)
To: John Crispin, Kalle Valo
Cc: linux-wireless, Shashidhar Lakkavalli, Vasanthakumar Thiagarajan
In-Reply-To: <20190401131416.22646-2-john@phrozen.org>
On Mon, 2019-04-01 at 15:14 +0200, John Crispin wrote:
>
> + * @IEEE80211_HW_SUPPORTS_80211_ENCAP: Hardware/driver supports 802.11
> + * encap for data frames.
What's this needed for,
> +void ieee80211_set_hw_80211_encap(struct ieee80211_vif *vif, bool enable);
if you have this?
But then again this one's missing docs so I don't yet know what it's
actually doing :-)
> /* reject WEP and TKIP keys if WEP failed to initialize */
> switch (params->cipher) {
> - case WLAN_CIPHER_SUITE_WEP40:
> case WLAN_CIPHER_SUITE_TKIP:
> + /* countermeasures wont work on encap offload mode so reject
> + * TKIP keys
> + */
> + if (ieee80211_hw_check(&local->hw, SUPPORTS_80211_ENCAP))
> + return -EINVAL;
> +
> + /* drop through */
"fall through" I believe is needed to shut up the compiler about it.
But is this really a good idea? To do this completely only on the
support check, vs. what's currently active?
And I think it should probably just disable the encap offload, rather
than reject TKIP.
> @@ -2379,6 +2386,9 @@ static int ieee80211_set_wiphy_params(struct wiphy *wiphy, u32 changed)
> if (changed & WIPHY_PARAM_FRAG_THRESHOLD) {
> ieee80211_check_fast_xmit_all(local);
>
> + if (ieee80211_is_hw_80211_encap(local))
> + return -EINVAL;
That seems misplaced? After checking fast_xmit? But again, similar as
above - IMHO it'd be better to disable encap offload.
> +void ieee80211_set_hw_80211_encap(struct ieee80211_vif *vif, bool enable)
> +{
> + struct ieee80211_sub_if_data *sdata = vif_to_sdata(vif);
> + struct ieee80211_local *local = sdata->local;
> +
> + if (!sdata->dev)
> + return;
> +
> + if (!ieee80211_hw_check(&local->hw, SUPPORTS_80211_ENCAP))
> + enable = 0;
false?
This should probably be a WARN_ON(), after all the driver is calling
this?
> + if (!ieee80211_hw_check(&local->hw, SUPPORTS_TX_FRAG) &&
> + (local->hw.wiphy->frag_threshold != (u32)-1))
> + enable = 0;
The driver might need to know that you overrode it, so return the actual
value?
This is also asymmetric with my comment above - if you don't allow
enabling it while fragmentation is enabled, but at the same time don't
disable it when trying to enable fragmentation, suddenly the order of
the two operations starts to matter and that might be rather confusing.
> + if (enable) {
> + sdata->dev->netdev_ops = &ieee80211_dataif_8023_ops;
Can we really play with that pointer like that? Though I guess the two
are identical except for the TX so it probably won't really matter.
> +bool ieee80211_is_hw_80211_encap(struct ieee80211_local *local)
Maybe rename to "have_vif_with" instead of "is"? That gets a bit long,
but you get the point.
> +++ b/net/mac80211/key.c
> @@ -197,6 +197,9 @@ static int ieee80211_key_enable_hw_accel(struct ieee80211_key *key)
> key->conf.keyidx,
> sta ? sta->sta.addr : bcast_addr, ret);
>
> + if (sdata->hw_80211_encap)
> + return -EINVAL;
> +
Seems that should be inside the "out_unsupported" label:
> out_unsupported:
> switch (key->conf.cipher) {
> case WLAN_CIPHER_SUITE_WEP40:
or possibly even inside the switch.
> + if (ieee80211_hw_check(hw, SUPPORTS_80211_ENCAP)) {
> + /* mac80211 always supports monitor unless we do 802.11
> + * encapsulation offloading.
> + */
> + hw->wiphy->interface_modes |= BIT(NL80211_IFTYPE_MONITOR);
> + hw->wiphy->software_iftypes |= BIT(NL80211_IFTYPE_MONITOR);
> + }
I really think we need this to be more flexible, Intel hardware for
example can do encap offload per queue, so theoretically we could even
enable/disable it per *station*, but I'd settle for per *interface*.
Certainly we don't want to suddenly find that monitor mode is limited
when we enable this?
Also, you still always have a way to disable it, right?
> /* mac80211 doesn't support more than one IBSS interface right now */
> for (i = 0; i < hw->wiphy->n_iface_combinations; i++) {
> diff --git a/net/mac80211/status.c b/net/mac80211/status.c
> index 5b9952b1caf3..8feafaab88a4 100644
> --- a/net/mac80211/status.c
> +++ b/net/mac80211/status.c
> @@ -1019,6 +1019,85 @@ void ieee80211_tx_rate_update(struct ieee80211_hw *hw,
> }
> EXPORT_SYMBOL(ieee80211_tx_rate_update);
>
> +void ieee80211_tx_status_8023(struct ieee80211_hw *hw,
> + struct ieee80211_vif *vif,
> + struct sk_buff *skb)
Do we really need this, vs. just using ieee80211_tx_status_noskb()?
> --- a/net/mac80211/tx.c
> +++ b/net/mac80211/tx.c
> @@ -1253,7 +1253,8 @@ static struct txq_info *ieee80211_get_txq(struct ieee80211_local *local,
> (info->control.flags & IEEE80211_TX_CTRL_PS_RESPONSE))
> return NULL;
>
> - if (unlikely(!ieee80211_is_data_present(hdr->frame_control))) {
> + if (!info->control.hw_80211_encap &&
> + unlikely(!ieee80211_is_data_present(hdr->frame_control))) {
> if ((!ieee80211_is_mgmt(hdr->frame_control) ||
> ieee80211_is_bufferable_mmpdu(hdr->frame_control) ||
> vif->type == NL80211_IFTYPE_STATION) &&
That could use a comment, I had to think about it for a while ;-)
> @@ -1400,6 +1401,7 @@ static void ieee80211_txq_enqueue(struct ieee80211_local *local,
> struct fq *fq = &local->fq;
> struct fq_tin *tin = &txqi->tin;
>
> + struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
blank line should be after this, not before :)
> @@ -2357,9 +2359,9 @@ static inline bool ieee80211_is_tdls_setup(struct sk_buff *skb)
> skb->data[14] == WLAN_TDLS_SNAP_RFTYPE;
> }
>
> -static int ieee80211_lookup_ra_sta(struct ieee80211_sub_if_data *sdata,
> - struct sk_buff *skb,
> - struct sta_info **sta_out)
> +int ieee80211_lookup_ra_sta(struct ieee80211_sub_if_data *sdata,
> + struct sk_buff *skb,
> + struct sta_info **sta_out)
> {
> struct sta_info *sta;
>
> @@ -2855,7 +2857,9 @@ void ieee80211_check_fast_xmit(struct sta_info *sta)
> struct ieee80211_chanctx_conf *chanctx_conf;
> __le16 fc;
>
> - if (!ieee80211_hw_check(&local->hw, SUPPORT_FAST_XMIT))
> + /* check for driver support and ieee80211 encap offload */
> + if (!ieee80211_hw_check(&local->hw, SUPPORT_FAST_XMIT) ||
> + sdata->hw_80211_encap)
> return;
Maybe we should treat encap offload as a sort of hardware-accelerated
fast-xmit, and enable/disable based on whether we'd normally
enable/disable fast-xmit? That way we already have all the
infrastructure for keeping track of whether we can use it or not, etc.
> /* Locking here protects both the pointer itself, and against concurrent
> @@ -3554,6 +3558,9 @@ struct sk_buff *ieee80211_tx_dequeue(struct ieee80211_hw *hw,
> hdr = (struct ieee80211_hdr *)skb->data;
> info = IEEE80211_SKB_CB(skb);
>
> + if (info->control.hw_80211_encap)
> + goto out;
I'm not sure this is right, you would probably still at least do the
ieee80211_tx_h_select_key() call a bit later, and possibly the A-MPDU
stuff?
You also don't assign the right vif pointer, which will probably work in
some cases but quite likely not in general.
Again, maybe treating it like fast-xmit-offload would make sense here,
and we could do this in ieee80211_xmit_fast_finish().
But that might very well not be the right model since fast-xmit is per
station, though the conditions are mostly not depending on the station,
except for auth/assoc state - which has the same issues with encap
offload though! Like ... you probably need to send EAPOL frames through
a non-encap-offload path?
> + /* TODO: Handle frames requiring wifi tx status to be notified */
:)
Maybe that means you do need ieee80211_tx_status_8023().
> + memset(info, 0, sizeof(*info));
> +
> + if (unlikely(sdata->control_port_protocol == ehdr->h_proto)) {
> + if (sdata->control_port_no_encrypt)
> + info->flags |= IEEE80211_TX_INTFL_DONT_ENCRYPT;
> + info->control.flags |= IEEE80211_TX_CTRL_PORT_CTRL_PROTO;
> + }
> +
> + if (multicast)
> + info->flags |= IEEE80211_TX_CTL_NO_ACK;
> +
> + info->hw_queue = sdata->vif.hw_queue[skb_get_queue_mapping(skb)];
> +
> + ieee80211_tx_stats(dev, skb->len);
> +
> + if (sta) {
> + sta->tx_stats.bytes[skb_get_queue_mapping(skb)] += skb->len;
> + sta->tx_stats.packets[skb_get_queue_mapping(skb)]++;
> + }
> +
> + if (sdata->vif.type == NL80211_IFTYPE_AP_VLAN)
> + sdata = container_of(sdata->bss,
> + struct ieee80211_sub_if_data, u.ap);
> +
> + info->control.hw_80211_encap = true;
> + info->control.vif = &sdata->vif;
There's lots of duplicate code here, maybe split that out? Perhaps it
should be inlined, but then it can be an inline function to keep all the
stats etc.
> +netdev_tx_t ieee80211_subif_start_xmit_8023(struct sk_buff *skb,
> + struct net_device *dev)
> +{
> + struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
> + struct sta_info *sta;
> +
> + if (WARN_ON(unlikely(!sdata->hw_80211_encap))) {
WARN_ON() contains unlikely()
> + if (ieee80211_lookup_ra_sta(sdata, skb, &sta))
> + goto out_free;
> +
> + ieee80211_8023_xmit(sdata, dev, sta, skb);
Does that handle ERR_PTR() sta correctly?
> @@ -4081,6 +4249,16 @@ static bool ieee80211_tx_pending_skb(struct ieee80211_local *local,
> }
> info->band = chanctx_conf->def.chan->band;
> result = ieee80211_tx(sdata, NULL, skb, true, 0);
> + } else if (info->control.hw_80211_encap) {
> + if (ieee80211_lookup_ra_sta(sdata, skb, &sta)) {
> + dev_kfree_skb(skb);
> + return true;
> + }
> +
> + if (IS_ERR(sta) || (sta && !sta->uploaded))
> + sta = NULL;
> +
> + result = ieee80211_tx_8023(sdata, skb, skb->len, sta, true);
This doesn't seem to make sense, how can you TX to a non-STA with encap
offload, you don't even know where to send the frame then?
johannes
^ permalink raw reply
* Re: question: crda timeout in cfg80211
From: Johannes Berg @ 2019-04-08 19:06 UTC (permalink / raw)
To: Sergey Matyukevich, linux-wireless@vger.kernel.org; +Cc: Igor Mitsyanko
In-Reply-To: <20190326124209.j6tdk5cz47kc6mdj@bars>
On Tue, 2019-03-26 at 12:42 +0000, Sergey Matyukevich wrote:
> Hi all,
>
> Linux cfg80211 regulatory subsystem uses CRDA timeout to ensure completion
> of regulatory updates, performed by user-space software. See call_crda
> functon in net/wireless/reg.c:
>
> static int call_crda(const char *alpha2)
> {
> ...
>
> queue_delayed_work(system_power_efficient_wq,
> &crda_timeout, msecs_to_jiffies(3142));
> return 0;
> }
>
> So regulatory update/reset operations shall be completed in 3142 msec.
> And that includes processing of regulatory notifiers for all the
> wireless cards in the system.
>
> It is not quite clear where this specific timeout value came from.
> Original commit (a90c7a313a1c5b) doesn't go into details about it.
>
> Any ideas where it could come from ?
No particular reason. It's just ~pi seconds, and IIRC Luis thought that
was funny :-)
Are you seeing any issues with that?
johannes
^ permalink raw reply
* Re: [PATCH v6 12/20] kvm/vmx: Emulate MSR TEST_CTL
From: Sean Christopherson @ 2019-04-08 17:48 UTC (permalink / raw)
To: Xiaoyao Li
Cc: Thomas Gleixner, Fenghua Yu, Ingo Molnar, Borislav Petkov,
H Peter Anvin, Dave Hansen, Paolo Bonzini, Ashok Raj,
Peter Zijlstra, Kalle Valo, Michael Chan, Ravi V Shankar,
linux-kernel, x86, linux-wireless, netdev, kvm
In-Reply-To: <423f329a8e194d799c6c41470266a04a9f90bc84.camel@linux.intel.com>
On Mon, Apr 08, 2019 at 05:54:25PM +0800, Xiaoyao Li wrote:
> On Fri, 2019-04-05 at 14:30 +0200, Thomas Gleixner wrote:
> > On Wed, 3 Apr 2019, Fenghua Yu wrote:
> > > @@ -1663,6 +1663,12 @@ static int vmx_get_msr(struct kvm_vcpu *vcpu, struct
> > > msr_data *msr_info)
> > > u32 index;
> > >
> > > switch (msr_info->index) {
> > > + case MSR_TEST_CTL:
> > > + if (!msr_info->host_initiated &&
> > > + !(vcpu->arch.core_capability & CORE_CAP_SPLIT_LOCK_DETECT))
> >
> > Errm? If the MSR_TEST_CTL is exposed to the guest via CPUID then the
> > rdmsr() in the guest is not supposed to fail just because
> > CORE_CAP_SPLIT_LOCK_DETECT is not set.
>
> you're right.
>
> > vmx->msr_test_ctl holds the proper value, which is either 0 or
> > CORE_CAP_SPLIT_LOCK_DETECT until more bits are supported.
> >
> > So the chek needs to be guest_cpuid_has(X86_FEATURE_CORE_CAPABILITY).
>
> I don't think so. There is no dependecy between
> guest_cpuid_has(X86_FEATURE_CORE_CAPABILITY) and MSR_TEST_CTL.
> guest_cpuid_has(X86_FEATURE_CORE_CAPABILITY) only indicates that guest has MSR
> CORE_CAPABILITY.
>
> Due to the fact that MSR_TEST_CTL is emulated with vmx->msr_test_ctl. I think it
> 's ok to always let userspace or guest to read MSR_TEST_CTL, it just returns the
> emulated value. Like you said, " vmx->msr_test_ctl holds the proper value, which
> is either 0 or CORE_CAP_SPLIT_LOCK_DETECT until more bits are supported."
Assuming the next version implements "vmx->msr_test_ctl_mask", KVM should
inject #GP if the guest attempts RDMSR(MSR_TEST_CTL) and the mask is zero.
It stands to reason that a kernel can only reasonably assume the MSR exists
if the (virtual) CPU supports at least one feature enabled via MSR_TEST_CTL.
^ permalink raw reply
* Re: [PATCH] ath10k: Remove ATH10K_STATE_RESTARTED in simulate fw crash
From: Michał Kazior @ 2019-04-08 17:27 UTC (permalink / raw)
To: Wen Gong; +Cc: Wen Gong, linux-wireless, ath10k@lists.infradead.org
In-Reply-To: <54b4dab1191d41b8b329f3ceb7017626@aptaiexm02f.ap.qualcomm.com>
On Mon, 8 Apr 2019 at 12:20, Wen Gong <wgong@qti.qualcomm.com> wrote:
> > -----Original Message-----
> > From: Wen Gong
> > Sent: Monday, April 1, 2019 2:11 PM
> > To: 'Michał Kazior' <kazikcz@gmail.com>
> > Cc: Wen Gong <wgong@codeaurora.org>; linux-wireless <linux-
> > wireless@vger.kernel.org>; ath10k@lists.infradead.org
> > Subject: RE: [EXT] Re: [PATCH] ath10k: Remove ATH10K_STATE_RESTARTED in
> > simulate fw crash
> >
> > >
> > > If it's still bothering you then please consider a crash counter
> > > threshold so that, e.g. after 5 crash-while-restarting it's going to
> > > give up. However I doubt it's worth the effort. My experience tells me
> > > firmware crashes during recovery are rarely, if at all, transient.
> > >
> > > The simulated fw crash is not representative here. It's a mere tool to
> > > test driver code.
> > >
> > Hi Michal,
> > There have a stress test case for the simulate fw crash, it will simulate fw
> > crash
> > in a very short time for each test, this will trigger the stress test fail.
> > The simulate fw crash process should not be run parallel, after this patch, the
> > Stress test case will pass.
> > >
>
> Hi Michał,
> Do you have some new comments?
My original use case was to be able to exercise the driver's
robustness in handling nested fw crashes, IOW crash-within-a-crash.
Your test case, as far as I understand, intends to perform
consecutive, non-nested fw crash simulation stress test.
Both of these are mutually exclusive and your patch fixes your test
case at the expense of breaking my original case.
To satisfy both I would suggest you either expose ar->state via
debugfs and make your test procedure wait for that to get back into ON
state before simulating a crash again, or to extend the set of current
simulate_fw_crash commands (currently just: soft, hard, assert,
hw-restart) to something that allows expressing the intent whether
crash-in-crash prevention is intended (your case) or not (my original
case).
This could be for example something like this:
echo soft wait-ready > simulate_fw_crash
The "wait-ready" extra keyword would imply crash-in-crash prevention.
This would keep existing tools working (both behavior and syntax) and
would allow your test case to be implemented.
Michał
^ permalink raw reply
* Re: [PATCH 09/12] pinctrl: sunxi: Prepare for alternative bias voltage setting methods
From: Ondřej Jirman @ 2019-04-08 13:08 UTC (permalink / raw)
To: Linus Walleij
Cc: linux-sunxi, Maxime Ripard, Chen-Yu Tsai, Rob Herring,
David Airlie, Daniel Vetter, Mark Rutland, Giuseppe Cavallaro,
Alexandre Torgue, Jose Abreu, David S. Miller, Maxime Coquelin,
Arend van Spriel, Franky Lin, Hante Meuleman, Chi-Hsien Lin,
Wright Feng, Kalle Valo, Naveen Gupta,
open list:DRM PANEL DRIVERS,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux ARM, linux-kernel@vger.kernel.org, netdev, linux-stm32,
linux-wireless, brcm80211-dev-list.pdl, brcm80211-dev-list,
open list:GPIO SUBSYSTEM
In-Reply-To: <CACRpkdY=G0rpxVZL8TKJOQcf+Na8o5_FwG0EyBfb-ZCYXi=_oA@mail.gmail.com>
Hi Linus,
On Mon, Apr 08, 2019 at 02:53:58PM +0200, Linus Walleij wrote:
> On Sat, Apr 6, 2019 at 1:45 AM <megous@megous.com> wrote:
>
> > From: Ondrej Jirman <megous@megous.com>
> >
> > H6 has a different I/O voltage bias setting method than A80. Prepare
> > existing code for using alternative bias voltage setting methods.
> >
> > Signed-off-by: Ondrej Jirman <megous@megous.com>
> > ---
> > drivers/pinctrl/sunxi/pinctrl-sun9i-a80.c | 2 +-
> > drivers/pinctrl/sunxi/pinctrl-sunxi.c | 38 ++++++++++++-----------
> > drivers/pinctrl/sunxi/pinctrl-sunxi.h | 4 ++-
>
> I need Maxime's ACK on these patches to merge.
>
> Can the pinctrl parts be applied independently of the rest
> of the changes when Maxime is happy with the patches?
Yes, the two pinctrl patches are independent of the rest of the series.
thank you,
Ondrej
> Yours,
> Linus Walleij
^ permalink raw reply
* Re: [PATCH 09/12] pinctrl: sunxi: Prepare for alternative bias voltage setting methods
From: Linus Walleij @ 2019-04-08 12:53 UTC (permalink / raw)
To: megous
Cc: linux-sunxi, Maxime Ripard, Chen-Yu Tsai, Rob Herring,
David Airlie, Daniel Vetter, Mark Rutland, Giuseppe Cavallaro,
Alexandre Torgue, Jose Abreu, David S. Miller, Maxime Coquelin,
Arend van Spriel, Franky Lin, Hante Meuleman, Chi-Hsien Lin,
Wright Feng, Kalle Valo, Naveen Gupta,
open list:DRM PANEL DRIVERS,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux ARM, linux-kernel@vger.kernel.org, netdev, linux-stm32,
linux-wireless, brcm80211-dev-list.pdl, brcm80211-dev-list,
open list:GPIO SUBSYSTEM
In-Reply-To: <20190405234514.6183-10-megous@megous.com>
On Sat, Apr 6, 2019 at 1:45 AM <megous@megous.com> wrote:
> From: Ondrej Jirman <megous@megous.com>
>
> H6 has a different I/O voltage bias setting method than A80. Prepare
> existing code for using alternative bias voltage setting methods.
>
> Signed-off-by: Ondrej Jirman <megous@megous.com>
> ---
> drivers/pinctrl/sunxi/pinctrl-sun9i-a80.c | 2 +-
> drivers/pinctrl/sunxi/pinctrl-sunxi.c | 38 ++++++++++++-----------
> drivers/pinctrl/sunxi/pinctrl-sunxi.h | 4 ++-
I need Maxime's ACK on these patches to merge.
Can the pinctrl parts be applied independently of the rest
of the changes when Maxime is happy with the patches?
Yours,
Linus Walleij
^ permalink raw reply
* Re: wireless-regdb: Please update regulatory rules for Japan (JP) on 60GHz
From: seth.forshee @ 2019-04-08 12:48 UTC (permalink / raw)
To: junping.xu@jaguarwave.com; +Cc: wireless-regdb, linux-wireless
In-Reply-To: <2019040118074276659215@jaguarwave.com>
On Mon, Apr 01, 2019 at 06:07:44PM +0800, junping.xu@jaguarwave.com wrote:
> Hi Forsheee,
> The 60GHz band defined for Japan was between 59-66GHz, but is has been Updated to support 57-66Ghz about 2 years ago。
> However, the latest wireless-regdb still containning the original regulatory information as below. Please help to check the infomation
> and update the regulatory.bin.
>
> db.txt:
> ......
> country JP: DFS-JP
> (2402 - 2482 @ 40), (20)
> (2474 - 2494 @ 20), (20), NO-OFDM
> (4910 - 4990 @ 40), (23)
> (5030 - 5090 @ 40), (23)
> (5170 - 5250 @ 80), (20), AUTO-BW
> (5250 - 5330 @ 80), (20), DFS, AUTO-BW
> (5490 - 5710 @ 160), (23), DFS
> # 60 GHz band channels 2-4 at 10mW,
> # ref: http://www.arib.or.jp/english/html/overview/doc/1-STD-T74v1_1.pdf
The only change which is required to change:
> (59000 - 66000 @ 2160), (10 mW)
to:
(57000 - 66000 @ 2160), (10 mW)
Is that right? Can you provide a link to the documentation which
supports this change? The current link above this rule appears to be
broken.
Thanks,
Seth
^ permalink raw reply
* Re: [linux-sunxi] [PATCH 00/12] Add support for Orange Pi 3
From: Ondřej Jirman @ 2019-04-08 12:46 UTC (permalink / raw)
To: Jagan Teki
Cc: Clément Péron, linux-sunxi, Maxime Ripard, Chen-Yu Tsai,
Rob Herring, Linus Walleij, David Airlie, Daniel Vetter,
Mark Rutland, Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
David S. Miller, Maxime Coquelin, Arend van Spriel, Franky Lin,
Hante Meuleman, Chi-Hsien Lin, Wright Feng, Kalle Valo,
Naveen Gupta, dri-devel, devicetree, linux-arm-kernel,
linux-kernel, netdev, linux-stm32, linux-wireless,
brcm80211-dev-list.pdl, brcm80211-dev-list, linux-gpio
In-Reply-To: <CAMty3ZDpppM5zBvxCBx3NjdVsRm4S93yz5kqi5Z5a0rhC-+zPg@mail.gmail.com>
Hi Jagan,
On Mon, Apr 08, 2019 at 11:31:22AM +0530, Jagan Teki wrote:
> On Sun, Apr 7, 2019 at 8:02 PM 'Ondřej Jirman' via linux-sunxi
> <linux-sunxi@googlegroups.com> wrote:
> >
> > On Sun, Apr 07, 2019 at 03:36:21PM +0200, Clément Péron wrote:
> > > Hi,
> > >
> > > On Sat, 6 Apr 2019 at 01:45, megous via linux-sunxi
> > > <linux-sunxi@googlegroups.com> wrote:
> > > >
> > > > From: Ondrej Jirman <megous@megous.com>
> > > >
> > > > This series implements support for Xunlong Orange Pi 3 board.
> > >
> > > OrangePi 3 Lite2 and One Plus boards support has already been merged.
> > > The support is not complete but you should rebase your patches on top
> > > of sunxi/for-next
> >
> > Hi,
> >
> > OrangePi 3 is somewhat different from these two boards (mostly it has a differnt
> > power tree). It doesn't use the AXP regulators that are defined in the
> > sun50i-h6-orangepi.dtsi in the same way.
> >
> > For example:
> >
> > - bldo3 (is turned always on in sun50i-h6-orangepi.dtsi but unused for opi3)
> > - cldo2 and cldo3 are unused on opi3 and have nothing to do with WiFi
> > - aldo3 is not for dram
> > - bldo1 on the other hand is for dram on opi3
> > - some other regulators are used for different/more functions and thus
> > named differntly
> > - USB id-det pin is differnt
> > - ...
>
> Based on my communication with OrangePI, OPI-3 has PCIE, 4 USB-3.0
> ports and AV are the key differences and rest seems to be similar. but
> if we have a diff or unused regulators may be we can't enable them in
> dtsi (I never looked that close as of now)
These regulators are not just disabled on Opi 3, they have different/no meaning.
Schematics allow for high amount of variability in the power tree (see all the
NC (not connected) / 0R resistors) in the schematic around AXP805. Every board
based on this Xunlong design can be subtly different.
I already suggested a maintainable solution, below. Where base dtsi has empty
config for regulators and every board based on that just defines it completely
for itself.
A few regulators (for CPU/GPU) will most probably have the same meaning on every
derived board, so these can probably be kept in dtsi without causing too much
annoyance.
It's unpleasant to have wrong regulator setup defined in an underlying dtsi, and
then trying to override it by removing/adding random properties in the board dts
for the new boards based on that, so that it fits.
The rest of the current HW descriptions in the sun50i-h6-orangepi.dtsi can be
shared (as of now).
thank you and regards,
o.
^ permalink raw reply
* Re: [wireless-regdb] [PATCH] wireless-regdb: Update regulatory rules for South Korea
From: Seth Forshee @ 2019-04-08 12:41 UTC (permalink / raw)
To: Peter Oh; +Cc: wireless-regdb@lists.infradead.org,
linux-wireless@vger.kernel.org
In-Reply-To: <1553711659-27600-1-git-send-email-peter.oh@bowerswilkins.com>
On Wed, Mar 27, 2019 at 06:34:29PM +0000, Peter Oh wrote:
> From: Peter Oh <peter.oh@bowerswilkins.com>
>
> Update power limit as documented in:
> https://www.rra.go.kr/FileDownSvl?file_type=LAWKR&file_parentseq=54&file_seq=27
> which revised on November 30, 2016.
>
> Signed-off-by: Peter Oh <peter.oh@bowerswilkins.com>
I usually try to check the changes against the documents, but in this
case I haven't been able to get any English tranlation for the document
(or even view the contents it in any meaningful way). I don't suppose
you know where to find an English version of the document?
Thanks,
Seth
> ---
> db.txt | 11 ++++++-----
> 1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/db.txt b/db.txt
> index 4fb1948..52c17e9 100644
> --- a/db.txt
> +++ b/db.txt
> @@ -695,11 +695,12 @@ country KP: DFS-JP
> (5735 - 5815 @ 20), (30)
>
> country KR: DFS-JP
> - (2402 - 2482 @ 40), (13)
> - (5170 - 5250 @ 80), (20), AUTO-BW
> - (5250 - 5330 @ 80), (20), DFS, AUTO-BW
> - (5490 - 5710 @ 160), (30), DFS
> - (5735 - 5835 @ 80), (30)
> + # ref: https://www.rra.go.kr/ko/reference/lawList_view.do?lw_seq=54&lw_type=3&searchCon=&lw_select=ALL&lw_model=&searchTxt=
> + (2402 - 2482 @ 40), (23)
> + (5170 - 5250 @ 80), (17), AUTO-BW
> + (5250 - 5330 @ 80), (23), DFS, AUTO-BW
> + (5490 - 5710 @ 160), (23), DFS
> + (5735 - 5835 @ 80), (23)
> # 60 GHz band channels 1-4,
> # ref: http://www.law.go.kr/%ED%96%89%EC%A0%95%EA%B7%9C%EC%B9%99/%EB%AC%B4%EC%84%A0%EC%84%A4%EB%B9%84%EA%B7%9C%EC%B9%99
> (57000 - 66000 @ 2160), (43)
> --
> 2.7.4
>
>
> _______________________________________________
> wireless-regdb mailing list
> wireless-regdb@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/wireless-regdb
^ permalink raw reply
* [PATCH] mac80211: only allocate one queue when using iTXQs
From: Johannes Berg @ 2019-04-08 12:39 UTC (permalink / raw)
To: linux-wireless; +Cc: Felix Fietkau, Johannes Berg
From: Johannes Berg <johannes.berg@intel.com>
There's no need to allocate than one queue in the iTXQs case
now that we no longer use ndo_select_queue to assign the AC.
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
net/mac80211/iface.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
index 6e1b031535d5..94459b2b3d2a 100644
--- a/net/mac80211/iface.c
+++ b/net/mac80211/iface.c
@@ -1763,13 +1763,13 @@ int ieee80211_if_add(struct ieee80211_local *local, const char *name,
txq_size += sizeof(struct txq_info) +
local->hw.txq_data_size;
- if (local->ops->wake_tx_queue)
+ if (local->ops->wake_tx_queue) {
if_setup = ieee80211_if_setup_no_queue;
- else
+ } else {
if_setup = ieee80211_if_setup;
-
- if (local->hw.queues >= IEEE80211_NUM_ACS)
- txqs = IEEE80211_NUM_ACS;
+ if (local->hw.queues >= IEEE80211_NUM_ACS)
+ txqs = IEEE80211_NUM_ACS;
+ }
ndev = alloc_netdev_mqs(size + txq_size,
name, name_assign_type,
--
2.17.2
^ permalink raw reply related
* Re: [PATCH] mac80211: optimize skb resizing
From: Johannes Berg @ 2019-04-08 12:26 UTC (permalink / raw)
To: Felix Fietkau, linux-wireless
In-Reply-To: <20190317181859.71841-1-nbd@nbd.name>
On Sun, 2019-03-17 at 19:18 +0100, Felix Fietkau wrote:
> When forwarding unicast packets from ethernet to batman-adv over 802.11s
> (with forwarding disabled), the typical required headroom to transmit
> encrypted packets on mt76 is 32 (802.11) + 6 (802.11s) + 8 (CCMP) +
> 2 (padding) + 6 (LLC) + 18 (batman-adv) - 14 (old ethernet header) = 58 bytes.
>
> On systems where NET_SKB_PAD is 64 this leads to a call to pskb_expand_head
> for every packet, since mac80211 also tries to allocate 16 bytes status
> headroom for radiotap headers.
>
> This patch fixes these unnecessary reallocations by only requiring the extra
> status headroom in ieee80211_tx_monitor()
> If however a reallocation happens before that call, the status headroom gets
> added there as well, in order to avoid double reallocation.
>
> The patch also cleans up the code by moving the headroom calculation to
> ieee80211_skb_resize.
This doesn't apply, not really sure why.
johannes
^ permalink raw reply
* Re: [PATCH 06/16] mac80211_hwsim: make copying of ciphers safer by checking the length
From: Johannes Berg @ 2019-04-08 12:22 UTC (permalink / raw)
To: Luca Coelho, kvalo; +Cc: linux-wireless, Luca Coelho
In-Reply-To: <20190315163634.17315-7-luca@coelho.fi>
On Fri, 2019-03-15 at 18:36 +0200, Luca Coelho wrote:
> From: Luca Coelho <luciano.coelho@intel.com>
>
> Make sure the length of the ciphers we are copying never exceeds the
> space we have for storing them. There is no risk of overcopying at
> the moment, because we check n_params before, but this makes this
> function safer in case someone changes something in the future.
I don't think this makes sense.
If anything, we should pass extack to mac80211_hwsim_new_radio() and do
the entire validation there, but doing the same thing twice in two
places just because of static checkers is useless.
johannes
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH 02/12] drm: sun4i: Add support for enabling DDC I2C bus to dw_hdmi glue
From: Ondřej Jirman @ 2019-04-08 12:17 UTC (permalink / raw)
To: Maxime Ripard
Cc: Chen-Yu Tsai, linux-sunxi, Rob Herring, Linus Walleij,
David Airlie, Daniel Vetter, Mark Rutland, Giuseppe Cavallaro,
Alexandre Torgue, Jose Abreu, David S. Miller, Maxime Coquelin,
Arend van Spriel, Franky Lin, Hante Meuleman, Chi-Hsien Lin,
Wright Feng, Kalle Valo, Naveen Gupta, dri-devel, devicetree,
linux-arm-kernel, linux-kernel, netdev, linux-stm32,
linux-wireless, brcm80211-dev-list.pdl, brcm80211-dev-list,
open list:GPIO SUBSYSTEM
In-Reply-To: <20190408084714.b6xdi2pxbjvzwsnu@flea>
On Mon, Apr 08, 2019 at 10:47:14AM +0200, Maxime Ripard wrote:
> On Mon, Apr 08, 2019 at 03:28:24PM +0800, Chen-Yu Tsai wrote:
> > On Mon, Apr 8, 2019 at 3:23 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > >
> > > On Sat, Apr 06, 2019 at 01:45:04AM +0200, megous@megous.com wrote:
> > > > From: Ondrej Jirman <megous@megous.com>
> > > >
> > > > Orange Pi 3 board requires enabling DDC I2C bus via some GPIO connected
> > > > transistors, before it can be used. Model this as a power supply for DDC
> > > > (via regulator framework).
> > > >
> > > > Signed-off-by: Ondrej Jirman <megous@megous.com>
> > >
> > > The DDC bus itself is usually attached to the HDMI connector, so it
> > > would make sense to make the supply also a property of the connector.
> >
> > I believe these are separate things. What this patch covers is power for
> > a voltage shifter between the SoC and HDMI DDC pins. The HDMI connector's
> > 5V supply to power the remote DDC chip is something else. And on the
> > Orange Pi 3 they are indeed separate supplies.
>
> Then maybe the endpoint link between the two would be the best place
> to put this?
Interestingly &hdmi node configures the DDC bus pins via pinctrl on the SoC
side, so I put this there too, because it's related to those pins. I'm not sure
if that changes anything in the discussion.
regards,
o.
> Maxime
>
> --
> Maxime Ripard, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
^ permalink raw reply
* Re: [PATCH 1/2] nl80211: Allow change CW to Ad-Hock network
From: Johannes Berg @ 2019-04-08 12:12 UTC (permalink / raw)
To: Andrea Greco; +Cc: David S. Miller, linux-wireless, netdev, linux-kernel
In-Reply-To: <20190221231147.6141-1-andrea.greco.gapmilano@gmail.com>
On Fri, 2019-02-22 at 00:11 +0100, Andrea Greco wrote:
> From: Andrea Greco <a.greco@4sigma.it>
>
> Add net-link support for change CW in Ad-Hock network
This doesn't really make sense, the ibss code overrides it again once it
actually re-joins etc.
johannes
^ permalink raw reply
* Re: [PATCH] mac80211: Honor SW_CRYPTO_CONTROL in AP VLAN mode
From: Johannes Berg @ 2019-04-08 12:09 UTC (permalink / raw)
To: Alexander Wetzel; +Cc: linux-wireless, mpubbise
In-Reply-To: <20190209140138.16692-1-alexander@wetzel-home.de>
On Sat, 2019-02-09 at 15:01 +0100, Alexander Wetzel wrote:
> Restore @SW_CRYPTO_CONTROL when interface is in AP Vlan mode and don't
> override driver decision for unicast keys.
>
> Fixes commit db3bdcb9c3ff ("mac80211: allow AP_VLAN operation on crypto
> controlled devices"), which should only have allow SW crypto fallback for
> group keys.
This confuses me.
The driver doesn't really know about AP_VLAN, so the original commit
intentionally did this, I think?
There was some kind of other fix related to this though?
johannes
^ permalink raw reply
* Re: [PATCH 2/2] mac8011: Add support for NL80211_STA_INFO_RX_MCAST_BCAST
From: Johannes Berg @ 2019-04-08 12:02 UTC (permalink / raw)
To: Narayanraddi Masti; +Cc: linux-wireless
In-Reply-To: <1549570566-28658-2-git-send-email-team.nmasti@gmail.com>
On Thu, 2019-02-07 at 12:16 -0800, Narayanraddi Masti wrote:
> Add support to collect per STA mcast/bcast packet information attribute
> NL80211_STA_INFO_RX_MCAST_BCAST.
This, honestly, I'm not a fan of.
We've added far too many statistics already, and we keep adding more.
This, in particular, also adds a bunch of 64-bit counters (is that
really needed?) which is relatively expensive on 32-bit platforms, and
it didn't even update the fast-RX path properly.
I really think we need to stick some eBPF hooks somewhere late enough to
get valid data, but early enough to get all the data, and then you can
keep all your favourite statistics there.
johannes
^ permalink raw reply
* Re: [PATCH 1/2] mac80211: Add support for NL80211_STA_INFO_AIRTIME_LINK_METRIC
From: Johannes Berg @ 2019-04-08 11:59 UTC (permalink / raw)
To: Narayanraddi Masti; +Cc: linux-wireless
In-Reply-To: <1549570566-28658-1-git-send-email-team.nmasti@gmail.com>
On Thu, 2019-02-07 at 12:16 -0800, Narayanraddi Masti wrote:
> Add support for mesh airtime link metric attribute
> NL80211_STA_INFO_AIRTIME_LINK_METRIC.
This looks good, applied.
johannes
^ permalink raw reply
* Re: [PATCHv2 2/2] mac80211: Implement API to configure station specific rssi threshold
From: Johannes Berg @ 2019-04-08 11:56 UTC (permalink / raw)
To: Tamizh chelvam; +Cc: linux-wireless
In-Reply-To: <1549361181-6237-3-git-send-email-tamizhr@codeaurora.org>
> +++ b/net/mac80211/cfg.c
> @@ -3889,6 +3889,110 @@ static int ieee80211_get_txq_stats(struct wiphy *wiphy,
> return drv_abort_pmsr(local, sdata, request);
> }
>
> +void sta_mon_rssi_config_free(struct sta_info *sta)
> +{
> + if (sta->rssi_config) {
> + kfree_rcu(sta->rssi_config, rcu_head);
> + sta->rssi_config = NULL;
> + }
> +}
This looks wrong, you should use RCU_INIT_POINTER()?
And you should do it the other way around I think? I.e.
old = sta->rssi_config;
RCU_INIT_POINTER(sta->rssi_config, NULL);
kfree_rcu(old, rcu_head);
> /*
> * The bandwidth threshold below which the per-station CoDel parameters will be
> * scaled to be more lenient (to prevent starvation of slow stations). This
> @@ -623,6 +650,7 @@ struct sta_info {
>
> struct cfg80211_chan_def tdls_chandef;
>
> + struct sta_mon_rssi_config *rssi_config;
Please add documentation (as built bot pointed out shortly after patch
posting!)
johannes
^ permalink raw reply
* Re: [PATCHv2 1/2] cfg80211: Add support to configure station specific RSSI threshold for AP mode
From: Johannes Berg @ 2019-04-08 11:54 UTC (permalink / raw)
To: Tamizh chelvam; +Cc: linux-wireless
In-Reply-To: <1549361181-6237-2-git-send-email-tamizhr@codeaurora.org>
On Tue, 2019-02-05 at 15:36 +0530, Tamizh chelvam wrote:
>
> + [NL80211_ATTR_CQM] = {
> + .type = NLA_NESTED,
> + .validation_data = nl80211_attr_cqm_policy,
> + },
This is wrong, you need to use NLA_POLICY_NESTED().
> + [NL80211_ATTR_STA_MON_FIXED_THOLD] = { .type = NLA_U8 },
Please use NLA_POLICY_RANGE(NLA_U8, 0, 1).
Also, I'm not sure I really understand the documentation for this value.
> /* Check all values negative and sorted */
> - for (i = 0; i < n_thresholds; i++) {
> + for (i = 0; i < n_tholds; i++) {
Why rename the perfectly fine variable to some abbreviation?
johannes
^ permalink raw reply
* Re: [PATCH v8 00/14] rtw88: mac80211 driver for Realtek 802.11ac wireless network chips
From: Kalle Valo @ 2019-04-08 11:33 UTC (permalink / raw)
To: Tony Chuang
Cc: Brian Norris, Johannes Berg, linux-wireless, Greg Kroah-Hartman,
Pkshih, Andy Huang, Larry Finger, Stanislaw Gruszka
In-Reply-To: <F7CD281DE3E379468C6D07993EA72F84D17D50A4@RTITMBSVM04.realtek.com.tw>
Tony Chuang <yhchuang@realtek.com> writes:
> Hi Kalle
>
>> From: Stanislaw Gruszka [mailto:sgruszka@redhat.com]
>> Sent: Wednesday, March 27, 2019 6:26 PM
>> To: Tony Chuang
>> Cc: Brian Norris; Kalle Valo; Johannes Berg; linux-wireless; Greg Kroah-Hartman;
>> Pkshih; Andy Huang; Larry Finger
>> Subject: Re: [PATCH v8 00/14] rtw88: mac80211 driver for Realtek 802.11ac
>> wireless network chips
>>
>
> It's just a gentle reply to ask if rtw88 is getting merged.
> Are you still reviewing or is there anything I can help.
I just have been busy with other stuff, sorry. I'll try find time for
this soon.
--
Kalle Valo
^ permalink raw reply
* RE: [PATCH] ath10k: Remove ATH10K_STATE_RESTARTED in simulate fw crash
From: Wen Gong @ 2019-04-08 10:19 UTC (permalink / raw)
To: Michał Kazior; +Cc: Wen Gong, linux-wireless, ath10k@lists.infradead.org
In-Reply-To: <02904ba8682441e1a89668c1345cbec9@aptaiexm02f.ap.qualcomm.com>
> -----Original Message-----
> From: Wen Gong
> Sent: Monday, April 1, 2019 2:11 PM
> To: 'Michał Kazior' <kazikcz@gmail.com>
> Cc: Wen Gong <wgong@codeaurora.org>; linux-wireless <linux-
> wireless@vger.kernel.org>; ath10k@lists.infradead.org
> Subject: RE: [EXT] Re: [PATCH] ath10k: Remove ATH10K_STATE_RESTARTED in
> simulate fw crash
>
> >
> > If it's still bothering you then please consider a crash counter
> > threshold so that, e.g. after 5 crash-while-restarting it's going to
> > give up. However I doubt it's worth the effort. My experience tells me
> > firmware crashes during recovery are rarely, if at all, transient.
> >
> > The simulated fw crash is not representative here. It's a mere tool to
> > test driver code.
> >
> Hi Michal,
> There have a stress test case for the simulate fw crash, it will simulate fw
> crash
> in a very short time for each test, this will trigger the stress test fail.
> The simulate fw crash process should not be run parallel, after this patch, the
> Stress test case will pass.
> >
Hi Michał,
Do you have some new comments?
> > Michał
^ permalink raw reply
* Re: [PATCH v6 12/20] kvm/vmx: Emulate MSR TEST_CTL
From: Xiaoyao Li @ 2019-04-08 9:54 UTC (permalink / raw)
To: Thomas Gleixner, Fenghua Yu
Cc: Ingo Molnar, Borislav Petkov, H Peter Anvin, Dave Hansen,
Paolo Bonzini, Ashok Raj, Peter Zijlstra, Kalle Valo,
Michael Chan, Ravi V Shankar, linux-kernel, x86, linux-wireless,
netdev, kvm
In-Reply-To: <alpine.DEB.2.21.1904051400230.1802@nanos.tec.linutronix.de>
Hi, Thomas,
On Fri, 2019-04-05 at 14:30 +0200, Thomas Gleixner wrote:
> On Wed, 3 Apr 2019, Fenghua Yu wrote:
> > @@ -1663,6 +1663,12 @@ static int vmx_get_msr(struct kvm_vcpu *vcpu, struct
> > msr_data *msr_info)
> > u32 index;
> >
> > switch (msr_info->index) {
> > + case MSR_TEST_CTL:
> > + if (!msr_info->host_initiated &&
> > + !(vcpu->arch.core_capability & CORE_CAP_SPLIT_LOCK_DETECT))
>
> Errm? If the MSR_TEST_CTL is exposed to the guest via CPUID then the
> rdmsr() in the guest is not supposed to fail just because
> CORE_CAP_SPLIT_LOCK_DETECT is not set.
you're right.
> vmx->msr_test_ctl holds the proper value, which is either 0 or
> CORE_CAP_SPLIT_LOCK_DETECT until more bits are supported.
>
> So the chek needs to be guest_cpuid_has(X86_FEATURE_CORE_CAPABILITY).
I don't think so. There is no dependecy between
guest_cpuid_has(X86_FEATURE_CORE_CAPABILITY) and MSR_TEST_CTL.
guest_cpuid_has(X86_FEATURE_CORE_CAPABILITY) only indicates that guest has MSR
CORE_CAPABILITY.
Due to the fact that MSR_TEST_CTL is emulated with vmx->msr_test_ctl. I think it
's ok to always let userspace or guest to read MSR_TEST_CTL, it just returns the
emulated value. Like you said, " vmx->msr_test_ctl holds the proper value, which
is either 0 or CORE_CAP_SPLIT_LOCK_DETECT until more bits are supported."
> > + return 1;
> > + msr_info->data = vmx->msr_test_ctl;
> > + break;
> > #ifdef CONFIG_X86_64
> > case MSR_FS_BASE:
> > msr_info->data = vmcs_readl(GUEST_FS_BASE);
> > @@ -1797,6 +1803,14 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, struct
> > msr_data *msr_info)
> > u32 index;
> >
> > switch (msr_index) {
> > + case MSR_TEST_CTL:
> > + if (!(vcpu->arch.core_capability & CORE_CAP_SPLIT_LOCK_DETECT))
> > + return 1;
>
> Again, this wants to be a check for the availability of the MSR in the
> guest cpuid and not to the CORE_CAP_SPLIT_LOCK_DETECT magic bit.
>
> > +
> > + if (data & ~TEST_CTL_ENABLE_SPLIT_LOCK_DETECT)
> > + return 1;
>
> and this one wants to be:
>
> if (data & vmx->msr_test_ctl_mask)
>
> so additional bits can be enabled later on at exactly one point in the
> code, i.e. during vm init.
Again, in wrmsr handler, since MSR_TEST_CTL is emulated, it can be considered
that guest always has this MSR. It just needs to return 1 when reserved bits are
set.
Using vmx->msr_test_ctl_mask is a good idea. I will use it in next version.
> > + vmx->msr_test_ctl = data;
> > + break;
> >
> > +static void atomic_switch_msr_test_ctl(struct vcpu_vmx *vmx)
> > +{
> > + u64 host_msr_test_ctl;
> > +
> > + /* if TEST_CTL MSR doesn't exist on the hardware, do nothing */
> > + if (rdmsrl_safe(MSR_TEST_CTL, &host_msr_test_ctl))
> > + return;
>
> Yikes. So on hosts which do not have MSR_TEST_CTL this takes a fault on
> every VM enter. The host knows whether it has this MSR or not.
>
> Even if it exists there is no point to do the rdmsr on every vmenter. The
> value should be cached per cpu. It only changes when:
>
> 1) #AC triggers in kernel space
>
> 2) The sysfs knob is flipped
>
> #1 It happens either _BEFORE_ or _AFTER_ atomic_switch_msr_test_ctl(). In
> both cases the cached value is correct in atomic_switch_msr_test_ctl().
>
> If it happens _AFTER_ atomic_switch_msr_test_ctl() then the VMEXIT will
> restore the enabled bit, but there is nothing you can do about that.
>
> #2 CANNOT happen in the context of vcpu_run().
>
> So this wants to be:
>
> host_msr_test_ctl = this_cpu_read(msr_test_ctl_cache);
You're right and thanks for your advice.
I will take it in next version.
> > + if (host_msr_test_ctl == vmx->msr_test_ctl)
> > + clear_atomic_switch_msr(vmx, MSR_TEST_CTL);
> > + else
> > + add_atomic_switch_msr(vmx, MSR_TEST_CTL, vmx->msr_test_ctl,
> > + host_msr_test_ctl, false);
> > +}
> > +
> > u64 kvm_get_core_capability(void)
> > {
> > - return 0;
> > + u64 data;
> > +
> > + rdmsrl_safe(MSR_IA32_CORE_CAPABILITY, &data);
>
> if (!boot_cpu_has(X86_FEATURE_CORE_CAPABILITY))
> return 0;
>
> > + /* mask non-virtualizable functions */
> > + data &= CORE_CAP_SPLIT_LOCK_DETECT;
>
> Why?
>
> > + /*
> > + * There will be a list of FMS values that have split lock detection
> > + * but lack the CORE CAPABILITY MSR. In this case, set
> > + * CORE_CAP_SPLIT_LOCK_DETECT since we emulate MSR CORE_CAPABILITY.
>
> That's completely wrong. X86_FEATURE_SPLIT_LOCK_DETECT is set when the
> capability is enumerated in CPUID or it's set via the FMS quirk.
>
> So:
> data = 0;
> > + if (boot_cpu_has(X86_FEATURE_SPLIT_LOCK_DETECT))
> > + data |= CORE_CAP_SPLIT_LOCK_DETECT;
> > +
> > + return data;
>
> is absolutely sufficient.
Right, X86_FEATURE_SPLIT_LOCK_DETECT is set in 2 situation:
#1 the capability is enumerated by CORE_CAP_SPLIT_LOCK_DETECT (bit 5 of
MSR_IA32_CORE_CAPABILITY)
#2 via FMS list, in which those processors have split lock detection
feature but don't have MSR_IA32_CORE_CAPABILITY.
There two pathes work well for host, accurately for real FMS.
However, when it comes to virtualization, we can emulate a different FMS of
guest from host.
Considering the following case:
The host cpu is ICELAKE_MOBILE, which doesn't have X86_FEATURE_CORE_CAPABILITY
but has X86_FEATURE_SPLIT_LOCK_DETECT.
If we run a guest with cpu model, Skylake. It will hidden the feature
X86_FEATURE_SPLIT_LOCK_DETECT from guest, since guest's MSR_IA32_CORE_CAPABILITY
reports zero, and FMS of guest doesn't match the list.
In this way, it failed to expose the X86_FEATURE_SPLIT_LOCK_DETECT to guest.
Since MSR_IA32_CORE_CAPBILITY is emulated in software for guest. We can enforce
using #1 to report X86_FEATURE_SPLIT_LOCK_DETECT of guest, thus guest can get
rid of the limitation of the FMS list.
> Thanks,
>
> tglx
^ permalink raw reply
* Re: [PATCH 0/6] stricter netlink validation
From: Johannes Berg @ 2019-04-08 9:01 UTC (permalink / raw)
To: Leon Romanovsky; +Cc: linux-wireless, netdev, Pablo Neira Ayuso, David Ahern
In-Reply-To: <20190408090010.GG3201@mtr-leonro.mtl.com>
> This series crashes on mlx4 devices with the following kernel panic.
Yeah, I know. Like I said elsewhere on the thread, I accidentally sent
out the wrong branch (not realizing I had made two). :-(
This should work:
https://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211-next.git/log/?h=netlink-validation
johannes
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox