From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from NAM01-SN1-obe.outbound.protection.outlook.com (mail-sn1nam01on0085.outbound.protection.outlook.com [104.47.32.85]) by dpdk.org (Postfix) with ESMTP id 4909A5692 for ; Fri, 13 Jan 2017 15:58:19 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=CAVIUMNETWORKS.onmicrosoft.com; s=selector1-cavium-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version; bh=0SyWfufIoGJ8Xt5Sh9AaVfqyb/NasbKgXBy/Dq5BkOo=; b=BZ1EZoaJX+GM5ohyU1WHj/gfI542CAZ2KlqH5/V1pZarkW0Qzm/VYxZU/0HkQXmpRoBvHOfsJF+c8RP4KLLxM4PhlHfg3TZYiEZ5MZCWBO5RSn1+ODX3Ql+Cujsyn0O98p3i56tSoW2RjF4DkSsWFnsZbWM018eFycCWRihBoXg= Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=Jerin.Jacob@cavium.com; Received: from localhost.localdomain (171.61.97.114) by BLUPR0701MB1715.namprd07.prod.outlook.com (10.163.85.141) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P384) id 15.1.829.7; Fri, 13 Jan 2017 14:58:13 +0000 Date: Fri, 13 Jan 2017 20:27:54 +0530 From: Jerin Jacob To: Ferruh Yigit CC: , , , , , , , John Griffin , Fiona Trahe , Deepak Kumar Jain Message-ID: <20170113145753.GB13558@localhost.localdomain> References: <1482832175-27199-1-git-send-email-jerin.jacob@caviumnetworks.com> <1484212646-10338-1-git-send-email-jerin.jacob@caviumnetworks.com> <1484212646-10338-16-git-send-email-jerin.jacob@caviumnetworks.com> <6bb9980b-f546-38d5-044a-63507510f6a5@intel.com> <20170113081641.GA17635@localhost.localdomain> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.7.1 (2016-10-04) X-Originating-IP: [171.61.97.114] X-ClientProxiedBy: MAXPR01CA0058.INDPRD01.PROD.OUTLOOK.COM (10.164.146.158) To BLUPR0701MB1715.namprd07.prod.outlook.com (10.163.85.141) X-MS-Office365-Filtering-Correlation-Id: 06ebc46c-581d-4744-f443-08d43bc49b13 X-Microsoft-Antispam: UriScan:; BCL:0; PCL:0; RULEID:(22001); SRVR:BLUPR0701MB1715; X-Microsoft-Exchange-Diagnostics: 1; BLUPR0701MB1715; 3:yuU3dxmqrmPPISUfy5hURplUzfBtt07ld8XUEn3G5qUuJeyFNFOVC0QFgAAk9ZbN3CkAzG6rOuKplH+c6aYHtvqgnfevFwXjMc3RTUa0aQRjTRahnzRKk6EmDCxt/VYRJMru1NoZgG78a+SK5J7frI3s8Ir4B9YmHAjOs0dlIJJQosG0ob1wMVOMebQUI8TvUdI+Nutm5ZCV7HRQhHT9hSDJyPkfxJWzpHqmoorEa+xWR8F6BOk+RWNPBtJdD8LPmtYuIWrieZiq3QHfEUMapA== X-Microsoft-Exchange-Diagnostics: 1; BLUPR0701MB1715; 25:nIHmfJzqvpuO6kk7takOVJlk/FDl3cRtaHyzUX2ayaRayrPWp+WkZ6lEPf0k32gzO0LW6iLV3Ofb5r67jF3vQ+V03HrQkPeTqOd2ssQSXgQ/akbBXgwad2rK3PS4JFbBtvvRRDsmIouuTkYERQXR2AMbHrsylw8c1wyT23iWXkWOwBhs/rN8uWGJe45i17c72Rgw2C+Jarci/PKIbG+0otC2hLW/7sntg43roDOLe3iQ39GoGeC55mtpIlEIrWaQMaMk4smv7JPDgI40XcN/veGTIHTnq8zx7iqDZVg6Z0zS4PkXolbb872dsu5ZirpjC9cZ2wHuBBPNhbbJzLxT0EYorYX2XvHXbNBKYAOCki8Wrgw4IygKJLbGi+zIYot/lJr4ioVwvSfPmk/HiVxpFQ3GMQCaR4l/BjVb8nmM1mXDbk4U5KSFtmGtCqdOItGTgtf7tyZUvi8HwJz5Zd/HreK1Zv9PfLkFu55wpItpevhC5sUHeJHArpfcOSxbU3jmIFO9eIh1Aj5ngw4sBBn2ctaIvX+/xFjYLAlR2YCyWD+fKdXEj+C8F60jxLCgTY5hJns+/ndJ8VhI/OqwbU36Ch4Iz/RGeCvtyQkHMXMZfEgH+j5cm6K06VCryU3wJsdO81zajNR5DfijgHRP+Ced23XGYFy7707i6taqP+f+bvMSMJd4pPs+/U9oQDtBxFn5dW2GWEDpLdrO1++aFH+rWJ6ldV1GODET6zHMtbfX0p1IloDTKvTM3UN0498wbSr57cBlSf17Hlf8wpYc4Nfs8shnGBqJ1wfW6bK7Y6YgInbjLABtXQqsXdQ1uqz+cpOmXZ6XMK7EjcwI4roCgUBIvA== X-Microsoft-Exchange-Diagnostics: 1; BLUPR0701MB1715; 31:zLkEdH/2CqvwezIJkPCPS46eag2GajB1ccd9e36NsKwfISorTNtkyJsfOTmJ9dAPE/g5JLjos11clUv9rGuohNx7uIYMOHa4pCWMueyTb1mz4I3eObA23/vBl4td8YczjtsBcoTPHin4i4Sv+n47oZyCWJBOGQ8bhudLC/q9Srd3OH1Z0cURUc/K6r98+mhSBrz7rGg6Q1spgbj6JSYISTOUKe1SMX+sFU0910orvUOO8n29j7VFgQhVoGp+8cYm; 20:Kz+xZz8cG9rsG5fRvPU6uxsrHibuJxVwiJaN5NkzLyMfh5djdc44zGlWA/vQCMcOOpkgHST3agwG0VjWS0YjHYBqwiK3klJuHQMlwG0fHGtupB80MLwcL01hJ8PiWmTmi6KhPBJ+YNpFRz2Rpw6FIMTu777tlbbVjhWwAv+SKFZUBDVhDxw4fd06Xiw3yqKHVZjddE8KRYPGMJV3a/VXhxzKMlWmLeSgnMN2vLfeu8XvAiuHzO44zOnzDPe4FoMuo/fAzwuV9aNX0bD4/WlvpoZpQjlCtVsAAqEZi335IQOj/YKfyVUgNtBBKhSVr4SuynyO77nebEoFMRH8QoDXy6ldUkCmTRyWpoFF4J1gUxBlZ01YqqOCbO3CZzfVw+Kf/UFTomzhlgtqPLCUeZgSFwPdNHSgGbYQiszzp4vBqbb/UVHtk7yM8a84aoYUtQap5uHtpfns0IdpQilixaHDDJQ9D9z9Jv/p2NEVbF8Pn2OePzXoczc3IT50Kb9pUWar9jAx5Ek39XrY/tYD3I984NT0VP4HPBtB6G9in4bmGn9RXP8RT8Z7ZMVQJh++EdY72v2MeXqrxsALWLxQmB5mxyf1rV4SBCV4ES8N9uyfmoU= X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:(788757137089)(228905959029699); X-Exchange-Antispam-Report-CFA-Test: BCL:0; PCL:0; RULEID:(6040375)(601004)(2401047)(8121501046)(5005006)(10201501046)(3002001)(6041248)(20161123555025)(20161123564025)(20161123562025)(20161123560025)(6072148); SRVR:BLUPR0701MB1715; BCL:0; PCL:0; RULEID:; SRVR:BLUPR0701MB1715; X-Microsoft-Exchange-Diagnostics: 1; BLUPR0701MB1715; 4:8TFNxbuH05iJeotzrLaLO5EUC86nrsw2soHanjmw4vNG5T44i6V2pdAAa9EdvITupaCmNKvP3sR89BwmNThaCWIL+7VNhZCnN8hELqUtqnd7J3iQroOf5kDhYBMs0Yc+JGv55r722zwXbYWYgKSRlYnanKnA6ncfKJuMN1xGVwMf+pKkWa+5kDMX50nN+jertvbCM3gnJ0i4H4ZWF2TGR7h0rIuJpXnni5prwGMIdeg9/dSDUuGhgayxPaygWBaw6FqqMmVJipiAC4gDc8WpjWqONXf08JKoDS4xIgi1+lXBanOOYz/MQcQ5QKZDi3rGN8rLSQOdpVKmIoQmDSuizqI9Gw2AnKOwltxsJ4N9LOvpkxOGk7v1BcOEbcXrYc//BgjSa7ecjJsGGWyS1lRfMFQpXLcieKa5bgAJ652j/R9bu+Fx/CeSNvpDRq9Soz+bT5XV3z2TiZpX1YMMTj7ceS7dc4lobEfamO2JGBgN/kacIEIXucA3UvLvdPQmOZX+bYMMQhwlmfGcYhiT3m64qMb8Gi6j1i6jK0p4LvnyliCatD82t/+b1GVJLfO6+uzHt1LpeO5YQGAjAX8drctArLEI2h0ZkFDKN//ziF5QU8hu+fyTFFF+AdUB+u9rzsQdeUmJMsKLcQAenZtAM2xDNQ== X-Forefront-PRVS: 018632C080 X-Forefront-Antispam-Report: SFV:NSPM; SFS:(10009020)(4630300001)(6009001)(6069001)(7916002)(39450400003)(189002)(199003)(76104003)(24454002)(377454003)(54356999)(101416001)(93886004)(76176999)(7736002)(9686003)(7416002)(97756001)(92566002)(68736007)(50986999)(106356001)(305945005)(105586002)(66066001)(42186005)(46406003)(81166006)(81156014)(8676002)(50466002)(61506002)(47776003)(33656002)(4326007)(6116002)(229853002)(23726003)(4001350100001)(1076002)(55016002)(3846002)(6916009)(2950100002)(110136003)(42882006)(54906002)(5660300001)(2906002)(83506001)(6666003)(25786008)(38730400001)(6506006)(189998001)(97736004)(18370500001); DIR:OUT; SFP:1101; SCL:1; SRVR:BLUPR0701MB1715; H:localhost.localdomain; FPR:; SPF:None; PTR:InfoNoRecords; MX:1; A:1; LANG:en; Received-SPF: None (protection.outlook.com: cavium.com does not designate permitted sender hosts) X-Microsoft-Exchange-Diagnostics: =?us-ascii?Q?1; BLUPR0701MB1715; 23:lIEqmaFx9YHndUMbdri7dt8LxFIcxBSGkWMqGjR?= =?us-ascii?Q?tr+15o/QfdZQK5JjNM/NzUNRf9ELQehxOnQxI4ExRUaD2Ug9+yb/geapoFY2?= =?us-ascii?Q?AjWeZdo7uRP6iVXrglUmfta1+MdHJGOqrjZ0//Z1Epx/H5m2ETT8SnHyambf?= =?us-ascii?Q?yp+H+/lliAIc2zCTpDigfIyZTaRnvt1cPoch9n+ULdiXXnTQiJniagYzJeoI?= =?us-ascii?Q?BHFbD22xdw+YpwDrvS/JeXwUEWHx4CdcOyP5hpG3glQICiHBv1Vztanp6SLk?= =?us-ascii?Q?6h3eawzwjbo+PC9VFZpdmN3u3mlDAzZiF3Vlyu3Eg+8X8h5YYgPKTw2+Oqxm?= =?us-ascii?Q?wSRrL8JJiH1S1F8e525D0tFS7qRoiyTWsz1jrbP46RaplPHHMq2O4C5MwFSa?= =?us-ascii?Q?Ijwvgc6P0xyr6OltH0yVl6ECoaCMtLisc7uQkbmaxK9f1DM1+HfTpp8CILE/?= =?us-ascii?Q?pVckQRGLm+eCVwQTpAMexiVWz6j0p1piiA37NOMTS1qHnK3qrNL4AIGUbwSr?= =?us-ascii?Q?FOTnnnut7iUDrP/R9JH5lozVJF94x1lCKI99doxdQ/ZnWWhC6Kn5J0YRG5be?= =?us-ascii?Q?tX3OAt3EcE3s3MYjgZrg9I1r5f3smmYqWRtvb0CTHKqs22WfzAdbH9gGsW96?= =?us-ascii?Q?Ra/bx3qjq+AS5JGzvBmSIGosA0QhheatryEwBjaLhL8QB2JZOpZwXY0XdK23?= =?us-ascii?Q?yhYqIIZ7vMjJ6UGfh7D56qx611YpfOL1okwJ3TEtspwNvvpWRfzLy+p3+I8G?= =?us-ascii?Q?r3sFc6fKjeLGRKnexIp5ZNys+CY39FV2CeU69Yznp1Rfsdpi9NfjeUvU5MH2?= =?us-ascii?Q?/CqFDlAkH3Ylj1lIL2155JNJnL32UPYA1vC9RkkiRz9NeZco8LykOXq2sjFO?= =?us-ascii?Q?JD3zCHTFm9aXIJfuxrwhS9EUzeRRDIgeXt0oaZqSEmEhn9T7H8HpdjeaGpAq?= =?us-ascii?Q?lKAmDplq3oaKOcjw9jomuqBp9AdelFPOtVPvFUY4oWyH3btgPoKrRm+cc3wj?= =?us-ascii?Q?f/taii2z4CNuKWpcS0EBChukt9yNlb0ZFfb+OLtp2CTsIXyaSzcQvDU/Qe1Z?= =?us-ascii?Q?SUI7v5IyVD+4P+dzADVvcWSnI/W8rXF/JKOFXJQz40kmN0khhA4tCavmKCSx?= =?us-ascii?Q?LFBHcukConLsZGeUKglplCm7E7a+uD8W71IP02eqeSvDV1n1lLb8o+5mF+XG?= =?us-ascii?Q?Sb6YeHcYyiZwIXfsEIWQcnThHlRHqLLnPtItZdwQiGQzP/JWiFe1RONdHB1A?= =?us-ascii?Q?vvhmA5T7R0sMe/rfdxrQbU0YLEfetetd+WupKWgxoIflEs3rqEzN1VRuuQFt?= =?us-ascii?Q?t5EOqrUNYc2RXYfQ0NLar+emmOOSIPJXK26lVD3MHMlogPyXNms4QTphAP3+?= =?us-ascii?Q?JlSijaw=3D=3D?= X-Microsoft-Exchange-Diagnostics: 1; BLUPR0701MB1715; 6:zmPtKai/t7lIRDY32LertIAXcAGhgWDV+FzQZUhwsIu8rpkdZoGHvwMvatUaxcZo+9Kt6kuWy+9PMSJmSnH8G6ZR1xaeHgQaR0cf2x6foJ3SozoWt3Nu2uRh4YnrAAuQCbyTiOS2ifvMxZrCRW4MCqAD0KAFhRn/KOEWwe52mloAMWKX3BaIIhjXitm+Wvd+Xd2Pdx73jmeY5FvXefBBafNG9xz/N8jR/Yxh+yI+NxVPwaYOY6Wno96wIWUoP5XfxOJMDSyZcxmYMHIjr830abVs7hcOrk/OaBJIOoD0NnuBHnAa8T2WKd4gPUjnyRNOmzim1GMTpsSr3/Kqy7FBb7zKu6cGFgMYib3eqjSzHQaH6GtgVDWy5pxX57uMDfO5gJOxDGQa7FAaegm/908leZt2tI1RHi2u7AxFV/NsyXk=; 5:Xz2lR41yJz0XG6EOemONrG/IkgUJMePr9HA1TAsGRZHTC5Uqe7LciDpXYHyL4ZP6SST0y7eAO2/XZ3ZX5pVRj+3A6WGlPZJWsAI+bjxjl0eB+aWjUyFrip1OPicybTmoQqIL7wfW3beXpptbGsGwOg==; 24:ig8FO8I66Lm1B2Ncnsvm1oGlkf2UAugsa4GvHvr5tq5jd/Cx5IR+wMjDZ56f34EpXWIBWda5v2yfyZOwJhHaT4gB0c9UfShuCHriXpjyZHc= SpamDiagnosticOutput: 1:99 SpamDiagnosticMetadata: NSPM X-Microsoft-Exchange-Diagnostics: 1; BLUPR0701MB1715; 7:fFz0rlUlu3oEzP+dDSOrY7hHE7iROmehTFhICDfqzTiZsLsSR52WOKOgdTeEbipzLvs6Yy2p+TGW0Sj9jAZpQ9dHd6c6KOK+Ak+tOpwxV5rCYXuYRzpUXH6Qy5vCEXlm2LXk7eaavjhqUWR1wPo7B1GwhksVOwydOXrT/f6zrjwH7mEJ7GHBQ6vl1qGdzlD1EnTOOcm4fIhVzbmbyjMHKBloC6zxdS1BevfcWXyl6eLsGvKC5NlUs1jbn6oJBJXJQcSWQW2CQfKXuNBUTnY0bwMUkLf0zzkAqOy9w+cJ6J1i94R7EELS32BGJHKL6yOggTNSmjyWE8BoVtixUYW+WJVJeEQJb4UO4kUx+yNpx5w54kp4SlZT61P+PMgV7QBTAlbKCAgWA1HLi/7kFLzF8dQaDaTy8ZIHyc54bDMSQOeljeJfslB0rET/vIYa3UqWEAHLEAoZ0bItNqiXudpRhQ== X-OriginatorOrg: caviumnetworks.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 13 Jan 2017 14:58:13.1525 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-Transport-CrossTenantHeadersStamped: BLUPR0701MB1715 Subject: Re: [dpdk-dev] [PATCH v3 15/29] crypto/qat: use eal I/O device memory read/write API 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: Fri, 13 Jan 2017 14:58:19 -0000 On Fri, Jan 13, 2017 at 11:32:29AM +0000, Ferruh Yigit wrote: > On 1/13/2017 8:17 AM, Jerin Jacob wrote: > > On Thu, Jan 12, 2017 at 07:09:22PM +0000, Ferruh Yigit wrote: > >> Hi Jerin, > >> > >> On 1/12/2017 9:17 AM, Jerin Jacob wrote: > >> <...> > >> > >>> +#include > >>> + > >>> /* CSR write macro */ > >>> -#define ADF_CSR_WR(csrAddr, csrOffset, val) \ > >>> - (void)((*((volatile uint32_t *)(((uint8_t *)csrAddr) + csrOffset)) \ > >>> - = (val))) > >>> +#define ADF_CSR_WR(csrAddr, csrOffset, val) \ > >>> + rte_write32(val, (((uint8_t *)csrAddr) + csrOffset)) > >> > >> For IA, this update introduces an extra compiler barrier (rte_io_wmb()), > >> which is indeed not a must, is this correct? > > > > AFAIK, Compiler barrier is required for IA. I am not an IA expert, if > > someone thinks it needs to changed then I can fix it in following commit > > in this patch series by making rte_io_wmb() and rte_io_rmb() as empty. > > > > Let me know. > > > > AFAIK, Linux kernel code has a barrier in readl/writel for IA. > > > > Typically we don't use any non relaxed versions in fast path.In fast > > typically all the drivers has explicit write barrier for doorbell write > > and followed by a relaxed version of write. IMO, In any event, it won't > > generate performance regression. > > > > [dpdk-master] $ git show > > 70c343bdc8c33a51a9db23cd58122bdfc120a58f > > commit 70c343bdc8c33a51a9db23cd58122bdfc120a58f > > Author: Jerin Jacob > > Date: Mon Dec 5 06:36:49 2016 +0530 > > > > eal/x86: define I/O device memory barriers for IA > > > > The patch does not provide any functional change for IA. > > I/O barriers are mapped to existing smp barriers. > > > > CC: Bruce Richardson > > CC: Konstantin Ananyev > > Signed-off-by: Jerin Jacob > > > > diff --git a/lib/librte_eal/common/include/arch/x86/rte_atomic.h > > b/lib/librte_eal/common/include/arch/x86/rte_atomic.h > > index 00b1cdf..4eac666 100644 > > --- a/lib/librte_eal/common/include/arch/x86/rte_atomic.h > > +++ b/lib/librte_eal/common/include/arch/x86/rte_atomic.h > > @@ -61,6 +61,12 @@ extern "C" { > > > > #define rte_smp_rmb() rte_compiler_barrier() > > > > +#define rte_io_mb() rte_mb() > > + > > +#define rte_io_wmb() rte_compiler_barrier() > > + > > +#define rte_io_rmb() rte_compiler_barrier() > > + > > /*------------------------- 16 bit atomic operations > > * -------------------------*/ > > > > #ifndef RTE_FORCE_INTRINSICS > > > >> > >> If so, does it make sense to override these functions for x86, and make > >> rte_writeX = rte_writeX_relaxed > >> rte_readX = rte_readX_relaxed > >> > >>> > >>> /* CSR read macro */ > >>> -#define ADF_CSR_RD(csrAddr, csrOffset) \ > >>> - (*((volatile uint32_t *)(((uint8_t *)csrAddr) + csrOffset))) > >>> +#define ADF_CSR_RD(csrAddr, csrOffset) \ > >>> + rte_read32((((uint8_t *)csrAddr) + csrOffset)) > >> > >> This patchset both introduces new rte_readX/rte_writeX functions, also > >> applies them into drivers. > >> > >> While applying them, it changes the behavior. > >> Like above code was doing a read, but after update it does read and > >> read_memory_barrier. > >> > >> What do you think this patchset updates usage in a manner that keeps > >> behavior exact same. Like using rte_read32_relaxed for this case. > >> And doing architecture related updates in a different patchset? > > > > Need to use rte_read32 at this commit otherwise it will break for ARM. > > That's was all point for this patchset. > > Why it breaks the ARM, is it because rte_*mb() updated for ARM in this > patchset (patch 7/29) ? Yes. > > I believe it is good to make these modifications in two phase: It is in two phases only. First introduced the API with implementation and enabled in each driver. Why did you think other-way around it is better? I can rework and test if there is any value addition. If you concerned about git bisect ability then I don't think we are loosing that in this model. Thoughts? > - First replace old usage with rte_readX/rte_writeX while keeping exact > same behavior > > - Second, do architecture specific changes. Both in eal and drivers > level if required. > > Thanks, > ferruh > > > For performance regression, we can always verify by taking delta > > between this changeset and the previous changeset. If you think, I need > > to make rte_io_wmb()/rte_io_rmb() as empty for IA then I could do that > > as well. > > > > > >> > >> This both makes easy to see architecture specific updates, and makes > >> easy to trace any possible performance issues by this patchset. > >> > >>> > >>> #define ADF_BANK_INT_SRC_SEL_MASK_0 0x4444444CUL > >>> #define ADF_BANK_INT_SRC_SEL_MASK_X 0x44444444UL > >>> > >> >