On 04-Oct-18 2:35 PM, Alejandro Lucero wrote:
On Wed, Oct 3, 2018 at 1:56 PM Burakov, Anatoly <anatoly.bura...@intel.com <mailto:anatoly.bura...@intel.com>> wrote:On 31-Aug-18 1:50 PM, Alejandro Lucero wrote: > Although VT-d emulation currently only supports 39 bits, it could > be iovas being within that supported range. This patch allows > IOVA mode in such a case. > > Indeed, memory initialization code can be modified for using lower > virtual addresses than those used by the kernel for 64 bits processes > by default, and therefore memsegs iovas can use 39 bits or less for > most system. And this is likely 100% true for VMs. > > Signed-off-by: Alejandro Lucero <alejandro.luc...@netronome.com <mailto:alejandro.luc...@netronome.com>> > --- > drivers/bus/pci/linux/pci.c | 15 ++++++++++----- > 1 file changed, 10 insertions(+), 5 deletions(-) > > diff --git a/drivers/bus/pci/linux/pci.c b/drivers/bus/pci/linux/pci.c > index 04648ac..215dc10 100644 > --- a/drivers/bus/pci/linux/pci.c > +++ b/drivers/bus/pci/linux/pci.c > @@ -588,10 +588,11 @@ > fclose(fp); > > mgaw = ((vtd_cap_reg & VTD_CAP_MGAW_MASK) >> VTD_CAP_MGAW_SHIFT) + 1; > - if (mgaw < X86_VA_WIDTH) > - return false; > > - return true; > + if (!rte_eal_check_dma_mask(mgaw)) > + return true; > + else > + return false; return rte_eal_check_dma_mask(mgaw) == 0; ? I guess that works and is more elegant. Thanks. > } > #elif defined(RTE_ARCH_PPC_64) > static bool > @@ -615,13 +616,17 @@ > { > struct rte_pci_device *dev = NULL; > struct rte_pci_driver *drv = NULL; > + int iommu_dma_mask_check_done = 0; > > FOREACH_DRIVER_ON_PCIBUS(drv) { > FOREACH_DEVICE_ON_PCIBUS(dev) { > if (!rte_pci_match(drv, dev)) > continue; > - if (!pci_one_device_iommu_support_va(dev)) > - return false; > + if (!iommu_dma_mask_check_done) { > + if (!pci_one_device_iommu_support_va(dev)) > + return false; > + iommu_dma_mask_check_done = 1; > + } > } The commit message doesn't explain why are we only checking a single device. Indeed, i am not 100% clear as to why, so some explanation in the commit message and preferably a comment in code would be more than welcome :)Because the pci_one_device_iommu_support_va function does always the same whatever the device is used in the call.
So, this code was always wrong and needlessly checked each device when it could've checked it a single time? OK, that makes it a bit clearer. Still, needs to be documented in comments/commit message :) The commit message IMO looks quite irrelevant to what happens in the commit. It almost feels like this commit should be split in two - first change the mgaw check, and then fix the PCI bus code to not check needlessly.
-- Thanks, Anatoly
