linux-kernel - Re: [PATCH v3 3/4] misc: fastrpc: Add support for new DSP IOVA formatting

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <yucfjksc5w2eac6fmsamknjhbwebxg7xsz6633vs2rw3qt4kfa@w2wsbfvszhb7>
Date: Tue, 18 Nov 2025 09:52:49 -0600
From: Bjorn Andersson <andersson@...nel.org>
To: Kumari Pallavi <kumari.pallavi@....qualcomm.com>
Cc: kpallavi@....qualcomm.com, srini@...nel.org, amahesh@....qualcomm.com, 
	arnd@...db.de, gregkh@...uxfoundation.org, quic_bkumar@...cinc.com, 
	ekansh.gupta@....qualcomm.com, linux-kernel@...r.kernel.org, quic_chennak@...cinc.com, 
	dri-devel@...ts.freedesktop.org, linux-arm-msm@...r.kernel.org, jingyi.wang@....qualcomm.com, 
	aiqun.yu@....qualcomm.com, ktadakam@....qualcomm.com
Subject: Re: [PATCH v3 3/4] misc: fastrpc: Add support for new DSP IOVA
 formatting

On Mon, Nov 17, 2025 at 12:32:59PM +0530, Kumari Pallavi wrote:
> 
> 
> On 11/14/2025 9:21 PM, Bjorn Andersson wrote:
> > On Fri, Nov 14, 2025 at 02:11:41PM +0530, Kumari Pallavi wrote:
> > > Implement the new IOVA formatting required by the DSP architecture change
> > > on Kaanapali SoC. Place the SID for DSP DMA transactions at bit 56 in the
> > > physical address. This placement is necessary for the DSPs to correctly
> > > identify streams and operate as intended.
> > > To address this, set SID position to bit 56 via OF matching on the fastrpc
> > > node; otherwise, default to legacy 32-bit placement.
> > > This change ensures consistent SID placement across DSPs.
> > > 
> > 
> > In patch 2 I said I think it would be a good idea to separate the two
> > perspectives (Linux/SMMU vs remote addresses).
> > 
> > Looking ta this patch I'm completely convinced that it's the right thing
> > to do!
> > 
> > > Signed-off-by: Kumari Pallavi <kumari.pallavi@....qualcomm.com>
> > > ---
> > >   drivers/misc/fastrpc.c | 46 +++++++++++++++++++++++++++++++++++-------
> > >   1 file changed, 39 insertions(+), 7 deletions(-)
> > > 
> > > diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
> > > index d6a7960fe716..bcf3c7f8d3e9 100644
> > > --- a/drivers/misc/fastrpc.c
> > > +++ b/drivers/misc/fastrpc.c
> > > @@ -33,7 +33,6 @@
> > >   #define FASTRPC_ALIGN		128
> > >   #define FASTRPC_MAX_FDLIST	16
> > >   #define FASTRPC_MAX_CRCLIST	64
> > > -#define FASTRPC_PHYS(p)	((p) & 0xffffffff)
> > >   #define FASTRPC_CTX_MAX (256)
> > >   #define FASTRPC_INIT_HANDLE	1
> > >   #define FASTRPC_DSP_UTILITIES_HANDLE	2
> > > @@ -105,6 +104,15 @@
> > >   #define miscdev_to_fdevice(d) container_of(d, struct fastrpc_device, miscdev)
> > > +/* Extract smmu pa from consolidated iova */
> > > +#define IPA_TO_DMA_ADDR(iova, sid_pos) (iova & ((1ULL << sid_pos) - 1ULL))
> > > +/*
> > > + * Prepare the consolidated iova to send to dsp by prepending the sid
> > > + * to smmu pa at the appropriate position
> > > + */
> > > +#define IOVA_FROM_SID_PA(sid, phys, sid_pos) \
> > > +       (phys += sid << sid_pos)
> > 
> > This is a horrible macro. It looks just like a function taking values,
> > it's named to sound like it takes a sid and pa and return an iova, but
> > it has side effects.
> > 
> > And what's up with the ordering? Take argument 1 and 3, and put the
> > result in argument 2?!
> > 
> 
> Thank you for the feedback regarding the macro implementation. I understand
> your concern about readability and hidden side effects.
> To address this, I’ve replaced the macro with an inline function
> 
> 
> static inline u64 fastrpc_compute_sid_offset(u64 sid, u32 sid_pos)
> {
>     return sid << sid_pos;
> }
> 
> 
> buf->dma_addr += fastrpc_compute_sid_offset(sid, sid_pos);
> 
> Could you confirm if this is in line with what you suggested?
> 

That is possible to read, so yes that would be much better. 

Regards,
Bjorn