15/07/2019 17:35, Jerin Jacob Kollanukkaran: > From: Thomas Monjalon <tho...@monjalon.net> > > 15/07/2019 16:26, Jerin Jacob Kollanukkaran: > > > > > Is there any specific reason why we always prefer PA if physical > > > > > addresses are available? Since we're already assuming that all > > > > > devices support PA and VA anyway, what's the harm in enabling VA by > > default? > > > > > > > > If PA is available, it means we are running as root. > > > > We can assume that using root is a choice, probably related to a > > > > preference for PA. > > > > > > # Even if we are running as root, Why to choose PA in case of DC? > > > ie. Following logic is not need > > > if (iova_mode == RTE_IOVA_DC) { > > > iova_mode = phys_addrs ? RTE_IOVA_PA : > > > RTE_IOVA_VA; > > > RTE_LOG(DEBUG, EAL, > > > "Buses did not request a specific IOVA > > > mode, using '%s' > > based on physical addresses availability.\n", > > > phys_addrs ? "PA" : "VA"); > > > } > > > > Why running as root if using VA anyway? > > We can assume the user knows what he is doing, so it is a user choice. > > We want to allow the user choosing, right? > > The user can override iova=pa/va as eal argument if user needs to run a > specific mode. > Running as root for various other reason(just be lazy) etc. it is not or it > should not > be connected to set the mode as PA.
Good point. I tend to prefer avoiding the use of EAL arguments because they may be unavailable, depending on the application. > > > # When DPDK running on guest, Anyway it can not access the real PA, It > > > will > > be IPA. > > > > What is IPA? Isn't it a beer? > > There may a beer with that name. In this context, it is "Intermediate > physical address" > > > > So I don't understand logic behind choose PA when DC. > > > To me, it make sense to choose PA when DC. > > > > You probably mean "choose VA". > > Yup. > > > > # To align with RTE_PCI_DRV_NEED_MAPPING flag and reflect it "need" > > > rather than support, I think, flag can be changed to > > > RTE_PCI_DRV_NEED_IOVA_AS_VA > > > > I think the most important is to have a good documentation of this flag (it > > was not done properly when Cavium introduced it initially). > > If you want to rename the flag, you can do it in a separate patch. > > If renaming, I really would like to get an answer to an old question: > > Why IO adress is called IOVA? The name "IOVA_AS_VA" looks strange. > > IOVA = IO virtual address > Since IOVA can be PA or VA, the name IOVA_AS_VA as chosen We could also call it "bus address" or "device address". I think the word "IOVA" was enforced by Linux. Anyway, my real issue when using "virtual" is that we don't really know what we are talking about: is it an IOMMU translated address on the device side or an MMU translated address on the application side? I think we should better explain things. One diagram which can help: https://en.wikipedia.org/wiki/Input%E2%80%93output_memory_management_unit#/media/File:MMU_and_IOMMU.svg > > For reference, one description of addressing: > > https://lists.linuxfoundation.org/pipermail/iommu/2018-May/027686.html > > > > About the naming, do you remember how I insisted to have a correct naming > > of all related stuff in DPDK? It was hard to get it accepted, the > > discussion was > > not nice and I stopped insisting to get all details fine because I just got > > bored. > > It was a really bad experience. > > I agree. > To me that bad experience was due to mostly not having enough technical > comments > On the proposal. Though I am not the author/owner of it. > > > You can ask why I remind this now? Because we must take care of all details, > > make sure our messages are well understood, and be cooperative. > > No disagreement. > If we see the history the meaning got changed/updated in this commit > By adding intel drivers to it. I would nt say it is big ideal, It just C > code, > It can be changed based on the need. I think, what really import is, > maintain the the feature and commitment towards fixing any issue. > > commit f37dfab21c988d2d0ecb3c82be4ba9738c7e51c7 > Author: Jianfeng Tan <jianfeng....@intel.com> > Date: Wed Oct 11 10:33:48 2017 +0000 > > drivers/net: enable IOVA mode for Intel PMDs > > If we want to enable IOVA mode, introduced by > commit 93878cf0255e ("eal: introduce helper API for IOVA mode"), > we need PMDs (for PCI devices) to expose this flag. > > Signed-off-by: Jianfeng Tan <jianfeng....@intel.com> > Acked-by: Anatoly Burakov <anatoly.bura...@intel.com> > Reviewed-by: Santosh Shukla <santosh.shu...@caviumnetworks.com> The doxygen meaning did not change from day one: /** Device driver supports IOVA as VA */ But the commit log meaning was: "Flag used when driver needs to operate in iova=va mode." And the Intel commit log had a different understanding: "If we want to enable IOVA mode, [..] we need PMDs [..] to expose this flag." Anyway we agree on the new meaning to be the original one the author had in mind (i.e. "driver needs"). > > > Other than above points, > > > Reviewed this patch and tested on octeontx2, It looks good to me.