[net-next,v2,2/9] octeon_ep: poll for control messages

Commit Message

Veerasenareddy Burru Nov. 29, 2022, 1:09 p.m. UTC

Poll for control messages until interrupts are enabled.
All the interrupts are enabled in ndo_open().
Add ability to listen for notifications from firmware before ndo_open().
Once interrupts are enabled, this polling is disabled and all the
messages are processed by bottom half of interrupt handler.

Signed-off-by: Veerasenareddy Burru <[e-mail]>
Signed-off-by: Abhijit Ayarekar <[e-mail]>
---
v1 -> v2:
 * removed device status oct->status, as it is not required with the
   modified implementation in 0001-xxxx.patch

 .../marvell/octeon_ep/octep_cn9k_pf.c         | 49 +++++++++----------
 .../ethernet/marvell/octeon_ep/octep_main.c   | 35 +++++++++++++
 .../ethernet/marvell/octeon_ep/octep_main.h   | 11 ++++-
 .../marvell/octeon_ep/octep_regs_cn9k_pf.h    |  4 ++
 4 files changed, 71 insertions(+), 28 deletions(-)

Comments

Leon Romanovsky Nov. 30, 2022, 9:30 a.m. UTC | #1

On Tue, Nov 29, 2022 at 05:09:25AM -0800, Veerasenareddy Burru wrote:
> Poll for control messages until interrupts are enabled.
> All the interrupts are enabled in ndo_open().

So what are you saying if I have your device and didn't enable network
device, you will poll forever?

