Re: [PATCH net-next v5 2/2] net: wwan: qcom_bam_dmux: Assign restricted DMA pool to remote processor

[Vishnu Santhosh <vishnu.santhosh@oss.qualcomm.com>]

On 06-11-2026 01:59 pm, Loic Poulain wrote:
> On Fri, Nov 6, 2026 at 5:36 AM Vishnu Santhosh
> <vishnu.santhosh@oss.qualcomm.com> wrote:
>>
>> On 05-11-2026 07:51 pm, Loic Poulain wrote:
>>> On Thu, Nov 5, 2026 at 11:42 AM Vishnu Santhosh
>>> <vishnu.santhosh@oss.qualcomm.com> wrote:
>>>> Some Qualcomm SoCs, such as Shikra, run the modem in a separate security
>>>> domain with restricted access to system memory. BAM-DMUX DMA mappings
>>>> and BAM descriptor FIFOs must therefore be constrained to a designated
>>>> memory region that can be shared with the modem. Accesses outside that
>>>> region can trigger an XPU violation.
>>>>
>>>> When qcom,vmid is present, require a restricted DMA pool and assign the
>>>> entire pool to both the local and remote execution environments before
>>>> requesting the DMA channels.
>>>>
>>>> This ensures that BAM-DMUX mappings are within the assigned region. When
>>>> the BAM DMA controller references the same pool, its descriptor FIFOs
>>>> are covered by the assignment as well.
>>>>
>>>> Track the assigned execution environments explicitly and reclaim the
>>>> pool for the local environment after DMA channels and mappings are
>>>> released during remove or probe error cleanup. Skip the reclaim and
>>>> leave the pool assigned if the remote side still holds its power vote,
>>>> since it may still access the pool.
>>>>
>>>> Fail probing when restricted DMA pool support is unavailable, when the
>>>> VMID property is malformed, when memory-region is missing, or when the
>>>> restricted DMA pool is not attached to the device.
>>>>
>>>> Co-developed-by: Deepak Kumar Singh <deepak.singh@oss.qualcomm.com>
>>>> Signed-off-by: Deepak Kumar Singh <deepak.singh@oss.qualcomm.com>
>>>> Signed-off-by: Vishnu Santhosh <vishnu.santhosh@oss.qualcomm.com>
>>>> ---
>>>>    drivers/net/wwan/Kconfig         |   1 +
>>>>    drivers/net/wwan/qcom_bam_dmux.c | 108 ++++++++++++++++++++++++++++++++++++++-
>>>>    2 files changed, 107 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/net/wwan/Kconfig b/drivers/net/wwan/Kconfig
>>>> index 958dbc7347fa84ee869439bf8b503037faab8bef..1b133c56231615269698140187ca3141dfe48dbf 100644
>>>> --- a/drivers/net/wwan/Kconfig
>>>> +++ b/drivers/net/wwan/Kconfig
>>>> @@ -65,6 +65,7 @@ config MHI_WWAN_MBIM
>>>>    config QCOM_BAM_DMUX
>>>>           tristate "Qualcomm BAM-DMUX WWAN network driver"
>>>>           depends on (DMA_ENGINE && PM && QCOM_SMEM_STATE) || COMPILE_TEST
>>>> +       select QCOM_SCM
>>>>           help
>>>>             The BAM Data Multiplexer provides access to the network data channels
>>>>             of modems integrated into many older Qualcomm SoCs, e.g. Qualcomm
>>>> diff --git a/drivers/net/wwan/qcom_bam_dmux.c b/drivers/net/wwan/qcom_bam_dmux.c
>>>> index cc6ace8d64371eb8d00c638a39b234ee540b83c9..0e7ad62f8b4e059264e89837f296ee13c0c31c9b 100644
>>>> --- a/drivers/net/wwan/qcom_bam_dmux.c
>>>> +++ b/drivers/net/wwan/qcom_bam_dmux.c
>>>> @@ -9,14 +9,17 @@
>>>>    #include <linux/completion.h>
>>>>    #include <linux/dma-mapping.h>
>>>>    #include <linux/dmaengine.h>
>>>> +#include <linux/firmware/qcom/qcom_scm.h>
>>>>    #include <linux/if_arp.h>
>>>>    #include <linux/interrupt.h>
>>>>    #include <linux/module.h>
>>>>    #include <linux/netdevice.h>
>>>> +#include <linux/of_reserved_mem.h>
>>>>    #include <linux/platform_device.h>
>>>>    #include <linux/pm_runtime.h>
>>>>    #include <linux/soc/qcom/smem_state.h>
>>>>    #include <linux/spinlock.h>
>>>> +#include <linux/swiotlb.h>
>>>>    #include <linux/wait.h>
>>>>    #include <linux/workqueue.h>
>>>>    #include <net/pkt_sched.h>
>>>> @@ -75,6 +78,9 @@ struct bam_dmux {
>>>>           struct completion pc_ack_completion;
>>>>
>>>>           struct dma_chan *rx, *tx;
>>>> +       phys_addr_t dma_pool_base;
>>>> +       size_t dma_pool_size;
>>>> +       u64 dma_pool_perms;
>>>>           struct bam_dmux_skb_dma rx_skbs[BAM_DMUX_NUM_SKB];
>>>>           struct bam_dmux_skb_dma tx_skbs[BAM_DMUX_NUM_SKB];
>>>>           spinlock_t tx_lock; /* Protect tx_skbs, tx_next_skb */
>>>> @@ -762,6 +768,90 @@ static int __maybe_unused bam_dmux_runtime_resume(struct device *dev)
>>>>           return 0;
>>>>    }
>>>>
>>>> +static int bam_dmux_assign_dma_pool(struct bam_dmux *dmux)
>>>> +{
>>>> +       struct device *dev = dmux->dev;
>>>> +       struct device_node *rmem_np;
>>>> +       struct reserved_mem *rmem;
>>>> +       struct qcom_scm_vmperm dst[2];
>>>> +       u64 src = BIT_ULL(QCOM_SCM_VMID_HLOS);
>>>> +       u32 vmid;
>>>> +       int ret;
>>>> +
>>>> +       if (!of_property_present(dev->of_node, "qcom,vmid"))
>>>> +               return 0;
>>>> +
>>>> +       ret = of_property_read_u32(dev->of_node, "qcom,vmid", &vmid);
>>>> +       if (ret)
>>>> +               return dev_err_probe(dev, ret, "Failed to read qcom,vmid\n");
>>>> +       if (vmid == QCOM_SCM_VMID_HLOS || vmid >= BITS_PER_TYPE(u64))
>>>> +               return dev_err_probe(dev, -EINVAL, "Invalid qcom,vmid %u\n", vmid);
>>>> +
>>>> +       if (!IS_ENABLED(CONFIG_DMA_RESTRICTED_POOL))
>>>> +               return dev_err_probe(dev, -EOPNOTSUPP,
>>>> +                                    "qcom,vmid requires DMA_RESTRICTED_POOL\n");
>>>> +
>>>> +       rmem_np = of_parse_phandle(dev->of_node, "memory-region", 0);
>>>> +       if (!rmem_np)
>>>> +               return dev_err_probe(dev, -EINVAL,
>>>> +                                            "qcom,vmid requires memory-region\n");
>>>> +
>>>> +       rmem = of_reserved_mem_lookup(rmem_np);
>>>> +       of_node_put(rmem_np);
>>>> +       if (!rmem)
>>>> +               return dev_err_probe(dev, -EINVAL,
>>>> +                                            "Failed to look up restricted DMA pool\n");
>>>> +
>>>> +       if (!is_swiotlb_for_alloc(dev))
>>>> +               return dev_err_probe(dev, -EINVAL,
>>>> +                                    "Restricted DMA pool not attached\n");
>>>> +
>>>> +       if (!qcom_scm_is_available())
>>>> +               return -EPROBE_DEFER;
>>>> +
>>>> +       dst[0].vmid = QCOM_SCM_VMID_HLOS;
>>>> +       dst[0].perm = QCOM_SCM_PERM_RW;
>>>> +       dst[1].vmid = vmid;
>>>> +       dst[1].perm = QCOM_SCM_PERM_RW;
>>>> +
>>>> +       ret = qcom_scm_assign_mem(rmem->base, rmem->size, &src, dst,
>>>> +                                 ARRAY_SIZE(dst));
>>>> +       if (ret)
>>>> +               return dev_err_probe(dev, ret,
>>>> +                                    "SCM assign restricted DMA pool failed\n");
>>>> +
>>>> +       dmux->dma_pool_base = rmem->base;
>>>> +       dmux->dma_pool_size = rmem->size;
>>>> +       /* Track the destination VMIDs explicitly for the reclaim operation. */
>>>> +       dmux->dma_pool_perms = BIT_ULL(QCOM_SCM_VMID_HLOS) | BIT_ULL(vmid);
>>>> +
>>>> +       return 0;
>>>> +}
>>>> +
>>>> +static void bam_dmux_reclaim_dma_pool(struct bam_dmux *dmux)
>>>> +{
>>>> +       struct qcom_scm_vmperm hlos = {
>>>> +               .vmid = QCOM_SCM_VMID_HLOS,
>>>> +               .perm = QCOM_SCM_PERM_RW,
>>>> +       };
>>>> +       u64 src = dmux->dma_pool_perms;
>>>> +       int ret;
>>>> +
>>>> +       if (!dmux->dma_pool_perms)
>>>> +               return;
>>>> +
>>>> +       ret = qcom_scm_assign_mem(dmux->dma_pool_base, dmux->dma_pool_size, &src,
>>>> +                                 &hlos, 1);
>>>> +       if (ret) {
>>>> +               dev_err(dmux->dev, "SCM reclaim restricted DMA pool failed: %d\n", ret);
>>>> +               return;
>>>> +       }
>>>> +
>>>> +       dmux->dma_pool_perms = 0;
>>>> +}
>>>> +
>>>> +static void bam_dmux_remove(struct platform_device *pdev);
>>>> +
>>>>    static int bam_dmux_probe(struct platform_device *pdev)
>>>>    {
>>>>           struct device *dev = &pdev->dev;
>>>> @@ -796,6 +886,10 @@ static int bam_dmux_probe(struct platform_device *pdev)
>>>>                                        "Failed to get pc-ack state\n");
>>>>           dmux->pc_ack_mask = BIT(bit);
>>>>
>>>> +       ret = bam_dmux_assign_dma_pool(dmux);
>>>> +       if (ret)
>>>> +               return ret;
>>>> +
>>>>           init_waitqueue_head(&dmux->pc_wait);
>>>>           init_completion(&dmux->pc_ack_completion);
>>>>           complete_all(&dmux->pc_ack_completion);
>>>> @@ -829,8 +923,11 @@ static int bam_dmux_probe(struct platform_device *pdev)
>>>>
>>>>           ret = irq_get_irqchip_state(dmux->pc_irq, IRQCHIP_STATE_LINE_LEVEL,
>>>>                                       &dmux->pc_state);
>>>> -       if (ret)
>>>> -               goto err_disable_pm;
>>>> +       if (ret) {
>>>> +               /* The PC IRQ may already have powered up the RX path */
>>>> +               bam_dmux_remove(pdev);
>>> it's still unclear in the comment and commit message why we can't go
>>> with err_disable_pm?
>> Agreed. I plan to update the comment in the next revision to:
>>
>> /*
>>    * The PC IRQ is already enabled and may have powered up the RX
>>    * path. Use the full remove() teardown so that RX DMA is stopped
>>    * before the pool is reclaimed.
>>    */
>>
>> and add to the commit message:
>>
>> If probe fails after the PC IRQ has been requested, clean up through
>> bam_dmux_remove(), since the IRQ may already have powered up the RX
>> path, which must be torn down before the pool is reclaimed.
>>
>> Does this look fine to you?
> That looks good, yes. Also, could this be avoided by requesting the
> IRQ with IRQF_NO_AUTOEN and only enabling it at the end of the probe
> sequence?

Yes, that is simpler. The PC IRQ then cannot run before probe enables
it, so an irq_get_irqchip_state() failure can use err_disable_pm and
the remove() call and its forward declaration go away.

For v6:

ret = devm_request_threaded_irq(dev, dmux->pc_irq, NULL,
            bam_dmux_pc_irq,
            IRQF_ONESHOT | IRQF_NO_AUTOEN,
            NULL, dmux);

if (ret)
     goto err_disable_pm;

[...]

enable_irq(dmux->pc_irq);

return 0;

Does this look fine?


Thanks,
Vishnu

>>>> +               return ret;
>>>> +       }
>>>>
>>>>           /* Check if remote finished initialization before us */
>>>>           if (dmux->pc_state) {
>>>> @@ -845,6 +942,7 @@ static int bam_dmux_probe(struct platform_device *pdev)
>>>>    err_disable_pm:
>>>>           pm_runtime_disable(dev);
>>>>           pm_runtime_dont_use_autosuspend(dev);
>>>> +       bam_dmux_reclaim_dma_pool(dmux);
>>>>           return ret;
>>>>    }
>>>>
>>>> @@ -879,6 +977,12 @@ static void bam_dmux_remove(struct platform_device *pdev)
>>>>           disable_irq(dmux->pc_irq);
>>>>           bam_dmux_power_off(dmux);
>>>>           bam_dmux_free_skbs(dmux->tx_skbs, DMA_TO_DEVICE);
>>>> +
>>>> +       /* The remote side may still access the pool if it holds its power vote */
>>>> +       if (!dmux->pc_state)
>>>> +               bam_dmux_reclaim_dma_pool(dmux);
>>>> +       else if (dmux->dma_pool_perms)
>>>> +               dev_warn(dev, "Leaving restricted DMA pool assigned to remote\n");
>>>>    }
>>>>
>>>>    static const struct dev_pm_ops bam_dmux_pm_ops = {
>>>>
>>>> --
>>>> 2.34.1
>>>>