> > > > > > > > > Subject: Re: [EXTERNAL] Re: [PATCH] virtio: vdpa: vDPA
> > > > > > > > > driver for Marvell OCTEON DPU devices
> > > > > > > > >
> > > > > > > > > On Wed, Apr 10, 2024 at 10:15:37AM +0000, Srujana Challa
> wrote:
> > > > > > > > > > > > > > +
> > > > > > > > > > > > > > + domain = iommu_get_domain_for_dev(dev);
> > > > > > > > > > > > > > + if (!domain || domain->type ==
> > > > > > > > > > > > > > + IOMMU_DOMAIN_IDENTITY)
> > > > > > > {
> > > > > > > > > > > > > > + dev_info(dev, "NO-IOMMU\n");
> > > > > > > > > > > > > > + octep_vdpa_ops.set_map =
> > > > > > > > > > > > > > + octep_vdpa_set_map;
> > > > > > > > > > > > >
> > > > > > > > > > > > > Is this a shortcut to have get better performance?
> > > > > > > > > > > > > DMA API should have those greacefully I think.
> > > > > > > > > > > > When IOMMU is disabled on host and set_map/dma_map
> > > > > > > > > > > > is not set, vhost-vdpa is reporting an error
> > > > > > > > > > > > "Failed to allocate domain, device is not
> > > > > > > > > > > IOMMU cache coherent capable\n".
> > > > > > > > > > > > Hence we are doing this way to get better performance.
> > > > > > > > > > >
> > > > > > > > > > > The problem is, assuming the device does not have
> > > > > > > > > > > any internal
> > > > > > > IOMMU.
> > > > > > > > > > >
> > > > > > > > > > > 1) If we allow it running without IOMMU, it opens a
> > > > > > > > > > > window for guest to attack the host.
> > > > > > > > > > > 2) If you see perforamnce issue with
> > > > > > > > > > > IOMMU_DOMAIN_IDENTITY, let's report it to DMA/IOMMU
> > > > > > > > > > > maintiner to fix that
> > > > > > > > > > It will be helpful for host networking case when iommu is
> disabled.
> > > > > > > > > > Can we take the vfio pci driver approach as a
> > > > > > > > > > reference where user explicitly set
> "enable_unsafe_noiommu_mode"
> > > > > > > > > > using module
> > > > > param?
> > > > > > > > >
> > > > > > > > > vfio is a userspace driver so it's userspace's responsibility.
> > > > > > > > > what exactly ensures correctness here? does the device
> > > > > > > > > have an on-chip iommu?
> > > > > > > > >
> > > > > > > > Our device features an on-chip IOMMU, although it is not
> > > > > > > > utilized for host-side targeted DMA operations. We
> > > > > > > > included no-iommu mode in our driver to ensure that host
> > > > > > > > applications, such as DPDK Virtio user PMD, continue to
> > > > > > > > function even when operating in a no-
> > > > > IOMMU mode.
> > > > > > >
> > > > > > > I may miss something but set_map() is empty in this driver.
> > > > > > > How could such isolation be done?
> > > > > >
> > > > > > In no-iommu case, there would be no domain right, and the user
> > > > > > of vhost-vdpa(DPDK virtio user pmd), would create the mapping
> > > > > > and pass the PA (= IOVA) to the device directly. So that,
> > > > > > device can directly DMA to the
> > > > > PA.
> > > > >
> > > > > Yes, but this doesn't differ too much from the case where DMA
> > > > > API is used with IOMMU disabled.
> > > > >
> > > > > Are you saying DMA API introduces overheads in this case?
> > > > No actually, current vhost-vdpa code is not allowing IOMMU
> > > > disabled mode, If set_map/dma_map op is not set. Hence, we are
> > > > setting set_map with dummy api to allow IOMMU disabled mode.
> > > >
> > > > Following is the code snippet from drivers/vhost/vdpa.c
> > > >
> > > > /* Device want to do DMA by itself */
> > > > if (ops->set_map || ops->dma_map)
> > > > return 0;
> > > >
> > > > bus = dma_dev->bus;
> > > > if (!bus)
> > > > return -EFAULT;
> > > >
> > > > if (!device_iommu_capable(dma_dev,
> > > IOMMU_CAP_CACHE_COHERENCY))
> > > > return -ENOTSUPP;
> > >
> > > Right, so here's the question.
> > >
> > > When IOMMU is disabled, if there's no isolation from the device
> > > on-chip IOMMU. It might have security implications. For example if
> > > we're using PA, userspace could attack the kernel.
> > >
> > > So there should be some logic in the set_map() to program the
> > > on-chip IOMMU to isolate DMA in that case but I don't see such
> > > implementation done in set_map().
> >
> > Our chip lacks support for on-chip IOMMU for host-side targeted DMA
> operations.
> > When using the DPDK virtio user PMD, we’ve noticed a significant 80%
> > performance improvement when IOMMU is disabled on specific x86
> > machines. This performance improvement can be leveraged by embedded
> > platforms where applications run in controlled environment.
> > May be it's a trade-off between security and performance.
> >
> > We can disable the no-iommu support by default and enable it through
> > some module parameter and taint the kernel similar to VFIO
> driver(enable_unsafe_noiommu_mode) right?
>
> Could be one way.
>
> Michael, any thoughts on this?
@Michael S. Tsirkin Could you confirm on above. I will send the v2 if it is fine for you.
Thanks
>
> Thanks
>
> > >
> > > >
> > > > Performance degradation when iommu enabled is not with DMA API but
> > > > the
> > > > x86 HW IOMMU translation performance on certain low end x86
> machines.
> > >
> > > This might be true but it's not specific to vDPA I think?
> > >
> > > Thanks
> > >
> > > >
> > > > >
> > > > > Thanks
> > > > >
> > > > > >
> > > > > > >
> > > > > > > > We observed performance impacts on certain low-end x86
> > > > > > > > machines when IOMMU mode was enabled.
> > > > > > > > I think, correctness is Host userspace application's
> > > > > > > > responsibility, in this case when vhost-vdpa is used with
> > > > > > > > Host application such as DPDK
> > > > > > > Virtio user PMD.
> > > > > > >
> > > > > > > Thanks
> > > > > > >
> > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > > Thanks
> > > > > > > > > >
> > > > > > > > > > Thanks.
> > > > > > > > > >
> > > > > > > >
> > > > > >
> > > >
> >
