On Wednesday, December 14, 2016 1:11:42 AM CET Holger Dengler wrote:
>
> diff --git a/include/uapi/linux/flexcard.h b/include/uapi/linux/flexcard.h
> new file mode 100644
> index 0000000..4e9f07b4
> --- /dev/null
> +++ b/include/uapi/linux/flexcard.h
> @@ -0,0 +1,64 @@

Why is this exported to user space?

> +
> +#include <linux/types.h>
> +
> +struct fc_version {
> +     __u8    dev;
> +     __u8    min;
> +     __u8    maj;
> +     __u8    reserved;
> +} __packed;

The __packed attribute is redundant here as all members
are just one byte anyway.

> +/* PCI BAR 0: Flexcard configuration */
> +struct fc_bar0_conf {
> +     __u32 r1;                       /* 000 */
> +     struct fc_version fc_fw_ver;    /* 004 */
> +     struct fc_version fc_hw_ver;    /* 008 */
> +     __u32 r2[3];                    /* 00c */
> +     __u64 fc_sn;                    /* 018 */
> +     __u32 fc_uid;                   /* 020 */
> +     __u32 r3[7];                    /* 024 */
> +     __u32 fc_lic[6];                /* 040 */
> +     __u32 fc_slic[6];               /* 058 */
> +     __u32 trig_ctrl1;               /* 070 */
> +     __u32 r4;                       /* 074 */
> +     __u32 trig_ctrl2;               /* 078 */
> +     __u32 r5[22];                   /* 07c */
> +     __u32 amreg;                    /* 0d4 */
> +     __u32 tiny_stat;                /* 0d8 */
> +     __u32 r6[5];                    /* 0dc */
> +     __u32 can_dat_cnt;              /* 0f0 */
> +     __u32 can_err_cnt;              /* 0f4 */
> +     __u32 fc_data_cnt;              /* 0f8 */
> +     __u32 r7;                       /* 0fc */
> +     __u32 fc_rocr;                  /* 100 */
> +     __u32 r8;                       /* 104 */
> +     __u32 pg_ctrl;                  /* 108 */
> +     __u32 pg_term;                  /* 10c */
> +     __u32 r9;                       /* 110 */
> +     __u32 irs;                      /* 114 */
> +     __u32 fr_tx_cnt;                /* 118 */
> +     __u32 irc;                      /* 11c */
> +     __u64 pcnt;                     /* 120 */
> +     __u32 r10;                      /* 128 */
> +     __u32 nmv_cnt;                  /* 12c */
> +     __u32 info_cnt;                 /* 130 */
> +     __u32 stat_trg_cnt;             /* 134 */
> +     __u32 r11;                      /* 138 */
> +     __u32 fr_rx_cnt;                /* 13c */
> +} __packed;

Here the __packed attribute is probably harmful, it prevents you
from accessing the members using 32-bit sized accesses and forces
bytewise accesses on some architectures, which tends to do the
wrong thing on MMIO.

        Arnd

Reply via email to