* Re: [PATCH v2 1/2] cgroup/cpuset: Avoid unnecessary cpus & mems update in cpuset_hotplug_update_tasks()
From: Ridong Chen @ 2026-06-24 5:51 UTC (permalink / raw)
To: Waiman Long, Tejun Heo, Johannes Weiner, Michal Koutný,
Jonathan Corbet, Shuah Khan
Cc: cgroups, linux-kernel, linux-doc, linux-kselftest
In-Reply-To: <20260623230413.1984188-2-longman@redhat.com>
On 6/24/2026 7:04 AM, Waiman Long wrote:
> As reported by sashiko [1], cpuset_hotplug_update_tasks() may perform
> unnecessary task iteration and updating of tasks' CPU and node masks
> when mems_allowed and/or cpus_allowed are not set in cpuset v2. It is
> due to the fact that the temporary new_cpus and new_mems masks do not
> inherit parent's effective_cpus/mems when they are empty which is the
> expected behavior for cpuset v2 since commit 4ec22e9c5a90 ("cpuset:
> Enable cpuset controller in default hierarchy").
>
> Fix that and avoid unnecessay work by enhancing
> compute_effective_cpumask() to add the empty cpumask check
> and inheriting the parent's versions if empty when in v2. A new
> compute_effective_nodemask() helper is also added to perform similar
> function for new effective_mems.
>
> Add new test_cpuset_prs.sh test cases to confirm that effective_cpus
> will inherit the parent's version if cpuset.cpus is empty.
>
> [1] https://sashiko.dev/#/patchset/20260621032816.1806773-1-longman%40redhat.com
>
> Suggested-by: Ridong Chen <ridong.chen@linux.dev>
> Fixes: 4ec22e9c5a90 ("cpuset: Enable cpuset controller in default hierarchy")
> Signed-off-by: Waiman Long <longman@redhat.com>
> ---
> kernel/cgroup/cpuset.c | 45 +++++++++++--------
> .../selftests/cgroup/test_cpuset_prs.sh | 11 ++++-
> 2 files changed, 35 insertions(+), 21 deletions(-)
>
> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
> index aff86acea701..044ddbf66f8e 100644
> --- a/kernel/cgroup/cpuset.c
> +++ b/kernel/cgroup/cpuset.c
> @@ -1094,12 +1094,35 @@ void cpuset_update_tasks_cpumask(struct cpuset *cs, struct cpumask *new_cpus)
> * @cs: the cpuset the need to recompute the new effective_cpus mask
> * @parent: the parent cpuset
> *
> + * For v2, the parent's effective_cpus is inherited if cpumask empty.
> * The result is valid only if the given cpuset isn't a partition root.
> */
> static void compute_effective_cpumask(struct cpumask *new_cpus,
> struct cpuset *cs, struct cpuset *parent)
> {
> - cpumask_and(new_cpus, cs->cpus_allowed, parent->effective_cpus);
> + bool has_cpus;
> +
> + has_cpus = cpumask_and(new_cpus, cs->cpus_allowed, parent->effective_cpus);
> + if (!has_cpus && is_in_v2_mode())
> + cpumask_copy(new_cpus, parent->effective_cpus);
> +}
> +
> +/**
> + * compute_effective_nodemask - Compute the effective nodemask of the cpuset
> + * @new_cpus: the temp variable for the new effective_mems mask
> + * @cs: the cpuset the need to recompute the new effective_mems mask
> + * @parent: the parent cpuset
> + *
> + * For v2, the parent's effective_mems is inherited if nodemask empty.
> + */
> +static void compute_effective_nodemask(nodemask_t *new_mems,
> + struct cpuset *cs, struct cpuset *parent)
> +{
> + bool has_mems;
> +
> + has_mems = nodes_and(*new_mems, cs->mems_allowed, parent->effective_mems);
> + if (!has_mems && is_in_v2_mode())
> + nodes_copy(*new_mems, parent->effective_mems);
> }
>
> /*
> @@ -2148,15 +2171,6 @@ static void update_cpumasks_hier(struct cpuset *cs, struct tmpmasks *tmp,
> goto update_parent_effective;
> }
>
> - /*
> - * If it becomes empty, inherit the effective mask of the
> - * parent, which is guaranteed to have some CPUs unless
> - * it is a partition root that has explicitly distributed
> - * out all its CPUs.
> - */
> - if (is_in_v2_mode() && !remote && cpumask_empty(tmp->new_cpus))
> - cpumask_copy(tmp->new_cpus, parent->effective_cpus);
> -
> /*
> * Skip the whole subtree if
> * 1) the cpumask remains the same,
> @@ -2704,14 +2718,7 @@ static void update_nodemasks_hier(struct cpuset *cs, nodemask_t *new_mems)
> cpuset_for_each_descendant_pre(cp, pos_css, cs) {
> struct cpuset *parent = parent_cs(cp);
>
> - bool has_mems = nodes_and(*new_mems, cp->mems_allowed, parent->effective_mems);
> -
> - /*
> - * If it becomes empty, inherit the effective mask of the
> - * parent, which is guaranteed to have some MEMs.
> - */
> - if (is_in_v2_mode() && !has_mems)
> - *new_mems = parent->effective_mems;
> + compute_effective_nodemask(new_mems, cp, parent);
>
> /* Skip the whole subtree if the nodemask remains the same. */
> if (nodes_equal(*new_mems, cp->effective_mems)) {
> @@ -3923,7 +3930,7 @@ static void cpuset_hotplug_update_tasks(struct cpuset *cs, struct tmpmasks *tmp)
>
> parent = parent_cs(cs);
> compute_effective_cpumask(&new_cpus, cs, parent);
> - nodes_and(new_mems, cs->mems_allowed, parent->effective_mems);
> + compute_effective_nodemask(&new_mems, cs, parent);
>
> if (!tmp || !cs->partition_root_state)
> goto update_tasks;
> diff --git a/tools/testing/selftests/cgroup/test_cpuset_prs.sh b/tools/testing/selftests/cgroup/test_cpuset_prs.sh
> index 0d41aa0d343d..ca9bc38fdb95 100755
> --- a/tools/testing/selftests/cgroup/test_cpuset_prs.sh
> +++ b/tools/testing/selftests/cgroup/test_cpuset_prs.sh
> @@ -495,13 +495,20 @@ REMOTE_TEST_MATRIX=(
> # Narrowing cpuset.cpus to previously sibling-excluded CPUs should
> # not return CPUs that were never actually owned.
> " C1-4:P1 . C1-2:P1 C1-3:P2 . . \
> - . . . C3 . . p1:4|c11:1-2|c12:3 \
> + . . . C3 . . p1:4|c11:1-2|c12:3 \
> p1:P1|c11:P1|c12:P2 3"
> # Expanding cpuset.cpus to include a previously sibling-excluded CPU
> # after the sibling has become a member should correctly request it.
> " C1-4:P1 . C1-2:P1 C1-3:P2 . . \
> - . . P0 C2-3 . . p1:1,4|c11:1|c12:2-3 \
> + . . P0 C2-3 . . p1:1,4|c11:1|c12:2-3 \
> p1:P1|c11:P0|c12:P2 2-3"
> + # Cpusets with empty cpuset.cpus should inherit parent's effective_cpus
> + " C1-4:P1 C5-6 C1-2 . C5 . \
> + . P1 P1 . . . p1:3-4|p2:5-6|c11:1-2|c12:3-4|c21:5|c22:5-6 \
> + p1:P1|p2:P1|c11:P1"
> + " C1-4:P1 C5-6 C1-2 . C5 . \
> + . P1 P1 . O5=0 . p1:3-4|p2:6|c11:1-2|c12:3-4|c21:6|c22:6 \
> + p1:P1|p2:P1|c11:P1"
> )
>
> #
LGTM.
Reviewed-by: Ridong Chen <ridong.chen@linux.dev>
--
Best regards
Ridong
^ permalink raw reply
* Re: [RFC PATCH 0/2] kasan: hw_tags: Add option to tag only at allocation time
From: Dev Jain @ 2026-06-24 4:14 UTC (permalink / raw)
To: Catalin Marinas
Cc: Harry Yoo, ryabinin.a.a, akpm, corbet, glider, andreyknvl,
dvyukov, vincenzo.frascino, kasan-dev, linux-mm, linux-kernel,
skhan, workflows, linux-doc, linux-arm-kernel, ryan.roberts,
anshuman.khandual, kaleshsingh, 21cnbao, david, will
In-Reply-To: <ajq-Ukmd9NBruhr5@arm.com>
On 23/06/26 10:41 pm, Catalin Marinas wrote:
> On Tue, Jun 23, 2026 at 10:32:16AM +0530, Dev Jain wrote:
>> On 22/06/26 10:43 pm, Catalin Marinas wrote:
>>> On Mon, Jun 22, 2026 at 09:42:10PM +0900, Harry Yoo wrote:
>>>> On 6/19/26 10:19 PM, Catalin Marinas wrote:
>>>>> On Thu, Jun 18, 2026 at 10:35:15PM +0900, Harry Yoo wrote:
>>>>>> On 6/12/26 1:44 PM, Dev Jain wrote:
>>>>>>> Now, when a memory object will be freed, it will retain the random tag it
>>>>>>> had at allocation time. This compromises on catching UAF bugs, till the
>>>>>>> time the object is not reallocated, at which point it will have a new
>>>>>>> random tag.
>>>>>>>
>>>>>>> Hence, not catching "use-after-free-before-reallocation" and not catching
>>>>>>> "double-free" will be the compromise for reduced KASAN overhead.
>>>>>>
>>>>>> I doubt users who care about security enough to enable HW_TAGS KASAN
>>>>>> are willing to compromise on security just to save a few instructions
>>>>>> to store tags in the free path.
>>>>>>
>>>>>> To me, it looks like too much of a compromise on security for little
>>>>>> performance gain.
>>>>>
>>>>> I don't think there's much compromise on security for use-after-free.
>>>>
>>>> I think it depends... OH, WAIT! I see what you mean.
>>>>
>>>> You mean use-after-free before reallocation does not lead to much
>>>> compromise on security because objects are initialized after allocation?
>>>>
>>>> You're probably right.
>>>>
>>>> Hmm, but stores to e.g.) free pointer, fields initialized by
>>>> constructor or accessed by SLAB_TYPESAFE_BY_RCU semantics after free
>>>> will be undiscovered if they happen before reallocation.
>>>
>>> Even with SLAB_TYPESAFE_BY_RCU, the object isn't tagged on free either
>>> (or realloc, only if the actual slab page ends up freed). But we don't
>>> get type confusion for such slab.
>>>
>>> However, without tagging on free, one could argue that it reduces
>>> security for cases where the page is re-allocated as untagged - e.g. all
>>> user pages mapped without PROT_MTE. Currently we have a deterministic
>>> tag check fault if the page is coloured as KASAN_TAG_INVALID. I think
>>
>> So you are saying that a stale kernel pointer can continue to use the
>> reallocated page, because for non-PROT_MTE case the page does not get
>> a new tag. Makes sense.
>
> Yes.
>
>>> for this patch, it might be better to only do such skip on free in
>>> kasan_poison_slab() rather than kasan_poison(). Freed pages would then
>>> be tagged.
>>
>> I think you mean to say, "skip tag on free when freeing pages into buddy"?
>
> No, I meant always poison via kasan_poison_pages(), as we currently do
> with KASAN_PAGE_FREE being set.
>
>> So that would mean, kasan_poison() will do the poisoning also in the
>> case of value == KASAN_PAGE_FREE.
Yeah sorry I wrote two contradictory things above, that's what I meant too.
>>
>>> An alternative would be tagging on free only with a new tag and skipping
>>> it on re-alloc. But we'd need to track when it's a completely new
>>> allocation or a reused object (I haven't looked I'm pretty sure it's
>>> doable).
>>
>> That was our original approach, and IIRC we had concluded there was no
>> security compromise. However it is difficult to implement - it has cases
>> like, what happens when two differently tagged pages are coalesced by
>> buddy and someone gets that large page as an allocation.
>
> Yeah, it's fairly complex.
>
> I think the more problematic case is when we can't detect
> use-after-reallocation and this happens when a page is reused untagged
> (probabilistically, also when the page is reused with the old tag). As
> an optimisation, it might be sufficient to skip poisoning when freeing
> an object into the slab but keep the poisoning when freeing a slab page
> into the buddy allocator. That's where the page may end up in a place
> untagged.
>
> Also for your optimisation to only tag on reallocation, do you have any
> code to read the current tag and avoid reusing it? That's useful for
> kmalloc caches or other merged kmem caches where we can have type
> confusion.
I don't have it, but should be fairly simple I guess. I just wanted to
keep it simple for now.
Anyhow someone needs to first test the current patchset to get some
numbers, we would be wasting time on this if no one gets an improvement.
>
^ permalink raw reply
* Re: [PATCH] cxl: docs/linux/dax-driver - fix typos
From: Alison Schofield @ 2026-06-24 3:37 UTC (permalink / raw)
To: Zenghui Yu
Cc: linux-cxl, linux-doc, linux-kernel, dave, jic23, dave.jiang,
vishal.l.verma, ira.weiny, djbw, gourry, corbet, skhan
In-Reply-To: <20260614161458.88942-1-zenghui.yu@linux.dev>
On Mon, Jun 15, 2026 at 12:14:58AM +0800, Zenghui Yu wrote:
> Fix two obvious typos in the "kmem conversion" section.
>
> Signed-off-by: Zenghui Yu <zenghui.yu@linux.dev>
Thanks,
Reviewed-by: Alison Schofield <alison.schofield@intel.com>
^ permalink raw reply
* [PATCH 2/2] cgroup/dmem: introduce dmem.events.local for local counts
From: Hongfu Li @ 2026-06-24 3:11 UTC (permalink / raw)
To: tj, hannes, mkoutny, corbet, skhan, dev, mripard, natalie.vock
Cc: cgroups, linux-doc, linux-kernel, dri-devel, Hongfu Li
In-Reply-To: <20260624031107.667253-1-lihongfu@kylinos.cn>
Add dmem.events.local for local-only low/max event counts per DMEM
region. Refactor the shared events show logic used by dmem.events.
Signed-off-by: Hongfu Li <lihongfu@kylinos.cn>
---
Documentation/admin-guide/cgroup-v2.rst | 5 ++--
kernel/cgroup/dmem.c | 32 +++++++++++++++++++++----
2 files changed, 31 insertions(+), 6 deletions(-)
diff --git a/Documentation/admin-guide/cgroup-v2.rst b/Documentation/admin-guide/cgroup-v2.rst
index afc924539a41..5e4dbe4a75c6 100644
--- a/Documentation/admin-guide/cgroup-v2.rst
+++ b/Documentation/admin-guide/cgroup-v2.rst
@@ -2881,7 +2881,7 @@ DMEM Interface Files
drm/0000:03:00.0/vram0 12550144
drm/0000:03:00.0/stolen 8650752
- dmem.events
+ dmem.events, dmem.events.local
A read-only file that reports the number of times each cgroup
has hit its configured memory limits. The format lists each
region on a single line, followed by the event counters::
@@ -2894,7 +2894,8 @@ DMEM Interface Files
``max`` counts how many times an allocation failed because the
cgroup or one of its ancestors hit ``dmem.max``.
- ``dmem.events`` contains hierarchical counts. This file exists
+ ``dmem.events`` contains hierarchical counts. ``dmem.events.local``
+ contains counts for only the cgroup itself. These files exist
for all cgroups except root.
HugeTLB
diff --git a/kernel/cgroup/dmem.c b/kernel/cgroup/dmem.c
index 79d4c5d0a046..29f8719561e6 100644
--- a/kernel/cgroup/dmem.c
+++ b/kernel/cgroup/dmem.c
@@ -60,6 +60,7 @@ struct dmemcg_state {
struct list_head pools;
struct cgroup_file events_file;
+ struct cgroup_file events_local_file;
};
enum dmemcg_memory_event {
@@ -84,6 +85,7 @@ struct dmem_cgroup_pool_state {
struct dmem_cgroup_pool_state *parent;
atomic_long_t events[DMEMCG_NR_EVENTS];
+ atomic_long_t events_local[DMEMCG_NR_EVENTS];
refcount_t ref;
bool inited;
@@ -196,6 +198,9 @@ static u64 get_resource_current(struct dmem_cgroup_pool_state *pool)
static void dmemcg_memory_event(struct dmem_cgroup_pool_state *pool,
enum dmemcg_memory_event event)
{
+ atomic_long_inc(&pool->events_local[event]);
+ cgroup_file_notify(&pool->cs->events_local_file);
+
for (; pool; pool = pool->parent) {
atomic_long_inc(&pool->events[event]);
cgroup_file_notify(&pool->cs->events_file);
@@ -203,11 +208,14 @@ static void dmemcg_memory_event(struct dmem_cgroup_pool_state *pool,
}
static long dmemcg_get_event(struct dmem_cgroup_pool_state *pool,
- enum dmemcg_memory_event event)
+ enum dmemcg_memory_event event, bool local)
{
if (!pool)
return 0;
+ if (local)
+ return atomic_long_read(&pool->events_local[event]);
+
return atomic_long_read(&pool->events[event]);
}
@@ -874,7 +882,7 @@ static int dmem_cgroup_region_max_show(struct seq_file *sf, void *v)
return dmemcg_limit_show(sf, v, get_resource_max);
}
-static int dmem_cgroup_region_events_show(struct seq_file *sf, void *v)
+static int dmemcg_events_show(struct seq_file *sf, void *v, bool local)
{
struct dmemcg_state *dmemcs = css_to_dmemcs(seq_css(sf));
struct dmem_cgroup_region *region;
@@ -885,14 +893,24 @@ static int dmem_cgroup_region_events_show(struct seq_file *sf, void *v)
seq_puts(sf, region->name);
seq_printf(sf, " low %ld max %ld\n",
- dmemcg_get_event(pool, DMEMCG_LOW),
- dmemcg_get_event(pool, DMEMCG_MAX));
+ dmemcg_get_event(pool, DMEMCG_LOW, local),
+ dmemcg_get_event(pool, DMEMCG_MAX, local));
}
rcu_read_unlock();
return 0;
}
+static int dmem_cgroup_region_events_show(struct seq_file *sf, void *v)
+{
+ return dmemcg_events_show(sf, v, false);
+}
+
+static int dmem_cgroup_region_events_local_show(struct seq_file *sf, void *v)
+{
+ return dmemcg_events_show(sf, v, true);
+}
+
static ssize_t dmem_cgroup_region_max_write(struct kernfs_open_file *of,
char *buf, size_t nbytes, loff_t off)
{
@@ -933,6 +951,12 @@ static struct cftype files[] = {
.file_offset = offsetof(struct dmemcg_state, events_file),
.flags = CFTYPE_NOT_ON_ROOT,
},
+ {
+ .name = "events.local",
+ .seq_show = dmem_cgroup_region_events_local_show,
+ .file_offset = offsetof(struct dmemcg_state, events_local_file),
+ .flags = CFTYPE_NOT_ON_ROOT,
+ },
{ } /* Zero entry terminates. */
};
--
2.25.1
^ permalink raw reply related
* [PATCH 1/2] cgroup/dmem: add per-region event counters
From: Hongfu Li @ 2026-06-24 3:11 UTC (permalink / raw)
To: tj, hannes, mkoutny, corbet, skhan, dev, mripard, natalie.vock
Cc: cgroups, linux-doc, linux-kernel, dri-devel, Hongfu Li
In-Reply-To: <20260624031107.667253-1-lihongfu@kylinos.cn>
Add dmem.events to report hierarchical low/max event counts per DMEM
region. Increment counters on dmem.max allocation failures and
dmem.low protection events. The file is available for non-root cgroups
only.
Signed-off-by: Hongfu Li <lihongfu@kylinos.cn>
---
Documentation/admin-guide/cgroup-v2.rst | 16 +++++++
kernel/cgroup/dmem.c | 61 ++++++++++++++++++++++++-
2 files changed, 76 insertions(+), 1 deletion(-)
diff --git a/Documentation/admin-guide/cgroup-v2.rst b/Documentation/admin-guide/cgroup-v2.rst
index 993446ab66d0..afc924539a41 100644
--- a/Documentation/admin-guide/cgroup-v2.rst
+++ b/Documentation/admin-guide/cgroup-v2.rst
@@ -2881,6 +2881,22 @@ DMEM Interface Files
drm/0000:03:00.0/vram0 12550144
drm/0000:03:00.0/stolen 8650752
+ dmem.events
+ A read-only file that reports the number of times each cgroup
+ has hit its configured memory limits. The format lists each
+ region on a single line, followed by the event counters::
+
+ drm/0000:03:00.0/vram0 low 0 max 3
+ drm/0000:03:00.0/stolen low 0 max 0
+
+ ``low`` counts how many times reclaim or eviction considered
+ the cgroup to be below its effective ``dmem.low`` protection.
+ ``max`` counts how many times an allocation failed because the
+ cgroup or one of its ancestors hit ``dmem.max``.
+
+ ``dmem.events`` contains hierarchical counts. This file exists
+ for all cgroups except root.
+
HugeTLB
-------
diff --git a/kernel/cgroup/dmem.c b/kernel/cgroup/dmem.c
index 4753a67d0f0f..79d4c5d0a046 100644
--- a/kernel/cgroup/dmem.c
+++ b/kernel/cgroup/dmem.c
@@ -8,6 +8,7 @@
* Copyright (C) 2016 Parav Pandit <pandit.parav@gmail.com>
*/
+#include <linux/atomic.h>
#include <linux/cgroup.h>
#include <linux/cgroup_dmem.h>
#include <linux/list.h>
@@ -57,6 +58,14 @@ struct dmemcg_state {
struct cgroup_subsys_state css;
struct list_head pools;
+
+ struct cgroup_file events_file;
+};
+
+enum dmemcg_memory_event {
+ DMEMCG_LOW,
+ DMEMCG_MAX,
+ DMEMCG_NR_EVENTS,
};
struct dmem_cgroup_pool_state {
@@ -74,6 +83,8 @@ struct dmem_cgroup_pool_state {
struct page_counter cnt;
struct dmem_cgroup_pool_state *parent;
+ atomic_long_t events[DMEMCG_NR_EVENTS];
+
refcount_t ref;
bool inited;
};
@@ -182,6 +193,24 @@ static u64 get_resource_current(struct dmem_cgroup_pool_state *pool)
return pool ? page_counter_read(&pool->cnt) : 0;
}
+static void dmemcg_memory_event(struct dmem_cgroup_pool_state *pool,
+ enum dmemcg_memory_event event)
+{
+ for (; pool; pool = pool->parent) {
+ atomic_long_inc(&pool->events[event]);
+ cgroup_file_notify(&pool->cs->events_file);
+ }
+}
+
+static long dmemcg_get_event(struct dmem_cgroup_pool_state *pool,
+ enum dmemcg_memory_event event)
+{
+ if (!pool)
+ return 0;
+
+ return atomic_long_read(&pool->events[event]);
+}
+
static void reset_all_resource_limits(struct dmem_cgroup_pool_state *rpool)
{
set_resource_min(rpool, 0);
@@ -345,6 +374,7 @@ bool dmem_cgroup_state_evict_valuable(struct dmem_cgroup_pool_state *limit_pool,
return true;
*ret_hit_low = true;
+ dmemcg_memory_event(test_pool, DMEMCG_LOW);
return false;
}
return true;
@@ -675,8 +705,12 @@ int dmem_cgroup_try_charge(struct dmem_cgroup_region *region, u64 size,
}
if (!page_counter_try_charge(&pool->cnt, size, &fail)) {
+ struct dmem_cgroup_pool_state *limit_pool;
+
+ limit_pool = container_of(fail, struct dmem_cgroup_pool_state, cnt);
+ dmemcg_memory_event(limit_pool, DMEMCG_MAX);
if (ret_limit_pool) {
- *ret_limit_pool = container_of(fail, struct dmem_cgroup_pool_state, cnt);
+ *ret_limit_pool = limit_pool;
css_get(&(*ret_limit_pool)->cs->css);
dmemcg_pool_get(*ret_limit_pool);
}
@@ -840,6 +874,25 @@ static int dmem_cgroup_region_max_show(struct seq_file *sf, void *v)
return dmemcg_limit_show(sf, v, get_resource_max);
}
+static int dmem_cgroup_region_events_show(struct seq_file *sf, void *v)
+{
+ struct dmemcg_state *dmemcs = css_to_dmemcs(seq_css(sf));
+ struct dmem_cgroup_region *region;
+
+ rcu_read_lock();
+ list_for_each_entry_rcu(region, &dmem_cgroup_regions, region_node) {
+ struct dmem_cgroup_pool_state *pool = find_cg_pool_locked(dmemcs, region);
+
+ seq_puts(sf, region->name);
+ seq_printf(sf, " low %ld max %ld\n",
+ dmemcg_get_event(pool, DMEMCG_LOW),
+ dmemcg_get_event(pool, DMEMCG_MAX));
+ }
+ rcu_read_unlock();
+
+ return 0;
+}
+
static ssize_t dmem_cgroup_region_max_write(struct kernfs_open_file *of,
char *buf, size_t nbytes, loff_t off)
{
@@ -874,6 +927,12 @@ static struct cftype files[] = {
.seq_show = dmem_cgroup_region_max_show,
.flags = CFTYPE_NOT_ON_ROOT,
},
+ {
+ .name = "events",
+ .seq_show = dmem_cgroup_region_events_show,
+ .file_offset = offsetof(struct dmemcg_state, events_file),
+ .flags = CFTYPE_NOT_ON_ROOT,
+ },
{ } /* Zero entry terminates. */
};
--
2.25.1
^ permalink raw reply related
* [PATCH 0/2] cgroup/dmem: add per-region event counters
From: Hongfu Li @ 2026-06-24 3:11 UTC (permalink / raw)
To: tj, hannes, mkoutny, corbet, skhan, dev, mripard, natalie.vock
Cc: cgroups, linux-doc, linux-kernel, dri-devel, Hongfu Li
This patch series adds event counters to the device memory (dmem) cgroup
controller.
The dmem controller exposes per-region limits and current usage, but
not how often those limits are hit. It is hard to tell whether failures
come from this cgroup, a parent limit, or pressure elsewhere in the
hierarchy.
To provide that visibility, this series introduces:
- dmem.events: reports hierarchical low/max counts per region.
- dmem.events.local: reports per-region low/max counts for this cgroup only.
Patch overview:
Patch 1/2:
- Add dmem.events with hierarchical low/max counters per region.
- Record dmem.max allocation failures and dmem.low protection events.
- Document the interface in cgroup-v2.rst.
Patch 2/2:
- Add dmem.events.local for local-only per-region counts.
- Share the events show logic between both files.
- Update cgroup-v2.rst accordingly.
Example output (dmem.events):
drm/0000:03:00.0/vram0 low 0 max 3
drm/0000:03:00.0/stolen low 0 max 0
low - reclaim/eviction considered the cgroup below its effective
dmem.low protection
max - allocation failed because the cgroup or an ancestor hit dmem.max
Both files exist for all non-root cgroups, like dmem.max and dmem.current.
These patches have been tested locally.
Hongfu Li (2):
cgroup/dmem: add per-region event counters
cgroup/dmem: introduce dmem.events.local for local counts
Documentation/admin-guide/cgroup-v2.rst | 17 +++++
kernel/cgroup/dmem.c | 85 ++++++++++++++++++++++++-
2 files changed, 101 insertions(+), 1 deletion(-)
--
2.25.1
^ permalink raw reply
* Re: [PATCH] crypto: af_alg - Add af_alg_restrict sysctl, defaulting to 1
From: Demi Marie Obenour @ 2026-06-24 2:09 UTC (permalink / raw)
To: Eric Biggers, Andy Lutomirski
Cc: linux-crypto, Herbert Xu, linux-kernel, linux-doc,
linux-bluetooth, iwd, linux-hardening, Milan Broz
In-Reply-To: <20260623192715.GE1850517@google.com>
[-- Attachment #1.1.1: Type: text/plain, Size: 2658 bytes --]
On 6/23/26 15:27, Eric Biggers wrote:
> On Tue, Jun 23, 2026 at 12:12:24PM -0700, Andy Lutomirski wrote:
>> On Mon, Jun 22, 2026 at 4:49 PM Eric Biggers <ebiggers@kernel.org> wrote:
>>>
>>> AF_ALG is a frequent source of vulnerabilities and a maintenance
>>> nightmare. It exposes far more functionality to userspace than ever
>>> should have been exposed, especially to unprivileged processes. Recent
>>> exploits have targeted kernel internal implementation details like
>>> "authencesn" that have zero use case for userspace access.
>>>
>>> Fortunately, AF_ALG is rarely used in practice, as userspace crypto
>>> libraries exist. And when it is used, only some functionality is known
>>> to be used, and many users are known to hold capabilities already.
>>> iwd for example requires CAP_NET_ADMIN and has a known algorithm list
>>> (https://lore.kernel.org/linux-crypto/bcbbef00-5881-421b-8892-7be6c04b832d@gmail.com/).
>>>
>>> Thus, let's restrict the set of allowed algorithms by default, depending
>>> on the capabilities held.
>>>
>>> Add a sysctl /proc/sys/crypto/af_alg_restrict with meaning:
>>>
>>> 0: unrestricted
>>> 1: limited functionality
>>> 2: completely disabled
>>>
>>> Set the default value to 1, which enables an algorithm allowlist for
>>> unprivileged processes and a slightly longer allowlist for privileged
>>> processes.
>>
>> In our brave new world of containers, this is a bit awkward. The
>> admin is sort of asking two separate questions:
>>
>> 1. Is the actual running distro and its privileged components capable
>> of working without AF_ALG or with only the parts marked as being
>> unprivileged?
>>
>> 2. Is the system running contains that need the unprivileged parts?
>> (Which is maybe just sha1 for ip? I really don't know.)
>>
>> Should there maybe be two separate options so that all options are
>> available? Or maybe something between 2 and 3 that means "limited
>> functionality and privileged modes are completely disabled"?
>
> If we want to offer more settings we could. I could see this getting
> quite complex pretty quickly once everyone weighs in, though. There's
> quite a bit of value in keeping things simple, even if the offered
> settings won't be optimal for every case.
>
> - Eric
What about exposing both allowlists to userspace and making them
configurable?
I'm mostly concerned about systems running code (possibly
closed-source) that uses algorithms that nobody here knows about.
It would be better to allow a single algorithm than to turn off all
restrictions.
--
Sincerely,
Demi Marie Obenour (she/her/hers)
[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 7253 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* [PATCH] Documentation: dev-tools: scripts/container prefers Podman
From: Coiby Xu @ 2026-06-24 1:38 UTC (permalink / raw)
To: linux-doc
Cc: Guillaume Tucker, Jonathan Corbet, Shuah Khan,
open list:DOCUMENTATION PROCESS, open list
Obviously scripts/container prefers Podman over Docker. Putting podman
before docker also makes it consistent with following parts of the doc
and the help text of the tool.
Signed-off-by: Coiby Xu <coiby.xu@gmail.com>
---
Documentation/dev-tools/container.rst | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/Documentation/dev-tools/container.rst b/Documentation/dev-tools/container.rst
index 452415b64662..9e23f79d5ae1 100644
--- a/Documentation/dev-tools/container.rst
+++ b/Documentation/dev-tools/container.rst
@@ -40,7 +40,7 @@ Available options:
``-r, --runtime RUNTIME``
- Container runtime name. Supported runtimes: ``docker``, ``podman``.
+ Container runtime name. Supported runtimes: ``podman``, ``docker``.
If not specified, the first one found on the system will be used
i.e. Podman if present, otherwise Docker.
@@ -75,8 +75,8 @@ working directory and adjust the user and group id as needed.
The container image which would typically include a compiler toolchain is
provided by the user and selected via the ``-i`` option. The container runtime
-can be selected with the ``-r`` option, which can be either ``docker`` or
-``podman``. If none is specified, the first one found on the system will be
+can be selected with the ``-r`` option, which can be either ``podman`` or
+``docker``. If none is specified, the first one found on the system will be
used while giving priority to Podman. Support for other runtimes may be added
later depending on their popularity among users.
--
2.54.0
^ permalink raw reply related
* Re: [PATCH v2] usbcore: Add quirk for 255-bytes initial config read
From: Alan Stern @ 2026-06-24 1:35 UTC (permalink / raw)
To: Nikhil Solanke
Cc: linux-usb, gregkh, linux-kernel, michal.pecio, stable, corbet,
skhan, linux-doc
In-Reply-To: <CAFgddhJk0EYG71fnKdio=RHC-cH+JmL-EZ7-oVD-LdHoa2TBSA@mail.gmail.com>
On Wed, Jun 24, 2026 at 02:44:07AM +0530, Nikhil Solanke wrote:
> > Moving this delay up here changes the behavior when the quirk flag isn't
> > set. While it agrees with the intention of the USB_QUIRK_DELAY_INIT
> > flag, such a change should be mentioned in the patch description.
>
> How should I mention it then? Nothing comes to mind besides the
> obvious: "Also move the USB_QUIRK_DELAY_INIT sleep to before the
> initial descriptor read, so the delay applies consistently regardless
> of whether USB_QUIRK_CONFIG_SIZE is set.". Or should i revert it back
> to original position?
Actually, the best approach here would be to put this single change into
a separate patch that comes before the current one. That removes issues
of making more than one functional change in one patch and improves
bisectability.
But to answer your question: In general, a patch's description should
explain the reasons for the changes that the patch makes. Especially
when a particular change doesn't appear, at first glance, to be related
to the patch's primary purpose. (On the other hand, it doesn't need to
explain in detail what the patch does; we can see that for ourselves
just by reading the patch's contents.)
> > > + /*
> > > + * Grab just the first descriptor so we know how long the whole
> > > + * configuration is. In case of quirky firmware, try to grab the
> > > + * whole thing in one go by asking for a 255-bytes sized buffer
> > > + * mirroring Windows behavior.
> > > + */
> >
> > This needs to be rewritten, as it is self-contradictory. When the quirk
> > flag is set we issue a 255-byte request to mimic the Windows behavior,
> > and only when the flag isn't set do we grab just the first descriptor.
>
> I am sorry I didn't understand how it is self contradictory. The
> comment does say, "in case of quirky firmware..."? Am i missing
> something?
Literally, what the comment says is: Grab just the first descriptor,
and if the quirk flag is set, get all the descriptors. That's a
contradiction -- you can get just the first, or you can get all of
them, but you can't do both at the same time!
> > > result = usb_get_descriptor(dev, USB_DT_CONFIG, cfgno,
> > > - desc, USB_DT_CONFIG_SIZE);
> > > + desc, usb_config_req_size);
> >
> > Don't make extraneous changes to the existing indentation (or whitespace
> > in general), here and below.
>
> Well the linux coding style guidelines mention that those descendants
> should preferably be aligned with the function open parenthesis. Since
> i did "touch" that line/part of code I though might as well indent it
> a bit accordingly. Should i revert the indent then (in this and the
> other place)?
The style used in this file is to indent continuation lines by 4 spaces,
because some of the continued statements are extremely long. If you
want to align new continuation lines with an open paren, you can -- but
you didn't even do that in the example above; you aligned it with the
space following the first comma.
And while you did change some nearby code, you did not change the code
in this line. So reformatting it is not justified.
> > > if (result != -EPIPE)
> > > goto err;
> > > dev_notice(ddev, "chopping to %d config(s)\n", cfgno);
> > > @@ -957,13 +976,25 @@ int usb_get_configuration(struct usb_device *dev)
> > > break;
> > > } else if (result < 4) {
> > > dev_err(ddev, "config index %d descriptor too short "
> > > - "(expected %i, got %i)\n", cfgno,
> > > - USB_DT_CONFIG_SIZE, result);
> > > + "(asked for %zu, got %i, expected at least %i)\n",
> > > + cfgno, usb_config_req_size, result, 4);
> > > result = -EINVAL;
> > > goto err;
> > > }
> > > +
> > > length = max_t(int, le16_to_cpu(desc->wTotalLength),
> > > - USB_DT_CONFIG_SIZE);
> > > + USB_DT_CONFIG_SIZE);
> >
> > This is another example of a change that has nothing to do with the
> > purpose of the patch.
>
> Isn't that what you told me to change? So the logs are accurate? I
> made that change because you suggested it. :')
My comment referred to the two lines directly above it, and I did not
suggest leaving the code exactly the same except for indenting it
farther. Or inserting an extra blank line just before the assignment to
length.
Alan Stern
^ permalink raw reply
* Re: [PATCH v7 03/10] tracing/probes: Support dumping fetcharg program for debugging dynamic events
From: Masami Hiramatsu @ 2026-06-24 0:18 UTC (permalink / raw)
To: Julian Braha
Cc: Steven Rostedt, Mathieu Desnoyers, Jonathan Corbet, Shuah Khan,
linux-kernel, linux-trace-kernel, linux-doc, linux-kselftest
In-Reply-To: <96b043ed-c527-4e5d-8eb7-631805da53fd@gmail.com>
On Tue, 23 Jun 2026 19:31:47 +0100
Julian Braha <julianbraha@gmail.com> wrote:
> Hi Masami,
>
> On 6/23/26 02:44, Masami Hiramatsu (Google) wrote:
>
> > +config PROBE_EVENTS_DUMP_FETCHARG
> > + depends on PROBE_EVENTS
> > + bool "Dump of dynamic probe event fetch-arguments"
> > + default n
>
> Sorry, kconfig nitpick: could you match the style used by the rest of
> the config options in this file? E.g. the type and prompt come first in
> the list of attributes?
Ah, good catch! Let me fix it.
Thanks,
>
> - Julian Braha
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply
* Re: [PATCH v8 05/46] KVM: Make CONFIG_KVM_VM_MEMORY_ATTRIBUTES selectable
From: Ackerley Tng @ 2026-06-24 0:14 UTC (permalink / raw)
To: Sean Christopherson, Julian Braha
Cc: aik, andrew.jones, binbin.wu, brauner, chao.p.peng, david,
jmattson, jthoughton, michael.roth, oupton, pankaj.gupta, qperret,
rick.p.edgecombe, rientjes, shivankg, steven.price, tabba, willy,
wyihan, yan.y.zhao, forkloop, pratyush, suzuki.poulose,
aneesh.kumar, liam, Paolo Bonzini, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, H. Peter Anvin, Steven Rostedt,
Masami Hiramatsu, Mathieu Desnoyers, Jonathan Corbet, Shuah Khan,
Shuah Khan, Vishal Annapurve, Andrew Morton, Chris Li,
Kairui Song, Kemeng Shi, Nhat Pham, Barry Song, Axel Rasmussen,
Yuanchu Xie, Wei Xu, Youngjun Park, Qi Zheng, Shakeel Butt,
Kiryl Shutsemau, Baoquan He, Jason Gunthorpe, Vlastimil Babka,
kvm, linux-kernel, linux-trace-kernel, linux-doc, linux-kselftest,
linux-mm, linux-coco
In-Reply-To: <ajnQVkLvFl_lMuGB@google.com>
Sean Christopherson <seanjc@google.com> writes:
> On Fri, Jun 19, 2026, Julian Braha wrote:
>> Hi Ackerley,
>>
>> On 6/19/26 01:31, Ackerley Tng via B4 Relay wrote:
>>
>> > config KVM_VM_MEMORY_ATTRIBUTES
>> > - bool
>> > + depends on KVM_SW_PROTECTED_VM || KVM_INTEL_TDX || KVM_AMD_SEV
>> > + bool "Enable per-VM PRIVATE vs. SHARED attributes (for CoCo VMs)"
>>
>> Sorry for the style nitpick, but could you keep the type and prompt as
>> the first attribute in the Kconfig option definition (like the other
>> options do)?
>
> No need to be sorry, I've no idea why I put the "depends" first. I don't even
> know if that qualifies as a nit :-)
>
> Ackerley, if you can provide your SoB (for Fuad's feedback), I can fixup when
> applying (assuming nothing else necessitates v9).
Thanks, didn't notice this when consolidating this revision.
Signed-off-by: Ackerley Tng <ackerleytng@google.com>
^ permalink raw reply
* Re: [RFC PATCH 0/6] mm/damon: hardware-sampled access reports
From: SeongJae Park @ 2026-06-24 0:14 UTC (permalink / raw)
To: Zeng Heng
Cc: SeongJae Park, Ravi Jonnalagadda, akinobu.mita, damon, linux-mm,
linux-kernel, linux-doc, akpm, corbet, bijan311, ajayjoshi,
honggyu.kim, yunjeong.mun
In-Reply-To: <7d46a48c-8805-09e1-4818-807953898fb4@huawei.com>
Hello Zeng,
On Tue, 23 Jun 2026 22:08:03 +0800 Zeng Heng <zengheng4@huawei.com> wrote:
> Hi Ravi,
>
> On 2026/5/30 0:56, Ravi Jonnalagadda wrote:
> > This series introduces a vendor and PMU-agnostic substrate inside DAMON
> > that consumes hardware-sampled access reports through the standard
> > perf-event interface. Userspace selects the PMU through sysfs (raw
> > type/config knobs), driving either Intel PEBS L3-miss sampling or AMD
> > IBS Op sampling.
> >
>
> [...]
>
> >
> > Ravi Jonnalagadda (6):
> > mm/damon: add struct damon_perf_event{,_attr} and per-ctx perf_events
> > list
> > mm/damon/sysfs-sample: expose perf_events configuration via sysfs
> > mm/damon/sysfs: install perf_events on apply
> > mm/damon/core: per-CPU SPSC ring drain and damon_perf_event lifecycle
> > mm/damon/vaddr: implement perf-event access check
> > mm/damon: add damos_node_eligible_mem_bp tracepoint
> >
> > include/linux/damon.h | 80 +++++
> > include/trace/events/damon.h | 49 +++
> > mm/damon/core.c | 403 ++++++++++++++++++++----
> > mm/damon/ops-common.h | 39 +++
> > mm/damon/sysfs-common.h | 6 +
> > mm/damon/sysfs-sample.c | 579 +++++++++++++++++++++++++++++++++++
> > mm/damon/sysfs.c | 3 +
> > mm/damon/vaddr.c | 267 ++++++++++++++++
> > 8 files changed, 1370 insertions(+), 56 deletions(-)
> >
> >
> > base-commit: 4c8ad15abf15eb480d3ad85f902001e35465ef18
>
> I wasn't able to apply this patch series to the linux (and linux-next)
> mainline branch, and also had trouble identifying the source of the base
> commit.
>
> Would you mind sharing where this baseline is from?
TLDR: I pushed [1] a tree having this series applied on top of the baseline to
GitHub. Please feel free to use it.
I think the baseline was a commit on damon/next tree [2]. Because damon/next
is continuously rebased, we cannot get the commit in a simple way. Fortunately
the commit is still available on my local tree. So I applied this patch series
on top of the commit and pushed [1] to a branch of DAMON kernel tree at GitHub.
Note that the branch is not guaranteed to exist there for long term. But
hopefully this series will be merged into the mainline before that.
[1] https://github.com/damonitor/linux/tree/ravi_hw_sampled_access_reports_rfc_v1
[2] https://origin.kernel.org/doc/html/latest/mm/damon/maintainer-profile.html#scm-trees
Thanks,
SJ
[...]
^ permalink raw reply
* Re: [PATCH v8 04/46] KVM: Decouple kvm_has_arch_private_mem from CONFIG_KVM_VM_MEMORY_ATTRIBUTES
From: Ackerley Tng @ 2026-06-24 0:13 UTC (permalink / raw)
To: Binbin Wu
Cc: aik, andrew.jones, brauner, chao.p.peng, david, jmattson,
jthoughton, michael.roth, oupton, pankaj.gupta, qperret,
rick.p.edgecombe, rientjes, shivankg, steven.price, tabba, willy,
wyihan, yan.y.zhao, forkloop, pratyush, suzuki.poulose,
aneesh.kumar, liam, Paolo Bonzini, Sean Christopherson,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Steven Rostedt, Masami Hiramatsu,
Mathieu Desnoyers, Jonathan Corbet, Shuah Khan, Shuah Khan,
Vishal Annapurve, Andrew Morton, Chris Li, Kairui Song,
Kemeng Shi, Nhat Pham, Barry Song, Axel Rasmussen, Yuanchu Xie,
Wei Xu, Youngjun Park, Qi Zheng, Shakeel Butt, Kiryl Shutsemau,
Baoquan He, Jason Gunthorpe, Vlastimil Babka, kvm, linux-kernel,
linux-trace-kernel, linux-doc, linux-kselftest, linux-mm,
linux-coco
In-Reply-To: <a21bfc05-787e-4cd8-89af-8579357e6a12@linux.intel.com>
Binbin Wu <binbin.wu@linux.intel.com> writes:
> On 6/19/2026 8:31 AM, Ackerley Tng via B4 Relay wrote:
>> From: Sean Christopherson <seanjc@google.com>
>>
>> When memory attributes become trackable in guest_memfd, the concept of
>> having private memory is no longer dependent on
>> CONFIG_KVM_VM_MEMORY_ATTRIBUTES.
>>
>> With this, on x86, kvm_arch_has_private_mem() is defined if some CoCo
>> platform support (or the testing CONFIG_KVM_SW_PROTECTED_VM) is compiled
>> in.
>>
>> Signed-off-by: Sean Christopherson <seanjc@google.com>
>> Co-developed-by: Ackerley Tng <ackerleytng@google.com>
>> Signed-off-by: Ackerley Tng <ackerleytng@google.com>
>
> Reviewed-by: Binbin Wu <binbin.wu@linux.intel.com>
>
> One nit below.
>
>> ---
>> arch/x86/include/asm/kvm_host.h | 4 +++-
>> include/linux/kvm_host.h | 2 +-
>> 2 files changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
>> index 8e8eb8a5e8a6b..1bde67cf6eb0e 100644
>> --- a/arch/x86/include/asm/kvm_host.h
>> +++ b/arch/x86/include/asm/kvm_host.h
>> @@ -2394,7 +2394,9 @@ void kvm_configure_mmu(bool enable_tdp, int tdp_forced_root_level,
>> int tdp_max_root_level, int tdp_huge_page_level);
>>
>>
>> -#ifdef CONFIG_KVM_VM_MEMORY_ATTRIBUTES
>> +#if defined(CONFIG_KVM_SW_PROTECTED_VM) || \
>> + defined(CONFIG_KVM_INTEL_TDX) || \
>> + defined(CONFIG_KVM_AMD_SEV)
>
> Nit:
> Vertically align the defined(XXX) statements for better readability?
>
Sean had this aligned with spaces, and checkpatch complained about
having no spaces before tabs, so I switched it to tabs instead since I
don't think alignment like that is officially documented either way.
Either way is fine :)
>> #define kvm_arch_has_private_mem(kvm) ((kvm)->arch.has_private_mem)
>> #endif
>>
>> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
>> index 201d0f2143976..d370e834d619e 100644
>> --- a/include/linux/kvm_host.h
>> +++ b/include/linux/kvm_host.h
>> @@ -722,7 +722,7 @@ static inline int kvm_arch_vcpu_memslots_id(struct kvm_vcpu *vcpu)
>> }
>> #endif
>>
>> -#ifndef CONFIG_KVM_VM_MEMORY_ATTRIBUTES
>> +#ifndef kvm_arch_has_private_mem
>> static inline bool kvm_arch_has_private_mem(struct kvm *kvm)
>> {
>> return false;
>>
^ permalink raw reply
* Re: [PATCH v8 01/46] KVM: guest_memfd: Introduce per-gmem attributes, use to guard user mappings
From: Ackerley Tng @ 2026-06-24 0:09 UTC (permalink / raw)
To: Sean Christopherson, Binbin Wu
Cc: aik, andrew.jones, brauner, chao.p.peng, david, jmattson,
jthoughton, michael.roth, oupton, pankaj.gupta, qperret,
rick.p.edgecombe, rientjes, shivankg, steven.price, tabba, willy,
wyihan, yan.y.zhao, forkloop, pratyush, suzuki.poulose,
aneesh.kumar, liam, Paolo Bonzini, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, H. Peter Anvin, Steven Rostedt,
Masami Hiramatsu, Mathieu Desnoyers, Jonathan Corbet, Shuah Khan,
Shuah Khan, Vishal Annapurve, Andrew Morton, Chris Li,
Kairui Song, Kemeng Shi, Nhat Pham, Barry Song, Axel Rasmussen,
Yuanchu Xie, Wei Xu, Youngjun Park, Qi Zheng, Shakeel Butt,
Kiryl Shutsemau, Baoquan He, Jason Gunthorpe, Vlastimil Babka,
kvm, linux-kernel, linux-trace-kernel, linux-doc, linux-kselftest,
linux-mm, linux-coco
In-Reply-To: <ajnjTJdQKD1Kz3tf@google.com>
Sean Christopherson <seanjc@google.com> writes:
> On Mon, Jun 22, 2026, Binbin Wu wrote:
>> On 6/19/2026 8:31 AM, Ackerley Tng via B4 Relay wrote:
>>
>> [...]
>>
>> >
>> > +static u64 kvm_gmem_get_attributes(struct inode *inode, pgoff_t index)
>> > +{
>> > + struct maple_tree *mt = &GMEM_I(inode)->attributes;
>> > + void *entry = mtree_load(mt, index);
>> > +
>> > + return WARN_ON_ONCE(!entry) ? 0 : xa_to_value(entry);
>>
>> If the entry is unexpectedly missing, returning 0 means the attribute would
>> be treated as shared. And then in kvm_gmem_fault_user_mapping(), it would
>> allow the userspace to fault in the folio.
>>
>> Should gmem deny such edge case?
>
> After several bugs this year where a WARN_ON_ONCE() fired, but was entirely
> insufficient to prevent true badness, I'm definitely senstive to making the "bad"
> behavior as harmless as possible.
>
I guess both are indeed awkward.
> However, in this case I think we're just hosed. If KVM treats the memory as
> private, KVM will incorrectly do prepare(), incorrectly allow populate(), and
> will caused missed invalidations (though I suppose __kvm_gmem_set_attributes()
> "only" lies to userspace in that case).
>
> That said, assuming SHARED is definitely odd for cases where guest_memfd *can't*
> hold shared memory. Ditto for assuming PRIVATE. What if we instead fall back to
> the "init" state, e.g.?
>
> static u64 kvm_gmem_get_attributes(struct inode *inode, pgoff_t index)
> {
> struct maple_tree *mt = &GMEM_I(inode)->attributes;
> void *entry = mtree_load(mt, index);
>
> if (WARN_ON_ONCE(!entry)) {
> bool shared = GMEM_I(inode)->flags & GUEST_MEMFD_FLAG_INIT_SHARED;
>
> return shared ? 0 : KVM_MEMORY_ATTRIBUTE_PRIVATE;
I was wondering if we should not only return the init state but also set
the init state, but that would involve performing a conversion to the
init state... Too complicated for an edge case.
> }
>
> return xa_to_value(entry);
> }
Thanks Binbin and Sean!
^ permalink raw reply
* RE: [PATCH 3/3] hwmon: (pmbus/fd5121): Add support FD5121, FD5123 and FD5125
From: Selvamani Rajagopal @ 2026-06-23 23:12 UTC (permalink / raw)
To: Guenter Roeck, Jonathan Corbet, Shuah Khan, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: linux-hwmon@vger.kernel.org, linux-doc@vger.kernel.org,
linux-kernel@vger.kernel.org, devicetree@vger.kernel.org
In-Reply-To: <320c44c9-924f-4b7e-a46a-37a72fa7267f@roeck-us.net>
> -----Original Message-----
> From: Guenter Roeck <groeck7@gmail.com> On Behalf Of Guenter Roeck
> Subject: Re: [PATCH 3/3] hwmon: (pmbus/fd5121): Add support FD5121, FD5123 and
> FD5125
>
>
> > + &sensor_dev_attr_vout_transition_rate.dev_attr.attr,
> > + NULL
>
> This is a complete no-go. We do not explose raw register data as sysfs
> attributes. You may expose essential register data as debugfs files,
> but only those deemed necessary. The above is just "let's blindly
> expose everything". Most of the above should be programmed in manufacturing
> and not be touched subsequently, much less as writeable attributes.
> Writing bad/unexpected values into many of those attributes can turn
> a board into a brick. It is bad enough that/if this is even possible,
> but exposing it as sysfs attribute would be a terrible idea.
>
> I am not going to review this driver any further at this point.
>
> Guenter
>
Thanks for your comment. Understood and agree on comment on exposing everything without enforcing what is being written/read.
We will discuss internally to see how to go about it. We need some custom registers for customers. We will identify those and
expose under debugfs, if needed.
> >
^ permalink raw reply
* [PATCH v2 2/2] cgroup/cpuset: Rebind/migrate mm only for threadgroup leader in cpuset_update_tasks_nodemask()
From: Waiman Long @ 2026-06-23 23:04 UTC (permalink / raw)
To: Tejun Heo, Johannes Weiner, Michal Koutný, Ridong Chen,
Jonathan Corbet, Shuah Khan
Cc: cgroups, linux-kernel, linux-doc, linux-kselftest, Waiman Long
In-Reply-To: <20260623230413.1984188-1-longman@redhat.com>
As reported by sashiko [1], cpuset_update_tasks_nodemask() will do
mpol_rebind_mm() and possibly cpuset_migrate_mm() for all threads of
a multithreaded process. Since commit 3df9ca0a2b8b ("cpuset: migrate
memory only for threadgroup leaders"), cpuset_attach() had been updated
to rebind and migrate memory only for threadgroup leaders to mark the
group leader as the owner of the mm_struct.
To be consistent and avoid unnecessary performance overhead for heavily
multithreaded processes, follow the cpuset_attach() example and perform
memory rebind and migration only for threadgroup leaders.
Also add a paragraph in cgroup-v2.rst under cpuset.mems that the
threadgroup leader is the memory owner of that threadgroup. Therefore
the non-leading threads shouldn't be in other cgroups whose "cpuset.mems"
doesn't fully overlap that of the group leader.
[1] https://sashiko.dev/#/patchset/20260621032816.1806773-1-longman%40redhat.com
Signed-off-by: Waiman Long <longman@redhat.com>
Reviewed-by: Ridong Chen <ridong.chen@linux.dev>
---
Documentation/admin-guide/cgroup-v2.rst | 7 +++++++
kernel/cgroup/cpuset.c | 4 ++++
2 files changed, 11 insertions(+)
diff --git a/Documentation/admin-guide/cgroup-v2.rst b/Documentation/admin-guide/cgroup-v2.rst
index 993446ab66d0..f9c353174a7e 100644
--- a/Documentation/admin-guide/cgroup-v2.rst
+++ b/Documentation/admin-guide/cgroup-v2.rst
@@ -2527,6 +2527,13 @@ Cpuset Interface Files
a need to change "cpuset.mems" with active tasks, it shouldn't
be done frequently.
+ For a multithreaded process, the threadgroup leader is
+ considered the owner of the group's memory. Memory policy
+ rebinding and migration will only happen with respect to the
+ threadgroup leader. To avoid unexpected result, non-leading
+ threads shouldn't be put into another cgroup whose "cpuset.mems"
+ doesn't fully overlap that of the threadgroup leader.
+
cpuset.mems.effective
A read-only multiple values file which exists on all
cpuset-enabled cgroups.
diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index 044ddbf66f8e..055ae54a040a 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -2673,6 +2673,10 @@ void cpuset_update_tasks_nodemask(struct cpuset *cs)
cpuset_change_task_nodemask(task, &newmems);
+ /* Rebind and migrate mm only for thread group leader */
+ if (!thread_group_leader(task))
+ continue;
+
mm = get_task_mm(task);
if (!mm)
continue;
--
2.54.0
^ permalink raw reply related
* [PATCH v2 1/2] cgroup/cpuset: Avoid unnecessary cpus & mems update in cpuset_hotplug_update_tasks()
From: Waiman Long @ 2026-06-23 23:04 UTC (permalink / raw)
To: Tejun Heo, Johannes Weiner, Michal Koutný, Ridong Chen,
Jonathan Corbet, Shuah Khan
Cc: cgroups, linux-kernel, linux-doc, linux-kselftest, Waiman Long
In-Reply-To: <20260623230413.1984188-1-longman@redhat.com>
As reported by sashiko [1], cpuset_hotplug_update_tasks() may perform
unnecessary task iteration and updating of tasks' CPU and node masks
when mems_allowed and/or cpus_allowed are not set in cpuset v2. It is
due to the fact that the temporary new_cpus and new_mems masks do not
inherit parent's effective_cpus/mems when they are empty which is the
expected behavior for cpuset v2 since commit 4ec22e9c5a90 ("cpuset:
Enable cpuset controller in default hierarchy").
Fix that and avoid unnecessay work by enhancing
compute_effective_cpumask() to add the empty cpumask check
and inheriting the parent's versions if empty when in v2. A new
compute_effective_nodemask() helper is also added to perform similar
function for new effective_mems.
Add new test_cpuset_prs.sh test cases to confirm that effective_cpus
will inherit the parent's version if cpuset.cpus is empty.
[1] https://sashiko.dev/#/patchset/20260621032816.1806773-1-longman%40redhat.com
Suggested-by: Ridong Chen <ridong.chen@linux.dev>
Fixes: 4ec22e9c5a90 ("cpuset: Enable cpuset controller in default hierarchy")
Signed-off-by: Waiman Long <longman@redhat.com>
---
kernel/cgroup/cpuset.c | 45 +++++++++++--------
.../selftests/cgroup/test_cpuset_prs.sh | 11 ++++-
2 files changed, 35 insertions(+), 21 deletions(-)
diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index aff86acea701..044ddbf66f8e 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -1094,12 +1094,35 @@ void cpuset_update_tasks_cpumask(struct cpuset *cs, struct cpumask *new_cpus)
* @cs: the cpuset the need to recompute the new effective_cpus mask
* @parent: the parent cpuset
*
+ * For v2, the parent's effective_cpus is inherited if cpumask empty.
* The result is valid only if the given cpuset isn't a partition root.
*/
static void compute_effective_cpumask(struct cpumask *new_cpus,
struct cpuset *cs, struct cpuset *parent)
{
- cpumask_and(new_cpus, cs->cpus_allowed, parent->effective_cpus);
+ bool has_cpus;
+
+ has_cpus = cpumask_and(new_cpus, cs->cpus_allowed, parent->effective_cpus);
+ if (!has_cpus && is_in_v2_mode())
+ cpumask_copy(new_cpus, parent->effective_cpus);
+}
+
+/**
+ * compute_effective_nodemask - Compute the effective nodemask of the cpuset
+ * @new_cpus: the temp variable for the new effective_mems mask
+ * @cs: the cpuset the need to recompute the new effective_mems mask
+ * @parent: the parent cpuset
+ *
+ * For v2, the parent's effective_mems is inherited if nodemask empty.
+ */
+static void compute_effective_nodemask(nodemask_t *new_mems,
+ struct cpuset *cs, struct cpuset *parent)
+{
+ bool has_mems;
+
+ has_mems = nodes_and(*new_mems, cs->mems_allowed, parent->effective_mems);
+ if (!has_mems && is_in_v2_mode())
+ nodes_copy(*new_mems, parent->effective_mems);
}
/*
@@ -2148,15 +2171,6 @@ static void update_cpumasks_hier(struct cpuset *cs, struct tmpmasks *tmp,
goto update_parent_effective;
}
- /*
- * If it becomes empty, inherit the effective mask of the
- * parent, which is guaranteed to have some CPUs unless
- * it is a partition root that has explicitly distributed
- * out all its CPUs.
- */
- if (is_in_v2_mode() && !remote && cpumask_empty(tmp->new_cpus))
- cpumask_copy(tmp->new_cpus, parent->effective_cpus);
-
/*
* Skip the whole subtree if
* 1) the cpumask remains the same,
@@ -2704,14 +2718,7 @@ static void update_nodemasks_hier(struct cpuset *cs, nodemask_t *new_mems)
cpuset_for_each_descendant_pre(cp, pos_css, cs) {
struct cpuset *parent = parent_cs(cp);
- bool has_mems = nodes_and(*new_mems, cp->mems_allowed, parent->effective_mems);
-
- /*
- * If it becomes empty, inherit the effective mask of the
- * parent, which is guaranteed to have some MEMs.
- */
- if (is_in_v2_mode() && !has_mems)
- *new_mems = parent->effective_mems;
+ compute_effective_nodemask(new_mems, cp, parent);
/* Skip the whole subtree if the nodemask remains the same. */
if (nodes_equal(*new_mems, cp->effective_mems)) {
@@ -3923,7 +3930,7 @@ static void cpuset_hotplug_update_tasks(struct cpuset *cs, struct tmpmasks *tmp)
parent = parent_cs(cs);
compute_effective_cpumask(&new_cpus, cs, parent);
- nodes_and(new_mems, cs->mems_allowed, parent->effective_mems);
+ compute_effective_nodemask(&new_mems, cs, parent);
if (!tmp || !cs->partition_root_state)
goto update_tasks;
diff --git a/tools/testing/selftests/cgroup/test_cpuset_prs.sh b/tools/testing/selftests/cgroup/test_cpuset_prs.sh
index 0d41aa0d343d..ca9bc38fdb95 100755
--- a/tools/testing/selftests/cgroup/test_cpuset_prs.sh
+++ b/tools/testing/selftests/cgroup/test_cpuset_prs.sh
@@ -495,13 +495,20 @@ REMOTE_TEST_MATRIX=(
# Narrowing cpuset.cpus to previously sibling-excluded CPUs should
# not return CPUs that were never actually owned.
" C1-4:P1 . C1-2:P1 C1-3:P2 . . \
- . . . C3 . . p1:4|c11:1-2|c12:3 \
+ . . . C3 . . p1:4|c11:1-2|c12:3 \
p1:P1|c11:P1|c12:P2 3"
# Expanding cpuset.cpus to include a previously sibling-excluded CPU
# after the sibling has become a member should correctly request it.
" C1-4:P1 . C1-2:P1 C1-3:P2 . . \
- . . P0 C2-3 . . p1:1,4|c11:1|c12:2-3 \
+ . . P0 C2-3 . . p1:1,4|c11:1|c12:2-3 \
p1:P1|c11:P0|c12:P2 2-3"
+ # Cpusets with empty cpuset.cpus should inherit parent's effective_cpus
+ " C1-4:P1 C5-6 C1-2 . C5 . \
+ . P1 P1 . . . p1:3-4|p2:5-6|c11:1-2|c12:3-4|c21:5|c22:5-6 \
+ p1:P1|p2:P1|c11:P1"
+ " C1-4:P1 C5-6 C1-2 . C5 . \
+ . P1 P1 . O5=0 . p1:3-4|p2:6|c11:1-2|c12:3-4|c21:6|c22:6 \
+ p1:P1|p2:P1|c11:P1"
)
#
--
2.54.0
^ permalink raw reply related
* [PATCH v2 0/2] cgroup/cpuset: Miscellaneous fixes and cleanups
From: Waiman Long @ 2026-06-23 23:04 UTC (permalink / raw)
To: Tejun Heo, Johannes Weiner, Michal Koutný, Ridong Chen,
Jonathan Corbet, Shuah Khan
Cc: cgroups, linux-kernel, linux-doc, linux-kselftest, Waiman Long
v2:
- Update patch 1 as suggested by Ridong Chen and add new test cases.
- Minor update to patch 2 code and comment log.
Patch 1 updates compute_effective_cpumask() and adds new
compute_effective_nodemask() helper to make sure that effective_cpus
and effective_mems will inherit parent's versions for v2 if
cpuset.cpus/cpuset.mems is empty.
Patch 2 makes cpuset_update_tasks_nodemask() to perform memory rebind
and migration only for thread group leader like cpuset_attach().
Waiman Long (2):
cgroup/cpuset: Avoid unnecessary cpus & mems update in
cpuset_hotplug_update_tasks()
cgroup/cpuset: Rebind/migrate mm only for threadgroup leader in
cpuset_update_tasks_nodemask()
Documentation/admin-guide/cgroup-v2.rst | 7 +++
kernel/cgroup/cpuset.c | 49 ++++++++++++-------
.../selftests/cgroup/test_cpuset_prs.sh | 11 ++++-
3 files changed, 46 insertions(+), 21 deletions(-)
--
2.54.0
^ permalink raw reply
* Re: [PATCH 3/3] hwmon: (pmbus/fd5121): Add support FD5121, FD5123 and FD5125
From: Guenter Roeck @ 2026-06-23 22:07 UTC (permalink / raw)
To: Selvamani.Rajagopal, Jonathan Corbet, Shuah Khan, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: linux-hwmon, linux-doc, linux-kernel, devicetree
In-Reply-To: <20260622-support-fd5121-from-onsemi-v1-3-b31767689c65@onsemi.com>
On 6/22/26 22:55, Selvamani Rajagopal via B4 Relay wrote:
> From: Selvamani Rajagopal <Selvamani.Rajagopal@onsemi.com>
>
> FD5121 is a dual-rail, multi-phase, digital controller that offers
> full telemtry options including input/output voltage, current as
> well as fault handling and identifications.
>
> These controllers are compliant with PMBus specification.
>
> Signed-off-by: Selvamani Rajagopal <Selvamani.Rajagopal@onsemi.com>
> ---
> MAINTAINERS | 8 +
> drivers/hwmon/pmbus/Kconfig | 9 +
> drivers/hwmon/pmbus/Makefile | 1 +
> drivers/hwmon/pmbus/fd5121.c | 1004 ++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 1022 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index d95d3ef77773..c0664c33324a 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -20135,6 +20135,14 @@ L: linux-mips@vger.kernel.org
> S: Maintained
> F: arch/mips/boot/dts/ralink/omega2p.dts
>
> +ONSEMI HARDWARE MONITOR DRIVER
> +M: Selva Rajagopal <selvamani.rajagopal@onsemi.com>
> +L: linux-hwmon@vger.kernel.org
> +S: Supported
> +W: https://www.onsemi.com
> +F: Documentation/devicetree/bindings/hwmon/pmbus/onnn,fd5121.yaml
> +F: drivers/hwmon/pmbus/fd5121.c
> +
> ONSEMI ETHERNET PHY DRIVERS
> M: Piergiorgio Beruto <piergiorgio.beruto@gmail.com>
> L: netdev@vger.kernel.org
> diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
> index c8cda160b5f8..3a06ed83539e 100644
> --- a/drivers/hwmon/pmbus/Kconfig
> +++ b/drivers/hwmon/pmbus/Kconfig
> @@ -179,6 +179,15 @@ config SENSORS_E50SN12051
> This driver can also be built as a module. If so, the module will
> be called e50sn12051.
>
> +config SENSORS_FD5121
> + tristate "FD5121/FD5123/FD5125 controllers from onsemi"
> + help
> + If you say yes here, you get support for onsemi
> + controllers FD5121, FD5123, FD5125.
> +
> + This driver can also be built as a module. If so, the module will
> + be called fd5121.
> +
> config SENSORS_INA233
> tristate "Texas Instruments INA233 and compatibles"
> help
> diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile
> index ffc05f493213..70f4afb41fe0 100644
> --- a/drivers/hwmon/pmbus/Makefile
> +++ b/drivers/hwmon/pmbus/Makefile
> @@ -13,6 +13,7 @@ obj-$(CONFIG_SENSORS_APS_379) += aps-379.o
> obj-$(CONFIG_SENSORS_BEL_PFE) += bel-pfe.o
> obj-$(CONFIG_SENSORS_BPA_RS600) += bpa-rs600.o
> obj-$(CONFIG_SENSORS_DELTA_AHE50DC_FAN) += delta-ahe50dc-fan.o
> +obj-$(CONFIG_SENSORS_FD5121) += fd5121.o
> obj-$(CONFIG_SENSORS_FSP_3Y) += fsp-3y.o
> obj-$(CONFIG_SENSORS_HAC300S) += hac300s.o
> obj-$(CONFIG_SENSORS_IBM_CFFPS) += ibm-cffps.o
> diff --git a/drivers/hwmon/pmbus/fd5121.c b/drivers/hwmon/pmbus/fd5121.c
> new file mode 100644
> index 000000000000..e68c6d6cabbd
> --- /dev/null
> +++ b/drivers/hwmon/pmbus/fd5121.c
> @@ -0,0 +1,1004 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright 2026 Semiconductor Components Industries, LLC ("onsemi").
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/err.h>
> +#include <linux/slab.h>
> +#include <linux/delay.h>
> +#include <linux/i2c.h>
> +#include <linux/hwmon.h>
> +#include <linux/hwmon-sysfs.h>
> +#include <linux/unaligned.h>
> +
> +#include "pmbus.h"
> +
> +enum chips { chip_fd5121, chip_fd5123, chip_fd5125 };
> +
> +#define CTLR_ID_UNKNOWN 0
> +#define CTLR_ID_FD5121 0xFD5121
> +#define CTLR_ID_FD5123 0xFD5123
> +#define CTLR_ID_FD5125 0xFD5125
> +
> +#define FD5121_NUM_PAGES 2
> +
> +/* Custom PMBUS commands */
> +#define PMBUS_REG_VOUT_MIN 0x2B
> +#define PMBUS_REG_POWER_MODE 0x34
> +#define PMBUS_REG_VIN_ON 0x35
> +#define PMBUS_REG_VIN_OFF 0x36
> +#define PMBUS_REG_VIN_UV_FAULT_RESPONSE 0x5A
> +#define PMBUS_REG_IIN_OC_FAULT_RESPONSE 0x5C
> +#define PMBUS_REG_TON_DELAY 0x60
> +#define PMBUS_REG_POUT_OP_FAULT_RESPONSE 0x69
> +#define PMBUS_REG_READ_VAUX 0x85
> +
> +#define PMBUS_REG_IKNEE_SET 0x2D
> +#define PMBUS_REG_PIN_COUNTER 0x2E
> +#define PMBUS_REG_VMIN_AWARE 0x2F
> +#define PMBUS_REG_VAUX_UV_FAULT_LIMIT 0x6C
> +#define PMBUS_REG_VAUX_OV_FAULT_LIMIT 0x6D
> +#define PMBUS_REG_VAUX_UV_FAULT_RESPONSE 0x6E
> +#define PMBUS_REG_VAUX_OV_FAULT_RESPONSE 0x6F
> +#define PMBUS_REG_VAUX_UV_WARNING 0x75
> +#define PMBUS_REG_VAUX_OV_WARNING 0x76
> +#define PMBUS_REG_MFR_FREE_USER_CONFIG_TABLES 0xCF
> +#define PMBUS_REG_MFR_ADDRESS_TABLE 0xD0
> +#define PMBUS_REG_MFR_STATUS_ONSEMI 0xD1
> +#define PMBUS_REG_MFR_UNLOCK 0xD2
> +#define PMBUS_REG_MFR_FAULTY_SPS 0xD3
> +#define PMBUS_REG_TLVR_FAULTS 0xD4
> +#define PMBUS_REG_MFR_USER_STORE_CONFIG_TAB 0xD5
> +#define PMBUS_REG_MFR_USER_CONFIG_INDEX 0xD6
> +#define PMBUS_REG_MFR_PWM_DISCONNECTION 0xD7
> +#define PMBUS_REG_MFR_VR_DISCONNECTION 0xD8
> +#define PMBUS_REG_MFR_TON_SLEW 0xD9
> +#define PMBUS_REG_MFR_TOFF_SLEW 0xDA
> +#define PMBUS_REG_MFR_RAIL_NAME 0xDB
> +#define PMBUS_REG_MFR_VOUT_DROOP 0xDC
> +#define PMBUS_REG_MFR_USER_RESTORE_CONFIG_TAB 0xDD
> +#define PMBUS_REG_MFR_SVR_GO 0xDE
> +#define PMBUS_REG_MFR_SET_PWD 0xDF
> +#define PMBUS_REG_MFR_CONFIG_ACTIVATE 0xE0
> +#define PMBUS_REG_MFR_CONFIG_RECOVER 0xE1
> +#define PMBUS_REG_MFR_OTP_DUMP 0xE2
> +#define PMBUS_REG_MFR_BBR_EN 0xE3
> +#define PMBUS_REG_MFR_DPM_MIN 0xE4
> +#define PMBUS_REG_MFR_VBOOT 0xE5
> +#define PMBUS_REG_MFR_PRECLAMP_OFFSET 0xE6
> +#define PMBUS_REG_MFR_TLVR_DIAGN 0xE7
> +#define PMBUS_REG_MFR_READ_VSYS 0xE8
> +#define PMBUS_REG_MFR_SPECIFIC_E9 0xE9
> +#define PMBUS_REG_MFR_SPECIFIC_EA 0xEA
> +#define PMBUS_REG_MFR_SS_CBC 0xEB
> +#define PMBUS_REG_MFR_AMD_STATUS 0xEC
> +#define PMBUS_REG_MFR_CHECKSUM 0xEE
> +#define PMBUS_REG_CSE_INDEX 0xF0
> +#define PMBUS_REG_COUT_MEASURE 0xF1
> +#define PMBUS_REG_VR_COUT 0xF2
> +#define PMBUS_REG_BBR_RAM 0xF3
> +#define PMBUS_REG_BBR_OTP 0xF4
> +#define PMBUS_REG_READ_PSYS 0xF5
> +#define PMBUS_REG_POSTCLAMP_OFFSET 0xF6
> +#define PMBUS_REG_PGOOD_DELAY 0xF7
> +#define PMBUS_REG_MFR_SPECIFIC_F8 0xF8
> +#define PMBUS_REG_MFR_SPECIFIC_F9 0xF9
> +#define PMBUS_REG_MFR_PWD_PROGRAM_RAM 0xFA
> +#define PMBUS_REG_MFR_PWD_PROGRAM_I2C 0xFB
> +#define PMBUS_REG_MFR_PWD_ENABLE_OTP_STORE 0xFC
> +
> +/* List of recognized commands */
> +static const u8 cc_list[] = {
> + PMBUS_PAGE,
> + PMBUS_OPERATION,
> + PMBUS_ON_OFF_CONFIG,
> + PMBUS_CLEAR_FAULTS,
> + PMBUS_WRITE_PROTECT,
> + PMBUS_CAPABILITY,
> + PMBUS_VOUT_MODE,
> + PMBUS_VOUT_COMMAND,
> + PMBUS_VOUT_MAX,
> + PMBUS_VOUT_MARGIN_HIGH,
> + PMBUS_VOUT_MARGIN_LOW,
> + PMBUS_VOUT_TRANSITION_RATE,
> + PMBUS_REG_VOUT_MIN,
> + PMBUS_REG_IKNEE_SET,
> + PMBUS_REG_PIN_COUNTER,
> + PMBUS_REG_VMIN_AWARE,
> + PMBUS_REG_POWER_MODE,
> + PMBUS_REG_VIN_ON,
> + PMBUS_REG_VIN_OFF,
> + PMBUS_VOUT_OV_FAULT_LIMIT,
> + PMBUS_VOUT_OV_FAULT_RESPONSE,
> + PMBUS_VOUT_UV_FAULT_LIMIT,
> + PMBUS_VOUT_UV_FAULT_RESPONSE,
> + PMBUS_IOUT_OC_FAULT_LIMIT,
> + PMBUS_IOUT_OC_FAULT_RESPONSE,
> + PMBUS_IOUT_OC_WARN_LIMIT,
> + PMBUS_OT_FAULT_LIMIT,
> + PMBUS_OT_FAULT_RESPONSE,
> + PMBUS_OT_WARN_LIMIT,
> + PMBUS_VIN_OV_FAULT_LIMIT,
> + PMBUS_VIN_OV_FAULT_RESPONSE,
> + PMBUS_VIN_OV_WARN_LIMIT,
> + PMBUS_VIN_UV_WARN_LIMIT,
> + PMBUS_VIN_UV_FAULT_LIMIT,
> + PMBUS_REG_VIN_UV_FAULT_RESPONSE,
> + PMBUS_IIN_OC_FAULT_LIMIT,
> + PMBUS_REG_IIN_OC_FAULT_RESPONSE,
> + PMBUS_IIN_OC_WARN_LIMIT,
> + PMBUS_REG_TON_DELAY,
> + PMBUS_POUT_OP_FAULT_LIMIT,
> + PMBUS_REG_POUT_OP_FAULT_RESPONSE,
> + PMBUS_POUT_OP_WARN_LIMIT,
> + PMBUS_PIN_OP_WARN_LIMIT,
> + PMBUS_REG_VAUX_UV_FAULT_LIMIT,
> + PMBUS_REG_VAUX_OV_FAULT_LIMIT,
> + PMBUS_REG_VAUX_UV_FAULT_RESPONSE,
> + PMBUS_REG_VAUX_OV_FAULT_RESPONSE,
> + PMBUS_REG_VAUX_UV_WARNING,
> + PMBUS_REG_VAUX_OV_WARNING,
> + PMBUS_STATUS_BYTE,
> + PMBUS_STATUS_WORD,
> + PMBUS_STATUS_VOUT,
> + PMBUS_STATUS_IOUT,
> + PMBUS_STATUS_INPUT,
> + PMBUS_STATUS_TEMPERATURE,
> + PMBUS_STATUS_CML,
> + PMBUS_STATUS_OTHER,
> + PMBUS_STATUS_MFR_SPECIFIC,
> + PMBUS_REG_READ_VAUX,
> + PMBUS_READ_VIN,
> + PMBUS_READ_IIN,
> + PMBUS_READ_VOUT,
> + PMBUS_READ_IOUT,
> + PMBUS_READ_TEMPERATURE_1,
> + PMBUS_READ_POUT,
> + PMBUS_READ_PIN,
> + PMBUS_REVISION,
> + PMBUS_MFR_ID,
> + PMBUS_MFR_MODEL,
> + PMBUS_MFR_REVISION,
> + PMBUS_IC_DEVICE_ID,
> + PMBUS_REG_MFR_FREE_USER_CONFIG_TABLES,
> + PMBUS_REG_MFR_ADDRESS_TABLE,
> + PMBUS_REG_MFR_STATUS_ONSEMI,
> + PMBUS_REG_MFR_UNLOCK,
> + PMBUS_REG_MFR_FAULTY_SPS,
> + PMBUS_REG_TLVR_FAULTS,
> + PMBUS_REG_MFR_USER_STORE_CONFIG_TAB,
> + PMBUS_REG_MFR_USER_CONFIG_INDEX,
> + PMBUS_REG_MFR_PWM_DISCONNECTION,
> + PMBUS_REG_MFR_VR_DISCONNECTION,
> + PMBUS_REG_MFR_TON_SLEW,
> + PMBUS_REG_MFR_TOFF_SLEW,
> + PMBUS_REG_MFR_RAIL_NAME,
> + PMBUS_REG_MFR_VOUT_DROOP,
> + PMBUS_REG_MFR_USER_RESTORE_CONFIG_TAB,
> + PMBUS_REG_MFR_SVR_GO,
> + PMBUS_REG_MFR_SET_PWD,
> + PMBUS_REG_MFR_CONFIG_ACTIVATE,
> + PMBUS_REG_MFR_CONFIG_RECOVER,
> + PMBUS_REG_MFR_OTP_DUMP,
> + PMBUS_REG_MFR_BBR_EN,
> + PMBUS_REG_MFR_DPM_MIN,
> + PMBUS_REG_MFR_VBOOT,
> + PMBUS_REG_MFR_PRECLAMP_OFFSET,
> + PMBUS_REG_MFR_TLVR_DIAGN,
> + PMBUS_REG_MFR_READ_VSYS,
> + PMBUS_REG_MFR_SPECIFIC_E9,
> + PMBUS_REG_MFR_SPECIFIC_EA,
> + PMBUS_REG_MFR_SS_CBC,
> + PMBUS_REG_MFR_AMD_STATUS,
> + PMBUS_REG_MFR_CHECKSUM,
> + PMBUS_REG_CSE_INDEX,
> + PMBUS_REG_COUT_MEASURE,
> + PMBUS_REG_VR_COUT,
> + PMBUS_REG_BBR_RAM,
> + PMBUS_REG_BBR_OTP,
> + PMBUS_REG_READ_PSYS,
> + PMBUS_REG_POSTCLAMP_OFFSET,
> + PMBUS_REG_PGOOD_DELAY,
> + PMBUS_REG_MFR_SPECIFIC_F8,
> + PMBUS_REG_MFR_SPECIFIC_F9,
> + PMBUS_REG_MFR_PWD_PROGRAM_RAM,
> + PMBUS_REG_MFR_PWD_PROGRAM_I2C,
> + PMBUS_REG_MFR_PWD_ENABLE_OTP_STORE,
> +};
> +
> +/* Following registers expect block read */
> +static const u8 blk_rd_cc[] = {
> + PMBUS_SMBALERT_MASK,
> + PMBUS_MFR_DATE,
> + PMBUS_IC_DEVICE_REV,
> +};
> +
> +struct fd5121_data {
> + struct attribute_group *groups[3];
> + struct pmbus_driver_info info;
> + struct device *dev;
> + u32 id;
> +};
> +
> +static s32 fd5121_read_block_data(const struct i2c_client *client,
> + u8 cmd_code, u8 len, u8 *pbuf)
> +{
> + s32 ret = 0;
> +
> + if (!i2c_check_functionality(client->adapter,
> + I2C_FUNC_SMBUS_READ_BLOCK_DATA)) {
> +
> + /* Payload length is in the first byte. */
> + ret = i2c_smbus_read_i2c_block_data(client, cmd_code,
> + len, pbuf);
> + if (ret < 0)
> + return ret;
> + ret = pbuf[0];
> + if (ret > len)
> + ret = len;
> + for (int idx = 0; idx < ret; idx++)
> + pbuf[idx] = pbuf[idx + 1];
> + return ret;
> + }
> + ret = i2c_smbus_read_block_data(client, cmd_code, pbuf);
> + if (ret < 0)
> + return ret;
> + return min_t(s32, ret, len);
> +}
> +
> +/* Command code that expects block read, not word read */
> +static bool fd5121_blk_rd_reg(u8 cmd_code)
> +{
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(blk_rd_cc); i++) {
> + if (cmd_code == blk_rd_cc[i])
> + return true;
> + }
> + return false;
> +}
> +
> +static ssize_t fd5121_send_byte_store(struct device *dev,
> + struct device_attribute *da,
> + const char *buf, size_t count)
> +{
> + struct i2c_client *client = to_i2c_client(dev->parent);
> + u8 val = 0;
> + int ret;
> +
> + ret = kstrtou8(buf, 10, &val);
> + if (ret < 0)
> + return ret;
> + ret = i2c_smbus_write_byte(client, val);
> + if (ret < 0)
> + return ret;
> + return count;
> +}
> +
> +static int fd5121_config_activate(struct i2c_client *client)
> +{
> + return i2c_smbus_write_byte_data(client,
> + PMBUS_REG_MFR_CONFIG_ACTIVATE, 0xAA);
> +}
> +
> +static ssize_t fd5121_byte_store(struct device *dev,
> + struct device_attribute *da,
> + const char *buf, size_t count)
> +{
> + struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
> + struct i2c_client *client = to_i2c_client(dev->parent);
> + u8 reg = attr->index;
> + int ret = 0;
> + u8 val = 0;
> +
> + switch (reg) {
> + case PMBUS_REG_MFR_CONFIG_ACTIVATE:
> + ret = fd5121_config_activate(client);
> + if (ret < 0)
> + return ret;
> + return count;
> + default:
> + ret = kstrtou8(buf, 10, &val);
> + if (ret < 0)
> + return ret;
> + break;
> + }
> + if (reg == PMBUS_PAGE && ((val != 0 && val != 1 &&
> + val != GENMASK(7, 0))))
> + return -EINVAL;
> + ret = i2c_smbus_write_byte_data(client, reg, val);
> + if (ret < 0)
> + return ret;
> + return count;
> +}
> +
> +static ssize_t fd5121_byte_show(struct device *dev,
> + struct device_attribute *da, char *buf)
> +{
> + struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
> + struct i2c_client *client = to_i2c_client(dev->parent);
> + u8 reg = attr->index;
> + s32 ret;
> +
> + ret = i2c_smbus_read_byte_data(client, reg);
> + if (ret < 0)
> + return ret;
> + return scnprintf(buf, PAGE_SIZE, "%d\n", ret & 0xFF);
> +}
> +
> +static ssize_t fd5121_word_store(struct device *dev,
> + struct device_attribute *da,
> + const char *buf, size_t count)
> +{
> + struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
> + struct i2c_client *client = to_i2c_client(dev->parent);
> + u8 reg = attr->index;
> + s16 val = 0;
> + int ret = 0;
> +
> + switch (reg) {
> + case PMBUS_REG_MFR_PWD_PROGRAM_RAM:
> + val = 0xC93F;
> + break;
> + default:
> + ret = kstrtos16(buf, 10, &val);
> + if (ret)
> + return ret;
> + break;
> + }
> + ret = i2c_smbus_write_word_data(client, reg, val);
> + if (ret < 0)
> + return ret;
> + return count;
> +}
> +
> +static ssize_t fd5121_word_show(struct device *dev,
> + struct device_attribute *da, char *buf)
> +{
> + struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
> + struct i2c_client *client = to_i2c_client(dev->parent);
> + u8 data[I2C_SMBUS_BLOCK_MAX] = { 0 };
> + u8 reg = attr->index;
> + s32 ret = 0;
> +
> + if (fd5121_blk_rd_reg(reg)) {
> + ret = fd5121_read_block_data(client, reg, 2, data);
> + if (ret >= 0)
> + ret = get_unaligned_le16(data);
> + } else
> + ret = i2c_smbus_read_word_data(client, reg);
> + if (ret < 0)
> + return ret;
> + return scnprintf(buf, PAGE_SIZE, "%d\n", ret & 0xFFFF);
> +}
> +
> +static s32 fd5121_write_block_data(const struct i2c_client *client,
> + u8 cmd_code, u8 len, u8 *pbuf)
> +{
> + s32 ret = 0;
> +
> + if (!i2c_check_functionality(client->adapter,
> + I2C_FUNC_SMBUS_WRITE_BLOCK_DATA))
> + ret = i2c_smbus_write_i2c_block_data(client, cmd_code,
> + len, pbuf);
> + else
> + ret = i2c_smbus_write_block_data(client, cmd_code,
> + len, pbuf);
> + return ret;
> +}
> +
> +static s32 fd5121_read_long(struct i2c_client *client, u8 cmd_code, u32 *pval)
> +{
> + u8 buffer[I2C_SMBUS_BLOCK_MAX] = { 0 };
> + s32 ret;
> +
> + ret = fd5121_read_block_data(client, cmd_code, 4, buffer);
> + if (ret < 0)
> + return ret;
> + if (ret < 4)
> + return -EIO;
> +
> + *pval = get_unaligned_le32(buffer);
> + return 0;
> +}
> +
> +static ssize_t fd5121_long_store(struct device *dev,
> + struct device_attribute *da,
> + const char *buf, size_t count)
> +{
> + struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
> + struct i2c_client *client = to_i2c_client(dev->parent);
> + u8 reg = attr->index;
> + u8 buffer[4];
> + u32 val = 0;
> + int ret = 0;
> + u8 len;
> +
> + ret = kstrtou32(buf, 10, &val);
> + if (ret)
> + return ret;
> +
> + len = (u8) sizeof(buffer);
> + for (u8 i = 0; i < len; i++) {
> + buffer[i] = val & 0xFF;
> + val >>= 8;
> + }
> + ret = fd5121_write_block_data(client, reg, len, buffer);
> + if (ret < 0)
> + return ret;
> + return count;
> +}
> +
> +static ssize_t fd5121_long_show(struct device *dev,
> + struct device_attribute *da, char *buf)
> +{
> + struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
> + struct i2c_client *client = to_i2c_client(dev->parent);
> + u8 reg = attr->index;
> + u32 val = 0;
> + s32 ret = 0;
> +
> + ret = fd5121_read_long(client, reg, &val);
> + if (ret < 0)
> + return ret;
> + return scnprintf(buf, PAGE_SIZE, "%d\n", val);
> +}
> +
> +static ssize_t fd5121_block_show(struct device *dev,
> + struct device_attribute *da, char *buf)
> +{
> + struct i2c_client *client = to_i2c_client(dev->parent);
> + struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
> + u8 buffer[I2C_SMBUS_BLOCK_MAX] = { 0 };
> + u8 reg = attr->index;
> + int printed = 0;
> + s32 ret = 0;
> + u8 len = 0;
> + int i = 0;
> +
> + if (reg == PMBUS_REG_MFR_FAULTY_SPS) {
> + int to_print = 0;
> +
> + len = 7;
> + ret = fd5121_read_block_data(client, reg, len, buffer);
> + if (ret < 0)
> + return ret;
> + printed = 0;
> + to_print = (ret < len) ? ret : len;
> + for (i = 0; i < to_print; i++)
> + printed += scnprintf(buf + printed,
> + PAGE_SIZE - printed,
> + "%02x", buffer[i]);
> + printed += scnprintf(buf + printed,
> + PAGE_SIZE - printed, "\n");
> + return printed;
> + } else if (reg == PMBUS_REG_BBR_RAM ||
> + reg == PMBUS_REG_BBR_OTP) {
> + u32 len = (reg == PMBUS_REG_BBR_OTP) ? 165 : 164;
> +
> + /* Extra byte may be needed in case we need to store
> + * the length of the data
> + */
> + u8 *tmp_in = kcalloc(len+1, sizeof(u8), GFP_KERNEL);
> +
> + if (tmp_in == NULL)
> + return -ENOMEM;
> + ret = fd5121_read_block_data(client, reg, len, tmp_in);
> + if (ret < 0) {
> + kfree(tmp_in);
> + return ret;
> + }
> +
> + printed = 0;
> + for (i = 0; i < ret; i++)
> + printed += scnprintf(buf + printed,
> + PAGE_SIZE - printed, "%02x",
> + tmp_in[i]);
> + printed += scnprintf(buf + printed,
> + PAGE_SIZE - printed, "\n");
> +
> + kfree(tmp_in);
> + return printed;
> + } else
> + return -ENODATA;
> +}
> +
> +static SENSOR_DEVICE_ATTR_RW(page, fd5121_byte,
> + PMBUS_PAGE);
> +static SENSOR_DEVICE_ATTR_RO(vout_raw, fd5121_word,
> + PMBUS_READ_VOUT);
> +static SENSOR_DEVICE_ATTR_RW(operation, fd5121_byte,
> + PMBUS_OPERATION);
> +static SENSOR_DEVICE_ATTR_RW(on_off_config, fd5121_byte,
> + PMBUS_ON_OFF_CONFIG);
> +static SENSOR_DEVICE_ATTR_WO(clear_faults, fd5121_byte,
> + PMBUS_CLEAR_FAULTS);
> +static SENSOR_DEVICE_ATTR_RW(write_protect, fd5121_byte,
> + PMBUS_WRITE_PROTECT);
> +static SENSOR_DEVICE_ATTR_RO(capability, fd5121_byte,
> + PMBUS_CAPABILITY);
> +static SENSOR_DEVICE_ATTR_RW(smbalert_mask, fd5121_word,
> + PMBUS_SMBALERT_MASK);
> +static SENSOR_DEVICE_ATTR_RO(vout_mode, fd5121_byte,
> + PMBUS_VOUT_MODE);
> +static SENSOR_DEVICE_ATTR_RW(vout_command, fd5121_word,
> + PMBUS_VOUT_COMMAND);
> +static SENSOR_DEVICE_ATTR_RW(vout_max, fd5121_word,
> + PMBUS_VOUT_MAX);
> +static SENSOR_DEVICE_ATTR_RW(vout_margin_high, fd5121_word,
> + PMBUS_VOUT_MARGIN_HIGH);
> +static SENSOR_DEVICE_ATTR_RW(vout_margin_low, fd5121_word,
> + PMBUS_VOUT_MARGIN_LOW);
> +static SENSOR_DEVICE_ATTR_RW(vout_transition_rate, fd5121_word,
> + PMBUS_VOUT_TRANSITION_RATE);
> +static SENSOR_DEVICE_ATTR_RW(vout_min, fd5121_word,
> + PMBUS_REG_VOUT_MIN);
> +static SENSOR_DEVICE_ATTR_RW(power_mode, fd5121_byte,
> + PMBUS_REG_POWER_MODE);
> +static SENSOR_DEVICE_ATTR_RW(vin_on, fd5121_word,
> + PMBUS_REG_VIN_ON);
> +static SENSOR_DEVICE_ATTR_RW(vin_off, fd5121_word,
> + PMBUS_REG_VIN_OFF);
> +static SENSOR_DEVICE_ATTR_RW(vin_uv_fault_response, fd5121_byte,
> + PMBUS_REG_VIN_UV_FAULT_RESPONSE);
> +static SENSOR_DEVICE_ATTR_RW(iin_oc_fault_response, fd5121_byte,
> + PMBUS_REG_IIN_OC_FAULT_RESPONSE);
> +static SENSOR_DEVICE_ATTR_RW(ton_delay, fd5121_word,
> + PMBUS_REG_TON_DELAY);
> +static SENSOR_DEVICE_ATTR_RW(pout_op_fault_response, fd5121_byte,
> + PMBUS_REG_POUT_OP_FAULT_RESPONSE);
> +static SENSOR_DEVICE_ATTR_RO(read_vaux, fd5121_word,
> + PMBUS_REG_READ_VAUX);
> +static SENSOR_DEVICE_ATTR_RW(iknee_set, fd5121_word,
> + PMBUS_REG_IKNEE_SET);
> +static SENSOR_DEVICE_ATTR_RW(pin_counter, fd5121_byte,
> + PMBUS_REG_PIN_COUNTER);
> +static SENSOR_DEVICE_ATTR_RW(vmin_aware, fd5121_word,
> + PMBUS_REG_VMIN_AWARE);
> +static SENSOR_DEVICE_ATTR_RW(vout_ov_fault_response, fd5121_byte,
> + PMBUS_VOUT_OV_FAULT_RESPONSE);
> +static SENSOR_DEVICE_ATTR_RW(vout_uv_fault_response, fd5121_byte,
> + PMBUS_VOUT_UV_FAULT_RESPONSE);
> +static SENSOR_DEVICE_ATTR_RW(iout_oc_fault_response, fd5121_byte,
> + PMBUS_IOUT_OC_FAULT_RESPONSE);
> +static SENSOR_DEVICE_ATTR_RW(ot_fault_response, fd5121_byte,
> + PMBUS_OT_FAULT_RESPONSE);
> +static SENSOR_DEVICE_ATTR_RW(vin_ov_fault_response, fd5121_byte,
> + PMBUS_VIN_OV_FAULT_RESPONSE);
> +static SENSOR_DEVICE_ATTR_RW(vaux_uv_fault_limit, fd5121_word,
> + PMBUS_REG_VAUX_UV_FAULT_LIMIT);
> +static SENSOR_DEVICE_ATTR_RW(vaux_ov_fault_limit, fd5121_word,
> + PMBUS_REG_VAUX_OV_FAULT_LIMIT);
> +static SENSOR_DEVICE_ATTR_RW(vaux_uv_fault_response, fd5121_byte,
> + PMBUS_REG_VAUX_UV_FAULT_RESPONSE);
> +static SENSOR_DEVICE_ATTR_RW(vaux_ov_fault_response, fd5121_byte,
> + PMBUS_REG_VAUX_OV_FAULT_RESPONSE);
> +static SENSOR_DEVICE_ATTR_RW(vaux_uv_warning, fd5121_word,
> + PMBUS_REG_VAUX_UV_WARNING);
> +static SENSOR_DEVICE_ATTR_RW(vaux_ov_warning, fd5121_word,
> + PMBUS_REG_VAUX_OV_WARNING);
> +static SENSOR_DEVICE_ATTR_RO(free_user_config_tables, fd5121_byte,
> + PMBUS_REG_MFR_FREE_USER_CONFIG_TABLES);
> +static SENSOR_DEVICE_ATTR_RW(address_table, fd5121_byte,
> + PMBUS_REG_MFR_ADDRESS_TABLE);
> +static SENSOR_DEVICE_ATTR_RW(status_onsemi, fd5121_word,
> + PMBUS_REG_MFR_STATUS_ONSEMI);
> +static SENSOR_DEVICE_ATTR_RO(status_byte, fd5121_byte,
> + PMBUS_STATUS_BYTE);
> +static SENSOR_DEVICE_ATTR_RO(status_cml, fd5121_byte,
> + PMBUS_STATUS_CML);
> +static SENSOR_DEVICE_ATTR_RO(status_other, fd5121_byte,
> + PMBUS_STATUS_OTHER);
> +static SENSOR_DEVICE_ATTR_RO(status_mfr_specific, fd5121_byte,
> + PMBUS_STATUS_MFR_SPECIFIC);
> +static SENSOR_DEVICE_ATTR_RO(revision, fd5121_byte,
> + PMBUS_REVISION);
> +static SENSOR_DEVICE_ATTR_RO(id, fd5121_long,
> + PMBUS_MFR_ID);
> +static SENSOR_DEVICE_ATTR_RO(model, fd5121_long,
> + PMBUS_MFR_MODEL);
> +static SENSOR_DEVICE_ATTR_RO(mfr_revision, fd5121_long,
> + PMBUS_MFR_REVISION);
> +static SENSOR_DEVICE_ATTR_RW(date, fd5121_word,
> + PMBUS_MFR_DATE);
> +static SENSOR_DEVICE_ATTR_RO(ic_device_id, fd5121_long,
> + PMBUS_IC_DEVICE_ID);
> +static SENSOR_DEVICE_ATTR_RO(ic_device_rev, fd5121_word,
> + PMBUS_IC_DEVICE_REV);
> +static SENSOR_DEVICE_ATTR_WO(unlock, fd5121_byte,
> + PMBUS_REG_MFR_UNLOCK);
> +static SENSOR_DEVICE_ATTR_RO(faulty_sps, fd5121_block,
> + PMBUS_REG_MFR_FAULTY_SPS);
> +static SENSOR_DEVICE_ATTR_RO(tlvr_faults, fd5121_word,
> + PMBUS_REG_TLVR_FAULTS);
> +static SENSOR_DEVICE_ATTR_RW(user_store_config_tab, fd5121_byte,
> + PMBUS_REG_MFR_USER_STORE_CONFIG_TAB);
> +static SENSOR_DEVICE_ATTR_RO(user_config_index, fd5121_byte,
> + PMBUS_REG_MFR_USER_CONFIG_INDEX);
> +static SENSOR_DEVICE_ATTR_RO(pwm_disconnection, fd5121_word,
> + PMBUS_REG_MFR_PWM_DISCONNECTION);
> +static SENSOR_DEVICE_ATTR_RO(vr_disconnection, fd5121_byte,
> + PMBUS_REG_MFR_VR_DISCONNECTION);
> +static SENSOR_DEVICE_ATTR_RW(ton_slew, fd5121_byte,
> + PMBUS_REG_MFR_TON_SLEW);
> +static SENSOR_DEVICE_ATTR_RW(toff_slew, fd5121_byte,
> + PMBUS_REG_MFR_TOFF_SLEW);
> +static SENSOR_DEVICE_ATTR_RW(rail_name, fd5121_word,
> + PMBUS_REG_MFR_RAIL_NAME);
> +static SENSOR_DEVICE_ATTR_RW(vout_droop, fd5121_byte,
> + PMBUS_REG_MFR_VOUT_DROOP);
> +static SENSOR_DEVICE_ATTR_WO(svr_go, fd5121_send_byte,
> + PMBUS_REG_MFR_SVR_GO);
> +static SENSOR_DEVICE_ATTR_RW(user_restore_config_tab, fd5121_byte,
> + PMBUS_REG_MFR_USER_RESTORE_CONFIG_TAB);
> +static SENSOR_DEVICE_ATTR_WO(set_pwd, fd5121_byte,
> + PMBUS_REG_MFR_SET_PWD);
> +static SENSOR_DEVICE_ATTR_RW(config_activate, fd5121_byte,
> + PMBUS_REG_MFR_CONFIG_ACTIVATE);
> +static SENSOR_DEVICE_ATTR_RW(config_recover, fd5121_byte,
> + PMBUS_REG_MFR_CONFIG_RECOVER);
> +static SENSOR_DEVICE_ATTR_RW(otp_dump, fd5121_byte,
> + PMBUS_REG_MFR_OTP_DUMP);
> +static SENSOR_DEVICE_ATTR_RW(bbr_en, fd5121_byte,
> + PMBUS_REG_MFR_BBR_EN);
> +static SENSOR_DEVICE_ATTR_RW(dpm_min, fd5121_byte,
> + PMBUS_REG_MFR_DPM_MIN);
> +static SENSOR_DEVICE_ATTR_RW(vboot, fd5121_word,
> + PMBUS_REG_MFR_VBOOT);
> +static SENSOR_DEVICE_ATTR_RW(preclamp_offset, fd5121_word,
> + PMBUS_REG_MFR_PRECLAMP_OFFSET);
> +static SENSOR_DEVICE_ATTR_RW(tlvr_diagn, fd5121_word,
> + PMBUS_REG_MFR_TLVR_DIAGN);
> +static SENSOR_DEVICE_ATTR_RO(vsys, fd5121_word,
> + PMBUS_REG_MFR_READ_VSYS);
> +static SENSOR_DEVICE_ATTR_RW(specific_e9, fd5121_word,
> + PMBUS_REG_MFR_SPECIFIC_E9);
> +static SENSOR_DEVICE_ATTR_RW(specific_ea, fd5121_long,
> + PMBUS_REG_MFR_SPECIFIC_EA);
> +static SENSOR_DEVICE_ATTR_RO(ss_cbc, fd5121_word,
> + PMBUS_REG_MFR_SS_CBC);
> +static SENSOR_DEVICE_ATTR_RO(amd_status, fd5121_byte,
> + PMBUS_REG_MFR_AMD_STATUS);
> +static SENSOR_DEVICE_ATTR_RO(checksum, fd5121_word,
> + PMBUS_REG_MFR_CHECKSUM);
> +static SENSOR_DEVICE_ATTR_RO(cse_index, fd5121_word,
> + PMBUS_REG_CSE_INDEX);
> +static SENSOR_DEVICE_ATTR_RW(cout_measure, fd5121_word,
> + PMBUS_REG_COUT_MEASURE);
> +static SENSOR_DEVICE_ATTR_RO(vr_cout, fd5121_word,
> + PMBUS_REG_VR_COUT);
> +static SENSOR_DEVICE_ATTR_RO(bbr_ram, fd5121_block,
> + PMBUS_REG_BBR_RAM);
> +static SENSOR_DEVICE_ATTR_RO(bbr_otp, fd5121_block,
> + PMBUS_REG_BBR_OTP);
> +static SENSOR_DEVICE_ATTR_RO(psys, fd5121_word,
> + PMBUS_REG_READ_PSYS);
> +static SENSOR_DEVICE_ATTR_RW(postclamp_offset, fd5121_word,
> + PMBUS_REG_POSTCLAMP_OFFSET);
> +static SENSOR_DEVICE_ATTR_RW(pgood_delay, fd5121_byte,
> + PMBUS_REG_PGOOD_DELAY);
> +static SENSOR_DEVICE_ATTR_RW(specific_f8, fd5121_word,
> + PMBUS_REG_MFR_SPECIFIC_F8);
> +static SENSOR_DEVICE_ATTR_RW(specific_f9, fd5121_long,
> + PMBUS_REG_MFR_SPECIFIC_F9);
> +static SENSOR_DEVICE_ATTR_RW(pwd_program_ram, fd5121_word,
> + PMBUS_REG_MFR_PWD_PROGRAM_RAM);
> +static SENSOR_DEVICE_ATTR_RW(pwd_program_i2c, fd5121_word,
> + PMBUS_REG_MFR_PWD_PROGRAM_I2C);
> +static SENSOR_DEVICE_ATTR_RW(pwd_enable_otp_store, fd5121_word,
> + PMBUS_REG_MFR_PWD_ENABLE_OTP_STORE);
> +
> +static struct attribute *fd5121_non_paged_attrs[] = {
> + &sensor_dev_attr_page.dev_attr.attr,
> + &sensor_dev_attr_capability.dev_attr.attr,
> + &sensor_dev_attr_pin_counter.dev_attr.attr,
> + &sensor_dev_attr_vaux_uv_fault_limit.dev_attr.attr,
> + &sensor_dev_attr_vaux_ov_fault_limit.dev_attr.attr,
> + &sensor_dev_attr_vaux_uv_warning.dev_attr.attr,
> + &sensor_dev_attr_vaux_ov_warning.dev_attr.attr,
> + &sensor_dev_attr_free_user_config_tables.dev_attr.attr,
> + &sensor_dev_attr_address_table.dev_attr.attr,
> + &sensor_dev_attr_unlock.dev_attr.attr,
> + &sensor_dev_attr_faulty_sps.dev_attr.attr,
> + &sensor_dev_attr_tlvr_faults.dev_attr.attr,
> + &sensor_dev_attr_user_store_config_tab.dev_attr.attr,
> + &sensor_dev_attr_user_config_index.dev_attr.attr,
> + &sensor_dev_attr_pwm_disconnection.dev_attr.attr,
> + &sensor_dev_attr_vr_disconnection.dev_attr.attr,
> + &sensor_dev_attr_user_restore_config_tab.dev_attr.attr,
> + &sensor_dev_attr_svr_go.dev_attr.attr,
> + &sensor_dev_attr_set_pwd.dev_attr.attr,
> + &sensor_dev_attr_config_activate.dev_attr.attr,
> + &sensor_dev_attr_config_recover.dev_attr.attr,
> + &sensor_dev_attr_otp_dump.dev_attr.attr,
> + &sensor_dev_attr_bbr_en.dev_attr.attr,
> + &sensor_dev_attr_vboot.dev_attr.attr,
> + &sensor_dev_attr_vsys.dev_attr.attr,
> + &sensor_dev_attr_specific_e9.dev_attr.attr,
> + &sensor_dev_attr_specific_ea.dev_attr.attr,
> + &sensor_dev_attr_ss_cbc.dev_attr.attr,
> + &sensor_dev_attr_checksum.dev_attr.attr,
> + &sensor_dev_attr_cse_index.dev_attr.attr,
> + &sensor_dev_attr_cout_measure.dev_attr.attr,
> + &sensor_dev_attr_vr_cout.dev_attr.attr,
> + &sensor_dev_attr_bbr_ram.dev_attr.attr,
> + &sensor_dev_attr_bbr_otp.dev_attr.attr,
> + &sensor_dev_attr_psys.dev_attr.attr,
> + &sensor_dev_attr_specific_f8.dev_attr.attr,
> + &sensor_dev_attr_specific_f9.dev_attr.attr,
> + &sensor_dev_attr_pwd_program_ram.dev_attr.attr,
> + &sensor_dev_attr_pwd_program_i2c.dev_attr.attr,
> + &sensor_dev_attr_pwd_enable_otp_store.dev_attr.attr,
> + &sensor_dev_attr_revision.dev_attr.attr,
> + &sensor_dev_attr_id.dev_attr.attr,
> + &sensor_dev_attr_model.dev_attr.attr,
> + &sensor_dev_attr_mfr_revision.dev_attr.attr,
> + &sensor_dev_attr_date.dev_attr.attr,
> + &sensor_dev_attr_ic_device_id.dev_attr.attr,
> + &sensor_dev_attr_ic_device_rev.dev_attr.attr,
> + NULL
> +};
> +
> +static struct attribute *fd5121_paged_attrs[] = {
> + &sensor_dev_attr_operation.dev_attr.attr,
> + &sensor_dev_attr_vout_raw.dev_attr.attr,
> + &sensor_dev_attr_on_off_config.dev_attr.attr,
> + &sensor_dev_attr_clear_faults.dev_attr.attr,
> + &sensor_dev_attr_write_protect.dev_attr.attr,
> + &sensor_dev_attr_smbalert_mask.dev_attr.attr,
> + &sensor_dev_attr_vout_mode.dev_attr.attr,
> + &sensor_dev_attr_vout_command.dev_attr.attr,
> + &sensor_dev_attr_vout_margin_high.dev_attr.attr,
> + &sensor_dev_attr_vout_margin_low.dev_attr.attr,
> + &sensor_dev_attr_vout_min.dev_attr.attr,
> + &sensor_dev_attr_vin_on.dev_attr.attr,
> + &sensor_dev_attr_vin_off.dev_attr.attr,
> + &sensor_dev_attr_vout_ov_fault_response.dev_attr.attr,
> + &sensor_dev_attr_vout_uv_fault_response.dev_attr.attr,
> + &sensor_dev_attr_iout_oc_fault_response.dev_attr.attr,
> + &sensor_dev_attr_ot_fault_response.dev_attr.attr,
> + &sensor_dev_attr_vin_ov_fault_response.dev_attr.attr,
> + &sensor_dev_attr_status_byte.dev_attr.attr,
> + &sensor_dev_attr_iknee_set.dev_attr.attr,
> + &sensor_dev_attr_vmin_aware.dev_attr.attr,
> + &sensor_dev_attr_power_mode.dev_attr.attr,
> + &sensor_dev_attr_vin_uv_fault_response.dev_attr.attr,
> + &sensor_dev_attr_iin_oc_fault_response.dev_attr.attr,
> + &sensor_dev_attr_ton_delay.dev_attr.attr,
> + &sensor_dev_attr_pout_op_fault_response.dev_attr.attr,
> + &sensor_dev_attr_vaux_uv_fault_response.dev_attr.attr,
> + &sensor_dev_attr_vaux_ov_fault_response.dev_attr.attr,
> + &sensor_dev_attr_status_onsemi.dev_attr.attr,
> + &sensor_dev_attr_status_cml.dev_attr.attr,
> + &sensor_dev_attr_status_other.dev_attr.attr,
> + &sensor_dev_attr_status_mfr_specific.dev_attr.attr,
> + &sensor_dev_attr_read_vaux.dev_attr.attr,
> + &sensor_dev_attr_ton_slew.dev_attr.attr,
> + &sensor_dev_attr_toff_slew.dev_attr.attr,
> + &sensor_dev_attr_rail_name.dev_attr.attr,
> + &sensor_dev_attr_vout_droop.dev_attr.attr,
> + &sensor_dev_attr_dpm_min.dev_attr.attr,
> + &sensor_dev_attr_preclamp_offset.dev_attr.attr,
> + &sensor_dev_attr_tlvr_diagn.dev_attr.attr,
> + &sensor_dev_attr_amd_status.dev_attr.attr,
> + &sensor_dev_attr_postclamp_offset.dev_attr.attr,
> + &sensor_dev_attr_pgood_delay.dev_attr.attr,
> + &sensor_dev_attr_vout_max.dev_attr.attr,
> + &sensor_dev_attr_vout_transition_rate.dev_attr.attr,
> + NULL
This is a complete no-go. We do not explose raw register data as sysfs
attributes. You may expose essential register data as debugfs files,
but only those deemed necessary. The above is just "let's blindly
expose everything". Most of the above should be programmed in manufacturing
and not be touched subsequently, much less as writeable attributes.
Writing bad/unexpected values into many of those attributes can turn
a board into a brick. It is bad enough that/if this is even possible,
but exposing it as sysfs attribute would be a terrible idea.
I am not going to review this driver any further at this point.
Guenter
> +};
> +
> +static struct attribute_group fd5121_groups[2] = {
> + { .name = "global", .attrs = fd5121_non_paged_attrs },
> + { .name = "paged", .attrs = fd5121_paged_attrs }
> +};
> +
> +/* Regulator descriptors for VOUT rails (VID encoded) */
> +static struct regulator_desc fd5121_reg_desc[] = {
> + PMBUS_REGULATOR_STEP_ONE("vout1", 3001, 1000, 200000),
> + PMBUS_REGULATOR_STEP_ONE("vout2", 3001, 1000, 200000),
> +};
> +
> +static int fd5121_valid_reg(struct i2c_client *client, int reg)
> +{
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(cc_list); i++) {
> + if (reg == cc_list[i])
> + return 0;
> + }
> +
> + if (fd5121_blk_rd_reg(reg))
> + return 0;
> + return -ENXIO;
> +}
> +
> +static int fd5121_read_word_data(struct i2c_client *client, int page,
> + int phase, int reg)
> +{
> + int ret;
> +
> + ret = fd5121_valid_reg(client, reg);
> + if (ret < 0)
> + return ret;
> +
> + ret = pmbus_read_word_data(client, page, phase, reg);
> + if (ret < 0)
> + return ret;
> +
> + /* Chip reports VOUT_MODE as vid. But gives raw value 1mV per bit.
> + * So, encode the READ_VOUT value so that it gets decoded and
> + * reported correctly.
> + */
> + if (reg == PMBUS_READ_VOUT)
> + ret = DIV_ROUND_CLOSEST(155000 - ret * 100, 625);
> + return ret;
> +}
> +
> +static int fd5121_read_byte_data(struct i2c_client *client, int page, int reg)
> +{
> + int ret;
> +
> + ret = fd5121_valid_reg(client, reg);
> + if (ret < 0)
> + return ret;
> +
> + return pmbus_read_byte_data(client, page, reg);
> +}
> +
> +static int fd5121_write_byte_data(struct i2c_client *client, int page,
> + int reg, u8 value)
> +{
> + int ret;
> +
> + ret = fd5121_valid_reg(client, reg);
> + if (ret < 0)
> + return ret;
> + return pmbus_write_byte_data(client, page, reg, value);
> +}
> +
> +static int fd5121_write_byte(struct i2c_client *client, int page, u8 byte)
> +{
> + return pmbus_write_byte(client, page, byte);
> +}
> +
> +static int fd5121_write_word_data(struct i2c_client *client, int page,
> + int reg, u16 word)
> +{
> + int ret;
> +
> + ret = fd5121_valid_reg(client, reg);
> + if (ret < 0)
> + return ret;
> + ret = pmbus_write_word_data(client, page, reg, word);
> + return ret;
> +}
> +
> +static u32 fd5121_get_dev_id(struct i2c_client *client)
> +{
> + u32 dev_id = 0;
> + s32 ret = 0;
> +
> + ret = fd5121_read_long(client, PMBUS_IC_DEVICE_ID, &dev_id);
> + if (ret < 0)
> + return CTLR_ID_UNKNOWN;
> +
> + switch (dev_id) {
> + case CTLR_ID_FD5121:
> + case CTLR_ID_FD5123:
> + case CTLR_ID_FD5125:
> + break;
> + default:
> + if (dev_id != 0)
> + dev_err(&client->dev, "Unknown device 0x%x",
> + dev_id);
> + return CTLR_ID_UNKNOWN;
> + }
> + return dev_id;
> +}
> +
> +static int fd5121_probe(struct i2c_client *client)
> +{
> + struct pmbus_driver_info *info;
> + struct fd5121_data *pdata;
> + u32 id;
> +
> + if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C))
> + return -EOPNOTSUPP;
> +
> + pdata = devm_kzalloc(&client->dev, sizeof(struct fd5121_data),
> + GFP_KERNEL);
> + if (pdata == NULL)
> + return -ENOMEM;
> +
> + pdata->dev = &client->dev;
> + pdata->groups[0] = &fd5121_groups[0];
> + pdata->groups[1] = &fd5121_groups[1];
> +
> + id = fd5121_get_dev_id(client);
> + if (id == CTLR_ID_UNKNOWN)
> + return -ENODEV;
> +
> + pdata->id = id;
> +
> + switch (id) {
> + case CTLR_ID_FD5121:
> + case CTLR_ID_FD5123:
> + case CTLR_ID_FD5125:
> + break;
> + default:
> + dev_err(&client->dev, "Failed to read device ID");
> + return -ENODEV;
> + }
> +
> + info = &pdata->info;
> + info->groups = (const struct attribute_group **)&pdata->groups[0];
> + info->write_word_data = fd5121_write_word_data;
> + info->write_byte = fd5121_write_byte;
> + info->write_byte_data = fd5121_write_byte_data;
> + info->read_word_data = fd5121_read_word_data;
> + info->read_byte_data = fd5121_read_byte_data;
> +
> + info->pages = FD5121_NUM_PAGES;
> + info->format[PSC_VOLTAGE_IN] = linear;
> + info->format[PSC_VOLTAGE_OUT] = vid;
> +
> + fd5121_reg_desc[0].id = 0;
> + fd5121_reg_desc[1].id = 1;
> +
> + /* Device implements VID coding with 1 mV steps from 0.200 V
> + * up to 3.200 V
> + */
> + info->num_regulators = FD5121_NUM_PAGES;
> + info->reg_desc = fd5121_reg_desc;
> + info->format[PSC_CURRENT_IN] = linear;
> + info->format[PSC_CURRENT_OUT] = linear;
> + info->format[PSC_POWER] = linear;
> + info->format[PSC_TEMPERATURE] = linear;
> + for (u8 idx = 0; idx < info->pages; idx++) {
> + info->func[idx] = PMBUS_HAVE_IOUT | PMBUS_HAVE_STATUS_IOUT;
> + info->func[idx] |= PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT;
> + info->func[idx] |= PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP;
> + info->func[idx] |= PMBUS_HAVE_PIN | PMBUS_HAVE_POUT;
> + info->func[idx] |= PMBUS_HAVE_VIN | PMBUS_HAVE_IIN;
> + info->func[idx] |= PMBUS_HAVE_STATUS_INPUT;
> + info->vrm_version[idx] = amd625mv;
> + }
> + return pmbus_do_probe(client, info);
> +}
> +
> +#ifdef CONFIG_OF
> +static const struct of_device_id fd5121_of_match[] = {
> + { .compatible = "onnn,fd5121" },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, fd5121_of_match);
> +#endif
> +
> +static const struct i2c_device_id fd5121_id[] = {
> + { "fd5121", chip_fd5121 },
> + { "fd5123", chip_fd5123 },
> + { "fd5125", chip_fd5125 },
> + { }
> +};
> +MODULE_DEVICE_TABLE(i2c, fd5121_id);
> +
> +static struct i2c_driver fd5121_driver = {
> + .driver = {
> + .name = "fd5121",
> + .of_match_table = of_match_ptr(fd5121_of_match),
> + },
> + .probe = fd5121_probe,
> + .id_table = fd5121_id,
> +};
> +
> +module_i2c_driver(fd5121_driver);
> +
> +MODULE_AUTHOR("Selva Rajagopal <selvamani.rajagopal@onsemi.com>");
> +MODULE_DESCRIPTION("PMBus driver for FD5121");
> +MODULE_LICENSE("GPL");
> +MODULE_IMPORT_NS("PMBUS");
> +
>
^ permalink raw reply
* Re: [PATCH] crypto: af_alg - Add af_alg_restrict sysctl, defaulting to 1
From: Rosen Penev @ 2026-06-23 22:06 UTC (permalink / raw)
To: Eric Biggers
Cc: linux-crypto, Herbert Xu, linux-kernel, linux-doc,
linux-bluetooth, iwd, linux-hardening, Milan Broz,
Demi Marie Obenour, Andy Lutomirski
In-Reply-To: <20260623214730.GA3281861@google.com>
On Tue, Jun 23, 2026 at 2:47 PM Eric Biggers <ebiggers@kernel.org> wrote:
>
> On Tue, Jun 23, 2026 at 02:28:17PM -0700, Rosen Penev wrote:
> > > +static const struct af_alg_allowlist_entry hash_allowlist[] = {
> > > + { "cmac(aes)", true }, /* iwd, bluez */
> > > + { "hmac(md5)", true }, /* iwd */
> > > + { "hmac(sha1)", true }, /* iwd */
> > > + { "hmac(sha224)", true }, /* iwd */
> > > + { "hmac(sha256)", true }, /* iwd */
> > > + { "hmac(sha384)", true }, /* iwd */
> > > + { "hmac(sha512)", true }, /* iwd, sha512hmac */
> > > + { "md4", true }, /* iwd */
> > > + { "md5", true }, /* iwd */
> > > + { "sha1", false }, /* iwd, iproute2 < 7.0 */
> > > + { "sha224", true }, /* iwd */
> > > + { "sha256", true }, /* iwd */
> > > + { "sha384", true }, /* iwd */
> > > + { "sha512", true }, /* iwd */
> > > + {},
> > In OpenWrt, https://gitlab.com/linux-afs/kafs-client and strongswan
> > seem to be the other users of the user API. I haven't looked into what
> > they need.
>
> [Please trim your replies, thanks!]
Not sure what you mean.
>
> https://gitlab.com/linux-afs/kafs-client uses AF_ALG only for
> "hmac(md5)", which I already put on the privileged allowlist due to iwd
> also using it. So it would still work by default with the current
> patch, unless it needs to use it unprivileged.
>
> (FWIW, a use of a single obsolete algorithm like this is also a good
> candidate for just replacing with local code...)
>
> https://github.com/strongswan/strongswan already supports userspace
> crypto libraries.
Oh lovely. Looks like this needs fixing in OpenWrt.
>
> - Eric
^ permalink raw reply
* Re: [PATCH] crypto: af_alg - Add af_alg_restrict sysctl, defaulting to 1
From: Eric Biggers @ 2026-06-23 21:47 UTC (permalink / raw)
To: Rosen Penev
Cc: linux-crypto, Herbert Xu, linux-kernel, linux-doc,
linux-bluetooth, iwd, linux-hardening, Milan Broz,
Demi Marie Obenour, Andy Lutomirski
In-Reply-To: <CAKxU2N_EGTWkvtPOxQXBroxGVXDf1atPoFVyRRu0wHOtEXVWaA@mail.gmail.com>
On Tue, Jun 23, 2026 at 02:28:17PM -0700, Rosen Penev wrote:
> > +static const struct af_alg_allowlist_entry hash_allowlist[] = {
> > + { "cmac(aes)", true }, /* iwd, bluez */
> > + { "hmac(md5)", true }, /* iwd */
> > + { "hmac(sha1)", true }, /* iwd */
> > + { "hmac(sha224)", true }, /* iwd */
> > + { "hmac(sha256)", true }, /* iwd */
> > + { "hmac(sha384)", true }, /* iwd */
> > + { "hmac(sha512)", true }, /* iwd, sha512hmac */
> > + { "md4", true }, /* iwd */
> > + { "md5", true }, /* iwd */
> > + { "sha1", false }, /* iwd, iproute2 < 7.0 */
> > + { "sha224", true }, /* iwd */
> > + { "sha256", true }, /* iwd */
> > + { "sha384", true }, /* iwd */
> > + { "sha512", true }, /* iwd */
> > + {},
> In OpenWrt, https://gitlab.com/linux-afs/kafs-client and strongswan
> seem to be the other users of the user API. I haven't looked into what
> they need.
[Please trim your replies, thanks!]
https://gitlab.com/linux-afs/kafs-client uses AF_ALG only for
"hmac(md5)", which I already put on the privileged allowlist due to iwd
also using it. So it would still work by default with the current
patch, unless it needs to use it unprivileged.
(FWIW, a use of a single obsolete algorithm like this is also a good
candidate for just replacing with local code...)
https://github.com/strongswan/strongswan already supports userspace
crypto libraries.
- Eric
^ permalink raw reply
* Re: [PATCH] crypto: af_alg - Add af_alg_restrict sysctl, defaulting to 1
From: Rosen Penev @ 2026-06-23 21:28 UTC (permalink / raw)
To: Eric Biggers
Cc: linux-crypto, Herbert Xu, linux-kernel, linux-doc,
linux-bluetooth, iwd, linux-hardening, Milan Broz,
Demi Marie Obenour, Andy Lutomirski
In-Reply-To: <20260622234803.6982-1-ebiggers@kernel.org>
On Mon, Jun 22, 2026 at 4:49 PM Eric Biggers <ebiggers@kernel.org> wrote:
>
> AF_ALG is a frequent source of vulnerabilities and a maintenance
> nightmare. It exposes far more functionality to userspace than ever
> should have been exposed, especially to unprivileged processes. Recent
> exploits have targeted kernel internal implementation details like
> "authencesn" that have zero use case for userspace access.
>
> Fortunately, AF_ALG is rarely used in practice, as userspace crypto
> libraries exist. And when it is used, only some functionality is known
> to be used, and many users are known to hold capabilities already.
> iwd for example requires CAP_NET_ADMIN and has a known algorithm list
> (https://lore.kernel.org/linux-crypto/bcbbef00-5881-421b-8892-7be6c04b832d@gmail.com/).
>
> Thus, let's restrict the set of allowed algorithms by default, depending
> on the capabilities held.
>
> Add a sysctl /proc/sys/crypto/af_alg_restrict with meaning:
>
> 0: unrestricted
> 1: limited functionality
> 2: completely disabled
>
> Set the default value to 1, which enables an algorithm allowlist for
> unprivileged processes and a slightly longer allowlist for privileged
> processes.
>
> Note that the list may be tweaked in the future. However, the common
> use cases such as iwd and bluez are taken into account already. I've
> tested that iwd still works with the default value of 1.
>
> Signed-off-by: Eric Biggers <ebiggers@kernel.org>
> ---
> Documentation/admin-guide/sysctl/crypto.rst | 36 +++++++++++
> Documentation/crypto/userspace-if.rst | 13 +++-
> crypto/af_alg.c | 72 +++++++++++++++++++--
> crypto/algif_aead.c | 11 ++++
> crypto/algif_hash.c | 24 +++++++
> crypto/algif_rng.c | 9 +++
> crypto/algif_skcipher.c | 20 ++++++
> include/crypto/if_alg.h | 8 +++
> 8 files changed, 184 insertions(+), 9 deletions(-)
>
> diff --git a/Documentation/admin-guide/sysctl/crypto.rst b/Documentation/admin-guide/sysctl/crypto.rst
> index b707bd314a64..9a1bd53287f4 100644
> --- a/Documentation/admin-guide/sysctl/crypto.rst
> +++ b/Documentation/admin-guide/sysctl/crypto.rst
> @@ -5,10 +5,46 @@
> These files show up in ``/proc/sys/crypto/``, depending on the
> kernel configuration:
>
> .. contents:: :local:
>
> +.. _af_alg_restrict:
> +
> +af_alg_restrict
> +===============
> +
> +Controls the level of restriction of AF_ALG.
> +
> +AF_ALG is a deprecated and rarely-used userspace interface that is a
> +frequent source of vulnerabilities. It also unnecessarily exposes a
> +large number of kernel implementation details. For more information
> +about AF_ALG, see :ref:`Documentation/crypto/userspace-if.rst
> +<crypto_userspace_interface>`.
> +
> +Starting in Linux v7.3, AF_ALG supports only a limited set of
> +algorithms by default. This sysctl allows the system administrator to
> +remove this restriction when needed for compatibility reasons, or to
> +go further and disable AF_ALG entirely. The default value is 1.
> +
> +=== ==================================================================
> +0 AF_ALG is unrestricted.
> +
> +1 AF_ALG is supported with a limited list of algorithms. The list
> + is designed for compatibility with known users such as iwd and
> + bluez that haven't yet been fixed to use userspace crypto code.
> +
> + Specifically, there is an allowlist for unprivileged processes
> + and a somewhat longer allowlist for processes that hold
> + CAP_SYS_ADMIN or CAP_NET_ADMIN in the initial user namespace.
> +
> + Attempts to bind() an AF_ALG socket with a disallowed algorithm
> + fail with ENOENT.
> +
> +2 AF_ALG is completely disabled. Attempts to create an AF_ALG
> + socket fail with EAFNOSUPPORT.
> +=== ==================================================================
> +
> fips_enabled
> ============
>
> Read-only flag that indicates whether FIPS mode is enabled.
>
> diff --git a/Documentation/crypto/userspace-if.rst b/Documentation/crypto/userspace-if.rst
> index ab93300c8e04..d6194346e366 100644
> --- a/Documentation/crypto/userspace-if.rst
> +++ b/Documentation/crypto/userspace-if.rst
> @@ -1,5 +1,7 @@
> +.. _crypto_userspace_interface:
> +
> User Space Interface
> ====================
>
> Introduction
> ------------
> @@ -10,13 +12,18 @@ code.
>
> AF_ALG is insecure and is deprecated. Originally added to the kernel in 2010,
> most kernel developers now consider it to be a mistake. Support for hardware
> accelerators, which was the original purpose of AF_ALG, has been removed.
>
> -AF_ALG continues to be supported only for backwards compatibility. On systems
> -where no programs using AF_ALG remain, the support for it should be disabled by
> -disabling ``CONFIG_CRYPTO_USER_API_*``.
> +AF_ALG continues to be supported only for backwards compatibility.
> +
> +Starting in Linux v7.3, the set of algorithms supported by AF_ALG is limited by
> +default. See :ref:`/proc/sys/crypto/af_alg_restrict <af_alg_restrict>`.
> +
> +On systems where no programs using AF_ALG remain, the support for it should be
> +disabled entirely by setting ``/proc/sys/crypto/af_alg_restrict`` to 2 or by
> +disabling ``CONFIG_CRYPTO_USER_API_*`` in the kernel configuration.
>
> Deprecation
> -----------
>
> AF_ALG was originally intended to provide userspace programs access to crypto
> diff --git a/crypto/af_alg.c b/crypto/af_alg.c
> index cce000e8590e..34b801568fba 100644
> --- a/crypto/af_alg.c
> +++ b/crypto/af_alg.c
> @@ -6,10 +6,11 @@
> *
> * Copyright (c) 2010 Herbert Xu <herbert@gondor.apana.org.au>
> */
>
> #include <linux/atomic.h>
> +#include <linux/capability.h>
> #include <crypto/if_alg.h>
> #include <linux/crypto.h>
> #include <linux/init.h>
> #include <linux/kernel.h>
> #include <linux/key.h>
> @@ -20,14 +21,32 @@
> #include <linux/rwsem.h>
> #include <linux/sched.h>
> #include <linux/sched/signal.h>
> #include <linux/security.h>
> #include <linux/string.h>
> +#include <linux/sysctl.h>
> +#include <linux/user_namespace.h>
> #include <keys/user-type.h>
> #include <keys/trusted-type.h>
> #include <keys/encrypted-type.h>
>
> +static int af_alg_restrict = 1;
> +
> +static const struct ctl_table af_alg_table[] = {
> + {
> + .procname = "af_alg_restrict",
> + .data = &af_alg_restrict,
> + .maxlen = sizeof(int),
> + .mode = 0644,
> + .proc_handler = proc_dointvec_minmax,
> + .extra1 = SYSCTL_ZERO,
> + .extra2 = SYSCTL_TWO,
> + },
> +};
> +
> +static struct ctl_table_header *af_alg_header;
> +
> struct alg_type_list {
> const struct af_alg_type *type;
> struct list_head list;
> };
>
> @@ -108,10 +127,43 @@ int af_alg_unregister_type(const struct af_alg_type *type)
>
> return err;
> }
> EXPORT_SYMBOL_GPL(af_alg_unregister_type);
>
> +static bool af_alg_capable(void)
> +{
> + return ns_capable_noaudit(&init_user_ns, CAP_NET_ADMIN) ||
> + capable(CAP_SYS_ADMIN);
> +}
> +
> +int af_alg_check_restriction(const char *name,
> + const struct af_alg_allowlist_entry allowlist[])
> +{
> + int level = READ_ONCE(af_alg_restrict);
> +
> + if (level == 0)
> + return 0;
> + if (level == 1) {
> + for (const struct af_alg_allowlist_entry *ent = allowlist;
> + ent->name; ent++) {
> + if (strcmp(name, ent->name) == 0 &&
> + (!ent->privileged || af_alg_capable()))
> + return 0;
> + }
> + }
> + /*
> + * Use -ENOENT (the error code for "algorithm not found") instead of
> + * -EACCES or -EPERM, for the highest chance of correctly triggering
> + * fallback code paths in userspace programs.
> + *
> + * Don't log a warning, since it would be noisy. iwd tries to bind a
> + * bunch of algorithms that it never uses.
> + */
> + return -ENOENT;
> +}
> +EXPORT_SYMBOL_GPL(af_alg_check_restriction);
> +
> static void alg_do_release(const struct af_alg_type *type, void *private)
> {
> if (!type)
> return;
>
> @@ -504,10 +556,13 @@ static int alg_create(struct net *net, struct socket *sock, int protocol,
> int kern)
> {
> struct sock *sk;
> int err;
>
> + if (READ_ONCE(af_alg_restrict) == 2)
> + return -EAFNOSUPPORT;
> +
> if (sock->type != SOCK_SEQPACKET)
> return -ESOCKTNOSUPPORT;
> if (protocol != 0)
> return -EPROTONOSUPPORT;
>
> @@ -1220,31 +1275,36 @@ int af_alg_get_rsgl(struct sock *sk, struct msghdr *msg, int flags,
> }
> EXPORT_SYMBOL_GPL(af_alg_get_rsgl);
>
> static int __init af_alg_init(void)
> {
> - int err = proto_register(&alg_proto, 0);
> + int err;
> +
> + af_alg_header = register_sysctl("crypto", af_alg_table);
>
> + err = proto_register(&alg_proto, 0);
> if (err)
> - goto out;
> + goto out_unregister_sysctl;
>
> err = sock_register(&alg_family);
> - if (err != 0)
> + if (err)
> goto out_unregister_proto;
>
> -out:
> - return err;
> + return 0;
>
> out_unregister_proto:
> proto_unregister(&alg_proto);
> - goto out;
> +out_unregister_sysctl:
> + unregister_sysctl_table(af_alg_header);
> + return err;
> }
>
> static void __exit af_alg_exit(void)
> {
> sock_unregister(PF_ALG);
> proto_unregister(&alg_proto);
> + unregister_sysctl_table(af_alg_header);
> }
>
> module_init(af_alg_init);
> module_exit(af_alg_exit);
> MODULE_DESCRIPTION("Crypto userspace interface");
> diff --git a/crypto/algif_aead.c b/crypto/algif_aead.c
> index 787aac8aeb24..b9217f9086aa 100644
> --- a/crypto/algif_aead.c
> +++ b/crypto/algif_aead.c
> @@ -32,10 +32,15 @@
> #include <linux/mm.h>
> #include <linux/module.h>
> #include <linux/net.h>
> #include <net/sock.h>
>
> +static const struct af_alg_allowlist_entry aead_allowlist[] = {
> + { "ccm(aes)", true }, /* bluez */
> + {},
> +};
> +
> static inline bool aead_sufficient_data(struct sock *sk)
> {
> struct alg_sock *ask = alg_sk(sk);
> struct sock *psk = ask->parent;
> struct alg_sock *pask = alg_sk(psk);
> @@ -342,10 +347,16 @@ static struct proto_ops algif_aead_ops_nokey = {
> .poll = af_alg_poll,
> };
>
> static void *aead_bind(const char *name)
> {
> + int err;
> +
> + err = af_alg_check_restriction(name, aead_allowlist);
> + if (err)
> + return ERR_PTR(err);
> +
> return crypto_alloc_aead(name, 0, AF_ALG_CRYPTOAPI_MASK);
> }
>
> static void aead_release(void *private)
> {
> diff --git a/crypto/algif_hash.c b/crypto/algif_hash.c
> index 5452ad6c1506..a8d958d51ece 100644
> --- a/crypto/algif_hash.c
> +++ b/crypto/algif_hash.c
> @@ -14,10 +14,28 @@
> #include <linux/mm.h>
> #include <linux/module.h>
> #include <linux/net.h>
> #include <net/sock.h>
>
> +static const struct af_alg_allowlist_entry hash_allowlist[] = {
> + { "cmac(aes)", true }, /* iwd, bluez */
> + { "hmac(md5)", true }, /* iwd */
> + { "hmac(sha1)", true }, /* iwd */
> + { "hmac(sha224)", true }, /* iwd */
> + { "hmac(sha256)", true }, /* iwd */
> + { "hmac(sha384)", true }, /* iwd */
> + { "hmac(sha512)", true }, /* iwd, sha512hmac */
> + { "md4", true }, /* iwd */
> + { "md5", true }, /* iwd */
> + { "sha1", false }, /* iwd, iproute2 < 7.0 */
> + { "sha224", true }, /* iwd */
> + { "sha256", true }, /* iwd */
> + { "sha384", true }, /* iwd */
> + { "sha512", true }, /* iwd */
> + {},
In OpenWrt, https://gitlab.com/linux-afs/kafs-client and strongswan
seem to be the other users of the user API. I haven't looked into what
they need.
> +};
> +
> struct hash_ctx {
> struct af_alg_sgl sgl;
>
> u8 *result;
>
> @@ -380,10 +398,16 @@ static struct proto_ops algif_hash_ops_nokey = {
> .accept = hash_accept_nokey,
> };
>
> static void *hash_bind(const char *name)
> {
> + int err;
> +
> + err = af_alg_check_restriction(name, hash_allowlist);
> + if (err)
> + return ERR_PTR(err);
> +
> return crypto_alloc_ahash(name, 0, AF_ALG_CRYPTOAPI_MASK);
> }
>
> static void hash_release(void *private)
> {
> diff --git a/crypto/algif_rng.c b/crypto/algif_rng.c
> index 4dfe7899f8fa..bd522915d56d 100644
> --- a/crypto/algif_rng.c
> +++ b/crypto/algif_rng.c
> @@ -48,10 +48,14 @@
>
> MODULE_LICENSE("GPL");
> MODULE_AUTHOR("Stephan Mueller <smueller@chronox.de>");
> MODULE_DESCRIPTION("User-space interface for random number generators");
>
> +static const struct af_alg_allowlist_entry rng_allowlist[] = {
> + {},
> +};
> +
> struct rng_ctx {
> #define MAXSIZE 128
> unsigned int len;
> struct crypto_rng *drng;
> u8 *addtl;
> @@ -199,10 +203,15 @@ static struct proto_ops __maybe_unused algif_rng_test_ops = {
>
> static void *rng_bind(const char *name)
> {
> struct rng_parent_ctx *pctx;
> struct crypto_rng *rng;
> + int err;
> +
> + err = af_alg_check_restriction(name, rng_allowlist);
> + if (err)
> + return ERR_PTR(err);
>
> pctx = kzalloc_obj(*pctx);
> if (!pctx)
> return ERR_PTR(-ENOMEM);
>
> diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c
> index df20bdfe1f1f..2b8069667974 100644
> --- a/crypto/algif_skcipher.c
> +++ b/crypto/algif_skcipher.c
> @@ -32,10 +32,24 @@
> #include <linux/mm.h>
> #include <linux/module.h>
> #include <linux/net.h>
> #include <net/sock.h>
>
> +static const struct af_alg_allowlist_entry skcipher_allowlist[] = {
> + { "adiantum(xchacha12,aes)", false }, /* cryptsetup */
> + { "adiantum(xchacha20,aes)", false }, /* cryptsetup */
> + { "cbc(aes)", true }, /* iwd */
> + { "cbc(des)", true }, /* iwd */
> + { "cbc(des3_ede)", true }, /* iwd */
> + { "ctr(aes)", true }, /* iwd */
> + { "ecb(aes)", true }, /* iwd, bluez */
> + { "ecb(des)", true }, /* iwd */
> + { "hctr2(aes)", false }, /* cryptsetup */
> + { "xts(aes)", false }, /* cryptsetup benchmark */
> + {},
> +};
> +
> static int skcipher_sendmsg(struct socket *sock, struct msghdr *msg,
> size_t size)
> {
> struct sock *sk = sock->sk;
> struct alg_sock *ask = alg_sk(sk);
> @@ -307,10 +321,16 @@ static struct proto_ops algif_skcipher_ops_nokey = {
> .poll = af_alg_poll,
> };
>
> static void *skcipher_bind(const char *name)
> {
> + int err;
> +
> + err = af_alg_check_restriction(name, skcipher_allowlist);
> + if (err)
> + return ERR_PTR(err);
> +
> return crypto_alloc_skcipher(name, 0, AF_ALG_CRYPTOAPI_MASK);
> }
>
> static void skcipher_release(void *private)
> {
> diff --git a/include/crypto/if_alg.h b/include/crypto/if_alg.h
> index 7643ba954125..4e9ed8e73403 100644
> --- a/include/crypto/if_alg.h
> +++ b/include/crypto/if_alg.h
> @@ -159,13 +159,21 @@ struct af_alg_ctx {
> unsigned int len;
>
> unsigned int inflight;
> };
>
> +struct af_alg_allowlist_entry {
> + const char *name;
> + bool privileged;
> +};
> +
> int af_alg_register_type(const struct af_alg_type *type);
> int af_alg_unregister_type(const struct af_alg_type *type);
>
> +int af_alg_check_restriction(const char *name,
> + const struct af_alg_allowlist_entry allowlist[]);
> +
> int af_alg_release(struct socket *sock);
> void af_alg_release_parent(struct sock *sk);
> int af_alg_accept(struct sock *sk, struct socket *newsock,
> struct proto_accept_arg *arg);
>
>
> base-commit: 1dc18801be29bc54709aa355b8acd80e183b03cd
> --
> 2.54.0
>
>
^ permalink raw reply
* Re: [PATCH 2/3] dt-bindings: hwmon: pmbus: Support for onsemi's FD5121
From: Conor Dooley @ 2026-06-23 21:14 UTC (permalink / raw)
To: Selvamani Rajagopal
Cc: Guenter Roeck, Jonathan Corbet, Shuah Khan, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, linux-hwmon@vger.kernel.org,
linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
devicetree@vger.kernel.org
In-Reply-To: <CYYPR02MB98280DF78A07EADACFD084EE83EE2@CYYPR02MB9828.namprd02.prod.outlook.com>
[-- Attachment #1: Type: text/plain, Size: 1717 bytes --]
On Tue, Jun 23, 2026 at 09:01:32PM +0000, Selvamani Rajagopal wrote:
>
> > -----Original Message-----
> > From: Conor Dooley <conor@kernel.org>
> > Subject: Re: [PATCH 2/3] dt-bindings: hwmon: pmbus: Support for onsemi's FD5121
> >
> >
> > > +
> > > +title: onsemi's multi-phase digital controllers
> >
> > Can someone explain to me what a "digital controller" actually is?
> > Seems very generi and that a word may have been left out, were it not
> > for the fact that this wording is used several times in the patch.
> >
>
> Thanks for reviewing.
>
> According to me, "digital controller" means the controller uses digital circuits to implement
> the features and functionality. We can remove "digital" and keep only controller. It won't make any
> difference for Linux documentation.
My point is that what's actually being controlled is missing. Maybe it
is obvious to you, but it is not to me. Your nodename in your example is
> + fd5121@50 {
which doesn't comply with node naming requirements and I wanted to come
up with a suggestion for what it should be.
I am assuming that its power or voltage that you're controlling so
either it should be hwmon@ or regulator@.
>
> > > +
> > > + enum:
> > > + - onnn,fd5121
> > > + - onnn,fd5123
> > > + - onnn,fd5125
> >
> > Your /OF/ match data in your driver suggests that you intended to permit
> > fallback compatibles here?
>
> Agree. Sorry about the discrepancy. Will fix it.
>
> >
> > |+#ifdef CONFIG_OF
> > |+static const struct of_device_id fd5121_of_match[] = {
> > |+ { .compatible = "onnn,fd5121" },
> > |+ { }
> > |+};
> > |+MODULE_DEVICE_TABLE(of, fd5121_of_match);
> > |+#endif
> >
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH v2] usbcore: Add quirk for 255-bytes initial config read
From: Nikhil Solanke @ 2026-06-23 21:14 UTC (permalink / raw)
To: Alan Stern
Cc: linux-usb, gregkh, linux-kernel, michal.pecio, stable, corbet,
skhan, linux-doc
In-Reply-To: <567e8866-4308-4e5f-819c-fe778dbf74f8@rowland.harvard.edu>
> Moving this delay up here changes the behavior when the quirk flag isn't
> set. While it agrees with the intention of the USB_QUIRK_DELAY_INIT
> flag, such a change should be mentioned in the patch description.
How should I mention it then? Nothing comes to mind besides the
obvious: "Also move the USB_QUIRK_DELAY_INIT sleep to before the
initial descriptor read, so the delay applies consistently regardless
of whether USB_QUIRK_CONFIG_SIZE is set.". Or should i revert it back
to original position?
> > +
> > + /*
> > + * Grab just the first descriptor so we know how long the whole
> > + * configuration is. In case of quirky firmware, try to grab the
> > + * whole thing in one go by asking for a 255-bytes sized buffer
> > + * mirroring Windows behavior.
> > + */
>
> This needs to be rewritten, as it is self-contradictory. When the quirk
> flag is set we issue a 255-byte request to mimic the Windows behavior,
> and only when the flag isn't set do we grab just the first descriptor.
I am sorry I didn't understand how it is self contradictory. The
comment does say, "in case of quirky firmware..."? Am i missing
something?
> > result = usb_get_descriptor(dev, USB_DT_CONFIG, cfgno,
> > - desc, USB_DT_CONFIG_SIZE);
> > + desc, usb_config_req_size);
>
> Don't make extraneous changes to the existing indentation (or whitespace
> in general), here and below.
Well the linux coding style guidelines mention that those descendants
should preferably be aligned with the function open parenthesis. Since
i did "touch" that line/part of code I though might as well indent it
a bit accordingly. Should i revert the indent then (in this and the
other place)?
> > if (result != -EPIPE)
> > goto err;
> > dev_notice(ddev, "chopping to %d config(s)\n", cfgno);
> > @@ -957,13 +976,25 @@ int usb_get_configuration(struct usb_device *dev)
> > break;
> > } else if (result < 4) {
> > dev_err(ddev, "config index %d descriptor too short "
> > - "(expected %i, got %i)\n", cfgno,
> > - USB_DT_CONFIG_SIZE, result);
> > + "(asked for %zu, got %i, expected at least %i)\n",
> > + cfgno, usb_config_req_size, result, 4);
> > result = -EINVAL;
> > goto err;
> > }
> > +
> > length = max_t(int, le16_to_cpu(desc->wTotalLength),
> > - USB_DT_CONFIG_SIZE);
> > + USB_DT_CONFIG_SIZE);
>
> This is another example of a change that has nothing to do with the
> purpose of the patch.
Isn't that what you told me to change? So the logs are accurate? I
made that change because you suggested it. :')
> > +
> > + /*
> > + * If the device returns the full length configuration
> > + * descriptor, skip the second read. Otherwise, send a second
>
> Strictly speaking, the configuration descriptor is only 9 bytes long.
> What you mean here is the entire configuration descriptor set.
Alright i'll reword it.
> > + * request asking for the full length.
> > + */
> > + if (result >= le16_to_cpu(desc->wTotalLength)) {
>
> Shouldn't this be: result >= length? No point in repeating the
> le16_to_cpu calculation.
Yess. initially the length assignment was happening afterwards in my
patch. then i decided to move it before the "if" statement since the
outcome of length was going to be similar in any case (within if and
after if). but then i forgot to modify the if too. Will fix it.
> Like above, this string should all be on one line.
Will fix all the strings as well
Nikhil Solanke
^ permalink raw reply
* RE: [PATCH 2/3] dt-bindings: hwmon: pmbus: Support for onsemi's FD5121
From: Selvamani Rajagopal @ 2026-06-23 21:01 UTC (permalink / raw)
To: Conor Dooley
Cc: Guenter Roeck, Jonathan Corbet, Shuah Khan, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, linux-hwmon@vger.kernel.org,
linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
devicetree@vger.kernel.org
In-Reply-To: <20260623-anybody-gutter-e6ca04f53bdb@spud>
> -----Original Message-----
> From: Conor Dooley <conor@kernel.org>
> Subject: Re: [PATCH 2/3] dt-bindings: hwmon: pmbus: Support for onsemi's FD5121
>
>
> > +
> > +title: onsemi's multi-phase digital controllers
>
> Can someone explain to me what a "digital controller" actually is?
> Seems very generi and that a word may have been left out, were it not
> for the fact that this wording is used several times in the patch.
>
Thanks for reviewing.
According to me, "digital controller" means the controller uses digital circuits to implement
the features and functionality. We can remove "digital" and keep only controller. It won't make any
difference for Linux documentation.
> > +
> > + enum:
> > + - onnn,fd5121
> > + - onnn,fd5123
> > + - onnn,fd5125
>
> Your /OF/ match data in your driver suggests that you intended to permit
> fallback compatibles here?
Agree. Sorry about the discrepancy. Will fix it.
>
> |+#ifdef CONFIG_OF
> |+static const struct of_device_id fd5121_of_match[] = {
> |+ { .compatible = "onnn,fd5121" },
> |+ { }
> |+};
> |+MODULE_DEVICE_TABLE(of, fd5121_of_match);
> |+#endif
>
^ 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