From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dispatch1-us1.ppe-hosted.com (dispatch1-us1.ppe-hosted.com [67.231.154.164]) by dpdk.org (Postfix) with ESMTP id 616771B2B8 for ; Thu, 18 Jan 2018 12:56:17 +0100 (CET) X-Virus-Scanned: Proofpoint Essentials engine Received: from webmail.solarflare.com (uk.solarflare.com [193.34.186.16]) (using TLSv1 with cipher ECDHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1-us4.ppe-hosted.com (Proofpoint Essentials ESMTP Server) with ESMTPS id BB472B40072; Thu, 18 Jan 2018 11:56:15 +0000 (UTC) Received: from [192.168.38.17] (84.52.114.114) by ukex01.SolarFlarecom.com (10.17.10.4) with Microsoft SMTP Server (TLS) id 15.0.1044.25; Thu, 18 Jan 2018 11:56:08 +0000 To: Yongseok Koh , Thomas Monjalon CC: Jianbo Liu , "dev@dpdk.org" , "Adrien Mazarguil" , =?UTF-8?Q?N=c3=a9lio_Laranjeiro?= , "jerin.jacob@caviumnetworks.com" , "konstantin.ananyev@intel.com" , "bruce.richardson@intel.com" , Chao Zhu References: <20171227042824.33373-1-yskoh@mellanox.com> <20180116091040.GA15629@arm.com> <3720864.2redlIt54T@xps> From: Andrew Rybchenko Message-ID: Date: Thu, 18 Jan 2018 14:56:03 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.2 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit Content-Language: en-GB X-Originating-IP: [84.52.114.114] X-ClientProxiedBy: ocex03.SolarFlarecom.com (10.20.40.36) To ukex01.SolarFlarecom.com (10.17.10.4) X-TM-AS-Product-Ver: SMEX-11.0.0.1191-8.100.1062-23604.003 X-TM-AS-Result: No--9.384700-0.000000-31 X-TM-AS-User-Approved-Sender: Yes X-TM-AS-User-Blocked-Sender: No X-MDID: 1516276576-HGzv+01ZOU5j Subject: Re: [dpdk-dev] [PATCH v2 1/8] eal: introduce DMA memory barriers X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 18 Jan 2018 11:56:17 -0000 On 01/17/2018 09:39 PM, Yongseok Koh wrote: >> On Jan 17, 2018, at 5:46 AM, Thomas Monjalon wrote: >> >> 16/01/2018 10:10, Jianbo Liu: >>> The 01/16/2018 10:49, Andrew Rybchenko wrote: >>>> On 01/16/2018 04:10 AM, Yongseok Koh wrote: >>>>> This commit introduces rte_dma_wmb() and rte_dma_rmb(), in order to >>>>> guarantee the ordering of coherent shared memory between the CPU and a DMA >>>>> capable device. >>>>> >>>>> Signed-off-by: Yongseok Koh >>>>> --- >>>>> lib/librte_eal/common/include/generic/rte_atomic.h | 18 ++++++++++++++++++ >>>>> 1 file changed, 18 insertions(+) >>>>> >>>>> diff --git a/lib/librte_eal/common/include/generic/rte_atomic.h b/lib/librte_eal/common/include/generic/rte_atomic.h >>>>> index 16af5ca57..2e0503ce6 100644 >>>>> --- a/lib/librte_eal/common/include/generic/rte_atomic.h >>>>> +++ b/lib/librte_eal/common/include/generic/rte_atomic.h >>>>> @@ -98,6 +98,24 @@ static inline void rte_io_wmb(void); >>>>> */ >>>>> static inline void rte_io_rmb(void); >>>>> +/** >>>>> + * Write memory barrier for coherent memory between lcore and IO device >>>>> + * >>>>> + * Guarantees that the STORE operations on coherent memory that >>>>> + * precede the rte_dma_wmb() call are visible to I/O device before the >>>>> + * STORE operations that follow it. >>>>> + */ >>>>> +static inline void rte_dma_wmb(void); >>>>> + >>>>> +/** >>>>> + * Read memory barrier for coherent memory between lcore and IO device >>>>> + * >>>>> + * Guarantees that the LOAD operations on coherent memory updated by >>>>> + * IO device that precede the rte_dma_rmb() call are visible to CPU >>>>> + * before the LOAD operations that follow it. >>>>> + */ >>>>> +static inline void rte_dma_rmb(void); >>>>> + >>>>> #endif /* __DOXYGEN__ */ >>>>> /** >>>> I'm not an ARMv8 expert so, my comments could be a bit ignorant. >>>> I'd like to understand the difference between io and added here dma >>>> barriers. >>>> The difference should be clearly explained. Otherwise we'll constantly hit >>>> on incorrect choice of barrier type. >>>> Also I don't understand why "dma" name is chosen taking into account >>>> that definition is bound to coherent memory between lcore and IO device. >>> A good explanation can be found here. >>> >>> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.kernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Ftorvalds%2Flinux.git%2Fcommit%2F%3Fid%3D1077fa36f23e259858caf6f269a47393a5aff523&data=02%7C01%7Cyskoh%40mellanox.com%7C7b526265cbf1449f3db208d55db0c55d%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636517936183877836&sdata=2%2Fi8Gs2n%2Fnbe9%2FJ3GWr22ndPmQVmvM2Xh12r3j1ZWlg%3D&reserved=0 >> I agree that something more is needed to explain when to use rte_io_*. >> The only difference between rte_io_* and rte_dma_* is "on coherent memory". > Okay will add more explanation and send out v3 soon. But, please note that > there's no concrete theory when to use which barrier. Actually, it is mostly > for ARMv8 because it provides more options for barriers. For other archs, as you > can see in the patches, there's no difference from IO barriers. Absence of concrete theory does not make choice of the memory barrier easier. I would say it complicates it significantly. I think it is a minimal requirement for the patchset to explain why a new type should be defined instead of just fixing of the rte_io_* barriers on ARMv8. What's the different? Which criteria should be checked/taken into account to make the right choice? As far as I can see igb_uio and uio_pci_generic do coherent DMA mapping. It is not that easy with VFIO since in theory it could be non-coherent if snooping is not supported by IOMMU. Don't know if it is real. If so, it makes barrier choice UIO driver-dependent. Sounds bad.