| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Mon, 3 Jul 2023 22:53:26 -0500
From: Mario Limonciello <mario.limonciello@....com>
To: "Quan, Evan" <Evan.Quan@....com>, "rafael@...nel.org"
<rafael@...nel.org>, "lenb@...nel.org" <lenb@...nel.org>,
"Deucher, Alexander" <Alexander.Deucher@....com>,
"Koenig, Christian" <Christian.Koenig@....com>,
"Pan, Xinhui" <Xinhui.Pan@....com>, "airlied@...il.com" <airlied@...il.com>,
"daniel@...ll.ch" <daniel@...ll.ch>,
"johannes@...solutions.net" <johannes@...solutions.net>,
"davem@...emloft.net" <davem@...emloft.net>,
"edumazet@...gle.com" <edumazet@...gle.com>,
"kuba@...nel.org" <kuba@...nel.org>, "pabeni@...hat.com"
<pabeni@...hat.com>, "mdaenzer@...hat.com" <mdaenzer@...hat.com>,
"maarten.lankhorst@...ux.intel.com" <maarten.lankhorst@...ux.intel.com>,
"tzimmermann@...e.de" <tzimmermann@...e.de>,
"hdegoede@...hat.com" <hdegoede@...hat.com>,
"jingyuwang_vip@....com" <jingyuwang_vip@....com>,
"Lazar, Lijo" <Lijo.Lazar@....com>,
"jim.cromie@...il.com" <jim.cromie@...il.com>,
"bellosilicio@...il.com" <bellosilicio@...il.com>,
"andrealmeid@...lia.com" <andrealmeid@...lia.com>,
"trix@...hat.com" <trix@...hat.com>, "jsg@....id.au" <jsg@....id.au>,
"arnd@...db.de" <arnd@...db.de>
Cc: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-acpi@...r.kernel.org" <linux-acpi@...r.kernel.org>,
"amd-gfx@...ts.freedesktop.org" <amd-gfx@...ts.freedesktop.org>,
"dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
"linux-wireless@...r.kernel.org" <linux-wireless@...r.kernel.org>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [PATCH V5 1/9] drivers core: Add support for Wifi band RF
mitigations
On 7/3/23 22:40, Quan, Evan wrote:
> [AMD Official Use Only - General]
>
>> -----Original Message-----
>> From: Limonciello, Mario <Mario.Limonciello@....com>
>> Sent: Saturday, July 1, 2023 12:41 AM
>> To: Quan, Evan <Evan.Quan@....com>; rafael@...nel.org; lenb@...nel.org;
>> Deucher, Alexander <Alexander.Deucher@....com>; Koenig, Christian
>> <Christian.Koenig@....com>; Pan, Xinhui <Xinhui.Pan@....com>;
>> airlied@...il.com; daniel@...ll.ch; johannes@...solutions.net;
>> davem@...emloft.net; edumazet@...gle.com; kuba@...nel.org;
>> pabeni@...hat.com; mdaenzer@...hat.com;
>> maarten.lankhorst@...ux.intel.com; tzimmermann@...e.de;
>> hdegoede@...hat.com; jingyuwang_vip@....com; Lazar, Lijo
>> <Lijo.Lazar@....com>; jim.cromie@...il.com; bellosilicio@...il.com;
>> andrealmeid@...lia.com; trix@...hat.com; jsg@....id.au; arnd@...db.de
>> Cc: linux-kernel@...r.kernel.org; linux-acpi@...r.kernel.org; amd-
>> gfx@...ts.freedesktop.org; dri-devel@...ts.freedesktop.org; linux-
>> wireless@...r.kernel.org; netdev@...r.kernel.org
>> Subject: Re: [PATCH V5 1/9] drivers core: Add support for Wifi band RF
>> mitigations
>>
>> On 6/30/2023 05:32, Evan Quan wrote:
>>> Due to electrical and mechanical constraints in certain platform
>>> designs there may be likely interference of relatively high-powered
>>> harmonics of the (G-)DDR memory clocks with local radio module
>>> frequency bands used by Wifi 6/6e/7.
>>>
>>> To mitigate this, AMD has introduced a mechanism that devices can use
>>> to notify active use of particular frequencies so that other devices
>>> can make relative internal adjustments as necessary to avoid this resonance.
>>>
>>> In order for a device to support this, the expected flow for device
>>> driver or subsystems:
>>>
>>> Drivers/subsystems contributing frequencies:
>>>
>>> 1) During probe, check `wbrf_supported_producer` to see if WBRF
>> supported
>>> for the device.
>>> 2) If adding frequencies, then call `wbrf_add_exclusion` with the
>>> start and end ranges of the frequencies.
>>> 3) If removing frequencies, then call `wbrf_remove_exclusion` with
>>> start and end ranges of the frequencies.
>>>
>>> Drivers/subsystems responding to frequencies:
>>>
>>> 1) During probe, check `wbrf_supported_consumer` to see if WBRF is
>> supported
>>> for the device.
>>> 2) Call the `wbrf_retrieve_exclusions` to retrieve the current
>>> exclusions on receiving an ACPI notification for a new frequency
>>> change.
>>>
>>> Co-developed-by: Mario Limonciello <mario.limonciello@....com>
>>> Signed-off-by: Mario Limonciello <mario.limonciello@....com>
>>> Co-developed-by: Evan Quan <evan.quan@....com>
>>> Signed-off-by: Evan Quan <evan.quan@....com>
>>> --
>>> v4->v5:
>>> - promote this to be a more generic solution with input argument taking
>>> `struct device` and provide better scalability to support non-ACPI
>>> scenarios(Andrew)
>>> - update the APIs naming and some other minor fixes(Rafael)
>>> ---
>>> drivers/base/Kconfig | 8 ++
>>> drivers/base/Makefile | 1 +
>>> drivers/base/wbrf.c | 227
>> ++++++++++++++++++++++++++++++++++++++++++
>>> include/linux/wbrf.h | 65 ++++++++++++
>>> 4 files changed, 301 insertions(+)
>>> create mode 100644 drivers/base/wbrf.c
>>> create mode 100644 include/linux/wbrf.h
>>>
>>> diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig index
>>> 2b8fd6bb7da0..5b441017b225 100644
>>> --- a/drivers/base/Kconfig
>>> +++ b/drivers/base/Kconfig
>>> @@ -242,4 +242,12 @@ config FW_DEVLINK_SYNC_STATE_TIMEOUT
>>> command line option on every system/board your kernel is expected
>> to
>>> work on.
>>>
>>> +config WBRF
>>> + bool "Wifi band RF mitigation mechanism"
>>> + default n
>>> + help
>>> + Wifi band RF mitigation mechanism allows multiple drivers from
>>> + different domains to notify the frequencies in use so that hardware
>>> + can be reconfigured to avoid harmonic conflicts.
>>> +
>>> endmenu
>>> diff --git a/drivers/base/Makefile b/drivers/base/Makefile index
>>> 3079bfe53d04..c844f68a6830 100644
>>> --- a/drivers/base/Makefile
>>> +++ b/drivers/base/Makefile
>>> @@ -26,6 +26,7 @@ obj-$(CONFIG_GENERIC_MSI_IRQ) += platform-msi.o
>>> obj-$(CONFIG_GENERIC_ARCH_TOPOLOGY) += arch_topology.o
>>> obj-$(CONFIG_GENERIC_ARCH_NUMA) += arch_numa.o
>>> obj-$(CONFIG_ACPI) += physical_location.o
>>> +obj-$(CONFIG_WBRF) += wbrf.o
>>>
>>> obj-y += test/
>>>
>>> diff --git a/drivers/base/wbrf.c b/drivers/base/wbrf.c new file mode
>>> 100644 index 000000000000..2163a8ec8a9a
>>> --- /dev/null
>>> +++ b/drivers/base/wbrf.c
>>> @@ -0,0 +1,227 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +/*
>>> + * Wifi Band Exclusion Interface
>>> + * Copyright (C) 2023 Advanced Micro Devices
>>> + *
>>> + */
>>> +
>>> +#include <linux/wbrf.h>
>>> +
>>> +static BLOCKING_NOTIFIER_HEAD(wbrf_chain_head);
>>> +static DEFINE_MUTEX(wbrf_mutex);
>>> +static struct exclusion_range_pool wbrf_pool;
>>> +
>>> +static int _wbrf_add_exclusion_ranges(struct wbrf_ranges_in *in) {
>>> + int i, j;
>>> +
>>> + for (i = 0; i < ARRAY_SIZE(in->band_list); i++) {
>>> + if (!in->band_list[i].start &&
>>> + !in->band_list[i].end)
>>> + continue;
>>> +
>>> + for (j = 0; j < ARRAY_SIZE(wbrf_pool.band_list); j++) {
>>> + if (wbrf_pool.band_list[j].start == in-
>>> band_list[i].start &&
>>> + wbrf_pool.band_list[j].end == in->band_list[i].end) {
>>> + wbrf_pool.ref_counter[j]++;
>>> + break;
>>> + }
>>> + }
>>> + if (j < ARRAY_SIZE(wbrf_pool.band_list))
>>> + continue;
>>> +
>>> + for (j = 0; j < ARRAY_SIZE(wbrf_pool.band_list); j++) {
>>> + if (!wbrf_pool.band_list[j].start &&
>>> + !wbrf_pool.band_list[j].end) {
>>> + wbrf_pool.band_list[j].start = in-
>>> band_list[i].start;
>>> + wbrf_pool.band_list[j].end = in-
>>> band_list[i].end;
>>> + wbrf_pool.ref_counter[j] = 1;
>>> + break;
>>> + }
>>> + }
>>> + if (j >= ARRAY_SIZE(wbrf_pool.band_list))
>>> + return -ENOSPC;
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int _wbrf_remove_exclusion_ranges(struct wbrf_ranges_in *in) {
>>> + int i, j;
>>> +
>>> + for (i = 0; i < ARRAY_SIZE(in->band_list); i++) {
>>> + if (!in->band_list[i].start &&
>>> + !in->band_list[i].end)
>>> + continue;
>>> +
>>> + for (j = 0; j < ARRAY_SIZE(wbrf_pool.band_list); j++) {
>>> + if (wbrf_pool.band_list[j].start == in-
>>> band_list[i].start &&
>>> + wbrf_pool.band_list[j].end == in->band_list[i].end) {
>>> + wbrf_pool.ref_counter[j]--;
>>> + if (!wbrf_pool.ref_counter[j]) {
>>> + wbrf_pool.band_list[j].start = 0;
>>> + wbrf_pool.band_list[j].end = 0;
>>> + }
>>> + break;
>>> + }
>>> + }
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int _wbrf_retrieve_exclusion_ranges(struct wbrf_ranges_out
>>> +*out) {
>>> + int out_idx = 0;
>>> + int i;
>>> +
>>> + memset(out, 0, sizeof(*out));
>>> +
>>> + for (i = 0; i < ARRAY_SIZE(wbrf_pool.band_list); i++) {
>>> + if (!wbrf_pool.band_list[i].start &&
>>> + !wbrf_pool.band_list[i].end)
>>> + continue;
>>> +
>>> + out->band_list[out_idx].start = wbrf_pool.band_list[i].start;
>>> + out->band_list[out_idx++].end = wbrf_pool.band_list[i].end;
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +/**
>>> + * wbrf_supported_producer - Determine if the device can report
>>> +frequencies
>>> + *
>>> + * @dev: device pointer
>>> + *
>>> + * WBRF is used to mitigate devices that cause harmonic interference.
>>> + * This function will determine if this device needs to report such
>> frequencies.
>>> + */
>>> +bool wbrf_supported_producer(struct device *dev) {
>>> + return true;
>>> +}
>>> +EXPORT_SYMBOL_GPL(wbrf_supported_producer);
>>> +
>>> +/**
>>> + * wbrf_add_exclusion - Add frequency ranges to the exclusion list
>>> + *
>>> + * @dev: device pointer
>>> + * @in: input structure containing the frequency ranges to be added
>>> + *
>>> + * Add frequencies into the exclusion list for supported consumers
>>> + * to react to.
>>> + */
>>> +int wbrf_add_exclusion(struct device *dev,
>>> + struct wbrf_ranges_in *in)
>>> +{
>>> + int r;
>>> +
>>> + mutex_lock(&wbrf_mutex);
>>> +
>>> + r = _wbrf_add_exclusion_ranges(in);
>>> +
>>> + mutex_unlock(&wbrf_mutex);
>>> + if (r)
>>> + return r;
>>> +
>>> + blocking_notifier_call_chain(&wbrf_chain_head, WBRF_CHANGED,
>> NULL);
>>> +
>>> + return 0;
>>> +}
>>> +EXPORT_SYMBOL_GPL(wbrf_add_exclusion);
>>> +
>>> +/**
>>> + * wbrf_remove_exclusion - Remove frequency ranges from the exclusion
>>> +list
>>> + *
>>> + * @dev: device pointer
>>> + * @in: input structure containing the frequency ranges to be removed
>>> + *
>>> + * Remove frequencies from the exclusion list for supported consumers
>>> + * to react to.
>>> + */
>>> +int wbrf_remove_exclusion(struct device *dev,
>>> + struct wbrf_ranges_in *in)
>>> +{
>>> + int r;
>>> +
>>> + mutex_lock(&wbrf_mutex);
>>> +
>>> + r = _wbrf_remove_exclusion_ranges(in);
>>> +
>>> + mutex_unlock(&wbrf_mutex);
>>> + if (r)
>>> + return r;
>>> +
>>> + blocking_notifier_call_chain(&wbrf_chain_head, WBRF_CHANGED,
>> NULL);
>>> +
>>> + return 0;
>>> +}
>>> +EXPORT_SYMBOL_GPL(wbrf_remove_exclusion);
>>> +
>>> +/**
>>> + * wbrf_supported_consumer - Determine if the device can react to
>>> +frequencies
>>> + *
>>> + * @dev: device pointer
>>> + *
>>> + * WBRF is used to mitigate devices that cause harmonic interference.
>>> + * This function will determine if this device needs to react to
>>> +reports from
>>> + * other devices for such frequencies.
>>> + */
>>> +bool wbrf_supported_consumer(struct device *dev) {
>>> + return true;
>>> +}
>>> +EXPORT_SYMBOL_GPL(wbrf_supported_consumer);
>>> +
>>> +/**
>>> + * wbrf_register_notifier - Register for notifications of frequency
>>> +changes
>>> + *
>>> + * @nb: driver notifier block
>>> + *
>>> + * WBRF is used to mitigate devices that cause harmonic interference.
>>> + * This function will allow consumers to register for frequency notifications.
>>> + */
>>> +int wbrf_register_notifier(struct notifier_block *nb) {
>>> + return blocking_notifier_chain_register(&wbrf_chain_head, nb); }
>>> +EXPORT_SYMBOL_GPL(wbrf_register_notifier);
>>> +
>>> +/**
>>> + * wbrf_unregister_notifier - Unregister for notifications of
>>> +frequency changes
>>> + *
>>> + * @nb: driver notifier block
>>> + *
>>> + * WBRF is used to mitigate devices that cause harmonic interference.
>>> + * This function will allow consumers to unregister for frequency
>> notifications.
>>> + */
>>> +int wbrf_unregister_notifier(struct notifier_block *nb) {
>>> + return blocking_notifier_chain_unregister(&wbrf_chain_head, nb); }
>>> +EXPORT_SYMBOL_GPL(wbrf_unregister_notifier);
>>> +
>>> +/**
>>> + * wbrf_retrieve_exclusions - Retrieve the exclusion list
>>> + *
>>> + * @dev: device pointer
>>> + * @out: output structure containing the frequency ranges to be
>>> +excluded
>>> + *
>>> + * Retrieve the current exclusion list */ int
>>> +wbrf_retrieve_exclusions(struct device *dev,
>>> + struct wbrf_ranges_out *out)
>>> +{
>>> + int r;
>>> +
>>> + mutex_lock(&wbrf_mutex);
>>> +
>>> + r = _wbrf_retrieve_exclusion_ranges(out);
>>> +
>>> + mutex_unlock(&wbrf_mutex);
>>> +
>>> + return r;
>>> +}
>>> +EXPORT_SYMBOL_GPL(wbrf_retrieve_exclusions);
>>> diff --git a/include/linux/wbrf.h b/include/linux/wbrf.h new file mode
>>> 100644 index 000000000000..3ca95786cef5
>>> --- /dev/null
>>> +++ b/include/linux/wbrf.h
>>> @@ -0,0 +1,65 @@
>>> +/* SPDX-License-Identifier: GPL-2.0 */
>>> +/*
>>> + * Wifi Band Exclusion Interface
>>> + * Copyright (C) 2023 Advanced Micro Devices */
>>> +
>>> +#ifndef _LINUX_WBRF_H
>>> +#define _LINUX_WBRF_H
>>> +
>>> +#include <linux/device.h>
>>> +
>>> +/* Maximum number of wbrf ranges */
>>> +#define MAX_NUM_OF_WBRF_RANGES 11
>>> +
>>> +struct exclusion_range {
>>> + /* start and end point of the frequency range in Hz */
>>> + uint64_t start;
>>> + uint64_t end;
>>> +};
>>> +
>>> +struct exclusion_range_pool {
>>> + struct exclusion_range band_list[MAX_NUM_OF_WBRF_RANGES];
>>> + uint64_t
>> ref_counter[MAX_NUM_OF_WBRF_RANGES];
>>> +};
>>> +
>>> +struct wbrf_ranges_in {
>>> + /* valid entry: `start` and `end` filled with non-zero values */
>>> + struct exclusion_range band_list[MAX_NUM_OF_WBRF_RANGES];
>>> +};
>>> +
>>> +struct wbrf_ranges_out {
>>> + uint32_t num_of_ranges;
>>> + struct exclusion_range band_list[MAX_NUM_OF_WBRF_RANGES];
>>> +} __packed;
>>> +
>>> +enum wbrf_notifier_actions {
>>> + WBRF_CHANGED,
>>> +};
>>> +
>>> +#ifdef CONFIG_WBRF
>>> +bool wbrf_supported_producer(struct device *dev); int
>>> +wbrf_add_exclusion(struct device *adev,
>>> + struct wbrf_ranges_in *in);
>>> +int wbrf_remove_exclusion(struct device *dev,
>>> + struct wbrf_ranges_in *in);
>>> +int wbrf_retrieve_exclusions(struct device *dev,
>>> + struct wbrf_ranges_out *out); bool
>>> +wbrf_supported_consumer(struct device *dev);
>>> +
>>> +int wbrf_register_notifier(struct notifier_block *nb); int
>>> +wbrf_unregister_notifier(struct notifier_block *nb); #else static
>>> +inline bool wbrf_supported_producer(struct device *dev) { return
>>> +false; } static inline int wbrf_add_exclusion(struct device *adev,
>>> + struct wbrf_ranges_in *in) { return -
>> ENODEV; } static inline
>>> +int wbrf_remove_exclusion(struct device *dev,
>>> + struct wbrf_ranges_in *in) { return -
>> ENODEV; } static inline int
>>> +wbrf_retrieve_exclusions(struct device *dev,
>>> + struct wbrf_ranges_out *out)
>> { return -ENODEV; } static
>>> +inline bool wbrf_supported_consumer(struct device *dev) { return
>>> +false; } static inline int wbrf_register_notifier(struct
>>> +notifier_block *nb) { return -ENODEV; } static inline int
>>> +wbrf_unregister_notifier(struct notifier_block *nb) { return -ENODEV;
>>> +} #endif
>>> +
>>
>> Right now there are stubs for non CONFIG_WBRF as well as other patches are
>> using #ifdef CONFIG_WBRF or having their own stubs. Like mac80211 patch
>> looks for #ifdef CONFIG_WBRF.
>>
>> I think we should pick one or the other.
> Right..
>>
>> Having other subsystems #ifdef CONFIG_WBRF will make the series easier to
>> land through multiple trees; so I have a slight leaning in that direction.
> I kind of expecting to use the other way. That is to make CONFIG_WBRF agnostic to other subsystems or drivers.
> They (other subsystems or drivers) can always assume those wbrf_xxxxx interfaces are available.
> What they need to care only are the return values of those interfaces.
> How do you think?
That's fine, thanks.
>
> Evan
>>
>>> +#endif /* _LINUX_WBRF_H */
>
Powered by blists - more mailing lists