ice: use GENMASK instead of BIT(n) - 1 in pack functions
authorJacob Keller <jacob.e.keller@intel.com>
Tue, 27 Feb 2024 00:14:55 +0000 (16:14 -0800)
committerTony Nguyen <anthony.l.nguyen@intel.com>
Mon, 4 Mar 2024 18:26:57 +0000 (10:26 -0800)
The functions used to pack the Tx and Rx context into the hardware format
rely on using BIT() and then subtracting 1 to get a bitmask. These
functions even have a comment about how x86 machines can't use this method
for certain widths because the SHL instructions will not work properly.

The Linux kernel already provides the GENMASK macro for generating a
suitable bitmask. Further, GENMASK is capable of generating the mask
including the shift_width. Since width is the total field width, take care
to subtract one to get the final bit position.

Since we now include the shifted bits as part of the mask, shift the source
value first before applying the mask.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
Tested-by: Pucha Himasekhar Reddy <himasekharx.reddy.pucha@intel.com> (A Contingent worker at Intel)
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
drivers/net/ethernet/intel/ice/ice_common.c

index 3622079cb5064f40ee88ed8e83055de72ebfd50e..e9e4a8ef79049fe840bc8d660effd3dd1894b7c0 100644 (file)
@@ -4379,14 +4379,11 @@ static void ice_pack_ctx_byte(u8 *src_ctx, u8 *dest_ctx,
 
        /* prepare the bits and mask */
        shift_width = ce_info->lsb % 8;
-       mask = (u8)(BIT(ce_info->width) - 1);
+       mask = GENMASK(ce_info->width - 1 + shift_width, shift_width);
 
        src_byte = *from;
-       src_byte &= mask;
-
-       /* shift to correct alignment */
-       mask <<= shift_width;
        src_byte <<= shift_width;
+       src_byte &= mask;
 
        /* get the current bits from the target bit string */
        dest = dest_ctx + (ce_info->lsb / 8);
@@ -4419,17 +4416,14 @@ static void ice_pack_ctx_word(u8 *src_ctx, u8 *dest_ctx,
 
        /* prepare the bits and mask */
        shift_width = ce_info->lsb % 8;
-       mask = BIT(ce_info->width) - 1;
+       mask = GENMASK(ce_info->width - 1 + shift_width, shift_width);
 
        /* don't swizzle the bits until after the mask because the mask bits
         * will be in a different bit position on big endian machines
         */
        src_word = *(u16 *)from;
-       src_word &= mask;
-
-       /* shift to correct alignment */
-       mask <<= shift_width;
        src_word <<= shift_width;
+       src_word &= mask;
 
        /* get the current bits from the target bit string */
        dest = dest_ctx + (ce_info->lsb / 8);
@@ -4462,25 +4456,14 @@ static void ice_pack_ctx_dword(u8 *src_ctx, u8 *dest_ctx,
 
        /* prepare the bits and mask */
        shift_width = ce_info->lsb % 8;
-
-       /* if the field width is exactly 32 on an x86 machine, then the shift
-        * operation will not work because the SHL instructions count is masked
-        * to 5 bits so the shift will do nothing
-        */
-       if (ce_info->width < 32)
-               mask = BIT(ce_info->width) - 1;
-       else
-               mask = (u32)~0;
+       mask = GENMASK(ce_info->width - 1 + shift_width, shift_width);
 
        /* don't swizzle the bits until after the mask because the mask bits
         * will be in a different bit position on big endian machines
         */
        src_dword = *(u32 *)from;
-       src_dword &= mask;
-
-       /* shift to correct alignment */
-       mask <<= shift_width;
        src_dword <<= shift_width;
+       src_dword &= mask;
 
        /* get the current bits from the target bit string */
        dest = dest_ctx + (ce_info->lsb / 8);
@@ -4513,25 +4496,14 @@ static void ice_pack_ctx_qword(u8 *src_ctx, u8 *dest_ctx,
 
        /* prepare the bits and mask */
        shift_width = ce_info->lsb % 8;
-
-       /* if the field width is exactly 64 on an x86 machine, then the shift
-        * operation will not work because the SHL instructions count is masked
-        * to 6 bits so the shift will do nothing
-        */
-       if (ce_info->width < 64)
-               mask = BIT_ULL(ce_info->width) - 1;
-       else
-               mask = (u64)~0;
+       mask = GENMASK_ULL(ce_info->width - 1 + shift_width, shift_width);
 
        /* don't swizzle the bits until after the mask because the mask bits
         * will be in a different bit position on big endian machines
         */
        src_qword = *(u64 *)from;
-       src_qword &= mask;
-
-       /* shift to correct alignment */
-       mask <<= shift_width;
        src_qword <<= shift_width;
+       src_qword &= mask;
 
        /* get the current bits from the target bit string */
        dest = dest_ctx + (ce_info->lsb / 8);