Re: [PATCH 00/15] soc: octeontx2: Add RVU admin function driver

Re: [PATCH 00/15] soc: octeontx2: Add RVU admin function driver

[Arnd Bergmann]

On Tue, Aug 28, 2018 at 12:57 PM <sunil.kovvuri@gmail.com> wrote:
>
> From: Sunil Goutham <sgoutham@marvell.com>
>
> Resource virtualization unit (RVU) on Marvell's OcteonTX2 SOC supports
> multiple PCIe SRIOV physical functions (PFs) and virtual functions (VFs).
> PF0 is called administrative / admin function (AF) and has privilege access
> to registers to provision different RVU functional blocks to each of
> PF/VF.
>
> This admin function (AF) driver acts as a configuration / administrative
> software which provisions functional blocks to a PF/VF on demand for them
> to work as one of the following
>  - A basic network controller (i.e NIC).
>  - NIC with packet filtering, shaping and scheduling capabilities.
>  - A crypto device.
>  - A combination of above etc.
>
> PF/VFs communicate with admin function via a shared memory region.
> This patch series adds logic for the following
>  - RVU AF driver with functional blocks provisioning support
>  - Mailbox infrastructure for communication between AF and PFs.
>  - CGX driver which provides information about physcial network
>    interfaces which AF processes and forwards required info to
>    PF/VF drivers.
>
> This is the first set of patches out of 70 odd patches.
>
> Note: This driver neither receives any data nor processes it i.e no I/O,
>       just does the hardware configuration.

Hi Sunil,

Thanks for posting this first series, I'm glad we're seeing support for this
chip family making some progress.

My feeling overall is that we need a review from the network driver
folks more than the arm-soc team etc, and that maybe the driver
as a whole should go into drivers/net/ethernet.

We support some couple of similar hardware already that has
both support for virtual functions and for crypto offload, including
the Chelsio cxgb4, Mellanox mlx5, NXP DPAA and probably others,
and we need to ensure that the exposed interfaces are all
compatible, and that you use the correct subsystems and
in-kernel abstractions for thing that are common.

      Arnd

Re: [PATCH 04/15] soc: octeontx2: Add mailbox support infra

[Arnd Bergmann]

On Tue, Aug 28, 2018 at 2:48 PM Sunil Kovvuri <sunil.kovvuri@gmail.com> wrote:
>
> On Tue, Aug 28, 2018 at 5:33 PM Arnd Bergmann <arnd@arndb.de> wrote:
> >
> > On Tue, Aug 28, 2018 at 12:57 PM <sunil.kovvuri@gmail.com> wrote:
> > >
> > > From: Aleksey Makarov <amakarov@marvell.com>
> > >
> > > This patch adds mailbox support infrastructure APIs.
> > > Each RVU device has a dedicated 64KB mailbox region
> > > shared with it's peer for communication. RVU AF has
> > > a separate mailbox region shared with each of RVU PFs
> > > and a RVU PF has a separate region shared with each of
> > > it's VF.
> > >
> > > These set of APIs are used by this driver (RVU AF) and
> > > other RVU PF/VF drivers eg netdev, crypto e.t.c.
> > >
> > > Signed-off-by: Aleksey Makarov <amakarov@marvell.com>
> > > Signed-off-by: Sunil Goutham <sgoutham@marvell.com>
> > > Signed-off-by: Lukasz Bartosik <lbartosik@marvell.com>
> >
> > Why does this driver not use the drivers/mailbox/ infrastructure?
> >
> This is a common administrative software driver which will be handling requests
> from kernel drivers and as well as drivers in userspace applications.
> We had to keep mailbox communication infrastructure same across all usages.

Can you explain more about the usage of userspace applications
and what interface you plan to use into the kernel?

Do you things like AF_XDP and virtual machines, or something else?

        Arnd

Re: [PATCH 04/15] soc: octeontx2: Add mailbox support infra