> Add ability to listen for notifications from firmware before ndo_open().
> Once interrupts are enabled, this polling is disabled and all the
> messages are processed by bottom half of interrupt handler.
> 
> Signed-off-by: Veerasenareddy Burru <[e-mail]>
> Signed-off-by: Abhijit Ayarekar <[e-mail]>
> ---
> v1 -> v2:
>  * removed device status oct->status, as it is not required with the
>    modified implementation in 0001-xxxx.patch
> 
>  .../marvell/octeon_ep/octep_cn9k_pf.c         | 49 +++++++++----------
>  .../ethernet/marvell/octeon_ep/octep_main.c   | 35 +++++++++++++
>  .../ethernet/marvell/octeon_ep/octep_main.h   | 11 ++++-
>  .../marvell/octeon_ep/octep_regs_cn9k_pf.h    |  4 ++
>  4 files changed, 71 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/net/ethernet/marvell/octeon_ep/octep_cn9k_pf.c b/drivers/net/ethernet/marvell/octeon_ep/octep_cn9k_pf.c
> index 6ad88d0fe43f..ace2dfd1e918 100644
> --- a/drivers/net/ethernet/marvell/octeon_ep/octep_cn9k_pf.c
> +++ b/drivers/net/ethernet/marvell/octeon_ep/octep_cn9k_pf.c
> @@ -352,27 +352,36 @@ static void octep_setup_mbox_regs_cn93_pf(struct octep_device *oct, int q_no)
>  	mbox->mbox_read_reg = oct->mmio[0].hw_addr + CN93_SDP_R_MBOX_VF_PF_DATA(q_no);
>  }
>  
> -/* Mailbox Interrupt handler */
> -static void cn93_handle_pf_mbox_intr(struct octep_device *oct)
> +/* Process non-ioq interrupts required to keep pf interface running.
> + * OEI_RINT is needed for control mailbox
> + */
> +static int octep_poll_non_ioq_interrupts_cn93_pf(struct octep_device *oct)
>  {
> -	u64 mbox_int_val = 0ULL, val = 0ULL, qno = 0ULL;
> +	u64 reg0;
> +	int handled = 0;

Reversed Christmas tree.

Thanks

Veerasenareddy Burru Nov. 30, 2022, 3:44 p.m. UTC | #2

> -----Original Message-----
> From: Leon Romanovsky <[e-mail]>
> Sent: Wednesday, November 30, 2022 1:30 AM
> To: Veerasenareddy Burru <[e-mail]>
> Cc: [e-mail]; [e-mail]; Liron Himi
> <[e-mail]>; Abhijit Ayarekar <[e-mail]>; Sathesh
> B Edara <[e-mail]>; Satananda Burla <[e-mail]>;
> [e-mail]; David S. Miller <[e-mail]>; Eric
> Dumazet <[e-mail]>; Jakub Kicinski <[e-mail]>;
> Paolo Abeni <[e-mail]>
> Subject: [EXT] Re: [PATCH net-next v2 2/9] octeon_ep: poll for control
> messages
> 
> External Email
> 
> ----------------------------------------------------------------------
> On Tue, Nov 29, 2022 at 05:09:25AM -0800, Veerasenareddy Burru wrote:
> > Poll for control messages until interrupts are enabled.
> > All the interrupts are enabled in ndo_open().
> 
> So what are you saying if I have your device and didn't enable network
> device, you will poll forever?
Yes, Leon. It will poll periodically until network interface is enabled.
> 
> > Add ability to listen for notifications from firmware before ndo_open().
> > Once interrupts are enabled, this polling is disabled and all the
> > messages are processed by bottom half of interrupt handler.
> >
> > Signed-off-by: Veerasenareddy Burru <[e-mail]>
> > Signed-off-by: Abhijit Ayarekar <[e-mail]>
> > ---
> > v1 -> v2:
> >  * removed device status oct->status, as it is not required with the
> >    modified implementation in 0001-xxxx.patch
> >
> >  .../marvell/octeon_ep/octep_cn9k_pf.c         | 49 +++++++++----------
> >  .../ethernet/marvell/octeon_ep/octep_main.c   | 35 +++++++++++++
> >  .../ethernet/marvell/octeon_ep/octep_main.h   | 11 ++++-
> >  .../marvell/octeon_ep/octep_regs_cn9k_pf.h    |  4 ++
> >  4 files changed, 71 insertions(+), 28 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/marvell/octeon_ep/octep_cn9k_pf.c
> > b/drivers/net/ethernet/marvell/octeon_ep/octep_cn9k_pf.c
> > index 6ad88d0fe43f..ace2dfd1e918 100644
> > --- a/drivers/net/ethernet/marvell/octeon_ep/octep_cn9k_pf.c
> > +++ b/drivers/net/ethernet/marvell/octeon_ep/octep_cn9k_pf.c
> > @@ -352,27 +352,36 @@ static void
> octep_setup_mbox_regs_cn93_pf(struct octep_device *oct, int q_no)
> >  	mbox->mbox_read_reg = oct->mmio[0].hw_addr +
> > CN93_SDP_R_MBOX_VF_PF_DATA(q_no);  }
> >
> > -/* Mailbox Interrupt handler */
> > -static void cn93_handle_pf_mbox_intr(struct octep_device *oct)
> > +/* Process non-ioq interrupts required to keep pf interface running.
> > + * OEI_RINT is needed for control mailbox  */ static int
> > +octep_poll_non_ioq_interrupts_cn93_pf(struct octep_device *oct)
> >  {
> > -	u64 mbox_int_val = 0ULL, val = 0ULL, qno = 0ULL;
> > +	u64 reg0;
> > +	int handled = 0;
> 
> Reversed Christmas tree.
Thanks for the feedback. Will revise the patch.
> 
> Thanks

Leon Romanovsky Dec. 1, 2022, 8:11 a.m. UTC | #3

On Wed, Nov 30, 2022 at 03:44:30PM +0000, Veerasenareddy Burru wrote:
> 
> 
> > -----Original Message-----
> > From: Leon Romanovsky <[e-mail]>
> > Sent: Wednesday, November 30, 2022 1:30 AM
> > To: Veerasenareddy Burru <[e-mail]>
> > Cc: [e-mail]; [e-mail]; Liron Himi
> > <[e-mail]>; Abhijit Ayarekar <[e-mail]>; Sathesh
> > B Edara <[e-mail]>; Satananda Burla <[e-mail]>;
> > [e-mail]; David S. Miller <[e-mail]>; Eric
> > Dumazet <[e-mail]>; Jakub Kicinski <[e-mail]>;
> > Paolo Abeni <[e-mail]>
> > Subject: [EXT] Re: [PATCH net-next v2 2/9] octeon_ep: poll for control
> > messages
> > 
> > External Email
> > 
> > ----------------------------------------------------------------------
> > On Tue, Nov 29, 2022 at 05:09:25AM -0800, Veerasenareddy Burru wrote:
> > > Poll for control messages until interrupts are enabled.
> > > All the interrupts are enabled in ndo_open().
> > 
> > So what are you saying if I have your device and didn't enable network
> > device, you will poll forever?
> Yes, Leon. It will poll periodically until network interface is enabled.

I don't know if it is acceptable behaviour in netdev, but it doesn't
sound right to me. What type of control messages will be sent by FW,
which PF should listen to them?

> > 
> > > Add ability to listen for notifications from firmware before ndo_open().
> > > Once interrupts are enabled, this polling is disabled and all the
> > > messages are processed by bottom half of interrupt handler.
> > >
> > > Signed-off-by: Veerasenareddy Burru <[e-mail]>
> > > Signed-off-by: Abhijit Ayarekar <[e-mail]>
> > > ---
> > > v1 -> v2:
> > >  * removed device status oct->status, as it is not required with the
> > >    modified implementation in 0001-xxxx.patch
> > >
> > >  .../marvell/octeon_ep/octep_cn9k_pf.c         | 49 +++++++++----------
> > >  .../ethernet/marvell/octeon_ep/octep_main.c   | 35 +++++++++++++
> > >  .../ethernet/marvell/octeon_ep/octep_main.h   | 11 ++++-
> > >  .../marvell/octeon_ep/octep_regs_cn9k_pf.h    |  4 ++
> > >  4 files changed, 71 insertions(+), 28 deletions(-)
> > >
> > > diff --git a/drivers/net/ethernet/marvell/octeon_ep/octep_cn9k_pf.c
> > > b/drivers/net/ethernet/marvell/octeon_ep/octep_cn9k_pf.c
> > > index 6ad88d0fe43f..ace2dfd1e918 100644
> > > --- a/drivers/net/ethernet/marvell/octeon_ep/octep_cn9k_pf.c
> > > +++ b/drivers/net/ethernet/marvell/octeon_ep/octep_cn9k_pf.c
> > > @@ -352,27 +352,36 @@ static void
> > octep_setup_mbox_regs_cn93_pf(struct octep_device *oct, int q_no)
> > >  	mbox->mbox_read_reg = oct->mmio[0].hw_addr +
> > > CN93_SDP_R_MBOX_VF_PF_DATA(q_no);  }
> > >
> > > -/* Mailbox Interrupt handler */
> > > -static void cn93_handle_pf_mbox_intr(struct octep_device *oct)
> > > +/* Process non-ioq interrupts required to keep pf interface running.
> > > + * OEI_RINT is needed for control mailbox  */ static int
> > > +octep_poll_non_ioq_interrupts_cn93_pf(struct octep_device *oct)
> > >  {
> > > -	u64 mbox_int_val = 0ULL, val = 0ULL, qno = 0ULL;
> > > +	u64 reg0;
> > > +	int handled = 0;
> > 
> > Reversed Christmas tree.
> Thanks for the feedback. Will revise the patch.

It is applicable to all patches.

And please fix your email client to properly add blank lines between
replies.

Thanks

> > 
> > Thanks

Veerasenareddy Burru Dec. 5, 2022, 4:46 a.m. UTC | #4

> -----Original Message-----
> From: Leon Romanovsky <[e-mail]>
> Sent: Thursday, December 1, 2022 12:11 AM
> To: Veerasenareddy Burru <[e-mail]>
> Cc: [e-mail]; [e-mail]; Liron Himi
> <[e-mail]>; Abhijit Ayarekar <[e-mail]>; Sathesh
> B Edara <[e-mail]>; Satananda Burla <[e-mail]>;
> [e-mail]; David S. Miller <[e-mail]>; Eric
> Dumazet <[e-mail]>; Jakub Kicinski <[e-mail]>;
> Paolo Abeni <[e-mail]>
> Subject: Re: [EXT] Re: [PATCH net-next v2 2/9] octeon_ep: poll for control
> messages
> 
> On Wed, Nov 30, 2022 at 03:44:30PM +0000, Veerasenareddy Burru wrote:
> >
> >
> > > -----Original Message-----
> > > From: Leon Romanovsky <[e-mail]>
> > > Sent: Wednesday, November 30, 2022 1:30 AM
> > > To: Veerasenareddy Burru <[e-mail]>
> > > Cc: [e-mail]; [e-mail]; Liron Himi
> > > <[e-mail]>; Abhijit Ayarekar <[e-mail]>;
> > > Sathesh B Edara <[e-mail]>; Satananda Burla
> > > <[e-mail]>; [e-mail]; David S. Miller
> > > <[e-mail]>; Eric Dumazet <[e-mail]>;
> Jakub
> > > Kicinski <[e-mail]>; Paolo Abeni <[e-mail]>
> > > Subject: [EXT] Re: [PATCH net-next v2 2/9] octeon_ep: poll for
> > > control messages
> > >
> > > External Email
> > >
> > > --------------------------------------------------------------------
> > > -- On Tue, Nov 29, 2022 at 05:09:25AM -0800, Veerasenareddy Burru
> > > wrote:
> > > > Poll for control messages until interrupts are enabled.
> > > > All the interrupts are enabled in ndo_open().
> > >
> > > So what are you saying if I have your device and didn't enable
> > > network device, you will poll forever?
> > Yes, Leon. It will poll periodically until network interface is enabled.
> 
> I don't know if it is acceptable behaviour in netdev, but it doesn't sound right
> to me. What type of control messages will be sent by FW, which PF should
> listen to them?
> 

These messages include periodic keep alive (heartbeat) messages from FW and control messages from VFs.
Every PF will be listening for its own control messages.

Thank you.

> > >
> > > > Add ability to listen for notifications from firmware before ndo_open().
> > > > Once interrupts are enabled, this polling is disabled and all the
> > > > messages are processed by bottom half of interrupt handler.
> > > >
> > > > Signed-off-by: Veerasenareddy Burru <[e-mail]>
> > > > Signed-off-by: Abhijit Ayarekar <[e-mail]>
> > > > ---
> > > > v1 -> v2:
> > > >  * removed device status oct->status, as it is not required with the
> > > >    modified implementation in 0001-xxxx.patch
> > > >
> > > >  .../marvell/octeon_ep/octep_cn9k_pf.c         | 49 +++++++++----------
> > > >  .../ethernet/marvell/octeon_ep/octep_main.c   | 35 +++++++++++++
> > > >  .../ethernet/marvell/octeon_ep/octep_main.h   | 11 ++++-
> > > >  .../marvell/octeon_ep/octep_regs_cn9k_pf.h    |  4 ++
> > > >  4 files changed, 71 insertions(+), 28 deletions(-)
> > > >
> > > > diff --git
> > > > a/drivers/net/ethernet/marvell/octeon_ep/octep_cn9k_pf.c
> > > > b/drivers/net/ethernet/marvell/octeon_ep/octep_cn9k_pf.c
> > > > index 6ad88d0fe43f..ace2dfd1e918 100644
> > > > --- a/drivers/net/ethernet/marvell/octeon_ep/octep_cn9k_pf.c
> > > > +++ b/drivers/net/ethernet/marvell/octeon_ep/octep_cn9k_pf.c
> > > > @@ -352,27 +352,36 @@ static void
> > > octep_setup_mbox_regs_cn93_pf(struct octep_device *oct, int q_no)
> > > >  	mbox->mbox_read_reg = oct->mmio[0].hw_addr +
> > > > CN93_SDP_R_MBOX_VF_PF_DATA(q_no);  }
> > > >
> > > > -/* Mailbox Interrupt handler */
> > > > -static void cn93_handle_pf_mbox_intr(struct octep_device *oct)
> > > > +/* Process non-ioq interrupts required to keep pf interface running.
> > > > + * OEI_RINT is needed for control mailbox  */ static int
> > > > +octep_poll_non_ioq_interrupts_cn93_pf(struct octep_device *oct)
> > > >  {
> > > > -	u64 mbox_int_val = 0ULL, val = 0ULL, qno = 0ULL;
> > > > +	u64 reg0;
> > > > +	int handled = 0;
> > >
> > > Reversed Christmas tree.
> > Thanks for the feedback. Will revise the patch.
> 
> It is applicable to all patches.
> 
> And please fix your email client to properly add blank lines between replies.
> 
> Thanks
> 
> > >
> > > Thanks

Leon Romanovsky Dec. 5, 2022, 8:10 a.m. UTC | #5

On Mon, Dec 05, 2022 at 04:46:31AM +0000, Veerasenareddy Burru wrote:
> 
> 
> > -----Original Message-----
> > From: Leon Romanovsky <[e-mail]>
> > Sent: Thursday, December 1, 2022 12:11 AM
> > To: Veerasenareddy Burru <[e-mail]>
> > Cc: [e-mail]; [e-mail]; Liron Himi
> > <[e-mail]>; Abhijit Ayarekar <[e-mail]>; Sathesh
> > B Edara <[e-mail]>; Satananda Burla <[e-mail]>;
> > [e-mail]; David S. Miller <[e-mail]>; Eric
> > Dumazet <[e-mail]>; Jakub Kicinski <[e-mail]>;
> > Paolo Abeni <[e-mail]>
> > Subject: Re: [EXT] Re: [PATCH net-next v2 2/9] octeon_ep: poll for control
> > messages
> > 
> > On Wed, Nov 30, 2022 at 03:44:30PM +0000, Veerasenareddy Burru wrote:
> > >
> > >
> > > > -----Original Message-----
> > > > From: Leon Romanovsky <[e-mail]>
> > > > Sent: Wednesday, November 30, 2022 1:30 AM
> > > > To: Veerasenareddy Burru <[e-mail]>
> > > > Cc: [e-mail]; [e-mail]; Liron Himi
> > > > <[e-mail]>; Abhijit Ayarekar <[e-mail]>;
> > > > Sathesh B Edara <[e-mail]>; Satananda Burla
> > > > <[e-mail]>; [e-mail]; David S. Miller
> > > > <[e-mail]>; Eric Dumazet <[e-mail]>;
> > Jakub
> > > > Kicinski <[e-mail]>; Paolo Abeni <[e-mail]>
> > > > Subject: [EXT] Re: [PATCH net-next v2 2/9] octeon_ep: poll for
> > > > control messages
> > > >
> > > > External Email
> > > >
> > > > --------------------------------------------------------------------
> > > > -- On Tue, Nov 29, 2022 at 05:09:25AM -0800, Veerasenareddy Burru
> > > > wrote:
> > > > > Poll for control messages until interrupts are enabled.
> > > > > All the interrupts are enabled in ndo_open().
> > > >
> > > > So what are you saying if I have your device and didn't enable
> > > > network device, you will poll forever?
> > > Yes, Leon. It will poll periodically until network interface is enabled.
> > 
> > I don't know if it is acceptable behaviour in netdev, but it doesn't sound right
> > to me. What type of control messages will be sent by FW, which PF should
> > listen to them?
> > 
> 
> These messages include periodic keep alive (heartbeat) messages from FW and control messages from VFs.
> Every PF will be listening for its own control messages.

@netdev, as I said, I don't know if it is valid behaviour in netdev.
Can you please comment?

Thanks

Jakub Kicinski Dec. 6, 2022, 12:16 a.m. UTC | #6

On Mon, 5 Dec 2022 10:10:34 +0200 Leon Romanovsky wrote:
> > These messages include periodic keep alive (heartbeat) messages
> > from FW and control messages from VFs. Every PF will be listening
> > for its own control messages.  
> 
> @netdev, as I said, I don't know if it is valid behaviour in netdev.
> Can you please comment?

Polling for control messages every 100ms?  Sure.

You say "valid in netdev" so perhaps you can educate us where/why it
would not be?

Leon Romanovsky Dec. 6, 2022, 8:58 a.m. UTC | #7

On Mon, Dec 05, 2022 at 04:16:26PM -0800, Jakub Kicinski wrote:
> On Mon, 5 Dec 2022 10:10:34 +0200 Leon Romanovsky wrote:
> > > These messages include periodic keep alive (heartbeat) messages
> > > from FW and control messages from VFs. Every PF will be listening
> > > for its own control messages.  
> > 
> > @netdev, as I said, I don't know if it is valid behaviour in netdev.
> > Can you please comment?
> 
> Polling for control messages every 100ms?  Sure.
> 
> You say "valid in netdev" so perhaps you can educate us where/why it
> would not be?

It doesn't seem right to me that idle device burns CPU cycles, while it
supports interrupts. If it needs "listen to FW", it will be much nicer to
install interrupts immediately and don't wait for netdev.

Thanks

Jakub Kicinski Dec. 6, 2022, 5:23 p.m. UTC | #8

On Tue, 6 Dec 2022 10:58:47 +0200 Leon Romanovsky wrote:
> > Polling for control messages every 100ms?  Sure.
> > 
> > You say "valid in netdev" so perhaps you can educate us where/why it
> > would not be?  
> 
> It doesn't seem right to me that idle device burns CPU cycles, while it
> supports interrupts. If it needs "listen to FW", it will be much nicer to
> install interrupts immediately and don't wait for netdev.

No doubt, if there is an alternative we can push for it to be
implemented. I guess this being yet another "IPU" there could
be possible workarounds in FW? As always with IPUs - hard to tell :/

If there is no alternative - it is what it is. 
It's up to customers to buy good HW.

That said, looking at what this set does - how are the VFs configured?
That's the showstopper for the series in my mind.

Veerasenareddy Burru Dec. 6, 2022, 9:19 p.m. UTC | #9

> -----Original Message-----
> From: Jakub Kicinski <[e-mail]>
> Sent: Tuesday, December 6, 2022 9:24 AM
> To: Leon Romanovsky <[e-mail]>
> Cc: Veerasenareddy Burru <[e-mail]>; David S. Miller
> <[e-mail]>; Eric Dumazet <[e-mail]>; Paolo
> Abeni <[e-mail]>; [e-mail]; linux-
> [e-mail]; Liron Himi <[e-mail]>; Abhijit Ayarekar
> <[e-mail]>; Sathesh B Edara <[e-mail]>;
> Satananda Burla <[e-mail]>; [e-mail]
> Subject: Re: [EXT] Re: [PATCH net-next v2 2/9] octeon_ep: poll for control
> messages
> 
> On Tue, 6 Dec 2022 10:58:47 +0200 Leon Romanovsky wrote:
> > > Polling for control messages every 100ms?  Sure.
> > >
> > > You say "valid in netdev" so perhaps you can educate us where/why it
> > > would not be?
> >
> > It doesn't seem right to me that idle device burns CPU cycles, while
> > it supports interrupts. If it needs "listen to FW", it will be much
> > nicer to install interrupts immediately and don't wait for netdev.
> 
> No doubt, if there is an alternative we can push for it to be implemented. I
> guess this being yet another "IPU" there could be possible workarounds in
> FW? As always with IPUs - hard to tell :/
> 
> If there is no alternative - it is what it is.
> It's up to customers to buy good HW.
> 
> That said, looking at what this set does - how are the VFs configured?
> That's the showstopper for the series in my mind.

VFs are created by writing to sriov_numvfs.

Jakub Kicinski Dec. 7, 2022, 1:26 a.m. UTC | #10

On Tue, 6 Dec 2022 21:19:26 +0000 Veerasenareddy Burru wrote:
> > That said, looking at what this set does - how are the VFs configured?
> > That's the showstopper for the series in my mind.  
> 
> VFs are created by writing to sriov_numvfs.

Configured, not enabled.

Veerasenareddy Burru Dec. 8, 2022, 3:17 a.m. UTC | #11

> -----Original Message-----
> From: Jakub Kicinski <[e-mail]>
> Sent: Tuesday, December 6, 2022 5:27 PM
> To: Veerasenareddy Burru <[e-mail]>
> Cc: Leon Romanovsky <[e-mail]>; David S. Miller
> <[e-mail]>; Eric Dumazet <[e-mail]>; Paolo
> Abeni <[e-mail]>; [e-mail]; linux-
> [e-mail]; Liron Himi <[e-mail]>; Abhijit Ayarekar
> <[e-mail]>; Sathesh B Edara <[e-mail]>;
> Satananda Burla <[e-mail]>; [e-mail]
> Subject: Re: [EXT] Re: [PATCH net-next v2 2/9] octeon_ep: poll for control
> messages
> 
> On Tue, 6 Dec 2022 21:19:26 +0000 Veerasenareddy Burru wrote:
> > > That said, looking at what this set does - how are the VFs configured?
> > > That's the showstopper for the series in my mind.
> >
> > VFs are created by writing to sriov_numvfs.
> 
> Configured, not enabled.

We have a follow up patch after this series implementing ndo_get_vf_xxx() and ndo_set_vf_xxx().

Thanks

Jakub Kicinski Dec. 8, 2022, 4:02 a.m. UTC | #12

On Thu, 8 Dec 2022 03:17:33 +0000 Veerasenareddy Burru wrote:
> We have a follow up patch after this series implementing
> ndo_get_vf_xxx() and ndo_set_vf_xxx().

We don't accept new drivers which use those interfaces.

Veerasenareddy Burru Dec. 8, 2022, 4:41 a.m. UTC | #13

> -----Original Message-----
> From: Jakub Kicinski <[e-mail]>
> Sent: Wednesday, December 7, 2022 8:02 PM
> To: Veerasenareddy Burru <[e-mail]>
> Cc: Leon Romanovsky <[e-mail]>; David S. Miller
> <[e-mail]>; Eric Dumazet <[e-mail]>; Paolo
> Abeni <[e-mail]>; [e-mail]; linux-
> [e-mail]; Liron Himi <[e-mail]>; Abhijit Ayarekar
> <[e-mail]>; Sathesh B Edara <[e-mail]>;
> Satananda Burla <[e-mail]>; [e-mail]
> Subject: Re: [EXT] Re: [PATCH net-next v2 2/9] octeon_ep: poll for control
> messages
> 
> On Thu, 8 Dec 2022 03:17:33 +0000 Veerasenareddy Burru wrote:
> > We have a follow up patch after this series implementing
> > ndo_get_vf_xxx() and ndo_set_vf_xxx().
> 
> We don't accept new drivers which use those interfaces.

Kindly suggest the acceptable interface.

Thanks.

Jakub Kicinski Dec. 8, 2022, 4:47 a.m. UTC | #14

On Thu, 8 Dec 2022 04:41:56 +0000 Veerasenareddy Burru wrote:
> > On Thu, 8 Dec 2022 03:17:33 +0000 Veerasenareddy Burru wrote:  
> > > We have a follow up patch after this series implementing
> > > ndo_get_vf_xxx() and ndo_set_vf_xxx().  
> > 
> > We don't accept new drivers which use those interfaces.  
> 
> Kindly suggest the acceptable interface.

Kindly make the minimal effort to follow the list :/

Perhaps others have the time to explain things to you, 
I believe my time is best spent elsewhere.

Veerasenareddy Burru Dec. 14, 2022, 7:15 a.m. UTC | #15

> -----Original Message-----
> From: Jakub Kicinski <[e-mail]>
> Sent: Wednesday, December 7, 2022 8:47 PM
> To: Veerasenareddy Burru <[e-mail]>
> Cc: Leon Romanovsky <[e-mail]>; David S. Miller
> <[e-mail]>; Eric Dumazet <[e-mail]>; Paolo
> Abeni <[e-mail]>; [e-mail]; linux-
> [e-mail]; Liron Himi <[e-mail]>; Abhijit Ayarekar
> <[e-mail]>; Sathesh B Edara <[e-mail]>;
> Satananda Burla <[e-mail]>; [e-mail]
> Subject: Re: [EXT] Re: [PATCH net-next v2 2/9] octeon_ep: poll for control
> messages
> 
> On Thu, 8 Dec 2022 04:41:56 +0000 Veerasenareddy Burru wrote:
> > > On Thu, 8 Dec 2022 03:17:33 +0000 Veerasenareddy Burru wrote:
> > > > We have a follow up patch after this series implementing
> > > > ndo_get_vf_xxx() and ndo_set_vf_xxx().
> > >
> > > We don't accept new drivers which use those interfaces.
> >
> > Kindly suggest the acceptable interface.
> 
> Kindly make the minimal effort to follow the list :/
> 
> Perhaps others have the time to explain things to you, I believe my time is
> best spent elsewhere.

I see the new drivers have to implement VF representors.
Will resubmit the patchset with representor support.

Patch

diff --git a/drivers/net/ethernet/marvell/octeon_ep/octep_cn9k_pf.c b/drivers/net/ethernet/marvell/octeon_ep/octep_cn9k_pf.c
index 6ad88d0fe43f..ace2dfd1e918 100644
--- a/drivers/net/ethernet/marvell/octeon_ep/octep_cn9k_pf.c
+++ b/drivers/net/ethernet/marvell/octeon_ep/octep_cn9k_pf.c
@@ -352,27 +352,36 @@  static void octep_setup_mbox_regs_cn93_pf(struct octep_device *oct, int q_no)
 	mbox->mbox_read_reg = oct->mmio[0].hw_addr + CN93_SDP_R_MBOX_VF_PF_DATA(q_no);
 }
 
