plugins/ocp: Use structure for ocp smart log

[sc108-lee]

printed log tested (same), except below
Remove wrong duplicated prints from stdout
NVMe base errata version
NVMe command set errata version

[igaw]

I've merged #2577, so this one needs a rebase. Thanks!

[hmi-jeon]

Hi @igaw

I rebased it. Thanks!

[igaw]

I am not seeing any layout changes:

struct ocp_smart_extended_log {
        __u8                       physical_media_units_written[16]; /*     0    16 */
        __u8                       physical_media_units_read[16]; /*    16    16 */
        __u8                       bad_user_nand_blocks_raw[6]; /*    32     6 */
        __le16                     bad_user_nand_blocks_normalized; /*    38     2 */
        __u8                       bad_system_nand_blocks_raw[6]; /*    40     6 */
        __le16                     bad_system_nand_blocks_normalized; /*    46     2 */
        __le64                     xor_recovery_count;   /*    48     8 */
        __le64                     uncorrectable_read_err_count; /*    56     8 */
        /* --- cacheline 1 boundary (64 bytes) --- */
        __le64                     soft_ecc_err_count;   /*    64     8 */
        __le32                     end_to_end_detected_err; /*    72     4 */
        __le32                     end_to_end_corrected_err; /*    76     4 */
        __u8                       system_data_used_percent; /*    80     1 */
        __u8                       refresh_counts[7];    /*    81     7 */
        __le32                     user_data_erase_count_max; /*    88     4 */
        __le32                     user_data_erase_count_min; /*    92     4 */
        __u8                       thermal_throttling_event_count; /*    96     1 */
        __u8                       thermal_throttling_current_status; /*    97     1 */
        __u8                       dssd_errata_version;  /*    98     1 */
        __le16                     dssd_point_version;   /*    99     2 */
        __le16                     dssd_minor_version;   /*   101     2 */
        __u8                       dssd_major_version;   /*   103     1 */
        __le64                     pcie_correctable_err_count; /*   104     8 */
        __le32                     incomplete_shoutdowns; /*   112     4 */
        __u8                       rsvd116[4];           /*   116     4 */
        __u8                       percent_free_blocks;  /*   120     1 */
        __u8                       rsvd121[7];           /*   121     7 */
        /* --- cacheline 2 boundary (128 bytes) --- */
        __le16                     capacitor_health;     /*   128     2 */
        __u8                       nvme_base_errata_version; /*   130     1 */
        __u8                       nvme_cmdset_errata_version; /*   131     1 */
        __u8                       rsvd132[4];           /*   132     4 */
        __le64                     unaligned_io;         /*   136     8 */
        __le64                     security_version;     /*   144     8 */
        __le64                     total_nuse;           /*   152     8 */
        __u8                       plp_start_count[16];  /*   160    16 */
        __u8                       endurance_estimate[16]; /*   176    16 */
        /* --- cacheline 3 boundary (192 bytes) --- */
        __le64                     pcie_link_retaining_count; /*   192     8 */
        __le64                     power_state_change_count; /*   200     8 */
        __le64                     lowest_permitted_fw_rev; /*   208     8 */
        __u8                       rsvd216[278];         /*   216   278 */
        /* --- cacheline 7 boundary (448 bytes) was 46 bytes ago --- */
        __le16                     log_page_version;     /*   494     2 */
        __u8                       log_page_guid[16];    /*   496    16 */

        /* size: 512, cachelines: 8, members: 41 */
} __attribute__((__packed__));