[Sunil Kovvuri]

On Tue, Aug 28, 2018 at 6:22 PM Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Tue, Aug 28, 2018 at 2:48 PM Sunil Kovvuri <sunil.kovvuri@gmail.com> wrote:
> >
> > On Tue, Aug 28, 2018 at 5:33 PM Arnd Bergmann <arnd@arndb.de> wrote:
> > >
> > > On Tue, Aug 28, 2018 at 12:57 PM <sunil.kovvuri@gmail.com> wrote:
> > > >
> > > > From: Aleksey Makarov <amakarov@marvell.com>
> > > >
> > > > This patch adds mailbox support infrastructure APIs.
> > > > Each RVU device has a dedicated 64KB mailbox region
> > > > shared with it's peer for communication. RVU AF has
> > > > a separate mailbox region shared with each of RVU PFs
> > > > and a RVU PF has a separate region shared with each of
> > > > it's VF.
> > > >
> > > > These set of APIs are used by this driver (RVU AF) and
> > > > other RVU PF/VF drivers eg netdev, crypto e.t.c.
> > > >
> > > > Signed-off-by: Aleksey Makarov <amakarov@marvell.com>
> > > > Signed-off-by: Sunil Goutham <sgoutham@marvell.com>
> > > > Signed-off-by: Lukasz Bartosik <lbartosik@marvell.com>
> > >
> > > Why does this driver not use the drivers/mailbox/ infrastructure?
> > >
> > This is a common administrative software driver which will be handling requests
> > from kernel drivers and as well as drivers in userspace applications.
> > We had to keep mailbox communication infrastructure same across all usages.
>
> Can you explain more about the usage of userspace applications
> and what interface you plan to use into the kernel?

Any PCI device here irrespective in what domain (kernel or userspace)
they are in
use common mailbox communication. Which is
# Write a mailbox msg (format is agreed between all parties) into
shared (between AF and other PF/VFs)
   memory region and trigger a interrupt to admin function.
# Admin function processes the msg and puts reply in the same memory
region and trigger
   IRQ to the requesting device. If the device has a driver instance
in kernel then it uses
   IRQ and userspace applications does polling on the IRQ status bit.

>
> Do you things like AF_XDP and virtual machines, or something else?

I meant drivers in DPDK which may or may not use AF_XDP.
And yes if a PCI device is attached to a virtual machine then that
also uses the same mailbox communication.

Sunil.

>
>         Arnd

Re: [PATCH 00/15] soc: octeontx2: Add RVU admin function driver

[Sunil Kovvuri]

On Tue, Aug 28, 2018 at 5:53 PM Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Tue, Aug 28, 2018 at 12:57 PM <sunil.kovvuri@gmail.com> wrote:
> >
> > From: Sunil Goutham <sgoutham@marvell.com>
> >
> > Resource virtualization unit (RVU) on Marvell's OcteonTX2 SOC supports
> > multiple PCIe SRIOV physical functions (PFs) and virtual functions (VFs).
> > PF0 is called administrative / admin function (AF) and has privilege access
> > to registers to provision different RVU functional blocks to each of
> > PF/VF.
> >
> > This admin function (AF) driver acts as a configuration / administrative
> > software which provisions functional blocks to a PF/VF on demand for them
> > to work as one of the following
> >  - A basic network controller (i.e NIC).
> >  - NIC with packet filtering, shaping and scheduling capabilities.
> >  - A crypto device.
> >  - A combination of above etc.
> >
> > PF/VFs communicate with admin function via a shared memory region.
> > This patch series adds logic for the following
> >  - RVU AF driver with functional blocks provisioning support
> >  - Mailbox infrastructure for communication between AF and PFs.
> >  - CGX driver which provides information about physcial network
> >    interfaces which AF processes and forwards required info to
> >    PF/VF drivers.
> >
> > This is the first set of patches out of 70 odd patches.
> >
> > Note: This driver neither receives any data nor processes it i.e no I/O,
> >       just does the hardware configuration.
>
> Hi Sunil,
>
> Thanks for posting this first series, I'm glad we're seeing support for this
> chip family making some progress.

