From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from EUR03-DB5-obe.outbound.protection.outlook.com (mail-eopbgr40089.outbound.protection.outlook.com [40.107.4.89]) by dpdk.org (Postfix) with ESMTP id 11A491B751 for ; Mon, 9 Apr 2018 13:54:44 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nxp.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version; bh=9wVgmSv/4SPoUnZ1gOBVJ4n9c4PjkegldHpdh3Rtj0Y=; b=L/Kwov9e2ggsTi9EoBtV/WXiHWzwZkwM+lcvXWmKUNy6sTwfgyk+ELp8lwwvajljU1wyneVgJqOybnFKR4zvFelo4PLTz7Xt+nJDoOiC56maF5A5zT8igUR6nhs6VVButUtvHSsM2iq2KOSQV8jsdMpyKMrnbGtzf+F4TnAWcDI= Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=shreyansh.jain@nxp.com; Received: from [10.232.14.39] (14.142.187.166) by AM5PR0402MB2770.eurprd04.prod.outlook.com (2603:10a6:203:98::12) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.653.12; Mon, 9 Apr 2018 11:54:41 +0000 To: "Burakov, Anatoly" Cc: dev@dpdk.org References: <65d1fceb-91d2-0f8a-9dc2-2530f06cccce@intel.com> From: Shreyansh Jain Message-ID: Date: Mon, 9 Apr 2018 17:39:55 +0530 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.7.0 MIME-Version: 1.0 In-Reply-To: <65d1fceb-91d2-0f8a-9dc2-2530f06cccce@intel.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit X-Originating-IP: [14.142.187.166] X-ClientProxiedBy: HK2PR02CA0192.apcprd02.prod.outlook.com (2603:1096:201:21::28) To AM5PR0402MB2770.eurprd04.prod.outlook.com (2603:10a6:203:98::12) X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-HT: Tenant X-MS-Office365-Filtering-Correlation-Id: 984d3d81-e110-4b71-fa9f-08d59e10ae51 X-Microsoft-Antispam: UriScan:; BCL:0; PCL:0; RULEID:(7020095)(4652020)(48565401081)(5600026)(4604075)(4534165)(7168020)(4627221)(201703031133081)(201702281549075)(2017052603328)(7153060)(7193020); SRVR:AM5PR0402MB2770; X-Microsoft-Exchange-Diagnostics: 1; AM5PR0402MB2770; 3:gIWce2pnrNb/uGEhfNNYasEwCuCCbSdGS9OAxmuVH8oMqko1HFyEASJqnMQ9+SXXjI5kMEumiPD8Y27RA2fqb2LGV4cUbumMLvJSIma/Rxh7LucH87y5JoB89/h2h0gsMvx1hdOQR1N7DWz4zuwH9e4jZjO0bD4oyKaoGotbP7F7wu0Soe6nQsG8rFsbOgfMO+KkzROqx5AQjKjSt/8QBy2IqRsJOWo5ytdZzwgVkhR0aO3tBjXs2MG/pMaGdg4q; 25:RPSBL5SMgrx3AX40dGuYAV3T4ZRQAhvyCHguTHb7lTmTBy9ocgogbcP54vxA1MDV62DlHw8UfWH0gVHXh4jjp2oVTJ+1832yWNpwb9lvA88tusut7rNMpaYA8sjvNJH9JoOu2Nbm+Fkg1Fe1cRPIIkMLFCG7LE9LTqOWaPJNCtNB3wFxlNkWwIcx/2QxwyKkQzJJQKNT94o23BrdxGr1dvHBOF41cgZh+MC8cjvk75ECKHdDXkUkxgcTMtcs5MDXkeHhoNOHREtLO1dWGFXXoDQWD6FKUoJNyk+A+TzQ5cQvmkdhv4apIPVPiU01BSUZu3It8+Lx1BOA1hOVgzJ+3A==; 31:dWa9gupFFBirMUnggQPCt6ef5ptPHuEKG7coRcmEavxYHPRHLoLtOeOGLt5M0pehDHse0oVvcV87yj+XWeRw6JE3mEiKWU1CVHtdcalXvqSQaphZ8fttyxHKMzpVKlfUAtY2Zlj2wfARWpEWxhHgHoUHjyZH+FfgwNhFGj0YQpjBXojmlmH7RTcVDQOZHs+9gWoKXAD3BFXaRguih8jVONbwpjUw3VbtmKa2IQI07+Y= X-MS-TrafficTypeDiagnostic: AM5PR0402MB2770: X-Microsoft-Exchange-Diagnostics: 1; AM5PR0402MB2770; 20:NqS7y3jCCnKRegrZfLDjJ62HNQ0xrFa7Jt0hBnCsce211ACqLwSmc/kTszOP3wpV2wTgWmD9grj2JYfHfTG9aiXbvE9YAUw05OwYHngBeSVIIwpX2ES8fZF5Wa3mMk9EZWKvQmaTegiT8bb8TEOqI2Ws/63swXRAhW0GRBzqOWKXITJI1zR6wcmAFcfTsj8xP44YnQB0ch0DpNEVDQFg7bwnrY1ClwbjGKDp7dXvxbUsPnO70sZDROWG91fFrGlc9Mv9YC2FoJhWjozHyxSxP6rP+xvItWdyE9HpTGK1BQbsIfOYBDDZGs+j36ev9z5twusdUaLoUGzPjDp2Smb5/rNTORlsp+IAC1Ur+bu6LEiDv4FYj0NHFuaPfbx1CxrELHWak58CKJpZ+2pwi3+6mZpU/r2pbxzOK6uOIXtqNE1V0QC9vQo0ntfwZ42AFvByMlk5+3uQGYK7UY22/t2asZndVAVk6/ucZdw9rgIuyuCBareRk/aObOsMKNf7oEBf; 4:/iMTXv4Uypgm7D683bvQmQ20FOR8NDYSoQEyAPhRSDqXdQAwuVobhC8/k06lqeocYt0z+p8vFayJHk6PRNrDL7G6yqNcrHoKQWUZEtYtXqj7/YOjNlHF8iwlnmMb9em75VWeyfnDRzLkGh+cQG5LXe/mEFXgyctUwNmIctd3GsiC3i4W+V3SdvG43FanoX6fVUBQoi+hrRObqKLzkFEkRdfch/ffflcV44CEvrKmNyZtAlbM4GfcyblgT3flqBblBRKynisOjpwjQrb27iddHb/AuKqSJ2ZhUBmD/9EwFXl7SmmQ4AQrmySMMmY5qbuNm78wYmkQhuSQVvT9yuH1BiwOo6L8HN5VXArE+cIZx7A= X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:(185117386973197)(228905959029699); X-Exchange-Antispam-Report-CFA-Test: BCL:0; PCL:0; RULEID:(8211001083)(6040522)(2401047)(8121501046)(5005006)(10201501046)(3231221)(944501327)(52105095)(3002001)(93006095)(93001095)(6055026)(6041310)(20161123564045)(201703131423095)(201702281528075)(20161123555045)(201703061421075)(201703061406153)(20161123560045)(20161123558120)(20161123562045)(6072148)(201708071742011); SRVR:AM5PR0402MB2770; BCL:0; PCL:0; RULEID:; SRVR:AM5PR0402MB2770; X-Forefront-PRVS: 0637FCE711 X-Forefront-Antispam-Report: SFV:NSPM; SFS:(10009020)(6049001)(39380400002)(376002)(346002)(396003)(39860400002)(366004)(199004)(189003)(58126008)(26005)(64126003)(50466002)(3846002)(16576012)(956004)(2870700001)(105586002)(6246003)(478600001)(36756003)(11346002)(316002)(2616005)(86362001)(446003)(31696002)(77096007)(8936002)(4326008)(97736004)(25786009)(6666003)(229853002)(16526019)(53936002)(2906002)(81156014)(81166006)(8676002)(305945005)(6916009)(66066001)(93886005)(5660300001)(106356001)(65826007)(47776003)(65956001)(5009440100003)(186003)(65806001)(55236004)(6486002)(6116002)(7736002)(386003)(59450400001)(476003)(68736007)(23676004)(52116002)(486006)(52146003)(53546011)(31686004)(2486003)(76176011)(67846002)(110426004); DIR:OUT; SFP:1101; SCL:1; SRVR:AM5PR0402MB2770; H:[10.232.14.39]; FPR:; SPF:None; LANG:en; PTR:InfoNoRecords; A:1; MX:1; Received-SPF: None (protection.outlook.com: nxp.com does not designate permitted sender hosts) X-Microsoft-Exchange-Diagnostics: =?utf-8?B?MTtBTTVQUjA0MDJNQjI3NzA7MjM6TlhuTm9uRFlqVjdicERTeGM2TGIyU3Z5?= =?utf-8?B?NmpLa01vM1lTWjQ1M2c2djRENXJOK2M3d2hVVDFOMzVQbWhXOTY4eW9iZEYz?= =?utf-8?B?dk8yeDd4akF4NzNXVzBzZ3JYVUpnRHJhV3RWNU4wZ0xqS2VhTUNWeDZOd1dp?= =?utf-8?B?OTNFWDYrZzZEYTNreDhwTnZxckJHd1BRbzlzeGtCTUQxcHdkaS9Lb09NUHE5?= =?utf-8?B?UTJScXNUNzlUdzNISXByeEdUdlp6TXE1Yk1KUmFWWXVyaFRCSWtJMkZ0dHJu?= =?utf-8?B?Zjh2bTVNRzU0Y0tFVGJRME5OakV3R0VnSmc0aGpRUFRVb1p4Y1dINklLUEVO?= =?utf-8?B?ZXhOSC9BR3lVMFExSUlNZWdNWHFoRnc5a2NVM3Q5VU82U1NtZW81WS9BSzRR?= =?utf-8?B?VGxXZU1ya1hKdG5heDZ2MW5YWUV4aDlqTmwwbk5wWDczVkxVcDdXaDNYSnJ0?= =?utf-8?B?K3EvYXVIZldYVXVyS3VOblU3UTVVOFJFOUQvV0hnVmw0bk1mMXlpeFQ5RkVp?= =?utf-8?B?Y3AweDh2MGNhc3ZRQUZzdTRJNStLUHUrV1ZBdWcyWlB1dE5HZG5PUGdYNTdv?= =?utf-8?B?cFRYY1hTZmJwV3BXNW1NbVVBTGF6TUpCUFg1enlEajUwVUhub3Rwa2swS1Fj?= =?utf-8?B?OFBFd1ZqazRnN09jZlFEM3BjOVRHUzA2WWJMdVk3ZW5YYXA0WWMzcUZTbmhU?= =?utf-8?B?U2RiZ05ybXh4cFE1a2YwdjJmbkNvd3dnay82Yk41aVJ4L3dkRFZBTXVsT24w?= =?utf-8?B?RC8vQ1pST1hGaHhBSVd3dS92MUNXVWVXM0M2akIvYjVjd1c2a0w2eElJbVdk?= =?utf-8?B?NDZHTjhGZ3dRYjRFL2hkT0dRNkFHS0J5ejVoMXF4V1E5dTdHVDhHa2J1S2tP?= =?utf-8?B?bVBSL05CSG1DcHVKNXl1UEZkamZpQlVrbzdZTERLcXRvbmpQbERvbFErbGY0?= =?utf-8?B?c0V5QnkreE01NVlRWXVmaGc3TWRQTWxCek9hVzJwL3NaNThYZExHZ0RHSlRo?= =?utf-8?B?S2d6Y3FMVGFFN0pHSkhyb2pjdnVaU040OVVxTHRNZ1ZOSlFHWXY3ZEVvZXg2?= =?utf-8?B?bFBERUxvcTNibCtkd1BmZUR1cFVzcWRqalNQNkZXZk1IYktKaCs1SGNJeU8w?= =?utf-8?B?V1U3THpzL1IzK3MwQmtxVC9MdlpHUFBudXlqNDJHMWllK255eTNDVXZHcWIv?= =?utf-8?B?WG40K1c5b3lZbGhaaW5iT2lnM3FXZWxiTUhQVkRCWlJzQXBPZEs0RlRFcnRk?= =?utf-8?B?Q3loaFRiZDEwb3k1OEluQmZvK2tUMDJHeURKcjZqcGNhU3hKNmVBRUJDMFpP?= =?utf-8?B?SjkwaGszTU1tbDNZK2ZJOW5tbm15ZWEyQXh2UmVaQlhvZmx4YktxclJ5ekJU?= =?utf-8?B?dFZPcnhRV04wT2lIaTJnQUNucXVaNy94dk4xcjBHOWdRaW4ydjF2aVFmNThH?= =?utf-8?B?ZUtEY0l6LzcxZkQzR1B1U29henhqOEJURVY2WE5WWGNkMXFFM3R6NWw4aldh?= =?utf-8?B?eDRmTXVpc1NWeUhxNUU0YmYrcTV5U2wwNFhqMkNiaXJIMEdoRGNWdHdDaFR1?= =?utf-8?B?RXZSU0NtSSt0ME9TM1REVmMyZStlT3ZkSDk2WWVYdGNLN1dMSEltaXlCaS80?= =?utf-8?B?bSsrbVR4ZUhEWWdGK1B3Y1NmRURkQThlUzluUVdYZGJ6c2xrV2hVdFdPNVlC?= =?utf-8?B?U0hkTXpleTRjbzA5SktvRVRSV0NuWDZxWGpFbEtIMUZwTjMrMDdvTkhUczhp?= =?utf-8?B?UmtyNzh4aXBsdlBObWpJSytVbDB4V0ZoOVdwNld6UGFLdm9IaHMzUnZyZEpX?= =?utf-8?B?Q3VJN2lDemFLNWQ3RzRWUnJjTVQyMi9pYzVPVWV2SEFPOWRzRHZMQ1JsM1Zq?= =?utf-8?B?SlQzcDlvWVByYjlZZmozZjBmbHU2bUhDZ1ZUMEQvUU9yMUV3V1JkejNjeTkr?= =?utf-8?B?dnJiek1iZzhKMDBlTGlNaVJYRzRodkRwTWdENkVTbFFqZXg0WitNN29CV1lH?= =?utf-8?B?NjhlaGl2WWM2WDNwZ3ZPanRBZCsvVzhkem9tT2QzRHd6UmRpZXk2RitJVkJs?= =?utf-8?B?dzNPWERiUVREY1FVbHU5UDdyZHVhWG8wOTZJbzZOSUtwSTB2bHBLaXZvK2dn?= =?utf-8?Q?uDZnBPp+6Rqdwi8hAaNaLQvEY=3D?= X-Microsoft-Antispam-Message-Info: GYfiyMkZdk0WP8/DqdOnuizIBkyPVKBiFf25MtSnyMqAxOoBuJOe5b2PnpDxddna89drDqyVKV382Ok43f/bjohfTwLfHnHMJvmGIBiNrKxFhRug69Jl2sFEFUP+FI0mC4X9if17zk1NLM7CdOP2cWNugWybg5n3jqEU3q7eq46lLwBqypQyhUztYsOdzKL+ X-Microsoft-Exchange-Diagnostics: 1; AM5PR0402MB2770; 6:DGd8zubypvjcDe+5XbE38BZjGWN3kh9mzXdoDd5Mh9pVXi1u5YFt1g97m4UUYukGphs/3rukAJmhnQY6nRJiQAVKU6eTjxX1DyUWhWV8fvoyhT8BaEgksoOAoW57EZqthDCBXjdjSVQOmRi1jxkbFjx4vkqa3LWGnimTWmxPw34Dnuqf1wnX4gyWmayltYFQdloOFAhoxsVOv15FOHdR++4r14MtMLgxVWd0jo5Oh1EJT5zjCsXjA1Sk7CbBhAWbGTw+3LRwNN8zeXVeljbFXgF1IfWXol0O14KvMa8rtDCtqPjPBj5ZFTfZ0WQ9BDHc1ABWuNGEDfJ/MISShgAEDFuGvHnbZjea0I4fwr608tpUrvWOMvoayQ3bfA8l493QySsL+uXdqywAW8Bw8qhUGow/kWkP1UIq51fCaFBdEa1rlqZAhvtFt+s1+9xjQ8KLIfkPJ946dpOAYjHO54syCA==; 5:qWh6hsB2dYC1WxOyhjhbd3r3BsqPz5hKZlYSAGnS0DUVTvCn7DwJy/TATwsEobcDOgVMxJKnR2Y5sjtXnSQLZ4gXocpRMyKHHhkueT7iyGAqe2YYgZRnJ/DLFlJtvNR5Zte84nwQwBzhRtYk0isK5XedKY5fetWTZ8rMxRH0I0M=; 24:5zABZ8+r3H24NNfYoBRDWV5Gq6wI5hshMEZl8CToGE4dy4qR2/neSl/jLzoN6FfO3e+7bj8tAiFW1EbKca4F3gVwMBscJNe2qdvExc5DqEs= SpamDiagnosticOutput: 1:99 SpamDiagnosticMetadata: NSPM X-Microsoft-Exchange-Diagnostics: 1; AM5PR0402MB2770; 7:dvdjp4WWd+C8wNDNzbCdMGVm2UnM76gbuNV08QBVnL6LtupAaVINK3t7PBJElbKvpIraGzH7hqsU/BhWeoJs0nj3JHkBdXYxZ2q9P2AkzZv7zi+IKPnkK3MrNsJAfi0MgvsnFBgeXbMhBypzZuk+ydpetrG0ZuuuRtVKSfCoY/ZYSyFyLFJLZn3+SMDCNvVeptA/66FLufk4kUbT7TfVnme23ead7VL8H8ievPCTSCVX0UH9iIYyQ8k2x6QwF5ZR X-OriginatorOrg: nxp.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 09 Apr 2018 11:54:41.1879 (UTC) X-MS-Exchange-CrossTenant-Network-Message-Id: 984d3d81-e110-4b71-fa9f-08d59e10ae51 X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635 X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM5PR0402MB2770 Subject: Re: [dpdk-dev] [PATCH v4 66/70] bus/fslmc: enable support for mem event callbacks for vfio 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: Mon, 09 Apr 2018 11:54:44 -0000 On Monday 09 April 2018 04:25 PM, Burakov, Anatoly wrote: > On 09-Apr-18 11:01 AM, Shreyansh Jain wrote: >> Hi Anatoly, >> >> On Monday 09 April 2018 01:48 AM, Anatoly Burakov wrote: >>> VFIO needs to map and unmap segments for DMA whenever they >>> become available or unavailable, so register a callback for >>> memory events, and provide map/unmap functions. >>> >>> Signed-off-by: Shreyansh Jain >>> Signed-off-by: Anatoly Burakov >>> --- > > <...> > >>> +        DPAA2_BUS_DEBUG("Calling with type=%d, va=%p, virt_addr=0x%" >>> PRIx64 ", iova=0x%" PRIx64 ", map_len=%zu\n", >> >> I would like to correct this message (80char + rewording) - What do >> you suggest? Should I send a new patch to you or just convey what >> should be changed? >> > > As far as i know, leaving strings on single line is good for grepping. > However, perhaps having PRIx64 etc in there breaks it anyway. Yes, that and the debug message was not helpful. This is what I had in mind. (DPAA2_BUS_DEBUG doesn't require an extra \n) DPAA2_BUS_DEBUG("Request for %s, va=%p, virt_addr=0x%" PRIx64 "," "iova=0x%" PRIx64 ", map_len=%zu", type == RTE_MEM_EVENT_ALLOC? "alloc" : "dealloc", va, virt_addr, iova_addr, map_len); > >>> +                type, va, virt_addr, iova_addr, map_len); >>> + >>> +        if (type == RTE_MEM_EVENT_ALLOC) >>> +            ret = fslmc_map_dma(virt_addr, iova_addr, map_len); >>> +        else >>> +            ret = fslmc_unmap_dma(virt_addr, iova_addr, map_len); >>> + >>> +        if (ret != 0) { >>> +            DPAA2_BUS_ERR("DMA Mapping/Unmapping failed. Map=%d, >>> addr=%p, len=%zu, err:(%d)", >>> +                    type, va, map_len, ret); >> >> Same as above. 80 Char issue. > > Same reasoning - leaving strings unbroken allows for easier grepping the > codebase, but i'm not sure what's our policy on having formatted strings > unbroken. My policy is not different, but the various variables being dumped cannot anyway help in grepping - So, keeping the variables on separate lines for 80chars is ok. "DMA Mapping/Unmapping failed." is enough for greps. > >> >>> +            return; >>> +        } >>> + >>> +        cur_len += map_len; >>> +    } >>> + >>> +    if (type == RTE_MEM_EVENT_ALLOC) >>> +        DPAA2_BUS_DEBUG("Total Mapped: addr=%p, len=%zu\n", >>> +                addr, len); >>> +    else > > <...> > >>> +    ret = rte_mem_event_callback_register("fslmc_memevent_clb", >>> +                          fslmc_memevent_cb); >>> +    if (ret && rte_errno == ENOTSUP) >>> +        DPAA2_BUS_DEBUG("Memory event callbacks not supported"); >>> +    else if (ret) >>> +        DPAA2_BUS_DEBUG("Unable to install memory handler"); >>> +    else >>> +        DPAA2_BUS_DEBUG("Installed memory callback handler"); >>>       /* Verifying that at least single segment is available */ >>>       if (i <= 0) { >>> +        /* TODO: Is this verification required any more? What would >>> +         * happen to non-legacy case where nothing was preallocated >>> +         * thus causing i==0? >>> +         */ >> >> And this as well - if call backs are not going to appear in case of >> legacy, this needs to be removed. > > Callbacks aren't only not going to appear in legacy mode - they will > also not appear on FreeBSD. We check this above, with checking rte_errno > value (if callbacks are not supported, it's set to ENOTSUP, and having > callbacks unsupported is not an error). > >> let me know how do you want to take these changes. >> > > Now that i think of it, this error condition is wrong. This is called in > both legacy and non-legacy mode. This is bus probe, no? For non-legacy > mode, it is entirely possible to start without any memory whatsoever. It > just so happens that rte_service API allocates some on init, and hence > you always have at least one segment - that may not be the case forever. > So, non-legacy mode, not having memsegs is not an error, it is expected > behavior, so maybe we should remove this error check altogether. Agree - that count was only required in the earlier case. It can be removed. > >>>           DPAA2_BUS_ERR("No Segments found for VFIO Mapping"); >>> +        rte_rwlock_read_unlock(mem_lock); >>>           return -1; >>>       } >>>       DPAA2_BUS_DEBUG("Total %d segments found.", i); >>> @@ -250,6 +367,11 @@ int rte_fslmc_vfio_dmamap(void) >>>        */ >>>       vfio_map_irq_region(&vfio_group); >>> +    /* Existing segments have been mapped and memory callback for >>> hotplug >>> +     * has been installed. >>> +     */ >>> +    rte_rwlock_read_unlock(mem_lock); >>> + >>>       return 0; >>>   } >>> >> >> > > I think there are enough changes, even if trivial. Maybe I can rework this patch and send you. If that is inconvenient, just extract from that whatever you want.