On Thu, Jul 16, 2026 at 8:59 PM Charles Perry <[email protected]> wrote: > > On Thu, Jul 16, 2026 at 07:39:52AM +0530, Rahul Pathak wrote: > > On Tue, Jul 14, 2026 at 11:04 PM Charles Perry > > <[email protected]> wrote: > > > > > > On Thu, Jul 09, 2026 at 11:26:02PM +0530, Rahul Pathak wrote: > > > > SBI MPXY interface implements the SBI MPXY > > > > extension to interact with supported message > > > > protocols for platform management and control > > > > > > > > Signed-off-by: Rahul Pathak <[email protected]> > > > > --- > > > > arch/riscv/Kconfig | 9 + > > > > arch/riscv/include/asm/sbi.h | 268 ++++++++++++++++++ > > > > arch/riscv/lib/Makefile | 1 + > > > > arch/riscv/lib/sbi_mpxy.c | 529 +++++++++++++++++++++++++++++++++++ > > > > 4 files changed, 807 insertions(+) > > > > create mode 100644 arch/riscv/lib/sbi_mpxy.c > > > > > > > > > > ... > > > > > > > diff --git a/arch/riscv/lib/sbi_mpxy.c b/arch/riscv/lib/sbi_mpxy.c > > > > new file mode 100644 > > > > index 00000000000..f95cedaabac > > > > --- /dev/null > > > > +++ b/arch/riscv/lib/sbi_mpxy.c > > > > @@ -0,0 +1,529 @@ > > > > +// SPDX-License-Identifier: GPL-2.0+ > > > > +/* > > > > + * SBI MPXY (Message Proxy) Generic Library > > > > + * > > > > + * Copyright (c) 2026, Rahul Pathak <[email protected]> > > > > + */ > > > > > > ... > > > > > > > + > > > > +/** > > > > + * sbi_mpxy_read_attrs() - Read a contiguous range of channel > > > > attributes > > > > + * > > > > + * @chan: Target channel > > > > + * @base_attr_id: First attribute ID in the range > > > > + * @attr_count: Number of attributes to read > > > > + * @attrs_buf: Output buffer (attr_count u32 words) > > > > + */ > > > > +int sbi_mpxy_read_attrs(struct mpxy_channel *chan, u32 base_attr_id, > > > > + u32 attr_count, u32 *attrs_buf) > > > > +{ > > > > + struct sbiret ret; > > > > + __le32 *shmem; > > > > + u32 end_id; > > > > + int i; > > > > + > > > > + if (!chan || !attr_count || !attrs_buf) > > > > + return -EINVAL; > > > > + > > > > + if (attr_count > mpxy_ctx.shmem_size / sizeof(u32)) > > > > + return -EINVAL; > > > > + > > > > + shmem = (__le32 *)shmem_base(); > > > > + if (!shmem) > > > > + return -ENXIO; > > > > + > > > > + end_id = base_attr_id + attr_count - 1; > > > > + > > > > + if (base_attr_id < SBI_MPXY_ATTR_STD_ATTR_MAX_IDX) { > > > > + /* Standard attributes */ > > > > + if (end_id >= SBI_MPXY_ATTR_STD_ATTR_MAX_IDX) > > > > + return -EINVAL; > > > > + > > > > + ret = sbi_ecall(SBI_EXT_MPXY, SBI_EXT_MPXY_READ_ATTRS, > > > > + chan->channel_id, base_attr_id, > > > > attr_count, > > > > + 0, 0, 0); > > > > + if (ret.error) > > > > + return -EINVAL; > > > > + > > > > + for (i = 0; i < attr_count; i++) > > > > + attrs_buf[i] = le32_to_cpu(shmem[i]); > > > > + > > > > + return 0; > > > > + } > > > > + > > > > + if (base_attr_id >= SBI_MPXY_ATTR_MSGPROTO_ATTR_START && > > > > + end_id < SBI_MPXY_ATTR_MSGPROTO_ATTR_END) { > > > > + /* Protocol-specific attributes */ > > > > + if (!chan->proto || !chan->proto->readattr) > > > > + return -ENOSYS; > > > > + > > > > + ret = sbi_ecall(SBI_EXT_MPXY, SBI_EXT_MPXY_READ_ATTRS, > > > > + chan->channel_id, base_attr_id, > > > > attr_count, > > > > + 0, 0, 0); > > > > + if (ret.error) > > > > + return -EINVAL; > > > > + > > > > + for (i = 0; i < attr_count; i++) > > > > + attrs_buf[i] = le32_to_cpu(shmem[i]); > > > > > > Shouldn't you call the ->readattr() callback here instead of manually > > > doing > > > the read? > > > > > > Also, I'm not quite sure I understand what purpose those callbacks serve. > > > From what I'm seeing, attributes are always read/write using > > > SBI_EXT_MPXY_[READ|WRITE]_ATTRS because the transport layer is always > > > mpxy. > > > Would a non-RPMI protocol do something different? > > > > Reading the attributes is simple and can be achieved with or without > > the callback but > > writing the attributes may (or may not) require any further protocol > > specific validation > > which is not assumed here and hence used the callback to keep things > > generic. > > > > That's why I didn't use callback for read but for write. I am fine to > > use callback for attributes read but I would prefer to not assume > > anything and keep write attribute callback. > > The only hard requirement that I have is that we shouldn't have dead code > and the ->readattr() callback is dead code in this version. I don't have > strong opinions on whether it needs to be called or removed.
If that's really bugging you then I suggest removing the RPMI implementation of readattr only and not the callback pointer field from the struct mpxy_proto_ops. > > Thanks, > Charles >