Thanks.

>
> My feeling overall is that we need a review from the network driver
> folks more than the arm-soc team etc, and that maybe the driver
> as a whole should go into drivers/net/ethernet.

This driver doesn't handle any network IO and moreever this driver has to handle
configuration requests from crypto driver as well. There will be
separate network and
crypto drivers which will be upstreamed into drivers/net/ethernet and
drivers/crypto.
And in future silicons there will be different types of functional
blocks which will be
added into this resource virtualization unit (RVU). Hence i thought
this driver is not a
right fit in drivers/net/ethernet.

>
> We support some couple of similar hardware already that has
> both support for virtual functions and for crypto offload, including
> the Chelsio cxgb4, Mellanox mlx5, NXP DPAA and probably others,

I agree, but i guess that is done to support inline crypto.
Here this driver has to handle requests from normal crypto driver
(drivers/crypto) as well.

> and we need to ensure that the exposed interfaces are all
> compatible, and that you use the correct subsystems and
> in-kernel abstractions for thing that are common.
>
>       Arnd

Re: [PATCH 00/15] soc: octeontx2: Add RVU admin function driver

[Sunil Kovvuri]

On Tue, Aug 28, 2018 at 6:54 PM Sunil Kovvuri <sunil.kovvuri@gmail.com> wrote:
>
> On Tue, Aug 28, 2018 at 5:53 PM Arnd Bergmann <arnd@arndb.de> wrote:
> >
> > On Tue, Aug 28, 2018 at 12:57 PM <sunil.kovvuri@gmail.com> wrote:
> > >
> > > From: Sunil Goutham <sgoutham@marvell.com>
> > >
> > > Resource virtualization unit (RVU) on Marvell's OcteonTX2 SOC supports
> > > multiple PCIe SRIOV physical functions (PFs) and virtual functions (VFs).
> > > PF0 is called administrative / admin function (AF) and has privilege access
> > > to registers to provision different RVU functional blocks to each of
> > > PF/VF.
> > >
> > > This admin function (AF) driver acts as a configuration / administrative
> > > software which provisions functional blocks to a PF/VF on demand for them
> > > to work as one of the following
> > >  - A basic network controller (i.e NIC).
> > >  - NIC with packet filtering, shaping and scheduling capabilities.
> > >  - A crypto device.
> > >  - A combination of above etc.
> > >
> > > PF/VFs communicate with admin function via a shared memory region.
> > > This patch series adds logic for the following
> > >  - RVU AF driver with functional blocks provisioning support
> > >  - Mailbox infrastructure for communication between AF and PFs.
> > >  - CGX driver which provides information about physcial network
> > >    interfaces which AF processes and forwards required info to
> > >    PF/VF drivers.
> > >
> > > This is the first set of patches out of 70 odd patches.
> > >
> > > Note: This driver neither receives any data nor processes it i.e no I/O,
> > >       just does the hardware configuration.
> >
> > Hi Sunil,
> >
> > Thanks for posting this first series, I'm glad we're seeing support for this
> > chip family making some progress.
>
> Thanks.
>
> >
> > My feeling overall is that we need a review from the network driver
> > folks more than the arm-soc team etc, and that maybe the driver
> > as a whole should go into drivers/net/ethernet.
>
> This driver doesn't handle any network IO and moreever this driver has to handle
> configuration requests from crypto driver as well. There will be
> separate network and
> crypto drivers which will be upstreamed into drivers/net/ethernet and
> drivers/crypto.
> And in future silicons there will be different types of functional
> blocks which will be
> added into this resource virtualization unit (RVU). Hence i thought
> this driver is not a
> right fit in drivers/net/ethernet.
>
> >
> > We support some couple of similar hardware already that has
> > both support for virtual functions and for crypto offload, including
> > the Chelsio cxgb4, Mellanox mlx5, NXP DPAA and probably others,
>
> I agree, but i guess that is done to support inline crypto.
> Here this driver has to handle requests from normal crypto driver
> (drivers/crypto) as well.
>
> > and we need to ensure that the exposed interfaces are all
> > compatible, and that you use the correct subsystems and
> > in-kernel abstractions for thing that are common.
> >
> >       Arnd

