DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] kni/ethtool/ixgbe: enforce access between ixgbe PCI and CPU
@ 2015-02-11  6:49 xuelin.shi
  2015-02-20 10:55 ` Thomas Monjalon
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: xuelin.shi @ 2015-02-11  6:49 UTC (permalink / raw)
  To: thomas.monjalon; +Cc: dev

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;
-- 
1.9.1

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [dpdk-dev] [PATCH] kni/ethtool/ixgbe: enforce access between ixgbe PCI and CPU
  2015-02-11  6:49 [dpdk-dev] [PATCH] kni/ethtool/ixgbe: enforce access between ixgbe PCI and CPU xuelin.shi
@ 2015-02-20 10:55 ` Thomas Monjalon
  2015-03-23 13:58   ` Thomas Monjalon
  2015-03-24  1:54 ` Zhang, Helin
  2015-06-15  0:52 ` Zhang, Helin
  2 siblings, 1 reply; 5+ messages in thread
From: Thomas Monjalon @ 2015-02-20 10:55 UTC (permalink / raw)
  To: dev

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;
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [dpdk-dev] [PATCH] kni/ethtool/ixgbe: enforce access between ixgbe PCI and CPU
  2015-02-20 10:55 ` Thomas Monjalon
@ 2015-03-23 13:58   ` Thomas Monjalon
  0 siblings, 0 replies; 5+ messages in thread
From: Thomas Monjalon @ 2015-03-23 13:58 UTC (permalink / raw)
  To: helin.zhang; +Cc: dev, xuelin.shi

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;
> > 
> 
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [dpdk-dev] [PATCH] kni/ethtool/ixgbe: enforce access between ixgbe PCI and CPU
  2015-02-11  6:49 [dpdk-dev] [PATCH] kni/ethtool/ixgbe: enforce access between ixgbe PCI and CPU xuelin.shi
  2015-02-20 10:55 ` Thomas Monjalon
@ 2015-03-24  1:54 ` Zhang, Helin
  2015-06-15  0:52 ` Zhang, Helin
  2 siblings, 0 replies; 5+ messages in thread
From: Zhang, Helin @ 2015-03-24  1:54 UTC (permalink / raw)
  To: xuelin.shi, thomas.monjalon; +Cc: dev

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [dpdk-dev] [PATCH] kni/ethtool/ixgbe: enforce access between ixgbe PCI and CPU
  2015-02-11  6:49 [dpdk-dev] [PATCH] kni/ethtool/ixgbe: enforce access between ixgbe PCI and CPU xuelin.shi
  2015-02-20 10:55 ` Thomas Monjalon
  2015-03-24  1:54 ` Zhang, Helin
@ 2015-06-15  0:52 ` Zhang, Helin
  2 siblings, 0 replies; 5+ messages in thread
From: Zhang, Helin @ 2015-06-15  0:52 UTC (permalink / raw)
  To: xuelin.shi, thomas.monjalon; +Cc: dev



> -----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

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2015-06-15  0:52 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-02-11  6:49 [dpdk-dev] [PATCH] kni/ethtool/ixgbe: enforce access between ixgbe PCI and CPU xuelin.shi
2015-02-20 10:55 ` Thomas Monjalon
2015-03-23 13:58   ` Thomas Monjalon
2015-03-24  1:54 ` Zhang, Helin
2015-06-15  0:52 ` Zhang, Helin

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ https://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git