RE: [PATCH v10 01/20] dlb: add skeleton for DLB driver
> -Original Message- > From: Greg KH > On Mon, Mar 08, 2021 at 08:00:00PM +0000, Chen, Mike Ximing wrote: > > > > Hi Greg, > > > > While waiting for the feedback from the networking maintainers, I am > > wondering if you have any other comments/suggestions that I should address > > in parallel. > > It's in my "to-review" queue, which is huge at the moment. But the > networking developers review will determine how this should go forward, > so I'll just wait for them to get to it. > I see the status of the submission (to netdev) is now marked as "Not Applicable" at netdev's patchwork site https://patchwork.kernel.org/project/netdevbpf/list/?series==197673=*==both= Looks like that the patch set is considered as not being networking related. (?) Thanks Mike
RE: [PATCH v10 01/20] dlb: add skeleton for DLB driver
> -Original Message- > From: Dan Williams > Sent: Monday, March 8, 2021 5:06 PM > To: Greg KH > Cc: Chen, Mike Ximing ; Linux Kernel Mailing List > ; Arnd Bergmann ; Pierre-Louis > Bossart ; Brandeburg, Jesse > ; k...@kernel.org; da...@davemloft.net; > Karlsson, Magnus ; Laatz, Kevin > ; maxi...@mellanox.com > Subject: Re: [PATCH v10 01/20] dlb: add skeleton for DLB driver > > [ add Kevin and Maxim ] > > On Mon, Mar 8, 2021 at 12:13 PM Greg KH wrote: > > > > On Mon, Mar 08, 2021 at 08:00:00PM +, Chen, Mike Ximing wrote: > [..] > > > Hi Greg, > > > > > > While waiting for the feedback from the networking maintainers, I am > > > wondering if you have any other comments/suggestions that I should > address > > > in parallel. > > > > It's in my "to-review" queue, which is huge at the moment. But the > > networking developers review will determine how this should go forward, > > so I'll just wait for them to get to it. > > > Mike, > > Perhaps it would help to solicit feedback from other networking > developers that have delivered kernel features to be consumed by DPDK. > A coarse look potentially flagsKevin and Maxim working on AF_XDP > updates that DPDK consumes, Perhaps they would be willing to take a > look? Thanks, Dan. Kevin/Maxim, Please feel free to let me know if you have any question about the submission. Thanks Mike
RE: [PATCH v10 01/20] dlb: add skeleton for DLB driver
> -Original Message- > From: Dan Williams > Sent: Tuesday, February 9, 2021 11:30 AM > To: Greg KH > Cc: Chen, Mike Ximing ; Linux Kernel Mailing List > ; Arnd Bergmann ; Pierre-Louis > Bossart ; Gage Eads > > Subject: Re: [PATCH v10 01/20] dlb: add skeleton for DLB driver > > On Tue, Feb 9, 2021 at 5:36 AM Greg KH wrote: > > > > On Wed, Jan 27, 2021 at 04:56:22PM -0600, Mike Ximing Chen wrote: > > > Add basic driver functionality (load, unload, probe, and remove callbacks) > > > for the DLB driver. > > > > > > Add documentation which describes in detail the hardware, the user > > > interface, device interrupts, and the driver's power-management strategy. > > > For more details about the driver see the documentation in the patch. > > > > > > Add a DLB entry to the MAINTAINERS file. > > > > > > Signed-off-by: Gage Eads > > > Signed-off-by: Mike Ximing Chen > > > Reviewed-by: Magnus Karlsson > > > Reviewed-by: Dan Williams > > > --- > > > Documentation/misc-devices/dlb.rst | 259 +++ > > > Documentation/misc-devices/index.rst | 1 + > > > MAINTAINERS | 8 + > > > drivers/misc/Kconfig | 1 + > > > drivers/misc/Makefile| 1 + > > > drivers/misc/dlb/Kconfig | 18 ++ > > > drivers/misc/dlb/Makefile| 9 + > > > drivers/misc/dlb/dlb_hw_types.h | 32 > > > drivers/misc/dlb/dlb_main.c | 156 > > > drivers/misc/dlb/dlb_main.h | 37 > > > 10 files changed, 522 insertions(+) > > > create mode 100644 Documentation/misc-devices/dlb.rst > > > create mode 100644 drivers/misc/dlb/Kconfig > > > create mode 100644 drivers/misc/dlb/Makefile > > > create mode 100644 drivers/misc/dlb/dlb_hw_types.h > > > create mode 100644 drivers/misc/dlb/dlb_main.c > > > create mode 100644 drivers/misc/dlb/dlb_main.h > > > > > > diff --git a/Documentation/misc-devices/dlb.rst b/Documentation/misc- > devices/dlb.rst > > > new file mode 100644 > > > index ..aa79be07ee49 > > > --- /dev/null > > > +++ b/Documentation/misc-devices/dlb.rst > > > @@ -0,0 +1,259 @@ > > > +.. SPDX-License-Identifier: GPL-2.0-only > > > + > > > +=== > > > +Intel(R) Dynamic Load Balancer Overview > > > +=== > > > + > > > +:Authors: Gage Eads and Mike Ximing Chen > > > + > > > +Contents > > > + > > > + > > > +- Introduction > > > +- Scheduling > > > +- Queue Entry > > > +- Port > > > +- Queue > > > +- Credits > > > +- Scheduling Domain > > > +- Interrupts > > > +- Power Management > > > +- User Interface > > > +- Reset > > > + > > > +Introduction > > > + > > > + > > > +The Intel(r) Dynamic Load Balancer (Intel(r) DLB) is a PCIe device that > > > +provides load-balanced, prioritized scheduling of core-to-core > communication. > > > + > > > +Intel DLB is an accelerator for the event-driven programming model of > > > +DPDK's Event Device Library[2]. The library is used in packet processing > > > +pipelines that arrange for multi-core scalability, dynamic > > > load-balancing, > and > > > +variety of packet distribution and synchronization schemes. > > > > As this is a networking related thing, I would like you to get the > > proper reviews/acks from the networking maintainers before I can take > > this. > > > > Or, if they think it has nothing to do with networking, that's fine too, > > but please do not try to route around them. > > To be clear, I did not sense any attempt to route around networking > review as it appeared generically centered around hardware accelerated > IPC. At the same time I don't know what I don't know about how this > might interact with networking initiatives so the review trip seems > reasonable to me. Hi Greg, While waiting for the feedback from the networking maintainers, I am wondering if you have any other comments/suggestions that I should address in parallel. Thanks Mike
RE: [PATCH v10 01/20] dlb: add skeleton for DLB driver
> -Original Message- > From: gre...@linuxfoundation.org > Sent: Thursday, February 18, 2021 2:53 AM > To: Chen, Mike Ximing > Cc: net...@vger.kernel.org; Linux Kernel Mailing List ker...@vger.kernel.org>; da...@davemloft.net; k...@kernel.org; a...@arndb.de; > Williams, Dan J ; > pierre-louis.boss...@linux.intel.com > Subject: Re: [PATCH v10 01/20] dlb: add skeleton for DLB driver > > On Thu, Feb 18, 2021 at 07:34:31AM +, Chen, Mike Ximing wrote: > > > > > > > -Original Message- > > > From: Mike Ximing Chen > > > Sent: Wednesday, February 10, 2021 12:54 PM > > > To: net...@vger.kernel.org > > > Cc: da...@davemloft.net; k...@kernel.org; a...@arndb.de; > > > gre...@linuxfoundation.org; Williams, Dan J ; > pierre- > > > louis.boss...@linux.intel.com; Gage Eads > > > Subject: [PATCH v10 01/20] dlb: add skeleton for DLB driver > > > > > > diff --git a/Documentation/misc-devices/dlb.rst b/Documentation/misc- > > > devices/dlb.rst > > > new file mode 100644 > > > index ..aa79be07ee49 > > > --- /dev/null > > > +++ b/Documentation/misc-devices/dlb.rst > > > @@ -0,0 +1,259 @@ > > > +.. SPDX-License-Identifier: GPL-2.0-only > > > + > > > +=== > > > +Intel(R) Dynamic Load Balancer Overview > > > +=== > > > + > > > +:Authors: Gage Eads and Mike Ximing Chen > > > + > > > +Contents > > > + > > > + > > > +- Introduction > > > +- Scheduling > > > +- Queue Entry > > > +- Port > > > +- Queue > > > +- Credits > > > +- Scheduling Domain > > > +- Interrupts > > > +- Power Management > > > +- User Interface > > > +- Reset > > > + > > > +Introduction > > > + > > > + > > > +The Intel(r) Dynamic Load Balancer (Intel(r) DLB) is a PCIe device that > > > +provides load-balanced, prioritized scheduling of core-to-core > > > communication. > > > + > > > +Intel DLB is an accelerator for the event-driven programming model of > > > +DPDK's Event Device Library[2]. The library is used in packet processing > > > +pipelines that arrange for multi-core scalability, dynamic > > > load-balancing, and > > > +variety of packet distribution and synchronization schemes. > > > + > > > +Intel DLB device consists of queues and arbiters that connect producer > > > +cores and consumer cores. The device implements load-balanced queueing > > > features > > > +including: > > > +- Lock-free multi-producer/multi-consumer operation. > > > +- Multiple priority levels for varying traffic types. > > > +- 'Direct' traffic (i.e. multi-producer/single-consumer) > > > +- Simple unordered load-balanced distribution. > > > +- Atomic lock free load balancing across multiple consumers. > > > +- Queue element reordering feature allowing ordered load-balanced > > > distribution. > > > + > > > > Hi Jakub/Dave, > > This is a device driver for a HW core-to-core communication accelerator. It > > is > submitted > > to "linux-kernel" for a module under device/misc. Greg suggested (see > > below) that > we > > also sent it to you for any potential feedback in case there is any > > interaction with > > networking initiatives. The device is used to handle the load balancing > > among CPU > cores > > after the packets are received and forwarded to CPU. We don't think it > > interferes > > with networking operations, but would appreciate very much your > review/comment on this. > > It's the middle of the merge window, getting maintainers to review new > stuff until after 5.12-rc1 is out is going to be a very difficult thing > to do. > Hi Jakub/Dave, Just wonder if you had a chance to take a look at our patch. With the close of 5.12 merge window, we would like to get the process moving again. > In the meantime, why don't you all help out and review submitted patches > to the mailing lists for the subsystems you all are trying to get this > patch into. I know maintainers would appreciate the help, right? > > thanks, > > greg k-h Did a few reviews last weekend, and will continue to help. Thanks Mike
RE: [PATCH 4/4] staging:rtl8712: replace cap_* definitions with native kernel WLAN_CAPABILITY_*
> -Original Message- > From: Ivan Safonov > Sent: Saturday, February 27, 2021 5:23 PM > To: Greg Kroah-Hartman > Cc: Florian Schilhabel ; Larry Finger > ; Michael Straube ; Pascal > Terjan ; de...@driverdev.osuosl.org; linux- > ker...@vger.kernel.org; Ivan Safonov > Subject: [PATCH 4/4] staging:rtl8712: replace cap_* definitions with native > kernel > WLAN_CAPABILITY_* > > cap_* definitions duplicate WLAN_CAPABILITY_*. Remove cap_* definitions, > improve code consistency. > > Signed-off-by: Ivan Safonov > --- > drivers/staging/rtl8712/ieee80211.c | 6 +++--- > drivers/staging/rtl8712/wifi.h | 7 --- > 2 files changed, 3 insertions(+), 10 deletions(-) > > diff --git a/drivers/staging/rtl8712/ieee80211.c > b/drivers/staging/rtl8712/ieee80211.c > index b4a099169c7c..13fc3c1ec0db 100644 > --- a/drivers/staging/rtl8712/ieee80211.c > +++ b/drivers/staging/rtl8712/ieee80211.c > @@ -173,11 +173,11 @@ int r8712_generate_ie(struct registry_priv > *registrypriv) > ie += 2; > /*capability info*/ > *(u16 *)ie = 0; > - *(__le16 *)ie |= cpu_to_le16(cap_IBSS); > + *(__le16 *)ie |= cpu_to_le16(WLAN_CAPABILITY_IBSS); > if (registrypriv->preamble == PREAMBLE_SHORT) > - *(__le16 *)ie |= cpu_to_le16(cap_ShortPremble); > + *(__le16 *)ie |= > cpu_to_le16(WLAN_CAPABILITY_SHORT_PREAMBLE); > if (dev_network->Privacy) > - *(__le16 *)ie |= cpu_to_le16(cap_Privacy); > + *(__le16 *)ie |= cpu_to_le16(WLAN_CAPABILITY_PRIVACY); > sz += 2; > ie += 2; > /*SSID*/ > diff --git a/drivers/staging/rtl8712/wifi.h b/drivers/staging/rtl8712/wifi.h > index b7889ac3dce9..f941efb1f4e2 100644 > --- a/drivers/staging/rtl8712/wifi.h > +++ b/drivers/staging/rtl8712/wifi.h > @@ -278,13 +278,6 @@ static inline unsigned char *get_hdr_bssid(unsigned char > *pframe) > #define AUTH_ODD_TO 0 > #define AUTH_EVEN_TO 1 > > -#define cap_ESS BIT(0) > -#define cap_IBSS BIT(1) > -#define cap_CFPollable BIT(2) > -#define cap_CFRequest BIT(3) > -#define cap_Privacy BIT(4) > -#define cap_ShortPremble BIT(5) > - > > /*- > * Below is the definition for 802.11i / 802.1x > > *-- > -- > 2.26.2 Reviewed-by: Mike Ximing Chen
RE: [PATCH 2/7] USB: serial: xr: use a table for device-specific settings
> -Original Message-
> From: Mauro Carvalho Chehab On Behalf Of Mauro
> Carvalho Chehab
>
> static int xr_probe(struct usb_serial *serial, const struct usb_device_id
> *id)
> {
> + struct xr_port_private *port_priv;
> +
> /* Don't bind to control interface */
> if (serial->interface->cur_altsetting->desc.bInterfaceNumber == 0)
> return -ENODEV;
>
> + port_priv = kzalloc(sizeof(*port_priv), GFP_KERNEL);
> + if (!port_priv)
> + return -ENOMEM;
> +
> + port_priv->model = id->driver_info;
> +
> + usb_set_serial_data(serial, port_priv);
> +
> return 0;
> }
>
> +static void xr_disconnect(struct usb_serial *serial)
> +{
> + struct xr_port_private *port_priv = usb_get_serial_data(serial);
> +
> + kfree(port_priv);
> + usb_set_serial_data(serial, 0);
Probably a nitpick, but shouldn't usb_set_serral_data() be called before
kfree()?
> +}
> +
> static const struct usb_device_id id_table[] = {
> - { USB_DEVICE(0x04e2, 0x1410) }, /* XR21V141X */
> + { USB_DEVICE(0x04e2, 0x1410), .driver_info = XR21V141X},
> { }
> };
> MODULE_DEVICE_TABLE(usb, id_table);
> @@ -566,6 +640,7 @@ static struct usb_serial_driver xr_device = {
> .id_table = id_table,
> .num_ports = 1,
> .probe = xr_probe,
> + .disconnect = xr_disconnect,
> .open = xr_open,
> .close = xr_close,
> .break_ctl = xr_break_ctl,
> --
> 2.29.2
RE: [PATCH 2/2] drivers: misc: add ripple counter driver
> -Original Message-
> From: Rasmus Villemoes
> Sent: Friday, February 26, 2021 9:14 AM
> To: Greg Kroah-Hartman ; Rob Herring
>
> Cc: devicet...@vger.kernel.org; linux-kernel@vger.kernel.org; Arnd Bergmann
> ; linux-...@vger.kernel.org; Rasmus Villemoes
>
> Subject: [PATCH 2/2] drivers: misc: add ripple counter driver
>
> The only purpose of this driver is to serve as a consumer of the input
> clock, to prevent it from being disabled by clk_disable_unused().
>
> Signed-off-by: Rasmus Villemoes
> ---
> drivers/misc/Kconfig | 7 +++
> drivers/misc/Makefile | 1 +
> drivers/misc/ripple-ctr.c | 31 +++
> 3 files changed, 39 insertions(+)
> create mode 100644 drivers/misc/ripple-ctr.c
>
> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> index f532c59bb59b..44b0b6ce42df 100644
> --- a/drivers/misc/Kconfig
> +++ b/drivers/misc/Kconfig
> @@ -445,6 +445,13 @@ config HISI_HIKEY_USB
> switching between the dual-role USB-C port and the USB-A host ports
> using only one USB controller.
>
> +config RIPPLE_CTR
> + tristate "Trivial ripple counter driver"
> + help
> + This provides a stub driver for a ripple counter, whose
> + only purpose is to request and enable the clock source
> + driving the counter.
> +
> source "drivers/misc/c2port/Kconfig"
> source "drivers/misc/eeprom/Kconfig"
> source "drivers/misc/cb710/Kconfig"
> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
> index 99b6f15a3c70..d560163068a9 100644
> --- a/drivers/misc/Makefile
> +++ b/drivers/misc/Makefile
> @@ -56,3 +56,4 @@ obj-$(CONFIG_HABANA_AI) += habanalabs/
> obj-$(CONFIG_UACCE) += uacce/
> obj-$(CONFIG_XILINX_SDFEC) += xilinx_sdfec.o
> obj-$(CONFIG_HISI_HIKEY_USB) += hisi_hikey_usb.o
> +obj-$(CONFIG_RIPPLE_CTR) += ripple-ctr.o
> diff --git a/drivers/misc/ripple-ctr.c b/drivers/misc/ripple-ctr.c
> new file mode 100644
> index ..f086eaf335df
> --- /dev/null
> +++ b/drivers/misc/ripple-ctr.c
> @@ -0,0 +1,31 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +static int ripple_ctr_probe(struct platform_device *pdev)
> +{
> + struct clk *clk;
> +
> + clk = devm_clk_get(>dev, NULL);
> + if (IS_ERR(clk))
> + return PTR_ERR(clk);
> + return clk_prepare_enable(clk);
> +}
> +
> +static const struct of_device_id ripple_ctr_ids[] = {
> + { .compatible = "linux,ripple-counter", },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, ripple_ctr_ids);
> +
> +static struct platform_driver ripple_ctr_driver = {
> + .driver = {
> + .name = "ripple-counter",
> + .of_match_table = ripple_ctr_ids,
> + },
> + .probe = ripple_ctr_probe,
> +};
> +module_platform_driver(ripple_ctr_driver);
Missing MODULE_LICENSE() tag?
See
https://www.kernel.org/doc/html/latest/process/license-rules.html#license-identifiers
> --
> 2.29.2
RE: [PATCH v10 01/20] dlb: add skeleton for DLB driver
> -Original Message- > From: gre...@linuxfoundation.org > Sent: Thursday, February 18, 2021 2:53 AM > To: Chen, Mike Ximing > Cc: net...@vger.kernel.org; Linux Kernel Mailing List ker...@vger.kernel.org>; da...@davemloft.net; k...@kernel.org; a...@arndb.de; > Williams, Dan J ; > pierre-louis.boss...@linux.intel.com > Subject: Re: [PATCH v10 01/20] dlb: add skeleton for DLB driver > > On Thu, Feb 18, 2021 at 07:34:31AM +, Chen, Mike Ximing wrote: > > > > > > > -Original Message- > > > From: Mike Ximing Chen > > > Sent: Wednesday, February 10, 2021 12:54 PM > > > To: net...@vger.kernel.org > > > Cc: da...@davemloft.net; k...@kernel.org; a...@arndb.de; > > > gre...@linuxfoundation.org; Williams, Dan J ; > pierre- > > > louis.boss...@linux.intel.com; Gage Eads > > > Subject: [PATCH v10 01/20] dlb: add skeleton for DLB driver > > > > > > diff --git a/Documentation/misc-devices/dlb.rst b/Documentation/misc- > > > devices/dlb.rst > > > new file mode 100644 > > > index ..aa79be07ee49 > > > --- /dev/null > > > +++ b/Documentation/misc-devices/dlb.rst > > > @@ -0,0 +1,259 @@ > > > +.. SPDX-License-Identifier: GPL-2.0-only > > > + > > > +=== > > > +Intel(R) Dynamic Load Balancer Overview > > > +=== > > > + > > > +:Authors: Gage Eads and Mike Ximing Chen > > > + > > > +Contents > > > + > > > + > > > +- Introduction > > > +- Scheduling > > > +- Queue Entry > > > +- Port > > > +- Queue > > > +- Credits > > > +- Scheduling Domain > > > +- Interrupts > > > +- Power Management > > > +- User Interface > > > +- Reset > > > + > > > +Introduction > > > + > > > + > > > +The Intel(r) Dynamic Load Balancer (Intel(r) DLB) is a PCIe device that > > > +provides load-balanced, prioritized scheduling of core-to-core > > > communication. > > > + > > > +Intel DLB is an accelerator for the event-driven programming model of > > > +DPDK's Event Device Library[2]. The library is used in packet processing > > > +pipelines that arrange for multi-core scalability, dynamic > > > load-balancing, and > > > +variety of packet distribution and synchronization schemes. > > > + > > > +Intel DLB device consists of queues and arbiters that connect producer > > > +cores and consumer cores. The device implements load-balanced queueing > > > features > > > +including: > > > +- Lock-free multi-producer/multi-consumer operation. > > > +- Multiple priority levels for varying traffic types. > > > +- 'Direct' traffic (i.e. multi-producer/single-consumer) > > > +- Simple unordered load-balanced distribution. > > > +- Atomic lock free load balancing across multiple consumers. > > > +- Queue element reordering feature allowing ordered load-balanced > > > distribution. > > > + > > > > Hi Jakub/Dave, > > This is a device driver for a HW core-to-core communication accelerator. It > > is > submitted > > to "linux-kernel" for a module under device/misc. Greg suggested (see > > below) that > we > > also sent it to you for any potential feedback in case there is any > > interaction with > > networking initiatives. The device is used to handle the load balancing > > among CPU > cores > > after the packets are received and forwarded to CPU. We don't think it > > interferes > > with networking operations, but would appreciate very much your > review/comment on this. > > It's the middle of the merge window, getting maintainers to review new > stuff until after 5.12-rc1 is out is going to be a very difficult thing > to do. > > In the meantime, why don't you all help out and review submitted patches > to the mailing lists for the subsystems you all are trying to get this > patch into. I know maintainers would appreciate the help, right? > > thanks, > > greg k-h Sure. I am a little new to the community and process, but will try to help. Thanks Mike
RE: [PATCH v10 01/20] dlb: add skeleton for DLB driver
> -Original Message- > From: Mike Ximing Chen > Sent: Wednesday, February 10, 2021 12:54 PM > To: net...@vger.kernel.org > Cc: da...@davemloft.net; k...@kernel.org; a...@arndb.de; > gre...@linuxfoundation.org; Williams, Dan J ; > pierre- > louis.boss...@linux.intel.com; Gage Eads > Subject: [PATCH v10 01/20] dlb: add skeleton for DLB driver > > diff --git a/Documentation/misc-devices/dlb.rst b/Documentation/misc- > devices/dlb.rst > new file mode 100644 > index ..aa79be07ee49 > --- /dev/null > +++ b/Documentation/misc-devices/dlb.rst > @@ -0,0 +1,259 @@ > +.. SPDX-License-Identifier: GPL-2.0-only > + > +=== > +Intel(R) Dynamic Load Balancer Overview > +=== > + > +:Authors: Gage Eads and Mike Ximing Chen > + > +Contents > + > + > +- Introduction > +- Scheduling > +- Queue Entry > +- Port > +- Queue > +- Credits > +- Scheduling Domain > +- Interrupts > +- Power Management > +- User Interface > +- Reset > + > +Introduction > + > + > +The Intel(r) Dynamic Load Balancer (Intel(r) DLB) is a PCIe device that > +provides load-balanced, prioritized scheduling of core-to-core communication. > + > +Intel DLB is an accelerator for the event-driven programming model of > +DPDK's Event Device Library[2]. The library is used in packet processing > +pipelines that arrange for multi-core scalability, dynamic load-balancing, > and > +variety of packet distribution and synchronization schemes. > + > +Intel DLB device consists of queues and arbiters that connect producer > +cores and consumer cores. The device implements load-balanced queueing > features > +including: > +- Lock-free multi-producer/multi-consumer operation. > +- Multiple priority levels for varying traffic types. > +- 'Direct' traffic (i.e. multi-producer/single-consumer) > +- Simple unordered load-balanced distribution. > +- Atomic lock free load balancing across multiple consumers. > +- Queue element reordering feature allowing ordered load-balanced > distribution. > + Hi Jakub/Dave, This is a device driver for a HW core-to-core communication accelerator. It is submitted to "linux-kernel" for a module under device/misc. Greg suggested (see below) that we also sent it to you for any potential feedback in case there is any interaction with networking initiatives. The device is used to handle the load balancing among CPU cores after the packets are received and forwarded to CPU. We don't think it interferes with networking operations, but would appreciate very much your review/comment on this. Thanks for your help. Mike > As this is a networking related thing, I would like you to get the >proper reviews/acks from the networking maintainers before I can take >this. > >Or, if they think it has nothing to do with networking, that's fine too, >but please do not try to route around them. > >thanks, > >greg k-h
RE: [PATCH v10 01/20] dlb: add skeleton for DLB driver
> -Original Message- > From: Dan Williams > Sent: Tuesday, February 9, 2021 11:30 AM > To: Greg KH > Cc: Chen, Mike Ximing ; Linux Kernel Mailing List > ; Arnd Bergmann ; Pierre-Louis > Bossart ; Gage Eads > > Subject: Re: [PATCH v10 01/20] dlb: add skeleton for DLB driver > > On Tue, Feb 9, 2021 at 5:36 AM Greg KH wrote: > > > > On Wed, Jan 27, 2021 at 04:56:22PM -0600, Mike Ximing Chen wrote: > > > Add basic driver functionality (load, unload, probe, and remove callbacks) > > > for the DLB driver. > > > > > > Add documentation which describes in detail the hardware, the user > > > interface, device interrupts, and the driver's power-management strategy. > > > For more details about the driver see the documentation in the patch. > > > > > > Add a DLB entry to the MAINTAINERS file. > > > > > > Signed-off-by: Gage Eads > > > Signed-off-by: Mike Ximing Chen > > > Reviewed-by: Magnus Karlsson > > > Reviewed-by: Dan Williams > > > --- > > > Documentation/misc-devices/dlb.rst | 259 +++ > > > Documentation/misc-devices/index.rst | 1 + > > > MAINTAINERS | 8 + > > > drivers/misc/Kconfig | 1 + > > > drivers/misc/Makefile| 1 + > > > drivers/misc/dlb/Kconfig | 18 ++ > > > drivers/misc/dlb/Makefile| 9 + > > > drivers/misc/dlb/dlb_hw_types.h | 32 > > > drivers/misc/dlb/dlb_main.c | 156 > > > drivers/misc/dlb/dlb_main.h | 37 > > > 10 files changed, 522 insertions(+) > > > create mode 100644 Documentation/misc-devices/dlb.rst > > > create mode 100644 drivers/misc/dlb/Kconfig > > > create mode 100644 drivers/misc/dlb/Makefile > > > create mode 100644 drivers/misc/dlb/dlb_hw_types.h > > > create mode 100644 drivers/misc/dlb/dlb_main.c > > > create mode 100644 drivers/misc/dlb/dlb_main.h > > > > > > diff --git a/Documentation/misc-devices/dlb.rst b/Documentation/misc- > devices/dlb.rst > > > new file mode 100644 > > > index ..aa79be07ee49 > > > --- /dev/null > > > +++ b/Documentation/misc-devices/dlb.rst > > > @@ -0,0 +1,259 @@ > > > +.. SPDX-License-Identifier: GPL-2.0-only > > > + > > > +=== > > > +Intel(R) Dynamic Load Balancer Overview > > > +=== > > > + > > > +:Authors: Gage Eads and Mike Ximing Chen > > > + > > > +Contents > > > + > > > + > > > +- Introduction > > > +- Scheduling > > > +- Queue Entry > > > +- Port > > > +- Queue > > > +- Credits > > > +- Scheduling Domain > > > +- Interrupts > > > +- Power Management > > > +- User Interface > > > +- Reset > > > + > > > +Introduction > > > + > > > + > > > +The Intel(r) Dynamic Load Balancer (Intel(r) DLB) is a PCIe device that > > > +provides load-balanced, prioritized scheduling of core-to-core > > > communication. > > > + > > > +Intel DLB is an accelerator for the event-driven programming model of > > > +DPDK's Event Device Library[2]. The library is used in packet processing > > > +pipelines that arrange for multi-core scalability, dynamic > > > load-balancing, and > > > +variety of packet distribution and synchronization schemes. > > > > As this is a networking related thing, I would like you to get the > > proper reviews/acks from the networking maintainers before I can take > > this. > > > > Or, if they think it has nothing to do with networking, that's fine too, > > but please do not try to route around them. > > > >thanks, > > > >greg k- > > > To be clear, I did not sense any attempt to route around networking > review as it appeared generically centered around hardware accelerated > IPC. At the same time I don't know what I don't know about how this > might interact with networking initiatives so the review trip seems > reasonable to me. OK. I have forwarded the patch set to netdev. Thanks! Mike
RE: [PATCH v9 04/20] dlb: add device ioctl layer and first three ioctls
> -Original Message- > From: Greg KH > Sent: Wednesday, January 27, 2021 9:04 AM > To: Chen, Mike Ximing > Cc: linux-kernel@vger.kernel.org; a...@arndb.de; Williams, Dan J > ; pierre-louis.boss...@linux.intel.com > Subject: Re: [PATCH v9 04/20] dlb: add device ioctl layer and first three > ioctls > > On Wed, Jan 27, 2021 at 01:59:50PM +, Chen, Mike Ximing wrote: > > > > > -Original Message- > > > From: Greg KH > > > Sent: Wednesday, January 27, 2021 7:29 AM > > > To: Chen, Mike Ximing > > > Cc: linux-kernel@vger.kernel.org; a...@arndb.de; Williams, Dan J > > > ; pierre-louis.boss...@linux.intel.com; Gage > > > Eads > > > > > > Subject: Re: [PATCH v9 04/20] dlb: add device ioctl layer and first three > > > ioctls > > > > > > On Fri, Jan 22, 2021 at 01:01:22PM -0600, Mike Ximing Chen wrote: > > > > --- /dev/null > > > > +++ b/include/uapi/linux/dlb.h > > > > @@ -0,0 +1,167 @@ > > > > +/* SPDX-License-Identifier: GPL-2.0-only */ > > > > > > As the bot points out, this is an "odd" license for a uapi .h file, are > > > you SURE about this? > > > > > > If so, I need an Intel lawyer's signed-off-by on it as well, so we know > > > to talk to in the future about it. > > > > > > thanks, > > > > > > greg k-h > > > > Sorry, it should be "GPL-2.0-only WITH Linux-syscall-note". > > Should I correct it and submit a new patch set (v10) now, or wait for more > feedback on the current patch set? > > Please consult your corporate lawyers when picking licenses for your > kernel files. I doubt they want me telling you what to do :) > > good luck! > > greg k-h OK. I submitted a new patch set (v10) with the fix on license. Thanks for the help. Mike
RE: [PATCH v9 04/20] dlb: add device ioctl layer and first three ioctls
> -Original Message- > From: Greg KH > Sent: Wednesday, January 27, 2021 7:29 AM > To: Chen, Mike Ximing > Cc: linux-kernel@vger.kernel.org; a...@arndb.de; Williams, Dan J > ; pierre-louis.boss...@linux.intel.com; Gage Eads > > Subject: Re: [PATCH v9 04/20] dlb: add device ioctl layer and first three > ioctls > > On Fri, Jan 22, 2021 at 01:01:22PM -0600, Mike Ximing Chen wrote: > > --- /dev/null > > +++ b/include/uapi/linux/dlb.h > > @@ -0,0 +1,167 @@ > > +/* SPDX-License-Identifier: GPL-2.0-only */ > > As the bot points out, this is an "odd" license for a uapi .h file, are > you SURE about this? > > If so, I need an Intel lawyer's signed-off-by on it as well, so we know > to talk to in the future about it. > > thanks, > > greg k-h Sorry, it should be "GPL-2.0-only WITH Linux-syscall-note". Should I correct it and submit a new patch set (v10) now, or wait for more feedback on the current patch set? Thanks Mike
RE: [PATCH v8 03/20] dlb: add resource and device initialization
> -Original Message- > From: Greg KH > Sent: Thursday, January 7, 2021 2:40 PM > To: Chen, Mike Ximing > Cc: linux-kernel@vger.kernel.org; a...@arndb.de; Williams, Dan J > ; pierre-louis.boss...@linux.intel.com; Gage Eads > > Subject: Re: [PATCH v8 03/20] dlb: add resource and device initialization > > On Mon, Jan 04, 2021 at 08:58:22PM -0600, Mike Ximing Chen wrote: > > Introduce dlb_bitmap_* functions, a thin convenience layer wrapping the > > Linux bitmap interfaces, used by the bitmaps in the dlb hardware types. > > No, you created custom #defines: > > > --- a/drivers/misc/dlb/dlb_hw_types.h > > +++ b/drivers/misc/dlb/dlb_hw_types.h > > @@ -5,6 +5,15 @@ > > #define __DLB_HW_TYPES_H > > > > #include > > +#include > > + > > +#include "dlb_bitmap.h" > > + > > +#define BITS_SET(x, val, mask) (x = ((x) & ~(mask)) \ > > +| (((val) << (mask##_LOC)) & (mask))) > > +#define BITS_CLR(x, mask) ((x) &= ~(mask)) > > +#define BIT_SET(x, mask)((x) |= (mask)) > > +#define BITS_GET(x, mask) (((x) & (mask)) >> (mask##_LOC)) > > We have functions for this, use them, don't create custom macros for > them. Use the Linux functions please. > > thanks, > > greg k-h FIELD_GET(_mask, _val) and FIELD_PREP(_mask, _val) in include/linux/bitfield.h are similar to our BITS_GET() and BITS_SET(). However in our case, mask##_LOC is a known constant defined in dlb_regs.h. So we don't need to use _buildin_ffs(mask) to calculate the location of mask as FIELD_GET() and FIELD_PREP() do. We can still use FIELD_GET and FIELD_PREP, but our macros are a little more efficient. Would it be OK to use them? Thanks Mike
RE: [PATCH v8 04/20] dlb: add device ioctl layer and first three ioctls
> -Original Message-
> From: Greg KH
> Sent: Saturday, January 9, 2021 3:34 AM
> To: Chen, Mike Ximing
> Cc: linux-kernel@vger.kernel.org; a...@arndb.de; Williams, Dan J
> ; pierre-louis.boss...@linux.intel.com
> Subject: Re: [PATCH v8 04/20] dlb: add device ioctl layer and first three
> ioctls
>
> On Sat, Jan 09, 2021 at 07:49:24AM +, Chen, Mike Ximing wrote:
> > > > +static int dlb_ioctl_arg_size[NUM_DLB_CMD] = {
> > > > + sizeof(struct dlb_get_device_version_args),
> > > > + sizeof(struct dlb_create_sched_domain_args),
> > > > + sizeof(struct dlb_get_num_resources_args)
> > >
> > > That list.
> > >
> > > Ugh, no. that's no way to write maintainable code that you will be able
> > > to understand in 2 years.
> > >
> >
> > dlb_ioctl() was implemented with switch-case and real function calls
> > previously.
> > We changed to the table/list implementation during a code restructure. I
> > will move
> > back to the old implementation.
>
> Who said to change this? And why did they say that? Please go back to
> those developers and point them at this thread so that doesn't happen
> again...
>
It was my fault:(. Will make sure it doesn't happen again.
>
> > > > +#define DLB_DEVICE_VERSION(x) (((x) >> 8) & 0xFF)
> > > > +#define DLB_DEVICE_REVISION(x) ((x) & 0xFF)
> > > > +
> > > > +enum dlb_revisions {
> > > > + DLB_REV_A0 = 0,
> > >
> > > What is a "revision" and why do you care about it?
> >
> > This is for different revisions of hardware. Each revision of hardware may
> > have a
> slight different configuration/feature.
>
> So what does this mean? What are you going to do based on it?
>
For now, it is primarily informational to user. Different HW revision may have
different features enabled. For example, certain features may not be available
in the earlier revision A0 and A1, but are enabled in the later revisions (B0,
C0, etc). User applications that depends on these features may fail on A0/A1
devices. They can check the device revision/version to confirm that the failure
is expected. Users can also implement workarounds to avoid failures.
Thanks
Mike
RE: [PATCH v8 04/20] dlb: add device ioctl layer and first three ioctls
> -Original Message- > From: Greg KH > Sent: Thursday, January 7, 2021 2:42 PM > To: Chen, Mike Ximing > Cc: linux-kernel@vger.kernel.org; a...@arndb.de; Williams, Dan J > ; pierre-louis.boss...@linux.intel.com; Gage Eads > > Subject: Re: [PATCH v8 04/20] dlb: add device ioctl layer and first three > ioctls > > > +/* [7:0]: device revision, [15:8]: device version */ > > +#define DLB_SET_DEVICE_VERSION(ver, rev) (((ver) << 8) | (rev)) > > + > > +static int > > +dlb_ioctl_get_device_version(struct dlb *dlb __attribute__((unused)), > > We don't use __attribute__((unused)) for function variables in Linux. > Please remove and tell whatever operating system you ported this from to > get with the times :) > > thanks, > > greg k-h OK. Will remove __attribute__((unused)) in the patch set. Thanks! Mike
RE: [PATCH v8 04/20] dlb: add device ioctl layer and first three ioctls
> -Original Message-
> From: Greg KH
> Sent: Thursday, January 7, 2021 2:51 PM
> To: Chen, Mike Ximing
> Cc: linux-kernel@vger.kernel.org; a...@arndb.de; Williams, Dan J
> ; pierre-louis.boss...@linux.intel.com; Gage Eads
>
> Subject: Re: [PATCH v8 04/20] dlb: add device ioctl layer and first three
> ioctls
>
> On Mon, Jan 04, 2021 at 08:58:23PM -0600, Mike Ximing Chen wrote:
> > Introduce the dlb device ioctl layer and the first three ioctls: query
> > device version, query available resources, and create a scheduling domain.
> > Also introduce the user-space interface file dlb_user.h.
> >
> > The device version query is designed to allow each DLB device version/type
> > to have its own unique ioctl API through the /dev/dlb%d node. Each such API
> > would share in common the device version command as its first command, and
> > all subsequent commands can be unique to the particular device.
> >
> > The hardware operation for scheduling domain creation will be added in a
> > subsequent commit.
> >
> > Signed-off-by: Gage Eads
> > Signed-off-by: Mike Ximing Chen
> > Reviewed-by: Magnus Karlsson
> > Reviewed-by: Dan Williams
> > ---
> > .../userspace-api/ioctl/ioctl-number.rst | 1 +
> > drivers/misc/dlb/Makefile | 2 +-
> > drivers/misc/dlb/dlb_bitmap.h | 32
> > drivers/misc/dlb/dlb_ioctl.c | 119 +
> > drivers/misc/dlb/dlb_ioctl.h | 11 ++
> > drivers/misc/dlb/dlb_main.c | 3 +
> > drivers/misc/dlb/dlb_main.h | 7 +
> > drivers/misc/dlb/dlb_pf_ops.c | 21 +++
> > drivers/misc/dlb/dlb_resource.c | 63 +++
> > drivers/misc/dlb/dlb_resource.h | 5 +
> > include/uapi/linux/dlb.h | 166 ++
> > 11 files changed, 429 insertions(+), 1 deletion(-)
> > create mode 100644 drivers/misc/dlb/dlb_ioctl.c
> > create mode 100644 drivers/misc/dlb/dlb_ioctl.h
> > create mode 100644 include/uapi/linux/dlb.h
> >
> > diff --git a/Documentation/userspace-api/ioctl/ioctl-number.rst
> b/Documentation/userspace-api/ioctl/ioctl-number.rst
> > index 55a2d9b2ce33..afca043d59f8 100644
> > --- a/Documentation/userspace-api/ioctl/ioctl-number.rst
> > +++ b/Documentation/userspace-api/ioctl/ioctl-number.rst
> > @@ -241,6 +241,7 @@ Code Seq#Include File
> Comments
> > 'h' 00-7F
> > conflict! Charon filesystem
> >
> > <mailto:zap...@interlan.net>
> > 'h' 00-1F linux/hpet.h
> > conflict!
> > +'h' 00-1F uapi/linux/dlb.h
> > conflict!
> > 'h' 80-8F fs/hfsplus/ioctl.c
> > 'i' 00-3F linux/i2o-dev.h
> > conflict!
> > 'i' 0B-1F linux/ipmi.h
> > conflict!
> > diff --git a/drivers/misc/dlb/Makefile b/drivers/misc/dlb/Makefile
> > index 8a49ea5fd752..aaafb3086d8d 100644
> > --- a/drivers/misc/dlb/Makefile
> > +++ b/drivers/misc/dlb/Makefile
> > @@ -7,4 +7,4 @@
> > obj-$(CONFIG_INTEL_DLB) := dlb.o
> >
> > dlb-objs := dlb_main.o
> > -dlb-objs += dlb_pf_ops.o dlb_resource.o
> > +dlb-objs += dlb_pf_ops.o dlb_resource.o dlb_ioctl.o
> > diff --git a/drivers/misc/dlb/dlb_bitmap.h b/drivers/misc/dlb/dlb_bitmap.h
> > index fb3ef52a306d..3ea78b42c79f 100644
> > --- a/drivers/misc/dlb/dlb_bitmap.h
> > +++ b/drivers/misc/dlb/dlb_bitmap.h
> > @@ -73,4 +73,36 @@ static inline void dlb_bitmap_free(struct dlb_bitmap
> *bitmap)
> > kfree(bitmap);
> > }
> >
> > +/**
> > + * dlb_bitmap_longest_set_range() - returns longest contiguous range of set
> > + * bits
> > + * @bitmap: pointer to dlb_bitmap structure.
> > + *
> > + * Return:
> > + * Returns the bitmap's longest contiguous range of set bits upon success,
> > + * <0 otherwise.
> > + *
> > + * Errors:
> > + * EINVAL - bitmap is NULL or is uninitialized.
> > + */
> > +static inline int dlb_bitmap_longest_set_range(struct dlb_bitmap *bitmap)
> > +{
> > + int max_len, len;
> > + int start, end;
> > +
> > + if (!bitmap || !bitmap->map)
> > + return -EINVAL;
> > +
> > + if (bitmap_weight(bitmap->map, bitmap->l
RE: [PATCH v8 04/20] dlb: add device ioctl layer and first three ioctls
> -Original Message- > From: Greg KH > Sent: Thursday, January 7, 2021 2:41 PM > To: Chen, Mike Ximing > Cc: linux-kernel@vger.kernel.org; a...@arndb.de; Williams, Dan J > ; pierre-louis.boss...@linux.intel.com; Gage Eads > > Subject: Re: [PATCH v8 04/20] dlb: add device ioctl layer and first three > ioctls > > > diff --git a/Documentation/userspace-api/ioctl/ioctl-number.rst > b/Documentation/userspace-api/ioctl/ioctl-number.rst > > index 55a2d9b2ce33..afca043d59f8 100644 > > --- a/Documentation/userspace-api/ioctl/ioctl-number.rst > > +++ b/Documentation/userspace-api/ioctl/ioctl-number.rst > > @@ -241,6 +241,7 @@ Code Seq#Include File > Comments > > 'h' 00-7F > > conflict! Charon filesystem > > > > <mailto:zap...@interlan.net> > > 'h' 00-1F linux/hpet.h > > conflict! > > +'h' 00-1F uapi/linux/dlb.h > > conflict! > > Why are you taking a range that you know there is a conflict for? OK. We will switch to a unused magic number and range, probably 0x81 00-1F. Thanks
RE: [PATCH v8 01/20] dlb: add skeleton for DLB driver
> -Original Message-
> From: Greg KH
> Sent: Thursday, January 7, 2021 2:36 PM
> To: Chen, Mike Ximing
> Cc: linux-kernel@vger.kernel.org; a...@arndb.de; Williams, Dan J
> ; pierre-louis.boss...@linux.intel.com; Gage Eads
>
> Subject: Re: [PATCH v8 01/20] dlb: add skeleton for DLB driver
>
> On Mon, Jan 04, 2021 at 08:58:20PM -0600, Mike Ximing Chen wrote:
> > +static int dlb_probe(struct pci_dev *pdev,
> > +const struct pci_device_id *pdev_id)
> > +{
> > + struct dlb *dlb;
> > + int ret;
> > +
> > + dlb = devm_kzalloc(>dev, sizeof(*dlb), GFP_KERNEL);
> > + if (!dlb)
> > + return -ENOMEM;
> > +
> > + pci_set_drvdata(pdev, dlb);
> > +
> > + dlb->pdev = pdev;
> > +
> > + spin_lock(_ids_lock);
> > + dlb->id = idr_alloc(_ids,
> > + (void *)dlb,
> > + 0,
> > + DLB_MAX_NUM_DEVICES - 1,
> > + GFP_KERNEL);
> > + spin_unlock(_ids_lock);
> > +
> > + if (dlb->id < 0) {
> > + dev_err(>dev, "probe: device ID allocation failed\n");
> > +
> > + ret = dlb->id;
> > + goto alloc_id_fail;
> > + }
> > +
> > + ret = pcim_enable_device(pdev);
> > + if (ret != 0) {
> > + dev_err(>dev, "pcim_enable_device() returned %d\n", ret);
> > +
> > + goto pci_enable_device_fail;
> > + }
> > +
> > + ret = pcim_iomap_regions(pdev,
> > +(1U << DLB_CSR_BAR) | (1U << DLB_FUNC_BAR),
> > +"dlb");
> > + if (ret != 0) {
> > + dev_err(>dev,
> > + "pcim_iomap_regions(): returned %d\n", ret);
> > +
> > + goto pci_enable_device_fail;
> > + }
> > +
> > + pci_set_master(pdev);
> > +
> > + if (pci_enable_pcie_error_reporting(pdev))
> > + dev_info(>dev, "[%s()] Failed to enable AER\n", __func__);
>
> Shouldn't that be dev_err() and you fail here?
>
Some of our earlier devices/platforms do not support AER.
pci_enable_pcie_error_reporting() fails,
everything else works fine. Will change to dev_err() when the old HWs are
phased out.
> And no need for __func__ please, the driver name and device is listed,
> that's all that is necessary.
Will remove __func__.
Thanks
>
> thanks,
>
> greg k-h