Hi Arnd,

I hope i have answered your queries.
Let us know if you have any more queries or feedback w.r.t to a
individual patch or the patchset on the whole.

Thanks,
Sunil.

Re: [PATCH 00/15] soc: octeontx2: Add RVU admin function driver

[Andrew Lunn]

> > > My feeling overall is that we need a review from the network driver
> > > folks more than the arm-soc team etc, and that maybe the driver
> > > as a whole should go into drivers/net/ethernet.
> >
> > This driver doesn't handle any network IO and moreever this driver has to handle
> > configuration requests from crypto driver as well. There will be
> > separate network and
> > crypto drivers which will be upstreamed into drivers/net/ethernet and
> > drivers/crypto.
> > And in future silicons there will be different types of functional
> > blocks which will be
> > added into this resource virtualization unit (RVU). Hence i thought
> > this driver is not a
> > right fit in drivers/net/ethernet.

Hi Sunil

Do you have a git branch for everything? I would like to look at the
actual Ethernet driver, and the full API this driver exports to other
drivers.

I think there real question here is, do you have split between this
driver and the actual device drivers in the right place? For me, link
up/down detection should be in the Ethernet driver, since it is not
shared with the crypto driver.

       Thanks
	Andrew

Re: [PATCH 04/15] soc: octeontx2: Add mailbox support infra

[Arnd Bergmann]

On Tue, Aug 28, 2018 at 3:23 PM Sunil Kovvuri <sunil.kovvuri@gmail.com> wrote:
>
> On Tue, Aug 28, 2018 at 6:22 PM Arnd Bergmann <arnd@arndb.de> wrote:
> >
> > On Tue, Aug 28, 2018 at 2:48 PM Sunil Kovvuri <sunil.kovvuri@gmail.com> wrote:
> > >
> > > On Tue, Aug 28, 2018 at 5:33 PM Arnd Bergmann <arnd@arndb.de> wrote:
> > > >
> > > > On Tue, Aug 28, 2018 at 12:57 PM <sunil.kovvuri@gmail.com> wrote:
> > > > >
> > > > > From: Aleksey Makarov <amakarov@marvell.com>
> > > > >
> > > > > This patch adds mailbox support infrastructure APIs.
> > > > > Each RVU device has a dedicated 64KB mailbox region
> > > > > shared with it's peer for communication. RVU AF has
> > > > > a separate mailbox region shared with each of RVU PFs
> > > > > and a RVU PF has a separate region shared with each of
> > > > > it's VF.
> > > > >
> > > > > These set of APIs are used by this driver (RVU AF) and
> > > > > other RVU PF/VF drivers eg netdev, crypto e.t.c.
> > > > >
> > > > > Signed-off-by: Aleksey Makarov <amakarov@marvell.com>
> > > > > Signed-off-by: Sunil Goutham <sgoutham@marvell.com>
> > > > > Signed-off-by: Lukasz Bartosik <lbartosik@marvell.com>
> > > >
> > > > Why does this driver not use the drivers/mailbox/ infrastructure?
> > > >
> > > This is a common administrative software driver which will be handling requests
> > > from kernel drivers and as well as drivers in userspace applications.
> > > We had to keep mailbox communication infrastructure same across all usages.
> >
> > Can you explain more about the usage of userspace applications
> > and what interface you plan to use into the kernel?
>
> Any PCI device here irrespective in what domain (kernel or userspace)
> they are in
> use common mailbox communication. Which is
> # Write a mailbox msg (format is agreed between all parties) into
> shared (between AF and other PF/VFs)
>    memory region and trigger a interrupt to admin function.
> # Admin function processes the msg and puts reply in the same memory
> region and trigger
>    IRQ to the requesting device. If the device has a driver instance
> in kernel then it uses
>    IRQ and userspace applications does polling on the IRQ status bit.

