Re: [iwl-next v1 1/4] ixgbe: add MDD support

[Michal Swiatkowski <michal.swiatkowski@linux.intel.com>]

On Fri, Feb 07, 2025 at 03:07:49PM +0000, Simon Horman wrote:
> On Fri, Feb 07, 2025 at 11:43:40AM +0100, Michal Swiatkowski wrote:
> > From: Paul Greenwalt <paul.greenwalt@intel.com>
> > 
> > Add malicious driver detection. Support enabling MDD, disabling MDD,
> > handling a MDD event, and restoring a MDD VF.
> > 
> > Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
> > Reviewed-by: Jedrzej Jagielski <jedrzej.jagielski@intel.com>
> > Reviewed-by: Marcin Szycik <marcin.szycik@linux.intel.com>
> > Signed-off-by: Paul Greenwalt <paul.greenwalt@intel.com>
> > Signed-off-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
> 
> ...
> 
> > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c
> 
> ...
> 
> > +/**
> > + * ixgbe_handle_mdd_x550 - handle malicious driver detection event
> > + * @hw: pointer to hardware structure
> > + * @vf_bitmap: output vf bitmap of malicious vfs
> > + */
> > +void ixgbe_handle_mdd_x550(struct ixgbe_hw *hw, unsigned long *vf_bitmap)
> > +{
> > +	u32 i, j, reg, q, div, vf, wqbr;
> > +
> > +	/* figure out pool size for mapping to vf's */
> > +	reg = IXGBE_READ_REG(hw, IXGBE_MRQC);
> > +	switch (reg & IXGBE_MRQC_MRQE_MASK) {
> > +	case IXGBE_MRQC_VMDQRT8TCEN:
> > +		div = IXGBE_16VFS_QUEUES;
> > +		break;
> > +	case IXGBE_MRQC_VMDQRSS32EN:
> > +	case IXGBE_MRQC_VMDQRT4TCEN:
> > +		div = IXGBE_32VFS_QUEUES;
> > +		break;
> > +	default:
> > +		div = IXGBE_64VFS_QUEUES;
> > +		break;
> > +	}
> > +
> > +	/* Read WQBR_TX and WQBR_RX and check for malicious queues */
> > +	for (i = 0; i < IXGBE_QUEUES_REG_AMOUNT; i++) {
> > +		wqbr = IXGBE_READ_REG(hw, IXGBE_WQBR_TX(i)) |
> > +		       IXGBE_READ_REG(hw, IXGBE_WQBR_RX(i));
> > +		if (!wqbr)
> > +			continue;
> > +
> > +		/* Get malicious queue */
> > +		for_each_set_bit(j, (unsigned long *)&wqbr,
> > +				 IXGBE_QUEUES_PER_REG) {
> 
> The type of wqbr is a u32, that is it is 32-bits wide.
> Above it's address is cast to unsigned long *.
> But, unsigned long may be 64-bits wide, e.g. on x86_64.
> 
> GCC 14.2.0 EXTRA_CFLAGS=-Warray-bounds builds report this as:
> 
> In file included from ./include/linux/bitmap.h:11,
>                  from ./include/linux/cpumask.h:12,
>                  from ./arch/x86/include/asm/paravirt.h:21,
>                  from ./arch/x86/include/asm/cpuid.h:71,
>                  from ./arch/x86/include/asm/processor.h:19,
>                  from ./arch/x86/include/asm/cpufeature.h:5,
>                  from ./arch/x86/include/asm/thread_info.h:59,
>                  from ./include/linux/thread_info.h:60,
>                  from ./include/linux/uio.h:9,
>                  from ./include/linux/socket.h:8,
>                  from ./include/uapi/linux/if.h:25,
>                  from ./include/linux/mii.h:12,
>                  from ./include/uapi/linux/mdio.h:15,
>                  from ./include/linux/mdio.h:9,
>                  from drivers/net/ethernet/intel/ixgbe/ixgbe_type.h:8,
>                  from drivers/net/ethernet/intel/ixgbe/ixgbe_x540.h:7,
>                  from drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c:4:
> In function ‘find_next_bit’,
>     inlined from ‘ixgbe_handle_mdd_x550’ at drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c:3907:3:
> ./include/linux/find.h:65:23: error: array subscript ‘long unsigned int[0]’ is partly outside array bounds of ‘u32[1]’ {aka ‘unsigned int[1]’} [-Werror=array-bounds=]
>    65 |                 val = *addr & GENMASK(size - 1, offset);
>       |                       ^~~~~
> 
> I think this can be addressed by changing the type of wqmbr to unsigned long.

Thanks for catching that, I will fix.

> 
> > +			/* Get queue from bitmask */
> > +			q = j + (i * IXGBE_QUEUES_PER_REG);
> > +			/* Map queue to vf */
> > +			vf = q / div;
> > +			set_bit(vf, vf_bitmap);
> > +		}
> > +	}
> > +}
> > +
> >  #define X550_COMMON_MAC \
> >  	.init_hw			= &ixgbe_init_hw_generic, \
> >  	.start_hw			= &ixgbe_start_hw_X540, \
> 
> ...