Hi Jerin, I will check how to improve this. Will resubmit this patch with your suggested changes.
Regards, S.Amarnath -----Original Message----- From: Jerin Jacob <jerinjac...@gmail.com> Sent: Tuesday, October 15, 2019 3:14 PM To: Somalapuram, Amaranath <amaranath.somalapu...@amd.com> Cc: dev@dpdk.org; sta...@dpdk.org Subject: Re: [dpdk-dev] [PATCH v1 5/6] crypto/ccp: enable IOMMU for CCP [CAUTION: External Email] On Tue, Oct 15, 2019 at 2:05 PM Somalapuram, Amaranath <amaranath.somalapu...@amd.com> wrote: > > Problem statement: As of now vdev device do not support IOMMU. > vdev device used to custom solution, even for software drives like openssl > uses vdev. I spend some time going through the driver. #The ideal architecture of this driver would have been to introduce a bus driver(see drivers/bus/ifpga/) which does all the PCI probes(drivers/crypto/ccp/rte_ccp_pmd.c and drivers/crypto/ccp/ccp_pci.c) and arrange the devices on the bus scan and enable "non bus" specific driver crypto driver. So that no pci probe etc in crypto driver. # On the upside, You DONT need to give vdev= on eal arguments, on AMD machines it can probe the crypto devices automatically and probe the crypto driver # I dont think, it is specific to vdev IOMMU support, If would have been PCI/Any bus driver, you would have similar changes. Right? # Major portion of this patch does following + if (iommu_mode == 2) + pst.src_addr = (phys_addr_t)rte_mem_virt2iova( + (void *) lsb_buf); + else + pst.src_addr = (phys_addr_t)rte_mem_virt2phy( + (void *) lsb_buf); + Since the following check already present common code, Do we need the above check, just calling rte_mem_virt2iova() is enough. Right? rte_iova_t rte_mem_virt2iova(const void *virtaddr) { if (rte_eal_iova_mode() == RTE_IOVA_VA) return (uintptr_t)virtaddr; return rte_mem_virt2phy(virtaddr); } > I feel its not advisable to put iommu in vdev. > moving the changes to vdev will effect rest of the vdev drives. > That will be big efforts. Every vdev drivers has their own implementation. > Need better design to move it to vdev or common place. > > Regards, > S.Amarnath > > -----Original Message----- > From: Jerin Jacob <jerinjac...@gmail.com> > Sent: Tuesday, October 15, 2019 1:47 PM > To: Somalapuram, Amaranath <amaranath.somalapu...@amd.com> > Cc: dev@dpdk.org; sta...@dpdk.org > Subject: Re: [dpdk-dev] [PATCH v1 5/6] crypto/ccp: enable IOMMU for > CCP > > [CAUTION: External Email] > > On Tue, Oct 15, 2019 at 12:32 PM <asoma...@amd.com> wrote: > > > > From: Amaranath Somalapuram <asoma...@amd.com> > > > > CCP use vdev framework, and vdev framework don’t support IOMMU. > > Adding custom IOMMU support for AMD CCP drives. > > Cc: sta...@dpdk.org > > > > + if (iommu_mode == 2) > > + pci->kdrv = RTE_KDRV_VFIO; > > + else if (iommu_mode == 0) > > + pci->kdrv = RTE_KDRV_IGB_UIO; > > + else if (iommu_mode == 1) > > + pci->kdrv = RTE_KDRV_UIO_GENERIC; > > The crypto driver should not have iommu mode-specific handling. > I am not sure about the problem statement. If the problem is, iommu > support for PCI based vdev device then move the solution to common > layer so that everyone can use it. If not, please share the problem > statement