Ok, so the mailbox here is a communication mechanism between
two device drivers that may run on the same kernel, or in different
instances (user space, virtual machine, ...), but each driver
only talks to the mailbox visible in its own device, right?

What is the purpose of the exported interface then? Is this
just an abstraction so each of the drivers can talk to its own
mailbox using a set of common helper functions?

      Arnd

Re: [PATCH 00/15] soc: octeontx2: Add RVU admin function driver

[Sunil Kovvuri]

On Thu, Aug 30, 2018 at 6:57 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> > > > My feeling overall is that we need a review from the network driver
> > > > folks more than the arm-soc team etc, and that maybe the driver
> > > > as a whole should go into drivers/net/ethernet.
> > >
> > > This driver doesn't handle any network IO and moreever this driver has to handle
> > > configuration requests from crypto driver as well. There will be
> > > separate network and
> > > crypto drivers which will be upstreamed into drivers/net/ethernet and
> > > drivers/crypto.
> > > And in future silicons there will be different types of functional
> > > blocks which will be
> > > added into this resource virtualization unit (RVU). Hence i thought
> > > this driver is not a
> > > right fit in drivers/net/ethernet.
>
> Hi Sunil
>
> Do you have a git branch for everything? I would like to look at the
> actual Ethernet driver, and the full API this driver exports to other
> drivers.

Hi Andrew,

I have pushed all patches into a github repo for your reference.
These are not the final patches, i still need to do some minor changes
before submitting
(i mean otherthan the ones i already submitted).

AF driver & CGX drivers:
https://github.com/sunilkovvuri/rvu_drivers/tree/master/drivers/soc/marvell/octeontx2

Ethernet drivers PF and VF
https://github.com/sunilkovvuri/rvu_drivers/tree/master/drivers/net/ethernet/marvell/octeontx2

Info exported/shared by AF driver with other drivers is
# Mailbox communication APIs, message IDs, message structs e.t.c
   https://github.com/sunilkovvuri/rvu_drivers/blob/master/drivers/soc/marvell/octeontx2/mbox.c
   https://github.com/sunilkovvuri/rvu_drivers/blob/master/drivers/soc/marvell/octeontx2/mbox.h
# Other structs and APIs
    https://github.com/sunilkovvuri/rvu_drivers/blob/master/drivers/soc/marvell/octeontx2/common.h
    https://github.com/sunilkovvuri/rvu_drivers/blob/master/drivers/soc/marvell/octeontx2/rvu_struct.h

>
> I think there real question here is, do you have split between this
> driver and the actual device drivers in the right place? For me, link
> up/down detection should be in the Ethernet driver, since it is not
> shared with the crypto driver.
>

As mentioned in the patch '[PATCH 13/15] soc: octeontx2: Add support
for CGX link management',
link up/down is detected by firmware. And firmware triggers a IRQ to
CGX driver, which then
takes the new status and sends a update to ethernet driver via a
mailbox communication.

Ethernet driver detects the link change and does the necessary stuff.
https://github.com/sunilkovvuri/rvu_drivers/blob/master/drivers/net/ethernet/marvell/octeontx2/otx2_pf.c#L116


>        Thanks
>         Andrew

Re: [PATCH 04/15] soc: octeontx2: Add mailbox support infra

[Sunil Kovvuri]

