Update all references of buf->phys and map->phys to buf->dma_addr and
map->dma_addr to accurately represent that these fields store DMA
addresses, not physical addresses. This change improves code clarity
and aligns with kernel conventions for dma_addr_t usage.
Signed-off-by: Kumari Pallavi <kumari.pallavi@oss.qualcomm.com>
---
drivers/misc/fastrpc.c | 76 ++++++++++++++++++++++--------------------
1 file changed, 40 insertions(+), 36 deletions(-)
diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
index ee652ef01534..d6a7960fe716 100644
--- a/drivers/misc/fastrpc.c
+++ b/drivers/misc/fastrpc.c
@@ -106,7 +106,7 @@
#define miscdev_to_fdevice(d) container_of(d, struct fastrpc_device, miscdev)
struct fastrpc_phy_page {
- u64 addr; /* physical address */
+ u64 addr; /* physical or dma address */
u64 size; /* size of contiguous region */
};
@@ -171,7 +171,7 @@ struct fastrpc_msg {
u64 ctx; /* invoke caller context */
u32 handle; /* handle to invoke */
u32 sc; /* scalars structure describing the data */
- u64 addr; /* physical address */
+ u64 addr; /* physical or dma address */
u64 size; /* size of contiguous region */
};
@@ -194,7 +194,7 @@ struct fastrpc_buf {
struct dma_buf *dmabuf;
struct device *dev;
void *virt;
- u64 phys;
+ u64 dma_addr;
u64 size;
/* Lock for dma buf attachments */
struct mutex lock;
@@ -217,7 +217,7 @@ struct fastrpc_map {
struct dma_buf *buf;
struct sg_table *table;
struct dma_buf_attachment *attach;
- u64 phys;
+ u64 dma_addr;
u64 size;
void *va;
u64 len;
@@ -320,11 +320,12 @@ static void fastrpc_free_map(struct kref *ref)
perm.vmid = QCOM_SCM_VMID_HLOS;
perm.perm = QCOM_SCM_PERM_RWX;
- err = qcom_scm_assign_mem(map->phys, map->len,
+ err = qcom_scm_assign_mem(map->dma_addr, map->len,
&src_perms, &perm, 1);
if (err) {
- dev_err(map->fl->sctx->dev, "Failed to assign memory phys 0x%llx size 0x%llx err %d\n",
- map->phys, map->len, err);
+ dev_err(map->fl->sctx->dev,
+ "Failed to assign memory dma_addr 0x%llx size 0x%llx err %d\n",
+ map->dma_addr, map->len, err);
return;
}
}
@@ -389,7 +390,7 @@ static int fastrpc_map_lookup(struct fastrpc_user *fl, int fd,
static void fastrpc_buf_free(struct fastrpc_buf *buf)
{
dma_free_coherent(buf->dev, buf->size, buf->virt,
- FASTRPC_PHYS(buf->phys));
+ FASTRPC_PHYS(buf->dma_addr));
kfree(buf);
}
@@ -408,12 +409,12 @@ static int __fastrpc_buf_alloc(struct fastrpc_user *fl, struct device *dev,
buf->fl = fl;
buf->virt = NULL;
- buf->phys = 0;
+ buf->dma_addr = 0;
buf->size = size;
buf->dev = dev;
buf->raddr = 0;
- buf->virt = dma_alloc_coherent(dev, buf->size, (dma_addr_t *)&buf->phys,
+ buf->virt = dma_alloc_coherent(dev, buf->size, (dma_addr_t *)&buf->dma_addr,
GFP_KERNEL);
if (!buf->virt) {
mutex_destroy(&buf->lock);
@@ -439,7 +440,7 @@ static int fastrpc_buf_alloc(struct fastrpc_user *fl, struct device *dev,
buf = *obuf;
if (fl->sctx && fl->sctx->sid)
- buf->phys += ((u64)fl->sctx->sid << 32);
+ buf->dma_addr += ((u64)fl->sctx->sid << 32);
return 0;
}
@@ -684,7 +685,7 @@ static int fastrpc_dma_buf_attach(struct dma_buf *dmabuf,
return -ENOMEM;
ret = dma_get_sgtable(buffer->dev, &a->sgt, buffer->virt,
- FASTRPC_PHYS(buffer->phys), buffer->size);
+ FASTRPC_PHYS(buffer->dma_addr), buffer->size);
if (ret < 0) {
dev_err(buffer->dev, "failed to get scatterlist from DMA API\n");
kfree(a);
@@ -733,7 +734,7 @@ static int fastrpc_mmap(struct dma_buf *dmabuf,
dma_resv_assert_held(dmabuf->resv);
return dma_mmap_coherent(buf->dev, vma, buf->virt,
- FASTRPC_PHYS(buf->phys), size);
+ FASTRPC_PHYS(buf->dma_addr), size);
}
static const struct dma_buf_ops fastrpc_dma_buf_ops = {
@@ -785,10 +786,10 @@ static int fastrpc_map_attach(struct fastrpc_user *fl, int fd,
map->table = table;
if (attr & FASTRPC_ATTR_SECUREMAP) {
- map->phys = sg_phys(map->table->sgl);
+ map->dma_addr = sg_phys(map->table->sgl);
} else {
- map->phys = sg_dma_address(map->table->sgl);
- map->phys += ((u64)fl->sctx->sid << 32);
+ map->dma_addr = sg_dma_address(map->table->sgl);
+ map->dma_addr += ((u64)fl->sctx->sid << 32);
}
for_each_sg(map->table->sgl, sgl, map->table->nents,
sgl_index)
@@ -815,10 +816,11 @@ static int fastrpc_map_attach(struct fastrpc_user *fl, int fd,
dst_perms[1].vmid = fl->cctx->vmperms[0].vmid;
dst_perms[1].perm = QCOM_SCM_PERM_RWX;
map->attr = attr;
- err = qcom_scm_assign_mem(map->phys, (u64)map->len, &src_perms, dst_perms, 2);
+ err = qcom_scm_assign_mem(map->dma_addr, (u64)map->len, &src_perms, dst_perms, 2);
if (err) {
- dev_err(sess->dev, "Failed to assign memory with phys 0x%llx size 0x%llx err %d\n",
- map->phys, map->len, err);
+ dev_err(sess->dev,
+ "Failed to assign memory with dma_addr 0x%llx size 0x%llx err %d\n",
+ map->dma_addr, map->len, err);
goto map_err;
}
}
@@ -1009,7 +1011,7 @@ static int fastrpc_get_args(u32 kernel, struct fastrpc_invoke_ctx *ctx)
struct vm_area_struct *vma = NULL;
rpra[i].buf.pv = (u64) ctx->args[i].ptr;
- pages[i].addr = ctx->maps[i]->phys;
+ pages[i].addr = ctx->maps[i]->dma_addr;
mmap_read_lock(current->mm);
vma = find_vma(current->mm, ctx->args[i].ptr);
@@ -1036,7 +1038,7 @@ static int fastrpc_get_args(u32 kernel, struct fastrpc_invoke_ctx *ctx)
goto bail;
rpra[i].buf.pv = args - ctx->olaps[oix].offset;
- pages[i].addr = ctx->buf->phys -
+ pages[i].addr = ctx->buf->dma_addr -
ctx->olaps[oix].offset +
(pkt_size - rlen);
pages[i].addr = pages[i].addr & PAGE_MASK;
@@ -1068,7 +1070,7 @@ static int fastrpc_get_args(u32 kernel, struct fastrpc_invoke_ctx *ctx)
list[i].num = ctx->args[i].length ? 1 : 0;
list[i].pgidx = i;
if (ctx->maps[i]) {
- pages[i].addr = ctx->maps[i]->phys;
+ pages[i].addr = ctx->maps[i]->dma_addr;
pages[i].size = ctx->maps[i]->size;
}
rpra[i].dma.fd = ctx->args[i].fd;
@@ -1150,7 +1152,7 @@ static int fastrpc_invoke_send(struct fastrpc_session_ctx *sctx,
msg->ctx = ctx->ctxid | fl->pd;
msg->handle = handle;
msg->sc = ctx->sc;
- msg->addr = ctx->buf ? ctx->buf->phys : 0;
+ msg->addr = ctx->buf ? ctx->buf->dma_addr : 0;
msg->size = roundup(ctx->msg_sz, PAGE_SIZE);
fastrpc_context_get(ctx);
@@ -1306,13 +1308,14 @@ static int fastrpc_init_create_static_process(struct fastrpc_user *fl,
if (fl->cctx->vmcount) {
u64 src_perms = BIT(QCOM_SCM_VMID_HLOS);
- err = qcom_scm_assign_mem(fl->cctx->remote_heap->phys,
+ err = qcom_scm_assign_mem(fl->cctx->remote_heap->dma_addr,
(u64)fl->cctx->remote_heap->size,
&src_perms,
fl->cctx->vmperms, fl->cctx->vmcount);
if (err) {
- dev_err(fl->sctx->dev, "Failed to assign memory with phys 0x%llx size 0x%llx err %d\n",
- fl->cctx->remote_heap->phys, fl->cctx->remote_heap->size, err);
+ dev_err(fl->sctx->dev,
+ "Failed to assign memory with dma_addr 0x%llx size 0x%llx err %d\n",
+ fl->cctx->remote_heap->dma_addr, fl->cctx->remote_heap->size, err);
goto err_map;
}
scm_done = true;
@@ -1332,7 +1335,7 @@ static int fastrpc_init_create_static_process(struct fastrpc_user *fl,
args[1].length = inbuf.namelen;
args[1].fd = -1;
- pages[0].addr = fl->cctx->remote_heap->phys;
+ pages[0].addr = fl->cctx->remote_heap->dma_addr;
pages[0].size = fl->cctx->remote_heap->size;
args[2].ptr = (u64)(uintptr_t) pages;
@@ -1361,12 +1364,12 @@ static int fastrpc_init_create_static_process(struct fastrpc_user *fl,
dst_perms.vmid = QCOM_SCM_VMID_HLOS;
dst_perms.perm = QCOM_SCM_PERM_RWX;
- err = qcom_scm_assign_mem(fl->cctx->remote_heap->phys,
+ err = qcom_scm_assign_mem(fl->cctx->remote_heap->dma_addr,
(u64)fl->cctx->remote_heap->size,
&src_perms, &dst_perms, 1);
if (err)
- dev_err(fl->sctx->dev, "Failed to assign memory phys 0x%llx size 0x%llx err %d\n",
- fl->cctx->remote_heap->phys, fl->cctx->remote_heap->size, err);
+ dev_err(fl->sctx->dev, "Failed to assign memory dma_addr 0x%llx size 0x%llx err %d\n",
+ fl->cctx->remote_heap->dma_addr, fl->cctx->remote_heap->size, err);
}
err_map:
fastrpc_buf_free(fl->cctx->remote_heap);
@@ -1455,7 +1458,7 @@ static int fastrpc_init_create_process(struct fastrpc_user *fl,
args[2].length = inbuf.filelen;
args[2].fd = init.filefd;
- pages[0].addr = imem->phys;
+ pages[0].addr = imem->dma_addr;
pages[0].size = imem->size;
args[3].ptr = (u64)(uintptr_t) pages;
@@ -1913,7 +1916,7 @@ static int fastrpc_req_mmap(struct fastrpc_user *fl, char __user *argp)
args[0].ptr = (u64) (uintptr_t) &req_msg;
args[0].length = sizeof(req_msg);
- pages.addr = buf->phys;
+ pages.addr = buf->dma_addr;
pages.size = buf->size;
args[1].ptr = (u64) (uintptr_t) &pages;
@@ -1941,11 +1944,12 @@ static int fastrpc_req_mmap(struct fastrpc_user *fl, char __user *argp)
if (req.flags == ADSP_MMAP_REMOTE_HEAP_ADDR && fl->cctx->vmcount) {
u64 src_perms = BIT(QCOM_SCM_VMID_HLOS);
- err = qcom_scm_assign_mem(buf->phys, (u64)buf->size,
+ err = qcom_scm_assign_mem(buf->dma_addr, (u64)buf->size,
&src_perms, fl->cctx->vmperms, fl->cctx->vmcount);
if (err) {
- dev_err(fl->sctx->dev, "Failed to assign memory phys 0x%llx size 0x%llx err %d",
- buf->phys, buf->size, err);
+ dev_err(fl->sctx->dev,
+ "Failed to assign memory dma_addr 0x%llx size 0x%llx err %d",
+ buf->dma_addr, buf->size, err);
goto err_assign;
}
}
@@ -2059,7 +2063,7 @@ static int fastrpc_req_mem_map(struct fastrpc_user *fl, char __user *argp)
args[0].ptr = (u64) (uintptr_t) &req_msg;
args[0].length = sizeof(req_msg);
- pages.addr = map->phys;
+ pages.addr = map->dma_addr;
pages.size = map->len;
args[1].ptr = (u64) (uintptr_t) &pages;
--
2.34.1On Fri, Nov 14, 2025 at 02:11:40PM +0530, Kumari Pallavi wrote:
> Update all references of buf->phys and map->phys to buf->dma_addr and
> map->dma_addr to accurately represent that these fields store DMA
> addresses, not physical addresses. This change improves code clarity
> and aligns with kernel conventions for dma_addr_t usage.
>
> Signed-off-by: Kumari Pallavi <kumari.pallavi@oss.qualcomm.com>
> ---
> drivers/misc/fastrpc.c | 76 ++++++++++++++++++++++--------------------
> 1 file changed, 40 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
> index ee652ef01534..d6a7960fe716 100644
> --- a/drivers/misc/fastrpc.c
> +++ b/drivers/misc/fastrpc.c
> @@ -106,7 +106,7 @@
> #define miscdev_to_fdevice(d) container_of(d, struct fastrpc_device, miscdev)
>
> struct fastrpc_phy_page {
> - u64 addr; /* physical address */
> + u64 addr; /* physical or dma address */
> u64 size; /* size of contiguous region */
> };
>
> @@ -171,7 +171,7 @@ struct fastrpc_msg {
> u64 ctx; /* invoke caller context */
> u32 handle; /* handle to invoke */
> u32 sc; /* scalars structure describing the data */
> - u64 addr; /* physical address */
> + u64 addr; /* physical or dma address */
Can you go all the way and make the type dma_addr_t? That way you don't
need to typecast the dma_alloc_coherent() and dma_free_coherent().
I believe it might complicate the places where you do math on it, but
that is a good thing, as it highlights those places where you do
something unexpected.
> u64 size; /* size of contiguous region */
> };
>
> @@ -194,7 +194,7 @@ struct fastrpc_buf {
> struct dma_buf *dmabuf;
> struct device *dev;
> void *virt;
> - u64 phys;
> + u64 dma_addr;
> u64 size;
> /* Lock for dma buf attachments */
> struct mutex lock;
> @@ -217,7 +217,7 @@ struct fastrpc_map {
> struct dma_buf *buf;
> struct sg_table *table;
> struct dma_buf_attachment *attach;
> - u64 phys;
> + u64 dma_addr;
> u64 size;
> void *va;
> u64 len;
> @@ -320,11 +320,12 @@ static void fastrpc_free_map(struct kref *ref)
>
> perm.vmid = QCOM_SCM_VMID_HLOS;
> perm.perm = QCOM_SCM_PERM_RWX;
> - err = qcom_scm_assign_mem(map->phys, map->len,
> + err = qcom_scm_assign_mem(map->dma_addr, map->len,
> &src_perms, &perm, 1);
> if (err) {
> - dev_err(map->fl->sctx->dev, "Failed to assign memory phys 0x%llx size 0x%llx err %d\n",
> - map->phys, map->len, err);
> + dev_err(map->fl->sctx->dev,
> + "Failed to assign memory dma_addr 0x%llx size 0x%llx err %d\n",
> + map->dma_addr, map->len, err);
> return;
> }
> }
> @@ -389,7 +390,7 @@ static int fastrpc_map_lookup(struct fastrpc_user *fl, int fd,
> static void fastrpc_buf_free(struct fastrpc_buf *buf)
> {
> dma_free_coherent(buf->dev, buf->size, buf->virt,
> - FASTRPC_PHYS(buf->phys));
> + FASTRPC_PHYS(buf->dma_addr));
> kfree(buf);
> }
>
> @@ -408,12 +409,12 @@ static int __fastrpc_buf_alloc(struct fastrpc_user *fl, struct device *dev,
>
> buf->fl = fl;
> buf->virt = NULL;
> - buf->phys = 0;
> + buf->dma_addr = 0;
> buf->size = size;
> buf->dev = dev;
> buf->raddr = 0;
>
> - buf->virt = dma_alloc_coherent(dev, buf->size, (dma_addr_t *)&buf->phys,
> + buf->virt = dma_alloc_coherent(dev, buf->size, (dma_addr_t *)&buf->dma_addr,
> GFP_KERNEL);
> if (!buf->virt) {
> mutex_destroy(&buf->lock);
> @@ -439,7 +440,7 @@ static int fastrpc_buf_alloc(struct fastrpc_user *fl, struct device *dev,
> buf = *obuf;
>
> if (fl->sctx && fl->sctx->sid)
> - buf->phys += ((u64)fl->sctx->sid << 32);
> + buf->dma_addr += ((u64)fl->sctx->sid << 32);
>
> return 0;
> }
> @@ -684,7 +685,7 @@ static int fastrpc_dma_buf_attach(struct dma_buf *dmabuf,
> return -ENOMEM;
>
> ret = dma_get_sgtable(buffer->dev, &a->sgt, buffer->virt,
> - FASTRPC_PHYS(buffer->phys), buffer->size);
> + FASTRPC_PHYS(buffer->dma_addr), buffer->size);
> if (ret < 0) {
> dev_err(buffer->dev, "failed to get scatterlist from DMA API\n");
> kfree(a);
> @@ -733,7 +734,7 @@ static int fastrpc_mmap(struct dma_buf *dmabuf,
> dma_resv_assert_held(dmabuf->resv);
>
> return dma_mmap_coherent(buf->dev, vma, buf->virt,
> - FASTRPC_PHYS(buf->phys), size);
> + FASTRPC_PHYS(buf->dma_addr), size);
In fact, we invoke dma_alloc_coherent() above to get a dma_addr_t, and
then we call map, unmap, and free on the lower 32 bits of that
address...
In other words, each time we reference dma_addr we have that implicit
thing that it's a composit of a dma_addr_t as seen from Linux's point of
view (which is matching the addresses in the SMMU page tables) and the
adjusted address that we use in communication with the firmware to
direct the accesses to the right SID + iova.
I think it would be quite nice to make this more explicit throughout the
code, rather than juggling the two perspectives in the same variable.
Regards,
Bjorn
> }
>
> static const struct dma_buf_ops fastrpc_dma_buf_ops = {
> @@ -785,10 +786,10 @@ static int fastrpc_map_attach(struct fastrpc_user *fl, int fd,
> map->table = table;
>
> if (attr & FASTRPC_ATTR_SECUREMAP) {
> - map->phys = sg_phys(map->table->sgl);
> + map->dma_addr = sg_phys(map->table->sgl);
> } else {
> - map->phys = sg_dma_address(map->table->sgl);
> - map->phys += ((u64)fl->sctx->sid << 32);
> + map->dma_addr = sg_dma_address(map->table->sgl);
> + map->dma_addr += ((u64)fl->sctx->sid << 32);
> }
> for_each_sg(map->table->sgl, sgl, map->table->nents,
> sgl_index)
> @@ -815,10 +816,11 @@ static int fastrpc_map_attach(struct fastrpc_user *fl, int fd,
> dst_perms[1].vmid = fl->cctx->vmperms[0].vmid;
> dst_perms[1].perm = QCOM_SCM_PERM_RWX;
> map->attr = attr;
> - err = qcom_scm_assign_mem(map->phys, (u64)map->len, &src_perms, dst_perms, 2);
> + err = qcom_scm_assign_mem(map->dma_addr, (u64)map->len, &src_perms, dst_perms, 2);
> if (err) {
> - dev_err(sess->dev, "Failed to assign memory with phys 0x%llx size 0x%llx err %d\n",
> - map->phys, map->len, err);
> + dev_err(sess->dev,
> + "Failed to assign memory with dma_addr 0x%llx size 0x%llx err %d\n",
> + map->dma_addr, map->len, err);
> goto map_err;
> }
> }
> @@ -1009,7 +1011,7 @@ static int fastrpc_get_args(u32 kernel, struct fastrpc_invoke_ctx *ctx)
> struct vm_area_struct *vma = NULL;
>
> rpra[i].buf.pv = (u64) ctx->args[i].ptr;
> - pages[i].addr = ctx->maps[i]->phys;
> + pages[i].addr = ctx->maps[i]->dma_addr;
>
> mmap_read_lock(current->mm);
> vma = find_vma(current->mm, ctx->args[i].ptr);
> @@ -1036,7 +1038,7 @@ static int fastrpc_get_args(u32 kernel, struct fastrpc_invoke_ctx *ctx)
> goto bail;
>
> rpra[i].buf.pv = args - ctx->olaps[oix].offset;
> - pages[i].addr = ctx->buf->phys -
> + pages[i].addr = ctx->buf->dma_addr -
> ctx->olaps[oix].offset +
> (pkt_size - rlen);
> pages[i].addr = pages[i].addr & PAGE_MASK;
> @@ -1068,7 +1070,7 @@ static int fastrpc_get_args(u32 kernel, struct fastrpc_invoke_ctx *ctx)
> list[i].num = ctx->args[i].length ? 1 : 0;
> list[i].pgidx = i;
> if (ctx->maps[i]) {
> - pages[i].addr = ctx->maps[i]->phys;
> + pages[i].addr = ctx->maps[i]->dma_addr;
> pages[i].size = ctx->maps[i]->size;
> }
> rpra[i].dma.fd = ctx->args[i].fd;
> @@ -1150,7 +1152,7 @@ static int fastrpc_invoke_send(struct fastrpc_session_ctx *sctx,
> msg->ctx = ctx->ctxid | fl->pd;
> msg->handle = handle;
> msg->sc = ctx->sc;
> - msg->addr = ctx->buf ? ctx->buf->phys : 0;
> + msg->addr = ctx->buf ? ctx->buf->dma_addr : 0;
> msg->size = roundup(ctx->msg_sz, PAGE_SIZE);
> fastrpc_context_get(ctx);
>
> @@ -1306,13 +1308,14 @@ static int fastrpc_init_create_static_process(struct fastrpc_user *fl,
> if (fl->cctx->vmcount) {
> u64 src_perms = BIT(QCOM_SCM_VMID_HLOS);
>
> - err = qcom_scm_assign_mem(fl->cctx->remote_heap->phys,
> + err = qcom_scm_assign_mem(fl->cctx->remote_heap->dma_addr,
> (u64)fl->cctx->remote_heap->size,
> &src_perms,
> fl->cctx->vmperms, fl->cctx->vmcount);
> if (err) {
> - dev_err(fl->sctx->dev, "Failed to assign memory with phys 0x%llx size 0x%llx err %d\n",
> - fl->cctx->remote_heap->phys, fl->cctx->remote_heap->size, err);
> + dev_err(fl->sctx->dev,
> + "Failed to assign memory with dma_addr 0x%llx size 0x%llx err %d\n",
> + fl->cctx->remote_heap->dma_addr, fl->cctx->remote_heap->size, err);
> goto err_map;
> }
> scm_done = true;
> @@ -1332,7 +1335,7 @@ static int fastrpc_init_create_static_process(struct fastrpc_user *fl,
> args[1].length = inbuf.namelen;
> args[1].fd = -1;
>
> - pages[0].addr = fl->cctx->remote_heap->phys;
> + pages[0].addr = fl->cctx->remote_heap->dma_addr;
> pages[0].size = fl->cctx->remote_heap->size;
>
> args[2].ptr = (u64)(uintptr_t) pages;
> @@ -1361,12 +1364,12 @@ static int fastrpc_init_create_static_process(struct fastrpc_user *fl,
>
> dst_perms.vmid = QCOM_SCM_VMID_HLOS;
> dst_perms.perm = QCOM_SCM_PERM_RWX;
> - err = qcom_scm_assign_mem(fl->cctx->remote_heap->phys,
> + err = qcom_scm_assign_mem(fl->cctx->remote_heap->dma_addr,
> (u64)fl->cctx->remote_heap->size,
> &src_perms, &dst_perms, 1);
> if (err)
> - dev_err(fl->sctx->dev, "Failed to assign memory phys 0x%llx size 0x%llx err %d\n",
> - fl->cctx->remote_heap->phys, fl->cctx->remote_heap->size, err);
> + dev_err(fl->sctx->dev, "Failed to assign memory dma_addr 0x%llx size 0x%llx err %d\n",
> + fl->cctx->remote_heap->dma_addr, fl->cctx->remote_heap->size, err);
> }
> err_map:
> fastrpc_buf_free(fl->cctx->remote_heap);
> @@ -1455,7 +1458,7 @@ static int fastrpc_init_create_process(struct fastrpc_user *fl,
> args[2].length = inbuf.filelen;
> args[2].fd = init.filefd;
>
> - pages[0].addr = imem->phys;
> + pages[0].addr = imem->dma_addr;
> pages[0].size = imem->size;
>
> args[3].ptr = (u64)(uintptr_t) pages;
> @@ -1913,7 +1916,7 @@ static int fastrpc_req_mmap(struct fastrpc_user *fl, char __user *argp)
> args[0].ptr = (u64) (uintptr_t) &req_msg;
> args[0].length = sizeof(req_msg);
>
> - pages.addr = buf->phys;
> + pages.addr = buf->dma_addr;
> pages.size = buf->size;
>
> args[1].ptr = (u64) (uintptr_t) &pages;
> @@ -1941,11 +1944,12 @@ static int fastrpc_req_mmap(struct fastrpc_user *fl, char __user *argp)
> if (req.flags == ADSP_MMAP_REMOTE_HEAP_ADDR && fl->cctx->vmcount) {
> u64 src_perms = BIT(QCOM_SCM_VMID_HLOS);
>
> - err = qcom_scm_assign_mem(buf->phys, (u64)buf->size,
> + err = qcom_scm_assign_mem(buf->dma_addr, (u64)buf->size,
> &src_perms, fl->cctx->vmperms, fl->cctx->vmcount);
> if (err) {
> - dev_err(fl->sctx->dev, "Failed to assign memory phys 0x%llx size 0x%llx err %d",
> - buf->phys, buf->size, err);
> + dev_err(fl->sctx->dev,
> + "Failed to assign memory dma_addr 0x%llx size 0x%llx err %d",
> + buf->dma_addr, buf->size, err);
> goto err_assign;
> }
> }
> @@ -2059,7 +2063,7 @@ static int fastrpc_req_mem_map(struct fastrpc_user *fl, char __user *argp)
> args[0].ptr = (u64) (uintptr_t) &req_msg;
> args[0].length = sizeof(req_msg);
>
> - pages.addr = map->phys;
> + pages.addr = map->dma_addr;
> pages.size = map->len;
>
> args[1].ptr = (u64) (uintptr_t) &pages;
> --
> 2.34.1
>
>
On 11/14/2025 9:14 PM, Bjorn Andersson wrote:
> On Fri, Nov 14, 2025 at 02:11:40PM +0530, Kumari Pallavi wrote:
>> Update all references of buf->phys and map->phys to buf->dma_addr and
>> map->dma_addr to accurately represent that these fields store DMA
>> addresses, not physical addresses. This change improves code clarity
>> and aligns with kernel conventions for dma_addr_t usage.
>>
>> Signed-off-by: Kumari Pallavi <kumari.pallavi@oss.qualcomm.com>
>> ---
>> drivers/misc/fastrpc.c | 76 ++++++++++++++++++++++--------------------
>> 1 file changed, 40 insertions(+), 36 deletions(-)
>>
>> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
>> index ee652ef01534..d6a7960fe716 100644
>> --- a/drivers/misc/fastrpc.c
>> +++ b/drivers/misc/fastrpc.c
>> @@ -106,7 +106,7 @@
>> #define miscdev_to_fdevice(d) container_of(d, struct fastrpc_device, miscdev)
>>
>> struct fastrpc_phy_page {
>> - u64 addr; /* physical address */
>> + u64 addr; /* physical or dma address */
>> u64 size; /* size of contiguous region */
>> };
>>
>> @@ -171,7 +171,7 @@ struct fastrpc_msg {
>> u64 ctx; /* invoke caller context */
>> u32 handle; /* handle to invoke */
>> u32 sc; /* scalars structure describing the data */
>> - u64 addr; /* physical address */
>> + u64 addr; /* physical or dma address */
>
> Can you go all the way and make the type dma_addr_t? That way you don't
> need to typecast the dma_alloc_coherent() and dma_free_coherent().
>
> I believe it might complicate the places where you do math on it, but
> that is a good thing, as it highlights those places where you do
> something unexpected.
>
While this not strictly limited to holding a dma_addr_t.
Based on historical behavior, when the FASTRPC_ATTR_SECUREMAP flag is
set, S2 mapping expects a physical address to be passed to the
qcom_scm_assign_mem() API.
reference-
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/drivers/misc/fastrpc.c?id=e90d911906196bf987492c94e38f10ca611dfd7b
If you suggest, I can define it as dma_addr_t and perform typecasting to
u64 wherever required.
Thanks,
Pallavi
>> u64 size; /* size of contiguous region */
>> };
>>
>> @@ -194,7 +194,7 @@ struct fastrpc_buf {
>> struct dma_buf *dmabuf;
>> struct device *dev;
>> void *virt;
>> - u64 phys;
>> + u64 dma_addr;
>> u64 size;
>> /* Lock for dma buf attachments */
>> struct mutex lock;
>> @@ -217,7 +217,7 @@ struct fastrpc_map {
>> struct dma_buf *buf;
>> struct sg_table *table;
>> struct dma_buf_attachment *attach;
>> - u64 phys;
>> + u64 dma_addr;
>> u64 size;
>> void *va;
>> u64 len;
>> @@ -320,11 +320,12 @@ static void fastrpc_free_map(struct kref *ref)
>>
>> perm.vmid = QCOM_SCM_VMID_HLOS;
>> perm.perm = QCOM_SCM_PERM_RWX;
>> - err = qcom_scm_assign_mem(map->phys, map->len,
>> + err = qcom_scm_assign_mem(map->dma_addr, map->len,
>> &src_perms, &perm, 1);
>> if (err) {
>> - dev_err(map->fl->sctx->dev, "Failed to assign memory phys 0x%llx size 0x%llx err %d\n",
>> - map->phys, map->len, err);
>> + dev_err(map->fl->sctx->dev,
>> + "Failed to assign memory dma_addr 0x%llx size 0x%llx err %d\n",
>> + map->dma_addr, map->len, err);
>> return;
>> }
>> }
>> @@ -389,7 +390,7 @@ static int fastrpc_map_lookup(struct fastrpc_user *fl, int fd,
>> static void fastrpc_buf_free(struct fastrpc_buf *buf)
>> {
>> dma_free_coherent(buf->dev, buf->size, buf->virt,
>> - FASTRPC_PHYS(buf->phys));
>> + FASTRPC_PHYS(buf->dma_addr));
>> kfree(buf);
>> }
>>
>> @@ -408,12 +409,12 @@ static int __fastrpc_buf_alloc(struct fastrpc_user *fl, struct device *dev,
>>
>> buf->fl = fl;
>> buf->virt = NULL;
>> - buf->phys = 0;
>> + buf->dma_addr = 0;
>> buf->size = size;
>> buf->dev = dev;
>> buf->raddr = 0;
>>
>> - buf->virt = dma_alloc_coherent(dev, buf->size, (dma_addr_t *)&buf->phys,
>> + buf->virt = dma_alloc_coherent(dev, buf->size, (dma_addr_t *)&buf->dma_addr,
>> GFP_KERNEL);
>> if (!buf->virt) {
>> mutex_destroy(&buf->lock);
>> @@ -439,7 +440,7 @@ static int fastrpc_buf_alloc(struct fastrpc_user *fl, struct device *dev,
>> buf = *obuf;
>>
>> if (fl->sctx && fl->sctx->sid)
>> - buf->phys += ((u64)fl->sctx->sid << 32);
>> + buf->dma_addr += ((u64)fl->sctx->sid << 32);
>>
>> return 0;
>> }
>> @@ -684,7 +685,7 @@ static int fastrpc_dma_buf_attach(struct dma_buf *dmabuf,
>> return -ENOMEM;
>>
>> ret = dma_get_sgtable(buffer->dev, &a->sgt, buffer->virt,
>> - FASTRPC_PHYS(buffer->phys), buffer->size);
>> + FASTRPC_PHYS(buffer->dma_addr), buffer->size);
>> if (ret < 0) {
>> dev_err(buffer->dev, "failed to get scatterlist from DMA API\n");
>> kfree(a);
>> @@ -733,7 +734,7 @@ static int fastrpc_mmap(struct dma_buf *dmabuf,
>> dma_resv_assert_held(dmabuf->resv);
>>
>> return dma_mmap_coherent(buf->dev, vma, buf->virt,
>> - FASTRPC_PHYS(buf->phys), size);
>> + FASTRPC_PHYS(buf->dma_addr), size);
>
> In fact, we invoke dma_alloc_coherent() above to get a dma_addr_t, and
> then we call map, unmap, and free on the lower 32 bits of that
> address...
>
> In other words, each time we reference dma_addr we have that implicit
> thing that it's a composit of a dma_addr_t as seen from Linux's point of
> view (which is matching the addresses in the SMMU page tables) and the
> adjusted address that we use in communication with the firmware to
> direct the accesses to the right SID + iova.
>
> I think it would be quite nice to make this more explicit throughout the
> code, rather than juggling the two perspectives in the same variable.
>
> Regards,
> Bjorn
>
>> }
>>
>> static const struct dma_buf_ops fastrpc_dma_buf_ops = {
>> @@ -785,10 +786,10 @@ static int fastrpc_map_attach(struct fastrpc_user *fl, int fd,
>> map->table = table;
>>
>> if (attr & FASTRPC_ATTR_SECUREMAP) {
>> - map->phys = sg_phys(map->table->sgl);
>> + map->dma_addr = sg_phys(map->table->sgl);
>> } else {
>> - map->phys = sg_dma_address(map->table->sgl);
>> - map->phys += ((u64)fl->sctx->sid << 32);
>> + map->dma_addr = sg_dma_address(map->table->sgl);
>> + map->dma_addr += ((u64)fl->sctx->sid << 32);
>> }
>> for_each_sg(map->table->sgl, sgl, map->table->nents,
>> sgl_index)
>> @@ -815,10 +816,11 @@ static int fastrpc_map_attach(struct fastrpc_user *fl, int fd,
>> dst_perms[1].vmid = fl->cctx->vmperms[0].vmid;
>> dst_perms[1].perm = QCOM_SCM_PERM_RWX;
>> map->attr = attr;
>> - err = qcom_scm_assign_mem(map->phys, (u64)map->len, &src_perms, dst_perms, 2);
>> + err = qcom_scm_assign_mem(map->dma_addr, (u64)map->len, &src_perms, dst_perms, 2);
>> if (err) {
>> - dev_err(sess->dev, "Failed to assign memory with phys 0x%llx size 0x%llx err %d\n",
>> - map->phys, map->len, err);
>> + dev_err(sess->dev,
>> + "Failed to assign memory with dma_addr 0x%llx size 0x%llx err %d\n",
>> + map->dma_addr, map->len, err);
>> goto map_err;
>> }
>> }
>> @@ -1009,7 +1011,7 @@ static int fastrpc_get_args(u32 kernel, struct fastrpc_invoke_ctx *ctx)
>> struct vm_area_struct *vma = NULL;
>>
>> rpra[i].buf.pv = (u64) ctx->args[i].ptr;
>> - pages[i].addr = ctx->maps[i]->phys;
>> + pages[i].addr = ctx->maps[i]->dma_addr;
>>
>> mmap_read_lock(current->mm);
>> vma = find_vma(current->mm, ctx->args[i].ptr);
>> @@ -1036,7 +1038,7 @@ static int fastrpc_get_args(u32 kernel, struct fastrpc_invoke_ctx *ctx)
>> goto bail;
>>
>> rpra[i].buf.pv = args - ctx->olaps[oix].offset;
>> - pages[i].addr = ctx->buf->phys -
>> + pages[i].addr = ctx->buf->dma_addr -
>> ctx->olaps[oix].offset +
>> (pkt_size - rlen);
>> pages[i].addr = pages[i].addr & PAGE_MASK;
>> @@ -1068,7 +1070,7 @@ static int fastrpc_get_args(u32 kernel, struct fastrpc_invoke_ctx *ctx)
>> list[i].num = ctx->args[i].length ? 1 : 0;
>> list[i].pgidx = i;
>> if (ctx->maps[i]) {
>> - pages[i].addr = ctx->maps[i]->phys;
>> + pages[i].addr = ctx->maps[i]->dma_addr;
>> pages[i].size = ctx->maps[i]->size;
>> }
>> rpra[i].dma.fd = ctx->args[i].fd;
>> @@ -1150,7 +1152,7 @@ static int fastrpc_invoke_send(struct fastrpc_session_ctx *sctx,
>> msg->ctx = ctx->ctxid | fl->pd;
>> msg->handle = handle;
>> msg->sc = ctx->sc;
>> - msg->addr = ctx->buf ? ctx->buf->phys : 0;
>> + msg->addr = ctx->buf ? ctx->buf->dma_addr : 0;
>> msg->size = roundup(ctx->msg_sz, PAGE_SIZE);
>> fastrpc_context_get(ctx);
>>
>> @@ -1306,13 +1308,14 @@ static int fastrpc_init_create_static_process(struct fastrpc_user *fl,
>> if (fl->cctx->vmcount) {
>> u64 src_perms = BIT(QCOM_SCM_VMID_HLOS);
>>
>> - err = qcom_scm_assign_mem(fl->cctx->remote_heap->phys,
>> + err = qcom_scm_assign_mem(fl->cctx->remote_heap->dma_addr,
>> (u64)fl->cctx->remote_heap->size,
>> &src_perms,
>> fl->cctx->vmperms, fl->cctx->vmcount);
>> if (err) {
>> - dev_err(fl->sctx->dev, "Failed to assign memory with phys 0x%llx size 0x%llx err %d\n",
>> - fl->cctx->remote_heap->phys, fl->cctx->remote_heap->size, err);
>> + dev_err(fl->sctx->dev,
>> + "Failed to assign memory with dma_addr 0x%llx size 0x%llx err %d\n",
>> + fl->cctx->remote_heap->dma_addr, fl->cctx->remote_heap->size, err);
>> goto err_map;
>> }
>> scm_done = true;
>> @@ -1332,7 +1335,7 @@ static int fastrpc_init_create_static_process(struct fastrpc_user *fl,
>> args[1].length = inbuf.namelen;
>> args[1].fd = -1;
>>
>> - pages[0].addr = fl->cctx->remote_heap->phys;
>> + pages[0].addr = fl->cctx->remote_heap->dma_addr;
>> pages[0].size = fl->cctx->remote_heap->size;
>>
>> args[2].ptr = (u64)(uintptr_t) pages;
>> @@ -1361,12 +1364,12 @@ static int fastrpc_init_create_static_process(struct fastrpc_user *fl,
>>
>> dst_perms.vmid = QCOM_SCM_VMID_HLOS;
>> dst_perms.perm = QCOM_SCM_PERM_RWX;
>> - err = qcom_scm_assign_mem(fl->cctx->remote_heap->phys,
>> + err = qcom_scm_assign_mem(fl->cctx->remote_heap->dma_addr,
>> (u64)fl->cctx->remote_heap->size,
>> &src_perms, &dst_perms, 1);
>> if (err)
>> - dev_err(fl->sctx->dev, "Failed to assign memory phys 0x%llx size 0x%llx err %d\n",
>> - fl->cctx->remote_heap->phys, fl->cctx->remote_heap->size, err);
>> + dev_err(fl->sctx->dev, "Failed to assign memory dma_addr 0x%llx size 0x%llx err %d\n",
>> + fl->cctx->remote_heap->dma_addr, fl->cctx->remote_heap->size, err);
>> }
>> err_map:
>> fastrpc_buf_free(fl->cctx->remote_heap);
>> @@ -1455,7 +1458,7 @@ static int fastrpc_init_create_process(struct fastrpc_user *fl,
>> args[2].length = inbuf.filelen;
>> args[2].fd = init.filefd;
>>
>> - pages[0].addr = imem->phys;
>> + pages[0].addr = imem->dma_addr;
>> pages[0].size = imem->size;
>>
>> args[3].ptr = (u64)(uintptr_t) pages;
>> @@ -1913,7 +1916,7 @@ static int fastrpc_req_mmap(struct fastrpc_user *fl, char __user *argp)
>> args[0].ptr = (u64) (uintptr_t) &req_msg;
>> args[0].length = sizeof(req_msg);
>>
>> - pages.addr = buf->phys;
>> + pages.addr = buf->dma_addr;
>> pages.size = buf->size;
>>
>> args[1].ptr = (u64) (uintptr_t) &pages;
>> @@ -1941,11 +1944,12 @@ static int fastrpc_req_mmap(struct fastrpc_user *fl, char __user *argp)
>> if (req.flags == ADSP_MMAP_REMOTE_HEAP_ADDR && fl->cctx->vmcount) {
>> u64 src_perms = BIT(QCOM_SCM_VMID_HLOS);
>>
>> - err = qcom_scm_assign_mem(buf->phys, (u64)buf->size,
>> + err = qcom_scm_assign_mem(buf->dma_addr, (u64)buf->size,
>> &src_perms, fl->cctx->vmperms, fl->cctx->vmcount);
>> if (err) {
>> - dev_err(fl->sctx->dev, "Failed to assign memory phys 0x%llx size 0x%llx err %d",
>> - buf->phys, buf->size, err);
>> + dev_err(fl->sctx->dev,
>> + "Failed to assign memory dma_addr 0x%llx size 0x%llx err %d",
>> + buf->dma_addr, buf->size, err);
>> goto err_assign;
>> }
>> }
>> @@ -2059,7 +2063,7 @@ static int fastrpc_req_mem_map(struct fastrpc_user *fl, char __user *argp)
>> args[0].ptr = (u64) (uintptr_t) &req_msg;
>> args[0].length = sizeof(req_msg);
>>
>> - pages.addr = map->phys;
>> + pages.addr = map->dma_addr;
>> pages.size = map->len;
>>
>> args[1].ptr = (u64) (uintptr_t) &pages;
>> --
>> 2.34.1
>>
>>
On Mon, Nov 17, 2025 at 12:37:33PM +0530, Kumari Pallavi wrote:
>
>
> On 11/14/2025 9:14 PM, Bjorn Andersson wrote:
> > On Fri, Nov 14, 2025 at 02:11:40PM +0530, Kumari Pallavi wrote:
> > > Update all references of buf->phys and map->phys to buf->dma_addr and
> > > map->dma_addr to accurately represent that these fields store DMA
> > > addresses, not physical addresses. This change improves code clarity
> > > and aligns with kernel conventions for dma_addr_t usage.
> > >
> > > Signed-off-by: Kumari Pallavi <kumari.pallavi@oss.qualcomm.com>
> > > ---
> > > drivers/misc/fastrpc.c | 76 ++++++++++++++++++++++--------------------
> > > 1 file changed, 40 insertions(+), 36 deletions(-)
> > >
> > > diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
> > > index ee652ef01534..d6a7960fe716 100644
> > > --- a/drivers/misc/fastrpc.c
> > > +++ b/drivers/misc/fastrpc.c
> > > @@ -106,7 +106,7 @@
> > > #define miscdev_to_fdevice(d) container_of(d, struct fastrpc_device, miscdev)
> > > struct fastrpc_phy_page {
> > > - u64 addr; /* physical address */
> > > + u64 addr; /* physical or dma address */
> > > u64 size; /* size of contiguous region */
> > > };
> > > @@ -171,7 +171,7 @@ struct fastrpc_msg {
> > > u64 ctx; /* invoke caller context */
> > > u32 handle; /* handle to invoke */
> > > u32 sc; /* scalars structure describing the data */
> > > - u64 addr; /* physical address */
> > > + u64 addr; /* physical or dma address */
> >
> > Can you go all the way and make the type dma_addr_t? That way you don't
> > need to typecast the dma_alloc_coherent() and dma_free_coherent().
> >
> > I believe it might complicate the places where you do math on it, but
> > that is a good thing, as it highlights those places where you do
> > something unexpected.
> >
>
> While this not strictly limited to holding a dma_addr_t.
> Based on historical behavior, when the FASTRPC_ATTR_SECUREMAP flag is
> set, S2 mapping expects a physical address to be passed to the
> qcom_scm_assign_mem() API.
> reference-
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/drivers/misc/fastrpc.c?id=e90d911906196bf987492c94e38f10ca611dfd7b
If I understand correctly, when FASTRPC_ATTR_SECUREMAP is set that
implies that the mapping happens in the firmware SID, i.e. outside of
Linux's view. So it's still a dma_addr_t, but it's mapped 1:1 with the
physical address.
I think this is another good reason to make the changes I suggested
below (which you didn't comment on?). Sometimes this "addr" is the
actual address and sometimes it contains the annotated address.
>
> If you suggest, I can define it as dma_addr_t and perform typecasting to u64
> wherever required.
Yes, maintain the mapping in a dma_addr_t, then you have the two cases:
1) Contexts with SMMU, you cast the dma_addr_t and annotate it with the
sid information before communicate the addresses to the firmware.
2) Contexts without SMMU, you make it clear that your dma_addr_t is 1:1
with physical addresses and you then pass it to qcom_scm_assign_mem() et
al.
That way the "addr" is always the actual iova (with or without mapping)
and the places where it's treated as a physical address or an annotated
"address" becomes very explicit.
Regards,
Bjorn
>
> Thanks,
> Pallavi
>
>
> > > u64 size; /* size of contiguous region */
> > > };
> > > @@ -194,7 +194,7 @@ struct fastrpc_buf {
> > > struct dma_buf *dmabuf;
> > > struct device *dev;
> > > void *virt;
> > > - u64 phys;
> > > + u64 dma_addr;
> > > u64 size;
> > > /* Lock for dma buf attachments */
> > > struct mutex lock;
> > > @@ -217,7 +217,7 @@ struct fastrpc_map {
> > > struct dma_buf *buf;
> > > struct sg_table *table;
> > > struct dma_buf_attachment *attach;
> > > - u64 phys;
> > > + u64 dma_addr;
> > > u64 size;
> > > void *va;
> > > u64 len;
> > > @@ -320,11 +320,12 @@ static void fastrpc_free_map(struct kref *ref)
> > > perm.vmid = QCOM_SCM_VMID_HLOS;
> > > perm.perm = QCOM_SCM_PERM_RWX;
> > > - err = qcom_scm_assign_mem(map->phys, map->len,
> > > + err = qcom_scm_assign_mem(map->dma_addr, map->len,
> > > &src_perms, &perm, 1);
> > > if (err) {
> > > - dev_err(map->fl->sctx->dev, "Failed to assign memory phys 0x%llx size 0x%llx err %d\n",
> > > - map->phys, map->len, err);
> > > + dev_err(map->fl->sctx->dev,
> > > + "Failed to assign memory dma_addr 0x%llx size 0x%llx err %d\n",
> > > + map->dma_addr, map->len, err);
> > > return;
> > > }
> > > }
> > > @@ -389,7 +390,7 @@ static int fastrpc_map_lookup(struct fastrpc_user *fl, int fd,
> > > static void fastrpc_buf_free(struct fastrpc_buf *buf)
> > > {
> > > dma_free_coherent(buf->dev, buf->size, buf->virt,
> > > - FASTRPC_PHYS(buf->phys));
> > > + FASTRPC_PHYS(buf->dma_addr));
> > > kfree(buf);
> > > }
> > > @@ -408,12 +409,12 @@ static int __fastrpc_buf_alloc(struct fastrpc_user *fl, struct device *dev,
> > > buf->fl = fl;
> > > buf->virt = NULL;
> > > - buf->phys = 0;
> > > + buf->dma_addr = 0;
> > > buf->size = size;
> > > buf->dev = dev;
> > > buf->raddr = 0;
> > > - buf->virt = dma_alloc_coherent(dev, buf->size, (dma_addr_t *)&buf->phys,
> > > + buf->virt = dma_alloc_coherent(dev, buf->size, (dma_addr_t *)&buf->dma_addr,
> > > GFP_KERNEL);
> > > if (!buf->virt) {
> > > mutex_destroy(&buf->lock);
> > > @@ -439,7 +440,7 @@ static int fastrpc_buf_alloc(struct fastrpc_user *fl, struct device *dev,
> > > buf = *obuf;
> > > if (fl->sctx && fl->sctx->sid)
> > > - buf->phys += ((u64)fl->sctx->sid << 32);
> > > + buf->dma_addr += ((u64)fl->sctx->sid << 32);
> > > return 0;
> > > }
> > > @@ -684,7 +685,7 @@ static int fastrpc_dma_buf_attach(struct dma_buf *dmabuf,
> > > return -ENOMEM;
> > > ret = dma_get_sgtable(buffer->dev, &a->sgt, buffer->virt,
> > > - FASTRPC_PHYS(buffer->phys), buffer->size);
> > > + FASTRPC_PHYS(buffer->dma_addr), buffer->size);
> > > if (ret < 0) {
> > > dev_err(buffer->dev, "failed to get scatterlist from DMA API\n");
> > > kfree(a);
> > > @@ -733,7 +734,7 @@ static int fastrpc_mmap(struct dma_buf *dmabuf,
> > > dma_resv_assert_held(dmabuf->resv);
> > > return dma_mmap_coherent(buf->dev, vma, buf->virt,
> > > - FASTRPC_PHYS(buf->phys), size);
> > > + FASTRPC_PHYS(buf->dma_addr), size);
> >
> > In fact, we invoke dma_alloc_coherent() above to get a dma_addr_t, and
> > then we call map, unmap, and free on the lower 32 bits of that
> > address...
> >
> > In other words, each time we reference dma_addr we have that implicit
> > thing that it's a composit of a dma_addr_t as seen from Linux's point of
> > view (which is matching the addresses in the SMMU page tables) and the
> > adjusted address that we use in communication with the firmware to
> > direct the accesses to the right SID + iova.
> >
> > I think it would be quite nice to make this more explicit throughout the
> > code, rather than juggling the two perspectives in the same variable.
> >
> > Regards,
> > Bjorn
> >
> > > }
> > > static const struct dma_buf_ops fastrpc_dma_buf_ops = {
> > > @@ -785,10 +786,10 @@ static int fastrpc_map_attach(struct fastrpc_user *fl, int fd,
> > > map->table = table;
> > > if (attr & FASTRPC_ATTR_SECUREMAP) {
> > > - map->phys = sg_phys(map->table->sgl);
> > > + map->dma_addr = sg_phys(map->table->sgl);
> > > } else {
> > > - map->phys = sg_dma_address(map->table->sgl);
> > > - map->phys += ((u64)fl->sctx->sid << 32);
> > > + map->dma_addr = sg_dma_address(map->table->sgl);
> > > + map->dma_addr += ((u64)fl->sctx->sid << 32);
> > > }
> > > for_each_sg(map->table->sgl, sgl, map->table->nents,
> > > sgl_index)
> > > @@ -815,10 +816,11 @@ static int fastrpc_map_attach(struct fastrpc_user *fl, int fd,
> > > dst_perms[1].vmid = fl->cctx->vmperms[0].vmid;
> > > dst_perms[1].perm = QCOM_SCM_PERM_RWX;
> > > map->attr = attr;
> > > - err = qcom_scm_assign_mem(map->phys, (u64)map->len, &src_perms, dst_perms, 2);
> > > + err = qcom_scm_assign_mem(map->dma_addr, (u64)map->len, &src_perms, dst_perms, 2);
> > > if (err) {
> > > - dev_err(sess->dev, "Failed to assign memory with phys 0x%llx size 0x%llx err %d\n",
> > > - map->phys, map->len, err);
> > > + dev_err(sess->dev,
> > > + "Failed to assign memory with dma_addr 0x%llx size 0x%llx err %d\n",
> > > + map->dma_addr, map->len, err);
> > > goto map_err;
> > > }
> > > }
> > > @@ -1009,7 +1011,7 @@ static int fastrpc_get_args(u32 kernel, struct fastrpc_invoke_ctx *ctx)
> > > struct vm_area_struct *vma = NULL;
> > > rpra[i].buf.pv = (u64) ctx->args[i].ptr;
> > > - pages[i].addr = ctx->maps[i]->phys;
> > > + pages[i].addr = ctx->maps[i]->dma_addr;
> > > mmap_read_lock(current->mm);
> > > vma = find_vma(current->mm, ctx->args[i].ptr);
> > > @@ -1036,7 +1038,7 @@ static int fastrpc_get_args(u32 kernel, struct fastrpc_invoke_ctx *ctx)
> > > goto bail;
> > > rpra[i].buf.pv = args - ctx->olaps[oix].offset;
> > > - pages[i].addr = ctx->buf->phys -
> > > + pages[i].addr = ctx->buf->dma_addr -
> > > ctx->olaps[oix].offset +
> > > (pkt_size - rlen);
> > > pages[i].addr = pages[i].addr & PAGE_MASK;
> > > @@ -1068,7 +1070,7 @@ static int fastrpc_get_args(u32 kernel, struct fastrpc_invoke_ctx *ctx)
> > > list[i].num = ctx->args[i].length ? 1 : 0;
> > > list[i].pgidx = i;
> > > if (ctx->maps[i]) {
> > > - pages[i].addr = ctx->maps[i]->phys;
> > > + pages[i].addr = ctx->maps[i]->dma_addr;
> > > pages[i].size = ctx->maps[i]->size;
> > > }
> > > rpra[i].dma.fd = ctx->args[i].fd;
> > > @@ -1150,7 +1152,7 @@ static int fastrpc_invoke_send(struct fastrpc_session_ctx *sctx,
> > > msg->ctx = ctx->ctxid | fl->pd;
> > > msg->handle = handle;
> > > msg->sc = ctx->sc;
> > > - msg->addr = ctx->buf ? ctx->buf->phys : 0;
> > > + msg->addr = ctx->buf ? ctx->buf->dma_addr : 0;
> > > msg->size = roundup(ctx->msg_sz, PAGE_SIZE);
> > > fastrpc_context_get(ctx);
> > > @@ -1306,13 +1308,14 @@ static int fastrpc_init_create_static_process(struct fastrpc_user *fl,
> > > if (fl->cctx->vmcount) {
> > > u64 src_perms = BIT(QCOM_SCM_VMID_HLOS);
> > > - err = qcom_scm_assign_mem(fl->cctx->remote_heap->phys,
> > > + err = qcom_scm_assign_mem(fl->cctx->remote_heap->dma_addr,
> > > (u64)fl->cctx->remote_heap->size,
> > > &src_perms,
> > > fl->cctx->vmperms, fl->cctx->vmcount);
> > > if (err) {
> > > - dev_err(fl->sctx->dev, "Failed to assign memory with phys 0x%llx size 0x%llx err %d\n",
> > > - fl->cctx->remote_heap->phys, fl->cctx->remote_heap->size, err);
> > > + dev_err(fl->sctx->dev,
> > > + "Failed to assign memory with dma_addr 0x%llx size 0x%llx err %d\n",
> > > + fl->cctx->remote_heap->dma_addr, fl->cctx->remote_heap->size, err);
> > > goto err_map;
> > > }
> > > scm_done = true;
> > > @@ -1332,7 +1335,7 @@ static int fastrpc_init_create_static_process(struct fastrpc_user *fl,
> > > args[1].length = inbuf.namelen;
> > > args[1].fd = -1;
> > > - pages[0].addr = fl->cctx->remote_heap->phys;
> > > + pages[0].addr = fl->cctx->remote_heap->dma_addr;
> > > pages[0].size = fl->cctx->remote_heap->size;
> > > args[2].ptr = (u64)(uintptr_t) pages;
> > > @@ -1361,12 +1364,12 @@ static int fastrpc_init_create_static_process(struct fastrpc_user *fl,
> > > dst_perms.vmid = QCOM_SCM_VMID_HLOS;
> > > dst_perms.perm = QCOM_SCM_PERM_RWX;
> > > - err = qcom_scm_assign_mem(fl->cctx->remote_heap->phys,
> > > + err = qcom_scm_assign_mem(fl->cctx->remote_heap->dma_addr,
> > > (u64)fl->cctx->remote_heap->size,
> > > &src_perms, &dst_perms, 1);
> > > if (err)
> > > - dev_err(fl->sctx->dev, "Failed to assign memory phys 0x%llx size 0x%llx err %d\n",
> > > - fl->cctx->remote_heap->phys, fl->cctx->remote_heap->size, err);
> > > + dev_err(fl->sctx->dev, "Failed to assign memory dma_addr 0x%llx size 0x%llx err %d\n",
> > > + fl->cctx->remote_heap->dma_addr, fl->cctx->remote_heap->size, err);
> > > }
> > > err_map:
> > > fastrpc_buf_free(fl->cctx->remote_heap);
> > > @@ -1455,7 +1458,7 @@ static int fastrpc_init_create_process(struct fastrpc_user *fl,
> > > args[2].length = inbuf.filelen;
> > > args[2].fd = init.filefd;
> > > - pages[0].addr = imem->phys;
> > > + pages[0].addr = imem->dma_addr;
> > > pages[0].size = imem->size;
> > > args[3].ptr = (u64)(uintptr_t) pages;
> > > @@ -1913,7 +1916,7 @@ static int fastrpc_req_mmap(struct fastrpc_user *fl, char __user *argp)
> > > args[0].ptr = (u64) (uintptr_t) &req_msg;
> > > args[0].length = sizeof(req_msg);
> > > - pages.addr = buf->phys;
> > > + pages.addr = buf->dma_addr;
> > > pages.size = buf->size;
> > > args[1].ptr = (u64) (uintptr_t) &pages;
> > > @@ -1941,11 +1944,12 @@ static int fastrpc_req_mmap(struct fastrpc_user *fl, char __user *argp)
> > > if (req.flags == ADSP_MMAP_REMOTE_HEAP_ADDR && fl->cctx->vmcount) {
> > > u64 src_perms = BIT(QCOM_SCM_VMID_HLOS);
> > > - err = qcom_scm_assign_mem(buf->phys, (u64)buf->size,
> > > + err = qcom_scm_assign_mem(buf->dma_addr, (u64)buf->size,
> > > &src_perms, fl->cctx->vmperms, fl->cctx->vmcount);
> > > if (err) {
> > > - dev_err(fl->sctx->dev, "Failed to assign memory phys 0x%llx size 0x%llx err %d",
> > > - buf->phys, buf->size, err);
> > > + dev_err(fl->sctx->dev,
> > > + "Failed to assign memory dma_addr 0x%llx size 0x%llx err %d",
> > > + buf->dma_addr, buf->size, err);
> > > goto err_assign;
> > > }
> > > }
> > > @@ -2059,7 +2063,7 @@ static int fastrpc_req_mem_map(struct fastrpc_user *fl, char __user *argp)
> > > args[0].ptr = (u64) (uintptr_t) &req_msg;
> > > args[0].length = sizeof(req_msg);
> > > - pages.addr = map->phys;
> > > + pages.addr = map->dma_addr;
> > > pages.size = map->len;
> > > args[1].ptr = (u64) (uintptr_t) &pages;
> > > --
> > > 2.34.1
> > >
> > >
>
On Mon, Nov 17, 2025 at 12:37:33PM +0530, Kumari Pallavi wrote:
>
>
> On 11/14/2025 9:14 PM, Bjorn Andersson wrote:
> > On Fri, Nov 14, 2025 at 02:11:40PM +0530, Kumari Pallavi wrote:
> > > Update all references of buf->phys and map->phys to buf->dma_addr and
> > > map->dma_addr to accurately represent that these fields store DMA
> > > addresses, not physical addresses. This change improves code clarity
> > > and aligns with kernel conventions for dma_addr_t usage.
> > >
> > > Signed-off-by: Kumari Pallavi <kumari.pallavi@oss.qualcomm.com>
> > > ---
> > > drivers/misc/fastrpc.c | 76 ++++++++++++++++++++++--------------------
> > > 1 file changed, 40 insertions(+), 36 deletions(-)
> > >
> > > diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
> > > index ee652ef01534..d6a7960fe716 100644
> > > --- a/drivers/misc/fastrpc.c
> > > +++ b/drivers/misc/fastrpc.c
> > > @@ -106,7 +106,7 @@
> > > #define miscdev_to_fdevice(d) container_of(d, struct fastrpc_device, miscdev)
> > > struct fastrpc_phy_page {
> > > - u64 addr; /* physical address */
> > > + u64 addr; /* physical or dma address */
> > > u64 size; /* size of contiguous region */
> > > };
> > > @@ -171,7 +171,7 @@ struct fastrpc_msg {
> > > u64 ctx; /* invoke caller context */
> > > u32 handle; /* handle to invoke */
> > > u32 sc; /* scalars structure describing the data */
> > > - u64 addr; /* physical address */
> > > + u64 addr; /* physical or dma address */
> >
> > Can you go all the way and make the type dma_addr_t? That way you don't
> > need to typecast the dma_alloc_coherent() and dma_free_coherent().
> >
> > I believe it might complicate the places where you do math on it, but
> > that is a good thing, as it highlights those places where you do
> > something unexpected.
> >
>
> While this not strictly limited to holding a dma_addr_t.
> Based on historical behavior, when the FASTRPC_ATTR_SECUREMAP flag is
> set, S2 mapping expects a physical address to be passed to the
> qcom_scm_assign_mem() API.
> reference-
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/drivers/misc/fastrpc.c?id=e90d911906196bf987492c94e38f10ca611dfd7b
>
> If you suggest, I can define it as dma_addr_t and perform typecasting to u64
> wherever required.
You don't need to typecase dma_addr_t when passing u64.
--
With best wishes
Dmitry
On 11/17/25 12:22 PM, Dmitry Baryshkov wrote:
> On Mon, Nov 17, 2025 at 12:37:33PM +0530, Kumari Pallavi wrote:
>>
>>
>> On 11/14/2025 9:14 PM, Bjorn Andersson wrote:
>>> On Fri, Nov 14, 2025 at 02:11:40PM +0530, Kumari Pallavi wrote:
>>>> Update all references of buf->phys and map->phys to buf->dma_addr and
>>>> map->dma_addr to accurately represent that these fields store DMA
>>>> addresses, not physical addresses. This change improves code clarity
>>>> and aligns with kernel conventions for dma_addr_t usage.
>>>>
>>>> Signed-off-by: Kumari Pallavi <kumari.pallavi@oss.qualcomm.com>
>>>> ---
>>>> drivers/misc/fastrpc.c | 76 ++++++++++++++++++++++--------------------
>>>> 1 file changed, 40 insertions(+), 36 deletions(-)
>>>>
>>>> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
>>>> index ee652ef01534..d6a7960fe716 100644
>>>> --- a/drivers/misc/fastrpc.c
>>>> +++ b/drivers/misc/fastrpc.c
>>>> @@ -106,7 +106,7 @@
>>>> #define miscdev_to_fdevice(d) container_of(d, struct fastrpc_device, miscdev)
>>>> struct fastrpc_phy_page {
>>>> - u64 addr; /* physical address */
>>>> + u64 addr; /* physical or dma address */
>>>> u64 size; /* size of contiguous region */
>>>> };
>>>> @@ -171,7 +171,7 @@ struct fastrpc_msg {
>>>> u64 ctx; /* invoke caller context */
>>>> u32 handle; /* handle to invoke */
>>>> u32 sc; /* scalars structure describing the data */
>>>> - u64 addr; /* physical address */
>>>> + u64 addr; /* physical or dma address */
>>>
>>> Can you go all the way and make the type dma_addr_t? That way you don't
>>> need to typecast the dma_alloc_coherent() and dma_free_coherent().
>>>
>>> I believe it might complicate the places where you do math on it, but
>>> that is a good thing, as it highlights those places where you do
>>> something unexpected.
>>>
>>
>> While this not strictly limited to holding a dma_addr_t.
>> Based on historical behavior, when the FASTRPC_ATTR_SECUREMAP flag is
>> set, S2 mapping expects a physical address to be passed to the
>> qcom_scm_assign_mem() API.
>> reference-
>> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/drivers/misc/fastrpc.c?id=e90d911906196bf987492c94e38f10ca611dfd7b
>>
>> If you suggest, I can define it as dma_addr_t and perform typecasting to u64
>> wherever required.
>
> You don't need to typecase dma_addr_t when passing u64.
*on arm anyway
Konrad
On Fri, Nov 14, 2025 at 02:11:40PM +0530, Kumari Pallavi wrote:
> Update all references of buf->phys and map->phys to buf->dma_addr and
> map->dma_addr to accurately represent that these fields store DMA
> addresses, not physical addresses. This change improves code clarity
> and aligns with kernel conventions for dma_addr_t usage.
Why do you mention dma_addr_t here?
>
> Signed-off-by: Kumari Pallavi <kumari.pallavi@oss.qualcomm.com>
> ---
> drivers/misc/fastrpc.c | 76 ++++++++++++++++++++++--------------------
> 1 file changed, 40 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
> index ee652ef01534..d6a7960fe716 100644
> --- a/drivers/misc/fastrpc.c
> +++ b/drivers/misc/fastrpc.c
> @@ -106,7 +106,7 @@
> #define miscdev_to_fdevice(d) container_of(d, struct fastrpc_device, miscdev)
>
> struct fastrpc_phy_page {
> - u64 addr; /* physical address */
> + u64 addr; /* physical or dma address */
What is the difference here? Aren't all of them DMA addresses?
> u64 size; /* size of contiguous region */
> };
>
> @@ -171,7 +171,7 @@ struct fastrpc_msg {
> u64 ctx; /* invoke caller context */
> u32 handle; /* handle to invoke */
> u32 sc; /* scalars structure describing the data */
> - u64 addr; /* physical address */
> + u64 addr; /* physical or dma address */
> u64 size; /* size of contiguous region */
> };
>
> @@ -194,7 +194,7 @@ struct fastrpc_buf {
> struct dma_buf *dmabuf;
> struct device *dev;
> void *virt;
> - u64 phys;
> + u64 dma_addr;
> u64 size;
> /* Lock for dma buf attachments */
> struct mutex lock;
> @@ -217,7 +217,7 @@ struct fastrpc_map {
> struct dma_buf *buf;
> struct sg_table *table;
> struct dma_buf_attachment *attach;
> - u64 phys;
> + u64 dma_addr;
> u64 size;
> void *va;
> u64 len;
> @@ -320,11 +320,12 @@ static void fastrpc_free_map(struct kref *ref)
>
> perm.vmid = QCOM_SCM_VMID_HLOS;
> perm.perm = QCOM_SCM_PERM_RWX;
> - err = qcom_scm_assign_mem(map->phys, map->len,
> + err = qcom_scm_assign_mem(map->dma_addr, map->len,
> &src_perms, &perm, 1);
> if (err) {
> - dev_err(map->fl->sctx->dev, "Failed to assign memory phys 0x%llx size 0x%llx err %d\n",
> - map->phys, map->len, err);
> + dev_err(map->fl->sctx->dev,
> + "Failed to assign memory dma_addr 0x%llx size 0x%llx err %d\n",
> + map->dma_addr, map->len, err);
> return;
> }
> }
> @@ -389,7 +390,7 @@ static int fastrpc_map_lookup(struct fastrpc_user *fl, int fd,
> static void fastrpc_buf_free(struct fastrpc_buf *buf)
> {
> dma_free_coherent(buf->dev, buf->size, buf->virt,
> - FASTRPC_PHYS(buf->phys));
> + FASTRPC_PHYS(buf->dma_addr));
> kfree(buf);
> }
>
> @@ -408,12 +409,12 @@ static int __fastrpc_buf_alloc(struct fastrpc_user *fl, struct device *dev,
>
> buf->fl = fl;
> buf->virt = NULL;
> - buf->phys = 0;
> + buf->dma_addr = 0;
> buf->size = size;
> buf->dev = dev;
> buf->raddr = 0;
>
> - buf->virt = dma_alloc_coherent(dev, buf->size, (dma_addr_t *)&buf->phys,
> + buf->virt = dma_alloc_coherent(dev, buf->size, (dma_addr_t *)&buf->dma_addr,
> GFP_KERNEL);
> if (!buf->virt) {
> mutex_destroy(&buf->lock);
> @@ -439,7 +440,7 @@ static int fastrpc_buf_alloc(struct fastrpc_user *fl, struct device *dev,
> buf = *obuf;
>
> if (fl->sctx && fl->sctx->sid)
> - buf->phys += ((u64)fl->sctx->sid << 32);
> + buf->dma_addr += ((u64)fl->sctx->sid << 32);
>
> return 0;
> }
> @@ -684,7 +685,7 @@ static int fastrpc_dma_buf_attach(struct dma_buf *dmabuf,
> return -ENOMEM;
>
> ret = dma_get_sgtable(buffer->dev, &a->sgt, buffer->virt,
> - FASTRPC_PHYS(buffer->phys), buffer->size);
> + FASTRPC_PHYS(buffer->dma_addr), buffer->size);
> if (ret < 0) {
> dev_err(buffer->dev, "failed to get scatterlist from DMA API\n");
> kfree(a);
> @@ -733,7 +734,7 @@ static int fastrpc_mmap(struct dma_buf *dmabuf,
> dma_resv_assert_held(dmabuf->resv);
>
> return dma_mmap_coherent(buf->dev, vma, buf->virt,
> - FASTRPC_PHYS(buf->phys), size);
> + FASTRPC_PHYS(buf->dma_addr), size);
> }
>
> static const struct dma_buf_ops fastrpc_dma_buf_ops = {
> @@ -785,10 +786,10 @@ static int fastrpc_map_attach(struct fastrpc_user *fl, int fd,
> map->table = table;
>
> if (attr & FASTRPC_ATTR_SECUREMAP) {
> - map->phys = sg_phys(map->table->sgl);
> + map->dma_addr = sg_phys(map->table->sgl);
> } else {
> - map->phys = sg_dma_address(map->table->sgl);
> - map->phys += ((u64)fl->sctx->sid << 32);
> + map->dma_addr = sg_dma_address(map->table->sgl);
> + map->dma_addr += ((u64)fl->sctx->sid << 32);
> }
> for_each_sg(map->table->sgl, sgl, map->table->nents,
> sgl_index)
> @@ -815,10 +816,11 @@ static int fastrpc_map_attach(struct fastrpc_user *fl, int fd,
> dst_perms[1].vmid = fl->cctx->vmperms[0].vmid;
> dst_perms[1].perm = QCOM_SCM_PERM_RWX;
> map->attr = attr;
> - err = qcom_scm_assign_mem(map->phys, (u64)map->len, &src_perms, dst_perms, 2);
> + err = qcom_scm_assign_mem(map->dma_addr, (u64)map->len, &src_perms, dst_perms, 2);
> if (err) {
> - dev_err(sess->dev, "Failed to assign memory with phys 0x%llx size 0x%llx err %d\n",
> - map->phys, map->len, err);
> + dev_err(sess->dev,
> + "Failed to assign memory with dma_addr 0x%llx size 0x%llx err %d\n",
> + map->dma_addr, map->len, err);
> goto map_err;
> }
> }
> @@ -1009,7 +1011,7 @@ static int fastrpc_get_args(u32 kernel, struct fastrpc_invoke_ctx *ctx)
> struct vm_area_struct *vma = NULL;
>
> rpra[i].buf.pv = (u64) ctx->args[i].ptr;
> - pages[i].addr = ctx->maps[i]->phys;
> + pages[i].addr = ctx->maps[i]->dma_addr;
>
> mmap_read_lock(current->mm);
> vma = find_vma(current->mm, ctx->args[i].ptr);
> @@ -1036,7 +1038,7 @@ static int fastrpc_get_args(u32 kernel, struct fastrpc_invoke_ctx *ctx)
> goto bail;
>
> rpra[i].buf.pv = args - ctx->olaps[oix].offset;
> - pages[i].addr = ctx->buf->phys -
> + pages[i].addr = ctx->buf->dma_addr -
> ctx->olaps[oix].offset +
> (pkt_size - rlen);
> pages[i].addr = pages[i].addr & PAGE_MASK;
> @@ -1068,7 +1070,7 @@ static int fastrpc_get_args(u32 kernel, struct fastrpc_invoke_ctx *ctx)
> list[i].num = ctx->args[i].length ? 1 : 0;
> list[i].pgidx = i;
> if (ctx->maps[i]) {
> - pages[i].addr = ctx->maps[i]->phys;
> + pages[i].addr = ctx->maps[i]->dma_addr;
> pages[i].size = ctx->maps[i]->size;
> }
> rpra[i].dma.fd = ctx->args[i].fd;
> @@ -1150,7 +1152,7 @@ static int fastrpc_invoke_send(struct fastrpc_session_ctx *sctx,
> msg->ctx = ctx->ctxid | fl->pd;
> msg->handle = handle;
> msg->sc = ctx->sc;
> - msg->addr = ctx->buf ? ctx->buf->phys : 0;
> + msg->addr = ctx->buf ? ctx->buf->dma_addr : 0;
> msg->size = roundup(ctx->msg_sz, PAGE_SIZE);
> fastrpc_context_get(ctx);
>
> @@ -1306,13 +1308,14 @@ static int fastrpc_init_create_static_process(struct fastrpc_user *fl,
> if (fl->cctx->vmcount) {
> u64 src_perms = BIT(QCOM_SCM_VMID_HLOS);
>
> - err = qcom_scm_assign_mem(fl->cctx->remote_heap->phys,
> + err = qcom_scm_assign_mem(fl->cctx->remote_heap->dma_addr,
> (u64)fl->cctx->remote_heap->size,
> &src_perms,
> fl->cctx->vmperms, fl->cctx->vmcount);
> if (err) {
> - dev_err(fl->sctx->dev, "Failed to assign memory with phys 0x%llx size 0x%llx err %d\n",
> - fl->cctx->remote_heap->phys, fl->cctx->remote_heap->size, err);
> + dev_err(fl->sctx->dev,
> + "Failed to assign memory with dma_addr 0x%llx size 0x%llx err %d\n",
> + fl->cctx->remote_heap->dma_addr, fl->cctx->remote_heap->size, err);
> goto err_map;
> }
> scm_done = true;
> @@ -1332,7 +1335,7 @@ static int fastrpc_init_create_static_process(struct fastrpc_user *fl,
> args[1].length = inbuf.namelen;
> args[1].fd = -1;
>
> - pages[0].addr = fl->cctx->remote_heap->phys;
> + pages[0].addr = fl->cctx->remote_heap->dma_addr;
> pages[0].size = fl->cctx->remote_heap->size;
>
> args[2].ptr = (u64)(uintptr_t) pages;
> @@ -1361,12 +1364,12 @@ static int fastrpc_init_create_static_process(struct fastrpc_user *fl,
>
> dst_perms.vmid = QCOM_SCM_VMID_HLOS;
> dst_perms.perm = QCOM_SCM_PERM_RWX;
> - err = qcom_scm_assign_mem(fl->cctx->remote_heap->phys,
> + err = qcom_scm_assign_mem(fl->cctx->remote_heap->dma_addr,
> (u64)fl->cctx->remote_heap->size,
> &src_perms, &dst_perms, 1);
> if (err)
> - dev_err(fl->sctx->dev, "Failed to assign memory phys 0x%llx size 0x%llx err %d\n",
> - fl->cctx->remote_heap->phys, fl->cctx->remote_heap->size, err);
> + dev_err(fl->sctx->dev, "Failed to assign memory dma_addr 0x%llx size 0x%llx err %d\n",
> + fl->cctx->remote_heap->dma_addr, fl->cctx->remote_heap->size, err);
> }
> err_map:
> fastrpc_buf_free(fl->cctx->remote_heap);
> @@ -1455,7 +1458,7 @@ static int fastrpc_init_create_process(struct fastrpc_user *fl,
> args[2].length = inbuf.filelen;
> args[2].fd = init.filefd;
>
> - pages[0].addr = imem->phys;
> + pages[0].addr = imem->dma_addr;
> pages[0].size = imem->size;
>
> args[3].ptr = (u64)(uintptr_t) pages;
> @@ -1913,7 +1916,7 @@ static int fastrpc_req_mmap(struct fastrpc_user *fl, char __user *argp)
> args[0].ptr = (u64) (uintptr_t) &req_msg;
> args[0].length = sizeof(req_msg);
>
> - pages.addr = buf->phys;
> + pages.addr = buf->dma_addr;
> pages.size = buf->size;
>
> args[1].ptr = (u64) (uintptr_t) &pages;
> @@ -1941,11 +1944,12 @@ static int fastrpc_req_mmap(struct fastrpc_user *fl, char __user *argp)
> if (req.flags == ADSP_MMAP_REMOTE_HEAP_ADDR && fl->cctx->vmcount) {
> u64 src_perms = BIT(QCOM_SCM_VMID_HLOS);
>
> - err = qcom_scm_assign_mem(buf->phys, (u64)buf->size,
> + err = qcom_scm_assign_mem(buf->dma_addr, (u64)buf->size,
> &src_perms, fl->cctx->vmperms, fl->cctx->vmcount);
> if (err) {
> - dev_err(fl->sctx->dev, "Failed to assign memory phys 0x%llx size 0x%llx err %d",
> - buf->phys, buf->size, err);
> + dev_err(fl->sctx->dev,
> + "Failed to assign memory dma_addr 0x%llx size 0x%llx err %d",
> + buf->dma_addr, buf->size, err);
> goto err_assign;
> }
> }
> @@ -2059,7 +2063,7 @@ static int fastrpc_req_mem_map(struct fastrpc_user *fl, char __user *argp)
> args[0].ptr = (u64) (uintptr_t) &req_msg;
> args[0].length = sizeof(req_msg);
>
> - pages.addr = map->phys;
> + pages.addr = map->dma_addr;
> pages.size = map->len;
>
> args[1].ptr = (u64) (uintptr_t) &pages;
> --
> 2.34.1
>
--
With best wishes
Dmitry
On 11/14/2025 5:45 PM, Dmitry Baryshkov wrote:
>> Update all references of buf->phys and map->phys to buf->dma_addr and
>> map->dma_addr to accurately represent that these fields store DMA
>> addresses, not physical addresses. This change improves code clarity
>> and aligns with kernel conventions for dma_addr_t usage.
> Why do you mention dma_addr_t here?
>
I mentioned dma_addr_t in the commit message because of the earlier
feedback highlighted about the confusing use of phys for fields that
actually store an IOVA-like address ('phys' with something more fitting
for an IOVA or DMA address).
https://lore.kernel.org/all/969bdb49-0682-4345-98f7-523404bb4213@app.fastmail.com/
>> Signed-off-by: Kumari Pallavi<kumari.pallavi@oss.qualcomm.com>
>> ---
>> drivers/misc/fastrpc.c | 76 ++++++++++++++++++++++--------------------
>> 1 file changed, 40 insertions(+), 36 deletions(-)
>>
>> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
>> index ee652ef01534..d6a7960fe716 100644
>> --- a/drivers/misc/fastrpc.c
>> +++ b/drivers/misc/fastrpc.c
>> @@ -106,7 +106,7 @@
>> #define miscdev_to_fdevice(d) container_of(d, struct fastrpc_device, miscdev)
>>
>> struct fastrpc_phy_page {
>> - u64 addr; /* physical address */
>> + u64 addr; /* physical or dma address */
> What is the difference here? Aren't all of them DMA addresses?
Yes, correct—both represent DMA addresses, just typed differently
depending on whether it originate from a physical or DMA mapping context.
ACK, I will update this in the next patch series.
Thanks,
Pallavi
© 2016 - 2026 Red Hat, Inc.