-/* Mailbox Interrupt handler */
-static void cn93_handle_pf_mbox_intr(struct octep_device *oct)
+/* Process non-ioq interrupts required to keep pf interface running.
+ * OEI_RINT is needed for control mailbox
+ */
+static int octep_poll_non_ioq_interrupts_cn93_pf(struct octep_device *oct)
 {
-	u64 mbox_int_val = 0ULL, val = 0ULL, qno = 0ULL;
+	u64 reg0;
+	int handled = 0;
 
-	mbox_int_val = readq(oct->mbox[0]->mbox_int_reg);
-	for (qno = 0; qno < OCTEP_MAX_VF; qno++) {
-		val = readq(oct->mbox[qno]->mbox_read_reg);
-		dev_dbg(&oct->pdev->dev,
-			"PF MBOX READ: val:%llx from VF:%llx\n", val, qno);
+	/* Check for OEI INTR */
+	reg0 = octep_read_csr64(oct, CN93_SDP_EPF_OEI_RINT);
+	if (reg0) {
+		dev_info(&oct->pdev->dev,
+			 "Received OEI_RINT intr: 0x%llx\n",
+			 reg0);
+		octep_write_csr64(oct, CN93_SDP_EPF_OEI_RINT, reg0);
+		if (reg0 & CN93_SDP_EPF_OEI_RINT_DATA_BIT_MBOX)
+			queue_work(octep_wq, &oct->ctrl_mbox_task);
+
+		handled = 1;
 	}
 
-	writeq(mbox_int_val, oct->mbox[0]->mbox_int_reg);
+	return handled;
 }
 
 /* Interrupts handler for all non-queue generic interrupts. */
 static irqreturn_t octep_non_ioq_intr_handler_cn93_pf(void *dev)
 {
 	struct octep_device *oct = (struct octep_device *)dev;
-	struct pci_dev *pdev = oct->pdev;
 	u64 reg_val = 0;
+	struct pci_dev *pdev = oct->pdev;
 	int i = 0;
 
 	/* Check for IRERR INTR */
@@ -434,24 +443,9 @@  static irqreturn_t octep_non_ioq_intr_handler_cn93_pf(void *dev)
 		goto irq_handled;
 	}
 
-	/* Check for MBOX INTR */
-	reg_val = octep_read_csr64(oct, CN93_SDP_EPF_MBOX_RINT(0));
-	if (reg_val) {
-		dev_info(&pdev->dev,
-			 "Received MBOX_RINT intr: 0x%llx\n", reg_val);
-		cn93_handle_pf_mbox_intr(oct);
+	/* Check for MBOX INTR and OEI INTR */
+	if (octep_poll_non_ioq_interrupts_cn93_pf(oct))
 		goto irq_handled;
-	}
-
-	/* Check for OEI INTR */
-	reg_val = octep_read_csr64(oct, CN93_SDP_EPF_OEI_RINT);
-	if (reg_val) {
-		dev_info(&pdev->dev,
-			 "Received OEI_EINT intr: 0x%llx\n", reg_val);
-		octep_write_csr64(oct, CN93_SDP_EPF_OEI_RINT, reg_val);
-		queue_work(octep_wq, &oct->ctrl_mbox_task);
-		goto irq_handled;
-	}
 
 	/* Check for DMA INTR */
 	reg_val = octep_read_csr64(oct, CN93_SDP_EPF_DMA_RINT);
@@ -712,6 +706,7 @@  void octep_device_setup_cn93_pf(struct octep_device *oct)
 
 	oct->hw_ops.enable_interrupts = octep_enable_interrupts_cn93_pf;
 	oct->hw_ops.disable_interrupts = octep_disable_interrupts_cn93_pf;
+	oct->hw_ops.poll_non_ioq_interrupts = octep_poll_non_ioq_interrupts_cn93_pf;
 
 	oct->hw_ops.update_iq_read_idx = octep_update_iq_read_index_cn93_pf;
 
diff --git a/drivers/net/ethernet/marvell/octeon_ep/octep_main.c b/drivers/net/ethernet/marvell/octeon_ep/octep_main.c
index aa7d0ced9807..c07588461030 100644
--- a/drivers/net/ethernet/marvell/octeon_ep/octep_main.c
+++ b/drivers/net/ethernet/marvell/octeon_ep/octep_main.c
@@ -18,6 +18,7 @@ 
 #include "octep_main.h"
 #include "octep_ctrl_net.h"
 
+#define OCTEP_INTR_POLL_TIME_MSECS    100
 struct workqueue_struct *octep_wq;
 
 /* Supported Devices */
@@ -512,6 +513,7 @@  static int octep_open(struct net_device *netdev)
 	ret = octep_get_link_status(oct);
 	if (!ret)
 		octep_set_link_status(oct, true);
+	oct->poll_non_ioq_intr = false;
 
 	/* Enable the input and output queues for this Octeon device */
 	oct->hw_ops.enable_io_queues(oct);
@@ -573,6 +575,11 @@  static int octep_stop(struct net_device *netdev)
 	oct->hw_ops.reset_io_queues(oct);
 	octep_free_oqs(oct);
 	octep_free_iqs(oct);
+
+	oct->poll_non_ioq_intr = true;
+	queue_delayed_work(octep_wq, &oct->intr_poll_task,
+			   msecs_to_jiffies(OCTEP_INTR_POLL_TIME_MSECS));
+
 	netdev_info(netdev, "Device stopped !!\n");
 	return 0;
 }
