Re: [PATCH v12 19/28] riscv/ptrace: riscv cfi status and state via ptrace and in core files
2025-03-20T16:09:12-07:00, Deepak Gupta :
> On Thu, Mar 20, 2025 at 3:24 PM Radim Krčmář wrote:
>> 2025-03-14T14:39:38-07:00, Deepak Gupta :
>> > Expose a new register type NT_RISCV_USER_CFI for risc-v cfi status and
>> > state. Intentionally both landing pad and shadow stack status and state
>> > are rolled into cfi state. Creating two different NT_RISCV_USER_XXX would
>> > not be useful and wastage of a note type. Enabling or disabling of feature
>> > is not allowed via ptrace set interface. However setting `elp` state or
>> > setting shadow stack pointer are allowed via ptrace set interface. It is
>> > expected `gdb` might have use to fixup `elp` state or `shadow stack`
>> > pointer.
>> >
>> > Signed-off-by: Deepak Gupta
>> > ---
>> > diff --git a/arch/riscv/include/uapi/asm/ptrace.h
>> > b/arch/riscv/include/uapi/asm/ptrace.h
>> > index 659ea3af5680..e6571fba8a8a 100644
>> > @@ -131,6 +131,24 @@ struct __sc_riscv_cfi_state {
>> > unsigned long ss_ptr; /* shadow stack pointer */
>> > };
>> >
>> > +struct __cfi_status {
>> > + /* indirect branch tracking state */
>> > + __u64 lp_en : 1;
>> > + __u64 lp_lock : 1;
>> > + __u64 elp_state : 1;
>> > +
>> > + /* shadow stack status */
>> > + __u64 shstk_en : 1;
>> > + __u64 shstk_lock : 1;
>>
>> I remember there was deep hatred towards bitfields in the Linux
>> community, have things changes?
>
> hmm. I didn't know about the strong hatred.
There is a good reason for it. :)
The C standard left important behavior as implementation-specific (by
mistake, I hope). I do like bitfields, but you have to be extra careful
when working with them.
> Although I can see lots of examples of this pattern in existing kernel code.
> No strong feelings on my side, I can change this and have it single 64bit
> field
> and accessed via bitmasks.
This is uapi and bitfields do not specify the internal representation.
A program compiled at a different time can see completely different
order of the bitfields, so the uapi would break.
We cannot use bitfields here.
Re: [PATCH v12 23/28] riscv: kernel command line option to opt out of user cfi
2025-03-20T15:31:09-07:00, Deepak Gupta :
> On Thu, Mar 20, 2025 at 2:35 PM Radim Krčmář wrote:
>> 2025-03-14T14:39:42-07:00, Deepak Gupta :
>> > +__setup("disable_riscv_usercfi=", setup_global_riscv_enable);
>>
>> I'd prefer two command line options instead.
>
> One for zicfilp and one for zicfiss ?
Yes. I don't want to suggest the naming, because I would have used
"nousercfi" without an argument for the current one.
Re: [PATCH net-next v7 4/9] net: devmem: Implement TX path
On Tue, Mar 18, 2025 at 1:53 AM Paolo Abeni wrote:
>
> Adding Kuniyuki.
>
> On 3/8/25 10:40 PM, Mina Almasry wrote:
> > @@ -931,10 +932,67 @@ int netdev_nl_bind_rx_doit(struct sk_buff *skb,
> > struct genl_info *info)
> > return err;
> > }
> >
> > -/* stub */
> > int netdev_nl_bind_tx_doit(struct sk_buff *skb, struct genl_info *info)
> > {
> > - return 0;
> > + struct net_devmem_dmabuf_binding *binding;
> > + struct list_head *sock_binding_list;
> > + struct net_device *netdev;
> > + u32 ifindex, dmabuf_fd;
> > + struct sk_buff *rsp;
> > + int err = 0;
> > + void *hdr;
> > +
> > + if (GENL_REQ_ATTR_CHECK(info, NETDEV_A_DEV_IFINDEX) ||
> > + GENL_REQ_ATTR_CHECK(info, NETDEV_A_DMABUF_FD))
> > + return -EINVAL;
> > +
> > + ifindex = nla_get_u32(info->attrs[NETDEV_A_DEV_IFINDEX]);
> > + dmabuf_fd = nla_get_u32(info->attrs[NETDEV_A_DMABUF_FD]);
> > +
> > + sock_binding_list = genl_sk_priv_get(&netdev_nl_family,
> > + NETLINK_CB(skb).sk);
> > + if (IS_ERR(sock_binding_list))
> > + return PTR_ERR(sock_binding_list);
> > +
> > + rsp = genlmsg_new(GENLMSG_DEFAULT_SIZE, GFP_KERNEL);
> > + if (!rsp)
> > + return -ENOMEM;
> > +
> > + hdr = genlmsg_iput(rsp, info);
> > + if (!hdr) {
> > + err = -EMSGSIZE;
> > + goto err_genlmsg_free;
> > + }
> > +
> > + rtnl_lock();
>
> The above could possibly be a rtnl_net_lock(), right?
>
> (not strictily related to this series) The same for the existing
> rtnl_lock() call in netdev-genl.c, right?
>
Actually I think this can follow the example set in commit
1d22d3060b9b ("net: drop rtnl_lock for queue_mgmt operations") and
take the netdev_get_by_index_lock().
--
Thanks,
Mina
Re: [PATCH net-next v7 3/9] net: devmem: TCP tx netlink api
On Tue, Mar 18, 2025 at 1:39 AM Paolo Abeni wrote:
>
> On 3/8/25 10:40 PM, Mina Almasry wrote:
> > From: Stanislav Fomichev
> >
> > Add bind-tx netlink call to attach dmabuf for TX; queue is not
> > required, only ifindex and dmabuf fd for attachment.
> >
> > Signed-off-by: Stanislav Fomichev
> > Signed-off-by: Mina Almasry
> >
> > ---
> >
> > v3:
> > - Fix ynl-regen.sh error (Simon).
> >
> > ---
> > Documentation/netlink/specs/netdev.yaml | 12
> > include/uapi/linux/netdev.h | 1 +
> > net/core/netdev-genl-gen.c | 13 +
> > net/core/netdev-genl-gen.h | 1 +
> > net/core/netdev-genl.c | 6 ++
> > tools/include/uapi/linux/netdev.h | 1 +
> > 6 files changed, 34 insertions(+)
> >
> > diff --git a/Documentation/netlink/specs/netdev.yaml
> > b/Documentation/netlink/specs/netdev.yaml
> > index 36f1152bfac3..e560b05eb528 100644
> > --- a/Documentation/netlink/specs/netdev.yaml
> > +++ b/Documentation/netlink/specs/netdev.yaml
> > @@ -743,6 +743,18 @@ operations:
> > - defer-hard-irqs
> > - gro-flush-timeout
> > - irq-suspend-timeout
> > +-
> > + name: bind-tx
> > + doc: Bind dmabuf to netdev for TX
> > + attribute-set: dmabuf
> > + do:
> > +request:
> > + attributes:
> > +- ifindex
> > +- fd
> > +reply:
> > + attributes:
> > +- id
> >
> > kernel-family:
> >headers: [ "linux/list.h"]
> > diff --git a/include/uapi/linux/netdev.h b/include/uapi/linux/netdev.h
> > index 7600bf62dbdf..7eb9571786b8 100644
> > --- a/include/uapi/linux/netdev.h
> > +++ b/include/uapi/linux/netdev.h
> > @@ -219,6 +219,7 @@ enum {
> > NETDEV_CMD_QSTATS_GET,
> > NETDEV_CMD_BIND_RX,
> > NETDEV_CMD_NAPI_SET,
> > + NETDEV_CMD_BIND_TX,
> >
> > __NETDEV_CMD_MAX,
> > NETDEV_CMD_MAX = (__NETDEV_CMD_MAX - 1)
> > diff --git a/net/core/netdev-genl-gen.c b/net/core/netdev-genl-gen.c
> > index 996ac6a449eb..f27608d6301c 100644
> > --- a/net/core/netdev-genl-gen.c
> > +++ b/net/core/netdev-genl-gen.c
> > @@ -99,6 +99,12 @@ static const struct nla_policy
> > netdev_napi_set_nl_policy[NETDEV_A_NAPI_IRQ_SUSPE
> > [NETDEV_A_NAPI_IRQ_SUSPEND_TIMEOUT] = { .type = NLA_UINT, },
> > };
> >
> > +/* NETDEV_CMD_BIND_TX - do */
> > +static const struct nla_policy netdev_bind_tx_nl_policy[NETDEV_A_DMABUF_FD
> > + 1] = {
> > + [NETDEV_A_DMABUF_IFINDEX] = NLA_POLICY_MIN(NLA_U32, 1),
> > + [NETDEV_A_DMABUF_FD] = { .type = NLA_U32, },
> > +};
> > +
> > /* Ops table for netdev */
> > static const struct genl_split_ops netdev_nl_ops[] = {
> > {
> > @@ -190,6 +196,13 @@ static const struct genl_split_ops netdev_nl_ops[] = {
> > .maxattr= NETDEV_A_NAPI_IRQ_SUSPEND_TIMEOUT,
> > .flags = GENL_ADMIN_PERM | GENL_CMD_CAP_DO,
> > },
> > + {
> > + .cmd= NETDEV_CMD_BIND_TX,
> > + .doit = netdev_nl_bind_tx_doit,
> > + .policy = netdev_bind_tx_nl_policy,
> > + .maxattr= NETDEV_A_DMABUF_FD,
> > + .flags = GENL_CMD_CAP_DO,
> > + },
> > };
> >
> > static const struct genl_multicast_group netdev_nl_mcgrps[] = {
> > diff --git a/net/core/netdev-genl-gen.h b/net/core/netdev-genl-gen.h
> > index e09dd7539ff2..c1fed66e92b9 100644
> > --- a/net/core/netdev-genl-gen.h
> > +++ b/net/core/netdev-genl-gen.h
> > @@ -34,6 +34,7 @@ int netdev_nl_qstats_get_dumpit(struct sk_buff *skb,
> > struct netlink_callback *cb);
> > int netdev_nl_bind_rx_doit(struct sk_buff *skb, struct genl_info *info);
> > int netdev_nl_napi_set_doit(struct sk_buff *skb, struct genl_info *info);
> > +int netdev_nl_bind_tx_doit(struct sk_buff *skb, struct genl_info *info);
> >
> > enum {
> > NETDEV_NLGRP_MGMT,
> > diff --git a/net/core/netdev-genl.c b/net/core/netdev-genl.c
> > index 2b774183d31c..6e5f2de4d947 100644
> > --- a/net/core/netdev-genl.c
> > +++ b/net/core/netdev-genl.c
> > @@ -931,6 +931,12 @@ int netdev_nl_bind_rx_doit(struct sk_buff *skb, struct
> > genl_info *info)
> > return err;
> > }
> >
> > +/* stub */
> > +int netdev_nl_bind_tx_doit(struct sk_buff *skb, struct genl_info *info)
> > +{
> > + return 0;
> > +}
> > +
> > void netdev_nl_sock_priv_init(struct list_head *priv)
> > {
> > INIT_LIST_HEAD(priv);
>
> I'm sorry, but this chunck does not apply cleanly anymore.
>
> Please rebase.
>
> Disclaimer: unfortunately I must note that due to the long backlog
> pending and the upcoming merge window, I'm unsure I'll be able to
> process the next revision before the net-next closing.
>
Thanks Paolo. I have also been under the weather the last couple of
days so I haven't even been able to repost. I guess I'll repost after
the merge window if netdev closes today.
--
Thanks,
Mina
Re: [PATCH net-next 0/6] netconsole: Add support for userdata release
Hello: This series was applied to netdev/net-next.git (main) by Paolo Abeni : On Fri, 14 Mar 2025 10:58:44 -0700 you wrote: > I am submitting a series of patches that introduce a new feature for the > netconsole subsystem, specifically the addition of the 'release' field > to the sysdata structure. This feature allows the kernel release/version > to be appended to the userdata dictionary in every message sent, > enhancing the information available for debugging and monitoring > purposes. > > [...] Here is the summary with links: - [net-next,1/6] netconsole: introduce 'release' as a new sysdata field https://git.kernel.org/netdev/net-next/c/42211e310781 - [net-next,2/6] netconsole: implement configfs for release_enabled https://git.kernel.org/netdev/net-next/c/343f90227070 - [net-next,3/6] netconsole: add 'sysdata' suffix to related functions https://git.kernel.org/netdev/net-next/c/b92c6fc43f4e - [net-next,4/6] netconsole: append release to sysdata https://git.kernel.org/netdev/net-next/c/cfcc9239e78a - [net-next,5/6] selftests: netconsole: Add tests for 'release' feature in sysdata https://git.kernel.org/netdev/net-next/c/4b73dc83ed96 - [net-next,6/6] docs: netconsole: document release feature https://git.kernel.org/netdev/net-next/c/56ad890de2cd You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html