struct ocp_smart_extended_log {
        __u8                       physical_media_units_written[16]; /*     0    16 */
        __u8                       physical_media_units_read[16]; /*    16    16 */
        __u8                       bad_user_nand_blocks_raw[6]; /*    32     6 */
        __le16                     bad_user_nand_blocks_normalized; /*    38     2 */
        __u8                       bad_system_nand_blocks_raw[6]; /*    40     6 */
        __le16                     bad_system_nand_blocks_normalized; /*    46     2 */
        __le64                     xor_recovery_count;   /*    48     8 */
        __le64                     uncorrectable_read_err_count; /*    56     8 */
        /* --- cacheline 1 boundary (64 bytes) --- */
        __le64                     soft_ecc_err_count;   /*    64     8 */
        __le32                     end_to_end_detected_err; /*    72     4 */
        __le32                     end_to_end_corrected_err; /*    76     4 */
        __u8                       system_data_used_percent; /*    80     1 */
        __u8                       refresh_counts[7];    /*    81     7 */
        __le32                     user_data_erase_count_max; /*    88     4 */
        __le32                     user_data_erase_count_min; /*    92     4 */
        __u8                       thermal_throttling_event_count; /*    96     1 */
        __u8                       thermal_throttling_current_status; /*    97     1 */
        __u8                       dssd_errata_version;  /*    98     1 */
        __u8                       dssd_point_version[2]; /*    99     2 */
        __u8                       dssd_minor_version[2]; /*   101     2 */
        __u8                       dssd_major_version;   /*   103     1 */
        __le64                     pcie_correctable_err_count; /*   104     8 */
        __le32                     incomplete_shoutdowns; /*   112     4 */
        __u8                       rsvd116[4];           /*   116     4 */
        __u8                       percent_free_blocks;  /*   120     1 */
        __u8                       rsvd121[7];           /*   121     7 */
        /* --- cacheline 2 boundary (128 bytes) --- */
        __le16                     capacitor_health;     /*   128     2 */
        __u8                       nvme_base_errata_version; /*   130     1 */
        __u8                       nvme_cmdset_errata_version; /*   131     1 */
        __u8                       rsvd132[4];           /*   132     4 */
        __le64                     unaligned_io;         /*   136     8 */
        __le64                     security_version;     /*   144     8 */
        __le64                     total_nuse;           /*   152     8 */
        __u8                       plp_start_count[16];  /*   160    16 */
        __u8                       endurance_estimate[16]; /*   176    16 */
        /* --- cacheline 3 boundary (192 bytes) --- */
        __le64                     pcie_link_retaining_count; /*   192     8 */
        __le64                     power_state_change_count; /*   200     8 */
        __le64                     lowest_permitted_fw_rev; /*   208     8 */
        __u8                       rsvd216[278];         /*   216   278 */
        /* --- cacheline 7 boundary (448 bytes) was 46 bytes ago --- */
        __le16                     log_page_version;     /*   494     2 */
        __u8                       log_page_guid[16];    /*   496    16 */

        /* size: 512, cachelines: 8, members: 41 */
};

This is with gcc on x86_64. How do you get a different layout?

[hmi-jeon]

struct is 4bytes align.
If packed is not used, align padding will occur in dssd_point_version.
So, I aligned to not use packed.
__u16 -> __u8[2]

	packed	packed X	packed X (__u16 -> __u8 [2])
size	512	520	512

[hmi-jeon]

Hi @igaw

I resolved the conflict.
Thanks.

[igaw]

Urgh! This is a nasty thing in the spec.

Anyway, you can't just cast the bytes to a __le16 type, because some architectures will horrible fail if you try to access a non aligned data type. x86 allows though.

You need to copy the value into a proper aligned buffer before you can access it with le16_to_cpu.

And the compiler should actually warn but since you add a hard cast on it, you silence it up. That's why I really like to see __packed go away. This is one of the reason __packed is almost always wrong.

[hmi-jeon]

Hi @igaw

I thought it was right to move the data to uint16_t as you said, so I fixed it.
If you have a better solution, please let me know.

Additionally, I updated ocp v2.6 spec for smart information extended.
Thank you for your review.

[igaw]

I thought it was right to move the data to uint16_t as you said, so I fixed it. If you have a better solution, please let me know.

Looks good. As far I am aware the memcpy approach is the canonical way to address the alignment issue.

Additionally, I updated ocp v2.6 spec for smart information extended. Thank you for your review.

Thanks!