On Thu, Aug 30, 2018 at 7:27 PM Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Tue, Aug 28, 2018 at 3:23 PM Sunil Kovvuri <sunil.kovvuri@gmail.com> wrote:
> >
> > On Tue, Aug 28, 2018 at 6:22 PM Arnd Bergmann <arnd@arndb.de> wrote:
> > >
> > > On Tue, Aug 28, 2018 at 2:48 PM Sunil Kovvuri <sunil.kovvuri@gmail.com> wrote:
> > > >
> > > > On Tue, Aug 28, 2018 at 5:33 PM Arnd Bergmann <arnd@arndb.de> wrote:
> > > > >
> > > > > On Tue, Aug 28, 2018 at 12:57 PM <sunil.kovvuri@gmail.com> wrote:
> > > > > >
> > > > > > From: Aleksey Makarov <amakarov@marvell.com>
> > > > > >
> > > > > > This patch adds mailbox support infrastructure APIs.
> > > > > > Each RVU device has a dedicated 64KB mailbox region
> > > > > > shared with it's peer for communication. RVU AF has
> > > > > > a separate mailbox region shared with each of RVU PFs
> > > > > > and a RVU PF has a separate region shared with each of
> > > > > > it's VF.
> > > > > >
> > > > > > These set of APIs are used by this driver (RVU AF) and
> > > > > > other RVU PF/VF drivers eg netdev, crypto e.t.c.
> > > > > >
> > > > > > Signed-off-by: Aleksey Makarov <amakarov@marvell.com>
> > > > > > Signed-off-by: Sunil Goutham <sgoutham@marvell.com>
> > > > > > Signed-off-by: Lukasz Bartosik <lbartosik@marvell.com>
> > > > >
> > > > > Why does this driver not use the drivers/mailbox/ infrastructure?
> > > > >
> > > > This is a common administrative software driver which will be handling requests
> > > > from kernel drivers and as well as drivers in userspace applications.
> > > > We had to keep mailbox communication infrastructure same across all usages.
> > >
> > > Can you explain more about the usage of userspace applications
> > > and what interface you plan to use into the kernel?
> >
> > Any PCI device here irrespective in what domain (kernel or userspace)
> > they are in
> > use common mailbox communication. Which is
> > # Write a mailbox msg (format is agreed between all parties) into
> > shared (between AF and other PF/VFs)
> >    memory region and trigger a interrupt to admin function.
> > # Admin function processes the msg and puts reply in the same memory
> > region and trigger
> >    IRQ to the requesting device. If the device has a driver instance
> > in kernel then it uses
> >    IRQ and userspace applications does polling on the IRQ status bit.
>
> Ok, so the mailbox here is a communication mechanism between
> two device drivers that may run on the same kernel, or in different
> instances (user space, virtual machine, ...), but each driver
> only talks to the mailbox visible in its own device, right?

Yes.

>
> What is the purpose of the exported interface then? Is this
> just an abstraction so each of the drivers can talk to its own
> mailbox using a set of common helper functions?
>
>       Arnd

Yes, that's correct.

In kernel there will be a minimum of 3 drivers which will use this
mailbox communication.
So instead of duplicating APIs and structures in every driver, we
thought of adding them
in this AF driver and export them to ethernet and crypto drivers.

Thanks,
Sunil.

Re: [PATCH 04/15] soc: octeontx2: Add mailbox support infra

[Arnd Bergmann]

