From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757178AbcBIOPv (ORCPT ); Tue, 9 Feb 2016 09:15:51 -0500 Received: from smtp-out6.electric.net ([192.162.217.191]:59918 "EHLO smtp-out6.electric.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756785AbcBIOPs convert rfc822-to-8bit (ORCPT ); Tue, 9 Feb 2016 09:15:48 -0500 From: David Laight To: "'Grygorii Strashko'" , "netdev@vger.kernel.org" , "David S . Miller" , "Arnd Bergmann" CC: Franklin S Cooper Jr , Sekhar Nori , "linux-kernel@vger.kernel.org" , Wingman Kwok , Murali Karicheri , Mugunthan V N Subject: RE: [PATCH] net: ti: netcp: restore get/set_pad_info() functionality Thread-Topic: [PATCH] net: ti: netcp: restore get/set_pad_info() functionality Thread-Index: AQHRY0GgM/S/JgIrAEmSoNJDdoEqe58jvybQ Date: Tue, 9 Feb 2016 14:13:04 +0000 Message-ID: <063D6719AE5E284EB5DD2968C1650D6D1CCDBFCC@AcuExch.aculab.com> References: <1455026303-17014-1-git-send-email-grygorii.strashko@ti.com> In-Reply-To: <1455026303-17014-1-git-send-email-grygorii.strashko@ti.com> Accept-Language: en-GB, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.202.99.200] Content-Type: text/plain; charset="Windows-1252" Content-Transfer-Encoding: 8BIT MIME-Version: 1.0 X-Outbound-IP: 213.249.233.130 X-Env-From: David.Laight@ACULAB.COM X-PolicySMART: 3396946, 3397078 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org From: Grygorii Strashko > Sent: 09 February 2016 13:58 > From: Arnd Bergmann > > The commit 899077791403 ("netcp: try to reduce type confusion in descriptors") > introduces a regression in Kernel 4.5-rc1 and it breaks > get/set_pad_info() functionality. > > The TI NETCP driver uses pad0 and pad1 fields of knav_dma_desc to > store DMA/MEM buffer pointer and buffer size respectively. And in both > cases for Keystone 2 the pointer type size is 32 bit regardless of > LAPE enabled or not, because CONFIG_ARCH_DMA_ADDR_T_64BIT originally > is not expected to be defined. > > !LAPE LPAE > sizeof(void*) 32bit 32bit > sizeof(dma_addr_t) 32bit 32bit > sizeof(phys_addr_t) 32bit 64bit > > Unfortunately, above commit changed buffer's pointers save/restore > code (get/set_pad_info()) and added intermediate conversation to u64 > which works incorrectly on 32bit Keystone 2 and causes TI NETCP driver > crash in RX/TX path due to "Unable to handle kernel NULL pointer" > exception. This issue was reported and discussed in [1]. > > Hence, fix it by partially reverting above commit and restoring > get/set_pad_info() functionality as it was before. You should really get rid of most of the horrid pointer-integer casts. Code like: > void *buf_ptr; ... > + get_pad_info((u32 *)&buf_ptr, &buf_len, ndesc); is just asking for trouble. You'd be better using assignments like: buf_ptr = (cast)get_pad_0(ndesc); buf_len = get_pad_1(ndesc); Then the values are passed (and cast) as numerics. In reality the 'pad' fields ought to be renamed - since they aren't pads. Perhaps they should be a union? David