Re: [PATCH 05/14] tracefs: replace call_rcu by kfree_rcu for simple kmem_cache_free callback

[Greg KH]

On Mon, Jun 10, 2024 at 11:40:54PM +0200, Vlastimil Babka wrote:
> On 6/10/24 10:36 PM, Steven Rostedt wrote:
> > On Mon, 10 Jun 2024 08:46:42 -0700
> > "Paul E. McKenney"  wrote:
> > 
> >> > > index 7c29f4afc23d..338c52168e61 100644
> >> > > --- a/fs/tracefs/inode.c
> >> > > +++ b/fs/tracefs/inode.c
> >> > > @@ -53,14 +53,6 @@ static struct inode *tracefs_alloc_inode(struct 
> >> > > super_block *sb)
> >> > >return &ti->vfs_inode;
> >> > >  }
> >> > >  
> >> > > -static void tracefs_free_inode_rcu(struct rcu_head *rcu)
> >> > > -{
> >> > > -  struct tracefs_inode *ti;
> >> > > -
> >> > > -  ti = container_of(rcu, struct tracefs_inode, rcu);
> >> > > -  kmem_cache_free(tracefs_inode_cachep, ti);  
> >> > 
> >> > Does this work?
> >> > 
> >> > tracefs needs to be freed via the tracefs_inode_cachep. Does
> >> > kfree_rcu() handle specific frees for objects that were not allocated
> >> > via kmalloc()?  
> >> 
> >> A recent change to kfree() allows it to correctly handle memory allocated
> >> via kmem_cache_alloc().  News to me as of a few weeks ago.  ;-)
> > 
> > If that's the case then:
> > 
> > Acked-by: Steven Rostedt (Google) 
> > 
> > Do we have a way to add a "Depends-on" tag so that anyone backporting this
> > will know that it requires the change to whatever allowed that to happen?
> 
> Looks like people use that tag, although no grep hits in Documentation, so
> Cc'ing workflows@ and Thorsten.
> 
> In this case it would be
> 
> Depends-on: c9929f0e344a ("mm/slob: remove CONFIG_SLOB")

Ick, no, use the documented way of handling this as described in the
stable kernel rules file.

thanks,

greg k-h

Re: [PATCH 2/3] tracing/kprobe: Remove unneeded WARN_ON_ONCE() in selftests

[Google]

On Mon, 10 Jun 2024 20:18:13 -0400
Steven Rostedt  wrote:

> On Tue, 11 Jun 2024 06:26:44 +0900
> "Masami Hiramatsu (Google)"  wrote:
> 
> > From: Masami Hiramatsu (Google) 
> > 
> > Since the kprobe-events selftest shows OK or NG with the reason, the
> > WARN_ON_ONCE()s for each place are redundant. Let's remove it.
> > 
> > Signed-off-by: Masami Hiramatsu (Google) 
> > ---
> >  kernel/trace/trace_kprobe.c |   26 +-
> >  1 file changed, 13 insertions(+), 13 deletions(-)
> > 
> > diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> > index 16383247bdbf..4abed36544d0 100644
> > --- a/kernel/trace/trace_kprobe.c
> > +++ b/kernel/trace/trace_kprobe.c
> > @@ -2023,18 +2023,18 @@ static __init int kprobe_trace_self_tests_init(void)
> > pr_info("Testing kprobe tracing: ");
> >  
> > ret = create_or_delete_trace_kprobe("p:testprobe 
> > kprobe_trace_selftest_target $stack $stack0 +0($stack)");
> > -   if (WARN_ON_ONCE(ret)) {
> > +   if (ret) {
> > pr_warn("error on probing function entry.\n");
> 
> Actually, you can consolidate this to:
> 
>   if (WARN_ONCE(ret, "error on probing function entry."))

Ahh, OK, let me update it.

> 
> > warn++;
> > } else {
> > /* Enable trace point */
> > tk = find_trace_kprobe("testprobe", KPROBE_EVENT_SYSTEM);
> > -   if (WARN_ON_ONCE(tk == NULL)) {
> > +   if (tk == NULL) {
> > pr_warn("error on getting new probe.\n");
> 
> And this to:
> 
>   if (WARN_ONCE(tk == NULL, "error on getting new probe."))

Thank you!

> 
> end so on.
> 
> -- Steve
> 
> > warn++;
> > } else {
> > file = find_trace_probe_file(tk, top_trace_array());
> > -   if (WARN_ON_ONCE(file == NULL)) {
> > +   if (file == NULL) {
> > pr_warn("error on getting probe file.\n");
> > warn++;
> > } else
> > @@ -2044,18 +2044,18 @@ static __init int kprobe_trace_self_tests_init(void)
> > }
> >  
> > ret = create_or_delete_trace_kprobe("r:testprobe2 
> > kprobe_trace_selftest_target $retval");
> > -   if (WARN_ON_ONCE(ret)) {
> > +   if (ret) {
> > pr_warn("error on probing function return.\n");
> > warn++;
> > } else {
> > /* Enable trace point */
> > tk = find_trace_kprobe("testprobe2", KPROBE_EVENT_SYSTEM);
> > -   if (WARN_ON_ONCE(tk == NULL)) {
> > +   if (tk == NULL) {
> > pr_warn("error on getting 2nd new probe.\n");
> > warn++;
> > } else {
> > file = find_trace_probe_file(tk, top_trace_array());
> > -   if (WARN_ON_ONCE(file == NULL)) {
> > +   if (file == NULL) {
> > pr_warn("error on getting probe file.\n");
> > warn++;
> > } else
> > @@ -2079,7 +2079,7 @@ static __init int kprobe_trace_self_tests_init(void)
> >  
> > /* Disable trace points before removing it */
> > tk = find_trace_kprobe("testprobe", KPROBE_EVENT_SYSTEM);
> > -   if (WARN_ON_ONCE(tk == NULL)) {
> > +   if (tk == NULL) {
> > pr_warn("error on getting test probe.\n");
> > warn++;
> > } else {
> > @@ -2089,7 +2089,7 @@ static __init int kprobe_trace_self_tests_init(void)
> > }
> >  
> > file = find_trace_probe_file(tk, top_trace_array());
> > -   if (WARN_ON_ONCE(file == NULL)) {
> > +   if (file == NULL) {
> > pr_warn("error on getting probe file.\n");
> > warn++;
> > } else
> > @@ -2098,7 +2098,7 @@ static __init int kprobe_trace_self_tests_init(void)
> > }
> >  
> > tk = find_trace_kprobe("testprobe2", KPROBE_EVENT_SYSTEM);
> > -   if (WARN_ON_ONCE(tk == NULL)) {
> > +   if (tk == NULL) {
> > pr_warn("error on getting 2nd test probe.\n");
> > warn++;
> > } else {
> > @@ -2108,7 +2108,7 @@ static __init int kprobe_trace_self_tests_init(void)
> > }
> >  
> > file = find_trace_probe_file(tk, top_trace_array());
> > -   if (WARN_ON_ONCE(file == NULL)) {
> > +   if (file == NULL) {
> > pr_warn("error on getting probe file.\n");
> > warn++;
> > } else
> > @@ -2117,20 +2117,20 @@ static __init int kprobe_trace_self_tests_init(void)
> > }
> >  
> > ret = create_or_delete_trace_kprobe("-:testprobe");
> > -   if (WARN_ON_ONCE(ret)) {
> > +   if (ret) {
> > pr_warn("error on deleting a probe.\n");
> > warn++;
> > }
> >  
> > ret = create_or_delete_trace_kprobe("-:testprobe2");
> > -   if (WARN_ON_ONCE(ret)) {
> > +   if (ret) {
> > pr_warn("error on deleting a probe.\n");
> > 

[PATCH 2/2] vdpa_sim_net: Add the support of set mac address

[Cindy Lu]

Add the function to support setting the MAC address.
For vdpa_sim_net, the driver will write the MAC address
to the config space, and other devices can implement
their own functions to support this.

Signed-off-by: Cindy Lu 
---
 drivers/vdpa/vdpa_sim/vdpa_sim_net.c | 18 +-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim_net.c 
b/drivers/vdpa/vdpa_sim/vdpa_sim_net.c
index cfe962911804..e961a08341c2 100644
--- a/drivers/vdpa/vdpa_sim/vdpa_sim_net.c
+++ b/drivers/vdpa/vdpa_sim/vdpa_sim_net.c
@@ -414,6 +414,21 @@ static void vdpasim_net_get_config(struct vdpasim 
*vdpasim, void *config)
net_config->status = cpu_to_vdpasim16(vdpasim, VIRTIO_NET_S_LINK_UP);
 }
 
+static int vdpasim_net_set_mac(struct vdpa_mgmt_dev *mdev,
+  struct vdpa_device *dev,
+  const struct vdpa_dev_set_config *config)
+{
+   struct vdpasim *vdpasim = container_of(dev, struct vdpasim, vdpa);
+
+   struct virtio_net_config *vio_config = vdpasim->config;
+
+   if (config->mask & (1 << VDPA_ATTR_DEV_NET_CFG_MACADDR)) {
+   memcpy(vio_config->mac, config->net.mac, ETH_ALEN);
+   return 0;
+   }
+   return -EINVAL;
+}
+
 static void vdpasim_net_setup_config(struct vdpasim *vdpasim,
 const struct vdpa_dev_set_config *config)
 {
@@ -510,7 +525,8 @@ static void vdpasim_net_dev_del(struct vdpa_mgmt_dev *mdev,
 
 static const struct vdpa_mgmtdev_ops vdpasim_net_mgmtdev_ops = {
.dev_add = vdpasim_net_dev_add,
-   .dev_del = vdpasim_net_dev_del
+   .dev_del = vdpasim_net_dev_del,
+   .set_mac = vdpasim_net_set_mac
 };
 
 static struct virtio_device_id id_table[] = {
-- 
2.45.0

[PATCH 1/2] vdpa: support set mac address from vdpa tool

[Cindy Lu]

Add new UAPI to support the mac address from vdpa tool
Function vdpa_nl_cmd_dev_config_set_doit() will get the
MAC address from the vdpa tool and then set it to the device.

The usage is: vdpa dev set name vdpa_name mac **:**:**:**:**:**

Here is sample:
root@L1# vdpa -jp dev config show vdpa0
{
"config": {
"vdpa0": {
"mac": "82:4d:e9:5d:d7:e6",
"link ": "up",
"link_announce ": false,
"mtu": 1500
}
}
}

root@L1# vdpa dev set name vdpa0 mac 00:11:22:33:44:55

root@L1# vdpa -jp dev config show vdpa0
{
"config": {
"vdpa0": {
"mac": "00:11:22:33:44:55",
"link ": "up",
"link_announce ": false,
"mtu": 1500
}
}
}

Signed-off-by: Cindy Lu 
---
 drivers/vdpa/vdpa.c   | 71 +++
 include/linux/vdpa.h  |  2 ++
 include/uapi/linux/vdpa.h |  1 +
 3 files changed, 74 insertions(+)

diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c
index a7612e0783b3..347ae6e7749d 100644
--- a/drivers/vdpa/vdpa.c
+++ b/drivers/vdpa/vdpa.c
@@ -1149,6 +1149,72 @@ static int vdpa_nl_cmd_dev_config_get_doit(struct 
sk_buff *skb, struct genl_info
return err;
 }
 
+static int vdpa_nl_cmd_dev_config_set_doit(struct sk_buff *skb,
+  struct genl_info *info)
+{
+   struct vdpa_dev_set_config set_config = {};
+   struct nlattr **nl_attrs = info->attrs;
+   struct vdpa_mgmt_dev *mdev;
+   const u8 *macaddr;
+   const char *name;
+   int err = 0;
+   struct device *dev;
+   struct vdpa_device *vdev;
+
+   if (!info->attrs[VDPA_ATTR_DEV_NAME])
+   return -EINVAL;
+
+   name = nla_data(info->attrs[VDPA_ATTR_DEV_NAME]);
+
+   down_write(&vdpa_dev_lock);
+   dev = bus_find_device(&vdpa_bus, NULL, name, vdpa_name_match);
+   if (!dev) {
+   NL_SET_ERR_MSG_MOD(info->extack, "device not found");
+   err = -ENODEV;
+   goto dev_err;
+   }
+   vdev = container_of(dev, struct vdpa_device, dev);
+   if (!vdev->mdev) {
+   NL_SET_ERR_MSG_MOD(
+   info->extack,
+   "Fail to find the specified management device");
+   err = -EINVAL;
+   goto mdev_err;
+   }
+   mdev = vdev->mdev;
+   if (nl_attrs[VDPA_ATTR_DEV_NET_CFG_MACADDR]) {
+   if (!(mdev->supported_features & BIT_ULL(VIRTIO_NET_F_MAC))) {
+   NL_SET_ERR_MSG_FMT_MOD(
+   info->extack,
+   "Missing features 0x%llx for provided 
attributes",
+   BIT_ULL(VIRTIO_NET_F_MAC));
+   err = -EINVAL;
+   goto mdev_err;
+   }
+   macaddr = nla_data(nl_attrs[VDPA_ATTR_DEV_NET_CFG_MACADDR]);
+   memcpy(set_config.net.mac, macaddr, ETH_ALEN);
+   set_config.mask |= BIT_ULL(VDPA_ATTR_DEV_NET_CFG_MACADDR);
+   if (mdev->ops->set_mac) {
+   err = mdev->ops->set_mac(mdev, vdev, &set_config);
+   } else {
+   NL_SET_ERR_MSG_FMT_MOD(
+   info->extack,
+   "%s device not support set mac address ", name);
+   }
+
+   } else {
+   NL_SET_ERR_MSG_FMT_MOD(info->extack,
+  "%s device not support this config ",
+  name);
+   }
+
+mdev_err:
+   put_device(dev);
+dev_err:
+   up_write(&vdpa_dev_lock);
+   return err;
+}
+
 static int vdpa_dev_config_dump(struct device *dev, void *data)
 {
struct vdpa_device *vdev = container_of(dev, struct vdpa_device, dev);
@@ -1285,6 +1351,11 @@ static const struct genl_ops vdpa_nl_ops[] = {
.doit = vdpa_nl_cmd_dev_stats_get_doit,
.flags = GENL_ADMIN_PERM,
},
+   {
+   .cmd = VDPA_CMD_DEV_CONFIG_SET,
+   .doit = vdpa_nl_cmd_dev_config_set_doit,
+   .flags = GENL_ADMIN_PERM,
+   },
 };
 
 static struct genl_family vdpa_nl_family __ro_after_init = {
diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h
index db15ac07f8a6..c97f4f1da753 100644
--- a/include/linux/vdpa.h
+++ b/include/linux/vdpa.h
@@ -581,6 +581,8 @@ struct vdpa_mgmtdev_ops {
int (*dev_add)(struct vdpa_mgmt_dev *mdev, const char *name,
   const struct vdpa_dev_set_config *config);
void (*dev_del)(struct vdpa_mgmt_dev *mdev, struct vdpa_device *dev);
+   int (*set_mac)(struct vdpa_mgmt_dev *mdev, struct vdpa_device *dev,
+  const struct vdpa_dev_set_config *config);
 };
 
 /**
diff --git a/include/uapi/linux/vdpa.h b/include/uapi/linux/vdpa.h
index 54b649ab0f22..53f249fb26bc 100644
--- a/include/uapi/linux/vdpa.h
+++ b/include/

Re: [PATCH] livepatch: introduce klp_func called interface

[zhang warden]

> We don't have very urgent use for this. As we discussed, various tracing
> tools are sufficient in most cases. I brought this up in the context of the
> "called" entry: if we are really adding a new entry, let's do "counter"
> instead of "called".
> 
> Thanks,
> Song

Hi, Song

I hope to find a way to optimize "called" will be set once in klp_ftrace_ops 
function because as Petr said this function should be as fast as possible. But 
if use "count", this variable will be called every time klp_ftrace_ops run.

Regards,
Wardenjohn

Re: [RFC PATCH] ftrace: Skip __fentry__ location of overridden weak functions

[Zheng Yejian]

On 2024/6/7 23:02, Peter Zijlstra wrote:

On Fri, Jun 07, 2024 at 07:52:11PM +0800, Zheng Yejian wrote:

ftrace_location() was changed to not only return the __fentry__ location
when called for the __fentry__ location, but also when called for the
sym+0 location after commit aebfd12521d9 ("x86/ibt,ftrace: Search for
__fentry__ location"). That is, if sym+0 location is not __fentry__,
ftrace_location() would find one over the entire size of the sym.

However, there is case that more than one __fentry__ exist in the sym
range (described below) and ftrace_location() would find wrong __fentry__
location by binary searching, which would cause its users like livepatch/
kprobe/bpf to not work properly on this sym!

The case is that, based on current compiler behavior, suppose:
  - function A is followed by weak function B1 in same binary file;
  - weak function B1 is overridden by function B2;
Then in the final binary file:
  - symbol B1 will be removed from symbol table while its instructions are
not removed;
  - __fentry__ of B1 will be still in __mcount_loc table;
  - function size of A is computed by substracting the symbol address of
A from its next symbol address (see kallsyms_lookup_size_offset()),
but because symbol info of B1 is removed, the next symbol of A is
originally the next symbol of B1. See following example, function
sizeof A will be (symbol_address_C - symbol_address_A):

  symbol_address_A
  symbol_address_B1 (Not in symbol table)
  symbol_address_C

The weak function issue has been discovered in commit b39181f7c690
("ftrace: Add FTRACE_MCOUNT_MAX_OFFSET to avoid adding weak function")
but it didn't resolve the issue in ftrace_location().

There may be following resolutions:


Oh gawd, sodding weak functions again.

I would suggest changing scipts/kallsyms.c to emit readily identifiable
symbol names for all the weak junk, eg:

   __weak_junk_N



Sorry for the late reply, I just had a long noon holiday :>

scripts/kallsyms.c is compiled and used to handle symbols in vmlinux.o
or vmlinux.a, see kallsyms_step() in scripts/link-vmlinux.sh, those
overridden weak symbols has been removed from symbol table of vmlinux.o
or vmlinux.a. But we can found those symbols from original xx/xx.o file,
for example, the weak free_initmem() in in init/main.c is overridden,
its symbol is not in vmlinx but is still in init/main.o .

How about traversing all origin xx/xx.o and finding all weak junk symbols ?


That instantly fixes the immediate problem and Steve's horrid hack can
go away.



Yes, this can be done in same patch series.


Additionally, I would add a boot up pass that would INT3 fill all such
functions and remove/invalidate all
static_call/static_jump/fentry/alternative entry that is inside of them.





--

Thanks,
Zheng Yejian

Re: [PATCH 2/3] tracing/kprobe: Remove unneeded WARN_ON_ONCE() in selftests

[Steven Rostedt]

On Tue, 11 Jun 2024 06:26:44 +0900
"Masami Hiramatsu (Google)"  wrote:

> From: Masami Hiramatsu (Google) 
> 
> Since the kprobe-events selftest shows OK or NG with the reason, the
> WARN_ON_ONCE()s for each place are redundant. Let's remove it.
> 
> Signed-off-by: Masami Hiramatsu (Google) 
> ---
>  kernel/trace/trace_kprobe.c |   26 +-
>  1 file changed, 13 insertions(+), 13 deletions(-)
> 
> diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> index 16383247bdbf..4abed36544d0 100644
> --- a/kernel/trace/trace_kprobe.c
> +++ b/kernel/trace/trace_kprobe.c
> @@ -2023,18 +2023,18 @@ static __init int kprobe_trace_self_tests_init(void)
>   pr_info("Testing kprobe tracing: ");
>  
>   ret = create_or_delete_trace_kprobe("p:testprobe 
> kprobe_trace_selftest_target $stack $stack0 +0($stack)");
> - if (WARN_ON_ONCE(ret)) {
> + if (ret) {
>   pr_warn("error on probing function entry.\n");

Actually, you can consolidate this to:

if (WARN_ONCE(ret, "error on probing function entry."))

>   warn++;
>   } else {
>   /* Enable trace point */
>   tk = find_trace_kprobe("testprobe", KPROBE_EVENT_SYSTEM);
> - if (WARN_ON_ONCE(tk == NULL)) {
> + if (tk == NULL) {
>   pr_warn("error on getting new probe.\n");

And this to:

if (WARN_ONCE(tk == NULL, "error on getting new probe."))

end so on.

-- Steve

>   warn++;
>   } else {
>   file = find_trace_probe_file(tk, top_trace_array());
> - if (WARN_ON_ONCE(file == NULL)) {
> + if (file == NULL) {
>   pr_warn("error on getting probe file.\n");
>   warn++;
>   } else
> @@ -2044,18 +2044,18 @@ static __init int kprobe_trace_self_tests_init(void)
>   }
>  
>   ret = create_or_delete_trace_kprobe("r:testprobe2 
> kprobe_trace_selftest_target $retval");
> - if (WARN_ON_ONCE(ret)) {
> + if (ret) {
>   pr_warn("error on probing function return.\n");
>   warn++;
>   } else {
>   /* Enable trace point */
>   tk = find_trace_kprobe("testprobe2", KPROBE_EVENT_SYSTEM);
> - if (WARN_ON_ONCE(tk == NULL)) {
> + if (tk == NULL) {
>   pr_warn("error on getting 2nd new probe.\n");
>   warn++;
>   } else {
>   file = find_trace_probe_file(tk, top_trace_array());
> - if (WARN_ON_ONCE(file == NULL)) {
> + if (file == NULL) {
>   pr_warn("error on getting probe file.\n");
>   warn++;
>   } else
> @@ -2079,7 +2079,7 @@ static __init int kprobe_trace_self_tests_init(void)
>  
>   /* Disable trace points before removing it */
>   tk = find_trace_kprobe("testprobe", KPROBE_EVENT_SYSTEM);
> - if (WARN_ON_ONCE(tk == NULL)) {
> + if (tk == NULL) {
>   pr_warn("error on getting test probe.\n");
>   warn++;
>   } else {
> @@ -2089,7 +2089,7 @@ static __init int kprobe_trace_self_tests_init(void)
>   }
>  
>   file = find_trace_probe_file(tk, top_trace_array());
> - if (WARN_ON_ONCE(file == NULL)) {
> + if (file == NULL) {
>   pr_warn("error on getting probe file.\n");
>   warn++;
>   } else
> @@ -2098,7 +2098,7 @@ static __init int kprobe_trace_self_tests_init(void)
>   }
>  
>   tk = find_trace_kprobe("testprobe2", KPROBE_EVENT_SYSTEM);
> - if (WARN_ON_ONCE(tk == NULL)) {
> + if (tk == NULL) {
>   pr_warn("error on getting 2nd test probe.\n");
>   warn++;
>   } else {
> @@ -2108,7 +2108,7 @@ static __init int kprobe_trace_self_tests_init(void)
>   }
>  
>   file = find_trace_probe_file(tk, top_trace_array());
> - if (WARN_ON_ONCE(file == NULL)) {
> + if (file == NULL) {
>   pr_warn("error on getting probe file.\n");
>   warn++;
>   } else
> @@ -2117,20 +2117,20 @@ static __init int kprobe_trace_self_tests_init(void)
>   }
>  
>   ret = create_or_delete_trace_kprobe("-:testprobe");
> - if (WARN_ON_ONCE(ret)) {
> + if (ret) {
>   pr_warn("error on deleting a probe.\n");
>   warn++;
>   }
>  
>   ret = create_or_delete_trace_kprobe("-:testprobe2");
> - if (WARN_ON_ONCE(ret)) {
> + if (ret) {
>   pr_warn("error on deleting a probe.\n");
>   warn++;
>   }
>  
>  end:
>   ret = dyn_events_release_all(&trace_kprobe_ops);
> - if (WARN_ON_ONCE(ret)) {
> + if (ret) {
>   pr_warn("error on cleaning up probes.\

Re: [PATCH 2/3] tracing/kprobe: Remove unneeded WARN_ON_ONCE() in selftests

[Google]

On Mon, 10 Jun 2024 17:40:52 -0400
Steven Rostedt  wrote:

> On Tue, 11 Jun 2024 06:26:44 +0900
> "Masami Hiramatsu (Google)"  wrote:
> 
> > From: Masami Hiramatsu (Google) 
> > 
> > Since the kprobe-events selftest shows OK or NG with the reason, the
> > WARN_ON_ONCE()s for each place are redundant. Let's remove it.
> 
> Note, the ktests we run to validate commits, fail when it detects a WARN()
> triggered.
> 
> If this fails in any configuration, ktest will not detect it failed.

Hmm, I think there are 2 options,
 - remove pr_warn() instead. (WARN_ON_ONCE + pr_warn is redundant)
 - Or, remove WARN_ON_ONCE() from each place, but add WARN_ON_ONCE() when
   `warn` is not zero.

Thank you,

> 
> -- Steve
> 
> 
> > 
> > Signed-off-by: Masami Hiramatsu (Google) 
> > ---
> >  kernel/trace/trace_kprobe.c |   26 +-
> >  1 file changed, 13 insertions(+), 13 deletions(-)
> 


-- 
Masami Hiramatsu (Google) 

Re: [PATCH v14 14/14] selftests/sgx: Add scripts for EPC cgroup testing

[Huang, Kai]

--- a/arch/x86/kernel/cpu/sgx/main.c
+++ b/arch/x86/kernel/cpu/sgx/main.c
@@ -1045,7 +1045,7 @@ static int __init sgx_init(void)
   if (!sgx_page_cache_init())
   return -ENOMEM;

-if (!sgx_page_reclaimer_init()) {
+if (!sgx_page_reclaimer_init() || !sgx_cgroup_init()) {
   ret = -ENOMEM;
   goto err_page_cache;
   }


Does it make more sense to move the sgx_cgroup_init() to the 
sgx_drv_init()?  The SGX cgroup only works for the driver side anyway. 
In this case, if something went wrong in sgx_cgroup_init(), the 
sgx_vepc_init() could still have a chance to work.


And IIUC we need to reset the "capacity" to 0 if sgx_cgroup_init() 
fails, no matter it is called inside sgx_drv_init() or sgx_init(), 
otherwise the "epc" would appear in the cgroup hierarchy as a misc 
cgroup resource.


Another option is to defer setting the capacity to the point where we 
have made sure sgx_drv_init() and sgx_cgroup_init() cannot fail.


Btw, I plan to review the rest from late of this week or next week 
because this week I have some other staff needs to be finished first.

Re: [PATCH -next 2/2] ftrace: Add kernel-doc comments for unregister_ftrace_direct() function

[Steven Rostedt]

On Fri,  7 Jun 2024 16:49:57 +0800
Yang Li  wrote:

> Added kernel-doc comments for the unregister_ftrace_direct() function to
> improve code documentation and readability.
> 

Someone else beat you to this.

-- Steve

> Reported-by: Abaci Robot 
> Closes: https://bugzilla.openanolis.cn/show_bug.cgi?id=9300
> Signed-off-by: Yang Li 
> ---
>  kernel/trace/ftrace.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index 4aeb1183ea9f..3b0dbd55cc05 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -5988,6 +5988,8 @@ EXPORT_SYMBOL_GPL(register_ftrace_direct);
>   * unregister_ftrace_direct - Remove calls to custom trampoline
>   * previously registered by register_ftrace_direct for @ops object.
>   * @ops: The address of the struct ftrace_ops object
> + * @addr: The address of the direct call to remove
> + * @free_filters: Boolean indicating whether to free the filters
>   *
>   * This is used to remove a direct calls to @addr from the nop locations
>   * of the functions registered in @ops (with by ftrace_set_filter_ip

Re: [PATCH -next 1/2] function_graph: Add kernel-doc comments for ftrace_graph_ret_addr() function

[Steven Rostedt]

On Fri,  7 Jun 2024 16:49:56 +0800
Yang Li  wrote:

> Added kernel-doc comments for the ftrace_graph_ret_addr() function to
> improve code documentation and readability.
> 
> Reported-by: Abaci Robot 
> Closes: https://bugzilla.openanolis.cn/show_bug.cgi?id=9299
> Signed-off-by: Yang Li 
> ---
>  kernel/trace/fgraph.c | 6 ++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/kernel/trace/fgraph.c b/kernel/trace/fgraph.c
> index a13551a023aa..4ad33e4cb8da 100644
> --- a/kernel/trace/fgraph.c
> +++ b/kernel/trace/fgraph.c
> @@ -872,6 +872,12 @@ ftrace_graph_get_ret_stack(struct task_struct *task, int 
> idx)
>  /**
>   * ftrace_graph_ret_addr - convert a potentially modified stack return 
> address
>   *  to its original value
> + * @task: pointer to the task_struct of the task being examined
> + * @idx: pointer to a state variable, should be initialized to zero
> + *before the first call

parameter descriptions should not go across more than one line. At least
not in my code. Also, you don't need to add that it needs to be initialized
here. That belongs in the body.

And it's not a state variable. The description you got that from is wrong.

I'll go update it and give you a reported by, as the entire description
needs a rewrite.

-- Steve


> + * @ret: the current return address found on the stack
> + * @retp: pointer to the return address on the stack, ignored if
> + * HAVE_FUNCTION_GRAPH_RET_ADDR_PTR is not defined
>   *
>   * This function can be called by stack unwinding code to convert a found 
> stack
>   * return address ('ret') to its original value, in case the function graph

Re: [PATCHv7 bpf-next 2/9] uprobe: Wire up uretprobe system call

[Google]

On Thu, 23 May 2024 14:11:42 +0200
Jiri Olsa  wrote:

> Wiring up uretprobe system call, which comes in following changes.
> We need to do the wiring before, because the uretprobe implementation
> needs the syscall number.
> 
> Note at the moment uretprobe syscall is supported only for native
> 64-bit process.
> 

BTW, this does not cleanly applied to probes/for-next, based on
6.10-rc1. Which version did you use?

Thank you,

> Reviewed-by: Oleg Nesterov 
> Reviewed-by: Masami Hiramatsu (Google) 
> Acked-by: Andrii Nakryiko 
> Signed-off-by: Jiri Olsa 
> ---
>  arch/x86/entry/syscalls/syscall_64.tbl | 1 +
>  include/linux/syscalls.h   | 2 ++
>  include/uapi/asm-generic/unistd.h  | 5 -
>  kernel/sys_ni.c| 2 ++
>  4 files changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/entry/syscalls/syscall_64.tbl 
> b/arch/x86/entry/syscalls/syscall_64.tbl
> index cc78226ffc35..47dfea0a827c 100644
> --- a/arch/x86/entry/syscalls/syscall_64.tbl
> +++ b/arch/x86/entry/syscalls/syscall_64.tbl
> @@ -383,6 +383,7 @@
>  459  common  lsm_get_self_attr   sys_lsm_get_self_attr
>  460  common  lsm_set_self_attr   sys_lsm_set_self_attr
>  461  common  lsm_list_modulessys_lsm_list_modules
> +462  64  uretprobe   sys_uretprobe
>  
>  #
>  # Due to a historical design error, certain syscalls are numbered differently
> diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
> index e619ac10cd23..5318e0e76799 100644
> --- a/include/linux/syscalls.h
> +++ b/include/linux/syscalls.h
> @@ -972,6 +972,8 @@ asmlinkage long sys_lsm_list_modules(u64 *ids, u32 *size, 
> u32 flags);
>  /* x86 */
>  asmlinkage long sys_ioperm(unsigned long from, unsigned long num, int on);
>  
> +asmlinkage long sys_uretprobe(void);
> +
>  /* pciconfig: alpha, arm, arm64, ia64, sparc */
>  asmlinkage long sys_pciconfig_read(unsigned long bus, unsigned long dfn,
>   unsigned long off, unsigned long len,
> diff --git a/include/uapi/asm-generic/unistd.h 
> b/include/uapi/asm-generic/unistd.h
> index 75f00965ab15..8a747cd1d735 100644
> --- a/include/uapi/asm-generic/unistd.h
> +++ b/include/uapi/asm-generic/unistd.h
> @@ -842,8 +842,11 @@ __SYSCALL(__NR_lsm_set_self_attr, sys_lsm_set_self_attr)
>  #define __NR_lsm_list_modules 461
>  __SYSCALL(__NR_lsm_list_modules, sys_lsm_list_modules)
>  
> +#define __NR_uretprobe 462
> +__SYSCALL(__NR_uretprobe, sys_uretprobe)
> +
>  #undef __NR_syscalls
> -#define __NR_syscalls 462
> +#define __NR_syscalls 463
>  
>  /*
>   * 32 bit systems traditionally used different
> diff --git a/kernel/sys_ni.c b/kernel/sys_ni.c
> index faad00cce269..be6195e0d078 100644
> --- a/kernel/sys_ni.c
> +++ b/kernel/sys_ni.c
> @@ -391,3 +391,5 @@ COND_SYSCALL(setuid16);
>  
>  /* restartable sequence */
>  COND_SYSCALL(rseq);
> +
> +COND_SYSCALL(uretprobe);
> -- 
> 2.45.1
> 


-- 
Masami Hiramatsu (Google) 

Re: [PATCHv7 bpf-next 8/9] selftests/bpf: Add uretprobe shadow stack test

[Google]

On Thu, 23 May 2024 14:11:48 +0200
Jiri Olsa  wrote:

> Adding uretprobe shadow stack test that runs all existing
> uretprobe tests with shadow stack enabled if it's available.
> 

According to the document and sample code, this looks good to me.

Reviewed-by: Masami Hiramatsu (Google) 

Thanks,

> Signed-off-by: Jiri Olsa 
> ---
>  .../selftests/bpf/prog_tests/uprobe_syscall.c | 60 +++
>  1 file changed, 60 insertions(+)
> 
> diff --git a/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c 
> b/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c
> index 3ef324c2db50..fda456401284 100644
> --- a/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c
> +++ b/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c
> @@ -9,6 +9,9 @@
>  #include 
>  #include 
>  #include 
> +#include 
> +#include 
> +#include 
>  #include "uprobe_syscall.skel.h"
>  #include "uprobe_syscall_executed.skel.h"
>  
> @@ -297,6 +300,56 @@ static void test_uretprobe_syscall_call(void)
>   close(go[1]);
>   close(go[0]);
>  }
> +
> +/*
> + * Borrowed from tools/testing/selftests/x86/test_shadow_stack.c.
> + *
> + * For use in inline enablement of shadow stack.
> + *
> + * The program can't return from the point where shadow stack gets enabled
> + * because there will be no address on the shadow stack. So it can't use
> + * syscall() for enablement, since it is a function.
> + *
> + * Based on code from nolibc.h. Keep a copy here because this can't pull
> + * in all of nolibc.h.
> + */
> +#define ARCH_PRCTL(arg1, arg2)   \
> +({   \
> + long _ret;  \
> + register long _num  asm("eax") = __NR_arch_prctl;   \
> + register long _arg1 asm("rdi") = (long)(arg1);  \
> + register long _arg2 asm("rsi") = (long)(arg2);  \
> + \
> + asm volatile (  \
> + "syscall\n" \
> + : "=a"(_ret)\
> + : "r"(_arg1), "r"(_arg2),   \
> +   "0"(_num) \
> + : "rcx", "r11", "memory", "cc"  \
> + );  \
> + _ret;   \
> +})
> +
> +#ifndef ARCH_SHSTK_ENABLE
> +#define ARCH_SHSTK_ENABLE0x5001
> +#define ARCH_SHSTK_DISABLE   0x5002
> +#define ARCH_SHSTK_SHSTK (1ULL <<  0)
> +#endif
> +
> +static void test_uretprobe_shadow_stack(void)
> +{
> + if (ARCH_PRCTL(ARCH_SHSTK_ENABLE, ARCH_SHSTK_SHSTK)) {
> + test__skip();
> + return;
> + }
> +
> + /* Run all of the uretprobe tests. */
> + test_uretprobe_regs_equal();
> + test_uretprobe_regs_change();
> + test_uretprobe_syscall_call();
> +
> + ARCH_PRCTL(ARCH_SHSTK_DISABLE, ARCH_SHSTK_SHSTK);
> +}
>  #else
>  static void test_uretprobe_regs_equal(void)
>  {
> @@ -312,6 +365,11 @@ static void test_uretprobe_syscall_call(void)
>  {
>   test__skip();
>  }
> +
> +static void test_uretprobe_shadow_stack(void)
> +{
> + test__skip();
> +}
>  #endif
>  
>  void test_uprobe_syscall(void)
> @@ -322,4 +380,6 @@ void test_uprobe_syscall(void)
>   test_uretprobe_regs_change();
>   if (test__start_subtest("uretprobe_syscall_call"))
>   test_uretprobe_syscall_call();
> + if (test__start_subtest("uretprobe_shadow_stack"))
> + test_uretprobe_shadow_stack();
>  }
> -- 
> 2.45.1
> 


-- 
Masami Hiramatsu (Google) 

Re: [PATCHv7 bpf-next 0/9] uprobe: uretprobe speed up

[Google]

On Wed, 5 Jun 2024 09:42:45 -0700
Andrii Nakryiko  wrote:

> On Fri, May 31, 2024 at 10:52 AM Andrii Nakryiko
>  wrote:
> >
> > On Thu, May 23, 2024 at 5:11 AM Jiri Olsa  wrote:
> > >
> > > hi,
> > > as part of the effort on speeding up the uprobes [0] coming with
> > > return uprobe optimization by using syscall instead of the trap
> > > on the uretprobe trampoline.
> > >
> > > The speed up depends on instruction type that uprobe is installed
> > > and depends on specific HW type, please check patch 1 for details.
> > >
> > > Patches 1-8 are based on bpf-next/master, but patch 2 and 3 are
> > > apply-able on linux-trace.git tree probes/for-next branch.
> > > Patch 9 is based on man-pages master.
> > >
> > > v7 changes:
> > > - fixes in man page [Alejandro Colomar]
> > > - fixed patch #1 fixes tag [Oleg]
> > >
> > > Also available at:
> > >   https://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
> > >   uretprobe_syscall
> > >
> > > thanks,
> > > jirka
> > >
> > >
> > > Notes to check list items in Documentation/process/adding-syscalls.rst:
> > >
> > > - System Call Alternatives
> > >   New syscall seems like the best way in here, because we need
> > >   just to quickly enter kernel with no extra arguments processing,
> > >   which we'd need to do if we decided to use another syscall.
> > >
> > > - Designing the API: Planning for Extension
> > >   The uretprobe syscall is very specific and most likely won't be
> > >   extended in the future.
> > >
> > >   At the moment it does not take any arguments and even if it does
> > >   in future, it's allowed to be called only from trampoline prepared
> > >   by kernel, so there'll be no broken user.
> > >
> > > - Designing the API: Other Considerations
> > >   N/A because uretprobe syscall does not return reference to kernel
> > >   object.
> > >
> > > - Proposing the API
> > >   Wiring up of the uretprobe system call is in separate change,
> > >   selftests and man page changes are part of the patchset.
> > >
> > > - Generic System Call Implementation
> > >   There's no CONFIG option for the new functionality because it
> > >   keeps the same behaviour from the user POV.
> > >
> > > - x86 System Call Implementation
> > >   It's 64-bit syscall only.
> > >
> > > - Compatibility System Calls (Generic)
> > >   N/A uretprobe syscall has no arguments and is not supported
> > >   for compat processes.
> > >
> > > - Compatibility System Calls (x86)
> > >   N/A uretprobe syscall is not supported for compat processes.
> > >
> > > - System Calls Returning Elsewhere
> > >   N/A.
> > >
> > > - Other Details
> > >   N/A.
> > >
> > > - Testing
> > >   Adding new bpf selftests and ran ltp on top of this change.
> > >
> > > - Man Page
> > >   Attached.
> > >
> > > - Do not call System Calls in the Kernel
> > >   N/A.
> > >
> > >
> > > [0] https://lore.kernel.org/bpf/ZeCXHKJ--iYYbmLj@krava/
> > > ---
> > > Jiri Olsa (8):
> > >   x86/shstk: Make return uprobe work with shadow stack
> > >   uprobe: Wire up uretprobe system call
> > >   uprobe: Add uretprobe syscall to speed up return probe
> > >   selftests/x86: Add return uprobe shadow stack test
> > >   selftests/bpf: Add uretprobe syscall test for regs integrity
> > >   selftests/bpf: Add uretprobe syscall test for regs changes
> > >   selftests/bpf: Add uretprobe syscall call from user space test
> > >   selftests/bpf: Add uretprobe shadow stack test
> > >
> >
> > Masami, Steven,
> >
> > It seems like the series is ready to go in. Are you planning to take
> > the first 4 patches through your linux-trace tree?
> 
> Another ping. It's been two weeks since Jiri posted the last revision
> that got no more feedback to be addressed and everyone seems to be
> happy with it.

Sorry about late reply. I agree that this is OK to go, since no other
comments. Let me pick this up to probes/for-next branch.

> 
> This is an important speed up improvement for uprobe infrastructure in
> general and for BPF ecosystem in particular. "Uprobes are slow" is one
> of the top complaints from production BPF users, and sys_uretprobe
> approach is significantly improving the situation for return uprobes
> (aka uretprobes), potentially enabling new use cases that previously
> could have been too expensive to trace in practice and reducing the
> overhead of the existing ones.
> 
> I'd appreciate the engagement from linux-trace maintainers on this
> patch set. Given it's important for BPF and that a big part of the
> patch set is BPF-based selftests, we'd also be happy to route all this
> through the bpf-next tree (which would actually make logistics for us
> much easier, but that's not the main concern). But regardless of the
> tree, it would be nice to make a decision and go forward with it.

I think it would be better to include those patches together in 
linux-tree. Can you review and ack to the last patch ? ([9/9])

Thank you,

> 
> Thank you!
> 
> >
> > >  arch/x86/entry/syscalls/syscall_64.tbl  

Re: [PATCH 1/3] tracing: Build event generation tests only as modules

[Steven Rostedt]

On Tue, 11 Jun 2024 06:26:34 +0900
"Masami Hiramatsu (Google)"  wrote:

> The kprobes and synth event generation test modules add events and lock
> (get a reference) those event file reference in module init function,
> and unlock and delete it in module exit function. This is because those
> are designed for playing as modules.
> 
> If we make those modules as built-in, those events are left locked in the
> kernel, and never be removed. This causes kprobe event self-test failure
> as below.

Reviewed-by: Steven Rostedt (Google) 

-- Steve

Re: [PATCH 05/14] tracefs: replace call_rcu by kfree_rcu for simple kmem_cache_free callback

[Vlastimil Babka]

On 6/10/24 10:36 PM, Steven Rostedt wrote:
> On Mon, 10 Jun 2024 08:46:42 -0700
> "Paul E. McKenney"  wrote:
> 
>> > > index 7c29f4afc23d..338c52168e61 100644
>> > > --- a/fs/tracefs/inode.c
>> > > +++ b/fs/tracefs/inode.c
>> > > @@ -53,14 +53,6 @@ static struct inode *tracefs_alloc_inode(struct 
>> > > super_block *sb)
>> > >  return &ti->vfs_inode;
>> > >  }
>> > >  
>> > > -static void tracefs_free_inode_rcu(struct rcu_head *rcu)
>> > > -{
>> > > -struct tracefs_inode *ti;
>> > > -
>> > > -ti = container_of(rcu, struct tracefs_inode, rcu);
>> > > -kmem_cache_free(tracefs_inode_cachep, ti);  
>> > 
>> > Does this work?
>> > 
>> > tracefs needs to be freed via the tracefs_inode_cachep. Does
>> > kfree_rcu() handle specific frees for objects that were not allocated
>> > via kmalloc()?  
>> 
>> A recent change to kfree() allows it to correctly handle memory allocated
>> via kmem_cache_alloc().  News to me as of a few weeks ago.  ;-)
> 
> If that's the case then:
> 
> Acked-by: Steven Rostedt (Google) 
> 
> Do we have a way to add a "Depends-on" tag so that anyone backporting this
> will know that it requires the change to whatever allowed that to happen?

Looks like people use that tag, although no grep hits in Documentation, so
Cc'ing workflows@ and Thorsten.

In this case it would be

Depends-on: c9929f0e344a ("mm/slob: remove CONFIG_SLOB")

> Or we need to update the change log to explicitly state that this should
> *not* be backported.
> 
> -- Steve

Re: [PATCH 2/3] tracing/kprobe: Remove unneeded WARN_ON_ONCE() in selftests

[Steven Rostedt]

On Tue, 11 Jun 2024 06:26:44 +0900
"Masami Hiramatsu (Google)"  wrote:

> From: Masami Hiramatsu (Google) 
> 
> Since the kprobe-events selftest shows OK or NG with the reason, the
> WARN_ON_ONCE()s for each place are redundant. Let's remove it.

Note, the ktests we run to validate commits, fail when it detects a WARN()
triggered.

If this fails in any configuration, ktest will not detect it failed.

-- Steve


> 
> Signed-off-by: Masami Hiramatsu (Google) 
> ---
>  kernel/trace/trace_kprobe.c |   26 +-
>  1 file changed, 13 insertions(+), 13 deletions(-)

[PATCH 3/3] tracing/kprobe: Remove cleanup code unrelated to selftest

[Masami Hiramatsu (Google)]

From: Masami Hiramatsu (Google) 

This cleanup all kprobe events code is not related to the selftest
itself, and it can fail by the reason unrelated to this test.
If the test is successful, the generated events are cleaned up.
And if not, we cannot guarantee that the kprobe events will work
correctly. So, anyway, there is no need to clean it up.

Signed-off-by: Masami Hiramatsu (Google) 
---
 kernel/trace/trace_kprobe.c |5 -
 1 file changed, 5 deletions(-)

diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 4abed36544d0..f94628c15c14 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -2129,11 +2129,6 @@ static __init int kprobe_trace_self_tests_init(void)
}
 
 end:
-   ret = dyn_events_release_all(&trace_kprobe_ops);
-   if (ret) {
-   pr_warn("error on cleaning up probes.\n");
-   warn++;
-   }
/*
 * Wait for the optimizer work to finish. Otherwise it might fiddle
 * with probes in already freed __init text.

[PATCH 2/3] tracing/kprobe: Remove unneeded WARN_ON_ONCE() in selftests

[Masami Hiramatsu (Google)]

From: Masami Hiramatsu (Google) 

Since the kprobe-events selftest shows OK or NG with the reason, the
WARN_ON_ONCE()s for each place are redundant. Let's remove it.

Signed-off-by: Masami Hiramatsu (Google) 
---
 kernel/trace/trace_kprobe.c |   26 +-
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 16383247bdbf..4abed36544d0 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -2023,18 +2023,18 @@ static __init int kprobe_trace_self_tests_init(void)
pr_info("Testing kprobe tracing: ");
 
ret = create_or_delete_trace_kprobe("p:testprobe 
kprobe_trace_selftest_target $stack $stack0 +0($stack)");
-   if (WARN_ON_ONCE(ret)) {
+   if (ret) {
pr_warn("error on probing function entry.\n");
warn++;
} else {
/* Enable trace point */
tk = find_trace_kprobe("testprobe", KPROBE_EVENT_SYSTEM);
-   if (WARN_ON_ONCE(tk == NULL)) {
+   if (tk == NULL) {
pr_warn("error on getting new probe.\n");
warn++;
} else {
file = find_trace_probe_file(tk, top_trace_array());
-   if (WARN_ON_ONCE(file == NULL)) {
+   if (file == NULL) {
pr_warn("error on getting probe file.\n");
warn++;
} else
@@ -2044,18 +2044,18 @@ static __init int kprobe_trace_self_tests_init(void)
}
 
ret = create_or_delete_trace_kprobe("r:testprobe2 
kprobe_trace_selftest_target $retval");
-   if (WARN_ON_ONCE(ret)) {
+   if (ret) {
pr_warn("error on probing function return.\n");
warn++;
} else {
/* Enable trace point */
tk = find_trace_kprobe("testprobe2", KPROBE_EVENT_SYSTEM);
-   if (WARN_ON_ONCE(tk == NULL)) {
+   if (tk == NULL) {
pr_warn("error on getting 2nd new probe.\n");
warn++;
} else {
file = find_trace_probe_file(tk, top_trace_array());
-   if (WARN_ON_ONCE(file == NULL)) {
+   if (file == NULL) {
pr_warn("error on getting probe file.\n");
warn++;
} else
@@ -2079,7 +2079,7 @@ static __init int kprobe_trace_self_tests_init(void)
 
/* Disable trace points before removing it */
tk = find_trace_kprobe("testprobe", KPROBE_EVENT_SYSTEM);
-   if (WARN_ON_ONCE(tk == NULL)) {
+   if (tk == NULL) {
pr_warn("error on getting test probe.\n");
warn++;
} else {
@@ -2089,7 +2089,7 @@ static __init int kprobe_trace_self_tests_init(void)
}
 
file = find_trace_probe_file(tk, top_trace_array());
-   if (WARN_ON_ONCE(file == NULL)) {
+   if (file == NULL) {
pr_warn("error on getting probe file.\n");
warn++;
} else
@@ -2098,7 +2098,7 @@ static __init int kprobe_trace_self_tests_init(void)
}
 
tk = find_trace_kprobe("testprobe2", KPROBE_EVENT_SYSTEM);
-   if (WARN_ON_ONCE(tk == NULL)) {
+   if (tk == NULL) {
pr_warn("error on getting 2nd test probe.\n");
warn++;
} else {
@@ -2108,7 +2108,7 @@ static __init int kprobe_trace_self_tests_init(void)
}
 
file = find_trace_probe_file(tk, top_trace_array());
-   if (WARN_ON_ONCE(file == NULL)) {
+   if (file == NULL) {
pr_warn("error on getting probe file.\n");
warn++;
} else
@@ -2117,20 +2117,20 @@ static __init int kprobe_trace_self_tests_init(void)
}
 
ret = create_or_delete_trace_kprobe("-:testprobe");
-   if (WARN_ON_ONCE(ret)) {
+   if (ret) {
pr_warn("error on deleting a probe.\n");
warn++;
}
 
ret = create_or_delete_trace_kprobe("-:testprobe2");
-   if (WARN_ON_ONCE(ret)) {
+   if (ret) {
pr_warn("error on deleting a probe.\n");
warn++;
}
 
 end:
ret = dyn_events_release_all(&trace_kprobe_ops);
-   if (WARN_ON_ONCE(ret)) {
+   if (ret) {
pr_warn("error on cleaning up probes.\n");
warn++;
}

[PATCH 1/3] tracing: Build event generation tests only as modules

[Masami Hiramatsu (Google)]

From: Masami Hiramatsu (Google) 

The kprobes and synth event generation test modules add events and lock
(get a reference) those event file reference in module init function,
and unlock and delete it in module exit function. This is because those
are designed for playing as modules.

If we make those modules as built-in, those events are left locked in the
kernel, and never be removed. This causes kprobe event self-test failure
as below.

[   97.349708] [ cut here ]
[   97.353453] WARNING: CPU: 3 PID: 1 at kernel/trace/trace_kprobe.c:2133 
kprobe_trace_self_tests_init+0x3f1/0x480
[   97.357106] Modules linked in:
[   97.358488] CPU: 3 PID: 1 Comm: swapper/0 Not tainted 
6.9.0-g699646734ab5-dirty #14
[   97.361556] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 
1.15.0-1 04/01/2014
[   97.363880] RIP: 0010:kprobe_trace_self_tests_init+0x3f1/0x480
[   97.365538] Code: a8 24 08 82 e9 ae fd ff ff 90 0f 0b 90 48 c7 c7 e5 aa 0b 
82 e9 ee fc ff ff 90 0f 0b 90 48 c7 c7 2d 61 06 82 e9 8e fd ff ff 90 <0f> 0b 90 
48 c7 c7 33 0b 0c 82 89 c6 e8 6e 03 1f ff 41 ff c7 e9 90
[   97.370429] RSP: :c9013b50 EFLAGS: 00010286
[   97.371852] RAX: fff0 RBX: 888005919c00 RCX: 
[   97.373829] RDX: 888003f4 RSI: 8236a598 RDI: 888003f40a68
[   97.375715] RBP:  R08: 0001 R09: 
[   97.377675] R10: 811c9ae5 R11: 8120c4e0 R12: 
[   97.379591] R13: 0001 R14: 0015 R15: 
[   97.381536] FS:  () GS:88807dcc() 
knlGS:
[   97.383813] CS:  0010 DS:  ES:  CR0: 80050033
[   97.385449] CR2:  CR3: 02244000 CR4: 06b0
[   97.387347] DR0:  DR1:  DR2: 
[   97.389277] DR3:  DR6: fffe0ff0 DR7: 0400
[   97.391196] Call Trace:
[   97.391967]  
[   97.392647]  ? __warn+0xcc/0x180
[   97.393640]  ? kprobe_trace_self_tests_init+0x3f1/0x480
[   97.395181]  ? report_bug+0xbd/0x150
[   97.396234]  ? handle_bug+0x3e/0x60
[   97.397311]  ? exc_invalid_op+0x1a/0x50
[   97.398434]  ? asm_exc_invalid_op+0x1a/0x20
[   97.399652]  ? trace_kprobe_is_busy+0x20/0x20
[   97.400904]  ? tracing_reset_all_online_cpus+0x15/0x90
[   97.402304]  ? kprobe_trace_self_tests_init+0x3f1/0x480
[   97.403773]  ? init_kprobe_trace+0x50/0x50
[   97.404972]  do_one_initcall+0x112/0x240
[   97.406113]  do_initcall_level+0x95/0xb0
[   97.407286]  ? kernel_init+0x1a/0x1a0
[   97.408401]  do_initcalls+0x3f/0x70
[   97.409452]  kernel_init_freeable+0x16f/0x1e0
[   97.410662]  ? rest_init+0x1f0/0x1f0
[   97.411738]  kernel_init+0x1a/0x1a0
[   97.412788]  ret_from_fork+0x39/0x50
[   97.413817]  ? rest_init+0x1f0/0x1f0
[   97.414844]  ret_from_fork_asm+0x11/0x20
[   97.416285]  
[   97.417134] irq event stamp: 13437323
[   97.418376] hardirqs last  enabled at (13437337): [] 
console_unlock+0x11c/0x150
[   97.421285] hardirqs last disabled at (13437370): [] 
console_unlock+0x101/0x150
[   97.423838] softirqs last  enabled at (13437366): [] 
handle_softirqs+0x23f/0x2a0
[   97.426450] softirqs last disabled at (13437393): [] 
__irq_exit_rcu+0x66/0xd0
[   97.428850] ---[ end trace  ]---

And also, since we can not cleanup dynamic_event file, ftracetest are
failed too.

To avoid these issues, build these tests only as modules.

Fixes: 9fe41efaca08 ("tracing: Add synth event generation test module")
Fixes: 64836248dda2 ("tracing: Add kprobe event command generation test module")
Signed-off-by: Masami Hiramatsu (Google) 
---
 kernel/trace/Kconfig |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
index 166ad5444eea..721c3b221048 100644
--- a/kernel/trace/Kconfig
+++ b/kernel/trace/Kconfig
@@ -1136,7 +1136,7 @@ config PREEMPTIRQ_DELAY_TEST
 
 config SYNTH_EVENT_GEN_TEST
tristate "Test module for in-kernel synthetic event generation"
-   depends on SYNTH_EVENTS
+   depends on SYNTH_EVENTS && m
help
   This option creates a test module to check the base
   functionality of in-kernel synthetic event definition and
@@ -1149,7 +1149,7 @@ config SYNTH_EVENT_GEN_TEST
 
 config KPROBE_EVENT_GEN_TEST
tristate "Test module for in-kernel kprobe event generation"
-   depends on KPROBE_EVENTS
+   depends on KPROBE_EVENTS && m
help
   This option creates a test module to check the base
   functionality of in-kernel kprobe event definition.

[PATCH 0/3] tracing: Fix some selftest issues

[Masami Hiramatsu (Google)]

Hi,

Here is v2 of a series of some fixes/cleanups for the test modules and
boot time selftest of kprobe events. The previous version is here;

https://lore.kernel.org/all/171671825710.39694.6859036369216249956.stgit@devnote2/

In this version, I just update the description of the first patch to add
what bad things happen when the modules are built in.

I found a WARNING message with some boot time selftest configuration, which
came from the combination of embedded kprobe generate API tests module and
ftrace boot-time selftest. Since kprobe and synthetic event generation API
test modules add new events and lock it. Thus dynamic event remove-all
operation failes. This also causes all ftracetest failed because it tries
to cleanup all dynamic events before running test cases.

The main problem is that these modules should not be built-in. But I also
think this WARNING message is useless (because there are warning messages
already) and the cleanup code is redundant. This series fixes those issues.

Thank you,

---

Masami Hiramatsu (Google) (3):
  tracing: Build event generation tests only as modules
  tracing/kprobe: Remove unneeded WARN_ON_ONCE() in selftests
  tracing/kprobe: Remove cleanup code unrelated to selftest


 kernel/trace/Kconfig|4 ++--
 kernel/trace/trace_kprobe.c |   29 -
 2 files changed, 14 insertions(+), 19 deletions(-)

--
Masami Hiramatsu (Google) 

Re: [PATCH 05/14] tracefs: replace call_rcu by kfree_rcu for simple kmem_cache_free callback

[Steven Rostedt]

On Mon, 10 Jun 2024 22:42:30 +0200
Vlastimil Babka  wrote:

> On 6/10/24 5:46 PM, Paul E. McKenney wrote:
> > On Mon, Jun 10, 2024 at 11:22:23AM -0400, Steven Rostedt wrote:  
> >> On Sun,  9 Jun 2024 10:27:17 +0200
> >> Julia Lawall  wrote:
> >>   
> >> > diff --git a/fs/tracefs/inode.c b/fs/tracefs/inode.c
> >> > index 7c29f4afc23d..338c52168e61 100644
> >> > --- a/fs/tracefs/inode.c
> >> > +++ b/fs/tracefs/inode.c
> >> > @@ -53,14 +53,6 @@ static struct inode *tracefs_alloc_inode(struct 
> >> > super_block *sb)
> >> >  return &ti->vfs_inode;
> >> >  }
> >> >  
> >> > -static void tracefs_free_inode_rcu(struct rcu_head *rcu)
> >> > -{
> >> > -struct tracefs_inode *ti;
> >> > -
> >> > -ti = container_of(rcu, struct tracefs_inode, rcu);
> >> > -kmem_cache_free(tracefs_inode_cachep, ti);  
> >> 
> >> Does this work?
> >> 
> >> tracefs needs to be freed via the tracefs_inode_cachep. Does
> >> kfree_rcu() handle specific frees for objects that were not allocated
> >> via kmalloc()?  
> > 
> > A recent change to kfree() allows it to correctly handle memory allocated
> > via kmem_cache_alloc().  News to me as of a few weeks ago.  ;-)  
> 
> Hey, I did try not to keep that a secret :)
> https://lore.kernel.org/all/20230310103210.22372-8-vba...@suse.cz/
> 

Heh, I didn't look at that patch very deeply.

-- Steve

Re: [PATCH 05/14] tracefs: replace call_rcu by kfree_rcu for simple kmem_cache_free callback

[Vlastimil Babka]

On 6/10/24 5:46 PM, Paul E. McKenney wrote:
> On Mon, Jun 10, 2024 at 11:22:23AM -0400, Steven Rostedt wrote:
>> On Sun,  9 Jun 2024 10:27:17 +0200
>> Julia Lawall  wrote:
>> 
>> > diff --git a/fs/tracefs/inode.c b/fs/tracefs/inode.c
>> > index 7c29f4afc23d..338c52168e61 100644
>> > --- a/fs/tracefs/inode.c
>> > +++ b/fs/tracefs/inode.c
>> > @@ -53,14 +53,6 @@ static struct inode *tracefs_alloc_inode(struct 
>> > super_block *sb)
>> >return &ti->vfs_inode;
>> >  }
>> >  
>> > -static void tracefs_free_inode_rcu(struct rcu_head *rcu)
>> > -{
>> > -  struct tracefs_inode *ti;
>> > -
>> > -  ti = container_of(rcu, struct tracefs_inode, rcu);
>> > -  kmem_cache_free(tracefs_inode_cachep, ti);
>> 
>> Does this work?
>> 
>> tracefs needs to be freed via the tracefs_inode_cachep. Does
>> kfree_rcu() handle specific frees for objects that were not allocated
>> via kmalloc()?
> 
> A recent change to kfree() allows it to correctly handle memory allocated
> via kmem_cache_alloc().  News to me as of a few weeks ago.  ;-)

Hey, I did try not to keep that a secret :)
https://lore.kernel.org/all/20230310103210.22372-8-vba...@suse.cz/

>   Thanx, Paul
> 
>> -- Steve
>> 
>> 
>> > -}
>> > -
>> >  static void tracefs_free_inode(struct inode *inode)
>> >  {
>> >struct tracefs_inode *ti = get_tracefs(inode);
>> > @@ -70,7 +62,7 @@ static void tracefs_free_inode(struct inode *inode)
>> >list_del_rcu(&ti->list);
>> >spin_unlock_irqrestore(&tracefs_inode_lock, flags);
>> >  
>> > -  call_rcu(&ti->rcu, tracefs_free_inode_rcu);
>> > +  kfree_rcu(ti, rcu);
>> >  }
>> >  
>> >  static ssize_t default_read_file(struct file *file, char __user *buf,
>> 

Re: [PATCH 05/14] tracefs: replace call_rcu by kfree_rcu for simple kmem_cache_free callback

[Steven Rostedt]

On Mon, 10 Jun 2024 08:46:42 -0700
"Paul E. McKenney"  wrote:

> > > index 7c29f4afc23d..338c52168e61 100644
> > > --- a/fs/tracefs/inode.c
> > > +++ b/fs/tracefs/inode.c
> > > @@ -53,14 +53,6 @@ static struct inode *tracefs_alloc_inode(struct 
> > > super_block *sb)
> > >   return &ti->vfs_inode;
> > >  }
> > >  
> > > -static void tracefs_free_inode_rcu(struct rcu_head *rcu)
> > > -{
> > > - struct tracefs_inode *ti;
> > > -
> > > - ti = container_of(rcu, struct tracefs_inode, rcu);
> > > - kmem_cache_free(tracefs_inode_cachep, ti);  
> > 
> > Does this work?
> > 
> > tracefs needs to be freed via the tracefs_inode_cachep. Does
> > kfree_rcu() handle specific frees for objects that were not allocated
> > via kmalloc()?  
> 
> A recent change to kfree() allows it to correctly handle memory allocated
> via kmem_cache_alloc().  News to me as of a few weeks ago.  ;-)

If that's the case then:

Acked-by: Steven Rostedt (Google) 

Do we have a way to add a "Depends-on" tag so that anyone backporting this
will know that it requires the change to whatever allowed that to happen?

Or we need to update the change log to explicitly state that this should
*not* be backported.

-- Steve

Re: [RFC v3 net-next 1/7] net: add rx_sk to trace_kfree_skb

[Yan Zhai]

On Mon, Jun 10, 2024 at 11:54 AM Steven Rostedt  wrote:
>
> On Thu, 6 Jun 2024 10:37:46 -0500
> Yan Zhai  wrote:
>
> > > name: kfree_skb
> > > ID: 1799
> > > format:
> > > field:unsigned short common_type;   offset:0;   size:2; 
> > > signed:0;
> > > field:unsigned char common_flags;   offset:2;   size:1; 
> > > signed:0;
> > > field:unsigned char common_preempt_count;   offset:3;   
> > > size:1; signed:0;
> > > field:int common_pid;   offset:4;   size:4; signed:1;
> > >
> > > field:void * skbaddr;   offset:8;   size:8; signed:0;
> > > field:void * location;  offset:16;  size:8; signed:0;
> > > field:unsigned short protocol;  offset:24;  size:2; signed:0;
> > > field:enum skb_drop_reason reason;  offset:28;  size:4; 
> > > signed:0;
> > >
> > > Notice that "protocol" is 2 bytes in size at offset 24, but "reason" 
> > > starts
> > > at offset 28. This means at offset 26, there's a 2 byte hole.
> > >
> > The reason I added the pointer as the last argument is trying to
> > minimize the surprise to existing TP users, because for common ABIs
> > it's fine to omit later arguments when defining a function, but it
> > needs change and recompilation if the order of arguments changed.
>
> Nothing should be hard coding the offsets of the fields. This is
> exported to user space so that tools can see where the fields are.
> That's the purpose of libtraceevent. The fields should be movable and
> not affect anything. There should be no need to recompile.
>
Oh I misunderstood previously. I was also thinking about the argument
order in TP_PROTO, but what you mentioned is just the order in
TP_STRUCT__entry, correct? I'd prefer to not change the argument order
but the struct field order can definitely be aligned better here.

Yan

> >
> > Looking at the actual format after the change, it does not add a new
> > hole since protocol and reason are already packed into the same 8-byte
> > block, so rx_skaddr starts at 8-byte aligned offset:
> >
> > # cat /sys/kernel/debug/tracing/events/skb/kfree_skb/format
> > name: kfree_skb
> > ID: 2260
> > format:
> > field:unsigned short common_type;   offset:0;
> > size:2; signed:0;
> > field:unsigned char common_flags;   offset:2;
> > size:1; signed:0;
> > field:unsigned char common_preempt_count;   offset:3;
> >  size:1; signed:0;
> > field:int common_pid;   offset:4;   size:4; signed:1;
> >
> > field:void * skbaddr;   offset:8;   size:8; signed:0;
> > field:void * location;  offset:16;  size:8; signed:0;
> > field:unsigned short protocol;  offset:24;  size:2; signed:0;
> > field:enum skb_drop_reason reason;  offset:28;
> > size:4; signed:0;
> > field:void * rx_skaddr; offset:32;  size:8; signed:0;
> >
> > Do you think we still need to change the order?
>
> Up to you, just wanted to point it out.
>
> -- Steve
>

Re: [PATCH 0/3] tracing: Fix some selftest issues

[Steven Rostedt]

On Mon, 10 Jun 2024 11:10:01 +0900
Masami Hiramatsu (Google)  wrote:

> > But you don't explain what exactly the conflict is. What about those
> > events causes kprobe selftests to fail?  
> 
> I also found another problem on these modules. These modules get trace
> event file references to prevent removing probes. Therefore, if we
> embed these modules, we can not remove these events forever!
> 
> /*
>  * Now get the gen_kprobe_test event file.  We need to prevent
>  * the instance and event from disappearing from underneath
>  * us, which trace_get_event_file() does (though in this case
>  * we're using the top-level instance which never goes away).
>  */
> gen_kprobe_test = trace_get_event_file(NULL, "kprobes",
>"gen_kprobe_test");
> if (IS_ERR(gen_kprobe_test)) {
> ret = PTR_ERR(gen_kprobe_test);
> goto delete;
> }
> 
> This means most ftracetest fails because we can not clean up the
> tracing state by removing dynamic events, which are installed while
> booting.
> Note that these references (locks) will be removed when the module
> is unloaded.

I'm fine if you want to force these as modules. Just make sure the change
log lists all the issues of them not being modules.

-- Steve

Re: [PATCH v5 1/2] sched/rt: Clean up usage of rt_task()

[Qais Yousef]

On 06/05/24 16:07, Daniel Bristot de Oliveira wrote:
> On 6/5/24 15:24, Qais Yousef wrote:
> >>> But rt is a shortened version of realtime, and so it is making *it less*
> >>> clear that we also have DL here.
> >> Can SCHED_DL be considered a real-time scheduling class as in opposite
> >> to SCHED_BATCH for instance? Due to its requirements it fits for a real
> >> time scheduling class, right?
> >> And RT (as in real time) already includes SCHED_RR and SCHED_FIFO.
> > Yeah I think the usage of realtime to cover both makes sense. I followed 
> > your
> > precedence with task_is_realtime().
> > 
> > Anyway. If people really find this confusing, what would make sense is to 
> > split
> > them and ask users to call rt_task() and dl_task() explicitly without this
> > wrapper. I personally like it better with the wrapper. But happy to follow 
> > the
> > crowd.
> 
> For me, doing dl_ things it is better to keep them separate, so I can
> easily search for dl_ specific checks.
> 
> rt_or_dl_task(p);

I posted a new version with this suggestion as the top patch so that it can be
shredded more :-)

Thanks for having a look.


Cheers

--
Qais Yousef

[PATCH v6 3/3] sched/rt: Rename realtime_{prio, task}() to rt_or_dl_{prio, task}()

[Qais Yousef]

Some find the name realtime overloaded. Use rt_or_dl() as an
alternative, hopefully better, name.

Suggested-by: Daniel Bristot de Oliveira 
Signed-off-by: Qais Yousef 
---
 fs/bcachefs/six.c |  2 +-
 fs/select.c   |  2 +-
 include/linux/ioprio.h|  2 +-
 include/linux/sched/rt.h  | 10 +-
 kernel/locking/rtmutex.c  |  4 ++--
 kernel/locking/rwsem.c|  4 ++--
 kernel/locking/ww_mutex.h |  2 +-
 kernel/sched/core.c   |  4 ++--
 kernel/sched/syscalls.c   |  2 +-
 kernel/time/hrtimer.c |  6 +++---
 kernel/trace/trace_sched_wakeup.c |  2 +-
 mm/page-writeback.c   |  4 ++--
 mm/page_alloc.c   |  2 +-
 13 files changed, 23 insertions(+), 23 deletions(-)

diff --git a/fs/bcachefs/six.c b/fs/bcachefs/six.c
index b30870bf7e4a..9cbd3c14c94f 100644
--- a/fs/bcachefs/six.c
+++ b/fs/bcachefs/six.c
@@ -335,7 +335,7 @@ static inline bool six_owner_running(struct six_lock *lock)
 */
rcu_read_lock();
struct task_struct *owner = READ_ONCE(lock->owner);
-   bool ret = owner ? owner_on_cpu(owner) : !realtime_task(current);
+   bool ret = owner ? owner_on_cpu(owner) : !rt_or_dl_task(current);
rcu_read_unlock();
 
return ret;
diff --git a/fs/select.c b/fs/select.c
index 8d5c1419416c..73fce145eb72 100644
--- a/fs/select.c
+++ b/fs/select.c
@@ -82,7 +82,7 @@ u64 select_estimate_accuracy(struct timespec64 *tv)
 * Realtime tasks get a slack of 0 for obvious reasons.
 */
 
-   if (realtime_task(current))
+   if (rt_or_dl_task(current))
return 0;
 
ktime_get_ts64(&now);
diff --git a/include/linux/ioprio.h b/include/linux/ioprio.h
index 75859b78d540..b25377b6ea98 100644
--- a/include/linux/ioprio.h
+++ b/include/linux/ioprio.h
@@ -40,7 +40,7 @@ static inline int task_nice_ioclass(struct task_struct *task)
 {
if (task->policy == SCHED_IDLE)
return IOPRIO_CLASS_IDLE;
-   else if (realtime_task_policy(task))
+   else if (rt_or_dl_task_policy(task))
return IOPRIO_CLASS_RT;
else
return IOPRIO_CLASS_BE;
diff --git a/include/linux/sched/rt.h b/include/linux/sched/rt.h
index 91ef1ef2019f..4e3338103654 100644
--- a/include/linux/sched/rt.h
+++ b/include/linux/sched/rt.h
@@ -11,7 +11,7 @@ static inline bool rt_prio(int prio)
return unlikely(prio < MAX_RT_PRIO && prio >= MAX_DL_PRIO);
 }
 
-static inline bool realtime_prio(int prio)
+static inline bool rt_or_dl_prio(int prio)
 {
return unlikely(prio < MAX_RT_PRIO);
 }
@@ -27,19 +27,19 @@ static inline bool rt_task(struct task_struct *p)
 
 /*
  * Returns true if a task has a priority that belongs to RT or DL classes.
- * PI-boosted tasks will return true. Use realtime_task_policy() to ignore
+ * PI-boosted tasks will return true. Use rt_or_dl_task_policy() to ignore
  * PI-boosted tasks.
  */
-static inline bool realtime_task(struct task_struct *p)
+static inline bool rt_or_dl_task(struct task_struct *p)
 {
-   return realtime_prio(p->prio);
+   return rt_or_dl_prio(p->prio);
 }
 
 /*
  * Returns true if a task has a policy that belongs to RT or DL classes.
  * PI-boosted tasks will return false.
  */
-static inline bool realtime_task_policy(struct task_struct *tsk)
+static inline bool rt_or_dl_task_policy(struct task_struct *tsk)
 {
int policy = tsk->policy;
 
diff --git a/kernel/locking/rtmutex.c b/kernel/locking/rtmutex.c
index 55c9dab37f33..c2a530d704b4 100644
--- a/kernel/locking/rtmutex.c
+++ b/kernel/locking/rtmutex.c
@@ -347,7 +347,7 @@ static __always_inline int __waiter_prio(struct task_struct 
*task)
 {
int prio = task->prio;
 
-   if (!realtime_prio(prio))
+   if (!rt_or_dl_prio(prio))
return DEFAULT_PRIO;
 
return prio;
@@ -435,7 +435,7 @@ static inline bool rt_mutex_steal(struct rt_mutex_waiter 
*waiter,
 * Note that RT tasks are excluded from same priority (lateral)
 * steals to prevent the introduction of an unbounded latency.
 */
-   if (realtime_prio(waiter->tree.prio))
+   if (rt_or_dl_prio(waiter->tree.prio))
return false;
 
return rt_waiter_node_equal(&waiter->tree, &top_waiter->tree);
diff --git a/kernel/locking/rwsem.c b/kernel/locking/rwsem.c
index ad8d4438bc91..dcbd7b45359a 100644
--- a/kernel/locking/rwsem.c
+++ b/kernel/locking/rwsem.c
@@ -631,7 +631,7 @@ static inline bool rwsem_try_write_lock(struct rw_semaphore 
*sem,
 * if it is an RT task or wait in the wait queue
 * for too long.
 */
-   if (has_handoff || (!realtime_task(waiter->task) &&
+   if (has_handoff || (!rt_or_dl_task(waiter->task) &&
!time_after(jiffies, 
waiter->timeout)))
return f

[PATCH v6 2/3] sched/rt, dl: Convert functions to return bool

[Qais Yousef]

{rt, realtime, dl}_{task, prio}() functions' return value is actually
a bool. Convert their return type to reflect that.

Suggested-by: Steven Rostedt (Google) 
Reviewed-by: Sebastian Andrzej Siewior 
Reviewed-by: Steven Rostedt (Google) 
Reviewed-by: Metin Kaya 
Signed-off-by: Qais Yousef 
---
 include/linux/sched/deadline.h |  8 +++-
 include/linux/sched/rt.h   | 16 ++--
 2 files changed, 9 insertions(+), 15 deletions(-)

diff --git a/include/linux/sched/deadline.h b/include/linux/sched/deadline.h
index 5cb88b748ad6..3a912ab42bb5 100644
--- a/include/linux/sched/deadline.h
+++ b/include/linux/sched/deadline.h
@@ -10,18 +10,16 @@
 
 #include 
 
-static inline int dl_prio(int prio)
+static inline bool dl_prio(int prio)
 {
-   if (unlikely(prio < MAX_DL_PRIO))
-   return 1;
-   return 0;
+   return unlikely(prio < MAX_DL_PRIO);
 }
 
 /*
  * Returns true if a task has a priority that belongs to DL class. PI-boosted
  * tasks will return true. Use dl_policy() to ignore PI-boosted tasks.
  */
-static inline int dl_task(struct task_struct *p)
+static inline bool dl_task(struct task_struct *p)
 {
return dl_prio(p->prio);
 }
diff --git a/include/linux/sched/rt.h b/include/linux/sched/rt.h
index a055dd68a77c..91ef1ef2019f 100644
--- a/include/linux/sched/rt.h
+++ b/include/linux/sched/rt.h
@@ -6,25 +6,21 @@
 
 struct task_struct;
 
-static inline int rt_prio(int prio)
+static inline bool rt_prio(int prio)
 {
-   if (unlikely(prio < MAX_RT_PRIO && prio >= MAX_DL_PRIO))
-   return 1;
-   return 0;
+   return unlikely(prio < MAX_RT_PRIO && prio >= MAX_DL_PRIO);
 }
 
-static inline int realtime_prio(int prio)
+static inline bool realtime_prio(int prio)
 {
-   if (unlikely(prio < MAX_RT_PRIO))
-   return 1;
-   return 0;
+   return unlikely(prio < MAX_RT_PRIO);
 }
 
 /*
  * Returns true if a task has a priority that belongs to RT class. PI-boosted
  * tasks will return true. Use rt_policy() to ignore PI-boosted tasks.
  */
-static inline int rt_task(struct task_struct *p)
+static inline bool rt_task(struct task_struct *p)
 {
return rt_prio(p->prio);
 }
@@ -34,7 +30,7 @@ static inline int rt_task(struct task_struct *p)
  * PI-boosted tasks will return true. Use realtime_task_policy() to ignore
  * PI-boosted tasks.
  */
-static inline int realtime_task(struct task_struct *p)
+static inline bool realtime_task(struct task_struct *p)
 {
return realtime_prio(p->prio);
 }
-- 
2.34.1

[PATCH v6 0/3] Clean up usage of rt_task()

[Qais Yousef]

Make rt_task() return true only for RT class and add new realtime_task() to
return true for RT and DL classes to avoid some confusion the old API can
cause.

No functional changes intended in patch 1. Patch 2 cleans up the return type as
suggested by Steve. Patch 3 uses rt_or_dl() instead of 'realtime' as suggested
by Daniel. As the name was debatable, I'll leave up to the maintainers to pick
their preference.

Changes since v5:

* Added a new patch to s/realtime/rt_or_dl/ as suggested by Daniel.
* Added Reviewed-bys.

Changes since v4:

* Simplify return of rt/realtime_prio() as the explicit true/false was
  not necessary.

Changes since v3:

* Make sure the 'new' bool functions return true/false instead of 1/0.
* Drop patch 2 about hrtimer usage of realtime_task() as ongoing
  discussion on v1 indicates its scope outside of this simple cleanup.

Changes since v2:

* Fix one user that should use realtime_task() but remained using
  rt_task() (Sebastian)
* New patch to convert all hrtimer users to use realtime_task_policy()
  (Sebastian)
* Add a new patch to convert return type to bool (Steve)
* Rebase on tip/sched/core and handle a conflict with code shuffle to
  syscalls.c
* Add Reviewed-by Steve

Changes since v1:

* Use realtime_task_policy() instead task_has_realtime_policy() (Peter)
* Improve commit message readability about replace some rt_task()
  users.

v1 discussion: 
https://lore.kernel.org/lkml/20240514234112.792989-1-qyou...@layalina.io/
v2 discussion: 
https://lore.kernel.org/lkml/20240515220536.823145-1-qyou...@layalina.io/
v3 discussion: 
https://lore.kernel.org/lkml/20240527234508.1062360-1-qyou...@layalina.io/
v4 discussion: 
https://lore.kernel.org/lkml/20240601213309.1262206-1-qyou...@layalina.io/
v5 discussion: 
https://lore.kernel.org/lkml/20240604144228.1356121-1-qyou...@layalina.io/

Qais Yousef (3):
  sched/rt: Clean up usage of rt_task()
  sched/rt, dl: Convert functions to return bool
  sched/rt: Rename realtime_{prio, task}() to rt_or_dl_{prio, task}()

 fs/bcachefs/six.c |  2 +-
 fs/select.c   |  2 +-
 include/linux/ioprio.h|  2 +-
 include/linux/sched/deadline.h| 14 ++---
 include/linux/sched/prio.h|  1 +
 include/linux/sched/rt.h  | 33 +--
 kernel/locking/rtmutex.c  |  4 ++--
 kernel/locking/rwsem.c|  4 ++--
 kernel/locking/ww_mutex.h |  2 +-
 kernel/sched/core.c   |  4 ++--
 kernel/sched/syscalls.c   |  2 +-
 kernel/time/hrtimer.c |  6 +++---
 kernel/trace/trace_sched_wakeup.c |  2 +-
 mm/page-writeback.c   |  4 ++--
 mm/page_alloc.c   |  2 +-
 15 files changed, 53 insertions(+), 31 deletions(-)

-- 
2.34.1

[PATCH v6 1/3] sched/rt: Clean up usage of rt_task()

[Qais Yousef]

rt_task() checks if a task has RT priority. But depends on your
dictionary, this could mean it belongs to RT class, or is a 'realtime'
task, which includes RT and DL classes.

Since this has caused some confusion already on discussion [1], it
seemed a clean up is due.

I define the usage of rt_task() to be tasks that belong to RT class.
Make sure that it returns true only for RT class and audit the users and
replace the ones required the old behavior with the new realtime_task()
which returns true for RT and DL classes. Introduce similar
realtime_prio() to create similar distinction to rt_prio() and update
the users that required the old behavior to use the new function.

Move MAX_DL_PRIO to prio.h so it can be used in the new definitions.

Document the functions to make it more obvious what is the difference
between them. PI-boosted tasks is a factor that must be taken into
account when choosing which function to use.

Rename task_is_realtime() to realtime_task_policy() as the old name is
confusing against the new realtime_task().

No functional changes were intended.

[1] 
https://lore.kernel.org/lkml/20240506100509.gl40...@noisy.programming.kicks-ass.net/

Reviewed-by: Phil Auld 
Reviewed-by: Steven Rostedt (Google) 
Reviewed-by: Sebastian Andrzej Siewior 
Signed-off-by: Qais Yousef 
---
 fs/bcachefs/six.c |  2 +-
 fs/select.c   |  2 +-
 include/linux/ioprio.h|  2 +-
 include/linux/sched/deadline.h|  6 --
 include/linux/sched/prio.h|  1 +
 include/linux/sched/rt.h  | 27 ++-
 kernel/locking/rtmutex.c  |  4 ++--
 kernel/locking/rwsem.c|  4 ++--
 kernel/locking/ww_mutex.h |  2 +-
 kernel/sched/core.c   |  4 ++--
 kernel/sched/syscalls.c   |  2 +-
 kernel/time/hrtimer.c |  6 +++---
 kernel/trace/trace_sched_wakeup.c |  2 +-
 mm/page-writeback.c   |  4 ++--
 mm/page_alloc.c   |  2 +-
 15 files changed, 49 insertions(+), 21 deletions(-)

diff --git a/fs/bcachefs/six.c b/fs/bcachefs/six.c
index 3a494c5d1247..b30870bf7e4a 100644
--- a/fs/bcachefs/six.c
+++ b/fs/bcachefs/six.c
@@ -335,7 +335,7 @@ static inline bool six_owner_running(struct six_lock *lock)
 */
rcu_read_lock();
struct task_struct *owner = READ_ONCE(lock->owner);
-   bool ret = owner ? owner_on_cpu(owner) : !rt_task(current);
+   bool ret = owner ? owner_on_cpu(owner) : !realtime_task(current);
rcu_read_unlock();
 
return ret;
diff --git a/fs/select.c b/fs/select.c
index 9515c3fa1a03..8d5c1419416c 100644
--- a/fs/select.c
+++ b/fs/select.c
@@ -82,7 +82,7 @@ u64 select_estimate_accuracy(struct timespec64 *tv)
 * Realtime tasks get a slack of 0 for obvious reasons.
 */
 
-   if (rt_task(current))
+   if (realtime_task(current))
return 0;
 
ktime_get_ts64(&now);
diff --git a/include/linux/ioprio.h b/include/linux/ioprio.h
index db1249cd9692..75859b78d540 100644
--- a/include/linux/ioprio.h
+++ b/include/linux/ioprio.h
@@ -40,7 +40,7 @@ static inline int task_nice_ioclass(struct task_struct *task)
 {
if (task->policy == SCHED_IDLE)
return IOPRIO_CLASS_IDLE;
-   else if (task_is_realtime(task))
+   else if (realtime_task_policy(task))
return IOPRIO_CLASS_RT;
else
return IOPRIO_CLASS_BE;
diff --git a/include/linux/sched/deadline.h b/include/linux/sched/deadline.h
index df3aca89d4f5..5cb88b748ad6 100644
--- a/include/linux/sched/deadline.h
+++ b/include/linux/sched/deadline.h
@@ -10,8 +10,6 @@
 
 #include 
 
-#define MAX_DL_PRIO0
-
 static inline int dl_prio(int prio)
 {
if (unlikely(prio < MAX_DL_PRIO))
@@ -19,6 +17,10 @@ static inline int dl_prio(int prio)
return 0;
 }
 
+/*
+ * Returns true if a task has a priority that belongs to DL class. PI-boosted
+ * tasks will return true. Use dl_policy() to ignore PI-boosted tasks.
+ */
 static inline int dl_task(struct task_struct *p)
 {
return dl_prio(p->prio);
diff --git a/include/linux/sched/prio.h b/include/linux/sched/prio.h
index ab83d85e1183..6ab43b4f72f9 100644
--- a/include/linux/sched/prio.h
+++ b/include/linux/sched/prio.h
@@ -14,6 +14,7 @@
  */
 
 #define MAX_RT_PRIO100
+#define MAX_DL_PRIO0
 
 #define MAX_PRIO   (MAX_RT_PRIO + NICE_WIDTH)
 #define DEFAULT_PRIO   (MAX_RT_PRIO + NICE_WIDTH / 2)
diff --git a/include/linux/sched/rt.h b/include/linux/sched/rt.h
index b2b9e6eb9683..a055dd68a77c 100644
--- a/include/linux/sched/rt.h
+++ b/include/linux/sched/rt.h
@@ -7,18 +7,43 @@
 struct task_struct;
 
 static inline int rt_prio(int prio)
+{
+   if (unlikely(prio < MAX_RT_PRIO && prio >= MAX_DL_PRIO))
+   return 1;
+   return 0;
+}
+
+static inline int realtime_prio(int prio)
 {
if (unlikely(prio < MAX_RT_PRIO))
return 1;
return 

[PATCH v10 2/8] remoteproc: k3-m4: Add a remoteproc driver for M4F subsystem

[Andrew Davis]

From: Martyn Welch 

The AM62x and AM64x SoCs of the TI K3 family has a Cortex M4F core in
the MCU domain. This core is typically used for safety applications in a
stand alone mode. However, some application (non safety related) may
want to use the M4F core as a generic remote processor with IPC to the
host processor. The M4F core has internal IRAM and DRAM memories and are
exposed to the system bus for code and data loading.

A remote processor driver is added to support this subsystem, including
being able to load and boot the M4F core. Loading includes to M4F
internal memories and predefined external code/data memories. The
carve outs for external contiguous memory is defined in the M4F device
node and should match with the external memory declarations in the M4F
image binary. The M4F subsystem has two resets. One reset is for the
entire subsystem i.e including the internal memories and the other, a
local reset is only for the M4F processing core. When loading the image,
the driver first releases the subsystem reset, loads the firmware image
and then releases the local reset to let the M4F processing core run.

Signed-off-by: Martyn Welch 
Signed-off-by: Hari Nagalla 
Signed-off-by: Andrew Davis 
---
 drivers/remoteproc/Kconfig   |  13 +
 drivers/remoteproc/Makefile  |   1 +
 drivers/remoteproc/ti_k3_m4_remoteproc.c | 760 +++
 3 files changed, 774 insertions(+)
 create mode 100644 drivers/remoteproc/ti_k3_m4_remoteproc.c

diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
index 48845dc8fa852..1a7c0330c91a9 100644
--- a/drivers/remoteproc/Kconfig
+++ b/drivers/remoteproc/Kconfig
@@ -339,6 +339,19 @@ config TI_K3_DSP_REMOTEPROC
  It's safe to say N here if you're not interested in utilizing
  the DSP slave processors.
 
+config TI_K3_M4_REMOTEPROC
+   tristate "TI K3 M4 remoteproc support"
+   depends on ARCH_K3 || COMPILE_TEST
+   select MAILBOX
+   select OMAP2PLUS_MBOX
+   help
+ Say m here to support TI's M4 remote processor subsystems
+ on various TI K3 family of SoCs through the remote processor
+ framework.
+
+ It's safe to say N here if you're not interested in utilizing
+ a remote processor.
+
 config TI_K3_R5_REMOTEPROC
tristate "TI K3 R5 remoteproc support"
depends on ARCH_K3
diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
index 91314a9b43cef..5ff4e2fee4abd 100644
--- a/drivers/remoteproc/Makefile
+++ b/drivers/remoteproc/Makefile
@@ -37,5 +37,6 @@ obj-$(CONFIG_ST_REMOTEPROC)   += st_remoteproc.o
 obj-$(CONFIG_ST_SLIM_REMOTEPROC)   += st_slim_rproc.o
 obj-$(CONFIG_STM32_RPROC)  += stm32_rproc.o
 obj-$(CONFIG_TI_K3_DSP_REMOTEPROC) += ti_k3_dsp_remoteproc.o
+obj-$(CONFIG_TI_K3_M4_REMOTEPROC)  += ti_k3_m4_remoteproc.o
 obj-$(CONFIG_TI_K3_R5_REMOTEPROC)  += ti_k3_r5_remoteproc.o
 obj-$(CONFIG_XLNX_R5_REMOTEPROC)   += xlnx_r5_remoteproc.o
diff --git a/drivers/remoteproc/ti_k3_m4_remoteproc.c 
b/drivers/remoteproc/ti_k3_m4_remoteproc.c
new file mode 100644
index 0..880e8f0ad1ba3
--- /dev/null
+++ b/drivers/remoteproc/ti_k3_m4_remoteproc.c
@@ -0,0 +1,760 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * TI K3 Cortex-M4 Remote Processor(s) driver
+ *
+ * Copyright (C) 2021-2024 Texas Instruments Incorporated - https://www.ti.com/
+ * Hari Nagalla 
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "omap_remoteproc.h"
+#include "remoteproc_internal.h"
+#include "ti_sci_proc.h"
+
+/**
+ * struct k3_m4_rproc_mem - internal memory structure
+ * @cpu_addr: MPU virtual address of the memory region
+ * @bus_addr: Bus address used to access the memory region
+ * @dev_addr: Device address of the memory region from remote processor view
+ * @size: Size of the memory region
+ */
+struct k3_m4_rproc_mem {
+   void __iomem *cpu_addr;
+   phys_addr_t bus_addr;
+   u32 dev_addr;
+   size_t size;
+};
+
+/**
+ * struct k3_m4_rproc_mem_data - memory definitions for a remote processor
+ * @name: name for this memory entry
+ * @dev_addr: device address for the memory entry
+ */
+struct k3_m4_rproc_mem_data {
+   const char *name;
+   const u32 dev_addr;
+};
+
+/**
+ * struct k3_m4_rproc_dev_data - device data structure for a remote processor
+ * @mems: pointer to memory definitions for a remote processor
+ * @num_mems: number of memory regions in @mems
+ */
+struct k3_m4_rproc_dev_data {
+   const struct k3_m4_rproc_mem_data *mems;
+   u32 num_mems;
+};
+
+/**
+ * struct k3_m4_rproc - k3 remote processor driver structure
+ * @dev: cached device pointer
+ * @rproc: remoteproc device handle
+ * @mem: internal memory regions data
+ * @num_mems: number of internal memory regions
+ * @rmem: reserved memory regions data
+ * @num_rmems: number of reserved memory regions
+ * @reset: reset control hand

[PATCH v10 6/8] arm64: dts: ti: k3-am642-sk: Add M4F remoteproc node

[Andrew Davis]

From: Hari Nagalla 

The AM64x SoCs of the TI K3 family have a Cortex M4F core in the MCU
domain. This core can be used by non safety applications as a remote
processor. When used as a remote processor with virtio/rpmessage IPC,
two carveout reserved memory nodes are needed. The first region is used
as a DMA pool for the rproc device, and the second region will furnish
the static carveout regions for the firmware memory.

The current carveout addresses and sizes are defined statically for
each rproc device. The M4F processor does not have an MMU, and as such
requires the exact memory used by the firmware to be set-aside.

Signed-off-by: Hari Nagalla 
Signed-off-by: Andrew Davis 
---
 arch/arm64/boot/dts/ti/k3-am642-sk.dts | 19 +++
 1 file changed, 19 insertions(+)

diff --git a/arch/arm64/boot/dts/ti/k3-am642-sk.dts 
b/arch/arm64/boot/dts/ti/k3-am642-sk.dts
index 5b028b3a3192f..727d467ed2c1d 100644
--- a/arch/arm64/boot/dts/ti/k3-am642-sk.dts
+++ b/arch/arm64/boot/dts/ti/k3-am642-sk.dts
@@ -99,6 +99,18 @@ main_r5fss1_core1_memory_region: r5f-memory@a310 {
no-map;
};
 
+   mcu_m4fss_dma_memory_region: m4f-dma-memory@a400 {
+   compatible = "shared-dma-pool";
+   reg = <0x00 0xa400 0x00 0x10>;
+   no-map;
+   };
+
+   mcu_m4fss_memory_region: m4f-memory@a410 {
+   compatible = "shared-dma-pool";
+   reg = <0x00 0xa410 0x00 0xf0>;
+   no-map;
+   };
+
rtos_ipc_memory_region: ipc-memories@a500 {
reg = <0x00 0xa500 0x00 0x0080>;
alignment = <0x1000>;
@@ -666,6 +678,13 @@ &main_r5fss1_core1 {
<&main_r5fss1_core1_memory_region>;
 };
 
+&mcu_m4fss {
+   mboxes = <&mailbox0_cluster6 &mbox_m4_0>;
+   memory-region = <&mcu_m4fss_dma_memory_region>,
+   <&mcu_m4fss_memory_region>;
+   status = "okay";
+};
+
 &ecap0 {
status = "okay";
/* PWM is available on Pin 1 of header J3 */
-- 
2.39.2

[PATCH v10 8/8] arm64: defconfig: Enable TI K3 M4 remoteproc driver

[Andrew Davis]

From: Hari Nagalla 

Some K3 platform devices (AM64x, AM62x) have a Cortex M4 core. Build
the M4 remote proc driver as a module for these platforms.

Signed-off-by: Hari Nagalla 
Signed-off-by: Andrew Davis 
---
 arch/arm64/configs/defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 57a9abe78ee41..b7a3488ad4f94 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -1369,6 +1369,7 @@ CONFIG_QCOM_Q6V5_PAS=m
 CONFIG_QCOM_SYSMON=m
 CONFIG_QCOM_WCNSS_PIL=m
 CONFIG_TI_K3_DSP_REMOTEPROC=m
+CONFIG_TI_K3_M4_REMOTEPROC=m
 CONFIG_TI_K3_R5_REMOTEPROC=m
 CONFIG_RPMSG_CHAR=m
 CONFIG_RPMSG_CTRL=m
-- 
2.39.2

[PATCH v10 7/8] arm64: dts: ti: k3-am642-evm: Add M4F remoteproc node

[Andrew Davis]

From: Hari Nagalla 

The AM64x SoCs of the TI K3 family have a Cortex M4F core in the MCU
domain. This core can be used by non safety applications as a remote
processor. When used as a remote processor with virtio/rpmessage IPC,
two carveout reserved memory nodes are needed. The first region is used
as a DMA pool for the rproc device, and the second region will furnish
the static carveout regions for the firmware memory.

The current carveout addresses and sizes are defined statically for
each rproc device. The M4F processor does not have an MMU, and as such
requires the exact memory used by the firmware to be set-aside.

Signed-off-by: Hari Nagalla 
Signed-off-by: Andrew Davis 
---
 arch/arm64/boot/dts/ti/k3-am642-evm.dts | 19 +++
 1 file changed, 19 insertions(+)

diff --git a/arch/arm64/boot/dts/ti/k3-am642-evm.dts 
b/arch/arm64/boot/dts/ti/k3-am642-evm.dts
index e20e4ffd0f1fa..6dbb26a1bc768 100644
--- a/arch/arm64/boot/dts/ti/k3-am642-evm.dts
+++ b/arch/arm64/boot/dts/ti/k3-am642-evm.dts
@@ -101,6 +101,18 @@ main_r5fss1_core1_memory_region: r5f-memory@a310 {
no-map;
};
 
+   mcu_m4fss_dma_memory_region: m4f-dma-memory@a400 {
+   compatible = "shared-dma-pool";
+   reg = <0x00 0xa400 0x00 0x10>;
+   no-map;
+   };
+
+   mcu_m4fss_memory_region: m4f-memory@a410 {
+   compatible = "shared-dma-pool";
+   reg = <0x00 0xa410 0x00 0xf0>;
+   no-map;
+   };
+
rtos_ipc_memory_region: ipc-memories@a500 {
reg = <0x00 0xa500 0x00 0x0080>;
alignment = <0x1000>;
@@ -763,6 +775,13 @@ &main_r5fss1_core1 {
<&main_r5fss1_core1_memory_region>;
 };
 
+&mcu_m4fss {
+   mboxes = <&mailbox0_cluster6 &mbox_m4_0>;
+   memory-region = <&mcu_m4fss_dma_memory_region>,
+   <&mcu_m4fss_memory_region>;
+   status = "okay";
+};
+
 &serdes_ln_ctrl {
idle-states = ;
 };
-- 
2.39.2

[PATCH v10 5/8] arm64: dts: ti: k3-am64: Add M4F remoteproc node

[Andrew Davis]

From: Hari Nagalla 

The AM64x SoCs of the TI K3 family have a Cortex M4F core in the MCU
domain. This core can be used by non safety applications as a remote
processor. When used as a remote processor with virtio/rpmessage IPC,
two carveout reserved memory nodes are needed.

Disable by default as this node is not complete until mailbox data
is provided in the board level DT.

Signed-off-by: Hari Nagalla 
Signed-off-by: Andrew Davis 
---
 arch/arm64/boot/dts/ti/k3-am64-mcu.dtsi | 13 +
 1 file changed, 13 insertions(+)

diff --git a/arch/arm64/boot/dts/ti/k3-am64-mcu.dtsi 
b/arch/arm64/boot/dts/ti/k3-am64-mcu.dtsi
index ec17285869da6..b98e8ad453289 100644
--- a/arch/arm64/boot/dts/ti/k3-am64-mcu.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am64-mcu.dtsi
@@ -160,4 +160,17 @@ mcu_esm: esm@410 {
reg = <0x00 0x410 0x00 0x1000>;
ti,esm-pins = <0>, <1>;
};
+
+   mcu_m4fss: m4fss@500 {
+   compatible = "ti,am64-m4fss";
+   reg = <0x00 0x500 0x00 0x3>,
+ <0x00 0x504 0x00 0x1>;
+   reg-names = "iram", "dram";
+   resets = <&k3_reset 9 1>;
+   firmware-name = "am64-mcu-m4f0_0-fw";
+   ti,sci = <&dmsc>;
+   ti,sci-dev-id = <9>;
+   ti,sci-proc-ids = <0x18 0xff>;
+   status = "disabled";
+   };
 };
-- 
2.39.2

[PATCH v10 4/8] arm64: dts: ti: k3-am625-sk: Add M4F remoteproc node

[Andrew Davis]

From: Hari Nagalla 

The AM62x SoCs of the TI K3 family have a Cortex M4F core in the MCU
domain. This core can be used by non safety applications as a remote
processor. When used as a remote processor with virtio/rpmessage IPC,
two carveout reserved memory nodes are needed. The first region is used
as a DMA pool for the rproc device, and the second region will furnish
the static carveout regions for the firmware memory.

The current carveout addresses and sizes are defined statically for
each rproc device. The M4F processor does not have an MMU, and as such
requires the exact memory used by the firmware to be set-aside.

Signed-off-by: Hari Nagalla 
Signed-off-by: Andrew Davis 
---
 .../arm64/boot/dts/ti/k3-am62x-sk-common.dtsi | 19 +++
 1 file changed, 19 insertions(+)

diff --git a/arch/arm64/boot/dts/ti/k3-am62x-sk-common.dtsi 
b/arch/arm64/boot/dts/ti/k3-am62x-sk-common.dtsi
index 3c45782ab2b78..167bec5c80006 100644
--- a/arch/arm64/boot/dts/ti/k3-am62x-sk-common.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am62x-sk-common.dtsi
@@ -48,6 +48,18 @@ ramoops@9ca0 {
pmsg-size = <0x8000>;
};
 
+   mcu_m4fss_dma_memory_region: m4f-dma-memory@9cb0 {
+   compatible = "shared-dma-pool";
+   reg = <0x00 0x9cb0 0x00 0x10>;
+   no-map;
+   };
+
+   mcu_m4fss_memory_region: m4f-memory@9cc0 {
+   compatible = "shared-dma-pool";
+   reg = <0x00 0x9cc0 0x00 0xe0>;
+   no-map;
+   };
+
secure_tfa_ddr: tfa@9e78 {
reg = <0x00 0x9e78 0x00 0x8>;
alignment = <0x1000>;
@@ -457,6 +469,13 @@ mbox_m4_0: mbox-m4-0 {
};
 };
 
+&mcu_m4fss {
+   mboxes = <&mailbox0_cluster0 &mbox_m4_0>;
+   memory-region = <&mcu_m4fss_dma_memory_region>,
+   <&mcu_m4fss_memory_region>;
+   status = "okay";
+};
+
 &usbss0 {
bootph-all;
status = "okay";
-- 
2.39.2

[PATCH v10 0/8] TI K3 M4F support on AM62 and AM64 SoCs

[Andrew Davis]

Hello all,

This is the continuation of the M4F RProc support series from here[0].
I'm helping out with the upstream task for Hari and so versions (v8+)
is a little different than the previous(v7-) postings[0]. Most notable
change I've introduced being the patches factoring out common support
from the current K3 R5 and DSP drivers have been dropped. I'd like
to do that re-factor *after* getting this driver in shape, that way
we have 3 similar drivers to factor out from vs trying to make those
changes in parallel with the series adding M4 support.

Anyway, details on our M4F subsystem can be found the
the AM62 TRM in the section on the same:

AM62x Technical Reference Manual (SPRUIV7A – MAY 2022)
https://www.ti.com/lit/pdf/SPRUIV7A

Thanks,
Andrew

[0] 
https://lore.kernel.org/linux-arm-kernel/20240202175538.1705-5-hnaga...@ti.com/T/

Changes for v10:
 - Rebased on v6.10-rc3
 - Added AM64 M4 support in DT
 - Addressed comments by Mathieu from v9

Changes for v9:
 - Fixed reserved-memory.yaml text in [1/5]
 - Split dts patch into one for SoC and one for board enable
 - Corrected DT property order and formatting [4/5][5/5]

Hari Nagalla (7):
  dt-bindings: remoteproc: k3-m4f: Add K3 AM64x SoCs
  arm64: dts: ti: k3-am62: Add M4F remoteproc node
  arm64: dts: ti: k3-am625-sk: Add M4F remoteproc node
  arm64: dts: ti: k3-am64: Add M4F remoteproc node
  arm64: dts: ti: k3-am642-sk: Add M4F remoteproc node
  arm64: dts: ti: k3-am642-evm: Add M4F remoteproc node
  arm64: defconfig: Enable TI K3 M4 remoteproc driver

Martyn Welch (1):
  remoteproc: k3-m4: Add a remoteproc driver for M4F subsystem

 .../bindings/remoteproc/ti,k3-m4f-rproc.yaml  | 125 +++
 arch/arm64/boot/dts/ti/k3-am62-mcu.dtsi   |  13 +
 .../arm64/boot/dts/ti/k3-am62x-sk-common.dtsi |  19 +
 arch/arm64/boot/dts/ti/k3-am64-mcu.dtsi   |  13 +
 arch/arm64/boot/dts/ti/k3-am642-evm.dts   |  19 +
 arch/arm64/boot/dts/ti/k3-am642-sk.dts|  19 +
 arch/arm64/configs/defconfig  |   1 +
 drivers/remoteproc/Kconfig|  13 +
 drivers/remoteproc/Makefile   |   1 +
 drivers/remoteproc/ti_k3_m4_remoteproc.c  | 760 ++
 10 files changed, 983 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/remoteproc/ti,k3-m4f-rproc.yaml
 create mode 100644 drivers/remoteproc/ti_k3_m4_remoteproc.c

-- 
2.39.2

[PATCH v10 1/8] dt-bindings: remoteproc: k3-m4f: Add K3 AM64x SoCs

[Andrew Davis]

From: Hari Nagalla 

K3 AM64x SoC has a Cortex M4F subsystem in the MCU voltage domain.
The remote processor's life cycle management and IPC mechanisms are
similar across the R5F and M4F cores from remote processor driver
point of view. However, there are subtle differences in image loading
and starting the M4F subsystems.

The YAML binding document provides the various node properties to be
configured by the consumers of the M4F subsystem.

Signed-off-by: Martyn Welch 
Signed-off-by: Hari Nagalla 
Signed-off-by: Andrew Davis 
Reviewed-by: Conor Dooley 
---
 .../bindings/remoteproc/ti,k3-m4f-rproc.yaml  | 125 ++
 1 file changed, 125 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/remoteproc/ti,k3-m4f-rproc.yaml

diff --git a/Documentation/devicetree/bindings/remoteproc/ti,k3-m4f-rproc.yaml 
b/Documentation/devicetree/bindings/remoteproc/ti,k3-m4f-rproc.yaml
new file mode 100644
index 0..2bd0752b6ba9e
--- /dev/null
+++ b/Documentation/devicetree/bindings/remoteproc/ti,k3-m4f-rproc.yaml
@@ -0,0 +1,125 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/remoteproc/ti,k3-m4f-rproc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: TI K3 M4F processor subsystems
+
+maintainers:
+  - Hari Nagalla 
+  - Mathieu Poirier 
+
+description: |
+  Some K3 family SoCs have Arm Cortex M4F cores. AM64x is a SoC in K3
+  family with a M4F core. Typically safety oriented applications may use
+  the M4F core in isolation without an IPC. Where as some industrial and
+  home automation applications, may use the M4F core as a remote processor
+  with IPC communications.
+
+$ref: /schemas/arm/keystone/ti,k3-sci-common.yaml#
+
+properties:
+  compatible:
+enum:
+  - ti,am64-m4fss
+
+  power-domains:
+maxItems: 1
+
+  "#address-cells":
+const: 2
+
+  "#size-cells":
+const: 2
+
+  reg:
+items:
+  - description: IRAM internal memory region
+  - description: DRAM internal memory region
+
+  reg-names:
+items:
+  - const: iram
+  - const: dram
+
+  resets:
+maxItems: 1
+
+  firmware-name:
+maxItems: 1
+description: Name of firmware to load for the M4F core
+
+  mboxes:
+description:
+  OMAP Mailbox specifier denoting the sub-mailbox, to be used for
+  communication with the remote processor. This property should match
+  with the sub-mailbox node used in the firmware image.
+maxItems: 1
+
+  memory-region:
+description:
+  phandle to the reserved memory nodes to be associated with the
+  remoteproc device. Optional memory regions available for firmware
+  specific purposes.
+  (see reserved-memory/reserved-memory.yaml in dtschema project)
+maxItems: 8
+items:
+  - description: regions used for DMA allocations like vrings, vring 
buffers
+ and memory dedicated to firmware's specific purposes.
+additionalItems: true
+
+required:
+  - compatible
+  - reg
+  - reg-names
+  - ti,sci
+  - ti,sci-dev-id
+  - ti,sci-proc-ids
+  - resets
+  - firmware-name
+
+unevaluatedProperties: false
+
+examples:
+  - |
+reserved-memory {
+#address-cells = <2>;
+#size-cells = <2>;
+
+mcu_m4fss_dma_memory_region: m4f-dma-memory@9cb0 {
+compatible = "shared-dma-pool";
+reg = <0x00 0x9cb0 0x00 0x10>;
+no-map;
+};
+
+mcu_m4fss_memory_region: m4f-memory@9cc0 {
+compatible = "shared-dma-pool";
+reg = <0x00 0x9cc0 0x00 0xe0>;
+no-map;
+};
+};
+
+soc {
+#address-cells = <2>;
+#size-cells = <2>;
+
+mailbox0_cluster0: mailbox-0 {
+#mbox-cells = <1>;
+};
+
+remoteproc@500 {
+compatible = "ti,am64-m4fss";
+reg = <0x00 0x500 0x00 0x3>,
+  <0x00 0x504 0x00 0x1>;
+reg-names = "iram", "dram";
+resets = <&k3_reset 9 1>;
+firmware-name = "am62-mcu-m4f0_0-fw";
+mboxes = <&mailbox0_cluster0>, <&mbox_m4_0>;
+memory-region = <&mcu_m4fss_dma_memory_region>,
+<&mcu_m4fss_memory_region>;
+ti,sci = <&dmsc>;
+ti,sci-dev-id = <9>;
+ti,sci-proc-ids = <0x18 0xff>;
+ };
+};
-- 
2.39.2

[PATCH v10 3/8] arm64: dts: ti: k3-am62: Add M4F remoteproc node

[Andrew Davis]

From: Hari Nagalla 

The AM62x SoCs of the TI K3 family have a Cortex M4F core in the MCU
domain. This core can be used by non safety applications as a remote
processor. When used as a remote processor with virtio/rpmessage IPC,
two carveout reserved memory nodes are needed.

Disable by default as this node is not complete until mailbox data
is provided in the board level DT.

Signed-off-by: Hari Nagalla 
Signed-off-by: Andrew Davis 
---
 arch/arm64/boot/dts/ti/k3-am62-mcu.dtsi | 13 +
 1 file changed, 13 insertions(+)

diff --git a/arch/arm64/boot/dts/ti/k3-am62-mcu.dtsi 
b/arch/arm64/boot/dts/ti/k3-am62-mcu.dtsi
index e66d486ef1f21..7f6f0007e8e81 100644
--- a/arch/arm64/boot/dts/ti/k3-am62-mcu.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am62-mcu.dtsi
@@ -173,4 +173,17 @@ mcu_mcan1: can@4e18000 {
bosch,mram-cfg = <0x0 128 64 64 64 64 32 32>;
status = "disabled";
};
+
+   mcu_m4fss: m4fss@500 {
+   compatible = "ti,am64-m4fss";
+   reg = <0x00 0x500 0x00 0x3>,
+ <0x00 0x504 0x00 0x1>;
+   reg-names = "iram", "dram";
+   resets = <&k3_reset 9 1>;
+   firmware-name = "am62-mcu-m4f0_0-fw";
+   ti,sci = <&dmsc>;
+   ti,sci-dev-id = <9>;
+   ti,sci-proc-ids = <0x18 0xff>;
+   status = "disabled";
+   };
 };
-- 
2.39.2

Re: [PATCH] nvdimm: make nd_class constant

[Dan Williams]

Greg Kroah-Hartman wrote:
> Now that the driver core allows for struct class to be in read-only
> memory, we should make all 'class' structures declared at build time
> placing them into read-only memory, instead of having to be dynamically
> allocated at runtime.

Change looks good to me,

Reviewed-by: Dan Williams 

...changelog grammar tripped me up though, how about:

"Now that the driver core allows for struct class to be in read-only
memory, it is possible to make all 'class' structures be declared at
build time. Move the class to a 'static const' declaration and register
it rather than dynamically create it."

Re: [lvc-project] [PATCH] remoteproc: imx_rproc: Adjust phandle parsing issue while remapping optional addresses in imx_rproc_addr_init()

[Fedor Pchelkin]

On Mon, 10. Jun 10:47, Mathieu Poirier wrote:
> On Thu, Jun 06, 2024 at 10:52:04AM +0300, Aleksandr Mishin wrote:
> > In imx_rproc_addr_init() "nph = of_count_phandle_with_args()" just counts
> > number of phandles. But phandles may be empty. So of_parse_phandle() in
> > the parsing loop (0 < a < nph) may return NULL which is later dereferenced.
> > Adjust this issue by adding NULL-return check.
> > 
> > Found by Linux Verification Center (linuxtesting.org) with SVACE.
> > 
> > Fixes: a0ff4aa6f010 ("remoteproc: imx_rproc: add a NXP/Freescale imx_rproc 
> > driver")
> > Signed-off-by: Aleksandr Mishin 
> > ---
> >  drivers/remoteproc/imx_rproc.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
> > index 5a3fb902acc9..39eacd90af14 100644
> > --- a/drivers/remoteproc/imx_rproc.c
> > +++ b/drivers/remoteproc/imx_rproc.c
> > @@ -726,6 +726,8 @@ static int imx_rproc_addr_init(struct imx_rproc *priv,
> > struct resource res;
> >  
> > node = of_parse_phandle(np, "memory-region", a);
> > +   if (!node)
> 
> You're missing an "of_node_put()" before continuing.
> 

The node is NULL in this case so of_node_put() is not needed..?

Btw, there is a "rsc-table" node->name check in the the end of the loop
body. It was added recently with commit 5e4c1243071d ("remoteproc:
imx_rproc: support remote cores booted before Linux Kernel"). Seems to me
it forgot that of_node_put() is called way before that.

Also commit 61afafe8b938 ("remoteproc: imx_rproc: Fix refcount leak in
imx_rproc_addr_init") was dealing with the last of_node_put() call here
but it's still not in the right place I'd say.

> > +   continue;
> > /* Not map vdevbuffer, vdevring region */
> > if (!strncmp(node->name, "vdev", strlen("vdev"))) {
> > of_node_put(node);
> > -- 
> > 2.30.2
> > 
> > 

Re: [PATCH v3] dt-bindings: remoteproc: k3-dsp: correct optional sram properties for AM62A SoCs

[Mathieu Poirier]

On Tue, Jun 04, 2024 at 12:14:50PM -0500, Hari Nagalla wrote:
> The C7xv-dsp on AM62A have 32KB L1 I-cache and a 64KB L1 D-cache. It
> does not have an addressable l1dram . So, remove this optional sram
> property from the bindings to fix device tree build warnings.
> 
> Signed-off-by: Hari Nagalla 
> ---
> Changes in v3:
> *) Use allOf keyword with separate ifs for each variant instead 
>of nested if/else conditions.
> 
> v2: https://lore.kernel.org/all/20240530164816.1051-1-hnaga...@ti.com/
> 
>  .../bindings/remoteproc/ti,k3-dsp-rproc.yaml  | 89 +++
>  1 file changed, 51 insertions(+), 38 deletions(-)
>

Applied

Thanks,
Mathieu

> diff --git 
> a/Documentation/devicetree/bindings/remoteproc/ti,k3-dsp-rproc.yaml 
> b/Documentation/devicetree/bindings/remoteproc/ti,k3-dsp-rproc.yaml
> index 9768db8663eb..b51bb863d759 100644
> --- a/Documentation/devicetree/bindings/remoteproc/ti,k3-dsp-rproc.yaml
> +++ b/Documentation/devicetree/bindings/remoteproc/ti,k3-dsp-rproc.yaml
> @@ -25,9 +25,6 @@ description: |
>host processor (Arm CorePac) to perform the device management of the remote
>processor and to communicate with the remote processor.
>  
> -allOf:
> -  - $ref: /schemas/arm/keystone/ti,k3-sci-common.yaml#
> -
>  properties:
>compatible:
>  enum:
> @@ -89,41 +86,57 @@ properties:
>should be defined as per the generic bindings in,
>Documentation/devicetree/bindings/sram/sram.yaml
>  
> -if:
> -  properties:
> -compatible:
> -  enum:
> -- ti,j721e-c66-dsp
> -then:
> -  properties:
> -reg:
> -  items:
> -- description: Address and Size of the L2 SRAM internal memory region
> -- description: Address and Size of the L1 PRAM internal memory region
> -- description: Address and Size of the L1 DRAM internal memory region
> -reg-names:
> -  items:
> -- const: l2sram
> -- const: l1pram
> -- const: l1dram
> -else:
> -  if:
> -properties:
> -  compatible:
> -enum:
> -  - ti,am62a-c7xv-dsp
> -  - ti,j721e-c71-dsp
> -  - ti,j721s2-c71-dsp
> -  then:
> -properties:
> -  reg:
> -items:
> -  - description: Address and Size of the L2 SRAM internal memory 
> region
> -  - description: Address and Size of the L1 DRAM internal memory 
> region
> -  reg-names:
> -items:
> -  - const: l2sram
> -  - const: l1dram
> +allOf:
> +  - if:
> +  properties:
> +compatible:
> +  enum:
> +- ti,j721e-c66-dsp
> +then:
> +  properties:
> +reg:
> +  items:
> +- description: Address and Size of the L2 SRAM internal memory 
> region
> +- description: Address and Size of the L1 PRAM internal memory 
> region
> +- description: Address and Size of the L1 DRAM internal memory 
> region
> +reg-names:
> +  items:
> +- const: l2sram
> +- const: l1pram
> +- const: l1dram
> +
> +  - if:
> +  properties:
> +compatible:
> +  enum:
> +- ti,j721e-c71-dsp
> +- ti,j721s2-c71-dsp
> +then:
> +  properties:
> +reg:
> +  items:
> +- description: Address and Size of the L2 SRAM internal memory 
> region
> +- description: Address and Size of the L1 DRAM internal memory 
> region
> +reg-names:
> +  items:
> +- const: l2sram
> +- const: l1dram
> +
> +  - if:
> +  properties:
> +compatible:
> +  enum:
> +- ti,am62a-c7xv-dsp
> +then:
> +  properties:
> +reg:
> +  items:
> +- description: Address and Size of the L2 SRAM internal memory 
> region
> +reg-names:
> +  items:
> +- const: l2sram
> +
> +  - $ref: /schemas/arm/keystone/ti,k3-sci-common.yaml#
>  
>  required:
>- compatible
> -- 
> 2.34.1
> 

Re: [PATCH] rpmsg: char: add missing MODULE_DESCRIPTION() macro

[Mathieu Poirier]

On Tue, Jun 04, 2024 at 06:53:44PM -0700, Jeff Johnson wrote:
> make allmodconfig && make W=1 C=1 reports:
> WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/rpmsg/rpmsg_char.o
> 
> Add the missing invocation of the MODULE_DESCRIPTION() macro.
> 
> Signed-off-by: Jeff Johnson 
> ---
>  drivers/rpmsg/rpmsg_char.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/rpmsg/rpmsg_char.c b/drivers/rpmsg/rpmsg_char.c
> index d7a342510902..73b9fa113b34 100644
> --- a/drivers/rpmsg/rpmsg_char.c
> +++ b/drivers/rpmsg/rpmsg_char.c
> @@ -566,4 +566,5 @@ static void rpmsg_chrdev_exit(void)
>  module_exit(rpmsg_chrdev_exit);
>  
>  MODULE_ALIAS("rpmsg:rpmsg_chrdev");
> +MODULE_DESCRIPTION("RPMSG device interface");
>  MODULE_LICENSE("GPL v2");
>

Applied

Thanks,
Mathieu

> ---
> base-commit: a693b9c95abd4947c2d06e05733de5d470ab6586
> change-id: 20240604-md-drivers-rpmsg_char-02914d244ec9
> 

Re: [PATCH] remoteproc: imx_rproc: Adjust phandle parsing issue while remapping optional addresses in imx_rproc_addr_init()

[Mathieu Poirier]

On Thu, Jun 06, 2024 at 10:52:04AM +0300, Aleksandr Mishin wrote:
> In imx_rproc_addr_init() "nph = of_count_phandle_with_args()" just counts
> number of phandles. But phandles may be empty. So of_parse_phandle() in
> the parsing loop (0 < a < nph) may return NULL which is later dereferenced.
> Adjust this issue by adding NULL-return check.
> 
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
> 
> Fixes: a0ff4aa6f010 ("remoteproc: imx_rproc: add a NXP/Freescale imx_rproc 
> driver")
> Signed-off-by: Aleksandr Mishin 
> ---
>  drivers/remoteproc/imx_rproc.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
> index 5a3fb902acc9..39eacd90af14 100644
> --- a/drivers/remoteproc/imx_rproc.c
> +++ b/drivers/remoteproc/imx_rproc.c
> @@ -726,6 +726,8 @@ static int imx_rproc_addr_init(struct imx_rproc *priv,
>   struct resource res;
>  
>   node = of_parse_phandle(np, "memory-region", a);
> + if (!node)

You're missing an "of_node_put()" before continuing.

> + continue;
>   /* Not map vdevbuffer, vdevring region */
>   if (!strncmp(node->name, "vdev", strlen("vdev"))) {
>   of_node_put(node);
> -- 
> 2.30.2
> 
> 

Re: [PATCH v6 2/5] remoteproc: Add TEE support

[Arnaud POULIQUEN]

On 6/7/24 11:33, Arnaud Pouliquen wrote:
> Add a remoteproc TEE (Trusted Execution Environment) driver
> that will be probed by the TEE bus. If the associated Trusted
> application is supported on secure part this driver offers a client
> interface to load a firmware in the secure part.
> This firmware could be authenticated by the secure trusted application.
> 
> Signed-off-by: Arnaud Pouliquen 
> ---
> update from previous version
> - make tee_rproc_get_loaded_rsc_table() local and replace this API by
>   tee_rproc_find_loaded_rsc_table()
> - map and unmap the resource table in tee_rproc_parse_fw to make a cached copy
> - use the new rproc_pa_to_va() API to map the resource table memory declared 
> in carevout
> - remove tee_rproc_release_loaded_rsc_table as no more used.
> ---
>  drivers/remoteproc/Kconfig  |  10 +
>  drivers/remoteproc/Makefile |   1 +
>  drivers/remoteproc/tee_remoteproc.c | 451 
>  include/linux/remoteproc.h  |   4 +
>  include/linux/tee_remoteproc.h  |  99 ++
>  5 files changed, 565 insertions(+)
>  create mode 100644 drivers/remoteproc/tee_remoteproc.c
>  create mode 100644 include/linux/tee_remoteproc.h
> 
> diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
> index 48845dc8fa85..6c1c07202276 100644
> --- a/drivers/remoteproc/Kconfig
> +++ b/drivers/remoteproc/Kconfig
> @@ -365,6 +365,16 @@ config XLNX_R5_REMOTEPROC
>  
> It's safe to say N if not interested in using RPU r5f cores.
>  
> +
> +config TEE_REMOTEPROC
> + tristate "Remoteproc support by a TEE application"
> + depends on OPTEE
> + help
> +   Support a remote processor with a TEE application. The Trusted
> +   Execution Context is responsible for loading the trusted firmware
> +   image and managing the remote processor's lifecycle.
> +   This can be either built-in or a loadable module.
> +
>  endif # REMOTEPROC
>  
>  endmenu
> diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
> index 91314a9b43ce..fa8daebce277 100644
> --- a/drivers/remoteproc/Makefile
> +++ b/drivers/remoteproc/Makefile
> @@ -36,6 +36,7 @@ obj-$(CONFIG_RCAR_REMOTEPROC)   += rcar_rproc.o
>  obj-$(CONFIG_ST_REMOTEPROC)  += st_remoteproc.o
>  obj-$(CONFIG_ST_SLIM_REMOTEPROC) += st_slim_rproc.o
>  obj-$(CONFIG_STM32_RPROC)+= stm32_rproc.o
> +obj-$(CONFIG_TEE_REMOTEPROC) += tee_remoteproc.o
>  obj-$(CONFIG_TI_K3_DSP_REMOTEPROC)   += ti_k3_dsp_remoteproc.o
>  obj-$(CONFIG_TI_K3_R5_REMOTEPROC)+= ti_k3_r5_remoteproc.o
>  obj-$(CONFIG_XLNX_R5_REMOTEPROC) += xlnx_r5_remoteproc.o
> diff --git a/drivers/remoteproc/tee_remoteproc.c 
> b/drivers/remoteproc/tee_remoteproc.c
> new file mode 100644
> index ..9455fd9d0d2d
> --- /dev/null
> +++ b/drivers/remoteproc/tee_remoteproc.c
> @@ -0,0 +1,451 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (C) STMicroelectronics 2024 - All Rights Reserved
> + * Author: Arnaud Pouliquen 
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "remoteproc_internal.h"
> +
> +#define MAX_TEE_PARAM_ARRY_MEMBER4
> +
> +/*
> + * Authentication of the firmware and load in the remote processor memory
> + *
> + * [in]  params[0].value.a:  unique 32bit identifier of the remote processor
> + * [in]   params[1].memref:  buffer containing the image of the 
> buffer
> + */
> +#define TA_RPROC_FW_CMD_LOAD_FW  1
> +
> +/*
> + * Start the remote processor
> + *
> + * [in]  params[0].value.a:  unique 32bit identifier of the remote processor
> + */
> +#define TA_RPROC_FW_CMD_START_FW 2
> +
> +/*
> + * Stop the remote processor
> + *
> + * [in]  params[0].value.a:  unique 32bit identifier of the remote processor
> + */
> +#define TA_RPROC_FW_CMD_STOP_FW  3
> +
> +/*
> + * Return the address of the resource table, or 0 if not found
> + * No check is done to verify that the address returned is accessible by
> + * the non secure context. If the resource table is loaded in a protected
> + * memory the access by the non secure context will lead to a data abort.
> + *
> + * [in]  params[0].value.a:  unique 32bit identifier of the remote processor
> + * [out]  params[1].value.a: 32bit LSB resource table memory address
> + * [out]  params[1].value.b: 32bit MSB resource table memory address
> + * [out]  params[2].value.a: 32bit LSB resource table memory size
> + * [out]  params[2].value.b: 32bit MSB resource table memory size
> + */
> +#define TA_RPROC_FW_CMD_GET_RSC_TABLE4
> +
> +/*
> + * Return the address of the core dump
> + *
> + * [in]  params[0].value.a:  unique 32bit identifier of the remote processor
> + * [out] params[1].memref:   address of the core dump image if exist,
> + *   else return Null
> + */
> +#define TA_RPROC_FW_CMD_GET_COREDUMP 5
> +
> +struct tee_rproc_context {
> + struct li

Re: [RFC v3 net-next 1/7] net: add rx_sk to trace_kfree_skb

[Steven Rostedt]

On Thu, 6 Jun 2024 10:37:46 -0500
Yan Zhai  wrote:

> > name: kfree_skb
> > ID: 1799
> > format:
> > field:unsigned short common_type;   offset:0;   size:2; 
> > signed:0;
> > field:unsigned char common_flags;   offset:2;   size:1; 
> > signed:0;
> > field:unsigned char common_preempt_count;   offset:3;   
> > size:1; signed:0;
> > field:int common_pid;   offset:4;   size:4; signed:1;
> >
> > field:void * skbaddr;   offset:8;   size:8; signed:0;
> > field:void * location;  offset:16;  size:8; signed:0;
> > field:unsigned short protocol;  offset:24;  size:2; signed:0;
> > field:enum skb_drop_reason reason;  offset:28;  size:4; 
> > signed:0;
> >
> > Notice that "protocol" is 2 bytes in size at offset 24, but "reason" starts
> > at offset 28. This means at offset 26, there's a 2 byte hole.
> >  
> The reason I added the pointer as the last argument is trying to
> minimize the surprise to existing TP users, because for common ABIs
> it's fine to omit later arguments when defining a function, but it
> needs change and recompilation if the order of arguments changed.

Nothing should be hard coding the offsets of the fields. This is
exported to user space so that tools can see where the fields are.
That's the purpose of libtraceevent. The fields should be movable and
not affect anything. There should be no need to recompile.

> 
> Looking at the actual format after the change, it does not add a new
> hole since protocol and reason are already packed into the same 8-byte
> block, so rx_skaddr starts at 8-byte aligned offset:
> 
> # cat /sys/kernel/debug/tracing/events/skb/kfree_skb/format
> name: kfree_skb
> ID: 2260
> format:
> field:unsigned short common_type;   offset:0;
> size:2; signed:0;
> field:unsigned char common_flags;   offset:2;
> size:1; signed:0;
> field:unsigned char common_preempt_count;   offset:3;
>  size:1; signed:0;
> field:int common_pid;   offset:4;   size:4; signed:1;
> 
> field:void * skbaddr;   offset:8;   size:8; signed:0;
> field:void * location;  offset:16;  size:8; signed:0;
> field:unsigned short protocol;  offset:24;  size:2; signed:0;
> field:enum skb_drop_reason reason;  offset:28;
> size:4; signed:0;
> field:void * rx_skaddr; offset:32;  size:8; signed:0;
> 
> Do you think we still need to change the order?

Up to you, just wanted to point it out.

-- Steve

Re: [PATCH] remoteproc: mediatek: Increase MT8188 SCP core0 DRAM size

[Mathieu Poirier]

On Thu, Jun 06, 2024 at 01:00:11PM +0200, AngeloGioacchino Del Regno wrote:
> Il 06/06/24 11:06, jason-ch chen ha scritto:
> > From: Jason Chen 
> > 
> > Increase MT8188 SCP core0 DRAM size for HEVC driver.

This is telling me _what_ gets done rather than _why_ it gets done.

> > 
> 
> so the second core only gets a maximum of 0x20 of SRAM?
> I'm not sure how useful is the secondary SCP core, at this point, with so 
> little
> available SRAM but... okay, as you wish.
> 
> Reviewed-by: AngeloGioacchino Del Regno 
> 
> 
> 
> > Signed-off-by: Jason Chen 
> > ---
> >   drivers/remoteproc/mtk_scp.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/remoteproc/mtk_scp.c b/drivers/remoteproc/mtk_scp.c
> > index b885a9a041e4..2119fc62c3f2 100644
> > --- a/drivers/remoteproc/mtk_scp.c
> > +++ b/drivers/remoteproc/mtk_scp.c
> > @@ -1390,7 +1390,7 @@ static const struct mtk_scp_sizes_data 
> > default_scp_sizes = {
> >   };
> >   static const struct mtk_scp_sizes_data mt8188_scp_sizes = {
> > -   .max_dram_size = 0x50,
> > +   .max_dram_size = 0x80,

Do you require to fix a "reserved-memory" node in a device tree file to account
for this?

Thanks,
Mathieu

> > .ipi_share_buffer_size = 600,
> >   };
> 
> 

Re: [PATCH 2/2] ring-buffer: Fix a race between readers and resize checks

[Steven Rostedt]

On Fri, 7 Jun 2024 10:29:03 +0200
Petr Pavlu  wrote:

> Another option could be to try traversing the whole list in smaller
> parts and give up the reader_lock in between them. This would need some
> care to make sure that the operation completes, e.g. the code would need
> to bail out if it detects a change on cpu_buffer->pages_read.

I think I like this approach the most. Perhaps even have a counter that
gets incremented everything a new reader page is taken. And if it
detects that, it restarts the check?

To prevent a DOS, we restart 3 times at most, and then just say "the
list is OK" and exit.

So basically, we release the lock within the loop per each sub-buffer,
and then check if the reader touch it when reacquiring the lock.

-- Steve

Re: [PATCH 05/14] tracefs: replace call_rcu by kfree_rcu for simple kmem_cache_free callback

[Paul E. McKenney]

On Mon, Jun 10, 2024 at 11:22:23AM -0400, Steven Rostedt wrote:
> On Sun,  9 Jun 2024 10:27:17 +0200
> Julia Lawall  wrote:
> 
> > diff --git a/fs/tracefs/inode.c b/fs/tracefs/inode.c
> > index 7c29f4afc23d..338c52168e61 100644
> > --- a/fs/tracefs/inode.c
> > +++ b/fs/tracefs/inode.c
> > @@ -53,14 +53,6 @@ static struct inode *tracefs_alloc_inode(struct 
> > super_block *sb)
> > return &ti->vfs_inode;
> >  }
> >  
> > -static void tracefs_free_inode_rcu(struct rcu_head *rcu)
> > -{
> > -   struct tracefs_inode *ti;
> > -
> > -   ti = container_of(rcu, struct tracefs_inode, rcu);
> > -   kmem_cache_free(tracefs_inode_cachep, ti);
> 
> Does this work?
> 
> tracefs needs to be freed via the tracefs_inode_cachep. Does
> kfree_rcu() handle specific frees for objects that were not allocated
> via kmalloc()?

A recent change to kfree() allows it to correctly handle memory allocated
via kmem_cache_alloc().  News to me as of a few weeks ago.  ;-)

Thanx, Paul

> -- Steve
> 
> 
> > -}
> > -
> >  static void tracefs_free_inode(struct inode *inode)
> >  {
> > struct tracefs_inode *ti = get_tracefs(inode);
> > @@ -70,7 +62,7 @@ static void tracefs_free_inode(struct inode *inode)
> > list_del_rcu(&ti->list);
> > spin_unlock_irqrestore(&tracefs_inode_lock, flags);
> >  
> > -   call_rcu(&ti->rcu, tracefs_free_inode_rcu);
> > +   kfree_rcu(ti, rcu);
> >  }
> >  
> >  static ssize_t default_read_file(struct file *file, char __user *buf,
> 

[PATCH v5] remoteproc: xlnx: add attach detach support

[Tanmay Shah]

It is possible that remote processor is already running before
linux boot or remoteproc platform driver probe. Implement required
remoteproc framework ops to provide resource table address and
connect or disconnect with remote processor in such case.

Signed-off-by: Tanmay Shah 
---

Changes in v5:
  - Fix comment on assigning DETACHED state to remoteproc instance
during driver probe.
  - Fix patch subject and remove "drivers"

Changes in v4:
  - Move change log out of commit text

Changes in v3:
  - Drop SRAM patch from the series
  - Change type from "struct resource_table *" to void __iomem *
  - Change comment format from /** to /*
  - Remove unmap of resource table va address during detach, allowing
attach-detach-reattach use case.
  - Unmap rsc_data_va after retrieving resource table data structure.
  - Unmap resource table va during driver remove op

Changes in v2:
  - Fix typecast warnings reported using sparse tool.
  - Fix following sparse warnings:

 drivers/remoteproc/xlnx_r5_remoteproc.c | 173 +++-
 1 file changed, 169 insertions(+), 4 deletions(-)

diff --git a/drivers/remoteproc/xlnx_r5_remoteproc.c 
b/drivers/remoteproc/xlnx_r5_remoteproc.c
index 84243d1dff9f..6ddce5650f95 100644
--- a/drivers/remoteproc/xlnx_r5_remoteproc.c
+++ b/drivers/remoteproc/xlnx_r5_remoteproc.c
@@ -25,6 +25,10 @@
 /* RX mailbox client buffer max length */
 #define MBOX_CLIENT_BUF_MAX(IPI_BUF_LEN_MAX + \
 sizeof(struct zynqmp_ipi_message))
+
+#define RSC_TBL_XLNX_MAGIC ((uint32_t)'x' << 24 | (uint32_t)'a' << 16 | \
+(uint32_t)'m' << 8 | (uint32_t)'p')
+
 /*
  * settings for RPU cluster mode which
  * reflects possible values of xlnx,cluster-mode dt-property
@@ -73,6 +77,26 @@ struct mbox_info {
struct mbox_chan *rx_chan;
 };
 
+/**
+ * struct rsc_tbl_data
+ *
+ * Platform specific data structure used to sync resource table address.
+ * It's important to maintain order and size of each field on remote side.
+ *
+ * @version: version of data structure
+ * @magic_num: 32-bit magic number.
+ * @comp_magic_num: complement of above magic number
+ * @rsc_tbl_size: resource table size
+ * @rsc_tbl: resource table address
+ */
+struct rsc_tbl_data {
+   const int version;
+   const u32 magic_num;
+   const u32 comp_magic_num;
+   const u32 rsc_tbl_size;
+   const uintptr_t rsc_tbl;
+} __packed;
+
 /*
  * Hardcoded TCM bank values. This will stay in driver to maintain backward
  * compatibility with device-tree that does not have TCM information.
@@ -95,20 +119,24 @@ static const struct mem_bank_data 
zynqmp_tcm_banks_lockstep[] = {
 /**
  * struct zynqmp_r5_core
  *
+ * @rsc_tbl_va: resource table virtual address
  * @dev: device of RPU instance
  * @np: device node of RPU instance
  * @tcm_bank_count: number TCM banks accessible to this RPU
  * @tcm_banks: array of each TCM bank data
  * @rproc: rproc handle
+ * @rsc_tbl_size: resource table size retrieved from remote
  * @pm_domain_id: RPU CPU power domain id
  * @ipi: pointer to mailbox information
  */
 struct zynqmp_r5_core {
+   void __iomem *rsc_tbl_va;
struct device *dev;
struct device_node *np;
int tcm_bank_count;
struct mem_bank_data **tcm_banks;
struct rproc *rproc;
+   u32 rsc_tbl_size;
u32 pm_domain_id;
struct mbox_info *ipi;
 };
@@ -621,10 +649,19 @@ static int zynqmp_r5_rproc_prepare(struct rproc *rproc)
 {
int ret;
 
-   ret = add_tcm_banks(rproc);
-   if (ret) {
-   dev_err(&rproc->dev, "failed to get TCM banks, err %d\n", ret);
-   return ret;
+   /*
+* For attach/detach use case, Firmware is already loaded so
+* TCM isn't really needed at all. Also, for security TCM can be
+* locked in such case and linux may not have access at all.
+* So avoid adding TCM banks. TCM power-domains requested during attach
+* callback.
+*/
+   if (rproc->state != RPROC_DETACHED) {
+   ret = add_tcm_banks(rproc);
+   if (ret) {
+   dev_err(&rproc->dev, "failed to get TCM banks, err 
%d\n", ret);
+   return ret;
+   }
}
 
ret = add_mem_regions_carveout(rproc);
@@ -662,6 +699,120 @@ static int zynqmp_r5_rproc_unprepare(struct rproc *rproc)
return 0;
 }
 
+static struct resource_table *zynqmp_r5_get_loaded_rsc_table(struct rproc 
*rproc,
+size_t *size)
+{
+   struct zynqmp_r5_core *r5_core;
+
+   r5_core = rproc->priv;
+
+   *size = r5_core->rsc_tbl_size;
+
+   return (struct resource_table *)r5_core->rsc_tbl_va;
+}
+
+static int zynqmp_r5_get_rsc_table_va(struct zynqmp_r5_core *r5_core)
+{
+   struct resource_table *rsc_tbl_addr;
+   struct device *dev = r5_core->dev;
+   struct rsc_tbl_data *rsc_data_va;
+ 

Re: [PATCH 05/14] tracefs: replace call_rcu by kfree_rcu for simple kmem_cache_free callback

[Steven Rostedt]

On Sun,  9 Jun 2024 10:27:17 +0200
Julia Lawall  wrote:

> diff --git a/fs/tracefs/inode.c b/fs/tracefs/inode.c
> index 7c29f4afc23d..338c52168e61 100644
> --- a/fs/tracefs/inode.c
> +++ b/fs/tracefs/inode.c
> @@ -53,14 +53,6 @@ static struct inode *tracefs_alloc_inode(struct 
> super_block *sb)
>   return &ti->vfs_inode;
>  }
>  
> -static void tracefs_free_inode_rcu(struct rcu_head *rcu)
> -{
> - struct tracefs_inode *ti;
> -
> - ti = container_of(rcu, struct tracefs_inode, rcu);
> - kmem_cache_free(tracefs_inode_cachep, ti);

Does this work?

tracefs needs to be freed via the tracefs_inode_cachep. Does
kfree_rcu() handle specific frees for objects that were not allocated
via kmalloc()?

-- Steve


> -}
> -
>  static void tracefs_free_inode(struct inode *inode)
>  {
>   struct tracefs_inode *ti = get_tracefs(inode);
> @@ -70,7 +62,7 @@ static void tracefs_free_inode(struct inode *inode)
>   list_del_rcu(&ti->list);
>   spin_unlock_irqrestore(&tracefs_inode_lock, flags);
>  
> - call_rcu(&ti->rcu, tracefs_free_inode_rcu);
> + kfree_rcu(ti, rcu);
>  }
>  
>  static ssize_t default_read_file(struct file *file, char __user *buf,

[PATCH v3 1/1] dt-bindings: remoteproc: imx_rproc: add minItems for power-domain

[Frank Li]

"fsl,imx8qxp-cm4" and "fsl,imx8qm-cm4" need minimum 2 power domains. Other
platform doesn't require 'power-domain'.

Signed-off-by: Frank Li 
---

Notes:
Change from v2 to v3
- only imx8qxp and imx8qm need power-domain, other platform don't need it.
- update commit message.

Change from v1 to v2
- set minitem to 2 at top
- Add imx8qm compatible string also
- use not logic to handle difference compatible string restriction
- update commit message.

pass dt_binding_check.

make ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- -j8  dt_binding_check 
DT_SCHEMA_FILES=fsl,imx-rproc.yaml
  SCHEMA  Documentation/devicetree/bindings/processed-schema.json
  CHKDT   Documentation/devicetree/bindings
  LINTDocumentation/devicetree/bindings
  DTEX
Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.example.dts
  DTC_CHK 
Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.example.dtb

 .../bindings/remoteproc/fsl,imx-rproc.yaml| 15 +++
 1 file changed, 15 insertions(+)

diff --git a/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml 
b/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml
index df36e29d974ca..57d75acb0b5e5 100644
--- a/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml
+++ b/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml
@@ -59,6 +59,7 @@ properties:
 maxItems: 32
 
   power-domains:
+minItems: 2
 maxItems: 8
 
   fsl,auto-boot:
@@ -99,6 +100,20 @@ allOf:
   properties:
 fsl,iomuxc-gpr: false
 
+  - if:
+  properties:
+compatible:
+  contains:
+enum:
+  - fsl,imx8qxp-cm4
+  - fsl,imx8qm-cm4
+then:
+  required:
+- power-domains
+else:
+  properties:
+power-domains: false
+
 additionalProperties: false
 
 examples:
-- 
2.34.1

[PATCH 1/6] remoteproc: omap: Use devm_rproc_alloc() helper

[Andrew Davis]

Use the device lifecycle managed allocation function. This helps prevent
mistakes like freeing out of order in cleanup functions and forgetting to
free on error paths.

Signed-off-by: Andrew Davis 
---
 drivers/remoteproc/omap_remoteproc.c | 20 
 1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/drivers/remoteproc/omap_remoteproc.c 
b/drivers/remoteproc/omap_remoteproc.c
index 8f50ab80e56f4..e91e016583802 100644
--- a/drivers/remoteproc/omap_remoteproc.c
+++ b/drivers/remoteproc/omap_remoteproc.c
@@ -1305,8 +1305,8 @@ static int omap_rproc_probe(struct platform_device *pdev)
return ret;
}
 
-   rproc = rproc_alloc(&pdev->dev, dev_name(&pdev->dev), &omap_rproc_ops,
-   firmware, sizeof(*oproc));
+   rproc = devm_rproc_alloc(&pdev->dev, dev_name(&pdev->dev), 
&omap_rproc_ops,
+firmware, sizeof(*oproc));
if (!rproc)
return -ENOMEM;
 
@@ -1318,15 +1318,15 @@ static int omap_rproc_probe(struct platform_device 
*pdev)
 
ret = omap_rproc_of_get_internal_memories(pdev, rproc);
if (ret)
-   goto free_rproc;
+   return ret;
 
ret = omap_rproc_get_boot_data(pdev, rproc);
if (ret)
-   goto free_rproc;
+   return ret;
 
ret = omap_rproc_of_get_timers(pdev, rproc);
if (ret)
-   goto free_rproc;
+   return ret;
 
init_completion(&oproc->pm_comp);
oproc->autosuspend_delay = DEFAULT_AUTOSUSPEND_DELAY;
@@ -1337,10 +1337,8 @@ static int omap_rproc_probe(struct platform_device *pdev)
pm_runtime_set_autosuspend_delay(&pdev->dev, oproc->autosuspend_delay);
 
oproc->fck = devm_clk_get(&pdev->dev, 0);
-   if (IS_ERR(oproc->fck)) {
-   ret = PTR_ERR(oproc->fck);
-   goto free_rproc;
-   }
+   if (IS_ERR(oproc->fck))
+   return PTR_ERR(oproc->fck);
 
ret = of_reserved_mem_device_init(&pdev->dev);
if (ret) {
@@ -1359,8 +1357,7 @@ static int omap_rproc_probe(struct platform_device *pdev)
 
 release_mem:
of_reserved_mem_device_release(&pdev->dev);
-free_rproc:
-   rproc_free(rproc);
+
return ret;
 }
 
@@ -1369,7 +1366,6 @@ static void omap_rproc_remove(struct platform_device 
*pdev)
struct rproc *rproc = platform_get_drvdata(pdev);
 
rproc_del(rproc);
-   rproc_free(rproc);
of_reserved_mem_device_release(&pdev->dev);
 }
 
-- 
2.39.2

[PATCH 4/6] remoteproc: da8xx: Use devm_rproc_alloc() helper

[Andrew Davis]

Use the device lifecycle managed allocation function. This helps prevent
mistakes like freeing out of order in cleanup functions and forgetting to
free on error paths.

Signed-off-by: Andrew Davis 
---
 drivers/remoteproc/da8xx_remoteproc.c | 15 ++-
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/drivers/remoteproc/da8xx_remoteproc.c 
b/drivers/remoteproc/da8xx_remoteproc.c
index 9041a0e07fb25..c8b7576937733 100644
--- a/drivers/remoteproc/da8xx_remoteproc.c
+++ b/drivers/remoteproc/da8xx_remoteproc.c
@@ -295,8 +295,8 @@ static int da8xx_rproc_probe(struct platform_device *pdev)
}
}
 
-   rproc = rproc_alloc(dev, "dsp", &da8xx_rproc_ops, da8xx_fw_name,
-   sizeof(*drproc));
+   rproc = devm_rproc_alloc(dev, "dsp", &da8xx_rproc_ops, da8xx_fw_name,
+sizeof(*drproc));
if (!rproc) {
ret = -ENOMEM;
goto free_mem;
@@ -313,7 +313,7 @@ static int da8xx_rproc_probe(struct platform_device *pdev)
 
ret = da8xx_rproc_get_internal_memories(pdev, drproc);
if (ret)
-   goto free_rproc;
+   goto free_mem;
 
platform_set_drvdata(pdev, rproc);
 
@@ -323,7 +323,7 @@ static int da8xx_rproc_probe(struct platform_device *pdev)
rproc);
if (ret) {
dev_err(dev, "devm_request_threaded_irq error: %d\n", ret);
-   goto free_rproc;
+   goto free_mem;
}
 
/*
@@ -333,7 +333,7 @@ static int da8xx_rproc_probe(struct platform_device *pdev)
 */
ret = reset_control_assert(dsp_reset);
if (ret)
-   goto free_rproc;
+   goto free_mem;
 
drproc->chipsig = chipsig;
drproc->bootreg = bootreg;
@@ -344,13 +344,11 @@ static int da8xx_rproc_probe(struct platform_device *pdev)
ret = rproc_add(rproc);
if (ret) {
dev_err(dev, "rproc_add failed: %d\n", ret);
-   goto free_rproc;
+   goto free_mem;
}
 
return 0;
 
-free_rproc:
-   rproc_free(rproc);
 free_mem:
if (dev->of_node)
of_reserved_mem_device_release(dev);
@@ -371,7 +369,6 @@ static void da8xx_rproc_remove(struct platform_device *pdev)
disable_irq(drproc->irq);
 
rproc_del(rproc);
-   rproc_free(rproc);
if (dev->of_node)
of_reserved_mem_device_release(dev);
 }
-- 
2.39.2

[PATCH 2/6] remoteproc: omap: Use devm action to release reserved memory

[Andrew Davis]

This helps prevent mistakes like freeing out of order in cleanup functions
and forgetting to free on error paths.

Signed-off-by: Andrew Davis 
---
 drivers/remoteproc/omap_remoteproc.c | 18 +++---
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/remoteproc/omap_remoteproc.c 
b/drivers/remoteproc/omap_remoteproc.c
index e91e016583802..df46be84658f7 100644
--- a/drivers/remoteproc/omap_remoteproc.c
+++ b/drivers/remoteproc/omap_remoteproc.c
@@ -1277,6 +1277,13 @@ static int omap_rproc_of_get_timers(struct 
platform_device *pdev,
return 0;
 }
 
+static void omap_rproc_mem_release(void *data)
+{
+   struct device *dev = data;
+
+   of_reserved_mem_device_release(dev);
+}
+
 static int omap_rproc_probe(struct platform_device *pdev)
 {
struct device_node *np = pdev->dev.of_node;
@@ -1346,19 +1353,17 @@ static int omap_rproc_probe(struct platform_device 
*pdev)
dev_warn(&pdev->dev, "Typically this should be provided,\n");
dev_warn(&pdev->dev, "only omit if you know what you are 
doing.\n");
}
+   ret = devm_add_action_or_reset(&pdev->dev, omap_rproc_mem_release, 
&pdev->dev);
+   if (ret)
+   return ret;
 
platform_set_drvdata(pdev, rproc);
 
ret = rproc_add(rproc);
if (ret)
-   goto release_mem;
+   return ret;
 
return 0;
-
-release_mem:
-   of_reserved_mem_device_release(&pdev->dev);
-
-   return ret;
 }
 
 static void omap_rproc_remove(struct platform_device *pdev)
@@ -1366,7 +1371,6 @@ static void omap_rproc_remove(struct platform_device 
*pdev)
struct rproc *rproc = platform_get_drvdata(pdev);
 
rproc_del(rproc);
-   of_reserved_mem_device_release(&pdev->dev);
 }
 
 static const struct dev_pm_ops omap_rproc_pm_ops = {
-- 
2.39.2

[PATCH 5/6] remoteproc: da8xx: Use devm action to release reserved memory

[Andrew Davis]

This helps prevent mistakes like freeing out of order in cleanup functions
and forgetting to free on error paths.

Signed-off-by: Andrew Davis 
---
 drivers/remoteproc/da8xx_remoteproc.c | 29 +--
 1 file changed, 14 insertions(+), 15 deletions(-)

diff --git a/drivers/remoteproc/da8xx_remoteproc.c 
b/drivers/remoteproc/da8xx_remoteproc.c
index c8b7576937733..1ce91516fc6e5 100644
--- a/drivers/remoteproc/da8xx_remoteproc.c
+++ b/drivers/remoteproc/da8xx_remoteproc.c
@@ -233,6 +233,13 @@ static int da8xx_rproc_get_internal_memories(struct 
platform_device *pdev,
return 0;
 }
 
+static void da8xx_rproc_mem_release(void *data)
+{
+   struct device *dev = data;
+
+   of_reserved_mem_device_release(dev);
+}
+
 static int da8xx_rproc_probe(struct platform_device *pdev)
 {
struct device *dev = &pdev->dev;
@@ -293,14 +300,13 @@ static int da8xx_rproc_probe(struct platform_device *pdev)
ret);
return ret;
}
+   devm_add_action_or_reset(&pdev->dev, da8xx_rproc_mem_release, 
&pdev->dev);
}
 
rproc = devm_rproc_alloc(dev, "dsp", &da8xx_rproc_ops, da8xx_fw_name,
 sizeof(*drproc));
-   if (!rproc) {
-   ret = -ENOMEM;
-   goto free_mem;
-   }
+   if (!rproc)
+   return -ENOMEM;
 
/* error recovery is not supported at present */
rproc->recovery_disabled = true;
@@ -313,7 +319,7 @@ static int da8xx_rproc_probe(struct platform_device *pdev)
 
ret = da8xx_rproc_get_internal_memories(pdev, drproc);
if (ret)
-   goto free_mem;
+   return ret;
 
platform_set_drvdata(pdev, rproc);
 
@@ -323,7 +329,7 @@ static int da8xx_rproc_probe(struct platform_device *pdev)
rproc);
if (ret) {
dev_err(dev, "devm_request_threaded_irq error: %d\n", ret);
-   goto free_mem;
+   return ret;
}
 
/*
@@ -333,7 +339,7 @@ static int da8xx_rproc_probe(struct platform_device *pdev)
 */
ret = reset_control_assert(dsp_reset);
if (ret)
-   goto free_mem;
+   return ret;
 
drproc->chipsig = chipsig;
drproc->bootreg = bootreg;
@@ -344,15 +350,10 @@ static int da8xx_rproc_probe(struct platform_device *pdev)
ret = rproc_add(rproc);
if (ret) {
dev_err(dev, "rproc_add failed: %d\n", ret);
-   goto free_mem;
+   return ret;
}
 
return 0;
-
-free_mem:
-   if (dev->of_node)
-   of_reserved_mem_device_release(dev);
-   return ret;
 }
 
 static void da8xx_rproc_remove(struct platform_device *pdev)
@@ -369,8 +370,6 @@ static void da8xx_rproc_remove(struct platform_device *pdev)
disable_irq(drproc->irq);
 
rproc_del(rproc);
-   if (dev->of_node)
-   of_reserved_mem_device_release(dev);
 }
 
 static const struct of_device_id davinci_rproc_of_match[] __maybe_unused = {
-- 
2.39.2

[PATCH 6/6] remoteproc: da8xx: Use devm_rproc_add() helper

[Andrew Davis]

Use the device lifecycle managed add function. This helps prevent mistakes
like deleting out of order in cleanup functions and forgetting to delete
on error paths.

Signed-off-by: Andrew Davis 
---
 drivers/remoteproc/da8xx_remoteproc.c | 21 +
 1 file changed, 1 insertion(+), 20 deletions(-)

diff --git a/drivers/remoteproc/da8xx_remoteproc.c 
b/drivers/remoteproc/da8xx_remoteproc.c
index 1ce91516fc6e5..c20cf33429da9 100644
--- a/drivers/remoteproc/da8xx_remoteproc.c
+++ b/drivers/remoteproc/da8xx_remoteproc.c
@@ -321,8 +321,6 @@ static int da8xx_rproc_probe(struct platform_device *pdev)
if (ret)
return ret;
 
-   platform_set_drvdata(pdev, rproc);
-
/* everything the ISR needs is now setup, so hook it up */
ret = devm_request_threaded_irq(dev, irq, da8xx_rproc_callback,
handle_event, 0, "da8xx-remoteproc",
@@ -347,7 +345,7 @@ static int da8xx_rproc_probe(struct platform_device *pdev)
drproc->irq_data = irq_data;
drproc->irq = irq;
 
-   ret = rproc_add(rproc);
+   ret = devm_rproc_add(dev, rproc);
if (ret) {
dev_err(dev, "rproc_add failed: %d\n", ret);
return ret;
@@ -356,22 +354,6 @@ static int da8xx_rproc_probe(struct platform_device *pdev)
return 0;
 }
 
-static void da8xx_rproc_remove(struct platform_device *pdev)
-{
-   struct rproc *rproc = platform_get_drvdata(pdev);
-   struct da8xx_rproc *drproc = rproc->priv;
-   struct device *dev = &pdev->dev;
-
-   /*
-* The devm subsystem might end up releasing things before
-* freeing the irq, thus allowing an interrupt to sneak in while
-* the device is being removed.  This should prevent that.
-*/
-   disable_irq(drproc->irq);
-
-   rproc_del(rproc);
-}
-
 static const struct of_device_id davinci_rproc_of_match[] __maybe_unused = {
{ .compatible = "ti,da850-dsp", },
{ /* sentinel */ },
@@ -380,7 +362,6 @@ MODULE_DEVICE_TABLE(of, davinci_rproc_of_match);
 
 static struct platform_driver da8xx_rproc_driver = {
.probe = da8xx_rproc_probe,
-   .remove_new = da8xx_rproc_remove,
.driver = {
.name = "davinci-rproc",
.of_match_table = of_match_ptr(davinci_rproc_of_match),
-- 
2.39.2

[PATCH 3/6] remoteproc: omap: Use devm_rproc_add() helper

[Andrew Davis]

Use the device lifecycle managed add function. This helps prevent mistakes
like deleting out of order in cleanup functions and forgetting to delete
on error paths.

Signed-off-by: Andrew Davis 
---
 drivers/remoteproc/omap_remoteproc.c | 10 +-
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/drivers/remoteproc/omap_remoteproc.c 
b/drivers/remoteproc/omap_remoteproc.c
index df46be84658f7..9ae2e831456d5 100644
--- a/drivers/remoteproc/omap_remoteproc.c
+++ b/drivers/remoteproc/omap_remoteproc.c
@@ -1359,20 +1359,13 @@ static int omap_rproc_probe(struct platform_device 
*pdev)
 
platform_set_drvdata(pdev, rproc);
 
-   ret = rproc_add(rproc);
+   ret = devm_rproc_add(&pdev->dev, rproc);
if (ret)
return ret;
 
return 0;
 }
 
-static void omap_rproc_remove(struct platform_device *pdev)
-{
-   struct rproc *rproc = platform_get_drvdata(pdev);
-
-   rproc_del(rproc);
-}
-
 static const struct dev_pm_ops omap_rproc_pm_ops = {
SET_SYSTEM_SLEEP_PM_OPS(omap_rproc_suspend, omap_rproc_resume)
SET_RUNTIME_PM_OPS(omap_rproc_runtime_suspend,
@@ -1381,7 +1374,6 @@ static const struct dev_pm_ops omap_rproc_pm_ops = {
 
 static struct platform_driver omap_rproc_driver = {
.probe = omap_rproc_probe,
-   .remove_new = omap_rproc_remove,
.driver = {
.name = "omap-rproc",
.pm = &omap_rproc_pm_ops,
-- 
2.39.2

Re: [PATCHv7 bpf-next 0/9] uprobe: uretprobe speed up

[Steven Rostedt]

On Wed, 5 Jun 2024 09:42:45 -0700
Andrii Nakryiko  wrote:

> Another ping. It's been two weeks since Jiri posted the last revision
> that got no more feedback to be addressed and everyone seems to be
> happy with it.

Sorry, there's been a lot going on.

> 
> This is an important speed up improvement for uprobe infrastructure in
> general and for BPF ecosystem in particular. "Uprobes are slow" is one
> of the top complaints from production BPF users, and sys_uretprobe
> approach is significantly improving the situation for return uprobes
> (aka uretprobes), potentially enabling new use cases that previously
> could have been too expensive to trace in practice and reducing the
> overhead of the existing ones.
> 
> I'd appreciate the engagement from linux-trace maintainers on this
> patch set. Given it's important for BPF and that a big part of the
> patch set is BPF-based selftests, we'd also be happy to route all this
> through the bpf-next tree (which would actually make logistics for us
> much easier, but that's not the main concern). But regardless of the
> tree, it would be nice to make a decision and go forward with it.

I'll be talking with Masami about this later today.

-- Steve

Re: [PATCH RESEND] nvdimm: add missing MODULE_DESCRIPTION() macros

[Jeff Johnson]

On 6/10/2024 6:58 AM, Ira Weiny wrote:
> Jeff Johnson wrote:
>> Fix the 'make W=1' warnings:
>> WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/nvdimm/libnvdimm.o
>> WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/nvdimm/nd_pmem.o
>> WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/nvdimm/nd_btt.o
>> WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/nvdimm/nd_e820.o
>> WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/nvdimm/of_pmem.o
>> WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/nvdimm/nd_virtio.o
> 
> Just to double check.  This is a resend of this patch?
> 
> https://lore.kernel.org/all/20240526-md-drivers-nvdimm-v1-1-172e682e7...@quicinc.com/
> 
> Dave Jiang, I'm picking up all these for the nvdimm tree and I think there
> were a couple I was not CC'ed on.  I'll coordinate with you because I'm
> still seeing a couple of these warnings on other modules in the test
> build.
> 
> Also I want to double check all the descriptions before I send for 6.11.
> 
> Jeff is it ok if I alter the text?  I know you mentioned to Jonathan you
> really just wanted to see the errors go away.

Yes, please make the text whatever makes the most sense. In most of these
cases I'm not a domain expert so I construct these descriptions based upon
code comments, Kconfig descriptions, and git descriptions, and in some cases
these are originally wrong due to cut-n-paste or the drivers have evolved so
that information is no longer accurate.

I need to add a version of that to my b4 cover letter!

/jeff

[PATCH v2 2/2] rust: add tracepoint support

[Alice Ryhl]

Make it possible to have Rust code call into tracepoints defined by C
code. It is still required that the tracepoint is declared in a C
header, and that this header is included in the input to bindgen.

Signed-off-by: Alice Ryhl 
---
 include/linux/tracepoint.h  | 18 +++-
 include/trace/define_trace.h|  7 ++
 rust/bindings/bindings_helper.h |  1 +
 rust/kernel/lib.rs  |  1 +
 rust/kernel/tracepoint.rs   | 47 +
 5 files changed, 73 insertions(+), 1 deletion(-)

diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
index 689b6d71590e..d82af4d77c9f 100644
--- a/include/linux/tracepoint.h
+++ b/include/linux/tracepoint.h
@@ -238,6 +238,20 @@ static inline struct tracepoint 
*tracepoint_ptr_deref(tracepoint_ptr_t *p)
 #define __DECLARE_TRACE_RCU(name, proto, args, cond)
 #endif
 
+/*
+ * Declare an exported function that Rust code can call to trigger this
+ * tracepoint. This function does not include the static branch; that is done
+ * in Rust to avoid a function call when the tracepoint is disabled.
+ */
+#define DEFINE_RUST_DO_TRACE(name, proto, args)
+#define DEFINE_RUST_DO_TRACE_REAL(name, proto, args)   \
+   notrace void rust_do_trace_##name(proto)\
+   {   \
+   __DO_TRACE(name,\
+   TP_ARGS(args),  \
+   cpu_online(raw_smp_processor_id()), 0); \
+   }
+
 /*
  * Make sure the alignment of the structure in the __tracepoints section will
  * not add unwanted padding between the beginning of the section and the
@@ -253,6 +267,7 @@ static inline struct tracepoint 
*tracepoint_ptr_deref(tracepoint_ptr_t *p)
extern int __traceiter_##name(data_proto);  \
DECLARE_STATIC_CALL(tp_func_##name, __traceiter_##name);\
extern struct tracepoint __tracepoint_##name;   \
+   extern void rust_do_trace_##name(proto);\
static inline void trace_##name(proto)  \
{   \
if (static_key_false(&__tracepoint_##name.key)) \
@@ -337,7 +352,8 @@ static inline struct tracepoint 
*tracepoint_ptr_deref(tracepoint_ptr_t *p)
void __probestub_##_name(void *__data, proto)   \
{   \
}   \
-   DEFINE_STATIC_CALL(tp_func_##_name, __traceiter_##_name);
+   DEFINE_STATIC_CALL(tp_func_##_name, __traceiter_##_name);   \
+   DEFINE_RUST_DO_TRACE(_name, TP_PROTO(proto), TP_ARGS(args))
 
 #define DEFINE_TRACE(name, proto, args)\
DEFINE_TRACE_FN(name, NULL, NULL, PARAMS(proto), PARAMS(args));
diff --git a/include/trace/define_trace.h b/include/trace/define_trace.h
index 00723935dcc7..b47cc036acba 100644
--- a/include/trace/define_trace.h
+++ b/include/trace/define_trace.h
@@ -72,6 +72,13 @@
 #define DECLARE_TRACE(name, proto, args)   \
DEFINE_TRACE(name, PARAMS(proto), PARAMS(args))
 
+/* If requested, create helpers for calling these tracepoints from Rust. */
+#ifdef CREATE_RUST_TRACE_POINTS
+#undef DEFINE_RUST_DO_TRACE
+#define DEFINE_RUST_DO_TRACE(name, proto, args)\
+   DEFINE_RUST_DO_TRACE_REAL(name, PARAMS(proto), PARAMS(args))
+#endif
+
 #undef TRACE_INCLUDE
 #undef __TRACE_INCLUDE
 
diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index ddb5644d4fd9..d442f9ccfc2c 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -15,6 +15,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index a0be9544996d..96f8f11c51f2 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -45,6 +45,7 @@
 pub mod sync;
 pub mod task;
 pub mod time;
+pub mod tracepoint;
 pub mod types;
 pub mod workqueue;
 
diff --git a/rust/kernel/tracepoint.rs b/rust/kernel/tracepoint.rs
new file mode 100644
index ..1005f09e0330
--- /dev/null
+++ b/rust/kernel/tracepoint.rs
@@ -0,0 +1,47 @@
+// SPDX-License-Identifier: GPL-2.0
+
+// Copyright (C) 2024 Google LLC.
+
+//! Logic for tracepoints.
+
+/// Declare the Rust entry point for a tracepoint.
+#[macro_export]
+macro_rules! declare_trace {
+($($(#[$attr:meta])* $pub:vis fn $name:ident($($argname:ident : 
$argtyp:ty),* $(,)?);)*) => {$(
+$( #[$attr] )*
+#[inline(always)]
+$pub unsafe fn $name($($argname : $argtyp),*) {
+#[cfg(CONFIG_TRACEPOINTS)]
+{
+use $crate::bindings::*;
+
+// SAFETY: It's always okay to query the static key for a 

[PATCH v2 1/2] rust: add static_key_false

[Alice Ryhl]

Add just enough support for static key so that we can use it from
tracepoints. Tracepoints rely on `static_key_false` even though it is
deprecated, so we add the same functionality to Rust.

It is not possible to use the existing C implementation of
arch_static_branch because it passes the argument `key` to inline
assembly as an 'i' parameter, so any attempt to add a C helper for this
function will fail to compile because the value of `key` must be known
at compile-time.

Signed-off-by: Alice Ryhl 
---
 rust/kernel/lib.rs|  1 +
 rust/kernel/static_key.rs | 87 +++
 scripts/Makefile.build|  2 +-
 3 files changed, 89 insertions(+), 1 deletion(-)

diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index fbd91a48ff8b..a0be9544996d 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -38,6 +38,7 @@
 pub mod prelude;
 pub mod print;
 mod static_assert;
+pub mod static_key;
 #[doc(hidden)]
 pub mod std_vendor;
 pub mod str;
diff --git a/rust/kernel/static_key.rs b/rust/kernel/static_key.rs
new file mode 100644
index ..6c3dbe14c98a
--- /dev/null
+++ b/rust/kernel/static_key.rs
@@ -0,0 +1,87 @@
+// SPDX-License-Identifier: GPL-2.0
+
+// Copyright (C) 2024 Google LLC.
+
+//! Logic for static keys.
+
+use crate::bindings::*;
+
+#[doc(hidden)]
+#[macro_export]
+#[cfg(target_arch = "x86_64")]
+macro_rules! _static_key_false {
+($key:path, $keytyp:ty, $field:ident) => {'my_label: {
+core::arch::asm!(
+r#"
+1: .byte 0x0f,0x1f,0x44,0x00,0x00
+
+.pushsection __jump_table,  "aw"
+.balign 8
+.long 1b - .
+.long {0} - .
+.quad {1} + {2} - .
+.popsection
+"#,
+label {
+break 'my_label true;
+},
+sym $key,
+const ::core::mem::offset_of!($keytyp, $field),
+);
+
+break 'my_label false;
+}};
+}
+
+#[doc(hidden)]
+#[macro_export]
+#[cfg(target_arch = "aarch64")]
+macro_rules! _static_key_false {
+($key:path, $keytyp:ty, $field:ident) => {'my_label: {
+core::arch::asm!(
+r#"
+1: nop
+
+.pushsection __jump_table,  "aw"
+.align 3
+.long 1b - ., {0} - .
+.quad {1} + {2} - .
+.popsection
+"#,
+label {
+break 'my_label true;
+},
+sym $key,
+const ::core::mem::offset_of!($keytyp, $field),
+);
+
+break 'my_label false;
+}};
+}
+
+/// Branch based on a static key.
+///
+/// Takes three arguments:
+///
+/// * `key` - the path to the static variable containing the `static_key`.
+/// * `keytyp` - the type of `key`.
+/// * `field` - the name of the field of `key` that contains the `static_key`.
+#[macro_export]
+macro_rules! static_key_false {
+// Forward to the real implementation. Separated like this so that we 
don't have to duplicate
+// the documentation.
+($key:path, $keytyp:ty, $field:ident) => {{
+// Assert that `$key` has type `$keytyp` and that `$key.$field` has 
type `static_key`.
+//
+// SAFETY: We know that `$key` is a static because otherwise the 
inline assembly will not
+// compile. The raw pointers created in this block are in-bounds of 
`$key`.
+static _TY_ASSERT: () = unsafe {
+let key: *const $keytyp = ::core::ptr::addr_of!($key);
+let _: *const $crate::bindings::static_key = 
::core::ptr::addr_of!((*key).$field);
+};
+
+$crate::static_key::_static_key_false! { $key, $keytyp, $field }
+}};
+}
+
+pub use {_static_key_false, static_key_false};
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index efacca63c897..60197c1c063f 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -263,7 +263,7 @@ $(obj)/%.lst: $(obj)/%.c FORCE
 # Compile Rust sources (.rs)
 # ---
 
-rust_allowed_features := new_uninit
+rust_allowed_features := asm_const,asm_goto,new_uninit
 
 # `--out-dir` is required to avoid temporaries being created by `rustc` in the
 # current working directory, which may be not accessible in the out-of-tree

-- 
2.45.2.505.gda0bf45e8d-goog

[PATCH v2 0/2] Tracepoints and static branch in Rust

[Alice Ryhl]

An important part of a production ready Linux kernel driver is
tracepoints. So to write production ready Linux kernel drivers in Rust,
we must be able to call tracepoints from Rust code. This patch series
adds support for calling tracepoints declared in C from Rust.

To use the tracepoint support, you must:

1. Declare the tracepoint in a C header file as usual.

2. Add #define CREATE_RUST_TRACE_POINTS next to your
   #define CREATE_TRACE_POINTS.

2. Make sure that the header file is visible to bindgen.

3. Use the declare_trace! macro in your Rust code to generate Rust
   functions that call into the tracepoint.

For example, the kernel has a tracepoint called `sched_kthread_stop`. It
is declared like this:

TRACE_EVENT(sched_kthread_stop,
TP_PROTO(struct task_struct *t),
TP_ARGS(t),
TP_STRUCT__entry(
__array(char,   comm,   TASK_COMM_LEN   )
__field(pid_t,  pid )
),
TP_fast_assign(
memcpy(__entry->comm, t->comm, TASK_COMM_LEN);
__entry->pid= t->pid;
),
TP_printk("comm=%s pid=%d", __entry->comm, __entry->pid)
);

To call the above tracepoint from Rust code, you must first ensure that
the Rust helper for the tracepoint is generated. To do this, you would
modify kernel/sched/core.c by adding #define CREATE_RUST_TRACE_POINTS.

Next, you would include include/trace/events/sched.h in
rust/bindings/bindings_helper.h so that the exported C functions are
visible to Rust, and then you would declare the tracepoint in Rust:

declare_trace! {
fn sched_kthread_stop(task: *mut task_struct);
}

This will define an inline Rust function that checks the static key,
calling into rust_do_trace_##name if the tracepoint is active. Since
these tracepoints often take raw pointers as arguments, it may be
convenient to wrap it in a safe wrapper:

mod raw {
declare_trace! {
fn sched_kthread_stop(task: *mut task_struct);
}
}

#[inline]
pub fn trace_sched_kthread_stop(task: &Task) {
// SAFETY: The pointer to `task` is valid.
unsafe { raw::sched_kthread_stop(task.as_raw()) }
}

A future expansion of the tracepoint support could generate these safe
versions automatically, but that is left as future work for now.

This is intended for use in the Rust Binder driver, which was originally
sent as an RFC [1]. The RFC did not include tracepoint support, but you
can see how it will be used in Rust Binder at [2]. The author has
verified that the tracepoint support works on Android devices.

This implementation implements support for static keys in Rust so that
the actual static branch happens in the Rust object file. However, the
__DO_TRACE body remains in C code. See v1 for an implementation where
__DO_TRACE is also implemented in Rust.

Link: 
https://lore.kernel.org/rust-for-linux/20231101-rust-binder-v1-0-08ba9197f...@google.com/
 [1]
Link: https://r.android.com/3119993 [2]
Signed-off-by: Alice Ryhl 
---
Changes in v2:
- Call into C code for __DO_TRACE.
- Drop static_call patch, as it is no longer needed.
- Link to v1: 
https://lore.kernel.org/r/20240606-tracepoint-v1-0-6551627bf...@google.com

---
Alice Ryhl (2):
  rust: add static_key_false
  rust: add tracepoint support

 include/linux/tracepoint.h  | 18 -
 include/trace/define_trace.h|  7 
 rust/bindings/bindings_helper.h |  1 +
 rust/kernel/lib.rs  |  2 +
 rust/kernel/static_key.rs   | 87 +
 rust/kernel/tracepoint.rs   | 47 ++
 scripts/Makefile.build  |  2 +-
 7 files changed, 162 insertions(+), 2 deletions(-)
---
base-commit: 1613e604df0cd359cf2a7fbd9be7a0bcfacfabd0
change-id: 20240606-tracepoint-31e15b90e471

Best regards,
-- 
Alice Ryhl 

Re: [PATCH RESEND] nvdimm: add missing MODULE_DESCRIPTION() macros

[Ira Weiny]

Jeff Johnson wrote:
> Fix the 'make W=1' warnings:
> WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/nvdimm/libnvdimm.o
> WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/nvdimm/nd_pmem.o
> WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/nvdimm/nd_btt.o
> WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/nvdimm/nd_e820.o
> WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/nvdimm/of_pmem.o
> WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/nvdimm/nd_virtio.o

Just to double check.  This is a resend of this patch?

https://lore.kernel.org/all/20240526-md-drivers-nvdimm-v1-1-172e682e7...@quicinc.com/

Dave Jiang, I'm picking up all these for the nvdimm tree and I think there
were a couple I was not CC'ed on.  I'll coordinate with you because I'm
still seeing a couple of these warnings on other modules in the test
build.

Also I want to double check all the descriptions before I send for 6.11.

Jeff is it ok if I alter the text?  I know you mentioned to Jonathan you
really just wanted to see the errors go away.

Ira

> 
> Signed-off-by: Jeff Johnson 
> ---
>  drivers/nvdimm/btt.c   | 1 +
>  drivers/nvdimm/core.c  | 1 +
>  drivers/nvdimm/e820.c  | 1 +
>  drivers/nvdimm/nd_virtio.c | 1 +
>  drivers/nvdimm/of_pmem.c   | 1 +
>  drivers/nvdimm/pmem.c  | 1 +
>  6 files changed, 6 insertions(+)
> 
> diff --git a/drivers/nvdimm/btt.c b/drivers/nvdimm/btt.c
> index 1e5aedaf8c7b..a47acc5d05df 100644
> --- a/drivers/nvdimm/btt.c
> +++ b/drivers/nvdimm/btt.c
> @@ -1721,6 +1721,7 @@ static void __exit nd_btt_exit(void)
>  
>  MODULE_ALIAS_ND_DEVICE(ND_DEVICE_BTT);
>  MODULE_AUTHOR("Vishal Verma ");
> +MODULE_DESCRIPTION("NVDIMM Block Translation Table");
>  MODULE_LICENSE("GPL v2");
>  module_init(nd_btt_init);
>  module_exit(nd_btt_exit);
> diff --git a/drivers/nvdimm/core.c b/drivers/nvdimm/core.c
> index 2023a661bbb0..f4b6fb4b9828 100644
> --- a/drivers/nvdimm/core.c
> +++ b/drivers/nvdimm/core.c
> @@ -540,6 +540,7 @@ static __exit void libnvdimm_exit(void)
>   nvdimm_devs_exit();
>  }
>  
> +MODULE_DESCRIPTION("NVDIMM (Non-Volatile Memory Device) core module");
>  MODULE_LICENSE("GPL v2");
>  MODULE_AUTHOR("Intel Corporation");
>  subsys_initcall(libnvdimm_init);
> diff --git a/drivers/nvdimm/e820.c b/drivers/nvdimm/e820.c
> index 4cd18be9d0e9..008b9aae74ff 100644
> --- a/drivers/nvdimm/e820.c
> +++ b/drivers/nvdimm/e820.c
> @@ -69,5 +69,6 @@ static struct platform_driver e820_pmem_driver = {
>  module_platform_driver(e820_pmem_driver);
>  
>  MODULE_ALIAS("platform:e820_pmem*");
> +MODULE_DESCRIPTION("NVDIMM support for e820 type-12 memory");
>  MODULE_LICENSE("GPL v2");
>  MODULE_AUTHOR("Intel Corporation");
> diff --git a/drivers/nvdimm/nd_virtio.c b/drivers/nvdimm/nd_virtio.c
> index 1f8c667c6f1e..35c8fbbba10e 100644
> --- a/drivers/nvdimm/nd_virtio.c
> +++ b/drivers/nvdimm/nd_virtio.c
> @@ -123,4 +123,5 @@ int async_pmem_flush(struct nd_region *nd_region, struct 
> bio *bio)
>   return 0;
>  };
>  EXPORT_SYMBOL_GPL(async_pmem_flush);
> +MODULE_DESCRIPTION("Virtio Persistent Memory Driver");
>  MODULE_LICENSE("GPL");
> diff --git a/drivers/nvdimm/of_pmem.c b/drivers/nvdimm/of_pmem.c
> index d3fca0ab6290..5134a8d08bf9 100644
> --- a/drivers/nvdimm/of_pmem.c
> +++ b/drivers/nvdimm/of_pmem.c
> @@ -111,5 +111,6 @@ static struct platform_driver of_pmem_region_driver = {
>  
>  module_platform_driver(of_pmem_region_driver);
>  MODULE_DEVICE_TABLE(of, of_pmem_region_match);
> +MODULE_DESCRIPTION("NVDIMM Device Tree support");
>  MODULE_LICENSE("GPL");
>  MODULE_AUTHOR("IBM Corporation");
> diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c
> index 598fe2e89bda..57cb30f8a3b8 100644
> --- a/drivers/nvdimm/pmem.c
> +++ b/drivers/nvdimm/pmem.c
> @@ -768,4 +768,5 @@ static struct nd_device_driver nd_pmem_driver = {
>  module_nd_driver(nd_pmem_driver);
>  
>  MODULE_AUTHOR("Ross Zwisler ");
> +MODULE_DESCRIPTION("NVDIMM Persistent Memory Driver");
>  MODULE_LICENSE("GPL v2");
> 
> ---
> base-commit: 416ff45264d50a983c3c0b99f0da6ee59f9acd68
> change-id: 20240526-md-drivers-nvdimm-121215a4b93f
> -- 
> Jeff Johnson 
> 

Re: [PATCH 1/3] rtla/osnoise: Use pretty formatting only on interactive tty

[Luis Claudio R. Goncalves]

This is a single patch, selected out of a test series. I messed up the
subject giving the impression that more patches were to come.

Sorry for any inconvenience,
Luis

On Mon, Jun 10, 2024 at 10:25:28AM -0300, Luis Claudio R. Goncalves wrote:
> osnoise top performs background/font color formatting that could make
> the text output confusing if not on a terminal. Use the changes from
> commit f5c0cdad6684a ("rtla/timerlat: Use pretty formatting only on
> interactive tty") as an inspiration to fix this problem.
> 
> Apply the formatting only if running on a tty, and not in quiet mode.
> 
> Suggested-by: Daniel Bristot de Oliveira 
> Reviewed-by: John Kacur 
> Reviewed-by: Clark Williams 
> Signed-off-by: Luis Claudio R. Goncalves 
> ---
>  tools/tracing/rtla/src/osnoise_top.c |   19 +++
>  1 file changed, 15 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/tracing/rtla/src/osnoise_top.c 
> b/tools/tracing/rtla/src/osnoise_top.c
> index 07ba55d4ec06f..d539fade99d48 100644
> --- a/tools/tracing/rtla/src/osnoise_top.c
> +++ b/tools/tracing/rtla/src/osnoise_top.c
> @@ -42,6 +42,7 @@ struct osnoise_top_params {
>   int hk_cpus;
>   int warmup;
>   int buffer_size;
> + int pretty_output;
>   cpu_set_t   hk_cpu_set;
>   struct sched_attr   sched_param;
>   struct trace_events *events;
> @@ -163,7 +164,9 @@ static void osnoise_top_header(struct osnoise_tool *top)
>  
>   get_duration(top->start_time, duration, sizeof(duration));
>  
> - trace_seq_printf(s, "\033[2;37;40m");
> + if (params->pretty_output)
> + trace_seq_printf(s, "\033[2;37;40m");
> +
>   trace_seq_printf(s, "  ");
>  
>   if (params->mode == MODE_OSNOISE) {
> @@ -174,12 +177,16 @@ static void osnoise_top_header(struct osnoise_tool *top)
>   }
>  
>   trace_seq_printf(s, "   ");
> - trace_seq_printf(s, "\033[0;0;0m");
> +
> + if (params->pretty_output)
> + trace_seq_printf(s, "\033[0;0;0m");
>   trace_seq_printf(s, "\n");
>  
>   trace_seq_printf(s, "duration: %9s | time is in us\n", duration);
>  
> - trace_seq_printf(s, "\033[2;30;47m");
> + if (params->pretty_output)
> + trace_seq_printf(s, "\033[2;30;47m");
> +
>   trace_seq_printf(s, "CPU Period   Runtime ");
>   trace_seq_printf(s, "   Noise ");
>   trace_seq_printf(s, " %% CPU Aval ");
> @@ -192,7 +199,8 @@ static void osnoise_top_header(struct osnoise_tool *top)
>   trace_seq_printf(s, "  IRQ  Softirq   Thread");
>  
>  eol:
> - trace_seq_printf(s, "\033[0;0;0m");
> + if (params->pretty_output)
> + trace_seq_printf(s, "\033[0;0;0m");
>   trace_seq_printf(s, "\n");
>  }
>  
> @@ -619,6 +627,9 @@ osnoise_top_apply_config(struct osnoise_tool *tool, 
> struct osnoise_top_params *p
>   auto_house_keeping(¶ms->monitored_cpus);
>   }
>  
> + if (isatty(1) && !params->quiet)
> + params->pretty_output = 1;
> +
>   return 0;
>  
>  out_err:
> -- 
> 2.45.2
> 
> 
---end quoted text---

[PATCH 1/3] rtla/osnoise: Use pretty formatting only on interactive tty

[Luis Claudio R. Goncalves]

osnoise top performs background/font color formatting that could make
the text output confusing if not on a terminal. Use the changes from
commit f5c0cdad6684a ("rtla/timerlat: Use pretty formatting only on
interactive tty") as an inspiration to fix this problem.

Apply the formatting only if running on a tty, and not in quiet mode.

Suggested-by: Daniel Bristot de Oliveira 
Reviewed-by: John Kacur 
Reviewed-by: Clark Williams 
Signed-off-by: Luis Claudio R. Goncalves 
---
 tools/tracing/rtla/src/osnoise_top.c |   19 +++
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/tools/tracing/rtla/src/osnoise_top.c 
b/tools/tracing/rtla/src/osnoise_top.c
index 07ba55d4ec06f..d539fade99d48 100644
--- a/tools/tracing/rtla/src/osnoise_top.c
+++ b/tools/tracing/rtla/src/osnoise_top.c
@@ -42,6 +42,7 @@ struct osnoise_top_params {
int hk_cpus;
int warmup;
int buffer_size;
+   int pretty_output;
cpu_set_t   hk_cpu_set;
struct sched_attr   sched_param;
struct trace_events *events;
@@ -163,7 +164,9 @@ static void osnoise_top_header(struct osnoise_tool *top)
 
get_duration(top->start_time, duration, sizeof(duration));
 
-   trace_seq_printf(s, "\033[2;37;40m");
+   if (params->pretty_output)
+   trace_seq_printf(s, "\033[2;37;40m");
+
trace_seq_printf(s, "  ");
 
if (params->mode == MODE_OSNOISE) {
@@ -174,12 +177,16 @@ static void osnoise_top_header(struct osnoise_tool *top)
}
 
trace_seq_printf(s, "   ");
-   trace_seq_printf(s, "\033[0;0;0m");
+
+   if (params->pretty_output)
+   trace_seq_printf(s, "\033[0;0;0m");
trace_seq_printf(s, "\n");
 
trace_seq_printf(s, "duration: %9s | time is in us\n", duration);
 
-   trace_seq_printf(s, "\033[2;30;47m");
+   if (params->pretty_output)
+   trace_seq_printf(s, "\033[2;30;47m");
+
trace_seq_printf(s, "CPU Period   Runtime ");
trace_seq_printf(s, "   Noise ");
trace_seq_printf(s, " %% CPU Aval ");
@@ -192,7 +199,8 @@ static void osnoise_top_header(struct osnoise_tool *top)
trace_seq_printf(s, "  IRQ  Softirq   Thread");
 
 eol:
-   trace_seq_printf(s, "\033[0;0;0m");
+   if (params->pretty_output)
+   trace_seq_printf(s, "\033[0;0;0m");
trace_seq_printf(s, "\n");
 }
 
@@ -619,6 +627,9 @@ osnoise_top_apply_config(struct osnoise_tool *tool, struct 
osnoise_top_params *p
auto_house_keeping(¶ms->monitored_cpus);
}
 
+   if (isatty(1) && !params->quiet)
+   params->pretty_output = 1;
+
return 0;
 
 out_err:
-- 
2.45.2

Re: [PATCH 6/7] interconnect: qcom: qcs404: Add regmaps and more bus descriptions

[kernel test robot]

Hi Adam,

kernel test robot noticed the following build errors:

[auto build test ERROR on robh/for-next]
[also build test ERROR on linus/master v6.10-rc3 next-20240607]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:
https://github.com/intel-lab-lkp/linux/commits/Adam-Skladowski/dt-bindings-interconnect-Add-Qualcomm-MSM8976-DT-bindings/20240610-022416
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
patch link:
https://lore.kernel.org/r/20240609182112.13032-7-a39.skl%40gmail.com
patch subject: [PATCH 6/7] interconnect: qcom: qcs404: Add regmaps and more bus 
descriptions
config: arm64-allmodconfig 
(https://download.01.org/0day-ci/archive/20240610/202406102141.1kh3lxfy-...@intel.com/config)
compiler: clang version 19.0.0git (https://github.com/llvm/llvm-project 
4403cdbaf01379de96f8d0d6ea4f51a085e37766)
reproduce (this is a W=1 build): 
(https://download.01.org/0day-ci/archive/20240610/202406102141.1kh3lxfy-...@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot 
| Closes: 
https://lore.kernel.org/oe-kbuild-all/202406102141.1kh3lxfy-...@intel.com/

All errors (new ones prefixed by >>):

>> drivers/interconnect/qcom/qcs404.c:1070:35: error: variable has incomplete 
>> type 'const struct regmap_config'
1070 | static const struct regmap_config qcs404_bimc_regmap_config = {
 |   ^
   drivers/interconnect/qcom/icc-rpm.h:136:15: note: forward declaration of 
'struct regmap_config'
 136 | const struct regmap_config *regmap_cfg;
 |  ^
   drivers/interconnect/qcom/qcs404.c:1137:35: error: variable has incomplete 
type 'const struct regmap_config'
1137 | static const struct regmap_config qcs404_pcnoc_regmap_config = {
 |   ^
   drivers/interconnect/qcom/icc-rpm.h:136:15: note: forward declaration of 
'struct regmap_config'
 136 | const struct regmap_config *regmap_cfg;
 |  ^
   drivers/interconnect/qcom/qcs404.c:1178:35: error: variable has incomplete 
type 'const struct regmap_config'
1178 | static const struct regmap_config qcs404_snoc_regmap_config = {
 |   ^
   drivers/interconnect/qcom/icc-rpm.h:136:15: note: forward declaration of 
'struct regmap_config'
 136 | const struct regmap_config *regmap_cfg;
 |  ^
   3 errors generated.


vim +1070 drivers/interconnect/qcom/qcs404.c

  1069  
> 1070  static const struct regmap_config qcs404_bimc_regmap_config = {
  1071  .reg_bits = 32,
  1072  .reg_stride = 4,
  1073  .val_bits = 32,
  1074  .max_register = 0x8,
  1075  .fast_io = true,
  1076  };
  1077  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 7/7] dt-bindings: interconnect: qcom: msm8939: Fix example

[Krzysztof Kozlowski]

On 09/06/2024 20:21, Adam Skladowski wrote:
> For now example list snoc_mm as children of bimc which is obviously
> not valid, change example and include rest of nocs in it.
> 
> Fixes: 462baaf4c628 ("dt-bindings: interconnect: qcom: Fix and separate out 
> MSM8939")
> Signed-off-by: Adam Skladowski 
> ---
>  .../bindings/interconnect/qcom,msm8939.yaml   | 22 ---
>  1 file changed, 14 insertions(+), 8 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/interconnect/qcom,msm8939.yaml 
> b/Documentation/devicetree/bindings/interconnect/qcom,msm8939.yaml
> index fd15ab5014fb..a77e6aa2fbee 100644
> --- a/Documentation/devicetree/bindings/interconnect/qcom,msm8939.yaml
> +++ b/Documentation/devicetree/bindings/interconnect/qcom,msm8939.yaml
> @@ -56,19 +56,25 @@ examples:
>- |
>  #include 
>  
> -snoc: interconnect@58 {
> -compatible = "qcom,msm8939-snoc";
> -reg = <0x0058 0x14000>;
> -#interconnect-cells = <1>;
> -};
> -
>  bimc: interconnect@40 {
>  compatible = "qcom,msm8939-bimc";
>  reg = <0x0040 0x62000>;
> -#interconnect-cells = <1>;
> +#interconnect-cells = <2>;
> +};
> +
> +pcnoc: interconnect@50 {
> +compatible = "qcom,msm8939-pcnoc";
> +reg = <0x0050 0x11000>;
> +#interconnect-cells = <2>;
> +};

Don't grow the examples.

It is enough to have one example to validate the schema and show how the
binding is used. If schema covers multiple combinations of devices and
their properties, then more than one example seems reasonable. This is
not the case. All of this is redundant... and redundant information is
not good because as this commit shows it leads to something which people
think is not correct and they find bugs. So just drop the redundant
information.

Keep only one, correct example.

Best regards,
Krzysztof

Re: [PATCH 4/7] interconnect: qcom: Add MSM8937 interconnect provider driver

[Krzysztof Kozlowski]

On 09/06/2024 20:20, Adam Skladowski wrote:
> Add driver for interconnect busses found in MSM8937 based platforms.
> The topology consists of four NoCs that are partially controlled
> by a RPM processor.
> 
> Signed-off-by: Adam Skladowski 
> ---

> +
> +static const struct of_device_id msm8937_noc_of_match[] = {
> + { .compatible = "qcom,msm8937-bimc", .data = &msm8937_bimc },
> + { .compatible = "qcom,msm8937-pcnoc", .data = &msm8937_pcnoc },
> + { .compatible = "qcom,msm8937-snoc", .data = &msm8937_snoc },
> + { .compatible = "qcom,msm8937-snoc-mm", .data = &msm8937_snoc_mm },

Please run scripts/checkpatch.pl and fix reported warnings. Then please
run `scripts/checkpatch.pl --strict` and (probably) fix more warnings.
Some warnings can be ignored, especially from --strict run, but the code
here looks like it needs a fix. Feel free to get in touch if the warning
is not clear.


Best regards,
Krzysztof

Re: [PATCH 3/7] dt-bindings: interconnect: Add Qualcomm MSM8937 DT bindings

[Krzysztof Kozlowski]

On 09/06/2024 20:20, Adam Skladowski wrote:
> Add bindings for Qualcomm MSM8937 Network-On-Chip interconnect devices.
> 
> Signed-off-by: Adam Skladowski 

Since I expect resend, all trivial nits from patch #1 apply here as well.

> ---
>  .../bindings/interconnect/qcom,msm8937.yaml   | 81 
>  .../dt-bindings/interconnect/qcom,msm8937.h   | 93 +++
>  2 files changed, 174 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/interconnect/qcom,msm8937.yaml
>  create mode 100644 include/dt-bindings/interconnect/qcom,msm8937.h
> 
> diff --git a/Documentation/devicetree/bindings/interconnect/qcom,msm8937.yaml 
> b/Documentation/devicetree/bindings/interconnect/qcom,msm8937.yaml
> new file mode 100644
> index ..39a1ca441bb2
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/interconnect/qcom,msm8937.yaml
> @@ -0,0 +1,81 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/interconnect/qcom,msm8937.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm MSM8937 Network-On-Chip interconnect
> +
> +maintainers:
> +  - Konrad Dybcio 
> +
> +description: |
> +  The Qualcomm MSM8937 interconnect providers support adjusting the
> +  bandwidth requirements between the various NoC fabrics.
> +
> +allOf:
> +  - $ref: qcom,rpm-common.yaml#
> +
> +properties:
> +  compatible:
> +enum:
> +  - qcom,msm8937-bimc
> +  - qcom,msm8937-pcnoc
> +  - qcom,msm8937-snoc
> +
> +  reg:
> +maxItems: 1
> +
> +patternProperties:
> +  '^interconnect-[a-z0-9\-]+$':
> +type: object
> +$ref: qcom,rpm-common.yaml#
> +description:
> +  The interconnect providers do not have a separate QoS register space,
> +  but share parent's space.
> +
> +allOf:
> +  - $ref: qcom,rpm-common.yaml#
> +
> +properties:
> +  compatible:
> +const: qcom,msm8937-snoc-mm
> +
> +required:
> +  - compatible
> +
> +unevaluatedProperties: false
> +
> +required:
> +  - compatible
> +  - reg
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +#include 
> +#include 
> +
> +bimc: interconnect@40 {
> +compatible = "qcom,msm8937-bimc";
> +reg = <0x0040 0x5a000>;
> +#interconnect-cells = <2>;
> +};

Drop node

> +
> +pcnoc: interconnect@50 {
> +compatible = "qcom,msm8937-pcnoc";
> +reg = <0x0050 0x13080>;
> +#interconnect-cells = <2>;
> +};

Drop node

> +



Best regards,
Krzysztof

Re: [PATCH 2/7] interconnect: qcom: Add MSM8976 interconnect provider driver

[Krzysztof Kozlowski]

On 09/06/2024 20:20, Adam Skladowski wrote:
> Add driver for interconnect busses found in MSM8976 based platforms.
> The topology consists of four NoCs that are partially controlled
> by a RPM processor.
> 
> Signed-off-by: Adam Skladowski 

> +
> +static const struct qcom_icc_desc msm8976_snoc_mm = {
> + .type = QCOM_ICC_NOC,
> + .nodes = msm8976_snoc_mm_nodes,
> + .num_nodes = ARRAY_SIZE(msm8976_snoc_mm_nodes),
> + .bus_clk_desc = &bus_2_clk,
> + .regmap_cfg = &msm8976_snoc_regmap_config,
> + .qos_offset = 0x7000,
> + .ab_coeff = 154,
> +};
> +
> +static const struct of_device_id msm8976_noc_of_match[] = {
> + { .compatible = "qcom,msm8976-bimc", .data = &msm8976_bimc },
> + { .compatible = "qcom,msm8976-pcnoc", .data = &msm8976_pcnoc },
> + { .compatible = "qcom,msm8976-snoc", .data = &msm8976_snoc },
> + { .compatible = "qcom,msm8976-snoc-mm", .data = &msm8976_snoc_mm },

Please run scripts/checkpatch.pl and fix reported warnings. Then please
run `scripts/checkpatch.pl --strict` and (probably) fix more warnings.
Some warnings can be ignored, especially from --strict run, but the code
here looks like it needs a fix. Feel free to get in touch if the warning
is not clear.

undocumented compatible



Best regards,
Krzysztof

Re: [PATCH 1/7] dt-bindings: interconnect: Add Qualcomm MSM8976 DT bindings

[Krzysztof Kozlowski]

On 09/06/2024 20:20, Adam Skladowski wrote:
> Add bindings for Qualcomm MSM8976 Network-On-Chip interconnect devices.
> 
> Signed-off-by: Adam Skladowski 

A nit, subject: drop second/last, redundant "DT bindings". The
"dt-bindings" prefix is already stating that these are bindings.
See also:
https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18

> ---
>  .../bindings/interconnect/qcom,msm8976.yaml   | 107 ++
>  .../dt-bindings/interconnect/qcom,msm8976.h   |  97 
>  2 files changed, 204 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/interconnect/qcom,msm8976.yaml
>  create mode 100644 include/dt-bindings/interconnect/qcom,msm8976.h
> 
> diff --git a/Documentation/devicetree/bindings/interconnect/qcom,msm8976.yaml 
> b/Documentation/devicetree/bindings/interconnect/qcom,msm8976.yaml
> new file mode 100644
> index ..bc9d08443e7c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/interconnect/qcom,msm8976.yaml
> @@ -0,0 +1,107 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/interconnect/qcom,msm8976.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm MSM8976 Network-On-Chip interconnect
> +
> +maintainers:
> +  - Konrad Dybcio 
> +
> +description: |

Do not need '|' unless you need to preserve formatting.

> +  The Qualcomm MSM8976 interconnect providers support adjusting the
> +  bandwidth requirements between the various NoC fabrics.
> +
> +properties:
> +  compatible:
> +enum:
> +  - qcom,msm8976-bimc
> +  - qcom,msm8976-pcnoc
> +  - qcom,msm8976-snoc
> +
> +  reg:
> +maxItems: 1
> +
> +  clock-names:
> +minItems: 1
> +maxItems: 2
> +
> +  clocks:
> +minItems: 1
> +maxItems: 2

I know that other qcom bindings use that order (clock-names -> clocks),
but at least for new bindings let's keep it more logical, so please
first clocks then clock-names.

> +
> +patternProperties:
> +  '^interconnect-[a-z0-9\-]+$':
> +type: object
> +$ref: qcom,rpm-common.yaml#
> +description:
> +  The interconnect providers do not have a separate QoS register space,
> +  but share parent's space.
> +
> +allOf:
> +  - $ref: qcom,rpm-common.yaml#

Drop both lines.

> +
> +properties:
> +  compatible:
> +const: qcom,msm8976-snoc-mm
> +
> +required:
> +  - compatible
> +
> +unevaluatedProperties: false

Put it after $ref (before description).

> +
> +required:
> +  - compatible
> +  - reg
> +
> +unevaluatedProperties: false

This goes after entire allOf section

> +
> +allOf:
> +  - $ref: qcom,rpm-common.yaml#
> +  - if:
> +  properties:
> +compatible:
> +  const: qcom,msm8976-snoc
> +
> +then:
> +  properties:
> +clocks:
> +  items:
> +- description: IPA clock from RPMCC
> +clock-names:
> +  const: ipa
> +
> +  required:
> +- clocks
> +- clock-names

Then why do you have two items? And what with clocks for other variants?
This is supposed to be fixed/specific for each device/variant.


> +
> +examples:
> +  - |
> +#include 
> +#include 
> +
> +bimc: interconnect@40 {
> +compatible = "qcom,msm8976-bimc";
> +reg = <0x0040 0x62000>;
> +#interconnect-cells = <2>;
> +};
> +
> +pcnoc: interconnect@50 {
> +compatible = "qcom,msm8976-pcnoc";
> +reg = <0x0050 0x14000>;
> +#interconnect-cells = <2>;
> +};

Drop both nodes. The node below is enough as an example.


Best regards,
Krzysztof

Re: [RFC bpf-next 01/10] uprobe: Add session callbacks to uprobe_consumer

[Jiri Olsa]

On Thu, Jun 06, 2024 at 09:52:39AM -0700, Andrii Nakryiko wrote:
> On Thu, Jun 6, 2024 at 9:46 AM Jiri Olsa  wrote:
> >
> > On Wed, Jun 05, 2024 at 10:50:11PM +0200, Jiri Olsa wrote:
> > > On Wed, Jun 05, 2024 at 07:56:19PM +0200, Oleg Nesterov wrote:
> > > > On 06/05, Andrii Nakryiko wrote:
> > > > >
> > > > > so any such
> > > > > limitations will cause problems, issue reports, investigation, etc.
> > > >
> > > > Agreed...
> > > >
> > > > > As one possible solution, what if we do
> > > > >
> > > > > struct return_instance {
> > > > > ...
> > > > > u64 session_cookies[];
> > > > > };
> > > > >
> > > > > and allocate sizeof(struct return_instance) + 8 *
> > > > >  and then at runtime pass
> > > > > &session_cookies[i] as data pointer to session-aware callbacks?
> > > >
> > > > I too thought about this, but I guess it is not that simple.
> > > >
> > > > Just for example. Suppose we have 2 session-consumers C1 and C2.
> > > > What if uprobe_unregister(C1) comes before the probed function
> > > > returns?
> > > >
> > > > We need something like map_cookie_to_consumer().
> > >
> > > I guess we could have hash table in return_instance that gets 'consumer 
> > > -> cookie' ?
> >
> > ok, hash table is probably too big for this.. I guess some solution that
> > would iterate consumers and cookies made sure it matches would be fine
> >
> 
> Yes, I was hoping to avoid hash tables for this, and in the common
> case have no added overhead.

hi,
here's first stab on that.. the change below:
  - extends current handlers with extra argument rather than adding new
set of handlers
  - store session consumers objects within return_instance object and
  - iterate these objects ^^^ in handle_uretprobe_chain

I guess it could be still polished, but I wonder if this could
be the right direction to do this.. thoughts? ;-)

thanks,
jirka


---
diff --git a/include/linux/uprobes.h b/include/linux/uprobes.h
index f46e0ca0169c..4e40e8352eac 100644
--- a/include/linux/uprobes.h
+++ b/include/linux/uprobes.h
@@ -34,15 +34,19 @@ enum uprobe_filter_ctx {
 };
 
 struct uprobe_consumer {
-   int (*handler)(struct uprobe_consumer *self, struct pt_regs *regs);
+   int (*handler)(struct uprobe_consumer *self, struct pt_regs *regs,
+   unsigned long *data);
int (*ret_handler)(struct uprobe_consumer *self,
unsigned long func,
-   struct pt_regs *regs);
+   struct pt_regs *regs,
+   unsigned long *data);
bool (*filter)(struct uprobe_consumer *self,
enum uprobe_filter_ctx ctx,
struct mm_struct *mm);
 
struct uprobe_consumer *next;
+   bool is_session;
+   unsigned int id;
 };
 
 #ifdef CONFIG_UPROBES
@@ -80,6 +84,12 @@ struct uprobe_task {
unsigned intdepth;
 };
 
+struct session_consumer {
+   long cookie;
+   unsigned int id;
+   int rc;
+};
+
 struct return_instance {
struct uprobe   *uprobe;
unsigned long   func;
@@ -88,6 +98,8 @@ struct return_instance {
boolchained;/* true, if instance is nested 
*/
 
struct return_instance  *next;  /* keep as stack */
+   int session_cnt;
+   struct session_consumer sc[1];  /* 1 for zero item marking the 
end */
 };
 
 enum rp_check {
diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index 2c83ba776fc7..cbd71dc06ef0 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -63,6 +63,8 @@ struct uprobe {
loff_t  ref_ctr_offset;
unsigned long   flags;
 
+   unsigned intsession_cnt;
+
/*
 * The generic code assumes that it has two members of unknown type
 * owned by the arch-specific code:
@@ -750,11 +752,30 @@ static struct uprobe *alloc_uprobe(struct inode *inode, 
loff_t offset,
return uprobe;
 }
 
+static void
+uprobe_consumer_account(struct uprobe *uprobe, struct uprobe_consumer *uc)
+{
+   static unsigned int session_id;
+
+   if (uc->is_session) {
+   uprobe->session_cnt++;
+   uc->id = ++session_id ?: ++session_id;
+   }
+}
+
+static void
+uprobe_consumer_unaccount(struct uprobe *uprobe, struct uprobe_consumer *uc)
+{
+   if (uc->is_session)
+   uprobe->session_cnt--;
+}
+
 static void consumer_add(struct uprobe *uprobe, struct uprobe_consumer *uc)
 {
down_write(&uprobe->consumer_rwsem);
uc->next = uprobe->consumers;
uprobe->consumers = uc;
+   uprobe_consumer_account(uprobe, uc);
up_write(&uprobe->consumer_rwsem);
 }
 
@@ -773,6 +794,7 @@ static bool consumer_del(struct uprobe *uprobe, struct 
uprobe_consumer *uc)
if (*con == uc) {
*con = uc->next;

Re: [PATCH] dts: imx8mq-librem5: Don't wake up on volume key press

[Martin Kepplinger]

Am Montag, dem 20.05.2024 um 12:57 +0200 schrieb Guido Günther:
> The only key that should wake up the phone is power button press.
> This
> prevents accidental wakeup due to e.g. pressing the buttons in the
> pocket or backpack and is in line what userspace uses to unblank the
> device.
> 
> Signed-off-by: Guido Günther 

Reviewed-by: Martin Kepplinger 

thank you,

  martin

> ---
>  arch/arm64/boot/dts/freescale/imx8mq-librem5.dtsi | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/freescale/imx8mq-librem5.dtsi
> b/arch/arm64/boot/dts/freescale/imx8mq-librem5.dtsi
> index ffb5fe61630d..1b39514d5c12 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mq-librem5.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mq-librem5.dtsi
> @@ -45,7 +45,6 @@ key-vol-down {
> gpios = <&gpio1 17 GPIO_ACTIVE_LOW>;
> linux,code = ;
> debounce-interval = <50>;
> -   wakeup-source;
> };
>  
> key-vol-up {
> @@ -53,7 +52,6 @@ key-vol-up {
> gpios = <&gpio1 16 GPIO_ACTIVE_LOW>;
> linux,code = ;
> debounce-interval = <50>;
> -   wakeup-source;
> };
> };
>  

Re: [PATCH 6/7] interconnect: qcom: qcs404: Add regmaps and more bus descriptions

[kernel test robot]

Hi Adam,

kernel test robot noticed the following build errors:

[auto build test ERROR on robh/for-next]
[also build test ERROR on linus/master v6.10-rc3 next-20240607]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:
https://github.com/intel-lab-lkp/linux/commits/Adam-Skladowski/dt-bindings-interconnect-Add-Qualcomm-MSM8976-DT-bindings/20240610-022416
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
patch link:
https://lore.kernel.org/r/20240609182112.13032-7-a39.skl%40gmail.com
patch subject: [PATCH 6/7] interconnect: qcom: qcs404: Add regmaps and more bus 
descriptions
config: arm64-defconfig 
(https://download.01.org/0day-ci/archive/20240610/202406101715.amp9vwkx-...@intel.com/config)
compiler: aarch64-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): 
(https://download.01.org/0day-ci/archive/20240610/202406101715.amp9vwkx-...@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot 
| Closes: 
https://lore.kernel.org/oe-kbuild-all/202406101715.amp9vwkx-...@intel.com/

All error/warnings (new ones prefixed by >>):

>> drivers/interconnect/qcom/qcs404.c:1070:21: error: variable 
>> 'qcs404_bimc_regmap_config' has initializer but incomplete type
1070 | static const struct regmap_config qcs404_bimc_regmap_config = {
 | ^
>> drivers/interconnect/qcom/qcs404.c:1071:10: error: 'const struct 
>> regmap_config' has no member named 'reg_bits'
1071 | .reg_bits = 32,
 |  ^~~~
>> drivers/interconnect/qcom/qcs404.c:1071:21: warning: excess elements in 
>> struct initializer
1071 | .reg_bits = 32,
 | ^~
   drivers/interconnect/qcom/qcs404.c:1071:21: note: (near initialization for 
'qcs404_bimc_regmap_config')
>> drivers/interconnect/qcom/qcs404.c:1072:10: error: 'const struct 
>> regmap_config' has no member named 'reg_stride'
1072 | .reg_stride = 4,
 |  ^~
   drivers/interconnect/qcom/qcs404.c:1072:23: warning: excess elements in 
struct initializer
1072 | .reg_stride = 4,
 |   ^
   drivers/interconnect/qcom/qcs404.c:1072:23: note: (near initialization for 
'qcs404_bimc_regmap_config')
>> drivers/interconnect/qcom/qcs404.c:1073:10: error: 'const struct 
>> regmap_config' has no member named 'val_bits'
1073 | .val_bits = 32,
 |  ^~~~
   drivers/interconnect/qcom/qcs404.c:1073:21: warning: excess elements in 
struct initializer
1073 | .val_bits = 32,
 | ^~
   drivers/interconnect/qcom/qcs404.c:1073:21: note: (near initialization for 
'qcs404_bimc_regmap_config')
>> drivers/interconnect/qcom/qcs404.c:1074:10: error: 'const struct 
>> regmap_config' has no member named 'max_register'
1074 | .max_register = 0x8,
 |  ^~~~
   drivers/interconnect/qcom/qcs404.c:1074:25: warning: excess elements in 
struct initializer
1074 | .max_register = 0x8,
 | ^~~
   drivers/interconnect/qcom/qcs404.c:1074:25: note: (near initialization for 
'qcs404_bimc_regmap_config')
>> drivers/interconnect/qcom/qcs404.c:1075:10: error: 'const struct 
>> regmap_config' has no member named 'fast_io'
1075 | .fast_io = true,
 |  ^~~
   drivers/interconnect/qcom/qcs404.c:1075:20: warning: excess elements in 
struct initializer
1075 | .fast_io = true,
 |^~~~
   drivers/interconnect/qcom/qcs404.c:1075:20: note: (near initialization for 
'qcs404_bimc_regmap_config')
>> drivers/interconnect/qcom/qcs404.c:1137:21: error: variable 
>> 'qcs404_pcnoc_regmap_config' has initializer but incomplete type
1137 | static const struct regmap_config qcs404_pcnoc_regmap_config = {
 | ^
   drivers/interconnect/qcom/qcs404.c:1138:10: error: 'const struct 
regmap_config' has no member named 'reg_bits'
1138 | .reg_bits = 32,
 |  ^~~~
   drivers/interconnect/qcom/qcs404.c:1138:21: warning: excess elements in 
struct initializer
1138 | .reg_bits = 32,
 | ^~
   drivers/interconnect/qcom/qcs404.c:1138:21: note: (near initialization for 
'qcs404_pcnoc_regmap_config')
 

[PATCH] nvdimm: make nd_class constant

[Greg Kroah-Hartman]

Now that the driver core allows for struct class to be in read-only
memory, we should make all 'class' structures declared at build time
placing them into read-only memory, instead of having to be dynamically
allocated at runtime.

Cc: Dan Williams 
Cc: Vishal Verma 
Cc: Dave Jiang 
Cc: Ira Weiny 
Cc: nvd...@lists.linux.dev
Signed-off-by: Greg Kroah-Hartman 
---
 drivers/nvdimm/bus.c | 19 ++-
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/nvdimm/bus.c b/drivers/nvdimm/bus.c
index 508aed017ddc..101c425f3e8b 100644
--- a/drivers/nvdimm/bus.c
+++ b/drivers/nvdimm/bus.c
@@ -25,9 +25,12 @@
 
 int nvdimm_major;
 static int nvdimm_bus_major;
-static struct class *nd_class;
 static DEFINE_IDA(nd_ida);
 
+static const struct class nd_class = {
+   .name = "nd",
+};
+
 static int to_nd_device_type(const struct device *dev)
 {
if (is_nvdimm(dev))
@@ -742,7 +745,7 @@ int nvdimm_bus_create_ndctl(struct nvdimm_bus *nvdimm_bus)
device_initialize(dev);
lockdep_set_class(&dev->mutex, &nvdimm_ndctl_key);
device_set_pm_not_required(dev);
-   dev->class = nd_class;
+   dev->class = &nd_class;
dev->parent = &nvdimm_bus->dev;
dev->devt = devt;
dev->release = ndctl_release;
@@ -765,7 +768,7 @@ int nvdimm_bus_create_ndctl(struct nvdimm_bus *nvdimm_bus)
 
 void nvdimm_bus_destroy_ndctl(struct nvdimm_bus *nvdimm_bus)
 {
-   device_destroy(nd_class, MKDEV(nvdimm_bus_major, nvdimm_bus->id));
+   device_destroy(&nd_class, MKDEV(nvdimm_bus_major, nvdimm_bus->id));
 }
 
 static const struct nd_cmd_desc __nd_cmd_dimm_descs[] = {
@@ -1320,11 +1323,9 @@ int __init nvdimm_bus_init(void)
goto err_dimm_chrdev;
nvdimm_major = rc;
 
-   nd_class = class_create("nd");
-   if (IS_ERR(nd_class)) {
-   rc = PTR_ERR(nd_class);
+   rc = class_register(&nd_class);
+   if (rc)
goto err_class;
-   }
 
rc = driver_register(&nd_bus_driver.drv);
if (rc)
@@ -1333,7 +1334,7 @@ int __init nvdimm_bus_init(void)
return 0;
 
  err_nd_bus:
-   class_destroy(nd_class);
+   class_unregister(&nd_class);
  err_class:
unregister_chrdev(nvdimm_major, "dimmctl");
  err_dimm_chrdev:
@@ -1347,7 +1348,7 @@ int __init nvdimm_bus_init(void)
 void nvdimm_bus_exit(void)
 {
driver_unregister(&nd_bus_driver.drv);
-   class_destroy(nd_class);
+   class_unregister(&nd_class);
unregister_chrdev(nvdimm_bus_major, "ndctl");
unregister_chrdev(nvdimm_major, "dimmctl");
bus_unregister(&nvdimm_bus_type);
-- 
2.45.2

Re: [PATCH v2 1/1] dt-bindings: remoteproc: imx_rproc: add minItems for power-domain

[Krzysztof Kozlowski]

On 07/06/2024 17:19, Frank Li wrote:
> On Fri, Jun 07, 2024 at 09:32:26AM +0200, Krzysztof Kozlowski wrote:
>> On 06/06/2024 17:00, Frank Li wrote:
>>> "fsl,imx8qxp-cm4" and "fsl,imx8qm-cm4" need minimum 2 power domains. Keep
>>> the same restriction for other compatible string.
>>>
>>> Signed-off-by: Frank Li 
>>> ---
>>>
>>> Notes:
>>> Change from v1 to v2
>>> - set minitem to 2 at top
>>> - Add imx8qm compatible string also
>>> - use not logic to handle difference compatible string restriction
>>> - update commit message.
>>> 
>>> pass dt_binding_check.
>>> 
>>> make ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- -j8  dt_binding_check 
>>> DT_SCHEMA_FILES=fsl,imx-rproc.yaml
>>>   SCHEMA  Documentation/devicetree/bindings/processed-schema.json
>>>   CHKDT   Documentation/devicetree/bindings
>>>   LINTDocumentation/devicetree/bindings
>>>   DTEX
>>> Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.example.dts
>>>   DTC_CHK 
>>> Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.example.dtb
>>>
>>>  .../bindings/remoteproc/fsl,imx-rproc.yaml | 14 ++
>>>  1 file changed, 14 insertions(+)
>>>
>>> diff --git 
>>> a/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml 
>>> b/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml
>>> index df36e29d974ca..da108a39df435 100644
>>> --- a/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml
>>> +++ b/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml
>>> @@ -59,6 +59,7 @@ properties:
>>>  maxItems: 32
>>>  
>>>power-domains:
>>> +minItems: 2
>>>  maxItems: 8
>>>  
>>>fsl,auto-boot:
>>> @@ -99,6 +100,19 @@ allOf:
>>>properties:
>>>  fsl,iomuxc-gpr: false
>>>  
>>> +  - if:
>>> +  properties:
>>> +compatible:
>>> +  not:
>>> +contains:
>>> +  enum:
>>> +- fsl,imx8qxp-cm4
>>> +- fsl,imx8qm-cm4
>>> +then:
>>> +  properties:
>>> +power-domains:
>>> +  minItems: 8
>>
>> What happened with the "else:"? How many power domains is needed for
>> other devices?
> 
> So far, only fsl,imx8qxp-cm4 ind fsl,imx8qm-cm4 need power domain (2-8). 
> Power-domains is option property. 
> 
> Can I just remove whole "if"?

No, I rather expect else.

Best regards,
Krzysztof