| Message ID | 1423637385-25077-1-git-send-email-xuelin.shi@freescale.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers |
Return-Path: <dev-bounces@dpdk.org> X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [IPv6:::1]) by dpdk.org (Postfix) with ESMTP id 0E34B5A51; Wed, 11 Feb 2015 08:54:38 +0100 (CET) Received: from na01-bn1-obe.outbound.protection.outlook.com (mail-bn1bon0136.outbound.protection.outlook.com [157.56.111.136]) by dpdk.org (Postfix) with ESMTP id F3CC42C7A for <dev@dpdk.org>; Wed, 11 Feb 2015 08:54:36 +0100 (CET) Received: from BN3PR0301CA0023.namprd03.prod.outlook.com (25.160.180.161) by DM2PR0301MB0686.namprd03.prod.outlook.com (25.160.96.24) with Microsoft SMTP Server (TLS) id 15.1.65.19; Wed, 11 Feb 2015 07:54:35 +0000 Received: from BN1AFFO11FD043.protection.gbl (2a01:111:f400:7c10::159) by BN3PR0301CA0023.outlook.office365.com (2a01:111:e400:4000::33) with Microsoft SMTP Server (TLS) id 15.1.87.13 via Frontend Transport; Wed, 11 Feb 2015 07:54:34 +0000 Received: from az84smr01.freescale.net (192.88.158.2) by BN1AFFO11FD043.mail.protection.outlook.com (10.58.52.190) with Microsoft SMTP Server (TLS) id 15.1.87.10 via Frontend Transport; Wed, 11 Feb 2015 07:54:34 +0000 Received: from localhost (rock.ap.freescale.net [10.193.20.106]) by az84smr01.freescale.net (8.14.3/8.14.0) with ESMTP id t1B7sVWp027550; Wed, 11 Feb 2015 00:54:32 -0700 From: <xuelin.shi@freescale.com> To: <thomas.monjalon@6wind.com> Date: Wed, 11 Feb 2015 14:49:45 +0800 Message-ID: <1423637385-25077-1-git-send-email-xuelin.shi@freescale.com> X-Mailer: git-send-email 1.8.4 X-EOPAttributedMessage: 0 Received-SPF: Fail (protection.outlook.com: domain of freescale.com does not designate 192.88.158.2 as permitted sender) receiver=protection.outlook.com; client-ip=192.88.158.2; helo=az84smr01.freescale.net; Authentication-Results: spf=fail (sender IP is 192.88.158.2) smtp.mailfrom=b29237@freescale.com; freescale.mail.onmicrosoft.com; dkim=none (message not signed) header.d=none; X-Forefront-Antispam-Report: CIP:192.88.158.2; CTRY:US; IPV:NLI; EFV:NLI; SFV:NSPM; SFS:(10019020)(6009001)(339900001)(6806004)(77096005)(86362001)(105606002)(50986999)(2351001)(106466001)(62966003)(57986006)(33646002)(76506005)(92566002)(87936001)(104016003)(47776003)(229853001)(86152002)(50466002)(77156002)(46102003)(48376002)(19580395003)(19580405001)(110136001)(50226001)(85426001)(36756003); DIR:OUT; SFP:1102; SCL:1; SRVR:DM2PR0301MB0686; H:az84smr01.freescale.net; FPR:; SPF:Fail; MLV:sfv; LANG:en; MIME-Version: 1.0 Content-Type: text/plain X-Microsoft-Antispam: UriScan:; X-Microsoft-Antispam: BCL:0;PCL:0;RULEID:;SRVR:DM2PR0301MB0686; X-Exchange-Antispam-Report-Test: UriScan:; X-Exchange-Antispam-Report-CFA-Test: BCL:0; PCL:0; RULEID:(601004); SRVR:DM2PR0301MB0686; X-Forefront-PRVS: 0484063412 X-Exchange-Antispam-Report-CFA-Test: BCL:0; PCL:0; RULEID:; SRVR:DM2PR0301MB0686; X-OriginatorOrg: freescale.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 11 Feb 2015 07:54:34.0870 (UTC) X-MS-Exchange-CrossTenant-Id: 710a03f5-10f6-4d38-9ff4-a80b81da590d X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=710a03f5-10f6-4d38-9ff4-a80b81da590d; Ip=[192.88.158.2] X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM2PR0301MB0686 Cc: dev@dpdk.org Subject: [dpdk-dev] [PATCH] kni/ethtool/ixgbe: enforce access between ixgbe PCI and CPU X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK <dev.dpdk.org> List-Unsubscribe: <http://dpdk.org/ml/options/dev>, <mailto:dev-request@dpdk.org?subject=unsubscribe> List-Archive: <http://dpdk.org/ml/archives/dev/> List-Post: <mailto:dev@dpdk.org> List-Help: <mailto:dev-request@dpdk.org?subject=help> List-Subscribe: <http://dpdk.org/ml/listinfo/dev>, <mailto:dev-request@dpdk.org?subject=subscribe> Errors-To: dev-bounces@dpdk.org Sender: "dev" <dev-bounces@dpdk.org> |
Commit Message
xuelin.shi@freescale.com
Feb. 11, 2015, 6:49 a.m. UTC
From: Xuelin Shi <xuelin.shi@freescale.com> make sure: CPU read from ixgbe with IXGBE_LE32_TO_CPUS CPU write to ixgbe with IXGBE_CPU_TO_LE32 otherwise, there is endian issue for ixgbe on BIG_ENDIAN CPU. Signed-off-by: Xuelin Shi <xuelin.shi@freescale.com> --- .../linuxapp/kni/ethtool/ixgbe/ixgbe_osdep.h | 24 ++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-)
Comments
Anyone to review this patch? 2015-02-11 14:49, xuelin.shi@freescale.com: > From: Xuelin Shi <xuelin.shi@freescale.com> > > make sure: > CPU read from ixgbe with IXGBE_LE32_TO_CPUS > CPU write to ixgbe with IXGBE_CPU_TO_LE32 > > otherwise, there is endian issue for ixgbe on BIG_ENDIAN CPU. > > Signed-off-by: Xuelin Shi <xuelin.shi@freescale.com> > --- > .../linuxapp/kni/ethtool/ixgbe/ixgbe_osdep.h | 24 ++++++++++++++++------ > 1 file changed, 18 insertions(+), 6 deletions(-) > > diff --git a/lib/librte_eal/linuxapp/kni/ethtool/ixgbe/ixgbe_osdep.h b/lib/librte_eal/linuxapp/kni/ethtool/ixgbe/ixgbe_osdep.h > index d161600..0612632 100644 > --- a/lib/librte_eal/linuxapp/kni/ethtool/ixgbe/ixgbe_osdep.h > +++ b/lib/librte_eal/linuxapp/kni/ethtool/ixgbe/ixgbe_osdep.h > @@ -53,6 +53,16 @@ > > #undef ASSERT > > +static inline uint32_t ixgbe_read_addr(volatile void* addr) > +{ > + return IXGBE_LE32_TO_CPUS(*((volatile uint32_t *)addr)); > +} > + > +static inline uint32_t ixgbe_write_addr(u32 value, volatile void* addr) > +{ > + return writel(IXGBE_CPU_TO_LE32(value), addr); > +} > + > #ifdef DBG > #define hw_dbg(hw, S, A...) printk(KERN_DEBUG S, ## A) > #else > @@ -91,19 +101,20 @@ > default: \ > break; \ > } \ > - writel((value), ((a)->hw_addr + (reg))); \ > + ixgbe_write_addr((value), ((a)->hw_addr + (reg))); \ > } while (0) > #else > -#define IXGBE_WRITE_REG(a, reg, value) writel((value), ((a)->hw_addr + (reg))) > +#define IXGBE_WRITE_REG(a, reg, value) \ > + ixgbe_write_addr((value), ((a)->hw_addr + (reg))) > #endif > > -#define IXGBE_READ_REG(a, reg) readl((a)->hw_addr + (reg)) > +#define IXGBE_READ_REG(a, reg) ixgbe_read_addr((a)->hw_addr + (reg)) > > #define IXGBE_WRITE_REG_ARRAY(a, reg, offset, value) ( \ > - writel((value), ((a)->hw_addr + (reg) + ((offset) << 2)))) > + ixgbe_write_addr((value), ((a)->hw_addr + (reg) + ((offset) << 2)))) > > #define IXGBE_READ_REG_ARRAY(a, reg, offset) ( \ > - readl((a)->hw_addr + (reg) + ((offset) << 2))) > + ixgbe_read_addr((a)->hw_addr + (reg) + ((offset) << 2))) > > #ifndef writeq > #define writeq(val, addr) do { writel((u32) (val), addr); \ > @@ -111,7 +122,8 @@ > } while (0); > #endif > > -#define IXGBE_WRITE_REG64(a, reg, value) writeq((value), ((a)->hw_addr + (reg))) > +#define IXGBE_WRITE_REG64(a, reg, value) \ > + writeq((cpu_to_le64(value)), ((a)->hw_addr + (reg))) > > #define IXGBE_WRITE_FLUSH(a) IXGBE_READ_REG(a, IXGBE_STATUS) > struct ixgbe_hw; >
Helin, any opinion? If nothing wrong is seen, it will be merged in few days. 2015-02-20 11:55, Thomas Monjalon: > Anyone to review this patch? > > 2015-02-11 14:49, xuelin.shi@freescale.com: > > From: Xuelin Shi <xuelin.shi@freescale.com> > > > > make sure: > > CPU read from ixgbe with IXGBE_LE32_TO_CPUS > > CPU write to ixgbe with IXGBE_CPU_TO_LE32 > > > > otherwise, there is endian issue for ixgbe on BIG_ENDIAN CPU. > > > > Signed-off-by: Xuelin Shi <xuelin.shi@freescale.com> > > --- > > .../linuxapp/kni/ethtool/ixgbe/ixgbe_osdep.h | 24 ++++++++++++++++------ > > 1 file changed, 18 insertions(+), 6 deletions(-) > > > > diff --git a/lib/librte_eal/linuxapp/kni/ethtool/ixgbe/ixgbe_osdep.h b/lib/librte_eal/linuxapp/kni/ethtool/ixgbe/ixgbe_osdep.h > > index d161600..0612632 100644 > > --- a/lib/librte_eal/linuxapp/kni/ethtool/ixgbe/ixgbe_osdep.h > > +++ b/lib/librte_eal/linuxapp/kni/ethtool/ixgbe/ixgbe_osdep.h > > @@ -53,6 +53,16 @@ > > > > #undef ASSERT > > > > +static inline uint32_t ixgbe_read_addr(volatile void* addr) > > +{ > > + return IXGBE_LE32_TO_CPUS(*((volatile uint32_t *)addr)); > > +} > > + > > +static inline uint32_t ixgbe_write_addr(u32 value, volatile void* addr) > > +{ > > + return writel(IXGBE_CPU_TO_LE32(value), addr); > > +} > > + > > #ifdef DBG > > #define hw_dbg(hw, S, A...) printk(KERN_DEBUG S, ## A) > > #else > > @@ -91,19 +101,20 @@ > > default: \ > > break; \ > > } \ > > - writel((value), ((a)->hw_addr + (reg))); \ > > + ixgbe_write_addr((value), ((a)->hw_addr + (reg))); \ > > } while (0) > > #else > > -#define IXGBE_WRITE_REG(a, reg, value) writel((value), ((a)->hw_addr + (reg))) > > +#define IXGBE_WRITE_REG(a, reg, value) \ > > + ixgbe_write_addr((value), ((a)->hw_addr + (reg))) > > #endif > > > > -#define IXGBE_READ_REG(a, reg) readl((a)->hw_addr + (reg)) > > +#define IXGBE_READ_REG(a, reg) ixgbe_read_addr((a)->hw_addr + (reg)) > > > > #define IXGBE_WRITE_REG_ARRAY(a, reg, offset, value) ( \ > > - writel((value), ((a)->hw_addr + (reg) + ((offset) << 2)))) > > + ixgbe_write_addr((value), ((a)->hw_addr + (reg) + ((offset) << 2)))) > > > > #define IXGBE_READ_REG_ARRAY(a, reg, offset) ( \ > > - readl((a)->hw_addr + (reg) + ((offset) << 2))) > > + ixgbe_read_addr((a)->hw_addr + (reg) + ((offset) << 2))) > > > > #ifndef writeq > > #define writeq(val, addr) do { writel((u32) (val), addr); \ > > @@ -111,7 +122,8 @@ > > } while (0); > > #endif > > > > -#define IXGBE_WRITE_REG64(a, reg, value) writeq((value), ((a)->hw_addr + (reg))) > > +#define IXGBE_WRITE_REG64(a, reg, value) \ > > + writeq((cpu_to_le64(value)), ((a)->hw_addr + (reg))) > > > > #define IXGBE_WRITE_FLUSH(a) IXGBE_READ_REG(a, IXGBE_STATUS) > > struct ixgbe_hw; > > > >
Hi Xuelin > -----Original Message----- > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of > xuelin.shi@freescale.com > Sent: Wednesday, February 11, 2015 2:50 PM > To: thomas.monjalon@6wind.com > Cc: dev@dpdk.org > Subject: [dpdk-dev] [PATCH] kni/ethtool/ixgbe: enforce access between ixgbe > PCI and CPU > > From: Xuelin Shi <xuelin.shi@freescale.com> > > make sure: > CPU read from ixgbe with IXGBE_LE32_TO_CPUS > CPU write to ixgbe with IXGBE_CPU_TO_LE32 > > otherwise, there is endian issue for ixgbe on BIG_ENDIAN CPU. I got your concern, but you modified in wrong places. Source files in linux/kni/ will be compiled into a kernel module. So the endian issue will be handled quite well by kernel functions like writel, readl, etc. And your modifications are not needed at all for KNI kernel module. But I think the similar changes are needed in librte_pmd_e1000, librte_pmd_ixgbe, librte_pmd_i40e, etc. Regards, Helin > > Signed-off-by: Xuelin Shi <xuelin.shi@freescale.com> > --- > .../linuxapp/kni/ethtool/ixgbe/ixgbe_osdep.h | 24 > ++++++++++++++++------ > 1 file changed, 18 insertions(+), 6 deletions(-) > > diff --git a/lib/librte_eal/linuxapp/kni/ethtool/ixgbe/ixgbe_osdep.h > b/lib/librte_eal/linuxapp/kni/ethtool/ixgbe/ixgbe_osdep.h > index d161600..0612632 100644 > --- a/lib/librte_eal/linuxapp/kni/ethtool/ixgbe/ixgbe_osdep.h > +++ b/lib/librte_eal/linuxapp/kni/ethtool/ixgbe/ixgbe_osdep.h > @@ -53,6 +53,16 @@ > > #undef ASSERT > > +static inline uint32_t ixgbe_read_addr(volatile void* addr) { > + return IXGBE_LE32_TO_CPUS(*((volatile uint32_t *)addr)); } > + > +static inline uint32_t ixgbe_write_addr(u32 value, volatile void* addr) > +{ > + return writel(IXGBE_CPU_TO_LE32(value), addr); } > + > #ifdef DBG > #define hw_dbg(hw, S, A...) printk(KERN_DEBUG S, ## A) > #else > @@ -91,19 +101,20 @@ > default: \ > break; \ > } \ > - writel((value), ((a)->hw_addr + (reg))); \ > + ixgbe_write_addr((value), ((a)->hw_addr + (reg))); \ > } while (0) > #else > -#define IXGBE_WRITE_REG(a, reg, value) writel((value), ((a)->hw_addr + > (reg))) > +#define IXGBE_WRITE_REG(a, reg, value) \ > + ixgbe_write_addr((value), ((a)->hw_addr + (reg))) > #endif > > -#define IXGBE_READ_REG(a, reg) readl((a)->hw_addr + (reg)) > +#define IXGBE_READ_REG(a, reg) ixgbe_read_addr((a)->hw_addr + (reg)) > > #define IXGBE_WRITE_REG_ARRAY(a, reg, offset, value) ( \ > - writel((value), ((a)->hw_addr + (reg) + ((offset) << 2)))) > + ixgbe_write_addr((value), ((a)->hw_addr + (reg) + ((offset) << 2)))) > > #define IXGBE_READ_REG_ARRAY(a, reg, offset) ( \ > - readl((a)->hw_addr + (reg) + ((offset) << 2))) > + ixgbe_read_addr((a)->hw_addr + (reg) + ((offset) << 2))) > > #ifndef writeq > #define writeq(val, addr) do { writel((u32) (val), addr); \ > @@ -111,7 +122,8 @@ > } while (0); > #endif > > -#define IXGBE_WRITE_REG64(a, reg, value) writeq((value), ((a)->hw_addr + > (reg))) > +#define IXGBE_WRITE_REG64(a, reg, value) \ > + writeq((cpu_to_le64(value)), ((a)->hw_addr + (reg))) > > #define IXGBE_WRITE_FLUSH(a) IXGBE_READ_REG(a, IXGBE_STATUS) > struct ixgbe_hw; > -- > 1.9.1
> -----Original Message----- > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of > xuelin.shi@freescale.com > Sent: Wednesday, February 11, 2015 2:50 PM > To: thomas.monjalon@6wind.com > Cc: dev@dpdk.org > Subject: [dpdk-dev] [PATCH] kni/ethtool/ixgbe: enforce access between ixgbe > PCI and CPU > > From: Xuelin Shi <xuelin.shi@freescale.com> > > make sure: > CPU read from ixgbe with IXGBE_LE32_TO_CPUS > CPU write to ixgbe with IXGBE_CPU_TO_LE32 > > otherwise, there is endian issue for ixgbe on BIG_ENDIAN CPU. > > Signed-off-by: Xuelin Shi <xuelin.shi@freescale.com> NACK, see my comments @ http://www.dpdk.org/ml/archives/dev/2015-March/015640.html > --- > .../linuxapp/kni/ethtool/ixgbe/ixgbe_osdep.h | 24 > ++++++++++++++++------ > 1 file changed, 18 insertions(+), 6 deletions(-) > > diff --git a/lib/librte_eal/linuxapp/kni/ethtool/ixgbe/ixgbe_osdep.h > b/lib/librte_eal/linuxapp/kni/ethtool/ixgbe/ixgbe_osdep.h > index d161600..0612632 100644 > --- a/lib/librte_eal/linuxapp/kni/ethtool/ixgbe/ixgbe_osdep.h > +++ b/lib/librte_eal/linuxapp/kni/ethtool/ixgbe/ixgbe_osdep.h > @@ -53,6 +53,16 @@ > > #undef ASSERT > > +static inline uint32_t ixgbe_read_addr(volatile void* addr) { > + return IXGBE_LE32_TO_CPUS(*((volatile uint32_t *)addr)); } > + > +static inline uint32_t ixgbe_write_addr(u32 value, volatile void* addr) > +{ > + return writel(IXGBE_CPU_TO_LE32(value), addr); } > + > #ifdef DBG > #define hw_dbg(hw, S, A...) printk(KERN_DEBUG S, ## A) > #else > @@ -91,19 +101,20 @@ > default: \ > break; \ > } \ > - writel((value), ((a)->hw_addr + (reg))); \ > + ixgbe_write_addr((value), ((a)->hw_addr + (reg))); \ > } while (0) > #else > -#define IXGBE_WRITE_REG(a, reg, value) writel((value), ((a)->hw_addr + (reg))) > +#define IXGBE_WRITE_REG(a, reg, value) \ > + ixgbe_write_addr((value), ((a)->hw_addr + (reg))) > #endif > > -#define IXGBE_READ_REG(a, reg) readl((a)->hw_addr + (reg)) > +#define IXGBE_READ_REG(a, reg) ixgbe_read_addr((a)->hw_addr + (reg)) > > #define IXGBE_WRITE_REG_ARRAY(a, reg, offset, value) ( \ > - writel((value), ((a)->hw_addr + (reg) + ((offset) << 2)))) > + ixgbe_write_addr((value), ((a)->hw_addr + (reg) + ((offset) << 2)))) > > #define IXGBE_READ_REG_ARRAY(a, reg, offset) ( \ > - readl((a)->hw_addr + (reg) + ((offset) << 2))) > + ixgbe_read_addr((a)->hw_addr + (reg) + ((offset) << 2))) > > #ifndef writeq > #define writeq(val, addr) do { writel((u32) (val), addr); \ > @@ -111,7 +122,8 @@ > } while (0); > #endif > > -#define IXGBE_WRITE_REG64(a, reg, value) writeq((value), ((a)->hw_addr + > (reg))) > +#define IXGBE_WRITE_REG64(a, reg, value) \ > + writeq((cpu_to_le64(value)), ((a)->hw_addr + (reg))) > > #define IXGBE_WRITE_FLUSH(a) IXGBE_READ_REG(a, IXGBE_STATUS) struct > ixgbe_hw; > -- > 1.9.1
diff --git a/lib/librte_eal/linuxapp/kni/ethtool/ixgbe/ixgbe_osdep.h b/lib/librte_eal/linuxapp/kni/ethtool/ixgbe/ixgbe_osdep.h index d161600..0612632 100644 --- a/lib/librte_eal/linuxapp/kni/ethtool/ixgbe/ixgbe_osdep.h +++ b/lib/librte_eal/linuxapp/kni/ethtool/ixgbe/ixgbe_osdep.h @@ -53,6 +53,16 @@ #undef ASSERT +static inline uint32_t ixgbe_read_addr(volatile void* addr) +{ + return IXGBE_LE32_TO_CPUS(*((volatile uint32_t *)addr)); +} + +static inline uint32_t ixgbe_write_addr(u32 value, volatile void* addr) +{ + return writel(IXGBE_CPU_TO_LE32(value), addr); +} + #ifdef DBG #define hw_dbg(hw, S, A...) printk(KERN_DEBUG S, ## A) #else @@ -91,19 +101,20 @@ default: \ break; \ } \ - writel((value), ((a)->hw_addr + (reg))); \ + ixgbe_write_addr((value), ((a)->hw_addr + (reg))); \ } while (0) #else -#define IXGBE_WRITE_REG(a, reg, value) writel((value), ((a)->hw_addr + (reg))) +#define IXGBE_WRITE_REG(a, reg, value) \ + ixgbe_write_addr((value), ((a)->hw_addr + (reg))) #endif -#define IXGBE_READ_REG(a, reg) readl((a)->hw_addr + (reg)) +#define IXGBE_READ_REG(a, reg) ixgbe_read_addr((a)->hw_addr + (reg)) #define IXGBE_WRITE_REG_ARRAY(a, reg, offset, value) ( \ - writel((value), ((a)->hw_addr + (reg) + ((offset) << 2)))) + ixgbe_write_addr((value), ((a)->hw_addr + (reg) + ((offset) << 2)))) #define IXGBE_READ_REG_ARRAY(a, reg, offset) ( \ - readl((a)->hw_addr + (reg) + ((offset) << 2))) + ixgbe_read_addr((a)->hw_addr + (reg) + ((offset) << 2))) #ifndef writeq #define writeq(val, addr) do { writel((u32) (val), addr); \ @@ -111,7 +122,8 @@ } while (0); #endif -#define IXGBE_WRITE_REG64(a, reg, value) writeq((value), ((a)->hw_addr + (reg))) +#define IXGBE_WRITE_REG64(a, reg, value) \ + writeq((cpu_to_le64(value)), ((a)->hw_addr + (reg))) #define IXGBE_WRITE_FLUSH(a) IXGBE_READ_REG(a, IXGBE_STATUS) struct ixgbe_hw;