RE: [PATCH net v3 3/3] net: ngbe: specify IRQ vector when the number of VFs is 7

["Jiawen Wu" <jiawenwu@trustnetic.com>]

On Mon, Jun 30, 2025 9:30 PM, Larysa Zaremba wrote:
> On Thu, Jun 26, 2025 at 04:48:04PM +0800, Jiawen Wu wrote:
> > For NGBE devices, the queue number is limited to be 1 when SRIOV is
> > enabled. In this case, IRQ vector[0] is used for MISC and vector[1] is
> > used for queue, based on the previous patches. But for the hardware
> > design, the IRQ vector[1] must be allocated for use by the VF[6] when
> > the number of VFs is 7. So the IRQ vector[0] should be shared for PF
> > MISC and QUEUE interrupts.
> >
> > +-----------+----------------------+
> > | Vector    | Assigned To          |
> > +-----------+----------------------+
> > | Vector 0  | PF MISC and QUEUE    |
> > | Vector 1  | VF 6                 |
> > | Vector 2  | VF 5                 |
> > | Vector 3  | VF 4                 |
> > | Vector 4  | VF 3                 |
> > | Vector 5  | VF 2                 |
> > | Vector 6  | VF 1                 |
> > | Vector 7  | VF 0                 |
> > +-----------+----------------------+
> >
> > Minimize code modifications, only adjust the IRQ vector number for this
> > case.
> >
> > Fixes: 877253d2cbf2 ("net: ngbe: add sriov function support")
> > Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com>
> 
> Looks fine in general, but I see 1 functional problem. Please, see below.
> 
> > ---
> >  drivers/net/ethernet/wangxun/libwx/wx_lib.c   | 9 +++++++++
> >  drivers/net/ethernet/wangxun/libwx/wx_sriov.c | 4 ++++
> >  drivers/net/ethernet/wangxun/libwx/wx_type.h  | 1 +
> >  drivers/net/ethernet/wangxun/ngbe/ngbe_main.c | 2 +-
> >  drivers/net/ethernet/wangxun/ngbe/ngbe_type.h | 2 +-
> >  5 files changed, 16 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/wangxun/libwx/wx_lib.c b/drivers/net/ethernet/wangxun/libwx/wx_lib.c
> > index 66eaf5446115..7b53169cd216 100644
> > --- a/drivers/net/ethernet/wangxun/libwx/wx_lib.c
> > +++ b/drivers/net/ethernet/wangxun/libwx/wx_lib.c
> > @@ -1794,6 +1794,13 @@ static int wx_acquire_msix_vectors(struct wx *wx)
> >  	wx->msix_entry->entry = nvecs;
> >  	wx->msix_entry->vector = pci_irq_vector(wx->pdev, nvecs);
> >
> > +	if (test_bit(WX_FLAG_IRQ_VECTOR_SHARED, wx->flags)) {
> > +		wx->msix_entry->entry = 0;
> > +		wx->msix_entry->vector = pci_irq_vector(wx->pdev, 0);
> > +		wx->msix_q_entries[0].entry = 0;
> > +		wx->msix_q_entries[0].vector = pci_irq_vector(wx->pdev, 1);
> > +	}
> > +
> >  	return 0;
> >  }
> >
> > @@ -2292,6 +2299,8 @@ static void wx_set_ivar(struct wx *wx, s8 direction,
> >
> >  	if (direction == -1) {
> >  		/* other causes */
> > +		if (test_bit(WX_FLAG_IRQ_VECTOR_SHARED, wx->flags))
> > +			msix_vector = 0;
> >  		msix_vector |= WX_PX_IVAR_ALLOC_VAL;
> >  		index = 0;
> >  		ivar = rd32(wx, WX_PX_MISC_IVAR);
> > diff --git a/drivers/net/ethernet/wangxun/libwx/wx_sriov.c b/drivers/net/ethernet/wangxun/libwx/wx_sriov.c
> > index e8656d9d733b..c82ae137756c 100644
> > --- a/drivers/net/ethernet/wangxun/libwx/wx_sriov.c
> > +++ b/drivers/net/ethernet/wangxun/libwx/wx_sriov.c
> > @@ -64,6 +64,7 @@ static void wx_sriov_clear_data(struct wx *wx)
> >  	wr32m(wx, WX_PSR_VM_CTL, WX_PSR_VM_CTL_POOL_MASK, 0);
> >  	wx->ring_feature[RING_F_VMDQ].offset = 0;
> >
> > +	clear_bit(WX_FLAG_IRQ_VECTOR_SHARED, wx->flags);
> >  	clear_bit(WX_FLAG_SRIOV_ENABLED, wx->flags);
> >  	/* Disable VMDq flag so device will be set in NM mode */
> >  	if (wx->ring_feature[RING_F_VMDQ].limit == 1)
> > @@ -78,6 +79,9 @@ static int __wx_enable_sriov(struct wx *wx, u8 num_vfs)
> >  	set_bit(WX_FLAG_SRIOV_ENABLED, wx->flags);
> >  	dev_info(&wx->pdev->dev, "SR-IOV enabled with %d VFs\n", num_vfs);
> >
> > +	if (num_vfs == 7 && wx->mac.type == wx_mac_em)
> > +		set_bit(WX_FLAG_IRQ_VECTOR_SHARED, wx->flags);
> > +
> >  	/* Enable VMDq flag so device will be set in VM mode */
> >  	set_bit(WX_FLAG_VMDQ_ENABLED, wx->flags);
> >  	if (!wx->ring_feature[RING_F_VMDQ].limit)
> > diff --git a/drivers/net/ethernet/wangxun/libwx/wx_type.h b/drivers/net/ethernet/wangxun/libwx/wx_type.h
> > index d392394791b3..c363379126c0 100644
> > --- a/drivers/net/ethernet/wangxun/libwx/wx_type.h
> > +++ b/drivers/net/ethernet/wangxun/libwx/wx_type.h
> > @@ -1191,6 +1191,7 @@ enum wx_pf_flags {
> >  	WX_FLAG_VMDQ_ENABLED,
> >  	WX_FLAG_VLAN_PROMISC,
> >  	WX_FLAG_SRIOV_ENABLED,
> > +	WX_FLAG_IRQ_VECTOR_SHARED,
> >  	WX_FLAG_FDIR_CAPABLE,
> >  	WX_FLAG_FDIR_HASH,
> >  	WX_FLAG_FDIR_PERFECT,
> > diff --git a/drivers/net/ethernet/wangxun/ngbe/ngbe_main.c b/drivers/net/ethernet/wangxun/ngbe/ngbe_main.c
> > index 68415a7ef12f..e0fc897b0a58 100644
> > --- a/drivers/net/ethernet/wangxun/ngbe/ngbe_main.c
> > +++ b/drivers/net/ethernet/wangxun/ngbe/ngbe_main.c
> > @@ -286,7 +286,7 @@ static int ngbe_request_msix_irqs(struct wx *wx)
> >  	 * for queue. But when num_vfs == 7, vector[1] is assigned to vf6.
> >  	 * Misc and queue should reuse interrupt vector[0].
> >  	 */
> > -	if (wx->num_vfs == 7)
> > +	if (test_bit(WX_FLAG_IRQ_VECTOR_SHARED, wx->flags))
> >  		err = request_irq(wx->msix_entry->vector,
> >  				  ngbe_misc_and_queue, 0, netdev->name, wx);
> >  	else
> > diff --git a/drivers/net/ethernet/wangxun/ngbe/ngbe_type.h b/drivers/net/ethernet/wangxun/ngbe/ngbe_type.h
> > index 6eca6de475f7..44ff62af7ae0 100644
> > --- a/drivers/net/ethernet/wangxun/ngbe/ngbe_type.h
> > +++ b/drivers/net/ethernet/wangxun/ngbe/ngbe_type.h
> > @@ -87,7 +87,7 @@
> >  #define NGBE_PX_MISC_IC_TIMESYNC		BIT(11) /* time sync */
> >
> >  #define NGBE_INTR_ALL				0x1FF
> > -#define NGBE_INTR_MISC(A)			BIT((A)->num_q_vectors)
> > +#define NGBE_INTR_MISC(A)			BIT((A)->msix_entry->vector)
> >
> 
> In V2, Michal advised you to use BIT((A)->msix_entry->entry in this macro. Given
> this macro is used for call to wx_intr_enable(), I would agree with him. The
> driver is not supposed to control msix_entry->vector contents, so it is a poor
> choice for such hardware-specific writes.

Thanks! A clerical error almost caused a major mistake.