On Thu, Aug 30, 2018 at 8:37 PM Sunil Kovvuri <sunil.kovvuri@gmail.com> wrote:
> On Thu, Aug 30, 2018 at 7:27 PM Arnd Bergmann <arnd@arndb.de> wrote:
> > On Tue, Aug 28, 2018 at 3:23 PM Sunil Kovvuri <sunil.kovvuri@gmail.com> wrote:
> > > On Tue, Aug 28, 2018 at 6:22 PM Arnd Bergmann <arnd@arndb.de> wrote:
> > > > On Tue, Aug 28, 2018 at 2:48 PM Sunil Kovvuri <sunil.kovvuri@gmail.com> wrote:
> > > Any PCI device here irrespective in what domain (kernel or userspace)
> > > they are in
> > > use common mailbox communication. Which is
> > > # Write a mailbox msg (format is agreed between all parties) into
> > > shared (between AF and other PF/VFs)
> > >    memory region and trigger a interrupt to admin function.
> > > # Admin function processes the msg and puts reply in the same memory
> > > region and trigger
> > >    IRQ to the requesting device. If the device has a driver instance
> > > in kernel then it uses
> > >    IRQ and userspace applications does polling on the IRQ status bit.
> >
> > What is the purpose of the exported interface then? Is this
> > just an abstraction so each of the drivers can talk to its own
> > mailbox using a set of common helper functions?
> >
>
> Yes, that's correct.
>
> In kernel there will be a minimum of 3 drivers which will use this
> mailbox communication.
> So instead of duplicating APIs and structures in every driver, we
> thought of adding them in this AF driver and export them to ethernet
> and crypto drivers.

Ok. My feeling is then that the API is fine, but that it should not
be part of the AF module but rather be a standalone module.

My comment about the generic mailbox API no longer applies
here: you don't have a single shared mailbox hardware interface,
but each device has its own mailbox register set, so there
is no point in setting up a separate device for it, but I see
no need for creating an artificial dependency on the AF
driver. E.g. in a virtual machine that only has one ethernet
interface, you otherwise wouldn't load that driver, right?

         Arnd

Re: [PATCH 04/15] soc: octeontx2: Add mailbox support infra

[Sunil Kovvuri]

On Fri, Aug 31, 2018 at 7:46 PM Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Thu, Aug 30, 2018 at 8:37 PM Sunil Kovvuri <sunil.kovvuri@gmail.com> wrote:
> > On Thu, Aug 30, 2018 at 7:27 PM Arnd Bergmann <arnd@arndb.de> wrote:
> > > On Tue, Aug 28, 2018 at 3:23 PM Sunil Kovvuri <sunil.kovvuri@gmail.com> wrote:
> > > > On Tue, Aug 28, 2018 at 6:22 PM Arnd Bergmann <arnd@arndb.de> wrote:
> > > > > On Tue, Aug 28, 2018 at 2:48 PM Sunil Kovvuri <sunil.kovvuri@gmail.com> wrote:
> > > > Any PCI device here irrespective in what domain (kernel or userspace)
> > > > they are in
> > > > use common mailbox communication. Which is
> > > > # Write a mailbox msg (format is agreed between all parties) into
> > > > shared (between AF and other PF/VFs)
> > > >    memory region and trigger a interrupt to admin function.
> > > > # Admin function processes the msg and puts reply in the same memory
> > > > region and trigger
> > > >    IRQ to the requesting device. If the device has a driver instance
> > > > in kernel then it uses
> > > >    IRQ and userspace applications does polling on the IRQ status bit.
> > >
> > > What is the purpose of the exported interface then? Is this
> > > just an abstraction so each of the drivers can talk to its own
> > > mailbox using a set of common helper functions?
> > >
> >
> > Yes, that's correct.
> >
> > In kernel there will be a minimum of 3 drivers which will use this
> > mailbox communication.
> > So instead of duplicating APIs and structures in every driver, we
> > thought of adding them in this AF driver and export them to ethernet
> > and crypto drivers.
>
> Ok. My feeling is then that the API is fine, but that it should not
> be part of the AF module but rather be a standalone module.
>
> My comment about the generic mailbox API no longer applies
> here: you don't have a single shared mailbox hardware interface,
> but each device has its own mailbox register set, so there
> is no point in setting up a separate device for it, but I see
> no need for creating an artificial dependency on the AF
> driver. E.g. in a virtual machine that only has one ethernet
> interface, you otherwise wouldn't load that driver, right?
>
>          Arnd

Good point, thanks for catching this.
Will look into this and post a v2 series.

Thanks,
Sunil.