@@ -865,6 +872,28 @@  static const struct net_device_ops octep_netdev_ops = {
 	.ndo_change_mtu          = octep_change_mtu,
 };
 
+/**
+ * octep_intr_poll_task - work queue task to process non-ioq interrupts.
+ *
+ * @work: pointer to mbox work_struct
+ *
+ * Process non-ioq interrupts to handle control mailbox, pfvf mailbox.
+ **/
+static void octep_intr_poll_task(struct work_struct *work)
+{
+	struct octep_device *oct = container_of(work, struct octep_device,
+						intr_poll_task.work);
+
+	if (!oct->poll_non_ioq_intr) {
+		dev_info(&oct->pdev->dev, "Interrupt poll task stopped.\n");
+		return;
+	}
+
+	oct->hw_ops.poll_non_ioq_interrupts(oct);
+	queue_delayed_work(octep_wq, &oct->intr_poll_task,
+			   msecs_to_jiffies(OCTEP_INTR_POLL_TIME_MSECS));
+}
+
 /**
  * octep_ctrl_mbox_task - work queue task to handle ctrl mbox messages.
  *
@@ -1101,6 +1130,10 @@  static int octep_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	}
 	INIT_WORK(&octep_dev->tx_timeout_task, octep_tx_timeout_task);
 	INIT_WORK(&octep_dev->ctrl_mbox_task, octep_ctrl_mbox_task);
+	INIT_DELAYED_WORK(&octep_dev->intr_poll_task, octep_intr_poll_task);
+	octep_dev->poll_non_ioq_intr = true;
+	queue_delayed_work(octep_wq, &octep_dev->intr_poll_task,
+			   msecs_to_jiffies(OCTEP_INTR_POLL_TIME_MSECS));
 
 	netdev->netdev_ops = &octep_netdev_ops;
 	octep_set_ethtool_ops(netdev);
@@ -1162,6 +1195,8 @@  static void octep_remove(struct pci_dev *pdev)
 	if (netdev->reg_state == NETREG_REGISTERED)
 		unregister_netdev(netdev);
 
+	oct->poll_non_ioq_intr = false;
+	cancel_delayed_work_sync(&oct->intr_poll_task);
 	octep_device_cleanup(oct);
 	pci_release_mem_regions(pdev);
 	free_netdev(netdev);
diff --git a/drivers/net/ethernet/marvell/octeon_ep/octep_main.h b/drivers/net/ethernet/marvell/octeon_ep/octep_main.h
index 123ffc13754d..70cc3e236cb4 100644
--- a/drivers/net/ethernet/marvell/octeon_ep/octep_main.h
+++ b/drivers/net/ethernet/marvell/octeon_ep/octep_main.h
@@ -73,6 +73,7 @@  struct octep_hw_ops {
 
 	void (*enable_interrupts)(struct octep_device *oct);
 	void (*disable_interrupts)(struct octep_device *oct);
+	int (*poll_non_ioq_interrupts)(struct octep_device *oct);
 
 	void (*enable_io_queues)(struct octep_device *oct);
 	void (*disable_io_queues)(struct octep_device *oct);
@@ -270,7 +271,15 @@  struct octep_device {
 
 	/* Work entry to handle ctrl mbox interrupt */
 	struct work_struct ctrl_mbox_task;
