Re: [PATCH 4/6] ehea: header files

Hi Jan-Bernd,

I haven't read all of this, but a few things caught my eye ...

cheers

On Wed, 2006-08-09 at 10:39 +0200, Jan-Bernd Themann wrote:

This looks like a very bad idea, what happens if you're running on a
machine with 64K pages?

+
+#define EHEA_ENABLE	1
+#define EHEA_DISABLE	0

Do you really need hash defines for 0 and 1 ? They're fairly well
understood in C as meaning true and false.

+
+/* Memory Regions */
+#define EHEA_MR_MAX_TX_PAGES 20
+#define EHEA_MR_TX_DATA_PN 3
+#define EHEA_MR_ACC_CTRL 0x00800000
+#define EHEA_RWQES_PER_MR_RQ2 10
+#define EHEA_RWQES_PER_MR_RQ3 10
+
+
+void ehea_set_ethtool_ops(struct net_device *netdev);
+
+#ifndef KEEP_EDEBS_BELOW
+#define KEEP_EDEBS_BELOW 8
+#endif
+
+extern int ehea_trace_level;
+
+#ifdef EHEA_NO_EDEB
+#define EDEB_P_GENERIC(level, idstring, format, args...) \
+	while (0 == 1) { \
+	    if(unlikely (level <= ehea_trace_level)) { \
+			printk("%s " idstring " "format "\n", \
+				__func__, ##args); \
+	  } \
+	} \
+
+#else
+
+#define EDEB_P_GENERIC(level,idstring,format,args...) \
+if (level < KEEP_EDEBS_BELOW) { \
+	do { \
+	    if(unlikely (level <= ehea_trace_level)) { \
+			printk("%s " idstring " "format "\n", \
+				__func__, ##args); \
+	  } \
+	} while (1 == 0); \
+}
+#endif
+
+#define EDEB(level, format, args...) \
+        EDEB_P_GENERIC(level, "", format, ##args)
+
+#define EDEB_ERR(level, format, args...) \
+        EDEB_P_GENERIC(level, "EHEA_ERROR", format, ##args)
+
+#define EDEB_EN(level, format, args...) \
+        EDEB_P_GENERIC(level, ">>>", format, ##args)
+
+#define EDEB_EX(level, format, args...) \
+        EDEB_P_GENERIC(level, "<<<", format, ##args)
+
+#define EHEA_BMASK(pos, length) (((pos) << 16) + (length))
+#define EHEA_BMASK_IBM(from, to) (((63 - to) << 16) + ((to) - (from) + 1))
+#define EHEA_BMASK_SHIFTPOS(mask) (((mask) >> 16) & 0xffff)
+#define EHEA_BMASK_MASK(mask) \
+	(0xffffffffffffffffULL >> ((64 - (mask)) & 0xffff))
+#define EHEA_BMASK_SET(mask, value) \
+        ((EHEA_BMASK_MASK(mask) & ((u64)(value))) << EHEA_BMASK_SHIFTPOS(mask))
+#define EHEA_BMASK_GET(mask, value) \
+        (EHEA_BMASK_MASK(mask) & (((u64)(value)) >> EHEA_BMASK_SHIFTPOS(mask)))
+
+extern void exit(int);
+
+#define EDEB_DMP(level, adr, len, format, args...) \
+if (level < KEEP_EDEBS_BELOW) { \
+     if(unlikely (level <= ehea_trace_level)) {  \
+        do { \
+                unsigned int x; \
+		unsigned int l = (unsigned int)(len); \
+                unsigned char *deb = (unsigned char*)(adr); \
+		for (x = 0; x < l; x += 16) { \
+		        EDEB(level, format " adr=%p ofs=%04x %016lx %016lx", \
+			     ##args, deb, x, *((u64 *)&deb[0]), \
+			     *((u64 *)&deb[8])); \
+			deb += 16; \
+		} \
+        } while (0); \
+     } \
+}
+
+
+/*
+ * struct generic ehea page
+ */
+struct ipz_page {
+	u8 entries[PAGE_SIZE];
+};
+
+/*
+ * struct generic queue in linux kernel virtual memory
+ */
+struct ipz_queue {
+	u64 current_q_offset;		/* current queue entry */
+	struct ipz_page **queue_pages;	/* array of pages belonging to queue */
+	u32 qe_size;			/* queue entry size */
+	u32 act_nr_of_sg;
+	u32 queue_length;      		/* queue length allocated in bytes */
+	u32 pagesize;
+	u32 toggle_state;		/* toggle flag - per page */
+	u32 reserved;			/* 64 bit alignment */
+};
+
+
+/*
+ *  h_galpa:
+ *  for pSeries this is a 64bit memory address where
+ *  I/O memory is mapped into CPU address space
+ */
+
+struct h_galpa {
+	u64 fw_handle;
+};

What is a h_galpa? And why does it need a struct if it's just a u64?

+
+struct h_galpas {
+	struct h_galpa kernel;	/* kernel space accessible resource,
+				   set to 0 if unused */
+	struct h_galpa user;	/* user space accessible resource
+				   set to 0 if unused */
+	u32 pid;		/* PID of userspace galpa checking */
+};
+
+struct ehea_qp;
+struct ehea_cq;
+struct ehea_eq;
+struct ehea_port;
+struct ehea_av;
+
+struct ehea_qp_init_attr {
+        /* input parameter */
+	u32 qp_token;
+	u8 low_lat_rq1;
+	u8 signalingtype;
+	u8 rq_count;
+	u8 eqe_gen;
+	u16 max_nr_send_wqes;
+	u16 max_nr_rwqes_rq1;
+	u16 max_nr_rwqes_rq2;
+	u16 max_nr_rwqes_rq3;
+	u8 wqe_size_enc_sq;
+	u8 wqe_size_enc_rq1;
+	u8 wqe_size_enc_rq2;
+	u8 wqe_size_enc_rq3;
+	u8 swqe_imm_data_len;
+	u16 port_nr;
+	u16 rq2_threshold;
+	u16 rq3_threshold;
+	u64 send_cq_handle;
+	u64 recv_cq_handle;
+	u64 aff_eq_handle;
+
+        /* output parameter */
+	u32 qp_nr;
+	u16 act_nr_send_wqes;
+	u16 act_nr_rwqes_rq1;
+	u16 act_nr_rwqes_rq2;
+	u16 act_nr_rwqes_rq3;
+	u8 act_wqe_size_enc_sq;
+	u8 act_wqe_size_enc_rq1;
+	u8 act_wqe_size_enc_rq2;
+	u8 act_wqe_size_enc_rq3;
+	u32 nr_sq_pages;
+	u32 nr_rq1_pages;
+	u32 nr_rq2_pages;
+	u32 nr_rq3_pages;
+	u32 liobn_sq;
+	u32 liobn_rq1;
+	u32 liobn_rq2;
+	u32 liobn_rq3;
+};
+
+struct ehea_eq_attr {
+	u32 type;
+	u32 max_nr_of_eqes;
+	u8 eqe_gen;
+	u64 eq_handle;
+	u32 act_nr_of_eqes;
+	u32 nr_pages;
+	u32 ist1;
+	u32 ist2;
+	u32 ist3;
+	u32 ist4;
+};
+
+struct ehea_eq {
+	struct ehea_adapter *adapter;
+	struct ipz_queue ipz_queue;
+	u64 ipz_eq_handle;
+	struct h_galpas galpas;
+	spinlock_t spinlock;
+	struct ehea_eq_attr attr;
+};
+
+struct ehea_qp {
+	struct ehea_adapter *adapter;
+	u64 ipz_qp_handle;	/* QP handle for h-calls */
+	struct ipz_queue ipz_squeue;
+	struct ipz_queue ipz_rqueue1;
+	struct ipz_queue ipz_rqueue2;
+	struct ipz_queue ipz_rqueue3;
+	struct h_galpas galpas;
+	struct ehea_qp_init_attr init_attr;
+};
+
+struct ehea_cq_attr {
+        /* input parameter */
+	u32 max_nr_of_cqes;
+	u32 cq_token;
+	u64 eq_handle;
+
+        /* output parameter */
+	u32 act_nr_of_cqes;
+	u32 nr_pages;
+};
+
+struct ehea_cq {
+	struct ehea_adapter *adapter;
+	u64 ipz_cq_handle;
+	struct ipz_queue ipz_queue;
+	struct h_galpas galpas;
+	struct ehea_cq_attr attr;
+};
+
+struct ehea_mr {
+	u64 handle;
+	u64 vaddr;
+	u32 lkey;
+};
+
+struct port_state {
+	int poll_max_processed;
+	int poll_receive_errors;
+	int ehea_poll;
+	int queue_stopped;
+	int min_swqe_avail;
+	u64 sqc_stop_sum;
+	int pkt_send;
+	int pkt_xmit;
+	int send_tasklet;
+	int nwqe;
+};
+
+#define EHEA_IRQ_NAME_SIZE 20
+struct ehea_port_res {
+	struct ehea_mr send_mr;
+	struct ehea_mr recv_mr;
+	spinlock_t xmit_lock;
+	struct ehea_port *port;
+	char int_recv_name[EHEA_IRQ_NAME_SIZE];
+	char int_send_name[EHEA_IRQ_NAME_SIZE];
+	struct ehea_qp *qp;
+	struct ehea_cq *send_cq;
+	struct ehea_cq *recv_cq;
+	struct ehea_eq *send_eq;
+	struct ehea_eq *recv_eq;
+	spinlock_t send_lock;
+	struct sk_buff **skb_arr_rq1;
+	struct sk_buff **skb_arr_rq2;
+	struct sk_buff **skb_arr_rq3;
+	struct sk_buff **skb_arr_sq;
+	int skb_arr_rq1_len;
+	int skb_arr_rq2_len;
+	int skb_arr_rq3_len;
+	int skb_arr_sq_len;
+	int skb_rq2_index;
+	int skb_rq3_index;
+	int skb_sq_index;
+	spinlock_t netif_queue;
+	atomic_t swqe_avail;
+	int swqe_ll_count;
+	int swqe_count;
+	u32 swqe_id_counter;
+	u64 tx_packets;
+	struct tasklet_struct send_comp_task;
+	spinlock_t recv_lock;
+	struct timer_list timer;	/* polling mode, no interrupts */
+	struct timer_list skb_timer;	/* skb cleanup timer */
+	struct port_state p_state;
+	u64 rx_packets;
+	u32 poll_counter;
+};
+
+
+struct ehea_adapter {
+	u64 handle;
+	u8 num_ports;
+	struct ehea_port *port[16];
+	struct ehea_eq *neq;
+	struct tasklet_struct neq_tasklet;
+	struct ehea_mr mr;
+	u32 pd;
+	u64 max_mc_mac;
+};
+
+
+struct ehea_mc_list {
+	struct list_head list;
+	u64 macaddr;
+};
+
+#define EHEA_MAX_PORT_RES 16
+struct ehea_port {
+	struct ehea_adapter *adapter;	 /* adapter that owns this port */
+	struct net_device *netdev;
+	struct net_device_stats stats;
+	struct ehea_port_res port_res[EHEA_MAX_PORT_RES];
+	struct device_node *of_dev_node; /* Open Firmware Device Node */
+	struct ehea_mc_list *mc_list;	 /* Multicast MAC addresses */
+	struct vlan_group *vgrp;
+	struct ehea_eq *qp_eq;
+	char int_aff_name[EHEA_IRQ_NAME_SIZE];
+	int allmulti;			 /* Indicates IFF_ALLMULTI state */
+	int promisc;		 	 /* Indicates IFF_PROMISC state */
+	int kernel_l_key;
+	int num_tx_qps;
+	u64 mac_addr;
+	u32 logical_port_id;
+	u32 port_speed;
+	u8 full_duplex;
+	u8 num_def_qps;
+};
+
+struct port_res_cfg {
+	int max_entries_rcq;
+	int max_entries_scq;
+	int max_entries_sq;
+	int max_entries_rq1;
+	int max_entries_rq2;
+	int max_entries_rq3;
+};

Enormous structs with no comments.

-- 
Michael Ellerman
IBM OzLabs

wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)

We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person