-
+	/* Wait queue for host to firmware requests */
+	wait_queue_head_t ctrl_req_wait_q;
+	/* List of objects waiting for h2f response */
+	struct list_head ctrl_req_wait_list;
+
+	/* Enable non-ioq interrupt polling */
+	bool poll_non_ioq_intr;
+	/* Work entry to poll non-ioq interrupts */
+	struct delayed_work intr_poll_task;
 };
 
 static inline u16 OCTEP_MAJOR_REV(struct octep_device *oct)
diff --git a/drivers/net/ethernet/marvell/octeon_ep/octep_regs_cn9k_pf.h b/drivers/net/ethernet/marvell/octeon_ep/octep_regs_cn9k_pf.h
index 3d5d39a52fe6..0466fd9a002d 100644
--- a/drivers/net/ethernet/marvell/octeon_ep/octep_regs_cn9k_pf.h
+++ b/drivers/net/ethernet/marvell/octeon_ep/octep_regs_cn9k_pf.h
@@ -364,4 +364,8 @@ 
 
 /* Number of non-queue interrupts in CN93xx */
 #define    CN93_NUM_NON_IOQ_INTR    16
+
+/* bit 0 for control mbox interrupt */
+#define CN93_SDP_EPF_OEI_RINT_DATA_BIT_MBOX	BIT_ULL(0)
+
 #endif /* _OCTEP_REGS_CN9K_PF_H_ */